fix(daemon): isolate parallel subAgent text streams in transcript reducer#4689
fix(daemon): isolate parallel subAgent text streams in transcript reducer#4689doudouOUC wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes daemon-mode transcript corruption when multiple subagents stream text concurrently by propagating parentToolCallId through emission → normalization → transcript reduction, then routing assistant/thought deltas into per-parent “active block” pointers to prevent cross-subagent interleaving.
Changes:
- Add
subagentMetasupport to message emission and ensure streamed subagent text chunks carry_meta.parentToolCallId. - Extend daemon UI event/block types and normalizer to extract and forward
parentToolCallIdforagent_message_chunk/agent_thought_chunk. - Update the transcript reducer to maintain per-parent active assistant/thought blocks (plus scoped clearing/finishing) and add unit tests covering parallel streaming scenarios.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/acp-integration/session/emitters/MessageEmitter.ts | Adds subagentMeta to emitted agent message/thought chunks via _meta fields. |
| packages/cli/src/acp-integration/session/SubAgentTracker.ts | Passes getSubagentMeta() into streamed text emission to stamp parent linkage. |
| packages/cli/src/acp-integration/session/SubAgentTracker.test.ts | Verifies _meta includes parentToolCallId / subagentType for streamed output. |
| packages/sdk-typescript/src/daemon/ui/types.ts | Adds optional parentToolCallId to text events and transcript text blocks; adds per-parent active-block maps to state. |
| packages/sdk-typescript/src/daemon/ui/normalizer.ts | Extracts parentToolCallId from _meta for agent message/thought chunk updates. |
| packages/sdk-typescript/src/daemon/ui/transcript.ts | Routes deltas per parentToolCallId, adds scoped clearActiveText, and ensures streaming flags are cleared across keyed blocks. |
| packages/sdk-typescript/src/daemon/ui/store.ts | Ensures new per-parent maps are initialized/cloned in store state creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📋 Review SummaryThis PR addresses a critical bug in daemon mode where parallel subAgent text streams were interleaved and merged into a single transcript block, causing garbled output in the WebShell. The fix implements per-parent isolation using a keyed map approach that extends the existing tool block isolation pattern to text/thought events. The implementation is well-designed with comprehensive test coverage (211 daemon UI tests). 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Review Round 1 — All findings addressed in 43c04c7
213 tests passing, 0 TypeScript errors. |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #4689 fix(daemon): isolate parallel subAgent text streams in transcript reducer
Type: Bug Fix
Change size: +578/-16 across 8 files, 2 commits
Multi-Round Review (Rounds 0-6): Clean — 0 findings
Round 0 (Design): Correct approach — extends the existing parentToolCallId isolation pattern from tool blocks to text/thought events. The per-parent keyed maps (activeAssistantBlockByParent, activeThoughtBlockByParent) are a natural extension that maintains backward compatibility for events without parentToolCallId.
Round 1 (Architecture):
- Clean three-layer flow: emission (
MessageEmitter+SubAgentTracker) → normalization (normalizer.ts) → reduction (transcript.ts) parentToolCallIdcorrectly extracted from_metain normalizer- Per-parent maps correctly initialized in
createState, copied incloneTranscriptState, cleaned intrimTranscriptState
Round 2 (Robustness):
- wenshao's findings (both addressed in commit 2):
- F1: Cross-eviction now finalizes
streaming = falseon evicted assistant block + test T8 - F2:
clearActiveTextscoped path now has test T9
- F1: Cross-eviction now finalizes
- Copilot's finding (addressed in commit 2): Added
kind !== 'user'guard to prevent user text events from entering keyed path
Round 3 (CI/Security): N/A — purely UI state management, no security concerns.
Round 4 (Performance): Per-parent map lookups are O(1). finishAssistant iterates keyed blocks — acceptable given typical concurrent subAgent count (< 10).
Round 5 (Bug Fix): Root cause correctly identified and fixed. No regressions — scalar path unchanged for events without parentToolCallId. 11 new tests (T1-T9 + 2 edge cases) provide good coverage.
Round 6 (Undirected): Maps are bounded by transcript size limit via trimTranscriptState. The kind !== 'user' guard correctly prevents the issue Copilot identified.
LGTM!
This review was generated by QoderWork AI
Review Round 2 — All findings addressed in bb35252
217 tests passing, 0 TypeScript errors. |
Review Round 3 — Addressed in 646a094
217 tests passing, 0 TypeScript errors. |
Review Round 4 — Addressed in 9997a76
219 tests passing, 0 TypeScript errors. |
Maintainer Verification ReportTested on: macOS Darwin 25.4.0 1. TypeScript Compilation
Verdict: Zero new TypeScript errors introduced. 2. Unit Tests — daemonUi (sdk-typescript)PR-specific tests (19/19 passed):
3. Unit Tests — SubAgentTracker (cli)Verified: 4. Code Review SummaryArchitecture: Clean per-parent keyed-map approach ( Key design strengths:
Backward compatibility: Verified — keyed path and scalar path are fully independent (comment at line 362-365 in No issues found. Ready to merge.
|
…ucer Thread parentToolCallId through the text emission → normalization → reduction pipeline so concurrent subAgent text chunks accumulate into independent transcript blocks instead of interleaving into one. Closes #4687
…n, guard user text, add tests - Finalize evicted assistant block's `streaming = false` when thought text for the same parentToolCallId triggers mutual exclusion - Guard parentId extraction: skip for `kind === 'user'` to prevent accidental keyed-map routing of user text events - Add T8 (thought evicts assistant + finalizes streaming) and T9 (thought text cleared by scoped clearActiveText from tool.update)
…4 tests - Extract buildChunkMeta() to eliminate duplicated _meta construction - Add JSDoc on DaemonTextTranscriptBlock.parentToolCallId - Add invariant comment on keyed/scalar path independence - Add T10-T13: thought map trim, finishAssistant thought clear, assistant→thought eviction, scoped clearActiveText streaming=false
Eliminate duplicated _meta extraction between agent_message_chunk and agent_thought_chunk cases per R3 review.
…and finishAssistant
doudouOUC
force-pushed
the
worktree-wiggly-meandering-lightning
branch
from
ca63fce to
ee53f7e
Compare
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] SubAgentTracker.test.ts:788 — agent_thought_chunk 测试缺少 _meta 断言
相邻的 agent_message_chunk 测试（line 719）已断言 _meta 含 parentToolCallId/subagentType，但 thought 测试（line 788）未加。若 emitMessage 的 thought 分支漏传 subagentMeta，现有测试不会发现——thought chunk 会落入 scalar 路径导致并行交错。建议镜像 message 测试的 _meta 断言。
— qwen3.7-max via Qwen Code /review
| case 'agent_message_chunk': { | ||
| const text = getTextContent(update['content']); | ||
| return text ? [{ ...base, type: 'assistant.text.delta', text }] : []; | ||
| if (!text) return []; |
There was a problem hiding this comment.
[Suggestion] 风格不一致：此处使用 early-return guard（if (!text) return []），而同 switch 中 user_message_chunk（line 407）和 shell_output/tool_output 使用三元表达式（return text ? [...] : []）。建议统一为同一种风格。
— qwen3.7-max via Qwen Code /review
| next.activeUserBlockId = block.id; | ||
| next.activeAssistantBlockId = undefined; | ||
| next.activeThoughtBlockId = undefined; | ||
| next.activeAssistantBlockByParent = {}; |
There was a problem hiding this comment.
[Suggestion] appendLocalUserTranscriptMessage 清理 keyed map 时未 finalize 流式块
直接 activeAssistantBlockByParent = {} 但未遍历引用的块设置 streaming = false。被孤立的 assistant 块保留 streaming: true，渲染层可能显示永久加载指示器。建议参考 finishAssistant 模式，清理前先 finalize：
for (const blockId of Object.values(next.activeAssistantBlockByParent)) {
const block = getWritableBlockById(next, blockId);
if (block?.kind === 'assistant') {
block.streaming = false;
block.updatedAt = next.now;
}
}
next.activeAssistantBlockByParent = {};
next.activeThoughtBlockByParent = {};— qwen3.7-max via Qwen Code /review
| : this.emitAgentMessage(text, timestamp, subagentMeta); | ||
| } | ||
|
|
||
| private buildChunkMeta( |
There was a problem hiding this comment.
[Suggestion] 两种不一致的序列化模式
buildChunkMeta（此处）显式选取字段（parentToolCallId、subagentType），而 emitUsageMetadata（line 126-127）使用对象展开（...subagentMeta）。若 SubagentMeta 增加新字段，emitUsageMetadata 会静默包含，buildChunkMeta 会静默丢弃。建议统一为显式提取或统一使用展开。
— qwen3.7-max via Qwen Code /review
Summary
/review派发的多个 Agent）的文本 chunk 交错合并到同一个 transcript block，导致 WebShell 上显示乱码SubAgentTracker.createStreamTextHandler发射agent_message_chunk时不带parentToolCallId；normalizer 不提取；transcript reducer 用单一activeAssistantBlockId接收所有文本parentToolCallId隔离模式，将其扩展到 text/thought 事件。不带parentToolCallId的文本完全走现有 scalar 路径（零行为变更）变更概览
MessageEmitter.tsemitAgentMessage/emitAgentThought/emitMessage加subagentMeta?参数SubAgentTracker.tscreateStreamTextHandler传this.getSubagentMeta()types.tsDaemonUiTextEvent+DaemonTextTranscriptBlock加parentToolCallId?；State 加两个 keyed mapnormalizer.tsagent_message_chunk/agent_thought_chunk从_meta提取parentToolCallIdtranscript.tsappendTextDeltaper-parent 隔离；finishAssistant遍历 map 清 streaming；clearActiveTextscoped clear；init/clone/trimstore.tscreateState浅拷贝新 map设计要点
activeAssistantBlockByParent: Record<string, string>按parentToolCallId维护独立的 active block pointer，与现有 scalaractiveAssistantBlockId共存tool.update只清自己 parent 的 text block，不影响其他并行 subAgentassistant.done后所有 keyed block 的streaming设为 false，防止 zombie spinnerparentToolCallId内生效MessageEmitter只从subagentMeta取parentToolCallId/subagentType，不 spread 整个对象，防止覆盖timestamp等保留字段已知局限（后续 PR）
transcriptToMessages.ts渲染层需用parentToolCallId替代位置启发式Closes #4687
Test plan
parentToolCallId的交替 delta → 独立 blockassistant.done后所有 map block streaming=falseparentToolCallId走 scalar 路径_meta含parentToolCallId🤖 Generated with Qwen Code