diff --git a/AGENTS.md b/AGENTS.md index d9d9d22c6ce..8f4dfb76836 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -417,6 +417,7 @@ A secondmate's idle endpoint is healthy, and parent supervision relies on its ro Waiting on a healthy supervision cycle is silent; empty polls, elapsed time, and no-change updates are not captain-facing progress. Never broadly kill watchers, especially never `pkill -f bin/fm-watch.sh`, because that can kill sibling firstmate homes. A forced repair must use the home-scoped owner path emitted by supervision instructions. +The same hazard applies to the behavior-test runner, which concurrent agents invoke with near-identical command lines: stop a suite with `bin/fm-test-run.sh --stop []`, never `pkill -f fm-test-run.sh`, and treat a run log carrying neither `FM_TEST_SUMMARY` nor `FM_TEST_ABORTED` as lost output rather than a pass. Guard warnings do not replace the contract. Queued wakes must be drained before other action, stale liveness must be repaired through the emitted protocol, and the worktree-tangle warning must be resolved without touching unlanded work. diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index 088c68b3fc4..41a8ef2efe6 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -24,6 +24,10 @@ # Aggregation (no suite execution): # fm-test-run.sh --aggregate-json [more lane.json...] # +# Run control (no suite execution): +# fm-test-run.sh --list-runs +# fm-test-run.sh --stop [] +# # Options: # --json write a deterministic timing artifact after the run # --list print selected script paths (one per line) and exit 0 @@ -42,6 +46,15 @@ # selected script is in the proven-isolated set # (bin/fm-test-isolation-proof.sh --list). Cap is 8. Stateful # families never schedule under --jobs. +# --list-runs print one FM_TEST_RUN record per registered live run and exit +# --stop [] +# stop registered runs by identity, never by process-name +# matching. With a run id, exactly that run. With no run id, +# only the runs registered from THIS repository root, so a +# concurrent agent's suite in another worktree is never a +# candidate. Broad `pkill -f fm-test-run.sh` is what this +# verb exists to replace: concurrent runs share a near +# identical command line, so no pattern can tell them apart. # -h, --help print this header # # Per-script machine-parseable markers (stdout): @@ -53,9 +66,22 @@ # FM_TEST_SUMMARY_FAMILY family= count= duration_ms= failed= # FM_TEST_SLOWEST rank= script= duration_ms= # +# Instead of that summary, an interrupted run prints exactly one terminator: +# FM_TEST_ABORTED run_id= signal= completed= selected= +# A killed run used to end mid-line, so a truncated log was indistinguishable +# from a short clean one and a reader counting failures off it concluded the +# suite passed. Treat a log with neither FM_TEST_SUMMARY nor FM_TEST_ABORTED as +# lost output, never as a pass. +# # Exit status is non-zero if any selected script exits non-zero or a configured # --fail-on-gate-skip token appears. Other gate skips (first meaningful line # matching ^skip:) remain successful and are counted as skipped_gate. +# An aborted run exits 128+ (143 for TERM, 130 for INT, 129 for HUP). +# +# Every executing run registers itself in a per-user registry directory +# (FM_TEST_RUN_REGISTRY, default $TMPDIR/fm-test-run-registry) and removes that +# record on exit. Records are pruned when their pid is gone, so a crashed run +# leaves no stale entry that a later --stop could act on. # # Family labels, the changed-file map, and production portable-shard composition # live in this script only (one owner). The proven-isolated candidate set remains @@ -88,6 +114,19 @@ EXCLUDE_FAMILIES=() FAIL_ON_GATE_SKIP= JOBS=1 JOBS_MAX=8 +LIST_RUNS=0 +STOP_REQUESTED=0 +STOP_RUN_ID= + +# Where executing runs register their identity so a concurrent agent can stop +# one exactly instead of pattern-matching a shared command line. Per-user by +# default because TMPDIR is per-user on macOS; FM_TEST_RUN_REGISTRY overrides it +# for tests and for anyone deliberately partitioning runs further. +RUN_REGISTRY="${FM_TEST_RUN_REGISTRY:-${TMPDIR:-/tmp}/fm-test-run-registry}" + +# Seconds to wait for a signalled run to write its terminator and exit before +# escalating to SIGKILL on whatever is left. +STOP_GRACE_SECONDS="${FM_TEST_RUN_STOP_GRACE:-10}" # How many separate-runner shards the portable serial remainder splits into. # One owner: CI lane names carry this count and are refused when they disagree. @@ -128,6 +167,211 @@ now_ms() { fi } +# --------------------------------------------------------------------------- +# Run registry: identity-scoped stop, so nobody has to reach for pkill. +# +# Concurrent agents invoke this runner with near-identical command lines +# ("--changed --base origin/main"), so `pkill -f` cannot distinguish one run +# from another and has killed a sibling agent's suite mid-flight. Each +# executing run therefore records its own identity here and --stop acts only on +# those records: by exact run id, or by this repository root. +# --------------------------------------------------------------------------- + +run_registry_file() { + printf '%s/%s.run\n' "$RUN_REGISTRY" "$1" +} + +# One field out of a registry record. Fields are "key=value", one per line, and +# values may contain "=" so only the first is a separator. +run_record_field() { + local file=$1 key=$2 line + [ -f "$file" ] || return 1 + while IFS= read -r line; do + case "$line" in + "$key="*) printf '%s\n' "${line#*=}"; return 0 ;; + esac + done <"$file" + return 1 +} + +pid_is_alive() { + kill -0 "$1" 2>/dev/null +} + +# Direct and transitive children of a pid, deepest last. Used so a stop reaches +# the test script the run is currently executing (and any --jobs workers) +# without ever touching a process the run does not own. +descendant_pids() { + local root=$1 snapshot + snapshot=$(ps -eo pid=,ppid= 2>/dev/null || true) + [ -n "$snapshot" ] || return 0 + # One breadth-first pass per level, driven off a single ps snapshot so the + # tree cannot shift underneath a walk that re-reads the process table. + printf '%s\n' "$snapshot" | awk -v root="$root" ' + { child[NR] = $1; parent[NR] = $2; rows = NR } + END { + want[root] = 1 + # Bounded by tree depth; each pass adopts children of everything marked + # so far, and stops as soon as a pass marks nothing new. + do { + added = 0 + for (i = 1; i <= rows; i++) { + if (want[parent[i]] && !want[child[i]]) { + want[child[i]] = 1 + print child[i] + added = 1 + } + } + } while (added) + } + ' +} + +# Drop records whose process is gone. A crashed run must not leave an entry +# that a later --stop could act on, and a recycled pid must not inherit one. +prune_run_registry() { + local file pid + [ -d "$RUN_REGISTRY" ] || return 0 + for file in "$RUN_REGISTRY"/*.run; do + [ -f "$file" ] || continue + pid=$(run_record_field "$file" pid || true) + if [ -z "$pid" ] || ! pid_is_alive "$pid"; then + rm -f "$file" + fi + done +} + +register_run() { + local file + mkdir -p "$RUN_REGISTRY" || die "could not create run registry: $RUN_REGISTRY" + chmod 0700 "$RUN_REGISTRY" 2>/dev/null || true + file=$(run_registry_file "$RUN_ID") + { + printf 'run_id=%s\n' "$RUN_ID" + printf 'pid=%s\n' "$$" + printf 'root=%s\n' "$ROOT" + printf 'fm_home=%s\n' "${FM_HOME:--}" + printf 'json=%s\n' "${JSON_PATH:--}" + printf 'selection=%s\n' "$SELECTION_DESC" + printf 'started=%s\n' "$RUN_STARTED_ISO" + } >"$file" || die "could not write run record: $file" + RUN_REGISTERED_FILE=$file +} + +list_runs() { + local file run_id pid root started selection printed=0 + prune_run_registry + for file in "$RUN_REGISTRY"/*.run; do + [ -f "$file" ] || continue + run_id=$(run_record_field "$file" run_id || echo unknown) + pid=$(run_record_field "$file" pid || echo unknown) + root=$(run_record_field "$file" root || echo unknown) + started=$(run_record_field "$file" started || echo unknown) + selection=$(run_record_field "$file" selection || echo unknown) + printf 'FM_TEST_RUN run_id=%s pid=%s started=%s root=%s selection=%s\n' \ + "$run_id" "$pid" "$started" "$root" "$selection" + printed=1 + done + [ "$printed" -eq 1 ] || log "no live runs registered in $RUN_REGISTRY" +} + +# Signal one registered run and, after its grace period, whatever it left +# behind. The run's own handler writes FM_TEST_ABORTED, so its log identifies +# itself as truncated rather than looking like a short clean run. +stop_one_run() { + local file=$1 run_id pid waited child + run_id=$(run_record_field "$file" run_id || echo unknown) + pid=$(run_record_field "$file" pid || true) + if [ -z "$pid" ] || ! pid_is_alive "$pid"; then + rm -f "$file" + log "run $run_id was already gone" + return 0 + fi + + log "stopping run $run_id (pid $pid)" + # The run's currently-executing test script is signalled alongside the run + # itself. Bash defers a trap until the foreground command returns, so TERMing + # only the run would leave it blocked in the middle of a long test and its + # terminator unwritten until the grace period expired and killed it silently. + for child in $(descendant_pids "$pid"); do + kill -TERM "$child" 2>/dev/null || true + done + kill -TERM "$pid" 2>/dev/null || true + + waited=0 + while [ "$waited" -lt "$STOP_GRACE_SECONDS" ] && pid_is_alive "$pid"; do + sleep 1 + waited=$((waited + 1)) + done + + # Anything still alive is the run's own subtree; the run itself already had + # its chance to exit cleanly. Children are collected before the final kill so + # a test script that ignored TERM cannot outlive its run. + for child in $(descendant_pids "$pid"); do + kill -KILL "$child" 2>/dev/null || true + done + if pid_is_alive "$pid"; then + kill -KILL "$pid" 2>/dev/null || true + log "run $run_id did not exit within ${STOP_GRACE_SECONDS}s; killed" + fi + rm -f "$file" + printf 'FM_TEST_STOPPED run_id=%s pid=%s\n' "$run_id" "$pid" +} + +# With a run id, stop exactly that run. Without one, stop only the runs this +# repository root started - never a sibling worktree's run, which is the exact +# collateral damage a broad pkill causes. +stop_runs() { + local want=$1 file root matched=0 + prune_run_registry + if [ -n "$want" ]; then + # Run ids are minted by this script as fm-test-run--. Validating + # the shape keeps a caller-supplied id from resolving to a path outside the + # registry, since the id is used to build a file this function rm -f's. + case "$want" in + */*) die "not a run id: $want (expected fm-test-run--; see --list-runs)" ;; + fm-test-run-[0-9]*-[0-9]*) ;; + *) die "not a run id: $want (expected fm-test-run--; see --list-runs)" ;; + esac + file=$(run_registry_file "$want") + [ -f "$file" ] || die "no live run registered with id: $want (see --list-runs)" + stop_one_run "$file" + return 0 + fi + [ -d "$RUN_REGISTRY" ] || die "no live run registered from this root: $ROOT" + for file in "$RUN_REGISTRY"/*.run; do + [ -f "$file" ] || continue + root=$(run_record_field "$file" root || true) + [ "$root" = "$ROOT" ] || continue + stop_one_run "$file" + matched=$((matched + 1)) + done + [ "$matched" -gt 0 ] \ + || die "no live run registered from this root: $ROOT (see --list-runs)" +} + +# Terminator for an interrupted run. Printed exactly once, to stdout, in place +# of FM_TEST_SUMMARY, so a truncated log is self-identifying. +# shellcheck disable=SC2329 # Invoked indirectly by the signal traps below. +abort_run() { + local signal=$1 status=$2 child + trap - TERM INT HUP + [ "${RUN_ABORTED:-0}" -eq 0 ] || exit "$status" + RUN_ABORTED=1 + for child in $(descendant_pids "$$"); do + kill -TERM "$child" 2>/dev/null || true + done + printf 'FM_TEST_ABORTED %s run_id=%s signal=%s completed=%s selected=%s\n' \ + "$(now_iso)" "${RUN_ID:-unknown}" "$signal" "${TOTAL:-0}" "${SELECTED_TOTAL:-0}" + exit "$status" +} + +# shellcheck disable=SC2329 # Invoked indirectly by the EXIT trap below. +cleanup_run() { + [ -z "${RUN_REGISTERED_FILE:-}" ] || rm -f "$RUN_REGISTERED_FILE" + [ -z "${RUN_TMP:-}" ] || rm -rf "$RUN_TMP" +} + # Primary family for one tests/*.test.sh basename. Unmapped scripts are # unclassified so new tests are still runnable and visible in summaries. family_for_basename() { @@ -1270,6 +1514,26 @@ while [ "$#" -gt 0 ]; do CHECK_COVERAGE=1 shift ;; + --list-runs) + LIST_RUNS=1 + shift + ;; + --stop) + STOP_REQUESTED=1 + shift + # An optional bare run id; anything starting with "-" is a later option. + if [ "$#" -gt 0 ]; then + case "$1" in + -*) ;; + *) STOP_RUN_ID=$1; shift ;; + esac + fi + ;; + --stop=*) + STOP_REQUESTED=1 + STOP_RUN_ID=${1#--stop=} + shift + ;; --aggregate-json) [ "$#" -gt 1 ] || die "--aggregate-json requires an output path" AGGREGATE_OUT=$2 @@ -1334,6 +1598,16 @@ if [ "$LIST_LANES" -eq 1 ]; then exit 0 fi +if [ "$LIST_RUNS" -eq 1 ]; then + list_runs + exit 0 +fi + +if [ "$STOP_REQUESTED" -eq 1 ]; then + stop_runs "$STOP_RUN_ID" + exit 0 +fi + if [ "$CHECK_COVERAGE" -eq 1 ]; then run_coverage_guard exit $? @@ -1443,16 +1717,26 @@ RUN_TMP=$(mktemp -d "${TMPDIR:-/tmp}/fm-test-run.XXXXXX") RECORDS="$RUN_TMP/records.tsv" FAMILIES_TSV="$RUN_TMP/families.tsv" : >"$RECORDS" -trap 'rm -rf "$RUN_TMP"' EXIT RUN_STARTED_ISO=$(now_iso) RUN_STARTED_MS=$(now_ms) RUN_ID="fm-test-run-${RUN_STARTED_MS}-$$" +RUN_REGISTERED_FILE= +RUN_ABORTED=0 TOTAL=0 +SELECTED_TOTAL=${#SCRIPTS[@]} FAILED=0 SKIPPED_GATE=0 AGG_RC=0 +# Registered before the first script so a stop issued at any point during the +# run finds this run's identity, and torn down on every exit path. +trap 'cleanup_run' EXIT +trap 'abort_run TERM 143' TERM +trap 'abort_run INT 130' INT +trap 'abort_run HUP 129' HUP +register_run + # Family accumulators as TSV lines updated in-memory via temp files. # family -> count, duration_ms, failed family_bump() { diff --git a/docs/scripts.md b/docs/scripts.md index e9492beae58..7a2d1cffafe 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -43,7 +43,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-install-herdr.sh` | Install CI's exact-version Herdr pin with official asset URL, SHA-256, and protocol checks | | `fm-install-treehouse.sh`| Install CI's exact-version Treehouse pin for real-Herdr E2E that needs spawn worktrees | | `fm-herdr-ci-cleanup.sh` | Snapshot and tear down only job-owned `fm-lab-*` sessions in the Herdr CI lane | -| `fm-test-run.sh` | Behavior-test runner: selection, portable lanes, proven-isolated `--jobs`, coverage guard, timing/JSON | +| `fm-test-run.sh` | Behavior-test runner: selection, portable lanes, proven-isolated `--jobs`, coverage guard, timing/JSON, identity-scoped `--list-runs`/`--stop` | | `fm-test-isolation-proof.sh` | Concurrent isolation proof and proven-isolated candidate set owner | | `fm-ensure-agents-md.sh` | Ensure a project's real `AGENTS.md`, its `CLAUDE.md` symlink, and the canonical self-governance section | | `fm-guard.sh` | Warn on primary-checkout tangles, pending queued wakes, and unhealthy supervision | diff --git a/tests/fm-test-run.test.sh b/tests/fm-test-run.test.sh index c8d6ea50822..786ba8bc9df 100755 --- a/tests/fm-test-run.test.sh +++ b/tests/fm-test-run.test.sh @@ -758,6 +758,164 @@ assert len(doc["scripts"])==3 pass "aggregate-json merges lane timing artifacts" } +# --- scoped stop ------------------------------------------------------------ +# +# The runner used to offer no way to stop one run, so agents reached for +# `pkill -f "fm-test-run.sh --changed"` and killed a concurrent agent's suite: +# every run's command line is near-identical, so no pattern can tell them +# apart. These tests pin the replacement - identity-scoped stop plus a loud +# terminator - because both properties fail silently when they regress. + +# Wait up to for to succeed. +fm_await() { + local seconds=$1; shift + local waited=0 + while [ "$waited" -lt "$((seconds * 10))" ]; do + if "$@"; then + return 0 + fi + sleep 0.1 + waited=$((waited + 1)) + done + return 1 +} + +fm_stop_fixture() { + local dir=$1 + cat >"$dir/slow.test.sh" <<'FIXTURE' +#!/usr/bin/env bash +echo "fixture running" +sleep 120 +FIXTURE + chmod +x "$dir/slow.test.sh" +} + +fm_registered_run_count() { + local reg=$1 n + n=$(find "$reg" -name '*.run' 2>/dev/null | wc -l | tr -d ' ') + [ "$n" = "$2" ] +} + +test_stop_targets_one_run_and_marks_the_log_aborted() { + local tmp reg run_id out rc + tmp=$(fm_test_tmproot fm-test-run-stop) || fail "could not create tmp root" + reg="$tmp/registry" + fm_stop_fixture "$tmp" + + FM_TEST_RUN_REGISTRY="$reg" "$RUNNER" "$tmp/slow.test.sh" >"$tmp/run.log" 2>&1 & + local runpid=$! + fm_await 30 fm_registered_run_count "$reg" 1 \ + || { kill -KILL "$runpid" 2>/dev/null; fail "run did not register itself"; } + + out=$(FM_TEST_RUN_REGISTRY="$reg" "$RUNNER" --list-runs) + assert_contains "$out" "FM_TEST_RUN run_id=" "--list-runs must report the live run" + run_id=$(printf '%s\n' "$out" | sed -n 's/.*run_id=\([^ ]*\).*/\1/p') + [ -n "$run_id" ] || { kill -KILL "$runpid" 2>/dev/null; fail "--list-runs printed no run id"; } + + out=$(FM_TEST_RUN_REGISTRY="$reg" "$RUNNER" --stop "$run_id" 2>&1) \ + || { kill -KILL "$runpid" 2>/dev/null; fail "--stop failed: $out"; } + assert_contains "$out" "FM_TEST_STOPPED run_id=$run_id" "--stop must confirm the run it stopped" + + rc=0 + wait "$runpid" || rc=$? + [ "$rc" = 143 ] || fail "a TERMed run must exit 143, got $rc" + + # The whole point of the terminator: a truncated log says so, instead of + # looking like a short clean run to anyone counting failures off it. + assert_grep "FM_TEST_ABORTED " "$tmp/run.log" \ + "aborted run must print the FM_TEST_ABORTED terminator" + assert_grep "run_id=$run_id signal=TERM" "$tmp/run.log" \ + "terminator must name the stopped run and the signal" + assert_no_grep "FM_TEST_SUMMARY " "$tmp/run.log" \ + "an aborted run must not print a summary that reads as a completed suite" + + # A finished run leaves no record for a later stop to act on. + fm_registered_run_count "$reg" 0 || fail "stopped run left a stale registry record" + out=$(FM_TEST_RUN_REGISTRY="$reg" "$RUNNER" --stop "$run_id" 2>&1) && \ + fail "--stop must refuse an unknown run id" + assert_contains "$out" "no live run registered with id" "refusal must name the missing run" + + pass "--stop stops exactly that run and its log identifies itself as aborted" +} + +test_stop_without_id_never_reaches_another_roots_run() { + local tmp reg sibling out rc + tmp=$(fm_test_tmproot fm-test-run-stop-scope) || fail "could not create tmp root" + reg="$tmp/registry" + fm_stop_fixture "$tmp" + + # A second checkout of the runner: same command line, different root. This is + # the concurrent-agent shape that broad pkill cannot distinguish. + sibling="$tmp/sibling" + mkdir -p "$sibling/bin" + cp "$RUNNER" "$sibling/bin/fm-test-run.sh" + + FM_TEST_RUN_REGISTRY="$reg" "$sibling/bin/fm-test-run.sh" "$tmp/slow.test.sh" \ + >"$tmp/sibling.log" 2>&1 & + local sibpid=$! + FM_TEST_RUN_REGISTRY="$reg" "$RUNNER" "$tmp/slow.test.sh" >"$tmp/mine.log" 2>&1 & + local mypid=$! + fm_await 30 fm_registered_run_count "$reg" 2 || { + kill -KILL "$sibpid" "$mypid" 2>/dev/null + fail "both concurrent runs did not register" + } + + out=$(FM_TEST_RUN_REGISTRY="$reg" "$RUNNER" --stop 2>&1) || { + kill -KILL "$sibpid" "$mypid" 2>/dev/null + fail "bare --stop failed: $out" + } + + rc=0 + wait "$mypid" || rc=$? + [ "$rc" = 143 ] || { kill -KILL "$sibpid" 2>/dev/null; fail "own run should have stopped, got $rc"; } + + kill -0 "$sibpid" 2>/dev/null || fail "bare --stop killed another root's run" + assert_no_grep "FM_TEST_ABORTED" "$tmp/sibling.log" "sibling run must be untouched" + + # And the sibling is still individually stoppable by identity. + out=$(FM_TEST_RUN_REGISTRY="$reg" "$RUNNER" --list-runs) + local sib_id + sib_id=$(printf '%s\n' "$out" | sed -n 's/.*run_id=\([^ ]*\).*/\1/p') + FM_TEST_RUN_REGISTRY="$reg" "$RUNNER" --stop "$sib_id" >/dev/null 2>&1 \ + || { kill -KILL "$sibpid" 2>/dev/null; fail "could not stop the sibling by id"; } + rc=0 + wait "$sibpid" || rc=$? + [ "$rc" = 143 ] || fail "sibling stop by id should exit 143, got $rc" + + pass "bare --stop is scoped to this root and never reaches a concurrent run" +} + +test_stop_refuses_when_nothing_is_registered() { + local tmp out + tmp=$(fm_test_tmproot fm-test-run-stop-empty) || fail "could not create tmp root" + + out=$(FM_TEST_RUN_REGISTRY="$tmp/registry" "$RUNNER" --stop 2>&1) \ + && fail "--stop must refuse when this root has no live run" + assert_contains "$out" "no live run registered from this root" \ + "refusal must name the root it searched" + + # The id builds a path this verb removes, so its shape is checked first. + out=$(FM_TEST_RUN_REGISTRY="$tmp/registry" "$RUNNER" --stop '../escape' 2>&1) \ + && fail "--stop must refuse a run id that is not the minted shape" + assert_contains "$out" "not a run id" "refusal must name the expected run-id shape" + + # A record whose process is gone is pruned, never signalled: a recycled pid + # must not inherit a dead run's identity. + mkdir -p "$tmp/registry" + { + printf 'run_id=fm-test-run-stale\n' + printf 'pid=999999\n' + printf 'root=%s\n' "$ROOT" + printf 'selection=scripts\n' + printf 'started=1970-01-01T00:00:00Z\n' + } >"$tmp/registry/fm-test-run-stale.run" + out=$(FM_TEST_RUN_REGISTRY="$tmp/registry" "$RUNNER" --list-runs 2>&1) + assert_not_contains "$out" "fm-test-run-stale" "--list-runs must prune a dead run's record" + [ -f "$tmp/registry/fm-test-run-stale.run" ] && fail "dead run record was not removed" + + pass "--stop refuses with no live run and dead records are pruned" +} + test_list_all_exact_suite_coverage test_family_selection test_single_script_selection @@ -776,3 +934,6 @@ test_portable_serial_shard_lane_refusals test_jobs_requires_proven_isolated test_jobs_parallel_scheduler_and_failure_propagation test_aggregate_json +test_stop_targets_one_run_and_marks_the_log_aborted +test_stop_without_id_never_reaches_another_roots_run +test_stop_refuses_when_nothing_is_registered