Open
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 |
|
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:
WalkthroughMigrates many endpoints from legacy addRoute patterns to explicit API.get/post handlers with AJV request/response schemas, tightens TypeScript and JSON schemas (me, integration, subscription, push, etc.), adds repository analysis CLI scripts, and applies defensive runtime fixes across client/server code and tests. Changes
Sequence Diagram(s)(none) Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #39553 +/- ##
===========================================
- Coverage 70.19% 70.18% -0.02%
===========================================
Files 3280 3280
Lines 116852 116856 +4
Branches 20666 20686 +20
===========================================
- Hits 82020 82011 -9
- Misses 31541 31550 +9
- Partials 3291 3295 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
force-pushed
the
chore/apis
branch
3 times, most recently
from
901177f to
6430325
Compare
ggazzo
force-pushed
the
chore/apis
branch
9 times, most recently
from
26e0171 to
542782d
Compare
|
✅ Layne — scan passed No security issues found on latest push. |
ggazzo
commented
Mar 20, 2026
Member
Author
|
/jira ARCH-1464 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
12 issues found across 40 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="scripts/analyze-weak-types.mjs">
<violation number="1" location="scripts/analyze-weak-types.mjs:46">
P2: `SCHEMA_WEAK_PATTERNS` is declared but never used, so array schemas without `items` are never detected.</violation>
</file>
<file name="apps/meteor/app/api/server/v1/subscriptions.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/subscriptions.ts:26">
P2: `subscriptions.get` response schema incorrectly validates `remove` items as `ISubscription`, but runtime returns only deletion markers (`_id`, `_deletedAt`).</violation>
</file>
<file name="apps/meteor/app/api/server/v1/custom-user-status.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/custom-user-status.ts:164">
P1: Omitting `statusType` on update now clears it to `''` instead of leaving it unchanged.</violation>
</file>
<file name="apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts">
<violation number="1" location="apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179">
P1: Truthy-gating these fields prevents clearing `avatar`, `emoji`, or `alias` to empty strings; use presence checks instead of value truthiness.</violation>
</file>
<file name="scripts/list-weak-response-schemas.mjs">
<violation number="1" location="scripts/list-weak-response-schemas.mjs:16">
P2: Using `URL.pathname` directly can break file scanning on encoded filesystem paths.</violation>
<violation number="2" location="scripts/list-weak-response-schemas.mjs:93">
P2: `bare-object` matching is too broad and flags valid multiline object schemas as weak.</violation>
</file>
<file name="apps/meteor/app/api/server/v1/misc.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/misc.ts:348">
P2: `statusText` is optional on users, so requiring it in the spotlight response schema can reject valid responses.</violation>
</file>
<file name="apps/meteor/app/api/server/v1/channels.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/channels.ts:643">
P1: Using `new Date(0)` counts all historical messages since 1970 as unread for new subscriptions. Fall back to `subscription.ts` to count only messages since the user joined.</violation>
</file>
<file name="apps/meteor/app/api/server/v1/moderation.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/moderation.ts:159">
P2: `createdAt` is validated as `string`, but this endpoint returns `IUser.createdAt` as a `Date`, causing response-schema validation to fail.</violation>
</file>
<file name="apps/meteor/client/lib/userData.ts">
<violation number="1" location="apps/meteor/client/lib/userData.ts:130">
P2: Do not default `email2fa.changedAt` to `new Date()` when the API omits it; this writes a fabricated "changed now" timestamp into user state.
(Based on your team's feedback about avoiding unsafe type assumptions and validating values before coercion.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/meteor/app/api/server/v1/invites.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/invites.ts:63">
P2: `sendInvitationEmail` response schema was loosened with `additionalProperties: true`; this weakens response validation and can hide accidental extra fields.</violation>
</file>
<file name="apps/meteor/app/api/server/v1/mailer.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/mailer.ts:33">
P2: Add a 403 response schema for the permission check on `mailer` so forbidden responses are documented and validated.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts
Show resolved
Hide resolved
![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: 10
♻️ Duplicate comments (1)
apps/meteor/app/api/server/v1/stats.ts (1)
14-21:⚠️ Potential issue | 🟠 Major
statisticsstill skips theIStatscontract.Line 14 only uses
IStatsas a generic; the schema itself accepts any object withsuccess: true. This endpoint does not have thefieldsprojection problem fromstatistics.list, so the actual stats shape can still be enforced here instead of leaving the response effectively untyped.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/stats.ts` around lines 14 - 21, The response schema statisticsResponseSchema currently only enforces success: true and thus ignores the IStats shape; update the Ajv schema used in statisticsResponseSchema (or replace it) to fully describe the IStats structure (all expected properties and types) and set additionalProperties: false (or explicitly include any allowed extra fields) so compile<IStats>(...) truly enforces the IStats contract; locate statisticsResponseSchema in stats.ts and expand the properties object to match IStats (or generate the schema from the TypeScript type) and then recompile with ajv.compile<IStats>.
🧹 Nitpick comments (4)
apps/meteor/tests/end-to-end/api/rooms.ts (1)
2381-2382: Drop the new inline wait comment.Please keep the intent in code instead of an implementation comment.
✂️ Suggested cleanup
- // need to wait for the name update finish await sleep(300);As per coding guidelines "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/end-to-end/api/rooms.ts` around lines 2381 - 2382, Remove the inline comment and express the intent by replacing the comment + await sleep(300) with a clearly named helper call (e.g., await waitForNameUpdate()), implement that helper to encapsulate the waiting strategy (for now it can call sleep(300) or, better, poll the name update condition) and keep the existing sleep function name (sleep) only as the underlying primitive; update any references in the test to call waitForNameUpdate() instead of using an implementation comment before sleep.scripts/list-unmigrated-api-endpoints.mjs (1)
30-45: Count verbs or label this output as “registrations.”A single
addRoute()can still expose multiple handlers, butscan()emits one record per path. That makes theTotalandendpointsoutput registration counts, not endpoint counts, so mixed-method routes understate the remaining migration work.Also applies to: 81-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/list-unmigrated-api-endpoints.mjs` around lines 30 - 45, The scan() function currently emits one record per path which undercounts multi-method handlers; modify scan() to either count verbs per registration or explicitly label records as "registrations": for each match in scan() (using the existing regex and extractPaths(match[2])), parse or extract the handler verbs for that addRoute invocation and include them in the results entries (e.g., add a verbs: [..] array or a registrations: true flag) so the output reflects handler multiplicity; also update the downstream aggregation logic that computes totals (the code that processes results into endpoints/Total around the later block that iterates over results) to sum verb counts when producing endpoint totals or to treat the output as registration counts when the records are labeled, ensuring the final "Total" and endpoints output are accurate and consistent.apps/meteor/app/api/server/v1/custom-user-status.ts (1)
189-209: Missing body schema forcustom-user-status.deleteendpoint.The endpoint accesses
this.bodyParams.customUserStatusId(line 200) but nobodyvalidator is defined in the route options. This allows arbitrary body payloads without AJV validation, relying on the manual check at lines 201-203 instead of schema enforcement.Consider adding a body schema for consistency with the other migrated endpoints:
♻️ Proposed fix
+const isCustomUserStatusDeleteProps = ajv.compile<{ customUserStatusId: string }>({ + type: 'object', + properties: { + customUserStatusId: { type: 'string' }, + }, + required: ['customUserStatusId'], + additionalProperties: false, +}); + const customUserStatusDeleteResponseSchema = ajv.compile<void>({ type: 'object', properties: { success: { type: 'boolean', enum: [true] }, }, required: ['success'], additionalProperties: false, }); API.v1.post( 'custom-user-status.delete', { authRequired: true, + body: isCustomUserStatusDeleteProps, response: { }, }, async function action() { const { customUserStatusId } = this.bodyParams; - if (!customUserStatusId) { - return API.v1.failure('The "customUserStatusId" params is required!'); - } await deleteCustomUserStatus(this.userId, customUserStatusId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/api/server/v1/custom-user-status.ts` around lines 189 - 209, Add a request body schema to the custom-user-status.delete route so AJV validates this.bodyParams.customUserStatusId instead of relying on a manual runtime check: define a request schema (e.g., customUserStatusDeleteRequestSchema) that requires a non-empty customUserStatusId (string) and add it to the route options under the body key of the API.v1.post call for 'custom-user-status.delete'; keep the existing runtime call to deleteCustomUserStatus(this.userId, customUserStatusId) unchanged but remove the manual presence check if you prefer strict schema validation only.apps/meteor/client/lib/userData.ts (1)
126-133: Inconsistent date fallback:new Date()vsnew Date(0)pattern.Lines 109-110 use
new Date(0)(epoch) as a sentinel for missing dates, but line 130 usesnew Date()(current time) for missingemail2fa.changedAt. This inconsistency could cause confusion — a missingchangedAtwould appear as "just now" rather than "unknown/never set."Consider using
new Date(0)for consistency:🔧 Suggested fix
...(email2fa ? { email2fa: { ...email2fa, - changedAt: email2fa.changedAt ? new Date(email2fa.changedAt) : new Date(), + changedAt: email2fa.changedAt ? new Date(email2fa.changedAt) : new Date(0), }, } : {}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/lib/userData.ts` around lines 126 - 133, The email2fa object construction is using new Date() as a fallback for email2fa.changedAt which is inconsistent with other places that use new Date(0) as a sentinel; update the fallback in the email2fa branch so changedAt uses new Date(0) when missing (i.e., replace new Date() with new Date(0)) to match the existing epoch sentinel behavior and ensure uniform "unknown/never set" semantics across the code that reads changedAt.
🤖 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/emoji-custom.ts`:
- Around line 27-38: The response schema emojiCustomAllResponseSchema currently
sets emojis: { type: 'array' } which doesn't validate array items; update the
schema passed to ajv.compile so the emojis property includes an items schema
(either items: { $ref: '<IEmojiCustom ref>' } if IEmojiCustom is registered with
AJV, or a concrete object schema matching IEmojiCustom) and keep
required/additionalProperties rules; ensure the updated shape still includes
success/count/offset/total and passes AJV compilation in the
emojiCustomAllResponseSchema declaration.
In `@apps/meteor/app/api/server/v1/integrations.ts`:
- Around line 36-38: The Typia schema generation currently only registers
IIntegration, but integrations.ts references IIncomingIntegration and
IOutgoingIntegration; open packages/core-typings/src/Ajv.ts and update the
typia.json.schemas(...) tuple to explicitly import and include
IIncomingIntegration and IOutgoingIntegration (following the pattern used for
IDirectoryChannelResult and IDirectoryUserResult) so those concrete refs are
emitted, or alternatively change the refs in integrations.ts to point to the
existing IIntegration schema name; ensure imports and the typia.json.schemas
call include the two constituent types (IIncomingIntegration,
IOutgoingIntegration) alongside IIntegration.
In `@apps/meteor/app/api/server/v1/push.ts`:
- Around line 235-306: The new GET routes 'push.get' and 'push.info' were added
but not included in the extracted route type, so update the module augmentation
that defines PushEndpoints to include these two routes with their expected
request/response shapes; locate the augmentation that declares PushEndpoints
(the same place that currently exposes 'push.token' and 'push.test') and add
entries for 'push.get' and 'push.info' matching the handlers (use the
pushGetResponseSchema shape for 'push.get' and pushInfoResponseSchema shape for
'push.info', and mark both as authRequired GET endpoints) so typed consumers see
the new routes.
In `@apps/meteor/app/api/server/v1/stats.ts`:
- Around line 98-117: The route API.v1.post handler for 'statistics.telemetry'
accepts an unvalidated body (this.bodyParams) and blindly casts/uses it in
function action; add a proper request body schema to the route options (e.g.,
statisticsTelemetryRequestSchema) that requires a params array of objects with
an eventName string and optional params object, enforce it in the route
definition alongside the existing response schemas, and update action to rely on
the validated shape (or defensively check each event contains eventName before
calling telemetryEvent) so telemetryEvent.call is never invoked with undefined
eventName.
In `@apps/meteor/app/api/server/v1/subscriptions.ts`:
- Around line 19-31: subscriptionsGetResponseSchema currently declares remove
items as full ISubscription docs but the TypeScript type expects tombstones
(Pick<ISubscription, '_id'> & {_deletedAt: Date}); update the schema for the
remove array inside subscriptionsGetResponseSchema so its items are an object
with only the tombstone shape (e.g. properties: { _id: { type: 'string' },
_deletedAt: { type: 'string', format: 'date-time' } } and required:
['_id','_deletedAt']) instead of referencing
'#/components/schemas/ISubscription', so Ajv validation matches the TypeScript
type.
In `@apps/meteor/app/authentication/server/startup/index.js`:
- Around line 214-218: Remove the raw console.log dumps that print full signup
payloads (the two console.log calls around beforeCreateUserCallback.run) and
replace them with either no logging or a redacted logger that only records safe
identifiers (e.g., user._id, user.emailHash or user.username, and a note that
callbacks ran) so sensitive fields (password hashes, OAuth tokens, emails in
plain text, approval state) are never written; update the code referencing
beforeCreateUserCallback, options, and user to use the redaction helper or
remove the logs, and apply the same change to the other occurrences mentioned
(the similar console.log uses at the other indicated locations).
In
`@apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`:
- Around line 179-181: The current spread checks like ...(integration.avatar &&
{ avatar: integration.avatar }) use truthiness and prevent explicit clears
(e.g., avatar: ''), so change those guards to test for undefined/property
presence instead (e.g., use Object.prototype.hasOwnProperty.call(integration,
'avatar') or integration.avatar !== undefined) for avatar, emoji, alias and
script in the update logic (the spread object construction where
integration.avatar/emoji/alias/script are conditionally added); this preserves
omitted fields as no-ops but allows explicit empty-string clears to overwrite
stored values.
In `@scripts/analyze-weak-types.mjs`:
- Around line 45-58: The scan never runs the array pattern from
SCHEMA_WEAK_PATTERNS because only findWeakSchemaBlocks() is invoked; update the
schema-only scan to also execute the array check by either extending
findWeakSchemaBlocks() to apply all SCHEMA_WEAK_PATTERNS (including the array
rule) or adding and calling a dedicated matcher (e.g.,
findWeakSchemaArrayMatches or applyPatternMatches) that runs the second pattern;
ensure the main flow that handles the --schema-only flag calls the new/updated
function so the "{ type: 'array' } (no items)" case is reported.
In `@scripts/list-weak-response-schemas.mjs`:
- Around line 81-95: The 'bare-object' rule in WEAK_PATTERNS currently only
checks the current line in its test(line, context) and thus flags multi-line
schemas where "properties:" or "$ref"/combiners appear on subsequent lines;
update the test in the 'bare-object' entry to perform a lookahead using the
provided context (e.g., inspect subsequent lines/tokens from context for a small
window like the next 5-10 non-empty lines or until indentation/closing token)
and return false if any of properties, $ref, allOf, oneOf, or anyOf are found in
that lookahead; keep the original current-line checks but rely on this
multi-line scan to avoid false positives for standard multi-line object schemas.
- Around line 41-64: The chained-route regex is too broad and matches unrelated
calls like settings.get; update the third pattern in routePatterns inside
extractEndpoints to only match method calls that are part of router chains (e.g.
API.v1, variables ending with "Router"/"router", or common app/router names) by
changing the regex to require a router-like identifier before the dot (for
example using a non-capturing group like
(?:(?:API\.v1|\w*Router|\w*router|\bapp|\broutes))\.(get|post|put|delete)\(\s*['"`]([^'"`]+)['"`])
and adjust the match indexing logic in extractEndpoints accordingly so name
comes from match[2] and method from match[1].
---
Duplicate comments:
In `@apps/meteor/app/api/server/v1/stats.ts`:
- Around line 14-21: The response schema statisticsResponseSchema currently only
enforces success: true and thus ignores the IStats shape; update the Ajv schema
used in statisticsResponseSchema (or replace it) to fully describe the IStats
structure (all expected properties and types) and set additionalProperties:
false (or explicitly include any allowed extra fields) so compile<IStats>(...)
truly enforces the IStats contract; locate statisticsResponseSchema in stats.ts
and expand the properties object to match IStats (or generate the schema from
the TypeScript type) and then recompile with ajv.compile<IStats>.
---
Nitpick comments:
In `@apps/meteor/app/api/server/v1/custom-user-status.ts`:
- Around line 189-209: Add a request body schema to the
custom-user-status.delete route so AJV validates
this.bodyParams.customUserStatusId instead of relying on a manual runtime check:
define a request schema (e.g., customUserStatusDeleteRequestSchema) that
requires a non-empty customUserStatusId (string) and add it to the route options
under the body key of the API.v1.post call for 'custom-user-status.delete'; keep
the existing runtime call to deleteCustomUserStatus(this.userId,
customUserStatusId) unchanged but remove the manual presence check if you prefer
strict schema validation only.
In `@apps/meteor/client/lib/userData.ts`:
- Around line 126-133: The email2fa object construction is using new Date() as a
fallback for email2fa.changedAt which is inconsistent with other places that use
new Date(0) as a sentinel; update the fallback in the email2fa branch so
changedAt uses new Date(0) when missing (i.e., replace new Date() with new
Date(0)) to match the existing epoch sentinel behavior and ensure uniform
"unknown/never set" semantics across the code that reads changedAt.
In `@apps/meteor/tests/end-to-end/api/rooms.ts`:
- Around line 2381-2382: Remove the inline comment and express the intent by
replacing the comment + await sleep(300) with a clearly named helper call (e.g.,
await waitForNameUpdate()), implement that helper to encapsulate the waiting
strategy (for now it can call sleep(300) or, better, poll the name update
condition) and keep the existing sleep function name (sleep) only as the
underlying primitive; update any references in the test to call
waitForNameUpdate() instead of using an implementation comment before sleep.
In `@scripts/list-unmigrated-api-endpoints.mjs`:
- Around line 30-45: The scan() function currently emits one record per path
which undercounts multi-method handlers; modify scan() to either count verbs per
registration or explicitly label records as "registrations": for each match in
scan() (using the existing regex and extractPaths(match[2])), parse or extract
the handler verbs for that addRoute invocation and include them in the results
entries (e.g., add a verbs: [..] array or a registrations: true flag) so the
output reflects handler multiplicity; also update the downstream aggregation
logic that computes totals (the code that processes results into endpoints/Total
around the later block that iterates over results) to sum verb counts when
producing endpoint totals or to treat the output as registration counts when the
records are labeled, ensuring the final "Total" and endpoints output are
accurate and consistent.
🪄 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: 64fd18d3-3d8b-4604-8dfd-ffb7e49aa940
📒 Files selected for processing (40)
apps/meteor/app/api/server/default/info.tsapps/meteor/app/api/server/default/openApi.tsapps/meteor/app/api/server/helpers/getUserInfo.tsapps/meteor/app/api/server/v1/channels.tsapps/meteor/app/api/server/v1/commands.tsapps/meteor/app/api/server/v1/custom-user-status.tsapps/meteor/app/api/server/v1/emoji-custom.tsapps/meteor/app/api/server/v1/integrations.tsapps/meteor/app/api/server/v1/invites.tsapps/meteor/app/api/server/v1/mailer.tsapps/meteor/app/api/server/v1/misc.tsapps/meteor/app/api/server/v1/moderation.tsapps/meteor/app/api/server/v1/push.tsapps/meteor/app/api/server/v1/stats.tsapps/meteor/app/api/server/v1/subscriptions.tsapps/meteor/app/authentication/server/startup/index.jsapps/meteor/app/integrations/server/lib/triggerHandler.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.tsapps/meteor/app/livechat/imports/server/rest/rooms.tsapps/meteor/client/lib/chats/readStateManager.tsapps/meteor/client/lib/userData.tsapps/meteor/client/lib/utils/mapSubscriptionFromApi.tsapps/meteor/tests/end-to-end/api/commands.tsapps/meteor/tests/end-to-end/api/rooms.tsapps/meteor/tests/end-to-end/apps/slash-command-test-simple.tsdocs/api-endpoint-migration.mdpackages/core-typings/src/Ajv.tspackages/core-typings/src/IIntegration.tspackages/core-typings/src/IMeApiUser.tspackages/core-typings/src/ISubscription.tspackages/core-typings/src/IVideoConference.tspackages/core-typings/src/index.tspackages/rest-typings/src/index.tspackages/rest-typings/src/v1/Ajv.tspackages/rest-typings/src/v1/me.tspackages/rest-typings/src/v1/me/meSuccessResponse.tsscripts/analyze-weak-types.mjsscripts/list-unmigrated-api-endpoints.mjsscripts/list-weak-response-schemas.mjs
apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts
Show resolved
Hide resolved
| const statisticsListResponseSchema = ajv.compile<{ statistics: IStats[]; count: number; offset: number; total: number }>({ | ||
| type: 'object', | ||
| properties: { | ||
| statistics: { type: 'array' }, |
Contributor
There was a problem hiding this comment.
Suggested change
| statistics: { type: 'array' }, | |
| statistics: { type: 'array', items: { $ref: '#/components/schemas/IStats' } }, |
Contributor
There was a problem hiding this comment.
And we would need to add it in Ajv.ts typia schemas
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/autotranslate.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/autotranslate.ts:94">
P1: Validate `field`/`value` compatibility before saving. The current line allows boolean values for `autoTranslateLanguage` (e.g., `true` -> `'1'`), which persists invalid language settings.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
Upgrade ajv from draft-07 to 2020-12 for unevaluatedProperties support. Add IMeApiUser type, update ISubscription, IIntegration, IVideoConference, IRoom types. Update rest-typings schemas and add user endpoint validators. Fix client-side type adjustments for new typings.
Update forbidden/unauthorized/success method signatures, add SuccessStatusCodes support, improve getUserInfo helper. Remove debug console.log from authentication startup.
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
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OpenAPI Endpoint Migration
Migrates API endpoints from legacy
addRouteto typed.get()/.post()with AJV schema validation for request and response payloads.Migration Progress
~66% migrated — 84
addRouteendpoints remaining (mostlychannelsandgroups).What's Included
unevaluatedPropertiesfor stricter schemasIMeApiUser,ISubscription,IRoom,IIntegrationtype improvementsforbidden()/unauthorized()/success()signature updates,SuccessStatusCodessupport$refschemas for typed responses,validateBadRequestErrorResponse/validateUnauthorizedErrorResponse/validateForbiddenErrorResponsereadStateManager,userData,mapSubscriptionFromApi,RoomsTable,EditRoomInfoWeak Schemas
508 response schemas still use
{ type: 'object' }withoutpropertiesor$ref. These are functional but should be strengthened incrementally.Commits (each migration is independently revertible)
Each commit contains the endpoint migration + its tests + related type fixes:
chore: upgrade Ajv to 2020-12 and update core/rest typingschore: API framework changeschore(api): migrate info, stats, mailerchore(api): migrate push, subscriptionschore(api): migrate invites, custom-user-status, emoji-customchore(api): migrate commandschore(api): migrate dm/imchore(api): migrate chat, autotranslatechore(api): migrate miscchore(api): migrate roomschore(api): migrate teamschore(api): migrate userschore(api): migrate integrations, moderationchore: add migration tracking scripts and docsTask: ARCH-2063