feat(doctor): preflight the two merge-queue governance preconditions ship-it needs (#4303)

[usirin]

What this does

ship-it's enqueue passes no merge-method flag on purpose — the merge queue owns SQUASH (ADR 0132). That makes it silently depend on two GitHub-side repo settings that no in-repo artifact expresses and no repo-scoped stand-up script can create: repo-level allow_auto_merge, and a merge_queue rule on the default branch. Nothing preflighted either, so a fresh repo could build, review, get an approval, and enqueue — and the merge would never land. That is the finish line, which is exactly where an adopter concludes the product does not work.

doctor.sh gains two Tier-2 checks (its own charter: gating — a stage fails deep without these), plus the adopter-facing pointer. It still writes nothing: every fix is a command or an admin path it prints.

The read discipline — an unreadable state is UNKNOWN, never the negative answer

Both checks distinguish three outcomes, not two: pass, missing-precondition, and read-failed. gh api writes its error body to stdout without applying --jq, so an unguarded capture reads the error as data (the #4223 class). Each read therefore keeps gh's exit status and admits only the literal values it expects — anything else resolves UNKNOWN with a reason line that says the read failed.

This is not theoretical here. A live probe turned up a fact worth recording: allow_auto_merge is admin-visible only. On a repo the token lacks admin on, GitHub omits the key entirely and gh api --jq '.allow_auto_merge' exits 0 printing nothing — so "absent" is indistinguishable from "false" if you look only at the value. That is precisely why the pass arm demands the literal true and the red for that case says could not read, not disabled, and points at needing an admin token.

Verified live against all three states

Target	allow_auto_merge	merge-queue rule
this repo (both configured)	✓ PASS	✓ PASS on default branch main
a public repo with no queue, default branch trunk	✗ READ FAILED (non-admin token — UNKNOWN, correctly not "disabled")	✗ MISSING PRECONDITION
a nonexistent repo	✗ READ FAILED	✗ READ FAILED (no branch to ask about)

The trunk case also exercises the no-hardcoded-main requirement: the branch is resolved from .default_branch, and a branch containing / is percent-encoded so it stays one path segment.

Acceptance criteria

• Tier-2 check reads repo-level allow_auto_merge, reds when false, prints the exact admin fix as a command (gh api -X PATCH … -F allow_auto_merge=true) and never runs it.
• Tier-2 check reads repos/{repo}/rules/branches/{branch} for the resolved default branch (never a hardcoded main) and reds when no applying rule has type == "merge_queue". Endpoint + rule shape reused from the existing reader in packages/pipeline-cli/src/tools/merge-intent/github.ts, not re-implemented.
• Both fail closed on an unreadable response, with a reason that says the read failed and is never indistinguishable from a pass (§ZS / ADR 0092).
• Each red names which precondition is missing and points at ADR 0132 (§Consequences for the ruleset toggle, §Addendum §1 for allow_auto_merge) instead of re-deriving the why.
• The adopter-facing stand-up surface (the plugin README) names both as repo-admin prerequisites, as a pointer into ADR 0132 rather than a second copy — a new ## Repo-admin prerequisites section, linked from How to kick it off, the doctor row, and the foreign-repo validation section. The "a logged-in gh is the only prerequisite" line is corrected to scope that claim to the plugin itself.
• ship-it/SKILL.md is untouched — no merge-method flag added. ADR 0132 owns the method through the queue.
• Nothing asserts the INFERRED non-TTY gh pr merge --auto behavior as verified. No live probe of it was run, and neither check depends on it — both preconditions are independently required whatever error shape a missing queue produces.

Deliberately not in scope

• The issue's premise that "the fix is not code" is not followed; triage already corrected it. A doctor.sh preflight is code.
• No governance state is created or mutated anywhere.
• doctor's required-label set is narrower than what triage needs to home an issue #4300's label-set gap is untouched (same file, different fix). Both land in doctor.sh, so whichever merges second should rebase.
• The live No issue expresses the Pipeline Anywhere acceptance test: foreign repo, second operator, end to end #4268 acceptance test is not run.

One adjacent defect observed, not fixed here

Probing the unreadable-repo case surfaced the same defect class, pre-existing, in the checks above mine: check 2b does WORKFLOWS=$(gh api … --jq '.total_count' 2>/dev/null || echo 0), so on a failed read gh's error body lands on stdout and || echo 0 appends to it — yielding {"message":"Not Found",…}0, a [: integer expression expected warning, and then the wrong reason line: "no CI workflows defined" when the truth is that the read failed. Check 3b (RUNEV) has the identical shape. That is out of this issue's acceptance criteria and enters through report → triage rather than as drive-by scope here; filed separately.

Gates

• pnpm typecheck — clean (29/29).
• pnpm lint:worktree — clean (no biome-handled changed files).
• pipeline-cli gh-phoenix lint-skills on both changed skill files — clean.
• bash -n + shellcheck on doctor.sh — clean; the only shellcheck warning (SC2043) is pre-existing in the Tier-3 loop I did not touch.
• pipeline-cli cp-classify classify on the changed paths — control-plane [path-match], BLOCKING (human merge).

Fixes #4303

[github-actions]

No preview deploy

• No preview deploy for this PR — its diff touches no deploy-relevant path, so no preview stack was minted and e2e is not applicable. _(af23cea)

• web — Stage pr-4346 torn down.

[usirin]

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

PR #4346 is §CP — cp-classify re-resolved the live CONTROL_PLANE_RE from origin/main and returned control-plane [path-match]: claude-plugins/kampus-pipeline/skills/doctor/doctor.sh matches the ^claude-plugins/kampus-pipeline/skills/([^/]+/)*[^/]+\.sh$ branch. My verdict is advisory only: it does not authorize a merge. Under the §CP hard gate (ADR 0135), a @kamp-us/control-plane member approves this at its current head and ship-it then enqueues it (ADR 0048 single merge authority) — there is no human hand-merge in the §CP path.

Reviewed-head: @ af23cea

Class fan — pipeline-cli class-probe classify --namespaces over the 3 changed files → has-code, has-skills ⇒ required namespaces review-code + review-skill. Both are posted at this head, each as its own comment. No apps/web/src/** path ⇒ review-design not required, not dispatched.

---

The central claim — independently re-verified, not taken on report

The PR rests on this: allow_auto_merge is admin-visible only; on a repo the token lacks admin on, GitHub omits the key entirely, so gh api --jq '.allow_auto_merge' exits 0 printing nothing, making "absent" and "false" indistinguishable by value. I re-ran the probe myself rather than accepting it:

$ gh api repos/cli/cli --jq '.allow_auto_merge'    # non-admin token
                                                   # (no output at all)
rc=0
$ gh api repos/cli/cli | jq 'has("allow_auto_merge")'
false
$ gh api repos/kamp-us/phoenix | jq 'has("allow_auto_merge"), .allow_auto_merge'
true
true

Confirmed. Exit 0, empty stdout, key genuinely absent from the payload on a non-admin repo; present and true where the token holds admin. The trap is real: a naive [ "$X" != "true" ] && echo "auto-merge disabled" would have confidently reported a fact it never read.

The consequence is correctly implemented in the code. doctor.sh at head admits only literals and routes everything else to UNKNOWN:

• pass arm: [ "$AM_RC" -eq 0 ] && [ "$AUTO_MERGE" = "true" ] — demands the literal true.
• disabled arm: [ "$AM_RC" -eq 0 ] && [ "$AUTO_MERGE" = "false" ] — demands the literal false.
• everything else (the omitted-key empty string, a non-zero gh exit, an error body) falls to the else arm, which reds as READ FAILED … — UNKNOWN, which is neither a pass nor 'disabled' and whose fix hint reads "re-run with a token holding ADMIN … GitHub omits allow_auto_merge for non-admins".

An unreadable read never resolves to a definite answer in either direction. That is the defect class this PR is a live instance of, and it is closed here.

Live three-state proof — re-run by me, all three reproduce

I executed doctor.sh at the PR head (materialized read-only into a per-run ref, never read from the base tree) against all three states the author claims:

Target	allow_auto_merge line	merge-queue line
kamp-us/phoenix (admin)	✓ repo-level auto-merge enabled	✓ default branch 'main' is merge-queue governed
cli/cli (public, non-admin, no queue)	✗ READ FAILED … (gh exit 0, returned: <empty>) — UNKNOWN, which is neither a pass nor 'disabled'	✗ MISSING PRECONDITION: no merge_queue rule applies to default branch **'trunk'**
kamp-us/this-repo-does-not-exist-4346	✗ READ FAILED … (gh exit 1, returned: {"message":"Not Found"…})	✗ READ FAILED: could not resolve the default branch … UNKNOWN, not a pass

The trunk case is the load-bearing one and it holds. The red names 'trunk', and the printed fix says "a ruleset targeting 'trunk'". The branch is read from gh api "repos/$REPO" --jq '.default_branch' and interpolated into both the endpoint and every message — no hardcoded main literal anywhere in the added lines. A hardcoded main would have red-flagged cli/cli for the wrong reason, which is exactly the Pipeline-Anywhere defect class; it does not.

Bonus correctness the AC did not ask for: the branch is percent-encoded (sed 's|/|%2F|g') so a legitimate release/main-style default branch stays one path segment instead of splitting the REST route.

Acceptance criteria — #4303, one at a time

• [PASS] AC1 — Tier-2 check reads repo-level allow_auto_merge, reds when false, prints the exact admin fix as a command (never runs it). doctor.sh check 2c. The red names allow_auto_merge=false and the exact enqueue failure string; the fix prints gh api -X PATCH "repos/$REPO" -F allow_auto_merge=true (typed -F, so a JSON boolean — not the string "true" a -f would send) plus the Settings path. doctor.sh writes nothing: every gh api in the added block is a GET, and the only PATCH is inside a fix "…" string.
• [PASS] AC2 — Tier-2 check reads repos/{repo}/rules/branches/{branch} for the RESOLVED default branch (never a hardcoded main) and reds when no applying rule has type == "merge_queue". Verified live on trunk (table above). Endpoint and rule shape reuse the merge-intent reader's approach, as the issue's rabbit-hole note required — rules/branches/<b>, not branches/<b>/protection (which 404s on a ruleset-governed repo). The predicate is [.[] | select(.type == "merge_queue")] | length, i.e. any applying rule, matching the AC's wording.
• [PASS] AC3 — Both fail closed on an unreadable response; never indistinguishable from a genuine pass. Three distinct unreadable shapes exercised live (non-admin omission, 404 on the repo read, 404 on the rules read); all three red, all three say the read failed, all three say UNKNOWN, all three increment fails. Both checks capture gh's exit status into an explicit *_RC and additionally shape-assert the payload (= "true" / = "false" for the boolean; grep -Eq '^[0-9]+$' for the count), so an error body arriving on stdout cannot be read as data (§ZS / ADR 0092; the ship-it's §CP approval gate resolves head, author, and roster with unguarded reads #4223 class). set -uo pipefail with no -e (head file, line 16) is what makes the X=$(…); RC=$? idiom actually work here.
• [PASS] AC4 — Each red names WHICH precondition is missing and points at ADR 0132 (§Consequences for the ruleset, §Addendum §1 for allow_auto_merge) rather than re-deriving the why. Verified verbatim in the live output: Why: ADR 0132 §Addendum §1 on the auto-merge red, Why: ADR 0132 §Consequences on the queue red, each fix line carrying the full ADR URL. Neither re-derives the reasoning.
• [PASS] AC5 — The adopter-facing stand-up surface names both preconditions as repo-admin prerequisites, as a POINTER into ADR 0132, not a second copy. claude-plugins/kampus-pipeline/README.md gains a ## Repo-admin prerequisites section (two-row table: where to click, why it matters), reached from a new step 2 in "How to kick it off", from the doctor roster row, and from the "Start with doctor" note. It states the what and then explicitly defers: "the reasoning, the ship-it enqueue blocked: merge_queue required on main but repo allow_auto_merge is false #1817 outage … are recorded once in ADR 0132 … Read it there." The anchor resolves (#repo-admin-prerequisites matches the heading).
• [PASS] AC6 — ship-it/SKILL.md:2055 unchanged; no merge-method flag added. The diffstat against origin/main shows exactly 3 files, none under ship-it/**. Read at the head ref, the enqueue is still bare gh pr merge $PR --auto.
• [PASS] AC7 — Nothing asserts the INFERRED non-TTY gh pr merge --auto behavior as verified. No claim about an error demanding --merge/--rebase/--squash, no exit code, no stderr shape appears in any of the three changed files at head. The one failure string quoted (Auto merge is not allowed for this repository (enablePullRequestAutoMerge)) is the allow_auto_merge=false string that ship-it/SKILL.md:2152 and ship-it's enqueue needs repo auto-merge + a queue ruleset no fresh repo has #4303's own body already carry as verified — a different claim from the inferred one. One residual note on this below.

AC half: 7/7 PASS.

Read-discipline checks I was asked to make

• Nothing reads merge-queue membership. The added lines make exactly three REST reads: repos/{repo} (twice) and repos/{repo}/rules/branches/{branch}. No mergeQueue, no entries, no graphql — the banned surface is untouched.
• Nothing depends on inferred non-TTY gh pr merge --auto behavior. Both checks read governance state; neither invokes gh pr merge, and no branch is predicated on what that command would print.

Fan-out (ADR 0079) — silent-failure / type-design / test-gap

• silent-failure: none introduced. This is the rare diff whose entire purpose is removing a silent-failure surface; every new read is exit-status-checked and value-shape-asserted. brief() clamps an error body to one 120-char line so a failed read stays attributable instead of smearing JSON across the checklist — deliberate and correct.
• type-design: n/a for shell, but the moral equivalent holds — the pass arm is a whitelist of two literals, so "unreadable" is unrepresentable as "answered".
• test-gap: doctor.sh has no unit-test harness anywhere in the repo, so this is the pre-existing shape of the artifact, not a regression this PR introduces. Not routed as an appended AC. The author substituted live three-state execution, which I independently reproduced — stronger evidence than a mock, for a script whose whole job is reading live GitHub state.

Shell hygiene and CI

• bash -n on the head file: clean (rc 0).
• shellcheck -s bash on the head file: one finding, SC2043 at line 198 (for pkg in @kampus/pipeline-cli) — pre-existing; the same construct sits at line 122 on origin/main. Zero findings on any added line.
• validate-skills.sh at the head: validate-skills: OK — 26 skills valid.
• CI at this head: all non-skipped checks green. Run-evidence bundle PRESENT for head af23cea3 — producer run 30224043479, artifact 8638101397, manifest.commit == head, schemaVersion 1; checks 2/2 pass; tests 2420/2420 passed, 0 failed, 0 skipped.

For the control-plane approver — three things to know before you approve

1. The disclosed pre-existing defect (#4347) was SEEN, judged, and deliberately left out of scope — I agree with that call. Checks 2b/3b use $(gh api … --jq '.total_count' 2>/dev/null || echo 0); on a failed read gh's error body lands on stdout and || echo 0 appends to it. I reproduced it — running against a nonexistent repo prints:

doctor.sh: line 115: [: {"message":"Not Found",…,"status":"404"}0: integer expression expected
  ✗ no CI workflows defined — ship-it's checks-green gate (Step 3) passes vacuously

A confident wrong reason line for a read that actually 404'd — the identical defect class this PR fixes, on a neighbouring check. Leaving it out is correct: it sits on lines this PR does not author, fixing it would widen a §CP diff already under human review, and it is filed as #4347 with its own reproduction. Crucially the new checks do not inherit the pattern — they capture $? explicitly and shape-assert instead of || echo 0. Approving this PR does not bless the old lines; it leaves them exactly as they were, tracked.

2. Merge-order collision — real, and the one thing to act on. #4303, #4300, and #4347 all land in doctor.sh. #4300 already has an open PR: #4337, and its doctor.sh hunks (@@ -5,11 +5,23 @@, @@ -46,49 +58,134 @@, @@ -144,6 +241,41 @@) overlap this PR's regions at both ends — the tier-header comment block and the Tier-1/Tier-2 body where checks 2c/2d are inserted. A textual conflict for whichever merges second is near-certain, not speculative. Whoever ships second must rebase, not race — exactly as #4303's own sequencing note instructed. Neither PR duplicates the other's fix (I checked: #4337 does not touch the total_count defect, so #4347 stays genuinely unowned), so this is merge mechanics, not a design conflict.

3. Two small residual notes — neither blocks, both yours to weigh.

• On the non-admin path the line reads READ FAILED … (gh exit 0, returned: <empty>). That single arm covers two genuinely different facts — the call failed, and the call succeeded but the field was withheld. Both are correctly UNKNOWN and neither resolves to an answer, so the invariant holds; but the label "READ FAILED" next to a printed gh exit 0 reads as mildly self-contradictory to a first-time adopter. The fix hint ("GitHub omits allow_auto_merge for non-admins") disambiguates it on the very next line, which is why this is a wording observation and not a finding.
• The queue red says "the merge never lands". That is a consequence stated from ADR 0132's own text (the queue owns the method), not the specific inferred --merge/--rebase/--squash error shape ship-it's enqueue needs repo auto-merge + a queue ruleset no fresh repo has #4303 forbids asserting — so AC7 passes. Worth knowing the sentence is still a paper consequence rather than an observed run.

Bottom line for the approver: the central claim is independently reproduced, the unreadable-read invariant holds in both directions, the default branch is genuinely resolved (proved against a repo whose default branch is trunk), all 7 acceptance criteria pass, the added lines are bash -n and shellcheck clean, skill-lint and CI are green, and the one known pre-existing defect in the file was correctly identified, filed, and left alone. The only thing I would not let slip is the #4337 rebase ordering.

[usirin]

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

PR #4346 touches the control plane — cp-classify re-resolved the live CONTROL_PLANE_RE from origin/main and returned control-plane [path-match] on claude-plugins/kampus-pipeline/skills/doctor/doctor.sh (the skills/**/*.sh branch of §CP). My verdict is advisory only: it does not authorize a merge. Under the §CP hard gate (ADR 0135), a @kamp-us/control-plane member approves this at its current head and ship-it then enqueues it (ADR 0048 single merge authority) — there is no human hand-merge in the §CP path.

Reviewed-head: @ af23cea

This is the review-skill half of a two-namespace fan. class-probe returned has-code, has-skills for the 3 changed files, so review-code and review-skill are both required at this head; the review-code advisory is posted as its own separate comment. The acceptance-criteria verification (7/7 PASS, with the allow_auto_merge admin-visibility claim independently re-probed and the three-state run independently reproduced) lives in that comment and is not repeated here — this one is the skill-rigor half over the two behavioral artifacts the diff touches: skills/doctor/SKILL.md and the plugin README.md.

Skill-rigor checklist (ADR 0073 §1) — all four run

• [PASS] 1. Behavioral correctness. The SKILL.md edits are descriptions of behavior that I executed, not claims taken on trust. Two new Tier-2 rows are added to the tier table (allow_auto_merge; a merge_queue rule applies to the **resolved** default branch), a paragraph naming them the "repo-admin governance pair" doctor can only report, and one new Conventions bullet: "An unreadable state is UNKNOWN, never the negative answer." I ran doctor.sh at the head against three repos and every one of those sentences matches what the script actually printed — including the emphasized word resolved, which the trunk run demonstrates rather than merely asserts. The doc does not overclaim: it says doctor prints the fix and never applies it, and the added block contains only GETs (the one PATCH is inside a fix "…" string).
• [PASS] 2. Trigger / description quality. The doctor frontmatter is untouched — the diff's SKILL.md hunks start at line 42, well below it. No trigger surface widened, no sibling lane shadowed. The plugin README.md roster row for doctor is extended with "the two repo-admin merge prerequisites", which sharpens the description of what doctor covers without changing when it fires.
• [PASS] 3. Cross-skill conflict / shadowing. No seam another skill reads is changed: no marker shape, no namespace, no §CP definition, no routing rule. The ship-it contract is cited (the no-merge-method-flag enqueue, the enablePullRequestAutoMerge failure string) and ship-it/SKILL.md is not in the diff, so there is no one-sided contract change. doctor's lane (a read-only preflight that prints fixes) is preserved exactly. One real collision to sequence, not a defect — see the note to the approver below.
• [PASS] 4. Gate-invariant preservation — the catastrophic check. This PR is §CP by path, so I walked the diff against the invariants. Nothing is removed or softened; the movement is strictly toward stricter. doctor.sh's contract is "exit 0 only when every Tier-1 and Tier-2 check passes" — two new Tier-2 checks make that exit harder to earn, never easier, and both new failure paths increment fails. There is no path through the added code that can turn a previously-failing run green. Specifically: ship-it's control-plane refusal, its per-namespace current-head PASS gate, the SHA-staleness refusal, the run-evidence assertion, the ACL author-gate, every reviewer's config-pin and conjunctive bar, the §CP set, and the §5/§6 matcher contracts are all outside this diff entirely — the change touches no gate skill and no formats contract. The one thing the added code could have weakened is the §ZS fail-closed rule (ADR 0092), and it does the opposite: it adds two more reads that fail closed on an unreadable response, where before there was no read at all.

Skill hygiene

• validate-skills.sh at the head: validate-skills: OK — 26 skills valid.
• Frontmatter untouched; no new skill introduced.
• Links: relative in-repo links (../gh-issue-intake-formats.md) resolve; ADR links use full github.com URLs consistent with the rest of the corpus; the new README anchor #repo-admin-prerequisites matches its ## Repo-admin prerequisites heading.
• No home / local / absolute / sibling-repo path appears anywhere in the diff.
• Doc-surface fit is right: the what and where to click lands on the adopter-facing README, the why stays a pointer into ADR 0132 (§Addendum §1, §Consequences) rather than a second copy — which is what AC5 asked for and what keeps the ADR the single source.

Deviation disclosure (§DEV) — clean

The PR body discloses every departure up front: the issue's "the fix is not code" premise deliberately not followed (triage had already corrected it), no governance state created or mutated, #4300's label gap untouched, the live #4268 acceptance test not run, and the adjacent pre-existing defect found-but-not-fixed. Nothing I found in the diff is undisclosed.

For the control-plane approver

The pre-existing defect the author left alone (#4347) — seen, judged, correct to leave. Checks 2b/3b use $(gh api … --jq '.total_count' 2>/dev/null || echo 0); on a failed read gh's error body lands on stdout and || echo 0 appends to it, so the script emits an integer expression expected warning and then a confidently wrong reason line ("no CI workflows defined") for a read that actually 404'd. I reproduced it. It is the same defect class this PR fixes, one check up the file — but it is on lines this PR does not author, and widening a §CP diff that a human has to read is the wrong trade. It is filed as #4347 with a reproduction, and the new checks do not inherit the pattern (they capture $? and shape-assert instead of || echo 0). Approving this leaves those old lines exactly as they are, tracked.

Sequencing — the one thing to act on. #4303 (this), #4300, and #4347 all land in doctor.sh, and #4300 already has an open PR: #4337. #4337's doctor.sh hunks (@@ -5,11 +5,23 @@, @@ -46,49 +58,134 @@, @@ -144,6 +241,41 @@) overlap this PR at both the tier-header comment block and the Tier-1/Tier-2 body where checks 2c/2d are inserted, and it edits skills/doctor/SKILL.md too. A textual conflict for whichever lands second is near-certain. Two follow-on notes for whoever ships second: (a) rebase rather than race, exactly as #4303's own sequencing note said; (b) #4337 widens doctor's required-label set, so the literal "15 required labels" that this PR's README/SKILL.md prose carries will go stale the moment #4337 lands — worth a glance during that rebase. Neither PR duplicates the other's fix, so this is merge mechanics only.

Verdict: skill-rigor 4/4 PASS, hygiene clean, no gate invariant touched. Combined with the review-code advisory's 7/7 on the acceptance criteria, this PR is substantively ready; it needs your §CP approval at this head, and it needs to be sequenced against #4337.