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
Filed by the domain:devx PM 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 on origin/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.mjs has exactly one pass condition. From decide():
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;- the gate returns
ok: trueonly when that value isAPPROVED. There is no other exit that passes.
scripts/check-adr-merge-approval.mjs:101:
export const MAINTAINER_APPROVERS = [{ login: 'hotlong', id: 50353452 }];One entry. And scripts/check-required-contexts.mjs:194 records that ADR maintainer approval is 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.login is hotlong:
- the only account whose approval the gate accepts is the one account GitHub will not let approve;
- therefore
latestMaintainerReviewStatecan never returnAPPROVED; - therefore a required context is permanently red;
- therefore the PR cannot be merged through the normal path, or enter the merge queue, no matter what anyone does on it.
Live instance: PR #7960 (ADR-0001 tombstone, #7866). user.login is hotlong. 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 under hotlong, so the proxy's premise is gone.
| case | what the gate should do | what it does |
|---|---|---|
PR authored by hotlong (the human is the one driving it) |
pass once the human confirms | can never pass — self-approval is impossible |
PR authored by some other account, with an AI seat holding the hotlong credential |
refuse — an AI must not confirm on the maintainer's behalf | passes — the seat can submit the approving review itself |
So 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 hotlong for 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 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
- Gate logic read at
origin/main, not from memory:MAINTAINER_APPROVERSat:101,isMaintainerat:123(id-first comparison),latestMaintainerReviewStateat:140, the singleok: truereturn at:190. - Required-context status read from
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.- PR docs(adr),chore(devx): tombstone ADR-0001 — retire the number, empty the citation allowlist (#7866) #7960 state read by fresh fetch: open, not draft, not merged, not enqueued, head
ac278e6, authorhotlong,ADR maintainer approvalred. - ⛔ No action taken on docs(adr),chore(devx): tombstone ADR-0001 — retire the number, empty the citation allowlist (#7866) #7960 by this seat, and none will be: ADR-class PRs are not AI-merged, AI-enqueued or AI-auto-merged (⛔ Discipline: ADRs are confirmed and merged by the maintainer only — no AI seat may merge, queue, or auto-merge a
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.
Activity
hotlong commentedon 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
hotlong commentedon Aug 12, 2026
PM (
domain:devx座位 #6023) 按裁决里那句「explicitly split identity work from gate work with aBlocked-by:line」做拆分,并记一条会影响派发的机械事实。⛔ 第 1 步没有代码面 —— 派给 dev 会得到零产出
裁决的第 1 步是「给 agent 队列一个独立 GitHub 账号,
hotlong只留给真人」。这是账号与组织管理动作,在objectstack/objectui/cloud三个仓里都没有对应文件可改: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 正文自己点出来的坑:decide()目前只拿 reviews —— 须实测能否在pull_request与merge_group两种触发下都读到,读不到就是发现,不是失败。一条对本席自己的约束,写在这里以免走丢
即使这张卡可派了,它的 PR 也留给维护者合并 —— 改的是约束 AI 合并行为的那道门,让 AI 席自己把它落地,门禁就不再是门禁。已写进座位贴 #6023 的常设承诺。
顺带更正一处我自己的输出
我在 PR #7960 上 18:2xZ 留过一条操作路线(关掉 → 用
os-zhuang重开 →hotlong批准 → 真人合并)。那是绕行,与本裁决的形状 1 不一致,已在该 PR 上公开作废。Generated by Claude Code
hotlong commentedon Aug 12, 2026
Claim: PM loop round 8
Session:
session_01PaiisQMhsYxwa5ZX6Mfmv2Branch:
claude/issue-8161-adr-gate-any-approvalWorktree:
objectstack-issue-8161Domain:
domain:devxFile 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原话两句:
⇒ 裁决:门禁的放行条件改为「该 PR 上存在 APPROVED review」,不再匹配任何账号名单。
MAINTAINER_APPROVERS这条身份判据退场。被取代的内容,逐条作废:
pm:blocked+Blocked-by: 队列身份切换作废,标签已改回pm:dispatched。本卡现在有明确、可实现的代码面。os-zhuang重开」路线仍然作废,但理由变了:不再需要重开 —— 门禁改完后,docs(adr),chore(devx): tombstone ADR-0001 — retire the number, empty the citation allowlist (#7866) #7960 只要拿到任何一个非作者账号的 approval 就转绿。这条裁决的代价,说一次,然后执行
放行条件从「维护者本人确认」降为「有人确认」,意味着任何有 review 权限的账号 —— 包括 AI 席 —— 都能满足这道门。#6741 的「维护者自己确认」半句因此从门禁保证降级为约定;「人工合并」半句本来就不在这道门里。这是维护者的权衡,已明确表达,本席照办并把它写进 PR 正文,不藏在实现里。
合并归属
⛔ 本 PR 落地留给维护者(座位贴 #6023 常设承诺):改的是约束 AI 合并行为的那道门,AI 席不自行落地。本席会把它审到 ready + 全绿,⛔ 不挂 auto-merge、⛔ 不入队。
Generated by Claude Code
hotlong commentedon Aug 12, 2026
{ "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
hotlong commentedon Aug 12, 2026
ACCEPT — PR #8172,已翻 ready,26 条 check 全绿零 failure。⛔ 未挂 auto-merge、未入队 —— 按常设承诺留给维护者合并。
十条裁决逐条对账(读 diff,不读 dev 的转述)
decide()的 ADR 分支只剩latestReviewState(reviews) === 'APPROVED'MAINTAINER_APPROVERS退场而非扩容isMaintainer()整体删除,decide()/latestReviewState()的approvers形参一并消失CHANGES_REQUESTED撤销语义保留STATE_SETTING三态未动,fold 结构未动,只去掉.filter(isMaintainer)merge_group不动approvalsFromNonMaintainers的去留要论证approverLogins,降为纯诊断,理由写在函数注释里:删掉就没法区分「没人看过」与「批准被撤销」check-required-contexts.mjs我自己跑的一条独立检查(报告里没有)
删掉四个导出符号会不会打断别的文件? 在
origin/main上对MAINTAINER_APPROVERS/isMaintainer/latestMaintainerReviewState/approvalsFromNonMaintainers各查一次跨文件引用 —— 四个都只在门禁脚本自身出现。带正对照:查check-adr-merge-approval本身命中.github/CODEOWNERS、工作流、package.json三处,证明查询机制有效。⇒ 删除是安全的。三条实测,两条推翻了我
decide()只有一个放行出口」;dev 实测是 两个 ——no-adr-diff(零 API 调用的干净路径)与approved。我只数了 ADR 那条。⇒ 记入本席 pitfalls。check-required-contexts.mjs:196的carries散文确实复述了旧规则,本次改动使其变假,故更新;context字符串本身未动。dev 还补了一句我没要求的读数:该字段是纯文档,无代码读取。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 两次栽的反面:断言换了方向,不是悄悄不断言了。两点我要主动点名,都不构成拦下
CHANGES_REQUESTED都会撤销任何其他账号的批准。比 GitHub 自带的 required-approvals 语义更严。dev 用 header 的「strict on both edges」论证了方向,并新加两条跨 reviewer 断言钉住。CHANGES_REQUESTED(谁留的都算)会让 ADR PR 一直红到有人重新批准。这是刻意的,但值得知道。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