sync: merge upstream kunchenguid/firstmate main (2 commits) - #41
Merged
Merged
Conversation
…nchenguid#3273) * fix(bin): keep home-summary publication bounded and off the watcher beat A home whose tasks had accumulated ordinary status history could not publish state/home-summary.json at all, and every attempt starved the watcher's liveness beacon while it failed silently. The producer's per-task open-decision fold spent tens of milliseconds per status line on a bash 3.2 global bracket-class substitution used only as a blank-line guard. On a real home that made the whole ledger producer take minutes, so publication burned its full FM_HOME_SUMMARY_TIMEOUT on every attempt and never completed. Replace that guard with an equivalent case glob in the one fold owner, which both the whole-file and cursor-backed folds use. Bound each per-task current-state read in the snapshot with FM_SNAPSHOT_CREW_STATE_TIMEOUT. For a remote secondmate that read crosses ssh, whose dead-peer detection deliberately never kills a slow-but-alive remote command, so nothing else bounded it. Detach the watcher's two publication triggers from the poll loop. The loop owns the beacon that fm-guard.sh reads as proof supervision is alive, and an inline publication put up to a full publication deadline between two beacon touches. A single in-flight publication is tracked so a slow one cannot accumulate clones. Report a repeatedly failing publication at session start. Publication stays deliberately non-fatal to its caller, so the existing bounded home-local failure record is now surfaced as a HOME_SUMMARY bootstrap line once the ledger is absent or stale and failures have been recorded since. * no-mistakes(review): Preserve home-summary failure attempt ordering * no-mistakes(review): Enforce durable home-summary single-flight and ordering * no-mistakes(review): Derive failure ordering from publication boundaries * no-mistakes(review): Restore best-effort failure logging and publication scoping * no-mistakes(review): Make ordering regression sensitive to one failure * no-mistakes(document): Correct HOME_SUMMARY diagnostic guidance
…henguid#3268) * fix(supervision): classify the appended status span, not the last line An actionable project update could be classified as routine and absorbed, so a worker that raised a decision, hit a blocker, failed, or finished stalled silently with the captain never told. Trigger, mask, symptom. A worker appends a captain-relevant event (`needs-decision`, `blocked`, `failed`, `done`). Any later routine append - a `working:` progress note - lands before the supervisor classifies the batch; the watcher's 30s signal-grace linger exists precisely to coalesce a status write with the same turn's turn-end, so this window is ordinary rather than rare. Both supervisors then asked "is the LAST line captain-relevant?", read the routine line, and absorbed the wake. The `.seen-*` suppressor advanced either way, so nothing ever re-read the event. When the crew was also provably working, the no-verb fallback absorbed it too, which is why the event disappeared completely instead of surfacing late. Reproduced end to end against a real watcher before any change: with the trailing `working:` append the watcher never exits and the wake queue stays empty; with that one line removed - the smallest counterfactual - the same `needs-decision` surfaces and queues. The away-mode daemon's `classify_signal` returns `self|routine signal` for a `blocked:` event under the same mask, which is the worse case because no captain is present to notice. The proven path was already in the tree: `status_open_decisions` fixed this exact masking for the durable decision fold, and its header states the rule - reading an append-only event log last-event-wins cannot represent an earlier event that a later unrelated line moved past. The classification path was never migrated to that read model. That is the earliest divergence, and the fix is to migrate it rather than to special-case the symptom. `status_span_first_actionable` in bin/fm-classify-lib.sh is the new single owner: it reads the bytes at or after a caller-supplied position and returns the first still-live captain-relevant event. Each supervisor supplies its own position, because the always-on watcher and the away-mode daemon classify the same stream independently and must not share one cursor: the watcher reads the size already recorded in its `.seen-*` signature (no new state) and its `.hb-surfaced-<task>` backstop marker, and the daemon its `.subsuper-seen-status-<task>` marker. Those two markers held the escalated line and now hold the escalated-through byte offset, which also removes a second defect in the same code - content dedup silently swallowed a genuinely new event whose text repeated an older one. An absent, malformed, or past-the-end position reads the whole log, so uncertainty surfaces events rather than losing them, and a marker an older build wrote as a status line reads that way too. Status logs are only ever appended to, including across a reused task id, so a recorded position keeps its meaning. A `needs-decision`/`blocked` event in the span is retired only when the whole-file fold proves its key closed; `status_open_decisions` stays the sole owner of that rule, so same-key reopening and reserved-key namespaces need no second implementation here. Every other captain-relevant event is terminal and always actionable. Both backstops now walk every status log instead of only those whose last line looks captain-relevant, because the event a backstop most needs to catch is exactly one a later append has moved past. That leaves `scan_captain_relevant_statuses` with no callers, and it is removed rather than left as a working copy of the defective read model. Regression coverage exercises the classifier and both supervisors through their own interfaces: the masked decision, the captain-reported release/install completion followed by cleanup chatter, and the away-mode blocker all surface; a routine append after an already-classified event stays absorbed, so the fix does not convert ordinary progress into wakes; and the heartbeat backstop catches a masked event the per-wake path missed. The end-to-end watcher tests drive a real fm-watch.sh with the crew reported as provably working, which is the configuration that made the original stall silent. Two further claims in the supplied RCA are deliberately not patched here. "Repeated operational recoveries produced all-clear replies despite known actions" is downstream of this same cause, not an independent contributor: an all-clear reply is the documented response when the specific event needs no action, so a classification that wrongly reported "no action" produces it, and correcting the classification removes it. "The project was subjected to validation requirements outside its accepted path" is delivery-mode selection, which AGENTS.md section 7 owns; no code changed here touches it, so it is out of scope. Harness and backend axes were inspected rather than assumed: nothing in this path reads a vendor-emitted signal. The status log's format and append protocol are Firstmate's own and identical for every harness, and no runtime backend reads or writes `.status` files (`bin/backends/*` contain no reference to them). The surrounding triage's only backend touchpoints - pane capture and the authoritative crew-state read - are unchanged. No live-harness guard applies and no per-harness verification record changes. Verified with `bin/fm-lint.sh`, `bin/fm-doc-audience-check.sh`, and `bin/fm-test-run.sh --changed --base origin/main`. * no-mistakes(review): Prevent status races and surface classification failures * no-mistakes(review): Surface unreadable signals and preserve AFK endpoints * no-mistakes(review): Route stale wakes through captured span verdicts * no-mistakes(review): Retire supervision offsets with reused task state * no-mistakes(review): Bind status offsets and preserve live decision origins * no-mistakes(review): Strengthen status identity with verified birth time * no-mistakes(review): Skip turn-end markers during status classification * no-mistakes(review): Preserve status presentation with platform-strength identities * no-mistakes(review): Retain failed wakes and advance routine checkpoints * no-mistakes(review): Surface all events and retain unreadable wakes * no-mistakes(review): Treat absent status logs as successful empty spans * no-mistakes(review): Bound repeated classification failures with durable receipts * revert(supervision): drop the failure-receipt and durable-retry machinery Captain-authorized revert to the minimal fix. Review rounds added a durable failure-receipt store and wake-retention-on-failure to bound repeated classification failures. That machinery grew larger than the fix it protected and kept producing its own defects: an unreadable log still looped forever because the always-on watcher never consulted the receipt, and the receipt was persisted before its diagnostic was durably queued, so a crash in between swallowed the alarm outright. Those two defects go away with the code that contained them rather than being repaired. Removed: the failure-receipt path, fingerprint, record and clear helpers and their retirement bookkeeping; the retention of a durable wake when classification fails; and the error-propagation plumbing in both supervisors that existed only to drive them. Kept, because it is the accepted fix rather than the declined machinery: span classification of the events appended since a supervisor last looked, in both supervisors and both backstops; reporting every actionable event in a span and committing a position only through what was reported; naming the live opening of a reopened decision; treating an absent log as ordinary and an unreadable one as worth reporting; the non-.status filter; and the platform-strength identity that guards a position commit without failing a read. Replacement behavior for a log that cannot be classified: report it once, do NOT advance the classification position so the content is classified from where it stopped once readable, and DO advance the wake signature so the report is bounded to one per distinct file state. Reporting and reading are different acts: telling the captain about a log is not the same as having read it, and only the latter may move a classification position. The residual risk is explicit and accepted: there is no guaranteed automatic retry inside a crash-mid-read window, and the locked session-start replay of the durable queue covers it. That rationale is recorded at mark_escalated_seen so a future reader does not reintroduce the retry as a "missing" guarantee. Also fixes lint failures that arrived with the review-fix commits and were never caught because the run never reached its lint step: an unfollowable conditional source directive, a second unquoted-expansion site left after a call was split across lines, cleanup of the file being read inside its own read loop (restructured to one post-loop teardown rather than three in-loop copies), stub functions in tests that are invoked indirectly, and a test local left unused when its assignment was replaced by a helper. bin/fm-lint.sh passes on the default branch, so these were introduced here. Verified with `bin/fm-lint.sh`, the end-to-end masked-decision and away-mode reproductions, and `bin/fm-test-run.sh` over the supervision, wake-queue, wake-drain, watch-arm and inactive-reconcile suites (6 scripts, 0 failures). * no-mistakes(review): Correct classification failure contract documentation * no-mistakes(review): Bound unreadable status reports without skipping classification * no-mistakes(review): Preserve escalation markers when buffering fails * no-mistakes(review): Detect permission recovery without advancing classification * no-mistakes(document): Document status span classification contract * no-mistakes(ci): Fixed CI failures by lazily loading classification helpers in fm-wake-lib, preserving minimal recovery/remote fixtures; added a public current-status marker helper and updated behavioral fixtures to use the v2 marker contract; resolved ShellCheck variable collisions in fm-control and fm-public-followup-lib. Verified fm-lint, bash syntax, fm-control, public-followup, wake-queue, send-resolve-key, captain-hold, pending-reply, remote-reply, remote-backlog-handoff, turnend-guard, and Claude autoarm tests. The Pi branch suite reached a separate local stock-render mismatch under Node 24; its CI-reported missing-classifier failure path is fixed * no-mistakes(review): Escalate blockers while preserving declared-wait cadence * no-mistakes(review): Clarify actionable events override wait self-handling * no-mistakes(review): Surface rejected decisions and dangling status links * no-mistakes(document): Document reserved-key reconciliation classification * no-mistakes(ci): Fixed the flaky portable serial CI test by modeling the retained staging directory as genuinely owned by a live process and aging both fixtures deterministically. This removes scheduler-timing dependence while verifying the worker reaps abandoned staging and preserves live staging. Verified with fm-remote-transport-lanes.test.sh, bin/fm-lint.sh, bash syntax, and git diff --check * no-mistakes(document): Correct away-mode classification documentation
Absorbs: - 4eb587d fix(bin): prevent routine updates from hiding actionable status (kunchenguid#3268) - 5f31097 fix(bin): keep home-summary publication from starving supervision (kunchenguid#3273) Two conflicts, both integrated rather than resolved by picking a side: - bin/fm-supervise-daemon.sh: the fork's stale-wedge deferral (PR #18, defers alarming on an idle pane that is provably still working a long in-contract foreground call) and upstream's fix (only clear the persistence marker when escalate_add actually succeeds, kunchenguid#3268) touched the same case arm. Kept the fork's deferral logic in full and wrapped its final escalate_add call in upstream's success-check guard. - bin/fm-watch.sh: both sides independently added a new function at the same insertion point - the fork's watcher_idle_eligibility_proven (PR #39, converge .watcher-down on a healthy watcher) and upstream's home_summary_refresh_detached/HOME_SUMMARY_PID (kunchenguid#3273, detach ledger publication from the poll loop so it cannot starve the liveness beacon). Neither existed at the merge base; kept both, since each is called elsewhere in the merged file (watcher_idle_eligibility_proven from watcher_cleanup, home_summary_refresh_detached from the poll loop). Everything else in the 30-file diff (upstream's classify_signal/classify_stale/ mark_status_seen rewrite from a last-line seen-marker to byte-offset status-span tracking, and its lazy fm-classify-lib.sh sourcing in fm-wake-lib.sh) auto-merged cleanly; verified no orphaned call sites remain (mark_escalated_seen, mark_status_seen, wedge_escalation_deferred, and the recovery-marker reuse-only/health_predicate additions PR #39 depends on all check out). Verification: bin/fm-lint.sh clean (ShellCheck 0.11.0, pinned; actionlint not installed locally, workflow lint skipped - no .github/workflows/ files in this diff). All 12 upstream-changed test files pass (fm-daemon, fm-watch-triage, fm-home-summary-refresh, fm-wake-drain-unread-status, fm-wake-queue, fm-watch-arm, fm-captain-hold-lifecycle, fm-inactive-reconcile, fm-pending-reply, fm-remote-reply, fm-remote-transport-lanes, fm-send-resolve-key): failed=0. One benign, pre-existing, byte-identical-to- upstream artifact noted, not fixed: fm-inactive-reconcile.test.sh's prime_seen helper calls _fm_status_file_size directly without first calling _fm_wake_require_classify, so a fresh subshell prints a harmless "command not found" before the marker write is skipped; upstream's own tests/fm-inactive-reconcile.test.sh and bin/fm-wake-lib.sh ship the identical code, and the test suite still passes end to end.
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
2 participants
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.
Summary
Syncs this fork with
kunchenguid/firstmatemain, absorbing the 2 commits the fork was behind:4eb587dfix(bin): prevent routine updates from hiding actionable status (fix(bin): prevent routine updates from hiding actionable status kunchenguid/firstmate#3268)5f31097fix(bin): keep home-summary publication from starving supervision (fix(bin): keep home-summary publication from starving supervision kunchenguid/firstmate#3273)The fork's 80 commits of its own work are preserved.
Conflict resolution
Two conflicts, both integrated rather than resolved by picking a side.
bin/fm-supervise-daemon.sh- the fork's stale-wedge deferral (PR #18, defers alarming on an idle pane that is provably still working a long in-contract foreground call) and upstream's fix (only clear the persistence marker whenescalate_addactually succeeds, kunchenguid#3268) touched the samecasearm. Kept the fork's deferral logic in full and wrapped its finalescalate_addcall in upstream's success-check guard.bin/fm-watch.sh- both sides independently added a new function at the same insertion point: the fork'swatcher_idle_eligibility_proven(PR #39, converge.watcher-downon a healthy watcher) and upstream'shome_summary_refresh_detached/HOME_SUMMARY_PID(kunchenguid#3273, detach ledger publication from the poll loop so it cannot starve the liveness beacon). Neither existed at the merge base; kept both, since each is called elsewhere in the merged file (watcher_idle_eligibility_provenfromwatcher_cleanup,home_summary_refresh_detachedfrom the poll loop).Everything else in the 30-file diff - upstream's
classify_signal/classify_stale/mark_status_seenrewrite from a last-line seen-marker to byte-offset status-span tracking, and its lazyfm-classify-lib.shsourcing infm-wake-lib.sh- auto-merged cleanly. Verified no orphaned call sites remain:mark_escalated_seen,mark_status_seen,wedge_escalation_deferred, and the recovery-markerreuse-only/health_predicateadditions PR #39 depends on all check out.Verification
bin/fm-lint.sh: clean (ShellCheck 0.11.0, pinned).actionlintisn't installed locally so workflow lint was skipped - not a concern, this diff touches no.github/workflows/files.failed=0:fm-daemon,fm-watch-triage,fm-home-summary-refresh,fm-wake-drain-unread-status,fm-wake-queue,fm-watch-arm,fm-captain-hold-lifecycle,fm-inactive-reconcile,fm-pending-reply,fm-remote-reply,fm-remote-transport-lanes,fm-send-resolve-key.fm-inactive-reconcile.test.sh'sprime_seenhelper calls_fm_status_file_sizedirectly without first calling_fm_wake_require_classify, so a fresh subshell prints a harmless "command not found" before the marker write is skipped. Confirmed bothtests/fm-inactive-reconcile.test.shandbin/fm-wake-lib.share byte-identical to upstream's own shipped versions here, and the test suite still passes end to end.CI is being skipped per the captain's instruction; these are the local checks standing in for it.