Skip to content

fix(standalone): Route serve shim through cli-entry - #5977

Merged
doudouOUC merged 2 commits into
QwenLM:mainfrom
doudouOUC:fix/standalone-serve-shim-fast-path
Jun 29, 2026
Merged

fix(standalone): Route serve shim through cli-entry#5977
doudouOUC merged 2 commits into
QwenLM:mainfrom
doudouOUC:fix/standalone-serve-shim-fast-path

Conversation

@doudouOUC

@doudouOUC doudouOUC commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Standalone archives now route qwen serve through the packaged CLI entry wrapper while keeping other standalone commands on the existing direct node --expose-gc lib/cli.js path. The standalone package builder also treats the entry wrapper as a required runtime artifact so release archives cannot be produced without the file that the serve fast path needs. When prepare:package has staged npm-only artifacts in dist/, standalone packaging now skips those artifacts instead of failing or copying them into the archive.

Why it's needed

Recent daemon startup work optimized the npm-installed cli-entry.js path, but the standalone bin/qwen and bin/qwen.cmd shims still invoked lib/cli.js directly with --expose-gc. That bypassed the compile-cache and serve fast-path behavior for standalone users, leaving daemon startup slower in the installation mode used by hosted standalone releases. Requiring cli-entry.js also needs to work with the documented bundle -> prepare:package -> package:standalone flow, where prepare:package writes npm package-only files that standalone archives should not ship.

Reviewer Test Plan

How to verify

Build a standalone archive and inspect the generated launchers. qwen serve should execute lib/cli-entry.js, while non-serve commands should still execute node --expose-gc lib/cli.js. The package builder should fail if dist/cli-entry.js is missing, and it should still package successfully when dist/ contains npm-only postinstall.js and patches artifacts from prepare:package without copying those artifacts into lib/.

Evidence (Before & After)

N/A - non-UI packaging fix. The regression coverage asserts the generated Unix and Windows archive shims, the missing-entry failure path, and the prepared dist/ case with npm-only artifacts.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️ not tested locally; Windows archive content is covered by script tests
🐧 Linux ⚠️ not tested locally; Unix archive content is covered by script tests

Environment (optional)

macOS local worktree with Node.js v26.0.0. Dependencies installed with npm ci --ignore-scripts --no-audit --progress=false in a clean worktree before validation.

Risk & Scope

  • Main risk or tradeoff: The serve fast path only matches qwen serve, so commands with flags before the subcommand continue to use the existing fallback behavior. Standalone archives intentionally skip npm-only postinstall artifacts because no npm install lifecycle runs inside the archive.
  • Not validated / out of scope: This PR does not move getCliVersion() after listen and does not change ACP bridge import timing; those remain separate startup follow-ups.
  • Breaking changes / migration notes: None. Non-serve standalone commands keep the existing --expose-gc path.

Linked Issues

Relates to #4748

中文说明

What this PR does

Standalone 包现在让 qwen serve 通过打包进去的 CLI entry wrapper 启动,同时保持其它 standalone 命令继续走现有的直接 node --expose-gc lib/cli.js 路径。standalone 打包脚本也把这个 entry wrapper 设为必需运行时产物,避免生成缺少 serve fast path 入口文件的发布包。当 prepare:package 已经在 dist/ 中放入 npm-only artifacts 时,standalone 打包现在会跳过这些文件,而不是失败或把它们复制进 archive。

Why it's needed

近期 daemon 启动优化已经覆盖 npm 安装的 cli-entry.js 路径,但 standalone 的 bin/qwenbin/qwen.cmd 仍然直接带 --expose-gc 调用 lib/cli.js。这会让 standalone 用户绕过 compile cache 和 serve fast path,导致 hosted standalone release 使用的安装方式里 daemon 启动仍然偏慢。把 cli-entry.js 设为必需文件后,也需要兼容文档里的 bundle -> prepare:package -> package:standalone 流程,因为 prepare:package 会写入 standalone archive 不应该携带的 npm package-only 文件。

Reviewer Test Plan

How to verify

