From efbbfbafb39a94932e385142d8c24c7b3df72d24 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Thu, 16 Jul 2026 21:21:12 -0700 Subject: [PATCH 1/6] =?UTF-8?q?fix(automerge):=20close=20the=20pr-classify?= =?UTF-8?q?=20TOCTOU=20race=20=E2=80=94=20classify=20against=20base-branch?= =?UTF-8?q?=20risk-paths.yml=20at=20decision=20time?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reusable trusted the live risk:* label as its canonical manual-merge signal, with a documented mitigation: the caller wires 'labeled' into trigger types so a late label re-runs the workflow and revokes. That re-run is structurally unreachable — pr-classify labels via GITHUB_TOKEN, and GITHUB_TOKEN-attributed events never trigger workflows. whois-api-llc/wxa-secrets#27 (2026-07-17): automerge read labels 4s before risk:blocked landed, armed, and the PR merged when checks went green. Every repo-specific blocked:/sensitive: path not covered by the global regex had this race. The verdict step now computes the classification itself at decision time: it reads .github/risk-paths.yml from the PR's BASE branch (a PR cannot edit its own risk file to escape; 404 = no repo policy, regex still gates; any other read failure fails CLOSED) and runs the same classify.mjs pr-classify fetches — one matcher, zero glob drift. The risk:* label stays honored as belt-and-suspenders (labels only ever ADD gating). No checks:read required (wxa-secrets deliberately omits it). selftest/test_automerge_riskfile_gate.sh extracts the shipped step and pins: the #27 race shape, sensitive, clean, label-only, base-ref pinning (incl. non-default base), 404 fall-through, fail-closed on API error and the 3000-file cap, and nocase parity via the real classify.mjs. Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-author-automerge.yml | 214 +++++++++--- .github/workflows/pr-classify.yml | 19 +- selftest/test_automerge_riskfile_gate.sh | 321 ++++++++++++++++++ selftest/test_workflow_guards.py | 1 + 4 files changed, 500 insertions(+), 55 deletions(-) create mode 100644 selftest/test_automerge_riskfile_gate.sh diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index af4b01e..2b09d1f 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -4,16 +4,22 @@ name: Claude-author Auto-merge (reusable) # (detected via branch prefix `claude/*`, `Co-Authored-By: Claude` trailer in # any PR commit, or the `auto-merge` label), provided the PR is neither: # -# 1. Labeled `risk:blocked` OR `risk:sensitive` by pr-classify.yml (canonical -# signals — the caller's risk-paths.yml `blocked:` / `sensitive:` lists -# determine these; covers any repo-specific path without us having to keep a -# parallel regex). Both mean manual click-merge: `blocked` is a hard -# privilege-escalation block, `sensitive` is a high-impact path the caller -# flags for human review (agent runtime, schema, migrations, etc.) — the -# reusable's global risk-tier regex (item 2) does NOT know a caller's -# repo-specific `sensitive:` paths, so honoring the label is the only way -# these get gated. `risk:sensitive` is a no-op for callers without a -# `sensitive:` list (label never applied). +# 1. Classified `blocked` OR `sensitive` by the caller's .github/risk-paths.yml +# (the `blocked:` / `sensitive:` lists — covers any repo-specific path +# without us having to keep a parallel regex). This verdict is computed BY +# THIS WORKFLOW at decision time: it runs the same classify.mjs that +# pr-classify.yml runs, against the rules file read from the PR's BASE +# branch. The pr-classify `risk:*` label is still honored as a +# belt-and-suspenders second signal (covers hand-applied labels), but it +# is no longer the only carrier — pr-classify labels via GITHUB_TOKEN, +# whose events never trigger workflows, so label-only gating was a TOCTOU +# race (see the classifier-verdict step below). Both classes mean manual +# click-merge: `blocked` is a hard privilege-escalation block, `sensitive` +# is a high-impact path the caller flags for human review (agent runtime, +# schema, migrations, etc.) — the reusable's global risk-tier regex +# (item 2) does NOT know a caller's repo-specific `sensitive:` paths, so +# the direct risk-paths read is the only way these get gated. `sensitive` +# is a no-op for callers without a `sensitive:` list. # # 2. Touching any risk-tier path category from the global CLAUDE.md policy: # - auth / login / signin / signup / logout / session(s) / oauth / @@ -201,57 +207,163 @@ jobs: echo "reason=$reason" >> "$GITHUB_OUTPUT" echo "Detection: claude_authored=$claude reason=$reason" - # Honor the pr-classify.yml verdict before doing per-path regex - # matching of our own. If the classifier labeled this PR - # `risk:blocked`, the policy is unambiguous: manual click-merge - # required, no bypass. This is the canonical signal — covers any - # future blocked path a caller's risk-paths.yml adds without us - # having to keep a parallel regex in sync. (Belt-and-suspenders - # with the regex below for the race window described next.) + # Classifier verdict, computed race-free at decision time. # - # Race window: pr-classify and this workflow both fire on - # `pull_request` opened. If we run faster than the classifier, we - # read `blocked=0` and fall through to the regex check below. The - # caller wires `labeled` into trigger types, so once the - # classifier applies the label this workflow re-runs and the - # revoke step below disables any auto-merge enabled on the first - # pass. The regex still catches the two paths we observed - # slipping through (.github/risk-paths.yml + CODEOWNERS) on the - # first run for that race window. + # HISTORY — why this step no longer trusts the label alone. + # pr-classify and this workflow both fire on the same pull_request + # event. The old design read the live risk:* labels and documented + # this mitigation: "the caller wires `labeled` into trigger types, + # so once the classifier applies the label this workflow re-runs + # and the revoke step disables any auto-merge enabled on the first + # pass." That re-run is structurally unreachable: pr-classify + # labels with GITHUB_TOKEN, and GITHUB_TOKEN-attributed events + # NEVER trigger workflows (GitHub's recursion prevention). So when + # this workflow won the race, nothing revoked the arm. Evidence, + # whois-api-llc/wxa-secrets#27 (2026-07-17 UTC): both workflows + # fired on ready_for_review at 00:40:37; this one read live labels + # at 00:40:43 (none yet), armed at 00:40:46; pr-classify applied + # risk:blocked at 00:40:50 — 4s late — and the PR merged at + # 00:42:30 when required checks went green. Every repo-specific + # blocked:/sensitive: path not covered by the global regex below + # had this race. + # + # FIX — compute the verdict ourselves, synchronously: + # + # 1. Belt-and-suspenders: honor a live risk:blocked/risk:sensitive + # label if one already landed (covers hand-applied labels; a + # label can only ADD gating here, never remove it). + # 2. Authoritative: read .github/risk-paths.yml from the PR's + # BASE branch — NOT the PR head, so a PR cannot edit its own + # risk file to escape — and classify the PR's changed files + # with the same classify.mjs pr-classify fetches (both pull it + # from topcoder1/ci-workflows@default-branch: one matcher, + # zero glob-semantics drift). blocked/sensitive ⇒ refuse. + # + # Fail-closed rules: risk-paths.yml unreadable for any reason other + # than a definitive 404 ⇒ fail the run (auto-merge never armed); + # classify.mjs guard/parse errors ⇒ fail; changed-file listing + # truncated at the REST 3000-file cap ⇒ fail. A 404 means the repo + # carries no repo-specific policy — the global risk-tier regex + # below still gates, which matches pre-classifier behavior. No + # `checks: read` is needed for any of this (deliberate: e.g. the + # wxa-secrets caller omits it to keep the Codex bypass disabled). + - name: Setup Node (risk-paths classification) + if: steps.detect.outputs.claude_authored == '1' + uses: actions/setup-node@v6 + with: + node-version: '22' + - name: Check classifier verdict id: classifier_verdict if: steps.detect.outputs.claude_authored == '1' env: GH_TOKEN: ${{ github.token }} PR: ${{ github.event.pull_request.number }} + BASE_REF: ${{ github.event.pull_request.base.ref }} run: | set -euo pipefail - # Query the API instead of the event payload — labels in the - # event snapshot are frozen at event time, but the classifier - # may have applied `risk:blocked` after the event fired. - labels=$(gh api "/repos/${{ github.repository }}/issues/$PR/labels" \ + retry() { local i out; for i in 1 2 3; do if out=$("$@"); then printf '%s' "$out"; return 0; fi; [ "$i" -lt 3 ] && sleep $((i * 5)); done; return 1; } + blocked=0 + verdict_label="" + verdict_source="" + + # Signal 1 (belt-and-suspenders): a risk:* label that already + # landed. Query the API, not the event payload — labels in the + # event snapshot are frozen at event time. A failed read + # degrades to "no label": Signal 2 below is the authoritative + # gate and fails closed on its own errors. + labels=$(gh api "/repos/${GITHUB_REPOSITORY}/issues/$PR/labels" \ --jq '.[].name' 2>/dev/null || echo "") - # Canonical "manual merge" signals from pr-classify.yml. risk:blocked - # = hard privilege-escalation block (no bypass). risk:sensitive = - # high-impact caller-specific path (agent runtime, schema, migrations) - # the global risk-tier regex below can't see, so the label is the only - # gate. Both refuse auto-merge here; the human click-merges after - # review. risk:sensitive is a no-op for callers without a `sensitive:` - # list (label never applied). if echo "$labels" | grep -qx 'risk:blocked'; then - echo "blocked=1" >> "$GITHUB_OUTPUT" - echo "verdict_label=risk:blocked" >> "$GITHUB_OUTPUT" - echo "::warning::PR labeled risk:blocked by pr-classify — auto-merge refused (no bypass applies)" + blocked=1; verdict_label="risk:blocked"; verdict_source="label" elif echo "$labels" | grep -qx 'risk:sensitive'; then - echo "blocked=1" >> "$GITHUB_OUTPUT" - echo "verdict_label=risk:sensitive" >> "$GITHUB_OUTPUT" - echo "::warning::PR labeled risk:sensitive by pr-classify — auto-merge refused (manual review-and-merge required)" - else - echo "blocked=0" >> "$GITHUB_OUTPUT" + blocked=1; verdict_label="risk:sensitive"; verdict_source="label" + fi + + # Signal 2 (authoritative, race-free): classify the changed + # files ourselves against the BASE branch's risk-paths.yml. + if [ "$blocked" -eq 0 ]; then + # Base-branch rules file. A 404 is definitive (no repo-specific + # policy → the global regex step still gates); anything else + # retries, then FAILS CLOSED — never arm auto-merge under an + # unreadable policy. + rp_state="" + rp_b64="" + for i in 1 2 3; do + rp_err=$(mktemp) + if rp_b64=$(gh api -X GET \ + "repos/${GITHUB_REPOSITORY}/contents/.github/risk-paths.yml" \ + -f ref="$BASE_REF" --jq '.content' 2>"$rp_err"); then + rp_state="present"; rm -f "$rp_err"; break + fi + if grep -q 'HTTP 404' "$rp_err"; then + rp_state="absent"; rm -f "$rp_err"; break + fi + cat "$rp_err" >&2; rm -f "$rp_err" + [ "$i" -lt 3 ] && sleep $((i * 5)) + done + if [ -z "$rp_state" ]; then + echo "::error::could not read .github/risk-paths.yml@${BASE_REF} after 3 attempts — refusing to arm auto-merge (fail closed). Re-run when the API recovers, or merge manually." + exit 1 + fi + + if [ "$rp_state" = "present" ]; then + # Changed-file names via the paginated files API (same call + # and same 3000-file fail-closed cap as pr-classify.yml). + changed=$(retry gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/files?per_page=100" \ + --paginate --jq '.[].filename') || { + echo "::error::could not list PR changed files after 3 attempts — refusing to arm auto-merge (fail closed)." + exit 1 + } + if [ "$(printf '%s\n' "$changed" | sed '/^$/d' | wc -l)" -ge 3000 ]; then + echo "::error::PR changes 3000+ files — file listing is truncated, cannot classify. Merge manually." + exit 1 + fi + + # Same classifier pr-classify.yml runs, fetched from the same + # place, run in a scratch dir so the PR checkout's own (head) + # risk-paths.yml is never consulted. classify.mjs failures + # (bracket/negation guards, YAML parse) exit nonzero ⇒ this + # step fails ⇒ fail closed, identical to pr-classify staying + # red on the same repo. + work=$(mktemp -d) + mkdir -p "$work/repo/.github" + printf '%s' "$rp_b64" | base64 -d > "$work/repo/.github/risk-paths.yml" + fetch_mjs() { + gh api "repos/topcoder1/ci-workflows/contents/.github/scripts/classify.mjs" \ + --jq '.content' + } + retry fetch_mjs | base64 -d > "$work/classify.mjs" + (cd "$work" && npm install --no-save --silent yaml@2 minimatch@10 >/dev/null) + class=$(cd "$work/repo" && printf '%s\n' "$changed" | node "$work/classify.mjs") + echo "risk-paths.yml@${BASE_REF} verdict: $class" + case "$class" in + blocked|sensitive) + blocked=1; verdict_label="risk:$class"; verdict_source="risk-paths" + ;; + esac + else + echo "No .github/risk-paths.yml on ${BASE_REF} — no repo-specific policy; the global risk-tier regex still applies." + fi + fi + + { + echo "blocked=$blocked" + echo "verdict_label=$verdict_label" + echo "verdict_source=$verdict_source" + } >> "$GITHUB_OUTPUT" + if [ "$blocked" -eq 1 ]; then + case "$verdict_source" in + label) why="labeled $verdict_label by pr-classify" ;; + *) why="classified $verdict_label against risk-paths.yml@${BASE_REF}" ;; + esac + echo "::warning::PR $why — auto-merge refused (manual click-merge required; no label or status-check bypass applies)" fi # Revoke auto-merge if a previous run of this workflow enabled it - # before the classifier finished labeling. Idempotent — `gh pr + # before this gate could see the verdict (armed before the direct + # risk-paths read existed, base risk policy expanded since, or a + # risk label hand-applied later). Idempotent — `gh pr # merge --disable-auto` returns non-zero when auto-merge wasn't # on; we tolerate that with `|| true` and verify state via the # follow-up gh pr view. @@ -662,6 +774,7 @@ jobs: GH_TOKEN: ${{ github.token }} PR: ${{ github.event.pull_request.number }} VERDICT: ${{ steps.classifier_verdict.outputs.verdict_label }} + VERDICT_SOURCE: ${{ steps.classifier_verdict.outputs.verdict_source }} run: | set -euo pipefail marker="" @@ -669,7 +782,11 @@ jobs: --jq ".[] | select(.body | contains(\"$marker\")) | .id" | head -1) { echo "$marker" - echo "**Auto-merge refused — \`${VERDICT:-risk:blocked}\` label set by pr-classify.**" + if [ "$VERDICT_SOURCE" = "risk-paths" ]; then + echo "**Auto-merge refused — \`${VERDICT:-risk:blocked}\`: changed files match the base branch's \`.github/risk-paths.yml\` (evaluated directly by the auto-merge gate — no label round-trip).**" + else + echo "**Auto-merge refused — \`${VERDICT:-risk:blocked}\` label set by pr-classify.**" + fi echo "" if [ "$VERDICT" = "risk:sensitive" ]; then echo "This PR touches a path in \`.github/risk-paths.yml\`'s \`sensitive:\` list" @@ -677,7 +794,8 @@ jobs: echo "sensitive-class PRs are reviewed (Claude + Adversarial + Codex) and then" echo "**human click-merged** — auto-merge does not apply. The reusable's global" echo "risk-tier regex can't see a caller's repo-specific \`sensitive:\` paths, so" - echo "the \`risk:sensitive\` label is what gates these." + echo "the \`risk:sensitive\` verdict (computed from the base branch's risk file," + echo "with the pr-classify label as a second signal) is what gates these." echo "" echo "If this is misclassified, edit the caller repo's \`.github/risk-paths.yml\`." else diff --git a/.github/workflows/pr-classify.yml b/.github/workflows/pr-classify.yml index ff45014..ec43933 100644 --- a/.github/workflows/pr-classify.yml +++ b/.github/workflows/pr-classify.yml @@ -92,8 +92,9 @@ jobs: changed=$(retry gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/files?per_page=100" \ --paginate --jq '.[].filename') # The listing endpoint caps at 3000 files. A truncated list can - # compute a falsely benign class, and the risk:* label is - # load-bearing for claude-author-automerge.yml — so stay red. + # compute a falsely benign class, and the risk:* label is still a + # gating input for claude-author-automerge.yml (belt-and-suspenders + # beside its own direct risk-paths read) — so stay red. if [ "$(printf '%s\n' "$changed" | sed '/^$/d' | wc -l)" -ge 3000 ]; then echo "::error::PR changes 3000+ files — file listing is truncated, cannot classify. Merge manually." exit 1 @@ -122,9 +123,12 @@ jobs: done # Create the label if it doesn't exist (idempotent — gh ignores conflict). gh label create "risk:$CLASS" --color "ededed" --description "Risk class: $CLASS" 2>/dev/null || true - # The label IS load-bearing (claude-author-automerge.yml reads it as - # its canonical manual-merge signal), so a persistent failure here - # must stay red — but transient API blips get retried first. + # The label still gates: claude-author-automerge.yml honors it as a + # belt-and-suspenders signal beside the direct risk-paths.yml read + # it performs itself (the label alone was a TOCTOU race — GITHUB_TOKEN + # label writes never retrigger workflows; wxa-secrets#27). It is also + # the primary signal for humans and BB-side tooling, so a persistent + # failure here must stay red — transient API blips get retried first. retry gh pr edit "$PR" --add-label "risk:$CLASS" - name: Annotate `blocked` class (manual-merge required) @@ -137,8 +141,9 @@ jobs: retry() { local i out; for i in 1 2 3; do if out=$("$@"); then printf '%s' "$out"; return 0; fi; [ "$i" -lt 3 ] && sleep $((i * 5)); done; return 1; } # IMPORTANT: this step does NOT exit non-zero. The reusable's job # is to *classify*, not to gate. Auto-merge gating happens in - # claude-author-automerge.yml which reads the `risk:blocked` label - # / risk-tier path regex and refuses to enable auto-merge. Failing + # claude-author-automerge.yml, which classifies the PR against the + # base branch's risk-paths.yml itself (plus the `risk:*` label and + # its global risk-tier regex) and refuses to enable auto-merge. Failing # the classify check here would (a) only add visual red without # blocking merge (since required-status-checks rulesets in caller # repos typically don't list this check), and (b) confuse "code diff --git a/selftest/test_automerge_riskfile_gate.sh b/selftest/test_automerge_riskfile_gate.sh new file mode 100644 index 0000000..4007b1f --- /dev/null +++ b/selftest/test_automerge_riskfile_gate.sh @@ -0,0 +1,321 @@ +#!/usr/bin/env bash +# Behavioral test for the classifier-verdict gate in +# claude-author-automerge.yml — the step that decides whether a +# Claude-authored PR may arm auto-merge. +# +# Incident (whois-api-llc/wxa-secrets#27, 2026-07-17T00:40Z): pr-classify +# and claude-author-automerge both fired on the same ready_for_review +# event. The automerge run queried live labels at :43 (none yet) and +# armed auto-merge at :46; pr-classify applied risk:blocked at :50 — 4s +# late — and the PR squash-merged when required checks went green. The +# workflow's documented mitigation ("caller wires `labeled` into trigger +# types so a re-run revokes") was structurally unreachable: pr-classify +# labels with GITHUB_TOKEN, and GITHUB_TOKEN-attributed events never +# trigger workflows. +# +# The fix: the verdict step no longer depends on the label round-trip. +# It reads .github/risk-paths.yml from the PR's BASE branch and runs the +# same classify.mjs pr-classify uses, synchronously, at decision time. +# This test EXTRACTS that step's bash from the workflow YAML and executes +# it against a stubbed `gh` + the REAL classify.mjs, pinning: +# +# 1. THE RACE: blocked-path PR with no labels yet ⇒ blocked=1 +# (source=risk-paths). This is the wxa-secrets#27 shape. +# 2. sensitive-path PR, no labels ⇒ blocked=1. +# 3. clean PR ⇒ blocked=0. +# 4. label belt-and-suspenders: risk:blocked label + clean files ⇒ +# blocked=1 (source=label). +# 5. BASE-REF PINNING: every rules read must carry ref= (the gh +# stub hard-fails otherwise), and a non-default base ref works — a +# PR must never be able to consult its own (head) risk file. +# 6. no risk-paths.yml on base (404) ⇒ blocked=0; the global regex +# step still gates downstream (that regex has its own selftest, +# test_automerge_risk_patterns.sh). +# 7. FAIL CLOSED: rules file unreadable (non-404 API failure) ⇒ the +# step exits nonzero — auto-merge is never armed under an +# unreadable policy. +# 8. FAIL CLOSED: 3000+ changed files (REST listing cap) ⇒ nonzero. +# 9. nocase parity: docs/SECRETS.md vs a lowercase '**/secrets*' +# blocked glob ⇒ blocked=1 — proves the gate inherits classify.mjs +# semantics (minimatch, dot:true, nocase on gating classes) instead +# of reimplementing them. +# 10. a FAILED label read degrades to "no label" but the direct +# risk-paths verdict still gates (label is fail-open only because +# Signal 2 is authoritative and fail-closed). +# +# Run from the repo root: +# bash selftest/test_automerge_riskfile_gate.sh +set -euo pipefail + +WF=.github/workflows/claude-author-automerge.yml +CLASSIFY=.github/scripts/classify.mjs +failed=0 +T=$(mktemp -d) +trap 'rm -rf "$T"' EXIT + +# --------------------------------------------------------------------------- +# 0a. Structural drift guards: the gate must use the SAME classifier, the +# SAME dependency pins, and read the rules from the BASE ref. +# --------------------------------------------------------------------------- +MJS_SOURCE='repos/topcoder1/ci-workflows/contents/.github/scripts/classify.mjs' +for wf in .github/workflows/pr-classify.yml "$WF"; do + if grep -q "$MJS_SOURCE" "$wf"; then + echo "✓ $wf fetches classify.mjs from the shared source" + else + echo "✗ $wf does not fetch classify.mjs from $MJS_SOURCE — matchers can drift" + failed=1 + fi + if grep -q 'yaml@2 minimatch@10' "$wf"; then + echo "✓ $wf pins classifier deps yaml@2 minimatch@10" + else + echo "✗ $wf does not pin classifier deps yaml@2 minimatch@10" + failed=1 + fi +done +if grep -q -- '-f ref="\$BASE_REF"' "$WF"; then + echo "✓ $WF reads risk-paths.yml pinned to the PR base ref" +else + echo "✗ $WF does not pin the risk-paths.yml read to \$BASE_REF" + failed=1 +fi + +# --------------------------------------------------------------------------- +# 0b. Extract the classifier-verdict step's run block — the shipped bash, +# not a mirrored copy. +# --------------------------------------------------------------------------- +awk ' + /^ - name: Check classifier verdict$/ { in_step=1 } + in_step && /^ run: \|/ { in_run=1; next } + in_run { + if ($0 ~ /^ / || $0 == "") { sub(/^ /, ""); print } + else { exit } + } +' "$WF" > "$T/gate.sh" + +if ! grep -q 'risk-paths.yml' "$T/gate.sh" || ! grep -q 'GITHUB_OUTPUT' "$T/gate.sh"; then + echo "✗ could not extract the classifier-verdict run block from $WF" + exit 1 +fi +echo "✓ extracted classifier-verdict step ($(wc -l < "$T/gate.sh" | tr -d ' ') lines)" + +# --------------------------------------------------------------------------- +# 0c. Stubs. `gh` dispatches on the requested URL; `sleep` no-ops the retry +# backoffs; `npm` links a prebuilt node_modules so each gate run is +# offline and fast while the shipped `npm install` line still executes. +# Knobs (env): +# STUB_LABELS — newline-separated label names ('' = none) +# STUB_LABELS_RC — nonzero: the labels call fails (API blip) +# STUB_RISK_FILE — path to the BASE-branch risk-paths.yml fixture; +# empty = 404 (file absent on base) +# STUB_RISK_RC — nonzero: contents call fails with a NON-404 error +# STUB_FILES — path to the newline-separated changed-file list +# REAL_CLASSIFY — path to the real classify.mjs served to the gate +# --------------------------------------------------------------------------- +mkdir -p "$T/bin" +cat > "$T/bin/gh" <<'STUB' +#!/usr/bin/env bash +args="$*" +case "$args" in + *issues/*/labels*) + [ "${STUB_LABELS_RC:-0}" != "0" ] && { echo "gh: Internal Server Error (HTTP 500)" >&2; exit 1; } + printf '%s\n' "${STUB_LABELS:-}" + ;; + *contents/.github/scripts/classify.mjs*) + base64 < "$REAL_CLASSIFY" + ;; + *contents/.github/risk-paths.yml*) + case "$args" in + *"ref=${BASE_REF}"*) ;; + *) echo "gh-stub: risk-paths read did not pin ref=${BASE_REF} (head/default-branch read?): $args" >&2; exit 64 ;; + esac + [ "${STUB_RISK_RC:-0}" != "0" ] && { echo "gh: Internal Server Error (HTTP 500)" >&2; exit 1; } + [ -z "${STUB_RISK_FILE:-}" ] && { echo "gh: Not Found (HTTP 404)" >&2; exit 1; } + base64 < "$STUB_RISK_FILE" + ;; + *pulls/*/files*) + cat "$STUB_FILES" + ;; + *) + echo "gh-stub: unexpected call: $args" >&2 + exit 64 + ;; +esac +STUB +chmod +x "$T/bin/gh" +printf '#!/usr/bin/env bash\nexit 0\n' > "$T/bin/sleep" +chmod +x "$T/bin/sleep" + +# Prebuild the classifier deps once (same pins pr-classify.yml installs). +mkdir -p "$T/deps" +(cd "$T/deps" && npm install --no-save --silent yaml@2 minimatch@10 >/dev/null 2>&1) +cat > "$T/bin/npm" < "$STUB_FILES" +} + +run_gate() { + OUT_FILE="$T/gh-output.txt" + : > "$OUT_FILE" + set +e + GATE_LOG=$(cd "$T" && \ + PATH="$T/bin:$PATH" \ + GITHUB_REPOSITORY="acme/fixture" PR=123 BASE_REF="$CASE_BASE_REF" \ + GITHUB_OUTPUT="$OUT_FILE" GH_TOKEN=stub \ + STUB_LABELS="$STUB_LABELS" STUB_LABELS_RC="$STUB_LABELS_RC" \ + STUB_RISK_FILE="$STUB_RISK_FILE" STUB_RISK_RC="$STUB_RISK_RC" \ + STUB_FILES="$STUB_FILES" REAL_CLASSIFY="$REAL_CLASSIFY" \ + bash gate.sh 2>&1) + GATE_RC=$? + set -e +} + +out_get() { grep "^$1=" "$OUT_FILE" | tail -1 | cut -d= -f2- || true; } + +# expect_verdict +expect_verdict() { + local desc="$1" want_blocked="$2" want_label="$3" want_source="$4" + if [ "$GATE_RC" != "0" ]; then + echo "✗ $desc — gate exited rc=$GATE_RC (want 0). Output:" + echo "$GATE_LOG" | sed 's/^/ /' + failed=1 + return + fi + local got_blocked got_label got_source + got_blocked=$(out_get blocked); got_label=$(out_get verdict_label); got_source=$(out_get verdict_source) + if [ "$got_blocked" = "$want_blocked" ] && [ "$got_label" = "$want_label" ] && [ "$got_source" = "$want_source" ]; then + echo "✓ $desc" + else + echo "✗ $desc — got blocked='$got_blocked' verdict_label='$got_label' verdict_source='$got_source'" \ + "(want '$want_blocked'/'$want_label'/'$want_source'). Output:" + echo "$GATE_LOG" | sed 's/^/ /' + failed=1 + fi +} + +# expect_fail_closed +expect_fail_closed() { + local desc="$1" needle="$2" + if [ "$GATE_RC" -ne 0 ] && echo "$GATE_LOG" | grep -q "$needle"; then + echo "✓ $desc" + else + echo "✗ $desc — want nonzero rc + '$needle'; got rc=$GATE_RC. Output:" + echo "$GATE_LOG" | sed 's/^/ /' + failed=1 + fi +} + +# Fixture rules — the wxa-secrets#27 shape: a repo-specific blocked tree the +# global risk-tier regex does NOT cover, plus a sensitive list. +cat > "$T/risk-fixture.yml" <<'YAML' +blocked: + - 'src/wxa_secrets/**' + - '**/secrets*' +sensitive: + - 'src/agent/runtime/**' +safe_test: + - 'tests/**' +trivial: + - 'docs/**' +YAML + +echo "" +echo "— behavioral cases —" + +# 1. THE RACE: no labels yet, blocked path ⇒ refused via direct read. +reset_case +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "src/wxa_secrets/onepassword.py" "tests/regression/test_op.py" > "$STUB_FILES" +run_gate +expect_verdict "race case: blocked path + no label yet ⇒ blocked (risk-paths)" 1 "risk:blocked" "risk-paths" + +# 2. sensitive path, no labels. +reset_case +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "src/agent/runtime/loop.py" > "$STUB_FILES" +run_gate +expect_verdict "sensitive path + no label ⇒ blocked (risk-paths)" 1 "risk:sensitive" "risk-paths" + +# 3. clean PR. +reset_case +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "src/util/helpers.py" "docs/notes.md" "tests/test_helpers.py" > "$STUB_FILES" +run_gate +expect_verdict "clean PR ⇒ not blocked" 0 "" "" + +# 4. label belt-and-suspenders: label present, files clean. +reset_case +STUB_LABELS="risk:blocked" +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "docs/notes.md" > "$STUB_FILES" +run_gate +expect_verdict "hand-applied risk:blocked label alone still gates" 1 "risk:blocked" "label" + +# 5. base-ref pinning on a non-default base (the stub exits 64 on any +# rules read that does not carry ref=, so every case above also +# asserts the pin; this one proves a non-'main' base is honored). +reset_case +CASE_BASE_REF="release/1.2" +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "src/wxa_secrets/store.py" > "$STUB_FILES" +run_gate +expect_verdict "rules read pinned to non-default base ref (release/1.2)" 1 "risk:blocked" "risk-paths" + +# 6. no risk-paths.yml on base (404) ⇒ not blocked here; global regex +# still gates downstream. +reset_case +printf '%s\n' "src/anything.py" > "$STUB_FILES" +run_gate +expect_verdict "risk-paths.yml absent on base (404) ⇒ falls through to regex" 0 "" "" + +# 7. FAIL CLOSED: rules unreadable (non-404). +reset_case +STUB_RISK_RC=1 +printf '%s\n' "src/anything.py" > "$STUB_FILES" +run_gate +expect_fail_closed "unreadable risk-paths.yml (HTTP 500 ×3) fails closed" "refusing to arm auto-merge" + +# 8. FAIL CLOSED: 3000-file listing cap. +reset_case +STUB_RISK_FILE="$T/risk-fixture.yml" +i=0 +while [ "$i" -lt 3000 ]; do echo "docs/f$i.md"; i=$((i + 1)); done > "$STUB_FILES" +run_gate +expect_fail_closed "3000+ changed files fails closed" "file listing is truncated" + +# 9. nocase parity with classify.mjs (wxa-jake-ai#875 shape): an uppercase +# variant must still hit a lowercase blocked glob. +reset_case +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "docs/SECRETS.md" > "$STUB_FILES" +run_gate +expect_verdict "docs/SECRETS.md hits lowercase '**/secrets*' (classify.mjs nocase inherited)" 1 "risk:blocked" "risk-paths" + +# 10. label read failure degrades to no-label, direct verdict still gates. +reset_case +STUB_LABELS_RC=1 +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "src/wxa_secrets/store.py" > "$STUB_FILES" +run_gate +expect_verdict "failed label read + blocked path ⇒ still blocked (risk-paths)" 1 "risk:blocked" "risk-paths" + +echo "" +if [ "$failed" -gt 0 ]; then + echo "FAIL: riskfile-gate case(s) regressed." + exit 1 +fi +echo "OK: all riskfile-gate cases pass." diff --git a/selftest/test_workflow_guards.py b/selftest/test_workflow_guards.py index a3a9d91..254d9a0 100644 --- a/selftest/test_workflow_guards.py +++ b/selftest/test_workflow_guards.py @@ -21,6 +21,7 @@ "script", [ "selftest/test_automerge_risk_patterns.sh", + "selftest/test_automerge_riskfile_gate.sh", "selftest/test_classify_bracket_guard.sh", "selftest/test_classify_nocase.sh", "selftest/test_pr_files_listing.sh", From 5f21c2c441decca27386e826770b74c55e2bbd92 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Thu, 16 Jul 2026 21:31:48 -0700 Subject: [PATCH 2/6] fix(automerge): harden the risk-paths gate per codex round-2 findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bind the arm to the event head SHA (gh pr merge --auto --match-head-commit): GraphQL rejects the enable atomically if the head moved between event and arm, so a mid-run push can never inherit an arm decided on older files. Head-moved failures exit 0 with a notice (the newer push's own run re-arms); other failures stay red. - 404 on the base ref now falls back to the DEFAULT branch's risk-paths.yml — a release/legacy base predating the policy file can't dodge the repo policy. Absent on both = policy-free repo (automerge-only callers stay supported). - Closed enum on classifier output: anything outside the documented class set fails closed instead of reading as benign. - Deliberately NOT pinning classify.mjs/deps beyond what pr-classify itself does — a one-sided pin would reintroduce matcher drift. Selftest: +default-branch-fallback case, +enum-violation case, +structural asserts for --match-head-commit and per-ref reads. Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-author-automerge.yml | 121 +++++++++++++----- selftest/test_automerge_riskfile_gate.sh | 114 +++++++++++++---- 2 files changed, 176 insertions(+), 59 deletions(-) diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index 2b09d1f..dc7db92 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -9,7 +9,8 @@ name: Claude-author Auto-merge (reusable) # without us having to keep a parallel regex). This verdict is computed BY # THIS WORKFLOW at decision time: it runs the same classify.mjs that # pr-classify.yml runs, against the rules file read from the PR's BASE -# branch. The pr-classify `risk:*` label is still honored as a +# branch (falling back to the default branch's file when the base ref +# predates it). The pr-classify `risk:*` label is still honored as a # belt-and-suspenders second signal (covers hand-applied labels), but it # is no longer the only carrier — pr-classify labels via GITHUB_TOKEN, # whose events never trigger workflows, so label-only gating was a TOCTOU @@ -241,12 +242,25 @@ jobs: # # Fail-closed rules: risk-paths.yml unreadable for any reason other # than a definitive 404 ⇒ fail the run (auto-merge never armed); - # classify.mjs guard/parse errors ⇒ fail; changed-file listing - # truncated at the REST 3000-file cap ⇒ fail. A 404 means the repo - # carries no repo-specific policy — the global risk-tier regex - # below still gates, which matches pre-classifier behavior. No - # `checks: read` is needed for any of this (deliberate: e.g. the - # wxa-secrets caller omits it to keep the Codex bypass disabled). + # classify.mjs guard/parse errors OR output outside the known class + # enum ⇒ fail; changed-file listing truncated at the REST 3000-file + # cap ⇒ fail. A 404 on the BASE ref falls back to the DEFAULT + # branch's risk-paths.yml (a release/legacy base that predates the + # policy file must not dodge the repo's policy — codex round-2 P1); + # only when the file is absent there too does the repo count as + # policy-free, and then the global risk-tier regex below still + # gates, which matches pre-classifier behavior (automerge-only + # repos without pr-classify stay supported). No `checks: read` is + # needed for any of this (deliberate: e.g. the wxa-secrets caller + # omits it to keep the Codex bypass disabled). + # + # Deliberately NOT pinned: classify.mjs is fetched from this repo's + # default branch and deps are pinned to majors (yaml@2 minimatch@10) + # — the SAME source and pins pr-classify.yml uses. Pinning only this + # side to an immutable ref would reintroduce matcher drift between + # the two workflows, which is the exact bug class this gate exists + # to close; callers reference the reusable @main anyway, so a pin + # here adds no supply-chain isolation in practice. - name: Setup Node (risk-paths classification) if: steps.detect.outputs.claude_authored == '1' uses: actions/setup-node@v6 @@ -260,12 +274,14 @@ jobs: GH_TOKEN: ${{ github.token }} PR: ${{ github.event.pull_request.number }} BASE_REF: ${{ github.event.pull_request.base.ref }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | set -euo pipefail retry() { local i out; for i in 1 2 3; do if out=$("$@"); then printf '%s' "$out"; return 0; fi; [ "$i" -lt 3 ] && sleep $((i * 5)); done; return 1; } blocked=0 verdict_label="" verdict_source="" + rules_ref="$BASE_REF" # Signal 1 (belt-and-suspenders): a risk:* label that already # landed. Query the API, not the event payload — labels in the @@ -281,30 +297,43 @@ jobs: fi # Signal 2 (authoritative, race-free): classify the changed - # files ourselves against the BASE branch's risk-paths.yml. + # files ourselves against the base branch's risk-paths.yml. if [ "$blocked" -eq 0 ]; then - # Base-branch rules file. A 404 is definitive (no repo-specific - # policy → the global regex step still gates); anything else - # retries, then FAILS CLOSED — never arm auto-merge under an - # unreadable policy. + # Rules file for a given ref. A 404 is definitive (file absent + # at that ref); anything else retries, then FAILS CLOSED — + # never arm auto-merge under an unreadable policy. Sets + # rp_state=present|absent and rp_b64 via globals. rp_state="" rp_b64="" - for i in 1 2 3; do - rp_err=$(mktemp) - if rp_b64=$(gh api -X GET \ - "repos/${GITHUB_REPOSITORY}/contents/.github/risk-paths.yml" \ - -f ref="$BASE_REF" --jq '.content' 2>"$rp_err"); then - rp_state="present"; rm -f "$rp_err"; break - fi - if grep -q 'HTTP 404' "$rp_err"; then - rp_state="absent"; rm -f "$rp_err"; break - fi - cat "$rp_err" >&2; rm -f "$rp_err" - [ "$i" -lt 3 ] && sleep $((i * 5)) - done - if [ -z "$rp_state" ]; then - echo "::error::could not read .github/risk-paths.yml@${BASE_REF} after 3 attempts — refusing to arm auto-merge (fail closed). Re-run when the API recovers, or merge manually." + read_rules() { + local ref="$1" i rp_err + rp_state="" + for i in 1 2 3; do + rp_err=$(mktemp) + if rp_b64=$(gh api -X GET \ + "repos/${GITHUB_REPOSITORY}/contents/.github/risk-paths.yml" \ + -f ref="$ref" --jq '.content' 2>"$rp_err"); then + rp_state="present"; rm -f "$rp_err"; return 0 + fi + if grep -q 'HTTP 404' "$rp_err"; then + rp_state="absent"; rm -f "$rp_err"; return 0 + fi + cat "$rp_err" >&2; rm -f "$rp_err" + [ "$i" -lt 3 ] && sleep $((i * 5)) + done + echo "::error::could not read .github/risk-paths.yml@${ref} after 3 attempts — refusing to arm auto-merge (fail closed). Re-run when the API recovers, or merge manually." exit 1 + } + read_rules "$rules_ref" + # Absent on the base ref ⇒ fall back to the DEFAULT branch's + # policy. A PR targeting a release/legacy branch created before + # risk-paths.yml existed must not dodge the repo's policy + # (codex round-2 P1). Only when the default branch lacks the + # file too is the repo genuinely policy-free. + if [ "$rp_state" = "absent" ] && [ -n "$DEFAULT_BRANCH" ] && [ "$rules_ref" != "$DEFAULT_BRANCH" ]; then + echo "No .github/risk-paths.yml on ${rules_ref} — falling back to the default branch's policy (${DEFAULT_BRANCH})." + rules_ref="$DEFAULT_BRANCH" + read_rules "$rules_ref" fi if [ "$rp_state" = "present" ]; then @@ -336,14 +365,24 @@ jobs: retry fetch_mjs | base64 -d > "$work/classify.mjs" (cd "$work" && npm install --no-save --silent yaml@2 minimatch@10 >/dev/null) class=$(cd "$work/repo" && printf '%s\n' "$changed" | node "$work/classify.mjs") - echo "risk-paths.yml@${BASE_REF} verdict: $class" + echo "risk-paths.yml@${rules_ref} verdict: $class" + # Closed enum: anything outside the classifier's documented + # classes (a garbled/partial line, an empty string, a future + # class this workflow doesn't know) must FAIL CLOSED, not + # fall through as benign (codex round-2 P2). case "$class" in blocked|sensitive) blocked=1; verdict_label="risk:$class"; verdict_source="risk-paths" ;; + standard|safe_test|safe_deps|safe_config|trivial) + ;; + *) + echo "::error::classify.mjs printed unexpected class '$class' — refusing to arm auto-merge (fail closed)." + exit 1 + ;; esac else - echo "No .github/risk-paths.yml on ${BASE_REF} — no repo-specific policy; the global risk-tier regex still applies." + echo "No .github/risk-paths.yml on ${BASE_REF} or the default branch — no repo-specific policy; the global risk-tier regex still applies." fi fi @@ -355,7 +394,7 @@ jobs: if [ "$blocked" -eq 1 ]; then case "$verdict_source" in label) why="labeled $verdict_label by pr-classify" ;; - *) why="classified $verdict_label against risk-paths.yml@${BASE_REF}" ;; + *) why="classified $verdict_label against risk-paths.yml@${rules_ref}" ;; esac echo "::warning::PR $why — auto-merge refused (manual click-merge required; no label or status-check bypass applies)" fi @@ -740,6 +779,8 @@ jobs: # `automerge_pat` secret docstring above for the full rationale. GH_TOKEN: ${{ secrets.automerge_pat || github.token }} PR_URL: ${{ github.event.pull_request.html_url }} + PR: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} METHOD: ${{ inputs.merge_method }} REASON: ${{ steps.detect.outputs.reason }} RISKY: ${{ steps.risk.outputs.risky }} @@ -760,7 +801,25 @@ jobs: else echo "::warning::Auth: GITHUB_TOKEN fallback — downstream push workflows (e.g. deploy) will be SILENCED on merge. Wire secrets.automerge_pat in the caller to fix." fi - gh pr merge --auto --"$METHOD" "$PR_URL" + # Bind the arm to the exact revision this run's gates evaluated + # (codex round-2 P1). --match-head-commit maps to GraphQL + # enablePullRequestAutoMerge.expectedHeadOid: GitHub rejects the + # arm atomically if the head moved between this event and now, so + # a push that lands mid-run can never inherit an arm decided on + # older files — its own synchronize run re-classifies and arms. + # (Residual, unchanged by this workflow: once armed, GitHub keeps + # auto-merge across later pushes; a post-arm push is handled by + # the synchronize re-run's classify+revoke, which wins while + # required checks are still running on the new head.) + if ! gh pr merge --auto --"$METHOD" --match-head-commit "$HEAD_SHA" "$PR_URL"; then + now=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}" --jq .head.sha 2>/dev/null || echo "") + if [ -n "$now" ] && [ "$now" != "$HEAD_SHA" ]; then + echo "::notice::head moved (${HEAD_SHA} → ${now}) between this event and arming — not arming from this run; the newer push's own run re-classifies and (re)arms." + exit 0 + fi + echo "::error::enable auto-merge failed and the head still matches ${HEAD_SHA} — investigate." + exit 1 + fi # Sticky comment when classifier marked the PR risk:blocked. Distinct # from the regex-based comment below — classifier-blocked has no diff --git a/selftest/test_automerge_riskfile_gate.sh b/selftest/test_automerge_riskfile_gate.sh index 4007b1f..bddb77b 100644 --- a/selftest/test_automerge_riskfile_gate.sh +++ b/selftest/test_automerge_riskfile_gate.sh @@ -28,9 +28,9 @@ # 5. BASE-REF PINNING: every rules read must carry ref= (the gh # stub hard-fails otherwise), and a non-default base ref works — a # PR must never be able to consult its own (head) risk file. -# 6. no risk-paths.yml on base (404) ⇒ blocked=0; the global regex -# step still gates downstream (that regex has its own selftest, -# test_automerge_risk_patterns.sh). +# 6. no risk-paths.yml on base OR default branch (404 twice) ⇒ +# blocked=0; the global regex step still gates downstream (that +# regex has its own selftest, test_automerge_risk_patterns.sh). # 7. FAIL CLOSED: rules file unreadable (non-404 API failure) ⇒ the # step exits nonzero — auto-merge is never armed under an # unreadable policy. @@ -42,6 +42,15 @@ # 10. a FAILED label read degrades to "no label" but the direct # risk-paths verdict still gates (label is fail-open only because # Signal 2 is authoritative and fail-closed). +# 11. DEFAULT-BRANCH FALLBACK (codex round-2 P1): base ref lacks the +# file (release branch predating the policy) ⇒ the default +# branch's policy is applied — a legacy base must not dodge it. +# 12. FAIL CLOSED on classifier-output enum violation (codex round-2 +# P2): classify.mjs printing anything outside the documented class +# set ⇒ nonzero, never silently benign. +# +# The enable-side companion (codex round-2 P1): the arm is bound to the +# event head SHA via `--match-head-commit`, asserted structurally below. # # Run from the repo root: # bash selftest/test_automerge_riskfile_gate.sh @@ -72,10 +81,16 @@ for wf in .github/workflows/pr-classify.yml "$WF"; do failed=1 fi done -if grep -q -- '-f ref="\$BASE_REF"' "$WF"; then - echo "✓ $WF reads risk-paths.yml pinned to the PR base ref" +if grep -q -- '-f ref="\$ref"' "$WF" && grep -q 'read_rules "\$rules_ref"' "$WF"; then + echo "✓ $WF reads risk-paths.yml pinned to an explicit ref (base, then default-branch fallback)" +else + echo "✗ $WF does not pin the risk-paths.yml read to an explicit ref" + failed=1 +fi +if grep -q -- '--match-head-commit "\$HEAD_SHA"' "$WF"; then + echo "✓ $WF binds the auto-merge arm to the event head SHA (--match-head-commit)" else - echo "✗ $WF does not pin the risk-paths.yml read to \$BASE_REF" + echo "✗ $WF does not bind the arm to the classified head SHA — a mid-run push could inherit the arm" failed=1 fi @@ -103,13 +118,17 @@ echo "✓ extracted classifier-verdict step ($(wc -l < "$T/gate.sh" | tr -d ' ') # backoffs; `npm` links a prebuilt node_modules so each gate run is # offline and fast while the shipped `npm install` line still executes. # Knobs (env): -# STUB_LABELS — newline-separated label names ('' = none) -# STUB_LABELS_RC — nonzero: the labels call fails (API blip) -# STUB_RISK_FILE — path to the BASE-branch risk-paths.yml fixture; -# empty = 404 (file absent on base) -# STUB_RISK_RC — nonzero: contents call fails with a NON-404 error -# STUB_FILES — path to the newline-separated changed-file list -# REAL_CLASSIFY — path to the real classify.mjs served to the gate +# STUB_LABELS — newline-separated label names ('' = none) +# STUB_LABELS_RC — nonzero: the labels call fails (API blip) +# STUB_RISK_FILE — BASE-ref risk-paths.yml fixture; '' = 404 +# STUB_RISK_DEFAULT_FILE — DEFAULT-branch fixture; '' = 404 +# STUB_RISK_RC — nonzero: base-ref read fails NON-404 +# STUB_FILES — newline-separated changed-file list (path) +# STUB_CLASSIFY_FILE — classifier served to the gate (defaults to +# the real classify.mjs via REAL_CLASSIFY) +# +# Any rules read whose ref is neither the base ref nor the default +# branch exits 64 — pinning is asserted on EVERY case, not just one. # --------------------------------------------------------------------------- mkdir -p "$T/bin" cat > "$T/bin/gh" <<'STUB' @@ -121,16 +140,29 @@ case "$args" in printf '%s\n' "${STUB_LABELS:-}" ;; *contents/.github/scripts/classify.mjs*) - base64 < "$REAL_CLASSIFY" + base64 < "${STUB_CLASSIFY_FILE:-$REAL_CLASSIFY}" ;; *contents/.github/risk-paths.yml*) - case "$args" in - *"ref=${BASE_REF}"*) ;; - *) echo "gh-stub: risk-paths read did not pin ref=${BASE_REF} (head/default-branch read?): $args" >&2; exit 64 ;; + _ref="" + case "$args" in *"ref=${BASE_REF}"*) _ref=base ;; esac + if [ -z "$_ref" ]; then + case "$args" in *"ref=${DEFAULT_BRANCH}"*) _ref=default ;; esac + fi + case "$_ref" in + base) + [ "${STUB_RISK_RC:-0}" != "0" ] && { echo "gh: Internal Server Error (HTTP 500)" >&2; exit 1; } + [ -z "${STUB_RISK_FILE:-}" ] && { echo "gh: Not Found (HTTP 404)" >&2; exit 1; } + base64 < "$STUB_RISK_FILE" + ;; + default) + [ -z "${STUB_RISK_DEFAULT_FILE:-}" ] && { echo "gh: Not Found (HTTP 404)" >&2; exit 1; } + base64 < "$STUB_RISK_DEFAULT_FILE" + ;; + *) + echo "gh-stub: risk-paths read with an unexpected ref (head read?): $args" >&2 + exit 64 + ;; esac - [ "${STUB_RISK_RC:-0}" != "0" ] && { echo "gh: Internal Server Error (HTTP 500)" >&2; exit 1; } - [ -z "${STUB_RISK_FILE:-}" ] && { echo "gh: Not Found (HTTP 404)" >&2; exit 1; } - base64 < "$STUB_RISK_FILE" ;; *pulls/*/files*) cat "$STUB_FILES" @@ -159,12 +191,14 @@ REAL_CLASSIFY="$PWD/$CLASSIFY" # --------------------------------------------------------------------------- # Runner + assertions. # --------------------------------------------------------------------------- -STUB_LABELS=""; STUB_LABELS_RC=0; STUB_RISK_FILE=""; STUB_RISK_RC=0 -STUB_FILES="$T/files.txt"; CASE_BASE_REF="main" +STUB_LABELS=""; STUB_LABELS_RC=0; STUB_RISK_FILE=""; STUB_RISK_DEFAULT_FILE="" +STUB_RISK_RC=0; STUB_CLASSIFY_FILE="" +STUB_FILES="$T/files.txt"; CASE_BASE_REF="main"; CASE_DEFAULT_BRANCH="main" reset_case() { - STUB_LABELS=""; STUB_LABELS_RC=0; STUB_RISK_FILE=""; STUB_RISK_RC=0 - CASE_BASE_REF="main" + STUB_LABELS=""; STUB_LABELS_RC=0; STUB_RISK_FILE=""; STUB_RISK_DEFAULT_FILE="" + STUB_RISK_RC=0; STUB_CLASSIFY_FILE="" + CASE_BASE_REF="main"; CASE_DEFAULT_BRANCH="main" : > "$STUB_FILES" } @@ -175,9 +209,11 @@ run_gate() { GATE_LOG=$(cd "$T" && \ PATH="$T/bin:$PATH" \ GITHUB_REPOSITORY="acme/fixture" PR=123 BASE_REF="$CASE_BASE_REF" \ + DEFAULT_BRANCH="$CASE_DEFAULT_BRANCH" \ GITHUB_OUTPUT="$OUT_FILE" GH_TOKEN=stub \ STUB_LABELS="$STUB_LABELS" STUB_LABELS_RC="$STUB_LABELS_RC" \ - STUB_RISK_FILE="$STUB_RISK_FILE" STUB_RISK_RC="$STUB_RISK_RC" \ + STUB_RISK_FILE="$STUB_RISK_FILE" STUB_RISK_DEFAULT_FILE="$STUB_RISK_DEFAULT_FILE" \ + STUB_RISK_RC="$STUB_RISK_RC" STUB_CLASSIFY_FILE="$STUB_CLASSIFY_FILE" \ STUB_FILES="$STUB_FILES" REAL_CLASSIFY="$REAL_CLASSIFY" \ bash gate.sh 2>&1) GATE_RC=$? @@ -275,12 +311,13 @@ printf '%s\n' "src/wxa_secrets/store.py" > "$STUB_FILES" run_gate expect_verdict "rules read pinned to non-default base ref (release/1.2)" 1 "risk:blocked" "risk-paths" -# 6. no risk-paths.yml on base (404) ⇒ not blocked here; global regex -# still gates downstream. +# 6. no risk-paths.yml on base OR default branch ⇒ not blocked here; +# global regex still gates downstream. reset_case +CASE_BASE_REF="release/0.9" printf '%s\n' "src/anything.py" > "$STUB_FILES" run_gate -expect_verdict "risk-paths.yml absent on base (404) ⇒ falls through to regex" 0 "" "" +expect_verdict "risk-paths.yml absent on base AND default (404×2) ⇒ falls through to regex" 0 "" "" # 7. FAIL CLOSED: rules unreadable (non-404). reset_case @@ -313,6 +350,27 @@ printf '%s\n' "src/wxa_secrets/store.py" > "$STUB_FILES" run_gate expect_verdict "failed label read + blocked path ⇒ still blocked (risk-paths)" 1 "risk:blocked" "risk-paths" +# 11. DEFAULT-BRANCH FALLBACK: base ref (legacy release branch) lacks the +# file; the default branch's policy must be applied — a legacy base +# must not dodge the repo's policy (codex round-2 P1). +reset_case +CASE_BASE_REF="release/0.9" +STUB_RISK_DEFAULT_FILE="$T/risk-fixture.yml" +printf '%s\n' "src/wxa_secrets/store.py" > "$STUB_FILES" +run_gate +expect_verdict "base 404 ⇒ default branch policy applied (legacy base can't dodge)" 1 "risk:blocked" "risk-paths" + +# 12. FAIL CLOSED on classifier-output enum violation: a classifier that +# prints something outside the documented class set must never be +# read as benign (codex round-2 P2). +reset_case +printf '%s\n' "console.log('bogus');" > "$T/bogus-classify.mjs" +STUB_CLASSIFY_FILE="$T/bogus-classify.mjs" +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "docs/notes.md" > "$STUB_FILES" +run_gate +expect_fail_closed "classifier output outside the class enum fails closed" "unexpected class" + echo "" if [ "$failed" -gt 0 ]; then echo "FAIL: riskfile-gate case(s) regressed." From 9907d2d1a4e745581ad74e020f79d2679133d78d Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Thu, 16 Jul 2026 21:35:34 -0700 Subject: [PATCH 3/6] fix(automerge): fail closed on an unreadable label set (codex round-3 P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hand-applied risk:blocked/risk:sensitive label is an independent signal the direct file classification cannot reconstruct — a human may know context no path glob carries. The previous '|| echo ""' degraded a failed label read to 'no label', which Signal 2 masked on blocked-path PRs but silently lost hand-applied labels on benign-path PRs. Now: retry x3, then refuse to arm (fail closed). Selftest: the old case 10 asserted the masked behavior; replaced with two fail-closed cases (blocked files + the load-bearing clean-files variant). Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-author-automerge.yml | 18 +++++++++++---- selftest/test_automerge_riskfile_gate.sh | 23 +++++++++++++++---- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index dc7db92..2e3a6b5 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -285,11 +285,19 @@ jobs: # Signal 1 (belt-and-suspenders): a risk:* label that already # landed. Query the API, not the event payload — labels in the - # event snapshot are frozen at event time. A failed read - # degrades to "no label": Signal 2 below is the authoritative - # gate and fails closed on its own errors. - labels=$(gh api "/repos/${GITHUB_REPOSITORY}/issues/$PR/labels" \ - --jq '.[].name' 2>/dev/null || echo "") + # event snapshot are frozen at event time. A HAND-APPLIED risk + # label is an independent signal the file classification below + # cannot reconstruct (a human may know context no path glob + # carries), so an unreadable label set FAILS CLOSED after + # retries rather than degrading to "no label" (codex round-3 + # P1 — the earlier fail-open here was masked by Signal 2 in + # the blocked-path case but lost hand-applied labels on + # benign-path PRs). + labels=$(retry gh api "/repos/${GITHUB_REPOSITORY}/issues/$PR/labels" \ + --jq '.[].name') || { + echo "::error::could not read PR labels after 3 attempts — refusing to arm auto-merge (fail closed). Re-run when the API recovers, or merge manually." + exit 1 + } if echo "$labels" | grep -qx 'risk:blocked'; then blocked=1; verdict_label="risk:blocked"; verdict_source="label" elif echo "$labels" | grep -qx 'risk:sensitive'; then diff --git a/selftest/test_automerge_riskfile_gate.sh b/selftest/test_automerge_riskfile_gate.sh index bddb77b..ae3f0a7 100644 --- a/selftest/test_automerge_riskfile_gate.sh +++ b/selftest/test_automerge_riskfile_gate.sh @@ -39,9 +39,12 @@ # blocked glob ⇒ blocked=1 — proves the gate inherits classify.mjs # semantics (minimatch, dot:true, nocase on gating classes) instead # of reimplementing them. -# 10. a FAILED label read degrades to "no label" but the direct -# risk-paths verdict still gates (label is fail-open only because -# Signal 2 is authoritative and fail-closed). +# 10. FAIL CLOSED on an unreadable label set — with blocked files AND +# with clean files. A hand-applied risk label is an independent +# signal the file classification cannot reconstruct, so a failed +# label read must never degrade to "no label" (codex round-3 P1: +# the clean-files variant is the one a blocked-path-only case +# masks). # 11. DEFAULT-BRANCH FALLBACK (codex round-2 P1): base ref lacks the # file (release branch predating the policy) ⇒ the default # branch's policy is applied — a legacy base must not dodge it. @@ -342,13 +345,23 @@ printf '%s\n' "docs/SECRETS.md" > "$STUB_FILES" run_gate expect_verdict "docs/SECRETS.md hits lowercase '**/secrets*' (classify.mjs nocase inherited)" 1 "risk:blocked" "risk-paths" -# 10. label read failure degrades to no-label, direct verdict still gates. +# 10. FAIL CLOSED on an unreadable label set — a hand-applied risk label +# is an independent signal the file check cannot reconstruct. The +# clean-files variant is the load-bearing one: with blocked files, +# Signal 2 would mask a fail-open here (codex round-3 P1). reset_case STUB_LABELS_RC=1 STUB_RISK_FILE="$T/risk-fixture.yml" printf '%s\n' "src/wxa_secrets/store.py" > "$STUB_FILES" run_gate -expect_verdict "failed label read + blocked path ⇒ still blocked (risk-paths)" 1 "risk:blocked" "risk-paths" +expect_fail_closed "unreadable label set (blocked path) fails closed" "could not read PR labels" + +reset_case +STUB_LABELS_RC=1 +STUB_RISK_FILE="$T/risk-fixture.yml" +printf '%s\n' "docs/notes.md" > "$STUB_FILES" +run_gate +expect_fail_closed "unreadable label set (clean files) fails closed — hand-applied label can't be silently lost" "could not read PR labels" # 11. DEFAULT-BRANCH FALLBACK: base ref (legacy release branch) lacks the # file; the default branch's policy must be applied — a legacy base From d1e8f1aa95fdc66b3f6845efcf8cd5a487965d13 Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Thu, 16 Jul 2026 21:41:47 -0700 Subject: [PATCH 4/6] fix(automerge): revoke stale arms when gates error; paginate label read (codex round-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 'Refusing to arm' is not fail-closed for a PR a previous run already armed: a gating step that errors kills the job before the revoke steps, leaving the stale arm to merge once checks pass. New terminal always() step revokes any existing arm whenever classifier_verdict or the risk scan errored; the job still ends red from the failed gate. - Label read now paginates (per_page=100 --paginate) — a hand-applied risk label past the 30-label default page must not read as absent. - Documented the deliberate no-revoke in the enable step's head-moved path: a stale run revoking could disarm the newer head's valid arm and strand clean PRs on rapid fixup pushes; a blocked post-arm push is revoked by its own synchronize run, which only loses if every required check beats ~1 min — not a real shape in this fleet. Selftest: structural pins for the always() error-revoke wiring and label pagination. Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-author-automerge.yml | 50 ++++++++++++++++--- selftest/test_automerge_riskfile_gate.sh | 25 +++++++++- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index 2e3a6b5..2813dd5 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -293,8 +293,8 @@ jobs: # P1 — the earlier fail-open here was masked by Signal 2 in # the blocked-path case but lost hand-applied labels on # benign-path PRs). - labels=$(retry gh api "/repos/${GITHUB_REPOSITORY}/issues/$PR/labels" \ - --jq '.[].name') || { + labels=$(retry gh api "/repos/${GITHUB_REPOSITORY}/issues/$PR/labels?per_page=100" \ + --paginate --jq '.[].name') || { echo "::error::could not read PR labels after 3 attempts — refusing to arm auto-merge (fail closed). Re-run when the API recovers, or merge manually." exit 1 } @@ -815,10 +815,16 @@ jobs: # arm atomically if the head moved between this event and now, so # a push that lands mid-run can never inherit an arm decided on # older files — its own synchronize run re-classifies and arms. - # (Residual, unchanged by this workflow: once armed, GitHub keeps - # auto-merge across later pushes; a post-arm push is handled by - # the synchronize re-run's classify+revoke, which wins while - # required checks are still running on the new head.) + # Deliberately NO revoke in the head-moved path below: this run's + # knowledge is stale, and a revoke here can land AFTER the newer + # head's own run validly armed — disarming a clean PR and + # stranding it until the next event (rapid fixup pushes make that + # ordering common). Residual, unchanged by this workflow: once + # armed, GitHub keeps auto-merge across later pushes; a blocked + # post-arm push is revoked by its own synchronize run (~1 min + # from push), which loses only if EVERY required check on the new + # head completes faster — not a real shape in this fleet, where + # Claude Review (minutes) is the universal required check. if ! gh pr merge --auto --"$METHOD" --match-head-commit "$HEAD_SHA" "$PR_URL"; then now=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}" --jq .head.sha 2>/dev/null || echo "") if [ -n "$now" ] && [ "$now" != "$HEAD_SHA" ]; then @@ -934,3 +940,35 @@ jobs: else gh pr comment "$PR" --body-file /tmp/automerge-comment.md fi + + # FAIL CLOSED for already-armed PRs (codex round-4 P1): "refusing + # to arm" is not enough when a previous run already armed. If a + # gating step ERRORED (rules unreadable after retries, label read + # failed, classifier enum violation, 3000-file listing cap), this + # run cannot know the PR is safe — and without this step the job + # would just go red while the stale arm survives and merges once + # checks pass. Runs last, under always(), because a failed step + # otherwise skips everything after it; outcome == 'failure' + # distinguishes a genuine error from skipped (non-Claude PRs, + # blocked short-circuits). The job still ends red from the failed + # gate step — this only guarantees the arm is gone. + - name: Revoke auto-merge if gates errored + if: | + always() && + steps.detect.outputs.claude_authored == '1' && + (steps.classifier_verdict.outcome == 'failure' || + steps.risk.outcome == 'failure') + env: + GH_TOKEN: ${{ github.token }} + PR_URL: ${{ github.event.pull_request.html_url }} + run: | + set -euo pipefail + echo "A gating step errored — revoking any existing auto-merge arm (fail closed for already-armed PRs)." + gh pr merge --disable-auto "$PR_URL" 2>&1 || true + state=$(gh pr view "$PR_URL" --json autoMergeRequest \ + --jq 'if .autoMergeRequest == null then "OFF" else "ON" end') + echo "Auto-merge state after error revoke: $state" + if [ "$state" = "ON" ]; then + echo "::error::auto-merge still ON after --disable-auto — investigate" + exit 1 + fi diff --git a/selftest/test_automerge_riskfile_gate.sh b/selftest/test_automerge_riskfile_gate.sh index ae3f0a7..14cafe9 100644 --- a/selftest/test_automerge_riskfile_gate.sh +++ b/selftest/test_automerge_riskfile_gate.sh @@ -52,8 +52,13 @@ # P2): classify.mjs printing anything outside the documented class # set ⇒ nonzero, never silently benign. # -# The enable-side companion (codex round-2 P1): the arm is bound to the -# event head SHA via `--match-head-commit`, asserted structurally below. +# Structural pins on the surrounding job (codex rounds 2 & 4): the arm +# is bound to the event head SHA via `--match-head-commit`; an always() +# cleanup step revokes any existing arm when a gating step ERRORS (a red +# run must not leave a stale arm alive — "refusing to arm" is not fail +# closed for a PR a previous run already armed); and the label read +# paginates (a hand-applied risk label past the 30-label default page +# must not read as absent). # # Run from the repo root: # bash selftest/test_automerge_riskfile_gate.sh @@ -96,6 +101,15 @@ else echo "✗ $WF does not bind the arm to the classified head SHA — a mid-run push could inherit the arm" failed=1 fi +if grep -q 'name: Revoke auto-merge if gates errored' "$WF" \ + && grep -A5 'name: Revoke auto-merge if gates errored' "$WF" | grep -q 'always()' \ + && grep -q "steps.classifier_verdict.outcome == 'failure'" "$WF" \ + && grep -q "steps.risk.outcome == 'failure'" "$WF"; then + echo "✓ $WF revokes an existing arm under always() when a gating step errors" +else + echo "✗ $WF lacks the always() error-revoke step — a red run would leave a stale arm alive" + failed=1 +fi # --------------------------------------------------------------------------- # 0b. Extract the classifier-verdict step's run block — the shipped bash, @@ -116,6 +130,13 @@ if ! grep -q 'risk-paths.yml' "$T/gate.sh" || ! grep -q 'GITHUB_OUTPUT' "$T/gate fi echo "✓ extracted classifier-verdict step ($(wc -l < "$T/gate.sh" | tr -d ' ') lines)" +if grep -q 'labels?per_page=100' "$T/gate.sh" && grep -B1 -A1 'labels?per_page=100' "$T/gate.sh" | grep -q -- '--paginate'; then + echo "✓ label read paginates (a hand-applied label past page 1 cannot read as absent)" +else + echo "✗ label read does not paginate — a risk label past the 30-label default page reads as absent" + failed=1 +fi + # --------------------------------------------------------------------------- # 0c. Stubs. `gh` dispatches on the requested URL; `sleep` no-ops the retry # backoffs; `npm` links a prebuilt node_modules so each gate run is From 5e73e3b7714ade4fab7d62a83142c71f8a8055fb Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Thu, 16 Jul 2026 21:44:40 -0700 Subject: [PATCH 5/6] fix(automerge): error-revoke must cover prerequisite failures too (codex round-5 P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A checkout/detect/setup-node error leaves the gate steps 'skipped', not 'failure' — the round-4 error-revoke condition never fired and a stale arm survived a red run. The cleanup now watches those outcomes as well, and when checkout/detect died before authorship detection it falls back to the payload-level claude/* branch prefix (trailer-only Claude PRs with a failed checkout remain uncovered — accepted, rare- squared, and the step itself needs no workspace so it runs even when checkout failed). Selftest: structural pin extended to the prerequisite outcomes. Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-author-automerge.yml | 21 +++++++++++++++++-- selftest/test_automerge_riskfile_gate.sh | 8 ++++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index 2813dd5..ee773fa 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -165,6 +165,7 @@ jobs: # path still work. steps: - name: Checkout PR head + id: checkout uses: actions/checkout@v6 with: fetch-depth: 0 @@ -262,6 +263,7 @@ jobs: # to close; callers reference the reusable @main anyway, so a pin # here adds no supply-chain isolation in practice. - name: Setup Node (risk-paths classification) + id: setup_node if: steps.detect.outputs.claude_authored == '1' uses: actions/setup-node@v6 with: @@ -952,12 +954,27 @@ jobs: # distinguishes a genuine error from skipped (non-Claude PRs, # blocked short-circuits). The job still ends red from the failed # gate step — this only guarantees the arm is gone. + # + # PREREQUISITE failures count too (codex round-5 P1): if checkout, + # detect, or setup-node errors, the gate steps end 'skipped' — not + # 'failure' — and the arm would survive on the outcome checks + # alone. Checkout/detect failure also means authorship was never + # detected, so the guard falls back to the payload-level claude/* + # branch prefix (the fleet's dominant Claude-PR shape; a + # trailer-only Claude PR with a failed checkout stays uncovered — + # accepted residual). This step needs no workspace, so it runs + # fine even when checkout itself failed. - name: Revoke auto-merge if gates errored if: | always() && - steps.detect.outputs.claude_authored == '1' && + (steps.detect.outputs.claude_authored == '1' || + (steps.detect.outcome != 'success' && + startsWith(github.event.pull_request.head.ref, 'claude/'))) && (steps.classifier_verdict.outcome == 'failure' || - steps.risk.outcome == 'failure') + steps.risk.outcome == 'failure' || + steps.setup_node.outcome == 'failure' || + steps.checkout.outcome == 'failure' || + steps.detect.outcome == 'failure') env: GH_TOKEN: ${{ github.token }} PR_URL: ${{ github.event.pull_request.html_url }} diff --git a/selftest/test_automerge_riskfile_gate.sh b/selftest/test_automerge_riskfile_gate.sh index 14cafe9..9163906 100644 --- a/selftest/test_automerge_riskfile_gate.sh +++ b/selftest/test_automerge_riskfile_gate.sh @@ -104,10 +104,12 @@ fi if grep -q 'name: Revoke auto-merge if gates errored' "$WF" \ && grep -A5 'name: Revoke auto-merge if gates errored' "$WF" | grep -q 'always()' \ && grep -q "steps.classifier_verdict.outcome == 'failure'" "$WF" \ - && grep -q "steps.risk.outcome == 'failure'" "$WF"; then - echo "✓ $WF revokes an existing arm under always() when a gating step errors" + && grep -q "steps.risk.outcome == 'failure'" "$WF" \ + && grep -q "steps.setup_node.outcome == 'failure'" "$WF" \ + && grep -q "steps.checkout.outcome == 'failure'" "$WF"; then + echo "✓ $WF revokes an existing arm under always() when a gating or prerequisite step errors" else - echo "✗ $WF lacks the always() error-revoke step — a red run would leave a stale arm alive" + echo "✗ $WF error-revoke step missing or not covering gate + prerequisite (checkout/setup-node) failures" failed=1 fi From 767e1a0966379d64b7ac5f58d33a555f2cece56d Mon Sep 17 00:00:00 2001 From: topcoder1 Date: Thu, 16 Jul 2026 21:47:40 -0700 Subject: [PATCH 6/6] fix(automerge): head-ownership guard on the error-revoke (codex round-6 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An older run erroring late could revoke unconditionally AFTER a newer-head run validly armed — disarming a clean PR with no event left to re-arm it (the same ordering hazard the enable step's head-moved path documents). The cleanup now skips only on a POSITIVE head-moved read (the newer head's run owns gating); a failed head read still revokes, failing toward safety. Codex round 6 also flagged (deferred, documented in the PR): a stale automated risk label can strand a de-risked PR until the next event — pure liveness, pre-existing behavior, needs the timeline-actor distinction (bot vs hand-applied) as its own change. Co-Authored-By: Claude Fable 5 --- .github/workflows/claude-author-automerge.yml | 14 ++++++++++++++ selftest/test_automerge_riskfile_gate.sh | 7 ++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index ee773fa..43ff42c 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -978,8 +978,22 @@ jobs: env: GH_TOKEN: ${{ github.token }} PR_URL: ${{ github.event.pull_request.html_url }} + PR: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | set -euo pipefail + # Ownership guard (codex round-6 P2): if the head moved since + # this run's event, the newer head's own run owns the arm + # decision — an unconditional revoke here could land AFTER that + # run validly armed, disarming a clean PR with no event left to + # re-arm it. Skip only on a POSITIVE head-moved read; if the + # read itself fails, revoke anyway (fail toward safety — the + # rare stale disarm self-heals on the next event). + now=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}" --jq .head.sha 2>/dev/null || echo "") + if [ -n "$now" ] && [ "$now" != "$HEAD_SHA" ]; then + echo "::notice::head moved (${HEAD_SHA} → ${now}) since this event — the newer head's run owns gating; skipping stale revoke." + exit 0 + fi echo "A gating step errored — revoking any existing auto-merge arm (fail closed for already-armed PRs)." gh pr merge --disable-auto "$PR_URL" 2>&1 || true state=$(gh pr view "$PR_URL" --json autoMergeRequest \ diff --git a/selftest/test_automerge_riskfile_gate.sh b/selftest/test_automerge_riskfile_gate.sh index 9163906..0e54cf1 100644 --- a/selftest/test_automerge_riskfile_gate.sh +++ b/selftest/test_automerge_riskfile_gate.sh @@ -106,10 +106,11 @@ if grep -q 'name: Revoke auto-merge if gates errored' "$WF" \ && grep -q "steps.classifier_verdict.outcome == 'failure'" "$WF" \ && grep -q "steps.risk.outcome == 'failure'" "$WF" \ && grep -q "steps.setup_node.outcome == 'failure'" "$WF" \ - && grep -q "steps.checkout.outcome == 'failure'" "$WF"; then - echo "✓ $WF revokes an existing arm under always() when a gating or prerequisite step errors" + && grep -q "steps.checkout.outcome == 'failure'" "$WF" \ + && grep -A40 'name: Revoke auto-merge if gates errored' "$WF" | grep -q 'skipping stale revoke'; then + echo "✓ $WF revokes an existing arm under always() when a gating or prerequisite step errors (head-ownership-guarded)" else - echo "✗ $WF error-revoke step missing or not covering gate + prerequisite (checkout/setup-node) failures" + echo "✗ $WF error-revoke step missing, not covering gate + prerequisite failures, or lacking the head-ownership guard" failed=1 fi