chore: migrate users.setStatus endpoint to new OpenAPI pattern with AJV validation#39485
Open
Yashika-code wants to merge 8 commits intoRocketChat:developfrom
Open
chore: migrate users.setStatus endpoint to new OpenAPI pattern with AJV validation#39485Yashika-code wants to merge 8 commits intoRocketChat:developfrom
Yashika-code wants to merge 8 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: f45b12e 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (15)📓 Common learnings📚 Learning: 2026-02-24T19:09:01.522ZApplied to files:
📚 Learning: 2026-03-09T18:39:14.020ZApplied to files:
📚 Learning: 2026-01-26T18:26:01.279ZApplied to files:
📚 Learning: 2026-01-17T01:51:47.764ZApplied to files:
📚 Learning: 2026-03-03T11:11:48.541ZApplied to files:
📚 Learning: 2026-02-23T17:53:06.802ZApplied to files:
📚 Learning: 2026-03-09T21:20:07.542ZApplied to files:
📚 Learning: 2026-02-24T19:05:56.710ZApplied to files:
📚 Learning: 2026-02-24T19:36:55.089ZApplied to files:
📚 Learning: 2026-02-24T19:16:35.307ZApplied to files:
📚 Learning: 2025-11-19T18:20:37.116ZApplied to files:
📚 Learning: 2025-11-19T18:20:07.720ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
🔇 Additional comments (2)
WalkthroughThis PR migrates the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/migrate-users-setStatus-openapi.md:
- Around line 2-3: The removal of the manual /v1/users.setStatus typing breaks
patch-level safety because the replacement augmentation lives in apps/meteor and
isn't visible when compiling `@rocket.chat/rest-typings` in isolation; restore the
explicit typing for the /v1/users.setStatus operation in
packages/rest-typings/src/v1/users.ts (or add an equivalent declaration file
inside the `@rocket.chat/rest-typings` package) so consumers compiling that
package alone still get the OperationResult types; reference the endpoint name
"/v1/users.setStatus" and the file packages/rest-typings/src/v1/users.ts in the
fix and add a brief comment noting the apps/meteor augmentation so future
maintainers understand why the manual typing must remain.
In `@apps/meteor/app/api/server/v1/users.ts`:
- Around line 891-911: The schema currently allows empty strings for
status/message because anyOf only checks key presence; update the ajv.compile
body schema (the object passed to ajv.compile in users.ts) so status and
message, when present, must be non-empty strings (e.g., add "minLength: 1" or a
non-empty pattern to the status and message property definitions) and adjust the
anyOf logic to still require one of those properties; then update the handler
logic that inspects body.status (the branch currently skipping on falsy values
between the status handling code and the broadcast code) to test for the
property presence instead of truthiness (use an explicit check like
Object.prototype.hasOwnProperty.call(body, 'status') or body.status !==
undefined) so empty strings are rejected by validation and the handler correctly
distinguishes omitted vs provided values.
- Around line 887-924: The response map for this rate-limited endpoint is
missing the 429 schema so the OpenAPI output won't show the runtime "Too Many
Requests" response; update the response object in the route definition (the same
block that contains rateLimiterOptions, body, and response) to include a 429
entry pointing to the shared 429 validator (e.g. add "429:
validateTooManyRequestsErrorResponse") or the equivalent schema used elsewhere
in the codebase so the automatic 429 handling from addRoute()/API.v1.post() is
declared in the contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8f77e97-c5d5-43fc-bb27-42291e049c0e
📒 Files selected for processing (3)
.changeset/migrate-users-setStatus-openapi.mdapps/meteor/app/api/server/v1/users.tspackages/rest-typings/src/v1/users.ts
💤 Files with no reviewable changes (1)
- packages/rest-typings/src/v1/users.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/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: 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.
📚 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-users-setStatus-openapi.md
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
.changeset/migrate-users-setStatus-openapi.md
📚 Learning: 2026-03-09T18:39:14.020Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:14.020Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/app/api/server/v1/users.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/users.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/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/api/server/v1/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/api/server/v1/users.ts
Comment on lines
+887
to
+924
| rateLimiterOptions: { | ||
| numRequestsAllowed: 5, | ||
| intervalTimeInMS: 60000, | ||
| }, | ||
| body: ajv.compile<{ | ||
| status?: string; | ||
| message?: string; | ||
| userId?: string; | ||
| username?: string; | ||
| user?: string; | ||
| }>({ | ||
| type: 'object', | ||
| properties: { | ||
| status: { type: 'string' }, | ||
| message: { type: 'string' }, | ||
| userId: { type: 'string' }, | ||
| username: { type: 'string' }, | ||
| user: { type: 'string' }, | ||
| }, | ||
| anyOf: [ | ||
| { required: ['message'] }, | ||
| { required: ['status'] }, | ||
| ], | ||
| additionalProperties: false, | ||
| }), | ||
| response: { | ||
| 400: validateBadRequestErrorResponse, | ||
| 401: validateUnauthorizedErrorResponse, | ||
| 403: validateForbiddenErrorResponse, | ||
| 200: ajv.compile<{ success: boolean }>({ | ||
| type: 'object', | ||
| properties: { | ||
| success: { type: 'boolean', enum: [true] }, | ||
| }, | ||
| required: ['success'], | ||
| additionalProperties: false, | ||
| }), | ||
| }, |
Contributor
There was a problem hiding this comment.
Add the 429 response schema to this contract.
This route is rate-limited, and API.v1.post() inherits the automatic 429 handling from addRoute(), but the response map only declares 200/400/401/403. The generated OpenAPI spec will miss a real runtime response.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/users.ts` around lines 887 - 924, The response
map for this rate-limited endpoint is missing the 429 schema so the OpenAPI
output won't show the runtime "Too Many Requests" response; update the response
object in the route definition (the same block that contains rateLimiterOptions,
body, and response) to include a 429 entry pointing to the shared 429 validator
(e.g. add "429: validateTooManyRequestsErrorResponse") or the equivalent schema
used elsewhere in the codebase so the automatic 429 handling from
addRoute()/API.v1.post() is declared in the contract.
Contributor
Author
There was a problem hiding this comment.
validateTooManyRequestsErrorResponse does not exist in @rocket.chat/rest-typings yet. Will add 429 schema once the shared validator is available.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Let me check how the existing validators are defined and whether there's already an inline 429 pattern used elsewhere in the codebase.
🏁 Script executed:
#!/bin/bash
# Check how existing validators are defined in rest-typings
rg -n "validateBadRequestErrorResponse|validateUnauthorizedErrorResponse|validateForbiddenErrorResponse|validateTooManyRequests" --type ts -A 5
# Also check if any 429 schema is defined inline elsewhere
rg -n "429" --type ts -B 2 -A 5 apps/meteor/app/api/server/v1/
Length of output: 69067
@Yashika-code, confirmed — there's no validateTooManyRequestsErrorResponse (or any 429 validator/schema) anywhere in the codebase yet. Your explanation is valid.
Two options to keep the OpenAPI contract complete without blocking this PR:
Option A – Inline schema (unblocks this PR immediately)
Add the 429 entry directly in the response map using ajv.compile, following the same shape as the existing error validators:
response: {
400: validateBadRequestErrorResponse,
401: validateUnauthorizedErrorResponse,
403: validateForbiddenErrorResponse,
429: ajv.compile<{ success: false; error: string }>({
type: 'object',
properties: {
success: { type: 'boolean', enum: [false] },
error: { type: 'string' },
},
required: ['success', 'error'],
additionalProperties: false,
}),
200: ajv.compile<{ success: boolean }>({ ... }),
},This can later be replaced with the shared validateTooManyRequestsErrorResponse once it's added to @rocket.chat/rest-typings.
Option B – Follow-up issue
Defer the 429 schema entirely and track it as a follow-up. Would you like me to open a GitHub issue to add validateTooManyRequestsErrorResponse to @rocket.chat/rest-typings so all rate-limited endpoints can reference it consistently?
🧠 Learnings used
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 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: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Contributor
Author
There was a problem hiding this comment.
Thanks for confirming. Since there is currently no shared 429 validator in @rocket.chat/rest-typings, I’ll defer adding the 429 schema for now to keep the migration PR consistent with existing endpoints. It can be added later once a shared validator is introduced.
Contributor
There was a problem hiding this comment.
@Yashika-code, understood — deferring is a reasonable call to keep this migration PR focused. Would you like me to open a follow-up GitHub issue to track adding validateTooManyRequestsErrorResponse to @rocket.chat/rest-typings so all rate-limited endpoints (including users.setStatus) can reference it consistently once it's available?
🧠 Learnings used
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 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: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/migrate-users-setStatus-openapi.md:
- Around line 2-3: The changeset incorrectly classifies the package key string
"@rocket.chat/rest-typings" as minor but the removal of public types is a
breaking change; update the changeset entry in
.changeset/migrate-users-setStatus-openapi.md to mark
"@rocket.chat/rest-typings" as major instead of minor, and ensure the changeset
text explains that the manual v1/users.setStatus typing was removed (so
consumers compiling packages in isolation will break) to correctly reflect the
semver impact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: add1c052-f36e-4f04-84b0-6a7191d2c80e
📒 Files selected for processing (1)
.changeset/migrate-users-setStatus-openapi.md
📜 Review details
🧰 Additional context used
🧠 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: 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.
📚 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-users-setStatus-openapi.md
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
.changeset/migrate-users-setStatus-openapi.md
📚 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:
.changeset/migrate-users-setStatus-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-users-setStatus-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: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
.changeset/migrate-users-setStatus-openapi.md
📚 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:
.changeset/migrate-users-setStatus-openapi.md
📚 Learning: 2026-02-23T17:53:18.785Z
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.
Applied to files:
.changeset/migrate-users-setStatus-openapi.md
Comment on lines
+2
to
+3
| "@rocket.chat/meteor": minor | ||
| "@rocket.chat/rest-typings": minor |
Contributor
There was a problem hiding this comment.
Classify @rocket.chat/rest-typings as major, not minor.
If this PR removes the manual /v1/users.setStatus typing from packages/rest-typings/src/v1/users.ts and replaces it with an augmentation in apps/meteor that isn't visible during isolated compilation (as the previous review comment indicates), consumers who depend on those types will experience a breaking change. Per semantic versioning, removing public types is a major-level breaking change, not a minor addition.
📋 Suggested changeset correction
---
"@rocket.chat/meteor": minor
-"@rocket.chat/rest-typings": minor
+"@rocket.chat/rest-typings": major
---Based on learnings, "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."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "@rocket.chat/meteor": minor | |
| "@rocket.chat/rest-typings": minor | |
| --- | |
| "@rocket.chat/meteor": minor | |
| "@rocket.chat/rest-typings": major | |
| --- |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/migrate-users-setStatus-openapi.md around lines 2 - 3, The
changeset incorrectly classifies the package key string
"@rocket.chat/rest-typings" as minor but the removal of public types is a
breaking change; update the changeset entry in
.changeset/migrate-users-setStatus-openapi.md to mark
"@rocket.chat/rest-typings" as major instead of minor, and ensure the changeset
text explains that the manual v1/users.setStatus typing was removed (so
consumers compiling packages in isolation will break) to correctly reflect the
semver impact.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/app/api/server/v1/users.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/users.ts:901">
P2: AJV schema now forbids empty `message`, but endpoint logic and tests rely on `message: ''` to clear status text, causing a functional regression.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/users.ts`:
- Around line 900-901: The schema validation for the request body currently
disallows empty status messages but the endpoint still treats '' as a valid
clear operation (see the message property in the schema and the call to
setStatusText around lines 947-949); revert the schema change so empty strings
are allowed by removing or setting minLength: 0 for the message field (i.e.,
change message: { type: 'string', minLength: 1 } to allow ''), leaving the rest
of the handler (including the call to setStatusText) unchanged to preserve
existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5fca4f2-33f1-4d59-be54-41d7b2af4812
📒 Files selected for processing (1)
apps/meteor/app/api/server/v1/users.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/users.ts
🧠 Learnings (12)
📓 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: 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.
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/users.ts
📚 Learning: 2026-03-09T18:39:14.020Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:14.020Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/app/api/server/v1/users.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/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:
apps/meteor/app/api/server/v1/users.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/users.ts
📚 Learning: 2026-02-24T19:16:35.307Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 39003
File: apps/meteor/client/lib/chats/flows/sendMessage.ts:39-45
Timestamp: 2026-02-24T19:16:35.307Z
Learning: In apps/meteor/client/lib/chats/flows/sendMessage.ts, when sdk.call('sendMessage', ...) throws an error, the message is intentionally left with temp: true (not deleted or cleaned up) to support a future retry UI feature. This allows users to retry sending failed messages rather than losing them.
Applied to files:
apps/meteor/app/api/server/v1/users.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/users.ts
📚 Learning: 2026-03-09T21:20:07.542Z
Learnt from: pierre-lehnen-rc
Repo: RocketChat/Rocket.Chat PR: 39386
File: apps/meteor/server/services/push/tokenManagement/findDocumentToUpdate.ts:12-15
Timestamp: 2026-03-09T21:20:07.542Z
Learning: In `apps/meteor/server/services/push/tokenManagement/findDocumentToUpdate.ts`, the early return `if (data.voipToken) return null` (Lines 13-15) is intentionally correct. VoIP token updates always include an `_id`, so they are handled by the `_id` lookup block above (Lines 5-9) and never reach this guard. The guard is only a safety net for edge cases where `_id` is absent or no document was found, preventing an incorrect `token + appName` fallback match for VoIP-only payloads.
Applied to files:
apps/meteor/app/api/server/v1/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: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
apps/meteor/app/api/server/v1/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/api/server/v1/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/api/server/v1/users.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Migrates the
users.setStatusREST API endpoint from the legacyAPI.v1.addRoutepattern to the new chainedAPI.v1.post()pattern with AJV request body and response schema validation and OpenAPI documentation support.Closes #34983
Endpoints migrated
users.setStatusaddRoute.post().post()chainedArchitectural changes
API.v1.addRoute('users.setStatus', ...)with the new chained.post('users.setStatus', ...)methodstatus,message,userId,username,userfields400,401,403error response schemasExtractRoutesFromAPI+declare modulefor automatic type inference/v1/users.setStatustyping fromrest-typings(now inferred via module augmentation)Files changed
apps/meteor/app/api/server/v1/users.ts— Main migrationpackages/rest-typings/src/v1/users.ts— Removed old manual typingapps/meteor/tests/end-to-end/api/users.ts— Updated test assertions to match new AJV error formatRelated project
This continues the REST API migration effort described in the GSoC 2026 project: Replace old REST API definitions over the new API.
cc @diego-sampaio @ggazzo
This PR is part of the ongoing REST API migration effort tracked in RocketChat/Rocket.Chat-Open-API#150
Summary by CodeRabbit