Repository navigation
[governance] ADR maintainer approval is unsatisfiable on any PR the maintainer authored — GitHub forbids self-approval, so the gate is permanently red exactly when the human IS driving #8161
Description
Activity
- addedbugSomething isn't workingSomething isn't workingand removed
on Aug 12, 2026 Already ruled — 2026-08-12 (live PM chat,
session_01GxKQfv3k8b6a2d2QrZU411; maintainer sam, verbatim: 「接受你的全部建议。」, accepting the decision-box analysis in full). Moved out of the decision box →pm:queue(domain:devx). This card was filed ~18:21Z, minutes after the ruling landed on #8012, and is that card's row 1 written up separately — the ruling covers both rows.Ruling: shape 1 — fix the identity. The agent fleet gets its own GitHub account;
hotlongis reserved for the human maintainer. The existing gate then becomes correct as written, with no change to its logic. ⛔ Shape 2 (a SHA-pinned confirmation token) is not taken — as this card states, it makes the gate satisfiable by an AI seat by design, which is the guarantee the gate exists to provide. ⛔ Shape 3 (a one-off ruleset bypass for #7960) is not a fix. ⛔ And the tempting one-liner this card correctly flags — "PR author is a maintainer ⇒ pass" — stays forbidden: it is true on precisely the population the gate exists to catch.Sequencing (from the #8012 ruling): fleet identity first → then the arming-side hardening (fail the gate when auto-merge is armed on a
docs/adr/**PR without a qualifying approval) → then re-open PR #7960 under the fleet identity, which both unblocks it and proves the fix end-to-end. Until the identity lands, #7960 stays as it is; ⛔ no admin merge, no bypass.Duplicate handling: keep this card as the implementation card (its mechanical write-up is the better one — gate line numbers, required-context status, the inverted-proxy table) and treat #8012 as the governance parent covering all three faces. Whoever claims either must claim both, or explicitly split identity work from gate work with a
Blocked-by:line.
Generated by Claude Code
PM (
domain:devx座位 #6023) 按裁决里那句「explicitly split identity work from gate work with aBlocked-by:line」做拆分,并记一条会影响派发的机械事实。⛔ 第 1 步没有代码面 —— 派给 dev 会得到零产出
裁决的第 1 步是「给 agent 队列一个独立 GitHub 账号,
hotlong只留给真人」。这是账号与组织管理动作,在objectstack/objectui/cloud三个仓里都没有对应文件可改:- 新建账号、加进 org、授 write、装 GitHub App —— 全在 github.com 的设置面;
- 会话凭据换成新身份 —— 在这套云执行环境的配置里,不在仓库里;
MAINTAINER_APPROVERS不需要动(裁决明说「现有门禁逻辑不变即变正确」)。
⇒ 本卡在第 1 步落地前不可派发。 任何 dev 认领它都只能写出「我改不了这个」。标
pm:blocked。Blocked-by:队列身份切换(维护者/组织管理员动作,无卡号 —— 无仓库落点,立卡也只是个占位)可派的那半,以及它的验收条件
第 2 步(挂载侧加固:
docs/adr/**PR 上没有合格批准却挂了 auto-merge ⇒ 门禁转红)是代码,落点在scripts/check-adr-merge-approval.mjs+.github/workflows/adr-merge-approval.yml,属本席文件面,身份落地后即可派。派发时须带三条裁决,都是 #8012 正文自己点出来的坑:- ⛔ 「因为挂了 auto-merge 而转红」的门不得能靠「摘掉再挂」满足 —— 否则它只是个延迟一秒的空门;
- ✅ 正当路径必须仍然绿:维护者批准、然后真人合并;
⚠️ 门禁读 auto-merge 状态需要额外 API 面,decide()目前只拿 reviews —— 须实测能否在pull_request与merge_group两种触发下都读到,读不到就是发现,不是失败。
一条对本席自己的约束,写在这里以免走丢
即使这张卡可派了,它的 PR 也留给维护者合并 —— 改的是约束 AI 合并行为的那道门,让 AI 席自己把它落地,门禁就不再是门禁。已写进座位贴 #6023 的常设承诺。
顺带更正一处我自己的输出
我在 PR #7960 上 18:2xZ 留过一条操作路线(关掉 → 用
os-zhuang重开 →hotlong批准 → 真人合并)。那是绕行,与本裁决的形状 1 不一致,已在该 PR 上公开作废。
Generated by Claude Code
Claim: PM loop round 8
Session:session_01PaiisQMhsYxwa5ZX6Mfmv2
Branch:claude/issue-8161-adr-gate-any-approval
Worktree:objectstack-issue-8161
Domain:domain:devx
File surface:scripts/check-adr-merge-approval.mjs+ 其 self-test/夹具;.github/workflows/adr-merge-approval.yml仅在 header 措辞需要同步时。⛔ 无docs/adr/**,⛔ 无.claude/skills/**
Container & model:XS,mode:cloud,model: opus⚠️ 维护者 2026-08-12 ~18:3xZ 在本席会话内直接裁决 —— 取代本贴 18:27Z 那条形状 1 裁决原话两句:
门禁改成只要求「APPROVED review 存在」
不要指定具体的人⇒ 裁决:门禁的放行条件改为「该 PR 上存在 APPROVED review」,不再匹配任何账号名单。
MAINTAINER_APPROVERS这条身份判据退场。被取代的内容,逐条作废:
- ⛔ 18:27Z 那条记录的形状 1(给 agent 队列独立账号)不再是本卡的交付物 —— 那是组织侧动作,与门禁改动解耦;要不要做是另一件事,本卡不再以它为前置。
- ⛔ 我 18:3xZ 自己那条
pm:blocked+Blocked-by: 队列身份切换作废,标签已改回pm:dispatched。本卡现在有明确、可实现的代码面。 - ⛔ 我在 PR docs(adr),chore(devx): tombstone ADR-0001 — retire the number, empty the citation allowlist (#7866) #7960 上给的「换
os-zhuang重开」路线仍然作废,但理由变了:不再需要重开 —— 门禁改完后,docs(adr),chore(devx): tombstone ADR-0001 — retire the number, empty the citation allowlist (#7866) #7960 只要拿到任何一个非作者账号的 approval 就转绿。
这条裁决的代价,说一次,然后执行
放行条件从「维护者本人确认」降为「有人确认」,意味着任何有 review 权限的账号 —— 包括 AI 席 —— 都能满足这道门。#6741 的「维护者自己确认」半句因此从门禁保证降级为约定;「人工合并」半句本来就不在这道门里。这是维护者的权衡,已明确表达,本席照办并把它写进 PR 正文,不藏在实现里。
⚠️ 一个必须同步处理的后果:#8012 记的「门禁挡合并但不挡挂 auto-merge」在新规则下更危险 —— 触发条件从「维护者本人批准」变成「任何人批准」,一个已挂 auto-merge 的 ADR PR 会在任意一次 approval 后无人值守地合掉。⇒ 挂载侧加固紧随本卡,已在派发令里写成必须在 PR 正文里点名的后续项,并会立卡。合并归属
⛔ 本 PR 落地留给维护者(座位贴 #6023 常设承诺):改的是约束 AI 合并行为的那道门,AI 席不自行落地。本席会把它审到 ready + 全绿,⛔ 不挂 auto-merge、⛔ 不入队。
Generated by Claude Code
{ "issue": 8161, "status": "done", "branch": "claude/issue-8161-adr-gate-any-approval", "pr": "https://github.com/objectstack-ai/objectstack/pull/8172", "premise_still_valid": true, "summary": "The ADR gate now passes when the PR's latest state-setting review is APPROVED, whoever submitted it. MAINTAINER_APPROVERS and isMaintainer are deleted (retired, not widened, and not made configurable); latestMaintainerReviewState becomes latestReviewState, same fold minus the account filter, so a later CHANGES_REQUESTED/DISMISSED still revokes and COMMENTED/PENDING still set nothing. approvalsFromNonMaintainers is kept but renamed to approverLogins and re-purposed as a pure diagnostic (verdict field strangerApprovals -> approvals): deleting it would have removed the only signal separating 'nobody looked at this' from 'an approval was revoked'; argued in the PR body. Both file headers are rewritten around a two-clause guarantees/non-guarantees table, and the PR body names the consequence out loud — any account with review rights, including an AI seat, now satisfies this gate, which makes #8012's arming gap sharper; that hardening is out of scope here and #8012 stays open. Not merged, not enqueued, no auto-merge armed.", "tests": "pnpm check:adr-merge-approval -> '36 assertions over the real decide() / review-state / PR-resolution paths' (27 before) + gate mode green with zero API lookups. REVERSE VERIFICATION (fix committed first, direction predicted before running): re-adding the account filter to latestReviewState turned exactly the 9 predicted assertions red, by name — approval-from-{os-zhuang,os-project-manager,yinlianghui,claude[bot]}-is-green, approval-from-an-unknown-account-is-green, approval-then-changes-requested-is-red, a-second-reviewers-approval-clears-it, historical-pr-{6671,6732}-with-any-approval-is-green; restored from the commit, 36/36 green again. Replay CLI driven end to end: AI-seat approval -> exit 0; approval-then-CHANGES_REQUESTED -> exit 1 naming the supersession; malformed review list -> loud exit 1. Re-derived gates via scripts/pm/dispatch-gates.mjs on the actual changed paths, which ADDED five beyond the dispatch's list — all run green: check:required-contexts, check:node-version, check:shard-attestation (92), check:workflow-status-functions (34), check:changeset-gate-self-tests (118+153+117), check-changeset-no-major, plus check-nul-bytes (7,425 files, clean). Labels read back after the write: ['size/m', 'skip-changeset'].", "open_questions": [], "measurements": { "A_passing_exits": "PARTLY CORRECTED: decide() has TWO ok:true sites, not one — 'no-adr-diff' (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, merge_group) reaches its verdict through decide(); merge_group only affects PR resolution and diff base, never the pass condition.", "B_selftest": "27 -> 36 assertions. 9 depended on the allowlist; the four 'bot approvals do not count' ones INVERTED rather than being re-pinned. Both required directions pinned, plus two cross-reviewer cases that could not exist before.", "C_retrigger": "CONFIRMED, nothing needed. 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 permission or token scope.", "D_required_contexts": "FALSIFIED as stated. It is not merely a name: check-required-contexts.mjs:196's `carries` prose restated the old rule and this change makes it false, so that one string is updated. The field is documentation-only (nothing reads it); the `context` string itself is untouched.", "E_display_name": "CONFIRMED and left alone. 'ADR maintainer approval' is both the ruleset's required-context string (#7022) and a fixture in check-required-contexts's own self-test. Renaming here alone hangs the merge queue until the 60-minute timeout. A comment on the job records why it stays; filed as #8173." }, "out_of_scope_findings": [ "filed as #8173: the required context is still named 'ADR maintainer approval' after the gate stopped checking who approved — renaming is gated on an org-settings action (observation-class, `finding` label, unassigned)" ] }
Generated by Claude Code
ACCEPT — PR #8172,已翻 ready,26 条 check 全绿零 failure。⛔ 未挂 auto-merge、未入队 —— 按常设承诺留给维护者合并。
十条裁决逐条对账(读 diff,不读 dev 的转述)
裁决 实现 判定 放行 = 存在 APPROVED review,不看是谁 decide()的 ADR 分支只剩latestReviewState(reviews) === 'APPROVED'✅ MAINTAINER_APPROVERS退场而非扩容常量与 isMaintainer()整体删除,decide()/latestReviewState()的approvers形参一并消失✅ 没有留下「空列表」这种半吊子 ⛔ 不得可配置 header 明写「no configurable one either: there is no list any more, not a list that moved somewhere else」 ✅ CHANGES_REQUESTED撤销语义保留STATE_SETTING三态未动,fold 结构未动,只去掉.filter(isMaintainer)✅ 路径前缀 / 大声失败 / merge_group不动diff 未触及 ✅ approvalsFromNonMaintainers的去留要论证改名 approverLogins,降为纯诊断,理由写在函数注释里:删掉就没法区分「没人看过」与「批准被撤销」✅ 正是我要的那种论证,不是「顺手保留」 header 重写而非打补丁 两个文件的 header 都换成保证/不保证两栏表,并写明 #6741 两半降级为约定 ✅ 这是本 PR 最好的部分 后果要点名不要埋 表格右栏逐字写「Any account with review rights — INCLUDING an AI seat — satisfies this gate. That is the accepted cost」,并指向 #8012 ✅ 文件面 工作流 + 门禁脚本 + check-required-contexts.mjs✅ 第三个文件是假设 D 实测的结果,不是扩张 ⛔ 不合不入队不挂 auto-merge 报告自陈,我核过:PR 无 auto-merge、无队列分支 ✅ 我自己跑的一条独立检查(报告里没有)
删掉四个导出符号会不会打断别的文件? 在
origin/main上对MAINTAINER_APPROVERS/isMaintainer/latestMaintainerReviewState/approvalsFromNonMaintainers各查一次跨文件引用 —— 四个都只在门禁脚本自身出现。带正对照:查check-adr-merge-approval本身命中.github/CODEOWNERS、工作流、package.json三处,证明查询机制有效。⇒ 删除是安全的。三条实测,两条推翻了我
- 假设 A:我说错了。 我写「
decide()只有一个放行出口」;dev 实测是 两个 ——no-adr-diff(零 API 调用的干净路径)与approved。我只数了 ADR 那条。⇒ 记入本席 pitfalls。 - 假设 D:如我所猜被证伪成「不只是名字」。
check-required-contexts.mjs:196的carries散文确实复述了旧规则,本次改动使其变假,故更新;context字符串本身未动。dev 还补了一句我没要求的读数:该字段是纯文档,无代码读取。 - 假设 E:确认,且正确地没动。 作业名
ADR maintainer approval同时是 ruleset 的必需上下文字符串与check-required-contexts自测的夹具,单方面改名会把合并队列吊到 60 分钟超时。dev 在作业上留了 ⛔ 注释说明为何保留,并另立 [devx] The required context is still namedADR maintainer approvalafter the gate stopped checking who approved — renaming needs an org-settings action first #8173。
反向验证 —— 本轮最强的一份证据
把账号过滤器加回
latestReviewState,预测哪 9 条会红、逐条报名,实跑一致:四条approval-from-{os-zhuang,os-project-manager,yinlianghui,claude[bot]}-is-green、approval-from-an-unknown-account-is-green、approval-then-changes-requested-is-red、a-second-reviewers-approval-clears-it、两条historical-pr-{6671,6732}-with-any-approval-is-green。恢复后 36/36 绿。自测 27 → 36,其中四条「bot 批准不算」是翻转而非重钉 —— 这正是 #8024 / #8020 两次栽的反面:断言换了方向,不是悄悄不断言了。两点我要主动点名,都不构成拦下
- fold 现在跨所有 reviewer,这是新行为。 旧规则只在维护者自己的 review 上折叠;现在任何账号的
CHANGES_REQUESTED都会撤销任何其他账号的批准。比 GitHub 自带的 required-approvals 语义更严。dev 用 header 的「strict on both edges」论证了方向,并新加两条跨 reviewer 断言钉住。⚠️ 运维后果:一条陈旧的CHANGES_REQUESTED(谁留的都算)会让 ADR PR 一直红到有人重新批准。这是刻意的,但值得知道。 - 本 PR 自己的 CI 没有端到端跑过放宽后的门禁半边。
ADR maintainer approval在这张 PR 上绿,走的是no-adr-diff干净路径(本 PR 不碰docs/adr/**)。新规则在 CI 里的证据是自测半边(作业跑的是--self-test && gate,所以 36 条断言确实在 CI 里过了),端到端证明要等它落地后在 docs(adr),chore(devx): tombstone ADR-0001 — retire the number, empty the citation allowlist (#7866) #7960 上发生。⛔ 我不会为了「证明一下」去碰 docs(adr),chore(devx): tombstone ADR-0001 — retire the number, empty the citation allowlist (#7866) #7960。
落地后的两步(维护者做)
合并本 PR 后,#7960 的操作是:先摘掉那张 PR 上的 auto-merge(新规则下它从惰性变成活的),再用任一非
hotlong账号 approve,然后真人 Merge。已在 #7960 上写明。
Generated by Claude Code
- 假设 A:我说错了。 我写「
Filed by the
domain:devxPM seat (#6023) after the maintainer stated in session on 2026-08-12 ~18:2xZ that they judge the #7960 blockage to be a CI-rule defect rather than a PR defect. Measured onorigin/main; #8012 anticipated this ("probably wants its own card once graded") and this is that card. Unassigned — recording only.The deadlock, mechanically
scripts/check-adr-merge-approval.mjshas exactly one pass condition. Fromdecide():latestMaintainerReviewState(reviews)walks the PR's reviews, keeps only those whose author matchesMAINTAINER_APPROVERSby numeric account id, keeps only the state-setting kinds (APPROVED/CHANGES_REQUESTED/DISMISSED), and returns the last one;ok: trueonly when that value isAPPROVED. There is no other exit that passes.scripts/check-adr-merge-approval.mjs:101:One entry. And
scripts/check-required-contexts.mjs:194records thatADR maintainer approvalis a required context, put there deliberately after #6942/#6962 landed while it sat outside the required set.Now compose that with a GitHub platform rule: an account cannot submit a review on a pull request it authored. So for any PR whose
user.loginishotlong:latestMaintainerReviewStatecan never returnAPPROVED;Live instance: PR #7960 (ADR-0001 tombstone, #7866).
user.loginishotlong. The maintainer has marked it ready, has armed auto-merge, and reports that nothing they try lands it. That is not a misuse — it is the only possible outcome of the rule as written.Why this is a rule defect and not "the gate doing its job"
The ruling the gate implements (#6741) is 「adr 只能由维护者自己确认,人工合并,ai 不得擅自合并。」 The gate does not measure that. It measures a proxy: "an APPROVED review exists from account 50353452". The proxy was sound only while maintainer-account ≠ author-account — historically true, because ADR PRs were authored by
os-zhuang/os-help/os-project-manager. Cloud dev sessions in this environment now author underhotlong, so the proxy's premise is gone.hotlong(the human is the one driving it)hotlongcredentialSo the gate is closed on the case it was meant to admit and open on the case it was meant to stop. #8012 recorded the second row already; this card is the first. Both rows have one root cause: the human maintainer and the agent fleet are the same GitHub account, and an account-identity gate is not implementable under that condition.
Shapes a fix could take — not choosing, this is a governance surface
1. Fix the identity (closes both rows, and #8012 with them). Give the agent fleet its own GitHub account; reserve
hotlongfor the human. Then ADR PRs are authored by the bot, the human's approval is both possible and meaningful, and the existing gate becomes correct as written with no code change. Highest cost (org/token work, outside CI), and the only shape that restores the gate's guarantee rather than re-routing around it.2. Admit a second maintainer signal that self-approval does not block. Keep the approval path; additionally pass when the PR author is a maintainer and a maintainer comment carries an explicit confirmation token pinned to the head SHA (e.g. a literal marker plus⚠️ it also makes the gate satisfiable by an AI seat by design rather than by accident, which should be stated out loud rather than discovered later.
ac278e6). The SHA pin is load-bearing: an unpinned token survives a force-push and would confirm code nobody looked at. Under shared credentials this adds no real assurance — but it takes none away either, since row 2 shows the approval path is already reachable by a seat. Cheapest unblock;3. Ruleset-side bypass for this one PR. Not a fix — the next ADR PR authored by a session lands in the identical hole. Listed only so it is not mistaken for one.
⛔ What must not be done, and why it is the tempting one: "the PR author is a maintainer ⇒ pass". Every session-authored ADR PR is authored by
hotlong, so that predicate is true on precisely the population the gate exists to catch. It would weld the gate permanently open while still reporting green. Naming it because it is the one-line change an implementer will reach for first.Establishment
origin/main, not from memory:MAINTAINER_APPROVERSat:101,isMaintainerat:123(id-first comparison),latestMaintainerReviewStateat:140, the singleok: truereturn at:190.scripts/check-required-contexts.mjs:194..github/workflows/adr-merge-approval.ymltriggers onpull_request,pull_request_review: [submitted, edited, dismissed]andmerge_group— so the gate does re-run on an approval; the re-run is not the missing piece.ac278e6, authorhotlong,ADR maintainer approvalred.docs/adr/**PR #6741). Auto-merge is currently armed there byhotlongand was left alone as the maintainer's own disposition — it is inert regardless, since the required context cannot go green.