test(cli): one owner for Process.Wait in the interrupt test

[dennisonbertram]

Refs #1422 — deliberately not "Closes". The Linux failure is not reproduced, so this should not be auto-closed by a merge.

What is proven

TestGoCodeScriptStopsHarnessdOnInterrupt called cmd.Process.Wait() twice — once in the deferred cleanup, once in the goroutine feeding the timeout select:

defer func() {
    _ = cmd.Process.Kill()
    _, _ = cmd.Process.Wait()   // second waiter
}()
...
go func() { _, _ = cmd.Process.Wait(); close(waitDone) }()

os.Process.Wait is not safe to call twice on the same process. That is a real defect, I introduced it in #1417, and it is the kind of thing that behaves differently across platforms and under the race detector. Fixed: the deferred cleanup now only kills, and Wait has one owner.

Also: the stub daemon orphaned its sleep child when bash was terminated. It now traps TERM and takes the child with it.

What was disproven, and why that matters

I filed #1422 with a confident diagnosis. It was wrong, and so was my second one. Both took under a minute to disprove by measuring instead of reasoning.

"The stub forces a 5-second force-kill path every run, leaving no headroom." Timed the wrapper's exit after a process-group SIGINT, driving it exactly as the test does:

wrapper exited 0.21s after SIGINT

A non-interactive bash does not defer SIGTERM behind a foreground sleep; it terminates. The graceful-shutdown wait is never reached.

"The stub doesn't really survive SIGINT, so the test passes for the wrong reason." Checked directly:

stub daemon survived group SIGINT: True

POSIX inherits an ignored disposition across exec, so trap '' INT makes the sleep ignore INT too. The premise is sound and the trap stays.

The issue body has been corrected and the title updated, so nobody implements against the version I got wrong.

What is still unknown

The Linux CI root cause is unconfirmed. Not reproduced on macOS: 8/8 under -race with four CPU hogs running, 3/3 idle.

So the widened budget (10s to 30s) is honest headroom for a loaded runner, not a fix justified by a timing story — the old 10s was itself chosen against the story I have now disproven. This PR removes a genuine concurrency defect and reduces fragility. It may or may not be what was failing.

#1422 stays open until CI shows a clean run without a re-run. A green that needed a re-run proves nothing, which is the same trap that let this reach main in the first place.

Verification

go test ./cmd/harnesscli -run TestGoCodeScript -race -count=2 — all seven tests, green. Exactly one cmd.Process.Wait() call remains in the file.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.