fix(signals): Ctrl-C and SIGTERM do not stop the loop

[Sofoklis-byte]

Summary

Ctrl-C does not stop Ralph. It prints Ralph loop interrupted. Cleaning up..., then the loop continues to the next iteration. Repeated Ctrl-C does nothing at all, and SIGTERM does not stop it either. In the run that surfaced this, only kill -9 ended the process — about seven minutes after the first stop attempt.

The reassuring log line is the dangerous part: an operator watching an autonomous loop believes it has stopped when it has not.

Root cause

trap cleanup SIGINT SIGTERM installs a handler that returns instead of exiting. cleanup() ends with:

# No exit here — EXIT trap handles natural termination

There is no EXIT trap. That trap line is the only one in the file. The comment describes a handler that does not exist, so nothing ever terminates the process, and bash resumes the loop as soon as the handler returns.

Four further faults sit behind that one:

	Fault
1	cleanup() returns; the signal handler therefore never terminates.
2	The EXIT trap the comment relies on does not exist anywhere in the file.
3	The _CLEANUP_DONE reentrancy guard — documented as covering an "EXIT + signal combination" that cannot occur — makes the second and every later signal a silent no-op. This is why repeated Ctrl-C cannot recover.
4	cleanup() reads $? as trap_exit_code. In a trap handler that is the last completed command's status, not 128+n, so the interrupted status is recorded only when the preceding command happened to fail. Under --dry-run it never prints.
5	claude is backgrounded and its pid is a local claude_pid, invisible to the handler, so a SIGTERM sent to the script leaves the child running.

Fix

A dedicated on_signal() replaces the bare trap:

1. restores default dispositions first, so a second Ctrl-C always kills even if teardown hangs;
2. stops the backgrounded child through a new CLAUDE_CHILD_PID — this leg proved insufficient and is corrected by 28cf3ca; see Follow-up below;
3. runs cleanup() for teardown, unchanged in meaning;
4. re-raises the signal, so the process dies of the signal with the correct 128+n status rather than falling out of a loop — anything supervising Ralph now sees a real interrupt;
5. sets _INTERRUPTED, which makes the interrupted-status record deterministic rather than dependent on the previous command's exit code.

cleanup() keeps its single-run guard and its existing behaviour. Normal (unsignalled) operation is unchanged.

Tests

tests/integration/test_signal_handling.bats — five tests, each verified to fail against the unpatched loop and pass against the fix. All run under --dry-run, so they make no API calls.

SIGTERM terminates the loop (exit 143)
SIGINT terminates the loop (exit 130)
repeated signals do not disarm the handler
the bare `trap cleanup SIGINT SIGTERM` form is gone
the child pid is reachable from the handler

Two harness notes are recorded in the test file, because both produced false results before they were understood:

• A background job started from a non-interactive shell inherits SIGINT as SIG_IGN, and bash cannot re-trap an inherited-ignored signal. A naive harness shows the loop surviving Ctrl-C whether or not the bug is present — it proves nothing. The tests reset SIGINT to default before exec, as an interactive terminal does.
• wait is used rather than a kill -0 poll, because a dead-but-unreaped child still answers kill -0. A watchdog bounds the wait so a hung loop cannot wedge the suite.

A sixth test was written and dropped: it asserted that no further iteration is logged after the interrupted line, but per fault 4 that line is not printed under --dry-run, so the assertion matched nothing and passed against the buggy code. It is mentioned here so it is not re-added later in good faith.

Follow-up — the child-kill leg was insufficient (28cf3ca)

The original fix reached the child pid, and the tests above assert exactly that: the child pid is reachable from the handler. Reaching it turned out not to be enough. In production the operator pressed Ctrl-C, got the prompt back, and the agent wrote a file 2s later, committed 17s later, and finished 47s after the stop, billing the call in full.

Measured mid-work under a real pty:

loop      pid 94095  pgid 94095   <- foreground group; receives the Ctrl-C
bash      pid 94100  pgid 94095   <- $! captured THIS, not the agent
gtimeout  pid 94107  pgid 94107   <- gtimeout setpgid()s ITSELF
claude    pid 94112  pgid 94107   <- the agent, in that other group

