fix(doctor): a failed workflows read is UNDETERMINED, never 'no CI workflows' (#4347)

[usirin]

Fixes #4347

What changed

doctor.sh checks 2b (CI workflows) and 3b (run-evidence producer) captured a gh api … --jq read as … 2>/dev/null || echo 0. gh does not apply --jq to an error response — it writes the raw error body to stdout and exits non-zero — so || echo 0 appended 0 to that body rather than replacing it. The subsequent numeric test then printed [: integer expression expected to stderr, evaluated false, and dropped into the negative arm: "no CI workflows defined", with a fix: telling the operator to add a workflow. A read that never ran was being reported as a definite negative answer, and the real cause (repo resolution / auth) stayed hidden behind it.

Both sites now use the three-outcome read discipline the file already holds at the Tier-2 allow_auto_merge / merge-queue checks (and, per claude-plugins/pipeline-crew/PROBES.md, "a probe that cannot itself execute must resolve to 'unknown', never 'down'"):

• gh's exit status is captured separately from the value (WF_RC=$? / RE_RC=$?) — || echo 0 is gone from both.
• The pass and genuinely-absent arms admit only a literal integer (grep -Eq '^[0-9]+$'); anything else routes to the undetermined arm.
• The undetermined arm is a distinct READ FAILED: … — UNKNOWN, which is neither a pass nor 'no …' line, quoting what the read actually returned via the existing brief() helper (from ship-it's enqueue needs repo auto-merge + a queue ruleset no fresh repo has #4303 / PR feat(doctor): preflight the two merge-queue governance preconditions ship-it needs (#4303) #4346, now merged).
• The fix: on the undetermined arm addresses the read — resolve the repo, check gh auth status, re-run — and explicitly says the line establishes nothing about whether CI exists.

Verdicts are unchanged. 2b still counts toward fails; 3b still never does (all three of its arms are WARN-only). This changes reasons and fixes, not the exit-code contract. doctor.sh stays write-free — every remediation is still a command it prints.

Verification — all three arms, run live

Unreadable repo (CLAUDE_PIPELINE_REPO=kamp-us/definitely-not-a-real-repo-4347), the issue's reproduction. No [: … integer expression expected line anywhere in the output; neither site prints its negative answer:

  ✗ READ FAILED: could not read the workflows of kamp-us/definitely-not-a-real-repo-4347 (gh exit 1,
    returned: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/actions/workflows#list-repository-workflows","s)
    — the workflow count is UNKNOWN, which is neither a pass nor 'no CI workflows'
      ↳ fix: resolve the repo and re-run once `gh auth status` shows a token that can read it —
             this line says NOTHING about whether CI exists, so do not add a workflow on the strength of it
  ⚠ READ FAILED: could not read the workflows of … — whether a run-evidence producer exists is
    UNKNOWN, which is neither a pass nor 'no run-evidence producer'

Exit 1, 4 blocking failures — same count and same direction as before the change.

Present (kamp-us/phoenix): ✓ CI workflows defined (44) / ✓ run-evidence producer present, verdict pipeline-ready, exit 0.

Genuinely absent, readable: octocat/git-consortium (a real repo whose total_count is 0) still prints ✗ no CI workflows defined with the add-a-workflow fix; octocat/Hello-World still prints ⚠ no run-evidence producer. The true-negative arms are intact and reachable.

pnpm lint:worktree is a clean skip (shell-only diff); pnpm typecheck green (29/29).

Check 3a — deliberately left out of scope

Per the acceptance criteria's explicit call: 3a (npm view "$pkg" version >/dev/null 2>&1) is NOT brought in scope. It is the same defect class by a different mechanism — there is no || echo 0 collapse and no value to shape-validate; distinguishing "the registry was unreachable" from "the package does not exist" requires parsing npm's error output, which is a different fix with a different failure mode. It is also WARN-only and touches nothing. Triage measured this issue at exactly two sites, and #4246 / #4247 are the filed guards that mechanize the class repo-wide — 3a is theirs, not this diff's, and folding it in would silently widen a §CP diff beyond the issue.

Control plane

pipeline-cli cp-classify classify → control-plane [path-match] — BLOCKING (human merge). A .sh under claude-plugins/kampus-pipeline/skills/ matches the live CONTROL_PLANE_RE. This PR stops at open; a human control-plane member approves and merges it.

Sequencing

Branched off main after PR #4346 merged, so brief() is reused rather than reinvented and the READ FAILED … UNKNOWN phrasing matches the convention that PR established. PR #4337 (for #4300) is still open and rewrites 1c / appends 3c/3d — no textual overlap with 2b or 3b.

🤖 Generated with Claude Code

[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. _(5281728)

• web — Stage pr-4363 torn down.

[usirin]

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

PR #4363 touches the control plane (cp-classify classify → control-plane [path-match] — a .sh under claude-plugins/kampus-pipeline/skills/ matches the live CONTROL_PLANE_RE) — the agent control plane / pipeline gates (ADR 0053/0065/0073). 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: @ 5281728

Verified against #4347's acceptance criteria + the skill-rigor checklist. Gate contract read from origin/main (not the working tree — the primary checkout was 8 commits behind at dispatch). All checks pass. Verification is empirical, not diff-reading: the head's doctor.sh was extracted read-only via git show <head-ref>:… and executed against three live targets.

Empirical runs — the three arms, at this head

Arm	CLAUDE_PIPELINE_REPO	2b	3b	exit
Unreadable	a nonexistent repo	✗ READ FAILED … the workflow count is UNKNOWN, which is neither a pass nor 'no CI workflows'	⚠ READ FAILED … whether a run-evidence producer exists is UNKNOWN, which is neither a pass nor 'no run-evidence producer'	1, 4 blocking
Genuinely absent, readable	octocat/git-consortium	✗ no CI workflows defined	⚠ no run-evidence producer	1, 4 blocking
Present	this repo	✓ CI workflows defined (44)	✓ run-evidence producer present	0, pipeline-ready

Both true-negative arms remain reachable — the fix narrows the pass/absent arms, it does not swallow them.

Acceptance criteria

• [PASS] 2b reports three outcomes, the third stating the count is UNKNOWN and explicitly not "no workflows" — doctor.sh:122-131; the undetermined line ends — the workflow count is UNKNOWN, which is neither a pass nor 'no CI workflows'. All three arms observed live (table above).
• [PASS] 3b does the same for the run-evidence producer; the unreadable case WARNs as undetermined, never "no run-evidence producer" — doctor.sh:231-238, observed live as ⚠ READ FAILED … is UNKNOWN, which is neither a pass nor 'no run-evidence producer'.
• [PASS] Neither site uses || echo 0; gh's status is captured separately and the pass/absent arms admit only a literal integer — WF_RC=$? (doctor.sh:120), RE_RC=$? (:229), both guarded by [ "$RC" -ne 0 ] || ! printf '%s' "$VAL" | grep -Eq '^[0-9]+$' (:122, :231). A repo-wide grep finds no surviving echo 0 at either site — the only remaining occurrence of the token is inside the explanatory comment at :112. Confirmed by the issue's own reproduction: against a nonexistent repo neither site printed its negative answer.
• [PASS] No [: … integer expression expected line for the unreadable-repo case — the unreadable run was captured with stderr merged (2>&1) and grep -n "integer expression" over the full transcript returns nothing (exit 1, no match).
• [PASS] The fix: on each undetermined arm addresses the read, never the negative remediation — 2b (:124): "resolve the repo and re-run once gh auth status shows a token that can read it — this line says NOTHING about whether CI exists, so do not add a workflow on the strength of it"; 3b (:233): "…ship-it guard 2's mode stays undetermined until this read succeeds". Neither says "add a workflow" or "ship run-evidence.yml"; those strings stay on the true-negative arms only.
• [PASS] Fail direction preserved, exit-code contract unchanged — 2b's undetermined arm carries fails=$((fails + 1)) (:125), so it still counts; 3b's three arms are WARN/PASS/WARN with no fails touch anywhere (:231-238). Proven live rather than asserted: on the unreadable repo the verdict reads "4 blocking check(s) failed", and exactly four ✗ lines are present (1c, 2b, 2c, 2d) — 3b's ⚠ contributed zero. Exit 1 on both failing arms, exit 0 on the present arm.
• [PASS] doctor.sh remains write-free — the diff adds only say / fix / fails=$((fails + 1)) calls; it introduces no mutating gh/git/npm invocation, and every remediation is still a printed command.
• [PASS] The 3a (npm view) scope call is stated explicitly in the PR — a dedicated "Check 3a — deliberately left out of scope" section declares it not brought in scope and argues why (different mechanism, no || echo 0 and no value to shape-validate; WARN-only; Guard: red a call site that discards a producer's exit status — pipe-masked and command-substitution variants #4246/Guard: no verb may seat an ordinary verdict on an exit code that also means it failed to run #4247 are the filed guards that mechanize the class). An explicit call either way is what the criterion asked for, and this is the honest one: 3a is genuinely untouched by the diff.

Scope boundaries — checked for honest statement, not silent widening

• [PASS] Boundary 1 (3a out of scope) — verified untouched. The diff's two hunks are confined to the 2b and 3b blocks; 3a's npm view loop is not in the diff at all.
• [PASS] Boundary 2 (the disclosed sibling defect at 1c) — genuinely left alone, exactly as disclosed. 1c's read is still the unguarded EXISTING=$(gh api "repos/$REPO/labels?per_page=100" --jq '.[].name' 2>/dev/null) (doctor.sh:60), and the live unreadable-repo run reproduces the sibling symptom verbatim: ✗ 15 required pipeline label(s) missing rather than "could not read repo labels", because the non-empty gh error body keeps EXISTING non-empty so the -z branch never fires. That is doctor's required-label set is narrower than what triage needs to home an issue #4300 / PR fix(doctor): cover triage's homing prerequisites, and tell a failed read from an absent one (#4300) #4337's territory and this diff does not touch it — correctly scoped, and honestly disclosed rather than quietly absorbed.

Skill rigor

• [PASS] Behavioral correctness — not inferred from the diff; all three arms executed live against the head-extracted script (table above), including the two the request asked to be re-run independently (unreadable, genuinely-absent). Observed output matches the claimed output. The new guard is strictly narrower than the one it replaces: the pass arm now additionally requires RC == 0 and a literal-integer shape, so no input that previously FAILed can now PASS — the change can only move a verdict toward undetermined, never toward a false green. The unresolved-REPO case (WORKFLOWS="", WF_RC=1 initialized at :116-117) also routes to the undetermined arm and still counts toward fails, preserving the prior direction.
• [PASS] Trigger / description quality — no trigger surface changed. doctor/SKILL.md frontmatter is not in the diff; the change is confined to the executable body of doctor.sh. Checked doctor/SKILL.md on origin/main for wording that would now be stale against the 2b/3b reason lines — it documents neither, so no doc drift is introduced.
• [PASS] Cross-skill conflict / shadowing — doctor.sh is a standalone, write-free preflight; it emits no marker, publishes no seam another skill parses, and this diff changes no shape a consumer reads. The new phrasing deliberately reuses the in-file READ FAILED: … — UNKNOWN, which is neither a pass nor '…' convention and the existing brief() helper established by the merged ship-it's enqueue needs repo auto-merge + a queue ruleset no fresh repo has #4303 / PR feat(doctor): preflight the two merge-queue governance preconditions ship-it needs (#4303) #4346, so the file converges on one wording rather than adding a third.
• [PASS] Gate-invariant preservation — no gate invariant is in this diff's reach. doctor.sh is an advisory adopter preflight, not a merge gate: it does not participate in ship-it's §CP refusal, the checks-green gate, any verdict namespace, or the §CP path set. Walked the invariants the §CP set protects and none is touched. The one contract that could have been weakened — the script's own exit-code contract — is explicitly preserved and was proven live (2b counts toward fails, 3b never does; exit 1/1/0 across the three arms). Direction of the change is fail-closed, consistent with PROBES.md (a probe that cannot itself execute must resolve to "unknown", never "down") and ADR 0092 §1 (emit what it scanned).

Note, non-blocking (not a defect in this PR): the PR body says "PR #4337 (for #4300) is still open" — it has since merged and is now the tip of main. That is body staleness, not a code defect; 2b/3b remain textually non-overlapping with 1c/3c/3d, so no conflict is implied. A rebase onto current main before enqueue is the normal merge-queue path.

All checks pass. review-skill does not merge, and this §CP verdict authorizes nothing: a @kamp-us/control-plane approval at head 528172800e78bee37bf90f5b190b79e4e7cc26b1 is what lets ship-it enqueue it (ADR 0135/0048). Merging will auto-close #4347 via Fixes #4347.

Verdict-written: 2026-07-27T02:40:01Z

[usirin]

Banked for control-plane approval

@cansirin — this PR is §CP by path match (claude-plugins/kampus-pipeline/skills/doctor/doctor.sh), so under ADR 0135 it needs a @kamp-us/control-plane approval at the current head before the pipeline may enqueue it. The human owns the judgment; the pipeline owns the mechanics — no hand-merge, the approval-aware shipper does the enqueue once your approval lands.

Review verdict: PASS — advisory form per ADR 0111/0151, bound to head 528172800e78bee37bf90f5b190b79e4e7cc26b1, posted as comment id 5086740403.

What the reviewer actually did

Not a diff read — all three arms were verified empirically by running the guard:

• an unreadable repo,
• a genuinely-absent-but-readable repo,
• a present repo.

The exit-code contract was proven live: on the unreadable arm, exactly four blocking ✗ lines, with check 3b's warning contributing zero.

The load-bearing safety property

The new guard is strictly narrower than what it replaces — the pass arm now additionally requires a zero exit status and a literal-integer shape. So no input that previously failed can now pass: the change can only move a result toward "undetermined", never toward a false green.

Known staleness (non-blocking)

The PR body says PR #4337 is still open. It has since merged and is now at the tip of main. The 2b / 3b hunks remain textually non-overlapping with 1c, so there is no conflict — a rebase before enqueue is the normal merge-queue path.

If the head moves for any reason, an approval given at the old head is dismissed (ADR 0058) and a fresh approval at the new head is required.