From af23cea3f3a18ba7f907a69c34230d7e6621960c Mon Sep 17 00:00:00 2001 From: usirin Date: Sun, 26 Jul 2026 15:40:24 -0700 Subject: [PATCH] feat(doctor): preflight the two merge-queue governance preconditions ship-it needs (#4303) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ship-it's enqueue passes no merge-method flag because the merge queue owns it (ADR 0132), so it silently depends on two GitHub-side repo settings no repo-scoped script can create: repo-level allow_auto_merge, and a merge_queue rule on the default branch. Nothing preflighted either, so a fresh repo built, reviewed, approved, and enqueued — and the merge never landed. doctor.sh gains two Tier-2 checks. Both resolve the default branch rather than assuming main, and both treat an unreadable response as UNKNOWN with a reason that says the read failed — never as the negative answer (gh api prints its error body to stdout without applying --jq, the #4223 class). Verified live against three states: this repo (both pass), a repo with no queue on a 'trunk' default branch (MISSING PRECONDITION), and a nonexistent repo (READ FAILED). allow_auto_merge turns out to be admin-visible only — absent, not false, for a non-admin token — which is exactly why the pass arm demands a literal true. Each red names which precondition is missing and points at ADR 0132 rather than re-deriving it. The plugin README gains the adopter-facing pointer naming both as repo-admin prerequisites. ship-it's enqueue command is untouched. Fixes #4303 Co-Authored-By: Claude Opus 5 --- claude-plugins/kampus-pipeline/README.md | 43 ++++++++-- .../kampus-pipeline/skills/doctor/SKILL.md | 18 ++++- .../kampus-pipeline/skills/doctor/doctor.sh | 78 ++++++++++++++++++- 3 files changed, 129 insertions(+), 10 deletions(-) diff --git a/claude-plugins/kampus-pipeline/README.md b/claude-plugins/kampus-pipeline/README.md index d20dcad2a..2b9f1c034 100644 --- a/claude-plugins/kampus-pipeline/README.md +++ b/claude-plugins/kampus-pipeline/README.md @@ -68,10 +68,14 @@ re-gates each round (the split-role firewall: implementer ≠ reviewer). ### How to kick it off 1. **Install the plugin** (see [Install](#install) below) and make sure `gh` is logged in — - the only prerequisite. -2. **Run `doctor` once** in the target repo to confirm the prerequisites (gh auth + scope, - the required labels, a CI signal). It turns "did I wire this up right?" into one command. -3. **Enter work through `report`** (or `wayfinder` for something still fuzzy). From there the + the only prerequisite the plugin itself carries. +2. **Have a repo admin satisfy the two GitHub-side merge prerequisites** *before* the first + run (see [Repo-admin prerequisites](#repo-admin-prerequisites) below). `doctor` reports + them; nothing in the plugin can create them. +3. **Run `doctor` once** in the target repo to confirm the prerequisites (gh auth + scope, + the required labels, a CI signal, and the two merge prerequisites). It turns "did I wire + this up right?" into one command. +4. **Enter work through `report`** (or `wayfinder` for something still fuzzy). From there the pipeline pulls it forward: `triage` makes it pickable, `write-code` picks the next triaged issue and opens a PR, a review skill gates it, and `ship-it` merges on green. @@ -94,7 +98,7 @@ handoff is a durable GitHub artifact, you can stop after any stage and resume la | `heal-ci` | Classify a red CI run into flake-vs-defect and emit one routed action — rerun a known transient once, or file a defect via `report`. | | `adr` | Record an architecture decision (Context / Decision / Consequences) into `.decisions/NNNN-slug.md` and the index, following supersede rules. | | `deslop-comments` | Ruthlessly cut comments that bury the code without earning their place — keeping load-bearing notes, collapsing duplicated "why" to ADR pointers. | -| `doctor` | Preflight a repo against the pipeline prerequisites (gh auth + scope, the 15 required labels, repo resolution, a CI signal, npm deps) and print a tiered pass/fail checklist with the exact fix command for each gap. | +| `doctor` | Preflight a repo against the pipeline prerequisites (gh auth + scope, the 15 required labels, repo resolution, a CI signal, the two [repo-admin merge prerequisites](#repo-admin-prerequisites), npm deps) and print a tiered pass/fail checklist with the exact fix command for each gap. | | `wayfinder` | The ideation-layer front door, upstream of the pipeline: chart a fuzzy destination into a living `wayfinder:map` issue, then work its open frontier of investigation/decision tickets — recording answers, graduating cleared fog, surfacing founder-decision-forks — until a concrete plan is ready for `triage` / `plan-epic` (epic #2421). | ## Install @@ -131,7 +135,31 @@ mean to operate on. (Be aware: a stale `CLAUDE_PIPELINE_REPO` will silently oper the wrong repo — the default-to-current-repo path is the safe one; the override is opt-in.) The skills use the [GitHub CLI (`gh`)](https://cli.github.com/) for all reads and writes, -so a logged-in `gh` is the only prerequisite. +so a logged-in `gh` is the only prerequisite the plugin itself carries. The two repo-admin +settings below are not the plugin's to carry — they are the host repo's. + +## Repo-admin prerequisites + +Two **GitHub-side repo settings** must be in place before the first run, and they are the +one prerequisite class the plugin cannot install: they live in GitHub repo governance, not +in the repo's files, so no in-repo artifact expresses them and no repo-scoped stand-up +script can create them. A repo admin sets both, once, per adopting repo. + +| Prerequisite | Where | Why | +|---|---|---| +| **Allow auto-merge** (`allow_auto_merge`) | Settings → General → Pull Requests → *Allow auto-merge* | `ship-it` arms the merge with `gh pr merge --auto`; without this the enqueue fails with `Auto merge is not allowed for this repository (enablePullRequestAutoMerge)`. | +| **A merge-queue rule on the default branch** | Settings → Rules → Rulesets → a ruleset targeting the default branch → *Require merge queue* (merge method SQUASH) | `ship-it` passes **no** merge-method flag on purpose — the queue owns the method. With no queue, there is no method. | + +**Why both, and why they are stated here rather than re-derived:** the reasoning, the +`#1817` outage that produced the first one, and the deliberate no-merge-method-flag decision +are recorded once in ADR [0132](https://github.com/kamp-us/phoenix/blob/main/.decisions/0132-merge-queue-for-base-freshness.md) +— §Addendum §1 for `allow_auto_merge`, §Consequences for the ruleset toggle (which that ADR +names *"a separate, last, human-only administrative step"*). Read it there. + +**These fail last, so check them first.** Both preconditions are only exercised at the +enqueue — the final step. Skip them and an adopter builds, reviews, approves, and enqueues +successfully, and then the merge simply never lands. `doctor` reads both as Tier-2 checks so +the gap surfaces before any work is done; it **prints** the fix and never applies it. ## Design notes — versioning and spec conformance @@ -181,7 +209,8 @@ the superseded file's status edit), never a regenerated index. **Start with `doctor`.** Before the first run in a freshly-adopted repo, run the `doctor` skill (`claude-plugins/kampus-pipeline/skills/doctor/doctor.sh`) — it asserts the prerequisites in one pass (gh auth + the `project` scope, the 15 required `status:*`/`type:*`/`p*` labels, repo -resolution, a CI signal, and the `@kampus/*` npm deps) and prints a tiered pass/fail +resolution, a CI signal, the two [repo-admin merge prerequisites](#repo-admin-prerequisites), +and the `@kampus/*` npm deps) and prints a tiered pass/fail checklist with the exact `gh label create …` / `gh auth refresh …` fix command for each gap. It turns "did I wire this up right?" into one checkable command instead of a first run that fails deep inside a `gh api` call. diff --git a/claude-plugins/kampus-pipeline/skills/doctor/SKILL.md b/claude-plugins/kampus-pipeline/skills/doctor/SKILL.md index 12ac8adad..ca77f22c1 100644 --- a/claude-plugins/kampus-pipeline/skills/doctor/SKILL.md +++ b/claude-plugins/kampus-pipeline/skills/doctor/SKILL.md @@ -42,12 +42,21 @@ Do not run the fix commands yourself. | | the 15 required labels exist (`status:*` spine, `type:*` class, `p*` priority) | the intake skills key on them — `report` applies `status:needs-triage`, `write-code` picks `status:triaged`, etc. A missing one fails a run mid-`gh api`. | | **2 — gating** | target repo resolves | a skill can't target a repo it can't name | | | at least one CI workflow exists | `ship-it` Step 3 gates on checks-green; with zero checks that gate passes vacuously | +| | repo-level `allow_auto_merge` is enabled | `ship-it`'s `gh pr merge --auto` cannot arm without it — the enqueue fails with `Auto merge is not allowed for this repository (enablePullRequestAutoMerge)` (ADR [0132](https://github.com/kamp-us/phoenix/blob/main/.decisions/0132-merge-queue-for-base-freshness.md) §Addendum §1) | +| | a `merge_queue` rule applies to the **resolved** default branch | `ship-it` deliberately passes no merge-method flag because the queue owns it (ADR 0132 §Consequences); with no queue there is no method and the merge never lands | | **3 — optional** | `@kampus/pipeline-cli` resolves on npm | `adr` / `review-plan` reach for its `decisions-index` / `epic-ledger` tools via `pnpm dlx` as the published fallback (epic #994); absent → those stages degrade | | | a `run-evidence` producer is defined | `ship-it` guard 2 runs strict when present, and **degrades to checks-green when absent** (ADR [0086](https://github.com/kamp-us/phoenix/blob/main/.decisions/0086-ship-it-foreign-repo-degradation.md)) — so this is informational, not a failure | The load-bearing pair is **auth + labels** (Tier 1): get those wrong and the very -first `report` → `triage` round trips into a raw GitHub API error. The Tier-2 pair -keeps `ship-it` honest; Tier-3 only ever downgrades a single stage. +first `report` → `triage` round trips into a raw GitHub API error. The Tier-2 checks +keep `ship-it` honest; Tier-3 only ever downgrades a single stage. + +The last two Tier-2 checks are the **repo-admin governance pair**, and they are the one +prerequisite class this checklist can only *report*: `allow_auto_merge` and the +default-branch merge-queue ruleset are GitHub-side repo settings that **no repo-scoped +script can create** — not doctor, not any stand-up script. They also fail *last*, at the +enqueue, after an adopter has already built, reviewed, approved, and armed a PR. Reading +them up front is what turns that finish-line halt into a first-run checklist line. ## Conventions @@ -56,6 +65,11 @@ keeps `ship-it` honest; Tier-3 only ever downgrades a single stage. human to run; surfacing them is the whole job. - One run, one checklist. This is a preflight, not a repair loop — it does not re-check after a fix; the operator re-runs it. +- **An unreadable state is UNKNOWN, never the negative answer.** A 403/404/5xx/empty body + reds with a reason that says *the read failed* — never "auto-merge is disabled", which is + a different fact. `gh api` prints its error body to stdout **without** applying `--jq`, so + every read keeps gh's exit status and admits only the literal values it expects (§ZS / + ADR [0092](https://github.com/kamp-us/phoenix/blob/main/.decisions/0092-gates-fail-closed-on-zero-scope.md); the defect class of #4223). - The required-label set in `doctor.sh` is the canonical one (name + color + description). If the pipeline's label dimensions change (see [`../gh-issue-intake-formats.md`](../gh-issue-intake-formats.md) §Pipeline labels), update the `REQUIRED_LABELS` diff --git a/claude-plugins/kampus-pipeline/skills/doctor/doctor.sh b/claude-plugins/kampus-pipeline/skills/doctor/doctor.sh index 5b8e497fe..9c1566340 100755 --- a/claude-plugins/kampus-pipeline/skills/doctor/doctor.sh +++ b/claude-plugins/kampus-pipeline/skills/doctor/doctor.sh @@ -7,7 +7,8 @@ # Tiers (a foreign-repo adopter reads top-down): # 1 load-bearing — gh auth + required labels. Without these the first # report/triage run fails deep inside a `gh api` call. -# 2 gating — repo resolution + a CI signal ship-it can gate on. +# 2 gating — repo resolution, a CI signal ship-it can gate on, and the +# merge-queue governance ship-it's enqueue silently depends on. # 3 optional — `pnpm dlx` packages + the run-evidence producer; their # absence degrades a stage, it does not break the pipeline. # @@ -18,6 +19,10 @@ PASS="✓"; FAIL="✗"; WARN="⚠" fails=0 say() { printf ' %s %s\n' "$1" "$2"; } +# One-line, length-clamped echo of whatever a read actually returned. A failed `gh api` prints its +# error BODY to stdout, so quoting it back verbatim would smear a multi-line JSON blob across the +# checklist — but dropping it entirely would leave "the read failed" unattributable. Clamp, don't cut. +brief() { printf '%s' "$1" | tr '\n\t' ' ' | cut -c1-120; } fix() { printf ' ↳ fix: %s\n' "$1"; } hdr() { printf '\n%s\n' "$1"; } @@ -115,6 +120,77 @@ else fails=$((fails + 1)) fi +# 2c/2d. the merge-queue governance regime ship-it's enqueue depends on. `gh pr merge $PR --auto` +# passes NO merge-method flag because the queue owns the method (ADR 0132), so it works only +# when the repo carries BOTH `allow_auto_merge` AND a default-branch ruleset with a +# `merge_queue` rule. Neither is expressible in-repo and neither can be created by a +# repo-scoped script — and both fail at the LAST step of a first run: an adopter builds, +# reviews, approves, enqueues, and the merge never lands (#4303). Read them here so the gap +# surfaces before the work, not after it. +# +# Read discipline for both (§ZS, ADR 0092): an unreadable response is UNKNOWN — never a pass, +# and never reported as the negative answer. `gh api` writes its error body to stdout WITHOUT +# applying `--jq`, so an unguarded capture reads the error as data (#4223); each read below +# therefore keeps gh's exit status and admits only the literal values it expects. +ADR0132="https://github.com/kamp-us/phoenix/blob/main/.decisions/0132-merge-queue-for-base-freshness.md" + +# `allow_auto_merge` is an ADMIN-VISIBLE field: on a repo the token lacks admin on, GitHub +# omits the key entirely and `gh api --jq` then exits 0 printing NOTHING (measured against a +# public repo). "Absent" is therefore indistinguishable from "false" if you only look at the +# value — which is why the pass arm below demands the literal `true` and everything that is +# not literally `true`/`false` resolves UNKNOWN rather than borrowing the negative answer. +AUTO_MERGE="" +AM_RC=1 +if [ -n "${REPO:-}" ]; then + AUTO_MERGE=$(gh api "repos/$REPO" --jq '.allow_auto_merge' 2>/dev/null) + AM_RC=$? +fi +if [ "$AM_RC" -eq 0 ] && [ "$AUTO_MERGE" = "true" ]; then + say "$PASS" "repo-level auto-merge enabled (allow_auto_merge) — ship-it's --auto enqueue can arm" +elif [ "$AM_RC" -eq 0 ] && [ "$AUTO_MERGE" = "false" ]; then + say "$FAIL" "MISSING PRECONDITION: repo-level auto-merge is disabled (allow_auto_merge=false) — ship-it's enqueue fails with 'Auto merge is not allowed for this repository (enablePullRequestAutoMerge)'. Why: ADR 0132 §Addendum §1" + fix "gh api -X PATCH \"repos/$REPO\" -F allow_auto_merge=true # repo-admin only, or Settings → General → Pull Requests → Allow auto-merge — $ADR0132" + fails=$((fails + 1)) +else + say "$FAIL" "READ FAILED: could not read allow_auto_merge on ${REPO:-} (gh exit $AM_RC, returned: $(brief "${AUTO_MERGE:-}")) — UNKNOWN, which is neither a pass nor 'disabled'" + fix "re-run with a token holding ADMIN on ${REPO:-the target repo} — GitHub omits allow_auto_merge for non-admins, so its state cannot be read (let alone asserted) from here. Why it matters: ADR 0132 §Addendum §1 — $ADR0132" + fails=$((fails + 1)) +fi + +# The queue rule is read for the repo's RESOLVED default branch — never a hardcoded `main`: an +# adopter's default branch is theirs to name, and asserting the rule on a branch they don't have +# would red for the wrong reason. Endpoint + rule shape reused from the merge-intent reader +# (packages/pipeline-cli/src/tools/merge-intent/github.ts): a ruleset-governed repo 404s on +# `branches//protection`, so `rules/branches/` is what answers this. +DEFAULT_BRANCH="" +DB_RC=1 +if [ -n "${REPO:-}" ]; then + DEFAULT_BRANCH=$(gh api "repos/$REPO" --jq '.default_branch' 2>/dev/null) + DB_RC=$? +fi +if [ "$DB_RC" -ne 0 ] || [ -z "$DEFAULT_BRANCH" ] || [ "$DEFAULT_BRANCH" = "null" ]; then + say "$FAIL" "READ FAILED: could not resolve the default branch of ${REPO:-} (gh exit $DB_RC, returned: $(brief "${DEFAULT_BRANCH:-}")) — the merge-queue rule check has no branch to ask about, so its state is UNKNOWN, not a pass" + fix "re-run once the repo resolves and \`gh auth status\` shows a token that can read it — then re-read ADR 0132 §Consequences: $ADR0132" + fails=$((fails + 1)) +else + # A default branch may legitimately contain '/' (e.g. `release/main`); percent-encode it so it + # stays ONE path segment instead of splitting the REST route. + BRANCH_PATH=$(printf '%s' "$DEFAULT_BRANCH" | sed 's|/|%2F|g') + MQ=$(gh api "repos/$REPO/rules/branches/$BRANCH_PATH" --jq '[.[] | select(.type == "merge_queue")] | length' 2>/dev/null) + MQ_RC=$? + if [ "$MQ_RC" -ne 0 ] || ! printf '%s' "$MQ" | grep -Eq '^[0-9]+$'; then + say "$FAIL" "READ FAILED: could not read the rules applying to default branch '$DEFAULT_BRANCH' (gh exit $MQ_RC, returned: $(brief "${MQ:-}")) — UNKNOWN, which is neither a pass nor 'no merge queue'" + fix "re-run once \`gh api \"repos/$REPO/rules/branches/$BRANCH_PATH\"\` reads cleanly — then re-read ADR 0132 §Consequences: $ADR0132" + fails=$((fails + 1)) + elif [ "$MQ" -gt 0 ]; then + say "$PASS" "default branch '$DEFAULT_BRANCH' is merge-queue governed (a merge_queue rule applies) — the queue supplies ship-it's merge method" + else + say "$FAIL" "MISSING PRECONDITION: no merge_queue rule applies to default branch '$DEFAULT_BRANCH' — ship-it enqueues with no merge method and the merge never lands. Why: ADR 0132 §Consequences" + fix "repo-admin, human-only: Settings → Rules → Rulesets → a ruleset targeting '$DEFAULT_BRANCH' → enable 'Require merge queue' (merge method SQUASH). ADR 0132 §Consequences calls this the separate, last, human-only administrative step — $ADR0132" + fails=$((fails + 1)) + fi +fi + hdr "Tier 3 — optional (degrades a stage; pipeline still runs)" # 3a. the consolidated `pnpm dlx` package adr / review-plan reach for (epic #994: