Repository navigation
[EPIC-U][LOU-U17] Sandboxed HTTP honors cancellation - #72
Merged
Merged
Conversation
sandboxHttpFetch now accepts an optional AbortSignal: an already-aborted signal rejects with an AbortError without running anything, an abort mid-flight rejects promptly, and the signal is forwarded to SandboxRunOptions.signal (NoopSandbox passes it to execFile so the child is killed). http.ts's sandbox transport and slack.ts's execute and sandboxExecute paths now pass the tool's abortSignal through. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016bY29ibQgv3MDdjAbUT4YR
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.
What
sandboxHttpFetchtakes an optionalsignal(SandboxFetchOptions.signal). An already-aborted signal rejects with anAbortErrorwithout callingsandbox.run(). An abort mid-flight rejects promptly with anAbortError. The signal is also forwarded tosandbox.run().SandboxRunOptionsgains an optionalsignal.NoopSandboxpasses it toexecFile, so the sandboxed child process is killed on abort.http.ts: the sandbox transport now forwardsinit.signal. The caller's signal was already linked into that controller, but the transport dropped it.slack.ts:executeandsandboxExecutenow pass the tool'sabortSignaltopostSlackAlertandpostSlackAlertViaSandbox, which hand it tofetchandsandboxHttpFetch.Why
After LOU-V1, an aborted run left the sandboxed HTTP request (a
node -esubprocess run throughSandboxAdapter.run) running, becausesandboxHttpFetchignored cancellation. There is no in-process undici request on the sandbox path. The signal is therefore raced againstrun()so it rejects promptly with any adapter, and it is also passed down viaSandboxRunOptions.signalso adapters that support it can kill the process.Acceptance criteria
sandboxHttpFetchaccepts an optionalsignaland rejects with anAbortError, including when the signal is already abortedabortSignalthrough; the non-sandbox paths do too (http already did, slack now does)run()options. No real network.src/execution/*untouchedHow verified
npx tsc --noEmit: cleannpm run lint: 0 errors, no new warnings from this change (the 3 warnings from my test file were fixed; the rest are pre-existing)npm run test:types: no errorsnpx vitest run: 133 files passed, 1960 passed | 3 skipped (afternpm run build; the importGraph tests need dist)npm run build && npm run build --workspace=packages/create-loushy-agent: successnpm run test:coverage: 1960 passed | 3 skippednpm run fallow: exit 0, no issuesNot done:
SubprocessSandbox(Docker) ignoressignal. Abort still rejects promptly at thesandboxHttpFetchlevel, but the container keeps running until it finishes or times out. This can be a follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_016bY29ibQgv3MDdjAbUT4YR
Generated by Claude Code