chore: Add OpenAPI support for the Rocket.Chat dm/im APIs endpoints#39340
Closed
ahmed-n-abdeltwab wants to merge 9 commits intoRocketChat:developfrom
Closed
chore: Add OpenAPI support for the Rocket.Chat dm/im APIs endpoints#39340ahmed-n-abdeltwab wants to merge 9 commits intoRocketChat:developfrom
ahmed-n-abdeltwab wants to merge 9 commits intoRocketChat:developfrom
Conversation
- migrating to a modern chained route definition syntax and utilizing shared AJV schemas for validation to enhance API documentation and ensure type safety through response validation.
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 |
🦋 Changeset detectedLatest commit: b03afc9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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:
WalkthroughRefactors DM/IM server routes to unified API.v1.addRoute handlers with shared AJV validation and pagination; removes multiple request-type exports/validators from rest-typings; adjusts public endpoint typings and expands AJV schema union to include IRoom and IUser. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API.v1.addRoute
participant Auth as Auth/Permissions
participant Validator as AJV Validator
participant DB as Database
Client->>API: HTTP GET/POST /v1/im.* or /v1/dm.*
API->>Auth: verify auth & permissions
Auth-->>API: allowed/denied
API->>Validator: validate request (isDmXProps / PaginatedRequest)
Validator-->>API: valid/errors
alt valid & allowed
API->>DB: query rooms/files/members/messages (paginated)
DB-->>API: results
API-->>Client: 200 {payload}
else invalid or denied
API-->>Client: 4xx error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
- migrating to a modern chained route definition syntax and utilizing shared AJV schemas for validation to enhance API documentation and ensure type safety through response validation.
- migrating to a modern chained route definition syntax and utilizing shared AJV schemas for validation to enhance API documentation and ensure type safety through response validation.
![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: 5
🧹 Nitpick comments (1)
apps/meteor/app/api/server/v1/im.ts (1)
1384-1397: Response type excludes fields returned by implementation.The
DmEndpointstype for/v1/im.membersand/v1/dm.membersdeclares the response withPick<IUser, '_id' | 'status' | 'name' | 'username' | 'utcOffset'>, but the implementation (lines 457-466) projects additional fields:statusText,federated, andfreeSwitchExtension. Consider updating the type to reflect all returned fields for accurate documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/im.ts` around lines 1384 - 1397, The declared response type for DmEndpoints '/v1/im.members' and '/v1/dm.members' is missing fields actually returned by the implementation (statusText, federated, freeSwitchExtension); update the response shape to include these fields in the user pick (or replace the Pick with a properly typed interface) so the PaginatedResult members array matches the projection used by the implementation (referencing the DmEndpoints definition and the implementation that projects statusText, federated, and freeSwitchExtension).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 189-211: The AJV schema for DmListEveryoneProps
(isDmListEveryoneProps) does not mark "query" as required while the TypeScript
type DmListEveryoneProps declares query: string (required); update the AJV
schema used in isDmListEveryoneProps to include a "required" array containing
"query" (or alternatively change the TypeScript type to make query optional) so
the runtime validation matches the type definition; locate the
isDmListEveryoneProps AJV compile block and add required: ["query"] to the
schema to fix the mismatch.
- Around line 578-596: The JSON schema DmClosePropsSchema requires userId but
the DmCloseProps type and dmCloseAction use this.userId (authenticated user)
instead of reading userId from the request body; update the schema to match the
type and behavior by removing userId from DmClosePropsSchema (remove the userId
property and from required) so only roomId is required, leaving DmCloseProps and
isDmCloseProps unchanged; ensure dmCloseAction continues to read userId from
this.userId.
- Around line 665-676: The AJV schema is missing the required constraint for
roomId which causes dmCountersAction to throw later; update the
isDmCountersProps schema to include required: ['roomId'] so validation fails
with a 400 when roomId is absent, and verify the DmCountersProps type/signature
still matches the schema (adjust types if necessary).
- Around line 153-156: The route registration call to API.v1.addRoute for
['dm.list','im.list'] is missing the validateParams option; update the options
object passed to API.v1.addRoute (the same object that currently contains
authRequired: true) to include validateParams: isDmListProps so the
isDmListProps validator is applied to the dm.list/im.list route.
- Around line 1349-1398: The DmEndpoints type declares endpoints '/v1/im.kick',
'/v1/dm.kick', '/v1/im.leave', and '/v1/dm.leave' but there are no corresponding
route implementations in this file; either remove those four entries from the
DmEndpoints type or add matching route handlers (e.g., addRoute/.post
implementations) implementing the kick and leave logic so the declared
signatures (DmKickProps, DmLeaveProps) are satisfied; locate the DmEndpoints
type and the file's route registration area to either delete the four endpoint
entries or implement POST handlers named for those routes that validate params
and execute the existing DM kick/leave behavior.
---
Nitpick comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 1384-1397: The declared response type for DmEndpoints
'/v1/im.members' and '/v1/dm.members' is missing fields actually returned by the
implementation (statusText, federated, freeSwitchExtension); update the response
shape to include these fields in the user pick (or replace the Pick with a
properly typed interface) so the PaginatedResult members array matches the
projection used by the implementation (referencing the DmEndpoints definition
and the implementation that projects statusText, federated, and
freeSwitchExtension).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 019f9781-68a7-442c-9ae9-c9140b945562
📒 Files selected for processing (13)
.changeset/wicked-bugs-itch.mdapps/meteor/app/api/server/v1/im.tspackages/core-typings/src/Ajv.tspackages/rest-typings/src/index.tspackages/rest-typings/src/v1/dm/DmCreateProps.tspackages/rest-typings/src/v1/dm/DmFileProps.tspackages/rest-typings/src/v1/dm/DmHistoryProps.tspackages/rest-typings/src/v1/dm/DmLeaveProps.tspackages/rest-typings/src/v1/dm/DmMembersProps.tspackages/rest-typings/src/v1/dm/DmMessagesProps.tspackages/rest-typings/src/v1/dm/dm.tspackages/rest-typings/src/v1/dm/im.tspackages/rest-typings/src/v1/dm/index.ts
💤 Files with no reviewable changes (9)
- packages/rest-typings/src/v1/dm/dm.ts
- packages/rest-typings/src/index.ts
- packages/rest-typings/src/v1/dm/DmHistoryProps.ts
- packages/rest-typings/src/v1/dm/index.ts
- packages/rest-typings/src/v1/dm/DmCreateProps.ts
- packages/rest-typings/src/v1/dm/DmMembersProps.ts
- packages/rest-typings/src/v1/dm/DmFileProps.ts
- packages/rest-typings/src/v1/dm/DmMessagesProps.ts
- packages/rest-typings/src/v1/dm/DmLeaveProps.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:
packages/core-typings/src/Ajv.tspackages/rest-typings/src/v1/dm/im.tsapps/meteor/app/api/server/v1/im.ts
🧠 Learnings (14)
📓 Common learnings
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.
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.
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
packages/core-typings/src/Ajv.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
packages/core-typings/src/Ajv.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:
packages/core-typings/src/Ajv.tspackages/rest-typings/src/v1/dm/im.tsapps/meteor/app/api/server/v1/im.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:
packages/core-typings/src/Ajv.tspackages/rest-typings/src/v1/dm/im.tsapps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
.changeset/wicked-bugs-itch.mdpackages/rest-typings/src/v1/dm/im.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
.changeset/wicked-bugs-itch.md
📚 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:
.changeset/wicked-bugs-itch.mdpackages/rest-typings/src/v1/dm/im.tsapps/meteor/app/api/server/v1/im.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:
.changeset/wicked-bugs-itch.md
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
.changeset/wicked-bugs-itch.md
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Applied to files:
packages/rest-typings/src/v1/dm/im.tsapps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-24T19:09:01.522Z
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:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-23T17:53:06.802Z
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:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
🔇 Additional comments (9)
packages/core-typings/src/Ajv.ts (2)
10-12: Good type-only import additions for schema coverage.Line 10 and Line 12 correctly add
IRoomandIUseras type-only imports, keeping runtime output unchanged while enabling stronger schema typing.
18-18: Schema root union update is consistent with OpenAPI migration needs.Line 18 appropriately extends the generated schema roots with
IRoom | IUser, which improves downstream schema availability through the exportedschemasentrypoint..changeset/wicked-bugs-itch.md (1)
1-7: LGTM!The changeset correctly documents the minor version bumps for the affected packages. The description accurately reflects the OpenAPI migration scope with chained route definitions and AJV schema validation.
packages/rest-typings/src/v1/dm/im.ts (1)
1-19: LGTM!The streamlined
ImEndpointsinterface correctly exposes only the/v1/im.createroute with appropriate typing. The localDmCreatePropstype definition aligns with the AJV schema in the meteor app implementation. The remaining DM/IM routes are appropriately declared via module augmentation in the meteor app, which is consistent with the OpenAPI migration pattern being established.apps/meteor/app/api/server/v1/im.ts (5)
1-35: LGTM!Imports are well-organized and all appear to be utilized throughout the file. The combination of core-typings, rest-typings validators, and ApiClass utilities provides proper type safety for the OpenAPI migration.
70-130: LGTM!The
DmCreatePropstype and AJV schema are well-aligned, correctly handling both singleusernameand comma-separatedusernamesvariants. The route registration for bothdm.createandim.createfollows the established aliasing pattern.
243-294: LGTM!The
DmFilePropsschema correctly usesoneOfto enforce the requirement of eitherroomIdorusername. The nullable properties accommodate query parameter behavior appropriately.
1321-1337: LGTM!The chained route definitions follow a clean, consistent pattern. Each endpoint is properly registered with its corresponding props configuration and action handler, with both
dm.*andim.*variants for aliasing.
1400-1403: LGTM!The module augmentation correctly extends the
@rocket.chat/rest-typingsEndpointsinterface with the combinedDmEndpointstype, making these routes available to consumers of the rest-typings package while keeping the implementation details in the meteor app.
Comment on lines
+189
to
+211
| type DmListEveryoneProps = PaginatedRequest<{ query: string; fields?: string }>; | ||
|
|
||
| const dmEndpoints = API.v1 | ||
| .post('im.delete', dmDeleteEndpointsProps, dmDeleteAction('im.delete')) | ||
| .post('dm.delete', dmDeleteEndpointsProps, dmDeleteAction('dm.delete')) | ||
| .post('dm.close', dmCloseEndpointsProps, dmCloseAction('dm.close')) | ||
| .post('im.close', dmCloseEndpointsProps, dmCloseAction('im.close')); | ||
| const isDmListEveryoneProps = ajv.compile<DmListEveryoneProps>({ | ||
| type: 'object', | ||
| properties: { | ||
| query: { | ||
| type: 'string', | ||
| }, | ||
| fields: { | ||
| type: 'string', | ||
| }, | ||
| offset: { | ||
| type: 'number', | ||
| }, | ||
| count: { | ||
| type: 'number', | ||
| }, | ||
| sort: { | ||
| type: 'string', | ||
| }, | ||
| }, | ||
| additionalProperties: false, | ||
| }); |
Contributor
There was a problem hiding this comment.
Schema-type mismatch: query is required in type but optional in schema.
DmListEveryoneProps declares query: string (required), but the AJV schema on lines 191-211 doesn't include query in a required array. This allows requests without query to pass validation while the TypeScript type expects it to be present.
Proposed fix
const isDmListEveryoneProps = ajv.compile<DmListEveryoneProps>({
type: 'object',
properties: {
query: {
type: 'string',
},
fields: {
type: 'string',
},
offset: {
type: 'number',
},
count: {
type: 'number',
},
sort: {
type: 'string',
},
},
+ required: ['query'],
additionalProperties: false,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/im.ts` around lines 189 - 211, The AJV schema
for DmListEveryoneProps (isDmListEveryoneProps) does not mark "query" as
required while the TypeScript type DmListEveryoneProps declares query: string
(required); update the AJV schema used in isDmListEveryoneProps to include a
"required" array containing "query" (or alternatively change the TypeScript type
to make query optional) so the runtime validation matches the type definition;
locate the isDmListEveryoneProps AJV compile block and add required: ["query"]
to the schema to fix the mismatch.
Comment on lines
+578
to
+596
| type DmCloseProps = { | ||
| roomId: string; | ||
| }; | ||
|
|
||
| const DmClosePropsSchema = { | ||
| type: 'object', | ||
| properties: { | ||
| roomId: { | ||
| type: 'string', | ||
| }, | ||
| userId: { | ||
| type: 'string', | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| API.v1.addRoute( | ||
| ['dm.messages.others', 'im.messages.others'], | ||
| { authRequired: true, permissionsRequired: ['view-room-administration'] }, | ||
| { | ||
| async get() { | ||
| if (settings.get('API_Enable_Direct_Message_History_EndPoint') !== true) { | ||
| throw new Meteor.Error('error-endpoint-disabled', 'This endpoint is disabled', { | ||
| route: '/api/v1/im.messages.others', | ||
| }); | ||
| } | ||
|
|
||
| const { roomId } = this.queryParams; | ||
| if (!roomId) { | ||
| throw new Meteor.Error('error-roomid-param-not-provided', 'The parameter "roomId" is required'); | ||
| } | ||
| required: ['roomId', 'userId'], | ||
| additionalProperties: false, | ||
| }; | ||
|
|
||
| const room = await Rooms.findOneById<Pick<IRoom, '_id' | 't'>>(roomId, { projection: { _id: 1, t: 1 } }); | ||
| if (!room || room?.t !== 'd') { | ||
| throw new Meteor.Error('error-room-not-found', `No direct message room found by the id of: ${roomId}`); | ||
| } | ||
| const isDmCloseProps = ajv.compile<DmCloseProps>(DmClosePropsSchema); |
Contributor
There was a problem hiding this comment.
Schema requires userId but type and action don't use it from body.
The DmClosePropsSchema requires both roomId and userId (line 592), but the DmCloseProps type only defines roomId, and the dmCloseAction reads userId from this.userId (the authenticated user), not from bodyParams. This mismatch will cause valid requests (containing only roomId) to fail AJV validation.
Proposed fix
const DmClosePropsSchema = {
type: 'object',
properties: {
roomId: {
type: 'string',
},
- userId: {
- type: 'string',
- },
},
- required: ['roomId', 'userId'],
+ required: ['roomId'],
additionalProperties: false,
};📝 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
| type DmCloseProps = { | |
| roomId: string; | |
| }; | |
| const DmClosePropsSchema = { | |
| type: 'object', | |
| properties: { | |
| roomId: { | |
| type: 'string', | |
| }, | |
| userId: { | |
| type: 'string', | |
| }, | |
| }, | |
| ); | |
| API.v1.addRoute( | |
| ['dm.messages.others', 'im.messages.others'], | |
| { authRequired: true, permissionsRequired: ['view-room-administration'] }, | |
| { | |
| async get() { | |
| if (settings.get('API_Enable_Direct_Message_History_EndPoint') !== true) { | |
| throw new Meteor.Error('error-endpoint-disabled', 'This endpoint is disabled', { | |
| route: '/api/v1/im.messages.others', | |
| }); | |
| } | |
| const { roomId } = this.queryParams; | |
| if (!roomId) { | |
| throw new Meteor.Error('error-roomid-param-not-provided', 'The parameter "roomId" is required'); | |
| } | |
| required: ['roomId', 'userId'], | |
| additionalProperties: false, | |
| }; | |
| const room = await Rooms.findOneById<Pick<IRoom, '_id' | 't'>>(roomId, { projection: { _id: 1, t: 1 } }); | |
| if (!room || room?.t !== 'd') { | |
| throw new Meteor.Error('error-room-not-found', `No direct message room found by the id of: ${roomId}`); | |
| } | |
| const isDmCloseProps = ajv.compile<DmCloseProps>(DmClosePropsSchema); | |
| type DmCloseProps = { | |
| roomId: string; | |
| }; | |
| const DmClosePropsSchema = { | |
| type: 'object', | |
| properties: { | |
| roomId: { | |
| type: 'string', | |
| }, | |
| }, | |
| required: ['roomId'], | |
| additionalProperties: false, | |
| }; | |
| const isDmCloseProps = ajv.compile<DmCloseProps>(DmClosePropsSchema); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/im.ts` around lines 578 - 596, The JSON schema
DmClosePropsSchema requires userId but the DmCloseProps type and dmCloseAction
use this.userId (authenticated user) instead of reading userId from the request
body; update the schema to match the type and behavior by removing userId from
DmClosePropsSchema (remove the userId property and from required) so only roomId
is required, leaving DmCloseProps and isDmCloseProps unchanged; ensure
dmCloseAction continues to read userId from this.userId.
Comment on lines
+665
to
+676
| const isDmCountersProps = ajv.compile<DmCountersProps>({ | ||
| type: 'object', | ||
| properties: { | ||
| roomId: { | ||
| type: 'string', | ||
| }, | ||
| userId: { | ||
| type: 'string', | ||
| }, | ||
| }, | ||
| additionalProperties: false, | ||
| }); |
Contributor
There was a problem hiding this comment.
Schema missing required for roomId.
The isDmCountersProps schema doesn't specify required: ['roomId'], but the dmCountersAction (line 732-734) throws an error if roomId is not provided. Adding the required constraint ensures validation fails early with a proper 400 response rather than a generic error.
Proposed fix
const isDmCountersProps = ajv.compile<DmCountersProps>({
type: 'object',
properties: {
roomId: {
type: 'string',
},
userId: {
type: 'string',
},
},
+ required: ['roomId'],
additionalProperties: false,
});📝 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 isDmCountersProps = ajv.compile<DmCountersProps>({ | |
| type: 'object', | |
| properties: { | |
| roomId: { | |
| type: 'string', | |
| }, | |
| userId: { | |
| type: 'string', | |
| }, | |
| }, | |
| additionalProperties: false, | |
| }); | |
| const isDmCountersProps = ajv.compile<DmCountersProps>({ | |
| type: 'object', | |
| properties: { | |
| roomId: { | |
| type: 'string', | |
| }, | |
| userId: { | |
| type: 'string', | |
| }, | |
| }, | |
| required: ['roomId'], | |
| additionalProperties: false, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/im.ts` around lines 665 - 676, The AJV schema
is missing the required constraint for roomId which causes dmCountersAction to
throw later; update the isDmCountersProps schema to include required: ['roomId']
so validation fails with a 400 when roomId is absent, and verify the
DmCountersProps type/signature still matches the schema (adjust types if
necessary).
Comment on lines
+1349
to
+1398
| type DmEndpoints = ExtractRoutesFromAPI<typeof dmEndpoints> & { | ||
| '/v1/im.kick': { | ||
| POST: (params: DmKickProps) => void; | ||
| }; | ||
| '/v1/dm.kick': { | ||
| POST: (params: DmKickProps) => void; | ||
| }; | ||
| '/v1/im.leave': { | ||
| POST: (params: DmLeaveProps) => void; | ||
| }; | ||
| '/v1/dm.leave': { | ||
| POST: (params: DmLeaveProps) => void; | ||
| }; | ||
| '/v1/im.list': { | ||
| GET: (params: PaginatedRequest<{ fields?: string }>) => PaginatedResult<{ ims: IRoom[] }>; | ||
| }; | ||
| '/v1/dm.list': { | ||
| GET: (params: PaginatedRequest<{ fields?: string }>) => PaginatedResult<{ ims: IRoom[] }>; | ||
| }; | ||
| '/v1/im.list.everyone': { | ||
| GET: (params: PaginatedRequest<{ query: string; fields?: string }>) => PaginatedResult<{ ims: IRoom[] }>; | ||
| }; | ||
| '/v1/dm.list.everyone': { | ||
| GET: (params: PaginatedRequest<{ query: string; fields?: string }>) => PaginatedResult<{ ims: IRoom[] }>; | ||
| }; | ||
| '/v1/im.files': { | ||
| GET: (params: DmFileProps) => PaginatedResult<{ | ||
| files: IUploadWithUser[]; | ||
| }>; | ||
| }; | ||
| '/v1/dm.files': { | ||
| GET: (params: DmFileProps) => PaginatedResult<{ | ||
| files: IUploadWithUser[]; | ||
| }>; | ||
| }; | ||
| '/v1/im.members': { | ||
| GET: (params: DmMemberProps) => PaginatedResult<{ | ||
| members: (Pick<IUser, '_id' | 'status' | 'name' | 'username' | 'utcOffset'> & { | ||
| subscription: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>; | ||
| })[]; | ||
| }>; | ||
| }; | ||
| '/v1/dm.members': { | ||
| GET: (params: DmMemberProps) => PaginatedResult<{ | ||
| members: (Pick<IUser, '_id' | 'status' | 'name' | 'username' | 'utcOffset'> & { | ||
| subscription: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>; | ||
| })[]; | ||
| }>; | ||
| }; | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for im.kick/dm.kick/im.leave/dm.leave endpoint implementations
rg -n "im\.kick|dm\.kick|im\.leave|dm\.leave" apps/meteor/app/api/server/Repository: RocketChat/Rocket.Chat
Length of output: 303
🏁 Script executed:
#!/bin/bash
# Search for endpoint implementations across entire API server directory
rg -n "addRoute.*im\.kick|addRoute.*dm\.kick|addRoute.*im\.leave|addRoute.*dm\.leave" apps/meteor/app/api/Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Get the im.ts file structure and search for all route definitions
wc -l apps/meteor/app/api/server/v1/im.tsRepository: RocketChat/Rocket.Chat
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Read the entire file to understand route definitions
cat -n apps/meteor/app/api/server/v1/im.ts | head -200Repository: RocketChat/Rocket.Chat
Length of output: 6932
🏁 Script executed:
#!/bin/bash
# Continue reading to find kick and leave endpoint definitions
sed -n '200,500p' apps/meteor/app/api/server/v1/im.ts | cat -nRepository: RocketChat/Rocket.Chat
Length of output: 8319
🏁 Script executed:
#!/bin/bash
# Search for kick and leave endpoints more broadly
rg -n "kick|leave" apps/meteor/app/api/server/v1/im.ts | head -40Repository: RocketChat/Rocket.Chat
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Read the section showing the DmEndpoints type definition
sed -n '1320,1403p' apps/meteor/app/api/server/v1/im.ts | cat -nRepository: RocketChat/Rocket.Chat
Length of output: 3794
🏁 Script executed:
#!/bin/bash
# Search entire apps/meteor for kick/leave endpoint implementations
rg -n "\.post\(.*kick|\.post\(.*leave" apps/meteor/app/api/ | head -20Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Search for any definition or implementation of kick/leave routes
rg -n "kick|leave" apps/meteor/app/api/server/ --type ts | grep -v "im.ts" | head -30Repository: RocketChat/Rocket.Chat
Length of output: 1056
🏁 Script executed:
#!/bin/bash
# Search for any post or addRoute in im.ts to verify all endpoints
rg -n "\.post\(|API\.v1\.addRoute" apps/meteor/app/api/server/v1/im.tsRepository: RocketChat/Rocket.Chat
Length of output: 787
🏁 Script executed:
#!/bin/bash
# Get full context of the addRoute calls to see which endpoints they register
for line in 110 153 213 296 424; do
echo "=== Line $line ==="
sed -n "${line},$((line+2))p" apps/meteor/app/api/server/v1/im.ts
doneRepository: RocketChat/Rocket.Chat
Length of output: 539
Remove kick and leave endpoints from DmEndpoints type or implement them.
The DmEndpoints type declares im.kick, dm.kick, im.leave, and dm.leave (lines 1350–1361), but these endpoints have no corresponding addRoute or .post() implementations in this file. Either implement these endpoints to match the type declaration, or remove them from the type if they are not intended to be exposed through this API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/im.ts` around lines 1349 - 1398, The
DmEndpoints type declares endpoints '/v1/im.kick', '/v1/dm.kick',
'/v1/im.leave', and '/v1/dm.leave' but there are no corresponding route
implementations in this file; either remove those four entries from the
DmEndpoints type or add matching route handlers (e.g., addRoute/.post
implementations) implementing the kick and leave logic so the declared
signatures (DmKickProps, DmLeaveProps) are satisfied; locate the DmEndpoints
type and the file's route registration area to either delete the four endpoint
entries or implement POST handlers named for those routes that validate params
and execute the existing DM kick/leave behavior.
Contributor
Author
There was a problem hiding this comment.
i add them temp becouse i have remove there files and in the next PR i will remove it dotn worry
Contributor
There was a problem hiding this comment.
@ahmed-n-abdeltwab, understood! Thanks for the context — keeping those stubs temporarily to maintain type compatibility while the underlying files are removed makes sense. Just make sure to track the cleanup in the follow-up PR so the unimplemented endpoints don't linger in the type declaration. Would you like me to open a follow-up GitHub issue to track the removal?
✏️ Learnings added
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:19.756Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
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.
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:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
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.
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:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
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.
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.
![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 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/app/api/server/v1/im.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/im.ts:155">
P2: The new strict query schema (`additionalProperties: false`) for `dm.list/im.list` will reject requests with any query parameters beyond `fields`, `offset`, `count`, and `sort`. Previously, the endpoint only required authentication and silently ignored extra parameters. This creates a breaking change for clients sending deprecated or extra query parameters, which will now receive HTTP 400 errors instead of successful responses. The TODO comment at line 161 acknowledges this is a breaking query parameter change.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
Author
|
@ggazzo 👍 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/meteor/app/api/server/v1/im.ts (3)
578-596:⚠️ Potential issue | 🟠 MajorDo not require
userIdin the close body.
DmClosePropsonly exposesroomId, and the action reads the caller fromthis.userId. KeepinguserIdrequired inDmClosePropsSchematurns valid room-only requests into 400s before the existing auth logic runs.Schema fix
const DmClosePropsSchema = { type: 'object', properties: { roomId: { type: 'string', }, - userId: { - type: 'string', - }, }, - required: ['roomId', 'userId'], + required: ['roomId'], additionalProperties: false, };Based on learnings: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups may be deferred to follow-ups to keep the migration PR focused.
Also applies to: 621-627
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/im.ts` around lines 578 - 596, DmClosePropsSchema currently requires userId which breaks calls that only send roomId because this.userId is used for the caller; update DmClosePropsSchema by removing 'userId' from the required array (or remove the userId property entirely if unused) so only roomId is required, keeping DmCloseProps, DmClosePropsSchema and isDmCloseProps intact and preserving existing auth behavior that reads this.userId.
189-211:⚠️ Potential issue | 🟡 MinorMake
queryoptional in the*.list.everyoneTS contract.
isDmListEveryonePropsdoes not requirequery, butDmListEveryonePropsand the exported route type still do. That makes the generated contract stricter than the runtime validator for no functional gain.Minimal fix
-type DmListEveryoneProps = PaginatedRequest<{ query: string; fields?: string }>; +type DmListEveryoneProps = PaginatedRequest<{ query?: string; fields?: string }>; ... '/v1/im.list.everyone': { - GET: (params: PaginatedRequest<{ query: string; fields?: string }>) => PaginatedResult<{ ims: IRoom[] }>; + GET: (params: PaginatedRequest<{ query?: string; fields?: string }>) => PaginatedResult<{ ims: IRoom[] }>; }; '/v1/dm.list.everyone': { - GET: (params: PaginatedRequest<{ query: string; fields?: string }>) => PaginatedResult<{ ims: IRoom[] }>; + GET: (params: PaginatedRequest<{ query?: string; fields?: string }>) => PaginatedResult<{ ims: IRoom[] }>; };Based on learnings: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups may be deferred to follow-ups to keep the migration PR focused.
Also applies to: 1368-1373
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/im.ts` around lines 189 - 211, The TypeScript contract DmListEveryoneProps currently marks query as required while the runtime validator is optional; update DmListEveryoneProps (and any exported route type using it, e.g., the *.list.everyone route type) to make query optional so the compile-time type matches the ajv validator isDmListEveryoneProps; locate the type declaration DmListEveryoneProps and change query: string to query?: string (and update any exported route type aliases that reference DmListEveryoneProps) to keep behavior unchanged.
660-676:⚠️ Potential issue | 🟡 MinorRequire
roomIdin the counters schema.
DmCountersPropsand the handler already treatroomIdas mandatory. Withoutrequired: ['roomId'], AJV accepts invalid calls and the endpoint fails later with a Meteor error instead of the validation path.Minimal fix
const isDmCountersProps = ajv.compile<DmCountersProps>({ type: 'object', properties: { roomId: { type: 'string', }, userId: { type: 'string', }, }, + required: ['roomId'], additionalProperties: false, });Also applies to: 731-734
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/im.ts` around lines 660 - 676, The AJV schema is missing the required constraint for roomId so invalid requests pass validation and later trigger Meteor errors; update the compiled schema used by isDmCountersProps to include required: ['roomId'] (matching the DmCountersProps type) so roomId is enforced at validation time, and apply the same fix to the other identical AJV schema instance in this file (the second isDmCountersProps-like schema around the later handler) to keep both handlers consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 132-151: The DmListProps schema removed the existing "query"
filter causing dm.list / im.list to lose behavior; restore the "query" property
in the AJV schema and ensure the route type still accepts it so the existing
handler (dm.list / im.list) can apply the filter as before—update the
ajv.compile call for DmListProps (and the exported route type) to include
properties.query: { type: 'object' } (or the exact shape previously used) and
keep additionalProperties:false so the handler using isDmListProps and the
dm.list/im.list route will receive and apply req.query as before.
---
Duplicate comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 578-596: DmClosePropsSchema currently requires userId which breaks
calls that only send roomId because this.userId is used for the caller; update
DmClosePropsSchema by removing 'userId' from the required array (or remove the
userId property entirely if unused) so only roomId is required, keeping
DmCloseProps, DmClosePropsSchema and isDmCloseProps intact and preserving
existing auth behavior that reads this.userId.
- Around line 189-211: The TypeScript contract DmListEveryoneProps currently
marks query as required while the runtime validator is optional; update
DmListEveryoneProps (and any exported route type using it, e.g., the
*.list.everyone route type) to make query optional so the compile-time type
matches the ajv validator isDmListEveryoneProps; locate the type declaration
DmListEveryoneProps and change query: string to query?: string (and update any
exported route type aliases that reference DmListEveryoneProps) to keep behavior
unchanged.
- Around line 660-676: The AJV schema is missing the required constraint for
roomId so invalid requests pass validation and later trigger Meteor errors;
update the compiled schema used by isDmCountersProps to include required:
['roomId'] (matching the DmCountersProps type) so roomId is enforced at
validation time, and apply the same fix to the other identical AJV schema
instance in this file (the second isDmCountersProps-like schema around the later
handler) to keep both handlers consistent.
🪄 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: 249dab7c-9df5-475d-a30d-9c5b173fd68e
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/im.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/app/api/server/v1/im.ts
🧠 Learnings (19)
📓 Common learnings
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.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:19.756Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 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.
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
📚 Learning: 2026-03-12T10:26:19.756Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:19.756Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 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:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-24T19:09:01.522Z
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:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/im.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/app/api/server/v1/im.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/app/api/server/v1/im.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-11T22:04:16.136Z
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:16.136Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-11T16:46:50.707Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39535
File: apps/meteor/app/apps/server/bridges/livechat.ts:249-249
Timestamp: 2026-03-11T16:46:50.707Z
Learning: In `apps/meteor/app/apps/server/bridges/livechat.ts`, `createVisitor()` intentionally does not propagate `externalIds` to `registerData`. This is by design: the method is deprecated (JSDoc: `deprecated Use createAndReturnVisitor instead. Note: This method does not support externalIds.`) to push callers toward `createAndReturnVisitor()`, which does support `externalIds`. Do not flag the missing `externalIds` field in `createVisitor()` as a bug.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-11T18:17:47.912Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/processMessageUploads.ts:112-119
Timestamp: 2026-03-11T18:17:47.912Z
Learning: In `apps/meteor/client/lib/chats/flows/processMessageUploads.ts`, when sending multiple file uploads, each file is confirmed via its own `/rooms.mediaConfirm/${rid}/${fileId}` call and produces a separate message. Only the first file's confirm payload carries the composed message text (`msg`); all subsequent files receive `msg: ''`. This one-message-per-file behavior is intentional by design — do not flag it as a bug or suggest batching into a single message.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-23T17:53:06.802Z
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:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/im.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/app/api/server/v1/im.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
🔇 Additional comments (1)
apps/meteor/app/api/server/v1/im.ts (1)
153-156: Please verifyaddRoutestill reads this validator.The other legacy
API.v1.addRoute(...)registrations in this file still pass AJV validators asvalidateParams. IfaddRoutehas not been updated to consumequery,dm.list/im.listwill skip validation here again.#!/bin/bash # Verify whether legacy addRoute consumes `query` or still expects `validateParams`. rg -nC4 '\baddRoute\s*\(' apps/meteor/app/api/server/ printf '\n---\n' rg -nC2 'validateParams:|query:\s*isDmListProps' apps/meteor/app/api/server/v1/im.ts
Comment on lines
+132
to
+151
| type DmListProps = PaginatedRequest<{ fields?: string }>; | ||
|
|
||
| const isDmListProps = ajv.compile<DmListProps>({ | ||
| type: 'object', | ||
| properties: { | ||
| roomId: { | ||
| fields: { | ||
| type: 'string', | ||
| }, | ||
| userId: { | ||
| offset: { | ||
| type: 'number', | ||
| }, | ||
| count: { | ||
| type: 'number', | ||
| }, | ||
| sort: { | ||
| type: 'string', | ||
| }, | ||
| }, | ||
| required: ['roomId', 'userId'], | ||
| additionalProperties: false, | ||
| }; | ||
|
|
||
| const isDmCloseProps = ajv.compile<DmCloseProps>(DmClosePropsSchema); | ||
|
|
||
| const dmCloseEndpointsProps = { | ||
| authRequired: true, | ||
| body: isDmCloseProps, | ||
| response: { | ||
| 400: validateBadRequestErrorResponse, | ||
| 401: validateUnauthorizedErrorResponse, | ||
| 403: validateForbiddenErrorResponse, | ||
| 200: ajv.compile<void>({ | ||
| type: 'object', | ||
| properties: { | ||
| success: { | ||
| type: 'boolean', | ||
| enum: [true], | ||
| }, | ||
| }, | ||
| required: ['success'], | ||
| additionalProperties: false, | ||
| }), | ||
| }, | ||
| }; | ||
| }); |
Contributor
There was a problem hiding this comment.
Keep the query filter on dm.list / im.list.
The new DmListProps / exported route type drop query, and the handler no longer applies it. Line 161 already calls out that this is a breaking change, so it should not ride along with a behavior-preserving OpenAPI migration.
Based on learnings: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups may be deferred to follow-ups to keep the migration PR focused.
Also applies to: 157-174, 1362-1367
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/im.ts` around lines 132 - 151, The DmListProps
schema removed the existing "query" filter causing dm.list / im.list to lose
behavior; restore the "query" property in the AJV schema and ensure the route
type still accepts it so the existing handler (dm.list / im.list) can apply the
filter as before—update the ajv.compile call for DmListProps (and the exported
route type) to include properties.query: { type: 'object' } (or the exact shape
previously used) and keep additionalProperties:false so the handler using
isDmListProps and the dm.list/im.list route will receive and apply req.query as
before.
Contributor
Author
There was a problem hiding this comment.
are you sure about what are you taking about ? you are miss reading the isDmListProps with DmClosePropsSchema both are two defrent thing and i havent change anyof them
Contributor
There was a problem hiding this comment.
@ahmed-n-abdeltwab, you're right that isDmListProps and DmClosePropsSchema are two completely separate schemas — I apologize for the confusing wording in my original comment.
To clarify, my concern is specifically about isDmListProps (the new schema for dm.list / im.list). The schema only includes fields, offset, count, and sort, with additionalProperties: false. This means a query parameter that the old endpoint previously accepted would now be rejected.
The TODO comment you added inside the handler itself is what caught my attention:
// TODO: CACHE: Add Breaking notice since we removed the query paramThis self-acknowledgment confirms the query param was intentionally removed — but since the PR is described as a behavior-preserving OpenAPI migration, dropping an accepted parameter is a breaking change that (per the established convention in this codebase) should ideally be a separate, explicitly flagged PR rather than bundled here.
The substance of the concern stands, but I agree my original comment was poorly worded by mentioning DmClosePropsSchema in the same breath. I'll note this for future clarity.
🐰 hops back to re-examine the evidence carefully
🧠 Learnings used
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:19.756Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
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.
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:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
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:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
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').
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.
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.
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.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/meteor/app/api/server/v1/im.ts (2)
563-577:⚠️ Potential issue | 🟠 Major
dm.closenow requires a body field the action never reads.
DmClosePropsanddmCloseAction()only useroomId, butDmClosePropsSchemastill requiresuserId. Valid callers sending just{ roomId }will fail AJV validation before the handler can use the authenticatedthis.userId.🩹 Proposed fix
const DmClosePropsSchema = { type: 'object', properties: { roomId: { type: 'string', }, - userId: { - type: 'string', - }, }, - required: ['roomId', 'userId'], + required: ['roomId'], additionalProperties: false, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/im.ts` around lines 563 - 577, The AJV schema DmClosePropsSchema requires userId even though the handler and DmCloseProps only use roomId, causing valid callers that rely on authenticated this.userId to fail validation; update DmClosePropsSchema to remove the userId requirement (either remove the userId property entirely or leave it optional) and change the required array to only include "roomId" so isDmCloseProps and dmCloseAction accept payloads like { roomId } while still disallowing additionalProperties if desired.
132-166:⚠️ Potential issue | 🟠 MajorRestore the
queryfilter ondm.list/im.list.The new request type, handler, and exported route contract all drop
query, so clients that previously filtered the DM list will now get different behavior in a PR that is supposed to be an OpenAPI migration only. Please keepqueryinDmListProps, thread it throughparseJsonQuery(), and include it in theRooms.findPaginated(...)selector; the/v1/im.listand/v1/dm.listtypings should match that restored contract too.Based on learnings: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Also applies to: 1343-1348
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/im.ts` around lines 132 - 166, Restore the removed "query" filter: update the DmListProps type to include query, make sure parseJsonQuery() reads and returns the query (e.g., const { sort = { name: 1 }, fields, query } = await this.parseJsonQuery()), and pass that query into the Rooms.findPaginated selector (merge it into the selector alongside { t: 'd', _id: { $in: subscriptions } }). Also ensure the exported route contract/typings for /v1/im.list and /v1/dm.list include the restored query property so the API surface matches the previous behavior (references: DmListProps, parseJsonQuery, Rooms.findPaginated).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 132-136: Define the missing AJV validator isDmListProps for
DmListProps before the API.v1.addRoute call: create a constant isDmListProps =
ajv.compile<DmListProps>(...) that validates an object with optional string
"fields" and numeric "offset" and "count" and optional string "sort" and sets
additionalProperties: false; then use that constant as the query validator
referenced in API.v1.addRoute(['dm.list','im.list'], { authRequired: true,
query: isDmListProps }, ...).
---
Duplicate comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 563-577: The AJV schema DmClosePropsSchema requires userId even
though the handler and DmCloseProps only use roomId, causing valid callers that
rely on authenticated this.userId to fail validation; update DmClosePropsSchema
to remove the userId requirement (either remove the userId property entirely or
leave it optional) and change the required array to only include "roomId" so
isDmCloseProps and dmCloseAction accept payloads like { roomId } while still
disallowing additionalProperties if desired.
- Around line 132-166: Restore the removed "query" filter: update the
DmListProps type to include query, make sure parseJsonQuery() reads and returns
the query (e.g., const { sort = { name: 1 }, fields, query } = await
this.parseJsonQuery()), and pass that query into the Rooms.findPaginated
selector (merge it into the selector alongside { t: 'd', _id: { $in:
subscriptions } }). Also ensure the exported route contract/typings for
/v1/im.list and /v1/dm.list include the restored query property so the API
surface matches the previous behavior (references: DmListProps, parseJsonQuery,
Rooms.findPaginated).
🪄 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: 8b1dbc35-7fae-4e41-82d1-53ede5221b19
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/im.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/app/api/server/v1/im.ts
🧠 Learnings (26)
📓 Common learnings
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.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:19.756Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 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.
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
📚 Learning: 2026-03-12T10:26:19.756Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:19.756Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-24T19:09:01.522Z
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:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 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:
apps/meteor/app/api/server/v1/im.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/app/api/server/v1/im.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/app/api/server/v1/im.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-11T22:04:16.136Z
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:16.136Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-11T16:46:50.707Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39535
File: apps/meteor/app/apps/server/bridges/livechat.ts:249-249
Timestamp: 2026-03-11T16:46:50.707Z
Learning: In `apps/meteor/app/apps/server/bridges/livechat.ts`, `createVisitor()` intentionally does not propagate `externalIds` to `registerData`. This is by design: the method is deprecated (JSDoc: `deprecated Use createAndReturnVisitor instead. Note: This method does not support externalIds.`) to push callers toward `createAndReturnVisitor()`, which does support `externalIds`. Do not flag the missing `externalIds` field in `createVisitor()` as a bug.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-23T17:53:06.802Z
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:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-01-26T18:26:01.279Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38227
File: apps/meteor/app/api/server/router.ts:44-49
Timestamp: 2026-01-26T18:26:01.279Z
Learning: In apps/meteor/app/api/server/router.ts, when retrieving bodyParams and queryParams from the Hono context via c.get(), do not add defensive defaults (e.g., ?? {}). The code should fail fast if these parameters are missing, as endpoint handlers expect them to be present and breaking here helps surface parsing problems rather than hiding them.
Applied to files:
apps/meteor/app/api/server/v1/im.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/app/api/server/v1/im.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/app/api/server/v1/im.ts
📚 Learning: 2026-03-11T18:15:43.905Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:43.905Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-11T18:17:47.912Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/processMessageUploads.ts:112-119
Timestamp: 2026-03-11T18:17:47.912Z
Learning: In `apps/meteor/client/lib/chats/flows/processMessageUploads.ts`, when sending multiple file uploads, each file is confirmed via its own `/rooms.mediaConfirm/${rid}/${fileId}` call and produces a separate message. Only the first file's confirm payload carries the composed message text (`msg`); all subsequent files receive `msg: ''`. This one-message-per-file behavior is intentional by design — do not flag it as a bug or suggest batching into a single message.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/app/api/server/v1/im.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/app/api/server/v1/im.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/app/api/server/v1/im.ts
Comment on lines
+132
to
+136
| type DmListProps = PaginatedRequest<{ fields?: string }>; | ||
|
|
||
| const dmDeleteAction = <Path extends string>(_path: Path): TypedAction<typeof dmDeleteEndpointsProps, Path> => | ||
| async function action() { | ||
| const { room } = await findDirectMessageRoom(this.bodyParams, this.userId); | ||
| API.v1.addRoute( | ||
| ['dm.list', 'im.list'], | ||
| { authRequired: true, query: isDmListProps }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bisDmListProps\b' apps/meteor/app/api/server/v1/im.ts
rg -nP '^\s*(import .*isDmListProps|const\s+isDmListProps\b|function\s+isDmListProps\b)' apps/meteor/app/api/server/v1/im.tsRepository: RocketChat/Rocket.Chat
Length of output: 116
🏁 Script executed:
head -50 apps/meteor/app/api/server/v1/im.tsRepository: RocketChat/Rocket.Chat
Length of output: 2558
🏁 Script executed:
rg -n 'DmListProps|isDmListProps' apps/meteor/app/api/server/v1/ --type tsRepository: RocketChat/Rocket.Chat
Length of output: 250
🏁 Script executed:
rg -rn 'isDmListProps' apps/meteor/app/api/server/ --type tsRepository: RocketChat/Rocket.Chat
Length of output: 136
🏁 Script executed:
rg -n 'const is.*Props\s*=' apps/meteor/app/api/server/v1/im.ts | head -20Repository: RocketChat/Rocket.Chat
Length of output: 802
🏁 Script executed:
sed -n '120,200p' apps/meteor/app/api/server/v1/im.tsRepository: RocketChat/Rocket.Chat
Length of output: 2135
Missing validator definition for isDmListProps.
Line 136 references isDmListProps as a query validator, but this constant is not defined anywhere in the file. Following the established pattern in this file (all other validators are defined via ajv.compile<*Props>(...)), you must add the missing validator definition for isDmListProps before the route handler, e.g.:
const isDmListProps = ajv.compile<DmListProps>({
type: 'object',
properties: {
fields: { type: 'string' },
offset: { type: 'number' },
count: { type: 'number' },
sort: { type: 'string' },
},
additionalProperties: false,
});
Without this, the file will fail TypeScript compilation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/im.ts` around lines 132 - 136, Define the
missing AJV validator isDmListProps for DmListProps before the API.v1.addRoute
call: create a constant isDmListProps = ajv.compile<DmListProps>(...) that
validates an object with optional string "fields" and numeric "offset" and
"count" and optional string "sort" and sets additionalProperties: false; then
use that constant as the query validator referenced in
API.v1.addRoute(['dm.list','im.list'], { authRequired: true, query:
isDmListProps }, ...).
Member
|
Hey, thanks for the contribution! 🙏 I'm closing this PR because the endpoint(s) covered here have already been migrated as part of a larger batch migration I'm working on (see #39820). I decided to go with a mass migration approach because reviewing individual PRs for each endpoint was taking too much of my time. That said, you're more than welcome to help by reviewing and testing the changes in #39820 to make sure everything is working correctly. Your input would be really valuable! Thanks again for your effort! 🚀 |
Contributor
Author
|
no problem at all, peace ✌️ |
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
2 participants
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.
Description:
This PR integrates OpenAPI support into the
Rocket.Chat API, migrate ofRocket.Chat APIendpoints to the new OpenAPI pattern. The update includes improved API documentation, enhanced type safety, and response validation using AJV.Key Changes:
Issue Reference:
Relates to #34983, part of the ongoing OpenAPI integration effort.
Testing:
Endpoints:
Looking forward to your feedback! 🚀
Summary by CodeRabbit
New Features
Improvements