chore(pnpm): gate pnpm-lock on package-lock and declare hoisted imports - #11625
Conversation
- Lockfile check: every pnpm-lock version must be locked in package-lock (one documented npm-lock gap, failing if it goes stale), and every dependency npm runs an install script for needs an explicit allowBuilds decision (fsevents: false records current behaviour). - Regenerate pnpm-lock with `pnpm import`, which is deterministic and drops 18 versions npm never locked; document it as the regeneration step. - Enforce import/no-extraneous-dependencies on shipped sources and declare the 15 imports that only resolved through hoisting, at already-locked versions. - Run the pnpm smoke workflow when package-lock.json changes. - Add the Stage 2 verification brief. Refs #10444
|
|
|
Thanks for the PR! Template looks good ✓ — every heading filled in, a real Tested-on table rather than a blank one, and a complete paragraph-by-paragraph Chinese translation. Problem: observed, not theoretical. The #10444 post-Stage-1 review measured the two lockfiles drifting three days after the bootstrap merged. Rather than take the numbers on faith I reproduced the drift class myself: comparing main's Direction: aligned. This hardens a Stage 1 that already landed ( Size: no core source paths — Approach: deriving the pnpm lockfile from the npm one is the right shape. It removes the hand-kept overrides that were the actual failure mode, and it makes the second gate compose correctly — because both trees now come from one source, "every npm install script has an explicit One honest question, not a blocker: this is three changes — the lockfile gates, the undeclared-import rule plus its 15 declarations, and the Stage 2 verification brief. Each is defensible and none is risky, but the lint rule in particular is an independent win that would have been easy to land and review on its own. Worth keeping in mind for next time rather than reworking now. Risk: no elevated risk signals — none of the changed files match the revert-correlated path set. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 每个标题都填了，Tested-on 表格是真实填写的而不是空表，中文翻译也是逐段完整的。 问题： 是已观测到的，不是理论性的。#10444 上 Stage 1 之后的复盘测量到，bootstrap 合入三天后两个 lockfile 就已经漂移。我没有直接采信这些数字，而是自己复现了这类漂移：把 main 的 方向： 对齐。这是在加固已经合入的 Stage 1（ 规模： 没有触及核心源码路径—— 方案： 让 pnpm lockfile 从 npm lockfile 派生，这个思路是对的。它去掉了真正的失效来源（手工维护的 overrides），也让第二个门能够正确组合——因为两棵树现在同源，"npm 的每个 install 脚本都有一条显式 一个诚恳的疑问，不是阻塞项：这里其实是三件事——lockfile 门、未声明 import 规则及其 15 处声明、Stage 2 验证说明。每一件都站得住，也都没有风险，但特别是那条 lint 规则，它是一个独立的收益，本来可以单独提交、单独 review。这点值得下次留意，现在不必返工。 风险： 无升级风险信号——改动的文件都不匹配与 revert 相关的路径集合。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI read the two lockfiles at this head directly and recomputed both new gates myself rather than reasoning about the script — so the pass/fail claims below are measurements, not a reading of the author's intent. Both gates do what they say. The version gate compares 1966 pnpm package keys against the 2012 The 15 new declarations change no resolved version. This was the thing most likely to be subtly wrong, so I checked all of them against the locked tree: The ESLint wiring is correct. Both new blocks reuse the exact Three things I'd note, none of them blocking:
No critical blockers, and no AGENTS.md violations: the script extends an existing file instead of adding a parallel one, the verification brief goes into Files changed (12)
TestingThis is an unattended CI run, so nothing was built or executed here — and per the review rules I did not run the PR's own script either. The gate results above come from recomputing both checks against the lockfiles fetched at this commit. Real evidence is the smoke workflow, which is the one thing that actually exercises the regenerated lockfile: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Read that with one caveat in mind: the three green Not verified, and why:
Sandboxed verification would settle the one claim nothing currently covers: 中文说明代码审查我直接读取了这个 head 上的两个 lockfile，自己重新算了两道新门，而不是去推断脚本的意图——所以下面的通过/失败结论是测量结果，不是对作者意图的解读。 两道门都名副其实。 版本门把 1966 个 pnpm 包 key 与 npm 锁定的 2012 个 15 处新增声明没有改变任何已解析的版本。 这是最容易出现微妙错误的地方，所以我逐一对照了锁定树： ESLint 接线是正确的。 两个新 block 复用了已有通用 block 完全相同的 有三点想指出，都不是阻塞项：
没有严重阻塞项，也没有违反 AGENTS.md 的地方：脚本是扩展已有文件而不是新加一个平行的；验证说明放进 测试这是一次无人值守的 CI 运行，所以这里没有构建或执行任何东西——按照 review 规则，我也没有运行 PR 自己的脚本。上面的门结果来自用该 commit 上取到的 lockfile 重新计算。真实证据是 smoke workflow，它是唯一真正验证了重新生成的 lockfile 的东西。 看这张表时请注意一点：三个绿色的 未验证的部分及原因：
沙箱验证可以解决目前唯一没有任何覆盖的说法： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — the central claims held up when I tried to break them; the nits are real but none of them is a reason to hold this back. Going in, my own answer to "the two lockfiles drifted and hand-kept overrides can't keep up" was the same shape as this: stop resolving the pnpm lockfile independently, derive it from the npm one, and assert in CI that the pnpm set is a subset of the npm set. So the approach isn't just acceptable, it's the one I'd have proposed. It beats my version in two places I wouldn't have thought of. The The simpler alternative I considered and rejected was deleting the pnpm bootstrap outright. It's genuinely less code, but #10444 is still open specifically for a Stage 2 go/no-go, and ripping out a merged Stage 1 would preempt a decision the maintainer reserved. This PR keeps that decision open while removing the reason the interim state was unsafe. What I'd want a reader to take away is that I did not take the description's word for anything. I recomputed both gates from the lockfiles at this commit: the version gate leaves exactly one unmatched entry out of 1966, and it's the declared exception; the build gate covers all seven packages npm flags. Both documented failure cases reproduce. All 15 new manifest declarations match versions already locked, including the six OpenTelemetry packages that turn out to have been nested under Three reservations, in the order I'd weight them:
The set-membership limitation is worth restating once here because it's the thing most likely to be over-trusted later: this gate proves a pnpm worktree never runs a version CI hasn't locked, not that it runs the same one. esbuild is the concrete case — 0.25.6 under npm at the root, 0.25.12 under pnpm, and 0.25.6 absent from the pnpm lockfile entirely. The author discloses this, it's bounded by npm staying authoritative for build and release, and the brief already names the measurement that would retire the question. Disclosed-and-bounded is the right disposition, not a defect. Six months from now this reads as a thank-you, not a curse: 105 lines of set arithmetic appended to an existing script, no new abstraction, no config surface, and it converts "nobody noticed the drift for three days" into a red check. One note for the record rather than as a concern — a maintainer's follow-up to their own Stage 1 and their own issue is exactly the shape where a gate should not relax, so I verified the claims instead of accepting them; they happened to hold. Approving, with the approval deferred until CI lands green on 中文说明Confidence: 4/5 —— 我试着推翻它的核心论断，结果都站得住；那些小问题是真实存在的，但没有一个构成拦下它的理由。 一开始，我对"两个 lockfile 漂移了、手工维护的 overrides 跟不上"这个问题给出的答案，与这个 PR 是同一个形态：不要再独立解析 pnpm lockfile，而是从 npm 的那份派生，并在 CI 里断言 pnpm 集合是 npm 集合的子集。所以这个方案不只是可接受，它就是我本来会提的方案。它在两个我不会想到的地方比我的版本更好： 我考虑过但否掉的更简方案是直接删掉 pnpm bootstrap。它确实代码更少，但 #10444 仍然开着，就是为了 Stage 2 的 go/no-go 决定；把已经合入的 Stage 1 拆掉，等于抢在维护者保留的那个决定之前行动。这个 PR 让那个决定继续悬着，同时消除了这段中间状态之所以不安全的原因。 我希望读者带走的一点是：描述里的任何说法我都没有直接采信。我用这个 commit 上的 lockfile 重算了两道门：版本门在 1966 项里只留下一项未匹配，而它正是声明的例外；构建门覆盖了 npm 标记出的全部七个包。两种文档化的失败情形都能复现。15 处新增 manifest 声明全部对应已锁定的版本，包括那六个 OpenTelemetry 包——它们其实一直嵌套在 三点保留意见，按我的权重排序：
集合成员判断这个局限值得在这里再说一次，因为它最可能在以后被过度信任：这道门证明的是 pnpm worktree 不会运行 CI 没锁定的版本，而不是运行的是同一个版本。esbuild 是具体例子——npm 根目录 0.25.6，pnpm 0.25.12，而 0.25.6 在 pnpm lockfile 里完全不出现。作者披露了这一点，它的影响范围受限于 npm 仍是构建与发布的权威路径，而且验证说明里已经点名了能了结这个问题的测量。"已披露且有边界"是正确的处置方式，不是缺陷。 六个月后再看，这是一份让人道谢而不是骂人的改动：105 行集合运算追加到已有脚本里，没有新抽象，没有配置面，并且把"漂移了三天没人发现"变成一条红色检查。有一点写进记录、但不是顾虑——维护者跟进自己的 Stage 1、自己的 issue，正是门不应该放松的形态，所以我去验证了这些论断而不是接受它们；结果它们成立。 同意合入，但批准推迟到 CI 在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
- NOTICES.txt: core's new OpenTelemetry declarations make the generator visit six Apache-2.0 entries in a different order. Same 655 entries, same text; only their order changes (6 header lines). - package-scripts test: expect package-lock.json in the pnpm smoke paths.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Agent-assisted review at 0c67d4872a0c60b8f6d29918f28170a6a72ad0a5 — no confirmed Critical; full current tooling/manifest/lockfile scope reviewed.
Independent data validation against this head (Python 3 + preinstalled PyYAML 6.0.1, not execution of PR code) found 2,012 unique npm name/version pairs and 1,966 pnpm package entries. The only pnpm version absent from npm is the documented mime-db@1.52.0 exception. All seven npm install-script package names have an allowBuilds decision. All 13 newly explicit dependency declarations match their npm workspace lock entries; their current npm resolution and pnpm importer version agree, including core's nested OpenTelemetry resolutions. I traced the corresponding runtime imports and reviewed the ESLint block ordering/web-templates package boundary, smoke workflow path additions, notices diff, bootstrap guidance and verification brief.
The gate's limit matters: scripts/check-lockfile.js:149-176 checks set membership, not per-importer resolution or whole dependency-graph equivalence. The allowBuilds check at :193-207 checks package names, not matching version-qualified approvals. Likewise devDependencies: true does not certify that a separately published package has every runtime dependency. These are scope limits, not proof of a current broken install; Stage 2 build/test parity remains unverified as the brief says.
Suggestion: scripts/tests/package-scripts.test.js:156-169 only checks today's successful lockfiles and log lines. Add small negative cases for an npm-unlocked pnpm version, a stale exception and an undecided install script. Removing the corresponding failure checks should fail those tests; successful current-data validation alone cannot protect those branches.
Prior history contains no standing Critical to close. This is tooling/manifests rather than a large core refactor; broad package impact warrants maintainer awareness, not a size-based rejection. Maintainer authorship was not independently verified.
Testing: the independent read-only lock/manifest validation above passed. No PR script, ESLint, package tests, install, build, lock regeneration, or workflow was executed. In particular I did not follow the verification brief's workflow-dispatch instructions. Exact remote head/base and paginated current diff/history used. Comment only; no approval implied.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": pnpm install --frozen-lockfile was never executed (no pnpm binary and no registry access in this review worktree, and a real install would dirty the shared tr…; "agent reverse-audit (round 1)": I did not execute a real pnpm install --frozen-lockfile , so registry reachability of the 1966 recorded integrity values is inferred from their byte-equality w…; "agent reverse-audit (round 2)": verifying whether packages/sdk-typescript 's build bundles or externalizes @qwen-code/acp-bridge / @qwen-code/qwen-code-core (no esbuild config at the pack….
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Not linted (tool limitation, not a blocker): .github/workflows/pnpm-worktree-smoke.yml — actionlint embedded-shell source mapping is not yet supported.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：pnpm install --frozen-lockfile was never executed (no pnpm binary and no registry access in this review worktree, and a real install would dirty the shared tr…；"agent reverse-audit (round 1)"：I did not execute a real pnpm install --frozen-lockfile , so registry reachability of the 1966 recorded integrity values is inferred from their byte-equality w…；"agent reverse-audit (round 2)"：verifying whether packages/sdk-typescript 's build bundles or externalizes @qwen-code/acp-bridge / @qwen-code/qwen-code-core (no esbuild config at the pack…。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
未检查（工具限制，非阻断）：.github/workflows/pnpm-worktree-smoke.yml——actionlint 对 workflow 内嵌 shell 的源映射尚未支持。
— qwen3.8-max via Qwen Code /review (v0.23.3)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
- fail closed when pnpm-lock.yaml has no packages section instead of printing the strongest success line over an unread graph - match git/file: dependency keys against npm's resolved field, whose pnpm key is the source rather than a version - count an allowBuilds entry only when its value is boolean, so pnpm's own 'set this to true or false' placeholder no longer passes the gate - drop the inert package-lock.json trigger from the pnpm smoke workflow; no step in that job reads it, and npm/pnpm agreement is already gated by ci.yml's Check lockfile step on the full profile - stage-2 evidence recipe: cover nested workspace roots in the reflink arm, sample peak worktree count at most 5 minutes apart, run the parity matrix in a throwaway workflow, and give the npm baseline the same skip guards the pnpm arm has - drive every gate branch with fixture-based negative tests
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix round — PR #11625Commit DispositionsImplemented (8 inline findings + the maintainer review-body suggestion):
Partially implemented (thread left open):
Escalated (maintainer decision needed, thread left open):
Deferred to follow-up:
Mutation probes (all restored after measurement)
Verification
中文说明Autofix 本轮处理 — PR #11625在 处理结论已实现（8 条行内发现 + 维护者 review 正文建议）：
部分实现（讨论串保持打开）：
已升级（需维护者决策，讨论串保持打开）：
已推迟到后续跟进：
变异探针（测量后均已还原）
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 8 selected review thread(s). · 已关闭全部选中的 8 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
PR #11625 — local verification on macOSTwo heads are covered, because AutoFix pushed
The delta between the two touches only Everything ran on a real macOS/APFS host in isolated worktrees of this branch: one installed Environment: macOS 26.6.2 arm64 · Node v24.18.1 · npm 11.16.0 · corepack pnpm 11.24.0. VerdictRecommend merge, with one thing worth fixing first (F1) and a description line that is now CI right now
1. The gates — 9 cases, run live at
|
| case | mutation | expected | actual |
|---|---|---|---|
| M0 | none: head as committed | pass | pass — the four promised lines |
| M1 | main's pnpm-lock.yaml put back |
fail | fail — 18 unlocked versions, the exact list in the PR body |
| M2 | fsevents deleted from allowBuilds |
fail | fail — names fsevents |
| M3 | canary: one fabricated version in pnpm-lock.yaml |
fail | fail — names fake-canary-package@9.9.9 |
| M4 | canary: npm starts locking mime-db@1.52.0 |
fail | fail — "remove these entries from knownNpmLockGaps" |
| M5 | canary: a new install-script dependency lands | fail | fail — names canary-native-thing |
| M6 | today's origin/main package-lock.json (post-merge) |
pass | pass |
| M7 | canary: pnpm-lock.yaml with no packages: section |
fail | fail — "has no packages section" (new in 1e43a5b2) |
| M8 | canary: allowBuilds placeholder string instead of a boolean |
fail | fail — names esbuild (new in 1e43a5b2) |
M3–M5 matter because "M0 passes, M1 fails" is also what a gate that only consults one hard-coded
list would produce. M4 exercises the staleness half of knownNpmLockGaps, which nothing in the
original test plan touched. M7 and M8 are the two fail-open holes 1e43a5b229 closed; both now
fail closed outside the test harness too.
scripts/tests/package-scripts.test.js at 1e43a5b229: 47 passed, 1 skipped (48), including
all seven new fixture-driven failure-branch tests.
M6 answers the question a maintainer actually has at merge time: main has moved 31 commits past
the merge-base and one of them (4ec9f63d0a) touched package-lock.json. The gate still passes
against today's main, and running corepack pnpm import against today's main
package-lock.json produces a pnpm-lock.yaml byte-identical to the one committed here — so
this lockfile is not already stale.
Recomputed independently. Rather than trust the script, I reimplemented both gates separately
and compared: 1,966 package keys in pnpm-lock.yaml, exactly one not locked by npm —
mime-db@1.52.0, the documented gap. Against the base pnpm-lock.yaml: 18, matching the PR's
list item for item. The residual drift the PR discloses also checks out — of 422 direct workspace
dependencies, 409 resolve identically and 13 differ, exactly the number claimed, and every
pnpm-side version is locked somewhere in npm's tree.
the 13, and the two of them that are runtime dependencies
. esbuild npm 0.25.6 pnpm 0.25.12
integrations/external-context-mem0 esbuild npm 0.25.6 pnpm 0.25.12
packages/chrome-extension esbuild npm 0.25.6 pnpm 0.25.12
packages/vscode-ide-companion esbuild npm 0.25.6 pnpm 0.25.12
packages/cli @types/node npm 22.19.19 pnpm 22.20.1
packages/mobile-mcp @types/node npm 22.20.0 pnpm 22.20.1
packages/qwen-live @types/node npm 22.19.19 pnpm 22.20.1
packages/sdk-typescript @types/node npm 22.19.19 pnpm 22.20.1
packages/vscode-ide-companion @types/node npm 22.19.19 pnpm 22.20.1
packages/web-shell @types/node npm 22.19.20 pnpm 22.20.1
packages/cli @types/semver npm 7.7.0 pnpm 7.7.1
packages/core fdir npm 6.4.6 pnpm 6.5.0
packages/core picomatch npm 4.0.4 pnpm 4.0.5
Eleven of the thirteen are build or type tooling. The two that are not are fdir and
picomatch, both in packages/core's dependencies: a pnpm worktree runs core against
6.5.0 / 4.0.5 where CI runs 6.4.6 / 4.0.4. npm locks those versions too (under vite and
tinyglobby), so the gate is right to pass — but that is the concrete shape of the accepted
parity gap, worth having on record next to the esbuild example the review already gave.
2. macOS — the ⚠️ row
corepack pnpm importreproduces the committed lockfile byte-for-byte. Deleting
pnpm-lock.yamland regenerating it frompackage-lock.jsonproduced a file identical to the
committed one, in 9s. The PR claimed only that two consecutive imports match each other, on
Linux; the stronger property — that the committed file is exactly what the documented command
produces, on a second OS — holds. The regenerated file then passes--frozen-lockfile.- The three assertions of the smoke job reproduce: the frozen install exits 0 — 1m12s from an
empty tree, 4s when re-verifying an installed one, which is what the figure shows — the
workspace link resolves inside the worktree, and the bootstrap leavesgit statusclean. The
first--offlineattempt on the cold tree falls back to--prefer-offlineon one missing
metadata entry (@node-rs/jieba-android-arm-eabi), the same cold-cache path the review already
identified onmain. fsevents: falseis safe, for a sharper reason than either the code comment or the review
gives — see F5.
3. The lint rule
npm run lint:ci passes on both trees (npm 129s, pnpm 119s) — 0 violations of the new rule, which
is the run the author deferred to CI.
Two mutations show both new config blocks are load-bearing:
- Reverting the three manifests to their base versions surfaces 30 import sites across 13
(package, dependency) pairs — exactly the set the PR declares, and no other rule fires. (The
body says "15 imports"; sites are 30 and declarations are 13. Cosmetic.) - Deleting the
web-templatespackageDirblock surfaces 13 sites in the two build sub-projects,
so that block is doing real work rather than documenting an intention.
All 13 new declarations resolve to a version already locked — checked against package-lock.json
with semver.satisfies, including the one that is not an exact match (semver ^7.7.2 → locked
7.8.5). The published CLI manifest is untouched by construction: scripts/prepare-package.js
writes dependencies: {} into dist/package.json.
4. Stage 2 brief, measurement 2 — hoisted parity
The brief asks for the CI matrix on a pnpm-installed hoisted tree. I ran the list it names on both
trees on this host, substituting test:scripts for the full test:ci:
| step | pnpm tree | npm tree |
|---|---|---|
npm run build |
PASS 193s | PASS 198s |
npm run typecheck |
FAIL 59s | PASS 79s |
npm run lint:ci |
PASS 119s | PASS 129s |
npm run test:scripts |
FAIL 763s — 2 failed / 2406 passed | FAIL 721s — 3 failed / 2405 passed |
npm run bundle |
PASS 4s | PASS 3s |
npm run check:serve-fast-path-bundle |
PASS 193s | PASS 181s |
The two test:scripts failures are the same pair on both installers
(qwen-autofix-workflow.test.js, one of them a 50s timeout); the npm tree adds a third
(install-script.test.js, a network-shaped assertion). None of them touches this diff, and pnpm
is not the worse of the two — so test:scripts is a parity pass with a host-environment asterisk.
So the brief's premise — "npm scripts run unchanged on a hoisted pnpm tree" — holds for build,
lint, bundle and the fast-path check, and not for typecheck. See F4.
5. Stage 2 brief, measurement 1 — per-worktree disk on APFS
Filesystem APFS, warm pnpm store, three fresh worktrees of the same commit, df deltas (the
brief is right that du is useless here — it reports 1.2 GiB for the tree that actually cost
40 MiB). A 20-second sleep control measured −1 MiB of background noise.
| method | command in the new worktree | df delta |
wall | du -sh |
|---|---|---|---|---|
| npm | QWEN_SKIP_PREPARE=1 QWEN_SKIP_NOTICE_GENERATION=1 npm ci --prefer-offline |
1,398 MiB | 28s | 982 MiB |
| pnpm | node scripts/setup-worktree.js |
40 MiB | 23s | 1.2 GiB |
| reflink copy | cp -c -R of the primary's node_modules + every workspace root's own |
64 MiB | 24s | 1.0 GiB |
All three of the brief's link checks pass on the reflink arm, including the one
1e43a5b229 added:
readlink node_modules/@qwen-code/qwen-code-core -> ../../packages/core
require.resolve core -> ./packages/core/package.json
require.resolve @larksuiteoapi/node-sdk (feishu) -> ./packages/channels/feishu/node_modules/…
Two things this settles for the APFS/developer-Mac row:
- The 93% saving the brief quotes is, if anything, understated here — 1,398 → 40 MiB is 97%.
- The reflink copy gets 95% of that saving without pnpm (64 MiB vs 40 MiB, a 24 MiB gap on a
1.4 GiB baseline). The brief names that as the comparison that decides whether disk alone
justifies Stage 2; on this filesystem, it does not.
Findings
F1 — the new git-dependency exemption does not fire for a real git dependency
1e43a5b229 adds a branch that exempts a pnpm key matching npm's resolved for a git/file:
dependency, and a test for it. The test writes both sides itself, so it only proves the exemption
works when the two strings are equal. They are not equal in practice:
root devDependency: "is-plain-obj": "github:sindresorhus/is-plain-obj#v4.1.0"
npm resolved : git+ssh://git@github.com/sindresorhus/is-plain-obj.git#68e8cc77…
pnpm packages key : is-plain-obj@https://codeload.github.com/sindresorhus/is-plain-obj/tar.gz/68e8cc77…
$ node scripts/check-lockfile.js → exit 1
Error: pnpm-lock.yaml resolves versions that package-lock.json does not lock.
Regenerate it from package-lock.json with `corepack pnpm import`:
- is-plain-obj@https://codeload.github.com/sindresorhus/is-plain-obj/tar.gz/68e8cc77…
npm normalises the spec to git+ssh://…#<sha>; pnpm keys the codeload tarball. The set membership
test can never match, and the remedy the error prints is the command that produced the state —
whoever hits this has no way out of the loop from the message alone.
I ran the git half; the file: half has the same shape (npm anchors file: at the referring
package, pnpm keys name@file:<workspace-relative path>) but I did not test it. Latent either way:
there are zero git and zero file: dependencies in either lockfile right now, so this
blocks nothing. But the branch exists precisely for the day one lands, and as written it will not
help then. Matching on the 40-hex commit SHA that appears in both strings (or on
resolution.tarball for a gitHosted: true entry) would. A fixture test that builds each side
with the tool that writes it — rather than by hand — would have caught this.
F2 — the PR description's "Smoke trigger" bullet is now out of date
1e43a5b229 reverted package-lock.json out of pnpm-worktree-smoke.yml's path filters and
added a comment explaining why, and the scripts test now asserts its absence. The description
still says "The pnpm smoke workflow now also runs when only the npm lockfile changes." Worth
editing before merge so the description matches the branch.
F3 — the type-only exemption leaves the rule short of what an isolated layout would catch
The rule config does not set includeTypes, so type-only imports are exempt; the comment
justifies that with "they disappear at build time". True for the bundle, not for tsc: an
isolated node_modules still has to resolve those specifiers to typecheck. Turning
includeTypes: true on surfaces 3 more (package, dependency) pairs across 5 sites today:
packages/core :: @opentelemetry/sdk-trace-base declared nowhere in the repo
packages/core :: @agentclientprotocol/sdk declared by 5 sibling packages, not by core
packages/cli :: cli-spinners declared nowhere in the repo
@opentelemetry/sdk-trace-base is the sharpest: it exists only at
packages/core/node_modules/@opentelemetry/sdk-trace-base, pulled in transitively by
@opentelemetry/sdk-node, and packages/core/src/telemetry/{file-exporters,log-to-span-processor}.ts
type-import it. Same class of fragility the PR is closing, one lane over. Either flip the flag and
declare the three, or say in the comment that the exemption is deliberate and why.
F4 — npm run typecheck fails on a pnpm-installed tree; not this PR's doing
One error:
packages/acp-bridge/src/process-registry.ts(744,3): error TS2322:
Type 'string | NonSharedBuffer' is not assignable to type 'string'.
Cause: the tree root's @types/node. npm puts 20.19.1 there, pnpm's hoisted layout puts 22.20.1,
and tsc -p integration-tests/tsconfig.json maps packages/acp-bridge/src/** in through paths,
so it reads the root copy. acp-bridge's own exact pin (20.19.1) is materialised correctly under
packages/acp-bridge/node_modules in both trees — only the root slot differs. A/B:
installing main's own pnpm-lock.yaml at 642d36e gives the same root 22.20.1 and the same
error, so this predates the PR. Recorded here because it is a direct answer to one of the two
questions the new brief exists to settle, and belongs in that brief's eventual results.md rather
than being rediscovered.
F5 — fsevents: false is correct; the review's stated consequence is not
The stage=2 review says the deny "means those lose the native FSEvents binding and fall back to
fs.watch". Measured here, that does not happen:
- All three fsevents copies (2.3.3 at the root, 2.3.2 under
playwright, and a third 2.3.2 — under
mobilewrightin the pnpm tree, underpackages/mobile-mcpin the npm tree) exist in both
trees with their prebuiltfsevents.nodepresent; the root binary is byte-identical between
the two trees. - Neither tree has a
build/directory, and fsevents 2.3.3's installed tarball contains no
binding.gypand noinstallscript at all — theinstall: node-gyp rebuildthat npm's
lockfile flag reflects lives only in the registry packument, which is stale relative to the
tarball. Neither installer compiles anything; the deny denies a script that never runs. - Live probe in the pnpm tree:
require('fsevents').watch(dir, cb)delivered an event for a file
created after the watch started, from the root copy and fromplaywright's nested one.
The PR's comment ("whose install script is only node-gyp rebuild") is right about intent and
slightly off about the tarball. Worth a sentence, since this entry is the one place the PR decides
rather than records.
F6 — two small asymmetries (nits)
knownNpmLockGapshas a staleness check (M4 proves it);allowBuildsdoes not, so an entry that
stops corresponding to an npm install script lingers silently. Today the set is exact — 7 npm
install-script packages, all decided, plus one workspace entry — so this is prevention, not a live
problem. A staleness check would have to whitelist the@qwen-code/qwen-code-core@file:packages/core
entry, which is probably why it was left out; saying so in a comment would be enough.- The rule's globs are
packages/**/src/**andintegrations/**/src/**,.ts/.tsxonly. Package
entry files (packages/cli/index.ts,packages/core/index.ts),packages/*/scripts/**, and
.mjsbuild files undersrc(e.g.packages/web-templates/src/insight/vite.config.mjs, which
importsviteand@vitejs/plugin-react) stay outside it. I scanned that uncovered surface by
hand and found nothing undeclared today — a coverage note, not a defect.
What I did not run
npm run test:ci in full (CI covers it), Windows, and the disk figures on the ECS runner hosts —
the brief's item 1 wants those per host class, and this is only the developer-Mac row.
中文版（点击展开）
PR #11625 — macOS 本地验证
覆盖了两个 head，因为验证期间 AutoFix 推上了 1e43a5b229：
1e43a5b229（当前 head）—— 门的变异矩阵、package-scripts套件，以及对该提交新增的
git 依赖分支做的一次实测。0c67d4872a—— lint 部分、hoisted parity 矩阵、磁盘测量。
两者之间的 diff 只动了 scripts/check-lockfile.js、它的测试、smoke workflow 的触发路径清单和
Stage 2 说明 —— 没有动任何 manifest、任何 lockfile、任何发布出去的源码 —— 所以后一组结果原样
适用。
所有结果都跑在一台真实的 macOS/APFS 机器上，用本分支的独立 worktree：一个用 npm ci 安装
（1,979 个包，4m00s），一个用 node scripts/setup-worktree.js 安装（pnpm 11.24.0，仓库钉住的
版本，1m12s）。macOS 正是 PR 标成 docs/verification/pnpm-stage2-evidence/README.md
交接出去的两项测量，也正好是这台机器能跑的，所以两项都在下面给出。
环境：macOS 26.6.2 arm64 · Node v24.18.1 · npm 11.16.0 · corepack pnpm 11.24.0。
（CI 的 smoke 任务用 Node 22.x；下面的结论都不依赖运行时版本。）
结论
建议合入，但有一处值得先修（F1），还有一行描述已经过期（F2）。测试计划里的每一条都复现了，
包括两种写明的失败情形；我另加的金丝雀说明这些门是因为正确的原因失败，而不是碰巧通过；
1e43a5b229 关掉的三个缺口也都是真的关上了 —— 除了 git 依赖那一个，它对真实的 git 依赖不生效。
当前 CI 状态
Lint & Static 在 1e43a5b229 上是红的，原因在本 diff 之外：它挂在 Check lint gate freshness
这一步，在安装之前。main 在 ae78d5b800（#11621）动过 eslint.legacy-filenames.mjs，而本分支
还没把它并进来 —— 这正是那道检查要抓的情况。merge origin/main 即可清掉。分支自己的 lint 在两棵
树上本地都是绿的（§3），门对今天 main 的 package-lock.json 也通过（M6），所以没有迹象说明它
背后还藏着第二个问题。
1. 两道门 —— 9 个用例，在 1e43a5b229 上实跑
（对应上文图 1）
| 用例 | 变异 | 预期 | 实际 |
|---|---|---|---|
| M0 | 不动：head 原样 | 通过 | 通过 —— 说好的四行 |
| M1 | 换回 main 的 pnpm-lock.yaml |
失败 | 失败 —— 18 个未锁定版本，与 PR 正文逐条吻合 |
| M2 | 从 allowBuilds 删掉 fsevents |
失败 | 失败 —— 点名 fsevents |
| M3 | 金丝雀：往 pnpm-lock.yaml 塞一个伪造版本 |
失败 | 失败 —— 点名 fake-canary-package@9.9.9 |
| M4 | 金丝雀：npm 开始锁定 mime-db@1.52.0 |
失败 | 失败 —— "remove these entries from knownNpmLockGaps" |
| M5 | 金丝雀：新来一个带 install 脚本的依赖 | 失败 | 失败 —— 点名 canary-native-thing |
| M6 | 今天的 origin/main 的 package-lock.json（合入后） |
通过 | 通过 |
| M7 | 金丝雀：pnpm-lock.yaml 没有 packages: 段 |
失败 | 失败 —— "has no packages section"（1e43a5b2 新增） |
| M8 | 金丝雀：allowBuilds 写成占位字符串而不是布尔 |
失败 | 失败 —— 点名 esbuild（1e43a5b2 新增） |
M3–M5 有必要，因为「M0 过、M1 挂」这一现象，一道只认死名单的门也做得到。M4 打的是
knownNpmLockGaps 的过期检测那一半，原测试计划里没有别的用例碰它。M7、M8 是 1e43a5b229
关掉的两个 fail-open 洞，在测试夹具之外同样是 fail closed。
1e43a5b229 上的 scripts/tests/package-scripts.test.js：47 通过、1 跳过（共 48），
包含全部 7 个新增的夹具式失败分支测试。
M6 回答的是维护者在合入时真正关心的问题：main 自 merge-base 起已经前进 31 个提交，其中
4ec9f63d0a 动过 package-lock.json。这道门对今天的 main 仍然通过；而且用今天 main 的
package-lock.json 跑 corepack pnpm import，产出的 pnpm-lock.yaml 与本 PR 提交的那份
逐字节相同 —— 所以这份 lockfile 还没有过期。
独立重算。 我没有直接信这个脚本，而是另写了一份实现来对照：pnpm-lock.yaml 有 1,966 个包
键，其中恰好 1 个没有被 npm 锁定 —— mime-db@1.52.0，即那条写明的例外。对照基线的
pnpm-lock.yaml 是 18 个，与 PR 的清单逐条一致。PR 自己披露的残余漂移也对得上：422 个
workspace 直接依赖里，409 个解析一致、13 个不同，正是它说的数字，而且每个 pnpm 侧的版本
npm 都在自己的依赖树里某处锁定过。
这 13 个，以及其中两个是运行时依赖
. esbuild npm 0.25.6 pnpm 0.25.12
integrations/external-context-mem0 esbuild npm 0.25.6 pnpm 0.25.12
packages/chrome-extension esbuild npm 0.25.6 pnpm 0.25.12
packages/vscode-ide-companion esbuild npm 0.25.6 pnpm 0.25.12
packages/cli @types/node npm 22.19.19 pnpm 22.20.1
packages/mobile-mcp @types/node npm 22.20.0 pnpm 22.20.1
packages/qwen-live @types/node npm 22.19.19 pnpm 22.20.1
packages/sdk-typescript @types/node npm 22.19.19 pnpm 22.20.1
packages/vscode-ide-companion @types/node npm 22.19.19 pnpm 22.20.1
packages/web-shell @types/node npm 22.19.20 pnpm 22.20.1
packages/cli @types/semver npm 7.7.0 pnpm 7.7.1
packages/core fdir npm 6.4.6 pnpm 6.5.0
packages/core picomatch npm 4.0.4 pnpm 4.0.5
13 个里有 11 个是构建或类型工具。剩下两个不是：fdir 和 picomatch，都在 packages/core 的
dependencies 里 —— pnpm worktree 跑 core 用的是 6.5.0 / 4.0.5，而 CI 用的是 6.4.6 / 4.0.4。
npm 也锁定了那两个版本（在 vite 和 tinyglobby 下面），所以这道门放行是对的；但这就是那条
「已接受的 parity 缺口」的具体形状，值得和评审已经给出的 esbuild 例子并列记在案。
2. macOS —— 那一行 ⚠️
（对应上文图 2）
corepack pnpm import逐字节复现了提交进来的 lockfile。 先删掉pnpm-lock.yaml，再从
package-lock.json重新生成，产出的文件与提交的那份完全相同，耗时 9s。PR 只声称「连续两次
import 彼此相同」，而且是在 Linux 上；更强的那个性质 —— 提交的这份就是文档里那条命令的产出，
并且在第二个操作系统上也成立 —— 是成立的。重新生成的文件随后也能通过--frozen-lockfile。- smoke 任务的三条断言都复现：frozen install 退出码 0 —— 空树冷装 1m12s，对已安装的树重新
校验是 4s（图里是后者）；workspace 链接解析到 worktree 内部；bootstrap 之后git status干净。
冷树上第一次--offline尝试因为一条缺失的元数据（@node-rs/jieba-android-arm-eabi）退回
--prefer-offline，与评审已在main上指出的冷缓存路径是同一个。 fsevents: false是安全的，理由比代码注释和评审给的都更锋利 —— 见 F5。
3. 那条 lint 规则
npm run lint:ci 在两棵树上都通过（npm 129s，pnpm 119s）—— 新规则 0 处违规，也就是作者交给 CI
的那一跑。
两个变异说明新增的两个配置块都在真干活：
- 把三个 manifest 还原成基线版本，会浮出 30 处 import、13 个（包，依赖）对 —— 正是 PR
声明的那一组，且没有别的规则报错。（正文写的是「15 处 import」；按位置数是 30，按声明数
是 13。属于措辞问题。） - 删掉
web-templates的packageDir块，会在两个构建子项目里浮出 13 处，所以那个块不是摆设。
13 条新声明全部解析到已锁定的版本 —— 用 semver.satisfies 对照 package-lock.json 验证过，
包括唯一一个不是精确相等的（semver ^7.7.2 → 锁定 7.8.5）。发布出去的 CLI manifest 从构造上
就不受影响：scripts/prepare-package.js 往 dist/package.json 里写的是 dependencies: {}。
4. Stage 2 说明的第 2 项测量 —— hoisted parity
（对应上文图 3）
说明要求在 pnpm 安装的 hoisted 树上跑一遍 CI 矩阵。我在这台机器上、两棵树上跑了它点名的那一串，
把完整的 test:ci 换成了 test:scripts：
| 步骤 | pnpm 树 | npm 树 |
|---|---|---|
npm run build |
PASS 193s | PASS 198s |
npm run typecheck |
FAIL 59s | PASS 79s |
npm run lint:ci |
PASS 119s | PASS 129s |
npm run test:scripts |
FAIL 763s —— 2 失败 / 2406 通过 | FAIL 721s —— 3 失败 / 2405 通过 |
npm run bundle |
PASS 4s | PASS 3s |
npm run check:serve-fast-path-bundle |
PASS 193s | PASS 181s |
test:scripts 的两处失败在两个安装器下是同一对（qwen-autofix-workflow.test.js，其中一个是
50s 超时）；npm 树还多一个（install-script.test.js，一条依赖网络形态的断言）。这些都跟本 diff
无关，而且 pnpm 并不更差 —— 所以这一行算 parity 通过，只是带一个宿主环境的星号。
也就是说，说明里的前提 ——「npm 脚本在 hoisted pnpm 树上原样可跑」—— 对 build、lint、bundle 和
fast-path 检查成立，对 typecheck 不成立。见 F4。
5. Stage 2 说明的第 1 项测量 —— APFS 上每个 worktree 的磁盘占用
文件系统 APFS，pnpm store 已预热，三个同一提交的全新 worktree，取 df 差值（说明里说 du
在这里没用是对的 —— 它对那棵实际只花了 40 MiB 的树报了 1.2 GiB）。用一个 20 秒的 sleep 做对照，
背景噪声是 −1 MiB。
| 方法 | 在新 worktree 里执行的命令 | df 差值 |
墙钟 | du -sh |
|---|---|---|---|---|
| npm | QWEN_SKIP_PREPARE=1 QWEN_SKIP_NOTICE_GENERATION=1 npm ci --prefer-offline |
1,398 MiB | 28s | 982 MiB |
| pnpm | node scripts/setup-worktree.js |
40 MiB | 23s | 1.2 GiB |
| reflink 拷贝 | 对主检出的 node_modules 及每个 workspace 根自己的那份做 cp -c -R |
64 MiB | 24s | 1.0 GiB |
说明里要求的三条链接检查在 reflink 这一臂全部通过，包括 1e43a5b229 新加的那条：
readlink node_modules/@qwen-code/qwen-code-core -> ../../packages/core
require.resolve core -> ./packages/core/package.json
require.resolve @larksuiteoapi/node-sdk (feishu) -> ./packages/channels/feishu/node_modules/…
对「APFS / 开发者 Mac」这一行，这组数字定下了两件事：
- 说明引用的 93% 节省在这里只多不少 —— 1,398 → 40 MiB 是 97%。
- reflink 拷贝在不用 pnpm 的情况下拿到了其中 95% 的节省（64 MiB 对 40 MiB，在 1.4 GiB 的
基线上只差 24 MiB）。说明把这一对比列为「磁盘本身是否足以支撑 Stage 2」的判据；在这个文件
系统上，答案是不足以。
发现
F1 —— 新增的 git 依赖豁免，对真实的 git 依赖不生效
（对应上文图 4）
1e43a5b229 增加了一条分支：如果某个 pnpm 键等于 npm 对 git/file: 依赖记的 resolved，就
豁免它；并配了一个测试。那个测试自己写两边的字符串，所以它只证明「两边相等时豁免有效」。而在
现实里两边并不相等：
根 devDependency: "is-plain-obj": "github:sindresorhus/is-plain-obj#v4.1.0"
npm resolved : git+ssh://git@github.com/sindresorhus/is-plain-obj.git#68e8cc77…
pnpm packages 键: is-plain-obj@https://codeload.github.com/sindresorhus/is-plain-obj/tar.gz/68e8cc77…
$ node scripts/check-lockfile.js → exit 1
Error: pnpm-lock.yaml resolves versions that package-lock.json does not lock.
Regenerate it from package-lock.json with `corepack pnpm import`:
- is-plain-obj@https://codeload.github.com/sindresorhus/is-plain-obj/tar.gz/68e8cc77…
npm 会把 spec 规范化成 git+ssh://…#<sha>，pnpm 则用 codeload 的 tarball 做键。集合包含判断
永远匹配不上；而且错误信息给出的补救办法，正是制造出这个状态的那条命令 —— 只看这条消息的人会在
原地打转。
我实测的是 git 这一半；file: 那一半形状相同（npm 以引用方为锚记 file:，pnpm 用
name@file:<相对 workspace 的路径> 做键），但我没有测。两种情况今天都只是潜在问题：两个
lockfile 里现在 git 和 file: 依赖各一个都没有，所以不阻塞。但这条分支
存在的意义正是「等哪天有了」，而照现在的写法，那天它帮不上忙。改成匹配两边字符串里都出现的
40 位 commit SHA（或者对 gitHosted: true 的条目匹配 resolution.tarball）就可以。夹具测试如果
让「写这一侧的工具」来生成这一侧、而不是手写，本来就能抓到。
F2 —— PR 描述里的 “Smoke trigger” 一条已经过期
1e43a5b229 把 package-lock.json 从 pnpm-worktree-smoke.yml 的路径过滤器里撤了回去，并加了
注释说明理由，scripts 测试现在断言它不在。而描述里仍然写着「只改 npm lockfile 时，pnpm smoke
workflow 现在也会运行」。合入前值得改一下，让描述和分支对得上。
F3 —— 豁免纯类型 import，使这条规则达不到 isolated 布局本该抓到的范围
规则配置没有设 includeTypes，所以纯类型 import 被豁免；注释给的理由是「它们在构建时消失」。
对 bundle 没错，对 tsc 不是：isolated 的 node_modules 仍然要解析这些说明符才能过类型检查。
把 includeTypes: true 打开，今天会多浮出 3 个（包，依赖）对、5 处：
packages/core :: @opentelemetry/sdk-trace-base 全仓没有任何 manifest 声明过
packages/core :: @agentclientprotocol/sdk 5 个兄弟包声明了，core 没有
packages/cli :: cli-spinners 全仓没有任何 manifest 声明过
@opentelemetry/sdk-trace-base 最尖锐：它只存在于
packages/core/node_modules/@opentelemetry/sdk-trace-base，由 @opentelemetry/sdk-node 传递
带入，而 packages/core/src/telemetry/{file-exporters,log-to-span-processor}.ts 会 type-import
它。这正是本 PR 要关掉的那类脆弱性，只是换了一条车道。要么打开开关并补上这三条声明，要么在
注释里写明这个豁免是有意为之以及为什么。
F4 —— npm run typecheck 在 pnpm 安装的树上失败；不是这个 PR 造成的
只有一个错误：
packages/acp-bridge/src/process-registry.ts(744,3): error TS2322:
Type 'string | NonSharedBuffer' is not assignable to type 'string'.
原因是树根的 @types/node：npm 放 20.19.1，pnpm 的 hoisted 布局放 22.20.1，而
tsc -p integration-tests/tsconfig.json 通过 paths 把 packages/acp-bridge/src/** 映射进来，
读到的就是根上那一份。acp-bridge 自己钉的 20.19.1 在两棵树里都正确落到了
packages/acp-bridge/node_modules —— 只有根上那个槽位不同。A/B： 在 642d36e 上安装
main 自己的 pnpm-lock.yaml，根上同样是 22.20.1、同样是这个错误，所以它早于本 PR。写在这里，
是因为它直接回答了新说明存在的两个问题之一，应该进那份说明将来的 results.md，而不是被人再
发现一次。
F5 —— fsevents: false 是对的，但评审给出的后果不对
stage=2 评审说这条 deny「让它们失去原生 FSEvents 绑定、退回 fs.watch」。实测并没有发生：
- 三份 fsevents（根上 2.3.3，
playwright下 2.3.2，第三份 2.3.2 —— 在 pnpm 树里位于
mobilewright下、在 npm 树里位于packages/mobile-mcp下）在两棵树里都在，预编译的
fsevents.node也都在；根上那份二进制在两棵树之间逐字节相同。 - 两棵树都没有
build/目录；fsevents 2.3.3 安装下来的 tarball 里既没有binding.gyp，也
根本没有install脚本 —— npm lockfile 那个标记反映的install: node-gyp rebuild只存在于
registry 的 packument 里，而它相对 tarball 是过期的。两个安装器都没有编译任何东西，这条 deny
拒掉的是一个本来就不会跑的脚本。 - pnpm 树里的实测探针：
require('fsevents').watch(dir, cb)对「开始监听之后」创建的文件收到了
事件，根上那份和playwright嵌套的那份都收到了。
PR 的注释（「install 脚本只是 node-gyp rebuild」）在意图上没错，在 tarball 这一点上略有出入。
值得补一句，因为这条目是本 PR 唯一一处「做决定」而不是「记录现状」的地方。
F6 —— 两处小的不对称（吹毛求疵）
knownNpmLockGaps有过期检测（M4 证明了），allowBuilds没有：某条目一旦不再对应任何 npm
install 脚本，会悄悄留着。今天这个集合是精确的 —— 7 个带 install 脚本的 npm 包全部有决定，外加
一条 workspace 条目 —— 所以这是预防，不是现存问题。真要加过期检测，得把
@qwen-code/qwen-code-core@file:packages/core那条放行，这大概就是没加的原因；在注释里说一句
就够了。- 规则的 glob 是
packages/**/src/**和integrations/**/src/**，且只覆盖.ts/.tsx。包的
入口文件（packages/cli/index.ts、packages/core/index.ts）、packages/*/scripts/**，以及
src下的.mjs构建文件（例如packages/web-templates/src/insight/vite.config.mjs，它 import
了vite和@vitejs/plugin-react）都在覆盖之外。我把这片没覆盖的面手工扫了一遍，今天没有
未声明的 import —— 属于覆盖面备注，不是缺陷。
没有跑的部分
完整的 npm run test:ci（CI 覆盖）、Windows，以及 ECS runner 主机上的磁盘数字 —— 说明的第 1 项
要的是按主机类别测，这里只是「开发者 Mac」那一行。
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- the stale "Smoke trigger" bullet in the PR description and the matching tick in #10444's checklist (scripts/tests/package-scripts.test.js:947) — already reported by @wenshao (issue comment 5642071764, finding F2)
- the missing staleness arm on pnpm-workspace.yaml allowBuilds, the sibling of the one knownNpmLockGaps got in this same diff (scripts/check-lockfile.js:215-221) — already reported by @wenshao (issue comment 5642071764, finding F6)
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — I did not execute corepack pnpm import or pnpm install --frozen-lockfile to confirm pnpm's override/frozen interaction first-hand (it would mutate th…; "agent reverse-audit (round 2)": an executed pnpm install --frozen-lockfile against the regenerated lockfile (no registry access and no writable install tree in this shared worktree) — instal…; "agent reverse-audit (round 3)": a full-repo lint run — I ran npx eslint only over the rule-reached packages whose manifests this PR did not amend ( packages/channels , integrations , sdk-t…; "agent reverse-audit (round 3)": whether pnpm-lock.yaml 's importer **spec strings** (not just names) match the amended manifests — a spec drift also fails pnpm install --frozen-lockfile on …; "agent reverse-audit (round 3)": whether corepack pnpm import applies the .pnpmfile.mjs readPackage hook and the 13 pnpm-workspace.yaml overrides , i.e. whether the new AGENTS.md regen…, and 2 more.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — I did not execute corepack pnpm import or pnpm install --frozen-lockfile to confirm pnpm's override/frozen interaction first-hand (it would mutate th…；"agent reverse-audit (round 2)"：an executed pnpm install --frozen-lockfile against the regenerated lockfile (no registry access and no writable install tree in this shared worktree) — instal…；"agent reverse-audit (round 3)"：a full-repo lint run — I ran npx eslint only over the rule-reached packages whose manifests this PR did not amend ( packages/channels , integrations , sdk-t…；"agent reverse-audit (round 3)"：whether pnpm-lock.yaml 's importer **spec strings** (not just names) match the amended manifests — a spec drift also fails pnpm install --frozen-lockfile on …；"agent reverse-audit (round 3)"：whether corepack pnpm import applies the .pnpmfile.mjs readPackage hook and the 13 pnpm-workspace.yaml overrides , i.e. whether the new AGENTS.md regen…，另有 2 条。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
— qwen3.8-max via Qwen Code /review (v0.23.3)
Quiesce the df window, name the worktree sampler's working directory and forbid host cron, and register the throwaway workflow's size baseline so the parity run's test:ci does not fail under both installers. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmty6iqs0zv
Step 2 now pushes the scratch branch before gh workflow run --ref (the ref is resolved server-side), notes the workflow token scope, and gives the merge-to- default / pull_request fallback when a branch-only dispatch is refused. Step 1 probes the checkout's own volume with diskutil info "$(pwd)" instead of the sealed read-only / system volume. Pushing, dispatching, and posting the results comment are marked operator actions. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmty8nwk9zy
Add two failure-branch cases: a knownNpmLockGaps entry that leaves the pnpm graph (the !pnpmVersions.includes arm), and an aliased install script reported under its real package name (the npmPackageName details.name arm). Both were untested and stayed green if the arm was deleted. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmty8nwk9zy
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
已核对 head 7637d8c4（base 4c072e88，12 files / +1164 -919）。
⛔ 先说阻塞项：这个 PR 现在跟 main 冲突
mergeable: false、mergeable_state: dirty、rebaseable: false。我在临时 worktree 里试合了一次，冲突只有两个文件：
scripts/check-lockfile.jsscripts/tests/package-scripts.test.js
正好是约二十分钟前刚合入的 #11371 改过的那两个（Playwright parity 段 + 它的 fixture-copy 测试）。因为两边都在同一锚点追加内容，所以是内容冲突。合 main 时必须同时保留：本 PR 的 CHECK_LOCKFILE_ROOT seam 与两段新门禁，以及 #11371 的 parity 段与其 11 个 arm。
我在 head 上实跑过的
node scripts/check-lockfile.js：五段全部通过（npm integrity、pnpm integrity、pnpm vs npm 版本一致、build approvals）。CHECK_LOCKFILE_ROOT这个新 seam 工作正常，能把门禁指向 fixture。
合并 main 之后会坏在哪（已复现，见 inline）
把 main 的 parity 段接到本 head 脚本尾部（即合并后的形态），再用本 PR 自己的 fixture —— 只拷 package-lock.json / pnpm-lock.yaml / pnpm-workspace.yaml —— 运行，parity 段会去读 <root>/package.json 并拿到 ENOENT，然后 process.exit(1)。所以 fixture 还得一起拷 package.json 与 packages/web-shell/package.json，否则本 PR 新增的、期望 exit 0 的两个 arm 合完全变红。
既有未解决线程（6 条 bot + 1 条你自己的）
- R2-6 成立：新加的
import/no-extraneous-dependencies只覆盖packages/**/src/**，而packages/web-shell的源码在client/（703 个 ts/tsx，不含 e2e），它是唯一被发布出去的 workspace（main/exports都指向dist/）。该块注释写的是"包必须声明自己源码 import 的东西"，web-shell 整块在检查之外。 - R1-1 成立，值得你判断：
pnpm-workspace.yaml是nodeLinker: 'hoisted'，而 lockfile 里@agentclientprotocol/sdk@0.14.1在不同 importer 分别配对zod@4.4.3与zod@3.25.76，物理上只会剩一份。SDK 的 peer 范围是^3.25.0 || ^4.0.0，两者都合法，所以我没跑pnpm install，不判它是不是 bug。 scripts/check-lockfile.js:15（你自己提的那条）成立：??不兜空字符串，CHECK_LOCKFILE_ROOT=""会让root变成''，之后所有路径相对 cwd。改成||或先 trim 都行。- R1-3、R1-8、R1-10、R2-5 我没有独立验证，不做判断。
本次不 approve：冲突未解 + 上述 P1。
Resolve the conflict with #11371 in the two files it and this branch both appended to: - scripts/check-lockfile.js keeps main's Playwright parity section and this branch's pnpm-vs-package-lock agreement and build-approval sections. The parity section runs first, so the earlier-shipped gate keeps its position relative to the lockfile integrity checks. - scripts/tests/package-scripts.test.js keeps both sides' assertions on the shared "checks both lockfiles for integrity and agreement" case, and this branch's new failure-branch suite. The parity section reads the pinned manifests as well as the lockfiles, so both fixtures now copy what it needs: package.json and packages/web-shell/package.json (plus pnpm-workspace.yaml for the new build-approval gate in the Playwright suite). Without them a fixture run dies on a missing package.json before reaching the detector each arm pins. Verified on the merge result: scripts/check-lockfile.js passes all five sections; scripts/tests/check-lockfile.test.js 11/11; scripts/tests/package-scripts.test.js 40/41 (the one failure, "bootstraps worktrees and preserves explicit hook settings", also fails on main); prettier and eslint clean on the three touched files.
- `CHECK_LOCKFILE_ROOT` was read with `??`, so an exported-but-empty variable made `root` the empty string and every path in this gate cwd-relative: it would then report on whatever lockfiles sit in that directory as if they were the repository's, and pass quietly when the cwd happens to be the repo root. Truthiness keeps the fixture seam and closes the empty case. - The version-agreement failure named exactly one remedy, `corepack pnpm import`, while pnpm-workspace.yaml carries eight `overrides` (three of them pin typescript) that no npm-side regeneration can reproduce. The message now names that layer, so a still-red gate points at what actually decides the value. Verified: `node scripts/check-lockfile.js` exits 0 on the committed tree, prettier and eslint are clean on the file, and both suites that execute the gate pass. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Re-reviewed at head c507a6f48c. Since my previous pass (pullrequestreview-5186295521) two things landed: a merge of main, and c507a6f48c.
The merge is clean where it matters. main brought #11371's Playwright parity block into the tail of the same file this PR extends, which is the collision both PRs' review threads predicted. At this head the two blocks coexist correctly: the pnpm version-agreement and build-approval gates run first, the parity block after, both read the same root, and node scripts/check-lockfile.js exits 0 on the committed tree printing all three section verdicts. Note the follow-up #11716 also edits this file's parity block, so whichever of the two merges second still takes a textual conflict there.
c507a6f48c closes two of the three findings I filed: the CHECK_LOCKFILE_ROOT seam is now read for truthiness, so an exported-but-empty variable falls back to the repository instead of silently repointing every path in the gate at the cwd; and the version-agreement failure now names pnpm-workspace.yaml's overrides: as the second layer that can decide a version, which was the remedy the message omitted.
Verified at this head, not inferred: node scripts/check-lockfile.js exit 0; prettier --check and eslint --max-warnings 0 clean on the changed file; scripts/tests/check-lockfile.test.js plus scripts/tests/package-scripts.test.js = 60 passed, 1 skipped (the latter is the suite that exercises the env seam with a real fixture directory, so the truthiness change is covered rather than merely read). CI at this head: 24 pass, 1 pending (review-pr).
Four threads remain open, each with its fix or its reason stated in-thread: R1-1 (the @agentclientprotocol/sdk zod peer rebind — deliberately not adjudicated from the summary, it needs a lockfile reading and may be a runtime behaviour change rather than gate quality), R2-6 (packages/web-shell/client/** is outside the new lint rule's globs; the package has no src/, is published, and declares 31 dependencies), R1-10 (decidedBuilds discards both the key's scope and its boolean value), and package-scripts.test.js:191 (not read yet). Two further findings I declined with evidence in-thread: R1-8, where the exemption is unreachable — zero of 2,263 non-link packages have a git/file: resolved at this head, and the two tools' string formats could not match anyway — and R1-3, whose check needs a networked pnpm install and belongs in the smoke workflow.
Not approving, for a procedural reason rather than a finding: the last commit on this branch is mine, so this account cannot sign an approval on it, and pushes here dismiss approvals anyway — the two that stood at 7637d8c40d are gone. This PR's paths carry no CODEOWNERS entry, so one approval from any other maintainer is what it needs.
中文说明
在 head c507a6f48c 上复审。相比我上一轮，落了两件事：合并 main，以及 c507a6f48c。
合并这一处是关键：main 把 #11371 的 Playwright 一致性块带进了本 PR 也在扩展的同一个文件尾部，正是两边 review 线程都预告过的碰撞。在当前 head 上两块共存正确——pnpm 的版本一致性与构建审批两道门禁在前、一致性块在后，两者读同一个 root，node scripts/check-lockfile.js 在提交树上 exit 0 并打出三段结论。另外 #11716 也改这个文件的一致性块，所以两者谁后合并谁仍会在同一处吃文本冲突。
c507a6f48c 关掉了我提的三条里的两条：CHECK_LOCKFILE_ROOT 改成按 truthiness 读（导出但为空的变量不再把门禁里每条路径静默改成按 cwd 解析），以及版本不一致的报错现在点名 pnpm-workspace.yaml 的 overrides: 这个同样能决定版本的层。
以下都是在该 head 上实测、不是推演：门禁 exit 0；prettier --check 与 eslint --max-warnings 0 对改动文件干净；check-lockfile.test.js + package-scripts.test.js 共 60 passed / 1 skipped（后者正是用真实夹具目录跑 env 接缝的套件，所以 truthiness 这个改动是被覆盖的，不只是读过）。CI 在该 head：24 pass、1 pending（review-pr）。
仍开着的 4 条线程，每条都在线程里写明了修法或理由：R1-1（zod peer 重绑，故意不凭摘要下结论，需要读锁文件，且可能是运行时行为变化而非门禁质量）、R2-6（新 lint 规则的 glob 覆盖不到 packages/web-shell/client/**；该包没有 src/、已发布、声明 31 个依赖）、R1-10（decidedBuilds 同时丢掉 key 的版本范围与布尔值）、package-scripts.test.js:191（尚未读）。另两条我已带证据 decline：R1-8（该豁免不可达——这 head 上 2263 个非 link 包里 resolved 以 git/file: 开头的是 0，且两个工具写的字符串本来也不可能相等）、R1-3（那个校验需要带网络的 pnpm install，应放在 smoke workflow 里）。
不 approve，是程序原因不是发现：这条分支最后一个 commit 是我提的，本账号无法对它签 approve；而且这仓库 push 即 dismiss，7637d8c40d 上那两票已经没了。本 PR 的路径不在 CODEOWNERS 里，所以只差任意一位其他维护者的一票。
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Re-review after head moved (7637d8c → c507a6f). Prior round (R1) had no findings; this round verifies the delta.
No blocking findings.
Approval blockers: none.
Delta from prior head — three changes, all improvements:
-
CHECK_LOCKFILE_ROOThardening (check-lockfile.jslines 13-14):- Old:
const root = process.env.CHECK_LOCKFILE_ROOT ?? join(__dirname, '..'); - New:
const envRoot = process.env.CHECK_LOCKFILE_ROOT?.trim(); const root = envRoot ? envRoot : join(__dirname, '..');
The
??form would have used""(empty string) if the variable was exported but empty, making everyjoin(root, ...)call produce a bare filename resolved against cwd. The new truthiness check falls through tojoin(__dirname, '..')in that case. Mechanism confirmed:join("", "package-lock.json")="package-lock.json"(relative, reads from cwd). Fix is correct. - Old:
-
Error message improvement (
unlockedPnpmVersionsbranch): added explanation that apnpm-workspace.yamloverrides:entry can cause a divergence that survivescorepack pnpm import. Informational-only; no logic change. -
Test fixture expansion (
package-scripts.test.jsrunCheckLockfile): now copiespackage.jsonandpackages/web-shell/package.jsonin addition to the lockfiles. Needed because the base now includes the Playwright parity section (from a merged predecessor PR) which reads both manifests. Without them the fixture run would die on a missing file before reaching the branch under test. Correct fix.check-lockfile.test.js: addedpnpm-workspace.yamltoFILES— consistent with the build-approval section needing it.
R1 findings carry-over: R1 had no findings. Nothing to rule fixed or still present.
Cross-check: wenshao's macOS verification covered 1e43a5b (the autofix head) and 0c67d4872a; the current head c507a6f is a rebase onto updated main with the same delta. No new reviewer findings to cross-check against.
Not covered: no new logic beyond the empty-string guard; rung 2+ not applicable.
Reviewed with AI assistance.
|
Closeout paused: this PR was merged at 2026-09-12 14:48 UTC with head The local changes address R1-1 (retain npm's ACP SDK/Zod binding), R1-10 (match exact-version unions and source keys instead of discarding scope), and R2-6 (cover Web Shell client sources in the existing import gate). Existing build allow/deny entries are unchanged. The pinned pnpm rejects semver ranges in approval keys, so the old Verified locally on macOS / Node 22.22.0 / pnpm 11.24.0: a real frozen install; the focused lockfile/script cases; Web Shell and changed-file lint; formatting. All five ACP importers share SDK 0.14.1 with Zod 3.25.76, and real valid initialize, malformed initialize and unknown-method wire responses match npm exactly. The SDK's existing malformed-request -32603 classification is preserved, not fixed upstream. Full builds/typecheck and cross-platform suites were not rerun. Scope: five existing files, +79/-12 lines, including the generated lockfile delta and focused tests. The two local commits remain available for a separately authorized follow-up. The published-round counter remains 2 because this pass was not pushed. |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
APPROVE
核对基线：head c507a6f48c9e495b8a66d520411bec6e5a97a317。
历史阻塞问题：无
当前 24 条线程里 3 条未解决（R1-1 pnpm 侧 zod peer 重绑、R1-10 decidedBuilds 只取键名、R2-6 eslint glob 覆盖不到 packages/web-shell/client/**），逐条读过，全部是 [Suggestion] 级，没有任何 Critical、安全、数据损坏或回归级别的历史阻塞问题。按本渠道策略 Suggestion 不作为合入门禁。
本轮独立扫描：未发现 Critical
这个 PR 的主要风险面是「新声明的依赖是否改变了实际解析结果」和「新门禁是否会假绿」，我按这两条主线核对：
- 新增的依赖声明都是对既有解析结果的忠实记录，没有改变 npm 树的版本。
packages/cli新声明chalk: ^4.1.2：package-lock.json里根node_modules/chalk本来就解析为 4.1.2，而packages/cli/src中三处出货代码（commands/extensions/utils.ts、commands/extensions/consent.ts、ui/utils/software-cursor.ts）都是import chalk from 'chalk'，此前正是从根 hoist 拿到 4.1.2。所以这是补声明，不是降级。其余 5.6.2 的副本各自嵌套在ink、ora、log-symbols、lint-staged、@secretlint/formatter下，不受影响。packages/core新声明zod: ^3.25.76：npm 树里根node_modules/zod解析为 3.25.76，@agentclientprotocol/sdk@0.14.1hoist 在根且 peer 为^3.25.0 || ^4.0.0，由根 zod 满足；4.4.3 的副本只嵌套在@modelcontextprotocol/client、@modelcontextprotocol/core、packages/mobile-mcp下。声明与既有解析一致。- 这一点很关键，因为 npm 才是 build/CI/packaging/release 的权威路径；R1-1 描述的 zod peer 重绑发生在
pnpm-lock.yaml，影响面限于 opt-in 且 install-only 的 pnpm worktree，且 SDK 的 peer 范围同时接受 3 与 4。
NOTICES.txt的 +6/-6 是纯重排。 移除的六行与新增的六行是同一个多重集合（@opentelemetry/api-logs@0.221.0、core@2.10.0、semantic-conventions@1.36.0、exporter-logs-otlp-grpc@0.221.0、otlp-exporter-base@0.221.0、otlp-transformer@0.221.0），只是api-logs从末尾移到前面，许可证正文未动。没有任何归属条目被删除，不构成合规问题。- 新增的 pnpm 门禁不会假绿。
scripts/check-lockfile.js的新段落里，pnpmLockfile.packages为空时直接console.error+process.exit(1)，明确写出「the version agreement gate read nothing」，因此读不到内容不会被当成通过；发现 pnpm 解析出 npm 未锁定的版本时process.exitCode = 1。唯一的白名单项mime-db@1.52.0还配了staleNpmLockGaps反向检查，一旦 npm 侧锁上了该项或 pnpm 侧不再出现就会报错，白名单不会静默腐烂。方向也选得对：只断言「pnpm worktree 不得跑 CI 的 npm install 未锁定的版本」，而不是要求两张依赖图相等（npm 保留嵌套副本、pnpm 会 dedupe，两者本就不可能相同）。 CHECK_LOCKFILE_ROOT用真值判断而非??，注释说明了原因：导出但为空的环境变量会让所有路径退化成 cwd 相对，门禁就会把当前目录下碰巧存在的 lockfile 当成仓库的来报告。这个细节处理是对的。eslint.config.js是收紧而非放宽。 新增的import/no-extraneous-dependencies设为error，作用于packages/**/src/**与integrations/**/src/**，测试文件按extraneousDependencyTestFiles排除，类型导入按规则本身豁免。当前 head 上Lint & Static为 pass，意味着「声明了但没导入」和「导入了但没声明」两个方向都干净——这也反过来印证了第 1 点的声明是忠实的。pnpm-workspace.yaml只新增fsevents: false及说明其 install script 仅为node-gyp rebuild、macOS smoke install 未报告 ignored build 的注释，与 pnpm 既有行为一致。
CI：当前 head 上 Lint & Static、Test (ubuntu-latest, Node 22.x)、Integration Tests (no-AK, No Sandbox)、web-shell E2E Smoke 全部 pass，没有由本 PR 引入的失败；review-pr 仍 pending，按策略不作为门禁。
其余意见
R2-6 指出的 glob 覆盖不到 packages/web-shell/client/**（该包已发布、704 个 .ts/.tsx 在 client/ 下且 src/ 为空）虽为 Suggestion，但它正好是新规则注释里声明要覆盖的那类包，建议后续把 packages/*/client/** 一并纳入。R1-10 关于 decidedBuilds 用 Object.keys() 丢弃 value、可能让 UNDECIDED_ALLOW_BUILD 占位符也算「已决定」的观察同样值得后续处理。两者都不阻塞本次合入。
结论：无历史阻塞问题，本轮未发现可证明的 Critical，提交 APPROVE。
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review: chore(pnpm): gate pnpm-lock on package-lock and declare hoisted imports
FINDINGS:
- No blockers found.
CLEAN AREAS:
- scripts/check-lockfile.js: Version agreement gate is correct. The direction (pnpm must not resolve versions npm has not locked) is the right one.
npmPackageNamehandles aliased installs (details.name) and nestednode_modules(lastIndexOf).pnpmPackageNamecorrectly handles scoped packages (indexOf('@', 1)skips leading@).knownNpmLockGapshas staleness detection. Build approval check correctly filters non-booleanallowBuildsplaceholders.CHECK_LOCKFILE_ROOTenv var uses truthiness check (not??) to guard against empty-string injection. - eslint.config.js: Hoisted imports rule is correctly scoped to
packages/**/src/**andintegrations/**/src/**, with test files excluded. Theweb-templatespackageDiroverride is correct for a package whose imports resolve through a different manifest. - package.json changes (cli, core, vscode-ide-companion): Dependency additions correctly declare previously-implicit hoisted dependencies.
package-lock.jsonchanges mirror these exactly. - pnpm-workspace.yaml:
fsevents: falseis correct — Darwin-only optional dep whosenode-gyp rebuildscript should not run under pnpm. - pnpm-lock.yaml: Regenerated via
pnpm import. Verified version changes are consistent with npm's locked versions (bothzod@4.4.3andsupports-color@7.2.0exist in npm's nested packages). - Tests: Comprehensive failure-branch coverage — version mismatch, stale
knownNpmLockGaps(both arms), missingallowBuilds, aliased package names, empty packages section, git dependency matching, spec-scopedallowBuildskeys, and placeholder entries. - AGENTS.md: Procedural change from
pnpm install --lockfile-onlytocorepack pnpm importcorrectly makes pnpm-lock.yaml derive from package-lock.json.
NEEDS_HUMAN:
- The zod peer rebinding (
@agentclientprotocol/sdknow resolveszod@4.4.3instead ofzod@3.25.76in the importers section) is functionally correct per the SDK's peer range (^3.25.0 || ^4.0.0) but represents a behavioral change in which zod version satisfies that peer under pnpm. Worth confirming intentional. eslint.config.jsglobs do not coverpackages/web-shell/client/**(onlysrc/**), so that workspace's shipped sources outsidesrc/are unchecked for extraneous dependencies. Minor coverage gap, not a blocker.
VERDICT: APPROVE
Core logic is correct and well-tested; the version agreement gate, build approval check, hoisted imports rule, and dependency declarations all work as intended with no reachable correctness defects.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review: chore(pnpm): gate pnpm-lock on package-lock and declare hoisted imports
FINDINGS:
- No blockers found.
CLEAN AREAS:
- scripts/check-lockfile.js: Version agreement gate is correct. The direction (pnpm must not resolve versions npm has not locked) is the right one.
npmPackageNamehandles aliased installs (details.name) and nestednode_modules(lastIndexOf).pnpmPackageNamecorrectly handles scoped packages (indexOf('@', 1)skips leading@).knownNpmLockGapshas staleness detection. Build approval check correctly filters non-booleanallowBuildsplaceholders.CHECK_LOCKFILE_ROOTenv var uses truthiness check (not??) to guard against empty-string injection. - eslint.config.js: Hoisted imports rule is correctly scoped to
packages/**/src/**andintegrations/**/src/**, with test files excluded. Theweb-templatespackageDiroverride is correct. - package.json changes (cli, core, vscode-ide-companion): Dependency additions correctly declare previously-implicit hoisted dependencies.
- pnpm-workspace.yaml:
fsevents: falseis correct. - pnpm-lock.yaml: Verified version changes consistent with npm locked versions.
- Tests: Comprehensive failure-branch coverage.
- AGENTS.md: Procedural change is correct.
NEEDS_HUMAN:
- zod peer rebinding is functionally correct but worth confirming intentional.
- eslint.config.js globs do not cover packages/web-shell/client/**. Minor coverage gap.
VERDICT: APPROVE
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Approve: Core logic is correct and well-tested. No blockers found. See detailed review in comments.
Detailed Code Review |
…xists Merging main brought in #11625, which regenerated pnpm-lock.yaml and dropped the vitest@3.2.7 snapshot keyed with @vitest/browser and supports-color@8.1.1. The packages/browser-use importer still referenced that key, so every Install leg failed with ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY. Re-key the importer to the snapshot its sibling packages already use; nothing else in the lockfile changes.
Picks up the lint-gate changes (eslint.config.js #11625, eslint.legacy-filenames.mjs #11443) that the Lint & Static freshness check flagged as stale on this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XNKaCrgfft8Y1Cdps1cf8E
…ness lane (QwenLM#11797) Follow-ups left open when QwenLM#11625 merged: its three unresolved review threads, the deferred finding in QwenLM#11681, and QwenLM#10524. - allowBuilds decisions now honour the whole key the way pnpm does: a bare name covers every version, an exact `name@version` (or a `||` union) covers only those, and a range or source-like key decides nothing. Reducing a key to its name let an approval scoped to one version cover the next (R1-10). - Pin `@agentclientprotocol/sdk>zod` to 3.25.76 so a pnpm worktree runs the zod the npm tree installs. Under zod 4 the SDK's own `instanceof z.ZodError` test stops matching and a malformed request answers -32602 instead of -32603; both versions are npm-locked, so the agreement gate cannot see the split (R1-1). - Cover web-shell's shipped client sources with the undeclared-import rule; it is the one published package whose sources are not under src/. Measured cost today: zero violations (R2-6). - Add a path-filtered workflow that regenerates pnpm-lock.yaml with `corepack pnpm import` and fails when the committed file differs, which is the freshness half set membership cannot cover (QwenLM#11681). - setup-worktree: accept only an executable file on PATH, so a corepack left without its exec bit reports the actionable error instead of EACCES (QwenLM#10524). - Name the esbuild divergence an accepted parity gap, and record what `fsevents: false` costs a macOS worktree.
|
Released in v0.23.4. |
8 participants
What this PR does
Follow-up to the post–Stage 1 review on #10444. It adds the consistency gates the dual-lockfile setup has been missing, and it fixes everything those gates flag today.
mime-types@2.1.35pinsmime-db1.52.0 exactly, but npm locks no nested copy. The check also fails if that exception goes stale.pnpm import. The pnpm lockfile is now generated from the npm lockfile withpnpm import, which is also the documented regeneration step from now on.@typescript-eslint/*@8.53.1,@vitest/browser@3.2.7,minimatch@10.2.6anddiff@8.0.4.import/no-extraneous-dependencieson shipped sources; tests are excluded and type-only imports are exempt. This is the check an isolatednode_moduleslayout would add, without changing the layout.ansi-escapes,chalk,open,semverzodand seven@opentelemetry/*packageslucide-reactnpm run buildbefore package tests.docs/verification/describes the two measurements that still decide Stage 2: per-worktree disk on the hosts that run agent worktrees, and a run of the CI matrix on a pnpm-installed hoisted tree.Why it's needed
The #10444 review found that the two lockfiles had already drifted three days after Stage 1 merged, and no gate noticed. Two separate symptoms:
The cause is systemic.
pnpm install --lockfile-onlyresolves every range on its own to the newest match, so hand-kept overrides cannot keep up. Deriving the pnpm lockfile from the npm one, and gating the direction that matters, makes the dual-lock setup safe to keep for as long as the pnpm bootstrap stays opt-in.After this PR, 13 direct dependencies still resolve to different versions under the two lockfiles, and each of them resolves under pnpm to a version npm also locks elsewhere in its tree.
The undeclared imports matter for the same reason. Under hoisting they are invisible until a package is installed on its own, and with the lint rule in place, isolated linking is no longer needed to find them.
Reviewer Test Plan
How to verify
fsevents.zod, makes lint fail.Evidence (Before & After)
N/A. This PR changes tooling and dependency manifests only.
Tested on
These checks ran on Linux with Node 22.23.2:
pnpm importtwice, with identical output, followed by a frozen lockfile-only install;node_modulesolder thanmain, so CI is the authority.Not run locally: build, typecheck, and unit tests, including the updated package-scripts test. CI covers those. macOS and Windows are covered by the smoke workflow.
Environment (optional)
N/A
Risk & Scope
corepack pnpm import. The old--lockfile-onlycommand will usually make the lockfile check fail.Linked Issues
Part of #10444. The issue stays open for the Stage 2 go/no-go decision.
中文说明
这个 PR 做了什么
这是 #10444 上 Stage 1 之后那次复盘 的后续。它补上双 lockfile 一直缺少的一致性门，并修掉这些门在今天会报出的所有问题。
mime-types@2.1.35精确要求mime-db1.52.0，但 npm 没有锁定嵌套副本。这条例外一旦过期，检查也会失败。pnpm import重新生成 pnpm lockfile。 pnpm lockfile 现在用pnpm import从 npm lockfile 生成，这也是今后文档里写的再生成步骤。@typescript-eslint/*@8.53.1、@vitest/browser@3.2.7、minimatch@10.2.6和diff@8.0.4。import/no-extraneous-dependencies；测试文件不在范围内，纯类型 import 豁免。这正是 isolatednode_modules布局才会带来的检查，但不需要改动布局。ansi-escapes、chalk、open、semverzod和 7 个@opentelemetry/*包lucide-reactnpm run build。docs/verification/下新增一份说明，描述仍然决定 Stage 2 的两项测量：跑 agent worktree 的主机上每个 worktree 的磁盘占用，以及在 pnpm 安装的 hoisted 树上跑一遍 CI 矩阵。为什么需要
#10444 的复盘发现，Stage 1 合入三天后两个 lockfile 就已经漂移，而且没有任何门察觉。两种症状：
原因是系统性的。
pnpm install --lockfile-only会把每个范围独立地解析到最新的匹配版本，手工维护的 overrides 跟不上。让 pnpm lockfile 从 npm lockfile 派生，并对要紧的那个方向设门，就能让双 lock 在 pnpm bootstrap 仍是 opt-in 期间安全地保留下去。合入本 PR 后，仍有 13 个直接依赖在两个 lockfile 下解析到不同版本，而且每一个在 pnpm 下解析到的版本，npm 在自己的依赖树里也锁定了。
未声明 import 的问题出于同样的原因。在 hoisting 下，它们要等某个包被单独安装时才会暴露；有了这条 lint 规则，就不再需要靠 isolated 链接去发现它们。
审阅者测试计划
如何验证
fsevents。zod），lint 就会失败。证据（前后对比）
N/A。本 PR 只改工具链和依赖 manifest。
测试平台
macOS⚠️ 、Windows ⚠️ 、Linux ✅。以下检查在 Linux、Node 22.23.2 上运行过：
pnpm import，输出相同，之后跑一次 frozen lockfile-only install；node_modules比main旧，所以以 CI 结果为准。本地没有运行：build、typecheck 和单元测试（包括改过的 package-scripts 测试）。这些由 CI 负责，macOS 和 Windows 由 smoke workflow 覆盖。
环境（可选）
N/A
风险与范围
corepack pnpm import重新生成 pnpm lockfile。旧的--lockfile-only命令通常会让 lockfile 检查失败。关联 Issue
属于 #10444 的一部分。该 issue 继续保持打开，用于 Stage 2 的 go/no-go 决定。