feat(py): exec-shaped backend seam for the execution worker - #244
Draft
jat255 wants to merge 4 commits into
Draft
Conversation
The first piece of M6 (kata f11e). Everything the execution driver needs from a process host is one call, so hosting the worker somewhere else -- a Connect container, most likely -- becomes another implementation of ExecBackend rather than an edit to the driver above it. Three behaviours here are the ones Inspect's subprocess utilities got right and are easy to get wrong: Input goes in on stdin, never as an argument, so there is no escaping to mishandle and no command-line length limit. stdin is written without a drain: a worker that dies during startup leaves nobody reading the pipe, and that is a failed call to report rather than a BrokenPipeError out of the plumbing. Output past the cap keeps the tail and lets the process finish. Killing on the cap would discard a result the code had already computed, and simply not reading would deadlock the child against a full pipe. The head is what gets dropped, since the result is usually last. Shutdown escalates rather than going straight to SIGKILL, so a worker that handles SIGTERM gets to clean up. After SIGKILL the wait is bounded: the child watcher can miss an exit, and a killed process is gone whether or not we observe it go. env replaces the parent's environment rather than extending it. The allowlist that decides what belongs in it is a separate task; this is only the mechanism that makes an allowlist possible at all. Tests drive real subprocesses. The one stand-in is for the missed-exit race, which cannot be provoked on demand.
Draining stdout and stderr ends at end-of-file, which a process can reach while still running: closing both streams and carrying on defeated the deadline entirely, and the call then waited on process.wait() with no bound at all. Both halves now sit inside the caller's deadline, so neither can outlast it. Found by review of 48934b2.
The driver cancels calls when a conversation goes away or the agent shuts down, and only the timeout path was ending the process. A cancelled call left the worker running: still holding the parent's file descriptors, still burning CPU, with nobody waiting on the result. Cancellation now goes through the same shutdown escalation as a timeout. The waiting happens inside an except block, where an await can be cut short, so there is a test covering a child that ignores SIGTERM to pin that SIGKILL still lands there.
The cleanup added in 650302f did its waiting inline in the except block, so a cancel landing during the SIGTERM grace period cut it short before SIGKILL and a child ignoring SIGTERM survived. One cancel was covered; two were not. Shutdown now runs as its own task, awaited through a shield, with the backend holding a reference so it cannot be collected mid-escalation. A second cancel stops us waiting on it, not the escalation itself. Found by review of 650302f.
jat255
marked this pull request as draft
September 2, 2026 00:18
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
1 participant
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.
First piece of M6's execution tool: the single call the driver will use to run the worker process, so that hosting the worker somewhere else later is a new implementation rather than a rewrite of the driver.
Summary
ExecBackendis the interface (exec(cmd, input, cwd, env, timeout)) andLocalBackendruns the worker as a child of this process with no isolation. Nothing consumes it yet; the worker, the protocol, and the sandbox are separate units of work.Three behaviours here are easy to get wrong, and each is pinned by a test. Input goes in on stdin rather than as an argument. Output past the cap keeps the tail and lets the process finish, because killing on the cap would discard a result the code had already computed, and not reading would deadlock the child against a full pipe. Shutdown escalates from SIGTERM to SIGKILL with bounded waits, and runs on timeout and on cancellation alike.
envreplaces the parent's environment rather than extending it. The allowlist that decides what belongs in it is a separate unit of work; this is only the mechanism that makes an allowlist possible.Review notes
The cancellation path is what deserves scrutiny. Shutdown runs as its own shielded task, with the backend holding a reference to it, so a caller that cancels twice cannot leave a SIGTERM-ignoring child alive. The reasoning is in a comment at the handler.
Tests drive real subprocesses. The one stand-in is for the missed-exit race, which cannot be provoked on demand.
Sandboxing is not in this PR, and is no longer deferred either. D3 was reversed, so M6 brings the worker sandbox to parity with
pkg-r. That is tracked as separate work and does not change anything here.Testing
uv run pytest(116 passed),uv run ruff check, anduv run pyrefly checkare clean inpkg-py/. Every commit was reviewed by roborev; two findings came back (the timeout not covering process exit, and a second cancellation aborting the shutdown escalation) and both were reproduced by a failing test before being fixed.