feat(acp): dedicated agent permission dialog via _meta.toolName (follow-up to #5085) - #5105
Merged
Conversation
…code) 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.
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.
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, test plan included. On direction: this is a solid follow-up to #5085. The core issue — ACP can't carry On approach: scope feels right — 16 files, +251/-0, purely additive, no protocol-level changes. The producer/consumer split is clean: one line on the primary producer ( Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有章节齐全，双语，包含测试计划。 方向：作为 #5085 的 follow-up，这是合理的。核心问题——ACP 因 Zod 校验无法携带 方案：范围合理——16 个文件，+251/-0，纯增量，不改协议。生产方/消费方划分清晰：主生产路径（ 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal (before reading diff): The problem is straightforward — consumers need to know which tool is requesting permission, but ACP's Findings (post-fix, head The implementation is clean across 16 files. The author addressed all items from @wenshao's review:
The No correctness bugs, security issues, or regressions found. Test ResultsUnit tests (all pass on Smoke test (tmux, dev build on main branch): CLI starts cleanly and responds to prompts. Agent permission dialog E2E: Not tested end-to-end in tmux here — triggering the Agent tool requires a model that decides to spawn a sub-agent, which can't be reliably forced with a simple prompt. However, @wenshao independently verified the full daemon/ACP end-to-end on the PR branch (real wire bytes through genuine 中文说明代码审查独立方案（读 diff 前）： 问题很直接——消费方需要知道哪个工具在请求权限，但 ACP 的 发现（修复后，head 实现跨 16 个文件都很干净。作者已处理 @wenshao 审查中的所有条目：
无正确性 bug、安全问题或回归。 测试结果单元测试全部通过（174 个测试）。Typecheck 干净。CLI 冒烟测试正常启动并响应。Agent 权限弹窗端到端由 @wenshao 独立在 PR 分支上完成验证（真实 ACP 线协议，通过 — Qwen Code · qwen3.7-max |
Collaborator
|
Stepping back: this is a clean, well-scoped follow-up that delivers exactly what #5085 deferred. The My independent proposal matched the PR's approach, which is a good sign — there's no simpler path being missed. The implementation is straightforward: one field on each producer path, one extraction in the adapter, one lift helper in the VS Code webview, two consumer checks with a shared predicate. No abstraction overhead, no speculative features. The author addressed every item from @wenshao's first-round review — the missing second producer path, the scattered The unit test suite is thorough — 174 tests passing across all affected packages, including new tests for both The deferred E2E gap from the original PR has been closed: @wenshao independently verified the full daemon/ACP end-to-end on the PR branch, capturing real wire bytes through the genuine If I had to maintain this in six months, I'd be fine — the change is isolated, the coupling point ( Approving. ✅ 中文说明退一步看：这是一个干净、范围合理的 follow-up，交付了 #5085 延后的功能。 我的独立方案与 PR 的方案一致，说明没有遗漏更简单的路径。实现很直接：每条生产路径一个字段，适配器一处提取，VS Code webview 一个 lift 辅助函数，两个消费端使用共享谓词检查。没有抽象开销，没有投机性功能。 作者已处理 @wenshao 首轮审查中的每一条——缺失的第二生产路径、散落的 单元测试充分——174 个测试跨所有受影响包全部通过。PermissionDrawer 测试正确验证 agent 专属提示仅在 原 PR 中延后的端到端缺口已由 @wenshao 独立补上：在 PR 分支上完成真实守护进程/ACP 端到端验证，通过 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously 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. ✅
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
|
Thanks for the PR! Template looks good ✓ On direction: This is a natural follow-up to #5085 — delivering the deferred "Launch this agent?" dialog via On approach: Scope is tight — 162 additions across 9 files, purely additive, no protocol-level changes. The producer (Session.ts mirrors Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 方向：作为 #5085 的 follow-up，通过 方案：范围紧凑——9 个文件 162 行新增，纯增量，不改协议。生产方（Session.ts 镜像 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
|
Qwen triage pipeline did not complete successfully. See workflow logs. |
wenshao
requested changes
Jun 14, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Critical] SubAgentTracker.ts:203-212 — the second producer path that creates requestPermission calls is missing _meta: { toolName } on the toolCall object. Session.ts:3311 correctly adds it, but SubAgentTracker.ts builds its own RequestPermissionRequest at line 203 without mirroring toolName. If a sub-agent invokes the agent tool (nested agents) or any future tool relying on _meta.toolName for specialized UI, consumers will fall back to the generic prompt.
toolCall: {
toolCallId: event.callId,
status: 'pending',
title,
content: buildPermissionRequestContent(fullConfirmationDetails),
locations,
kind,
rawInput: state?.args,
+ _meta: { toolName: event.name },
},— qwen3.7-max via Qwen Code /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)
Collaborator
Author
|
@wenshao thanks for the thorough review — addressed everything in c638756:
Tests: SubAgentTracker 24 ✓, Session 125 ✓, PermissionDrawer 2 ✓, transcriptAdapter 6 ✓, useWebViewMessages 17 ✓. typecheck clean across webui / web-shell / vscode. 🤖 Generated with Qwen Code |
Collaborator
|
Qwen Code review did not complete successfully: Qwen review aborted with an API error before posting comments. See workflow logs. |
Collaborator
✅ Local end-to-end verification — agent permission dialog over the real ACP wireI ran the daemon/ACP end-to-end that this PR deferred ("Daemon end-to-end deferred to reviewer re-verify"). Tested at PR head How I tested
Result — producer side (the deferred part) ✅The agent's // WITH PR (head c6387560e), real wire bytes:
"toolCall": { … "kind":"other", "rawInput":{…}, "_meta":{"toolName":"agent"} }
// WITHOUT PR (counterfactual: Session.ts _meta line reverted + rebundled):
"toolCall": { … "kind":"other", "rawInput":{…} } // no _metaSo the one-line mirror in
Result — consumer side + regression (unit, against the captured frame shape) ✅
The captured real frame is shape-identical to the adapter test's fixture, so the chain connects end to end: real producer → Caveat (harness, not the PR)My minimal client doesn't implement the VerdictThe deferred daemon/ACP end-to-end is confirmed working. The 中文说明✅ 本地端到端验证 —— 真实 ACP 线协议上的 agent 权限弹窗我补做了本 PR 延后的守护进程/ACP 端到端验证(PR 里写的"Daemon end-to-end deferred to reviewer re-verify")。基于 PR HEAD 测试方法
结果 —— 生产方(被延后的部分)✅agent 的 // 有本 PR(head c6387560e),真实线上字节:
"toolCall": { … "kind":"other", "rawInput":{…}, "_meta":{"toolName":"agent"} }
// 无本 PR(反事实:回退 Session.ts 那一行 + 重新 bundle):
"toolCall": { … "kind":"other", "rawInput":{…} } // 没有 _meta即:
结果 —— 消费方 + 回归(针对抓到的帧形状的单测)✅
抓到的真实帧与 adapter 单测的 fixture 形状一致,因此整条链路端到端打通:真实生产方 → 线上 说明(harness 局限,非本 PR 问题)最小客户端未实现 结论被延后的守护进程/ACP 端到端已确认可用: |
Collaborator
Author
|
@wenshao huge thanks for actually building the branch and capturing the real wire bytes through the genuine One note for when you circle back: the head you verified ( (The unrelated |
Collaborator
|
Qwen Code review did not complete successfully: Qwen review aborted with an API error before posting comments. See workflow logs. |
wenshao
approved these changes
Jun 14, 2026
Collaborator
|
@qwen-code /triage |
wenshao
reviewed
Jun 14, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ Downgraded from Approve to Comment: CI failing (review-pr bot — API error in prior run, not a code defect). All R1 feedback properly addressed in c638756: SubAgentTracker _meta mirroring, liftToolNameFromMeta extraction with tests, AGENT_TOOL_NAME constant, PermissionDrawer ordering. Typecheck clean, 41/41 tests pass. — qwen3.7-max via Qwen Code /review
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. ✅
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
Gives the Agent (sub-agent) tool a dedicated "Launch this agent?" permission prompt in both UI surfaces — the VS Code
PermissionDrawerand the daemon web-shellToolApproval— without relying on a protocol toolkind. The producer mirrors the canonical tool name onto thesession/request_permissionframe via_meta.toolName(thetool_callframe already carries it), and each consumer reads that to special-case the Agent tool's prompt.Why it's needed
Follow-up to #5085. That PR established that ACP defines no
agentToolKind(the daemon'sClientSideConnectionZod-validates and drops akind:'agent'frame), so it kept the wire on'other'and deferred the user-visible "Launch this agent?" dialog to a_meta.toolName-based approach._metais an open passthrough that already rides the validated wire (thetool_callframe ships_meta.toolName), so it can carry tool identity where a protocolkindcannot. This PR delivers that deferred dialog.Reviewer Test Plan
How to verify
PermissionDrawerrenders "Launch this agent?" only whentoolName === 'agent'; web-shellextractPendingPermissionliftstoolNamefromtoolCall._meta.toolName.Session.tsadds_meta: { toolName }to therequest_permissiontoolCall(mirrors the existingtool_callframe); existingrequestPermissionassertions useobjectContaining, so they still pass.qwen servewith a stub model that forces anagenttool call; the permission prompt should read "Launch this agent?" (web-shell) / "Launch this agent?" drawer (VS Code), with the agent's description as subtitle. (Deferred drawer re-verify offered by @wenshao on fix(acp): add internal Kind.Agent, keep ACP wire on 'other' (no-regression) #5085.)Evidence (Before & After)
Unit-test evidence:
Tested on
Environment (optional)
Local: per-package
vitest run+npm run typecheck. Daemon end-to-end deferred to reviewer re-verify.Risk & Scope
_meta.toolNameis additive on a passthrough field; nokind/wire change, so no ACP validation impact (unlike the original fix(acp): add internal Kind.Agent, keep ACP wire on 'other' (no-regression) #5085 regression). Consumers default to existing behavior when_meta.toolNameis absent.Linked Issues
Follow-up to #5085.
中文说明
这个 PR 做了什么
给 Agent（子代理）工具在两个 UI 面上都提供专属的"Launch this agent?"权限提示——VS Code 的
PermissionDrawer和守护进程 web-shell 的ToolApproval——且不依赖协议的工具kind。生产方把规范工具名通过_meta.toolName镜像到session/request_permission帧上（tool_call帧本就携带它），各消费方读取它来对 Agent 工具的提示做专门处理。为什么需要
#5085 的 follow-up。那个 PR 确认 ACP 没有
agent这个ToolKind（守护进程的ClientSideConnection会 Zod 校验并丢弃kind:'agent'帧），所以它把线协议保持在'other'，并把用户可见的"Launch this agent?"弹窗延后到基于_meta.toolName的方案。_meta是已在被校验的线协议上传输的开放透传字段（tool_call帧已携带_meta.toolName），因此能承载协议kind无法承载的工具身份。本 PR 交付这个被延后的弹窗。风险与范围
_meta.toolName是在透传字段上的新增；不改kind/线协议，因此无 ACP 校验影响（不同于 fix(acp): add internal Kind.Agent, keep ACP wire on 'other' (no-regression) #5085 的回归）。_meta.toolName缺失时消费方回退到既有行为。关联 Issue
#5085 的 follow-up。
🤖 Generated with Qwen Code