perf(autofix): stop restoring a 2.65 GB npm cache to protect a 29 s install - #8681
Conversation
…nstall `actions/setup-node` is configured with `cache: 'npm'` in all three heavy autofix jobs. Measured on one review-address leg, that step took 339s: Node itself was free (`Found in cache @ .../_tool/node/22.23.2/x64`) and 2,654,052,865 bytes arrived at ~10 MB/s. The `npm ci` it protects ran in 29s in the very next step. The bill is per job, not per run: build-cli pays it once (280s measured), issue-autofix once, and EVERY review-address leg once — up to ten legs a scan, five at a time. Observed leg costs ranged from 446s to 1207s. The persistent pool keeps ~/.npm across jobs, so the download buys nothing there. The hosted fallback is ephemeral and still wants it, so the cache is chosen from `runner.environment` rather than from a copy of the runs-on expression, which differs per job and would drift. Also narrows an existing negative pin. `expect(workflow).not.toContain( "runner.environment == 'self-hosted'")` was added to keep the reverted dedicated-runner design out, whose artefact was a `command -v node` step gated on exactly that expression (removed in #6261). That step is pinned out by name on the following line, so the substring form only forbade the `runner` context by accident — the same test requires `RUNNER_ENVIRONMENT: '${{ runner.environment }}'` a few lines below. It now matches the shape that was actually reverted: a step whose entire `if:` is that expression. Mutation-checked: re-adding that step still fails the test.
|
Thanks for the PR! Template looks good ✓
Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓
进入代码审查 🔍 — 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. |
Code reviewThe round-0 blocker is fixed, and the fix is the interesting part. Recap: the original The ternary is now correct.
That matches the shape of every other ternary in this repo ( The gap that hid round 0 is closed structurally. The new The maintainer's line-by-line findings are all addressed — I checked each against the current diff, not the round summary:
Premise observability: the two jobs with a Non-blocking, for the record: the negative-pin regex covers Testing evidenceUnattended CI run — no local execution here; the evidence is the PR's own CI on the reviewed commit, read through the API.
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 On the central claim — every piece that is verifiable pre-merge is verified: which input each pool gets is settled by the suite itself (the semantic assertion is the round-0 failure made failable); empty 中文说明round-0 阻断项已修复，且修复方式正是关键所在。原式 三元式现在正确：真值字面量放在 round-0 之所以能隐藏的缺口，这次在结构上被封死：新测试抽取被钉住的表达式，用一个实现 GHA 操作数值语义（ maintainer 的逐行意见均已落实（逐条对照当前 diff 核实，而非只看轮次总结）：步骤切片复用模块级
前提可观测性：有 非阻断备注：负向断言正则覆盖 测试证据：无人值守 CI 运行，不做本地执行——证据为通过 API 读取的本 commit 自身 CI。运行 关于核心主张——合入前可验证的部分均已验证：哪个池拿到哪个输入由测试套件钉死（语义断言把 round-0 的失败做成了可失败项）；空 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the round-0 blocker is fixed the right way, and the fix is pinned by the test that would have caught it; two marginal notes, neither blocking. Stepping back: my independent proposal for this problem — key the cache on The two notes, both non-blocking: the negative pin's regex covers If I have to maintain this in six months, the operand-order comment at the first copy with lockstep pointers at the other two is exactly what I want to find. Approving, pinned to the reviewed commit. 中文说明置信度：4/5 —— round-0 阻断项以正确的方式修复，且修复由"本可以捕获它"的测试钉住；两条边缘备注，均不阻断。 退一步看：我对这个问题的独立方案——按 两条非阻断备注：负向断言正则覆盖 如果六个月后由我维护，第一份拷贝处的操作数顺序注释加其余两处的联动指针，正是我希望看到的样子。批准，钉住所评审的 commit。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
The cache expression evaluates to 'npm' on both pools — GHA's &&/|| return operands and the empty string is falsy, so runner.environment == 'self-hosted' && '' || 'npm' never disables the restore. As written this is a runtime no-op; the full runner-source trace and the fix shape are in my notes above. 🙏
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| # The hosted fallback is ephemeral and still wants the cache, so the | ||
| # choice reads the runner rather than restating the runs-on | ||
| # expression, which differs per job and would drift. | ||
| cache: "${{ runner.environment == 'self-hosted' && '' || 'npm' }}" |
There was a problem hiding this comment.
[Critical] The conditional never disables the cache: GitHub Actions &&/|| return operand values (not booleans), && binds tighter than ||, and '' is falsy — so runner.environment == 'self-hosted' && '' || 'npm' evaluates to 'npm' on every runner. Self-hosted: true && '' → '', then '' || 'npm' → 'npm'; hosted: false && '' → false → 'npm'. setup-node therefore receives cache: 'npm' on both pools — byte-identical to the line this replaces, so the change is a runtime no-op (verified by evaluating the exact expression with GitHub's own expression-evaluator implementation: npm for self-hosted and github-hosted alike). — Failure scenario: every build-cli / issue-autofix / review-address leg on the ECS pool → still restores the ~2.65 GB npm cache (~339 s measured per leg, up to ten legs per scan) — exactly the cost this PR exists to remove — and it ships green, because the new tests pin the literal string of this expression.
| cache: "${{ runner.environment == 'self-hosted' && '' || 'npm' }}" | |
| cache: "${{ runner.environment != 'self-hosted' && 'npm' || '' }}" |
The inversion puts the falsy literal last, where the idiom is safe: self-hosted → false && 'npm' → false → false || '' → '' (setup-node skips caching on an empty cache — verified at the pinned v6.4.0 SHA; this repo's package.json has no packageManager field, so the auto-detect fallback cannot re-enable it), hosted → 'npm'. The test pins need the same correction.
中文说明
这个条件表达式永远不会禁用缓存：GitHub Actions 的 &&/|| 返回操作数的值（而非布尔值），&& 优先级高于 ||，且 '' 为假值 —— 所以 runner.environment == 'self-hosted' && '' || 'npm' 在所有 runner 上都求值为 'npm'。self-hosted：true && '' → ''，然后 '' || 'npm' → 'npm'；托管 runner：false && '' → false → 'npm'。setup-node 在两个池上收到的都是 cache: 'npm' —— 与被替换的那一行逐字节等价，因此本改动在运行时是空操作（已用 GitHub 官方表达式求值器实现实测该表达式：self-hosted 与 github-hosted 结果均为 npm）。 —— 失败场景：ECS 池上的每一个 build-cli / issue-autofix / review-address leg → 仍会恢复约 2.65 GB 的 npm 缓存（单 leg 实测约 339 秒，每次扫描最多十个 leg）—— 正是本 PR 要消除的开销 —— 而且会全绿合入，因为新增测试钉死的是这个表达式的字面字符串。
建议的修复反转条件，把假值字面量放到该惯用法下安全的位置（最后）：self-hosted → false && 'npm' → false → false || '' → ''（setup-node 对空 cache 输入跳过缓存 —— 已在钉住的 v6.4.0 SHA 上验证；本仓库 package.json 没有 packageManager 字段，自动检测回退不会重新启用缓存），托管 runner → 'npm'。测试断言需同步修正。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| # The hosted fallback is ephemeral and still wants the cache, so the | ||
| # choice reads the runner rather than restating the runs-on | ||
| # expression, which differs per job and would drift. | ||
| cache: "${{ runner.environment == 'self-hosted' && '' || 'npm' }}" |
There was a problem hiding this comment.
[Critical] Same defect as the thread on line 911 (identical copy in issue-autofix): runner.environment == 'self-hosted' && '' || 'npm' evaluates to 'npm' on every runner, because GHA &&/|| return operand values, && binds tighter than ||, and '' is falsy — self-hosted: true && '' → '' → '' || 'npm' → 'npm'; hosted: false && '' → false → 'npm'. The cache restore this PR removes therefore still runs here too. — Failure scenario: every issue-autofix run on the ECS pool → still restores the ~2.65 GB npm cache (~339 s measured per leg) — the change is a runtime no-op, certified by string-matching tests.
| cache: "${{ runner.environment == 'self-hosted' && '' || 'npm' }}" | |
| cache: "${{ runner.environment != 'self-hosted' && 'npm' || '' }}" |
The inversion puts the falsy literal last, where the idiom is safe: self-hosted → '' (setup-node skips caching on an empty cache — verified at the pinned v6.4.0 SHA), hosted → 'npm'.
中文说明
与第 911 行的问题相同（issue-autofix 中的同一份拷贝）：runner.environment == 'self-hosted' && '' || 'npm' 在所有 runner 上都求值为 'npm'，因为 GHA 的 &&/|| 返回操作数的值、&& 优先级高于 ||、且 '' 为假值 —— self-hosted：true && '' → '' → '' || 'npm' → 'npm'；托管：false && '' → false → 'npm'。本 PR 要移除的缓存恢复因此在这里依然发生。 —— 失败场景：ECS 池上的每一次 issue-autofix 运行 → 仍会恢复约 2.65 GB 的 npm 缓存（单 leg 实测约 339 秒）—— 改动是运行时空操作，且被字符串匹配的测试背书合入。
修复反转条件，把假值字面量放到最后：self-hosted → ''（setup-node 对空 cache 跳过缓存 —— 已在钉住的 v6.4.0 SHA 上验证），托管 → 'npm'。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| # The hosted fallback is ephemeral and still wants the cache, so the | ||
| # choice reads the runner rather than restating the runs-on | ||
| # expression, which differs per job and would drift. | ||
| cache: "${{ runner.environment == 'self-hosted' && '' || 'npm' }}" |
There was a problem hiding this comment.
[Critical] Same defect as the thread on line 911 (identical copy in review-address): runner.environment == 'self-hosted' && '' || 'npm' evaluates to 'npm' on every runner, because GHA &&/|| return operand values, && binds tighter than ||, and '' is falsy — self-hosted: true && '' → '' → '' || 'npm' → 'npm'; hosted: false && '' → false → 'npm'. The cache restore this PR removes therefore still runs on every review-address leg — up to ten per scan. — Failure scenario: every review-address leg on the ECS pool → still restores the ~2.65 GB npm cache (~339 s measured per leg, up to ten legs per scan) — the change is a runtime no-op, certified by string-matching tests.
| cache: "${{ runner.environment == 'self-hosted' && '' || 'npm' }}" | |
| cache: "${{ runner.environment != 'self-hosted' && 'npm' || '' }}" |
The inversion puts the falsy literal last, where the idiom is safe: self-hosted → '' (setup-node skips caching on an empty cache — verified at the pinned v6.4.0 SHA), hosted → 'npm'.
中文说明
与第 911 行的问题相同（review-address 中的同一份拷贝）：runner.environment == 'self-hosted' && '' || 'npm' 在所有 runner 上都求值为 'npm'，因为 GHA 的 &&/|| 返回操作数的值、&& 优先级高于 ||、且 '' 为假值 —— self-hosted：true && '' → '' → '' || 'npm' → 'npm'；托管：false && '' → false → 'npm'。本 PR 要移除的缓存恢复因此在每一个 review-address leg 上依然发生 —— 每次扫描最多十个 leg。 —— 失败场景：ECS 池上的每一个 review-address leg → 仍会恢复约 2.65 GB 的 npm 缓存（单 leg 实测约 339 秒，每次扫描最多十个 leg）—— 改动是运行时空操作，且被字符串匹配的测试背书合入。
修复反转条件，把假值字面量放到最后：self-hosted → ''（setup-node 对空 cache 跳过缓存 —— 已在钉住的 v6.4.0 SHA 上验证），托管 → 'npm'。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| with: | ||
| node-version: '22.x' | ||
| cache: 'npm' | ||
| # No remote npm cache on the persistent pool. Measured on one |
There was a problem hiding this comment.
[Suggestion] Once the expression below is fixed to actually yield '' on the persistent pool, setup-node v6's package-manager-cache input (default true) can silently re-enable the remote cache: when cache is falsy, setup-node falls into its auto-detect branch and reads package.json's packageManager / devEngines.packageManager field — if it names npm, it restores and saves the remote cache anyway. Today this repo declares neither field (verified), so the landmine is dormant; but any future commit adding e.g. "packageManager": "npm@10.x" (common when adopting corepack) resurrects the exact 2.65 GB restore this PR removes, on every persistent-pool leg, with the string-matching tests still green. Applies to all three steps (also at 3132 and 3492). — Concrete cost: a future package-manager pin silently re-adds ~339 s/leg, up to ten legs per scan.
package-manager-cache: falseAdd this input to the three steps — harmless on the hosted fallback, where the explicit cache: 'npm' branch takes precedence over auto-detection.
中文说明
一旦下方表达式修正、在持久化池上真正产生 ''，setup-node v6 的 package-manager-cache 输入（默认 true）可能悄悄重新启用远程缓存：当 cache 为假值时，setup-node 会进入自动检测分支，读取 package.json 的 packageManager / devEngines.packageManager 字段 —— 若其指明 npm，就会照样恢复并保存远程缓存。目前本仓库两个字段都未声明（已验证），该雷暂未触发；但未来任何添加如 "packageManager": "npm@10.x" 的提交（采用 corepack 时很常见）都会在持久化池的每个 leg 上复活本 PR 刚移除的 2.65 GB 恢复，而字符串匹配的测试依然全绿。三个步骤同理（3132、3492 处）。 —— 具体代价：未来一次 package-manager 钉版就会悄悄加回每 leg 约 339 秒、每次扫描最多十个 leg 的开销。
在三个步骤中添加该输入即可 —— 对托管兜底无影响，那里显式的 cache: 'npm' 分支优先于自动检测。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // up to ten per scan, plus build-cli and issue-autofix. | ||
| const setupSteps = [ | ||
| ...workflow.matchAll( | ||
| /- name: 'Set up Node\.js'\n[\s\S]*?cache-dependency-path: '[^']*'\n/g, |
There was a problem hiding this comment.
[Suggestion] This capture cannot see a fourth setup-node step that omits (or double-quotes) cache-dependency-path — an optional input — so the regression the comment below claims to block can ship green. Probe-confirmed: with a synthetic fourth job hardcoding cache: 'npm' and omitting or double-quoting cache-dependency-path, matchAll still finds only the original three steps, and toHaveLength(3) plus every expression check passes; only the single-quoted variant is caught. — Failure scenario: a fourth dual-pool job is added with a hardcoded cache but no (or a double-quoted) cache-dependency-path → all assertions pass → the silent 2.65 GB-per-run cost ships despite this guard.
Count step names independently of their inputs, or reuse the lookahead-bounded nodeSetupSteps helper (line 176):
expect(workflow.match(/- name: 'Set up Node\.js'\n/g)).toHaveLength(3);中文说明
这个捕获看不到省略（或用双引号书写）cache-dependency-path 的第四个 setup-node 步骤 —— 该输入是可选的 —— 因此下方注释声称要拦截的回归可以全绿合入。已用探针确认：构造一个硬编码 cache: 'npm' 但省略（或双引号书写）cache-dependency-path 的第四个 job 后，matchAll 仍只找到原来的三个步骤，toHaveLength(3) 与全部表达式断言照常通过；只有单引号变体会被捕获。 —— 失败场景：新增第四个双池 job、硬编码缓存但不带（或双引号带）cache-dependency-path → 所有断言通过 → 每 run 2.65 GB 的隐性开销绕过该守卫合入。
独立于输入统计步骤名，或复用以 lookahead 定界的 nodeSetupSteps 辅助（第 176 行）：
expect(workflow.match(/- name: 'Set up Node\.js'\n/g)).toHaveLength(3);— qwen3.8-max via Qwen Code /review (v0.21.7)
| // 2,654,052,865 bytes at ~10 MB/s were the npm cache restore — guarding | ||
| // an `npm ci` that took 29s in the very next step. Every leg pays it, | ||
| // up to ten per scan, plus build-cli and issue-autofix. | ||
| const setupSteps = [ |
There was a problem hiding this comment.
[Suggestion] This local regex re-derives the setup steps instead of reusing the module-level nodeSetupSteps helper (line 176), and the two captures now define the same set with different boundaries (helper: lookahead to the next step name; here: up to cache-dependency-path:). The expression literal and toHaveLength(3) are also pinned twice (here and in the sibling recipe test), and the final toContain("|| 'npm' }}") is strictly implied by the loop above it — verified it can never fail independently. — Concrete cost: any recipe change must update both tests in lockstep, and an edit moving cache-dependency-path: fails the two tests with divergent failure qualities pointing at different causes for one edit.
Iterate nodeSetupSteps and keep only the genuinely new not.toContain("cache: 'npm'") checks — or fold those two lines into 'keeps the Node setup recipe identical across the autofix jobs'.
中文说明
这个本地正则重新推导了 setup 步骤，而没有复用模块级 nodeSetupSteps 辅助（第 176 行）；两个捕获现在以不同边界定义同一个集合（辅助：前瞻到下一个步骤名；此处：到 cache-dependency-path: 为止）。表达式字面量与 toHaveLength(3) 也被钉了两处（此处与同级的 recipe 测试），且最后的 toContain("|| 'npm' }}") 被上方循环严格蕴含 —— 已验证它永远不会独立失败。 —— 具体代价：任何 recipe 变更都要同步改两个测试；移动 cache-dependency-path: 的一次编辑会让两个测试以指向不同原因的失败方式报错。
改为遍历 nodeSetupSteps、只保留真正新增的 not.toContain("cache: 'npm'") 检查 —— 或把这两行并入 'keeps the Node setup recipe identical across the autofix jobs'。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // expression is still identical across the three, which is what this | ||
| // test is for. | ||
| expect(step).toContain( | ||
| `cache: "\${{ runner.environment == 'self-hosted' && '' || 'npm' }}"`, |
There was a problem hiding this comment.
[Suggestion] This pin compares text only — the suite has no mechanism to detect a GHA expression whose operand-value &&/|| semantics defeat its intent, which is exactly the bug class the Critical in this review demonstrates (the suite ran fully green with the no-op expression). Probe-confirmed: a semantic assertion flips between the buggy and the fixed expression, while this literal pin PASSes against the no-op and FAILs against the corrected workflow. — Failure scenario: a future edit changes the expression and updates the two literal pins (all the tests require), reintroducing a falsy operand on the wrong side of &&/|| → both tests pass green → self-hosted legs silently restore the 2.65 GB cache again (~339 s/leg, up to ten legs per scan).
Add a semantic assertion alongside the text pins: evaluate the cache expression for both runner facts and assert '' when runner.environment == 'self-hosted' and 'npm' when 'github-hosted' — a small evaluator for ==/!=/&&/|| with GHA operand-value semantics, or @actions/expressions as a dev dependency.
中文说明
该断言只比较文本 —— 套件没有任何机制能检测出被操作数值 &&/|| 语义破坏意图的 GHA 表达式，而这正是本次评审中 Critical 所演示的缺陷类别（套件在空操作表达式下全绿通过）。探针确认：语义断言能在有缺陷与修正后的表达式之间翻转，而该字面量钉死在空操作版本上通过、在修正后的工作流上反而失败。 —— 失败场景：未来某次编辑改动表达式并同步更新两处字面量断言（测试的全部要求），把假值重新放回 &&/|| 的错误一侧 → 两个测试全绿 → self-hosted leg 再次悄悄恢复 2.65 GB 缓存（每 leg 约 339 秒，每次扫描最多十个 leg）。
在文本钉死之外补充语义断言：对两种 runner 事实求值 cache 表达式，断言 runner.environment == 'self-hosted' 时为 ''、'github-hosted' 时为 'npm' —— 可用一个实现 GHA 操作数值语义 ==/!=/&&/|| 的小求值器，或将 @actions/expressions 作为开发依赖。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| `cache: "\${{ runner.environment == 'self-hosted' && '' || 'npm' }}"`, | ||
| ); | ||
| // Keyed on the RUNNER, not on a copy of the job's runs-on expression: |
There was a problem hiding this comment.
[Suggestion] Same as the thread at lines 6282-6285 (the twin pin in 'does not restore the remote npm cache on the persistent pool'): this assertion compares text only, so it cannot detect a GHA expression whose operand-value &&/|| semantics defeat its intent — the bug class this PR itself ships (the suite ran fully green with the no-op expression). Probe-confirmed: a semantic assertion flips between the buggy and the fixed expression, while this literal pin PASSes against the no-op and FAILs against the corrected workflow. — Failure scenario: a future edit changes the expression and updates the two literal pins, reintroducing a falsy operand on the wrong side of &&/|| → both tests pass green → self-hosted legs silently restore the 2.65 GB cache again. The comment above this line claims the test blocks cache regressions; as written it blocks only literal drift.
Add a semantic assertion alongside the text pins: evaluate the cache expression for both runner facts and assert '' when runner.environment == 'self-hosted' and 'npm' when 'github-hosted' — a small evaluator for ==/!=/&&/|| with GHA operand-value semantics, or @actions/expressions as a dev dependency.
中文说明
与 6282-6285 行的问题相同（'does not restore the remote npm cache on the persistent pool' 中的孪生断言）：该断言只比较文本，无法检测被操作数值 &&/|| 语义破坏意图的 GHA 表达式 —— 正是本 PR 自身携带的缺陷类别（套件在空操作表达式下全绿通过）。探针确认：语义断言能在有缺陷与修正后的表达式之间翻转，而该字面量钉死在空操作版本上通过、在修正后的工作流上反而失败。 —— 失败场景：未来某次编辑改动表达式并更新两处字面量断言，把假值重新放回 &&/|| 的错误一侧 → 两个测试全绿 → self-hosted leg 再次悄悄恢复 2.65 GB 缓存。上方注释声称该测试拦截缓存回归；就现写法而言，它只拦截字面量漂移。
在文本钉死之外补充语义断言：对两种 runner 事实求值 cache 表达式，断言 runner.environment == 'self-hosted' 时为 ''、'github-hosted' 时为 'npm' —— 可用一个实现 GHA 操作数值语义 ==/!=/&&/|| 的小求值器，或将 @actions/expressions 作为开发依赖。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
Qwen Code review did not complete successfully. Run review failed. See workflow logs for details. A transient error is retried automatically; if you are seeing this, retry with |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
Code reviewReviewed at Blocker — the expression evaluates to
|
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8681All three Critical findings and all five Suggestions are resolved in code, together with the points raised in the maintainer's issue-level review. One commit: Critical — the cache ternary was a runtime no-op (R1-1/R1-2/R1-3, maintainer blocker)Confirmed: GitHub Actions cache: "${{ runner.environment != 'self-hosted' && 'npm' || '' }}"self-hosted → Suggestions — resolved
Maintainer issue-level review — additional points
VerificationCommands actually run this round (working tree → after commit):
中文说明Autofix 评审轮次 — PR #8681三个 Critical 与五个 Suggestion 全部在代码中解决，maintainer 在 issue 级评审中提出的各点也一并处理。单个提交： Critical — 缓存三元式是运行时空操作（R1-1/R1-2/R1-3，maintainer 阻断项）已确认：GitHub Actions 的 cache: "${{ runner.environment != 'self-hosted' && 'npm' || '' }}"self-hosted → Suggestion — 已解决
Maintainer issue 级评审 — 其余各点
验证本轮实际运行的命令（工作区 → 提交后）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/qwen-autofix.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(workflow).not.toMatch( | ||
| /if:\s*(?:\|-\s*)?\$\{\{[^}]*(?:runner\.environment\s*==\s*'self-hosted'|'self-hosted'\s*==\s*runner\.environment)/, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The rewritten revert-tripwire requires a ${{ }} wrapper immediately after if: (optionally a |- block scalar), so valid GHA spellings the old substring pin caught now evade it — probe-verified against the verbatim regex: the unwrapped if: runner.environment == 'self-hosted' (the common spelling — GitHub auto-evaluates bare if: conditions as expressions), the literal | and folded >- block scalars, and quoted scalars such as if: "${{ ... }}" all return false, whereas the old not.toContain pin returned true. The quoted evasion is aggravated: if: "${{ ... }}" is this repo's own house style (14 sites in ci.yml alone), so a re-implementation copying a sibling workflow dodges this tripwire by default — the comment above claims coverage "in every spelling". The weakening was not forced by this PR (the new cache expression uses !=; the old pin passes green on this workflow), and the rewrite did add reversed-operand coverage — so widen rather than revert. Residual coverage: the step-level not.toMatch(/^\s*if:/m) pin sees only the three captured 'Set up Node.js' steps, and the name pin only the exact historical step name. — Failure scenario: a future change re-adds the #6261-reverted design (a step gating a node check on runner.environment == 'self-hosted') in one of the slipping spellings under any other step name → every pin stays green and the reverted design ships, with ECS-routed runs skipping or duplicating Node setup.
| expect(workflow).not.toMatch( | |
| /if:\s*(?:\|-\s*)?\$\{\{[^}]*(?:runner\.environment\s*==\s*'self-hosted'|'self-hosted'\s*==\s*runner\.environment)/, | |
| ); | |
| expect(workflow).not.toMatch( | |
| /if:\s*(?:[|>]-?\s*)?["']?\s*(?:\$\{\{)?[^}\n]*(?:runner\.environment\s*==\s*'self-hosted'|'self-hosted'\s*==\s*runner\.environment)/, | |
| ); |
中文说明
重写后的回退绊线要求 if: 紧跟 ${{ }}（可选 |- 块标量），因此旧子串断言能捕获的合法 GHA 写法现在会绕过它 —— 已对正则原文实测：未包裹的 if: runner.environment == 'self-hosted'（常见写法 —— GitHub 会把裸 if: 条件自动当表达式求值）、字面 | 与折叠 >- 块标量、以及 if: "${{ ... }}" 这类带引号的标量全部返回 false，而旧的 not.toContain 断言返回 true。带引号的绕过更严重：if: "${{ ... }}" 正是本仓库自己的风格（仅 ci.yml 就有 14 处），照抄兄弟工作流的实现会默认绕过这条绊线 —— 上方注释声称覆盖“所有写法”。这次收窄并非本 PR 所迫（新的 cache 表达式用 !=；旧断言在当前工作流上是绿的），且重写补充了反转操作数的覆盖 —— 所以应放宽正则而不是回退。剩余覆盖：步骤级 not.toMatch(/^\s*if:/m) 只覆盖被捕获的三个 'Set up Node.js' 步骤，名称断言只覆盖那个历史步骤的确切名称。 —— 失败场景：未来某次改动以漏掉的写法、任意其他步骤名恢复 #6261 中被回退的设计（以 runner.environment == 'self-hosted' 为条件的 node 检查步骤）→ 所有断言保持绿色，被回退的设计合入，ECS 路由的运行会跳过或重复 Node 安装。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| } | ||
| pos += 2; | ||
| const right = parsePrimary(); | ||
| return op === '==' ? left === right : left !== right; |
There was a problem hiding this comment.
[Suggestion] This semantic oracle compares with JS ===/!==, but GitHub Actions ==/!= are documented as case-insensitive and loosely type-coercing ("GitHub ignores case when comparing strings"; loose equality with numeric coercion) — so the oracle disagrees with Actions for a documented class of inputs, while the test's comment claims it evaluates "the way Actions does". Probe-verified divergences: runner.environment != 'Self-Hosted' with fact self-hosted — the helper yields 'npm', Actions yields ''; github.event.pull_request.number == '8681' with numeric fact 8681 — helper false, Actions true. No impact today: the only evaluated expression compares case-matching string facts where both semantics agree. — Failure scenario: a future semantic pin reusing this helper with case- or type-divergent operands asserts the wrong expected values and passes green while the workflow's runtime behavior diverges — exactly the "evaluates wrong at runtime" class this helper exists to catch.
| return op === '==' ? left === right : left !== right; | |
| if (typeof left === 'string' && typeof right === 'string') { | |
| const l = left.toLowerCase(); | |
| const r = right.toLowerCase(); | |
| return op === '==' ? l === r : l !== r; | |
| } | |
| return op === '==' ? left === right : left !== right; |
中文说明
这个语义求值器用 JS 的 ===/!== 比较，但 GitHub Actions 的 ==/!= 文档明确为大小写不敏感、宽松类型比较（“比较字符串时忽略大小写”；带数值强制转换的宽松相等）—— 因此对文档定义的这一类输入，求值器与 Actions 结果不一致，而测试注释声称“按 Actions 的方式”求值。实测确认的分歧：事实为 self-hosted 时 runner.environment != 'Self-Hosted' —— 求值器得 'npm'，Actions 得 ''；数值事实 8681 时 github.event.pull_request.number == '8681' —— 求值器 false，Actions true。当前无影响：唯一被求值的表达式比较的是大小写一致的字符串事实，两种语义结果相同。 —— 失败场景：未来复用该求值器的语义断言若使用大小写或类型不同的操作数，会断言错误的期望值并全绿通过，而工作流运行时行为已经分叉 —— 正是这个求值器要拦截的“运行时求值错误”类别。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| # 'Set up Node.js' assumes this pool keeps ~/.npm across jobs; | ||
| # log its size so a move to per-job containers is visible. | ||
| du -sh "${HOME}/.npm" 2>/dev/null || echo '~/.npm is absent on this runner.' |
There was a problem hiding this comment.
[Suggestion] This du -sh canary — the sole observability guard for the persistent-~/.npm assumption the cache removal now bets on — is pinned by no contract test, while every other load-bearing line of its host step is pinned (the envCheckSteps loop asserts docker info and exit 1; zero matches for du -sh/.npm in the test file). Probe-verified: deleting both canary lines (here and the identical copy in review-address, ~line 3480) keeps the entire suite green; adding the pin below flips the probe (green on pristine, red with the canary removed). — Failure scenario: a future refactor of 'Check runner environment' deletes the canary green; if the pool later moves to per-job containers/ephemeral HOME, npm ci --prefer-offline silently pays the full download again — the exact ~339s-per-leg regression this PR removes — with no log evidence the visibility line ever existed. Fix lives in scripts/tests/qwen-autofix-workflow.test.js, inside the envCheckSteps loop (one assertion covers both copies):
expect(step).toContain('du -sh "${HOME}/.npm"');中文说明
这个 du -sh 金丝雀 —— 缓存移除后所依赖的“持久化池保留 ~/.npm”假设的唯一可观测性守卫 —— 没有任何契约测试钉住，而同一步骤里其他所有关键行都有断言（envCheckSteps 循环断言了 docker info 和 exit 1；测试文件中 du -sh/.npm 零匹配）。实测确认：删掉两处金丝雀（此处与 review-address 中约 3480 行的同一份拷贝）整个套件依然全绿；加上下面的断言后探针翻转（原样绿、删除后红）。 —— 失败场景：未来对 'Check runner environment' 的重构顺手删掉金丝雀且保持绿色；若池子日后改为按 job 容器 / 临时 HOME，npm ci --prefer-offline 将悄悄重新支付完整下载 —— 正是本 PR 移除的单 leg 约 339 秒的回归 —— 且日志里没有任何痕迹证明这条可见性行曾经存在。修复位于 scripts/tests/qwen-autofix-workflow.test.js 的 envCheckSteps 循环内（一条断言覆盖两处拷贝）：expect(step).toContain('du -sh "${HOME}/.npm"');
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // All three consumers, so a fourth job with a hardcoded cache fails | ||
| // here rather than quietly paying 2.65 GB per run — counted by step | ||
| // name, so no choice of inputs can dodge the capture. | ||
| expect(nodeSetupSteps).toHaveLength(3); |
There was a problem hiding this comment.
[Suggestion] This consumer-count pin keys on steps named exactly 'Set up Node.js' (the nodeSetupSteps capture), so a setup-node consumer under any other step name dodges every pin in this test — while the comment claims "a fourth job with a hardcoded cache fails here". Nothing else pins the total number of actions/setup-node consumers (the only other references sit inside the same name-keyed loop), and the "short jobs stay hosted" pins cover only currently-enumerated job names. Probe-verified: injecting a fourth consumer named 'Setup Node.js' with cache: 'npm' leaves toHaveLength(3) and every pin green; an action-reference count pin catches it (a same-named fourth step IS caught by the current pin — the gap is precisely the renamed case). — Failure scenario: a future job routes onto the same persistent-pool ternary and copies the pre-PR recipe under a different step name → every pin green, the new leg silently pays the ~2.65 GB remote-cache restore (~339 s per leg) on every run.
| // All three consumers, so a fourth job with a hardcoded cache fails | |
| // here rather than quietly paying 2.65 GB per run — counted by step | |
| // name, so no choice of inputs can dodge the capture. | |
| expect(nodeSetupSteps).toHaveLength(3); | |
| // All three consumers, so a fourth job with a hardcoded cache fails | |
| // here rather than quietly paying 2.65 GB per run — counted by step | |
| // name, so no choice of inputs can dodge the capture; the action-ref | |
| // count below also closes the renamed-step dodge. | |
| expect(nodeSetupSteps).toHaveLength(3); | |
| expect( | |
| workflow.match(/uses: 'actions\/setup-node@/g) ?? [], | |
| ).toHaveLength(3); |
中文说明
这个消费者计数断言以步骤名恰好是 'Set up Node.js' 为准（nodeSetupSteps 捕获），因此任何其他步骤名的 setup-node 消费者都会绕过本测试的所有断言 —— 而注释声称“第四个硬编码缓存的 job 会在这里失败”。没有其他地方钉住 actions/setup-node 消费者的总数（其余引用都在同一个按名捕获的循环里），“短 job 保持托管”的断言也只覆盖当前枚举的 job 名。实测确认：注入一个名为 'Setup Node.js'、带 cache: 'npm' 的第四个消费者，toHaveLength(3) 与所有断言照常通过；按 action 引用的计数断言能捕获（同名第四步会被现有断言捕获 —— 漏洞恰在改名情形）。 —— 失败场景：未来某个 job 路由到同一持久化池三元表达式、以不同步骤名复制本 PR 之前的配方 → 所有断言绿色，新 leg 每次运行悄悄支付约 2.65 GB 的远程缓存恢复（每 leg 约 339 秒）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const cacheExpression = | ||
| nodeSetupSteps[0].match(/cache: "\$\{\{ ([^}]+) \}\}"/)?.[1] ?? ''; |
There was a problem hiding this comment.
[Suggestion] Every cache pin this PR adds — this extraction plus the toContain pins in both tests (~6360-6363 and ~7127-7129) — is unanchored against YAML structure, so a commented-out copy of the ternary line satisfies them all while a different active cache: input ships. The semantic test — introduced precisely because text pins cannot tell a working ternary from a defeated one — reads its expression from the same unanchored match and is defeated identically. Probe-verified against this PR's own pin code: commenting out cache: "${{ ... }}" and package-manager-cache: false and setting active cache: 'npm' on all three steps (valid YAML, parsed) leaves the entire suite green (122/122); anchored /^\s*cache:/m pins and an anchored extraction flip the probe to red. — Failure scenario: a temporary cache re-enable while debugging (or a hand-revert) comments out the ternary and sets active cache: 'npm' → the suite stays green while every ECS-routed leg restores ~2.65 GB (~339s per leg, up to ten review-address legs plus build-cli and issue-autofix per scan). Fix: anchor this extraction to /^\s*cache: "\$\{\{ ([^}]+) \}\}"/m (a comment line starts with #, so ^\s*cache: cannot match one), and convert both toContain cache pins to toMatch(/^\s*cache: "\$\{\{ runner\.environment != 'self-hosted' && 'npm' \|\| '' \}\}"/m) plus toMatch(/^\s*package-manager-cache: false/m).
| const cacheExpression = | |
| nodeSetupSteps[0].match(/cache: "\$\{\{ ([^}]+) \}\}"/)?.[1] ?? ''; | |
| const cacheExpression = | |
| nodeSetupSteps[0].match(/^\s*cache: "\$\{\{ ([^}]+) \}\}"/m)?.[1] ?? ''; |
中文说明
本 PR 新增的所有 cache 断言 —— 此处提取加上两个测试里的 toContain 断言（约 6360-6363 与 7127-7129 行）—— 都没有锚定 YAML 结构，因此被注释掉的三元表达式拷贝可以全部满足，而另一个生效的 cache: 输入照常合入。这个语义测试明明是因为“文本断言分不清能用与被语义破坏的三元表达式”才引入的，却从同一个未锚定的匹配里读取表达式，被同样的方式击穿。已按本 PR 自己的断言代码实测：把三个步骤的 cache: "${{ ... }}" 与 package-manager-cache: false 注释掉、改为生效的 cache: 'npm'（合法 YAML，已解析验证），整个套件依然全绿（122/122）；改用锚定的 /^\s*cache:/m 断言与锚定提取后探针变红。 —— 失败场景：为调试临时恢复缓存（或手工回退）时注释掉三元表达式、启用 cache: 'npm' → 套件全绿，而每个 ECS 路由的 leg 都在恢复约 2.65 GB（每 leg 约 339 秒，每次扫描最多十个 review-address leg，外加 build-cli 与 issue-autofix）。修复：把此处提取锚定为 /^\s*cache: "\$\{\{ ([^}]+) \}\}"/m（注释行以 # 开头，^\s*cache: 不会匹配），并把两处 toContain cache 断言改为 toMatch(/^\s*cache: "\$\{\{ runner\.environment != 'self-hosted' && 'npm' \|\| '' \}\}"/m) 与 toMatch(/^\s*package-manager-cache: false/m)。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
Qwen Code review did not complete successfully. Run review failed. See workflow logs for details. A transient error is retried automatically; if you are seeing this, retry with |
|
@qwen-code /triage |
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM, no blockers. Well-measured fix (one review-address leg spent 339s restoring 2.65GB of npm cache to protect a 29s npm ci, on a pool that keeps ~/.npm across jobs anyway). The cache ternary is correct under GHA's operand-value &&/|| semantics — self-hosted resolves to '' (no restore), github-hosted to 'npm' — and the standout is the test: a minimal evalGhaExpression evaluator pins the expression semantically (self-hosted->'' / hosted->'npm') because a text pin can't catch a ternary that reads right but evaluates wrong (this PR's first attempt restored the cache on both pools). I verified the evaluator's &&/||/truthy semantics match Actions. package-manager-cache:false guards against a future packageManager field silently re-enabling the cache; setup-node stays unconditional (test forbids any if:); the du -sh ~/.npm log makes a future move to per-job containers visible. Pure CI perf, no secrets/privilege change. No P0/P1.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review feedback addressed (round 2, PR #8681)All five automated-reviewer suggestions were implemented in a single commit ( Feedback points and decisions
No findings were declined, deferred, or escalated; nothing required a maintainer decision. No conflict resolution was needed ( VerificationCommands actually run and their results:
中文说明Autofix 审查反馈已处理（第 2 轮，PR #8681）自动审查器的五条建议已全部在单个提交中实现（ 反馈点与决定
没有拒绝、推迟或升级任何反馈点；没有需要维护者决定的事项。无需解决冲突（ 验证实际运行的命令及结果：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
…ds (QwenLM#8683) * fix(review): stop the agent transcript from executing workflow commands The review agent streams its entire transcript to stdout, and the runner scans every line for workflow commands. A tool result that quotes a file containing one therefore gets EXECUTED. Observed on run 31167034020 (PR QwenLM#8681). That PR changes an `actions/setup-node` input, so the agent read the action's own main.ts, which legitimately contains: core.info(`##[add-matcher]${path.join(matchersPath, 'tsc.json')}`); The runner took the rest of the JSON line as a matcher path: ##[error]Unable to process command '{"type":"user","uuid":...' successfully. ##[error]The path '...' is too long, or a component of the specified path is too long. Three of those, and the step failed after 1h37m — a full review discarded for quoting a file. Nothing about that PR is unusual: any review whose transcript quotes `##[...]` or `::...::` breaks the same way, including every review of this repository's own workflows. Wrap the agent invocation in `::stop-commands::`, with a token randomised per attempt so no output the agent produces can guess it and re-enable parsing early. Parsing resumes immediately after PIPESTATUS is captured: `echo` clobbers PIPESTATUS, so resuming any earlier would read the echo's status instead of the agent's and report every timeout or crash as a clean run. Resuming is on the errexit-disabled straight line, so it is reached on success, crash and timeout alike — leaving it off would silence the job's own ::error:: and the fallback comment's diagnostics for the rest of the run. Tested by driving the real extracted retry loop with a stub agent that emits `##[add-matcher]`, asserting the bracket contains it, that the token is random rather than fixed, and that parsing resumes on success, hard exit and timeout. Mutation-checked: removing the guard, never resuming, resuming before the status capture, and using a fixed token each fail. * fix(review): resume workflow commands on a line the runner can see Round-2 review follow-ups on the stop-commands guard. The resume was `echo`d, so a `--kill-after` SIGKILL that cut the agent off mid-line appended it to that fragment. The runner matches `::cmd::` at a line start only, so parsing stayed off for the rest of the job — losing the retry `::warning::` and every later diagnostic on the one path the guard exists to survive. Emit it with a leading newline. The ordering assertions had no teeth: `indexOf` returns -1 when a line is deleted or reworded, and -1 satisfies `toBeLessThan`. Deleting the stop line left the suite green. Every anchor is now asserted present. Cover the outcomes no scenario reached: an agent that streams and then dies (the stub `timeout` exited before ever running it), a failing log write (the only early return left unpinned), and a retry, which pins the bracket as per-attempt with a token the previous attempt cannot reuse. Mutation-tested, 7 of 7 caught: reverting the printf, moving the resume past the tee check or before the PIPESTATUS capture, hoisting the bracket out of the function, fixing the token, and deleting either end. --------- Co-authored-by: verify <verify@local>
|
Released in v0.21.8. |
What this PR does
Stops
actions/setup-nodefrom restoring the remote npm cache on the persistent self-hosted pool, in all three heavy autofix jobs. The ephemeral hosted fallback keeps it.Why it's needed
The cache costs an order of magnitude more than the install it protects. Measured on one
review-addressleg (job92825809455):2.65 GB restored to save 29 seconds.
And it is billed per job, not per run:
build-clionce (280s measured),issue-autofixonce, and everyreview-addressleg once — up to ten legs per scan atmax-parallel: 5. Across a sample of scheduled runs the per-leg cost ranged from 446s to 1207s.Context: one
autofix/takeoverround on #8368 takes ~7 hours end to end, of which the autofix pipeline itself is ~2 hours. This step is a large, wholly avoidable slice of that, and it repeats every round.The persistent pool keeps
~/.npmacross jobs, so the download buys nothing there — which is whynpm ci --prefer-offlinealready finishes in 29s. The choice readsrunner.environmentrather than restating theruns-onexpression, which differs per job and would drift out of sync.Reviewer Test Plan
How to verify
Re-run the measurement on any recent scheduled run:
Then:
Expected: 122/122. After merge, the same step on an ECS-routed leg should drop from ~340s to the low seconds while
Install dependenciesstays in the tens of seconds.Evidence (Before & After)
N/A for UI. Before is the breakdown above; after can only be observed on a real ECS-routed run, so it is the post-merge check named above rather than something reproducible in this PR.
Mutation-tested — 4 of 4 caught: revert one job to
cache: 'npm'; blanket-disable the cache on both pools; invert the runner condition; and re-add the revertedUse pre-installed Node.js (self-hosted)step.Tested on
Risk & Scope
~/.npmon a given ECS runner is cold — a fresh host, or a pruned HOME — that job pays a coldnpm ciinstead of a 2.65 GB download. The 29s measurement is from a warm runner, so the exact cold cost is not measured here; it is bounded by the 225s thatInstall dependencies and buildtakes inbuild-cli, which is still below the 265s download. The hosted fallback is untouched.cache: 'npm'appears in ten others — but that is a separate change and this one does not depend on it.One thing worth a reviewer's eye: this narrows an existing negative pin.
expect(workflow).not.toContain("runner.environment == 'self-hosted'")was added to keep the reverted dedicated-runner design out, whose artefact was acommand -v nodestep gated on exactly that expression (removed in #6261). That step is pinned out by name on the following line, so the substring form forbade therunnercontext by accident — the same test requiresRUNNER_ENVIRONMENT: '${{ runner.environment }}'a few lines below. It now matches the shape that was actually reverted: a step whose entireif:is that expression. The mutation above confirms re-adding that step still fails.Linked Issues
Part of reducing
autofix/takeoverround latency, alongside #8676 (removes the 41-minute cron wait for fork PRs). Independent of it — this one helps every lane and every round.中文说明
What this PR does
在三个重型 autofix job 中，停止在持久化自托管池上恢复远程 npm 缓存；临时性的托管 runner 保留缓存。
Why it's needed
这个缓存的代价比它要加速的安装高一个数量级。实测某个
review-addressleg（job92825809455）：恢复 2.65 GB，只为省 29 秒。
而且这笔账是按 job 而非按 run 计的：
build-cli一次（实测 280s）、issue-autofix一次、每个review-addressleg 各一次——一次扫描最多 10 个 leg，max-parallel: 5。在若干定时 run 的样本中，单 leg 成本从 446s 到 1207s 不等。背景：#8368 的一轮
autofix/takeover端到端约 7 小时，其中 autofix 自身流水线约 2 小时。这一步是其中一大块完全可以避免的开销，而且每轮重复。持久化池的
~/.npm在各 job 间保留，所以那次下载什么也没买到——这正是npm ci --prefer-offline只要 29 秒的原因。缓存开关读取runner.environment，而不是复制runs-on表达式：后者各 job 不同，容易失同步。Reviewer Test Plan
How to verify
在任意一个近期的定时 run 上复跑测量：
然后：
预期 122/122。合入后，ECS 路由的 leg 上同一步骤应从约 340s 降到个位数秒级，而
Install dependencies仍在几十秒量级。Evidence (Before & After)
界面部分 N/A。Before 即上面的耗时分解；After 只能在真实的 ECS 路由 run 上观察，因此以上述合入后检查为准，而不是本 PR 内可复现的东西。
变异测试 —— 4 个全部被捕获：把其中一个 job 改回
cache: 'npm'；两个池都关闭缓存；反转 runner 条件；以及恢复已被回退的Use pre-installed Node.js (self-hosted)步骤。Tested on
Risk & Scope
~/.npm是冷的（新机器，或 HOME 被清理），该 job 将支付一次冷npm ci而不是 2.65 GB 下载。29 秒是热态测得的，冷态确切成本本 PR 未测；其上界是build-cli里Install dependencies and build的 225s，仍低于 265s 的下载。托管 runner 不受影响。cache: 'npm'），但那是独立改动，本 PR 不依赖它。有一处需要评审者留意：本 PR 收窄了一条既有的负向断言。
expect(workflow).not.toContain("runner.environment == 'self-hosted'")当初是为了防止已被回退的「专用 runner」设计复活，那个设计的产物是一个以该表达式为if:的command -v node步骤（在 #6261 中删除）。该步骤已在下一行按名称单独钉死，因此子串形式只是顺带禁掉了runner上下文——同一个测试在几行之后还要求工作流包含RUNNER_ENVIRONMENT: '${{ runner.environment }}'。现在它匹配真正被回退的形状：整个if:就是该表达式的步骤。上面的变异测试确认，恢复那个步骤仍会让测试失败。Linked Issues
属于降低
autofix/takeover轮次延时的一部分，与 #8676（消除 fork PR 的 41 分钟 cron 等待）并行。两者相互独立——本 PR 惠及所有车道与每一轮。