refactor(pipeline): adopt verdict read across ship-it / write-code / heal-ci — delete the 3 inline resolvers

[usirin]

Adopts the already-shipped pipeline-cli verdict read verb across the three skills that each hand-copied the ADR-0058 SHA-bound verdict-marker resolver, deleting all three inline copies and citing the verb in this same landing commit (the #3254 same-commit-adoption rule). This is adoption of an existing tested core, not a new core (#2824).

What changed

surface	inline resolver deleted	now
skills/ship-it	4 per-namespace marker jq blocks (code/doc/skill/design) + their is_current application	verdict read --gate <g> --expect PASS|FAIL per present namespace
skills/write-code	Step-1 per-gate resolution, Step-R1 3 per-namespace marker jq, Step-R2 latest-wins body pick	verdict read per gate; R2 reads the body via the comment id the verb returns
skills/heal-ci	CODE/DOC marker jq + staleness test	verdict read --gate <g> --expect FAIL
adoption-lint/manifest.ts	the 2 verdict read grandfathered exemptions	removed — the pile is drawn to zero, only the drive-issue.js mirror survives

The verb owns the ADR-0055 write+ author-gate, the latest-wins pick, and the ADR-0058 SHA-staleness refusal, folded into one exit code. Its existing unit tests remain the contract — no verdict-resolution behavior is changed.

Behavior that is MORE than a single (PR, gate) resolution — preserved, not folded in

This was the reason #3265 deferred the migration (the non-drop-in rewrite). Each residue stays inline, and is now explicitly justified in-file as something the verb does not do:

• write-code's multi-PR pre-pick scan and the N=3 FAIL-round count. verdict read resolves the latest verdict; it does not count rounds. The round count (author-gated, clustered by >120s gap) and the cap exclusion are untouched.
• heal-ci's N=3 cap count and its native CHANGES_REQUESTED fold.
• ship-it's native APPROVED/CHANGES_REQUESTED review fold — the verb reads marker comments; a native review is a different record type (and needs no ACL gate, GitHub author-attributes it).
• ship-it's §CP advisory resolution. An advisory is SHA-less in its first line by design (ADR 0111), so the verb resolves that namespace none and cannot author-gate it — the advisory's own author-gated pick stays ship-it's, which is why the $authorized ACL set is still built there.
• is_current survives only for the two signals the verb does not resolve: a native review's commit_id and a §CP advisory's body Reviewed-head SHA.

Adoption is genuine, not a citation false-positive

adoption-lint's citation check is a substring match, so a stray mention of the verb can clear a file that still re-derives. Verified this is not that: each skill has real $VERDICT read --pr call sites (ship-it 8, write-code 5, heal-ci 2). The three files still match the decision's signature only via legitimate residue — the native-review sort_by(...) | last, the ADR-0115 claim-marker resolver (a different decision), the run-evidence run lookup, and the round-count ACL loop — none of which is a verdict-marker resolver.

Note ship-it was never grandfathered: it was already clearing the lint via its pre-existing pipeline-cli verdict post citation while still re-deriving read. That false-clear is now a real adoption.

Guards

adoption-lint clean (62 files / 6 decisions, zero exemption findings) · pnpm lint 0 · pnpm typecheck 0 (33/33) · pipeline-cli tests 1606 passed (113 files) · gh-phoenix lint-skills clean · leak-guard sweep clean.

Out of scope

Changing verdict read's behavior, and the ship-it / write-code lifecycle graduations (#3691 / #3692).

§CP change (adoption-lint manifest + 3 gate skills) — banks for human control-plane approval, not auto-merge.

Fixes #3686

[usirin]

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

Reviewed-head: @ aa584d7

§CP PR (packages/pipeline-cli/**, skills/ship-it/** and skills/write-code/** match the live CONTROL_PLANE_RE, re-resolved from origin/main), so this gate is advisory and authorizes nothing on its first line (ADR 0111/0151). Code class scope: packages/pipeline-cli/src/tools/adoption-lint/manifest.ts (the only non-skill file in the diff, +7/-19).

Re-review at the new head. The prior review-skill FAIL (@ 5ab6a4d) was in the skill class; the code half was and remains clean. Every body checkbox is [PASS].

Acceptance criteria

• [PASS] AC2 (code half) — adoption-lint passes with the verdict read grandfathered exemptions drawn down, no growth. The manifest diff removes both grandfathered entries (skills/heal-ci/SKILL.md, skills/write-code/SKILL.md), leaving EXEMPTIONS with exactly one member: the mirror for .claude/workflows/drive-issue.js. That mirror is legitimate and unchanged — a runtime .js orchestrator surface that cannot import the TS verb core, so it must inline what a skill would cite. The verdict read grandfather pile is at zero.
• [PASS] The docblock is updated in the same commit and is accurate. It now records that Adopt verdict read across ship-it / write-code / heal-ci — delete the 3 divergent inline copies #3686 migrated all three consumers to cite the verb same-commit, enumerates the residue each scan legitimately keeps (write-code's N=3 FAIL-round count, the multi-PR pre-pick scan, ship-it's native-review fold + §CP advisory), and states the pile is drawn to zero and re-growth is forbidden. Each of those four residue claims is true at head — I verified them individually in the skill-class review. No stale #3619/#3265 deferral text survives.
• [PASS] AC3 (code half) — no behavioral change to verdict read. packages/pipeline-cli/src/tools/verdict/ is untouched by this PR. The verb's core (verdict-match.ts, command.ts, github.ts) and its unit tests are unmodified, so "the shared verb's existing unit tests remain the contract" holds literally. The change is a data-only edit to a manifest constant plus a comment.

Verification — the lint result is earned, not vacuous

The single risk in a diff like this is a manifest edit that clears the lint by removing its teeth. Falsified directly:

• Full-corpus run at head using the workflow's own selection (find claude-plugins/kampus-pipeline -type f \( -name '*.md' -o -name '*.sh' \) plus drive-issue.js — 62 files): adoption-lint: clean — no un-cited re-derivations of a tool-owned decision, and every declared exemption is valid.
• Falsification: stripping every verdict read citation from write-code/SKILL.md in a disposable tree (14 → 0) turned the lint red with exactly the expected finding — re-derives the ADR-0058 SHA-bound verdict-marker resolution that pipeline-cli verdict read owns, instead of citing the verb (#3254 / #2102). Restored; tree verified clean.
• The exemption validator still has teeth too: the same falsification run surfaced [mirror] … declared mirror exemption names a file not in scope — stale, remove it when drive-issue.js was excluded from the corpus, confirming the "every declared exemption is valid" half is live rather than a no-op after the two removals.

So the removals are a genuine draw-down: the lint would fire if the citations regressed, and it does not fire now because the consumers really do cite the verb.

Type/API design

• [PASS] The entries removed are plain Exemption records in a ReadonlyArray<Exemption>; the array is non-empty after the edit, so the tool's fail-closed "empty decision set / empty corpus ⇒ exit 3" guard (ADR 0092) is unaffected.
• [PASS] No signature, schema, or exported-type change — nothing downstream can be broken by this diff.
• [PASS] lint / format / typecheck and packages unit tests both green at head.

Checks at head

• [PASS] lint crew corpus for inline re-derivations of tool-owned decisions — success; reproduced locally on the full corpus and falsified.
• [PASS] lint / format / typecheck, packages unit tests, unit + client tests — success.
• [PASS] check every registered tool has a one-line description, check every packages/* member has a README.md, check every package.json dep is on catalog:/workspace: — success.
• [PASS] scan changed files for leaks, scan PR commits for secrets, sweep pipeline-crew for personal-data leaks — success. No machine-local, absolute, home, or sibling-repo path in the diff.
• [INFRA] integration tests — failure at head, unrelated to this diff. Job log root cause is Cloudflare D1 rate-limiting (429 … code 971, "Please wait and consider throttling your request speed") in content-removal-substrate.test.ts, with leaked-stage teardown warnings (Orphan integration-stage sweep / reliable teardown on partial deploy #690/apps/web pr-<n> preview stacks leak: cleanup job reports success but workers survive on Cloudflare #813). This diff touches one manifest constant and no worker/D1/runtime surface; the job passes on recent main. Needs a re-run before merge, not a repair.
• [PENDING] e2e (reads + authed + flows, blocking) — in progress at review time; the merge actor must confirm it green.

Fan-out (silent-failure · type-design · test-gap)

• test-gap: the draw-down's durability rests on adoption-lint continuing to fire for these three files. That is now enforced structurally — the decision stays declared in DECISIONS while the exemptions are gone, so any future re-derivation reds the build without needing a new test. Falsified above; no gap.
• silent-failure: nothing on the code side. The one residual on this axis lives in ship-it's fold and is recorded in the review-skill advisory (pre-existing, non-blocking).
• No out-of-scope findings worth a report from the code class.

Note for the merge actor

Every body checkbox is [PASS], so this advisory is enqueue-eligible on the code namespace (§6.6/ADR 0151). Two gating conditions remain outside this verdict: re-run integration tests (D1 429 flake) and confirm e2e is green.

[usirin]

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

Reviewed-head: @ aa584d7

§CP PR (skills/ship-it/**, skills/write-code/** and packages/pipeline-cli/** match the live CONTROL_PLANE_RE, re-resolved from origin/main), so this gate is advisory and authorizes nothing on its first line (ADR 0111/0151). Skill class scope: skills/ship-it/SKILL.md, skills/write-code/SKILL.md, skills/heal-ci/SKILL.md.

Re-review of the prior FAIL (comment 5017948256, @ 5ab6a4d). The AC3 defect is genuinely repaired. Every body checkbox is [PASS].

The prior FAIL — newest-wins restoration

The ruling was: the prose (ship-it lines 783/952) is the source of truth, and the refactor's FAIL-precedence fold must be reverted to newest-wins. Verified restored, and correctly.

• [PASS] 1. at: .submitted_at is back, and the marker created_at is genuinely recoverable for the right comment. REVIEW carries {state, sha: .commit_id, at: .submitted_at} again in all three skills. The recovery path is sound, and I checked the mechanism rather than the claim:

  • read emits Console.log(JSON.stringify({...result.outcome, headSha, gate})) before the satisfied/fail branch (verdict/command.ts), so the outcome JSON — including commentId — prints on both exit paths. The capture is valid even when the verb exits non-zero.
  • The commentId is genuinely the right comment. resolveVerdict filters candidates by author-ACL and polarity-agnostic polarityRe (PASS or FAIL), sorts by (createdAt, id), and takes latest. --expect enters only through isReviewed (the exit code) and outcomeReason. So commentId is the latest marker in the namespace regardless of polarity — exactly the operand newest-wins needs. This is why ship-it can take it from the --expect PASS call and write-code/heal-ci from the --expect FAIL call and both get the same comment.
  • The endpoint is right: listComments reads repos/{repo}/issues/{pr}/comments, so the id is an issue-comment id and gh api repos/$REPO/issues/comments/$ID --jq .created_at addresses that same comment.
  • The extra round-trip to recover a timestamp the verb already had internally is a wart, but the correct scoped choice here — the issue explicitly puts "changing verdict read's behavior" out of scope, so adding createdAt to the outcome record belongs to a follow-up, not this PR.

• [PASS] 2. The fold genuinely implements newest-wins. I traced all four orderings at one head H against the code at head, not the comment above it:

  #	Older event	Newer event	Fold result	Correct?
  1	marker FAIL @ H (t1)	native APPROVED (t2)	RAT > MARKER_AT → APPROVED arm → CODE_PASS=1, CODE_FAIL=0 → merges	yes — the old FAIL does not block
  2	native CHANGES_REQUESTED (t1)	marker PASS @ H (t2)	RAT > MARKER_AT false → fold skipped → CODE_PASS=1 stands → merges	yes
  3	native APPROVED (t1)	marker FAIL @ H (t2)	fold skipped → CODE_FAIL=1 stands → blocks	yes — a newer FAIL blocks
  4	marker PASS @ H (t1)	native CHANGES_REQUESTED (t2)	RAT > MARKER_AT → CR arm → CODE_FAIL=1, CODE_PASS=0 → blocks	yes

  The wedge the ruling named is closed: case 1 (FAIL → repair → re-PASS at the same head) now clears instead of blocking forever.

  Two edge cases are handled correctly and deliberately:

  • No marker at all (_tag: "none") → no commentId → MARKER_AT="" → the review is the only current-head event and decides alone. Correct.
  • A stale/sha-less marker → commentId is present, but ship-it's [ "$((CODE_PASS + CODE_FAIL))" -gt 0 ] guard is false (the verb exits non-zero on both polarities), so MARKER_AT stays empty and the non-participating marker cannot suppress the review. That guard is load-bearing, not incidental — without it a stale marker's created_at would enter the compare.

  I also confirmed the shell primitives under bash (not my zsh): [ "$a" \> "$b" ] is a working lexical compare (ISO-8601-UTC ⇒ chronological, as the comment claims), and [ x = y ] && CODE_FAIL=1 || CODE_FAIL=0 behaves correctly in both branches — the &&-arm is an assignment that always exits 0, so the || cannot misfire.

• [PASS] 3. The same defect in write-code R1 and heal-ci is independently fixed, prose included. Confirmed both, separately from ship-it:

  • write-code R1: fold now [ "$RSTATE" = "CHANGES_REQUESTED" ] && CODE_FAIL=1 || CODE_FAIL=0 behind the newest-wins gate — a newer APPROVED clears CODE_FAIL where the old [ -n "$RSHA" ] && [ "$RSTATE" = "CHANGES_REQUESTED" ] && … CODE_FAIL=1 only ever raised it. Prose updated (R1 preamble): "The fold is newest-wins by timestamp, matching ship-it Step 2 … so a newer APPROVED clears an older FAIL rather than the FAIL standing forever."
  • heal-ci: identical fold and identical clearing behavior. Prose updated: "The fold is newest-wins by timestamp (as in ship-it Step 2 / write-code R1) … a current-head CHANGES_REQUESTED is a code FAIL unless a newer current-head marker already PASS'd."
    Both state the rule rather than leaving it implicit in the block — which is what the ruling required.

• [PASS] 4. _tag == "current" is the same fact as a second verdict read, not a weakening. resolveVerdict returns current iff the latest authorized marker's @ <sha> prefix-matches the head; isReviewed = _tag === "current" && polarity === expect. So _tag == "current" ⟺ (--expect PASS exits 0 or --expect FAIL exits 0) ⟺ ship-it's CODE_PASS + CODE_FAIL > 0. Exactly equivalent, and it saves a redundant API round-trip. The two skills use the _tag form because they only make the --expect FAIL call; ship-it uses the sum because it already has both. Both are correct expressions of "a current-head marker verdict stands".

• [PASS] 5. The 5xx fix is fail-closed, and ship-it needed no change — but I falsified the stated rationale first. The claim is "absent stdout JSON == transport failure". I tested the actual failure path rather than accepting it, and the premise is imprecise while the guard is still effective:

  • A failing verdict read (I forced one against a nonexistent PR) does not produce empty stdout — the Effect error log goes to stdout (confirmed: captured 3402 chars with 2>/dev/null). So the capture is non-JSON, not absent.
  • jq -e . on that blob exits 4 → VERDICT_UNKNOWN=1 trips. The guard works for the real transport-failure mode.
  • Measured on jq 1.6 for completeness: invalid JSON → 4, null → 1, empty → 0. So the guard keys on non-JSON, not on absence. It happens to be correct because the failure path is never actually silent.
  • ship-it needing no change is right: an unresolved namespace leaves <g>_PASS=0 and <g>_FAIL=0 ⇒ unverified ⇒ refuse. The asymmetry is principled — ship-it's failure direction is refuse-to-merge (safe), write-code/heal-ci's was skip-a-repair (silent loss), so only the latter two need the explicit DEFER.

Original scope — re-verified at this head

• [PASS] The three inline resolvers stay deleted and cite the verb. Confirmed by running adoption-lint check over the full CI corpus (the workflow's own find claude-plugins/kampus-pipeline -type f \( -name '*.md' -o -name '*.sh' \) + drive-issue.js, 62 files): clean — no un-cited re-derivations of a tool-owned decision, and every declared exemption is valid.
• [PASS] The lint is NON-vacuous — falsified. I stripped every verdict read citation from write-code/SKILL.md in a disposable tree (14 → 0) and re-ran: the lint went red with precisely the expected row — write-code/SKILL.md: re-derives the ADR-0058 SHA-bound verdict-marker resolution that pipeline-cli verdict read owns, instead of citing the verb. Restored; tree verified clean afterward. The clean result is earned, not a matcher that cannot fire.
• [PASS] Grandfathered exemptions are at ZERO. Both heal-ci/write-code grandfathered entries are removed; only the mirror (drive-issue.js, a runtime .js that cannot import the TS core) survives, and the manifest docblock now states the pile is at zero and re-growth is forbidden.
• [PASS] write-code's multi-PR scan + N=3 FAIL-round cap preserved. [ "$ROUNDS" -ge 3 ] && continue intact; the ROUNDS jq (write+ ACL gate, >120s gap clustering) is untouched by the repair commit.
• [PASS] heal-ci's N=3 cap preserved. ROUNDS < 3 condition intact.
• [PASS] ship-it's §CP advisory resolution untouched. Step 2.§CP is unchanged; is_current is retained and re-scoped to exactly the two signals the verb does not resolve (the native review's commit_id, the advisory's body Reviewed-head), with its empty-$1 short-circuit intact.
• [PASS] Path hygiene. No home / absolute / machine-local / vault / sibling-repo path on any added line.

Non-blocking observations — none block this PR

1. ship-it's code fold has no VERDICT_UNKNOWN equivalent, and its empty-MARKER_AT branch is not fail-closed. If both verdict read calls fail transport-wise while the reviews fetch succeeds, MARKER_AT is empty and a current-head APPROVED sets CODE_PASS=1 — even if a newer current-head FAIL marker exists but went unread. I checked the base commit before raising this: the pre-PR block had the identical hole (one comments_file fetch; a failure yielded null markers and the same review-only fold), so this is pre-existing and unchanged, not a regression, and out of this PR's "no behavioral change" scope. Worth a follow-up now that the sibling skills have the guard and ship-it is the merge authority — the fix mirrors what landed here.
2. The VERDICT_UNKNOWN detector keys on non-JSON, not on absence. Effective today only because the failure path writes its error log to stdout. A genuinely empty capture ($VERDICT unresolved, exec failure) exits 0 on jq 1.6 and would not trip. One-token hardening: [ -n "$J" ] && jq -e . …. Robustness nit, not a defect in the shipped behavior.
3. API cost (carried over, unchanged): Step 1 issues 3 verdict read invocations per open authored PR, each re-fetching head + comments + collaborator permissions. Bounded by the agent's open-PR count.

Checks at head

• [PASS] lint crew corpus for inline re-derivations of tool-owned decisions — success; reproduced locally on the full corpus and falsified.
• [PASS] validate skill frontmatter, lint skill corpus for GraphQL-path gh calls + invalid YAML frontmatter, check docs have no dead internal links — success.
• [PASS] lint / format / typecheck, packages unit tests, unit + client tests — success.
• [PASS] sweep pipeline-crew for personal-data leaks, scan changed files for leaks, scan PR commits for secrets — success.
• [INFRA] integration tests — failure at head, unrelated to this diff. Root cause from the job log is Cloudflare D1 rate-limiting: 429 {"code":971,"message":"Please wait and consider throttling your request speed"} across content-removal-substrate.test.ts, plus leaked-stage teardown warnings (Orphan integration-stage sweep / reliable teardown on partial deploy #690/apps/web pr-<n> preview stacks leak: cleanup job reports success but workers survive on Cloudflare #813). This diff changes three SKILL.md files and one manifest constant — no worker, D1, or runtime surface. Passes on recent main. Needs a re-run before merge, not a repair.
• [PENDING] e2e (reads + authed + flows, blocking) — still in progress at review time; the merge actor must confirm it green.

Fan-out (silent-failure · type-design · test-gap)

The silent-failure axis is where this PR does its work, and the two additions (newest-wins clearing + the DEFER-on-unresolved) both close real silent-failure paths. Observation 1 above is the one residual on that axis; it is pre-existing, so it is noted rather than routed as a FAIL or an appended AC. No out-of-scope findings worth a report beyond that follow-up.

Note for the merge actor

Every body checkbox is [PASS], so this advisory is enqueue-eligible on the skill namespace (§6.6/ADR 0151). Two gating conditions remain outside this verdict: re-run integration tests (D1 429 flake) and confirm e2e is green.

[usirin]

repair round 1 — re-pushed at `aa584d7bcfd98addedc2aa1377c3839f6f1c0f47` (was `5ab6a4d1`).

Blocking finding addressed — newest-wins restored (not FAIL-precedence)

The refactored native-review fold had become FAIL-precedence while the prose in the same section still specified newest-wins (ship-it SKILL.md "the newest of {latest decisive review, latest review-code marker comment} by timestamp", and "an old FAIL behind a newer PASS does not block"). The refactor had also dropped the timestamps newest-wins needs (REVIEW's at: .submitted_at, and the marker objects' created_at).

Resolution: the prose is the source of truth; the code now matches it. FAIL-precedence was not a conservative default — a PR FAIL'd, repaired, and re-PASSed at the same head would have stayed permanently blocked by the superseded FAIL, wedging the repair → re-review → ship loop. It also violated #3686's "no behavioral change to verdict resolution" AC.

• REVIEW carries at: .submitted_at again in all three surfaces.
• The marker's created_at is recovered from the resolving comment id verdict read prints on stdout (the verb's outcome record carries no timestamp), so the fold has both sides of the compare. _tag == "current" is what says "a current-head marker verdict stands"; a none/sha-less/stale one does not participate, so the review then decides alone.
• The fold picks the newest event by timestamp (ISO-8601-UTC, so a lexical > is the chronological compare). A newer APPROVED clears an older FAIL; a newer marker stands over an older review.
• write-code R1 and heal-ci had the same defect (they only ever raised CODE_FAIL, never cleared it on a newer APPROVED) — fixed identically, since the same PR introduced it and their prose states the same rule.

Non-blocking point also addressed (it was small)

The transient-5xx fail-open: verdict read prints its outcome JSON on both exit paths, so absent JSON means the namespace never resolved at all — a transport error, not a verdict. write-code R1 and heal-ci now flag that as VERDICT_UNKNOWN and defer rather than silently read it as "no FAIL" and skip a real repair. ship-it needs no change: an unresolved namespace already leaves both <g>_PASS and <g>_FAIL at 0, which is exactly unverified → refuse.

I did not reduce the API-call count — that would mean re-hand-rolling the resolution the PR just deleted, which is the opposite of this issue.

Preserved from the original diff

Three inline verdict-marker resolvers stay deleted and cite pipeline-cli verdict read; grandfathered exemptions stay at zero; write-code's multi-PR scan + N=3 FAIL-round cap and heal-ci's N=3 cap intact; ship-it's §CP advisory resolution untouched.

Guards re-run at this head

• pnpm typecheck — 33/33 pass
• pnpm lint — clean (9 pre-existing warnings, 0 errors)
• pipeline-cli adoption-lint check on the full CI corpus (find claude-plugins/kampus-pipeline -type f \( -name '*.md' -o -name '*.sh' \) + drive-issue.js) — clean — no un-cited re-derivations, non-vacuous (whole corpus scanned)
• pipeline-cli tests — 113 files / 1606 tests pass
• validate-skills.sh — 26 skills valid; validate-gate-path-drift.sh — 17 checks OK
• Every edited bash block bash -n clean

The head moved, so the prior verdicts are dismissed — re-review needed against aa584d7b.