feat(telemetry): Track tool execution outcomes - #8180
Conversation
github-actions
Bot
added
the
review/self-reported
E2E / behavioral verification reportBaseline
Automated verification
Behavioral results
Out of scopeSpeculation, direct |
|
@qwen-code /resolve |
|
Qwen Code did not run conflict resolution for this request. PR #8180 is draft. |
|
@qwen-code /resolve |
1 similar comment
|
@qwen-code /resolve |
|
Qwen Code did not run conflict resolution for this request. PR #8180 is draft. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
60542f1 to
78b294d
Compare
|
Qwen Code did not run conflict resolution for this request. PR #8180 does not currently have merge conflicts with main. |
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
🔄 Qwen Triage is running — watch live progress. Stage results will post in this thread as they complete. 🔄 Qwen Triage 正在运行 —— 查看实时进度。各阶段结果完成后会更新在本线程。 |
|
Re-run triggered by @wenshao — gate re-check on the current head
Moving on to code review. 🔍 中文说明由 @wenshao 触发的重跑——在当前 head
进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code review at
|
| File | What changed |
|---|---|
docs/design/2026-07-31-tool-execution-status.md |
New design doc - valid terminal/execution pairs, normalization rules, dashboard cutover guidance |
docs/developers/development/telemetry.md |
Documents the new event fields, counter, and span attributes next to the existing schema |
packages/cli/src/acp-integration/session/Session.ts |
ACP half of the contract - explicit status at every terminal path, abort checks after each await, fault-tolerant emits |
packages/cli/src/acp-integration/session/Session.test.ts |
Race-shaped tests for the ACP paths (+1,576 lines) |
packages/cli/src/acp-integration/session/SubAgentTracker.ts + test |
Emit failures swallowed to debug log; abort-driven cancels no longer fire onPermissionCancel |
packages/cli/src/nonInteractiveCli.ts + test |
Synthetic pre-execution responses marked not_started |
packages/cli/src/ui/hooks/useGeminiStream.ts + test |
Passes executionStatus through result recording |
packages/cli/src/ui/hooks/useReactToolScheduler.ts + useToolScheduler.test.ts |
mapToDisplay handles cancelled calls whose tool/invocation never resolved |
packages/core/src/core/coreToolScheduler.ts |
Core half of the contract - outcome frozen at settle, PostToolBatch span deferral, queue drain hardening |
packages/core/src/core/coreToolScheduler.test.ts |
Outcome matrix and cancellation-race coverage (+1,721 lines) |
packages/core/src/core/turn.ts + tests |
ToolExecutionStatus type; optional executionStatus on ToolCallResponseInfo (back-compat) |
packages/core/src/hooks/types.ts |
Doc comment for the PostToolBatch serialized response fields |
packages/core/src/telemetry/constants.ts + index.ts |
Failure-kind vocabulary centralized; new counter exported |
packages/core/src/telemetry/loggers.ts + test |
normalizeToolCallEvent exported (unknown when absent, error fields deleted on non-errors); per-sink isolation via runToolTelemetrySink |
packages/core/src/telemetry/metrics.ts + test |
New low-cardinality qwen-code.tool.execution.count counter (execution_status + tool_type only) |
packages/core/src/telemetry/qwen-logger/qwen-logger.ts + test |
Gains call_id and execution_status; deliberately no MCP server name or args |
packages/core/src/telemetry/session-tracing.ts + test |
Execution spans gain identity (tool name, call id) and status; span start is fail-safe to NOOP |
packages/core/src/telemetry/log-to-span-processor.ts + test |
Cancelled tool-call events stay UNSET even when legacy error fields exist |
packages/core/src/telemetry/types.ts |
ToolCallEvent gains call_id and optional execution_status |
packages/core/src/tools/mcp-tool.ts + test |
Parent-abort race fix; request-timeout classification to EXECUTION_TIMEOUT |
packages/core/src/tools/tool-error.ts + priorReadEnforcement.ts |
StructuredToolError relocated, cycle broken (issue #2) |
The 36th file, packages/vscode-ide-companion/scripts/generate-notices.js, is Prettier-forced formatting churn (see findings above).
Test evidence — the PR's own CI (this run executed no PR code)
Per triage rules this is an unattended run: no PR build, test, or script was executed here. The evidence is the PR's own CI on the reviewed commit, fetched via API, plus the named live-behavior signals below.
All checks on afd349ca are complete — nothing pending, nothing failing:
| Check | Conclusion |
|---|---|
| Test (ubuntu-latest, Node 22.x) | ✅ success |
| Desktop Shell (ubuntu-22.04) | ✅ success |
| web-shell E2E Smoke (ubuntu-latest, Node 22.x) | ✅ success |
| Real daemon E2E / Java 11 | ✅ success |
| SDK Java matrix (ubuntu 11/17/21, macOS 21, windows 21) | ✅ success |
| review-pr | ✅ success |
| Integration Tests (CLI, No Sandbox) | ⚪ skipped |
| Test (macos-latest / windows-latest, Node 22.x) | ⚪ skipped |
The Integration Tests (CLI, No Sandbox) skip is the one check a human should be aware of — the automated /review flagged the same: that suite did not run in CI and its assertions are covered by unit tests only. All other gates are green.
Live-behavior signal (named, per 2b-bis): the central claim here is behavioural — the outcome contract holding under cancellation races — and a green suite alone does not pin it. Two stronger signals exist, and I'm naming both rather than papering over the gap:
- @wenshao's real-environment verification (Aug 2, comment above): full outcome matrix against a real CLI, real OTLP exporter, real ACP agent, and a real TUI in tmux, with a before/after baseline on
main. That is maintainer evidence on head83e685da4— clearly attributed, not this bot's own run. - The delta between that head and this one (
afd349ca) is six commits of review-feedback fixes and test hardening. A sandboxed@qwen-code /verifyrun dispatched by this triage trigger is in flight on the current head (watch it live); its report will be posted to this thread when it completes. The code under verification comes from a fork, so the report should be read with the same skepticism as the fork's own CI logs.
No user-visible TUI or stream-JSON surface changes (by design — verified as protocol-clean in the maintainer's run), so there is no tmux before/after to drive even on a local run.
中文说明
代码审查（基于 afd349ca）
这是第 11 轮 autofix、多次维护者评审之后的重跑，审查重点是当前 head 是否真正落地了它所声称的内容。我通读了全部生产代码 diff，未发现阻塞性问题。
@wenshao 8 月 2 日评审中的两个必改项在此 head 上可验证地已修复：
- 取消消息不一致（问题 pre-release: fix ci #1）——
coreToolScheduler.tscatch 块中两处取消消息现在都以executionThrew标志为条件：abort 下执行抛错时报告'User cancelled tool execution.'（与 ACP 一致），只有真正执行完成后的路径才保留 "The tool had already completed; its output was discarded."。diff 中已核实，包括失败 hook 之后的二次检查。 - 模块循环（问题 Where is the config saved? #2）——
StructuredToolError移入tool-error.ts；priorReadEnforcement.ts为兼容保留 re-export；mcp-tool.ts直接从tool-error.js导入。依赖方向现在是单向的。已核实。
三个被拒绝的条目（#3 guarded-await 辅助函数、#4 会话销毁时的 span 收尾、#5 warn 级 sink 日志）在第 11 轮有带理由的书面说明，设计文档本身也将其列为后续项。这些属于维护者裁量项，不是门槛项。我独立核实了其中一个事实性声明——#6 generate-notices.js 的格式化噪音成立：main 上两行约 88 字符的 collectDependencies(...) 超过 80 字符宽度，任何触及该文件的提交都会被项目的 Prettier 工具链重新格式化。纯格式化噪音，仅此而已。
交叉核对： 自动化 /review（第 2 轮，v0.21.3）在这个 head 上报告 零发现。我自己对 coreToolScheduler.ts、Session.ts、mcp-tool.ts 和遥测模块的审查结论一致——有两处值得点名表扬：withPostToolBatchStop 只在被替换的调用本身带有 executionStatus 时才把它带入替换响应（批次停止不会凭空捏造工具从未产生的结果）；MCP 的 createParentAbortRace 修复是超越遥测的真实 bug 修复（SDK 的 abort 拒绝与真实超时共用 -32001 码，先在转发前冻结父结果，EXECUTION_TIMEOUT 分类才可信）。
序列图展示了整个契约所依赖的 settle/freeze/cancel 流程（英文见上）：调度器在 execute() 边界打开 execution span 并标记 executing；执行 settle（成功、工具错误、抛异常）时立即冻结执行结果；随后运行 PostToolUse/PostToolUseFailure hook；若父 abort 在 settle 之后到达，取消获胜——终态为 cancelled，但冻结的执行结果保留；最后每个实际尝试恰好产出一条归一化事件和一个 execution span。
变更文件概览表见上（21 组覆盖全部 36 个文件）。
测试证据——PR 自己的 CI（本次运行未执行任何 PR 代码）
按 triage 规则，这是无人值守运行：未构建、未测试、未执行任何 PR 脚本。证据为通过 API 获取的被审 commit 的 CI 结果，加上下文点名的实时行为信号。
afd349ca 上所有 check 均已完成——无 pending、无失败（表格见上）。Integration Tests (CLI, No Sandbox) 的跳过是唯一需要人工留意的 check——自动化 /review 也指出了同一点：该套件未在 CI 运行，其断言仅由单测覆盖。其余所有门禁均为绿色。
实时行为信号（按 2b-bis 规则点名）： 本 PR 的核心主张是行为性的——取消竞态下状态契约依然成立——仅靠绿色套件无法锁定。此处点名两个更强的信号，而不是掩盖缺口：
- @wenshao 的真实环境验证（8 月 2 日，见上方评论）：真实 CLI、真实 OTLP 导出器、真实 ACP agent、tmux 真实 TUI 上的完整结果矩阵，并与
main做了前后对照。那是维护者在 head83e685da4上的证据——明确标注来源，不是本机器人的运行结果。 - 该 head 与当前 head（
afd349ca）之间的差异是六个 commit 的评审反馈修复与测试加固。由本次 triage 触发的沙箱@qwen-code /verify运行正在当前 head 上进行（链接见上），完成后报告会发布到本线程。被验证代码来自 fork，阅读报告时应与对待 fork 自身 CI 日志保持同样的审慎。
本 PR 无用户可见的 TUI 或 stream-JSON 面变更（设计如此——维护者验证已确认协议无泄漏），因此即使在本地运行中也无需 tmux 前后对照。
— Qwen Code · qwen3.8-max-preview
Reviewed at afd349ca705415bf40c213787848b00ecf7ae0c4 · re-run with @qwen-code /triage
|
Confidence: 3/5 — clean re-review on Stepping back over the whole arc of this PR:
⏸️ Deferring to @wenshao. The gate's remaining question is pure policy, not doubt: this is a fork PR touching core infrastructure at ~3,011 production lines, which the two-tier rule escalates for maintainer awareness — and awareness here has been thorough (12 review rounds plus a full real-environment verification on Aug 2). If the in-flight sandboxed verify report lands clean on this head and you are satisfied the embedder-facing semantic changes above are consciously accepted, this is ready for a human approval. One housekeeping note: the standing 中文说明置信度：3/5 —— 在 退后一步看这个 PR 的完整轨迹：
⏸️ 移交 @wenshao。 门槛剩下的问题是纯策略性的，而非疑虑：这是一个触及核心基础设施约 3,011 行生产代码的 fork PR，两级规则将其升级为维护者关注——而此处的关注已经非常充分（12 轮评审加 8 月 2 日的完整真实环境验证）。如果进行中的沙箱验证报告在当前 head 上干净落地，且你确认上述嵌入方语义变更是被有意接受的，这个 PR 就可以由人类批准了。一个内务提示：本 PR 上挂着的 — Qwen Code · qwen3.8-max-preview Reviewed at |
Review —
|
| test | expected executionStatus |
|---|---|
should execute a tool successfully (call1) |
'success' |
should return an error if tool is not found (call2) |
'not_started' |
should return an error if tool validation fails (call3) |
'not_started' |
should return an error if tool execution fails (call4) |
'error' |
should return an unhandled exception error if execution throws (call5) |
'error' |
should correctly format llmContent with inlineData (call6) |
'success' |
Those values are all correct per the design doc, so this is stale expectations only — no deeper defect. Worth noting that the PR body's verification claim ("821 focused Core tests passing") is what let this through: the focused-test approach skipped the one non-focused file the change touches. Please run the full suite before the next push.
🟠 Important
I1 — Scope: this is three or four PRs in one trench coat.
Beyond the execution-status axis, the diff also lands:
- a substantial rewrite of MCP cancellation/timeout semantics in
packages/core/src/tools/mcp-tool.ts—createParentAbortRace, aparentAbortControllerreplacing the raw parent signal insideAbortSignal.any([...]),isMcpRequestTimeoutmapping-32001toStructuredToolError(EXECUTION_TIMEOUT), and an earlythrowinhandleReconnectOnErrorwhen the signal is aborted; - ~40 new
cancelPreExecutionIfAborted/cancelBeforeExecutionIfAbortedcheckpoints acrosscoreToolScheduler.tsandSession.tsthat reclassify many pre-execution terminals fromerrortocancelled; SubAgentTracker.tschanges with no telemetry content at all —.catch()on the emitters plus!abortSignal.abortedgating ononPermissionCancel?.();executeSingleToolCallno longer re-throwing — thethrow error;that used to follow thesetStatusInternalatcoreToolScheduler.ts:4183-4195is gone. Real behavior change: a prelude throw in one tool no longer aborts the remaining batches inattemptExecutionOfScheduledCalls, andschedule()no longer rejects for it. This looks like a genuine improvement, but it is invisible in the PR description;drainRequestQueueIfIdle()added to_schedule'sfinally— this fixes a real stall (a_schedulethat produces zero tool calls never reachescheckAndNotifyCompletion, so the queue was previously never drained), also undocumented;- blanket try/catch hardening in
loggers.ts,session-tracing.ts, andnotifyToolCallsUpdate.
Each of these is individually defensible. Together they make a 5,695/1,067-line diff whose blast radius spans Core + CLI + ACP + MCP cancellation, and which cannot be reverted piecemeal if a regression surfaces. At minimum I'd split out the MCP cancellation rewrite and the SubAgentTracker changes.
I2 — deriveSpanStatus cancellation short-circuit is global, not tool-scoped.
packages/core/src/telemetry/log-to-span-processor.ts:461:
if (attrs['status'] === 'cancelled') {
return { code: SpanStatusCode.UNSET };
}This runs before the error / error.message checks and applies to every log record converted to a span, not just tool_call. Several other events carry a status that can be 'cancelled' (telemetry/types.ts:975, :1004, :1456). Any of those emitted with an error attribute now silently lose their ERROR status. If the intent is tool-scoped — and the new test name (keeps cancelled tool calls UNSET) says it is — gate on attrs['event.name'] === EVENT_TOOL_CALL.
I3 — the deferred PostToolBatch span and the call whose response gets replaced can be different calls.
coreToolScheduler.ts:2517 picks the span to defer as the last validating call:
const postToolBatchParentCallId = newToolCalls.findLast(
(toolCall) => toolCall.status === 'validating',
)?.request.callId;but withPostToolBatchStop replaces completedCalls[length - 1] — the last call in request order, which can be a pre-validation error/cancel that never got a span (spans are only opened in the status === 'validating' loop at :2553). When those two differ, the setToolSpanFailure(stoppedSpan, TOOL_FAILURE_KIND_POST_HOOK_STOPPED, …) at :5744 no-ops, so the batch-stop failure_kind is lost on exactly the call it describes, while a different span is held open for nothing. Keying both off the same call (e.g. defer the last completed call's span) would close the gap.
I4 — cancellation now discards already-produced successful output; the PR body says the opposite.
cancelAfterPostProcessing in core (defined at coreToolScheduler.ts:4807, applied at :4902, :5208, :5369, :5490) and the post-processing re-check in Session.runTool (Session.ts:8702-8713) convert a call whose execute() already returned successfully into terminal cancelled, replacing responseParts with Tool execution was cancelled. when the abort lands during post-processing (PostToolUse hook, large-output persistence, vision bridge). The tool's side effects — file writes, shell mutations — remain; the model just never sees what happened.
That is a defensible reading of Ctrl+C and matches the existing during-execution behavior, but "Evidence (Before & After): N/A — this change does not alter user-visible TUI or JSON output" doesn't cover it. Please call it out under Risk & Scope.
🟡 Suggestions
- S1 —
packages/cli/src/utils/nonInteractiveHelpers.ts:351(buildResponse) still constructs aToolCallResponseInfowithoutexecutionStatus, so subagent-derived responses normalize tounknown. That contradicts the design doc's "Built-in Core and ACP producers always set the field". Either populate it or add subagent projections to the doc's explicit exclusions (they're in the PR's out-of-scope list but not the doc's). - S2 — ACP
Session.runToolpasses bare string literals ('tool_cancelled','tool_timeout','tool_error','tool_exception') toendToolExecutionSpan({ error })while core uses theTOOL_SPAN_STATUS_*constants. Export and reuse them so the two producers of the same span can't drift. - S3 —
notifyToolCallsUpdatenow swallows observer exceptions intodebugLogger.warn. That downgrades a whole class of TUI bug from loud to silent;debugLogger.errorwould at least keep it findable. - S4 —
executionStatusnow reaches session-recording JSONL (recordToolResults→sanitizeToolCallResultForRecordingis a pass-through). Additive and harmless, but worth a line in the doc's Compatibility section since replay readers will see a new field. - S5 — The doc states "Only
success/errorandsuccess/cancelledare invalid combinations", but nothing enforces it. A dev-mode assertion (or a normalization step at the telemetry boundary) would keep the contract from rotting as new terminal paths get added. - S6 — Branch is 5 commits behind
origin/main(merge base0d3c864). Rebase before merge.
✅ What holds up
- Test coverage is substantive, not decorative. 780 targeted core tests pass locally on the head commit —
coreToolScheduler346,mcp-tool75,session-tracing150,loggers73,metrics48,log-to-span-processor52,turn36. The new race tests genuinely drive the abort-in-flight paths (resolve-enablement-after-abort, resolve-permission-deny-after-abort) rather than mirroring the implementation. - Freezing at settle is the right design.
executionStatusis captured immediately beforeinvocation.execute()(pessimistically'error') and only narrowed on settle, so setup failures staynot_startedand a synchronous throw insideexecute()is still correctly attributed. TheexecSpan-inside-the-callback placement makes the span boundary match that contract exactly, and I confirmed there's noawaitbetween the abort guard and the transition, so the widenedscheduledwindow can't be re-entered. - Metric hygiene.
qwen-code.tool.execution.countcarries onlyexecution_status+tool_type, andtool.call.countis untouched — feat(core): Normalize tool-call terminal telemetry #8176's schema survives. - Telemetry can no longer break a tool call.
runToolTelemetrySinkaround each fan-out sink plus try/catch instartToolSpan/startToolExecutionSpanis the right fix, andnormalizeToolCallEventdeletingerror/error_typeon non-error terminals is strictly cleaner than the previousundefined-valued keys. CancelledToolCall.tool/invocationoptionality is handled at the one runtime-sensitive consumer (useReactToolScheduler.mapToDisplay), and TypeScript covers the rest.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] CI is red: Test (ubuntu-latest, Node 22.x) fails with 6 test failures in packages/core/src/core/nonInteractiveToolExecutor.test.ts. The PR adds executionStatus to every CoreToolScheduler response, but the 6 toStrictEqual expectations in that file (lines 104, 189, 227, 271, 308, 350) were not updated. Reproduced locally at HEAD 842dce5: Tests 6 failed | 3 passed. The fix is mechanical — add executionStatus: 'success' or 'not_started' (per the design doc's terminal/execution pairs) to each expectation. This was first reported by @wenshao in an earlier review and remains unfixed at the current commit.
— qwen3.8-max-preview via Qwen Code /review
| execution_status: 'error', | ||
| }), | ||
| }), | ||
| expect.objectContaining({ |
There was a problem hiding this comment.
[Suggestion] The gamma assertion in this PostToolBatch payload test omits execution_status, leaving the "error-in-result-body" path unverified for the new field. — Concrete cost: gamma’s execute resolves (not rejects) with { error: { message: ‘gamma failed’ } }, which the scheduler normalizes to executionStatus = ‘error’ via the toolResult.error ? ‘error’ : ‘success’ branch (coreToolScheduler.ts ~line 4743). This is a distinct code path from beta’s rejection-based error. If a future refactor incorrectly sets execution_status for the resolved-with-error path, this test would still pass because gamma’s expect.objectContaining never checks the field.
| execution_status: 'error', | |
| }), | |
| }), | |
| expect.objectContaining({ | |
| tool_response: expect.objectContaining({ | |
| error: ‘gamma failed’, | |
| error_type: ToolErrorType.UNKNOWN, | |
| execution_status: ‘error’, | |
| }), |
中文说明
[Suggestion] 此 PostToolBatch 载荷测试中的 gamma 断言缺少 execution_status，导致“结果体中包含 error”的路径未被新字段验证。
— qwen3.8-max-preview via Qwen Code /review
| resultDisplay: undefined, | ||
| error, | ||
| errorType, | ||
| errorType: errorType ?? ToolErrorType.EXECUTION_DENIED, |
There was a problem hiding this comment.
[Suggestion] The EXECUTION_DENIED default is applied to loop-detection skips, which are not approval-mode denials. — Concrete cost: recordSkippedToolCall is called without an explicit errorType at three loop-detection call sites (lines 6608, 6819, 7124). Before this diff those sites recorded errorType: undefined (mapped to UNKNOWN by telemetry). After this diff they record EXECUTION_DENIED, inflating the denial metric.
| errorType: errorType ?? ToolErrorType.EXECUTION_DENIED, | |
| errorType: errorType ?? ToolErrorType.UNKNOWN, |
中文说明
[Suggestion] EXECUTION_DENIED 默认值被应用于循环检测跳过，而这些并非审批模式拒绝。任何按 error_type === ‘execution_denied’ 过滤的遥测面板现在也会将循环检测跳过计入，导致拒绝指标失真。
— qwen3.8-max-preview via Qwen Code /review
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round: no action neededAll actionable feedback sections for this round are empty:
The only feedback item newer than the last evaluation is a non-Critical PR comment by No code changes were made and no commit was created. The PR head stays at its current commit. 中文说明Autofix 评审轮次：无需处理本轮所有可执行的反馈区域均为空：
上次评估之后唯一的反馈条目是 本轮未做任何代码改动，也未创建任何提交。PR 分支停留在当前提交上。 Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Review:
|
…ants afd349c gated the cancel message on executionThrew but left the two wordings as bare literals at four sites and added no test. That is the exact shape the bug had: it was introduced by editing one literal and missing the others. Extract TOOL_CANCELLED_{BEFORE,AFTER}_COMPLETION_MESSAGE so the four sites cannot drift, and add regression tests for both paths — a tool interrupted mid-flight (execute() rejected under abort) must report "User cancelled tool execution.", while a cancel after execute() returned must report that the output was discarded. The mid-flight test fails against the pre-afd349ca behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Maintainer verification — real stack, base vs. headI built this PR locally and exercised it end to end against the real CLI binary, not unit mocks:
1. Terminal status vs. execution status — the core claim
The middle column is the point. On base, rows 02–06 are all Side checks on the same runs:
2. ACP path
The last row is the one worth calling out to reviewers: this is a behaviour fix, not just a new field. On base, a user cancelling the ACP permission prompt was recorded as 3. Cancellation, from a real TUINot simulated: real
4. QwenLogger payloadQwenLogger only ships on a >60s flush interval, so I ran a long TUI session and intercepted the actual
5. Compatibility
6. Focused test suites (local, PR head)
7. What I could not verify, and one observation
VerdictEvery user-visible claim in the description that I could reach from outside the process reproduced, on both the Core scheduler and the ACP paths, with a real before/after. The execution axis is correctly 中文版本Maintainer 验证 —— 真实栈，base 对比 head我在本地构建了这个 PR，并针对真实 CLI 产物（而不是单测 mock）做了端到端验证：完整跑
1. 终态与执行状态的分离 —— 核心诉求见上文第一张图。重点在中间那一列：在 base 上，02–06 行全部是 同一批运行里的旁证：
2. ACP 路径
第二张图的最后一行值得 reviewer 特别注意：这是一个行为修复，不只是新增字段。 在 base 上，用户在 ACP 权限弹窗中取消，会被记录成 3. 真实 TUI 下的取消不是模拟：真实 结果是 4. QwenLogger 载荷QwenLogger 只在超过 60 秒的 flush 间隔后才上报，所以我跑了一个较长的 TUI 会话，并在进程内拦截了真实的
5. 兼容性
6. 定向测试（本地，PR head）
7. 未能验证的部分，以及一个观察
结论描述中所有我能从进程外部观测到的用户可见声明都复现了，Core 调度器和 ACP 两条路径都验证过，并且有真实的变更前后对比。执行轴在我能构造的四种执行前拒绝路径上都正确为 |
|
@qwen-code /triage |
Classifying every `-32001` as EXECUTION_TIMEOUT skips handleReconnectOnError, which previously recovered one real case: the transport dies mid-request, the SDK request times out because no response will ever arrive, and the server is already recorded DISCONNECTED. That reconnected and retried; now it hard-fails and the user has to retry by hand. Divert back to the reconnect path only on positive evidence the transport is dead. Note that getMCPServerStatus() reports DISCONNECTED for servers it has never seen, so the guard checks for a *recorded* DISCONNECTED — the naive comparison misroutes every timeout from a server whose status was never registered, which broke four existing timeout tests when tried. A timeout on a healthy server is still EXECUTION_TIMEOUT: retrying it after a reconnect would just double the wait. The client-side idle timeout keeps classifying unconditionally; it is our own timer, not a transport signal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 223 passed · 0 failed · 223 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：223 通过 · 0 失败 · 223 总计 Verification reportPR #8180 Deep Verification — feat(telemetry): Track tool execution outcomes (follow-up round 2)Verdict: 中文摘要
Previous-finding status (follow-up round)Both head and base advanced again since the previous round (head
Also re-confirmed: the embedder-facing " ScopeCentral claim (re-measure): Delta claims (new since the previous round, this round's focus):
Secondary claims (re-measure): telemetry normalization (call_id + execution_status on every tool_call event, error-field absence on success/cancel, preserved classification, low-cardinality execution counter) and execution child spans (axis attributes, correct OTel status codes, attempted-executions-only). A/B lifecycle matrix (central claim)Harness
105/105 scripted assertions (incl. 3 module-provenance guards and 2 MCP delta probe (commit
|
| cell | transport state | head outcome | base outcome |
|---|---|---|---|
| dead_transport_recover | recorded DISCONNECTED, -32001, recovered tool available |
success / success / discover=1 / recovered output delivered | success / ABSENT / discover=1 / recovered output delivered |
| healthy_timeout | recorded CONNECTED, -32001, no recovery |
error / error / execution_timeout / discover=0 | error / ABSENT / unhandled_exception / discover=0 |
| unregistered_timeout | never registered, -32001, no recovery |
error / error / execution_timeout / discover=0 | error / ABSENT / unhandled_exception / discover=1 |
| parent_abort_race | CONNECTED; abort mid-flight; SDK rejects -32001 10 ms later |
cancelled / cancelled / before_completion / discover=0 | cancelled / ABSENT / before_completion / discover=0 |
53/53 scripted assertions. Readings:
- "Keep MCP reconnect" proven end-to-end: on a dead transport both arms reconnect (1 discovery) and the retry delivers the recovered tool's output as a
success— head preserves base's recovery path byte-for-byte in observable outcome, adding the execution axis on top. - Classification change: healthy-transport timeouts become a structured
EXECUTION_TIMEOUT(head) instead of an unclassified rethrow (base); no reconnect is attempted for them on either arm. - Abort race: with the SDK rejecting
-32001right after the parent abort, head finalizescancelled— withoutcreateParentAbortRacethe-32001could win and the user cancel would misclassify asexecution_timeout(head'saborted = signal.aborted && !executionTimedOutwould flip to an error terminal). Base also landscancelledhere via its ownsignal.abortedarbitration. - Unregistered-server asymmetry (head discover=0 vs base discover=1) — see Findings Where is the config saved? #2.
Wire-oracle telemetry (secondary claims)
Harness wire-telemetry.mjs initializes the real OTel SDK through the production telemetry.outfile seam (FileSpanExporter/FileLogExporter/FileMetricExporter — nothing stubbed downstream of the OTel API), runs 4 real tool calls (success / throw / validation / cancel_executing) through the compiled scheduler with getUsageStatisticsEnabled: true (HOME redirected to a scratch dir to keep the QwenLogger side-effect-free), flushes via shutdownTelemetry(), and run-wire.mjs parses the OTLP file (pretty-printed record stream split by brace depth). 65/65 assertions:
- Logs: 4/4
tool_callrecords carry non-emptycall_id+ the expectedexecution_status(success/error/not_started/cancelled); success and cancelled events haveerror,error_type,error.message,error.typekey-absent; throw carrieserror.type=unhandled_exception+ the message; validation carrieserror.type=invalid_tool_params. - Spans: exactly 3
qwen-code.tool.executionspans (the not_started validation cell produces none, and also no parentqwen-code.toolspan — 3 parents, not 4); every head execution span carriesexecution_status+tool.call_id; OTel status codes success→OK(1), throw→ERROR(2), cancelled→UNSET(0); parent spans carry noexecution_statusaxis (theirtool.call_idis pre-existing correlation). - Metrics:
qwen-code.tool.execution.countpresent; every dataPoint's dimensions are exactly{execution_status, tool_type}(low-cardinality confirmed —getCommonAttributescontributes nothing without asession.idopt-in); all four statuses observed.qwen-code.tool.call.count(feat(core): Normalize tool-call terminal telemetry #8176 terminal counter) preserved with itsstatusdimension. - Base control: 4 tool_call logs with no
call_id/execution_status; 3 execution spans exist pre-PR but lack both axis attributes; no execution counter; terminal counter intact.
Mutation matrix (vacuity checks)
Source-level single-point mutants run under vitest (which executes source, not dist); each mutant reverted via git checkout afterward and the tree verified clean (git status --porcelain empty). Witness: 04-mutation-matrix.png; raw logs logs/gate-*.txt.
| mutant | change | suite result | classification |
|---|---|---|---|
| clean baseline | – | coreToolScheduler.test.ts 354/354, loggers.test.ts 75/75 | controls green |
| M1 | settle-success branch 'success' → 'not_started' |
KILLED — 11 failed | 343 passed (354), failures expected 'not_started' to be 'success' |
central axis pinned; positive control that the suite is live |
| M-D2 | executionThrew cancel-message gate removed at both catch sites (≡ reverting the previous round's unpinned behavior) |
KILLED — exactly 1 red: tells the model a mid-flight cancellation never completed fails on expected ... to contain 'User cancelled tool execution.' (received the wrong "already completed" notice) |
delta commit 6c07b28's new test pins the gate — previous finding #1 is fixed, and the kill fails the intended message assertion, not an import/compile artifact |
| M-D2R | reverse: both ternaries → unconditional before-completion message | SURVIVED — 354/354 green | coverage gap — see Findings #1 |
| M-unknown | normalizeToolCallEvent fallback ?? 'unknown' → ?? 'success' |
KILLED — 6 red in loggers.test.ts | normalization fallback pinned, including the delta-added unknown-normalization test |
Corrections
1. Commit afd349ca's "Revert unrelated Prettier reformat in generate-notices.js" message (carried from the previous round, re-measured at this head): the aggregate diff still carries the 16-line reformat of packages/vscode-ide-companion/scripts/generate-notices.js. Measured fresh: the HEAD^1 version of the file fails npx prettier --check when checked inside the base worktree (repo .prettierrc.json applies — exit 1, style issues reported), while the head version passes (exit 0). The residual diff is therefore a legitimate Prettier normalization; reverting it as the message implies would fail the formatting gate. Functionally a no-op (dev-only notices script); a description/commit-message clarification, not a code-change request. (Methodology note: checking the base file from outside the repo tree passes prettier spuriously — config lookup matters.)
2. Description clarification carried from the previous round (re-measured, still accurate): execution-span existence-gating predates the PR — base already creates qwen-code.tool.execution spans for attempted executions only (3 spans in the fresh base wire run). The PR's span contribution is the execution-status axis plus the formalized contract, not the span's existence.
Findings
1. Coverage gap (Suggestion): the cancel-message gate's false branch at the two catch sites is pinned by no test (mutant M-D2R survives).
The delta commit 6c07b28 pinned the gate's true branch: M-D2 (gate → unconditional AFTER message) is killed by tells the model a mid-flight cancellation never completed. The reverse mutant M-D2R (gate → unconditional BEFORE message, 'User cancelled tool execution.' at both catch sites, lines ~5582 and ~5666) leaves all 354/354 tests green. The false branch (executionThrew === false inside the catch) is the "post-processing threw after execute() settled, then a cancellation finalizes" shape — documented in the code's own comment ("throws raised after execute() already settled — e.g. by a post-processing transform") and reachable: the try block continues past executionSettled = true with fallible work (maybePersistLargeToolResult, truncateLlmContent, persisted-output merging). Classification: ordinary coverage gap, not dead code — the behavior at head is correct (A/B's cancel_post_settle/cancel_hook_window cells show the AFTER message is chosen on the settle path); the risk is a future silent flip of this branch. Not a merge condition. A fixture that would pin it: a scheduler test where execute() resolves successfully, a post-processing step throws, and the turn signal is aborted — asserting the response carries "The tool had already completed; its output was discarded." (The two settle-path sites at lines ~4790/4846 are unconditional AFTER by construction and are covered by the A/B cells, so they are not part of this gap.)
Reproduce: node tmp/pr8180-verify-20260803-041658/mutate.mjs md2r && cd packages/core && npx vitest run src/core/coreToolScheduler.test.ts → 354 passed; node tmp/pr8180-verify-20260803-041658/mutate.mjs revert.
2. Informational: unregistered-MCP-server timeouts no longer attempt reconnect (measured base/head asymmetry).
In the MCP probe's unregistered_timeout cell (server status never recorded), base attempts a reconnect (discoverToolsForServer called once — base's shouldAttemptReconnect sees getMCPServerStatus()'s DISCONNECTED-by-default for unknown servers) and then fails with unhandled_exception; head classifies the same -32001 as EXECUTION_TIMEOUT and does not attempt reconnect (discover=0). This matches the code's documented intent ("Default to 'timeout' and only divert on positive evidence the transport is dead") and the PR's stated timeout-classification goal, but it is a behavioral narrowing versus base that the PR body does not call out (the body names the permission-transport and MCP-error reclassifications, not this one). Dashboards watching MCP reconnect rates for never-registered servers will see the change. No action required.
3. Informational (carried): cancel_hook_window axis asymmetry. Re-measured in the A/B matrix — see the previous-finding status table, row 2.
Not covered
- ACP side (
Session.ts, the PR's largest surface) exercised only by the CLI gate —Session.test.ts+SubAgentTracker.test.tswithin the 866-test CLI run (mock-based); not driven through a mock-free A/B or the real ACP stdio protocol. Core/ACP "same contract" proven on the Core half only; the ACP half statically checked to mirror the contract (own message constants, frozen executionStatus). - Cancellation-during-failure-hooks and approved-plan redaction (Reviewer Test Plan step 3): gate coverage only (in the 354-green suite, including the plan-redaction test).
- Live MCP server / real stdio transport: the probe reproduces the shape of the reconnect path — a spy registry + a fake direct client encoding the SDK's
-32001semantics — not a live transport; the SDK's abort-rejection behavior is taken from the PR's own comment as the fake's contract. - OTLP proto/grpc serialization: the wire oracle used the file-exporter seam — identical upstream of the exporter; protocol-specific wire bytes not exercised.
- QwenLogger MCP-metadata privacy: QwenLogger was instantiated (gated on the same
getUsageStatisticsEnabledthe tool_call path needs) with HOME redirected to a scratch dir; its event content/upload path was not asserted end-to-end this round. - Per-commit attribution: depth-2 shallow checkout —
git rev-list HEAD^1..HEAD^2returns the boundary artifact1while the snapshot lists 28 commits; per-commit verification unreachable, aggregateHEAD^1..HEADverified. (SnapshotbaseRefOid41f0e3cais newer than the checkout's base tip2ae8cd96— main advanced after the merge ref was built; the A/B uses the checkout'sHEAD^1.) - Repo-wide lint not run (CI-covered); repo-wide typecheck run here (exit 0).
- Live model-backed TUI session (author-stated out of scope); PostToolBatch replacement-span attribution follow-up (author-stated).
Methodology
Lane container (node:22-bookworm, Node v22.23.2, npm 10.9.8). Head tree pre-built by CI (npm ci + npm run build at merge commit 03128a879); dist freshness verified against both delta commits (constant extraction and isExecutionTimeoutFailure present in dist; dist built 04:11, before any harness mutation touched sources at ~05:00). Base packages/core rebuilt in a scratch worktree at HEAD^1 (2ae8cd96) via the package's own build script (npm run build → tsc --build, exit 0) after symlinking the root node_modules and head's nested packages/core/node_modules (8 packages — ajv, diff, fdir, ignore, json-schema-traverse, mime, picomatch, undici — without which the base build fails d.ts resolution). Clean control: the PR leaves package.json/package-lock.json untouched (0-line diff), packages/core has zero internal @qwen-code/* dependencies (packages/core/node_modules/@qwen-code does not exist), and per-run provenance was asserted by the harnesses (base loads …/tmp/base-tree/packages/core/dist/…); base dist sanity-checked to lack executionStatus/isExecutionTimeoutFailure (0 occurrences vs 34/3 on head). Worktree removed after capture (git worktree list shows only the main tree); rebuilding it is git worktree add tmp/base-tree <baseRefOid> + the two symlinks + npm run build in tmp/base-tree/packages/core. Harnesses (ab-matrix.mjs, mcp-probe.mjs, wire-telemetry.mjs, evaluators run-ab.mjs, run-mcp.mjs, run-wire.mjs, mutate.mjs, summarizers mutation-matrix.mjs, gates-summary.mjs) live in the artifact dir; head-arm runs stay rerunnable (base-arm reruns need the worktree rebuilt). The only seams are config (approvalMode, telemetry.outfile, QWEN_CODE_TOOL_EXECUTION_TIMEOUT_MS, HOME redirect), the registry spies, and the fake MCP client — dependencies, not the unit under test. Mutations were applied by mutate.mjs (occurrence-count-guarded string substitution) and reverted via git checkout --; after every mutant the tree was verified byte-clean. Gates ran via per-package npx vitest run <files> (logs gate-*.txt). Raw per-cell output in logs/; evidence images in evidence/.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
…QwenLM#8180) Two blocking items from the maintainer review: 1. Post-processing cancellations dropped persistedOutputFiles (and visionBridgeNotice) along with the model-visible output, orphaning files the tool had already spilled to disk. createCancelledResponse now carries both, and every cancelAfterPostProcessing site passes what it has; the settle-then-abort and hook-stop paths do the same. 2. A -32001 that lands while the parent signal is aborted is the SDK's abort rejection or a timeout that raced with a cancel; classifying it EXECUTION_TIMEOUT would count user cancels against the timeout SLI. isExecutionTimeoutFailure now defers to the abort in both catch blocks, regardless of which side settled the race first. The two tests that pinned the opposite timeout-wins ordering are updated to the abort-wins semantics the review asked for. Co-Authored-By: Qwen Code <noreply@alibaba-inc.com>
|
🤖 Addressed the latest review feedback (round 12/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 12/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #8180 (round: Critical-only mode)Commit: This round is in Critical-only mode. The maintainer review explicitly named two blocking items — concern (2) Addressed (blocking)Concern (2): post-processing cancellation drops On the ACP note in the review: the main ACP post-execution cancellation path already preserves the references — Concern (4): MCP Declined with evidenceConcern (1), partial — revert the Deferred / needs a maintainer decisionConcern (1), main — split the MCP cancellation/timeout change into its own PR. This requires creating a separate PR and re-scoping this one, which is beyond this workflow's authority (additive commits on this branch only). Left for the maintainer. Concern (3) — "had already completed" on the result-returned path. Deferred for a maintainer decision. The two-wording contract is documented in the design doc and was deliberately pinned with regression tests two commits ago ( Concern (5) — invalid params recorded as Smaller points (unreachable-looking Conflict notes
VerificationCommands actually run (repo root unless noted):
中文说明Address-review 总结 —— PR #8180（本轮：Critical-only 模式）提交： 本轮处于 Critical-only 模式。维护者评审明确点名了两个阻塞项——问题 (2) 已处理（阻塞项）问题 (2)：后处理阶段的取消会丢弃 关于评审中提到的 ACP 部分：ACP 的主后执行取消路径本来就保留了引用—— 问题 (4)：MCP 有证据的拒绝问题 (1) 部分——还原 延后 / 需要维护者决策问题 (1) 主体——把 MCP 取消/超时改动拆成独立 PR。 这需要创建新的 PR 并重新划定本 PR 的范围，超出本工作流的权限（只能在当前分支上做增量提交）。留给维护者处理。 问题 (3)——"had already completed" 出现在结果已返回的路径上。 延后，等待维护者决策。两种措辞的契约写在设计文档中，并且两个提交前（ 问题 (5)——无效参数被记录为 较小意见（ 冲突说明
验证实际运行的命令（未注明者均在仓库根目录）：
Deferred non-Critical feedbackCritical-only mode is active after 10 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 10 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM — the executionStatus state machine is correctly implemented in both Core and ACP with the same contract. Verified: all 20+ pre-execution denial paths pass 'not_started', freeze-on-settle holds through post-hooks/batch/recording, cancellation wins over failure hooks consistently (timeout wins over cancellation), terminal-state guard prevents double-write. Security: no new exposure — QwenLogger receives only call_id + execution_status (no mcp_server_name, no function_args), execution counter deliberately omits function_name for low cardinality. Test coverage outstanding: ~4400 test lines / ~2200 production (2:1), ~18 cancellation race scenarios in both paths, all pre-execution rejections assert not_started on both axes.
Non-blocking:
- (P2) Core/ACP state machine duplication — cancellation messages already diverged ('User cancelled tool execution.' vs 'Tool execution was cancelled.'). Extract shared classifier + message constants as follow-up.
- (P2) ~45 hand-placed abort checks after every await — documented maintenance hazard. Track a structural guard (wrapped awaits or lint rule) as follow-up.
- (P3) Session.ts:9131 outer catch missing executionTimeoutException guard for parity with inner catch
- (P3) Design doc overstates QwenLogger guarantee (tool_name embeds mcp____ prefix, pre-existing) — soften wording
|
Released in v0.21.5. |
5 participants
What this PR does
This PR introduces an execution-specific outcome alongside the existing terminal tool-call status. The terminal status continues to describe the final result of the whole call, while
executionStatusrecords whetherinvocation.execute()was entered and whether it succeeded, failed, or was cancelled.Core scheduler and ACP tool calls now use the same state contract, freeze the execution outcome when execution settles, and preserve that result through hooks, result persistence, recording, image bridging, and batch post-processing. Validation failures, permission denial, PreToolUse blocking, the host invocation guard introduced by #8176, duplicate calls, and other synthetic pre-execution responses are explicitly marked as not started. Cancellation that arrives while failure hooks settle now wins consistently in both Core and ACP, while an approved plan is redacted from model history before a post-processing cancellation is finalized.
Tool-call telemetry is normalized once before fan-out to UI telemetry, chat recording, QwenLogger, OTLP logs, and metrics. The change adds call IDs, execution status, consistent cancellation and error classification, a low-cardinality execution-outcome counter, and execution child spans that exist only for attempted executions. It preserves the terminal
statusdimension onqwen-code.tool.call.countestablished by #8176 and does not otherwise change that metric schema. QwenLogger receives terminal and execution outcomes but does not receive MCP server names or function arguments. User-facing TUI and stream-JSON protocols remain compatible.Why it's needed
The terminal result cannot distinguish a tool implementation that failed after execution started from validation, permission, hook, or post-processing failures that prevented or followed execution. This makes the observed tool failure rate misleading and obscures whether the executable tool boundary is healthy.
The additional execution axis provides a stable execution failure-rate denominator while excluding cancellations, calls that never started, and historical events whose execution state is unavailable. It also makes Core and ACP failure handling observable under the same contract.
Reviewer Test Plan
How to verify
Automated verification on the latest PR head completed with 466 focused Core tests and 506 ACP session tests passing. The repository build, typecheck, full lint, and diff whitespace checks also passed.
Evidence (Before & After)
N/A — this change does not alter user-visible TUI or stream-JSON output.
Tested on
Environment (optional)
Node.js v22.22.3, npm 10.9.8, sandbox disabled for local unit, build, lint, and typecheck verification.
Risk & Scope
/forkexecution, MCP-internal retries, provisional subagent-result reconciliation, shell exit or signal metadata, OpenTelemetrygen_ai.execute_tool.durationalignment, generic retryability, and historical-data migration are also out of scope.executionStatus;CancelledToolCall.tooland.invocationcan now be absent for cancellation before resolution. Per-tool execution failures delivered throughCoreToolScheduler.schedule(): Promise<void>now complete through update/completion callbacks instead of rejecting the scheduling promise, while scheduler-level queue/setup failures can still reject;handleConfirmationResponse()terminalizes confirmation-flow errors and then rethrows. Permission transport failures are classified asunhandled_exceptionrather than user denial, recognized MCP execution failures usemcp_tool_error, and post-processing cancellation replaces the model-visible tool response. The terminal metric schema from feat(core): Normalize tool-call terminal telemetry #8176 is preserved. The execution counter has onlyexecution_statusandtool_typeevent dimensions, while globally configured common attributes such as opt-insession.idmay also be present. Dashboards should cut over by deployment time orservice.versionand should not mix historical events into the execution-failure SLI.Linked Issues
Closes #8179
中文说明
本 PR 的作用
本 PR 在现有工具调用终态之外新增独立的执行结果。终态继续描述整个调用的最终结果，
executionStatus则记录是否进入了invocation.execute()，以及执行成功、失败还是被取消。Core 调度器和 ACP 工具调用现在使用同一套状态约定，在执行 settle 时冻结执行结果，并在 hook、结果持久化、recording、图片桥接和批次后处理期间保持该结果不变。校验失败、权限拒绝、PreToolUse 阻断、#8176 引入的 host invocation guard、重复调用以及其他执行前 synthetic response 都会被显式标记为未开始执行。失败 hook settle 期间到达的取消现在会在 Core 和 ACP 中一致地胜出；批准的计划会先从模型历史中脱敏，再完成后处理取消的终态化。
工具调用遥测在扇出到 UI telemetry、聊天记录、QwenLogger、OTLP 日志和指标之前统一归一化一次。本变更新增 call ID、执行状态、一致的取消与错误分类、低基数执行结果计数器，以及仅在实际尝试执行时存在的 execution 子 Span。它保留 #8176 在
qwen-code.tool.call.count上建立的终态status维度，不再修改该指标的其他 schema。QwenLogger 接收终态和执行结果，但不接收 MCP server 名称或函数参数。用户可见的 TUI 和 stream-JSON 协议保持兼容。为什么需要
终态无法区分工具实现在开始执行后失败，与校验、权限、hook 或后处理导致的执行前或执行后失败。这会使观测到的工具失败率失真，也无法判断真正的工具执行边界是否健康。
新增执行轴后，可以建立稳定的执行失败率分母，并排除取消、从未开始执行的调用，以及缺少执行状态的历史事件；同时 Core 与 ACP 的失败处理也能按照同一约定观测。
Reviewer 测试计划
如何验证
基于最新 PR head 的自动化验证结果为 466 个 Core 定向测试和 506 个 ACP session 测试通过；仓库 build、typecheck、完整 lint 和 diff 空白检查也全部通过。
证据（变更前后）
N/A — 本变更不修改用户可见的 TUI 或 stream-JSON 输出。
测试平台
环境（可选）
Node.js v22.22.3、npm 10.9.8；本地单元测试、build、lint 和 typecheck 验证未启用 sandbox。
风险与范围
/fork执行、MCP 内部重试、子代理 provisional result 协调、Shell exit 或 signal 元数据、OpenTelemetrygen_ai.execute_tool.duration对齐、通用 retryability 和历史数据迁移也不在本次范围内。executionStatus；对于工具解析前取消，CancelledToolCall.tool和.invocation现在可能不存在。通过CoreToolScheduler.schedule(): Promise<void>执行的单工具失败现在经 update/completion callback 完成，不再拒绝调度 Promise，但调度器级队列或初始化故障仍可拒绝；handleConfirmationResponse()会先终态化确认流程错误再重新抛出。权限传输故障归类为unhandled_exception而不是用户拒绝，已识别 MCP 执行故障使用mcp_tool_error，后处理取消会替换模型可见的工具响应。feat(core): Normalize tool-call terminal telemetry #8176 建立的终态指标 schema 保持不变。execution counter 只有execution_status和tool_type事件维度，但也可能包含全局配置的公共属性，例如显式启用的session.id。面板应按部署时间或service.version切换，并且不应把历史事件混入执行失败率 SLI。关联 Issue
Closes #8179