fix(core): foreground agent entry lingering in status bar after completion#3921
Merged
yiliang114 merged 1 commit intoMay 7, 2026
Merged
Conversation
…etion In BackgroundTaskRegistry.unregisterForeground(), emitStatusChange was called before agents.delete(). This meant the React refresh callback (rebuilds snapshot via getAll()) still saw the entry in the Map with status='running'. After deletion, no second status-change fired, so React state never updated — the entry remained visible as '1 local agent' in the footer pill and the timer kept ticking in the dialog. Swap the order: delete first, then emit. Now getAll() in the callback returns an empty snapshot for the removed entry, and React correctly drops it from the UI.
wenshao
approved these changes
May 7, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
3 tasks
wenshao
added a commit
to wenshao/qwen-code
that referenced
this pull request
May 7, 2026
Three real bugs flagged by gpt-5.5 via /qreview, plus 4 doc / sanitization nits from Copilot. All 7 threads close together since they share the same surfaces. ## Critical fixes 1. **Foreground subagents disappeared mid-parent-turn** (PRRT_kwDOPB-92c6AYvL9). Post-QwenLM#3921 swap-order, `unregisterForeground` drops the entry from the panel snapshot the moment the subagent finishes. The previous round's `!isPending` gate on `SubagentScrollbackSummary` then suppressed the inline summary too, leaving the user with nothing on screen for the run until the parent committed. - Drop the `!isPending` gate — `unregisterForeground` already removes the row from the panel, so the inline summary can fire in BOTH live and committed phases without duplicating it. - Tighten the `ToolGroupMessage` live-phase hide so it only filters `running` / `paused` / `background` task entries (`isPanelOwnedSubagentTool`), not terminal ones. Terminal entries pass through immediately so the summary lands. - The "panel-owned" predicate is now distinct from the broader "subagent tool entry" predicate (`isSubagentToolEntry`) and the "terminal subagent" predicate (`isTerminalSubagentTool`); each usage site picks the one it actually means. 2. **Compact mode dropped the scrollback summary** (PRRT_kwDOPB-92c6AYvLw). Force-expanding the group made the container go through the expanded path, but `ToolMessage`'s own compact-mode gate (`!compactMode || forceShowResult ? renderer : 'none'`) still suppressed the result block, so `SubagentScrollbackSummary` never rendered for compact-mode users. Pass `forceShowResult={true}` for terminal subagent tool entries so the result block is always rendered. 3. **`mergeCompactToolGroups.isForceExpandGroup` didn't know about terminal subagents** (PRRT_kwDOPB-92c6AYvMC). The committed- history preprocessor merged adjacent tool_groups before render, so a terminal `task_execution` group could be absorbed into a compact batch (its `tool_use_summary` label dropped), and the render-time force-expand check never got a chance to override. Mirror the `hasCommittedTerminalSubagent` predicate inside `isForceExpandGroup` so preprocessing and rendering agree. ## Doc / sanitization nits - `BackgroundStatusChangeCallback` doc now lists every emitter (register / complete / fail / cancel / finalizeCancelled / finalizeCancellationIfPending / abandon / unregisterForeground / reset) and groups them by ordering camp (keeps-the-entry vs removes-the-entry — `reset` joins `unregisterForeground` in the delete-then-emit camp). - ANSI-escape `data.subagentName` in the focus-holder banner and the queued marker (`SubagentExecutionRenderer`) — same threat model as the panel rows and `SubagentScrollbackSummary`. ## Coverage delta - New ToolMessage case: live-phase terminal subagent now renders inline (replaces the prior "no scrollback summary" assertion that was the symptom of the AYvL9 bug). - New ToolGroupMessage cases: terminal subagent in live phase renders inline; `forceShowResult=true` propagates for terminal subagent tools (mock now exposes the prop). - New mergeCompactToolGroups parametrized cases: terminal subagent in any of completed / failed / cancelled stays its own batch. 280 tests pass across cli messages + utils + background-view + core/agents. TS clean.
wenshao
added a commit
to wenshao/qwen-code
that referenced
this pull request
May 8, 2026
…ase too Resolved comment 3203286936 codified the design intent that `SubagentScrollbackSummary` "fires in BOTH live and committed phases" to bridge `unregisterForeground`'s post-delete panel-snapshot drop and the parent turn committing. Non-compact mode honored that contract (terminal subagents render the summary inline whenever they appear in `inlineToolCalls`), but compact mode still gated `hasCommittedTerminalSubagent` on `!isPending`, so a foreground subagent finishing mid-turn under compact mode produced NOTHING inline until the parent committed — exactly the gap the bridge was meant to close. Drop the `!isPending` arm and rename `hasCommittedTerminalSubagent` → `hasTerminalSubagent`. The force-expand now applies to terminal subagents in either phase; compact-mode users see the same outcome line non-compact users already get. Mirrors `SubagentExecutionRenderer`'s ungated terminal-summary path and `mergeCompactToolGroups.isForceExpandGroup`'s no-isPending-gate preprocessing rule. Tests: - Flip "compact mode: live group with completed subagent stays compact" → "force-expands so the summary bridges the panel-snapshot drop". Update rationale to reflect post-QwenLM#3921 reality (panel evicts terminal foreground rows immediately). - Add "compact mode: live mixed group with terminal subagent + sibling force-expands and renders both" — covers the bridge in mixed groups. - Update two stale `hasCommittedTerminalSubagent` cross-references in `mergeCompactToolGroups.{ts,test.ts}` comments.
TaimoorSiddiquiOfficial
pushed a commit
to TaimoorSiddiquiOfficial/HopCode
that referenced
this pull request
May 8, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wenshao
added a commit
that referenced
this pull request
May 8, 2026
…Change emit (#3919) * fix(cli,core): isPending gate on subagent scrollback summary + post-delete statusChange emit Two follow-ups from PR #3909 review. 1. **Re-introduce `isPending` gate on `SubagentExecutionRenderer`'s scrollback summary** (Copilot finding on PRRT_kwDOPB-92c6AUQHn). The verbose inline frame retirement collapsed `SubagentExecutionRenderer` to "render the summary whenever a subagent reaches a terminal status" — but with `isPending` removed in #3909, that fired in BOTH live (pendingHistoryItems) AND committed (Static) phases. Live-phase rendering duplicated the row LiveAgentPanel already paints below the composer until the parent turn committed. Add `isPending` back to `ToolMessageProps` purely as a gate for this one render path: the summary fires only when `!isPending` (committed). `ToolGroupMessage` forwards the flag (it kept the prop on its own interface for upstream compat the whole time). Test gap closed by the new `live (isPending) terminal subagent → no scrollback summary (panel owns the row)` case. 2. **Emit `statusChange` AFTER delete in `unregisterForeground`** (Copilot finding on PRRT_kwDOPB-92c6AUQGc + the panel-only reconciliation it spawned). The shared snapshot in `useBackgroundTaskView` only refreshes on `statusChange`, and `unregisterForeground` previously fired exactly once — BEFORE delete — so the snapshot froze with the agent as "running" while `registry.get()` returned undefined. Result: `BackgroundTasksDialog` list mode showed a ghost "running" row with cancel hints whose `x` was a no-op, contradicting what the panel already showed (synthesized neutral terminal). Fire `statusChange` a second time AFTER `agents.delete()` so snapshot consumers see the registry-less state and stop surfacing the agent. The first emit still mirrors complete/fail/cancel/finalize ordering (callbacks that re-read `registry.get` see the entry); the second emit is the new contract for snapshot-based views. React batches the two resulting setState calls into one re-render so consumers re-render exactly once. Updated the existing "emits status change before removing the entry" test to capture both emits and explicitly assert that the second observes the registry-less state. Added a sibling test covering the post-delete `getAll()` count. Coverage: 190 passing tests across core + cli (background-view + ToolMessage + ToolGroupMessage + useBackgroundTaskView). * fix(cli,core): compact-mode terminal subagent expansion + statusChange context flag Five review findings on PR #3919: 1. **Compact mode bypassed the scrollback summary** (gpt-5.5 via /qreview, ToolGroupMessage:324). `ToolGroupMessage` returns `CompactToolGroupDisplay` before the ToolMessage path when `compactMode === true`, so the new `isPending` gate on `SubagentExecutionRenderer` only protected the expanded path — committed terminal subagents in compact mode never reached `SubagentScrollbackSummary` and the LiveAgentPanel → committed- summary handoff broke for users who turned compact mode on. Force-expand the group when `!isPending` AND any tool call has a terminal `task_execution` resultDisplay. Stay compact while the parent turn is still live (`isPending`) — the panel below the composer owns that surface and an inline summary would duplicate it. Coverage: 4 new ToolGroupMessage cases (compact + completed-committed expands; compact + running-live stays compact; compact + completed-live stays compact; compact + failed-committed expands). 2. **Snapshot-coupled comment in `packages/core`** (Copilot, background-tasks.ts:292). The comment named CLI/UI consumers (`useBackgroundTaskView`, `BackgroundTasksDialog`) and asserted React batching guarantees from a core file. Reword to "snapshot-style consumers that re-pull `getAll()` from inside the callback" and drop the framework-specific batching claim. 3. **Two-phase emit needed an explicit signal** (Copilot, background-tasks.ts:283). Emitting `statusChange` twice without distinguishing the phases forced consumers to either do duplicate work or risk persisting a stale `entry` from the second callback. Add an optional second arg `context?: { removed?: boolean }` to `BackgroundStatusChangeCallback`; the post-delete emit passes `{ removed: true }` so consumers can disambiguate without re-querying the registry. Backwards compatible — existing callbacks ignore the new arg. Tests updated to assert both `mock.calls[0][1] === undefined` and `mock.calls[1][1] === { removed: true }`. 4. **`isPending` doc clarified** (Copilot, ToolMessage.tsx:507). Made the default semantics explicit: omitted/undefined is treated as committed (not pending); live-area renderers MUST pass `true` explicitly to suppress the scrollback summary. 5. (4 of the threads were duplicate Copilot fires of #2 + #3.) Coverage: 219 test files / 3369 passing across cli/ui + core/agents. * docs(cli): update ToolGroupMessageProps.isPending JSDoc The previous prop comment claimed `isPending` was "not consumed by the group body" — true at the time, but the body now reads it for two real purposes (compact-mode gating + forwarding to ToolMessage). Update the doc so future callers / tests don't treat it as legacy. Addresses Copilot finding on PRRT_kwDOPB-92c6AYE0V. * fix(cli): hide live-phase subagent tool entries — LiveAgentPanel owns the row User report: with compact mode OFF, a running subagent shows up twice — once as the parent tool group's `task` row (status icon + name + description), once as the LiveAgentPanel row beneath the composer. Same agent, two surfaces, redundant. Filter `task_execution` tool entries out of the expanded `ToolGroupMessage` while `isPending=true` so the panel is the single source of truth for in-flight subagents. The entry returns once the parent turn commits (`isPending=false`), letting `SubagentScrollbackSummary` land inside the parent's tool group as a persistent audit trail. Exception: subagents with a pending approval still render, because the focus-routed banner / queued marker is the only inline surface that lets users answer the prompt without opening the dialog. If a group is purely panel-owned (e.g. a single Task call with no sibling tools), the entire `ToolGroupMessage` returns `null` so an empty bordered container doesn't float above the panel. Coverage: +4 ToolGroupMessage cases — running entry hidden in live phase / mixed group keeps siblings / pending-approval entry still renders / committed entry comes back for the audit trail. * refactor(cli): tighten subagent-tool helper naming + ANSI-safe scrollback summary Self-audit + independent review found 5 cleanup items on the live-phase hide path; all addressed in one commit since none are behavioral changes: 1. **Move `allEntriesPanelOwned` short-circuit BEFORE `showCompact`** so a pure-subagent group in compact mode is also hidden during the live phase (previously CompactToolGroupDisplay rendered a single summary line above the panel — a mild duplicate on top of what the non-compact path already fixed). 2. **Rename `isLiveSubagentTool` → `isSubagentToolEntry`.** The helper identifies a tool's resultDisplay shape; it doesn't check live-state. The previous name conflated "predicate" with "use case" and read as if it returned true only during the live phase. 3. **DRY up `hasCommittedTerminalSubagent`** to use `isSubagentToolEntry` instead of inlining its own type-narrowing. 4. **ANSI-escape `subagentName` / `taskDescription` / `terminateReason`** in `SubagentScrollbackSummary`. Same threat model as the panel rows and HistoryItemDisplay — these strings come from subagent config (user-authored) and LLM output and could carry terminal control sequences. The stats fields (tool count / duration / tokens) flow through trusted formatters and don't need escaping. 5. **Doc comments updated** to reflect the four real responsibilities of `isPending` on `ToolGroupMessageProps` (hide pure groups, force-expand committed compact, per-tool filter, forward to ToolMessage), to clarify that the keyboard-focused subagent id can point at a hidden tool harmlessly (the iterator returns `null` before the focus prop is computed), and to drop the redundant "EXCEPT" clause on the per-tool filter in favor of a single sentence. Coverage unchanged: 251 passing tests across messages / background-view / core/agents; broader 3374-test sweep clean; TS clean on both cli and core packages. * fix(cli,core): address 3 critical review findings + ANSI/doc cleanups Three real bugs flagged by gpt-5.5 via /qreview, plus 4 doc / sanitization nits from Copilot. All 7 threads close together since they share the same surfaces. ## Critical fixes 1. **Foreground subagents disappeared mid-parent-turn** (PRRT_kwDOPB-92c6AYvL9). Post-#3921 swap-order, `unregisterForeground` drops the entry from the panel snapshot the moment the subagent finishes. The previous round's `!isPending` gate on `SubagentScrollbackSummary` then suppressed the inline summary too, leaving the user with nothing on screen for the run until the parent committed. - Drop the `!isPending` gate — `unregisterForeground` already removes the row from the panel, so the inline summary can fire in BOTH live and committed phases without duplicating it. - Tighten the `ToolGroupMessage` live-phase hide so it only filters `running` / `paused` / `background` task entries (`isPanelOwnedSubagentTool`), not terminal ones. Terminal entries pass through immediately so the summary lands. - The "panel-owned" predicate is now distinct from the broader "subagent tool entry" predicate (`isSubagentToolEntry`) and the "terminal subagent" predicate (`isTerminalSubagentTool`); each usage site picks the one it actually means. 2. **Compact mode dropped the scrollback summary** (PRRT_kwDOPB-92c6AYvLw). Force-expanding the group made the container go through the expanded path, but `ToolMessage`'s own compact-mode gate (`!compactMode || forceShowResult ? renderer : 'none'`) still suppressed the result block, so `SubagentScrollbackSummary` never rendered for compact-mode users. Pass `forceShowResult={true}` for terminal subagent tool entries so the result block is always rendered. 3. **`mergeCompactToolGroups.isForceExpandGroup` didn't know about terminal subagents** (PRRT_kwDOPB-92c6AYvMC). The committed- history preprocessor merged adjacent tool_groups before render, so a terminal `task_execution` group could be absorbed into a compact batch (its `tool_use_summary` label dropped), and the render-time force-expand check never got a chance to override. Mirror the `hasCommittedTerminalSubagent` predicate inside `isForceExpandGroup` so preprocessing and rendering agree. ## Doc / sanitization nits - `BackgroundStatusChangeCallback` doc now lists every emitter (register / complete / fail / cancel / finalizeCancelled / finalizeCancellationIfPending / abandon / unregisterForeground / reset) and groups them by ordering camp (keeps-the-entry vs removes-the-entry — `reset` joins `unregisterForeground` in the delete-then-emit camp). - ANSI-escape `data.subagentName` in the focus-holder banner and the queued marker (`SubagentExecutionRenderer`) — same threat model as the panel rows and `SubagentScrollbackSummary`. ## Coverage delta - New ToolMessage case: live-phase terminal subagent now renders inline (replaces the prior "no scrollback summary" assertion that was the symptom of the AYvL9 bug). - New ToolGroupMessage cases: terminal subagent in live phase renders inline; `forceShowResult=true` propagates for terminal subagent tools (mock now exposes the prop). - New mergeCompactToolGroups parametrized cases: terminal subagent in any of completed / failed / cancelled stays its own batch. 280 tests pass across cli messages + utils + background-view + core/agents. TS clean. * fix(cli): drop `'paused'` arm from isPanelOwnedSubagentTool — not in AgentResultDisplay union CI Lint failed with TS2367: the previous round's `isPanelOwnedSubagentTool` checked for `status === 'paused'` but `AgentResultDisplay.status` (the tool-result-side type) only carries `'running' | 'completed' | 'failed' | 'cancelled' | 'background'`. The `'paused'` status lives on the registry-side `BackgroundTaskStatus` union and is only ever surfaced through `LiveAgentPanel` directly, never through a `task_execution` payload. Drop the dead arm and add a comment so a future "let's also check paused here" doesn't get re-introduced. * fix(cli): apply panel-ownership filter once before compact-mode decision Mixed live groups (running subagent + sibling tool) leaked the panel-owned subagent into `CompactToolGroupDisplay`'s count and `getActiveTool` selection, because `showCompact` returned BEFORE the inline `.map()` filter ran. Compact-mode users would see e.g. `task × 2 Delegate task to subagent` even though LiveAgentPanel already owned the subagent row below the composer. Derive `inlineToolCalls` once via `useMemo` immediately after the existing hook block and use it consistently for the compact summary, sizing math, and the render map. The early-return for "all-entries-panel-owned" collapses into `inlineToolCalls.length === 0` (gated on `isPending` so the legacy empty-input committed-phase snapshot is preserved). Remove the inner `.map()` filter — the upstream derivation already excluded the same entries. JSDoc updates: - `ToolGroupMessageProps.isPending` now describes the real flow (build inlineToolCalls / force-expand / forward to ToolMessage for parity). - `ToolMessageProps.isPending` is documented as forwarded-but-inert (`SubagentExecutionRenderer` doesn't gate on it; the live-phase filter and the unconditional terminal summary do the actual work). Regression test: live mixed group in compact mode → sibling wins active-tool, count collapses to 1, no `× 2` suffix, no subagent description in the header. Addresses Copilot review comments 3205262972 / 3205263020 (doc/code mismatch) and gpt-5.5 critical 3205288299 (compact-mode leak). * fix(cli): force-expand compact groups on terminal subagent in live phase too Resolved comment 3203286936 codified the design intent that `SubagentScrollbackSummary` "fires in BOTH live and committed phases" to bridge `unregisterForeground`'s post-delete panel-snapshot drop and the parent turn committing. Non-compact mode honored that contract (terminal subagents render the summary inline whenever they appear in `inlineToolCalls`), but compact mode still gated `hasCommittedTerminalSubagent` on `!isPending`, so a foreground subagent finishing mid-turn under compact mode produced NOTHING inline until the parent committed — exactly the gap the bridge was meant to close. Drop the `!isPending` arm and rename `hasCommittedTerminalSubagent` → `hasTerminalSubagent`. The force-expand now applies to terminal subagents in either phase; compact-mode users see the same outcome line non-compact users already get. Mirrors `SubagentExecutionRenderer`'s ungated terminal-summary path and `mergeCompactToolGroups.isForceExpandGroup`'s no-isPending-gate preprocessing rule. Tests: - Flip "compact mode: live group with completed subagent stays compact" → "force-expands so the summary bridges the panel-snapshot drop". Update rationale to reflect post-#3921 reality (panel evicts terminal foreground rows immediately). - Add "compact mode: live mixed group with terminal subagent + sibling force-expands and renders both" — covers the bridge in mixed groups. - Update two stale `hasCommittedTerminalSubagent` cross-references in `mergeCompactToolGroups.{ts,test.ts}` comments.
DragonnZhang
pushed a commit
that referenced
this pull request
May 8, 2026
…etion (#3921) In BackgroundTaskRegistry.unregisterForeground(), emitStatusChange was called before agents.delete(). This meant the React refresh callback (rebuilds snapshot via getAll()) still saw the entry in the Map with status='running'. After deletion, no second status-change fired, so React state never updated — the entry remained visible as '1 local agent' in the footer pill and the timer kept ticking in the dialog. Swap the order: delete first, then emit. Now getAll() in the callback returns an empty snapshot for the removed entry, and React correctly drops it from the UI.
DragonnZhang
pushed a commit
that referenced
this pull request
May 8, 2026
…Change emit (#3919) * fix(cli,core): isPending gate on subagent scrollback summary + post-delete statusChange emit Two follow-ups from PR #3909 review. 1. **Re-introduce `isPending` gate on `SubagentExecutionRenderer`'s scrollback summary** (Copilot finding on PRRT_kwDOPB-92c6AUQHn). The verbose inline frame retirement collapsed `SubagentExecutionRenderer` to "render the summary whenever a subagent reaches a terminal status" — but with `isPending` removed in #3909, that fired in BOTH live (pendingHistoryItems) AND committed (Static) phases. Live-phase rendering duplicated the row LiveAgentPanel already paints below the composer until the parent turn committed. Add `isPending` back to `ToolMessageProps` purely as a gate for this one render path: the summary fires only when `!isPending` (committed). `ToolGroupMessage` forwards the flag (it kept the prop on its own interface for upstream compat the whole time). Test gap closed by the new `live (isPending) terminal subagent → no scrollback summary (panel owns the row)` case. 2. **Emit `statusChange` AFTER delete in `unregisterForeground`** (Copilot finding on PRRT_kwDOPB-92c6AUQGc + the panel-only reconciliation it spawned). The shared snapshot in `useBackgroundTaskView` only refreshes on `statusChange`, and `unregisterForeground` previously fired exactly once — BEFORE delete — so the snapshot froze with the agent as "running" while `registry.get()` returned undefined. Result: `BackgroundTasksDialog` list mode showed a ghost "running" row with cancel hints whose `x` was a no-op, contradicting what the panel already showed (synthesized neutral terminal). Fire `statusChange` a second time AFTER `agents.delete()` so snapshot consumers see the registry-less state and stop surfacing the agent. The first emit still mirrors complete/fail/cancel/finalize ordering (callbacks that re-read `registry.get` see the entry); the second emit is the new contract for snapshot-based views. React batches the two resulting setState calls into one re-render so consumers re-render exactly once. Updated the existing "emits status change before removing the entry" test to capture both emits and explicitly assert that the second observes the registry-less state. Added a sibling test covering the post-delete `getAll()` count. Coverage: 190 passing tests across core + cli (background-view + ToolMessage + ToolGroupMessage + useBackgroundTaskView). * fix(cli,core): compact-mode terminal subagent expansion + statusChange context flag Five review findings on PR #3919: 1. **Compact mode bypassed the scrollback summary** (gpt-5.5 via /qreview, ToolGroupMessage:324). `ToolGroupMessage` returns `CompactToolGroupDisplay` before the ToolMessage path when `compactMode === true`, so the new `isPending` gate on `SubagentExecutionRenderer` only protected the expanded path — committed terminal subagents in compact mode never reached `SubagentScrollbackSummary` and the LiveAgentPanel → committed- summary handoff broke for users who turned compact mode on. Force-expand the group when `!isPending` AND any tool call has a terminal `task_execution` resultDisplay. Stay compact while the parent turn is still live (`isPending`) — the panel below the composer owns that surface and an inline summary would duplicate it. Coverage: 4 new ToolGroupMessage cases (compact + completed-committed expands; compact + running-live stays compact; compact + completed-live stays compact; compact + failed-committed expands). 2. **Snapshot-coupled comment in `packages/core`** (Copilot, background-tasks.ts:292). The comment named CLI/UI consumers (`useBackgroundTaskView`, `BackgroundTasksDialog`) and asserted React batching guarantees from a core file. Reword to "snapshot-style consumers that re-pull `getAll()` from inside the callback" and drop the framework-specific batching claim. 3. **Two-phase emit needed an explicit signal** (Copilot, background-tasks.ts:283). Emitting `statusChange` twice without distinguishing the phases forced consumers to either do duplicate work or risk persisting a stale `entry` from the second callback. Add an optional second arg `context?: { removed?: boolean }` to `BackgroundStatusChangeCallback`; the post-delete emit passes `{ removed: true }` so consumers can disambiguate without re-querying the registry. Backwards compatible — existing callbacks ignore the new arg. Tests updated to assert both `mock.calls[0][1] === undefined` and `mock.calls[1][1] === { removed: true }`. 4. **`isPending` doc clarified** (Copilot, ToolMessage.tsx:507). Made the default semantics explicit: omitted/undefined is treated as committed (not pending); live-area renderers MUST pass `true` explicitly to suppress the scrollback summary. 5. (4 of the threads were duplicate Copilot fires of #2 + #3.) Coverage: 219 test files / 3369 passing across cli/ui + core/agents. * docs(cli): update ToolGroupMessageProps.isPending JSDoc The previous prop comment claimed `isPending` was "not consumed by the group body" — true at the time, but the body now reads it for two real purposes (compact-mode gating + forwarding to ToolMessage). Update the doc so future callers / tests don't treat it as legacy. Addresses Copilot finding on PRRT_kwDOPB-92c6AYE0V. * fix(cli): hide live-phase subagent tool entries — LiveAgentPanel owns the row User report: with compact mode OFF, a running subagent shows up twice — once as the parent tool group's `task` row (status icon + name + description), once as the LiveAgentPanel row beneath the composer. Same agent, two surfaces, redundant. Filter `task_execution` tool entries out of the expanded `ToolGroupMessage` while `isPending=true` so the panel is the single source of truth for in-flight subagents. The entry returns once the parent turn commits (`isPending=false`), letting `SubagentScrollbackSummary` land inside the parent's tool group as a persistent audit trail. Exception: subagents with a pending approval still render, because the focus-routed banner / queued marker is the only inline surface that lets users answer the prompt without opening the dialog. If a group is purely panel-owned (e.g. a single Task call with no sibling tools), the entire `ToolGroupMessage` returns `null` so an empty bordered container doesn't float above the panel. Coverage: +4 ToolGroupMessage cases — running entry hidden in live phase / mixed group keeps siblings / pending-approval entry still renders / committed entry comes back for the audit trail. * refactor(cli): tighten subagent-tool helper naming + ANSI-safe scrollback summary Self-audit + independent review found 5 cleanup items on the live-phase hide path; all addressed in one commit since none are behavioral changes: 1. **Move `allEntriesPanelOwned` short-circuit BEFORE `showCompact`** so a pure-subagent group in compact mode is also hidden during the live phase (previously CompactToolGroupDisplay rendered a single summary line above the panel — a mild duplicate on top of what the non-compact path already fixed). 2. **Rename `isLiveSubagentTool` → `isSubagentToolEntry`.** The helper identifies a tool's resultDisplay shape; it doesn't check live-state. The previous name conflated "predicate" with "use case" and read as if it returned true only during the live phase. 3. **DRY up `hasCommittedTerminalSubagent`** to use `isSubagentToolEntry` instead of inlining its own type-narrowing. 4. **ANSI-escape `subagentName` / `taskDescription` / `terminateReason`** in `SubagentScrollbackSummary`. Same threat model as the panel rows and HistoryItemDisplay — these strings come from subagent config (user-authored) and LLM output and could carry terminal control sequences. The stats fields (tool count / duration / tokens) flow through trusted formatters and don't need escaping. 5. **Doc comments updated** to reflect the four real responsibilities of `isPending` on `ToolGroupMessageProps` (hide pure groups, force-expand committed compact, per-tool filter, forward to ToolMessage), to clarify that the keyboard-focused subagent id can point at a hidden tool harmlessly (the iterator returns `null` before the focus prop is computed), and to drop the redundant "EXCEPT" clause on the per-tool filter in favor of a single sentence. Coverage unchanged: 251 passing tests across messages / background-view / core/agents; broader 3374-test sweep clean; TS clean on both cli and core packages. * fix(cli,core): address 3 critical review findings + ANSI/doc cleanups Three real bugs flagged by gpt-5.5 via /qreview, plus 4 doc / sanitization nits from Copilot. All 7 threads close together since they share the same surfaces. ## Critical fixes 1. **Foreground subagents disappeared mid-parent-turn** (PRRT_kwDOPB-92c6AYvL9). Post-#3921 swap-order, `unregisterForeground` drops the entry from the panel snapshot the moment the subagent finishes. The previous round's `!isPending` gate on `SubagentScrollbackSummary` then suppressed the inline summary too, leaving the user with nothing on screen for the run until the parent committed. - Drop the `!isPending` gate — `unregisterForeground` already removes the row from the panel, so the inline summary can fire in BOTH live and committed phases without duplicating it. - Tighten the `ToolGroupMessage` live-phase hide so it only filters `running` / `paused` / `background` task entries (`isPanelOwnedSubagentTool`), not terminal ones. Terminal entries pass through immediately so the summary lands. - The "panel-owned" predicate is now distinct from the broader "subagent tool entry" predicate (`isSubagentToolEntry`) and the "terminal subagent" predicate (`isTerminalSubagentTool`); each usage site picks the one it actually means. 2. **Compact mode dropped the scrollback summary** (PRRT_kwDOPB-92c6AYvLw). Force-expanding the group made the container go through the expanded path, but `ToolMessage`'s own compact-mode gate (`!compactMode || forceShowResult ? renderer : 'none'`) still suppressed the result block, so `SubagentScrollbackSummary` never rendered for compact-mode users. Pass `forceShowResult={true}` for terminal subagent tool entries so the result block is always rendered. 3. **`mergeCompactToolGroups.isForceExpandGroup` didn't know about terminal subagents** (PRRT_kwDOPB-92c6AYvMC). The committed- history preprocessor merged adjacent tool_groups before render, so a terminal `task_execution` group could be absorbed into a compact batch (its `tool_use_summary` label dropped), and the render-time force-expand check never got a chance to override. Mirror the `hasCommittedTerminalSubagent` predicate inside `isForceExpandGroup` so preprocessing and rendering agree. ## Doc / sanitization nits - `BackgroundStatusChangeCallback` doc now lists every emitter (register / complete / fail / cancel / finalizeCancelled / finalizeCancellationIfPending / abandon / unregisterForeground / reset) and groups them by ordering camp (keeps-the-entry vs removes-the-entry — `reset` joins `unregisterForeground` in the delete-then-emit camp). - ANSI-escape `data.subagentName` in the focus-holder banner and the queued marker (`SubagentExecutionRenderer`) — same threat model as the panel rows and `SubagentScrollbackSummary`. ## Coverage delta - New ToolMessage case: live-phase terminal subagent now renders inline (replaces the prior "no scrollback summary" assertion that was the symptom of the AYvL9 bug). - New ToolGroupMessage cases: terminal subagent in live phase renders inline; `forceShowResult=true` propagates for terminal subagent tools (mock now exposes the prop). - New mergeCompactToolGroups parametrized cases: terminal subagent in any of completed / failed / cancelled stays its own batch. 280 tests pass across cli messages + utils + background-view + core/agents. TS clean. * fix(cli): drop `'paused'` arm from isPanelOwnedSubagentTool — not in AgentResultDisplay union CI Lint failed with TS2367: the previous round's `isPanelOwnedSubagentTool` checked for `status === 'paused'` but `AgentResultDisplay.status` (the tool-result-side type) only carries `'running' | 'completed' | 'failed' | 'cancelled' | 'background'`. The `'paused'` status lives on the registry-side `BackgroundTaskStatus` union and is only ever surfaced through `LiveAgentPanel` directly, never through a `task_execution` payload. Drop the dead arm and add a comment so a future "let's also check paused here" doesn't get re-introduced. * fix(cli): apply panel-ownership filter once before compact-mode decision Mixed live groups (running subagent + sibling tool) leaked the panel-owned subagent into `CompactToolGroupDisplay`'s count and `getActiveTool` selection, because `showCompact` returned BEFORE the inline `.map()` filter ran. Compact-mode users would see e.g. `task × 2 Delegate task to subagent` even though LiveAgentPanel already owned the subagent row below the composer. Derive `inlineToolCalls` once via `useMemo` immediately after the existing hook block and use it consistently for the compact summary, sizing math, and the render map. The early-return for "all-entries-panel-owned" collapses into `inlineToolCalls.length === 0` (gated on `isPending` so the legacy empty-input committed-phase snapshot is preserved). Remove the inner `.map()` filter — the upstream derivation already excluded the same entries. JSDoc updates: - `ToolGroupMessageProps.isPending` now describes the real flow (build inlineToolCalls / force-expand / forward to ToolMessage for parity). - `ToolMessageProps.isPending` is documented as forwarded-but-inert (`SubagentExecutionRenderer` doesn't gate on it; the live-phase filter and the unconditional terminal summary do the actual work). Regression test: live mixed group in compact mode → sibling wins active-tool, count collapses to 1, no `× 2` suffix, no subagent description in the header. Addresses Copilot review comments 3205262972 / 3205263020 (doc/code mismatch) and gpt-5.5 critical 3205288299 (compact-mode leak). * fix(cli): force-expand compact groups on terminal subagent in live phase too Resolved comment 3203286936 codified the design intent that `SubagentScrollbackSummary` "fires in BOTH live and committed phases" to bridge `unregisterForeground`'s post-delete panel-snapshot drop and the parent turn committing. Non-compact mode honored that contract (terminal subagents render the summary inline whenever they appear in `inlineToolCalls`), but compact mode still gated `hasCommittedTerminalSubagent` on `!isPending`, so a foreground subagent finishing mid-turn under compact mode produced NOTHING inline until the parent committed — exactly the gap the bridge was meant to close. Drop the `!isPending` arm and rename `hasCommittedTerminalSubagent` → `hasTerminalSubagent`. The force-expand now applies to terminal subagents in either phase; compact-mode users see the same outcome line non-compact users already get. Mirrors `SubagentExecutionRenderer`'s ungated terminal-summary path and `mergeCompactToolGroups.isForceExpandGroup`'s no-isPending-gate preprocessing rule. Tests: - Flip "compact mode: live group with completed subagent stays compact" → "force-expands so the summary bridges the panel-snapshot drop". Update rationale to reflect post-#3921 reality (panel evicts terminal foreground rows immediately). - Add "compact mode: live mixed group with terminal subagent + sibling force-expands and renders both" — covers the bridge in mixed groups. - Update two stale `hasCommittedTerminalSubagent` cross-references in `mergeCompactToolGroups.{ts,test.ts}` comments.
B-A-M-N
pushed a commit
to B-A-M-N/qwen-code
that referenced
this pull request
May 8, 2026
…Change emit (QwenLM#3919) * fix(cli,core): isPending gate on subagent scrollback summary + post-delete statusChange emit Two follow-ups from PR QwenLM#3909 review. 1. **Re-introduce `isPending` gate on `SubagentExecutionRenderer`'s scrollback summary** (Copilot finding on PRRT_kwDOPB-92c6AUQHn). The verbose inline frame retirement collapsed `SubagentExecutionRenderer` to "render the summary whenever a subagent reaches a terminal status" — but with `isPending` removed in QwenLM#3909, that fired in BOTH live (pendingHistoryItems) AND committed (Static) phases. Live-phase rendering duplicated the row LiveAgentPanel already paints below the composer until the parent turn committed. Add `isPending` back to `ToolMessageProps` purely as a gate for this one render path: the summary fires only when `!isPending` (committed). `ToolGroupMessage` forwards the flag (it kept the prop on its own interface for upstream compat the whole time). Test gap closed by the new `live (isPending) terminal subagent → no scrollback summary (panel owns the row)` case. 2. **Emit `statusChange` AFTER delete in `unregisterForeground`** (Copilot finding on PRRT_kwDOPB-92c6AUQGc + the panel-only reconciliation it spawned). The shared snapshot in `useBackgroundTaskView` only refreshes on `statusChange`, and `unregisterForeground` previously fired exactly once — BEFORE delete — so the snapshot froze with the agent as "running" while `registry.get()` returned undefined. Result: `BackgroundTasksDialog` list mode showed a ghost "running" row with cancel hints whose `x` was a no-op, contradicting what the panel already showed (synthesized neutral terminal). Fire `statusChange` a second time AFTER `agents.delete()` so snapshot consumers see the registry-less state and stop surfacing the agent. The first emit still mirrors complete/fail/cancel/finalize ordering (callbacks that re-read `registry.get` see the entry); the second emit is the new contract for snapshot-based views. React batches the two resulting setState calls into one re-render so consumers re-render exactly once. Updated the existing "emits status change before removing the entry" test to capture both emits and explicitly assert that the second observes the registry-less state. Added a sibling test covering the post-delete `getAll()` count. Coverage: 190 passing tests across core + cli (background-view + ToolMessage + ToolGroupMessage + useBackgroundTaskView). * fix(cli,core): compact-mode terminal subagent expansion + statusChange context flag Five review findings on PR QwenLM#3919: 1. **Compact mode bypassed the scrollback summary** (gpt-5.5 via /qreview, ToolGroupMessage:324). `ToolGroupMessage` returns `CompactToolGroupDisplay` before the ToolMessage path when `compactMode === true`, so the new `isPending` gate on `SubagentExecutionRenderer` only protected the expanded path — committed terminal subagents in compact mode never reached `SubagentScrollbackSummary` and the LiveAgentPanel → committed- summary handoff broke for users who turned compact mode on. Force-expand the group when `!isPending` AND any tool call has a terminal `task_execution` resultDisplay. Stay compact while the parent turn is still live (`isPending`) — the panel below the composer owns that surface and an inline summary would duplicate it. Coverage: 4 new ToolGroupMessage cases (compact + completed-committed expands; compact + running-live stays compact; compact + completed-live stays compact; compact + failed-committed expands). 2. **Snapshot-coupled comment in `packages/core`** (Copilot, background-tasks.ts:292). The comment named CLI/UI consumers (`useBackgroundTaskView`, `BackgroundTasksDialog`) and asserted React batching guarantees from a core file. Reword to "snapshot-style consumers that re-pull `getAll()` from inside the callback" and drop the framework-specific batching claim. 3. **Two-phase emit needed an explicit signal** (Copilot, background-tasks.ts:283). Emitting `statusChange` twice without distinguishing the phases forced consumers to either do duplicate work or risk persisting a stale `entry` from the second callback. Add an optional second arg `context?: { removed?: boolean }` to `BackgroundStatusChangeCallback`; the post-delete emit passes `{ removed: true }` so consumers can disambiguate without re-querying the registry. Backwards compatible — existing callbacks ignore the new arg. Tests updated to assert both `mock.calls[0][1] === undefined` and `mock.calls[1][1] === { removed: true }`. 4. **`isPending` doc clarified** (Copilot, ToolMessage.tsx:507). Made the default semantics explicit: omitted/undefined is treated as committed (not pending); live-area renderers MUST pass `true` explicitly to suppress the scrollback summary. 5. (4 of the threads were duplicate Copilot fires of #2 + #3.) Coverage: 219 test files / 3369 passing across cli/ui + core/agents. * docs(cli): update ToolGroupMessageProps.isPending JSDoc The previous prop comment claimed `isPending` was "not consumed by the group body" — true at the time, but the body now reads it for two real purposes (compact-mode gating + forwarding to ToolMessage). Update the doc so future callers / tests don't treat it as legacy. Addresses Copilot finding on PRRT_kwDOPB-92c6AYE0V. * fix(cli): hide live-phase subagent tool entries — LiveAgentPanel owns the row User report: with compact mode OFF, a running subagent shows up twice — once as the parent tool group's `task` row (status icon + name + description), once as the LiveAgentPanel row beneath the composer. Same agent, two surfaces, redundant. Filter `task_execution` tool entries out of the expanded `ToolGroupMessage` while `isPending=true` so the panel is the single source of truth for in-flight subagents. The entry returns once the parent turn commits (`isPending=false`), letting `SubagentScrollbackSummary` land inside the parent's tool group as a persistent audit trail. Exception: subagents with a pending approval still render, because the focus-routed banner / queued marker is the only inline surface that lets users answer the prompt without opening the dialog. If a group is purely panel-owned (e.g. a single Task call with no sibling tools), the entire `ToolGroupMessage` returns `null` so an empty bordered container doesn't float above the panel. Coverage: +4 ToolGroupMessage cases — running entry hidden in live phase / mixed group keeps siblings / pending-approval entry still renders / committed entry comes back for the audit trail. * refactor(cli): tighten subagent-tool helper naming + ANSI-safe scrollback summary Self-audit + independent review found 5 cleanup items on the live-phase hide path; all addressed in one commit since none are behavioral changes: 1. **Move `allEntriesPanelOwned` short-circuit BEFORE `showCompact`** so a pure-subagent group in compact mode is also hidden during the live phase (previously CompactToolGroupDisplay rendered a single summary line above the panel — a mild duplicate on top of what the non-compact path already fixed). 2. **Rename `isLiveSubagentTool` → `isSubagentToolEntry`.** The helper identifies a tool's resultDisplay shape; it doesn't check live-state. The previous name conflated "predicate" with "use case" and read as if it returned true only during the live phase. 3. **DRY up `hasCommittedTerminalSubagent`** to use `isSubagentToolEntry` instead of inlining its own type-narrowing. 4. **ANSI-escape `subagentName` / `taskDescription` / `terminateReason`** in `SubagentScrollbackSummary`. Same threat model as the panel rows and HistoryItemDisplay — these strings come from subagent config (user-authored) and LLM output and could carry terminal control sequences. The stats fields (tool count / duration / tokens) flow through trusted formatters and don't need escaping. 5. **Doc comments updated** to reflect the four real responsibilities of `isPending` on `ToolGroupMessageProps` (hide pure groups, force-expand committed compact, per-tool filter, forward to ToolMessage), to clarify that the keyboard-focused subagent id can point at a hidden tool harmlessly (the iterator returns `null` before the focus prop is computed), and to drop the redundant "EXCEPT" clause on the per-tool filter in favor of a single sentence. Coverage unchanged: 251 passing tests across messages / background-view / core/agents; broader 3374-test sweep clean; TS clean on both cli and core packages. * fix(cli,core): address 3 critical review findings + ANSI/doc cleanups Three real bugs flagged by gpt-5.5 via /qreview, plus 4 doc / sanitization nits from Copilot. All 7 threads close together since they share the same surfaces. ## Critical fixes 1. **Foreground subagents disappeared mid-parent-turn** (PRRT_kwDOPB-92c6AYvL9). Post-QwenLM#3921 swap-order, `unregisterForeground` drops the entry from the panel snapshot the moment the subagent finishes. The previous round's `!isPending` gate on `SubagentScrollbackSummary` then suppressed the inline summary too, leaving the user with nothing on screen for the run until the parent committed. - Drop the `!isPending` gate — `unregisterForeground` already removes the row from the panel, so the inline summary can fire in BOTH live and committed phases without duplicating it. - Tighten the `ToolGroupMessage` live-phase hide so it only filters `running` / `paused` / `background` task entries (`isPanelOwnedSubagentTool`), not terminal ones. Terminal entries pass through immediately so the summary lands. - The "panel-owned" predicate is now distinct from the broader "subagent tool entry" predicate (`isSubagentToolEntry`) and the "terminal subagent" predicate (`isTerminalSubagentTool`); each usage site picks the one it actually means. 2. **Compact mode dropped the scrollback summary** (PRRT_kwDOPB-92c6AYvLw). Force-expanding the group made the container go through the expanded path, but `ToolMessage`'s own compact-mode gate (`!compactMode || forceShowResult ? renderer : 'none'`) still suppressed the result block, so `SubagentScrollbackSummary` never rendered for compact-mode users. Pass `forceShowResult={true}` for terminal subagent tool entries so the result block is always rendered. 3. **`mergeCompactToolGroups.isForceExpandGroup` didn't know about terminal subagents** (PRRT_kwDOPB-92c6AYvMC). The committed- history preprocessor merged adjacent tool_groups before render, so a terminal `task_execution` group could be absorbed into a compact batch (its `tool_use_summary` label dropped), and the render-time force-expand check never got a chance to override. Mirror the `hasCommittedTerminalSubagent` predicate inside `isForceExpandGroup` so preprocessing and rendering agree. ## Doc / sanitization nits - `BackgroundStatusChangeCallback` doc now lists every emitter (register / complete / fail / cancel / finalizeCancelled / finalizeCancellationIfPending / abandon / unregisterForeground / reset) and groups them by ordering camp (keeps-the-entry vs removes-the-entry — `reset` joins `unregisterForeground` in the delete-then-emit camp). - ANSI-escape `data.subagentName` in the focus-holder banner and the queued marker (`SubagentExecutionRenderer`) — same threat model as the panel rows and `SubagentScrollbackSummary`. ## Coverage delta - New ToolMessage case: live-phase terminal subagent now renders inline (replaces the prior "no scrollback summary" assertion that was the symptom of the AYvL9 bug). - New ToolGroupMessage cases: terminal subagent in live phase renders inline; `forceShowResult=true` propagates for terminal subagent tools (mock now exposes the prop). - New mergeCompactToolGroups parametrized cases: terminal subagent in any of completed / failed / cancelled stays its own batch. 280 tests pass across cli messages + utils + background-view + core/agents. TS clean. * fix(cli): drop `'paused'` arm from isPanelOwnedSubagentTool — not in AgentResultDisplay union CI Lint failed with TS2367: the previous round's `isPanelOwnedSubagentTool` checked for `status === 'paused'` but `AgentResultDisplay.status` (the tool-result-side type) only carries `'running' | 'completed' | 'failed' | 'cancelled' | 'background'`. The `'paused'` status lives on the registry-side `BackgroundTaskStatus` union and is only ever surfaced through `LiveAgentPanel` directly, never through a `task_execution` payload. Drop the dead arm and add a comment so a future "let's also check paused here" doesn't get re-introduced. * fix(cli): apply panel-ownership filter once before compact-mode decision Mixed live groups (running subagent + sibling tool) leaked the panel-owned subagent into `CompactToolGroupDisplay`'s count and `getActiveTool` selection, because `showCompact` returned BEFORE the inline `.map()` filter ran. Compact-mode users would see e.g. `task × 2 Delegate task to subagent` even though LiveAgentPanel already owned the subagent row below the composer. Derive `inlineToolCalls` once via `useMemo` immediately after the existing hook block and use it consistently for the compact summary, sizing math, and the render map. The early-return for "all-entries-panel-owned" collapses into `inlineToolCalls.length === 0` (gated on `isPending` so the legacy empty-input committed-phase snapshot is preserved). Remove the inner `.map()` filter — the upstream derivation already excluded the same entries. JSDoc updates: - `ToolGroupMessageProps.isPending` now describes the real flow (build inlineToolCalls / force-expand / forward to ToolMessage for parity). - `ToolMessageProps.isPending` is documented as forwarded-but-inert (`SubagentExecutionRenderer` doesn't gate on it; the live-phase filter and the unconditional terminal summary do the actual work). Regression test: live mixed group in compact mode → sibling wins active-tool, count collapses to 1, no `× 2` suffix, no subagent description in the header. Addresses Copilot review comments 3205262972 / 3205263020 (doc/code mismatch) and gpt-5.5 critical 3205288299 (compact-mode leak). * fix(cli): force-expand compact groups on terminal subagent in live phase too Resolved comment 3203286936 codified the design intent that `SubagentScrollbackSummary` "fires in BOTH live and committed phases" to bridge `unregisterForeground`'s post-delete panel-snapshot drop and the parent turn committing. Non-compact mode honored that contract (terminal subagents render the summary inline whenever they appear in `inlineToolCalls`), but compact mode still gated `hasCommittedTerminalSubagent` on `!isPending`, so a foreground subagent finishing mid-turn under compact mode produced NOTHING inline until the parent committed — exactly the gap the bridge was meant to close. Drop the `!isPending` arm and rename `hasCommittedTerminalSubagent` → `hasTerminalSubagent`. The force-expand now applies to terminal subagents in either phase; compact-mode users see the same outcome line non-compact users already get. Mirrors `SubagentExecutionRenderer`'s ungated terminal-summary path and `mergeCompactToolGroups.isForceExpandGroup`'s no-isPending-gate preprocessing rule. Tests: - Flip "compact mode: live group with completed subagent stays compact" → "force-expands so the summary bridges the panel-snapshot drop". Update rationale to reflect post-QwenLM#3921 reality (panel evicts terminal foreground rows immediately). - Add "compact mode: live mixed group with terminal subagent + sibling force-expands and renders both" — covers the bridge in mixed groups. - Update two stale `hasCommittedTerminalSubagent` cross-references in `mergeCompactToolGroups.{ts,test.ts}` comments.
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
…etion (QwenLM#3921) In BackgroundTaskRegistry.unregisterForeground(), emitStatusChange was called before agents.delete(). This meant the React refresh callback (rebuilds snapshot via getAll()) still saw the entry in the Map with status='running'. After deletion, no second status-change fired, so React state never updated — the entry remained visible as '1 local agent' in the footer pill and the timer kept ticking in the dialog. Swap the order: delete first, then emit. Now getAll() in the callback returns an empty snapshot for the removed entry, and React correctly drops it from the UI.
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
…Change emit (QwenLM#3919) * fix(cli,core): isPending gate on subagent scrollback summary + post-delete statusChange emit Two follow-ups from PR QwenLM#3909 review. 1. **Re-introduce `isPending` gate on `SubagentExecutionRenderer`'s scrollback summary** (Copilot finding on PRRT_kwDOPB-92c6AUQHn). The verbose inline frame retirement collapsed `SubagentExecutionRenderer` to "render the summary whenever a subagent reaches a terminal status" — but with `isPending` removed in QwenLM#3909, that fired in BOTH live (pendingHistoryItems) AND committed (Static) phases. Live-phase rendering duplicated the row LiveAgentPanel already paints below the composer until the parent turn committed. Add `isPending` back to `ToolMessageProps` purely as a gate for this one render path: the summary fires only when `!isPending` (committed). `ToolGroupMessage` forwards the flag (it kept the prop on its own interface for upstream compat the whole time). Test gap closed by the new `live (isPending) terminal subagent → no scrollback summary (panel owns the row)` case. 2. **Emit `statusChange` AFTER delete in `unregisterForeground`** (Copilot finding on PRRT_kwDOPB-92c6AUQGc + the panel-only reconciliation it spawned). The shared snapshot in `useBackgroundTaskView` only refreshes on `statusChange`, and `unregisterForeground` previously fired exactly once — BEFORE delete — so the snapshot froze with the agent as "running" while `registry.get()` returned undefined. Result: `BackgroundTasksDialog` list mode showed a ghost "running" row with cancel hints whose `x` was a no-op, contradicting what the panel already showed (synthesized neutral terminal). Fire `statusChange` a second time AFTER `agents.delete()` so snapshot consumers see the registry-less state and stop surfacing the agent. The first emit still mirrors complete/fail/cancel/finalize ordering (callbacks that re-read `registry.get` see the entry); the second emit is the new contract for snapshot-based views. React batches the two resulting setState calls into one re-render so consumers re-render exactly once. Updated the existing "emits status change before removing the entry" test to capture both emits and explicitly assert that the second observes the registry-less state. Added a sibling test covering the post-delete `getAll()` count. Coverage: 190 passing tests across core + cli (background-view + ToolMessage + ToolGroupMessage + useBackgroundTaskView). * fix(cli,core): compact-mode terminal subagent expansion + statusChange context flag Five review findings on PR QwenLM#3919: 1. **Compact mode bypassed the scrollback summary** (gpt-5.5 via /qreview, ToolGroupMessage:324). `ToolGroupMessage` returns `CompactToolGroupDisplay` before the ToolMessage path when `compactMode === true`, so the new `isPending` gate on `SubagentExecutionRenderer` only protected the expanded path — committed terminal subagents in compact mode never reached `SubagentScrollbackSummary` and the LiveAgentPanel → committed- summary handoff broke for users who turned compact mode on. Force-expand the group when `!isPending` AND any tool call has a terminal `task_execution` resultDisplay. Stay compact while the parent turn is still live (`isPending`) — the panel below the composer owns that surface and an inline summary would duplicate it. Coverage: 4 new ToolGroupMessage cases (compact + completed-committed expands; compact + running-live stays compact; compact + completed-live stays compact; compact + failed-committed expands). 2. **Snapshot-coupled comment in `packages/core`** (Copilot, background-tasks.ts:292). The comment named CLI/UI consumers (`useBackgroundTaskView`, `BackgroundTasksDialog`) and asserted React batching guarantees from a core file. Reword to "snapshot-style consumers that re-pull `getAll()` from inside the callback" and drop the framework-specific batching claim. 3. **Two-phase emit needed an explicit signal** (Copilot, background-tasks.ts:283). Emitting `statusChange` twice without distinguishing the phases forced consumers to either do duplicate work or risk persisting a stale `entry` from the second callback. Add an optional second arg `context?: { removed?: boolean }` to `BackgroundStatusChangeCallback`; the post-delete emit passes `{ removed: true }` so consumers can disambiguate without re-querying the registry. Backwards compatible — existing callbacks ignore the new arg. Tests updated to assert both `mock.calls[0][1] === undefined` and `mock.calls[1][1] === { removed: true }`. 4. **`isPending` doc clarified** (Copilot, ToolMessage.tsx:507). Made the default semantics explicit: omitted/undefined is treated as committed (not pending); live-area renderers MUST pass `true` explicitly to suppress the scrollback summary. 5. (4 of the threads were duplicate Copilot fires of QwenLM#2 + QwenLM#3.) Coverage: 219 test files / 3369 passing across cli/ui + core/agents. * docs(cli): update ToolGroupMessageProps.isPending JSDoc The previous prop comment claimed `isPending` was "not consumed by the group body" — true at the time, but the body now reads it for two real purposes (compact-mode gating + forwarding to ToolMessage). Update the doc so future callers / tests don't treat it as legacy. Addresses Copilot finding on PRRT_kwDOPB-92c6AYE0V. * fix(cli): hide live-phase subagent tool entries — LiveAgentPanel owns the row User report: with compact mode OFF, a running subagent shows up twice — once as the parent tool group's `task` row (status icon + name + description), once as the LiveAgentPanel row beneath the composer. Same agent, two surfaces, redundant. Filter `task_execution` tool entries out of the expanded `ToolGroupMessage` while `isPending=true` so the panel is the single source of truth for in-flight subagents. The entry returns once the parent turn commits (`isPending=false`), letting `SubagentScrollbackSummary` land inside the parent's tool group as a persistent audit trail. Exception: subagents with a pending approval still render, because the focus-routed banner / queued marker is the only inline surface that lets users answer the prompt without opening the dialog. If a group is purely panel-owned (e.g. a single Task call with no sibling tools), the entire `ToolGroupMessage` returns `null` so an empty bordered container doesn't float above the panel. Coverage: +4 ToolGroupMessage cases — running entry hidden in live phase / mixed group keeps siblings / pending-approval entry still renders / committed entry comes back for the audit trail. * refactor(cli): tighten subagent-tool helper naming + ANSI-safe scrollback summary Self-audit + independent review found 5 cleanup items on the live-phase hide path; all addressed in one commit since none are behavioral changes: 1. **Move `allEntriesPanelOwned` short-circuit BEFORE `showCompact`** so a pure-subagent group in compact mode is also hidden during the live phase (previously CompactToolGroupDisplay rendered a single summary line above the panel — a mild duplicate on top of what the non-compact path already fixed). 2. **Rename `isLiveSubagentTool` → `isSubagentToolEntry`.** The helper identifies a tool's resultDisplay shape; it doesn't check live-state. The previous name conflated "predicate" with "use case" and read as if it returned true only during the live phase. 3. **DRY up `hasCommittedTerminalSubagent`** to use `isSubagentToolEntry` instead of inlining its own type-narrowing. 4. **ANSI-escape `subagentName` / `taskDescription` / `terminateReason`** in `SubagentScrollbackSummary`. Same threat model as the panel rows and HistoryItemDisplay — these strings come from subagent config (user-authored) and LLM output and could carry terminal control sequences. The stats fields (tool count / duration / tokens) flow through trusted formatters and don't need escaping. 5. **Doc comments updated** to reflect the four real responsibilities of `isPending` on `ToolGroupMessageProps` (hide pure groups, force-expand committed compact, per-tool filter, forward to ToolMessage), to clarify that the keyboard-focused subagent id can point at a hidden tool harmlessly (the iterator returns `null` before the focus prop is computed), and to drop the redundant "EXCEPT" clause on the per-tool filter in favor of a single sentence. Coverage unchanged: 251 passing tests across messages / background-view / core/agents; broader 3374-test sweep clean; TS clean on both cli and core packages. * fix(cli,core): address 3 critical review findings + ANSI/doc cleanups Three real bugs flagged by gpt-5.5 via /qreview, plus 4 doc / sanitization nits from Copilot. All 7 threads close together since they share the same surfaces. ## Critical fixes 1. **Foreground subagents disappeared mid-parent-turn** (PRRT_kwDOPB-92c6AYvL9). Post-QwenLM#3921 swap-order, `unregisterForeground` drops the entry from the panel snapshot the moment the subagent finishes. The previous round's `!isPending` gate on `SubagentScrollbackSummary` then suppressed the inline summary too, leaving the user with nothing on screen for the run until the parent committed. - Drop the `!isPending` gate — `unregisterForeground` already removes the row from the panel, so the inline summary can fire in BOTH live and committed phases without duplicating it. - Tighten the `ToolGroupMessage` live-phase hide so it only filters `running` / `paused` / `background` task entries (`isPanelOwnedSubagentTool`), not terminal ones. Terminal entries pass through immediately so the summary lands. - The "panel-owned" predicate is now distinct from the broader "subagent tool entry" predicate (`isSubagentToolEntry`) and the "terminal subagent" predicate (`isTerminalSubagentTool`); each usage site picks the one it actually means. 2. **Compact mode dropped the scrollback summary** (PRRT_kwDOPB-92c6AYvLw). Force-expanding the group made the container go through the expanded path, but `ToolMessage`'s own compact-mode gate (`!compactMode || forceShowResult ? renderer : 'none'`) still suppressed the result block, so `SubagentScrollbackSummary` never rendered for compact-mode users. Pass `forceShowResult={true}` for terminal subagent tool entries so the result block is always rendered. 3. **`mergeCompactToolGroups.isForceExpandGroup` didn't know about terminal subagents** (PRRT_kwDOPB-92c6AYvMC). The committed- history preprocessor merged adjacent tool_groups before render, so a terminal `task_execution` group could be absorbed into a compact batch (its `tool_use_summary` label dropped), and the render-time force-expand check never got a chance to override. Mirror the `hasCommittedTerminalSubagent` predicate inside `isForceExpandGroup` so preprocessing and rendering agree. ## Doc / sanitization nits - `BackgroundStatusChangeCallback` doc now lists every emitter (register / complete / fail / cancel / finalizeCancelled / finalizeCancellationIfPending / abandon / unregisterForeground / reset) and groups them by ordering camp (keeps-the-entry vs removes-the-entry — `reset` joins `unregisterForeground` in the delete-then-emit camp). - ANSI-escape `data.subagentName` in the focus-holder banner and the queued marker (`SubagentExecutionRenderer`) — same threat model as the panel rows and `SubagentScrollbackSummary`. ## Coverage delta - New ToolMessage case: live-phase terminal subagent now renders inline (replaces the prior "no scrollback summary" assertion that was the symptom of the AYvL9 bug). - New ToolGroupMessage cases: terminal subagent in live phase renders inline; `forceShowResult=true` propagates for terminal subagent tools (mock now exposes the prop). - New mergeCompactToolGroups parametrized cases: terminal subagent in any of completed / failed / cancelled stays its own batch. 280 tests pass across cli messages + utils + background-view + core/agents. TS clean. * fix(cli): drop `'paused'` arm from isPanelOwnedSubagentTool — not in AgentResultDisplay union CI Lint failed with TS2367: the previous round's `isPanelOwnedSubagentTool` checked for `status === 'paused'` but `AgentResultDisplay.status` (the tool-result-side type) only carries `'running' | 'completed' | 'failed' | 'cancelled' | 'background'`. The `'paused'` status lives on the registry-side `BackgroundTaskStatus` union and is only ever surfaced through `LiveAgentPanel` directly, never through a `task_execution` payload. Drop the dead arm and add a comment so a future "let's also check paused here" doesn't get re-introduced. * fix(cli): apply panel-ownership filter once before compact-mode decision Mixed live groups (running subagent + sibling tool) leaked the panel-owned subagent into `CompactToolGroupDisplay`'s count and `getActiveTool` selection, because `showCompact` returned BEFORE the inline `.map()` filter ran. Compact-mode users would see e.g. `task × 2 Delegate task to subagent` even though LiveAgentPanel already owned the subagent row below the composer. Derive `inlineToolCalls` once via `useMemo` immediately after the existing hook block and use it consistently for the compact summary, sizing math, and the render map. The early-return for "all-entries-panel-owned" collapses into `inlineToolCalls.length === 0` (gated on `isPending` so the legacy empty-input committed-phase snapshot is preserved). Remove the inner `.map()` filter — the upstream derivation already excluded the same entries. JSDoc updates: - `ToolGroupMessageProps.isPending` now describes the real flow (build inlineToolCalls / force-expand / forward to ToolMessage for parity). - `ToolMessageProps.isPending` is documented as forwarded-but-inert (`SubagentExecutionRenderer` doesn't gate on it; the live-phase filter and the unconditional terminal summary do the actual work). Regression test: live mixed group in compact mode → sibling wins active-tool, count collapses to 1, no `× 2` suffix, no subagent description in the header. Addresses Copilot review comments 3205262972 / 3205263020 (doc/code mismatch) and gpt-5.5 critical 3205288299 (compact-mode leak). * fix(cli): force-expand compact groups on terminal subagent in live phase too Resolved comment 3203286936 codified the design intent that `SubagentScrollbackSummary` "fires in BOTH live and committed phases" to bridge `unregisterForeground`'s post-delete panel-snapshot drop and the parent turn committing. Non-compact mode honored that contract (terminal subagents render the summary inline whenever they appear in `inlineToolCalls`), but compact mode still gated `hasCommittedTerminalSubagent` on `!isPending`, so a foreground subagent finishing mid-turn under compact mode produced NOTHING inline until the parent committed — exactly the gap the bridge was meant to close. Drop the `!isPending` arm and rename `hasCommittedTerminalSubagent` → `hasTerminalSubagent`. The force-expand now applies to terminal subagents in either phase; compact-mode users see the same outcome line non-compact users already get. Mirrors `SubagentExecutionRenderer`'s ungated terminal-summary path and `mergeCompactToolGroups.isForceExpandGroup`'s no-isPending-gate preprocessing rule. Tests: - Flip "compact mode: live group with completed subagent stays compact" → "force-expands so the summary bridges the panel-snapshot drop". Update rationale to reflect post-QwenLM#3921 reality (panel evicts terminal foreground rows immediately). - Add "compact mode: live mixed group with terminal subagent + sibling force-expands and renders both" — covers the bridge in mixed groups. - Update two stale `hasCommittedTerminalSubagent` cross-references in `mergeCompactToolGroups.{ts,test.ts}` comments.
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.
Summary
emitStatusChangeandagents.deleteinBackgroundTaskRegistry.unregisterForeground()— now delete-first, emit-second.refresh()→getAll()) to still see the foreground entry in the Map withstatus='running'. After deletion, no second status-change fired, so React state never updated. The entry lingered as "1 local agent" in the footer pill and the timer kept ticking in the dialog.background-tasks.tsand the updated test assertion inbackground-tasks.test.ts.Validation
Commands run:
Prompts / inputs used: "用Explore agent帮我找一下项目里有哪些vitest.config文件" (in both old and fixed versions via tmux TUI)
Expected result: After the foreground sub-agent completes, the footer pill should not show "1 local agent" and the dialog timer should stop.
Observed result:
YOLO 模式 · 1 local agentafter agent completes. Bug reproduced.YOLO modewith no "local agent" after agent completes. Bug fixed.Quickest reviewer verification path: Run
npm run bundle && node dist/cli.js --approval-mode yolo, send a message that triggers a foreground agent, observe the footer pill disappears after the agent completes.Evidence:
Before (old version) — agent completed but status bar still shows "1 local agent":
After (fixed version) — agent completed, status bar clean:
Scope / Risk
registry.get(agentId)during the event will getundefinedinstead of the entry. The current UI callback (refreshFromRegistry = () => refresh()) usesgetAll()(notget()) and is unaffected.complete()which keeps the entry in the Map — not affected by this change).Testing Matrix
Testing matrix notes:
Linked Issues / Bugs
No linked issues
🤖 Generated with Qwen Code