Repository navigation
Move hosted Subrouter onboarding to Stack Auth - #9261
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe change replaces local Subrouter tenant management with hosted-service access through Stack tokens. It removes Vault CLI authentication flows, tenant persistence, related routes and UI, and adds hosted account-route coverage and a CLI configuration endpoint. ChangesHosted Subrouter migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Request as Account request
participant Context as resolveSubrouterRequestContext
participant Stack as Stack authentication
participant Client as HostedSubrouterClient
participant Hosted as Hosted Subrouter service
Request->>Context: Resolve access token
Context->>Stack: Read token or cookie authentication
Stack-->>Context: Return access token
Context->>Client: Create hosted client
Client->>Hosted: exchangeTeam(accessToken, team)
Hosted-->>Client: Return tenant
Client->>Hosted: List, create, delete, or repair account
Hosted-->>Client: Return account response
Client-->>Request: Return normalized account data
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/services/subrouter/routeHelpers.ts (1)
175-183: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not forward upstream 401 and 403 to the caller.
resolveSubrouterRequestContextalready verified the caller's Stack session and team permission before the upstream call. If the hosted service rejects the exchanged token, Line 179 returns that 401 or 403 to the browser. The client then treats a hosted-service failure as its own session failure and can enter a sign-out loop. Map upstream 401 and 403 to 502 and keep pass-through for genuine client-input statuses.Also add the failing operation to the log line. The previous handler logged it, and status alone does not identify which upstream call failed.
🐛 Proposed fix
- const status = err.status >= 400 && err.status < 500 + const status = err.status >= 400 && err.status < 500 && + err.status !== 401 && err.status !== 403 ? err.status : 502;🤖 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/services/subrouter/routeHelpers.ts` around lines 175 - 183, Update the HostedSubrouterError handling in resolveSubrouterRequestContext to map upstream 401 and 403 responses to 502 while preserving pass-through for other genuine 4xx client-input statuses. Extend the console.error log to include the failing upstream operation alongside the status, using the operation context already available to this handler.web/app/api/subrouter/accounts/[accountId]/repair/route.ts (1)
36-45: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRepair is now a non-atomic create-then-delete with no rollback.
Lines 38-41 create the replacement account first and then delete the requested account. If
deleteAccountthrows, Line 44 returns an error to the caller while the new account remains upstream. The tenant is left with both the stale account and the replacement, and the caller has no signal that a partial write occurred. A client retry creates a further duplicate, because the request carries no idempotency key.Choose one of the following:
- Ask the hosted service for a single atomic replace endpoint and call it here, keeping one mutation path.
- Delete the stale account before creating the replacement, and roll back the delete failure explicitly.
- Report the partial state distinctly, so the caller can reconcile instead of retrying blindly.
Note that
web/tests/hosted-subrouter-routes.test.tsLines 210-237 asserts thePOST, POST, DELETEorder and therefore locks in the current non-atomic sequence. Add a case where the delete fails, and assert the reconciliation behavior you choose.As per coding guidelines: "For optimistic UI or CLI updates, use one mutation path, track pending state with a request ID or previous snapshot, reconcile with the authoritative result, and explicitly roll back on failure."
🤖 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/subrouter/accounts/`[accountId]/repair/route.ts around lines 36 - 45, Replace the non-atomic create-then-delete flow in the repair route with a single atomic hosted-service replacement operation, if available, and keep the response based on that operation’s authoritative result. Update the existing hosted-subrouter tests to cover replacement failure and verify no partial mutation or blind-retry behavior; remove the test’s dependency on the current POST, POST, DELETE sequence.Source: Coding guidelines
🤖 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/cli/config/route.ts`:
- Around line 10-22: Update the GET function to validate
env.NEXT_PUBLIC_STACK_PROJECT_ID and
env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY before constructing the success
payload; when either is absent, return an HTTP 503 response with an actionable
configuration error instead of version 1 data. Preserve the existing 200
response and auth payload when both values are configured.
- Around line 6-8: Update the route constants so STACK_CONFIRM_URL derives from
the current deployment origin or an appropriate environment value instead of
hardcoding cmux.com. Replace HOSTED_SUBROUTER_URL with the exported
DEFAULT_HOSTED_SUBROUTER_URL from hostedClient.ts, updating that symbol’s export
and the route import so advertised and called URLs share one default.
In `@web/app/api/subrouter/accounts/route.ts`:
- Around line 20-22: The tenant is resolved repeatedly by account handlers
instead of being shared per request. Extend SubrouterRequestContext and
resolveSubrouterRequestContext in web/services/subrouter/requestContext.ts to
include the resolved tenant, then remove exchangeTeam calls and use the context
tenant in GET and POST in web/app/api/subrouter/accounts/route.ts, the account
handler in web/app/api/subrouter/accounts/[accountId]/route.ts, and the repair
handler in web/app/api/subrouter/accounts/[accountId]/repair/route.ts; preserve
existing account operations while ensuring each request performs the exchange
only once.
In `@web/db/migrations/20260730210000_drop_vault_cli_auth_requests/migration.sql`:
- Around line 1-2: Preserve the tenant-key source before removing
subrouter_tenants: update the migration to archive/export its tenant_id and
encrypted_tenant_key data, or remove/comment out the subrouter_tenants drop
while documenting the hosted service’s authoritative and reversible recovery
path. Keep the vault_cli_auth_requests removal intact, and if this migration is
intended for web.dbMigrations, add the required registration and explanatory
comment.
In `@web/tests/hosted-subrouter-routes.test.ts`:
- Around line 43-50: Add a test in the hosted subrouter route suite that
overrides the mocked getAuthJson behavior to return no access token, then invoke
the cookie-authenticated request and assert a 401 response with calls remaining
empty. Keep the existing beforeEach setup and authenticated test behavior
unchanged.
- Around line 3-6: Capture the original values of the four SUBROUTER_*
environment variables before mutating them, then update the existing afterAll
cleanup to restore each prior value, removing the variable when it was
originally undefined. Keep the current globalThis.fetch restoration unchanged.
---
Outside diff comments:
In `@web/app/api/subrouter/accounts/`[accountId]/repair/route.ts:
- Around line 36-45: Replace the non-atomic create-then-delete flow in the
repair route with a single atomic hosted-service replacement operation, if
available, and keep the response based on that operation’s authoritative result.
Update the existing hosted-subrouter tests to cover replacement failure and
verify no partial mutation or blind-retry behavior; remove the test’s dependency
on the current POST, POST, DELETE sequence.
In `@web/services/subrouter/routeHelpers.ts`:
- Around line 175-183: Update the HostedSubrouterError handling in
resolveSubrouterRequestContext to map upstream 401 and 403 responses to 502
while preserving pass-through for other genuine 4xx client-input statuses.
Extend the console.error log to include the failing upstream operation alongside
the status, using the operation context already available to this handler.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9ea67e2b-318a-4ee0-9e13-d758c5cf25c7
📒 Files selected for processing (44)
web/app/[locale]/dashboard/dashboard-shell.tsxweb/app/[locale]/dashboard/layout.tsxweb/app/[locale]/dashboard/subrouter/page.tsxweb/app/[locale]/dashboard/vault/cli-auth/approve-form.tsxweb/app/[locale]/dashboard/vault/cli-auth/loading.tsxweb/app/[locale]/dashboard/vault/cli-auth/page.tsxweb/app/api/account/route.tsweb/app/api/cli/config/route.tsweb/app/api/subrouter/accounts/[accountId]/repair/route.tsweb/app/api/subrouter/accounts/[accountId]/route.tsweb/app/api/subrouter/accounts/route.tsweb/app/api/subrouter/leases/[leaseId]/events/route.tsweb/app/api/subrouter/leases/route.tsweb/app/api/subrouter/logout/route.tsweb/app/api/subrouter/teams/route.tsweb/app/api/vault/cli/auth/approve/route.tsweb/app/api/vault/cli/auth/poll/route.tsweb/app/api/vault/cli/auth/start/route.tsweb/app/env.tsweb/db/migrations/20260730210000_drop_vault_cli_auth_requests/migration.sqlweb/db/schema.tsweb/messages/en.jsonweb/messages/ja.jsonweb/services/subrouter/README.mdweb/services/subrouter/accountInput.tsweb/services/subrouter/client.tsweb/services/subrouter/crypto.tsweb/services/subrouter/hostedClient.tsweb/services/subrouter/requestContext.tsweb/services/subrouter/routeHelpers.tsweb/services/subrouter/tenants.tsweb/services/subrouter/types.tsweb/services/vault/cliAuth.tsweb/services/vault/routeHelpers.tsweb/tests/account-route.test.tsweb/tests/cli-config-route.test.tsweb/tests/dashboard-subrouter-page.test.tsxweb/tests/hosted-subrouter-client.test.tsweb/tests/hosted-subrouter-routes.test.tsweb/tests/subrouter-accounts-route.test.tsweb/tests/subrouter-crypto.test.tsweb/tests/subrouter-tenants.test.tsweb/tests/vault-cli-auth.test.tsweb/tests/vault-route-helpers.test.ts
💤 Files with no reviewable changes (26)
- web/tests/vault-route-helpers.test.ts
- web/app/api/vault/cli/auth/start/route.ts
- web/app/[locale]/dashboard/vault/cli-auth/loading.tsx
- web/tests/subrouter-crypto.test.ts
- web/tests/subrouter-tenants.test.ts
- web/services/vault/cliAuth.ts
- web/app/[locale]/dashboard/vault/cli-auth/approve-form.tsx
- web/services/subrouter/crypto.ts
- web/tests/subrouter-accounts-route.test.ts
- web/db/schema.ts
- web/app/[locale]/dashboard/vault/cli-auth/page.tsx
- web/tests/vault-cli-auth.test.ts
- web/messages/ja.json
- web/messages/en.json
- web/services/subrouter/tenants.ts
- web/app/api/vault/cli/auth/approve/route.ts
- web/app/api/subrouter/logout/route.ts
- web/app/api/vault/cli/auth/poll/route.ts
- web/app/api/subrouter/leases/route.ts
- web/app/api/subrouter/leases/[leaseId]/events/route.ts
- web/app/[locale]/dashboard/dashboard-shell.tsx
- web/services/vault/routeHelpers.ts
- web/services/subrouter/client.ts
- web/app/api/account/route.ts
- web/tests/account-route.test.ts
- web/app/api/subrouter/teams/route.ts
|
Vercel preview: https://cmux-ach3z85fl-manaflow.vercel.app The preview is READY. Vercel SSO protection applies to browser access. The same worktree API is also running locally on port 3928 for CLI onboarding dogfood. |
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. |
|
Updated Vercel preview: https://cmux-qn18ci3eq-manaflow.vercel.app\n\nDeployment is READY for commit 1d0af02. |
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/app/api/cli/config/route.ts`:
- Line 18: Replace the provider-specific error value in the CLI config route
with the stable provider-neutral code "cli_auth_unavailable". Update the CLI
consumer to recognize this code and adjust web/tests/cli-config-route.test.ts to
assert the new response and consumer behavior, ensuring no provider names or
implementation details remain in the public API body.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f63d5169-6723-451c-baf9-84a189b9a045
📒 Files selected for processing (8)
web/app/api/cli/config/route.tsweb/db/migrations/20260730210000_drop_vault_cli_auth_requests/migration.sqlweb/services/subrouter/README.mdweb/services/subrouter/constants.tsweb/services/subrouter/hostedClient.tsweb/tests/cli-config-route.test.tsweb/tests/hosted-subrouter-client.test.tsweb/tests/hosted-subrouter-routes.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/tests/cli-config-route.test.ts (1)
5-19: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMake the success test independent of ambient environment variables.
If either Stack variable is absent,
GET()correctly returns503, so this test fails before checking the success payload. IfSUBROUTER_HOSTED_URLis set, the expected default URL is also incorrect. The route trims values, but the test compares the untrimmed environment values.Set explicit test values for all three variables, then restore the originals in
finally.🤖 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/tests/cli-config-route.test.ts` around lines 5 - 19, Update the success test around GET() to set explicit values for NEXT_PUBLIC_STACK_PROJECT_ID, NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY, and SUBROUTER_HOSTED_URL before invoking the route. Assert the payload using those configured values, then restore each original environment value in a finally block, preserving undefined values when they were initially absent.
🤖 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.
Outside diff comments:
In `@web/tests/cli-config-route.test.ts`:
- Around line 5-19: Update the success test around GET() to set explicit values
for NEXT_PUBLIC_STACK_PROJECT_ID, NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY, and
SUBROUTER_HOSTED_URL before invoking the route. Assert the payload using those
configured values, then restore each original environment value in a finally
block, preserving undefined values when they were initially absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 54b25ba1-021e-4ac2-89aa-3d1d649bee62
📒 Files selected for processing (2)
web/app/api/cli/config/route.tsweb/tests/cli-config-route.test.ts
|
Current Vercel preview: https://cmux-44folm436-manaflow.vercel.app\n\nDeployment is READY for commit a6f73a8. |
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/tests/cli-config-route.test.ts`:
- Line 17: Update the test around the GET request to control
SUBROUTER_HOSTED_URL and the required Stack credential environment variables
with known values, then assert the corresponding URL response. Restore every
original environment value in a finally block so the test does not depend on or
leak ambient CI state.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b367e257-ceb6-435e-87ec-299e7f8322d6
📒 Files selected for processing (3)
web/services/subrouter/README.mdweb/services/subrouter/constants.tsweb/tests/cli-config-route.test.ts
…nboarding # Conflicts: # web/tests/subrouter-accounts-route.test.ts
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. |
|
Exact-head Vercel preview for Deployment is READY and returns HTTP 200 through authenticated Vercel CLI access. The same head completed a tagged |
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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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. |
The hosted Subrouter account and Stack authentication flows landed in #9261. CodeRouter now stores credentials with KMS encryption (#9686) and enforces private/team account permissions and VM pools (#12771). Keep those shipped implementations instead of adding an unused SR_VAULT_KEY credential store and incomplete device-code flow. The resulting tree is identical to main at a149b7e. Current-path focused tests: 63 passed; web complexity gate passed.
1 participant
Replaces the database-backed CLI auth bridge with Stack Auth native CLI authentication:
/api/cli/config/api/vault/cli/authflow and its Aurora tablesGo service dependency: manaflow-ai/subrouter#120
Verification:
bun run typecheckbun run test(870 pass, 128 intentionally skipped)bun run db:checkThe commits keep the behavior tests separate from the implementation.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Moves hosted Subrouter onboarding to Stack Auth so the dashboard and API use the user’s Stack session with a strict cutover gate and resumable cleanup. Adds a public CLI config endpoint and brokers exact team → hosted tenant exchange; account deletion now retires hosted and legacy tenants with bounded, checkpointed progress.
New Features
/api/cli/configto publish non‑secret CLI config; defaults tohttps://sr.cmux.com(SUBROUTER_HOSTED_URLoverride) and returns 503 with a provider‑neutral error when unconfigured.createHostedSubrouterClientand POST/api/subrouter/exchangeto broker an exact team → hosted tenant exchange using the caller’s Stack access token.subrouter_tenants.hosted_ready_at; blocks legacy‑mapped teams until ready and shows a migration‑pending state in the dashboard.SUBROUTER_STACK_TENANT_DELETE_TOKEN; retires hosted and legacy tenants with serialized, fail‑closed retries and visible checkpoints; skips cleanup when the hosted service isn’t configured./dashboard/subrouterand/api/subrouter/*to use the user’s Stack token, keep team checks, and redact hosted account health to status only; preserves/api/subrouter/teams,/api/subrouter/leases, and/api/subrouter/logout; removes the legacy Vault CLI setup link. Requires Go servicemanaflow-ai/subrouter#120.Migration
SUBROUTER_STACK_TENANT_DELETE_TOKEN(required outside preview) and optionalSUBROUTER_HOSTED_URL; keep legacySUBROUTER_ADMIN_TOKEN/SUBROUTER_BASE_URLonly for migration and retirement of pre‑hosted tenants.hosted_finalization_started_atandhosted_ready_attosubrouter_tenants; addhosted_subrouter_deleted_team_idsandlegacy_subrouter_retired_tenant_idstoaccount_deletion_tombstones.bun subrouter:migrate-legacyto exchange legacy tenants, mark cutover readiness, and optionally finalize sources; the migration source is pinned to the explicit target (stagingorproduction) to avoid cross‑env mistakes.Written for commit 3f04cc4. Summary will update on new commits.
Summary by CodeRabbit