fix(branchsync): settle wedged custody records via axi sync --recover --keep-local (#824)

[hmcdaniel03]

Intent

Fixes #824: a custody record whose gate branch SHA and preserved pipeline head SHA disagree cannot currently be recovered or aborted — recover_custody keeps being advertised, both axi sync --recover and axi sync --recover --keep-local refuse with the same blocked_recover_gate_diverged, and axi abort --run <id> is an idempotent no-op on the already-failed run.

This branch implements the settlement described in the issue:

• axi sync --recover --keep-local settles a wedged custody record when the bound run is terminal and the preserved head is either missing or in disagreement with the gate. Custody is returned at the kept local head, and where the gate branch still names a different head it is compare-and-swapped onto the kept head. Every still-reachable copy of the recorded head is pinned first, and the settlement refuses rather than proceeding when a surviving head cannot be anchored.
• Terminal-run abort names that settlement instead of being a bare no-op.
• Inspection and the TUI both advertise it, so the CLI and the TUI have the same exit.
• Default --recover behavior is unchanged.

Addressing your review

Rebased onto current main. Only AGENTS.md conflicted, because main extracted that section into .agents/skills/; the resolution takes main's copy verbatim and re-homes our prose into .agents/skills/branch-sync-and-push-safety/SKILL.md. AGENTS.md is byte-identical to main. #883 is mergeable against kunchenguid:main.

Gap (1) is closed by construction rather than by cleanup. The displaced-gate-head anchor guards exactly one operation — the compare-and-swap that moves the gate branch off gateHead — and nothing before that swap can strand it, because refs/heads/<branch> still names it. So the write now happens immediately before the swap, and every refusal that can precede it leaves no anchor of its own. That is stronger than deleting the ref on the refusal path, because a cleanup can itself fail and leave the same false claim.

The accounting, since our earlier "four" caused confusion: at the previous tip recoverKeepLocal had 8 refusal returns, 5 of them after the anchor write, and 4 of those 5 made the untrue "no files or refs were changed" claim — the fifth said "no local files or refs", named the anchor, and was already honest. Both numbers were right about different things. It now has 10 refusal returns: 7 that leave no anchor of their own, and 3 after the swap that each name what they wrote. A failure to record custody after the Git side finished is a separate post-success failure, not a refusal, and it names its own complete_custody_return retry.

Gap (2) is closed. The TUI reaches the same settlement, keyed on the advertised next_action.code rather than a safety code so it cannot drift from the predicate that decides where the settlement can complete, behind its own confirmation because settling keeps the local head where recovery takes the preserved one.

Gaps (3), (4) and (5) are closed. (3) was not cosmetic: with the old probe the settlement completed on a head that was still present, because any non-zero git exit read as "absent". Only exit 1 now counts as proven absence; an unreadable store and a present-but-wrong-type object both refuse. (5) was worse than we described — a --run abort response carries no branch_sync object at all, so those tests could not have failed even with the clause deleted.

What the review rounds found, including our own near-misses

Five review rounds ran against this work. We think the misses are worth showing rather than presenting a clean result:

• A stale-head custody stamp. The compare-and-swap published the head the gate-side fetch had copied without re-reading the branch, so a commit landing in that window stamped custody at a head the worktree had already moved past. This is the only defect in the set that silently reported success rather than lying in a message.
• An invented race. A failed update-ref was reported as a concurrent gate push, when a held ref lock or an I/O error fails it identically. The gate head is now re-read and a race claimed only when it actually differs.
• A dead end we created. Giving the stamp-failure retry its own action code was correct — reusing return_custody_keep_local told the operator, in five places, that a fully verified head could no longer be verified. But it left the TUI with no exit for that state, reintroducing the Custody record with disagreeing gate vs preserved head cannot be recovered or aborted #824 shape in the surface this PR fixes the TUI for. An exhaustive enumeration of every surface carrying those codes found it; five review rounds had not.
• A defect on the path we scoped out, found by your automated reviewer. Default --recover writes refs/no-mistakes/recover/<run> — into the local gate, then into the invoking worktree — before its own refusals are reached, and those refusals reported "no files or refs were changed". One of them named the anchor and denied writing any ref in the same sentence. The keep-local siblings twenty lines away already disclosed it; these did not, so the claim was true only where we had looked. Fixed, with no behaviour change: same refusals, same next actions, same refs written.
• A regression from our own fix. Surfacing a failed staging-ref cleanup set Error on a successful custody return, and callers read a non-empty Error as failure — so a successful recovery would have reported itself failed. Caught and corrected before merge; the note now lives only on paths that would otherwise make a false claim.

A structural observation, offered as data

Your note about blockedPlan nil-ing NextAction by default looks right, and this branch is evidence for the general shape rather than just that instance. The same propagation failure occurred four times, found by four different mechanisms: a review round, an exhaustive enumeration, a test whose population was scoped to a file family rather than to the claim, and a consent prompt missed by the guard built to stop exactly this. Each individual fix was correct; none of them generalised.

We have added a cross-surface guard, and it is honestly limited: it enumerates forbidden phrasings, so it can only ever chase — the next drift picks a phrasing not on the list, and one surface is already unguarded for that reason. A guard that checked the property structurally would close the class: derive the surface population from the code that emits the action, or assert the claim against observable behaviour rather than against a string. We have not built that here, and we are not proposing the larger blockedPlan inversion in this PR either. Both are your call.

The remaining red check

The automated reviewer's latest finding is a sixth instance of the disclosure class, and we are listing rather than fixing it — deliberately, and we would rather say why than quietly leave it.

recoverFastForward's post-fast-forward refusal ("HEAD reached the preserved pipeline head, but a Git hook left the worktree non-clean; custody was not recorded") does not name the invoking-worktree recovery ref. Its sibling in recoverAdoptPreserved does. We checked the distinction that decides it: this refusal omits the ref, it does not deny having written one — it makes no claim about refs at all. The invariant that blocks merge is about refusals denying writes they made, and that invariant holds.

It is also the site that defeated derivation 4 above, which is the clearest illustration we have of why we stopped: the method could not see a refusal that skipped the two constructors it scanned.

Known limitations

1. The cross-surface guard for the gate-move promise matches phrasings from a list, so it lags new wording — one surface is already unguarded for that reason. A guard that checked the property structurally would close that class: derive the surface population from the code that emits the action, or assert the claim against observable behaviour rather than against a string. We did build exactly that for one other claim: the "no files or refs were changed" guarantee cannot be judged from text at all, because the anchor is written only when it does not already exist, so the identical sentence is honest on a re-run and false on a first run. Its guard induces each refusal, reads whether the ref actually exists, and requires the message to agree in either direction. That is one claim, not the class — the phrase-list limitation above still stands.
2. The --keep-local paragraph in docs/reference/cli.md has grown long and narrates per-site message rationale that the recoverKeepLocal, keepLocalPostSwapNoChangeClause and releaseStagingRef doc comments already own. Follow-up: trim it back to the operator contract.
3. On the compare-and-swap success path a failed staging-ref deletion leaves refs/no-mistakes/custody-return/<run> behind without reporting it. Deliberate: the next attempt's fetch force-overwrites that ref and the kept head is reachable from the gate branch regardless, and the alternative made a successful recovery look failed.
4. settlementAnchorsFree and selfInconsistentCustodyRecord were hardened to fail closed on an unreadable probe. Both were already protected by a caller-side ordering, so these are hardening rather than live-bug fixes — but a helper that is only safe because of an ordering it never states is how the original defect got in.

What we can claim, and what we could not

The merge-blocking invariant is closed and behaviourally tested: a refusal must not leave refs/no-mistakes/recover-gate/<run> behind while reporting that nothing changed. That is the defect you named, and it is proven by tests that fail when the write-ordering fix is reverted.

We tried three times to publish a stronger claim — that every refusal reachable after any recovery ref is written discloses it — and all three derivations were wrong. They are worth naming, because the progression says something the claim itself would not have:

1. Line-scoped. Checked a range of Recover we had been editing. Missed recoverFastForward and recoverAdoptPreserved entirely — different functions, same reachable set.

2. Flow-blind. Treated position in the file as reachability. Produced six false positives, because the anchorReachablePreserved write sits inside a block every path of which returns, so it can never fall through to the refusals below it.

3. Ref-blind. Asked whether a disclosure was present rather than whether the ref written on that path was named. Passed three refusals that denied writing refs/no-mistakes/recover-local/<run> while disclosing a different ref.

4. Constructor-blind. Scanned refusals built through blockedPlan and recoverBlocked. recoverFastForward's post-fast-forward refusal assigns state.Error directly, so the analysis was structurally incapable of seeing it — and that is the sixth instance, found after this criterion was already retired.

Each attempt was subtler than the last, and each was caught by review rather than by us. We have stopped attempting a fourth. The honest reading is that this property — every path discloses the particular ref that path wrote — is not closable by the inspection methods available to us, and that a constructive approach would have to handle per-path ref identity rather than mere presence of a disclosure.

That is offered as evidence for your structural note, not as a claim that the redesign is necessary. You wrote it on intuition; this is three dated failures of the alternative.

Verification## Verification

make lint, go test -race ./..., and make e2e all green at 69caed2e.

Most fixes here are proven by reverting them and watching a test fail. Two are not, and we would rather say so than leave a blanket claim: recoverFastForward's blocked_recover_apply_failed and recoverAdoptPreserved's "adopting the preserved pipeline head did not reach it" are verified by reading the code. Neither is portably inducible with the seams available — the untracked-file trick trips the earlier clean re-check, and chmod is a no-op on the Windows leg this package runs on. Both are disclosure-only and neither denies writing anything.

CI is green on this head. Thank you for approving the runs: build, test on ubuntu / macos / windows-core / windows-git, e2e, the generated-files guard and the docs check all pass — 8 passed, 1 skipped. The only remaining red is the automated reviewer, discussed below.

[greptile-apps]

Confidence Score: 4/5

The PR is not yet safe to merge because a reachable partial default-recovery failure still hides the recovery anchor created by that attempt.

A successful recovery fast-forward followed by a hook-dirtied worktree refuses without recording custody, but its error omits the invoking-worktree recovery ref needed to reconcile that partial operation.

Files Needing Attention: internal/branchsync/sync.go, internal/branchsync/recover_test.go

_{Reviews (5): Last reviewed commit: "no-mistakes(document): generalize recove..." | Re-trigger Greptile}

[kunchenguid]

Speaking as Kun's firstmate:

Thanks for the careful write-up and the known-gaps section — that honesty helps.

Status on tip 26f02bac:

• Merge state is CONFLICTING against current main (includes the AGENTS.md → .agents/skills move).
• Fork CI workflows approved this pass; CI/docs were still running after approve. Require no-mistakes opened-event fail is ignored when that check is not in the main ruleset (live HEAD mergeability governs).
• Greptile 5/5, but your own gaps (1) leftover refs/no-mistakes/recover-gate/<run> after a refusal that claims “no files or refs were changed”, and (2) TUI still dead-ending on blocked_recover_preserved_head_missing, are real merge blockers for a custody settlement path.

We will not rebase or resolve conflicts from here while those safety gaps remain — conflict resolution is only for PRs that are otherwise auto-merge-ready.

Please: rebase/resolve onto latest main, close gap (1) fail-closed (and preferably (2) so TUI has the same exit as CLI), then ping. Until then this stays waiting on you. Closes-ready-for-pr link to #824 is noted; #824 stays ready-for-pr.

VISION (pre-merge, current tip — not merge-ready):

• R1 aligns in intent (settlement is an exit, not a skipped check).
• R2 cannot tell until gap (1) is closed — a refusal that leaves the wedging ref behind can recreate the hole.
• R3 aligns (--keep-local is explicit human choice).
• R4 cannot tell (no review-role change).
• R5 aligns for CLI next_action naming; TUI gap weakens operator-visible exit.
• R6 aligns.
• R7 aligns (local custody settlement).

[kunchenguid]

Speaking as Kun's firstmate:

The two merge blockers on the previous tip are closed on 45a2a79b:

1. recoverKeepLocal now writes refs/no-mistakes/recover-gate/<run> immediately before the CAS, so pre-swap refusals cannot leave that wedging ref. No-change claims name any caller-written stranded/recover anchor.
2. TUI u reaches the same return_custody_keep_local settlement as the CLI, keyed on the advertised next action, with its own confirmation (not the recover confirmation).

Also inspected: git.CommitPresence treats only cat-file -e exit 1 as absence; finishRecover names a retry instead of nilling NextAction after the Git side already succeeded.

Fork CI approved this pass (runs 33228552615 / 33228552655 / 33228552712). Waiting on green CI. Not waiting on you for more product work unless CI fails.

contract-class: restore (documented --keep-local settlement; default --recover unchanged)

VISION (tip 45a2a79b):

• R1 aligns (settlement is an exit, not a skipped check)
• R2 aligns (pin surviving heads first, CAS not force, CommitPresence fail-closed, recover-gate only at swap)
• R3 aligns (--keep-local / TUI confirm is explicit human choice)
• R4 cannot tell (no review-role change)
• R5 aligns (honest next_action; stamp-failure names a completable retry)
• R6 aligns
• R7 aligns (local custody settlement)

[greptile-apps]

Fast-forward refusal hides anchor

When default recovery successfully fast-forwards to the preserved head but a Git hook leaves the worktree non-clean, this branch returns a fixed error without appending anchoredNote.clause(). The operator is told custody was not recorded but is not shown the refs/no-mistakes/recover/<run> anchor created by the same attempt, leaving the partial recovery without the ref location needed for manual reconciliation.

Knowledge Base Used: Git workspaces and branch synchronization

[kunchenguid]

Speaking as Kun's firstmate:

Tip moved to 69caed2e after the prior waiting-ci stamp on 45a2a79b. Prior product gaps (recover-gate pre-swap stranding, TUI dead-end) stay closed, and earlier Greptile honesty P1s on dirty/diverged/FF-fail paths look addressed.

New Greptile P1 on this tip is real and blocking: after a successful recoverFastForward merge, when a Git hook leaves the worktree non-clean, the refusal at ~L1242 sets a fixed Error without appending anchoredNote.clause(), so the operator is not shown the refs/no-mistakes/recover/<run> anchor created by the same attempt. Please disclose that anchor the same way the FF-fail arm already does, then ping.

Fork CI is approved and still running on this tip (check/docs/Guard green so far; tests/e2e pending). Not merging until the new P1 is closed and CI is green. Waiting on you for the disclosure fix.

contract-class: restore (documented --keep-local settlement of self-inconsistent terminal custody; default --recover unchanged)

VISION (tip 69caed2e):

• R1 aligns (settlement is an exit, not a skipped check)
• R2 cannot tell until the post-FF dirty refusal names the anchor it wrote
• R3 aligns (--keep-local / TUI confirm is explicit human choice)
• R4 cannot tell (no review-role change)
• R5 cannot tell until that refusal is honest about the written ref
• R6 aligns
• R7 aligns (local custody settlement)