构建 standalone archive 并检查生成的启动脚本。qwen serve 应该执行 lib/cli-entry.js,非 serve 命令应该仍然执行 node --expose-gc lib/cli.js。如果缺少 dist/cli-entry.js,打包脚本应该失败;当 dist/ 中含有 prepare:package 生成的 npm-only postinstall.jspatches artifacts 时,打包应该仍然成功,并且这些 artifacts 不会被复制进 lib/

Evidence (Before & After)

N/A - 非 UI 的打包修复。回归测试会断言生成的 Unix 和 Windows archive shim 内容、缺失 entry wrapper 时的失败路径,以及准备过的 dist/ 中含 npm-only artifacts 的场景。

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️ 本地未测试;Windows archive 内容由脚本测试覆盖
🐧 Linux ⚠️ 本地未测试;Unix archive 内容由脚本测试覆盖

Environment (optional)

macOS 本地干净 worktree,Node.js v26.0.0。验证前在干净 worktree 里使用 npm ci --ignore-scripts --no-audit --progress=false 安装依赖。

Risk & Scope

  • Main risk or tradeoff: serve fast path 只匹配 qwen serve,所以子命令前带 flag 的调用会继续使用现有 fallback 行为。standalone archive 会有意跳过 npm-only postinstall artifacts,因为 archive 内部不会运行 npm install lifecycle。
  • Not validated / out of scope: 这个 PR 不把 getCliVersion() 移到 listen 之后,也不修改 ACP bridge 的导入时机;这些仍是独立的启动优化后续项。
  • Breaking changes / migration notes: 无。非 serve standalone 命令保留现有 --expose-gc 路径。

Linked Issues

Relates to #4748

@doudouOUC
doudouOUC marked this pull request as ready for review June 29, 2026 03:54
Copilot AI review requested due to automatic review settings June 29, 2026 03:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required sections present, bilingual, risk/scope well articulated.

On direction: this is a narrowly-scoped packaging fix — extending the existing cli-entry.js fast path (compile cache + in-process import for serve) to standalone archives. Clearly aligned with the project's daemon startup performance work (#4748). No product-direction concerns.

On approach: 2 files, +157/-5 — the diff is tight and every line serves the stated goal. Shell-level if $1 == serve branching in generated shims is the simplest possible implementation. Making cli-entry.js a required dist asset is a sensible guard. The npm-only artifact skip (DIST_NPM_PACKAGE_ONLY_ENTRIES) fixes a real gap in the bundle → prepare:package → package:standalone flow. No scope creep.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ — 所有必需章节齐全,双语,风险/范围描述清晰。

