Skip to content

fix(channels): suppress nested subagent output - #6696

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
qqqys:codex/issue-6694-suppress-subagent-output
Jul 11, 2026
Merged

fix(channels): suppress nested subagent output#6696
wenshao merged 1 commit into
QwenLM:mainfrom
qqqys:codex/issue-6694-suppress-subagent-output

Conversation

@qqqys

@qqqys qqqys commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR prevents channel delivery from collecting assistant message chunks that belong to nested subagents. Root-agent chunks continue through the existing response-boundary handling, while nested Agent/Explore reports remain available as ACP session updates for clients that render subagent activity.

Why it's needed

Nested subagent output is emitted as an assistant message chunk with parent-tool metadata. Channel bridges previously treated those chunks as root-agent response text, so an intermediate research report, including absolute local paths, could be delivered to DingTalk before the root agent's final answer. Filtering at the channel boundary preserves ACP observability without exposing intermediate subagent content to messaging platforms.

Reviewer Test Plan

How to verify

Exercise both a direct ACP-backed channel session and a daemon-managed channel session with an Agent/Explore subagent that emits a research report before the root agent responds. Confirm the completed channel response contains only the root agent's final answer. Also confirm ordinary root-agent chunks are still delivered and the raw ACP session update for nested activity remains available to non-channel clients.

Evidence (Before & After)

Before: the focused regression tests returned Nested research report.Final answer.. After: both paths return only Final answer., and 59 focused bridge tests pass.

Tested on

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

Environment (optional)

Node.js workspace after npm install; focused Vitest tests plus full build and typecheck.

Risk & Scope

  • Main risk or tradeoff: Channel adapters no longer surface nested subagent prose, by design; root-agent output and raw ACP updates are unchanged.
  • Not validated / out of scope: Live DingTalk delivery with production credentials.
  • Breaking changes / migration notes: None.

Linked Issues

Fixes #6694

中文说明

这个 PR 做了什么

这个 PR 阻止 channel 投递收集属于嵌套子代理的 assistant message chunk。根代理文本仍沿用现有 response boundary 处理;嵌套 Agent/Explore 报告仍作为 ACP session update 保留,供需要展示子代理活动的客户端使用。

为什么需要

嵌套子代理输出会以带有父工具元数据的 assistant message chunk 发出。此前 channel bridge 会把这些 chunk 当作根代理回复文本,因此包含绝对本地路径的中间研究报告可能在根代理最终回复前被投递到钉钉。把过滤放在 channel 边界既保留 ACP 可观测性,又避免向消息平台暴露子代理中间内容。

Reviewer 测试计划

如何验证

分别通过直接 ACP channel session 和 daemon 管理的 channel session 触发 Agent/Explore 子代理,使其在根代理回复前输出研究报告。确认 channel 完整回复只包含根代理最终答案;同时确认普通根代理 chunk 仍正常投递,嵌套活动的原始 ACP session update 仍可供非 channel 客户端使用。

证据(修复前后)

修复前:聚焦回归测试返回 Nested research report.Final answer.。修复后:两条路径都只返回 Final answer.,59 个 bridge 聚焦测试全部通过。

测试平台

OS 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境(可选)

执行 npm install 后的 Node.js 工作区;运行了聚焦 Vitest 测试、完整 build 和 typecheck。

风险与范围

  • 主要风险或权衡:channel adapter 不再展示嵌套子代理文本,这是预期行为;根代理输出和原始 ACP update 不变。
  • 未验证 / 不在范围内:需要生产凭据的真实钉钉投递。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

Fixes #6694

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: This is an observed bug, not theoretical. Issue #6694 documents DingTalk channel replies exposing nested subagent intermediate reports — including absolute local file paths. There's a clear description of the observed behavior, evidence (exposed text matched the Explore tool result), and a temporary mitigation (denying agent permissions). The PR also references the prior fix in #6615 whose response-boundary handling didn't cover this nested path.

