Skip to content

perf(startup): Load undici lazily behind package-local dynamic imports - #7455

Merged
doudouOUC merged 4 commits into
QwenLM:mainfrom
doudouOUC:perf/lazy-undici
Jul 22, 2026
Merged

perf(startup): Load undici lazily behind package-local dynamic imports#7455
doudouOUC merged 4 commits into
QwenLM:mainfrom
doudouOUC:perf/lazy-undici

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

perf(startup): load undici lazily behind package-local dynamic imports

What this PR does

Moves the undici HTTP client out of the eager startup closure. Undici was the single largest remaining third-party contributor to ACP child cold start after the telemetry work: about 2 MiB of parse/compile cost across two bundled copies, paid on every startup even though undici is only needed once a request actually goes out — proxy dispatchers, API preconnect, IDE client fetch, GitHub setup, self-update. All eight value-import sites now load undici behind a dynamic import, funneled through a small single-flight helper kept in each package.

The helper also solves a bundler interoperability trap that motivated its existence: esbuild compiles the CommonJS undici package into a default-only dynamic chunk with no named exports, so a plain const { Agent } = await import('undici') works in Node and under vitest but destructures undefined in the bundled CLI — a failure mode that local test runs cannot catch. The helper normalizes the module shape (unwrapping only the exact default-only form), and the helper is deliberately duplicated per package rather than shared, because each package resolves its own undici copy and a shared helper would silently escape test mocks in the other package.

Two ordering guarantees are preserved explicitly. The global proxy dispatcher that used to install synchronously during config construction now installs behind a stored promise that config initialization awaits, so the dispatcher is always in place before any network activity. The channel proxy path awaits the same installation before startup proceeds. The existing bundle-closure guard gains a check that fails CI if a static undici import ever re-enters the ACP eager closure.

Why it's needed

