feat(core): add Goal v3 state protocol - #7517
Conversation
|
Re-running the full gate at the current head ( Template looks good ✓ Problem: this is a Direction: aligned. Size: touches core ( Approach: scope feels right for a contract-first slice. Pure logic, no runtime wiring, no drive-by refactors, preserves the existing Goal surface. The public API surface ( Moving on to code review. 🔍 中文说明在当前 head（ 模板完整 ✓ 问题：这是一个 方向：对齐。 规模：触及核心（ 方案：对于契约先行的切片，范围合理。纯逻辑、无 runtime 接入、无顺手重构、保留现有 Goal 接口。公共 API 面（ 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewRe-reviewed the full diff at the current head (10 files, +1834). My independent proposal for a versioned Goal contract: version constant, typed snapshot/record/control-request shapes, a pure reducer with optimistic concurrency (expected id + revision), strict parsers rejecting unknown keys and stale versions, newest-first recovery stopping on a malformed newest record, fresh-identity legacy migration, and a backward-compatible projection. The PR does all of this cleanly — pure logic, no runtime wiring, colocated tests. No critical blockers. Conventions are solid: ESM, kebab-case files, license headers, colocated Files changed (10 of 10)
Real-Scenario TestingThis slice is a non-UI state/persistence contract with no runtime wired up — no TUI path to drive, so before/after terminal output is N/A (the PR says the same). The meaningful check for a protocol slice: build the package, import the public subpath through the export map, and exercise documented invariants against the built artifact. Done in an isolated worktree at the PR head: All documented invariants hold against the built artifact. Existing Goal store/hook/judge/loop tests stay green (153 total). tmux was unavailable on this CI runner; the protocol-level verification above covers the meaningful surface for a non-UI slice. 中文说明代码审查在当前 head 重新审查了完整 diff（10 个文件，+1834）。读 diff 前我的独立方案：版本常量、类型化的 snapshot/record/control-request 形状、带乐观并发的纯 reducer、拒绝未知键和过期版本的严格解析器、遇到畸形最新记录即停止的恢复、全新身份的旧状态迁移，以及回到旧形状的兼容投影。PR 完整实现了这些，做得干净——纯逻辑、无 runtime 接入、测试就近放置。 无关键阻塞项。约定扎实：ESM、kebab-case 文件名、license 头、就近 真实场景测试本切片是非 UI 的状态/持久化契约，尚未接入任何 runtime——无可驱动的 TUI 路径，before/after 终端输出不适用。对协议切片有意义的检查：构建包、通过 export map 导入公共子路径、对构建产物验证文档化不变量。已在 PR head 的隔离 worktree 中完成（见上方终端输出）。 所有文档化不变量在构建产物上均成立。现有 Goal store/hook/judge/loop 测试保持全绿（共 153 项）。本 CI 运行器无 tmux；上述协议级验证覆盖了非 UI 切片的有意义表面。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean review at every stage, maintainer has reviewed the full diff, run local verification, and approved. Stepping back: this is a well-built contract-first slice. The approach matches what I'd propose independently — a pure, versioned contract with optimistic concurrency, strict parsing, and careful recovery/migration — and the fix commits since the initial review addressed @wenshao's feedback (the paused-legacy recovery change is load-bearing). Typecheck, build, and the public The reason this is a 4 rather than 5: it's ~955 production lines of new core infrastructure defining public API surface before runtime adoption, so the contract may still shift once real integration lands. That's inherent to the strategy and the maintainer has signed off on it. The open forward-integration items (projection ↔ canonical @wenshao reviewed the full diff, ran local verification (typecheck + build + 149 tests + public subpath + A/B on fix commits), and approved: "No issues found. LGTM! ✅". Approving on the maintainer's sign-off. ✅ 中文说明置信度：4/5 —— 每个阶段审查都很干净，维护者已审查完整 diff、跑了本地验证并批准。 退一步看：这是一个构建良好的契约先行切片。方案与我独立提出的一致——带版本、带乐观并发、严格解析、并有谨慎恢复/迁移路径的纯契约——初次审查后的修复 commit 处理了 @wenshao 的反馈（暂停旧状态恢复的改动是承重的）。typecheck、build 和公共 之所以是 4 而非 5：这是约 955 行新增核心基础设施，在 runtime 接入前定义了公共 API 面，真正集成后契约仍可能变动。这是该策略的固有属性，维护者已签字认可。开放的前向集成事项（投影 ↔ 规范 @wenshao 审查了完整 diff、跑了本地验证（typecheck + build + 149 测试 + 公共子路径 + 修复提交 A/B），并批准："No issues found. LGTM! ✅"。基于维护者签字批准。✅ — Qwen Code · qwen3.8-max-preview Reviewed at |
|
⏸️ Deferring to @wenshao — re-ran the full triage on the current head ( I'm still deferring rather than auto-approving because this is a fork PR adding ~953 production lines of core Goal infrastructure that defines public API surface before runtime adoption — our core-module policy caps that at a human sign-off. The calls left are architectural and yours to make: is the contract shape stable enough for the follow-up PRs to build on, and is the "Goal v3" title vs 中文说明⏸️ 转交 @wenshao —— 已在当前 head（ 我仍然转交而非自动批准，因为这是一个 fork PR，新增约 953 行核心 Goal 基础设施，并在 runtime 接入前就定义了公共 API 面——核心模块政策将其限制为需人工签字。剩下的是架构判断，由你决定：契约形状是否足够稳定以支撑后续 PR，以及 "Goal v3" 标题与 — Qwen Code · qwen3.8-max-preview |
Code Review —
|
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: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
— qwen3.7-max via Qwen Code /review
| export function goalRequiresExactPermit(snapshot: GoalSnapshotV2): boolean { | ||
| return ( | ||
| snapshot.goal !== null && | ||
| (snapshot.goal.status === 'active' || snapshot.activity === 'running') | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] goalRequiresExactPermit has no test coverage for any of its branches — Failure scenario: a future change inverts or loosens the optimistic-concurrency gate condition (e.g., returning true for paused goals, or false when goal.status === 'active' but activity === 'idle') → no test catches either blocking legitimate model sends or allowing sends without proper concurrency enforcement.
Add tests exercising: (1) goal === null → false, (2) active goal with idle activity → true, (3) paused goal with idle activity → false, (4) any goal with activity === 'running' → true.
— qwen3.7-max via Qwen Code /review
| if (current.status !== 'active' && current.status !== 'paused') { | ||
| throw new GoalInvalidTransitionError( | ||
| 'Only an active or paused Goal can finish a turn', | ||
| snapshotOf(current), | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] reduceGoalTurnFinished guard rejecting blocked/usage_limited/complete goals is untested — Failure scenario: the guard is accidentally widened (e.g., changing to current.status !== 'complete') → turn counts and activeTimeMs silently accumulate on terminal goals, corrupting accounting.
Add a test calling reduceGoalTurnFinished on a blocked or complete goal, asserting it throws GoalInvalidTransitionError.
— qwen3.7-max via Qwen Code /review
| ])('rejects a blocked audit with %s', (_label, blockedAudit) => { | ||
| expect( | ||
| parseGoalStateRecordPayloadV2({ | ||
| v: 2, | ||
| cause: 'turn_finished', | ||
| snapshot: snapshot(goalRecord()), | ||
| blockedAudit, | ||
| }), | ||
| ).toBeUndefined(); |
There was a problem hiding this comment.
[Suggestion] Six rejection tests for invalid blockedAudit but no positive test for a valid blockedAudit round-trip — Failure scenario: if isBlockedAudit's acceptance path breaks (e.g., count <= 3 changed to count < 1), every valid blockedAudit payload becomes silently unparseable; the six rejection tests still pass, masking the regression.
Add a test parsing a payload with a valid blockedAudit (e.g., { fingerprint: 'fp', count: 2, turnIds: ['t1', 't2'] }) and asserting the result includes the deep-cloned audit object.
— qwen3.7-max via Qwen Code /review
| if (current.status === 'active') { | ||
| throw new GoalInvalidTransitionError( | ||
| 'An active Goal cannot be resumed', | ||
| snapshotOf(current), | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] resume on an already-active goal is a distinct error branch with no test — Failure scenario: if the current.status === 'active' guard is removed or inverted, resume on an active goal falls through to transitionGoal, producing a silent no-op transition that bumps updatedAt and recomputes activeTimeMs.
Add a test calling reduceGoalControl with action: 'resume' on an active goal, asserting it throws GoalInvalidTransitionError.
— qwen3.7-max via Qwen Code /review
| parseGoalSnapshotV2, | ||
| parseGoalStateRecordPayloadV2, | ||
| } from './goal-reducer.js'; | ||
| export { projectGoalStateToLegacy } from './goal-legacy-projection.js'; |
There was a problem hiding this comment.
[Suggestion] The goalWire barrel re-exports projectGoalStateToLegacy but not the types describing its return value — Failure scenario: a downstream package importing from @qwen-code/core/goalWire cannot name LegacyGoalProjection or its constituent types, forcing consumers to use ReturnType<typeof projectGoalStateToLegacy> or reach past the curated API. The internal barrel goals/index.ts exports all five types, confirming this omission is a gap.
| export { projectGoalStateToLegacy } from './goal-legacy-projection.js'; | |
| export { projectGoalStateToLegacy } from './goal-legacy-projection.js'; | |
| export type { | |
| LegacyActiveGoal, | |
| LegacyGoalProjection, | |
| LegacyGoalStatus, | |
| LegacyGoalStatusKind, | |
| LegacyGoalTerminal, | |
| } from './goal-legacy-projection.js'; |
— qwen3.7-max via Qwen Code /review
| readonly lastReason?: string; | ||
| } | ||
|
|
||
| export type ActiveGoal = LegacyActiveGoal; |
There was a problem hiding this comment.
[Suggestion] ActiveGoal type alias collides with activeGoalStore.ts's ActiveGoal interface — different fields (optional vs required tokensAtStart/hookId) — Failure scenario: IDE auto-import or a future export * barrel change resolves the wrong type, producing structurally incompatible usage. The alias is dead code (not re-exported from either barrel, zero consumers).
| export type ActiveGoal = LegacyActiveGoal; |
Remove this line. Consumers should use LegacyActiveGoal directly.
— qwen3.7-max via Qwen Code /review
| goal: { | ||
| goalId: input.goalId, | ||
| revision: 1, | ||
| objective: input.objective.trim(), |
There was a problem hiding this comment.
[Suggestion] createMigratedGoalState trims the objective but does not reject empty results — Failure scenario: a future direct caller passes ' ' → produces a payload with objective: '' → on recovery, parseGoalRecord rejects the empty objective → recoverGoalFromRecords returns unsupported, silently losing the migrated goal. The known caller (recoverLegacyGoal) guards upstream, but the public API doesn't.
| objective: input.objective.trim(), | |
| const trimmed = input.objective.trim(); | |
| if (!trimmed) { | |
| throw new Error('Migrated Goal objective must not be empty'); | |
| } |
Then use trimmed in the returned payload's objective field.
— qwen3.7-max via Qwen Code /review
|
已修复。 验证： |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
| case 'turn_finished': | ||
| case 'verifier_accept': | ||
| case 'verifier_reject': | ||
| return payload.snapshot.goal?.status === 'complete' | ||
| ? 'achieved' | ||
| : payload.snapshot.goal?.status === 'blocked' || | ||
| payload.snapshot.goal?.status === 'usage_limited' | ||
| ? 'aborted' | ||
| : 'checking'; |
There was a problem hiding this comment.
[Suggestion] The turn_finished / verifier_accept / verifier_reject case group's inner ternary returns 'achieved' for complete, 'aborted' for blocked/usage_limited, and 'checking' otherwise — but only the 'checking' branch is exercised by tests (via turn_finished with 'active' and 'paused' goals). — Failure scenario: if the ternary logic were inverted or a status comparison used the wrong string literal, no test would catch it. A regression in verifier result projection ships undetected.
// Add parameterized tests mirroring the existing turn_finished patterns:
// payload('turn_finished', 'complete') → goalStatus.kind === 'achieved'
// payload('turn_finished', 'blocked') → goalStatus.kind === 'aborted'
// payload('verifier_accept', 'complete') → goalStatus.kind === 'achieved'
// payload('verifier_reject', 'active') → goalStatus.kind === 'checking'— qwen3.7-max via Qwen Code /review
| if (current.status === 'complete') { | ||
| throw new GoalInvalidTransitionError( | ||
| 'A completed Goal cannot be resumed', | ||
| snapshotOf(current), | ||
| ); | ||
| } | ||
| if (current.status === 'active') { | ||
| throw new GoalInvalidTransitionError( | ||
| 'An active Goal cannot be resumed', | ||
| snapshotOf(current), | ||
| ); | ||
| } | ||
| return transitionGoal(current, transition.now, { status: 'active' }); |
There was a problem hiding this comment.
[Suggestion] The resume action is handled by implicit fall-through — after create, clear, replace, edit, and pause all early-return, whatever remains is treated as resume. There is no explicit request.action === 'resume' guard and no assertNever exhaustiveness check. The same PR uses assertNever in legacyStatusKind (goal-legacy-projection.ts:133) for an analogous discriminated union. — Failure scenario: if a new GoalControlRequest action variant is added (e.g., 'archive'), it silently falls through to the resume logic, transitioning any non-complete, non-active goal to 'active' without a compile-time error.
| if (current.status === 'complete') { | |
| throw new GoalInvalidTransitionError( | |
| 'A completed Goal cannot be resumed', | |
| snapshotOf(current), | |
| ); | |
| } | |
| if (current.status === 'active') { | |
| throw new GoalInvalidTransitionError( | |
| 'An active Goal cannot be resumed', | |
| snapshotOf(current), | |
| ); | |
| } | |
| return transitionGoal(current, transition.now, { status: 'active' }); | |
| if (current.status === 'complete') { | |
| throw new GoalInvalidTransitionError( | |
| 'A completed Goal cannot be resumed', | |
| snapshotOf(current), | |
| ); | |
| } | |
| if (current.status === 'active') { | |
| throw new GoalInvalidTransitionError( | |
| 'An active Goal cannot be resumed', | |
| snapshotOf(current), | |
| ); | |
| } | |
| if (request.action !== 'resume') { | |
| assertNever(request); | |
| } | |
| return transitionGoal(current, transition.now, { status: 'active' }); |
— qwen3.7-max via Qwen Code /review
| if (current.status !== 'active' && current.status !== 'paused') { | ||
| throw new GoalInvalidTransitionError( |
There was a problem hiding this comment.
[Suggestion] reduceGoalTurnFinished explicitly rejects blocked, usage_limited, and complete goals (throws GoalInvalidTransitionError), but no test asserts this rejection. Only positive paths (active, paused) are tested. — Failure scenario: a future refactor loosens the guard (e.g., also accepts complete). The function silently increments turnCount on a terminal goal, corrupting its accounting.
// Add:
it.each(['blocked', 'usage_limited', 'complete'] as const)(
'rejects turn_finished on a %s goal',
(status) => {
expect(() => reduceGoalTurnFinished(goalRecord({ status }), { now: 200 }))
.toThrow(GoalInvalidTransitionError);
},
);— qwen3.7-max via Qwen Code /review
| return payload | ||
| ? { kind: 'v2', payload } | ||
| : { | ||
| kind: 'unsupported', | ||
| reason: `Goal lifecycle record ${record.uuid} is malformed or uses an unsupported version`, | ||
| }; |
There was a problem hiding this comment.
[Suggestion] recoverGoalFromRecords returns unsupported immediately when the newest goal_state record is malformed or on a non-system record, without continuing to check older records that may be perfectly valid. The PR description states this is by design ("Recovery deliberately stops on an invalid newest record"). However, the existing test only uses a single record, so the multi-record shadowing behavior is indistinguishable from an accidental bug. — Failure scenario: a future change accidentally changes the early return to continue and valid older records are suddenly revived — but no test documents the current contract, so the regression goes undetected.
// Add a multi-record test that documents the deliberate stop-on-invalid contract:
// Input: [older-valid-system-goal_state, newer-user-goal_state]
// Assert: result.kind === 'unsupported'— qwen3.7-max via Qwen Code /review
| parseGoalSnapshotV2, | ||
| parseGoalStateRecordPayloadV2, | ||
| } from './goal-reducer.js'; | ||
| export { projectGoalStateToLegacy } from './goal-legacy-projection.js'; |
There was a problem hiding this comment.
[Suggestion] projectGoalStateToLegacy is exported via the public ./goalWire package surface, but its return type (LegacyGoalProjection) and all constituent types (LegacyGoalStatus, LegacyGoalStatusKind, LegacyActiveGoal, LegacyGoalTerminal) are not. The sibling barrel index.ts exports both the function and all five legacy types, confirming they are part of the module's public surface. — Failure scenario: a consumer importing from @anthropic-ai/core/goalWire can call the function and get type inference on the result, but cannot write let proj: LegacyGoalProjection or declare an interface field without reaching into deep internal paths.
| export { projectGoalStateToLegacy } from './goal-legacy-projection.js'; | |
| export { projectGoalStateToLegacy } from './goal-legacy-projection.js'; | |
| export type { | |
| LegacyActiveGoal, | |
| LegacyGoalProjection, | |
| LegacyGoalStatus, | |
| LegacyGoalStatusKind, | |
| LegacyGoalTerminal, | |
| } from './goal-legacy-projection.js'; |
— qwen3.7-max via Qwen Code /review
| case 'verifier_accept': | ||
| case 'verifier_reject': |
There was a problem hiding this comment.
[Suggestion] verifier_accept and verifier_reject are handled here but never tested — Failure scenario: a future change differentiates verifier_reject (e.g., returning 'aborted' directly) → the legacy projection for verifier outcomes silently changes without any test failing.
Add verifier_accept and verifier_reject to the it.each parameterization in goal-legacy-projection.test.ts alongside turn_finished, or add dedicated test cases confirming they project identically for the same snapshot statuses.
— qwen3.7-max via Qwen Code /review
| const LEGACY_ACTIVE_KINDS = new Set(['set', 'checking']); | ||
| const LEGACY_STOPPED_KINDS = new Set([ | ||
| 'achieved', | ||
| 'cleared', | ||
| 'failed', | ||
| 'aborted', | ||
| ]); |
There was a problem hiding this comment.
[Critical] 'paused' is a valid LegacyGoalStatusKind produced by projectGoalStateToLegacy but is absent from both LEGACY_ACTIVE_KINDS and LEGACY_STOPPED_KINDS — Failure scenario: a paused v2 Goal's legacy projection has kind: 'paused'; if recovery falls back to legacy records, recoverLegacyGoal classifies it as unsupported → the user's paused Goal is reported as unrecoverable.
| const LEGACY_ACTIVE_KINDS = new Set(['set', 'checking']); | |
| const LEGACY_STOPPED_KINDS = new Set([ | |
| 'achieved', | |
| 'cleared', | |
| 'failed', | |
| 'aborted', | |
| ]); | |
| const LEGACY_ACTIVE_KINDS = new Set(['set', 'checking']); | |
| const LEGACY_STOPPED_KINDS = new Set([ | |
| 'achieved', | |
| 'cleared', | |
| 'failed', | |
| 'aborted', | |
| 'paused', | |
| ]); |
— qwen3.7-max via Qwen Code /review
| if (current.status === 'active') { | ||
| throw new GoalInvalidTransitionError( | ||
| 'An active Goal cannot be resumed', | ||
| snapshotOf(current), | ||
| ); | ||
| } | ||
| return transitionGoal(current, transition.now, { status: 'active' }); |
There was a problem hiding this comment.
[Suggestion] The resume action is handled by implicit fallthrough — after create, clear, replace, edit, and pause all early-return, whatever remains is treated as resume. There is no explicit request.action === 'resume' guard and no exhaustiveness check — Failure scenario: a developer adds a new action to GoalControlRequest (e.g. 'archive') and forgets to handle it here → for paused/blocked/usage_limited goals, the reducer silently sets status: 'active'.
Wrap the final block in an explicit if (request.action === 'resume') and add const _exhaustive: never = request; after it, matching the assertNever pattern used in goal-legacy-projection.ts.
— qwen3.7-max via Qwen Code /review
| it.each(['paused', 'blocked', 'usage_limited'] as const)( | ||
| 'edits a %s goal without changing its status', | ||
| (status) => { |
There was a problem hiding this comment.
[Suggestion] No test covers resume from blocked or usage_limited status — the reducer's fallthrough silently transitions both to active — Failure scenario: in the legacy projection, blocked/usage_limited map to 'aborted', suggesting terminal-ish semantics, yet resume silently re-activates them with no test asserting this is intended. If these states should reject resume, the defect ships uncaught.
Add a parameterized test: it.each(['blocked', 'usage_limited'] as const)('resumes a %s goal to active', ...) — or, if resume from these states should be rejected, add the guard and rejection test.
— qwen3.7-max via Qwen Code /review
|
已修复：暂停状态的旧版 Goal 记录现在按已停止状态恢复，不再判为 unsupported。 验证证据： |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
| if (current.status === 'complete') { | ||
| throw new GoalInvalidTransitionError( | ||
| 'A completed Goal cannot be resumed', |
There was a problem hiding this comment.
[Suggestion] The resume handler is an implicit fall-through — no request.action === 'resume' guard, no exhaustive never check. A future action added to GoalControlRequest and handled in parseGoalControlRequest (which has default: return undefined) but forgotten here would silently execute resume logic — Concrete cost: a paused goal set to active instead of being cancelled, or a confusing "An active Goal cannot be resumed" error for an unrelated action.
| if (current.status === 'complete') { | |
| throw new GoalInvalidTransitionError( | |
| 'A completed Goal cannot be resumed', | |
| // request.action === 'resume' is the only remaining union member here | |
| if (current.status === 'complete') { | |
| throw new GoalInvalidTransitionError( | |
| 'A completed Goal cannot be resumed', |
Or restructure as switch (request.action) with default: { const _exhaustive: never = request; throw … }.
— qwen3.8-max-preview via Qwen Code /review
| export function goalRequiresExactPermit(snapshot: GoalSnapshotV2): boolean { | ||
| return ( | ||
| snapshot.goal !== null && |
There was a problem hiding this comment.
[Suggestion] goalRequiresExactPermit is exported as a public safety gate for turn permits but has zero test coverage — Concrete cost: if a future refactor inverts || to && or drops the activity check, paused-goal-with-running-activity sends would bypass the permit gate undetected.
Add a small describe('goalRequiresExactPermit') block: active goal → true, null goal → false, paused + idle → false, paused + running → true.
— qwen3.8-max-preview via Qwen Code /review
| ])('rejects a blocked audit with %s', (_label, blockedAudit) => { | ||
| expect( | ||
| parseGoalStateRecordPayloadV2({ |
There was a problem hiding this comment.
[Suggestion] All six blockedAudit cases are rejection tests; no positive test that a valid blockedAudit is accepted and preserved via structuredClone — Concrete cost: if the acceptance path regresses (e.g. truthiness guard inverted), every valid blocked audit is silently dropped from persisted records with no test failure.
Add one positive case: blockedAudit: { fingerprint: 'fp', count: 2, turnIds: ['t1', 't2'] } and assert the result deep-equals the input.
— qwen3.8-max-preview via Qwen Code /review
| if (current.status !== 'active' && current.status !== 'paused') { | ||
| throw new GoalInvalidTransitionError( | ||
| 'Only an active or paused Goal can finish a turn', |
There was a problem hiding this comment.
[Suggestion] This guard rejecting complete/blocked/usage_limited is not exercised by any test — Concrete cost: if removed during a refactor, reduceGoalTurnFinished silently increments turnCount and activeTimeMs on a terminal goal; existing tests still pass, and the corruption surfaces later as a nonsensical legacy projection.
Add it.each(['complete', 'blocked', 'usage_limited'])('rejects turn finish for %s goal', …) asserting GoalInvalidTransitionError.
— qwen3.8-max-preview via Qwen Code /review
| const value = { ...snapshot(goalRecord()), activity }; | ||
|
|
||
| expect(parseGoalSnapshotV2(value)).toEqual(value); |
There was a problem hiding this comment.
[Suggestion] parseGoalSnapshotV2 tests only exercise non-null goals; the goal: null early-return branch has zero coverage — Concrete cost: if the value['goal'] === null guard breaks, every cleared-goal snapshot is silently rejected as undefined with no test failure.
Add: expect(parseGoalSnapshotV2(snapshot(null))).toEqual(snapshot(null)).
— qwen3.8-max-preview via Qwen Code /review
| expect( | ||
| parseGoalControlRequest({ action: 'create', objective: 'ship' }), | ||
| ).toEqual({ | ||
| action: 'create', | ||
| objective: 'ship', | ||
| }); |
There was a problem hiding this comment.
[Suggestion] parseGoalControlRequest has no positive test for any versioned action (replace, edit, pause, resume, clear) — only create is acceptance-tested; every other assertion is a rejection — Concrete cost: if isExpectedVersion is tightened or the return object's field names swap, the test suite still passes.
Add at least one positive assertion per versioned branch, e.g. edit with objective + version and pause with version only.
— qwen3.8-max-preview via Qwen Code /review
| updatedAt: 160, | ||
| }); | ||
|
|
||
| const finished = reduceGoalTurnFinished(paused, { now: 225 }); |
There was a problem hiding this comment.
[Suggestion] reduceGoalTurnFinished's lastReason conditional-spread branch is never exercised — both test calls omit lastReason — Concrete cost: if the spread is removed or inverted, lastReason silently stops being set on turn finish; downstream consumers (goalHook.ts, goalCommand.ts, goal-legacy-projection.ts) see stale values with no test failure.
Add a test passing lastReason: 'evidence found' and asserting it lands on the record.
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. 3 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. 3 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
Local verification —
|
| Check | Command | Result |
|---|---|---|
| Full goals suite | vitest run src/goals/ |
✅ 7 files, 149 tests, 0 failures |
| Type check | tsc --noEmit |
✅ clean |
| Build | build_package.js |
✅ emits dist/src/goals/goal-wire.{js,d.ts} |
Public ./goalWire subpath |
downstream consumer via export map | ✅ 15/15 assertions |
| Review-fix commits load-bearing | A/B revert | ✅ both proven |
On the test count (138 → 149)
The body says "7 test files, 138 tests". The current head runs 149. This fully reconciles and is not a defect:
- Patch 1 snapshot: 46 new + 92 existing = 138 (the "46 new tests" the triage bot cited).
- The two review-fix commits added 11 tests → 57 new + 92 existing = 149.
The body's number is just pre-review-fix. Worth bumping "138" to "149" so the PR text matches head.
Public subpath — verified as a real downstream consumer
A throwaway package imports @qwen-code/qwen-code-core/goalWire by name; Node's export map resolves it to the built dist/src/goals/goal-wire.js. Beyond the author's "parsed a running snapshot" claim, I confirmed the full contract:
- All three runtime activities (
idle/running/verifying) round-trip throughparseGoalSnapshotV2. - Persistence gate is idle-only:
parseGoalStateRecordPayloadV2rejects arunningsnapshot, accepts anidleone. - Strictness holds through the public surface: unknown top-level key and wrong version (
v:3) both rejected. goalRequiresExactPermitandprojectGoalStateToLegacybehave as specified.
Load-bearing check (A/B on the two review-fix commits)
Reverting both fixes fails exactly their two guarding tests, nothing else:
- Paused legacy recovery (patch 3): without
'paused'inLEGACY_STOPPED_KINDS, a paused legacy Goal degrades to{ kind: 'unsupported' }instead of{ kind: 'none' }— i.e. it would surface a recovery error rather than cleanly declining to revive. - Empty migrated objective (patch 2): without the guard,
createMigratedGoalState(' ')silently builds an invalid record instead of throwing.
Review findings — status re-checked against the checked-out branch
- pre-release: fix ci #1 (
ActiveGoalalias footgun): already addressed.export type ActiveGoal = LegacyActiveGoalwas removed in the review-fix commit — confirmed absent at head. - Where is the config saved? #2 (projection
activeGoalnot assignable to canonicalActiveGoal): confirmed still open.activeGoalStore.ActiveGoalrequirestokensAtStart: numberandhookId: string(both non-optional); the projection never populates them. Harmless for a display-only projection, but it will not type-check if fed intosetActiveGoal()— worth pinning down the intended consumer on the adoption PR. - 如何自定义密钥文件 .env可能与其他文件冲突 #3 (
goal_statesubtype has no writer): confirmed.'goal_state'is not a member of theChatRecord.subtypeunion and no code emits it; it only appears in the new reader + its tests. The v2 recovery path is therefore exercised against synthetic records only until a follow-up extendsChatRecordand adds a writer. Compiles today solely becauseGoalRecoveryRecordwidenssubtypetostring.
Verdict
Merge-ready as a contract-first slice. Under real execution the change is green (typecheck + build + 149 tests), the public ./goalWire surface resolves and enforces its documented semantics, and both review-fix commits are proven load-bearing. The open items are forward-integration follow-ups, not blockers:
- Bump the "138" in the body to 149.
- Track Where is the config saved? #2 (projection ↔ canonical
ActiveGoalshape) and 如何自定义密钥文件 .env可能与其他文件冲突 #3 (goal_statewriter +ChatRecord.subtypemember) explicitly on the adoption PR — that's where the v2 recovery path first becomes exercisable end-to-end.
中文版本
本地验证 — feat(core): add Goal v3 state protocol（#7517）
承接我之前的评审：当时我手工推演了逻辑，但没有执行测试套件（分支未在本地 checkout）。现在我已在隔离 worktree 中 checkout PR 头并实际跑通全部流程。所有声明均可复现，且两个评审修复提交都是“承重”的（缺一即挂）。
环境： 隔离 worktree，头 a171c4903c · Node v22 · macOS · node_modules 从已装好的主仓软链（无需重装）。
结果
| 检查项 | 命令 | 结果 |
|---|---|---|
| 完整 goals 套件 | vitest run src/goals/ |
✅ 7 文件，149 测试，0 失败 |
| 类型检查 | tsc --noEmit |
✅ 通过 |
| 构建 | build_package.js |
✅ 产出 dist/src/goals/goal-wire.{js,d.ts} |
公共 ./goalWire 子路径 |
下游消费方经 export map 导入 | ✅ 15/15 断言 |
| 评审修复提交是否承重 | A/B 回退 | ✅ 均已证明 |
（证据截图见上方英文版。）
关于测试数量（138 → 149）
PR 正文写“7 文件、138 测试”，当前头跑出的是 149。这完全可对上，不是缺陷：
- Patch 1 快照：46 新 + 92 既有 = 138（即 triage 机器人所说的“46 个新测试”）。
- 两个评审修复提交新增 11 个测试 → 57 新 + 92 既有 = 149。
正文数字只是评审修复前的旧值，建议把“138”更新为“149”，让正文与头一致。
公共子路径 —— 以真实下游消费方验证
一个临时包按名导入 @qwen-code/qwen-code-core/goalWire；Node 的 export map 将其解析到构建产物 dist/src/goals/goal-wire.js。在作者“解析了一个 running 快照”之外，我确认了完整契约：
- 三种运行活动（
idle/running/verifying）都能经parseGoalSnapshotV2往返。 - 持久化闸门为 仅 idle：
parseGoalStateRecordPayloadV2拒绝running快照、接受idle快照。 - 严格性经公共面依然成立：多余顶层键与错误版本（
v:3）均被拒绝。 goalRequiresExactPermit与projectGoalStateToLegacy行为符合规范。
承重性检查（对两个评审修复提交做 A/B）
同时回退两个修复后，恰好只有其两个守护测试失败，别无其它：
- 暂停态旧版恢复（patch 3）：若
LEGACY_STOPPED_KINDS缺'paused'，暂停态旧 Goal 退化为{ kind: 'unsupported' }（应为{ kind: 'none' }）——即会抛出恢复错误，而非干净地拒绝复活。 - 空的迁移目标（patch 2）：若无守卫，
createMigratedGoalState(' ')会静默构造出非法记录，而不是抛错。
评审意见 —— 对照已 checkout 分支复核状态
- pre-release: fix ci #1（
ActiveGoal别名陷阱）：已修复。export type ActiveGoal = LegacyActiveGoal已在评审修复提交中删除，头上确认不存在。 - Where is the config saved? #2（投影
activeGoal无法赋给规范ActiveGoal）：确认仍开放。activeGoalStore.ActiveGoal要求tokensAtStart: number与hookId: string（均非可选），而投影从不填充它们。作为纯展示投影无害，但若喂给setActiveGoal()会类型检查失败——建议在接入 PR 上敲定消费方。 - 如何自定义密钥文件 .env可能与其他文件冲突 #3（
goal_statesubtype 无写入方）：确认。'goal_state'不在ChatRecord.subtype联合类型里、也无任何代码写入；只出现在新读取逻辑及其测试中。因此 v2 恢复路径目前仅对合成记录生效，需等后续 PR 扩展ChatRecord并加入写入方。当前能编译，仅因GoalRecoveryRecord把subtype放宽成了string。
结论
作为“契约先行”切片，可以合并。 真实执行下改动全绿（typecheck + build + 149 测试），公共 ./goalWire 面可解析并强制其文档化语义，两个评审修复提交均被证明承重。开放项属于后续接入的跟进事项，而非合并阻塞：
- 把正文的“138”更新为 149。
- 在接入 PR 上显式跟踪 Where is the config saved? #2（投影 ↔ 规范
ActiveGoal形状）与 如何自定义密钥文件 .env可能与其他文件冲突 #3（goal_state写入方 +ChatRecord.subtype成员）——那也是 v2 恢复路径首次可做端到端验证之处。
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. 4 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
— qwen3.7-max via Qwen Code /review
| ...(displayGoal?.lastReason === undefined | ||
| ? {} | ||
| : { lastReason: displayGoal.lastReason }), |
There was a problem hiding this comment.
[Suggestion] The lastReason conditional spread appears in three sites (goalStatus line 66, activeGoal line 82, goalTerminal line 97) but no test constructs a goal with lastReason: undefined to verify the field is omitted — Failure scenario: if a conditional spread is accidentally inverted or removed (e.g., { lastReason: displayGoal.lastReason } unconditionally), every test still passes because the test fixture GOAL always sets lastReason: 'continuing'. Downstream consumers would receive lastReason: undefined on goals that never had one, or silently lose it on goals that did.
Suggested test addition:
it('omits lastReason from projection when absent', () => {
const goalWithoutReason = { ...GOAL, lastReason: undefined };
const projected = projectGoalStateToLegacy(
{ ...SNAPSHOT, goal: goalWithoutReason },
undefined,
{ cause: 'edit', now: 200 },
);
expect(projected.goalStatus).not.toHaveProperty('lastReason');
expect(projected.activeGoal).not.toHaveProperty('lastReason');
});— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
| @@ -0,0 +1,483 @@ | |||
| /** | |||
There was a problem hiding this comment.
[Critical] The resume transition in reduceGoalControl only excludes complete and active, meaning blocked/usage_limited states can be resumed. Tests only cover resuming from paused. If this is intentional (e.g. unblocking), it needs test coverage and a comment explaining the intent. If not, it's a state machine gap.
— qwen3.8-max-preview via Qwen Code /review
| @@ -0,0 +1,145 @@ | |||
| /** | |||
There was a problem hiding this comment.
[Critical] recoverGoalFromRecords fails fast on the latest corrupted goal_state record and returns unsupported without scanning older valid records. A single corrupt write breaks the entire recovery chain. Consider logging and continuing the backward scan, or including the last valid state in the unsupported result.
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
已修复。验证证据：goal-persistence.test.ts 13/13 tests passed；npm run build 与 npm run typecheck 均通过。
| @@ -0,0 +1,126 @@ | |||
| /** | |||
There was a problem hiding this comment.
[Suggestion] blockedAudit.count <= 3 threshold is hardcoded as a magic number in the wire parser. Extract to a named constant (e.g. GOAL_BLOCKER_MAX_COUNT) to avoid silent drift if the business rule changes upstream.
— qwen3.8-max-preview via Qwen Code /review
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Review Summary
Solid versioned state protocol implementation. State machine transitions are deterministic and complete, wire parsing uses whitelist + per-field validation, optimistic concurrency (goalId + revision) semantics are clear. No any type leaks. Test coverage is good for the main paths.
Two concerns need author confirmation before merge:
(1) Resume from blocked/usage_limited (goal-reducer.ts): The resume path only excludes complete and active, so blocked/usage_limited can be resumed. Tests only cover resume from paused. If intentional (unblocking), needs test + comment; if not, it's a state machine gap.
(2) Recovery fail-fast on corrupt record (goal-persistence.ts): recoverGoalFromRecords returns unsupported on the latest corrupted record without scanning older valid ones. A single corrupt write breaks the entire recovery chain. Consider continuing the backward scan or including the last valid state.
(3) Magic number blockedAudit.count <= 3 in wire parser — extract to named constant.
(4) parseGoalStateRecordPayloadV2 only accepts activity === 'idle' — if a crash writes a mid-turn record with running/verifying, it becomes unrecoverable. Add a defensive comment documenting the turn-boundary write assumption.
(5) Legacy recovery uses as SlashCommandRecordPayload type assertion — prefer unknown + type guard to match the strict wire parser style.
Minor: goal-wire.ts barrel overlaps with index.ts exports; PAUSED_GOAL_SYSTEM_REMINDER prompt text in a types/constants file feels misplaced; hasOnlyKeys naming suggests bidirectional check.
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] goal-persistence.ts:39-57 — recoverGoalFromRecords returns unsupported on the first corrupted goal_state record without scanning older valid records. A single corrupt write breaks the entire recovery chain. This is an existing Critical from @yiliang114 (comment 3636155070) that still stands — the PR description states this is intentional, but the blocker was not resolved.
— qwen3.7-max via Qwen Code /review
| }), | ||
| ).toBeUndefined(); | ||
| expect( | ||
| parseGoalControlRequest({ action: 'create', objective: 'ship' }), |
There was a problem hiding this comment.
[Suggestion] parseGoalControlRequest has a positive round-trip test only for the create action. The replace, edit, pause, resume, and clear actions have only negative tests (empty objective, missing revision, zero revision), so a valid wire input for these actions is never confirmed to parse successfully.
Failure scenario: a developer adds a new optional field to the clear action and updates the type and reducer, but forgets to add the field to the hasOnlyKeys allow-list in parseGoalControlRequest. Every valid clear request carrying the new field is silently rejected (undefined). Because no positive test exists for clear parsing, the regression is invisible at the unit level.
| parseGoalControlRequest({ action: 'create', objective: 'ship' }), | |
| parseGoalControlRequest({ action: 'create', objective: 'ship' }), | |
| ).toEqual({ | |
| action: 'create', | |
| objective: 'ship', | |
| }); | |
| expect( | |
| parseGoalControlRequest({ | |
| action: 'pause', | |
| expectedGoalId: 'g-1', | |
| expectedRevision: 1, | |
| }), | |
| ).toEqual({ | |
| action: 'pause', | |
| expectedGoalId: 'g-1', | |
| expectedRevision: 1, | |
| }); | |
| expect( | |
| parseGoalControlRequest({ | |
| action: 'clear', | |
| expectedGoalId: 'g-1', | |
| expectedRevision: 1, | |
| }), | |
| ).toEqual({ | |
| action: 'clear', | |
| expectedGoalId: 'g-1', | |
| expectedRevision: 1, | |
| }); |
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] goal-persistence.ts:39-55 — recoverGoalFromRecords returns unsupported on the first corrupted goal_state record without scanning older valid records. A single corrupt write breaks the entire recovery chain. This is an existing Critical from @yiliang114 (comment 3636155070) that still stands at the reviewed commit — the PR description states this is intentional (fail-closed), but the blocker was not resolved or dismissed by the author.
— qwen3.8-max-preview via Qwen Code /review
| assertExpectedVersion( | ||
| current, | ||
| request.expectedGoalId, | ||
| request.expectedRevision, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] reduceGoalControl(null, …) with a non-create action (e.g. pause, edit, clear) is untested — the assertExpectedVersion(null, …) path that throws GoalConflictError with a { v: 2, goal: null, activity: 'idle' } snapshot has no exercising test — Failure scenario: if assertExpectedVersion is refactored to assume current is non-null (the create branch already handles null separately, making the !current guard look redundant), a control request arriving when no goal exists throws a TypeError on current.goalId instead of the clean GoalConflictError callers need for graceful recovery
Consider adding:
it('throws GoalConflictError with a null-goal snapshot for non-create actions when no goal exists', () => {
try {
reduceGoalControl(null, {
request: { action: 'pause', expectedGoalId: 'g-1', expectedRevision: 1 },
now: 200,
nextGoalId: 'unused',
cursor: { recordId: 'r-200' },
});
throw new Error('expected conflict');
} catch (error) {
expect(error).toBeInstanceOf(GoalConflictError);
expect((error as GoalConflictError).current).toEqual({ v: 2, goal: null, activity: 'idle' });
}
});— qwen3.8-max-preview via Qwen Code /review
Continue past malformed lifecycle records so a newer corrupt write cannot hide the latest valid Goal snapshot.
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: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
— qwen3.7-max via Qwen Code /review
| it.each(['create', 'replace', 'edit', 'resume', 'migrated'] as const)( | ||
| 'projects %s as legacy set with an active projection', | ||
| (cause) => { |
There was a problem hiding this comment.
[Suggestion] verifier_accept and verifier_reject are distinct GoalStateCause values handled by legacyStatusKind but are never exercised by any test — Failure scenario: a future refactor splits the turn_finished/verifier_accept/verifier_reject fall-through to give either verifier cause its own branch, and accidentally maps one to the wrong LegacyGoalStatusKind (e.g., 'set' instead of 'checking'). The existing parameterized tests only cover turn_finished and the five "set" causes, so the regression ships undetected.
| it.each(['create', 'replace', 'edit', 'resume', 'migrated'] as const)( | |
| 'projects %s as legacy set with an active projection', | |
| (cause) => { | |
| it.each(['create', 'replace', 'edit', 'resume', 'migrated'] as const)( | |
| 'projects %s as legacy set with an active projection', | |
| (cause) => { |
Add verifier_accept and verifier_reject to the parameterized test, e.g. as separate cases that verify legacyStatusKind mapping produces the correct LegacyGoalStatusKind for each verifier cause × goal-status combination.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.8-max-preview via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
5 participants
What this PR does
Introduces the versioned Goal v3 state contract as the first independently reviewable slice of #7494. It defines lifecycle state, optimistic-concurrency controls, deterministic transitions, strict wire parsing, turn-boundary persistence and recovery, legacy migration, and compatibility projections for existing consumers.
The public snapshot parser accepts all valid runtime activities (
idle,running, andverifying), while persisted lifecycle records remain restricted to stableidleboundaries. Existing Goal behavior remains available during the staged migration.Why it's needed
The original Goal redesign spans core runtime, transcript replay, CLI, ACP, SDK, WebShell, and Desktop. Landing the authoritative state contract first gives later PRs a small, stable dependency and avoids reviewing the complete cross-surface implementation as one large change.
Strict parsing and version checks prevent malformed or stale lifecycle state from being silently accepted. Recovery deliberately stops on an invalid newest record instead of reviving older state, and legacy sessions migrate only the objective into a fresh identity and evidence boundary.
Reviewer Test Plan
How to verify
Confirm that create, replace, edit, pause, resume, clear, and turn completion preserve the documented identity, revision, cursor, elapsed-time, and status invariants. Confirm that stale revisions and invalid transitions return explicit errors, no turn or elapsed-time threshold automatically terminates a Goal, all three runtime activities round-trip through the public parser, and only idle snapshots are accepted for persistence.
Confirm that recovery selects the newest valid lifecycle state, refuses a malformed newest record, does not revive stopped legacy Goals, and migrates an active legacy Goal into a fresh v2 identity. Confirm that the existing Goal store, hook, judge, and loop tests remain green.
Local result: 7 test files passed, 138 tests passed. Core type checking and build passed. The built public Goal wire subpath imported successfully and parsed a running snapshot.
Evidence (Before & After)
N/A — this PR defines the non-UI state and persistence contract. User-facing TUI and WebShell evidence will be attached to their dedicated follow-up PRs.
Tested on
Environment (optional)
Node.js 24, no sandbox.
Risk & Scope
Linked Issues
Split from #7494.
中文说明
这个 PR 做了什么
作为 #7494 拆分后的第一个可独立评审改动，引入带版本的 Goal v3 状态契约。它定义生命周期状态、乐观并发控制、确定性状态迁移、严格的线协议解析、轮次边界持久化与恢复、旧状态迁移，以及面向现有消费方的兼容投影。
公共快照解析器接受全部合法运行活动（
idle、running和verifying），持久化生命周期记录则仍只接受稳定的idle边界。分阶段迁移期间，现有 Goal 行为继续保留。为什么需要
原始 Goal 重设计横跨 core runtime、会话重放、CLI、ACP、SDK、WebShell 和 Desktop。先合入权威状态契约，可以为后续 PR 提供小而稳定的依赖，避免一次评审完整的跨界面实现。
严格解析和版本校验可以避免畸形或过期生命周期状态被静默接受。恢复逻辑遇到最新无效记录时会明确停止，不会错误复活更旧状态；旧会话迁移时只继承目标文本，并创建新的身份和证据边界。
评审测试计划
如何验证
确认创建、替换、编辑、暂停、恢复、清除和轮次完成操作遵守身份、修订号、游标、活动时长和状态不变量。确认过期修订号和非法迁移返回明确错误，不会因轮数或时长阈值自动终止 Goal，三种运行活动都能通过公共解析器往返，并且只有 idle 快照可以持久化。
确认恢复逻辑选择最新合法生命周期状态、拒绝畸形的最新记录、不复活已停止的旧 Goal，并将活跃旧 Goal 迁移为新的 v2 身份。确认现有 Goal store、hook、judge 和 loop 测试继续通过。
本地结果：7 个测试文件通过，共 138 项测试通过。Core 类型检查与构建通过。构建后的公共 Goal 线协议子路径可成功导入，并能解析 running 快照。
证据（改动前后）
不适用——本 PR 只定义非 UI 的状态和持久化契约。TUI 与 WebShell 的用户界面证据会分别附在对应的后续 PR 中。
测试平台
环境（可选）
Node.js 24，无沙箱。
风险与范围
关联事项
从 #7494 拆分。