fix(cloud-agent-next): decide worktree deletion exclusivity from a local reference index

[eshurakov]

Problem

Deleting a Cloud Agent worktree failed every time for an account with many root sessions:

INTERNAL_SERVER_ERROR  cliSessionsV2.deleteWorktree
"Worktree deletion is incomplete; retry the same worktree"

The message is a catch-all. Axiom production logs show the real cause — both attempts ended at stage runtime_cleanup after 30127 ms / 30126 ms:

[do-retry] Non-retryable error
  { operation: deleteWorktreeResources, attempt: 1,
    error: "Worktree ownership lookup timed out", retryable: false }

Root cause (verified from logs and read-only SQL)

SandboxControl.fenceAndCheckWorktreeExclusivity resolved sandbox exclusivity by calling SESSION_INGEST.canDestroyCloudAgentWorktreeSandbox and then issuing one serial getRuntimeLocation() DO RPC per owner the ledger could not locate, with no per-call timeout, inside withTimeout(..., DEADLINE_MS.stopAttempt) = 30 s.

Measured for the affected user: 540 root sessions, 522 of them with no worktree, while the DB side of the check is ~44 ms. 522 × ~57 ms ≈ 30 s, so the timeout always fired and withRetry never retried it (a plain timeout carries no retryable flag). The walk is unbounded and account-wide.

Refuted: a missing index made the ownership SQL slow. The plans are already 44 ms — the cost was RPC fan-out.

Change

The SandboxControl DO for a sandbox now owns a durable per-session reference index for it, and exclusivity is decided from local state:

• NEW sandbox-control/session-references.ts — the index domain and the single hasForeignReference predicate answering "is this evidence foreign to the target worktree?".
• durable-state.ts — the session_references key, a strict schema (512 entries / 96 KiB, per-field maxima), absent → empty, malformed → throws, erased with the sandbox record.
• SandboxControl.ts — attachSession records a reference atomically with its route; detachSession keeps the removed route as a tombstone (so a detached session still blocks destruction); the worktree prune clears detached entries; fenceAndCheckWorktreeExclusivity decides from references ∪ routes, with no withTimeout; new forgetSessionReference RPC.
• sandbox-control/worktree-ownership.ts — the serial walk becomes reconcileSandboxReferences: one 20 s budget over the ledger RPC plus a concurrency-8 locator pool, preserving owner-ordered verdicts, returning an incomplete verdict instead of throwing.
• sandbox-session/control-rpc.ts, sandbox-session/SandboxSession.ts — deadline-scoped best-effort forget on session delete; failure does not block the user-visible deletion.

Pre-deployment references that cannot be derived from the index are covered by one bounded reconciliation per sandbox, gated by a persisted marker. Design invariants that keep the verdict safe:

• a ledger shared verdict never certifies completeness (it carries no evidence, so it blocks only the current decision);
• only a completed walk with no foreign reference and no missing history sets the marker;
• unavailable history stays recoverable, and capacity overflow fails closed.

Verification

Focused gates on the merged tree:

Check	Result
pnpm -C services/cloud-agent-next run typecheck	exit 0
unit (session-references, durable-state, worktree-ownership, lifecycle, session-message-queue)	653 passed
test/integration/worktree-deletion.test.ts (real workerd/Miniflare DO SQLite)	28 passed
run lint	0 warnings, 0 errors
run format:check	clean

End-to-end, against the real Durable Object under workerd: the diagnostic shows decision: exclusive with evidence: runtime_reconciliation / ledger, session_locator 0 and timed out 0; and all three directions hold — foreign session attached ⇒ no stop, foreign session detached (tombstone) ⇒ no stop, no foreign reference ⇒ destroyed. Worktree ownership lookup timed out and resolveSandboxExclusivity no longer exist anywhere in the repo.

Not verified against a running production system: the live deleteWorktree success, the real Postgres ledger and real container destruction. Deleting a real worktree is a destructive production action and was not run; the production-scale timing (~522 owners, ~57 ms) is projected from the measured latency, not re-measured.

Accepted trade-offs and follow-ups

• The "no per-session RPCs" property holds in steady state. The first deletion on a sandbox whose marker is unset still performs one reconciliation over the ledger's owner list — now bounded and concurrency-8 (~3.8 s projected at the measured latency instead of a 30 s serial timeout), degrading to "not exclusive" rather than throwing. Later deletions are purely local.
• A session that is registered but never attached no longer blocks destruction; recovery is runtime recreation, and control attach is awaited before the workspace is materialised.
• A pre-deployment detached reference whose ledger and locator evidence are both gone yields unavailable ⇒ block (fail safe, but that sandbox is not destroyable by worktree deletion until evidence reappears).
• Capacity overflow is terminal per sandbox until the record is erased by another path.
• Follow-up, deliberately out of scope: observability (retryable classification and per-owner resolution logs) and the client-side catch-all at worktree-deletion.ts:221-232, which still hides the real cause for other errors.

