test(ci): make release classifier stub module-safe - #10402
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical. The Ubuntu CI job Direction: aligned — this is CI/test robustness with no product-surface impact. No CHANGELOG signal is expected for a test-only change. Size: not applicable — a single test file, +1/−1, no core paths touched. Approach: minimal and correct. Risk: no elevated risk signals — the changed file matches none of the revert-correlated paths. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:是已观测到的,不是理论性的。运行本发布说明分类测试的 Ubuntu CI 任务 方向:对齐——这是 CI/测试健壮性修复,不涉及产品面。仅测试的改动本就不需要 CHANGELOG 信号。 规模:不适用——单个测试文件,+1/−1,未触及核心路径。 方案:最小且正确。 风险:无升级风险信号——改动文件不匹配任何与回滚相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review — no blockers, no convention violations. Before reading the diff my baseline fix for "an extensionless Node stub that may be interpreted as ESM can't rely on One honest scope note, not a blocker: CI only exercises this stub in the runner's default module mode. The ESM-forced mode that actually triggers the bug ( Test evidence (this PR's own CI, via API — I do not run PR code): Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The Sandboxed verification would settle the one path CI doesn't cover: 中文说明代码审查——无阻塞问题,无规范违规。在读 diff 之前,我对"可能被解释为 ESM 的无扩展名 Node 模拟命令不能依赖 一个诚实的范围说明(非阻塞):CI 只在 runner 的默认模块模式下运行该模拟命令。真正触发 bug 的 ESM 强制模式( 测试证据(来自本 PR 自身 CI 的 API——我不运行 PR 代码):CI 状态见上方表格(finalize 任务会在检查完成后更新)。 沙箱验证可以覆盖 CI 未涉及的一条路径: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — a clean, minimal, correct fix for an observed CI flake; the only reasons not to approve this second are that the directly-relevant test is still running and the ESM-forced trigger path is covered by API semantics rather than a CI lane. Stepping back, this is exactly the fix I'd have proposed before seeing the diff. The problem is real, not theoretical — I confirmed a failing Two non-blocking nits, named so they aren't silently dropped:
So: the verdict is approve, but approval is deferred until CI lands green on 中文说明退一步看,这正是我在看 diff 之前就会提出的修复。问题是真实的而非理论性的——我确认了关联 PR #10115 上 两个非阻塞的小点,点名以免被悄悄丢掉:
因此:结论是批准,但推迟到 CI 在 — 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. |
chiga0
left a comment
There was a problem hiding this comment.
单行修复,消除了 CI .github/scripts helper tests 步骤的间歇性失败(已在 #9984、#10115 等多个 PR 上复现)。
改动逻辑:测试在临时目录生成一个 gh 可执行存根,存根脚本内部用 require('node:fs') 写文件。当 Node 将无扩展名脚本解释为 ESM 时,require 不可用,存根静默失败,测试输出为空、CI 报红。改为 process.getBuiltinModule('node:fs') 后,CJS 和 ESM 两种模式均可运行。
验证
- 整个文件只有一处
require(— 存根中的唯一 CJS 调用,修复完整。 Test (ubuntu-latest, Node 22.x)本次 CI:✅(之前各 PR 均报红)Post Coverage Comment:✅(该步骤依赖 Test 步骤,随之变绿)Desktop Shellubuntu/windows:✅
一个注意点(非 blocking):process.getBuiltinModule() 在 Node 22.3.0 引入,项目 engines 写的是 >=22.0.0,理论上存在 22.0–22.2 的覆盖空白。实际上 CI 用 22.x 解析为当前最新(远高于 22.3),自托管 runner 的 "pre-installed Node" 也已是 22.14;June 2024 发布的 22.3.0 距今已 14 个月以上,现实中遇到 22.0–22.2 的运行环境概率极低。若要彻底消除这一疑虑,可在 Node 22 最低版本上补一次本地验证,但不需要等它再做其他改动。
没有 blocking 发现。
Reviewed with AI assistance.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Local verification report — real environment, A/B against the merge commitThis PR was already merged ( Verdict: the change is correct and does exactly what it claims. It fixes one real failure mode, fixes nothing else, and breaks nothing else. One portability edge is noted at the bottom. SetupTwo detached worktrees, real
Runner: macOS 15 (Darwin 25.6.0), arm64. Node 22.23.2 for the primary runs — matching CI's Results
Scenarios 2 and 3 are the discriminating ones, and both flip in the right direction. Root cause — confirmed, and simpler than the description suggestsNo Node flag is needed to hit this. The repo root That is the flag-free reproduction, and it is the one I'd keep in the test plan. A note on the recipe in the PR body: Blast radiusRunning the complete helper gate on both legs, with Completeness of the fix, checked across the whole surface:
So there is no second instance of this bug left behind. One non-blocking follow-up: the
|
| commit | |
|---|---|
| BASE | 2742d10f31(squash 合入的父提交) |
| HEAD | d6533785bd(本 PR 合入 main 后的状态) |
运行机:macOS 15(Darwin 25.6.0),arm64。主验证用 Node 22.23.2,与 CI 的 node-version: '22.x' 对齐;另用 22.2.0 / 22.3.0 / 24.18.1 做边界探测。被测命令就是 ci.yml 实际使用的那条:
node --test --test-concurrency=1 ${{ env.HELPER_TESTS }}
结果
| # | 场景 | BASE 2742d10f31 |
HEAD d6533785bd |
|---|---|---|---|
| 1 | 普通运行,默认 TMPDIR,Node 22.23.2 |
3/3 通过,exit 0 | 3/3 通过,exit 0 |
| 2 | TMPDIR 位于 checkout 内部,不带任何 Node 标志(自托管 runner 的形态) |
2/3,exit 1 | 3/3,exit 0 |
| 3 | NODE_OPTIONS=--experimental-default-type=module |
2/3,exit 1 | 3/3,exit 0 |
| 4 | 完整 helper 门 —— 20 个文件,443 个用例 | 43 失败 | 42 失败(差值恰为本用例) |
| 5 | Node 22.2.0(声明的 engines 下限),普通运行 |
3/3 通过,exit 0 | 2/3,exit 1 |
场景 2 和 3 是判别性场景,两者都朝正确方向翻转。
根因 —— 已确认,且比描述里写的更简单
触发它根本不需要任何 Node 标志。 仓库根 package.json 带 "type": "module"。生成的 gh stub 是无扩展名的,因此 Node 会拿最近的祖先 package.json 来判定它的模块类型。只要 runner 的 TMPDIR / RUNNER_TEMP 落在 checkout 内部(这正是我们自托管 runner 的常态,也和那里能看到的 .qwen/tmp/... 残留一致),mkdtempSync(join(tmpdir(), …)) 就会把 stub 建在仓库树下,Node 随即按 ES module 加载它。此时 require 未定义,stub 以 1 退出,分类器不输出汇总,断言便在空 stdout 上失败。
这就是无标志的复现路径,我建议测试计划里保留这一条。关于 PR 描述中的配方补充一点:NODE_OPTIONS=--experimental-default-type=module 确实能复现,但仅限 Node 22 —— Node 24 直接拒绝该标志(--experimental-default-type= is not allowed in NODE_OPTIONS,作为直接参数则是 bad option)。鉴于 .nvmrc 是 22、CI 跑 22.x,眼下无害;但一旦升级到 Node 24,那条配方会悄无声息地失去复现能力,而 TMPDIR 形态的复现不会。
影响面
在两条腿上跑完整 helper 门(TMPDIR 置于 checkout 内):恰好只有一个子用例改变状态,其余全部不动。
修复完整性,按整个面清查:
- 任何
.github/scripts/**/*.test.mjs中都不再残留require(。 classify-release-notes.test.mjs:232是唯一一处写 Node shebang stub 的 helper 测试。其余同类(assign-issue-owner、upsert-bot-comment、resolve-sandbox-image等)写的都是#!/bin/sh或#!/bin/bashstub,从构造上就对这类问题免疫。
所以没有遗漏的第二处同类缺陷。
一条不阻塞的后续建议:engines 下限
process.getBuiltinModule() 是 Node v22.3.0 才加入的(本地实测:22.2.0 没有,22.3.0 有)。但 package.json 声明的是 "engines": { "node": ">=22.0.0" },.nvmrc 也只锁了宽松的 22。在 Node 22.0.x–22.2.x 上,这次改动会把一个原本通过的用例变成失败:
TypeError: process.getBuiltinModule is not a function
CI 不受影响 —— setup-node 配 22.x 会解析到最新的 22 —— 所以只会咬到本地还停在旧 22.x 的贡献者。不过这次修复毕竟是拿一个窄环境的失败换了另一个,下面任一条单行改动都能彻底了结:
- 首选 —— 沿用仓库自己的先例,把 stub 写成 POSIX shell 脚本,与其它 helper 测试的 stub 一致。这样能同时绕开 Node 模块语义和版本下限两件事:
'#!/bin/sh', `printf '%s %s\\n' "$number" "$action" >> ${JSON.stringify(updates)}`,
- 仍用 Node,但改成
import('node:fs').then(fs => …)—— 在所有 Node 22.x 上,CJS 与 ESM 两种模式都成立。 - 或者干脆把
engines.node提到>=22.3.0,如果我们认可把它作为真实下限。
范围与说明
- 完整门里两条腿都存在的那 42 个失败属于既有问题且仅限 macOS:
qwen-triage-workflow.test.mjs里两个 flakiness-gate 套件会调用 GNUtimeout,而 macOS 不自带。两腿完全一致,与本 PR 无关。 - 仅在 macOS 上验证,未覆盖 Linux 与 Windows。
- 记录一个后续复跑此测试时值得注意的环境陷阱:由于 stub 的 shebang 是
#!/usr/bin/env node,stub 会从PATH解析node,而外层 runner 可能是另一个 Node。版本混用的 shell 会让两条腿都因无关原因失败。本次两条腿均在PATH上使用了单一统一的 Node。
Pick up the classify-release-notes helper-test fix (#10402) so CI runs green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Pick up the classify-release-notes helper-test fix (#10402) so CI runs green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Pick up the classify-release-notes helper-test fix (#10402) so CI runs green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Resolve ArtifactPanel.tsx import conflict by keeping both the PR's SessionWorkflowInspector import and main's TerminalPanel import; both components are referenced in the merged file. Also picks up the classify-release-notes helper-test fix (#10402). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Pick up the classify-release-notes helper-test fix (#10402) so CI runs green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Bring in #10402 (test(ci): make release classifier stub module-safe), which fixes the classify-release-notes helper-test failure this branch's last CI run hit; no conflicts. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Released in v0.22.3. |




What this PR does
Makes the release-note classifier test stub work whether Node interprets the extensionless executable as CommonJS or ESM.
Why it's needed
The Ubuntu CI job intermittently failed on some self-hosted runners because the generated
ghstub used CommonJS-onlyrequire. When the runner interpreted that extensionless script as ESM, label updates failed and the test produced no summary output.Reviewer Test Plan
How to verify
Run the release-note classifier test normally and again with Node forced to treat extensionless scripts as ESM. Both runs should pass all three test cases, including continuing label updates after one lookup failure.
Evidence (Before & After)
N/A — test-only compatibility fix.
Tested on
Environment (optional)
Node.js 22.14.0; verified in normal mode and with
NODE_OPTIONS=--experimental-default-type=module.Risk & Scope
Linked Issues
Observed in the CI failures on #9984 and #10115.
中文说明
本 PR 的改动
让发布说明分类测试中的模拟命令在 Node 将无扩展名可执行文件解释为 CommonJS 或 ESM 时都能正常运行。
为什么需要
部分自托管 runner 上的 Ubuntu CI 会间歇失败,因为生成的
gh模拟命令使用了仅 CommonJS 可用的require。当 runner 将该无扩展名脚本解释为 ESM 时,标签更新失败,测试无法输出汇总结果。Reviewer Test Plan
如何验证
正常运行发布说明分类测试,然后强制 Node 将无扩展名脚本解释为 ESM 再运行一次。两次都应通过全部三个用例,包括单次查询失败后继续更新其他标签的场景。
证据(修改前后)
不适用——仅测试兼容性修复。
测试平台
环境(可选)
Node.js 22.14.0;已在普通模式和
NODE_OPTIONS=--experimental-default-type=module模式下验证。风险与范围
关联问题
该问题出现在 #9984 和 #10115 的 CI 失败中。