fix(skills): make collapse-pr gate 3e row 1 opt-in and stop it publishing local work

[s-annam]

Summary

Round-4 review of #851 found three blocking defects, all in /collapse-pr gate 3e row 1 — the ~230 lines added late to #851 that no reviewer had seen when it merged. All three share one root cause, and it is the same one rounds 1 and 2 kept hitting: a safety property stated as prose while the executable block carried no condition at all.

Row 1's table entry reads "uncommitted changes this run authored". That is a precondition, but nothing in the code enforced it, and the iterate block runs the rows for every matched worktree. On a checkout that was clean but held a local-only commit, row 1 therefore ran anyway: the dirty check passed, git commit failed on a clean tree and was silently discarded (no set -e), WT_PRE_FIX_SHA became the user's commit, and it was cherry-picked and pushed into the PR.

That reproduces on the skill's own flagship example. In the #842 case the main checkout was clean and held exactly one local-only commit, 6ba92e9 — under the merged code that commit is published into PR #842, while the report describes the checkout as row 3 "preserved + reset" and never mentions the push.

Refs #849
Refs #851

What changed

Blocking

1. Row 1 is opt-in in code. ROW1=run|skip now gates the commit, ancestry and publish blocks. A worktree that is not --authored-worktree refuses on dirt (row 2) and otherwise skips to rows 3/4; the authored worktree skips when clean, which is the documented happy path where pr-review step 4 already committed.
2. The cherry-pick branch no longer pushes. It pushed from the throwaway worktree, which shares .git/hooks and has no node_modules, so the managed pre-push hook failed on every run — and Step 0b's bypass licence does not reach it, because cherry-picked content is new relative to origin and no tree-identity assertion covers it. The fix now stays in $WORKDIR and Step 5's collapse publishes it under the lease gate 3d already pinned. One push instead of two, and the unverified-tree question never arises.
3. That branch's backup ref is real code, not a placeholder comment. # ... row 3's backup runs here ... was the only thing between row 3's reset --hard and unrecoverable loss, and the prose then told row 3 not to mint one. Row 3 now short-circuits on $BACKUP.

Secondary — all the same class, a status that was computed and then discarded:

• Step 5 and row 1 capture the push status with || PUSH_FAILED=1 and chain the rollback.
• The cherry-pick is chained; an unchained conflict fell through to a push of $WORKDIR's unchanged head, dropping the fix while reporting success.
• Row 2's refusal runs the teardown in code instead of a comment saying it should — it fires in worktree mode, the one mode where the teardown is not a no-op.
• PR discovery distinguishes "no PR exists" from "the call failed". An empty PR_NUM selects inplace, so an auth expiry or rate limit silently redirected the rewrite at the user's own checkout.
• pr-review's pre-existing-dirty subtraction parses porcelain properly. cut -c4- mangles a rename's bare second field, so it under-subtracted and left the user's in-progress rename whitelisted — the opposite of the direction the surrounding prose claimed was safe.
• pr-review's partial-failure rationale now matches the code and names the inherited-state case it does not cover.

Nits: temp-file cleanup in Step 5b; a distinct exit 2 for the no-net-change no-op so pr-review stops reading it as a refused gate; a TOCTOU-aware message on backup-ref collision; the -z quoting claim dropped from the cut -c4- rationale; and collapse-pr's "single source of truth" claim narrowed to the skills, since CONTRIBUTING-PROCESS.md legitimately restates the rationale for humans.

Review focus

• .claude/skills/collapse-pr/SKILL.md gate 3e row 1 — does ROW1=skip reach every block that writes? A missed one is the same defect again.
• The cherry-pick branch — with no push there, is REMOTE_SHA still the right lease at Step 5, and does the scoped re-pin leave BASE_SHA correct?
• Step 5's || PUSH_FAILED=1 — is there any path that reaches Step 6 with a failed push still unreported?

Test plan

• npm run verify green — ran via the managed pre-push hook on this branch, not bypassed
• fallow clean on the 3 changed files
• All four in-scope skill frontmatters parse as strict YAML
• No git -C "$WORKDIR" push remains anywhere in gate 3e
• The # ... row 3's backup runs here ... placeholder is gone
• Arrives as exactly one commit

How this was found

/collapse-pr was dogfooded against PR #842 before this review — the run that produced a8e0909 on main. Two commits collapsed to one, tree 56a742c98fb9d6afe1370ab5f9432c82abc6c5d3 byte-identical on both sides, zero diff, lease pinned to 72358ab and accepted. The gates behaved as documented end to end.

That run also turned up a defect the review did not: reading the push's status through a pipe. $PIPESTATUS is a bash array, and under zsh it expands empty — so the rollback fired on a push that had succeeded. The skill now says not to pipe the push at all, and why.

[cloudflare-workers-and-pages]

Deploying offlinecv with    Cloudflare Pages

Latest commit:	2160aad
Status:	✅  Deploy successful!
Preview URL:	https://46d8b76a.offlinecv.pages.dev
Branch Preview URL:	https://gh-852.offlinecv.pages.dev

View logs