Repository navigation
fix(headless): reap the scoped command's output replay - #2146
Merged
Merged
Conversation
The scope wrapper captures a command's output to a file and replays it after the command exits, so a descendant outliving the command cannot hold the exec's stdout open. The replay was itself that hole. It writes to the caller's stdout, so it blocks in pipe_write once the caller stops reading, which is precisely what an agent-phase timeout does; and it sat outside every teardown handle, in neither the command's process group nor the wrapper PID, inheriting no scope marker. Teardown walked past it. The exec then never saw EOF. The cell hung with no progress until something killed the container, and the verifier never ran, so the cell produced no reward at all rather than a graded failure. Eleven cells were lost this way in one graded run, all on arms whose agent runs inside the container. Give the replay its own process group and record it beside the command's, under a name the scope-wide `*.pgid` sweep already reaps. That keeps the handle in the mechanism teardown already trusts on every platform, rather than the `/proc` marker sweep, which does not exist on macOS. The test reproduces the strand without Docker and asserts the replay actually blocked first, so it cannot decay into a tautology.
Three independent reviews caught the same hole: matching the process table by the stdout path alone also matches the wrapper, whose `bash -c` script text contains that path. The pre-cleanup guard then passes with no replay alive at all, which is a false PASS in the one direction that matters — the guard exists precisely so the test cannot decay into a tautology. Match the command itself instead, and guard `ps` alongside `bash`. Also correct the docstring. A container-level check showed the command's own process group does reap the replay under that topology, so the earlier claim that every cleanup loop walks past it was too strong. What holds is narrower and is what the fix rests on: the replay has no handle of its own, and whether anything catches it is an accident of where the shell left it.
Astro-Han
marked this pull request as ready for review
August 4, 2026 12:51
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.
Summary
The scope wrapper captures a command's output to a file and replays it after the command exits, so that a descendant outliving the command cannot hold the exec's stdout open. The replay itself is the hole: it writes to the caller's stdout, so it blocks in
pipe_writeonce the caller stops reading — precisely what an agent-phase timeout does — and teardown has no handle on it. It is not the wrapper PID and it carries no scope marker, so whether anything reaps it comes down to which process group the shell happened to leave it in.When nothing does, the exec never sees EOF. The cell hangs with no progress, and the verifier never runs, so the cell produces no reward at all rather than a graded failure.
This gives the replay its own process group and records it beside the command's, under a name the scope-wide
*.pgidsweep already reaps. The handle stays in the mechanism teardown already trusts on every platform, rather than the/procmarker sweep, which does not exist on macOS.Refs #1970
Verification
python3 packages/headless/harbor/tests/test_process_scope.py— 5 passed, 0 failed. Withprocess_scope.pyrestored frommain, the new test fails withteardown left 1 process(es) holding the exec's pipe, so it is red-first on the real defect.The test reproduces the strand without Docker: it runs the wrapper against a payload larger than a pipe buffer with a caller that never reads, asserts the replay is actually blocked, then runs both teardown signals and asserts nothing still holds the pipe. Draining the pipe to check for EOF would unblock the replay and hide the defect, so it reads the process table instead, matching the replay command itself rather than the stdout path — the path alone also matches the wrapper's own script text, which would let the guard pass with no replay alive.
packages/headless/harbor/tests/test_harness_compat.pyreports 5 pre-existing failures on this machine (ModuleNotFoundError: No module named 'harbor'); identical on a cleanmain.Not run: Biome lint/format and workspace typecheck, which do not cover Python. No TypeScript changed.
Root cause
Observed live on the benchmark host, mid-hang, in a graded cell:
The captured stdout was 20 MB against a 64 KiB pipe buffer. The agent process and the wrapper shell were both gone — teardown had reaped them — and
MAKA_HARBOR_COMMAND_SCOPEwas absent from the survivor's environ.The wrapper is shared with the host-side bridged tool execs, which are individually bounded and were not observed to strand.
Scope
A container-level check found that under that topology the command's own process group does reap the replay, so this change is not by itself sufficient to explain the production strand. In the run that stranded, the teardown exec was itself SIGKILLed partway through by Harbor's compose-command timeout (
process scope ... TERM cleanup failed: Command failed (exit -9)in the trial log), so it never completed its loops. That is a separate defect and is not addressed here.What this PR fixes is that the replay has no teardown handle of its own and survives on an accident of process-group placement. On a host with no
/procthe marker sweep is a no-op and nothing reaps it at all.Follow-up
&and theprintfleaves the same unhandled orphan. The command's own pgid has had the identical window since before this change, so the fix belongs at that shared layer rather than only on the replay. Raised independently by three reviewers, accepted as a tracked P2.