Skip to content

fix(goal): stamp tool results in every host, so Goals can prove anything - #10175

Merged
qqqys merged 3 commits into
QwenLM:mainfrom
qqqys:goal/stamp-tool-results
Aug 27, 2026
Merged

fix(goal): stamp tool results in every host, so Goals can prove anything#10175
qqqys merged 3 commits into
QwenLM:mainfrom
qqqys:goal/stamp-tool-results

Conversation

@qqqys

@qqqys qqqys commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Records tool results with the Goal turn permit in the headless and ACP hosts, so their results can become evidence. The rule is extracted into one exported helper, goalToolResultProvenance, and the interactive scheduler — which already got this right inline — is switched over to it rather than left as a fourth copy: no permit means no stamp; get_goal and update_goal are stamped goal_runtime so the Goal's own bookkeeping stays out of the catalog it would otherwise cite as proof of itself; every other tool result becomes citable external evidence.

The two hosts read the permit differently because they execute tools differently. Headless already had it — ToolCallRequestInfo.goalContext, filled in by Turn — and simply ignored it at the recording call. ACP runs tools straight from the model's function calls and has no request object, so it reads the ambient goalTurnContext that the session enters around the whole turn (and explicitly exits for non-Goal prompts, which is why a plain prompt still records nothing).

Outside a Goal turn both call sites keep their former two-argument shape, so ordinary recording is byte-for-byte unchanged.

Why it's needed

The evidence catalog derives a record's provenance from the permit stamped on it — goal-evidence.ts: this.provenance = this.parsedGoalContext ? coherentEvidenceProvenance(record) : undefined, and a record with no provenance produces no catalog entry. The string goalContext did not appear anywhere in nonInteractiveCli.ts or Session.ts.

So in those two hosts — precisely the ones an unattended long-running Goal actually uses — the catalog could only ever hold delivered_output (assistant turns, which are stamped) and user_input. tool_result is the only provenance that maps to external_fact, so no Goal running there could prove anything about the world:

