chore(api): address PR review issues for OpenAPI endpoint migration#39864
Merged
ggazzo merged 17 commits intochore/apisfrom Mar 26, 2026
Merged
chore(api): address PR review issues for OpenAPI endpoint migration#39864ggazzo merged 17 commits intochore/apisfrom
ggazzo merged 17 commits intochore/apisfrom
Conversation
- Replace bare `{ type: 'object' }` with `$ref IMessage` in response schemas where applicable
- Add `// relaxed:` comments to deliberately loose schemas explaining why
- Restore HTTP 202 status for commands.list when apps aren't loaded
- Restore explicit null check in commands.run/preview thread validation
- Add missing body validator to custom-user-status.delete endpoint
- Extract shared dmCreateAction factory for dm.create/im.create (DRY)
- Remove statusText from spotlight required fields (not always present)
- Fix sendInvitationEmailResponseSchema additionalProperties to false
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Update success method in APIClass to accept an optional status code. - Modify commands.list response to utilize the new success method with a 202 status code when apps are not loaded. - Refactor thread validation checks in commands.run and commands.preview for consistency.
Deleted messages only contain `_id` and `_deletedAt`, not full IMessage objects. The response schema was incorrectly referencing IMessage, causing validation to fail with "must have required property 'rid'". Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Updated the response schema to clarify that deleted messages only include `_id` and `_deletedAt` properties, ensuring proper validation and alignment with the expected structure.
…se ISO string format Modified the response schema for deleted messages to ensure `_deletedAt` is returned as an ISO string instead of a Date object, improving consistency and compatibility with the API response structure.
…om-user-status endpoint Revised the error response for the custom-user-status API to provide a clearer message indicating that the 'customUserStatusId' parameter is required, enhancing the clarity of API validation feedback.
The onCreateUserAsync function had 3 console.log calls that could expose password hashes, OAuth tokens, and emails in server logs. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Prevents cross-room file metadata exposure by checking that the upload referenced by startingFromId belongs to the same room being queried. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
IIncomingIntegration and IOutgoingIntegration don't exist as separate schemas in Typia. Use IIntegration (which is their union) instead. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The endpoint accepted unvalidated input and could call telemetryEvent with undefined eventName. Adds isTelemetryPayload AJV validator. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
topic can be undefined when not provided in the request body, but the response schema only accepted string, causing validation failures. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The mailer endpoint has permissionsRequired but was missing the 403 response schema, and was also missing the 400 bad request schema. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The endpoint has authRequired but was missing the 401 unauthorized response schema for OpenAPI documentation. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
WalkthroughAPI schemas and validators were refined (added/ reordered response codes, nullable field), a telemetry payload validator was added and used by the stats endpoint, minor AJV schema formatting was applied, and three debug console.log calls were removed from account creation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## chore/apis #39864 +/- ##
==============================================
- Coverage 70.59% 70.57% -0.02%
==============================================
Files 3256 3256
Lines 115792 115792
Branches 21064 21017 -47
==============================================
- Hits 81744 81721 -23
- Misses 31986 32009 +23
Partials 2062 2062
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/rest-typings/src/v1/statistics.ts">
<violation number="1" location="packages/rest-typings/src/v1/statistics.ts:79">
P1: Telemetry payload validation is too permissive and does not match `TelemetryPayload` type (event enum and required per-event fields are not enforced).</violation>
<violation number="2" location="packages/rest-typings/src/v1/statistics.ts:80">
P2: `timestamp` schema allows `null`, but `TelemetryPayload` type only allows `number | undefined`, causing a validator/type mismatch.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| items: { | ||
| type: 'object', | ||
| properties: { | ||
| eventName: { type: 'string' }, |
Contributor
There was a problem hiding this comment.
P1: Telemetry payload validation is too permissive and does not match TelemetryPayload type (event enum and required per-event fields are not enforced).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/rest-typings/src/v1/statistics.ts, line 79:
<comment>Telemetry payload validation is too permissive and does not match `TelemetryPayload` type (event enum and required per-event fields are not enforced).</comment>
<file context>
@@ -68,6 +68,27 @@ const StatisticsListSchema = {
+ items: {
+ type: 'object',
+ properties: {
+ eventName: { type: 'string' },
+ timestamp: { type: 'number', nullable: true },
+ },
</file context>
| type: 'object', | ||
| properties: { | ||
| eventName: { type: 'string' }, | ||
| timestamp: { type: 'number', nullable: true }, |
Contributor
There was a problem hiding this comment.
P2: timestamp schema allows null, but TelemetryPayload type only allows number | undefined, causing a validator/type mismatch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/rest-typings/src/v1/statistics.ts, line 80:
<comment>`timestamp` schema allows `null`, but `TelemetryPayload` type only allows `number | undefined`, causing a validator/type mismatch.</comment>
<file context>
@@ -68,6 +68,27 @@ const StatisticsListSchema = {
+ type: 'object',
+ properties: {
+ eventName: { type: 'string' },
+ timestamp: { type: 'number', nullable: true },
+ },
+ required: ['eventName'],
</file context>
Suggested change
| timestamp: { type: 'number', nullable: true }, | |
| timestamp: { type: 'number' }, |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/app/api/server/v1/im.ts`:
- Around line 272-276: dmSetTopicResponseSchema's TypeScript generic doesn't
match the JSON schema: the schema allows topic to be null but the generic is {
topic?: string }, causing a type/validation mismatch. Update the generic on
dmSetTopicResponseSchema to reflect that topic can be null (e.g., { topic?:
string | null }) so the compile-time type aligns with the schema's nullable
topic property; ensure any callers that rely on topic adjust for possible null.
🪄 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: ef9c7f91-4a0a-489b-9973-80b399239b93
📒 Files selected for processing (9)
apps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/integrations.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/rooms.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/authentication/server/startup/index.jspackages/rest-typings/src/index.tspackages/rest-typings/src/v1/statistics.ts
💤 Files with no reviewable changes (1)
- apps/meteor/app/authentication/server/startup/index.js
📜 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). (15)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.0 coverage (3/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.0 coverage (1/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.0 coverage (5/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.0 coverage (4/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.0 coverage (2/5)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.0 (4/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.0 (3/4)
- GitHub Check: 🔨 Test API Livechat (CE) / MongoDB 8.0 (1/1)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.0 (1/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.0 (2/4)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.0 coverage (1/1)
- GitHub Check: 🔨 Test API (CE) / MongoDB 8.0 (1/1)
- GitHub Check: 🔨 Test API Livechat (EE) / MongoDB 8.0 coverage (1/1)
- GitHub Check: 🔨 Test Federation Matrix
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tspackages/rest-typings/src/v1/statistics.tsapps/meteor/app/api/server/v1/rooms.tspackages/rest-typings/src/index.ts
🧠 Learnings (25)
📓 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: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: apps/meteor/app/api/server/v1/users.ts:862-869
Timestamp: 2026-03-16T23:33:15.721Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs (e.g., PR `#39676` for users.register in apps/meteor/app/api/server/v1/users.ts), calls to `this.parseJsonQuery()` inside migrated handlers are intentionally preserved without adding a corresponding `query` AJV schema to the route options. Adding query-param schemas for the `fields`/`sort`/`query` parameters consumed by `parseJsonQuery()` is a separate cross-cutting concern shared by many endpoints (e.g., users.create, users.update, users.list) and is explicitly out of scope for individual endpoint migration PRs. Do not flag the absence of a `query` schema for `parseJsonQuery()` usage as a violation of OpenAPI/AJV contract during migration reviews.
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
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: 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.
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.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/api/server/v1/stats.ts:98-117
Timestamp: 2026-03-20T13:52:29.575Z
Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.
📚 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/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/integrations.tsapps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tspackages/rest-typings/src/v1/statistics.ts
📚 Learning: 2026-03-20T13:52:29.575Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/api/server/v1/stats.ts:98-117
Timestamp: 2026-03-20T13:52:29.575Z
Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tspackages/rest-typings/src/v1/statistics.ts
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tspackages/rest-typings/src/v1/statistics.tspackages/rest-typings/src/index.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.ts
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.ts
📚 Learning: 2026-03-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/integrations.ts
📚 Learning: 2026-03-15T14:31:23.493Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:891-899
Timestamp: 2026-03-15T14:31:23.493Z
Learning: In RocketChat/Rocket.Chat, `IUser.inactiveReason` in `packages/core-typings/src/IUser.ts` is typed as `'deactivated' | 'pending_approval' | 'idle_too_long'` (optional, no `null`), but the database stores `null` for newly created users. The Typia-generated `$ref: '#/components/schemas/IUser'` schema therefore correctly rejects `null` for `inactiveReason`. This causes the test "should create a new user with default roles" to fail when response validation is active (TEST_MODE). The fix is to add `| null` to `inactiveReason` in core-typings and rebuild Typia schemas in a separate PR. Do not flag this test failure as a bug introduced by the users.create OpenAPI migration (PR `#39647`). Do not suggest inlining a custom schema to work around it, as migration rules require using `$ref` when a Typia schema exists.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/integrations.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tsapps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2026-03-10T08:13:52.153Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tsapps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tsapps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tspackages/rest-typings/src/v1/statistics.tsapps/meteor/app/api/server/v1/rooms.tspackages/rest-typings/src/index.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/app/api/server/v1/im.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tspackages/rest-typings/src/v1/statistics.tsapps/meteor/app/api/server/v1/rooms.tspackages/rest-typings/src/index.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/stats.ts
📚 Learning: 2026-03-14T14:58:58.834Z
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Applied to files:
apps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.ts
📚 Learning: 2026-03-16T23:33:15.721Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: apps/meteor/app/api/server/v1/users.ts:862-869
Timestamp: 2026-03-16T23:33:15.721Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs (e.g., PR `#39676` for users.register in apps/meteor/app/api/server/v1/users.ts), calls to `this.parseJsonQuery()` inside migrated handlers are intentionally preserved without adding a corresponding `query` AJV schema to the route options. Adding query-param schemas for the `fields`/`sort`/`query` parameters consumed by `parseJsonQuery()` is a separate cross-cutting concern shared by many endpoints (e.g., users.create, users.update, users.list) and is explicitly out of scope for individual endpoint migration PRs. Do not flag the absence of a `query` schema for `parseJsonQuery()` usage as a violation of OpenAPI/AJV contract during migration reviews.
Applied to files:
apps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/integrations.ts
📚 Learning: 2026-03-11T18:15:53.272Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2026-03-19T13:59:40.678Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/uploads.ts:45-49
Timestamp: 2026-03-19T13:59:40.678Z
Learning: In `apps/meteor/app/apps/server/converters/uploads.ts`, the `room` async handler in `convertToApp` uses non-null assertions (`upload.rid!` and `result!`) intentionally. The data flow guarantees that any upload reaching this point must have a `rid`; if it does not, throwing an error is the desired behavior (fail-fast / data integrity guard). Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
📚 Learning: 2026-03-11T18:17:53.972Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/processMessageUploads.ts:112-119
Timestamp: 2026-03-11T18:17:53.972Z
Learning: In `apps/meteor/client/lib/chats/flows/processMessageUploads.ts`, when sending multiple file uploads, each file is confirmed via its own `/rooms.mediaConfirm/${rid}/${fileId}` call and produces a separate message. Only the first file's confirm payload carries the composed message text (`msg`); all subsequent files receive `msg: ''`. This one-message-per-file behavior is intentional by design — do not flag it as a bug or suggest batching into a single message.
Applied to files:
apps/meteor/app/api/server/v1/rooms.ts
🔇 Additional comments (7)
apps/meteor/app/api/server/v1/integrations.ts (1)
33-40: Schema unification toIIntegrationlooks correct and scope-tight.The updated
$refusage is consistent across create/list/get (and reused remove/update responses), and keeps this migration focused on response-contract alignment without introducing endpoint logic changes.Based on learnings: "In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior."
Also applies to: 145-152, 282-292
apps/meteor/app/api/server/v1/mailer.ts (1)
1-8: Response contract update formaileris solid.The added
400/403validators are correctly imported and mapped, aligning declared responses with body validation and permission enforcement.Also applies to: 38-43
apps/meteor/app/api/server/v1/emoji-custom.ts (1)
183-184:emoji-custom.createresponse declarations look correct.Explicit
400and401mappings improve endpoint contract clarity without introducing logic changes.apps/meteor/app/api/server/v1/rooms.ts (1)
649-651: LGTM! Good security fix to prevent cross-room metadata exposure.The validation correctly ensures that
startingFromIdbelongs to the requested room before using it for pagination. Silently treating invalid IDs as absent (rather than erroring) is a reasonable approach that doesn't leak information about whether the ID exists in another room.packages/rest-typings/src/index.ts (1)
251-251: Good entrypoint export addition.Line 251 correctly exposes
./v1/statisticsfor downstream consumers of@rocket.chat/rest-typings.apps/meteor/app/api/server/v1/stats.ts (1)
103-116: Telemetry forwarding shape is correctly normalized.Line 114–Line 115 correctly strips
eventNameand passes only event payload data totelemetryEvent.call, which matches the telemetry handler contract.packages/rest-typings/src/v1/statistics.ts (1)
71-91:⚠️ Potential issue | 🟠 MajorAJV schema must enforce event-specific fields to match the
Paramdiscriminated union type.The schema at lines 79-82 accepts any string
eventNameand requires only that field, so payloads missingcommand(forslashCommandsStats) orsettingsId(forupdateCounter) pass validation despite violating the TypeScript type contract. At runtime,telemetryEvent.call(eventName, rest)then fails when handlers consume event-specific data.The fix must replace the generic items schema with a
oneOfdiscriminated union that enforceseventNameas an enum, requires event-specific fields, and disallows additional properties for each event variant:Proposed fix
const TelemetryPayloadSchema = { type: 'object', properties: { params: { type: 'array', items: { - type: 'object', - properties: { - eventName: { type: 'string' }, - timestamp: { type: 'number', nullable: true }, - }, - required: ['eventName'], + oneOf: [ + { + type: 'object', + properties: { + eventName: { const: 'slashCommandsStats' }, + command: { type: 'string' }, + timestamp: { type: 'number', nullable: true }, + }, + required: ['eventName', 'command'], + additionalProperties: false, + }, + { + type: 'object', + properties: { + eventName: { const: 'updateCounter' }, + settingsId: { type: 'string' }, + timestamp: { type: 'number', nullable: true }, + }, + required: ['eventName', 'settingsId'], + additionalProperties: false, + }, + ], }, }, }, required: ['params'], additionalProperties: false, };⛔ Skipped due to learnings
Learnt from: ggazzo Repo: RocketChat/Rocket.Chat PR: 39553 File: apps/meteor/app/api/server/v1/stats.ts:98-117 Timestamp: 2026-03-20T13:52:29.575Z Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.Learnt from: amitb0ra Repo: RocketChat/Rocket.Chat PR: 39647 File: apps/meteor/app/api/server/v1/users.ts:710-757 Timestamp: 2026-03-15T14:31:28.969Z Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.Learnt from: amitb0ra Repo: RocketChat/Rocket.Chat PR: 39676 File: apps/meteor/app/api/server/v1/users.ts:862-869 Timestamp: 2026-03-16T23:33:15.721Z Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs (e.g., PR `#39676` for users.register in apps/meteor/app/api/server/v1/users.ts), calls to `this.parseJsonQuery()` inside migrated handlers are intentionally preserved without adding a corresponding `query` AJV schema to the route options. Adding query-param schemas for the `fields`/`sort`/`query` parameters consumed by `parseJsonQuery()` is a separate cross-cutting concern shared by many endpoints (e.g., users.create, users.update, users.list) and is explicitly out of scope for individual endpoint migration PRs. Do not flag the absence of a `query` schema for `parseJsonQuery()` usage as a violation of OpenAPI/AJV contract during migration reviews.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: 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.Learnt from: amitb0ra Repo: RocketChat/Rocket.Chat PR: 39676 File: .changeset/migrate-users-register-openapi.md:3-3 Timestamp: 2026-03-16T21:50:42.118Z Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Comment on lines
272
to
276
| const dmSetTopicResponseSchema = ajv.compile<{ topic?: string }>({ | ||
| type: 'object', | ||
| properties: { | ||
| topic: { type: 'string' }, | ||
| topic: { type: 'string', nullable: true }, | ||
| success: { type: 'boolean', enum: [true] }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify mismatch between schema nullability and TypeScript generic in im.setTopic response schema
rg -n -C3 "const dmSetTopicResponseSchema|ajv\.compile<\{ topic\?: string \}>|topic: \{ type: 'string', nullable: true \}"Repository: RocketChat/Rocket.Chat
Length of output: 1092
🏁 Script executed:
# Find and examine the dmSetTopicAction handler to verify what it actually returns
rg -n -A 20 "const dmSetTopicAction|dmSetTopicAction.*=" apps/meteor/app/api/server/v1/im.ts | head -50Repository: RocketChat/Rocket.Chat
Length of output: 899
🏁 Script executed:
# Check other response schemas in im.ts to see if they have similar type/schema divergences
rg -n "const dm.*ResponseSchema = ajv.compile" apps/meteor/app/api/server/v1/im.tsRepository: RocketChat/Rocket.Chat
Length of output: 703
🏁 Script executed:
# Check if the endpoint type was migrated from rest-typings or if it's new
fd "im.ts" packages/rest-typings/ 2>/dev/null | head -5Repository: RocketChat/Rocket.Chat
Length of output: 103
🏁 Script executed:
# Get the complete return statement from dmSetTopicAction
rg -n -A 5 "return API.v1.success\({" apps/meteor/app/api/server/v1/im.ts | grep -A 5 "293:" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 177
🏁 Script executed:
# Check the original rest-typings source for this endpoint to see if the type/schema mismatch was pre-existing
cat packages/rest-typings/src/v1/dm/im.ts | head -100Repository: RocketChat/Rocket.Chat
Length of output: 2438
🏁 Script executed:
# Get the complete return statement from dmSetTopicAction to see what's actually returned
rg -n -A 8 "const dmSetTopicAction = " apps/meteor/app/api/server/v1/im.ts | tail -15Repository: RocketChat/Rocket.Chat
Length of output: 495
🏁 Script executed:
# Check the git history or comment for when nullable was added to topic in response schema
git log -p --all -S "topic: { type: 'string', nullable: true }" -- apps/meteor/app/api/server/v1/im.ts | head -50Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Get the complete dmSetTopicAction to see the full return statement
rg -n -A 20 "return API.v1.success" apps/meteor/app/api/server/v1/im.ts | grep -A 3 "313:"Repository: RocketChat/Rocket.Chat
Length of output: 127
🏁 Script executed:
# Check if topic field is always populated or could be null/undefined in response
rg -n -B5 -A15 "const dmSetTopicAction" apps/meteor/app/api/server/v1/im.ts | tail -25Repository: RocketChat/Rocket.Chat
Length of output: 821
Align dmSetTopicResponseSchema generic with nullable topic.
The schema now allows topic: null, but the generic type is still { topic?: string }, creating a type/schema contract mismatch.
Proposed fix
-const dmSetTopicResponseSchema = ajv.compile<{ topic?: string }>({
+const dmSetTopicResponseSchema = ajv.compile<{ topic?: string | null }>({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const dmSetTopicResponseSchema = ajv.compile<{ topic?: string }>({ | |
| type: 'object', | |
| properties: { | |
| topic: { type: 'string' }, | |
| topic: { type: 'string', nullable: true }, | |
| success: { type: 'boolean', enum: [true] }, | |
| const dmSetTopicResponseSchema = ajv.compile<{ topic?: string | null }>({ | |
| type: 'object', | |
| properties: { | |
| topic: { type: 'string', nullable: true }, | |
| success: { type: 'boolean', enum: [true] }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/api/server/v1/im.ts` around lines 272 - 276,
dmSetTopicResponseSchema's TypeScript generic doesn't match the JSON schema: the
schema allows topic to be null but the generic is { topic?: string }, causing a
type/validation mismatch. Update the generic on dmSetTopicResponseSchema to
reflect that topic can be null (e.g., { topic?: string | null }) so the
compile-time type aligns with the schema's nullable topic property; ensure any
callers that rely on topic adjust for possible null.
IIncomingIntegration and IOutgoingIntegration DO exist as separate Typia schemas. The union type IIntegration is not generated. Reverts the incorrect change from 154983f. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…ons.create The integrationSuccessSchema still referenced #/components/schemas/IIntegration which Typia does not generate (it's a union alias). Use the same oneOf pattern applied to the other two schemas in 2f937ff. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
ggazzo
changed the title
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.
…ge parameter
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
{ type: 'object' }with$ref IMessagein response schemas where applicable// relaxed:comments to deliberately loose schemas explaining whyCo-Authored-By: Claude Opus 4.6 (1M context) [email protected]
Summary by CodeRabbit
Bug Fixes
Improvements
Chores