refactor(cli): drop the legacy Goal Stop-hook plumbing

[qqqys]

What this PR does

The CLI stops calling the first-generation Goal Stop-hook implementation. restoreGoal.ts loses the seven exports that drove it — the history restorer that registered the hook, the terminal-event observer and recorder, the two blocked-reason gates, and the terminal-goal finder — none of which had a single production caller. What stays is the part three live call sites actually use: the transcript scanner that finds a Goal worth offering to restore, and the card parsing around it.

The ACP message emitter stops attaching a goal sub-object to _meta.stopHookLoop. That object could only ever be populated from the legacy in-memory store, which nothing writes in a real session. One consumer did read it — the Web Shell's session provider — but that branch returned null unconditionally, deliberately suppressing per-iteration "checking" events, so it produced no UI. The branch goes with it and the explanation stays as a comment.

The rest is dead test scaffolding: mocks of legacy core exports in five suites, and thirty-two mocks of a Session.installGoalTerminalObserver method that does not exist on Session in this revision.

The legacy core modules themselves are untouched. This PR removes the CLI's references so they can be deleted next.

Why it's needed

Goal v3 owns every surface. The legacy generation is not a second live path — registerGoalHook's only production caller was restoreGoalFromHistory in this file, and that function had no callers of its own — so what remained was cost: a 387-line module where two thirds was unreachable, a wire field nothing could populate, and test files that mocked functions the code under test never calls, which is the kind of scaffolding that makes a reader think a path is live when it is not.

Together with the core-side sibling, this is what makes deleting goalHook.ts, goalJudge.ts and activeGoalStore.ts a mechanical follow-up rather than a large-scope refactor.

Reviewer Test Plan

How to verify

The CLI's vitest config aliases @qwen-code/qwen-code-core straight to source, so no build is needed:

cd packages/cli && npx vitest run src/ui/utils/restoreGoal.test.ts
cd packages/cli && npx vitest run src/ui/commands/goalCommand.test.ts
cd packages/cli && npx vitest run src/nonInteractiveCliCommands.test.ts
cd packages/cli && npx vitest run src/acp-integration/session/emitters
cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts
cd packages/cli && npx vitest run src/ui/hooks/use-llm-stream.test.tsx

Run them one file per invocation on a small host; a single combined invocation gets OOM-killed here.

The claim worth checking is that the retained half of restoreGoal.ts is exactly what its three importers need. acpAgent.ts, session/Session.ts and session/recovered-goal-update.ts import only findGoalToRestore, collectGoalStatusItemsFromRecords, parseGoalStatusItem, isTranscriptItemRecord and the two types — all kept.

Two tests were retargeted rather than deleted, and those are the ones to read: the 10,000-character objective pin from #6665 now asserts against the live transcript scanner instead of the deleted restorer, and the start-time cases assert the setAt the scanner returns instead of the default clock the deleted hook registration applied.

Evidence (Before & After)

Non–user-visible refactor: N/A for screenshots. The evidence is that no legacy symbol is reachable from the CLI, and every suite still passes:

$ git grep -nE "registerGoalHook|unregisterGoalHook|getActiveGoal|setActiveGoal|clearActiveGoal|activeGoalEquals|getLastGoalTerminal|setLastGoalTerminal|setGoalTerminalObserver|__resetActiveGoalStoreForTests|GoalTerminalEvent|GoalTerminalKind" -- packages/cli/src
(no matches)

restoreGoal.test.ts                                    34 passed
goalCommand.test.ts                                    65 passed
nonInteractiveCliCommands.test.ts                      52 passed
acp-integration/session/emitters (3 files)            115 passed
acpAgent.test.ts                                      622 passed
use-llm-stream.test.tsx                               279 passed
acpAgent.worktree.test.ts + history-replayer.test.ts   55 passed

prettier --check and eslint are clean on all ten changed files.

Tested on

OS	Status
🍏 macOS	⚠️
🪟 Windows	⚠️
🐧 Linux	✅

Environment (optional)

Linux, package-local vitest for packages/cli, one test file per invocation.

Risk & Scope

• Main risk or tradeoff: one wire field goes away, _meta.stopHookLoop.goal. Its only reader was a Web Shell branch that returned null on every path, so removing both is byte-identical in behaviour, but it is a wire surface and worth a reviewer's eye. Everything else removed was unreachable: seven exports with no production caller, and mocks of functions the code under test never calls.
• Not validated / out of scope: the legacy core modules still compile and keep their own suites; deleting them is the next PR. projectLegacyActiveGoal, the headless active_goal event, _meta.goalStatus, _meta.goalTerminal, BridgeSessionGoal.active and the goal_status history card are all deliberately untouched. packages/cli/tsconfig.json reports type errors in seven files this PR never touches (activeWorkState, ChannelBaseOptions.locale, agentsRespawned) — those come from unbuilt sibling package dists on this host, and a worktree with a complete npm run build type-checks the CLI with zero errors.
• Breaking changes / migration notes: none beyond the stopHookLoop.goal field above. No settings, persisted records, or user-visible behaviour change.

One follow-up found while doing this and left out on purpose: session/Session.ts still imports and calls core's abortGoalForStopHookCap. Its own comment explains the legacy store has no writer for daemon sessions, so the call always returns false and the canonical-runtime branch always runs. Dropping that guard is behaviour-bearing enough to deserve its own commit.

Linked Issues

Part of #10795. Part of #4228.

中文说明

这个 PR 做了什么

CLI 不再调用第一代 Goal Stop-hook 实现。restoreGoal.ts 去掉了驱动它的七个导出——注册该 hook 的历史恢复器、终态事件观察者与记录器、两个"被阻止原因"的判断门，以及终态 Goal 查找器——它们在生产代码里一个调用者都没有。留下的是三处存活调用点真正在用的部分：从 transcript 里找出值得提议恢复的 Goal 的扫描器，以及围绕它的卡片解析。

ACP 的消息发射器不再往 _meta.stopHookLoop 上挂 goal 子对象。那个对象只可能由内存里的 legacy store 填充，而真实会话里没有任何东西写它。确实有一个消费者读过它——Web Shell 的 session provider——但那个分支在所有路径上都无条件 return null，刻意抑制逐轮的"checking"事件，所以它不产生任何 UI。该分支随之移除，解释以注释形式保留。