All three were unreachable. The only terminal state such a Goal could reach on its own was the token-budget stop (#9891). Reported as #10172.

Reviewer Test Plan

How to verify

  • cd packages/core && npx vitest run src/goals/ src/core/coreToolScheduler.test.ts — 839 pass, including 8 new cases pinning the shared rule (ordinary tool stamped, permit copied not aliased, get_goal/update_goal marked goal_runtime, nothing stamped outside a turn, and the same four through the ambient reader).
  • cd packages/cli && npx vitest run src/acp-integration src/nonInteractiveCli.test.ts — 1913 pass. Each host gains a test that drives a real Goal turn through one ordinary tool and one get_goal, asserting the recorded options per call id, plus a test that a non-Goal turn records with no options at all.
  • Mutation probes run during development: headless stamp removed → exactly the headless test fails (131 others green); ACP stamp removed → exactly the ACP test fails; the shared rule collapsed to always-goal_runtime → exactly the bookkeeping assertion fails.
  • tsc --noEmit clean for the changed code in both packages; prettier + eslint clean.

A host-level assertion is required and the core-side tests cannot substitute: they build evidence windows from hand-written records that already carry the stamp, which is exactly why the whole suite stayed green while two of three hosts produced no evidence at all.

Evidence (Before & After)

The same headless run on the same working directory — a Chinese objective over eight documents, with GOAL_DEFAULT_TOKEN_BUDGET temporarily shrunk to 400,000 to bound the cost (the constant is unchanged in this diff):

before after
tool_result records carrying the stamp 0 / 31 20 / 20
Goal turns 5 1
verifier rejections 8 0
terminal state usage_limited (budget spent) complete
tokens 542,051 228,302

Before, the objective was actually finished — 结论.md was written with all eight sections and the model confirmed it with run_shell_command — and the Goal still could not prove it. The rejections say so directly: "the cited evidence has proofKind 'delivered_output', which proves only that the assistant said so", and finally "An immediate blocker requires cited user input or external tool evidence", closing the escape hatch too.

After, the verifier accepts on the first turn and its reason cites the evidence uuids the earlier run could never produce: "Evidence confirms all eight module documents were read (ce8a0e37–371a7949), the working directory was verified…".

Tested on

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

Environment (optional)

Unit tests plus two live headless runs against DashScope, before and after, described above.

Risk & Scope

Linked Issues

中文说明

这个 PR 做了什么

在 headless 与 ACP 两个 host 中,记录工具结果时带上 Goal 轮次 permit,使其结果能够成为证据。该规则被抽取为唯一的导出函数 goalToolResultProvenance,并且把本来就写对了的交互式调度器也切换过去,而不是留下第四份拷贝:没有 permit 就不盖戳;get_goalupdate_goalgoal_runtime,使 Goal 自身的账本不进入它本可能引以为证的目录(否则就是循环论证);其余工具结果成为可引用的外部证据。

两个 host 取 permit 的方式不同,因为它们执行工具的方式不同。headless 本来就持有它——ToolCallRequestInfo.goalContext,由 Turn 填充——只是在记录调用处忽略了。ACP 直接从模型的 function call 执行工具,没有请求对象,因此读取 session 在整个轮次外围进入的 goalTurnContext(非 Goal 的 prompt 会被显式 exit,所以普通 prompt 依然不记录任何戳)。

在 Goal 轮次之外,两个调用点保持原本的两参数形态,普通记录逐字节不变。

为什么需要

证据目录以记录上的 permit 戳派生 provenance(goal-evidence.ts),没有 provenance 就不产生目录条目。而 goalContext 这个字符串在 nonInteractiveCli.tsSession.ts 中根本不存在。

于是在这两个 host——恰恰是无人值守长跑 Goal 真正使用的两个——目录只可能包含 delivered_outputuser_inputtool_result 是唯一映射到 external_fact 的 provenance,因此那里的 Goal 无法证明关于世界的任何事:独立验证器会正确拒绝仅以模型自述为证的完成;#10125infeasible 要求引用 external_fact;立即型 external 阻塞要求外部证据。三条路径全部不可达,唯一能自主到达的终局只剩 #9891 的预算停止。已记录为 #10172

评审验证计划

如何验证

  • cd packages/core && npx vitest run src/goals/ src/core/coreToolScheduler.test.ts——839 通过,含 8 个新用例固定共享规则(普通工具盖戳、permit 是复制而非别名、get_goal/update_goal 标记为 goal_runtime、轮次外不盖戳,以及经由 ambient 读取器的同样四条)。
  • cd packages/cli && npx vitest run src/acp-integration src/nonInteractiveCli.test.ts——1913 通过。每个 host 新增一个测试:驱动真实 Goal 轮次执行一次普通工具与一次 get_goal,按 call id 断言记录选项;另有一个测试断言非 Goal 轮次不带任何选项。
  • 开发期间的变异检验:去掉 headless 盖戳 → 恰好挂 headless 测试(其余 131 绿);去掉 ACP 盖戳 → 恰好挂 ACP 测试;把共享规则塌缩为恒 goal_runtime → 恰好挂账本断言。
  • 两个包中改动代码的 tsc --noEmit 干净;prettier + eslint 干净。

必须在 host 层断言,core 侧测试无法替代:它们使用的是本身就带戳的手写记录——这正是三个 host 里有两个完全不产生证据、而全量测试却始终全绿的原因。

证据(前后对比)

同一次 headless 运行、同一工作目录——中文 objective、八份文档,为控制成本将 GOAL_DEFAULT_TOKEN_BUDGET 临时改为 400,000(该常量在本 diff 中未改动):

修复前 修复后
带戳的 tool_result 记录 0 / 31 20 / 20
Goal 轮数 5 1
验证拒绝次数 8 0
终局状态 usage_limited(预算耗尽) complete
token 542,051 228,302

修复前,目标其实已经完成——结论.md 八节齐全,模型用 run_shell_command 复核过——但 Goal 无法证明这一点。拒绝理由直说了:「所引用证据的 proofKind 是 'delivered_output',只能证明助手这样说过」,最后是「立即型阻塞需要引用用户输入或外部工具证据」,连逃生通道也一并关闭。

修复后,验证器在第一轮即接受,理由引用的正是此前根本无法产生的证据 uuid:「Evidence confirms all eight module documents were read (ce8a0e37–371a7949), the working directory was verified…」。

已测试平台

Linux ✅;macOS / Windows ⚠️(CI 覆盖)。单元测试之外,另有针对 DashScope 的两次真实 headless 运行(前后对比,见上)。

风险与范围

关联 Issue

The evidence catalog derives a record's provenance from the Goal permit
stamped on it -- no parsed goal context, no provenance, no catalog
entry. The interactive scheduler stamps tool results; the headless and
ACP hosts never did. Neither file contained the string `goalContext`.

So in those two hosts -- the ones an unattended long-running Goal
actually uses -- the catalog could only ever hold `delivered_output`
and `user_input`. `tool_result` is the only provenance that maps to
`external_fact`, which means no Goal there could prove anything about
the world: an independent verifier correctly refuses a completion
proved only by the model's own words, and the `infeasible` and
`external` blockers both require external evidence. The only terminal
state such a Goal could reach on its own was the token-budget stop.

The rule now lives in one place, `goalToolResultProvenance`, and the
interactive scheduler is switched over to it too rather than keeping a
fourth copy: no permit means no stamp; `get_goal` and `update_goal` are
stamped `goal_runtime` so the Goal's own bookkeeping stays out of the
catalog it would otherwise cite as proof; everything else becomes
citable external evidence.

The two hosts read the permit differently because they execute tools
differently. Headless already had it -- `ToolCallRequestInfo.goalContext`,
filled in by `Turn` -- and simply ignored it at the recording call. ACP
runs tools straight from the model's function calls, so it reads the
ambient `goalTurnContext` the session enters around the whole turn.

Outside a Goal turn both call sites keep their former two-argument
shape, so ordinary recording is unchanged.

Measured on the same headless run, before and after (budget shrunk to
400k for the test): 0/31 tool results stamped, 5 turns, 8 verifier
rejections, ending in `usage_limited` with the objective actually done
but unprovable -> 20/20 stamped, one turn, `verifier_accept`,
`complete`, at 42% of the tokens. The accepting verifier cites the
evidence uuids the earlier run could never produce.

Mutation probes: headless stamp removed -> 1 fail; ACP stamp removed ->
1; the shared rule collapsed to always-`goal_runtime` -> 1.
@qqqys

qqqys commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

Comment thread packages/cli/src/acp-integration/session/Session.ts
Comment thread packages/core/src/core/coreToolScheduler.ts
Comment thread packages/core/src/goals/goal-tool-result-provenance.ts Outdated
Comment thread packages/cli/src/nonInteractiveCli.ts
Comment thread packages/cli/src/acp-integration/session/Session.ts
@qqqys

qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover stop

… every site

Three findings from the first review round, each with a probe that
now pins it:

- ACP read the ambient Goal permit with `goalTurnContext.getStore()`,
  but a background notification fires from async resources created
  inside the turn that spawned the task, so the store is inherited into
  it long after that turn ended. `#executePrompt` already leaves the
  store for every non-Goal turn; the notification and cron drains now do
  the same, so an automatic turn's tool results cannot be stamped as
  evidence for a Goal turn that never made those calls.
- The scheduler call passed a third argument of `undefined` outside a
  Goal turn, where the removed code passed two. `recordToolResult` did
  not care, but the arity-sensitive executor test did. It now keeps the
  two-argument shape outside a turn, matching the two host sites.
- The TUI still carried two inline copies of the rule this change
  extracted, so the claim that every site routed through one helper was
  false at the commit that made it. Both now call the helper, and the
  existing TUI recording test becomes a mutation check on it.

Both new host tests also cover a tool that fails inside the Goal turn:
a failed command is exactly the external fact an `infeasible`
completion cites, so the stamp must not depend on success.

Mutation probes, each killing exactly one test: drop the notification
exit wrapper; stamp only on success in headless; stamp only on success
in ACP; collapse the shared rule to always-`goal_runtime` (the TUI test
now goes red where it stayed green before); scheduler back to an
unconditional third argument.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

eea2367acb 处理了本轮全部 5 条(线程内已逐条回复并 resolve):

  • ACP 血缘泄漏(Critical):通知轮和 cron 轮的执行都包进 goalTurnContext.exit(...),与 #executePrompt 对非 Goal 轮次的处理一致。新增回归测试从 goalTurnContext.run(permit, …) 内部入队通知并断言其工具结果记录只有两个参数;去掉 exit 包装即挂。
  • scheduler 参数个数(Critical):改为与两个 host 相同的条件展开,nonInteractiveToolExecutor.test.ts 回绿。
  • TUI 两处内联拷贝:都改走 goalToolResultProvenance(request),helper 注释改为列出真正覆盖的五处;把 helper 塌缩为恒 goal_runtime 现在会让 TUI 测试变红。
  • 两个 host 的失败工具路径:各自的 Goal 轮次测试都加了一个返回 error 的工具调用,断言 status: 'error' 且盖戳不变。

验证:core 849、CLI 2147 全绿;五组变异检验各精确杀一个;prettier/eslint 干净。6 文件 +163/−34。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed. Suggestions are inline.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent 6b": I could not execute the unit tests ( npx vitest run src/goals/goal-tool-result-provenance.test.ts ) — vitest is not installed/resolvable in this worktree ( Ca….

中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

未探索到全部深度(达到工具调用预算):"agent 6b"I could not execute the unit tests ( npx vitest run src/goals/goal-tool-result-provenance.test.ts ) — vitest is not installed/resolvable in this worktree ( Ca…

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

Comment thread packages/cli/src/acp-integration/session/Session.ts
Comment thread packages/cli/src/nonInteractiveCli.test.ts
Comment thread packages/cli/src/acp-integration/session/Session.test.ts Outdated
…not the arity

The cron drain got the same goalTurnContext.exit guard as the
notification drain in the previous commit, but only the notification
guard had a test: removing the cron wrapper left all 717 Session tests
green. It now has the mirror test -- fire the cron job from inside a
Goal turn's store, let the cron turn run a tool, assert the result
records unstamped -- and removing the wrapper turns it red.

The notification test asserted the recording call had exactly two
arguments, where its headless sibling asserts the third is undefined.
The third parameter is a plain optional read via `options?.`, so the
two forms are observationally identical today but the arity form would
fail a behaviour-preserving refactor to a direct optional argument (the
shape the TUI sites already use). Both now assert the semantic form.

The headless Goal-turn test carried a dead first mockImplementation,
fully replaced by the second before the run; deleted.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

c0281be620 处理了第二轮全部 3 条(线程内已逐条回复并 resolve),都是测试卫生:

  • cron 守卫补回归测试:镜像通知轮的血缘测试——从 goalTurnContext.run(permit, …) 内部触发 cron,断言其工具结果不带戳。评审指出的不对称已关闭:去掉 cron 的 exit 包装现在恰好挂这一个测试(此前 717 全绿)。
  • ACP 断言改为语义形式:两个血缘测试都用 expect(call[2]).toBeUndefined(),与 headless 兄弟测试一致,行为保持的调用形态重构不会误报。
  • 删除 headless 测试中的死 mock 块。

验证:CLI 2148 全绿;prettier/eslint 干净。2 文件 +82/−18。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues. LGTM! ✅

Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/nonInteractiveCli.test.ts:2915 — [probe] 3-arg runNonInteractive calls mis-bind the prompt into the settings slot (hidden by the TODO(5691) typecheck exclude)
中文说明

无阻断问题。LGTM!✅

收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

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

@qqqys
qqqys added this pull request to the merge queue Aug 27, 2026
Merged via the queue into QwenLM:main with commit c6220b6 Aug 27, 2026
145 of 147 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(goal): headless and ACP never stamp tool results, so external_fact evidence is unreachable

4 participants