fix(classify): drop false-red exit-1 on blocked class#21
Merged
Conversation
The "Block merge for blocked class" step in pr-classify.yml has been exiting 1 on any PR that touches blocked paths (.github/workflows/**, Dockerfiles, secrets, infra, etc.). This makes both the PR Classify check AND the PR Codex Review check (which uses the same reusable for its classify job) report failure on the PR UI. But the exit-1 was never doing real work: caller repos' branch-protection rulesets only require the actual CI checks (Claude Review / build / lint), not the classify check. The auto-merge gate that *actually* prevents risky-PR auto-merge lives in claude-author-automerge.yml — it reads the class output and the path regex, refuses to enable auto-merge, and posts a sticky comment instead. That gate works without failing CI. So the exit-1 is pure visual noise: - It doesn't block merge (branch protection doesn't require this check). - It conflates "code is broken" with "policy says manual merge". - Maintainers click merge anyway, alarm-fatigued by the red dot. Fix: replace exit-1 with a ::warning:: annotation plus a sticky comment on the PR explaining the policy. The classify job exits 0; the risk:blocked label is still set; downstream gates still consume the class output. Verified on topcoder1/webcrawl PR #124: branch-protection ruleset required-checks = [Claude Review, build, lint] (gh api repos/.../rulesets); no classify check is required. Auto-merge gating happens in claude-author-automerge.yml's own risk-tier path regex (which already matched .github/workflows/** before this change).
|
No issues found. Diff is small and contained: removes |
4 tasks
topcoder1
added a commit
that referenced
this pull request
May 5, 2026
) ## Summary Mirrors the \`automerge_pat\` pattern already present in [\`claude-author-automerge.yml\`](.github/workflows/claude-author-automerge.yml#L51) (added in #21, verified on \`whois-api-llc/wxa_webcat#201\`). Without this, callers that wire \`secrets: inherit\` get the PAT upgrade for the Claude-author lane but not for safe-paths — every safe-paths auto-merge still pushes as \`GITHUB_TOKEN\` and GitHub suppresses downstream push workflows. ## Why [whois-api-llc/wxa_vpn#251](whois-api-llc/wxa_vpn#251) — three consecutive safe-paths auto-merges (wxa_vpn#247, #249, #250) merged via the bot identity \`app/github-actions\` and produced **zero** push-on-main workflow runs (no Lint, no Tests, no Build & Deploy). #249 in particular was a fix for a 25-hour production outage; the kill-after fix sat in main for ~3 hours undeployed until a human kicked \`workflow_dispatch\` manually. #248 in the same window was human-merged and fired the full CI chain — confirming the GITHUB_TOKEN loop-prevention is the load-bearing variable. ## Behavior change - **PAT wired in caller**: merge commit is attributed to the PAT identity → downstream push-on-main workflows fire normally. \`USING_PAT=1\` log line. - **PAT NOT wired**: falls back to \`GITHUB_TOKEN\` (current behavior) and emits a \`::warning::\` line so the silenced-deploy condition is visible in the run log instead of failing silently. ## Auto-merge rationale **Manual click-merge** — touches \`.github/workflows/\` (CI infra). The reusable affects every fleet caller, so reviewer should confirm the conditional fallback looks right before shipping. ## Codex pre-review PASS — single-file YAML change, mirrors a verified pattern, adds an opt-in path with safe fallback. The expression \`secrets.automerge_pat != '' && '1' || '0'\` is the same idiom used in claude-author-automerge.yml line 366. ## Test plan - [ ] Merge this PR - [ ] In any caller repo with \`secrets: inherit\`, set repo secret \`automerge_pat\` to a fine-grained PAT (contents r/w, pull-requests r/w) - [ ] Open a docs-only PR and confirm: - Workflow log shows \`Auth: caller-supplied automerge_pat — downstream workflows WILL trigger on merge.\` - After auto-merge resolves, \`gh api repos/.../actions/runs?head_sha=<merge_sha>\` returns Lint, Tests, and Build & Deploy runs - [ ] In a caller WITHOUT \`automerge_pat\` set, confirm the \`::warning::\` line appears (and current behavior is unchanged otherwise) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
topcoder1
added a commit
that referenced
this pull request
Jun 10, 2026
…otate (#95) ## Root cause this responds to On 2026-06-10 15:20–16:21Z, GitHub had a platform incident: *"sporadic authentication failures, impacting approximately 15% of API traffic. Erroneous 401 responses"* (githubstatus.com). Valid `GITHUB_TOKEN`s were intermittently treated as anonymous. In whois-api-llc/disposable-email-domains (first blocked-path PR since codex-review install), this reddened `classify / Classify PR Risk` — a **required** check there — on two consecutive pushes (runs 27288936851, 27289393205, 27289393136). Same job, same token: `gh pr edit` succeeded at 16:02:59 and a `gh` write 401'd at 16:03:01. A rerun 20h later passed unchanged — **there was no token-plumbing bug** (the suspected missing `GH_TOKEN`/`secrets: inherit` was a red herring; `github.token` was set at every step). The incident exposed two real structural weaknesses, fixed here: ## Changes 1. **`retry` on every gh call that gates the job** (3 attempts, 5s/10s backoff). Stdout is buffered per attempt so command substitutions never capture error bodies from failed attempts. Turns a 15% per-call platform blip into ~0.3%. 2. **Annotate step is now truly non-fatal** — its own comment has always promised "this step does NOT exit non-zero", but a gh failure violated that on 2026-06-10. Comment-write failures downgrade to `::warning::` (it's a policy notice; gating lives in the label + claude-author-automerge regex). 3. **New `skip_label` input** — fleet repos run this reusable from TWO workflows (standalone PR Classify + embedded in PR Codex Review), racing to write the same label and sticky comment, and producing two check contexts both named `classify / Classify PR Risk` (polluting required-check evaluation). Embedded callers will pass `skip_label: true` and just consume the `risk_class` output. Follow-up: fleet caller-template rollout renames the embedded job to `codex-classify` so the contexts stop colliding. 4. Fix stale 'exits 1' comment in this repo's own codex caller (exit-1 dropped in #21). NOTE: this repo's own caller does **not** get `skip_label: true` — it's the only labeler here (no standalone classify caller in ci-workflows). Backward compatible: `skip_label` defaults to `false`; existing callers see retry hardening only. ## Verification - `actionlint` (incl. shellcheck on run blocks): PASS - retry helper unit-tested locally: flaky-cmd (fail×2 with garbage stdout, then success) → substitution captured only clean output, 3 attempts; always-fail in `$(… || true)` → empty capture, no script abort **Codex pre-review:** PASS — no P1. Two P2s: (1) `gh pr view` in label step un-retried → fixed in this PR (wrapped in retry); (2) pre-existing word-splitting if a risk label name contained spaces → declined, label names come from the classifier's fixed enum (no spaces possible). **Auto-merge rationale:** manual click-merge — `.github/workflows/**` is on the high-risk list, and this is the fleet-wide reusable: a bad merge breaks classify on every PR in 40+ repos until reverted. 🤖 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.
Summary
pr-classify.ymlhas been exiting1on any PR that touches blocked paths (.github/workflows/**,Dockerfile*,**/secrets*,infra/**, etc.). That made both the PR Classify check and the PR Codex Review check (which calls the same reusable for its classify job) show red on every infra-touching PR — even when the actual code is fine and the maintainer is going to manually click merge anyway.Why the exit-1 was dead code
Confirmed on
topcoder1/webcrawl(the canonical caller):The classify check is not in the required-status-check list, so the exit-1 never actually blocked merge. It was pure UI noise.
The real auto-merge gate is
claude-author-automerge.yml's risk-tier path regex (lines 100-115 of that workflow). It already includes^\.github/workflows/.*and refuses to enable auto-merge when matched, posting a sticky comment instead. That gate works without failing the classify check.What this PR changes
exit 1with a::warning::annotation (visible in the Actions log).<!-- pr-classify:blocked -->marker) explaining the manual-merge policy inline on the PR.risk:blockedlabel (downstream gates still consume it).class=blocked(downstream gates still consume it).Net effect: callers get a green-or-failed CI status that reflects code correctness. Auto-merge gating stays exactly where it was. The "manual merge required" semantics are now a comment + warning + label, not a fake-red check.
Verification
<!-- pr-classify:blocked -->) idempotent across re-runs.Auto-merge rationale
This PR modifies
.github/workflows/pr-classify.yml(a workflow file). Per CLAUDE.md auto-merge policy, that's a high-risk surface — manual merge is required. The classify workflow on this very PR will demonstrate the new behavior (warning + comment + green check, instead of red + exit-1).Codex pre-review
Skipped (workflow-only diff, behavioral change is small and obvious).
🤖 Generated with Claude Code