diff --git a/ui/desktop/src/acp/__tests__/gooseSessionNotifications.test.ts b/ui/desktop/src/acp/__tests__/gooseSessionNotifications.test.ts index 36e0ad3a0..be651ae8a 100644 --- a/ui/desktop/src/acp/__tests__/gooseSessionNotifications.test.ts +++ b/ui/desktop/src/acp/__tests__/gooseSessionNotifications.test.ts @@ -29,6 +29,7 @@ function makeState(): AdapterState { return { messages: [message('u1', 'user'), message('a1', 'assistant'), message('a2', 'assistant')], localSteerTextByMessageId: new Map(), + toolCallStatesById: new Map(), }; } @@ -130,6 +131,7 @@ describe('applyGooseSessionNotification', () => { const state: AdapterState = { messages: [message('u1', 'user')], localSteerTextByMessageId: new Map(), + toolCallStatesById: new Map(), }; const changes = applyGooseSessionNotification(state, messageUsageNotification('missing')); diff --git a/ui/desktop/src/acp/__tests__/sessionNotificationAdapter.test.ts b/ui/desktop/src/acp/__tests__/sessionNotificationAdapter.test.ts index d63b855ba..321f7a4ab 100644 --- a/ui/desktop/src/acp/__tests__/sessionNotificationAdapter.test.ts +++ b/ui/desktop/src/acp/__tests__/sessionNotificationAdapter.test.ts @@ -1,13 +1,23 @@ import type { GooseSessionNotification_unstable } from '@aaif/goose-sdk'; import type { RequestPermissionRequest, SessionNotification } from '@agentclientprotocol/sdk'; import { describe, expect, it } from 'vitest'; -import type { Message, NotificationEvent } from '../../types/message'; +import { getToolResponses, type Message, type NotificationEvent } from '../../types/message'; import { createAcpSessionNotificationAdapter, type AcpChatStateChange, } from '../sessionNotificationAdapter'; const SESSION_ID = 'session-1'; +const DEFAULT_TOOL_CALL = { + sessionUpdate: 'tool_call', + toolCallId: 'tool-1', + title: 'Read file', + status: 'pending', +} as const; +const DEFAULT_TOOL_CALL_UPDATE = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-1', +} as const; function acpUpdate(update: SessionNotification['update']): SessionNotification { return { @@ -414,28 +424,154 @@ describe('createAcpSessionNotificationAdapter', () => { }); }); - it('uses failed tool response text content when raw output is absent', () => { + it('preserves content from an unfinished tool call update', () => { const adapter = createAcpSessionNotificationAdapter(); + const result = { type: 'text' as const, text: 'Intermediate result' }; + const content = [{ type: 'content' as const, content: result }]; - const failedToolStateChanges = adapter.apply( - acpUpdate({ - sessionUpdate: 'tool_call_update', - toolCallId: 'tool-1', - status: 'failed', - title: 'Read file', - content: [ - { - type: 'content', - content: { type: 'text', text: 'file not found' }, - }, - ], - }) + adapter.apply(acpUpdate(DEFAULT_TOOL_CALL)); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, content })); + + const messages = expectOnlyMessagesChange( + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, status: 'completed' })) ); - const messages = expectOnlyMessagesChange(failedToolStateChanges); - expect(firstContent(messages[0])).toMatchObject({ + expect(messages).toHaveLength(2); + expect(firstContent(messages[1])).toMatchObject({ type: 'toolResponse', id: 'tool-1', + metadata: { + title: 'Read file', + status: 'completed', + }, + toolResult: { + status: 'success', + value: { + content: [result], + isError: false, + }, + }, + }); + }); + + it('replaces earlier content with later content', () => { + const adapter = createAcpSessionNotificationAdapter(); + const initialResult = { type: 'text' as const, text: 'First result' }; + const replacementResult = { type: 'text' as const, text: 'Second result' }; + const initialContent = [{ type: 'content' as const, content: initialResult }]; + const replacementContent = [{ type: 'content' as const, content: replacementResult }]; + + adapter.apply(acpUpdate(DEFAULT_TOOL_CALL)); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, content: initialContent })); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, content: replacementContent })); + + const messages = expectOnlyMessagesChange( + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, status: 'completed' })) + ); + + expect(firstContent(messages[1])).toMatchObject({ + type: 'toolResponse', + id: 'tool-1', + metadata: { + content: replacementContent, + }, + toolResult: { + status: 'success', + value: { + content: [replacementResult], + isError: false, + }, + }, + }); + }); + + it('clears earlier content with an empty content update', () => { + const adapter = createAcpSessionNotificationAdapter(); + const result = { type: 'text' as const, text: 'Intermediate result' }; + const content = [{ type: 'content' as const, content: result }]; + + adapter.apply(acpUpdate(DEFAULT_TOOL_CALL)); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, content })); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, content: [] })); + + const messages = expectOnlyMessagesChange( + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, status: 'completed' })) + ); + + expect(firstContent(messages[1])).toMatchObject({ + type: 'toolResponse', + id: 'tool-1', + metadata: { + content: [], + }, + toolResult: { + status: 'success', + value: { + content: [], + isError: false, + }, + }, + }); + }); + + it('preserves unsupported content without rendering it', () => { + const adapter = createAcpSessionNotificationAdapter(); + const diff = { + type: 'diff' as const, + path: '/tmp/file.txt', + oldText: 'old content', + newText: 'new content', + }; + const terminalReference = { + type: 'terminal' as const, + terminalId: 'terminal-1', + }; + const content = [diff, terminalReference]; + + adapter.apply(acpUpdate(DEFAULT_TOOL_CALL)); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, content })); + + const messages = expectOnlyMessagesChange( + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, status: 'completed' })) + ); + + expect(firstContent(messages[1])).toMatchObject({ + type: 'toolResponse', + id: 'tool-1', + metadata: { + content, + }, + toolResult: { + status: 'success', + value: { + content: [], + isError: false, + }, + }, + }); + }); + + it('uses unfinished content for a failed response when raw output is absent', () => { + const adapter = createAcpSessionNotificationAdapter(); + const result = { type: 'text' as const, text: 'file not found' }; + const content = [{ type: 'content' as const, content: result }]; + + adapter.apply(acpUpdate(DEFAULT_TOOL_CALL)); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, content })); + + const messages = expectOnlyMessagesChange( + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, status: 'failed' })) + ); + + expect(messages).toHaveLength(2); + expect(firstContent(messages[1])).toMatchObject({ + type: 'toolResponse', + id: 'tool-1', + metadata: { + title: 'Read file', + status: 'failed', + content, + }, toolResult: { status: 'error', error: 'file not found', @@ -443,6 +579,85 @@ describe('createAcpSessionNotificationAdapter', () => { }); }); + it('keeps interleaved tool call state isolated by ID', () => { + const adapter = createAcpSessionNotificationAdapter(); + const toolOneResult = { type: 'text' as const, text: 'First tool result' }; + const toolTwoResult = { type: 'text' as const, text: 'Second tool result' }; + const toolOneContent = [{ type: 'content' as const, content: toolOneResult }]; + const toolTwoContent = [{ type: 'content' as const, content: toolTwoResult }]; + + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL, title: 'First tool' })); + adapter.apply( + acpUpdate({ ...DEFAULT_TOOL_CALL, toolCallId: 'tool-2', title: 'Second tool' }) + ); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, content: toolOneContent })); + adapter.apply( + acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, toolCallId: 'tool-2', content: toolTwoContent }) + ); + + let messages = expectOnlyMessagesChange( + adapter.apply( + acpUpdate({ + ...DEFAULT_TOOL_CALL_UPDATE, + toolCallId: 'tool-2', + status: 'completed', + }) + ) + ); + const toolTwoResponse = messages + .flatMap(getToolResponses) + .find((response) => response.id === 'tool-2'); + + expect(toolTwoResponse).toMatchObject({ + metadata: { + title: 'Second tool', + content: toolTwoContent, + }, + toolResult: { + status: 'success', + value: { + content: [toolTwoResult], + }, + }, + }); + + messages = expectOnlyMessagesChange( + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, status: 'completed' })) + ); + const toolOneResponse = messages + .flatMap(getToolResponses) + .find((response) => response.id === 'tool-1'); + + expect(toolOneResponse).toMatchObject({ + metadata: { + title: 'First tool', + content: toolOneContent, + }, + toolResult: { + status: 'success', + value: { + content: [toolOneResult], + }, + }, + }); + }); + + it('does not create a duplicate response for repeated completed updates', () => { + const adapter = createAcpSessionNotificationAdapter(); + + adapter.apply(acpUpdate(DEFAULT_TOOL_CALL)); + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, status: 'completed' })); + + const messages = expectOnlyMessagesChange( + adapter.apply(acpUpdate({ ...DEFAULT_TOOL_CALL_UPDATE, status: 'completed' })) + ); + const responses = messages + .flatMap(getToolResponses) + .filter((response) => response.id === 'tool-1'); + + expect(responses).toHaveLength(1); + }); + it('maps in-progress tool message notifications', () => { const adapter = createAcpSessionNotificationAdapter(); @@ -583,9 +798,7 @@ describe('createAcpSessionNotificationAdapter', () => { status: { type: 'progress', message: 'Still working' }, }) ); - expect(progressStateChanges).toEqual([ - { type: 'progressMessage', message: 'Still working' }, - ]); + expect(progressStateChanges).toEqual([{ type: 'progressMessage', message: 'Still working' }]); }); }); diff --git a/ui/desktop/src/acp/adapter/shared.ts b/ui/desktop/src/acp/adapter/shared.ts index 20ab5d71e..389aff824 100644 --- a/ui/desktop/src/acp/adapter/shared.ts +++ b/ui/desktop/src/acp/adapter/shared.ts @@ -18,8 +18,11 @@ export type AcpChatStateChange = export interface AdapterState { messages: Message[]; localSteerTextByMessageId: Map; + toolCallStatesById: Map; } +export type ToolCallState = Omit; + export interface GooseMessageMeta { messageId?: string; created?: number; diff --git a/ui/desktop/src/acp/adapter/tools.ts b/ui/desktop/src/acp/adapter/tools.ts index 7a571bc49..c0bcb2473 100644 --- a/ui/desktop/src/acp/adapter/tools.ts +++ b/ui/desktop/src/acp/adapter/tools.ts @@ -18,9 +18,12 @@ import { rawInputToArguments, toolIdentity, type ToolIdentity, + type ToolCallState, } from './shared'; export function applyToolCall(state: AdapterState, update: ToolCall): AcpChatStateChange[] { + updateToolCallState(state, update); + const gooseMeta = getGooseMessageMeta(update); const message = getOrCreateAssistantMessageForUpdate(state, gooseMeta); @@ -56,33 +59,58 @@ export function applyToolCallUpdate( state: AdapterState, update: ToolCallUpdate ): AcpChatStateChange[] { - if (update.status !== 'completed' && update.status !== 'failed') { + const toolCallState = updateToolCallState(state, update); + const isFinished = toolCallState.status === 'completed' || toolCallState.status === 'failed'; + + if (!isFinished) { const notificationChange = toolNotificationChange(update); return notificationChange ? [notificationChange] : []; } if (hasToolResponse(state, update.toolCallId)) { + state.toolCallStatesById.delete(update.toolCallId); return messagesChange(state); } const gooseMeta = getGooseMessageMeta(update); const message = getOrCreateToolResponseMessageForUpdate(state, gooseMeta); const identity = toolIdentity(update); - const metadata = toolResponseMetadata(update, identity); + const metadata = toolResponseMetadata(toolCallState, identity); message.content.push({ type: 'toolResponse', id: update.toolCallId, toolResult: - update.status === 'failed' - ? { status: 'error', error: toolError(update) } - : { status: 'success', value: toolResultValue(update, mcpAppMetadata(update)) }, + toolCallState.status === 'failed' + ? { status: 'error', error: toolError(toolCallState) } + : { + status: 'success', + value: toolResultValue(toolCallState, mcpAppMetadata(update)), + }, ...(metadata ? { metadata } : {}), }); + state.toolCallStatesById.delete(update.toolCallId); return messagesChange(state); } +function updateToolCallState( + state: AdapterState, + update: ToolCall | ToolCallUpdate +): ToolCallState { + const toolCallState = mergeToolCallState(state.toolCallStatesById.get(update.toolCallId), update); + state.toolCallStatesById.set(update.toolCallId, toolCallState); + return toolCallState; +} + +function mergeToolCallState( + previous: ToolCallState | undefined, + update: ToolCall | ToolCallUpdate +): ToolCallState { + const { _meta: _ignoredMeta, ...toolCallStateUpdate } = update; + return { ...previous, ...toolCallStateUpdate }; +} + function getOrCreateAssistantMessageForUpdate( state: AdapterState, gooseMeta: GooseMessageMeta diff --git a/ui/desktop/src/acp/sessionNotificationAdapter.ts b/ui/desktop/src/acp/sessionNotificationAdapter.ts index adebd8c5e..ac34a6bfa 100644 --- a/ui/desktop/src/acp/sessionNotificationAdapter.ts +++ b/ui/desktop/src/acp/sessionNotificationAdapter.ts @@ -37,6 +37,7 @@ export function createAcpSessionNotificationAdapter( const state: AdapterState = { messages: initialMessages.map(cloneMessage), localSteerTextByMessageId: new Map(localSteerTextByMessageId), + toolCallStatesById: new Map(), }; return {