Direction: Clearly aligned — leaking intermediate subagent output (with local paths) to messaging platforms is a real bug with security implications. This is squarely within the channel system's scope.

Size: Not applicable — packages/channels/ is not a core module path. 8 production lines, 86 test lines. Very focused.

Approach: The fix is minimal and well-scoped. Both bridges get a 2-line guard that checks _meta.parentToolCallId to identify nested subagent chunks and skip them. The daemon bridge correctly reuses the existing isRecord helper; the ACP bridge uses an as cast consistent with surrounding code. Tests mirror the actual bug scenario. No scope creep.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:这是已观测到的 bug,不是理论性问题。Issue #6694 记录了钉钉 channel 回复中泄露了嵌套子代理的中间报告——包括绝对本地文件路径。有清晰的行为描述、证据(泄露文本与 Explore 工具结果一致)和临时缓解措施(禁用 agent 权限)。PR 还引用了 #6615 的先前修复,其 response-boundary 处理未覆盖此嵌套路径。

方向:明确对齐——向消息平台泄露子代理中间输出(包含本地路径)是一个有安全隐患的真实 bug,完全在 channel 系统范围内。

规模:不适用——packages/channels/ 不是核心模块路径。8 行生产代码,86 行测试。非常聚焦。

方案:修复方案最小且范围精准。两个 bridge 各加 2 行守卫代码,通过检查 _meta.parentToolCallId 识别嵌套子代理 chunk 并跳过。Daemon bridge 正确复用了已有的 isRecord helper;ACP bridge 的 as 类型转换与周围代码风格一致。测试覆盖了实际 bug 场景。无范围蔓延。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: the agent_message_chunk event in ACP carries _meta.parentToolCallId when it comes from a nested subagent. Both bridges should check this field early in the handler and skip the chunk if present. This is the same pattern already used in packages/webui/src/daemon/session/mappers.ts (line 363: if (meta?.['parentToolCallId'] !== undefined) return undefined).

The PR's approach matches this exactly. The fix is 2 production lines per bridge, placed at the top of the agent_message_chunk case before any text accumulation. Each bridge uses its local convention — AcpBridge uses an as cast (consistent with the surrounding code's record-casting style), DaemonChannelBridge uses the existing isRecord helper (defined at line 88 of the same file). No correctness issues, no security holes, no regressions.

No issues found. Clean, minimal, follows established patterns.

Testing

Build: npm run build ✅ | Typecheck: npm run typecheck

Focused tests (57 → 59 with PR): all pass. The two new regression tests directly reproduce the bug scenario — a nested subagent chunk followed by a root-agent chunk — and assert only the root text is delivered.

Live DingTalk channel reproduction was not possible (no credentials available in this environment). The unit tests mock the exact event sequence from issue #6694 and verify both bridge paths.

Before (main branch, without PR)

 ✓ src/DaemonChannelBridge.test.ts (32 tests) 53ms
 ✓ src/AcpBridge.test.ts (25 tests) 12031ms

 Test Files  2 passed (2)
      Tests  57 passed (57)

