feat(goal): redesign goal lifecycle across clients - #7494
Conversation
E2E verification reportValidated on macOS arm64 with a freshly bundled local CLI and a fresh session.
Automated verification:
|
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
gwinthis
left a comment
gwinthis
left a comment
There was a problem hiding this comment.
Architecture Review — PR #7494
Verdict: ✅ Approve with deep interest — textbook event-sourced state machine replacing inference-based lifecycle.
Architecture overview
The new Goal system follows a clean layered architecture (27 files in packages/core/src/goals/):
goal-protocol.ts → State machine definition (transitions, events, states)
goal-reducer.ts → Pure state reducer (deterministic transitions)
goal-runtime.ts → Runtime orchestration (turn ownership, admission)
goal-persistence.ts → Transcript-backed persistence (replay after restart)
goal-wire.ts → Cross-client wire format (TUI/headless/ACP/WebShell/Desktop)
goal-evidence.ts → Delivered-output evidence tracking
goal-verifier.ts → Independent completion verification
goal-turn-context.ts → Exact turn ownership
activeGoalStore.ts → In-memory active goal state
goal-tools.ts → Model-facing tools
goalHook.ts → Hook integration
goalJudge.ts → Completion judgment
goal-cutover.ts → Legacy → new migration
goal-legacy-projection.ts → Backward compatibility
Key design patterns
1. Explicit state machine > inferred state
Before: lifecycle state was inferred from display cards and Stop hooks — race-prone, divergent across clients.
After: explicit transitions (create, replace, edit, pause, resume, clear, complete, blocked) with exact turn ownership. Every client sees the same authoritative state.
2. Transcript-backed persistence (event sourcing)
Goals are persisted in the transcript, not in ephemeral state. After restart, the authoritative snapshot is restored from the transcript — not from UI state or hook memory. This is the event sourcing pattern: the transcript IS the source of truth.
3. Independent completion verification
goal-verifier.ts verifies completion independently of the model's self-report. The model says "done" → the verifier checks against the Goal's criteria → only then does the state transition to complete. This prevents premature completion.
4. Fail-closed on ownership loss
When persistence or turn ownership is lost, the Goal fails closed (stops) rather than continuing in an ambiguous state. This is consistent with the project's broader fail-closed philosophy (Vision Bridge, Epoch Token).
5. Cross-client consistency via wire format
goal-wire.ts defines the serialization format shared by all five clients. The reducer is pure and client-agnostic; each client renders the same state differently but never diverges.
6. Intentional non-goals
No token budgets, no fixed turn limits. The lifecycle is about state management, not resource limiting. This is explicitly documented in the PR description — good scope discipline.
Observations
-
Legacy migration path:
goal-cutover.ts+goal-legacy-projection.tshandle backward compatibility with legacy Goal display records. Legacy records remain readable but new Goals use the durable lifecycle. -
Queue semantics: ordinary messages stay queued while a Goal owns the model; explicit insertion is admitted at a turn boundary. This separates "interrupt the Goal" from "add to the conversation."
-
Test coverage: 293 Core + 778 CLI + 268 WebShell + 451 SDK + 33 Desktop tests. Integration tests verify the full lifecycle including restart replay.
-
Scale: +31,697 additions across 218 files is large, but the architecture is clean — each file has a single responsibility and the layer boundaries are well-defined.
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "session_goal_control" |
— Qwen Code · serve A/B
TUI + WebShell complete E2E validationValidated commit: ResultAll tested Goal lifecycle and queue interactions passed. No additional source-code fix was required during this pass. TUI interaction chain
TUI — create, automatic continuation, stop condition, verifier completion:
TUI — edit revision, new output, stop condition, terminal completion:
WebShell interaction chainFresh session:
WebShell — Goal created directly from the welcome screen and active strip shown:
WebShell — ordinary message remains queued while the Goal continues:
WebShell — explicit insertion is acknowledged and does not terminate the Goal:
WebShell — pause waits at the turn boundary:
WebShell — edited Goal resumes with the revised objective:
WebShell — inserted stop condition is verified and the Goal reaches terminal state:
WebShell —
Automated regression evidence
Timing observationThe client remained in the running state and scheduled the next turn automatically. Visible gaps between replies were model/request latency while the processing indicator remained active, not a stopped Goal loop requiring user interaction. |
|
Thanks for the PR — this is a substantial redesign of the Goal lifecycle. Template looks good ✓ Problem: The PR describes concrete race conditions in the existing hook-driven Goal implementation — completed Goals leaving unsent queued messages, insertion interrupting the loop, turns continuing after terminal completion, UI state diverging from the transcript. These are plausible failure modes for a hook-inferred lifecycle. That said, there are no linked issues or user reports demonstrating these bugs in practice. For a Direction: Aligned. Claude Code ships Size: This is a very large PR — 10,952 production lines across 137 files (of which 1,590 core production lines in Approach: The core idea — replacing hook-inferred lifecycle with explicit create/replace/edit/pause/resume/clear/complete/blocked transitions, turn ownership, and transcript-backed evidence — is sound. The scope, however, is enormous for a single PR: it simultaneously rewrites the core Goal engine, updates the TUI, adds WebShell composer controls and Goal management views, adds Desktop Goal UI, extends the SDK, and modifies the ACP bridge. Have you considered splitting this into (1) core lifecycle + TUI, (2) WebShell/Desktop UI, (3) SDK/ACP protocol? Each would be independently reviewable and revertable. Flagging for maintainer awareness given the core-path scale and cross-package breadth. Moving on to code review. 🔍 中文说明感谢贡献！这是一次对 Goal 生命周期的大规模重新设计。 模板完整 ✓ 问题： PR 描述了现有 Hook 驱动 Goal 实现中的具体竞态条件——Goal 完成后留下未发送的排队消息、插入意外中断循环、终态完成后继续模型轮次、UI 状态与持久化记录不一致。这些对于 Hook 推断的生命周期来说是合理的故障模式。不过，没有关联的 issue 或用户报告来证明这些 bug 实际发生过。对于 方向： 对齐。Claude Code 已发布 规模： 这是一个非常大的 PR——10,952 行生产代码，涉及 137 个文件（其中 1,590 行核心生产代码 在 方案： 核心思路——用显式的 create/replace/edit/pause/resume/clear/complete/blocked 状态转换、轮次所有权和基于记录的证据替代 Hook 推断的生命周期——是合理的。但范围对于单个 PR 来说过大：同时重写了核心 Goal 引擎、更新了 TUI、新增了 WebShell 输入框控制和 Goal 管理界面、新增了 Desktop Goal UI、扩展了 SDK、修改了 ACP bridge。是否考虑过拆分为 (1) 核心生命周期 + TUI，(2) WebShell/Desktop UI，(3) SDK/ACP 协议？每部分都可以独立审查和回滚。 因核心路径规模和跨包广度，标记维护者关注。进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal vs. PR approach: I would have built the same core — an explicit Goal state machine with versioned snapshots, turn permits, and transcript-backed evidence in Architecture is sound. The new No critical blockers found. The code follows project conventions (ESM, strict TS, kebab-case, collocated tests). The 165 new Goal tests all pass. Build succeeds. One observation: Files changed (30 of 218 shown)
Real-Scenario Testingtmux is not available on this CI runner, so I used headless mode with Before (installed qwen 0.20.0)After (this PR via npm run dev)Unit tests (core Goal module)Build中文说明代码审查独立方案 vs. PR 方案： 我会构建相同的核心——在 架构合理。 新的 未发现关键阻塞问题。 代码遵循项目约定。165 个新 Goal 测试全部通过。构建成功。 一个观察： 真实场景测试此 CI 环境无 tmux，使用无头模式 修改前（已安装 qwen 0.20.0）： 旧的 Hook 驱动行为——单轮，无生命周期事件。模型直接确认并回复。 修改后（本 PR）： 显式生命周期——Goal 创建 → 活动状态转换 → get_goal 读取目标 → 交付内容 → 获取证据 UUID → update_goal 提出完成 → 独立验证排队。 单元测试： 9 个测试文件，165 个测试全部通过。 构建： 成功。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 3/5 — clean review and the architecture is genuinely better than what it replaces, but 1,590 core production lines across 6 packages from a fork needs a maintainer's sign-off. The core redesign is the right call. Replacing hook-inferred Goal state with an explicit, versioned state machine ( My reservation is purely about scope, not quality. This PR simultaneously rewrites the Goal engine, updates the TUI, adds WebShell and Desktop Goal UI, extends the SDK, and modifies the ACP bridge — 218 files, ~11K production lines. Each of those client integrations could land as a follow-up once the core lifecycle is reviewed and stable. Splitting would make each piece independently reviewable and revertable, and would let the core engine get battle-tested before the UI layers build on it. Deferring to the maintainer for the scope call. The code itself is ready; the question is whether it should land as one PR or several. 中文说明信心：3/5 — 审查干净，架构确实比被替代的方案更好，但来自 fork 的 1,590 行核心生产代码横跨 6 个包，需要维护者签字。 核心重新设计是正确的。用显式、带版本的状态机替代 Hook 推断的 Goal 状态，从构造上而非约定上消除了整类竞态条件。独立验证器使用结构化 schema 和不可信证据处理，设计周到。165 个测试全面，构建通过，无头端到端测试展示了完整生命周期：创建 → 轮次许可 → get_goal → 交付内容 → 证据捕获 → update_goal → 验证排队。 我的保留意见纯粹关于范围，而非质量。这个 PR 同时重写了 Goal 引擎、更新了 TUI、新增了 WebShell 和 Desktop Goal UI、扩展了 SDK、修改了 ACP bridge——218 个文件，约 11K 行生产代码。每个客户端集成都可以作为后续 PR，在核心生命周期审查稳定后再落地。拆分后每部分都可以独立审查和回滚，也让核心引擎在 UI 层构建之前先经过实战检验。 转交维护者做范围决定。代码本身已就绪；问题是应该作为一个 PR 还是多个 PR 落地。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
⏸️ Deferring to @wenshao @tanzhenxin @yiliang114 @LaZzyMan — this PR touches 1,590 core production lines across 6 packages (218 files total, ~11K production lines). The code quality is solid and the architecture is a clear improvement, but the cross-package scope from a fork contributor needs a maintainer's call on whether to land as one PR or split into phases (core + TUI first, then WebShell/Desktop/SDK/ACP). Needs a human decision on this one. |
TUI tmux E2E reportThis is a fresh end-to-end TUI pass for the Goal v3 interaction model. Every scenario ran in a new tmux session with an isolated Environment: macOS arm64, Node.js 25.9.0, Qwen Code 0.20.1, Result summary
Bug found and fixed during this passReproduction: pause an automatically continuing Goal, queue a normal user message, submit Fix: treat Interaction evidence1. Finite automatic loop completes without a tail turn
2. Ctrl+Q holds a message while the current Goal turn is active
3. Queued input drains at the CLI turn boundary; clear needs no confirmation
4. Pause, edit, resume: revision 2 and the new objective are used
5. Stop-condition input reaches verifier and leaves no active Goal
6. Paused Goal restores across a real process restart
7. Enter steers the active Goal; the edited objective remains in force
8. Invalid controls produce concise errors
Automated verification
CLI/WebShell behavior boundaryThe CLI intentionally exposes two keyboard paths while a Goal is running: Enter is Observed timing: normal automatic turns continued without an artificial safety delay. Independent terminal verification can take model/provider time (roughly 10–20 seconds in these runs), but no fixed 60-minute window, 50-turn Goal cap, or Goal-specific wall-clock timeout was observed. |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Thanks for the substantial lifecycle redesign and the follow-up hardening pass. The ownership and trust boundaries look solid, and the latest commit fixes the earlier cross-client recovery issues I checked. I am requesting changes for four remaining correctness/scalability blockers; details are inline. Non-blocking test and simplification notes are inline as well. CI note: the current Ubuntu job also exits after the CLI Vitest worker exceeds its heap limit, despite the reported assertions passing, so the branch still needs a green CI run before merge.
| this.turnParentUuids.push(this.lastRecordUuid); | ||
| const record: ChatRecord = { | ||
| ...this.createBaseRecord('user'), | ||
| subtype: 'goal_runtime', |
There was a problem hiding this comment.
[P1] Hide Goal runtime prompts from replay presentation
recordGoalRuntimeMessage deliberately persists the model-facing continuation as a user record, but both DefaultTranscriptReplayMachine.projectUserRecord and buildResumedHistoryItems treat every other user subtype as displayable. After any automatic Goal turn, /resume and paged daemon history (and therefore WebShell/Desktop history) emit the internal Continue working on the active Goal... prompt as if the user typed it, while live delivery emits no corresponding user chunk. Please keep this record for model/evidence reconstruction but explicitly exclude goal_runtime in both presentation projectors, with TUI and ACP replay contract tests.
| restore(records: readonly ChatRecord[]): Promise<void> { | ||
| return enqueue(async () => { | ||
| assertAvailable(); | ||
| if (restored) return; |
There was a problem hiding this comment.
[P1] Rebase the Goal runtime when rewinding
This restore is only usable during initialization, but both ACP and TUI rewind only truncate model history and re-root ChatRecordingService; neither updates the live Goal runtime. Rewinding a paused or terminal Goal to before its create record leaves the current process serving the removed Goal, while restarting recovers a different state. Resuming the stale Goal can then point evidenceCursor at the dead branch and end in cursor_not_found/usage_limited. Please make rewind atomically rebase or reset the runtime from the surviving active chain (or persist an explicit reset and update the runtime), and cover both clients.
| goalContext: GoalTurnPermit, | ||
| ): void { | ||
| try { | ||
| this.turnParentUuids.push(this.lastRecordUuid); |
There was a problem hiding this comment.
[P1] Do not count automatic Goal prompts as selectable user turns
Pushing this record into turnParentUuids makes an automatic continuation a top-level rewind boundary, while the TUI and ACP truncation mappers count plain role: user API contents and have no subtype metadata. For U0 -> A0 -> U_goal -> A_goal -> U1, selecting the visible U1 can truncate before U_goal and discard A_goal; in both mode the files and conversation can land on different boundaries. Please use one explicit real-user-turn mapping across UI, API history, and recorder instead of counting all user text, and add a Goal-plus-rewind contract test.
| for (let index = cursorIndex + 1; index < input.records.length; index++) { | ||
| const evidence = eligibleEvidence(input.records[index]!, input); | ||
| if (!evidence) continue; | ||
| catalog.push(stripContent(evidence)); |
There was a problem hiding this comment.
[P1] Bound the total Goal evidence payload
This catalog grows with every eligible record after the cursor; the per-entry preview cap does not limit entry count, total bytes, or lineage IDs. Since every Goal turn is told to call get_goal and Goals have no fixed turn/token budget, repeated payload and scan work grows quadratically; 1,000 ordinary entries already produce roughly 355 KB of JSON per call. Terminal verification is unbounded too: evidenceRefs has no maxItems, and every cited record is expanded to full content before verifier JSON is built. Please add explicit catalog/byte and verifier reference/evidence budgets, with defined truncation or pagination semantics.
| return ( | ||
| value === undefined || | ||
| (isRecord(value) && | ||
| hasOnlyKeys(value, ['fingerprint', 'count', 'turnIds']) && |
There was a problem hiding this comment.
[Suggestion] Add negative parser cases for blockedAudit
This strict persisted shape protects the three-consecutive-turn blocker gate, but the parser tests currently cover only an audit-free valid payload. Could we add table-driven cases for count 0/4, turnIds/count mismatch, empty fingerprint or turn ID, and extra keys? That would catch accidental loosening of this recovery boundary.
| validateBlockerCoverage(input.proposal, citedRecords, analysis); | ||
|
|
||
| return { | ||
| catalog: analysis.catalog.map((entry) => ({ ...entry })), |
There was a problem hiding this comment.
[Non-blocking / Ponytail] ValidatedGoalEvidence.catalog and .lineageTurnIds are cloned and returned here, but production only reads citedRecords. Returning only citedRecords removes dead API surface and avoids copying the already-growing catalog during verification.
| ): Promise<ChatRecord>; | ||
| } | ||
|
|
||
| export async function recordMigratedGoalState( |
There was a problem hiding this comment.
[Non-blocking / Ponytail] recordMigratedGoalState and GoalStateRecorder have no production caller; only their dedicated test uses this wrapper, while production calls createMigratedGoalState directly. Deleting the wrapper, interface, and test removes about 39 lines without changing behavior.
| } | ||
| }); | ||
|
|
||
| it('removes the legacy Goal runtime modules', () => { |
There was a problem hiding this comment.
[Non-blocking / Ponytail] These path- and identifier-spelling assertions do not protect a stable behavior: harmless renames fail them, while equivalent legacy behavior under another name passes. Compile/import checks plus the behavioral Goal suites cover the real cutover. Keeping the public-export assertion if desired and deleting the remaining core/CLI cutover checks removes roughly 55 lines.
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. Not reviewed: chunk 6, chunk 7, chunk 8, chunk 9, chunk 10, chunk 11, chunk 17, chunk 18, chunk 19, chunk 20, chunk 21, chunk 22, chunk 24, chunk 25, chunk 26, chunk 27, chunk 28, chunk 29, chunk 30, chunk 31, chunk 32, chunk 34, chunk 35, chunk 36, chunk 37, chunk 38, chunk 39, chunk 40, chunk 41, chunk 42, chunk 43, chunk 49, chunk 50, chunk 51, chunk 53, chunk 54, chunk 55, chunk 56, chunk 57, chunk 58, chunk 63, chunk 64, chunk 68, chunk 90, chunk 91, chunk 93, chunk 94, chunk 95, chunk 98, chunk 100, chunk 102, chunk 103, chunk 104, chunk 105, chunk 106, chunk 109, chunk 110, chunk 111, chunk 112, chunk 113, chunk 127, chunk 128, chunk 129, chunk 130 — no agent reported covering these; nobody read them. Not reviewed: chunk agents (1-130) — not launched due to PR scale (153 agents required, practical limit reached with whole-diff + invariant agents covering the 6 heavy files and key cross-file traces). Not reviewed: Agent 0: Issue fidelity & root-cause ownership, chunk 1, chunk 2, chunk 3, chunk 4, chunk 5, chunk 6, chunk 7, chunk 8, chunk 9, chunk 10, chunk 11, chunk 12, chunk 13, chunk 14, chunk 15, chunk 16, chunk 17, chunk 18, chunk 19, chunk 20, chunk 21, chunk 22, chunk 23, chunk 24, chunk 25, chunk 26, chunk 27, chunk 28, chunk 29, chunk 30, chunk 31, chunk 32, chunk 33, chunk 34, chunk 35, chunk 36, chunk 37, chunk 38, chunk 39, chunk 40, chunk 41, chunk 42, chunk 43, chunk 44, chunk 45, chunk 46, chunk 47, chunk 48, chunk 49, chunk 50, chunk 51, chunk 52, chunk 53, chunk 54, chunk 55, chunk 56, chunk 57, chunk 58, chunk 59, chunk 60, chunk 61, chunk 62, chunk 63, chunk 64, chunk 65, chunk 66, chunk 67, chunk 68, chunk 69, chunk 70, chunk 71, chunk 72, chunk 73, chunk 74, chunk 75, chunk 76, chunk 77, chunk 78, chunk 79, chunk 80, chunk 81, chunk 82, chunk 83, chunk 84, chunk 85, chunk 86, chunk 87, chunk 88, chunk 89, chunk 90, chunk 91, chunk 92, chunk 93, chunk 94, chunk 95, chunk 96, chunk 97, chunk 98, chunk 99, chunk 100, chunk 101, chunk 102, chunk 103, chunk 104, chunk 105, chunk 106, chunk 107, chunk 108, chunk 109, chunk 110, chunk 111, chunk 112, chunk 113, chunk 114, chunk 115, chunk 116, chunk 117, chunk 118, chunk 119, chunk 120, chunk 121, chunk 122, chunk 123, chunk 124, chunk 125, chunk 126, chunk 127, chunk 128, chunk 129, chunk 130, Test coverage matrix (whole-diff), Agent 1b: Removed-behavior audit, Agent 1c: Cross-file tracer, Invariant agent A: state, timers, collections — packages/cli/src/acp-integration/session/Session.ts, Invariant agent B: counters, return values, error taxonomies — packages/cli/src/acp-integration/session/Session.ts, Invariant agent C: config fields, early returns — packages/cli/src/acp-integration/session/Session.ts, Invariant agent A: state, timers, collections — packages/cli/src/acp-integration/session/history-replayer.ts, Invariant agent B: counters, return values, error taxonomies — packages/cli/src/acp-integration/session/history-replayer.ts, Invariant agent C: config fields, early returns — packages/cli/src/acp-integration/session/history-replayer.ts, Invariant agent A: state, timers, collections — packages/cli/src/ui/hooks/useGeminiStream.ts, Invariant agent B: counters, return values, error taxonomies — packages/cli/src/ui/hooks/useGeminiStream.ts, Invariant agent C: config fields, early returns — packages/cli/src/ui/hooks/useGeminiStream.ts, Invariant agent A: state, timers, collections — packages/core/src/core/client.ts, Invariant agent B: counters, return values, error taxonomies — packages/core/src/core/client.ts, Invariant agent C: config fields, early returns — packages/core/src/core/client.ts, Invariant agent A: state, timers, collections — packages/core/src/core/coreToolScheduler.ts, Invariant agent B: counters, return values, error taxonomies — packages/core/src/core/coreToolScheduler.ts, Invariant agent C: config fields, early returns — packages/core/src/core/coreToolScheduler.ts, Invariant agent A: state, timers, collections — packages/web-shell/client/components/dialogs/GoalsDialog.tsx, Invariant agent B: counters, return values, error taxonomies — packages/web-shell/client/components/dialogs/GoalsDialog.tsx, Invariant agent C: config fields, early returns — packages/web-shell/client/components/dialogs/GoalsDialog.tsx — its prompt was built, but no agent on record was launched with it. Not reviewed: reverse audit — no auditor was launched with a prompt this skill builds — the pass that hunts what the rest of the review missed ran, if at all, without the method its brief carries. Not reviewed: verification — a verifier ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and the posted findings cannot be counted as verified against it. [Critical] packages/cli/src/acp-integration/acpAgent.ts: The bulk replay path in loadSession does not forward replayPage.goalState to collectHistoryReplayUpdates. The resume path correctly passes it. When an ACP client bulk-loads a session whose history exceeds the page size, the HistoryReplayer starts without pre-existing Goal state, and the TranscriptReplayMachine may project incorrect legacy metadata. Fix: add goalState: replayPage.goalState to the collectHistoryReplayUpdates call in the live-session branch.
— qwen3.7-max via Qwen Code /review
| if (isGoalControlPrompt) { | ||
| return this.#executePrompt(params, new AbortController()); | ||
| } |
There was a problem hiding this comment.
[Critical] The /goal control-prompt path creates a throwaway AbortController that is never stored on any session field (pendingPrompt, goalAbortController, or otherwise), making the resulting #executePrompt call unreachable by cancelPendingPrompt(). — Failure scenario: A user sends /goal then immediately cancels. cancelPendingPrompt() snapshots hadPrompt = false (the /goal path never sets this.pendingPrompt) and hadGoal = false (no automatic goal queue entry exists for a /goal command). Cancel throws NOT_CURRENTLY_GENERATING_CANCEL_MESSAGE, even though #executePrompt is actively running. The /goal handler proceeds to completion — calling #queueGoalStateEmission and awaiting this.goalEmissionTail — after the client was told nothing was generating. The goal state emission lands on the wire after the cancel acknowledgement, producing a surprise state update the client believed it had stopped.
| if (isGoalControlPrompt) { | |
| return this.#executePrompt(params, new AbortController()); | |
| } | |
| if (isGoalControlPrompt) { | |
| const pendingSend = new AbortController(); | |
| this.pendingPrompt = pendingSend; | |
| try { | |
| return await this.#executePrompt(params, pendingSend); | |
| } finally { | |
| if (this.pendingPrompt === pendingSend) this.pendingPrompt = null; | |
| } | |
| } |
— qwen3.7-max via Qwen Code /review
| const control = useCallback( | ||
| async (item: DaemonGoal, action: 'pause' | 'resume' | 'clear') => { | ||
| const goal = item.snapshot.goal; | ||
| if (!goal) return; |
There was a problem hiding this comment.
[Suggestion] The old handleClear required window.confirm() before clearing a goal; the new unified control function dispatches 'clear' with no confirmation, and the clearConfirm i18n key was removed from translations. A user who clicks the trash icon accidentally (e.g., mis-aiming for the adjacent pause/resume button) immediately sends a clear command to the daemon with no recovery path. Pause and resume are reversible; clear is destructive and irreversible within this UI. Consider adding a confirmation step for the 'clear' action only — either re-introduce a window.confirm guard inside control when action === 'clear', or add a confirmation at the button's onClick handler.
— qwen3.7-max via Qwen Code /review
Size review — suggest splittingThis PR is +32,510 / −10,983 across 231 files. The total diff (~43k lines) far exceeds GitHub's 20k-line limit, so the diff cannot be loaded in the GitHub UI or via the API. Reviewing a cross-client lifecycle replacement at this size is not feasible in a single pass. I understand this is harder to split than a purely additive feature — the PR replaces the legacy hook-driven Goal system rather than extending it, so core and at least the primary consumer (CLI/TUI) must land together to avoid a broken intermediate state. That said, an expand → migrate → contract sequence can keep every PR under the 20k diff limit:
Key observations from the diff:
One more note: this PR modifies 51 files in 中文本 PR 共 +32,510 / −10,983，涉及 231 个文件，diff 总量约 43k 行，远超 GitHub 的 20k 行限制，无法在 GitHub UI 或 API 中加载。 理解这是替换式重设计，比纯新增更难拆。但可以用 expand → migrate → contract 模式拆成 4 个 PR，每个都在 20k 限制内：
关键数据：
另外：本 PR 修改了 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Unresolved, please confirm: [Critical] packages/core/src/goals/goal-runtime.ts — @yiliang114 P1: Goal runtime not rebased on rewind. Requires tracing rewind → Goal runtime integration across multiple files. [Critical] packages/core/src/goals/goal-evidence.ts — @yiliang114 P1: Goal evidence catalog unbounded. Catalog grows with every eligible record; no entry count, byte, or lineage ID limit. Not reviewed: chunk 1, chunk 2, chunk 3, chunk 4, chunk 5, chunk 6, chunk 7, chunk 8, chunk 9, chunk 10, chunk 11, chunk 12, chunk 21, chunk 22, chunk 23, chunk 24, chunk 25, chunk 26, chunk 27, chunk 28, chunk 29, chunk 30, chunk 31, chunk 32, chunk 33, chunk 34, chunk 35, chunk 36, chunk 37, chunk 38, chunk 39, chunk 40, chunk 41, chunk 42, chunk 43, chunk 44, chunk 45, chunk 46, chunk 47, chunk 48, chunk 49, chunk 50, chunk 51, chunk 52, chunk 53, chunk 54, chunk 55, chunk 56, chunk 57, chunk 58, chunk 59, chunk 60, chunk 61, chunk 62, chunk 63, chunk 64, chunk 65, chunk 66, chunk 67, chunk 76, chunk 77, chunk 78, chunk 79, chunk 80, chunk 81, chunk 82, chunk 83, chunk 84, chunk 85, chunk 86, chunk 87, chunk 88, chunk 89, chunk 90, chunk 91, chunk 92, chunk 93, chunk 94, chunk 95, chunk 96, chunk 97, chunk 98, chunk 99, chunk 100, chunk 101, chunk 102, chunk 103, chunk 104, chunk 105, chunk 106, chunk 107, chunk 108, chunk 109, chunk 110, chunk 111, chunk 112, chunk 113, chunk 114, chunk 115, chunk 116, chunk 117, chunk 118, chunk 119, chunk 120, chunk 121, chunk 122, chunk 123, chunk 124, chunk 126, chunk 127, chunk 128, chunk 129, chunk 130, chunk 131, chunk 132, chunk 133 — no agent reported covering these; nobody read them. Not reviewed: chunk agents (1-133) — PR scale: 156 agents required, focused review on cross-file traces, build/test, and invariant agents for 3 heavy files. Not reviewed: Agent 0: Issue fidelity — not launched (no linked issues, feature PR). Not reviewed: Test coverage matrix — not launched. Not reviewed: verification — no new findings to verify. Not reviewed: Agent 0: Issue fidelity & root-cause ownership, chunk 1, chunk 2, chunk 3, chunk 4, chunk 5, chunk 6, chunk 7, chunk 8, chunk 9, chunk 10, chunk 11, chunk 12, chunk 13, chunk 14, chunk 15, chunk 16, chunk 17, chunk 18, chunk 19, chunk 20, chunk 21, chunk 22, chunk 23, chunk 24, chunk 25, chunk 26, chunk 27, chunk 28, chunk 29, chunk 30, chunk 31, chunk 32, chunk 33, chunk 34, chunk 35, chunk 36, chunk 37, chunk 38, chunk 39, chunk 40, chunk 41, chunk 42, chunk 43, chunk 44, chunk 45, chunk 46, chunk 47, chunk 48, chunk 49, chunk 50, chunk 51, chunk 52, chunk 53, chunk 54, chunk 55, chunk 56, chunk 57, chunk 58, chunk 59, chunk 60, chunk 61, chunk 62, chunk 63, chunk 64, chunk 65, chunk 66, chunk 67, chunk 68, chunk 69, chunk 70, chunk 71, chunk 72, chunk 73, chunk 74, chunk 75, chunk 76, chunk 77, chunk 78, chunk 79, chunk 80, chunk 81, chunk 82, chunk 83, chunk 84, chunk 85, chunk 86, chunk 87, chunk 88, chunk 89, chunk 90, chunk 91, chunk 92, chunk 93, chunk 94, chunk 95, chunk 96, chunk 97, chunk 98, chunk 99, chunk 100, chunk 101, chunk 102, chunk 103, chunk 104, chunk 105, chunk 106, chunk 107, chunk 108, chunk 109, chunk 110, chunk 111, chunk 112, chunk 113, chunk 114, chunk 115, chunk 116, chunk 117, chunk 118, chunk 119, chunk 120, chunk 121, chunk 122, chunk 123, chunk 124, chunk 125, chunk 126, chunk 127, chunk 128, chunk 129, chunk 130, chunk 131, chunk 132, chunk 133, Test coverage matrix (whole-diff), Agent 1b: Removed-behavior audit, Agent 1c: Cross-file tracer, Agent 7: Build & test verification, Invariant agent A: state, timers, collections — packages/cli/src/acp-integration/session/Session.ts, Invariant agent B: counters, return values, error taxonomies — packages/cli/src/acp-integration/session/Session.ts, Invariant agent C: config fields, early returns — packages/cli/src/acp-integration/session/Session.ts, Invariant agent A: state, timers, collections — packages/cli/src/acp-integration/session/history-replayer.ts, Invariant agent B: counters, return values, error taxonomies — packages/cli/src/acp-integration/session/history-replayer.ts, Invariant agent C: config fields, early returns — packages/cli/src/acp-integration/session/history-replayer.ts, Invariant agent A: state, timers, collections — packages/cli/src/ui/hooks/useGeminiStream.ts, Invariant agent B: counters, return values, error taxonomies — packages/cli/src/ui/hooks/useGeminiStream.ts, Invariant agent C: config fields, early returns — packages/cli/src/ui/hooks/useGeminiStream.ts, Invariant agent A: state, timers, collections — packages/core/src/core/client.ts, Invariant agent B: counters, return values, error taxonomies — packages/core/src/core/client.ts, Invariant agent C: config fields, early returns — packages/core/src/core/client.ts, Invariant agent A: state, timers, collections — packages/core/src/core/coreToolScheduler.ts, Invariant agent B: counters, return values, error taxonomies — packages/core/src/core/coreToolScheduler.ts, Invariant agent C: config fields, early returns — packages/core/src/core/coreToolScheduler.ts, Invariant agent A: state, timers, collections — packages/web-shell/client/components/dialogs/GoalsDialog.tsx, Invariant agent B: counters, return values, error taxonomies — packages/web-shell/client/components/dialogs/GoalsDialog.tsx, Invariant agent C: config fields, early returns — packages/web-shell/client/components/dialogs/GoalsDialog.tsx — its prompt was built, but no agent on record was launched with it. Not reviewed: reverse audit — no auditor was launched with a prompt this skill builds — the pass that hunts what the rest of the review missed ran, if at all, without the method its brief carries.
— qwen3.7-max via Qwen Code /review
|
This PR is being split into focused, dependency-ordered changes so each contract and surface can be reviewed independently. Please treat #7494 as the umbrella/reference diff rather than the merge target while the split is in progress. The first slice is now available as #7517: the versioned Goal state protocol, deterministic reducer, persistence/recovery contract, legacy migration, and compatibility projection. Planned follow-ups are: evidence and verifier; runtime and tools; recording/replay/rewind; core engine integration; TUI; non-interactive CLI; ACP/serve; SDK/WebUI; WebShell; Desktop; and final legacy cleanup. Each UI slice will carry its own E2E evidence and screenshots. |
5 participants
What this PR does
This replaces the legacy hook-driven Goal loop with a durable, versioned lifecycle shared by the TUI, headless CLI, ACP/daemon, WebShell, and Desktop. Goals now have explicit create, replace, edit, pause, resume, clear, complete, and blocked transitions; exact turn ownership; transcript-backed evidence; independent completion verification; and authoritative replay after restart.
The user experience is aligned across clients without adding token or fixed-turn budgets. Ordinary messages remain queued while a Goal owns the model, explicit insertion is admitted at a turn boundary, terminal Goal updates end the current model turn without a trailing reply, and clearing a Goal is immediate. WebShell and Desktop receive compact composer controls and Goal management views while the TUI retains its existing visual language.
Why it's needed
The previous Goal implementation inferred lifecycle state from display cards and Stop hooks. That made completion, resume, queued-message insertion, persistence, and cross-client rendering race-prone: a completed Goal could leave an unsent queued message, insertion could interrupt the Goal unexpectedly, a turn could continue after terminal completion, and UI state could diverge from the durable transcript.
This design gives every client the same authoritative state and concurrency contract. It separates ordinary queued prompts from explicit insertion, preserves exact delivered-output evidence, fails closed when persistence or turn ownership is lost, and removes the historical 50-turn behavior from the Goal lifecycle.
Reviewer Test Plan
How to verify
/goal Reply test until I type qqq. Confirm the Goal begins immediately, repeats onlytest, and remains active without a token or fixed-turn budget.qqq. Confirm the user input is durably recorded, completion is independently verified, the Goal becomes complete, and no extra Goal response or model turn appears after the terminal update.Evidence (Before & After)
Before: Goal completion could be followed by an extra model turn, queued insertion could be rejected or interrupt the loop, and legacy display-card replay could revive stale state. After: independent CLI E2E session
c2bee17f-b724-4904-af84-d756a0d590b8completed Goal852ef05e-ea4d-472d-bbe2-a8e68c333a6din four exact outputs (J,X,R,F), the verifier accepted the completion, and no fifth Goal turn or trailing reply was emitted. Focused verification passed 293 Core tests, 778 CLI tests (1 skipped), 268 WebShell tests, 451 SDK tests, and 33 Desktop tests; the full build and workspace typecheck also passed.Tested on
Environment (optional)
macOS arm64, Node.js 25.9.0 for repository validation, local bundled CLI for the independent Goal E2E, no sandbox.
Risk & Scope
Linked Issues
N/A
中文说明
这个 PR 做了什么
本 PR 将旧的、由 Hook 驱动的 Goal 循环替换为一个持久化、带版本的生命周期，并在 TUI、无头 CLI、ACP/daemon、WebShell 和 Desktop 之间共享。Goal 现在具有明确的创建、替换、编辑、暂停、恢复、清除、完成和阻塞状态转换，同时具备精确的轮次所有权、基于会话记录的证据、独立完成验证，以及重启后的权威状态回放。
各客户端的用户体验已完成对齐，但没有新增 token 预算或固定轮数预算。Goal 占用模型时，普通消息会继续保持排队；只有显式点击插入后，消息才会在安全的轮次边界进入；终态 Goal 更新会结束当前模型轮次，不再产生额外回复；清除 Goal 会立即执行。WebShell 和 Desktop 增加了紧凑的输入框控制条与 Goal 管理界面，TUI 则保留原有视觉风格。
为什么需要它
旧 Goal 实现通过展示卡片和 Stop Hook 推断生命周期状态。这使完成、恢复、排队消息插入、持久化和跨客户端渲染容易发生竞态：Goal 完成后可能留下未发送的排队消息，插入可能意外中断 Goal，终态完成后仍可能继续一轮，UI 状态也可能与持久化会话记录不一致。
新设计为所有客户端提供同一个权威状态与并发契约。它把普通排队消息与显式插入分离，保留精确的已交付输出证据，在持久化或轮次所有权丢失时关闭失败，并从 Goal 生命周期中移除了历史上的 50 轮行为。
Reviewer 测试计划
如何验证
/goal Reply test until I type qqq。确认 Goal 立即开始，只重复输出test，并且保持进行中，不展示 token 或固定轮数预算。qqq。确认用户输入被持久化记录，完成状态经过独立验证，Goal 进入完成态，并且终态更新后不再出现额外 Goal 回复或模型轮次。证据（修改前与修改后）
修改前：Goal 完成后可能继续额外模型轮次，排队消息插入可能被拒绝或中断循环，旧展示卡片回放还可能恢复过期状态。修改后：独立 CLI 端到端会话
c2bee17f-b724-4904-af84-d756a0d590b8中，Goal852ef05e-ea4d-472d-bbe2-a8e68c333a6d用四次精确输出（J、X、R、F）完成，验证器接受完成结果，并且没有出现第五轮 Goal 或尾随回复。定向验证通过 293 个 Core 测试、778 个 CLI 测试（1 个跳过）、268 个 WebShell 测试、451 个 SDK 测试和 33 个 Desktop 测试；完整构建与全工作区类型检查也均通过。已测试平台
环境（可选）
macOS arm64；仓库验证使用 Node.js 25.9.0；独立 Goal 端到端验证使用本地打包 CLI；未使用 sandbox。
风险与范围
关联 Issue
无