Skip to content

fix(cli): prevent silent VP-mode crash by adding uncaughtException handler and error visibility - #8088

Merged
wenshao merged 12 commits into
QwenLM:mainfrom
chiga0:fix-vp-silent-crash-v2
Jul 31, 2026
Merged

fix(cli): prevent silent VP-mode crash by adding uncaughtException handler and error visibility#8088
wenshao merged 12 commits into
QwenLM:mainfrom
chiga0:fix-vp-silent-crash-v2

Conversation

@chiga0

@chiga0 chiga0 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a process.on('uncaughtException') handler and strengthens error visibility in VP (alternate-screen) mode. Related to #7971 #7972 #7779 #7781.

This PR does not claim to fix the crashes reported in those issues — it ensures that the next time a crash occurs, the error is captured in the debug log and visible on the terminal, so the actual root cause can be identified and fixed.

Why it's needed

Users report the CLI exiting silently during model streaming with VP mode enabled (ui.useTerminalBuffer: true) — no error message, no debug log entry, empty stderr.

Investigation of a real crash session confirmed:

  • The model stream stopped mid-thought (135 chars accumulated), then the process exited
  • No [ERROR], [FATAL], or exception entry in the debug log
  • No uncaughtException handler exists anywhere in packages/cli/src
  • PR fix(cli): surface unhandled rejections and render errors instead of swallowing them #7406's ErrorBoundary and unhandledRejection handler are present but only cover React render errors and promise rejections — synchronous exceptions bypass both

The silence mechanism: in VP mode, Node's default uncaught-exception stack trace goes to stderr → alternate screen buffer → discarded when teardown writes ?1049l. The user sees their shell prompt with no trace of what happened.

Changes (6 files, +117/-15)

Core fix: uncaughtException handler (gemini.tsx)

  • setupUncaughtExceptionHandler(sessionId) with synchronous fs.appendFileSync to the debug log (async debugLogger.error() would be abandoned by process.exit)
  • Leaves the alternate screen (?1049l) before writing to stderr, so the error is visible on the main screen
  • Exits with code 1

VP main-screen error echo (ErrorBoundary.tsx + startInteractiveUI.tsx)

  • consumeLastRenderError() stores the last caught render error at module level
  • After instance.unmount() leaves the alternate screen in the cleanup chain, the error is echoed to stderr on the main screen

Kitty protocol signal handler fix (kittyProtocolDetector.ts)

SIGHUP handler (gemini.tsx)

MarkdownDisplay defense (ConversationMessages.tsx)

  • ThinkBody's MarkdownDisplay wrapped with ErrorBoundary — partial markdown during thought streaming degrades to plain text instead of crashing the VP tree

How to verify

After this PR, any crash will leave a trace:

grep -E "UNCAUGHT_EXCEPTION|FATAL_RENDER_ERROR|Unhandled Promise" ~/.qwen/debug/<session-id>.txt
Marker Error type Next step
[UNCAUGHT_EXCEPTION] Synchronous exception Stack trace points to the exact code
[FATAL_RENDER_ERROR] React render error Component stack points to the component
Unhandled Promise Rejection Async rejection Reason + stack point to the uncaught promise

No --debug flag needed — the debug log file is written by default.

…ndler and error visibility

VP (alternate-screen) mode swallows all error output: uncaught exceptions
write their stack trace to stderr which lands on the alternate screen
buffer, then gets discarded when teardown switches back to the primary
buffer. The user sees a silent exit with no error message and nothing in
the debug log.

Root cause: no `uncaughtException` handler existed anywhere in the CLI.
PR QwenLM#7406's ErrorBoundary and unhandledRejection handler only cover React
render errors and promise rejections — synchronous exceptions bypass both.

Changes:
- Add `setupUncaughtExceptionHandler()` with sync debug-log write,
  alternate-screen exit before stderr output, and clean process.exit(1)
- Add `consumeLastRenderError()` to ErrorBoundary for VP main-screen echo
  after unmount leaves the alternate screen
