Skip to content

feat: harden secondmate recovery and coordination - #16

Merged
eyevanovich merged 7 commits into
mainfrom
fm/firstmate-upstream-secondmate-reliability
Jul 31, 2026
Merged

eyevanovich merged 7 commits into
mainfrom
fm/firstmate-upstream-secondmate-reliability

Conversation

@eyevanovich

Copy link
Copy Markdown
Owner

Intent

Port the approved upstream secondmate reliability batch onto the current Firstmate fork in two sub-batches: first, confident missing endpoint detection and startup recovery; second, correlated secondmate replies plus generation-bound inherited-config rereads. Preserve fork-specific safety, canonical FIRSTMATE_OP operational inputs, direct-report ownership, all five harness/backend semantics, and the full inheritance allowlist (crew-dispatch.json, crew-harness, backlog-backend, forge-hosts, signing-agent, and captain-shared.md), while continuing to exclude secondmate-harness, local captain.md, and learnings.md. Pending replies must remain parent-owned, correlation-bound, observable without reading secondmate chat, and resilient to malformed, symlinked, hard-linked, wrong-device, wrong-home, or wrong-destination private artifacts. Config rereads must use exact validated destination bytes, generation-specific pointers, durable retry/quarantine handling, and per-home serialization so older values cannot overtake newer ones. Add extensive hermetic coverage, preserve direct captain input behavior, validate every script/test/backend, and ship the committed branch for review without merging it.

What Changed

  • Recover confidently missing or dead secondmate endpoints during startup while preserving ambiguous and unreadable sessions.
  • Add parent-owned, correlation-bound reply tracking with final-report resolution, bounded recovery, and observable escalation.
  • Deliver inherited-config rereads from immutable, generation-specific snapshots with serialized coalescing, retry, and quarantine handling to prevent stale values overtaking newer ones.

Risk Assessment

✅ Low: Captain, the latest change makes stale generations logically ineligible before best-effort cleanup, preserves merged immutable values across partial updates, and leaves a crash-recoverable successor generation without introducing a substantiated remaining defect.

Testing

The previously successful full baseline plus focused end-to-end scripts exercised confident endpoint recovery across backend semantics, parent-owned correlation and escalation without chat scraping, hostile private-artifact rejection, exact-byte generation-bound config rereads with retry/quarantine/serialization, inheritance and direct-input behavior, and session-start recovery; all passed, reviewer-visible CLI transcripts were captured, and the worktree remained unchanged.