其余是死掉的测试脚手架：五个测试套里对 legacy core 导出的 mock，以及三十二处对 Session.installGoalTerminalObserver 的 mock——这个方法在当前版本的 Session 上并不存在。

legacy core 模块本身未被触及。本 PR 移除 CLI 侧的引用，以便下一个 PR 删除它们。

为什么需要

Goal v3 拥有所有界面。legacy 那一代并不是第二条存活路径——registerGoalHook 在生产代码里唯一的调用者就是本文件里的 restoreGoalFromHistory，而它自己没有任何调用者——所以剩下的只有代价：一个 387 行的模块里三分之二不可达、一个没有东西能填充的线协议字段，以及一批 mock 了被测代码从不调用的函数的测试文件——而这类脚手架正是让读者误以为某条路径还活着的原因。

与 core 侧的姊妹 PR 合起来，这就是让删除 goalHook.ts、goalJudge.ts 与 activeGoalStore.ts 变成一次机械跟进、而不是一次大范围重构的前提。

评审验证方式

如何验证

CLI 的 vitest 配置把 @qwen-code/qwen-code-core 直接别名到源码，所以不需要构建：

cd packages/cli && npx vitest run src/ui/utils/restoreGoal.test.ts
cd packages/cli && npx vitest run src/ui/commands/goalCommand.test.ts
cd packages/cli && npx vitest run src/nonInteractiveCliCommands.test.ts
cd packages/cli && npx vitest run src/acp-integration/session/emitters
cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts
cd packages/cli && npx vitest run src/ui/hooks/use-llm-stream.test.tsx

在内存较小的机器上请一个文件一次调用；把它们合成一次调用会被 OOM 杀掉。

值得核对的论断是：restoreGoal.ts 保留下来的那一半，恰好就是它三个引入方需要的东西。acpAgent.ts、session/Session.ts 与 session/recovered-goal-update.ts 只引入 findGoalToRestore、collectGoalStatusItemsFromRecords、parseGoalStatusItem、isTranscriptItemRecord 与两个类型——全部保留。

有两个测试是被改指对象而不是删除，那两个值得读：来自 #6665 的一万字符目标长度固定断言，现在针对存活的 transcript 扫描器而不是被删掉的恢复器；起始时间那几个用例现在断言扫描器返回的 setAt，而不是被删掉的 hook 注册所应用的默认时钟。

证据（前后对比）

不面向用户的重构：截图部分为 N/A。证据是 CLI 侧已无法到达任何 legacy 符号，且每个测试套仍然通过：

$ git grep -nE "registerGoalHook|unregisterGoalHook|getActiveGoal|setActiveGoal|clearActiveGoal|activeGoalEquals|getLastGoalTerminal|setLastGoalTerminal|setGoalTerminalObserver|__resetActiveGoalStoreForTests|GoalTerminalEvent|GoalTerminalKind" -- packages/cli/src
(no matches)

restoreGoal.test.ts                                    34 passed
goalCommand.test.ts                                    65 passed
nonInteractiveCliCommands.test.ts                      52 passed
acp-integration/session/emitters（3 个文件）           115 passed
acpAgent.test.ts                                      622 passed
use-llm-stream.test.tsx                               279 passed
acpAgent.worktree.test.ts + history-replayer.test.ts   55 passed

十个改动文件上的 prettier --check 与 eslint 均干净。

测试环境

OS	Status
🍏 macOS	⚠️
🪟 Windows	⚠️
🐧 Linux	✅

运行环境（可选）

Linux，packages/cli 的包内 vitest，一个测试文件一次调用。

风险与范围

• 主要风险或权衡：有一个线协议字段消失了，_meta.stopHookLoop.goal。它唯一的读者是 Web Shell 里一个在所有路径上都 return null 的分支，所以两者一起移除在行为上逐字节相同；但它毕竟是线协议面，值得评审者过一眼。其余被移除的都是不可达的：七个没有生产调用者的导出，以及对被测代码从不调用的函数的 mock。
• 未验证 / 不在范围内：legacy core 模块仍在编译，仍保有它们自己的测试套；删除它们是下一个 PR。projectLegacyActiveGoal、headless 的 active_goal 事件、_meta.goalStatus、_meta.goalTerminal、BridgeSessionGoal.active 与 goal_status 历史卡片都刻意未动。packages/cli/tsconfig.json 在本 PR 从未触及的七个文件里报出类型错误（activeWorkState、ChannelBaseOptions.locale、agentsRespawned）——它们来自本机上未构建的兄弟包 dist；在一个完成了 npm run build 的 worktree 里，CLI 的类型检查零报错。
• 破坏性变更 / 迁移说明：除上述 stopHookLoop.goal 字段外没有。设置、持久化记录与用户可见行为均未改变。

顺手发现但刻意留在范围外的一项跟进：session/Session.ts 仍然引入并调用 core 的 abortGoalForStopHookCap。它自己的注释说明 legacy store 对 daemon 会话没有写入方，所以该调用永远返回 false，规范运行时那一支永远执行。移除这道保护有行为含义，值得单独一次提交。

关联 Issue

Part of #10795. Part of #4228.

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[qwen-code-ci-bot]

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head f0c8e84. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

_{Full-resolution recordings (.webm) are attached to the workflow run.}

— Qwen Code · web-shell visuals

[qwen-code-ci-bot]

Thanks for the PR — this one is unusually easy to gate, because the central claim is checkable statically rather than by reproduction.

Template looks good ✓ — every required heading is present and the Chinese translation is complete rather than abbreviated.

Problem: Real, and I verified it rather than taking the description's word for it. At the base commit, the only production references anywhere in packages/cli/src to the legacy Goal store symbols (registerGoalHook, unregisterGoalHook, getActiveGoal, setActiveGoal, clearActiveGoal, activeGoalEquals, get/setLastGoalTerminal, setGoalTerminalObserver, GoalTerminalEvent/GoalTerminalKind) live in restoreGoal.ts and MessageEmitter.ts — the two files this PR strips — and every one of them sits inside code the diff deletes. The seven removed exports only ever call each other; their sole outside references are test files, one stale comment, and one design doc. So the "closed dead cluster" claim holds.

