Repository navigation
fix(pi): restore watcher continuity across successor gaps and make extension log opt-in - #5489
Conversation
34ebdaa to
7d0005b
Compare
|
Speaking as Kun's firstmate: stamped waiting-ci. HEAD contract-class: new-default (split: repair paths are restore; always-on log is new-default).
Overall class for auto-merge: new-default — will not auto-merge even when green. Firstmate-flag deferred until otherwise ready (CI still running). Help the PR; no competing PR. VISION: One captain/interface — repair aligns; always-on log adds below-deck noise surface (partial); Authority aligns (no new autonomy grant); Scripts/judgment aligns; Restart — continuity repair aligns; Delegation n/a; Fleet/vendor — Pi adapter repair aligns; Scope — watcher continuity aligns, new diagnostic file is scope creep. Overall: aligns on repair, does not auto-admit for the new default-on 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.
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.
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.
7d0005b to
0037e9e
Compare
|
… 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 kunchenguid#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
|
Replying to the scope review on the diagnostic log: it is now opt-in and default-off. Setting |
|
Speaking as Kun's firstmate: whole thread re-read (prior waiting-ci new-default stamp on HEAD Closes #5492: body Contract-class: restore (own tip-vs-main; FM-LEARN-CLAIMS / FM-LEARN-4627). Continuity repair restores the already-specified watcher recovery path: VISION.md (each rule)
Outcome: waiting-ci. Firstmate flag no. Workflow approvals this pass: CI |
…successor-gap-fix # Conflicts: # docs/configuration.md # docs/watcher-continuity.md # tests/fm-watch-arm.test.sh
…re 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.
|
Speaking as Kun's firstmate: this is merged. Thank you @RibatTRW — really appreciate you taking the time on this. |
Upstream advanced by one commit (kunchenguid#5489, Pi watcher continuity across successor gaps and an opt-in extension log) while this sync was being verified. It merges cleanly: the only fork-touched path it changes is docs/configuration.md, where both sides are kept, and the fork delta against upstream is unchanged.
* fix: reclaim orphaned watcher arms on the next park (kunchenguid#6335) * fix(bin): take over the watcher cycle a main-only pass-through leaves An attended main-only pass-through leaves a successor watcher cycle running through main's handling turn. The session's next park attached to that cycle instead of owning it, so the successor's arm, orphaned by its host's exit, kept owning the watcher while the new park's arm polled it twice a second until the next close or the park boundary, hours later in a quiet second mate. A second-mate restart hit this every time, since its persist request is a main-only close. The host now records the successor it leaves for main, and the next host's first cycle runs bin/fm-watch-arm.sh --take-over on it: when that arm still owns the healthy watcher, the new arm stops it, reports a reason the cycle delivered first, and otherwise owns a fresh cycle. The stop's own downtime publication is undone over an acknowledged episode when no wake was appended in between, so the handover wakes nobody. * no-mistakes(review): Keep left-arm record until the orphaned arm is gone * no-mistakes(review): Relinquish successor arm only after durably recording it * no-mistakes(review): Relinquish successor only after its record reads back * no-mistakes(document): Clarify successor takeover guarantees and authoritative documentation * no-mistakes(ci): Fixed ci-1: acknowledgement restore now requires the exact taken-over arm/watcher ledger row with signal=TERM, awaited within a short bound. Otherwise takeover proceeds without erasing downtime. Added a self-exit regression confirmed failing before the fix and passing afterward; ordinary takeover tests and the full watcher-arm suite pass. Updated Generation reuse documentation. Syntax, diff checks, and ShellCheck pass with existing SC1091/SC2034 warnings excluded. ci-2 remains unchanged * no-mistakes(document): Clarify watcher take-over recovery and restart limits * fix(bin): restore downtime on supervision-host hand-back when the successor already closed (kunchenguid#6355) * fix(bin): restore supervision host hand-back continuity * no-mistakes(review): Scope host hand-back failure fallback to lost pending:handling * no-mistakes(review): Remove stray scratch test copy tests/.tmp-rest.test.sh * no-mistakes(test): Initialise successor globals so early hand-back survives set -u * no-mistakes(document): Document host hand-back downtime failure and Claude lost-handback notice * no-mistakes(ci): I fixed the Greptile finding. The rule that must hold: when the supervision host hands back an actionable wake, its rewake is refused, and no watcher is healthy, the hand-back still has to reach main as a delivered notice. That must be true whether the recovery marker is `pending:handling` or `announced:handling`. Only one place applies this check: the lost hand-back fallback in `bin/fm-claude-stop-autoarm.sh`. **Fix:** that check now accepts both `pending:handling:*` and `announced:handling:*` tokens (a one-line change). Nothing else in the fallback changed: - Refusals on any other marker, such as an already acknowledged one, still exit 0 silently and open no failure episode. - The notice is still sent once per episode, and repeats are recorded as `failed-suppressed`. **Tests:** - `tests/fm-claude-stop-autoarm.test.sh`: the lost hand-back test now runs as a shared helper with two variants, one writing a `pending:handling` marker and a new one writing `announced:handling` (`test_host_lost_announced_handback_notifies_once_per_episode`). - `tests/fm-supervision-host.test.sh`: the end-to-end test where downtime restoration fails is now a shared helper too, with a new `announced` variant (`test_claude_stop_hook_notifies_when_closed_announced_successor_downtime_restore_fails`). It moves the handling episode to `announced` before the host hands back. Without the fix the hook would exit 0 here; the test requires exit 2, `outcome=failed` and a delivered failure notice. **Verification (all under nice -n 10):** - The full `tests/fm-claude-stop-autoarm.test.sh` suite passed (rc=0), including both lost hand-back variants and the benign-refusal test. - In `tests/fm-supervision-host.test.sh` I ran only the four hand-back test functions, all passing (rc=0). The suite can't run single functions, so I used a temporary copy with a trimmed test list and deleted it afterwards; `git status` shows only the 3 intended files changed. - shellcheck is clean on all three changed files. I did not run `bin/fm-lint.sh`. - I did not run the new tests against the unfixed code; the claim that they fail without the fix comes from reading the old check * fix: reduce remote-job polling process churn (kunchenguid#6363) * perf: cut remote-job idle process creation in the three hot loops Post-update host measurement still attributes most idle churn to three per-sample loops: result-consumer state reads and date calls, the delta reader's capture/hash pass on every poll, and the lane preemption scan's per-field pipelines. This drops each to its minimum without touching the contracts around them. * fm_remote_job_read_state gains an optional result-variable form backed by fm_remote_job_read_line, a builtin-only bounded record read (regular non-symlink file, byte bound, one newline-terminated line, tolerated unterminated tail, no carriage returns). fm_remote_job_wait samples state and the SECONDS clock with no per-sample children; one date call converts the epoch deadline once. * fm-remote-delta-read stats the log each poll and re-runs the bounded capture and hashing only when size, mtime, ctime, inode, or device change. The snapshot's own stat writes the comparison key, so a log that moves between the gate and the capture is never read as stable. * worker_preempting_waiter_exists reads state, home, and the staged argv head with builtins only. The now-unused worker_job_command goes away. The bounded reads use -d '' -n, which behaves identically on the macOS stock bash 3.2 and current bash; -N does not exist on 3.2. Tests cover the malformed-record corpus, delta identity gating, fork-free lane scanning through counting PATH shims, and same-home versus cross-home preemption. No signal traps or sleep contracts change. * no-mistakes(review): Restore subsecond delta keys and byte-bounded builtin record reads * no-mistakes(document): Clarify delta snapshot caching and coarse-timestamp fallback * no-mistakes(lint): Scope UTF-8 regression locales to individual function calls * no-mistakes(ci): Fixed both lint failures by applying the documented production-library analysis boundary at the two affected test imports. Runtime behavior is unchanged; the library remains independently linted. Canonical full-analysis lint passed for the library and both suites, as did bash syntax checks and git diff --check * fix(bin): recognize titled Claude top rules and preserve grey slash commands (kunchenguid#5963) * fix(composer): read a titled Claude top rule as the composer's edge A named Claude Code session draws its title into the composer's top rule. The strict separator predicate rejected that row, so the closing rule read as a lower unmatched separator and an idle, empty composer classified unknown on every cursorless backend, refusing fm-send, exit, and relaunch. Spare a bare agent-glyph row sandwiched between a width-proven titled rule and the screen's only unmatched separator directly below it. The strict separator predicate, dead-shell rule, and blank-row posture are unchanged. Fixes kunchenguid#5601 Fixes kunchenguid#5558 * no-mistakes(test): Keep Claude's grey slash command in Herdr payload proof * no-mistakes(test): Make missing-herdr version check hermetic to installed herdr * fix: prevent Pi trust prompts in seeded secondmate homes (kunchenguid#6387) * fix: pre-approve Pi trust for seeded secondmate homes Unattended first launches of Firstmate-seeded Pi secondmate homes stalled on "Trust project folder?" until Enter. Probe --approve like --tui-mode and pass it only for --secondmate when help advertises it (.fm-secondmate-home signal), leaving ordinary workers and older Pi unchanged. * no-mistakes(document): Consolidate Pi seeded-home trust documentation ownership * no-mistakes(ci): Fixed Lint 1’s unused polling counter. Diagnosed Behavior portable serial 4 as a pre-existing delta-reader test clock race; replaced timing-dependent rewrite and deletion with deterministic executable-boundary synchronization. ShellCheck, Bash syntax checks, and git diff --check passed. Delta-reader tests passed three consecutive runs; all three live Pi trust cases passed. Production behavior unchanged * fix(bin): retry ShellCheck roots that hit the memory ceiling without --external-sources (kunchenguid#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 <seconds> 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 * fix(pi): restore watcher continuity across successor gaps and make extension log opt-in (kunchenguid#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 kunchenguid#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. * fix(pi): hide duplicate assistant finals from hidden processing retries (kunchenguid#5863) * fix(pi): silence unacknowledged processing retry replies Suppress autonomous processing prose before persistence and during streaming while retaining tool calls, signed reasoning, and retryable outcomes. Restore ordinary output after acknowledgement or a user message. Fixes kunchenguid#4954 * no-mistakes(review): Silence only processing retries, keep first presentation visible * fix(pi): preserve differing processing retry replies * test(pi): accept Pi 1.0.1's renamed HTML export renderer lookup (kunchenguid#6530) Pi 1.0.1's createToolHtmlRenderer reads getToolRenderers and ignores getToolDefinition. Calm /export still includes stock grep HTML; the fixture has to pass the lookup key the installed Pi actually reads. * fix(bin): tolerate transient quota read failures (kunchenguid#6490) * fix(procevent-quota): tolerate consecutive slow quota-axi reads The quota allowance poll treated any quota_json failure as terminal, so one slow quota-axi --json (measured max ~29s under a 48s derived bound) shut the watch down until someone re-armed it, and the detail always said "missing/incompatible". Tolerate three consecutive failed or timed-out reads before going terminal, reset the streak on any good read, and report a timeout distinctly from a missing or incompatible tool. Each timed poll runs exactly one bounded --version and one bounded --json: validate the captured version text through fm_quota_axi_version_compatible rather than launching a second probe, and describe a mixed failure streak by count plus last cause. * no-mistakes(document): Document quota polling failure tolerance * no-mistakes(ci): Fixed ci-2 and ci-3. Permanent quota read failures (rc 2 missing, rc 3 incompatible) now report on the first poll, while transient rc 1/4 failures retain the existing three-failure retry behavior. The missing-binary test now uses an isolated PATH without quota-axi and asserts both permanent failures stop at condition_polls: 1. Verification passed: tests/fm-procevent-quota.test.sh, canonical fast lint for both changed files, bash syntax checks, and git diff --check * no-mistakes(review): Classify untimed quota version failures as transient * no-mistakes(document): Clarify quota polling failure budget * fix(bin): clarify scratch guidance and dirty teardown refusals (kunchenguid#6505) * fix(teardown): clarify scratch guidance and dirty worktree refusals Keep ship proof material outside the task worktree and distinguish untracked-only leftovers from tracked edits without changing cleanup guards. Fixes kunchenguid#6319 * fix(ci): Fixed ci-3 only. Both promotion outputs now replace the scout restriction and require external scratch storage and a clean worktree before done. Verification: 27 delivery tests passed, five mutations caught, restored test passed, pinned ShellCheck and syntax/whitespace checks passed. ci-1, ci-2, and ci-4 remain untouched * fix(bin): escalate inbox instructions blocked by busy workers (kunchenguid#6518) * Escalate inbox instructions stuck behind a busy worker Count consecutive busy-deferred due doorbells durably and escalate at the configured bound without typing into the worker pane. Fixes kunchenguid#6445 * fix(review): Fix inbox escalation deduplication and busy streak resets * fix(review): Preserve busy inbox escalations through daemon supervision * fix(document): Correct busy-inbox escalation documentation * fix(ci): Fixed SC2034 in tests/fm-task-inbox.test.sh by including the loop counter in the failure diagnostic. Source-aware lint, all 34 inbox tests, and git diff --check pass. Behavior portable serial 6 reproduces identically on base 1f3e769 and target 78156b8 with Pi 1.0.1: an unrelated renderer API change breaks the unchanged Calm test. No Calm changes made; that failure is addressed separately by kunchenguid#6516. Logs retained in scratchpad-ci/ * fix(ci): Fixed ci-2, ci-3, and ci-4: successor failures surface, reset alerts deduplicate, and oversized busy limits fall back to two. Passed 47 inbox tests, 7 focused daemon checks, all 13 mutation checks, lint, documentation checks, and diff checks. Evidence: scratchpad-ci-selected/summary.json. ci-1 remains unchanged and unwaived. Fresh live Herdr proof remains with the outer driver * fix: prevent unnecessary remote worker turnover (kunchenguid#6431) * fix: prevent healthy remote job worker turnover * no-mistakes(review): Serialize full LaunchAgent repair and verify launchd-tracked owners * no-mistakes(review): Let launchd-tracked unpublished spawns start before reloading * no-mistakes(document): Document remote worker heartbeat and serialized LaunchAgent recovery * no-mistakes(ci): Full CI log showed the idle-worker regression exceeded its outdated command budget (82 versus 80) after independent heartbeat ownership checks were added. Raised the budget to 120 while retaining the separate busy-poll sleep limit. Remote-job and LaunchAgent executable tests passed, as did bash syntax validation and git diff --check. No production behavior changed * no-mistakes(ci): Fixed missing readiness recovery under verified live ownership, preserving the serving PID and private file mode. Added executable regressions for deletion during a blocked sweep and stale readiness diagnostics without LaunchAgent reload. Deletion regression failed before the fix. Both remote-job test suites, bash syntax validation, and git diff --check passed * no-mistakes(ci): Full CI log identified a flaky ownership-loss test racing an already-authorized heartbeat refresh. Replaced backdating and a fixed sleep with bounded observation of readiness expiry through the public probe. Production behavior unchanged. Remote-job and LaunchAgent executable suites passed; bash syntax validation and git diff --check passed * fix: wait for launchd bootout cleanup * no-mistakes(document): Document remote worker recovery and read-only turnover verification * no-mistakes(review): Publish worker identity before lock owner records * no-mistakes(document): Document worker identity publication safety invariant * no-mistakes(ci): Fixed ci-1: replacement workers discard predecessor readiness before publishing identity and roll back identity if lock-owner recording fails. Added executable regressions reproducing both defects. LaunchAgent and orphan-reap suites, shell syntax checks, and git diff --check passed. No live service state was modified * no-mistakes(ci): Restored lock-owner-before-identity publication and removed the identity rollback and reordering-only tests. Retained an executable regression proving predecessor readiness is rejected until replacement startup completes. LaunchAgent and orphan-reap suites, shell syntax checks, and git diff --check passed. Other changes remain intact; the retained-identity interrupted-repair edge remains out of scope. No live service state was modified * fix: speed up remote-job sequence claim cleanup (kunchenguid#6575) * fix(remote-job): reap expired seq claims with one directory walk The hourly claim sweep forked uname+stat per .seq-claims entry and blocked serving for ~85s at ~17k dirs. Delete expired empty claim dirs with a single find -exec rmdir batch and cache the host uname for remaining mtime reads. * no-mistakes(review): Restore original path mtime helper and drop uname cache * no-mistakes(test): Restore claim retention eligibility; focused retention and serving tests pass * no-mistakes(document): Document single-walk claim cleanup and regression entrypoints * no-mistakes(ci): Fixed Lint 2’s reproduced SC1091 by adding the tests/lib.sh ShellCheck source directive to the retention test. Runtime behavior is unchanged. ShellCheck passed for both new claim tests; Bash syntax, the retention behavior test, and git diff --check passed * fix(tests): keep fixture registries out of git worktree roots A TMPDIR pointed at a repository root placed live .fm-test-* registries beside tracked files, and a concurrent git add during the claim-walk CI fix round committed three of them. Route registries and fixture roots through a TMPDIR that refuses git worktree roots, remove the stray files, and pin the escape with a behavioral cleanup test. * no-mistakes(review): Preserve whole-second claim expiry in single-walk sweep * no-mistakes(document): Correct temporary-directory resolution documentation * no-mistakes(ci): Fixed ci-3 by changing only the stale, fresh, and read-only orphan fixture paths in tests/fm-test-fixture-cleanup.test.sh to use $FM_TEST_TMPDIR. All seven tests passed both normally and with TMPDIR set to the worktree root. Shell syntax and git diff --check passed * no-mistakes(document): Align merged docs with upstream hand-back and policy owners --------- Co-authored-by: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: RibatTRW <aydinhrrs@gmail.com> Co-authored-by: Ian Brown <742554+zestysoft@users.noreply.github.com> Co-authored-by: Joseph Kim <jokim1@gmail.com> Co-authored-by: Mickaël Rémond <mremond@process-one.net> Co-authored-by: Hezki <hezki@users.noreply.github.com>
…tension log opt-in (kunchenguid#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 kunchenguid#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.
Intent
Work on the merge conflict for #5489.
Context needed to read the ask: that pull request, "fix(pi): restore watcher continuity across successor gaps and make extension log opt-in", comes from the fork RibatTRW/firstmate, head branch
fm/firstmate-watcher-successor-gap-fix(head 3b237f6), basemain, and closes #5492.It treats a dead-but-unclosed arm child as an empty slot via
liveArmChildso repair and retry start a fresh arm, pins handling confirmation to the restoration's own recovery token so a superseded generation is delivered without a failure appendix and nothing is retired unless the failed token names the exact current pid and generation, treats an already-acknowledged handling or downtime episode as a confirming no-op when the caller names its generation, and makes the Pi extension diagnostic logstate/.watch-extension.logopt-in and off by default (only a positiveFM_WATCH_EXTENSION_LOG_KEEP_LINESenables it).GitHub now reports the pull request as conflicting with
main, so it cannot merge.The pull request's existing behavior must be preserved while bringing it up to date with current
main.Yes lets do the recommended fixes
Those recommended fixes are items 1-4 of a code review of that pull request:
What Changed
.pi/extensions/fm-primary-pi-watch.ts) now treats a dead-but-unclosed arm child as an empty slot (liveArmChild) in repair, scheduled retry and deferred close. Handling confirmation and its single retry use the restoration's own recovery token. A generation mismatch (exit status 3) is reported as superseded and is offered to the Pi supervision branch like a confirmed delivery, with no failure appendix and nothing retired. A failed confirmation retires the current arm only when the failed token names its exact pid and generation and that pid is dead.fm-wake-lib.shtreats an already-acknowledged handling or downtime episode as a confirming no-op when the caller names its generation. Without a named generation it is still rejected. The Pi extension diagnostic logstate/.watch-extension.logis now opt-in and off by default. Only a positiveFM_WATCH_EXTENSION_LOG_KEEP_LINESenables it, and the new knob is documented indocs/configuration.md.docs/watcher-continuity.mdscopes the recovery-token confirm, retry and narrowed retire guard to Pi, since omp and OpenCode still confirm against the current successor. The extension and arm test suites cover:Risk Assessment
✅ Low: The change is well-bounded to the Pi extension and the handling-confirmation marker, and I found no concrete defect. Superseded deliveries now route like confirmed ones, the retire guard matches the recorded token including generation, and docs are scoped to Pi. Tests were added for each of the four requested items, but I did not run them.
Testing
I ran the two targeted test files that exercise this change, and both pass with exit 0. They cover superseded delivery being offered to the branch, dead-child fresh-arm restart on repair/retry/deferred close, the opt-in log, and handling confirmation. I did not run a real Pi or Herdr lab session, so no scenario is recorded as a live pass. I also did not revert each fix to confirm its test fails.
Evidence: Pi extension test log
Source: Pi extension test log
Evidence: watch-arm test log
Source: watch-arm test log
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
bash tests/fm-pi-watch-extension.test.sh(60 ok, exit 0)bash tests/fm-watch-arm.test.sh(30 ok, exit 0)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.