From d719ef3d9abdd11b0a8aea57c74de074feca99b9 Mon Sep 17 00:00:00 2001 From: Tiago Date: Fri, 2 Oct 2026 19:36:05 -0300 Subject: [PATCH 1/3] fix(bin): retry ShellCheck roots that hit the memory ceiling without --external-sources (#6443) * fix(lint): retry memory-bound roots without external sources * no-mistakes(review): Make fallback tests portable and correct source-following telemetry * no-mistakes(review): Remove committed parity fixtures and use disposable test roots * no-mistakes(document): Document ShellCheck memory fallback and telemetry * no-mistakes(review): Cover bounded and unbounded fallback RSS behavior * no-mistakes(document): Correct stale lint fallback documentation * no-mistakes(document): Correct stale lint test documentation * no-mistakes(ci): The memory fallback (the retry without --external-sources) now gets only the time left in its root's original deadline, so it can no longer outlast the CI job. Invariant: one root's first attempt plus its fallback must fit inside a single FM_LINT_ROOT_SECONDS deadline, plus the cleanup grace. Only one site started a new deadline: the fallback call in fm_lint_run_root. The deadline is the only budget involved, because the memory limit already applies to each process separately. Changes in bin/fm-lint.sh: - fm_lint_exec_root now takes a argument instead of always reading FM_LINT_INTERNAL_ROOT_SECS. - The first attempt passes the full deadline. - The fallback passes floor((start + deadline - now) / 1000) seconds. - When bounds are enforced and less than 1 second is left, no retry starts. fm_exec_timed rejects 0 seconds, so the retry cannot run with no time. The root keeps reason=memory, and the shard output says "no time left in its Ns deadline to retry without it". - Unbounded local runs have no deadline and behave as before. - The header comment now describes the shared deadline. Changes in tests/fm-lint.test.sh: a new test, test_memory_fallback_spends_only_the_remaining_root_deadline, runs only on hosts that can enforce bounds. It uses a 6 s deadline and 1 s grace. - Case 1: the first attempt runs 3 s and then fails with memory status 251. The test asserts one fallback ran, reported reason=timeout, and the root's recorded duration is under 7000 ms. - Case 2: the first attempt runs 5.2 s. The test asserts no fallback starts, the skip is explained, and the sidecar records memory with source-following 1. Verification: - Full `nice -n 10 bash tests/fm-lint.test.sh` passed, including the new test, in about 5 minutes. - Case 1 run against the HEAD script: the root took 9168 ms, so the under-7000 ms check fails before the fix. - `bin/fm-lint.sh bin/fm-lint.sh tests/fm-lint.test.sh` reported no findings. - The CI workflow is unchanged, so the Test step still runs only tests/fm-lint.test.sh with nice -n 10 and the 12 GiB ShellCheck limit --- bin/fm-lint.sh | 199 +++++++++++++++++++++++--------- docs/fm-test-portable-shards.md | 8 +- tests/fm-lint.test.sh | 186 +++++++++++++++++++++++++++-- 3 files changed, 322 insertions(+), 71 deletions(-) diff --git a/bin/fm-lint.sh b/bin/fm-lint.sh index c58fed9c977..b472af511a3 100755 --- a/bin/fm-lint.sh +++ b/bin/fm-lint.sh @@ -7,13 +7,13 @@ # both use this owner without duplicating lint configuration. # The explicit --fast mode is local-only and disables ShellCheck's extended # dataflow analysis while preserving ordinary shell lint checks and source -# following. CI, main, and merge-base-less runs keep --norc --external-sources -# with full dataflow over the whole canonical set. An ordinary local branch -# (changed-file mode, including the no-mistakes lint step) drops +# following. CI, main, and merge-base-less runs attempt --norc +# --external-sources with full dataflow for each canonical root. An ordinary +# local branch (changed-file mode, including the no-mistakes lint step) drops # --external-sources, keeps dataflow, and excludes SC1091, SC2034, SC2153, -# and SC2329, the codes that need library context. Those codes still run in -# CI over the whole set. Explicit paths keep --external-sources with the -# selected dataflow mode. +# and SC2329, the codes that need library context. CI checks those codes +# on source-following attempts (see the memory fallback below). Explicit +# paths attempt --external-sources with the selected dataflow mode. # Tests stop source analysis at imported production modules because CI analyzes # every production shell separately as a canonical, source-aware root. # The default (no explicit-path) path also runs bin/fm-lint-workflows.sh so a @@ -25,8 +25,8 @@ # - In CI (GITHUB_ACTIONS=true or CI=true), on the main branch, or when no # merge-base against origin/main (or local main) can be found, it lints # the full canonical set: bin/*.sh bin/backends/*.sh tests/*.sh, with -# --external-sources and full dataflow. This is what CI always runs, so -# CI coverage never depends on a local diff. +# --external-sources and full dataflow first. CI coverage never depends +# on a local diff; memory failures may take the narrower retry below. # - Otherwise (an ordinary local branch with a real merge-base) it lints # only the canonical-set files changed since that merge-base, including # uncommitted local edits, via plain local `git diff` (no network, no @@ -50,9 +50,10 @@ # two CI runners, each with those same concurrency-limited workers. # Partitions are complete, disjoint, and byte-weight balanced; --list-files # exposes their actual roots. -# Partition mode is always full source-aware analysis, never changed-only or -# --fast, and does not accept explicit paths. Each partition also runs workflow -# lint and backend-purity checks, keeping either invocation independently useful. +# Partition mode starts with full source-aware analysis, never changed-only +# or --fast, and does not accept explicit paths. Each partition also runs +# workflow lint and backend-purity checks, keeping either invocation +# independently useful. # # With FM_LINT_REQUIRE_BOUNDS=1, which CI sets, every per-root ShellCheck # process runs under an enforced envelope: a wall deadline @@ -71,29 +72,42 @@ # cannot apply the address-space limit at all) each root still runs in its # own ShellCheck process with identical diagnostics, just unbounded. # +# If a source-following root exits with a memory failure, it is retried once +# without --external-sources under the same memory limit and only the time +# left in that root's original deadline; with under a second left, the +# memory failure stands without a retry. A clean retry passes +# with an explicit memory-fallback reason and warning; only the same +# cross-file-dependent codes omitted in local no-source lint are excluded. +# Other findings and failed retries still fail lint. The retry's diagnostics +# replace the failed attempt's output; peak RSS is the maximum of both attempts. +# # Per-root evidence is incremental: workers append begin/end records (root, -# mode, shard, start, end, duration, exit status, reason, and peak RSS when -# measured) to a roots log as each root completes, so a mid-run kill still -# leaves the completed record and names the root in flight as -# begun-but-unfinished. With --telemetry the log is retained at +# mode, shard, start, end, duration, final exit status, reason, peak RSS when +# measured, and whether the final attempt followed sources) to a roots log +# as each root completes, so a mid-run kill still leaves the completed record +# and names the root in flight as begun-but-unfinished. With --telemetry the +# log is retained at # .roots.tsv (or .roots.tsv if there is no # .tsv suffix); otherwise it lives only in the -# run's scratch dir. Reason values are ok, findings, timeout, memory, -# signal:, limit-unavailable, or error:. Memory requires process-level -# evidence (a GHC exhaustion status or runtime error on stderr), not an echoed -# source excerpt or an OOM phrase in a filename. In partition mode begin/end +# run's scratch dir. Reason values are ok, findings, memory-fallback, +# timeout, memory, signal:, limit-unavailable, or error:. +# Memory requires process-level evidence (a GHC exhaustion status or runtime +# error on stderr), not an echoed source excerpt or an OOM phrase in a +# filename. In partition mode begin/end # lines also stream to stderr, and an abnormal root end is always reported # there. # # Optional quiet telemetry writes one bounded TSV snapshot of content and source # graph identity, wall/CPU/RSS, shard load, and competing ShellCheck processes. +# source_followed_directives counts directives only for roots whose final +# attempt followed sources, not roots that passed or failed a no-source retry. # # Usage: # fm-lint.sh lint the context-selected file set (see above) # fm-lint.sh --fast [path]... local lint with extended analysis disabled # fm-lint.sh ... lint explicit roots with the same config # fm-lint.sh --jobs <1|2> [path]... override concurrent worker count -# fm-lint.sh --partition <1of2|2of2> lint one full-rigor canonical CI partition +# fm-lint.sh --partition <1of2|2of2> lint one canonical CI partition (see fallback above) # fm-lint.sh --telemetry ... write a quiet metrics snapshot # fm-lint.sh --required-version print the ShellCheck pin # fm-lint.sh --list-files print the file set that would be linted @@ -101,8 +115,8 @@ set -u REQUIRED_SHELLCHECK=0.11.0 -# Cross-file codes that need --external-sources. Local changed-file mode -# cannot judge them, so they stay CI-only. +# Cross-file codes that need --external-sources. No-source checks (local +# changed-file mode and memory fallback) cannot judge them. LOCAL_NOX_EXCLUDE=SC1091,SC2034,SC2153,SC2329 SELF_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)" SELF="$SELF_DIR/fm-lint.sh" @@ -160,6 +174,17 @@ fm_lint_root_rss() { # printf '%s\n' "${kib:-unavailable}" } +fm_lint_max_root_rss() { # + local first=$1 second=$2 + case "$first" in ''|unavailable|*[!0-9]*) printf '%s\n' "$second"; return ;; esac + case "$second" in ''|unavailable|*[!0-9]*) printf '%s\n' "$first"; return ;; esac + if [ "$first" -gt "$second" ]; then + printf '%s\n' "$first" + else + printf '%s\n' "$second" + fi +} + # Map a root's exit status onto the reported reason vocabulary without # pretending every signal or nonzero exit is a memory kill: only process-level # memory-failure evidence earns the memory reason - GHC's heap-exhaustion @@ -209,24 +234,11 @@ fm_lint_classify_root() { # esac } -# Run one selected root in its own ShellCheck process, record its lifecycle -# in the roots log, and append its diagnostics to the shard output. -fm_lint_run_root() { # - local index=$1 path=$2 output_dir=$3 shard_index=$4 - local root_out="$output_dir/root.$shard_index.$index.out" - local root_err="$output_dir/root.$shard_index.$index.err" - local rss_file="$output_dir/root.$shard_index.$index.rss" - local start_ms end_ms duration_ms invocation_rc=0 reason rss_kib - start_ms=$(fm_lint_now_ms) - if [ -n "${FM_LINT_INTERNAL_ROOTS_LOG:-}" ]; then - printf 'begin\t%s\t%s\t%s\t%s\t%s\n' \ - "$index" "$path" "$shard_index" "${FM_LINT_INTERNAL_MODE:-}" "$start_ms" \ - >> "$FM_LINT_INTERNAL_ROOTS_LOG" - fi - if [ "${FM_LINT_INTERNAL_PROGRESS:-0}" = 1 ]; then - printf 'fm-lint: begin %s (shard %s, %s mode)\n' \ - "$path" "$shard_index" "${FM_LINT_INTERNAL_MODE:-unknown}" >&2 - fi +# Run one ShellCheck invocation under the given deadline and the per-root +# address-space limit, returning its exit status in FM_LINT_LAST_RC. +fm_lint_exec_root() { # + local path=$1 root_out=$2 root_err=$3 rss_file=$4 seconds=$5 invocation_rc=0 + shift 5 if [ "${FM_LINT_INTERNAL_BOUNDED:-none}" != none ]; then # The watchdog runs in a process group of its own (the same setpgrp hop the # workers use), so the owner's TERM-then-KILL group sweep cannot kill it @@ -237,33 +249,105 @@ fm_lint_run_root() { # # the watchdog is still starting is detected too. ( FM_EXEC_TIMED_OWNER_PID=$$ exec "${FM_LINT_PERL_BIN:-perl}" -e 'setpgrp(0, 0) or die "setpgrp: $!"; exec @ARGV or die "exec: $!"' \ "${BASH:-bash}" "$SELF" --internal-timed \ - "$FM_LINT_INTERNAL_ROOT_SECS" "$FM_LINT_INTERNAL_GRACE" \ + "$seconds" "$FM_LINT_INTERNAL_GRACE" \ "${BASH:-bash}" "$SELF" --internal-root "$rss_file" "$FM_LINT_INTERNAL_MEMORY_KIB" \ - "$FM_LINT_SHELLCHECK" "${FM_LINT_WORKER_ARGS[@]}" -- "$path" ) > "$root_out" 2> "$root_err" & + "$FM_LINT_SHELLCHECK" "$@" -- "$path" ) > "$root_out" 2> "$root_err" & FM_LINT_WORKER_RUN_PID=$! wait "$FM_LINT_WORKER_RUN_PID" || invocation_rc=$? FM_LINT_WORKER_RUN_PID= else - "$FM_LINT_SHELLCHECK" "${FM_LINT_WORKER_ARGS[@]}" -- "$path" > "$root_out" 2> "$root_err" & + "$FM_LINT_SHELLCHECK" "$@" -- "$path" > "$root_out" 2> "$root_err" & FM_LINT_WORKER_RUN_PID=$! wait "$FM_LINT_WORKER_RUN_PID" || invocation_rc=$? FM_LINT_WORKER_RUN_PID= fi + FM_LINT_LAST_RC=$invocation_rc +} + +# Run one selected root, retry memory failures without source following, record +# its lifecycle in the roots log, and append the final diagnostics. +fm_lint_run_root() { # + local index=$1 path=$2 output_dir=$3 shard_index=$4 + local root_out="$output_dir/root.$shard_index.$index.out" + local root_err="$output_dir/root.$shard_index.$index.err" + local rss_file="$output_dir/root.$shard_index.$index.rss" + local fallback_out="$output_dir/root.$shard_index.$index.fallback.out" + local fallback_err="$output_dir/root.$shard_index.$index.fallback.err" + local fallback_rss="$output_dir/root.$shard_index.$index.fallback.rss" + local start_ms end_ms duration_ms invocation_rc=0 reason rss_kib initial_rc initial_reason + local fallback_secs + local final_follow_sources=${FM_LINT_INTERNAL_FOLLOW_SOURCES:-1} + local -a fallback_args + start_ms=$(fm_lint_now_ms) + if [ -n "${FM_LINT_INTERNAL_ROOTS_LOG:-}" ]; then + printf 'begin\t%s\t%s\t%s\t%s\t%s\n' \ + "$index" "$path" "$shard_index" "${FM_LINT_INTERNAL_MODE:-}" "$start_ms" \ + >> "$FM_LINT_INTERNAL_ROOTS_LOG" + fi + if [ "${FM_LINT_INTERNAL_PROGRESS:-0}" = 1 ]; then + printf 'fm-lint: begin %s (shard %s, %s mode)\n' \ + "$path" "$shard_index" "${FM_LINT_INTERNAL_MODE:-unknown}" >&2 + fi + fm_lint_exec_root "$path" "$root_out" "$root_err" "$rss_file" \ + "$FM_LINT_INTERNAL_ROOT_SECS" "${FM_LINT_WORKER_ARGS[@]}" + invocation_rc=$FM_LINT_LAST_RC + reason=$(fm_lint_classify_root "$invocation_rc" "$root_err") + initial_rc=$invocation_rc + initial_reason=$reason + # The retry spends what is left of this root's one deadline rather than a + # fresh one, so both attempts together still fit the budget CI sized its job + # timeout around. + fallback_secs=$(( (start_ms + FM_LINT_INTERNAL_ROOT_SECS * 1000 - $(fm_lint_now_ms)) / 1000 )) + if [ "$reason" = memory ] \ + && [ "${FM_LINT_INTERNAL_FOLLOW_SOURCES:-1}" -eq 1 ] \ + && [ "${FM_LINT_INTERNAL_BOUNDED:-none}" != none ] \ + && [ "$fallback_secs" -lt 1 ]; then + printf 'fm-lint: %s hit the memory ceiling with --external-sources (reason=%s rc=%s); no time left in its %ss deadline to retry without it\n' \ + "$path" "$initial_reason" "$initial_rc" "$FM_LINT_INTERNAL_ROOT_SECS" >> "$output_dir/shard.$shard_index.out" + rss_kib=$(fm_lint_root_rss "$rss_file") + cat "$root_out" "$root_err" >> "$output_dir/shard.$shard_index.out" + elif [ "$reason" = memory ] \ + && [ "${FM_LINT_INTERNAL_FOLLOW_SOURCES:-1}" -eq 1 ]; then + fallback_args=() + for arg in "${FM_LINT_WORKER_ARGS[@]}"; do + [ "$arg" = --external-sources ] || fallback_args+=("$arg") + done + [ -z "$LOCAL_NOX_EXCLUDE" ] || fallback_args+=("--exclude=$LOCAL_NOX_EXCLUDE") + fm_lint_exec_root "$path" "$fallback_out" "$fallback_err" "$fallback_rss" \ + "$fallback_secs" "${fallback_args[@]}" + final_follow_sources=0 + invocation_rc=$FM_LINT_LAST_RC + reason=$(fm_lint_classify_root "$invocation_rc" "$fallback_err") + rss_kib=$(fm_lint_max_root_rss \ + "$(fm_lint_root_rss "$rss_file")" "$(fm_lint_root_rss "$fallback_rss")") + printf 'fm-lint: %s hit the memory ceiling with --external-sources (reason=%s rc=%s); retried without it' \ + "$path" "$initial_reason" "$initial_rc" >> "$output_dir/shard.$shard_index.out" + if [ "$reason" = ok ]; then + reason=memory-fallback + invocation_rc=0 + printf '; fallback passed with cross-file codes excluded (%s)\n' "$LOCAL_NOX_EXCLUDE" \ + >> "$output_dir/shard.$shard_index.out" + else + printf '; fallback reason=%s rc=%s\n' "$reason" "$invocation_rc" \ + >> "$output_dir/shard.$shard_index.out" + fi + cat "$fallback_out" "$fallback_err" >> "$output_dir/shard.$shard_index.out" + else + rss_kib=$(fm_lint_root_rss "$rss_file") + cat "$root_out" "$root_err" >> "$output_dir/shard.$shard_index.out" + fi end_ms=$(fm_lint_now_ms) duration_ms=$((end_ms - start_ms)) - rss_kib=$(fm_lint_root_rss "$rss_file") - reason=$(fm_lint_classify_root "$invocation_rc" "$root_err") if [ -n "${FM_LINT_INTERNAL_ROOTS_LOG:-}" ]; then - printf 'end\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ + printf 'end\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ "$index" "$path" "$shard_index" "${FM_LINT_INTERNAL_MODE:-}" \ - "$start_ms" "$end_ms" "$duration_ms" "$invocation_rc" "$reason" "$rss_kib" \ + "$start_ms" "$end_ms" "$duration_ms" "$invocation_rc" "$reason" "$rss_kib" "$final_follow_sources" \ >> "$FM_LINT_INTERNAL_ROOTS_LOG" fi - if [ "${FM_LINT_INTERNAL_PROGRESS:-0}" = 1 ] || { [ "$reason" != ok ] && [ "$reason" != findings ]; }; then + if [ "${FM_LINT_INTERNAL_PROGRESS:-0}" = 1 ] || { [ "$reason" != ok ] && [ "$reason" != findings ] && [ "$reason" != memory-fallback ]; }; then printf 'fm-lint: end %s reason=%s rc=%s duration_ms=%s rss_kib=%s\n' \ "$path" "$reason" "$invocation_rc" "$duration_ms" "$rss_kib" >&2 fi - cat "$root_out" "$root_err" >> "$output_dir/shard.$shard_index.out" return "$invocation_rc" } @@ -1206,7 +1290,11 @@ if [ -n "$TELEMETRY" ]; then : > "$TMP_ROOT/source-targets" source_directives=0 source_boundaries=0 + source_followed=0 + awk -F '\t' '$1 == "end" && $12 == 0 { print $2 }' "$ROOTS_LOG" > "$TMP_ROOT/no-source-indices" + root_index=0 for path in "${ROOTS[@]}"; do + root_index=$((root_index + 1)) if [ -f "$path" ]; then bytes=$(wc -c < "$path" 2>/dev/null | tr -d '[:space:]') case "$bytes" in ''|*[!0-9]*) bytes=0 ;; esac @@ -1219,17 +1307,18 @@ if [ -n "$TELEMETRY" ]; then sub(/[[:space:]].*$/, "", target) print target } - ' "$path" >> "$TMP_ROOT/source-targets" + ' "$path" > "$TMP_ROOT/root-source-targets" + cat "$TMP_ROOT/root-source-targets" >> "$TMP_ROOT/source-targets" + if [ "$FOLLOW_SOURCES" -eq 1 ] \ + && ! grep -qx "$root_index" "$TMP_ROOT/no-source-indices"; then + followed_here=$(grep -cv '^/dev/null$' "$TMP_ROOT/root-source-targets" || true) + source_followed=$((source_followed + followed_here)) + fi fi done source_directives=$(wc -l < "$TMP_ROOT/source-targets" | tr -d '[:space:]') source_boundaries=$(grep -c '^/dev/null$' "$TMP_ROOT/source-targets" 2>/dev/null || true) case "$source_boundaries" in ''|*[!0-9]*) source_boundaries=0 ;; esac - if [ "$FOLLOW_SOURCES" -eq 1 ]; then - source_followed=$((source_directives - source_boundaries)) - else - source_followed=0 - fi source_targets=$(LC_ALL=C sort -u "$TMP_ROOT/source-targets" | wc -l | tr -d '[:space:]') content_cksum=$(cksum "$TMP_ROOT/content-cksums" | awk '{print $1 "-" $2}') git_head=$(git rev-parse HEAD 2>/dev/null || printf 'unavailable') diff --git a/docs/fm-test-portable-shards.md b/docs/fm-test-portable-shards.md index 1e152cda2e9..63b670792f8 100644 --- a/docs/fm-test-portable-shards.md +++ b/docs/fm-test-portable-shards.md @@ -105,11 +105,11 @@ Portable shards, each portable serial shard, and the Herdr lane upload runner-ge ## Lint partitions and end-to-end latency -`bin/fm-lint.sh` owns two canonical CI partitions, each running full source-aware ShellCheck analysis, workflow validation, and backend-purity checks. -CI requires its per-root bounds, so an unenforceable deadline or address-space limit refuses lint rather than running uncapped; the script header owns the envelope and per-root execution contract. -Its `--list-files` interface exposes partition membership; `tests/fm-lint.test.sh` verifies complete/disjoint executed roots and unchanged analysis flags. +`bin/fm-lint.sh` owns two canonical CI partitions, each attempting full source-aware ShellCheck analysis and running workflow validation and backend-purity checks. +CI requires its per-root bounds, so an unenforceable deadline or address-space limit refuses lint rather than running uncapped; the script header owns the envelope, per-root execution contract, and memory fallback. +Its `--list-files` interface exposes partition membership; `tests/fm-lint.test.sh` verifies complete/disjoint executed roots, initial analysis flags, and fallback reporting. The workflow uploads each partition's quiet telemetry plus its per-root lifecycle sidecar to distinguish analysis cost, memory use, and host contention. -No fast mode, path skips, reduced checks, or paid runner provisioning is part of this layout. +No fast mode, path skips, or paid runner provisioning is part of this layout. The longer-term performance objective remains a complete green run under fifteen minutes including start delay, but the current watch-triage floor alone exceeds that objective. The immediate packing target is the runner's modeled script budget, not a claim that more shards alone can make an indivisible script faster. diff --git a/tests/fm-lint.test.sh b/tests/fm-lint.test.sh index 66a6967ed91..624f916b6d8 100755 --- a/tests/fm-lint.test.sh +++ b/tests/fm-lint.test.sh @@ -3,10 +3,9 @@ # # bin/fm-lint.sh is the single owner invoked by CI # (.github/workflows/ci.yml) and by the pre-push gate (.no-mistakes.yaml -# commands.lint). CI runs its two full-rigor canonical partitions; the local -# gate uses its context-selected default. Their selection differs deliberately, -# while this owner keeps analysis flags, configuration, and tool versions from -# drifting. +# commands.lint). CI and the local gate deliberately select different roots; +# bin/fm-lint.sh owns their analysis modes, memory fallback, configuration, +# and tool versions. # Regression origin: with no commands.lint configured, the local no-mistakes # lint step never ran the deterministic shell lint, so PRs passed local # validation yet failed CI on info/warning findings such as SC2015, SC1007, and @@ -1532,6 +1531,172 @@ test_root_memory_limit_reports_a_named_death() { pass "a root refused by its enforced memory limit fails by name with a memory reason" } +test_memory_failure_retries_without_external_sources() { + local tmp fakebin fixture out rc log rss_kib require_bounds=0 mode + local -a modes=(0) + if fm_lint_bounds_supported; then + require_bounds=1 + modes=(1 0) + fi + tmp=$(fm_test_tmproot fm-lint-memory-fallback) + fakebin=$(fm_fakebin "$tmp") + fixture="$tmp/teardown.sh" + log="$tmp/flags.log" + printf '#!/usr/bin/env bash\n# shellcheck source=lib.sh\nexit 0\n' > "$fixture" + cat > "$fakebin/shellcheck" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --version ]; then + printf 'ShellCheck - shell script analysis tool\nversion: 0.11.0\n' + exit 0 +fi +follow=no +exclude=none +while [ "$#" -gt 0 ] && [ "$1" != -- ]; do + case "$1" in + --external-sources) follow=yes ;; + --exclude=*) exclude=${1#--exclude=} ;; + esac + shift +done +shift +printf '%s\t%s\n' "$follow" "$exclude" >> "$FM_TEST_FALLBACK_LOG" +if [ "$follow" = yes ]; then + printf 'shellcheck: Heap exhausted;\n' >&2 + exit 251 +fi +exit 0 +SH + chmod +x "$fakebin/shellcheck" + + for mode in "${modes[@]}"; do + : > "$log" + rc=0 + out=$(PATH="$fakebin:$PATH" FM_LINT_JOBS=1 FM_LINT_REQUIRE_BOUNDS="$mode" \ + FM_TEST_FALLBACK_LOG="$log" "$LINT" --telemetry "$tmp/pass.$mode.tsv" "$fixture" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "a clean no-source fallback did not pass (bounded=$mode)"$'\n'"$out" + assert_grep $'source_directives\t1' "$tmp/pass.$mode.tsv" "telemetry lost the root's source directive" + assert_grep $'source_followed_directives\t0' "$tmp/pass.$mode.tsv" \ + "telemetry counted a source directive that the passing fallback did not follow" + [ "$(cat "$log")" = "$(printf 'yes\tnone\nno\tSC1091,SC2034,SC2153,SC2329')" ] \ + || fail "the memory failure did not retry without external sources and exclude only cross-file codes"$'\n'"$(cat "$log")" + assert_contains "$out" "hit the memory ceiling with --external-sources (reason=memory rc=251)" \ + "the fallback was not identified in the output" + assert_contains "$out" "fallback passed with cross-file codes excluded (SC1091,SC2034,SC2153,SC2329)" \ + "the narrower fallback result was not disclosed" + awk -F '\t' '$1 == "end" && $3 ~ /teardown\.sh$/ && $9 == 0 && $10 == "memory-fallback" { found=1 } END { exit !found }' \ + "$tmp/pass.$mode.roots.tsv" || fail "the clean fallback was not recorded distinctly" + rss_kib=$(awk -F '\t' '$1 == "end" && $3 ~ /teardown\.sh$/ { print $11 }' "$tmp/pass.$mode.roots.tsv") + if [ "$mode" -eq 1 ]; then + assert_grep $'meta\tbounds_enforced\t1' "$tmp/pass.$mode.roots.tsv" \ + "the bounded fallback did not enforce bounds" + case "$rss_kib" in ''|*[!0-9]*) fail "the fallback attempts lost per-root RSS reporting: $rss_kib" ;; esac + else + assert_grep $'meta\tbounds_enforced\t0' "$tmp/pass.$mode.roots.tsv" \ + "the unbounded fallback unexpectedly enforced bounds" + [ "$rss_kib" = unavailable ] || fail "the unbounded fallback unexpectedly reported RSS: $rss_kib" + fi + done + + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): real fallback finding check" + return + fi + local real_shellcheck + real_shellcheck=$(command -v shellcheck) + cat > "$fakebin/shellcheck" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --version ]; then + exec "$FM_REAL_SHELLCHECK" "$@" +fi +for arg in "$@"; do + if [ "$arg" = --external-sources ]; then + printf 'shellcheck: Heap exhausted;\n' >&2 + exit 251 + fi +done +exec "$FM_REAL_SHELLCHECK" "$@" +SH + chmod +x "$fakebin/shellcheck" + # shellcheck disable=SC2016 # The fixture intentionally contains an unexpanded parameter. + printf '#!/usr/bin/env bash\nx=$1\nprintf "%%s\\n" $x\n' > "$fixture" + rc=0 + out=$(PATH="$fakebin:$PATH" FM_REAL_SHELLCHECK="$real_shellcheck" \ + FM_LINT_JOBS=1 FM_LINT_REQUIRE_BOUNDS="$require_bounds" \ + "$LINT" --telemetry "$tmp/finding.tsv" "$fixture" 2>&1) || rc=$? + [ "$rc" -eq 1 ] || fail "a real ShellCheck finding in the fallback did not fail lint (exit $rc)"$'\n'"$out" + assert_contains "$out" "fallback reason=findings rc=1" \ + "the fallback finding was not identified" + assert_contains "$out" "SC2086" "the real fallback finding was not reported" + awk -F '\t' '$1 == "end" && $3 ~ /teardown\.sh$/ && $9 == 1 && $10 == "findings" { found=1 } END { exit !found }' \ + "$tmp/finding.roots.tsv" || fail "the fallback finding was not recorded as a failure" + pass "memory failures retry without source following, exclude cross-file codes, and preserve a real fallback finding" +} + +test_memory_fallback_spends_only_the_remaining_root_deadline() { + if ! fm_lint_bounds_supported; then + pass "SKIP (host cannot enforce the bounded envelope): fallback deadline check" + return + fi + local tmp fakebin fixture log out rc duration_ms + tmp=$(fm_test_tmproot fm-lint-fallback-deadline) + fakebin=$(fm_fakebin "$tmp") + fixture="$tmp/teardown.sh" + log="$tmp/attempts.log" + printf '#!/usr/bin/env bash\nexit 0\n' > "$fixture" + cat > "$fakebin/shellcheck" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --version ]; then + printf 'ShellCheck - shell script analysis tool\nversion: 0.11.0\n' + exit 0 +fi +for arg in "$@"; do + if [ "$arg" = --external-sources ]; then + printf 'follow\n' >> "$FM_TEST_ATTEMPT_LOG" + sleep "$FM_TEST_FIRST_SECS" + printf 'shellcheck: Heap exhausted;\n' >&2 + exit 251 + fi +done +printf 'fallback\n' >> "$FM_TEST_ATTEMPT_LOG" +sleep 60 +exit 0 +SH + chmod +x "$fakebin/shellcheck" + + # A 3s first attempt leaves about 3s of the 6s deadline, so the retry is + # killed there; a fresh deadline would let the root run for about 9s. + : > "$log" + rc=0 + out=$(PATH="$fakebin:$PATH" FM_LINT_JOBS=1 FM_LINT_REQUIRE_BOUNDS=1 \ + FM_LINT_ROOT_SECONDS=6 FM_LINT_ROOT_GRACE=1 \ + FM_TEST_ATTEMPT_LOG="$log" FM_TEST_FIRST_SECS=3 \ + "$LINT" --telemetry "$tmp/partial.tsv" "$fixture" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "a fallback cut off by the root deadline unexpectedly passed" + [ "$(cat "$log")" = "$(printf 'follow\nfallback')" ] \ + || fail "the root did not retry once with its remaining time"$'\n'"$(cat "$log")" + assert_contains "$out" "fallback reason=timeout" \ + "the fallback was not stopped by the root's remaining deadline"$'\n'"$out" + duration_ms=$(awk -F '\t' '$1 == "end" && $3 ~ /teardown\.sh$/ { print $8 }' "$tmp/partial.roots.tsv") + [ "$duration_ms" -lt 7000 ] \ + || fail "the first attempt and fallback together exceeded the root deadline plus grace: ${duration_ms}ms" + + # With under a second of the deadline left, no retry starts. + : > "$log" + rc=0 + out=$(PATH="$fakebin:$PATH" FM_LINT_JOBS=1 FM_LINT_REQUIRE_BOUNDS=1 \ + FM_LINT_ROOT_SECONDS=6 FM_LINT_ROOT_GRACE=1 \ + FM_TEST_ATTEMPT_LOG="$log" FM_TEST_FIRST_SECS=5.2 \ + "$LINT" --telemetry "$tmp/spent.tsv" "$fixture" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "a memory failure with no deadline left unexpectedly passed" + [ "$(cat "$log")" = follow ] \ + || fail "a fallback started with no time left in the root deadline"$'\n'"$(cat "$log")" + assert_contains "$out" "no time left in its 6s deadline to retry without it" \ + "the skipped fallback was not explained"$'\n'"$out" + awk -F '\t' '$1 == "end" && $3 ~ /teardown\.sh$/ && $10 == "memory" && $12 == 1 { found=1 } END { exit !found }' \ + "$tmp/spent.roots.tsv" || fail "the unretried memory failure was not recorded as a source-following memory failure" + pass "a memory fallback runs only within the time left in its root's original deadline" +} + test_memory_evidence_outranks_findings_and_signal_reasons() { local tmp fakebin roots_log out rc name reason bounded local -a roots modes @@ -1800,13 +1965,8 @@ test_seeded_module_boundary_parity() { pass "SKIP (ShellCheck $REQUIRED not resolved): seeded source-boundary parity check" return fi - local tmp rel adapter dispatcher dep owner test_root out rc - tmp=$(mktemp -d "$ROOT/.fm-lint-parity.XXXXXX") - if [ "${#FM_TEST_CLEANUP_DIRS[@]}" -eq 0 ]; then - trap fm_test_cleanup EXIT - fi - FM_TEST_CLEANUP_DIRS+=("$tmp") - rel=${tmp#"$ROOT/"} + local tmp adapter dispatcher dep owner test_root out rc + tmp=$(fm_test_tmproot fm-lint-parity) adapter="$tmp/adapter.sh" dispatcher="$tmp/dispatcher.sh" dep="$tmp/owner-dep.sh" @@ -1834,7 +1994,7 @@ owner_dependency_value=ok SH cat > "$owner" < Date: Sat, 3 Oct 2026 10:16:13 +0800 Subject: [PATCH 2/3] fix(pi): restore watcher continuity across successor gaps and make extension log opt-in (#5489) * Fix Pi watcher successor-gap confirmations and add extension log Accept an already-acknowledged handling confirmation as a no-op when the generation matches, confirm the restoration's own recovery token with a superseded (not rejected) outcome on generation mismatch, retire an arm on confirm failure only when the failed token names that exact pid, and record restore attempts, readiness timeouts, and confirm results in the bounded state/.watch-extension.log. Regression tests: already-acked no-op plus mismatch/dead-pid/lock-mismatch rejections and the manual-restart churn contract in fm-watch-arm.test.sh, and a mid-restore marker advance with no rejection appendix in fm-pi-watch-extension.test.sh. * Treat a dead arm child as an empty slot so repair and retry recover startArm and scheduleRetry answered unchanged while holding a ChildProcess whose OS process was already gone but whose close had not fired, so neither the repair tool nor the retry timer started anything until that close fired. Gate slot occupancy on a liveness check (exit/signal codes plus pid probe) and start a fresh arm instead, with a regression test driving the repair tool against a dead-but-unclosed child. * no-mistakes(document): Document new Pi extension log knob * no-mistakes(review): Fix confirm-failure retire token match, add distinct-pid test * no-mistakes(document): Clarify retire guard needs pid and generation * Make the Pi extension diagnostic log opt-in and default-off Only a positive FM_WATCH_EXTENSION_LOG_KEEP_LINES enables state/.watch-extension.log. Unset, empty, non-numeric, zero, and negative values disable logging entirely, so the default run writes nothing and never creates the file. The shared positiveInteger fallback semantics stay untouched for the retry and timeout knobs. docs/configuration.md owns the knob contract and docs/watcher-continuity.md points at it. Tests: the superseded-delivery case runs opted in, and a new case proves unset, zero, and non-numeric values create no log file while delivery still succeeds. * no-mistakes(document): Qualify extension-log coverage bullet as opt-in * no-mistakes(ci): The two reported checks (CI run 36372002913, Require no-mistakes run 36372069208) show conclusion action_required with 0 jobs and no logs because this is a fork PR (RibatTRW/firstmate) and GitHub is holding the workflow runs awaiting maintainer approval; that gate is external to the code and needs a maintainer to approve the runs. While verifying the change locally I found a real defect the approved CI run would hit: the PR's new test test_handling_delivered_rejects_a_superseded_generation failed deterministically. Invariant violated: reopen-announced (a non-successor/manual arm start) mints a fresh recovery generation only when the durable wake queue holds unrecovered work; an announced episode with an empty queue must be left untouched so idle arm starts never churn generations (the [ -s queue ] guard from #4819, relied on by bin/fm-watch.sh:2413 and covered by the append-reopens and announcement-bound sibling tests). The test called reopen with an empty queue and expected churn, so the fix establishes the queued-work precondition (append one wake, re-announce, re-read the generation) before asserting the reopen mints and the old confirmation mismatches. Test-only change, 14 lines in tests/fm-watch-arm.test.sh. Verified: fm-watch-arm.test.sh 25/25 ok on two consecutive runs, fm-pi-watch-extension.test.sh 55/55 ok, and shellcheck reports only one pre-existing warning outside the edited region * no-mistakes(document): Restore blank line in watcher-continuity docs * Route superseded Pi deliveries like confirmed ones and cover the retire guard A superseded handling confirmation now falls through to the normal delivery path, so an accepting supervision branch owns the wake instead of main. Scope the watcher-continuity token-pinned confirmation and narrowed retire rule to Pi, since omp and OpenCode still confirm against the current successor. Add tests that fail when the retire guard, the scheduled-retry gate, or the deferred-close gate is reverted, relabel the churned-generation characterization test, and use a reaped pid for the dead-pid rejection. --- .pi/extensions/fm-primary-pi-watch.ts | 116 +++- bin/fm-wake-lib.sh | 11 + docs/configuration.md | 1 + docs/watcher-continuity.md | 17 +- tests/fm-pi-watch-extension.test.sh | 783 ++++++++++++++++++++++++++ tests/fm-watch-arm.test.sh | 116 ++++ 6 files changed, 1024 insertions(+), 20 deletions(-) diff --git a/.pi/extensions/fm-primary-pi-watch.ts b/.pi/extensions/fm-primary-pi-watch.ts index c7f1605dba7..51dad37a20a 100644 --- a/.pi/extensions/fm-primary-pi-watch.ts +++ b/.pi/extensions/fm-primary-pi-watch.ts @@ -149,6 +149,8 @@ const armScript = `${fmRoot}/bin/fm-watch-arm.sh`; const marker = `${state}/.pi-watch-extension-loaded`; const handoffDir = `${state}/extensions/pi-primary-watch`; const actionableHandoff = `${handoffDir}/session-replacement-actionable.json`; +const extensionLog = `${state}/.watch-extension.log`; +const extensionLogMaxLines = extensionLogKeepLines(); const extensionVersion = `sha256:${createHash("sha256").update(readFileSync(extensionFile)).digest("hex")}`; const retryBaseMs = positiveInteger("FM_WATCH_REARM_RETRY_BASE_MS", 250); const retryMaxMs = positiveInteger("FM_WATCH_REARM_RETRY_MAX_MS", 4000); @@ -213,6 +215,18 @@ function positiveInteger(name: string, fallback: number): number { return Math.floor(value); } +// Opt-in bound for the extension diagnostic log: only a positive +// FM_WATCH_EXTENSION_LOG_KEEP_LINES enables logging, so the default run +// writes nothing. Unset, empty, non-numeric, zero, and negative values +// disable the log entirely instead of falling back to a silent default. +function extensionLogKeepLines(): number { + const raw = process.env.FM_WATCH_EXTENSION_LOG_KEEP_LINES; + if (raw === undefined || raw.trim() === "") return 0; + const value = Math.floor(Number(raw)); + if (!Number.isFinite(value) || value <= 0) return 0; + return value; +} + function parentPid(pid: string): string { const result = spawnSync("ps", ["-o", "ppid=", "-p", pid], { encoding: "utf8" }); if (result.status !== 0) return ""; @@ -228,6 +242,21 @@ function pidAlive(pid: string): boolean { } } +// An arm child whose process is gone but whose close event has not fired yet +// (stdio pipes still held) must not keep the single-flight slot: neither a +// repair call nor a scheduled retry would start anything until that close +// finally fires. Callers that gate on slot occupancy use this instead of +// owner.child so both paths can always recover. + +function liveArmChild(owner: SessionGeneration): ChildProcess | null { + const child = owner.child; + if (!child) return null; + if (child.exitCode !== null || child.signalCode !== null) return null; + const pid = child.pid; + if (pid === undefined || !pidAlive(String(pid))) return null; + return child; +} + function lockOwnership(): LockOwnership { let lockPid = ""; try { @@ -314,6 +343,35 @@ function nodeErrorCode(error: unknown): string { : ""; } +// Bounded diagnostic record for restore attempts, readiness timeouts, and +// handling-confirmation targets and results. Opt-in through +// FM_WATCH_EXTENSION_LOG_KEEP_LINES and off by default: a disabled log +// returns before touching the filesystem, so it never creates its file. +// Purely observational: a logging failure never changes supervision +// behavior. docs/watcher-continuity.md owns what the arm layer already +// records; this file is the extension side. +function appendExtensionLog(detail: string): void { + if (extensionLogMaxLines <= 0) return; + try { + mkdirSync(state, { recursive: true }); + const cleaned = detail.replace(/[\r\n\t]+/g, " ").slice(0, 512); + const line = `${new Date().toISOString()} pid=${process.pid} ${cleaned}`; + let previous = ""; + try { + previous = readFileSync(extensionLog, "utf8"); + } catch (error) { + if (nodeErrorCode(error) !== "ENOENT") return; + } + const joined = `${previous}${previous === "" || previous.endsWith("\n") ? "" : "\n"}${line}\n`; + const kept = joined.split("\n").slice(-(extensionLogMaxLines + 1)).join("\n"); + const temporary = `${extensionLog}.tmp-${process.pid}`; + writeFileSync(temporary, kept, { mode: 0o600 }); + renameSync(temporary, extensionLog); + } catch { + // Diagnostic only: never fail supervision for observability. + } +} + function createPendingActionable(message: string, predecessorArmPid: string): PendingActionableClose { return { version: 1, @@ -611,6 +669,7 @@ export default function (pi: ExtensionAPI) { function confirmHandlingDelivery(recovery: { generation: string; watcherPid: string }): { ok: boolean; detail: string; + superseded?: boolean; } { try { const result = spawnSync( @@ -623,6 +682,11 @@ export default function (pi: ExtensionAPI) { }, ); if (result.status === 0) return { ok: true, detail: "" }; + if (result.status === 3) { + // The marker advanced past this restoration's generation mid-restore, + // so a newer pipeline owns the episode now: superseded, not rejected. + return { ok: false, detail: "", superseded: true }; + } const stderr = (result.stderr || "").trim(); return { ok: false, @@ -638,16 +702,15 @@ export default function (pi: ExtensionAPI) { } function confirmHandlingDeliveryWithRetry( - owner: SessionGeneration, recovery: { generation: string; watcherPid: string }, - ): { ok: boolean; detail: string } { - const snapshot = (): { generation: string; watcherPid: string } => { - const current = owner.child ? armRecovery.get(owner.child) : undefined; - return current ?? recovery; - }; - const first = confirmHandlingDelivery(snapshot()); - if (first.ok) return first; - return confirmHandlingDelivery(snapshot()); + ): { ok: boolean; detail: string; superseded?: boolean } { + // Confirm the restoration's own recovery token, never a fresh snapshot of + // the current arm child: a successor replaced during the restore window + // must not turn this delivery into a false rejection, and a retry must + // not retire a newer healthy watcher. + const first = confirmHandlingDelivery(recovery); + if (first.ok || first.superseded) return first; + return confirmHandlingDelivery(recovery); } function offerWakeToBranch(message: string): Promise | null { @@ -668,11 +731,25 @@ export default function (pi: ExtensionAPI) { ): Promise { if (!generationIsLive(owner)) return false; if (recovery) { - const confirmed = confirmHandlingDeliveryWithRetry(owner, recovery); - if (!confirmed.ok) { - const watcherPid = recovery.watcherPid; - if (!pidAlive(watcherPid)) { - await retireArm(owner.child); + const confirmed = confirmHandlingDeliveryWithRetry(recovery); + appendExtensionLog( + `confirm generation=${recovery.generation} watcherPid=${recovery.watcherPid} result=${confirmed.ok ? "confirmed" : confirmed.superseded ? "superseded" : "rejected"}`, + ); + // A superseded result means a newer pipeline owns this episode now: it + // routes like a confirmed delivery below, with no failure appended, and + // retires nothing. + if (!confirmed.ok && !confirmed.superseded) { + const failedPid = recovery.watcherPid; + const current = owner.child; + const currentRecovery = current ? armRecovery.get(current) : undefined; + if ( + current && + currentRecovery?.watcherPid === failedPid && + currentRecovery?.generation === recovery.generation && + !pidAlive(failedPid) + ) { + appendExtensionLog(`retire pid=${failedPid} reason=confirm-failure`); + await retireArm(current); } return await sendWake(owner, `${message}\n\n${confirmed.detail}`, pending); } @@ -850,7 +927,7 @@ export default function (pi: ExtensionAPI) { // been idle. const deferred = owner.deferredClose; owner.deferredClose = null; - if (deferred && !owner.child && !owner.retryTimer) { + if (deferred && !liveArmChild(owner) && !owner.retryTimer) { scheduleRetry(owner, deferred.message, deferred.predecessorArmPid); } } @@ -912,10 +989,14 @@ export default function (pi: ExtensionAPI) { if (!generationIsLive(owner)) return { failure: "" }; const replacement = startArm(owner, predecessorArmPid); const successorChild = owner.child; + appendExtensionLog( + `restore attempt=${attempt} predecessor=${predecessorArmPid || "none"} start=${replacement.ok ? `ok pid=${successorChild?.pid ?? "none"}` : "failed"}`, + ); if (replacement.ok && successorChild && await waitForReadiness(successorChild)) { return { failure: "", recovery: armRecovery.get(successorChild) }; } if (replacement.ok) { + appendExtensionLog(`restore attempt=${attempt} readiness=timeout pid=${successorChild?.pid ?? "none"}`); failure = "watcher: FAILED - Pi extension could not verify a ready successor watcher"; if (!(await retireArm(successorChild))) { return { @@ -931,11 +1012,12 @@ export default function (pi: ExtensionAPI) { if (attempt === retryLimit) break; await waitForRetry(attempt + 1); } + appendExtensionLog(`restore exhausted attempts=${retryLimit + 1} outcome=hand-to-main`); return { failure: `${failure}\nwatcher: FAILED - Pi extension could not restore watcher continuity after ${retryLimit} retries` }; } function scheduleRetry(owner: SessionGeneration, message: string, predecessorArmPid: string): void { - if (!generationIsLive(owner) || owner.child || owner.retryTimer) return; + if (!generationIsLive(owner) || liveArmChild(owner) || owner.retryTimer) return; const ownership = lockOwnership(); if (ownership !== "owned") { surfaceFailure(owner, `watcher: FAILED - Pi extension cannot restore continuity because this session no longer owns the lock\n${message}`); @@ -969,7 +1051,7 @@ export default function (pi: ExtensionAPI) { }; } publishGenerationOwner(owner, "active"); - if (owner.child) { + if (liveArmChild(owner)) { return { ok: true, message: `watcher: unchanged - Pi extension already owns an arm child; no manual re-arm needed; ${repairOnlyHint}`, diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index dfa387079b0..ddc708aadc8 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -813,6 +813,17 @@ _fm_recovery_marker_begin_handling() { fi case "$line" in pending:handling:*|announced:handling:*) ;; + acked:handling:*|acked:downtime:*) + # An already-retired episode confirms as a no-op when the caller names + # its generation: the drain acknowledged it after the successor started + # but before the delivery confirmation ran. Without a named generation + # there is nothing to match, so keep the rejection. + # docs/watcher-continuity.md owns the recovery-episode contract. + if [ -z "$expected_generation" ]; then + fm_lock_release "$lock" + return 1 + fi + ;; pending:downtime:*) if ! _fm_recovery_marker_write_locked "$marker" handling "$generation"; then fm_lock_release "$lock" diff --git a/docs/configuration.md b/docs/configuration.md index 3bf8ffee64b..5538d3ed6a0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -2351,6 +2351,7 @@ FM_WATCH_REARM_RETRY_MAX_MS=4000 # Pi/OpenCode adapter cap for exponential con FM_WATCH_REARM_RETRY_LIMIT=5 # Pi/OpenCode adapter launch-failure retries before surfacing restoration failure FM_WATCH_CYCLE_LOG_MAX_BYTES=262144 # size cap for the arm-owned watcher lifecycle ledger FM_WATCH_CYCLE_LOG_KEEP_LINES=1000 # newest complete lifecycle rows considered when the ledger is capped +FM_WATCH_EXTENSION_LOG_KEEP_LINES=0 # opt-in Pi extension diagnostic log (state/.watch-extension.log); unset, empty, non-numeric, zero, or negative disables logging, a positive value keeps that many newest rows; logging never changes supervision behavior FM_WATCHER_STALE_GRACE=300 # defaults to FM_GUARD_GRACE if set, else the poll-derived grace (docs/turnend-guard.md "Guard grace and the poll cadence"); seconds before a fresh arm refuses a live holder's stale beacon (attached arms: FM_WATCHER_STALL_BOUND) FM_WATCHER_STALL_BOUND= # live-holder stall bound; default and arm/re-arm behavior: docs/turnend-guard.md "Guard grace and the poll cadence" FM_SIGNAL_GRACE=30 # seconds to coalesce nearby status and turn-end signals into one wake diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index 9d9e19dadd5..f14717596b4 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -42,6 +42,7 @@ Each adapter: - Preserves one child or scheduled retry at a time. - Applies bounded exponential retry after an unexpected or failed close. +Pi treats an arm child whose process is already gone as an empty slot even while its close event is still pending, so a repair call or a scheduled retry starts a fresh arm instead of answering unchanged. A failed follow-up never cancels continuity restoration. ### Pi session replacement @@ -133,14 +134,19 @@ After an actionable Pi, omp, or OpenCode child close, the adapter: 1. Waits for the predecessor process to close. 2. Starts and verifies one singleton successor. -3. Confirms the handling handoff against that successor before scheduling the follow-up. +3. Confirms the handling handoff before scheduling the follow-up: Pi confirms against the restoration's own recovery token, while omp and OpenCode confirm against the current successor. 4. Delivers the original wake. A complete Pi reason line can be observed while the predecessor is still finishing durable cleanup. That line is retained for replacement handoff, but the adapter never treats that already-ready predecessor as its own successor. -If the handoff confirmation fails, the adapter retries it once against the current generation and successor. -A failed confirmation is a restoration failure: the adapter classifies the error, retires a successor that is no longer alive, and surfaces exactly one typed message. +If the handoff confirmation fails, the adapter retries it once: Pi against that same token, omp and OpenCode against the current generation and successor. +A failed confirmation is a restoration failure: the adapter classifies the error and surfaces exactly one typed message. +Pi retires the current successor only when the failed token names its exact watcher pid and generation and that pid is no longer alive, while omp and OpenCode retire the current successor whenever the restoration's watcher pid is no longer alive. +On Pi a generation mismatch means a newer pipeline superseded this delivery mid-restore, so the wake routes like a confirmed delivery, with no failure appendix, and nothing is retired. +An already-acknowledged episode confirms as a no-op when the confirmation names its generation, because the drain acknowledged it after the successor started but before the confirmation ran. +The Pi extension diagnostic log is opt-in and off by default: only a positive FM_WATCH_EXTENSION_LOG_KEEP_LINES value appends restore attempts, readiness timeouts, and confirmation targets and results to state/.watch-extension.log, a bounded record that never changes supervision behavior. +docs/configuration.md owns the knob's default and accepted values. A failed confirmation is never swallowed. ### Readiness timeout and retry @@ -425,6 +431,9 @@ The same suite covers ordinary same-process session replacement for `/new`, `/re - Repeated transitions with exactly one live cycle. - Disappearance of the shutting-down refusal after a valid replacement activates. - Terminal quit still refusing late rearm. +- A mid-restore marker advance that delivers the wake with no rejection appendix, offers it to an accepting supervision branch like a confirmed delivery, and records the attempt and the confirm result in the bounded extension log when opted in. +- A failed confirmation for a stale successor that spares a newer arm started by a repair. +- A repair, a scheduled retry, and a deferred close over a dead-but-unclosed arm child that each start a fresh arm instead of stalling. The guard and session-start suites prove that active generation evidence tolerates a fresh-beacon handoff. They also prove that a legacy or handoff-phase watcher marker from an absent replacement extension still raises the outage diagnostic. @@ -444,6 +453,8 @@ They also prove that a legacy or handoff-phase watcher marker from an absent rep - A watcher close inside the handling window that must leave the printed acknowledgement valid. - A re-arm whose recovery cycle is slowed after confirmation and must still surface rather than read as a watcher that stayed live. - The self-healing moved-generation acknowledgement that consumes its handled rows and names its remedy. +- The already-acknowledged confirmation no-op for a matching generation, with its mismatched-generation, dead-pid, and lock-mismatch rejections preserved. +- The manual-restart generation churn that makes a confirmation for the churned generation report a mismatch, which an arm check without a reopen leaves in place. - A take-over that stays quiet after a confirmed TERM, still surfaces queued work and self-exit downtime, and attaches without stopping a cycle the named arm does not own. - The disposable-checkout arm refusal. - The home-gone and state-gone watcher exits. diff --git a/tests/fm-pi-watch-extension.test.sh b/tests/fm-pi-watch-extension.test.sh index f9d3af76f0c..1d810c59926 100755 --- a/tests/fm-pi-watch-extension.test.sh +++ b/tests/fm-pi-watch-extension.test.sh @@ -1660,6 +1660,781 @@ EOF pass "Pi refused handling handshake is classified and not swallowed" } +test_pi_confirm_failure_retires_arm_with_distinct_watcher_pid() { + local repo home plugin log stop retired out status + repo="$TMP_ROOT/pi-confirm-distinct-root" + home="$TMP_ROOT/pi-confirm-distinct-home" + log="$TMP_ROOT/pi-confirm-distinct.log" + stop="$TMP_ROOT/pi-confirm-distinct.stop" + retired="$TMP_ROOT/pi-confirm-distinct.retired" + mkdir -p "$repo/bin" "$home/state" "$home/config" + install_pi_watch_extension_fixture "$repo" + plugin="$repo/.pi/extensions/fm-primary-pi-watch.ts" + cat > "$repo/bin/fm-watch-arm.sh" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --handling-delivered ]; then + printf 'refused generation=%s watcher=%s\n' "$2" "$4" >> "${FM_ARM_LOG:?}" + exit 1 +fi +printf 'arm=%s predecessor=%s\n' "$$" "${FM_WATCH_PREDECESSOR_ARM_PID:-none}" >> "${FM_ARM_LOG:?}" +count=$(grep -c '^arm=' "$FM_ARM_LOG") +if [ "$count" -eq 1 ]; then + printf 'watcher: started pid=%s (beacon fresh)\n' "$$" + printf 'signal: synthetic actionable close\n' + exit 0 +fi +sleep 0.02 & dead=$!; wait "$dead" 2>/dev/null || true +if [ "$dead" = "$$" ]; then dead=1; fi +printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-distinct\n' "$dead" +trap 'printf "retired\n" > "${FM_RETIRED_FILE:?}"; exit 0' TERM INT +while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done +SH + chmod +x "$repo/bin/fm-watch-arm.sh" + out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" FM_RETIRED_FILE="$retired" node --input-type=module 2>&1 <<'EOF' +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +let tool = null; +let prompt = ""; +const pi = { + on() {}, + registerCommand() {}, + registerTool(candidate) { + if (candidate.name === "fm_watch_arm_pi") tool = candidate; + }, + sendUserMessage: async (message) => { + prompt += message; + }, +}; +writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); +const mod = await import(pathToFileURL(process.env.PLUGIN).href); +mod.default(pi); +await tool.execute("tool-call-confirm-distinct", {}, undefined, undefined, {}); +for (let i = 0; i < 250 && !prompt.includes("handling delivery confirmation was rejected"); i += 1) { + await new Promise((resolve) => setTimeout(resolve, 20)); +} +if (!prompt.includes("handling delivery confirmation was rejected")) { + throw new Error(`failed handshake was swallowed: ${prompt}`); +} +let retired = false; +for (let i = 0; i < 250 && !retired; i += 1) { + await new Promise((resolve) => setTimeout(resolve, 20)); + retired = existsSync(process.env.FM_RETIRED_FILE); +} +if (!retired) { + const rows = existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") + : []; + throw new Error(`broken arm survived a failed confirmation with a distinct watcher pid: ${rows.join(" | ")}`); +} +const rows = readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n"); +const armRows = rows.filter((row) => row.startsWith("arm=")); +if (armRows.length !== 2) throw new Error(`expected one successor arm, got ${armRows.length}: ${rows.join(" | ")}`); +const watcherPid = rows.find((row) => row.startsWith("refused "))?.split("watcher=")[1]; +const armPid = armRows[1].split(" ")[0].slice("arm=".length); +if (!watcherPid || watcherPid === armPid) { + throw new Error(`fixture did not use distinct arm and watcher pids: ${rows.join(" | ")}`); +} +writeFileSync(process.env.FM_STOP_FILE, "stop\n"); +process.exit(0); +EOF +) + status=$? + expect_code 0 "$status" "Pi must retire the failed arm when the watcher pid differs from the arm pid: $out" + [ -z "$out" ] || fail "Pi confirm-distinct test printed output: $out" + pass "Pi confirm failure retires the named arm with distinct watcher pid" +} + +# A failed confirmation retires only the arm its own recovery token names. +# Here the restored successor has already exited (its stdout still held open, +# so its close never fires) and a manual repair has started a newer arm before +# the successor reports ready; the confirmation then fails with the +# successor's dead watcher pid, and the newer healthy arm must survive. +test_pi_confirm_failure_spares_a_newer_arm() { + local repo home plugin log stop go retired out status + repo="$TMP_ROOT/pi-confirm-newer-root" + home="$TMP_ROOT/pi-confirm-newer-home" + log="$TMP_ROOT/pi-confirm-newer.log" + stop="$TMP_ROOT/pi-confirm-newer.stop" + go="$TMP_ROOT/pi-confirm-newer.go" + retired="$TMP_ROOT/pi-confirm-newer.retired" + mkdir -p "$repo/bin" "$home/state" "$home/config" + install_pi_watch_extension_fixture "$repo" + plugin="$repo/.pi/extensions/fm-primary-pi-watch.ts" + cat > "$repo/bin/fm-watch-arm.sh" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --handling-delivered ]; then + printf 'refused generation=%s watcher=%s\n' "$2" "$4" >> "${FM_ARM_LOG:?}" + exit 1 +fi +printf 'arm=%s\n' "$$" >> "${FM_ARM_LOG:?}" +count=$(grep -c '^arm=' "$FM_ARM_LOG") +if [ "$count" -eq 1 ]; then + printf 'watcher: started pid=%s (beacon fresh)\n' "$$" + printf 'signal: synthetic actionable close\n' + exit 0 +fi +if [ "$count" -eq 2 ]; then + sleep 0 & dead=$!; wait "$dead" 2>/dev/null || true + ( + while [ ! -e "${FM_GO_FILE:?}" ]; do sleep 0.02; done + printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-stale\n' "$dead" + while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.05; done + ) & + exit 0 +fi +printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-newer\n' "$$" +trap 'printf "retired\n" > "${FM_RETIRED_FILE:?}"; exit 0' TERM INT +while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done +SH + chmod +x "$repo/bin/fm-watch-arm.sh" + out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" FM_GO_FILE="$go" FM_RETIRED_FILE="$retired" node --input-type=module 2>&1 <<'EOF' +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +let tool = null; +let prompt = ""; +const pi = { + on() {}, + registerCommand() {}, + registerTool(candidate) { + if (candidate.name === "fm_watch_arm_pi") tool = candidate; + }, + sendUserMessage: async (message) => { + prompt += message; + }, +}; +const armRows = () => existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n").filter((row) => row.startsWith("arm=")) + : []; +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); +writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); +const mod = await import(pathToFileURL(process.env.PLUGIN).href); +mod.default(pi); +await tool.execute("tool-call-confirm-newer-first", {}, undefined, undefined, {}); +for (let i = 0; i < 250 && armRows().length < 2; i += 1) await sleep(20); +if (armRows().length < 2) throw new Error("restoration never started a successor"); +const successorPid = Number(armRows()[1].slice("arm=".length)); +let dead = false; +for (let i = 0; i < 250 && !dead; i += 1) { + try { + process.kill(successorPid, 0); + await sleep(20); + } catch { + dead = true; + } +} +if (!dead) throw new Error(`successor pid ${successorPid} never exited`); +const repair = await tool.execute("tool-call-confirm-newer-repair", {}, undefined, undefined, {}); +if (!repair.content[0].text.includes("started Pi extension arm child")) { + throw new Error(`repair did not start a newer arm: ${repair.content[0].text}`); +} +for (let i = 0; i < 250 && armRows().length < 3; i += 1) await sleep(20); +if (armRows().length !== 3) throw new Error(`expected a newer third arm: ${armRows().join(" | ")}`); +const newerPid = Number(armRows()[2].slice("arm=".length)); +writeFileSync(process.env.FM_GO_FILE, "go\n"); +for (let i = 0; i < 250 && !prompt.includes("handling delivery confirmation was rejected"); i += 1) await sleep(20); +if (!prompt.includes("handling delivery confirmation was rejected")) { + throw new Error(`the stale successor's confirmation never failed: ${prompt}`); +} +await sleep(200); +if (existsSync(process.env.FM_RETIRED_FILE)) { + throw new Error("a failed confirmation for a stale successor retired the newer healthy arm"); +} +process.kill(newerPid, 0); +writeFileSync(process.env.FM_STOP_FILE, "stop\n"); +process.exit(0); +EOF +) + status=$? + expect_code 0 "$status" "Pi must not retire a newer arm when a stale successor's confirmation fails: $out" + [ -z "$out" ] || fail "Pi confirm-newer test printed output: $out" + pass "Pi confirm failure for a stale successor spares the newer arm" +} + +# A marker that advanced mid-restore supersedes the in-flight delivery: the +# shell reports a generation mismatch (status 3), so the wake must be +# delivered with no rejection appendix, nothing may be retired, and the +# attempt plus the confirm result must land in the bounded extension log. +# The log assertions run opted in (FM_WATCH_EXTENSION_LOG_KEEP_LINES=50 +# below); the default-off contract lives in +# test_pi_extension_log_stays_off_unless_opted_in. +test_pi_superseded_delivery_has_no_rejection_appendix() { + local repo home plugin log stop out status extension_log + repo="$TMP_ROOT/pi-handling-superseded-root" + home="$TMP_ROOT/pi-handling-superseded-home" + log="$TMP_ROOT/pi-handling-superseded.log" + stop="$TMP_ROOT/pi-handling-superseded.stop" + extension_log="$home/state/.watch-extension.log" + mkdir -p "$repo/bin" "$home/state" "$home/config" + install_pi_watch_extension_fixture "$repo" + plugin="$repo/.pi/extensions/fm-primary-pi-watch.ts" + cat > "$repo/bin/fm-watch-arm.sh" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --handling-delivered ]; then + printf 'superseded generation=%s watcher=%s\n' "$2" "$4" >> "${FM_ARM_LOG:?}" + exit 3 +fi +printf 'arm=%s predecessor=%s\n' "$$" "${FM_WATCH_PREDECESSOR_ARM_PID:-none}" >> "${FM_ARM_LOG:?}" +count=$(grep -c '^arm=' "$FM_ARM_LOG") +if [ "$count" -eq 1 ]; then + printf 'watcher: started pid=%s (beacon fresh)\n' "$$" + printf 'signal: synthetic actionable close\n' + exit 0 +fi +printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-generation\n' "$$" +trap 'exit 0' TERM INT +while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done +SH + chmod +x "$repo/bin/fm-watch-arm.sh" + out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" FM_WATCH_EXTENSION_LOG_KEEP_LINES=50 node --input-type=module 2>&1 <<'EOF' +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +let tool = null; +let prompt = ""; +const pi = { + on() {}, + registerCommand() {}, + registerTool(candidate) { + if (candidate.name === "fm_watch_arm_pi") tool = candidate; + }, + sendUserMessage: async (message) => { + prompt += message; + }, +}; +writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); +const mod = await import(pathToFileURL(process.env.PLUGIN).href); +mod.default(pi); +await tool.execute("tool-call-handling-superseded", {}, undefined, undefined, {}); +for (let i = 0; i < 250 && !prompt.includes("FIRSTMATE WATCHER WAKE"); i += 1) { + await new Promise((resolve) => setTimeout(resolve, 20)); +} +if (!prompt.includes("FIRSTMATE WATCHER WAKE")) throw new Error(`missing follow-up: ${prompt}`); +if (prompt.includes("handling delivery confirmation was rejected")) { + throw new Error(`a superseded delivery carried a rejection appendix: ${prompt}`); +} +if ((prompt.match(/FIRSTMATE WATCHER WAKE/g) || []).length !== 1) { + throw new Error(`a superseded delivery was not a single plain message: ${prompt}`); +} +const rows = existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") + : []; +if (rows.filter((row) => row.startsWith("superseded ")).length < 1) { + throw new Error(`handling-delivered was never attempted: ${rows.join(" | ")}`); +} +writeFileSync(process.env.FM_STOP_FILE, "stop\n"); +process.exit(0); +EOF +) + status=$? + expect_code 0 "$status" "Pi must deliver a superseded wake with no rejection appendix: $out" + [ -z "$out" ] || fail "Pi superseded-delivery test printed output: $out" + [ -f "$extension_log" ] || fail "Pi extension recorded no bounded restore/confirm log" + grep -qF "restore attempt=" "$extension_log" \ + || fail "extension log has no restore attempt: $(cat "$extension_log")" + grep -qF "result=superseded" "$extension_log" \ + || fail "extension log has no superseded confirm result: $(cat "$extension_log")" + pass "Pi superseded handling delivery carries no rejection appendix and is logged" +} + +# A superseded confirmation is not a failure, so the wake routes exactly like +# a confirmed delivery: an accepting supervision branch owns it and main gets +# no follow-up. The same fixture with the confirmation succeeding is the +# control, so the case cannot go vacuous. +test_pi_superseded_delivery_is_offered_to_branch() { + local repo home plugin log stop out status + repo="$TMP_ROOT/pi-superseded-branch-root" + home="$TMP_ROOT/pi-superseded-branch-home" + log="$TMP_ROOT/pi-superseded-branch.log" + stop="$TMP_ROOT/pi-superseded-branch.stop" + mkdir -p "$repo/bin" "$home/state" "$home/config" + install_pi_watch_extension_fixture "$repo" + plugin="$repo/.pi/extensions/fm-primary-pi-watch.ts" + cat > "$repo/bin/fm-watch-arm.sh" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --handling-delivered ]; then + printf 'confirm generation=%s watcher=%s status=%s\n' "$2" "$4" "${FM_CONFIRM_STATUS:?}" >> "${FM_ARM_LOG:?}" + exit "$FM_CONFIRM_STATUS" +fi +printf 'arm=%s\n' "$$" >> "${FM_ARM_LOG:?}" +count=$(grep -c '^arm=' "$FM_ARM_LOG") +if [ "$count" -eq 1 ]; then + printf 'watcher: started pid=%s (beacon fresh)\n' "$$" + printf 'signal: superseded synthetic wake\n' + exit 0 +fi +printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-generation\n' "$$" +trap 'exit 0' TERM INT +while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done +SH + chmod +x "$repo/bin/fm-watch-arm.sh" + out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" node --input-type=module 2>&1 <<'EOF' +import { readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +async function runScenario(confirmStatus) { + process.env.FM_CONFIRM_STATUS = String(confirmStatus); + writeFileSync(process.env.FM_ARM_LOG, ""); + const offers = []; + let mainPrompt = ""; + let tool = null; + const handlers = new Map(); + const bus = { + on(channel, handler) { + handlers.set(channel, [...(handlers.get(channel) ?? []), handler]); + return () => {}; + }, + emit(channel, data) { + for (const handler of handlers.get(channel) ?? []) handler(data); + }, + }; + bus.on("fm-branch-supervision:dispatch", (offer) => { + offers.push(offer.message); + offer.accept(); + }); + const pi = { + on() {}, + events: bus, + registerCommand() {}, + registerTool(candidate) { + if (candidate.name === "fm_watch_arm_pi") tool = candidate; + }, + sendUserMessage: async (message) => { + mainPrompt += message; + }, + }; + const mod = await import(`${pathToFileURL(process.env.PLUGIN).href}?confirm=${confirmStatus}`); + mod.default(pi); + await tool.execute(`tool-call-superseded-branch-${confirmStatus}`, {}, undefined, undefined, {}); + for (let i = 0; i < 250 && offers.length === 0 && mainPrompt === ""; i += 1) { + await new Promise((resolve) => setTimeout(resolve, 20)); + } + const rows = readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n"); + return { offers, mainPrompt, rows }; +} + +writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); +writeFileSync(`${process.env.FM_HOME}/state/superseded-branch.meta`, "project=/projects/approved\nwindow=fm-superseded-branch\n"); +writeFileSync(`${process.env.FM_HOME}/state/.wake-queue`, "1\t1\tsignal\tsuperseded-branch.status\tsignal: superseded synthetic wake\n"); +for (const confirmStatus of [0, 3]) { + const result = await runScenario(confirmStatus); + if (!result.rows.some((row) => row.startsWith("confirm ") && row.endsWith(`status=${confirmStatus}`))) { + throw new Error(`confirm status ${confirmStatus} was never exercised: ${result.rows.join(" | ")}`); + } + if (result.offers.length !== 1) { + throw new Error(`confirm status ${confirmStatus}: expected one branch offer, got ${result.offers.length}; main got: ${result.mainPrompt}`); + } + if (!result.offers[0].includes("signal: superseded synthetic wake")) { + throw new Error(`confirm status ${confirmStatus}: offer missed the wake reason: ${result.offers[0]}`); + } + if (result.offers[0].includes("handling delivery confirmation was rejected")) { + throw new Error(`confirm status ${confirmStatus}: offer carried a rejection appendix: ${result.offers[0]}`); + } + if (result.mainPrompt !== "") { + throw new Error(`confirm status ${confirmStatus}: accepted offer still reached main: ${result.mainPrompt}`); + } +} +writeFileSync(process.env.FM_STOP_FILE, "stop\n"); +process.exit(0); +EOF +) + status=$? + expect_code 0 "$status" "Pi must offer a superseded wake to the branch exactly like a confirmed one: $out" + [ -z "$out" ] || fail "Pi superseded branch-offer test printed output: $out" + pass "Pi superseded handling delivery is offered to the branch like a confirmed one" +} + +# The extension diagnostic log is opt-in and default-off: the same +# mid-restore supersession that logs when opted in must create no +# state/.watch-extension.log file with the knob unset, zero, or +# non-numeric, while the wake is still delivered with no rejection +# appendix. Each knob value runs in a fresh home because the extension +# reads the knob once at module load. +test_pi_extension_log_stays_off_unless_opted_in() { + local repo driver mode home log stop extension_log knob_value out status + repo="$TMP_ROOT/pi-extension-log-off-root" + driver="$TMP_ROOT/pi-extension-log-off-driver.mjs" + mkdir -p "$repo/bin" + install_pi_watch_extension_fixture "$repo" + cat > "$repo/bin/fm-watch-arm.sh" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --handling-delivered ]; then + printf 'superseded generation=%s watcher=%s\n' "$2" "$4" >> "${FM_ARM_LOG:?}" + exit 3 +fi +printf 'arm=%s predecessor=%s\n' "$$" "${FM_WATCH_PREDECESSOR_ARM_PID:-none}" >> "${FM_ARM_LOG:?}" +count=$(grep -c '^arm=' "$FM_ARM_LOG") +if [ "$count" -eq 1 ]; then + printf 'watcher: started pid=%s (beacon fresh)\n' "$$" + printf 'signal: synthetic actionable close\n' + exit 0 +fi +printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-generation\n' "$$" +trap 'exit 0' TERM INT +while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done +SH + chmod +x "$repo/bin/fm-watch-arm.sh" + cat > "$driver" <<'EOF' +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +let tool = null; +let prompt = ""; +const pi = { + on() {}, + registerCommand() {}, + registerTool(candidate) { + if (candidate.name === "fm_watch_arm_pi") tool = candidate; + }, + sendUserMessage: async (message) => { + prompt += message; + }, +}; +writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); +const mod = await import(pathToFileURL(process.env.PLUGIN).href); +mod.default(pi); +await tool.execute("tool-call-handling-superseded", {}, undefined, undefined, {}); +for (let i = 0; i < 250 && !prompt.includes("FIRSTMATE WATCHER WAKE"); i += 1) { + await new Promise((resolve) => setTimeout(resolve, 20)); +} +if (!prompt.includes("FIRSTMATE WATCHER WAKE")) throw new Error(`missing follow-up: ${prompt}`); +if (prompt.includes("handling delivery confirmation was rejected")) { + throw new Error(`a superseded delivery carried a rejection appendix: ${prompt}`); +} +if ((prompt.match(/FIRSTMATE WATCHER WAKE/g) || []).length !== 1) { + throw new Error(`a superseded delivery was not a single plain message: ${prompt}`); +} +const rows = existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") + : []; +if (rows.filter((row) => row.startsWith("superseded ")).length < 1) { + throw new Error(`handling-delivered was never attempted: ${rows.join(" | ")}`); +} +writeFileSync(process.env.FM_STOP_FILE, "stop\n"); +process.exit(0); +EOF + for mode in unset zero bogus; do + home="$TMP_ROOT/pi-extension-log-off-home-$mode" + log="$TMP_ROOT/pi-extension-log-off-$mode.log" + stop="$TMP_ROOT/pi-extension-log-off-$mode.stop" + extension_log="$home/state/.watch-extension.log" + mkdir -p "$home/state" "$home/config" + if [ "$mode" = unset ]; then + out=$(PLUGIN="$repo/.pi/extensions/fm-primary-pi-watch.ts" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" node "$driver" 2>&1) + else + if [ "$mode" = zero ]; then knob_value=0; else knob_value="not-a-number"; fi + out=$(PLUGIN="$repo/.pi/extensions/fm-primary-pi-watch.ts" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" FM_WATCH_EXTENSION_LOG_KEEP_LINES="$knob_value" node "$driver" 2>&1) + fi + status=$? + expect_code 0 "$status" "Pi superseded delivery must still succeed with the log $mode: $out" + [ -z "$out" ] || fail "Pi log-off ($mode) run printed output: $out" + [ ! -e "$extension_log" ] || fail "Pi extension wrote its diagnostic log with the knob $mode: $(cat "$extension_log")" + done + pass "Pi extension diagnostic log stays off unless opted in" +} + +# A repair call must not no-op on an arm child whose process is already dead +# while its close event is still pending (stdio pipe held): the first arm +# below exits at once but leaves a pipe holder behind, so the extension still +# holds the handle with no close fired. The repair must start a fresh arm +# rather than answer unchanged. +test_pi_repair_starts_fresh_arm_over_dead_child() { + local repo home plugin log stop holder out status + repo="$TMP_ROOT/pi-stale-child-root" + home="$TMP_ROOT/pi-stale-child-home" + log="$TMP_ROOT/pi-stale-child.log" + stop="$TMP_ROOT/pi-stale-child.stop" + holder="$TMP_ROOT/pi-stale-child-holder.sh" + mkdir -p "$repo/bin" "$home/state" "$home/config" + install_pi_watch_extension_fixture "$repo" + plugin="$repo/.pi/extensions/fm-primary-pi-watch.ts" + cat > "$holder" <<'SH' +#!/usr/bin/env bash +while [ ! -e "${1:?}" ]; do sleep 0.05; done +SH + chmod +x "$holder" + cat > "$repo/bin/fm-watch-arm.sh" <<'SH' +#!/usr/bin/env bash +printf 'arm=%s predecessor=%s\n' "$$" "${FM_WATCH_PREDECESSOR_ARM_PID:-none}" >> "${FM_ARM_LOG:?}" +count=$(grep -c '^arm=' "$FM_ARM_LOG") +if [ "$count" -eq 1 ]; then + printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-generation\n' "$$" + ("${FM_HOLDER:?}" "${FM_STOP_FILE:?}" >&1 2>/dev/null &) + exit 0 +fi +printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-generation\n' "$$" +trap 'exit 0' TERM INT +while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done +SH + chmod +x "$repo/bin/fm-watch-arm.sh" + out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" FM_HOLDER="$holder" node --input-type=module 2>&1 <<'EOF' +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +let tool = null; +const pi = { + on() {}, + registerCommand() {}, + registerTool(candidate) { + if (candidate.name === "fm_watch_arm_pi") tool = candidate; + }, + sendUserMessage: async () => {}, +}; +writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); +const mod = await import(pathToFileURL(process.env.PLUGIN).href); +mod.default(pi); +const first = await tool.execute("tool-call-first-arm", {}, undefined, undefined, {}); +if (!first.content[0].text.includes("started Pi extension arm child")) { + throw new Error(`first arm did not start: ${first.content[0].text}`); +} +const armRows = () => existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n") + : []; +let armPid = ""; +for (let i = 0; i < 250 && !armPid; i += 1) { + await new Promise((resolve) => setTimeout(resolve, 20)); + const row = armRows().find((line) => line.startsWith("arm=")); + if (row) armPid = row.split(" ")[0].slice("arm=".length); +} +if (!armPid) throw new Error("first arm never logged its pid"); +let dead = false; +for (let i = 0; i < 250 && !dead; i += 1) { + try { + process.kill(Number(armPid), 0); + } catch { + dead = true; + } + if (!dead) await new Promise((resolve) => setTimeout(resolve, 20)); +} +if (!dead) throw new Error(`first arm pid ${armPid} never exited`); +const second = await tool.execute("tool-call-repair", {}, undefined, undefined, {}); +const text = second.content[0].text; +if (!text.includes("started Pi extension arm child")) { + throw new Error(`repair did not start a fresh arm: ${text}`); +} +if (text.includes("unchanged")) { + throw new Error(`repair no-opped on a dead child: ${text}`); +} +let rearmed = false; +for (let i = 0; i < 250 && !rearmed; i += 1) { + await new Promise((resolve) => setTimeout(resolve, 20)); + rearmed = armRows().filter((line) => line.startsWith("arm=")).length >= 2; +} +if (!rearmed) { + throw new Error(`repair started no second arm: ${armRows().join(" | ")}`); +} +writeFileSync(process.env.FM_STOP_FILE, "stop\n"); +process.exit(0); +EOF +) + status=$? + expect_code 0 "$status" "Pi repair must start a fresh arm over a dead child handle: $out" + [ -z "$out" ] || fail "Pi stale-child repair test printed output: $out" + pass "Pi repair starts a fresh arm instead of no-opping on a dead child" +} + +# A scheduled continuity retry must not stall behind a dead-but-unclosed arm +# child. The first arm exits with its stdout held open, a manual repair starts +# a second arm that does the same, and then the first arm's close finally +# fires: its retry must see the dead second arm as an empty slot and start a +# fresh arm. +test_pi_scheduled_retry_starts_fresh_arm_over_dead_child() { + local repo home plugin log stop release out status + repo="$TMP_ROOT/pi-retry-dead-root" + home="$TMP_ROOT/pi-retry-dead-home" + log="$TMP_ROOT/pi-retry-dead.log" + stop="$TMP_ROOT/pi-retry-dead.stop" + release="$TMP_ROOT/pi-retry-dead.release" + mkdir -p "$repo/bin" "$home/state" "$home/config" + install_pi_watch_extension_fixture "$repo" + plugin="$repo/.pi/extensions/fm-primary-pi-watch.ts" + cat > "$repo/bin/fm-watch-arm.sh" <<'SH' +#!/usr/bin/env bash +printf 'arm=%s\n' "$$" >> "${FM_ARM_LOG:?}" +count=$(grep -c '^arm=' "$FM_ARM_LOG") +printf 'watcher: started pid=%s (beacon fresh)\n' "$$" +if [ "$count" -eq 1 ]; then + (while [ ! -e "${FM_RELEASE_FILE:?}" ]; do sleep 0.02; done) & + exit 0 +fi +if [ "$count" -eq 2 ]; then + (while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.05; done) & + exit 0 +fi +trap 'exit 0' TERM INT +while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done +SH + chmod +x "$repo/bin/fm-watch-arm.sh" + out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" FM_RELEASE_FILE="$release" FM_WATCH_REARM_RETRY_BASE_MS=20 node --input-type=module 2>&1 <<'EOF' +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +let tool = null; +const pi = { + on() {}, + registerCommand() {}, + registerTool(candidate) { + if (candidate.name === "fm_watch_arm_pi") tool = candidate; + }, + sendUserMessage: async () => {}, +}; +const armRows = () => existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n").filter((row) => row.startsWith("arm=")) + : []; +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); +async function waitForArms(count) { + for (let i = 0; i < 250 && armRows().length < count; i += 1) await sleep(20); + if (armRows().length < count) throw new Error(`expected ${count} arms: ${armRows().join(" | ")}`); + return Number(armRows()[count - 1].slice("arm=".length)); +} +async function waitForExit(pid) { + for (let i = 0; i < 250; i += 1) { + try { + process.kill(pid, 0); + } catch { + return; + } + await sleep(20); + } + throw new Error(`arm pid ${pid} never exited`); +} +writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); +const mod = await import(pathToFileURL(process.env.PLUGIN).href); +mod.default(pi); +await tool.execute("tool-call-retry-dead-first", {}, undefined, undefined, {}); +await waitForExit(await waitForArms(1)); +const repair = await tool.execute("tool-call-retry-dead-repair", {}, undefined, undefined, {}); +if (!repair.content[0].text.includes("started Pi extension arm child")) { + throw new Error(`repair did not start a second arm: ${repair.content[0].text}`); +} +await waitForExit(await waitForArms(2)); +writeFileSync(process.env.FM_RELEASE_FILE, "release\n"); +for (let i = 0; i < 250 && armRows().length < 3; i += 1) await sleep(20); +if (armRows().length !== 3) { + throw new Error(`the first arm's close scheduled no retry over the dead second arm: ${armRows().join(" | ")}`); +} +writeFileSync(process.env.FM_STOP_FILE, "stop\n"); +process.exit(0); +EOF +) + status=$? + expect_code 0 "$status" "Pi scheduled retry must start a fresh arm over a dead child handle: $out" + [ -z "$out" ] || fail "Pi scheduled-retry dead-child test printed output: $out" + pass "Pi scheduled retry starts a fresh arm instead of stalling on a dead child" +} + +# A verified successor that closes while its wake is still being delivered +# defers its retry to the end of that delivery. If a manual repair meanwhile +# left a dead-but-unclosed arm in the slot, the deferred retry must still +# start a fresh arm. +test_pi_deferred_close_starts_fresh_arm_over_dead_child() { + local repo home plugin log stop release out status + repo="$TMP_ROOT/pi-deferred-dead-root" + home="$TMP_ROOT/pi-deferred-dead-home" + log="$TMP_ROOT/pi-deferred-dead.log" + stop="$TMP_ROOT/pi-deferred-dead.stop" + release="$TMP_ROOT/pi-deferred-dead.release" + mkdir -p "$repo/bin" "$home/state" "$home/config" + install_pi_watch_extension_fixture "$repo" + plugin="$repo/.pi/extensions/fm-primary-pi-watch.ts" + cat > "$repo/bin/fm-watch-arm.sh" <<'SH' +#!/usr/bin/env bash +if [ "${1:-}" = --handling-delivered ]; then + exit 0 +fi +printf 'arm=%s\n' "$$" >> "${FM_ARM_LOG:?}" +count=$(grep -c '^arm=' "$FM_ARM_LOG") +if [ "$count" -eq 1 ]; then + printf 'watcher: started pid=%s (beacon fresh)\n' "$$" + printf 'signal: synthetic actionable close\n' + exit 0 +fi +if [ "$count" -eq 2 ]; then + printf 'watcher: started pid=%s (beacon fresh) recovery-generation=fixture-generation\n' "$$" + while [ ! -e "${FM_RELEASE_FILE:?}" ]; do sleep 0.02; done + exit 0 +fi +printf 'watcher: started pid=%s (beacon fresh)\n' "$$" +if [ "$count" -eq 3 ]; then + (while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.05; done) & + exit 0 +fi +trap 'exit 0' TERM INT +while [ ! -e "$FM_STOP_FILE" ]; do sleep 0.02; done +SH + chmod +x "$repo/bin/fm-watch-arm.sh" + out=$(PLUGIN="$plugin" FM_HOME="$home" FM_ROOT_OVERRIDE="$repo" FM_ARM_LOG="$log" FM_STOP_FILE="$stop" FM_RELEASE_FILE="$release" FM_WATCH_REARM_RETRY_BASE_MS=20 node --input-type=module 2>&1 <<'EOF' +import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +let tool = null; +let deliveryStarted = false; +let finishDelivery = () => {}; +const deliveryHeld = new Promise((resolve) => { + finishDelivery = resolve; +}); +const pi = { + on() {}, + registerCommand() {}, + registerTool(candidate) { + if (candidate.name === "fm_watch_arm_pi") tool = candidate; + }, + sendUserMessage: async () => { + deliveryStarted = true; + await deliveryHeld; + }, +}; +const armRows = () => existsSync(process.env.FM_ARM_LOG) + ? readFileSync(process.env.FM_ARM_LOG, "utf8").trim().split("\n").filter((row) => row.startsWith("arm=")) + : []; +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); +async function waitForArms(count) { + for (let i = 0; i < 250 && armRows().length < count; i += 1) await sleep(20); + if (armRows().length < count) throw new Error(`expected ${count} arms: ${armRows().join(" | ")}`); + return Number(armRows()[count - 1].slice("arm=".length)); +} +async function waitForExit(pid) { + for (let i = 0; i < 250; i += 1) { + try { + process.kill(pid, 0); + } catch { + return; + } + await sleep(20); + } + throw new Error(`arm pid ${pid} never exited`); +} +writeFileSync(`${process.env.FM_HOME}/state/.lock`, `${process.pid}\n`); +const mod = await import(pathToFileURL(process.env.PLUGIN).href); +mod.default(pi); +await tool.execute("tool-call-deferred-dead-first", {}, undefined, undefined, {}); +const successorPid = await waitForArms(2); +for (let i = 0; i < 250 && !deliveryStarted; i += 1) await sleep(20); +if (!deliveryStarted) throw new Error("the restored wake was never delivered"); +writeFileSync(process.env.FM_RELEASE_FILE, "release\n"); +await waitForExit(successorPid); +await sleep(100); +const repair = await tool.execute("tool-call-deferred-dead-repair", {}, undefined, undefined, {}); +if (!repair.content[0].text.includes("started Pi extension arm child")) { + throw new Error(`repair did not start an arm after the successor closed: ${repair.content[0].text}`); +} +await waitForExit(await waitForArms(3)); +finishDelivery(); +for (let i = 0; i < 250 && armRows().length < 4; i += 1) await sleep(20); +if (armRows().length !== 4) { + throw new Error(`the deferred close started no retry over the dead repair arm: ${armRows().join(" | ")}`); +} +writeFileSync(process.env.FM_STOP_FILE, "stop\n"); +process.exit(0); +EOF +) + status=$? + expect_code 0 "$status" "Pi deferred close must start a fresh arm over a dead child handle: $out" + [ -z "$out" ] || fail "Pi deferred-close dead-child test printed output: $out" + pass "Pi deferred close starts a fresh arm instead of stalling on a dead child" +} + test_pi_hung_successor_falls_back_to_typed_wake() { local repo home plugin log out status repo="$TMP_ROOT/pi-hung-successor-root" @@ -4424,6 +5199,14 @@ test_pi_heartbeat_restoration_failure_stays_on_main test_pi_watcher_failure_never_offered_to_branch test_pi_away_record_collapses_eligibility_and_keeps_vetoes_on_main test_pi_handling_delivery_failure_is_typed_once +test_pi_confirm_failure_retires_arm_with_distinct_watcher_pid +test_pi_confirm_failure_spares_a_newer_arm +test_pi_superseded_delivery_has_no_rejection_appendix +test_pi_superseded_delivery_is_offered_to_branch +test_pi_extension_log_stays_off_unless_opted_in +test_pi_repair_starts_fresh_arm_over_dead_child +test_pi_scheduled_retry_starts_fresh_arm_over_dead_child +test_pi_deferred_close_starts_fresh_arm_over_dead_child test_pi_hung_successor_falls_back_to_typed_wake test_pi_unretired_successor_falls_back_without_retry test_pi_late_unretired_close_resumes_supervision diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 0267782497b..fb822035c98 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1470,6 +1470,120 @@ test_reaper_stops_a_tracked_watcher() { pass "watch-arm: the test reaper stops a watcher armed for a tracked temporary home" } +# A handling-delivery confirmation for an episode the drain already +# acknowledged must succeed as a no-op when the generation matches: the pid is +# alive and holds the lock, so the handling is already retired, not rejected. +# A mismatched generation, a dead pid, and a lock mismatch stay rejections. +test_handling_delivered_accepts_already_acked_generation() { + local dir home state pid identity generation status dead + dir=$(make_case handling-delivered-acked) + home="$dir/home" + state="$dir/state" + mkdir -p "$home/data" "$state/.watch.lock" + sleep 60 & + pid=$! + identity=$(bash -c '. "$1"; fm_pid_identity "$2"' _ "$ROOT/bin/fm-wake-lib.sh" "$pid") \ + || fail "could not read the fixture watcher identity" + printf '%s' "$home" > "$state/.watch.lock/fm-home" + printf '%s' "$WATCH" > "$state/.watch.lock/watcher-path" + printf '%s' "$identity" > "$state/.watch.lock/pid-identity" + FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_recovery_marker_publish "$2" downtime' \ + _ "$ROOT/bin/fm-wake-lib.sh" "$state/.watcher-down" \ + || fail "could not publish the fixture downtime episode" + generation=$(recovery_marker_generation "$state/.watcher-down") + [ -n "$generation" ] || fail "published episode left no recovery generation" + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$WATCH_ARM" --handling-delivered "$generation" \ + --watcher-pid "$pid" || fail "confirmed prompt delivery did not begin handling" + FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_recovery_marker_ack "$2" "$3"' \ + _ "$ROOT/bin/fm-wake-lib.sh" "$state/.watcher-down" "$generation" \ + || fail "could not acknowledge the fixture handling episode" + case "$(cat "$state/.watcher-down")" in + acked:handling:"$generation") ;; + *) fail "acknowledged episode did not retire: $(cat "$state/.watcher-down")" ;; + esac + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$WATCH_ARM" --handling-delivered "$generation" \ + --watcher-pid "$pid" + expect_code 0 "$?" "an already-acknowledged confirmation must succeed as a no-op" + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$WATCH_ARM" --handling-delivered "superseded.0.deadbeef" \ + --watcher-pid "$pid" 2>/dev/null + expect_code 3 "$?" "a superseded generation must stay rejected" + sleep 0 & + dead=$! + wait "$dead" 2>/dev/null || true + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$WATCH_ARM" --handling-delivered "$generation" \ + --watcher-pid "$dead" 2>/dev/null + expect_code 1 "$?" "a dead watcher pid must stay rejected" + printf 'foreign-identity\n' > "$state/.watch.lock/pid-identity" + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$WATCH_ARM" --handling-delivered "$generation" \ + --watcher-pid "$pid" 2>/dev/null + status=$? + printf '%s' "$identity" > "$state/.watch.lock/pid-identity" + expect_code 1 "$status" "a lock mismatch must stay rejected" + kill -KILL "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + pass "watch-arm: an already-acknowledged handling confirmation succeeds as a no-op" +} + +# A non-successor arm start mints a fresh generation, and a confirmation for +# the churned generation reports a mismatch (status 3). The closing arm check +# without a reopen - the marker step a handling successor runs - keeps the +# churned generation. This characterizes existing marker behavior that the Pi +# superseded-delivery path relies on. +test_handling_delivered_rejects_a_superseded_generation() { + local dir home state pid identity first second status + dir=$(make_case handling-delivered-superseded) + home="$dir/home" + state="$dir/state" + mkdir -p "$home/data" "$state/.watch.lock" + sleep 60 & + pid=$! + identity=$(bash -c '. "$1"; fm_pid_identity "$2"' _ "$ROOT/bin/fm-wake-lib.sh" "$pid") \ + || fail "could not read the fixture watcher identity" + printf '%s' "$home" > "$state/.watch.lock/fm-home" + printf '%s' "$WATCH" > "$state/.watch.lock/watcher-path" + printf '%s' "$identity" > "$state/.watch.lock/pid-identity" + FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_recovery_marker_publish "$2" downtime && fm_recovery_marker_arm_check "$2"' \ + _ "$ROOT/bin/fm-wake-lib.sh" "$state/.watcher-down" \ + || fail "could not announce the fixture downtime episode" + first=$(recovery_marker_generation "$state/.watcher-down") + case "$(cat "$state/.watcher-down")" in + announced:downtime:"$first") ;; + *) fail "announced episode has the wrong shape: $(cat "$state/.watcher-down")" ;; + esac + # Reopening mints a fresh generation only when unrecovered work is queued: + # an announced episode with an empty queue must survive untouched, so queue + # one wake and re-announce first. Without this the reopen below is a no-op + # by design (no idle churn) and the fresh-generation assertion below fails. + append_wake "$state" check inbox:fixture 'check: manual-restart churn fixture' \ + || fail "could not queue the fixture wake for the manual restart" + FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_recovery_marker_arm_check "$2"' \ + _ "$ROOT/bin/fm-wake-lib.sh" "$state/.watcher-down" \ + || fail "could not re-announce the queued fixture episode" + first=$(recovery_marker_generation "$state/.watcher-down") + case "$(cat "$state/.watcher-down")" in + announced:downtime:"$first") ;; + *) fail "queued episode has the wrong shape: $(cat "$state/.watcher-down")" ;; + esac + FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_recovery_marker_reopen_announced "$2"' \ + _ "$ROOT/bin/fm-wake-lib.sh" "$state/.watcher-down" \ + || fail "a manual arm start could not reopen the announced episode" + second=$(recovery_marker_generation "$state/.watcher-down") + [ -n "$second" ] && [ "$second" != "$first" ] \ + || fail "a non-successor arm start did not mint a fresh generation: $(cat "$state/.watcher-down")" + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$WATCH_ARM" --handling-delivered "$first" \ + --watcher-pid "$pid" 2>/dev/null + expect_code 3 "$?" "a confirmation for the churned generation must report a mismatch" + FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_recovery_marker_arm_check "$2"' \ + _ "$ROOT/bin/fm-wake-lib.sh" "$state/.watcher-down" \ + || fail "the arm check after the reopen could not run" + status=$(recovery_marker_generation "$state/.watcher-down") + [ "$status" = "$second" ] \ + || fail "an arm check without a reopen minted another generation: $(cat "$state/.watcher-down")" + kill -KILL "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + pass "watch-arm: a churned generation's handling confirmation reports a mismatch and an arm check keeps it" +} + test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain test_arm_refuses_an_unusable_launch_confirm_window @@ -1495,6 +1609,8 @@ test_handling_window_close_keeps_the_acknowledgement_valid test_moved_generation_acknowledgement_is_self_healing test_downtime_marker_does_not_follow_symlink test_stop_ends_the_home_watcher_and_publishes_downtime +test_handling_delivered_accepts_already_acked_generation +test_handling_delivered_rejects_a_superseded_generation test_take_over_attaches_to_a_cycle_the_named_arm_does_not_own test_take_over_owns_a_fresh_cycle_and_keeps_queued_work_surfacing test_take_over_preserves_downtime_from_watcher_self_exit From 5796796499fee22f8f9f76d12b5a458bbc2b60f4 Mon Sep 17 00:00:00 2001 From: Rajesh Rajendiran Date: Sun, 4 Oct 2026 02:57:16 -0500 Subject: [PATCH 3/3] no-mistakes(ci): Fixed both CI failures. Updated Pi HTML-renderer fixtures for old/current resolver APIs, extended only Herdr recovery lock waits while preserving ordinary fallback timing, and covered all sibling renderer consumers. Herdr presentation E2E, Pi branch tests, lint, syntax, and diff checks pass. Calm renderer regression now passes; later local DOM checks were blocked only by unavailable Chrome --- bin/fm-spawn.sh | 9 +++++---- tests/fm-calm-pi-extension.test.sh | 8 ++++++-- tests/fm-pi-branch-extension.test.sh | 14 ++++++++++++-- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 288a9cbfad0..40a6bbcdd2e 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -1393,14 +1393,15 @@ trap spawn_abort_cleanup EXIT # One bounded lock per live Herdr session/socket, shared across all homes. # is required so secondmate and primary spawns serialize against the -# same session without writing any other home's state directory. +# same session without writing any other home's state directory. Recovery may +# pass a longer bound because it cannot safely fall back to a fresh flat task. spawn_herdr_presentation_order_lock_acquire() { - local session=${1:-} attempt lock_path + local session=${1:-} max_attempts=${2:-50} attempt lock_path [ -n "$session" ] || session=$(fm_backend_herdr_session) lock_path=$(fm_backend_herdr_presentation_session_lock_path "$session") || return 1 HERDR_PRESENTATION_ORDER_LOCK="$lock_path" attempt=0 - while [ "$attempt" -lt 50 ]; do + while [ "$attempt" -lt "$max_attempts" ]; do if fm_lock_try_acquire "$HERDR_PRESENTATION_ORDER_LOCK"; then HERDR_PRESENTATION_ORDER_LOCK_HELD=1 return 0 @@ -3655,7 +3656,7 @@ else echo "error: herdr presentation recovery could not ensure its exact named session" >&2 exit 1 } - spawn_herdr_presentation_order_lock_acquire "$HERDR_SES" || { + spawn_herdr_presentation_order_lock_acquire "$HERDR_SES" 150 || { echo "error: herdr presentation recovery could not acquire its session lock; refusing a concurrent resume" >&2 exit 1 } diff --git a/tests/fm-calm-pi-extension.test.sh b/tests/fm-calm-pi-extension.test.sh index 287c5de2b0e..65bf3b49f7a 100755 --- a/tests/fm-calm-pi-extension.test.sh +++ b/tests/fm-calm-pi-extension.test.sh @@ -1662,8 +1662,10 @@ for (const { name, actual } of rows) { async function assertStockHtmlRendering(command, submitData) { editorText = command; terminalInputHandler(submitData); + const resolveToolDefinition = (name) => tools.find((tool) => tool.name === name); const htmlRenderer = createToolHtmlRenderer({ - getToolDefinition: (name) => tools.find((tool) => tool.name === name), + getToolDefinition: resolveToolDefinition, + getToolRenderers: resolveToolDefinition, theme, cwd: process.cwd(), }); @@ -1693,8 +1695,10 @@ await assertStockHtmlRendering("/export calm.html", "\r"); getKeybindings().setUserBindings({ "tui.input.submit": "alt+s" }); editorText = "/export remapped.html"; terminalInputHandler("\r"); +const resolveUnmatchedToolDefinition = (name) => tools.find((tool) => tool.name === name); const unmatchedRenderer = createToolHtmlRenderer({ - getToolDefinition: (name) => tools.find((tool) => tool.name === name), + getToolDefinition: resolveUnmatchedToolDefinition, + getToolRenderers: resolveUnmatchedToolDefinition, theme, cwd: process.cwd(), }); diff --git a/tests/fm-pi-branch-extension.test.sh b/tests/fm-pi-branch-extension.test.sh index fd651cefa01..b6446a0478a 100644 --- a/tests/fm-pi-branch-extension.test.sh +++ b/tests/fm-pi-branch-extension.test.sh @@ -5214,8 +5214,18 @@ if (JSON.stringify(actualRow.render(100)) !== JSON.stringify(stockRow.render(100 } pi.events.emit("firstmate:calm-presentation", { active: true, stockExportRendering: true }); -const stockHtml = createToolHtmlRenderer({ getToolDefinition: () => stockDefinition, theme, cwd: process.cwd() }); -const actualHtml = createToolHtmlRenderer({ getToolDefinition: () => actualDefinition, theme, cwd: process.cwd() }); +const stockHtml = createToolHtmlRenderer({ + getToolDefinition: () => stockDefinition, + getToolRenderers: () => stockDefinition, + theme, + cwd: process.cwd(), +}); +const actualHtml = createToolHtmlRenderer({ + getToolDefinition: () => actualDefinition, + getToolRenderers: () => actualDefinition, + theme, + cwd: process.cwd(), +}); const stockCall = stockHtml.renderCall("stock-html", "fm_branch_outcomes", args); const actualCall = actualHtml.renderCall("actual-html", "fm_branch_outcomes", args); const stockResult = stockHtml.renderResult("stock-html", "fm_branch_outcomes", result.content, result.details, false);