refactor: Migrate livechat/users endpoints to OpenAPI with AJV validation#39584
Open
amitb0ra wants to merge 5 commits intoRocketChat:developfrom
Open
refactor: Migrate livechat/users endpoints to OpenAPI with AJV validation#39584amitb0ra wants to merge 5 commits intoRocketChat:developfrom
amitb0ra wants to merge 5 commits intoRocketChat:developfrom
Conversation
🦋 Changeset detectedLatest commit: 0c5e53f 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:
WalkthroughReplaced legacy addRoute for livechat/users with OpenAPI-style per-route Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API.v1 (route layer)
participant Service as LivechatUsersManager
participant DB as Database
Client->>API: HTTP GET/POST/DELETE /v1/livechat/users/{manager|user} ...
API->>API: AJV validate request (query/body)
API->>Service: authorize & invoke handler
Service->>DB: query/update user records
DB-->>Service: result
Service-->>API: prepare response
API->>API: AJV validate response
API-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/meteor/app/livechat/imports/server/rest/users.ts (2)
69-70: Remove inline comment explaining schema fallback.This explanatory comment violates coding guidelines. The reason for using
{ type: 'object' }can be documented in a PR description or ADR if needed, but should not remain in the code.Suggested fix
const paginatedUsersResponseSchema = ajv.compile<{ users: object[]; count: number; offset: number; total: number; }>({ type: 'object', properties: { - // ILivechatAgent is not registered in typia (extends IUser with livechat-specific fields), - // so we use { type: 'object' } as a fallback. users: { type: 'array', items: { type: 'object' } }, count: { type: 'number' },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/livechat/imports/server/rest/users.ts` around lines 69 - 70, Remove the inline explanatory comment about the ILivechatAgent schema fallback: find the comment referencing "ILivechatAgent is not registered in typia (extends IUser with livechat-specific fields), so we use { type: 'object' } as a fallback" and delete it so only the code uses the fallback schema ({ type: 'object' }) without the explanatory note; if documentation is needed, move the explanation to the PR description or an ADR instead.
11-11: Remove section divider comments.Per coding guidelines, code comments should be avoided in the implementation. The section divider comments on lines 11, 59, and 258 (
// --- Local types...,// --- Response schemas ---,// --- Type augmentation ---) should be removed.Also applies to: 59-59, 258-258
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/livechat/imports/server/rest/users.ts` at line 11, Remove the three section divider comments that violate the coding guideline: delete the lines containing "// --- Local types and AJV schemas (moved from rest-typings) ---", "// --- Response schemas ---", and "// --- Type augmentation ---" in the users.ts file; search for these exact comment strings (or similar divider comments) near the top of the module and remove them so only code and necessary inline comments remain, ensuring no other identical divider comments are left in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rest-typings/src/v1/omnichannel.ts`:
- Line 4664: The POST signature for the /livechat/users/agent route is
incorrectly using PaginatedRequest<{ username: string }>, causing a mismatch
with the shared handler which expects a plain body; update the POST type for the
route currently declared as "POST: (params: PaginatedRequest<{ username: string
}>) => { success: boolean }" to "POST: (params: { username: string }) => {
success: boolean }" so it matches the other routes (/livechat/users/manager and
/livechat/users/user) and the POST livechat/users/:type handler.
---
Nitpick comments:
In `@apps/meteor/app/livechat/imports/server/rest/users.ts`:
- Around line 69-70: Remove the inline explanatory comment about the
ILivechatAgent schema fallback: find the comment referencing "ILivechatAgent is
not registered in typia (extends IUser with livechat-specific fields), so we use
{ type: 'object' } as a fallback" and delete it so only the code uses the
fallback schema ({ type: 'object' }) without the explanatory note; if
documentation is needed, move the explanation to the PR description or an ADR
instead.
- Line 11: Remove the three section divider comments that violate the coding
guideline: delete the lines containing "// --- Local types and AJV schemas
(moved from rest-typings) ---", "// --- Response schemas ---", and "// --- Type
augmentation ---" in the users.ts file; search for these exact comment strings
(or similar divider comments) near the top of the module and remove them so only
code and necessary inline comments remain, ensuring no other identical divider
comments are left in the file.
🪄 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: e9949ec8-d2c8-4776-a74b-6619bd1d6195
📒 Files selected for processing (3)
.changeset/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/rest-typings/src/v1/omnichannel.tsapps/meteor/app/livechat/imports/server/rest/users.ts
🧠 Learnings (11)
📓 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: 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: 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: 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.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
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: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:49.806Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON which automatically deserializes Date objects. Stream handlers (e.g., sdk.stream()) receive Date fields as Date objects directly and do not require manual conversion using `new Date()`. Only REST API responses require manual date conversion because they return plain JSON where dates are serialized as strings.
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.
📚 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/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.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:
.changeset/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.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/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.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/migrate-livechat-users-openapi.md
📚 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:
.changeset/migrate-livechat-users-openapi.mdpackages/rest-typings/src/v1/omnichannel.tsapps/meteor/app/livechat/imports/server/rest/users.ts
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
packages/rest-typings/src/v1/omnichannel.tsapps/meteor/app/livechat/imports/server/rest/users.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/rest-typings/src/v1/omnichannel.tsapps/meteor/app/livechat/imports/server/rest/users.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/rest-typings/src/v1/omnichannel.tsapps/meteor/app/livechat/imports/server/rest/users.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/livechat/imports/server/rest/users.ts
📚 Learning: 2026-03-11T16:46:55.955Z
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:55.955Z
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/livechat/imports/server/rest/users.ts
🔇 Additional comments (3)
apps/meteor/app/livechat/imports/server/rest/users.ts (2)
258-265: LGTM!The type augmentation pattern using
ExtractRoutesFromAPIand module augmentation to extend@rocket.chat/rest-typingsEndpoints interface follows the established convention for OpenAPI migrations.
241-255: This concern is unfounded. The POST handler at lines 184-198 also callsaddAgentandaddManager, which throwError('error-invalid-user')when users are not found, yet it has no try-catch wrapper. Both handlers directly thrownew Error('Invalid type')without catching. This pattern indicates the framework's error handling (via Hono) manages exceptions from route handlers appropriately. No changes needed.> Likely an incorrect or invalid review comment..changeset/migrate-livechat-users-openapi.md (1)
1-6: LGTM!The changeset correctly documents the minor version bumps for both packages and accurately describes the OpenAPI migration with AJV validation for the livechat/users endpoints.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
4633-4639: Return type should includedepartmentsfield to match actual API response.The backend calls
findManagers()which usesUsers.findAgentsWithDepartments(). According to the model definition inpackages/models/src/models/Users.ts, this method returns documents with an addeddepartments: string[]field via aggregation lookup.The current return type
ILivechatAgent[]doesn't reflect this, which may cause type mismatches for consumers expecting the full response shape.Suggested fix
'/v1/livechat/users/manager': { GET: ( params: PaginatedRequest<{ text?: string; fields?: string; onlyAvailable?: boolean; excludeId?: string; showIdleAgents?: boolean }>, ) => PaginatedResult<{ - users: ILivechatAgent[]; + users: (ILivechatAgent & { departments: string[] })[]; }>; POST: (params: { username: string }) => { success: boolean }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rest-typings/src/v1/omnichannel.ts` around lines 4633 - 4639, The GET response type for '/v1/livechat/users/manager' is missing the aggregated departments field; update the PaginatedResult return type so each user includes departments: string[] (e.g., change PaginatedResult<{ users: ILivechatAgent[] }> to PaginatedResult<{ users: (ILivechatAgent & { departments: string[] })[] }>), matching the data produced by findManagers()/Users.findAgentsWithDepartments().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rest-typings/src/v1/omnichannel.ts`:
- Around line 4642-4649: The typings declare endpoints '/v1/livechat/users/user'
and '/v1/livechat/users/user/:_id' that the backend does not support (handler in
apps/meteor/app/livechat/imports/server/rest/users.ts only accepts type ===
'agent' or 'manager' and throws "Invalid type" at the check around line 167);
either remove these two endpoint entries from
packages/rest-typings/src/v1/omnichannel.ts or implement corresponding backend
handling for type === 'user' in the server handler (add acceptance, validation
and response logic matching the typings), and ensure the REST handler and the
PaginatedResult/POST shapes (as declared) stay consistent between the typing
symbols and the server functions.
- Line 4665: The POST signature for the /v1/livechat/users/agent endpoint
references an undefined type POSTLivechatUsersTypeProps which will break
TypeScript; replace that reference with the same parameter type used by the
other livechat user POST endpoints (i.e. use { username: string }) or
alternatively declare POSTLivechatUsersTypeProps as an alias for { username:
string } so the POST: (params: POSTLivechatUsersTypeProps) => { success: boolean
} becomes consistent with the manager/user endpoints.
---
Nitpick comments:
In `@packages/rest-typings/src/v1/omnichannel.ts`:
- Around line 4633-4639: The GET response type for '/v1/livechat/users/manager'
is missing the aggregated departments field; update the PaginatedResult return
type so each user includes departments: string[] (e.g., change PaginatedResult<{
users: ILivechatAgent[] }> to PaginatedResult<{ users: (ILivechatAgent & {
departments: string[] })[] }>), matching the data produced by
findManagers()/Users.findAgentsWithDepartments().
🪄 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: 4e1cd62f-97ec-4de5-9c68-4c90836547ca
📒 Files selected for processing (1)
packages/rest-typings/src/v1/omnichannel.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/rest-typings/src/v1/omnichannel.ts
🧠 Learnings (8)
📓 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: 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: 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: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
packages/rest-typings/src/v1/omnichannel.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:
packages/rest-typings/src/v1/omnichannel.ts
📚 Learning: 2026-03-11T16:46:55.955Z
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:55.955Z
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:
packages/rest-typings/src/v1/omnichannel.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:
packages/rest-typings/src/v1/omnichannel.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/rest-typings/src/v1/omnichannel.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/rest-typings/src/v1/omnichannel.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/rest-typings/src/v1/omnichannel.ts
amitb0ra
force-pushed
the
refactor/migrate-livechat-users-openapi
branch
from
5603116 to
dc8f9f2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/meteor/app/livechat/imports/server/rest/users.ts (2)
61-89: Tighten theuserpayload schemas before publishing these OpenAPI responses.
userandusers[*]are validated as plainobject, so AJV will not catch contract drift and Swagger will not show the fields these endpoints actually return. The POST path already returns a stable{ _id, username }shape, and the GET-by-id path projects a fixed field set, so these schemas should be explicit (or share a small reusable schema) instead of staying opaque.Also applies to: 100-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/livechat/imports/server/rest/users.ts` around lines 61 - 89, The response schemas are too loose: replace the anonymous { type: 'object' } for user and users[*] with an explicit schema describing the actual fields returned (at minimum the POST response shape {_id, username} and the GET-by-id projected fields) and reuse it across paginatedUsersResponseSchema and postUserResponseSchema (e.g. create a single userResponseSchema and reference it in users.items and in user). Ensure success remains required and additionalProperties is set appropriately; update the same pattern used around the other schemas referenced in the diff (paginatedUsersResponseSchema, postUserResponseSchema and the nearby schemas mentioned in the comment) so the OpenAPI output and AJV validation reflect the real fields.
11-12: Please drop the added implementation comments from this migration diff.The section headers, fallback note, and TODO add noise to a file that is already readable through names and schemas. If you want to keep this PR narrowly scoped, this is fine as a quick follow-up.
As per coding guidelines, "Avoid code comments in the implementation." Based on learnings, maintainers prefer to defer style-only cleanups in OpenAPI migration PRs.
Also applies to: 59-60, 69-70, 224-224, 258-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/livechat/imports/server/rest/users.ts` around lines 11 - 12, Remove the added implementation comments and section headers introduced in this migration (e.g. the "// --- Local types and AJV schemas (moved from rest-typings) ---" header, the fallback note comment, and the "TODO" implementation notes) so the file contains only code and schemas; search for those exact comment strings in users.ts and delete them (and any other similar migration-only commentary) to comply with the "Avoid code comments in the implementation" guideline.
🤖 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/livechat/imports/server/rest/users.ts`:
- Around line 260-264: The generated type LivechatUsersEndpoints =
ExtractRoutesFromAPI<typeof livechatUsersEndpoints> is exposing literal route
keys like 'livechat/users/:type' and 'livechat/users/:type/:_id', which don't
match the concrete endpoint aliases in the public Endpoints interface; update
the augmentation so the Endpoints interface in module
'@rocket.chat/rest-typings' is extended with the concrete alias keys used in
packages/rest-typings/src/v1/omnichannel.ts (or stop exporting
LivechatUsersEndpoints until you map the literal paths to those aliases).
Concretely, replace or wrap the current augmentation of Endpoints with a mapped
type or explicit aliasing that translates the ExtractRoutesFromAPI keys from
livechatUsersEndpoints into the actual public endpoint names so the exported
Endpoints matches the manual omnichannel aliases.
---
Nitpick comments:
In `@apps/meteor/app/livechat/imports/server/rest/users.ts`:
- Around line 61-89: The response schemas are too loose: replace the anonymous {
type: 'object' } for user and users[*] with an explicit schema describing the
actual fields returned (at minimum the POST response shape {_id, username} and
the GET-by-id projected fields) and reuse it across paginatedUsersResponseSchema
and postUserResponseSchema (e.g. create a single userResponseSchema and
reference it in users.items and in user). Ensure success remains required and
additionalProperties is set appropriately; update the same pattern used around
the other schemas referenced in the diff (paginatedUsersResponseSchema,
postUserResponseSchema and the nearby schemas mentioned in the comment) so the
OpenAPI output and AJV validation reflect the real fields.
- Around line 11-12: Remove the added implementation comments and section
headers introduced in this migration (e.g. the "// --- Local types and AJV
schemas (moved from rest-typings) ---" header, the fallback note comment, and
the "TODO" implementation notes) so the file contains only code and schemas;
search for those exact comment strings in users.ts and delete them (and any
other similar migration-only commentary) to comply with the "Avoid code comments
in the implementation" guideline.
🪄 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: 261c76f2-e53d-4d5e-a1d1-9adc16605912
📒 Files selected for processing (3)
.changeset/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/migrate-livechat-users-openapi.md
- packages/rest-typings/src/v1/omnichannel.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/livechat/imports/server/rest/users.ts
🧠 Learnings (8)
📓 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: 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: 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: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/app/livechat/imports/server/rest/users.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/livechat/imports/server/rest/users.ts
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/app/livechat/imports/server/rest/users.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/livechat/imports/server/rest/users.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/livechat/imports/server/rest/users.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/livechat/imports/server/rest/users.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/livechat/imports/server/rest/users.ts
amitb0ra
force-pushed
the
refactor/migrate-livechat-users-openapi
branch
from
dc8f9f2 to
5dc4b44
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
4642-4648:⚠️ Potential issue | 🟠 MajorRemove the unsupported
/livechat/users/useralias.
apps/meteor/app/livechat/imports/server/rest/users.tsonly acceptstype === 'agent'ortype === 'manager'for these migrated handlers. Keeping/v1/livechat/users/userin the public typings advertises an endpoint that will always fail withInvalid type, and the sibling'/v1/livechat/users/user/:_id'just below has the same problem.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rest-typings/src/v1/omnichannel.ts` around lines 4642 - 4648, The public typings advertise an unsupported alias: remove the '/v1/livechat/users/user' route entry (and its sibling '/v1/livechat/users/user/:_id') from packages/rest-typings/src/v1/omnichannel.ts so the declared REST surface matches the actual handlers that only accept type === 'agent' or 'manager'; locate and delete the objects keyed by '/v1/livechat/users/user' and '/v1/livechat/users/user/:_id' in the file (keeping the correct '/v1/livechat/users' routes) so the typings no longer advertise endpoints that will always return "Invalid type".
🧹 Nitpick comments (1)
apps/meteor/app/livechat/imports/server/rest/users.ts (1)
10-11: Drop the new section-header comments.These headers are just organizing comments in implementation code. Please remove them and let the symbols/grouping carry the structure instead.
As per coding guidelines "Avoid code comments in the implementation".
Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/livechat/imports/server/rest/users.ts` around lines 10 - 11, Remove the section-header comment lines that act as organizational headers (e.g. the "// --- Local types and AJV schemas (moved from rest-typings) ---" line and the similar header at the other location around line 58) in apps/meteor/app/livechat/imports/server/rest/users.ts; leave the actual declarations (types, AJV schema objects, functions like the local type definitions or schema constants) intact so the symbols and groupings convey structure rather than using comment headers.
🤖 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/livechat/imports/server/rest/users.ts`:
- Around line 99-107: The GET /v1/livechat/users/agent/:_id handler (validated
by getUserByIdResponseSchema) can return { user: null } on missing/unauthorized
IDs but the REST typing for the route in
packages/rest-typings/src/v1/omnichannel.ts still marks user as non-null; update
the concrete route response type for '/v1/livechat/users/agent/:_id' to make the
user field nullable (object | null) to match getUserByIdResponseSchema (and
apply the same nullable change to any other routes in that file flagged by the
reviewer in the 200-225 range), or alternatively change the handler to return a
non-200 failure (e.g., 404/403) instead—pick one approach and make both the AJV
schema and the route typing consistent (references: getUserByIdResponseSchema
and the '/v1/livechat/users/agent/:_id' entry in omnihchannel.ts).
---
Duplicate comments:
In `@packages/rest-typings/src/v1/omnichannel.ts`:
- Around line 4642-4648: The public typings advertise an unsupported alias:
remove the '/v1/livechat/users/user' route entry (and its sibling
'/v1/livechat/users/user/:_id') from packages/rest-typings/src/v1/omnichannel.ts
so the declared REST surface matches the actual handlers that only accept type
=== 'agent' or 'manager'; locate and delete the objects keyed by
'/v1/livechat/users/user' and '/v1/livechat/users/user/:_id' in the file
(keeping the correct '/v1/livechat/users' routes) so the typings no longer
advertise endpoints that will always return "Invalid type".
---
Nitpick comments:
In `@apps/meteor/app/livechat/imports/server/rest/users.ts`:
- Around line 10-11: Remove the section-header comment lines that act as
organizational headers (e.g. the "// --- Local types and AJV schemas (moved from
rest-typings) ---" line and the similar header at the other location around line
58) in apps/meteor/app/livechat/imports/server/rest/users.ts; leave the actual
declarations (types, AJV schema objects, functions like the local type
definitions or schema constants) intact so the symbols and groupings convey
structure rather than using comment headers.
🪄 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: 6d3e752e-b91b-4855-9e0d-a45f5a03aa05
📒 Files selected for processing (3)
.changeset/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/migrate-livechat-users-openapi.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
🧠 Learnings (15)
📓 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: 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: 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: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
📚 Learning: 2026-03-11T16:46:55.955Z
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:55.955Z
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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/rest-typings/src/v1/omnichannel.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:
packages/rest-typings/src/v1/omnichannel.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:
packages/rest-typings/src/v1/omnichannel.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:
packages/rest-typings/src/v1/omnichannel.ts
amitb0ra
force-pushed
the
refactor/migrate-livechat-users-openapi
branch
from
5dc4b44 to
2324f5f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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/livechat/imports/server/rest/users.ts (1)
240-254:⚠️ Potential issue | 🟡 MinorUnhandled exceptions from
removeAgent/removeManagerwill propagate as 500 errors.Both functions throw
Error('error-invalid-user')when the user is not found. Additionally, they callremoveUserFromRolesAsync, which throwsMeteorError('error-invalid-user')if validation fails. These exceptions are not caught and will bypass the fallbackAPI.v1.failure('error-removing-user')response, resulting in unhandled 500 errors instead of the declared 400 error response.Wrap the calls in try-catch to handle these errors gracefully:
Example
try { if (await removeAgent(this.urlParams._id)) { return API.v1.success(); } } catch (error) { return API.v1.failure('error-removing-user'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/livechat/imports/server/rest/users.ts` around lines 240 - 254, The action handler calls removeAgent/removeManager (which can throw Error('error-invalid-user') and call removeUserFromRolesAsync that may throw MeteorError) without catching exceptions, causing 500s; wrap the await removeAgent(this.urlParams._id) and await removeManager(this.urlParams._id) calls in try-catch blocks inside the action function and on any caught error return API.v1.failure('error-removing-user') so validation errors are converted to the intended failure response rather than propagating.
♻️ Duplicate comments (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
4642-4649:⚠️ Potential issue | 🟠 Major
/v1/livechat/users/userendpoints have no backend handler.All operations on
/v1/livechat/users/userand/v1/livechat/users/user/:_idfail at runtime. The backend implementation only supportstype === 'agent'ortype === 'manager'and throwsError('Invalid type')for requests withtype === 'user', affecting GET list, POST, GET by ID, and DELETE operations.Remove these endpoint definitions from
packages/rest-typings/src/v1/omnichannel.tsor add backend support for theusertype inapps/meteor/app/livechat/imports/server/rest/users.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rest-typings/src/v1/omnichannel.ts` around lines 4642 - 4649, The REST typings declare endpoints for '/v1/livechat/users/user' and '/v1/livechat/users/user/:_id' but the backend (apps/meteor/app/livechat/imports/server/rest/users.ts) only supports type === 'agent' or 'manager' and throws for 'user'; either remove those endpoint entries from packages/rest-typings/src/v1/omnichannel.ts, or implement full backend support for the 'user' type in the users REST handlers (update the request router/handler in apps/meteor/app/livechat/imports/server/rest/users.ts to accept type === 'user' for GET list, POST, GET by ID and DELETE, ensure responses match ILivechatAgent/expected shapes and update any validation/permission checks), then keep the typings in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rest-typings/src/v1/omnichannel.ts`:
- Line 4665: Update the POST response type for the livechat agent endpoint to
match the server shape (returning an object with user) by replacing the current
signature POST: (params: { username: string }) => { success: boolean } with a
signature that returns the user object (e.g., POST: (params: { username: string
}) => { user: User } or the appropriate user type used elsewhere); locate the
signature string "POST: (params: { username: string }) => { success: boolean }"
in the omnichannel typings and align it with the manager endpoint's corrected
return type.
- Around line 4639-4648: The POST return type for the endpoints
'/v1/livechat/users/manager' and '/v1/livechat/users/user' is incorrect: update
their POST signatures in the rest typings so they return the user object the
server actually returns (i.e. change POST: (params: { username: string }) => {
success: boolean } to POST: (params: { username: string }) => { user:
ILivechatAgent }), matching the server's postUserResponseSchema and the
ILivechatAgent type used elsewhere.
---
Outside diff comments:
In `@apps/meteor/app/livechat/imports/server/rest/users.ts`:
- Around line 240-254: The action handler calls removeAgent/removeManager (which
can throw Error('error-invalid-user') and call removeUserFromRolesAsync that may
throw MeteorError) without catching exceptions, causing 500s; wrap the await
removeAgent(this.urlParams._id) and await removeManager(this.urlParams._id)
calls in try-catch blocks inside the action function and on any caught error
return API.v1.failure('error-removing-user') so validation errors are converted
to the intended failure response rather than propagating.
---
Duplicate comments:
In `@packages/rest-typings/src/v1/omnichannel.ts`:
- Around line 4642-4649: The REST typings declare endpoints for
'/v1/livechat/users/user' and '/v1/livechat/users/user/:_id' but the backend
(apps/meteor/app/livechat/imports/server/rest/users.ts) only supports type ===
'agent' or 'manager' and throws for 'user'; either remove those endpoint entries
from packages/rest-typings/src/v1/omnichannel.ts, or implement full backend
support for the 'user' type in the users REST handlers (update the request
router/handler in apps/meteor/app/livechat/imports/server/rest/users.ts to
accept type === 'user' for GET list, POST, GET by ID and DELETE, ensure
responses match ILivechatAgent/expected shapes and update any
validation/permission checks), then keep the typings in sync.
🪄 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: 4d0d51bb-f461-4fc6-8cf9-bbee1e5e1509
📒 Files selected for processing (3)
.changeset/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
🧠 Learnings (17)
📓 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: 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: 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: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
📚 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/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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:
.changeset/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/migrate-livechat-users-openapi.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: 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/migrate-livechat-users-openapi.md
📚 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:
.changeset/migrate-livechat-users-openapi.mdapps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/app/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.ts
📚 Learning: 2026-03-11T16:46:55.955Z
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:55.955Z
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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.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/livechat/imports/server/rest/users.tspackages/rest-typings/src/v1/omnichannel.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/rest-typings/src/v1/omnichannel.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:
packages/rest-typings/src/v1/omnichannel.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:
packages/rest-typings/src/v1/omnichannel.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:
packages/rest-typings/src/v1/omnichannel.ts
🔇 Additional comments (9)
apps/meteor/app/livechat/imports/server/rest/users.ts (8)
10-11: LGTM!The section header comments are acceptable for code organization and improve readability of the local types/schemas structure.
24-41: LGTM!The
LivechatUsersManagerGETLocalSchemacorrectly defines all query parameters with appropriate types and nullable settings. The schema matches the TypeScript type definition.
47-56: LGTM!The POST body schema correctly requires
usernameas a non-nullable string withadditionalProperties: falsefor strict validation.
60-78: LGTM!The paginated response schema appropriately uses
{ type: 'object' }forILivechatAgentitems since the type isn't registered in Typia. The comment explaining this is helpful technical documentation.
90-97: LGTM!The
successOnlyResponseSchemafollows the established convention of usingajv.compile<void>({...})for success-only responses. Based on learnings, this pattern is intentionally used across migrated endpoints.
111-167: LGTM!The GET handler correctly:
- Uses
permissionsRequired: emptyStringArraysince permissions are checked inline per type- Applies appropriate permission checks for
agentvsmanagertypes- Returns proper forbidden response with
'error-not-authorized'- Defaults
onlyAvailabletofalsewhich matches thefindAgentsfunction signature
169-198: LGTM!The POST handler correctly validates the body using
isPOSTLivechatUsersTypeLocaland returns{ user }on success. Theview-livechat-managerpermission is appropriately required for both agent and manager creation.
200-225: LGTM!The GET by ID handler correctly:
- Validates type is either 'agent' or 'manager'
- Maps type to appropriate role for database query
- Uses proper projection to limit returned fields
- Returns nullable user (addresses past review comment about response typing)
The TODO comment at line 223 is appropriate for flagging future improvement.
.changeset/migrate-livechat-users-openapi.md (1)
1-6: LGTM!The changeset correctly declares minor version bumps for both affected packages. The description accurately summarizes the migration to chained route definitions with AJV validation.
amitb0ra
force-pushed
the
refactor/migrate-livechat-users-openapi
branch
from
2324f5f to
16bfe5c
Compare
ahmed-n-abdeltwab
suggested changes
Mar 14, 2026
Contributor
ahmed-n-abdeltwab
left a comment
ahmed-n-abdeltwab
left a comment
There was a problem hiding this comment.
Please update the rest-typings to focus only on the /v1/livechat/users/:type and /v1/livechat/users/:type/:id routes
![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 14 files (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/client/views/omnichannel/agents/AgentInfo.tsx">
<violation number="1" location="apps/meteor/client/views/omnichannel/agents/AgentInfo.tsx:29">
P1: Migrating to `/livechat/users/:type/:_id` introduces a null-return path (`user: null`) that this component does not guard, causing runtime crashes on successful responses for missing/non-agent users.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…tion - Migrate livechat/users/:type and livechat/users/:type/:_id from addRoute() to chained .get()/.post()/.delete() pattern - Add local AJV schemas for query, body, and response validation - Add ExtractRoutesFromAPI type augmentation for auto-generated Endpoints types - Remove manual type definitions and schemas from rest-typings - Update all client consumers to use parameterized route paths
amitb0ra
force-pushed
the
refactor/migrate-livechat-users-openapi
branch
from
ef08e49 to
f1ed33c
Compare
- Restore original type/variable names (remove Local suffix) per migration rules
- Use $ref: '#/components/schemas/IUser' instead of bare { type: 'object' }
- Revert logic changes: remove forbidden() args, onlyAvailable default, failure() args
- Keep concrete path aliases (/v1/livechat/users/agent, /manager, etc.) in rest-typings
- Revert all client-side file changes (concrete aliases preserved for useEndpoint compatibility)
- Only remove parameterized /v1/livechat/users/:type entries from OmnichannelEndpoints
Contributor
There was a problem hiding this comment.
1 issue found across 15 files (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="packages/rest-typings/src/v1/omnichannel.ts">
<violation number="1" location="packages/rest-typings/src/v1/omnichannel.ts:4655">
P2: `/v1/livechat/users/agent/:_id` GET is typed as non-null, but server route can return `user: null` for missing IDs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Remove /v1/livechat/users/agent, /v1/livechat/users/manager, /v1/livechat/users/agent/:_id, /v1/livechat/users/manager/:_id concrete aliases from omnichannel.ts - Types now come solely from ExtractRoutesFromAPI augmentation via /v1/livechat/users/:type and /v1/livechat/users/:type/:_id - Update all client-side useEndpoint/useEndpointMutation calls to use parameterized paths with type keys - Update storybook stories and test mocks accordingly
Contributor
There was a problem hiding this comment.
2 issues found across 14 files (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/client/views/omnichannel/departments/DepartmentAgentsTable/AddAgent.tsx">
<violation number="1" location="apps/meteor/client/views/omnichannel/departments/DepartmentAgentsTable/AddAgent.tsx:22">
P2: Endpoint migration allows `user: null`, but AddAgent still destructures `user` as non-null, causing a runtime crash when agent lookup returns null.</violation>
</file>
<file name="apps/meteor/client/views/omnichannel/agents/AgentInfo.tsx">
<violation number="1" location="apps/meteor/client/views/omnichannel/agents/AgentInfo.tsx:29">
P1: Switched endpoint now allows successful `user: null`, but component still dereferences `data.user` and can crash on not-found responses.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/meteor/client/views/omnichannel/departments/DepartmentAgentsTable/AddAgent.tsx
Show resolved
Hide resolved
- AgentInfo.tsx: handle user: null by showing 'User_not_found' - DepartmentAgentsTable/AddAgent.tsx: guard against null user - useAgentsList.ts: relax formatAgentItem type for object response
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.
Proposed changes
Migrates the
livechat/users/:typeandlivechat/users/:type/:_idendpoints from the legacyaddRoute()pattern to the new OpenAPI-compatible chained route pattern with inline AJV validation schemas.Resolves: RocketChat/Rocket.Chat-Open-API#150
Changes
apps/meteor/app/livechat/imports/server/rest/users.tsAPI.v1.addRoute()calls with the new chainedAPI.v1.get().post().get().delete()patternExtractRoutesFromAPI+declare moduleaugmentation for type extractionGET /livechat/users/:type,POST /livechat/users/:type,GET /livechat/users/:type/:_id,DELETE /livechat/users/:type/:_idpackages/rest-typings/src/v1/omnichannel.ts/livechat/users/${string},/livechat/users/${string}/${string}) that were previously in the sharedEndpointsinterface/livechat/users/agent) for client-sideuseEndpoint()compatibility.changeset/migrate-livechat-users-openapi.mdminorbump for@rocket.chat/meteorand@rocket.chat/rest-typingsTest results
Summary by CodeRabbit
New Features
Refactor