chore: MESSAGE_MAX_PARSE_LENGTH environment variable#40306
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a configurable Message_MaxMarkdownParseLength setting and client hook. When a message's text length exceeds the configured limit, markdown parsing is skipped and content is treated as plain text. The limit is enforced server-side and propagated through normalization, body extraction, rendering components, and tests. ChangesMarkdown parse-length feature
Sequence DiagramsequenceDiagram
participant Component as Message Component
participant Hook as useMaxMarkdownParseLength
participant Setting as Server Settings
participant Processor as useMessageBody / useNormalizedMessage
participant Parser as Markdown Parser
participant Renderer as UI Renderer
Component->>Hook: request limit
Hook->>Setting: read Message_MaxMarkdownParseLength
Setting-->>Hook: return limit (or 0/disabled)
Hook-->>Component: return numeric limit
Component->>Processor: process message + maxMarkdownParseLength
alt message length > limit
Processor->>Renderer: return plain-text AST (single paragraph)
Renderer->>Renderer: render plain text
else message length ≤ limit
Processor->>Parser: parse markdown
Parser-->>Processor: return AST
Processor->>Renderer: return AST
Renderer->>Renderer: render formatted content
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40306 +/- ##
===========================================
+ Coverage 69.63% 69.70% +0.06%
===========================================
Files 3325 3323 -2
Lines 122799 122782 -17
Branches 21881 21894 +13
===========================================
+ Hits 85517 85590 +73
+ Misses 33934 33854 -80
+ Partials 3348 3338 -10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
✅ Layne — scan passed No security issues found on latest push. |
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent the client-side message parsing pipeline from causing severe performance issues (up to “breaking” the workspace) when handling very large message payloads, by introducing a maximum parse size and bypassing parsing/rendering paths when the content exceeds that threshold.
Changes:
- Introduces a
useMaxMessageParseSizehook based on theMessage_MaxAllowedSizesetting, capped by a new hard limit constant. - Updates message normalization/body hooks to skip parsing when
msg.lengthexceeds the max parse size (falling back to raw text / a minimal AST). - Wires the new limit through message rendering variants and quote attachment rendering, and adds unit tests for the new behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx | Adds maxMessageParseSize parameter and bypasses parsing for oversized messages. |
| apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts | Adds unit tests validating the new bypass behavior and parser invocation conditions. |
| apps/meteor/client/lib/constants.ts | Introduces MESSAGE_PARSE_HARD_LIMIT constant used to cap parsing size. |
| apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx | Passes max parse size into normalization to avoid parsing oversized messages in thread view. |
| apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx | Passes max parse size into normalization to avoid parsing oversized messages in room view. |
| apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx | Uses max parse size when computing preview body to avoid parsing oversized parent messages. |
| apps/meteor/client/components/message/hooks/useNormalizedMessage.ts | Adds max-size bypass; returns minimal md AST for oversized messages. |
| apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts | Adds tests ensuring parsing is skipped for oversized messages and attachments are preserved. |
| apps/meteor/client/components/message/hooks/useMaxMessageParseSize.ts | New hook computing max parse size from settings with a hard cap. |
| apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsx | Avoids rendering parsed md for oversized quote attachment text. |
| apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx | Adds tests for quote attachment md rendering vs plain-text fallback based on size. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/i18n/src/locales/en.i18n.json (1)
3520-3521: Minor wording: prefer explicit “markdown parsing disabled” phrasing.The text already says “without markdown parsing”, which is close, but you may want to standardize to “markdown parsing disabled” / “markdown rendering disabled” so it matches the implementation wording and reduces ambiguity between “parsing” vs “rendering.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/src/locales/en.i18n.json` around lines 3520 - 3521, Update the translation value for "Message_MaxAllowedSize_Description" to use the standardized phrasing "markdown parsing disabled" or "markdown rendering disabled" (whichever matches the implementation) so it reads like: Maximum number of characters allowed per message. Messages exceeding this limit will be displayed as plain text with markdown parsing disabled.; locate the "Message_MaxAllowedSize_Description" key in packages/i18n/src/locales/en.i18n.json and replace the current sentence fragment "without markdown parsing" with the chosen standardized phrase to match the code behavior.apps/meteor/client/components/message/hooks/useMaxMessageParseSize.ts (1)
6-10: Remove implementation block comment in this TS hook.Line 6-10 adds implementation comments in code; this repo guideline asks to avoid those in TS/JS implementation files.
As per coding guidelines: "`**/*.{ts,tsx,js}`: ... Avoid code comments in the implementation".Proposed diff
-/** - * Returns the maximum allowed size for message parsing. - * Uses Math.min to ensure it never exceeds the hard limit to avoid performance issues. - * Always returns a number, never null or undefined. - */ export const useMaxMessageParseSize = (): number => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/components/message/hooks/useMaxMessageParseSize.ts` around lines 6 - 10, Remove the implementation block comment at the top of useMaxMessageParseSize.ts (the multi-line comment describing return behavior on lines 6-10); per repo guidelines, delete this inline implementation comment and leave only any necessary exported symbol (useMaxMessageParseSize) and its code, ensuring no other implementation-level comments remain in that hook file.apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts (1)
35-63: Add an equality boundary test (length === maxMessageParseSize).A dedicated boundary case would lock the intended strict
>behavior and prevent off-by-one regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts` around lines 35 - 63, Add a boundary test for msg length === maxMessageParseSize: in useMessageBody.spec.ts add a test (e.g., "should call parser when msg length equals maxMessageParseSize") that creates a message whose msg length equals the provided max, includes an md array, mocks mockParseMessageTextToAstMarkdown to return the parsed md, calls renderHook(() => useMessageBody(message as any, maxSize)), and asserts mockParseMessageTextToAstMarkdown was called with the message and that result.current equals the md; reference useMessageBody and mockParseMessageTextToAstMarkdown to locate where to add the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx`:
- Around line 36-43: The test currently only asserts MessageContentBody is
absent; update the 'renders plain text when text exceeds maxMessageParseSize'
test in QuoteAttachment.spec.tsx to also assert the fallback plain text is
rendered by checking for the longText (the 'a'.repeat(101) value) or its visible
text node, e.g., assert that the longText from the local longText variable (or
the attachment.text on the attachment built from baseAttachment) is present in
the document; keep the existing check for absence of MessageContentBody to
ensure parser is not used and add a presence assertion for the plain text
fallback rendered by QuoteAttachment.
In `@apps/meteor/client/components/message/hooks/useNormalizedMessage.ts`:
- Around line 89-91: The oversized-message branch currently passes
message.attachments directly into normalizeAttachments causing in-place mutation
of the input; fix by deep-cloning message.attachments before calling
normalizeAttachments (e.g., create a clonedAttachments variable from
message.attachments using a safe deep clone) and pass that clone into
normalizeAttachments so the hook does not mutate the original message object;
ensure references in this change point to normalizeAttachments and the
attachments property used in useNormalizedMessage.
In `@packages/i18n/src/locales/en.i18n.json`:
- Around line 3519-3521: Update the admin-facing string
Message_MaxAllowedSize_Description to mention the hard ceiling imposed by
MESSAGE_PARSE_HARD_LIMIT (20000 characters) so admins know that even if they set
a higher value, markdown rendering will be disabled for messages above 20000
characters; edit the value of Message_MaxAllowedSize_Description to clearly
state both the configurable limit and the hard limit (20000) and that messages
exceeding the hard limit will be treated as plain text without markdown parsing.
---
Nitpick comments:
In `@apps/meteor/client/components/message/hooks/useMaxMessageParseSize.ts`:
- Around line 6-10: Remove the implementation block comment at the top of
useMaxMessageParseSize.ts (the multi-line comment describing return behavior on
lines 6-10); per repo guidelines, delete this inline implementation comment and
leave only any necessary exported symbol (useMaxMessageParseSize) and its code,
ensuring no other implementation-level comments remain in that hook file.
In `@apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts`:
- Around line 35-63: Add a boundary test for msg length === maxMessageParseSize:
in useMessageBody.spec.ts add a test (e.g., "should call parser when msg length
equals maxMessageParseSize") that creates a message whose msg length equals the
provided max, includes an md array, mocks mockParseMessageTextToAstMarkdown to
return the parsed md, calls renderHook(() => useMessageBody(message as any,
maxSize)), and asserts mockParseMessageTextToAstMarkdown was called with the
message and that result.current equals the md; reference useMessageBody and
mockParseMessageTextToAstMarkdown to locate where to add the test.
In `@packages/i18n/src/locales/en.i18n.json`:
- Around line 3520-3521: Update the translation value for
"Message_MaxAllowedSize_Description" to use the standardized phrasing "markdown
parsing disabled" or "markdown rendering disabled" (whichever matches the
implementation) so it reads like: Maximum number of characters allowed per
message. Messages exceeding this limit will be displayed as plain text with
markdown parsing disabled.; locate the "Message_MaxAllowedSize_Description" key
in packages/i18n/src/locales/en.i18n.json and replace the current sentence
fragment "without markdown parsing" with the chosen standardized phrase to match
the code behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3689c07-5010-4e2e-8a18-0dd67fb4835a
📒 Files selected for processing (14)
.changeset/cool-flowers-stop.mdapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useMaxMessageParseSize.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.tsapps/meteor/client/components/message/variants/ThreadMessagePreview.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/lib/constants.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/server/settings/message.tspackages/i18n/src/locales/en.i18n.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/settings/message.tsapps/meteor/client/components/message/variants/ThreadMessagePreview.tsxapps/meteor/client/components/message/hooks/useMaxMessageParseSize.tsapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/lib/constants.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
🧠 Learnings (40)
📓 Common learnings
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 38623
File: apps/meteor/app/lib/server/functions/cleanRoomHistory.ts:146-149
Timestamp: 2026-04-18T12:32:53.425Z
Learning: In `apps/meteor/app/lib/server/functions/cleanRoomHistory.ts` (PR `#38623`), the read-receipt cleanup (both `ReadReceipts.removeByMessageIds` and `ReadReceiptsArchive.removeByMessageIds`) is intentionally only performed in the limited prune path (`limit && selectedMessageIds`). The unlimited/delete-all path (`limit === 0`) deliberately skips cleaning up orphaned read receipts in both hot and cold storage — this is by design. Do not flag this as a bug or missing cleanup in future reviews.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/server/settings/message.tsapps/meteor/client/components/message/variants/ThreadMessagePreview.tsxapps/meteor/client/components/message/hooks/useMaxMessageParseSize.tsapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/lib/constants.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tspackages/i18n/src/locales/en.i18n.jsonapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/server/settings/message.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/server/settings/message.ts
📚 Learning: 2026-04-10T22:42:05.539Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 40075
File: apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx:69-71
Timestamp: 2026-04-10T22:42:05.539Z
Learning: In `apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx`, the submit handler converts an empty/whitespace-only description to `undefined` (`description?.trim() || undefined`) intentionally. All downstream image-rendering components (`AttachmentImage`, `ImagePreview`, `ImageItem`, `ImageGallery`) default `undefined` alt to `''`, so the `<img alt="">` attribute is always present. Do not flag this `undefined` conversion as a bug preventing alt text from being cleared.
Applied to files:
apps/meteor/server/settings/message.tsapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/server/settings/message.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/server/settings/message.tsapps/meteor/client/components/message/hooks/useMaxMessageParseSize.tsapps/meteor/client/lib/constants.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/server/settings/message.tsapps/meteor/client/components/message/hooks/useMaxMessageParseSize.tsapps/meteor/client/lib/constants.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
.changeset/cool-flowers-stop.md
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
.changeset/cool-flowers-stop.mdapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
.changeset/cool-flowers-stop.mdapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
.changeset/cool-flowers-stop.md
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
.changeset/cool-flowers-stop.mdapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/cool-flowers-stop.md
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/client/components/message/variants/ThreadMessagePreview.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/client/components/message/variants/ThreadMessagePreview.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/components/message/variants/ThreadMessagePreview.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/components/message/hooks/useMaxMessageParseSize.tsapps/meteor/client/lib/constants.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-04-17T23:32:07.223Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/messages.ts:348-352
Timestamp: 2026-04-17T23:32:07.223Z
Learning: In `apps/meteor/app/apps/server/converters/messages.ts`, the `timestamp` handler inside `_convertAttachmentsToApp` uses a non-null assertion (`attachment.ts!`) intentionally. The `ts` property on `MessageAttachment` is optional only to accommodate MessageAttachment creation-time scenarios; by the time `_convertAttachmentsToApp` is called, the message has already been stored and the attachment is guaranteed to have a `ts` value. Do not flag this non-null assertion as unsafe during code review.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/lib/constants.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2026-04-28T14:08:43.140Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 40105
File: apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.ts:54-67
Timestamp: 2026-04-28T14:08:43.140Z
Learning: In `apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.ts`, setting `isJumpingToMessage.current = true` before the guard clauses (RoomHistoryManager.isLoading check, message not found check) is intentional. The flag means "a jump is pending/in progress" and must stay `true` through all intermediate early-return paths (loading, unresolved message, etc.) so that downstream scroll and load behavior is suppressed while waiting for the jump conditions to be satisfied. Do not flag this as a "flag stuck true" bug.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
📚 Learning: 2026-04-18T12:32:53.425Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 38623
File: apps/meteor/app/lib/server/functions/cleanRoomHistory.ts:146-149
Timestamp: 2026-04-18T12:32:53.425Z
Learning: In `apps/meteor/app/lib/server/functions/cleanRoomHistory.ts` (PR `#38623`), the read-receipt cleanup (both `ReadReceipts.removeByMessageIds` and `ReadReceiptsArchive.removeByMessageIds`) is intentionally only performed in the limited prune path (`limit && selectedMessageIds`). The unlimited/delete-all path (`limit === 0`) deliberately skips cleaning up orphaned read receipts in both hot and cold storage — this is by design. Do not flag this as a bug or missing cleanup in future reviews.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx
📚 Learning: 2026-03-11T18:17:53.972Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/processMessageUploads.ts:112-119
Timestamp: 2026-03-11T18:17:53.972Z
Learning: In `apps/meteor/client/lib/chats/flows/processMessageUploads.ts`, when sending multiple file uploads, each file is confirmed via its own `/rooms.mediaConfirm/${rid}/${fileId}` call and produces a separate message. Only the first file's confirm payload carries the composed message text (`msg`); all subsequent files receive `msg: ''`. This one-message-per-file behavior is intentional by design — do not flag it as a bug or suggest batching into a single message.
Applied to files:
apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsx
📚 Learning: 2026-02-23T17:53:18.785Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-14T14:58:58.834Z
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2026-04-17T18:33:27.211Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:27.211Z
Learning: In RocketChat/Rocket.Chat (`apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`), `executeBlockActionHandler` invocations originating from a **modal** surface intentionally do NOT include a `block_action_room` (room property) in the interaction payload. Modals are not scoped to a room, so no room id is available in that context. Do not flag the absence of a room assertion in the modal block-action test as a missing coverage bug; instead, document it explicitly with a `test.step` asserting the room entry is `undefined`.
Applied to files:
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
🔇 Additional comments (12)
apps/meteor/server/settings/message.ts (1)
168-172: No issues found. The i18n keyMessage_MaxAllowedSize_Descriptionexists inpackages/i18n/src/locales/en.i18n.jsonwith appropriate content, and the reference in the setting is correct..changeset/cool-flowers-stop.md (1)
2-6: Changeset scope and release type look correct.Patch bumps and summary text are aligned with the runtime-safe parsing limit fix in this PR.
apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx (1)
34-57: Good propagation of parse-size guard into thread previews.Passing
maxMessageParseSizeintouseMessageBodykeeps preview rendering behavior consistent with the new hard-limit strategy.apps/meteor/client/lib/constants.ts (1)
6-6: Hard-limit constant extraction is clean and reusable.Centralizing the 20k ceiling in
constants.tsmakes the parser guard easy to reuse and maintain.apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx (1)
21-48: Room content wiring is consistent with the size-limited normalization design.This correctly applies the max parse size at the normalization entry point for room messages.
apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsx (1)
16-80: Quote attachment fallback behavior is aligned with the hard-limit strategy.The size-gated branch cleanly avoids markdown-body rendering when quote text exceeds the parse limit.
apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx (1)
18-41: Thread content correctly adopts bounded normalization.This keeps thread rendering consistent with the new parse-size enforcement path.
apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts (1)
35-67: Targeted regression coverage looks solid.The tests validate both branches of the size gate and assert the non-parsing fallback behavior effectively.
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx (1)
5-34: Good branch setup for parse-size gating.The mocks and the within-limit assertion cleanly validate the parse-enabled path.
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts (1)
35-63: Coverage of the three control-flow branches looks solid.The tests correctly verify parser bypass on oversized messages, parser invocation for md within limit, and raw-msg return without md.
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx (1)
9-20: The parse-bypass guard and memo dependency update are correct.This implements the oversized-message short-circuit cleanly and keeps memoization aligned with the new input.
Also applies to: 46-46
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts (1)
60-60: Hard-limit bypass logic and dependency wiring are well integrated.The new signature, plain-text md fallback, and memo dependency update align with the parse-protection goal.
Also applies to: 80-88, 106-115
Message_MaxMarkdownParseLength setting
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.ts (1)
4-8: ⚡ Quick winRemove implementation docblock to match repo style rule.
Please drop this inline block comment and keep the hook self-explanatory through naming and logic.
Suggested cleanup
-/** - * Returns the maximum number of characters a message can have for markdown parsing. - * Returns Infinity when the setting is 0 or negative, meaning the limit is disabled - * and all messages will be parsed regardless of length. - */ export const useMaxMarkdownParseLength = (): number => {As per coding guidelines
**/*.{ts,tsx,js}: "Avoid code comments in the implementation".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.ts` around lines 4 - 8, Remove the implementation docblock at the top of the useMaxMarkdownParseLength hook file; leave the hook declaration and its logic intact (e.g., the useMaxMarkdownParseLength function) so the name and code alone describe behavior, ensuring no other inline explanatory comments remain in that file to comply with the repo's "Avoid code comments in the implementation" rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.ts`:
- Around line 4-8: Remove the implementation docblock at the top of the
useMaxMarkdownParseLength hook file; leave the hook declaration and its logic
intact (e.g., the useMaxMarkdownParseLength function) so the name and code alone
describe behavior, ensuring no other inline explanatory comments remain in that
file to comply with the repo's "Avoid code comments in the implementation" rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c980574-5ebd-4e6b-a0aa-3d72961db50e
📒 Files selected for processing (13)
.changeset/cool-flowers-stop.mdapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.tsxapps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.tsapps/meteor/client/components/message/variants/ThreadMessagePreview.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/server/settings/message.tspackages/i18n/src/locales/en.i18n.json
✅ Files skipped from review due to trivial changes (3)
- apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx
- .changeset/cool-flowers-stop.md
- packages/i18n/src/locales/en.i18n.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/meteor/server/settings/message.ts
- apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsx
- apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 📦 Track Image Sizes
- GitHub Check: 🔎 Code Check / Code Lint
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.tsapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
🧠 Learnings (46)
📓 Common learnings
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 38623
File: apps/meteor/app/lib/server/functions/cleanRoomHistory.ts:146-149
Timestamp: 2026-04-18T12:32:53.425Z
Learning: In `apps/meteor/app/lib/server/functions/cleanRoomHistory.ts` (PR `#38623`), the read-receipt cleanup (both `ReadReceipts.removeByMessageIds` and `ReadReceiptsArchive.removeByMessageIds`) is intentionally only performed in the limited prune path (`limit && selectedMessageIds`). The unlimited/delete-all path (`limit === 0`) deliberately skips cleaning up orphaned read receipts in both hot and cold storage — this is by design. Do not flag this as a bug or missing cleanup in future reviews.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.tsapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-04-29T20:06:34.862Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40268
File: apps/meteor/client/startup/incomingMessages.ts:21-25
Timestamp: 2026-04-29T20:06:34.862Z
Learning: In `apps/meteor/client/startup/incomingMessages.ts`, the `Messages.state.update` predicate that strips `ignored` from records when `'ignored' in sub` is false (i.e., the subscription update has no `ignored` field) is intentional. Absence of `ignored` in a `subscriptions-changed` event means the user's ignore list is empty/reset, so clearing all existing `ignored` flags on messages for that room is the correct behavior. Do not flag this as an unintentional ignored-state reset on unrelated subscription updates.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx
📚 Learning: 2026-04-18T12:32:53.425Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 38623
File: apps/meteor/app/lib/server/functions/cleanRoomHistory.ts:146-149
Timestamp: 2026-04-18T12:32:53.425Z
Learning: In `apps/meteor/app/lib/server/functions/cleanRoomHistory.ts` (PR `#38623`), the read-receipt cleanup (both `ReadReceipts.removeByMessageIds` and `ReadReceiptsArchive.removeByMessageIds`) is intentionally only performed in the limited prune path (`limit && selectedMessageIds`). The unlimited/delete-all path (`limit === 0`) deliberately skips cleaning up orphaned read receipts in both hot and cold storage — this is by design. Do not flag this as a bug or missing cleanup in future reviews.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-04-10T22:42:05.539Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 40075
File: apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx:69-71
Timestamp: 2026-04-10T22:42:05.539Z
Learning: In `apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx`, the submit handler converts an empty/whitespace-only description to `undefined` (`description?.trim() || undefined`) intentionally. All downstream image-rendering components (`AttachmentImage`, `ImagePreview`, `ImageItem`, `ImageGallery`) default `undefined` alt to `''`, so the `<img alt="">` attribute is always present. Do not flag this `undefined` conversion as a bug preventing alt text from being cleared.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-04-17T18:33:27.211Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:27.211Z
Learning: In RocketChat/Rocket.Chat (`apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`), `executeBlockActionHandler` invocations originating from a **modal** surface intentionally do NOT include a `block_action_room` (room property) in the interaction payload. Modals are not scoped to a room, so no room id is available in that context. Do not flag the absence of a room assertion in the modal block-action test as a missing coverage bug; instead, document it explicitly with a `test.step` asserting the room entry is `undefined`.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2026-01-19T18:17:46.433Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38262
File: apps/meteor/client/lib/utils/normalizeMessagePreview/getMessagePreview.ts:24-33
Timestamp: 2026-01-19T18:17:46.433Z
Learning: In the Rocket.Chat repository, usernames (lastMessage.u.username) and display names (lastMessage.u.name) are sanitized/escaped centrally elsewhere in the system, so individual display functions like getMessagePreview do not need to escape these values before interpolating them into strings.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx
📚 Learning: 2026-04-17T23:32:07.223Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/messages.ts:348-352
Timestamp: 2026-04-17T23:32:07.223Z
Learning: In `apps/meteor/app/apps/server/converters/messages.ts`, the `timestamp` handler inside `_convertAttachmentsToApp` uses a non-null assertion (`attachment.ts!`) intentionally. The `ts` property on `MessageAttachment` is optional only to accommodate MessageAttachment creation-time scenarios; by the time `_convertAttachmentsToApp` is called, the message has already been stored and the attachment is guaranteed to have a `ts` value. Do not flag this non-null assertion as unsafe during code review.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2026-04-20T17:11:59.452Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40225
File: apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts:55-71
Timestamp: 2026-04-20T17:11:59.452Z
Learning: In `apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts`, the concern about an empty `?appId=` query param bypassing the truthy check and overriding the path `appId` in the `makeAppLogsQuery` spread is not relevant. The AJV query schema (`isAppLogsProps`) validates and rejects invalid/empty `appId` values before the action handler is reached, making the in-handler guard sufficient as-is. Do not flag this pattern as a vulnerability in future reviews of this file.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx
📚 Learning: 2026-02-24T19:22:52.984Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:52.984Z
Learning: In RocketChat/Rocket.Chat Playwright e2e tests, prefer using translated text and ARIA roles (getByRole, getByText, etc.) over data-qa locators. If translation values change, update the corresponding test locators accordingly. Never prefer data-qa locators.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx
📚 Learning: 2026-02-23T17:53:18.785Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-14T14:58:58.834Z
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-10-16T21:09:51.816Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/keychain.ts:148-156
Timestamp: 2025-10-16T21:09:51.816Z
Learning: In the RocketChat/Rocket.Chat repository, only platforms with native crypto.randomUUID() support are targeted, so fallback implementations for crypto.randomUUID() are not required in E2EE or cryptographic code.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.tsapps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts
📚 Learning: 2026-04-28T14:08:46.920Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 40105
File: apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.ts:54-67
Timestamp: 2026-04-28T14:08:46.920Z
Learning: In `apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.ts`, setting `isJumpingToMessage.current = true` before the guard clauses (RoomHistoryManager.isLoading check, message not found check) is intentional. The flag means "a jump is pending/in progress" and must stay `true` through all intermediate early-return paths (loading, unresolved message, etc.) so that downstream scroll and load behavior is suppressed while waiting for the jump conditions to be satisfied. Do not flag this as a "flag stuck true" bug.
Applied to files:
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsxapps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-11T18:15:53.272Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-11T18:17:53.972Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/processMessageUploads.ts:112-119
Timestamp: 2026-03-11T18:17:53.972Z
Learning: In `apps/meteor/client/lib/chats/flows/processMessageUploads.ts`, when sending multiple file uploads, each file is confirmed via its own `/rooms.mediaConfirm/${rid}/${fileId}` call and produces a separate message. Only the first file's confirm payload carries the composed message text (`msg`); all subsequent files receive `msg: ''`. This one-message-per-file behavior is intentional by design — do not flag it as a bug or suggest batching into a single message.
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-03-19T13:59:40.678Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/uploads.ts:45-49
Timestamp: 2026-03-19T13:59:40.678Z
Learning: In `apps/meteor/app/apps/server/converters/uploads.ts`, the `room` async handler in `convertToApp` uses non-null assertions (`upload.rid!` and `result!`) intentionally. The data flow guarantees that any upload reaching this point must have a `rid`; if it does not, throwing an error is the desired behavior (fail-fast / data integrity guard). Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts
🔇 Additional comments (7)
apps/meteor/client/components/message/hooks/useNormalizedMessage.ts (1)
83-100: Length-gated fallback path is implemented correctly.The oversized-message branch cleanly bypasses markdown parsing, preserves plaintext rendering via
md, and wiresmaxMarkdownParseLengthinto memo dependencies correctly.Also applies to: 113-122
apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.ts (1)
9-18: Hook behavior for disabled/invalid values looks good.Returning
Infinityfor non-positive values keeps the “0 = disabled” contract simple and safe for call sites.apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx (1)
21-22: Caller wiring is correct for the new parse-length guard.The component now correctly sources and forwards
maxMarkdownParseLengthinto normalization.Also applies to: 46-49
apps/meteor/client/components/message/hooks/useNormalizedMessage.spec.ts (1)
35-67: Great coverage for the new threshold behavior.These tests validate both parser bypass and normal parsing paths, plus attachment preservation in the bypass branch.
apps/meteor/client/components/message/content/attachments/QuoteAttachment.spec.tsx (1)
31-44: QuoteAttachment fallback assertions are solid.You now verify both branches: parser-backed rendering for short text and visible plaintext fallback for oversized text.
apps/meteor/client/views/room/MessageList/hooks/useMessageBody.spec.ts (1)
35-63: Good branch coverage foruseMessageBodylength gating.The suite captures all key outcomes introduced by the new
maxMarkdownParseLengthparameter.apps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx (1)
9-20: Oversized-message fast-path is correctly integrated.The guard cleanly skips parsing for long messages, and dependency tracking includes the new threshold input.
Also applies to: 46-46
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2aa76f2 to
a5d7cb3
Compare
8a28f86 to
496020d
Compare
Proposed changes (including videos or screenshots)
Adds new
MESSAGE_MAX_PARSE_LENGTHenvironment variable to limit markdown parsing when message length exceeds a defined amount of characters. Its default value will be 0, meaning there's no limit and every message will be parsed. The default value will be changed from 0 to a reasonable, efficient value on 9.0, to avoid inserting a breaking change along with this PR.Note: The client-side PR is #40600
Changes:
BeforeSaveMarkdownParseraccording to the environment variable valueBeforeSaveMarkdownParserunites tests covering the environment variable valueinjectIntoHeadto send the variable value to the clientIssue(s)
SUP-1019
Steps to test or reproduce
MESSAGE_MAX_PARSE_LENGTHenvironment variable set to100.< 100characters containing markdown (e.g.,Hello **bold**). Verify themdproperty is appended to the message record.> 100characters containing markdown. Verify themdproperty is NOT appended to the message record.0. Verify that any message now has themdproperty.Further comments
Summary by CodeRabbit
New Features
Configuration
Tests