fix(ci): the ADR merge gate requires an APPROVED review, from anyone

[hotlong]

Fixes#8161

The ADR merge gate matched the approving account against a hard-coded MAINTAINER_APPROVERS list. That proxy became unsatisfiable once cloud sessions began authoring PRs under the maintainer's own account: GitHub forbids self-approval, so a required context was permanently red on exactly the PRs the human was driving — while an AI seat holding the same credential could still satisfy it.

The ruling this implements

Maintainer, 2026-08-12, verbatim (quoted untranslated):

门禁改成只要求「APPROVED review 存在」
不要指定具体的人

So the account list is retired, not widened: there is no list any more, and deliberately no configurable replacement (a repo/org variable is writable by the same shared identities the original constant refused, which is why the old header ruled that surface out — that reasoning still holds).

This supersedes the earlier "give the agent fleet its own GitHub account" ruling recorded on the card; that is an org-side action, now decoupled from the gate and not a prerequisite for this change.

What changed

• MAINTAINER_APPROVERS and isMaintainer are deleted.
• latestMaintainerReviewState becomes latestReviewState, same fold, minus the account filter. The revocation semantics are unchanged: APPROVED / CHANGES_REQUESTED / DISMISSED set the standing, the last one wins, COMMENTED and PENDING set nothing. The pass condition is the current standing, never "an APPROVED review has ever existed" — an unrevokable approval would be a one-way door.
• decide()'s verdict kinds: maintainer-approved → approved, missing-maintainer-approval → missing-approval.
• Everything else is untouched: the docs/adr/** path-prefix match with no size exemption, the zero-API-lookup clean path, the loud-failure discipline (an unfetchable or unreadable review list is exit 1, never exit 0), the merge_group resolution and its fail-closed over-approximation, and the missing-input behaviour.

approvalsFromNonMaintainers — kept, renamed, re-purposed

Its name asserted a maintainer/non-maintainer distinction the gate no longer draws, so leaving it as-is was not an option. It is not deleted either: it is now approverLogins(reviews), listing every login that has submitted an approving review, and it earns its place on the RED path, where the confusing case is "someone approved, yet the check is red". The verdict field strangerApprovals becomes approvals, and the failure message now explains the supersession instead of scolding the approver:

APPROVED review(s) from 'os-zhuang' exist but no longer stand:
a later CHANGES_REQUESTED superseded them. An approval is revocable by design.

Deleting it would have removed the only signal that distinguishes "nobody looked at this" from "an approval was revoked" — two very different reds.

Cross-reviewer semantics — a new surface, resolved in the strict direction

With the account filter gone the fold runs over all reviewers, so two cases exist that could not arise before. Both are pinned in the self-test:

• reviewer A approves, reviewer B requests changes ⇒ red (a second reviewer's CHANGES_REQUESTED does revoke);
• reviewer A requests changes, reviewer B approves ⇒ green (it takes a fresh approval, from anyone, to clear it).

The first is the reason for folding globally rather than per-reviewer: "a later CHANGES_REQUESTED still revokes an earlier APPROVED" holds unconditionally, which is the fail-closed reading.

Headers rewritten, not patched

Both headers argued at length for the account allowlist. They now carry a two-clause table stating what the gate guarantees (a docs/adr/** diff cannot reach a mergeable state with no current approving review) and what it no longer guarantees (that the approver is the maintainer; that a human performed the merge). #6741's 「维护者自己确认」/「人工合并」 survive as convention, and the header says so rather than letting code and comment drift apart again.

⚠️ The consequence, stated rather than buried

Under the new rule, any account with review rights on this repository — including an AI seat — satisfies this gate. That is the maintainer's accepted trade, made explicitly.

It also makes the gap recorded in #8012 materially sharper: this gate blocks merging but does not block arming auto-merge, and the trigger for that armed merge moves from "the maintainer approves" to "anyone approves" — an armed ADR PR can now be merged unattended by the first approval that lands. That hardening is out of scope here and is tracked at #8012; this PR does not change arming behaviour. (#8012 remains open.)

Measurements — the PM's hypotheses, including the falsified ones

A — passing exits. Confirmed, with one correction: decide() has twook: true sites, not one. no-adr-diff (the zero-lookup clean path) and approved. reportVerdict has exactly two return 0 sites, both guarded by verdict.ok. Every route — gate mode, --pr, --files-json/--reviews-json, and merge_group — reaches its verdict through decide(); merge_group only affects PR resolution and the diff base, never the pass condition. Verified structurally and by driving the replay CLI (approved ⇒ exit 0; approved-then-revoked ⇒ exit 1; malformed review list ⇒ loud exit 1).

B — self-test. 27 assertions before, 36 after. Nine depended on the allowlist and were rewritten rather than re-pinned; the four "bot approvals do not count" style assertions inverted. Reverse verification (account filter restored on the committed file, direction predicted before running): exactly the 9 predicted assertions went red, by name — the four per-seat approvals, the unknown-account approval, the revocation pin, the cross-reviewer clearing pin, and both historical replays with an approval. The new pins assert something. Both directions the card required are pinned: an approval from a non-maintainer account passes, and a later CHANGES_REQUESTED still turns it red.

C — re-trigger. Confirmed against the workflow: pull_request_review: [submitted, edited, dismissed] already re-runs the gate on any approval, and permissions: pull-requests: read covers listing reviews from any account. No extra wiring, permission, or token scope is needed — noted in the trigger comment.

D — required-context registry: partly falsified. It does not merely name the check. scripts/check-required-contexts.mjs:196 carries prose restating the old rule ("only the maintainer own-account approval may land a docs/adr/** merge"), which this change makes false. That one string is updated; the context string itself is untouched. The field is documentation-only — nothing in the check reads it — so this is a self-consistency fix, not a behavioural one.

E — display name: confirmed, and left alone.ADR maintainer approval is both the required-context string in the main ruleset (#7022) and a fixture in check-required-contexts's own self-test, which pins that renaming the job goes red. Renaming here alone would leave the ruleset waiting for a context that never reports and hang the merge queue until the 60-minute timeout. The word "maintainer" now over-claims; a comment on the job says so and says why it stays. Correcting it needs an org-settings action first, so it belongs in a follow-up card, not here.

Verification

pnpm check:adr-merge-approval
✓ --self-test: 36 assertions over the real decide() / review-state / PR-resolution paths.
✅ No files under docs/adr/ in this diff (3 changed file(s)) — zero API lookups on the clean path.
pnpm check:required-contexts OK (9 registered contexts, adr-merge-approval among them)
pnpm check:node-version OK (27 setup-node steps, all Node 22)
pnpm check:shard-attestation OK (92 assertions)
pnpm check:workflow-status-functions OK (34 assertions; 24 workflows scanned)
pnpm check:changeset-gate-self-tests OK (118 + 153 + 117 assertions)
node scripts/check-changeset-no-major.mjs OK
node scripts/check-nul-bytes.mjs OK (7,425 text files, no control bytes)

The gate list was re-derived against the actual changed paths with node scripts/pm/dispatch-gates.mjs; it added check:node-version, check:shard-attestation, check:workflow-status-functions, check:changeset-gate-self-tests and check-changeset-no-major beyond the families the dispatch named. All run and green.

Changeset: route 2, skip-changeset. The diff touches only scripts/** and .github/workflows/** — no released package, so this PR declares no release of its own.

Merge ownership

⛔ Not merged, not enqueued, no auto-merge armed by this session — this changes the gate that constrains AI merges, so landing it is the maintainer's.

---

Generated by Claude Code

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 12, 2026 6:43pm