- Remove SIGTERM/SIGINT handlers from kittyProtocolDetector.ts that
  raced with the main signal handlers (QwenLM#7779)
- Add SIGHUP handler alongside SIGTERM/SIGINT (QwenLM#7781)
- Guard ThinkBody's MarkdownDisplay with per-item ErrorBoundary so
  partial markdown during thought streaming degrades to plain text

Related: QwenLM#7971 QwenLM#7972 QwenLM#7779 QwenLM#7781
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on f71d37d and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— f71d37d 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR — and thanks to @wenshao for the two rounds of real-pty A/B validation already in this thread.

Template: mostly there — What this PR does, Why it's needed, and a How to verify section are all present. The formal Reviewer Test Plan / Evidence (Before & After) / Tested on / Risk & Scope / Linked Issues headings and the Chinese <details> are missing from the body. Not blocking here — the maintainer has already supplied thorough before/after evidence in-thread — but worth tidying the body to match the template before merge.

Problem: real and reproduced — not theoretical hardening. The linked issues (#7971 #7972 #7779 #7781) report the CLI exiting silently in VP / alternate-screen mode, and the mechanism is concretely demonstrated: Node's default uncaught-exception trace goes to stderr → alternate buffer → discarded when teardown writes ?1049l. @wenshao reproduced this in a real pty with byte-offset evidence (baseline writes the trace at offset 18629 but only leaves the alt screen at 18947 — the trace lands inside the discarded buffer). The PR honestly does not claim to fix the underlying crash; it makes the next crash diagnosable. The problem genuinely exists.

Direction: aligned. Crash visibility / diagnostics is squarely within a CLI's core mission, and the scope maps cleanly to the linked issues (uncaughtException visibility, the Kitty signal race in #7779, SIGHUP in #7781). No auth / sandbox / model-selection / telemetry surface touched.

Size: not a core-module PR — every change sits under packages/cli/src (cli.ts, gemini.tsx, ui/, utils/); none touches the protected auth/providers/models/config/tools/services paths. ~304 production lines vs ~163 test lines, well under any threshold.

Approach: the scope feels right. Each change ties to a stated goal and a linked issue, and the leaf-module extraction (utils/uncaught-exception-handler.ts) is necessary, not speculative — it breaks the esbuild entry cycle that was killing the entire bundled CLI (the prior red Test (ubuntu-latest, Node 22.x)). No drive-by refactors. The Kitty signal-handler removal and the SIGHUP addition could arguably have been separate PRs, but they're small and tightly coupled to the VP-teardown theme; not worth splitting now.

Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths.

Moving on to code review. 🔍

中文说明

感谢贡献,也感谢 @wenshao 已在本帖中完成的两轮真实 pty A/B 验证。

模板: 基本完整 —— What this PR doesWhy it's needed 以及 How to verify 都在。但正文缺少正式的 Reviewer Test Plan / Evidence (Before & After) / Tested on / Risk & Scope / Linked Issues 标题和中文 <details>。此处不阻塞 —— 维护者已在帖中提供了充分的 before/after 证据 —— 但建议合并前把正文整理成模板格式。

问题: 真实且已复现,不是理论性加固。关联 issue(#7971 #7972 #7779 #7781)报告 CLI 在 VP / 备用屏模式下静默退出,机制已被具体证明:Node 默认的未捕获异常堆栈写到 stderr → 备用缓冲区 → teardown 写 ?1049l 时一并丢弃。@wenshao 在真实 pty 中用字节偏移证据复现了这一点(基线在偏移 18629 写堆栈,直到 18947 才离开备用屏 —— 堆栈落在被丢弃的缓冲区里)。本 PR 诚实地不声称修复底层崩溃,只是让下一次崩溃可被诊断。问题确实存在。

方向: 对齐。崩溃可见性 / 诊断完全属于 CLI 的核心职责,范围也与关联 issue 清晰对应(uncaughtException 可见性、#7779 的 Kitty 信号竞态、#7781 的 SIGHUP)。未触及 auth / sandbox / 模型选择 / telemetry 等敏感面。

规模: 非核心模块 PR —— 所有改动都在 packages/cli/src 下(cli.ts、gemini.tsx、ui/、utils/),没有触及受保护的 auth/providers/models/config/tools/services 路径。约 304 行生产代码 vs 约 163 行测试代码,远低于任何阈值。

方案: 范围合理。每处改动都对应一个明确目标和某个关联 issue,而叶子模块(utils/uncaught-exception-handler.ts)的抽取是必要的、非投机性的 —— 它打破了此前导致整个打包 CLI 死亡的 esbuild entry 环(即此前变红的 Test (ubuntu-latest, Node 22.x))。没有顺手重构。Kitty 信号处理器的移除和 SIGHUP 的新增本可拆成单独 PR,但体量小、与 VP teardown 主题强耦合,现在不值得拆。

风险: 无升级风险信号 —— 改动文件均未命中与 revert 相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Code review

My independent take before reading the diff: for "silent VP crash", add a process-level uncaughtException handler that leaves the alternate screen and writes the stack to stderr and the debug log synchronously before process.exit, and surface the ErrorBoundary's caught render error after unmount(). The PR does exactly this — plus the consolidation, the Kitty signal fix, SIGHUP, and the ThinkBody guard. It matches the proposal.

No critical blockers and no AGENTS.md violations. The one thing that was a blocker — the esbuild entry cycle that turned dist/cli.js into a dead re-export stub and broke the entire bundled CLI — is now fixed: gemini.tsx and cli.ts both import handleUncaughtException / isExpectedPtyRaceError from the new leaf module utils/uncaught-exception-handler.ts (which imports nothing but stdioHelpers, so no cycle), and cli.ts keeps a re-export for cli.test.ts with a comment explaining why the helpers must live outside the entry. This is exactly the fix @wenshao prescribed, and he verified it survives bundling (single definition in one chunk, 1 live listener against the bundle).

A few things I checked specifically and was satisfied with:

  • Exactly one listener. setupUncaughtExceptionHandler removes the basic entry-point handler and any prior session handler before installing its own; two would conflict (the first calls process.exit before the second runs). Confirmed live by the maintainer (listener count = 1 at fault time).
  • Synchronous log write. fs.appendFileSync + fs.mkdirSync(recursive) before process.exit — async debugLogger.error() would be abandoned. The mkdirSync also resolves the round-1 finding where the write silently no-opped on a fresh machine and the "(logged to debug file)" message was false.
  • recordForExitEcho scoping. Only the fatal top-level boundary stores the error; the ThinkBody boundary handles its error inline and does not feed the exit echo, so a clean /quit produces no spurious "Rendering error" line (verified by the maintainer, and pinned by the new ErrorBoundary.test.tsx cases).
  • Kitty teardown. Removing the racy SIGTERM/SIGINT handlers and keeping only the process.on('exit') fallback is correct — signal teardown now flows through installInteractiveSignalHandlers → runExitCleanup → disableKittyProtocol after Ink leaves the alt screen. Both paths are gated on config.isInteractive(), with the signal handlers installed before detection, so there's no enabled-but-unhandled window.
  • Conventions. New file is kebab-case, ESM, no any (everything is unknown + narrowing), and the comments explain the non-obvious why (esbuild hoisting, sync write, alt-screen ordering) rather than narrating the code.

Non-blocking follow-ups (both already raised by @wenshao, both verified by him, neither affecting the VP path this PR targets):

  1. The handler gates the ?1049l alt-screen exit on process.stdout.isTTY, which is true in modes that never enter the alt screen (-p, --acp, serve, screen-reader, useTerminalBuffer: false). When an outer program owns the alt screen and shells out to qwen -p, an uncaught exception now tears that outer screen down. Suggested fix: gate on a "we entered the alt screen" flag set by startInteractiveUI rather than on isTTY. Only fires on an already-fatal crash, so genuinely non-blocking.
  2. PR-body wording: the ThinkBody boundary guards the expanded thought view; while a thought streams collapsed (the default) ThinkBody returns a plain <Text> tail and never reaches MarkdownDisplay. The code is right — just the description is slightly imprecise.
Files changed (9)
File What changed
packages/cli/src/utils/uncaught-exception-handler.ts New leaf module holding the PTY-race guard and the basic entry-point handler; breaks the esbuild entry cycle
packages/cli/src/cli.ts Drops the inline handler + helpers, wires process.on('uncaughtException', handleUncaughtException), re-exports the helpers for cli.test.ts
packages/cli/src/gemini.tsx Adds the session-aware handler (sync debug-log write, leave-alt-screen-before-stderr), SIGHUP with exit 129
packages/cli/src/ui/components/shared/ErrorBoundary.tsx Module-level last-render-error store + recordForExitEcho opt-in
packages/cli/src/ui/startInteractiveUI.tsx Echoes a stored render error to stderr after leaving the alt screen
packages/cli/src/ui/components/messages/ConversationMessages.tsx Wraps the expanded-thought MarkdownDisplay in a non-fatal ErrorBoundary
packages/cli/src/ui/utils/kittyProtocolDetector.ts Removes the racy SIGTERM/SIGINT handlers, keeps the exit fallback
packages/cli/src/gemini.test.tsx Tests SIGHUP exit 129 and the VP exit-time render-error echo
packages/cli/src/ui/components/shared/ErrorBoundary.test.tsx Tests the consume-once store and the recordForExitEcho gating

Test evidence (the PR's own CI)

This is an unattended run, so I'm quoting the PR's CI rather than executing anything. The previously-red Test (ubuntu-latest, Node 22.x) was caused by the esbuild cycle above — @wenshao diagnosed it precisely (the entry compiled to a 629 B dead stub) and the leaf-module fix is now in. On the current head that test is re-running; windows/macos/integration are skipped because this is a fork PR. No check is red right now.

Final CI results for f71d37d (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The behavioural claim itself does not rest on the author's word: @wenshao (a maintainer, write access) ran two rounds of decisive real-pty A/B validation — fault injected as a genuine uncaughtException with product code untouched, byte stream replayed through a real terminal emulator — confirming the trace now lands on the main screen, the debug entry is written synchronously, the PTY guard and clean /quit show no regression, SIGHUP exits 129, and the fix survives the esbuild bundle. That is stronger than what a sandboxed lane would produce for the core claim, so I'm not requesting one to settle it. The surfaces he explicitly left open — a real Kitty/Ghostty terminal and Windows/macOS — are judged non-blocking; @qwen-code /verify remains available as a sponsored run (a maintainer's comment approves the head it runs against, with a pre-execution risk screen and workspace wipe) if anyone wants those closed, reading the resulting report with the same skepticism as the fork's own CI.

中文说明

代码审查

我在看 diff 之前的独立方案:针对「VP 静默崩溃」,加一个进程级 uncaughtException 处理器,在 process.exit 之前离开备用屏并把堆栈同步写到 stderr 和调试日志,再在 unmount() 之后把 ErrorBoundary 捕获的渲染错误回显出来。本 PR 正是这么做的 —— 还附带了处理器合并、Kitty 信号修复、SIGHUP 和 ThinkBody 守卫。与我的方案一致。

无关键阻塞项,也无 AGENTS.md 违规。 唯一曾经构成阻塞的点 —— esbuild entry 环把 dist/cli.js 变成死的 re-export 空壳、从而废掉整个打包 CLI —— 现已修复:gemini.tsxcli.ts 都从新的叶子模块 utils/uncaught-exception-handler.ts 引入 handleUncaughtException / isExpectedPtyRaceError(该模块只引入 stdioHelpers,不成环),cli.tscli.test.ts 保留 re-export 并注释说明了 helper 为何必须放在 entry 之外。这正是 @wenshao 给出的修法,他也验证了该修复在打包后依然成立(整个 bundle 中单一定义、崩溃时实测 1 个监听器)。

我专门核对并放心的几点:监听器恰好一个(先移除基础处理器与上一次 session 的处理器再安装,避免两个监听器互相抢先 process.exit,维护者实测为 1);同步写日志(appendFileSync + mkdirSync(recursive),异步会被 process.exit 抛弃,mkdirSync 同时解决了首轮发现的「全新机器上静默失效、提示语为假」的问题);recordForExitEcho 收窄(只有致命的顶层边界记录错误,ThinkBody 边界自行内联处理、不喂给退出回显,正常 /quit 不会打出多余行,且有新增测试钉住);Kitty teardown(移除有竞态的 SIGTERM/SIGINT、只保留 exit 兜底是正确的,信号清理现在在 Ink 离开备用屏之后走 runExitCleanup → disableKittyProtocol,且两条路径都由 isInteractive() 把关、信号处理器先装,不存在「已启用但无人处理」的窗口);规范(新文件 kebab-case、ESM、无 any、注释只解释非显而易见的 why)。

非阻塞后续(均由 @wenshao 提出并由其验证,都不影响本 PR 主攻的 VP 路径):其一,处理器退备用屏只由 process.stdout.isTTY 把关,而该条件在 -p--acpserve、读屏、useTerminalBuffer: false 等从不进入备用屏的模式下也为真;当外层程序占用备用屏并 shell out 跑 qwen -p 时,未捕获异常会拆掉外层屏幕。建议改为按「是否我们进入的」标志判断。仅在已经致命的崩溃时触发,确属非阻塞。其二,PR 描述措辞:ThinkBody 边界保护的是已展开的思考视图,折叠流式输出时不会走到 MarkdownDisplay;代码是对的,只是描述略不精确。

测试证据(本 PR 自身的 CI)

这是无人值守运行,因此我引用 PR 的 CI 而非自行执行任何代码。此前变红的 Test (ubuntu-latest, Node 22.x) 正是上面的 esbuild 环所致 —— @wenshao 精确诊断(entry 被编译成 629 B 的死空壳),叶子模块修复现已落地。在当前 head 上该测试正在重跑;windows/macos/integration 因为是 fork PR 而被跳过。当前没有任何检查变红。

行为层面的结论并不依赖作者自述:@wenshao(维护者,有写权限)做了两轮决定性的真实 pty A/B 验证 —— 以真正的 uncaughtException 注入故障、未改产品代码,捕获的字节流经真实终端模拟器回放 —— 确认堆栈现在落在主屏、调试记录被同步写入、PTY 守卫与正常 /quit 无回归、SIGHUP 退出 129、且修复在 esbuild bundle 中依然成立。这比沙箱通道对核心结论能给出的证据更强,因此我不要求再跑一个来定论。他明确未覆盖的面 —— 真实 Kitty/Ghostty 终端与 Windows/macOS —— 被判定为非阻塞;如需关闭,@qwen-code /verify 仍可作为赞助运行(由维护者评论批准其运行的 head,并带执行前风险筛查与工作区清空)使用,并应以看待 fork 自身 CI 同样的审慎态度阅读其报告。

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — solid, well-scoped fix that does exactly what it claims; the only reservations are two non-blocking follow-ups already flagged by the maintainer, and the current head's unit CI is still finishing.

Stepping back: this PR is honest about what it is — it does not fix the reported crashes, it makes the next one diagnosable — and the problem is real and reproduced, not a hypothesis. The implementation is straightforward and matches what I would have written: one consolidated uncaughtException listener, a synchronous debug-log write, leave-the-alt-screen-before-stderr, and a render-error echo after unmount. The earlier blocker (the esbuild entry cycle that silently killed the whole bundled CLI) is genuinely resolved by the leaf-module extraction, not papered over, and the maintainer confirmed it holds in the shipped bundle.

The reason this is 4/5 and not 5/5 is the ?1049l-on-isTTY gate: it's a real, if narrow, regression for an outer program that owns the alt screen and shells out to qwen -p. It only fires on an already-fatal crash and the VP path is unaffected, so it is not a blocker — but it's worth a follow-up issue so it isn't lost. The PR-body template headings and the slightly imprecise ThinkBody wording are hygiene, not substance.

I'd maintain this in six months without cursing the author — the comments explain the non-obvious constraints, the tests pin the behaviours that matter (single listener, consume-once echo, SIGHUP 129), and there's no speculative scaffolding.

Verdict: approve. I'm not posting the approval this instant because the current head's Test (ubuntu-latest, Node 22.x) — the very check this PR previously broke — is still in progress, and approving now would attest to a result that doesn't exist yet. Approval is deferred until CI lands green on the reviewed commit; the marker below carries that instruction.

中文说明

置信度:4/5 —— 一个扎实、范围恰当的修复,做到了它所声称的一切;唯一的保留是维护者已经指出的两个非阻塞后续项,以及当前 head 的单元 CI 仍在收尾。

退一步看:这个 PR 对自身定位很诚实 —— 它不修复上报的崩溃,而是让下一次崩溃可被诊断 —— 而问题是真实且已复现的,不是假设。实现很直接,与我会写的一致:一个合并后的 uncaughtException 监听器、同步写调试日志、先离开备用屏再写 stderr、以及 unmount 后的渲染错误回显。此前的阻塞项(悄无声息废掉整个打包 CLI 的 esbuild entry 环)是通过叶子模块抽取真正解决的,而非糊弄过去,维护者也确认它在发布 bundle 中成立。

之所以是 4/5 而非 5/5,是因为 ?1049l 仅由 isTTY 把关这一点:对一个占用备用屏并 shell out 跑 qwen -p 的外层程序来说,这是一个真实但很窄的回归。它只在已经致命的崩溃时触发、且 VP 路径不受影响,因此不构成阻塞 —— 但值得开个后续 issue 以免被遗忘。PR 正文的模板标题和 ThinkBody 措辞略不精确属于规范层面,不涉及实质。

六个月后维护它我不会骂作者 —— 注释解释了非显而易见的约束,测试钉住了关键行为(单一监听器、消费即清的回显、SIGHUP 129),也没有投机性的脚手架。

结论:批准。 我此刻不立即提交批准,因为当前 head 的 Test (ubuntu-latest, Node 22.x) —— 也正是本 PR 此前弄红的那一项 —— 仍在进行中,现在批准等于为一个尚不存在的结果背书。批准推迟到 CI 在被审 commit 上变绿;下方标记承载该指令。

Qwen Code · qwen3.8-max-preview

Reviewed at f71d37d3a15557ced877f822528226f562a098d2 · 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 — CI landed green after the 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.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/cli/src/gemini.tsx Outdated
Comment thread packages/cli/src/ui/components/messages/ConversationMessages.tsx
Comment thread packages/cli/src/ui/components/shared/ErrorBoundary.tsx
Comment thread packages/cli/src/gemini.tsx Outdated
Comment thread packages/cli/src/gemini.tsx Outdated
Comment thread packages/cli/src/gemini.tsx
@chiga0

chiga0 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Jul 30, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

qwen-code-dev-bot commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 7 finishedview run. See this round's report below.

中文说明

AutoFix 第 7 轮已完成 —— 查看运行。本轮报告见下方。

QwenLM#8088)

The VP-crash handler added a second uncaughtException listener that conflicted with the pre-existing one in runCliEntryPoint: the first listener's process.exit(1) ran before the second, leaving the visibility feature inert for real errors, and the new listener lacked the PTY-race guard, crashing the session on benign teardown errors. Replace the startup handler with one session-aware listener (PTY guard, isTTY-guarded alternate-screen leave, writeStderrLineSafe), sanitize the inline render-error fallback, gate the exit-time render-error echo on onError, and cover SIGHUP exit code.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Review feedback addressed

All six findings were resolved in code in a single commit. No conflicts (--conflict false).

Critical

  • [rc:3680065843] gemini.tsx — conflicting uncaughtException handlers. Confirmed against the code: runCliEntryPoint in cli.ts registers an uncaughtException listener first. For a real error it writes to stderr and calls process.exit(1), which terminates the process before Node reaches the second (gemini) listener — so the visibility feature was inert. For a benign PTY teardown error the cli.ts listener returns early without exiting, so the gemini listener ran second and, lacking the isExpectedPtyRaceError() guard, crashed the session with "Fatal: uncaught exception".
    • Fix (consolidate to a single listener, per the suggested approach): extracted the cli.ts handler into an exported handleUncaughtException (pure refactor, no behavior change) and made setupUncaughtExceptionHandler in gemini.tsx process.removeListener('uncaughtException', handleUncaughtException) before installing one session-aware listener. The consolidated listener (a) returns early on isExpectedPtyRaceError(error), (b) leaves the alternate screen before writing stderr, (c) writes the debug log synchronously, and (d) exits 1. It is also idempotent — it removes any handler a previous call installed — so exactly one listener is ever active (this also cleared a MaxListenersExceededWarning that surfaced when main() runs repeatedly under test).

Suggestions (all implemented)

  • [rc:3680065849] ConversationMessages.tsx — unsanitized fallback. The inline ThinkBody fallback now renders sanitizeTerminalText(err.message), matching the two sibling ErrorBoundary fallbacks and closing the terminal-injection path. Added sanitizeTerminalText to the existing textUtils.js import.
  • [rc:3680065866] ErrorBoundary.tsx — unconditional lastRenderError. lastRenderError is now set only when this.props.onError is present, so a non-fatal inline boundary (no onError, app continues) no longer feeds the exit-time "Rendering error (logged to debug file)" echo — which was also a false claim, since that path logs nothing. Updated the existing consumeLastRenderError test to pass an onError and added a regression test asserting a no-onError boundary does not store the error.
  • [rc:3680065870] gemini.tsxwriteStderrLine can throw EPIPE. The handler now uses writeStderrLineSafe (try/catch-wrapped) so an EPIPE on a closed stderr can no longer abort the handler before process.exit(1).
  • [rc:3680065871] gemini.tsx — unconditional alternate-screen escapes. The \x1b[?1049l / \x1b[?25h writes are now guarded by process.stdout.isTTY, so a redirected stdout (e.g. qwen --prompt … > out.txt) is not corrupted by escape codes.
  • [rc:3680065873] gemini.tsx — SIGHUP untested. Added still exits on SIGHUP with code 129 to gemini.test.tsx, modeled on the existing SIGTERM→143 test: the process.on spy now also captures SIGHUP, the handler is invoked, and the test asserts runExitCleanup runs once and process.exit is called with 129.

Verification

Commands actually run (all from a clean npm ci + build performed by the runner):

  • npm run typecheck — passed
  • npm run build — passed
  • npm run bundle — passed
  • npx eslint on all six touched files — passed (no findings)
  • vitest run src/ui/components/shared/ErrorBoundary.test.tsx src/cli.test.ts (packages/cli) — 61 passed
  • vitest run src/gemini.test.tsx (packages/cli) — 56 passed (includes the new SIGHUP test; MaxListenersExceededWarning no longer appears)
  • vitest run src/ui/components/messages/ConversationMessages.test.tsx (packages/cli) — 20 passed
中文说明

已处理的评审反馈

全部六条发现均已在一次提交中通过代码修复。无冲突(--conflict false)。

Critical(严重)

  • [rc:3680065843] gemini.tsxuncaughtException 处理器冲突。 已对照代码确认:cli.ts 中的 runCliEntryPoint 会先注册一个 uncaughtException 监听器。对于真实错误,它会向 stderr 写入并调用 process.exit(1),从而在 Node 到达第二个(gemini)监听器之前就终止进程——因此错误可见化功能实际上是失效的。对于良性的 PTY 拆卸竞态错误,cli.ts 监听器会提前返回而不退出,于是 gemini 监听器随后运行,而它缺少 isExpectedPtyRaceError() 守卫,会以 "Fatal: uncaught exception" 使会话崩溃。
    • 修复(按建议方案合并为单一监听器):cli.ts 中的处理器提取为导出的 handleUncaughtException(纯重构,行为不变),并让 gemini.tsx 中的 setupUncaughtExceptionHandler 在安装一个会话感知监听器之前先执行 process.removeListener('uncaughtException', handleUncaughtException)。合并后的监听器:(a) 在 isExpectedPtyRaceError(error) 时提前返回;(b) 在写入 stderr 之前 离开备用屏幕(alternate screen);(c) 同步写入调试日志;(d) 以 1 退出。它还是幂等的——会移除上一次调用所安装的处理器——因此任何时候都恰好只有一个监听器处于激活状态(这也消除了 main() 在测试中反复运行时出现的 MaxListenersExceededWarning)。

Suggestions(建议,全部已实现)

  • [rc:3680065849] ConversationMessages.tsx — 未净化的回退文本。 内联的 ThinkBody 回退现在渲染 sanitizeTerminalText(err.message),与另外两个同级 ErrorBoundary 回退保持一致,堵上了终端注入路径。已将 sanitizeTerminalText 加入既有的 textUtils.js 导入。
  • [rc:3680065866] ErrorBoundary.tsx — 无条件的 lastRenderError 现在仅当存在 this.props.onError 时才设置 lastRenderError,因此非致命的内联边界(无 onError,应用继续运行)不再触发退出时的 "Rendering error (logged to debug file)" 提示——该提示在此路径下也是不实声明,因为这条路径根本不会记录任何日志。已更新既有的 consumeLastRenderError 测试以传入 onError,并新增回归测试断言无 onError 的边界不会存储错误。
  • [rc:3680065870] gemini.tsxwriteStderrLine 可能抛出 EPIPE。 处理器现在使用 writeStderrLineSafe(带 try/catch 包裹),因此已关闭的 stderr 上的 EPIPE 不会再在 process.exit(1) 之前中断处理器。
  • [rc:3680065871] gemini.tsx — 无条件的备用屏幕转义序列。 \x1b[?1049l / \x1b[?25h 的写入现在由 process.stdout.isTTY 守卫,因此被重定向的 stdout(例如 qwen --prompt … > out.txt)不会被转义码破坏。
  • [rc:3680065873] gemini.tsx — SIGHUP 缺少测试。gemini.test.tsx 中新增 still exits on SIGHUP with code 129,以既有的 SIGTERM→143 测试为模板:process.on 间谍现在也会捕获 SIGHUP,调用该处理器,并断言 runExitCleanup 运行一次且 process.exit129 被调用。

验证

实际运行的命令(均基于 runner 执行的干净 npm ci + build):

  • npm run typecheck — 通过
  • npm run build — 通过
  • npm run bundle — 通过
  • 对全部六个被修改文件运行 npx eslint — 通过(无发现)
  • vitest run src/ui/components/shared/ErrorBoundary.test.tsx src/cli.test.ts(packages/cli)— 61 通过
  • vitest run src/gemini.test.tsx(packages/cli)— 56 通过(含新增的 SIGHUP 测试;MaxListenersExceededWarning 不再出现)
  • vitest run src/ui/components/messages/ConversationMessages.test.tsx(packages/cli)— 20 通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

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

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its E2E suite did not run locally.

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/cli/src/gemini.tsx Outdated
Comment thread packages/cli/src/ui/components/shared/ErrorBoundary.tsx Outdated
Comment thread packages/cli/src/ui/startInteractiveUI.tsx

@doudouOUC doudouOUC 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. 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/cli/src/ui/startInteractiveUI.tsx
Comment thread packages/cli/src/gemini.tsx
Comment thread packages/cli/src/ui/components/shared/ErrorBoundary.tsx Outdated
Comment thread packages/cli/src/gemini.tsx Outdated
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

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

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its E2E suite did not run locally.

[Critical] packages/cli/src/gemini.tsx:185 — the previously-filed blocker (inline comment 3681539981) STILL STANDS at the current HEAD. The session-aware handler is typed (error: Error) => void (line 172) and dereferences error.message / error.stack directly (line 190) with no instanceof Error guard, regressing the basic handleUncaughtException it replaces (cli.ts uses error: unknown + instanceof + String(error) fallback). Node delivers the thrown value as unknown at runtime. Failure scenario: a non-Error reaches the handler — for throw null / throw undefined, isExpectedPtyRaceError returns false (cli.ts:402 guards on instanceof Error), so control reaches line 190 where null.message throws a TypeError inside this last-resort handler, skipping the debug-log write, the alternate-screen leave, the stderr Fatal message and process.exit(1) — the original error is lost and the crash visibility this PR adds is defeated in exactly the VP mode it targets; a thrown string/number logs undefined instead of the value the old handler surfaced via String(error). Fix: widen the stored type to ((error: unknown) => void) and normalize at the top of the handler (const error = rawError instanceof Error ? rawError : new Error(String(rawError));). The other prior blocker (handler conflict, comment 3680065843) is FIXED by this diff — process.removeListener('uncaughtException', handleUncaughtException) (line 180) plus the isExpectedPtyRaceError guard (line 186) resolve both sub-issues.

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/cli/src/gemini.tsx
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下:

Review feedback addressed

All 8 inline findings (1 Critical, 7 Suggestion) are resolved in this commit.

Critical

  • rc:3681539981 (gemini.tsx:185) — The session-aware uncaughtException handler typed its parameter as (error: Error) => void and dereferenced error.message / error.stack directly, with no instanceof Error guard. Node delivers the thrown value as unknown at runtime; a non-Error (e.g. throw null) would crash inside this last-resort handler, defeating the crash visibility this PR adds.
    • Fix: Widened the stored type to ((error: unknown) => void), renamed the parameter to rawError, and added normalization at the top of the handler: const error = rawError instanceof Error ? rawError : new Error(String(rawError));. This mirrors the existing handleUncaughtException in cli.ts.

Suggestions

  • rc:3681539987 + rc:3681542930 (ErrorBoundary.tsx:76) — lastRenderError was set whenever onError was present, but the TranscriptView's non-fatal boundary also passes onError (for debug logging). A recovered render error in the transcript view would cause a spurious "Rendering error" message on a later normal /quit.

    • Fix: Added a dedicated recordForExitEcho?: boolean prop. The module-level store is now gated on this prop instead of onError. Only the fatal top-level boundary in startInteractiveUI.tsx sets it. Updated tests to match the new semantics.
  • rc:3681539990 (startInteractiveUI.tsx:317) — The cleanup-path echo of a stored render error had no test.

    • Fix: Added two tests in the startInteractiveUI describe block: one that seeds a render error and asserts the stderr output, and one that verifies no echo when no error was stored.
  • rc:3681542917 (startInteractiveUI.tsx:319) — renderError.message was written to stderr without sanitizeTerminalText, while other error-display paths in this PR sanitize.

    • Fix: Imported sanitizeTerminalText and applied it to the stderr echo.
  • rc:3681542927 (gemini.tsx:190) — Same issue as the Critical finding (rc:3681539981), raised independently.

    • Fix: Covered by the same normalization change.
  • rc:3681542935 (gemini.tsx:179) — The comment incorrectly claimed handleUncaughtException "lacks both the PTY-race guard", but it contains an explicit isExpectedPtyRaceError check.

    • Fix: Corrected the comment to say "lacks the debug-log write and the alternate-screen handling below".
  • rc:3682213519 (gemini.tsx:213) — The stderr message unconditionally claimed "(logged to debug file)" even though the fs.appendFileSync write is best-effort and its failure is swallowed.

    • Fix: Added a logged flag that tracks whether the write succeeded; the stderr message now conditionally includes "(logged to debug file)".

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • vitest run src/ui/components/shared/ErrorBoundary.test.tsx (packages/cli) — 8 passed
  • vitest run src/gemini.test.tsx (packages/cli) — 58 passed
  • vitest run src/cli.test.ts (packages/cli) — 53 passed
中文说明

已处理的审查反馈

本次提交解决了全部 8 条内联发现(1 条 Critical,7 条 Suggestion)。

Critical

  • rc:3681539981gemini.tsx:185)— 会话感知的 uncaughtException 处理程序将其参数类型定义为 (error: Error) => void,并直接解引用 error.message / error.stack,没有 instanceof Error 守卫。Node 在运行时将抛出的值作为 unknown 传递;非 Error 值(如 throw null)会在这个最后防线处理程序内部崩溃,从而破坏本 PR 添加的崩溃可见性。
    • 修复:将存储类型扩展为 ((error: unknown) => void),将参数重命名为 rawError,并在处理程序顶部添加规范化:const error = rawError instanceof Error ? rawError : new Error(String(rawError));。这与 cli.ts 中现有的 handleUncaughtException 保持一致。

Suggestion

  • rc:3681539987 + rc:3681542930ErrorBoundary.tsx:76)— 只要存在 onErrorlastRenderError 就会被设置,但 TranscriptView 的非致命边界也传递了 onError(用于调试日志)。TranscriptView 中恢复的渲染错误会导致后续正常 /quit 时输出多余的 "Rendering error" 消息。

    • 修复:添加了专用的 recordForExitEcho?: boolean 属性。模块级存储现在由此属性而非 onError 控制。只有 startInteractiveUI.tsx 中的致命顶层边界设置它。更新了测试以匹配新语义。
  • rc:3681539990startInteractiveUI.tsx:317)— 存储的渲染错误的清理路径回显没有测试。

    • 修复:在 startInteractiveUI describe 块中添加了两个测试:一个注入渲染错误并断言 stderr 输出,另一个验证没有错误时不输出回显。
  • rc:3681542917startInteractiveUI.tsx:319)— renderError.message 未经 sanitizeTerminalText 处理就写入 stderr,而本 PR 中的其他错误显示路径都进行了清理。

    • 修复:导入 sanitizeTerminalText 并应用于 stderr 回显。
  • rc:3681542927gemini.tsx:190)— 与 Critical 发现(rc:3681539981)相同的问题,由另一位审查者独立提出。

    • 修复:由同一规范化更改覆盖。
  • rc:3681542935gemini.tsx:179)— 注释错误地声称 handleUncaughtException "缺少 PTY 竞态守卫",但它包含明确的 isExpectedPtyRaceError 检查。

    • 修复:将注释更正为 "lacks the debug-log write and the alternate-screen handling below"。
  • rc:3682213519gemini.tsx:213)— stderr 消息无条件声称 "(logged to debug file)",即使 fs.appendFileSync 写入是尽力而为的,其失败会被吞掉。

    • 修复:添加了 logged 标志来跟踪写入是否成功;stderr 消息现在有条件地包含 "(logged to debug file)"。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • vitest run src/ui/components/shared/ErrorBoundary.test.tsx(packages/cli)— 8 个通过
  • vitest run src/gemini.test.tsx(packages/cli)— 58 个通过
  • vitest run src/cli.test.ts(packages/cli)— 53 个通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@wenshao

wenshao commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Real-environment validation ✅

I built this PR merged into current main and exercised it against the real CLI binary in a real pty with VP / alternate-screen mode active, rather than relying on unit tests alone. Summary: the fix does what it claims, and I found no regressions. One non-blocking follow-up is noted at the bottom.

Validation matrix: 6 scenarios, baseline vs patched

Setup

Baseline main @ f4cd6e1d8
Candidate PR head 1d43cd35d merged into main8c3719b41 (merges cleanly, no conflicts)
Environment macOS 26.6 arm64, Node v24.18.1, real pty 120×34, TERM=xterm-256color
VP mode Confirmed active — ESC[?1049h observed in the captured byte stream

Faults were injected as genuine asynchronous uncaughtExceptions from a timer callback via node --import, with no product code modified. The captured pty byte stream was replayed through a real terminal emulator (@xterm/headless) so the screenshots show exactly what a user's terminal would display — including alternate-screen semantics.

The core bug, reproduced and fixed

On main, the stack trace is written — the problem is purely ordering. In the captured stream the trace lands at byte 42530, and the alternate screen is not torn down until byte 43070. The trace is written to the alt buffer and discarded with it:

Before/after: crash output discarded with the alt buffer vs surviving on the main buffer

Results

# Scenario main @ f4cd6e1d8 main + this PR
1 Uncaught exception in VP mode exit 1, nothing visible — trace discarded with the alt buffer exit 1, full stack trace on the main buffer ✅ fixed
2 Same, QWEN_DEBUG_LOG_FILE=1 0 UNCAUGHT_EXCEPTION entries in the debug log 1 entry, written synchronously before process.exit() ✅ fixed
3 Benign PTY race (read EIO) suppressed, session unaffected suppressed — PTY guard preserved in the new handler ✅ no regression
4 Clean /quit exit 0, no spurious output exit 0, no spurious Rendering error line ✅ no regression
5 SIGHUP (terminal closed) killed by signal 1 — no handler clean exit 129 via the cleanup chain ✅ fixed
6 uncaughtException listener count (measured live at crash time) 1 1 — consolidation confirmed, no double-handler ✅ confirmed

Row 6 is the specific thing the follow-up commit fixes, measured in the live process rather than inferred: exactly one listener is active at crash time, so the session-aware handler is the one that actually runs.

Row 4 confirms the onError gating works — a normal /quit produces no bogus "Rendering error (logged to debug file)" line.

Static checks

  • gemini.test.tsx + ErrorBoundary.test.tsx64/64 pass
  • npm run typecheck — clean
  • eslint over all 6 touched files — clean

Not covered by this validation

Stating these plainly so the coverage claim isn't overread:

  • kittyProtocolDetector.ts changes. The test pty does not respond to the kitty capability query, so the protocol was never enabled (ESC[>1u never sent) and the removed SIGTERM/SIGINT handlers were never on the exit path. This needs a real Kitty/Ghostty terminal to verify.
  • The ThinkBody ErrorBoundary. Triggering it requires a malformed-markdown render error during thought streaming, which I could not induce deterministically without a live model turn.
  • Terminal state was otherwise restored correctly in every run (1049l, 25h, 1002l/1006l, 2004l all observed on exit).

Follow-up finding (non-blocking)

The debug-log half of the handler silently no-ops on a fresh machine.

Debug-log write no-ops when ~/.qwen/debug is absent

fs.appendFileSync(Storage.getDebugLogPath(sessionId), …) creates the file but not the directory. ~/.qwen/debug/ is created lazily by debugLogger, which only runs when QWEN_DEBUG_LOG_FILE is enabled — unset for virtually all users. So on a default install the append throws ENOENT and is swallowed by the surrounding catch {}, while stderr still prints "Fatal: uncaught exception (logged to debug file)".

I verified this by isolating the single variable — same build, same injected fault, QWEN_DEBUG_LOG_FILE unset in both runs:

  • ~/.qwen/debug/ exists → debug file created, 1 UNCAUGHT_EXCEPTION entry, message accurate
  • ~/.qwen/debug/ absent → no file created, 0 entries, message is false

Impact is limited — the primary fix (stack trace on the main screen) works in both cases; only the persisted record is lost. Suggested one-liner:

fs.mkdirSync(path.dirname(logPath), { recursive: true });
fs.appendFileSync(logPath, line, 'utf8');

Happy to see this land as-is and fix the above in a follow-up.

🇨🇳 中文版本

真实环境验证 ✅

我把这个 PR 合并到当前 main 后完整构建,并在真实 pty 中运行真实的 CLI 二进制、且 VP / 备用屏幕(alternate screen)模式处于激活状态下做了验证,而不是只跑单元测试。结论:**该修复确实达成了它声称的效果,且未发现回归。**文末有一个不阻塞合并的后续问题。

环境

基线 main @ f4cd6e1d8
候选 PR head 1d43cd35d 合并进 main8c3719b41(合并干净,无冲突)
环境 macOS 26.6 arm64,Node v24.18.1,真实 pty 120×34,TERM=xterm-256color
VP 模式 已确认激活 —— 在捕获的字节流中观察到 ESC[?1049h

故障通过 node --import 在定时器回调中抛出,是真正的异步 uncaughtException未修改任何产品代码。捕获的 pty 字节流经由真实终端模拟器(@xterm/headless)回放,因此截图展示的就是用户终端实际会显示的内容,包括备用屏幕的语义。

核心问题的复现与修复

main 上,堆栈其实是被写出来了的,问题纯粹在于顺序。在捕获的字节流中,堆栈写在第 42530 字节,而备用屏幕直到第 43070 字节才退出。也就是说堆栈被写进了备用缓冲区,随缓冲区一起被丢弃。

结果

# 场景 main @ f4cd6e1d8 main + 本 PR
1 VP 模式下的未捕获异常 exit 1,什么都看不到 —— 堆栈随备用缓冲区丢弃 exit 1,完整堆栈保留在主缓冲区 ✅ 已修复
2 同上,QWEN_DEBUG_LOG_FILE=1 调试日志中 0 条 UNCAUGHT_EXCEPTION 1 条,在 process.exit()同步写入 ✅ 已修复
3 良性 PTY 竞态(read EIO 被抑制,会话不受影响 被抑制 —— 新处理器保留了 PTY 守卫 ✅ 无回归
4 正常 /quit exit 0,无多余输出 exit 0,没有多余的 Rendering error ✅ 无回归
5 SIGHUP(关闭终端) 被信号 1 杀死 —— 无处理器 干净退出 129,走完整清理链 ✅ 已修复
6 uncaughtException 监听器数量(崩溃时实测) 1 1 —— 确认已合并,无双处理器 ✅ 已确认

第 6 行正是后续 commit 所修复的问题,这里是在活进程中实测而非推断:崩溃时刻恰好只有一个监听器,因此真正执行的是那个感知 session 的处理器。

第 4 行确认 onError 门控生效 —— 正常 /quit 不会再打印虚假的 "Rendering error (logged to debug file)"。

静态检查

  • gemini.test.tsx + ErrorBoundary.test.tsx —— 64/64 通过
  • npm run typecheck —— 通过
  • 对 6 个改动文件跑 eslint —— 通过

本次验证未覆盖的部分

明确说明,避免高估覆盖范围:

  • kittyProtocolDetector.ts 的改动。 测试用 pty 不响应 kitty 能力查询,因此该协议从未启用(ESC[>1u 从未发出),被移除的 SIGTERM/SIGINT 处理器也就从未出现在退出路径上。这需要真实的 Kitty/Ghostty 终端来验证。
  • ThinkBodyErrorBoundary 触发它需要在思考流式输出过程中产生 markdown 渲染错误,我无法在没有真实模型请求的情况下稳定构造。
  • 其余各场景中终端状态均正确恢复(退出时都观察到 1049l25h1002l/1006l2004l)。

后续问题(不阻塞合并)

处理器中写调试日志的那一半,在全新机器上会静默失效。

fs.appendFileSync(Storage.getDebugLogPath(sessionId), …) 只会创建文件,不会创建目录。而 ~/.qwen/debug/ 是由 debugLogger 懒创建的,只有在启用 QWEN_DEBUG_LOG_FILE 时才会执行 —— 而绝大多数用户并未设置该变量。因此在默认安装下,append 会抛出 ENOENT 并被外层 catch {} 吞掉,但 stderr 仍然打印 "Fatal: uncaught exception (logged to debug file)"

我通过只改变这一个变量做了验证(同一构建、同一注入故障,两次运行都未设置 QWEN_DEBUG_LOG_FILE):

  • ~/.qwen/debug/ 存在 → 创建了调试文件,1 条 UNCAUGHT_EXCEPTION,提示属实
  • ~/.qwen/debug/ 不存在 → 未创建文件,0 条记录,提示是假的

影响有限 —— 主要修复(堆栈显示在主屏幕)在两种情况下都正常,只有持久化记录会丢失。建议的一行修复:

fs.mkdirSync(path.dirname(logPath), { recursive: true });
fs.appendFileSync(logPath, line, 'utf8');

我认为可以先按现状合并,上述问题在后续 PR 中处理即可。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@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. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/cli/src/gemini.tsx Outdated
Comment thread packages/cli/src/ui/components/messages/ConversationMessages.tsx
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 AutoFix could not start — a setup step failed (or the run was cancelled) before the agent ran, so no fix was attempted. This is normally a transient infra issue, a broken base build, or a cancelled run — not this PR. It will retry on the next scan.

AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why).

Run log: https://github.com/QwenLM/qwen-code/actions/runs/30547201647


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 AutoFix could not start — a setup step failed (or the run was cancelled) before the agent ran, so no fix was attempted. This is normally a transient infra issue, a broken base build, or a cancelled run — not this PR. It will retry on the next scan.

AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why).

Run log: https://github.com/QwenLM/qwen-code/actions/runs/30551642622


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@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. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its E2E suite did not run locally.

— qwen3.8-max-preview via Qwen Code /review

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

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

⚠️ Downgraded from Approve to Comment: CI failing: Test (ubuntu-latest, Node 22.x). Reviewed.

— qwen3.8-max-preview via Qwen Code /review

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Round summary — no action taken

No new review feedback (inline comments, reviews, or issue-level comments) was
received this round. The deferred non-Critical feedback is in critical-only
mode and was not touched per policy.

Failed CI check investigation

The Test (ubuntu-latest, Node 22.x) check reports FAILURE. Investigation:

  • All directly related unit tests pass locally (142+ tests across
    gemini.test.tsx, cli.test.ts, ErrorBoundary.test.tsx,
    ConversationMessages.test.tsx, kittyProtocolDetector.test.ts,
    HistoryItemDisplay.test.tsx, TranscriptView.test.tsx,
    TranscriptView.errorFallback.test.tsx, review/run.test.ts,
    debugLogger.test.ts).
  • ESLint, Prettier, TypeScript typecheck, and i18n check all pass.
  • Settings schema is up to date (no diff after
    generate:settings-schema).
  • The latest merge from main brought in only one file
    (integration-tests/sdk-typescript/permission-control.test.ts), which is
    unrelated to this PR's changes.
  • CI logs are not accessible from this environment (no GitHub credentials),
    so the specific failing step/test could not be identified.
  • The full test suite (npm run test:ci) could not be run to completion in
    this sandbox (resource timeout).

The failure is likely a flaky or environment-specific test unrelated to this
PR's changes. A CI re-run is recommended.

中文说明

本轮总结 — 未做任何改动

本轮未收到新的评审反馈(行内评论、评审或 Issue 级别评论)。延后的非 Critical 反馈处于仅处理 Critical 模式,按策略未做改动。

CI 失败检查调查

Test (ubuntu-latest, Node 22.x) 检查报告 FAILURE。调查结果:

  • 所有直接相关的单元测试在本地均通过(142+ 个测试,涵盖
    gemini.test.tsxcli.test.tsErrorBoundary.test.tsx
    ConversationMessages.test.tsxkittyProtocolDetector.test.ts
    HistoryItemDisplay.test.tsxTranscriptView.test.tsx
    TranscriptView.errorFallback.test.tsxreview/run.test.ts
    debugLogger.test.ts)。
  • ESLint、Prettier、TypeScript 类型检查和 i18n 检查均通过。
  • Settings schema 是最新的generate:settings-schema 后无差异)。
  • 最近一次从 main 的合并仅引入了一个文件
    integration-tests/sdk-typescript/permission-control.test.ts),与本 PR 的改动无关。
  • 本环境无法访问 CI 日志(无 GitHub 凭据),因此无法确定具体失败的步骤/测试。
  • 完整测试套件(npm run test:ci)在本沙箱中无法运行完成(资源超时)。

该失败很可能是与本 PR 改动无关的不稳定测试或环境特定测试。建议重新运行 CI。

Deferred non-Critical feedback

Critical-only mode is active after 5 change-producing rounds: the automated reviewer's non-Critical suggestions below are deferred and stay open for human follow-up — do not modify code, resolve threads, or reply on their behalf. Maintainer feedback defers only once its author has already had 2 regular feedback batches addressed in this window's Critical-only tail — an account can host an automated reviewer loop, so the brake keys on measured regeneration, not identity; authors at their budget, if any, are named below. (A maintainer can lift the mode itself: @qwen-code /retry starts a fresh counting window.)

中文说明

完成 5 个产生改动的轮次后进入仅处理 Critical 的模式:以上为自动评审的非 Critical 建议,予以延后、保持开放并留待人工跟进——不要为其修改代码、解决线程或代为回复。维护者的反馈仅在其本人于本窗口 Critical-only 阶段已被处理 2 批常规反馈之后才会被延后——账号可能挂着自动评审循环,因此刹车依据实测的再生频度而非身份;达到预算的作者(如有)在下方点名。(如需解除该模式,评论 @qwen-code /retry 即可开启新的计数窗口。)

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

wenshao
wenshao previously approved these changes Jul 31, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@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. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its E2E suite did not run locally.

— qwen3.8-max-preview via Qwen Code /review

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 AutoFix ran out of time before finishing (timeout (3000000ms)) (attempt 6/100) — it will retry on the next scan.

What I found before stopping:
Qwen failed during address-review: timeout (3000000ms).

See the Qwen Autofix agent step logs for model/tool output.

Run log: https://github.com/QwenLM/qwen-code/actions/runs/30601427258


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

No action taken

No new review feedback (reviews, inline comments, or issue-level comments) was received this round. The deferred non-Critical suggestions remain deferred under Critical-only mode.

Failed CI check: pre-existing, unrelated to this PR

The Test (ubuntu-latest Node 22.x) check fails on the integration test integration-tests/cli/qwen-serve-streaming.test.ts — specifically the "daemon Todo Stop Guard replay" case, which expects 4 fake-server requests but receives 2 (expected 2 to be 4).

Evidence this is pre-existing: I restored the origin/main versions of every file this PR changes, rebuilt, and re-ran the same integration test. It fails identically on main branch code:

Error: Matcher did not succeed in time.
 ❯ cli/qwen-serve-streaming.test.ts:519:8
Caused by: AssertionError: expected 2 to be 4 // Object.is equality

The failing test exercises the daemon's Todo Stop Guard retry loop, which is unrelated to this PR's changes (uncaught-exception handler hardening, SIGHUP exit code, ErrorBoundary exit-time echo, and kitty-protocol signal-handler scoping). No code path touched by this PR is involved in the guard's request-count behavior.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • Prettier check on all 8 changed files — passed
  • npm run generate:settings-schema + git status — schema up-to-date
  • vitest run src/gemini.test.tsx (packages/cli) — 58 passed
  • vitest run src/cli.test.ts (packages/cli) — 53 passed
  • vitest run src/ui/components/shared/ErrorBoundary.test.tsx (packages/cli) — 8 passed
  • vitest run src/ui/components/messages/ConversationMessages.test.tsx (packages/cli) — 20 passed
  • vitest run src/ui/utils/kittyProtocolDetector.test.ts (packages/cli) — 3 passed
  • Full vitest run for packages/cli — all tests passed
  • integration-tests/cli/qwen-serve-streaming.test.ts — 1 failed (pre-existing on main), 3 passed
  • integration-tests/cli/qwen-serve-routes.test.ts — passed
  • integration-tests/fake-openai-server.test.ts — passed
  • integration-tests/cli/daemon-invocation-context.test.ts — passed
中文说明

未采取任何操作

本轮没有收到新的评审反馈(评审、行内评论或 Issue 级评论)。延后的非 Critical 建议在 Critical-only 模式下继续保持延后状态。

CI 检查失败:属于已有问题,与本 PR 无关

Test (ubuntu-latest Node 22.x) 检查在集成测试 integration-tests/cli/qwen-serve-streaming.test.ts 上失败——具体是 "daemon Todo Stop Guard replay" 用例,期望 fake-server 收到 4 次请求但实际只收到 2 次(expected 2 to be 4)。

证明这是已有问题的证据: 我将本 PR 修改的所有文件恢复为 origin/main 版本,重新构建并重新运行了同一集成测试。在 main 分支代码上同样失败:

Error: Matcher did not succeed in time.
 ❯ cli/qwen-serve-streaming.test.ts:519:8
Caused by: AssertionError: expected 2 to be 4 // Object.is equality

失败的测试验证的是守护进程的 Todo Stop Guard 重试循环,与本 PR 的更改(uncaught-exception 处理器加固、SIGHUP 退出码、ErrorBoundary 退出时回显、kitty 协议信号处理器作用域)无关。本 PR 触及的任何代码路径都不涉及 guard 的请求计数行为。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • 对全部 8 个修改文件进行 Prettier 检查 — 通过
  • npm run generate:settings-schema + git status — schema 为最新
  • vitest run src/gemini.test.tsx(packages/cli)— 58 通过
  • vitest run src/cli.test.ts(packages/cli)— 53 通过
  • vitest run src/ui/components/shared/ErrorBoundary.test.tsx(packages/cli)— 8 通过
  • vitest run src/ui/components/messages/ConversationMessages.test.tsx(packages/cli)— 20 通过
  • vitest run src/ui/utils/kittyProtocolDetector.test.ts(packages/cli)— 3 通过
  • packages/cli 完整 vitest run — 全部测试通过
  • integration-tests/cli/qwen-serve-streaming.test.ts — 1 失败(main 上已有问题),3 通过
  • integration-tests/cli/qwen-serve-routes.test.ts — 通过
  • integration-tests/fake-openai-server.test.ts — 通过
  • integration-tests/cli/daemon-invocation-context.test.ts — 通过

Deferred non-Critical feedback

Critical-only mode is active after 5 change-producing rounds: the automated reviewer's non-Critical suggestions below are deferred and stay open for human follow-up — do not modify code, resolve threads, or reply on their behalf. Maintainer feedback defers only once its author has already had 2 regular feedback batches addressed in this window's Critical-only tail — an account can host an automated reviewer loop, so the brake keys on measured regeneration, not identity; authors at their budget, if any, are named below. (A maintainer can lift the mode itself: @qwen-code /retry starts a fresh counting window.)

中文说明

完成 5 个产生改动的轮次后进入仅处理 Critical 的模式:以上为自动评审的非 Critical 建议,予以延后、保持开放并留待人工跟进——不要为其修改代码、解决线程或代为回复。维护者的反馈仅在其本人于本窗口 Critical-only 阶段已被处理 2 批常规反馈之后才会被延后——账号可能挂着自动评审循环,因此刹车依据实测的再生频度而非身份;达到预算的作者(如有)在下方点名。(如需解除该模式,评论 @qwen-code /retry 即可开启新的计数窗口。)

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@doudouOUC doudouOUC 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. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

Comment on lines +175 to +181
export function setupUncaughtExceptionHandler(config: Config) {
// runCliEntryPoint() registered the basic handleUncaughtException at startup,
// before the session ID existed. Replace it now: two listeners conflict — the
// first calls process.exit(1) so the second never runs — and the basic one
// lacks the debug-log write and the alternate-screen handling below. Also drop
// any handler a previous call installed so exactly one listener is ever active.
process.removeListener('uncaughtException', handleUncaughtException);

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.

[Suggestion] setupUncaughtExceptionHandler (~60 lines) has no dedicated unit tests despite having multiple testable branches. — Concrete cost: handler replacement logic (two-listener conflict), synchronous debug log write (async abandoned by process.exit), alternate-screen escape (TTY guard), and PTY race suppression could all silently regress. The SIGHUP and render-error echo paths added in the same PR are tested; this function is not.

Suggested tests: (a) PTY race error is suppressed; (b) debug log is written synchronously with correct format; (c) alternate-screen escape sequences are written when stdout.isTTY; (d) escape sequences are skipped when stdout is not a TTY; (e) process.exit(1) is called; (f) previous handler is removed before new one is installed.

— qwen3.7-max via Qwen Code /review

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.

Good catch — setupUncaughtExceptionHandler does deserve dedicated coverage for the branches you list (PTY-race suppression, the synchronous debug-log write, the isTTY alternate-screen guard, process.exit(1), and removing the previous handler before installing the new one).

Deferring this one for now: the PR has completed five change-producing rounds and is in critical-only mode, so this round lands only the Critical that was dead-bundling the CLI (the entry↔lazy-module cycle, fixed by moving the helpers into utils/uncaught-exception-handler.ts). Per the repo's review policy, non-Critical suggestions past five rounds are deferred to a follow-up rather than widening the diff here. Leaving this thread open so the six test cases are tracked and not dropped — they'd make a good small follow-up PR.

中文说明

说得对——setupUncaughtExceptionHandler 确实值得为你列出的这些分支补专门的覆盖(PTY 竞态抑制、同步写调试日志、isTTY 备用屏守卫、process.exit(1),以及在安装新处理器前先移除旧处理器)。

先延后这一项:本 PR 已经完成五个产生改动的轮次、进入仅处理 Critical 的模式,因此本轮只落地那个让打包 CLI 失效的 Critical(entry↔懒加载模块成环,已通过将 helper 挪入 utils/uncaught-exception-handler.ts 修复)。按仓库的评审政策,超过五轮后的非 Critical 建议延后到后续处理,以免在此处扩大 diff。保持本线程开放,以便跟踪这六个测试用例、不被丢弃——它们很适合作为一个小的后续 PR。

@wenshao

wenshao commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

The red Test (ubuntu-latest, Node 22.x) here is real and it is caused by this PR — not a stale base. The base-update bot re-merged main at 02:30 on that assumption and the re-run failed identically, so it is worth spelling out, because the failure lands nowhere near the line that causes it.

Root cause

This line in gemini.tsx:

import { handleUncaughtException, isExpectedPtyRaceError } from './cli.js';

cli.ts is the esbuild entry point (esbuild.config.js), and gemini.tsx is only ever reached lazily, through await import('./gemini.js') in runCliEntry(). Importing the entry back from a lazily-loaded module creates a cycle, and the bundle is built with splitting: true, so esbuild resolves it by moving the entry module's body into a shared chunk and leaving dist/cli.js as a bare re-export stub.

That silently disables the bootstrap at the bottom of cli.ts:

if (
  process.argv[1] !== undefined &&
  import.meta.url === pathToFileURL(process.argv[1]).href
) {
  void runCliEntryPoint();
}

Inside a chunk, import.meta.url is the chunk's own URL, so it never equals argv[1]. runCliEntryPoint() is never called — the bundled CLI starts, does nothing, and exits 0.

That is exactly what the smoke test reports, three times over:

daemon exited with 0 before listening:
stdout=
stderr=

Worth noting how much this breaks: the entire bundled CLI is dead, not just qwen serve. The no-AK integration smoke test is simply the only CI step that executes dist/cli.js; tsc, eslint and every src-based unit test stay green because they run against src/.

Measurements

Built this PR's head (2ea51c4e) with the real esbuild.config.js options:

Build dist/cli.js Bootstrap guard
This PR's head 629 B re-export stub relocated into chunks/chunk-*.js
Same tree, only that one import removed 12,268 B in the entry, intact
With the fix below 11,467 B in the entry, intact

Confirmed the mechanism separately on a minimal esbuild reproduction too: with the cycle plus splitting: true, running the output prints nothing and exits 0; drop the cycle and it runs normally.

Fix

Move the two helpers into a leaf module both sides can import. The uncaughtException behavior this PR adds is unaffected — only where the functions live changes.

Add packages/cli/src/utils/uncaught-exception-handler.ts holding getErrnoCode, isExpectedPtyRaceError and handleUncaughtException verbatim, then:

--- a/packages/cli/src/gemini.tsx
+++ b/packages/cli/src/gemini.tsx
-import { handleUncaughtException, isExpectedPtyRaceError } from './cli.js';
+import {
+  handleUncaughtException,
+  isExpectedPtyRaceError,
+} from './utils/uncaught-exception-handler.js';
--- a/packages/cli/src/cli.ts
+++ b/packages/cli/src/cli.ts
+import {
+  handleUncaughtException,
+  isExpectedPtyRaceError,
+} from './utils/uncaught-exception-handler.js';
+
+// Re-exported for existing importers. gemini.tsx must import these from
+// ./utils/uncaught-exception-handler.js directly: a static import of this file
+// from a module the bundle loads lazily makes esbuild hoist this entry into a
+// shared chunk, which silently disables the bootstrap guard at the bottom.
+export { handleUncaughtException, isExpectedPtyRaceError };

and delete the two definitions from cli.ts. The re-export keeps cli.test.ts working as-is — it imports isExpectedPtyRaceError from ./cli.js.

Two things that will bite otherwise:

  • the new file must be kebab-case; check-file/filename-naming-convention rejects uncaughtExceptionHandler.ts at --max-warnings 0.
  • please keep a comment explaining why the helpers live outside cli.ts. Nothing about them looks entry-specific, so they are an easy candidate for someone to fold back in later.

With this applied on top of 2ea51c4e: src/cli.test.ts + src/gemini.test.tsx 111 passed, tsc --noEmit clean, eslint --max-warnings 0 clean, and the entry is back to compiling into dist/cli.js.

Follow-up

No CI gate catches this class today, which is why it surfaced as an unrelated-looking daemon failure. I opened #8203 to add one: it asserts, from the esbuild metafile the startup closure checks already read, that dist/cli.js still compiles packages/cli/src/cli.ts. Run against this PR's head it fails with a message naming the cause and the fix; against the fixed tree it passes. That PR is the gate only — the import change above still belongs here.

中文说明

这里 Test (ubuntu-latest, Node 22.x) 的红是真实的,而且是本 PR 引入的——不是 base 过期。02:30 时 update-branch 机器人按「base 过期」重新合了一次 main,重跑后报错完全一样,所以有必要说清楚:失败的位置离真正的元凶非常远。

根因

gemini.tsx 里的这一行:

import { handleUncaughtException, isExpectedPtyRaceError } from './cli.js';

cli.ts 是 esbuild 的 entry(见 esbuild.config.js),而 gemini.tsx 只会被 runCliEntry() 里的 await import('./gemini.js') 懒加载。从懒加载模块反过来静态引用 entry 就形成了环,而 bundle 开着 splitting: true,esbuild 的处理方式是把 entry 的模块体搬进共享 chunk,dist/cli.js 只剩一个 re-export 空壳。

这会悄无声息地废掉 cli.ts 底部的 bootstrap:

if (
  process.argv[1] !== undefined &&
  import.meta.url === pathToFileURL(process.argv[1]).href
) {
  void runCliEntryPoint();
}

在 chunk 里 import.meta.url 指向 chunk 自身,永远不等于 argv[1]runCliEntryPoint() 一次都不会被调用——打包后的 CLI 启动、什么都不做、exit 0。

这正是冒烟测试连报三次的内容:

daemon exited with 0 before listening:
stdout=
stderr=

值得强调影响面:整个打包 CLI 都是死的,不只是 qwen serve。no-AK 集成冒烟测试只是唯一会执行 dist/cli.js 的 CI 步骤;tsc、eslint 和所有基于 src 的单测都是绿的,因为它们跑的是 src/

实测数据

用真实的 esbuild.config.js 选项构建了本 PR 的 head(2ea51c4e):

构建 dist/cli.js bootstrap 守卫
本 PR head 629 B re-export 空壳 被搬进 chunks/chunk-*.js
同一棵树,只删掉那一行 import 12,268 B 在 entry 内,完好
应用下面的修法后 11,467 B 在 entry 内,完好

另外用一个最小 esbuild 复现单独验证了机制:加上环并开启 splitting: true 后,跑产物无任何输出、exit 0;去掉环则一切正常。

修法

把这两个 helper 挪到一个两边都能引用的叶子模块。本 PR 新增的 uncaughtException 行为完全不受影响——变的只是函数放在哪儿。

新增 packages/cli/src/utils/uncaught-exception-handler.ts,原样收纳 getErrnoCodeisExpectedPtyRaceErrorhandleUncaughtException,然后(diff 见上方英文部分),并从 cli.ts 删掉这两个定义。保留的 re-export 让 cli.test.ts 不用改——它是从 ./cli.jsisExpectedPtyRaceError 的。

有两点不注意会踩:

  • 新文件名必须是 kebab-case;check-file/filename-naming-convention--max-warnings 0 下会直接拒掉 uncaughtExceptionHandler.ts
  • 请保留一段注释说明这两个 helper 为什么要放在 cli.ts 之外。它们看起来跟 entry 没什么关系,后面很容易被人合并回去。

2ea51c4e 上应用之后:src/cli.test.ts + src/gemini.test.tsx 111 passed,tsc --noEmit 干净,eslint --max-warnings 0 干净,entry 也重新编译进了 dist/cli.js

后续

目前 CI 没有任何门禁能拦住这一类问题,所以它才会以一个看起来毫不相干的 daemon 失败的形式暴露出来。我开了 #8203 来补这道门禁:它从启动闭包检查本来就在读的 esbuild metafile 出发,断言 dist/cli.js 里仍然编译进了 packages/cli/src/cli.ts。拿本 PR 的 head 去跑会失败并给出指明成因与修法的提示,对修好的树则通过。那个 PR 只做门禁——上面的 import 改动仍然要在这里做。


Reviewed with Claude Code (Opus 5, 1M context)

@wenshao

wenshao commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Round-2 real-environment validation at 307442d6e9

The head has moved since my earlier validation (the Config-based handler, the recordForExitEcho scoping, the mkdirSync follow-up, plus three base merges). I rebuilt and re-ran everything against the current head — and this round I also closed the two gaps I had to leave open last time: kittyProtocolDetector.ts and the ThinkBody ErrorBoundary. Both now have decisive A/B evidence.

Verdict: still good to merge. The mkdirSync follow-up I asked for is implemented and verified. One new non-blocking follow-up at the bottom.

Validation matrix: 10 scenarios, baseline vs PR

Setup

Baseline main @ 702932cc7c (this PR's merge base)
Candidate PR head 307442d6e9. The head has since moved to 2ea51c4e80, but that is a main merge only — the eight files this PR owns are byte-identical between the two, so everything below still applies.
Build one worktree, one npm ci; both arms built with the same toolchain and the same node_modules. The two dist trees differ in exactly the six files the PR touches — nothing else.
Environment Linux, Node v22.22.2, real pty 118×34 via pty.fork(), TERM=xterm-256color
Faults a genuine synchronous uncaughtException raised from a timer callback via node --import (product code untouched). The two render faults are byte-identical injections applied to both arms (verified by md5sum) and removed afterwards.

The two gaps from last round, now closed

1. kittyProtocolDetector.ts — removing the SIGTERM/SIGINT handlers

Last time the harness pty did not answer the Kitty capability query, so the protocol was never enabled and the removed handlers were never on the exit path. This round the harness answers CSI ? u itself, so ESC[>1u is genuinely pushed and the teardown path is real.

Kitty pop vs alternate-screen exit, byte offsets

The flags are pushed once on the main screen at startup (offset 7) and once more on the alternate screen (offset 3278, pushKittyProtocolFlags). On main, the Kitty signal handler pops before Ink leaves the alternate screen, so the pop is spent on a buffer that is about to be discarded — and because disableProtocol() latches on protocolEnabled, the later disableKittyProtocol() in the cleanup chain becomes a no-op. Net result on baseline: zero pops reach the main screen, which is exactly #7779. The PR gets it right on both signals.

I also checked the obvious regression risk — is there a path where the protocol is enabled but installInteractiveSignalHandlers() is not installed? No: both are gated on config.isInteractive(), and the signal handlers are installed first (gemini.tsx:935 before the detection at :941). The process.on('exit') fallback is retained for process.exit() paths.

2. ThinkBody's ErrorBoundary

Driven with a mock provider streaming real reasoning_content, with a markdown render fault forced for the thought text only, then expanded with alt+t.

ThinkBody boundary A/B

  • main: the fault reaches the top-level boundary → the entire TUI is replaced by "Something went wrong while rendering.", FATAL_RENDER_ERROR in the debug log, process exits 1. The session is gone.
  • PR: THINK_RENDER_ERROR logged, 0 FATAL_RENDER_ERROR, only the thought block degrades to plain text, the session keeps running and /quit still exits 0.

One precision note for the PR body: this boundary guards the expanded thought view. While a thought streams collapsed — the default — ThinkBody returns a plain <Text> tail window and never reaches MarkdownDisplay, so "partial markdown during thought streaming" only applies once the user has expanded the thought (alt+t / click / ctrl+o). Worth a one-line wording tweak; the code is right.

Core behaviour re-confirmed at this head

Crash visibility A/B

In the captured byte stream the ordering is unambiguous: baseline writes the trace at offset 18629 and only leaves the alternate screen at 18947 — trace inside the discarded buffer. The PR leaves at 18622 and writes the trace at 18695 — on the main screen.

Additional rows re-checked at this head:

  • mkdirSync follow-up — fixed and verified. With QWEN_DEBUG_LOG_FILE unset and ~/.qwen/debug absent, the PR now creates the directory and writes 1 UNCAUGHT_EXCEPTION entry, so the "(logged to debug file)" wording is truthful on a fresh machine. Baseline: no directory, 0 entries.
  • Benign pty race — no regression. I made the injector record that it actually threw (read EIO, code: EIO), then confirmed the session survived to a clean /quit exit 0 in both arms. The suppression genuinely fires rather than the fault silently not happening.
  • Clean /quit — no regression. Exit 0 in both arms, no spurious Rendering error line, so the recordForExitEcho scoping does not produce false positives.
  • Render-error echo works end-to-end. With a forced React render fault, the PR echoes Rendering error (logged to debug file): … at offset 18973, after the alternate-screen exit at 18947; baseline emits nothing to the main screen.
  • Listener count measured live at fault time: 1 in both arms — the consolidation holds.
  • SIGHUP: baseline is killed by signal 1 (WIFSIGNALED, no exit code, no Kitty pop); the PR exits cleanly with 129 through runExitCleanup().

Also verified: the fix survives bundling

process.removeListener('uncaughtException', handleUncaughtException) only works if cli.ts and gemini.tsx resolve the same function object. That is obvious in the per-file tsc output, but not in the shipped esbuild bundle, where the two files land in different chunks — a duplicated copy there would leave two listeners, the basic one would win, and the whole fix would be silently dead in the published artifact.

Checked: handleUncaughtException is defined exactly once (dist/chunks/chunk-6KMALBFR.js) and the gemini chunk imports that binding. A live crash run against the bundle reports 1 listener, the trace on the main screen, and the debug entry written. Same behaviour as the per-file build.

New follow-up (non-blocking): ?1049l is emitted even when this process never entered the alternate screen

Outer TUI evicted by the handler

The handler gates the alternate-screen exit on process.stdout.isTTY, which is true in plenty of modes that never enter it: -p, --acp, serve, screen-reader mode, and ui.useTerminalBuffer: false. In a plain shell that is harmless — I verified that earlier output stays intact in both arms. But when an outer program owns the alternate screen and shells out to qwen -p, an uncaught exception now tears that screen down: #{alternate_on} goes 1 → 0, the outer program keeps running with its display destroyed, and the crash text lands on top of the user's shell scrollback. Baseline leaves the outer screen intact.

Suggested fix — gate on whether we entered it rather than on isTTY, e.g. have startInteractiveUI set a module-level flag when it renders with alternateScreen: true, and read that flag in the handler. The VP path this PR targets is unaffected either way.

Not covered

Stating these plainly so the coverage claim isn't overread:

  • A real Kitty/Ghostty terminal. The harness answers the capability query on a real pty, which drives the same code path and gives byte-exact ordering, but it is not a real terminal's flag stack.
  • Windows / macOS. Linux only this round.
  • The original user-reported crash from the linked issues. The PR does not claim to fix it — it makes it visible — and I did not reproduce the underlying fault.

Static checks

  • gemini.test.tsx + ErrorBoundary.test.tsx66/66 pass
  • Full npm ci build of all packages, and npm run bundle — both clean
🇨🇳 中文版本

第二轮真实环境验证(307442d6e9)✅

上一轮验证以来 HEAD 已经前进(改为传入 Config 的处理器、recordForExitEcho 的收窄、mkdirSync 的后续修复,外加三次 base 合并)。我基于当前 HEAD 重新构建并跑了全部用例,并且补上了上一轮明确未覆盖的两块kittyProtocolDetector.tsThinkBodyErrorBoundary。这两块现在都有决定性的 A/B 证据。

**结论:仍然可以合并。**我上轮提出的 mkdirSync 后续问题已经实现并验证通过。文末有一个新的、不阻塞合并的后续问题。

环境

基线 main @ 702932cc7c(即本 PR 的 merge base)
候选 PR head 307442d6e9。期间 HEAD 已前进到 2ea51c4e80,但那只是一次 main 合并 —— 本 PR 自身的 8 个文件在两者之间逐字节相同,因此下述结论仍然适用。
构建 同一个 worktree、同一次 npm ci;两个 arm 用同一套工具链、同一份 node_modules 构建。两棵 dist 树的差异恰好只有本 PR 改动的那 6 个文件,没有别的。
环境 Linux,Node v22.22.2,pty.fork() 起的真实 pty 118×34,TERM=xterm-256color
故障注入 通过 node --import 在定时器回调里抛出真正的同步 uncaughtException(未改动任何产品代码)。两处渲染故障是逐字节相同的注入,两个 arm 都打md5sum 已核对),测完即还原。

上轮的两个空白,本轮补齐

1. kittyProtocolDetector.ts —— 移除 SIGTERM/SIGINT 处理器

上一轮的 pty 不会回应 kitty 能力查询,所以该协议从未启用,被移除的处理器也从未出现在退出路径上。本轮 harness 自己回应 CSI ? u,因此 ESC[>1u 是真的被推入了,退出路径是真实的。

标志位在启动时在主屏推入一次(偏移 7),进入备用屏后再推一次(偏移 3278,pushKittyProtocolFlags)。在 main 上,kitty 的信号处理器在 Ink 离开备用屏之前就 pop 了,这一次 pop 消耗在了一个即将被丢弃的缓冲区上;又因为 disableProtocol()protocolEnabled 做了闩锁,清理链里后续的 disableKittyProtocol() 就变成了 no-op。基线的净结果是:主屏一次 pop 都没收到,这正是 #7779。本 PR 在 SIGTERM 和 SIGINT 两条路径上都改对了。

我也检查了最明显的回归风险——是否存在「协议已启用但没装 installInteractiveSignalHandlers()」的路径?没有:两者都由 config.isInteractive() 把关,且信号处理器先装(gemini.tsx:935 在探测的 :941 之前)。process.on('exit') 兜底也保留了。

2. ThinkBodyErrorBoundary

用 mock provider 流式返回真实的 reasoning_content 驱动,仅对思考文本强制一个 markdown 渲染故障,然后用 alt+t 展开:

  • main:故障一路冒泡到顶层边界 → 整个 TUI 被替换成 "Something went wrong while rendering.",调试日志里是 FATAL_RENDER_ERROR,进程退出码 1,会话彻底丢失。
  • 本 PR:记录 THINK_RENDER_ERRORFATAL_RENDER_ERROR0,只有思考块降级成纯文本,会话继续存活,/quit 仍然是 exit 0。

关于 PR 描述的一点精确性建议:这个边界保护的是已展开的思考视图。思考在折叠状态下流式输出时(也就是默认情况),ThinkBody 返回的是纯 <Text> 尾窗,根本不会走到 MarkdownDisplay。所以「思考流式输出过程中的半截 markdown」这一说法只在用户展开之后(alt+t / 点击 / ctrl+o)才成立。代码是对的,建议顺手改一下措辞。

在当前 HEAD 上复核的核心行为

捕获的字节流里顺序非常清楚:基线在偏移 18629 写堆栈,直到 18947 才离开备用屏 —— 堆栈落在了会被丢弃的缓冲区里;本 PR 在 18622 离开、18695 写堆栈 —— 落在主屏上。

其余复核项:

  • **mkdirSync 后续修复 —— 已修复并验证。**在 QWEN_DEBUG_LOG_FILE 未设置、~/.qwen/debug 不存在的情况下,本 PR 会创建目录并写入 1 条 UNCAUGHT_EXCEPTION,因此 "(logged to debug file)" 这句提示在全新机器上是属实的。基线:目录不存在,0 条记录。
  • **良性 pty 竞态 —— 无回归。**我让注入器在抛出时先落一条标记(read EIOcode: EIO),确认异常确实抛了,然后两个 arm 都活到了 /quit 并 exit 0。也就是说抑制逻辑是真的生效,而不是故障压根没发生。
  • **正常 /quit —— 无回归。**两个 arm 都是 exit 0,都没有多余的 Rendering error,说明 recordForExitEcho 的收窄不会产生误报。
  • **渲染错误回显端到端可用。**强制一个 React 渲染故障后,本 PR 在偏移 18973 输出 Rendering error (logged to debug file): …,位于 18947 的备用屏退出之后;基线在主屏什么也没有。
  • 崩溃时刻实测监听器数量:两个 arm 都是 1 —— 合并为单一监听器的结论成立。
  • SIGHUP:基线被信号 1 杀死(WIFSIGNALED,没有退出码,也没有 kitty pop);本 PR 走完 runExitCleanup() 后干净地以 129 退出。

另外验证:修复在打包产物中依然有效

process.removeListener('uncaughtException', handleUncaughtException) 只有在 cli.tsgemini.tsx 解析到同一个函数对象时才有效。在逐文件的 tsc 产物里这显然成立,但在实际发布的 esbuild bundle 里就不一定了——这两个文件会落到不同 chunk,一旦被复制成两份,就会有两个监听器,先注册的基础处理器胜出,整个修复在发布产物中会静默失效

已核查:handleUncaughtException 在整个 bundle 中只定义了一次(dist/chunks/chunk-6KMALBFR.js),gemini 那个 chunk 导入的正是这个绑定。对 bundle 实跑一次崩溃:监听器数量 1,堆栈显示在主屏,调试记录已写入 —— 与逐文件构建表现一致。

新的后续问题(不阻塞合并):即使本进程从未进入备用屏,也会发出 ?1049l

处理器里退出备用屏的动作只由 process.stdout.isTTY 把关,而这个条件在很多从不进入备用屏的模式下都为真:-p--acpserve、读屏模式,以及 ui.useTerminalBuffer: false。在普通 shell 里这是无害的——我验证过两个 arm 之前的输出都完好。但如果外层程序占用着备用屏并 shell out 去跑 qwen -p,此时发生未捕获异常就会把外层的屏幕拆掉:#{alternate_on} 从 1 变成 0,外层程序还在跑但界面已被破坏,崩溃文本被倾泻到用户的 shell 回滚区上。基线则会保持外层屏幕完好。

建议的修法——按「是不是我们进入的」来判断,而不是按 isTTY:例如在 startInteractiveUIalternateScreen: true 渲染时置一个模块级标志,处理器读这个标志。无论怎么改,本 PR 主攻的 VP 路径都不受影响。

本次未覆盖的部分

明确说明,避免高估覆盖范围:

  • **真实的 Kitty/Ghostty 终端。**harness 在真实 pty 上回应了能力查询,走的是同一条代码路径,也给出了逐字节的顺序证据,但它毕竟不是真实终端的标志位栈。
  • **Windows / macOS。**本轮只在 Linux 上跑。
  • **用户最初上报的崩溃本身。**本 PR 并没有声称修掉它——只是让它可见——我也没有复现底层故障。

静态检查

  • gemini.test.tsx + ErrorBoundary.test.tsx —— 66/66 通过
  • 全量 npm ci 构建(所有 package)与 npm run bundle —— 均通过

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

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

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its E2E suite did not run locally.

— qwen3.8-max-preview via Qwen Code /review

Comment thread packages/cli/src/gemini.tsx Outdated
gemini.tsx statically imported handleUncaughtException and
isExpectedPtyRaceError from cli.ts, the esbuild entry point. gemini.tsx
is only ever loaded lazily, so that import formed an entry<->lazy cycle;
with splitting enabled esbuild hoisted the entry body into a shared chunk
and left dist/cli.js as a re-export stub. The bootstrap guard at the
bottom of cli.ts then never ran and the bundled CLI exited 0 silently.

Move the helpers (and the private getErrnoCode) into a new leaf module,
utils/uncaught-exception-handler.ts, imported by both cli.ts and
gemini.tsx. cli.ts re-exports them so existing importers (cli.test.ts)
are unaffected, and the handler reuses the shared writeStderrLine from
stdioHelpers.ts. The uncaughtException behavior itself is unchanged.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 7/100 轮)。改动内容与我反驳保留之处如下:

Autofix review round — PR #8088

Addressed the Critical that was killing the entire bundled CLI. One non-Critical suggestion is deferred under critical-only mode (details below). No base conflict (--conflict false), so no merge was performed.

Feedback triage

# Source Kind Decision
rc:3688709417 @qwen-code-ci-bot Critical Fixed — resolved in code
rc:3688233617 @doudouOUC Suggestion Deferred (critical-only mode) — thread left open with a reply
issue comment @wenshao Root-cause analysis Confirms the Critical; addressed by the fix below
issue comment @wenshao Round-2 validation ✅ + non-blocking ?1049l follow-up Validation acknowledged; the non-blocking follow-up is deferred (out of scope for this round)
failed check Test (ubuntu-latest Node 22.x) CI failure Root cause is the Critical below; fixed and statically verified

Critical fixed: entry↔lazy-module cycle dead-bundled the CLI (rc:3688709417)

gemini.tsx statically imported handleUncaughtException and isExpectedPtyRaceError from ./cli.js. cli.ts is the esbuild entry point, while gemini.tsx is only ever reached lazily via await import('./gemini.js'). That back-import formed an entry↔lazy cycle; with splitting: true, esbuild hoisted the entry body into a shared chunk and left dist/cli.js as a bare re-export stub. The bootstrap guard at the bottom of cli.ts (import.meta.url === pathToFileURL(process.argv[1]).href) then evaluated inside the chunk, never matched, and runCliEntryPoint() was never called — the bundled CLI started, did nothing, and exited 0. This is the daemon exited with 0 before listening smoke-test failure; tsc, eslint and every src-based unit test stayed green because they run against src/.

Fix (matches the reviewer's recommended approach):

  • Added a leaf module packages/cli/src/utils/uncaught-exception-handler.ts holding getErrnoCode, isExpectedPtyRaceError, and handleUncaughtException verbatim. It imports only writeStderrLine from the existing leaf ./utils/stdioHelpers.js (byte-identical semantics to the old private helper in cli.ts), so it introduces no cycle and no duplicated helper.
  • gemini.tsx now imports the two helpers from ./utils/uncaught-exception-handler.js instead of ./cli.js, breaking the cycle.
  • cli.ts imports the helpers from the leaf module (it still registers handleUncaughtException via process.on('uncaughtException', …)) and re-exports them, with a comment explaining why gemini.tsx must not import this entry file. The re-export keeps cli.test.ts working unchanged. The private writeStderrLine in cli.ts is retained for its remaining callers (handleCriticalError, runCliEntryPoint).

The uncaughtException behavior this PR adds is unchanged — only where the helpers live moved.

Suggestion deferred: unit tests for setupUncaughtExceptionHandler (rc:3688233617)

This is a genuine, valuable suggestion (the six listed test cases for the PTY-race suppression, synchronous debug-log write, alternate-screen escape guard, process.exit(1), and handler-replacement branches). It is non-Critical, and this PR has completed five change-producing rounds, so under critical-only mode (AGENTS.md: "land only Critical fixes … and defer remaining Suggestions to a follow-up") it is deferred rather than added here, to keep this round focused on the bundle-breaking Critical. The thread is left open with a reply recording the deferral so it is not silently dropped; the test cases are noted for a follow-up.

Non-blocking follow-up acknowledged (@wenshao, round 2)

The round-2 real-environment validation is acknowledged (still good to merge; mkdirSync follow-up verified). The new non-blocking observation — ?1049l is gated on process.stdout.isTTY rather than on whether this process actually entered the alternate screen, so an outer program's alternate screen can be torn down by a crash in qwen -p — is real but explicitly non-blocking and does not affect the VP path this PR targets. It is deferred to a follow-up (e.g. a module-level "we entered the alternate screen" flag set by startInteractiveUI), out of scope for this critical-only round.

Conflict notes

None. --conflict false; origin/main was not merged.

Verification

Commands actually run this round (repo root unless noted):

  • npm run buildpassed (all packages compiled).
  • npm run typecheckpassed (tsc --noEmit clean).
  • npm run lintpassed (eslint . --ext .ts,.tsx && eslint integration-tests, clean; the new kebab-case filename passes the filename-naming rule).
  • npx vitest run src/cli.test.ts src/gemini.test.tsx (in packages/cli) — 111 passed (cli.test.ts 53, gemini.test.tsx 58). cli.test.ts still imports isExpectedPtyRaceError from ./cli.js and passes via the re-export.
  • npm run bundlepassed; dist/cli.js is 12,470 bytes, a real entry again versus the ~629–719 byte dead re-export stub measured on the broken head.
  • Static bundle verification (surrogate for the CI smoke test that only executes dist/cli.js; the CLI was not run directly):
    • the bootstrap guard (argv[1]) is present in the dist/cli.js entry, so runCliEntryPoint() is reachable again;
    • handleUncaughtException is defined exactly once across the whole bundle (dist/chunks/chunk-QL4TN4HS.js);
    • the entry and the gemini chunk both import that same binding (import { handleUncaughtException, isExpectedPtyRaceError } from "./chunk-QL4TN4HS.js"), so process.removeListener('uncaughtException', handleUncaughtException) in gemini.tsx removes the exact function the entry registered — the single-listener consolidation holds in the shipped artifact (the invariant @wenshao flagged). The second occurrence of the PTY-race string in the bundle is core's unrelated shellExecutionService.ts, not a duplicate handler.

The workflow's independent CI (including the Test (ubuntu-latest Node 22.x) integration smoke test) remains the final verification gate.

中文说明

Autofix 评审轮次 — PR #8088

本轮修复了导致整个打包 CLI 失效的 Critical 问题。一条非 Critical 建议在仅处理 Critical 的模式下予以延后(详见下文)。无 base 冲突(--conflict false),因此未执行合并。

反馈分诊

# 来源 类型 决定
rc:3688709417 @qwen-code-ci-bot Critical 已修复 — 在代码中解决
rc:3688233617 @doudouOUC Suggestion 延后(仅处理 Critical 模式)— 线程保持开放并附回复
issue 评论 @wenshao 根因分析 印证了该 Critical;已由下方修复解决
issue 评论 @wenshao 第二轮验证 ✅ + 不阻塞的 ?1049l 后续问题 验证结论已知悉;不阻塞的后续问题予以延后(本轮范围之外)
失败检查 Test (ubuntu-latest Node 22.x) CI 失败 根因即下方 Critical;已修复并通过静态验证

已修复的 Critical:entry↔懒加载模块成环导致打包 CLI 失效(rc:3688709417)

gemini.tsx 此前从 ./cli.js 静态导入了 handleUncaughtExceptionisExpectedPtyRaceErrorcli.ts 是 esbuild 的 entry,而 gemini.tsx 只会经由 await import('./gemini.js') 懒加载到达。这条反向导入构成了 entry↔懒加载的环;在 splitting: true 下,esbuild 把 entry 的模块体搬进共享 chunk,dist/cli.js 只剩一个 re-export 空壳。于是 cli.ts 底部的 bootstrap 守卫(import.meta.url === pathToFileURL(process.argv[1]).href)在 chunk 内求值、永远不相等,runCliEntryPoint() 一次都不会被调用——打包后的 CLI 启动、什么都不做、exit 0。这正是冒烟测试 daemon exited with 0 before listening 的失败原因;tsc、eslint 和所有基于 src 的单测都是绿的,因为它们跑的是 src/

修法(与评审建议的方案一致):

  • 新增叶子模块 packages/cli/src/utils/uncaught-exception-handler.ts,原样收纳 getErrnoCodeisExpectedPtyRaceErrorhandleUncaughtException。它只从已有的叶子模块 ./utils/stdioHelpers.js 导入 writeStderrLine(与 cli.ts 旧的私有 helper 语义逐字节相同),因此既不引入环,也不产生重复的 helper。
  • gemini.tsx 改为从 ./utils/uncaught-exception-handler.js 导入这两个 helper,不再从 ./cli.js 导入,从而打破环。
  • cli.ts 从该叶子模块导入这两个 helper(它仍然通过 process.on('uncaughtException', …) 注册 handleUncaughtException),并将它们 re-export 出去,同时附注释说明为什么 gemini.tsx 不能导入这个 entry 文件。该 re-export 让 cli.test.ts 无需改动即可继续工作。cli.ts 中的私有 writeStderrLine 保留,供其余调用方(handleCriticalErrorrunCliEntryPoint)使用。

本 PR 新增的 uncaughtException 行为完全不变——变的只是这些 helper 放在哪儿。

延后的建议:为 setupUncaughtExceptionHandler 补单测(rc:3688233617)

这是一条真实且有价值的建议(针对 PTY 竞态抑制、同步写调试日志、备用屏转义守卫、process.exit(1) 以及处理器替换分支所列的六个测试用例)。它属于非 Critical,而本 PR 已经完成五个产生改动的轮次,因此在仅处理 Critical 的模式下(AGENTS.md:「只落地 Critical 修复……其余 Suggestion 延后到后续处理」),本轮不予新增,以便专注于打断打包的 Critical。线程保持开放并附回复记录此次延后,避免被静默丢弃;这些测试用例已记录,留待后续补齐。

已知悉的不阻塞后续问题(@wenshao,第二轮)

第二轮真实环境验证结论已知悉(仍可合并;mkdirSync 后续问题已验证)。新提出的不阻塞观察——?1049l 只由 process.stdout.isTTY 把关、而非按本进程是否真的进入过备用屏来判断,因此 qwen -p 内的崩溃可能会拆掉外层程序的备用屏——是真实存在的,但明确不阻塞合并,也不影响本 PR 主攻的 VP 路径。予以延后到后续处理(例如由 startInteractiveUI 置一个模块级「我们已进入备用屏」标志),不在本轮仅处理 Critical 的范围内。

冲突说明

无。--conflict false;未合并 origin/main

验证

本轮实际运行的命令(除非特别说明,均在仓库根目录):

  • npm run build通过(所有 package 编译成功)。
  • npm run typecheck通过tsc --noEmit 干净)。
  • npm run lint通过eslint . --ext .ts,.tsx && eslint integration-tests,干净;新的 kebab-case 文件名通过文件命名规则)。
  • npx vitest run src/cli.test.ts src/gemini.test.tsx(在 packages/cli 内)— 111 通过(cli.test.ts 53 个,gemini.test.tsx 58 个)。cli.test.ts 仍从 ./cli.js 导入 isExpectedPtyRaceError,经由 re-export 通过。
  • npm run bundle通过dist/cli.js12,470 字节,重新成为一个真正的 entry,而在损坏的 head 上测得的是约 629–719 字节的 re-export 空壳。
  • 静态 bundle 验证(作为只会执行 dist/cli.js 的 CI 冒烟测试的替代手段;未直接运行 CLI):
    • bootstrap 守卫(argv[1])存在于 dist/cli.js entry 中,因此 runCliEntryPoint() 重新可达;
    • handleUncaughtException 在整个 bundle 中恰好定义一次dist/chunks/chunk-QL4TN4HS.js);
    • entry gemini chunk 都导入同一个绑定(import { handleUncaughtException, isExpectedPtyRaceError } from "./chunk-QL4TN4HS.js"),因此 gemini.tsx 中的 process.removeListener('uncaughtException', handleUncaughtException) 移除的正是 entry 注册的那个函数——单一监听器的合并结论在发布产物中成立(即 @wenshao 指出的不变量)。bundle 中 PTY 竞态字符串的第二处出现是 core 里无关的 shellExecutionService.ts,并非重复的 handler。

工作流独立的 CI(包括 Test (ubuntu-latest Node 22.x) 集成冒烟测试)仍是最终的验证门禁。

Deferred non-Critical feedback

Critical-only mode is active after 5 change-producing rounds: the automated reviewer's non-Critical suggestions below are deferred and stay open for human follow-up — do not modify code, resolve threads, or reply on their behalf. Maintainer feedback defers only once its author has already had 2 regular feedback batches addressed in this window's Critical-only tail — an account can host an automated reviewer loop, so the brake keys on measured regeneration, not identity; authors at their budget, if any, are named below. (A maintainer can lift the mode itself: @qwen-code /retry starts a fresh counting window.)

中文说明

完成 5 个产生改动的轮次后进入仅处理 Critical 的模式:以上为自动评审的非 Critical 建议,予以延后、保持开放并留待人工跟进——不要为其修改代码、解决线程或代为回复。维护者的反馈仅在其本人于本窗口 Critical-only 阶段已被处理 2 批常规反馈之后才会被延后——账号可能挂着自动评审循环,因此刹车依据实测的再生频度而非身份;达到预算的作者(如有)在下方点名。(如需解除该模式,评论 @qwen-code /retry 即可开启新的计数窗口。)

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@wenshao

wenshao commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@wenshao
wenshao enabled auto-merge July 31, 2026 07:53
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

⚠️ The bot has no review of its own on f71d37d3a15557ced877f822528226f562a098d2. If this re-run was meant to approve, it did not — an approval left by another account is a separate vote and does not count as the bot's own.

The stage comments above were updated with the latest result. View workflow run.

@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 — CI landed green after the review. ✅

@wenshao
wenshao added this pull request to the merge queue Jul 31, 2026
Merged via the queue into QwenLM:main with commit 90052f2 Jul 31, 2026
66 of 67 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.3.

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

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants