Repository navigation
fix(sandbox): review fixes for settle safety, steer vs stop, and steer streaming - #47
Merged
OmarAlex24 merged 9 commits intoNov 13, 2026
Merged
Conversation
`sandboxes.resolveSettle` deleted or kept without checking that the chat was still settled or that a decision still waited, so a stale phone card or an in-flight click could delete the disk of a chat the user had just un-settled. It now re-reads the record and the host thread and returns `resolved: false` without acting; web and mobile say nothing changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resume and stop wrote back the record they read when they started, so an un-settle during a resume brought "Keep sandbox?" back on an active chat, and a settle answered meanwhile lost its `settleHandled`. Only `setSettlePending` now writes the settle fields; every other record write in the provisioner keeps the ones stored at write time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep on the phone and in Settings left a running sandbox running and billing, while the web prompt's "Keep snapshot" stopped it first on the client. The host's `resolveSettle` keep now stops a running sandbox, a failed stop leaving the decision pending; the web prompt no longer stops it itself, and the phone card and Settings row say Keep stops it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A settle under "delete" checked only git state, so any settle of the mirror, auto-settle included, could delete a sandbox mid-task while its own thread, delegated tasks, or a sub-sandbox task still ran. Before the work check it now reads the host thread's runs and tasks and, through the bridge's link, the sandbox thread's (`SandboxMirror.activity`), and asks (`work`) when anything runs or the sandbox does not answer. The `work` copy now says its agents may still be working. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Relaunching over an abandoned sandbox deletes the old record, and the bridge then settles the chat's host thread. The settle service found the new record for the same thread and applied `onSettle` to it: it stopped the fresh sandbox, asked spuriously, or deleted a clean clone. A settle now applies only to a sandbox created before the thread's `settledAt`, both on the event and in `reconcile`. Domain events carry no command id, so the bridge's settle cannot be told apart by its `sandbox-deleted-settle` id here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lete A chat with no host mirror (launched before the bridge, or whose link failed) showed "will be deleted in 10 s" on settle, although nothing on the host follows it. Only a chat the host mirrors gets that notice now; any other gets the plain Settled notice and a note that its sandbox is left as it is, with a link to Settings → Sandboxes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In attach mode nothing in the baseline counted as sent and the cut-off was the message's `startedAt`, so an answer begun before a steer and updated after it was streamed again from its first character. Items that started before the message now count as sent at their length in the baseline read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The steer and stop tests followed `Deferred.await(started)` with a bare `Effect.yieldNow`. Neither needs it: `started` completes once the sandbox has pushed the run, and both pass without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…working The ACP adapter sent the bridge's steer `_meta` for every interrupt that asked for no runtime restart, which includes the detach behind force-stop and session replacement. The bridge then left the sandbox and its delegated tasks running after a force-stop. The interrupt input now has an explicit `steer`, set only where a steer restarts the run (`interruptAndAwaitTerminal`); every other interrupt cancels plainly. Other ACP providers set no `steerCancelMeta` and are unaffected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
OmarAlex24
changed the title
OmarAlex24
marked this pull request as ready for review
November 13, 2026 13:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
An independent review of this stack (#33-#46) found nine defects. The serious ones could delete, stop or keep running a sandbox against the user's choice:
Change
One commit per finding, each with a test that failed on the previous code:
sandboxes.resolveSettleacts only while a decision is pending and the host thread is still settled; otherwise it changes nothing and returnsresolved: false, and every client says so.settlePendingandsettleHandledstored at write time; onlysetSettlePendingchanges them.SandboxMirror.activity); live work, or a sandbox that does not answer, asks with reasonwork.settledAt, both on the event and inreconcile.session/cancelis sent only when the interrupt carries an explicitsteerflag, set where a steer restarts the run; detach and force-stop are Stop.Effect.yieldNow.Scope and approval
Requested by the fork owner in a T3 Code session. Part of stack pingdotgg#43 on
feat/sandbox-environments.Verification
SandboxSettle.test.ts,SandboxProvisioner.test.ts,bridge.test.ts,AcpRegistryAdapterV2.test.tswith a new detach case, the coordinator's notice tests)vp run check:changedagainst feat: usage compares local and sandbox chats week by week #46's branch passes format, lint, knip, typecheck (38 packages), 284 tests in 11 files and react compilerresolveSettle's result type gainsresolved. Not yet run in a real client.Model and harness: Claude Opus 5.5 (review and implementation subagents at high effort, coordinated by Claude Opus 5.5) in Claude Code, running in T3 Code.
🤖 Generated with Claude Code