feat(moderation): add explainable reasoning for flagged messages#39943
Open
sharanyamahajan wants to merge 6 commits intoRocketChat:developfrom
Open
feat(moderation): add explainable reasoning for flagged messages#39943sharanyamahajan wants to merge 6 commits intoRocketChat:developfrom
sharanyamahajan wants to merge 6 commits intoRocketChat:developfrom
Conversation
Added a new line to app.json to improve configuration consistency. Changes- 1. Updated app.json 2. Added the required configuration line Notes-This is a minor configuration update and does not affect application functionality.
chore: update app.json configuration
Removed unnecessary line breaks in the UserCard component to improve code readability and maintain formatting consistency. No functional changes were introduced.
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
WalkthroughThis PR introduces Docker containerization for production and debug environments, adds explainable moderation reasoning for flagged messages, refactors message search parsing, simplifies federation matrix event handling, and configures VS Code development tooling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/server/lib/parseMessageSearchQuery.ts">
<violation number="1" location="apps/meteor/server/lib/parseMessageSearchQuery.ts:202">
P1: Narrowing `consumeOrder` to only `asc|desc` breaks previously supported sort aliases and causes unparsed sort tokens to be treated as message text search.</violation>
<violation number="2" location="apps/meteor/server/lib/parseMessageSearchQuery.ts:212">
P2: Regex-based message search behavior was removed: `consumeMessageText` now always uses `$text`, so `forceRegex` and regex-literal query handling no longer work despite still being accepted/passed.</violation>
</file>
<file name=".dockerignore">
<violation number="1" location=".dockerignore:16">
P1: Overly broad `.dockerignore` glob excludes real source paths (`composer*`/`compose*`) from Docker build context, risking incomplete or broken images.</violation>
</file>
<file name="compose.yaml">
<violation number="1" location="compose.yaml:7">
P1: New `compose.yaml` is incomplete for running Rocket.Chat because it omits MongoDB service/connection configuration used by this codebase.</violation>
</file>
<file name="ee/packages/federation-matrix/src/events/member.ts">
<violation number="1" location="ee/packages/federation-matrix/src/events/member.ts:172">
P2: Avatar reset safety check is effectively tautological, so empty `avatar_url` join events can reset avatars without a meaningful origin guard.</violation>
</file>
<file name="apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts">
<violation number="1" location="apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts:301">
P2: Unbounded console logging in the message notification path can be abused to flood logs and degrade server performance.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+202
to
204
| return text.replace(/(?:order|sort):(asc|desc)/g, (_: string, direction: string) => { | ||
| this.options.sort = { ts: direction === 'asc' ? 1 : -1 }; | ||
| return ''; |
Contributor
There was a problem hiding this comment.
P1: Narrowing consumeOrder to only asc|desc breaks previously supported sort aliases and causes unparsed sort tokens to be treated as message text search.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/server/lib/parseMessageSearchQuery.ts, line 202:
<comment>Narrowing `consumeOrder` to only `asc|desc` breaks previously supported sort aliases and causes unparsed sort tokens to be treated as message text search.</comment>
<file context>
@@ -218,77 +198,39 @@ class MessageSearchQueryParser {
- ts: -1,
- };
- }
+ return text.replace(/(?:order|sort):(asc|desc)/g, (_: string, direction: string) => {
+ this.options.sort = { ts: direction === 'asc' ? 1 : -1 };
return '';
</file context>
Suggested change
| return text.replace(/(?:order|sort):(asc|desc)/g, (_: string, direction: string) => { | |
| this.options.sort = { ts: direction === 'asc' ? 1 : -1 }; | |
| return ''; | |
| return text.replace(/(?:order|sort):(asc|ascend|ascending|desc|descend|descending)/g, (_: string, direction: string) => { | |
| this.options.sort = { ts: direction.startsWith('asc') ? 1 : -1 }; | |
| return ''; |
| **/*.jfm | ||
| **/charts | ||
| **/docker-compose* | ||
| **/compose* |
Contributor
There was a problem hiding this comment.
P1: Overly broad .dockerignore glob excludes real source paths (composer*/compose*) from Docker build context, risking incomplete or broken images.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .dockerignore, line 16:
<comment>Overly broad `.dockerignore` glob excludes real source paths (`composer*`/`compose*`) from Docker build context, risking incomplete or broken images.</comment>
<file context>
@@ -0,0 +1,24 @@
+**/*.jfm
+**/charts
+**/docker-compose*
+**/compose*
+**/Dockerfile*
+**/node_modules
</file context>
| build: | ||
| context: . | ||
| dockerfile: ./Dockerfile | ||
| environment: |
Contributor
There was a problem hiding this comment.
P1: New compose.yaml is incomplete for running Rocket.Chat because it omits MongoDB service/connection configuration used by this codebase.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At compose.yaml, line 7:
<comment>New `compose.yaml` is incomplete for running Rocket.Chat because it omits MongoDB service/connection configuration used by this codebase.</comment>
<file context>
@@ -0,0 +1,10 @@
+ build:
+ context: .
+ dockerfile: ./Dockerfile
+ environment:
+ NODE_ENV: production
+ ports:
</file context>
| }, | ||
| }; | ||
| } | ||
| this.query.$text = { $search: text }; |
Contributor
There was a problem hiding this comment.
P2: Regex-based message search behavior was removed: consumeMessageText now always uses $text, so forceRegex and regex-literal query handling no longer work despite still being accepted/passed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/server/lib/parseMessageSearchQuery.ts, line 212:
<comment>Regex-based message search behavior was removed: `consumeMessageText` now always uses `$text`, so `forceRegex` and regex-literal query handling no longer work despite still being accepted/passed.</comment>
<file context>
@@ -218,77 +198,39 @@ class MessageSearchQueryParser {
- },
- };
- }
+ this.query.$text = { $search: text };
+ this.options.projection = { score: { $meta: 'textScore' } };
</file context>
| if ('avatar_url' in content) { | ||
| const currentUser = await Users.findOneById(joiningUser._id); | ||
|
|
||
| if (!content.avatar_url && currentUser?.avatarOrigin === joiningUser.avatarOrigin) { |
Contributor
There was a problem hiding this comment.
P2: Avatar reset safety check is effectively tautological, so empty avatar_url join events can reset avatars without a meaningful origin guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ee/packages/federation-matrix/src/events/member.ts, line 172:
<comment>Avatar reset safety check is effectively tautological, so empty `avatar_url` join events can reset avatars without a meaningful origin guard.</comment>
<file context>
@@ -198,12 +157,23 @@ async function handleInvite({
+ if ('avatar_url' in content) {
+ const currentUser = await Users.findOneById(joiningUser._id);
+
+ if (!content.avatar_url && currentUser?.avatarOrigin === joiningUser.avatarOrigin) {
+ await Users.resetAvatar(joiningUser._id);
+ }
</file context>
| if (reasons.length > 0) { | ||
| (message as any).moderationReasons = reasons; | ||
|
|
||
| console.log('Moderation reasoning:', { |
Contributor
There was a problem hiding this comment.
P2: Unbounded console logging in the message notification path can be abused to flood logs and degrade server performance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts, line 301:
<comment>Unbounded console logging in the message notification path can be abused to flood logs and degrade server performance.</comment>
<file context>
@@ -278,6 +278,31 @@ export async function sendMessageNotifications(message: IMessage, room: IRoom, u
+ if (reasons.length > 0) {
+ (message as any).moderationReasons = reasons;
+
+ console.log('Moderation reasoning:', {
+ userId: message.u?._id ?? 'unknown',
+ reasons,
</file context>
coderabbitai
bot
added
the
type: feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
ee/packages/federation-matrix/src/events/member.ts (1)
316-316: KeeproomIdin the leave error message for faster incident triageLine 316 removed
roomIdfrom the thrown error, which makes federation leave failures harder to correlate across rooms.Proposed fix
- throw new Error(`Room not found while leaving user ${userId}`); + throw new Error(`Room not found while leaving user ${userId} from room ${roomId}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/packages/federation-matrix/src/events/member.ts` at line 316, The thrown error message currently omits roomId (it reads throw new Error(`Room not found while leaving user ${userId}`)), making incidents harder to correlate; update the throw to include roomId as well (e.g., throw new Error(`Room not found while leaving user ${userId} in room ${roomId}`)) in the leave handling code in member.ts so both userId and roomId appear in the error; ensure you reference the same userId and roomId variables in that scope.compose.yaml (1)
1-10: Consider adding production-readiness configurations.For a production compose file, consider adding:
restart: unless-stoppedfor container resilience- Volume mounts for data persistence
- Health checks
- Database/MongoDB service dependency (Rocket.Chat requires MongoDB)
This is acceptable for MVP/local testing purposes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compose.yaml` around lines 1 - 10, The docker-compose service block for rocketchat needs production-ready settings: add a restart: unless-stopped in the rocketchat service, mount persistent volumes via a volumes: entry (e.g., map host data dir to /app/uploads or appropriate Rocket.Chat data paths) to preserve data, add a healthcheck: section (curl or netcat against port 3000) so orchestration can detect unhealthy containers, and add a dependent MongoDB service (e.g., service name mongo) with proper environment/volumes and reference it from rocketchat via depends_on and MONGO_URL (or equivalent) so Rocket.Chat can connect to the database.apps/meteor/app/utils/server/escapeRegExp.ts (1)
1-3: Avoid a secondescapeRegExpimplementation.This PR already uses
escapeRegExpfrom@rocket.chat/string-helpersinapps/meteor/server/lib/parseMessageSearchQuery.ts. Keeping another copy here creates two sources of truth for the same escaping behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/utils/server/escapeRegExp.ts` around lines 1 - 3, Duplicate escapeRegExp implementation: remove this local export of function escapeRegExp and instead import and re-export (or directly use) the canonical escapeRegExp from `@rocket.chat/string-helpers` where needed; update any callers expecting this local function to import from `@rocket.chat/string-helpers` (or re-export the imported symbol under the same name) so there is a single source of truth for escapeRegExp across the codebase.apps/meteor/server/lib/parseMessageSearchQuery.ts (2)
243-244: Keep the exported options type explicit.Changing the public helper to
options: anythrows away the compiler guarantees on a parser API that still has a concrete option shape. Reuse that shape here instead of erasing it.Type-safe signature
-export function parseMessageSearchQuery(text: string, options: any) { +export function parseMessageSearchQuery( + text: string, + options: { user?: IUser; offset?: number; limit?: number; forceRegex?: boolean }, +) { const parser = new MessageSearchQueryParser(options); return parser.parse(text); }As per coding guidelines,
**/*.{ts,tsx,js}should "Write concise, technical TypeScript/JavaScript with accurate typing".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/server/lib/parseMessageSearchQuery.ts` around lines 243 - 244, The exported function parseMessageSearchQuery currently erases types with options: any; update its signature to reuse the concrete options shape from MessageSearchQueryParser (e.g., import and use MessageSearchQueryParserOptions or derive via ConstructorParameters<typeof MessageSearchQueryParser>[0]) so the function preserves compiler guarantees; change the parameter type and adjust imports accordingly while keeping the implementation using new MessageSearchQueryParser(options).
58-58: Drop the “FIXED” markers before merge.These comments do not document an invariant or business rule, so they will just drift as the code changes.
As per coding guidelines,
**/*.{ts,tsx,js}should "Avoid code comments in the implementation".Also applies to: 165-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/server/lib/parseMessageSearchQuery.ts` at line 58, Remove the temporary “FIXED” comment markers left in apps/meteor/server/lib/parseMessageSearchQuery.ts (they appear around the comment at the top of the file and again near line 165); simply delete those markers so the implementation contains no stray "FIXED" comments and keep only meaningful documentation or remove the comment entirely to comply with the guideline to avoid implementation comments.
🤖 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/app/lib/server/lib/sendNotificationsOnMessage.ts`:
- Around line 301-304: Replace the unconditional console.log in the moderation
path with a controlled debug-level log: use SystemLogger.debug instead of
console.log in sendNotificationsOnMessage (the block that logs 'Moderation
reasoning' and the object containing message.u?._id and reasons) so that spam
detection events are only emitted at debug verbosity; preserve the same message
text and object shape (including userId: message.u?._id ?? 'unknown' and
reasons) when calling SystemLogger.debug.
- Around line 282-300: Add a formally typed optional moderationReasons field to
the IMessage type (e.g., moderationReasons?: Array<{type: string; message:
string;}>) instead of casting to any, then assign the reasons directly
(message.moderationReasons = reasons) in sendNotificationsOnMessage.ts so the
injected metadata survives downstream re-casting (see afterSaveMessage.ts:10).
Replace console.log calls used for moderation logging with the project logger
(import and call the existing server logger used elsewhere, e.g., processLogger
or the app's logger) to keep logs consistent.
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 208-214: consumeMessageText() was changed to always build a Mongo
$text query which ignores the existing forceRegex flag (and makes
Message_AlwaysSearchRegExp no-op); update consumeMessageText to respect the
stored forceRegex flag (or the incoming parameter) and, when forceRegex is true,
build the regex-based query path instead of setting this.query.$text and
this.options.projection to textScore, otherwise proceed with the $text branch;
ensure the same flag is used consistently with the caller in messageSearch
(Message_AlwaysSearchRegExp) so behavior for existing workspaces remains
unchanged and projection is only set for the $text branch.
In `@Dockerfile`:
- Around line 4-5: The Dockerfile currently copies only package.json and runs
"npm install" then moves node_modules — replace that with a Yarn v4-friendly
flow: update the COPY step to include yarn-specific files (.yarn, .yarnrc.yml,
yarn.lock along with package.json and workspace manifests) and change the RUN
command (the line with RUN npm install --production --silent && mv node_modules
../) to run "yarn install --immutable" (or "yarn install --immutable
--inline-builds" if builds are needed) and remove the "mv node_modules ../"
move; ensure the Dockerfile uses the Dockerfile COPY and RUN entries shown so
the workspace-aware Yarn install produces correct workspace deps without
relocating node_modules.
- Line 10: The root Dockerfile currently ends with CMD ["node","index.js"] which
is incorrect and deprecated; either delete or mark this Dockerfile as deprecated
in its header, or replace it with the proper multi-stage Meteor build logic used
for production: add a build stage that runs meteor build to produce the bundle
and a final stage that copies the Meteor bundle (with main.js) into the runtime
image and sets the correct CMD/startup; locate the erroneous CMD line in the
root Dockerfile and align its behavior with the multi-stage pattern used for
production builds (generate bundle, copy bundle/main.js into the image, and run
node main.js).
In `@ee/packages/federation-matrix/src/events/member.ts`:
- Around line 246-253: The current mapping still converts a missing
content.avatar_url into null (e.g., avatarUrl = content.avatar_url || null),
which defeats the earlier guard and can clear avatars on join; change the
mapping so you only set an avatar value when the field is present (use a
conditional like: if ('avatar_url' in content) assign content.avatar_url,
otherwise leave it undefined/no-op) and keep the existing
Users.resetAvatar(joiningUser._id) logic that runs only when avatar_url is
explicitly present and falsy; update the code that creates the avatar variable
(the place using content.avatar_url || null) to use an existence check instead
of || to avoid unintended clears.
---
Nitpick comments:
In `@apps/meteor/app/utils/server/escapeRegExp.ts`:
- Around line 1-3: Duplicate escapeRegExp implementation: remove this local
export of function escapeRegExp and instead import and re-export (or directly
use) the canonical escapeRegExp from `@rocket.chat/string-helpers` where needed;
update any callers expecting this local function to import from
`@rocket.chat/string-helpers` (or re-export the imported symbol under the same
name) so there is a single source of truth for escapeRegExp across the codebase.
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 243-244: The exported function parseMessageSearchQuery currently
erases types with options: any; update its signature to reuse the concrete
options shape from MessageSearchQueryParser (e.g., import and use
MessageSearchQueryParserOptions or derive via ConstructorParameters<typeof
MessageSearchQueryParser>[0]) so the function preserves compiler guarantees;
change the parameter type and adjust imports accordingly while keeping the
implementation using new MessageSearchQueryParser(options).
- Line 58: Remove the temporary “FIXED” comment markers left in
apps/meteor/server/lib/parseMessageSearchQuery.ts (they appear around the
comment at the top of the file and again near line 165); simply delete those
markers so the implementation contains no stray "FIXED" comments and keep only
meaningful documentation or remove the comment entirely to comply with the
guideline to avoid implementation comments.
In `@compose.yaml`:
- Around line 1-10: The docker-compose service block for rocketchat needs
production-ready settings: add a restart: unless-stopped in the rocketchat
service, mount persistent volumes via a volumes: entry (e.g., map host data dir
to /app/uploads or appropriate Rocket.Chat data paths) to preserve data, add a
healthcheck: section (curl or netcat against port 3000) so orchestration can
detect unhealthy containers, and add a dependent MongoDB service (e.g., service
name mongo) with proper environment/volumes and reference it from rocketchat via
depends_on and MONGO_URL (or equivalent) so Rocket.Chat can connect to the
database.
In `@ee/packages/federation-matrix/src/events/member.ts`:
- Line 316: The thrown error message currently omits roomId (it reads throw new
Error(`Room not found while leaving user ${userId}`)), making incidents harder
to correlate; update the throw to include roomId as well (e.g., throw new
Error(`Room not found while leaving user ${userId} in room ${roomId}`)) in the
leave handling code in member.ts so both userId and roomId appear in the error;
ensure you reference the same userId and roomId variables in that scope.
🪄 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: ab5c0554-25ce-4412-93c2-80a982b45ffe
📒 Files selected for processing (11)
.dockerignore.vscode/launch.json.vscode/tasks.jsonDockerfileapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsapps/meteor/app/utils/server/escapeRegExp.tsapps/meteor/client/components/UserCard/UserCard.tsxapps/meteor/server/lib/parseMessageSearchQuery.tscompose.debug.yamlcompose.yamlee/packages/federation-matrix/src/events/member.ts
💤 Files with no reviewable changes (1)
- apps/meteor/client/components/UserCard/UserCard.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/app/utils/server/escapeRegExp.tsapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/server/lib/parseMessageSearchQuery.ts
🧠 Learnings (25)
📚 Learning: 2026-03-23T19:33:43.067Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 39818
File: apps/meteor/client/hooks/usePruneWarningMessage.ts:45-56
Timestamp: 2026-03-23T19:33:43.067Z
Learning: In Rocket.Chat, cron expressions used for the retention policy (e.g., `RetentionPolicy_Advanced_Precision_Cron`) follow the `cron` npm package convention, which uses **0-indexed months** (0 = January, 11 = December) rather than POSIX-style 1–12. In `apps/meteor/client/hooks/usePruneWarningMessage.ts`, the local `sendAt` logic intentionally mirrors this behavior—do **not** adjust `date.getMonth()` with `+1` or similar changes, because it will break tests and introduce inconsistency with the backend.
Applied to files:
.dockerignore
📚 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/app/utils/server/escapeRegExp.tsapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/server/lib/parseMessageSearchQuery.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/app/utils/server/escapeRegExp.tsapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/server/lib/parseMessageSearchQuery.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:
compose.debug.yaml
📚 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/app/lib/server/lib/sendNotificationsOnMessage.tsapps/meteor/server/lib/parseMessageSearchQuery.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/app/lib/server/lib/sendNotificationsOnMessage.tsee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2026-02-20T09:04:55.725Z
Learnt from: Shreyas2004wagh
Repo: RocketChat/Rocket.Chat PR: 38681
File: apps/meteor/server/modules/streamer/streamer.module.ts:307-313
Timestamp: 2026-02-20T09:04:55.725Z
Learning: In apps/meteor/server/modules/streamer/streamer.module.ts, the catch block in sendToManySubscriptions intentionally uses SystemLogger.debug (not error or warn) for per-subscription delivery failures to keep logs less noisy, as this was a deliberate design choice reviewed and approved by KevLehman.
Applied to files:
apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
📚 Learning: 2026-02-24T19:16:35.307Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 39003
File: apps/meteor/client/lib/chats/flows/sendMessage.ts:39-45
Timestamp: 2026-02-24T19:16:35.307Z
Learning: In apps/meteor/client/lib/chats/flows/sendMessage.ts, when sdk.call('sendMessage', ...) throws an error, the message is intentionally left with temp: true (not deleted or cleaned up) to support a future retry UI feature. This allows users to retry sending failed messages rather than losing them.
Applied to files:
apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.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/app/lib/server/lib/sendNotificationsOnMessage.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/app/lib/server/lib/sendNotificationsOnMessage.tsee/packages/federation-matrix/src/events/member.tsapps/meteor/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/src/events/member.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:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
ee/packages/federation-matrix/src/events/member.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:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Applied to files:
ee/packages/federation-matrix/src/events/member.ts
📚 Learning: 2026-03-16T23:33:15.721Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: apps/meteor/app/api/server/v1/users.ts:862-869
Timestamp: 2026-03-16T23:33:15.721Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs (e.g., PR `#39676` for users.register in apps/meteor/app/api/server/v1/users.ts), calls to `this.parseJsonQuery()` inside migrated handlers are intentionally preserved without adding a corresponding `query` AJV schema to the route options. Adding query-param schemas for the `fields`/`sort`/`query` parameters consumed by `parseJsonQuery()` is a separate cross-cutting concern shared by many endpoints (e.g., users.create, users.update, users.list) and is explicitly out of scope for individual endpoint migration PRs. Do not flag the absence of a `query` schema for `parseJsonQuery()` usage as a violation of OpenAPI/AJV contract during migration reviews.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.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/server/lib/parseMessageSearchQuery.ts
📚 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/server/lib/parseMessageSearchQuery.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/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
🪛 Trivy (0.69.3)
Dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[error] 5-6: 'apt-get' missing '--no-install-recommends'
'--no-install-recommends' flag is missed: 'apt-get update && apt-get install -y build-essential git'
Rule: DS-0029
(IaC/Dockerfile)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🔇 Additional comments (5)
.dockerignore (1)
1-24: LGTM!Standard
.dockerignorepatterns that appropriately exclude development artifacts, IDE metadata, and build context noise. Excludingnode_modulesis correct since dependencies are installed fresh in the container..vscode/launch.json (1)
1-11: LGTM!Standard VS Code Docker debug launch configuration that correctly references the
docker-run: debugtask defined intasks.json.compose.debug.yaml (1)
1-12: LGTM!Appropriate debug configuration with Node inspector enabled. The
0.0.0.0:9229binding is expected for container debugging scenarios where the debugger needs to be accessible from the host.Note: Ensure this compose file is only used in local development environments, as the debugger binding to all interfaces could pose a security risk if exposed on a network.
.vscode/tasks.json (1)
1-39: LGTM!Well-structured VS Code tasks for Docker build and run workflows. The debug task appropriately sets
DEBUG="*"for verbose logging and enables Node.js debugging.apps/meteor/server/lib/parseMessageSearchQuery.ts (1)
47-50: Good hardening of user-controlled regex filters.Escaping usernames and attachment search tokens before inserting them into Mongo
$regexclauses removes regex metacharacter injection while preserving the current query syntax.Also applies to: 65-68, 111-113, 125-127, 139-141
Comment on lines
+282
to
+300
| const reasons: any[] = []; | ||
|
|
||
| if (message.msg && message.msg.length > 200) { | ||
| reasons.push({ | ||
| type: 'long_message', | ||
| message: 'Message unusually long (possible spam)', | ||
| }); | ||
| } | ||
|
|
||
| if (/(.)\1{5,}/.test(message.msg || '')) { | ||
| reasons.push({ | ||
| type: 'repetition', | ||
| message: 'Repeated characters detected', | ||
| }); | ||
| } | ||
|
|
||
| if (reasons.length > 0) { | ||
| (message as any).moderationReasons = reasons; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether moderationReasons is part of the formal message contract and where it is used.
rg -n "interface IMessage|moderationReasons" packages/core-typings/src/IMessage/IMessage.ts
rg -n "moderationReasons" apps/meteor --type ts --type js
rg -n "afterSaveMessage\\(|callbacks\\.run\\('afterSaveMessage'" apps/meteor/app/lib/server/lib/afterSaveMessage.ts apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsRepository: RocketChat/Rocket.Chat
Length of output: 722
🏁 Script executed:
cat -n packages/core-typings/src/IMessage/IMessage.ts | head -200Repository: RocketChat/Rocket.Chat
Length of output: 6380
🏁 Script executed:
rg -i "moderation|reason" packages/core-typings/src/IMessage/ --context 3Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
rg -n "TODO|FIXME" apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
cat packages/core-typings/src/IMessage/IMessage.ts | wc -lRepository: RocketChat/Rocket.Chat
Length of output: 69
🏁 Script executed:
tail -100 packages/core-typings/src/IMessage/IMessage.tsRepository: RocketChat/Rocket.Chat
Length of output: 2621
🏁 Script executed:
rg "moderationReasons" apps/meteor --type ts --type js -B 5 -A 5Repository: RocketChat/Rocket.Chat
Length of output: 965
🏁 Script executed:
cat -n apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts | sed -n '295,310p'Repository: RocketChat/Rocket.Chat
Length of output: 505
🏁 Script executed:
rg "import.*logger|import.*Logger|from.*logger" apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
rg "console\.log" apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsRepository: RocketChat/Rocket.Chat
Length of output: 106
Formalize moderation metadata typing instead of any mutation.
Line 299 bypasses the type contract with (message as any), while IMessage does not declare moderationReasons. The field is injected without formal typing in a callback flow, creating an implicit, untyped payload surface. Additionally, the field will be lost when the callback result is re-cast to IMessage in afterSaveMessage.ts:10.
Additionally, lines 301–304 use console.log for moderation logging instead of the project's logging infrastructure. This should use the appropriate logger.
Suggested tightening in this file
+type ModerationReason = {
+ type: 'long_message' | 'repetition';
+ message: string;
+};
+
+type MessageWithModerationReasons = IMessage & {
+ moderationReasons?: ModerationReason[];
+};
-
- const reasons: any[] = [];
+ const reasons: ModerationReason[] = [];
...
if (reasons.length > 0) {
- (message as any).moderationReasons = reasons;
+ (message as MessageWithModerationReasons).moderationReasons = reasons;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const reasons: any[] = []; | |
| if (message.msg && message.msg.length > 200) { | |
| reasons.push({ | |
| type: 'long_message', | |
| message: 'Message unusually long (possible spam)', | |
| }); | |
| } | |
| if (/(.)\1{5,}/.test(message.msg || '')) { | |
| reasons.push({ | |
| type: 'repetition', | |
| message: 'Repeated characters detected', | |
| }); | |
| } | |
| if (reasons.length > 0) { | |
| (message as any).moderationReasons = reasons; | |
| type ModerationReason = { | |
| type: 'long_message' | 'repetition'; | |
| message: string; | |
| }; | |
| type MessageWithModerationReasons = IMessage & { | |
| moderationReasons?: ModerationReason[]; | |
| }; | |
| const reasons: ModerationReason[] = []; | |
| if (message.msg && message.msg.length > 200) { | |
| reasons.push({ | |
| type: 'long_message', | |
| message: 'Message unusually long (possible spam)', | |
| }); | |
| } | |
| if (/(.)\1{5,}/.test(message.msg || '')) { | |
| reasons.push({ | |
| type: 'repetition', | |
| message: 'Repeated characters detected', | |
| }); | |
| } | |
| if (reasons.length > 0) { | |
| (message as MessageWithModerationReasons).moderationReasons = reasons; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts` around lines
282 - 300, Add a formally typed optional moderationReasons field to the IMessage
type (e.g., moderationReasons?: Array<{type: string; message: string;}>) instead
of casting to any, then assign the reasons directly (message.moderationReasons =
reasons) in sendNotificationsOnMessage.ts so the injected metadata survives
downstream re-casting (see afterSaveMessage.ts:10). Replace console.log calls
used for moderation logging with the project logger (import and call the
existing server logger used elsewhere, e.g., processLogger or the app's logger)
to keep logs consistent.
Comment on lines
+301
to
+304
| console.log('Moderation reasoning:', { | ||
| userId: message.u?._id ?? 'unknown', | ||
| reasons, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the current state of the file around lines 301-304
cat -n apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts | sed -n '290,320p'Repository: RocketChat/Rocket.Chat
Length of output: 1002
🏁 Script executed:
# Check if SystemLogger is imported in this file
grep -n "SystemLogger\|import.*logger" apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Look at the broader context of this function to understand what it does
cat -n apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts | sed -n '270,330p'Repository: RocketChat/Rocket.Chat
Length of output: 2177
🏁 Script executed:
# Check how SystemLogger is used elsewhere in the codebase for similar scenarios
rg "SystemLogger.debug" -A 3 -B 1 --max-count=5Repository: RocketChat/Rocket.Chat
Length of output: 4540
🏁 Script executed:
# Check if there are other console.log statements in this file
grep -n "console\.log\|console\.error\|console\.warn" apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsRepository: RocketChat/Rocket.Chat
Length of output: 110
🏁 Script executed:
# Check if there are other moderation-related logs in the codebase
rg "moderation|Moderation" -i --max-count=10 -B 2 -A 2Repository: RocketChat/Rocket.Chat
Length of output: 50379
🏁 Script executed:
# Verify the correct import path for SystemLogger
rg "from.*logger.*system|import.*SystemLogger" -l | head -5Repository: RocketChat/Rocket.Chat
Length of output: 302
🏁 Script executed:
# Check a similar file to see how imports are structured
cat -n apps/meteor/app/mail-messages/server/functions/sendMail.ts | head -20Repository: RocketChat/Rocket.Chat
Length of output: 832
🏁 Script executed:
# Look for moderation context - check if this relates to spam detection
rg "long_message|repetition" -B 5 -A 5Repository: RocketChat/Rocket.Chat
Length of output: 5537
Replace console.log with SystemLogger.debug for controlled logging in this message processing hot path.
Line 301 logs spam detection events with console.log, which outputs unconditionally. Use SystemLogger.debug to allow log verbosity to be controlled by log level configuration.
Suggested change
+import { SystemLogger } from '../../../../server/lib/logger/system';
...
- console.log('Moderation reasoning:', {
- userId: message.u?._id ?? 'unknown',
- reasons,
- });
+ SystemLogger.debug({
+ msg: 'Moderation reasoning',
+ mid: message._id,
+ userId: message.u?._id ?? 'unknown',
+ reasonTypes: reasons.map((reason) => reason.type),
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts` around lines
301 - 304, Replace the unconditional console.log in the moderation path with a
controlled debug-level log: use SystemLogger.debug instead of console.log in
sendNotificationsOnMessage (the block that logs 'Moderation reasoning' and the
object containing message.u?._id and reasons) so that spam detection events are
only emitted at debug verbosity; preserve the same message text and object shape
(including userId: message.u?._id ?? 'unknown' and reasons) when calling
SystemLogger.debug.
Comment on lines
208
to
214
| private consumeMessageText(text: string) { | ||
| text = text.trim().replace(/\s\s/g, ' '); | ||
|
|
||
| if (text === '') { | ||
| return text; | ||
| } | ||
| text = text.trim(); | ||
| if (!text) return text; | ||
|
|
||
| if (/^\/.+\/[imxs]*$/.test(text)) { | ||
| const r = text.split('/'); | ||
| this.query.msg = { | ||
| $regex: r[1], | ||
| $options: r[2], | ||
| }; | ||
| } else if (this.forceRegex) { | ||
| this.query.msg = { | ||
| $regex: text, | ||
| $options: 'i', | ||
| }; | ||
| } else { | ||
| this.query.$text = { | ||
| $search: text, | ||
| }; | ||
| this.options.projection = { | ||
| score: { | ||
| $meta: 'textScore', | ||
| }, | ||
| }; | ||
| } | ||
| this.query.$text = { $search: text }; | ||
| this.options.projection = { score: { $meta: 'textScore' } }; | ||
|
|
Contributor
There was a problem hiding this comment.
forceRegex stops working with this change.
consumeMessageText() now always builds a Mongo $text query, but the parser still accepts/stores forceRegex and apps/meteor/server/methods/messageSearch.ts:47-52 still passes Message_AlwaysSearchRegExp. That makes the setting a no-op and changes search behavior for existing workspaces.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts` around lines 208 - 214,
consumeMessageText() was changed to always build a Mongo $text query which
ignores the existing forceRegex flag (and makes Message_AlwaysSearchRegExp
no-op); update consumeMessageText to respect the stored forceRegex flag (or the
incoming parameter) and, when forceRegex is true, build the regex-based query
path instead of setting this.query.$text and this.options.projection to
textScore, otherwise proceed with the $text branch; ensure the same flag is used
consistently with the caller in messageSearch (Message_AlwaysSearchRegExp) so
behavior for existing workspaces remains unchanged and projection is only set
for the $text branch.
Comment on lines
+4
to
+5
| COPY ["package.json", "package-lock.json*", "npm-shrinkwrap.json*", "./"] | ||
| RUN npm install --production --silent && mv node_modules ../ |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if yarn.lock exists and examine workspace configuration
echo "=== Checking for lock files ==="
ls -la yarn.lock package-lock.json 2>/dev/null || echo "Lock files not found at root"
echo -e "\n=== Workspace configuration in package.json ==="
jq '.workspaces' package.json 2>/dev/null || cat package.json | grep -A 10 '"workspaces"'
echo -e "\n=== Package manager field ==="
jq '.packageManager' package.json 2>/dev/nullRepository: RocketChat/Rocket.Chat
Length of output: 383
🏁 Script executed:
cat -n Dockerfile | head -20Repository: RocketChat/Rocket.Chat
Length of output: 423
Critical: This monorepo uses yarn workspaces but the Dockerfile uses npm install.
The root package.json declares yarn workspaces and pins packageManager to yarn@4.12.0. Using npm install will not properly resolve these workspace dependencies since npm does not natively understand yarn's workspace feature.
Additionally, moving node_modules to the parent directory (mv node_modules ../) is unconventional and may cause module resolution issues.
🔧 Proposed fix using yarn with --immutable
FROM node:lts-alpine
ENV NODE_ENV=production
WORKDIR /usr/src/app
-COPY ["package.json", "package-lock.json*", "npm-shrinkwrap.json*", "./"]
-RUN npm install --production --silent && mv node_modules ../
+COPY ["package.json", "yarn.lock", "./"]
+RUN yarn install --production --immutable
COPY . .
EXPOSE 3000
RUN chown -R node /usr/src/app
USER node
CMD ["node", "index.js"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| COPY ["package.json", "package-lock.json*", "npm-shrinkwrap.json*", "./"] | |
| RUN npm install --production --silent && mv node_modules ../ | |
| COPY ["package.json", "yarn.lock", "./"] | |
| RUN yarn install --production --immutable |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` around lines 4 - 5, The Dockerfile currently copies only
package.json and runs "npm install" then moves node_modules — replace that with
a Yarn v4-friendly flow: update the COPY step to include yarn-specific files
(.yarn, .yarnrc.yml, yarn.lock along with package.json and workspace manifests)
and change the RUN command (the line with RUN npm install --production --silent
&& mv node_modules ../) to run "yarn install --immutable" (or "yarn install
--immutable --inline-builds" if builds are needed) and remove the "mv
node_modules ../" move; ensure the Dockerfile uses the Dockerfile COPY and RUN
entries shown so the workspace-aware Yarn install produces correct workspace
deps without relocating node_modules.
| EXPOSE 3000 | ||
| RUN chown -R node /usr/src/app | ||
| USER node | ||
| CMD ["node", "index.js"] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if index.js exists at root and find potential entrypoints
echo "=== Checking for index.js at root ==="
ls -la index.js 2>/dev/null || echo "index.js not found at root"
echo -e "\n=== Main field in package.json ==="
jq '.main' package.json
echo -e "\n=== Looking for potential entrypoints ==="
fd -t f -d 2 'index.js|main.js|server.js' . 2>/dev/null | head -10Repository: RocketChat/Rocket.Chat
Length of output: 220
🏁 Script executed:
#!/bin/bash
# Verify Dockerfile exists and check its content
echo "=== Dockerfile content ==="
if [ -f "Dockerfile" ]; then
cat -n Dockerfile
else
echo "Dockerfile not found at root"
fi
echo -e "\n=== Repository structure (top-level directories) ==="
ls -la | grep "^d" | head -20
echo -e "\n=== Root package.json details ==="
cat -n package.json | head -40
echo -e "\n=== Looking for .dockerignore, docker-compose, etc ==="
ls -la | grep -i docker
echo -e "\n=== Check for monorepo indicators ==="
if [ -f "lerna.json" ]; then echo "=== lerna.json found ==="; cat -n lerna.json; fi
if [ -f "pnpm-workspace.yaml" ]; then echo "=== pnpm-workspace.yaml found ==="; cat -n pnpm-workspace.yaml; fiRepository: RocketChat/Rocket.Chat
Length of output: 3578
🏁 Script executed:
#!/bin/bash
# Search for entrypoints with greater depth and different patterns
echo "=== Searching for index.js, main.js, server.js up to depth 5 ==="
fd -t f -d 5 'index\.js$' . 2>/dev/null | head -15
echo -e "\n=== Checking apps/ subdirectory if it exists ==="
if [ -d "apps" ]; then
echo "apps/ directory found, listing structure:"
ls -la apps/
fi
echo -e "\n=== Checking for TypeScript/compiled entry points ==="
fd -t f -d 5 'index\.ts$|server\.ts$|main\.ts$' . 2>/dev/null | head -15
echo -e "\n=== Check if packages/app-* subdirectories exist ==="
find . -maxdepth 2 -type d -name "app-*" | head -10Repository: RocketChat/Rocket.Chat
Length of output: 1241
🏁 Script executed:
#!/bin/bash
# Check full Dockerfile for build stages
echo "=== Full Dockerfile ==="
cat -n Dockerfile
echo -e "\n=== apps/meteor/package.json ==="
head -50 apps/meteor/package.json
echo -e "\n=== Check for build output or entry in root ==="
ls -la | grep -E '\.meteor|build|dist|server'
echo -e "\n=== Check apps/meteor structure ==="
ls -la apps/meteor/ | head -20Repository: RocketChat/Rocket.Chat
Length of output: 3979
🏁 Script executed:
#!/bin/bash
# Check docker-compose to understand how container is run
echo "=== docker-compose-local.yml (first 50 lines) ==="
head -50 docker-compose-local.yml
echo -e "\n=== docker-compose-ci.yml (first 50 lines) ==="
head -50 docker-compose-ci.yml
echo -e "\n=== Look for any .meteorrc, .meteor.json, or meteor config ==="
find . -maxdepth 3 -name ".meteorrc" -o -name "meteor.json" -o -name ".meteor" -type d 2>/dev/null | head -10Repository: RocketChat/Rocket.Chat
Length of output: 3487
🏁 Script executed:
#!/bin/bash
# Check if there are build instructions or scripts at root
echo "=== Build-related scripts at root ==="
ls -la | grep -i build
echo -e "\n=== Check .dockerignore content ==="
cat -n .dockerignore
echo -e "\n=== Look for any npm/yarn scripts that might build ==="
cd apps/meteor && grep -A 5 '"scripts"' package.json | head -20Repository: RocketChat/Rocket.Chat
Length of output: 1104
🏁 Script executed:
#!/bin/bash
# Check the actual Dockerfile used in builds
echo "=== apps/meteor/.docker/Dockerfile.alpine ==="
if [ -f "apps/meteor/.docker/Dockerfile.alpine" ]; then
cat -n apps/meteor/.docker/Dockerfile.alpine | head -50
else
echo "File not found"
fi
echo -e "\n=== Check all Dockerfiles in the repository ==="
find . -name "Dockerfile*" -type f | head -20
echo -e "\n=== Check .github workflows to see how the image is built ==="
ls -la .github/workflows/ | grep -i dockerRepository: RocketChat/Rocket.Chat
Length of output: 2952
🏁 Script executed:
#!/bin/bash
# Understand the build process better
echo "=== Check if Meteor generates the index.js during build ==="
# Look for references to the build output
find . -maxdepth 3 -name "*.ts" -o -name "*.js" | xargs grep -l "index.js" 2>/dev/null | head -10
echo -e "\n=== Check apps/meteor/.docker directory ==="
ls -la apps/meteor/.docker/
echo -e "\n=== Look for entry point configuration in meteor settings ==="
cat apps/meteor/.meteorignore 2>/dev/null
ls -la apps/meteor/.meteor/Repository: RocketChat/Rocket.Chat
Length of output: 1060
🏁 Script executed:
#!/bin/bash
# Check recent changes to root Dockerfile
echo "=== Recent git log for root Dockerfile ==="
git log --oneline -10 Dockerfile 2>/dev/null || echo "Could not get git log"
echo -e "\n=== Check if root Dockerfile is referenced in any CI/CD or build scripts ==="
grep -r "Dockerfile" .github/ .gitpod* .devcontainer/ 2>/dev/null | grep -v ".alpine" | grep -v ".debian" | head -10
echo -e "\n=== Check if root Dockerfile is used in docker-compose files ==="
grep -l "dockerfile" docker-compose*.yml 2>/dev/null | xargs grep -h "dockerfile" 2>/dev/null
echo -e "\n=== Look at the actual Meteor build output structure ==="
ls -la apps/meteor/bundle 2>/dev/null || echo "No bundle directory found (expected - it's created during build)"Repository: RocketChat/Rocket.Chat
Length of output: 1863
🏁 Script executed:
#!/bin/bash
# Check what the proper entry point should be for Meteor
echo "=== Check apps/meteor/.docker/Dockerfile.alpine for actual CMD ==="
tail -20 apps/meteor/.docker/Dockerfile.alpine
echo -e "\n=== Check Dockerfile.debian for comparison ==="
tail -20 apps/meteor/.docker/Dockerfile.debianRepository: RocketChat/Rocket.Chat
Length of output: 1039
The root Dockerfile is incomplete and not used in production builds.
Line 10 references index.js, which doesn't exist at the repository root. However, this Dockerfile is deprecated—the actual production Dockerfile is apps/meteor/.docker/Dockerfile.alpine, which correctly expects main.js in the Meteor bundle at /app/bundle.
The root Dockerfile lacks the build stage required to generate this bundle via meteor build. Either remove this outdated file, document it as deprecated, or replace it with the proper multi-stage Meteor build logic from apps/meteor/.docker/Dockerfile.alpine.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` at line 10, The root Dockerfile currently ends with CMD
["node","index.js"] which is incorrect and deprecated; either delete or mark
this Dockerfile as deprecated in its header, or replace it with the proper
multi-stage Meteor build logic used for production: add a build stage that runs
meteor build to produce the bundle and a final stage that copies the Meteor
bundle (with main.js) into the runtime image and sets the correct CMD/startup;
locate the erroneous CMD line in the root Dockerfile and align its behavior with
the multi-stage pattern used for production builds (generate bundle, copy
bundle/main.js into the image, and run node main.js).
Comment on lines
+246
to
+253
| // ✅ FIX: Safe avatar handling | ||
| if ('avatar_url' in content) { | ||
| const currentUser = await Users.findOneById(joiningUser._id); | ||
|
|
||
| if (!content.avatar_url && currentUser?.avatarOrigin === joiningUser.avatarOrigin) { | ||
| await Users.resetAvatar(joiningUser._id); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
avatar_url safety fix is incomplete; missing field can still clear avatars
Line 270 still maps absent avatar_url to null (content.avatar_url || null), so join events without that field can trigger avatar reset despite the new guard at Lines 246-253.
Proposed fix
- // ✅ FIX: Safe avatar handling
- if ('avatar_url' in content) {
- const currentUser = await Users.findOneById(joiningUser._id);
-
- if (!content.avatar_url && currentUser?.avatarOrigin === joiningUser.avatarOrigin) {
- await Users.resetAvatar(joiningUser._id);
- }
- }
-
const room = await Rooms.findOneFederatedByMrid(roomId);
@@
const senderServerName = extractDomainFromMatrixUserId(userId);
// handle avatar updates to membership events
- if (senderServerName !== federationSDK.getConfig('serverName')) {
- // TODO if there is no avatar_url we may want to validate first if we should remove the user avatar because if may be dealing with an old join event, and the user may have changed their avatar since then, so we need to check if the avatar_url is different from the current one before removing it
- void downloadAndSetAvatarDebounced(joiningUser._id, joiningUser, content.avatar_url || null);
+ if (senderServerName !== federationSDK.getConfig('serverName') && 'avatar_url' in content) {
+ if (!content.avatar_url) {
+ const currentUser = await Users.findOneById(joiningUser._id);
+ if (currentUser?.avatarOrigin === joiningUser.avatarOrigin) {
+ await Users.resetAvatar(joiningUser._id);
+ }
+ } else {
+ void downloadAndSetAvatarDebounced(joiningUser._id, joiningUser, content.avatar_url);
+ }
}Also applies to: 268-271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/packages/federation-matrix/src/events/member.ts` around lines 246 - 253,
The current mapping still converts a missing content.avatar_url into null (e.g.,
avatarUrl = content.avatar_url || null), which defeats the earlier guard and can
clear avatars on join; change the mapping so you only set an avatar value when
the field is present (use a conditional like: if ('avatar_url' in content)
assign content.avatar_url, otherwise leave it undefined/no-op) and keep the
existing Users.resetAvatar(joiningUser._id) logic that runs only when avatar_url
is explicitly present and falsy; update the code that creates the avatar
variable (the place using content.avatar_url || null) to use an existence check
instead of || to avoid unintended clears.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
This PR introduces a basic explainable reasoning layer for moderation-related message processing.
Currently, moderation or spam-related checks do not provide any context or explanation for why a message might be considered suspicious. This makes it difficult for moderators to understand or trust automated actions.
Changes made:
moderationReasonsarray to the message object when such patterns are detectedExample:
A message like:
"heyyyyyyyyyyyyyyyy!!!!"
Will generate:
Result:
Issue(s)
Closes #39941
Steps to test or reproduce
Send a message with repeated characters:
Send a very long message (200+ characters)
Check server logs:
moderationReasonsis attached to the message objectFurther comments
This is an MVP implementation focused on introducing a reasoning structure with minimal impact on existing logic.
The solution is intentionally lightweight:
This design allows future extensions such as:
The goal is to establish a scalable foundation for explainable moderation in Rocket.Chat.
Summary by CodeRabbit
New Features
Refactor
Style