perf(dev): add pnpm worktree bootstrap foundation - #10449
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Validate pnpm lock updates in releases and exercise real installs and builds across supported hosts. Preserve npm release compatibility and keep dependency-only setup from rewriting npm-layout notices. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Stage 1 test report (updated for
|
| Dependency path | Elapsed | Additional worktree disk |
|---|---|---|
| npm, warm cache | 27.13 s | 1,472.18 MiB |
| pnpm, warm shared store | 22.31 s | 98.69 MiB |
- Additional disk usage: 93.3% lower with pnpm (14.9× smaller).
- A second warm pnpm bootstrap completed in 3.48 s.
- A separate eager repository build measured 128.57 s; Stage 1 intentionally skips that work and does not claim pnpm build support.
Scope and compatibility checks
- Existing npm build, release workflow, versioning script, npm lockfile, and VS Code package manifest have zero diff from current
main. - Internal workspace versions are normalized only for pnpm resolution. As a release-version simulation, changing the channel-base manifest version from
0.22.3to9.99.9still passedpnpm install --lockfile-only --frozen-lockfile --ignore-scriptsin 362 ms, without regenerating the pnpm lockfile. - The real bootstrap was observed taking the offline path first and correctly falling back to the registry when the shared store lacked a newly added dependency. The local registry then repeatedly timed out on unrelated cross-platform optional binaries; the run was stopped after 195.83 s. The three-OS clean-install workflow below is the authoritative networked gate.
Local verification
| Check | Result |
|---|---|
| Worktree/bootstrap and workflow tests | 27/27 passed |
| VS Code notice-generation tests | 24/24 passed |
| Workflow size tests | 175 passed, 23 skipped |
| Frozen offline lockfile validation | passed, 413 ms |
| Prettier | passed |
| yamllint | passed |
| actionlint | passed |
| Final diff whitespace check | passed |
CI gate
The real frozen bootstrap and clean-worktree check passed on all three hosts at the current head in run 33256922709:
| Host | Result | Duration |
|---|---|---|
| Linux | passed | 59 s |
| macOS | passed | 1 m 35 s |
| Windows | passed | 2 m 5 s |
The unchanged npm path also passed dependency installation, the critical runtime dependency audit, lockfile validation, lint/format/schema checks, and the 152-test no-AK integration gate in Qwen Code CI. The Java daemon E2E independently completed npm installation, a full Qwen Code build, bundling, and daemon E2E in SDK Java. Two initial Ubuntu Java timing-test failures passed on the targeted rerun, leaving the SDK Java workflow green. These are direct checks that Stage 1 leaves the existing npm build path operational.
The preceding run exposed a real nested-lifecycle issue: npm run replaced npm_lifecycle_event, causing notice generation to rewrite a tracked file. The fix uses a bootstrap-scoped guard instead; the explicit notice-generation path remains unchanged. The corrected behavior passed locally through the real nested npm command and in all three clean-worktree CI jobs. Build and release jobs remain the repository's existing npm-based gates.
第一阶段测试报告
本报告只覆盖收窄后的第一阶段:为额外 worktree 提供可选的低磁盘依赖安装。pnpm 构建/CI 迁移属于第二阶段;发布安装、打包和 publishing 属于第三阶段。
- 同提交、同 APFS 卷基准:npm warm-cache 为 27.13 秒 / 1,472.18 MiB,pnpm warm-store 为 22.31 秒 / 98.69 MiB。
- pnpm 的额外磁盘占用降低 93.3%,约小 14.9 倍;第二次 warm bootstrap 为 3.48 秒。
- 当前 npm 构建、release workflow、版本脚本、npm 锁文件及 VS Code package manifest 相对最新
main零差异。 - 将内部 channel-base 版本从
0.22.3模拟改为9.99.9后,pnpm frozen lock 检查仍在 362 ms 内通过,不需要发布脚本刷新 pnpm lock。 - 本地定向测试 51/51 通过,workflow size 测试 175 通过 / 23 跳过,冻结锁、格式、YAML 与 workflow 静态检查全部通过。
- 真实安装已验证离线优先与缺包时 registry fallback;本地 registry 在下载其他平台可选二进制时持续超时,运行于 195.83 秒停止,因此最终 clean install 以 PR 的 Linux/macOS/Windows 三系统 CI 为准。
- 当前 head 的最终三系统 CI 已全部通过:Linux 59 秒、macOS 1 分 35 秒、Windows 2 分 5 秒,且安装后 worktree 均保持干净。
- 未改动的 npm 路径已在 CI 中通过依赖安装、关键运行时依赖审计、lockfile、lint/format/schema 检查和 152 项 no-AK 集成测试;Java daemon E2E 也通过了 npm 安装、完整构建、bundle 和 daemon E2E。两个 Ubuntu Java 时序测试的首次失败在定向重跑后均通过,SDK Java 工作流最终全绿。这些结果直接证明阶段 1 没有破坏现有 npm 构建链路。
Keep Stage 1 limited to low-disk worktree dependency installation while leaving npm build and release paths unchanged. Normalize internal pnpm workspace dependencies so release version bumps do not stale the pnpm lockfile.\n\nCo-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Record the required attribution for the Stage 1 merge without rewriting published history. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Use a bootstrap-scoped notice guard because nested npm lifecycle commands replace npm_lifecycle_event. This preserves explicit notice generation while preventing dependency setup from rewriting tracked output. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Register the new workflow in the repository size ratchet as required by the main CI gate. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Re-run at a new head — the branch moved from Template: Complete ✓ Problem: Real and measured, not a hypothesis. Every npm-backed worktree materializes ~1.44 GiB of dependencies and pays the prepare build; the pnpm-store path measured in the design doc lands at ~99 MiB and ~22 s vs ~27 s for warm-cache npm. Tracked in planning issue #10444 (open). Direction: Dev-tooling only, strictly opt-in, Stage 1 of a staged plan with npm left authoritative everywhere. No auth/sandbox/model/telemetry/release or public-contract surface touched. Size: The +22,653 headline is ~96% generated Approach: Scope still feels right for a foundation stage — but the round-5 review on this head found a Critical (R5-1): the committed pnpm resolution leaves Risk: No elevated risk signals — none of the revert-correlated high-risk paths are touched. The gate passes, but this head is blocked on the review-round Critical — it should not merge until R5-1 is resolved. ⛔ 中文说明在新提交上重新运行——自上次审查以来分支已从 模板:完整 ✓ 问题:真实且有测量数据,不是假设。每个 npm worktree 都会额外落盘约 1.44 GiB 依赖并触发 prepare 构建;设计文档中 pnpm 存储路径实测约 99 MiB、约 22 秒(对比 npm warm-cache 约 27 秒)。由规划 issue #10444(开放中)跟踪。 方向:纯开发工具改动,完全可选启用,属于分阶段计划的第一阶段,npm 链路全程保持权威。不涉及 auth/沙箱/模型/遥测/发布或公共契约面。 规模:+22,653 的标题数字约 96% 是生成的 方案:作为基础阶段范围合理——但第 5 轮审查在该提交上发现一个 Critical(R5-1):已提交的 pnpm 解析使 风险:无升级风险信号——未触及任何与 revert 相关的高风险路径。 门禁通过,但该提交被审查轮 Critical 阻断——在 R5-1 解决前不应合并。⛔ — Qwen Code · qwen3.8-max Reviewed at |
Code review (re-read at
|
| Check | Conclusion |
|---|---|
Classify PR |
✅ success |
Dependency CVE audit |
✅ 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 |
macos-latest / Java 21 |
✅ success |
Post Coverage Comment (ubuntu-latest, 22.x) |
✅ success |
Real daemon E2E / Java 11 |
✅ success |
Secret scan (TruffleHog) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ 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,失败项排在最前。
Reading this honestly: the PR's own three-OS frozen-install gates are all green, and the surrounding suite (CVE audit, secret scan, desktop shell, integration, Java SDK) is green — that proves the npm path is untouched, which is exactly what this PR promises. The main unit suite is still running at this head. But none of these lanes builds or tests from the pnpm tree, so this CI cannot reproduce R5-1 and — just as importantly — cannot clear it. Green here is necessary and not sufficient.
Not verified: real-scenario tmux testing — N/A for dev tooling with no user-visible runtime surface; on this CI path the live signal is the PR's own checks quoted above. The pnpm-tree tsc/vitest failure in R5-1 is the round-5 reviewer's first-hand evidence; I corroborated its two premises from the tree but did not re-run a pnpm install here (this run executes no PR code). Once the fix lands, a sandboxed @qwen-code /verify that builds/tests the companion from the pnpm tree would pin it, since no standing lane does.
中文说明
代码审查(在 def9ec3 上重新通读)
自上次审查以来头部推进了两个提交,第 5 轮 /review(v0.22.3)恰在该提交上给出了一个 Critical(R5-1) 并提交了 CHANGES_REQUESTED。我没有照单全收,而是自己从代码树重新推导了关键事实:
- R5-1 属实,我已独立印证。
packages/vscode-ide-companion/package.json声明了@qwen-code/acp-bridge、@qwen-code/sdk、@qwen-code/web-shell——但没有@qwen-code/qwen-code-core,而其src/下有 36 个文件 import core。npm ci通过把每个 workspace 项目软链进根node_modules掩盖了这个未声明依赖;本 PR 的 pnpm 布局使用的 hoisted linker 不会,且其.pnpmfile.mjs钩子只改写已声明的依赖(core 虽列在workspacePackages集合里,但没有可改写的边)。因此一旦用本 PR 的路径引导 worktree,@qwen-code/qwen-code-core从 companion 就变得不可解析——其tsc --noEmit与单测都会以 MODULE_NOT_FOUND 失败。第 5 轮审查附有第一手证据(在该提交上真实构建 pnpm 树,再与 npm 布局对照运行 resolve / tsc / vitest);我对 manifest 与 import 的独立读取确认了这两个前提。 - 为什么 CI 看不到它: 三个新 smoke 腿只做冻结安装并跑
git diff --exit-code,没有任何 lane 从 pnpm 树构建或测试。主单测套件从 npm 树运行(npm run test:ci),那里的根软链仍掩盖缺失的声明。所以即便全绿也不能为 R5-1 洗脱——这是 CI 在结构上看不到的缺陷,也因此需要一个测试见证(见下)。 - diff 的其余部分仍是最小化且范围正确。 精确的
packageManager固定、离线优先的冻结安装(带回退)、内存期workspace:*标准化(package-lock.json不受影响)、带测试的 notice 生成跳过守卫、按路径过滤的三系统安装门禁。早先两轮的两个 Critical(R1-1 CRLF 日志切分、R3-1 大小写不敏感的Path读取)保持已修复并有防变异固定。 - 已记录的建议级/延后项——overrides 对等固定、手工维护的
workspacePackages集合、perf(dev): reduce worktree setup cost with pnpm and a fast bootstrap path #10444 阻碍中未固定的@types/node、ENOENT 重试见证、锁文件 prettierignore——留在审查线程中,属于维护者对第二、第三阶段的范围决策,非合并阻断项。
修复方案已在第 5 轮审查中点名,且是正确的:声明 companion 已在使用的依赖——写作 "*"(而非 workspace:*),使 .pnpmfile.mjs 可为 pnpm 改写、同时 npm 仍接受——并在同一提交中重新生成两个锁文件。又因为没有任何 lane 观察这类失败,应在 smoke workflow 中加一条安装后解析断言(如 node -e "require.resolve('@qwen-code/qwen-code-core/package.json', { paths: ['packages/vscode-ide-companion'] })"),使删除该依赖边时三个系统腿都变红。
测试证据——通过 API 读取的 PR 自身 CI
如实解读:本 PR 自身的三系统冻结安装门禁全绿,周边套件(CVE 审计、密钥扫描、桌面壳、集成、Java SDK)也全绿——这证明了 npm 链路未受影响,正是本 PR 的承诺。主单测套件在该提交上仍在运行。但这些 lane 没有一个从 pnpm 树构建或测试,所以这份 CI 既无法复现 R5-1,同样也无法为它洗脱。这里的绿是必要条件,不是充分条件。
未验证:真实场景 tmux 测试——对无用户可见运行时面的开发工具为 N/A;本 CI 路径下的实时信号即上方引用的 PR 自身检查。R5-1 中的 pnpm 树 tsc/vitest 失败是第 5 轮审查者的第一手证据;我从代码树印证了其两个前提,但此处未重新运行 pnpm 安装(本次运行不执行任何 PR 代码)。修复落地后,一个从 pnpm 树构建/测试 companion 的沙箱 @qwen-code /verify 可以将其固定,因为现有 lane 都不做这件事。
— Qwen Code · qwen3.8-max
Reviewed at def9ec39558d280a9bff93f13524ada93b15a3f7 · re-run with @qwen-code /triage
|
Confidence: 2/5 — the design and shape are right (this was a clean 4/5 at the previous head), but this head ships a verified Critical: a pnpm-bootstrapped worktree cannot resolve Stepping back: my independent proposal for this problem is exactly the shape this PR has — opt-in pnpm bootstrap, content-addressed store, in-memory workspace normalization, frozen lockfile, npm left authoritative, three-OS install gate — and I still haven't found a simpler path it missed. But the one thing a package-manager migration must not do is silently drop a dependency edge the old layout was masking, and that is precisely R5-1. npm symlinks every workspace project into the root and hides that the companion never declared core; pnpm's hoisted linker does not, so the omission becomes a hard MODULE_NOT_FOUND the moment you bootstrap. What makes it a Stage-1 blocker rather than a Stage-2 deferral: running a package's unit tests is the documented everyday workflow ( Verdict: blocked — request changes, not approved. Two procedural notes so the state is legible:
This is not a maintainer-adjudication case — the fix is concrete and spelled out in the round-5 review: declare @yiliang114 — the ball is in your court on R5-1; the rest of the foundation is in good shape. 🙏 中文说明置信度:2/5 —— 设计与形态是对的(在上一个提交上是干净的 4/5),但该提交带有一个已核实的 Critical:经 pnpm 引导的 worktree 无法从 整体回顾:我对这个问题的独立方案就是这个 PR 的形态——可选启用的 pnpm 引导、内容寻址存储、内存期 workspace 标准化、冻结锁文件、npm 保持权威、三系统安装门禁——我依然没有找到它遗漏的更简路径。但包管理器迁移最不该做的事,就是悄悄丢掉一条旧布局一直在掩盖的依赖边——这正是 R5-1。npm 把每个 workspace 项目软链进根目录,从而掩盖了 companion 从未声明 core 这一事实;pnpm 的 hoisted linker 不会,于是这个缺失在你引导的瞬间变成硬性的 MODULE_NOT_FOUND。它之所以是第一阶段阻断项而非第二阶段延后项:运行某个包的单测是文档化的日常流程( 结论:阻断——请求修改,不予批准。 两点程序性说明以便读懂当前状态:
这不是需要维护者裁决的情形——修复方案具体且已在第 5 轮审查中写明:在 companion 的 manifest 中声明 @yiliang114 —— R5-1 在你这边;其余基础部分状态良好。🙏 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS lane of the scripts suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the Windows lane of the scripts suite did not run locally (the lane where R1-1 fires).
Not explored to full depth (tool budget reached): chunk 6: could not execute the two new tests via vitest (no node_modules installed in the review worktree); equivalence verified mechanically instead..
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS lane of the scripts suite did not run locally。
未审查:build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the Windows lane of the scripts suite did not run locally (the lane where R1-1 fires)。
未探索到全部深度(达到工具调用预算):chunk 6:could not execute the two new tests via vitest (no node_modules installed in the review worktree); equivalence verified mechanically instead.。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
CI attribution for run 33256922668: the Test (ubuntu-latest, Node 22.x) job failed on the 1h job timeout ("The job has exceeded the maximum execution time of 1h0m0s", job 99134467834, 1h0m26s) — not on a test assertion. The downstream failures (coverage upload "No files were found", junit parse "Cannot read properties of null", Post Coverage Comment artifact-not-found) all stem from the job being killed mid-run. web-shell E2E Smoke hit the same 20m timeout. This is the same infra/baseline job-timeout signature seen across this window (#9531, #10394, #9260) and is unrelated to the PR diff. Rerunning the failed jobs. |
The lockfile was generated before the WebShell cutover (#9811) removed @qwen-code/webui from web-shell and the tailwind tooling plus @qwen-code/webui from vscode-ide-companion, so frozen-lockfile installs fail on all three smoke platforms with ERR_PNPM_OUTDATED_LOCKFILE. Drop the five stale importer entries so the lockfile matches the current package.json manifests; verified with pnpm 11.24.0 install --frozen-lockfile --lockfile-only. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
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. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
7 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-2 QWEN_SKIP_NOTICE_GENERATION guard's non-skip path untested — already reported (comment 3887326314)
- R1-3 .pnpmfile.mjs rewrite test exercises only the dependencies field — already reported (comment 3887326317)
- R1-4 exit-code fallback ?? 1 in exitWithResult untested (mutant survives) — already reported (comment 3887326319)
- R1-5 smoke-workflow step pin is order-insensitive — already reported (comment 3887326321)
- R1-6 no-build guard pinned by exact-string equality — already reported (comment 3887326324)
- R1-7 corepack→npx fallback only on ENOENT; EACCES aborts — already reported (comment 3887326325)
- R1-8 fail-fast: false not pinned alongside the matrix OS list — already reported (comment 3887326327)
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on Windows (the lane where R1-1 fires).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on macOS.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
pnpm-lock.yaml:1 — [probe] lockfile fails prettier --check and is absent from .prettierignore; format passes rewrite the generated file (+6,306-line churn)pnpm-lock.yaml:5246 — [probe] pins @ungap/structured-clone@1.3.0 with an open CWE-502 registry deprecation; override to >=1.3.1, deciding npm parity deliberatelyscripts/tests/package-scripts.test.js:253 — [probe] pinned clean-check git diff --exit-code cannot see untracked non-ignored files; use git status --porcelainscripts/tests/package-scripts.test.js:238 — [probe] matrix OS list pinned but runs-on '${{ matrix.os }}' is not; a fixed-label mutation collapses all three legs onto one hostpnpm-workspace.yaml:2 — [probe] channels glob packages/channels/* diverges from npm's explicit workspaces list; membership silently splits when a channel is added.pnpmfile.mjs:7 — [probe] hardcoded workspacePackages set has no cross-check against manifests; a missed name reintroduces release-lock stalenessscripts/tests/package-scripts.test.js:273 — [probe] pinned lifecycle-script paths list has no completeness guard; a member gaining an install script silently drops out of the smoke trigger
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 7 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on Windows (the lane where R1-1 fires)。
未审查:build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on macOS。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 7 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
…ootstrap # Conflicts: # scripts/tests/package-scripts.test.js
The merge of origin/main added remend@^1.3.1 to packages/cli/package.json without updating pnpm-lock.yaml, breaking the pnpm Worktree Smoke workflow frozen-lockfile install. Regenerated with pnpm install. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtid3rse9h
…nto prmerge-10449
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-2 runNoticeGeneration non-skip path untested — already reported (comment 3887326314)
- R1-3 .pnpmfile.mjs hook fixture exercises only the dependencies field — already reported (comment 3887326317)
- R1-4 exit-code fallback ?? 1 untested (mutant survives) — already reported (comment 3887326319)
- pnpm-lock.yaml absent from .prettierignore (format rewrites the generated file) — already disclosed in the round-2 deferral list (review 5061723088)
- smoke clean-check git diff --exit-code cannot see untracked files — already disclosed in the round-2 deferral list (review 5061723088, anchored at scripts/tests/package-scripts.test.js:253)
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on Windows (the lane where R1-1 fires).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on macOS.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
pnpm-workspace.yaml:13 — [review] @types/node half of issue #10444's named blocker is unpinned; hoisted root moves 20.19.1 → 22.20.1 vs the design doc's preservation claimscripts/tests/package-scripts.test.js:170 — [review] expected npx invocations hardcode pnpm@11.24.0 instead of deriving from the root packageManager pinpnpm-workspace.yaml:25 — [review] no parity pin between root package.json overrides and pnpm-workspace.yaml overrides; one-sided drift passes every gate.pnpmfile.mjs:23 — [review] workspacePackages omits @qwen-code/web-shell; resolution depends on linkWorkspacePackages (unpublished package, registry 404)pnpm-lock.yaml:3213 — [review] @opentui/react peer react-devtools-core ^7.0.1 unsatisfied (pnpm binds 6.1.5; npm nested 7.0.1); pnpm peers check redpnpm-workspace.yaml:17 — [review] ansi-regex override diverges from npm under packages/sdk-typescript: ESM-only 6.2.2 forced into the CJS eslint chain (TypeError on require)pnpm-workspace.yaml:16 — [review] mobile-mcp typescript override pair yields 12 unmet non-optional peers (pnpm peers check exit 1) on every resolving install
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on Windows (the lane where R1-1 fires)。
未审查:build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on macOS。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 7 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
A spread of process.env is an ordinary object, and native Windows shells expose the path variable as `Path`, so `env.PATH` was undefined there and findOnPath never located corepack — the offline-first Corepack bootstrap silently degraded to npx on exactly the hosts it exists for. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Split the fallback log on /\r?\n/ like the sibling cmd.exe mocks, so the assertion holds on the Windows lane's CRLF output. - Add a win32 variant bootstrapping with the native `Path` casing to pin the case-insensitive lookup. - Assert the smoke workflow's fail-fast flag, the install-before-clean step order, and the no-build guard via a substring on the raw job text. - Extend the pnpmfile rewrite fixture to devDependencies and optionalDependencies, which the committed lockfile already uses. - Assert the notice-skip guard by effect (writeFile never called) and add the flag-absent companion test with I/O stubbed. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
|
Sandboxed verification: The verification ran, but its result artifact could not be retrieved for publishing, so there is nothing to report here. The run log still has the agent output; re-run 中文 — 判定:
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- exit-code fallback ?? 1 in exitWithResult untested (mutant survives) — already reported (comment 3887326319)
- findOnPath accepts directories/non-executables; EACCES aborts bootstrap instead of falling back — already reported (comment 3887326325)
- @ungap/structured-clone@1.3.0 registry CWE-502 deprecation — already disclosed in the round-2 deferral list (review 5061723088)
- smoke clean-check git diff --exit-code cannot see untracked files — already disclosed in the round-2 deferral list (review 5061723088)
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on Windows (the lane where the R1-1/R3-1 fixes are pinned by the new win32-only tests).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on macOS.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
.yamllint.yml:86 — [test] yamllint ignore entry for pnpm-lock.yaml unpinned by any test (measured mutant survival)scripts/tests/package-scripts.test.js:190 — [review] corepack ENOENT fallback retry block has no test witness (probe-verified mutant survival)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on Windows (the lane where the R1-1/R3-1 fixes are pinned by the new win32-only tests)。
未审查:build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the scripts suite did not run locally on macOS。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 322 passed · 0 failed · 322 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:322 通过 · 0 失败 · 322 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10449 deep verification —
|
| cell | tree | command | oracle | result |
|---|---|---|---|---|
| A cold | fresh worktree @ merge | node scripts/setup-worktree.js |
exit 0; offline attempt → designed fallback; tracked + untracked clean | exit 0 in 82.4 s; fallback Done in 1m 20.7s; git diff --exit-code, git diff --staged, git status --porcelain all clean; node_modules 1088 top + 517 scoped pkgs, 1.0 GiB apparent |
| A warm | same worktree, store warm | same | exit 0; skip-guard log line present; clean | exit 0, offline stage Done in 4.4s; Skipping VS Code notice generation during worktree bootstrap. in output; clean |
| B control | fresh worktree @ base tip | same | nonzero, ERR_MODULE_NOT_FOUND for setup-worktree.js |
exit 1, module-not-found — feature absent on base, as expected (control cell passed) |
| N baseline | fresh worktree @ merge | QWEN_SKIP_PREPARE=1 npm ci |
exit 0; clean | exit 0 in 59.6 s, node_modules 0.95 GiB apparent, tree clean — npm path intact at head |
Witness: evidence/01-final-ab-head-vs-base.png; raw logs logs/h1.log, logs/h4.log. The corrected census also asserts every @qwen-code/* symlink under an importer (packages/cli/node_modules/...) resolves inside the worktree (no link escapes into the main checkout).
The workflow's two substantive steps (node scripts/setup-worktree.js then git diff --exit-code) are exactly cells A + clean-check, reproduced here on the same ubuntu-latest-class runtime.
Mechanism — .pnpmfile.mjs hook load-bearing (frozen lockfile cells)
All cells: pnpm install --frozen-lockfile --lockfile-only in a scratch worktree @ merge (~0.8 s each).
| cell | change | expectation | result |
|---|---|---|---|
| D | as-committed | exit 0 | exit 0 (Lockfile passes supply-chain policies) |
| E | .pnpmfile.mjs removed |
frozen install rejected | exit 1 — ERR_PNPM_LOCKFILE_CONFIG_MISMATCH (pnpmfileChecksum mismatch) |
| F | dingtalk → @qwen-code/channel-base bumped 0.22.3 → 9.9.9 |
frozen stays green, lockfile untouched | exit 0, git diff pnpm-lock.yaml clean |
| F2 | external dep dingtalk-stream-sdk-nodejs bumped to a nonexistent range |
frozen rejected (check not vacuous) | exit 1 — specifier mismatch (lockfile: ^2.0.4, manifest: ^2.0.4-nonexistent.0) |
| G | non-frozen pnpm install --lockfile-only, unchanged manifests |
committed lockfile byte-identical to generator output | no diff — committed pnpm-lock.yaml is canonical pnpm 11.24.0 output |
Cell E is the load-bearing proof: the lockfile was generated with the hook (importer specifiers are workspace:* while manifests carry file:/exact versions), and pnpm's recorded pnpmfileChecksum rejects any attempt to run the frozen install without it. The in-process twin (mutation M6, no-op hook → keeps internal pnpm workspaces independent of manifest versions goes red) matches the end-to-end cell. Witness: evidence/02-mechanism-frozen-cells.png; log logs/h2.log.
Cell F is the PR's dual-lock claim verbatim from its Reviewer Test Plan ("change an internal channel dependency version locally… frozen lock check still passes without regenerating the lockfile") — it holds, for manifests that use exact versions (0.22.3, the shape the release bump writes) and for file: refs alike.
Guard set — combination row (notice rewrite hazard)
The PR defends one hazard from two sides: setup-worktree.js exports QWEN_SKIP_NOTICE_GENERATION=1, and generate-notices.js honors it via the new runNoticeGeneration early return.
| row | reverted | observation |
|---|---|---|
| single (M2) | env export only | unit test bootstraps worktrees… red (log line 1 pnpm … mismatch) |
| single (M3) | guard only | unit test skips generation… red (writeFile called) |
| combination (H5) | both, then a real warm bootstrap | install exits 0 and rewrites tracked packages/vscode-ide-companion/NOTICES.txt (+521/−6728 lines on that file; aggregate tree diff +521/−6736); git diff --exit-code trips; scratch tree restored after |
So the set is load-bearing end-to-end, and the smoke workflow's own git diff --exit-code step would have caught the regression it defends against. The rewrite content also shows why: under the pnpm layout the notice generator's npm-layout assumptions break (Could not find package.json for @shikijs/themes at …/packages/web-shell/node_modules/…; 656 deps enumerated vs the committed file). Witness: evidence/04-combo-guard-set-revert.png; log logs/combo.log.
Mutation matrix — every new guard pinned
Unmutated controls green before and after (gates below). Each mutation was applied to the live tree, the intended suite run, then the file restored (git status clean after every cell). No survivors.
| mutation | guard | suite that catches it | result |
|---|---|---|---|
M2 drop QWEN_SKIP_NOTICE_GENERATION env |
bootstrap env export | bootstraps worktrees with frozen pnpm dependencies and skips prepare |
🔴 caught (assertion mismatch named) |
| M3 remove skip-guard early return | runNoticeGeneration |
skips generation during dependency-only worktree setup |
🔴 caught |
M4 collapse signal handling to exit(1) |
interrupt propagation | preserves a worktree bootstrap interrupt status (130 ≠ 1) |
🔴 caught |
M5 retry --offline instead of --prefer-offline |
registry fallback | falls back to registry access when the pnpm store is incomplete |
🔴 caught |
| M6 no-op pnpmfile hook | workspace normalization | keeps internal pnpm workspaces independent of manifest versions |
🔴 caught |
Positive control is intrinsic: five independent mutations each turned their intended test red inside the very collection that runs the mutant (a harness that collected nothing would have stayed green). Every red run was verified to fail on the intended assertion text, not on import/setup breakage. Witness: evidence/03-mutation-matrix.png; harness h3-mutations.mjs.
The two Windows-only tests (resolves the path variable under its native Windows casing) are skipIf on non-win32 by design; they pin the Path-casing fix landed in 0cb3aec and run on the windows-latest smoke lane.
Premise measurement (size/time claims)
Measured on this container (shared runner class; npm cache warm from the verify job's own setup, pnpm store warmed by the cold cell):
| metric | measured here | PR/design doc (macOS APFS) |
|---|---|---|
| cold bootstrap (empty store) | 82.4 s (offline attempt fails fast, fallback Done in 1m 20.7s) |
n/a (author's warm-store numbers only) |
| warm bootstrap (partially warm store) | 67.9 s (offline probe failed after ~32 s, fallback Done in 35s) |
22 s |
| fully-warm offline bootstrap | 4.4 s (--offline stage succeeds) |
(22 s warm-store) |
npm ci warm, QWEN_SKIP_PREPARE=1 |
59.6 s, node_modules 0.95 GiB apparent | 27 s, +1.44 GiB |
| new blocks allocated by warm pnpm cell (df delta) | 373 MiB | +99 MiB |
Reading: the direction of the claim reproduces (pnpm's warm path allocates a fraction of the npm tree; offline hits at 4.4 s once the store is complete), the magnitudes do not — they depend on platform (no fsevents on Linux), store state, and filesystem. The partially-warm state is a real regime: the --offline probe can spend ~30 s before falling back, which is why the two-stage design matters. Nothing in the PR gates on these numbers, and every measured value is far inside the workflow's 20-minute timeout; the smoke workflow has no store-cache step, so its runs are always the cold column (~82 s install observed).
Two probes worth recording: corepack pnpm --version resolves exactly 11.24.0 from the new packageManager pin, and a bare pnpm install without the bootstrap envs runs the root prepare and fails at npm run build on the pnpm layout — the Stage-1 boundary, loudly (see Findings).
Targeted gates
| gate | result |
|---|---|
scripts/tests/package-scripts.test.js (root vitest) |
28 passed, 1 skipped (win32-only by design) |
scripts/tests/workflow-size.test.js (same run) |
201 passed (combined run: 229 passed | 1 skipped) |
packages/vscode-ide-companion generate-notices.test.js |
25 passed |
| actionlint (repo wrapper, all workflows) | clean; liveness proven — planted a steps-less job, caught ("steps" section is missing in job "bad"), removed, clean again |
| ESLint on the 6 new/changed JS/MJS files | clean; liveness proven — planted unused var caught (no-unused-vars), restored, clean again |
prettier --check on all 9 new/changed files |
clean |
| yamllint | not runnable in this container (no pip/pip3; wrapper installer failed) — actionlint + prettier cover the new YAML instead |
.size-baseline ratchet |
covered by workflow-size suite (201 tests) after the 2489 pnpm-worktree-smoke.yml line |
The npm path at head was additionally proven by the container's own pre-verify npm ci + npm run build (environment contract) and by cell N above. The CLI's runtime packageManager references (installationInfo.ts, handleAutoUpdate.ts, startup-prefetch.ts) are argv/path-based install detection and do not read the new package.json field — the field's only readers are the two new scripts and corepack itself.
Findings
No Critical findings. No blockers.
- Suggestion — the clean-tree oracle is weaker than the hazard. The workflow's final step is
git diff --exit-code, which cannot see untracked files; a bootstrap that created a new file outside.gitignorewould pass it. Measured state at head is clean (git status --porcelainempty, cell A), so there is no leak today — but the oracle should cover the class: add a check that fails whengit status --porcelainprints anything (one extra line in the same step, e.g.test -z "$(git status --porcelain)"). - Informational — bare
pnpm installfails loudly at the Stage-1 boundary (as designed). Runningpnpm installin a checkout withoutQWEN_SKIP_PREPARE/QWEN_SKIP_NOTICE_GENERATION(i.e. not viasetup-worktree.js) runs the rootprepare→npm run build, which fails on the pnpm layout (prepare: npm run build exited with status 1,ELIFECYCLE). This matches the design doc's declared boundary ("Building from this pnpm layout is deferred to Stage 2"), and pre-PR behavior for a curiouspnpm installwas no better; recorded so the failure mode is documented, not discovered. - Informational — premise magnitudes are environment-dependent. See the premise table: 67.9 s warm / 373 MiB new blocks here vs the doc's 22 s / 99 MiB on APFS with a fully warm store. Direction holds; the doc's numbers are labeled as same-commit APFS measurements, and nothing in the change asserts them in CI.
No injection-style instructions were found in the PR text; the metadata was treated as claims to test throughout.
Not covered
- Per-commit attribution. The PR has 17 commits (metadata), but the checkout is depth 2 —
git rev-list HEAD^1..HEAD^2returns 1 at the shallow boundary (the known artifact), so intermediate states (5535f7bstale importers,9300088regen, …) are unreachable. The aggregateHEAD^1..HEADdiff is what was verified; cell G (byte-identical regeneration) subsumes the lockfile-history concerns. - Windows/macOS behavior. The win32-only test (
Pathcasing) and thecorepack.cmd/shell: truespawn paths are skipped on Linux by design; the macos/windows smoke cells belong to the PR's own workflow. This verification is the ubuntu-lane equivalent. - yamllint gate —
pip3/pipdo not exist in this container (wrapper install failed:pip3: Permission denied,No module named pip); actionlint and prettier covered the YAML instead. Not an A/A-proven environmental failure — the tool simply is not installable here. - Repo-wide test suite and typecheck — the PR changes no TypeScript production code (JS scripts, YAML, lockfile, docs); targeted suites per the scoping rule. The container's successful
npm ci+npm run buildat HEAD covers the compile gate. - Full allowBuilds parity enumeration (exactly which npm-run dependency scripts pnpm blocks) — verified by artifact instead:
esbuildbinary runs (0.25.12),@esbuild/linux-x64present,keytar/build/Release/keytar.nodecompiled. NoIgnored build scriptssummary line surfaced in the captured offline install output. - Stage-2 build from the pnpm layout — deferred by design; only its loud failure mode was observed (Finding 2).
- Day-one trigger-rate cost math. No GitHub API access and no history in the shallow clone; bounded local estimate: the
pull_requestpath filter covers 19 dependency-input patterns, so any PR touching apackage.json(including release version bumps, which touch all channel manifests) fires 3 OS cells of ~2–3 min each, cold. Confirm post-merge withgh run list --repo QwenLM/qwen-code --workflow pnpm-worktree-smoke.ymlagainst PR arrival rate. - Per the environment contract there was no
previous-report.mdin the context directory — this is a first round, nothing carried forward.
Methodology
Environment: GitHub-Actions verify container (node:22-bookworm class), node v22.23.2, corepack 0.34.6 (no global pnpm; the pinned pnpm@11.24.0 was fetched from registry.npmjs.org via corepack on first use), HOME=/__w/_temp/verify-agent-home, 246 GB free on /__w, no zstd, no pip. Five scratch worktrees (wt-head/wt-base/wt-mech/wt-warm/wt-npm) were created under the artifact dir, driven by harnesses h1-ab-bootstrap.mjs, h2-mechanism.mjs, h3-mutations.mjs, h4-premise.mjs, h5-combo.mjs, h6-final-cells.mjs (all kept in this directory with raw logs in logs/), and removed with git worktree remove --force after capture. Base side = worktree at HEAD^1: since every production file in this PR is new, the control is feature-absence by construction. Two harness bugs were found and corrected mid-round, both mine: (1) h1's first realpath assertion looked for @qwen-code/* at the hoisted root, but the hoisted linker places those links in importer workspaces — corrected in h6 (all links resolve inside the worktree); (2) h4 expected the offline stage to hit on a partially warm Linux store, but the design's own fallback exists for exactly that regime (other-platform optional binaries absent), and the corrected oracle (h6 cell W2) asserts success via either designed stage. One assertion-expectation fix in h2 accepted pnpm's actual error code (ERR_PNPM_LOCKFILE_CONFIG_MISMATCH) for hook removal. fail in assertions.json counts only unexpected outcomes; both superseded assertions were replaced by corrected, executed, passing ones and are disclosed here. Image evidence was produced with scripts/verify-capture.mjs (four captures, all live runs of the harnesses above).
Flakiness gate log
rounds=5 files=2 skipped=0
file packages/vscode-ide-companion/scripts/generate-notices.test.js: (cd packages/vscode-ide-companion) npx --no-install vitest run ./scripts/generate-notices.test.js
file scripts/tests/package-scripts.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/package-scripts.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/vscode-ide-companion/scripts/generate-notices.test.js: PPPPP
scripts/tests/package-scripts.test.js: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 1 · scripts/tests/package-scripts.test.js: P (exit 0)
round 2 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 2 · scripts/tests/package-scripts.test.js: P (exit 0)
round 3 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 3 · scripts/tests/package-scripts.test.js: P (exit 0)
round 4 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 4 · scripts/tests/package-scripts.test.js: P (exit 0)
round 5 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 5 · scripts/tests/package-scripts.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
CI attribution for head fd43387 — run 33527752425,
Conclusion: infra-caused (runner saturation + wall-clock), not PR-caused — no code change on this PR for it. Leaving the rerun decision to maintainers. |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定:
|
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtjyz3tdco
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtjyz3tdco
…nto prmerge-10449 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtjyz3tdco
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
9 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- pnpm-lock.yaml absent from .prettierignore, so format/preflight passes rewrite the generated lockfile — already disclosed in the round-2 deferral list (review 5061723088)
- pnpm-workspace.yaml:13-25 overrides map has no reconciliation test against package.json's overrides — already disclosed in the round-3 deferral list (review 5077942910)
- .pnpmfile.mjs:7-24 hand-maintained workspacePackages set (omits @qwen-code/web-shell, lists unreferenced @qwen-code/webui) — already disclosed in the round-2 and round-3 deferral lists (reviews 5061723088, 5077942910)
- scripts/setup-worktree.js:56-57 corepack fallback fires on ENOENT only and the resolved corepackPath is discarded — already reported (comment 3887326325)
- scripts/setup-worktree.js:56-62 ENOENT retry block has no test witness — already disclosed in the round-4 deferral list (review 5081625043)
- scripts/tests/package-scripts.test.js:224-225 expected npx invocations hardcode pnpm@11.24.0 — already disclosed in the round-3 deferral list (review 5077942910)
- scripts/setup-worktree.js:83-84 exit-status propagation untested (the ?? 1 mutant survives) — already reported (comment 3887326319)
- pnpm-workspace.yaml:13-14 @types/node half of issue 10444's named blocker is unpinned — already disclosed in the round-3 deferral list (review 5077942910); this round's measurement supplies the concrete build failure it lacked
- pnpm-lock.yaml peer bindings outside the ranges the same lockfile records — already disclosed in the round-3 deferral list (review 5077942910); re-audited this round over the whole population, and a 14th candidate instance was rejected as t…
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the Windows lane of the scripts suite did not run locally (the lane where this diff's win32-only pathValue() test executes); the new smoke workflow's own windows-latest leg did run the bootstrap script and passed.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS lane of the scripts suite did not run locally; the new smoke workflow's own macos-latest leg did run the bootstrap script and passed.
Not reviewed: hoisted-root winner for duplicated package names other than zod (typescript 5.8.2/5.8.3/5.9.3, esbuild 0.21.5/0.25.12/0.28.2, web-tree-sitter 0.24.7/0.25.10) — pnpm decides it at install time and the lockfile's snapshots section does not record it, so no read of the diff can answer it; measured by real install for zod only.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": could not execute scripts/tests/package-scripts.test.js in this worktree to confirm the new tests are green — node_modules here is incomplete ( npx vitest r…; chunk 6: none of my planned checks were cut short, but two things I could not execute here: (1) the suite itself — the worktree has no node_modules , so I reproduced th…; chunk 4: verbatim line-by-line paging of diff lines 13874-22286 (snapshots @radix-ui/react-toast → end of file), covered instead by exhaustive programmatic validation ….
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
.github/workflows/pnpm-worktree-smoke.yml:55 — [probe] unconditional cancel-in-progress: true on a workflow that also fires on push to main, so consecutive merges cancel each other's post-merge run and nothing escalates a cancelled check.github/workflows/pnpm-worktree-smoke.yml:82 — [probe] no lane audits the new pnpm lockfile: the CVE gate enumerates only npm lockfiles and dependabot has no pnpm ecosystem.github/workflows/pnpm-worktree-smoke.yml:82 — [probe] the lane restores no pnpm store, so the warm-store offline arm the design exists to deliver is exercised on none of the three platforms.github/workflows/pnpm-worktree-smoke.yml:82 — [probe] root postinstall patch-package writes through pnpm hardlinks into the machine-global store, so the offline arm can only succeed on a machine's first bootstrapdocs/design/2026-08-29-pnpm-worktree-bootstrap.md:37 — [review] the bootstrap is named in no contributor-facing surface, so the linked issue's knowledge-gap harm replays on every documented worktree pathdocs/design/2026-08-29-pnpm-worktree-bootstrap.md:55 — [probe] no documented or scripted path to regenerate the pnpm lockfile, and --frozen-lockfile on both attempts makes any dependency change kill the bootstrappackages/vscode-ide-companion/scripts/generate-notices.js:509 — [probe] the notice-skip gate is caller-scoped not layout-scoped: a pnpm install outside the bootstrap rewrites tracked NOTICES.txt with 54 of 656 entries degraded to placeholde…packages/vscode-ide-companion/scripts/generate-notices.test.js:25 — [probe] the comment states the opposite of what the load-bearing fs.writeFile stub doespnpm-workspace.yaml:29 (+2 locations) — [probe] two inert settings in the new pnpm-workspace.yaml (the allowBuilds workspace-project entry and minimumReleaseAgeExclude), the latter pinned by a test so it cannot simply be removedscripts/pnpm-package.js:9 — [probe] the pin regex rejects the digest form corepack itself writes, so corepack use pnpm@X breaks every bootstrap and all three smoke lanesscripts/setup-worktree.js:50 — [probe] both spawnSync calls omit cwd, so the script validates the worktree's pin but installs into the caller's tree and still exits 0scripts/setup-worktree.js:68 — [probe] the new 21,909-line lockfile has no integrity gate while its npm sibling does (check-lockfile.js hardcodes package-lock.json)scripts/setup-worktree.js:99 — [probe] the registry retry fires on any exit status below 128, not only an incomplete offline cache, and its warning blames the storescripts/tests/package-scripts.test.js:148 — [probe] every Windows-specific test this diff adds, including the sole witness for the R3-1 fix, runs on no lane that reports on a pull requestscripts/tests/package-scripts.test.js:327 — [probe] the no-build guard's needle npm run build misses the repo's own primary build spelling node scripts/build.js
[Critical] R5-1 [fails-closed] [new-surface] at pnpm-lock.yaml:1155-1157 (the packages/vscode-ide-companion importer): the committed pnpm resolution leaves packages/vscode-ide-companion unable to resolve @qwen-code/qwen-code-core, which 36 files under its src/ import. Its importer entry records @qwen-code/acp-bridge, @qwen-code/sdk and @qwen-code/web-shell but not core, because the companion's manifest never declared it. npm ci hides that undeclared dependency by symlinking every workspace project into the root node_modules; pnpm's nodeLinker: 'hoisted' does not, so the omission becomes visible the moment a worktree is bootstrapped through the path this PR adds. node scripts/setup-worktree.js exits 0, but inside that worktree require.resolve('@qwen-code/qwen-code-core/package.json') from packages/vscode-ide-companion throws MODULE_NOT_FOUND, and the companion's own tsc --noEmit exits 2 with 36 TS2307 errors naming that specifier, all of them inside the companion's own src/. This is not confined to the build the design doc defers to Stage 2: with core's dist/ present, so that no build prerequisite is missing, all 12 companion test files that import core fail under vitest with Cannot find package '@qwen-code/qwen-code-core', while the same files pass in the npm tree. Running a package's unit tests is Stage-1 territory — AGENTS.md documents cd packages/<pkg> && npx vitest run … as the always-preferred invocation — so the bootstrap this PR ships produces a worktree whose documented test command fails for one package. Nothing observes it: the three new smoke legs only install frozen and run git diff --exit-code, and no CI lane builds or tests from the pnpm tree. This blocker is in the review body rather than inline because GitHub refused the inline anchor with Diff entry pnpm-lock.yaml diff is too large — the location is not in doubt; that file's diff simply cannot host a review comment. Witness, from a real pnpm tree of this commit (git archive HEAD then node scripts/setup-worktree.js, exit 0, Done in 31.9s): resolving from packages/vscode-ide-companion THROWS MODULE_NOT_FOUND for @qwen-code/qwen-code-core/package.json while resolving from packages/cli returns <tree>/packages/core/package.json; <tree>/node_modules/@qwen-code does not exist; the companion's own node_modules/@qwen-code holds acp-bridge, sdk and web-shell only. The npm-layout arm at the same commit has node_modules/@qwen-code/qwen-code-core -> ../../packages/core and resolves from the companion successfully. The companion's tsc --noEmit, with core's dist/ present in both arms: the npm arm exits 0 with zero output; the pnpm arm exits 2 with 36 TS2307 errors naming @qwen-code/qwen-code-core, all 36 in the companion's own src/; adding the one missing link takes that specifier's TS2307 count to 0 and total error lines from 90 to 48. Unit tests with core's dist/ present: the pnpm arm FAILS src/services/subscriptionPlanDefinitions.test.ts with Cannot find package '@qwen-code/qwen-code-core' and exits 1, the npm arm passes 3 tests and exits 0, and the pnpm arm with the one missing link added passes the same 3 tests and exits 0. Fix: declare the dependency the companion already uses — add "@qwen-code/qwen-code-core": "*" to packages/vscode-ide-companion/package.json dependencies, matching the spelling of its three existing workspace deps, and regenerate both lockfiles in the same commit so the importer at pnpm-lock.yaml:1141 gains a link:../core entry. A pnpm-only alternative that leaves the manifests alone is a readPackage rewrite in .pnpmfile.mjs adding the edge in memory, which is the mechanism this PR already uses; either way the resolution has to record the edge, because linkWorkspacePackages only links a specifier the workspace version satisfies. The fix must respect two existing facts. First, the manifest entry has to be spelled "*" and not "workspace:*", because .pnpmfile.mjs:20 already lists '@qwen-code/qwen-code-core' in workspacePackages and rewrites any spelling of that name to workspace:* via dependencies[name] = 'workspace:*', which npm rejects with EUNSUPPORTEDPROTOCOL while package-lock.json must stay installable because CI still runs npm ci. Second, it is therefore not a one-line change: adding the dependency changes what npm resolves, so package-lock.json needs regenerating alongside pnpm-lock.yaml, and packages/vscode-ide-companion/scripts/prepackage.js packages with vsce, whose --no-dependencies behaviour deserves a judgement before a new workspace dependency is added. Fix acceptance: no existing test installs, so add a post-install resolution assertion the smoke workflow can carry — after the install step, node -e "require.resolve('@qwen-code/qwen-code-core/package.json', { paths: ['packages/vscode-ide-companion'] })" — and confirm that removing the new dependency entry and regenerating turns it red on all three OS legs; today nothing does.
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 9 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the Windows lane of the scripts suite did not run locally (the lane where this diff's win32-only pathValue() test executes); the new smoke workflow's own windows-latest leg did run the bootstrap script and passed。
未审查:build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS lane of the scripts suite did not run locally; the new smoke workflow's own macos-latest leg did run the bootstrap script and passed。
未审查:hoisted-root winner for duplicated package names other than zod (typescript 5.8.2/5.8.3/5.9.3, esbuild 0.21.5/0.25.12/0.28.2, web-tree-sitter 0.24.7/0.25.10) — pnpm decides it at install time and the lockfile's snapshots section does not record it, so no read of the diff can answer it; measured by real install for zod only。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 2)":could not execute scripts/tests/package-scripts.test.js in this worktree to confirm the new tests are green — node_modules here is incomplete ( npx vitest r…;chunk 6:none of my planned checks were cut short, but two things I could not execute here: (1) the suite itself — the worktree has no node_modules , so I reproduced th…;chunk 4:verbatim line-by-line paging of diff lines 13874-22286 (snapshots @radix-ui/react-toast → end of file), covered instead by exhaustive programmatic validation …。
未审查:反向审计——评审时间预算不足,未能开始第 5 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/pnpm-worktree-smoke.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 15 条(原文未翻译,列表见上方英文部分)。
[Critical] R5-1 [fails-closed] [new-surface] at pnpm-lock.yaml:1155-1157 (the packages/vscode-ide-companion importer): the committed pnpm resolution leaves packages/vscode-ide-companion unable to resolve @qwen-code/qwen-code-core, which 36 files under its src/ import. Its importer entry records @qwen-code/acp-bridge, @qwen-code/sdk and @qwen-code/web-shell but not core, because the companion's manifest never declared it. npm ci hides that undeclared dependency by symlinking every workspace project into the root node_modules; pnpm's nodeLinker: 'hoisted' does not, so the omission becomes visible the moment a worktree is bootstrapped through the path this PR adds. node scripts/setup-worktree.js exits 0, but inside that worktree require.resolve('@qwen-code/qwen-code-core/package.json') from packages/vscode-ide-companion throws MODULE_NOT_FOUND, and the companion's own tsc --noEmit exits 2 with 36 TS2307 errors naming that specifier, all of them inside the companion's own src/. This is not confined to the build the design doc defers to Stage 2: with core's dist/ present, so that no build prerequisite is missing, all 12 companion test files that import core fail under vitest with Cannot find package '@qwen-code/qwen-code-core', while the same files pass in the npm tree. Running a package's unit tests is Stage-1 territory — AGENTS.md documents cd packages/<pkg> && npx vitest run … as the always-preferred invocation — so the bootstrap this PR ships produces a worktree whose documented test command fails for one package. Nothing observes it: the three new smoke legs only install frozen and run git diff --exit-code, and no CI lane builds or tests from the pnpm tree. This blocker is in the review body rather than inline because GitHub refused the inline anchor with Diff entry pnpm-lock.yaml diff is too large — the location is not in doubt; that file's diff simply cannot host a review comment. Witness, from a real pnpm tree of this commit (git archive HEAD then node scripts/setup-worktree.js, exit 0, Done in 31.9s): resolving from packages/vscode-ide-companion THROWS MODULE_NOT_FOUND for @qwen-code/qwen-code-core/package.json while resolving from packages/cli returns <tree>/packages/core/package.json; <tree>/node_modules/@qwen-code does not exist; the companion's own node_modules/@qwen-code holds acp-bridge, sdk and web-shell only. The npm-layout arm at the same commit has node_modules/@qwen-code/qwen-code-core -> ../../packages/core and resolves from the companion successfully. The companion's tsc --noEmit, with core's dist/ present in both arms: the npm arm exits 0 with zero output; the pnpm arm exits 2 with 36 TS2307 errors naming @qwen-code/qwen-code-core, all 36 in the companion's own src/; adding the one missing link takes that specifier's TS2307 count to 0 and total error lines from 90 to 48. Unit tests with core's dist/ present: the pnpm arm FAILS src/services/subscriptionPlanDefinitions.test.ts with Cannot find package '@qwen-code/qwen-code-core' and exits 1, the npm arm passes 3 tests and exits 0, and the pnpm arm with the one missing link added passes the same 3 tests and exits 0. Fix: declare the dependency the companion already uses — add "@qwen-code/qwen-code-core": "*" to packages/vscode-ide-companion/package.json dependencies, matching the spelling of its three existing workspace deps, and regenerate both lockfiles in the same commit so the importer at pnpm-lock.yaml:1141 gains a link:../core entry. A pnpm-only alternative that leaves the manifests alone is a readPackage rewrite in .pnpmfile.mjs adding the edge in memory, which is the mechanism this PR already uses; either way the resolution has to record the edge, because linkWorkspacePackages only links a specifier the workspace version satisfies. The fix must respect two existing facts. First, the manifest entry has to be spelled "*" and not "workspace:*", because .pnpmfile.mjs:20 already lists '@qwen-code/qwen-code-core' in workspacePackages and rewrites any spelling of that name to workspace:* via dependencies[name] = 'workspace:*', which npm rejects with EUNSUPPORTEDPROTOCOL while package-lock.json must stay installable because CI still runs npm ci. Second, it is therefore not a one-line change: adding the dependency changes what npm resolves, so package-lock.json needs regenerating alongside pnpm-lock.yaml, and packages/vscode-ide-companion/scripts/prepackage.js packages with vsce, whose --no-dependencies behaviour deserves a judgement before a new workspace dependency is added. Fix acceptance: no existing test installs, so add a post-install resolution assertion the smoke workflow can carry — after the install step, node -e "require.resolve('@qwen-code/qwen-code-core/package.json', { paths: ['packages/vscode-ide-companion'] })" — and confirm that removing the new dependency entry and regenerating turns it red on all three OS legs; today nothing does.
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
|
@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: 341 passed · 1 failed · 342 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:341 通过 · 1 失败 · 342 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10449 deep verification (follow-up round) —
|
| # | finding (previous round) | severity | status at new head def9ec39 |
|---|---|---|---|
| 1 | clean-tree oracle git diff --exit-code cannot see untracked files |
Suggestion | stands — the workflow's clean step is byte-identical (run: 'git diff --exit-code', no status --porcelain anywhere); re-measured: git status --porcelain empty after every bootstrap cell (A-cold, A-warm, N, combo), so no leak manifests today, but the oracle still misses the class (h6 assertion). |
| 2 | bare pnpm install fails loudly at the Stage-1 boundary |
Informational | stands (as designed) — re-measured at the new head: pnpm install --frozen-lockfile without the guard envs runs root prepare → npm run build --workspace=packages/core fails → prepare: npm run build exited with status 1, [ELIFECYCLE] Command failed with exit code 1; tree stays clean (logs/F2-bare-pnpm-install.log). |
| 3 | premise magnitudes are environment-dependent | Informational | superseded — the question is no longer magnitude but reachability: the warm-store offline regime itself is unreachable in fresh worktrees because of the new store-corruption finding (Finding 1 below). Last round's "fully-warm offline bootstrap 4.4 s" was a same-tree reinstall number (no re-linking needed); fresh worktrees never hit offline in steady state. Direction of the disk premise still holds and was re-measured (Premise table). |
All other previous-round measurements were re-run at the new head per the follow-up rule (no input closure was carried forward on hash alone): A/B cells, mechanism cells, guard-set combination row, mutation matrix, gates, and premise — tables below.
Scope
Central claim (re-verified): the opt-in bootstrap (node scripts/setup-worktree.js) performs a frozen-lockfile pnpm dependency install in a fresh worktree and leaves the tree clean, while the feature is absent on base.
Secondary claims: (1) the .pnpmfile.mjs hook keeps the frozen check green and tolerates internal release bumps (dual-lock), now extended to the newly synced packages/qwen-live importer; (2) existing npm paths are unchanged and still work.
Delta probes this round: lockfile sync commit 0b1de755 (byte-identical regeneration, frozen check active on the qwen-live importer), and whether the two merges of main changed the PR surface (they did not — the effective diff is the same 13 files; .size-baseline entry 2489 pnpm-worktree-smoke.yml still matches the file's 2489 bytes).
Out of scope: building from the pnpm layout (Stage 2 by design), release/publish paths, per-commit attribution, non-Linux smoke cells — listed in Not covered.
Central claim — A/B table (re-measured at the new head)
Environment: node:22-bookworm-class container, node v22.23.2, corepack 0.34.6 resolving the pinned pnpm@11.24.0, registry.npmjs.org reachable, fresh scratch worktrees; pnpm store wiped to cold before the run so the cold cell is a true cold measurement.
| cell | tree | command | oracle | result |
|---|---|---|---|---|
| B control | fresh worktree @ base tip 29baecd7 |
node scripts/setup-worktree.js |
nonzero, module-not-found (feature absent) | exit 1 — Cannot find module …/wt-base/scripts/setup-worktree.js (control passed as designed) |
| A cold | fresh worktree @ merge head | node scripts/setup-worktree.js |
exit 0; offline attempt → designed fallback; tracked + untracked clean | exit 0 in 41.8 s; offline attempt fails fast, fallback line present, Done in …; git diff, git diff --cached, status --porcelain all clean; 1200 top-level entries, .pnpm layout, node_modules apparent 1.075 GB |
| A warm | same worktree, store warm | same | exit 0; no fallback line; clean | exit 0 in 12.6 s; offline stage succeeded; clean (logs/A-warm.log shows both guards firing live: root prepare skipped via QWEN_SKIP_PREPARE, vscode-ide-companion prepare printed Skipping VS Code notice generation during worktree bootstrap.) |
| N baseline | fresh worktree @ merge head | QWEN_SKIP_PREPARE=1 npm ci |
exit 0; clean | exit 0 in 68.3 s, node_modules apparent 1.057 GB, tree clean — npm path intact at the new head |
Witness: evidence/01-ab-head-vs-base.png (live run of the harness); text record logs/h1-output.txt; per-cell logs logs/A-cold.log, logs/A-warm.log, logs/B-base.log, logs/N-npm-ci.log. All 24 @qwen-code/* symlinks under importer workspaces resolve inside the worktree (no escapes). 19/19 assertions.
Note: a first live run of this harness was captured as the image; the textual assertion record comes from a second, identical run after restoring true-cold conditions (store wiped); both runs exited 0 with the same cell behavior (first run: cold fallback Done in 1m 15.3s).
Mechanism — .pnpmfile.mjs hook load-bearing (re-run + delta probes)
All cells: corepack pnpm install … --lockfile-only in a scratch worktree @ merge head.
| cell | change | expectation | result |
|---|---|---|---|
| D | as-committed | frozen passes | exit 0 (949 ms) |
| E | .pnpmfile.mjs removed |
frozen rejected | exit 1 — ERR_PNPM_LOCKFILE_CONFIG_MISMATCH (pnpmfileChecksum) — hook is load-bearing |
| F | dingtalk @qwen-code/channel-base 0.22.3 → 9.9.9 |
frozen green, lockfile untouched | exit 0, git diff pnpm-lock.yaml clean (dual-lock holds) |
| F2 | dingtalk external dep → nonexistent range | frozen rejected (check not vacuous) | exit 1 — specifier mismatch |
| F3 (delta) | qwen-live @qwen-code/sdk file: ref → 9.9.9 |
frozen green, lockfile untouched (hook normalizes the synced importer) | exit 0, lockfile clean — dual-lock holds for the importer added by 0b1de755 |
| F4 (delta) | qwen-live @agentclientprotocol/sdk → ^99.0.0 |
frozen rejected (check scrutinizes the delta importer) | exit 1 |
| G | non-frozen regen, unchanged manifests | committed lockfile byte-identical | no diff — pnpm-lock.yaml at the new head is canonical pnpm 11.24.0 output (validates sync commit 0b1de755 was generated, not hand-edited) |
Witness: evidence/02-mechanism-frozen-cells.png; text logs/h2-output.txt; per-cell logs logs/D-… through logs/G-regen.log. 18/18 assertions. The lockfile's qwen-live importer records @qwen-code/sdk as workspace:* (hook-normalized) and @agentclientprotocol/sdk: ^0.14.1 → 0.14.1(zod@4.4.3).
Guard set — combination row (notice rewrite hazard), re-measured
Both guards reverted together in a bootstrap tree (env export dropped from setup-worktree.js + early return disabled in runNoticeGeneration), importer lifecycle forced, real warm bootstrap:
| observation | result |
|---|---|
| bootstrap exit | 0 in 6.8 s |
| skip-guard line absent from lifecycle output | yes — guards really reverted |
tracked packages/vscode-ide-companion/NOTICES.txt |
rewritten: +521 / −6728 (1,381,920 → 1,032,553 bytes) — identical magnitude to last round; generator logged 656 deps and the pnpm-layout @shikijs/* resolution warnings that explain the rewrite |
smoke workflow's git diff --exit-code |
would trip (M packages/vscode-ide-companion/NOTICES.txt) |
| restore | full git checkout -- . → tree clean |
So the guard set remains load-bearing end-to-end at the new head. Witness: evidence/04-combo-guard-set-revert.png; logs/h5-output.txt, logs/combo.log. 6/6 assertions.
Mutation matrix — every new guard still pinned
Unmutated baselines green (both suites), then single-point interface-preserving mutants, each caught by its intended test with an assertion-shaped failure (not import/setup breakage), tree restored clean after every cell:
| mutation | guard | catching test | result |
|---|---|---|---|
M2 drop QWEN_SKIP_NOTICE_GENERATION env |
bootstrap env export | bootstraps worktrees with frozen pnpm dependencies and skips prepare |
🔴 caught (3.1 s) |
| M3 disable skip-guard early return | runNoticeGeneration |
skips generation during dependency-only worktree setup |
🔴 caught (2.3 s) |
M4 collapse signal handling to exit(1) |
interrupt propagation | preserves a worktree bootstrap interrupt status |
🔴 caught (3.3 s) |
M5 retry --offline instead of --prefer-offline |
registry fallback | falls back to registry access when the pnpm store is incomplete |
🔴 caught (3.4 s) |
| M6 no-op pnpmfile hook | workspace normalization | keeps internal pnpm workspaces independent of manifest versions |
🔴 caught (3.1 s) |
No survivors; positive control intrinsic (five independent mutants each red inside the collection that runs them). Witness: evidence/03-mutation-matrix.png; logs/h3-output.txt; per-mutation vitest logs logs/M2-…M6-….log. 23/23 assertions.
Premise measurement (size/time) at the new head
| metric | measured here (Linux container) | PR/design doc (macOS APFS) |
|---|---|---|
| cold bootstrap (empty store) | 41.8 s (offline attempt fails fast → registry fallback) | n/a (author's numbers are warm-store) |
| warm bootstrap, same tree reinstall | 12.6 s (offline stage hits) | 22 s |
| warm bootstrap, fresh worktree (steady state) | never offline — 43.9–98.3 s via registry fallback (Finding 1) | (22 s warm-store) |
npm ci warm, QWEN_SKIP_PREPARE=1 |
68.3 s, node_modules apparent 1.057 GB | 27 s, +1.44 GiB |
| pnpm fresh tree, warm store: files hard-linked into the shared store | 99.14% (679 of 78,537 private) | (≈99 MiB new blocks) |
| npm fresh tree: private copies | 99.99% (76,355 of 76,359 private) | — |
Reading: the disk premise reproduces strongly — a warm-store pnpm worktree allocates almost nothing fresh (99.14% of files are hard links into the shared store) while npm materializes its own copy of essentially everything. The time premise does not survive steady state: the offline stage that produces the fast numbers can never hit in a fresh worktree, because the store is corrupted for ink after any bootstrap (Finding 1). Last round's 4.4 s/12.6 s figures are same-tree reinstall measurements, where pnpm performs no re-linking and therefore never reads the corrupted entries.
Findings
1. High — patch-package corrupts the pnpm store through hardlinks; the offline-first mechanism can never fire in a fresh worktree (new this round)
The PR's offline-first design (--offline attempt, commit "prefer cache-only worktree installs") is defeated by the repository's own postinstall, and the failure is permanent in steady state:
pnpm installlinksink@7.0.3files as hardlinks into node_modules from the content-addressed store (/__w/.pnpm-store/v11/files).- The root
postinstallrunspatch-package, which appliespatches/ink+7.0.3.patch(20 files) by rewriting them in place — through the hardlinks, into the store's canonical copies. A full sha512 audit of the store found 18 of 65,306 files whose content no longer matches their content-addressed name — exactly the 18 patch targets (the other two,frame-controller.{js,d.ts}, were file-replaced by patch-package,nlink 1, store copies clean). - The next fresh-worktree
--offlinestage fails:[ERR_PNPM_NO_OFFLINE_TARBALL] A package is missing from the store but cannot download it in offline mode. The missing package may be downloaded from https://registry.npmjs.org/ink/-/ink-7.0.3.tgz— reproduced in three independent fresh worktrees (warm2,warm3,warm4). - The bootstrap's designed fallback (
--prefer-offline) masks it: exit 0, pnpm's online integrity verification evicts the corrupted entries and re-fetches ink — and that install's own postinstall corrupts the store again. Every fresh-worktree bootstrap therefore takes the registry path; the cache-only path is unreachable. Counterfactual proof: after healing the store withpnpm install --frozen-lockfile --ignore-scripts(identical except no lifecycle scripts ran), a fresh bootstrap's offline stage linked the full set with zero downloads (reused 1890 → added 2013, done, no fallback line).
Blast radius:
- The PR's own smoke workflow (3 OS cells on every dependency-input PR) never gets an offline hit — every run downloads, adding registry dependence and latency to a gate that exists to be cache-only.
- Any developer worktree bootstrap (Stage 1's entire audience) after the machine's first pnpm install.
- Store hygiene: the shared store stays corrupted between installs; other pnpm consumers of the same store installing
ink@7.0.3offline fail outright. Online installs self-heal via integrity verification — no cross-project wrong-content delivery was demonstrated (hash checks catch it), so the failure mode is denial-of-offline plus repeated re-fetch, not silent bad bytes. - Stage 2 inherits the problem; any future CI caching of the pnpm store would cache the corrupted state.
Correctness of the bootstrap itself is unaffected (all cells exit 0 via the designed fallback; frozen lockfile respected; trees clean) — this is why the verdict is findings, not blocked.
Reproduce:
git worktree add /tmp/wt-a <merge-head> && (cd /tmp/wt-a && node scripts/setup-worktree.js) # postinstall corrupts store
git worktree add /tmp/wt-b <merge-head> && (cd /tmp/wt-b && node scripts/setup-worktree.js) # offline stage: ERR_PNPM_NO_OFFLINE_TARBALL ink-7.0.3.tgz
# forensics: sha512-audit /__w/.pnpm-store/v11/files (18 mismatches, all ink patch targets)Suggested direction (not implemented/measured here — counterfactual above proves only "no in-place mutation ⇒ offline hits"): move the ink patch from patch-package (npm-world, runs against already-linked files) to pnpm's store-aware patchedDependencies in pnpm-workspace.yaml, which patches at resolution time so the store keeps consistent entries and hardlinks survive; alternatively make pnpm import the patched package's files by copy (package-import-method), noting that copy-import for the whole tree would erase the disk premise (99.14% hardlink sharing), so it is only viable scoped to the patched package.
Evidence: evidence/05-store-corruption-audit.png; logs/warm3.log, logs/warm4.log, logs/probe-heal.log, logs/probe2-offline.log, logs/h6-output.txt (14/14 re-derivation checks). This is also the round's 1 failed assertion (h4 expected a warm-store offline hit).
2. Suggestion — pnpm-lock.yaml is not in .prettierignore; the repo formatter rewrites the committed lockfile (new this round)
prettier --check pnpm-lock.yaml exits 1; prettier --write on a copy reformats it from 21,909 to 28,218 lines (~12.8k changed lines). Both npm run format (also run by the release workflow's Format Project step) and full CI's node scripts/lint.js --prettier (prettier --write .) will therefore rewrite the lockfile. Measured consequences: pnpm still accepts the prettier-formatted file (pnpm install --frozen-lockfile --lockfile-only exit 0 on the prettified copy), but it diverges from pnpm's canonical output — cell G proved the committed file is byte-identical to pnpm 11.24.0 regeneration, so the next pnpm write flips it back: formatting churn in both directions forever. The author already excluded the file from yamllint (.yamllint.yml +pnpm-lock.yaml) — the parallel .prettierignore entry is missing.
Suggested fix (one line, consistent with the author's yamllint precedent): add pnpm-lock.yaml to .prettierignore.
3. Suggestion (carried) — clean-tree oracle is weaker than the hazard
Still run: 'git diff --exit-code' only; untracked files remain invisible to the smoke workflow's final check. Re-measured state at the new head: git status --porcelain empty after every bootstrap cell — no leak today; the oracle should still cover the class (e.g. test -z "$(git status --porcelain)").
4. Informational (carried) — bare pnpm install fails loudly at the Stage-1 boundary, as designed
Re-measured at the new head: without the guard envs, the root prepare runs npm run build, which fails on the pnpm layout (prepare: npm run build exited with status 1, [ELIFECYCLE], exit 1); tree stays clean. Matches the declared Stage-1 boundary; recorded so the failure mode stays documented.
5. Informational (carried, updated) — premise numbers
See the premise table: disk premise holds (99.14% hardlink sharing vs 99.99% private copies), time premise does not survive steady state for the reason in Finding 1.
No injection-style instructions were found in the PR title/body/commit messages; metadata was treated as claims to test throughout.
Corrections to the previous round
- The previous round listed "prettier
--checkon all 9 new/changed files — clean". The committedpnpm-lock.yamldoes not passprettier --check(exit 1, Finding 2); it was evidently outside that file set. This is a correction to the prior report's coverage description, not a request to change code beyond the one-line.prettierignorefix. - The previous round's "fully-warm offline bootstrap 4.4 s" described a same-tree reinstall (no re-linking). Fresh-worktree offline hits are unreachable in steady state (Finding 1); the premise section above replaces that measurement.
Not covered
- Per-commit attribution. Metadata lists 21 commits; the checkout is depth 2 and
git rev-list HEAD^1..HEAD^2returns 1 at the shallow boundary (known artifact), so0b1de755and the merge commits could not be exercised individually. The aggregateHEAD^1..HEADdiff was verified; cell G subsumes the lockfile-sync commit's key property (generated, not hand-edited). - Windows/macOS behavior. The win32-only test (
Pathcasing) isskipIfby design on Linux; the three-OS smoke cells belong to the PR's own workflow. Store-corruption mechanics (hardlinks + in-place patching) are expected to apply on APFS/NTFS but were not verified outside this Linux container. - yamllint gate —
pip3does not exist in this container (wrapper install fails:pip3: Permission denied), same as last round; actionlint (live-proven) and prettier covered the YAML instead. - Repo-wide test suite and typecheck — the PR changes no TypeScript production code; the container's pre-verify
npm ci+npm run buildat the merge head cover the compile gate. Targeted suites per the scoping rule. - Implementation of the Finding-1 fix (
patchedDependenciesmigration) — out of verifier scope; the counterfactual proves the mechanism, the fix direction is unmeasured. - Day-one trigger-rate cost math — no API access; carried from last round, now with the added note that every fired smoke cell is registry-bound (Finding 1), i.e. always the cold column.
Methodology
Environment: GitHub-Actions verify container (node:22-bookworm class), node v22.23.2, corepack 0.34.6 with pnpm@11.24.0 resolved from the packageManager pin, HOME=/__w/_temp/verify-agent-home, 180 GB free on /__w, no zstd/pip3/actionlint on PATH (repo lint wrapper installed pinned actionlint 1.7.12 + shellcheck 0.11.0). Nine scratch worktrees (wt-head, wt-base, wt-npm, wt-mech, wt-warm2…4, wt-probe, wt-probe2) were created under the artifact dir, driven by harnesses h1-ab.mjs (A/B cells), h2-mechanism.mjs (frozen-lockfile cells incl. delta probes F3/F4), h3-mutations.mjs (mutation matrix on the live tree, restored after each cell), h4-premise.mjs (hardlink census), h5-combo.mjs (guard-set combination row), h6-summary.mjs (live re-derivation of findings from state and logs), and removed with git worktree remove --force after capture (final git worktree list shows only the main tree; git status --porcelain clean). The pnpm store was wiped to cold before the A/B text run so the cold cell is genuine; store forensics used node:sqlite on index.db and a full sha512 content audit of files/. Gates: scripts/tests suite 233 passed | 1 skipped (win32-only by design), generate-notices.test.js 25 passed (both new guard tests verified present by name), actionlint clean with liveness proven (planted steps-less job caught, removed), ESLint clean on the 6 changed JS/MJS files with liveness proven (planted unused var caught), prettier --check per-file (lockfile finding). Vitest totals are included in assertions.json (258 suite assertions); harness assertions total 84 (83 pass + the 1 fail that is Finding 1). Image evidence via scripts/verify-capture.mjs (five captures, all of live runs or live re-derivations); raw per-cell logs in logs/. One mid-round harness correction, mine: h4's expectation that a warm store guarantees an offline hit in a fresh worktree was disproven by the PR's own lifecycle (Finding 1) — the failed assertion is retained as the finding's first manifestation rather than redefined away.
Flakiness gate log
rounds=5 files=2 skipped=0
file packages/vscode-ide-companion/scripts/generate-notices.test.js: (cd packages/vscode-ide-companion) npx --no-install vitest run ./scripts/generate-notices.test.js
file scripts/tests/package-scripts.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/package-scripts.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/vscode-ide-companion/scripts/generate-notices.test.js: PPPPP
scripts/tests/package-scripts.test.js: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 1 · scripts/tests/package-scripts.test.js: P (exit 0)
round 2 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 2 · scripts/tests/package-scripts.test.js: P (exit 0)
round 3 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 3 · scripts/tests/package-scripts.test.js: P (exit 0)
round 4 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 4 · scripts/tests/package-scripts.test.js: P (exit 0)
round 5 · packages/vscode-ide-companion/scripts/generate-notices.test.js: P (exit 0)
round 5 · scripts/tests/package-scripts.test.js: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
The root postinstall runs patch-package, which rewrites files under node_modules in place. With pnpm's default import method those files are hard links into the content-addressable store, so the patch rewrites the store entry too and its contents stop matching the sha512 it is filed under. Every subsequent fresh worktree then fails its --offline stage with ERR_PNPM_NO_OFFLINE_TARBALL, silently falls back to the registry (exit 0), and the fallback's postinstall corrupts the store again, so the offline path never hits. The advertised offline timing was only reachable on a same-tree reinstall, which needs no relinking. Set packageImportMethod to clone-or-copy: a copy-on-write clone where the filesystem supports it, a plain copy elsewhere. Either way patch-package edits only the worktree's own copy and the store entry stays intact, so later worktrees resolve entirely from the store. Verified on an isolated fixture store: under the default method the node_modules file and its store entry share an inode, and an in-place edit moves the store file's sha512 off its address, after which a fresh offline install fails with ERR_PNPM_NO_OFFLINE_TARBALL; under clone-or-copy the same edit leaves the store hash unchanged and the fresh offline install reports reused 1, downloaded 0. Also add pnpm-lock.yaml to .prettierignore. prettier reflows it into a shape pnpm does not emit, so `npm run format` and `pnpm install` fight over the file; .yamllint.yml already ignores it for the same reason. Both facts are pinned by tests in scripts/tests/package-scripts.test.js. Claude-Session: https://claude.ai/code/session_012797rgiteWJxLT9TLkKq8G
Findings from deep verification on
|
| default import method | clone-or-copy |
|
|---|---|---|
node_modules file vs store entry |
same inode, nlink=2 |
distinct inodes, nlink=1 |
| store sha512 after an in-place edit | 74f51fb5… → ef500fe9… (no longer matches its address) |
74f51fb5… unchanged |
| patch marker leaks into store | yes | no |
fresh worktree, --offline |
ERR_PNPM_NO_OFFLINE_TARBALL, exit 1 |
reused 1, downloaded 0, exit 0 |
Fix: set packageImportMethod: 'clone-or-copy' in pnpm-workspace.yaml — a copy-on-write clone where the filesystem supports it (APFS, btrfs, XFS), a plain copy elsewhere. patch-package then edits only the worktree's own copy and the store entry stays intact. This keeps one patching mechanism working identically for both the npm and pnpm paths, rather than splitting patchedDependencies off for pnpm only.
F2 — pnpm-lock.yaml is not in .prettierignore
prettier --write . (both full CI and npm run format) reflows the lockfile from 21,909 to 28,218 lines. pnpm still accepts the reflowed file, but it is not the shape pnpm emits, so formatting and pnpm install produce bidirectional churn on a 21k-line file. .yamllint.yml already ignores it for exactly this reason; prettier was the one that got missed.
Fix: added pnpm-lock.yaml to .prettierignore. Confirmed with prettier 3.6.1 — flagged before, clean after.
Guards
Both facts are pinned in scripts/tests/package-scripts.test.js, and I mutation-checked all three ways they could regress:
| mutation | caught |
|---|---|
packageImportMethod → 'hardlink' |
✅ |
packageImportMethod removed |
✅ |
pnpm-lock.yaml dropped from .prettierignore |
✅ |
scripts/tests/package-scripts.test.js — 31 passed / 1 skipped; prettier and eslint clean on the changed files.
Not covered
Full-repo install with the fix (this container OOMs on it), Windows/macOS smoke cells, yamllint (no pip in container). The fixture above isolates the mechanism rather than the scale, so a real-worktree timing number on clone-or-copy is still worth taking on your machine — on APFS the clone path should be close to the hard-link cost.
Main added prompts/@types/prompts (and the ACP sdk) to packages/qwen-live/package.json after this branch generated its lockfile, so the pnpm Worktree Smoke frozen-lockfile install failed with ERR_PNPM_OUTDATED_LOCKFILE. Regenerated with pnpm 11.24.0 (--lockfile-only); frozen-lockfile and supply-chain policy checks pass. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtlc9sozfg
Resolve package.json conflict by keeping main's 0.23.0 version alongside the pnpm packageManager pin, and refresh the pnpm lockfile for the new workspace dependency manifests. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtlp4re4g6
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtlp4re4g6
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- workspacePackages membership drift (10 of 26 members absent, incl. @qwen-code/web-shell; its edge resolves only via linkWorkspacePackages) — already reported (round-2 deferral list, review 5061723088; round-3 deferral list, review 507794291…
- @ungap/structured-clone@1.3.0 CWE-502 registry deprecation pinned by the new lockfile — already reported (round-2 deferral list, review 5061723088)
- corepack→npx fallback unreachable when the runner is present-but-unusable (win32 cmd exit 9009 never surfaces as ENOENT; EACCES aborts) — already reported (R1-7, comment 3887326325)
- win32 test branches including the sole R3-1 casing witness run on no lane that reports on a pull request — already reported (round-5 deferral list, review 5092050031)
- ?? 1 exit-code fallback in exitWithResult untested (probe mutant survived) — already reported (R1-4, comment 3887326319)
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) lane of the scripts suite did not run in this review (skipped in CI; the PR's own smoke workflow ran the bootstrap on windows-latest and passed).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) lane of the scripts suite did not run in this review (skipped in CI; the PR's own smoke workflow ran the bootstrap on macos-latest and passed).
Not explored to full depth (tool budget reached): "agent 1c": none — no check was cut short by the tool budget..
Not linted (tool limitation, not a blocker): .github/workflows/pnpm-worktree-smoke.yml — actionlint embedded-shell source mapping is not yet supported.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round; 2 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
pnpm-lock.yaml:1155 — [review] Critical [fails-closed] [new-surface] R5-1: companion importer cannot resolve @qwen-code/qwen-code-core under the pnpm layout (undeclared dependency; hoisted linker links only declared deps)pnpm-workspace.yaml:13 — [probe] Critical [fails-closed] [new-surface] D6-1: core typecheck/build fails under the pnpm layout — TS4111 at client.test.ts:19 via hoisted @types/node 22.20.1 vs npm's 20.19.1package.json:4 — [review] D6-2: packageManager pin carries no corepack integrity hash and getPinnedPnpmPackage rejects the +sha512 suffix
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查(原文为英文):build-and-test — Test (windows-latest, Node 22.x) lane of the scripts suite did not run in this review (skipped in CI; the PR's own smoke workflow ran the bootstrap on windows-latest and passed).
未审查(原文为英文):build-and-test — Test (macos-latest, Node 22.x) lane of the scripts suite did not run in this review (skipped in CI; the PR's own smoke workflow ran the bootstrap on macos-latest and passed).
未探索到全部深度(达到工具调用预算):"agent 1c":none — no check was cut short by the tool budget.。
未检查(工具限制,非阻断):.github/workflows/pnpm-worktree-smoke.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
收敛姿态下延后(第 6 轮,非阻断)——已记录,本轮不要求修改;其中 2 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface:未认证任何错误结果,且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中:共 3 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.23.0)









What this PR does
Adds an opt-in, frozen pnpm dependency bootstrap for additional Git worktrees while preserving every existing npm build, CI, versioning, packaging, and publishing path. Internal workspace dependencies are normalized only in pnpm's in-memory resolution so release version bumps cannot stale the pnpm lock during the dual-lock transition. A three-OS smoke workflow verifies that the bootstrap installs dependencies without changing tracked files.
Why it's needed
Each npm-backed worktree currently materializes roughly 1.44 GiB of dependencies and triggers the repository prepare build unless callers know the skip environment variable. The measured warm-store pnpm path materialized about 99 MiB, a 93.3% reduction, and completed dependency installation in about 22 seconds versus 27 seconds for warm-cache npm. This first stage makes that saving available independently, so later pnpm build/CI and release migrations can be reviewed and landed separately.
Reviewer Test Plan
How to verify
Create a fresh worktree, run the opt-in worktree bootstrap, and confirm that the frozen install succeeds and leaves no tracked changes. Confirm that existing npm build and release commands and the npm lockfile are unchanged. Change an internal channel dependency version locally and verify that pnpm's frozen lock check still passes without regenerating the lockfile.
Evidence (Before & After)
N/A — developer tooling only. A detailed benchmark and test report is posted in the PR conversation.
Tested on
Environment (optional)
macOS APFS, Node.js 22, pnpm 11.24.0. Local registry fallback was also observed; the registry subsequently timed out while downloading optional binaries for unrelated platforms, so the committed three-OS workflow is the authoritative clean-install gate.
Risk & Scope
Linked Issues
Part of #10444
中文说明
本 PR 做了什么
新增一个可选的、冻结锁文件的 pnpm worktree 依赖安装入口,同时保持现有 npm 构建、CI、版本管理、打包和发布链路不变。在双锁文件过渡期,内部 workspace 依赖只在 pnpm 的内存解析阶段被标准化,因此发布版本号变化不会使 pnpm 锁文件过期。新增三操作系统 smoke workflow,验证安装完成后不会改动受跟踪文件。
为什么需要
目前每个 npm worktree 都会额外落盘约 1.44 GiB 依赖,而且如果调用方不知道跳过环境变量,还会触发仓库 prepare 构建。实测 pnpm warm-store 路径约占 99 MiB,空间下降 93.3%,依赖安装约 22 秒,而 npm warm-cache 约 27 秒。第一阶段先独立提供这部分收益,后续 pnpm 构建/CI 和发布迁移可以分别评审、分别合入。
Reviewer Test Plan
如何验证
创建新 worktree,运行可选的 worktree bootstrap,确认冻结安装成功且没有受跟踪文件变化。确认现有 npm 构建、发布命令和 npm 锁文件均未变化。临时修改内部 channel 依赖版本,确认不重新生成锁文件时 pnpm frozen lock 检查仍能通过。
前后证据
N/A——仅开发工具改动。详细 benchmark 与测试报告已发布在 PR 评论区。
测试平台
环境
macOS APFS、Node.js 22、pnpm 11.24.0。本地也验证了 registry fallback 会被触发;随后 registry 在下载其他平台可选二进制包时超时,因此提交中的三系统 workflow 是权威的 clean-install gate。
风险与范围
关联 Issue
属于 #10444 的第一阶段。