fix(bin): preserve captain calls during teardown - #3502
Open
kunchenguid wants to merge 13 commits into
Open
Conversation
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (3): Last reviewed commit: "no-mistakes(document): Document pending-..." | Re-trigger Greptile |
A scout that held its own work item for the captain - which is what captain-hold-lifecycle prefers, "hold the work item the question gates" - was closed by `bin/fm-teardown.sh`'s automatic backlog transition. The completion gate passed, cleanup ran, and the captain's question moved to Done with no recorded answer: the one thing the policy says must never happen. The contract was self-contradictory, so this fixes it at the design level with one owner. `bin/fm-captain-hold.sh` gains the read-only `open` predicate (0 still an open captain call, 1 not, 2 cannot tell) and the `retain` command that carries a call across the removal of the work record that discovered it: it records the finished work's deliverable in the task body and returns the row to Queued, keeping the hold. Teardown asks `open` before its close, and on a captain-held row defers only the close - every other cleanup step runs as before, no pending-close record is staged (a replay would close the call at the next session start), and "cannot tell" refuses before any destructive step rather than reading as permission. `--force` does not lift the deferral: it authorizes discarding unlanded work, never the captain's question. Queued-and-held is the shape every other captain call already has and what `captain_actionable` requires, so the retained call lands in Bearings' Captain's Call instead of reading as work still under way, and the paired meta/backlog invariant is preserved rather than excepted. `fm-captain-hold.sh answer` remains the only act that closes a captain call.
…the established unsafe pending-close diagnostic, released-answer retention recovery queues the ownerless task without restoring its hold, and the lifecycle regression/docs reflect that invariant. Verified with fm-captain-hold-lifecycle, fm-backlog-atomicity, fm-lint.sh, and git diff --check
kunchenguid
force-pushed
the
fm/fm-teardown-closes-captain-held-scout-r1
branch
from
September 2, 2026 07:49
e32f388 to
6a2abd7
Compare
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
Fix a captain-hold-lifecycle INTEGRITY bug at the design level: bin/fm-teardown.sh closed a captain-held scout task, dropping the captain call without a recorded answer - exactly what the captain-hold-lifecycle skill says must NEVER happen.
Reproduced in the main home on 2026-09-01: a scout finished its report; firstmate ran fm-captain-hold.sh hold on that same task (the skill says 'prefer holding the work item the question gates'); fm-captain-hold.sh complete passed; then fm-teardown.sh (report present, landed-work test passed) moved the task to Done, CLOSING the captain-held task with NO recorded captain answer. The contract was self-contradictory: the skill tells firstmate to HOLD the originating work item while teardown's automatic backlog transition CLOSES that item.
The task was to fix it at the design level with ONE owner, choosing the cleaner of (A) teardown refuses or DEFERS the Done transition while the item is captain-held with no recorded answer, keeping the item open with its report recorded as its artifact, or (B) the skill mints a SEPARATE held task so the scout's own row can close. Whichever was chosen had to keep a SINGLE durable captain-call row and NEVER close it without fm-captain-hold.sh answer, and the choice had to be stated in the PR.
Chose (A) deliberately: (B) contradicts the skill's own 'prefer holding the work item the question gates', is enforceable only by agent memory instead of deterministically in the code that actually closes rows, and leaves the same hole open for any other captain-held task (a captain-gated ship, not only a scout). (A) is deterministic, fails closed, covers every kind, and keeps exactly one durable row.
ACCEPTED DECISIONS, all deliberate and already ruled on by the supervisor - do not re-litigate them as if they were oversights:
There is NO remaining accepted-but-unimplemented work in this area. Every remedy the supervisor authorized during earlier review rounds is implemented on this branch.
Regressions live in tests/fm-captain-hold-lifecycle.test.sh and exercise the real executables (fm-captain-hold.sh, fm-teardown.sh, fm-bootstrap.sh, fm-bearings-snapshot.sh) with observable behavior only, never source-text matching: a scout that holds its own work item survives cleanup open, queued, still captain-held, carrying its deliverable, visible in Bearings' Captain's Call, with no pending-close record and no leftover retention record; a --force cleanup leaves it open too; an ordinary non-held scout still closes to Done with its report link; only fm-captain-hold.sh answer closes the call; a concurrent captain answer during retention is neither lost nor reopened; a kill after the reopen recovers through session start to a durably held call; a reported cleanup failure recovers the call as held; a stale record does not mutate a replacement worker; and a released answer completes recovery cleanly without restoring the hold.
Constraints: bash 3.2 safe; bin/fm-lint.sh clean; one sentence per line and plain dashes in tracked Markdown, never an em dash; no agent co-author trailer on any commit; single-owner contracts with the affected script headers and docs/captain-hold-lifecycle.md updated rather than duplicated; bin/fm-lock-lib.sh and bin/fm-wake-lib.sh remain the owners of locking mechanics and bin/fm-timeout-lib.sh the owner of bounded execution.
This run is a re-validation of an already-implemented branch. The PR's automated attestation check is red only because the PR body attestation still names an older head than the current one; the push step re-binding the attestation to the current head is what clears it.
What Changed
Risk Assessment
✅ Low: The change is well-bounded, preserves the stated captain-call invariants across normal, concurrent, and recovery paths, and introduces no substantiated source defect.
Testing
The focused end-to-end lifecycle suite exercised teardown deferral, answer-only closure, pending-close refusal, concurrency, crash/bootstrap recovery, cleanup failures, stale generations, released answers, relocated backlogs, and unreadable state; all scenarios passed, and CLI verification confirmed recovery is the sole retention entry point. No visual artifact was needed because this is a CLI lifecycle change.
Evidence: Captain-hold lifecycle end-to-end transcript
Source: Captain-hold lifecycle end-to-end transcript
Evidence: Public CLI help and rejected standalone retain command
Source: Public CLI help and rejected standalone retain command
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
bin/fm-captain-hold.sh:1033- The required criterion says Queued retention preserves the meta/backlog invariant because teardown removes the worker record, but the advertised standaloneretaincommand callstasks_axi reopenand reports success without removingstate/<id>.metaor requiring a retention marker. Concrete path: invokeretainon an active In-flight captain-held task; it becomes Queued and held while its live worker metadata remains, contradictingbin/fm-backlog-transition-lib.sh:7-16and presenting active work as an ownerless Captain's Call. Guardcommand_retainat this earliest boundary so it can run only within an authorized retention transition, or authorize different standalone semantics; the remedy changes the accepted public command behavior and therefore needs user approval.🔧 Fix: Remove unsafe standalone captain retention command
1 error still open:
bin/fm-captain-hold.sh:463-holdacquires the task lock but does not reject an existing<id>.backlog-closemarker. Concrete sequence: ordinary teardown stages that marker, fails during cleanup and releases the lock;fm-captain-hold.sh hold <id>then succeeds on the still-open row; the next bootstrap replays the older close marker and closes the newly captain-held task without a recorded answer. Under the same lock, refuse creating/reasserting a hold while a pending close exists, before mutating the row, so an authorized close intent cannot survive underneath a new captain call.🔧 Fix: Block captain holds over pending closes
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-captain-hold-lifecycle.test.shbin/fm-captain-hold.sh --helpbin/fm-captain-hold.sh retain sample-task --note finished(confirmed unsupported command exits 2)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.