Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .opencode/plugins/fm-primary-watch-arm.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,9 @@ function retryDelay(attempt) {

function waitForRetry(attempt) {
return new Promise((resolve) => {
const timer = setTimeout(resolve, retryDelay(attempt));
timer.unref();
// A recovery wake is not best-effort: keep this arm's restoration chain
// alive until it launches the next bounded attempt or reports the failure.
setTimeout(resolve, retryDelay(attempt));
});
}

Expand Down
7 changes: 5 additions & 2 deletions .pi/extensions/fm-primary-pi-watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,11 @@ export default function (pi: ExtensionAPI) {

function waitForRetry(attempt: number): Promise<void> {
return new Promise((resolveRetry) => {
const timer = setTimeout(resolveRetry, retryDelay(attempt));
timer.unref();
// The actionable-close restoration chain is an owned delivery contract,
// not a best-effort background retry. Once an unready successor is
// retired there may be no child handle left to keep Pi alive; retain the
// timer until we launch the next bounded attempt or surface its failure.
setTimeout(resolveRetry, retryDelay(attempt));
});
}

Expand Down
114 changes: 72 additions & 42 deletions bin/fm-pr-check-migrate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
# registered custom checks remain armed, and every other task poll is
# quarantined for private review. A current X-mode shim is preserved by exact
# content, while the recognized older byte-static shim is refreshed in place.
# Usage: fm-pr-check-migrate.sh [--checks-safe]
# Usage: fm-pr-check-migrate.sh [--checks-safe] [--watcher-lock-held=<pid>]
# `--watcher-lock-held` is watcher-internal: the caller must be the current
# `fm-watch.sh` lock holder for this exact home. It lets the watcher complete
# preflight under its already-held singleton rather than trying to pause itself.
set -u

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
Expand All @@ -26,19 +29,38 @@ NONCANONICAL_PREFIX='!noncanonical'
LEGACY_NONCANONICAL_PREFIX=_noncanonical

ALLOW_INCOMPLETE_REPAIRS=0
if [ "$#" -eq 1 ] && [ "$1" = --checks-safe ]; then
ALLOW_INCOMPLETE_REPAIRS=1
elif [ "$#" -ne 0 ]; then
echo "error: invalid PR check migration request" >&2
WATCHER_LOCK_HELD_PID=
for arg in "$@"; do
case "$arg" in
--checks-safe) ALLOW_INCOMPLETE_REPAIRS=1 ;;
--watcher-lock-held=*) WATCHER_LOCK_HELD_PID=${arg#--watcher-lock-held=} ;;
*)
echo "error: invalid PR check migration request" >&2
exit 2
;;
esac
done
if [ -n "$WATCHER_LOCK_HELD_PID" ] && [ "$ALLOW_INCOMPLETE_REPAIRS" -ne 1 ]; then
echo "error: --watcher-lock-held requires --checks-safe" >&2
exit 2
fi
case "$WATCHER_LOCK_HELD_PID" in
''|*[!0-9]*)
[ -z "$WATCHER_LOCK_HELD_PID" ] || {
echo "error: --watcher-lock-held requires a numeric watcher pid" >&2
exit 2
}
;;
esac

# shellcheck source=bin/fm-pr-lib.sh
. "$SCRIPT_DIR/fm-pr-lib.sh"
# shellcheck source=bin/fm-x-lib.sh
. "$SCRIPT_DIR/fm-x-lib.sh"
# shellcheck source=bin/fm-check-lib.sh
. "$SCRIPT_DIR/fm-check-lib.sh"
# shellcheck source=bin/fm-wake-lib.sh
. "$SCRIPT_DIR/fm-wake-lib.sh"

umask 077
if [ ! -e "$STATE" ] && [ ! -L "$STATE" ]; then
Expand All @@ -52,6 +74,14 @@ if [ ! -d "$STATE" ] || [ -L "$STATE" ]; then
exit 1
fi

