Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@
* 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 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';
Expand Down Expand Up @@ -88,7 +96,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<AgentInteractive, typeof IDLE>();
Expand Down Expand Up @@ -191,19 +199,76 @@ 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 () => {
Comment thread
yiliang114 marked this conversation as resolved.
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');
});

// 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: false },
])(
'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();
}

// 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);
const app = await renderWithView('agent-a');
Expand Down
39 changes: 7 additions & 32 deletions packages/cli/src/ui/components/agent-view/AgentComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -71,7 +70,6 @@ export const AgentComposer: React.FC<AgentComposerProps> = ({ agentId }) => {
setAgentInputBufferText,
setAgentTabBarFocused,
setAgentApprovalMode,
setAgentMessageQueue,
appendToAgentMessageQueue,
} = useAgentViewActions();
const agent = agents.get(agentId);
Expand Down Expand Up @@ -198,39 +196,16 @@ export const AgentComposer: React.FC<AgentComposerProps> = ({ 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<readonly string[] | null>(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();
Expand Down
157 changes: 127 additions & 30 deletions packages/cli/src/ui/contexts/AgentViewContext.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -34,6 +35,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
Expand All @@ -59,11 +83,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();
Expand Down Expand Up @@ -113,11 +133,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?: (
Expand Down Expand Up @@ -174,11 +190,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?: (
Expand Down Expand Up @@ -237,11 +249,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?: (
Expand Down Expand Up @@ -287,11 +295,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?: (
Expand Down Expand Up @@ -334,11 +338,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?: (
Expand Down Expand Up @@ -384,3 +384,100 @@ 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() {
const config = makeConfig();
const probeActions: {
registerAgent?: (
agentId: string,
a: AgentInteractive,
modelId: string,
color: string,
modelName?: string,
) => 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 <Text>a:[{queue.join(',')}]</Text>;
}

const app = render(
<AgentViewProvider config={config}>
<Probe />
</AgentViewProvider>,
);
return { app, probeActions };
}

const seedAndQueue = async (
probeActions: ReturnType<typeof renderQueuedFailedAgent>['probeActions'],
agent: AgentInteractive,
) => {
await act(async () => {
probeActions.registerAgent?.('agent-a', agent, 'm', 'c', undefined);
});
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();
await seedAndQueue(probeActions, agent);

expect(agent.enqueueMessage).not.toHaveBeenCalled();
expect(app.lastFrame()).toContain('a:[]');
});

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();
await seedAndQueue(probeActions, agent);

expect(agent.enqueueMessage).not.toHaveBeenCalled();
expect(app.lastFrame()).toContain('a:[]');
});
});
Loading
Loading