fix(hooks): remove the dead updatedMCPToolOutput field (#5422)

[ken-jo]

What this PR does

Removes the updatedMCPToolOutput field from the PostToolUseOutput hook type. The field is declared but never read — a repo-wide search finds it only as the declaration in packages/core/src/hooks/types.ts, with no consumer in the executor, aggregator, scheduler, or SDK (the analogous updatedInput is fully wired by contrast).

Why it's needed

The hook-output contract should reflect what the runtime actually honors. A declared-but-unimplemented field misleads integrators into emitting updatedMCPToolOutput expecting it to rewrite MCP tool output, when nothing reads it. If that capability is wanted later it would need real wiring plus tests; this just drops the dead declaration. Reported in #5422; confirmed dead by the triage and by @wenshao's review.

Reviewer Test Plan

How to verify

Search the repo for updatedMCPToolOutput: before this PR it appears only as the type field in packages/core/src/hooks/types.ts; after, there are zero occurrences. Because nothing consumes it, removing it changes no runtime behavior. npm run build (typecheck) still passes.

Evidence (Before & After)

N/A — non-user-visible, type-only change.

Tested on

OS	Status
🍏 macOS	N/A
🪟 Windows	N/A
🐧 Linux	✅ typecheck / build

Environment (optional)

N/A — type-only change, no runtime.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Hey @ken-jo — thanks for the PR! The change itself looks reasonable (removing a dead field), but the PR body doesn't follow the PR template.

The template requires a few sections that help reviewers understand and verify the change:

• What this PR does — describe the change in prose
• Why it's needed — motivation / problem being solved
• Reviewer Test Plan — how to verify, with evidence (Before/After or N/A for type-only changes) and a "Tested on" table
• Risk & Scope — main risk, what's out of scope, breaking changes
• Linked Issues — Fixes #5422 (you already have this in the body, just needs to be under the heading)
• 中文说明 — Chinese translation in a <details> block

Could you update the PR body to follow the template? It should be quick since the change is small — most sections can be brief.

中文说明

嗨 @ken-jo ——感谢你的 PR！改动本身是合理的（移除一个未使用的字段），但 PR 正文没有按照 PR 模板 来填写。

模板要求几个部分来帮助审查者理解和验证改动：

• What this PR does — 用文字描述改动
• Why it's needed — 动机 / 要解决的问题
• Reviewer Test Plan — 如何验证，附上证据（Before/After，或者纯类型改动写 N/A）和"Tested on"表格
• Risk & Scope — 主要风险、不在范围内的内容、破坏性变更
• Linked Issues — Fixes #5422（你正文里已经有了，只需要放到对应标题下）
• 中文说明 — 用 <details> 块提供中文翻译

能否按照模板更新一下 PR 正文？改动很小，大部分部分可以简单写几句就好。

— Qwen Code · qwen3.7-max

[wenshao]

No review findings. Downgraded from Approve to Comment: CI still running.

The removed updatedMCPToolOutput field is confirmed dead — zero references anywhere in the codebase. Clean removal.

— qwen3.7-max via Qwen Code /review

[ken-jo]

Updated the PR body to follow the template (added the What / Why / Reviewer Test Plan sections). Thanks for the review @wenshao.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

On direction: dead-code cleanup in the hooks type system — squarely within scope and well-aligned with roadmap/hooks-events. Keeping the hook-output contract honest (only declaring what the runtime actually consumes) is exactly the kind of hygiene that prevents integrators from relying on phantom fields. No concerns.

On approach: this is as minimal as it gets — one line removed, no consumers affected. The contrast with updatedInput (18 references, fully wired through aggregator/handler/tests) makes the case clear. Nothing to cut, nothing to split.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

方向：hook 类型系统中的死代码清理——完全在 roadmap/hooks-events 范围内。保持 hook 输出契约的真实性（只声明运行时实际消费的字段）可以防止集成者依赖不存在的功能。没有顾虑。

方案：已经是最小化改动——删除一行，没有消费者受影响。与 updatedInput（18 处引用，在聚合器/处理器/测试中完整接入）的对比很清楚。没有可砍的部分，不需要拆分。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Independent proposal: Read the PR title ("remove the dead updatedMCPToolOutput field") and the "Why" section. My approach: grep for updatedMCPToolOutput across the repo, confirm it only appears as the type declaration in packages/core/src/hooks/types.ts, then remove that one line and run the build to verify. This is identical to what the PR does.

Diff comparison: The PR matches my independent proposal exactly — a single-line deletion in PostToolUseOutput. No other files touched. The contrast is stark: updatedInput has 18 references (fully wired through hookAggregator.ts, hookEventHandler.ts, and tests), while updatedMCPToolOutput has exactly 1 (the orphaned declaration). No correctness issues, no security concerns, no regressions possible.

No blockers. No nitpicks.

Testing

Type-only change, no user-visible behavior to compare before/after. Verified build and hook tests on the PR branch:

$ cd packages/core && npx vitest run src/hooks/hookEventHandler.test.ts src/hooks/hookAggregator.test.ts

 RUN  v3.2.4 /home/runner/work/qwen-code/qwen-code/packages/core

 ✓ src/hooks/hookAggregator.test.ts (49 tests) 22ms
 ✓ src/hooks/hookEventHandler.test.ts (125 tests) 114ms

 Test Files  2 passed (2)
      Tests  174 passed (174)
   Duration  4.71s

Build (npm run build) also passes — 0 errors, only pre-existing curly-brace warnings in vscode-ide-companion (unrelated).

中文说明

代码审查

独立方案： 根据 PR 标题和"Why"部分，我的做法是：在仓库中搜索 updatedMCPToolOutput，确认它只作为类型声明存在于 packages/core/src/hooks/types.ts，然后删除该行并运行构建验证。与 PR 做法完全一致。

Diff 对比： PR 与我的独立方案完全匹配——在 PostToolUseOutput 中删除一行。对比明显：updatedInput 有 18 处引用（通过 hookAggregator.ts、hookEventHandler.ts 和测试完整接入），而 updatedMCPToolOutput 只有 1 处（孤立的声明）。没有正确性问题、安全问题或回归风险。

无阻塞项。无细节意见。

测试

纯类型变更，无用户可见行为可对比。在 PR 分支上验证了构建和 hook 测试：174 个测试全部通过，构建 0 错误。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

This is a textbook dead-code removal. The field was declared, never consumed, and the contrast with its sibling updatedInput (18 references, fully wired) made the diagnosis unambiguous. My independent proposal was identical to the PR: grep, confirm orphaned, delete the line.

Build passes, all 174 hook tests pass, diff is one line. Nothing more to ask for.

Approving. ✅

中文说明

这是一个教科书级的死代码移除。该字段被声明但从未被消费，与其兄弟字段 updatedInput（18 处引用，完整接入）的对比使诊断非常明确。我的独立方案与 PR 完全一致：搜索、确认孤立、删除该行。

构建通过，174 个 hook 测试全部通过，diff 只有一行。没有更多可要求的了。

批准 ✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

✅ Verification report — real build + live-binary test

Verdict: confirmed dead field, safe to merge. The removal is type-only with provably zero runtime footprint: it deletes one optional interface member that nothing reads, that is stripped before any hook output is consumed, and that is fully erased by the TypeScript compiler (the emitted .js is byte-for-byte identical with or without it).

I built the real qwen binary from this PR (v0.18.3) in an isolated worktree and verified the claim four independent ways — static dataflow, dist byte-identity, the full typecheck + hooks test suites, and a live MCP-tool + PostToolUse-hook run in the actual TUI.

---

1. Static dataflow — zero consumers

updatedMCPToolOutput occurs exactly once in the entire source tree — its own declaration:

Search	Result
*.ts / *.tsx (excl. dist, node_modules)	1 hit — the declaration at packages/core/src/hooks/types.ts
*.js compiled output under packages/*/dist	0 hits
*.json / *.md (config + docs)	0 hits

Tracing the PostToolUse execution path confirms why: a raw hook result is wrapped by createHookOutput(...), and DefaultHookOutput's constructor only copies continue / stopReason / suppressOutput / systemMessage / decision / reason / hookSpecificOutput — updatedMCPToolOutput is dropped on the floor at that boundary. There is no getter, no result-struct field, and no apply-site for it anywhere.

Contrast — the sibling field updatedInput is fully wired (this is what a live field looks like):
PermissionRequestHookOutput.getUpdatedToolInput() (types.ts:653) → PermissionRequestHookResult.updatedInput (toolHookTriggers.ts:511,572) → applied via setArgsInternal(...) in coreToolScheduler.ts:2085-2093. The symmetric output-rewrite chain that updatedMCPToolOutput implies was never built.

2. Origin — incidental cruft, dead ~5 weeks

git log -S shows the field was introduced by a single, unrelated commit and never touched since:

fd53527aad — feat(cli): support batch deletion of sessions in /delete (#3733) (2026-05-13)

It rode in on a session-deletion feature and has been dead from day one.

3. Type-only → zero runtime footprint (dist byte-identity)

TypeScript interfaces compile to nothing. I built the compiled module with and without the field and hashed it:

Build	core/dist/src/hooks/types.js SHA-256
Field present (parent)	4ac2022042e4ab1700b5d6d0d0669836153cd715720eebb6bec66a9b096fa11a
Field removed (this PR)	4ac2022042e4ab1700b5d6d0d0669836153cd715720eebb6bec66a9b096fa11a

Identical. Removing the field cannot change runtime behavior, because the runtime never contained it.

4. Build + tests (this PR branch)

• npm run build (full monorepo tsc typecheck) → exit 0, 0 errors (15 pre-existing lint warnings, unrelated). No in-repo TS consumer breaks.
• Hooks + toolHookTriggers suites → 23 files / 698 tests passed, 0 failed.

5. Live real-binary test (the decisive behavioral proof)

In a tmux session I ran the actual built qwen against an isolated home containing:

• a real stdio MCP server exposing get_secret → returns MCP_ORIGINAL_OUTPUT_Z5423;
• a PostToolUse command hook (matcher get_secret) that emits, in one output object, both an honored field and the dead field:
  • hookSpecificOutput.additionalContext = "POSTHOOK_ADDLCTX_Z5423"
  • updatedMCPToolOutput = { content:[{ text:"HIJACKED_BY_DEADFIELD_Z5423" }] }

The model called the MCP tool; the hook fired (marker: FIRED tool_name=mcp__secret__get_secret). The tool result actually delivered to the model (verbatim, from the persisted transcript) was:

{ "output": "MCP_ORIGINAL_OUTPUT_Z5423\nPOSTHOOK_ADDLCTX_Z5423" }

Field set by the hook	Intent	Observed in the running binary
additionalContext	append context to the result	✅ honored — appended to what the model saw
updatedMCPToolOutput	replace the MCP tool output	❌ ignored — HIJACKED_BY_DEADFIELD_Z5423 count = 0 in transcript and on screen

Because both fields come from the same hook output, this is a clean within-run A/B: it rules out "the hook simply didn't run." The honored sibling worked; the field this PR removes did nothing.

---

One consideration (non-blocking)

PostToolUseOutput is re-exported from @qwen-code/qwen-code-core (src/index.ts: export * from './hooks/types.js'), so this is technically a removal from that package's public type surface. In practice it's benign: the field was optional and never honored, the @qwen-code/sdk-typescript public surface never referenced it, and any external code that set it was emitting a value the runtime silently discarded. The change simply turns a silent no-op into a helpful compile-time signal. Not a runtime break.

Scope / honesty

Verified: static dataflow, dist byte-identity, full typecheck, hooks/toolHookTriggers suites (698 tests), and a live MCP + PostToolUse run on the real binary. I did not run the entire repo test matrix (unnecessary for a type-only change the typecheck already covers). Recommendation: merge.

🇨🇳 中文版（完整对应）

✅ 验证报告 —— 真实构建 + 真机测试

结论：确认是死字段，可以安全合并。 该删除是纯类型改动、运行时零足迹：删掉的是一个无人读取的可选接口成员，它在任何 hook 输出被消费之前就已被丢弃，并且会被 TypeScript 编译器完全擦除（编译出的 .js 加不加它都逐字节相同）。

我在隔离的 worktree 里用本 PR 构建了真实的 qwen 二进制（v0.18.3），并用四种独立方式验证了该主张 —— 静态数据流、dist 字节级一致、完整类型检查 + hooks 测试套件，以及在真实 TUI 中跑一遍 MCP 工具 + PostToolUse hook。

1. 静态数据流 —— 零消费者

updatedMCPToolOutput 在整个源码树中只出现一次 —— 即它自己的声明：

搜索范围	结果
*.ts / *.tsx（排除 dist、node_modules）	1 处 —— packages/core/src/hooks/types.ts 的声明
packages/*/dist 下编译产物 *.js	0 处
*.json / *.md（配置 + 文档）	0 处

追踪 PostToolUse 执行路径可知原因：原始 hook 结果会被 createHookOutput(...) 包装，而 DefaultHookOutput 的构造函数只拷贝 continue / stopReason / suppressOutput / systemMessage / decision / reason / hookSpecificOutput，updatedMCPToolOutput 在这个边界就被直接丢弃。它在任何地方都没有 getter、没有结果结构体字段、没有应用点。

对照 —— 兄弟字段 updatedInput 是完整接线的（这才是"活字段"的样子）：
PermissionRequestHookOutput.getUpdatedToolInput()（types.ts:653）→ PermissionRequestHookResult.updatedInput（toolHookTriggers.ts:511,572）→ 在 coreToolScheduler.ts:2085-2093 通过 setArgsInternal(...) 应用。updatedMCPToolOutput 所暗示的那条对称的输出改写链从未被实现。

2. 来源 —— 顺带混入的死代码，已死约 5 周

git log -S 显示该字段由一个毫不相关的提交引入，此后从未改动：

fd53527aad —— feat(cli): support batch deletion of sessions in /delete (#3733)（2026-05-13）

它搭着一个"会话删除"功能混进来，从第一天起就是死的。

3. 纯类型 → 运行时零足迹（dist 字节级一致）

TypeScript 接口编译后什么都不剩。我分别在保留和删除该字段的情况下构建编译模块并取哈希：

构建	core/dist/src/hooks/types.js 的 SHA-256
字段存在（父提交）	4ac2022042e4ab1700b5d6d0d0669836153cd715720eebb6bec66a9b096fa11a
字段删除（本 PR）	4ac2022042e4ab1700b5d6d0d0669836153cd715720eebb6bec66a9b096fa11a

完全相同。 删除它不可能改变运行时行为，因为运行时从来就没有它。

4. 构建 + 测试（本 PR 分支）

• npm run build（整个 monorepo 的 tsc 类型检查）→ 退出码 0，0 错误（15 个预存 lint 警告，与本 PR 无关）。仓库内没有任何 TS 消费者被破坏。
• Hooks + toolHookTriggers 套件 → 23 个文件 / 698 个测试通过，0 失败。

5. 真机行为测试（决定性证据）

我在 tmux 会话里用真实构建的 qwen 运行，隔离 home 中配置了：

• 一个真实的 stdio MCP server，暴露 get_secret → 返回 MCP_ORIGINAL_OUTPUT_Z5423；
• 一个 PostToolUse 命令 hook（matcher get_secret），在同一个输出对象里同时发出一个被尊重的字段和这个死字段：
  • hookSpecificOutput.additionalContext = "POSTHOOK_ADDLCTX_Z5423"
  • updatedMCPToolOutput = { content:[{ text:"HIJACKED_BY_DEADFIELD_Z5423" }] }

模型调用了该 MCP 工具；hook 触发（marker: FIRED tool_name=mcp__secret__get_secret）。真正交付给模型的工具结果（取自持久化 transcript，逐字）为：

{ "output": "MCP_ORIGINAL_OUTPUT_Z5423\nPOSTHOOK_ADDLCTX_Z5423" }

hook 设置的字段	意图	在运行的二进制中观察到
additionalContext	向结果追加上下文	✅ 被尊重 —— 追加进了模型看到的内容
updatedMCPToolOutput	替换 MCP 工具输出	❌ 被忽略 —— HIJACKED_BY_DEADFIELD_Z5423 在 transcript 和屏幕上计数均为 0

由于两个字段来自同一个 hook 输出，这构成了一次干净的同跑 A/B：排除了"hook 根本没跑"的可能。被尊重的兄弟字段生效了，而本 PR 删除的这个字段什么都没做。

一点考量（不阻塞合并）

PostToolUseOutput 通过 @qwen-code/qwen-code-core（src/index.ts: export * from './hooks/types.js'）再导出，所以严格说这是从该包的公共类型表面移除一个成员。但实际无害：该字段是可选的、从未被尊重，@qwen-code/sdk-typescript 的公共表面也从未引用它，任何设置它的外部代码都只是在发一个运行时会静默丢弃的值。这次改动只是把一个静默的 no-op 变成了一个有用的编译期信号，不是运行时破坏。

范围 / 诚实说明

已验证：静态数据流、dist 字节级一致、完整类型检查、hooks/toolHookTriggers 套件（698 个测试），以及真机上的 MCP + PostToolUse 实跑。我没有跑整个仓库的测试矩阵（对一个类型检查已经覆盖的纯类型改动来说没必要）。建议：合并。

[ken-jo]

Thanks for the thorough check @wenshao — a real-build + live-binary verification is more than I expected for a dead-field removal, much appreciated. Ready to merge whenever suits you.