Direction: Aligned. Goal v3 owns these surfaces, and removing the CLI's references before deleting the core modules is the right order — it turns the core-side deletion into a mechanical follow-up instead of one large cross-package refactor. No upstream CHANGELOG signal bears on this: it is internal plumbing removal, not a capability change.

Size: Cross-package (packages/cli + packages/web-shell), so the Stage 0 cross-package clause applies. 255 production lines (restoreGoal.ts 228, DaemonSessionProvider.tsx 15, MessageEmitter.ts 12) vs. 701 test lines, 0 generated/schema. Under the 500-line Tier 1 threshold, and you are a listed area owner with write access, so Tier 1 does not apply. Well under the 1000-line advisory too. Note that 228 of the 255 production lines are concentrated in one file rather than spread thin — this is depth in a single module, not a broad sweep.

Approach: Scope feels right, and it is genuinely minimal — deletion only, no drive-by restructuring, no formatting churn. Two honest notes, neither a blocker:

• The description says the three importers pull parseGoalStatusItem, isTranscriptItemRecord and the two types as well. They actually import only findGoalToRestore and collectGoalStatusItemsFromRecords. Those two functions plus GoalStatusItem / RestorableGoal stay exported with no external production consumer — pre-existing, not something this PR introduces, but they are the obvious next entries in the follow-up deletion.
• docs/design/2026-08-08-selective-session-restore.md:455 still names restoreGoalFromHistory(). It goes stale the moment this merges; worth a line here or in the sibling PR.

Risk: Stage 1e matched a high-risk path — packages/cli/src/acp-integration/session/emitters/MessageEmitter.ts (acp-integration). That is also the file carrying the one non-mechanical part of the change (an ACP wire field going away), so it is exactly where review attention belongs. Full Stage 2 depth and CI evidence before any approval.

Moving on to code review. 🔍

中文说明

感谢贡献！这个 PR 的准入判断 unusually 容易，因为它的核心论断可以静态核对，而不需要复现。

模板完整 ✓ —— 所有必需小节都在，中文翻译也是完整的，没有省略。

问题： 真实存在，而且我自己核对过，没有只采信描述。在 base commit 上，packages/cli/src 里对 legacy Goal store 符号（registerGoalHook、unregisterGoalHook、getActiveGoal、setActiveGoal、clearActiveGoal、activeGoalEquals、get/setLastGoalTerminal、setGoalTerminalObserver、GoalTerminalEvent/GoalTerminalKind）的生产代码引用，只出现在 restoreGoal.ts 与 MessageEmitter.ts 里——正是本 PR 要清理的两个文件——而且每一处引用都位于 diff 删掉的代码内部。被移除的七个导出只互相调用；它们在外部仅有的引用是测试文件、一处过时注释和一份设计文档。所以"自闭合的死代码簇"这个论断成立。

方向： 对齐。Goal v3 已经拥有这些界面，而且先移除 CLI 侧引用、再删除 core 模块，顺序是对的——这让 core 侧的删除变成一次机械跟进，而不是一次大范围跨包重构。上游 CHANGELOG 对此没有参考意义：这是内部管线清理，不是能力变化。

规模： 跨包（packages/cli + packages/web-shell），因此适用 Stage 0 的跨包条款。生产代码 255 行（restoreGoal.ts 228、DaemonSessionProvider.tsx 15、MessageEmitter.ts 12），测试 701 行，生成/schema 0 行。低于 Tier 1 的 500 行阈值，且你是有 write 权限的登记 area owner，所以 Tier 1 不适用；也远低于 1000 行的大 PR 建议线。注意 255 行里有 228 行集中在单个文件——这是单模块的深度改动，不是大面积扫改。

方案： 范围合理，也确实是最小改动——只有删除，没有顺手重构，没有格式化噪音。两点如实提出，都不是阻塞项：

• 描述里说三个引入方还引入了 parseGoalStatusItem、isTranscriptItemRecord 与两个类型。实际上它们只引入 findGoalToRestore 与 collectGoalStatusItemsFromRecords。那两个函数加上 GoalStatusItem / RestorableGoal 仍然是导出状态且没有外部生产消费者——这是既有情况，不是本 PR 引入的，但它们是后续删除 PR 里最顺理成章的下一批。
• docs/design/2026-08-08-selective-session-restore.md:455 仍然提到 restoreGoalFromHistory()。本 PR 一合并它就过时了；值得在这里或姊妹 PR 里补一行。

风险： Stage 1e 命中高风险路径——packages/cli/src/acp-integration/session/emitters/MessageEmitter.ts（acp-integration）。它也正好是承载本次改动中唯一非机械部分（一个 ACP 线协议字段消失）的文件，所以审查注意力就该放在这里。批准前需要完整的 Stage 2 深度与 CI 证据。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at d9e83853918e5c50fbabfd20b629d7cf27301813 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

I read the diff against an independent baseline: before opening it, I grepped the base tree for every symbol the title implies is going away, to see for myself who still calls them. The PR's claims survived that, which is not what I expected going in from a 908-deletion diff.

No Critical blockers found. What I verified:

• The dead cluster is genuinely closed. The seven removed exports (restoreGoalFromHistory, installGoalTerminalObserver, recordGoalStatusItem, goalTerminalEventToHistoryItem, findLastTerminalGoal, goalRestoreBlockedBy, goalConditionBlockedBy) plus the RestoreGoalResult / GoalRestoreBlockedReason types only ever call each other. Outside restoreGoal.ts their sole references are test files, one stale comment in history-replayer.test.ts, and one design doc. No production caller exists.
• The legacy store is unreachable from CLI production code after this. A base-tree grep for all twelve legacy symbols across packages/cli/src, excluding tests, returns hits in exactly two files: restoreGoal.ts and MessageEmitter.ts. Both are files this PR strips, and every hit is inside a deleted hunk. That is a clean, complete removal rather than a partial one.
• The wire-field removal is behaviour-identical, and I checked the reader rather than trusting the description. In normalizeGoalStatusEvent, the deleted block was if (!isRecord(loop)) return null; … if (!condition) return null; … return null; — every path already returned null. The guards only decided when to return the same value, so collapsing to return null cannot change behaviour. The explanatory comment is preserved, which is the right call.
• Nothing unmodified breaks. Session.test.ts is not in this diff but asserts _meta.stopHookLoop at three sites — all three use expect.objectContaining({ iterationCount, reasons }) and none mention goal, so dropping the field cannot fail them.
• No orphaned imports. isRecord (31 uses) and getString (19 uses) remain heavily used in DaemonSessionProvider.tsx. In the slimmed restoreGoal.ts, every retained import is still used, and isTerminalGoalStatusKind, writeStderrLineSafe, Config, GoalTerminalEvent/Kind and the four hook functions are correctly dropped along with their only callers.
• The three live importers are satisfied. acpAgent.ts, session/Session.ts and session/recovered-goal-update.ts each import only findGoalToRestore and collectGoalStatusItemsFromRecords — both kept.
• The removed test mocks were vacuous, not load-bearing. goalCommand.ts, use-llm-stream.ts and nonInteractiveCliCommands.ts never call the symbols their suites mocked, so assertions like expect(mockSetActiveGoal).not.toHaveBeenCalled() were trivially true and the deleted 'skips redundant active_goal store updates' case tested nothing. The 32 installGoalTerminalObserver: vi.fn() entries mock a method that does not exist on Session. Removing scaffolding that misleads a reader into thinking a path is live is the strongest part of this PR.
• The two retargeted tests keep the real invariants. The feat(cli): allow long /goal conditions #6665 no-length-cap pin moved from restoreGoalFromHistory to findGoalToRestore has no condition cap, and the start-time cases moved to findGoalToRestore carries the original start time. The invariants survive on the function that survives; only the deleted function's registration wiring lost coverage, which is correct since that wiring is gone.

One thing worth a reviewer's eye, not a blocker: acpAgent.test.ts drops registerGoalHook, setGoalTerminalObserver and setLastGoalTerminal from its @qwen-code/qwen-code-core mock factory. Those keys now resolve to the real core implementations in that suite instead of no-ops. The removed comment claimed they were "Reached through the real ui/utils/restoreGoal.js on the resume path" — my grep says otherwise, so the comment was stale and the mocks were dead. But this is the one place where a wrong judgement shows up as a subtle test-environment change rather than a red build, so it is the specific thing the unit suite has to settle.

Files changed (all 10)

File	What changed
packages/cli/src/ui/utils/restoreGoal.ts	The real change: 228 lines out, dropping the seven dead exports and their imports. Keeps the transcript scanner and card parsing.
packages/cli/src/ui/utils/restoreGoal.test.ts	473 out / 36 in. Deletes suites for removed functions; retargets the #6665 cap pin and the start-time cases onto findGoalToRestore.
packages/cli/src/acp-integration/acpAgent.test.ts	Drops the legacy core mock entries and the 32 installGoalTerminalObserver stubs. The mock-factory change noted above lives here.
packages/cli/src/acp-integration/session/emitters/MessageEmitter.ts	Stops attaching the goal sub-object to _meta.stopHookLoop; drops the getActiveGoal import. High-risk path per Stage 1e.
packages/web-shell/client/daemon/session/DaemonSessionProvider.tsx	Collapses the sole reader of that field — every path already returned null — to a plain return null with the reasoning kept as a comment.
packages/cli/src/ui/hooks/use-llm-stream.test.tsx	Removes four legacy store mocks; retargets one assertion to mockAddItem not receiving a goal_status card. Deletes one vacuous case.
packages/cli/src/ui/commands/goalCommand.test.ts	Removes the core mock factory and the two not-called assertions that were trivially true.
packages/cli/src/nonInteractiveCliCommands.test.ts	Drops __resetActiveGoalStoreForTests from beforeEach/afterEach and its import.
packages/cli/src/acp-integration/session/history-replayer.test.ts	One stale comment line referencing restoreGoalFromHistory removed.
packages/cli/src/acp-integration/acpAgent.worktree.test.ts	One installGoalTerminalObserver stub removed.

No sequence diagram: this is pure deletion with no new or reshaped runtime flow, so a diagram would add nothing.

Testing

This is an unattended CI run, so I did not build or execute anything from this PR — the evidence below is the PR's own CI, read through the API at d9e83853918e5c50fbabfd20b629d7cf27301813.

Zero failures at fetch time. Lint & Static (ubuntu-latest, Node 22.x) is green, which independently settles the static risks I checked by hand above — orphaned imports, typecheck, prettier and eslint on all ten files. Capture web-shell visuals and both Desktop Shell jobs are green too, which is the relevant signal for the DaemonSessionProvider.tsx edit.

The load-bearing check has not landed yet: Test (ubuntu-latest, Node 22.x) was still in_progress. That is the job which settles the acpAgent.test.ts mock-factory question, and it is the only reason this review is not finished. I am not polling it — the Qwen Triage Finalize job rewrites the table below in place once CI settles. Test (macos-latest) and Test (windows-latest) are skipped in this matrix, so the suite only runs on Linux here; for a platform-agnostic deletion that exposure is low, but it is real and I am not claiming it away. verify and tmux-testing were also skipped, so no A/B proof and no TUI capture ran.

Not verified: the pass counts quoted in the PR description (restoreGoal.test.ts 34, goalCommand.test.ts 65, nonInteractiveCliCommands.test.ts 52, emitters 115, acpAgent.test.ts 622, use-llm-stream.test.tsx 279, worktree+replayer 55) are the author's own local Linux run, not evidence I re-ran — I could not and did not execute them. Treat them as the author's claim until Test (ubuntu-latest) reports.

Sandboxed verification would settle the remaining gap: @qwen-code /verify — that dropping registerGoalHook / setGoalTerminalObserver / setLastGoalTerminal from the acpAgent.test.ts core mock factory leaves that suite pinning the same behaviour is not observable from the diff, since the failure mode is a silently changed test environment rather than a red build. An A/B against the base build would show whether the retained assertions are load-bearing in the same way before and after. There is no user-visible surface here, so /tmux would add nothing.