if [ -n "$WATCHER_LOCK_HELD_PID" ]; then
if [ "${PPID:-}" != "$WATCHER_LOCK_HELD_PID" ] \
|| ! fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$WATCHER_LOCK_HELD_PID" "$FM_HOME"; then
echo "PR_CHECK_MIGRATION: watcher-lock-held requires the current watcher singleton; refusing to scan state checks" >&2
exit 1
fi
fi

migration_marker_content_valid() {
local file=$1 value
{ exec 7< "$file"; } 2>/dev/null || return 1
Expand Down Expand Up @@ -261,53 +291,53 @@ if ! x_shim_locked_scan_needed; then
[ "$ALLOW_INCOMPLETE_REPAIRS" -eq 1 ] && scan_complete && exit 0
fi

# shellcheck source=bin/fm-wake-lib.sh disable=SC1091
. "$SCRIPT_DIR/fm-wake-lib.sh"

stopped_watcher=0
pid=$(cat "$WATCH_LOCK/pid" 2>/dev/null || true)
if fm_pid_alive "$pid"; then
if ! fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$pid" "$FM_HOME"; then
echo "PR_CHECK_MIGRATION: watcher ownership is ambiguous; review state/.watch.lock before rearming polls" >&2
exit 1
lock_held=0
if [ -z "$WATCHER_LOCK_HELD_PID" ]; then
pid=$(cat "$WATCH_LOCK/pid" 2>/dev/null || true)
if fm_pid_alive "$pid"; then
if ! fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$pid" "$FM_HOME"; then
echo "PR_CHECK_MIGRATION: watcher ownership is ambiguous; review state/.watch.lock before rearming polls" >&2
exit 1
fi
kill -TERM "$pid" 2>/dev/null || {
echo "PR_CHECK_MIGRATION: watcher could not be paused; review state/.watch.lock before rearming polls" >&2
exit 1
}
stopped_watcher=1
i=0
while [ "$i" -lt 100 ] && fm_pid_alive "$pid"; do
sleep 0.05
i=$((i + 1))
done
if fm_pid_alive "$pid"; then
echo "PR_CHECK_MIGRATION: watcher did not pause; review state/.watch.lock before rearming polls" >&2
exit 1
fi
fi
kill -TERM "$pid" 2>/dev/null || {
echo "PR_CHECK_MIGRATION: watcher could not be paused; review state/.watch.lock before rearming polls" >&2
exit 1
}
stopped_watcher=1

i=0
while [ "$i" -lt 100 ] && fm_pid_alive "$pid"; do
while [ "$i" -lt 100 ]; do
if fm_lock_try_acquire "$WATCH_LOCK"; then
lock_held=1
break
fi
# A concurrent migration may have completed while this process waited.
# Its validated marker proves the old watcher crossed the boundary, so this
# process can continue to the normal watcher singleton instead of competing
# with the newly started watcher for a second migration lock.
if migration_complete && ! x_shim_locked_scan_needed; then
exit 0
fi
sleep 0.05
i=$((i + 1))
done
if fm_pid_alive "$pid"; then
echo "PR_CHECK_MIGRATION: watcher did not pause; review state/.watch.lock before rearming polls" >&2
if [ "$lock_held" -ne 1 ]; then
echo "PR_CHECK_MIGRATION: watcher exclusion could not be acquired; review state/.watch.lock before rearming polls" >&2
exit 1
fi
fi

lock_held=0
i=0
while [ "$i" -lt 100 ]; do
if fm_lock_try_acquire "$WATCH_LOCK"; then
lock_held=1
break
fi
# A concurrent migration may have completed while this process waited.
# Its validated marker proves the old watcher crossed the boundary, so this
# process can continue to the normal watcher singleton instead of competing
# with the newly started watcher for a second migration lock.
if migration_complete && ! x_shim_locked_scan_needed; then
exit 0
fi
sleep 0.05
i=$((i + 1))
done
if [ "$lock_held" -ne 1 ]; then
echo "PR_CHECK_MIGRATION: watcher exclusion could not be acquired; review state/.watch.lock before rearming polls" >&2
exit 1
fi
watch_recovery_required=0
if [ "$stopped_watcher" -eq 1 ] || [ -n "${FM_LOCK_RECOVERED_PID:-}" ]; then
watch_recovery_required=1
Expand Down
23 changes: 15 additions & 8 deletions bin/fm-watch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -713,14 +713,6 @@ if [ "${BASH_SOURCE[0]}" != "$0" ]; then
return 0
fi

# Before acquiring the watcher lock or enumerating any runnable check, replace
# or quarantine checks created by older versions. The migration compares bytes
# and reads data only; it never invokes legacy check files through Bash.
"$SCRIPT_DIR/fm-pr-check-migrate.sh" --checks-safe || {
echo "watcher: PR check migration blocked; refusing to execute state checks" >&2
exit 1
}

if ! fm_lock_try_acquire "$WATCH_LOCK"; then
BEAT="$STATE/.last-watcher-beat"
if [ -n "${FM_LOCK_HELD_PID:-}" ]; then
Expand Down Expand Up @@ -787,6 +779,13 @@ printf '%s\n' "$FM_WATCH_DELIVERY_IDENTITY" > "$WATCH_LOCK/pid-identity" 2>/dev/

[ -e "$STATE/.last-heartbeat" ] || touch "$STATE/.last-heartbeat"

# Run the non-executing PR-check preflight only after this watcher owns the
# singleton and reaches its first loop iteration. The initial beat below is
# therefore honest: this process holds the home lock, has registered its
# identity, and is in the supervision loop. It does not claim that preflight
# completed or that a normal signal/check scan has run yet.
startup_migration_pending=1

# A merged poll may have queued its terminal wake and then lost the process
# between receipt publication and fixed-path removal.
# Finish only identity-bound retirement receipts before any check can run.
Expand Down Expand Up @@ -838,6 +837,14 @@ while :; do
# alive. Supervision scripts warn when this goes stale with tasks in flight.
touch "$STATE/.last-watcher-beat"

if [ "$startup_migration_pending" -eq 1 ]; then
"$SCRIPT_DIR/fm-pr-check-migrate.sh" --checks-safe --watcher-lock-held="$WATCHER_PID" || {
echo "watcher: PR check migration blocked; refusing to execute state checks" >&2
exit 1
}
startup_migration_pending=0
fi

# Parent-owned secondmate pending-reply reconciliation: resolve correlated
# parent reports, observe backend busy/idle turn completion, send one recovery
# repost after grace, and escalate once if the recovery turn is also missed.
Expand Down
4 changes: 4 additions & 0 deletions docs/watcher-continuity.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ It waits at most one readiness timeout per attempt, then sends TERM and waits a
If the unready arm does not retire within that bound, the adapter keeps ownership, starts no overlapping retry, and delivers the typed fallback immediately.
When that retained arm later closes, its actual close is classified as a new supervised event without replaying the earlier fallback.
After the configured retry bound is exhausted, it delivers the original wake with a typed continuity-restoration failure even if every successor arm hung without reporting readiness.
For Pi, the actionable-close restoration retry timer remains referenced only until it launches the next bounded attempt or delivers that typed failure, so retiring an unready successor cannot let the runtime exit before Pi's owned wake-delivery contract completes.
OpenCode's actionable-close restoration timer follows the same lifecycle.
The background retry timers for non-actionable closes remain unreferenced.
This is deliberate Option B ordering: the fleet is protected before the model handles the wake whenever restoration succeeds, but the model is never left blind when it does not.

Claude's Stop hook starts the successor arm at the next Stop after the handling turn, rather than before notification as Pi and OpenCode do.
Expand Down Expand Up @@ -75,6 +78,7 @@ Only the watcher process touches `state/.last-watcher-beat`; no helper process c
## Regression coverage

`tests/fm-pi-watch-extension.test.sh` checks Pi's first-cycle-or-explicit-repair tool metadata and ownership-based redundant-call no-ops, then simulates actionable and empty child closes against the actual Pi and OpenCode close handlers, blocks prompt delivery to prove the successor launches first, verifies single-flight behavior, changes the session lock before close to prove ownership is rechecked, and hangs each successor arm to prove bounded fallback delivery includes the typed restoration failure.
Its no-extra-handle Pi control proves that the original actionable wake and bounded restoration failure are delivered after two retries even when no arm child remains to keep the runtime alive.
The same suite covers ordinary same-process session replacement for `/new`, `/resume`, and `/fork`, same-instance shutdown-plus-start, stale prior-generation callbacks, repeated transitions with exactly one live cycle, disappearance of the shutting-down refusal after a valid replacement activates, and terminal quit still refusing late rearm.
`tests/fm-watch-arm.test.sh` covers durable queue replay, real remote parent-replies ingestion into the authoritative status log, decision-only OPEN DECISIONS recovery, interrupted handling replay, generation-bound acknowledgement, a persistent live successor after recovery, a watcher close inside the handling window that must leave the printed acknowledgement valid, and the self-healing moved-generation acknowledgement that consumes its handled rows and names its remedy.
`tests/fm-watcher-lock.test.sh` covers verified-successor attach, recovery publication before stale-lock removal, the typed self-eviction failure, bounded and successor-linked lifecycle rows, and a SIGSTOP counterfactual that distinguishes a live PID from a stale beacon before classifying termination.
Expand Down
57 changes: 57 additions & 0 deletions tests/fm-pi-watch-extension.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,62 @@ EOF
pass "Pi hung successor falls back to one typed actionable wake"
}

