fix(bin): preserve Relay follow-up loops until explicit disposition - #2763
Merged
Conversation
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains. Reviews (4): Last reviewed commit: "no-mistakes(lint): Remove unused bounded..." | Re-trigger Greptile |
Delivering a promised-final reply was deleting the only record that tied a public thread to later work, so a follow-on ship silently owed no closing reply. Retain the registration after delivery, rechain follow-on work onto the same thread, and make retire --reason the only close.
…n terminal states
kunchenguid
force-pushed
the
fm/relay-followup-baton-fix-r1
branch
from
August 22, 2026 01:08
7070f12 to
51bdbca
Compare
Owner
Author
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
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
Ship captain-authorized disposition (a): stop firstmate silently dropping the CLOSING public reply across a Relay investigation-to-fix chain. Delivery retains the registration as state=delivered at the five non-retire removal sites; retire --reason is the only close and must persist a private receipt (reason= and retired_at=, mode 600) before unlinking; pending prints open-loop; rechain hands the thread on fail-closed; G1-G4 as designed. G3 teardown warning stays ship + recorded pr= (captain approved as-is twice; session-start G1 covers local-only/scout/undispatched). brief must emit actual required deliverable keys and fail closed when they cannot be read. Unresolved pending lines classify followup_expires_at the same way open-loop does.
Later accepted review/CI decisions: stamp pending settled records delivered; serialize rechain claims; preserve delivered/retired register state; do not report a missing/retired loop as OPEN; a failed source retirement must keep the source claimed by the one published destination (rechain_to). The Linux CI failure of "the retirement receipt must be private" was a GNU vs BSD stat probe (GNU stat -f is --file-system and can succeed with a non-600 value); the receipt itself is already chmod 600. Keep a Linux-safe mode probe (stat -c first, or uname Darwin vs else). Greptile is advisory, not a required merge check.
Homes pick this up after merge + self-update. One PR. Do not merge.
What Changed
state=delivered, surface open loops and expiry warnings, and requireretire --reasonto persist a private receipt before removal.rechainhandling that binds one follow-on obligation to the retained thread context and emits briefs with the actual required deliverable keys.Risk Assessment
✅ Low: The durable follow-up lifecycle changes are internally consistent with the stated intent, and no material source defect was found.
Testing
The targeted end-to-end public-followup suite passed, including retained delivery, pending/open-loop, rechain, retirement receipt, expiry, teardown, brief, and promotion scenarios; manual CLI evidence also demonstrated valid brief output, malformed-contract rejection, and warning-with-success remote promotion. No worktree artifacts remain.
Evidence: Public-followup brief and remote-parent promotion user flow
Source: Public-followup brief and remote-parent promotion user flow
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (6) ✅
bin/fm-promote.sh:211- The accepted requirement says promotion must resolve the parent “the same way bin/fm-teardown.sh does” and warn rather than block. For a durableroute=remote, promotion unconditionally does nothing, while teardown checks the child home’s own.envtoken and treats an active Relay as unresolved. Thus a remote-marked child with local Relay consent silently receives neither a warning nor a rechain hint after promotion. Mirror teardown’s remote-route token check and emit the non-blocking unresolved-parent warning.bin/fm-public-followup.sh:308- Re-registering a delivered obligation preservesstate=deliveredbut rewrites its relation, work home, work ID, generation, and request fields from the new arguments. If the typed obligation has two valid work relations, replayingregisterwith the second silently moves the retained baton: promotion/handoff hints now target the wrong work and retirement clears the wrong legacy link. At the locked registration boundary, require an existing record’s identity fields to match, or preserve the complete delivered record unchanged.🔧 Fix: Preserve delivered follow-up bindings during registration replay
4 issues (3 errors, 1 warning) still open:
bin/fm-public-followup.sh:1228- A delivered loop bound tosecondmate:<id>cannot be retired after that secondmate home is removed. Delivery already clears the legacy link and teardown permits delivered registrations, butretirecallsclear_public_followup_link, which requires resolving the now-deleted child home and therefore refuses the only supported close. At the shared legacy-cleanup boundary, treat a provably absent legacy link as already cleared while still failing on unverifiable state.bin/fm-public-followup.sh:1135- On a first rechain attempt, an already-existing obligation with matching request, expected-final data, expiry, and binding is accepted as the destination. If its retained registration is alreadystate=delivered, the source is retired without creating fresh owed work, silently collapsing the baton. Require the destination ID to be unused whenrechain_tois first claimed; permit an existing destination only when resuming that recorded claim.bin/fm-promote.sh:211- The accepted requirement says promotion must resolve the parent the same way teardown does and warn rather than block. Forroute=remote, promotion unconditionally does nothing, while teardown checks the child home's own.envtoken and marks an active local Relay binding unresolved. Such a promoted scout receives neither the unresolved-parent warning nor a rechain hint. Mirror teardown's remote-route token check and emit the non-blocking warning.bin/fm-public-followup.sh:607- Ifretireremoves a registration afterpendingsnapshots registry IDs but before this stamp,fm_pf_registry_stamp_deliveredreturns 3 andpendingaborts. Session start then discards the command's entire output, temporarily hiding unrelated loops. Distinguish a concurrently retired record by its receipt and skip it; continue failing for an unexplained missing registration.🔧 Fix: Harden public follow-up retirement and rechain races
2 errors still open:
bin/fm-public-followup.sh:677- When both the secondmate metadata and registry binding are absent,public_followup_secondmate_homereturns 3, andclear_public_followup_linktreats that as proof the legacy link is gone. A still-existing but unbound child can therefore retainx_requestwhile retire records a receipt and removes the only registration. Treat missing bindings as unverifiable; only accept a recorded canonical child path that is now absent, or persist/clear this state at the teardown boundary.bin/fm-promote.sh:211- The accepted requirement says promotion must resolve the parent as teardown does and warn rather than block. Forroute=remote, promotion does nothing, while teardown checks the child home's.envand marks an active local Relay token as unresolved. Such a promotion silently emits neither the unresolved-parent warning nor a rechain hint. Mirror teardown's remote-route token check without refusing the kind flip.🔧 Fix: Fail closed on unresolved secondmate retirement
3 errors still open:
bin/fm-public-followup.sh:712- Cleanup resolves the current home for asecondmate:<id>but never compares it with the registration's newwork_home_path. If the original home is torn down and that ID is reassigned, retiring the old loop can run--clearagainst the replacement home's same-named task, silently deleting an unrelated Relay link. Likewise, rc=3 accepts any newly recorded absent path without proving the original path is absent. Enforce the recorded canonical path at this shared cleanup boundary before clearing or treating the link as gone.bin/fm-public-followup.sh:354- The required criterion says brief must “emit actual required deliverable keys and fail closed when they cannot be read,” but this jq filter silently drops invalid array members. Forrequired_deliverables=["pr_url",7], brief succeeds and emits onlypr_url, producing an incomplete contract. Validate the entire non-empty array before emitting any keys, including the key shape accepted by fm-public-followup-emit.sh.bin/fm-promote.sh:211- The accepted requirement says promotion must resolve the parent the same way teardown does and warn rather than block. Forroute=remote, promotion does nothing, while teardown checks the child home's.envtoken and treats an active local Relay binding as unresolved. Such a promotion emits neither the required warning nor a rechain hint. Mirror teardown's remote-route token check without refusing the kind flip.🔧 Fix: Bind secondmate cleanup to its recorded canonical home
2 errors still open:
bin/fm-public-followup.sh:361-briefclaims to print an exact command, but each generated deliverable line ends with two literal backslashes and the expansion adds a blank line before--outcome-text. Pasting the output terminates the emitter command early and attempts to execute--outcome-textseparately. Emit one continuous shell command with exactly one continuation backslash per intermediate line.bin/fm-public-followup.sh:1105- The required fail-closed rechain path rejects onlyexpired;fm_pf_followup_window_classcan returnunknownwhen the retained timestamp cannot be parsed, yet rechain proceeds to create and bind a destination without proving the thread remains reachable. Accept onlyokorclosing, and refuseunknownbefore claiming or publishing the destination.🔧 Fix: Fix rechain command output and expiry validation
3 errors still open:
bin/fm-public-followup.sh:712- A pre-upgrade secondmate registration has nowork_home_path, so cleanup now fails before resolving its otherwise-valid current home. After delivery,pendingcan stamp it delivered, but every supportedretire --reasonstill fails, permanently stranding the loop. Support legacy records at this shared cleanup boundary without weakening the reassigned-home check for newly recorded paths.bin/fm-public-followup.sh:319- An open registration may still be rewritten to another valid relation while delivery is posting because delivery does not hold the registration lock across validation, legacy-link cleanup, and the delivered stamp. With valid relations A and B, a concurrent replay binding B can land after delivery validates A; delivery then clears B's link and stamps the retained baton against B, leaving A unreconciled without reporting an error. Serialize this transition at the registration boundary or use an identity-checked atomic stamp.bin/fm-public-followup-lib.sh:278- The fail-closed expiry check is bypassable on GNU systems becausedate -daccepts values such astomorroweven though this function claims to parse RFC3339 timestamps. Such a corrupted retained value classifies asokorclosing, allowingrechaininstead of refusing an unverifiable thread window. Validate the exact supported RFC3339 shape before invoking platform-specific date parsing.🔧 Fix: Validate brief keys and warn on remote promotion
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-public-followup.test.shExecutedfm-public-followup.sh briefwith valid and malformed typed deliverable contracts, confirming executable output and fail-closed behavior.Executedfm-promote.shfor a remote-parent secondmate with a local Relay token, confirming non-blocking warning and persistedkind=ship.✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: Remove unused bounded-wait loop variable
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.