Skip to content

fix(coderouter): initialize Cloud VM account pools - #16397

Merged
austinywang merged 4 commits into
mainfrom
16389-cloud-coderouter-accounts
Oct 1, 2026
Merged

austinywang merged 4 commits into
mainfrom
16389-cloud-coderouter-accounts

Conversation

@austinywang

@austinywang austinywang commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #16389

Cloud CodeRouter sessions can report healthy team accounts on the host while a VM's default pool has never been initialized, leaving the VM's Codex selector empty. This change snapshots the selected team's shared accounts into the VM's default pool before the first signed model-plane token is persisted. A durable pool-initialization marker makes the snapshot one-time, so later token refreshes preserve deliberate pool revocations; native and Claude account families seed independently.

Impact map

  • Source of truth: cloud_vms.owner_team_id, its default coderouter_pool_id, team-visible CodeRouter account rows, and the new coderouter_pool_initializations marker.
  • Callers and lifecycle: issueVmAuthorizationToken is called by VM model-plane provisioning; guest account listing and Codex selection consume the resulting pool through accountAccessPredicate. Existing account-import, sharing, token revocation, and destroy paths remain unchanged.
  • Cross-surface effects: the web migration/schema add only the initialization marker; no Swift, guest image, Codex configuration, or user-facing copy changes.
  • Regression coverage: the DB behavior test clears a VM's pool, issues the model-plane token, verifies a shared Codex account is listed and selected, verifies a shared Claude account is present and a private Codex account is excluded, then revokes the Codex grant and verifies a later token refresh does not restore it.
  • Security boundary: only team-visible accounts are snapshotted into the default organization pool. Private imports remain private and require explicit sharing before a Cloud VM can use them.

Validation

  • git diff --check passed.
  • python3 scripts/verify-local.py --only feature-flags passed.
  • Focused Bun DB test was attempted locally but could not start because this checkout has no installed web/node_modules (effect was missing after the disk-full install).
  • Hosted web CI: typecheck, production build, four web-test shards, migration apply, and web status passed on the prior head; the focused manual web-validation run is executing the DB behavior suite for the current head.
  • Base SHA: a20ed74ca17b759af39fc53be8227078e8387f6d
  • Head SHA: 4730d22703290415bbc55e3f0035f174bd6d3afd
  • Merge gate: CONFLICT CHECK: PASS on the prior head; rerun required after this push.
  • Review follow-up: added private/Claude assertions and backfilled markers for pools that predate this migration.
  • Changelog: Fixed Cloud CodeRouter VM pools now expose the selected organization's shared accounts to Codex.

Summary by CodeRabbit

  • New Features
    • VMs now receive access to team-visible native and Claude accounts in their team’s default pool when first authorized.
  • Bug Fixes
    • Team-visible legacy Codex accounts remain available in VM requests even if pool grants are cleared.
    • Private accounts remain hidden, and accounts no longer granted to a pool are removed from VM account listings.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f01455f6-0bdd-45f5-aceb-ce2ab9630c78

📥 Commits

Reviewing files that changed from the base of the PR and between a20ed74 and 4730d22.

📒 Files selected for processing (4)
  • web/db/migrations/20261001000000_coderouter_vm_pool_initialization/migration.sql
  • web/db/schema.ts
  • web/services/coderouter/repository.ts
  • web/tests/coderouter-vm-scope-db-behavior.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

VM authorization now records initialization for a qualifying team default pool and grants it team-visible native and Claude accounts when the marker is new. A migration marks existing pools as initialized. A database test checks VM visibility for shared Codex accounts.

Changes

CodeRouter VM pool initialization

Layer / File(s) Summary
Default pool initialization and account grants
web/db/schema.ts, web/db/migrations/20261001000000_coderouter_vm_pool_initialization/migration.sql, web/services/coderouter/repository.ts, web/tests/coderouter-vm-scope-db-behavior.test.ts
The schema and migration add an initialization marker and mark existing pools as initialized. During VM authorization, the repository adds team-visible native and Claude account grants when it newly marks a qualifying team default pool. The database test checks shared Codex account visibility when grants are cleared and after a grant is removed.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant VMAuthorization
  participant Repository
  participant Database
  participant TokenSigner
  VMAuthorization->>Repository: Request VM authorization token
  Repository->>Database: Insert initialization marker for qualifying default pool
  alt Marker is new
    Repository->>Database: Grant team-visible native and Claude accounts
  end
  Repository->>TokenSigner: Sign VM authorization token
Loading

Merge Risk: ⚪ Minimal · up to 4730d

The change initializes new eligible VM pools while preserving private-account exclusions and later revocations. No concrete merge-blocking issue was identified; normal database validation should complete before deployment.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4730d

The change keeps account access tied to the VM’s owning team and preserves deliberate grant removals after initialization. No introduced security violation was established. Concurrent pool reassignment and production rollout behavior remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Initialization changes a shared team default pool, not a VM-exclusive pool. Its grant effects therefore extend to eligible VMs using that same pool, while account-team checks and composite foreign keys constrain the authority to that team.