Daemon cold start (#4748) still paid undici's module cost in every ACP child process before this change. On the 2C4G reference machine, dropping it from the eager closure is worth −89.5 ms P50 of process-to-first-session latency (candidate faster in all 30 of 30 benchmark pairs), on top of the telemetry lazy-loading gains. The eager closure shrinks from 15.42 MiB to 13.39 MiB, and resident memory after the first session drops by about 8 MB.

Reviewer Test Plan

How to verify

  • Default startup: behaves as before; undici loads only when a network-touching path runs. The bundle guard proves the closure claim statically: npm run build && cross-env DEV=true npm run bundle && node scripts/check-serve-fast-path-bundle.js now also fails on any static undici import in the ACP closure.
  • Proxy configured (--proxy or settings): outbound requests still go through the proxy dispatcher; the dispatcher is installed before config initialization completes. Same for the channel proxy path.
  • IDE companion connected: IDE HTTP requests still honor NO_PROXY for the IDE host.
  • Bundled CLI smoke (the case unit tests cannot cover): node dist/cli.js -p "reply with exactly: ok" completes a real model round-trip — this exercises the esbuild default-only chunk shape end to end.
  • Unit tests: cd packages/core && npx vitest run src/utils/runtimeFetchOptions.test.ts src/utils/runtime-fetch-options.no-proxy.test.ts and cd packages/cli && npx vitest run src/utils/apiPreconnect.test.ts src/commands/channel/start.test.ts src/services/setup-github.test.ts.

Evidence (Before & After)

N/A (no UI change). Paired benchmark on a 2C4G Linux host, 30 pairs per scenario, control = the telemetry-split build, candidate = this change:

  • Cold first session: process→first-session paired P50 −89.5 ms (1336.8 → 1255.2), paired P95 −17.1 ms; candidate faster in 30/30 pairs; RSS after first session −8.1 MB (423.3 → 415.2).
  • Preheated: no regression (P50 80.7 → 78.0 ms; P95 131.9 → 84.0 ms).
  • Functional checks in the same run: concurrency (two parallel sessions, both succeed), telemetry-disabled, and legacy single-session all pass with no residual processes.
  • Bundle closure: ACP eager closure 15.42 MiB / 132 chunks → 13.39 MiB / 130 chunks; undici bytes in the eager closure 2057 KiB → 0.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

macOS: unit tests, typecheck, lint, bundle guards, and a real-model smoke run against the bundled CLI. Linux (2C4G): paired cold/preheated benchmarks against the bundled CLI.

Risk & Scope

  • Main risk or tradeoff: dispatcher installation for the --proxy path is now asynchronous; it is awaited during config initialization (and before channel startup), so ordering relative to network activity is unchanged. A failure to load undici for the proxy dispatcher is now logged instead of thrown from the constructor — the process previously could not reach that state without undici already loaded.
  • Not validated / out of scope: the remaining lazy-loading candidates listed in Cold-start follow-ups: remaining lazy-loading candidates from the ACP eager-closure audit #7264.
  • Breaking changes / migration notes: none.

Linked Issues

Part of #4748. Implements candidate 4 (lazy undici) from #7264.

中文说明

本 PR 做了什么

把 undici HTTP 客户端移出急切启动闭包。在 telemetry 懒加载工作之后,undici 是 ACP 子进程冷启动中最大的单个第三方开销:两份打包拷贝合计约 2 MiB 的解析/编译成本,每次启动都要支付,而 undici 只在真正发出请求时才被需要——代理 dispatcher、API 预连接、IDE 客户端 fetch、GitHub 配置、自更新。全部 8 个值导入点现在都通过动态 import 加载 undici,收敛到每个包内一个小的单飞 helper。

这个 helper 同时解决了促使它存在的一个打包器互操作陷阱:esbuild 会把 CommonJS 的 undici 包编译成只有 default 导出、没有命名导出的动态 chunk,因此裸写 const { Agent } = await import('undici') 在 Node 和 vitest 下正常,在打包后的 CLI 中却解构出 undefined——本地测试完全无法发现这种失败。helper 对模块形态做归一化(只在恰好为 default-only 形态时解包);并且刻意在两个包中各保留一份而不是共享,因为两个包各自解析自己的 undici 拷贝,共享 helper 会让另一个包的测试 mock 被静默绕过。

两个顺序保证被显式保留:原先在 config 构造期间同步安装的全局代理 dispatcher,现在通过一个被 config 初始化 await 的 promise 安装,保证 dispatcher 一定先于任何网络活动就位;channel 代理路径同样在启动继续之前 await 安装完成。现有 bundle 闭包守卫新增一项检查:任何静态 undici 导入重新进入 ACP 急切闭包都会让 CI 失败。

为什么需要

在本改动之前,Daemon 冷启动(#4748)的每个 ACP 子进程仍要支付 undici 的模块成本。在 2C4G 参考机器上,把它移出急切闭包带来进程到首 session 延迟 P50 −89.5 ms(30 对基准中 30 对全部更快),叠加在 telemetry 懒加载收益之上。急切闭包从 15.42 MiB 缩小到 13.39 MiB,首 session 后常驻内存下降约 8 MB。

评审验证计划

如何验证

  • 默认启动:行为不变;undici 只在网络相关路径运行时才加载。bundle 守卫静态证明闭包结论:npm run build && cross-env DEV=true npm run bundle && node scripts/check-serve-fast-path-bundle.js 现在会对 ACP 闭包中的任何静态 undici 导入报错。
  • 配置代理(--proxy 或 settings):出站请求仍走代理 dispatcher;dispatcher 在 config 初始化完成前安装到位。channel 代理路径同理。
  • IDE companion 连接时:IDE HTTP 请求仍对 IDE host 遵守 NO_PROXY
  • 打包 CLI 冒烟(单测覆盖不到的场景):node dist/cli.js -p "reply with exactly: ok" 完成一次真实模型往返——端到端验证 esbuild default-only chunk 形态。
  • 单元测试:cd packages/core && npx vitest run src/utils/runtimeFetchOptions.test.ts src/utils/runtime-fetch-options.no-proxy.test.ts 以及 cd packages/cli && npx vitest run src/utils/apiPreconnect.test.ts src/commands/channel/start.test.ts src/services/setup-github.test.ts

证据(前后对比)

N/A(无 UI 变化)。2C4G Linux 机器成对基准,每场景 30 对,control = telemetry 拆分构建,candidate = 本改动:

  • 冷启动首 session:进程→首 session 成对 P50 −89.5 ms(1336.8 → 1255.2),成对 P95 −17.1 ms;30/30 对 candidate 全部更快;首 session 后 RSS −8.1 MB(423.3 → 415.2)。
  • 预热路径:无回归(P50 80.7 → 78.0 ms;P95 131.9 → 84.0 ms)。
  • 同一轮功能检查:并发(两个并行 session 均成功)、telemetry 关闭、legacy 单 session 全部通过,无残留进程。
  • bundle 闭包:ACP 急切闭包 15.42 MiB / 132 chunks → 13.39 MiB / 130 chunks;急切闭包内 undici 字节 2057 KiB → 0。

测试平台

macOS:单元测试、typecheck、lint、bundle 守卫、针对打包 CLI 的真实模型冒烟。Linux(2C4G):针对打包 CLI 的成对冷启动/预热基准。

风险与范围

  • 主要风险/权衡:--proxy 路径的 dispatcher 安装现在是异步的;config 初始化(以及 channel 启动前)会 await 它,因此相对网络活动的顺序不变。代理 dispatcher 的 undici 加载失败现在记录日志而非从构造函数抛出——此前进程不可能在 undici 未加载的情况下到达该状态。
  • 未验证/超出范围:Cold-start follow-ups: remaining lazy-loading candidates from the ACP eager-closure audit #7264 中列出的其余懒加载候选。
  • 破坏性变更/迁移说明:无。

关联 Issue

属于 #4748 的一部分。实现了 #7264 中的候选项 4(undici 懒加载)。

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

E2E Test & Benchmark Report

Environment: 2C4G Linux (Alibaba Cloud ECS), bundled CLI (dist/cli.js), paired control/candidate runs (alternating order, 30 pairs per scenario) to cancel machine drift. Control = the telemetry-split build (#7276); candidate = this change.

Cold first session (30 pairs)

Metric Control Candidate Δ
process → first session, P50 1336.8 ms 1255.2 ms paired P50 −89.5 ms
process → first session, P95 1370.2 ms 1305.9 ms paired P95 −17.1 ms
mean 1339.0 ms 1253.1 ms −85.9 ms
pairs candidate faster 30 / 30
RSS after first session, P50 423.3 MB 415.2 MB −8.1 MB

Preheated (30 pairs)

Metric Control Candidate
first session latency P50 80.7 ms 78.0 ms
first session latency P95 131.9 ms 84.0 ms

No regression; candidate tail is tighter.

Functional checks (same run)

  • ✅ Concurrent multi-session over one daemon — both sessions succeed, no residual processes
  • ✅ Telemetry disabled — success
  • ✅ Legacy single-session path — success
  • ✅ First-ever start (cache-cold warmup) — success on both variants

Bundle closure & interop (macOS, this branch)

  • ACP eager closure: 15.42 MiB / 132 chunks → 13.39 MiB / 130 chunks; undici bytes in the eager closure 2057 KiB → 0
  • Closure guard passes and now fails CI on any static undici import re-entering the ACP closure
  • Bundled-CLI smoke with a real model round-trip passes — this is the case that exercises esbuild's default-only CJS chunk shape, which local vitest cannot catch
  • Unit tests: core 551 (fetch options / config / IDE client) and cli 105 (preconnect / channel start / GitHub setup / standalone update) all green after rebase onto current main; typecheck / lint / prettier clean

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed performance issue with strong evidence — 30 paired cold-start benchmarks showing −89.5 ms P50 (candidate faster in 30/30 pairs), bundle closure shrinking from 15.42 → 13.39 MiB, and RSS −8.1 MB. Linked to #4748 (daemon cold start) and implements candidate 4 from #7264. This is a measured, tracked problem, not a theoretical one.

Direction: aligned. Daemon cold start is a tracked priority (#4748), and this is the next candidate in the lazy-loading plan (#7264) after the telemetry phases. CHANGELOG reference: Claude Code has done similar lazy-loading work for startup performance (e.g. lazy SDK loading in 1.0.x releases), confirming the area is relevant.

Size: 202 production logic lines (additions + deletions, excluding tests/docs), 114 test lines, 110 design-doc lines. Core paths touched (packages/core/src/config/, packages/core/src/utils/, packages/core/src/ide/, packages/core/src/core/) — author is a collaborator, so the two-tier gate is exempt. Well under the 500-line threshold regardless.

Approach: the scope feels right. All 19 changed files serve the single goal of moving undici behind dynamic imports — 8 value-import sites converted, a single-flight normalizer per package (justified: different undici copies, test mock isolation), ordering guarantees preserved via stored promises, and a bundle guard to prevent regression. No unrelated changes or drive-by refactors. The design doc clearly explains the esbuild CJS interop trap and the alternatives rejected.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的性能问题,有充分证据——30 对冷启动基准测试显示 P50 −89.5 ms(30/30 对 candidate 全部更快),bundle 闭包从 15.42 → 13.39 MiB,RSS −8.1 MB。关联 #4748(daemon 冷启动),实现 #7264 中的候选项 4。这是经过测量和跟踪的问题,不是理论性的。

方向:对齐。Daemon 冷启动是跟踪中的优先级(#4748),这是 telemetry 阶段之后懒加载计划(#7264)中的下一个候选项。CHANGELOG 参考:Claude Code 也做过类似的启动性能懒加载工作(如 1.0.x 版本中的 SDK 懒加载),确认该方向相关。

规模:202 行生产逻辑(增删合计,不含测试/文档),114 行测试,110 行设计文档。触及核心路径(packages/core/src/config/packages/core/src/utils/packages/core/src/ide/packages/core/src/core/)——作者为 collaborator,两层门控豁免。无论如何都远低于 500 行阈值。

方案:范围合理。全部 19 个改动文件都服务于将 undici 移入动态 import 这一单一目标——8 个值导入点转换、每包一个单飞归一化 helper(有正当理由:不同的 undici 拷贝、测试 mock 隔离)、通过存储的 promise 保持顺序保证、以及防止回归的 bundle 守卫。没有无关改动或顺手重构。设计文档清楚解释了 esbuild CJS 互操作陷阱和被否决的替代方案。

进入代码审查 🔍

Qwen Code · qwen3.7-max

Reviewed at 306bf4b105e57bc6dafca711a6760a3da02ef103 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Clean implementation. The approach is exactly what this problem calls for: replace 8 static import { … } from 'undici' sites with dynamic imports behind a single-flight normalizer, preserve ordering guarantees for the proxy dispatcher, and add a bundle guard to prevent regression.

The esbuild CJS interop handling is the subtle part and it's done right — the Object.keys(mod).length === 1 && keys[0] === 'default' check correctly distinguishes the bundled default-only chunk from Node/vitest named exports, without probing mod.default (which would throw on vitest mock proxies). The per-package duplication is justified: cli and core resolve different undici copies, and a shared helper would escape test mocks.

The ordering guarantee for the proxy dispatcher is preserved correctly: Config stores the installation as a proxyDispatcherReady promise and initialize() awaits it before any network activity. The channel proxy path (resolveProxy) is now async and awaited in start.ts. The IDE client builds the agent lazily behind a promise that the fetch wrapper awaits.

No correctness bugs, security holes, or regressions found. No AGENTS.md violations — the code follows project conventions (ESM, no any, kebab-case files, collocated tests, comments explain "why").

One observation (non-blocking): the error handling in config.ts changed from throwing to logging when the proxy dispatcher fails to install. The PR documents this as acceptable since the process previously couldn't reach that state without undici already loaded — reasonable.

Files changed (19)
File What changed
docs/design/2026-07-21-lazy-undici-loading.md Design doc covering the problem, approach, CJS interop, and benchmarks
packages/cli/src/commands/channel/proxy.ts resolveProxy now async, loads undici dynamically
packages/cli/src/commands/channel/start.test.ts Tests updated to await async resolveProxy
packages/cli/src/commands/channel/start.ts Awaits the now-async resolveProxy
packages/cli/src/services/setup-github.ts ProxyAgent loaded via dynamic import
packages/cli/src/utils/apiPreconnect.test.ts Tests use waitForPreconnect for async assertions
packages/cli/src/utils/apiPreconnect.ts Preconnect fires behind async IIFE with dynamic undici
packages/cli/src/utils/gitUtils.ts ProxyAgent loaded via dynamic import
packages/cli/src/utils/load-undici.ts New single-flight helper with CJS default-only normalization
packages/cli/src/utils/standalone-update.ts fetch loaded via dynamic import, type-only import for Response
packages/core/src/config/config.ts Proxy dispatcher installs async, awaited in initialize()
packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts Preloads undici mock before sync constructor
packages/core/src/core/contentGenerator.ts Awaits preloadRuntimeFetchModule before provider constructors
packages/core/src/ide/ide-client.ts IDE fetch agent built lazily behind a promise
packages/core/src/index.ts Exports preloadRuntimeFetchModule
packages/core/src/utils/runtime-fetch-options.no-proxy.test.ts Preloads undici in beforeAll
packages/core/src/utils/runtimeFetchOptions.test.ts Preloads undici mock in beforeAll
packages/core/src/utils/runtimeFetchOptions.ts Core loadUndici/preload/requireUndici helpers, sync builders use requireUndici
scripts/check-serve-fast-path-bundle.js Adds undici to FORBIDDEN_ACP_PACKAGES guard

Unit Tests

All 128 tests pass across both packages:

  • packages/core: runtimeFetchOptions.test.ts (65 tests) + runtime-fetch-options.no-proxy.test.ts (3 tests) — 68 passed
  • packages/cli: apiPreconnect.test.ts (26 tests) + start.test.ts (24 tests) + setup-github.test.ts (10 tests) — 60 passed

Typecheck clean for both packages/core and packages/cli.

Real-Scenario Testing

Dev build (npm run dev)

$ npm run dev -- -p 'reply with exactly: ok' --output-format text 2>&1 | head -50

> @qwen-code/qwen-code@0.20.0 dev
> node scripts/dev.js -p reply with exactly: ok --output-format text

Warning: QWEN_HOME points to "/home/github-runner/actions-runner/_work/_temp/qwen-home" but no settings.json was found there. Existing config remains at "/home/github-runner/.qwen" — OAuth tokens, set
tings, memory, extensions, and skills are not auto-migrated. Copy them manually if you want them to apply at the new location.
Warning: QWEN_HOME points to "/home/github-runner/actions-runner/_work/_temp/qwen-home" but no settings.json was found there. Existing config remains at "/home/github-runner/.qwen" — OAuth tokens, set
tings, memory, extensions, and skills are not auto-migrated. Copy them manually if you want them to apply at the new location.
ok

Bundled CLI (node dist/cli.js) — the critical CJS interop test

$ node dist/cli.js -p 'reply with exactly: ok' --output-format text 2>&1 | head -50
Warning: QWEN_HOME points to "/home/github-runner/actions-runner/_work/_temp/qwen-home" but no settings.json was found there. Existing config remains at "/home/github-runner/.qwen" — OAuth tokens, set
tings, memory, extensions, and skills are not auto-migrated. Copy them manually if you want them to apply at the new location.
Warning: QWEN_HOME points to "/home/github-runner/actions-runner/_work/_temp/qwen-home" but no settings.json was found there. Existing config remains at "/home/github-runner/.qwen" — OAuth tokens, set
tings, memory, extensions, and skills are not auto-migrated. Copy them manually if you want them to apply at the new location.
ok

Both complete a real model round-trip successfully. The bundled CLI smoke confirms the esbuild default-only chunk normalization works end to end — this is the failure mode that unit tests cannot catch.

中文说明

代码审查

实现干净。方案正是这个问题所需要的:将 8 个静态 import { … } from 'undici' 替换为动态 import,收敛到单飞归一化 helper 后面,保持代理 dispatcher 的顺序保证,并添加 bundle 守卫防止回归。

esbuild CJS 互操作处理是微妙的部分,做得正确——Object.keys(mod).length === 1 && keys[0] === 'default' 检查正确区分了打包后的 default-only chunk 和 Node/vitest 的命名导出,且不会探测 mod.default(那会在 vitest mock 代理上抛异常)。每包复制一份 helper 有正当理由:cli 和 core 解析不同的 undici 拷贝,共享 helper 会绕过测试 mock。

代理 dispatcher 的顺序保证被正确保留:Config 将安装存为 proxyDispatcherReady promise,initialize() 在任何网络活动前 await 它。channel 代理路径(resolveProxy)现在是异步的,在 start.ts 中被 await。IDE 客户端在 promise 后面懒构建 agent,fetch 包装器 await 它。

未发现正确性 bug、安全漏洞或回归。无 AGENTS.md 违规——代码遵循项目规范(ESM、无 any、kebab-case 文件名、测试共置、注释解释"为什么")。

一个观察(非阻塞):config.ts 中代理 dispatcher 安装失败时的错误处理从抛异常改为记录日志。PR 文档说明这是可接受的,因为此前进程不可能在 undici 未加载的情况下到达该状态——合理。

单元测试

两个包共 128 个测试全部通过。Typecheck 干净。

真实场景测试

dev 构建和打包 CLI 均成功完成真实模型往返。打包 CLI 冒烟确认了 esbuild default-only chunk 归一化端到端工作——这是单元测试无法捕获的失败模式。

Qwen Code · qwen3.7-max

Reviewed at 306bf4b105e57bc6dafca711a6760a3da02ef103 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean across every stage; would merge without hesitation.

This is a well-executed performance PR. The problem is real and measured (−89.5 ms P50 cold start, 30/30 paired benchmarks, −2 MiB eager closure), the implementation is the minimal change that solves it, and the tricky part — esbuild's CJS default-only chunk shape — is handled correctly and verified with a bundled CLI smoke test that unit tests can't cover.

Going back to my independent proposal: I would have done essentially the same thing — dynamic imports at each use site, a normalizer for the CJS interop, preload/require split for sync consumers, stored promise for the proxy dispatcher ordering. The PR matches or exceeds that in every respect. The per-package helper duplication is the right call (different undici copies, test mock isolation), and the design doc explains the rejected alternatives clearly.

Every change in the diff serves the stated goal. No drive-by refactors, no scope creep. The bundle guard is a nice touch — it makes the invariant self-enforcing. If I had to maintain this in six months, I'd thank the author: the code is straightforward, the comments explain the non-obvious "why", and the fail-loud requireUndici() guard makes misuse impossible to miss.

中文说明

置信度:5/5 —— 每个阶段都干净;毫不犹豫可以合并。

这是一个执行良好的性能 PR。问题真实且有测量数据(冷启动 P50 −89.5 ms,30/30 对基准,急切闭包 −2 MiB),实现是解决问题的最小改动,微妙的部分——esbuild 的 CJS default-only chunk 形态——处理正确,并通过单元测试无法覆盖的打包 CLI 冒烟测试验证。

回顾我的独立方案:我基本上会做同样的事——每个使用点动态 import、CJS 互操作归一化、同步消费者的 preload/require 分离、代理 dispatcher 顺序的存储 promise。PR 在每个方面都达到或超过了这个方案。每包复制 helper 是正确的选择(不同的 undici 拷贝、测试 mock 隔离),设计文档清楚解释了被否决的替代方案。

diff 中的每个改动都服务于既定目标。没有顺手重构,没有范围膨胀。bundle 守卫是一个很好的补充——它使不变量自我执行。如果六个月后我要维护这段代码,我会感谢作者:代码直白,注释解释了非显而易见的"为什么",fail-loud 的 requireUndici() 守卫使误用不可能被忽略。

Qwen Code · qwen3.7-max

Reviewed at 306bf4b105e57bc6dafca711a6760a3da02ef103 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

Comment thread packages/core/src/utils/runtimeFetchOptions.ts
Comment thread packages/core/src/ide/ide-client.ts
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/utils/runtimeFetchOptions.ts
Address review: web search builds fetch options outside the content
generator preload path, so 33 web-search tests (and any standalone
search invocation) hit the requireUndici fail-loud guard. Also redact
and rethrow proxy dispatcher install failures, guard early promise
rejections against unhandledRejection, and pin the guard message with
a test.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/config/config.ts
Comment thread packages/cli/src/utils/gitUtils.ts
Comment thread packages/cli/src/utils/load-undici.ts
Address review suggestions: add parameterized tests for the CJS
unwrap normalization used by both core and cli loadUndici helpers,
and verify getLatestGitHubRelease instantiates ProxyAgent when a
proxy argument is passed.
Export UndiciModule type and loosen test helper typing so the cli
package builds under tsc --build.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

@doudouOUC
doudouOUC added this pull request to the merge queue Jul 22, 2026
Merged via the queue into QwenLM:main with commit 042f637 Jul 22, 2026
45 checks passed
@wenshao

wenshao commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Local verification — ✅ all headline claims reproduce

I verified this PR locally as a maintainer on a real npm ci worktree (not symlinked deps), building both the PR head (301c9c7) and its base (567de03, the parent of the first PR commit). Every load-bearing claim in the description holds; I found no regressions. One transparency note on a harness limitation is at the bottom.

Environment: Linux, Node v22.22.2, npm 10.9.7; PR + base each built with npm ciDEV=true npm run bundle.

What I confirmed

# Claim Result
1 undici leaves the ACP eager static closure; guard passes check-serve-fast-path-bundle.js exit 0; undici in closure 2057.4 KiB → 0
2 Eager closure shrinks ~2 MiB 15.60 → 13.58 MiB (−2.02 MiB); base carried two static undici copies
3 The guard now fails on a static undici re-import in the ACP closure ✅ PR guard flags 2 offenders on the base metafile (acpAgent → undici, ~1 MiB each)
4 esbuild compiles undici into a default-only chunk (the interop trap) ✅ bundled chunk = export default require_undici(), 0 named exports
5 The loadUndici unwrap is load-bearing and unit tests can't catch it ✅ A/B: PR bundle → ok; naive helper (no unwrap) → Agent is not a constructor
6 Listed unit suites pass ✅ core 185/185, cli 97/97 (incl. new interop / fail-loud-guard / ProxyAgent tests)
7 Async proxy-dispatcher ordering is preserved ✅ fail-loud before preload; EnvHttpProxyAgent after; initialize() awaits it

Structural evidence (bundle guard + metafile + chunk shape)

structural

  • [1] Guard passes on the PR bundle: undici is no longer statically reachable from acpAgent.
  • [2] Metafile diff (my own walker over dist/esbuild.json, following only non-dynamic-import edges from acpAgent): base eager closure 15.60 MiB with 2057.4 KiB of undici inside it; PR closure 13.58 MiB with 0. undici is retained as two DYNAMIC chunks (one per package copy) — i.e. still bundled, just code-split behind the dynamic import(). The −2.02 MiB / 2057 KiB→0 deltas match the description (small absolute differences from the PR's 15.42→13.39 are just how the closure set is totaled).
  • [3] Running the PR's guard against the base metafile reports exactly 2 undici offenders, both on the static path acpAgent-…js → chunk-…js, confirming the guard actually catches the regression it claims to.
  • [4] The bundled undici chunk really is default-only: grep -c 'export{' = 0, sole export export default require_undici(). So a naive const { Agent } = await import('undici') destructures undefined in the bundle — the trap the PR describes.

Runtime A/B (the trap is real, and invisible to vitest)

runtime

  • [6] Bundled-CLI round-trip against a mock OpenAI endpoint (node dist/cli.js -p "reply with exactly: ok", OpenAI auth): the PR bundle completes with stdout ok, exit 0 — exercising the default-only chunk end to end via createContentGenerator → preloadRuntimeFetchModule → new Agent(...).
  • The killer A/B: I replaced only the helper's unwrap with the naive form (import('undici').then(mod => mod)), rebuilt core + rebundled, and the same round-trip died with Agent is not a constructor (exit 1). This is precisely the failure "local test runs cannot catch" — the 282 unit tests above stay green because vitest synthesizes named exports for the CJS module. Restored afterward.
  • [7] Ordering guarantee (compiled core/dist artifact): calling a sync dispatcher builder before preload throws the actionable undici is not loaded yet; await preloadRuntimeFetchModule() guard; after await preloadRuntimeFetchModule() it returns a live EnvHttpProxyAgent. That is the mechanism behind Config.initialize() awaiting proxyDispatcherReady before any network activity.

Repro

# worktrees: PR head + base (parent of first PR commit)
git worktree add wt-pr 301c9c7 && git worktree add --detach wt-base 567de03
for d in wt-pr wt-base; do (cd $d && npm ci && DEV=true npm run bundle); done

# [1] guard  [2]/[3] metafile  [4] chunk shape
cd wt-pr && node scripts/check-serve-fast-path-bundle.js
grep -c 'export{' dist/chunks/undici-*.js          # -> 0
grep -o 'export default [a-z_()]*' dist/chunks/undici-*.js

# [6] bundled round-trip vs a mock OpenAI server (OPENAI_* + QWEN_DEFAULT_AUTH_TYPE=openai)
node dist/cli.js -p "reply with exactly: ok"        # -> ok
# A/B: patch loadUndici to `import('undici').then(m=>m)`, rebuild core+bundle -> "Agent is not a constructor"

# [7] ordering, against built core
node -e "const m=await import('./packages/core/dist/src/utils/runtimeFetchOptions.js');
try{m.getOrCreateSharedDispatcher('http://p')}catch(e){console.log('throws:',e.message)}
await m.preloadRuntimeFetchModule(); console.log(m.getOrCreateSharedDispatcher('http://p').constructor.name)"

Note (not a blocker)

I also tried a live --proxy round-trip (forcing outbound traffic through a local forward proxy). It stalled (Operation cancelled) — but the base bundle stalls identically, so this is a limitation of proxying localhost-http through a localhost-http proxy in my harness, not a regression from this PR. The async-install ordering itself is covered deterministically by [7] and by the runtime-fetch-options.no-proxy suite.

Verdict: verification-wise this is solid — the perf claim (undici out of cold start, ~2 MiB closure drop) and the subtle esbuild-interop fix both hold up under real builds, and the new guard + tests genuinely lock them in. LGTM from my side.

中文版(合并参考)

本地验证 — ✅ 所有关键结论均可复现

我作为维护者在真实 npm ci 工作树(未软链依赖)上验证了本 PR,分别构建了 PR head(301c9c7)与其 base(567de03,即 PR 首个 commit 的父提交)。描述中每一条关键结论都成立,未发现回归。末尾有一条关于测试环境限制的透明说明。

环境: Linux,Node v22.22.2,npm 10.9.7;PR 与 base 均 npm ciDEV=true npm run bundle

确认项

# 结论 结果
1 undici 移出 ACP 急切静态闭包,守卫通过 ✅ 守卫 exit 0;闭包内 undici 2057.4 KiB → 0
2 急切闭包缩小约 2 MiB 15.60 → 13.58 MiB(−2.02 MiB);base 中带有两份静态 undici 拷贝
3 守卫现在会对闭包内静态 undici 重新导入报错 ✅ 用 PR 的守卫跑 base metafile,报出 2 个 offenderacpAgent → undici,各约 1 MiB)
4 esbuild 把 undici 编译成 default-only chunk(互操作陷阱) ✅ 打包 chunk = export default require_undici()命名导出为 0
5 loadUndici 的解包是关键,且单测无法发现该问题 ✅ A/B:PR 打包 → ok;朴素 helper(不解包)→ Agent is not a constructor
6 列出的单测套件通过 ✅ core 185/185,cli 97/97(含新增互操作 / fail-loud 守卫 / ProxyAgent 测试)
7 异步代理 dispatcher 的顺序保证保持不变 ✅ preload 前 fail-loud;preload 后得到 EnvHttpProxyAgentinitialize() 会 await

结构性证据(见上方第一张图)

  • [1] 守卫在 PR 打包上通过:undici 不再从 acpAgent 静态可达。
  • [2] metafile 对比(我自写的遍历器,从 acpAgent 只沿非 dynamic-import 边走):base 急切闭包 15.60 MiB、内含 2057.4 KiB undici;PR 闭包 13.58 MiB、内含 0。undici 仍在包内,但被拆成两个 DYNAMIC chunk(每个包一份),即代码分割到动态 import() 之后。−2.02 MiB / 2057 KiB→0 与描述一致(与 15.42→13.39 的细微绝对差异只是闭包集合的统计口径不同)。
  • [3]PR 的守卫跑 base metafile 恰好报出 2 个 undici offender,静态路径均为 acpAgent-…js → chunk-…js,证明守卫确实能捕获它所声称要防的回归。
  • [4] 打包后的 undici chunk 确为 default-only:grep -c 'export{' = 0,唯一导出 export default require_undici()。因此裸写 const { Agent } = await import('undici') 在打包后解构出 undefined

运行时 A/B(陷阱是真实的,且单测看不见,见上方第二张图)

  • [6] 针对 mock OpenAI 端点的打包 CLI 往返node dist/cli.js -p "reply with exactly: ok",OpenAI 认证):PR 打包输出 ok、exit 0,端到端走过了 default-only chunk(createContentGenerator → preloadRuntimeFetchModule → new Agent(...))。
  • 关键 A/B: 我只把 helper 的解包改成朴素形式(import('undici').then(mod => mod)),重建 core 并重打包,同样的往返即报 Agent is not a constructor(exit 1)。这正是"本地测试无法发现"的失败——上面 282 个单测仍全绿,因为 vitest 会为 CJS 合成命名导出。验证后已还原。
  • [7] 顺序保证(编译后的 core/dist 产物):preload 前调用同步 dispatcher 构造会抛出可操作的 undici is not loaded yet; await preloadRuntimeFetchModule() 守卫;await preloadRuntimeFetchModule() 后返回真实的 EnvHttpProxyAgent。这就是 Config.initialize() 在任何网络活动前 await proxyDispatcherReady 的机制。

说明(非阻塞)

我还尝试了真实 --proxy 往返(强制出站流量走本地 forward proxy),结果卡住(Operation cancelled)——但 base 打包同样卡住,所以这是我这套环境里"本地 http 经本地 http 代理"的限制,不是本 PR 的回归。异步安装的顺序本身已由 [7] 与 runtime-fetch-options.no-proxy 套件确定性覆盖。

结论: 从验证角度看非常扎实——性能结论(undici 移出冷启动、闭包缩小约 2 MiB)与那个微妙的 esbuild 互操作修复在真实构建下都成立,新增的守卫 + 测试也确实把它们锁住了。我这边 LGTM。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants