chore: Add OpenAPI Support to rooms.favorite API#35995
Merged
ggazzo merged 9 commits intoRocketChat:developfrom Feb 23, 2026
Merged
chore: Add OpenAPI Support to rooms.favorite API#35995ggazzo merged 9 commits intoRocketChat:developfrom
ggazzo merged 9 commits intoRocketChat:developfrom
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 2186c0b 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 |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
|
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-rooms
branch
from
02f921e to
4bf8f3d
Compare
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-rooms
branch
from
4bf8f3d to
75d5e8a
Compare
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
1 similar comment
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
This was referenced Jul 17, 2025
Contributor
Author
cardoso
previously approved these changes
Aug 8, 2025
Contributor
Author
|
Looks fine |
… by 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.
ahmed-n-abdeltwab
force-pushed
the
feat/openapi-rooms
branch
from
dbacf9e to
0d4b946
Compare
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:
WalkthroughMoves Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as Rooms API
participant Validator as AJV Validator
participant Finder as Room Finder
participant Service as toggleFavoriteMethod
participant DB as Database
Client->>API: POST /rooms.favorite {roomId|roomName, favorite}
API->>Validator: validate(body)
Validator-->>API: valid / invalid
alt valid
API->>Finder: locate room by id or name
Finder-->>API: room
API->>Service: toggleFavoriteMethod(user, room, favorite)
Service->>DB: update favorite flag
DB-->>Service: success
Service-->>API: result
API-->>Client: 200 {success:true, roomId/roomName, favorite}
else invalid
API-->>Client: 400 Bad Request
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/app/api/server/v1/rooms.ts (1)
1085-1115: 🛠️ Refactor suggestion | 🟠 MajorInconsistent indentation in the
rooms.invitehandler.The
rooms.inviteblock has mismatched indentation compared to the other chained routes (e.g.,rooms.favoriteandrooms.roles). Specifically:
- Line 1087: route name
'rooms.invite'is not indented to match the.post(on Line 1086.- Line 1107:
try {has 3 levels of indentation while the rest of the handler body uses 1 level.- Lines 1113–1114: closing braces use inconsistent tab depth.
This appears to be a leftover from converting the standalone route into the chained pattern. Please re-indent this block to match the style used in the
rooms.favoritehandler below.♻️ Proposed indentation fix
) .post( - 'rooms.invite', - { - authRequired: true, - body: isRoomsInviteProps, - response: { - 400: validateBadRequestErrorResponse, - 401: validateUnauthorizedErrorResponse, - 200: ajv.compile<void>({ - type: 'object', - properties: { - success: { type: 'boolean', enum: [true] }, - }, - required: ['success'], - additionalProperties: false, - }), + 'rooms.invite', + { + authRequired: true, + body: isRoomsInviteProps, + response: { + 400: validateBadRequestErrorResponse, + 401: validateUnauthorizedErrorResponse, + 200: ajv.compile<void>({ + type: 'object', + properties: { + success: { type: 'boolean', enum: [true] }, + }, + required: ['success'], + additionalProperties: false, + }), + }, }, - }, - async function action() { - const { roomId, action } = this.bodyParams; - - try { - await FederationMatrix.handleInvite(roomId, this.userId, action); - return API.v1.success(); - } catch (error) { - return API.v1.failure({ error: `Failed to handle invite: ${error instanceof Error ? error.message : String(error)}` }); - } - + async function action() { + const { roomId, action } = this.bodyParams; + + try { + await FederationMatrix.handleInvite(roomId, this.userId, action); + return API.v1.success(); + } catch (error) { + return API.v1.failure({ error: `Failed to handle invite: ${error instanceof Error ? error.message : String(error)}` }); + } }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/rooms.ts` around lines 1085 - 1115, The `rooms.invite` route block has inconsistent indentation; reformat the chained .post call so its opening '.post(' and route name 'rooms.invite' align with other chained routes (e.g., rooms.favorite), and normalize the handler body indentation (including the try/catch and the closing braces) to use the same indentation level as other handlers; locate the `.post('rooms.invite', { authRequired: true, body: isRoomsInviteProps, ... }, async function action() { ... FederationMatrix.handleInvite(roomId, this.userId, action); return API.v1.success(); } )` and adjust whitespace so the function, try/catch, and return/API.v1.failure lines match the surrounding style.
🧹 Nitpick comments (1)
apps/meteor/app/api/server/v1/rooms.ts (1)
1137-1149: Redundantfavoriteexistence check — AJV already enforces this.Line 1140 checks
this.bodyParams.hasOwnProperty('favorite'), but theisRoomsFavoritePropsvalidator (Line 1120) already requiresfavoritein both schema variants. Iffavoriteis missing, the request is rejected before the handler executes. This check is dead code.♻️ Proposed simplification
async function action() { const { favorite } = this.bodyParams; - if (!this.bodyParams.hasOwnProperty('favorite')) { - return API.v1.failure("The 'favorite' param is required"); - } - const room = await findRoomByIdOrName({ params: this.bodyParams }); await toggleFavoriteMethod(this.userId, room._id, favorite); return API.v1.success(); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/rooms.ts` around lines 1137 - 1149, The handler's explicit existence check for the favorite param is redundant because the isRoomsFavoriteProps validator already enforces that favorite is present; remove the dead branch in the action() function (the this.bodyParams.hasOwnProperty('favorite') check and its API.v1.failure return) and let the validator reject invalid requests, keeping the existing extraction const { favorite } = this.bodyParams and subsequent calls to findRoomByIdOrName and toggleFavoriteMethod unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/tricky-boxes-type.mdapps/meteor/app/api/server/v1/rooms.tspackages/rest-typings/src/v1/rooms.ts
💤 Files with no reviewable changes (1)
- packages/rest-typings/src/v1/rooms.ts
🧰 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/rooms.ts
🧠 Learnings (6)
📚 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:
.changeset/tricky-boxes-type.mdapps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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/rooms.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/rooms.ts (3)
packages/rest-typings/src/v1/Ajv.ts (3)
ajv(24-24)validateBadRequestErrorResponse(47-47)validateUnauthorizedErrorResponse(70-70)apps/meteor/app/channel-settings/server/methods/saveRoomSettings.ts (1)
favorite(348-350)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(74-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
.changeset/tricky-boxes-type.md (1)
1-6: LGTM!Changeset correctly marks both packages as
patchfor the non-breaking OpenAPI migration, and the description is clear.apps/meteor/app/api/server/v1/rooms.ts (3)
928-936: LGTM!The
RoomsFavoriteunion type correctly models the two mutually exclusive request body shapes, consistent with theanyOfJSON schema below.
947-970: LGTM!Schema correctly uses
anyOfwithadditionalProperties: falseon each variant, ensuring exactly one ofroomIdorroomNameis accepted alongsidefavorite.
1152-1157: LGTM!The
RoomEndpointstype extraction from the chained builder and the module augmentation correctly propagate the new route types into@rocket.chat/rest-typings.
🤖 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/rooms.ts`:
- Around line 1122-1132: The 200 response schema compiled with ajv.compile in
the rooms.favorite response block allows any boolean for the success property;
update the schema used by ajv.compile (the object with properties.success) to
constrain success to only true by adding enum: [true] to the success property so
it matches the rooms.invite pattern and prevents { success: false } from
validating.
---
Outside diff comments:
In `@apps/meteor/app/api/server/v1/rooms.ts`:
- Around line 1085-1115: The `rooms.invite` route block has inconsistent
indentation; reformat the chained .post call so its opening '.post(' and route
name 'rooms.invite' align with other chained routes (e.g., rooms.favorite), and
normalize the handler body indentation (including the try/catch and the closing
braces) to use the same indentation level as other handlers; locate the
`.post('rooms.invite', { authRequired: true, body: isRoomsInviteProps, ... },
async function action() { ... FederationMatrix.handleInvite(roomId, this.userId,
action); return API.v1.success(); } )` and adjust whitespace so the function,
try/catch, and return/API.v1.failure lines match the surrounding style.
---
Nitpick comments:
In `@apps/meteor/app/api/server/v1/rooms.ts`:
- Around line 1137-1149: The handler's explicit existence check for the favorite
param is redundant because the isRoomsFavoriteProps validator already enforces
that favorite is present; remove the dead branch in the action() function (the
this.bodyParams.hasOwnProperty('favorite') check and its API.v1.failure return)
and let the validator reject invalid requests, keeping the existing extraction
const { favorite } = this.bodyParams and subsequent calls to findRoomByIdOrName
and toggleFavoriteMethod unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #35995 +/- ##
===========================================
- Coverage 70.58% 70.56% -0.03%
===========================================
Files 3178 3189 +11
Lines 111796 112703 +907
Branches 20213 20417 +204
===========================================
+ Hits 78914 79530 +616
- Misses 30831 31113 +282
- Partials 2051 2060 +9 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/rooms.ts
🧰 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/rooms.ts
🧠 Learnings (6)
📚 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/rooms.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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/rooms.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/rooms.ts (2)
apps/meteor/app/channel-settings/server/methods/saveRoomSettings.ts (1)
favorite(348-350)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(74-78)
🔇 Additional comments (3)
apps/meteor/app/api/server/v1/rooms.ts (3)
1152-1156: LGTM — type extraction and module augmentation are consistent with the consolidatedroomEndpointschain.
947-970: No action required forRoomsFavoriteSchema— theanyOfpattern is safe in this codebase.The shared
ajvinstance inpackages/rest-typings/src/v1/Ajv.tsdoes not configure theremoveAdditionaloption. It uses:coerceTypes: true,allowUnionTypes: true,code: { source: true }, anddiscriminator: true. Therefore, theanyOfwithadditionalProperties: falseinside each subschema will validate correctly without risk of silent data mutation. Valid requests like{ roomId, favorite }will pass the second branch of theanyOfas expected.Likely an incorrect or invalid review comment.
1104-1113:⚠️ Potential issue | 🟡 MinorFix the API response schema violation in the error handler.
The catch block returns
API.v1.failure({ error: ... }), which produces{success: false, error: "..."}. This response violates the declared 200 schema that requires{success: true}withadditionalProperties: false. Either return an appropriate error status code (4xx) or adjust the response schema to allow failure responses with error details.The endpoint is federation-specific by design (accept/reject invite TO a room); no breaking change concern applies.
⛔ Skipped due to learnings
Learnt from: sampaiodiego Repo: RocketChat/Rocket.Chat PR: 37532 File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927 Timestamp: 2025-12-09T20:01:07.355Z Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it. <!-- </add_learning>Learnt from: ricardogarim Repo: RocketChat/Rocket.Chat PR: 37377 File: apps/meteor/ee/server/hooks/federation/index.ts:86-88 Timestamp: 2025-11-04T16:49:19.107Z Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.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.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.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.
🤖 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/rooms.ts`:
- Around line 1107-1112: The catch block is returning API.v1.failure(...) which
violates the declared 200 response schema (success must be true); replace this
with throwing a proper Meteor.Error (or MeteorError) so the framework converts
it to a 4xx response consistent with other endpoints—specifically, in the
handler that calls FederationMatrix.handleInvite(roomId, this.userId, action)
remove the API.v1.failure(...) return and instead throw new Meteor.Error(...)
(including a short error code and the caught error message) so the route yields
a validated error response; alternatively, if you prefer to keep API.v1.failure,
update the route’s response schema to declare a matching 4xx error response, but
prefer throwing Meteor.Error for consistency.
- Around line 1138-1142: Remove the redundant runtime guard that checks
this.bodyParams.hasOwnProperty('favorite') in the Rooms favorite handler: AJV
already enforces required via isRoomsFavoriteProps / RoomsFavoriteSchema before
the action runs, and destructuring const { favorite } = this.bodyParams happens
before the check anyway, so delete the if
(!this.bodyParams.hasOwnProperty('favorite')) { return API.v1.failure(...) }
block and leave the destructuring and subsequent logic intact.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/meteor/app/api/server/v1/rooms.ts (1)
1094-1101:ajv.compile<void>loses the validated type — use the actual response shape.Both 200-response validators use
voidas the generic parameter, but the schema describes{ success: boolean }. This meansValidateFunction<void>is returned, stripping type information from any downstream consumer that inspects.data.♻️ Proposed fix (apply to both occurrences)
- 200: ajv.compile<void>({ + 200: ajv.compile<{ success: boolean }>({Also applies to: 1121-1132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/rooms.ts` around lines 1094 - 1101, The AJV validators are compiled with ajv.compile<void>, which loses the response type; change those generics to the actual response shape (e.g., ajv.compile<{ success: boolean }>) so the returned ValidateFunction has the correct typed `.data`; update both 200-response validators (the ajv.compile calls shown and the other occurrence at the later 1121-1132 block) to use the concrete type instead of void.
🤖 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/rooms.ts`:
- Around line 1138-1142: findRoomByIdOrName is being called with the default
checkedArchived = true, which causes archived rooms to throw error-room-archived
and prevents toggleFavoriteMethod from removing favorites; update the call that
currently uses findRoomByIdOrName({ params: this.bodyParams }) to pass
checkedArchived: false (e.g. findRoomByIdOrName({ params: this.bodyParams,
checkedArchived: false })) so the endpoint can unfavorite archived rooms before
calling toggleFavoriteMethod(this.userId, room._id, favorite).
- Around line 1107-1112: The catch block around FederationMatrix.handleInvite
currently constructs new Meteor.Error with the full interpolated message as the
first (code) argument; change it to pass a stable error code as the first arg
and the human-readable message as the second, preserving any existing structured
code/message from the caught error (e.g. use (error && error.code) ||
'federation-handle-invite-failed' for the first argument and (error &&
error.message) || String(error) for the second) when throwing new Meteor.Error
in the catch.
---
Nitpick comments:
In `@apps/meteor/app/api/server/v1/rooms.ts`:
- Around line 1094-1101: The AJV validators are compiled with ajv.compile<void>,
which loses the response type; change those generics to the actual response
shape (e.g., ajv.compile<{ success: boolean }>) so the returned ValidateFunction
has the correct typed `.data`; update both 200-response validators (the
ajv.compile calls shown and the other occurrence at the later 1121-1132 block)
to use the concrete type instead of void.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/rooms.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/rooms.ts
🧠 Learnings (10)
📚 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/rooms.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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/rooms.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.ts
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/rooms.ts (3)
packages/rest-typings/src/v1/rooms.ts (1)
isRoomsInviteProps(712-712)apps/meteor/app/channel-settings/server/methods/saveRoomSettings.ts (1)
favorite(348-350)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(74-78)
🔇 Additional comments (2)
apps/meteor/app/api/server/v1/rooms.ts (2)
928-970: LGTM — schema and type are well-formed.The
anyOf+additionalProperties: falsepattern correctly models theRoomsFavoriteunion: AJV evaluates each branch independently, so{ roomId, favorite }passes only the second branch and{ roomId, roomName, favorite }fails both. The compile-time generic matches the runtime schema.
1148-1152: LGTM — type consolidation is clean.
RoomEndpointsnow derives solely fromExtractRoutesFromAPI<typeof roomEndpoints>, correctly reflecting bothrooms.inviteandrooms.favoritebeing consolidated into the chained route definition.
ggazzo
reviewed
Feb 23, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/meteor/app/api/server/v1/rooms.ts (1)
1140-1140:⚠️ Potential issue | 🟡 Minor
checkedArchived: true(default) prevents unfavoriting archived rooms.Users who favorited a room before it was archived will be unable to unfavorite it, as
findRoomByIdOrNamethrowserror-room-archivedbefore reachingtoggleFavoriteMethod. PasscheckedArchived: falsesince archival status should not gate favorite toggling.Proposed fix
- const room = await findRoomByIdOrName({ params: this.bodyParams }); + const room = await findRoomByIdOrName({ params: this.bodyParams, checkedArchived: 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/rooms.ts` at line 1140, findRoomByIdOrName currently uses its default checkedArchived:true which causes it to throw error-room-archived and prevents toggleFavoriteMethod from running; update the call at the start of the favorite toggle flow to call findRoomByIdOrName({ params: this.bodyParams, checkedArchived: false }) so archived status does not block unfavoriting, preserving existing behavior elsewhere that relies on the default.
🧹 Nitpick comments (1)
apps/meteor/app/api/server/v1/rooms.ts (1)
1121-1132:ajv.compile<void>doesn't reflect the actual response shape.The schema describes
{ success: boolean }, but the generic parameter isvoid. Other endpoints in this file (e.g.,rooms.adminRooms.privateRoomsat Line 1040) pass accurate generics. SinceExtractRoutesFromAPIderives route types from these validators, the inferred 200 response type will bevoidinstead of{ success: true }.Suggested fix
- 200: ajv.compile<void>({ + 200: ajv.compile<{ success: true }>({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/rooms.ts` around lines 1121 - 1132, The 200-response Ajv validator currently uses ajv.compile<void> which causes the inferred route type to be void; update the generic to match the schema shape (e.g., ajv.compile<{ success: true }> or ajv.compile<{ success: boolean }> to match your intent) for the validator so ExtractRoutesFromAPI derives the correct response type; locate the validator call (ajv.compile) in this handler and replace the void generic with the accurate response interface, mirroring the pattern used by rooms.adminRooms.privateRooms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/app/api/server/v1/rooms.ts`:
- Line 1140: findRoomByIdOrName currently uses its default checkedArchived:true
which causes it to throw error-room-archived and prevents toggleFavoriteMethod
from running; update the call at the start of the favorite toggle flow to call
findRoomByIdOrName({ params: this.bodyParams, checkedArchived: false }) so
archived status does not block unfavoriting, preserving existing behavior
elsewhere that relies on the default.
---
Nitpick comments:
In `@apps/meteor/app/api/server/v1/rooms.ts`:
- Around line 1121-1132: The 200-response Ajv validator currently uses
ajv.compile<void> which causes the inferred route type to be void; update the
generic to match the schema shape (e.g., ajv.compile<{ success: true }> or
ajv.compile<{ success: boolean }> to match your intent) for the validator so
ExtractRoutesFromAPI derives the correct response type; locate the validator
call (ajv.compile) in this handler and replace the void generic with the
accurate response interface, mirroring the pattern used by
rooms.adminRooms.privateRooms.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/rooms.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). (3)
- GitHub Check: ⚙️ Variables Setup
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🧰 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/rooms.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
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-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/rooms.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/rooms.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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/rooms.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/rooms.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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/rooms.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/rooms.ts
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/rooms.ts (3)
packages/rest-typings/src/v1/rooms.ts (1)
isRoomsInviteProps(712-712)apps/meteor/app/channel-settings/server/methods/saveRoomSettings.ts (1)
favorite(348-350)apps/meteor/app/api/server/ApiClass.ts (1)
ExtractRoutesFromAPI(74-78)
🔇 Additional comments (3)
apps/meteor/app/api/server/v1/rooms.ts (3)
928-970: Well-structured discriminated union schema forrooms.favoritebody validation.The
RoomsFavoritetype andRoomsFavoriteSchemacleanly model the two mutually exclusive input shapes (roomIdvsroomName), andadditionalProperties: falseon each branch correctly prevents mixing them.
1137-1145: Clean action handler — dead-code guard correctly removed.The previous
hasOwnProperty('favorite')guard is gone, and AJV now handles validation before the action runs. The handler is concise and correct.
1148-1153: Type extraction and module augmentation look correct.
RoomEndpointsis cleanly derived fromroomEndpointsand properly extends theEndpointsinterface.
Member
|
/jira ARCH-1935 |
ggazzo
previously approved these changes
Feb 23, 2026
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
ggazzo
reviewed
Feb 23, 2026
ggazzo
previously approved these changes
Feb 23, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/meteor/app/api/server/v1/rooms.ts (1)
1144-1144:⚠️ Potential issue | 🟡 Minor
checkedArchived: true(default) prevents unfavoriting archived rooms.
findRoomByIdOrNamethrowserror-room-archivedbeforetoggleFavoriteMethodis reached, making it impossible to remove a favorite from an archived room. This issue was raised in a previous review and remains unaddressed.🐛 Proposed fix
- const room = await findRoomByIdOrName({ params: this.bodyParams }); + const room = await findRoomByIdOrName({ params: this.bodyParams, checkedArchived: 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/rooms.ts` at line 1144, The call to findRoomByIdOrName in the toggle favorite flow is using the default checkedArchived:true which causes an error-room-archived to be thrown before toggleFavoriteMethod can run; change the invocation in the toggle favorite endpoint so it passes checkedArchived:false (e.g., findRoomByIdOrName({ params: this.bodyParams, checkedArchived: false })) so archived rooms are returned and toggleFavoriteMethod can proceed to remove favorites from archived rooms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/app/api/server/v1/rooms.ts`:
- Line 1144: The call to findRoomByIdOrName in the toggle favorite flow is using
the default checkedArchived:true which causes an error-room-archived to be
thrown before toggleFavoriteMethod can run; change the invocation in the toggle
favorite endpoint so it passes checkedArchived:false (e.g., findRoomByIdOrName({
params: this.bodyParams, checkedArchived: false })) so archived rooms are
returned and toggleFavoriteMethod can proceed to remove favorites from archived
rooms.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/rooms.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). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🧰 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/rooms.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
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-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/rooms.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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/rooms.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/rooms.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:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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/rooms.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
apps/meteor/app/api/server/v1/rooms.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/rooms.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/rooms.ts
🔇 Additional comments (2)
apps/meteor/app/api/server/v1/rooms.ts (2)
928-970: LGTM — schema, type, and validator are consistent.The
anyOfwithadditionalProperties: falseon both branches correctly ensures mutual exclusivity ofroomIdandroomNamewhile requiringfavoriteon both, and the generic type parameter matches the union type.
1152-1152: LGTM — type derivation is correct.
RoomEndpointsnow cleanly derives from the consolidatedroomEndpointschain, which includes all migrated routes.
ggazzo
reviewed
Feb 23, 2026
ggazzo
changed the title
Contributor
Author
|
thanks , im going to update this repo RocketChat/Rocket.Chat-Open-API#150 and remove the rooms.favorite from it |
ahmed-n-abdeltwab
added a commit
to ahmed-n-abdeltwab/Rocket.Chat
that referenced
this pull request
Feb 24, 2026
Co-authored-by: Guilherme Gazzo <guilhermegazzo@gmail.com> Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
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
4 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:
Verified that the API response schemas are correctly documented in Swagger UI.
All tests passed without any breaking changes
Endpoints:
Favorite/Unfavourite a Room
Looking forward to your feedback! 🚀
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Task: ARCH-1985