feat(ci): fail the startup bundle check when the CLI entry is hoisted into a chunk - #8203
Conversation
… into a chunk
`packages/cli/src/cli.ts` is the esbuild entry point and bootstraps only under a
main-module guard:
if (process.argv[1] !== undefined &&
import.meta.url === pathToFileURL(process.argv[1]).href) {
void runCliEntryPoint();
}
The bundle is built with `splitting: true`. If any module the entry loads lazily
(e.g. `gemini.tsx`, reached through `await import('./gemini.js')`) adds a static
`import ... from './cli.js'`, esbuild moves the entry module's body into a shared
chunk and leaves `dist/cli.js` as a re-export stub. Inside a chunk
`import.meta.url` is the chunk's own URL, so the guard never matches and the
bundled CLI exits 0 without running anything.
Nothing catches that today: tsc, eslint and every src-based unit test stay green,
because the breakage only exists in the bundle. The single CI step that executes
`dist/cli.js` is the no-AK integration smoke test, which reports it as
`daemon exited with 0 before listening` from three unrelated serve suites — a
symptom that points nowhere near the import that caused it.
Assert instead that the entry output still compiles the entry module. When the
entry is hoisted, `dist/cli.js` keeps no inputs of its own, so the metafile the
existing closure checks already read is a precise signal, and the diagnostic can
name both the cause and the fix.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template: the section names differ from the template ( Problem: observed, not theoretical. #8088 is stuck red on precisely this failure mode — the no-AK integration smoke test reports Direction: aligned. This is build-integrity tooling that turns a silent, hard-to-diagnose bundle breakage into a deterministic, well-messaged failure at the existing closure-check step. It touches no auth/sandbox/model-selection/telemetry/release surface and no public contract. CHANGELOG: N/A — internal CI infrastructure, not a user-facing feature. Size: not applicable. Changes are confined to Approach: the scope feels right and genuinely minimal. It reuses the esbuild metafile the existing closure checks already read (a deterministic signal — no string-matching against generated code), plugs into the same Risk: no elevated risk signals — neither changed file matches the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板： 小节命名与模板不同（ 问题： 已观测到，非理论性问题。#8088 正好卡在这个失败模式上——no-AK 集成冒烟测试从三个互不相干的 方向： 对齐。这是构建完整性工具，把一个静默、难以诊断的 bundle 损坏，变成在现有闭包检查步骤里一个确定的、报错清晰的失败。它不触及 auth/sandbox/模型选择/telemetry/release 面，也不涉及公共契约。CHANGELOG：不适用——内部 CI 基础设施，非用户可见功能。 规模： 不适用。改动只在 方案： 范围合理、确实最小化。它复用了现有闭包检查已经在读的 esbuild metafile（确定性判据——不对生成代码做字符串匹配），接入同一个 风险： 无升级风险信号——两个改动文件都未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewMy independent take before reading the diff: the right fix is a deterministic metafile assertion in the existing closure-check script — read The implementation is clean and idiomatic for this file:
One non-blocking observation: the new check does a direct No correctness bugs, security concerns, or regressions found; no AGENTS.md convention violations. Test evidence (PR's own CI)
Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 No sandboxed lane ( 中文说明代码审查读 diff 之前我的独立判断：正确的修法是在现有闭包检查脚本里加一个确定性的 metafile 断言——读 实现干净、符合该文件的惯例：
一个非阻塞的观察：新检查直接用 未发现正确性 bug、安全隐患或回归；无 AGENTS.md 约定违规。 测试证据（PR 自身的 CI）
无需沙箱通道（ — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — minimal, well-tested gate that solves a real, observed CI failure mode; only nit is the minor normalization-consistency point above, and CI's Linux unit suite is still in flight so I'm deferring the approval until it lands green. Stepping back: this is the kind of PR that pays for itself. The failure it guards against is genuinely nasty — a bundle that builds clean, passes The implementation matches what I'd have written independently — I went looking for a simpler path and didn't find one. It reuses the metafile the sibling checks already read, adds ~26 lines of production logic, and the four new tests pin both the detection logic and the The one reservation, already noted in Stage 2, is cosmetic: the direct Approval is deferred until CI lands green on 中文说明置信度：4/5 —— 一个最小化、测试充分的门禁，解决了一个真实、已观测到的 CI 失败模式；唯一的瑕疵是上面提到的归一化一致性小点，且 CI 的 Linux 单测套件仍在跑，所以我推迟到它变绿再批准。 退一步看：这个 PR 物有所值。它防的失败确实棘手——bundle 构建干净、 实现与我独立会写的一致——我找过更简路径，没找到。它复用了兄弟检查已经在读的 metafile，新增约 26 行生产逻辑，4 条新测试同时钉住了检测逻辑和 唯一的保留（Stage 2 已提）是表面性的：直接的 批准推迟到 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Local verification: built the real bundle and reproduced the failure this gate exists forRan on Linux, Node v22.22.2, worktree at Verdict: does what it says, safe to merge. One Low finding and two optional follow-ups at the bottom; none of them block. 1. Baseline — no false positive on a healthy tree
Unmodified PR head: check passes, 2. The regression, reproduced for realReplayed #8088's edit verbatim onto the PR head —
3. The gate is the entire deltaBoth scripts read the same
4.
|
| # | mutation | result |
|---|---|---|
| M1 | ok: inputs.includes(ENTRY_INPUT) → ok: true |
killed (2 tests) |
| M2 | ENTRY_INPUT → packages/cli/src/gemini.tsx |
killed (2) |
| M3 | ENTRY_OUTPUT → dist/chunks/cli.js |
killed (4) |
| M4 | drop the missing-entry-output throw |
killed (1) |
| M5 | drop && entryResult.ok from the success-line condition |
survived |
| M6 | drop process.exitCode = 1 in the new failure branch |
killed (1) |
| M7 | exact input match → substring match (i.includes('cli')) |
survived |
| M8 | diagnostic no longer names the entry input | killed (1) |
| M9 | checkEntryBootstrapIntact() never called from main() |
killed (1) |
M6 is the one that matters — a gate that prints a diagnostic and still exits 0 is worth nothing, and the new CLI test kills it. The two survivors are coverage gaps, not defects; the shipped code is correct in both cases.
Finding (Low) — checkEntryBootstrapIntact() skips this file's own path normalization
checkEntryBootstrapIntact() looks the entry up with a raw key (metafile.outputs['dist/cli.js']) and compares with a raw inputs.includes('packages/cli/src/cli.ts'). The other three checks in this file all go through normalizeOutputs() / normalizeMetafilePath(), and the suite carries a dedicated backslash case (serve-fast-path-bundle-check.test.js:258). Given a healthy metafile whose paths use \, the three existing checks still pass and the new one fails — with a message that points at the wrong problem:
Blast radius is small: the CI step is deliberately Linux-only (per the comment above it in ci.yml). But CONTRIBUTING.md tells every contributor to run npm run preflight before submitting, and preflight includes this step. Reusing what is already in the file fixes it:
const outputs = normalizeOutputs(metafile); // already defined in this file
const output = outputs.get(ENTRY_OUTPUT);
// ...
const inputs = Object.keys(output.inputs ?? {}).map(normalizeMetafilePath);Optional follow-ups
- Derive the entry input from the metafile.
output.entryPointsurvives hoisting (see §4), soinputs.includes(output.entryPoint ?? ENTRY_INPUT)keeps the same detection power while self-healing ifcli.tsis ever moved or renamed — today that rename produces a "code splitting hoisted the entry" message for something that is not code splitting. - Two assertions to close the mutation gaps. In the CLI test,
expect(...).not.toContain('Startup bundle closure checks passed.')(kills M5); and a case wheredist/cli.jsholds only a differentcli-*.tsinput, assertingok === false(kills M7, pins exact matching).
Note
#8088's current head (f71d37d3, "move uncaught-exception helpers to a leaf module") already applies exactly the fix this diagnostic recommends, so landing this gate turns nothing red today — it keeps that class of regression from silently coming back.
中文版
本地验证：真实构建 bundle，并复现了这个门禁要拦的故障
环境：Linux、Node v22.22.2，worktree 在 c8569df8（base 01afcb0bb）。下面所有结论都来自真实的 npm run check:serve-fast-path-bundle（就是 CI 里那一步，完整 esbuild 打包，389 个 output），不是手写的 metafile。
结论：功能与描述一致，可以合入。 底部有 1 个 Low 问题和 2 条可选跟进，都不阻塞。
1. 基线 —— 健康代码树上不会误报
未改动的 PR head：检查通过，dist/cli.js 13,307 B，inputs = ["packages/cli/src/cli.ts"]，node dist/cli.js --version 输出 0.21.2。
2. 真实复现回归
把 #8088 的改动原样搬到 PR head 上——packages/cli/src/gemini.tsx（entry 只通过 await import('./gemini.js') 才会加载它）静态 import ./cli.js 里的 isExpectedPtyRaceError——然后重新构建：
dist/cli.js：13,307 B → 665 B 的纯 re-export 空壳；metafileinputs: []；cli.ts被搬到dist/chunks/chunk-RZCLGFSK.js。--version、--help、serve --port 45231全部 0 输出、exit 0。serve根本不监听——正是 PR 描述里那个daemon exited with 0 before listening: stdout= stderr=。
3. 门禁就是全部增量
两个脚本读的是同一份 dist/esbuild.json：
| 检查脚本 | 健康 bundle | 带 #8088 那条 import 的 bundle |
|---|---|---|
base（main @ 01afcb0bb） |
通过，exit 0 | 通过，exit 0 —— 看不见 |
本 PR（c8569df8） |
通过，exit 0 | exit 1 + 诊断信息 |
4. 选 inputs 是对的
值得记一笔，因为最顺手的替代方案是个坑：在被提到 chunk 之后的 metafile 里，那个空壳 output 仍然带着 entryPoint: "packages/cli/src/cli.ts"（见第 2 张图）。用 entryPoint 做判据会漏报，只有 inputs 会翻转。
5. 测试与 lint
scripts/tests/serve-fast-path-bundle-check.test.js：base 31 passed → PR 35 passed，与描述一致。- 整个
scriptsvitest project：817 passed / 5 failed。5 条全在tests/generate-release-notes.test.js（vi.mocked(...).mockImplementationOnce is not a function），base 上同样失败——与本 PR 无关。 - 两个改动文件的
prettier --check与eslint均干净。
6. 变异测试 —— 9 个变异体，杀掉 7 个
| # | 变异 | 结果 |
|---|---|---|
| M1 | ok: inputs.includes(ENTRY_INPUT) → ok: true |
killed（2 条用例） |
| M2 | ENTRY_INPUT → packages/cli/src/gemini.tsx |
killed（2） |
| M3 | ENTRY_OUTPUT → dist/chunks/cli.js |
killed（4） |
| M4 | 去掉 entry output 缺失时的 throw |
killed（1） |
| M5 | 从成功提示的条件里去掉 && entryResult.ok |
存活 |
| M6 | 去掉新增失败分支里的 process.exitCode = 1 |
killed（1） |
| M7 | 精确匹配改成子串匹配（i.includes('cli')） |
存活 |
| M8 | 诊断信息不再点名 entry input | killed（1） |
| M9 | main() 里根本不调用 checkEntryBootstrapIntact() |
killed（1） |
M6 是最关键的一个——一个只打印诊断却仍然 exit 0 的门禁毫无价值，而新增的 CLI 用例把它杀掉了。两个存活的是覆盖率缺口，不是缺陷：现有代码在这两点上都是正确的。
问题（Low）—— checkEntryBootstrapIntact() 绕过了本文件自己的路径归一化
checkEntryBootstrapIntact() 用原始 key 取 entry（metafile.outputs['dist/cli.js']），也用原始的 inputs.includes('packages/cli/src/cli.ts') 比较。而这个文件里另外三个检查都走 normalizeOutputs() / normalizeMetafilePath()，测试里还专门有一条反斜杠用例（serve-fast-path-bundle-check.test.js:258）。给一份路径用 \ 的健康 metafile：原有三个检查照常通过，新检查失败，而且报错指向了完全无关的方向：
影响面有限：这个 CI 步骤按设计只跑 Linux（ci.yml 里那一步上方的注释写明了）。但 CONTRIBUTING.md 要求所有贡献者提交前跑 npm run preflight，而 preflight 包含这一步。直接复用文件里已有的东西即可：
const outputs = normalizeOutputs(metafile); // 本文件里已有
const output = outputs.get(ENTRY_OUTPUT);
// ...
const inputs = Object.keys(output.inputs ?? {}).map(normalizeMetafilePath);可选跟进
- 从 metafile 推导 entry input。
output.entryPoint在被提到 chunk 后依然存在（见第 4 节），所以inputs.includes(output.entryPoint ?? ENTRY_INPUT)检测能力不变，同时在cli.ts被移动/改名时能自愈——按目前的写法，改名会得到一句"code splitting 把 entry 提走了"的误导性报错。 - 两条断言补上变异缺口。 CLI 用例里加
expect(...).not.toContain('Startup bundle closure checks passed.')（杀 M5）；再加一个dist/cli.js只含另一个cli-*.tsinput 的用例并断言ok === false（杀 M7，钉死精确匹配语义）。
备注
#8088 当前 head（f71d37d3，"move uncaught-exception helpers to a leaf module"）已经采用了本诊断推荐的修法，所以这个门禁合入后今天不会把任何 PR 变红——它防的是这一类回归再悄悄回来。
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Verified the premise against the repo before approving: esbuild.config.js does build packages/cli/src/cli.ts → dist/cli.js with splitting: true and writes the metafile the existing closure checks already read, and check:serve-fast-path-bundle rebuilds the bundle first in both ci.yml and release.yml, so the gate sees a fresh metafile. Using metafile inputs instead of string-matching generated code is the right call, and the failure modes are loud (entry rename / metafile shape change throw with the rebuild command) rather than silently passing. The four new tests each pin a distinct contract, and seeding makeMetafile() with a healthy entry output keeps the existing CLI-exit cases failing for their own reasons. One optional hardening thought inline — not blocking.
| } | ||
|
|
||
| const inputs = Object.keys(output.inputs ?? {}); | ||
| return { ok: inputs.includes(ENTRY_INPUT), inputs }; |
There was a problem hiding this comment.
One thing I noticed here: this accepts the entry input on key presence alone. Your empirical check showed the hoisted stub reports inputs: [] on the current esbuild, so this is correct today — but if a future esbuild version ever lists the entry in the stub output with bytesInOutput: 0, the check would pass while the bundle is still broken. Requiring output.inputs[ENTRY_INPUT].bytesInOutput > 0 would pin the invariant to "the entry's code is actually in the entry file" rather than to the current metafile shape. Fine as a follow-up or not at all.
|
Released in v0.21.3. |
3 participants
Problem
packages/cli/src/cli.tsis the esbuild entry point (esbuild.config.js) and bootstraps only under a main-module guard:The bundle is built with
splitting: true. If any module the entry loads lazily — e.g.gemini.tsx, reached throughawait import('./gemini.js')— adds a staticimport ... from './cli.js', esbuild moves the entry module's body into a shared chunk and leavesdist/cli.jsas a bare re-export stub. Inside a chunkimport.meta.urlis the chunk's own URL, so the guard can never match:runCliEntryPoint()is never called and the bundled CLI exits 0 without doing anything.Nothing catches this today.
tsc, eslint and every src-based unit test stay green, because the breakage exists only in the bundle. The one CI step that executesdist/cli.jsis the no-AK integration smoke test, and what it reports is:from three unrelated
qwen servesuites — a symptom that points nowhere near the import that caused it. #8088 has been stuck on exactly this: the base-update bot re-mergedmainassuming a stale base, AutoFix has burned six attempts, and the PR is still red.Change
checkEntryBootstrapIntact()asserts thatdist/cli.jsstill compilespackages/cli/src/cli.ts. When the entry is hoisted,dist/cli.jskeeps no inputs of its own, so the esbuild metafile that the existing closure checks already read is a precise, deterministic signal — no string matching against generated code.It runs alongside the serve fast-path, ACP and sdk-impl closure checks in the same
npm run check:serve-fast-path-bundlestep, and the diagnostic names both the cause and the fix:Verification
Built #8088's head with the real
esbuild.config.jsoptions and ran this check against both bundle shapes:dist/cli.js[]['packages/cli/src/cli.ts']Startup bundle closure checks passed.scripts/tests/serve-fast-path-bundle-check.test.js: 35 passed (31 existing + 4 new — healthy entry, hoisted entry, entry output absent from the metafile, and the CLI exit code). The sharedmakeMetafile()helper now includes a healthydist/cli.jsoutput so the existing cases keep exercising the closure checks rather than tripping the new one.Scope
This is the gate only. The
./cli.jsimport that #8088 introduces is that PR's to fix; the fix there is to move the shared helpers into a leaf module — details posted on #8088.中文说明
问题
packages/cli/src/cli.ts是 esbuild 的 entry（见esbuild.config.js），它只在自己是主模块时才会 bootstrap：bundle 开了
splitting: true。只要 entry 懒加载的任何模块（比如通过await import('./gemini.js')进来的gemini.tsx）加一条静态import ... from './cli.js'，esbuild 就会把 entry 的模块体搬进共享 chunk，dist/cli.js只剩一个 re-export 空壳。在 chunk 里import.meta.url指向 chunk 自身，守卫永远不成立：runCliEntryPoint()一次都不会被调用，打包后的 CLI 什么都不做就 exit 0。目前没有任何检查能发现它。
tsc、eslint、所有基于 src 的单测全是绿的，因为只有 bundle 坏了。CI 里唯一真正执行dist/cli.js的是 no-AK 集成冒烟测试，而它报出来的是：来自三个互不相干的
qwen serve套件——这个症状离真正的元凶（那一行 import）十万八千里。#8088 就卡在这上面：update-branch 机器人以为是 base 过期又合了一次 main，AutoFix 烧掉了 6 次尝试，PR 至今还是红的。改动
checkEntryBootstrapIntact()断言dist/cli.js里仍然编译进了packages/cli/src/cli.ts。entry 一旦被提到 chunk 里，dist/cli.js就不再拥有自己的 inputs，所以现有闭包检查已经在读的 esbuild metafile 就是一个精确、确定的判据——不需要对生成代码做字符串匹配。它和 serve fast-path、ACP、sdk-impl 三个闭包检查跑在同一个
npm run check:serve-fast-path-bundle步骤里，报错信息同时点出成因和修法（见上方英文部分的输出示例）。验证
用真实的
esbuild.config.js选项构建了 #8088 的 head，再用本检查跑两种 bundle 形状：dist/cli.js[]['packages/cli/src/cli.ts']Startup bundle closure checks passed.scripts/tests/serve-fast-path-bundle-check.test.js：35 passed（31 条原有 + 4 条新增——正常 entry、被提到 chunk 的 entry、metafile 中缺少 entry 输出、以及 CLI 退出码）。共用的makeMetafile()里补了一个健康的dist/cli.js输出，这样原有用例继续考验闭包检查，而不是被新检查拦下。范围
本 PR 只做门禁。#8088 引入的那条
./cli.jsimport 由该 PR 自己修——修法是把共享 helper 挪到叶子模块，细节已发在 #8088 上。