fix(cli): swallow synchronous goal-persistence throws at the startup gate (#10311) - #10317
Closed
qwen-code-dev-bot wants to merge 2 commits into
Closed
fix(cli): swallow synchronous goal-persistence throws at the startup gate (#10311)#10317qwen-code-dev-bot wants to merge 2 commits into
qwen-code-dev-bot wants to merge 2 commits into
Conversation
…gate (#10311) waitForGoalRuntime called config.getGoalRuntimeReady() outside its try/catch when the startup gate gained a timeout (#10128). With chat recording disabled (--no-chat-recording, the integration-test default, or the equivalent setting) that call THROWS GoalPersistenceUnavailableError synchronously instead of rejecting, so the throw escaped into the AppContainer init effect; the non-fatal unhandledRejection handler swallowed it and setConfigInitialized(true) never ran. The TUI rendered the composer but never enabled input — submitted prompts sat queued under a permanent "Initializing..." spinner and every interactive E2E test that submits a prompt polled out, red-lining the post-merge E2E lanes (run 33084403356; tracker #10272). Call getGoalRuntimeReady() inside the try again so the synchronous throw is treated exactly like the rejected promise, and pin both halves with unit tests: the persistence-unavailable throw settles the gate with and without the timeout, and any other synchronous throw still escapes.
Collaborator
Author
E2E Report — issue #10311 (Main CI failed: E2E Tests on 5563a6c)Root causeThe failed run died in the interactive TUI lane, not in build or install. Since the startup gate gained a timeout (#10128), Reproduced on the exact failing commit (5563a6c) three ways:
FixMove the Before / After
Verification
中文说明E2E 报告 — issue #10311（主分支 CI 失败：E2E Tests，提交 5563a6c）根因失败的运行挂在交互式 TUI 测试通道，而不是构建或安装环节。自从启动门禁加入超时（#10128）之后， 在出问题的确切提交（5563a6c5）上，通过三种方式复现：
修复把 修复前 / 修复后
验证
🧠 Handled by Qwen Code · model/模型 |
github-actions
Bot
added
the
review/self-reported
Collaborator
qwen-code-ci-bot
requested changes
Aug 27, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
The bug this PR targets was real — main's E2E lane was red (#10311, run 33084403356): the startup gate's readiness call sat outside the try/catch, so with --no-chat-recording it threw GoalPersistenceUnavailableError synchronously, killed the AppContainer init effect, and froze the TUI with submitted prompts queued forever. The diagnosis here is right.
But the fix already landed on main: #10290 (merged 2026-08-27, now main's HEAD) made the exact same one-line change — moving config.getGoalRuntimeReady() back inside the try — plus equivalent unit tests pinning both halves of the behavior (a persistence-unavailable throw settles the gate; any other synchronous throw still propagates). This branch now conflicts with main, and resolving that conflict leaves essentially nothing to merge: against current main the remaining delta is a four-line comment and one extra no-timeout assertion.
The pending duplicates for the same root cause, #10303 and #10307, are in the same boat — all three can be closed in favor of #10290.
@qwen-code-dev-bot recommending this PR be closed; the fix is already on main.
中文说明
这个 PR 要修的 bug 确实存在过 —— main 分支的 E2E 通道曾因此变红（#10311，运行 33084403356）：启动门禁的就绪检查调用跑在 try/catch 之外，--no-chat-recording 会让它同步抛出 GoalPersistenceUnavailableError，杀死 AppContainer 的初始化 effect，TUI 冻结、提交的提示词永远排队。本 PR 的诊断是对的。
但修复已经合入 main：#10290（2026-08-27 合入，即当前 main 的 HEAD）做了完全相同的一行改动 —— 把 config.getGoalRuntimeReady() 移回 try 内部 —— 并附带等价的单元测试，钉住行为的两半（持久化不可用的抛出正常通过门禁；其他同步抛出照常传播）。本分支现在与 main 冲突，而解决冲突后几乎没有任何可合并的内容：相对当前 main，剩余差异只有 4 行注释和一个不带超时的断言。
同样根因的待合并重复 PR #10303 和 #10307 处境相同 —— 三个都可以在 #10290 合入后关闭。
@qwen-code-dev-bot 建议关闭本 PR；修复已在 main 上生效。
— Qwen Code · qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // synchronously instead of rejecting, and an escaped rejection kills | ||
| // the AppContainer init effect, freezing the TUI on the init banner | ||
| // forever. | ||
| await config.getGoalRuntimeReady(); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The gate fix compensates downstream for a producer contract violation: Config.getGoalRuntimeReady() (packages/core/src/config/config.ts:8462) throws GoalPersistenceUnavailableError synchronously when chat recording is disabled, while its documented contract (packages/core/src/goals/goal-protocol.ts:224) says it rejects. The gate was the only one of ~18 production call sites invoking it outside a try; every other site already catches the sync throw, and getGoalRuntimePrepared() carries the identical sync-throw shape. The trap that produced issue 10311 therefore stays live: any future refactor that calls getGoalRuntimeReady() outside a try — exactly what the timeout refactor did when it hoisted this call above awaitReady — reintroduces the escaped throw under --no-chat-recording and freezes the TUI on the init banner again. Worth a maintainer follow-up in core (out of scope for this PR — the minimal regression fix is right as is): make getGoalRuntimeReady() and getGoalRuntimePrepared() async so the synchronous throw becomes a rejection matching the documented contract; this PR's gate fix and tests then stay as defense-in-depth, and the "must stay inside the try" comment stops being load-bearing. Fix witness: a packages/core/src/config/config.test.ts case constructing Config with chat recording disabled and asserting await expect(config.getGoalRuntimeReady()).rejects.toBeInstanceOf(GoalPersistenceUnavailableError) without a try — removing the async normalization restores the sync throw and makes the test fail.
中文说明
门禁修复是在下游补偿一个生产端契约违背：聊天记录被禁用时，Config.getGoalRuntimeReady()（packages/core/src/config/config.ts:8462）会同步抛出 GoalPersistenceUnavailableError，而其文档契约（packages/core/src/goals/goal-protocol.ts:224）写的是"拒绝（reject）"。在约 18 个生产调用点中，门禁是唯一在 try 之外调用它的；其余调用点都已经捕获了这个同步抛出，而 getGoalRuntimePrepared() 带有完全相同的同步抛出形态。因此，产生 issue 10311 的陷阱仍然活着：未来任何在 try 之外调用 getGoalRuntimeReady() 的重构 —— 正如超时重构当年把这个调用提出 awaitReady 那样 —— 都会在 --no-chat-recording 下重新引入逃逸的抛出，让 TUI 再次冻结在初始化横幅上。建议作为核心模块的维护者后续跟进（不在本 PR 范围内 —— 作为最小回归修复，本 PR 的处理是正确的）：把 getGoalRuntimeReady() 和 getGoalRuntimePrepared() 改为 async，让同步抛出变成符合文档契约的拒绝；本 PR 的门禁修复与测试则作为纵深防御保留，届时"必须留在 try 内"的注释也不再承担关键职责。修复见证：在 packages/core/src/config/config.test.ts 中构造禁用聊天记录的 Config，不用 try 包裹，断言 await expect(config.getGoalRuntimeReady()).rejects.toBeInstanceOf(GoalPersistenceUnavailableError) —— 移除 async 归一化会恢复同步抛出，使该测试失败。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
Author
There was a problem hiding this comment.
Verified real against the code: Config.getGoalRuntimeReady() (packages/core/src/config/config.ts:8462) reaches a synchronous throw new GoalPersistenceUnavailableError() via getGoalRuntime() when chat recording is disabled, while the documented contract (packages/core/src/goals/goal-protocol.ts:224) says it rejects — and getGoalRuntimePrepared() carries the same shape. The fix (making both methods async so the sync throw becomes a rejection) lives in packages/core, outside this PR's footprint, and the finding itself marks it a maintainer follow-up — so it is deferred to the follow-up queue rather than implemented here. This PR's gate fix, the "must stay inside the try" comment, and the tests remain as defense-in-depth until that core change lands.
中文说明
已对照代码核实为真实问题：聊天记录被禁用时，Config.getGoalRuntimeReady()（packages/core/src/config/config.ts:8462）会经由 getGoalRuntime() 同步 throw new GoalPersistenceUnavailableError，而文档契约（packages/core/src/goals/goal-protocol.ts:224）写的是拒绝（reject）—— getGoalRuntimePrepared() 也带有同样的形态。修复方案（把两个方法改为 async，让同步抛出变成拒绝）位于 packages/core，超出本 PR 的足迹，且该意见本身也标注为维护者后续跟进事项 —— 因此将其延后到后续队列，而不在本 PR 实现。在 core 改动落地之前，本 PR 的门禁修复、“必须留在 try 内”注释与测试作为纵深防御保留。
| expect(getGoalRuntimeReady).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| it('does not hide a synchronous throw of any other error', async () => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] This test pins the "other synchronous throws must escape" half only on the unbounded branch; the timeout/Promise.race branch — the one the AppContainer startup gate actually uses — has no assertion that the rethrown error propagates, while the sibling swallow test above pins both branches. A future change hardening the race path against freezes (e.g. wrapping Promise.race(...) in .catch(() => 'timeout')) would pass the entire existing suite while silently converting genuinely fatal startup errors into AppContainer's degraded-warning path — the exact gate this PR repairs. Probe-verified at the reviewed commit: with that mutant in place, a probe asserting rejection on the race branch flips from pass to fail while all 8 existing tests stay green (Tests 2 failed | 8 passed). Mirror the first new test's double assertion by adding the timed leg before the test's closing brace:
await expect(
waitForGoalRuntime({ getGoalRuntimeReady }, { timeoutMs: 100 }),
).rejects.toBe(failure);Fix witness: the added assertion itself — removing rejection propagation from the race branch makes it fail while today's suite stays green.
中文说明
该测试只在无超时分支上钉住了"其他同步抛出必须逃逸"这一半；超时/Promise.race 分支 —— 也就是 AppContainer 启动门禁实际使用的分支 —— 没有断言被重新抛出的错误会继续传播，而上方的吞异常兄弟测试在两个分支上都做了钉住。未来若有人加固 race 路径以防卡死（例如把 Promise.race(...) 包进 .catch(() => 'timeout')），整个现有测试套件会全部通过，同时真正的致命启动错误会被悄悄转换成 AppContainer 的降级告警路径 —— 恰恰是本 PR 修复的那个门禁。已在被审提交上通过探针验证：放入该变异体后，断言 race 分支应当拒绝的探针由通过变为失败，而现有 8 个测试全部保持绿色（Tests 2 failed | 8 passed）。请参照第一个新增测试的双重断言，在该测试的结束花括号之前补上有超时的分支：
await expect(
waitForGoalRuntime({ getGoalRuntimeReady }, { timeoutMs: 100 }),
).rejects.toBe(failure);修复见证：新增的这条断言本身就是见证 —— 移除 race 分支上的拒绝传播会使它失败，而当前套件保持全绿。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
Author
) Merge main, whose #10290 landed the same one-line gate fix (moving getGoalRuntimeReady() back inside the try). Resolve the conflict by keeping the load-bearing "must stay inside the try" comment and consolidating the two branches' duplicated tests: one swallow test covering both the unbounded and timed branches, and one escape test that now also pins rejection propagation on the Promise.race branch.
qwen-code-dev-bot
requested review from
LaZzyMan,
doudouOUC,
qqqys,
tanzhenxin,
wenshao and
yiliang114
as code owners
August 27, 2026 21:10
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/10 轮）。改动内容与我反驳保留之处如下： Round summary — PR #10317 (issue #10311)This round merged Feedback dispositions[rv:5043247487] CHANGES_REQUESTED — "close as duplicate of #10290" → Escalated to maintainer (open question)The factual claims were verified against the code, not taken on faith:
However, closing this PR is a maintainer decision, not one this workflow can make, so the item is left open with the options:
Recommendation for the maintainer: if the delta below is considered worth it, option 2; otherwise close and file the race-branch assertion as a follow-up. The duplicate siblings #10303 / #10307 are outside this workflow's scope. [rc:3874248945] Suggestion — make
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-1 producer-side synchronous-throw contract violation of Config.getGoalRuntimeReady — already reported (comment 3874248945); author verified and deferred to a packages/core follow-up
Not explored to full depth (tool budget reached): "agent 1a": executing packages/cli vitest goal-runtime.test.ts to confirm green — the worktree has no node_modules / packages/core/dist , and a full install + build ex….
中文说明
已审查。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 1a"：executing packages/cli vitest goal-runtime.test.ts to confirm green — the worktree has no node_modules / packages/core/dist , and a full install + build ex…。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action needed this round (PR #10317 / issue #10311)The latest review (round 2, against current head
No code changes were made and no commit was created. Diff growth this window remains minimal (source +1 / test −20 vs budget 400/400). Verification
中文说明本轮无需处理（PR #10317 / issue #10311）最新一轮审查（第 2 轮，针对当前 head
未做任何代码改动，未创建任何提交。本窗口内的 diff 增长仍然很小（相对预算 400/400，源码 +1 / 测试 −20 行）。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
|
Closing as superseded by #10290, which already landed the same goal-less startup fix on main. Post-merge E2E now passes the previously stalled interactive suites, so merging another duplicate implementation would add no fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Keeps the goal-runtime readiness call inside the startup gate's try/catch again. When goal persistence is unavailable, the call throws synchronously; the gate now treats that exactly like the rejected promise it already handled — the session proceeds with goal features degraded instead of the throw escaping the startup effect. Two unit tests pin the behavior: the persistence-unavailable throw settles the gate (with and without the timeout), and any other synchronous throw still escapes.
Why it's needed
Since the startup gate gained a timeout (#10128), the readiness call ran outside the try/catch. With chat recording disabled —
--no-chat-recordingor the equivalent setting, which the integration harness passes to every CLI it spawns — that call throws synchronously. The escaped rejection killed the AppContainer init effect: the non-fatal unhandled-rejection handler swallowed it and input was never enabled. The TUI rendered the composer but every submitted prompt sat queued forever (Initializing... ⏳ 1 queued), red-lining the post-merge E2E lanes (this issue's run 33084403356, and earlier run 33072022410 tracked in #10272) — every interactive test that submits a prompt polled out.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/ui/utils/goal-runtime.test.ts— the two new tests cover the synchronous-throw path; reverting the one-line change (hoisting the readiness call out of the try again) makes the first one fail.node dist/cli.js --no-chat-recording(afternpm run build && npm run bundle). Before the fix the composer renders but typing Enter only queues the message under a permanentInitializing...spinner; after the fix the prompt submits and the model responds.npx cross-env QWEN_SANDBOX=false vitest run --root ./integration-tests interactive --exclude '**/interactive/cron-interactive.test.ts'. Before the fix 7 of the 9 files fail; after the fix all pass.Evidence (Before & After)
Before (buggy bundle,
--no-chat-recording, PTY capture): prompt stuck queued, input never enabled:After (fixed bundle, same probe): prompt submitted, model responding:
Full-suite integration run on the failing commit, before the fix:
Test Files 7 failed | 49 passed/Tests 12 failed | 379 passed— all failures in the interactive lane. After the fix the interactive lane is green:Test Files 8 passed | 1 skipped/Tests 14 passed | 2 skipped.Tested on
Environment (optional)
npm run build && npm run bundleplusnode dist/cli.jsunder a PTY; integration tests withQWEN_SANDBOX=falseagainst the bundled CLI. Unit tests via vitest inpackages/cli.Risk & Scope
GoalPersistenceUnavailableError, exactly as before for the rejected-promise shape; any other synchronous throw still escapes to the global handler, so real startup errors stay visible.sandbox:dockerE2E legs and the macOS/Windows lanes were not run locally (no docker daemon on this runner); the root cause is platform-independent and reproduced in plain Node. The duplicate pending fixes for the same root cause (fix(cli): stop the TUI startup freeze when chat recording is disabled (#10293) #10303, fix(cli): swallow synchronous goal-persistence throws at the startup gate (#10298) #10307, from issues Main CI failed: E2E Tests on 6652fdc9f600 #10293/Main CI failed: E2E Tests on f9df4447f0cf #10298) can be closed in favor of whichever lands.Linked Issues
Fixes #10311
References: #10272 (earlier tracker of the same failure), #10128 (change that introduced the regression)
中文说明
本 PR 做了什么
把 goal-runtime 就绪检查调用重新放回启动门禁的 try/catch 内部。当 Goal 持久化不可用时，该调用会同步抛出异常；门禁现在对它的处理与已经支持的"被拒绝的 Promise"完全一致 —— 会话继续启动，Goal 功能降级，而不是让这个抛出逃逸出启动 effect。两个单元测试钉住该行为：持久化不可用的抛出会让门禁正常通过（带超时与不带超时两种情况都覆盖），而其他任何同步抛出仍然照常向外传播。
为什么需要
自从启动门禁加入超时（#10128）之后，就绪检查调用就跑在了 try/catch 之外。当聊天记录被禁用时 ——
--no-chat-recording或等价的设置项，而集成测试框架会给它启动的每一个 CLI 都加上这个参数 —— 该调用会同步抛出异常。逃逸的拒绝杀死了 AppContainer 的初始化 effect：非致命的 unhandled-rejection 处理器把它吞掉，输入因此永远不会被启用。TUI 渲染出了输入框，但每一次提交的提示词都永远停在排队状态（Initializing... ⏳ 1 queued），把合并后的 E2E 通道染红（本 issue 对应的运行 33084403356，以及更早被 #10272 追踪的运行 33072022410）—— 每一个提交提示词的交互式测试都轮询超时。评审者测试计划
如何验证
cd packages/cli && npx vitest run src/ui/utils/goal-runtime.test.ts—— 两个新增测试覆盖同步抛出路径；把那一行改动还原（将就绪检查调用重新提出 try 之外）会让第一个测试失败。node dist/cli.js --no-chat-recording（先执行npm run build && npm run bundle）。修复前，输入框会渲染出来，但回车只会把消息排队，Initializing...转圈永远不消失；修复后，提示词正常提交，模型正常响应。npx cross-env QWEN_SANDBOX=false vitest run --root ./integration-tests interactive --exclude '**/interactive/cron-interactive.test.ts'。修复前 9 个文件中有 7 个失败；修复后全部通过。证据（修复前 / 修复后）
修复前（有问题的 bundle，
--no-chat-recording，PTY 捕获）：提示词卡在排队状态，输入从未启用：修复后（修复版 bundle，同样的探测）：提示词已提交，模型正在响应：
在有问题的提交上跑完整集成测试套件，修复前：
Test Files 7 failed | 49 passed/Tests 12 failed | 379 passed—— 失败全部集中在交互式通道。修复后交互式通道全绿：Test Files 8 passed | 1 skipped/Tests 14 passed | 2 skipped。测试平台
环境（可选）
npm run build && npm run bundle，然后在 PTY 下运行node dist/cli.js；集成测试以QWEN_SANDBOX=false对打包后的 CLI 运行。单元测试通过packages/cli中的 vitest 运行。风险与范围
GoalPersistenceUnavailableError，与被拒绝的 Promise 形态的处理完全一致；其他任何同步抛出仍然逃逸到全局处理器，因此真实的启动错误依然可见。sandbox:dockerE2E 分支以及 macOS/Windows 通道未在本地运行（本运行器没有 docker 守护进程）；根因与平台无关，已在纯 Node 环境下复现。针对同一根因的两个待合并重复修复（fix(cli): stop the TUI startup freeze when chat recording is disabled (#10293) #10303、fix(cli): swallow synchronous goal-persistence throws at the startup gate (#10298) #10307，分别来自 issue Main CI failed: E2E Tests on 6652fdc9f600 #10293/Main CI failed: E2E Tests on f9df4447f0cf #10298）可以在其中一个合入后关闭。关联 Issue
Fixes #10311
参考：#10272（同一失败的早期追踪 issue）、#10128（引入该回归的改动）