fix(search): include attachment fields in message search#39960
Open
NagajyothiChukka wants to merge 5 commits intoRocketChat:developfrom
Open
fix(search): include attachment fields in message search#39960NagajyothiChukka wants to merge 5 commits intoRocketChat:developfrom
NagajyothiChukka wants to merge 5 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent 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)
WalkthroughMessage search parsing now builds MongoDB Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 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:262">
P2: Explicit regex queries without flags (`/pattern/`) now become case-insensitive by default, changing previous case-sensitive behavior.</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: 1
🧹 Nitpick comments (1)
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts (1)
13-20: Add coverage for the new regex branches and the still-supported aliases.This table only exercises the escaped-literal path. It doesn't cover
/pattern/flags,forceRegex: true, or literal input containing regex metacharacters, and it no longer asserts aliases likehas:link,has:pin, orhas:mapeven though the parser still accepts them. A few targeted cases here would make the refactor much safer.Also applies to: 22-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts` around lines 13 - 20, Add unit tests in parseMessageSearchQuery.spec.ts to cover the regex parsing branches and legacy alias support: add cases where the query uses regex literal syntax like "/pattern/i" (ensure flags are parsed), where the parser is invoked with forceRegex: true, and where the input contains regex metacharacters (e.g., "foo.*bar") to verify escaping vs. regex behavior; also add assertions that alias tokens such as "has:link", "has:pin", and "has:map" are still recognized and produce the expected filter output. Use the existing textSearch helper pattern to assert constructed query objects and target the parser function used in this spec so each new case verifies the correct branch (/pattern/flags, forceRegex path, escaped-literal path) and alias handling.
🤖 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 247-278: The current consumeMessageText replaces plain-text
searches with $regex for msg and attachments; change it so only explicit regex
inputs (match from /^\/(.+)\/([imxs]*)$/ or this.forceRegex) set this.query.$or
with $regex entries for msg and attachments, while plain text queries instead
set this.query.$text = { $search: text } (and remove this.query.$or) so the
Messages text index and relevance ranking are used; update the logic in
consumeMessageText to branch on whether a regex was requested (use regex/options
and populate this.query.$or) or not (delete this.query.$or and set
this.query.$text), referencing the regex, options, forceRegex, and
this.query.$or/$text symbols to locate the code to change.
---
Nitpick comments:
In `@apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts`:
- Around line 13-20: Add unit tests in parseMessageSearchQuery.spec.ts to cover
the regex parsing branches and legacy alias support: add cases where the query
uses regex literal syntax like "/pattern/i" (ensure flags are parsed), where the
parser is invoked with forceRegex: true, and where the input contains regex
metacharacters (e.g., "foo.*bar") to verify escaping vs. regex behavior; also
add assertions that alias tokens such as "has:link", "has:pin", and "has:map"
are still recognized and produce the expected filter output. Use the existing
textSearch helper pattern to assert constructed query objects and target the
parser function used in this spec so each new case verifies the correct branch
(/pattern/flags, forceRegex path, escaped-literal path) and alias handling.
🪄 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: 95829177-6019-468a-b88a-65bff358cf9f
📒 Files selected for processing (2)
apps/meteor/server/lib/parseMessageSearchQuery.tsapps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.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 (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/lib/parseMessageSearchQuery.tsapps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
🧠 Learnings (14)
📚 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.tsapps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.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.tsapps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.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.tsapps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.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.tsapps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.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/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/unit/server/lib/parseMessageSearchQuery.spec.ts
Comment on lines
+247
to
+278
| private consumeMessageText(text: string) { | ||
| text = text.trim().replace(/\s\s+/g, ' '); | ||
|
|
||
| if (!text || text.length < 2) { | ||
| delete this.query.$or; | ||
| return ''; | ||
| } | ||
|
|
||
| return text; | ||
| let regex: string; | ||
| let options = 'i'; | ||
|
|
||
| const match = text.match(/^\/(.+)\/([imxs]*)$/); | ||
|
|
||
| if (match) { | ||
| regex = match[1]; | ||
| options = match[2] || 'i'; | ||
| } | ||
|
|
||
| else if (this.forceRegex) { | ||
| regex = text; | ||
| } | ||
|
|
||
| else { | ||
| regex = escapeRegExp(text); | ||
| } | ||
|
|
||
| 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 } }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== apps/meteor/server/lib/parseMessageSearchQuery.ts =="
nl -ba apps/meteor/server/lib/parseMessageSearchQuery.ts | sed -n '247,278p'
echo
echo "== apps/meteor/server/methods/messageSearch.ts =="
nl -ba apps/meteor/server/methods/messageSearch.ts | sed -n '47,83p'
echo
echo "== packages/models/src/models/Messages.ts =="
nl -ba packages/models/src/models/Messages.ts | sed -n '46,82p'Repository: RocketChat/Rocket.Chat
Length of output: 181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== apps/meteor/server/lib/parseMessageSearchQuery.ts (lines 247-278) =="
cat -n apps/meteor/server/lib/parseMessageSearchQuery.ts | sed -n '247,278p'
echo
echo "== apps/meteor/server/methods/messageSearch.ts (lines 47-83) =="
cat -n apps/meteor/server/methods/messageSearch.ts | sed -n '47,83p'
echo
echo "== packages/models/src/models/Messages.ts (lines 46-82) =="
cat -n packages/models/src/models/Messages.ts | sed -n '46,82p'Repository: RocketChat/Rocket.Chat
Length of output: 3947
Restore $text search for non-regex queries; reserve $regex for explicit regex input.
Lines 273–278 apply $regex to all plain searches, which changes matching semantics (multi-word queries must be contiguous and have no relevance ranking) and bypasses the existing text index on msg (line 55 of Messages.ts). The four attachment fields (attachments.text/title/description) have no corresponding indexes, so they are unindexed under this approach.
Reserve $regex for inputs that explicitly request it (regex syntax /pattern/ or forceRegex setting). For plain text, use $text to maintain word-boundary matching, relevance ranking, and the existing search index performance.
Current code (lines 273–278)
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 } },
];
🤖 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 - 278,
The current consumeMessageText replaces plain-text searches with $regex for msg
and attachments; change it so only explicit regex inputs (match from
/^\/(.+)\/([imxs]*)$/ or this.forceRegex) set this.query.$or with $regex entries
for msg and attachments, while plain text queries instead set this.query.$text =
{ $search: text } (and remove this.query.$or) so the Messages text index and
relevance ranking are used; update the logic in consumeMessageText to branch on
whether a regex was requested (use regex/options and populate this.query.$or) or
not (delete this.query.$or and set this.query.$text), referencing the regex,
options, forceRegex, and this.query.$or/$text symbols to locate the code to
change.
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 new implementation comment block.Line [244]-Line [246] introduces an implementation comment in a TS file; prefer self-explanatory code/function naming here.
As per coding guidelines,
**/*.{ts,tsx,js}: “Avoid code comments in the implementation”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/server/lib/parseMessageSearchQuery.ts` around lines 244 - 246, Remove the implementation comment block that reads "Query in message text + attachments" in the parseMessageSearchQuery code—delete that comment so the code relies on self-explanatory naming instead; locate the comment near the parseMessageSearchQuery function (or surrounding helper functions that build the message text + attachments query) and remove the three-line comment block starting with /** and ending with */.
🤖 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 258-263: The explicit regex parser currently only accepts flags
[imxs] so patterns like /foo/u are treated as plain text; update the regex in
parseMessageSearchQuery (the match = text.match(/^\/(.+)\/([imxs]*)$/) logic) to
include the Unicode flag by changing the character class to include 'u' (e.g.
[imxsu]) so the parser recognizes and extracts the 'u' flag into options for
MongoDB $regex; also remove the duplicated JSDoc block(s) labeled "/** Query in
message text */" (the duplicate at lines ~241–246) to avoid redundancy.
---
Nitpick comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 244-246: Remove the implementation comment block that reads "Query
in message text + attachments" in the parseMessageSearchQuery code—delete that
comment so the code relies on self-explanatory naming instead; locate the
comment near the parseMessageSearchQuery function (or surrounding helper
functions that build the message text + attachments query) and remove the
three-line comment block starting with /** and ending with */.
🪄 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: 5aab1020-3969-4693-aa9d-3330de301146
📒 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 (14)
📚 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-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-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-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-03-10T08:13:52.153Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
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-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.
Applied to files:
apps/meteor/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: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/server/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-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
Comment on lines
+258
to
+263
| const match = text.match(/^\/(.+)\/([imxs]*)$/); | ||
|
|
||
| if (match) { | ||
| regex = match[1]; | ||
| options = match[2]; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MongoDB official docs: what $optionsflags are accepted for$regex, and is u valid (even if redundant)?
💡 Result:
The official MongoDB documentation for the $regex query operator lists the following $options flags: i (case insensitivity), m (multiline anchors), x (extended whitespace/comments), s (dot matches newlines), and u (Unicode support). The u flag is explicitly accepted but redundant, as UTF is enabled by default for $regex. Aggregation operators like $regexMatch and $regexFind support i, m, s, and x but do not list u.
Citations:
- 1: https://www.mongodb.com/docs/manual/reference/operator/query/regex/
- 2: https://www.mongodb.com/docs/v4.2/reference/operator/query/regex/index.html
🏁 Script executed:
head -n 300 apps/meteor/server/lib/parseMessageSearchQuery.ts | tail -n 100Repository: RocketChat/Rocket.Chat
Length of output: 2825
🏁 Script executed:
cat -n apps/meteor/server/lib/parseMessageSearchQuery.ts | sed -n '240,285p'Repository: RocketChat/Rocket.Chat
Length of output: 1246
Add u flag support to explicit regex pattern matching.
Line 258 only allows [imxs] in the regex character class, so patterns like /foo/u fall back to plain-text search. MongoDB's $regex operator explicitly supports the u (Unicode) flag, so the parser should accept it.
Suggested patch
- const match = text.match(/^\/(.+)\/([imxs]*)$/);
+ const match = text.match(/^\/(.+)\/([imsxu]*)$/);Also note: lines 241–246 contain duplicate JSDoc blocks (/** Query in message text */ and /** Query in message text + attachments */). Consider removing the redundancy.
📝 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 match = text.match(/^\/(.+)\/([imxs]*)$/); | |
| if (match) { | |
| regex = match[1]; | |
| options = match[2]; | |
| } | |
| const match = text.match(/^\/(.+)\/([imsxu]*)$/); | |
| if (match) { | |
| regex = match[1]; | |
| options = match[2]; | |
| } |
🤖 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 - 263,
The explicit regex parser currently only accepts flags [imxs] so patterns like
/foo/u are treated as plain text; update the regex in parseMessageSearchQuery
(the match = text.match(/^\/(.+)\/([imxs]*)$/) logic) to include the Unicode
flag by changing the character class to include 'u' (e.g. [imxsu]) so the parser
recognizes and extracts the 'u' flag into options for MongoDB $regex; also
remove the duplicated JSDoc block(s) labeled "/** Query in message text */" (the
duplicate at lines ~241–246) to avoid redundancy.
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:281">
P1: Non-regex search now incorrectly requires both `$text` and attachment regex matches, narrowing results instead of matching message text OR 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: 3
🤖 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 248-267: The code currently normalizes whitespace in the variable
text before detecting regexes, which corrupts patterns like "/foo bar/"; change
the flow in parseMessageSearchQuery so regex detection/extraction happens
against the original raw text (use the pre-normalized text for the match with
/^\/(.+)\/([imxs]*)$/ and this.forceRegex logic), then apply
text.trim().replace(/\s\s+/g, ' ') only for non-regex handling; ensure you still
delete this.query.$or and this.query.$text and return '' when appropriate, and
keep references to variables match, regex, options and this.forceRegex
consistent.
- Around line 279-289: The current else branch sets this.query.$text and
separately sets this.query.$or, causing an implicit AND between $text and the
$or clause; to fix, move the message text condition into the same $or array so
text-index matches are OR'd with attachment matches: instead of leaving
this.query.$text plus a separate this.query.$or, include an entry { msg: {
$text: { $search: text } } } or (better) add the msg condition that mirrors the
$text search semantics into this.query.$or alongside { 'attachments.text'... },
{ 'attachments.title'... }, { 'attachments.description'... } so the final query
uses a single $or combining msg and attachment predicates (keep this.query.$text
only if you rely on text index scoring, otherwise remove it to avoid the
implicit AND).
- Around line 256-274: The code injects raw user regex into this.query.$or when
match or this.forceRegex is true; to fix, validate and safely construct the
RegExp before building this.query.$or: attempt to create a RegExp object from
the extracted regex and options inside a try/catch (using the same escapeRegExp
fallback used on the non-regex path), and on error either escape the pattern
(escapeRegExp(regex)) or reject it with a clear error; additionally enforce a
sane max pattern length (e.g. 100 chars) and refuse obviously dangerous inputs
before assigning to this.query.$or (refer to variables match, this.forceRegex,
regex, options, and this.query.$or and ensure Messages.find() receives only the
validated/sanitized regex).
🪄 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: 70d0ad79-39d2-478f-9055-9dc761332ba3
📒 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-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-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-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-03-10T08:13:52.153Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
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-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.
Applied to files:
apps/meteor/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: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/server/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
Comment on lines
+248
to
267
| text = text.trim().replace(/\s\s+/g, ' '); | ||
|
|
||
| if (text === '') { | ||
| return text; | ||
| } | ||
| if (!text || text.length < 2) { | ||
| delete this.query.$or; | ||
| delete this.query.$text; | ||
| return ''; | ||
| } | ||
|
|
||
| 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', | ||
| }; | ||
| const match = text.match(/^\/(.+)\/([imxs]*)$/); | ||
|
|
||
| if (match || this.forceRegex) { | ||
| let regex: string; | ||
| let options = 'i'; | ||
|
|
||
| if (match) { | ||
| regex = match[1]; | ||
| options = match[2]; | ||
| } else { | ||
| this.query.$text = { | ||
| $search: text, | ||
| }; | ||
| this.options.projection = { | ||
| score: { | ||
| $meta: 'textScore', | ||
| }, | ||
| }; | ||
| regex = text; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/meteor/server/lib/parseMessageSearchQuery.ts | head -300 | tail -100Repository: RocketChat/Rocket.Chat
Length of output: 3383
🏁 Script executed:
# Verify if text is used elsewhere after normalization and before regex parsing
rg -n "consumeMessageText" apps/meteor/server/lib/parseMessageSearchQuery.tsRepository: RocketChat/Rocket.Chat
Length of output: 171
🏁 Script executed:
# Check if there are other code paths in parse() method that might call consumeMessageText
cat -n apps/meteor/server/lib/parseMessageSearchQuery.ts | sed -n '294,330p'Repository: RocketChat/Rocket.Chat
Length of output: 1617
Whitespace normalization corrupts regex patterns before parsing.
Line 248 normalizes repeated whitespace before the regex detection on line 256, so /foo bar/ becomes /foo bar/ before extraction at line 263. This silently changes regex semantics, and users cannot preserve multiple consecutive spaces in patterns. Regex patterns must be extracted before any whitespace cleanup.
🤖 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 248 - 267,
The code currently normalizes whitespace in the variable text before detecting
regexes, which corrupts patterns like "/foo bar/"; change the flow in
parseMessageSearchQuery so regex detection/extraction happens against the
original raw text (use the pre-normalized text for the match with
/^\/(.+)\/([imxs]*)$/ and this.forceRegex logic), then apply
text.trim().replace(/\s\s+/g, ' ') only for non-regex handling; ensure you still
delete this.query.$or and this.query.$text and return '' when appropriate, and
keep references to variables match, regex, options and this.forceRegex
consistent.
Comment on lines
+256
to
+274
| const match = text.match(/^\/(.+)\/([imxs]*)$/); | ||
|
|
||
| if (match || this.forceRegex) { | ||
| let regex: string; | ||
| let options = 'i'; | ||
|
|
||
| if (match) { | ||
| regex = match[1]; | ||
| options = match[2]; | ||
| } else { | ||
| this.query.$text = { | ||
| $search: text, | ||
| }; | ||
| this.options.projection = { | ||
| score: { | ||
| $meta: 'textScore', | ||
| }, | ||
| }; | ||
| regex = text; | ||
| } | ||
|
|
||
| return text; | ||
| 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 } }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "parseMessageSearchQuery.ts" | head -5Repository: RocketChat/Rocket.Chat
Length of output: 117
🏁 Script executed:
find . -type f -name "messageSearch.ts" | head -5Repository: RocketChat/Rocket.Chat
Length of output: 111
🏁 Script executed:
wc -l ./apps/meteor/server/lib/parseMessageSearchQuery.tsRepository: RocketChat/Rocket.Chat
Length of output: 121
🏁 Script executed:
sed -n '240,285p' ./apps/meteor/server/lib/parseMessageSearchQuery.tsRepository: RocketChat/Rocket.Chat
Length of output: 985
🏁 Script executed:
sed -n '1,353p' ./apps/meteor/server/lib/parseMessageSearchQuery.ts | tail -100Repository: RocketChat/Rocket.Chat
Length of output: 2648
🏁 Script executed:
cat -n ./apps/meteor/server/methods/messageSearch.ts | head -80Repository: RocketChat/Rocket.Chat
Length of output: 2914
🏁 Script executed:
sed -n '76,100p' ./apps/meteor/server/methods/messageSearch.tsRepository: RocketChat/Rocket.Chat
Length of output: 608
🏁 Script executed:
grep -n "try\|catch\|error" ./apps/meteor/server/methods/messageSearch.ts | head -20Repository: RocketChat/Rocket.Chat
Length of output: 183
🏁 Script executed:
# Check if there's any error handling or rate limiting at the Meteor method level
rg -n "error|catch|limit|rate" ./apps/meteor/server/methods/messageSearch.tsRepository: RocketChat/Rocket.Chat
Length of output: 484
🏁 Script executed:
# Look for any validation of regex before use
ast-grep --pattern 'match($_)' --type tsRepository: RocketChat/Rocket.Chat
Length of output: 245
🏁 Script executed:
# Look for any validation of regex patterns in the parseMessageSearchQuery file
rg -n "try|catch|valid|error" ./apps/meteor/server/lib/parseMessageSearchQuery.tsRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Check if there's application-level error handling that might wrap these calls
rg -n "messageSearch|catch.*Error" ./apps/meteor/server/methods/ -A 2 | head -40Repository: RocketChat/Rocket.Chat
Length of output: 1426
🏁 Script executed:
# Check if there's any global error handling middleware in Meteor methods
find ./apps/meteor/server -name "*.ts" -type f | xargs grep -l "onError\|errorHandler\|catch" | head -10Repository: RocketChat/Rocket.Chat
Length of output: 567
🏁 Script executed:
# Verify the regex pattern matching itself - check if the options validation is sufficient
sed -n '250,290p' ./apps/meteor/server/lib/parseMessageSearchQuery.tsRepository: RocketChat/Rocket.Chat
Length of output: 1015
Add validation guardrails around raw regex patterns before sending to MongoDB.
Lines 256–274 forward user-controlled regex patterns directly into $regex for msg and three attachment fields without validation. When forceRegex is enabled or user input matches /pattern/flags, the extracted pattern is passed directly to Messages.find() (line 79 of messageSearch.ts). Invalid patterns like /(/ cause unhandled MongoDB errors; expensive expressions like /a{999999}/ can force costly scans. Wrap the pattern in a try-catch block or validate it before constructing the query. Note that the non-regex path uses escapeRegExp() for safety—apply similar protection to the regex path.
🤖 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 256 - 274,
The code injects raw user regex into this.query.$or when match or
this.forceRegex is true; to fix, validate and safely construct the RegExp before
building this.query.$or: attempt to create a RegExp object from the extracted
regex and options inside a try/catch (using the same escapeRegExp fallback used
on the non-regex path), and on error either escape the pattern
(escapeRegExp(regex)) or reject it with a clear error; additionally enforce a
sane max pattern length (e.g. 100 chars) and refuse obviously dangerous inputs
before assigning to this.query.$or (refer to variables match, this.forceRegex,
regex, options, and this.query.$or and ensure Messages.find() receives only the
validated/sanitized regex).
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughMessage search parsing now builds MongoDB Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/meteor/server/lib/parseMessageSearchQuery.ts (2)
248-267:⚠️ Potential issue | 🟠 MajorRegex input is mutated before regex parsing.
Line 248 normalizes repeated spaces before Line 256 parses
/pattern/flags, so explicit regex like/foo bar/is changed to/foo bar/. Parse regex from raw trimmed text first; normalize whitespace only for non-regex search.Suggested flow fix
- text = text.trim().replace(/\s\s+/g, ' '); + const raw = text.trim(); - if (!text || text.length < 2) { + if (!raw || raw.length < 2) { delete this.query.$or; delete this.query.$text; return ''; } - const match = text.match(/^\/(.+)\/([imxs]*)$/); + const match = raw.match(/^\/(.+)\/([imxs]*)$/); if (match || this.forceRegex) { let regex: string; let options = 'i'; if (match) { regex = match[1]; options = match[2]; } else { - regex = text; + regex = raw; } + // ... + } else { + text = raw.replace(/\s\s+/g, ' '); + // ... }🤖 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 248 - 267, The code normalizes whitespace in text before checking for a regex literal, which mangles patterns like /foo bar/; update parseMessageSearchQuery logic so you first trim only (no whitespace collapsing) and test the raw trimmed text for the regex literal (the match = text.match(/^\/(.+)\/([imxs]*)$/) check and the forceRegex branch using this.forceRegex), and only after deciding it is NOT a regex literal perform the repeated-space normalization (text = text.trim().replace(/\s\s+/g, ' ')); ensure regex and options are derived from the original trimmed text when match is truthy.
258-274:⚠️ Potential issue | 🟠 MajorValidate user-provided regex before assigning to Mongo
$regex.In the explicit/forced regex path, raw
regex/optionsare forwarded directly. Invalid or pathological patterns can trigger query errors. Add a pre-validation guard (new RegExp(...)in try/catch) and fallback/reject behavior.Minimal guardrail patch
if (match || this.forceRegex) { let regex: string; let options = 'i'; if (match) { regex = match[1]; options = match[2]; } else { regex = text; } + + try { + // validate pattern/options before shipping to Mongo + // eslint-disable-next-line no-new + new RegExp(regex, options); + } catch { + regex = escapeRegExp(regex); + options = '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 } }, ];MongoDB behavior for invalid `$regex` patterns in find queries: does the server return a query error, and are there recommended validation safeguards before sending user-provided regex?🤖 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 - 274, In the block that handles explicit/forced regex (the logic using match, this.forceRegex, and assigning this.query.$or with $regex/$options), validate the user-supplied pattern and flags by attempting new RegExp(regex, options) inside a try/catch before assigning to the Mongo query; on success use the original assignment, and on failure either reject/throw a validation error or fall back to a safe behavior (e.g., skip the $regex branch or use an escaped literal match) so invalid or pathological patterns never get forwarded to MongoDB.
🧹 Nitpick comments (1)
apps/meteor/server/lib/parseMessageSearchQuery.ts (1)
280-280: Remove inline implementation comment.
// NORMAL SEARCHshould be removed to align with repository guidance.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` at line 280, Remove the inline implementation comment "// NORMAL SEARCH" from the parseMessageSearchQuery implementation so the code follows the repository guideline of avoiding inline implementation comments; edit the function or module containing parseMessageSearchQuery in apps/meteor/server/lib/parseMessageSearchQuery.ts and delete that comment line (reference symbol: parseMessageSearchQuery).
🤖 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 281-286: Remove the inline comment "// NORMAL SEARCH" and stop
placing {$text: {$search: text}} inside the same top-level this.query.$or array
with unindexed $regex clauses; instead move the text search into its own
top-level predicate (use this.query.$text = { $search: text }) and set
this.query.$or to contain only the attachment regex clauses (e.g.,
'attachments.text', 'attachments.title', 'attachments.description' using
escapeRegExp). Update the code paths that build this.query.$or so they only
include the regex branches (no $text), and ensure any logic that previously
relied on $text being in the $or now checks the separate this.query.$text
predicate; reference symbols: this.query.$or, this.query.$text, escapeRegExp,
'attachments.text', 'attachments.title', 'attachments.description'.
---
Duplicate comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 248-267: The code normalizes whitespace in text before checking
for a regex literal, which mangles patterns like /foo bar/; update
parseMessageSearchQuery logic so you first trim only (no whitespace collapsing)
and test the raw trimmed text for the regex literal (the match =
text.match(/^\/(.+)\/([imxs]*)$/) check and the forceRegex branch using
this.forceRegex), and only after deciding it is NOT a regex literal perform the
repeated-space normalization (text = text.trim().replace(/\s\s+/g, ' ')); ensure
regex and options are derived from the original trimmed text when match is
truthy.
- Around line 258-274: In the block that handles explicit/forced regex (the
logic using match, this.forceRegex, and assigning this.query.$or with
$regex/$options), validate the user-supplied pattern and flags by attempting new
RegExp(regex, options) inside a try/catch before assigning to the Mongo query;
on success use the original assignment, and on failure either reject/throw a
validation error or fall back to a safe behavior (e.g., skip the $regex branch
or use an escaped literal match) so invalid or pathological patterns never get
forwarded to MongoDB.
---
Nitpick comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Line 280: Remove the inline implementation comment "// NORMAL SEARCH" from the
parseMessageSearchQuery implementation so the code follows the repository
guideline of avoiding inline implementation comments; edit the function or
module containing parseMessageSearchQuery in
apps/meteor/server/lib/parseMessageSearchQuery.ts and delete that comment line
(reference symbol: parseMessageSearchQuery).
🪄 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: 34a0b185-3b52-44da-a5cf-294d59d4bf9e
📒 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 (14)
📚 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-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-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-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-03-10T08:13:52.153Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
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-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.
Applied to files:
apps/meteor/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: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2026-03-19T13:59:40.678Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/uploads.ts:45-49
Timestamp: 2026-03-19T13:59:40.678Z
Learning: In `apps/meteor/app/apps/server/converters/uploads.ts`, the `room` async handler in `convertToApp` uses non-null assertions (`upload.rid!` and `result!`) intentionally. The data flow guarantees that any upload reaching this point must have a `rid`; if it does not, throwing an error is the desired behavior (fail-fast / data integrity guard). Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/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
Comment on lines
+281
to
+286
| this.query.$or = [ | ||
| { $text: { $search: text } }, | ||
| { 'attachments.text': { $regex: escapeRegExp(text), $options: 'i' } }, | ||
| { 'attachments.title': { $regex: escapeRegExp(text), $options: 'i' } }, | ||
| { 'attachments.description': { $regex: escapeRegExp(text), $options: 'i' } }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MongoDB docs: when $textappears inside an$or, must all $orclauses be index-supported? What happens when sibling$orclauses are unindexed$regex predicates?
💡 Result:
Yes, when $text appears inside an $or, all $or clauses must be supported by the text index (i.e., all must be indexable via the text index). MongoDB will reject the query with an error if any clause cannot use the text index. If sibling $or clauses include unindexed $regex predicates, the query fails because $regex cannot use the text index (text indexes support $text queries, not $regex). MongoDB requires the text index to support the entire $or for $text-in-$or queries, so unindexed clauses like $regex prevent index usage and cause the query to be rejected. This is explicitly stated in the official MongoDB documentation for the $text operator: "All $or clauses must be indexed to use $text."
Citations:
- 1: https://www.mongodb.com/docs/manual/reference/operator/query/text/
- 2: https://www.mongodb.com/docs/v8.0/reference/operator/query/text/
🏁 Script executed:
cat -n apps/meteor/server/lib/parseMessageSearchQuery.ts | sed -n '270,300p'Repository: RocketChat/Rocket.Chat
Length of output: 1884
🏁 Script executed:
# Search for text index configuration or creation
rg -i "text.*index|createIndex.*text|ensureIndex" --type ts --type jsRepository: RocketChat/Rocket.Chat
Length of output: 1891
🏁 Script executed:
# Check if there are tests for this parsing logic
fd -t f "\.test\.|\.spec\." | xargs rg -l "parseMessageSearchQuery\|consumeMessageText" 2>/dev/nullRepository: RocketChat/Rocket.Chat
Length of output: 50
$text inside $or with unindexed attachment $regex clauses will be rejected by MongoDB at runtime.
MongoDB requires all $or clauses to be indexable when $text is used. The code at lines 281-286 places $text alongside unindexed $regex predicates on attachment fields, causing MongoDB to reject the entire query. This is a query execution error, not a performance issue.
Also, remove the inline comment // NORMAL SEARCH at line 279 to comply with project guidelines.
Proposed fix: regex-only $or branch
else {
- // NORMAL SEARCH
- this.query.$or = [
- { $text: { $search: text } },
- { 'attachments.text': { $regex: escapeRegExp(text), $options: 'i' } },
- { 'attachments.title': { $regex: escapeRegExp(text), $options: 'i' } },
- { 'attachments.description': { $regex: escapeRegExp(text), $options: 'i' } },
- ];
+ const escaped = escapeRegExp(text);
+ this.query.$or = [
+ { msg: { $regex: escaped, $options: 'i' } },
+ { 'attachments.text': { $regex: escaped, $options: 'i' } },
+ { 'attachments.title': { $regex: escaped, $options: 'i' } },
+ { 'attachments.description': { $regex: escaped, $options: 'i' } },
+ ];
}🤖 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 281 - 286,
Remove the inline comment "// NORMAL SEARCH" and stop placing {$text: {$search:
text}} inside the same top-level this.query.$or array with unindexed $regex
clauses; instead move the text search into its own top-level predicate (use
this.query.$text = { $search: text }) and set this.query.$or to contain only the
attachment regex clauses (e.g., 'attachments.text', 'attachments.title',
'attachments.description' using escapeRegExp). Update the code paths that build
this.query.$or so they only include the regex branches (no $text), and ensure
any logic that previously relied on $text being in the $or now checks the
separate this.query.$text predicate; reference symbols: this.query.$or,
this.query.$text, escapeRegExp, 'attachments.text', 'attachments.title',
'attachments.description'.
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 did not include attachment content such as:
Solution
Testing
Notes
This is a bug fix and does not require a changeset.
Summary by CodeRabbit