Real-scenario tmux testing: N/A — unattended CI run (tmux is local-invocation only, and PR code is never executed here), and there is no user-visible behaviour change to drive anyway.

中文说明

代码审查

我没有直接看 diff，而是先独立取了一个基线：在打开 diff 之前，先在 base 树上 grep 了标题暗示要移除的每一个符号，自己确认还有谁在调用它们。这个 PR 的论断经受住了核对——对一个删了 908 行的 diff 来说，这和我进去之前的预期不一样。

未发现 Critical 阻塞项。 我核对过的内容：

• 死代码簇确实是闭合的。 被移除的七个导出（restoreGoalFromHistory、installGoalTerminalObserver、recordGoalStatusItem、goalTerminalEventToHistoryItem、findLastTerminalGoal、goalRestoreBlockedBy、goalConditionBlockedBy）加上 RestoreGoalResult / GoalRestoreBlockedReason 两个类型，只互相调用。在 restoreGoal.ts 之外，它们仅有的引用是测试文件、history-replayer.test.ts 里一处过时注释，以及一份设计文档。没有任何生产调用者。
• 改动之后 CLI 生产代码无法再到达 legacy store。 在 base 树上对全部十二个 legacy 符号 grep packages/cli/src（排除测试），命中的文件恰好只有两个：restoreGoal.ts 与 MessageEmitter.ts。两者都是本 PR 清理的文件，而且每一处命中都在被删除的 hunk 内部。这是一次干净完整的移除，不是局部移除。
• 线协议字段的移除在行为上完全相同，而且我是去读了那个 reader，没有采信描述。 normalizeGoalStatusEvent 里被删掉的那段是 if (!isRecord(loop)) return null; … if (!condition) return null; … return null;——每一条路径本来就返回 null。那些判断只决定了何时返回同一个值，所以收敛成 return null 不可能改变行为。解释性注释被保留下来，这个处理是对的。
• 未被改动的文件不会被打破。 Session.test.ts 不在本 diff 里，但它在三处断言 _meta.stopHookLoop——三处都用 expect.objectContaining({ iterationCount, reasons })，都没有提到 goal，所以去掉该字段不可能让它们失败。
• 没有留下悬空 import。 isRecord（31 处）与 getString（19 处）在 DaemonSessionProvider.tsx 里仍被大量使用。精简后的 restoreGoal.ts 里每个保留的 import 都仍在用，而 isTerminalGoalStatusKind、writeStderrLineSafe、Config、GoalTerminalEvent/Kind 与四个 hook 函数都随其唯一调用者一起正确移除了。
• 三处存活的引入方需求都被满足。 acpAgent.ts、session/Session.ts 与 session/recovered-goal-update.ts 各自只引入 findGoalToRestore 与 collectGoalStatusItemsFromRecords——两者都保留了。
• 被移除的测试 mock 是空转的，不是承重的。 goalCommand.ts、use-llm-stream.ts 与 nonInteractiveCliCommands.ts 从不调用各自测试套里 mock 的那些符号，所以像 expect(mockSetActiveGoal).not.toHaveBeenCalled() 这样的断言恒为真，被删掉的 'skips redundant active_goal store updates' 用例什么也没测。那 32 处 installGoalTerminalObserver: vi.fn() mock 的是 Session 上并不存在的方法。清掉这类会让读者误以为某条路径还活着的脚手架，是本 PR 最有价值的部分。
• 两个改指对象的测试保住了真正的不变量。 feat(cli): allow long /goal conditions #6665 的无长度上限固定断言从 restoreGoalFromHistory 移到了 findGoalToRestore has no condition cap，起始时间那几个用例移到了 findGoalToRestore carries the original start time。不变量存活在了存活的那个函数上；只有被删函数的注册接线失去了覆盖，而这是正确的，因为那段接线已经不存在了。

有一处值得评审者过目，但不是阻塞项： acpAgent.test.ts 从它的 @qwen-code/qwen-code-core mock 工厂里去掉了 registerGoalHook、setGoalTerminalObserver 与 setLastGoalTerminal。这几个 key 现在在该测试套里解析到真实的 core 实现，而不再是空转 mock。被删掉的那行注释声称它们"在 resume 路径上经由真实的 ui/utils/restoreGoal.js 被触达"——我的 grep 结果与之矛盾，所以那行注释是过时的，mock 也是死的。但这是唯一一个判断出错时会表现为测试环境的细微变化、而不是红色构建的地方，所以它正是单元测试套必须敲定的那一件事。

文件清单见上表（共 10 个文件）。没有画时序图：这是纯删除，没有新增或重塑的运行时流程，画了也没有信息量。

测试

这是一次无人值守的 CI 运行，所以我没有构建或执行本 PR 的任何代码——下面的证据是 PR 自己的 CI，通过 API 在 d9e83853918e5c50fbabfd20b629d7cf27301813 上读取的。

抓取时刻零失败。Lint & Static (ubuntu-latest, Node 22.x) 已通过，它独立敲定了我上面手工核对过的静态风险——十个文件上的悬空 import、类型检查、prettier 与 eslint。Capture web-shell visuals 与两个 Desktop Shell 作业也是绿的，这对 DaemonSessionProvider.tsx 的改动是相关信号。

承重的检查还没落地：Test (ubuntu-latest, Node 22.x) 当时仍是 in_progress。 那个作业才是敲定 acpAgent.test.ts mock 工厂问题的地方，也是这份审查尚未结束的唯一原因。我没有轮询它——CI 落定后 Qwen Triage Finalize 作业会就地改写下面的表格。本矩阵里 Test (macos-latest) 与 Test (windows-latest) 是 skipped，所以测试套这里只在 Linux 上跑；对一个与平台无关的删除改动来说这个暴露面很低，但它确实存在，我不会把它说成没有。verify 与 tmux-testing 同样是 skipped，所以没有 A/B 证明，也没有 TUI 抓取。

未验证：PR 描述里引用的通过数（restoreGoal.test.ts 34、goalCommand.test.ts 65、nonInteractiveCliCommands.test.ts 52、emitters 115、acpAgent.test.ts 622、use-llm-stream.test.tsx 279、worktree+replayer 55）是作者自己在 Linux 上跑的结果，不是我重跑的证据——我不能也没有执行它们。在 Test (ubuntu-latest) 报告之前，请把它们当作作者的说法。

