feat(pipeline-cli): review-head materialization verb — de-quadruplicate the gate skills' PR-head checkout (#3690)

[usirin]

What

Extract the quadruplicated "materialize the PR's current review head" logic the gate skills each hand-copied (#793 / #1807) into one tested pipeline-cli review-head verb, and adopt it across the four gate skills same-commit (#3254 same-commit adoption).

The verb (packages/pipeline-cli/src/tools/review-head/)

• resolve-head.ts — the IO-free resolution core: extract the latest head SHA + ref from the REST pull payload (fail-safe to unresolvable on a missing/closed PR, a null head, or a non-40-hex SHA), plus the deterministic detached-only checkout plan (per-run refs/pr/<pr>-<nonce> ref, detach: true, never a git checkout <headRef> branch switch — §RO). Unit-tested for the three AC cases: latest head, detached-vs-branch, missing-PR fail-safe.
• materialize.ts + git-io.ts — the gh REST + git IO shell (ReviewHead service, ReviewHeadLive layer):
  • pipeline-cli review-head resolve --pr N — the current head SHA/ref only (review-design reviews a preview URL, not a tree).
  • pipeline-cli review-head materialize --pr N [--worktree] — fetch pull/<pr>/head into a per-run ref (never the launched tree), assert the fetched ref IS the resolved head (§HEAD web: phoenix-relay-idiom — every page on @connection + idiomatic Relay #2, aborts on a moved head), and with --worktree add a throwaway detached worktree (review-head-<pr>-*, the shape worktree-sweep reclaims).
• Registered in the tool registry; shows up in pipeline-cli commands compact.

Adoption, same-commit (#3254)

• adoption-lint/manifest.ts declares the review-head materialize decision (fingerprint: the pull/<pr>/head: fetch refspec AND the per-run refs/pr/ ref — narrow enough that review-design's SHA-only .head.sha read isn't a false finding).
• All four gate skills + the gh-issue-intake-formats.md §HEAD contract migrated to cite the verb in the same commit — so no review-head grandfathered entry is needed (born drawn-down to zero). adoption-lint check over the full corpus is clean.

Out of scope

Verdict resolution (verdict read, story 1) and any per-gate review judgment (must-not-cross #3251) — untouched.

Guards

pnpm typecheck clean · biome check clean · adoption-lint check clean (8 decisions, full 62-file CI corpus) · 1580 pipeline-cli tests green · live end-to-end verified (resolve/materialize --worktree against a real PR, plus the missing-PR fail-safe).

This is a §CP change (packages/pipeline-cli + the gate skills) — it banks for human approval, not auto-merge.

Fixes #3690

🤖 Generated with Claude Code

[usirin]

review-code: advisory — blocking-set PR (manual merge)

Reviewed-head: @ 0ff7f3f

Verdict: PASS (advisory). Re-review after the conflict-resolving rebase (was 5eb9839b); the prior verdicts were dismissed by the force-push (ADR 0058), so this is a fresh, independently-verified gate bound to the new head.

§CP classification confirmed against the live CONTROL_PLANE_RE (re-resolved from origin/main): all 12 changed files match, so this PR takes the §CP-advisory form (no first-line @ <sha>; head bound in Reviewed-head: above) per ADR 0111/0151.

Silent-revert scrutiny (the priority ask) — CLEAN

This rebased over many landed siblings. Verified independently against origin/main at merge-base 11f0932d, not taken from the author's report.

adoption-lint/manifest.ts — 8 DECISIONS, no drops. All seven of main's entries are present and byte-identical in position order (verdict read, tracker apply-triage, tracker create-issue, tracker post-verdict, tracker graduate, epic-splice apply, intake-compose sub-issue), plus the new review-head materialize = 8.

The one deletion (+18/-1) is confirmed the PR's own header-docblock line being extended, not a dropped entry. The full diff is exactly two hunks: the docblock continuation line, and the appended review-head materialize entry. Nothing else.

Exemptions block is byte-identical to main — verified by a direct diff of the EXEMPTIONS-onward slice of both revisions. (The two trailing verdict read entries at head lines 188/195 are the unchanged exemptions, shifted +17 by the insertion above them — not stray decisions.)

registry.ts — the invariant holds, verified structurally rather than by counting. Every imported *Command symbol is also present in registeredTools and vice versa: 58 imported, 58 registered, and both comm -23 (imported-not-registered) and comm -13 (registered-not-imported) are empty. Against main: 57 registered → 58, with comm -23 main head empty (nothing dropped) and exactly one addition, reviewHeadCommand. The diff is +5/-0, purely additive.

Diff scope — git diff origin/main...HEAD is exactly the 12 intended files, 695 insertions / 88 deletions. No collateral.

claim is-mine (#3698) is correctly absent — that PR is not on main yet; expected, not a revert.

Acceptance criteria (#3690)

• A pipeline-cli verb materializes the PR's current review head, replacing the four inline copies. pipeline-cli review-head resolve|materialize, registered and reachable. All four gate skills now cite it; a grep of the four SKILL.md files at head finds zero residual hand-rolled pull/<pr>/head: fetch refspecs.
• The four gate skills cite the verb; adoption-lint passes. Ran adoption-lint at the exact CI scope (adoption-lint.yml find + drive-issue.js): clean — no un-cited re-derivations … every declared exemption is valid, exit 0. No pre-existing review-head exemptions existed to draw down (exemptions block unchanged), so that clause is vacuously satisfied.
• The head-resolution core is unit-tested (latest head, detached-vs-branch, missing-PR fail-safe). All three named categories are covered explicitly. 11/11 pass.

The §RO / §HEAD invariants

• Detached-only, by construction — not by convention. MaterializePlan.detach is typed as the literal true, and the checkout target is the per-run ref, never head.headRef. The IO shell runs git worktree add --detach <dir> <prRef>. There is no git checkout / git switch / gh pr checkout anywhere in the tool. A cross-fork head is handled by the same pull/<pr>/head refspec, so there is never a branch to switch to — the "detached vs branch" decision is made once in the pure core and is unrepresentable otherwise.
• Per-run ref is nonce-uniqued. refs/pr/<pr>-<randomUUID()>, closing the Parallel reviewers share a scratchpad path — a sibling can overwrite it, risking a wrong-head verdict #1807 same-PR concurrent-review collision. Unit-asserted that two plans for one PR yield distinct refs.
• Fail-safe unresolvable. null head, null sha, and non-40-hex sha all resolve to unresolvable with a reason; the CLI lowers it to stderr + non-zero exit. A 404 from gh api pulls/<pr> is separately caught at the IO edge as a clean refusal rather than a stack trace, covering the missing/closed-PR case end to end.
• The fetched ref is asserted to BE the head before use. materialize re-rev-parses the per-run ref after the fetch and fails HeadMismatchError on any divergence, aborting rather than binding a verdict to a tree it did not fetch (§HEAD web: phoenix-relay-idiom — every page on @connection + idiomatic Relay #2).
• No machine-local/absolute/home paths. Worktree dirs come from mkdtempSync(join(tmpdir(), …)). A scan of the added lines only across all 12 files finds zero /Users/, ~/, /home/, or vault paths. (The /Users/ hits in the full-file scan are pre-existing pattern literals inside leak-guard regexes — the documented canonical false positive.)

Adoption-lint teeth — I verified the reasoning myself, and I agree with the framing

I did not take the author's account on trust; I ran both falsification tests against the real CI scope.

Test A — strip the citation only. Replaced every pipeline-cli review-head occurrence in review-skill/SKILL.md (0 remaining) and re-ran the lint: clean, exit 0. The author's claim is correct — stripping the citation alone does not red.

Test B — re-introduce an uncited hand-copy. Appended a PR_REF="refs/pr/$PR-$(uuidgen)" + git fetch origin "pull/${PR}/head:$PR_REF" block and stripped the citation. The lint failed, exit 1, naming exactly this decision:

adoption-lint: FAIL — 1 inline re-derivation(s) of a tool-owned decision that never cite the owning verb (#3254):
  claude-plugins/kampus-pipeline/skills/review-skill/SKILL.md: re-derives the §HEAD PR-head-checkout that
  `pipeline-cli review-head materialize` owns … (#3690 / #793 / #1807 / #3254)

(Both mutations were made in a throwaway detached worktree and reverted; the PR was not touched.)

I agree the forward-guard framing is correct. adoption-lint's contract is precisely "a re-derivation present in the text that fails to cite the owning verb" — the signature must be present for the rule to fire. Same-commit adoption removes the re-derivation, so the signature is gone and the rule has nothing to match. That is the entry working as designed (#3254), not a hole: the entry's job is to catch the re-introduction, which Test B proves it does, naming the right decision. Calling the post-adoption clean "structurally vacuous for this decision" is an honest and accurate description, and proving teeth by injection is the right method.

Two things worth recording rather than blocking on:

• The two-tell co-occurrence signature (pull/${PR}/head: and refs/pr/) is well-chosen: the baseline stays clean even though review-code line 296 mentions refs/pr/$PR-<uuid> in descriptive prose and review-design touches head SHAs without any checkout. No false positives across the whole corpus.
• adoption-lint does not (and structurally cannot) catch a future edit that deletes the citation and leaves no mechanism behind — i.e. a gate that simply stops materializing the head. That is out of this lint's scope by design; §HEAD prose plus each gate's own FATAL: review-head materialize did not yield a head worktree guard cover it.

Gates run at this head

gate	result
tsgo typecheck (@kampus/pipeline-cli)	clean
vitest run src/tools/review-head	11/11 pass
biome check (7 changed files)	clean, no fixes
adoption-lint check (exact CI scope)	clean, exit 0
adoption-lint teeth (injection)	reds correctly

Non-blocking nits (not defects; no action required to land)

1. planMaterialization's worktreeDir is dead in the live path. materialize.ts calls it without worktreeDir and then computes the real dir itself via mkdtempSync, so the plan field is only ever exercised by the unit test. Harmless, but the plan no longer fully describes the materialization it names.
2. Guard asymmetry in review-doc. review-code and review-skill both follow the verb with a [ -n "${REVIEW_WT:-}" ] && … || FATAL guard, but review-doc's ref-only path uses a bare eval "$(… | jq …)" with no emptiness check — on a verb failure PR_REF is simply unset and later git show "$PR_REF:<path>" misbehaves quietly rather than aborting loudly. Worth a follow-up for symmetry.

Verdict

PASS — advisory. No silent reverts, the §RO/§HEAD invariants are enforced by construction rather than convention, the ACs are met, and the adoption-lint teeth claim holds up under my own falsification. Merge is manual per §CP; this comment is advisory only.

[usirin]

review-skill: advisory — blocking-set PR (manual merge)

Reviewed-head: @ 0ff7f3f

Verdict: PASS (advisory). Fresh re-review after the conflict-resolving rebase (was 5eb9839b) — the force-push dismissed the prior verdicts (ADR 0058). §CP-advisory form (no first-line @ <sha>; head bound in Reviewed-head: above) per ADR 0111/0151, confirmed against the live CONTROL_PLANE_RE re-resolved from origin/main: all five behavioral artifacts in this diff are §CP.

Behavioral surface reviewed at head: review-code/SKILL.md, review-doc/SKILL.md, review-skill/SKILL.md, review-design/SKILL.md, and the gh-issue-intake-formats.md §HEAD contract.

The de-quadruplication is complete and correctly directed

The load-bearing question for a skill gate here is whether the behavior still fully specifies itself after the mechanism moved into a verb — a skill that cites a tool but drops the invariant the tool enforces is a regression even when the tool is right.

Zero residual re-derivation. A grep of all four gate SKILL.md files at head finds no hand-rolled pull/<pr>/head: fetch refspec anywhere. Every remaining refs/pr/ mention is descriptive prose about what the verb does, not an instruction to re-derive it. All four cite pipeline-cli review-head with the issue trail (#3690 / #793 / #1807).

Each gate takes the right mode, and the SKILL text explains why rather than just what:

• review-code / review-skill → materialize --pr "$PR" --worktree (full detached tree for typecheck/lint).
• review-doc → materialize --pr "$PR" ref-only, reading via git show "$PR_REF:<path>" — correct for a diff-only gate.
• review-design → the lighter resolve --pr "$PR", head SHA only, correctly justified inline as "this gate reviews the preview URL, not a checked-out tree."

The invariants survive the extraction — they were not thinned into a bare tool call. Each migrated block still states, at the call site, that the verb never runs gh pr checkout / git checkout / git switch, why (the harness resets cwd to the shared PRIMARY between Bash calls → #2270/#1103 detaches the human's main), that §RO forbids it outright, that pull/<pr>/head covers cross-fork, and that the ref is nonce-uniqued against the #1807 collision. The #1807 mktemp-handle discipline (. "$WT_FILE" re-source, never re-derive from a shared PR-namespaced leaf) is preserved verbatim in all three tree-materializing gates, as is review-code's ADR-0067 full-checkout note and the instruction-denylist removal + absence-assert. This is a genuine citation, not a summary that quietly sheds the reasoning.

review-code and review-skill add a fail-loud guard after the verb — [ -n "${REVIEW_WT:-}" ] && [ -n "${PR_REF:-}" ] && [ -n "${HEAD_SHA:-}" ] || FATAL … never review the base tree; §HEAD — so a verb failure aborts instead of silently reviewing the base. Correct direction: on failure the verb writes nothing to stdout, jq emits nothing, the file is empty, and the guard fires.

The §HEAD contract rewrite (gh-issue-intake-formats.md)

Step 1 now delegates resolution + fetch + the fetched-ref-IS-the-head assertion to the verb, and step 2 is honestly reduced to **(owned by the verb, step 1)** rather than deleted — the numbering is preserved so the downstream §HEAD #2 / §HEAD #4 cross-references throughout the gate skills still resolve. That is the right call; renumbering would have silently broken live citations. The §RO prohibition ("never gh pr checkout / git checkout / git switch … the verb refuses this outright") and the iso_preflight-before-the-verb ordering are both stated explicitly. Both worked examples (ref-only and --worktree) are runnable as written.

The one boundary I checked closely: the verb is scoped as mechanism, not isolation gate — command.ts and materialize.ts both state the caller runs the §RO-iso primary-checkout preflight before materialize, and §HEAD step 1 repeats it. The gate keeps ownership of isolation; the tool owns only the deterministic checkout. That separation is stated consistently in all three places with no drift.

Silent-revert scrutiny (skill surface) — CLEAN

Verified against origin/main at merge-base 11f0932d, independently of the author's report: git diff origin/main...HEAD touches exactly the 12 intended files. No sibling skill under claude-plugins/ was reverted, reformatted, or otherwise collaterally modified by the rebase. Within the five behavioral files, every hunk is the §HEAD migration — no unrelated prose reverted to a pre-#3695/#3696/#3697/#3702/#3703/#3704 state.

Skill hygiene

• No machine-local, absolute, home, vault, or sibling-repo paths in any added line across all 12 files (added-lines-only scan). The /Users/ occurrences in the full-file scan are pre-existing leak-guard pattern literals — the documented canonical false positive, not real paths.
• All in-repo references are repo-relative markdown links that resolve (../gh-issue-intake-formats.md, packages/pipeline-cli/src/tools/review-head/).
• Issue/ADR citations are specific and correct: review-head materialization verb — de-quadruplicate the gate skills' PR-head checkout (#793 / #1807) #3690, review-code/review-doc in isolation:worktree review base code, not PR head #793, Parallel reviewers share a scratchpad path — a sibling can overwrite it, risking a wrong-head verdict #1807, Investigation: the adoption seam — what forces a skill to consume the CLI verb that owns its step? #3254, Review/ship agents' bare git checkout detaches the human's shared primary checkout (cwd resets out of worktree) #2270, Worktree review/ship agent bare git checkout detaches the shared primary HEAD, silently breaking the puller #1103, Concurrent same-session reviewers clobber each other's scratchpad path/ref state #1465, ADR 0052, ADR 0058, ADR 0067.
• No stale instruction left behind: nothing tells a gate to fetch the head itself anymore.
• adoption-lint at the exact CI scope over the whole crew corpus: clean, exit 0, every declared exemption still valid.

Adoption-lint teeth — independently verified, framing agreed

I ran both falsification tests myself (throwaway detached worktree, reverted after; the PR was not touched):

• Strip the citation alone → clean, exit 0. The author's "structurally vacuous for this decision" claim is accurate — with the hand-copies removed same-commit, there is no signature left for the rule to match.
• Re-introduce an uncited hand-copy (both tells: the pull/${PR}/head: refspec and a refs/pr/ ref) → FAIL, exit 1, naming this decision by verb and issue trail.

I agree the forward-guard framing is correct. The rule's contract is "an inline re-derivation that does not cite the owning verb"; same-commit adoption legitimately removes the antecedent. The entry's value is prospective — it fails the re-introduction, which is exactly what Test B demonstrates. The two-tell co-occurrence signature is also well-chosen for this surface: it keeps review-code's descriptive refs/pr/ prose and review-design's SHA-only resolution out of the finding set, and the whole corpus baseline is clean with no false positives.

Non-blocking nit

review-doc's ref-only path uses a bare eval "$(pipeline-cli review-head materialize … | jq …)" with no emptiness guard, unlike the [ -n … ] || FATAL that review-code and review-skill both got. On a verb failure PR_REF is simply unset and a later git show "$PR_REF:<path>" degrades quietly instead of aborting loudly — the same false-PASS-shaped hazard §HEAD exists to close, just narrower here since the gate reads per-file. Worth a follow-up for symmetry; not a blocker, since the verb's own failure modes are all fail-closed with non-zero exits.

Verdict

PASS — advisory. The behavioral contract is fully preserved through the extraction, the §RO/§HEAD invariants are restated at every call site rather than thinned to a bare tool reference, §HEAD's numbering is kept so downstream citations still resolve, and the adoption-lint teeth claim holds under my own falsification. Merge is manual per §CP; this comment is advisory only.

[usirin]

§CP bank — reviewed-ready, awaiting control-plane approval.

Both gates PASS (advisory) at head 5eb9839b:

• review-code — #issuecomment-5017827929
• review-skill — #issuecomment-5017828974

CI green. Per ADR 0135 this control-plane PR needs a non-author @kamp-us/control-plane approval at the current head before the approval-aware shipper may enqueue. Assigned to @cansirin + @notusirin.

[usirin]

§CP re-bank — reviewed-ready at the NEW head, awaiting fresh control-plane approval.

Rebased to resolve conflicts with landed siblings (#3696/#3697/#3702/#3703/#3704); head moved to 0ff7f3f9, dismissing any prior approval + verdicts (ADR 0058).

Fresh gates both PASS (advisory) at 0ff7f3f9:

• review-code — #issuecomment-5017827929
• review-skill — #issuecomment-5017828974

Silent-revert scrutiny clean, verified against origin/main (merge-base 11f0932d) rather than from the coder report: registry.ts 58 imported / 58 registered with comm empty in BOTH directions (nothing dropped from main, one addition — reviewHeadCommand); manifest.ts retains all 7 landed decisions plus review-head materialize = 8, exemptions block byte-identical to main; diff scope exactly the 12 intended files.

Adoption-lint verified as a genuine forward guard: stripping the citation alone is clean (expected — same-commit adoption removed the antecedent, per #3254), while injecting a re-introduced uncited hand-copy FAILS exit 1 naming the decision.

Per ADR 0135 this needs a fresh non-author @kamp-us/control-plane approval at 0ff7f3f9 before the shipper may enqueue. @cansirin @notusirin