feat(core): Normalize tool-call terminal telemetry

[doudouOUC]

What this PR does

This PR establishes one terminal contract for tool-call outcomes before they fan out to telemetry consumers. Core soft errors without a machine-readable classification now become unknown, while every emitted tool-call event derives the compatibility success field from its authoritative status, removes stale error fields from success and cancelled events, supplies unknown for unclassified errors, and replaces blank tool names with unknown_tool.

The normalized event is shared by UI telemetry, chat recording, QwenLogger, OpenTelemetry logs, and tool-call metrics. The tool-call counter gains the low-cardinality status dimension while retaining success and source compatibility for direct metric callers. QwenLogger gains status and tool_type without collecting tool arguments, results, stack traces, or MCP server names. The telemetry and usage-statistics documentation now describes the contract and its rollout boundary.

Why it's needed

Core and ACP currently produce tool-call terminal events through different paths, so status, success, error, and error_type can disagree or be absent. That causes logs, metrics, hooks, and session records to represent the same outcome differently and leaves unclassified failures in a missing bucket. A single normalization boundary makes downstream analysis deterministic while preserving existing public tool and metric APIs.

Reviewer Test Plan

How to verify

• Run cd packages/core && npx vitest run src/core/coreToolScheduler.test.ts src/telemetry/loggers.test.ts src/telemetry/metrics.test.ts src/telemetry/qwen-logger/qwen-logger.test.ts; expect all four files and 471 tests to pass.
• Confirm the scheduler soft-error case records error_type: "unknown" in the completed call, PostToolBatch payload, and chat recording without changing the visible error message.
• Confirm success and cancelled events reach every telemetry consumer without stale error fields, while explicit error classifications remain unchanged and missing classifications become unknown.
• Confirm the tool-call counter includes status but the latency histogram does not, and a legacy direct caller that omits status still records a derived value.
• Run npm run build, npm run typecheck, and npm run lint; expect all commands to succeed.

Evidence (Before & After)

N/A — this is a non-UI telemetry contract change.

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested
🐧 Linux	⚠️ not tested

Environment (optional)

macOS 26.4.1, Node.js 24.12.0, npm 10.9.8, local unit-test environment without sandboxing.

Risk & Scope

• Main risk or tradeoff: Existing unclassified Core failures move from a missing error_type bucket to unknown, success and cancelled telemetry no longer retains stale error fields, and the existing tool-call counter gains a new dimension.
• Not validated / out of scope: Live QwenLogger and OTLP backend ingestion were not exercised. The global CLI dry-run was blocked before model execution by local Keychain error SecItemCopyMatching failed -50. ACP permission-cancellation status fixes, ACP raw tool_result normalization, PostToolUseFailure classification, primary tool-span classification, and legacy UI totalFail semantics remain follow-up work. The new status metric must not become the stability SLO source until producer-side status fixes land.
• Breaking changes / migration notes: No public API break is intended. Existing direct metric callers may omit status; historical queries should coalesce missing error_type values to unknown. No data backfill is required.

Linked Issues

Closes #8175

中文说明

本 PR 做了什么

本 PR 在工具调用结果分发到各遥测消费者之前建立统一的终态契约。Core 中没有机器可读分类的软错误现在会归为 unknown；每个工具调用事件都以 status 为权威来源推导兼容字段 success，从成功和取消事件中移除残留错误字段，为未分类错误补充 unknown，并将空白工具名替换为 unknown_tool。

归一化后的事件由 UI 遥测、会话记录、QwenLogger、OpenTelemetry 日志和工具调用指标共同使用。工具调用计数器新增低基数 status 维度，同时保留 success 并兼容直接调用指标 API 的旧调用方。QwenLogger 新增 status 和 tool_type，但不采集工具参数、结果、堆栈或 MCP 服务名。遥测及用量统计文档同步说明了这套契约及其上线边界。

为什么需要

Core 与 ACP 当前通过不同路径产生工具调用终态事件，因此 status、success、error 和 error_type 可能互相矛盾或缺失，导致日志、指标、Hook 与会话记录对同一结果给出不同表示，并让未分类失败落入缺失值桶。统一的归一化边界可以在保留现有公开工具与指标 API 的同时，让下游分析结果保持确定。

Reviewer 测试计划

验证方式

• 运行 cd packages/core && npx vitest run src/core/coreToolScheduler.test.ts src/telemetry/loggers.test.ts src/telemetry/metrics.test.ts src/telemetry/qwen-logger/qwen-logger.test.ts；预期四个测试文件、471 项测试全部通过。
• 确认调度器软错误场景在已完成调用、PostToolBatch 载荷和会话记录中都写入 error_type: "unknown"，且用户可见错误信息不变。
• 确认成功与取消事件到达每个遥测消费者时不再携带残留错误字段；显式错误分类保持不变，缺失分类则变为 unknown。
• 确认工具调用计数器包含 status，延迟直方图不包含该维度；省略 status 的旧直接调用方仍会记录推导后的值。
• 运行 npm run build、npm run typecheck 和 npm run lint；预期所有命令成功。

前后对比证据

N/A——这是非 UI 的遥测契约变更。

测试平台

操作系统	状态
🍏 macOS	✅ 已测试
🪟 Windows	⚠️ 未测试
🐧 Linux	⚠️ 未测试

环境（可选）

macOS 26.4.1、Node.js 24.12.0、npm 10.9.8，本地无沙箱单元测试环境。

风险与范围

• 主要风险或权衡：Core 中现有未分类失败会从缺失 error_type 桶迁移到 unknown；成功与取消遥测不再保留残留错误字段；现有工具调用计数器会新增一个维度。
• 未验证或范围外：未实际验证 QwenLogger 与 OTLP 后端接收。全局 CLI 试运行在模型执行前被本地 Keychain 错误 SecItemCopyMatching failed -50 阻断。ACP 权限取消状态修复、ACP 原始 tool_result 归一化、PostToolUseFailure 分类、主要工具 Span 分类和旧 UI totalFail 语义仍属于后续工作。在生产端状态修复落地前，新增状态指标不能作为稳定性 SLO 数据源。
• 破坏性变更或迁移说明：不计划引入公开 API 破坏。现有直接指标调用方可以省略 status；历史查询应将缺失的 error_type 合并为 unknown。无需回填数据。

关联 Issue

Closes #8175

[doudouOUC]

E2E Test Report

Scope

Validate that an unclassified soft tool error keeps its visible failure behavior while the terminal telemetry and PostToolBatch payload gain error_type: "unknown".

Global CLI baseline

• Version: qwen 0.21.1
• Attempted command: qwen --safe-mode -y --output-format stream-json --prompt 'Call the tool_search tool exactly once with query set to an empty string. Do not call any other tool. After the tool result, stop and briefly report it.'
• Result: blocked before the model request with local Keychain error SecItemCopyMatching failed -50; no tool call was executed.

Deterministic verification

• cd packages/core && npx vitest run src/core/coreToolScheduler.test.ts src/telemetry/loggers.test.ts src/telemetry/metrics.test.ts src/telemetry/qwen-logger/qwen-logger.test.ts
• Result: 4 test files passed, 471 tests passed.
• The scheduler test covers the same untyped ToolResult.error path and verifies the completed call, PostToolBatch payload, and chat recording all contain error_type: "unknown".
• The logger tests verify the normalized terminal contract across UI telemetry, chat recording, QwenLogger, OpenTelemetry logs, and metrics, including operation without the OpenTelemetry SDK.
• npm run build, npm run typecheck, and npm run lint passed.

Remaining limitation

Live QwenLogger and OTLP backend ingestion were not exercised. Producer-side ACP cancellation classification remains out of scope, so the new status dimension should not be used as the stability SLO source until that follow-up lands.

[doudouOUC]

Comprehensive review at head b19ef7a. Verification performed: all 471 tests in the four cited files pass; npm run typecheck and ESLint on all eight changed TS files pass; every one of the ~24 createErrorResponse call sites passes a concrete ToolErrorType, so the signature narrowing is safe; recordToolCallMetrics has exactly one in-repo caller (loggers.ts, which now always passes status), so the optional parameter preserves source compatibility; no instanceof ToolCallEvent checks exist anywhere, so the normalizer returning a plain-object copy is safe; the UNKNOWN fallback sits only in the non-timeout soft-error branch while the timeout branch keeps EXECUTION_TIMEOUT; Core cancelled responses already carried error: undefined, so the cancelled-event error stripping changes nothing on the Core path.

Suggestion — ACP cancellation reason is lost in the desktop transcript replay.

buildQwenTranscriptTelemetryMessage in packages/desktop/packages/shared/src/agent/qwen-agent.ts (lines 4436–4443) renders recorded ui_telemetry tool_call events as isError = uiEvent.success === false || uiEvent.status === 'error', then displays error || 'Tool failed'. Before this PR, the ACP error/cancel producer (Session.ts errorResponse) emitted cancelled events with error: error.message, so the transcript showed the concrete cancellation reason. After normalization, cancelled events reach chat recording with error stripped, so the same replay now degrades to the generic Tool failed text. The toolStatus: 'error' labeling for cancelled calls is pre-existing (cancelled always had success: false) and is covered by the declared totalFail follow-up, but the loss of the cancellation reason text is a new behavior introduced by this PR and is not named in the Risk & Scope section.

The contract itself is right — cancelled events should not carry error fields. The fix belongs in the consumer: drive the transcript branch off status (cancelled → a dedicated Cancelled result/status instead of falling into the error branch reading a now-absent error). Either fold that consumer update into this PR or add it explicitly to the follow-up list in the design doc so it is not silently dropped.

No Critical issues found. The design doc, telemetry doc, and settings doc accurately describe the implemented behavior, and the tests assert the actual contract (including mutation-of-input checks and the OTel-disabled path).

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[doudouOUC]

Inline comments for the desktop-transcript regression detailed in my previous review, anchored to the exact lines.

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

Problem: A real architectural inconsistency, documented in #8175 — Core and ACP produce tool-call terminal events through different paths, so status, success, error, and error_type can disagree or be absent for the same outcome. This is a consistency/contract improvement (the issue is type/enhancement), not a fix for a single observed failure, so a before/after reproduction isn't the right bar here; the acceptance criteria are concrete and testable instead. One note for transparency: the issue is self-reported by the same author, and the category/telemetry / scope/core / priority/P2 labels were applied by the bot's auto-classification rather than a human triager — but the direction now has explicit maintainer sign-off (see @wenshao's end-to-end verification below).

Direction: Reasonable and within scope — one normalization boundary before the fan-out is the natural fix. CHANGELOG has no direct reference, but telemetry consistency is clearly relevant. This touches telemetry, which is a sensitive area, so the prior run flagged it for a maintainer rather than carrying it to approval. That flag is now resolved: @wenshao rebuilt the branch, exercised it against a real CLI and a real ACP agent with the OTel file exporter, and confirmed every claim in the Test Plan reproduces.