After (with PR applied)

 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > binds a daemon session and collects assistant chunks during prompt 5ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > passes approval mode to the session factory 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > drains daemon chunks queued with prompt completion 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > returns only the final turn text after daemon tool calls 1ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > excludes nested subagent text from the daemon response 1ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > returns only the final turn text after daemon auto-approved tool calls 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > treats daemon permission requests as turn boundaries 1ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > treats daemon plan updates as turn boundaries 1ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > rejects prompt and emits protocol error on turn_error 3ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > resolves the turn barrier when a session is cancelled during prompt drain 1ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > emits tool, thought, model, commands, and session lifecycle events 4ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > keeps available commands scoped per daemon session 3ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > surfaces command aliases (altNames) carried in the wire snapshot 2ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > drops a command whose altNames is a malformed (non-array) wire value 2ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > routes permission responses back through the owning daemon session 6ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > rejects malformed permission resolution outcomes 3ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > ignores permission resolution events from non-owning sessions 3ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > replaces duplicate daemon sessions and clears stale ownership state 3ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > rejects unknown sessions and concurrent prompts for one session 1ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > passes image prompt blocks and aborts prompts when a session dies 1ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > aborts in-flight prompts when the bridge stops 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > aborts in-flight prompts when cancelling a session 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > clears permission ownership when daemon permission responses fail 3ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > treats terminal stream frames and completion as session death 5ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > loads an existing daemon session and forwards cancel/model changes 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > rejects mismatched daemon session ids while loading 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > surfaces malformed daemon events through the error channel 2ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > listSessions returns empty array when no sessions are attached 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > listSessions returns attached sessions with hasActivePrompt status 2ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > listSessions excludes dropped sessions 1ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > listSessions shows hasActivePrompt false after cancelSession 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > listSessions returns empty after bridge stop 0ms
 ✓ src/DaemonChannelBridge.test.ts > DaemonChannelBridge > listSessions reflects session replacement with same ID 0ms
 ✓ src/AcpBridge.test.ts > AcpBridge > registers the channel loop MCP server once across concurrent calls 1ms
 ✓ src/AcpBridge.test.ts > AcpBridge > waits for pending channel loop MCP registration before creating a session 1ms
 ✓ src/AcpBridge.test.ts > AcpBridge > retries channel loop MCP registration when the runtime skips registration 1ms
 ✓ src/AcpBridge.test.ts > AcpBridge > sanitizes skipped channel loop MCP registration reasons 1ms
 ✓ src/AcpBridge.test.ts > AcpBridge > returns a synthetic payload ack for MCP notifications 1ms
 ✓ src/AcpBridge.test.ts > AcpBridge > handles mid-turn queue drain requests from the ACP child 0ms
 ✓ src/AcpBridge.test.ts > AcpBridge > returns only the final turn text after tool calls 0ms
 ✓ src/AcpBridge.test.ts > AcpBridge > excludes nested subagent text from the final response 0ms
 ✓ src/AcpBridge.test.ts > AcpBridge > returns only the final turn text after auto-approved tool calls 0ms
 ✓ src/AcpBridge.test.ts > AcpBridge > preserves text when tool calls are not pending 0ms
 ✓ src/AcpBridge.test.ts > AcpBridge > treats plan updates as turn boundaries for TodoWrite-only rounds 0ms
 ✓ src/AcpBridge.test.ts > AcpBridge > treats permission requests as turn boundaries 1ms
 ✓ src/AcpBridge.test.ts > AcpBridge > rejects channel loop tool calls when no handler matches the session 1ms
 ✓ src/AcpBridge.test.ts > AcpBridge > uses the only channel loop tool handler when canHandle is omitted 0ms
 ✓ src/AcpBridge.test.ts > AcpBridge > kills the ACP child when it reports a large event loop stall 1002ms
 ✓ src/AcpBridge.test.ts > AcpBridge > kills the ACP child when a stall line is coalesced with prior stderr 1003ms
 ✓ src/AcpBridge.test.ts > AcpBridge > does not kill the ACP child for a small event loop stall warning 1002ms
 ✓ src/AcpBridge.test.ts > AcpBridge > ignores non-perf stderr that mentions an event loop stall 1002ms
 ✓ src/AcpBridge.test.ts > AcpBridge > does not kill the ACP child again after it is already killed 1001ms
 ✓ src/AcpBridge.test.ts > AcpBridge > relays ACP permission requests instead of auto-approving them 1003ms
 ✓ src/AcpBridge.test.ts > AcpBridge > allows permission request listeners to respond synchronously 1002ms
 ✓ src/AcpBridge.test.ts > AcpBridge > falls back to the tool call id for permission requests without a session id 1001ms
 ✓ src/AcpBridge.test.ts > AcpBridge > resolves matching pending permissions as cancelled when a session is cancelled 1002ms
 ✓ src/AcpBridge.test.ts > AcpBridge > resolves pending permissions as cancelled after the response timeout 1003ms
 ✓ src/AcpBridge.test.ts > AcpBridge > resolves pending permissions as cancelled when the ACP child exits 1002ms
 ✓ src/AcpBridge.test.ts > AcpBridge > resolves pending permissions as cancelled on stop 1002ms

 Test Files  2 passed (2)
      Tests  59 passed (59)
   Start at  04:15:15
   Duration  12.32s (transform 153ms, setup 0ms, collect 249ms, tests 12.09s, environment 0ms, prepare 128ms)

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

