fix(bin): harden supervision recovery - #99
Merged
Conversation
…ash merge The glasses-file-event-wake entry still named pre-squash branch commit 74d836d, which no longer resolves anywhere in this clone or on origin, so the fork-surface check failed for every session. PR #97 landed as 9f158d3; record the same post-merge repair the bridge capability above documents. Upstream: none (fork-manifest hygiene, not a port)
…henguid#1545) Port upstream 88b2a94 onto this fork's diverged supervision code, adapting rather than overwriting. Fault 1, session-lock identity and ownership. Upstream rewrote its bin/fm-session-lock-lib.sh; this fork never had that file, so the same semantics land in this fork's owners instead. bin/fm-primary-scope-lib.sh gains whole-path-component and argv[0] harness evidence (fm_harness_path_name, fm_harness_process_matches), a contiguous verified-harness ancestry walk (fm_harness_ancestry_pids), membership ownership (fm_harness_ancestry_contains), the outermost-of-run resolver (fm_harness_ancestry_pid), and a relation whose ancestry answer is membership rather than one chosen pid. bin/fm-lock.sh resolves harness_pid through that shared owner and now recognizes a live lock held inside this session's own ancestry as its own earlier acquisition, keeping the recorded owner instead of refusing or rewriting it. Fault 2, attached arms reporting delivered cycles as failures. The watcher publishes each printed reason to state/.watch-deliveries.log bound to its PID and process identity before releasing the singleton (bin/fm-watch.sh wake + bin/fm-wake-lib.sh, which owns the ledger once for watcher and arm), fm_watcher_healthy returns the matched identity, and bin/fm-watch-arm.sh tracks cycle identity and resolves an unobservable close against the delivery record: a matching record reports the delivered wake and exits zero, only a cycle that delivered nothing stays the typed nonzero failure. Covers started-child and attached closes on every harness protocol. Fork-preserving adaptations: - FM_HARNESS_RE keeps this fork's verified set (no pi-signed); loose holder-liveness survives as a boundary-checked argv-token tier so profile launchers named codex-primary still read as live holders while ordinary ~/.claude/hooks paths do not. - The ancestry walk keeps the fork's eight-parent budget, preserving documented parity with Pi's lockOwnership(). - docs/verification/supervision.md hunks skipped: this fork has no such file. - Upstream's e2e drove bin/fm-claude-stop-autoarm.sh, absent here; the end-to-end layer targets this fork's real bin/fm-lock.sh in orphaned version-named, daemon-parented, and combined process trees instead. - Hermeticity hardening for every supervision suite (incident-driven): tests/lib.sh drops inherited operational-home variables and pins a temp hermetic FM_HOME, teardown reaps only path-scoped children, and new tests/fm-supervision-test-isolation.test.sh proves a test watcher or --restart can never resolve or stop a real home. - tests/fm-test-run.sh registers the three suites in watcher-wake-lock and serial shard 2; fixture homes pinned explicitly in the affected suites per the hermeticity contract. Full bin/fm-test-run.sh --all passes at this tree (140 scripts, 0 failed); shellcheck clean via bin/fm-lint.sh on every touched script. Upstream: 88b2a94
…unchenguid#1495) Port the transferable core of upstream 33a4287: every supervision consumer now decides alarm state with the same identity-matched live-watcher and fresh-beacon predicate, so a dead or reused watcher pid with a leftover beacon can no longer read as healthy. bin/fm-guard.sh drops its beacon-fresh-only field and calls fm_watcher_healthy (bin/fm-wake-lib.sh) - the predicate the arm wrapper and turn-end guard already use. bin/fm-supervision-lib.sh's docstring records that split: banner text from the status fields, decisions from the shared predicate. Skipped as inapplicable to this fork, which has no Claude Stop auto-arm and no --claude guard mode: - bin/fm-claude-stop-autoarm.sh failure episodes, bounded retries, one-time attended fail-open, and post-alarm suppression; - lock role records (fm_lock_set_role/fm_lock_role) and fm_failure_episode_reset, whose only consumers are that subsystem; - the Claude repair-line rewording, X-mode-as-need source, and procevent sources, which would misdocument this fork's different continuity model. Regression coverage ports upstream's: a fresh beacon without a live watcher stays an alarm; healthy-recovery cases record genuinely live identity-matched holders instead of touching only the beacon; drain liveness asserts the same; secondmate config-push fixtures pin live watchers where they assert silent stderr. fork-surface.conf claims the ported surface as the supervision-recovery-port capability. Upstream: 33a4287
…unchenguid#1661) Port the transferable core of upstream d0461e4: the pull guard's health verdict becomes model-aware and its banner names the real failing condition instead of always blaming the beacon. bin/fm-wake-lib.sh gains fm_supervision_model and fm_watcher_supervision_verdict. Under a between-turns arm-owner model (FM_SUPERVISION_MODEL=autoarm) a fresh beacon with no live watcher is healthy mid-turn and only a stale beacon alarms; under persistent models a live identity-matched watcher with a fresh beacon is still required. This fork maps every detected harness - Claude included - to the persistent model, because its Claude continuity runs tracked background arms rather than a Stop-hook auto-arm; the override keeps the model branch exercised and upstream-shaped. bin/fm-guard.sh consumes the verdict: the once-per-episode dedup keys on the failing condition instead of the beacon mtime (which any beacon touch used to churn into a fresh episode), and the banner now says whether the home lacks a live watcher process or a genuinely fresh beacon. The turn-end guard keeps the PID-strict predicate unchanged, as does the arm layer. Skipped as inapplicable: upstream's spawn-time FM_SUPERVISION_MODEL injection for secondmates (this fork launches every harness under the persistent default), the autoarm/claude-protocol doc rewrites, and the procevent/X-mode need sources this fork's guard has never had. docs/scripts.md rows for the guard and drain reflect the supervision- health wording. Upstream: d0461e4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Port three upstream supervision-recovery commits from kunchenguid/firstmate into this fork (Amplify-Logic/firstmate), adapting each to this fork's diverged code rather than cherry-picking: 88b2a94 (session-lock harness identity via path components/argv[0], contiguous-ancestry ownership, outermost-of-run lock pid, own-lock recognition in fm-lock.sh; attached watch-arm resolves unobservable closes against a PID+identity-bound delivery ledger instead of false FAILED; fork equivalents live in bin/fm-primary-scope-lib.sh and bin/fm-lock.sh, not upstream's fm-session-lock-lib.sh), 33a4287 (only its transferable core: pull guard uses the identity-matched live-watcher health predicate so a fresh leftover beacon never reads healthy; upstream's Claude Stop auto-arm subsystem is deliberately skipped because this fork never had it), and d0461e4 (model-aware supervision verdict fm_watcher_supervision_verdict with reason-keyed banner episodes; this fork maps every harness including claude to the persistent model). Captain-accepted adaptations during the port, all deliberate: supervision tests are hermetic by construction (tests/lib.sh drops inherited FM_HOME/FM_ROOT_OVERRIDE/FM_STATE_OVERRIDE and pins a temp hermetic home; teardown reaps ONLY path-scoped children - processes whose command path lies inside the worktree or registered fixture temp dirs - never by bare script name, after leaked test watchers stole the primary home's watch lock); new regression suite tests/fm-supervision-test-isolation.test.sh owns that guarantee; several existing fixtures were pinned to explicit fixture homes for the same reason; fork-surface.conf gained a supervision-recovery-port capability claiming the new suites and repaired the glasses capability's commit reference to the landed squash merge 9f158d3 (previously an unresolvable pre-squash sha that failed every run); a fourth upstream commit f9b9d43 (durable re-arm recovery) was deliberately DEFERRED by captain decision after its recovery-marker state machine could not be made green against tests/fm-watch-triage.test.sh - it must NOT appear in this diff. Each ported commit passes shellcheck (bin/fm-lint.sh) and the behavior suite; the branch carries one extra hygiene commit for the manifest repair. Do not treat the skipped autoarm subsystem, the persistent-model mapping for claude, or the deferred fourth commit as omissions.
What Changed
Risk Assessment
✅ Low: Captain, the follow-up correctly fixes both teardown scope defects, and the complete branch diff now conforms to the required port adaptations without introducing another substantiated material risk.
Testing
Port-specific real-process CLI scenarios, the full watcher-lock regression, and the retained watch-triage boundary all passed with reviewer-visible transcripts; one peer-startup test flaked only under the initial combined load and passed on a clean full-suite rerun. No visual artifact was applicable because the change affects shell supervision and CLI output rather than UI.
Evidence: Port acceptance CLI transcript
Evidence: Watcher-lock clean rerun
Evidence: Watch-triage deferred-boundary transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
tests/lib.sh:150- The required guarantee says teardown reaps processes “whose command path lies inside the worktree,” but this pattern quotes*, making it literal; it also only matches commands beginning with the worktree path, while interpreted scripts typically appear asbash /worktree/bin/.... Consequently tracked watchers can survive teardown. Match the worktree path anywhere as a path component.tests/lib.sh:152- The required guarantee says teardown reaps children in “registered fixture temp dirs,” but 121 suites callfm_test_tmprootthrough command substitution. Those array mutations occur in a subshell and never reachFM_TEST_CLEANUP_DIRS, so the new scope gate cannot recognize their fixture directories and leaked processes remain alive. Convert callers to the parent-shellFM_TEST_LAST_TMPROOTpattern or redesign registration to persist across command substitution.🔧 Fix: Fix path-scoped test process cleanup
✅ Re-checked - no issues remain.
tests/fm-watcher-lock.test.sh:993- The combined run intermittently failed the peer-startup synchronization scenario with “arm child did not stand down behind the peer watcher” and then did not terminate promptly. A clean isolated rerun of the complete watcher-lock suite passed, including that exact scenario, classifying this as a load-sensitive flaky test rather than a reproducible product failure.Inspectedgit diff --stat ef164dd6617cc5abbf7f6d1cfefd01d73c64915e..ecb5899896d7f0945533f89ae5073cd845f29c2band the changed-file list.Ranbin/fm-test-run.sh tests/fm-session-lock-ancestry.test.sh tests/fm-watch-arm.test.sh tests/fm-guard-stale-banner.test.sh tests/fm-supervision-test-isolation.test.sh tests/fm-watcher-lock.test.sh tests/fm-watch-triage.test.sh; the four port-specific suites passed before a load-sensitive watcher-lock failure, after which the non-terminating combined run was interrupted.Ranbin/fm-test-run.sh tests/fm-watch-triage.test.sh; the complete retained triage behavior passed.Ranbin/fm-test-run.sh tests/fm-watcher-lock.test.sh; the complete isolated rerun passed, including the previously failing peer-startup scenario.Searched the target diff withrg -n 'f9b9d43|recovery-marker|Claude Stop|autoarm'to verify no deferred recovery-marker implementation or Claude Stop subsystem was introduced; matches were limited to model support/tests and explanatory comments.Checkedgit status --shortafter testing; the working tree remained clean.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.