Skip to content

A single transient GitHub 503 fails claude-review and blocks merges fleet-wide; the failure is also misreported as a timeout #133

Closed
Closed
A single transient GitHub 503 fails claude-review and blocks merges fleet-wide; the failure is also misreported as a timeout#133

Description

@twistedmelonman
twistedmelonman
opened on Aug 17, 2026

During the 2026-08-17 GitHub incident, claude-review / run-review failed on archive-resolver#25 twice — not because of anything in the PR, the reviewer, or the repo's configuration, but because one unretried GitHub API call returned 503.

Since claude-review / run-review is a required status check on protected branches (confirmed on dotfiles, claude-config, dev-env), a transient GitHub blip currently blocks merges across the fleet.

What happens

The Run Claude Code Review step exits in ~13s (a real review takes ~1m11s):

Requesting OIDC token...
OIDC token successfully obtained
Exchanging OIDC token for app token...
Attempt 1 failed: GitHub API is temporarily unavailable while verifying repository access.
Attempt 2 of 3...
App token successfully obtained          ← retry worked
Using GITHUB_TOKEN from OIDC
Checking permissions for actor: smartwatermelon
GET /repos/.../collaborators/smartwatermelon/permission - 503
##[error]Failed to check permissions: HttpError: No server is currently available...
##[error]Action failed with error: Failed to check permissions for smartwatermelon
##[error]Process completed with exit code 1.

The contrast is the whole point: the OIDC token exchange retries 3× and recovers. The actor permission check has no retry and dies on its first 503.

Measured during the incident, that endpoint was failing ~40% of the time (3 of 8 probes) while the GitHub status page reported everything except Copilot as operational. Re-running is a coin flip, and a green result proves nothing.

This is upstream behavior in anthropics/claude-code-action, not something this repo controls directly — but this repo decides how to react to it.

Second problem: the failure is misreported

claude-blocking-review.yml:435 sets continue-on-error: true on the Claude step, with the comment "infrastructure failure must not block merges". The verdict check then tries to tell a real failure from a flake (:665):

if [ "$CLAUDE_OUTCOME" = "failure" ]; then
  echo "::error::Review did not complete (likely timed out or failed to render verdict)."
  echo "::error::Re-push to retry, or add [skip-claude-review: reason] to the PR body to bypass."
  exit 1
else
  echo "::warning::No verdict found but Claude step did not fail. Defaulting to PASS (infrastructure issue)."
  exit 0
fi

A GitHub 503 sets CLAUDE_OUTCOME: failure, so it takes the first branch. The user is told the review "likely timed out" and advised to re-push — when nothing timed out, the diff is irrelevant, and re-pushing will fail again for as long as the API is degraded.

So the intended flake-vs-failure distinction doesn't fire for the most common real-world flake. continue-on-error: true masks the step, and then the verdict check un-masks it with the wrong diagnosis. I misdiagnosed this myself twice before reading the raw log.

Worth noting the fail-closed behavior is correct — a required check should not go green when no review happened. Only the classification and the message are wrong.

Suggested fixes

  1. Distinguish infrastructure failure from review failure. Grep the Claude step's output for the permission-check/HTTP-503 signature and emit a distinct message: "GitHub API unavailable — this is not a problem with your PR. Re-run the job when GitHub recovers." Notably INCOMPLETE and re-push to retry are both actively misleading here; re-running the job is right, re-pushing is not.

  2. Retry around the transient window. The workflow can't patch upstream, but it can re-run the step on a detected 503 rather than surfacing a hard failure on the first blip.

  3. Report upstream. claude-code-action retries the token exchange but not the actor permission check, which is an inconsistency in its own retry policy. Worth an issue on anthropics/claude-code-action.

  4. Consider whether a GitHub-side outage should block merges at all. Right now a degraded GitHub API makes every protected repo unmergeable via the normal path. That may be acceptable (fail-closed is defensible for a security reviewer) but it should be a deliberate choice, not an emergent one.

