Skip to content

ci: give minimum Node qualification runner headroom - #4836

Merged
huangruiteng merged 2 commits into
mainfrom
codex/minimum-node-ci-timeout
Sep 21, 2026
Merged

huangruiteng merged 2 commits into
mainfrom
codex/minimum-node-ci-timeout

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

Problem

The required minimum-Node qualification on PR #4833 was cancelled at the 10-minute job limit after its TypeScript tests had already passed more than 1,000 cases. Recent successful runs also finish close to that ceiling, so runner variance can turn a healthy full qualification into a clock failure.

Change

  • raise only node-minimum-compatibility from 10 to 15 minutes
  • retain the complete non-SQLite control-plane test selection and required merge gate
  • add a workflow contract test that prevents fixing the timeout by silently filtering or sharding away coverage

Validation

  • python -m pytest -q tests/test_python_ci_workflow.py — 44 passed
  • python -m ruff check tests/test_python_ci_workflow.py — passed
  • git diff --check origin/main...HEAD — passed

This is an evidence-backed job budget adjustment; it does not weaken minimum-runtime coverage or make the lane optional.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

这个 PR 修的是一个有直接证据的 CI 预算问题:#4833node-minimum-compatibility 在完整 TypeScript suite 已经执行到后段时,被 10 分钟 job timeout 取消,随后 required merge gate 失败。这里的目标不是让 CI “更宽松”,而是让同一套最小 Node 兼容性资格验证由断言/进程结果决定成败,而不是被过紧的外层时钟随机截断。

改动思路

