From 162d62bb2844814904384098e70756af68a5a1dc Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Fri, 28 Aug 2026 00:02:45 +0800 Subject: [PATCH 1/4] fix(cli): deliver Agent View queued follow-ups from the provider Queue delivery lived in an AgentComposer effect, but DefaultAppLayout keys the composer by the active view. An agent that settled to idle (or a terminal status) while its tab was unfocused never flushed: queued follow-ups stayed accepted-but-undelivered until the user revisited the tab, or were shown as "queued" forever after a terminal status. Move delivery to the always-mounted AgentViewProvider: one AgentQueueFlusher child per registered agent joins and enqueues the queue when the agent settles to a non-terminal idle, and drops the queue when the agent becomes terminal. The composer now only displays the queue. Delivery stays exactly-once via the queue-identity dedupe, re-anchored from the composer mount scope to the flusher. Fixes #10148 Co-authored-by: Qwen-Coder --- .../AgentComposer.queuedMessages.test.tsx | 50 ++++++++++++- .../components/agent-view/AgentComposer.tsx | 39 ++-------- .../src/ui/contexts/AgentViewContext.test.tsx | 59 ++++++++------- .../cli/src/ui/contexts/AgentViewContext.tsx | 73 ++++++++++++++++++- 4 files changed, 155 insertions(+), 66 deletions(-) diff --git a/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx b/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx index 7767f9c1569..09bb76cf57e 100644 --- a/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx +++ b/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx @@ -10,6 +10,12 @@ * renders AgentComposer with `key={activeView}`, so switching tabs unmounts * the composer; any queue held only in local component state is discarded * and the message is never delivered. + * + * Also covers #10148 -- delivery must not depend on the keyed composer + * being mounted: queued follow-ups are flushed when the agent settles to + * idle even while the user is on another teammate tab, and the queue is + * dropped when the agent reaches a terminal status (it can never be + * delivered then, and a permanent "queued" display would remain otherwise). */ import { render } from 'ink-testing-library'; @@ -88,7 +94,7 @@ function makeFakeAgent(): AgentInteractive { } as unknown as AgentInteractive; } -describe('AgentComposer queued follow-ups (#10069)', () => { +describe('Agent View queued follow-ups (#10069, #10148)', () => { let agentA: AgentInteractive; let agentB: AgentInteractive; const streamingByAgent = new Map(); @@ -191,17 +197,55 @@ describe('AgentComposer queued follow-ups (#10069)', () => { expect(agentA.enqueueMessage).toHaveBeenCalledTimes(1); }); - it('flushes when returning to a tab after the agent becomes idle', async () => { + it('delivers queued follow-ups while the user is on another tab (#10148)', async () => { streamingByAgent.set(agentA, BUSY); const app = await renderWithView('agent-a'); submitCapture.current!('follow-up while away'); + await switchTo(app, 'agent-a'); + expect(app.lastFrame()).toContain('follow-up while away'); + + // The user keeps working on teammate B while A finishes its round. await switchTo(app, 'agent-b'); streamingByAgent.set(agentA, IDLE); - await switchTo(app, 'agent-a'); + await switchTo(app, 'agent-b'); + // Delivery must not wait for the user to revisit A's tab: A's composer + // is unmounted here, only the provider persists (#10148). expect(agentA.enqueueMessage).toHaveBeenCalledTimes(1); expect(agentA.enqueueMessage).toHaveBeenCalledWith('follow-up while away'); + + // Revisiting A later must not re-deliver. + await switchTo(app, 'agent-a'); + expect(agentA.enqueueMessage).toHaveBeenCalledTimes(1); + expect(app.lastFrame()).not.toContain('follow-up while away'); + }); + + it('drops the queue when the agent reaches a terminal status (#10148)', async () => { + streamingByAgent.set(agentA, BUSY); + const app = await renderWithView('agent-a'); + + submitCapture.current!('never delivered'); + await switchTo(app, 'agent-a'); + expect(app.lastFrame()).toContain('never delivered'); + + // A fails while the user is on B; queued follow-ups can never arrive. + await switchTo(app, 'agent-b'); + streamingByAgent.set(agentA, { + status: AgentStatus.FAILED, + streamingState: StreamingState.Idle, + isInputActive: false, + elapsedTime: 0, + lastPromptTokenCount: 0, + }); + await switchTo(app, 'agent-b'); + + expect(agentA.enqueueMessage).not.toHaveBeenCalled(); + + // The undeliverable queue must be dropped, not shown as "queued" forever. + await switchTo(app, 'agent-a'); + expect(app.lastFrame()).not.toContain('never delivered'); + expect(agentA.enqueueMessage).not.toHaveBeenCalled(); }); it('joins multiple queued follow-ups into one prompt after a tab switch', async () => { diff --git a/packages/cli/src/ui/components/agent-view/AgentComposer.tsx b/packages/cli/src/ui/components/agent-view/AgentComposer.tsx index 037c0c8d954..6193267befe 100644 --- a/packages/cli/src/ui/components/agent-view/AgentComposer.tsx +++ b/packages/cli/src/ui/components/agent-view/AgentComposer.tsx @@ -18,10 +18,9 @@ */ import { Box, Text, useStdin } from 'ink'; -import { useCallback, useEffect, useMemo, useRef } from 'react'; +import { useCallback, useEffect, useMemo } from 'react'; import { AgentStatus, - isTerminalStatus, ApprovalMode, APPROVAL_MODES, } from '@qwen-code/qwen-code-core'; @@ -71,7 +70,6 @@ export const AgentComposer: React.FC = ({ agentId }) => { setAgentInputBufferText, setAgentTabBarFocused, setAgentApprovalMode, - setAgentMessageQueue, appendToAgentMessageQueue, } = useAgentViewActions(); const agent = agents.get(agentId); @@ -198,39 +196,16 @@ export const AgentComposer: React.FC = ({ agentId }) => { [buffer, agentTabBarFocused, setAgentTabBarFocused], ); - // ── Message queue (accumulate while streaming, flush as one prompt on idle) ── + // ── Message queue display ── // - // The queue lives in AgentViewContext (keyed by agentId), not in local - // state: the layout keys this component by the active view, so switching - // teammate tabs unmounts it and a local queue would be silently dropped - // before the flush below ever runs (#10069). + // Queued follow-ups live in AgentViewContext (keyed by agentId) and are + // delivered by the provider's always-mounted per-agent flusher, not here: + // the layout keys this component by the active view, so a flush effect in + // this component would only run while the agent's tab is focused (#10069, + // #10148). const messageQueue = agentMessageQueues.get(agentId) ?? EMPTY_MESSAGE_QUEUE; - // When agent becomes idle (and not terminal), flush queued messages. - const flushedQueueRef = useRef(null); - useEffect(() => { - if ( - streamingState === StreamingState.Idle && - messageQueue.length > 0 && - status !== undefined && - !isTerminalStatus(status) - ) { - if (flushedQueueRef.current === messageQueue) return; - flushedQueueRef.current = messageQueue; - const combined = messageQueue.join('\n'); - setAgentMessageQueue(agentId, []); - interactiveAgent?.enqueueMessage(combined); - } - }, [ - streamingState, - messageQueue, - interactiveAgent, - status, - agentId, - setAgentMessageQueue, - ]); - const handleSubmit = useCallback( (text: string) => { const trimmed = text.trim(); diff --git a/packages/cli/src/ui/contexts/AgentViewContext.test.tsx b/packages/cli/src/ui/contexts/AgentViewContext.test.tsx index 894d8220eb4..b8f52bb98f8 100644 --- a/packages/cli/src/ui/contexts/AgentViewContext.test.tsx +++ b/packages/cli/src/ui/contexts/AgentViewContext.test.tsx @@ -34,6 +34,29 @@ function makeConfig(): Config { } as unknown as Config; } +/** + * Minimal AgentInteractive stub. The provider mounts a per-agent queue + * flusher that derives streaming state via useAgentStreamingState, so the + * stub must cover that surface even in storage tests. Status stays + * undefined so the flusher never delivers here (delivery is covered by + * AgentComposer.queuedMessages.test.tsx). + */ +function makeInteractiveAgent(): AgentInteractive { + return { + getCore: () => ({ + runtimeContext: { + getApprovalMode: () => ApprovalMode.DEFAULT, + setApprovalMode: vi.fn(), + }, + }), + getStatus: () => undefined, + getPendingApprovals: () => new Map(), + getLastPromptTokenCount: () => 0, + getEventEmitter: () => undefined, + enqueueMessage: vi.fn(), + } as unknown as AgentInteractive; +} + describe('AgentViewProvider in-process bridges', () => { // Regression guard. The team bridge (useTeamInProcess) was authored but // never mounted in the provider, so teammate TEAMMATE_JOINED events never @@ -59,11 +82,7 @@ describe('AgentViewProvider in-process bridges', () => { it('clears embedded shell focus when switching agent tabs', async () => { const config = makeConfig(); - const interactiveAgent = { - getCore: () => ({ - runtimeContext: { getApprovalMode: () => ApprovalMode.DEFAULT }, - }), - } as AgentInteractive; + const interactiveAgent = makeInteractiveAgent(); function Probe() { const state = useAgentViewState(); @@ -113,11 +132,7 @@ describe('AgentViewProvider in-process bridges', () => { // state change lands in its own commit (the production focus seed is // a keypress in a commit well after the tab switch). const config = makeConfig(); - const interactiveAgent = { - getCore: () => ({ - runtimeContext: { getApprovalMode: () => ApprovalMode.DEFAULT }, - }), - } as AgentInteractive; + const interactiveAgent = makeInteractiveAgent(); const probeActions: { registerAgent?: ( @@ -174,11 +189,7 @@ describe('AgentViewProvider in-process bridges', () => { describe('AgentViewProvider per-agent message queues', () => { it('stores queues per agent and clears them when emptied or unregistered', async () => { const config = makeConfig(); - const interactiveAgent = { - getCore: () => ({ - runtimeContext: { getApprovalMode: () => ApprovalMode.DEFAULT }, - }), - } as AgentInteractive; + const interactiveAgent = makeInteractiveAgent(); const probeActions: { registerAgent?: ( @@ -237,11 +248,7 @@ describe('AgentViewProvider per-agent message queues', () => { it('appends queued messages without losing same-batch updates', async () => { const config = makeConfig(); - const interactiveAgent = { - getCore: () => ({ - runtimeContext: { getApprovalMode: () => ApprovalMode.DEFAULT }, - }), - } as AgentInteractive; + const interactiveAgent = makeInteractiveAgent(); const probeActions: { registerAgent?: ( @@ -287,11 +294,7 @@ describe('AgentViewProvider per-agent message queues', () => { // manager detaching while the user submits), the append must not // resurrect the queue entry the delete just removed. const config = makeConfig(); - const interactiveAgent = { - getCore: () => ({ - runtimeContext: { getApprovalMode: () => ApprovalMode.DEFAULT }, - }), - } as AgentInteractive; + const interactiveAgent = makeInteractiveAgent(); const probeActions: { registerAgent?: ( @@ -334,11 +337,7 @@ describe('AgentViewProvider per-agent message queues', () => { it('clears all queued messages when all agents unregister', async () => { const config = makeConfig(); - const interactiveAgent = { - getCore: () => ({ - runtimeContext: { getApprovalMode: () => ApprovalMode.DEFAULT }, - }), - } as AgentInteractive; + const interactiveAgent = makeInteractiveAgent(); const probeActions: { registerAgent?: ( diff --git a/packages/cli/src/ui/contexts/AgentViewContext.tsx b/packages/cli/src/ui/contexts/AgentViewContext.tsx index 41de61e736d..c34d34b2958 100644 --- a/packages/cli/src/ui/contexts/AgentViewContext.tsx +++ b/packages/cli/src/ui/contexts/AgentViewContext.tsx @@ -25,12 +25,15 @@ import { useState, } from 'react'; import { + isTerminalStatus, type AgentInteractive, type ApprovalMode, type Config, } from '@qwen-code/qwen-code-core'; import { useArenaInProcess } from '../hooks/useArenaInProcess.js'; +import { useAgentStreamingState } from '../hooks/useAgentStreamingState.js'; import { useTeamInProcess } from '../hooks/useTeamInProcess.js'; +import { StreamingState } from '../types.js'; // ─── Types ────────────────────────────────────────────────── @@ -60,7 +63,9 @@ export interface AgentViewState { * Queued follow-up messages per agent (keyed by agentId). Held here — * not in the composer — because the layout keys AgentComposer by the * active view, so switching teammate tabs unmounts the composer and any - * component-local queue would be silently discarded (#10069). + * component-local queue would be silently discarded (#10069). Delivery + * also lives here (AgentQueueFlusher) so queues flush even while the + * agent's tab is unfocused (#10148). */ agentMessageQueues: ReadonlyMap; } @@ -134,6 +139,66 @@ export function useAgentViewActions(): AgentViewActions { return useContext(AgentViewActionsContext) ?? DEFAULT_ACTIONS; } +// ─── Queue delivery ───────────────────────────────────────── + +// Shared empty queue identity so agents without queued messages don't +// allocate on every render. +const EMPTY_MESSAGE_QUEUE: readonly string[] = []; + +/** + * Always-mounted delivery for one registered agent's queued follow-ups. + * + * AgentViewProvider mounts one flusher per registered agentId. Delivery + * cannot live in AgentComposer: the layout renders it keyed by the active + * view, so switching teammate tabs unmounts the composer while the queues + * persist — an agent that settles to idle (or a terminal status) while + * unfocused would otherwise keep accepted-but-undelivered messages forever + * (#10148). + */ +function AgentQueueFlusher({ agentId }: { agentId: string }) { + const { agents, agentMessageQueues } = useAgentViewState(); + const { setAgentMessageQueue } = useAgentViewActions(); + const interactiveAgent = agents.get(agentId)?.interactiveAgent; + const { status, streamingState } = useAgentStreamingState(interactiveAgent); + const messageQueue = agentMessageQueues.get(agentId) ?? EMPTY_MESSAGE_QUEUE; + + // Dedupe by queue identity: effects run twice per commit under + // StrictMode, and the clear below only lands on the next render — without + // this the same queue would be delivered twice. + const flushedQueueRef = useRef(null); + + useEffect(() => { + if (status !== undefined && isTerminalStatus(status)) { + // The agent can never accept these messages now; drop them so the + // display doesn't show undeliverable "queued" follow-ups forever. + if (messageQueue.length > 0) { + setAgentMessageQueue(agentId, []); + } + return; + } + if ( + streamingState === StreamingState.Idle && + messageQueue.length > 0 && + status !== undefined + ) { + if (flushedQueueRef.current === messageQueue) return; + flushedQueueRef.current = messageQueue; + const combined = messageQueue.join('\n'); + setAgentMessageQueue(agentId, []); + interactiveAgent?.enqueueMessage(combined); + } + }, [ + streamingState, + messageQueue, + interactiveAgent, + status, + agentId, + setAgentMessageQueue, + ]); + + return null; +} + // ─── Provider ─────────────────────────────────────────────── interface AgentViewProviderProps { @@ -385,6 +450,12 @@ export function AgentViewProvider({ return ( + {/* Always-mounted queue delivery, one flusher per registered agent + — delivery must not depend on the keyed composer being mounted + (#10148). */} + {[...agents.keys()].map((agentId) => ( + + ))} {children} From 7635da09ed5484787e5da3e647c662c6cf7622ad Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Wed, 2 Sep 2026 21:01:16 +0800 Subject: [PATCH 2/4] fix(cli): deliver queued follow-ups to FAILED agents, drop only at COMPLETED/CANCELLED The Agent View queue flusher dropped pending follow-ups at any terminal status, but FAILED is not terminal for delivery: core's enqueueMessage has no terminal guard and restarts the run loop (agent-interactive.ts, "should survive round errors and recover"), so a failed teammate still processes the queued text. Dropping it silently lost the only copy. Narrow the drop branch to COMPLETED/CANCELLED, where the master abort is tripped or the agent is shut down and delivery is genuinely impossible, and extend the regression test to cover all three terminal statuses. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtk2jph6cv --- .../AgentComposer.queuedMessages.test.tsx | 72 ++++++++++++------- .../cli/src/ui/contexts/AgentViewContext.tsx | 12 ++-- 2 files changed, 53 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx b/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx index 09bb76cf57e..f81e9af3930 100644 --- a/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx +++ b/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx @@ -14,8 +14,9 @@ * Also covers #10148 -- delivery must not depend on the keyed composer * being mounted: queued follow-ups are flushed when the agent settles to * idle even while the user is on another teammate tab, and the queue is - * dropped when the agent reaches a terminal status (it can never be - * delivered then, and a permanent "queued" display would remain otherwise). + * dropped only at COMPLETED/CANCELLED (delivery is genuinely impossible + * there; a FAILED agent still processes queued follow-ups — core's + * enqueueMessage restarts the run loop — so those must be delivered). */ import { render } from 'ink-testing-library'; @@ -221,32 +222,49 @@ describe('Agent View queued follow-ups (#10069, #10148)', () => { expect(app.lastFrame()).not.toContain('follow-up while away'); }); - it('drops the queue when the agent reaches a terminal status (#10148)', async () => { - streamingByAgent.set(agentA, BUSY); - const app = await renderWithView('agent-a'); - - submitCapture.current!('never delivered'); - await switchTo(app, 'agent-a'); - expect(app.lastFrame()).toContain('never delivered'); - - // A fails while the user is on B; queued follow-ups can never arrive. - await switchTo(app, 'agent-b'); - streamingByAgent.set(agentA, { - status: AgentStatus.FAILED, - streamingState: StreamingState.Idle, - isInputActive: false, - elapsedTime: 0, - lastPromptTokenCount: 0, - }); - await switchTo(app, 'agent-b'); - - expect(agentA.enqueueMessage).not.toHaveBeenCalled(); + // Only COMPLETED/CANCELLED are terminal for delivery: a FAILED agent's + // enqueueMessage restarts the run loop (core agent-interactive.ts), so + // its queued follow-ups must be delivered, not dropped. + it.each([ + { status: AgentStatus.COMPLETED, delivered: false }, + { status: AgentStatus.CANCELLED, delivered: false }, + { status: AgentStatus.FAILED, delivered: true }, + ])( + 'handles the queue when the agent reaches $status (#10148)', + async ({ status, delivered }) => { + streamingByAgent.set(agentA, BUSY); + const app = await renderWithView('agent-a'); + + submitCapture.current!('queued at terminal'); + await switchTo(app, 'agent-a'); + expect(app.lastFrame()).toContain('queued at terminal'); + + // The agent reaches the terminal status while the user is on B. + await switchTo(app, 'agent-b'); + streamingByAgent.set(agentA, { + status, + streamingState: StreamingState.Idle, + isInputActive: false, + elapsedTime: 0, + lastPromptTokenCount: 0, + }); + await switchTo(app, 'agent-b'); + + if (delivered) { + expect(agentA.enqueueMessage).toHaveBeenCalledTimes(1); + expect(agentA.enqueueMessage).toHaveBeenCalledWith( + 'queued at terminal', + ); + } else { + expect(agentA.enqueueMessage).not.toHaveBeenCalled(); + } - // The undeliverable queue must be dropped, not shown as "queued" forever. - await switchTo(app, 'agent-a'); - expect(app.lastFrame()).not.toContain('never delivered'); - expect(agentA.enqueueMessage).not.toHaveBeenCalled(); - }); + // Either way the queue is cleared — no permanent "queued" display. + await switchTo(app, 'agent-a'); + expect(app.lastFrame()).not.toContain('queued at terminal'); + expect(agentA.enqueueMessage).toHaveBeenCalledTimes(delivered ? 1 : 0); + }, + ); it('joins multiple queued follow-ups into one prompt after a tab switch', async () => { streamingByAgent.set(agentA, BUSY); diff --git a/packages/cli/src/ui/contexts/AgentViewContext.tsx b/packages/cli/src/ui/contexts/AgentViewContext.tsx index c34d34b2958..5b7c8b92947 100644 --- a/packages/cli/src/ui/contexts/AgentViewContext.tsx +++ b/packages/cli/src/ui/contexts/AgentViewContext.tsx @@ -25,7 +25,7 @@ import { useState, } from 'react'; import { - isTerminalStatus, + AgentStatus, type AgentInteractive, type ApprovalMode, type Config, @@ -168,9 +168,13 @@ function AgentQueueFlusher({ agentId }: { agentId: string }) { const flushedQueueRef = useRef(null); useEffect(() => { - if (status !== undefined && isTerminalStatus(status)) { - // The agent can never accept these messages now; drop them so the - // display doesn't show undeliverable "queued" follow-ups forever. + if (status === AgentStatus.COMPLETED || status === AgentStatus.CANCELLED) { + // COMPLETED/CANCELLED agents can never accept these messages (master + // abort tripped / agent shut down), so drop them — otherwise the + // display shows undeliverable "queued" follow-ups forever. FAILED is + // not terminal for delivery: enqueueMessage has no terminal guard and + // restarts the run loop (core agent-interactive.ts), so a failed + // agent still processes queued follow-ups — fall through and deliver. if (messageQueue.length > 0) { setAgentMessageQueue(agentId, []); } From f1a49d12bfabd99aa2e496159d530180af8b0c40 Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Thu, 3 Sep 2026 01:21:51 +0800 Subject: [PATCH 3/4] fix(cli): gate FAILED queue delivery on recoverability and team ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two FAILED flavors cannot accept queued follow-ups, and delivering to them certifies falsely: - Fatal failure (core sets error, not lastRoundError): the chat was never created or the run loop threw, so enqueueMessage restarts a loop whose runOneRound early-returns on !this.chat — silently consuming the message while settleRoundStatus flips FAILED to IDLE, erasing the failure state (R5-1). - Team-managed teammate: TeamManager tears the agent down synchronously on terminal status (TEAMMATE_EXITED, event bridge detached, per-agent state dropped) and the backend releases its resources, so a delivered follow-up resurrects a deaf agent nobody accounts for (R6-1). Register each agent's source and drop the queue for both flavors. A FAILED arena agent whose round merely errored stays deliverable, as pinned by the #10148 tests. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtkb4cktdd --- .../src/ui/contexts/AgentViewContext.test.tsx | 119 ++++++++++++++++++ .../cli/src/ui/contexts/AgentViewContext.tsx | 51 ++++++-- packages/cli/src/ui/hooks/useTeamInProcess.ts | 5 + 3 files changed, 167 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/ui/contexts/AgentViewContext.test.tsx b/packages/cli/src/ui/contexts/AgentViewContext.test.tsx index b8f52bb98f8..aadd51943c8 100644 --- a/packages/cli/src/ui/contexts/AgentViewContext.test.tsx +++ b/packages/cli/src/ui/contexts/AgentViewContext.test.tsx @@ -9,6 +9,7 @@ import { Text } from 'ink'; import { act, useEffect, useRef } from 'react'; import { describe, it, expect, vi } from 'vitest'; import { + AgentStatus, ApprovalMode, type AgentInteractive, type Config, @@ -383,3 +384,121 @@ describe('AgentViewProvider per-agent message queues', () => { expect(lastFrame()).toContain('a:[]'); }); }); + +describe('AgentQueueFlusher FAILED delivery gate (#10315 review)', () => { + /** + * Stub of an agent that has reached FAILED. `error` set models a fatal + * failure (chat never created / run loop threw — core sets `error`, not + * `lastRoundError`); `error` undefined models a recoverable round failure. + */ + function makeFailedAgent(error: string | undefined): AgentInteractive { + return { + getCore: () => ({ + runtimeContext: { + getApprovalMode: () => ApprovalMode.DEFAULT, + setApprovalMode: vi.fn(), + }, + }), + getStatus: () => AgentStatus.FAILED, + getError: () => error, + getLastRoundError: () => (error === undefined ? 'round boom' : undefined), + getPendingApprovals: () => new Map(), + getLastPromptTokenCount: () => 0, + getEventEmitter: () => undefined, + enqueueMessage: vi.fn(), + } as unknown as AgentInteractive; + } + + function renderQueuedFailedAgent( + agent: AgentInteractive, + source?: 'arena' | 'team', + ) { + const config = makeConfig(); + const probeActions: { + registerAgent?: ( + agentId: string, + a: AgentInteractive, + modelId: string, + color: string, + modelName?: string, + src?: 'arena' | 'team', + ) => void; + setAgentMessageQueue?: (agentId: string, queue: string[]) => void; + } = {}; + + function Probe() { + const state = useAgentViewState(); + const actions = useAgentViewActions(); + probeActions.registerAgent = actions.registerAgent; + probeActions.setAgentMessageQueue = actions.setAgentMessageQueue; + const queue = state.agentMessageQueues.get('agent-a') ?? []; + return a:[{queue.join(',')}]; + } + + const app = render( + + + , + ); + return { app, probeActions, source }; + } + + const seedAndQueue = async ( + probeActions: ReturnType['probeActions'], + agent: AgentInteractive, + source?: 'arena' | 'team', + ) => { + await act(async () => { + probeActions.registerAgent?.( + 'agent-a', + agent, + 'm', + 'c', + undefined, + source, + ); + }); + await act(async () => { + probeActions.setAgentMessageQueue?.('agent-a', ['queued follow-up']); + }); + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setImmediate(resolve)); + }; + + it('drops the queue without delivering when a chat-less FAILED agent (fatal error) settles', async () => { + // Chat-creation failure: enqueueMessage would restart a loop whose + // runOneRound early-returns on `!this.chat`, silently consuming the + // message and settling FAILED → IDLE (erasing the failure state). + const agent = makeFailedAgent('Failed to create chat session'); + const { app, probeActions } = renderQueuedFailedAgent(agent); + await seedAndQueue(probeActions, agent); + + expect(agent.enqueueMessage).not.toHaveBeenCalled(); + expect(app.lastFrame()).toContain('a:[]'); + }); + + it('drops the queue without delivering when a FAILED team-managed agent settles', async () => { + // TeamManager tears a teammate down synchronously on terminal status + // (TEAMMATE_EXITED, event bridge detached, per-agent state dropped — + // "a terminated teammate can never reach IDLE again"), so delivering + // would resurrect a deaf agent. + const agent = makeFailedAgent(undefined); + const { app, probeActions } = renderQueuedFailedAgent(agent, 'team'); + await seedAndQueue(probeActions, agent, 'team'); + + expect(agent.enqueueMessage).not.toHaveBeenCalled(); + expect(app.lastFrame()).toContain('a:[]'); + }); + + it('still delivers to a FAILED arena agent whose round merely errored', async () => { + // Recoverable flavor: no fatal error, not team-managed — core's + // intentionally unguarded enqueueMessage restarts the run loop. + const agent = makeFailedAgent(undefined); + const { app, probeActions } = renderQueuedFailedAgent(agent); + await seedAndQueue(probeActions, agent); + + expect(agent.enqueueMessage).toHaveBeenCalledTimes(1); + expect(agent.enqueueMessage).toHaveBeenCalledWith('queued follow-up'); + expect(app.lastFrame()).toContain('a:[]'); + }); +}); diff --git a/packages/cli/src/ui/contexts/AgentViewContext.tsx b/packages/cli/src/ui/contexts/AgentViewContext.tsx index 5b7c8b92947..1fabc779aa8 100644 --- a/packages/cli/src/ui/contexts/AgentViewContext.tsx +++ b/packages/cli/src/ui/contexts/AgentViewContext.tsx @@ -44,6 +44,13 @@ export interface RegisteredAgent { /** Human-friendly model name (e.g. "GLM 5"). */ modelName?: string; color: string; + /** + * Which in-process surface registered this agent. Team-managed agents are + * owned by TeamManager, which treats every terminal status (FAILED + * included) as final and tears the agent down synchronously — see the + * delivery gate in AgentQueueFlusher. Undefined means arena-owned. + */ + source?: 'arena' | 'team'; } export interface AgentViewState { @@ -80,6 +87,7 @@ export interface AgentViewActions { modelId: string, color: string, modelName?: string, + source?: 'arena' | 'team', ): void; unregisterAgent(agentId: string): void; unregisterAll(): void; @@ -158,7 +166,8 @@ const EMPTY_MESSAGE_QUEUE: readonly string[] = []; function AgentQueueFlusher({ agentId }: { agentId: string }) { const { agents, agentMessageQueues } = useAgentViewState(); const { setAgentMessageQueue } = useAgentViewActions(); - const interactiveAgent = agents.get(agentId)?.interactiveAgent; + const registered = agents.get(agentId); + const interactiveAgent = registered?.interactiveAgent; const { status, streamingState } = useAgentStreamingState(interactiveAgent); const messageQueue = agentMessageQueues.get(agentId) ?? EMPTY_MESSAGE_QUEUE; @@ -168,13 +177,36 @@ function AgentQueueFlusher({ agentId }: { agentId: string }) { const flushedQueueRef = useRef(null); useEffect(() => { - if (status === AgentStatus.COMPLETED || status === AgentStatus.CANCELLED) { - // COMPLETED/CANCELLED agents can never accept these messages (master - // abort tripped / agent shut down), so drop them — otherwise the - // display shows undeliverable "queued" follow-ups forever. FAILED is - // not terminal for delivery: enqueueMessage has no terminal guard and - // restarts the run loop (core agent-interactive.ts), so a failed - // agent still processes queued follow-ups — fall through and deliver. + // A FAILED agent is only deliverable when the failure is a recoverable + // round error on a live, still-listened-to agent. Two FAILED flavors + // cannot accept delivery: + // - Fatal failure (core sets `error`, not `lastRoundError`): the chat + // was never created or the run loop threw. enqueueMessage restarts a + // loop whose runOneRound early-returns on `!this.chat`, silently + // consuming the message while settleRoundStatus flips FAILED → IDLE, + // erasing the failure state (core agent-interactive.ts). + // - Team-managed teammate: TeamManager handles a terminal status + // synchronously in the same STATUS_CHANGE emit — unassigns tasks, + // emits TEAMMATE_EXITED, detaches the event bridge, drops per-agent + // state ("a terminated teammate can never reach IDLE again") — and + // the backend releases the agent's resources, so a delivered + // follow-up would resurrect a deaf agent nobody accounts for. + // A FAILED arena agent whose round merely errored is alive and still + // listened to: core's intentionally unguarded enqueueMessage restarts + // its run loop — fall through and deliver. + const failedUndeliverable = + status === AgentStatus.FAILED && + (interactiveAgent?.getError() !== undefined || + registered?.source === 'team'); + if ( + status === AgentStatus.COMPLETED || + status === AgentStatus.CANCELLED || + failedUndeliverable + ) { + // These agents can never accept the queued messages (master abort + // tripped / agent shut down / fatally failed / torn down by the + // team), so drop them — otherwise the display shows undeliverable + // "queued" follow-ups forever. if (messageQueue.length > 0) { setAgentMessageQueue(agentId, []); } @@ -195,6 +227,7 @@ function AgentQueueFlusher({ agentId }: { agentId: string }) { streamingState, messageQueue, interactiveAgent, + registered, status, agentId, setAgentMessageQueue, @@ -287,6 +320,7 @@ export function AgentViewProvider({ modelId: string, color: string, modelName?: string, + source?: 'arena' | 'team', ) => { registeredIdsRef.current.add(agentId); setAgents((prev) => { @@ -296,6 +330,7 @@ export function AgentViewProvider({ modelId, color, modelName, + source, }); return next; }); diff --git a/packages/cli/src/ui/hooks/useTeamInProcess.ts b/packages/cli/src/ui/hooks/useTeamInProcess.ts index ce794a7e25d..9e4331817a0 100644 --- a/packages/cli/src/ui/hooks/useTeamInProcess.ts +++ b/packages/cli/src/ui/hooks/useTeamInProcess.ts @@ -111,6 +111,10 @@ export function useTeamInProcess( member.name, member.color ?? nextColor(), member.name, + // TeamManager owns this agent's lifecycle: terminal statuses + // (FAILED included) are torn down synchronously, so the queue + // flusher must not deliver to it once failed. + 'team', ); ownedAgentIds.add(member.agentId); } @@ -136,6 +140,7 @@ export function useTeamInProcess( event.name, event.color ?? nextColor(), event.name, + 'team', ); ownedAgentIds.add(event.agentId); return; From 8041a8d0c7e8b2b6d47e6c536f4e8171b4497dd0 Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Thu, 3 Sep 2026 03:59:07 +0800 Subject: [PATCH 4/4] fix(cli): treat all FAILED agents as undeliverable in the queue flusher The recoverable-FAILED fall-through delivered queued follow-ups to an arena agent that InProcessBackend's one-shot terminal watcher had already finalized at the FAILED settle (releaseAgentResources removed the monitor notification route and fired the exit callback), while ArenaManager discards FAILED -> RUNNING (only COMPLETED -> RUNNING revival is sanctioned). The revived round burned tokens outside every record, with monitor notifications dropped and the second settle never re-released. Drop queued follow-ups at every terminal status instead, and remove the now-unused `source` registration plumbing the old gate consulted. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtkhjtuvdq --- .../AgentComposer.queuedMessages.test.tsx | 17 +++--- .../src/ui/contexts/AgentViewContext.test.tsx | 49 +++++----------- .../cli/src/ui/contexts/AgentViewContext.tsx | 57 +++++++------------ packages/cli/src/ui/hooks/useTeamInProcess.ts | 5 -- 4 files changed, 45 insertions(+), 83 deletions(-) diff --git a/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx b/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx index f81e9af3930..ea8dc9655cb 100644 --- a/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx +++ b/packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx @@ -14,9 +14,10 @@ * Also covers #10148 -- delivery must not depend on the keyed composer * being mounted: queued follow-ups are flushed when the agent settles to * idle even while the user is on another teammate tab, and the queue is - * dropped only at COMPLETED/CANCELLED (delivery is genuinely impossible - * there; a FAILED agent still processes queued follow-ups — core's - * enqueueMessage restarts the run loop — so those must be delivered). + * dropped at every terminal status (COMPLETED/CANCELLED/FAILED — a FAILED + * agent has already been released by the backend's one-shot terminal + * watcher, so delivery would revive it outside ArenaManager's records; + * see the gate in AgentViewContext). */ import { render } from 'ink-testing-library'; @@ -222,13 +223,15 @@ describe('Agent View queued follow-ups (#10069, #10148)', () => { expect(app.lastFrame()).not.toContain('follow-up while away'); }); - // Only COMPLETED/CANCELLED are terminal for delivery: a FAILED agent's - // enqueueMessage restarts the run loop (core agent-interactive.ts), so - // its queued follow-ups must be delivered, not dropped. + // Every terminal status is terminal for delivery. For FAILED in + // particular: the backend has already released the agent's resources at + // the FAILED settle (core InProcessBackend.ts) and ArenaManager discards + // FAILED → RUNNING, so the queued follow-ups must be dropped, not + // delivered to a revived round nobody records. it.each([ { status: AgentStatus.COMPLETED, delivered: false }, { status: AgentStatus.CANCELLED, delivered: false }, - { status: AgentStatus.FAILED, delivered: true }, + { status: AgentStatus.FAILED, delivered: false }, ])( 'handles the queue when the agent reaches $status (#10148)', async ({ status, delivered }) => { diff --git a/packages/cli/src/ui/contexts/AgentViewContext.test.tsx b/packages/cli/src/ui/contexts/AgentViewContext.test.tsx index aadd51943c8..9f8f2642282 100644 --- a/packages/cli/src/ui/contexts/AgentViewContext.test.tsx +++ b/packages/cli/src/ui/contexts/AgentViewContext.test.tsx @@ -409,10 +409,7 @@ describe('AgentQueueFlusher FAILED delivery gate (#10315 review)', () => { } as unknown as AgentInteractive; } - function renderQueuedFailedAgent( - agent: AgentInteractive, - source?: 'arena' | 'team', - ) { + function renderQueuedFailedAgent() { const config = makeConfig(); const probeActions: { registerAgent?: ( @@ -421,7 +418,6 @@ describe('AgentQueueFlusher FAILED delivery gate (#10315 review)', () => { modelId: string, color: string, modelName?: string, - src?: 'arena' | 'team', ) => void; setAgentMessageQueue?: (agentId: string, queue: string[]) => void; } = {}; @@ -440,23 +436,15 @@ describe('AgentQueueFlusher FAILED delivery gate (#10315 review)', () => { , ); - return { app, probeActions, source }; + return { app, probeActions }; } const seedAndQueue = async ( probeActions: ReturnType['probeActions'], agent: AgentInteractive, - source?: 'arena' | 'team', ) => { await act(async () => { - probeActions.registerAgent?.( - 'agent-a', - agent, - 'm', - 'c', - undefined, - source, - ); + probeActions.registerAgent?.('agent-a', agent, 'm', 'c', undefined); }); await act(async () => { probeActions.setAgentMessageQueue?.('agent-a', ['queued follow-up']); @@ -470,35 +458,26 @@ describe('AgentQueueFlusher FAILED delivery gate (#10315 review)', () => { // runOneRound early-returns on `!this.chat`, silently consuming the // message and settling FAILED → IDLE (erasing the failure state). const agent = makeFailedAgent('Failed to create chat session'); - const { app, probeActions } = renderQueuedFailedAgent(agent); + const { app, probeActions } = renderQueuedFailedAgent(); await seedAndQueue(probeActions, agent); expect(agent.enqueueMessage).not.toHaveBeenCalled(); expect(app.lastFrame()).toContain('a:[]'); }); - it('drops the queue without delivering when a FAILED team-managed agent settles', async () => { - // TeamManager tears a teammate down synchronously on terminal status - // (TEAMMATE_EXITED, event bridge detached, per-agent state dropped — - // "a terminated teammate can never reach IDLE again"), so delivering - // would resurrect a deaf agent. + it('drops the queue without delivering when a FAILED agent whose round merely errored settles', async () => { + // Recoverable flavor (lastRoundError set, error undefined). Tempting to + // deliver — core's unguarded enqueueMessage does restart the run loop — + // but at the FAILED settle the backend's one-shot watcher already ran + // releaseAgentResources (monitor routing gone) and fired the exit + // callback, and ArenaManager discards FAILED → RUNNING, so the revived + // round would run outside every record (core InProcessBackend.ts / + // ArenaManager.ts). const agent = makeFailedAgent(undefined); - const { app, probeActions } = renderQueuedFailedAgent(agent, 'team'); - await seedAndQueue(probeActions, agent, 'team'); - - expect(agent.enqueueMessage).not.toHaveBeenCalled(); - expect(app.lastFrame()).toContain('a:[]'); - }); - - it('still delivers to a FAILED arena agent whose round merely errored', async () => { - // Recoverable flavor: no fatal error, not team-managed — core's - // intentionally unguarded enqueueMessage restarts the run loop. - const agent = makeFailedAgent(undefined); - const { app, probeActions } = renderQueuedFailedAgent(agent); + const { app, probeActions } = renderQueuedFailedAgent(); await seedAndQueue(probeActions, agent); - expect(agent.enqueueMessage).toHaveBeenCalledTimes(1); - expect(agent.enqueueMessage).toHaveBeenCalledWith('queued follow-up'); + expect(agent.enqueueMessage).not.toHaveBeenCalled(); expect(app.lastFrame()).toContain('a:[]'); }); }); diff --git a/packages/cli/src/ui/contexts/AgentViewContext.tsx b/packages/cli/src/ui/contexts/AgentViewContext.tsx index 1fabc779aa8..aa87639c3f7 100644 --- a/packages/cli/src/ui/contexts/AgentViewContext.tsx +++ b/packages/cli/src/ui/contexts/AgentViewContext.tsx @@ -44,13 +44,6 @@ export interface RegisteredAgent { /** Human-friendly model name (e.g. "GLM 5"). */ modelName?: string; color: string; - /** - * Which in-process surface registered this agent. Team-managed agents are - * owned by TeamManager, which treats every terminal status (FAILED - * included) as final and tears the agent down synchronously — see the - * delivery gate in AgentQueueFlusher. Undefined means arena-owned. - */ - source?: 'arena' | 'team'; } export interface AgentViewState { @@ -87,7 +80,6 @@ export interface AgentViewActions { modelId: string, color: string, modelName?: string, - source?: 'arena' | 'team', ): void; unregisterAgent(agentId: string): void; unregisterAll(): void; @@ -177,36 +169,32 @@ function AgentQueueFlusher({ agentId }: { agentId: string }) { const flushedQueueRef = useRef(null); useEffect(() => { - // A FAILED agent is only deliverable when the failure is a recoverable - // round error on a live, still-listened-to agent. Two FAILED flavors - // cannot accept delivery: - // - Fatal failure (core sets `error`, not `lastRoundError`): the chat - // was never created or the run loop threw. enqueueMessage restarts a - // loop whose runOneRound early-returns on `!this.chat`, silently - // consuming the message while settleRoundStatus flips FAILED → IDLE, - // erasing the failure state (core agent-interactive.ts). - // - Team-managed teammate: TeamManager handles a terminal status - // synchronously in the same STATUS_CHANGE emit — unassigns tasks, - // emits TEAMMATE_EXITED, detaches the event bridge, drops per-agent - // state ("a terminated teammate can never reach IDLE again") — and - // the backend releases the agent's resources, so a delivered - // follow-up would resurrect a deaf agent nobody accounts for. - // A FAILED arena agent whose round merely errored is alive and still - // listened to: core's intentionally unguarded enqueueMessage restarts - // its run loop — fall through and deliver. - const failedUndeliverable = - status === AgentStatus.FAILED && - (interactiveAgent?.getError() !== undefined || - registered?.source === 'team'); + // FAILED is undeliverable in every flavor. At the FAILED settle the + // backend's one-shot terminal watcher has already run + // releaseAgentResources (monitor notification routing removed, owned + // monitors cancelled) and fired the exit callback + // (core InProcessBackend.ts), and ArenaManager sanctions only + // COMPLETED → RUNNING revival — FAILED → RUNNING is discarded — so a + // delivered follow-up would restart the run loop (core's intentionally + // unguarded enqueueMessage) for an agent every record still counts as + // dead: the revived round burns tokens outside ArenaManager's books, + // monitor notifications have no route, and the second settle is never + // re-released or re-reported (the watcher is one-shot). The fatal + // flavor (core sets `error`, not `lastRoundError`) is worse: the chat + // was never created, so the restarted loop's runOneRound + // early-returns on `!this.chat`, silently consuming the message while + // settleRoundStatus flips FAILED → IDLE, erasing the failure state + // (core agent-interactive.ts). Team-managed teammates are likewise + // torn down synchronously by TeamManager on any terminal status. if ( status === AgentStatus.COMPLETED || status === AgentStatus.CANCELLED || - failedUndeliverable + status === AgentStatus.FAILED ) { // These agents can never accept the queued messages (master abort - // tripped / agent shut down / fatally failed / torn down by the - // team), so drop them — otherwise the display shows undeliverable - // "queued" follow-ups forever. + // tripped / agent shut down / failed / torn down), so drop them — + // otherwise the display shows undeliverable "queued" follow-ups + // forever. if (messageQueue.length > 0) { setAgentMessageQueue(agentId, []); } @@ -227,7 +215,6 @@ function AgentQueueFlusher({ agentId }: { agentId: string }) { streamingState, messageQueue, interactiveAgent, - registered, status, agentId, setAgentMessageQueue, @@ -320,7 +307,6 @@ export function AgentViewProvider({ modelId: string, color: string, modelName?: string, - source?: 'arena' | 'team', ) => { registeredIdsRef.current.add(agentId); setAgents((prev) => { @@ -330,7 +316,6 @@ export function AgentViewProvider({ modelId, color, modelName, - source, }); return next; }); diff --git a/packages/cli/src/ui/hooks/useTeamInProcess.ts b/packages/cli/src/ui/hooks/useTeamInProcess.ts index 9e4331817a0..ce794a7e25d 100644 --- a/packages/cli/src/ui/hooks/useTeamInProcess.ts +++ b/packages/cli/src/ui/hooks/useTeamInProcess.ts @@ -111,10 +111,6 @@ export function useTeamInProcess( member.name, member.color ?? nextColor(), member.name, - // TeamManager owns this agent's lifecycle: terminal statuses - // (FAILED included) are torn down synchronously, so the queue - // flusher must not deliver to it once failed. - 'team', ); ownedAgentIds.add(member.agentId); } @@ -140,7 +136,6 @@ export function useTeamInProcess( event.name, event.color ?? nextColor(), event.name, - 'team', ); ownedAgentIds.add(event.agentId); return;