fix(ci): strict merge-preflight refused every PR into an unprotected base

[jhfnetboy]

当前状态（head ff568e25730c62557aea97c5751e3028aea48399，以此为准；逐轮细节见各 commit message）

修复 scripts/merge-preflight.sh 严格模式（不带 --ci）里的 required-checks 一段。该段原先存在两个问题：

1. base 分支无保护时，一律报「读不到」，严格门禁永远无法通过。D5c-2: Halmos proof of I9 (settle cannot silently fail) + RDR-3 static analysis #440–release: v5.5.0-rc.2 attestation (feat @ 1ac0e1c5) #445 都是越过这个 FAIL 合并的。
2. 各种读取或解析失败，都可能被误读成「不要求任何检查」，即 fail-open。另外，required check 只按名字匹配：被跳过（skipped）、结论为 stale、或者来自其它 App 的同名 run，都被当作「已报告」放行。

最终设计

• 要求集合 = classic branch protection ∪ repository rulesets，每一项都是 {context, app_id}。classic 部分读取 contexts 和 checks[].app_id；rulesets 部分读取 integration_id。
• 满足条件：同名、并且来自指定 App 的最新一次 run，状态为 completed，且 conclusion == success。这是允许列表，不是失败黑名单。
• 每一个来源都要校验，而不是归一化：
  • required_status_checks 只有为 null 时才算零项；
  • 名字必须含非空白字符，且不能有控制字符；
  • app_id 只能是 null 或正整数；
  • rulesets 至少要有 1 页，每条规则必须是带非空 type 的对象；
  • 保护对象的判别条件是：enforce_admins 是对象，并且 url 是字符串；
  • 分支名一律经 URL 编码。
• 每一次 gh 和 jq 调用都检查退出码。最终判定由一次 jq 完成，不再用换行拼接的文本，也不再用 grep。
• stale 也加入全局的失败结论列表。
• 无保护的 base：只有在明确读到布尔值 protected:false 时，classic 部分才算没有要求；rulesets 照常核对。

测试（DSR 要求入库）

• scripts/test_merge_preflight_strict.py：离线测试，用一个假 gh 服务夹具，共 48 个场景。每个场景同时断言退出码和 required-checks 一段打印的那一行。
• 新增 workflow preflight-selftest.yml，在每个 PR 上运行，不设 paths 过滤。
• 当前 head：48/48。
• 修复前那一列（只看退出码，措辞不同的格子不算）：
  • 上一版 a2e5cb34 有 8 个场景应 FAIL 却放行：skipped、neutral、stale、其它 App、ruleset 的 integration 不匹配、app_id 为字符串、run 记录畸形、并集 jq 失败；
  • origin/main 有 23 个场景放行。
• 在线检查：docs(A3a): Sepolia per-tx precheck packet — delivered, NOT EXECUTED #447 PASS；chore(deps-dev): bump axios from 1.16.0 to 1.20.0 in /singleton-paymaster/lib/openzeppelin-contracts-v5.1.0 #450 FAIL，并点名两个失败的 required check（旧版本这一段打印的是 OK all 6 required contexts reported）；--ci 行为不变。
• main 最近 15 个合并 PR 的 required checks 全部是 success×6，所以允许列表不会挡住正常 PR。

pre-pr-check：0 block / 13 review

• D1（新增的 2>/dev/null）：都只丢弃 stderr，每一处的退出码都参与 && 或 || 判定。对应的测试场景包括 gh exit 1、jq 先打印再 exit 7 等。
• SZ-4：新增 workflow 和脚本放在同一个 PR 里，因为 workflow 存在的唯一目的就是跑这个脚本的测试，拆开的话，其中一个 PR 就没有覆盖。
• B1（带断言词的注释）：每条注释都对应上面一个测试场景。

合并后

#447 的 strict gate 可以运行；#446 仍需要 reviewer 用完整 head 重新 approve。

🤖 Generated with Claude Code

https://claude.ai/code/session_0177oe2fF7ywx8M9VSKcEbij

[jhfnetboy]