沙箱验证可以补上剩下的缺口：@qwen-code /verify —— 从 acpAgent.test.ts 的 core mock 工厂里去掉 registerGoalHook / setGoalTerminalObserver / setLastGoalTerminal 之后，该测试套是否仍然钉住同样的行为，这一点从 diff 上看不出来，因为它的失效模式是测试环境被悄悄改变，而不是构建变红。对 base 构建做 A/B 能显示保留下来的断言在改动前后是否同样承重。这里没有用户可见界面，所以 /tmux 加不了任何东西。

真实场景 tmux 测试：N/A —— 无人值守 CI 运行（tmux 仅限本地调用，且这里从不执行 PR 代码），而且本来也没有用户可见的行为变化可驱动。

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

Check	Conclusion
Capture web-shell visuals (ubuntu-latest, Node 22.x)	✅ success
Classify PR	✅ success
Desktop Shell (ubuntu-22.04)	✅ success
Desktop Shell (windows-2022)	✅ success
Integration Tests (no-AK, No Sandbox)	✅ success
Lint & Static (ubuntu-latest, Node 22.x)	✅ success
macos-latest / Java 21	✅ success
OpenTUI no-flicker gate	✅ success
Real daemon E2E / Java 11	✅ success
Test (ubuntu-latest, Node 22.x)	✅ success
TUI parity snapshots (ink vs opentui)	✅ success
ubuntu-latest / Java 11	✅ success
ubuntu-latest / Java 17	✅ success
ubuntu-latest / Java 21	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success
windows-latest / Java 21	✅ success

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

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at d9e83853918e5c50fbabfd20b629d7cf27301813 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 3/5 — clean review, no blocking findings; the cap is pure policy, because a fork refactor cannot be auto-approved and needs a maintainer's sign-off.

Going in, I expected a 908-deletion diff from a fork to have a body in it somewhere. It doesn't. My independent proposal from the title alone was: grep every legacy symbol, confirm no production callers, delete the exports and their imports, delete the test mocks that stub them, and leave the core modules for a follow-up. The PR does exactly that — and beats my baseline in one respect I would not have thought to chase from the title: it also found the _meta.stopHookLoop.goal wire field and its reader in Web Shell, and proved that reader was a no-op rather than just deleting it and hoping.

The scope discipline is the part I'd single out. It would have been easy to bundle the abortGoalForStopHookCap guard in session/Session.ts, which is dead for the same reason — the author found it, wrote it up, and deliberately left it out because removing it is behaviour-bearing. That is the correct call and it is the reason this PR is reviewable at all.

Six months from now this is a thank-you, not a curse: the follow-up core deletion becomes mechanical because the CLI side is already clean, and the misleading mocks that made a reader think a legacy path was live are gone.

Two things I want on the record before a human decides, neither of them a defect in the code:

Merge order matters across the Goal series. #11458 (refactor(core): stop routing Goal turns through the legacy Stop hook) is the core-side sibling this PR refers to, opened the same day. This one must land first or together: it removes the CLI's imports of core's legacy exports, so if the core deletion went in ahead of it, main would have CLI code importing symbols that no longer exist. Worth sequencing explicitly rather than merging whichever goes green first.

