fix(search): include attachment fields in message search#39898
Open
NagajyothiChukka wants to merge 10 commits intoRocketChat:developfrom
Open
fix(search): include attachment fields in message search#39898NagajyothiChukka wants to merge 10 commits intoRocketChat:developfrom
NagajyothiChukka wants to merge 10 commits intoRocketChat:developfrom
Conversation
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
|
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:
Walkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
NagajyothiChukka
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 3 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="package.json.save">
<violation number="1" location="package.json.save:77">
P2: `package.json.save` is malformed JSON due to stray characters (`},.` and a `.` line), which can break JSON parsing tools and appears to be an unintended backup artifact.</violation>
</file>
<file name=".yarnrc.yml">
<violation number="1" location=".yarnrc.yml:9">
P2: Disabling Yarn strict SSL weakens TLS certificate validation for dependency downloads, increasing MITM/tampering risk. Keep strict SSL enabled unless there is a documented internal-registry requirement.</violation>
</file>
<file name="apps/meteor/server/lib/parseMessageSearchQuery.ts">
<violation number="1" location="apps/meteor/server/lib/parseMessageSearchQuery.ts:258">
P1: `forceRegex` configuration is ignored after this change; message search now always uses regex mode, causing a behavior regression.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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: 3
🧹 Nitpick comments (2)
apps/meteor/server/lib/parseMessageSearchQuery.ts (2)
244-247: Fix inconsistent indentation and duplicate comment block.The JSDoc comment and method declaration have inconsistent indentation (mixing tabs and spaces at different levels), and there appears to be a duplicate comment header. This violates the project's formatting standards.
Proposed fix
- /** - * Query in message text + attachments - */ - private consumeMessageText(text: string) { + /** + * Query in message text and attachments + */ + private consumeMessageText(text: string) {🤖 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 244 - 247, The JSDoc block and the method declaration for consumeMessageText have mixed tabs/spaces and a duplicated comment header; remove the duplicate comment, convert indentation to the project's standard (use spaces consistently) and align the JSDoc directly above the private consumeMessageText(text: string) declaration so the comment and method share identical indentation and formatting; ensure the comment uses JSDoc style (/** ... */) only once and that the method signature line has no leading tabs.
258-272: Implementation correctly extends search to attachment fields.The
$orquery with regex conditions onmsg,attachments.text,attachments.title, andattachments.descriptionfulfills the PR objective. The use ofescapeRegExpfor non-regex input prevents regex injection.For production deployments with large message collections, consider adding indexes on the attachment fields if search performance degrades:
db.rocketchat_message.createIndex({ "attachments.text": "text", "attachments.title": "text", "attachments.description": "text" })🤖 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 258 - 272, The current change correctly extends the search ($or conditions in parseMessageSearchQuery.ts using this.query.$or) to include attachments fields, but for production performance add a database text index on the attachment fields; create an index on the messages collection (e.g., via db.rocketchat_message.createIndex with "attachments.text", "attachments.title", and "attachments.description" as text fields) or include an appropriate migration/DB-setup step to ensure these fields are indexed for large collections.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.yarnrc.yml:
- Around line 9-10: The .yarnrc.yml setting enableStrictSsl: false disables TLS
certificate validation and exposes dependency downloads to MITM; change this to
enableStrictSsl: true and, if you need to trust a corporate CA, remove the
global disable and instead configure caFilePath to point to the PEM file of the
corporate CA (or document and gate this override for only the specific
environment), ensuring any temporary disablement is reverted and the rationale
is recorded.
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 244-276: The consumeMessageText implementation ignores the
instance property this.forceRegex so Message_AlwaysSearchRegExp has no effect;
update consumeMessageText (in the ParseMessageSearchQuery class) to first check
this.forceRegex and, if true, treat the input text as a regex pattern (i.e. do
not escape it) when building this.query.$or: if the text matches
/^\/(.+)\/([imxs]*)$/ extract pattern and flags, otherwise use the raw text as
the pattern and apply a sensible default option (e.g. 'i'); if this.forceRegex
is false, keep the existing behavior that escapes plain text with escapeRegExp
and only treats /pattern/flags as regex. Ensure references to this.forceRegex,
consumeMessageText, escapeRegExp, and this.query.$or are updated accordingly.
In `@package.json.save`:
- Around line 1-89: Delete the accidentally committed editor recovery file
package.json.save (it contains malformed JSON around the "engines" block and
stray '.' characters before the "volta" object); remove the file from the
commit/history (git rm --cached package.json.save or git rm package.json.save
and commit, and if needed remove it from branch history with git filter-branch
or BFG), and add an entry to .gitignore to prevent future commits of editor
recovery files (e.g., *.save or specific filename).
---
Nitpick comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 244-247: The JSDoc block and the method declaration for
consumeMessageText have mixed tabs/spaces and a duplicated comment header;
remove the duplicate comment, convert indentation to the project's standard (use
spaces consistently) and align the JSDoc directly above the private
consumeMessageText(text: string) declaration so the comment and method share
identical indentation and formatting; ensure the comment uses JSDoc style (/**
... */) only once and that the method signature line has no leading tabs.
- Around line 258-272: The current change correctly extends the search ($or
conditions in parseMessageSearchQuery.ts using this.query.$or) to include
attachments fields, but for production performance add a database text index on
the attachment fields; create an index on the messages collection (e.g., via
db.rocketchat_message.createIndex with "attachments.text", "attachments.title",
and "attachments.description" as text fields) or include an appropriate
migration/DB-setup step to ensure these fields are indexed for large
collections.
🪄 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: eda91686-1dd1-475e-b4fa-feae6fd10893
📒 Files selected for processing (3)
.yarnrc.ymlapps/meteor/server/lib/parseMessageSearchQuery.tspackage.json.save
📜 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). (1)
- GitHub Check: cubic · AI code reviewer
🧰 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/server/lib/parseMessageSearchQuery.ts
🧠 Learnings (12)
📓 Common learnings
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.
📚 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:
.yarnrc.yml
📚 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/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/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:
package.json.save
📚 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:
package.json.save
📚 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:
package.json.save
📚 Learning: 2026-03-17T16:08:37.572Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 39690
File: packages/ui-voip/package.json:11-11
Timestamp: 2026-03-17T16:08:37.572Z
Learning: In `packages/ui-voip/package.json` (RocketChat/Rocket.Chat), the team deliberately chose to use `rm -rf dist` directly in the `"build"` script instead of `rimraf`, as they decided against introducing the `rimraf` dependency for this package. Do not flag `rm -rf dist` in the ui-voip build script as a cross-platform issue requiring rimraf.
Applied to files:
package.json.save
📚 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:
package.json.save
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
package.json.save
📚 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:
package.json.save
📚 Learning: 2026-01-08T15:03:59.621Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38068
File: apps/meteor/tests/data/apps/app-packages/README.md:14-16
Timestamp: 2026-01-08T15:03:59.621Z
Learning: For the RocketChat/Rocket.Chat repository, do not analyze or report formatting issues (such as hard tabs vs spaces, line breaks, etc.). The project relies on automated linting tools to enforce formatting standards.
Applied to files:
package.json.save
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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:250">
P2: Minimum query-length protection was removed, enabling expensive 1-character regex searches across message and attachment fields.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/server/lib/parseMessageSearchQuery.ts (1)
244-246: Remove the duplicate method comment.There are now two back-to-back docblocks above
consumeMessageText. Please update the existing one instead of adding a second comment. As per coding guidelines, "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/server/lib/parseMessageSearchQuery.ts` around lines 244 - 246, There are duplicate docblocks placed above the consumeMessageText function; remove the extra/back-to-back comment and keep a single, appropriate docblock for consumeMessageText (the one that documents "Query in message text + attachments"), updating it if needed rather than adding a second comment; locate the comments surrounding the consumeMessageText symbol and delete the redundant block so only one docblock remains.
🤖 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/server/lib/parseMessageSearchQuery.ts`:
- Around line 259-262: The current parsing uses text.split('/') which breaks
patterns containing escaped slashes; replace that logic in the regex-parsing
branch (the if that currently tests /^\/.+\/[imxs]*$/ and assigns to regex and
options) with a single RegExp match that captures the whole pattern allowing
escaped slashes (e.g. use a capture like /^\/((?:\\\/|[^\/])+)\/([imxs]*)$/ and
extract match[1] -> regex and match[2] -> options), and add unit tests covering
inputs with escaped slashes such as '/https:\/\/gitlab.example/i' and
'/foo\/bar/' to prevent regressions.
---
Nitpick comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 244-246: There are duplicate docblocks placed above the
consumeMessageText function; remove the extra/back-to-back comment and keep a
single, appropriate docblock for consumeMessageText (the one that documents
"Query in message text + attachments"), updating it if needed rather than adding
a second comment; locate the comments surrounding the consumeMessageText symbol
and delete the redundant block so only one docblock remains.
🪄 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: e8280a1b-1d5c-41eb-92fe-b2bbfb835d9e
📒 Files selected for processing (1)
apps/meteor/server/lib/parseMessageSearchQuery.ts
📜 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). (1)
- GitHub Check: cubic · AI code reviewer
🧰 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/server/lib/parseMessageSearchQuery.ts
🧠 Learnings (13)
📚 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-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/lib/parseMessageSearchQuery.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/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2026-03-16T22:56:54.500Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39677
File: packages/models/src/helpers/omnichannel/agentStatus.ts:10-29
Timestamp: 2026-03-16T22:56:54.500Z
Learning: In `packages/models/src/helpers/omnichannel/agentStatus.ts` (PR `#39677`), the `queryStatusAgentOnline` function intentionally omits the `$or` offline-status guard for non-bot agents when `isLivechatEnabledWhenAgentIdle === true`. This is by design: the setting `Livechat_enabled_when_agent_idle` (`accept_chats_when_agent_idle`) means agents should receive chats even when idle/offline, so the offline filter must be removed in that path. Bots are always status-agnostic and are always included regardless of their online/offline status. Do not flag this as a bug.
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: 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/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/server/lib/parseMessageSearchQuery.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/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-12T17:12:49.121Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:28-33
Timestamp: 2026-03-12T17:12:49.121Z
Learning: Rocket.Chat — apps/meteor/client/lib/chats/flows/uploadFiles.ts: When E2E_Enable_Encrypt_Files is disabled, plaintext file uploads are allowed in E2E rooms; this fallback is expected and should not be flagged as a security regression.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
📚 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/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 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/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/server/lib/parseMessageSearchQuery.ts
🔇 Additional comments (1)
apps/meteor/server/lib/parseMessageSearchQuery.ts (1)
247-278:⚠️ Potential issue | 🟠 MajorThe plain-text regex branch regresses normal search semantics.
This now both broadens single-character searches and narrows multi-word searches:
abecomes a very broad query, whilegitlab alertonly matches that exact phrase in that order. Please preserve the existing plain-text matching behavior and only expand the searched fields to attachments.⛔ Skipped due to 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: 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.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.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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:274">
P2: forceRegex is still accepted and passed (e.g., messageSearch uses Message_AlwaysSearchRegExp), but consumeMessageText no longer honors it. Non-/pattern/flags input is always escaped, so callers relying on forceRegex for raw regex will silently get literal matches instead of regex semantics.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/meteor/server/lib/parseMessageSearchQuery.ts (1)
247-280:⚠️ Potential issue | 🟠 Major
forceRegexproperty is still not used, breakingMessage_AlwaysSearchRegExpsetting.Despite the past review comment being marked as addressed, the current code still does not reference
this.forceRegexanywhere inconsumeMessageText. The property is set at line 35 but never checked, meaning theMessage_AlwaysSearchRegExpsetting has no effect.When
forceRegexis true, the plain text should be treated as a regex pattern rather than being escaped.Proposed fix to honor forceRegex
private consumeMessageText(text: string) { text = text.trim().replace(/\s\s+/g, ' '); if (!text || text.length < 2) { delete this.query.$or; return ''; } const match = text.match(/^\/(.+)\/([imxs]*)$/); - if (match) { + if (match || this.forceRegex) { - const regex = match[1]; - const options = match[2]; + const regex = match ? match[1] : text; + const options = match ? match[2] : 'i'; this.query.$or = [ { msg: { $regex: regex, $options: options } }, { 'attachments.text': { $regex: regex, $options: options } }, { 'attachments.title': { $regex: regex, $options: options } }, { 'attachments.description': { $regex: regex, $options: options } }, ]; return text; } const safeText = escapeRegExp(text);🤖 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 247 - 280, The consumeMessageText method ignores this.forceRegex so Message_AlwaysSearchRegExp has no effect; modify consumeMessageText to check this.forceRegex when the input is not already a /.../pattern: if this.forceRegex is true, treat the raw text as a regex pattern (do not call escapeRegExp) and use it in the $regex/$options entries (same as when match found), otherwise continue to use escapeRegExp and the case‑insensitive 'i' option; additionally, guard regex creation by catching invalid regex errors (from new RegExp or similar) and fall back to the escaped pattern to avoid throwing. Ensure references to this.forceRegex, consumeMessageText, and escapeRegExp are updated.
🧹 Nitpick comments (2)
apps/meteor/server/lib/parseMessageSearchQuery.ts (2)
244-281: Inconsistent indentation: uses spaces instead of tabs.The
consumeMessageTextmethod uses 2-space indentation while the rest of the file uses tabs. Reformat to use tabs for consistency with the class.🤖 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 244 - 281, The consumeMessageText method uses 2-space indentation inconsistent with the file's tab-based style; reformat the entire consumeMessageText function (including the JSDoc comment, all control blocks, array literals, and return statements) to use tabs instead of spaces so it matches the surrounding class formatting and lint rules, ensuring no other code changes are made and keeping the same logic and spacing inside strings/regexes; locate the function by its name consumeMessageText to apply the change.
261-266: Potential performance impact:$regexon unindexed attachment fields.The
$orquery uses$regexonattachments.text,attachments.title, andattachments.description. Per the Messages model index definitions, these fields have no indexes. This will result in collection scans for the regex portions of the query, which may be slow on large message collections.Consider adding compound or partial indexes on these attachment fields if search performance becomes problematic in production. Monitor query execution times after deployment.
Also applies to: 273-278
🤖 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 261 - 266, The use of $regex against unindexed attachment fields in parseMessageSearchQuery (the this.query.$or block referencing attachments.text, attachments.title, attachments.description) will force collection scans; to fix, add appropriate indexes for these fields (e.g., text or compound/partial indexes tailored to your query patterns) and/or revise the query to use indexed fields or a text index; update the Messages model/index definitions to include indexes for attachments.text, attachments.title, and attachments.description (or create a composite/partial index if you only search when certain flags are set), then deploy and monitor query execution times to validate performance.
🤖 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/server/lib/parseMessageSearchQuery.ts`:
- Around line 273-278: The tests expect a $text search and textScore projection
but the implementation in parseMessageSearchQuery.ts builds this.query.$or with
case-insensitive $regex on fields msg, attachments.text, attachments.title, and
attachments.description and leaves projection empty; update
parseMessageSearchQuery.spec.ts to assert the query uses an $or array of regex
clauses (matching safeText / case-insensitive) and remove any assertions
expecting a projection with score: { $meta: 'textScore' }, instead assert
options.projection is empty (or omitted) and options.sort/skip/limit match the
implementation.
---
Duplicate comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 247-280: The consumeMessageText method ignores this.forceRegex so
Message_AlwaysSearchRegExp has no effect; modify consumeMessageText to check
this.forceRegex when the input is not already a /.../pattern: if this.forceRegex
is true, treat the raw text as a regex pattern (do not call escapeRegExp) and
use it in the $regex/$options entries (same as when match found), otherwise
continue to use escapeRegExp and the case‑insensitive 'i' option; additionally,
guard regex creation by catching invalid regex errors (from new RegExp or
similar) and fall back to the escaped pattern to avoid throwing. Ensure
references to this.forceRegex, consumeMessageText, and escapeRegExp are updated.
---
Nitpick comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 244-281: The consumeMessageText method uses 2-space indentation
inconsistent with the file's tab-based style; reformat the entire
consumeMessageText function (including the JSDoc comment, all control blocks,
array literals, and return statements) to use tabs instead of spaces so it
matches the surrounding class formatting and lint rules, ensuring no other code
changes are made and keeping the same logic and spacing inside strings/regexes;
locate the function by its name consumeMessageText to apply the change.
- Around line 261-266: The use of $regex against unindexed attachment fields in
parseMessageSearchQuery (the this.query.$or block referencing attachments.text,
attachments.title, attachments.description) will force collection scans; to fix,
add appropriate indexes for these fields (e.g., text or compound/partial indexes
tailored to your query patterns) and/or revise the query to use indexed fields
or a text index; update the Messages model/index definitions to include indexes
for attachments.text, attachments.title, and attachments.description (or create
a composite/partial index if you only search when certain flags are set), then
deploy and monitor query execution times to validate performance.
🪄 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: 871b291b-9eb2-4d6b-a7a5-5fda745423ff
📒 Files selected for processing (1)
apps/meteor/server/lib/parseMessageSearchQuery.ts
📜 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/server/lib/parseMessageSearchQuery.ts
🧠 Learnings (15)
📚 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/lib/parseMessageSearchQuery.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-03-16T22:56:54.500Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39677
File: packages/models/src/helpers/omnichannel/agentStatus.ts:10-29
Timestamp: 2026-03-16T22:56:54.500Z
Learning: In `packages/models/src/helpers/omnichannel/agentStatus.ts` (PR `#39677`), the `queryStatusAgentOnline` function intentionally omits the `$or` offline-status guard for non-bot agents when `isLivechatEnabledWhenAgentIdle === true`. This is by design: the setting `Livechat_enabled_when_agent_idle` (`accept_chats_when_agent_idle`) means agents should receive chats even when idle/offline, so the offline filter must be removed in that path. Bots are always status-agnostic and are always included regardless of their online/offline status. Do not flag this as a bug.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.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/server/lib/parseMessageSearchQuery.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/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-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/server/lib/parseMessageSearchQuery.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/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-12T17:12:49.121Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:28-33
Timestamp: 2026-03-12T17:12:49.121Z
Learning: Rocket.Chat — apps/meteor/client/lib/chats/flows/uploadFiles.ts: When E2E_Enable_Encrypt_Files is disabled, plaintext file uploads are allowed in E2E rooms; this fallback is expected and should not be flagged as a security regression.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
📚 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/lib/parseMessageSearchQuery.ts
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, PBKDF2 iteration count validation is not enforced because the iterations parameter is outside the user's control and is system-managed.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2025-09-15T13:10:30.049Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 36868
File: packages/http-router/src/Router.ts:416-425
Timestamp: 2025-09-15T13:10:30.049Z
Learning: In packages/http-router/src/Router.ts, the dispatch() method's use of replaceAll('//', '/') on the full path is acceptable because URL normalization and query string handling is performed by the caller function before dispatch() is invoked.
Applied to files:
apps/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 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/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/server/lib/parseMessageSearchQuery.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/meteor/server/lib/parseMessageSearchQuery.ts (2)
244-246: Code comments should be removed per coding guidelines.The comment block violates the project's coding guideline for TypeScript files. Consider removing it or moving the documentation to the function signature's JSDoc if needed elsewhere.
As per coding guidelines: "Avoid code comments in the implementation" for
**/*.{ts,tsx,js}files.🤖 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 244 - 246, Remove the inline implementation comment block ("Query in message text + attachments") that violates the project's no-code-comments guideline; if the information is needed, move it into the function JSDoc for parseMessageSearchQuery (or the surrounding exported function/type) so the intent is preserved without inline comments in the implementation.
270-275: Attachment search adds unindexed collection scans; consider adding indexes for large deployments.The new attachment search functionality uses
$regexon unindexed fields (attachments.text,attachments.title,attachments.description), which will cause full collection scans. Additionally,$regexqueries cannot leverage the existing text index on themsgfield—text indexes only work with the$textoperator.For large deployments with millions of messages, consider:
- Adding indexes on frequently-searched attachment fields
- Documenting the performance characteristics of attachment search
- Monitoring query performance once deployed
🤖 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 270 - 275, The new search adds $regex on unindexed fields (this.query.$or with attachments.text, attachments.title, attachments.description) which will cause full collection scans; add appropriate indexes and/or a text index that covers attachment fields and update queries to use indexed operators (or create separate indexes for attachments.text/title/description and/or a compound/text index that includes these fields) to avoid full collection scans, and update docs to note attachment-search performance characteristics and add monitoring/alerts for query latency on the message collection so you can detect regressions at scale.
🤖 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/server/lib/parseMessageSearchQuery.ts`:
- Around line 260-263: The code currently overwrites the default
case-insensitive flag when parsing regex literals because match[2] can be an
empty string; update the assignment in the parse block (where regex, options and
match are used) so that options is only set to match[2] when it's non-empty
(e.g., options = match[2] || 'i' or check truthiness), leaving the existing
default 'i' intact when the user provides no flags; adjust the logic around the
regex/options assignment in parseMessageSearchQuery so empty flags don't produce
a case-sensitive search.
---
Nitpick comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 244-246: Remove the inline implementation comment block ("Query in
message text + attachments") that violates the project's no-code-comments
guideline; if the information is needed, move it into the function JSDoc for
parseMessageSearchQuery (or the surrounding exported function/type) so the
intent is preserved without inline comments in the implementation.
- Around line 270-275: The new search adds $regex on unindexed fields
(this.query.$or with attachments.text, attachments.title,
attachments.description) which will cause full collection scans; add appropriate
indexes and/or a text index that covers attachment fields and update queries to
use indexed operators (or create separate indexes for
attachments.text/title/description and/or a compound/text index that includes
these fields) to avoid full collection scans, and update docs to note
attachment-search performance characteristics and add monitoring/alerts for
query latency on the message collection so you can detect regressions at scale.
🪄 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: 3bed6d00-c052-4e7d-9672-929ffee57904
📒 Files selected for processing (1)
apps/meteor/server/lib/parseMessageSearchQuery.ts
📜 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). (1)
- GitHub Check: cubic · AI code reviewer
🧰 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/server/lib/parseMessageSearchQuery.ts
🧠 Learnings (15)
📚 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/lib/parseMessageSearchQuery.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/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2026-03-16T22:56:54.500Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39677
File: packages/models/src/helpers/omnichannel/agentStatus.ts:10-29
Timestamp: 2026-03-16T22:56:54.500Z
Learning: In `packages/models/src/helpers/omnichannel/agentStatus.ts` (PR `#39677`), the `queryStatusAgentOnline` function intentionally omits the `$or` offline-status guard for non-bot agents when `isLivechatEnabledWhenAgentIdle === true`. This is by design: the setting `Livechat_enabled_when_agent_idle` (`accept_chats_when_agent_idle`) means agents should receive chats even when idle/offline, so the offline filter must be removed in that path. Bots are always status-agnostic and are always included regardless of their online/offline status. Do not flag this as a bug.
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: 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/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/server/lib/parseMessageSearchQuery.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/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-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-03-12T17:12:49.121Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:28-33
Timestamp: 2026-03-12T17:12:49.121Z
Learning: Rocket.Chat — apps/meteor/client/lib/chats/flows/uploadFiles.ts: When E2E_Enable_Encrypt_Files is disabled, plaintext file uploads are allowed in E2E rooms; this fallback is expected and should not be flagged as a security regression.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
📚 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/lib/parseMessageSearchQuery.ts
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, PBKDF2 iteration count validation is not enforced because the iterations parameter is outside the user's control and is system-managed.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.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/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 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/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/server/lib/parseMessageSearchQuery.ts
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.
Fixes #2938
Problem
Search does not include attachment content such as:
Solution
Extended the message search query to include attachment fields
(attachments.text, attachments.title, attachments.description)
using regex-based matching.
Testing
Before
After
Summary by CodeRabbit
Bug Fixes
New Features