feat(core): add Goal v3 worker tools - #7729
Conversation
Local verification report
Covered chains:
No TUI or Web Shell screenshots are attached because registration and client wiring are intentionally outside this PR. |
Review + Linux verification report (clean install, full goals suite + typecheck)Verdict: the contract design is solid and thoroughly pinned by its tests. 269/269 goals-package tests and the full workspace What the design gets right
Verification evidence (head cdf577-era tarball of
|
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.
中文说明
已审查。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
| return await runtime.getGoalForWorker(permit); | ||
| } catch (error) { | ||
| if ( | ||
| error instanceof Error && | ||
| (error.message === 'Goal runtime has been disposed' || | ||
| error.message === STALE_GOAL_TURN_MESSAGE) | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] 'Goal runtime has been disposed' is independently hardcoded in goal-runtime.ts (where it is thrown) and here (where it is matched by string). No shared constant links the two modules. — Failure scenario: a refactor in goal-runtime.ts changes the disposal message text → workerViewForPermit no longer matches → the raw disposal error escapes to the LLM consumer instead of the stable STALE_GOAL_TURN_MESSAGE, violating the stale-permit contract without any compile-time or test failure alerting the change.
中文说明
[Suggestion] 'Goal runtime has been disposed' 在 goal-runtime.ts（抛出位置）和此处（字符串匹配位置）中分别硬编码，两个模块之间没有共享常量关联。— 失败场景：goal-runtime.ts 中的重构修改了 dispose 消息文本 → workerViewForPermit 无法匹配 → 原始 dispose 错误泄露给 LLM 消费者，而非返回稳定的 STALE_GOAL_TURN_MESSAGE，违反 stale-permit 契约且无编译时或测试失败提醒。
— qwen3.7-max via Qwen Code /review
| blockerKind: 'authority', | ||
| }), | ||
| ); | ||
| await runtime.dispatch({ | ||
| action: 'pause', |
There was a problem hiding this comment.
[Suggestion] blockerKind is never tested reaching recordTerminalProposal — the only test that builds with blockerKind: 'authority' invalidates the permit before execute, so the forwarding path is untested. — Failure scenario: a future refactor silently drops blockerKind from the proposal object at goal-tools.ts:214-217. Downstream consumers in goal-evidence.ts and goal-runtime.ts branch on blockerKind to distinguish authority/external from repeated blockers — a silent drop changes blocker audit behavior without a test failure.
Suggested fix: add a test that runs a status: 'blocked' proposal with blockerKind to completion (no permit invalidation) and asserts recordTerminalProposal receives a proposal containing blockerKind: 'authority'.
中文说明
[建议] blockerKind 从未被测试到达 recordTerminalProposal — 唯一使用 blockerKind: 'authority' 构建的测试在执行前就使 permit 失效，因此转发路径未被测试。— 失败场景：未来重构静默丢弃 goal-tools.ts:214-217 处提案对象中的 blockerKind。goal-evidence.ts 和 goal-runtime.ts 中的下游消费者根据 blockerKind 区分 authority/external 和 repeated blockers — 静默丢弃会改变阻塞审计行为且无测试失败。
建议修复：添加一个测试，使用 status: 'blocked' 和 blockerKind 运行提案至完成（不使 permit 失效），并断言 recordTerminalProposal 接收到的提案包含 blockerKind: 'authority'。
— qwen3.7-max via Qwen Code /review
| expect(JSON.parse(String(result.llmContent))).toEqual({ | ||
| active: true, | ||
| snapshot, | ||
| evidenceCatalog: { |
There was a problem hiding this comment.
[Suggestion] The verifierFeedback projection branch in projectWorkerView (goal-tools.ts:375-378) is never exercised — no GetGoalTool test mock returns verifierFeedback in the worker view. — Failure scenario: if projectWorkerView is refactored and the verifierFeedback spread is dropped or renamed, the GetGoalTool response silently omits verifier feedback that the LLM worker relies on to adjust behavior after a verification retry. No test in this file would detect the regression.
Suggested fix: extend this test (or add a new one) where getGoalForWorker resolves with verifierFeedback: 'retry: missing edge case' and assert the parsed llmContent includes verifierFeedback: 'retry: missing edge case'.
中文说明
[建议] projectWorkerView（goal-tools.ts:375-378）中的 verifierFeedback 投影分支从未被测试 — 没有 GetGoalTool 测试 mock 在 worker view 中返回 verifierFeedback。— 失败场景：如果 projectWorkerView 被重构且 verifierFeedback 展开被丢弃或重命名，GetGoalTool 响应会静默省略 verifier 反馈，而 LLM worker 依赖此反馈在验证重试后调整行为。此文件中没有测试会检测到回归。
建议修复：扩展此测试（或新增测试），使 getGoalForWorker 解析为 verifierFeedback: 'retry: missing edge case'，并断言解析后的 llmContent 包含 verifierFeedback: 'retry: missing edge case'。
— qwen3.7-max via Qwen Code /review
| export interface GoalToolResult extends ToolResult { | ||
| terminateTurn?: boolean; | ||
| } |
There was a problem hiding this comment.
[Suggestion] terminateTurn is a new optional field set by UpdateGoalInvocation.execute() but never read by any production consumer in the codebase. The base ToolResult interface has no terminateTurn field, and no session loop, tool execution pipeline, or result-processing code reads it. — Failure scenario: when these tools are eventually registered in the tool pipeline, terminateTurn: true will be silently dropped — the model continues producing output instead of ending the turn as the tool contract intends.
Suggested fix: either add terminateTurn to the ToolResult interface and wire it into the session loop to break the turn, or remove the field if the nextAction LLM-facing text is the intended mechanism.
中文说明
[建议] terminateTurn 是 UpdateGoalInvocation.execute() 设置的新可选字段，但代码库中没有任何生产消费者读取它。基础 ToolResult 接口没有 terminateTurn 字段，没有 session 循环、工具执行管道或结果处理代码读取它。— 失败场景：当这些工具最终注册到工具管道时，terminateTurn: true 将被静默丢弃 — 模型继续输出而非按工具契约预期结束本轮。
建议修复：将 terminateTurn 添加到 ToolResult 接口并连接到 session 循环以中断本轮，或者如果 nextAction 面向 LLM 的文本是预期机制，则移除此字段。
— qwen3.7-max via Qwen Code /review
| const evidenceEntries = view.evidenceCatalog?.entries; | ||
| if (evidenceEntries) { |
There was a problem hiding this comment.
[Suggestion] evidenceRefs validation is gated on evidenceCatalog being truthy; when it is undefined, any evidenceRefs bypass all validation and reach recordTerminalProposal unchallenged. — Failure scenario: production getGoalForWorker returns a GoalWorkerView without evidenceCatalog when no evidenceSource is configured (goal-runtime.ts:762-770). In that path, view.evidenceCatalog?.entries evaluates to undefined, if (evidenceEntries) is falsy, and the entire evidence-validation block is skipped. The proposal is recorded with whatever evidenceRefs the model passed — fabricated UUIDs are accepted.
中文说明
[建议] evidenceRefs 验证以 evidenceCatalog 为真值作为门控；当其为 undefined 时，任何 evidenceRefs 都绕过所有验证并无条件到达 recordTerminalProposal。— 失败场景：当未配置 evidenceSource 时，生产环境的 getGoalForWorker 返回一个不包含 evidenceCatalog 的 GoalWorkerView（goal-runtime.ts:762-770）。在该路径中，view.evidenceCatalog?.entries 求值为 undefined，if (evidenceEntries) 为假，整个证据验证块被跳过。提案以模型传入的任意 evidenceRefs 被记录 — 伪造的 UUID 被接受。
— qwen3.7-max via Qwen Code /review
| const citesDeliveredOutput = evidenceEntries.some( | ||
| (entry) => | ||
| citedEvidenceRefs.has(entry.uuid) && | ||
| entry.proofKind === 'delivered_output', | ||
| ); | ||
| const uncitedCurrentDeliveredOutput = citesDeliveredOutput | ||
| ? evidenceEntries |
There was a problem hiding this comment.
[Suggestion] The uncited-current-turn-delivered-output guard only activates when at least one cited evidenceRefs entry is itself a delivered_output. When the LLM cites zero delivered_output entries — even though the catalog contains delivered_output entries from the current turn — the guard is dormant. — Failure scenario: the LLM calls update_goal with status: 'complete' citing only a tool_result evidence ref while the catalog contains a current-turn delivered_output entry. citesDeliveredOutput is false, uncitedCurrentDeliveredOutput is [], and recordTerminalProposal is called. The proposal is recorded without the verifier seeing the current turn's delivered output.
| const citesDeliveredOutput = evidenceEntries.some( | |
| (entry) => | |
| citedEvidenceRefs.has(entry.uuid) && | |
| entry.proofKind === 'delivered_output', | |
| ); | |
| const uncitedCurrentDeliveredOutput = citesDeliveredOutput | |
| ? evidenceEntries | |
| const citesDeliveredOutput = evidenceEntries.some( | |
| (entry) => | |
| citedEvidenceRefs.has(entry.uuid) && | |
| entry.proofKind === 'delivered_output', | |
| ); | |
| const uncitedCurrentDeliveredOutput = evidenceEntries | |
| .filter( | |
| (entry) => | |
| entry.proofKind === 'delivered_output' && | |
| entry.turnId === permit.turnId && | |
| !citedEvidenceRefs.has(entry.uuid), | |
| ) | |
| .map((entry) => entry.uuid); |
中文说明
[建议] 未引用的当前轮次 delivered_output 守卫仅在至少一个被引用的 evidenceRefs 条目本身是 delivered_output 时才激活。当 LLM 未引用任何 delivered_output 条目时 — 即使目录中包含当前轮次的 delivered_output 条目 — 该守卫处于休眠状态。— 失败场景：LLM 调用 update_goal 时 status: 'complete' 仅引用 tool_result 证据 ref，而目录包含当前轮次的 delivered_output 条目。citesDeliveredOutput 为 false，uncitedCurrentDeliveredOutput 为 []，recordTerminalProposal 被调用。提案被记录时 verifier 无法看到当前轮次的交付输出。
— qwen3.7-max via Qwen Code /review
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Round-2 verification (head with
|
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. 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
| if (!this.runtime || !this.permit) { | ||
| throw new Error('No active Goal is available for this turn'); | ||
| } |
There was a problem hiding this comment.
[Suggestion] UpdateGoalTool throws a raw Error when called outside a goalTurnContext, but no test covers this behavior. By contrast, GetGoalTool returns a structured response ({ active: false }) for the same condition, and that path is tested at goal-tools.test.ts:103–113. — Failure scenario: if the throw-vs-return contract is ever changed (e.g., to match GetGoalTool's graceful return), no test would catch the regression.
| if (!this.runtime || !this.permit) { | |
| throw new Error('No active Goal is available for this turn'); | |
| } | |
| if (!this.runtime || !this.permit) { | |
| return { | |
| llmContent: JSON.stringify({ active: false, error: 'No active Goal is available for this turn' }), | |
| returnDisplay: 'No active Goal is available for this turn.', | |
| }; | |
| } |
Alternatively, add a test asserting the current throw behavior:
it('throws when called outside a permitted Goal turn', async () => {
const runtime = activeRuntime();
const invocation = new UpdateGoalTool(makeConfig(runtime)).build({
status: 'complete',
reason: 'test',
evidenceRefs: ['ref-1'],
});
await expect(invocation.execute()).rejects.toThrow('No active Goal is available for this turn');
});— 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. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
| } else if ( | ||
| receipt.readyForVerification && | ||
| snapshot.goal?.goalId === this.permit.goalId && | ||
| snapshot.goal.revision === this.permit.revision && | ||
| snapshot.goal.status === 'active' |
There was a problem hiding this comment.
[Suggestion] The goalId and revision checks in this else if are dead conditions — snapshotForPermit on line 213 already throws if either mismatches the permit, so both are unconditionally true here. The only discriminating conditions are receipt.readyForVerification and snapshot.goal.status === 'active'. The early snapshotForPermit call at line 142 (whose return value is discarded) adds a redundant clone of the entire snapshot. — Failure scenario: no runtime bug today, but the dead conditions mask the actual decision logic. A future refactor that removes the seemingly-useless early call (it returns nothing) and later removes the post-recording snapshotForPermit would leave these checks as the sole validation — but they no longer throw, they just fall through to the else branch silently.
| } else if ( | |
| receipt.readyForVerification && | |
| snapshot.goal?.goalId === this.permit.goalId && | |
| snapshot.goal.revision === this.permit.revision && | |
| snapshot.goal.status === 'active' | |
| } else if ( | |
| receipt.readyForVerification && | |
| snapshot.goal.status === 'active' |
中文说明
[Suggestion] 此 else if 中的 goalId 和 revision 检查是死条件——第 213 行的 snapshotForPermit 在任一不匹配时已经抛出异常，所以到这里两者必然为 true。真正起区分作用的条件只有 receipt.readyForVerification 和 snapshot.goal.status === 'active'。第 142 行的早期 snapshotForPermit 调用丢弃了返回值，额外做了一次完整的快照深拷贝。——失败场景：当前没有运行时 bug，但死条件掩盖了真正的决策逻辑。未来如果重构者删除看似无用的早期调用（它没有返回值），再删除录制后的 snapshotForPermit，这些检查就会变成唯一的校验——但它们不再抛出异常，只会静默落入 else 分支。
— qwen3.7-max via Qwen Code /review
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
[Suggestion] The throw error fallback for unexpected errors from getGoalForWorker has no test. — Failure scenario: existing tests cover the two normalized branches ('Goal runtime has been disposed' and STALE_GOAL_TURN_MESSAGE). If this throw error line were accidentally replaced with throw staleGoalTurnError() (silently swallowing unexpected errors as stale permits), no test would detect the regression.
| throw error; | |
| } | |
| } | |
| throw error; |
Consider adding a test where getGoalForWorker rejects with an unexpected error (e.g. new Error('unexpected database failure')) and asserting the error propagates as-is.
中文说明
[Suggestion] getGoalForWorker 抛出意外错误时的 throw error 回退路径没有测试。——失败场景：现有测试覆盖了两种已知错误的标准化分支（'Goal runtime has been disposed' 和 STALE_GOAL_TURN_MESSAGE）。如果这行 throw error 被意外替换为 throw staleGoalTurnError()（将意外错误静默当作过期 permit 处理），没有任何测试能检测到这一回归。
建议添加一个测试，让 getGoalForWorker 以意外错误（如 new Error('unexpected database failure')）拒绝，并断言该错误原样传播。
— qwen3.7-max via Qwen Code /review
|
Round-3 note: the locale-key completion commit is verified trivial — |
Review — Goal v3 worker tools (verified at
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 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
| ...(this.params.blockerKind | ||
| ? { blockerKind: this.params.blockerKind } | ||
| : {}), |
There was a problem hiding this comment.
[Suggestion] No test verifies that blockerKind reaches recordTerminalProposal. The only test using blockerKind: 'authority' invalidates the permit before execute, so the forwarding path is untested. — Failure scenario: if the conditional spread were removed or the field name changed, recordTerminalProposal would silently receive blockerKind: undefined, causing all blocked proposals to enter the blocked-audit path instead of immediate verification readiness. No test would catch it.
— qwen3.7-max via Qwen Code /review
| returnDisplay = | ||
| 'Proposal recorded for blocker audit; it is not yet ready for independent verification and no terminal lifecycle change was committed.'; |
There was a problem hiding this comment.
[Suggestion] The fourth returnDisplay branch — reached when receipt.recorded is true, readyForVerification is false, and the goal is not paused — has no test. — Failure scenario: submitting a blocked proposal with blockerKind: 'repeated' (below audit threshold) produces this "blocker audit" display string, which is never asserted. A typo or wrong string in this branch would ship undetected.
— 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. Suggestions are inline. 3 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查——无阻断问题。 建议见行内评论。 3 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— qwen3.7-max via Qwen Code /review
| export * from './goal-evidence.js'; | ||
| export * from './goal-verifier.js'; | ||
| export * from './goal-runtime.js'; | ||
| export { goalTurnContext } from './goal-turn-context.js'; |
There was a problem hiding this comment.
[Suggestion] goal-tools.ts exports (GetGoalTool, UpdateGoalTool, GoalToolConfig) are missing from this barrel — every other goals module (goal-runtime.js, goal-evidence.js, goal-verifier.js, etc.) re-exports through index.ts. When the follow-up PR registers these tools in createToolRegistry(), the factory cannot import { GetGoalTool } from '../goals/index.js' and must reach into ../goals/goal-tools.js directly.
Concrete cost: inconsistent import paths across the codebase once tool registration lands.
| export { goalTurnContext } from './goal-turn-context.js'; | |
| export { goalTurnContext } from './goal-turn-context.js'; | |
| export { | |
| GetGoalTool, | |
| UpdateGoalTool, | |
| } from './goal-tools.js'; | |
| export type { | |
| GoalToolConfig, | |
| GetGoalToolParams, | |
| UpdateGoalToolParams, | |
| GoalToolResult, | |
| } from './goal-tools.js'; |
中文说明
[建议] goal-tools.ts 的导出（GetGoalTool、UpdateGoalTool、GoalToolConfig）未加入此 barrel —— 其他所有 goals 模块（goal-runtime.js、goal-evidence.js、goal-verifier.js 等）都通过 index.ts 重新导出。当后续 PR 在 createToolRegistry() 中注册这些工具时，工厂无法通过 import { GetGoalTool } from '../goals/index.js' 导入，而必须直接引用 ../goals/goal-tools.js。
具体代价：工具注册落地后，代码库中会出现不一致的导入路径。
— qwen3.7-max via Qwen Code /review
Round-2 review — Goal v3 worker tools (re-verified at
|
| Round-1 finding | Fix | Mutation result |
|---|---|---|
nextAction built before branching — ordered "End this turn" even on audit-only blockers where terminateTurn is correctly absent |
:218 now branches on receipt.readyForVerification |
revert to unconditional → 1 test fails ✅ |
terminateTurn always-true mutant was green (untested) |
new keeps audit-only blocker proposals in the current turn test |
terminateTurn: true unconditionally → 1 test fails ✅ (was green) |
Trimmed-duplicate check was vacuous — its test passed on ajv's uniqueItems message via a /unique|duplicate/i alternation |
test now uses ['same-reference', ' same-reference '] (ajv can't see it) + asserts the exact custom message |
delete :299-304 → 1 test fails ✅ (was green) |
Impossible instruction: "call get_goal in that same response before update_goal" — same-response tool calls are chosen before results exist |
reworded to "call get_goal, wait for its result, and call update_goal in a later model step", pinned by a .not.toContain('in that same response') assertion |
n/a — string contract, negative assertion prevents regression |
The returnDisplay/nextAction disagreement is genuinely gone: for an audit-only repeated blocker the model now gets Continue this turn… and no terminateTurn, matching the receipt.
Remaining findings
1. Unbounded delivered-output guard vs. the 12-item schema cap — carried over, unchanged (goal-tools.ts:185 vs :268)
update_goal requires every current-turn delivered_output entry to be cited, but evidenceRefs is capped at maxItems: 12 (matching VERIFIER_REFERENCE_LIMIT in goal-evidence.ts:18). With 13 current-turn delivered-output entries in the catalog, completion is unreachable in both directions — probe against the real tool:
B1 cite 13 -> BUILD REJECTED: "params/evidenceRefs must NOT have more than 12 items"
B2 cite 12 -> {"proposalRecorded":false,"readyForVerification":false,
"uncitedCurrentDeliveredOutput":["delivered-12"],
"error":"The completion proposal omitted delivered output from the current Goal turn…"}
The catalog admits up to CATALOG_ENTRY_LIMIT = 100 entries and proofKindOf('assistant_output') === 'delivered_output', so any turn with ≥13 recorded assistant outputs hits this. Still not live — there is no in-tree producer of goalContext on transcript records and no non-test evidenceSource wiring — so this is a design risk for whichever slice lands the transcript producer, not a defect here. Worth resolving now while the contract is cheap to change: bound the guard to the most recent ≤12 current-turn delivered outputs, or coalesce a turn's delivered output into one catalog entry.
2. update_goal fails hard where get_goal fails gracefully — and the path is completely untested (goal-tools.ts:137)
A1 get_goal (no permit) -> {"llmContent":"{\"active\":false}","returnDisplay":"No active Goal is available for this turn."}
A2 update_goal (no permit) -> THREW: Error "No active Goal is available for this turn"
Every other rejection in this tool returns a structured {proposalRecorded:false, …, error} payload the model can act on; this one throws a raw Error that surfaces as a tool failure. Once registration lands, a model calling update_goal with no active Goal is an ordinary, expected event — the structured shape is the better contract. It is also unpinned: replacing the whole if (!this.runtime || !this.permit) body with a stub return leaves 21/21 green.
3. Two dead branches in the returnDisplay chain (goal-tools.ts:226-235)
snapshot.goal?.status === 'paused' at :232 is unreachable. dispatch({action:'pause'}) clears currentPermit (goal-runtime.ts:876-885), so getGoalForWorker throws the stale-permit error long before the display chain runs — the PR's own rejects a proposal after pause invalidates its permit test proves exactly that. Deleting the branch: 21/21 green. The && snapshot.goal?.status === 'active' guard at :227-229 is likewise always true at that point — dropping it: 21/21 green. With both gone the chain collapses to three cases, and the second snapshotForPermit(...) at :213 exists only to read a status that can only be 'active'.
Worth flagging beyond the cleanup: the else fallback labels everything it catches "Proposal recorded for blocker audit; it is not yet ready for independent verification". If a status ever becomes reachable there that isn't active/paused, a readyForVerification: true proposal gets a user-facing display that contradicts its own terminateTurn: true. Branching on receipt.readyForVerification rather than on snapshot status would be failure-proof.
4. The status === 'complete' scoping of the uncited guard is still untested (goal-tools.ts:185)
Deleting the condition leaves 21/21 green. The behaviour is correct — probe B3 confirms a blocked proposal citing 12 of 13 delivered outputs is recorded with readyForVerification: true — but nothing pins it, so a future edit could silently start rejecting blocker proposals for uncited delivered output.
Smaller items
execute()ignores theAbortSignal(:64,:135).get_goalawaitsevidenceSource.flush()andreadActiveTranscriptChain()— real I/O once wired — with no cancellation path.terminateTurnhas no consumer in-tree (onlygoal-tools.tsand its test). Deferred by design per the PR body; noting it so the consuming slice doesn't ship without it.ca.jscarries othertoolDisplayName.*keys but didn't getGoal/UpdateGoal. Not CI-gated (onlyzh/zh-TWhavestrictParity), so optional.
What holds up (unchanged from round 1)
Capture-at-build / validate-at-execute is real, not decorative — I re-confirmed by mutation that deleting the discarded snapshotForPermit(...) at :142 fails 2 tests and lets recordTerminalProposal run against a mismatched session, and that the typeof getSnapshot !== 'function' guard fails 1 test. Every delayed-execution path (edit / replace / clear / finish / dispose / session swap / pause) funnels into one stable error, and the read tool never touches the session runtime outside a permitted turn.
Verification
macOS, isolated worktree at eebac98a8: goal-tools + goal-turn-context 22/22; full src/goals suite 271/271 across 13 files; Prettier clean on all changed files; npm run check-i18n ✅. Seven single-point mutations run (4 killed, 3 green — the three green ones are findings 2, 3, and 4 above).
中文说明
结论
这是对 bbbe160d4 首轮评审的复审。我在隔离 worktree 中对新 head 重跑了同一套可执行探针和变异矩阵，下面的结论都来自实际执行，而不是读 diff。
仍然没有 blocker，并且这次的修复是真修复——首轮 5 个问题中已关闭 3 个，且每个修复都经变异验证确实被测试锁住。
已确认修复
nextAction现在按readyForVerification分支（:218）：改回无条件版本 → 1 个测试失败 ✅。仅审计用的repeatedblocker 现在返回「继续本轮」且没有terminateTurn，与 receipt 一致。terminateTurn只在 ready 时发出：改成无条件true→ 1 个测试失败 ✅（首轮该变异是绿的）。- 去空格重复校验不再是空测试：测试改用
['same-reference', ' same-reference ']（ajv 的uniqueItems看不到），并断言精确的自定义消息；删除:299-304→ 1 个测试失败 ✅（首轮是绿的）。 - 不可能执行的指令已改写：「在同一个 response 里调用
get_goal」→「调用get_goal，等待其结果，在后续 model step 中调用update_goal」，并用.not.toContain('in that same response')防回归。
遗留问题
- 无上界的 delivered-output 校验 vs. 12 条 schema 上限（
:185vs:268，沿用首轮，未变）。当前轮有 13 条 delivered-output 时完成无法达成：引用 13 条 → ajv 报「不得超过 12 项」；引用 12 条 →uncitedCurrentDeliveredOutput:["delivered-12"]。目录上限是 100 条，且assistant_output一律映射为delivered_output。由于树内还没有goalContext记录的生产者，也没有非测试的evidenceSource接线，目前尚不可触发——属于后续切片的设计风险。建议趁契约还便宜时处理：把校验限制在最近 ≤12 条，或把一轮的 delivered output 合并成一条目录项。 update_goal硬失败，而get_goal优雅降级（:137）。无 permit 时get_goal返回{active:false}，update_goal抛原始Error。本工具其它所有拒绝路径都返回结构化的{proposalRecorded:false, …, error}；注册落地后「没有活跃 Goal 时调用update_goal」是很正常的事件，结构化返回是更好的契约。该路径完全没有测试：把整个分支体换成桩返回，21/21 仍然全绿。returnDisplay链里有两个死分支（:226-235）。status === 'paused'（:232）不可达——pause会清空currentPermit（goal-runtime.ts:876-885），getGoalForWorker会先抛 stale-permit 错误，PR 自己的 pause 测试正好证明了这点；删除后 21/21 全绿。:227-229的status === 'active'判断在该处恒为真，去掉后同样全绿。另外else兜底会把所有落入其中的情况都标成「blocker audit，尚未可验证」，一旦出现新的可达状态，readyForVerification: true的提案就会拿到与自身terminateTurn: true矛盾的展示文案；按receipt.readyForVerification分支会更稳。- 校验中的
status === 'complete'限定仍无测试（:185）。删掉该条件后 21/21 全绿。行为本身是对的（探针 B3 确认blocked提案引用 13 条中的 12 条仍会被记录），但没有测试锁住。
次要项
execute()忽略AbortSignal（:64、:135）；get_goal会 awaitflush()和readActiveTranscriptChain()，接线后是真实 I/O，却无法取消。terminateTurn树内无消费者，按 PR 说明属于后续切片，这里只作提醒。ca.js有其它toolDisplayName.*键但没补Goal/UpdateGoal；CI 未对其做 parity 门禁（只有zh/zh-TW有strictParity），可选。
仍然成立的优点
capture-at-build / validate-at-execute 是实打实的：删除 :142 处被丢弃的 snapshotForPermit(...) → 2 个测试失败，且 recordTerminalProposal 会在不匹配的 session 上被调用；删除 typeof getSnapshot !== 'function' 守卫 → 1 个测试失败。所有延迟执行路径（edit / replace / clear / finish / dispose / session 切换 / pause）都汇聚到同一个稳定错误。
验证
macOS，隔离 worktree，head eebac98a8：goal-tools + goal-turn-context 22/22；src/goals 全量 271/271（13 个文件）；改动文件 Prettier 全部通过；npm run check-i18n ✅。共跑 7 个单点变异（4 个被杀，3 个仍绿——绿的 3 个即上面的第 2、3、4 项）。
| type: 'object', | ||
| properties: { | ||
| status: { type: 'string', enum: ['complete', 'blocked'] }, | ||
| reason: { type: 'string', minLength: 1 }, |
There was a problem hiding this comment.
[Critical] Bound proposal reasons before recording them
reason has no upper bound, but the downstream verifier rejects requests above 64 KB. A valid update_goal call with a current evidence UUID and a sufficiently large reason is therefore recorded as ready, ends the turn, then throws GoalVerifierInputTooLargeError; the runtime classifies that generic verifier error as usage_limited and persists the Goal in that state without a verifier decision. Enforce a shared character and UTF-8 byte limit in both the schema and runtime validation before recordTerminalProposal, leaving room for the rest of the verifier payload, and reject oversized input as an invalid tool argument.
— Codex GPT-5 via Qwen Code /review
There was a problem hiding this comment.
已修复
验证证据：commit 9597bf2 在 tool schema、tool 参数校验与 GoalRuntime 记录入口共享 8,000 字符 / 16,000 UTF-8 字节上限；超限不会占用提案槽。npx vitest run src/goals：279/279 通过；npm run build、npm run typecheck 通过。
| if (typeof getSnapshot !== 'function') { | ||
| throw staleGoalTurnError(); | ||
| } | ||
| const snapshot = getSnapshot.call(runtime); |
There was a problem hiding this comment.
[Critical] Recheck the full permit and runtime availability atomically
Disposal can run after getGoalForWorker() performs its final permit check but before this continuation resumes. Because dispose() leaves the snapshot intact and getSnapshot() does not assert availability, this goal/revision-only check passes: a real-runtime microtask probe returned active: true with the disposed session's old snapshot and evidence catalog, while the equivalent update_goal path leaked raw Goal runtime has been disposed. Add a synchronous runtime operation that atomically asserts the runtime is operational and the complete permit, including turnId, is current while returning the snapshot; use it after the awaited read and normalize stale/disposal errors from subsequent runtime calls.
— Codex GPT-5 via Qwen Code /review
There was a problem hiding this comment.
已修复
验证证据：commit 9597bf2 新增同步 getSnapshotForPermit，原子校验 operational 状态及 goalId/revision/turnId，并在 worker read、snapshot 与 proposal 记录路径统一归一化 stale/disposal。真实 runtime dispose 竞态用例通过；Goal 回归 279/279 通过。
| if ( | ||
| view.goalId !== this.permit.goalId || | ||
| view.revision !== this.permit.revision | ||
| ) { | ||
| throw staleGoalTurnError(); | ||
| } | ||
| const payload = projectWorkerView(view, snapshot); |
There was a problem hiding this comment.
[Suggestion] Exercise the worker-view identity guards directly
Current mismatch tests either make getGoalForWorker() throw or make the snapshot mismatch first, so neither resolved-view identity check is reached. A future routing regression could return Goal B's worker view while the snapshot still identifies permitted Goal A; removing or inverting this guard would then expose Goal B's evidence or validate Goal A against it without failing the existing suite. Add table-driven tests for both tools with a mismatched returned goalId or revision and a matching snapshot, asserting the stable stale-permit error and no proposal recording.
— Codex GPT-5 via Qwen Code /review
There was a problem hiding this comment.
已修复
验证证据：commit 9597bf2 新增 get_goal/update_goal × goalId/revision 表驱动用例，在匹配 snapshot 下直接触发 worker-view identity guard，并断言不记录 proposal；Goal 回归 279/279 通过。
| 'A transcript record uuid from evidenceCatalog.entries, not a turnId or lineageTurnId.', | ||
| }, | ||
| }, | ||
| blockerKind: { |
There was a problem hiding this comment.
[Suggestion] Document the blocker modes' scheduling contract
This optional enum does not tell the model that authority and external blockers can proceed immediately while omission is treated as a repeated-blocker audit requiring the same reason across three turns. If the model omits the field for a missing-authority blocker and naturally paraphrases its reason on retries, the fingerprint resets and the Goal can loop indefinitely. Document each mode and its evidence requirements in the tool/schema, require or explicitly define omission for blocked proposals, and include the three-turn stable-reason recovery rule in the non-ready nextAction.
— Codex GPT-5 via Qwen Code /review
There was a problem hiding this comment.
已修复
验证证据：commit 9597bf2 在 tool 描述与 blockerKind schema 中明确 authority/external/repeated 语义、缺省为 repeated audit，并在 non-ready nextAction 中写明连续三轮使用相同模式与实质相同 reason；相关契约测试及 Goal 回归 279/279 通过。
| type: 'string', | ||
| enum: ['authority', 'external', 'repeated'], | ||
| description: | ||
| 'authority: a user or maintainer decision or permission is required; external: an evidenced external resource or capability is unavailable; repeated: the same evidenced blocker with a materially identical reason across three consecutive Goal turns. Omission uses the repeated-blocker audit.', |
There was a problem hiding this comment.
[Critical] Normalize the omitted blocker mode before auditing
This description makes omission equivalent to repeated, but recordTerminalProposal fingerprints the two allowed forms as "\n<reason>" and "repeated\n<reason>". If three consecutive turns alternate omission and explicit repeated while reporting the same evidenced blocker and reason, every switch resets the count and the Goal never reaches blocked verification. Normalize omission to repeated before constructing the fingerprint and cover a mixed-form three-turn sequence.
— Codex GPT-5 via Qwen Code /review
There was a problem hiding this comment.
已修复
验证证据：commit a0581d2 将省略的 blockerKind 在 durable audit 指纹中归一化为 repeated；混合“省略 → repeated → 省略”的三轮回归用例通过。npx vitest run src/goals：280/280 通过；npm run build && npm run typecheck 通过。
| goalLifecycleChanged: false, | ||
| nextAction: receipt.readyForVerification | ||
| ? 'End this turn without user-facing text. Do not claim the Goal is complete or blocked. The Goal status card will report the independent verification result.' | ||
| : 'Continue this turn without claiming the Goal is complete or blocked. A repeated-blocker audit requires the same blocker mode and materially identical reason across three consecutive Goal turns, with current evidence cited on each turn.', |
There was a problem hiding this comment.
[Critical] Align “materially identical” with the durable audit
The response promises that materially identical reasons accumulate, but the runtime fingerprints the free-form reason byte-for-byte. A model can report the same blocker as “waiting for access”, “still waiting for access”, and “access is still unavailable”; each accepted proposal resets the audit to one, so blocked verification is never reached despite following this guidance. Either require exact reason reuse in the contract or fingerprint a stable blocker identifier/canonical representation, with a regression test for the chosen behavior.
— Codex GPT-5 via Qwen Code /review
There was a problem hiding this comment.
已修复
验证证据：commit a0581d2 将 tool 描述、schema 与 non-ready nextAction 统一为必须复用完全相同的 reason 文本，与 runtime 的 durable byte-exact audit 一致；Goal 回归 280/280 通过。
|
|
||
| export const GOAL_STATE_VERSION = 2 as const; | ||
| export const GOAL_PROPOSAL_REASON_MAX_CHARACTERS = 8_000; | ||
| export const GOAL_PROPOSAL_REASON_MAX_BYTES = 16_000; |
There was a problem hiding this comment.
[Critical] Keep valid inputs within the verifier request budget
A valid 24,000-byte delivered-output evidence record is serialized both in evidence and currentDeliveredOutput; adding a valid 16,000-byte reason already reaches roughly 64,000 bytes before goal, proposal, and evidence metadata. The verifier then throws GoalVerifierInputTooLargeError, and the Goal becomes usage_limited even though every individual input satisfies its limit. Budget these limits against the fully serialized request or avoid duplicating delivered output.
— Codex GPT-5 via Qwen Code /review
There was a problem hiding this comment.
已修复
验证证据：commit a0581d2 用 currentTurnId 标识当前轮 delivered_output，移除 evidence content 的重复序列化；24,000-byte evidence + 16,000-byte reason 的回归用例通过，Goal 回归 280/280、build/typecheck 通过。
| */ | ||
|
|
||
| export const GOAL_STATE_VERSION = 2 as const; | ||
| export const GOAL_PROPOSAL_REASON_MAX_CHARACTERS = 8_000; |
There was a problem hiding this comment.
[Suggestion] Cover the inclusive reason boundaries
The tests exercise short valid values and values just over these limits, but not exactly 8,000 characters or 16,000 UTF-8 bytes. An accidental > to >= change would reject documented maximum-sized reasons while the current tests still pass. Add exact-boundary acceptance cases for both limits.
— Codex GPT-5 via Qwen Code /review
There was a problem hiding this comment.
已修复
验证证据：commit a0581d2 新增 8,000 字符与 16,000 UTF-8 字节精确边界接受用例；Goal 回归 280/280 通过。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
| const view = await workerViewForPermit(this.runtime, permit); | ||
| snapshotForPermit(this.runtime, permit); | ||
| if ( | ||
| view.goalId !== this.permit.goalId || |
There was a problem hiding this comment.
[Suggestion] snapshotForPermit is called here but its return value is discarded — the constructed GoalSnapshotV2 (including a structuredClone of the goal record) is thrown away. workerViewForPermit on the line above already performs the same assertOperational() and isCurrentPermit() checks, so this call adds no validation. — Failure scenario: no incorrect behaviour, but the wasted structuredClone costs a clone cycle on every update_goal call and misleads future readers into assuming the snapshot is consumed downstream. The purposeful snapshotForPermit call at line 225 (after recordTerminalProposal) remains.
| const view = await workerViewForPermit(this.runtime, permit); | |
| snapshotForPermit(this.runtime, permit); | |
| if ( | |
| view.goalId !== this.permit.goalId || | |
| const view = await workerViewForPermit(this.runtime, permit); | |
| if ( | |
| view.goalId !== this.permit.goalId || |
中文说明
[Suggestion] 此处调用了 snapshotForPermit 但返回值被丢弃——构建的 GoalSnapshotV2（包括 goal 记录的 structuredClone）被直接扔掉。上一行的 workerViewForPermit 已经执行了相同的 assertOperational() 和 isCurrentPermit() 检查，因此该调用没有提供任何额外校验。— 失败场景：不会产生错误行为，但浪费的 structuredClone 在每次 update_goal 调用时多消耗一次克隆周期，并误导后续读者以为快照会在下游被使用。第 225 行（recordTerminalProposal 之后）有目的性的 snapshotForPermit 调用保持不变。
— qwen3.7-max via Qwen Code /review
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: chunk 2, chunk 5, chunk 3, chunk 1, chunk 4 — launched with a prompt that is not the one the CLI built. Not reviewed: Test coverage matrix (whole-diff), Agent 1b: Removed-behavior audit, Agent 1c: Cross-file tracer, Agent 7: Build & test verification — 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
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: reverse-audit — stopped at the five-round hard cap before two consecutive dry rounds.
[Critical] packages/core/src/goals/goal-verifier.ts:103 — Blocked proposals can cite an older failure or unanswered authority request while omitting newer contradictory evidence. Blocker coverage requires only one cited user/tool record, and the verifier receives only cited records without ordering context, so it can incorrectly commit blocked after a later success or answer. Include all relevant bounded evidence, or at least every record newer than the cited blocker, and fail closed when that view cannot fit.
— Codex GPT-5 via Qwen Code /review
| proposal.blockerKind !== 'external' | ||
| ) { | ||
| const fingerprint = `${proposal.blockerKind ?? ''}\n${proposal.reason}`; | ||
| const fingerprint = `${proposal.blockerKind ?? 'repeated'}\n${proposal.reason}`; |
There was a problem hiding this comment.
[Critical] Migrate recovered repeated-blocker fingerprints
Version-2 state already persists omitted blocker kinds as \n<reason>. After upgrading, the third identical omitted-kind proposal produces repeated\n<reason>, fails the equality check, and resets a two-turn audit to one instead of becoming verifiable. Accept the legacy empty-kind form during comparison or migrate recovered audits to the canonical fingerprint before counting.
— Codex GPT-5 via Qwen Code /review
| async execute(): Promise<GoalToolResult> { | ||
| if (!this.runtime || !this.permit) { | ||
| throw new Error('No active Goal is available for this turn'); |
There was a problem hiding this comment.
[Critical] Honor cancellation before recording a proposal
Both Goal invocations omit the required AbortSignal. If a turn is cancelled while getGoalForWorker() is awaiting evidence I/O, update_goal can resume later and still call recordTerminalProposal, mutating Goal state from a cancelled tool call. Accept and observe the signal, race or interrupt the asynchronous read, and recheck immediately before proposal recording.
— Codex GPT-5 via Qwen Code /review
| revision: number; | ||
| objective: string; | ||
| }; | ||
| currentTurnId: string; |
There was a problem hiding this comment.
[Critical] Preserve the exported verifier-input contract
GoalVerifierInput is publicly re-exported, so replacing optional currentDeliveredOutput with required currentTurnId breaks existing TypeScript consumers. Already-compiled JavaScript callers silently lose the old field and omit the new identifier, degrading verifier input. Keep a deprecated compatibility branch or explicitly version and document this as a breaking API change.
— Codex GPT-5 via Qwen Code /review
|
|
||
| export interface GoalRuntime { | ||
| getSnapshot(): GoalSnapshotV2; | ||
| getSnapshotForPermit(permit: GoalTurnPermit): GoalSnapshotV2; |
There was a problem hiding this comment.
[Critical] Avoid breaking existing GoalRuntime implementers
GoalRuntime is publicly re-exported, and adding this required method makes every downstream class, mock, or object literal implementing the previous interface fail TypeScript compilation. The tool adapter already checks this capability at runtime, so preserve source compatibility by making it optional there or introduce a separate extended interface for permit-scoped snapshots.
— Codex GPT-5 via Qwen Code /review
| type: 'array', | ||
| minItems: 1, | ||
| uniqueItems: true, | ||
| maxItems: 12, |
There was a problem hiding this comment.
[Critical] Align the reference cap with required delivered output
Completion requires citing every current-turn delivered_output, but the schema and runtime allow only 12 references. A valid turn with 13 output records can neither cite all 13 nor omit one; even 12 outputs leave no slot for an external test result. Aggregate current-turn output separately or otherwise reserve enough bounded capacity, and add a regression covering this limit.
— Codex GPT-5 via Qwen Code /review
| ) { | ||
| throw staleGoalTurnError(); | ||
| } | ||
| const evidenceEntries = view.evidenceCatalog?.entries; |
There was a problem hiding this comment.
[Critical] Do not treat a truncated catalog as exhaustive
The catalog drops older records after 100 entries or 24 KB, but this validation ignores evidenceCatalog.truncated. An early incorrect visible message can fall outside the retained entries, allowing completion to cite only the later required output; the verifier also receives only cited records and never sees the violation. Fail closed when current-turn output is not provably exhaustive or include a separately bounded aggregate of all current-turn output.
— Codex GPT-5 via Qwen Code /review
| '{"decision":"accept","reason":"grounded"}', | ||
| ); | ||
| const value = input(); | ||
| value.proposal.reason = 'r'.repeat(16_000); |
There was a problem hiding this comment.
[Critical] Budget the serialized request, including escaping
This boundary test uses escape-free strings—and its 16,000-character reason is not protocol-valid under the 8,000-character cap. A valid 8,000-quote reason plus 24,000 quote bytes of evidence serializes to about 64,253 bytes, exceeds the 64,000-byte verifier limit, and is persisted as usage_limited. Size upstream limits against worst-case JSON encoding and cover quote/backslash/control-heavy valid inputs.
— Codex GPT-5 via Qwen Code /review
| const proposal: GoalTerminalProposal = { | ||
| status: this.params.status, | ||
| reason: this.params.reason.trim(), | ||
| evidenceRefs: this.params.evidenceRefs.map((reference) => |
There was a problem hiding this comment.
[Suggestion] Exercise a valid catalog-backed proposal
Catalog-backed tests cover rejection only, while every successful recording test uses a runtime without an evidenceCatalog. A regression that rejects all valid catalog UUIDs could therefore leave the suite green. Add a happy-path case with a real catalog UUID and assert the normalized proposal reaches recordTerminalProposal.
— Codex GPT-5 via Qwen Code /review
Round 3 — local build & real-execution verification at
|
| # | Finding (round) | Status at 70f977ba4 |
How I checked |
|---|---|---|---|
| R1-1 | nextAction returned unconditionally |
✅ Fixed at eebac98a8 |
still branch-pinned, re-confirmed |
| R1-2 | trimmed-duplicate test was vacuous | ✅ Fixed at eebac98a8 |
still kills its mutant |
| R1-3 | impossible "call get_goal in that same response" instruction |
✅ Fixed at eebac98a8 |
.not.toContain assertion still present |
| R2-A | maxItems: 12 unreachable vs. the unbounded delivered-output guard |
✅ Fixed at 9597bf27e |
cap is now GOAL_EVIDENCE_REFERENCE_LIMIT = CATALOG_ENTRY_LIMIT (100). Mutant M16 (revert to 12) is killed by the new test admits thirteen delivered outputs plus independent evidence — the fix is load-bearing |
| R2-B | update_goal hard-throws where get_goal degrades (goal-tools.ts:147-149) |
mutant M7 (replace the whole throw with a stub return) → 284/284 still green | |
| R2-C | dead returnDisplay branch for status === 'paused' (goal-tools.ts:255-258) |
mutant M6 (delete the branch) → survives. The PR's own test rejects a proposal after pause invalidates its permit proves execute() rejects with the stale-permit error before that branch can be reached |
|
| R2-D | status === 'complete' in the uncited-output guard is unpinned |
mutant M3 → survives |
2 · New finding 1 — a Goal past 16 turns can never be proposed complete (blocker, contract-level)
buildGoalEvidenceCatalog sets one truncated flag from two unrelated conditions (goal-evidence.ts:148-151):
analysis.catalogTruncated— evidence entries were actually dropped, andanalysis.lineageTurnIds.length > CATALOG_LINEAGE_LIMIT(16) — only the returned turn-id list was trimmed.
goal-tools.ts:188-193 refuses every completion proposal on that flag. So from Goal turn 17 onward update_goal status:"complete" is refused even though the catalog is fully exhaustive — and it never recovers, because each new turn makes the lineage one longer, never shorter. The evidence cursor is only rewritten by create / edit / replace in goal-reducer.ts; reduceGoalTurnFinished never advances it, so the lineage grows monotonically for the life of a revision.
The runtime's own validator disagrees: goal-evidence.ts:181-186 gates on analysis.catalogTruncated only, and accepts the exact proposal the tool just refused.
At 17 turns: 34/34 entries present, nothing dropped, truncated=true, tool refuses, runtime validator accepts. Turns 18/20/21/22/30 — all refused. blocked proposals still record on the same transcript, so a long-running Goal can only ever terminate as blocked (or be rescued by a user edit/replace, which bumps the revision and resets the cursor — something the model cannot do).
The remediation text the model receives — "Continue in a new Goal turn with a smaller evidence set" — cannot be acted on: catalog size is a function of the transcript, not of what the model cites.
Suggested fix (small, and it makes the tool agree with the validator):
// goal-evidence.ts
return {
entries: analysis.catalog.map((entry) => ({ ...entry })),
lineageTurnIds: analysis.lineageTurnIds.slice(-CATALOG_LINEAGE_LIMIT),
truncated: analysis.catalogTruncated, // entries dropped
lineageTruncated: analysis.lineageTurnIds.length > CATALOG_LINEAGE_LIMIT, // list trimmed
};…leaving goal-tools.ts:190 gating on truncated alone.
3 · New finding 2 — the R2 fix closed the count axis but not the byte axis (blocker, contract-level)
The same "must cite everything / may only cite so much" collision I reported at R2 still exists, measured in bytes instead of entries:
goal-tools.ts:194-218demands that every current-turndelivered_outputbe cited.- Catalog admission counts a 240-char preview per entry (
CATALOG_PREVIEW_LIMIT), so a catalog stays untruncated. goal-evidence.ts:191-196caps the full content of cited records atVERIFIER_EVIDENCE_BYTE_LIMIT = 24_000.
A single Goal turn that delivered more than 24 KB of assistant output therefore has no compliant proposal: cite all → evidence_payload_too_large; cite fewer → uncitedCurrentDeliveredOutput.
End-to-end through the real createGoalRuntime: the tool accepts and reports readyForVerification: true, finishTurn runs, the evidence validator throws before the verifier LLM is ever called, the Goal stays active, and the next turn is handed the feedback "Cited Goal evidence exceeds the 24000-byte verifier limit." — which the model cannot satisfy. It retries, and after enough retries Finding 1 closes the door permanently.
The new immediate_blocker_newer_evidence_required rule added this round (goal-evidence.ts:436-444) has the same shape: an authority/external blocker must cite every newer catalog record, and pane C shows citing fewer is rejected too.
Worth noting: GOAL_VERIFIER_REQUEST_BYTE_LIMIT was raised 64 KB → 256 KB in a0581d20d, but VERIFIER_EVIDENCE_BYTE_LIMIT stayed at 24 KB — there is now headroom to raise it, or to bound what the tool-side guard demands (e.g. require only the newest current-turn outputs that fit the budget).
4 · Mutation matrix — 17 mutants against the PR's own suite
Baseline 284/284 green at 70f977ba4 (271 at R2; goal-tools.test.ts grew 21 → 30 tests). Each mutation was applied to production code only, reverted after the run, and attributed to a named failing test rather than a count.
9 killed — the round's substantive fixes are all genuinely pinned: the reference-cap change (M16), the reason byte cap (M8), both blocker-fingerprint fixes (M10, M11), the newer-evidence rule (M12), currentTurnId (M13), the verifier budget (M14), the truncation guard itself (M1).
8 survived — beyond the three carried-over ones (M3, M6, M7):
- M5 — removing the 16-turn lineage clause from
catalog.truncatedchanges nothing. The exact rule behind Finding 1 has no test at all; the only truncation test ingoal-tools.test.tshand-stubstruncated: true, and every test ingoal-evidence.test.tsdrives truncation through the entry/byte caps within a single turn. - M2 — making the truncated guard reject
blockedproposals too is not caught. That is the one escape hatch left by Finding 1, and nothing pins it. - M4 — dropping
status === 'complete'from the validator'scatalog_truncatedgate is not caught. - M9 — the reason character cap is dominated by the byte cap in every fixture.
- M15 — dropping the post-worker-view
throwIfAborted()inupdate_goalis not caught.
5 · Root cause, annotated
6 · Gates and regression control
| Gate | Result |
|---|---|
npx vitest run src/goals |
284 / 284 pass |
Full packages/core suite |
17 231 pass, 9 fail |
↳ same 9 on main control (0f56e35c0) |
identical failure set — 0 PR-attributable regressions |
npm run check-i18n |
✅ all checks passed |
npx prettier --check (all changed .ts/.tsx) |
clean |
npx eslint (15 changed TS files) |
clean — positive control (planted any + unused import) produced 2 errors, so the linter is live |
npx tsc --noEmit (packages/core) |
31 errors, none in src/goals; all in files this PR does not touch (ide-client.ts, sessionService*.ts, client.test.ts, client-mcp-registrar.test.ts) — artifacts of the symlinked node_modules in my worktree |
The 9 core failures (memoryLifecycle.integration, client-mcp-registrar, 7× agent-headless) reproduce byte-for-byte on the merged main commit, so they are pre-existing in my local environment and unrelated to this PR.
7 · Reachability and recommendation
Both new findings are contract-level, not user-reachable at this commit, which matches the PR's stated scope. I verified this rather than assuming it: nothing outside packages/core/src/goals/ writes goalContext, no non-test caller constructs createGoalRuntime with an evidenceSource, and neither tool is registered anywhere.
Recommendation: the slice is clean as an unwired contract — no regressions, gates green, and every fix this round is mutation-confirmed load-bearing. Finding 1 is a two-line change and I would rather see it fixed here than inherited by the slice that wires the transcript producer, because at that point a Goal that runs past 16 turns silently becomes uncompletable. Finding 2 needs a decision on which side gives (raise VERIFIER_EVIDENCE_BYTE_LIMIT, or bound what the tool-side guard demands) — that one is reasonable to defer to the verifier slice as long as it is tracked. The surviving mutants M2/M5 in particular deserve tests whichever way you resolve Finding 1.
中文版本
第 3 轮 — 在 70f977ba4 上的本地构建与真实执行验证
这是对我此前两轮（R1 @ bbbe160d4、R2 @ eebac98a8）的复验。R2 之后的增量：9597bf27e、a0581d20d、70f977ba4，外加一次 main 合并（110a600e7）。
以下全部结论都来自执行 PR 的代码，不是靠阅读得出的；每一条旧结论都重新测过，没有直接沿用。
1 · 旧结论在新 head 上的状态
| # | 结论（轮次） | 70f977ba4 状态 |
验证方式 |
|---|---|---|---|
| R1-1 | nextAction 无条件返回 |
✅ 已修（eebac98a8） |
仍按 receipt 分支，复验通过 |
| R1-2 | 去重测试形同虚设 | ✅ 已修（eebac98a8） |
仍能杀死对应变异体 |
| R1-3 | "同一次响应里再调 get_goal" 的不可能指令 |
✅ 已修（eebac98a8） |
.not.toContain 断言仍在 |
| R2-A | maxItems: 12 与无上限的 delivered-output 校验冲突 |
✅ 已修（9597bf27e） |
上限改为 GOAL_EVIDENCE_REFERENCE_LIMIT = CATALOG_ENTRY_LIMIT（100）。变异体 M16（改回 12）被新测试 admits thirteen delivered outputs plus independent evidence 杀死，说明修复是有效承载的 |
| R2-B | update_goal 在无 permit 时硬抛异常（goal-tools.ts:147-149） |
M7（整段替换为桩返回）→ 284/284 依然全绿 | |
| R2-C | status === 'paused' 的 returnDisplay 死分支（goal-tools.ts:255-258） |
M6（删除该分支）→ 存活。PR 自己的 rejects a proposal after pause invalidates its permit 证明 execute() 会先以 stale-permit 抛出 |
|
| R2-D | uncited 校验里的 status === 'complete' 未被测试固定 |
M3 → 存活 |
2 · 新结论 1 — 超过 16 轮的 Goal 永远无法提出 complete（阻断级，契约层）
buildGoalEvidenceCatalog（goal-evidence.ts:148-151）把两件互不相关的事合并成一个 truncated 标志：
analysis.catalogTruncated— 确实丢弃了证据条目；analysis.lineageTurnIds.length > CATALOG_LINEAGE_LIMIT（16）— 只是返回的 turn-id 列表被裁剪了。
goal-tools.ts:188-193 依据这个标志拒绝所有 complete 提案。于是从第 17 个 Goal 轮次开始，update_goal status:"complete" 在目录完全没有丢任何条目的情况下也被拒绝，而且永远不会恢复：每多一轮 lineage 只会更长。证据游标只在 create / edit / replace 时被重写（goal-reducer.ts），reduceGoalTurnFinished 从不推进它，所以同一 revision 内 lineage 单调增长。
运行时自己的校验器并不同意：goal-evidence.ts:181-186 只看 analysis.catalogTruncated，会接受工具刚刚拒绝的同一份提案。
第 17 轮实测：34/34 条目齐全、没有丢弃、truncated=true、工具拒绝、运行时校验器接受。18/20/21/22/30 轮全部被拒。同一份记录上 blocked 仍可记录，所以长跑的 Goal 只能以 blocked 收尾（或靠用户 edit/replace 抬升 revision 重置游标——模型自己做不到）。
模型收到的补救文案 "Continue in a new Goal turn with a smaller evidence set" 无法执行：目录大小取决于 transcript，而不是模型引用了什么。
建议修法（很小，且能让工具与校验器一致）：拆成 truncated（条目被丢）与 lineageTruncated（列表被裁）两个字段，goal-tools.ts:190 只看前者。
3 · 新结论 2 — R2 的修复补上了"数量"轴，但"字节"轴仍然敞着（阻断级，契约层）
我在 R2 报的"必须全引用 / 只能引用这么多"冲突依然存在，只是从条目数换成了字节数：
goal-tools.ts:194-218要求引用每一条当前轮次的delivered_output；- 目录准入只按每条 240 字符预览计算，所以目录不会被标记 truncated；
goal-evidence.ts:191-196对被引用记录的完整内容设了VERIFIER_EVIDENCE_BYTE_LIMIT = 24_000上限。
因此，只要某一个 Goal 轮次交付了超过 24 KB 的助手输出，就不存在合规提案：全引 → evidence_payload_too_large；少引 → uncitedCurrentDeliveredOutput。
端到端实测（真实 createGoalRuntime）：工具接受并返回 readyForVerification: true，finishTurn 执行后证据校验先抛异常、verifier 大模型根本没被调用，Goal 保持 active，下一轮拿到反馈 "Cited Goal evidence exceeds the 24000-byte verifier limit."——而模型无法满足它。它会不断重试，重试到一定次数后结论 1 会把门彻底关死。
本轮新增的 immediate_blocker_newer_evidence_required（goal-evidence.ts:436-444）是同一形状：authority/external 阻塞必须引用每一条更新的目录记录，而对照实验 C 显示少引也会被拒。
另外：a0581d20d 把 GOAL_VERIFIER_REQUEST_BYTE_LIMIT 从 64 KB 提到了 256 KB，但 VERIFIER_EVIDENCE_BYTE_LIMIT 仍是 24 KB——现在其实有余量可以调高，或者把工具侧的强制引用范围收敛（例如只要求预算内最新的那些当前轮输出）。
4 · 变异矩阵 — 17 个变异体对 PR 自带测试
基线在 70f977ba4 上 284/284 全绿（R2 是 271；goal-tools.test.ts 从 21 增至 30 个用例）。每个变异只改生产代码、跑完即还原，并且归因到具体失败用例名而不是失败数量。
9 个被杀 —— 本轮的实质性修复都被真正固定住了：引用上限（M16）、reason 字节上限（M8）、两处 blocker fingerprint 修复（M10、M11）、newer-evidence 规则（M12）、currentTurnId（M13）、verifier 预算（M14）、以及截断守卫本身（M1）。
8 个存活 —— 除三条旧结论（M3、M6、M7）外：
- M5 —— 把 16 轮 lineage 条件从
catalog.truncated里删掉，测试毫无反应。结论 1 背后的规则完全没有测试：goal-tools.test.ts里唯一的截断用例是手工写死truncated: true，而goal-evidence.test.ts的所有用例都在单一轮次内用条目/字节上限触发截断。 - M2 —— 让截断守卫连
blocked一起拒绝，没有测试发现。而这正是结论 1 留下的唯一逃生口。 - M4 —— 从校验器的
catalog_truncated判断里去掉status === 'complete'，没有测试发现。 - M9 —— reason 的字符上限在所有 fixture 里都被字节上限掩盖。
- M15 —— 去掉
update_goal中 worker view 之后的throwIfAborted()，没有测试发现。
5 · 门禁与回归对照
| 门禁 | 结果 |
|---|---|
npx vitest run src/goals |
284 / 284 通过 |
packages/core 全量 |
17 231 通过，9 失败 |
↳ main 对照（0f56e35c0）同样 9 条 |
失败集合完全一致 —— 无 PR 引入的回归 |
npm run check-i18n |
✅ 全部通过 |
npx prettier --check（全部改动 .ts/.tsx） |
干净 |
npx eslint（15 个改动 TS 文件） |
干净 —— 阳性对照（植入 any + 未使用 import）报出 2 个错误，说明 linter 确实在工作 |
npx tsc --noEmit（packages/core） |
31 个错误，src/goals 内为 0；全部落在本 PR 未触及的文件上，属于我 worktree 里 symlink node_modules 的环境产物 |
9 条 core 失败（memoryLifecycle.integration、client-mcp-registrar、7 × agent-headless）在合并进来的 main 提交上逐条复现，属于我本地环境的既有问题，与本 PR 无关。
6 · 可达性与建议
两条新结论都是契约层问题，在当前提交下用户不可达，这与 PR 自述的范围一致。这一点我做了验证而非假设：packages/core/src/goals/ 之外没有任何地方写 goalContext，没有非测试代码用 evidenceSource 构造 createGoalRuntime，两个工具也未在任何地方注册。
建议：作为一个尚未接线的契约切片，这个 PR 是干净的——无回归、门禁全绿、本轮每一处修复都经变异验证确实承载。结论 1 只是两行改动，我更希望在这里修掉，而不是留给接入 transcript producer 的那个切片——到那时，跑过 16 轮的 Goal 会悄无声息地变得永远无法完成。结论 2 需要决定让哪一边让步（调高 VERIFIER_EVIDENCE_BYTE_LIMIT，或收敛工具侧强制引用的范围），只要有跟踪，推迟到 verifier 切片是合理的。无论结论 1 怎么修，存活的 M2 / M5 都值得补上测试。
Verified in an isolated worktree at 70f977ba4, Node v22.23.1 / macOS. Probes and the mutation harness are reproducible on request.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
| const view = await workerViewForPermit(this.runtime, permit, signal); | ||
| signal.throwIfAborted(); | ||
| snapshotForPermit(this.runtime, permit); |
There was a problem hiding this comment.
[Suggestion] snapshotForPermit is called here but its return value is discarded — the constructed GoalSnapshotV2 (including a structuredClone of the goal record) is thrown away. workerViewForPermit on the line above already performs the staleness check via view.goalId/view.revision comparison. The same snapshotForPermit function is called again at line 237 (after recordTerminalProposalForPermit) where the result is captured and used for returnDisplay.
— Failure scenario: every update_goal invocation pays for a redundant snapshot materialization (getSnapshotForPermit → getSnapshot() → structuredClone(snapshot)). No test asserts this call's side effects or count.
| const view = await workerViewForPermit(this.runtime, permit, signal); | |
| signal.throwIfAborted(); | |
| snapshotForPermit(this.runtime, permit); | |
| const view = await workerViewForPermit(this.runtime, permit, signal); | |
| signal.throwIfAborted(); |
中文说明
[Suggestion] 此处调用了 snapshotForPermit 但丢弃了返回值——构建的 GoalSnapshotV2（包括对 goal 记录的 structuredClone）被直接丢弃。上一行的 workerViewForPermit 已经通过 view.goalId/view.revision 比较执行了过期检查。相同的 snapshotForPermit 函数在第 237 行（recordTerminalProposalForPermit 之后）再次被调用，该处的结果被捕获并用于 returnDisplay。
— 失败场景：每次 update_goal 调用都会为冗余的快照实例化付出代价（getSnapshotForPermit → getSnapshot() → structuredClone(snapshot)）。没有测试断言此调用的副作用或调用次数。
— qwen3.7-max via Qwen Code /review
|
已修复。验证证据：commit |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Review Summary
This PR introduces two Goal v3 worker tools (get_goal and update_goal) with comprehensive safety mechanisms. Overall, the implementation is well-designed with strong security posture.
✅ Strengths
Security & Correctness
- Fail-closed by design: Stale permits, disposed runtimes, and session swaps all result in stable
stale-permiterrors rather than silent corruption - Double validation: Worker view is validated against permit (
goalId,revision) after async reads, preventing TOCTOU issues - Session swap protection: Runtime is captured at invocation build time, not execute time
- Evidence isolation: Only catalog UUIDs are accepted, not
goalId,turnId, orlineageTurnIds - Defensive cloning:
structuredCloneused consistently for permits and views - Proper signal handling: AbortSignal cleanup with
removeEventListenerin finally block
Architecture
- Clean separation between read (
GetGoalTool) and propose (UpdateGoalTool) operations - Tools never expose lifecycle controls — proposals are recorded, not committed
- Current delivered output must be cited for completion proposals (prevents premature completion claims)
- Truncated catalog blocks completion proposals (exhaustiveness guarantee)
Test Coverage (984 lines)
- Comprehensive coverage of permit invalidation scenarios (edit, replace, clear, finish)
- Session swap attack tests
- Cancellation propagation tests
- Worker view mismatch validation
- Evidence reference validation edge cases
- Repeated blocker audit behavior
🔍 Minor Observations
1. Consider adding a comment to goal-turn-context.ts
// Uses Node AsyncLocalStorage to propagate the turn permit across
// async boundaries while preventing cross-turn contamination.
export const goalTurnContext = new AsyncLocalStorage<GoalTurnPermit>();The file is minimal and readers may benefit from understanding why AsyncLocalStorage is the right primitive here.
2. Consider documenting the rationale for constants
// 8000 characters / 16000 UTF-8 bytes allows detailed reasoning while
// keeping verifier requests bounded.
export const GOAL_PROPOSAL_REASON_MAX_CHARACTERS = 8_000;
export const GOAL_PROPOSAL_REASON_MAX_BYTES = 16_000;3. Verifier request limit increased (64_000 → 256_000)
This is noted in the diff. Ensure this aligns with downstream LLM token limits and budgets.
4. Evidence reference limit
GOAL_EVIDENCE_REFERENCE_LIMIT is set to CATALOG_ENTRY_LIMIT (100). This seems reasonable but worth verifying against the verifier's context window budget.
⚠️ Questions
-
Integration test gap: The PR notes that registration, scheduler propagation, and model-loop handling are deferred. Is there a tracking issue or subsequent PR that wires these tools into the actual agent loop?
-
terminateTurnsignal: WhenreadyForVerificationis true,terminateTurn: trueis returned. This is consumed by the scheduler in a later PR — confirming this is the intended handoff contract?
Verdict
Approve with minor suggestions. The implementation is solid, security-conscious, and well-tested. The deferred integration points are clearly documented in the PR description.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Review: Goal v3 Worker Tools
This is a well-architected PR that introduces the get_goal and update_goal worker tools with proper permit-based context isolation. Overall, the implementation is solid with excellent test coverage.
✅ Strengths
1. Permit Capture & Stale-Permit Protection
The use of AsyncLocalStorage (goalTurnContext) to capture the permit at tool invocation time is elegant:
const contextPermit = goalTurnContext.getStore();
const permit = contextPermit ? structuredClone(contextPermit) : undefined;This ensures delayed executions after edit/replace/clear/finish/disposal fail closed with a stable STALE_GOAL_TURN_MESSAGE error rather than operating on stale or swapped sessions. The tests in goal-tools.test.ts thoroughly cover these paths.
2. Session-Swap Safety
The runtime is captured at build() time, not at execute() time:
const runtime = permit ? this.config.getGoalRuntime() : undefined;Combined with the snapshot check, this prevents cross-session data leakage even if the session is swapped between build and execution.
3. Evidence Validation
The update_goal tool correctly rejects:
- Lineage turn IDs and other non-UUID references
- Completions that omit current delivered output
- Proposals when the catalog is truncated
- Oversized reason strings (character + byte limits)
4. Comprehensive Test Coverage
984 lines of tests for 439 lines of implementation (~2.2:1 ratio). Tests cover:
- Permit invalidation across all lifecycle actions (edit, replace, clear, finish)
- Session swap protection
- Evidence edge cases (truncated catalog, uncited output)
- Cancellation propagation
- Goal/workerview mismatch detection
5. Non-Terminal Proposal Contract
The tools correctly do not change Goal lifecycle directly—they only propose, leaving authority to the verifier/runtime. The terminateTurn signal is only set when readyForVerification is true.
🔍 Observations & Suggestions
1. Minor: Consider explicit type for GetGoalToolParams
export type GetGoalToolParams = Record<string, never>;This is correct but could use a comment explaining why no parameters are needed (the permit is captured from context).
2. Verifier Request Size Increase
The limit was raised from 64KB to 256KB:
const GOAL_VERIFIER_REQUEST_BYTE_LIMIT = 256_000;The test keeps maximum valid evidence and proposal reason within the request limit validates this works, but consider documenting the rationale for this increase in the code or PR description.
3. Evidence Catalog Truncation Handling
The truncatedCatalogResult() helper returns a clear error message. However, the check happens after validating evidence references:
if (this.params.status === 'complete' && view.evidenceCatalog?.truncated) {
return truncatedCatalogResult();
}This is correct behavior (validate references first), but could be documented more explicitly.
4. Legacy Compatibility in Verifier
The verifier maintains backward compatibility:
...(!input.currentTurnId && input.currentDeliveredOutput
? { currentDeliveredOutput: [...input.currentDeliveredOutput] }
: {}),This is good for migration. The test preserves the legacy delivered-output input contract covers this.
🛡️ Security Considerations
Passing Review - No security concerns identified:
-
No sensitive data leakage: The
projectWorkerViewfunction explicitly excludesfullTranscriptand only exposes bounded evidence catalog entries with previews (not full content). -
Proper error normalization:
throwNormalizedRuntimeErrorconverts disposal/stale errors to a stable message, preventing information leakage through error messages. -
Evidence UUID validation: Rejects
goalId,turnId, andlineageTurnIdsas evidence references, preventing confusion attacks. -
AbortSignal propagation: Cancellation is properly handled via
signal.throwIfAborted()and Promise.race patterns.
📋 Minor Nits
-
Typo in test description (non-blocking):
it('rejects completion that omits current delivered output', ...
Could be clearer as "rejects completion proposal that omits..."
-
Copyright year: The new files use
2026 Qwen Team. Verify this matches project convention (other files use 2025).
📊 Summary
| Aspect | Assessment |
|---|---|
| Correctness | ✅ Solid permit-based isolation, proper stale-handling |
| Security | ✅ No issues, proper error normalization |
| Test Coverage | ✅ Excellent (2.2:1 test:code ratio) |
| Code Quality | ✅ Clean, well-documented |
| Documentation | ✅ Good inline comments, clear PR description |
Recommendation: Approve and merge. The implementation follows good patterns, has thorough test coverage, and correctly implements the Goal v3 worker tool contract with proper lifecycle isolation.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.7-max via Qwen Code /review
Round 4 — local build & real-execution verification at
|
| catalog entries | lineage shown | truncated |
update_goal(complete) |
|
|---|---|---|---|---|
R3 70f977ba4 |
17 | 16 | true |
refused — proposalRecorded=false |
R4 6ef08931a |
17 | 16 | false |
recorded — ready=true, terminateTurn=true |
The new test keeps completion available after the lineage display window fills also pins it properly: mutating line 148 back to the old two-condition expression is killed (M1 below). In R3 the equivalent mutant survived — that rule had zero coverage. That is a real improvement, not just a patch.
2. R3 finding 2 — the byte-axis contradiction is narrowed ~10×, not closed (non-blocking)
VERIFIER_EVIDENCE_BYTE_LIMIT was raised to 256_000 to match GOAL_VERIFIER_REQUEST_BYTE_LIMIT. The two constants are now numerically equal, but they do not measure the same thing:
goal-evidence.ts:189sumsBuffer.byteLength(record.content)— raw content bytes of the cited records.goal-verifier.ts:120measuresBuffer.byteLength(JSON.stringify(payload))— the whole serialised request: goal envelope, objective, proposal reason,evidenceRefs, per-record wrappers, and JSON-escaped content.
The second is always the first plus a strictly positive envelope, so a band of payload sizes passes the tool gate and then fails the verifier gate. I binary-searched the actual boundary:
| cited evidence shape | max raw bytes that reach the verifier | unsatisfiable band |
|---|---|---|
| 1 record, plain ASCII (zero escaping) | 255,566 | 434 B (0.17%) |
| 20 records, plain ASCII | 252,753 | 3,247 B (1.27%) |
| 1 record, 10% newlines (ordinary model output) | 232,333 | 23,667 B (9.24%) |
The band is nonzero even with no escaping at all, so it is structural, not an artifact of my test data. Ordinary multi-line output widens it to ~9%.
End-to-end consequence at head, with 250,000 raw bytes of current-turn delivered output (275,002 bytes once serialised): the catalog admits the record (catalog admission counts the 240-char preview, not content), update_goal returns readyForVerification: true and terminateTurn: true, and then at the turn boundary verifierContents throws. Because GoalVerifierInputTooLargeError is not an InvalidGoalEvidenceReferenceError, runVerification (goal-runtime.ts:546-553) classifies it as usage_limited — so the Goal halts in usage_limited and the verifier LLM is never called.
Two honest qualifications on severity, which is why I am not calling this a blocker:
- It is much harder to reach than the 24 KB version. You now need ~232–256 KB of cited evidence in a single proposal. At 24 KB this was ordinary; at 232 KB it is not.
usage_limitedis resumable.reduceGoalResumeonly blockscompleteandactive, so the Goal is not a dead end — unlike the R3 shape, which sat in anactiveretry loop with unactionable feedback.
Suggested follow-up (not required for this PR): make the evidence gate strictly smaller than the request gate — e.g. VERIFIER_EVIDENCE_BYTE_LIMIT = 192_000 — or have the tool-side gate measure the serialised size the verifier will actually build, so the two gates cannot drift apart again.
3. Mutation matrix — 8 killed / 5 survived of 13 (baseline 286/286 green)
Every mutant I could write on this round's changed lines is caught. That is the main thing I wanted to establish, because R3's fix landed in an area with no coverage at all.
Killed: reverting the fix (M1), suppressing truncation (M2), dropping the lineage slice (M3), reverting the byte cap to 24k (M4), loosening it 10× (M5), deleting the byte check (M6), shrinking the lineage window to 4 (M7), and inverting the current-turn match in the uncited guard (M13).
The 5 survivors are all carried over from earlier rounds, none introduced here:
- M8 / M12 — the
complete-only restriction on the truncated-catalog gate is unpinned on both the evidence layer and the tool layer. I confirmed by execution that the escape hatch works today (with a truncated catalog,completeis refused andblockedis recorded and ready) — but nothing guards it, and it is the only terminal exit a long Goal has when the catalog truncates. - M9 (R2 carry) —
get_goalwithout a permit returns{"active":false};update_goalwithout a permit throws a rawError. Asymmetric and untested. Not user-reachable while the tools are unregistered. - M10 (R2 carry) — I re-tested rather than carrying the claim on faith: pausing a Goal while a permit is held invalidates the permit, so
update_goalthrowsGoal turn permit is no longer validbefore reaching thereturnDisplaychain. Thepausedbranch atgoal-tools.ts:255-257is confirmed unreachable. - M11 (R2 carry) — the
status === 'complete'check in the uncited-delivered-output guard is unpinned; if it regressed,blockedproposals would also be forced to cite current delivered output.
4. Local gates at 6ef08931a
packages/coresrc/goals— 286 / 286 passedpackages/corefull suite — 17,233 passed, 9 failed. I ran the same three files onmain(bc2a35760) without this PR and got the identical 9 failures, name-for-name (memoryLifecycle×1,client-mcp-registrar×1,agent-headless×7). Pre-existing, none insrc/goals.tsc --noEmit— 31 errors, 0 insrc/goals; all 31 sit in files this PR does not touch (environmental, my symlinkednode_modules).prettier --checkon all 18 changed files — passeslinton the 7 changed core files — pass. Positive control: injecting an unused import + an explicitanyproduces 2 errors, so the gate is actually live.npm run check-i18n— pass
Nit (no action needed)
verifierInput computes currentDeliveredOutput (goal-runtime.ts:344-350), but verifierContents only serialises it when currentTurnId is absent (goal-verifier.ts:111-113) — and the runtime always sets currentTurnId. I captured the real payload: via the runtime the field is never on the wire. That matches the system prompt (the current turn's output is identified by turnId === currentTurnId), so it is redundant-by-design rather than wrong. Worth knowing that goal-runtime.test.ts asserts on it via toHaveBeenCalledWith, i.e. one layer above the wire, so that assertion would not notice if the field mattered and went missing.
中文版本
第 4 轮 —— 在 6ef08931a 上的本地构建与真实执行验证
接续 R1 bbbe160d4、R2 eebac98a8 与 R3 70f977ba4。以下全部在隔离 worktree 中于当前 head 实际执行，环境为 macOS / Node 22。
结论：R3 的阻塞问题确实已修复，并且我用受控 A/B 实验验证，而不是只读 diff。 R3 的第二个问题范围收窄了约 10 倍，但在结构上并未真正关闭 —— 我不再认为它阻塞合并。没有发现新的阻塞问题。就我这边而言可以合并，剩下那一项适合作为后续跟进。
相对 R3 只有一个新提交 6ef08931a，改动 2 个文件：证据目录的 truncated 不再混入 lineage 窗口溢出，VERIFIER_EVIDENCE_BYTE_LIMIT 由 24_000 改为 256_000。
1. R3 阻塞问题 —— 超过 16 个 turn 的 Goal 永远无法提交完成：已修复 ✅
R3 的核心缺陷是 buildGoalEvidenceCatalog 用一个 truncated 标志承载了两件无关的事，导致从第 17 个 turn 起，即使目录完整无截断，每次 complete 提案都会被拒绝。现在这两者已正确拆开。
我在工具层做了端到端复测：用真实 GoalRuntime 跑满 17 个真实 Goal turn，再通过 goalTurnContext 调用真实的 GetGoalTool 与 UpdateGoalTool，而不是单独调用证据辅助函数。随后我在同一个 worktree、用同一个探针，只把 packages/core/src/goals/goal-evidence.ts 回退到 70f977ba4 再跑一遍，确保修复是唯一变量：
| 目录条目 | lineage 展示 | truncated |
update_goal(complete) |
|
|---|---|---|---|---|
R3 70f977ba4 |
17 | 16 | true |
被拒绝 —— proposalRecorded=false |
R4 6ef08931a |
17 | 16 | false |
已记录 —— ready=true、terminateTurn=true |
新增测试 keeps completion available after the lineage display window fills 也真正锁住了这个行为：把第 148 行改回旧的双条件表达式会被杀死（下文 M1）。而在 R3 中对应的 mutant 是存活的 —— 那条规则当时完全没有覆盖。这是实质性的改进，不只是打补丁。
2. R3 第二个问题 —— 字节维度的矛盾收窄约 10 倍，但未关闭（不阻塞）
VERIFIER_EVIDENCE_BYTE_LIMIT 提升到 256_000，与 GOAL_VERIFIER_REQUEST_BYTE_LIMIT 相同。两个常量数值虽然相等，但度量的并不是同一个东西：
goal-evidence.ts:189累加Buffer.byteLength(record.content)—— 被引用记录的原始内容字节。goal-verifier.ts:120度量Buffer.byteLength(JSON.stringify(payload))—— 整个序列化请求：goal 外层结构、objective、提案 reason、evidenceRefs、每条记录的包装字段，以及 JSON 转义后的内容。
后者恒等于前者加上一个严格为正的外层开销，因此必然存在一段区间：能通过工具侧闸门，却过不了 verifier 闸门。我用二分法测出了真实边界：
| 引用证据形态 | 能真正到达 verifier 的最大原始字节 | 不可满足区间 |
|---|---|---|
| 1 条记录，纯 ASCII（零转义） | 255,566 | 434 B（0.17%） |
| 20 条记录，纯 ASCII | 252,753 | 3,247 B（1.27%） |
| 1 条记录，10% 换行（普通模型输出） | 232,333 | 23,667 B（9.24%） |
即使完全没有转义，该区间依然非零，说明这是结构性的，而不是我构造数据造成的假象。普通的多行输出会把区间拉宽到约 9%。
端到端后果：当前 turn 交付输出为 250,000 原始字节（序列化后 275,002 字节）时，目录会收下这条记录（目录准入统计的是 240 字符预览，不是内容），update_goal 返回 readyForVerification: true 与 terminateTurn: true，随后在 turn 边界 verifierContents 抛错。由于 GoalVerifierInputTooLargeError 不是 InvalidGoalEvidenceReferenceError，runVerification（goal-runtime.ts:546-553）会把它归类为 usage_limited —— 于是 Goal 停在 usage_limited，verifier LLM 从未被调用。
关于严重度，有两点必须如实说明，这也是我不把它列为阻塞的原因：
- 比 24 KB 版本难触发得多。 现在需要单次提案引用约 232–256 KB 证据。24 KB 时这很常见，232 KB 则不然。
usage_limited可恢复。reduceGoalResume只拦截complete与active，所以 Goal 不是死局 —— 不同于 R3 那种停在active、反馈又不可执行的重试循环。
后续建议（本 PR 不必处理）：让证据闸门严格小于请求闸门，例如 VERIFIER_EVIDENCE_BYTE_LIMIT = 192_000；或者让工具侧闸门直接度量 verifier 实际会构造的序列化大小，从根本上避免两个闸门再次漂移。
3. 变异测试矩阵 —— 13 个 mutant 中杀死 8 个 / 存活 5 个（基线 286/286 通过）
针对本轮改动行我能写出的 mutant 全部被捕获。这正是我最想确认的一点，因为 R3 的修复恰好落在一个完全没有覆盖的区域。
被杀死：回退修复（M1）、屏蔽截断（M2）、去掉 lineage slice（M3）、字节上限回退到 24k（M4）、放宽 10 倍（M5）、删除字节检查（M6）、lineage 窗口缩到 4（M7）、以及反转未引用守卫中的当前 turn 匹配（M13）。
存活的 5 个全部是此前几轮遗留，本轮没有引入新的：
- M8 / M12 —— 截断目录闸门中"仅限
complete"这一限制，在证据层和工具层都没有被锁住。我用执行确认了当前逃生通道是有效的（目录截断时complete被拒、blocked被记录且 ready），但没有任何测试守护它，而这是长 Goal 在目录截断后唯一的终态出口。 - M9（R2 遗留）—— 无 permit 时
get_goal返回{"active":false}，update_goal却直接抛原始Error。不对称且无测试。工具尚未注册，用户不可达。 - M10（R2 遗留）—— 我重新执行验证而非沿用旧结论：持有 permit 时暂停 Goal 会使 permit 失效，
update_goal会在到达returnDisplay分支之前抛出Goal turn permit is no longer valid。goal-tools.ts:255-257的paused分支确认不可达。 - M11（R2 遗留）—— 未引用交付输出守卫中的
status === 'complete'判断没有被锁住；一旦回归，blocked提案也会被强制要求引用当前交付输出。
4. 6ef08931a 上的本地门禁
packages/coresrc/goals—— 286 / 286 通过packages/core全量套件 —— 17,233 通过、9 失败。我在main（bc2a35760）上不带本 PR 跑了同样三个文件，得到逐条同名的相同 9 个失败（memoryLifecycle×1、client-mcp-registrar×1、agent-headless×7）。属于既有失败，且都不在src/goals。tsc --noEmit—— 31 个错误，src/goals中 0 个；31 个全部落在本 PR 未触及的文件（环境性，源于我 symlink 的node_modules）。prettier --check覆盖全部 18 个改动文件 —— 通过eslint覆盖 7 个改动的 core 文件 —— 通过。正对照：注入一个未使用 import 加一个显式any会报 2 个错误，说明这个门禁确实生效。npm run check-i18n—— 通过
小问题（无需处理）
verifierInput 会计算 currentDeliveredOutput（goal-runtime.ts:344-350），但 verifierContents 仅在 currentTurnId 缺失时才序列化它（goal-verifier.ts:111-113），而 runtime 总会设置 currentTurnId。我抓取了真实 payload：经由 runtime，该字段从不出现在实际请求中。这与系统提示一致（当前 turn 的输出通过 turnId === currentTurnId 识别），所以属于设计上的冗余而非错误。值得留意的是 goal-runtime.test.ts 通过 toHaveBeenCalledWith 对它做了断言，即断言在实际请求的上一层，因此即便该字段真的重要并且丢失了，这个断言也不会发现。
|
Released in v0.21.1. |
|
|
6 participants
What this PR does
Adds the two Goal v3 worker tools and the exact-turn context they require.
The read tool exposes only the current Goal snapshot, bounded evidence catalog, and verifier feedback for the captured permitted turn. The update tool records a non-terminal completion or blocker proposal, validates evidence references against the latest bounded catalog, and asks the scheduler to end the turn only when the proposal is ready for independent verification.
Both tools capture the session runtime and permit when the invocation is built. Delayed execution after edit, replace, clear, finish, disposal, or session swap fails closed with a stable stale-permit error instead of reading or mutating another Goal.
Why it's needed
Goal v3 separates model proposals from lifecycle authority. The model needs a narrow way to read authoritative Goal state and propose terminal outcomes, while the session runtime and verifier remain solely responsible for committing completion or blocked status.
This is the fourth focused Goal v3 slice. It defines the tools and context only; registration, scheduler propagation, model-loop handling, and client integration remain in later PRs.
Reviewer Test Plan
How to verify
Build the read tool outside a permitted Goal turn and confirm it reports no active Goal without reading the session runtime.
Build both tools inside a permitted turn, then delay execution until after edit, replace, clear, finish, disposal, or a session-runtime swap. Confirm every stale invocation fails closed and never accesses the replacement session.
Read a bounded evidence catalog, submit a proposal using a catalog UUID, and confirm the Goal remains active while the proposal receipt reports whether verification is ready. Submit an unknown UUID, a lineage turn ID, duplicate references, or a missing current delivered-output reference and confirm the proposal is not recorded.
Confirm a ready proposal returns the Goal-specific turn-termination signal and instructs the model to emit no additional user-facing completion text. Confirm an audit-only repeated blocker does not terminate the turn.
Evidence (Before & After)
N/A — this PR defines worker-tool contracts and has no user-visible UI wiring.
Tested on
Environment (optional)
Node.js 22 workspace, no sandbox.
Risk & Scope
Linked Issues
N/A
中文说明
本 PR 做了什么
新增两个 Goal v3 worker 工具及其所需的精确轮次上下文。
读取工具只向捕获到 permit 的当前轮次暴露 Goal 快照、有界证据目录和 verifier 反馈。更新工具记录非终态的完成或阻塞提案，使用最新有界目录校验证据引用，并且只在提案已可进入独立验证时要求 scheduler 结束本轮。
两个工具都在构建 invocation 时捕获 session runtime 和 permit。若执行被延迟到 edit、replace、clear、finish、dispose 或 session 切换之后，会用稳定的 stale-permit 错误失败关闭，不会读取或修改另一个 Goal。
为什么需要
Goal v3 将模型提案与生命周期权限分离。模型需要一个很窄的接口来读取权威 Goal 状态并提出终态结果，而完成或阻塞状态仍只能由 session runtime 和 verifier 提交。
这是 Goal v3 的第四个聚焦切片，只定义工具和上下文；注册、scheduler 传播、模型循环处理和客户端集成都留给后续 PR。
Reviewer 测试计划
如何验证
在没有 permitted Goal turn 时构建读取工具，确认它返回没有 active Goal，且完全不读取 session runtime。
在 permitted turn 中构建两个工具，然后把执行延迟到 edit、replace、clear、finish、dispose 或 session runtime 切换之后。确认所有过期 invocation 都失败关闭，且不会访问替换后的 session。
读取有界证据目录，使用目录 UUID 提交提案，确认 Goal 仍保持 active，同时 receipt 只报告是否已可验证。提交未知 UUID、lineage turn ID、重复引用或遗漏当前 delivered output 的引用，确认提案不会被记录。
确认可验证提案返回 Goal 专属的结束轮次信号，并要求模型不再输出额外的用户可见完成文案；仅用于 blocker audit 的提案不会结束本轮。
证据（前后对比）
不适用——本 PR 只定义 worker 工具契约，没有用户可见 UI 接线。
测试环境
环境（可选）
Node.js 22 工作区，无 sandbox。
风险与范围
关联问题
不适用