fix(review): degrade gracefully when CI check status is unavailable (MNG-1750)

[aaight]

Summary

Fixes MNG-1750 — the review agent boot no longer dies with BootFailureError: plan resolution failed when the reviewer credential is a fine-grained PAT lacking the Actions: Read permission.

Previously, fetchPRContextStep called githubClient.getCheckSuiteStatus(...) unconditionally. A reviewer PAT without Actions: Read makes the GitHub Actions API return a 403 (Resource not accessible by personal access token), which propagated up through the Phase-2 context pipeline (profiles.ts does not wrap steps in try/catch) and got re-wrapped as a BootFailureError. Because boot failure precedes log persistence, this surfaced as a "silent 22s crash" with a NULL cascade_log.

What changed

1. New formatCheckStatusUnavailable formatter — src/gadgets/github/core/getPRChecks.ts

An exported helper next to the existing formatCheckStatus. Produces a message that is visibly distinct from formatCheckStatus's No CI checks configured string, so the agent can tell "CASCADE tried to read CI status and failed" apart from "there is nothing to check". Includes the Actions: Read permission hint and takes a plain string (never a raw Octokit RequestError, which can carry the Authorization header).

2. Non-fatal check-status fetch — src/agents/definitions/contextSteps.ts

Wrapped only the getCheckSuiteStatus + formatCheckStatus calls in a tight try/catch. On failure:

• Logs WARN CI check status unavailable with the sanitized error.message (never the raw error object).
• Injects the formatCheckStatusUnavailable text as the GetPRChecks result and flips the description to CI check status unavailable.

getPR and getPRDiff remain outside the try/catch — they stay fatal, because a review without the PR itself is meaningless. The GetPRChecks injection is still pushed unconditionally; on the success path the result, params, and description are byte-identical to before.

3. Docs — CLAUDE.md (and AGENTS.md, a symlink to it)

Added one paragraph to the "Review agent — context shape (debugging)" section documenting the informational-not-fatal CI-status behavior. AGENTS.md is a symlink to CLAUDE.md, so both stay byte-identical (guarded by tests/unit/architecture-docs.test.ts).

Tests

• tests/unit/agents/definitions/contextSteps.test.ts — new MNG-1750 — graceful CI check-status degradation block:
  • 403 from getCheckSuiteStatus → boot proceeds + UNAVAILABLE injection carrying the upstream message + Actions: Read hint + description === 'CI check status unavailable'.
  • WARN log carries the upstream error message.
  • Success path → GetPRChecks result contains PR #1092 Check Status: 1/1, description unchanged (Pre-fetched CI check status).
  • getPR failure still throws (PR not found).
  • getPRDiff failure still throws (diff unavailable) — locks the "diff is fatal" invariant.
• tests/unit/gadgets/github/core/getPRChecks.test.ts — focused formatCheckStatusUnavailable tests: contains the error message, the Actions: Read hint, the PR number, is distinct from No CI checks configured (via the UNAVAILABLE marker), and never emits [object Object].

Verification

• npx vitest run tests/unit/agents/definitions/contextSteps.test.ts tests/unit/gadgets/github/core/getPRChecks.test.ts → 54 passed
• npx vitest run tests/unit/agents/ tests/unit/gadgets/github/ → 1038 passed (no adjacent regressions)
• npx vitest run tests/unit/architecture-docs.test.ts → 41 passed (docs sync guardrail)
• npm run typecheck → clean
• npx biome check on changed files → clean

Out of scope (unchanged)

Router-side getCheckSuiteStatus callers gate dispatch decisions and deliberately fail closed (check-suite-success, check-suite-failure, pr-ready-to-merge, post-completion-review). The mid-run getPRChecks gadget already returns errors through its structured envelope. None of those were touched.

🤖 Generated with Claude Code

🕵️ claude-code · claude-opus-4-8 · run details

[codecov]

Codecov Report

❌ Patch coverage is 97.05882% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/agents/definitions/contextSteps.ts	94.44%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[nhopeatall]

Summary

APPROVE — a correct, tightly-scoped fix that makes CI check-status fetch non-fatal during review-agent boot. I traced the root cause and verified the fix end to end.

Verification performed

• Root cause confirmed. Phase 2 of the context pipeline (profiles.ts:247-253) does not wrap trigger-pipeline steps in try/catch, so a 403 thrown by getCheckSuiteStatus inside fetchPRContextStep propagates and is re-wrapped as BootFailureError. The prContext → fetchPRContextStep mapping (strategies.ts:36) and getCheckSuiteStatus's use of the Actions API (github/client.ts:358, which a fine-grained PAT without Actions: Read rejects) both check out.
• Scope is correct. Only getCheckSuiteStatus + formatCheckStatus are inside the try/catch; getPR and getPRDiff remain fatal.
• Secret hygiene. Only error.message is logged/injected (error instanceof Error ? error.message : String(error)), never the raw Octokit RequestError; formatCheckStatusUnavailable takes a plain string. Consistent with the rest of contextSteps.ts.
• Success path unchanged, checkStatusFormatted/checkStatusDescription are definitely-assigned in both branches.
• "Retry" guidance is accurate — the review agent has scm:read, which provides the GetPRChecks gadget for mid-run re-fetch.
• Strict improvement across all 7 prContext consumers (review, respond-to-ci/review/pr-comment, resolve-conflicts): a getCheckSuiteStatus error previously crashed boot for all of them; now it degrades. No regression — respond-to-ci gets its real signal from GetCIRunLogs, not the boot summary.
• Tests — ran the two touched files locally: 54 passed. Docs guard is satisfied (AGENTS.md is a symlink to CLAUDE.md, so both stay byte-identical).

Minor (non-blocking, optional)

• formatCheckStatusUnavailable presents the "Probable cause: ... 403 ... Actions: Read" explanation for every caught error, but the catch is broad (any failure — transient network blip, 429, 500 — not just 403). The message is hedged as "Probable cause" and always includes the real Upstream error: <message>, and the agent can re-fetch via GetPRChecks, so this is cosmetic — the 403/PAT case is the overwhelmingly common one. No change required.

🕵️ claude-code · claude-opus-4-8 · run details