[oakimov]

Problem

A TERM, INT, HUP, or QUIT sent only to the pid of scripts/isolated-check.sh or scripts/isolate-run.sh (what a tool runner's kill <pid> does) ran the script's EXIT trap, which removed the archive or stage, while the sandboxed command kept running: reparented, with network, and on the --live path with CURSOR_API_KEY. Neither script signalled its child, and the namespace PID 1 forwards only signals it receives itself. forwards termination to the command session signals the whole process group, which reaches the boundary directly, so it did not catch this.

Fix

• scripts/isolate-run.sh: isolate_supervise, ported from oakimov/opencode-plugin-compat@c78395c. The Linux unshare call and the macOS sandbox-exec call both go through it. The boundary runs as its own session from a background job, with stdin kept and INT/QUIT restored. TERM, INT, HUP, and QUIT are forwarded to that session; a session still running ISOLATE_TERM_GRACE seconds later (default 10) is killed, and anything left in it is killed before the staging files are removed. A cancelled run exits 128 plus the signal number. The file follows the reference except for one header sentence on SIGKILL and the stop sequence in the next item.
• scripts/isolate-run.sh, beyond the reference: a trap that bash takes between two commands does not end the wait that follows it. A signal trapped before isolate_supervise had stored the leader's pid, or just before its wait started, was recorded, and the script then waited for the command to end by itself: forever for a command that ignores TERM. Nothing was removed under the running command, but the cancellation never finished. The trap now runs the stop sequence itself (isolate_sup_stop): send the signal once the session exists, kill the session after the grace period, return when the leader has gone. isolate_supervise calls it once more for a signal trapped before the leader was known. The same boundaries in check_supervise already exit 143 at once.
• scripts/isolated-check.sh: one wrapper, check_supervise, starts every boundary. check_run_isolated and check_run_live (the call that passes the key) both use it. It runs isolate-run.sh in the background, passes a cancellation on as TERM, waits until it has exited, then exits 128 plus the signal number so the EXIT trap removes the archive last.
• scripts/live-test.sh: it already stopped its OpenCode process before removing its files on TERM, INT, and HUP. QUIT left that process running (bash 3.2 dies without running the EXIT trap, bash 5 keeps waiting), so QUIT is now trapped like INT and TERM.

No provider code changes: src/ and dist/ are untouched, so prompt caching and both OpenCode majors are unaffected.

Verification

Each new test signals only the script's pid and checks with ps that the command is gone once the script has exited.

Test	Unfixed scripts	With this change
isolate-run.sh kills a command that ignores TERM before removing its stage	fail: command still running	pass
isolate-run.sh forwards TERM so the command can stop itself	fail: command still running	pass
isolated-check.sh waits for the boundary before removing the archive on SIGTERM	fail: command still running	pass
isolated-check.sh waits for the boundary before removing the archive on SIGINT	fail: the script never exits	pass
isolated-check.sh waits for the live boundary, which holds the key, before removing the archive	fail: command still running	pass
removes plugin-list files and stops OpenCode when the live test receives SIGQUIT	fail: OpenCode stand-in still running	pass
isolate-run.sh stops a command when TERM is trapped before the session leader is known	fail: the script never exits	pass
isolate-run.sh stops a command when TERM is trapped just before the wait starts	fail: the script never exits	pass

The same before and after on both platforms; for the last two rows, "unfixed" is the first commit of this PR. The live-boundary case runs the real check_run_live against a stand-in live-test.sh and a placeholder key. The last two cases take the trap at an exact command boundary: a BASH_ENV hook sets a DEBUG trap that sends TERM to the script just before the named command.

• macOS 15 (sandbox-exec, bash 3.2 and 5.3): bun run typecheck, bun run build, then bun test ./test three times in a row: 3414 pass, 5 skip (the Linux-only cases), 0 fail each time, with no stored Cursor credential visible to the tests. No marker process left in ps -A -o args= afterwards.
• Linux (the test-harness image with the runner's own container options: uid 10001, no capabilities, seccomp, apparmor and systempaths unconfined; util-linux 2.41.5, bash 5.2): test/review-scripts.test.ts three times in a row: 54 pass, 0 fail, 0 skip each time, no marker process left.
• Re-checked on that Linux image: unshare --fork stays alive after TERM to its pid, the namespace PID 1 is in its process group, and a group SIGKILL ends the namespace.

Fixture fix in test/review-eval.test.ts

One full macOS run in six failed in only the settling runners leave no owned process or workspace behind; the transfer keeps its service: the term runner returned 1 with its workspace in place instead of 143. The fixture's descendant appended to heartbeat in the workspace every 50 ms and opened the file with create, so a write between the runner's unlink and rmdir brought the file back and the removal failed with ENOTEMPTY. The descendant now creates the file once, before it reports ready, and appends without creating. With a writer that does not pause, the term runner failed 15 of 15 runs before and none of 15 after. It is a separate commit.

Other scripts

• scripts/test-harness.py defers INT and TERM to checkpoints and settles the check's process group before it removes the snapshot. It removes nothing on any other signal.
• scripts/review-publish.py, scripts/complete-resolutions.py, scripts/review-eval.py, the install and configure scripts, and the TypeScript generators start no background child and have no exit handler that removes files.

Limits, now documented in docs/pr-review.md

• SIGKILL cannot be forwarded. Killing either script that way can leave the boundary running.
• macOS has no pid namespace: a process that starts its own session inside sandbox-exec is not stopped with the boundary. On Linux the namespace ends with its PID 1.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous startup-cancellation issue is fixed.

Summary

The PR stops sandboxed commands before removing their staging files or archive when cancellation targets only the script’s pid.

• Isolation runs stop their session before staging files are removed.
• Check runs wait for their isolation boundary before removing the archive.
• The live test stops OpenCode when it receives QUIT.
• The review fixture keeps its heartbeat file from returning during cleanup.

The previous startup-cancellation finding is fixed. No new actionable issues were found. Tests were not run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Cancellation reaches script pid] --> B[Forward signal to boundary]
  B --> C{Boundary stops within grace period?}
  C -->|No| D[Kill boundary process group]
  C -->|Yes| E[Wait for boundary exit]
  D --> E
  E --> F[Remove staging files and archive]
  F --> G[Exit with cancellation status]

Loading

_{Reviews (2) · Last reviewed commit: "Stop the review fixture's writer from br..." · Reviewed by Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!