方向:范围很窄的打包修复——把已有的 cli-entry.js 快速路径(compile cache + serve 的进程内 import)扩展到 standalone 包。与项目的 daemon 启动性能优化方向(#4748)一致,无产品方向顾虑。

方案:2 个文件,+157/-5——diff 紧凑,每行都服务于目标。生成 shim 中 shell 层级的 if $1 == serve 分支是最简实现。将 cli-entry.js 设为必需 dist 产物是合理的防护。npm-only 产物跳过(DIST_NPM_PACKAGE_ONLY_ENTRIES)修复了 bundle → prepare:package → package:standalone 流程中的真实缺陷。无范围蔓延。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal before reading the diff: modify writeShims() to add if $1 == serve → cli-entry.js branching in both Unix and Windows shim generators, add cli-entry.js to DIST_REQUIRED_PATHS, and handle the prepare:package npm-only artifact case in copyRuntimeAssets. This is exactly what the PR does — no simpler path missed.

Reviewed the diff carefully:

  • Unix shim: ${1:-} with set -e is correct POSIX sh; exec replaces the process cleanly. Serve check before the generic exec fallback — correct ordering.
  • Windows shim: %~1 strips quotes correctly; goto + exit /b %ERRORLEVEL% is idiomatic batch, both branches exit explicitly so no fall-through.
  • DIST_REQUIRED_PATHS: cli-entry.js promoted from allowed to required. Improved error message points to npm run bundle and npm run prepare:package — developer-friendly.
  • DIST_NPM_PACKAGE_ONLY_ENTRIES: postinstall.js and patches skipped in copyRuntimeAssets — fixes the real prepare:package → package:standalone flow breakage. The set is small and well-named.
  • ensureMinimalDist parameters: includeCliEntry (default true) and includeNpmPackageArtifacts (default false) — backward-compatible, existing tests unaffected.

No critical blockers, no AGENTS.md violations. Clean, minimal diff.

Testing

Ran the full standalone release packaging suite against PR head ecba1e90 on Linux (Node 22.x):

 ❯ scripts/tests/install-script.test.js (115 tests | 1 failed | 69 skipped) 970ms

   ✓ standalone release packaging > requires the standalone cli-entry wrapper in dist 32ms
   ✓ standalone release packaging > packages a win-x64 standalone archive 47ms
   ✓ standalone release packaging > skips npm-only artifacts staged in dist 48ms
   ✓ standalone release packaging > packages a Unix standalone archive with a serve fast path shim 50ms

   × standalone release packaging > does not package audio-capture test artifacts
     → ENOENT: packages/audio-capture/dist (pre-existing build environment issue, not related to this PR)

   Test Files  1 failed (1)
        Tests  1 failed | 45 passed | 69 skipped (115)

All 4 PR-specific tests pass. The single failure is a pre-existing issue — packages/audio-capture/dist doesn't exist because the package wasn't built in this worktree.

Note on tmux testing: this PR modifies build/packaging scripts (shim generators and copyRuntimeAssets), not runtime CLI behavior. The generated shim content is what's being tested, not interactive terminal output — tmux testing is not applicable here. Unit tests are the correct verification approach for this type of change.

Maintainer @wenshao has independently verified this PR with a real end-to-end standalone build on macOS, live process-tree inspection confirming qwen serve routes through cli-entry.js, and mutation testing proving each source change is guarded by its corresponding test. That verification goes beyond what unit tests alone can confirm.

中文说明

代码审查

看 diff 之前的独立方案:修改 writeShims() 在 Unix 和 Windows shim 生成器中加入 if $1 == serve → cli-entry.js 分支,把 cli-entry.js 加入 DIST_REQUIRED_PATHS,并在 copyRuntimeAssets 中处理 prepare:package 的 npm-only 产物。PR 的实现与此完全一致——没有遗漏更简单的路径。

逐项审查:

  • Unix shimset -e${1:-} 是正确 POSIX sh;exec 替换进程。serve 检查在通用 exec 之前——顺序正确。
  • Windows shim%~1 正确剥离引号;goto + exit /b %ERRORLEVEL% 是标准 batch,两个分支都有显式 exit,无 fall-through。
  • DIST_REQUIRED_PATHScli-entry.js 从允许升级为必需。改进后的错误信息指向 npm run bundlenpm run prepare:package
  • DIST_NPM_PACKAGE_ONLY_ENTRIESpostinstall.jspatchescopyRuntimeAssets 中跳过——修复了真实的 prepare:package → package:standalone 流程断裂。
  • ensureMinimalDist 参数includeCliEntry(默认 true)和 includeNpmPackageArtifacts(默认 false)向后兼容。

无关键阻塞项,无 AGENTS.md 违规。Diff 干净、最小化。

测试

在 Linux(Node 22.x)上对 PR head ecba1e90 运行完整 standalone release packaging 测试套件:全部 4 个 PR 新增测试通过。唯一失败项(does not package audio-capture test artifacts)是预先存在的构建环境问题,与本 PR 无关。

tmux 测试说明: 本 PR 修改的是构建/打包脚本(shim 生成器和 copyRuntimeAssets),非运行时 CLI 行为。tmux 测试不适用于此类变更。

维护者 @wenshao 已独立验证此 PR:macOS 上的真实端到端 standalone 构建、进程树检查确认 qwen serve 经由 cli-entry.js、变异测试证明每处源码改动都有对应测试守护。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

This is a focused, well-scoped packaging fix. The motivation is clear — standalone users were missing the serve fast path that npm installs already had. The implementation is the minimal change: shell-level branching in two generated shims, a required-asset guard, and an npm-only artifact skip for the prepare:package → package:standalone flow.

My independent proposal before reading the diff was identical — if serve → cli-entry.js in the shim generators is the simplest possible approach. Every change in the diff serves the stated goal; nothing is unnecessary.

All 4 new tests pass on Linux. The one pre-existing failure is unrelated. Maintainer @wenshao's independent verification — real end-to-end build, live process-tree confirmation, and mutation testing — provides strong evidence that this works correctly and the regression tests actually guard the change.

LGTM, approving. ✅

中文说明

这是一个聚焦、范围合理的打包修复。动机清晰——standalone 用户缺少 npm 安装已有的 serve 快速路径。实现是最小改动:两个生成 shim 中的 shell 分支、必需产物守卫、以及 prepare:package → package:standalone 流程中的 npm-only 产物跳过。

看 diff 之前的独立方案与提交完全一致——shim 生成器中的 if serve → cli-entry.js 是最简单的实现。diff 中每处改动都服务于目标,没有多余内容。

Linux 上 4 个新增测试全部通过。1 个预先存在的失败与 PR 无关。维护者 @wenshao 的独立验证——真实端到端构建、进程树确认、变异测试——提供了强有力的证据。

LGTM,批准。✅

Qwen Code · qwen3.7-max

@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. ✅

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

@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 review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

wenshao
wenshao previously approved these changes Jun 29, 2026

@wenshao wenshao 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

@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 review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

@doudouOUC doudouOUC self-assigned this Jun 29, 2026

@wenshao wenshao 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.

The PR description reports 2 files / +154/-5, but the actual branch diff against main is 22 files / +5261/-51 — the branch bundles previously-reviewed features (loop task file + tick resolver from #5884/#5890, autofix workflow tiering, relative postinstall patch dir from #5973) on top of the titular serve shim. Triage stages 1-3 evaluated only the described scope; reviewers skimming the description will not expect ~5k lines of loop / cronScheduler / workflow changes when they open the diff. Consider splitting the bundled features into separate PRs so each lands with an accurate description and a focused review.

Comment thread scripts/tests/install-script.test.js

@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 critical issues found. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

Comment thread scripts/create-standalone-package.js
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@wenshao

wenshao commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

✅ Maintainer local verification — real build + functional + mutation testing

Verified PR head ecba1e90 (rebased on current main, merge-base = 98dec4fa) on a clean worktree. This goes beyond the script-level test coverage: I built a real standalone archive with a real Node runtime and ran it end-to-end, then confirmed the regression tests actually guard the change via mutation testing. Verdict: looks correct, safe to merge.

Environment: macOS (darwin-arm64), Node v22.22.2, fresh npm ci worktree.


1. Real end-to-end standalone build (darwin-arm64)

Ran the documented flow that this PR targets — npm run bundlenpm run prepare:packagepackage:standalone — against a real Node runtime archive, the exact case the old code broke on.

  • prepare:package did stage the npm-only artifacts into dist/ (postinstall.js, patches/), reproducing the failure scenario.
  • package:standalone succeeded (before this PR it would abort with Unexpected dist asset: …/dist/patches).
  • Extracted archive contents — npm-only artifacts correctly skipped, fast-path entry present:
PRESENT: lib/cli-entry.js
PRESENT: lib/cli.js
ABSENT : lib/postinstall.js      # npm-only, correctly skipped
ABSENT : lib/patches             # npm-only, correctly skipped
  • Generated bin/qwen (Unix) and bin/qwen.cmd (Windows) carry the exact serve-routing logic, and the .cmd has both exit /b %ERRORLEVEL% guards so non-serve never falls through into :serve.

2. Functional run of the built archive (the key evidence)

Launched the real packaged binary bin/qwen serve --port 41799 --no-web and inspected the live process tree:

65019  65008  …/qwen-code/node/bin/node  …/lib/cli-entry.js serve --port 41799 --no-web
65048  65019  …/qwen-code/node/bin/node  --max-old-space-size=16384 --expose-gc  …/lib/cli.js --acp

This matches the design exactly:

  • Daemon host (65019) runs through lib/cli-entry.js → in-process import of cli.js (the serve fast path: single process, enableCompileCache(), no --expose-gc on the host — which the npm install does too).
  • ACP child (65048) independently adds --expose-gc, exactly as the cli-entry.js comment promises ("the daemon host never calls global.gc() — only its ACP children do").

Daemon was fully functional:

qwen serve listening on http://127.0.0.1:41799 (mode=http-bridge, …)
startup timing: processToListenMs=270
GET /health → {"status":"ok"}

Non-serve path still uses the original launcher (verified live):

67438  67183  …/node/bin/node  --expose-gc  …/lib/cli.js --version
→ 0.19.3

3. Regression tests

Full scripts/tests/install-script.test.js on the PR head: 106 passed | 9 skipped (9 are platform-gated). The standalone release packaging block (incl. the 3 new/updated cases) is all green.

Note: one [vitest-worker]: Timeout calling "onTaskUpdate" was emitted once under the suite's high thread count — a vitest IPC flake, not a test failure (all assertions passed; not reproducible on the targeted re-runs).

4. Mutation testing (proves the new tests actually catch regressions)

Reverted each source change individually and confirmed the matching test flips to red:

Reverted change Test that failed Failure
serve routing in both shims packages a Unix standalone archive with a serve fast path shim + packages a win-x64 standalone archive expected '…' to contain 'if [ "${1:-}" = "serve" ]; then'
cli-entry.js removed from DIST_REQUIRED_PATHS requires the standalone cli-entry wrapper in dist expected throw, none thrown
npm-only skip removed from copyRuntimeAssets skips npm-only artifacts staged in dist Unexpected dist asset: …/dist/patches

The third mutation also confirms the exact bug this PR fixes: without the skip, the real prepare:package → package:standalone flow aborts on the staged patches/ dir.


Notes / observations (non-blocking)

  • The serve path intentionally drops --expose-gc from the daemon host process (it previously had it). This is by design and converges standalone with npm-install behavior — global.gc() is only used by ACP children, which still get --expose-gc (confirmed in the process tree above). Not a regression.
  • Fast-path routing only triggers when serve is the first argument (qwen --flag serve falls back), which the PR already documents and is consistent with cli-entry.js's own argv[2] === 'serve' check.
  • Windows .cmd was validated by content/structure + the win-x64 packaging test (can't execute cmd.exe on macOS); the Unix shim — identical across macOS/Linux — was exercised live on darwin.
🇨🇳 中文版本(完整对应)

✅ 维护者本地验证 —— 真实构建 + 功能运行 + 变异测试

在干净 worktree 上验证 PR head ecba1e90(已基于当前 main,merge-base = 98dec4fa)。验证范围超出脚本级测试:我用真实 Node 运行时构建了真正的 standalone 包并端到端运行,再用变异测试确认回归测试确实能守住本次改动。结论:实现正确,可以合并。

环境: macOS(darwin-arm64),Node v22.22.2,全新 npm ci 的 worktree。


1. 真实端到端 standalone 构建(darwin-arm64)

按本 PR 针对的文档流程跑了一遍 —— npm run bundlenpm run prepare:packagepackage:standalone —— 并使用真实的 Node 运行时 archive,正是旧代码会出错的场景。

  • prepare:package 确实把 npm-only 产物写进了 dist/postinstall.jspatches/),复现了失败场景。
  • package:standalone 构建成功(本 PR 之前会以 Unexpected dist asset: …/dist/patches 中止)。
  • 解包后内容 —— npm-only 产物被正确跳过,fast-path 入口存在:
PRESENT: lib/cli-entry.js
PRESENT: lib/cli.js
ABSENT : lib/postinstall.js      # npm-only,已正确跳过
ABSENT : lib/patches             # npm-only,已正确跳过
  • 生成的 bin/qwen(Unix)和 bin/qwen.cmd(Windows)都带有正确的 serve 路由逻辑;.cmd 含两处 exit /b %ERRORLEVEL% 守卫,保证非 serve 命令不会 fall-through 进入 :serve

2. 对构建产物的真实功能运行(关键证据)

启动真实打包出来的二进制 bin/qwen serve --port 41799 --no-web,检查运行时进程树:

65019  65008  …/qwen-code/node/bin/node  …/lib/cli-entry.js serve --port 41799 --no-web
65048  65019  …/qwen-code/node/bin/node  --max-old-space-size=16384 --expose-gc  …/lib/cli.js --acp

完全符合设计:

  • Daemon 主进程(65019)lib/cli-entry.js → in-process import cli.js(即 serve fast path:单进程、enableCompileCache()、主进程不带 --expose-gc —— 与 npm 安装方式一致)。
  • ACP 子进程(65048) 自己独立加 --expose-gc,正如 cli-entry.js 注释所述("daemon 主进程从不调用 global.gc(),只有它的 ACP 子进程会")。

Daemon 完全可用:

qwen serve listening on http://127.0.0.1:41799 (mode=http-bridge, …)
startup timing: processToListenMs=270
GET /health → {"status":"ok"}

非 serve 路径仍走原启动器(已实测):

67438  67183  …/node/bin/node  --expose-gc  …/lib/cli.js --version
→ 0.19.3

3. 回归测试

在 PR head 上跑完整的 scripts/tests/install-script.test.js106 通过 | 9 跳过(9 个是平台门控)。standalone release packaging 块(含 3 个新增/更新用例)全绿。

说明:高线程数下偶发一次 [vitest-worker]: Timeout calling "onTaskUpdate" —— 这是 vitest 的 IPC 抖动,不是测试失败(所有断言均通过,定向重跑不复现)。

4. 变异测试(证明新增测试确实能抓回归)

逐一回退每处源码改动,确认对应测试翻红:

回退的改动 失败的测试 失败信息
两个 shim 的 serve 路由 packages a Unix standalone archive with a serve fast path shim + packages a win-x64 standalone archive expected '…' to contain 'if [ "${1:-}" = "serve" ]; then'
DIST_REQUIRED_PATHS 移除 cli-entry.js requires the standalone cli-entry wrapper in dist 期望抛错,实际未抛
copyRuntimeAssets 移除 npm-only 跳过 skips npm-only artifacts staged in dist Unexpected dist asset: …/dist/patches

第三个变异同时印证了本 PR 修复的确切 bug:缺少该跳过逻辑时,真实的 prepare:package → package:standalone 流程会在 staged 的 patches/ 目录上中止。


备注 / 观察(不阻塞合并)

  • serve 路径有意让 daemon 主进程不再带 --expose-gc(之前是带的)。这是有意为之,使 standalone 与 npm 安装行为收敛 —— global.gc() 只被 ACP 子进程使用,而子进程仍会拿到 --expose-gc(见上方进程树)。不是回归。
  • Fast-path 路由仅在 serve第一个参数时触发(qwen --flag serve 会走 fallback),PR 已说明此限制,且与 cli-entry.js 自身 argv[2] === 'serve' 的判断一致。
  • Windows .cmd 通过内容/结构核对 + win-x64 打包测试验证(macOS 无法执行 cmd.exe);Unix shim —— macOS/Linux 完全相同 —— 已在 darwin 上实测运行。

@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 review findings. Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

@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. ✅

@wenshao

wenshao commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

✅ Local real-build verification (maintainer reference)

Verified PR head ecba1e90b in an isolated worktree on macOS darwin-arm64 / Node v22.22.2, driven through a real tmux session. Beyond the unit tests, I built an actual standalone archive (real Node runtime + the genuine cli-entry.js wrapper that prepare:package emits) and executed the generated shim to observe routing at runtime.

1. Packaging test suite (scripts/tests/install-script.test.js, at PR head)

105 passed · 9 skipped (platform-gated) · 1 failed

The 4 new/changed tests are all green:

  • requires the standalone cli-entry wrapper in dist
  • packages a win-x64 standalone archive (asserts the new goto serve + two exit /b)
  • skips npm-only artifacts staged in dist
  • packages a Unix standalone archive with a serve fast path shim

The 1 failure — does not package audio-capture test artifacts — is environmental, not a regression: it fails with ENOENT … packages/audio-capture/dist because the native addon was never built in my fresh worktree (the test fabricates a fake prebuild, which forces copyNativeAddon past its early-return into cpSync(audio-capture/dist)). The PR touches no audio-capture code. Proof: after staging a minimal audio-capture/dist, the same test passes. CI builds the addon first, so it's green there.

2. Real archive build + content assertions

node scripts/create-standalone-package.js --target darwin-arm64 --node-archive <real node> …Created qwen-code-darwin-arm64.tar.gz. Extracted contents:

Check Result
lib/cli-entry.js shipped ✅ present
lib/postinstall.js (npm-only) ✅ skipped (absent)
lib/patches/ (npm-only) ✅ skipped (absent)
node/bin/node ✅ present & executable

3. Runtime routing — generated bin/qwen executed (the decisive test)

Discriminator: serve routes through cli-entry.js (in-process import, compile cache, no --expose-gcexposeGc=false); everything else runs --expose-gc cli.js directly (exposeGc=true).

Invocation Route Observed
qwen serve cli-entry fast path exposeGc=false
qwen serve --port 9 cli-entry fast path exposeGc=false
qwen --version --expose-gc cli.js exposeGc=true
qwen (no args) --expose-gc cli.js exposeGc=true
qwen --version serve fallback (flag before subcmd) exposeGc=true ✅ (documented limitation)
cli-entry.js <non-serve> spawns --expose-gc child child ppid = cli-entry pid ✅

The serve path was re-run 9× (copied node, via shim, and system node) — 9/9 stable. (One initial run hit a Killed: 9; reproduced as a one-off macOS SIGKILL on a freshly-copied hardened-runtime node binary, unrelated to the PR — it never recurred.)

4. Mutation testing (do the tests actually guard the behavior?)

Reverting each PR behavior makes its test fail — confirming the tests are meaningful:

  • Remove shim serve-routing → both shim tests fail (expected … to contain 'if "%~1"=="serve" goto serve' / 'if [ "${1:-}" = "serve" ]').
  • Drop cli-entry.js from DIST_REQUIRED_PATHSrequires the standalone cli-entry wrapper fails (expected [Function] to throw).
  • Drop the npm-only skip → skips npm-only artifacts fails (Unexpected dist asset: …/dist/patches).

Observations (non-blocking)

  1. serve now runs without --expose-gc — intentional, and it makes standalone serve match the npm-installed serve path (whose bin is already cli-entry.js). The only global.gc() consumer is memoryPressureMonitor's trigger_gc, which is guarded (typeof global.gc === 'function') and degrades to a warning; channel subprocesses still get --expose-gc explicitly via the ACP bridge (spawnChannel.ts). So the standalone daemon's main process loses the explicit trigger_gc GC — worth a conscious ack, but consistent with npm behavior and gracefully handled.
  2. Maintenance coupling: DIST_NPM_PACKAGE_ONLY_ENTRIES (postinstall.js, patches) must stay in sync with whatever prepare-package.js emits as npm-only. If a future npm-only artifact is added without updating this set, packaging fails loudly (Unexpected dist asset) rather than mis-shipping — safe default.
  3. Windows shim verified by content assertion + logic review (the exit /b %ERRORLEVEL% after the non-serve line correctly prevents fall-through into :serve); not executed (no Windows host), consistent with the PR's own status table.

Verdict

LGTM — recommend merge. Routing is correct and stable at runtime, content/skip/required-asset behaviors all hold, and the new tests genuinely guard them. The lone non-passing test is an environmental build-artifact gap, not a code issue.

中文版(完整对应)

✅ 本地真实构建验证(维护者合并参考)

在隔离 worktree 中检出 PR head ecba1e90b,环境 macOS darwin-arm64 / Node v22.22.2,全程通过真实 tmux 会话执行。除单元测试外,我还真实构建了一个 standalone archive(真实 Node runtime + prepare:package 实际生成的 cli-entry.js wrapper),并实际执行生成的 shim,在运行时观测路由行为。

1. 打包测试套件(scripts/tests/install-script.test.js,PR head)

105 通过 · 9 跳过(平台门控)· 1 失败

4 个新增/修改的测试全部通过:

  • requires the standalone cli-entry wrapper in dist
  • packages a win-x64 standalone archive(断言新增的 goto serve 与两个 exit /b
  • skips npm-only artifacts staged in dist
  • packages a Unix standalone archive with a serve fast path shim

唯一失败项 does not package audio-capture test artifacts环境问题,非回归:报错 ENOENT … packages/audio-capture/dist,因为我的全新 worktree 从未构建该 native addon(该测试伪造一个 prebuild,从而让 copyNativeAddon 越过提前返回、进入 cpSync(audio-capture/dist))。PR 完全没有改动 audio-capture。佐证: 补上一个最小的 audio-capture/dist 后,同一测试即通过。CI 会先构建 addon,所以在 CI 是绿的。

2. 真实 archive 构建 + 内容断言

node scripts/create-standalone-package.js --target darwin-arm64 --node-archive <真实 node> …Created qwen-code-darwin-arm64.tar.gz。解包后:

检查 结果
lib/cli-entry.js 已打包 ✅ 存在
lib/postinstall.js(npm-only) ✅ 已跳过(不存在)
lib/patches/(npm-only) ✅ 已跳过(不存在)
node/bin/node ✅ 存在且可执行

3. 运行时路由 —— 实际执行生成的 bin/qwen(决定性验证)

判别依据:servecli-entry.js(进程内 import、compile cache、不带 --expose-gcexposeGc=false);其余命令直接走 --expose-gc cli.jsexposeGc=true)。

调用 路由 观测
qwen serve cli-entry 快路径 exposeGc=false
qwen serve --port 9 cli-entry 快路径 exposeGc=false
qwen --version --expose-gc cli.js exposeGc=true
qwen(无参数) --expose-gc cli.js exposeGc=true
qwen --version serve 回退(子命令前带 flag) exposeGc=true ✅(已记录的限制)
cli-entry.js <非 serve> spawn 带 --expose-gc 子进程 子进程 ppid = cli-entry pid ✅

serve 路径重跑 9 次(复制版 node、经 shim、系统 node)—— 9/9 稳定。(首次出现过一次 Killed: 9,复现确认为 macOS 对刚复制的 hardened-runtime node 二进制的一次性 SIGKILL,与 PR 无关,未再复现。)

4. 变异测试(测试是否真正守住行为?)

逐一回退 PR 的每项行为,对应测试都失败 —— 证明测试有效:

  • 移除 shim 的 serve 路由 → 两个 shim 测试失败(expected … to contain 'if "%~1"=="serve" goto serve' / 'if [ "${1:-}" = "serve" ]')。
  • DIST_REQUIRED_PATHS 移除 cli-entry.jsrequires the standalone cli-entry wrapper 失败(expected [Function] to throw)。
  • 移除 npm-only 跳过逻辑 → skips npm-only artifacts 失败(Unexpected dist asset: …/dist/patches)。

观察项(不阻塞合并)

  1. serve 现在不带 --expose-gc —— 这是有意为之,使 standalone serve 与 npm 安装版 serve 路径一致(npm 的 bin 本就是 cli-entry.js)。global.gc() 的唯一使用方是 memoryPressureMonitortrigger_gc,它有 typeof global.gc === 'function' 守卫并降级为告警;channel 子进程仍通过 ACP bridge(spawnChannel.ts)显式获得 --expose-gc。即 standalone daemon 主进程失去了显式 trigger_gc GC —— 值得维护者明确认可,但与 npm 行为一致且已优雅处理。
  2. 维护耦合: DIST_NPM_PACKAGE_ONLY_ENTRIESpostinstall.jspatches)必须与 prepare-package.js 实际产出的 npm-only 文件保持同步。若将来新增 npm-only 产物却未更新此集合,打包会以 Unexpected dist asset 显式失败、而非错误打包 —— 默认行为是安全的。
  3. Windows shim 通过内容断言 + 逻辑复核验证(非 serve 行之后的 exit /b %ERRORLEVEL% 正确防止落入 :serve);未实际执行(无 Windows 主机),与 PR 自身的状态表一致。

结论

LGTM —— 建议合并。 路由在运行时正确且稳定,内容打包 / 跳过 / 必需产物校验均成立,新增测试也确实守住了这些行为。唯一未通过的测试是环境构建产物缺失,并非代码问题。

Verified locally in an isolated worktree at ecba1e90b with a real standalone build + tmux runtime execution + mutation testing.

@doudouOUC
doudouOUC added this pull request to the merge queue Jun 29, 2026
Merged via the queue into QwenLM:main with commit aa8f9bb Jun 29, 2026
30 checks passed
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.

4 participants