test_pi_hung_successor_recovery_keeps_the_runtime_alive() {
local repo home plugin log prompt out status
repo="$TMP_ROOT/pi-hung-successor-runtime-root"
home="$TMP_ROOT/pi-hung-successor-runtime-home"
log="$TMP_ROOT/pi-hung-successor-runtime.log"
prompt="$TMP_ROOT/pi-hung-successor-runtime.prompt"
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=$(wc -l < "$FM_ARM_LOG" | tr -d '[:space:]')
if [ "$count" -eq 1 ]; then
printf 'watcher: started pid=%s (beacon fresh)\n' "$$"
printf 'signal: synthetic wake\n'
exit 0
fi
trap 'exit 0' TERM INT
while :; 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_PROMPT_FILE="$prompt" FM_PI_ARM_READY_TIMEOUT_MS=250 FM_WATCH_REARM_RETRY_BASE_MS=5 FM_WATCH_REARM_RETRY_MAX_MS=10 FM_WATCH_REARM_RETRY_LIMIT=2 node --input-type=module 2>&1 <<'EOF'
import { 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 (message) => {
writeFileSync(process.env.FM_PROMPT_FILE, 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-hung-successor-runtime", {}, undefined, undefined, {});
EOF
)
status=$?
expect_code 0 "$status" "Pi hung-successor runtime control must start"
[ -z "$out" ] || fail "Pi hung-successor runtime control printed output: $out"
[ -f "$prompt" ] || fail "Pi left the restoration chain before delivering the bounded hung-successor wake"
grep -q 'signal: synthetic wake' "$prompt" \
|| fail "Pi runtime control lost the original actionable wake"
grep -q 'could not restore watcher continuity after 2 retries' "$prompt" \
|| fail "Pi runtime control did not deliver the bounded restoration failure"
[ "$(wc -l < "$log" | tr -d '[:space:]')" -eq 4 ] \
|| fail "Pi runtime control did not launch the bounded successor and retries"
pass "Pi hung-successor restoration keeps the runtime alive through wake delivery"
}

test_pi_unretired_successor_falls_back_without_retry() {
local repo home plugin log release out status
repo="$TMP_ROOT/pi-unretired-successor-root"
Expand Down Expand Up @@ -2156,6 +2212,7 @@ test_pi_redundant_tool_call_is_owned_noop
test_pi_scheduled_retry_call_is_owned_noop
test_pi_actionable_close_starts_single_successor_before_delivery
test_pi_hung_successor_falls_back_to_typed_wake
test_pi_hung_successor_recovery_keeps_the_runtime_alive
test_pi_unretired_successor_falls_back_without_retry
test_pi_late_unretired_close_resumes_supervision
test_pi_empty_close_retries_instead_of_disappearing
Expand Down
Loading
Loading