Skip to content

fix: guarantee secondmate outcomes reach the parent channel - #3569

Open
kunchenguid wants to merge 28 commits into
mainfrom
fm/fm-secondmate-parent-channel-structural-r1
Open

fix: guarantee secondmate outcomes reach the parent channel#3569
kunchenguid wants to merge 28 commits into
mainfrom
fm/fm-secondmate-parent-channel-structural-r1

Conversation

@kunchenguid

@kunchenguid kunchenguid commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Intent

DESIGN-GRADE root-cause fix, captain-approved: make sure ANY captain-facing outcome from a secondmate reaches the parent channel deterministically, not just PR-ready ones. Captain intent verbatim: 'the root problem is not specific to PRs, right? it looks like any message or outcomes from second mates can miss. we need to make sure our fixes are addressing this in a principled, fundamental way, not surgically treating the symptoms of just this PR update miss.' Root cause (main-home investigation 2026-09-02, four instances across two mate homes): watcher delivery to the mate worked within a minute every time; the mate then addressed the captain in its OWN chat (unread by anyone) instead of appending to the parent channel, because AGENTS.md section 9 tells every firstmate to reach the captain and the persona addresses captain, while the charter return-channel rule is a smaller, later rule. PRs, findings, blockers, decisions, and failures all fail the same way. Design goal: the parent channel must NOT depend on the model remembering to write to it.