This is a clean, focused bugfix that does exactly what it says. The problem is real (issue #6694 documents DingTalk channel replies leaking subagent output including absolute local file paths), the fix is minimal (4 production lines across 2 bridges), and the approach follows an established pattern already used in the webui package's session mappers. The regression tests directly mock the bug scenario and pass on both bridge paths. Build and typecheck are clean.

Approving. ✅

中文说明

这是一个干净、聚焦的 bug 修复,完全符合 PR 描述。问题是真实的(issue #6694 记录了钉钉 channel 回复泄露子代理输出,包括绝对本地文件路径),修复方案最小(两个 bridge 共 4 行生产代码),方案遵循了 webui 包 session mappers 中已有的成熟模式。回归测试直接 mock 了 bug 场景,两条 bridge 路径均通过。Build 和 typecheck 均正常。

批准。✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

✅ Local validation report (maintainer)

I built and ran real tests against this PR's own checkout (b530c9a69). The fix works, the new tests are meaningful, there are no regressions, and build + typecheck are clean. It merges cleanly into current main.

Environment

  • macOS (Darwin 24.6.0) · Node v22.23.1 · npm 10.9.8
  • PR branch codex/issue-6694-suppress-subagent-output @ b530c9a69, run as the @qwen-code/channel-base Vitest project
  • GitHub reports the branch as MERGEABLE (it is a few commits behind main, but the 4 changed files don't conflict)

What the fix does

Both bridges now skip agent_message_chunk updates whose _meta.parentToolCallId is a string (i.e. nested Agent/Explore subagent output), so only the root agent's text is collected into the channel response. Crucially, the raw ACP sessionUpdate is still emitted after the switch (AcpBridge.ts:354, DaemonChannelBridge.ts:630), so subagent activity stays fully observable to non-channel ACP clients — exactly as the PR describes.

1) Focused bridge suite — all green

$ vitest run --project @qwen-code/channel-base AcpBridge.test.ts DaemonChannelBridge.test.ts

 ✓ src/DaemonChannelBridge.test.ts  (33 tests)   48ms
 ✓ src/AcpBridge.test.ts            (26 tests)   12056ms

 Test Files  2 passed (2)
      Tests  59 passed (59)
   Duration  12.38s

2) Regression proof — the new tests genuinely catch the bug

I reverted only the 8 production lines (kept the two new tests) to confirm they fail for the right reason, then restored:

# AFTER  (PR guard in place) — the two new regression tests
 ✓ AcpBridge > excludes nested subagent text from the final response
 ✓ DaemonChannelBridge > excludes nested subagent text from the daemon response
      Tests  2 passed | 664 skipped (666)

# BEFORE (guard reverted) — the same two tests now fail:
 × AcpBridge > excludes nested subagent text from the final response
     → expected 'Nested research report.Final answer.' to be 'Final answer.'
 × DaemonChannelBridge > excludes nested subagent text from the daemon response
     → expected 'Nested research report.Final answer.' to be 'Final answer.'
      Tests  2 failed | 664 skipped (666)

The leaked Nested research report. prefix is exactly the #6694 symptom (a nested subagent report reaching the channel reply). This mirrors the real-world report that leaked absolute local paths — the nested-chunk content is arbitrary; the mechanism is identical.

3) Full package sweep + build/typecheck — clean

$ vitest run --project @qwen-code/channel-base
      Tests  666 passed (666)     # 15 files, no regressions

$ tsc --build packages/channels/base/tsconfig.json
      exit 0                       # emitted dist contains the guard in both bridges

