docs(goal): retire the Stop-hook era prose after the implementation was removed - #11922
Conversation
…as removed The first-generation Goal implementation was deleted in QwenLM#11900, but a design doc, an i18n key, two comments and a setting description still describe it: the input-control design for a Goal loop that ran as a blocking Stop hook, the `/goal` description string nothing reads any more, an ACP control comment naming an in-memory store and a type that no longer exist, a "judge's verdict" field comment, and the per-turn tool-call cap saying Goal iterations are Stop-hook continuations.
github-actions
Bot
added
the
review/self-reported
…copied from the bridge
|
|
|
Thanks for the PR! Template looks good ✓ — every required heading is present, the Risk & Scope bullets are filled in, and the Chinese translation is complete rather than abbreviated. Problem: observed, not theoretical, and I confirmed it independently rather than taking the description's word. At this PR's base ( Direction: aligned. Retiring prose that points at deleted code is documentation accuracy, not scope creep, and it is cheap to review. Upstream's CHANGELOG has no direct reference to this (it is qwen-code's own Goal v1 → v3 migration), but Size: this does reach core paths — Approach: the scope feels right, and I specifically like that it resisted the obvious over-reach. Deleting the design doc rather than rewriting it is correct — I checked, and it has no I have two completeness questions from the code review — both comment-only, neither a blocker — detailed below in Stage 2. Risk: no elevated risk signals. None of the changed files match the high-risk paths from the revert-history analysis. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需标题都在，Risk & Scope 三项都填了，中文说明也是完整翻译而非摘要。 问题： 是已观测到的问题，不是理论性加固。我没有只采信 PR 描述，而是独立核实过：在本 PR 的 base（ 方向： 对齐。清理指向已删除代码的文字属于文档准确性，不是范围扩张，而且 review 成本很低。上游 CHANGELOG 没有直接对应的条目（这是 qwen-code 自己的 Goal v1 → v3 迁移），但 规模： 本 PR 确实触及核心路径—— 方案： 范围合理，尤其欣赏它没有明显越界。删除而非重写那篇设计文档是对的——我查过，它没有 代码审查里我有两个完整性方面的疑问——都只是注释层面，都不是阻塞项——详见下方 Stage 2。 风险： 无升级风险信号。改动文件均未命中基于 revert 历史分析得出的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI read the base tree at The The The tool-call cap rewording is correct, and I checked the mechanism rather than trusting it. Goal turns reset the detector on their own path — The locale key removal is safe. The design doc deletion orphans nothing. No Two things it left behind — non-blocking, but both are the same sentence it just fixedNeither is a regression; the base is equally stale. But this PR's whole deliverable is comment accuracy, so they are worth a look before merge. 1. That is the source of the wording this PR corrected in three user-facing copies. The description says the cap wording "was checked against the code rather than deleted" — this is the code comment it was checked against, and it still attributes 2. The PR retires "the judge's verdict" on line 374 but leaves "Judge turns" and "the first stop-hook evaluation" on line 371 — both retired concepts, and No other blockers. No over-abstraction, no duplication introduced, no code in the wrong package — the diff is comments, one description string, markdown, and unused locale data. Files changed (11)
TestingThis is an unattended CI run, so per the gate's rules I did not build, run, or check out any of this PR's code — the evidence below is the PR's own CI read through the API, plus my static verification of each claim against the base tree. The unit suite is red — and it is not this PR
I am classifying this as pre-existing and not caused by this PR, and here is the evidence rather than the assertion:
I classified this from the diff and the cross-PR reproduction, not from anything the log text says about itself. For what it is worth, an open follow-up issue #11699 ("Follow-up work left by #10841 (skill naming)") looks like the right neighbourhood — that is a lead for whoever owns it, not something I verified. What did pass, and what it provesThe two checks that could actually break here both ran and both passed. The i18n removal and the generated-schema coupling are the only ways this diff can fail mechanically, and they are gated by steps in the Step 28 is the author's Everything else I verified by reading code at the base and head commits: the handler return shape, the Also green on this head: Two entries that are not build signal: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Real-scenario tmux testing: N/A. Nothing here is user-visible at runtime — the diff is comments, markdown, one settings description string, and a locale key with no reader. There is no TUI surface to drive, and no behavioural claim for a sandboxed lane to settle. 中文说明代码审查我在
工具调用上限的改写是正确的，我核实的是机制而不是采信结论。 Goal 轮走自己独立的重置路径—— 删除 locale key 是安全的。 删除设计文档不会留下孤儿。 它没有 两处遗留 —— 非阻塞，但都是它刚刚修好的同一句话两者都不是回归；base 上同样陈旧。但本 PR 的全部交付物就是注释准确性，所以合并前值得看一眼。 1. 这正是本 PR 在三处面向用户的副本里改正的那段措辞的源头。PR 描述说上限措辞是 "checked against the code rather than deleted"——而这就是它对照的那段代码注释，它仍然把 2. PR 在第 374 行退掉了 "the judge's verdict"，却在第 371 行留下了 "Judge turns" 和 "the first stop-hook evaluation"——两者都是已退役的概念，而 无其他阻塞项。没有过度抽象，没有引入重复，没有把代码放错 package——diff 全部是注释、一条描述字符串、markdown 和无人使用的 locale 数据。 测试这是一次无人值守的 CI 运行，因此按 gate 规则，我没有构建、运行或 checkout 本 PR 的任何代码——下面的证据是通过 API 读到的本 PR 自己的 CI，加上我在 base 代码树上对每条断言的静态核实。 单元测试套件变红了——但不是本 PR 造成的
我将其判定为既有问题、非本 PR 造成，下面给的是证据而不是断言：
我这个判定来自 diff 与跨 PR 重现，而不是日志文本对自身性质的任何说法。顺带一提，open 状态的后续 issue #11699（"Follow-up work left by #10841 (skill naming)"）看起来属于同一片区域——这是给负责人的线索，不是我核实过的结论。 通过了什么，以及它能证明什么真正可能让本 diff 机械性失败的两个检查都跑了，而且都通过了。 i18n 删除与生成 schema 的耦合是唯一的两种失败方式，它们由 第 28 步就是作者的 其余部分我都是通过在 base 与 head commit 上阅读代码来核实的：handler 返回结构、 本 head 上同时为绿的还有： 有两项不属于构建信号： （CI 表格见上方英文部分，未在中文里重复。） 真实场景 tmux 测试：N/A。这里没有任何运行时可见的改动——diff 全部是注释、markdown、一条设置描述字符串和一个无读取者的 locale key。没有可驱动的 TUI 界面，也没有需要沙箱通道来定论的行为性断言。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after✅ No screenshot changes against the PR base. Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Confidence: 4/5 — every claim in this PR checks out against the code, and the only two things I would change are stale comments it did not touch. The one red check is not this PR's, and I have the evidence for that call below. Stepping back: this is a PR whose entire product is accuracy, so the honest test is whether it leaves the tree more accurate or just differently inaccurate. It leaves it more accurate, and I could not find a single claim in the description that did not survive contact with the code. That is not nothing — the description asserts a specific mechanism for the tool-call cap ("each runtime-scheduled Goal turn is its own The scope discipline is the part I would hold up as an example. Deleting a design doc instead of updating it, and saying why; removing a locale key from four files and not the other five, because the other five never had it; naming three Stop-hook-mentioning design docs it deliberately left alone with a reason for each. That is a PR that knows exactly where its edges are. My one hesitation about the edges is finding 1 in Stage 2 — it fixed the three downstream copies of a sentence and left the code comment the sentence came from, in a core-path file. If that was a deliberate choice to keep the diff out of Six months from now this is a thank-you, not a curse: nobody will have to reverse-engineer why the docs describe a Stop hook that does not exist. Two notes on process rather than content. The diff spans four packages and reaches a core path ( The part a maintainer needs to decide, because I cannotVerdict on the diff: approve. Verdict on the merge: blocked by something this PR did not cause.
That matters mechanically: the marker below asks the finalize job to approve once every check on this commit is green, and a red unit suite means it will withhold and flag rather than approve. I am deliberately not approving in this run — CI is also still incomplete ( So this needs a human call, and there are exactly two sane ways out: land the 中文说明Confidence: 4/5 —— 本 PR 的每一条断言都经得住与代码对照；我唯一想改的两处，是它没有触碰的陈旧注释。唯一变红的那项检查不属于本 PR，下面给出我这个判断的证据。 退一步看：这是一个全部产出即"准确性"的 PR，所以诚实的判据是它让代码树更准确，还是只是换了一种不准确。它让代码树更准确，而且我在描述里找不到任何一条断言在与代码接触后不成立。这一点并非无关紧要——描述对工具调用上限断言了一个具体机制（"each runtime-scheduled Goal turn is its own 范围克制是我最愿意拿来当范例的部分。删除而不是更新一篇设计文档，并说明原因；只从四个文件里删 locale key 而不动另外五个，因为另外五个本来就没有；点名三篇它刻意保留的、提到 Stop hook 的设计文档并各给一条理由。这是一个清楚知道自己边界在哪里的 PR。我对边界唯一的犹豫是 Stage 2 里的第 1 条发现——它改了三处下游副本，却留下了那句话的源头代码注释，而那个文件在核心路径上。如果这是为了把 diff 挡在 六个月后回看，这是让人道谢而不是让人咒骂的改动：不会再有人需要反推为什么文档描述了一个并不存在的 Stop hook。 两点关于流程而非内容的说明。diff 跨了四个 package 并触及核心路径（ 需要维护者定夺的部分，因为我定不了对 diff 的结论：approve。对合并的结论：被一件本 PR 没有造成的事情阻塞。
这在机制上很关键：下面这个 marker 请求 finalize 任务在本 commit 的所有检查变绿后批准，而单元测试变红意味着它会选择不予批准并标记状态，而不是批准。我这次刻意没有批准——CI 也仍未跑完（ 所以这里需要人来定夺，而且只有两条合理出路：把 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- T1-1 loopDetectionService.ts:148-151 stale cap comment — already reported (comment 5677033129)
- T1-2 web-shell types.ts:371 stale iterations comment — already reported (comment 5677033129)
Not explored to full depth (tool budget reached): "agent 1a": I did not run the web-shell or cli unit suites; my verification of the touched files was static (Prettier, node --check , and cross-reading the implementations….
中文说明
已审查。 建议见行内评论。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 1a"：I did not run the web-shell or cli unit suites; my verification of the touched files was static (Prettier, node --check , and cross-reading the implementations…。
— qwen3.8-max via Qwen Code /review (v0.23.3)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
github-actions
Bot
removed
the
review/self-reported
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings. Pure documentation/comment cleanup following the Goal Stop-hook removal in #11900. No runtime behavior changes; the four edited TypeScript files produce identical JavaScript with comments removed.
Reviewed with AI assistance.
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
APPROVE
核对基线：head 24610428b89e9777b523defaee16f8211d882b38（base 12b8cbc4，15 个文件 +20/-73）。该 head 上 22 项非跳过检查全部 success、无 pending、无失败（Test (ubuntu-latest, Node 22.x)、Lint & Static、Integration Tests (no-AK, No Sandbox)、web-shell E2E Smoke、TUI parity snapshots、OpenTUI no-flicker gate、Real daemon E2E / Java 11、Capture web-shell visuals、Java/Desktop 各档）。3 条线程全部关闭、全是 [Suggestion]、无 Critical。说明：本 PR 已于 2026-09-15T12:56:11Z 由作者本人合入，本票是补记，没有参与放行。
逐条按本 head 重判历史项：R1-1 说正文里的 Closes #10795 会把仍有活的追踪 issue 顺带关掉 —— 现已改为 Part of #10795，且 #10795 在本 head 时刻仍为 open，落实。R1-3 说 packages/acp-bridge/src/status.ts 改写后，同一条数据路径的姊妹文档 DaemonGoal 头注释还停留在「只活在子进程内存里」，两边互相打脸 —— 本 head 的 packages/web-shell/client/daemon/workspace/types.ts:360-363 已改成「读自持久化的 Goal 运行时，含 paused/blocked，不含未加载会话」，与 status.ts 一致，两个字段注释也一并跟上（iterations 改成「Canonical Goal turns completed so far」、lastReason 改成「Why the Goal last stopped, or the verifier's most recent reason」）。
独立扫描未发现 Critical：四个「代码」文件的改动全是注释或文案 —— use-llm-stream.ts、useMessageQueue.ts 各 +2 行注释、零逻辑改动；loopDetectionService.ts 是三条注释重写，我核了它的新说法：client.ts:4574 确实在 StopHookLoop 分支里 loopDetector.reset(prompt_id)，client.ts:3272 在 startsInteraction 处重置（运行时安排的 Goal turn 走这条），紧跟其后还有 takePendingGoalEvents()，所以「两类续跑各自开新预算」站得住。model.maxToolCallsPerTurn 的同一句话在三处同步改了（docs/users/configuration/settings.md、settingsSchema.ts、packages/vscode-ide-companion/schemas/settings.schema.json），没有留下互相矛盾的副本；hooks.md 里去掉的 /goal 注册来源与 #11900 删掉实现一致。删掉的 4 份语言包里的 'Set a goal — keep working until the condition is met' 在本 head 全仓引用数为 0（base 上也只有语言包自己，没有生产者），所以是清孤儿键，不会让任何提示退化成裸键名。
记录（不阻塞）：R1-2 我核过仍然成立 —— 随 docs/design/goal-loop-input-control.md 一起删掉的，不只是 Stop-hook 那段：它的 Design 条目记录的「活跃 turn 期间队列只额外排空 /goal、clear 只生效不再喂模型、多个 Goal 命令只发最后一个指令」这套行为在本 head 依然是活代码（GoalQueueControlMode 三种模式与 popNextSubmission 仍在 useMessageQueue.ts，本 PR 只给那里加了注释），而 docs/ 下已无任何文件提到这两个名字，树里对这块的唯一定档就没了。建议后续单独补一份只写现存行为的设计说明（或把该段挪进 Goal v3 的设计文档），不影响本次合入。
5 participants
What this PR does
Removes stale Goal Stop-hook descriptions after the old hook, judge and in-memory store were retired. The settings reference, schema descriptions, hook browser documentation and bridge/Web Shell comments now describe runtime-scheduled Goal turns and persisted state. An unused locale key and the obsolete Stop-hook design are removed. The still-live reasons for prioritizing Goal controls and not restoring already executed commands are preserved next to their implementations.
Why it's needed
#11900 removed the old Goal hook, judge and store, but documentation still directed readers to that retired execution path. Goal turns and blocking Stop-hook continuations each reset the tool-call budget through separate paths. The workspace Goal list also includes paused and blocked goals from resident sessions; it is not a list of memory-only running goals.
#10795 remains open: compatibility projections and history parsing are still used. This documentation cleanup does not claim to finish their removal or change the existing wire format.
Reviewer Test Plan
How to verify
Check that Goal turns are described separately from blocking Stop-hook continuations and that all three settings-description copies agree. The hook browser documentation should no longer list
/goalas a Session hook source. Goal list comments should match persisted snapshots and canonical turn counts, including paused and blocked goals. Queue comments should explain why controls bypass ordinary input and why executed commands are not restored after cancelled steering preparation.Evidence (Before & After)
N/A for runtime behavior: only documentation, comments, a setting description and an unused locale key change. Local checks passed: build, typecheck, focused CLI tests (333), core loop-detector tests (145), ESLint, Prettier, i18n and
git diff --check. The four edited TypeScript files produce identical JavaScript with comments removed.Tested on
Environment (optional)
Node.js 22.17.0. No runtime behavior or UI layout changes.
Risk & Scope
Linked Issues
Part of #10795
Part of #4228
中文说明
这个 PR 做了什么
在旧 Goal hook、judge 与内存 store 退役之后，移除陈旧的 Stop-hook 描述。设置参考、schema 描述、hook 浏览器文档以及 bridge/Web Shell 注释现在描述由运行时调度的 Goal 轮和持久化状态。删除未使用的 locale key 与过时的 Stop-hook 设计文档，并在实现位置保留仍然有效的设计理由：Goal 控制命令为何优先处理，以及已执行命令为何不参与恢复。
为什么需要
#11900 删除了旧 Goal hook、judge 与 store，但文档仍把读者引向已退役的执行路径。Goal 轮与阻塞式 Stop-hook 续跑通过各自独立的路径重置工具调用预算。工作区 Goal 列表还包含驻留会话中已暂停、阻塞的 Goal，并非仅存于内存的运行中 Goal 列表。
#10795 保持开放：兼容投影和历史解析仍被使用。本次文档清理不宣称完成它们的移除，也不改变现有传输格式。
评审验证方式
如何验证
确认 Goal 轮与阻塞式 Stop-hook 续跑被分别描述，且三处设置描述保持一致。hook 浏览器文档不应再把
/goal列为 Session hook 来源。Goal 列表注释应符合持久化快照与实际 Goal 轮数，包括已暂停和阻塞的 Goal。队列注释应解释控制命令为何绕过普通输入，以及 steering 准备取消后为何不恢复已经执行的命令。证据（前后对比）
运行行为不适用：仅修改文档、注释、一条设置描述和未使用的 locale key。本地检查通过：构建、类型检查、CLI 定向测试（333 个）、core 循环检测测试（145 个）、ESLint、Prettier、i18n 与
git diff --check。四个修改的 TypeScript 文件去除注释后生成的 JavaScript 完全一致。测试平台
环境（可选）
Node.js 22.17.0。没有运行行为或 UI 布局变更。
风险与范围
关联 Issue
Part of #10795
Part of #4228