Skip to content

fix(sandbox): emit gone for orphaned thread sandboxes on shared scope - #5219

Merged
pedrofrxncx merged 1 commit into
mainfrom
fix/sandbox-events-shared-thread-stale-handle
Jul 24, 2026
Merged

fix(sandbox): emit gone for orphaned thread sandboxes on shared scope#5219
pedrofrxncx merged 1 commit into
mainfrom
fix/sandbox-events-shared-thread-stale-handle

Conversation

@pedrofrxncx

@pedrofrxncx pedrofrxncx commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Opening a thread whose sandbox has been reaped leaves the Preview pinned on "Reserving sandbox" forever. Reproduced in prod on fila/0ced73dc… — reproduces for the thread owner too, so this is not the read-only/teammate gate.

Reading the SSE stream live from the page:

GET /api/fila/sandbox/decopilot_…/thread%3A0ced73dc…%2Fconn_u3zc…/events
→ event: phase  data: {"kind":"claiming","since":…}
   …then nothing, forever

claiming maps to CLAIM_PHASE_COPY.claiming = "Reserving sandbox". After 90s (NO_CLAIM_MAX_MS) emitLifecycle gives up silently (settle(false), no gone), the stream ends, EventSource reconnects, gets claiming again.

Root cause

providerKind === "agent-sandbox"sharedSandboxIdscope === "shared", and that branch resolved vmEntry only from agent_sandbox_sessions. A thread-scoped sandbox (load_repo) is recorded on the thread while agent-sandbox truth lives in the session registry (resolve-provider.ts:152"Legacy hosted metadata is deliberately ignored"), so a reap leaves the thread entry orphaned with no session row to reconcile it.

Verified in prod PG for this thread:

  • threads.metadata.sandboxMap[owner][branch]["agent-sandbox"].sandboxHandle = conn-u3zc-li4kbjlk1xzqlm-d6f02ea54cdba440
  • agent_sandbox_sessionsno row for the branch
  • no sandboxclaim in the cluster for that handle (reaped)

So vmEntry came back null, the stale-handle probe at the vmEntry?.sandboxHandle === claimName gate was skipped, and gone was never emitted. The gate would have fired — computeClaimHandle yields exactly the handle sitting in thread metadata. This is precisely the loop the file header (:85-89) says the thread fallback exists to prevent; that fallback was only reachable from the user-scoped else branch.

Three client recovery paths all key off state this never produced, which is why nothing recovers (no SANDBOX_START in the network log across a full page load):

path gate why it never fires
auto-start !vmEntry orphan entry grafted from thread metadata ⇒ vmEntry non-null
self-heal events.notFound set only by gone
overlay dismissal resolvePreviewDisplay needs progressStatus !== "doing" claimPhase: "claiming" keeps it "doing"

Changes

  • Fall back to the thread sandboxMap in the shared branch when there is no session row at all — a session mid-transition (provisioning/stopping) is live state the thread map must not override.
  • resolveSharedThreadVm scans every user key. A shared claim handle derives from the projectRef and carries no userId, so the entry may sit under the thread owner's key when a teammate opens the thread; keying on the viewer would skip the probe and loop on claiming.
  • Clear the thread's sandboxMap entry in cleanupStaleEntry regardless of scope. The shared reap path returns early when there's no session row, which would leave the dangling handle in metadata and make every SSE reconnect re-enter the stale path (the 404 flood the existing comment warns about).

After this, the reaped handle probes dead → gonenotFound → self-heal fires SANDBOX_START → fresh sandbox.

Testing

  • apps/api/src/api/routes/sandbox-events-handler.test.ts — 4 unit tests on the pure resolveSharedThreadVm, including the teammate-key case that caused the regression. Pure logic, no mocks (tier 1 per TESTING.md).
  • bun run fmt, bun run --cwd apps/api check (tsc clean), bun run lint (0 errors; 14 pre-existing warnings, none in changed files).
  • bun test apps/api/src/api apps/api/src/tools/sandbox apps/api/src/sandbox: 112 fail with the change vs 113 fail on the same tree without it — all pre-existing integration tests needing real Postgres/NATS, none related.

Follow-ups (not in this PR)

  • load-repo.ts:217 claims "ensureSandbox already persisted the sandbox record on the thread (provisionSandbox → setThreadSandboxMapEntry — the single writer)". False for agent-sandboxstart.ts:749 only calls it in the non-shared else branch. It then streams a data-open-preview carrying an agent-sandbox sandboxMap the server deliberately ignores, which chat-context.tsx:945 patches into the client thread row. That mismatch is what mints these orphans.
  • emitLifecycle's 90s no-claim timeout ends the stream silently, making "no claim ever appeared" indistinguishable from "still waiting". Worth considering gone there as a backstop.

Summary by cubic

Fixes the preview hanging on “Reserving sandbox” for shared agent-sandbox threads by detecting orphaned entries, emitting gone, and letting the client self-heal with SANDBOX_START.

  • Bug Fixes
    • In shared scope, when no agent_sandbox_sessions row exists, fall back to the thread sandboxMap to probe the stale handle and emit gone.
    • Scan all user keys via resolveSharedThreadVm so teammates opening a thread can detect the stale shared handle.
    • Always clear the thread sandboxMap entry during cleanup to prevent reconnect loops and 404 floods.
    • Added unit tests for resolveSharedThreadVm covering the teammate-key scenario.

Written for commit abdf017. Summary will update on new commits.

Review in cubic

Opening a thread whose sandbox was reaped left the Preview pinned on
"Reserving sandbox" forever: the events SSE emitted `phase: claiming` and
never advanced, so the client had nothing to self-heal from.

`agent-sandbox` always resolves to `scope: "shared"`, and that branch
resolved `vmEntry` only from `agent_sandbox_sessions`. A thread-scoped
sandbox (load_repo) is recorded on the thread while agent-sandbox truth
lives in the session registry, so a reap leaves the thread entry orphaned
with no session row to reconcile it. `vmEntry` came back null, the
stale-handle probe at the `vmEntry?.sandboxHandle === claimName` gate was
skipped, and `gone` was never emitted — exactly the loop the file header
says the thread fallback exists to prevent, except that fallback was
only reachable from the user-scoped `else` branch.

Three client recovery paths all key off state this never produced:
auto-start needs `!vmEntry`, self-heal needs `notFound` (set only by
`gone`), and `resolvePreviewDisplay` keeps the blocking overlay up while
`claimPhase` reads `claiming`.

- Fall back to the thread sandboxMap in the shared branch when there is
  no session row at all (a session mid-transition is live state the
  thread map must not override).
- Scan every user key via `resolveSharedThreadVm`: a shared claim handle
  derives from the projectRef and carries no userId, so the entry may sit
  under the thread owner's key when a teammate opens the thread.
- Clear the thread's sandboxMap entry in `cleanupStaleEntry` regardless
  of scope. The shared reap path returns early with no session row, which
  would leave the dangling handle in metadata and make every SSE
  reconnect re-enter the stale path.
@pedrofrxncx
pedrofrxncx merged commit bd19aa4 into main Jul 24, 2026
15 checks passed
@pedrofrxncx
pedrofrxncx deleted the fix/sandbox-events-shared-thread-stale-handle branch July 24, 2026 18:41
decocms Bot pushed a commit that referenced this pull request Jul 24, 2026
PR: #5219 fix(sandbox): emit `gone` for orphaned thread sandboxes on shared scope
Bump type: patch

- decocms (apps/api/package.json): 4.125.1 -> 4.125.2

Deploy-Scope: server
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