This is one of 13 open PRs from the same author in the last 48 hours, including a three-part Goal cluster (#11457, #11458, #11459). I evaluated this one on its own merits and it held up — but I'm flagging the volume because that is exactly the condition where a reviewer starts approving out of fatigue rather than judgement, and because the three Goal PRs interact and probably deserve to be read together rather than one at a time.

Why I am not approving. Not because I found anything. The approval guardrail is deterministic: a cross-repository PR whose title is a refactor type never gets auto-approved, and this is both. I checked — isCrossRepository=true, title matches ^refactor — and the guardrail has no author exemption, so it applies even though you are a listed area owner with write access. For the same reason I left no deferred-approval marker in this comment: that path would hand the finalize job a standing approval instruction the guardrail forbids, so it is withheld even though CI is still running.

On the evidence: Lint & Static is green and settles every static risk I checked by hand. Test (ubuntu-latest, Node 22.x) was still running when I posted Stage 2, and it is the one check that settles the acpAgent.test.ts mock-factory change — the only place in this diff where a wrong judgement would show up as a subtly different test environment instead of a red build. Please don't merge on the strength of my review alone; wait for that job.

Escalation. This needs a human call, and I could not resolve an accountable owner deterministically: the PR carries no labels, so the area map did not match (need-discussion is required and no scope/* label is present), and there are no prior human reviews to fall back on. I am deliberately not @mentioning a guessed login. Note that the core-goals area owner is you, the author — so this needs a different maintainer to sign off. Adding a scope/core or need-discussion label would let the resolver pick someone accountable on a re-run.

⏸️ Deferring to a maintainer — fork refactor guardrail, plus the merge-order question against #11458. Needs a human call on this one.

中文说明

Confidence: 3/5 —— 审查干净，没有阻塞项；这个上限纯粹来自政策，因为 fork 的 refactor PR 不能自动批准，需要维护者签字。

进去之前我以为一个来自 fork、删了 908 行的 diff 总会在某处露馅。它没有。仅凭标题我给出的独立方案是：grep 每一个 legacy 符号，确认没有生产调用者，删掉这些导出及其 import，删掉 stub 它们的测试 mock，core 模块留给后续 PR。这个 PR 做的正是这些——而且在有一点上超过了我的基线，那是我从标题不会想到去追的：它还找到了 _meta.stopHookLoop.goal 这个线协议字段以及 Web Shell 里的 reader，并且证明了那个 reader 是空操作，而不是直接删掉了事。

范围克制是我最想点出来的部分。本来很容易把 session/Session.ts 里的 abortGoalForStopHookCap 保护一起带走——它因为同样的原因也是死的。作者发现了它、写进了描述、并且刻意留在范围外，因为移除它有行为含义。这个判断是对的，也正是这个 PR 之所以还能被审查的原因。

六个月后回看，这是一件让人感谢而不是骂人的事：core 侧的后续删除会变成机械操作，因为 CLI 侧已经干净了；而那些让读者误以为 legacy 路径还活着的误导性 mock 也消失了。

在人做决定之前，有两点我要记录在案，都不是代码缺陷：

Goal 系列的合并顺序很重要。 #11458（refactor(core): stop routing Goal turns through the legacy Stop hook）就是本 PR 提到的 core 侧姊妹 PR，同一天开的。这一个必须先合或同时合：它移除了 CLI 对 core legacy 导出的 import，所以如果 core 侧的删除先进去，main 上就会出现 CLI 代码引用已不存在的符号。值得明确排个顺序，而不是谁先变绿谁先合。

这是同一作者过去 48 小时内 13 个开放 PR 之一，其中包括一个三件套的 Goal 集群（#11457、#11458、#11459）。我是按它自身的价值评估这一个的，它站得住——但我把体量点出来，因为那正是审查者会因为疲劳而不是判断力开始放行的条件，也因为这三个 Goal PR 互相影响，大概值得放在一起读，而不是一个个读。

为什么我没有批准。 不是因为我发现了什么。批准护栏是确定性的：标题为 refactor 类型的跨仓库 PR 永不自动批准，而这个 PR 两条都占。我核对过——isCrossRepository=true，标题匹配 ^refactor——并且该护栏没有作者豁免，所以即便你是有 write 权限的登记 area owner，它依然适用。出于同样的原因，我在这条评论里没有留下延迟批准标记：那条路径会把一份护栏所禁止的常设批准指令交到 finalize 作业手里，所以即使 CI 还在跑，它也被扣下了。

关于证据： Lint & Static 已绿，它敲定了我手工核对过的每一项静态风险。我发 Stage 2 时 Test (ubuntu-latest, Node 22.x) 还在跑，而它是唯一能敲定 acpAgent.test.ts mock 工厂改动的检查——那是这个 diff 里唯一一个判断出错会表现为测试环境细微不同、而不是构建变红的地方。请不要仅凭我的审查就合并；等那个作业出结果。

升级处理。 这需要人来定，而我无法确定性地解析出一个负责人：这个 PR 没有任何 label，所以 area map 没有匹配上（需要 need-discussion，且没有任何 scope/* label），也没有既有的真人审查可以回退。我刻意没有 @ 一个猜出来的登录名。注意 core-goals 的 area owner 就是你，也就是作者——所以这需要另一位维护者签字。加上 scope/core 或 need-discussion label，重跑时解析器就能选出负责人。

⏸️ 转交维护者 —— fork refactor 护栏，加上与 #11458 的合并顺序问题。这个需要人来定。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at d9e83853918e5c50fbabfd20b629d7cf27301813 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

Not explored to full depth (tool budget reached): "agent 1d": none — no check was cut short..

Test Plan (not a blocker): session/Session.ts — no such file or directory; session/recovered-goal-update.ts — no such file or directory; 34 passed — this review observed 29435, 6854, 515 passed; 65 passed — this review observed 29435, 6854, 515 passed; 52 passed — this review observed 29435, 6854, 515 passed; and 4 more.

中文说明

已审查。 建议见行内评论。

未探索到全部深度（达到工具调用预算）："agent 1d"：none — no check was cut short.。

Test Plan（非阻断）：session/Session.ts — no such file or directory; session/recovered-goal-update.ts — no such file or directory; 34 passed — this review observed 29435, 6854, 515 passed; 65 passed — this review observed 29435, 6854, 515 passed; 52 passed — this review observed 29435, 6854, 515 passed; and 4 more。

— qwen3.8-max via Qwen Code /review (v0.23.1)

[qqqys]

Both taken, in f0c8e84. One correction to the second finding's premise, which I only found because I went to check it.

R1-1 — the unpinned stopHookLoop payload. Confirmed exactly as filed, including the sweep: stopHookLoop occurs on six lines, MessageEmitter.test.ts has emitGoalStatus and emitGoalState describes and no emitStopHookLoop at all, and all three Session.test.ts assertions read the payload through toContainEqual(expect.objectContaining(...)). Added the exact-equality case beside its two siblings.

I ran the mutation you specified rather than assuming it works. With the goal sub-object restored on the payload:

suite	result under the mutation
new emitStopHookLoop exact-shape case	1 failed
Session.test.ts -t "Stop hook loop"	1 passed

So the gap was real and is now closed by the assertion that bites rather than by one that would have let the projection back in.

R1-2 — the enumeration in the comment. The substance is right and I fixed it: normalizeGoalStatus admits set, cleared, achieved, failed, aborted, usage_limited and paused, every non-null result becomes a card, and TERMINAL_GOAL_STATUS_KINDS is only ['achieved','aborted','failed'] — so "only terminal events and the initial set event become cards" is wrong about three kinds, and reconciling the allowlist to it would regress the bug recorded beside the paused entry.

But the inaccuracy is not new in this diff. The comment I replaced said the same thing:

$ git show 1919ff97:packages/web-shell/client/daemon/session/DaemonSessionProvider.tsx
  # "...The active goal state is already visible in the status bar; only terminal
  #  events and the initial "set" event are shown as transcript cards."

Both halves of that claim survived my rewrite verbatim in meaning. So this is a pre-existing inaccuracy in a comment this branch happens to have rewritten, not one it introduced — worth knowing because it changes whether the finding is in scope. I fixed it anyway: the comment was rewritten here, so correcting it here is cheaper than leaving a known-wrong enumeration for the documentation slice to find. The new wording points at normalizeGoalStatus as the authority, names the three non-terminal kinds it admits, and says this return is only the fallthrough for an event carrying neither a status nor a terminal.

One thing that is not mine, flagged so it does not get attributed to this branch. DaemonSessionProvider.test.tsx → does NOT stop reconnect on session_closed with reason "idle_timeout" fails on a loaded host: it normally runs in ~2s against a 5s timeout, and at load average 4 on this 4-core box it takes 5.1s. My only change to that file is four comment lines, so I checked it both ways under the same load — 3 of 3 failures with my changes, and 3 of 3 failures with them stashed. Pre-existing timing sensitivity, not a regression here.

MessageEmitter.test.ts 30 passed; DaemonSessionProvider.test.tsx 317 passed with that one flake.

[yiliang114]

Approving. restoreGoal.ts drops the registerGoalHook/setGoalTerminalObserver/unregisterGoalHook plumbing and the terminal-observer scan, pairing with #11458's core-side removal of the Stop-hook routing. Net −830 lines, tests green, CI fully green (Lint, Test, web-shell E2E, no-AK integration, Real daemon E2E, Capture visuals). Bot review is Suggestion-only, no blockers.

[chiga0]

No blocking findings.
Approval blockers: none.

Scope

Source-only review. Reviewed all 11 changed files in the diff.

Not reviewed: packages/core/src/goals/ (untouched by this PR — the legacy core modules are unchanged), macOS and Windows runtime behavior (no host), execution rung (working tree unavailable).

---

Findings

None.

---

What was checked

Wire format change (_meta.stopHookLoop.goal removal)

The removed field in MessageEmitter.emitStopHookLoop (MessageEmitter.ts:95-113) was conditional on getActiveGoal(this.sessionId) returning a non-null value. The only CLI writer to the active goal store was registerGoalHook, whose only CLI caller was restoreGoalFromHistory. A search of the repository confirms restoreGoalFromHistory had no production callers in the CLI (only test files). So getActiveGoal returned undefined on every real call and the goal sub-object was never emitted.

The Web Shell reader (DaemonSessionProvider.tsx:5295–5304) checked loop['goal'] for a record and a non-empty condition, then returned null unconditionally on the last line regardless. Removing the branch is byte-identical in behavior.

The new emitStopHookLoop test in MessageEmitter.test.ts uses exact equality (not objectContaining) specifically to guard against accidental reintroduction of the goal sub-object. This addresses the CI bot's inline suggestion at MessageEmitter.ts:105, which the author confirmed was incorporated in the head commit.

Removed exports from restoreGoal.ts

Verified that all three production importers of restoreGoal.js at head — acpAgent.ts (line ~460), session/Session.ts (line ~330), and session/recovered-goal-update.ts (lines 17–20) — import only the kept symbols: findGoalToRestore and collectGoalStatusItemsFromRecords. The PR description additionally lists parseGoalStatusItem, isTranscriptItemRecord, and two types as kept, consistent with the retained module surface. None of the seven deleted exports (restoreGoalFromHistory, findLastTerminalGoal, goalTerminalEventToHistoryItem, recordGoalStatusItem, installGoalTerminalObserver, goalRestoreBlockedBy, goalConditionBlockedBy) appear in production callers outside of the files modified by this PR.

Test scaffold cleanup

installGoalTerminalObserver: vi.fn() is removed from 28 Session mock objects in acpAgent.test.ts and siblings. Confirmed the method does not appear in Session.ts at head — these were mocks of a method that does not exist, which is the kind of scaffolding that misleads readers about live paths.

The two retargeted tests (10,000-character objective pin and start-time cases) now assert directly against findGoalToRestore's return value, which is verifiable without the legacy hook machinery.

Cross-check against existing reviews

yiliang114 (APPROVED): Noted the removal pairs with core-side sibling PR #11458. Consistent with my reading — confirmed, not a miss.

qwen-code-ci-bot suggestion 1 (MessageEmitter.ts:105): "no exact-shape test for the payload." The new emitStopHookLoop describe block with exact equality addresses this directly. Author confirmed incorporation.

qwen-code-ci-bot suggestion 2 (DaemonSessionProvider.tsx:5301): Comment style. Excluded per §C.

---

Unreviewed dimensions

• Execution rung not run: no working tree available. The PR's own test results show all six suites passing on Linux.
• packages/core/src/goals/ untouched: no cross-package regression risk from this PR.
• macOS and Windows: no terminal-dependent or platform-branching change introduced.

Reviewed with AI assistance.

[qwen-code-dev-bot]

Approved at head f0c8e841.

Required CI is green here — Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke, Capture web-shell visuals, TUI parity snapshots and the OpenTUI no-flicker gate all completed successfully; only review-pr (the reviewer's own job) is still running.

What I checked, rather than trusting the description:

• Every removed export was already dead at the base commit. For each of restoreGoalFromHistory, findLastTerminalGoal, goalTerminalEventToHistoryItem, recordGoalStatusItem, installGoalTerminalObserver, goalRestoreBlockedBy and goalConditionBlockedBy, a repo-wide grep at 3a262e09 finds only its own definition in restoreGoal.ts plus this module's tests — no production caller. Since this PR changes no other production file that could have called them, nothing user-visible was cut; the three production importers (acpAgent.ts:460, session/Session.ts:330, session/recovered-goal-update.ts:20) reference only findGoalToRestore and collectGoalStatusItemsFromRecords, both retained along with their internal helpers.
• The one wire narrowing is behavior-identical. The deleted stopHookLoop branch in normalizeGoalStatusEvent ended in return null on every path — the isRecord(loop) / isRecord(goal) / !condition guards only ever short-circuited to that same null, and createGoalStatusUiEvent is reachable solely through the goalStatus / goalTerminal branches above it. So dropping it changes no card, and the replacement comment is accurate: normalizeGoalStatus (:5337-5353) does admit set, cleared, achieved, failed, aborted, usage_limited and paused, with the regression note recorded beside paused.
• The narrowed payload is now pinned. MessageEmitter.test.ts asserts the _meta.stopHookLoop object with exact equality, so re-attaching the first-generation goal sub-object — a partial revert or a bad merge — goes red instead of riding alongside _meta.goalState.
• Two retargets rather than deletions, which are the ones worth reading: the #6665 no-cap pin now asserts against the live scanner ('x'.repeat(10_000) at restoreGoal.test.ts:276) and the start-time cases assert the scanner's setAt.

No new Critical found. Order-independent with the core-side sibling: this PR leaves goalHook.ts / activeGoalStore.ts exported and untouched, and either landing first still compiles. The abortGoalForStopHookCap call that survives in session/Session.ts is already called out here as its own behaviour-bearing follow-up, which is the right split.

[qwen-code-ci-bot]

Released in v0.23.3.