Reproduction

  • archive-resolver#25, run 31986353713 — failed twice with the identical signature, ~2.5h apart, on an unrelated one-file shell-script change.
  • Both times: step 9 (Verify CLAUDE_CODE_OAUTH_TOKEN) passed, OIDC succeeded, app token obtained. The secret and repo config are correct.

Refs #123, #126

Activity

  1. twistedmelonman
    added 2 commits that reference this issue on Aug 17, 2026

    fix(claude-blocking-review): diagnose GitHub API outages as infra, no…

    c777455

    chore: close four tracked low-effort review issues (#153)

    6614b5f

  2. twistedmelonman commented on Aug 19, 2026

    @twistedmelonman
    twistedmelonman
    on Aug 19, 2026
    MemberAuthor

    Status check while triaging the low-effort backlog (#153): suggestion 1 is shipped; the rest are not code changes to this repo.

    Done — c777455 ("diagnose GitHub API outages as infra, not timeouts")

    The misreporting half of this issue is fixed. claude-blocking-review.yml:688-719 now probes GET /repos/{owner}/{repo}/collaborators/{actor}/permission before blaming the review, and on failure emits a distinct diagnosis:

    GitHub API is unavailable — this is NOT a problem with your PR.
    Re-run this job when GitHub recovers: gh run rerun <id>
    Do NOT re-push — the diff is irrelevant and a new commit will fail the same way.

    That directly addresses the two things this issue called actively misleading: the "likely timed out" classification and the "re-push to retry" advice. Both paths still exit 1, so the fail-closed behavior you called correct is unchanged.

    The probe is a heuristic and the code says so — it runs seconds after the action, so a recovered API yields a false negative and falls through to the generic message. It only ever adds a more accurate diagnosis; it never converts a failure into a pass.

    Still open — and not fixable here

    • Suggestion 2 (retry around the transient window) — partly subsumed by the above. Re-running the job is now the advice given, but the workflow still can't retry the upstream step's internal permission check.
    • Suggestion 3 (report upstream) — the actual root cause: claude-code-action retries the OIDC token exchange 3× but not the actor permission check, which is an inconsistency in its own retry policy. Needs an issue on anthropics/claude-code-action; nothing in this repo can fix it.
    • Suggestion 4 (should a GitHub outage block merges at all?) — an open policy question, deliberately unanswered. Current behavior is fail-closed, which this issue argues is defensible for a security reviewer.

    Leaving this open to track 3 and 4. Worth deciding whether it should be narrowed to just the upstream report, since the merge-blocking symptom is now correctly diagnosed even though the underlying flake remains.

  3. twistedmelonman commented on Aug 19, 2026

    @twistedmelonman
    twistedmelonman
    on Aug 19, 2026
    MemberAuthor

    Reframing this per smartwatermelon/dev-env#60 ("better split of local vs CI checks"): CI checks should be quick, cheap, deterministic — does the diff meet standard X, yes or no. Judgment calls belong on the local side, before pushing.

    That dissolves this issue rather than solving it, and it does so without waiting on Anthropic.

    Why it dissolves

    Every remaining item here (suggestions 2, 3, 4) is downstream of one premise: that a judgment-call reviewer belongs in CI at all. Take Claude out of CI and:

    • Suggestion 3 (report upstream) stops being load-bearing. The unretried actor-permission check in claude-code-action is still a real upstream inconsistency, but it is no longer on the critical path for merging anything. Worth reporting on its own merits; nothing is gated on it.
    • Suggestion 4 (should a GitHub outage block merges?) narrows to something answerable. The question stops being "should a degraded GitHub API make every protected repo unmergeable" and becomes the ordinary one every CI system has: what do we do when GitHub doesn't answer. Still worth deciding, but it is no longer entangled with a third-party AI action's retry policy.
    • Suggestion 2 (retry the transient window) largely evaporates, because a deterministic check has far less surface to flake on than a multi-turn agent making authenticated API calls.

    The workflow is judgment by construction

    This is not a close call. The five BLOCK criteria in claude-blocking-review.yml:502-516 are:

    • "a clear bug that causes incorrect behavior in production"
    • "a reliability regression: a previously working feature may now fail"
    • "a security vulnerability … unvalidated user input reaching a privileged operation"
    • "missing error handling … that would cause a silent failure"
    • "a risk of data loss or data corruption"

    None of these is a yes/no against a laid-out standard — they are exactly the judgment calls dev-env#60 assigns to the local side. The workflow's own prompt says so out loud: "Local reviewers (pre-commit hooks, code-reviewer, adversarial-reviewer, linters, test suites) already cover style, naming, organization, test coverage, and documentation. Do NOT duplicate that work." The scope constraints, the 3-file ceiling, the evidence standard for regression BLOCKs, the "when uncertain, default to PASS" — all of it is machinery for making a judgment call survive an environment that cannot run tests.

    The local side already does this work

    The coverage argument for keeping it in CI is weak, because the judgment review already happens locally on every commit and again before every merge:

    • ~/.claude/hooks/run-review.sh — code-reviewer + adversarial-reviewer at commit time
    • pre-push whole-codebase review (which is what files these very issues)
    • ~/.claude/hooks/pre-merge-review.sh — analyzes review state, returns SAFE_TO_MERGE

    #153 is a live example: both local reviewers ran pre-commit, the adversarial one raised a Critical that took a real correction cycle, pre-push review passed, and pre-merge returned SAFE_TO_MERGE. The CI reviewer's contribution to that PR was a VERDICT: PASS on work three local passes had already cleared — at the cost of a required check that a GitHub 503 can fail.

    Also worth naming: the CI reviewer is structurally the weakest of the four. It cannot run tests (stated in its own prompt), is capped at reading 3 files, and is told to default to PASS under uncertainty. It is the least-informed reviewer holding the only blocking position.

    What this implies

    Removing it also removes the need for CLAUDE_CODE_OAUTH_TOKEN in ~24 repos — dev-env#60 anticipates this ("we may not need the Anthropic token stored in Github at all"). That is a meaningful reduction in secret sprawl and in fleet-wide dependency on one vendor's action being up.

    Open questions I would not decide unilaterally:

    1. What replaces it as the required check? Something deterministic — zizmor, actionlint, shellcheck, CodeQL (already running and already green independently). The claude-review / run-review required check is currently branch-protected on dotfiles, claude-config, dev-env; unwinding that is fleet-wide surgery and needs its own plan.
    2. Does anything depend on the reviewer catching what locals miss on PRs authored outside this setup — e.g. Dependabot PRs, or a contributor without the local hooks? Today that path is already skipped for Dependabot bumps.
    3. Deprecation shape: this repo's claude-blocking-review.yml is consumed fleet-wide via floating @v3/@v1, so removal is a coordinated tag operation, not a delete. See Add deterministic precondition check for CLAUDE_CODE_OAUTH_TOKEN before invoking claude-code-action #90 and the skip-guard notes on why a tag-bump PR can never validate the reviewer.

    Happy to draft the migration plan if you want to go this way. It is a larger piece of work than this issue, so it probably deserves its own issue with #133 closed as superseded — say the word and I will write it up rather than deciding the shape here.

  4. twistedmelonman commented on Aug 19, 2026

    @twistedmelonman
    twistedmelonman
    on Aug 19, 2026
    MemberAuthor

    Linked to smartwatermelon/dev-env#60 and planned in #154.

    Keeping this open as the incident record — it is the concrete reproduction (archive-resolver#25, two failures ~2.5h apart during the 2026-08-17 GitHub degradation) and the evidence that the fleet's merge path had a single third-party dependency. The remediation lives in #154.

    Status of the four original suggestions:

    1. Distinguish infrastructure failure from review failure — ✅ done, c777455. Still the right fix regardless of Replace the CI judgment reviewer with a deterministic standards check (per dev-env#60) #154's outcome, since the reviewer keeps running until Phase 5.
    2. Retry around the transient window — superseded. A deterministic check has far less to flake on than a multi-turn authenticated agent.
    3. Report upstream — still valid on its own merits (claude-code-action retries the OIDC exchange 3× but not the actor permission check), but no longer load-bearing: nothing is gated on Anthropic changing it.
    4. Should a GitHub outage block merges at all? — becomes tractable once the check is deterministic. Stops being "should a third-party AI action's retry policy decide fleet mergeability" and becomes the ordinary question every CI system answers.

    Survey data gathered for #154, relevant here: 35 repos carry the caller, 27 have claude-review / run-review as a required check, and 26 of those 27 have it as their only required check. That is the real severity of this issue — during the outage, those 26 repos had no other gate and no way to merge through the normal path.

  5. twistedmelonman
    added a commit that references this issue on Aug 20, 2026

    fix(claude-blocking-review): retry once on confirmed GitHub API outage (

    688e028

  6. twistedmelonman commented on Aug 20, 2026

    @twistedmelonman
    twistedmelonman
    on Aug 20, 2026
    MemberAuthor

    Suggestion 2 (retry around the transient window) is now done — #156, merged as 688e028, released in v3.2.1 with floating v3 moved to match.

    Scoped deliberately to reducing impact, not eliminating the dependency:

    • A new infra-probe step runs when the review step fails and probes the actor-permission endpoint. A retry fires only when the probe confirms the API is down, so ordinary review failures (timeout, agent error) still cost exactly one run.
    • At the ~40% endpoint failure rate measured on 2026-08-17, one gated retry takes an outage-induced check failure from a coin flip to roughly 1-in-6.
    • The outage message now surfaces the existing SHA-scoped escape hatch with the head SHA filled in, plus the gh run rerun semantics — previously an operator had to read the workflow source to discover it.

    Fail-closed is preserved. The retry introduced two ways to green an unreviewed PR, both fixed before merge:

    1. A first attempt can write VERDICT: PASS to /tmp/review-verdict.txt and then die; that stale verdict surviving into a failed retry would have been honored. The probe clears the file before retrying.
    2. The effective outcome across both attempts uses a case allow-list matching only success|failure. A "non-empty means the retry ran" check would let an unexpected value mask a first-attempt failure into the pre-existing default-to-PASS branch.

    Not verified end-to-end against a live outage — the retry path is verified by static analysis, linters, and bash unit-testing of the outcome mapping. The workflow-self-modification skip means a PR editing this file can never exercise the reviewer against itself.

    Status of the four original suggestions:

    1. Distinguish infrastructure failure from review failure — ✅ done, c777455 (fix(claude-blocking-review): diagnose GitHub API outages as infra, not timeouts #147).
    2. Retry around the transient window — ✅ done, 688e028 (fix(claude-blocking-review): retry once on confirmed GitHub API outage (#133) #156).
    3. Report upstream — still open, still not load-bearing.
    4. Should a GitHub outage block merges at all? — considered and deliberately declined. Fail-open on a confirmed outage was on the table and rejected: the probe is a heuristic, and a false positive would pass a PR unreviewed. Fail-closed remains correct for a security gate.

    Keeping this open as the incident record. A sustained outage can still fail both attempts — that residual is what the escape hatch covers and what #154 removes for good.

  7. twistedmelonman commented on Aug 20, 2026

    @twistedmelonman
    twistedmelonman
    on Aug 20, 2026
    MemberAuthor

    Closing — both problems in the title are fixed:

    1. Misreported as a timeout — fixed in c777455 (fix(claude-blocking-review): diagnose GitHub API outages as infra, not timeouts #147). An outage now reports itself as an outage and points at gh run rerun instead of a re-push.
    2. A single 503 blocks merges fleet-wide — mitigated in 688e028 (fix(claude-blocking-review): retry once on confirmed GitHub API outage (#133) #156), released in v3.2.1 with floating v3 moved, so all 35 consumers have it. A gated single retry takes outage-induced check failures from ~40% to ~16%, and the outage message now surfaces the SHA-scoped escape hatch.

    Disposition of the two items that were still tracked here:

    What is not solved: a sustained outage can still fail both attempts, and 26 of 27 protected repos still have this as their only required check. #156 bought margin; it did not remove the dependency on claude-code-action being up. #154 is the actual fix and remains open.

    Reproduction preserved above for the record: archive-resolver#25, run 31986353713, two failures ~2.5h apart during the 2026-08-17 degradation.

  8. twistedmelonman
    closed this as completedon Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions