Skip to content

Commit c1d026e

Browse files
os-zhuangclaude
andauthored
feat(devx): gate .claude/skills/** on the ADR merge-approval check (#9395) (#9407)
Implements the maintainer ruling on #9319 decision 2 (2026-08-17): the `ADR maintainer approval` gate's path predicate extends to `.claude/skills/**`, so Prime Directive #14's human-merge reservation is machine-enforced on the lane's own protocol surface rather than declared. - the single ADR prefix becomes a GOVERNED_SURFACES table: identical pass conditions (an APPROVED standing + no armed auto-merge), distinct failure wording, each surface naming its own rule; - the #8012 armed-auto-merge clause reaches the new path class identically, with its own emptiness proof (identical reviews, opposite verdicts); - one path hit governs the whole PR -- a mixed diff is never judged by proportion, pinned by comparing a 1-of-4 verdict with a 1-of-1 one; - the report renderer is pure, so the "each surface names its own rule" requirement is asserted on the text an operator actually reads; - PR #9238 -- the skills-only PR the merge queue landed with zero reviews -- joins the historical replay fixtures as a real capture, pinned RED. The workflow needed no trigger change: it carries no `paths` filter (a path filter would skip the whole workflow and hang the queue on a required context that never reports), and #9238 already produced a completed `ADR maintainer approval` check run through the clean path. No existing check is weakened: every prior assertion still runs, and the self-test goes from 66 to 96 assertions. Fixes #9395 Co-authored-by: Claude <noreply@anthropic.com>
1 parent e0ff5b7 commit c1d026e

2 files changed

Lines changed: 563 additions & 83 deletions

File tree

.github/workflows/adr-merge-approval.yml

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,30 @@ name: ADR Merge Approval
33
# Machine enforcement of the 2026-08-12 ruling (maintainer, verbatim):
44
# 「门禁改成只要求「APPROVED review 存在」」/「不要指定具体的人」
55
#
6-
# A PR whose diff touches docs/adr/** must carry an APPROVED review before it
7-
# is mergeable. The gate does NOT check WHO approved: any account with review
8-
# rights on this repo — including an AI seat — satisfies it, which is the
6+
# A PR whose diff touches a GOVERNED SURFACE must carry an APPROVED review
7+
# before it is mergeable. The gate does NOT check WHO approved: any account with
8+
# review rights on this repo — including an AI seat — satisfies it, which is the
99
# accepted cost of the ruling and is stated in full in the guarantee table at
1010
# the head of scripts/check-adr-merge-approval.mjs. The approval must be
1111
# current: a later CHANGES_REQUESTED or DISMISSED revokes it.
1212
#
13+
# Two surfaces are governed, with identical pass conditions and distinct failure
14+
# wording (each red names its own rule):
15+
#
16+
# docs/adr/** — the ADR approval ruling above (#6741, #8161).
17+
# .claude/skills/** — Prime Directive #14's human-merge reservation, machine-
18+
# enforced since the 2026-08-17 ruling on #9319 decision
19+
# 2, verbatim: 「The `ADR maintainer approval` gate's path
20+
# predicate extends to `.claude/skills/**`, making Prime
21+
# Directive #14 enforced rather than declared」. The
22+
# measured bypass it answers: PR #9238, a skills-only PR
23+
# whose own body said it was awaiting a human merge, was
24+
# flipped ready and landed on `main` by the merge queue
25+
# with ZERO reviews. The published `skills/` catalog is
26+
# deliberately NOT included — the ruling named
27+
# `.claude/skills/**`, and the catalog's merge posture is
28+
# a separate decision nobody has made.
29+
#
1330
# Second clause (#8012): the PR must ALSO not be sitting on an armed
1431
# auto-merge. Arming is not merging — it is a standing instruction to merge
1532
# later — so no gate used to fire on it, and an AI seat armed one on a live
@@ -34,18 +51,35 @@ name: ADR Merge Approval
3451
# different AI-operated seats merged docs/adr/** PRs within the following hour
3552
# (#6671, #6732; the full record and both replays live in
3653
# scripts/check-adr-merge-approval.mjs and its --self-test). Both had ZERO
37-
# reviews of any kind, so both stay red under the widened rule too. Drafting
38-
# ADR PRs stays open to every seat; only the merge is gated.
54+
# reviews of any kind, so both stay red under the widened rule too. The same
55+
# measurement repeated on the skill surface nine days later (#9238, landed by
56+
# the queue with zero reviews), which is why that surface is now gated here
57+
# too — all three replay RED in the self-test. Drafting stays open to every
58+
# seat on both surfaces; only the merge is gated.
3959
#
4060
# Deliberately NO `paths` filter, on either trigger — the same choice
4161
# changeset-presence.yml made in objectui (#3769) and for the same reason
4262
# (objectui#3523): a path filter skips the WHOLE workflow, so the check
4363
# context is never CREATED on a non-matching PR, and a required context that
4464
# never reports leaves the PR pending in the merge queue until the ruleset's
4565
# 60-minute timeout. This gate reports on every PR instead: the script reads
46-
# the diff and decides, and a diff that does not touch docs/adr/** passes
66+
# the diff and decides, and a diff that touches no governed prefix passes
4767
# with zero API lookups.
4868
#
69+
# ⚠️ That is ALSO why extending the predicate to `.claude/skills/**` (#9395)
70+
# needed NO trigger change here — verified, not assumed, in two directions:
71+
# (a) structurally: no `paths`/`paths-ignore` filter exists on any trigger,
72+
# so path membership cannot affect whether this workflow runs at all;
73+
# (b) empirically: PR #9238 — a `.claude/skills/**`-ONLY diff — already
74+
# produced a completed `ADR maintainer approval` check run (job
75+
# 95293672713, conclusion `success` via the zero-lookup clean path). The
76+
# run existed before the widening; what changes is the script's verdict
77+
# on it, never whether it reports. Adding a `paths` filter for the new
78+
# prefix would REGRESS this into the never-reports failure above.
79+
# The `auto_merge_enabled` / `auto_merge_disabled` types below are likewise
80+
# path-agnostic, so the #8012 clause observes arming on the new surface on
81+
# exactly the same terms.
82+
#
4983
# This file is one of the enforcement surfaces .github/CODEOWNERS routes to
5084
# the maintainer: weakening or removing the gate is itself a governance
5185
# change and carries the same review requirement the gate enforces.
@@ -107,9 +141,10 @@ jobs:
107141
# spelling in scripts/check-required-contexts.mjs — renaming it here alone
108142
# leaves the ruleset waiting for a context that never reports, which hangs
109143
# the merge queue until the 60-minute timeout. The word "maintainer" now
110-
# over-claims (see this file's header: any approver counts); correcting it
111-
# is a settings action nobody in CI can perform, so it is tracked as
112-
# follow-up work rather than done here.
144+
# over-claims (see this file's header: any approver counts) and the word
145+
# "ADR" now under-claims (the skill surface is gated too, #9395); both are
146+
# settings actions nobody in CI can perform, so the name stays exactly as
147+
# the ruleset spells it and the drift is tracked as follow-up work.
113148
name: ADR maintainer approval
114149
runs-on: ubuntu-latest
115150
timeout-minutes: 5
@@ -129,11 +164,11 @@ jobs:
129164
node-version: '22'
130165

131166
# A checkout, setup-node, and one `node` call over `git diff` — no
132-
# install, no build. The self-test runs first (repo convention), then
133-
# the gate. GITHUB_TOKEN is only read on the gated path (a docs/adr/**
134-
# diff needs the PR's review list and arming state); the clean path does
135-
# zero lookups.
136-
- name: Require an APPROVED review and no armed auto-merge on docs/adr/** diffs
167+
# install, no build (the script imports node builtins only). The self-test
168+
# runs first (repo convention), then the gate. GITHUB_TOKEN is only read on
169+
# the gated path (a governed diff needs the PR's review list and arming
170+
# state); the clean path does zero lookups.
171+
- name: Require an APPROVED review and no armed auto-merge on docs/adr/** and .claude/skills/** diffs
137172
env:
138173
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
139174
run: node scripts/check-adr-merge-approval.mjs --self-test && node scripts/check-adr-merge-approval.mjs

0 commit comments

Comments
 (0)