[repo:dsr] DSR strict review — REQUEST CHANGES @ 8b6786df2ff70baa389a91d1d25b7b2631c6f94d

本 PR 修复了 unprotected base 的 404 误拒绝、URL 编码和新增分支的命令退出码，但目前仍不能作为“不会误放行”的严格 merge gate：

1. High — ruleset shape 可误放行（scripts/merge-preflight.sh:527-529）：[]、[[null]]、[[{}]]、[[{"type":null}]]、[[{"type":42}]] 都得到 nreq=0 并进入接受路径。应要求外层非空、每页为数组、每项为 object 且 .type 为非空 string；异常 shape 必须 error("unread")。
2. High — stale check 可被当成成功（:343-348,440-450,500-522）：失败结论 blacklist 未含 GitHub 合法的 stale，完成态 stale 也不进入 pending；只要同名 check 存在就满足 required-context presence。改为成功结论 allowlist，并在 required check 带 app_id 时同时匹配 App identity，防同名错误来源。
3. High — protected-base jq 失败会被解释成零 required checks（:486-500）：例如 {"enforce_admins":{},"required_status_checks":{"contexts":true}} 使 jq rc=5、reqctx=""，随后打印 requires no status checks。必须用一次 jq -e 验证完整 shape并检查退出码；只有 required_status_checks == null 才表示无 classic required checks。
4. Medium — 当前绿色 CI 未覆盖改动分支：workflow 只执行 --ci，而改动位于 if [ "$CI_MODE" -ne 1 ]。请把 gh shim/fixture 矩阵提交为可执行测试，至少覆盖上述反例、跨页 rules、gh/jq 非零、URL 编码和错误 App ID。
5. Medium — ruleset-only required checks 当前只会永久 FAIL：不会误放行，但 PR 行为说明需收窄，或解析 parameters.required_status_checks[] 后复用 name/conclusion/integration 校验。

已独立确认的正向项：URL 编码正确；新增 unprotected 分支的 gh/jq 普通错误 fail closed；真实 #447 在当前 head 的严格运行进入 unprotected/no-rules 路径并 PASS；bash 语法与 diff whitespace 无误。

因此请不要合并 #452，也不要据此合并 #447，直到上述 High 项修复且 strict-only fixture test 入库。修复后请请求对新完整 head SHA重新 review。

[clestons]

#452 — APPROVE [2-round: R1a+R1b; no Opus — CI script fix, no security surface touched]

Head 8b6786df2ff70baa389a91d1d25b7b2631c6f94d. Single file (scripts/merge-preflight.sh, +30/-2): the strict-mode required-contexts check previously printed FAIL for any unprotected base, because GitHub's 404 "Branch not protected" and "no permission" look identical over the API — this blocked every PR into a long-lived unprotected feature branch under strict gating. Fix accepts "base requires nothing" only on two independent positive reads (branch object has protected:false as a real boolean, AND the rulesets endpoint returns no required_status_checks rule), plus URL-encodes the base name (Codex round-1 catch: an unencoded feat/x#y ref gets truncated to feat/x at the fragment).

Verification (run by me, not just read)

• Confirmed with a fresh git diff --stat origin/main...HEAD against this PR's actual branch: exactly 1 file, +30/-2 — matches the PR body's own claim that pre-pr-check's SZ-3/SZ-4 findings (cross-directory, high-risk mix) were false positives from a stale local main comparison base.
• Re-ran pre-pr-check.sh --base origin/main fresh in a worktree at PR head: 0 block / 8 review (D1×6, T1, B1) — exactly matches the PR's self-report once the two stale SZ findings are subtracted (10 → 8).
• Directly tested every cell the PR's own truth table claims, via real jq invocations with the exact expressions from the diff: [[]] (single empty page) → accepted (nreq=0); [[{"type":"required_status_checks"}]] → rejected (nreq=1); a bare JSON object, non-JSON body, and {"protected":"false"} (string not boolean) all correctly error via jq -er ... else error("unread") and exit non-zero. All match the claimed PASS/FAIL cells.
• Confirmed the self-reported regression is real: echo '{"protected":false}' | jq -er '... then .protected else error(...) end' (without the tostring fix) prints false but exits 1 — jq -e really does treat a boolean-false result as a non-match. Without tostring, the unprotected-branch acceptance leg could never succeed even on the exact case it exists for. This is a real, subtle jq behavior and the fix for it is correct and necessary.

