fix([issue-134]): assert review-fix commits are pushed before opening a PR or merging

[atomantic]

Closes #134

Summary

Every reviewer loop pushes its own fix commits as its final step, but nothing downstream ever verified it happened. An orchestrator that improvised a loop body instead of following it left the fixes committed locally: gh pr create opened a PR containing only the pre-review commit, every reviewer still reported clean, CI still passed (it was testing the pushed tree, green because it predated the fixes), and the merge gate had nothing to object to. The failure was completely silent — the incident in #134 lost four review findings, including a restored data-loss guard, and needed git reflog to recover them.

Primary fix — lib/multi-reviewer-loop.md. Series dispatch gains a step 5 that asserts the pass's commits reached the remote, pushes them with the loop files' own one-retry, and records a new push-failed status if the push still fails. Parallel dispatch reuses the same block after its single union apply. Landing it in the wrapper fixes it once for all seven commands that drive it (pr, pr-better, review, release, better, better-swift, depfree).

push-failed is inconclusive and never merge-eligible, and — unlike the other inconclusive statuses — is never excused by ~opt. A non-blocking reviewer's findings are still real fixes; ~opt opts out of blocking on a missing verdict, not out of noticing that the merged tree isn't the reviewed tree. It is wired into the stop-mode table, the stop-mode prose, the aggregate inconclusive rule, the clean rule, the optional-exclusion note, the parallel-mode aggregate, and /do:release's restatement of the same rule.

Backstops — commands/do/pr.md. At the two moments unpushed work becomes user-visible damage: Open the PR pushes anything outstanding first and refuses to create the PR if that push fails, and the merge gate refuses to merge while the branch is ahead of its remote, naming the unpushed SHAs. The merge-gate snippet exits non-zero rather than printing a result someone has to interpret — the failure mode here is precisely a step being skimmed past.

Edge cases handled

• No upstream. /do:review and /do:better can run on a never-pushed branch; the check no-ops rather than erroring. Detached HEAD and a missing origin take the same path.
• Upstream tracking a local branch (branch.<n>.remote=.). On such a branch @{u}..HEAD is non-empty by construction, so a naive check would fire on a perfectly healthy branch and record a false push-failed that blocks the merge. Treated as "no remote to assert against".
• Push target derivation. Taken from branch.<n>.remote / .merge, not by splitting the abbrev-ref — a remote name may itself contain a slash, and a local upstream abbreviates with no slash at all. Since .merge is already fully qualified the destination is HEAD:$PUSH_BRANCH, never HEAD:refs/heads/….
• Never a bare git push (fans out under push.default=matching) and never git push origin HEAD (resolves <dst> to the local branch name, so on a differently-named upstream it creates a spurious remote branch and leaves the PR head stale — Assert review-fix commits are pushed before opening a PR or merging #134's own failure).
• Scoped to the pass. A pass that committed nothing skips the check, so it never publishes deliberately-unpushed local work.
• Hard errors stay dirty. A cli-error/broken-build/test-failed/rejected pass keeps its status instead of being rewritten to push-failed, which would downgrade the aggregate to inconclusive and slip past /do:pr's abort-before-creating-the-PR gate.
• --reviewer-applies is unaffected — that flag changes who edits, not who pushes.

Test plan

• npm test — 185 pass, 0 fail (3 new contract tests in test/review-loop-contract.test.js).
• New tests pin: the assertion and its no-upstream skip; push-failed reaching every aggregate rule that must consume it; the ~opt exemption being scoped rather than unconditional; the config-based target derivation and local-upstream skip; every prescribed push using the derived form (scanned from raw text, since the in-block occurrences carry no backticks); and both do:pr guards.
• Both git push failure modes were reproduced in scratch repos before fixing, not reasoned about: git push origin HEAD on a branch tracking a differently-named upstream created a spurious refs/heads/local-name and left @{u}..HEAD non-empty; a local-tracking upstream abbreviated to a bare main, mangling the split into a bogus remote.
• The prescribed push block was dogfooded — the last three commits on this branch were pushed with exactly the command the spec now prescribes, including surviving a slash in the branch name (refs/heads/next/issue-134).

Review status

--review-with claude,ollama~opt (saved default), series mode.

Pass	Agent	Optional	Iterations	Status	Notes
1	claude	no	3/3	guardrail	13 findings across 3 rounds, all applied
2	ollama	yes	1/3	clean	qwen3.6:35b; 8 findings, 5 applied, 3 rejected

Aggregate: inconclusive — the claude pass hit its built-in 3-iteration cap while still landing substantive findings, which is guardrail rather than clean by the loop's own rules ("nobody chose that ceiling"). Left unmerged deliberately: a further round may still have something to say.

Round 2 caught a CRITICAL that round 1's own fix introduced — git push origin HEAD, which silently reintroduced #134's exact failure inside the guard against it. Round 1 caught a self-contradiction where the inconclusive rule's trailing "~opt passes are ignored here" negated the push-failed carve-out three clauses earlier, which would have let an ~opt reviewer's stranded fixes merge.