Repository navigation
ci: fail Lint & Static when the base moved the lint gate - #11383
Conversation
The lane checks out refs/pull/N/head by deliberate trade-off (the merge ref is rebuilt asynchronously and served stale), so a green run proves the branch passes the gate AS THE BRANCH DEFINES IT, not as the base defines it. #11117 flipped the Prettier lane from --write to --check on main while #11216's branch still ran --write; it merged green, main went red, and every later full-profile PR failed lint for a reason outside its own diff (#11378 cleaned up). Nothing else catches this: the ruleset requires reviews, not status checks, and the merge queue is off. Add a "Check lint gate freshness" step before the expensive steps: for each gate-defining file (lint.js, the eslint/prettier configs, ci.yml, the check itself), compare the base's latest commit touching it against the PR head via the swapped three-dot compare; a "diverged" or "behind" verdict fails the lane with the offending commits and the rebase remedy. package.json/lockfile are excluded deliberately: at 50-100 merges a day that would re-create the update treadmill, and a prettier/eslint bump cannot sneak semantics past a branch anyway since both run repo-wide. API errors fail open with a warning — an advisory gate must not hold every PR hostage during an API brownout. The check runs its decisions through an injected fetchJson so the suites stay network-free; the wiring test pins the step's gate, position and env, and the gate-file list is mutation-witnessed. Verified against the live API: a head at origin/main passes, a head at the parent of the last ci.yml change fails naming that commit, and missing/bad tokens warn without failing.
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Read-only review pass at head 388434da. This PR has no recorded blocking finding — no Critical was ever posted on it — so this pass is an independent Critical-only scan. I read the complete production diff (the 209-line check script and the 21-line workflow hunk; the other 194 lines are its node:test suite) plus the job's token scope, and found no Critical. Approving.
The load-bearing containment logic is correct. findStaleGateFiles asks for the newest base-side commit per gate file (/repos/{repo}/commits?path=…&sha={baseRef}&per_page=1) and then tests whether the branch contains it with the swapped three-dot compare /repos/{repo}/compare/{baseGateSha}...{headSha}, reading contained = status === 'ahead' || status === 'identical'. That matches GitHub's compare semantics — the status describes the second ref relative to the first, so ahead/identical mean the gate commit is an ancestor of the branch head, while behind and diverged both mean the base's gate change never reached the branch and are correctly reported as stale. The direction of the error is also right in the two edge cases that matter: a gate file that never existed on the base yields no commit and is skipped rather than treated as stale (if (!commit) return null), so a branch that adds a gate file — this PR itself — is not self-blocked; and path/baseRef are URL-encoded while the two SHAs are hex, so no query can be malformed by a branch name.
The gate really blocks when it fires, and really yields when it cannot know. main() prints the report to stderr and sets process.exitCode = 1, so a stale branch fails the step and therefore the job. Every path where the check cannot reach a verdict exits 0 with a ::warning:: left in the log — missing GITHUB_TOKEN/BASE_REF/HEAD_SHA/GITHUB_REPOSITORY, and any throw from the API calls, since curl -f turns an HTTP error into a non-zero exit that execFile rejects and the outer try/catch catches. For an advisory gate in a repo whose ruleset requires reviews rather than status checks, fail-open on a brownout is the correct direction: it cannot hold every PR hostage, and its absence degrades to today's behaviour rather than to a red tree.
Scoping is sound. The step runs only on pull_request, and is skipped when skip_ci is true or the profile is docs_only; the comment's reasoning holds — merge_group validates the prospective merged tree and push validates the base itself, so neither event has a stale green to catch, while github_ci_only runs the lint helper step and stays covered. It sits ahead of Node setup and install, so the cost is a handful of API calls before the expensive work: two per gate file, seven files, well inside the GITHUB_TOKEN budget. The header also explains why the per-file commits?path= lookup is used instead of reading one compare's file list — that list caps at 300 files, and a branch a few days behind a repo merging 50-100 PRs a day would blow past the cap and silently drop gate files from the scan, which is exactly the silent-miss failure this check exists to prevent.
Token hygiene and bounds are handled. The request goes through execFile with an argument array, so nothing is shell-interpolated; --max-time 30 and a 16 MiB maxBuffer bound it; and the catch deliberately rebuilds the error from curl's exit code and last stderr line, because execFile's own message embeds the full argv — bearer token included — which Actions masks but a local run would print. That is the right instinct and it is documented at the site.
The token scope the step needs is present, so it is not a silent no-op. The Lint & Static job declares its own permissions: block granting contents: 'read' (ci.yml:862-863), which is what both endpoints require; a job-level block overrides the workflow-level one, so this was worth checking rather than assuming.
Test wiring is complete. check-lint-gate-freshness.test.mjs was added to both HELPER_TESTS_DEP_FREE and HELPER_TESTS (ci.yml:89-90), so it runs in the dependency-free fast lane and the full helper lane; the suite uses node:test only, which is what makes the dep-free lane correct for it. The one thing I could not verify from the diff is whether node resolves on every runner before the "Set up Node.js" step — hosted runners ship it and the adjacent ECS comment says the machine's Node is reused, so it should, and a miss would fail loudly on the first PR rather than silently. I did not treat that as a Critical.
CI at this head. 16 checks pass with 50 skipped and nothing failing or pending, so the lane this PR modifies is green on the branch that modifies it. The recorded rationale — that a green run here proves the branch passes the gate as the branch defines it — is precisely the gap the check closes, and excluding package.json/the lockfile is reasoned rather than arbitrary: both tools run repo-wide, so a version bump must bring the whole tree under the new semantics in its own run, whereas a change to how the gate runs can slip past a branch untouched.
中文说明：本次在 head 388434da 上评审。该 PR 没有任何历史阻塞记录（从未有人在此提出 Critical），因此本轮是独立的 Critical-only 扫描：我读完了全部生产 diff（209 行检查脚本与 21 行工作流改动，另外 194 行是其 node:test 套件）以及该 job 的令牌权限，未发现 Critical，故提交 Approve。承重的包含关系判定是正确的。 findStaleGateFiles 先按文件取 base 侧最新提交（/repos/{repo}/commits?path=…&sha={baseRef}&per_page=1），再用交换后的三点比较 /repos/{repo}/compare/{baseGateSha}...{headSha} 判断分支是否包含它，并以 contained = status === 'ahead' || status === 'identical' 判定。这与 GitHub 比较接口的语义一致——status 描述的是第二个 ref 相对第一个 ref 的位置，因此 ahead/identical 表示该门禁提交是分支 head 的祖先，而 behind 与 diverged 都表示 base 的门禁改动从未到达分支，被正确判为陈旧。两个关键边界的出错方向也是对的：从未在 base 上存在过的门禁文件取不到提交，会被跳过而不是判为陈旧（if (!commit) return null），因此新增门禁文件的分支——也就是本 PR 自己——不会自我阻塞；path 与 baseRef 都做了 URL 编码，而两个 SHA 是十六进制，因此分支名不会把查询拼坏。该门禁在应当拦截时确实拦截，在无法判定时确实放行。 main() 把报告写到 stderr 并设置 process.exitCode = 1，因此陈旧分支会让该步骤乃至整个 job 失败；而所有无法得出裁决的路径都以 0 退出并在日志中留下 ::warning::——包括 GITHUB_TOKEN/BASE_REF/HEAD_SHA/GITHUB_REPOSITORY 缺失，以及 API 调用抛出的任何错误（curl -f 会把 HTTP 错误变成非零退出，execFile 据此 reject，外层 try/catch 捕获）。对于一个"分支规则集只要求评审、不要求状态检查"的仓库里的建议性门禁来说，接口故障时 fail-open 是正确方向：它不会把所有 PR 扣为人质，其失效也只是退回今天的行为，而不是让树变红。作用域划分合理。 该步骤只在 pull_request 上运行，并在 skip_ci 为真或 profile 为 docs_only 时跳过；注释里的理由成立——merge_group 验证的是预期合并后的树，push 验证的是 base 自身，两者都不存在"陈旧的绿灯"可抓，而 github_ci_only 会跑 lint 辅助步骤因此仍被覆盖。它位于 Node 安装与依赖安装之前，所以代价只是昂贵步骤之前的少量 API 调用：每个门禁文件两次、共七个文件，远在 GITHUB_TOKEN 配额之内。脚本头部也解释了为何用按文件的 commits?path= 查询、而不是读一次比较结果的文件列表——该列表上限 300 个文件，而在每天合并 50-100 个 PR 的仓库里落后几天的分支会轻易突破上限，从而把门禁文件从扫描中静默丢掉，而那正是本检查要防止的"静默漏检"。令牌处理与调用上界都妥当。 请求经 execFile 以参数数组发出，因此没有任何 shell 插值；--max-time 30 与 16 MiB 的 maxBuffer 限定了时长与体积；catch 中刻意用 curl 的退出码与最后一行 stderr 重建错误信息，因为 execFile 自身的消息会包含完整 argv（其中就有 bearer token）——Actions 会掩码该 secret，但本地运行会打印出来。这个判断是对的，且写在了现场注释里。该步骤所需的令牌权限确实存在，因此它不会静默空转。 Lint & Static 这个 job 自己声明了 permissions: 并授予 contents: 'read'（ci.yml:862-863），而这正是两个接口所需；由于 job 级权限块会覆盖工作流级，这一点值得实查而非假设。测试接线完整。 check-lint-gate-freshness.test.mjs 同时被加入 HELPER_TESTS_DEP_FREE 与 HELPER_TESTS（ci.yml:89-90），因此它在无依赖快速通道与完整辅助通道中都会运行；该套件只用 node:test，这也正是它适合无依赖通道的原因。唯一我无法从 diff 判定的是：在 "Set up Node.js" 步骤之前，各类 runner 上是否都能解析到 node——托管 runner 自带，相邻的 ECS 注释也说明复用机器上的 Node，因此应当可以；即便缺失，也会在第一个 PR 上立即显式失败，而不是静默失效，故我未把它当作 Critical。CI：本 head 上 16 项通过、50 项跳过，无失败也无 Pending，即本 PR 修改的那条检查在修改它的分支上是绿的。其立论——此处的绿灯只证明分支按分支自己定义的门禁通过——正是本检查要闭合的缺口；而排除 package.json 与 lockfile 也是有理由的取舍而非随意：两个工具都是全仓运行，因此版本升级必须在升级它的那个 PR 自己的运行中把整棵树带入新语义，唯有"门禁如何运行"的改动才可能绕过分支。
github-actions
Bot
added
the
skip-changelog-auto
2 participants
What this PR does
Adds a "Check lint gate freshness" step to the Lint & Static job, ahead of the install and lint steps. For each gate-defining file — the lint script, the ESLint and Prettier configs, the CI workflow itself, and the new check — the step asks the GitHub API two questions: what is the newest commit on the PR's base branch that touched the file, and does this branch contain it. A branch that lacks a base-side gate change fails in seconds with the offending commits, dates and links listed, plus a rebase instruction. API failures warn and pass: an advisory gate must not hold every PR hostage during an API brownout.
package.jsonand the lockfile are deliberately excluded — at 50–100 merges a day that would force every in-flight PR to rebase behind every dependency bump, and a Prettier/ESLint version bump cannot sneak changed semantics past a branch anyway, because both tools run repo-wide and the bumping PR must bring the whole tree under the new semantics in its own run.Why it's needed
The lane checks out the PR head alone — a deliberate trade-off recorded at the checkout step, because GitHub rebuilds the merge ref asynchronously and can serve it stale. A green run therefore proves the branch passes the gate as the branch defines it, not as the base defines it. #11117 flipped the Prettier lane from
--writeto--checkon main while #11216's branch still ran--write; #11216 merged green, main went red, and every later full-profile PR failed lint for a reason outside its own diff until #11378 reformatted the file. Nothing else catches this class today: the branch ruleset requires reviews, not status checks, and the merge queue has been disabled since 2026-07-02. Gate-file changes are rare (the lint script saw five commits in the months before this check), so an occasional forced rebase is cheap insurance.Reviewer Test Plan
How to verify
The decision logic is covered by a nine-case node:test suite (every compare status, empty base-side history, error propagation, the workflow wiring, and a mutation witness on the gate-file list):
node --test .github/scripts/check-lint-gate-freshness.test.mjs. For a live check against the real API, run the script with a token and two heads: a fresh head (the base tip) prints the OK line and exits 0; a stale head (the parent of the last base commit touching the CI workflow) exits 1 naming that commit, its date and URL, and the rebase remedy. Missing or invalid tokens exit 0 with a::warning::— the fail-open path.Expected on this PR's own run: the step passes, because the branch contains every base-side gate change as of its branch point.
Evidence (Before & After)
N/A — no UI or TUI surface. Before: a branch stale against a base-side gate change merges green and reddens main. After (simulated against the live API with a stale head):
Tested on
Environment (optional)
Local macOS: the nine-case suite, the wiring assertions, Prettier/ESLint/yamllint/actionlint on the touched files, the workflow size ratchet (within the allowance), and the full scripts test suite — its only failures sit in unrelated, environment-dependent files and reproduce identically with this change stashed. Live-API runs covered pass, fail and both fail-open shapes. The HTTP layer shells out to curl precisely so the ECS runners' squid proxy variables are honored; node builtins only, so the step can run before any dependency install.
Risk & Scope
Linked Issues
None filed; the incident analysis is on #11378 and the #11117 → #11216 timeline.
中文说明
这个 PR 做了什么
在 Lint & Static job 里、安装与 lint 各步骤之前，新增一个 "Check lint gate freshness" 步骤。对每个闸门定义文件——lint 脚本、ESLint 与 Prettier 配置、CI 工作流本身、以及这个新检查——该步骤向 GitHub API 问两个问题：PR 的 base 分支上最近一次改动该文件的是哪个提交，以及当前分支是否包含它。缺少 base 侧闸门改动的分支会在几秒内失败，输出列出相关提交、日期与链接，并给出 rebase 指引。API 失败时告警并放行：一个劝告性闸门不能在 API 故障时劫持所有 PR。
package.json与 lockfile 被刻意排除——在每天 50–100 个合入的频率下，纳入它们会迫使每个在途 PR 跟在每次依赖升级后面 rebase；而且 Prettier/ESLint 版本升级也无法把变更后的语义偷运过分支，因为这两个工具都是全仓运行，发起升级的 PR 必须在自己的运行里让整棵树满足新语义。为什么需要
这条 lane checkout 的是 PR head 本身——这是 checkout 步骤处记录的刻意取舍，因为 GitHub 异步重建 merge ref，可能提供陈旧内容。因此绿色运行证明的是「分支通过了分支所定义的闸门」，而不是「base 所定义的闸门」。#11117 在 main 上把 Prettier lane 从
--write切成--check，而 #11216 的分支仍在跑--write；#11216 绿着合入，main 变红，之后每个 full-profile PR 都因与自身 diff 无关的原因 lint 失败，直到 #11378 重新格式化该文件。今天没有任何其他机制兜住这一类问题：分支 ruleset 要求的是 review 而不是 status check，merge queue 自 2026-07-02 起停用。闸门文件的变更很稀少（lint 脚本在本次检查之前的几个月里只有五次提交），所以偶尔强制 rebase 一次是廉价的保险。审阅测试计划
如何验证
判定逻辑由九个 node:test 用例覆盖（每种 compare 状态、base 侧无历史、错误传播、工作流接线，以及闸门文件清单的 mutation witness）：
node --test .github/scripts/check-lint-gate-freshness.test.mjs。要用真实 API 做活体验证，带 token 跑两次脚本：新鲜 head（base 最新提交）打印 OK 行并以 0 退出；陈旧 head（base 上最后一次触碰 CI 工作流的提交的父提交）以 1 退出，并指名该提交、日期、URL 与 rebase 指引。缺失或无效 token 时以 0 退出并打印::warning::——即 fail-open 路径。本 PR 自己的运行预期：该步骤通过，因为分支包含了截至分叉点的全部 base 侧闸门改动。
前后对比证据
N/A——不涉及 UI 或 TUI 界面。改动前：落后于 base 侧闸门变更的分支绿着合入并使 main 变红。改动后（用陈旧 head 对真实 API 模拟）：
测试环境
环境（可选）
本地 macOS：九用例套件、接线断言、对改动文件跑 Prettier/ESLint/yamllint/actionlint、workflow 体积棘轮（在余量内）、以及完整 scripts 测试套件——其仅有的失败都在与本改动无关的环境依赖型文件里，且把本改动 stash 后照样复现。真实 API 运行覆盖了通过、失败与两种 fail-open 形态。HTTP 层刻意改用 curl，就是为了遵守 ECS runner 的 squid 代理环境变量；脚本只用 node 内置模块，因此可以在任何依赖安装之前运行。
风险与范围
关联 Issue
未单独建 issue；事故分析见 #11378 以及 #11117 → #11216 的时间线。