Channel payload — before vs after

Text delivered to the messaging platform
Before Nested research report.Final answer.
After Final answer.

Verdict

LGTM — safe to merge. The change is minimal (8 src lines, symmetric across both bridges), correctly scoped to the channel boundary, backed by focused regression tests that fail without it, and it preserves ACP observability. Only caveat, already noted in the PR: live DingTalk delivery with production credentials was not exercised — the fix operates purely at the ACP/channel chunk boundary, which is what these tests cover.

🇨🇳 中文说明(点击展开)

✅ 本地验证报告(维护者)

我基于该 PR 自身的检出(b530c9a69)做了真实的本地构建与测试。修复有效、新增测试确实能覆盖问题、无回归,构建与类型检查均通过。 可干净合并到当前 main

环境

  • macOS(Darwin 24.6.0)· Node v22.23.1 · npm 10.9.8
  • PR 分支 codex/issue-6694-suppress-subagent-output @ b530c9a69,以 @qwen-code/channel-base Vitest project 运行
  • GitHub 显示分支 可合并(MERGEABLE):虽落后 main 若干提交,但改动的 4 个文件无冲突

这个修复做了什么

两个 bridge 现在会跳过 _meta.parentToolCallId 为字符串的 agent_message_chunk(即嵌套 Agent/Explore 子代理输出),因此只有根代理文本会被收集进 channel 回复。关键点:原始 ACP sessionUpdate 仍在 switch 之后照常发出(AcpBridge.ts:354DaemonChannelBridge.ts:630),因此子代理活动对非 channel 的 ACP 客户端依然完全可见——与 PR 描述一致。

1)聚焦 bridge 测试——全部通过

$ vitest run --project @qwen-code/channel-base AcpBridge.test.ts DaemonChannelBridge.test.ts

 ✓ src/DaemonChannelBridge.test.ts  (33 tests)   48ms
 ✓ src/AcpBridge.test.ts            (26 tests)   12056ms

 Test Files  2 passed (2)
      Tests  59 passed (59)

2)回归验证——新测试确实能抓住这个 bug

只回退了那 8 行生产代码(保留新测试),确认测试因正确的原因失败,随后已还原:

# 修复后(保留 guard)——两个新回归测试
 ✓ AcpBridge > excludes nested subagent text from the final response
 ✓ DaemonChannelBridge > excludes nested subagent text from the daemon response
      Tests  2 passed | 664 skipped (666)

# 修复前(回退 guard)——同样两个测试失败:
 × 期望 'Nested research report.Final answer.' 等于 'Final answer.'
 × 期望 'Nested research report.Final answer.' 等于 'Final answer.'
      Tests  2 failed | 664 skipped (666)

泄漏出来的 Nested research report. 前缀正是 #6694 的症状(嵌套子代理报告进入了 channel 回复)。这与线上泄漏绝对本地路径的情况机理完全一致——嵌套内容本身是任意文本,机制相同。

3)整包测试 + 构建/类型检查——干净

$ vitest run --project @qwen-code/channel-base
      Tests  666 passed (666)     # 15 个文件,无回归

$ tsc --build packages/channels/base/tsconfig.json
      exit 0                       # 产物 dist 中两个 bridge 都包含该 guard

Channel 投递内容——修复前后对比

投递到消息平台的文本
修复前 Nested research report.Final answer.
修复后 Final answer.

结论

LGTM——可以合并。 改动很小(8 行生产代码,两个 bridge 对称处理),准确地落在 channel 边界上,有能在缺失时失败的聚焦回归测试保障,同时保留了 ACP 可观测性。唯一说明(PR 中已注明):未使用生产凭据做真实钉钉投递——但本修复完全作用于 ACP/channel chunk 边界,正是这些测试所覆盖的范围。

@wenshao
wenshao added this pull request to the merge queue Jul 11, 2026
Merged via the queue into QwenLM:main with commit 2428741 Jul 11, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

channels: suppress nested subagent output in DingTalk replies

3 participants