fix: repair legacy Herdr task bindings - #10
Merged
Conversation
Task records written before endpoint_task_id= existed can never pass fm_backend_validate_task_endpoint on an opaque backend, so teardown and control refuse them forever while the watcher keeps alerting on their finished endpoints. Add bin/fm-herdr-legacy-repair.sh: an explicit, deterministic, idempotent bind-only repair for a legacy primary-home Herdr record. It restores the single missing endpoint_task_id= line only when independent evidence all agrees - exact record shape via the shared validator, worktree/project identity and origin agreement, clean worktree, landed work through the shared landed predicates, and live read-only Herdr evidence (structured pane-gone, or exact workspace/tab/pane topology with an fm-<id> tab label, an absent or finished agent, and a matching foreground cwd). Any missing, ambiguous, mismatched, dirty, unlanded, live-working, or unreadable evidence refuses with a concrete diagnostic and preserves everything. It never closes a pane, deletes a record, or issues a mutating Herdr command; cleanup remains fm-teardown.sh, which re-runs its own complete landed-work and confirmed-close safety against the repaired record. Extract teardown's landed-work predicates verbatim into bin/fm-landed-lib.sh so both callers share one owner; teardown keeps its historical wrapper names and messages. The validator's legacy-Herdr refusal now names the repair path. tmux legacy records still self-identify by window name and need no repair; zellij/orca/cmux legacy records share the masking condition but stay out of this narrow path and refuse by name. Regression coverage: tests/fm-herdr-legacy-repair.test.sh (modern bound no-op, repair-exactly-once, idempotent rerun, every mismatch class, dirty/ unlanded, active/nonterminal, missing/ambiguous fields, unrelated-record and default-session preservation, refusal before any lifecycle mutation, lock contention) and tests/fm-herdr-legacy-repair-e2e.test.sh against real Herdr through the guarded lab with the default-session tripwire.
The partial-bin teardown fixture was broken by two upstream reconciles it never followed: it linked fm-wake-lib.sh twice (an ln 'File exists' error on every run), and it lacked fm-session-lock-lib.sh, which the tmux adapter now sources - a missing sourced sibling aborts teardown mid-kill under set -e even inside a guarded call, so the suite failed on main before this branch. Replace the duplicate link with fm-landed-lib.sh (teardown now sources the shared landed-work predicates) and add the missing session-lock lib, restoring all three gotmp assertions. Also restructure the legacy-repair suite's default-branch fallback so the checkout retry is an explicit if rather than an A && B || C chain (shellcheck SC2015).
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
Implement a small, guarded Firstmate maintenance fix for legacy Herdr task records that predate endpoint_task_id= and therefore can never be cleaned after their work has landed. Observed symptom: repeated monitoring alerts from the inactive firstmate-source-reconciliation session even though PR 9 is merged, its no-mistakes run completed 13/13 checks, and its branch is clean - normal cleanup correctly refuses because the opaque Herdr endpoint lacks an exact task binding. The captain explicitly authorized a safe reconciliation/cleanup fix for this Firstmate case.
Requirements and accepted decisions:
What Changed
fm-landed-lib.shfor teardown and repair while preserving teardown’s complete landed-work decision.Risk Assessment
✅ Low: The guarded repair is well-bounded, the prior review findings are resolved, and no remaining source-verifiable defect or intent contradiction was substantiated.
Testing
The author-reported prior validation remained baseline context; this phase independently exercised the guarded repair’s full portable safety matrix, the narrow gotmp cleanup regression, and real Herdr 0.8.0 behavior. After applying the repository’s documented test-only validation-agent bypass, real-runtime mismatch refusal, exactly-once binding, idempotency, pane preservation, and isolated-session protection all passed; a reviewer-visible CLI transcript was captured, and the worktree remained clean.
Evidence: Real Herdr legacy-repair E2E evidence
Real Herdr 0.8.0 transcript showing mismatch refusal, exactly-once repair, idempotent rerun, and pane preservation.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (4) ✅
bin/fm-herdr-legacy-repair.sh:142- Intent requires “kind=secondmate ... refuse by name” and “Any missing, duplicate ... evidence refuses,” but this hunk reads the lastkind=value and only compares it withsecondmate. An ambiguous record containingkind=secondmatefollowed bykind=shippasses this scope gate and can be bound when the remaining evidence agrees. Require exactly one unambiguous primary-taskkind=value before candidate validation, with an executable regression covering this duplicate ordering.🔧 Fix: Reject ambiguous legacy Herdr task kinds
1 error still open:
bin/fm-herdr-legacy-repair.sh:198- The requirement says “Any missing, duplicate ... ambiguous ... evidence refuses,” but this line silently selects the lastpr=value. A legacy record with duplicate PR fields can therefore use the last merged PR to prove unpushed work landed and receive a binding while its evidence remains ambiguous. This repeats the same causal theme as the fixed duplicate-kind bypass; require zero or one unambiguouspr=record before landed-work validation and add an executable duplicate-PR refusal regression.🔧 Fix: Reject ambiguous legacy Herdr PR evidence
4 errors still open:
bin/fm-herdr-legacy-repair.sh:79- Intent requires the script’s “header/--help [to own] the mechanics,” butusageprints only lines 2–15, omitting even the Usage line plus all evidence, refusal, locking, and mutation mechanics. Expand--helpto expose the owned operational contract.bin/fm-herdr-legacy-repair.sh:70- Intent requires “the single mutation” to be the atomic metadata write and refusals to preserve everything, but sourcingfm-wake-lib.shrunsmkdir -p "$STATE"before help, argument validation, gate refusal, or metadata checks. Thus--helpor a refused invocation against a missing state directory creates durable state. Avoid the side-effecting source path until an existing valid state directory is proven, or use side-effect-free lock support.bin/fm-herdr-legacy-repair.sh:123- Intent requires secondmate homes to “refuse by name,” but-efollows symlinks, so a secondmate home with a dangling.fm-secondmate-homemarker is treated as primary and its ordinary ship/scout record can be bound. Treat any marker path, including a symlink or otherwise unreadable marker, as refusal evidence.tests/fm-herdr-legacy-repair.test.sh:351- Intent explicitly requires public-interface coverage for a mismatched pane, but this topology test covers tab, workspace membership, label, cwd, and unreadable responses without ever returning a differentpane_idor staging awindow/herdr_pane_idmismatch. Add an executable refusal-and-preservation case for the pane mismatch.🔧 Fix: Harden legacy Herdr repair preflight and topology
1 error still open:
bin/fm-herdr-legacy-repair.sh:300- Intent requires “a present pane must ... hold either no agent or a finished idle/done agent” and says unreadable evidence must refuse, but this path ignores the agent-name field:{"result":{"agent":{"agent_status":"idle"}}}is classified as live, parsed as an empty name plusidle, and authorizes the binding. Require a nonempty registered-agent identity before accepting idle/done, with an executable malformed-agent refusal case.🔧 Fix: Reject malformed legacy Herdr agent identities
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-herdr-legacy-repair.test.shbash tests/fm-herdr-legacy-repair-e2e.test.sh— initial standalone attempt was correctly stopped by the validation-agent lifecycle guard; no product logic ranFM_GATE_REFUSE_BYPASS=1 bash tests/fm-herdr-legacy-repair-e2e.test.shbash tests/fm-gotmp.test.shFM_GATE_REFUSE_BYPASS=1 bash tests/fm-herdr-legacy-repair-e2e.test.sh 2>&1 | tee /var/folders/pl/kn_kjwr11lv6jtr8pt8j93y00000gn/T/no-mistakes-evidence/01M017ZV9SB2MW9EP28RTNQMVF/herdr-legacy-repair-real-e2e.txtgit rev-parse HEADandgit status --short— confirmed target commit166bb99f35599d3577955b7109495fd2d7985b13and no testing residue✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.