Size: ~83 core production lines (scheduler 4, loggers 53, metrics 5, qwen-logger 2, desktop 19) plus ~399 test lines and ~93 docs lines. Well-proportioned — the bulk is tests and the design doc. No size escalation.

Approach: The scope is right and I don't see a materially simpler path — the contract has to be enforced at the one choke point (logToolCall) that all five consumers pass through, plus the producer-side default in the scheduler. The desktop change (qwen-agent.ts) reuses existing helpers to render cancelled as "Interrupted" — a legitimate consumer adaptation now that status is authoritative, and @wenshao's red/green analysis confirmed it prevents a real regression (reverting it makes cancelled replays show "Tool failed" again).

Risk: No elevated risk signals — none of the changed files match the revert-correlated high-risk paths. The practical risk is the contract migration itself (unclassified errors move from a missing error_type bucket to unknown, the counter gains a status dimension, stale error fields are dropped), which the design doc handles with explicit rollout checks and @wenshao verified end-to-end.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

问题： 真实的架构不一致，#8175 有记录——Core 与 ACP 通过不同路径产生工具调用终态事件，因此同一结果的 status、success、error、error_type 可能互相矛盾或缺失。这是一致性/契约改进（issue 为 type/enhancement），不是针对单个已观测故障的修复，因此 before/after 复现不是合适的门槛；其验收标准具体且可测试。透明起见说明一句：该 issue 由同一作者自报，category/telemetry / scope/core / priority/P2 标签由机器人自动分类打上——但方向现已有维护者明确背书（见下方 @wenshao 的端到端验证）。

方向： 合理且在范围内——在扇出前的单一规范化边界是自然的修复。CHANGELOG 无直接引用，但遥测一致性显然相关。这触及遥测这一敏感区域，因此前次运行转交了维护者。该转交现已解决：@wenshao 重建了分支，用真实 CLI 和真实 ACP agent 配合 OTel 文件导出器验证，确认 Test Plan 中的每项声明均可复现。

规模： 约 83 行核心生产代码（scheduler 4、loggers 53、metrics 5、qwen-logger 2、desktop 19）加约 399 行测试、93 行文档。比例合理——大部分是测试和设计文档。无规模升级。

方案： 范围合理，没有更简路径——契约必须在所有五个消费者都经过的唯一咽喉点（logToolCall）强制执行，外加 scheduler 的生产者侧默认值。desktop 改动复用既有 helper 将 cancelled 渲染为 "Interrupted"——既然 status 为权威来源，这是合理的消费端适配，@wenshao 的红/绿分析确认它防止了真实回归（还原后取消的回放重新显示 "Tool failed"）。

风险： 无升级风险信号——改动文件均未匹配 revert 相关高风险路径。实际风险是契约迁移本身，设计文档已用明确的上线检查项处理，@wenshao 已端到端验证。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 4c77f80895c28374d33f45fee4505ce9e183c322 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code Review

Independent proposal: Faced with terminal fields disagreeing across consumers, I'd (1) default unclassified soft errors to UNKNOWN at the scheduler, (2) normalize once in logToolCall — derive success from status, clear stale error fields on success/cancelled, default blank names/types — before any consumer sees the event, (3) add status to the counter with a legacy-boolean fallback, (4) extend QwenLogger with status/tool_type, and (5) write the contract down.

Comparison: The PR matches this almost line for line, and I didn't find a simpler path it missed. The normalization boundary sits in the right place — normalizeToolCallEvent runs once and feeds uiTelemetry, chat recording, QwenLogger, OTel logs, and metrics from the same copy; it returns a new object (the original is provably unmutated, and a test asserts it) and is idempotent. The scheduler fix is two lines, and narrowing createErrorResponse's errorType from ToolErrorType | undefined to ToolErrorType pushes the guarantee to compile time across all call sites.

No critical blockers. One finding @wenshao's live verification surfaced that the author should address:

• normalizeToolCallEvent unguarded .trim() on function_name (loggers.ts:248). logToolCall is a public export of @qwen-code/core, and the Core call site sits in a try { … } finally { … } without a catch. An event missing function_name (requires an SDK/extension caller violating the type, but the type is public) previously logged harmlessly; now it throws TypeError: Cannot read properties of undefined (reading 'trim') and takes down the batch teardown. @wenshao confirmed this against the compiled artifact. The fix is one line: (event.function_name ?? '').trim(). This is the maintainer's stated precondition for LGTM.

Observations a maintainer should weigh (none rising to a block):

• The contract changes are deliberate and documented, and @wenshao verified them end-to-end: unclassified Core errors move from a missing error_type to unknown, success/cancelled events lose stale error fields, and the counter gains a low-cardinality status dimension. The design doc's rollout-checks section is the right mitigation.
• Cancelled events now drop the cancellation reason string (error is cleared alongside error_type). @wenshao noted this is a design tradeoff, not a defect — the error field is free-text diagnostics, and the PR's contract says cancelled events omit error fields. Worth considering whether to preserve error on cancelled events for debuggability, but not blocking.
• The legacy recordToolCallMetrics fallback maps a status-less success: false to error, which can't express cancelled. No current in-repo caller omits status, but a comment at the derivation site would help future readers.
• QwenLogger forwards status and tool_type but deliberately not mcp_server_name or function_args; the test asserts their absence. Good privacy discipline.

Files changed (production logic; tests and docs omitted)

