fix(runtime): terminate stray sandbox processes between iterations - #6753
Conversation
PR Summary by QodoTerminate stray sandbox processes between runtime iterations
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
|
🤖 Finished Review · ✅ Success · Started 4:22 PM UTC · Completed 4:39 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.72 |
Site previewPreview: https://810e5229-site.fullsend-ai.workers.dev Commit: |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
Risk Assessment: elevated (3/5) DetailsAnchored to prior score 3/elevated — all signals essentially unchanged. Tier 1 identical: 22 files, 1299 lines, large blast radius, CI workflow change. Tier 2 confirms extreme churn in core files (run.go 85/30d with 14 authors, run_test.go 46/12, sandbox.go 26/8). Offset by decent test ratio (0.32), 4 net-new files with clean history, additive design, and non-first-time author. Previous runRisk Assessment: elevated (3/5) DetailsAnchored to prior score 3/elevated — all signals essentially unchanged. Tier 1 signals identical: 19 files, ~1257 lines, large blast radius, CI workflow change. Tier 2 confirms extreme churn in core files (run.go 85 commits/30d with 14 authors, sandbox.go 26/8, run_test.go 46/12). Partially offset by decent test ratio (0.32), 4 net-new files with no prior history, additive design, and non-first-time author. Previous run (2)Risk Assessment: elevated (3/5) DetailsAnchored to prior score 3/elevated — all signals unchanged. Large blast radius (19 files, 1231 lines) and CI workflow change drive Tier 1 upward despite no protected/security-sensitive paths. Tier 2 reveals extreme churn in core files: run.go (86 commits/30d, 14 authors, 139 fix/reverts), sandbox.go (26/9/43). Partially offset by decent test ratio (0.32), additive design, and non-first-time author. Previous run (3)Risk Assessment: elevated (3/5) DetailsElevated risk driven by Tier 2 git history: runtime and sandbox packages remain under intense active development with very high churn (15 authors, 54% fix/revert rate in 90 days). The PR introduces a new subsystem (stray process termination) in this high-churn area. Large blast radius (18 files across runtime, sandbox, cli, docs, CI) and CI workflow change add Tier 1 weight. Partially offset by decent test ratio (0.33), no protected or security-sensitive paths, additive/rollback-safe design, and well-scoped implementation. Anchored to prior score 3/elevated — signals unchanged. Previous run (4)Risk Assessment: elevated (3/5) DetailsElevated risk driven by Tier 2 git history: runtime and sandbox packages are under intense active development with high churn, 12 distinct authors in 90 days, and frequent fix/revert commits. The PR introduces a new subsystem (stray process termination, 183-line new file plus shell test harness) in this high-churn area. CI workflow change (Makefile) adds Tier 1 weight. Partially offset by good test ratio (0.31), no protected paths, and well-scoped change relative to the parent issue. |
ReviewFindingsLow
Previous runReviewFindingsLow
Next steps:
Previous run (2)ReviewFindingsLow
Next steps:
Previous run (3)Looks good to me Previous run (4)ReviewFindingsLow
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
ready-for-merge
6be9955 to
5a3d0ca
Compare
|
🤖 Finished Review · ✅ Success · Started 4:51 PM UTC · Completed 5:08 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.94 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
5a3d0ca to
d2aab23
Compare
|
🤖 Finished Review · ✅ Success · Started 6:22 PM UTC · Completed 7:04 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
removed
the
ready-for-merge
Reviewer asks (rh-hemartin) and the two remaining bot findings:
- docs/runtimes.md: the between-iterations paragraph is replaced by one
step in the sequence diagram ("clean up stray processes"); the page is
a runtime chooser, not a reference.
- docs/contributing/runtime-implementation.md: the interface table row
is the suggested short form; the ClearIterationArtifacts contract
(sweep first, then files; failed sweep = warning; held under the
runner's sandbox lock) is a paragraph under the table instead.
- "quiesce" is gone: sandboxQuiesceMu/withSandboxQuiesce are now
sandboxMu/withSandboxLock (+ acquireSandboxLock, sandboxLockWarnAfter,
sandboxLockPoll and the test names).
- withSandboxLock takes a context: a waiter whose run is shutting down
abandons the wait with ctx.Err() instead of sitting behind the
holder's in-flight sandbox exec (up to the ~45s hold budget). Both the
notify and the nil-notify paths poll TryLock now. New test
TestWithSandboxLock_AbandonsTheWaitWhenCancelled; runOIDCRefresh
already drops errors once its context is cancelled.
- sandbox.KeepAliveCommand moved into the file's const block.
Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
DummyPlaybackRuntime.ClearIterationArtifacts ran the file cleanup only, unlike ClaudeRuntime/PiRuntime/DummyRuntime — inconsistent with the documented per-iteration cleanup contract, since dummy-playback execs also run in the real sandbox. Add the same clearStrayProcesses sweep, with tests mirroring DummyRuntime's. Also add the sweep as a step in the validation-loop pseudocode in docs/guides/dev/cli-internals.md, which still showed the loop without it. Assisted-by: Claude (fix), Codex (review) Signed-off-by: Wayne Sun <gsun@redhat.com>
9b2fccd to
0af942b
Compare
|
🤖 Finished Review · ✅ Success · Started 9:00 PM UTC · Completed 9:36 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.46 |
Superseded by updated review
fullsend-ai-review
Bot
added
the
ready-for-merge
|
Compare with e.g. GitHub Agentic Workflows. There's no equivalent to this in an agent definition: A GH-AW agent step runs an agent to completion, then reports the result. There's hence no "process leakage from prior agent steps". Personally what I think would be much cleaner is tell the agent about the validation steps (in my projects I like However of course yes, one wants a deterministic check - and that's what classic CI is for. GH-AW also does allow intermixing regular actions steps and agent steps which can be used for this. (Though I think in the general case, it gets a bit complicated to synchronously wait for all action results currently with GH-AW; I'm still digging into that) |
|
🤖 Finished Retro · ✅ Success · Started 9:41 PM UTC · Completed 9:51 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.67 |
Retro: PR #6753 — Terminate stray sandbox processes between iterationsWorkflow overview: Human-authored PR by waynesun09 fixing stray processes surviving between sandbox iterations (refs #6464). 3 commits, 22 files, Timeline
What went well
Evidence for existing issues (no new proposals needed)
|
The stray-process sweep waited a fixed 2s between TERM and KILL. A maintainer objected on #6753 that this leaves an agent no room to flush state on SIGTERM, and that objection lands hardest on the codex steer path: ClearIterationArtifacts sweeps leftovers from a run that is already over, but codexSteerQueue.interrupt stops a process the runner intends to CONTINUE, one turn of a thread it is about to resume. The grace is now a parameter of the snippet (__GRACE_TICKS__ for the poll loop, __GRACE_LABEL__ for its own comment). ClearIterationArtifacts keeps 2s and renders byte-for-byte what it rendered before — testdata/kill_stray_processes.sh is unchanged, and its golden test still passes untouched. The codex interrupt gets 10s through interruptSweep, which is the default of the existing injectable `sweep:` field, so tests can still replace the whole sweep. The exec timeout scales with the grace rather than staying at a flat 15s. That bound exists to catch a hung gateway; left fixed, raising the grace would have meant the timeout fired during the TERM wait and the KILL pass never ran — the sweep would have started leaking exactly the processes it exists to remove. Default stays 15s (2s + 13s headroom), the interrupt gets 23s, and a test asserts the timeout always outlasts the grace with room to spare. The interrupt rendering is pinned in testdata/kill_stray_processes_interrupt.sh the same way the default one is, and kill_stray_processes_test.sh now takes an optional snippet path so either can be executed under a real shell. Both were run: the interrupt golden passes, taking ~26s against the default's ~16s because the TERM-ignoring fixture now takes its full 10s before the KILL lands — which is the evidence that the longer wait actually elapses and the KILL pass still works, rather than just that the string renders. A test also normalises the two renderings and asserts they differ ONLY in the grace, so the process-selection logic the sandbox depends on cannot fork. codex_steer.go now records why the grace differs there, alongside the note that every interrupt leaves a dangling tool call in the rollout (codex logs "Custom tool call output is missing" on the resume and tolerates it). Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The stray-process sweep waited a fixed 2s between TERM and KILL. A maintainer objected on #6753 that this leaves an agent no room to flush state on SIGTERM, and that objection lands hardest on the codex steer path: ClearIterationArtifacts sweeps leftovers from a run that is already over, but codexSteerQueue.interrupt stops a process the runner intends to CONTINUE, one turn of a thread it is about to resume. The grace is now a parameter of the snippet (__GRACE_TICKS__ for the poll loop, __GRACE_LABEL__ for its own comment). ClearIterationArtifacts keeps 2s and renders byte-for-byte what it rendered before — testdata/kill_stray_processes.sh is unchanged, and its golden test still passes untouched. The codex interrupt gets 10s through interruptSweep, which is the default of the existing injectable `sweep:` field, so tests can still replace the whole sweep. The exec timeout scales with the grace rather than staying at a flat 15s. That bound exists to catch a hung gateway; left fixed, raising the grace would have meant the timeout fired during the TERM wait and the KILL pass never ran — the sweep would have started leaking exactly the processes it exists to remove. Default stays 15s (2s + 13s headroom), the interrupt gets 23s, and a test asserts the timeout always outlasts the grace with room to spare. The interrupt rendering is pinned in testdata/kill_stray_processes_interrupt.sh the same way the default one is, and kill_stray_processes_test.sh now takes an optional snippet path so either can be executed under a real shell. Both were run: the interrupt golden passes, taking ~26s against the default's ~16s because the TERM-ignoring fixture now takes its full 10s before the KILL lands — which is the evidence that the longer wait actually elapses and the KILL pass still works, rather than just that the string renders. A test also normalises the two renderings and asserts they differ ONLY in the grace, so the process-selection logic the sandbox depends on cannot fork. codex_steer.go now records why the grace differs there, alongside the note that every interrupt leaves a dangling tool call in the rollout (codex logs "Custom tool call output is missing" on the resume and tolerates it). Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The stray-process sweep waited a fixed 2s between TERM and KILL. A maintainer objected on #6753 that this leaves an agent no room to flush state on SIGTERM, and that objection lands hardest on the codex steer path: ClearIterationArtifacts sweeps leftovers from a run that is already over, but codexSteerQueue.interrupt stops a process the runner intends to CONTINUE, one turn of a thread it is about to resume. The grace is now a parameter of the snippet (__GRACE_TICKS__ for the poll loop, __GRACE_LABEL__ for its own comment). ClearIterationArtifacts keeps 2s and renders byte-for-byte what it rendered before — testdata/kill_stray_processes.sh is unchanged, and its golden test still passes untouched. The codex interrupt gets 10s through interruptSweep, which is the default of the existing injectable `sweep:` field, so tests can still replace the whole sweep. The exec timeout scales with the grace rather than staying at a flat 15s. That bound exists to catch a hung gateway; left fixed, raising the grace would have meant the timeout fired during the TERM wait and the KILL pass never ran — the sweep would have started leaking exactly the processes it exists to remove. Default stays 15s (2s + 13s headroom), the interrupt gets 23s, and a test asserts the timeout always outlasts the grace with room to spare. The interrupt rendering is pinned in testdata/kill_stray_processes_interrupt.sh the same way the default one is, and kill_stray_processes_test.sh now takes an optional snippet path so either can be executed under a real shell. Both were run: the interrupt golden passes, taking ~26s against the default's ~16s because the TERM-ignoring fixture now takes its full 10s before the KILL lands — which is the evidence that the longer wait actually elapses and the KILL pass still works, rather than just that the string renders. A test also normalises the two renderings and asserts they differ ONLY in the grace, so the process-selection logic the sandbox depends on cannot fork. codex_steer.go now records why the grace differs there, alongside the note that every interrupt leaves a dangling tool call in the rollout (codex logs "Custom tool call output is missing" on the resume and tolerates it). Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The stray-process sweep waited a fixed 2s between TERM and KILL. A maintainer objected on #6753 that this leaves an agent no room to flush state on SIGTERM, and that objection lands hardest on the codex steer path: ClearIterationArtifacts sweeps leftovers from a run that is already over, but codexSteerQueue.interrupt stops a process the runner intends to CONTINUE, one turn of a thread it is about to resume. The grace is now a parameter of the snippet (__GRACE_TICKS__ for the poll loop, __GRACE_LABEL__ for its own comment). ClearIterationArtifacts keeps 2s and renders byte-for-byte what it rendered before — testdata/kill_stray_processes.sh is unchanged, and its golden test still passes untouched. The codex interrupt gets 10s through interruptSweep, which is the default of the existing injectable `sweep:` field, so tests can still replace the whole sweep. The exec timeout scales with the grace rather than staying at a flat 15s. That bound exists to catch a hung gateway; left fixed, raising the grace would have meant the timeout fired during the TERM wait and the KILL pass never ran — the sweep would have started leaking exactly the processes it exists to remove. Default stays 15s (2s + 13s headroom), the interrupt gets 23s, and a test asserts the timeout always outlasts the grace with room to spare. The interrupt rendering is pinned in testdata/kill_stray_processes_interrupt.sh the same way the default one is, and kill_stray_processes_test.sh now takes an optional snippet path so either can be executed under a real shell. Both were run: the interrupt golden passes, taking ~26s against the default's ~16s because the TERM-ignoring fixture now takes its full 10s before the KILL lands — which is the evidence that the longer wait actually elapses and the KILL pass still works, rather than just that the string renders. A test also normalises the two renderings and asserts they differ ONLY in the grace, so the process-selection logic the sandbox depends on cannot fork. codex_steer.go now records why the grace differs there, alongside the note that every interrupt leaves a dangling tool call in the rollout (codex logs "Custom tool call output is missing" on the resume and tolerates it). Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The stray-process sweep waited a fixed 2s between TERM and KILL. A maintainer objected on #6753 that this leaves an agent no room to flush state on SIGTERM, and that objection lands hardest on the codex steer path: ClearIterationArtifacts sweeps leftovers from a run that is already over, but codexSteerQueue.interrupt stops a process the runner intends to CONTINUE, one turn of a thread it is about to resume. The grace is now a parameter of the snippet (__GRACE_TICKS__ for the poll loop, __GRACE_LABEL__ for its own comment). ClearIterationArtifacts keeps 2s and renders byte-for-byte what it rendered before — testdata/kill_stray_processes.sh is unchanged, and its golden test still passes untouched. The codex interrupt gets 10s through interruptSweep, which is the default of the existing injectable `sweep:` field, so tests can still replace the whole sweep. The exec timeout scales with the grace rather than staying at a flat 15s. That bound exists to catch a hung gateway; left fixed, raising the grace would have meant the timeout fired during the TERM wait and the KILL pass never ran — the sweep would have started leaking exactly the processes it exists to remove. Default stays 15s (2s + 13s headroom), the interrupt gets 23s, and a test asserts the timeout always outlasts the grace with room to spare. The interrupt rendering is pinned in testdata/kill_stray_processes_interrupt.sh the same way the default one is, and kill_stray_processes_test.sh now takes an optional snippet path so either can be executed under a real shell. Both were run: the interrupt golden passes, taking ~26s against the default's ~16s because the TERM-ignoring fixture now takes its full 10s before the KILL lands — which is the evidence that the longer wait actually elapses and the KILL pass still works, rather than just that the string renders. A test also normalises the two renderings and asserts they differ ONLY in the grace, so the process-selection logic the sandbox depends on cannot fork. codex_steer.go now records why the grace differs there, alongside the note that every interrupt leaves a dangling tool call in the rollout (codex logs "Custom tool call output is missing" on the resume and tolerates it). Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The stray-process sweep waited a fixed 2s between TERM and KILL. A maintainer objected on #6753 that this leaves an agent no room to flush state on SIGTERM, and that objection lands hardest on the codex steer path: ClearIterationArtifacts sweeps leftovers from a run that is already over, but codexSteerQueue.interrupt stops a process the runner intends to CONTINUE, one turn of a thread it is about to resume. The grace is now a parameter of the snippet (__GRACE_TICKS__ for the poll loop, __GRACE_LABEL__ for its own comment). ClearIterationArtifacts keeps 2s and renders byte-for-byte what it rendered before — testdata/kill_stray_processes.sh is unchanged, and its golden test still passes untouched. The codex interrupt gets 10s through interruptSweep, which is the default of the existing injectable `sweep:` field, so tests can still replace the whole sweep. The exec timeout scales with the grace rather than staying at a flat 15s. That bound exists to catch a hung gateway; left fixed, raising the grace would have meant the timeout fired during the TERM wait and the KILL pass never ran — the sweep would have started leaking exactly the processes it exists to remove. Default stays 15s (2s + 13s headroom), the interrupt gets 23s, and a test asserts the timeout always outlasts the grace with room to spare. The interrupt rendering is pinned in testdata/kill_stray_processes_interrupt.sh the same way the default one is, and kill_stray_processes_test.sh now takes an optional snippet path so either can be executed under a real shell. Both were run: the interrupt golden passes, taking ~26s against the default's ~16s because the TERM-ignoring fixture now takes its full 10s before the KILL lands — which is the evidence that the longer wait actually elapses and the KILL pass still works, rather than just that the string renders. A test also normalises the two renderings and asserts they differ ONLY in the grace, so the process-selection logic the sandbox depends on cannot fork. codex_steer.go now records why the grace differs there, alongside the note that every interrupt leaves a dangling tool call in the rollout (codex logs "Custom tool call output is missing" on the resume and tolerates it). Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
The stray-process sweep waited a fixed 2s between TERM and KILL. A maintainer objected on #6753 that this leaves an agent no room to flush state on SIGTERM, and that objection lands hardest on the codex steer path: ClearIterationArtifacts sweeps leftovers from a run that is already over, but codexSteerQueue.interrupt stops a process the runner intends to CONTINUE, one turn of a thread it is about to resume. The grace is now a parameter of the snippet (__GRACE_TICKS__ for the poll loop, __GRACE_LABEL__ for its own comment). ClearIterationArtifacts keeps 2s and renders byte-for-byte what it rendered before — testdata/kill_stray_processes.sh is unchanged, and its golden test still passes untouched. The codex interrupt gets 10s through interruptSweep, which is the default of the existing injectable `sweep:` field, so tests can still replace the whole sweep. The exec timeout scales with the grace rather than staying at a flat 15s. That bound exists to catch a hung gateway; left fixed, raising the grace would have meant the timeout fired during the TERM wait and the KILL pass never ran — the sweep would have started leaking exactly the processes it exists to remove. Default stays 15s (2s + 13s headroom), the interrupt gets 23s, and a test asserts the timeout always outlasts the grace with room to spare. The interrupt rendering is pinned in testdata/kill_stray_processes_interrupt.sh the same way the default one is, and kill_stray_processes_test.sh now takes an optional snippet path so either can be executed under a real shell. Both were run: the interrupt golden passes, taking ~26s against the default's ~16s because the TERM-ignoring fixture now takes its full 10s before the KILL lands — which is the evidence that the longer wait actually elapses and the KILL pass still works, rather than just that the string renders. A test also normalises the two renderings and asserts they differ ONLY in the grace, so the process-selection logic the sandbox depends on cannot fork. codex_steer.go now records why the grace differs there, alongside the note that every interrupt leaves a dangling tool call in the rollout (codex logs "Custom tool call output is missing" on the resume and tolerates it). Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
3 participants
Summary
A tool command such as
nohup python3 -c 'time.sleep(300)' &started by the agent's bash tool survives the agent's normal exit inside the sandbox: pi's built-inbashkills its process group only on abort/timeout, and Claude Code's Bash behaves the same. fullsend reuses the sandbox for the next validation-retry iteration, andClearIterationArtifactsonlyrm -rf'd files — so strays from iteration 1 kept running into iteration 2 (holding files open, eating CPU/memory, writing into the workspace the next iteration reads). Verified empirically on 2026-08-29 while evaluating pi sub-agents: three backgrounded commands were still alive, reparented to PID 1, after the agent process exited.Change: a runtime-neutral sweep (
internal/runtime/stray_processes.go) that everyClearIterationArtifacts(Claude Code, pi, dummy) runs before the file cleanup. The POSIX-sh snippet (golden-pinned intestdata/kill_stray_processes.sh) kills every process of the sandbox user except the exec shell and its ancestors (the channel back to the runner), its own helpers, zombies, and the sandbox keep-alive main process (sleep infinity, now a shared constantsandbox.KeepAliveCommandused bycreateOnce); TERM first, KILL survivors after 2 s; warn-only, never fails the iteration. Uses onlyps/awk/kill/sleep/id(all in the sandbox image; nopkill/pgrep). With the podman driver the OpenShell supervisor is PID 1 as root (crates/openshell-driver-podman/src/container.rsuser: "0:0"), outside the sandbox user's process view, and the ancestry walk covers other drivers; the keep-alive (observedps -o args==sleep infinity, ppid 1) is spared by its argv (path-qualified first token tolerated) because killing it makes the sandbox terminal on OpenShell 0.0.111+ — a consequence is that an agent-started literalsleep infinityis spared too. The sweep is serialized against the credential refreshers through the runner's sandbox lock (sandboxMu, held acrossClearIterationArtifacts, the OIDC token upload and the OpenAI auth seed), so a refresh can no longer be killed mid-upload; apsfailure inside the sweep exits 3 and surfaces as a warning instead of a silent "0 killed".Test plan
go test ./internal/runtime/... ./internal/sandbox/...— only the two knownTestDummyRuntime_*failures that occur whenever a local OpenShell gateway is running; patch coverage: script builder 100%,killStrayProcesses91.7%,clearStrayProcesses100%, all threeClearIterationArtifacts100%internal/runtime/kill_stray_processes_test.sh(registered inmake script-test): plain stray exits 143, TERM-ignoring stray exits 137, the test shell survives, a second sweep kills 0make lintclean (shellcheck on the golden)localhost/fullsend-sandbox, OpenShell 0.0.116, same create flags ascreateOnce):nohup sleep 300 &, a TERM-ignoringsleep 300andpython3 -c 'time.sleep(300)'survived the exec; the sweep reportedstray processes killed: 3in 2 s, onlysleep infinityremained, the sandbox stayedReady, a follow-up exec worked, a second sweep reported 0docs/runtimes.mdgets one step in the "How a run uses the runtime" diagram;docs/contributing/runtime-implementation.mdstates theClearIterationArtifactscontract (sweep first, then files; failed sweep = warning) under the interface tablepsfailure → exit 3 + warning; batched liveness probe (oneps -pper tick); keep-alive match tolerant of a path-qualifiedsleep;TestKeepAliveCommandMatchesSweepExclusion; shell test asserts the fakepsis first on PATH; pi fail-open test; sweep duration logged; refresher/sweep serialization with tests (TestRefreshOIDCToken_WaitsForSandboxLock,TestReseedOpenAIAuth_WaitsForSandboxLock); the changed snippet re-verified live (3 strays killed, keep-alive spared, zombie skipped, sandbox stayed Ready)Refs #6464 (found during the pi runtime extension evaluation).
Review round 2 (Claude + Grok, clean of HIGH/MEDIUM after fixes): the sandbox lock now taken through a panic-safe
withSandboxLockhelper at all four sites; the OpenAI seed holds it only around the atomicauthSeedexec; the iteration loop reports when it has waited >5 s for the lock; the hold budget (≈45 s worst case vs the 4-minute OIDC tick) is documented next to the mutex; the liveness probe'sps -pfailure is now detected (KILL the full TERM'd list, thenps -p failed+ exit 3) with a shell-test case; comments corrected (/bin/bash -cupload chain, full 4-minute token window,id -u); OpenCode stub annotated; Claude-runtime fail-open test; Makefile help.go test -raceon the lock/refresh tests, 11/11 shell cases,make lintclean.Review round 3 (
9b2fccd3, rebased on main after fix(pi): activate grep/find/ls and ship rg + fd in the sandbox image #6752): reviewer asks —docs/runtimes.mdparagraph replaced by a diagram step, interface table row shortened with the contract moved to a paragraph under the table, "quiesce" renamed to the sandbox lock (sandboxMu/withSandboxLock); bot lows —withSandboxLocktakes a context and abandons the wait when the run is cancelled (TestWithSandboxLock_AbandonsTheWaitWhenCancelled),sandbox.KeepAliveCommandmoved into the const block.go test ./internal/cli/...green,-raceon the lock tests,make lintclean.Review round 4 (
0af942bfd, rebased on main after docs: clarify mint role vs identity so custom agents work by default #6772): bot lows —DummyPlaybackRuntime.ClearIterationArtifactsnow runs theclearStrayProcessessweep before the file cleanup, matchingClaudeRuntime/PiRuntime/DummyRuntime(new tests:TestDummyPlaybackRuntime_ClearIterationArtifacts_SweepsStraysBeforeFiles,_SweepFailureIsNotAnError);docs/guides/dev/cli-internals.md's validation-loop pseudocode now shows the sweep as a step between iterations.go build/targeted tests/make lint-allclean.