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]>
|
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 |
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughThis pull request enhances API reliability and type safety by adding optional status code support to the core API success method, refining endpoint response schemas with more precise types, implementing request validation via AJV schemas, refactoring shared endpoint code, and updating corresponding REST type definitions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Suggested labels
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 #39851 +/- ##
==============================================
- Coverage 70.60% 70.60% -0.01%
==============================================
Files 3256 3256
Lines 115791 115792 +1
Branches 21043 21053 +10
==============================================
- Hits 81753 81750 -3
- Misses 31976 31979 +3
- Partials 2062 2063 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
- 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.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 11 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="apps/meteor/app/api/server/v1/commands.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/commands.ts:158">
P2: This branch returns HTTP 202, but `commands.list` only defines schemas for 200/400/401. Use a declared status code or add a 202 response schema to keep the endpoint contract consistent.</violation>
</file>
<file name="apps/meteor/app/api/server/v1/chat.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/chat.ts:678">
P2: Do not default missing `_deletedAt` to the current time in sync payloads; this returns incorrect deletion metadata.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| total: 0, | ||
| success: true as const, | ||
| }, | ||
| 202, |
There was a problem hiding this comment.
P2: This branch returns HTTP 202, but commands.list only defines schemas for 200/400/401. Use a declared status code or add a 202 response schema to keep the endpoint contract consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/api/server/v1/commands.ts, line 158:
<comment>This branch returns HTTP 202, but `commands.list` only defines schemas for 200/400/401. Use a declared status code or add a 202 response schema to keep the endpoint contract consistent.</comment>
<file context>
@@ -146,13 +146,17 @@ const commandsEndpoints = API.v1
+ total: 0,
+ success: true as const,
+ },
+ 202,
+ );
}
</file context>
| 202, | |
| 200, |
| ? result.deleted.map((msg) => ({ | ||
| _id: msg._id, | ||
| _deletedAt: | ||
| '_deletedAt' in msg && msg._deletedAt instanceof Date ? msg._deletedAt.toISOString() : new Date().toISOString(), |
There was a problem hiding this comment.
P2: Do not default missing _deletedAt to the current time in sync payloads; this returns incorrect deletion metadata.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/api/server/v1/chat.ts, line 678:
<comment>Do not default missing `_deletedAt` to the current time in sync payloads; this returns incorrect deletion metadata.</comment>
<file context>
@@ -655,7 +670,14 @@ const chatEndpoints = API.v1
+ ? result.deleted.map((msg) => ({
+ _id: msg._id,
+ _deletedAt:
+ '_deletedAt' in msg && msg._deletedAt instanceof Date ? msg._deletedAt.toISOString() : new Date().toISOString(),
+ }))
+ : [],
</file context>
1 participant
…ge parameter
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Tests