File	What changed
packages/core/src/core/coreToolScheduler.ts	unclassified soft error defaults to UNKNOWN; errorType param narrowed to non-optional
packages/core/src/telemetry/loggers.ts	new normalizeToolCallEvent boundary feeding all five consumers; OTel error aliases split
packages/core/src/telemetry/metrics.ts	counter gains optional status, derived from success for legacy callers
packages/core/src/telemetry/qwen-logger/qwen-logger.ts	forwards status and tool_type only
packages/desktop/packages/shared/src/agent/qwen-agent.ts	transcript restore renders cancelled as "Interrupted"

Testing

This is the PR's own CI on the reviewed commit (I don't run PR code):

Check	Conclusion
Test (ubuntu-latest, Node 22.x)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success
precheck-pr / precheck	✅ success
Classify PR	✅ success
Test (macos-latest, Node 22.x)	⏭️ skipped
Test (windows-latest, Node 22.x)	⏭️ skipped
Integration Tests (CLI, No Sandbox)	⏭️ skipped

Ubuntu unit tests and the web-shell E2E smoke pass; macOS/Windows and integration are skipped (fork PR CI limitation). The normalization is a pure data transform with no platform-specific behavior, and the new tests pin the contract directly (error_type→unknown, success derived from status, stale fields cleared, legacy metric fallback, the QwenLogger field allow-list), so the Ubuntu pass is meaningful signal.

Maintainer live verification (additional evidence): @wenshao rebuilt the branch on Linux (Node 22, npm ci + npm run build) and exercised it against a real qwen CLI and a real qwen --acp agent with the OTel file exporter, A/B'd against the merge base. Key results: (1) a real unclassified soft error (enter_worktree in a non-git dir) lands as error_type: "unknown" in OTel logs, the counter, and the on-disk session record, with byte-identical user-visible output; (2) a real ACP cancellation splits cleanly from failures on the new status counter dimension; (3) red/green analysis — neutralizing each hunk individually produces exactly the expected test failures (472 → 471/467/471/471/471), confirming every hunk is load-bearing; (4) the full packages/core telemetry + scheduler suite (29 files, 1135 tests) and the desktop history suite (41 tests) pass. Not verified: live QwenLogger/OTLP backend ingestion — the unit tests mock the OTel SDK and QwenLogger.enqueueLogEvent, so the emitted wire payload is asserted against mocks, not a real sink. The maintainer's file-exporter run covers the OTel log shape but not the RUM upload path.

中文说明

代码审查

独立方案： 面对终端字段在消费者间不一致，我会 (1) 在 scheduler 将未分类软错误默认为 UNKNOWN，(2) 在 logToolCall 中一次性规范化——从 status 派生 success、清除成功/取消事件的残留错误字段、默认空白名称/类型——再交给任何消费者，(3) 计数器加 status 并保留旧版布尔回退，(4) 扩展 QwenLogger 的 status/tool_type，(5) 写下契约。

对比： PR 与上述方案几乎逐行一致，没有遗漏更简路径。规范化边界位置正确——normalizeToolCallEvent 运行一次，用同一份副本供给 uiTelemetry、会话记录、QwenLogger、OTel 日志和指标；返回新对象（原事件未被修改，有测试断言），且幂等。scheduler 修复两行，将 createErrorResponse 的 errorType 从 ToolErrorType | undefined 收窄为 ToolErrorType，把保证推到编译期覆盖所有调用点。

无关键阻塞项。@wenshao 的实测发现了一个作者应处理的问题：

• normalizeToolCallEvent 未保护地对 function_name 调用 .trim()（loggers.ts:248）。 logToolCall 是 @qwen-code/core 的公开导出，Core 调用点位于无 catch 的 try { … } finally { … } 中。缺少 function_name 的事件过去只是无害地记录，现在会抛 TypeError 并带崩整个批次的收尾流程。@wenshao 已对编译产物实测确认。修复只需一行：(event.function_name ?? '').trim()。这是维护者明确提出的 LGTM 前提条件。

供维护者权衡的观察（单独均不构成阻塞）：契约变更有意且有文档，@wenshao 已端到端验证。取消事件现在丢弃取消原因字符串（error 与 error_type 一同清除），@wenshao 认为这是设计取舍而非缺陷。旧版 recordToolCallMetrics 回退将无 status 的 success: false 映射为 error，无法表达 cancelled，建议在推导处加注释。QwenLogger 只转发 status 和 tool_type，不转发 mcp_server_name/function_args（测试断言其缺失），隐私纪律良好。

测试

Ubuntu 单元测试与 web-shell E2E 冒烟通过；macOS/Windows 与集成测试因 fork CI 限制被跳过。规范化是纯数据转换，无平台特定行为，新增测试直接钉住契约，因此 Ubuntu 通过是有意义信号。

维护者实测（补充证据）： @wenshao 在 Linux 上重建分支并配合真实 CLI 和 ACP agent 及 OTel 文件导出器验证，与 merge base A/B 对比。关键结果：(1) 真实未分类软错误在 OTel 日志、计数器和磁盘会话记录中均归为 error_type: "unknown"，用户可见输出完全一致；(2) 真实 ACP 取消在新 status 计数器维度上与失败干净分离；(3) 红/绿分析——逐个中和每个 hunk 产生预期失败（472 → 471/467/471/471/471），确认每个 hunk 都是承重的；(4) 完整 packages/core 遥测 + 调度器套件（29 文件、1135 项测试）和 desktop 历史套件（41 项测试）通过。未验证：线上 QwenLogger/OTLP 后端摄取——单元测试 mock 了 OTel SDK 与 QwenLogger.enqueueLogEvent。维护者的文件导出器运行覆盖了 OTel 日志形状，但未覆盖 RUM 上传路径。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 4c77f80895c28374d33f45fee4505ce9e183c322 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 4/5 — clean, well-tested normalization with a thorough maintainer end-to-end verification; one actionable fix (@wenshao's .trim() guard) stands between this and LGTM.

Stepping back: this PR does one thing and does it well. The normalization boundary in logToolCall is the natural answer — it's the single choke point all five consumers pass through, so one function enforces the contract everywhere, and my independent proposal landed in the same place. The ~83 production lines are all necessary, and the tests pin the contract (immutability, idempotency, the legacy fallback, the QwenLogger allow-list) rather than just the happy path. If I had to maintain this in six months I'd thank the author — the contract table in the design doc makes the invariant obvious.

What changed since the last run: @wenshao rebuilt the branch on Linux, drove it against a real CLI and a real ACP agent with the OTel file exporter, A/B'd every contract row against the merge base, and ran a red/green analysis confirming every hunk is load-bearing. The prior run's reason for deferring — "the direction hasn't had explicit maintainer sign-off" — is resolved. The telemetry contract change now has a maintainer's eyes and a maintainer's verdict: every claim in the Test Plan reproduces.

Why I'm not approving yet: @wenshao's verification surfaced one actionable fix — normalizeToolCallEvent calls event.function_name.trim() without guarding against undefined, which would throw in the telemetry path and crash the batch teardown if an SDK/extension caller omits the field. The maintainer stated plainly: "处理完第 1 条即可 LGTM." The fix is one line ((event.function_name ?? '').trim()), and once it lands this is ready to ship. The other three review notes (cancelled-event error stripping as a design tradeoff, the legacy cancelled→error fallback comment, and the 471→472 test count) are non-blocking.

For the author: the .trim() guard is the one remaining item. The maintainer's other notes are suggestions, not blockers — address them if you agree, or note why you don't.

中文说明

置信度：4/5 — 干净、测试充分的规范化，维护者已做彻底端到端验证；一个可操作的修复（@wenshao 的 .trim() 防护）是 LGTM 前的最后一步。

总体来看：这个 PR 只做一件事，而且做得好。logToolCall 中的规范化边界是自然的答案——它是所有五个消费者都经过的唯一咽喉点，一个函数即可在各处强制契约，我的独立方案也落在同一处。约 83 行生产代码都是必要的；测试钉住的是契约（不可变性、幂等性、旧版回退、QwenLogger 允许列表），而非仅 happy path。半年后维护它我会感谢作者——设计文档中的契约表让不变量一目了然。

与上次运行相比的变化：@wenshao 在 Linux 上重建了分支，用真实 CLI 和真实 ACP agent 配合 OTel 文件导出器验证，与 merge base 逐行 A/B 对比了契约表，并做了红/绿分析确认每个 hunk 都是承重的。前次运行推迟的原因——"方向尚未有维护者明确背书"——已解决。遥测契约变更现已有维护者过目和定论：Test Plan 中的每项声明均可复现。

为何尚未批准：@wenshao 的验证发现了一个可操作的修复——normalizeToolCallEvent 未保护地对 function_name 调用 .trim()，如果 SDK/扩展调用方省略该字段，会在遥测路径中抛异常并带崩批次收尾。维护者明确表示："处理完第 1 条即可 LGTM。"修复只需一行（(event.function_name ?? '').trim()），一旦落地即可合并。其余三条评审意见（取消事件错误剥离属设计取舍、旧版 cancelled→error 回退注释、471→472 测试计数）均不构成阻塞。

致作者：.trim() 防护是唯一剩余项。维护者的其他意见是建议而非阻塞——如果你同意就处理，或说明为何不处理。

— Qwen Code · qwen3.8-max-preview

_{Reviewed at 4c77f80895c28374d33f45fee4505ce9e183c322 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[qwen-code-ci-bot]

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.8-max-preview via Qwen Code /review

[qwen-code-ci-bot]

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

中文说明

⚠️ 已从批准降级为评论：CI still running。 已审查。

— qwen3.8-max-preview via Qwen Code /review

[wenshao]

Maintainer verification — built and exercised locally on Linux ✅

I rebuilt this branch from scratch (npm ci + npm run build) in a clean worktree and ran the terminal contract against a real qwen CLI and a real qwen --acp agent driven by a mock OpenAI endpoint, with the OpenTelemetry file exporter capturing the actual qwen-code.tool_call log records and qwen-code.tool.call.count metric points. Everything was A/B'd against the merge-base c3620bc7a, built and installed the same way.

Verdict: every claim in the Test Plan reproduces. Four review notes below, one of which I'd like fixed before merge (a new unguarded .trim() in the telemetry path).

PR head	4c77f80 (b19ef7a + 4c77f80)
Base	c3620bc7a (merge-base with main)
Platform	🐧 Linux 6.12.63 x86-64, Node v22.22.2, npm 10.9.7, bun 1.3.14
Gates	npm run build ✅ · npm run typecheck ✅ · npm run lint ✅

---

1. Live end-to-end: unclassified soft error now lands as unknown

Real headless run (--yolo -p … --telemetry --telemetry-target local --telemetry-outfile) in a non-git working directory, mock model driving three tool calls:

• read_file on an existing file → success
• read_file on a missing file → classified error (file_not_found)
• enter_worktree → the real unclassified soft-error site (enter-worktree.ts errorResult() returns error: { message } with no type)

qwen-code.tool_call log records, verbatim from the two outfiles:

BASE  {"function_name":"enter_worktree","status":"error","success":false,
       "error":"Cannot create a worktree: <work> is not a git repository…",
       "error.message":"Cannot create a worktree: …"}
                                            ← no error_type, no error.type

PR    {"function_name":"enter_worktree","status":"error","success":false,
       "error":"Cannot create a worktree: <work> is not a git repository…",
       "error_type":"unknown",
       "error.message":"Cannot create a worktree: …","error.type":"unknown"}

The visible tool output is byte-identical on both sides; only the classification changes. The same normalization reaches the on-disk session record — the ui_telemetry entry in ~/.qwen/projects/<slug>/chats/<id>.jsonl carries "error_type": null on base and "error_type": "unknown" on this PR, which is what the desktop transcript replay reads.

qwen-code.tool.call.count from the same run:

BASE  {function_name:read_file,      success:true,  decision:auto_accept, tool_type:native} = 1
      {function_name:read_file,      success:false, decision:auto_accept, tool_type:native} = 1
      {function_name:enter_worktree, success:false, decision:auto_accept, tool_type:native} = 1

PR    {function_name:read_file,      status:success, success:true,  …} = 1
      {function_name:read_file,      status:error,   success:false, …} = 1
      {function_name:enter_worktree, status:error,   success:false, …} = 1

qwen-code.tool.call.latency stayed {function_name}-only on both sides, as the design doc says it should.

2. Live end-to-end: real ACP cancellation

Drove a real qwen --acp stdio agent over JSON-RPC — initialize → session/new → session/prompt, mock model calls run_shell_command on a 30 s command, then session/cancel 2.5 s into execution:

BASE  log: status=cancelled success=false   counter: {function_name, success:false, tool_type}
PR    log: status=cancelled success=false   counter: {function_name, status:cancelled, success:false, tool_type}

This is the practical payoff of the new dimension: on base, a cancelled call and a failed call are literally the same counter series (success:false), so no query can separate user cancellations from real failures. On this PR they split cleanly.

I also checked the adjacent case — a run_shell_command that hits its own per-call timeout reports status:error with error_type:execution_timeout on both sides, so timeouts are not swept into the cancelled bucket.

3. Compiled-artifact A/B of the full contract table

The remaining contract rows need producer payloads that the live paths don't currently emit (the Core scheduler already nulls error/errorType on cancel via createCancelledResponse). I fed the built logToolCall from each tree's packages/core/dist the exact object literals the two ACP producer sites construct (Session.ts:7119 and Session.ts:8328) and captured what reaches the chat-recording consumer:

case	base → consumer	this PR → consumer
ACP errorResponse() with an already-aborted signal	status=cancelled success=false error="Tool execution was cancelled"	status=cancelled success=false (error fields removed)
ACP soft failure, no classification	status=error error_type=<absent>	status=error error_type="unknown"
success event carrying a leftover error	status=success error="stale" error_type="execution_failed"	status=success (error fields removed)
blank function_name	function_name=""	function_name="unknown_tool"

The normalizer is non-mutating in all four cases — the caller's event object is unchanged after logToolCall returns, confirmed on both trees.

4. Red/green — each fix hunk neutralized one at a time

hunk neutralized	result
coreToolScheduler.ts ?? ToolErrorType.UNKNOWN	1 failed | 314 passed
normalizeToolCallEvent() short-circuited to {...event}	5 failed | 106 passed
metrics.ts status derivation line removed	1 failed | 45 passed
qwen-logger.ts status/tool_type removed	1 failed | 39 passed
desktop cancelled-replay hunk reverted to base (bun)	1 failed | 40 passed
unmodified PR tree	0 failed | 472 passed

The desktop failure is the informative one — reverting the hunk turns the replayed cancelled call back into toolResult: "Tool failed", which is exactly the regression the normalizer would otherwise introduce once error is stripped from cancelled events. Good catch pulling that fix into the same PR.

5. Wider regression sweep

• packages/core src/telemetry + coreToolScheduler.test.ts — 29 files / 1135 tests passed
• packages/desktop qwen-agent-slash-history.test.ts (bun) — 41 passed
• npm run build, npm run typecheck, npm run lint — all clean

---

Review notes

1. normalizeToolCallEvent dereferences function_name unguarded — please harden (loggers.ts:248)

function_name: event.function_name.trim().length > 0 ? … : 'unknown_tool',

logToolCall is a public export of @qwen-code/core (telemetry/index.ts), and the Core call site (coreToolScheduler.ts:5279) sits in a try { … } finally { … } with no catch, immediately before recordToolResults() and onAllToolCallsComplete(). An event without function_name used to log harmlessly; now it throws and takes the batch-completion path down with it. Against the built dist:

BASE  logToolCall returned normally; consumer got function_name=undefined
PR    logToolCall THREW: TypeError: Cannot read properties of undefined (reading 'trim')

Both in-repo producers do set the field, so this needs a type violation from an SDK/extension caller to trigger — but the fix is one character of defensiveness and the blast radius is a whole tool batch:

function_name: (event.function_name ?? '').trim().length > 0 ? event.function_name : 'unknown_tool',

The same treatment is worth applying to the unknown_tool fallback so it emits 'unknown_tool' rather than undefined when the field is missing.

2. Cancelled events now drop the cancellation reason string

error: isError ? event.error : undefined means the ACP errorResponse() path (Session.ts:7117–7134) loses its message. That path sets status: 'cancelled' and the thrown exception's message whenever the abort signal is already aborted — so if an abort races a genuine tool exception, the only record of what actually threw disappears from every telemetry surface at once (OTel log, QwenLogger, chat recording, UI). The Risk section does disclose "no longer retains stale error fields", and I could not force that race live, so this is a design call rather than a defect — but error is a free-form diagnostic string, not a classification, and it's the error_type field that actually needs to be authoritative. Consider keeping error on cancelled events and stripping only error_type, or preserving the reason under a distinct attribute so the "cancelled" bucket stays debuggable.

3. Legacy recordToolCallMetrics derivation maps status-less cancellations to error (metrics.ts:596)

status: attributes.status ?? (attributes.success ? 'success' : 'error')

No in-repo caller omits status today, but the design doc explicitly offers this as the compatibility path — so the first external caller that records a cancellation without a status will silently inflate the error bucket, which is the one dimension the doc says operators should watch during rollout. A short comment at the derivation (or in the contract doc's compatibility section) noting "legacy callers cannot express cancelled" would prevent that.

4. Test Plan says 471 tests; the current head runs 472

The count is from the first commit — 4c77f80 adds preserves an explicitly classified error type to loggers.test.ts. Worth bumping so the Test Plan matches what a reviewer sees.

---

None of these block the contract itself: status is authoritative end to end, the unknown bucket lands in real telemetry, the counter gains a genuinely useful dimension, and the latency histogram and public APIs are untouched. LGTM once note 1 is addressed.

中文版

Maintainer 验证 —— 已在 Linux 上本地构建并实测 ✅

我在干净的 worktree 中用 npm ci + npm run build 完整重建了本分支，并针对真实 qwen CLI 与真实 qwen --acp agent（由 mock OpenAI 端点驱动）执行了这套终态契约，通过 OpenTelemetry 文件导出器抓取真实的 qwen-code.tool_call 日志记录和 qwen-code.tool.call.count 指标点。所有对比都以相同方式构建安装的 merge-base c3620bc7a 作为基线。

结论：Test Plan 中的每一条声明都可复现。 下面有 4 条评审意见，其中第 1 条建议合并前修复（遥测路径上新增了一处未做保护的 .trim()）。

PR head	4c77f80（b19ef7a + 4c77f80）
Base	c3620bc7a（与 main 的 merge-base）
平台	🐧 Linux 6.12.63 x86-64、Node v22.22.2、npm 10.9.7、bun 1.3.14
门禁	npm run build ✅ · npm run typecheck ✅ · npm run lint ✅

1. 端到端实测：未分类软错误现在归为 unknown

在非 git 目录中真实执行 headless 运行（--yolo -p … --telemetry --telemetry-target local --telemetry-outfile），mock 模型依次驱动三次工具调用：read_file 命中（成功）、read_file 缺失文件（已分类错误 file_not_found）、enter_worktree（真实的未分类软错误点，errorResult() 返回不带 type 的 error: { message }）。

两份 outfile 中 qwen-code.tool_call 的原始记录：

BASE  status=error  error="Cannot create a worktree: … is not a git repository…"
                    ← 没有 error_type，也没有 error.type
PR    status=error  error="Cannot create a worktree: …"
                    error_type="unknown"   error.type="unknown"

两侧用户可见的工具输出完全一致，只有分类发生变化。归一化同样落到磁盘会话记录：~/.qwen/projects/<slug>/chats/<id>.jsonl 中的 ui_telemetry 条目在 base 上是 "error_type": null，在本 PR 上是 "error_type": "unknown" —— 这正是 desktop 转录回放读取的字段。

同一次运行的 qwen-code.tool.call.count：base 三个数据点只有 success；本 PR 三个数据点都带上了 status。qwen-code.tool.call.latency 两侧都仍然只有 {function_name}，与设计文档一致。

2. 端到端实测：真实 ACP 取消

通过 JSON-RPC 驱动真实的 qwen --acp stdio agent（initialize → session/new → session/prompt），mock 模型调用 30 秒的 run_shell_command，执行 2.5 秒后发送 session/cancel：

BASE  log: status=cancelled success=false   counter: {function_name, success:false, tool_type}
PR    log: status=cancelled success=false   counter: {function_name, status:cancelled, success:false, tool_type}

这正是新维度的实际价值：在 base 上，取消的调用和失败的调用是同一条计数器时间序列（success:false），任何查询都无法把用户取消和真实失败区分开；本 PR 让它们干净地分开。

我另外验证了相邻场景：命中自身 per-call timeout 的 run_shell_command 在两侧都是 status:error + error_type:execution_timeout，超时不会被扫进 cancelled 桶。

3. 编译产物层面的完整契约 A/B

剩余的契约行需要现有活路径不会产生的 producer 载荷（Core 调度器已经通过 createCancelledResponse 把取消的 error/errorType 置空）。我把两棵树 packages/core/dist 中已编译的 logToolCall 直接喂入两个 ACP producer 站点（Session.ts:7119 与 Session.ts:8328）构造的字面量对象，观察到达会话记录消费者的内容：

场景	base → 消费者	本 PR → 消费者
ACP errorResponse() 且信号已 abort	status=cancelled error="Tool execution was cancelled"	status=cancelled（错误字段已移除）
ACP 软失败、无分类	status=error error_type=<缺失>	status=error error_type="unknown"
携带残留 error 的成功事件	status=success error="stale" error_type="execution_failed"	status=success（错误字段已移除）
空 function_name	function_name=""	function_name="unknown_tool"

四个场景中归一化都不修改调用方对象，两棵树上均已确认。

4. 红/绿 —— 逐个中和修复 hunk

被中和的 hunk	结果
coreToolScheduler.ts 的 ?? ToolErrorType.UNKNOWN	1 failed | 314 passed
normalizeToolCallEvent() 短路为 {...event}	5 failed | 106 passed
metrics.ts status 推导行删除	1 failed | 45 passed
qwen-logger.ts 的 status/tool_type 删除	1 failed | 39 passed
desktop 取消回放 hunk 还原为 base（bun）	1 failed | 40 passed
未修改的 PR 树	0 failed | 472 passed

desktop 那条最有信息量：还原后回放的取消调用重新变回 toolResult: "Tool failed" —— 这正是 error 从 cancelled 事件中剥离后本会引入的回归。把这个修复一并放进同一个 PR 是对的。

5. 更大范围回归

• packages/core 的 src/telemetry + coreToolScheduler.test.ts —— 29 个文件 / 1135 项测试通过
• packages/desktop 的 qwen-agent-slash-history.test.ts（bun）—— 41 通过
• npm run build、npm run typecheck、npm run lint —— 全部通过

评审意见

1. normalizeToolCallEvent 未保护地解引用 function_name，建议加固（loggers.ts:248）

logToolCall 是 @qwen-code/core 的公开导出（telemetry/index.ts），而 Core 调用点（coreToolScheduler.ts:5279）位于一个没有 catch 的 try { … } finally { … } 中，且紧邻 recordToolResults() 与 onAllToolCallsComplete() 之前。缺少 function_name 的事件过去只是无害地记录，现在会抛异常并把整个批次的收尾流程一起带崩。对已编译产物实测：

BASE  logToolCall 正常返回；消费者收到 function_name=undefined
PR    logToolCall 抛出：TypeError: Cannot read properties of undefined (reading 'trim')

仓库内两个 producer 都会设置该字段，所以要触发需要 SDK/扩展调用方违反类型 —— 但修复只是一个防御性写法，而影响面是一整批工具调用：

function_name: (event.function_name ?? '').trim().length > 0 ? event.function_name : 'unknown_tool',

同样建议让 unknown_tool 兜底在字段缺失时真的产出 'unknown_tool' 而不是 undefined。

2. 取消事件现在丢失了取消原因字符串

error: isError ? event.error : undefined 会让 ACP errorResponse() 路径（Session.ts:7117–7134）丢掉其 message。该路径在信号已 abort 时会同时设置 status: 'cancelled' 和抛出异常的 message —— 因此当 abort 与真实工具异常竞争时，"到底抛了什么"的唯一记录会同时从所有遥测面（OTel 日志、QwenLogger、会话记录、UI）消失。Risk 一节确实声明了"不再保留残留错误字段"，我也无法在实机上稳定复现这个竞争，所以这属于设计取舍而非缺陷。但 error 是自由文本诊断信息、不是分类，真正需要权威化的是 error_type。可以考虑在 cancelled 事件上保留 error、只剥离 error_type，或把原因保存到另一个属性，让 cancelled 桶仍然可调试。

3. 旧版 recordToolCallMetrics 推导会把无 status 的取消映射成 error（metrics.ts:596）

目前仓库内没有省略 status 的调用方，但设计文档明确把它作为兼容路径提供 —— 于是第一个不带 status 记录取消的外部调用方，会悄悄抬高 error 桶，而这恰恰是文档要求运营方在灰度期重点观察的维度。建议在推导处（或契约文档的兼容性章节）加一句注释说明"旧调用方无法表达 cancelled"。

4. Test Plan 写的是 471 项测试，当前 head 实际是 472

该数字来自第一个提交；4c77f80 又向 loggers.test.ts 添加了 preserves an explicitly classified error type。建议同步更新，让 Test Plan 与 reviewer 实际看到的一致。

以上都不影响契约本身：status 端到端具备权威性，unknown 桶确实落到了真实遥测中，计数器新增维度确有价值，延迟直方图与公开 API 均未受影响。处理完第 1 条即可 LGTM。

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Triage re-run completed without a new review.

⚠️ The bot has no review of its own on 4c77f80895c28374d33f45fee4505ce9e183c322. If this re-run was meant to approve, it did not — an approval left by another account is a separate vote and does not count as the bot's own.

The stage comments above were updated with the latest result. View workflow run.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Triage re-run completed without a new review.

⚠️ The bot has no review of its own on 4c77f80895c28374d33f45fee4505ce9e183c322. If this re-run was meant to approve, it did not — an approval left by another account is a separate vote and does not count as the bot's own.

The stage comments above were updated with the latest result. View workflow run.

[yiliang114]

LGTM — normalization logic is correct, single boundary at logToolCall, scheduler closes the unclassified-soft-error gap at the source. QwenLogger exclusions tested. Contract well-documented.

Non-blocking:

1. Guard function_name with ?? '' before .trim() to prevent TypeError on malformed ACP events
2. Follow-up to sanitize/truncate error_message in QwenLogger RUM events (pre-existing PII surface)
3. Confirm isQwenUserInterruptStatus/isQwenToolFailureStatus exist on base branch

[qwen-code-ci-bot]

Released in v0.21.3.