From d4556794a7fbf0597c137124c9133591321231c1 Mon Sep 17 00:00:00 2001 From: Sofoklis-byte Date: Sun, 9 Aug 2026 19:17:44 +1000 Subject: [PATCH 1/4] fix(signals): make Ctrl-C and SIGTERM actually stop the loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SIGINT/SIGTERM trap called cleanup(), which returns instead of exiting, so bash resumed the main loop: the operator saw "Ralph loop interrupted. Cleaning up..." and the loop kept running. Five faults in one control: 1. trap cleanup SIGINT SIGTERM — the handler never terminated. 2. The comment claimed "EXIT trap handles natural termination". There is no EXIT trap anywhere in the file; that trap line was the only one. 3. The _CLEANUP_DONE reentrancy guard, documented as covering an "EXIT + signal combination" that does not exist, made every signal after the first a silent no-op. 4. cleanup() read $? as trap_exit_code. In a trap handler that is the last completed command's status, not 128+n, so the "interrupted" status was recorded only when the preceding command happened to fail. 5. The claude child is backgrounded with a `local claude_pid`, invisible to the handler, so a SIGTERM to the script left the child running. Fix: a dedicated on_signal() that restores default dispositions first (so a second Ctrl-C always kills even if cleanup hangs), stops the child via a new CLAUDE_CHILD_PID global, runs cleanup, then re-raises the signal so the process dies with the correct 128+n status. _INTERRUPTED makes the interrupted-status record deterministic. Proof: 7/7 in --dry-run (no API call), each patched case paired with the unpatched original as a known-false control. SIGINT to pgroup patched DEAD 1s (130) · original ALIVE at 15s SIGTERM to pid patched DEAD 2s (143) · original ALIVE at 15s SIGINT x4 patched DEAD on first · original ALIVE, Loop #3 no signal patched still looping — normal path unaffected The original's control log reproduces the reported symptom exactly: "interrupted. Cleaning up..." followed by "Completed Loop #1". Not covered: dry-run returns before spawning a child, so the CLAUDE_CHILD_PID kill leg is reasoned, not measured. Co-Authored-By: Claude Opus 5 (1M context) --- ralph_loop.sh | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/ralph_loop.sh b/ralph_loop.sh index a7c2f1e0..8f4af8f2 100755 --- a/ralph_loop.sh +++ b/ralph_loop.sh @@ -2099,6 +2099,9 @@ execute_claude_code() { # Get PID and monitor progress local claude_pid=$! + # Expose the child to on_signal(): claude_pid is local, and a SIGTERM + # sent to this script does not reach a backgrounded child. + CLAUDE_CHILD_PID=$claude_pid local progress_counter=0 # Early failure detection: if the command doesn't exist or fails immediately, @@ -2174,6 +2177,7 @@ EOF # Wait for the process to finish and get exit code wait $claude_pid exit_code=$? + CLAUDE_CHILD_PID="" fi # Issue #75: pull changed files back from the cloud sandbox BEFORE the @@ -2452,16 +2456,41 @@ cleanup() { # Only record "interrupted" status for abnormal exits (non-zero exit code) # Normal exit (code 0) preserves the status already written by the main loop - if [[ $loop_count -gt 0 && $trap_exit_code -ne 0 ]]; then + # _INTERRUPTED is authoritative for signal paths: in a trap handler $? is the + # status of the last completed command, NOT 128+n, so trap_exit_code alone + # records the interrupt only when the preceding command happened to fail. + if [[ $loop_count -gt 0 && ( $trap_exit_code -ne 0 || "${_INTERRUPTED:-false}" == "true" ) ]]; then log_status "INFO" "Ralph loop interrupted. Cleaning up..." reset_session "manual_interrupt" update_status "$loop_count" "$(cat "$CALL_COUNT_FILE" 2>/dev/null || echo "0")" "interrupted" "stopped" fi - # No exit here — EXIT trap handles natural termination + # No exit here — cleanup() is teardown only, and is called from both the + # normal loop-exit path and on_signal() below. The signal handler owns + # termination; see the warning there. } # Set up signal handlers -trap cleanup SIGINT SIGTERM +# ⚠️ cleanup() deliberately does NOT exit — it is teardown only. A signal trap +# that merely calls it RETURNS, and bash resumes the main loop: the operator +# sees "interrupted", and the loop keeps running. The handler must terminate +# the process itself, and must also stop the backgrounded Claude child, which +# a SIGTERM sent to this script alone never reaches. +_INTERRUPTED=false +on_signal() { + local sig="$1" + # Restore defaults FIRST so a second ^C always kills, even if cleanup hangs. + trap - SIGINT SIGTERM + _INTERRUPTED=true + log_status "WARN" "Received SIG${sig} — stopping Ralph…" + if [[ -n "${CLAUDE_CHILD_PID:-}" ]] && kill -0 "$CLAUDE_CHILD_PID" 2>/dev/null; then + kill -TERM "$CLAUDE_CHILD_PID" 2>/dev/null + fi + cleanup + # Re-raise so we die of the signal with the correct exit status (128+n). + kill -s "$sig" $$ +} +trap 'on_signal INT' SIGINT +trap 'on_signal TERM' SIGTERM # Global variable for loop count (needed by cleanup function) loop_count=0 From 0a17289cb358545c2d63575a413727e88065b568 Mon Sep 17 00:00:00 2001 From: Sofoklis-byte Date: Sun, 9 Aug 2026 20:05:19 +1000 Subject: [PATCH 2/4] test(signals): regression tests for the stop control Five tests, each verified to FAIL against the unpatched loop and pass against the fix: - 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 All run under --dry-run, so no API calls are made. Two harness notes worth keeping, both of which 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, which proves nothing. The perl wrapper resets SIGINT to default before exec, as an interactive terminal would. - `wait` is used rather than a kill -0 poll: 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 that line is only printed when the preceding command happened to fail, so under --dry-run it never appears and the assertion matched nothing. It passed against the buggy code and was therefore worthless. Co-Authored-By: Claude Opus 5 (1M context) --- tests/integration/test_signal_handling.bats | 126 ++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 tests/integration/test_signal_handling.bats diff --git a/tests/integration/test_signal_handling.bats b/tests/integration/test_signal_handling.bats new file mode 100644 index 00000000..f160f46a --- /dev/null +++ b/tests/integration/test_signal_handling.bats @@ -0,0 +1,126 @@ +#!/usr/bin/env bats +# +# Regression tests for the SIGINT/SIGTERM stop control. +# +# The bug: `trap cleanup SIGINT SIGTERM` invoked a cleanup() that deliberately +# returns rather than exiting ("No exit here — EXIT trap handles natural +# termination" — but no EXIT trap existed). The handler therefore returned and +# bash resumed the main loop: Ralph logged "Ralph loop interrupted. Cleaning +# up..." and kept running. The _CLEANUP_DONE reentrancy guard then made every +# subsequent signal a silent no-op, so repeated Ctrl-C could not recover. +# +# These tests run the loop with --dry-run, so they make no API calls. +# +# NOTE ON SIGINT: 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 therefore shows the loop surviving Ctrl-C whether or +# not the bug is present. The perl wrapper resets SIGINT to its default +# disposition before exec, which is what an interactive terminal supplies. + +load ../helpers/test_helper + +RALPH_LOOP="${BATS_TEST_DIRNAME}/../../ralph_loop.sh" + +# Build a minimal but valid Ralph project in the per-test temp dir. +setup_ralph_project() { + mkdir -p .ralph/specs src + printf 'Say hello. Do nothing else.\n' > .ralph/PROMPT.md + printf '# fix plan\n- nothing\n' > .ralph/fix_plan.md + printf '# agent\nTest fixture.\n' > .ralph/AGENT.md + printf 'ALLOWED_TOOLS="Read"\n' > .ralphrc +} + +# Start the loop in its own process group with default signal dispositions. +# Sets LOOP_PID. Must NOT be called in a command substitution, or the job +# becomes a child of the subshell and `wait` cannot reach it. +start_loop() { + perl -e '$SIG{INT}="DEFAULT"; $SIG{QUIT}="DEFAULT"; setpgrp(0,0); exec @ARGV' \ + bash "$RALPH_LOOP" --dry-run > loop.log 2>&1 & + LOOP_PID=$! +} + +# Wait until the loop is actually executing an iteration, or fail. +wait_for_loop() { + local waited=0 + while [ "$waited" -lt 40 ]; do + grep -q "DRY RUN" loop.log 2>/dev/null && return 0 + kill -0 "$LOOP_PID" 2>/dev/null || return 1 + sleep 1 + waited=$((waited + 1)) + done + return 1 +} + +# Block until the loop exits, with a hard watchdog so a hung test cannot wedge +# the suite. Sets LOOP_RC to the loop's exit status. `wait` is used rather than +# a kill -0 poll because a dead-but-unreaped child still answers kill -0. +reap_loop() { + ( sleep 15; kill -9 "-$LOOP_PID" 2>/dev/null ) & + local watchdog=$! + # A signalled child returns 128+n; capture it without tripping bats' errexit. + LOOP_RC=0 + wait "$LOOP_PID" 2>/dev/null || LOOP_RC=$? + kill "$watchdog" 2>/dev/null || true + wait "$watchdog" 2>/dev/null || true + return 0 +} + +@test "SIGTERM terminates the loop instead of being swallowed" { + setup_ralph_project + start_loop + wait_for_loop || { kill -9 "-$LOOP_PID" 2>/dev/null; skip "loop did not start"; } + + kill -TERM "$LOOP_PID" + reap_loop + + if [ "$LOOP_RC" -ne 143 ]; then # 128 + SIGTERM + echo "expected exit 143, got $LOOP_RC; log:"; cat loop.log + return 1 + fi +} + +@test "SIGINT terminates the loop instead of being swallowed" { + command -v perl >/dev/null || skip "perl required to reset SIGINT disposition" + setup_ralph_project + start_loop + wait_for_loop || { kill -9 "-$LOOP_PID" 2>/dev/null; skip "loop did not start"; } + + kill -INT "-$LOOP_PID" # process group, as a terminal would + reap_loop + + if [ "$LOOP_RC" -ne 130 ]; then # 128 + SIGINT + echo "expected exit 130, got $LOOP_RC; log:"; cat loop.log + return 1 + fi +} + +@test "repeated signals do not disarm the handler" { + setup_ralph_project + start_loop + wait_for_loop || { kill -9 "-$LOOP_PID" 2>/dev/null; skip "loop did not start"; } + + # Under the reentrancy-guard bug the first signal disarmed cleanup() and + # every later one was a silent no-op. The loop must die on the first. + kill -TERM "$LOOP_PID" + kill -TERM "$LOOP_PID" 2>/dev/null || true + reap_loop + + if [ "$LOOP_RC" -eq 137 ]; then + echo "loop had to be SIGKILLed by the watchdog; log:"; cat loop.log + return 1 + fi +} + +@test "the signal trap is not a bare call to cleanup" { + # cleanup() is teardown only and must never be installed as the handler on + # its own: it returns, and bash then resumes the loop. + run grep -E '^trap[[:space:]]+cleanup[[:space:]]+SIGINT[[:space:]]+SIGTERM' "$RALPH_LOOP" + [ "$status" -ne 0 ] +} + +@test "the child Claude pid is exposed to the signal handler" { + # A SIGTERM addressed to the script does not reach a backgrounded child, + # so the handler needs a non-local pid to stop it. + run grep -q 'CLAUDE_CHILD_PID' "$RALPH_LOOP" + [ "$status" -eq 0 ] +} From 28cf3ca62e53e23d2f063825981cde1737cfba49 Mon Sep 17 00:00:00 2001 From: Sofoklis-byte Date: Mon, 10 Aug 2026 00:00:58 +1000 Subject: [PATCH 3/4] fix(signals): stop the whole agent tree, not one pid Ctrl-C returned the prompt but the agent kept working and kept billing. Measured mid-work under a real pty: loop pid 94095 pgid 94095 <- foreground group; gets 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 execs gtimeout in its place and sometimes does not, varying with execution context, so $! does not reliably name the agent. Ctrl-C reaches only the foreground group, so the agent never sees it; on_signal then TERMs the subshell and gtimeout plus the agent are orphaned and run on. 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. Add list_descendants() and kill_tree(), and call kill_tree 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; never signals itself, an ancestor, or a process group. Written for bash 3.2, which #!/bin/bash on macOS means it must be: no mapfile, no readarray, no array expansions that abort under set -u. Evidence, control vs fixed: real agent, real Ctrl-C 3 files 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 (3.2) control fails as it must -> 6/6 Not addressed here: CLAUDE_CHILD_PID is assigned only in the background branch, so the child-kill leg is a no-op under --live, where the agent runs as a foreground pipeline and is never registered. Co-Authored-By: Claude Opus 5 (1M context) --- ralph_loop.sh | 138 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 1 deletion(-) diff --git a/ralph_loop.sh b/ralph_loop.sh index 8f4af8f2..29e12c2c 100755 --- a/ralph_loop.sh +++ b/ralph_loop.sh @@ -2469,6 +2469,135 @@ cleanup() { # termination; see the warning there. } +# ⚠️ WHY STOPPING NEEDS A TREE WALK AND NOT A kill. +# +# `$!` does NOT reliably name the agent. 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, and which one you get varies +# with the execution context. Measured, mid-work, under a real pty: +# +# loop pid 94095 pgid 94095 <- foreground group; gets 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 +# +# Two consequences, both load-bearing: +# - Ctrl-C signals only the foreground group, so the agent never sees it. +# - TERMing $! kills the subshell; gtimeout and the agent are orphaned and +# keep working. Measured: files written 6s, 11s and 17s after the stop, +# and the API call billed in full. +# A `kill -$PGID` is NOT the answer either: the agent is not in our group, so +# that signals the loop itself and spares the agent. Membership must be +# established by parentage before anything is signalled, never assumed. +# +# Targets bash 3.2 (`#!/bin/bash` on macOS): no mapfile, no readarray, no +# associative arrays, no array expansions that break under `set -u`. + +# Print every descendant of $1 (excluding $1), one pid per line, shallowest +# first, proven against the live ppid table. +list_descendants() { + _ld_root="$1" + [ -n "$_ld_root" ] || return 0 + _ld_snapshot=`ps -eo pid=,ppid= 2>/dev/null` + [ -n "$_ld_snapshot" ] || return 0 + + _ld_frontier="$_ld_root" + _ld_found="" + _ld_guard=0 + while [ -n "$_ld_frontier" ]; do + _ld_guard=`expr $_ld_guard + 1` + [ "$_ld_guard" -gt 64 ] && break + _ld_next="" + while read -r _ld_p _ld_pp; do + [ -n "$_ld_p" ] || continue + for _ld_f in $_ld_frontier; do + if [ "$_ld_pp" = "$_ld_f" ]; then + _ld_next="$_ld_next $_ld_p" + break + fi + done + done </dev/null | tr -d ' '` + done + _kt_forbids() { + for _kt_x in $_kt_forbidden; do + [ "$1" = "$_kt_x" ] && return 0 + done + return 1 + } + + # ⛔ ENUMERATE BEFORE SIGNALLING. Once the root dies its descendants are + # reparented to launchd and no later walk can ever find them. + _kt_members="" + for _kt_p in `list_descendants "$_kt_root"`; do + _kt_forbids "$_kt_p" || _kt_members="$_kt_p $_kt_members" + done + _kt_forbids "$_kt_root" || _kt_members="$_kt_members $_kt_root" + [ -n "$_kt_members" ] || return 0 + + for _kt_p in $_kt_members; do kill -TERM "$_kt_p" 2>/dev/null; done + + _kt_waited=0 + _kt_tenths=`expr $_kt_grace \* 10` + while [ "$_kt_waited" -lt "$_kt_tenths" ]; do + _kt_any=no + for _kt_p in $_kt_members; do + if kill -0 "$_kt_p" 2>/dev/null; then _kt_any=yes; break; fi + done + [ "$_kt_any" = "no" ] && return 0 + sleep 0.1 + _kt_waited=`expr $_kt_waited + 1` + done + + # Re-enumerate from SURVIVING members (not the root, which may be gone) to + # catch anything spawned during the grace window. + _kt_late="" + for _kt_p in $_kt_members; do + if kill -0 "$_kt_p" 2>/dev/null; then + for _kt_q in `list_descendants "$_kt_p"`; do + _kt_forbids "$_kt_q" || _kt_late="$_kt_late $_kt_q" + done + fi + done + for _kt_p in $_kt_members $_kt_late; do + kill -0 "$_kt_p" 2>/dev/null && kill -KILL "$_kt_p" 2>/dev/null + done + + sleep 0.3 + for _kt_p in $_kt_members $_kt_late; do + if kill -0 "$_kt_p" 2>/dev/null; then return 1; fi + done + return 0 +} + # Set up signal handlers # ⚠️ cleanup() deliberately does NOT exit — it is teardown only. A signal trap # that merely calls it RETURNS, and bash resumes the main loop: the operator @@ -2483,7 +2612,14 @@ on_signal() { _INTERRUPTED=true log_status "WARN" "Received SIG${sig} — stopping Ralph…" if [[ -n "${CLAUDE_CHILD_PID:-}" ]] && kill -0 "$CLAUDE_CHILD_PID" 2>/dev/null; then - kill -TERM "$CLAUDE_CHILD_PID" 2>/dev/null + # NOT `kill -TERM "$CLAUDE_CHILD_PID"`: that pid is usually a wrapper, + # and killing it orphans the agent, which keeps working and keeps + # billing. Walk the tree and terminate every verified member. + if kill_tree "$CLAUDE_CHILD_PID" 5; then + log_status "INFO" "Claude process tree stopped" + else + log_status "ERROR" "⛔ Claude process tree SURVIVED SIGKILL — check for strays before relaunching" + fi fi cleanup # Re-raise so we die of the signal with the correct exit status (128+n). From 99f2323344516e155442e87f73c4ae0cf0ecd6ec Mon Sep 17 00:00:00 2001 From: Sofoklis-byte Date: Mon, 10 Aug 2026 14:03:53 +1000 Subject: [PATCH 4/4] test(signals): regression tests for the process-tree kill Seven tests, each verified to FAIL against unpatched main at 0c1d7bf and pass against the fix: 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 They source the shipped ralph_loop.sh in a subshell, so the functions under test are the ones that ship and the loop's own traps never land in the bats shell. Trees are built from sleep; no API calls, no cost. The control is inside the suite rather than in a commit message: test 2 asserts the OLD single-pid TERM still fails to stop a tree. If it ever goes green, the kill_tree tests are proving nothing and it fails loudly. Test 4 is the production shape. gtimeout setpgid()s itself, so the agent is not in the loop's group; the test asserts the fixture actually straddles a group boundary before asserting the tree died, or it would pass on a fixture that never tested the thing. Two harness notes, both of which produced wrong results first: - 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. A test that goes green against the buggy code is worthless -- the same fault this suite dropped a sixth test for. - Cleanup must not be a test's last command: killing an already-dead pid exits non-zero and fails the test for the wrong reason. Suite: 1286 tests, 13 failures, the same 13 pre-existing platform failures present on unpatched main. Co-Authored-By: Claude Opus 5 (1M context) --- tests/integration/test_process_tree_kill.bats | 173 ++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 tests/integration/test_process_tree_kill.bats diff --git a/tests/integration/test_process_tree_kill.bats b/tests/integration/test_process_tree_kill.bats new file mode 100644 index 00000000..35a041b7 --- /dev/null +++ b/tests/integration/test_process_tree_kill.bats @@ -0,0 +1,173 @@ +#!/usr/bin/env bats +# +# Regression tests for kill_tree() / list_descendants() — the stop control's +# child-kill leg. +# +# The bug these cover: the loop stored `$!` in CLAUDE_CHILD_PID and sent it a +# single SIGTERM. That pid is not reliably the agent. 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 the timeout binary and +# sometimes does not, varying with execution context. Killing the captured pid +# therefore killed a wrapper and orphaned the agent, which kept working and +# kept billing. A process-group kill is not the answer either — the agent is +# not in the loop's group, so `-$PGID` signals the loop and spares the agent. +# +# These tests use `sleep` trees. They make no API calls and cost nothing. +# +# Each behavioural test that proves kill_tree works is paired with a control +# that must genuinely FAIL first — see "a single-pid TERM leaves the tree +# alive". If that control ever passes, the others prove nothing and it fails +# loudly rather than going green for the wrong reason. +# +# Written for bash 3.2 (`#!/bin/bash` on macOS): no mapfile, no readarray, and +# no array expansion that aborts under `set -u`. + +load ../helpers/test_helper + +RALPH_LOOP="${BATS_TEST_DIRNAME}/../../ralph_loop.sh" + +# Run a snippet with the real ralph_loop.sh sourced, in a subshell so the +# loop's own SIGINT/SIGTERM traps are never installed in the bats shell. +# The functions under test are the ones in the shipped file, not a copy. +in_loop_ctx() { + bash -c "source '$RALPH_LOOP' >/dev/null 2>&1; $1" +} + +@test "list_descendants finds a child and a grandchild through a wrapper" { + run in_loop_ctx ' + bash -c "bash -c \"sleep 30\" & sleep 30" & + root=$! + sleep 1 + n=0 + for p in $(list_descendants "$root"); do n=$((n + 1)); done + kill -KILL $(list_descendants "$root") "$root" 2>/dev/null || true + echo "descendants=$n" + ' + [ "$status" -eq 0 ] + [[ "$output" == *"descendants=2"* ]] +} + +@test "a single-pid TERM leaves the tree alive — the defect this fix exists to stop" { + # KNOWN-FALSE CONTROL. This asserts the OLD behaviour still fails. If this + # ever passes, every kill_tree test below is proving nothing. + run in_loop_ctx ' + bash -c "bash -c \"sleep 30\" & sleep 30" & + root=$! + sleep 1 + kids=$(list_descendants "$root") + kill -TERM "$root" 2>/dev/null # exactly the old child-kill leg + sleep 1 + alive=0 + for p in $kids; do kill -0 "$p" 2>/dev/null && alive=$((alive + 1)); done + kill -KILL $kids 2>/dev/null || true + echo "survivors=$alive" + ' + [ "$status" -eq 0 ] + [[ "$output" == *"survivors=2"* ]] +} + +@test "kill_tree clears the whole tree from a live root" { + run in_loop_ctx ' + bash -c "bash -c \"sleep 30\" & sleep 30" & + root=$! + sleep 1 + kids=$(list_descendants "$root") + kill_tree "$root" 3 + sleep 1 + alive=0 + for p in $root $kids; do kill -0 "$p" 2>/dev/null && alive=$((alive + 1)); done + kill -KILL $root $kids 2>/dev/null || true + echo "alive=$alive" + ' + [ "$status" -eq 0 ] + [[ "$output" == *"alive=0"* ]] +} + +@test "kill_tree reaches a descendant in a DIFFERENT process group" { + # The production shape: the timeout binary puts itself and the agent into + # their own group, so group membership cannot be assumed from the pid the + # loop captured. Parentage is the only test that survives this. + command -v perl >/dev/null || skip "perl required to create a new process group" + run in_loop_ctx ' + bash -c "perl -e \"setpgrp(0,0); exec q(sleep), q(30)\" & sleep 30" & + root=$! + sleep 1 + kids=$(list_descendants "$root") + rootpg=$(ps -o pgid= -p "$root" | tr -d " ") + crossed=no + for p in $kids; do + pg=$(ps -o pgid= -p "$p" 2>/dev/null | tr -d " ") + [ -n "$pg" ] && [ "$pg" != "$rootpg" ] && crossed=yes + done + kill_tree "$root" 3 + sleep 1 + alive=0 + for p in $root $kids; do kill -0 "$p" 2>/dev/null && alive=$((alive + 1)); done + kill -KILL $root $kids 2>/dev/null || true + echo "crossed=$crossed alive=$alive" + ' + [ "$status" -eq 0 ] + # The fixture must actually straddle a group boundary, or it tests nothing. + [[ "$output" == *"crossed=yes"* ]] + [[ "$output" == *"alive=0"* ]] +} + +@test "kill_tree does not kill its own caller" { + # A candidate fix that signalled -$PGID killed the supervisor and left the + # target running. A stop control that kills the supervisor is worse than + # one that does nothing. + run in_loop_ctx ' + bash -c "sleep 30" & + root=$! + sleep 1 + kill_tree "$root" 3 + # The target MUST be dead as well, or this test passes vacuously + # wherever kill_tree does not exist — a test that goes green against + # the unpatched loop is worthless. + if kill -0 "$root" 2>/dev/null; then target=alive; else target=dead; fi + kill -KILL "$root" 2>/dev/null || true + # Cleanup must not be the last command: killing an already-dead pid + # exits non-zero and would fail the test for the wrong reason. + kill -0 $$ 2>/dev/null && echo "caller=alive target=$target" \ + || echo "caller=dead target=$target" + ' + [ "$status" -eq 0 ] + [[ "$output" == *"caller=alive"* ]] + [[ "$output" == *"target=dead"* ]] +} + +@test "kill_tree escalates to SIGKILL past a process that ignores SIGTERM" { + run in_loop_ctx ' + bash -c "trap \"\" TERM; sleep 30" & + stubborn=$! + sleep 1 + kill_tree "$stubborn" 2 + sleep 1 + if kill -0 "$stubborn" 2>/dev/null; then + kill -KILL "$stubborn" 2>/dev/null + echo "stubborn=survived" + else + echo "stubborn=killed" + fi + ' + [ "$status" -eq 0 ] + [[ "$output" == *"stubborn=killed"* ]] +} + +@test "the signal handler stops the tree, not a single pid" { + # Comment lines are excluded deliberately: the handler carries a comment + # naming the old form to explain why it is wrong, and a naive grep counts + # that as the bug still being present. + run bash -c "grep -v '^[[:space:]]*#' '$RALPH_LOOP' | grep -cF 'kill -TERM \"\$CLAUDE_CHILD_PID\"'" + [ "$output" -eq 0 ] + + run bash -c "grep -cF 'kill_tree \"\$CLAUDE_CHILD_PID\"' '$RALPH_LOOP'" + [ "$output" -ge 1 ] +}