fix(ci): restore Windows test baseline - #11787
Conversation
|
Thanks for taking on the Windows lane — a scheduled baseline that has been red for days is exactly the kind of thing that quietly costs everyone regression signal. Template ✓ — every required section is there, bilingual, and the Risk & Scope block actually names the tradeoffs instead of waving at them. Problem — observed, not theoretical, and I checked rather than taking the description's word for it. Run 34713863264 really is Direction — aligned. Restoring CI signal is maintenance of the thing that protects everything else, and none of this reaches auth, model selection, telemetry, release, or a public contract. Two caveats I'd want on the record, because the description undersells both:
Size — core paths are touched ( Approach — the portability work itself is mostly disciplined and I want to call out what's good: Three things I'd push back on, none of them large:
Risk — no elevated risk signals: the Stage 1e high-risk path scan matched nothing in the production file set (the One structural note worth flagging, since it shapes how much of this anyone can actually verify: Moving on to code review. 🔍 中文说明感谢接手 Windows 流水线——定时基线连续红了好几天,正是那种会悄悄让所有人失去回归信号的问题。 模板 ✓ ——必填小节齐全、双语,Risk & Scope 也真实写出了取舍,而不是含糊带过。 问题——是已观测到的,不是理论性的,而且我核实过,没有只采信描述。Run 34713863264 确实是 方向——对齐。恢复 CI 信号是在维护那个保护其他一切的东西,而且这些改动都没有触及 auth、模型选择、telemetry、发布或对外契约。有两点希望明确记录,因为描述对两者的说法都偏轻:
规模——确实触及核心路径( 方案——可移植性工作本身大体是有章法的,好的部分我想点出来:用 有三点我要提出异议,都不算大:
风险——无升级风险信号:Stage 1e 高危路径扫描在生产文件集合中没有命中( 有一点结构性说明值得提出,因为它决定了这些改动实际能被验证到什么程度:常规 PR CI 上 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI read the title and the "Why it's needed" section first and wrote down what I'd do before opening the diff: fix the two real product bugs ( 🔴 Blocker — the
|
| File | What changed |
|---|---|
.github/workflows/ci.yml |
Adds git config --global core.autocrlf false to the Windows test lane, inside the job's throwaway CI home |
.github/workflows/windows-runner-smoke.yml |
Same autocrlf line for the self-hosted smoke lane |
package-lock.json |
Locks the WASI tokenizer plus @napi-rs/wasm-runtime, @tybys/wasm-util, and two alpha @emnapi packages |
pnpm-lock.yaml |
pnpm side of the same dependency addition; kept in step with npm as AGENTS.md requires |
pnpm-workspace.yaml |
Pins @tybys/wasm-util in overrides and excludes it from the release-age policy — this is the blocker |
packages/qwen-live/package.json |
Adds @node-rs/jieba-wasm32-wasi@2.0.2 to production dependencies |
packages/core/src/code-mode/host-client.ts |
Wraps the tsx --import specifier in pathToFileURL — real product fix for Windows drive paths |
packages/core/src/utils/no-follow-open.ts |
Moves to BigIntStats and coerces both sides with BigInt() so mixed shapes still compare |
packages/core/src/services/worktreeSessionService.ts |
Sidecar identity checks switched wholesale to bigint: nlink !== 1n, ino === 0n, size !== BigInt(bytesRead) |
packages/core/src/tools/artifact/artifact-snapshots.ts |
readdir hoisted out of the short-circuit — unconditional now, no stated Windows link |
packages/cli/src/services/web-shell-brand.ts |
Logo guard moved to BigIntStats (nlink > 1n, size > BigInt(MAX)), but the fd re-check keeps bare !== |
packages/cli/src/serve/fs/workspace-file-system.ts |
Three lstat calls gain { bigint: true }; safe because assertSameFile already coerces via toBigInt |
packages/qwen-live/src/proactive/monitor-debug-store.ts |
Production guard: mode-bits and uid checks skipped on Windows |
| …and 26 test files | Platform guards, safe-integer inode gates, native path fixtures, and bigint-aware stat mocks |
Testing evidence
This is an unattended CI run, so per the triage rules I did not build, run, or execute anything from this PR — no npm, no vitest, no checkout. Everything below is read from the PR's own CI through the API at the reviewed commit. I also independently confirmed the two workflow runs the description cites rather than trusting the links: the "before" run 34713863264 is genuinely Qwen Code CI, event=schedule, head_branch=main, conclusion=failure; the "after" run 34772671680 is Windows Self-Hosted Runner Validation, dispatched on this branch at exactly the reviewed SHA — and it is still in_progress, so the headline claim is not yet substantiated by anything.
85 check-runs on the commit: 22 success, 58 skipped, 5 in progress, 0 failures. Nothing is red, but the absence of red is weaker evidence than it looks, because the two lanes that would exercise this PR's actual subject are skipped and the repo-level script tests don't run on Ubuntu:
Test (windows-latest, Node 22.x)— skipped. This is the lane the PR exists to fix.Test (macos-latest, Node 22.x)— skipped. Also runsnpm run test:ci, so it also coverstest:scripts.Test (ubuntu-latest, Node 22.x)— in progress, and runstest:ci:workspaces, which excludes thescripts/testssuite that the blocker above lives in. A green result here will not clear it.Validate runner— in progress. The dispatched Windows self-hosted validation, and the only lane in flight that runstest:ci.Lint & Static (ubuntu-latest, Node 22.x)— success, so formatting and typechecking are clean, including theAwaited<ReturnType<typeof fsp.lstat>>annotation onleafnow that the call returnsBigIntStats.Install (windows-latest),Install (macos-latest),Install (ubuntu-latest)— success. The new dependency resolves and installs on all three platforms, which answers the lockfile-consistency question but says nothing about whether the tokenizer loads.Desktop Shell (windows-2022),windows-latest / Java 21— success.Integration Tests (no-AK, No Sandbox),Real daemon E2E / Java 11,Serve A/B,OpenTUI no-flicker gate,TUI parity snapshots— success or in progress; none failing.
Not verified, and why: the Windows unit suite has not run to completion on this branch, so every Windows-side claim in the description remains the author's, not evidence. Linux is marked Test (ubuntu-latest) is still running, so the regression risk the bigint conversions carry on the platform that currently works is unmeasured. The tokenizer fallback has no test at all — nothing in the diff asserts that @node-rs/jieba actually resolves @node-rs/jieba-wasm32-wasi when the native DLL is absent, so a green Windows run would not distinguish "the fallback works" from "the import path never got exercised". The author reports macOS ✅ locally; that is their claim and I have not re-run it.
Final CI results for 6099768 (auto-updated by the triage finalize job after CI completed):
| Check | Conclusion |
|---|---|
Test (ubuntu-latest, Node 22.x) |
❌ failure |
Classify PR |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Install (macos-latest) |
✅ success |
Install (ubuntu-latest) |
✅ success |
Install (windows-latest) |
✅ success |
Integration Tests (no-AK, No Sandbox) |
✅ success |
Lint & Static (ubuntu-latest, Node 22.x) |
✅ success |
Live Host (macos-latest) |
✅ success |
macos-latest / Java 21 |
✅ success |
OpenTUI no-flicker gate |
✅ success |
Real daemon E2E / Java 11 |
✅ success |
Serve A/B (ubuntu-latest, Node 22.x) |
✅ success |
TUI parity snapshots (ink vs opentui) |
✅ success |
ubuntu-latest / Java 11 |
✅ success |
ubuntu-latest / Java 17 |
✅ success |
ubuntu-latest / Java 21 |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
windows-latest / Java 21 |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。
Sandboxed verification would settle part of this: @qwen-code /verify — the two test-efficacy regressions above mean the suite stays green without pinning the change, which is precisely the gap an A/B run against the base build closes. Specifically, it would show whether the web-shell-brand fd identity re-check and the worktreeSessionService sidecar dev/ino guards still fail closed when only the inode differs, or whether they now trip on the type-mismatched dev comparison first. The author has write access, so @qwen-code /tmux is also available, though this PR has no TUI surface and I wouldn't spend a run on it. Neither lane substitutes for the Windows validation run finishing — that one is already dispatched and is the only thing that can confirm the baseline is actually restored.
中文说明
代码审查
我先只读标题和"为什么需要",在打开 diff 之前写下了自己的方案:修掉两个真正的产品缺陷(--import 需要 file:// URL,dev/ino 身份校验需要能承受 NTFS 的 64 位文件 ID),然后把确实只适用于 POSIX 的夹具放到平台守卫后面,并且跳过而不是删除断言,同时尽量不让分词器回退进入共享依赖树。前两点这个 PR 与我的方案一致,第三点做得比我预想的更好——Number.isSafeInteger(inode) || inode <= 0 守卫和 differentIdentity() 辅助函数的思路是对的,而意识到在有精度损失的浮点 inode 上 ino + 1 是个静默空操作,是很敏锐的发现。它在依赖放置上与我设想的不同,并且夹带了一处会打破既有仓库测试的改动。
🔴 阻塞项 —— minimumReleaseAgeExclude 的新增打破了本 PR 未修改的仓库测试
pnpm-workspace.yaml 多了一个条目 '@tybys/wasm-util@0.10.4',而 scripts/tests/package-scripts.test.js:386 用 toEqual(['@qwen-code/channel-base']) 精确锁定了这个数组(不是 toMatchObject),两元素数组必然失败。该测试文件不在本 PR 的 39 个改动文件里,所以没有任何地方更新这个期望值。测试里的 root 是 path.resolve(__dirname, '../..'),即仓库根目录,读的正是本 PR 修改的那个文件。
之所以现在还没变红,是通道覆盖问题,值得说清楚:该测试跑在 npm run test:scripts 下,属于 npm run test:ci 而不属于 npm run test:ci:workspaces。Test (ubuntu-latest, Node 22.x) 跑的是后者(ci.yml:764)。真正跑 test:ci 的两个通道——Test (windows-latest)(ci.yml:1653)和 Test (macos-latest)(ci.yml:1556)——在本 PR 上都是 skipped。所以唯一会执行它的,只有那个仍在运行中的 dispatch Windows 验证 run。
机械失败之外还有第二层。这个测试的名字本身就是策略:release-age 例外列表应当只放内部包,并且与版本无关。@tybys/wasm-util@0.10.4 是外部 registry 包且精确锁版本,两条都违反。把这个字符串加进期望值能让 CI 变绿,但同时删掉了该测试存在的意义。
两条出路,我倾向于第一条:
- 去掉这个例外。
@napi-rs/wasm-runtime@1.2.4声明的是@tybys/wasm-util: ^0.10.3,而overrides里锁到0.10.4正是导致解析到过新版本的直接原因。如果0.10.3(或该 range 允许的版本)能满足 release-age 策略,那么同时移除 override 和 exclude 条目即可解决,无需改动策略。值得先确认这个 pin 本身是否必要。 - 如果这个例外确实必需,那它属于供应链策略决定,而不是 Windows CI 细节——请有意识地更新测试,并在描述中说明为什么现在可以接受一个外部且锁版本的条目。
🟡 run-qwen-serve.test.ts —— 断言变成了空断言
hostname 由 '0' 改为 '0.0.0.0' 后,输入值、bind、boundAddress 三者完全相同,因此无论生产代码上报的是"输入值"还是"实际绑定地址",断言都会通过——这个测试原本要抓的东西没有了。而紧邻上方的注释仍写着"操作者的拼写与 socket 地址在此不同,因此这里锁定的是启动时上报实际绑定值而非输入值",现在这句描述的是一个不存在的情形。
需要放弃 '0' 是合理的——它是 POSIX 的 inet_aton 式简写,Windows 解析器不接受。但测试意图本身是可移植的:换一个与绑定结果不同的拼写('localhost' → 127.0.0.1 是最明显的跨平台组合),或者把 '0' 这一例限定在非 Windows 并保留。至少必须删掉那条过期注释,否则它会让后来的读者以为这个测试比实际更强。
🟡 web-shell-brand.test.ts —— inode 扰动已不再驱动断言
生产代码现在从 fs.lstatSync(filePath, { bigint: true, throwIfNoEntry: false }) 取 expectedStat,即 BigIntStats。而 "refuses a file swapped between the lstat and the open" 中的一次性 mock 仍然调用不带任何选项的 fsActual.fstatSync(fd),返回 number 形态的 Stats。
readRegularFileNoFollow 用裸 !== 比较且不做强转,所以 stat.dev !== expectedStat.dev 是 number !== bigint——恒为真——于是拒绝在 dev 这一项就触发了,ino 根本没被比较到。测试仍然通过,因为它只断言了通用的 'changed while it was being read' 警告。但它自己的注释写着"扰动 ino 正是驱动这一点的手段;删掉这个复检就会变红",这对 ino 这一项已不再成立:把 inode 比较删掉也依然是绿的。
一个词的修复——fsActual.fstatSync(fd, { bigint: true })——就能恢复。与 no-follow-open.ts 的对比很说明问题:那边 PR 把 assertSameIdentity 放宽为 { dev: number | bigint; ino: number | bigint } 并对两侧都做了 BigInt(...) 强转,因此同类扰动测试在混合形态下仍然有区分力。web-shell-brand.ts 没有这样处理,其测试也没有跟上。值得决定采用两种约定中的哪一种,因为它们现在不一致。
🟡 artifact-snapshots.ts —— 与 Windows 无关的顺手改动
把 await fs.readdir(references) 从 && 中提出来使它变成无条件执行,于是在 operationId 有值的分支上它也会运行,而此前根本不会。我跟了一遍,它接近于无副作用:此时 snapshotDirectory 已经对 references 做过 realpath;即便目录缺失,后续的 writeFile 也会抛出同样的 ENOENT 进入同一个 catch,其 !(code === 'ENOENT' && !operationId) 分支在两种写法下都会重新抛出。所以没有可观测的行为变化——只是每次 owned restore 多一次 syscall。
这恰恰是我建议去掉它的理由。它是 diff 中唯一一处既没有配套测试改动、描述里也没有关联任何 Windows 失败的生产改动。如果它确实在修什么,描述应当说明;如果没有,它不该出现在一个自称"每处改动都服务于基线"的 PR 里。
说明(非阻塞)
monitor-debug-store.ts是生产代码,不是测试。privateDirectory在 Windows 上跳过 mode 位与 uid 比较。可以理解——Node 在该平台报告的是合成的 mode 位,检查会无条件抛错——但这移除了该平台上"拒绝共享或属主异常的 debug 根目录"这道防护,而 PR 正文把它归入了被跳过的断言。测试侧,"rejects shared or symlink archive roots without touching their contents" 现在把整个 shared-root 部分(chmod 0o755、initialize()→ false、create()→ undefined)放进了非 Windows 守卫内,所以在 Windows 上它只验证 symlink 那一半,而测试名仍然声称两者。- 依赖放置。
@node-rs/jieba-wasm32-wasi放在dependencies而非optionalDependencies,导致 Linux 和 macOS 也会安装这条 WASI 依赖链,而其中包含@emnapi/core@2.0.0-alpha.4与@emnapi/runtime@2.0.0-alpha.4——两个 alpha 预发布版本因此进入了所有平台的生产依赖树,只为一个仅 Windows 需要的回退。两个 lockfile 是一起更新的,符合 AGENTS.md 的要求。 - 做得好的部分,免得被埋没:
pathToFileURL(require.resolve('tsx'))真正修掉了ERR_UNSUPPORTED_ESM_URL_SCHEME这个产品缺陷,而不是绕过它;git config --global core.autocrlf false作用域是安全的,因为该 job 把HOME/USERPROFILE导出为${{ runner.temp }}/qwen-ci-home;.replaceAll('\\', '/')针对的是 git 自身始终使用正斜杠的 porcelain 与 config 输出,而不是盲目重写断言;overwriteGitfile复用了文件里已有的辅助函数而非新增一个;opentui-footer.test.tsx编码了真实的 Windows 裸 Tab 回退行为而不是跳过,使两个平台都保留了覆盖。
测试证据
这是一次无人值守的 CI 运行,因此按 triage 规则我没有构建、运行或执行本 PR 的任何代码——没有 npm、没有 vitest、没有 checkout。以下内容全部是在被审查的那个 commit 上通过 API 读取 PR 自身的 CI 得到的。描述中引用的两个 workflow run 我也独立核实过,而不是采信链接:修复前的 run 34713863264 确实是 Qwen Code CI、event=schedule、head_branch=main、conclusion=failure;修复后的 run 34772671680 是 Windows Self-Hosted Runner Validation,在本分支上、正好在被审查的那个 SHA 上 dispatch 触发——而它仍在 in_progress,所以那个核心主张目前还没有任何证据支撑。
该 commit 上共 85 个 check-run:22 成功、58 跳过、5 进行中、0 失败。没有红的,但"没有红"这件事的证明力比看上去弱,因为真正检验本 PR 主题的两个通道被跳过了,而仓库级脚本测试在 Ubuntu 上不跑。上表(英文部分)列出了逐项结论,此处不再重复。
未验证的部分及原因:Windows 单元测试套件尚未在本分支上跑完,因此描述中所有 Windows 侧的说法目前仍只是作者的陈述,不是证据。Linux 被作者标记为 Test (ubuntu-latest) 仍在运行,所以 bigint 转换给当前正常工作的平台带来的回归风险还没有被测到。分词器回退完全没有测试——diff 中没有任何断言验证 @node-rs/jieba 在原生 DLL 缺失时确实会解析到 @node-rs/jieba-wasm32-wasi,因此即使 Windows run 变绿,也无法区分"回退生效了"和"这条 import 路径根本没被执行到"。作者称本地 macOS ✅;这是他们的主张,我没有重跑。
沙箱化验证可以解决其中一部分:@qwen-code /verify —— 上面两处测试效力退化意味着套件在不锁定该改动的情况下依然是绿的,而这正是与 base 构建做 A/B 所能填补的缺口。具体地说,它能表明 web-shell-brand 的 fd 身份复检与 worktreeSessionService 的 sidecar dev/ino 守卫在"仅 inode 不同"时是否仍然 fail closed,还是现在会先在类型不匹配的 dev 比较上触发。作者具备写权限,因此 @qwen-code /tmux 也可用,不过本 PR 没有 TUI 界面,我不建议为它花一次运行。两条通道都不能替代 Windows 验证 run 跑完——那一个已经 dispatch,也是唯一能确认基线是否真的恢复的手段。
— Qwen Code · qwen3.8-max-2026-09-02
Reviewed at 6099768ffbec01650363ab9b5356fd2b59c4f287 · re-run with @qwen-code /triage
|
Confidence: 2/5 — the portability work is good and the problem is real, but the PR breaks an existing repo test it didn't update, and the only lane that would have caught it is skipped. Stepping back: I verified the premise myself rather than accepting it. Run 34713863264 is genuinely a failed scheduled Against my own independent proposal, the PR is better in the places I expected it to be worse. I assumed a "make CI green on Windows" PR would mostly be What I can't get past is that it doesn't currently pass its own repo's tests. The part that concerns me more than the failure is why it was invisible. Two secondary things I'd want fixed rather than merely noted. Am I being unfair to a well-intentioned infra PR from someone with admin on the repo? I checked that instinct. The blocker is objective, the two efficacy regressions are one-line fixes each, and Requesting changes on the blocker plus the two assertion fixes. The Windows validation run is still in flight, so the headline claim is unverified either way — but that isn't what I'm gating on, and it doesn't need to finish before the YAML and test issues are addressed. 中文说明Confidence: 2/5 —— 可移植性工作本身做得好,问题也真实存在,但本 PR 打破了一个它并未更新的既有仓库测试,而唯一能发现这一点的通道恰好被跳过了。 退一步看:我自己核实了前提,而不是直接采信。Run 34713863264 确实是 与我自己的独立方案相比,这个 PR 在我原以为会更弱的地方反而更好。我原以为一个"让 CI 在 Windows 上变绿"的 PR 主要是 我无法放过的是:它目前通不过自己仓库的测试。 比这个失败本身更让我在意的是它为什么没被看见。常规 PR CI 上 另有两点我希望被修掉,而不只是记录在案。 我是不是在苛待一个来自仓库 admin、用心良好的基础设施 PR?我检查过这个直觉。阻塞项是客观的,两处效力退化各自都是一行修复,而 基于该阻塞项加上两处断言修复,请求变更(request changes)。Windows 验证 run 仍在进行,所以核心主张无论如何都还未被验证——但那不是我把关的依据,而且在 YAML 与测试问题被处理之前,它也不需要先跑完。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Needs some rethinking — see my notes above. 🙏
One blocker and two one-line test fixes:
Blocker — breaks an existing repo test. pnpm-workspace.yaml adds '@tybys/wasm-util@0.10.4' to minimumReleaseAgeExclude, but scripts/tests/package-scripts.test.js:386 pins that array exactly with toEqual(['@qwen-code/channel-base']), and that test file isn't in this diff. It runs under npm run test:scripts (part of test:ci, not of test:ci:workspaces), so Test (ubuntu-latest) won't catch it and both lanes that would are skipped on this PR — only the still-running Windows validation run executes it.
Note the test's name is the policy: that list is meant to hold internal packages and be version-independent. @tybys/wasm-util@0.10.4 is external and version-pinned, so it breaks both halves — adding the string to the expectation would green CI while deleting the invariant. Preferred fix: @napi-rs/wasm-runtime@1.2.4 only asks for ^0.10.3, so check whether the overrides pin to 0.10.4 is load-bearing at all; dropping the override and the exclude entry together resolves this with no policy change.
run-qwen-serve.test.ts — hostname: '0' → '0.0.0.0' makes the typed value identical to bind and boundAddress, so the assertion passes whichever one production reports. The comment above it ("the operator spelling and the socket address differ here") now describes a situation that doesn't exist. 'localhost' → 127.0.0.1 keeps the intent portably.
web-shell-brand.test.ts — the fstatSync mock calls fsActual.fstatSync(fd) with no options while production now uses { bigint: true } for expectedStat. Since readRegularFileNoFollow compares with bare !==, stat.dev !== expectedStat.dev is number !== bigint and always trips first, so the ino perturbation the test was written around is never compared. It passes, but deleting the inode check would no longer turn it red. fsActual.fstatSync(fd, { bigint: true }) restores it.
Also worth a look, non-blocking: the artifact-snapshots.ts readdir hoist is the only production edit with no test change and no stated Windows link (I traced it as behaviorally inert, which is the argument for dropping it); and @node-rs/jieba-wasm32-wasi in dependencies rather than optionalDependencies pulls @emnapi/*@2.0.0-alpha.4 into the production tree on Linux and macOS for a Windows-only fallback.
The portability work itself is genuinely good — the Number.isSafeInteger inode gates, differentIdentity() replacing the lossy-float ino + 1 no-op, the BigInt() coercion in assertSameIdentity, and pathToFileURL(require.resolve('tsx')) as a real product fix rather than a test accommodation. That's why this is a send-back, not a rethink.
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
- pnpm-workspace.yaml: drop '@tybys/wasm-util@0.10.4' from minimumReleaseAgeExclude. scripts/tests/package-scripts.test.js pins that list to internal, version-independent entries. The exclusion is also inert: minimumReleaseAge is not configured anywhere in the repo, and pnpm-lock.yaml records `overrides` but not minimumReleaseAgeExclude -- so the '@tybys/wasm-util': '0.10.4' override stays, because dropping it would desync the lockfile and fail --frozen-lockfile. - web-shell-brand.test.ts: build the swapped-file fake as BigIntStats. Production compares bigint fields with a bare `!==`, so a number-backed fake tripped `dev` first and the perturbed inode was never reached; removing the perturbation now turns the test red again. - run-qwen-serve.test.ts: '0' is unresolvable on Windows (getaddrinfo ENOTFOUND 0), so the wildcard case keeps '0.0.0.0' and witnesses the socket rather than a spelling: the operator typed port 0, so the report must carry the ephemeral port the listener actually got. Spelling normalisation stays pinned platform-independently in remote-quickstart.test.ts. - artifact-snapshots.ts: revert the readdir hoist to main. It is behaviourally inert (the catch rethrows ENOENT on the operationId path either way), has no test and no Windows link. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmu07e78o2w
|
Addressed in 1. Blocker — Dropped The 2.
So the wildcard case keeps 3. The fake is now built from 4a. Confirmed inert: on the 4b. Nothing in the repo imports it ( Verification
|
…ss fixes The previous commit dropped '@tybys/wasm-util@0.10.4' from minimumReleaseAgeExclude to satisfy scripts/tests/package-scripts.test.js:390, on the reasoning that the entry was inert because minimumReleaseAge is not configured in this repo. That reasoning was wrong and CI measured it: the gate is enforced by pnpm itself, not by repo config, so removing the exclude failed 'Install frozen pnpm worktree dependencies' on all three platforms with Lockfile failed supply-chain policy check (1975 entries) @tybys/wasm-util@0.10.4 was published at 2026-09-13T08:00:13.464Z, within the minimumReleaseAge cutoff (2026-09-12T20:03:50.286Z) (run 34779568628; Install was green at 6099768, which carries the entry). Restoring the line makes pnpm-workspace.yaml byte-identical to that head again. The three other changes from that commit are kept, since they are independent of the exclude and were green in run 34779568625 (Test 18m58s, Lint & Static 12m36s, Integration no-AK 7m8s): the web-shell-brand.test.ts BigIntStats fake that re-arms the inode witness, the run-qwen-serve.test.ts socket witness replacing the tautological '0.0.0.0' assertion, and the artifact-snapshots.ts revert to main. This leaves the package-scripts.test.js assertion red exactly as it was before this round. That is now a measured conflict rather than an open question: the repo test pins the exclude list to internal, version-independent entries, while pnpm needs a version-pinned exemption for a package published the same day. Resolving it means either dropping the '@tybys/wasm-util': '0.10.4' override and regenerating both lockfiles so an older, age-compliant version resolves, or relaxing the invariant to admit external version-pinned excludes. Both are supply-chain policy calls for the author/maintainer, so neither is made here. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmu07e78o2w
|
Correction to my previous comment — one of its claims was wrong, and CI proved it. Reverted in I wrote that the
Kept from the previous commit, since they are independent of the exclude and were green in run 34779568625 (
Where that leaves your blocker finding: still open, and now measured rather than argued.
The two ways out both need an owner decision, so I am not picking one in a maintenance pass:
Flagging one thing I found while measuring, unchanged by any of this: a second latent bypass of the same guard sits at |
…exempting it Test (ubuntu-latest, Node 22.x) failed at scripts/tests/package-scripts.test.js:390 (run 34781999892), which pins minimumReleaseAgeExclude to internal, version-independent entries, while the previous head carried '@tybys/wasm-util@0.10.4' there to satisfy pnpm's minimumReleaseAge gate. The two looked mutually exclusive, but only because the pin chose the newest release. @node-rs/jieba-wasm32-wasi pulls in @napi-rs/wasm-runtime@1.2.4, which accepts ^0.10.3, and 0.10.3 was published 2026-06-23 -- well outside the 24h window that rejected 0.10.4 (published 2026-09-13T08:00:13Z). So the pnpm override now pins 0.10.3, the exclude entry is dropped, and both lockfiles follow: package-lock.json must lock the same version or scripts/check-lockfile.js's "pnpm lockfile against package-lock.json" agreement gate fails, which also reddens the two check-lockfile fixture branches in package-scripts.test.js because they copy the real lockfiles. The pin lives on the pnpm side only, mirroring how the typescript pins already diverge; npm has no release-age gate, so no package.json override is added and the nested @tailwindcss/oxide-wasm32-wasi copy of @tybys/wasm-util is left alone. Once 0.10.4 ages out of the window, a maintainer can drop the override from both sides if nothing else needs the pin. Verified locally: pnpm 11.24.0 `install --lockfile-only` reports "Lockfile passes supply-chain policies (1975 entries)"; scripts/tests/package-scripts.test.js 49 passed / 1 skipped (was 3 failed); scripts/tests/check-lockfile.test.js 14 passed; prettier --check clean. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmu0ce8qb32
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 19 passed · 0 failed · 19 total Flakiness gate: 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:19 通过 · 0 失败 · 19 总计 抖动门: Verification reportPR 11787 — fix(ci): restore Windows test baselineVerdict: 中文摘要
Central claim and A/BCentral claim. The PR's production changes make file-identity security guards exact where they were previously lossy: Windows reports 64-bit file IDs, A/B cells. Both arms are the real production module compiled from source (
8/8 assertions per arm, 0 unexpected. Witness: Secondary claim 1 — the re-armed witnesses are non-vacuous. Commit Secondary claim 2 — dependency/lockfile consistency. CorrectionsNone required. The two commit-message claims I tested (the re-armed BigIntStats witness; the pnpm age-gate enforcement) both held under measurement. FindingsF1 — Suggestion: the
|
| # | reverted hunk | suite | result |
|---|---|---|---|
| M1 | web-shell-brand.ts BigIntStats lstat/fstat |
web-shell-brand.test.ts |
SURVIVED — 90/90 green |
| M2 | worktreeSessionService.ts bigint sidecar identity |
worktreeSessionService.test.ts |
KILLED — 1 failed / 31 passed, AssertionError: expected { state: 'invalid', … } to deeply equal … (the reason field differs) |
| M3 | no-follow-open.ts bigint identity compare |
no-follow-open.test.ts |
SURVIVED — 17/17 green |
| M4 | monitor-debug-store.ts win32 permission skip |
monitor-debug-store.test.ts |
SURVIVED — 13/13 green (unobservable on Linux, see F3) |
The two survivors are not vacuous-harness artifacts: same-file positive controls prove the suites do exercise those guards — disabling the fd identity re-check in web-shell-brand.ts turns 2 tests red, and no-op'ing assertSameIdentity in no-follow-open.ts turns 7 red (positive-controls.txt). Classification: coverage gap, not dead code and not redundant defence — the behaviour is correct (the A/B above proves the mechanism), but every existing fixture perturbs inodes inside the safe-integer range, where number and bigint comparisons agree, so a regression of the bigint change would ship green. The missing fixture is exactly harness/identity-ab.mjs: perturb the fake's ino to two values that differ only above 2^53 and assert the refusal.
F3 — Suggestion: the new win32 branch is untestable off a real Windows runner
monitor-debug-store.ts now skips the mode & 0o077 / uid checks when process.platform === 'win32'. M4 survives because on Linux the mutation is a no-op by construction, and no test stubs process.platform; the suite's own mode assertions are if (process.platform !== 'win32')-guarded (lines 228, 412). So the branch is covered only by the Windows lane this PR is trying to make green. Acceptable for a CI-baseline PR, but a vi.stubGlobal('process', {…platform:'win32'}) case would make it reviewable anywhere. Related accepted-tradeoff boundary, checked and correct: same-file.test.ts now skips when !Number.isSafeInteger(inode), matching production's deliberate degradation to canonical-spelling comparison — the skip is honest, not a coverage dodge.
F4 — Suggestion: the dependency footprint is understated, and it ships an alpha prerelease chain on every platform
@node-rs/jieba-wasm32-wasi@2.0.2 is a non-optional dependency of packages/qwen-live, so it installs on Linux and macOS too: 2.2 MB on top of the 11 MB native @node-rs/jieba (+20%), pulling @emnapi/core@2.0.0-alpha.4, @emnapi/runtime@2.0.0-alpha.4 (nested under @node-rs/jieba-wasm32-wasi/node_modules/), @emnapi/wasi-threads@2.0.1 and @napi-rs/wasm-runtime@1.2.4. The PR body calls this "a small dependency footprint". Reachability is real and verified in the shipped loader (node_modules/@node-rs/jieba/index.js:646,655 requires @node-rs/jieba-wasm32-wasi when the native binding fails to load), so the dependency is not dead — but the alpha-prerelease chain and the all-platform cost belong in the risk section, and an optionalDependencies entry keyed to win32 would avoid installing it elsewhere.
Checked and acceptable (not findings)
integrations/external-context-mem0/src/auto-recall.integration.test.tsaddssocket.on('error', () => undefined)on the decoy stalled-TLS server (the "expected Windows socket reset" commit). The suppression is scoped to the fixture's own decoy socket; the test's oracle — the CLI child exiting successfully after the provider timeout — is untouched, and an aborted client resetting the decoy is the scenario by design. Information is not lost anywhere a reader looks.git config --global core.autocrlf falseinci.yml/windows-runner-smoke.ymlis load-bearing despite.gitattributes(* text=auto eol=lf): the attribute governs the main checkout, but the tests create temporary repos that inherit no.gitattributes, where a Windows-installedautocrlf=truewould produce CRLF checkouts and break path/identity assertions. It is set inside the isolatedHOME=${{ runner.temp }}/qwen-ci-homeafter that directory is created, and is a no-op on Linux where the same step also runs.
Not covered
- No Windows execution. This container is Linux. The Windows half of the central claim is proven by mechanism (the lossy-double A/B above) plus the repo's own documented NTFS rounding, not by a Windows run; the PR's own Windows validation runs (34713863264, 34800332338) are cited by the author and were not independently observed.
- Per-commit attribution impossible. The checkout is depth 2:
git rev-list HEAD^1..HEAD^2returns 1 commit while$QWEN_VERIFY_CONTEXTlists 7 (b7a3ed7e,6099768f,3cf18045,d1535642,adba6daf,3bd8e9bbe2,c2c00a10). All verification is against the aggregateHEAD^1..HEADdiff. run-qwen-serve.test.ts: only the one changed case ran (-t 'reports what the socket bound…', 1 passed / 427 skipped by filter); the remaining 427 cases in that 13k-line file were not executed here.- Full workspace suites and
npm run typecheckwere not re-run end to end; the CI image had already built every workspace at HEAD (which type-checks), and I ran the changed-file suites instead (counts above). - The fresh-resolve measurement in F1 used the public registry from this container; it proves what npm's resolver picks for
^0.10.3today, not what it picked when the author generated the lockfile. verify-capture.mjswas used as shipped; both evidence images are its output.
Methodology
Environment: the CI verify container (node:22-bookworm, no GitHub token), working tree = refs/pull/11787/merge at depth 2, npm ci + npm run build pre-run at HEAD. Harnesses live in harness/: identity-ab.mjs drives the real production module compiled per-arm by esbuild from HEAD^1 and working-tree sources behind fs-shim.mjs (a shallow mutable copy of node:fs with O_NOFOLLOW removed, mirroring Windows), asserting reachability (stat call counts) before the oracle; mutation-matrix.sh and positive-controls.sh revert or disable one hunk at a time via git show HEAD^1:<file> / python3 string edits and always restore (git status --porcelain empty at exit). Gates ran per workspace with npx vitest run <files> from the package directory. Raw logs: logs-head-suites.txt, logs-cli-suites.txt, logs-mutation.txt, logs-controls.txt; images in evidence/.
Flakiness gate log
rounds=5 files=27 skipped=0
file integrations/external-context-mem0/src/auto-recall.integration.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/auto-recall.integration.test.ts
file packages/acp-bridge/src/sessionAttachments.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/sessionAttachments.test.ts
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/commands/review/cleanup.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/cleanup.test.ts
file packages/cli/src/commands/review/findings.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/findings.test.ts
file packages/cli/src/commands/review/lib/git.integration.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/git.integration.test.ts
file packages/cli/src/commands/review/lib/same-file.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/same-file.test.ts
file packages/cli/src/commands/review/lib/worktree.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/worktree.test.ts
file packages/cli/src/commands/review/repo-context.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/repo-context.test.ts
file packages/cli/src/commands/review/scratch-tree.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/scratch-tree.test.ts
file packages/cli/src/commands/review/test-efficacy.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/test-efficacy.test.ts
file packages/cli/src/serve/conversations/conversation-workspace.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/conversation-workspace.test.ts
file packages/cli/src/serve/conversations/standalone-deletion-journal.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/standalone-deletion-journal.test.ts
file packages/cli/src/serve/run-qwen-serve.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/run-qwen-serve.test.ts
file packages/cli/src/serve/session-attachments-root.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/session-attachments-root.test.ts
file packages/cli/src/services/web-shell-brand.test.ts: (cd packages/cli) npx --no-install vitest run ./src/services/web-shell-brand.test.ts
file packages/cli/src/ui/opentui/input-prompt.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/input-prompt.test.tsx
file packages/cli/src/ui/opentui/opentui-footer.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/opentui-footer.test.tsx
file packages/core/src/services/session-sources.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/session-sources.test.ts
file packages/core/src/services/worktreeSessionService.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/worktreeSessionService.test.ts
file packages/core/src/tools/workflow/workflow.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/workflow/workflow.test.ts
file packages/core/src/utils/no-follow-open.test.ts: (cd packages/core) npx --no-install vitest run ./src/utils/no-follow-open.test.ts
file packages/qwen-live/src/language-preferences.test.ts: (cd packages/qwen-live) npx --no-install vitest run ./src/language-preferences.test.ts
file packages/qwen-live/src/memory/config.test.ts: (cd packages/qwen-live) npx --no-install vitest run ./src/memory/config.test.ts
file packages/qwen-live/src/memory/service.test.ts: (cd packages/qwen-live) npx --no-install vitest run ./src/memory/service.test.ts
file packages/qwen-live/src/proactive/monitor-debug-store.test.ts: (cd packages/qwen-live) npx --no-install vitest run ./src/proactive/monitor-debug-store.test.ts
file packages/vscode-ide-companion/src/diff-manager.test.ts: (cd packages/vscode-ide-companion) npx --no-install vitest run ./src/diff-manager.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
integrations/external-context-mem0/src/auto-recall.integration.test.ts: PPPP
packages/acp-bridge/src/sessionAttachments.test.ts: PPPP
packages/cli/src/acp-integration/acpAgent.test.ts: PPPP
packages/cli/src/commands/review/cleanup.test.ts: PPPP
packages/cli/src/commands/review/findings.test.ts: PPPP
packages/cli/src/commands/review/lib/git.integration.test.ts: PPPP
packages/cli/src/commands/review/lib/same-file.test.ts: PPPP
packages/cli/src/commands/review/lib/worktree.test.ts: PPPP
packages/cli/src/commands/review/repo-context.test.ts: PPPP
packages/cli/src/commands/review/scratch-tree.test.ts: PPP
packages/cli/src/commands/review/test-efficacy.test.ts: PPP
packages/cli/src/serve/conversations/conversation-workspace.test.ts: PPP
packages/cli/src/serve/conversations/standalone-deletion-journal.test.ts: PPP
packages/cli/src/serve/run-qwen-serve.test.ts: PPP
packages/cli/src/serve/session-attachments-root.test.ts: PPP
packages/cli/src/services/web-shell-brand.test.ts: PPP
packages/cli/src/ui/opentui/input-prompt.test.tsx: PPP
packages/cli/src/ui/opentui/opentui-footer.test.tsx: PPP
packages/core/src/services/session-sources.test.ts: PPP
packages/core/src/services/worktreeSessionService.test.ts: PPP
packages/core/src/tools/workflow/workflow.test.ts: PPP
packages/core/src/utils/no-follow-open.test.ts: PPP
packages/qwen-live/src/language-preferences.test.ts: PPP
packages/qwen-live/src/memory/config.test.ts: PPP
packages/qwen-live/src/memory/service.test.ts: PPP
packages/qwen-live/src/proactive/monitor-debug-store.test.ts: PPP
packages/vscode-ide-companion/src/diff-manager.test.ts: PPP
verdict: timeout
summary: only 3 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · integrations/external-context-mem0/src/auto-recall.integration.test.ts: P (exit 0)
round 1 · packages/acp-bridge/src/sessionAttachments.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/cleanup.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/findings.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/git.integration.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/same-file.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/worktree.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/repo-context.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/scratch-tree.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/test-efficacy.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/conversations/conversation-workspace.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/conversations/standalone-deletion-journal.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/session-attachments-root.test.ts: P (exit 0)
round 1 · packages/cli/src/services/web-shell-brand.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/input-prompt.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/opentui/opentui-footer.test.tsx: P (exit 0)
round 1 · packages/core/src/services/session-sources.test.ts: P (exit 0)
round 1 · packages/core/src/services/worktreeSessionService.test.ts: P (exit 0)
round 1 · packages/core/src/tools/workflow/workflow.test.ts: P (exit 0)
round 1 · packages/core/src/utils/no-follow-open.test.ts: P (exit 0)
round 1 · packages/qwen-live/src/language-preferences.test.ts: P (exit 0)
round 1 · packages/qwen-live/src/memory/config.test.ts: P (exit 0)
round 1 · packages/qwen-live/src/memory/service.test.ts: P (exit 0)
round 1 · packages/qwen-live/src/proactive/monitor-debug-store.test.ts: P (exit 0)
round 1 · packages/vscode-ide-companion/src/diff-manager.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/auto-recall.integration.test.ts: P (exit 0)
round 2 · packages/acp-bridge/src/sessionAttachments.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/cleanup.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/findings.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/git.integration.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/same-file.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/worktree.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/repo-context.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/scratch-tree.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/test-efficacy.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/conversation-workspace.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/standalone-deletion-journal.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/session-attachments-root.test.ts: P (exit 0)
round 2 · packages/cli/src/services/web-shell-brand.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/opentui/input-prompt.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/opentui/opentui-footer.test.tsx: P (exit 0)
round 2 · packages/core/src/services/session-sources.test.ts: P (exit 0)
round 2 · packages/core/src/services/worktreeSessionService.test.ts: P (exit 0)
round 2 · packages/core/src/tools/workflow/workflow.test.ts: P (exit 0)
round 2 · packages/core/src/utils/no-follow-open.test.ts: P (exit 0)
round 2 · packages/qwen-live/src/language-preferences.test.ts: P (exit 0)
round 2 · packages/qwen-live/src/memory/config.test.ts: P (exit 0)
round 2 · packages/qwen-live/src/memory/service.test.ts: P (exit 0)
round 2 · packages/qwen-live/src/proac
...truncated -- full content in the run artifacts.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-11 boot-level test no longer discriminates bind from boundAddress — already reported (review 5191690673, the open CHANGES_REQUESTED body, item 2 of 3)
Not reviewed: reverse-audit round 5's three incidental candidates — never verified, because the loop stopped on the time budget before a round could rule on them.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped by the PR-event matrix filter; Windows ground was covered instead by the dispatched validation run at ancestor 6099768, whose logs this review analyzed, not by a run at this head.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and no darwin run exists at this head; the diff's platform branches were walked analytically, not executed.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 4)": executing any changed suite — a run needs npm run build in this shared worktree first, so every conclusion above is read-based, including both findings' Windo…; "agent reverse-audit (round 4)": standalone-deletion-journal.test.ts:348-430 (tail of the "same-session fence" body) and worktree.test.ts 's overwriteGitfile helper region beyond the diff …; "agent 6c": did not cross-check packages/vscode-ide-companion/src/diff-manager.ts production path resolution against the new path.join -based Uri.joinPath mock (only t…; "agent 6c": did not read managedConversationExpectation / getConversationDirectoryName behind the new expectation.child.inode === 0 skip in acpAgent.test.ts:7373-7378; "agent 6c": did not execute any of the changed suites or npm run typecheck / check:lockfile ; all findings are from reading the post-change sources, and 5 more.
Not reviewed: reverse audit — stopped before round 6 by the review time budget.
Not linted (tool limitation, not a blocker): .github/workflows/ci.yml — actionlint embedded-shell source mapping is not yet supported; .github/workflows/windows-runner-smoke.yml — actionlint embedded-shell source mapping is not yet supported.
Test Plan (not a blocker): 325 tests passing — this review observed 2070, 31231, 26315, 1016, 587, 2021, 7990 passed.
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查(原文为英文):reverse-audit round 5's three incidental candidates — never verified, because the loop stopped on the time budget before a round could rule on them.
未审查(原文为英文):build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查(原文为英文):build-and-test — Test (windows-latest, Node 22.x) was skipped by the PR-event matrix filter; Windows ground was covered instead by the dispatched validation run at ancestor 6099768, whose logs this review analyzed, not by a run at this head.
未审查(原文为英文):build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and no darwin run exists at this head; the diff's platform branches were walked analytically, not executed.
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 4)":executing any changed suite — a run needs npm run build in this shared worktree first, so every conclusion above is read-based, including both findings' Windo…;"agent reverse-audit (round 4)":standalone-deletion-journal.test.ts:348-430 (tail of the "same-session fence" body) and worktree.test.ts 's overwriteGitfile helper region beyond the diff …;"agent 6c":did not cross-check packages/vscode-ide-companion/src/diff-manager.ts production path resolution against the new path.join -based Uri.joinPath mock (only t…;"agent 6c":did not read managedConversationExpectation / getConversationDirectoryName behind the new expectation.child.inode === 0 skip in acpAgent.test.ts:7373-7378;"agent 6c":did not execute any of the changed suites or npm run typecheck / check:lockfile ; all findings are from reading the post-change sources,另有 5 条。
未审查:反向审计——评审时间预算不足,未能开始第 6 轮。
未检查(工具限制,非阻断):.github/workflows/ci.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持;.github/workflows/windows-runner-smoke.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
Test Plan(非阻断):325 tests passing — this review observed 2070, 31231, 26315, 1016, 587, 2021, 7990 passed。
— qwen3.8-max via Qwen Code /review (v0.23.3)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
|
Windows ECS validation is green on the current head ( |
Each of these was measured, not argued; the mutation named in every case is
the one that used to leave the suite green.
- no-follow-open.test.ts: add the >2^53 identity case. Removing
`{ bigint: true }` from all four production stat call sites left 17/17
green, so the Windows-precision conversion this PR exists for was
unwitnessed. The fixture uses 2^60 offsets, where the double spacing is
256 and both ids collapse to one value; 2^53+1 / 2^53+2 do NOT collapse
(2^53+2 is exactly representable), so those constants cannot pin it.
Two-arm measured: intact 18/18 pass, mutated exactly 1 fails.
- web-shell-brand.test.ts: stat the planted regular file with
`{ bigint: true }`. Production compares BigIntStats with a bare `!==`, so
the number-backed fake tripped `dev` on a type mismatch alone and would
have refused the correct file too. Probed both arms: pre-fix
`typeof expectedStat.dev === 'number'` with the `dev` clause true,
post-fix both sides bigint with `dev` false and the refusal driven by a
real inode mismatch.
- conversation-workspace.test.ts: move the `inodeVerifiable` gate below the
two `toBe(identity)` assertions. They are inode-independent, so gating
them deleted a witness the Windows lane was green on. Modelled an
unverifiable-inode host: the test reports skipped, not failed, which is
only reachable once both assertions have passed.
- worktree.test.ts: split `localFilterCommands` in two. The fail-closed
branch is platform-free by construction and the base Windows log shows it
passing there; only the `a\nb` fixture is Windows-impossible, so only that
half keeps the gate.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Both workflows now run `git config --global core.autocrlf false` twice: once before the checkout and once inside the test step, where HOME/USERPROFILE are redirected to a freshly created empty directory. The second line therefore writes a different global config than its twin, and on ci.yml's hosted windows-2022 fallback the pre-checkout step never runs at all because it is gated on `runner.environment == 'self-hosted'`. It was the only one of the three occurrences with no comment, so deduping it looks free and silently returns CRLF conversion to every git operation the suites perform. Replaying no-ak-integration-ci.test.js's own assertions with the second occurrence deleted gives identical indices and still passes, so no test notices the removal. Comments only. The ordering assertions search for the FIRST occurrence and none of the added text matches a needle they look for; the 14 assertions in no-ak-integration-ci.test.js still pass. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The Lint & Static lane's "Check lint gate freshness" step failed at 7c5b501: main advanced 16 commits past this branch's merge base ef0b8a6 and changed two of the gate-defining files the check watches, eslint.config.js and .github/scripts/check-lint-gate-freshness.mjs. The lane checks out refs/pull/N/head alone, so a green run only proves the branch passes the gate as the branch defines it — incorporating main is what makes the branch validate against the base's current gate. No conflicts: main did not touch ci.yml, so the autocrlf why-comments added here are unaffected, and none of the four re-armed test files changed upstream. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
chiga0
left a comment
There was a problem hiding this comment.
Scope: Production changes (no-follow-open.ts, worktreeSessionService.ts, web-shell-brand.ts, native-lsp-service.ts, host-client.ts, artifact-snapshots.ts) read fully. CI: ci.yml, windows-runner-smoke.yml. Triage: Standard/Deep.
Not reviewed: Windows runtime behaviour; CI state.
Findings
No blocking findings.
Bot R1-1 (no-follow-open.ts, BigInt regression) — refuted. Both call sites now use { bigint: true } for both before and after. assertSameIdentity wraps both sides in BigInt() which is idempotent on bigints and converting on numbers, so mixed-type pairs still compare correctly. The 'refuses an identity that differs only above 2^53' test is the case that motivated this change. R1-1 does not hold against this diff.
Bot R1-2 (ci.yml, autocrlf) — unsettled. Cannot verify without a Windows runner. PR provides evidence of Windows CI passing at d7b8b97c68f8. Not added to approvalBlockers.
Bot R1-3 (WASI tokenizer) — unsettled; Windows CI evidence provided. The PR demonstrates the fallback path loads on Windows. Not blocking.
Bot R1-6 (second BigInt mock in web-shell-brand.test.ts) — minor. One of the two fstatSync mocks in the test drives the isFile() arm, not the dev/ino check, so no false negative — just a cleanup opportunity.
Own checks: pathToFileURL fix in host-client.ts correctly handles Windows drive paths for --import. BigInt comparisons in worktreeSessionService.ts consistent throughout. monitor-debug-store.ts uid guard now correctly type-checks process.getuid. vi.restoreAllMocks() additions prevent mock leakage. All correct.
No blocking findings. Approval blockers: none.
Reviewed with AI assistance.
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Reviewed head b36b347b against base e04f2ec5. No new blocker; two notes and one process caveat.
- The ci-bot blocker is fixed at this head.
pnpm-workspace.yaml'sminimumReleaseAgeExcludenow holds only'@qwen-code/channel-base'; the tokenizer pin lives under the overrides block with an explanatory comment about the age-compliant version, soscripts/tests/package-scripts.test.jsis no longer broken by an added entry. I re-read both files at the head rather than trusting the resolved flags. R1-26still stands. The skips atstandalone-deletion-journal.test.ts:144/261/309are stillit.skipIf(process.platform === 'win32')with no reason recorded, and thesame-file.test.tsgate widened fromino === 0to!Number.isSafeInteger(inode). I do not read the widening as a coverage loss worth blocking:same-file.tsdocuments that only safe positive values are exact identity proof and degrades to canonical spelling below that, and mainstream Linux inode values still run the assertion. What is missing is the one-line comment naming the volume class that now skips — which is what R1-26 asks for.- Process caveat. The Windows evidence in the PR body is at
d7b8b97c, which predates this head (b36b347b, and the21f40604merge), so the head itself is unvalidated on Windows. A Linux-green run cannot cover that lane; a re-dispatch before merge would.
Not chased to a conclusion (non-blocking): native-lsp-service.ts:782 replaces fs.accessSync(filePath, R_OK) with an isUsableWorkspaceSymbolFile check that also requires isFile() and root-prefix containment — a behaviour change on every platform inside a Windows-baseline PR; and artifact-snapshots.ts:187 hoists a readdir out of the !operationId short-circuit, adding an unconditional syscall with no functional change.
Reviewed with AI assistance.
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
APPROVE
已核对 head b36b347bf53dad0c1aad06e51b495d4a5648876c(vs base e04f2ec5d4)。required 全部完成且 success(Test (ubuntu-latest, Node 22.x)、Lint & Static、Integration Tests (no-AK, No Sandbox)、web-shell E2E Smoke、TUI parity snapshots、OpenTUI no-flicker gate;27 项 pass、0 fail,唯一 pending 是评审机器人自身档 review-pr)。
这不只是一次测试加跳过,生产侧改动我逐处读过并认为正确:
packages/core/src/utils/no-follow-open.ts:assertSameIdentity改收{ dev, ino }(number | bigint)并用BigInt()比较,两侧取统计信息都改为{ bigint: true }(lstatSync、fstatSync、handle.stat({bigint:true}))。这修的是真实缺陷:NTFS 上 number 形式的 dev/ino 有精度损失,两个不同文件可能比出相同值;hasVerifiableInode的「无法验证即拒绝」前置门保持不变,因此方向是收紧不是放宽。packages/cli/src/serve/fs/workspace-file-system.ts:resolveSameHostToolWriteTarget的三处lstat同样改为bigint: true,与上面的身份比较口径一致。packages/core/src/code-mode/host-client.ts:dev 入口的--import参数由裸require.resolve('tsx')改为pathToFileURL(...).href,这是 Node ESM loader 的正确形式,也正好是 Windows 盘符路径失败的那个成因;生产构建分支未动。packages/core/src/lsp/native-lsp-service.ts:预热候选从fs.accessSync(R_OK)改为复用isUsableWorkspaceSymbolFile并抛出有信息的错误,仍在原 try 内,失败处理路径不变。.github/workflows/ci.yml与windows-runner-smoke.yml各补一行git config --global core.autocrlf false,注释说明为什么不是重复:该步骤重定向了 HOME/USERPROFILE,读到的是刚创建的空 profile,而预 checkout 那一处又只在 self-hosted 上生效 —— 删掉这行会让 CRLF 转换回到测试里且没有测试会报红。这个理由我按两处步骤的 gating 核过,成立。pnpm-workspace.yaml把@tybys/wasm-util钉到0.10.3:0.10.4 刚发布、被minimumReleaseAge门禁挡下,注释说明为什么选「钉版本」而不是「加豁免」(package-scripts.test.js钉住该豁免列表只放内部包)。方向我认同:不去放宽门禁,而是绕开新鲜度问题。
记录三点(不阻塞):
packages/qwen-live/src/proactive/monitor-debug-store.ts的privateDirectory()现在只在非 win32 上检查mode & 0o077与 uid。判断我认同(Windows 上 POSIX mode/uid 不可信,原检查在 Windows 等于必然失败),但它确实是一次平台性放宽:Windows 侧现在只剩isDirectory()与isSymbolicLink(),目录可写性要靠 ACL。建议在同一处注释里点明「Windows 依赖 ACL,POSIX 位检查不适用」,并考虑后续补一条 ACL 校验。- R1-26 仍挂着未解决,作者明确回复这是需要 owner 决策的安全边界问题(
lib/same-file.ts:24的statSync不带 bigint 使硬链接身份退化成 realpath 拼写比较,findings.ts:1274/repo-context.ts:371的抗别名门在 Windows 上失效;standalone-deletion-journal.ts:118-127同理)。这两处都不是本 PR 引入的,本 PR 只是移除了它们在 CI 里的信号,所以我按你的处理签字;但请按你自己说的「需要 owner 决策」把它落成 issue(PR 描述现在是Linked Issues: N/A),否则这条 visibly open 的线程会在合并后失去着力点。 - 平台跳过里有些点没有就地原因:
commands/review/cleanup.test.ts:339、serve/conversations/standalone-deletion-journal.test.ts:262、:310的if (process.platform === 'win32') { ctx.skip(); }上面没有一行说明(对比lib/same-file.test.ts:93、findings.test.ts:1459那种写清了 inode 退化原因的写法)。我逐个查了每个ctx.skip()前的条件,全部是 win32 或 inode 可验证性门(没有无条件跳过),因此 Linux 侧覆盖不损失,这里只差注释。补一行「为什么跳过」即可,读代码的人需要能区分「平台工件」与「真有生产缺口」。
isSameFile and the deletion-journal swap check fail open on Windows
#11848
The Lint & Static lane's step 8 (Check lint gate freshness) failed on 8e1a0b9 in 23s: .github/workflows/ci.yml changed on main at c666ec1 (fix(ci): restore Windows test baseline, #11787) after this branch last incorporated main at 351b5fc, and the compare reported diverged. The lane checks out the branch head alone, so its green would have validated a stale gate. No product change. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Resolve overlaps with #11787's Windows test-baseline restore: - monitor-debug-store.ts: take main's win32 gate (skips POSIX mode+uid checks on Windows). Semantically equivalent to this PR's mode-only gate, and already verified on the Windows CI lane. - memory/config.test.ts: take main's tmpdir-based fixtures, which subsume this PR's join->resolve assertion tweak for the same Windows path failure. File is now identical to main. - language-preferences/service/monitor-debug-store tests: keep this PR's dedicated it.skipIf(win32) permission tests, which cover the same 0600/0700 assertions that #11787 gated inline, and report the skip instead of passing a test that asserted nothing. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…QwenLM#11853) Follow-up to QwenLM#11787, carrying review finding R1-26 from that PR. It charged that several Windows skips turned a *measured* failure into a green skip with no reason recorded at the code site, and that in two cases the red had been witnessing a real production gap rather than a portability artifact. It split the remedy in two and asked that they not collapse into one generic "add a comment". Both halves are here; the production conversion itself is not, and is tracked in QwenLM#11848. (a) Fixture artifact — witness restored rather than annotated. `cleanup.test.ts`'s "reads process.cwd() ONCE per run" was skipped on Windows because the count came back 16 against an expected 1. The cause is that file's module-level `node:path` posix mock: a real Windows cwd (`C:\…`) is not posix-absolute, so every downstream `resolve()` re-reads `process.cwd()`. Production uses win32 semantics there and has exactly one live `process.cwd()` (`cleanup.ts:754`) with no platform branch, so the invariant was never broken — only the measurement was. Pinning the spy to the fixture root (`/repo`, the spelling every other fixture in that file keys on) makes the count platform-independent, so the skip is deleted and the witness now runs on every lane. Mutation-checked: adding one downstream `process.cwd()` read reds it with "expected 1 times, but got 2 times". (b) Real gaps — the gates now say so, and one issue owns them. `findings.test.ts`, `repo-context.test.ts`, `same-file.test.ts` and `standalone-deletion-journal.test.ts` each carry a comment stating that on a volume whose ids exceed the safe-integer range the guard under test is INERT, not merely untestable, name the production call site responsible, and point at QwenLM#11848. `same-file.test.ts` previously conflated the two cases its single gate covers — `ino === 0` (FAT/exFAT/SMB) degrades to canonical spellings BY DESIGN and is pinned by the test immediately below it, while an NTFS 64-bit file index degrades only because `tryStat` asks for a number-backed `Stats`. Those need different responses and now read differently. QwenLM#11848 carries the shared root cause, the two comparators to convert (`lib/same-file.ts:24`, `standalone-deletion-journal.ts:678`), and two constraints: do not widen `hasVerifiableInode`'s signature, because core's looser predicate is deliberate and tightening it would flip `assertVerifiableTranscriptIdentity` on >2^53 Windows transcript inodes; and `refuses inode identity above the safe-integer range` must be rewritten as a genuine BigIntStats fake rather than deleted, since its mock assigns `stats.ino = 2 ** 60` as a *number* to two distinct files. Comments plus one spy change; no production code touched. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Released in v0.23.4. |


What this PR does
Restores the Windows CI baseline by making runtime paths and filesystem identity checks portable, keeping POSIX-only permission and filesystem semantics out of Windows assertions, and installing the tokenizer's supported WASI fallback when the native Windows binding cannot load.
It also configures the isolated test home consistently for Git line endings and updates Windows-only fixtures to use standard file-URL conversion and deterministic missing-environment values while preserving the behavior each test is intended to cover.
Why it's needed
The self-hosted Windows lane completed setup and dependency installation but failed across multiple workspaces. The failures came from a small set of shared portability assumptions: raw drive paths passed to Node's ESM loader, omitted
TEMPvalues that Windows libuv restores from the parent environment, lossy numeric inode comparisons on NTFS, POSIX-only path, permission, symlink, and deleted-working-directory fixtures, and an unavailable native tokenizer DLL with no installed fallback. This left the scheduled Windows baseline continuously red and unable to provide useful regression signal.Reviewer Test Plan
How to verify
Dispatch the Windows self-hosted runner validation workflow for this branch and confirm that dependency installation succeeds, the complete workspace and root-script test command runs to completion, and the former code-mode, Live tokenizer, filesystem identity, path, UI shortcut, and fixture failures are absent. On a non-Windows host, confirm that the focused filesystem, code-mode, Live, ACP bridge, CLI, and VS Code companion suites continue to pass.
Evidence (Before & After)
Before: the scheduled Windows job failed after setup with failures across ACP Bridge, CLI, Core, Qwen Live, and the VS Code companion: https://github.com/QwenLM/qwen-code/actions/runs/34713863264
After: the manually dispatched Windows validation passed at
d7b8b97c68f8in 23m16s. Setup, dependency installation, the completenpm run test:cicommand, report generation, and cleanup all succeeded; the root-script segment alone completed 75 files and 1,522 tests with no failures: https://github.com/QwenLM/qwen-code/actions/runs/34806714807Tested on
Environment (optional)
macOS local development checkout with Node.js 22; Windows x64 self-hosted ECS runner through the repository validation workflow; Ubuntu hosted runner through the normal PR checks.
Risk & Scope
Linked Issues
No issue is closed by this PR. One is filed out of it: #11848 tracks two pre-existing
production gaps that this PR's Windows skips removed the only CI signal for
(
isSameFilefailing open on a hard link, and the deletion journal's swap detectiongoing inert on 64-bit NTFS file ids). Both are unchanged code, so they are tracked
rather than fixed here; the gates now say so at the code site.
中文说明
本 PR 做了什么
恢复 Windows CI 基线:让运行时路径和文件系统身份校验具备跨平台兼容性,不再在 Windows 上断言仅适用于 POSIX 的权限与文件系统语义,并在 Windows 原生分词器无法加载时安装其官方支持的 WASI 回退实现。
同时统一隔离测试 HOME 下的 Git 换行符配置,并将仅在 Windows 失败的测试夹具改为使用标准 file URL 转换和确定性的环境变量缺失值,同时保留每个测试原本要覆盖的行为。
为什么需要
自托管 Windows 流水线能够完成环境准备和依赖安装,但随后在多个 workspace 中失败。根因集中在少量共享的跨平台假设:把盘符路径直接传给 Node ESM loader、假定省略的
TEMP不会被 Windows libuv 从父环境补回、在 NTFS 上用有精度损失的 number 比较 inode、依赖仅适用于 POSIX 的路径、权限、符号链接和已删除工作目录夹具,以及原生分词器 DLL 不可用且未安装回退实现。这使定时 Windows 基线持续失败,无法提供有效的回归信号。Reviewer 测试计划
如何验证
为本分支触发 Windows 自托管 runner 验证工作流,确认依赖安装成功、完整 workspace 与根级脚本测试运行结束,并且此前的 code mode、Live 分词器、文件身份、路径、UI 快捷键和测试夹具失败均不再出现。在非 Windows 主机上,确认文件系统、code mode、Live、ACP Bridge、CLI 和 VS Code companion 的相关测试继续通过。
证据(修复前后)
修复前:定时 Windows 任务在环境准备后失败,ACP Bridge、CLI、Core、Qwen Live 和 VS Code companion 均有失败:https://github.com/QwenLM/qwen-code/actions/runs/34713863264
修复后:手动触发的 Windows 验证在提交
d7b8b97c68f8上以 23m16s 全部通过。环境准备、依赖安装、完整npm run test:ci、报告生成和清理均成功;其中根级脚本阶段共运行 75 个测试文件、1,522 个用例且无失败:https://github.com/QwenLM/qwen-code/actions/runs/34806714807已测试平台
环境(可选)
macOS 本地开发检出,Node.js 22;Windows x64 自托管 ECS runner,通过仓库验证工作流运行;Ubuntu 托管 runner,通过常规 PR checks 运行。
风险与范围
关联 Issue
本 PR 不关闭任何 issue,但从评审中提交了一个:#11848 跟踪两个既有的生产缺陷——本 PR 的 Windows 跳过移除了它们唯一的 CI 信号(
isSameFile对硬链接向失败方向敞开,以及删除日志的替换检测在 64 位 NTFS 文件 id 上失效)。两者都是未改动的既有代码,因此只做跟踪、不在本 PR 内修复;门禁处已就地写明。