Follow-up commit: do not walk owners that cannot be control-plane

The first commit bounds the walk but still visits every owner the ledger cannot locate — for the affected account, ~522 of ~540 roots are legacy agent_* sessions with no worktree. The second commit stops consulting those through their session DO, because a legacy session cannot hold a control-plane sandbox id: isolated ids hash the session id, shared ids derive from a plane-versioned route key (shared-v3 vs shared-control-v1), the failover override only appends a fixed suffix to the same route key, and the only cross-session copy of a workspace sandbox stays inside one worktree.

Those owners still block when the ledger has already recorded an allocation naming the target sandbox and provider. Every owner that could be control-plane — all worktree owners and all workspace_* owners — keeps its previous lookup, owner-ordered verdict and unavailable behaviour unchanged.

Accepted trade, recorded deliberately: the legacy locator channel is withdrawn. A legacy session that referenced the target sandbox without a matching recorded allocation no longer blocks, so that workspace could be destroyed. Current normal routing paths do not create that cross-plane assignment, and persisted assignments are not universally provenance-validated, so historical or atypical records are not proven absent. Existing tests that asserted the withdrawn channel are re-pointed at control-plane owners, and the recorded-allocation blocker keeps its own regression test.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of the follow-up commit (61a7beea, on top of e9d0d771) covering the bounded reconciliation follow-up that skips runtime locator lookups for owners that cannot be control-plane; no high-confidence defects were found in the changed code.

Verification notes specific to this increment:

• sessionPlaneFromId(...) === 'control' in lookupSessionRuntimeLocator is behavior-preserving versus the prior startsWith('workspace_') (see session-plane.ts:11).
• The new ownerNeedsRuntimeLookup gate returns foreign when a no-worktree owner's recorded allocationLocation names the target sandbox and provider, and located otherwise; worktree owners and owners with any workspace_* session keep the previous owner-ordered lookup and unavailable behavior.
• The withdrawn legacy locator channel is a deliberate, explicitly documented trade (the PR records it and re-points the affected tests), so it is reported as intended behavior, not a defect.
• The premise is consistent with sandbox-id.ts:397-406: isolated ids hash the session id and shared route keys embed a plane-versioned segment (shared-v3 vs shared-control-v1), so a legacy agent_* session cannot hold the control-plane sandbox id.
• The increment adds no timers, pools, or listeners, so no memory-leak exposure is introduced; test-harness cleanup for legacy sessions was removable because those tests now use control-plane sessions.

Files Reviewed (3 files)

• services/cloud-agent-next/src/sandbox-control/worktree-ownership.ts
• services/cloud-agent-next/src/sandbox-control/worktree-ownership.test.ts
• services/cloud-agent-next/test/integration/worktree-deletion.test.ts

Previous Review Summary (commit e9d0d77)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit e9d0d77)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the durable session-reference index, the bounded reconcileSandboxReferences rewrite, and their SandboxControl/SandboxSession integration, plus the added unit and workerd integration tests; no high-confidence defects were found.

Verification notes:

• Exclusivity now fails closed: overflowed forces a foreign verdict, malformed persisted state throws, budget/history exhaustion returns an incomplete verdict, and every non-exclusive early return releases exclusiveDeletionWorktreeId.
• Verdict precedence is owner-ordered and cannot be reordered by completion order; deadline exhaustion is detected through withDORetry's TimeoutError and cannot be mistaken for an identity mismatch (and vice versa).
• No new leaks: withCallDeadline clears its timer in finally, withDORetry disposes its scope/listeners, and the reconciliation state (outcomes, cursor pool) is bounded per call and erased with the sandbox record.
• The two fence calls in one runWorktreeDeletion can each run a reconciliation while reconciled is false; this stays bounded (~concurrency-8 pool, 5s per-call cap, 20s budget) and matches the documented trade-off.

Files Reviewed (13 files)

• services/cloud-agent-next/src/persistence/SandboxControl.ts
• services/cloud-agent-next/src/sandbox-control/durable-state.ts
• services/cloud-agent-next/src/sandbox-control/durable-state.test.ts
• services/cloud-agent-next/src/sandbox-control/session-references.ts
• services/cloud-agent-next/src/sandbox-control/session-references.test.ts
• services/cloud-agent-next/src/sandbox-control/worktree-ownership.ts
• services/cloud-agent-next/src/sandbox-control/worktree-ownership.test.ts
• services/cloud-agent-next/src/sandbox-control/lifecycle.test.ts
• services/cloud-agent-next/src/sandbox-session/SandboxSession.ts
• services/cloud-agent-next/src/sandbox-session/control-rpc.ts
• services/cloud-agent-next/src/sandbox-session/session-fixture.test-helpers.ts
• services/cloud-agent-next/src/sandbox-session/session-message-queue.test.ts
• services/cloud-agent-next/test/integration/worktree-deletion.test.ts

---

_{Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0}

_{Review guidance: REVIEW.md from base branch main}