Repository navigation
Gate hosted coderouter on Pro - #9699
Conversation
|
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:
📝 WalkthroughWalkthroughThe PR adds principal-scoped CodeRouter tokens, hosted Pro entitlement checks, subscription-triggered token revocation, and Sentry event filtering and scrubbing. It also adds injectable route dependencies, database migrations, environment validation, UI messaging, and tests. ChangesCodeRouter entitlements and observability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionRoute
participant EntitlementStore
participant RouteTokenRepository
Client->>SessionRoute: POST session request
SessionRoute->>EntitlementStore: Check hosted Pro entitlement
EntitlementStore-->>SessionRoute: Return entitlement status
SessionRoute->>RouteTokenRepository: Issue user-scoped route token
RouteTokenRepository-->>SessionRoute: Return token
SessionRoute-->>Client: Return token and request-derived openaiBaseUrl
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (22 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/app/api/stripe/webhook/route.ts (1)
181-185: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAuthorization Bypass (CWE-862): Missing Authorization
Reachability: External
Reachability path
● Entry web/tests/stripe-webhook-route.test.ts │ ▼ ● Sink web/app/api/stripe/webhook/route.tsRevoke tokens after invoice subscription updates return inactive.
applySubscriptionUpdateclassifiespast_dueas an active Stripe status, but invoice webhook processing can still returnscope: "user", isActive: falsefor unaligned subscription statuses. The invoice branch callsapplySubscriptionUpdatewithout the inactive-user revocation check used incustomer.subscription.*; run the same result guard here and cover invoice failures with a regression test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/api/stripe/webhook/route.ts` around lines 181 - 185, Update the invoice webhook branch around applySubscriptionUpdate to inspect a non-skipped result with scope "user" and isActive false, and revoke that user’s tokens using the same guard and revocation path as the customer.subscription.* handling. Preserve the existing invoice_subscription_unmapped response for skipped results and processed response for active updates, and add a regression test covering an inactive invoice update and token revocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/api/coderouter/session/route.ts`:
- Around line 57-62: Remove the fixed "retry-after": "5" header from the
entitlement_unavailable response in the session route. Keep the 503 status and
no-store cache control, returning the retryable error without a wall-clock retry
policy; do not add sleep-based synchronization or an untested replacement.
- Around line 40-45: Update the entitlement response in the coderouter session
route to obtain the user-facing message, upgrade URL, and self-host URL through
the existing locale-specific localization API rather than hardcoded literals.
Add corresponding catalog entries for every supported locale, preserving the
existing pro_required response structure and keys.
In `@web/app/env.ts`:
- Around line 200-202: Update CODEROUTER_HOSTED_PRO_REQUIRED and the
hostedProRequired() decision so an absent hosted entitlement setting fails
closed instead of returning false; require an explicit configured value or
derive it from authoritative hosting configuration, ensuring authenticated
non-Pro users cannot receive route tokens when hosted enforcement is required.
---
Outside diff comments:
In `@web/app/api/stripe/webhook/route.ts`:
- Around line 181-185: Update the invoice webhook branch around
applySubscriptionUpdate to inspect a non-skipped result with scope "user" and
isActive false, and revoke that user’s tokens using the same guard and
revocation path as the customer.subscription.* handling. Preserve the existing
invoice_subscription_unmapped response for skipped results and processed
response for active updates, and add a regression test covering an inactive
invoice update and token revocation.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 64c0d5c8-748a-4067-9cf2-403ff195963e
📒 Files selected for processing (14)
web/app/api/coderouter/session/route.tsweb/app/api/stripe/webhook/route.tsweb/app/env.tsweb/db/migrations/20260806020000_coderouter_hosted_entitlements/migration.sqlweb/db/schema.tsweb/instrumentation-client.tsweb/instrumentation.tsweb/next.config.tsweb/services/coderouter/repository.tsweb/services/errors.tsweb/services/sentry.tsweb/tests/coderouter-sentry.test.tsweb/tests/coderouter-session-route.test.tsweb/tests/stripe-webhook-route.test.ts
| { | ||
| error: "pro_required", | ||
| message: "Hosted coderouter requires cmux Pro.", | ||
| upgradeUrl: "https://cmux.com/pricing?from=coderouter", | ||
| selfHostUrl: "https://github.com/manaflow-ai/coderouter#self-hosting", | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Localize the entitlement response.
Move the user-facing message and recovery destinations to a locale-specific API source. Add matching entries for every supported locale.
As per coding guidelines, “User-facing text must use localized APIs and matching catalogs.” As per path instructions, apply full-internationalization.md.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/app/api/coderouter/session/route.ts` around lines 40 - 45, Update the
entitlement response in the coderouter session route to obtain the user-facing
message, upgrade URL, and self-host URL through the existing locale-specific
localization API rather than hardcoded literals. Add corresponding catalog
entries for every supported locale, preserving the existing pro_required
response structure and keys.
Sources: Coding guidelines, Path instructions
5fa8277 to
814c225
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@web/db/migrations/20260806020000_coderouter_hosted_entitlements/migration.sql`:
- Around line 6-7: Update the index creation statement for
"coderouter_route_tokens_user_expiry_idx" to use PostgreSQL’s concurrent
index-building form, and ensure this migration runs through the project’s
non-transactional migration path so token writes remain available during
deployment.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fd87cc76-7711-43b9-835f-013a887fbc63
📒 Files selected for processing (14)
web/app/api/coderouter/session/route.tsweb/app/api/stripe/webhook/route.tsweb/app/env.tsweb/db/migrations/20260806020000_coderouter_hosted_entitlements/migration.sqlweb/db/schema.tsweb/instrumentation-client.tsweb/instrumentation.tsweb/next.config.tsweb/services/coderouter/repository.tsweb/services/errors.tsweb/services/sentry.tsweb/tests/coderouter-sentry.test.tsweb/tests/coderouter-session-route.test.tsweb/tests/stripe-webhook-route.test.ts
| CREATE INDEX IF NOT EXISTS "coderouter_route_tokens_user_expiry_idx" | ||
| ON "coderouter_route_tokens" ("stack_user_id", "expires_at"); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Create this index without blocking token writes.
Line 6 uses normal CREATE INDEX. PostgreSQL blocks writes to coderouter_route_tokens while it builds the index. This can block token authentication, issuance, and revocation during deployment.
Use CREATE INDEX CONCURRENTLY in a non-transactional migration path.
Proposed change
-CREATE INDEX IF NOT EXISTS "coderouter_route_tokens_user_expiry_idx"
+CREATE INDEX CONCURRENTLY IF NOT EXISTS "coderouter_route_tokens_user_expiry_idx"🧰 Tools
🪛 SQLFluff (4.2.2)
[error] 6-7: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
🪛 Squawk (2.61.0)
[warning] 6-7: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/db/migrations/20260806020000_coderouter_hosted_entitlements/migration.sql`
around lines 6 - 7, Update the index creation statement for
"coderouter_route_tokens_user_expiry_idx" to use PostgreSQL’s concurrent
index-building form, and ensure this migration runs through the project’s
non-transactional migration path so token writes remain available during
deployment.
Source: Linters/SAST tools
814c225 to
22890e7
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
22890e7 to
aaf9758
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/api/stripe/webhook/route.ts`:
- Around line 165-172: The inactive-user token revocation currently only runs in
the direct subscription-event path. Move the `result.scope === "user" &&
!result.isActive` check and
`dependencies.revokeCoderouterRouteTokens(result.stackUserId)` call into the
shared subscription-processing logic used by `applySubscriptionUpdate`, ensuring
both `invoice.paid` and `invoice.payment_failed` paths execute it while
preserving skipped-result handling.
In
`@web/db/migrations/20260806021000_coderouter_route_token_principals/migration.sql`:
- Around line 6-7: Update the migration around the coderouter_route_tokens
stack_user_id alteration to first add a valid CHECK constraint enforcing
stack_user_id IS NOT NULL, then apply ALTER COLUMN stack_user_id SET NOT NULL so
PostgreSQL can reuse the validated constraint and avoid an exclusive table scan.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ed3c8855-1100-4961-b1d9-4a729c516e95
📒 Files selected for processing (15)
web/app/api/coderouter/session/route.tsweb/app/api/stripe/webhook/route.tsweb/app/env.tsweb/db/migrations/20260806020000_coderouter_hosted_entitlements/migration.sqlweb/db/migrations/20260806021000_coderouter_route_token_principals/migration.sqlweb/db/schema.tsweb/instrumentation-client.tsweb/instrumentation.tsweb/next.config.tsweb/services/coderouter/repository.tsweb/services/errors.tsweb/services/sentry.tsweb/tests/coderouter-sentry.test.tsweb/tests/coderouter-session-route.test.tsweb/tests/stripe-webhook-route.test.ts
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
aaf9758 to
3832b7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/services/sentry.ts`:
- Around line 26-30: Update scrubSentryEvent in web/services/sentry.ts:26-30 to
also remove event.user.id before delivery. Update the expectation at
web/tests/coderouter-sentry.test.ts:53 so scrubbing results in no user data,
including the persistent identifier.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 513ebb84-c1cf-4fb8-a186-2875c75953e5
📒 Files selected for processing (14)
web/app/api/coderouter/session/route.tsweb/app/api/stripe/webhook/route.tsweb/app/env.tsweb/db/migrations/20260806020000_coderouter_hosted_entitlements/migration.sqlweb/db/migrations/20260806021000_coderouter_route_token_principals/migration.sqlweb/db/schema.tsweb/instrumentation.tsweb/next.config.tsweb/services/coderouter/repository.tsweb/services/errors.tsweb/services/sentry.tsweb/tests/coderouter-sentry.test.tsweb/tests/coderouter-session-route.test.tsweb/tests/stripe-webhook-route.test.ts
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/coderouter/page.tsx`:
- Around line 19-24: Move the hard-coded user-facing text in the coderouter page
into next-intl locale catalogs, adding message keys for every supported locale
that cover the labels, availability text, and link labels around the referenced
sections. Update the page’s rendering logic to read those keys through the
existing localization API, including the “included with cmux Pro” link text,
while preserving the current links and layout.
In `@web/services/sentry.ts`:
- Around line 20-23: Update the request-header sanitization in the Sentry
beforeSend flow around event.request.headers to remove all headers or retain
only an explicit allowlist of safe diagnostic headers, ensuring opaque x-api-key
credentials cannot reach Sentry. Add a regression test covering an opaque
x-api-key value and verify it is absent from the event sent through beforeSend.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 595a54db-4dcb-4e2d-a343-e4a49e16b235
📒 Files selected for processing (15)
web/app/api/coderouter/session/route.tsweb/app/api/stripe/webhook/route.tsweb/app/coderouter/page.tsxweb/app/env.tsweb/db/migrations/20260806020000_coderouter_hosted_entitlements/migration.sqlweb/db/migrations/20260806021000_coderouter_route_token_principals/migration.sqlweb/db/schema.tsweb/instrumentation.tsweb/next.config.tsweb/services/coderouter/repository.tsweb/services/errors.tsweb/services/sentry.tsweb/tests/coderouter-sentry.test.tsweb/tests/coderouter-session-route.test.tsweb/tests/stripe-webhook-route.test.ts
| <a | ||
| href="https://cmux.com/pricing?from=coderouter" | ||
| className="rounded-full border border-black/10 bg-white px-3 py-1 font-mono text-[11px] text-black/50 transition-colors hover:text-black" | ||
| > | ||
| included with cmux Pro | ||
| </a> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching coderouter/page.tsx:"
git ls-files | rg '(^|/)coderouter/page\.tsx$|(^|/)messages/|web/i18n/routing\.ts|next-intl' || true
echo
echo "Target file:"
if git ls-files | grep -q 'web/app/coderouter/page.tsx'; then
wc -l web/app/coderouter/page.tsx
cat -n web/app/coderouter/page.tsx
fi
echo
echo "Routing locales:"
if [ -f web/i18n/routing.ts ]; then
cat -n web/i18n/routing.ts
fi
echo
echo "Message dirs and sample files:"
git ls-files 'web/messages/*' | head -100Repository: manaflow-ai/cmux
Length of output: 6763
Move the landing-page copy into the locale catalogs.
The labels around lines 19-24, 45-49, and 67-79 are hard-coded, so the page does not read user-facing text from a locale-specific runtime source. Add next-intl message keys covering each supported locale, and use those keys for the labels, availability text, and link labels.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/app/coderouter/page.tsx` around lines 19 - 24, Move the hard-coded
user-facing text in the coderouter page into next-intl locale catalogs, adding
message keys for every supported locale that cover the labels, availability
text, and link labels around the referenced sections. Update the page’s
rendering logic to read those keys through the existing localization API,
including the “included with cmux Pro” link text, while preserving the current
links and layout.
Sources: Coding guidelines, Path instructions
3832b7a to
56125e1
Compare
|
Addressed the review findings: removed fixed Retry-After and API prose, made production entitlement configuration mandatory, centralized inactive subscription revocation so invoice paths are covered, removed all Sentry request headers and user identity, added opaque API-key and invoice regression tests, and used a validated CHECK before SET NOT NULL. The hard-coded landing additions were reverted rather than adding untranslated copy. The non-concurrent index recommendation is intentionally not applied: this project’s Drizzle migrator runs migrations transactionally, where |
1 participant
Summary
Testing
bun run typecheckbun run db:checkDemo
Review trigger
@coderabbitai review
Checklist