perf(cli): import core modules directly where no test mocks the package - #10946
perf(cli): import core modules directly where no test mocks the package#10946yiliang114 wants to merge 2 commits into
Conversation
Importing from the core package root evaluates its whole export graph — a bit over six hundred modules — however little of it a file uses. On the release lane the cli workspace spends more time collecting modules than running tests, and on the main lane it now takes 84 minutes on its own, most of it collection. These 130 files ask for named modules instead. They were chosen by checking, for every test whose module graph reaches them, whether that test replaces the core package with a mock: a test that swaps the package wholesale stops intercepting once the code under test imports a module directly, and a test that spreads the real package and overrides a few names only matters if one of those names is what the file imports. Files with either kind of coupling are left for a later change that moves the mocks at the same time. Only import statements move; every other line is byte-identical.
|
Thanks for the PR — and for writing the measurements down in #10908 first, that makes this easy to evaluate. Template looks good ✓ Problem: observed and measured, not theoretical. #10908 is open with concrete numbers from a real run ( Direction: aligned, and more so than the PR body claims. The repo already forbids this pattern inside core — Size: 130 files, 996 production lines (382 added / 614 deleted), 0 test lines, 0 generated/schema lines. It does touch core paths as the gate defines them — Approach: the scope feels right, and I checked the minimality claim rather than taking it — see the Stage 2 comment for the detail. Short version: it holds. Every added and removed line in the diff is an import statement, and the set of 248 names removed from the barrel is exactly the set of 248 re-added through subpaths, so nothing was dropped, added, or drive-by refactored along the way. Deferring the mock-coupled half to a follow-up is the right cut, not an arbitrary one. Two things I'd flag before the code review, neither of them about the transform itself:
Risk: Stage 1e matched one revert-correlated path — Moving on to code review. 🔍 中文说明感谢贡献——也感谢先在 #10908 里把测量数据写清楚，这让评估变得很容易。 模板完整 ✓ 问题： 已观测且有测量，不是理论性加固。#10908 处于 open 状态，带有真实 run 的具体数字（ 方向： 对齐，而且比 PR 正文说的更对齐。仓库内部本来就已经禁止这个模式—— 规模： 130 个文件，996 行生产代码（新增 382 / 删除 614），测试行 0，生成/schema 行 0。按 gate 的定义确实触及核心路径—— 方案： 范围合理，而且我没有直接采信「改动最小」的说法，是核对过的——细节见 Stage 2 评论。简版结论：成立。diff 里每一条新增和删除的行都是 import 语句；从 barrel 移除的 248 个名字，与通过子路径重新加回来的 248 个名字完全一致，所以没有丢名字、没有加名字、也没有夹带顺手重构。把与 mock 耦合的那一半留给后续 PR，是正确的切分而不是随意切分。 进入代码审查前有两点要先说，都不是针对这次转换本身：
风险： Stage 1e 命中一条与 revert 相关的路径—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewMy independent proposal first. Reading only the title and "Why it's needed", I'd have reached for the same fix — a codemod from the root barrel to per-module subpaths, staged — because the alternatives are worse: making the barrel itself lazy is higher-leverage per file but changes core semantics, aliasing the barrel to a stub inside vitest is cheap but quietly changes what tests exercise, and more sharding is already ruled out by the shard timings in #10908. Where I'd have been insistent is on two preconditions before the codemod runs: settle how the new specifiers resolve under plain Node (tsc The transform itself is clean — I checked it rather than trusting it. Three static checks over the full diff:
Type/value classification is preserved too, which matters under So: no correctness finding in the rewrite. My three concerns are all about what happens around it. 1. No CI lane runs against this PR, so the stated oracle never executed (blocking)The safety argument for a 130-file import rewrite is "a wrong specifier fails at import time rather than subtly, so a green cli suite is the signal." That property is true, but it only produces a signal if something imports the files. On this base, nothing does: There's no green suite on the PR this one depends on either. #10917 at This matters most for the part you explicitly asked a reviewer to judge: whether "a spread mock that does not override the imported name is safe to leave alone" holds for how this codebase writes mocks. That is precisely the kind of assumption that shows up as a red test and stays invisible in a diff read. #10908 counts 135 cli test files calling 2. The new specifier form is not resolvable by plain Node, and two consumers execute the unbundled output (blocking)At this PR's base commit None of this PR's 102 specifiers match any of those keys. Two consumers run that unbundled output under plain Node:
Unaffected: the published package ( Two things make me treat this as a real gap rather than a theoretical one. This repo has been bitten by exactly this divergence before — To be explicit about the limits of this finding: it is static reasoning. Triage never executes PR-derived code, so I did not run 3. The diff is not Prettier-formatted (blocking, one command to fix)
The Non-blocking
Dependency surface this PR creates (18 subpath roots)Useful for finding 2 — each root is a key
102 distinct module specifiers across those roots; all 102 verified to point at an existing file under TestingWhich evidence this comment carries: the PR's own CI check results, read through the GitHub API for the reviewed commit. This is an unattended CI run, so per the triage rules I did not build, run, or test any PR-derived code, and I drove no real-scenario tmux session — that lane is local-invocation only. Everything above about the rewrite's correctness is static analysis of the diff text plus reads of the base tree.
Read that table as an absence, not as a pass. There is no Not verified, and why:
Sandboxed verification would settle most of that, and you have write access so both lanes are available directly rather than sponsored: 中文说明代码审查先说我自己的方案。 只看标题和「为什么需要」，我会选同样的做法——把 root barrel 导入按模块改写成子路径，并且分批推进——因为其他选项更差：把 barrel 本身改成惰性导入，单文件收益更高但会改动 core 语义；在 vitest 里把 barrel 别名成 stub 很便宜，但会悄悄改变测试所验证的东西；继续加分片已经被 #10908 里的分片耗时排除。我会坚持的两个前置条件是：先查清这些新说明符在纯 Node 下怎么解析（tsc 的 转换本身是干净的——我是核对过的，不是采信。 对完整 diff 做了三项静态检查：
type / value 的分类也保留了，这在 所以改写本身没有正确性问题。我的三点顾虑都在它周围。 1. 这个 PR 没有任何 CI lane 会跑，所以正文里的判据从未执行（拦截项）130 个文件的 import 改写，其安全论证是「错误的说明符会在 import 时就失败，所以 cli 套件全绿就是信号」。这个性质成立，但只有在真的有东西去 import 这些文件时才会产生信号。在这个 base 上没有： 它所依赖的那个 PR 也没有绿套件。#10917 在 这一点对你明确请 reviewer 判断的那部分最关键：「展开式 mock 只要没覆盖被导入的名字就可以不动」这个规则，在本仓库的 mock 写法下是否成立。这正是那种会以红色测试暴露、而在读 diff 时完全看不见的假设。#10908 统计有 135 个 cli 测试文件调用 2. 新的说明符形态在纯 Node 下无法解析，而有两个消费方会执行未打包的产物（拦截项）在本 PR 的 base 提交 本 PR 的 102 个说明符没有一个能匹配上述任何 key。 有两个消费方会在纯 Node 下执行这份未打包产物：
不受影响的：发布包（ 有两点让我不把它当理论问题。这个仓库以前就踩过同样的分歧—— 明确说明这条的边界：它是静态推理。triage 从不执行 PR 带来的代码，所以我没有跑 3. diff 没有经过 Prettier 格式化（拦截项，一条命令可修）
非拦截项
测试本评论携带的证据类型： 通过 GitHub API 读取的、本 PR 自身在所审提交上的 CI check 结果。这是无人值守的 CI 运行，按 triage 规则我没有构建、运行或测试任何 PR 带来的代码，也没有驱动真实场景的 tmux 会话——那条 lane 只适用于本地调用。上面所有关于改写正确性的内容，都是对 diff 文本的静态分析加上对 base 树的读取。 把上面那张表当作「缺失」来读，不要当作「通过」。表里没有 未验证项及原因：
沙箱化验证可以解决其中大部分，而且你有写权限，两条 lane 都可以直接触发而不是赞助运行： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 2/5 — the rewrite itself is the cleanest part of this; what I can't get past is that nothing has executed it, and one static check says a real runtime path breaks. Stepping back. The transform is genuinely good work, and better than a codemod has any right to be: the name set is provably lossless, type-vs-value classification survived But the question the gate has to answer is whether this is ready to merge, and there I keep landing on the same thing: this PR has never been imported by anything. The stated oracle is exactly right — a wrong specifier fails loudly at import time — but the base branch filters out every lane that would have triggered it, and the base PR's own Test lane died on a runner disconnect before executing a single test. So the safety argument is sound in principle and unevidenced in fact. That's not a small gap for a 130-file change whose whole justification is mechanical reliability, and it's the gap that also swallows the two things I'd most want a run to answer: whether The Node-resolution finding is the one I'd want answered before anything else, and I want to be careful not to overstate it: it's static reasoning, I did not run The Prettier finding is trivial and I almost left it out; I didn't, because it's the one item here that is certain rather than reasoned, and because it would turn the first On the things I checked myself rather than took on faith: I confirmed the minimality claim instead of assuming it, and I looked for a simpler path — there isn't one. Making the barrel lazy touches core semantics, stubbing it in vitest changes what tests exercise, and more sharding is already answered by your own shard timings. Staging it 130-then-the-rest is the right cut, and deferring the mock-coupled half is a judgement call I agree with. If I were maintaining this in six months I'd thank you for the migration; I'd also want the lint rule from phase ① so the 130 files don't quietly drift back. So: not approving, and not because of doubt about intent or craft. Requesting changes on three specific items — get one real run's worth of evidence, settle the 中文说明Confidence: 2/5 —— 这次改写本身是整个 PR 里最干净的部分；我过不去的是：它从未被执行过，而一项静态检查指出有一条真实的运行时路径会断。 退一步看整体。转换本身做得很好，甚至比一个 codemod 应有的水平更好：名字集合可证明无丢失、type 与 value 的分类在 但 gate 要回答的问题是它是否可以合并，而在这点上我反复落到同一件事：这个 PR 到目前为止没有被任何东西 import 过。 你写的判据完全正确——错误的说明符会在 import 时大声失败——但 base 分支把每一个会触发它的 lane 都过滤掉了，而 base PR 自己的 Test lane 在执行任何一个测试之前就因 runner 断连死掉了。所以这个安全论证在原理上成立、在事实上没有证据。对一个 130 个文件、全部正当性都建立在「机械可靠性」上的改动来说，这个缺口不小；而且它同时吞掉了我最希望由一次真实运行来回答的两件事： Node 解析那条是我最希望在其他一切之前得到回答的，同时我也想小心不要夸大它：它是静态推理，我没有运行 Prettier 那条很琐碎，我几乎没写进去；最终写了，因为它是这里唯一「确定」而非「推理」得出的一项，也因为它会让第一次基于 关于我自己核对而没有直接采信的部分：我验证了「改动最小」的说法而没有假设它成立，也找过是否存在更简的路径——没有。把 barrel 改成惰性会触及 core 语义，在 vitest 里替换成 stub 会改变测试所验证的内容，继续加分片已经被你自己的分片耗时回答过了。分成「先 130 个、剩下的后续」是正确的切分，把与 mock 耦合的那一半延后，我同意这个判断。如果六个月后由我来维护，我会感谢这次迁移；同时我也会想要阶段 ① 里的那条 lint 规则，以免这 130 个文件悄悄退回原状。 所以：不批准，而且不是因为对意图或工艺有疑问。就三项具体内容请求修改——拿到一次真实运行的证据、按你选定的方向把 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@yiliang114 Requesting changes on three specific items — full reasoning in my Stage 2 and Stage 3 notes above, short version here.
- Nothing has executed this diff. The base is
perf/core-subpath-imports, andQwen Code CI/Security Checks/serve-aball filterpull_requesttomain/release/**, so there is noTest, noLint & Static(ESLint + Prettier) and no typecheck on this PR. #10917'sTest (ubuntu-latest, Node 22.x)also never completed — runner disconnect, step 16 has a null conclusion. Your oracle ("a wrong specifier fails at import time") is right, but it hasn't run. Either retarget tomainonce #10917 lands, or trigger@qwen-code /verify/ aworkflow_dispatchCI run and paste the result. - The new specifier form isn't in core's
exportsmap. At basea1a84a25it holds., eight named subpaths,./package.json,./dist/*,./src/*— none of the 102<dir>/<file>.jsspecifiers match. They resolve via tsconfigpaths(tsc/esbuild/tsx) and via #10917's vitest aliases, buttsc --buildemits specifiers verbatim intopackages/cli/dist, andscripts/start.js(npm start) plus.github/workflows/serve-ab.ymlboth hand that output to barenode→ERR_PACKAGE_PATH_NOT_EXPORTED. #10908 lists exactly this as an unverified precondition. Widenexports, or state that the unbundled dist isn't a supported runtime and adjust those two callers. Static reasoning only — I did not run it, so one real run settles it either way. - Prettier.
.prettierrc.jsonsetsprintWidth: 80; 279 of 382 added lines exceed it and ~39 are multi-name imports Prettier would expand.npm run format.
The rewrite itself needs no changes — I verified the 248 removed barrel names are exactly the 248 re-added through subpaths, all 102 specifiers point at real modules, and nothing outside an import statement moved. Non-blocking: #10908's phase ① pairs this migration with a lint rule, and architecture/no-core-root-barrel-import is registered only for packages/core/src/**, so nothing stops these 130 files drifting back.
中文说明
@yiliang114 就三项具体内容请求修改——完整推理见上面的 Stage 2 与 Stage 3 评论，这里是简版。
- 这份 diff 从未被执行过。 base 是
perf/core-subpath-imports，而Qwen Code CI/Security Checks/serve-ab的pull_request都过滤到main/release/**，所以本 PR 上没有Test、没有Lint & Static（ESLint + Prettier）、也没有 typecheck。#10917 的Test (ubuntu-latest, Node 22.x)同样从未跑完——runner 断连，第 16 步结论为 null。你的判据（「错误的说明符会在 import 时失败」）是对的，但它没有跑过。要么等 #10917 合并后把 base 重定向到main，要么触发@qwen-code /verify或一次workflow_dispatchCI 运行并把结果贴出来。 - 新的说明符形态不在 core 的
exportsmap 里。 在 basea1a84a25上，它只包含.、八个具名子路径、./package.json、./dist/*、./src/*——102 个<dir>/<file>.js说明符没有一个匹配。它们能通过 tsconfigpaths（tsc/esbuild/tsx）和 #10917 的 vitest alias 解析，但tsc --build会把说明符原样输出到packages/cli/dist，而scripts/start.js（npm start）和.github/workflows/serve-ab.yml都会把这份产物直接交给裸node→ERR_PACKAGE_PATH_NOT_EXPORTED。#10908 正是把这一条列为未验证的前置条件。要么扩展exports，要么明确未打包的 dist 不是受支持的运行时并调整这两个调用方。这仅是静态推理——我没有运行它，所以一次真实运行就能定论。 - Prettier。
.prettierrc.json设了printWidth: 80；382 行新增里有 279 行超出，其中约 39 行是 Prettier 会展开的多名导入。跑一下npm run format。
改写本身不需要改动——我核对过：从 barrel 移除的 248 个名字与通过子路径重新加回来的 248 个完全一致，102 个说明符全部指向真实模块，除 import 语句外没有任何一行发生移动。非拦截项：#10908 的阶段 ① 把这次迁移与一条 lint 规则配套提出，而 architecture/no-core-root-barrel-import 只注册在 packages/core/src/** 上，所以没有任何东西阻止这 130 个文件退回原状。
— Qwen Code · qwen3.8-max-2026-09-02
Reviewed at 028826b58db0c9951d269b1954287f5bc3222b17 · re-run with @qwen-code /triage
…nd wrap long imports Two problems with the previous commit, both found by CI. The symbol map resolved a re-exported name to the module that re-exports it rather than the one that declares it, so `ProviderModelConfig` was asked of `models/types` when it is declared in `providers/types`, and the build failed to typecheck. A checker now confirms, for every generated specifier, that the named module really does export that symbol — following its own re-exports — and it reports one bad pair out of 492. The formatting pass that was supposed to run over these files had silently done nothing: invoked from the repository root against paths outside it, Prettier skips the files and still reports success, so long import statements went out unwrapped. Rerunning it properly reflows 47 files. Two files are left with an over-long line Prettier would wrap, because that line is over-long on the base commit too and the lint gate does not flag it; reformatting it here would be unrelated noise. Every other line outside an import statement stays byte-identical.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): chunk 7: contents of scripts/start.js — whether root npm start / the release flow executes the esbuild bundle ( dist/cli.js ) or the raw tsc dist/index.js .; chunk 7: release/publish pipeline — what artifact actually ships as the qwen bin (raw tsc output vs bundle)..
中文说明
未探索到全部深度（达到工具调用预算）：chunk 7：contents of scripts/start.js — whether root npm start / the release flow executes the esbuild bundle ( dist/cli.js ) or the raw tsc dist/index.js .；chunk 7：release/publish pipeline — what artifact actually ships as the qwen bin (raw tsc output vs bundle).。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| } from '@qwen-code/qwen-code-core'; | ||
| import type { Config } from '@qwen-code/qwen-code-core/config/config.js'; | ||
| import { IdeClient } from '@qwen-code/qwen-code-core/ide/ide-client.js'; | ||
| import { initializeTelemetry } from '@qwen-code/qwen-code-core/telemetry/sdk.js'; |
There was a problem hiding this comment.
[Critical] R1-1: Rewriting this file (and eight more of the migrated files) from the root barrel to subpath specifiers silently breaks the colocated tests' vi.mock('@qwen-code/qwen-code-core', …) interception: vitest keys mocks by resolved module ID, and the subpath specifiers resolve — via the wildcard alias in packages/cli/vitest.config.ts — to different module IDs than the barrel the tests mock, so the real implementations now run against the test stubs. This file is the representative anchor; the measured pairing is: startup-prefetch.test.ts (8 failures), lsp-config-watcher.test.ts (5), systemController.test.ts (3, systemController.ts + baseController.ts), useResumeCommand.test.ts (9), session-swap-telemetry.test.ts (6, via session-switch.ts), live-session.test.ts (20 — the real CoreToolScheduler constructor throws TypeError: options.config.getToolRegistry is not a function against the stub Config), workflow-save-overlay.test.tsx (3 — the real saveWorkflowScript crashes against the stub config), DiscoverTab.test.tsx (1), SourcesTab.test.tsx (1): 56 PR-caused failures from a full packages/cli suite run attributed against the merge base. The PR's stated oracle is red, and the description's selection rule ("files where no test mocks the package") is violated for these nine — this is issue #10908's named risk ("tests stay green while no longer testing what they claim") materializing, except here the tests mostly go red: the worse shape is the ones that could stay green while testing something else (e.g. the real initializeTelemetry now executes inside unit tests and the "swallows telemetry initialization failures" test exercises no error path).
Witness:
npm test --workspace=packages/cli → Test Files 11 failed | 992 passed; Tests 72 failed | 28054 passed
test-delta rerun at merge base a1a84a25: failing file set = settings.test.ts, run-qwen-serve-live.test.ts only
→ the 9 files above are net-new failures on the PR (pre-existing ones excluded)
live-session.test.ts: TypeError: options.config.getToolRegistry is not a function (coreToolScheduler.ts:1480)
startup-prefetch.test.ts 'swallows telemetry initialization failures': mockWarn — Number of calls: 0
Fix: either revert these nine files to the barrel import (exclude them from batch 1, matching the PR's own selection rule), or co-migrate each affected test's vi.mock to the exact subpath specifiers its subject now imports (e.g. vi.mock('@qwen-code/qwen-code-core/utils/debugLogger.js', …)) in the same commit — the remedy the PR description itself prescribes for mock-coupled files. Note the fix must respect that packages/cli/vitest.config.ts:62-78 resolves the wildcard alias /^@qwen-code\/qwen-code-core\/(.*)$/ → core/src/$1 and the exact-match root alias to different module IDs (the file's own comment documents that importing one core module rather than the package root depends on it), so a co-migrated mock must spell exactly the specifier the source file imports. Acceptance: the nine failing test files above are red at HEAD and must return to green with the fix — and go red again if a subpath import is reintroduced without mock coverage.
中文说明
[Critical] R1-1：把这个文件（以及另外 8 个被迁移的文件）从根 barrel 改为子路径说明符，会悄悄破坏同目录测试里的 vi.mock('@qwen-code/qwen-code-core', …) 拦截：vitest 按解析后的模块 ID 建立 mock，而子路径说明符经由 packages/cli/vitest.config.ts 的通配 alias 解析到的模块 ID 与测试所 mock 的 barrel 不同，于是真实实现会直接跑在测试桩上。本文件是代表性锚点；实测对应关系为：startup-prefetch.test.ts（8 个失败）、lsp-config-watcher.test.ts（5）、systemController.test.ts（3，systemController.ts + baseController.ts）、useResumeCommand.test.ts（9）、session-swap-telemetry.test.ts（6，经由 session-switch.ts）、live-session.test.ts（20 —— 真实 CoreToolScheduler 构造函数对桩 Config 抛 TypeError: options.config.getToolRegistry is not a function）、workflow-save-overlay.test.tsx（3 —— 真实 saveWorkflowScript 对桩配置崩溃）、DiscoverTab.test.tsx（1）、SourcesTab.test.tsx（1）：一次完整 packages/cli 套件运行并对照合并基线归因后，共 56 个由本 PR 导致的失败。PR 声明的判据（绿套件）是红的，且正文的筛选规则（「没有测试 mock 该包的文件」）在这 9 个文件上被违反——这正是 #10908 点名的风险（「测试仍然绿着、却不再测试它声称的东西」）成真；这里多数测试直接变红，更糟的形态是那些可能继续绿、但测的已经是别的东西的用例（例如真实 initializeTelemetry 现在会在单测里执行，而「吞掉 telemetry 初始化失败」的用例不再走任何错误路径）。
修复：要么把这 9 个文件还原为 barrel 导入（从 batch 1 中排除，符合 PR 自己的筛选规则），要么在同一个提交里把各测试的 vi.mock 同步迁移到源文件现在实际导入的子路径说明符（如 vi.mock('@qwen-code/qwen-code-core/utils/debugLogger.js', …)）——即 PR 正文为 mock 耦合文件开出的药方。注意 packages/cli/vitest.config.ts:62-78 把通配 alias 与根精确匹配 alias 解析为不同的模块 ID（该文件注释也写明按模块导入依赖此映射），因此迁移后的 mock 必须与源文件的导入说明符逐字一致。验收标准：上述 9 个测试文件当前为红，修复后必须回绿；若在缺少 mock 覆盖的情况下重新引入子路径导入，它们应再次变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| import type { Config } from '@qwen-code/qwen-code-core/config/config.js'; | ||
| import { createDebugLogger } from '@qwen-code/qwen-code-core/utils/debugLogger.js'; |
There was a problem hiding this comment.
[Critical] R1-2: [fails-closed] [regression] The 101 new <dir>/<file>.js subpath specifiers are not covered by packages/core/package.json exports (which holds only ., the eight named subpaths, ./package.json, ./dist/*, ./src/*), and tsc emits them verbatim into packages/cli/dist. Bare Node therefore rejects them at module load: npm start (scripts/start.js spawns node packages/cli → main: dist/index.js) crashes at startup with ERR_PACKAGE_PATH_NOT_EXPORTED before serving any input — and, new evidence beyond the existing thread's static analysis, npm run dev crashes too, because scripts/dev.js's loader intercepts only the exact bare specifier, letting subpaths fall through to Node's exports check. .github/workflows/serve-ab.yml:294/307 (node … packages/cli/dist/index.js) hits the same crash. Typecheck, vitest and the esbuild bundle stay green only because each has a private mapping (tsconfig paths / the vitest alias / esbuild reading that tsconfig), which is why no existing lane catches it; the published npm artifact is unaffected (prepare-package.js repoints at the bundle). This is the run the existing discussion asked for ("one real run settles it") — it settles it against: the CLI does not start outside the bundle on this branch.
Witness:
$ node --input-type=module -e "...import.meta.resolve('@qwen-code/qwen-code-core/utils/debugLogger.js')" (cwd packages/cli)
ERR_PACKAGE_PATH_NOT_EXPORTED: Package subpath './utils/debugLogger.js' is not defined by "exports"
$ node packages/cli/dist/index.js -p 'hi'
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: ... imported from .../packages/cli/dist/src/config/lsp-config-watcher.js (exit 1)
$ npm run dev -- -p 'hi'
same error from packages/cli/src/config/lsp-config-watcher.ts, via scripts/dev.js's resolve hook (exit 1)
base form still resolves: import.meta.resolve('@qwen-code/qwen-code-core') → packages/core/dist/index.js
Fix: register the subpath roots in packages/core/package.json exports — e.g. a "./*" pattern mapping into dist/src/ (exact-match named entries keep precedence), or explicitly declare the unbundled dist an unsupported runtime and adjust scripts/start.js, scripts/dev.js and serve-ab.yml to match. Either way this settles the precondition #10908 named as "must be settled before the codemod runs"; the gap is partly owned by base #10917, but this PR scales the exposure from 2 files to 130. The fix must match the manifest's existing convention — entries pair types with import and map into ./dist/src/... (e.g. ./transcriptRecords → ./dist/src/utils/transcript-records.js), and the existing ./package.json, ./dist/*, ./src/* passthroughs must not be shadowed. Acceptance: a plain-Node smoke test (no tsx/vitest/esbuild) that builds and dynamically imports one cli-used core subpath specifier must pass with the exports entries and fail with ERR_PACKAGE_PATH_NOT_EXPORTED when they are removed.
中文说明
[Critical] R1-2: [fails-closed] [regression] 101 个新的 <dir>/<file>.js 子路径说明符都不在 packages/core/package.json 的 exports map 里（现有键只有 .、八个具名子路径、./package.json、./dist/*、./src/*），而 tsc 会把说明符原样输出到 packages/cli/dist。因此裸 Node 在模块加载时就会拒绝它们：npm start（scripts/start.js 派生 node packages/cli → main: dist/index.js）在服务任何输入之前就带着 ERR_PACKAGE_PATH_NOT_EXPORTED 崩掉——并且，这是超出现有讨论静态推理的新证据：npm run dev 同样会崩，因为 scripts/dev.js 的 loader 只拦截精确的裸说明符，子路径会落到 Node 的 exports 检查上。.github/workflows/serve-ab.yml:294/307（node … packages/cli/dist/index.js）会踩到同样的崩溃。typecheck、vitest 和 esbuild bundle 之所以绿，只是因为各自有私有映射（tsconfig paths / vitest alias / esbuild 读取该 tsconfig），这正是现有 lane 都发现不了它的原因；发布产物不受影响（prepare-package.js 会改指向 bundle）。已有讨论说「一次真实运行就能定论」——这条证据就是那次运行：结论是否定的，本分支上除 bundle 外的启动路径都起不来。
修复：在 packages/core/package.json 的 exports 中注册这些子路径根——例如加一条映射到 dist/src/ 的 "./*" 通配（精确匹配的具名条目仍优先），或者明确未打包的 dist 不是受支持的运行时并相应调整 scripts/start.js、scripts/dev.js 与 serve-ab.yml。无论哪个方向，都等于落实 #10908 列为「必须在 codemod 前查清」的前置条件；该缺口部分归属于 base #10917，但本 PR 把暴露面从 2 个文件放大到 130 个。修复须符合 manifest 现有约定——条目成对携带 types 与 import 并映射到 ./dist/src/...（如 ./transcriptRecords → ./dist/src/utils/transcript-records.js），且不能遮蔽现有的 ./package.json、./dist/*、./src/* 直通条目。验收标准：一个纯 Node（不经 tsx/vitest/esbuild）的冒烟测试，在构建后动态导入一个 cli 实际使用的 core 子路径说明符，加入 exports 条目后必须通过、移除后必须以 ERR_PACKAGE_PATH_NOT_EXPORTED 失败。
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
Closing in favour of #10957, which carries the whole change and is green. This branch still holds the pre-revert set. Once the stack was retargeted at So the diff on this PR no longer describes anything that should land. Merging it on its own would reintroduce exactly the modules that were shown to break, and because its base is not #10957 is based on 中文说明关闭，改由 #10957 承载全部改动——它已全绿。 本分支仍是回退之前的内容。整个栈改到 因此本 PR 的 diff 已不代表任何应该落地的内容。单独合并它会把已被证实会挂的那批模块重新带进来；而且由于它的 base 不是 #10957 基于 |
2 participants
Stacked on #10917 — that PR adds the resolver mapping these imports depend on, so this one is based on it and will retarget to main once it lands.
What this PR does
Moves 130 cli files from importing the core package root to importing the specific modules they use. Only import statements change; every other line is byte-identical to its current contents.
Why it's needed
Importing from the package root evaluates core's whole export graph, a bit over six hundred modules, however little of it a file actually uses. The cost shows up as collection rather than assertions: on the release lane the cli workspace reported 2223s collecting against 1372s running tests, and on the main lane it now takes 84 minutes on its own with collection running roughly three times the test time. A file that imports the package root costs about 11.5s before its first assertion; importing one module instead costs about 2s.
Sharding cannot reach this. The three release shards finish within a minute of each other, so the split is already balanced and adding shards only divides a fixed per-file cost — a cost that grows with every test file added, and the suite has grown 87% in ten weeks.
Background and measurements are in #10908.
How these files were chosen
The constraint is not which files import the package root — most of them do — but which ones can move without silently changing what a test exercises. For every test whose module graph reaches a candidate file, this checks how that test treats the core package:
What is left is 130 files where no test's treatment of the package changes. The excluded ones need their mocks moved in the same commit as the code, which is a separate change.
Reviewer Test Plan
How to verify
A wrong specifier fails at import time rather than subtly, so a green cli suite is the signal. The mechanical property worth confirming is that nothing outside an import statement moved — comparing each file with its parent outside the import block should come back identical.
The judgement worth a reviewer's eye is the exclusion rule above: whether "a spread mock that does not override the imported name is safe to leave alone" holds for how this codebase writes mocks.
Evidence (Before & After)
N/A — no user-visible behavior changes.
Tested on
Not run locally; relying on CI.
Risk & Scope
Linked Issues
Refs #10908
中文说明
本 PR 基于 #10917（那个 PR 提供了这些导入所依赖的解析映射），待其合并后会自动重定向到 main。
这个 PR 做了什么
把 130 个 cli 文件从「导入 core 包根」改为「导入实际用到的具体模块」。只有 import 语句变化，其余每一行与当前内容逐字节相同。
为什么需要
从包根导入会求值 core 的整个导出图（六百多个模块），无论调用方实际只用了多少。代价体现在模块收集而非断言上：release lane 上 cli 收集耗时 2223s、跑测试 1372s；main lane 上 cli 现在单独就要 84 分钟，收集时间约为测试时间的三倍。一个从包根导入的文件在第一条断言前要花约 11.5s，改成按模块导入只要约 2s。
分片解决不了这个问题。三个 release 分片耗时相差不到一分钟，说明切分已经均衡，继续加分片只是在摊薄一个固定的单文件成本——而这个成本随每个新增测试文件线性增长，套件在十周内长了 87%。
背景与测量数据见 #10908。
这些文件是怎么挑的
约束不在于「哪些文件导入了包根」（大多数都导入了），而在于「哪些能在不悄悄改变测试所验证内容的前提下迁移」。对每个候选文件，检查所有模块图能到达它的测试如何处理 core 包：
剩下的 130 个文件，没有任何测试对该包的处理方式会因此改变。被排除的那些需要在同一个提交里同步搬迁 mock，属于另一次改动。
风险与范围