Skip to content

fix(sandbox): stop trusting legacy agent-sandbox entries on the agent row - #5225

Merged
pedrofrxncx merged 1 commit into
mainfrom
fix/client-ignores-legacy-agent-sandbox-map
Jul 24, 2026
Merged

fix(sandbox): stop trusting legacy agent-sandbox entries on the agent row#5225
pedrofrxncx merged 1 commit into
mainfrom
fix/client-ignores-legacy-agent-sandbox-map

Conversation

@pedrofrxncx

@pedrofrxncx pedrofrxncx commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

A preview hangs for exactly one member while a teammate opening the same thread provisions normally. Reported on clovis/3934190e…: stuck for @Tavano (the thread's own author), instant for another member. Tavano's read — "acho que o meu browser não rodou SANDBOX_START" — was exactly right.

Root cause

The agent row is not authoritative for agent-sandbox:

  • Server resolves that kind exclusively from agent_sandbox_sessions and deliberately ignores metadata.sandboxMapreadRecordedKinds, apps/api/src/sandbox/resolve-provider.ts:151-155.
  • Nothing writes it there anymore — SANDBOX_START only calls setSandboxMapEntry in the non-shared else branch (start.ts:749).

Every such entry is a leftover from before the kind became org-shared. The client still read them (agent-shell-layout/index.tsx passed entity?.metadata?.sandboxMap straight into parseBranchMap(sandboxMap[userId][branch])).

Evidence from prod for branch tavano-vgfdyxrm:

source handle live?
connections.metadata.sandboxMap[tavano][tavano-vgfdyxrm]["agent-sandbox"] tavano-vgfdyxrm-77c242b4e71f6488 404 — reaped
agent_sandbox_sessions tavano-vgfdyxrm-5e9d9abaf97aabe0 200 — live

The session row's created_at is the moment the other member opened the thread, with their id as created_by — it did not exist while Tavano was stuck.

A leftover entry makes vmEntry non-null for whichever user key holds it, so shouldAutoStart returns false (!args.vmEntry) and no SANDBOX_START is ever issued for that member. Meanwhile previewUrl points at the reaped sandbox and there is nothing to self-heal from: the direct-daemon probe only treats HTTP 404 as gone (isDirectDaemonEventsGoneStatus), and a dead hostname throws and is swallowed to false. A member whose key is simply absent from the map gets vmEntry === null, hits the ownership gate, acknowledges, and provisions. The asymmetry is decided purely by which user key the leftover happens to sit under.

Changes

  • withoutLegacyAgentSandboxEntries — strips agent-sandbox from the agent-row sandboxMap in VmEventsBridge, leaving the session registry as the only source for that kind. Mirrors the server rule.
  • Also strips the legacy cluster spelling, which parseBranchMap normalizes to agent-sandbox and would otherwise reintroduce the bug through the back door. (0 rows in prod today, and storage normalizes on read — cheap guard against a row slipping through.)
  • Returns the input reference unchanged when there's nothing to strip.

Behaviour-neutral otherwise: when a session row exists, its graft in VmEventsBridge already overwrites the stripped entry. With no session row, vmEntry goes null → auto-start fires → sandbox provisions, which is what should have happened for Tavano.

The thread-map graft is deliberately left alone — that one is a live feature (read-only view of a teammate's running sandbox), not legacy.

Scale

agent-sandbox sandboxMap entries with no matching session row:
  1789 entries · 320 agents · 112 users

Each was a latent "preview never starts, for one member, on one branch." This makes them inert — no data migration needed.

Relationship to #5219

Same root disease (a client trusting agent-sandbox state the server declared dead), different store. #5219 covers a leftover on the thread row and is gated on threadIdFromBranch(branch), which needs a thread: prefix. "tavano-vgfdyxrm" returns null, so #5219 does not help here. The two are independent and can merge in either order.

Testing

  • apps/web/src/lib/agent-sandbox-map.test.ts — 6 unit tests: the exact regression, the cluster spelling, user-desktop siblings preserved, sibling branches/users untouched, reference identity, undefined passthrough. Pure logic, no mocks (tier 1 per TESTING.md).
  • bun run fmt, bun run --cwd apps/web check (tsc clean), bun run lint (0 errors; 14 pre-existing warnings, none in changed files).
  • bun test apps/web/src/components/sandbox apps/web/src/lib: 574 pass / 6 fail — the same 6 fail on the same tree without this change (clearPersistedQueryCache, readCachedOrg/writeCachedOrg; localStorage readonly-property issues in the bun env).

Note

This particular thread already healed itself: the other member's open created the session row, whose graft now overwrites the stale entry. It will not reproduce there anymore — the 1789 other entries are where the remaining risk is.


Summary by cubic

Stops trusting legacy agent-row agent-sandbox entries so previews start reliably for everyone. The client now resolves agent-sandbox only from the session registry, preventing stale map entries from blocking auto-start or pointing to reaped sandboxes.

  • Bug Fixes
    • Filter agent-sandbox and legacy cluster keys out of the agent-row sandboxMap via withoutLegacyAgentSandboxEntries in VmEventsBridge.
    • Keep the thread sandbox map unchanged (it’s a live read-only view).
    • Behavior: if no session row exists, auto-start fires; if a session exists, its graft stays authoritative.
    • Makes 1,789 stale entries inert; no migration needed.

Written for commit 30f440d. Summary will update on new commits.

Review in cubic

… row

A preview could hang for exactly one member while a teammate opening the
same thread provisioned normally — "works for me, stuck for you".

The agent row is not authoritative for `agent-sandbox`. The server resolves
that kind exclusively from the org-scoped session registry and deliberately
ignores `metadata.sandboxMap` (`readRecordedKinds` in resolve-provider.ts),
and nothing writes it there anymore — `SANDBOX_START` only calls
`setSandboxMapEntry` for non-shared kinds. Every such entry is a leftover
from before the kind became org-shared. The client still read them.

A leftover entry makes `vmEntry` non-null for whichever user key happens to
hold it, so `shouldAutoStart` returns false and no SANDBOX_START is ever
issued for that member, while `previewUrl` points at a reaped sandbox so
there is nothing to self-heal from either: the direct-daemon probe only
treats HTTP 404 as `gone`, and a dead hostname throws and is swallowed. A
member whose key is absent from the same map gets `vmEntry === null` and
provisions normally — hence the asymmetry, which is decided purely by which
user key the leftover sits under.

Filter agent-sandbox out of the agent-row sandboxMap in VmEventsBridge,
leaving the session registry as the only source for that kind. Also strips
the legacy `cluster` spelling, which `parseBranchMap` normalizes to
agent-sandbox and would otherwise reintroduce the bug.

Behaviour-neutral otherwise: when a session row exists its graft already
overwrites the stripped entry. The thread-map graft is deliberately left
alone — that one is a live feature (read-only view of a teammate's running
sandbox), not legacy.

Makes the 1789 such entries currently in production inert, so no data
migration is needed.
@pedrofrxncx
pedrofrxncx merged commit 5ffec8a into main Jul 24, 2026
14 checks passed
@pedrofrxncx
pedrofrxncx deleted the fix/client-ignores-legacy-agent-sandbox-map branch July 24, 2026 19:21
pedrofrxncx pushed a commit that referenced this pull request Jul 27, 2026
#5240)

Reverts #5112 (share agent sandboxes by branch), #5116 (always share
agent sandboxes) and #5132 (use shared staging branch), restoring the
pre-#5112 behaviour: hosted agent sandboxes are per-user again, and a
new GitHub-backed thread gets a generated branch instead of `staging`.

Reconstructed against current paths rather than reverted: #5120 split
apps/mesh into apps/api + apps/web and hoisted branch-name.ts,
runtime-defaults.ts and thread/schema.ts into packages/shared, and #5174
deleted vm-events.ts outright, so none of the three commits reverse-apply.

Core change: SandboxId goes back to a flat `{ userId, projectRef }` and
sandboxIdKey back to `userId:projectRef`, so every hosted claim handle
changes. Live shared claims must be drained BEFORE this deploys.

Also reverted, because they only exist inside the shared model:
- #5225 withoutLegacyAgentSandboxEntries — left in place it would strip
  the sandboxMap entries the reverted server writes back, leaving
  shouldAutoStart permanently true (silent, compiles fine)
- #5219 resolveSharedThreadVm and the shared-scope branch
- #5183 stale-provisioning reset (lived inside the deleted storage)

Deliberately preserved:
- migrations 137/138 and their index.ts entries — Kysely's
  #ensureNoMissingMigrations runs unconditionally, so deleting an applied
  migration crash-loops every pod. The tables are dropped by a follow-up
  forward migration after the drain, not by this commit.
- #5114/#5118 withClaimGitLock, #5126 org-context guard, #5143 analytics
  gate, #5171 owner-keyed thread graft, #5195/#5207/#5210/#5221 client
  auto-retry, the daemon status-code train and the settings-validation train
- #5112's org-fs least-privilege narrowing and the start dedup key fix
- #5116's squashed queue-tray pluralization (en + pt-br)
- the remote-branch-name validations #5132 deleted, since this restores
  the dynamic origin/HEAD lookup that made them load-bearing

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Pedro França <pedrofrxncx@deco.cx>
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.

1 participant