Trust Boundaries and Controls

  • observed — Signed VM credentials are checked against the persisted token hash, claimed team, owner and VM, expiration, revocation, live VM owner team, pool team, and allowed VM status. Newly created grants do not replace those identity checks.

Resilience and Maintainability Implications

  • observed — The added database regression coverage exercises shared Codex listing and selection, shared Claude visibility, private-account exclusion, and preservation of a removed grant after subsequent token issuance. This covers sequential behavior, not concurrent pool reassignment.

Hardening Proposals

  • proposed — Bind both grant inserts to the pool ID returned by marker insertion, or serialize pool reassignment during initialization. This would preserve the checked pool identity across statements if same-team reassignment is supported; no attacker-accessible reassignment path was established.
🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses the coding requirements in [#16389]. issueVmAuthorizationToken initializes the VM default pool only for the selected team and grants team-visible native and Claude accounts. The ini…
Out of Scope Changes check ✅ Passed The migration, schema change, repository transaction, and database regression test all support VM credential bootstrap and pool behavior required by [#16389]. No unrelated change is demonstrated by th…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 …
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS — The pull request changes CodeRouter VM account-pool initialization, database schema/migration, token issuance, and tests. It does not change Cloud terminal creation, cmux-tui clients, transport…
Cmux Swift Actor Isolation ✅ Passed PASS: The pull request changes only one SQL migration and three TypeScript files under web/. The authoritative diff contains no Swift files or Swift actor-isolation constructs, so it introduces no p…
Cmux Swift Blocking Runtime ✅ Passed PASS: The authoritative pull-request diff changes only SQL and TypeScript files. It contains no Swift changes, so it does not introduce or expand the specified Swift blocking or timing-based synchroni…
Cmux Browser Automation Off-Main ✅ Passed PASS. The pull request changes only web database schema, migration, CodeRouter repository logic, and a database behavior test. It does not modify cmux browser socket automation commands, `Sources/Term…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only four web files. The authoritative diff contains no Swift, Xcode, agent-history, or interactive-path changes, so the expensive synchronous Swift load condition does …
Cmux Cache Substitution Correctness ✅ Passed The check does not apply. The production diff is additive and does not replace a fresh read with a cache. issueVmAuthorizationToken reads the VM, pool, and current team-visible native and Claude acc…
Cmux No Hacky Sleeps ✅ Passed The PR introduces no fixed sleeps, timers, polling, delayed dispatch, or wall-clock waits. The production change uses a database transaction and conflict marker to initialize VM pool grants. The test …
Cmux Algorithmic Complexity ✅ Passed No algorithmic-complexity violation is introduced. The production change uses three set-based SQL statements, not nested application loops or in-memory joins. Each initialization query targets one VM …
Cmux Swift Concurrency ✅ Passed PASS: The authoritative PR diff changes only four web TypeScript/SQL files. It contains no Swift files and no Swift concurrency patterns, so the cmux Swift concurrency check is not applicable.
Cmux Swift @Concurrent ✅ Passed The pull request changes only SQL and TypeScript files. The authoritative diff contains no Swift files, Swift functions, or Swift call sites. Therefore it does not introduce or change any behavior cov…
Cmux Swift Package Boundaries ✅ Passed The pull request changes only SQL and TypeScript files. The authoritative diff contains no Swift, SwiftPM, Xcode project, or workspace changes, so the Swift package-boundary check is not applicable.
Cmux Swiftpm Lockfiles ✅ Passed PASS. The authoritative PR diff contains only four web database/service/test files: one SQL migration, one TypeScript schema change, one TypeScript repository change, and one TypeScript test. It chang…
Cmux Swift Logging ✅ Passed The pull request changes only SQL and TypeScript files. It adds no Swift or other production Swift logging changes, and the changed-file patch contains no prohibited Swift logging calls.
Cmux User-Facing Error Privacy ✅ Passed PASS — The production diff adds only database schema, migration, and internal pool-initialization logic. It adds no user-facing error, alert, command output, API error body, or recovery copy. The new …
Cmux Full Internationalization ✅ Passed PASS. The PR changes only a database migration/schema, CodeRouter pool initialization logic, and a database behavior test. It adds no Swift text, web UI copy, API response text, metadata copy, rendere…
Cmux Swiftui State Layout ✅ Passed The review-scoped diff changes only SQL and TypeScript files. It contains no Swift or SwiftUI changes, so the SwiftUI state/layout failure conditions do not apply.
Cmux Architecture Rethink ✅ Passed The custom check applies to Swift architectural changes. The authoritative PR diff changes only four web files: a SQL migration, web/db/schema.ts, web/services/coderouter/repository.ts, and a data…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The pull request changes only SQL and TypeScript files. The authoritative diff contains no Swift changes and introduces no NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGroup c…
Cmux Source Artifacts ✅ Passed PASS: The PR changes only an intentional database migration, schema declaration, CodeRouter service source, and database regression test. The new migration is under web/db/migrations/..., and the ot…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request changes only four web files. It contains no Swift file under a production Sources/ path, so this check is not applicable.
Title check ✅ Passed The title clearly and concisely describes the primary change: initializing Cloud VM account pools for CodeRouter.
Description check ✅ Passed The description provides the problem, resulting behavior, implementation scope, security boundary, regression coverage, validation results, and changelog entry. It uses equivalent sections such as Imp…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread web/tests/coderouter-vm-scope-db-behavior.test.ts
@austinywang

Copy link
Copy Markdown
Contributor Author

Review audit

Head audited: 4730d22703290415bbc55e3f0035f174bd6d3afd.

Source Finding / comment Disposition Evidence
Cubic inline thread discussion_r4152622263 Regression test did not prove Claude-family initialization or private Codex exclusion. Accepted and fixed. Added both assertions in d3b66d3a7295020b3c5c63dbaf687b0868d75ee1; replied in-thread; thread resolved. Current DB behavior suite passed in the manual web-validation run.
Independent correctness review (/root/review_pool_init) Existing pools needed initialization markers before the new runtime path, or first post-deploy refresh could recreate revoked grants. Accepted and fixed. Added the existing-pool marker backfill in 4730d22703290415bbc55e3f0035f174bd6d3afd. Migration checks and current web CI passed.
CodeRabbit status comment Review-in-progress/status metadata only; no actionable finding was posted. No action required. Monitored through closeout. No actionable review thread/comment remains.
CLA Assistant comment Contributor/CLA confirmation only. No action required. CLA check passed.

All actionable review findings are addressed or resolved. No merge or controller build is requested for this web-only change.

@austinywang
austinywang merged commit 6aa6343 into main Oct 1, 2026
75 checks passed
@austinywang
austinywang deleted the 16389-cloud-coderouter-accounts branch October 1, 2026 08:18
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Merge receipt for 4730d22703: every check was green at merge (18 verified; 19 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Oct 1, 2026
6aa6343 fix(coderouter): initialize Cloud VM account pools (manaflow-ai#16397)
lawrencecchen added a commit that referenced this pull request Oct 1, 2026
…16405)

Production outage: every Cloud VM create fails with model_plane_unavailable
since 2026-10-01 08:44 UTC. #16397 inserts into coderouter_pool_initializations,
but its migration 20261001000000 was never applied to staging or production.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@lawrencecchen

Copy link
Copy Markdown
Contributor

Reverted by #16405 to stop a production outage: every Cloud VM create failed with model_plane_unavailable from 2026-10-01 08:44 UTC (30 of 30 creates), because migration 20261001000000_coderouter_vm_pool_initialization was never applied to staging or production. To re-land: apply that migration to staging, then production, and only then merge the code again.

lawrencecchen added a commit that referenced this pull request Oct 2, 2026
…#16572)

Reverts #16405. #16397 was reverted because its migration
20261001000000_coderouter_vm_pool_initialization was never applied to
staging or production, so every Cloud VM create failed. This re-land
merges only after that migration is applied to staging and production.

Co-authored-by: austinywang <austinywang@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
rustybret pushed a commit to rustybret/bmux that referenced this pull request Oct 2, 2026
0bfd027 test(cloud): fix the Cloud header and moved-panel focus tests that never ran (manaflow-ai#16539)
c5c4345 localization: accept numbered placeholders in any order (manaflow-ai#16376)
456edeb fix(settings): replace custom sidebar mockups with real previews (manaflow-ai#16569)
98dc3ab Prototype: cmux Cloud as a remote MCP server (manaflow-ai#16568)
6c22525 test(remote): isolate tmux stale-surface fixture (manaflow-ai#16566)
3ec9918 Re-land "fix(coderouter): initialize Cloud VM account pools (manaflow-ai#16397)" (manaflow-ai#16572)
2b895a5 Fix browser paste routing with terminal text box beta (manaflow-ai#6380) (manaflow-ai#16560)
2bd3455 localization: check Swift defaultValue literals against their catalog en value (manaflow-ai#16396)
c43086e test(cli): expect --mark-read to mark every listed inbox message (manaflow-ai#16537)
fcda4f0 test(feed): wait for zero-wait Codex permission acceptance before checking attention (manaflow-ai#16536)
7d57a03 fix(remote): evict stale persistent SSH bridge leases (manaflow-ai#16558)
d630cb8 docs: add protected-folder diagnostics for tmux sessions (manaflow-ai#12219)
7dceaac test: create cwd fixtures that new terminals now resolve on disk (manaflow-ai#16538)
28cc575 docs: cover surface resume binding CLI contract (manaflow-ai#16473)
5c7dca1 Fix idle zsh PR probes triggering chpwd hooks (manaflow-ai#16553)

# Conflicts:
#	.github/workflows/ci-guards.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cloud CodeRouter org switch leaves Codex with no usable account

2 participants