portable_timeout is a shell function, so backgrounding it forks a subshell; bash sometimes replaces that subshell with an exec of gtimeout and sometimes does not, varying with execution context. $! therefore does not reliably name the agent. Two consequences:

• Ctrl-C signals only the foreground process group, so the agent never sees it.
• TERMing $! kills the subshell; gtimeout and the agent are orphaned and keep working.

A process-group kill is not the answer either: the agent is not in our group, so -$PGID signals the loop itself and spares the agent. (Measured — a candidate doing this killed the supervisor and left the target running.)

28cf3ca adds list_descendants() and kill_tree() and calls the latter from on_signal(). Membership is proven by parentage and enumerated before anything is signalled — once the root dies its descendants reparent to launchd and no later walk can find them. Deepest-first TERM, 5s grace, then SIGKILL to survivors; it never signals itself, an ancestor, or a process group. Written for bash 3.2, which #!/bin/bash on macOS requires: no mapfile, no readarray, no array expansions that abort under set -u.

Control vs fixed:

Test	Control (before 28cf3ca)	After
Real agent, real Ctrl-C under a pty	3 files written after the stop; ran to completion; $0.242	0 files, killed at 5 turns, $0.105
Zero-cost stub, repeated	fails 3/3	passes 3/3
Library selftest, bash 3.2	control fails as it must	6/6

Regression tests (99f2323). tests/integration/test_process_tree_kill.bats — seven tests, each verified to fail against unpatched main at 0c1d7bf and pass against the fix. They source the shipped ralph_loop.sh in a subshell, so the functions under test are the ones that ship; trees are built from sleep, so there are no API calls.

list_descendants finds a child and a grandchild through a wrapper
a single-pid TERM leaves the tree alive  (known-false control)
kill_tree clears the whole tree from a live root
kill_tree reaches a descendant in a DIFFERENT process group
kill_tree does not kill its own caller
kill_tree escalates to SIGKILL past a TERM-ignorer
the signal handler stops the tree, not a single pid

The control lives inside the suite: test 2 asserts the OLD single-pid TERM still fails to stop a tree, so if it ever goes green the kill_tree tests are proving nothing and it says so. Test 4 asserts the fixture actually straddles a process-group boundary before asserting the tree died.

⚠️ Test 5 originally passed against unpatched main — kill_tree does not exist there, so nothing died and the caller trivially survived. It now also asserts the target died. This is the same fault the sixth signal test was dropped for, and it is recorded rather than quietly fixed.

What this evidence is still not. The behavioural proof of the original defect came from an external pty harness and a faithful miniature of the loop's spawn-and-signal architecture. The control has never been exercised against a live ralph_loop.sh end to end.

Known gap, not addressed here. CLAUDE_CHILD_PID is assigned only in the background branch. Under --live the agent runs as a foreground pipeline and is never registered, so the child-kill leg is a no-op in that mode.

Test suite status

Re-measured on macOS at 28cf3ca, not carried from an earlier run:

Suite	Result
npm test at 99f2323	1286 tests, 13 failures
npm test at unpatched main 0c1d7bf	1274 tests, 13 failures
npm run test:e2e at 28cf3ca	18 passed, 0 failures

The 13 failures are identical in name and number across both runs, so this PR introduces none of them. They are platform failures (head: illegal line count -- -1, a Linux-only notify-send case, a BSD stat fallback case), none in code this PR touches. The count differs by 12 because this PR adds 5 signal tests and 7 process-tree tests.

The e2e suite runs ralph_loop.sh as a real subprocess and includes termination signal during execution records interrupted status and preserves call count, which exercises the signal path this PR changes.

Reproducing the bug

cd any-ralph-project
./ralph_loop.sh --dry-run     # in a terminal
# press Ctrl-C
# => "Ralph loop interrupted. Cleaning up..." then "=== Completed Loop #2 ==="