fix(automerge): grant checks:read + harden Codex bypass against API errors#27
Merged
Merged
Conversation
…rrors Two follow-ups to PR #25's Option B Codex bypass, both surfaced by the first install of the new reusable on whois-api-llc/wxa-mcp-server#80. 1. **Permission gap.** The Option B step queries /repos/{repo}/commits/{sha}/check-runs to read Codex Review status. The default GITHUB_TOKEN doesn't carry `checks: read`, so the call returns 403 "Resource not accessible by integration" → jq tries to iterate over null → workflow crashes → bypass step exit code 5 → `automerge / automerge` shows red. Add `checks: read` to the reusable's permissions block. 2. **Crash hardening.** Even with the right permission, transient issues happen: 404 on commits that haven't propagated yet, network blips, future API contract changes. Wrap each gh-api call in `|| echo '{}'` and use `jq 'try (...) // empty'` so the step ALWAYS sets bypass=0 (never crashes) when the API returns invalid JSON. This preserves the principle that the bypass should be additive: if it can't determine Codex passed, it falls through to the existing "blocked comment" path. Today's behaviour was a hard crash, which skipped the blocked comment AND failed the workflow check itself. Both fixes mirror the discipline pattern from the wxa_webcat 2026-04-27 incident memo: "Strict-by-default with explicit escape hatches" — but the escape hatch on a feature flag should always be `feature off`, never `crash everything`. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
topcoder1
enabled auto-merge (squash)
May 3, 2026 22:31
|
No issues found. Both fixes are correct — |
2 tasks
topcoder1
added a commit
that referenced
this pull request
May 3, 2026
#28) Hot-fix to [#27](#27). The `checks: read` permission I added to the reusable's job-level block creates a cap-mismatch with existing fleet callers — they grant only `contents+pull-requests` at the workflow level, and GitHub fails the workflow at `startup_failure` before any step runs. Caught on [whois-api-llc/wxa-mcp-server#80, run 25292746179](https://github.com/whois-api-llc/wxa-mcp-server/actions/runs/25292746179): the auto-merge install PR's first run after the fix landed showed empty job list + `startup_failure` conclusion. The signature of a permissions-cap conflict at workflow start. Without this revert, every existing Claude PR on every fleet repo (44+ callers) would startup_failure on its next run. Bad outcome. ## Fix - Drop `checks: read` from the reusable's job-level permissions. - Keep the graceful 403 handling in the Option B step from #27 (`gh api ... || echo '{}'` + `jq 'try (...) // empty'`). Without checks:read, the step gracefully returns `bypass=0` — Option A (label bypass) and the no-risky-paths fast-path still work. - Update the caller template at `~/.claude/templates/ci-workflows/callers/claude-author-automerge.yml` to grant `checks: read`. New fleet installs get Option B by default. ## Migration for existing 44 callers Existing callers continue to work after this lands. To opt in to Option B, they add a one-line `checks: read` to their workflow-level permissions block. I'll ship that as a fleet-wide migration in a separate PR (probably scripted via `gh api` for batch). ## Test plan - [x] `actionlint` clean. - [ ] Post-merge: re-trigger wxa-mcp-server#80 → expect `Claude-author Auto-merge` to NOT startup_failure (it'll go through the normal risk-tier check, find `.github/workflows/**` is a risk path, post the blocked comment as designed). **Auto-merge rationale:** **NOT eligible** — touches `.github/workflows/**`. Manual click-merge. **Codex pre-review:** skipped — straight revert + comment update. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
topcoder1
added a commit
that referenced
this pull request
Jul 17, 2026
…base-branch risk-paths.yml at decision time (#121) ## Problem — fleet-wide TOCTOU race between pr-classify and claude-author-automerge Evidence (whois-api-llc/wxa-secrets#27, 2026-07-17 UTC): both reusables fired on the same `ready_for_review` event at 00:40:37. The automerge run queried live labels at 00:40:43 (none yet), armed auto-merge at 00:40:46; pr-classify applied `risk:blocked` at 00:40:50 — 4 seconds late — and the PR merged at 00:42:30 when Claude Review went green, despite `src/wxa_secrets/**` being in the repo's `blocked:` list. The documented mitigation ("caller wires `labeled` into trigger types so a re-run revokes") is structurally unreachable: pr-classify labels with `GITHUB_TOKEN`, and GITHUB_TOKEN-attributed events never trigger workflows. Every repo-specific `blocked:`/`sensitive:` path not covered by the central regex had this race. ## Fix — compute the classifier verdict at decision time, race-free The `classifier_verdict` step no longer depends on the label round-trip: 1. **Belt-and-suspenders**: a live `risk:blocked`/`risk:sensitive` label still gates (hand-applied labels are an independent signal). The label read now paginates and **fails closed** after 3 retries — it can no longer silently degrade to "no label". 2. **Authoritative**: the step reads `.github/risk-paths.yml` from the PR's **base branch** (a PR cannot edit its own risk file to escape; on base-ref 404 it falls back to the **default branch's** file so legacy/release bases can't dodge policy; absent on both = policy-free repo, global regex still gates) and classifies the changed files with the **same `classify.mjs` pr-classify fetches** — one matcher, zero glob-semantics drift, nocase/bracket/negation guards inherited. `blocked`/`sensitive` ⇒ refuse + revoke + sticky comment. Fail-closed everywhere it matters: non-404 rules read, failed label read, failed file listing, ≥3000-file truncation, classifier parse/guard errors, and **output outside the class enum** all refuse to arm. A terminal `always()` step revokes any *pre-existing* arm when a gating **or prerequisite** (checkout/detect/setup-node) step errors — a red run must not leave a stale arm alive — with a head-ownership guard so a stale run never disarms a newer head's valid arm. Arming itself is bound to the event head SHA (`--match-head-commit` → GraphQL `expectedHeadOid`, verified live), so a mid-run push can never inherit an arm decided on older files. No `checks: read` required anywhere (wxa-secrets deliberately omits it). No caller changes needed; the reusable floats `@main`, so this lands fleet-wide on merge. Cost: ~10–20s per Claude-PR run (setup-node + npm + classify). Alternatives rejected: polling for the classify check needs `checks:read` in every caller; PAT-labeling still leaves an arm→revoke window that loses when required checks are already green. ## Selftest (`selftest/test_automerge_riskfile_gate.sh`, CI-enforced via pytest) Extracts the shipped step from the YAML and executes it against a stubbed `gh` + the real `classify.mjs`: the #27 race shape, sensitive, clean, label-only, base-ref pinning (every case; incl. non-default base), default-branch fallback, 404×2 fall-through, fail-closed on API error / 3000-file cap / unreadable labels (blocked **and** clean files) / enum violation, nocase parity. Structural pins: shared classifier source + dep pins in both workflows, `--match-head-commit`, `always()` error-revoke wiring incl. prerequisites + head guard, label pagination. ## Verification - `actionlint`: clean (both workflows) - `bash selftest/test_automerge_riskfile_gate.sh`: 21/21 - `uv run pytest -q`: 9 passed (new selftest registered in `test_workflow_guards.py`) - `gh pr merge --auto --match-head-commit <wrong-sha>` probed live: rejected atomically, combo confirmed ## Codex rounds: 6 (hard cap) r1 clean → r2 3 findings (2 fixed: head-SHA binding, 404 fallback + enum guard; pin/integrity sub-point rejected: one-sided pinning would reintroduce matcher drift vs pr-classify) → r3 1 fixed (fail-closed label read) → r4 3 (fixed: error-revoke step, label pagination; rejected: revoke-on-head-moved in enable path — would strand clean PRs on rapid fixup pushes; documented instead) → r5 1 fixed (prerequisite failures skip gates → covered) → r6 0 P1 / 2 P2 (fixed: head-ownership guard on error-revoke; **deferred**: stale *automated* label can strand a de-risked PR until the next event — pure liveness, pre-existing, self-heals; clean fix is distinguishing bot- vs hand-applied labels via timeline actor, follow-up). Known residuals (documented in-file): post-arm blocked push relies on its synchronize run's revoke (~1 min), which loses only if every required check beats it — not a real shape in this fleet; trailer-only Claude PRs with a failed checkout aren't covered by the error-revoke authorship fallback. Related follow-up chip spawned: the manual-hold step's own fail-open label read (pre-existing, same class). wxa-secrets already shipped a repo-side race-proof gate (CODEOWNERS + require_code_owner_review, whois-api-llc/wxa-secrets#29); this central fix covers the rest of the fleet. Auto-merge rationale: manual merge — production infrastructure (`.github/workflows/**`), and PRs to topcoder1/ci-workflows are always manual. Opened as draft per fleet policy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-ups to #25's Option B Codex bypass. Both surfaced when the new reusable ran for the first time on whois-api-llc/wxa-mcp-server#80 — the auto-merge install PR for that repo. Output:
Fix 1 —
checks: readpermission. The Option B step queries/repos/{repo}/commits/{sha}/check-runsto read Codex Review status. The defaultGITHUB_TOKENonpull_requestevents does NOT carry that permission by default, so the call returns 403 → jq fails on null → workflow crashes.Fix 2 — crash hardening. Even with the right permission, transient issues happen (404 on un-propagated commits, network blips, future API contract changes). Wrap each
gh apicall in|| echo '{}'and usejq 'try (...) // empty'so the step ALWAYS setsbypass=0when the API returns invalid JSON. The bypass remains strictly additive: if it can't prove Codex passed, the existing 'blocked comment' path still runs.Today's behaviour was a hard crash that ALSO skipped the blocked comment, leaving the user with a red 'automerge / automerge' check and no helpful message. After this fix: API failures degrade to 'no bypass, post the blocked comment as usual'.
Test plan
actionlintclean.automerge / automergesucceeds with bypass=0 (no Codex on this repo's pre-existing PRs since they're already merged) and the existing blocked-comment posts.Auto-merge rationale: NOT eligible — touches
.github/workflows/**. Manual click-merge.Codex pre-review: skipped — straightforward fix; the diff is small enough that Codex would add latency without value.
🤖 Generated with Claude Code