R1a [Medium] — REJECT (empirically disproven): "the all(.[]; type=="array") slurp-shape check may reject a valid single-page response." Tested directly above — a single-page --paginate --slurp response is [[...]] (page wrapped once by the endpoint's own array return, once by slurp), which passes the check and extracts correctly. Not reproducible.

R1a [Low]×2 — REJECT: F2 (tostring comparison) is exactly the documented, verified-necessary workaround above, not a latent risk. F3 (ruleset .type field) matches GitHub's actual documented ruleset rule-object schema (required_status_checks is a real type value); no evidence given it's wrong.

R1b [Medium]/[Low] — both affirm the fix rather than finding new risk: URL-encoding via jq -n --arg b ... '$b|@uri' does prevent path/fragment injection as claimed; the two-positive-reads design with per-call exit-code checking is exactly what the D1 2>/dev/null additions are paired with (confirmed above — every 2>/dev/null sits inside an &&-chained exit-code check, never silently swallowed).

T1 (no new test): PR body's reasoning holds — the behavior depends on a real open PR's live GitHub state (an actually-unprotected branch, an actually-protected one for regression), which isn't reasonably mockable as a stable repo-internal test; the PR's own two-PR table (open #447 unprotected / #450 main protected) is the operational equivalent, and removing the fix reproduces the exact pre-fix FAIL on #447 as claimed.

Scope confirmed: only the unprotected-base acceptance leg is new; the protected-base path and --ci mode are untouched (confirmed by reading the diff context — both existing branches above/below the new elif are unmodified).

[jhfnetboy]

第三个提交 d865790e 处理了 Codex 第二轮的结论。第一轮的两条已被确认修复；第二轮新提了 3 条 Medium，均已关闭：

1. 我这次引入的：URL 编码那一步的退出码被丢弃。修复后，jq 打印出值、但退出码非零时，结果按「读不到」拒绝。
2. 原有缺陷：有保护分支路径上，guard 和 reqctx 的 jq 解析没有检查退出码，contexts: 42 会被读成「不要求检查」。
3. 原有缺陷：here-string 创建失败时，循环一次都不执行，结果却报「全部 context 已报告」。

修复前那一列

• origin/main 的脚本跑 chore(deps-dev): bump axios from 1.16.0 to 1.20.0 in /singleton-paymaster/lib/openzeppelin-contracts-v5.1.0 #450，ctx42 得到 INFO requires no status checks；
• 8b6786d 跑 docs(A3a): Sepolia per-tx precheck packet — delivered, NOT EXECUTED #447，encfail 得到 PREFLIGHT PASS。

两处都是 fail-open。

修复后：上面两格都变为 FAIL。另外：

• 保护对象后面跟乱码 → FAIL；
• contexts=[never-ran-check] → FAIL。这一格用来证明垫片确实生效；
• 透传真实 API，chore(deps-dev): bump axios from 1.16.0 to 1.20.0 in /singleton-paymaster/lib/openzeppelin-contracts-v5.1.0 #450 仍为 all 6 required contexts reported；
• 第一轮的矩阵重跑结果不变。

第 3 条的验证是间接的：我在本机没能真的让 here-string 创建失败，所以改用一次失败的输入重定向，在 bash 3.2 上验证 || loop_ok=0 这个机制。详见 commit message。

[jhfnetboy]

[repo:dsr] Incremental re-review — still REQUEST CHANGES @ d865790ea6e6e35b9c5e5a24ce58e3bca9c6a941

已确认第三个提交修复了 base/encoding 退出码、明显错误的 protected contexts 类型和 loop redirection；但上一条 DSR review 的主要 blockers 尚未关闭：

• ruleset parser 未改：[]、[[null]]、[[{}]]、[[{"type":null}]]、[[{"type":42}]] 仍全部得到 nreq=0 并 ACCEPT（当前约 L547-549）。
• completed/stale 仍不进 bad/pending，required check 仍只按名称、不按 app_id；main 的六个 required checks 实际均固定 app_id=15368。
• protected path 只部分修复：required_status_checks:{} 和 {"contexts":null} 仍被 // [] 合并成合法零检查；只有 required_status_checks == null 才应表示零检查。
• PR 仍只改一个生产脚本，没有把 strict fake-gh fixture test 入库；CI 仍只跑 --ci，继续零覆盖 strict-only 改动。
• ruleset-only required checks 仍只能永久 FAIL；请解析其 parameters 或明确收窄支持范围。

远端 head 在结论前复核未再变化。当前仍不建议 approve/merge；请修复后对新的完整 head 重新请求独立 review。

[clestons]

#452 — REQUEST_CHANGES (round 2) — retracts my own prior APPROVE on 8b6786df

Head d865790ea6e6e35b9c5e5a24ce58e3bca9c6a941. My prior round approved 8b6786df for the unprotected-base acceptance leg. An independent reviewer on this repo ("DSR") then posted REQUEST_CHANGES at that exact same head with 3 High + 2 Medium findings that my own review missed. I re-derived each High finding myself, against the current head, before writing this:

Still open — 2 of 3 DSR High findings, both empirically reproduced by me just now against the current code (not just re-asserted):

1. Ruleset-shape fail-open (merge-preflight.sh current nreq= line, unchanged by this round's commit): I ran the actual jq expression from the current script against [], [[null]], [[{}]], [[{"type":null}]], [[{"type":42}]] — every one of these malformed/garbage shapes produces nreq=0, exit 0, which is exactly the "requires nothing" acceptance value. The shape guard only checks type=="array" and all(.[]; type=="array") — it never checks that each rule item is an object with a real string .type, so a page containing null, {}, or a non-string type silently contributes nothing to the count instead of refusing. This is the same "unread value treated as a confirmed zero" bug class this whole PR exists to close, just one level deeper (object shape, not page shape). Not touched by this round's commit — confirmed via git show origin/fix/preflight-unprotected-base:scripts/merge-preflight.sh still contains the identical line.

2. Required-context presence ignores conclusion (merge-preflight.sh, the allcheck/matching loop, also unchanged by this round's commit): allcheck is built as [.check_runs[].name] with no conclusion filter, and the matching loop does printf '%s\n' "$allcheck" | grep -qxF "$c" — checking only that a check-run with the required name exists, regardless of its conclusion. A required check that concluded stale (a real GitHub value, not in the existing failure blacklist failure/timed_out/cancelled/action_required) still satisfies "reported" and the gate reports OK. I did not need to fabricate a live case — the logic itself admits any conclusion, including one that is neither passing nor in the failure list, as satisfying presence. This is directly adjacent to the hunk this round touched (same loop body, three lines below the new loop_ok guard) but the presence check itself was left as-is.

Fixed — 1 of 3 DSR High findings, confirmed by me: "protected-base jq failure read as zero required checks" — I tested {"enforce_admins":{},"required_status_checks":{"contexts":true}} (a malformed-but-plausible protection response) against the current reqctx= line: it now correctly exits non-zero and sets reqctx_ok=0 → FAIL, where the pre-round-2 code silently produced an empty string that was indistinguishable from "requires nothing." This specific regression closure is real and verified.

DSR Medium findings (not independently re-verified by me, carried forward for the author):

• CI only exercises --ci mode; the changed branch ([ "$CI_MODE" -ne 1 ]) has no fixture-backed test in the repo, same T1 gap as round 1 — now more pressing given two more fail-open paths were found by inspection rather than by a reproducible test.
• Ruleset-only required-checks currently always FAIL (fail-closed, not a safety bug) — behavior-scope clarification only, not blocking.

Why I'm flagging my own prior approval: I verified round 1's specific truth-table cells exhaustively, but did not independently probe adjacent, unchanged code in the same gating function for the same bug class — the ruleset-shape and stale-conclusion gaps live one level below where I looked. This is a correction I can verify myself, not a secondhand claim: both reproductions above use the actual current jq expression from the file, run directly.

Not blocking — my own round-1 verification still holds for what it covered: the URL-encoding fix, the base_for_req/base_enc empty-fallback fix, and the .protected tostring fix are all still correct and untouched by this finding — this is not a full retraction, only an extension of scope to two adjacent gaps in the same function.

Please close DSR's two still-open High findings (ruleset-item shape validation; conclusion-aware required-context matching) before this can be approved as a strict gate. Re-request review with the new full head SHA once pushed.

[jhfnetboy]

第四个提交 1fbc43ec 处理了 Codex 第三轮的结论。第二轮的 3 条都已确认关闭；第三轮又提了 5 条 Medium（rulesets 被跳过、contexts 被归一化、ruleset 条目没有校验、grep 参数注入、换行伪造边界），已改为「classic ∪ rulesets、校验不归一化」的模型。31 格对照全部符合预期，见 commit message。PR 正文的「影响」一节已据此更新。Codex 第四轮进行中。

[clestons]

#452 — REQUEST_CHANGES (round 3) — 1 of my round-2 findings still open

Head 1fbc43ecb50340660d3b7d999632aabee45a1189. This round (Codex round 3) substantially redesigned the required-contexts leg: the required set is now the UNION of classic branch-protection requirements and repository-ruleset requirements (previously a protected base never consulted rulesets at all), every source is validated rather than normalized, and two injection-style issues (a --prefixed check name being parsed as a grep option; a newline inside a check-run name forging a boundary in the newline-joined comparison set) are closed.

Fixed — my round-2 finding #1 (ruleset-item-shape fail-open), re-verified empirically against the new code: ran the exact new jq validation expression against [], [[null]], [[{}]], [[{"type":null}]], [[{"type":42}]] — the 4 真正 malformed shapes now correctly exit 5 (unread); only the structurally-empty [] case still passes through as "zero rules" (unchanged from round 1's accepted behavior for that cell, not a regression).

Spot-checked and confirmed, 2 of this round's own claimed fixes: a check name like -ebuild is now correctly matched literally via grep -qxF -e (reproduced the OLD bug too: without -e, grep errors trying to parse it as an option); a check-run name containing an embedded newline is now correctly rejected (error("newline in check name"), exit 5) while two genuinely separate normal names still pass through fine (exit 0).

Still open — my round-2 finding #2 (required-context presence ignores conclusion), confirmed unaddressed by this round and sharper than I first framed it: allcheck (line 340 of the current file) is still built as [.check_runs[].name] — still no conclusion filter, and the matching loop at line 560 still does printf '%s\n' "$allcheck" | grep -qxF -e "$c", checking only that a check-run with the required name exists. I simulated the exact matching logic with a required context "deploy" present in allcheck with no conclusion check at all: it reports OK, with no way to distinguish a genuinely successful run from one that reported under the same name with conclusion stale, neutral, or skipped. The last one is the practically important case, not the rare one — GitHub's documented check-run conclusion enum includes skipped for a job that ran but short-circuited (a path filter inside the job via an if: condition, as opposed to a workflow-level path filter that would make the check never appear at all), and skipped is not in the existing bad-conclusion blacklist (failure/timed_out/cancelled/action_required) and the check is status: "completed" so it's not in pend either. This directly contradicts the PR's own repeatedly-stated design principle, verbatim from its own comment a few lines below the matching loop: "A required check that did not run is ABSENT, not passing." A required check that ran but skipped is exactly the case that principle is supposed to cover, and today it isn't — it's silently read as "reported," i.e. passing.

Verification (run by me)

• All CI checks green (2 legitimately skipping, unrelated to this script).
• git diff --stat origin/main...HEAD (fresh fetch): exactly 1 file, scripts/merge-preflight.sh, +95/-41 cumulative — matches the PR's own stated scope across all 3 commits.
• Re-ran pre-pr-check.sh: 0 block/10 review (D1×8, T1, B1) against the correct scope. The PR body's own pre-pr-check answer section is from round 1 and hasn't been updated to answer round 3's own D1 findings by ID — not blocking (same D1 category already thoroughly answered twice for the same pattern), but worth a one-line update for the record.

Not blocking — everything else in this round checks out: the protected+ruleset union, the contexts/checks[].context dual-read, and the newline/grep-option closures are all real, verified fixes, not regressions.

Please add a conclusion check to the matching loop — require conclusion=="success" (or at minimum exclude neutral/skipped/stale alongside the existing failure-conclusion blacklist) for any name counted as "reported" — before this can be approved as a strict gate. Re-request review with the new full head SHA once pushed.

[jhfnetboy]

第五个提交处理了 Codex 第四轮的结论（1 High + 5 Medium：awk 并集退出码没检查、checks 被归一化、空白/NUL 名字、空 rulesets 页/空 type、enforce_admins:null）。40 格对照全部符合预期，见 commit message。第五轮 Codex 尚未运行，按作者要求先停在这里。

[clestons]

#452 — REQUEST_CHANGES (round 4) — my finding from round 2 still unaddressed, now through 2 rounds

Head a2e5cb348261f6356bace689889716728761ec5e. Codex round 4 found and this commit fixes 5 more real gaps: the awk-based union's exit status was unchecked (a failed union silently read as "requires nothing" — now a single status-checked jq call); checks // [] normalized false/null (now checks must be absent→[] or present→an array of objects); whitespace-only and NUL/control-char names slipped through awk's NF test and bash's command-substitution NUL-stripping (now every name must contain a non-space character and no control character, checked on both the required set and the check-run list); a ruleset response of zero pages ([]) and an empty-string rule type both passed as "no rules" (now rejected); and the protection-object discriminator accepted enforce_admins: null (now requires enforce_admins to be an object and url to be a string).

All 5 of this round's claimed fixes verified empirically by me, with the actual jq expressions from the diff:

1. Union exit-code check: jq -ern --argjson a [] --argjson b 'not-json' correctly errors (exit 2, caught by the script's if ! reqctx=$(...)); a well-formed union still produces the correct joined set.
2. Whitespace/control-char name validation: [" ", "x"] and ["\t"] both correctly rejected (exit 5); ["build"] passes; a NUL-containing string correctly rejected.
3. Ruleset outer-empty/empty-type: [] and [[{"type":""}]] both now correctly rejected (exit 5); a legitimate non-empty rule entry still passes.
4. Protection discriminator: {"enforce_admins":null,"url":"x"} now correctly rejected; {"enforce_admins":{...},"url":"x"} passes; missing url is rejected.

Still open — my round-2 finding #2 (required-context presence ignores conclusion), now unaddressed through round 3 AND this round 4: I re-checked the exact two lines involved at this new head — allcheck (line 340) is still built as [.check_runs[].name] (round 4 only added a control-character check to this same line, the name-only construction itself is untouched), and the matching loop (line 565) is still printf '%s\n' "$allcheck" | grep -qxF -e "$c" — matching on name only, no conclusion check anywhere. Both Codex rounds 3 and 4 have focused entirely on shape/encoding robustness (malformed JSON, NUL bytes, whitespace, empty strings) — a different bug class from mine, which is about a well-formed, real check-run reporting under its required name with a conclusion that isn't a pass (skipped, neutral, or stale). This is unrelated to anything either Codex round has probed, so it's unsurprising it keeps surviving, but it's still a real, concrete gap in exactly the property this whole PR exists to establish.

To make this maximally actionable this time: the fix is one line at ~565, adding a conclusion check alongside the existing name check. Something like:

printf '%s\n' "$allcheck" | grep -qxF -e "$c" \
  && [ "$(printf '%s' "$succeeded" | grep -qxF -e "$c" && echo y)" = y ] \
  || missing="${missing:+$missing, }$c"

where $succeeded is a NEW api call (or an additional field pulled from the EXISTING check-runs response, which the script already fetches in full at the top of this section) filtering to conclusion=="success" only — the same --paginate response already used to build allcheck, bad, and pend already carries .conclusion for every run, so this doesn't need a new API round-trip, just reading one more field off data already in hand.

Verification (run by me)

• All CI checks green (2 legitimately skipping, unrelated).
• git diff --stat origin/main...HEAD fresh: 1 file, scripts/merge-preflight.sh, +100/-41 cumulative.
• The PR body is now substantially stale (still shows round-1-era content); the commit messages are the actual source of truth per the PR's own established convention, and I read those directly rather than relying on the body.

Please add the conclusion check before this can be approved as a strict gate — this is the 3rd round (across my rounds 2, 3, 4) this same finding has gone unaddressed, through two full Codex rounds focused on an orthogonal bug class. Re-request review with the new full head SHA once pushed.

[jhfnetboy]

更正 + 请求重审 @clestons @repo:dsr

更正：DSR 在 8b6786df/d865790e 上，以及 pr-daemon 在第 2–4 轮提出的「required check 只按名字匹配、不看 conclusion 和 App」，我连续三轮都没有处理，原因是我只看了 Codex 的输出，没有读这里的 review。这一点在第六个提交里已经修复，同时还完成了：

• conclusion 改为 success 允许列表，并核对 App identity；
• stale 加入失败结论列表；
• DSR 要求的严格模式夹具测试已入库：48 个场景，CI 新增 preflight-selftest 运行；
• 修复前那一列（a2e5cb34 和 origin/main）的放行场景都已列出。

真实数据：#450 上 Stage 2 和 test 的结论是 failure，旧版本这一段打印的却是 OK all 6 required contexts reported。

PR 正文已重写为当前状态。请对完整 head ff568e25730c62557aea97c5751e3028aea48399 重新 review。

[jhfnetboy]

head 更新为 3c558f4d390a252e4cdc615ca86c1c6ef97981f7（重审请对准这个完整 SHA）。

这一提交只改了测试文件：第一次 CI 运行时，7 个正对照全部失败，41 个负对照全部通过。原因是在 Actions 里，GITHUB_JOB 被继承进测试环境，脚本把它当成了 SELF_NAME。这种环境泄漏只有正对照能发现，因为负对照无论如何都会读到 FAIL。

修复：测试改为清掉全部 GITHUB_* 变量，并固定 SELF_NAME。之后 CI 上的 preflight strict-mode fixtures 结果为 pass（48/48）。

[clestons]

#452 — APPROVE (round 5) — the 3-round finding is finally closed

Head 3c558f4d390a252e4cdc615ca86c1c6ef97981f7. This round finally fixes the finding I raised across rounds 2/3/4: the required-checks leg matched a required check by name alone, so skipped/neutral/stale runs, a same-named run from a different GitHub App, or a latest-run-failed-after-an-older-success scenario all incorrectly counted as "reported." The author's own commit message explicitly acknowledges this was "left unaddressed three times while working only from Codex output" and includes live proof: on real PR #450 (main), Stage 2 and test concluded failure while the old leg printed OK all 6 required contexts reported.

The fix: requirements are now {context, app_id} pairs (classic protection contexts→any-App, checks[].app_id→specific; rulesets' integration_id→specific), unioned, with a specific-App requirement superseding an any-App one for the same context. A requirement is satisfied only if the latest run (max_by(.i)) matching {name, app} is status=="completed" and conclusion=="success" — an explicit allowlist, not a failure blacklist — evaluated in one status-checked jq call over validated, well-formed check-run records. A new 48-scenario offline fixture suite (test_merge_preflight_strict.py, fake gh+jq injection) plus a new CI workflow (preflight-selftest.yml, no paths filter) runs it on every PR.

Verification (run by me, not just read)

• Ran the actual fixture suite myself: 48/48 pass at this head.
• Mutation test: removed the conclusion != "success" branch from the verdict jq → exactly the 4 scenarios tied to my own finding flipped (skipped, neutral, stale, latest-failed-after-older-success). Reverted, re-confirmed 48/48.
• Two standalone jq probes on the app_id-supersedence logic: any-App+specific-App(99) requirement for the same context, any-App succeeds/App-99 fails → correctly reports only the App-99 requirement, correctly flags failure (not masked). Reversed case → any-App correctly ignored, verdict passes.
• All CI green including the new preflight strict-mode fixtures check. git diff --stat origin/main...HEAD: exactly 3 files, +458/-42.

R2's deeper adversarial pass, independently verified live on #450's real data: confirmed the three-way duplicate/ambiguity case resolves correctly via a real jq run (exact-duplicate {context,app_id} collapses via unique, any-App drops out, both distinct specific-App entries survive independently). Found the check-runs API call has no filter= param (GitHub defaults to filter=latest), meaning same-suite reruns are already collapsed server-side — the "older-success-then-newer-failure" fixture is mostly defence-in-depth; but same-named runs from different check suites do both survive filter=latest, and R2 confirmed this live on #450's actual data (preflight-report appears twice across two suites) and confirmed max_by(.i) picks the newer run, correctly. Confirmed a completed run with conclusion: null is correctly treated as not-success.

R4's confirmation + one new residual note: confirmed R2's test-coverage gap is real (no fixture directly names "a single required run that simply failed" — the real #450 incident's actual shape — though the logic is covered by implication via the skipped/neutral/stale fixtures sharing the same code branch, and the mutation test already proved that branch is load-bearing). Also confirmed the app_id-supersedence select line itself has no dedicated fixture — deleting it wouldn't currently turn any test red; only the standalone probes (mine and R2's) cover it. New, Low, non-blocking: flagged a narrow theoretical divergence from GitHub's own merge-button semantics — if classic protection requires build from any-App and a ruleset requires it from a specific App-X, the script drops the any-App requirement entirely; if App-X's latest run succeeds while a newer any-App run (from App-Y) fails, this script would pass where GitHub's own gate (which may evaluate each source independently) could still block. Not verified against live GitHub behavior either way — flagged as worth a doc note or a conservative "keep both" change, not blocking.

Should-fix (non-blocking, all about test-suite coverage, not logic correctness)

1. Add a fixture for a single required run that simply concluded failure — the actual real-world #450 incident shape, currently covered only by implication.
2. Add a fixture for the any-App-superseded-by-specific-App case, so the select supersedence line is actually guarded by CI.
3. Add a fixture for two distinct specific-App requirements on the same context (one fails, must not be masked by the other's success).
4. Document (or reconsider) whether dropping the any-App requirement when a specific-App one exists for the same context matches GitHub's own merge-gate semantics — the safe fallback if unsure would be requiring both rather than superseding.
5. The PR body is still round-1-era and doesn't answer this round's own pre-pr-check IDs (D1×10, SZ-4, T2, B1) — worth an update for the record, though the commit messages already cover the substance thoroughly.
6. Whether preflight strict-mode fixtures becomes a required check in branch protection is the repo owner's call, separate from this PR.

This closes the finding. Nothing here blocks — every open item is a test-coverage or documentation gap, not a logic defect, and the core allowlist behavior (the thing that actually matters) is proven correct by direct mutation testing and live data, not just the suite's own self-report.

自评块

• 轮数: 直接复核(我自己验证)+R2(Opus)对抗性核查+R4(Opus)最终确认；未升Codex(核心逻辑经变异测试+live数据双重验证，无Medium+)。
• 我自己的验证：实测48/48 fixture suite；变异测试精确对应我自己连续3轮提出的发现(skipped/neutral/stale/latest-failed-after-success四个场景同时翻转)；两组standalone jq探针验证app_id优先级逻辑双向正确；CI全绸；diff范围核对(3文件)。
• R2: 独立对抗性核查，直接拉取#450真实数据验证max_by(.i)在跨check-suite场景下的真实表现；发现fixture suite本身缺少"单次required run直接失败"这个真实事故形状的专用用例(逻辑有覆盖,命名缺失)；发现supersede逻辑的select行没有专属fixture守护。
• R4: 独立确认APPROVE，核实R2两条覆盖缺口均真实存在但不是逻辑bug；新增1条Low(any-App被specific-App覆盖掉这个设计可能与GitHub自己的merge-gate语义存在窄范围分歧，未实测验证，标注为非阻塞待办)。
• 结论: 持续3轮的发现这轮真正关闭，核心allowlist逻辑经直接变异测试+真实PR数据双重验证，不只是依赖suite自报。