fix: repair the Windows test lane's standing failures - #10064
Merged
Conversation
Fixes the eight failure clusters that keep Test (windows-latest) red on
the ECS nightly lane — three product bugs and five test bugs.
Product:
- extension-git-client: GIT_CONFIG_GLOBAL was os.devNull, whose win32
spelling ('\\.\nul') Git for Windows rejects with "Invalid argument",
failing every restricted-env git command on Windows. Use the literal
'/dev/null', which git's compat layer maps on every platform.
- sessionArtifacts: the pre-open/post-open identity check compared
numeric dev/ino. NTFS file ids are 64-bit and lose precision above
2^53 as Numbers, so two files created close together can round to the
same ino and defeat the swap (TOCTOU) detection. Compare bigint stats.
- dws-event-stream: clearing a stale stderr error on a healthy stdout
line was gated on the child still being alive, but on Windows the pipe
routinely drains only after exitCode is set, so the final healthy line
never cleared it and close reported the stale error. A clean (code 0)
exit now certifies recorded errors as stale at close.
Tests:
- daemon-git-worktree-guard: fixtures pasted native Windows paths into
command strings unquoted; on the bash lane the tokenizer consumes the
backslashes (C:\t\repo -> C:trepo), so resolvable targets tested as
unresolvable ones and cd shifts silently failed in place. Interpolate
through cmdPath(), which spells them with forward slashes on win32 —
what bash preserves and Windows resolves. Assertions keep the native
spelling; the guard canonicalizes targets through realpath.
- review/lib/worktree: git marks a linked worktree's .git gitfile hidden
on Windows and CREATE_ALWAYS on a hidden file fails with EPERM;
rewrite gitfiles via unlink-then-write.
- config: compare getCwd() against path.resolve(targetDir) — the
constructor resolves it, so the POSIX fixture comes back
drive-qualified on win32.
- SessionMessageHandler: complete the pathToFileURL shim's mirror case —
parse POSIX-spelled fixture paths with { windows: false } on win32 so
expected file:// URLs read the same on every host.
- node-repl.semantics: dispose the kernel before removing its cwd (a
process's cwd cannot be removed on Windows) and retry the removal
across the child's asynchronous teardown.
wenshao
requested review from
LaZzyMan,
doudouOUC,
qqqys,
tanzhenxin and
yiliang114
as code owners
August 26, 2026 02:16
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template: the body doesn't use the template's literal headings ( Problem: observed, with hard evidence. I verified the cited runs through the API: the 2026-08-25 nightly (run 32889631784) failed as described, and Direction: clearly aligned — restoring a CI lane that has been red for a long time, and three of the eight clusters are real cross-platform product bugs (restricted-env extension installs broken on Windows, a defeatable TOCTOU identity check on NTFS, a stale-error race in the DWS event stream), not test cosmetics. No CHANGELOG signal applies to test-lane/platform fixes. Size: core paths touched: 35 production lines ( Approach: the scope feels exactly right — one minimal fix per failure cluster, nothing to cut. Cutting 80% would leave the lane red for the cut clusters, and the hosted-pool surface is explicitly deferred rather than bolted on. Every hunk maps to a named cluster. Risk: no high-risk paths matched the revert-correlation list; no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ 模板：正文没有使用模板的字面标题（ 问题：已观测到，且有硬证据。我通过 API 核实了引用的 run：2026-08-25 nightly（run 32889631784）确实失败， 方向：明确对齐——修复一条长期全红的 CI lane，且 8 个失败簇中有 3 个是真实的跨平台产品 bug（Windows 上受限环境扩展安装必挂、NTFS 上可被击穿的 TOCTOU 身份检查、DWS 事件流的过期错误竞态），不是测试表面工作。测试设施/平台类修复不适用 CHANGELOG 信号。 规模：触及核心路径的生产代码 35 行（ 方案：范围恰到好处——每个失败簇一处最小修复，没有可砍的部分。砍掉 80% 只会让被砍的簇继续红，托管池的问题面被明确推迟而非顺手塞进来。每个 hunk 都能对应到一个已命名的失败簇。 风险：未命中 revert 相关性高风险路径列表；无升级风险信号。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewNo blockers found. My independent proposal for "repair the standing Windows-lane failures" is exactly what this diff does — one minimal fix per cluster — and the implementation holds up under checking:
Comments throughout explain the why (NTFS 64-bit file ids, bash backslash-escape semantics, Windows pipe drain ordering) — exactly the kind of context this code needs. Testing evidenceThis is an unattended CI run — no local builds or test executions were performed; the evidence below is the PR's own CI, read through the GitHub API on the reviewed commit. The Windows lane does not run on the pull-request trigger (that's the standing arrangement this PR exists to eventually reverse), so the direct Windows evidence is the Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Several checks are still running (the ubuntu/macos PR lanes, Serve A/B, SDK Java, secret scan); the table above records them as pending and the finalize job will update it once CI settles. Nothing red so far on anything this PR plausibly causes. Not verified independently: no maintainer-triggered sandboxed run ( 中文说明代码审查未发现阻塞问题。我对"修复 Windows lane 存量失败"的独立方案正是本 diff 所做的——每个失败簇一处最小修复——且实现经得起核对：
各处注释解释的都是为什么（NTFS 64 位文件 id、bash 反斜杠转义语义、Windows 管道排空时序）——正是这些代码所需要的上下文。 测试证据本次为无人值守 CI 运行——未做任何本地构建或测试执行；以下证据是 PR 自身的 CI，通过 GitHub API 在被审提交上读取。 Windows lane 不在 pull request 触发上运行（这正是本 PR 要最终扭转的存量安排），因此 Windows 的直接证据是在本 head SHA 上以 （CI 表格见英文部分机器可读区域，finalize 任务会在 CI 落定后更新。） 部分检查仍在运行（ubuntu/macos PR lane、Serve A/B、SDK Java、密钥扫描）；表格如实记为 pending。目前没有任何可由本 PR 合理导致的红色。 未独立核实：未请求维护者触发的沙箱运行（ — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; would merge without hesitation once CI lands. Stepping back: this is what a well-run repair pass looks like. The motivation is unambiguous — I verified both the red baseline run and the green run on this SHA through the API, so the before/after isn't a claim, it's a measurement. My independent proposal for the stated goal was one minimal fix per cluster, and the diff matches it hunk for hunk; I didn't find a simpler path it missed, and nothing in it goes beyond the stated scope (the hosted-pool surface is sensibly deferred, not smuggled in). The three product fixes are the kind of cross-platform bugs that age badly if left unfixed — a git env value that rejects every restricted-env extension install on Windows, a TOCTOU identity check that NTFS file ids can defeat, and a pipe-ordering race that reports stale errors — and each gets the exact right fix rather than a workaround. In six months the comments (NTFS 64-bit file ids, bash backslash semantics, Windows drain ordering) will save whoever touches this code an afternoon of rediscovery. Every change is necessary; nothing to curse the author for. CI on the reviewed commit still has lanes in flight (ubuntu/macos PR test lanes, Serve A/B, SDK Java, secret scan), so approval is deferred until CI lands green on 中文说明置信度：5/5 —— 每个阶段都干净；CI 一落定即可毫不犹豫合入。 退一步看：这是一次执行得当的修复。动机毫不含糊——红色基线 run 和本 SHA 上的绿色 run 我都通过 API 核实过，所以 before/after 不是声明而是测量值。我对既定目标的独立方案就是每个失败簇一处最小修复，diff 与之逐 hunk 吻合；没找到它错过的更简路径，也没有任何超出既定范围的内容（托管池问题面被合理推迟，而非夹带进来）。 三个产品修复属于那种拖着不修会越变越糟的跨平台 bug——一个让 Windows 上所有受限环境扩展安装必挂的 git 环境变量、一个能被 NTFS 文件 id 击穿的 TOCTOU 身份检查、一个上报过期错误的管道时序竞态——每一个都得到了恰如其分的修复而非绕过。六个月后，这些注释（NTFS 64 位文件 id、bash 反斜杠语义、Windows 排空时序）能为接手的人省下一个下午的重新摸索。每处改动都是必要的；没有任何值得埋怨作者的地方。 被审提交上的 CI 还有 lane 在跑（ubuntu/macos PR 测试 lane、Serve A/B、SDK Java、密钥扫描），因此批准推迟到 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
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. |
Collaborator
🩺 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 |
qwen-code-ci-bot
previously approved these changes
Aug 26, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Collaborator
Author
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
Collaborator
|
🤝 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 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 6b": run worktree.test.ts on the Linux lane; "agent 6b": run config.test.ts derived-ownership test on the Linux lane; "agent 6b": run node-repl.semantics.test.ts symlink-cwd test on the Linux lane; "agent 6b": run SessionMessageHandler.test.ts on the Linux lane.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 6b"：run worktree.test.ts on the Linux lane；"agent 6b"：run config.test.ts derived-ownership test on the Linux lane；"agent 6b"：run node-repl.semantics.test.ts symlink-cwd test on the Linux lane；"agent 6b"：run SessionMessageHandler.test.ts on the Linux lane。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (!isSameFile(preOpenStat, await handle.stat({ bigint: true }))) { | ||
| return { status: 'missing', escaped: true }; | ||
| } | ||
| const stat = await handle.stat(); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The bigint identity check introduces a second handle.stat() — one { bigint: true } fstat for isSameFile, then a Number-precision fstat for isFile()/size/mtimeMs — where this path issued one stat before. Every getWorkspaceStatus refresh now pays one extra fstat round-trip plus a thread-pool await; on the NTFS/Windows stack this PR targets, that is the comparatively expensive direction. There is no correctness impact (both stats read the same pinned open file description), and the second stat turns out to be load-bearing: a { bigint: true } stat truncates mtimeMs to an integer (probe: …438n vs the Number stat's …438.456), so folding the two together would break the strict-equality expectedMtimeMs === stat.mtimeMs fast path for any fractional-mtime file and could report spurious changed for files over 100 MiB. Please add a one-line comment at the second stat documenting why it cannot be folded into the bigint one, so the next reader doesn't "simplify" it away.
中文说明
bigint 身份检查引入了第二次 handle.stat()——先用 { bigint: true } fstat 做 isSameFile,再用 Number 精度 fstat 取 isFile()/size/mtimeMs——而这条路径之前只做一次 stat。现在每次 getWorkspaceStatus 刷新都多付一次 fstat 往返加一次线程池 await;在本 PR 针对的 NTFS/Windows 栈上,这正是相对昂贵的方向。无正确性影响(两次 stat 读的是同一个已打开的文件描述),而且第二次 stat 其实是承重结构:{ bigint: true } 的 mtimeMs 会截断为整数(探针实测:…438n 对 Number stat 的 …438.456),把两者合并会破坏任何分数 mtime 文件的 expectedMtimeMs === stat.mtimeMs 严格相等快速路径,并可能让超过 100 MiB 的文件误报 changed。请在第二次 stat 处加一行注释说明它为何不能并入 bigint 那次,免得下个读者把它"简化"掉。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // so its liveness gate (which protects errors from being cleared | ||
| // by lines buffered across a crash) also swallows that clear. | ||
| reportError( | ||
| code === 0 ? processError(code) : (lastError ?? processError(code)), |
Collaborator
There was a problem hiding this comment.
[Suggestion] The new branch — discarding a recorded lastError on a clean (code 0) exit — has no test that deterministically exercises it. The only exit-0 fixture (dws-event-recovered-source.mjs) clears lastError via a healthy stdout line before close, so on the Linux lane old and new code are observably identical; the fixtures that keep lastError both exit 1 and exercise the untouched fallback. Verified by mutation probe: reverting this line to lastError ?? processError(code) passes the whole suite 6/6 across five runs, while a ready → stderr error → exit 0 discriminator fixture reports 'DWS event consumer stopped (0).' on this code and the stale stderr error on the revert. Add that fixture (stderr error JSON after the last stdout line, then exit 0 with no further stdout) plus a test asserting onError receives the code-0 processError — otherwise a future revert reintroduces the exact spurious stale-error report on Windows that this PR fixes, with the whole suite still green.
中文说明
新分支——clean exit(code 0)时丢弃已记录的 lastError——没有任何测试能确定性地触发它。唯一的 exit-0 夹具(dws-event-recovered-source.mjs)在 close 之前就通过健康 stdout 行清掉了 lastError,所以 Linux lane 上新旧代码行为完全相同;保留 lastError 的夹具都 exit 1,走的是未改动的回退分支。变异探针验证:把这行还原为 lastError ?? processError(code) 后,整个套件连续 5 次运行全部 6/6 通过;而 ready → stderr 错误 → exit 0 的判别夹具在本代码上报 'DWS event consumer stopped (0).',在还原版上报 stale stderr 错误。建议补上该夹具(最后一条 stdout 之后写一条 stderr 错误 JSON,然后 exit 0 且不再有 stdout)并断言 onError 收到 code-0 的 processError——否则未来一次还原就会在全套件全绿的情况下,把本 PR 修掉的 Windows 误报 stale 错误原样带回来。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| } | ||
|
|
||
| function isSameFile(before: Stats, after: Stats): boolean { | ||
| function isSameFile(before: BigIntStats, after: BigIntStats): boolean { |
Collaborator
There was a problem hiding this comment.
[Suggestion] The bigint comparison — the heart of this fix — has no test exercising file IDs ≥ 2^53. No fixture injects values above 2^53 and real Linux test inodes are small, so Number and BigInt equality agree there. Verified by mutation: reverting both identity stat calls to numeric passes sessionArtifacts.test.ts 126/126, and implementing the injected-stats test (lstat spy returns ino: 2n**53n, the opened handle's stat returns ino: 2n**53n + 1n, assert { status: 'missing' }) fails under that mutant with observed status 'available' — the exact TOCTOU defeat — while passing under this code. Add such a test so a numeric revert cannot ship green: as it stands, the swap guarantee again depends on whether NTFS happens to assign IDs that survive Number rounding (Number(2**53 + 1) === Number(2**53)).
中文说明
bigint 比对——本修复的核心——没有任何测试覆盖 ≥ 2^53 的文件 id。没有夹具注入超过 2^53 的值,Linux 测试的真实 inode 又很小,Number 与 BigInt 的相等性在那里结果一致。变异验证:把两处身份 stat 还原为 Number 后,sessionArtifacts.test.ts 126/126 全过;而实现注入 stats 的测试(lstat spy 返回 ino: 2n**53n,打开句柄的 stat 返回 ino: 2n**53n + 1n,断言 { status: 'missing' })在该变异体下以观察到的状态 'available' 失败——正是 TOCTOU 被击穿——在本代码下则通过。建议补上该测试,否则 Number 还原可以全绿合入:现状下,swap 保证又回到依赖 NTFS 恰好分配出 Number 舍入下不冲突的 id(Number(2**53 + 1) === Number(2**53))。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| () => `X='git reset --hard'; cd ${cmdPath(outsideRepo)}; $X`, | ||
| () => `X=git; Y='-C ${outsideRepo} reset --hard'; $X $Y`, | ||
| () => | ||
| `eval 'GIT_WORK_TREE=${outsideRepo}'; export GIT_WORK_TREE; git reset --hard`, |
Collaborator
There was a problem hiding this comment.
[Suggestion] The cmdPath conversion this PR introduces is applied to entry 1 of this list but not to entries 3-4 (this eval'd GIT_WORK_TREE assignment and the bare assignment below it) or the round-7 bare-assignment payloads — all unquoted/eval'd positions where bash escape-processes backslashes. The only Windows lane is win32+bash (workflow-level defaults.run.shell: bash → Git-Bash sets MSYSTEM), and these tests are ungated, so on that lane the raw paths mangle before any relocation exists — probe-verified on bash 5.2: C:\temp\outside\repo arrives as C:tempoutsiderepo in unquoted, bare-assignment and eval'd forms alike. The allowed: false assertions then stay green via the fail-closed unresolvable-target denial (the resolvable spelling yields the outside-session denial these tests name) instead of exercising env-var relocation resolution — so a guard regression in that resolution ships green on the very lane cmdPath was written for. Route entries 3-4 and the round-7 payloads through cmdPath(...) too; slashes survive assignment/eval re-parse and resolve identically on Windows. (The relink list was checked and needs no change — its denial fires on the .git destination regardless of source resolvability.)
中文说明
本 PR 引入的 cmdPath 转换应用到了该列表的第 1 项,但没有应用到第 3-4 项(这个被 eval 的 GIT_WORK_TREE 赋值及其下面的裸赋值)和 round-7 的裸赋值载荷——它们都是 bash 会把反斜杠当转义处理的未加引号/eval 位置。唯一的 Windows lane 是 win32+bash(工作流级 defaults.run.shell: bash → Git-Bash 设置 MSYSTEM),这些用例未做平台门控,所以在该 lane 上,原生路径在任何重定向存在之前就被破坏——已在 bash 5.2 上实测:C:\temp\outside\repo 在未加引号、裸赋值和 eval 形式下都变成 C:tempoutsiderepo。随后 allowed: false 断言靠失败关闭的"不可解析目标"拒绝继续通过(可解析拼写才会得到这些用例所称的越界拒绝),而不再验证环境变量重定向解析——因此该解析上的 guard 回归会在 cmdPath 正是为其而写的 lane 上全绿合入。请把第 3-4 项和 round-7 载荷也过一遍 cmdPath(...);正斜杠在赋值/eval 重解析中保留,且在 Windows 上解析结果相同。(relink 列表已核查,无需改动——其拒绝由 .git 目标触发,与源是否可解析无关。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
Collaborator
- Document why the workspace-status identity fstat cannot be folded into the Number stat (bigint stats truncate mtimeMs and return BigInt fields). - Add a clean-exit fixture + test proving the dws consumer reports the code-0 process error instead of a stale stderr error. - Add an injected-stats test detecting swaps whose file ids differ only above 2^53, so a numeric identity revert cannot ship green. - Route the remaining unquoted/eval'd env-var assignment payloads in the worktree guard tests through cmdPath so they stay resolvable on the win32+bash lane.
Collaborator
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #10064 review feedbackThe automated reviewer posted 4 Suggestion findings (round 1, COMMENTED review). All 4 were verified against the code first, then implemented — each was valuable, in scope, and consistent with this PR's purpose. No Critical/Required items, no conflicts ( Dispositions
Also reviewed, no action needed: review body [rv:5028142425] (disclosed review gaps only, no actionable request beyond the inline suggestions) and issue comment [ic:5419952874] (serve daemon A/B: no response changes against the PR base). Files changed
Commit: Verification
中文说明本轮总结 — PR #10064 审查反馈自动审查者提交了 4 条 Suggestion 发现（第 1 轮，COMMENTED 审查）。全部 4 条均先对照代码核实，随后实现——每一条都有价值、在范围内、且与本 PR 的目标一致。无 Critical/Required 项，无冲突（ 处理结论
另已审阅、无需处理：审查正文 [rv:5028142425]（仅披露审查缺口，除行内建议外无可执行请求）和 issue 评论 [ic:5419952874]（serve daemon A/B：相对 PR base 无响应变化）。 变更文件
提交： 验证
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
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/extension/extension-git-client.ts:132 — [probe] GIT_CONFIG_GLOBAL: '/dev/null' fix is pinned by no test — expect.any(String) lets an os.devNull regression pass on every platformpackages/cli/src/serve/daemon-git-worktree-guard.test.ts:2463 (+2 locations) — [probe] cmdPath rollout left two round-7 payloads unconverted (pipe cd and live GIT_DIR substitution) — they stop exercising the resolved-outside arm on the win3…
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Comment on lines
+3296
to
+3299
| const injectIno = (stat: Stats | BigIntStats, ino: bigint): void => { | ||
| stat.ino = typeof stat.ino === 'bigint' ? ino : Number(ino); | ||
| }; | ||
| const originalLstat = fs.lstat.bind(fs); |
Collaborator
There was a problem hiding this comment.
[Suggestion] This new bigint-identity test pastes the same ~12-line options-forwarding fs.lstat spy wrapper that this PR also added to the adjacent swap test — both bind the original, forward entry + options, and cast as typeof fs.lstat; they differ only in the hook body. Only the swap-test copy carries the rationale comment (a spy that dropped { bigint: true } would hand back numeric stats that never === the bigint fstat, hiding a broken identity comparison behind a type mismatch), and the two copies must now be maintained in lockstep: if a later edit stops one copy forwarding options (or changes its cast), the other keeps the old shape and the bigint path these tests exist to cover silently stops being exercised — with both tests still green. Extract one local helper that owns the bind/forward/cast mechanics and the explanatory comment, letting each test supply only its hook body, e.g.:
function spyOnLstatForwarding(
hook: (
entry: Parameters<typeof fs.lstat>[0],
stat: Stats | BigIntStats,
) => void | Promise<void>,
) {
const originalLstat = fs.lstat.bind(fs);
return vi.spyOn(fs, 'lstat').mockImplementation((async (
entry: Parameters<typeof fs.lstat>[0],
options?: Parameters<typeof fs.lstat>[1],
) => {
const stat = await originalLstat(
entry,
options as Parameters<typeof originalLstat>[1],
);
await hook(entry, stat);
return stat;
}) as typeof fs.lstat);
}中文说明
新增的 bigint 身份测试原样粘贴了本 PR 同时加进相邻 swap 测试的约 12 行「透传 options 的 fs.lstat spy 包装器」——两者都绑定原函数、透传 entry + options 并作 as typeof fs.lstat 转型,仅钩子体不同。只有 swap 测试那份带着理由注释(不透传 { bigint: true } 的 spy 会返回永远不等于 bigint fstat 的 Number stats,用类型错配掩盖坏掉的身份比对),两份现在必须同步维护:一旦后续编辑改掉其中一份(比如不再透传 options 或改了转型),另一份保持旧形状,这两个测试所要覆盖的 bigint 比对路径就会在全绿的情况下悄悄失效。建议抽一个本地 helper 统管绑定/透传/转型机制和那条解释注释,每个测试只提供钩子体,示例见上。
— qwen3.8-max via Qwen Code /review (v0.22.0)
doudouOUC
reviewed
Aug 26, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Second opinion — clean, no new findings
Reviewed the full diff at head 040352f9 alongside the round 1 (SHA 4e4660c0) and round 2 reviews. The PR is unchanged since round 2 — no new commits.
Existing findings
All 5 round-1/round-2 Suggestions (R1-1 through R1-4, R2-1) and the 2 deferred probes are still standing at HEAD. All are low-severity and non-blocking — the triage correctly assessed them as Suggestions.
My analysis
Read every hunk across all 8 files (657 diff lines). Summary per cluster:
-
sessionArtifacts (product bug): bigint identity check is correct. The two
handle.stat()calls are intentional — the bigint one checks identity, the second provides Number fields for downstream size/mtime. The test spy now forwardslstatoptions, keeping the test honest. The new bigint-identity test injects inos at2^53and2^53+1to verify the guard catches swaps that Number precision would mask. -
dws-event-stream (product bug): the
code === 0 ? processError(code) : lastErrorbranch correctly certifies stale errors as stale on a clean exit. The crash path and stdout liveness gate are untouched. The test fixture writes stderr before exiting 0 — exactly the race condition on Windows pipe drain ordering. -
extension-git-client (product bug):
os.devNull→'/dev/null'is correct — Git's compat layer handles the POSIX spelling on every platform. Theosimport removal is clean (no other references). Already consistent with the workflow test scripts that use'/dev/null'forGIT_CONFIG_GLOBAL. -
daemon-git-worktree-guard (test bug): 39 fixtures converted via
cmdPath(). Verified every hunk: only unquoted command-string interpolations are changed; single-quoted interpolations, spoofed cmd/powershell zones, and the win32-transform pinning zone are untouched. Denial-reason assertions keep native spelling since the guard realpath-canonicalizes. -
worktree, config, SessionMessageHandler, node-repl (test bugs): each fix is minimal and correct —
overwriteGitfilesidesteps hidden-file CREATE_ALWAYS EPERM,path.resolvematches the constructor's behavior, the{ windows: false }shim branch mirrors the existingC:\branch, anddispose-before-rm handles Windows EBUSY correctly.
New findings
None. Every hunk is necessary, minimal, and correctly implemented. The comments (NTFS 64-bit file ids, bash backslash-escape semantics, Windows pipe drain ordering) are exactly the context this code needs.
Verification note
git fetch is blocked on this Windows machine (known getaddrinfo issue), so I could not build, typecheck, or run tests. The PR's own CI evidence — Test (windows-latest, Node 22.x) green on the ECS pool (run 32920750212) — is the authoritative before/after for all eight clusters.
— qwen3.8-max via Qwen Code /review
Collaborator
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #10064Feedback dispositions[rc:3862810371] [Suggestion] Duplicated options-forwarding
[rv:5030490867] Review body ("Partially reviewed — gaps disclosed") — No action required this round.
No conflict resolution was requested ( Changes
VerificationCommands actually run, in order:
中文说明Autofix 轮次总结 — PR #10064反馈处理[rc:3862810371] [Suggestion]
[rv:5030490867] 审查主体(「部分审查——缺口已披露」)——本轮无需处理。
未要求解决冲突( 变更内容
验证实际执行的命令(按顺序):
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
approved these changes
Aug 26, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary — PR #10064 (no action)Feedback triaged
Context
ResultNo changes were made this round; the branch remains at commit 中文说明Autofix 轮次总结 — PR #10064（无操作）反馈分诊
背景
结果本轮未做任何改动；分支仍停留在提交 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
approved these changes
Aug 26, 2026
This was referenced Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the eight failure clusters that keep
Test (windows-latest, Node 22.x)red — the standing failures that took the macOS/Windows lanes off pull requests in #10059. Scope: everything that fails on the ECS pool (the nightly lane, run 32889631784: 54 tests across 8 files), which is the lane #10059 leaves live. Three are product bugs; five are test bugs.Product bugs
extension-git-client(5 tests): every restricted-env git command fails on Windows. The scrubbed child environment setsGIT_CONFIG_GLOBAL: os.devNull, whose win32 value is\\.\nul— Git for Windows rejects that spelling withfatal: unable to access '\\.\nul': Invalid argument. This is not test-only: any credentialed ornetworkPolicy: 'public'extension install on a real Windows machine hits it. Fixed to the literal/dev/null, which git's compat layer maps on every platform.sessionArtifacts(1 test): the swap (TOCTOU) guard can be defeated on NTFS. The pre-open/post-open identity check compared numericdev/ino. NTFS file ids are 64-bit (sequence << 48 | record), and Numbers lose precision above 2^53 — with any sequence ≥ 32, adjacent MFT records round to the same numeric ino, so a file swapped betweenlstatandopenpassed the identity check and reportedchangedinstead ofmissing. The check now compares{ bigint: true }stats. The test'slstatspy also forwards options now — a spy that droppedbigintwould make the comparison always mismatch and hide a broken guard behind a type mismatch.dws-event-stream(1 test): stale-error clearing raced the pipe on Windows. A healthy stdout line clears a previously recorded stderr error only while the child is still alive (exitCode === null) — the gate that protects post-mortem lines from clearing the error that explains a crash. On Windows the final line routinely drains afterexitCodeis set, so the clear never ran and close reported the stale error. A clean (code 0) exit now certifies recorded errors as stale at close; the liveness gate still protects the crash path, and the fixture suite's exit-code convention (error sources exit 1, the recovered source exits 0) pins both directions.Test bugs
daemon-git-worktree-guard(39 tests). Fixtures pasted native Windows paths into command strings unquoted. The guard models the executing shell faithfully, and the lane underdefaults.run.shell: bashsetsMSYSTEM, sogetShellConfiguration()is the bash lane — where an unquoted backslash is a POSIX escape:git -C C:\t\reporeaches the tokenizer asC:trepo. Resolvable targets tested as unresolvable ones,cdshifts silently failed in place, and both deny-reasons and allow-expectations went wrong. The failing fixtures now interpolate throughcmdPath(), which spells paths with forward slashes on win32 — what bash preserves and Windows resolves identically. Assertions keep the native spelling: the guard canonicalizes targets throughrealpathbefore printing them. Quoted interpolations, the win32-transform pinning zone, and the spoofed cmd/powershell zones are untouched.review/lib/worktree(4 tests). git marks a linked worktree's.gitgitfile hidden on Windows, and opening a hidden file with truncate (writeFileSync's CREATE_ALWAYS) fails with EPERM. Gitfile rewrites go through an unlink-then-write helper.config(1 test).getCwd()returns the constructor-resolvedtargetDir, so the POSIX fixture/path/to/targetcomes back drive-qualified on win32. Compare againstpath.resolve(targetDir)— the form the file's other assertions already use.SessionMessageHandler(2 tests). The test'spathToFileURLshim already forced windows-mode parsing forC:\fixtures on POSIX but had no mirror: on win32 a POSIX-spelled fixture (/workspace/export.html) got drive-qualified against the process cwd. The shim now parses those with{ windows: false }, so expectedfile://URLs read the same on every host.node-repl.semantics(1 test). The test removed the kernel child's cwd while the child was alive — a directory that is any process's cwd cannot be removed on Windows (EBUSY). Dispose the kernel first and retry the removal across its asynchronous teardown.Verification
workflow_dispatchof ci.yml on this branch — run 32920750212 — ran the fulltest:cionecs-winwith the production env (bash shell, HOME redirect, scrubbed keys):Test (windows-latest, Node 22.x)succeeded, the first green run of that lane on this pool (the 2026-08-25 nightly and every measured PR run were red).cmdPath()and the shim's mirror branch are identity off-win32, so the Linux semantics of the guard fixtures are unchanged by construction.--max-warnings 0), prettier, and per-packagetsc --noEmitclean on every touched package (remaining cli/vscode tsc noise is pre-existing stale-dist drift in files this PR does not touch).Out of scope
The hosted
windows-2022image adds a separate, mostly environmental failure surface (system-gitconfigcore.autocrlf=trueleaking into test-created repos,RUNNER~18.3 short names in TEMP, daemon-start ECONNREFUSED cascades — ~25 more files). After #10059 that pool is only the kill-switch fallback; it needs its own pass — hermetic git config in test fixtures first — before the pull-request trigger (which runs hosted for security) can come back.中文说明
修复让
Test (windows-latest, Node 22.x)常年全红的八个失败簇——也就是 #10059 把 macOS/Windows lane 摘出 pull request 的那批存量失败。范围:ECS 池(nightly lane,run 32889631784:8 个文件 54 条用例)上能复现的全部失败,这正是 #10059 保留的那条 lane。其中 3 个产品 bug、5 个测试 bug。产品 bug
extension-git-client(5 条):Windows 上所有受限环境 git 命令必挂。 洗净的子进程环境设了GIT_CONFIG_GLOBAL: os.devNull,win32 下取值\\.\nul,Git for Windows 直接报fatal: unable to access '\\.\nul': Invalid argument。不是测试专属:真实 Windows 机器上任何带凭证或networkPolicy: 'public'的扩展安装都会踩中。改为字面量/dev/null,git 的兼容层在所有平台都认。sessionArtifacts(1 条):NTFS 上 swap(TOCTOU)守卫可被击穿。 open 前后的身份比对用的是 Number 精度的dev/ino。NTFS 文件 id 是 64 位(sequence << 48 | record),Number 在 2^53 以上丢精度——sequence ≥ 32 时相邻 MFT 记录会舍入成同一个 ino,于是 lstat 和 open 之间被换掉的文件通过了身份检查,报changed而非missing。现改用{ bigint: true }精确比对。测试里的lstatspy 同步改为透传 options——不透传bigint的 spy 会让比对永远不等,用类型错配掩盖坏掉的守卫。dws-event-stream(1 条):stale 错误清除在 Windows 上输给管道时序。 健康 stdout 行只在子进程存活(exitCode === null)时清除先前记录的 stderr 错误——这个门是防止崩溃后残留的 stdout 把解释崩溃的错误清掉。但 Windows 上最后一行往往在exitCode置位之后才被读到,清除永远不执行,close 上报了 stale 错误。现在 clean exit(code 0)在 close 时把已记录错误判定为 stale;存活门继续保护崩溃路径,fixture 的退出码约定(错误源 exit 1、恢复源 exit 0)把两个方向都钉住。测试 bug
daemon-git-worktree-guard(39 条)。 夹具把原生 Windows 路径不加引号地拼进命令串。guard 如实模拟执行 shell,而 runner 的defaults.run.shell: bash会设置MSYSTEM,getShellConfiguration()判为 bash lane——裸反斜杠是 POSIX 转义:git -C C:\t\repo到 tokenizer 手里成了C:trepo。可解析目标变成不可解析、cd静默原地失败、deny 理由和 allow 预期双双跑偏。失败夹具现经cmdPath()插值,win32 上拼成正斜杠——bash 保留、Windows 同样解析。断言保持原生拼写:guard 打印目标前会过realpath规范化。带引号的插值、win32 变换钉住区、spoof 的 cmd/powershell 区一律未动。review/lib/worktree(4 条)。 git 在 Windows 上把 linked worktree 的.gitgitfile 标为隐藏,对隐藏文件做截断打开(writeFileSync的 CREATE_ALWAYS)报 EPERM。gitfile 重写改为先 unlink 再写的 helper。config(1 条)。getCwd()返回构造器 resolve 过的targetDir,POSIX 夹具/path/to/target在 win32 回来带盘符。对照path.resolve(targetDir)——文件里其他断言本就这么写。SessionMessageHandler(2 条)。 测试的pathToFileURLshim 只处理了 POSIX 上模拟C:\夹具的方向,缺反向:win32 上 POSIX 拼写的夹具(/workspace/export.html)会按进程 cwd 补盘符。补上{ windows: false }分支后,期望的file://URL 在所有平台一致。node-repl.semantics(1 条)。 测试在 kernel 子进程存活时删它的 cwd——Windows 上任何进程的 cwd 目录都删不掉(EBUSY)。先 dispose 再删,并对子进程异步退出做重试。验证
workflow_dispatchci.yml——run 32920750212,以生产环境(bash shell、HOME 重定向、清空密钥)在ecs-win上跑完整test:ci:Test (windows-latest, Node 22.x)成功,是该池上这条 lane 首次转绿(2026-08-25 的 nightly 和所有测量到的 PR 运行都是红的)。cmdPath()和 shim 的镜像分支在非 win32 上是恒等函数,guard 夹具的 Linux 语义构造上不变。--max-warnings 0)、prettier、各包tsc --noEmit干净(cli/vscode 残余 tsc 噪声是本 PR 未触碰文件的陈旧 dist 漂移)。范围之外
托管
windows-2022镜像另有一批以环境为主的失败面(系统 gitconfig 的core.autocrlf=true渗进测试自建仓库、TEMP 里的RUNNER~18.3 短名、daemon 起不来导致的 ECONNREFUSED 级联——约 25 个文件)。#10059 之后该池只是 kill-switch 兜底;等测试夹具先做 git 配置封闭化,再谈恢复(出于安全跑在托管池的)pull-request 触发。