Evidence: Correlated pending-reply end-to-end transcript
ok - normal correlated reply resolves once (idempotent)
ok - correlated progress waits for the final answer
ok - completed turn with no report triggers exactly one recovery
ok - recovery attempts reconcile without reinjection
ok - recovery reply resolves the original expectation
ok - second missed turn escalates once and remains durable
ok - failed escalation publication remains retryable and publishes once
ok - transport success cannot masquerade as reply success
ok - undelivered records remain immutable across scan paths
ok - delivery confirmation fallback reconciles durably
ok - unrelated events and stale correlation ids cannot resolve
ok - restart preserves expectation and exact parent destination
ok - wrong-home reports are detected but do not silently acknowledge
ok - direct unmarked captain input creates no expectation
ok - fm-send marked secondmate path creates pending and embeds corr
ok - status-pointed document resolves the expectation
ok - optional helper report resolves without being required for correctness
ok - backend busy/idle observation covers Pi/Claude paths without conversation scrape
ok - tmux and zellij unknown states use bounded capture fallback
ok - tick skips terminal records and reuses target observations
ok - correlations are reused only for matching open task records
ok - tick end-to-end: miss -> one recovery -> escalate -> durable
ok - failed transport discards undelivered expectation only
ok - private pending records reject malformed and foreign artifacts
ok - unsafe delivery, parent-status, and wrong-home artifacts are never followed
ok - private pending directory symlinks are refused without side effects
ok - all pending-reply tests passed
Evidence: Secondmate endpoint detection and recovery transcript
ok - fm_backend_tmux_agent_state: separates live, dead, missing, ambiguous, and unreadable
ok - fm_backend_tmux_agent_state: rejects malformed targets before probing tmux
ok - fm_backend_herdr_agent_state: preserves missing/no-agent/live/unknown husk behavior
ok - fm_backend_agent_state: routes tmux/Herdr and preserves Zellij/Orca/cmux unverified semantics
ok - sweep: a confirmed-dead secondmate endpoint is killed and respawned
ok - sweep: an already-live secondmate is untouched and distinguishable in verbose diagnostics
ok - sweep: an authoritatively missing Pi secondmate window is relaunched
ok - sweep: an existing ambiguous Pi process prevents duplicate recovery
ok - sweep: transient target unreadability never licenses recovery
ok - sweep: failed relaunch diagnostics distinguish a confidently missing endpoint
ok - sweep: an unverified harness blocks recovery with a concrete diagnostic
ok - sweep: idempotent by construction - a live secondmate is never re-touched on a later run
ok - sweep: skipped entirely under FM_BOOTSTRAP_DETECT_ONLY=1, exactly like the other mutating sweeps
ok - sweep: a silent no-op with no kind=secondmate meta present (a secondmate home's own natural scoping)
# all fm-secondmate-liveness tests passed
Evidence: Inherited-config propagation and generation safety transcript
ok - A1 fm-harness.sh secondmate resolves the fallback chain; crew mode unchanged
ok - C1 fm-harness.sh secondmate-model/secondmate-effort resolve the optional tokens; bare harness stays empty (backward-compat)
ok - B1 propagate_inheritable_config: copy, idempotence, convergence, absence-mirror, exclusion, no-op, skip diagnostics
ok - B2 spawn: secondmate runs the secondmate harness; its home inherits declared config
ok - B3 spawn: an absent secondmate-harness falls back to the crew harness (backward-compat)
ok - B4 spawn: no config at all -> own harness and no propagation side effects
ok - B5 spawn: an explicit per-spawn harness arg overrides config/secondmate-harness
ok - B6 spawn: an unverified resolved secondmate harness is refused (guard intact)
ok - C2 spawn: a bare harness-only secondmate-harness file launches with no model/effort flag (backward-compat)
ok - C3 spawn: config/secondmate-harness's model token threads --model into the launch and meta
ok - C4 spawn: config/secondmate-harness's model+effort tokens thread into the launch and meta
ok - C5 spawn: an explicit --model overrides config/secondmate-harness's model token; the file's effort token still applies
ok - C6 spawn: an explicit --effort overrides config/secondmate-harness's effort token; the file's model token still applies
ok - C7 spawn: an explicit --harness starts with clean model/effort defaults
ok - C8 spawn: an explicit --harness still honors explicit model/effort flags
ok - C9 spawn: the harness fallback chain still resolves with no tokens; crew/scout launches are unaffected by this feature
ok - B7 bootstrap sweep pushes, re-converges, and mirrors absence; never inherits secondmate-harness
ok - B8 bootstrap sweep propagates config even when the home's tracked files are already current
ok - B9 bootstrap sweep defers new inherited config until the home ignores it
ok - B10 bootstrap sweep with no inherited config is a config no-op and still fast-forwards
ok - B11 bootstrap sweep surfaces config propagation failures
ok - B11 bootstrap rereads completed config writes after partial propagation
ok - B12 config-push propagates via shared live discovery, reports items, rereads on change only, and does not fast-forward
ok - B13 config-push reports dirty, non-allowing, and invalid homes without failing warnings-only runs
ok - B14 config-push exits nonzero on real propagation errors
ok - B14 config-push rereads completed config writes after partial propagation
ok - B15 config reread is per-home, exact-byte, ordered, and pointer-only
ok - B16 config reread isolation, ABSENT, generation safety, send failure, and retry
ok - B20 config reread publication failures retain exact generations for retry
ok - B21 config reread captures immutable bytes directly into reserved generations
ok - B21 config reread never falls back to mutable retry reports
ok - B21 config reread serializes concurrent propagation and delivery
ok - B22 full config reread retry queues coalesce before new publication
ok - B23 mixed config reread generations coalesce atomically
ok - B26 config reread supersedes older pending generations
ok - B27 legacy mutable retry reports are quarantined
ok - B28 config reread coalescing preserves prior validated items
ok - B29 cleanup failure cannot reactivate superseded config generations
ok - B17 config reread skips unchanged homes and reads destination post-write bytes
ok - B18 bootstrap config reread path works; spawn flexibility remains defaults-only
ok - B19 bootstrap respawns before inherited-config reread
ok - B25 spawn quarantines stale rereads without blocking relaunch
ok - B24 bootstrap detect-only mode remains filesystem read-only
# all fm-secondmate-harness tests passed
Evidence: Session-start recovery transcript
ok - context digest distinguishes ABSENT, empty-but-present, and populated files
ok - a lock refusal prints a loud read-only banner, skips every mutating step, and still completes the digest
ok - digest sections are ordered diagnostics-first, bulk-context-last
ok - session start: configured and auto-detected Herdr homes never require tmux
ok - session start: an absent recorded tmux window relaunches its Pi secondmate exactly once
ok - session start: an existing ambiguous Pi process prevents duplicate recovery
ok - session start: transient tmux unreadability never licenses a relaunch
ok - session start: the proven bare-shell recovery path remains intact
ok - session start: a confirmed Herdr husk is closed and relaunched
ok - status tail is bounded to the configured line count, with the full log path always printed
ok - orphan status logs are printed once with bounded tails
ok - tmux endpoint liveness is reported per task: alive for a live window, dead for a gone one
ok - herdr endpoint liveness is reported per task: alive for a live pane, dead for a gone one
ok - fm-session-start.sh composes the real fm-lock.sh, fm-bootstrap.sh, and fm-wake-drain.sh output verbatim
ok - compatible tasks-axi backlog rendering is compact, bounded, and preserves recovery metadata
ok - manual backlog rendering prints only title lines with hold and blocker metadata
ok - unavailable or incompatible tasks-axi falls back to compact manual backlog rendering
ok - an empty fleet reports (none) for in-flight tasks and an absent AFK flag
ok - session start emits X-mode cadence guidance in the harness supervision block
ok - next step delegates watcher ownership to the AFK daemon
ok - session start emits exactly one detected harness block and reports Pi extension load state
ok - session start rejects stale Pi loaded markers
ok - session start accepts current Pi markers written before lock acquisition
ok - session start rejects Pi sessions missing the turn-end guard marker
ok - session start rejects Pi loaded markers from previous sessions

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed (4) ✅
  • 🚨 bin/fm-config-inherit-lib.sh:1081 - The required invariant “older values cannot overtake newer ones” remains reachable. If generation G1 is pending, a later push creates G2, then this code sorts and sends G1 before G2; if G1 succeeds but G2 fails, the live secondmate applies G1 after the destination already contains G2. Coalesce/supersede older pending generations before delivery, or otherwise enforce latest-generation application at this shared boundary.
  • 🚨 bin/fm-config-inherit-lib.sh:1005 - The required “exact validated destination bytes” are not durably retained when initial instruction creation fails before producing a temporary file. The code saves only the change report, then a later retry rebuilds that generation by rereading the mutable destination, which may already contain a newer value. Capture immutable destination bytes during the originating locked propagation, and never reconstruct an old generation from current destination state.
  • 🚨 bin/fm-pending-reply-lib.sh:601 - A pending reply resolves on any parent status line containing the correlation token, including a working progress line. The secondmate brief permits material working reports, so a correlated progress update can terminally resolve the expectation before the actual answer arrives, defeating the required observable pending-reply guarantee. Restrict resolution to answer-bearing terminal/status-pointer forms or define an explicit acknowledgement grammar.

🔧 Fix: Captain, enforce monotonic rereads and final replies
2 errors still open:

  • 🚨 bin/fm-config-inherit-lib.sh:991 - The required durable retry/quarantine behavior is blocked by any .report artifact left by the previous implementation: this branch returns before capturing or sending the current immutable generation, and every later push repeats the same failure. Quarantine unsupported mutable reports at this shared boundary, then continue from a freshly captured immutable destination snapshot.
  • 🚨 bin/fm-config-inherit-lib.sh:1020 - When coalescing backlog, the synthetic report marks every allowlisted item as pushed regardless of the current propagation report. A skipped or failed destination—such as a symlink or non-gitignored path—is therefore rendered as ABSENT and sent as authoritative, even though propagation deliberately left it unchanged. Build the latest snapshot only from destination states validated as copied or authoritatively absent; do not convert unvalidated items into removals.

🔧 Fix: Captain, quarantine legacy retries and preserve validation
1 error still open:

  • 🚨 bin/fm-config-inherit-lib.sh:1085 - Coalescing a partial propagation drops still-needed authoritative bytes from the prior immutable generation. Example: G1 contains a successfully copied forge-hosts change but its send is pending; G2 validates only crew-harness because forge-hosts is now skipped or errors. The new snapshot contains only crew-harness, then deletes G1, so the secondmate never receives the validated G1 forge-hosts value. At this boundary, merge current validated items with the latest immutable values for currently unvalidated items before retiring the backlog.

🔧 Fix: Captain, preserve validated items across partial coalescing
1 error still open:

  • 🚨 bin/fm-config-inherit-lib.sh:1158 - The required monotonic guarantee remains reachable when retiring merged backlog artifacts fails. This loop ignores rm failures, publishes the merged generation, and forgets the surviving old paths. On a later partial push, a surviving old generation can be merged without the already-delivered newer value and reapply older bytes. Require complete retirement/quarantine before publication, or keep the newest merged snapshot in the durable backlog until stale artifacts are conclusively neutralized.

🔧 Fix: Captain, prevent superseded config generations from reactivating
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"
  • Configured baseline: command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"
  • bash tests/fm-pending-reply.test.sh
  • bash tests/fm-secondmate-liveness.test.sh
  • bash tests/fm-secondmate-harness.test.sh
  • bash tests/fm-session-start.test.sh
  • git status --short to verify testing introduced no worktree artifacts
✅ **Document** - passed

✅ No issues found.

🔧 **Lint** - 1 issue found → auto-fixed ✅
  • ⚠️ linter found issues (exit code 1)

🔧 Fix: Fix ShellCheck temp handoff and unreachable assertions
✅ Re-checked - no issues remain.

✅ **Push** - passed

✅ No issues found.

@eyevanovich
eyevanovich merged commit b1e6a2a into main Jul 31, 2026
4 of 5 checks passed
@eyevanovich
eyevanovich deleted the fm/firstmate-upstream-secondmate-reliability branch July 31, 2026 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant