fix(acp): add internal Kind.Agent, keep ACP wire on 'other' (no-regression) - #5085
Merged
Conversation
The Agent tool was using Kind.Other as a catch-all, causing WebUI permission dialogs to show generic titles and descriptions. Adding a dedicated Kind.Agent value enables agent-specific UI rendering in permission drawers, tool labels, and export normalization. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
qwen-code-ci-bot
requested changes
Jun 13, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Hi @doudouOUC — thanks for the PR! The change itself (adding Kind.Agent) looks well-scoped and the test coverage is solid 👍
However, the PR body doesn't follow our PR template. The template requires specific sections that help maintainers review efficiently:
## What this PR does/## Why it's needed— currently under## Summary, but split into "what" and "why" helps reviewers quickly orient## Reviewer Test Plan— withHow to verify,Evidence (Before & After), andTested onsubsections. Your test plan lists unit test results (great!), but the reviewer test plan is about how a reviewer can confirm the change works end-to-end## Risk & Scope— the note about the@agentclientprotocol/sdktype assertion is exactly the kind of thing that belongs here## Linked Issues— even "N/A" is fine<details>中文说明</details>— bilingual body for our mixed-language maintainer team
Could you reformat the PR body to match the template? The substance is good — this is just about structure so reviewers can move fast.
嗨 @doudouOUC —— 感谢你的 PR！Kind.Agent 这个改动范围清晰，测试覆盖也很到位 👍
不过 PR 正文没有按照 PR 模板 填写。模板要求的几个部分有助于维护者快速审查：
## What this PR does/## Why it's needed—— 当前合并在## Summary下，分开写"做了什么"和"为什么做"更方便审阅## Reviewer Test Plan—— 包含How to verify、Evidence (Before & After)、Tested on子项。你列出了单元测试结果（很好！），但 reviewer test plan 是告诉审查者如何端到端验证改动## Risk & Scope—— 关于@agentclientprotocol/sdk类型断言的说明正好适合放在这里## Linked Issues—— 填 "N/A" 也行<details>中文说明</details>—— 方便不同语言的维护者阅读
能否按模板重新整理一下 PR 正文？内容本身没问题，主要是结构上的调整。
— Qwen Code · qwen3.7-max
wenshao
reviewed
Jun 13, 2026
Address wenshao review on PR #5085 — add assertion that getToolDisplayLabel({ kind: 'agent' }) returns 'Agent', matching the sibling task/skill cases.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Author
Review follow-upThanks for the review — addressed both items.
The Copilot review was a quota-limit no-op — nothing to address there. Fix commit: 349e035. |
wenshao
reviewed
Jun 13, 2026
…k cast Address wenshao review on PR #5085: - Add normalize.test.ts case asserting an agent-kind tool call is preserved as 'agent' through normalizeSessionData. - Add TODO(acp-sdk) comment on the KIND_MAP 'agent' cast explaining why we emit 'agent' (webui SSE consumer) rather than mapping to 'other'.
Collaborator
Author
Review follow-up (round 2)@wenshao posted a second batch — all addressed.
Commit: 2606ced. All review threads resolved. |
Collaborator
🔬 Local runtime verification —
|
tool_call frame to SSE client |
kind on the wire |
daemon invalid_union |
|
|---|---|---|---|
Parent 2ba4ca90a (pre-PR) |
✅ delivered | "other" |
0 |
This PR 28464d71f |
❌ dropped | — | 1 |
Pre-PR — the frame arrives (kind:"other", in the SDK enum):
{"type":"session_update","data":{"update":{
"sessionUpdate":"tool_call","toolCallId":"call_…","status":"in_progress",
"title":"Agent: probe agent kind","kind":"other",
"_meta":{"toolName":"agent","provenance":"builtin"}}}}This PR — the frame is rejected at the daemon (yolo, plain notification):
Error handling notification {
method: 'session/update',
params: { update: { sessionUpdate: 'tool_call', title: 'Agent: probe agent kind',
kind: 'agent', … } } }
{ code: -32603, message: 'Internal error',
data: [ { code: 'invalid_union', message: 'Invalid input', path: […] } ] }
SSE subtypes delivered → pre-PR: tool_call ×1, tool_call_update ×1. This PR: tool_call ×0, tool_call_update ×1 (an orphaned "completed" update for a call the client never saw start).
This PR — default mode rejects the confirmation too (so the drawer never opens; permission_request events delivered = 0):
Error handling request {
method: 'session/request_permission',
params: { toolCall: { title: 'probe agent kind', kind: 'agent',
status: 'pending', … }, options: [4] } }
{ code: -32603, data: [ { code: 'invalid_union', message: 'Invalid input' } ] }
Root cause
The pinned ACP SDK rejects the value at runtime. @agentclientprotocol/sdk@0.14.1 ToolKind enum =
delete, edit, execute, fetch, move, other, read, search, switch_mode, think
— no agent. The daemon's ACP client connection (via @qwen-code/acp-bridge) Zod-validates every session/update / session/request_permission it receives from the qwen --acp child; kind:"agent" fails invalid_union and the frame is dropped before fan-out to SSE. The 'agent' as ToolKind cast in ToolCallEmitter.ts silences tsc, but not the runtime schema check.
The A/B isolates this conclusively: kind is the only difference between the two runs, and "other" passes while "agent" fails.
Why the PR's green tests miss it
ToolCallEmitter.test.ts asserts mapToolKind(Kind.Agent) === 'agent', and the webui/web-shell tests assert the consumers handle 'agent'. Both are correct in isolation — neither crosses the @agentclientprotocol/sdk validation boundary that exists only in the live daemon/JSONRPC path. So unit-green + wire-red is expected here.
Scope / impact
- Affects every
qwen serveclient today (web-shell SSE +packages/webuidaemon provider), because--http-bridgeis the default and only mode and always routes through the validating ACP child. Also the VS Code/Zed JSONRPC path the note already flagged. - It is a regression, not just "feature not wired yet": the agent
tool_call(and its permission prompt) reached SSE clients before this PR (askind:"other"); now they're dropped entirely. Net UX through the daemon goes from "generic but working dialog" to "no dialog + orphaned tool update."
What's already correct (keep)
The consumer side is fine and will light up the moment the value can be delivered: PermissionDrawer title/subtitle for 'agent', labelUtils → 'Agent', web-shell messageTypes/transcriptToMessages, and the Java SDK enum + schema additions. The gap is purely the producer emitting a value the pinned wire schema rejects.
Recommendation
Sequence the wire schema ahead of (or with) the producer:
- Preferred: bump
@agentclientprotocol/sdkto a version whoseToolKindincludesagent(and regen the JSON schema) in this PR, then the rest works as written; or - No-regression interim: keep the client-side
'agent'handling but haveToolCallEmitteremitagent's kind as'other'until the negotiated SDK advertisesagent(capability/version gate). This preserves today's working delivery while staging the UI.
Either way, drop the as ToolKind cast — it's hiding the very mismatch that breaks at runtime.
Reproduce
# build PR and parent
git worktree add wt-pr 28464d71f && (cd wt-pr && npm ci && npm run build)
git worktree add wt-base 2ba4ca90a && (cd wt-base && npm ci && npm run build)
# mock OpenAI server: return one tool_call for `agent` on the first turn, else plain text
# point the daemon at it and drive a session over SSE:
OPENAI_BASE_URL=http://127.0.0.1:8771/v1 OPENAI_API_KEY=sk-mock OPENAI_MODEL=mock-model \
QWEN_HOME=<isolated> node wt-<pr|base>/packages/cli/dist/index.js serve --port 0 --workspace <ws> --token T
# DaemonClient: createOrAttachSession → subscribeEvents → promptNonBlocking("launch an agent …")
# observe: daemon stderr `invalid_union` on `kind:"agent"`; SSE stream missing the tool_call frame (PR) vs present (base)
Verified locally on Linux, Node v22; tmux-isolated daemon + mock; deterministic stub model. Incidental: a benign craft/drainMidTurnQueue "method not found" appears in both builds (unrelated version-skew, not part of this finding).
🇨🇳 中文版（点击展开）
🔬 本地真实运行验证 —— feat(core): add Kind.Agent（tmux 下真实 qwen serve 守护进程）
结论：❌ 不通过 —— 存在阻断性回归。 我从源码分别构建了本 PR 与其父提交，启动真实的 qwen serve 守护进程（未对传输层做任何 mock），用一个强制触发 agent 工具调用的桩 OpenAI 服务器驱动它，并观察实时 SSE 数据流。在本 PR 上，agent 工具的 tool_call 以及权限确认帧都在守护进程的 ACP 边界处被拒绝、根本到不了 SSE 客户端（web-shell / webui）；而在合入本 PR 之前它们是能正常下发的。因此本 PR 的核心功能（"Launch this agent?" 抽屉）在守护进程路径下根本不会出现，且一条原本可用的链路发生了回归。
这正是 PR 说明里一笔带过的情况（"JSONRPC 路径会优雅拒绝"），但它低估了影响范围：webui/web-shell 的 SSE 路径恰恰位于同一个 JSONRPC 跳转的下游，所以"SSE 路径立即可用"的说法并不成立。
验证方式
- 从源码构建两个 worktree（
npm ci && npm run build）：本 PR28464d71f与其父提交2ba4ca90a（无Kind.Agent，agent 工具为Kind.Other）。 - 真实守护进程：
qwen serve --port 0 --workspace … --hostname 127.0.0.1（默认--http-bridge=true，会派生qwen --acp子进程——这是当前唯一已实现的模式）。 - 通过
OPENAI_BASE_URL/_API_KEY/_MODEL把模型指向一个零依赖的 mock OpenAI 服务器；它在第一轮返回一个agent工具调用，之后返回纯文本。完全确定性，不调用真实模型。 - 客户端：项目自带的
DaemonClient（@qwen-code/sdk）——createOrAttachSession→subscribeEvents（SSE）→promptNonBlocking，并完整抓取每一帧与守护进程的 stderr。 - 相同 mock、相同 prompt、两种审批模式（
default与yolo），分别对两个构建运行——这是一个唯一变量就是本 PR的 A/B 对照。
证据 —— A/B 对照（输入完全相同，yolo 模式）
tool_call 帧是否到达 SSE 客户端 |
线上的 kind |
守护进程 invalid_union |
|
|---|---|---|---|
父提交 2ba4ca90a（PR 前） |
✅ 已下发 | "other" |
0 |
本 PR 28464d71f |
❌ 被丢弃 | — | 1 |
PR 前 —— 帧正常到达（kind:"other"，在 SDK 枚举内）：
{"type":"session_update","data":{"update":{
"sessionUpdate":"tool_call","status":"in_progress",
"title":"Agent: probe agent kind","kind":"other",
"_meta":{"toolName":"agent","provenance":"builtin"}}}}本 PR —— 帧在守护进程处被拒绝（yolo，普通通知）：
Error handling notification { method: 'session/update',
params: { update: { sessionUpdate: 'tool_call', kind: 'agent', … } } }
{ code: -32603, data: [ { code: 'invalid_union', message: 'Invalid input' } ] }
下发的 SSE 子类型 → PR 前：tool_call ×1、tool_call_update ×1；本 PR：tool_call ×0、tool_call_update ×1（一个孤立的"completed"更新——对应一个客户端从未见过"开始"的调用）。
本 PR —— default 模式下确认请求同样被拒绝（因此抽屉永远打不开；下发的 permission_request 事件数 = 0）：
Error handling request { method: 'session/request_permission',
params: { toolCall: { title: 'probe agent kind', kind: 'agent', … }, options: [4] } }
{ code: -32603, data: [ { code: 'invalid_union', message: 'Invalid input' } ] }
根因
被固定版本的 ACP SDK 在运行时拒绝。@agentclientprotocol/sdk@0.14.1 的 ToolKind 枚举为：
delete, edit, execute, fetch, move, other, read, search, switch_mode, think
—— 没有 agent。守护进程的 ACP 客户端连接（经 @qwen-code/acp-bridge）会对从 qwen --acp 子进程收到的每个 session/update / session/request_permission 做 Zod 校验；kind:"agent" 触发 invalid_union，帧在扇出到 SSE 之前就被丢弃。ToolCallEmitter.ts 里的 'agent' as ToolKind 断言只骗过了 tsc，骗不过运行时的 schema 校验。
A/B 对照已将其彻底坐实：两次运行唯一的差异就是 kind，"other" 通过而 "agent" 失败。
为什么 PR 的绿色测试没发现
ToolCallEmitter.test.ts 断言 mapToolKind(Kind.Agent) === 'agent'，webui/web-shell 测试断言消费方能处理 'agent'。两者在隔离环境下都正确——但都没有跨越只存在于真实守护进程/JSONRPC 路径上的 @agentclientprotocol/sdk 校验边界。所以"单测绿、线上红"在这里是必然的。
影响范围
- 影响当前每一个
qwen serve客户端（web-shell SSE +packages/webui的 daemon provider），因为--http-bridge是默认且唯一模式，且必然经过会做校验的 ACP 子进程。同样影响 PR 说明已提及的 VS Code/Zed JSONRPC 路径。 - 这是回归，而非"功能尚未接通"：本 PR 之前 agent 的
tool_call（及其权限提示）是能到达 SSE 客户端的（以kind:"other"），现在被整帧丢弃。守护进程路径上的实际体验由"通用但可用的对话框"退化为"无对话框 + 孤立的工具更新"。
已经正确、应保留的部分
消费方实现没问题，一旦该值能被下发就会立即生效：PermissionDrawer 对 'agent' 的标题/副标题、labelUtils → 'Agent'、web-shell 的 messageTypes/transcriptToMessages，以及 Java SDK 的枚举与 schema 新增。问题纯粹在于生产方发出了一个被固定线协议 schema 拒绝的值。
建议
让线协议 schema 先于（或与）生产方一起落地：
- 首选： 在本 PR 中把
@agentclientprotocol/sdk升到ToolKind含agent的版本（并重新生成 JSON schema），其余代码即可按现状工作；或 - 无回归的过渡方案： 保留客户端对
'agent'的处理，但让ToolCallEmitter在协商到的 SDK 尚未声明agent之前，把 agent 的 kind 仍按'other'发出（按能力/版本做 gate）。这样既能保住今天可用的下发，又能提前铺好 UI。
无论哪种方案，都应去掉 as ToolKind 断言——它正好掩盖了在运行时把功能打断的那个不匹配。
复现
# 构建 PR 与父提交
git worktree add wt-pr 28464d71f && (cd wt-pr && npm ci && npm run build)
git worktree add wt-base 2ba4ca90a && (cd wt-base && npm ci && npm run build)
# mock OpenAI 服务器：第一轮对 `agent` 返回一个 tool_call，其余返回纯文本
# 把守护进程指向它，并通过 SSE 驱动一个会话：
OPENAI_BASE_URL=http://127.0.0.1:8771/v1 OPENAI_API_KEY=sk-mock OPENAI_MODEL=mock-model \
QWEN_HOME=<隔离目录> node wt-<pr|base>/packages/cli/dist/index.js serve --port 0 --workspace <ws> --token T
# DaemonClient：createOrAttachSession → subscribeEvents → promptNonBlocking("launch an agent …")
# 观察：守护进程 stderr 出现 `kind:"agent"` 的 `invalid_union`；SSE 流中 tool_call 帧缺失（PR）/存在（base）
在 Linux、Node v22 上本地验证；tmux 隔离的守护进程 + mock；确定性桩模型。附带说明：两个构建里都出现了无害的 craft/drainMidTurnQueue"method not found"（版本错配，与本结论无关）。
Collaborator
Author
|
@wenshao — you're right, and I was wrong to push back earlier. I reproduced the mechanism in the code: the daemon builds One additional finding that rules out your option 1: I checked the published SDK line and no version carries Given that, emitting
Fix incoming on this branch; I'll follow up here when it's pushed. Thanks for the thorough daemon-level verification — it caught a real one. |
…' kind wenshao's daemon-level A/B verification showed emitting kind:'agent' over ACP is a regression: the daemon's ClientSideConnection Zod-validates every session/update + session/request_permission from the qwen --acp child before SSE fan-out, and @agentclientprotocol/sdk has no 'agent' ToolKind (verified through 0.25.1), so the frame is rejected (invalid_union) and dropped — the agent tool_call + permission dialog that previously reached SSE clients as kind:'other' now never arrive. Shrink to a no-regression safe version: - ToolCallEmitter maps the internal Kind.Agent to 'other' on the wire (drops the 'agent' as ToolKind cast that only fooled tsc, not the runtime schema). - Revert the wire/UI/protocol 'agent' additions that depended on a value the protocol can't carry: PermissionDrawer/labelUtils kind branches, web-shell DaemonMessageToolKind + inferToolKind, Java SDK schema/enum, export allowlist. Kind.Agent stays in core as the internal tool category. The dedicated agent permission dialog will be delivered via _meta.toolName (which already rides the validated wire) in a follow-up PR, not via a protocol kind.
doudouOUC
changed the title
Collaborator
Author
|
Fix pushed in
This restores the pre-PR The dedicated "Launch this agent?" dialog is deferred to a follow-up that carries agent-ness over |
Collaborator
🔁 Re-verification — the title now says "no-regression", but the code is unchanged; the regression still stands ❌Follow-up to my first verification (FAIL — Verdict: ❌ still FAIL — blocking regression unchanged. The title describes the fix I recommended (option 2), but the code wasn't updated to do it. 1. The wire schema still rejects
|
agent tool's session/request_permission (toolCall.kind) |
reaches SSE client? | daemon invalid_union |
|
|---|---|---|---|
BEFORE (main, 'other') |
kind:"other" |
✅ delivered (drawer can open) | 0 |
AFTER (this PR, 'agent') |
kind:"agent" |
❌ rejected at the ACP boundary | 1 |
AFTER — the daemon rejects it before fan-out (verbatim daemon stderr):
Error handling request {
method: 'session/request_permission',
params: { toolCall: { toolCallId: 'call_agent_1', status: 'pending',
title: 'probe agent kind', kind: 'agent', … } } }
{ code: -32603, … code: 'invalid_union', message: 'Invalid input' }
So the agent tool's permission request is dropped at the daemon and the SSE client (web-shell / webui) never receives it — the "Launch this agent?" drawer never opens. BEFORE, the same request arrives carrying kind:"other". The only variable between the two runs is the three Kind.Agent lines.
3. What needs to change to match the title
The title promises "keep ACP wire on 'other'", which is exactly my prior option 2 — but the code still emits 'agent'. To actually deliver no-regression, do one of:
- Map the wire value to
'other'until the negotiated SDK advertisesagent:[Kind.Agent]: 'other'inKIND_MAP(keep the internalKind.Agent+ all the client-side'agent'UI handling), and drop theas ToolKindcast — it's what hides this mismatch fromtsc; or - Bump
@agentclientprotocol/sdkto a version whoseToolKindincludesagent(and regenerate the JSON schema) in this PR.
As it stands, the producer emits a value the pinned wire schema rejects, so every qwen serve client (the default --http-bridge path) regresses for the agent tool. The consumer-side additions (PermissionDrawer, labelUtils, web-shell, Java SDK) are correct and should stay — they light up the moment the value can be delivered.
Incidental (both builds, unrelated): a benign
craft/drainMidTurnQueue-32601 Method not foundappears from version skew — not part of this finding.
🇨🇳 中文版（点击展开）
🔁 复核 —— 标题改成了"no-regression"，但代码没变；回归依旧存在 ❌
承接我的首次验证（FAIL —— kind:'agent' 在守护进程的 ACP 边界被拒、帧被丢弃）。此后 PR 标题改成了 "fix(acp): add internal Kind.Agent, keep ACP wire on 'other' (no-regression)"——但 head SHA 仍是 28464d71f，git diff 28464d71f <当前 head> 为空。"keep ACP wire on 'other'" 这一说法并未在代码中实现：ToolCallEmitter.ts 仍是 [Kind.Agent]: 'agent' as ToolKind。在 Linux 6.12 / Node v22.22.2 上复核，回归与之前完全一致地复现。
结论：❌ 仍然 FAIL —— 阻断性回归未变。 标题描述的正是我建议的修复（方案 2），但代码没有照做。
1. 线协议 schema 仍然拒绝 'agent'（针对真实固定版 SDK 的确定性验证）
直接运行守护进程的同一套校验 —— @agentclientprotocol/sdk@0.14.1 的 zToolKind：
zToolKind.safeParse("other") -> OK ✅
zToolKind.safeParse("agent") -> REJECTED ❌ (invalid_union)
固定版 SDK 的 ToolKind = read | edit | delete | move | search | execute | think | fetch | switch_mode | other——没有 agent。本 PR 未升级该 SDK，也没有线协议层的降级，因此 KIND_MAP[Kind.Agent] = 'agent' 产出一个会被守护进程拒绝的值。
2. 真实守护进程 A/B（真实 qwen serve，tmux）—— 在当前代码上重新复现
当前 main（0db327317）即等效父提交（agent 工具 = Kind.Other → 'other'）；本 PR 恰好新增三行（Kind.Agent 枚举 + agent.ts 的 super 调用 + KIND_MAP）。单 worktree A/B，mock OpenAI 强制触发一次 agent 工具调用，真实守护进程 + SSE：
agent 工具的 session/request_permission（toolCall.kind） |
是否到达 SSE 客户端？ | 守护进程 invalid_union |
|
|---|---|---|---|
BEFORE（main，'other'） |
kind:"other" |
✅ 下发（抽屉可打开） | 0 |
AFTER（本 PR，'agent'） |
kind:"agent" |
❌ 在 ACP 边界被拒 | 1 |
AFTER —— 守护进程在扇出前拒绝（守护进程 stderr 原文）：
Error handling request {
method: 'session/request_permission',
params: { toolCall: { toolCallId: 'call_agent_1', status: 'pending',
title: 'probe agent kind', kind: 'agent', … } } }
{ code: -32603, … code: 'invalid_union', message: 'Invalid input' }
即 agent 工具的权限请求在守护进程处被丢弃，SSE 客户端（web-shell / webui）根本收不到——"Launch this agent?" 抽屉永远打不开。BEFORE 时，同一请求以 kind:"other" 正常到达。两次运行唯一的差异就是那三行 Kind.Agent。
3. 要让代码匹配标题，需要做的改动
标题承诺 "keep ACP wire on 'other'"，这正是我此前的方案 2——但代码仍发 'agent'。要真正做到无回归，二选一：
- 把线上值映射为
'other'，直到协商到的 SDK 声明agent为止：在KIND_MAP里写[Kind.Agent]: 'other'（保留内部Kind.Agent与全部客户端'agent'UI 处理），并去掉as ToolKind断言——正是它把这个不匹配对tsc隐藏了；或 - 升级
@agentclientprotocol/sdk到ToolKind含agent的版本（并重新生成 JSON schema）。
当前状态下，生产方发出了被固定线协议 schema 拒绝的值，所以每个 qwen serve 客户端（默认 --http-bridge 路径）的 agent 工具都会回归。消费方新增（PermissionDrawer、labelUtils、web-shell、Java SDK）是正确的，应保留——一旦该值可被下发就会立即生效。
附带（两个构建都有、无关）：版本错配导致的无害
craft/drainMidTurnQueue-32601 Method not found——与本结论无关。
Collaborator
Author
|
You're completely right, and the root cause is on me: my fix commits never reached the PR branch. My local branch's upstream was misconfigured (pointed at a different branch name), so every Now actually pushed — PR head is One deliberate divergence from your rec to flag: I also reverted the consumer-side |
qqqys
approved these changes
Jun 14, 2026
qqqys
left a comment
qqqys
left a comment
Collaborator
There was a problem hiding this comment.
Critical ACP wire regression is resolved in 2d0c997: internal Kind.Agent now maps to the protocol-valid 'other' wire kind, the invalid 'agent' ToolKind additions were removed, and the required CI checks are green. No new critical issues found in this pass.
Collaborator
✅ Re-verification — head
|
toolCall.kind on the wire |
reaches SSE client? | daemon invalid_union |
|
|---|---|---|---|
my FAIL run ('agent') |
kind:"agent" |
❌ rejected at the boundary | 1 |
this head ('other') |
kind:"other" |
✅ delivered (3 permission frames, drawer can open) | 0 |
The fixed run is byte-for-byte the BEFORE/main behavior (permission request delivered with kind:"other", zero rejections in daemon stderr) — i.e. no regression.
Tests & static
ToolCallEmitter.test.ts47/47;agent.test.ts+coreToolScheduler.test.ts302/302.- The new assertion
expect(emitter.mapToolKind(Kind.Agent)).toBe('other')is load-bearing: mutatingKIND_MAP[Kind.Agent]back to'agent'→mapToolKind > should map all Kind values correctlyfails. - Deterministic against the real pinned SDK:
zToolKind.safeParse('other') → OK,'agent' → REJECTED— so the new wire value is protocol-valid.
Scope note (non-blocking, just so the merge is informed)
Because the wire now emits 'other' (same as before) and the consumer 'agent' branches were reverted, this PR is wire-neutral: ACP/SSE clients see the agent tool exactly as they do today. Its net effect is the internal Kind.Agent scaffolding — there's no user-visible categorization change yet (I grepped: nothing off-wire consumes Kind.Agent in this PR). The actual "Launch this agent?" UI is the deferred follow-up you described (keying on _meta.toolName, which is already on the validated wire). That's a reasonable split — just flagging that the PR title's "improve UI categorization" lands in the follow-up, not here. The follow-up is where I'd re-verify the drawer end-to-end.
Thanks for the transparent root-cause on the push mishap — and the SHA-advanced confirmation this time made re-verification straightforward.
🇨🇳 中文版（点击展开）
✅ 复核 —— head 2d0c997a1：线协议修复这次真的进分支了；回归已解决
承接我的上次复核（标题写了"no-regression"，但当时 head 与我最初 FAIL 的提交字节一致）。你之后真正推送了——head 现在是 2d0c997a1——并确认之前分支确实没动（upstream 配置错误）。我在 Linux 6.12 / Node v22.22.2 上复核了新 head（tmux 下真实 qwen serve 守护进程 + 固定版 SDK 校验 + 改动的测试套件）。
结论：✅ 已修复 —— 守护进程路径的回归已解决。建议合并（附一条范围说明，便于你清楚合的是什么）。
修复内容（相对 merge-base 的真实 PR diff = 6 文件，+15/−3）
ToolCallEmitter现在映射[Kind.Agent]: 'other'，且'agent' as ToolKind断言已删除——即我建议的无回归方案。那段 8 行注释正确地说明了原因（守护进程的ClientSideConnection在扇出到 SSE 前会对每个session/update/session/request_permission做 Zod 校验）。- 内部
Kind.Agent保留（枚举 +agent.ts声明它）；内部agentTool.kind === 'agent'，线上为'other'。 - 消费侧的
'agent'分支（PermissionDrawer、labelUtils、web-shell、Java SDK、normalize.ts）已还原为main——已核对字节一致——因此不存在依赖协议无法承载的kind的死代码。@agentclientprotocol/sdk未升级（仍为0.14.1）。
真实守护进程 —— 回归消失（真实 qwen serve，mock 强制触发 agent 工具调用）
与我 FAIL 报告相同的 A/B。agent 工具的 session/request_permission 现在能通过 ACP 边界并到达 SSE 客户端：
线上 toolCall.kind |
是否到达 SSE 客户端？ | 守护进程 invalid_union |
|
|---|---|---|---|
我的 FAIL 运行（'agent'） |
kind:"agent" |
❌ 边界处被拒 | 1 |
本 head（'other'） |
kind:"other" |
✅ 已下发（3 个权限帧，抽屉可打开） | 0 |
修复后的运行与 BEFORE/main 行为逐字节一致（权限请求以 kind:"other" 下发，守护进程 stderr 零拒绝）——即无回归。
测试 & 静态
ToolCallEmitter.test.ts47/47；agent.test.ts+coreToolScheduler.test.ts302/302。- 新断言
expect(emitter.mapToolKind(Kind.Agent)).toBe('other')有约束力：把KIND_MAP[Kind.Agent]改回'agent'→mapToolKind > should map all Kind values correctly失败。 - 针对真实固定版 SDK 的确定性校验：
zToolKind.safeParse('other') → OK、'agent' → REJECTED——所以新的线上值是协议合法的。
范围说明（不阻塞，仅供知情合并）
由于线上现在发 'other'（与之前相同）且消费侧 'agent' 分支已还原，本 PR 是线协议中性的：ACP/SSE 客户端看到的 agent 工具与今天完全一样。其净效果是内部 Kind.Agent 脚手架——本 PR 暂无用户可见的分类变化（我 grep 过：本 PR 没有任何线下游消费 Kind.Agent）。真正的 "Launch this agent?" UI 是你所说的后续 follow-up（基于 _meta.toolName，它已在校验后的线协议里）。这个拆分是合理的——只是提醒：标题里的 "improve UI categorization" 落在 follow-up，而非本 PR。抽屉的端到端验证我会放到 follow-up 再做。
感谢你对推送失误的坦诚根因说明——这次 SHA 确实前进了，复核很顺畅。
Collaborator
Author
|
Thanks for the thorough re-verification and the merge rec. Agreed on the scope: this PR is wire-neutral internal- |
Collaborator
|
@qwen-code /triage |
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required sections present including bilingual 中文说明. On direction: adding an internal On approach: this is about as minimal as it gets — 6 files, +15/−3, purely internal scaffolding. The wire stays on The review history here is notable: @wenshao caught a real regression in an earlier state of this PR (emitting Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必需章节齐全，包括双语中文说明。 方向：为子代理工具添加内部 方案：已经是最小化了——6 个文件，+15/−3，纯内部脚手架。线上保持 审查历史值得关注：@wenshao 在此 PR 的早期状态中捕获了一个真实的回归（在线上发送 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewThe diff is tight and correct. Six files, each change does exactly one thing:
No correctness issues, no security concerns, no AGENTS.md violations. The change follows project conventions (ESM, strict TS, collocated tests). Test ResultsTypecheck: clean ✅ Smoke Test (tmux, dev build with PR applied)CLI starts and responds correctly. The model lists "subagents" among available tools — confirming the agent tool (which now internally declares Since this PR is wire-neutral (the ACP wire still emits 中文说明代码审查Diff 紧凑且正确。6 个文件，每个改动只做一件事：
无正确性问题，无安全隐患，无 AGENTS.md 违规。改动遵循项目规范（ESM、严格 TS、测试与源码并列）。 测试结果类型检查：通过 ✅ 冒烟测试（tmux，应用 PR 后的 dev 构建）CLI 正常启动和响应。模型列出了"subagents"作为可用工具——确认了 agent 工具（现在内部声明 由于本 PR 是线协议中性的（ACP 线上仍发 — Qwen Code · qwen3.7-max |
Collaborator
ReflectionStepping back: this PR went through a genuinely impressive review cycle. @wenshao's daemon-level A/B testing caught a real regression that unit tests couldn't see (Zod validation at the ACP boundary is invisible to in-process tests), and @doudouOUC responded with a clean fix that maps My independent take: if I were solving "give the agent tool a distinct internal category," I'd do exactly this — add the enum value, declare it in the tool, map it to a valid wire value, and add a load-bearing test. The 8-line comment in The one reservation I flagged in Stage 1 — that All 349 tests pass. Typecheck and lint are clean. The smoke test confirms the CLI works. The wire behavior is identical to pre-PR Verdict: approve. ✅ This is clean internal scaffolding with a clear follow-up plan, a load-bearing test, and thorough review-cycle evidence that the regression is resolved. Shipping it. 中文说明反思退一步看：这个 PR 经历了令人印象深刻的审查周期。@wenshao 的守护进程级 A/B 测试捕获了单元测试看不到的真实回归（ACP 边界的 Zod 校验对进程内测试不可见），@doudouOUC 以干净的修复做出了回应——在线上把 我的独立判断：如果我要解决"给 agent 工具一个独立的内部分类"，我会做完全一样的事——添加枚举值、在工具中声明它、映射到合法的线上值，并添加有约束力的测试。 我在阶段 1 提出的一个保留意见—— 349 个测试全部通过。类型检查和 lint 均干净。冒烟测试确认 CLI 正常工作。线上行为与 PR 前的 结论：批准。 ✅ 这是干净的内部分类脚手架，有清晰的后续计划、有约束力的测试，以及充分的审查周期证据证明回归已解决。可以合并。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jun 14, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
approved these changes
Jun 14, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — glm-5.2 via Qwen Code /review
Collaborator
Author
|
Follow-up is up: #5105 — delivers the "Launch this agent?" prompt via |
wenshao
pushed a commit
that referenced
this pull request
Jun 14, 2026
…ow-up to #5085) (#5105) * feat(acp): carry _meta.toolName on permission frame; agent drawer (vscode) WIP: producer mirrors _meta.toolName onto session/request_permission; webui PermissionDrawer + vscode webview map it to render 'Launch this agent?' for the Agent tool without a protocol kind. Daemon/web-shell surface + tests follow. * feat(web-shell): dedicated agent permission prompt via _meta.toolName Thread the canonical tool name from the permission frame's _meta.toolName through web-shell's PermissionRequest so ToolApproval renders 'Launch this agent?' for the Agent tool, mirroring the vscode PermissionDrawer. Add tests for the toolName extraction and the agent drawer title. * fix(acp): mirror _meta.toolName on second producer path + address review Address @wenshao's review on #5105: - [Critical] SubAgentTracker's approval handler builds its own RequestPermissionRequest (the second producer path, for nested sub-agent tool calls) and was missing `_meta: { toolName }`. Session.ts adds it on the primary path; mirror it here so nested agents (and any future tool relying on _meta.toolName for specialized UI) don't fall back to the generic prompt. Locked with a _meta assertion in the approval test. - Dedupe the three hardcoded 'agent' string matches behind a single shared `AGENT_TOOL_NAME` / `isAgentTool` in @qwen-code/webui (re-exported via daemon-react-sdk, same pattern as DAEMON_APPROVAL_MODES), consumed by PermissionDrawer (webui) and ToolApproval (web-shell). - Move the agent check to the top of PermissionDrawer.getTitle() so it wins over kind-based checks, matching ToolApproval's isAgent-first ordering across the two surfaces. - Extract the _meta.toolName lifting logic in useWebViewMessages into a testable `liftToolNameFromMeta` helper and cover the three cases wenshao flagged: lift onto toolName, preserve a pre-existing toolName, no-op when _meta is absent (plus undefined-toolCall guard). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
doudouOUC
added a commit
that referenced
this pull request
Jun 15, 2026
…ssion) (#5085) * feat(core): add Kind.Agent for Agent tool to improve UI categorization The Agent tool was using Kind.Other as a catch-all, causing WebUI permission dialogs to show generic titles and descriptions. Adding a dedicated Kind.Agent value enables agent-specific UI rendering in permission drawers, tool labels, and export normalization. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * test(webui): lock agent label mapping in labelUtils test Address wenshao review on PR #5085 — add assertion that getToolDisplayLabel({ kind: 'agent' }) returns 'Agent', matching the sibling task/skill cases. * test(cli): cover agent kind in export normalization + document acp-sdk cast Address wenshao review on PR #5085: - Add normalize.test.ts case asserting an agent-kind tool call is preserved as 'agent' through normalizeSessionData. - Add TODO(acp-sdk) comment on the KIND_MAP 'agent' cast explaining why we emit 'agent' (webui SSE consumer) rather than mapping to 'other'. * fix(acp): map Kind.Agent to 'other' on the wire; drop unusable 'agent' kind wenshao's daemon-level A/B verification showed emitting kind:'agent' over ACP is a regression: the daemon's ClientSideConnection Zod-validates every session/update + session/request_permission from the qwen --acp child before SSE fan-out, and @agentclientprotocol/sdk has no 'agent' ToolKind (verified through 0.25.1), so the frame is rejected (invalid_union) and dropped — the agent tool_call + permission dialog that previously reached SSE clients as kind:'other' now never arrive. Shrink to a no-regression safe version: - ToolCallEmitter maps the internal Kind.Agent to 'other' on the wire (drops the 'agent' as ToolKind cast that only fooled tsc, not the runtime schema). - Revert the wire/UI/protocol 'agent' additions that depended on a value the protocol can't carry: PermissionDrawer/labelUtils kind branches, web-shell DaemonMessageToolKind + inferToolKind, Java SDK schema/enum, export allowlist. Kind.Agent stays in core as the internal tool category. The dedicated agent permission dialog will be delivered via _meta.toolName (which already rides the validated wire) in a follow-up PR, not via a protocol kind.
doudouOUC
added a commit
that referenced
this pull request
Jun 15, 2026
…ow-up to #5085) (#5105) * feat(acp): carry _meta.toolName on permission frame; agent drawer (vscode) WIP: producer mirrors _meta.toolName onto session/request_permission; webui PermissionDrawer + vscode webview map it to render 'Launch this agent?' for the Agent tool without a protocol kind. Daemon/web-shell surface + tests follow. * feat(web-shell): dedicated agent permission prompt via _meta.toolName Thread the canonical tool name from the permission frame's _meta.toolName through web-shell's PermissionRequest so ToolApproval renders 'Launch this agent?' for the Agent tool, mirroring the vscode PermissionDrawer. Add tests for the toolName extraction and the agent drawer title. * fix(acp): mirror _meta.toolName on second producer path + address review Address @wenshao's review on #5105: - [Critical] SubAgentTracker's approval handler builds its own RequestPermissionRequest (the second producer path, for nested sub-agent tool calls) and was missing `_meta: { toolName }`. Session.ts adds it on the primary path; mirror it here so nested agents (and any future tool relying on _meta.toolName for specialized UI) don't fall back to the generic prompt. Locked with a _meta assertion in the approval test. - Dedupe the three hardcoded 'agent' string matches behind a single shared `AGENT_TOOL_NAME` / `isAgentTool` in @qwen-code/webui (re-exported via daemon-react-sdk, same pattern as DAEMON_APPROVAL_MODES), consumed by PermissionDrawer (webui) and ToolApproval (web-shell). - Move the agent check to the top of PermissionDrawer.getTitle() so it wins over kind-based checks, matching ToolApproval's isAgent-first ordering across the two surfaces. - Extract the _meta.toolName lifting logic in useWebViewMessages into a testable `liftToolNameFromMeta` helper and cover the three cases wenshao flagged: lift onto toolName, preserve a pre-existing toolName, no-op when _meta is absent (plus undefined-toolCall guard). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Adds a
Kind.Agentvalue to the core toolKindenum and has the Agent (sub-agent) tool report it instead of the catch-allKind.Other, giving the agent tool a distinct internal category. On the ACP wire,Kind.Agentis mapped to'other'inToolCallEmitter, because the Agent Client Protocol defines noagentToolKind. The earlier consumer-side / protocol-side'agent'additions (WebUIPermissionDrawer/labelUtils, web-shell tool-kind types, Java SDK schema/enum, export allowlist) have been reverted — they depended on a wire value the protocol can't carry.Why it's needed
The original goal was to give sub-agent permission dialogs in the daemon WebUI a dedicated "Launch this agent?" treatment instead of the generic
kind:"other"dialog. A reviewer's daemon-level A/B verification (thanks @wenshao) showed that emittingkind:'agent'over ACP is actually a regression: the daemon'sClientSideConnection(packages/acp-bridge/src/bridge.ts:1199) Zod-validates everysession/updateandsession/request_permissionit receives from theqwen --acpchild before fanning out to SSE clients.@agentclientprotocol/sdkhas noagentToolKind(verified through the latest0.25.1), so the frame failsinvalid_unionand is dropped — the agenttool_calland permission dialog that previously reached SSE clients askind:'other'stop arriving entirely. The'agent' as ToolKindcast silencedtscbut not the runtime schema.This PR therefore steps back to a no-regression state: keep the internal category, but emit a protocol-valid kind on the wire. The dedicated agent permission dialog will be delivered separately via
_meta.toolName(which already rides the validated wire) in a follow-up PR, rather than via a protocol kind that doesn't exist.Reviewer Test Plan
How to verify
mapToolKind(Kind.Agent)returns'other'(not'agent'), while the Agent tool's owntool.kindisKind.Agentinternally.'agent'tool-kind branches (reverted to baseline), so nothing depends on a wire value the ACP schema rejects.qwen servedaemon with a stub model that forces anagenttool call; confirm thetool_call/permission frames are delivered (askind:'other') with noinvalid_unionin the daemon stderr.Evidence (Before & After)
invalid_uniononkind:'agent'; SSE clients receivetool_call ×0+ an orphanedtool_call_update; permission dialog never opens.kind:'other'on the wire, frames delivered, behavior matches pre-PRmain. No user-visible change yet — the dedicated dialog is deferred to the follow-up.Unit-test evidence:
Tested on
Environment (optional)
Local: per-package
vitest run. wenshao's daemon A/B was on Linux / Node v22.Risk & Scope
kind:'other') and removes the regression.Kind.Agentremains an inert internal category (mapped to'other'at every external boundary) until the follow-up wires the_meta.toolName-based UI._meta.toolName-based follow-up). Windows/Linux verified via CI only.MUTATOR_KINDS/CONCURRENCY_SAFE_KINDSunchanged; agent concurrency is keyed by tool name, not kind.Linked Issues
N/A
中文说明
这个 PR 做了什么
在 core 的
Kind枚举里新增Kind.Agent，并让 Agent（子代理）工具上报它而非兜底的Kind.Other，给 agent 工具一个独立的内部分类。在 ACP 线协议上，ToolCallEmitter把Kind.Agent映射成'other'，因为 Agent Client Protocol 根本没有agent这个ToolKind。此前加在消费侧/协议侧的'agent'（WebUIPermissionDrawer/labelUtils、web-shell 的 tool-kind 类型、Java SDK schema/枚举、导出白名单）已全部回退——它们依赖一个线协议无法承载的值。为什么需要
最初目标是让守护进程 WebUI 里子代理的权限弹窗有专属的"Launch this agent?"展示，而不是通用的
kind:"other"。但 reviewer 的守护进程级 A/B 验证（感谢 @wenshao）表明，在 ACP 上发kind:'agent'其实是回归：守护进程的ClientSideConnection（packages/acp-bridge/src/bridge.ts:1199）会在扇出给 SSE 客户端之前，对从qwen --acp子进程收到的每个session/update和session/request_permission做 Zod 校验。@agentclientprotocol/sdk没有agent这个ToolKind（查到最新0.25.1仍无），于是帧触发invalid_union被丢弃——原本以kind:'other'能到达 SSE 客户端的 agenttool_call和权限弹窗，现在完全收不到。'agent' as ToolKind断言只骗过了tsc，骗不过运行时 schema。因此本 PR 退回到无回归状态：保留内部分类，但在线协议上发一个合法的 kind。专属 agent 权限弹窗将在后续 PR 中通过
_meta.toolName（已在被校验的线协议上传输）单独实现，而不是通过一个并不存在的协议 kind。风险与范围
kind:'other'），消除回归。Kind.Agent作为内部分类暂时是惰性的（在所有外部边界都映射成'other'），直到后续 PR 接上基于_meta.toolName的 UI。_meta.toolName方案）；Windows/Linux 仅经 CI。MUTATOR_KINDS/CONCURRENCY_SAFE_KINDS不变；agent 并发按工具名而非 kind 判定。关联 Issue
N/A
🤖 Generated with Qwen Code