Skip to content

fix(ci): the ADR gate goes red while auto-merge is armed on a docs/adr/** PR - #8302

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-8012-adr-gate-automerge-arming
Aug 13, 2026
Merged

fix(ci): the ADR gate goes red while auto-merge is armed on a docs/adr/** PR#8302
os-zhuang merged 2 commits into
mainfrom
claude/issue-8012-adr-gate-automerge-arming

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Part of #8012 — the maintainer-approved half (b), gate hardening. See "What this PR deliberately does not do" for the halves left open.

The gap, and why the obvious rule does not close it

Arming auto-merge is not merging, so no gate fired on it. An AI seat armed one on a live docs/adr/** PR at 11:15Z on 2026-08-12 while this gate was red. Nothing merged — a human disarmed it two minutes later — but the state left behind was one review away from an unattended merge with every check reporting success.

⚠️The ruling's literal wording is a measured no-op, and this PR implements its intent instead. The card and the ruling both say the gate should fail "when auto-merge is armed on a docs/adr/** PR without a qualifying approval". That predicate cannot change any verdict:

statebefore this PRunder the literal wordingunder this PR
armed + not approvedalready RED (missing approval)RED — no changeRED
armed + approvedGREEN ← the defectGREEN — the clause cannot fireRED

The armed-and-unapproved state is already red for the missing approval, so a rule that fires only there rejects nothing that was not rejected already. And the defect materialises one event later — at the approval, which is exactly when the literal predicate stops being true. So the clause here fires on the arming whether or not the PR is approved; the approved case is the only one in which the unattended merge actually happens.

This is strictly stricter in every direction — no threshold relaxed, no bypass added, no path by which a PR's own author, or any account, satisfies the gate by identity. The gate still reads state only, never actors.

What landed

  • scripts/check-adr-merge-approval.mjs — pass condition gains clause (2): auto-merge must not be armed. Both clauses are evaluated so a red names every reason at once.
  • armingFrom() reads the state from the liveGET /repos/{owner}/{repo}/pulls/{n}. The event payload is deliberately not trusted: GitHub does not document auto_merge as a member of the pull_request object carried by pull_request_review, and at least one projection in use here (pull_request_read) drops the field entirely. A payload with noauto_merge key is refused rather than read as disarmed — absent and null are different facts, and only null means disarmed.
  • getArming is mandatory on decide(): a caller that omits it throws. A forgotten argument must not coast on a default that reads as "auto-merge is off".
  • .github/workflows/adr-merge-approval.ymlpull_request gains auto_merge_enabled (arming is otherwise a silent state change this workflow never observes) and auto_merge_disabled (the zero-friction exit). The three default activity types are restated because naming any types: replaces the default set. Job name: unchanged; permissions unchanged — pull-requests: read already covers the PR read.

Measured, not assumed

Auto-merge state is reachable from where the gate runs.GET /pulls/{n} carries auto_merge in both states, measured against this repo on 2026-08-13: null on #7960, and a populated object on #6208 (enabled_by: os-zhuang, merge_method: merge) — that real capture is the self-test fixture, not a hand-written imitation.

The disarm/re-arm wrinkle.decide() keeps no state between runs, so the sequence green → re-arm → red → disarm → green is judged fresh each time; there is no earlier green for a re-arm to inherit. Pinned as arming-is-read-fresh-so-rearming-is-red-again. GitHub additionally refuses arming on a PR that can be merged immediately ("The option to enable auto-merge is shown only on pull requests that cannot be merged immediately", docs), so the fully-green case cannot be armed at all.

The window this does NOT close, stated rather than assumed. A PR that is approved, green here, and still waiting on some other required check can be armed in that window; auto_merge_enabled re-runs this gate, which then goes red, but if the other check goes green first the merge fires before the red lands. The race needs the arming to be the last blocking action. It is not closed here — closing it belongs to the ruleset side (option 2), which no CI job can perform. Recorded in the file header rather than left for a reader to discover.

Not a deadlock (#8161's lesson). Every red is cleared by an action any account can take — disable auto-merge — and disarming re-runs the gate, which then clears itself with no push, no re-review, no admin.

Not judged on merge_group builds, on purpose: by then the PR passed this gate at the PR level, where auto-merge waits, so a red there adds no safety and only evicts — and if "merge when ready" sets auto_merge as part of enqueueing, judging it there would make ADR PRs permanently unqueueable. The verdict records that the question was not asked (judged: false, armed: null) rather than quietly answering it "no".

The self-test is an instrument, not decoration

66 assertions (was 35). Per this family's standing lesson — a suite that passes first try is a signal to prove it is not empty — the new clause was mutation-tested, six distinct breaks, each caught by named assertions:

mutationcaught by
M1 delete clause (2)armed-and-approved-is-red, armed-clause-changes-a-verdict-that-would-otherwise-be-green, the-7960-incident-state-replays-red, +4
M2 the ruling's literal wording (armed AND unapproved)same 6 — this is the mutation that matters
M3 missing auto_merge key defaults to disarmeda-missing-auto_merge-key-is-diagnosed-as-a-projection-not-as-a-bad-value
M4 getArming optional, defaulting to not-judgedomitting-getArming-throws, omitting-getArming-throws-on-a-clean-diff-too
M5 parser reads an armed payload as disarmedcaptured-armed-pull-reads-as-armed, +4
M6 "not judged" recorded as armed: falsenot-judged-arming-is-null-never-false

⚠️M3 survived the first round and that is the honest headline of this suite: the original assertion only checked that the parser threw with a message mentioning auto_merge, which the generic fallback error also does, so it pinned nothing. The safety property held either way; what the dedicated guard buys is the operator being told which situation they are in. The assertion now pins that diagnosis, and M3 fails as it should. Without mutation testing this suite would have shipped with one empty assertion in it.

armed-clause-changes-a-verdict-that-would-otherwise-be-green is a permanent emptiness proof: two runs over identical reviews differing only in the arming bit must land on opposite verdicts.

Also replayed end-to-end over real captured payloads from open ADR PR #8291 — identical files and approval, differing only in the arming object: green when disarmed, red when armed.

What this PR deliberately does not do

Landing

⚠️ This PR modifies a gate that constrains AI merge behaviour, so it is left for the maintainer to merge — not enqueued, not auto-merged, no auto-merge armed on it. Opened as draft deliberately.

skip-changeset: scripts + workflow only, no package changes, releases nothing.

Verification

node scripts/check-adr-merge-approval.mjs --self-test 66 assertions
node scripts/check-adr-merge-approval.mjs clean path, zero API lookups
pnpm check:required-contexts / :workflow-status-functions / :node-version
pnpm check:shard-attestation / :changeset-gate-self-tests / :nul-bytes
node scripts/check-changeset-no-major.mjs
npx eslint scripts/check-adr-merge-approval.mjs

All green. Gate list re-derived against the actual changed paths via scripts/pm/dispatch-gates.mjs — no delta from the dispatch.


Generated by Claude Code

…r/** PR
Arming auto-merge is not merging, so no gate fired on it: an AI seat armed
one on a live docs/adr/** PR at 11:15Z on 2026-08-12 while this gate was red.
The next approving review would have turned the check green and GitHub would
have merged the PR unattended, with every check reporting success — 「人工合并」
bypassed while nothing looked wrong.
The gate now reads the PR's live auto-merge state and is red while it is
armed, whether or not the PR is approved. The approved case is the only one
in which the unattended merge actually happens; a rule firing only on the
unapproved case would change no verdict, since that state is already red for
the missing approval.
Not a deadlock: anyone can disable auto-merge, and the new auto_merge_disabled
trigger re-runs the gate so it clears itself.
Refs #8012
Mutation testing found the looser assertion passed on a mutant that deleted
the Object.hasOwn guard: undefined falls through to the not-an-object branch
and throws a message that also contains 'auto_merge'. Refs #8012
@vercel

vercelBot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 13, 2026 4:02am

Request Review

@os-zhuangos-zhuang added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed ci/cd labels Aug 13, 2026 — with Claude
@os-zhuang
os-zhuang marked this pull request as ready for review August 13, 2026 04:28
@os-zhuang
os-zhuang added this pull request to the merge queueAug 13, 2026
Merged via the queue into main with commit ebb14ecAug 13, 2026
27 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8012-adr-gate-automerge-arming branch August 13, 2026 10:30
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@os-zhuang