改动只调整现有 node-minimum-compatibility job 的 timeout-minutes,从 10 增到 15;不改变 job 条件、Node 版本、测试发现逻辑、排除规则、Node invocation、required-check 关系或 merge gate。同时在已有 workflow contract test 中固定三件事:仍遍历全部 tests/control_plane_ts/*.test.ts,仍使用原来的完整 Node 命令,并且不能通过 --test-name-patternshard 偷偷缩减覆盖来换取时长。

具体改动

  • .github/workflows/python-tests.yml:只把 minimum-Node lane 的外层预算从 10 分钟改为 15 分钟,并解释当前 8–11 分钟的实测区间与 runner variance。
  • tests/test_python_ci_workflow.py:新增一个 job-block contract test,绑定新的预算与完整覆盖形状。
  • 基线/当前 head 的真实 runner 证据一致:#4833 run 35542808078 的 minimum-Node job 在 10m16s 被取消;本 PR exact-head run 35544194850 使用相同完整选择,在 10m31s 成功。这个差异说明 10 分钟确实低于健康执行时长,而 15 分钟提供了约 4.5 分钟的抖动余量。
  • 本地验证:workflow focused pytest 44 passed,Ruff passed,YAML parse passed,git diff --check passed;远端 exact-head workflow 和 merge-gate 均成功。

对主干的风险

主要代价是:真正 deadlock/hang 的 minimum-Node job 最坏会多占用 5 分钟才被 GitHub 取消。这个风险是有界且局部的,只影响 core-tests 启用时的一个 job;普通断言失败仍会立即以非零退出,lane 仍然 required,merge gate 也没有弱化。

我特别检查了 budget change 的合理性:这里没有通过删测试、改扫描根、过滤名称或注册无关阈值来“把失败改绿”;对比的是同一 job、同一完整测试选择下的真实 wall-clock 结果。若未来稳定时长继续接近 15 分钟,下一步应该 profile/压缩或做明确保持覆盖的并行化,而不是机械继续抬高上限。

我的整体评价

这是一个范围很窄、证据充分的回归预算调整。预算 owner、完整覆盖和 merge obligation 一起被保留并增加了回归测试;当前 head 也在真实 GitHub runner 上证明 15 分钟足够、10 分钟不足。未发现 blocking finding,结论是 APPROVE

未来向前看:此处无需再抽象新的 CI budget 框架;把测量证据留在 PR review、把 durable invariant 留在现有 workflow test,已经是更容易回滚和维护的边界。

Reviewed exact head: 1892d7f5ebea4a70e23c3e9d994f3d948eed5c7f

English verdict: APPROVE — the five-minute job-local increase is evidence-backed by equal full-suite runner observations, preserves the required minimum-Node coverage and merge gate, and bounds the only material downside to five additional minutes on a true hang.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

动机

本次审阅绑定 exact head 1892d7f5ebea4a70e23c3e9d994f3d948eed5c7f。这个 PR 修的是 evidence-backed CI regression budget:minimum-Node lane 的完整 TypeScript qualification 已经进入后段,却会被 10 分钟 job timeout 取消,导致 required merge gate 因外层时钟而不是测试结论失败。目标不是“让 CI 更宽松”,而是让同一套完整资格验证由断言/进程结果决定成败。

改动思路

改动只调整现有 node-minimum-compatibility job 的外层 timeout-minutes,从 10 提到 15;不改变触发条件、Node 版本、test discovery、SQLite exclusions、Node invocation、required-check 关系或 merge gate。已有 workflow contract test 被扩展为同时固定预算和覆盖形状,明确禁止通过 --test-name-pattern 或 sharding 偷偷缩短运行时间。

这是合理的 budget decision:被保护的 invariant 是“minimum supported Node 上完整执行适用的 control-plane TypeScript suite”,而不是“必须在一个历史偶然值 10 分钟内结束”。同 workload 的既有 runner evidence 表明健康执行可能超过 10 分钟;15 分钟提供 bounded variance headroom,同时保留 assertion failure 的即时失败语义。

具体改动

关键代码讲解

  1. .github/workflows/python-tests.yml::jobs.node-minimum-compatibility.timeout-minutes 从 10 改为 15;作用域只限一个 required job。
  2. test_minimum_node_lane_keeps_full_coverage_with_runner_headroom 固定 15 分钟预算、tests/control_plane_ts/*.test.ts 完整遍历和原有 Node command。
  3. 同一测试明确拒绝 --test-name-patternshard,防止以后把预算问题“修”成 coverage reduction。

本地验证:workflow contract suite 44 passed,Ruff 通过,YAML parse 通过,git diff --check 通过;current-main merge-tree 成功。

对主干的风险

最强反例是一个真实 deadlock/hang:外层 timeout 现在最多晚 5 分钟才取消,增加 runner 占用和反馈延迟。这个代价有界且局部,只影响 core-tests 启用时的 minimum-Node job;普通断言失败和进程非零退出仍立即失败,lane 仍是 required,merge gate 没有被绕过。

本改动没有用删测试、缩 scan root、改 exclusions、增加 unrelated threshold 或把 required lane 变 optional 来隐藏失败。若以后健康时长继续逼近 15 分钟,应 profile/compact,或做明确保持覆盖的并行化;不应机械继续抬高预算。

语义与 CI 对齐

这里调整的是 evidence-backed regression budget,不是 hard correctness limit,也没有改变测试语义。workflow owner 与 contract test 一起更新,budget 和所保护的完整 coverage 保持对齐。

我的整体评价

未发现 blocking finding,结论是 APPROVE。这是一个范围窄、可回滚、维护成本低的 CI 预算修复;它保留了完整 minimum-Node qualification,并把“不能靠过滤或 sharding 换绿”固化成 durable regression contract。

未来向检查结论:无需为一个 job-local scalar 新增通用 CI budget framework。把 measured trade-off 留在审阅证据、把 durable invariant 留在现有 workflow test,是更清晰的边界。

English verdict: APPROVE — exact head 1892d7f5ebea4a70e23c3e9d994f3d948eed5c7f makes an evidence-backed five-minute increase to one required minimum-Node job, preserves the complete test selection and merge gate, and bounds the only material downside to five extra minutes on a true hang.

…-timeout

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

Reviewed exact head: 584497801666bd304fa18eeca4af80cec89682ea (codex/minimum-node-ci-timeout, re-review after the branch was merged with main 92185803b).

动机

#4833 上必须通过的最小 Node 资格 lane 在 job 的时钟上限处被取消,而它的 TypeScript 用例当时已经跑过一千多个;近期成功的运行也贴着上限完成。也就是说,一次健康的完整资格验证会被 runner 抖动变成时钟失败。更糟的是它教给评审者的习惯:把这种取消当成噪声重跑。而最省事的"修法"——给 lane 加 --test-name-pattern 或切开 shard——会真正缩小兼容性覆盖,而且当时没有任何测试会反对。

改动思路

给 lane 留出余量,同时用一条 workflow 契约测试把"不许靠缩减覆盖换绿"钉住。测试断言的是契约形状而不是某次事故的文本:lane 必须保留 glob 驱动的完整选择、必须是一次 node … --test "${tests[@]}" 调用、不得出现 --test-name-pattern、不得出现 shard。这样预算调整可以重复使用,而不会成为下一轮缩覆盖的入口。

本 head 相对上次评审的唯一变化是与 main 的合并,而这次合并暴露出一件必须说清楚的事:main 已经通过它自己的提交把同一 lane 的 10 分钟改成了 15 分钟83ab0c0eb "ci: preserve minimum Node conformance coverage",带一条等价注释)。冲突因此只落在 timeout-minutes: 15 上方那段解释性注释上。我保留了 main 的注释、放弃本分支措辞重复的那份,于是这个 PR 相对当前 main 的真实增量只有那条测试git diff origin/main HEAD = tests/test_python_ci_workflow.py +12)。

具体改动

相对当前 main:1 个文件、+12/-0。

  • tests/test_python_ci_workflow.py:新增 test_minimum_node_lane_keeps_full_coverage_with_runner_headroom,从 workflow 文本里切出 node-minimum-compatibility 块,断言 timeout-minutes: 15for test in tests/control_plane_ts/*.test.ts、单次 node --no-warnings --experimental-sqlite --experimental-strip-types --test "${tests[@]}"、以及 --test-name-pattern / shard 均不出现。
  • .github/workflows/python-tests.yml:本条分支原有的注释改动已被 main 的等价注释取代,因此在本 head 上该文件与 main 逐字节一致;lane 的 15 分钟预算本身在 main 上已经生效。

关键内容讲解

  • tests/test_python_ci_workflow.py:65:切块方式(在 node-minimum-compatibility:node-forward-compatibility: 之间取文本)保证断言只作用于这一条 lane,不会因为别的 lane 加了过滤就误报,也不会因为别的 lane 的变化而漏报。
  • 四条内容断言各挡一种"缩覆盖换绿"的路径:换掉 glob 循环、把一次调用拆成多次、加 --test-name-pattern 过滤、切 shard。只有第一条 timeout-minutes: 15 是关于预算的,其余三条是关于覆盖的——这正是它比"把 YAML 抄一遍"更有价值的地方。
  • 把测试放进既有的 tests/test_python_ci_workflow.py(同文件已有 checks 聚合、stage2c 门禁、shard 覆盖等 workflow 契约),而不是新建文件,符合仓库"同一变更理由放同一处"的惯例。

我复核的关键点(都是自己跑的):

  • pytest -q tests/test_python_ci_workflow.py44 passedruff check → all checks passed;workflow YAML 解析通过(14 个 job,node-minimum-compatibility.timeout-minutes == 15)。
  • 变异验证(关键):我把 --test-name-pattern=smoke 注入 lane 的 node 调用后重跑,test_minimum_node_lane_keeps_full_coverage_with_runner_headroom 失败(1 failed / 43 passed);恢复文件后 44 passed、git status 干净。这说明这条断言是承重的,不是装饰。
  • 重复度扫描rg -ln 'timeout-minutes' tests/ examples/ 只有三个文件,其中没有任何一个此前断言过这条 lane 的预算;rg -ln 'node-minimum-compatibility' 里除本测试还有 examples/github-actions-runtime-smoke.py,而后者只断言 merge gate 的 needs 列表。也就是说这条覆盖边界此前无人看守,不是重复建设。
  • loopx canary premerge --from-git-diff:status passed、self_merge_allowed: truemanual_holds: 0git diff --check 干净。

对主干的风险

风险面很窄:本 head 不触碰 loopx/**apps/**packages/**,不改权限、持久化状态、quota/scheduler/todo 规则,也不改必需检查集合;它只增加一条针对 CI 契约的测试。真正需要防的回归是"以后有人缩减 lane 覆盖"——这正是变异验证证明被挡住的那条路径。两个诚实的边界:(1) 测试钉的是配置,不是真实耗时,15 分钟这个数仍是经验判断;(2) 它无法证明每个 runner 上都能跑完,只有 CI 自己能给这个答案。回退成本一个 commit。

我的整体评价

结论 APPROVE。这条 12 行的契约测试保护的是"必需 lane 的覆盖不许因为时钟被削掉"这一边界,此前确实无人看守,且我通过注入过滤实测它会失败。需要明确记在案的是:本 PR 声称的"把 10 分钟改成 15 分钟"在当前 main 上已经由另一提交完成,因此这个 head 的实际贡献是那条守卫,而不是分钟数;这也让这次合并变成一次纯粹的测试增量,风险与回退面都很小。

English verdict: APPROVE - exact head 5844978 (re-review after the branch merged main 9218580). The conflict was only the explanatory comment above timeout-minutes: 15, and main already carried an equivalent comment from its own commit, so the resolution keeps main's wording and the delivered delta against current main is the +12-line workflow contract test alone - the timeout raise itself is already in main. The test asserts the minimum-Node lane keeps its glob-driven full selection in a single node --test invocation with no --test-name-pattern filter and no sharding, plus the headroom value, so the cheap way to make a cancelled lane green (narrowing coverage) now fails the suite. I verified that this is load-bearing rather than decorative by injecting --test-name-pattern=smoke into the lane in a scratch copy: the guard failed (1 failed, 43 passed), and after restoring the file the module is 44 passed with a clean git status. A coverage scan found no prior assertion of this lane's budget or selection (rg -ln 'timeout-minutes' tests/ examples/ returns three files, none of them about this lane), so the guard is not duplicative; the author's open queue is the maintainer's ordinary multi-topic backlog rather than a same-shape smoke batch. ruff passes, the workflow parses, git diff --check is clean, and canary premerge reports passed with self_merge_allowed true and manual_holds 0. No blocking finding; residual limits are that the guard pins configuration rather than runtime duration, so the 15-minute budget stays an empirical judgement that only a real CI run can confirm.

@huangruiteng
huangruiteng merged commit 7469129 into main Sep 21, 2026
12 of 17 checks passed
@huangruiteng
huangruiteng deleted the codex/minimum-node-ci-timeout branch September 21, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant