ci: split lint and static checks out of the Test job - #10756
Conversation
The `test` job bundled two halves with opposite reliability profiles: twenty deterministic lint/static steps (ESLint, actionlint, shellcheck, yamllint, Prettier, schema/notices freshness, lockfile, workspace isolation, TUI dep direction, bundle closure, helper tests, workflow size) and the unit-test run, which was measured failing 41% of post-merge runs on flake alone this week. main still has zero required status checks — a duplicate-declaration compile error shipped to main on 09-01 and sat red for half an hour precisely because nothing gates merges — but the combined job could never become required while the flaky half holds the deterministic half hostage. Move the twenty static steps into a new `lint_and_static` job with the same event surface, the same runner routing, the same release-sync convention (run on skip_ci PRs, no-op per step, so a required context still reports), and its own copy of the checkout/node/install prelude plus the CI-profile classifier (the classifier is one gh api call; duplicating it costs seconds and keeps `test`'s `ci_profile` output — which web_shell_e2e_smoke consumes — untouched). The surgery is a pure move, proven at YAML level: the old job's 40 steps equal the new jobs' union exactly, every step is field-for-field identical to its original, every other job is byte-identical, and `lint_and_static.if` equals `test.if` — pinned by a new equality assertion that rides on test's whole-literal pin, so the two gates cannot diverge silently. Event-matrix A/B against main shows exactly one new row: lint_and_static running on push, pull_request, merge_group and workflow_dispatch, and not on schedule. Mutation-checked: a lint gate that silently drops push, or silently joins the nightly, each reddens two assertions. Scope disclosure: this lane could NOT have caught the 09-01 incident — that was an esbuild transform error inside a test file, ESLint accepts the duplicate binding (verified against the actual broken file), and web-shell's tsconfig excludes *.test.tsx from typecheck. Catching that class needs the unit-test lane, which can only become required once its flake rate is fixed. This split makes the half that CAN be required independent of that work. The size-baseline bump records the duplicated prelude comments; the growth is the split itself.
|
|
|
Thanks for the PR! Template looks good ✓
Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review — the central claim is "pure move", so I verified it at the byte level rather than taking the description at its word:
Two non-blocking notes: the checkout-ref literal pin ("checks out the commit the push reported") still covers only the Testing — workflow-only change; the evidence is the PR's own CI. The new Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查 —— 核心主张是"纯移动"，因此我逐字节核实而非照单接受：从 两条非阻塞提醒：checkout-ref 整串钉目前只钉 测试 —— 仅 workflow 改动，证据来自本 PR 自身的 CI。新的 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, verified pure move with a real motivation; two non-blocking nits already named (checkout-ref pin coverage, flat timeout). This is the split I would have written. The motivation is not hypothetical: a compile error merged into The remaining uncertainty is execution, not design: the two checks that matter most here — the new lint half, and the unit-test run carrying the guard suites — are still in flight on this PR at review time. Approval is therefore deferred until CI lands green on the reviewed commit; the finalize job pins the approval to that commit and will withhold it if anything lands red or the head moves. Follow-ups worth keeping in sight: the new job's real p50/p90 once a few runs exist (before it becomes required), and the ruleset change itself (admin, correctly out of scope here). 中文说明Confidence: 4/5 —— 干净、已逐字节核实的纯移动，动因真实；两个非阻塞点已在上一条点名（checkout-ref 钉的覆盖范围、固定超时）。 这正是我会写的拆分。动因不是假设：昨天一个编译错误合进了 main，今天 ruleset 依然没有任何 required check（已核实），而一个合并后运行约 40% 抖红的 test job 永远无法成为 required check。实现是这个想法的最小形态——20 个确定性步骤逐字节移动、前置链忠实拷贝、同一门由测试钉住、其余分毫未动——而诚实的范围说明（这条 lane 本身抓不住 09-01 那次事故）表明作者追求的是真正的门，而不是门的表象。 剩余的不确定性在执行而非设计：此处最关键的两个检查——新的 lint 半边，与承载守卫套件的单测运行——在审查时仍在本 PR 上进行中。因此批准推迟到 CI 在被审提交上落绿为止；finalize 会把批准钉在该提交上，任何变红或 head 移动都会被扣下。值得继续盯的后续：新 job 跑出若干次后的真实 p50/p90（设为 required 之前），以及 ruleset 变更本身（管理员操作，正确地不在本 PR 范围内）。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
The first real run of the new lane was beheaded at exactly 30 minutes: a cold-cache pool runner paid 892s for npm install where the sibling Test job on the same run paid 347s, ESLint and Prettier both ran at ~2x their historical timings, and the job died mid bundle-closure. The 13-15 minute prediction in the split commit came from warm-cache step timings and did not price the first run on any given runner. 45 matches desktop_shell and still surfaces a wedged runner within the hour.
|
First real run of the lane: Root cause is a cold-cache first run, not a wedged step: on runner Pushed 中文说明新 lane 首跑： |
|
Independent verification at head Pure move, verified against the actual base Job-level wiring: No stranded cross-step data flow: every Guards: Live lane: the head's own Not approving — the deferred approval was voided by the head move and the re-review is still pending — but nothing blocking from my side. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查。 建议见行内评论。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.3)
…lane # Conflicts: # .github/workflows/.size-baseline
Nine review findings, one root cause: the split copied test's behavior without copying its guards — every contract test that pins test's checkout, recovery, telemetry, budget and token surface enumerated jobs by name and never reached the copy. Contract extensions: the checkout-ref whole-literal pin now loops both jobs; no-ak-integration-ci's guardCalls gains a lint_and_static entry pinned to test's event-aware expected_sha shape; qwen-triage's unconditional-ownership-restore suite gains the new job; ci-disk-pressure pins the lane's sampler AND its collector — the collector was simply missing (the lane produced the #10035 telemetry and destroyed it with the runner temp dir on the exact ENOSPC death the sampler explains), now added with a per-job artifact name since upload-artifact v4+ rejects duplicates when both jobs fail in one run. New pins beside the gate-equality test: timeout stays the measured 45 (30 was the beheading value), and permissions stays exactly {contents: read} — the workflow grants checks/statuses write, and this lane runs contributor lifecycle scripts. Fidelity fix: the size gate moved back above the dependency install, the fail-fast position it deliberately held in the pre-split job — after npm ci it delivered its verdict ~6 minutes late warm, ~15 cold, and hid size violations behind unrelated install failures. An ordering assertion in workflow-size.test.js pins the position. Drift guard: the merge that landed mid-review proved the copies rot — #10799's trusted-classifier rework updated test's prelude and left the lint copy stale, resolved by hand. A shared-prelude equality test now holds every same-named step field-for-field identical (sole deliberate divergence: the collector's artifact name), so the next drift is a red test, not review archaeology. Prose: the three stale two-job narratives (test's header, HELPER_TESTS, the lanes test file) now describe the three-job reality. All nine mutations red: push-arm drop, verify-head drop, gate reordering, collector drop, ownership-restore drop, timeout lowered to 30, permissions deleted, permissions widened, one-character prelude drift.
The flat 45 beheaded a second honest run: on cold-cache hk3 pool runners every step lands 2-4x its warm timing (install 940s vs 347s, bundle closure 681s vs 171s) and the serial sum cleared 45 minutes with no step wedged. The pool contention that sized test at 120/60 and no_ak at 60/30 applies here identically; a bare constant priced only the warm half of that reality. ECS 90 / hosted 45, same expression shape as test's, pins updated with both measurements.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — the build-test harness timed out its packages/cli build (138s deadline, infrastructure) and never reached its test phase, and the full packages/cli vitest suite (239 files) did not complete locally; the substituted ground was typecheck (exit 0), all 23 HELPER_TESTS files (510 pass), test:scripts (2095 pass, 4 load-induced failures that pass in isolation and read no ci.yml), the three changed guard suites (250/250) and AppContainer.test.tsx (179 pass, the only suite referencing the one changed production path).
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": the full npx vitest run scripts/tests sweep was inconclusive on this host — dozens of unrelated files failed with 5 s test timeouts and vi.mocked(...).mockCl…; "agent reverse-audit (round 3)": did not confirm .prettierignore covers packages/vscode-ide-companion/schemas/settings.schema.json and NOTICES.txt , i.e. whether Run Prettier 's prettier…; "agent reverse-audit (round 2)": running prettier --check on README.md / packages/cli/src/ui/AppContainer.tsx to confirm the two reformats are formatter-mandated rather than hand-aligned …; "agent reverse-audit (round 2)": executing scripts/tests/ci-platform-lanes.test.js , no-ak-integration-ci.test.js and workflow-size.test.js at the reviewed commit to see the new assertions….
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
.github/workflows/ci.yml:1169 — [review] D2-1 root-CLI-bundle provenance scan now fail-opens in every CI job.github/workflows/ci.yml:789 — [review] D2-2 new check name escapes the fleet dashboard's name-keyed CI-red classifier.github/workflows/ci.yml:1038 — [review] D2-3 second npm ci bundles 136 MiB nothing in the lane reads
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — the build-test harness timed out its packages/cli build (138s deadline, infrastructure) and never reached its test phase, and the full packages/cli vitest suite (239 files) did not complete locally; the substituted ground was typecheck (exit 0), all 23 HELPER_TESTS files (510 pass), test:scripts (2095 pass, 4 load-induced failures that pass in isolation and read no ci.yml), the three changed guard suites (250/250) and AppContainer.test.tsx (179 pass, the only suite referencing the one changed production path)。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：the full npx vitest run scripts/tests sweep was inconclusive on this host — dozens of unrelated files failed with 5 s test timeouts and vi.mocked(...).mockCl…；"agent reverse-audit (round 3)"：did not confirm .prettierignore covers packages/vscode-ide-companion/schemas/settings.schema.json and NOTICES.txt , i.e. whether Run Prettier 's prettier…；"agent reverse-audit (round 2)"：running prettier --check on README.md / packages/cli/src/ui/AppContainer.tsx to confirm the two reformats are formatter-mandated rather than hand-aligned …；"agent reverse-audit (round 2)"：executing scripts/tests/ci-platform-lanes.test.js , no-ak-integration-ci.test.js and workflow-size.test.js at the reviewed commit to see the new assertions…。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| new Map( | ||
| (ci.jobs[job].steps ?? []).map((s) => [ | ||
| s.name, | ||
| JSON.stringify(s, Object.keys(s).sort()), |
There was a problem hiding this comment.
[Suggestion] R2-1: JSON.stringify(s, Object.keys(s).sort()) does not compare the fields this guard says it guards. An array replacer is a property allowlist applied at every nesting level, so each step's nested with: and env: serialise as {}: the guard cannot see drift in Checkout's ref/fetch-depth, setup-node's node-version/cache/cache-dependency-path/registry-url, or Use trusted CI profile's TRUSTED_CI_PROFILE.
That is exactly the drift class the comment above names — "#10799's trusted-classifier rework, which updated test's copy and left the lint one stale". Reworking only lint_and_static's copy of any of those values ships a divergence between the two lanes with this test green. The consequential one is the profile binding: steps.ci_profile.outputs.ci_profile gates 18 of the lane's steps on == 'full', so a lint copy that resolved to docs_only would skip every moved check and report green having run no lint at all. Scope note — today's YAML is correct (with a recursive serialiser the intact tree passes 34/34); the defect is that the guard cannot see the fields it claims to.
Witness:
guard serialised (Checkout): {"id":"checkout","if":"…","name":"Checkout","uses":"actions/checkout@df4cb1c…","with":{}}
real nested with: {"ref":"${{ github.event.inputs.branch_ref || …}}","fetch-depth":1}
guard serialised (Use trusted CI profile): {"env":{},"id":"ci_profile",…}
real nested env: {"TRUSTED_CI_PROFILE":"${{ needs.classify_pr.outputs.ci_profile }}"}
mutation TRUSTED_CI_PROFILE→'docs_only' (lint copy only): guard-equal? true | plain-equal? false
mutation node-version→'18.x' + fetch-depth→0 + delete cache: guard-equal? true | plain-equal? false
→ population: vitest 8/8 files, node:test 203 pass, 0 fail
FLIP (recursive key-sorted serialiser, same mutation): × keeps every shared prelude step identical … Tests 1 failed | 33 passed
FLIP on intact tree: Test Files 1 passed (1) Tests 34 passed (34)
Canonicalise recursively instead of using an array replacer, or store the parsed step objects and let toEqual deep-compare them (it ignores key order, which is all the .sort() was buying):
const canon = (v) =>
Array.isArray(v)
? v.map(canon)
: v && typeof v === 'object'
? Object.fromEntries(Object.keys(v).sort().map((k) => [k, canon(v[k])]))
: v;
const byName = (job) =>
new Map((ci.jobs[job].steps ?? []).map((s) => [s.name, JSON.stringify(canon(s))]));A deep comparison must keep deliberatelyDivergent excluding Upload disk-pressure samples (line 363), because that step's nested with.name differs by design — .github/scripts/ci-disk-pressure.test.mjs:112 asserts notEqual on it — and must not change which steps count as shared, since expect(shared.length).toBeGreaterThanOrEqual(12) is met exactly today (measured shared.length === 12).
Acceptance criterion: keeps every shared prelude step identical between test and lint_and_static must go red when lint_and_static's Checkout gets with.fetch-depth: 0, or its Use trusted CI profile gets env.TRUSTED_CI_PROFILE: 'docs_only'; both pass as committed, so please apply the fix, re-apply one mutation, and confirm it reds.
中文说明
JSON.stringify(s, Object.keys(s).sort()) 并没有比较这个守卫自称在比的字段。数组形式的 replacer 是一份属性白名单，会在每一层嵌套上生效，因此每个步骤的 with: 与 env: 都序列化成 {}：Checkout 的 ref/fetch-depth、setup-node 的 node-version/cache/cache-dependency-path/registry-url、以及 Use trusted CI profile 的 TRUSTED_CI_PROFILE，这个守卫全都看不见。
这正是上方注释点名的那类漂移——「#10799 的 trusted-classifier 改造更新了 test 的副本、把 lint 的副本留旧了」。只改 lint_and_static 副本里上述任何一个值，两条通道就会出现分歧，而这个测试仍然是绿的。后果最严重的是 profile 绑定：steps.ci_profile.outputs.ci_profile 让本通道 18 个步骤以 == 'full' 为门，因此若 lint 副本解析成 docs_only，所有被移动的检查都会跳过，通道会在一个 lint 都没跑的情况下报绿。范围说明：今天的 YAML 是正确的（换成递归序列化器后原树 34/34 通过），缺陷在于守卫看不见它声称在看的字段。
建议不要用数组 replacer，而是递归规范化；或者直接保存解析后的步骤对象交给 toEqual 做深比较（它本身忽略键顺序，而 .sort() 买到的也只有这一点）。深比较必须继续用 deliberatelyDivergent 排除 Upload disk-pressure samples（第 363 行），因为该步骤嵌套的 with.name 是有意不同的（.github/scripts/ci-disk-pressure.test.mjs:112 断言 notEqual），也不能改变被视为共享的步骤集合——expect(shared.length).toBeGreaterThanOrEqual(12) 目前刚好卡在下限（实测 shared.length === 12）。
验收标准：给 lint_and_static 的 Checkout 加上 with.fetch-depth: 0，或给它的 Use trusted CI profile 加上 env.TRUSTED_CI_PROFILE: 'docs_only'，keeps every shared prelude step identical between test and lint_and_static 必须变红；两种变异在当前代码上都通过，所以请应用修复后重做一次变异并确认它变红。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| const byName = (job) => | ||
| new Map( | ||
| (ci.jobs[job].steps ?? []).map((s) => [ |
There was a problem hiding this comment.
[Suggestion] R2-2: this guard keys steps into a Map by name, so it is order-blind by construction, and nothing else pins the position of Use trusted CI profile (id: 'ci_profile') — the producer of steps.ci_profile.outputs.ci_profile — inside the new 33-step job. This is a separate hole from the nested-field blindness reported at line 354 and from the intersection-only scope at line 364: fixing either of those adds no ordering.
GitHub's steps context only carries steps that "have an id specified and have already run", and dereferencing a nonexistent property "will evaluate to an empty string". So if a merge-conflict resolution or a later edit places the producer below any of its consumers — or drops id: 'ci_profile' — all 26 gates reading steps.ci_profile.outputs.ci_profile == 'full' become '' == 'full' → false, every moved check skips, and the job completes green having executed nothing. This is not hypothetical drift in this particular job: the ordering assertion added elsewhere in this same fix round exists because the split initially parked Check workflow file size on the wrong side of npm ci.
Witness:
AUTHORITY (docs.github.com/en/actions/learn-github-actions/contexts, fetched live):
"The steps context contains information about the steps in the current job that have an id specified and have already run."
"If you attempt to dereference a nonexistent property, it will evaluate to an empty string."
intact tree: lint_and_static steps=33, ci_profile producer index=4, consumers=26, consumers below producer=26
ordering pins that exist for this job: restoreIdx<checkoutIdx (qwen-triage-workflow.test.mjs:154-159), gate<install (workflow-size.test.js:52-68)
MUTATION (scratch tree): moved 'Use trusted CI profile' below 'Install dependencies'
→ producer index 4→11, consumers below producer 26→20
helper node:test suites: # pass 78 / # fail 0 (baseline identical)
qwen-triage-workflow.test.mjs: # pass 125 / # fail 0 (baseline identical)
→ NO suite detects the reorder
also measured: gateRunsOn throws on any unmodelled `steps.` term but is called only with job.if, so no test evaluates a single step-level gate in the 33-step job
Add an order assertion beside the equality loop in the same test — compare the shared prelude's relative order, and pin the profile producer ahead of its first consumer:
const namesIn = (job) => (ci.jobs[job].steps ?? []).map((s) => s.name);
expect(
namesIn('lint_and_static').filter((n) => shared.includes(n)),
'shared prelude reordered',
).toEqual(namesIn('test').filter((n) => shared.includes(n)));
const lintNames = namesIn('lint_and_static');
expect(lintNames.indexOf('Use trusted CI profile')).toBeLessThan(
lintNames.indexOf('Install dependencies'),
);The order comparison must keep deliberatelyDivergent (line 363) out of the compared sequence, and must not contradict the two orderings already pinned for this job: restoreIdx < checkoutIdx (.github/scripts/qwen-triage-workflow.test.mjs:154-159) and gate < install (scripts/tests/workflow-size.test.js:52-68).
Acceptance criterion: with the assertion added, moving - name: 'Use trusted CI profile' (with its id/env/run) below - name: 'Install dependencies' in lint_and_static must turn that test red — today the same mutation leaves all 250 tests in the three changed files green, so please confirm the red after adding it.
中文说明
这个守卫用名字把步骤存进 Map，因此在结构上就是顺序无关的；而新 job 这 33 个步骤里，Use trusted CI profile（id: 'ci_profile'，即 steps.ci_profile.outputs.ci_profile 的生产者）的位置没有任何其它测试钉住。这与第 354 行的嵌套字段盲区、第 364 行的「只比交集」范围问题是三个不同的洞：修好其中任何一个都不会带来顺序约束。
GitHub 的 steps 上下文只包含「已指定 id 且已经运行过」的步骤，而对不存在的属性解引用「会求值为空字符串」。因此一旦合并冲突的解决方式或后续编辑把生产者放到任何消费者之下（或删掉 id: 'ci_profile'），26 个读取 steps.ci_profile.outputs.ci_profile == 'full' 的门都会变成 '' == 'full' → false，所有被移动的检查全部跳过，job 在什么都没执行的情况下报绿。对这个 job 来说这不是假想的漂移：同一批修复里新增的顺序断言之所以存在，正是因为拆分最初把 Check workflow file size 放到了 npm ci 的错误一侧。
建议在同一条测试里、等值循环旁边加一条顺序断言：比较共享前置链的相对顺序，并钉住 profile 生产者位于其第一个消费者之前。顺序比较必须继续把 deliberatelyDivergent（第 363 行）排除在被比较序列之外，也不能与该 job 已被钉住的两条顺序冲突：restoreIdx < checkoutIdx（.github/scripts/qwen-triage-workflow.test.mjs:154-159）与 gate < install（scripts/tests/workflow-size.test.js:52-68）。
验收标准：加上断言后，把 lint_and_static 里的 - name: 'Use trusted CI profile'（连同其 id/env/run）移到 - name: 'Install dependencies' 之下，该测试必须变红——今天同样的变异下三个改动文件的 250 个测试全绿，所以请加完后确认这个红。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| const shared = [...t.keys()].filter( | ||
| (n) => l.has(n) && !deliberatelyDivergent.has(n), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R2-3: the only structural guard this change adds compares the two jobs' name-intersection — exactly the 12 shared prelude steps — so the twenty moved lint/static steps, the payload the new job exists to run, are pinned by nothing: not their presence, not their if gates, not which job holds them. Measured across 99 test files, 18 of the 20 have no name and no command mention anywhere; the two exceptions are Check workflow file size (workflow-size.test.js:70-79) and Run i18n check (a job-blind whole-file toContain at qwen-autofix-workflow.test.js:11995).
Delete - name: 'Run Prettier' from lint_and_static, or flip its if from ci_profile == 'full' to == 'docs_only', and every suite stays green — the step is absent from the intersection, so the equality loop skips it. The mirror mutation is equally invisible: re-adding a step to test grows the intersection, and the floor below is a toBeGreaterThanOrEqual, so a conflict resolution that keeps both copies duplicates the work per PR with nothing red. The cost lands on the split's stated purpose: this lane is being prepared to become a required status check, and a required check that silently stops running ESLint, the lockfile check, the runtime-dependency audit or the schema/NOTICES drift checks still reports green on every PR and every post-merge push.
Witness:
mutation: delete lint_and_static's `- name: 'Run Prettier'` (ci.yml:1122-1124) → population green (vitest 8/8 files, node:test 203/203)
mutation: flip its `if` to `ci_profile == 'docs_only'` → population green
mutation: re-add `Run Prettier` to `test` (intersection 12 → 13) → population green
sweep over 99 test files × 20 moved steps: 18 steps have NO name and NO command mention anywhere
gate census of the twenty: index 5 `Check workflow file size` is skip_ci-only, index 14 `GitHub CI helper checks` is `github_ci_only`, the other 18 are `ci_profile == 'full'`
Pin the moved set by name and gate in this file: assert the exact ordered list of lint_and_static step names carrying steps.ci_profile.outputs.ci_profile == 'full' equals the expected payload, and assert none of those names appears in test.
Do not write a uniform-gate assertion — two of the twenty are deliberately gated differently: .github/workflows/ci.yml:993 - name: 'Check workflow file size' carries if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" on purpose (a PR touching only .github/ classifies as github_ci_only and is exactly the PR most likely to push a workflow past GitHub's 500 KB start-runs limit), and .github/workflows/ci.yml:1075 - name: 'GitHub CI helper checks' carries … ci_profile == 'github_ci_only'.
Acceptance criterion: the new assertion must go red when - name: 'Run Prettier' is deleted from lint_and_static, red when its if is changed to ci_profile == 'docs_only', and red when the same step is re-added to test; no existing test reds on any of the three, so please confirm all three.
中文说明
这次改动新增的唯一的结构性守卫，比较的是两个 job 的名字交集——恰好是 12 个共享前置步骤——因此被移动的那 20 个 lint/静态步骤（新 job 存在的意义所在）没有任何钉子：既不钉它们是否存在，也不钉它们的 if 门，也不钉它们属于哪个 job。在 99 个测试文件上实测，20 个里有 18 个在任何地方都没有名字或命令的提及；两个例外是 Check workflow file size（workflow-size.test.js:70-79）和 Run i18n check（qwen-autofix-workflow.test.js:11995 一处不区分 job 的整文件 toContain）。
从 lint_and_static 删掉 - name: 'Run Prettier'，或把它的 if 从 ci_profile == 'full' 翻成 == 'docs_only'，所有套件仍然是绿的——该步骤不在交集里，等值循环直接跳过。反方向的变异同样不可见：把步骤重新加回 test 会扩大交集，而下面的下限用的是 toBeGreaterThanOrEqual，于是一次「两边都保留」的冲突解决会让每个 PR 重复跑一遍这份工作而没有任何测试变红。代价正好落在拆分的初衷上：这条通道正在被准备成 required status check，而一个悄悄不再跑 ESLint、lockfile 检查、运行时依赖审计或 schema/NOTICES 漂移检查的 required check，在每个 PR 和每次合并后 push 上依然报绿。
建议在本文件里按名字和门把这批被移动的步骤钉住：断言 lint_and_static 中带 steps.ci_profile.outputs.ci_profile == 'full' 的步骤名有序列表恰好等于预期集合，并断言这些名字都不出现在 test 中。不要写成统一的门断言——20 个里有两个是有意用不同的门的：.github/workflows/ci.yml:993 的 Check workflow file size 故意只以 skip_ci 为门（只改 .github/ 的 PR 会被分类为 github_ci_only，而它恰恰最可能把 workflow 推过 GitHub 的 500 KB start-runs 上限），.github/workflows/ci.yml:1075 的 GitHub CI helper checks 带的是 … ci_profile == 'github_ci_only'。
验收标准：新断言必须在以下三种情况下变红——从 lint_and_static 删除 - name: 'Run Prettier'、把它的 if 改成 ci_profile == 'docs_only'、以及把同一步骤重新加回 test；现有测试对这三者都不会变红，请逐一确认。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| // run, and upload-artifact v4+ rejects duplicate artifact names, so the | ||
| // collector carries a per-job name. Everything else that shares a name | ||
| // must be byte-identical. | ||
| const deliberatelyDivergent = new Set(['Upload disk-pressure samples']); |
There was a problem hiding this comment.
[Suggestion] R2-4: the divergence exemption is keyed on the whole step, not on the one field that must differ (with.name), so Upload disk-pressure samples never enters the comparison loop at all — and the lint copy's uses: action revision ends up compared by nothing anywhere in the repository. Fixing the serialiser blindness reported at line 354 does not close this: the step is filtered out before any field is read.
Retag the lint copy's actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a to a mutable @v4, or leave it behind when test's copy is bumped to a new revision, and every suite stays green while the lane runs an unpinned, owner-repointable action revision holding the job's token. This is not a strawman drift — the comment above records that this exact job pair already drifted once (#10799), and every other uses: in ci.yml is SHA-pinned, so this is the one step in the new job whose revision no test holds.
Witness:
MUTATION retag lint copy uses: 'actions/upload-artifact@043fb46d…' → 'actions/upload-artifact@v4':
✓ scripts/tests/ci-platform-lanes.test.js (34 tests); helper node:test 78/78; qwen-triage-workflow 125/125
→ EVERY suite green with an unpinned, owner-repointable revision holding the job's token
grep 043fb46d1a93c77aae656e7c1c64a875d1fc6a0a over all *.js/*.mjs/*.ts → No matches
grep retention-days over tests → 6 matches, none for this step in either job
R2-1 interaction: deep serializer patched in + retag mutation → Test Files 1 passed (fixing the serialiser does NOT close this)
Exempt the field, not the step: build each map value from a copy with with.name deleted, then drop the deliberatelyDivergent set entirely — or assert uses: equality for the collector explicitly.
const byName = (job) =>
new Map(
(ci.jobs[job].steps ?? []).map((s) => {
const { name: _artifactName, ...rest } = s.with ?? {};
return [s.name, JSON.stringify(canon({ ...s, with: rest }))];
}),
);The narrowed comparison must keep with.name out of the equality, because .github/scripts/ci-disk-pressure.test.mjs:113-117 asserts notEqual(upload.with.name, step('Upload disk-pressure samples').with.name, 'artifact names must differ or the second failing job cannot upload') — the two guards would otherwise contradict each other.
Acceptance criterion: keeps every shared prelude step identical between test and lint_and_static must go red when the lint copy's uses: at .github/workflows/ci.yml:1069 is changed to a different revision or a mutable tag, and must stay green with the two with.name values differing as they do today; please confirm both halves.
中文说明
这个豁免是按整个步骤为键的，而不是按唯一必须不同的字段（with.name），因此 Upload disk-pressure samples 根本不会进入比较循环——lint 副本的 uses: action 版本号在整个仓库里没有任何东西在比。修掉第 354 行报告的序列化器盲区并不能关掉这个洞：该步骤在读取任何字段之前就被过滤掉了。
把 lint 副本的 actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a 改成可变的 @v4，或者在 test 副本升级到新版本号时把它落下，所有套件仍然是绿的，而这条通道会带着一个未固定、可被所有者重定向、且持有 job token 的 action 版本运行。这不是假想的漂移——上方注释就记录了这一对 job 已经漂移过一次（#10799），而 ci.yml 里其它每一个 uses: 都是 SHA 固定的，所以这是新 job 里唯一一个版本号没有测试把住的步骤。
建议豁免字段而不是整个步骤：用删掉 with.name 的副本来构造 map 的值，然后彻底去掉 deliberatelyDivergent 集合；或者显式断言收集器步骤的 uses: 相等。收窄后的比较必须继续把 with.name 排除在等值之外，因为 .github/scripts/ci-disk-pressure.test.mjs:113-117 断言 notEqual(upload.with.name, step('Upload disk-pressure samples').with.name, 'artifact names must differ or the second failing job cannot upload')，否则两个守卫会互相矛盾。
验收标准：把 .github/workflows/ci.yml:1069 处 lint 副本的 uses: 改成另一个版本号或可变标签时，keeps every shared prelude step identical between test and lint_and_static 必须变红；而在两个 with.name 保持现有差异的情况下必须仍然是绿的。请确认这两半。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| // dispatch and push; widening this — say by copying `test`'s block when | ||
| // adding a reporter — hands write tokens to that code. `test`'s own | ||
| // wider block is legitimate: dorny/test-reporter needs `checks: write`. | ||
| expect(ci.jobs.lint_and_static.permissions).toEqual({ contents: 'read' }); |
There was a problem hiding this comment.
[Suggestion] R2-5: this change adds five pins for lint_and_static — timeout, gate equality, permissions, prelude identity, checkout ref — but nothing pins its runs-on, the one line of the new job that decides where the contributor code this very test's comment names actually executes.
ci.yml:800 routes through classify_pr, whose Select Linux runner step is the fork-trust boundary: for a pull_request where SAME_REPO=false, the author association is not OWNER|MEMBER|COLLABORATOR, and the event is not merge_group/push, it emits ["ubuntu-latest"]. Replace that line with a literal ["self-hosted", "linux", "x64", "ecs-qwen"] and the indirection is bypassed whole, so an untrusted fork PR's npm ci — which runs the PR's own lifecycle scripts and, via scripts/prepare.js, a full build and bundle of attacker-chosen source — executes on the persistent, workspace-reusing pool with its shared $HOME/.cache/qwen-code/npm. Nothing detects it. A second consequence rides along: timeout-minutes still reads ubuntu_runner, which still reports ["ubuntu-latest"] for that fork, so the mutated lane lands on cold-cache pool hardware under the hosted 45-minute ceiling — the configuration this job's own comment records as beheaded twice. To be precise about severity: on pull_request events the PR supplies its own ci.yml, so the indirection was never a boundary against a malicious PR's own diff; what such a pin protects is main-branch state against a maintainer-side regression, which is what every other pin here protects too.
Witness:
MUTATION ci.yml:800 → runs-on: '["self-hosted", "linux", "x64", "ecs-qwen"]' (YAML parses; test's runs-on untouched):
vitest ci-platform-lanes + workflow-size + no-ak-integration-ci + review-worktree-cleanup-workflow: 271/271
node:test ci-runner-routing + ci-disk-pressure: 26/26; qwen-triage-workflow: 125/125
FULL population: scripts/tests 2110 (2096 passed, 11 skipped, 3 harness-only failures that reproduce on the restored tree or pass in isolation)
HELPER_TESTS 23 files: 510/510 → 2620 tests, zero attributable failures
real pick_runner executed (SAME_REPO=false AUTHOR_ASSOCIATION=NONE ECS_DISABLED= EVENT_NAME=pull_request):
Selected Linux runner: ["ubuntu-latest"] ← the mutation stops reading this output entirely
timeout desync, both arms probed:
MUTATED: runs-on resolves to ["self-hosted","linux","x64","ecs-qwen"] effective timeout-minutes: 45
INTACT : runs-on resolves to ["ubuntu-latest"] effective timeout-minutes: 45
pin census: 5 ci.yml Linux jobs route through ubuntu_runner; pins exist for 2; test and integration_cli are pre-existing gaps, this diff adds a third
Add beside the permissions pin in the same describe:
it('routes lint_and_static through the classify_pr fork-trust output', () => {
expect(String(ci.jobs.lint_and_static['runs-on'])).toBe(
String(ci.jobs.test['runs-on']),
);
expect(String(ci.jobs.lint_and_static['runs-on'])).toBe(
"${{ fromJSON(needs.classify_pr.outputs.ubuntu_runner || '[\"ubuntu-latest\"]') }}",
);
});Match the whole fromJSON(...) expression rather than a substring, as scripts/tests/no-ak-integration-ci.test.js:548-550 already does for test_windows — scripts/tests/review-worktree-cleanup-workflow.test.js:58-63 is not a pin here: its enumeration filter /ubuntu_runner|ecs-qwen/ deliberately matches either the indirection or a hardcoded ecs-qwen label array ("jobs can also hard-code the shared label array"), so it admits the literal form.
Acceptance criterion: that new test must go red under the mutation above (ci.yml:800 → a literal pool label array), which today leaves 2620 tests green; both arms were run here — intact 35 passed, mutated 1 failed | 34 passed — so please confirm the red on your side.
中文说明
这次改动为 lint_and_static 新增了五个钉子——timeout、门相等、permissions、前置链一致、checkout ref——但没有钉它的 runs-on，而这一行正是决定「这条测试自己的注释所点名的贡献者代码究竟在哪里执行」的唯一一行。
ci.yml:800 经过 classify_pr 路由，其 Select Linux runner 步骤就是 fork 信任边界：对于 SAME_REPO=false、作者关联不是 OWNER|MEMBER|COLLABORATOR、事件也不是 merge_group/push 的 pull_request，它输出 ["ubuntu-latest"]。把这一行换成字面量 ["self-hosted", "linux", "x64", "ecs-qwen"]，这层间接就被整个绕过，于是一个不可信 fork PR 的 npm ci——它会执行该 PR 自己的生命周期脚本，并经 scripts/prepare.js 对攻击者选定的源码做完整 build 和 bundle——就会跑在会复用工作区的持久化机群上，共享 $HOME/.cache/qwen-code/npm。没有任何测试会发现。还有第二个后果：timeout-minutes 仍然读 ubuntu_runner，而它对那个 fork 仍然报 ["ubuntu-latest"]，于是变异后的通道会落在冷缓存的机群硬件上、却带着托管机的 45 分钟上限——正是这个 job 自己的注释记录过两次被斩首的配置。关于严重度的准确说法：在 pull_request 事件上 PR 自带 ci.yml，所以这层间接从来不是抵御恶意 PR 自身 diff 的边界；这类钉子保护的是 main 分支状态不受维护者侧的回归影响，这也是这里其它每个钉子保护的东西。
建议在同一 describe 里、permissions 钉子旁边加上对整个 fromJSON(...) 表达式的相等断言（参照 scripts/tests/no-ak-integration-ci.test.js:548-550 对 test_windows 的写法），而不是子串匹配——scripts/tests/review-worktree-cleanup-workflow.test.js:58-63 在这里不构成钉子：它的枚举过滤器 /ubuntu_runner|ecs-qwen/ 有意同时匹配间接形式和硬编码的 ecs-qwen 标签数组（「job 也可以硬编码共享标签数组」），因此它会放行字面量形式。
验收标准：新测试必须在上述变异下变红（ci.yml:800 换成字面量机群标签数组），而今天这个变异下 2620 个测试全绿；本次两侧都跑过——原树 35 通过、变异后 1 failed | 34 passed——请在你这边确认这个红。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| const lintJob = getWorkflowJob(workflow, 'lint_and_static'); | ||
| const guardCalls = { | ||
| test: getWorkflowStep(ubuntuJob, GUARD_STEP), | ||
| lint_and_static: getWorkflowStep(lintJob, GUARD_STEP), |
There was a problem hiding this comment.
[Suggestion] R2-6: this change makes lint_and_static a third Use trusted CI profile consumer (.github/workflows/ci.yml:971, beside test at :543 and integration_no_ak at :1877) and extends this map for it, but the sibling enumeration at lines 299-304 — whose comment still reads "Both consumers … Neither may execute a classifier from the PR checkout" — was left covering two jobs, so the new lane's trusted-profile step sits outside every semantic pin on that contract.
That loop's toContain("TRUSTED_CI_PROFILE: '${{ needs.classify_pr.outputs.ci_profile }}'") and degraded-path-warning assertions are the only pins on the step's env: binding. Drop or repoint that one line in lint_and_static and profile="${TRUSTED_CI_PROFILE:-full}" (ci.yml:980) silently falls back to full: every docs_only and github_ci_only PR then runs the whole 33-step lane — npm ci, ESLint, shellcheck, actionlint, bundle closure — on the contended pool, with no degraded-path warning to distinguish it from a genuine full classification and every suite green. That is precisely the regression this same file already names at lines 289-291 ("empties the output and every consumer silently falls back to full — fail-safe direction, but a silent perf regression nothing would notice").
Witness:
MUTATION (scratch tree): lint_and_static's env repointed TRUSTED_CI_PROFILE: '${{ needs.classify_pr.outputs.ci_profile }}' → 'full'
vitest scripts suite: 2 failed | 74 passed, Tests 2 failed | 2097 passed | 11 skipped ← byte-identical to baseline
helper node:test suites: # pass 78 / # fail 0 ; qwen-triage-workflow.test.mjs: # pass 125 / # fail 0
→ NO suite moves
A/B on the step body extracted verbatim from each arm's YAML, run with classify_pr's output stubbed to docs_only:
INTACT: TRUSTED_CI_PROFILE=docs_only stdout="Selected CI profile: docs_only" GITHUB_OUTPUT="ci_profile=docs_only"
MUTANT: TRUSTED_CI_PROFILE=full stdout="Selected CI profile: full" GITHUB_OUTPUT="ci_profile=full"
(and no degraded-path ::warning — indistinguishable from a genuine full classification)
Enumeration: grep 'Use trusted CI profile' → only :302 (ubuntuJob='test') and :303 (gateJob='integration_no_ak'); the lint job appears in none of them
Resolve the lint job in that it and add it to the loop, and reword the comment to "Every consumer … none may execute a classifier from the PR checkout":
for (const profileStep of [
getWorkflowStep(ubuntuJob, 'Use trusted CI profile'),
getWorkflowStep(getWorkflowJob(workflow, 'lint_and_static'), 'Use trusted CI profile'),
getWorkflowStep(gateJob, 'Use trusted CI profile'),
]) {Better still, derive the list the way scripts/tests/review-worktree-cleanup-workflow.test.js:59-71 derives its sweep copies (enumerate jobs containing a step named Use trusted CI profile) so the next consumer fails here instead of on the runners. The fix must stay test-only: scripts/tests/ci-platform-lanes.test.js:370-373 requires every same-named step to be byte-identical between the two jobs, and all three copies are currently identical, so the added entry passes without editing ci.yml — but any "correction" made to the lint copy while adding it would trip that guard.
Acceptance criterion: after the loop covers the lint copy, changing .github/workflows/ci.yml:975 from TRUSTED_CI_PROFILE: '${{ needs.classify_pr.outputs.ci_profile }}' to TRUSTED_CI_PROFILE: 'full' must turn this file red; today npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/no-ak-integration-ci.test.js scripts/tests/ci-platform-lanes.test.js stays green under that mutation, so please confirm the red.
中文说明
这次改动让 lint_and_static 成为 Use trusted CI profile 的第三个消费者（.github/workflows/ci.yml:971，与 :543 的 test、:1877 的 integration_no_ak 并列），也为它扩展了这里的 map；但第 299-304 行的同族枚举——注释仍写着「Both consumers … Neither may execute a classifier from the PR checkout」——只覆盖两个 job，于是新通道的 trusted-profile 步骤落在该契约所有语义钉子之外。
那个循环里的 toContain("TRUSTED_CI_PROFILE: '${{ needs.classify_pr.outputs.ci_profile }}'") 和降级路径告警断言，是这个步骤 env: 绑定的唯一钉子。把 lint_and_static 里这一行删掉或改指向，profile="${TRUSTED_CI_PROFILE:-full}"（ci.yml:980）就会静默回落到 full：于是每个 docs_only 和 github_ci_only 的 PR 都会在紧张的机群上跑完这条 33 步通道——npm ci、ESLint、shellcheck、actionlint、bundle closure——而且没有降级路径告警来把它和真正的 full 分类区分开，所有套件全绿。这正是本文件第 289-291 行已经点名的那种回归（「清空输出后每个消费者都静默回落到 full——方向上是 fail-safe，但是一种没人会注意到的静默性能回归」）。
建议在那个 it 里解析出 lint job 并加入循环，同时把注释改成「Every consumer … none may execute a classifier from the PR checkout」；更好的做法是像 scripts/tests/review-worktree-cleanup-workflow.test.js:59-71 推导其 sweep 副本那样推导这个列表（枚举所有含名为 Use trusted CI profile 步骤的 job），这样下一个消费者会在这里失败，而不是在 runner 上失败。修复必须只动测试：scripts/tests/ci-platform-lanes.test.js:370-373 要求两个 job 之间所有同名步骤逐字节一致，而三份副本目前完全一致，所以新增条目无需改 ci.yml 就能通过——但在添加时顺手「修正」lint 副本会触发那个守卫。
验收标准：循环覆盖 lint 副本之后，把 .github/workflows/ci.yml:975 从 TRUSTED_CI_PROFILE: '${{ needs.classify_pr.outputs.ci_profile }}' 改成 TRUSTED_CI_PROFILE: 'full' 必须让本文件变红；今天在该变异下 npx vitest run --config ./scripts/tests/vitest.config.ts scripts/tests/no-ak-integration-ci.test.js scripts/tests/ci-platform-lanes.test.js 仍然是绿的，请确认这个红。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| name: 'Lint & Static (ubuntu-latest, Node 22.x)' | ||
| needs: 'classify_pr' | ||
| # Split out of `test` so this half can become a required status check on | ||
| # its own: these steps are deterministic (no test execution, no shared |
There was a problem hiding this comment.
[Suggestion] R2-7: the banner and header carrying this lane's required-check rationale overstate it twice, and both contradictions are inside this same job. (a) "no shared ECS worker contention" here is answered six lines below by the timeout comment this head commit rewrote: "this lane was beheaded twice running honestly on cold-cache hk3-pool runners … The pool's contention is why test runs 120/60". (b) The banner at line 786 ("everything that judges the tree without executing it") and "no test execution" here are false for three of the lane's own steps: Check serve fast-path bundle closure cleans, builds and bundles the CLI, and GitHub CI helper checks plus Run .github/scripts helper tests execute the 23-file HELPER_TESTS suite, 18 of whose members spawn subprocesses.
This block is the recorded justification for the follow-up the PR body names — adding Lint & Static (ubuntu-latest, Node 22.x) to the main ruleset as a required status check. A maintainer acting on "no test execution" and "no contention" makes a cold-pool build/bundle (this job's own numbers: install 940s, bundle closure 681s vs a warm 171s) and a subprocess-spawning test suite merge-blocking, which is the hostage-taking the split exists to end; and the banner also reads as the admission test for whatever moves into the lane next, so the next executing step gets admitted on a false criterion. Separately, all four incidents cited as the motivation failed in Run tests and generate reports, a step that stayed in test, so the required check argued for here would have been green on all four merges — the description already concedes that, so only this comment needs to stop implying otherwise.
Witness:
ci.yml:786 # Lint & Static: everything that judges the tree without executing it.
ci.yml:792 # its own: these steps are deterministic (no test execution, no shared
ci.yml:801-805 (rewritten by head commit e1fbabdd34)
# Routed like test's, not a bare constant: this lane was beheaded twice
# running honestly on cold-cache hk3-pool runners — at 30 minutes flat
# (install 892s vs the sibling Test job's 347s) and again at 45 … The pool's
# contention is why test runs 120/60 and no_ak 60/30 …
package.json:98 "check:serve-fast-path-bundle": "node scripts/clean-package-build-artifacts.js && npm run build -- --cli-only && cross-env DEV=true npm run bundle && node scripts/check-serve-fast-path-bundle.js"
ci.yml:1075 / 1178-1181 → run: 'node --test --test-concurrency=1 ${{ env.HELPER_TESTS }}'
sweep: HELPER_TESTS members = 23; members importing child_process/spawnSync/execFileSync/spawn( = 18
gh api runs/33515371387|33526417145|33533976387|33549134248/jobs → failing step in every case:
Test (ubuntu-latest, Node 22.x) failure Run tests and generate reports (×3)
Test (windows-latest, Node 22.x) failure Run tests and generate reports (×1)
Reword lines 786-796 to what the evidence supports: the lane is deterministic in outcome (no vitest unit-test run, no coverage) but does build and bundle for the serve-fast-path closure check and does execute the .github/scripts helper suite, and it runs on the same contended pool — hence the routed 90/45 ceiling. Keep the required-check rationale on the property that is true: the 41% flake measurement was of Run tests and generate reports, a step that stayed in test.
The correction must be to the comment, not to the steps or the ceiling: scripts/tests/ci-platform-lanes.test.js:499-514 pins ci-disk-pressure.test.mjs as a HELPER_TESTS member and --test-concurrency=1 on every step whose run contains env.HELPER_TESTS, and :100 pins timeoutMinutesOn('lint_and_static', ECS_RUNNER) at 90 — an allowance that exists precisely because of the contention the header denies.
中文说明
承载本通道 required-check 论证的横幅与头部注释有两处夸大，而两处矛盾都在同一个 job 内部。(a) 这里的「no shared ECS worker contention」在六行之下就被本 head commit 重写过的 timeout 注释反驳了：「this lane was beheaded twice running honestly on cold-cache hk3-pool runners … The pool's contention is why test runs 120/60」。(b) 第 786 行的横幅（「everything that judges the tree without executing it」）和这里的「no test execution」对本通道自己的三个步骤是不成立的：Check serve fast-path bundle closure 会清理、构建并打包 CLI，GitHub CI helper checks 与 Run .github/scripts helper tests 会执行 23 个文件的 HELPER_TESTS 套件，其中 18 个成员会派生子进程。
这一段正是 PR 正文点名的后续动作（把 Lint & Static (ubuntu-latest, Node 22.x) 加入 main ruleset 作为 required status check）的书面理由。若维护者依据「不执行测试」和「无机群争用」去操作，就会把一个冷缓存机群上的 build/bundle（本 job 自己的数字：install 940s、bundle closure 681s 对暖缓存 171s）和一个派生子进程的测试套件变成阻断合并的东西——而这恰恰是拆分想要终结的「人质」局面；同时这段横幅也读作「下一步什么可以搬进这条通道」的准入标准，于是下一个会执行的步骤会凭一个错误的标准被放进来。另外，被列为动因的四次事故全部失败在 Run tests and generate reports，而该步骤留在了 test，所以这里论证的 required check 在那四次合并上都会是绿的——PR 正文已经承认这一点，因此只需要让这段注释不再暗示相反的事实。
建议把第 786-796 行改写成证据支持的说法：本通道在结果上是确定性的（不跑 vitest 单测、不出覆盖率），但确实会为 serve-fast-path 闭合检查做 build 和 bundle，也确实会执行 .github/scripts 辅助套件，并且它跑在同一个存在争用的机群上——所以才有按路由的 90/45 上限。required-check 的论证请留在那条成立的性质上：41% 的抖动测量针对的是 Run tests and generate reports，而那个步骤留在了 test。
修正必须落在注释上，而不是步骤或上限上：scripts/tests/ci-platform-lanes.test.js:499-514 钉住了 ci-disk-pressure.test.mjs 是 HELPER_TESTS 成员、并钉住每个 run 含 env.HELPER_TESTS 的步骤都带 --test-concurrency=1，:100 钉住了 timeoutMinutesOn('lint_and_static', ECS_RUNNER) 为 90——而这个额度存在的理由，正是头部注释所否认的那种群争用。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| ); | ||
| }); | ||
|
|
||
| it('gives lint_and_static the same sampler and its own collector', () => { |
There was a problem hiding this comment.
[Suggestion] R2-8: this new pin certifies the lane's #10035 telemetry, but the lane's only sampler producer lives inside Install dependencies and is reaped by that step's trap … EXIT, so the 19 substantive steps after it write no samples and there is no failure-time df/meminfo dump. test has both halves — a second sampler inside Run tests and generate reports plus an RC-gated df -hT; df -i; grep MemAvailable dump — and the lint lane has no analogue.
To be fair about scope: the thinness itself is a faithfully copied pre-existing property, not a regression — at the merge base the same 18 steps already sat between test's two producers, so an ENOSPC death in Run ESLint or the bundle-closure step already produced an install-phase-only artifact. What is new here is the claim attached to the copy (ci.yml:1058-1066: "Without it the lane produces the telemetry and destroys it with the runner temp dir on the exact ENOSPC death the sampler exists to explain") and this pin certifying it, which together overstate the coverage for precisely the lane's heaviest steps. Note this is a different axis from the collector-position finding at ci.yml:1067: moving the collector to the end makes the artifact upload, with nothing new in it.
Witness:
grep -n DISK_SAMPLES .github/workflows/ci.yml → producers at 608 (test/Install), 707 (test/Run tests), 1041 (lint/Install); no fourth
parsed step maps:
lint job DISK_SAMPLES producers: 11:Install dependencies (ONE)
test job DISK_SAMPLES producers: 11:Install dependencies | 14:Run tests and generate reports [+failure df dump]
behavioural probe (lint install step's verbatim run:, `npm ci` stubbed to `sleep 2`, RUNNER_TEMP=$tmp):
samples at step exit: 1
samples 25s after step exit: 1 ← the trap … EXIT reaped the sampler; nothing writes during the 19 later steps
BASE arm (git show 3e8d92a7:.github/workflows/ci.yml, job `test`):
13 Install dependencies PRODUCER / 15-32 the 18 moved steps, no producer / 34 Run tests PRODUCER,DF-DUMP / 35 collector
→ the same 18 steps already sat between test's two producers, so the thinness is copied, not introduced
Either mirror test's second sampler into the lane's disk-hungry step (minimally Check serve fast-path bundle closure), appending to the same ${RUNNER_TEMP}/disk-pressure-samples.log, or add one if: '${{ failure() }}'-gated df -hT; df -i; grep -E 'MemTotal|MemAvailable' /proc/meminfo step immediately before the collector so the upload carries the state at failure time. If neither is wanted, narrow the comment and this pin to what the lane actually records (install-phase telemetry only).
Any added producer must reuse the guarded form this file pins at line 60 — DISK_SAMPLES="${RUNNER_TEMP}/disk-pressure-samples.log" followed by if [ ! -s "$DISK_SAMPLES" ]; then … fi — not a bare >, which would truncate the install-window samples the collector uploads; and lines 69-84 require assert.equal(headerLine(install), headerLine(tests)) and assert.equal(sampleFormat(install), sampleFormat(tests)), so a second producer must emit an identical DISKCONTEXT header and DFSAMPLE format.
Acceptance criterion: extend this test to assert that a lint_and_static step after Install dependencies also starts a sampler appending DFSAMPLE lines to that file (or, for the df-dump variant, that a failure-gated dump step exists); removing the added producer must then turn it red while all 250 currently-green tests in ci-platform-lanes/workflow-size/no-ak-integration-ci still pass.
中文说明
这个新钉子认证了本通道的 #10035 遥测，但本通道唯一的采样生产者位于 Install dependencies 内部，并被该步骤的 trap … EXIT 回收，因此其后的 19 个实质步骤不写任何样本，也没有失败时刻的 df/meminfo 转储。test 两半都有——Run tests and generate reports 内部的第二个采样器，加上一个以 RC 为门的 df -hT; df -i; grep MemAvailable 转储——而 lint 通道没有对应物。
关于范围需要公道地说：这种「薄」本身是被忠实复制过来的既有属性，不是回归——在合并基线上，同样的 18 个步骤本来就位于 test 的两个生产者之间，所以在 Run ESLint 或 bundle-closure 步骤中因 ENOSPC 死掉，本来也只能得到一份仅覆盖安装阶段的产物。这里新出现的是附着在这份副本上的说法（ci.yml:1058-1066：「Without it the lane produces the telemetry and destroys it with the runner temp dir on the exact ENOSPC death the sampler exists to explain」）以及为它背书的这个钉子，两者合起来恰好在本通道最重的步骤上夸大了覆盖范围。请注意这与 ci.yml:1067 的收集器位置问题是不同的轴：把收集器移到末尾只会让产物被上传，而里面没有新内容。
建议二选一：把 test 的第二个采样器镜像到本通道吃磁盘的步骤里（至少是 Check serve fast-path bundle closure），追加写入同一个 ${RUNNER_TEMP}/disk-pressure-samples.log；或者在收集器之前加一个以 if: '${{ failure() }}' 为门的 df -hT; df -i; grep -E 'MemTotal|MemAvailable' /proc/meminfo 步骤，让上传的产物带上失败时刻的状态。如果两者都不想做，请把注释和这个钉子收窄到本通道实际记录的内容（仅安装阶段遥测）。
任何新增的生产者都必须复用本文件第 60 行钉住的带守卫写法——DISK_SAMPLES="${RUNNER_TEMP}/disk-pressure-samples.log" 后接 if [ ! -s "$DISK_SAMPLES" ]; then … fi——而不是裸的 >（那会截断收集器要上传的安装窗口样本）；第 69-84 行要求 assert.equal(headerLine(install), headerLine(tests)) 与 assert.equal(sampleFormat(install), sampleFormat(tests))，所以第二个生产者必须输出完全一致的 DISKCONTEXT 头部和 DFSAMPLE 格式。
验收标准：扩展本测试，断言 lint_and_static 中位于 Install dependencies 之后的某个步骤也会启动一个向该文件追加 DFSAMPLE 行的采样器（若采用 df 转储方案，则断言存在一个以 failure 为门的转储步骤）；此后删除新增的生产者必须让它变红，同时 ci-platform-lanes/workflow-size/no-ak-integration-ci 中当前全绿的 250 个测试仍然通过。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
| - name: 'Upload disk-pressure samples' | ||
| if: '${{ failure() }}' | ||
| uses: 'actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a' # v7.0.1 |
There was a problem hiding this comment.
[Suggestion] R1-6: (fix-induced) the collector this step was added for now sits at index 13 of 33 — before all 19 substantive lint/static steps — so it can only ever collect a prelude failure. Round 1 asked for the missing failure()-gated collector and ef2555d added exactly that, with the right fields; what the fix introduced is the position. if: failure() is evaluated once, when the step is reached, and the runner never revisits it, so on every failure mode this lane actually has the collector was already passed and skipped while nothing had failed.
When the job dies in Run ESLint (index 20), Generate settings schema, Check serve fast-path bundle closure (index 31, the step this job's own comment prices at 681s cold vs 171s warm) or Run .github/scripts helper tests (index 32) — including the disk/IO-pressure death the sampler exists to explain — ${RUNNER_TEMP}/disk-pressure-samples.log is destroyed with the runner temp dir and the failure reads as an unexplained flake. That is the outcome the comment directly above says this step exists to prevent. test does not have the hole because its collector (index 15) follows its heavy step (index 14). One precision from verification: the ENOSPC-mid-install death the comment names is covered — install is index 11 and Report npm cache usage is always() — so the loss is confined to downstream failures.
Witness:
lint_and_static step indices at HEAD (33 steps): 11 Install dependencies, 12 Report npm cache usage,
13 Upload disk-pressure samples (if: ${{ failure() }}), 14 GitHub CI helper checks, 20 Run ESLint,
31 Check serve fast-path bundle closure, 32 Run .github/scripts helper tests
test step indices (20 steps): 11 Install dependencies, 14 Run tests and generate reports,
15 Upload disk-pressure samples ← the mirror fires because it follows the heavy step
AUTHORITY (docs.github.com/en/actions/reference/workflows-and-actions/expressions, fetched):
"failure — Returns true when any previous step of a job fails."
sampler coverage: test's heavy step runs its own sampler loop; in lint_and_static only step 11 touches DISK_SAMPLES
FIX VERIFIED (scratch tree, collector moved to index 32, last step):
ci-disk-pressure.test.mjs → # tests 3 # pass 3 # fail 0
ci-platform-lanes + workflow-size → Test Files 2 passed, Tests 236 passed
Move this whole step block to the end of lint_and_static's step list (after Run .github/scripts helper tests), keeping if: '${{ failure() }}' and the disk-pressure-lint-… artifact name, and move its six-line explanatory comment with it. failure() matches any earlier failed step regardless of distance, so prelude coverage is unchanged and body coverage is added; no always() is needed.
The move must keep the step's name and fields: .github/scripts/ci-disk-pressure.test.mjs:104-115 asserts upload.if === '${{ failure() }}', upload.with['if-no-files-found'] === 'ignore', upload.with.path === '${{ runner.temp }}/disk-pressure-samples.log' and assert.notEqual on the artifact name; the name is also the exclusion key in scripts/tests/ci-platform-lanes.test.js:363's deliberatelyDivergent set, whose expect(shared.length).toBeGreaterThanOrEqual(12) floor is met exactly today.
Acceptance criterion: add an ordering assertion to .github/scripts/ci-disk-pressure.test.mjs's gives lint_and_static the same sampler and its own collector — the index of Upload disk-pressure samples must be greater than the index of Run .github/scripts helper tests. With the collector at index 13 that assertion is red and after the move it is green (measured here: 3/3 pass plus 236 tests across the two suites), so please confirm the red before moving it.
中文说明
（由上一轮修复引入）这个步骤被加进来所要服务的收集器，现在位于 33 个步骤中的第 13 位——在全部 19 个实质 lint/静态步骤之前——因此它永远只能收集到前置链的失败。上一轮指出缺少以 failure() 为门的收集器，ef2555d613 按要求加了，字段也都对；这次修复引入的问题是位置。if: failure() 只在执行到该步骤时求值一次，runner 不会回头再评估，所以在本通道真正会有的每一种失败下，收集器都已经在「什么都没失败」的时候被跳过并过去了。
当 job 死在 Run ESLint（第 20 位）、Generate settings schema、Check serve fast-path bundle closure（第 31 位，也就是本 job 注释里标价为冷缓存 681s、暖缓存 171s 的那一步）或 Run .github/scripts helper tests（第 32 位）时——包括采样器正是为了解释的磁盘/IO 压力死亡——${RUNNER_TEMP}/disk-pressure-samples.log 会随 runner 临时目录一起被销毁，失败看起来就是一次无法解释的抖动。这正是紧挨上方的注释所说「这个步骤存在的目的」是要防止的结果。test 没有这个洞，因为它的收集器（第 15 位）排在它的重步骤（第 14 位）之后。验证给出的一点精确化：注释点名的「安装过程中 ENOSPC 死亡」是被覆盖的——install 在第 11 位，而 Report npm cache usage 带 always()——所以损失仅限于下游失败。
建议把整个步骤块移到 lint_and_static 步骤列表的末尾（Run .github/scripts helper tests 之后），保留 if: '${{ failure() }}' 和 disk-pressure-lint-… 产物名，并把它那六行说明注释一起移过去。failure() 会匹配任意更早失败的步骤，与距离无关，所以前置链的覆盖不变、同时补上了主体部分的覆盖；不需要 always()。
移动时必须保留该步骤的名字与字段：.github/scripts/ci-disk-pressure.test.mjs:104-115 断言 upload.if === '${{ failure() }}'、upload.with['if-no-files-found'] === 'ignore'、upload.with.path === '${{ runner.temp }}/disk-pressure-samples.log' 以及产物名的 assert.notEqual；这个名字同时也是 scripts/tests/ci-platform-lanes.test.js:363 中 deliberatelyDivergent 集合的排除键，而该集合的 expect(shared.length).toBeGreaterThanOrEqual(12) 下限目前刚好卡满。
验收标准：给 .github/scripts/ci-disk-pressure.test.mjs 的 gives lint_and_static the same sampler and its own collector 加一条顺序断言——Upload disk-pressure samples 的下标必须大于 Run .github/scripts helper tests 的下标。收集器在第 13 位时该断言是红的，移动之后是绿的（本次实测：3/3 通过，加上两个套件的 236 个测试），所以请在移动之前先确认这个红。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
3 participants
What this PR does
Moves the twenty deterministic lint/static steps out of the
testjob into a newlint_and_staticjob — same event surface (ifis equal totest's, pinned by a test), same runner routing throughclassify_pr, same release-sync convention (runs on skip_ci PRs, no-ops per step, so a required context still reports), with its own copy of the checkout/node/install prelude and of the CI-profile classifier.testkeeps the unit-test run, the Chrome-extension scan, and the reporting steps; itsci_profileoutput, whichweb_shell_e2e_smokeconsumes, is untouched. The surgery is a pure move: no step was edited on the way.Why it's needed
mainstill carries zero required status checks — on 09-01 a duplicate-declaration compile error merged and sat red onmainuntil #10729 (the post-merge lane from #10345 caught it and filed the issue, but nothing had blocked the merge). The path to a required check runs through this split: the oldtestjob bundled twenty deterministic static steps with a unit-test run that failed 41% of post-merge runs on flake alone this week, and a job that flaky can never be made required without red-walling every PR. Splitting lets the deterministic half become a required check on its own while the flake in the unit-test half is fixed independently.Honest scope note: this lane could not have caught the 09-01 incident itself. That was an esbuild transform error inside a test file; ESLint accepts the duplicate binding (verified against the actual broken file), and web-shell's tsconfig excludes
*.test.tsx, so typecheck never sees it either. Catching that class requires the unit-test lane — which is exactly why the halves must be separable: the lint half should not wait for the flake work, and the flake work should not be pressured by the lint half's hostages.Follow-up (admin, after this lands and runs green for a few days): add
Lint & Static (ubuntu-latest, Node 22.x)to themainruleset as a required status check. Not part of this PR.Reviewer Test Plan
How to verify
Three properties, each mechanically checkable:
1. Pure move. At YAML level, old
test's 40 steps must equal the union of newtest(20) andlint_and_static(32, of which 12 are the shared prelude), field-for-field, and every other job must be untouched:2. Event surface. Evaluating every job's
iffor every event on both trees (the substitute-then-evaluate techniqueci-platform-lanes.test.jsnow uses) must show exactly one new row —lint_and_staticon push / pull_request / merge_group / workflow_dispatch, not schedule — and no other cell changed.3. Guards.
npx vitest run --config ./scripts/tests/vitest.config.ts ci-platform-lanes workflow-size no-ak-integration-ci— includes the newlint_and_static shares test's exact gateequality pin and the updated push-lane exclusivity set.Evidence (Before & After)
Event-matrix diff against
origin/main(77d41f4) — one added row, nothing else:Pure-move proof output:
集合守恒: True · 语义漂移: 无 · 其它 job 变化: 无 · lint.if == test.if: TrueMutation checks (each must redden, did redden): lint gate silently dropping push → 2 failed; lint gate silently joining the nightly → 2 failed. Suites: ci-platform-lanes + workflow-size + no-ak + main-ci-failure-issue + capture-tmux-ci + sdk-java + serve-ab + qwen-pr-review = 464 passed; ci-runner-routing 23/23; actionlint clean; prettier clean.
ESLint-cannot-catch-it evidence (scope note above): re-injecting #10729's duplicate
language = 'en'binding intoChatEditor.test.tsxand runningnpx eslinton it exits 0.Tested on
Workflow YAML plus its guard tests; suites run locally on macOS, Linux covered by this PR's CI.
Environment (optional)
N/A — workflow and test changes only.
Risk & Scope
lint_and_staticjob's real-world p50/p90 (predicted ~13–15 min from step timings; worth reading off actual runs before the ruleset change).Test (ubuntu-latest, Node 22.x)and outputs are unchanged; a new check nameLint & Static (ubuntu-latest, Node 22.x)appears.Linked Issues
Refs #10721, #10736, #10740, #10744 (the 09-01 main-red incident that motivates the required-check path).
中文说明
这个 PR 做了什么
把
testjob 里 20 个确定性的 lint/静态检查步骤挪到新的lint_and_staticjob —— 事件面与test完全一致（if相等，且有测试钉住这个相等）、同样经classify_pr做 runner 路由、同样遵守 release-sync 约定（skip_ci PR 上照常运行、逐步骤 no-op，让 required context 仍能上报），并自带一份 checkout/node/install 前置链与 CI-profile 分类器副本。test保留单测、Chrome 扩展扫描与报告步骤；它对外的ci_profile输出（web_shell_e2e_smoke在消费）不受影响。这次手术是纯移动：没有任何步骤在挪动途中被编辑。为什么需要它
main至今没有任何 required status check —— 09-01 一个重复声明的编译错误合入后在main上红了半小时，直到 #10729 修复（#10345 的合并后 lane 抓到了它并自动开了 issue，但没有任何东西阻止那次合并）。通往 required check 的路要先经过这次拆分：旧testjob 把 20 个确定性静态步骤和一个本周实测 41% 合并后运行因抖动而红的单测跑捆在一起，这么抖的 job 永远不能设为 required，否则会红墙所有 PR。拆开之后，确定性的那一半可以独立成为 required check，而单测那一半的抖动治理可以独立推进。诚实的范围说明：这条 lane 抓不住 09-01 那次事故本身。那是测试文件内的 esbuild transform 错误；ESLint 接受那个重复绑定（用真实的坏文件验证过），web-shell 的 tsconfig 又把
*.test.tsx排除在 typecheck 之外。要挡住那一类错误需要单测 lane —— 这恰恰是两半必须可分离的原因：lint 半边不该等抖动治理，抖动治理也不该被 lint 半边的人质绑架。后续（管理员操作，待本 PR 合入并稳定运行几天后）：在
mainruleset 中把Lint & Static (ubuntu-latest, Node 22.x)加为 required status check。不在本 PR 范围内。评审验证方案
如何验证
三个性质，均可机械检查：1. 纯移动 —— YAML 层面旧
test的 40 步必须逐字段等于新test（20）与lint_and_static（32，其中 12 步为共享前置链）的并集，且其它每个 job 逐字节不变（验证脚本见上方英文正文）。2. 事件面 —— 在两棵树上对每个 job 的if逐事件求值，差异必须恰好是一行新增：lint_and_static在 push / pull_request / merge_group / workflow_dispatch 运行、不在 schedule 运行，其余格子全部不变。3. 守卫 —— 运行ci-platform-lanes workflow-size no-ak-integration-ci套件，包含新的门相等钉与更新后的 push 独占集。证据（前后对比）
对
origin/main（77d41f48）的事件矩阵 diff 恰好一行新增（见上方英文正文）。纯移动证明输出：集合守恒 True、语义漂移无、其它 job 变化无、lint.if == test.if True。变异检查（都必须变红、也都变红了）：lint 门静默丢 push → 2 failed；lint 门静默混上 nightly → 2 failed。套件合计 464 passed；路由测试 23/23；actionlint 与 prettier 干净。ESLint 挡不住 09-01 错误的证据：把 #10729 的重复language = 'en'绑定注回ChatEditor.test.tsx后npx eslint退出码为 0。测试平台
Workflow YAML 及其守卫测试；套件在本地 macOS 运行，Linux 由本 PR 的 CI 覆盖。
运行环境（可选）
N/A —— 仅涉及 workflow 与测试改动。
风险与范围
lint_and_static的真实 p50/p90（按步骤耗时推算约 13–15 分钟；建议在 ruleset 变更前用真实运行数据核对）。Test (ubuntu-latest, Node 22.x)与输出不变；新增一个 check 名称Lint & Static (ubuntu-latest, Node 22.x)。关联 Issue
参考 #10721、#10736、#10740、#10744（09-01 main 变红事件，required-check 路线的动因）。