DESIGN FIRST, reviewed by the captain in this PR: docs/secondmate-parent-channel.md is the design note and the contract owner, and must stay prominent. It evaluates four candidate mechanisms on coverage across all outcome kinds, harness independence, false-positive noise, and failure direction: (A) a secondmate turn-end hook mirroring captain-facing chat text into the parent channel is REJECTED as a delivery mechanism because every firstmate turn contains captain-facing text by mandate, judging which sentence is an outcome is itself model behavior, every harness payload differs (Claude transcript path, Codex and Grok Stop payloads, Cursor stop hook with no text and no blocking, Pi extension, OpenCode plugin, Kimi with no project hooks) so it needs live per-harness proof, it is unbounded noise, and a broken hook fails silent; (B) charter and persona carve-out is ADOPTED as the belt only (it is the instruction that already failed four times but is the only mechanism for outcomes that exist nowhere but the model's reasoning); (C) a deterministic per-poll mirror of child ledgers onto the parent channel is ADOPTED as the guarantee (harness independent, file reads only, per-child cursor and receipts, age threshold so the mate keeps first-responder authority, at-least-once failure direction, mate-side because only the mate watcher can read the mate state and the existing local and remote parent route already carries it); (D) at-source typed lines from the scripts that record outcomes is ADOPTED layered under C for immediacy and richer context. Delivery rule: the machinery reports facts, the mate reports judgement.

Implementation: bin/fm-parent-channel-lib.sh is the single owner of parent-channel resolution (local route = parent home state/.status, remote route = mate home state/parent-replies.status) and idempotent exact-line append; bin/fm-merge-outcome-lib.sh and bin/fm-inactive-reconcile.sh now publish through it (behavior otherwise unchanged). bin/fm-parent-mirror-lib.sh plus bin/fm-parent-mirror.sh sweep every direct child ledger on every mate watcher poll (fm-watch.sh, placed after the slow checks so a merge poll's own loud failure is never masked, before the signal scan so chatty siblings cannot starve it): a done line or legacy free-text captain-relevant line is delivered immediately keyed by ledger offset with the recorded pr=, a scout report pointer data//report.md, mode and yolo; a failed line standing as the last line, and a needs-decision or blocked decision still open in the child's fold, are raised once they stand past FM_PARENT_MIRROR_OPEN_SECS (default 600, valid 60..86400) with deterministic text, keyed mirror--, identified by key plus opening line so a re-opened key is a new delivery, and closed on the parent with a keyed resolved line when the child's fold closes; an untrackable decision line (invalid or reserved key) is delivered at once under the done verb so it never opens an uncloseable parent decision; only whole newline-terminated lines are examined; per-child records live under state/parent-mirror/ and are rewritten atomically; every lock wait is bounded (busy child or busy sweep is left for the next poll) and the teardown-side retire takes no sweep lock so lock order is always meta lock first and no deadlock is possible; a delivery problem or unreadable channel is queued once per unhandled episode as a durable check wake in the mate home and printed only when newly queued, so the mate is woken once per episode not every poll; a main home is a silent no-op. bin/fm-pr-check.sh sweeps the registered child after arming its merge poll so the ready line reaches the parent at registration time with the canonical URL (reported as actionable on stderr if it cannot, poll stays armed). bin/fm-captain-hold.sh publishes needs-decision [key=captain-hold-] on hold and the matching resolved line on answer (including repaired and released), idempotently, reported as actionable if it cannot. bin/fm-teardown.sh runs the child's final sweep under its held meta lock before removing the record, retires the mirror record on success, and keeps or creates an orphan record on failure that later sweeps retry because the ledger outlives the record. bin/fm-inactive-reconcile.sh yields to the mirror in a secondmate home: a child whose ledger already ends in a terminal captain verb is the mirror's evidence and is skipped, while a silent ledger with fm-crew-state done or failed remains the inactive scan's own, so one outcome is never reported twice. The charter scaffold (bin/fm-brief.sh --secondmate) gains a 'The captain and the parent channel' section before the marker rules stating nobody reads the chat, the parent channel IS the captain in that home, the machinery delivers child facts, and only judgement (marked-request answers, recommendations, the mate's own blockers) is the mate's to append. AGENTS.md carries one-line carve-outs at the persona address rule, in section 9 after the reach-the-captain list, and a state tree line for parent-mirror/. docs/architecture.md, docs/remote-secondmates.md, docs/configuration.md, and docs/scripts.md cross-link or index the new owner and scripts; docs/documentation-audiences.json classifies the new doc as maintainer-architecture.

Acceptance and behavioral tests (real executable paths, never source-text): a secondmate turn that talks to the captain without an append yields a parent line because the durable outcome behind it (child ledger line, PR registration, hold record, terminal child) is delivered by the machinery regardless of the turn; a PR-ready registration in a secondmate home yields a parent line (tests/fm-parent-mirror.test.sh test_pr_check_registration_delivers_now); a mate ledger terminal child with no parent line raises a parent wake within one poll (test_watcher_poll_delivers_terminal_child drives the real watcher). tests/fm-parent-mirror.test.sh is new (family secondmate) covering once-only done delivery with context, scout report pointer, whole-line delivery, thresholded decision and failure delivery with keyed close, re-open under the same key, silence for anything handled inside the threshold, untrackable decision lines, remote route, PR registration hook, retire path with orphan retry, bounded lock waits, main-home inertness, once-per-episode unreadable-binding diagnostic, and the real watcher poll. tests/fm-captain-hold-lifecycle.test.sh gains a mate-home case exercising the real hold, answer, and scout teardown hooks. tests/fm-inactive-reconcile.test.sh secondmate cases were changed to silent ledgers (deliberate: the mirror now owns terminal-verb ledgers) and gain a yield test. tests/fm-pr-merge.test.sh's unbound-secondmate case now asserts no merge record fell back to the main-home queue while allowing and requiring the registration's channel diagnostic wake (deliberate loud behavior). tests/fm-brief.test.sh pins the charter rule. bin/fm-test-run.sh registers the new suite, its changed-file families, and its measured duration hint.

Constraints honored: bash 3.2 safe (no associative arrays, no mapfile), shellcheck clean via bin/fm-lint.sh, one sentence per line in Markdown, plain dashes, no agent co-author trailer, firstmate-coding-guidelines followed (one owner per contract, inline-stub in AGENTS.md, script headers own mechanics). Out of scope by design: a parent-side sweep of remote mate ledgers (the parent cannot read them; the mate-side mirror plus the existing remote reply relay covers it), and any harness hook reading assistant text.

What Changed

  • Add a deterministic, poll-driven mirror that delivers child outcomes to local or remote parent channels with durable cursors, receipts, bounded locking, and retryable orphan state.
  • Centralize idempotent parent-channel publishing and integrate it with PR registration, captain holds and resolutions, merge outcomes, inactive reconciliation, watcher polling, and teardown.
  • Document the parent-channel contract, strengthen secondmate charter guidance, and add regression coverage for delivery, deduplication, failure recovery, and lifecycle behavior.

Risk Assessment

✅ Low: The change is broad but its delivery, retry, locking, ledger-folding, and teardown invariants are consistently implemented, with no new material source defect substantiated.

Testing

The parent mirror, captain-hold, inactive-reconciliation, PR-merge, wake, and brief behaviors passed; the broad teardown run exceeded the imposed 180-second cap, but its change-specific orphan-preservation case was rerun independently and passed. An end-to-end CLI demonstration confirmed deterministic parent-channel delivery, update identity preservation, keyed closure, and immediate terminal delivery without a model-authored parent append.

Evidence: End-to-end secondmate parent-channel delivery transcript

Source: End-to-end secondmate parent-channel delivery transcript

$ child ledger contains a captain-facing decision; the mate model does not append to the parent
needs-decision [key=route]: choose north or south
$ sweep at t=1000 (inside grace period)
[parent channel still empty]
$ child updates the same open decision; sweep at t=1030
[parent channel still empty]
$ child updates again; sweep at t=1061 (past original opening threshold)
blocked [key=mirror-5-child-route]: mirror: child=child decision route (opened at line 1) open past 60s without an answer or a captain hold: latest recommendation is west mode=no-mistakes yolo=off
$ child resolves explicitly; next sweep closes the parent decision
blocked [key=mirror-5-child-route]: mirror: child=child decision route (opened at line 1) open past 60s without an answer or a captain hold: latest recommendation is west mode=no-mistakes yolo=off
resolved [key=mirror-5-child-route]: mirror: child=child decision route (opened at line 1) closed
$ child records a terminal outcome; next sweep delivers it immediately
blocked [key=mirror-5-child-route]: mirror: child=child decision route (opened at line 1) open past 60s without an answer or a captain hold: latest recommendation is west mode=no-mistakes yolo=off
resolved [key=mirror-5-child-route]: mirror: child=child decision route (opened at line 1) closed
done [key=mirror-5-child-l190]: mirror: child=child implementation shipped mode=no-mistakes yolo=off

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed (18) ✅
  • 🚨 bin/fm-parent-mirror-lib.sh:363 - Contradicts the required “only whole newline-terminated lines are examined” contract. Although the cursor loop ignores an unterminated tail, last_status_line and both decision-fold calls still consume it. For example, an unterminated failed: build broke observed at t=1000 is recorded as standing and can be published at t=1060 before any newline exists; an unterminated decision similarly starts aging immediately. Derive failure and decision state from the captured newline-terminated span rather than the live whole file.
  • 🚨 bin/fm-parent-mirror-lib.sh:572 - A child retired while its failure or decision is still inside the age threshold loses that outcome permanently. The final sweep returns success after recording the pending terminal/open state, then lines 572-574 delete the record; after metadata removal, later sweeps enumerate neither that child nor its ledger. Orphan sweeps also delete a successful record unconditionally at line 494, even when delayed state remains pending. This contradicts the required deterministic delivery of a standing failed line or open decision. Retain an orphan until all pending thresholded state is delivered or closes.

🔧 Fix: Guarantee complete-line and retired-child outcome delivery
4 errors still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:143 - Contradicts the required “only whole newline-terminated lines are examined” criterion: fm_parent_mirror_owns_ledger calls last_status_line on the live ledger. For working: finishing\ndone: complete without a final newline and current state done, inactive reconciliation yields to the mirror, while the mirror correctly ignores the partial done line, so no outcome is delivered until another write occurs. Compute ownership from the same captured newline-terminated prefix used by the mirror.
  • 🚨 bin/fm-parent-mirror-lib.sh:199 - Contradicts the required bounded-lock guarantee. _fm_parent_mirror_diagnostic calls fm_wake_queued_keys and fm_wake_append, both of which use unbounded fm_lock_acquire_wait; a live-held wake-queue lock during a channel or delivery error can therefore wedge the watcher indefinitely. Route diagnostic queue operations through a bounded shared wake-queue boundary.
  • 🚨 bin/fm-parent-channel-lib.sh:113 - The claimed idempotent exact-line append is a check-then-append race. Two concurrent retries can both observe the line absent and both append it, producing duplicate captain outcomes without error. Serialize the existence check and append at this shared owner using a bounded destination-specific lock; authorization is needed because the remedy adds lock-state machinery.
  • 🚨 bin/fm-watch.sh:1658 - The watcher redirects stderr through the predictable $STATE/.parent-mirror.stderr path without rejecting symlinks. A symlink placed there causes every poll to truncate an arbitrary user-writable target before the mirror starts. Use a private mktemp file under a verified state directory and remove that exact file afterward.

🔧 Fix: Secure watcher mirror diagnostic capture
6 errors still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:143 - The required “only whole newline-terminated lines are examined” invariant is still violated because ownership reads the live ledger. With working: finishing\ndone: complete lacking its final newline and current state done, inactive reconciliation yields to the mirror even though the mirror correctly ignores the partial terminal line. Compute ownership from a captured newline-terminated prefix.
  • 🚨 bin/fm-parent-mirror-lib.sh:203 - The required bounded-lock guarantee is still violated: diagnostics call fm_wake_queued_keys and fm_wake_append, which use unbounded wake-queue lock waits. A live-held wake lock can wedge the watcher precisely while reporting a channel or delivery failure. Route this through a bounded wake-queue operation; adding that shared boundary needs authorization.
  • 🚨 bin/fm-parent-channel-lib.sh:113 - The claimed idempotent exact-line append remains a check-then-append race. Two concurrent publishers can both observe the line absent and append duplicate captain outcomes. Serialize the check and append at this shared owner; the required destination-lock state needs authorization.
  • 🚨 bin/fm-parent-mirror-lib.sh:420 - A failed parent close is never retried unless the child ledger changes again. After close publication fails, the record retains the mirrored opening but advances its cursor; the next unchanged sweep synthesizes that retained entry as still open, so lines 440-442 preserve it indefinitely instead of retrying resolved. Re-fold the ledger whenever a retained close is pending, or otherwise preserve an explicit retryable-close state.
  • 🚨 bin/fm-parent-mirror-lib.sh:528 - An orphan sweep removes its record after successfully examining only the complete prefix, even when the ledger has an unterminated tail. If that tail is a mid-write done, failed, or decision line and its newline lands after metadata removal, no record remains to enumerate the ledger, so the outcome is lost permanently. Retain retired/orphan records while captured bytes extend beyond the last newline; changing retirement persistence semantics should be confirmed by the user.
  • 🚨 bin/fm-parent-mirror-lib.sh:567 - The required PR-registration behavior says the ready line reaches the parent at registration time and an inability is actionable, but a busy sweep or child lock returns success here. Consequently fm-pr-check.sh prints only armed although nothing was delivered until a later poll. Targeted registration sweeps must distinguish “deferred because busy” from successful delivery; that observable behavior requires user confirmation.

🔧 Fix: Retry failed mirrored decision closures
8 errors still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:143 - The required “only whole newline-terminated lines are examined” invariant is still violated by ownership checking the live ledger. With working: finishing\ndone: complete lacking its final newline and current state done, inactive reconciliation yields to the mirror, while the mirror ignores the partial terminal line; if the write never completes, the durable current-state outcome remains unreported. Compute ownership from a captured newline-terminated prefix.
  • 🚨 bin/fm-parent-mirror-lib.sh:203 - The required “every lock wait is bounded” guarantee is still violated: diagnostics call fm_wake_queued_keys and fm_wake_append, both of which wait indefinitely on the wake-queue lock. A live-held wake lock can wedge the watcher while reporting a channel or delivery failure. Route diagnostics through a bounded shared wake-queue operation; adding that boundary requires authorization.
  • 🚨 bin/fm-parent-channel-lib.sh:113 - The claimed idempotent exact-line append remains a check-then-append race. Two concurrent publishers can both observe the line absent and append duplicate captain outcomes. Serialize the check and append at this shared owner; the required destination-lock state needs authorization.
  • 🚨 bin/fm-parent-mirror-lib.sh:535 - An orphan sweep still deletes its record after examining only the complete prefix, even when the ledger has an unterminated tail. If that tail completes after metadata removal, no record remains to enumerate the ledger and the outcome is permanently lost. Retain retired/orphan records while captured bytes extend beyond the last newline; this retirement-persistence change requires authorization.
  • 🚨 bin/fm-parent-mirror-lib.sh:574 - The required PR-registration behavior says the ready line reaches the parent at registration time and inability is actionable, but contention on either the sweep or child lock returns success. fm-pr-check.sh consequently prints only armed although delivery was deferred. Targeted registration sweeps must distinguish contention from successful delivery; that observable behavior requires user confirmation.
  • 🚨 bin/fm-parent-mirror-lib.sh:387 - A later failure with identical text is suppressed if an intervening recovery and recurrence both occur between polls. After failed: build broke is reported, appending working: retrying and then the same failed: build broke leaves terminal_line equal and terminal_reported=1, so the new offset-keyed failure is never published. Tracking the standing failure’s offset would distinguish events, but that adds durable record state and needs authorization.
  • 🚨 bin/fm-captain-hold.sh:534 - A failed parent close cannot be recovered by an idempotent answer retry. Once the backlog mutation succeeds, a retry enters either the already-done branch here or the already-released branch later and returns without calling publish_parent_hold; the parent decision therefore remains open permanently. Republish the matching resolved line in both idempotent retry branches.
  • 🚨 bin/fm-parent-mirror-lib.sh:496 - Mirrored decision keys are not collision-free across children: child a-b with key c and child a with key b-c both publish parent key mirror-a-b-c. The second opening replaces the first in the parent fold, and either close can close the other decision. Use an injective encoding or length-delimited composition; changing the specified parent key format requires authorization.

🔧 Fix: Republish parent hold closures on retries
8 errors still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:143 - The required “only whole newline-terminated lines are examined” invariant is violated because ownership reads the live ledger. With working: finishing\ndone: complete lacking its final newline and current state done, inactive reconciliation yields while the mirror ignores the partial terminal line, leaving the durable outcome unreported. Compute ownership from a captured newline-terminated prefix.
  • 🚨 bin/fm-parent-mirror-lib.sh:203 - The required “every lock wait is bounded” guarantee is violated: diagnostics call fm_wake_queued_keys and fm_wake_append, whose wake-queue lock waits are unbounded. A live-held wake lock can wedge the watcher while it reports a delivery failure. Add a bounded shared wake-queue operation.
  • 🚨 bin/fm-parent-channel-lib.sh:113 - The claimed idempotent exact-line append is a check-then-append race. Concurrent publishers can both observe the line absent and append duplicate outcomes. Serialize the check and append at this shared owner with destination-specific lock state.
  • 🚨 bin/fm-parent-mirror-lib.sh:535 - An orphan sweep deletes its record after examining only the complete prefix even when the ledger has an unterminated tail. If that tail completes afterward, no record remains to enumerate the ledger and the outcome is permanently lost. Retain retired/orphan records while captured bytes extend beyond the final newline.
  • 🚨 bin/fm-parent-mirror-lib.sh:574 - The required registration-time delivery and actionable-failure behavior is contradicted because contention on the sweep or child lock returns success. fm-pr-check.sh can print only armed although delivery was deferred. Targeted registration sweeps must distinguish contention from successful delivery.
  • 🚨 bin/fm-parent-mirror-lib.sh:387 - A recurring failure with identical text is suppressed when recovery and recurrence happen between polls. After reporting failed: build broke, appending working: retrying and the same failure leaves terminal_line equal and terminal_reported=1, so the new offset-keyed event is never published. Persist the standing failure’s offset or equivalent event identity.
  • 🚨 bin/fm-parent-mirror-lib.sh:496 - Mirrored decision keys collide across valid child/key pairs: child a-b with key c and child a with key b-c both produce mirror-a-b-c. One opening can replace or close the other in the parent fold. Use an injective or length-delimited key composition.
  • 🚨 bin/fm-captain-hold.sh:773 - The required rule that captain-hold answers publish a matching resolved line and report inability as actionable is still violated by the batch answers path. An already-recorded answer returns closed here without retrying publication; on a first attempt, line 789 captures command_answer’s actionable delivery warning and discards it when the command succeeds. Route both successful and idempotent batch answers through the shared publication retry and preserve its actionable stderr.

🔧 Fix: Harden deterministic parent outcome delivery
3 issues (2 errors, 1 warning) still open:

  • 🚨 bin/fm-captain-hold.sh:157 - Captain-hold publications are deduplicated solely by their rendered text, but the existing lifecycle explicitly supports a new answer on a re-held task. After releasing a hold, re-holding the same task with the same reason suppresses the new needs-decision line; with a different reason the opening is appended, but the later identical resolved line is suppressed, leaving the parent's fold open. This contradicts the required publication of each hold and matching resolution while preserving idempotent retries; authorize an occurrence identity derived from the durable hold lifecycle.
  • 🚨 bin/fm-parent-channel-lib.sh:139 - An interrupted append can leave the complete intended text without its terminating newline. On retry, grep -x treats that unterminated tail as already delivered, so the function returns success without repairing framing; the next outcome is concatenated onto it and both channel events become malformed. The shared append boundary should only deduplicate newline-terminated records and repair or delimit an incomplete tail before appending.
  • ⚠️ bin/fm-parent-mirror-lib.sh:226 - The required once-per-unhandled-episode diagnostic is still racy because queued-key inspection and append use separate lock acquisitions. A watcher sweep and concurrent targeted PR sweep can both observe the key absent, then append duplicate queue rows and emit duplicate wakes. Provide a bounded append-if-key-absent operation that performs the check and append under one wake-queue lock.

🔧 Fix: Preserve distinct hold and diagnostic occurrences
3 issues (2 errors, 1 warning) still open:

  • 🚨 bin/fm-captain-hold.sh:578 - The required distinct delivery for every re-held occurrence still fails when the captain gives the same answer again. After occurrence 1 is released, re-holding opens occurrence 2; answering with the same digest enters this interrupted-close branch, reuses occurrence 1, and publishes resolved for occurrence 1, leaving occurrence 2 open forever. This contradicts the criterion that “each hold-and-answer pair is a DISTINCT delivery.” Distinguishing an interrupted close from a new same-answer occurrence requires an explicit durable hold generation or equivalent durable state, so that remedy needs user authorization.
  • 🚨 bin/fm-wake-lib.sh:1528 - The new bounded diagnostic append is not fully bounded: after acquiring the wake-queue lock it calls _fm_wake_append_locked, which calls _fm_recovery_marker_publish; that function waits indefinitely on .watcher-down.lock. A live holder of that lock can therefore wedge a mirror sweep—and the watcher—while reporting a broken parent channel, violating the required bounded-lock invariant. Use a bounded recovery-marker acquisition throughout this bounded append path and release the queue lock on every failure.
  • ⚠️ bin/fm-parent-mirror-lib.sh:230 - fm_wake_append_if_key_absent_bounded returns 3 when the diagnostic is already queued, but this branch treats every nonzero result as contention and prints the payload to stderr. Consequently every subsequent watcher poll logs the same standing problem, contrary to the once-per-unhandled-episode contract. Treat return 3 as an expected quiet no-op and reserve stderr degradation for actual append or lock failures.

🔧 Fix: Track hold generations and bound diagnostic locking
2 errors still open:

  • 🚨 bin/fm-captain-hold.sh:399 - The generation marker is embedded in unrestricted task text and this global substitution deletes every matching line, including legitimate body or captain-decision content. A concrete failure is an answer containing Captain hold generation: 99: if closing is interrupted after recording the answer, retry reads 99 instead of the real generation, writes another resolution, and closes the wrong parent key, leaving the original opening unresolved. Use a structurally distinguishable durable field/block with compatible parsing; changing the persisted representation requires authorization.
  • 🚨 bin/fm-teardown.sh:2884 - This contradicts the required criterion that teardown “keeps or creates an orphan record on failure that later sweeps retry.” If delivery fails and the orphan record cannot be written (for example state/parent-mirror is an unsafe symlink), this branch merely claims the record was retained and continues to remove the child metadata. Later sweeps enumerate metadata and orphan records, not bare ledgers, so the outcome becomes permanently unreachable. Teardown must not remove the discoverable child record unless delivery succeeded or a durable orphan record is confirmed.

🔧 Fix: Protect hold state and orphan delivery
2 issues (1 error, 1 warning) still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:334 - Mirror scratch files use predictable PID-based paths and are opened with truncating redirections before validating that state/parent-mirror is a real directory. A symlinked directory or pre-created child.record.capture.<watcher-pid> symlink can therefore make a sweep truncate an arbitrary writable target before the later record-write safety check fails. Validate the directory before any write and allocate all capture, prefix, and span files with mktemp.
  • ⚠️ bin/fm-captain-hold.sh:541 - The new structured-state claim is still bypassed when selecting the next generation: resolution_record_count globally counts marker phrases anywhere in unrestricted task text. For example, after generation 1 is released with an answer containing Resolution recorded by fm-captain-hold., the next hold is labeled occurrence 3 rather than 2. This contradicts the documented guarantee that unrestricted captain prose cannot impersonate lifecycle state; confine counting to structurally valid resolution headers. Because this challenges the accepted persisted-generation behavior, the remedy needs user review.

🔧 Fix: Secure mirror scratch and generation counting
2 issues (1 error, 1 warning) still open:

  • 🚨 bin/fm-teardown.sh:2892 - This contradicts the required guarantee that a failed final delivery leaves an orphan record whose ledger “outlives the record” for later sweeps. After accepting a durable orphan, teardown calls status_retire_presentation_task, which deletes state/<child>.status; the next orphan sweep therefore removes the orphan without delivering anything. An unterminated tail is lost through the same path because retire can return success while retaining tail=1. Preserve the ledger at the shared status-retirement boundary until the orphan has delivered all complete bytes and no tail remains.
  • ⚠️ bin/fm-captain-hold.sh:411 - The fallback still lets unrestricted captain prose impersonate a resolution because it recognizes header-shaped sequences without an end delimiter. A legacy released record whose answer contains Resolution recorded by fm-captain-hold., digest/mode fields, a blank line, and Captain decision: is counted twice, so its next hold publishes occurrence 3 instead of 2. Confine counting to genuinely delimited script-owned blocks as required by the accepted generation contract.

🔧 Fix: Preserve orphan ledgers and delimit resolution records
1 error still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:362 - The mirror records the child's spawn_gen but never compares it with rec_incarnation. Concrete sequence: a failure is observed and its threshold clock starts, the child is relaunched within the window (which changes spawn_gen but does not append a status line), and a later poll still sees the old failed tail and publishes it once the old clock expires. This violates the documented invariant that a failure handled by relaunch inside the window is never raised. Mark the prior-generation terminal as handled when the incarnation changes, or append a deterministic generation boundary at the shared relaunch transition so the old failure cannot remain the standing failure; ensure a genuinely new failed line at a later offset remains reportable.

🔧 Fix: Suppress relaunched child failures
2 errors still open:

  • 🚨 bin/fm-parent-channel-lib.sh:138 - An existing destination is rejected only when it is a symlink, not when it is a FIFO or other non-regular object. With state/<mate>.status pre-created as a FIFO, grep at line 147 blocks indefinitely waiting for a reader, wedging the mate watcher despite the bounded-delivery contract. Require an existing destination to be a regular, non-symlinked file before reading or appending.
  • 🚨 bin/fm-parent-mirror-lib.sh:587 - The intent requires mirrored outcomes to carry the recorded pr=, mode, and yolo context, including outcomes retried from durable orphan records. After teardown removes the child metadata, this orphan path calls _fm_parent_mirror_child with an empty metadata path, so a previously failed final delivery retries without that context; for example, done: checks green with the canonical PR only in metadata reaches the parent without its URL. Preserving this context requires extending the orphan record or otherwise retaining durable metadata, so the remedy needs authorization.

🔧 Fix: Validate channels and preserve orphan context
1 warning still open:

  • ⚠️ bin/fm-captain-hold.sh:561 - A failed first hold consumes an occurrence even though no hold was created. Concrete sequence: write_hold_generation records generation 1, tasks_axi hold then fails transiently, and a retry sees hold_kind != captain plus generation 1 and advances to occurrence 2. The first successfully published hold is therefore mislabeled occurrence 2. Reuse an unresolved recorded generation when recovering this interrupted transition, and increment only when that generation has a completed resolution.

🔧 Fix: Reuse generations after interrupted holds
2 issues (1 error, 1 warning) still open:

  • 🚨 bin/fm-captain-hold.sh:649 - The new parent-close path still trusts the permissive body_has_resolution_record and unscoped digest/mode readers, while occurrence parsing accepts only delimited records. A legitimate pre-change resolution retry therefore publishes resolved [key=captain-hold-<id>-0], and matching undelimited header-shaped prose can impersonate a resolution. This contradicts the required guarantees that legacy records remain recognized and that “unrestricted captain prose cannot impersonate lifecycle state.” Parse one validated record consistently for presence, digest, mode, and generation, with a safe anchored legacy fallback.
  • ⚠️ bin/fm-parent-mirror-lib.sh:350 - Every poll reads and copies each entire child ledger from byte zero despite maintaining a durable offset, and any changed ledger is then folded in full. A large append-only ledger therefore imposes unbounded repeated I/O and parsing on the watcher, delaying other child delivery and the following signal scan. After the initial recovery scan, process only bytes from the recorded cursor and evolve the stored terminal/open state incrementally.

🔧 Fix: Validate resolutions and incrementally fold ledgers
1 error still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:456 - Incremental terminal selection treats a trailing blank line as the ledger's last event, unlike last_status_line. For failed: boom\n\n, fm_parent_mirror_owns_ledger sees the last nonblank failure so inactive reconciliation yields, but this code selects the blank line, clears terminal state, and never reports the failure. Select the last nonblank processed line while retaining its absolute byte offset.

🔧 Fix: Preserve failures across trailing blank lines
2 errors still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:346 - A ledger identity change or shrink discards open_lines before reconciling mirrored decisions. Concrete sequence: decision api is mirrored, the ledger is replaced during recovery with a new working: ledger, and this reset erases the receipt without publishing resolved, leaving the parent decision open forever. Preserve previously mirrored entries as pending closures across the reset, with the existing durable retry behavior, before folding the new ledger identity.
  • 🚨 bin/fm-parent-mirror-lib.sh:346 - The same ledger reset retains the previous terminal fields. If an unreported failed: old failure ledger is replaced or truncated to blank lines, terminal selection starts from that stale failure at line 455 and eventually reports it even though it is absent from the new ledger. Clear terminal line, offset, age, and reported state whenever identity or size invalidates the cursor.

🔧 Fix: Reconcile decisions and failures across ledger resets
2 errors still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:346 - Truncation is detected only when the final file size is below the cursor. If a ledger is truncated in place and rewritten to at least the previous offset before the next sweep, its inode is unchanged and this condition does not reset; scanning begins inside the replacement content, so complete outcomes before the old offset are permanently missed and old open decisions can remain falsely open. The durable fix needs a validated prefix checkpoint or equivalent reset identity; because that extends the persisted record/schema, authorize the remedy before implementation.
  • 🚨 bin/fm-parent-mirror-lib.sh:592 - A detected ledger replacement resets opening line numbers but mirrored decision keys and exact publication text contain no ledger generation. Concrete sequence: api opens at line 1 and is mirrored, the ledger is replaced with the identical line, reset publishes the old resolution, and after the threshold the new opening is treated as already appended because its key and full line equal the old opening; the record marks it mirrored while the parent fold remains resolved. Include a durable ledger occurrence in event identity; this requires persisted identity semantics and therefore needs user authorization.

🔧 Fix: Document append-only ledger assumptions
1 error still open:

  • 🚨 bin/fm-parent-mirror-lib.sh:534 - Every repeated needs-decision or blocked line for an already-open key replaces its origin, incorrectly treating an update as a close and reopen. For example, after api has been mirrored, appending another needs-decision [key=api] emits resolved for the original and starts a fresh threshold, leaving the parent falsely closed while the child remains continuously open; repeated updates can postpone delivery indefinitely. Assign a new origin only when the key was not open before processing the line, while retaining the original age/receipt and current folded note and verb until an explicit resolved or captain-held transition.

🔧 Fix: Preserve decision identity across updates
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-parent-mirror.test.sh
  • bash tests/fm-captain-hold-lifecycle.test.sh
  • bash tests/fm-inactive-reconcile.test.sh
  • bash tests/fm-pr-merge.test.sh
  • bash tests/fm-wake-queue.test.sh
  • bash tests/fm-brief.test.sh
  • bash tests/fm-teardown.test.sh (exceeded the 180-second local cap after its relevant case passed)
  • Focused execution of test_parent_mirror_requires_durable_orphan_before_record_removal and test_bounded_wake_queue_operations
  • Manual bin/fm-parent-mirror.sh sweep workflow using an isolated secondmate home: opened and updated a decision without any model parent append, delivered the latest state after the original threshold, closed it on resolution, and immediately delivered a terminal outcome
  • git status --short confirmed testing left the worktree clean
✅ **Document** - passed

✅ No issues found.

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

🔧 Fix: Fix ShellCheck issues in mirror and hold tests
✅ Re-checked - no issues remain.

✅ **Push** - passed

✅ No issues found.

Add the design note and contract owner for how a captain-facing outcome
inside a secondmate home reaches the parent firstmate without depending on
the mate model remembering to append it: a deterministic ledger mirror as
the guarantee, at-source typed lines for immediacy, and the charter carve-out
as the belt, with the turn-end text mirror rejected as harness-dependent and
model-behavior-bound.
…erministically

Any captain-facing outcome from a secondmate could miss when the mate model
addressed the captain in its own unread chat instead of appending to the
parent channel. Make delivery independent of model behavior:

- bin/fm-parent-channel-lib.sh is the one owner of channel resolution and
  idempotent append; the merge outcome path and the inactive-outcome scan now
  publish through it.
- bin/fm-parent-mirror.sh and its library sweep every child ledger on each
  watcher poll in a secondmate home: a done line is delivered at once with
  its recorded PR, report pointer, mode, and merge posture; a failed line, an
  open decision, or an open blocker is raised once it stands past a threshold
  and closed on the parent when the child's own fold closes it. Records are
  per-child cursors under state/parent-mirror/, lock waits are bounded, and
  every delivery problem is queued once as a durable wake.
- bin/fm-pr-check.sh delivers the ready line at registration time,
  bin/fm-captain-hold.sh publishes holds and answers, and bin/fm-teardown.sh
  runs the child's final sweep before removing its record, keeping an orphan
  record when delivery fails.
- The inactive-outcome scan yields terminal-verb ledgers to the mirror in a
  secondmate home so one outcome is never reported twice.
- The charter opens with the parent-channel rule and confines the mate's own
  appends to judgement; AGENTS.md carries the carve-out at the persona
  address rule and the escalation list.

docs/secondmate-parent-channel.md owns the design and the contract.
A decision re-opened under the same key with the same note after a close
produced a byte-identical mirror line and was deduplicated away. Carry the
opening line from the classifier's open-decision origins in the record and
in the mirrored text so every opening is a distinct delivery.
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Reviews (3): Last reviewed commit: "no-mistakes(ci): Fixed captain-answer re..." | Re-trigger Greptile

Comment thread bin/fm-captain-hold.sh Outdated
…w retry the prior durable parent-channel close and refuse to advance while delivery is unavailable, preventing orphaned decisions. Teardown skips channel resolution when no ledger evidence exists, and affected secondmate fixtures now include valid parent bindings and required libraries. Updated design documentation and behavioral regression coverage. Relevant captain-hold, mirror, teardown, backlog, home-summary, gotmp, public-followup, Herdr checkpoint, and lint checks passed
Comment thread bin/fm-captain-hold.sh Outdated
…cation failures return nonzero instead of falsely reporting success. Batch retries now count failed publication as skipped. Added regression coverage and verified with captain-hold lifecycle tests, parent-mirror tests, full lint, and git diff checks
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