diff --git a/packages/sdk-typescript/src/daemon/DaemonHttpError.ts b/packages/sdk-typescript/src/daemon/DaemonHttpError.ts index d863b9374a1..63a9ee2c5bd 100644 --- a/packages/sdk-typescript/src/daemon/DaemonHttpError.ts +++ b/packages/sdk-typescript/src/daemon/DaemonHttpError.ts @@ -23,3 +23,55 @@ export class DaemonHttpError extends Error { this.body = body; } } + +// Kept local (instead of reusing `isRecord` from `acpTransportUtils.ts` or +// `ui/utils.ts`) so this leaf module stays dependency-free: those modules +// pull the ACP route table / UI helpers into the budgeted browser bundles. +function getErrorBodyRecord( + body: unknown, +): Record | undefined { + return typeof body === 'object' && body !== null && !Array.isArray(body) + ? (body as Record) + : undefined; +} + +/** + * Type guard for the daemon's `GET /session/:id/subagents/:toolCallId` 404 + * contract: `{ code: 'session_not_found', sessionId, toolCallId? }`. Pass + * `toolCallId` to require the body to identify that specific missing agent + * (a session-level 404 carries no identifying `toolCallId`); omit it to + * accept both. + */ +export function isSubagentSessionNotFound( + error: unknown, + toolCallId?: string, +): boolean { + if (!(error instanceof DaemonHttpError) || error.status !== 404) { + return false; + } + const body = getErrorBodyRecord(error.body); + if (body?.['code'] !== 'session_not_found') return false; + return toolCallId === undefined || body['toolCallId'] === toolCallId; +} + +/** + * Type guard for the session-level variant of that same 404 contract: the + * daemon could not find the parent session itself, so the body carries + * `code: 'session_not_found'` with no identifying `toolCallId` (an + * explicitly `null` id is treated the same as an absent one). + * + * A missing parent session is not the only producer: a multi-workspace + * daemon answers this same shape while the owning workspace entry is + * merely not active (for example draining before removal, or transitioning + * to a replacement runtime), which the daemon treats as reversible. Treat + * this error as recoverable, not as proof the session is permanently gone. + */ +export function isSessionLevelNotFound(error: unknown): boolean { + if (!(error instanceof DaemonHttpError) || error.status !== 404) { + return false; + } + const body = getErrorBodyRecord(error.body); + if (body?.['code'] !== 'session_not_found') return false; + const toolCallId = body['toolCallId']; + return toolCallId === undefined || toolCallId === null; +} diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index b474a85cdea..55e4f1355e4 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -23,6 +23,10 @@ export { type RestoreSessionRequest, type SubscribeOptions, } from './DaemonClient.js'; +export { + isSessionLevelNotFound, + isSubagentSessionNotFound, +} from './DaemonHttpError.js'; // Transport abstraction layer export { DaemonTransportClosedError } from './DaemonTransport.js'; export type { diff --git a/packages/sdk-typescript/test/unit/isSubagentSessionNotFound.test.ts b/packages/sdk-typescript/test/unit/isSubagentSessionNotFound.test.ts new file mode 100644 index 00000000000..541f78e912e --- /dev/null +++ b/packages/sdk-typescript/test/unit/isSubagentSessionNotFound.test.ts @@ -0,0 +1,161 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + DaemonHttpError, + isSessionLevelNotFound, + isSubagentSessionNotFound, +} from '../../src/daemon/DaemonHttpError.js'; + +const missingAgentBody = { + code: 'session_not_found', + sessionId: 'session-1', + toolCallId: 'call-1', +}; + +describe('isSubagentSessionNotFound', () => { + it('matches a 404 whose body identifies the missing agent', () => { + expect( + isSubagentSessionNotFound( + new DaemonHttpError(404, missingAgentBody, 'not found'), + 'call-1', + ), + ).toBe(true); + }); + + it('matches a session-level 404 when no toolCallId is required', () => { + expect( + isSubagentSessionNotFound( + new DaemonHttpError( + 404, + { code: 'session_not_found', sessionId: 'session-1' }, + 'not found', + ), + ), + ).toBe(true); + }); + + it.each([ + ['non-DaemonHttpError', new Error('not found'), 'call-1'], + [ + 'non-404 status', + new DaemonHttpError(500, missingAgentBody, 'server error'), + 'call-1', + ], + [ + 'missing code', + new DaemonHttpError(404, { toolCallId: 'call-1' }, 'not found'), + 'call-1', + ], + [ + 'wrong code', + new DaemonHttpError( + 404, + { ...missingAgentBody, code: 'workspace_not_found' }, + 'not found', + ), + 'call-1', + ], + [ + 'missing toolCallId in body', + new DaemonHttpError( + 404, + { code: 'session_not_found', sessionId: 'session-1' }, + 'not found', + ), + 'call-1', + ], + [ + 'null toolCallId in body', + new DaemonHttpError( + 404, + { code: 'session_not_found', sessionId: 'session-1', toolCallId: null }, + 'not found', + ), + 'call-1', + ], + [ + 'mismatched toolCallId', + new DaemonHttpError(404, missingAgentBody, 'not found'), + 'call-other', + ], + ])('rejects %s', (_label, error, toolCallId) => { + expect(isSubagentSessionNotFound(error, toolCallId as string)).toBe(false); + }); + + it('rejects non-object bodies', () => { + expect( + isSubagentSessionNotFound( + new DaemonHttpError(404, 'session_not_found', 'not found'), + 'call-1', + ), + ).toBe(false); + }); +}); + +describe('isSessionLevelNotFound', () => { + it('matches a 404 whose body has no toolCallId', () => { + expect( + isSessionLevelNotFound( + new DaemonHttpError( + 404, + { code: 'session_not_found', sessionId: 'session-1' }, + 'not found', + ), + ), + ).toBe(true); + }); + + it('matches a 404 whose body carries a null toolCallId', () => { + expect( + isSessionLevelNotFound( + new DaemonHttpError( + 404, + { + code: 'session_not_found', + sessionId: 'session-1', + toolCallId: null, + }, + 'not found', + ), + ), + ).toBe(true); + }); + + it('rejects an agent-level 404', () => { + expect( + isSessionLevelNotFound( + new DaemonHttpError(404, missingAgentBody, 'not found'), + ), + ).toBe(false); + }); + + it('rejects a 404 whose body carries a different code', () => { + expect( + isSessionLevelNotFound( + new DaemonHttpError( + 404, + { code: 'workspace_not_found', sessionId: 'session-1' }, + 'not found', + ), + ), + ).toBe(false); + }); + + it('rejects non-404 and non-matching errors', () => { + expect( + isSessionLevelNotFound( + new DaemonHttpError( + 500, + { code: 'session_not_found', sessionId: 'session-1' }, + 'server error', + ), + ), + ).toBe(false); + expect(isSessionLevelNotFound(new Error('not found'))).toBe(false); + }); +}); diff --git a/packages/web-shell/client/components/MessageList.dom.test.tsx b/packages/web-shell/client/components/MessageList.dom.test.tsx index 4756475e1bc..99fcc2b8fb2 100644 --- a/packages/web-shell/client/components/MessageList.dom.test.tsx +++ b/packages/web-shell/client/components/MessageList.dom.test.tsx @@ -601,7 +601,7 @@ describe('MessageList — turn collapse (DOM)', () => { expect(toggleRow(c, 'u1').getAttribute('aria-expanded')).toBe('false'); }); - it('keeps the final answer when active agents are pinned after it', () => { + it('keeps latest assistant content when active agents are pinned after it', () => { const activeAgent = agentMsg('agent-1'); activeAgent.tools[0]!.status = 'pending'; const c = mount([ @@ -611,16 +611,581 @@ describe('MessageList — turn collapse (DOM)', () => { asstMsg('a1'), ]); - expect( - c - .querySelector('[data-testid="msg-a1"]') - ?.getAttribute('data-assistant-actions'), - ).toBe('true'); + expect(assistantActions(c, 'a1')).toBe('false'); click(toggle(c, 'u1')); expect(has(c, 'a1')).toBe(true); expect(parallelAgentsSummary(c)).toBeNull(); }); + it('does not mark narration as final before agents are summarized', () => { + const activeAgent = agentMsg('agent-1'); + activeAgent.tools[0]!.status = 'pending'; + const c = mount([ + userMsg('u1'), + activeAgent, + agentMsg('agent-2'), + asstMsg('a1'), + ]); + + expect(assistantActions(c, 'a1')).toBe('false'); + + const awaitingSummaryMessages = [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('a1'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + backgroundNotificationMsg('bg-2', 'call-agent-2'), + ]; + rerenderMessages(c, awaitingSummaryMessages); + expect(assistantActions(c, 'a1')).toBe('false'); + + rerenderMessages(c, [...awaitingSummaryMessages, asstMsg('summary')]); + expect(assistantActions(c, 'a1')).toBe('false'); + expect(assistantActions(c, 'summary')).toBe('true'); + }); + + it('keeps actions suppressed for stale agents until they reconcile terminal', () => { + const firstAgent = agentMsg('agent-1'); + const secondAgent = agentMsg('agent-2'); + firstAgent.tools[0]!.status = 'pending'; + secondAgent.tools[0]!.status = 'pending'; + const messages = [userMsg('u1'), firstAgent, secondAgent, asstMsg('a1')]; + const c = mount(messages, undefined, { + catchingUp: true, + isResponding: false, + }); + + rerenderMessages(c, messages, { + catchingUp: false, + isResponding: false, + }); + expect(assistantActions(c, 'a1')).toBe('false'); + + rerenderMessages( + c, + [userMsg('u1'), agentMsg('agent-1'), agentMsg('agent-2'), asstMsg('a1')], + { catchingUp: false, isResponding: false }, + ); + expect(assistantActions(c, 'a1')).toBe('true'); + }); + + it('shows final actions for stale agents in a readonly transcript', () => { + const staleAgent = agentMsg('agent-1'); + staleAgent.tools[0]!.status = 'pending'; + const c = mount([userMsg('u1'), staleAgent, asstMsg('a1')], undefined, { + transcriptRenderMode: 'readonly', + }); + + expect(assistantActions(c, 'a1')).toBe('true'); + }); + + it('keeps final actions for a pending foreground agent in a completed turn', () => { + const foregroundAgent = agentMsg('agent-1'); + foregroundAgent.tools[0]!.status = 'pending'; + foregroundAgent.tools[0]!.args = { + subagent_type: 'explore', + run_in_background: false, + }; + const c = mount([userMsg('u1'), foregroundAgent, asstMsg('a1')]); + + expect(assistantActions(c, 'a1')).toBe('true'); + }); + + it('keeps turn-2 final actions while a turn-1 agent stays pending', () => { + const pendingAgent = agentMsg('agent-1'); + pendingAgent.tools[0]!.status = 'pending'; + const c = mount([ + userMsg('u1'), + pendingAgent, + asstMsg('a1'), + userMsg('u2'), + asstMsg('a2'), + ]); + + expect(assistantActions(c, 'a2')).toBe('true'); + expect(assistantActions(c, 'a1')).toBe('false'); + }); + + it('releases a delayed sibling footer hold only after a bounded grace', () => { + vi.useFakeTimers(); + const firstAgent = agentMsg('agent-1'); + const secondAgent = agentMsg('agent-2'); + firstAgent.tools[0]!.status = 'pending'; + secondAgent.tools[0]!.status = 'pending'; + const c = mount([ + userMsg('u1'), + firstAgent, + secondAgent, + asstMsg('launched'), + ]); + + const secondAgentStillActive = agentMsg('agent-2'); + secondAgentStillActive.tools[0]!.status = 'pending'; + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + secondAgentStillActive, + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('waiting'), + ]); + expect(assistantActions(c, 'waiting')).toBe('false'); + + // The sibling reconciles terminal before its notification arrives: the + // hold stays until the grace expires, in case the notification is merely + // delayed. + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('waiting'), + ]); + expect(assistantActions(c, 'waiting')).toBe('false'); + + act(() => { + vi.advanceTimersByTime(5_000); + }); + expect(assistantActions(c, 'waiting')).toBe('true'); + + // A late notification still re-hides the narration until the summary. + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('waiting'), + backgroundNotificationMsg('bg-2', 'call-agent-2'), + asstMsg('summary'), + ]); + expect(assistantActions(c, 'waiting')).toBe('false'); + expect(assistantActions(c, 'summary')).toBe('true'); + }); + + it('restores final actions when a completed sibling notification is lost', () => { + vi.useFakeTimers(); + const firstAgent = agentMsg('agent-1'); + const secondAgent = agentMsg('agent-2'); + firstAgent.tools[0]!.status = 'pending'; + secondAgent.tools[0]!.status = 'pending'; + const c = mount([ + userMsg('u1'), + firstAgent, + secondAgent, + asstMsg('launched'), + ]); + + const secondAgentStillActive = agentMsg('agent-2'); + secondAgentStillActive.tools[0]!.status = 'pending'; + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + secondAgentStillActive, + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('waiting'), + ]); + expect(assistantActions(c, 'waiting')).toBe('false'); + + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('summary'), + ]); + // The hold survives until the grace expires, in case the sibling + // notification is merely delayed; afterwards the lost notification can + // no longer hide the final answer. + expect(assistantActions(c, 'summary')).toBe('false'); + act(() => { + vi.advanceTimersByTime(5_000); + }); + expect(assistantActions(c, 'summary')).toBe('true'); + }); + + it('does not restart the unmatched-completion grace for a non-agent notification', () => { + vi.useFakeTimers(); + const firstAgent = agentMsg('agent-1'); + const secondAgent = agentMsg('agent-2'); + firstAgent.tools[0]!.status = 'pending'; + secondAgent.tools[0]!.status = 'pending'; + const c = mount([ + userMsg('u1'), + firstAgent, + secondAgent, + asstMsg('launched'), + ]); + + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('summary'), + ]); + // The sibling's completion notification is lost: the hold is bounded. + expect(assistantActions(c, 'summary')).toBe('false'); + + act(() => { + vi.advanceTimersByTime(3_000); + }); + // A non-agent notification must not restart the grace timer; the bound + // still runs from the agent notification. + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('summary'), + monitorNotificationMsg('monitor'), + ]); + expect(assistantActions(c, 'summary')).toBe('false'); + + act(() => { + vi.advanceTimersByTime(2_000); + }); + expect(assistantActions(c, 'summary')).toBe('true'); + }); + + it('keeps a released footer released for a monitor notification after a catch-up cycle', () => { + vi.useFakeTimers(); + const firstAgent = agentMsg('agent-1'); + const secondAgent = agentMsg('agent-2'); + firstAgent.tools[0]!.status = 'pending'; + secondAgent.tools[0]!.status = 'pending'; + const c = mount([ + userMsg('u1'), + firstAgent, + secondAgent, + asstMsg('launched'), + ]); + + const secondAgentStillActive = agentMsg('agent-2'); + secondAgentStillActive.tools[0]!.status = 'pending'; + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + secondAgentStillActive, + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('waiting'), + ]); + expect(assistantActions(c, 'waiting')).toBe('false'); + + // The second sibling reconciles terminal before its notification arrives; + // the hold lasts until the bounded grace expires. + const settled = [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('waiting'), + ]; + rerenderMessages(c, settled); + expect(assistantActions(c, 'waiting')).toBe('false'); + act(() => { + vi.advanceTimersByTime(5_000); + }); + expect(assistantActions(c, 'waiting')).toBe('true'); + + // A catch-up cycle re-establishes the notification baseline, so the + // grace deactivates without any agent notification or turn change. + rerenderMessages(c, settled, { catchingUp: true }); + rerenderMessages(c, settled, { catchingUp: false }); + expect(assistantActions(c, 'waiting')).toBe('true'); + + // A non-agent notification reactivates the coarse grace afterwards but + // cannot change which agents are unmatched, so it must not re-arm the + // expired latch and re-hide the already-released footer. The turn stays + // released: `undefined` means it even collapsed (the narration row is + // folded away), which is the opposite of a re-hide. + rerenderMessages(c, [...settled, monitorNotificationMsg('monitor')], { + catchingUp: false, + }); + expect(assistantActions(c, 'waiting')).not.toBe('false'); + act(() => { + vi.advanceTimersByTime(5_000); + }); + expect(assistantActions(c, 'waiting')).not.toBe('false'); + + // A genuine new lost-completion episode in the same turn still receives + // a full grace window after the catch-up cycle. The model narrates after + // launching agent-3, so the turn's final footer is gated again. + rerenderMessages( + c, + [ + ...settled, + monitorNotificationMsg('monitor'), + agentMsg('agent-3'), + asstMsg('final'), + ], + { catchingUp: false }, + ); + expect(assistantActions(c, 'final')).toBe('false'); + act(() => { + vi.advanceTimersByTime(4_999); + }); + expect(assistantActions(c, 'final')).toBe('false'); + act(() => { + vi.advanceTimersByTime(1); + }); + expect(assistantActions(c, 'final')).toBe('true'); + }); + + it('does not consume the unmatched-completion grace while the turn is still streaming', () => { + vi.useFakeTimers(); + const firstAgent = agentMsg('agent-1'); + const secondAgent = agentMsg('agent-2'); + firstAgent.tools[0]!.status = 'pending'; + secondAgent.tools[0]!.status = 'pending'; + const c = mount( + [userMsg('u1'), firstAgent, secondAgent, asstMsg('launched')], + undefined, + { isResponding: true }, + ); + + // Agent-1 completes mid-response while the model keeps streaming. + const secondAgentStillActive = agentMsg('agent-2'); + secondAgentStillActive.tools[0]!.status = 'pending'; + rerenderMessages( + c, + [ + userMsg('u1'), + agentMsg('agent-1'), + secondAgentStillActive, + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + ], + { isResponding: true }, + ); + + // Agent-2 reconciles terminal with its notification delayed. isResponding + // hides the turn anyway, so streaming past the grace window must not + // consume the budget before the hold can actually gate the footer. + rerenderMessages( + c, + [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + ], + { isResponding: true }, + ); + act(() => { + vi.advanceTimersByTime(10_000); + }); + + // When streaming ends, the full grace window must still be available. + rerenderMessages( + c, + [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + ], + { isResponding: false }, + ); + expect(assistantActions(c, 'launched')).toBe('false'); + act(() => { + vi.advanceTimersByTime(4_999); + }); + expect(assistantActions(c, 'launched')).toBe('false'); + act(() => { + vi.advanceTimersByTime(1); + }); + expect(assistantActions(c, 'launched')).toBe('true'); + }); + + it('releases the footer after grace when the final narration precedes the notification', () => { + vi.useFakeTimers(); + const firstAgent = agentMsg('agent-1'); + const secondAgent = agentMsg('agent-2'); + firstAgent.tools[0]!.status = 'pending'; + secondAgent.tools[0]!.status = 'pending'; + const c = mount([ + userMsg('u1'), + firstAgent, + secondAgent, + asstMsg('launched'), + ]); + + // The sibling's notification lands after the turn's final narration (the + // ordinary placement) and agent-2 reconciles terminal without its own + // notification ever arriving. + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + ]); + expect(assistantActions(c, 'launched')).toBe('false'); + + act(() => { + vi.advanceTimersByTime(5_000); + }); + // Grace expiry must release the footer even though the narration + // precedes the notification; a truly lost notification cannot hide the + // final footer forever. + expect(assistantActions(c, 'launched')).toBe('true'); + }); + + it('gives a later lost-completion episode a full grace after an earlier matched hold', () => { + vi.useFakeTimers(); + const firstAgent = agentMsg('agent-1'); + firstAgent.tools[0]!.status = 'pending'; + const c = mount( + [userMsg('u1'), firstAgent, asstMsg('launched')], + undefined, + { + isResponding: true, + }, + ); + + // Agent-1 completes mid-turn and its (matched) notification lands while + // the model keeps working: a benign hold arms the grace timer. + rerenderMessages( + c, + [ + userMsg('u1'), + agentMsg('agent-1'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + ], + { isResponding: true }, + ); + act(() => { + vi.advanceTimersByTime(5_000); + }); + + // The model launches agent-2 in the same turn and emits the final + // answer; agent-2 is still active, so the footer stays suppressed. + const secondAgent = agentMsg('agent-2'); + secondAgent.tools[0]!.status = 'pending'; + rerenderMessages( + c, + [ + userMsg('u1'), + agentMsg('agent-1'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + secondAgent, + asstMsg('final'), + ], + { isResponding: false }, + ); + expect(assistantActions(c, 'final')).toBe('false'); + + // Agent-2 reconciles terminal but its notification is lost. The genuine + // unmatched episode must receive a fresh grace window even though the + // benign mid-turn hold already expired the latch. + rerenderMessages( + c, + [ + userMsg('u1'), + agentMsg('agent-1'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + agentMsg('agent-2'), + asstMsg('final'), + ], + { isResponding: false }, + ); + expect(assistantActions(c, 'final')).toBe('false'); + act(() => { + vi.advanceTimersByTime(4_999); + }); + expect(assistantActions(c, 'final')).toBe('false'); + act(() => { + vi.advanceTimersByTime(1); + }); + expect(assistantActions(c, 'final')).toBe('true'); + }); + + it('restarts the unmatched-completion grace when another agent notification lands mid-hold', () => { + vi.useFakeTimers(); + const agents = [ + agentMsg('agent-1'), + agentMsg('agent-2'), + agentMsg('agent-3'), + ]; + for (const agent of agents) { + agent.tools[0]!.status = 'pending'; + } + const c = mount([userMsg('u1'), ...agents, asstMsg('launched')]); + + // All three reconcile terminal but only agent-1's notification arrives, + // so the hold arms a 5s bound from T0. + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + agentMsg('agent-3'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + asstMsg('summary'), + ]); + expect(assistantActions(c, 'summary')).toBe('false'); + + act(() => { + vi.advanceTimersByTime(4_000); + }); + + // Agent-2's notification lands mid-hold while agent-3 stays unmatched; + // the bound restarts from the new notification (keep the final narration + // after it so the ordering rule does not mask the grace state). + rerenderMessages(c, [ + userMsg('u1'), + agentMsg('agent-1'), + agentMsg('agent-2'), + agentMsg('agent-3'), + asstMsg('launched'), + backgroundNotificationMsg('bg-1', 'call-agent-1'), + backgroundNotificationMsg('bg-2', 'call-agent-2'), + asstMsg('summary'), + ]); + expect(assistantActions(c, 'summary')).toBe('false'); + + // The original bound (T0+5s) has passed; the restarted one still holds. + act(() => { + vi.advanceTimersByTime(1_000); + }); + expect(assistantActions(c, 'summary')).toBe('false'); + + act(() => { + vi.advanceTimersByTime(4_000); + }); + expect(assistantActions(c, 'summary')).toBe('true'); + }); + + it('keeps completed turn actions while the latest turn awaits agents', () => { + const activeAgent = agentMsg('agent-2'); + activeAgent.tools[0]!.status = 'pending'; + const c = mount([ + userMsg('u1'), + asstMsg('a1'), + userMsg('u2'), + agentMsg('agent-1'), + activeAgent, + asstMsg('a2'), + ]); + + expect(assistantActions(c, 'a1')).toBe('true'); + expect(assistantActions(c, 'a2')).toBe('false'); + }); + it('keeps an automatically expanded terminal group mounted until its delay expires', () => { vi.useFakeTimers(); const firstAgent = agentMsg('agent-1'); diff --git a/packages/web-shell/client/components/MessageList.test.ts b/packages/web-shell/client/components/MessageList.test.ts index 54e1e22801d..621acf4def0 100644 --- a/packages/web-shell/client/components/MessageList.test.ts +++ b/packages/web-shell/client/components/MessageList.test.ts @@ -1100,6 +1100,7 @@ function collapseItems( isResponding: boolean; pendingApprovalCallId: string | null; backgroundSummaryGraceActive: boolean; + waitForUnmatchedAgentCompletions: boolean; automaticallyExpandedAgentKeys: ReadonlySet; enabled: boolean; }> = {}, @@ -1109,6 +1110,8 @@ function collapseItems( isResponding: opts.isResponding ?? false, pendingApprovalCallId: opts.pendingApprovalCallId ?? null, backgroundSummaryGraceActive: opts.backgroundSummaryGraceActive ?? true, + waitForUnmatchedAgentCompletions: + opts.waitForUnmatchedAgentCompletions ?? true, automaticallyExpandedAgentKeys: opts.automaticallyExpandedAgentKeys, enabled: opts.enabled ?? true, }); @@ -1773,6 +1776,31 @@ describe('applyTurnCollapse', () => { expect(currentTurn?.liveStartedAt).toBe(2_000); }); + it('collapses the latest turn once the unmatched-completion grace expires', () => { + const items = groupParallelAgents([ + { ...makeUserMessage('u1'), timestamp: 1_000 }, + makeBackgroundAgentToolGroup('a1', 'completed'), + makeBackgroundAgentToolGroup('a2', 'completed'), + makeAssistantMessage('launched'), + makeBackgroundNotification('notification-a1', 'call-a1'), + ]); + + // While the grace window is active the unmatched sibling keeps the turn + // open. + const held = collapseOf(collapseItems(items), 'u1'); + expect(held?.collapsed).toBe(false); + expect(held?.liveStartedAt).toBe(1_000); + + // Once the grace expires the turn collapses even though the final + // narration precedes the notification. + const released = collapseOf( + collapseItems(items, { waitForUnmatchedAgentCompletions: false }), + 'u1', + ); + expect(released?.collapsed).toBe(true); + expect(released?.liveStartedAt).toBeUndefined(); + }); + it('releases a background summary wait when its grace period expires', () => { const items = groupParallelAgents([ { ...makeUserMessage('u1'), timestamp: 1_000 }, diff --git a/packages/web-shell/client/components/MessageList.tsx b/packages/web-shell/client/components/MessageList.tsx index 5fb6519ef23..1d5a77de789 100644 --- a/packages/web-shell/client/components/MessageList.tsx +++ b/packages/web-shell/client/components/MessageList.tsx @@ -58,6 +58,10 @@ const noopTurnOutputAction = () => undefined; const RELOAD_TRANSCRIPT_DELAY_MS = 120_000; const TURN_LAYOUT_ANIMATION_MS = 180; const AGENT_SUMMARY_COLLAPSE_DELAY_MS = 400; +// A reconciled-terminal sibling whose completion notification is delayed (not +// lost) lands within moments; bound the unmatched-completion hold so a truly +// lost notification cannot hide the final footer forever. +const UNMATCHED_AGENT_COMPLETION_GRACE_MS = 5_000; interface MessageListProps { messages: Message[]; @@ -519,6 +523,12 @@ export interface ApplyTurnCollapseOptions { isResponding: boolean; activeTurnStartedAt?: number; backgroundSummaryGraceActive?: boolean; + /** + * Whether the final turn's collapse should keep waiting for unmatched + * background-agent completions. Pass false once the bounded grace expires + * so a lost notification cannot pin the turn expanded forever. + */ + waitForUnmatchedAgentCompletions?: boolean; automaticallyExpandedAgentKeys?: ReadonlySet; /** * Tool-call id of a pending approval, if any. The turn containing it is @@ -573,7 +583,15 @@ function findFinalAnswerIndex( function collectFinalAssistantTurnIds( items: readonly DisplayItem[], - isResponding: boolean, + { + isResponding, + latestTurnAwaitsAgentSummary, + gateBackgroundAgentStatus, + }: { + isResponding: boolean; + latestTurnAwaitsAgentSummary: boolean; + gateBackgroundAgentStatus: boolean; + }, ): ReadonlyMap { const userIdxs: number[] = []; for (let i = 0; i < items.length; i++) { @@ -585,9 +603,23 @@ function collectFinalAssistantTurnIds( const turnIdByAssistantId = new Map(); for (let k = 0; k < userIdxs.length; k++) { - if (k === userIdxs.length - 1 && isResponding) continue; const start = userIdxs[k]; const end = (k + 1 < userIdxs.length ? userIdxs[k + 1] : items.length) - 1; + if ( + k === userIdxs.length - 1 && + (isResponding || + (gateBackgroundAgentStatus && latestTurnAwaitsAgentSummary)) + ) { + continue; + } + // A turn that still owns active background-agent work is not final, + // whether it is the latest turn or the user has moved on to a newer one. + if ( + gateBackgroundAgentStatus && + turnHasActiveBackgroundAgent(items, start, end) + ) { + continue; + } const turnHead = items[start]; const answerIdx = findFinalAnswerIndex(items, start, end, false); if (answerIdx < 0) continue; @@ -1316,7 +1348,7 @@ export function getSessionTimelineRangeForIndexes( */ /** Does any tool-carrying row in [start, end] hold a tool matching `pred`? */ function someTurnToolCall( - items: DisplayItem[], + items: readonly DisplayItem[], start: number, end: number, pred: (tool: ACPToolCall) => boolean, @@ -1358,6 +1390,20 @@ function turnHasActiveAgent( ); } +function turnHasActiveBackgroundAgent( + items: readonly DisplayItem[], + start: number, + end: number, +): boolean { + return someTurnToolCall( + items, + start, + end, + (tool) => + isBackgroundSubAgentToolCall(tool) && isActiveToolStatus(tool.status), + ); +} + function turnHasAutomaticallyExpandedAgent( items: DisplayItem[], start: number, @@ -1400,22 +1446,21 @@ function backgroundAgentCallIds(item: DisplayItem): string[] { return []; } -function backgroundAgentCompletion( - item: DisplayItem, +function backgroundAgentCompletionForMessage( + message: Message, ): { callId?: string } | null { if ( - item.type !== 'message' || - item.message.role !== 'system' || - item.message.source !== 'background_notification' + message.role !== 'system' || + message.source !== 'background_notification' ) { return null; } const identifiesAgent = - item.message.content + message.content ?.trimStart() .toLowerCase() .startsWith('background agent ') === true; - const data = item.message.data; + const data = message.data; if (typeof data !== 'object' || data === null || Array.isArray(data)) { return identifiesAgent ? {} : null; } @@ -1427,12 +1472,28 @@ function backgroundAgentCompletion( return typeof toolUseId === 'string' ? { callId: toolUseId } : {}; } -function turnAwaitsBackgroundSummary( +function backgroundAgentCompletion( + item: DisplayItem, +): { callId?: string } | null { + return item.type === 'message' + ? backgroundAgentCompletionForMessage(item.message) + : null; +} + +interface BackgroundAgentSummaryState { + lastNotificationIndex: number; + sawAgentCompletion: boolean; + unmatchedAgentCallIds: ReadonlySet; +} + +// Returns null when nothing in this turn's tail awaits a background summary: +// no notification landed, or a terminal turn status precedes every one. +function backgroundAgentSummaryState( items: DisplayItem[], start: number, end: number, - agentNotificationsOnly = false, -): boolean { + agentNotificationsOnly: boolean, +): BackgroundAgentSummaryState | null { let lastNotificationIndex = -1; let latestNotificationAgentCallId: string | undefined; for (let i = end; i > start; i--) { @@ -1442,7 +1503,7 @@ function turnAwaitsBackgroundSummary( item.message.source === 'turn_error' || item.message.source === 'prompt_cancelled' ) { - if (lastNotificationIndex < 0) return false; + if (lastNotificationIndex < 0) return null; continue; } if (item.message.source === 'background_notification') { @@ -1454,7 +1515,7 @@ function turnAwaitsBackgroundSummary( } } } - if (lastNotificationIndex < 0) return false; + if (lastNotificationIndex < 0) return null; let latestAgentLaunchIndex = -1; if (latestNotificationAgentCallId) { @@ -1479,6 +1540,8 @@ function turnAwaitsBackgroundSummary( } } + const unmatchedAgentCallIds = new Set(); + let sawAgentCompletion = false; if (latestAgentLaunchIndex >= 0) { let batchStart = latestAgentLaunchIndex; for (let i = latestAgentLaunchIndex; i >= 0; i--) { @@ -1489,9 +1552,7 @@ function turnAwaitsBackgroundSummary( } } - const unmatchedAgentCallIds = new Set(); const anonymousCompletionCandidates: Array> = []; - let sawAgentCompletion = false; for (let i = batchStart; i <= end; i++) { const item = items[i]; for (const callId of backgroundAgentCallIds(item)) { @@ -1513,19 +1574,39 @@ function turnAwaitsBackgroundSummary( break; } } - if (sawAgentCompletion && unmatchedAgentCallIds.size > 0) { - return true; - } } + return { lastNotificationIndex, sawAgentCompletion, unmatchedAgentCallIds }; +} - for (let i = lastNotificationIndex + 1; i <= end; i++) { +function turnAwaitsBackgroundSummary( + items: DisplayItem[], + start: number, + end: number, + agentNotificationsOnly = false, + waitForUnmatchedAgentCompletions = true, +): boolean { + const state = backgroundAgentSummaryState( + items, + start, + end, + agentNotificationsOnly, + ); + if (!state) return false; + // A lost completion may hold the turn only for the caller's grace window; + // the ordering rule below is reserved for matched notifications whose + // summary narration is still expected. + if (state.sawAgentCompletion && state.unmatchedAgentCallIds.size > 0) { + return waitForUnmatchedAgentCompletions; + } + for (let i = state.lastNotificationIndex + 1; i <= end; i++) { const item = items[i]; if (item.type === 'message' && item.message.role === 'thinking') { return false; } } - - return findFinalAnswerIndex(items, start, end, false) < lastNotificationIndex; + return ( + findFinalAnswerIndex(items, start, end, false) < state.lastNotificationIndex + ); } export function applyTurnCollapse( @@ -1535,6 +1616,7 @@ export function applyTurnCollapse( isResponding, activeTurnStartedAt, backgroundSummaryGraceActive = true, + waitForUnmatchedAgentCompletions = true, automaticallyExpandedAgentKeys, pendingApprovalCallId, includeSubagentToolUsageInMetrics = true, @@ -1575,7 +1657,13 @@ export function applyTurnCollapse( const awaitsBackgroundSummary = isLastTurn && backgroundSummaryGraceActive && - turnAwaitsBackgroundSummary(items, start, end); + turnAwaitsBackgroundSummary( + items, + start, + end, + false, + waitForUnmatchedAgentCompletions, + ); const hasPendingApproval = turnOwnsCallId( items, start, @@ -2571,6 +2659,70 @@ export const MessageList = memo( } return 0; }, [displayItems]); + // Forced-'pending' background-agent statuses only mean "live work" where + // reconciliation can classify them; a static transcript has no live state, + // so a stale card there must not suppress the final footer. + const gateBackgroundAgentStatus = transcriptRenderMode === 'interactive'; + const latestTurnHasActiveBackgroundAgent = useMemo( + () => + gateBackgroundAgentStatus && + turnHasActiveBackgroundAgent( + displayItems, + latestTurnStartIndex, + displayItems.length - 1, + ), + [displayItems, gateBackgroundAgentStatus, latestTurnStartIndex], + ); + const latestTurnBackgroundSummaryState = useMemo( + () => + backgroundAgentSummaryState( + displayItems, + latestTurnStartIndex, + displayItems.length - 1, + true, + ), + [displayItems, latestTurnStartIndex], + ); + // The grace reset/timer keys on the unmatched set, not the raw + // notification id: a notification that cannot change which agents are + // unmatched — an earlier-turn agent completing, or any monitor/shell-task + // notification — must neither restart the bound nor re-arm an expired one. + const latestTurnUnmatchedAgentKey = useMemo(() => { + const callIds = latestTurnBackgroundSummaryState?.unmatchedAgentCallIds; + return callIds && callIds.size > 0 ? [...callIds].sort().join('|') : ''; + }, [latestTurnBackgroundSummaryState]); + const latestTurnHoldsUnmatchedAgentCompletion = + backgroundSummaryGraceActive && + !latestTurnHasActiveBackgroundAgent && + (latestTurnBackgroundSummaryState?.sawAgentCompletion ?? false) && + (latestTurnBackgroundSummaryState?.unmatchedAgentCallIds.size ?? 0) > 0; + const [ + unmatchedCompletionGraceExpired, + setUnmatchedCompletionGraceExpired, + ] = useState(false); + // Re-arm the latch only when the episode changes: the unmatched set or + // the turn itself changed, or streaming ended and the hold can gate the + // footer again. A benign matched-notification hold never consumes the + // latch because the timer below only runs for unmatched completions. + useEffect(() => { + setUnmatchedCompletionGraceExpired(false); + }, [latestTurnUnmatchedAgentKey, latestTurnStartIndex, isResponding]); + useEffect(() => { + // isResponding hides the turn anyway, so the grace must not be + // consumed while streaming; the full window starts when the hold can + // actually gate the final footer. + if (!latestTurnHoldsUnmatchedAgentCompletion || isResponding) return; + const timer = setTimeout( + () => setUnmatchedCompletionGraceExpired(true), + UNMATCHED_AGENT_COMPLETION_GRACE_MS, + ); + return () => clearTimeout(timer); + }, [ + latestTurnHoldsUnmatchedAgentCompletion, + latestTurnUnmatchedAgentKey, + latestTurnStartIndex, + isResponding, + ]); const latestTurnAwaitsAgentSummary = useMemo( () => backgroundSummaryGraceActive && @@ -2579,8 +2731,16 @@ export const MessageList = memo( latestTurnStartIndex, displayItems.length - 1, true, + latestTurnHasActiveBackgroundAgent || + !unmatchedCompletionGraceExpired, ), - [backgroundSummaryGraceActive, displayItems, latestTurnStartIndex], + [ + backgroundSummaryGraceActive, + displayItems, + latestTurnHasActiveBackgroundAgent, + latestTurnStartIndex, + unmatchedCompletionGraceExpired, + ], ); const latestTurnParallelAgentKeys = useMemo(() => { const keys = new Set(); @@ -2716,8 +2876,18 @@ export const MessageList = memo( return null; }, [isResponding, mergedMessages]); const finalAssistantTurnIdByAssistantId = useMemo( - () => collectFinalAssistantTurnIds(displayItems, isResponding), - [displayItems, isResponding], + () => + collectFinalAssistantTurnIds(displayItems, { + isResponding, + latestTurnAwaitsAgentSummary, + gateBackgroundAgentStatus, + }), + [ + displayItems, + gateBackgroundAgentStatus, + isResponding, + latestTurnAwaitsAgentSummary, + ], ); // ── Per-turn collapse ──────────────────────────────────────────────── @@ -2850,6 +3020,9 @@ export const MessageList = memo( isResponding, activeTurnStartedAt, backgroundSummaryGraceActive, + waitForUnmatchedAgentCompletions: + latestTurnHasActiveBackgroundAgent || + !unmatchedCompletionGraceExpired, automaticallyExpandedAgentKeys, pendingApprovalCallId: pendingApproval?.toolCallId ?? null, includeSubagentToolUsageInMetrics, @@ -2901,6 +3074,8 @@ export const MessageList = memo( isResponding, activeTurnStartedAt, backgroundSummaryGraceActive, + latestTurnHasActiveBackgroundAgent, + unmatchedCompletionGraceExpired, pendingApproval?.toolCallId, collapseEnabled, hideFirstUserMessage, diff --git a/packages/web-shell/client/hooks/useMessages.test.ts b/packages/web-shell/client/hooks/useMessages.test.ts index 6e03fb68ae6..c27f5724206 100644 --- a/packages/web-shell/client/hooks/useMessages.test.ts +++ b/packages/web-shell/client/hooks/useMessages.test.ts @@ -3,9 +3,10 @@ import { act, createElement, StrictMode } from 'react'; import { createRoot } from 'react-dom/client'; import { describe, expect, it, vi } from 'vitest'; -import type { - DaemonStatusTranscriptBlock, - DaemonTranscriptBlock, +import { + DaemonHttpError, + type DaemonStatusTranscriptBlock, + type DaemonTranscriptBlock, } from '@qwen-code/sdk/daemon'; import { type BackgroundAgentResolution, @@ -128,10 +129,39 @@ function backgroundAgentBlock(toolCallId: string): DaemonTranscriptBlock { function deferred() { let resolve!: (value: T) => void; - const promise = new Promise((res) => { + let reject!: (error: unknown) => void; + const promise = new Promise((res, rej) => { resolve = res; + reject = rej; }); - return { promise, resolve }; + return { promise, resolve, reject }; +} + +function mountStatusConsumer(options: { allTools?: boolean } = {}) { + const container = document.createElement('div'); + const root = createRoot(container); + const t = (key: string) => key; + function Consumer() { + const messages = useMessages(t); + const status = options.allTools + ? messages + .flatMap((message) => + message.role === 'tool_group' + ? message.tools.map((tool) => tool.status) + : [], + ) + .join(',') + : messages[0]?.role === 'tool_group' + ? messages[0].tools[0]?.status + : undefined; + return createElement('div', null, status); + } + return { + container, + render: () => + root.render(createElement(StrictMode, null, createElement(Consumer))), + unmount: () => root.unmount(), + }; } describe('background agent task reconciliation', () => { @@ -225,28 +255,15 @@ describe('background agent task reconciliation', () => { ).toMatchObject([{ role: 'tool_group', tools: [{ status: 'pending' }] }]); }); - it('queries once for a pending card, ignores streaming, and retries after reconnect', async () => { + it('queries once for a pending card and retains terminal state after reconnect', async () => { hookState.blocks = [backgroundAgentBlock('agent-call')]; hookState.resolveSubagentSession.mockReset(); hookState.resolveSubagentSession.mockResolvedValue( backgroundAgentResolution('completed'), ); - const container = document.createElement('div'); - const root = createRoot(container); - const t = (key: string) => key; - function Consumer() { - const messages = useMessages(t); - const status = - messages[0]?.role === 'tool_group' - ? messages[0].tools[0]?.status - : undefined; - return createElement('div', null, status); - } - - const renderConsumer = () => - root.render(createElement(StrictMode, null, createElement(Consumer))); + const { container, render, unmount } = mountStatusConsumer(); - await act(async () => renderConsumer()); + await act(async () => render()); await vi.waitFor(() => { expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1); expect(hookState.resolveSubagentSession).toHaveBeenCalledWith( @@ -265,18 +282,411 @@ describe('background agent task reconciliation', () => { streaming: true, }), ]; - await act(async () => renderConsumer()); + await act(async () => render()); expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1); hookState.connection.status = 'disconnected'; - await act(async () => renderConsumer()); + await act(async () => render()); hookState.connection.status = 'connected'; - await act(async () => renderConsumer()); + await act(async () => render()); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1); + + await act(async () => unmount()); + }); + + it('retries a running background agent with bounded backoff until terminal', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockResolvedValue( + backgroundAgentResolution('running'), + ); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1); + expect(container.textContent).toBe('pending'); + + // The backoff doubles after each non-terminal result and caps at 60s. + // Asserted synchronously at exact 1-ms boundaries: vi.waitFor would + // advance fake timers by its poll interval and blur them. + for (const [index, delay] of [ + 3_000, 6_000, 12_000, 24_000, 48_000, 60_000, + ].entries()) { + await act(async () => vi.advanceTimersByTimeAsync(delay - 1)); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(index + 1); + await act(async () => vi.advanceTimersByTimeAsync(1)); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(index + 2); + expect(container.textContent).toBe('pending'); + } + + hookState.resolveSubagentSession.mockResolvedValueOnce( + backgroundAgentResolution('completed'), + ); + await act(async () => vi.advanceTimersByTimeAsync(60_000)); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(8); + expect(container.textContent).toBe('completed'); + + await act(async () => unmount()); + vi.useRealTimers(); + }); + + it('treats a missing background agent as terminal after repeated misses', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'agent-call' }, + 'not found', + ), + ); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2); + expect(container.textContent).toBe('failed'); + }); + + await act(async () => unmount()); + vi.useRealTimers(); + }); + + it('recovers a missing background agent that registers after a first miss', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession + .mockRejectedValueOnce( + new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'agent-call' }, + 'not found', + ), + ) + .mockResolvedValueOnce(backgroundAgentResolution('completed')); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2); + expect(container.textContent).toBe('completed'); + }); + + await act(async () => unmount()); + vi.useRealTimers(); + }); + + it('treats a permanent client error as terminal without retrying', async () => { + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError(400, { code: 'invalid_tool_call_id' }, 'bad request'), + ); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => expect(container.textContent).toBe('failed')); + + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1); + await act(async () => unmount()); + }); + + it('retries a rate-limited query instead of failing the agent', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession + .mockRejectedValueOnce( + new DaemonHttpError( + 429, + { code: 'rate_limit_exceeded', retryAfterMs: 500 }, + 'rate limited', + ), + ) + .mockResolvedValueOnce(backgroundAgentResolution('completed')); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + await vi.waitFor(() => + expect(warnSpy).toHaveBeenCalledWith( + '[web-shell] background agent reconciliation retry scheduled', + { + sessionId: 'session-1', + callIds: ['agent-call'], + errors: ['HTTP 429 rate_limit_exceeded'], + }, + ), + ); + + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2); + expect(container.textContent).toBe('completed'); + }); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); + }); + + it('stops retrying and fails agents whose errors exhaust the retry budget', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError(500, { code: 'internal_error' }, 'server error'), + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + // Seven scheduled retries (3s, 6s, 12s, 24s, 48s, then capped at 60s); + // the eighth round exhausts the budget instead of scheduling another. + for (const delay of [ + 3_000, 6_000, 12_000, 24_000, 48_000, 60_000, 60_000, + ]) { + await act(async () => vi.advanceTimersByTimeAsync(delay)); + } + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(8); + expect(container.textContent).toBe('failed'); + }); + expect(warnSpy).toHaveBeenCalledWith( + '[web-shell] background agent reconciliation retry budget exhausted; marking agents failed', + { + sessionId: 'session-1', + callIds: ['agent-call'], + errors: ['HTTP 500 internal_error'], + }, + ); + + // The timer chain has stopped: no further polling. + await act(async () => vi.advanceTimersByTimeAsync(300_000)); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(8); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); + }); + + it('keeps the grown retry delay when other agent notifications arrive', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError(500, { code: 'internal_error' }, 'server error'), + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + + // First retry schedules at the 3s base; it errors, so the next delay + // grows to 6s. + await act(async () => vi.advanceTimersByTimeAsync(3_000)); await vi.waitFor(() => expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2), ); - await act(async () => root.unmount()); + // A notification for another agent triggers an immediate round but must + // not reset the backoff: the following retry is attempt 3 (12s), not a + // fresh 3s base. + hookState.blocks = [ + ...hookState.blocks, + baseBlock({ + id: 'terminal-notification', + kind: 'assistant', + text: '', + meta: { + source: 'background_notification', + backgroundTask: { + kind: 'agent', + taskId: 'other-agent', + status: 'completed', + }, + }, + }), + ]; + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3), + ); + + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3); + await act(async () => vi.advanceTimersByTimeAsync(9_000)); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(4), + ); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); + }); + + it('does not carry missing-agent miss counts across a reconnect', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'agent-call' }, + 'not found', + ), + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + hookState.connection.status = 'disconnected'; + await act(async () => render()); + hookState.connection.status = 'connected'; + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2), + ); + // The post-reconnect miss starts a fresh grace window, so the agent + // stays pending instead of failing on a stale second miss. + expect(container.textContent).toBe('pending'); + + await act(async () => vi.advanceTimersByTimeAsync(60_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3); + expect(container.textContent).toBe('failed'); + }); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); + }); + + it('gives a session-level 404 the same grace as a missing agent', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError( + 404, + { code: 'session_not_found', sessionId: 'session-1' }, + 'not found', + ), + ); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + // The daemon answers this shape for transient workspace states too, so + // a first session-level miss keeps polling instead of failing the card. + expect(container.textContent).toBe('pending'); + + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2); + expect(container.textContent).toBe('failed'); + }); + + await act(async () => unmount()); + vi.useRealTimers(); + }); + + it('recovers a session-level 404 when the next round reaches the daemon', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession + .mockRejectedValueOnce( + new DaemonHttpError( + 404, + { code: 'session_not_found', sessionId: 'session-1' }, + 'not found', + ), + ) + .mockResolvedValueOnce(backgroundAgentResolution('completed')); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2); + expect(container.textContent).toBe('completed'); + }); + + await act(async () => unmount()); + vi.useRealTimers(); + }); + + it('does not fail an agent on a 404 identifying a different tool call', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'other-call' }, + 'not found', + ), + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + // The mismatched 404 is transient; reconciliation must keep polling + // instead of abandoning the card at pending. + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2), + ); + expect(container.textContent).toBe('pending'); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); }); it('reconciles after a terminal agent notification without toolUseId', async () => { @@ -287,21 +697,9 @@ describe('background agent task reconciliation', () => { hookState.resolveSubagentSession .mockResolvedValueOnce(backgroundAgentResolution('running')) .mockResolvedValueOnce(backgroundAgentResolution('completed')); - const container = document.createElement('div'); - const root = createRoot(container); - const t = (key: string) => key; - function Consumer() { - const messages = useMessages(t); - const status = - messages[0]?.role === 'tool_group' - ? messages[0].tools[0]?.status - : undefined; - return createElement('div', null, status); - } - const renderConsumer = () => - root.render(createElement(StrictMode, null, createElement(Consumer))); + const { container, render, unmount } = mountStatusConsumer(); - await act(async () => renderConsumer()); + await act(async () => render()); expect(container.textContent).toBe('pending'); expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1); @@ -321,13 +719,13 @@ describe('background agent task reconciliation', () => { }, }), ]; - await act(async () => renderConsumer()); + await act(async () => render()); await vi.waitFor(() => { expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2); expect(container.textContent).toBe('completed'); }); - await act(async () => root.unmount()); + await act(async () => unmount()); }); it('ignores an older response after the pending Agent set expands', async () => { @@ -342,27 +740,17 @@ describe('background agent task reconciliation', () => { .mockReturnValueOnce(older.promise) .mockReturnValueOnce(newerA.promise) .mockReturnValueOnce(newerB.promise); - const container = document.createElement('div'); - const root = createRoot(container); - const t = (key: string) => key; - function Consumer() { - const statuses = useMessages(t).flatMap((message) => - message.role === 'tool_group' - ? message.tools.map((tool) => tool.status) - : [], - ); - return createElement('div', null, statuses.join(',')); - } - const renderConsumer = () => - root.render(createElement(StrictMode, null, createElement(Consumer))); + const { container, render, unmount } = mountStatusConsumer({ + allTools: true, + }); - await act(async () => renderConsumer()); + await act(async () => render()); expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1); hookState.blocks = [ backgroundAgentBlock('agent-a'), backgroundAgentBlock('agent-b'), ]; - await act(async () => renderConsumer()); + await act(async () => render()); expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3); await act(async () => { @@ -373,7 +761,70 @@ describe('background agent task reconciliation', () => { await act(async () => older.resolve(backgroundAgentResolution('running'))); expect(container.textContent).toBe('completed,completed'); - await act(async () => root.unmount()); + await act(async () => unmount()); + }); + + it('does not consume grace misses for a superseded round', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + const older = deferred(); + const newer = deferred(); + hookState.resolveSubagentSession + .mockReturnValueOnce(older.promise) + .mockReturnValueOnce(newer.promise) + .mockResolvedValue(backgroundAgentResolution('completed')); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1); + + // A notification supersedes the in-flight round while it is pending. + hookState.blocks = [ + ...hookState.blocks, + baseBlock({ + id: 'terminal-notification', + kind: 'assistant', + text: '', + meta: { + source: 'background_notification', + backgroundTask: { + kind: 'agent', + taskId: 'legacy-agent', + status: 'completed', + }, + }, + }), + ]; + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2), + ); + + // Both rounds hit the same unregistered-daemon 404. The stale round's + // 404 lands first, then the live round's: only the live round's miss + // may count, so the stale one must not pre-increment the counter and + // fail the agent on the live round's first miss. + const miss = new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'agent-call' }, + 'not found', + ); + await act(async () => { + older.reject(miss); + newer.reject(miss); + }); + expect(container.textContent).toBe('pending'); + + // The live round keeps polling and reconciles on the next attempt. + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3); + expect(container.textContent).toBe('completed'); + }); + + await act(async () => unmount()); + vi.useRealTimers(); }); it('applies successful resolutions when another pending Agent fails', async () => { @@ -390,25 +841,16 @@ describe('background agent task reconciliation', () => { ? Promise.resolve(backgroundAgentResolution('completed')) : Promise.reject(new Error('not found')), ); - const container = document.createElement('div'); - const root = createRoot(container); - const t = (key: string) => key; - function Consumer() { - const statuses = useMessages(t).flatMap((message) => - message.role === 'tool_group' - ? message.tools.map((tool) => tool.status) - : [], - ); - return createElement('div', null, statuses.join(',')); - } - const renderConsumer = () => - root.render(createElement(StrictMode, null, createElement(Consumer))); + const { container, render, unmount } = mountStatusConsumer({ + allTools: true, + }); - await act(async () => renderConsumer()); + await act(async () => render()); await vi.waitFor(() => { - expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2); + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3); expect(container.textContent).toBe('completed,pending'); }); + expect(hookState.resolveSubagentSession.mock.calls[2]?.[1]).toBe('agent-b'); hookState.resolveSubagentSession.mockImplementation( (_sessionId: string, callId: string) => @@ -432,13 +874,13 @@ describe('background agent task reconciliation', () => { }, }), ]; - await act(async () => renderConsumer()); + await act(async () => render()); await vi.waitFor(() => { expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(4); expect(container.textContent).toBe('completed,completed'); }); - await act(async () => root.unmount()); + await act(async () => unmount()); }); it('ignores an older response after switching sessions', async () => { @@ -451,23 +893,11 @@ describe('background agent task reconciliation', () => { hookState.resolveSubagentSession .mockReturnValueOnce(sessionA.promise) .mockReturnValueOnce(sessionB.promise); - const container = document.createElement('div'); - const root = createRoot(container); - const t = (key: string) => key; - function Consumer() { - const messages = useMessages(t); - const status = - messages[0]?.role === 'tool_group' - ? messages[0].tools[0]?.status - : undefined; - return createElement('div', null, status); - } - const renderConsumer = () => - root.render(createElement(StrictMode, null, createElement(Consumer))); + const { container, render, unmount } = mountStatusConsumer(); - await act(async () => renderConsumer()); + await act(async () => render()); hookState.connection.sessionId = 'session-b'; - await act(async () => renderConsumer()); + await act(async () => render()); expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2); await act(async () => @@ -479,7 +909,410 @@ describe('background agent task reconciliation', () => { ); expect(container.textContent).toBe('completed'); - await act(async () => root.unmount()); + await act(async () => unmount()); hookState.connection.sessionId = 'session-1'; }); + + it('keeps retry tolerance for the completion query of a long-running agent', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockResolvedValue( + backgroundAgentResolution('running'), + ); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + // Drive the agent through the full backoff ladder while it answers + // running; healthy rounds must not consume the failure budget. + for (const delay of [ + 3_000, 6_000, 12_000, 24_000, 48_000, 60_000, 60_000, + ]) { + await act(async () => vi.advanceTimersByTimeAsync(delay)); + } + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(8), + ); + expect(container.textContent).toBe('pending'); + + // The completion notification triggers the final query; a single transient + // 500 there must recover rather than permanently fail the completed agent. + hookState.resolveSubagentSession + .mockRejectedValueOnce( + new DaemonHttpError(500, { code: 'internal_error' }, 'server error'), + ) + .mockResolvedValueOnce(backgroundAgentResolution('completed')); + hookState.blocks = [ + ...hookState.blocks, + baseBlock({ + id: 'terminal-notification', + kind: 'assistant', + text: '', + meta: { + source: 'background_notification', + backgroundTask: { + kind: 'agent', + taskId: 'legacy-agent', + status: 'completed', + }, + }, + }), + ]; + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(9), + ); + expect(container.textContent).toBe('pending'); + + await act(async () => vi.advanceTimersByTimeAsync(60_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(10); + expect(container.textContent).toBe('completed'); + }); + + await act(async () => unmount()); + vi.useRealTimers(); + }); + + it('fails only the erroring agent when the budget exhausts with several pending agents', async () => { + vi.useFakeTimers(); + hookState.blocks = [ + backgroundAgentBlock('agent-a'), + backgroundAgentBlock('agent-b'), + ]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockImplementation( + (_sessionId: string, callId: string) => + callId === 'agent-a' + ? Promise.reject( + new DaemonHttpError( + 500, + { code: 'internal_error' }, + 'server error', + ), + ) + : Promise.resolve(backgroundAgentResolution('running')), + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container, render, unmount } = mountStatusConsumer({ + allTools: true, + }); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2), + ); + expect(container.textContent).toBe('pending,pending'); + + // Agent A errors every round while agent B answers running; once the + // budget exhausts only the erroring agent is marked failed. + for (const delay of [ + 3_000, 6_000, 12_000, 24_000, 48_000, 60_000, 60_000, + ]) { + await act(async () => vi.advanceTimersByTimeAsync(delay)); + } + await vi.waitFor(() => { + expect(container.textContent).toBe('failed,pending'); + }); + + // A's failure shrinks the pending set, opening a fresh retry scope: the + // healthy sibling keeps polling and still reconciles terminal. + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenLastCalledWith( + 'session-1', + 'agent-b', + ), + ); + hookState.resolveSubagentSession.mockImplementation( + (_sessionId: string, callId: string) => + callId === 'agent-b' + ? Promise.resolve(backgroundAgentResolution('completed')) + : Promise.reject( + new DaemonHttpError( + 500, + { code: 'internal_error' }, + 'server error', + ), + ), + ); + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => { + expect(container.textContent).toBe('failed,completed'); + }); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); + }); + + it('does not fail a healthy sibling that blips when another agent exhausts the budget', async () => { + vi.useFakeTimers(); + hookState.blocks = [ + backgroundAgentBlock('agent-a'), + backgroundAgentBlock('agent-b'), + ]; + hookState.resolveSubagentSession.mockReset(); + let bCalls = 0; + hookState.resolveSubagentSession.mockImplementation( + (_sessionId: string, callId: string) => { + if (callId === 'agent-a') { + return Promise.reject( + new DaemonHttpError( + 500, + { code: 'internal_error' }, + 'server error', + ), + ); + } + bCalls += 1; + // Agent B answers running for seven rounds and has a single + // transient blip exactly in the round that exhausts agent A. + return bCalls === 8 + ? Promise.reject( + new DaemonHttpError( + 500, + { code: 'internal_error' }, + 'server error', + ), + ) + : Promise.resolve(backgroundAgentResolution('running')); + }, + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container, render, unmount } = mountStatusConsumer({ + allTools: true, + }); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2), + ); + for (const delay of [ + 3_000, 6_000, 12_000, 24_000, 48_000, 60_000, 60_000, + ]) { + await act(async () => vi.advanceTimersByTimeAsync(delay)); + } + await vi.waitFor(() => { + expect(container.textContent).toBe('failed,pending'); + }); + + // B's own consecutive-error count is one, so it keeps polling on a + // fresh scope and still reconciles terminal. + hookState.resolveSubagentSession.mockImplementation( + (_sessionId: string, callId: string) => + callId === 'agent-b' + ? Promise.resolve(backgroundAgentResolution('completed')) + : Promise.reject( + new DaemonHttpError( + 500, + { code: 'internal_error' }, + 'server error', + ), + ), + ); + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => { + expect(container.textContent).toBe('failed,completed'); + }); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); + }); + + it('keeps the full retry budget when the client identity changes between rounds', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError(500, { code: 'internal_error' }, 'server error'), + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const originalClient = hookState.client; + const { container, render, unmount } = mountStatusConsumer(); + + try { + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + // The embedding host swaps the client identity while staying + // connected. The settled round was already processed, so the hook must + // issue a fresh query instead of attaching a second handler to the + // cached promise and counting the same round twice. + hookState.client = { + resolveSubagentSession: hookState.resolveSubagentSession, + }; + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2), + ); + expect(container.textContent).toBe('pending'); + + // The double-count must not shorten the documented budget: failure + // still takes eight erroring rounds in total. + for (const delay of [6_000, 12_000, 24_000, 48_000, 60_000, 60_000]) { + await act(async () => vi.advanceTimersByTimeAsync(delay)); + } + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(8); + expect(container.textContent).toBe('failed'); + }); + } finally { + await act(async () => unmount()); + hookState.client = originalClient; + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); + + it('re-arms the missing-agent grace after a successful response', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession + .mockRejectedValueOnce( + new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'agent-call' }, + 'not found', + ), + ) + .mockResolvedValueOnce(backgroundAgentResolution('running')) + .mockRejectedValueOnce( + new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'agent-call' }, + 'not found', + ), + ) + .mockRejectedValueOnce( + new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'agent-call' }, + 'not found', + ), + ); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + // Round 2 answers running, resetting the miss counter. + await act(async () => vi.advanceTimersByTimeAsync(3_000)); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2), + ); + expect(container.textContent).toBe('pending'); + + // Round 3 misses again, but the earlier success re-armed the grace, so a + // single fresh miss stays pending instead of failing. + await act(async () => vi.advanceTimersByTimeAsync(6_000)); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3), + ); + expect(container.textContent).toBe('pending'); + + // Round 4 is the second miss of the fresh window and fails the agent. + await act(async () => vi.advanceTimersByTimeAsync(12_000)); + await vi.waitFor(() => { + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(4); + expect(container.textContent).toBe('failed'); + }); + + await act(async () => unmount()); + vi.useRealTimers(); + }); + + it('does not consume the error budget for tolerated missing-agent misses', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + // Alternate in-grace 404 misses and healthy answers for far more rounds + // than the error budget allows; each tolerated miss is followed by a + // success, so none of them may accumulate toward exhaustion. + for (let round = 0; round < 10; round += 1) { + hookState.resolveSubagentSession + .mockRejectedValueOnce( + new DaemonHttpError( + 404, + { code: 'session_not_found', toolCallId: 'agent-call' }, + 'not found', + ), + ) + .mockResolvedValueOnce(backgroundAgentResolution('running')); + } + hookState.resolveSubagentSession.mockResolvedValue( + backgroundAgentResolution('running'), + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + expect(container.textContent).toBe('pending'); + + // Drive nineteen more rounds over the full backoff ladder; each step + // advances exactly one retry so the alternating miss/success rounds + // interleave with React's flushes. + for (let round = 0; round < 19; round += 1) { + await act(async () => + vi.advanceTimersByTimeAsync(Math.min(3_000 * 2 ** round, 60_000)), + ); + } + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(20), + ); + expect(container.textContent).toBe('pending'); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); + }); + + it('starts a fresh retry budget after a reconnect', async () => { + vi.useFakeTimers(); + hookState.blocks = [backgroundAgentBlock('agent-call')]; + hookState.resolveSubagentSession.mockReset(); + hookState.resolveSubagentSession.mockRejectedValue( + new DaemonHttpError(500, { code: 'internal_error' }, 'server error'), + ); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container, render, unmount } = mountStatusConsumer(); + + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), + ); + // Six retries take the budget to one transient error short of exhaustion. + for (const delay of [3_000, 6_000, 12_000, 24_000, 48_000, 60_000]) { + await act(async () => vi.advanceTimersByTimeAsync(delay)); + } + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(7), + ); + expect(container.textContent).toBe('pending'); + + // A reconnect resets the budget, so the next error opens a fresh ladder + // instead of exhausting the pre-disconnect budget. + hookState.connection.status = 'disconnected'; + await act(async () => render()); + hookState.connection.status = 'connected'; + await act(async () => render()); + await vi.waitFor(() => + expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(8), + ); + expect(container.textContent).toBe('pending'); + + await act(async () => unmount()); + warnSpy.mockRestore(); + vi.useRealTimers(); + }); }); diff --git a/packages/web-shell/client/hooks/useMessages.ts b/packages/web-shell/client/hooks/useMessages.ts index 64e07d95919..5d24b856875 100644 --- a/packages/web-shell/client/hooks/useMessages.ts +++ b/packages/web-shell/client/hooks/useMessages.ts @@ -1,5 +1,10 @@ import { useEffect, useMemo, useRef, useState } from 'react'; -import type { DaemonTranscriptBlock } from '@qwen-code/sdk/daemon'; +import { + DaemonHttpError, + isSessionLevelNotFound, + isSubagentSessionNotFound, + type DaemonTranscriptBlock, +} from '@qwen-code/sdk/daemon'; import { useConnection, useTranscriptBlocks, @@ -17,11 +22,31 @@ type Translator = ( vars?: Record, ) => string; +const BACKGROUND_AGENT_RECONCILIATION_RETRY_BASE_MS = 3_000; +const BACKGROUND_AGENT_RECONCILIATION_RETRY_MAX_MS = 60_000; +// Cap on consecutive transient-error rounds for one pending agent. An agent +// whose own error count reaches the cap is marked failed so the UI unblocks, +// while healthy siblings keep polling. Healthy non-terminal responses back +// off on the same delay ladder but do not consume this budget, so a +// long-running agent's completion query keeps its retry tolerance. +const BACKGROUND_AGENT_RECONCILIATION_MAX_ATTEMPTS = 8; +// The daemon registers a launched background task shortly after the tool +// call appears in the transcript, so a first `session_not_found` can race +// registration. Require repeated misses before treating the agent as gone. +const MISSING_BACKGROUND_AGENT_GRACE_MISSES = 2; + export interface BackgroundAgentResolution { status: string; durationMs?: number; } +interface ReconciliationRound { + resolutions: Map; + errors: ReadonlyArray<{ callId: string; error: unknown }>; + notFounds: ReadonlyArray; + succeeded: ReadonlyArray; +} + export function transcriptBlocksToLocalizedMessages( blocks: readonly DaemonTranscriptBlock[], t: Translator, @@ -51,6 +76,16 @@ function getRecord(value: unknown): Record | undefined { : undefined; } +function describeReconciliationError(error: unknown): string { + if (error instanceof DaemonHttpError) { + const code = getRecord(error.body)?.['code']; + return typeof code === 'string' + ? `HTTP ${error.status} ${code}` + : `HTTP ${error.status}`; + } + return error instanceof Error ? error.message : String(error); +} + export function getBackgroundAgentNotificationKey( blocks: readonly DaemonTranscriptBlock[], ): string { @@ -152,27 +187,70 @@ export function useMessagesFromBlocks( () => transcriptBlocksToLocalizedMessages(blocks, t), [blocks, t], ); + const [resolutionSnapshot, setResolutionSnapshot] = useState<{ + sessionId: string; + resolutions: ReadonlyMap; + }>(); + const reconciledMessages = useMemo(() => { + if ( + !resolutionSnapshot || + resolutionSnapshot.sessionId !== connection.sessionId + ) { + return messages; + } + return reconcileBackgroundAgentResolutions( + messages, + resolutionSnapshot.resolutions, + ); + }, [connection.sessionId, messages, resolutionSnapshot]); const pendingBackgroundAgentKey = useMemo( - () => getPendingBackgroundAgentKey(messages), - [messages], + () => getPendingBackgroundAgentKey(reconciledMessages), + [reconciledMessages], ); const backgroundAgentNotificationKey = useMemo( () => getBackgroundAgentNotificationKey(blocks), [blocks], ); - const [resolutionSnapshot, setResolutionSnapshot] = useState<{ - sessionId: string; - resolutions: ReadonlyMap; - }>(); + const [reconciliationAttempt, setReconciliationAttempt] = useState(0); const reconciliationRequestRef = useRef< | { key: string; - request: Promise>; + request: Promise; + processed: boolean; } | undefined >(undefined); + // Keyed by session + pending-agent set (not the notification key) so other + // agents' notifications cannot reset the backoff and keep the retry delay + // pinned at its base. `attempts` drives the backoff delay; `errorAttempts` + // tracks consecutive transient-error rounds per callId toward the budget. + const retryBackoffRef = useRef<{ + key: string; + attempts: number; + errorAttempts: ReadonlyMap; + }>({ + key: '', + attempts: 0, + errorAttempts: new Map(), + }); + const missingAgentMissesRef = useRef(new Map()); + const lastConnectionKeyRef = useRef(undefined); useEffect(() => { + // Miss counts and the retry budget may not span connection transitions: + // a post-reconnect 404 is a fresh race with registration, and a restarted + // daemon deserves a fresh retry ladder rather than the pre-disconnect + // attempt count. + const connectionKey = `${connection.sessionId}:${connection.status}`; + if (lastConnectionKeyRef.current !== connectionKey) { + lastConnectionKeyRef.current = connectionKey; + missingAgentMissesRef.current.clear(); + retryBackoffRef.current = { + key: '', + attempts: 0, + errorAttempts: new Map(), + }; + } const sessionId = connection.sessionId; if ( !sessionId || @@ -192,44 +270,178 @@ export function useMessagesFromBlocks( return; } const requestKey = `${sessionId}:${pendingBackgroundAgentKey}:${backgroundAgentNotificationKey}`; - const existingRequest = reconciliationRequestRef.current; + const retryScopeKey = `${sessionId}:${pendingBackgroundAgentKey}`; + const cachedRound = reconciliationRequestRef.current; const callIds = pendingBackgroundAgentKey.split('|'); - const request = - existingRequest?.key === requestKey - ? existingRequest.request - : Promise.allSettled( - callIds.map(async (callId) => { + for (const callId of [...missingAgentMissesRef.current.keys()]) { + if (!callIds.includes(callId)) { + missingAgentMissesRef.current.delete(callId); + } + } + const roundErrors: Array<{ callId: string; error: unknown }> = []; + const roundNotFounds: string[] = []; + // A settled round that was already processed must not be reused: a + // re-run (for example a client identity swap) would attach a second + // handler and count the same round against the retry budget twice. + const roundIsReusable = + !!cachedRound && cachedRound.key === requestKey && !cachedRound.processed; + const request = roundIsReusable + ? cachedRound.request + : Promise.allSettled( + callIds.map(async (callId) => { + try { const resolution = await workspace.client.resolveSubagentSession( sessionId, callId, ); return [callId, resolution] as const; - }), - ).then((results) => { - const resolutions = new Map(); - results.forEach((result) => { + } catch (error) { if ( - result.status === 'fulfilled' && - isTerminalBackgroundAgentStatus(result.value[1].status) + isSubagentSessionNotFound(error, callId) || + isSessionLevelNotFound(error) + ) { + // Both 404 shapes also occur while the daemon is racing + // registration or the owning workspace runtime is + // transiently inactive, so the active round's handler alone + // counts them against the missing-agent grace. + roundNotFounds.push(callId); + } else if ( + error instanceof DaemonHttpError && + error.status >= 400 && + error.status < 500 && + error.status !== 404 && + error.status !== 429 ) { - resolutions.set(...result.value); + // Permanent client errors never recover on retry; make the + // card terminal so it can stop gating the UI. A 429 is the + // daemon's rate-limit signal and unrecognized 404 shapes + // stay transient, so neither may fail the agent. + return [callId, { status: 'failed' }] as const; } - }); - return resolutions; + roundErrors.push({ callId, error }); + throw error; + } + }), + ).then((results) => { + const resolutions = new Map(); + const succeeded: string[] = []; + results.forEach((result) => { + if (result.status !== 'fulfilled') return; + succeeded.push(result.value[0]); + if (isTerminalBackgroundAgentStatus(result.value[1].status)) { + resolutions.set(result.value[0], result.value[1]); + } }); - reconciliationRequestRef.current = { key: requestKey, request }; + return { + resolutions, + errors: roundErrors, + notFounds: roundNotFounds, + succeeded, + }; + }); + const round = roundIsReusable + ? cachedRound + : { key: requestKey, request, processed: false }; + reconciliationRequestRef.current = round; let active = true; + let retryTimer: ReturnType | undefined; request - .then((resolutions) => { - if (active) { - setResolutionSnapshot((current) => ({ - sessionId, - resolutions: new Map([ - ...(current?.sessionId === sessionId ? current.resolutions : []), - ...resolutions, - ]), - })); + .then(({ resolutions, errors, notFounds, succeeded }) => { + if (!active) return; + round.processed = true; + // Grace-miss accounting lives in the active handler, not the per-call + // closure: a superseded round's late 404 must not consume grace that + // belongs to the live round. + for (const callId of succeeded) { + missingAgentMissesRef.current.delete(callId); + } + for (const callId of notFounds) { + const misses = (missingAgentMissesRef.current.get(callId) ?? 0) + 1; + missingAgentMissesRef.current.set(callId, misses); + if (misses >= MISSING_BACKGROUND_AGENT_GRACE_MISSES) { + resolutions.set(callId, { status: 'failed' }); + } + } + let unresolved = resolutions.size < callIds.length; + const failedCallIds: string[] = []; + let retryDelayMs = BACKGROUND_AGENT_RECONCILIATION_RETRY_BASE_MS; + if (unresolved) { + const previous = + retryBackoffRef.current.key === retryScopeKey + ? retryBackoffRef.current + : { + key: retryScopeKey, + attempts: 0, + errorAttempts: new Map(), + }; + const attempts = previous.attempts + 1; + // Consecutive error rounds are tracked per callId so one agent's + // persistent errors cannot exhaust a shared budget and fail a + // healthy sibling; a callId absent from this round's errors + // resets implicitly. Healthy non-terminal responses back off but + // never consume the budget. + const errorAttempts = new Map(); + for (const entry of errors) { + const count = (previous.errorAttempts.get(entry.callId) ?? 0) + 1; + errorAttempts.set(entry.callId, count); + if (count >= BACKGROUND_AGENT_RECONCILIATION_MAX_ATTEMPTS) { + failedCallIds.push(entry.callId); + resolutions.set(entry.callId, { status: 'failed' }); + } + } + unresolved = resolutions.size < callIds.length; + retryDelayMs = Math.min( + BACKGROUND_AGENT_RECONCILIATION_RETRY_BASE_MS * 2 ** (attempts - 1), + BACKGROUND_AGENT_RECONCILIATION_RETRY_MAX_MS, + ); + retryBackoffRef.current = unresolved + ? { key: retryScopeKey, attempts, errorAttempts } + : { key: retryScopeKey, attempts: 0, errorAttempts: new Map() }; + } else { + retryBackoffRef.current = { + key: retryScopeKey, + attempts: 0, + errorAttempts: new Map(), + }; + } + if (failedCallIds.length > 0) { + console.warn( + '[web-shell] background agent reconciliation retry budget exhausted; marking agents failed', + { + sessionId, + callIds: failedCallIds, + errors: errors.map((entry) => + describeReconciliationError(entry.error), + ), + }, + ); } + setResolutionSnapshot((current) => ({ + sessionId, + resolutions: new Map([ + ...(current?.sessionId === sessionId ? current.resolutions : []), + ...resolutions, + ]), + })); + if (!unresolved) return; + if (errors.length > 0) { + console.warn( + '[web-shell] background agent reconciliation retry scheduled', + { + sessionId, + callIds: errors.map((entry) => entry.callId), + errors: errors.map((entry) => + describeReconciliationError(entry.error), + ), + }, + ); + } + retryTimer = setTimeout(() => { + if (reconciliationRequestRef.current?.request === request) { + reconciliationRequestRef.current = undefined; + } + setReconciliationAttempt((attempt) => attempt + 1); + }, retryDelayMs); }) .catch(() => { if (reconciliationRequestRef.current?.request === request) { @@ -238,6 +450,7 @@ export function useMessagesFromBlocks( }); return () => { active = false; + clearTimeout(retryTimer); }; }, [ backgroundAgentNotificationKey, @@ -246,21 +459,11 @@ export function useMessagesFromBlocks( connection.sessionId, connection.status, pendingBackgroundAgentKey, + reconciliationAttempt, workspace.client, ]); - return useMemo(() => { - if ( - !resolutionSnapshot || - resolutionSnapshot.sessionId !== connection.sessionId - ) { - return messages; - } - return reconcileBackgroundAgentResolutions( - messages, - resolutionSnapshot.resolutions, - ); - }, [connection.sessionId, messages, resolutionSnapshot]); + return reconciledMessages; } export function useMessages(t: Translator): Message[] {