diff --git a/packages/a2a-server/src/agent/task.ts b/packages/a2a-server/src/agent/task.ts index 1defbdd36c..c969e601c3 100644 --- a/packages/a2a-server/src/agent/task.ts +++ b/packages/a2a-server/src/agent/task.ts @@ -27,7 +27,8 @@ import { type ToolCallConfirmationDetails, type Config, type UserTierId, - type AnsiOutput, + type ToolLiveOutput, + isSubagentProgress, EDIT_TOOL_NAMES, processRestorableToolCalls, } from '@google/gemini-cli-core'; @@ -336,11 +337,13 @@ export class Task { private _schedulerOutputUpdate( toolCallId: string, - outputChunk: string | AnsiOutput, + outputChunk: ToolLiveOutput, ): void { let outputAsText: string; if (typeof outputChunk === 'string') { outputAsText = outputChunk; + } else if (isSubagentProgress(outputChunk)) { + outputAsText = JSON.stringify(outputChunk); } else { outputAsText = outputChunk .map((line) => line.map((token) => token.text).join('')) diff --git a/packages/cli/src/ui/components/MainContent.tsx b/packages/cli/src/ui/components/MainContent.tsx index fbcc962663..7386a246e7 100644 --- a/packages/cli/src/ui/components/MainContent.tsx +++ b/packages/cli/src/ui/components/MainContent.tsx @@ -34,6 +34,7 @@ export const MainContent = () => { const confirmingTool = useConfirmingTool(); const showConfirmationQueue = confirmingTool !== null; + const confirmingToolCallId = confirmingTool?.tool.callId; const scrollableListRef = useRef>(null); @@ -41,7 +42,7 @@ export const MainContent = () => { if (showConfirmationQueue) { scrollableListRef.current?.scrollToEnd(); } - }, [showConfirmationQueue, confirmingTool]); + }, [showConfirmationQueue, confirmingToolCallId]); const { pendingHistoryItems, diff --git a/packages/cli/src/ui/components/messages/SubagentProgressDisplay.test.tsx b/packages/cli/src/ui/components/messages/SubagentProgressDisplay.test.tsx new file mode 100644 index 0000000000..e8b67301ad --- /dev/null +++ b/packages/cli/src/ui/components/messages/SubagentProgressDisplay.test.tsx @@ -0,0 +1,193 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { render, cleanup } from '../../../test-utils/render.js'; +import { SubagentProgressDisplay } from './SubagentProgressDisplay.js'; +import type { SubagentProgress } from '@google/gemini-cli-core'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { Text } from 'ink'; + +vi.mock('ink-spinner', () => ({ + default: () => ⠋, +})); + +describe('', () => { + afterEach(() => { + vi.restoreAllMocks(); + cleanup(); + }); + + it('renders correctly with description in args', async () => { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [ + { + id: '1', + type: 'tool_call', + content: 'run_shell_command', + args: '{"command": "echo hello", "description": "Say hello"}', + status: 'running', + }, + ], + }; + + const { lastFrame, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders correctly with displayName and description from item', async () => { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [ + { + id: '1', + type: 'tool_call', + content: 'run_shell_command', + displayName: 'RunShellCommand', + description: 'Executing echo hello', + args: '{"command": "echo hello"}', + status: 'running', + }, + ], + }; + + const { lastFrame, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders correctly with command fallback', async () => { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [ + { + id: '2', + type: 'tool_call', + content: 'run_shell_command', + args: '{"command": "echo hello"}', + status: 'running', + }, + ], + }; + + const { lastFrame, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders correctly with file_path', async () => { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [ + { + id: '3', + type: 'tool_call', + content: 'write_file', + args: '{"file_path": "/tmp/test.txt", "content": "foo"}', + status: 'completed', + }, + ], + }; + + const { lastFrame, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('truncates long args', async () => { + const longDesc = + 'This is a very long description that should definitely be truncated because it exceeds the limit of sixty characters.'; + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [ + { + id: '4', + type: 'tool_call', + content: 'run_shell_command', + args: JSON.stringify({ description: longDesc }), + status: 'running', + }, + ], + }; + + const { lastFrame, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders thought bubbles correctly', async () => { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [ + { + id: '5', + type: 'thought', + content: 'Thinking about life', + status: 'running', + }, + ], + }; + + const { lastFrame, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders cancelled state correctly', async () => { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [], + state: 'cancelled', + }; + + const { lastFrame, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders "Request cancelled." with the info icon', async () => { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [ + { + id: '6', + type: 'thought', + content: 'Request cancelled.', + status: 'error', + }, + ], + }; + + const { lastFrame, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + }); +}); diff --git a/packages/cli/src/ui/components/messages/SubagentProgressDisplay.tsx b/packages/cli/src/ui/components/messages/SubagentProgressDisplay.tsx new file mode 100644 index 0000000000..b34a904b3e --- /dev/null +++ b/packages/cli/src/ui/components/messages/SubagentProgressDisplay.tsx @@ -0,0 +1,151 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type React from 'react'; +import { Box, Text } from 'ink'; +import { theme } from '../../semantic-colors.js'; +import Spinner from 'ink-spinner'; +import type { + SubagentProgress, + SubagentActivityItem, +} from '@google/gemini-cli-core'; +import { TOOL_STATUS } from '../../constants.js'; +import { STATUS_INDICATOR_WIDTH } from './ToolShared.js'; + +export interface SubagentProgressDisplayProps { + progress: SubagentProgress; +} + +const formatToolArgs = (args?: string): string => { + if (!args) return ''; + try { + const parsed: unknown = JSON.parse(args); + if (typeof parsed !== 'object' || parsed === null) { + return args; + } + + if ( + 'description' in parsed && + typeof parsed.description === 'string' && + parsed.description + ) { + return parsed.description; + } + if ('command' in parsed && typeof parsed.command === 'string') + return parsed.command; + if ('file_path' in parsed && typeof parsed.file_path === 'string') + return parsed.file_path; + if ('dir_path' in parsed && typeof parsed.dir_path === 'string') + return parsed.dir_path; + if ('query' in parsed && typeof parsed.query === 'string') + return parsed.query; + if ('url' in parsed && typeof parsed.url === 'string') return parsed.url; + if ('target' in parsed && typeof parsed.target === 'string') + return parsed.target; + + return args; + } catch { + return args; + } +}; + +export const SubagentProgressDisplay: React.FC< + SubagentProgressDisplayProps +> = ({ progress }) => { + let headerText: string | undefined; + let headerColor = theme.text.secondary; + + if (progress.state === 'cancelled') { + headerText = `Subagent ${progress.agentName} was cancelled.`; + headerColor = theme.status.warning; + } else if (progress.state === 'error') { + headerText = `Subagent ${progress.agentName} failed.`; + headerColor = theme.status.error; + } else if (progress.state === 'completed') { + headerText = `Subagent ${progress.agentName} completed.`; + headerColor = theme.status.success; + } + + return ( + + {headerText && ( + + + {headerText} + + + )} + + {progress.recentActivity.map((item: SubagentActivityItem) => { + if (item.type === 'thought') { + const isCancellation = item.content === 'Request cancelled.'; + const icon = isCancellation ? 'ℹ ' : '💭'; + const color = isCancellation + ? theme.status.warning + : theme.text.secondary; + + return ( + + + {icon} + + + {item.content} + + + ); + } else if (item.type === 'tool_call') { + const statusSymbol = + item.status === 'running' ? ( + + ) : item.status === 'completed' ? ( + {TOOL_STATUS.SUCCESS} + ) : item.status === 'cancelled' ? ( + + {TOOL_STATUS.CANCELED} + + ) : ( + {TOOL_STATUS.ERROR} + ); + + const formattedArgs = item.description || formatToolArgs(item.args); + const displayArgs = + formattedArgs.length > 60 + ? formattedArgs.slice(0, 60) + '...' + : formattedArgs; + + return ( + + {statusSymbol} + + + {item.displayName || item.content} + + {displayArgs && ( + + + {displayArgs} + + + )} + + + ); + } + return null; + })} + + + ); +}; diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 29e485a27c..5ec2a18e06 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -75,6 +75,7 @@ export const ToolGroupMessage: React.FC = ({ status: t.status, approvalMode: t.approvalMode, hasResultDisplay: !!t.resultDisplay, + parentCallId: t.parentCallId, }); }), [allToolCalls, isLowErrorVerbosity], diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx index 8e0fc4442a..1c29407e91 100644 --- a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx +++ b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx @@ -11,7 +11,11 @@ import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js'; import { AnsiOutputText, AnsiLineText } from '../AnsiOutput.js'; import { MaxSizedBox } from '../shared/MaxSizedBox.js'; import { theme } from '../../semantic-colors.js'; -import type { AnsiOutput, AnsiLine } from '@google/gemini-cli-core'; +import { + type AnsiOutput, + type AnsiLine, + isSubagentProgress, +} from '@google/gemini-cli-core'; import { useUIState } from '../../contexts/UIStateContext.js'; import { tryParseJSON } from '../../../utils/jsonoutput.js'; import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js'; @@ -20,6 +24,7 @@ import { ScrollableList } from '../shared/ScrollableList.js'; import { SCROLL_TO_ITEM_END } from '../shared/VirtualizedList.js'; import { ACTIVE_SHELL_MAX_LINES } from '../../constants.js'; import { calculateToolContentMaxLines } from '../../utils/toolLayoutUtils.js'; +import { SubagentProgressDisplay } from './SubagentProgressDisplay.js'; // Large threshold to ensure we don't cause performance issues for very large // outputs that will get truncated further MaxSizedBox anyway. @@ -167,6 +172,8 @@ export const ToolResultDisplay: React.FC = ({ {formattedJSON} ); + } else if (isSubagentProgress(truncatedResultDisplay)) { + content = ; } else if ( typeof truncatedResultDisplay === 'string' && renderOutputAsMarkdown diff --git a/packages/cli/src/ui/components/messages/__snapshots__/SubagentProgressDisplay.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/SubagentProgressDisplay.test.tsx.snap new file mode 100644 index 0000000000..8a4c5bd4c4 --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/SubagentProgressDisplay.test.tsx.snap @@ -0,0 +1,41 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[` > renders "Request cancelled." with the info icon 1`] = ` +"ℹ Request cancelled. +" +`; + +exports[` > renders cancelled state correctly 1`] = ` +"Subagent TestAgent was cancelled. +" +`; + +exports[` > renders correctly with command fallback 1`] = ` +"⠋ run_shell_command echo hello +" +`; + +exports[` > renders correctly with description in args 1`] = ` +"⠋ run_shell_command Say hello +" +`; + +exports[` > renders correctly with displayName and description from item 1`] = ` +"⠋ RunShellCommand Executing echo hello +" +`; + +exports[` > renders correctly with file_path 1`] = ` +"✓ write_file /tmp/test.txt +" +`; + +exports[` > renders thought bubbles correctly 1`] = ` +"💭 Thinking about life +" +`; + +exports[` > truncates long args 1`] = ` +"⠋ run_shell_command This is a very long description that should definitely be tr... +" +`; diff --git a/packages/cli/src/ui/hooks/toolMapping.ts b/packages/cli/src/ui/hooks/toolMapping.ts index db9df81566..1bc6d09903 100644 --- a/packages/cli/src/ui/hooks/toolMapping.ts +++ b/packages/cli/src/ui/hooks/toolMapping.ts @@ -48,6 +48,7 @@ export function mapToDisplay( const baseDisplayProperties = { callId: call.request.callId, + parentCallId: call.request.parentCallId, name: displayName, description, renderOutputAsMarkdown, diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index 55048ef6bc..2a8e66789c 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -98,6 +98,7 @@ export interface ToolCallEvent { export interface IndividualToolCallDisplay { callId: string; + parentCallId?: string; name: string; description: string; resultDisplay: ToolResultDisplay | undefined; diff --git a/packages/core/src/agents/browser/browserAgentInvocation.ts b/packages/core/src/agents/browser/browserAgentInvocation.ts index 0de9564c39..9df543300e 100644 --- a/packages/core/src/agents/browser/browserAgentInvocation.ts +++ b/packages/core/src/agents/browser/browserAgentInvocation.ts @@ -16,8 +16,11 @@ import type { Config } from '../../config/config.js'; import { LocalAgentExecutor } from '../local-executor.js'; -import type { AnsiOutput } from '../../utils/terminalSerializer.js'; -import { BaseToolInvocation, type ToolResult } from '../../tools/tools.js'; +import { + BaseToolInvocation, + type ToolResult, + type ToolLiveOutput, +} from '../../tools/tools.js'; import { ToolErrorType } from '../../tools/tool-error.js'; import type { AgentInputs, SubagentActivityEvent } from '../types.js'; import type { MessageBus } from '../../confirmation-bus/message-bus.js'; @@ -82,7 +85,7 @@ export class BrowserAgentInvocation extends BaseToolInvocation< */ async execute( signal: AbortSignal, - updateOutput?: (output: string | AnsiOutput) => void, + updateOutput?: (output: ToolLiveOutput) => void, ): Promise { let browserManager; diff --git a/packages/core/src/agents/local-executor.test.ts b/packages/core/src/agents/local-executor.test.ts index 8f7269b784..df8755015c 100644 --- a/packages/core/src/agents/local-executor.test.ts +++ b/packages/core/src/agents/local-executor.test.ts @@ -711,25 +711,28 @@ describe('LocalAgentExecutor', () => { expect.arrayContaining([ expect.objectContaining({ type: 'THOUGHT_CHUNK', - data: { text: 'T1: Listing' }, + data: expect.objectContaining({ text: 'T1: Listing' }), }), expect.objectContaining({ type: 'TOOL_CALL_END', - data: { name: LS_TOOL_NAME, output: 'file1.txt' }, + data: expect.objectContaining({ + name: LS_TOOL_NAME, + output: 'file1.txt', + }), }), expect.objectContaining({ type: 'TOOL_CALL_START', - data: { + data: expect.objectContaining({ name: TASK_COMPLETE_TOOL_NAME, args: { finalResult: 'Found file1.txt' }, - }, + }), }), expect.objectContaining({ type: 'TOOL_CALL_END', - data: { + data: expect.objectContaining({ name: TASK_COMPLETE_TOOL_NAME, output: expect.stringContaining('Output submitted'), - }, + }), }), ]), ); diff --git a/packages/core/src/agents/local-executor.ts b/packages/core/src/agents/local-executor.ts index 513424ad32..47217213f7 100644 --- a/packages/core/src/agents/local-executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -269,13 +269,22 @@ export class LocalAgentExecutor { }; } - const { nextMessage, submittedOutput, taskCompleted } = + const { nextMessage, submittedOutput, taskCompleted, aborted } = await this.processFunctionCalls( functionCalls, combinedSignal, promptId, onWaitingForConfirmation, ); + + if (aborted) { + return { + status: 'stop', + terminateReason: AgentTerminateMode.ABORTED, + finalResult: null, + }; + } + if (taskCompleted) { const finalResult = submittedOutput ?? 'Task completed successfully.'; return { @@ -857,6 +866,7 @@ export class LocalAgentExecutor { nextMessage: Content; submittedOutput: string | null; taskCompleted: boolean; + aborted: boolean; }> { const allowedToolNames = new Set(this.toolRegistry.getAllToolNames()); // Always allow the completion tool @@ -864,6 +874,7 @@ export class LocalAgentExecutor { let submittedOutput: string | null = null; let taskCompleted = false; + let aborted = false; // We'll separate complete_task from other tools const toolRequests: ToolCallRequestInfo[] = []; @@ -878,8 +889,24 @@ export class LocalAgentExecutor { // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const toolName = functionCall.name as string; + let displayName = toolName; + let description: string | undefined = undefined; + + try { + const tool = this.toolRegistry.getTool(toolName); + if (tool) { + displayName = tool.displayName ?? toolName; + const invocation = tool.build(args); + description = invocation.getDescription(); + } + } catch { + // Ignore errors during formatting for activity emission + } + this.emitActivity('TOOL_CALL_START', { name: toolName, + displayName, + description, args, }); @@ -1077,8 +1104,9 @@ export class LocalAgentExecutor { this.emitActivity('ERROR', { context: 'tool_call', name: toolName, - error: 'Tool call was cancelled.', + error: 'Request cancelled.', }); + aborted = true; } // Add result to syncResults to preserve order later @@ -1111,6 +1139,7 @@ export class LocalAgentExecutor { nextMessage: { role: 'user', parts: toolResponseParts }, submittedOutput, taskCompleted, + aborted, }; } diff --git a/packages/core/src/agents/local-invocation.test.ts b/packages/core/src/agents/local-invocation.test.ts index 91efcd399f..77509881af 100644 --- a/packages/core/src/agents/local-invocation.test.ts +++ b/packages/core/src/agents/local-invocation.test.ts @@ -4,17 +4,25 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest'; +import { + describe, + it, + expect, + vi, + beforeEach, + afterEach, + type Mocked, +} from 'vitest'; import type { LocalAgentDefinition, SubagentActivityEvent, AgentInputs, + SubagentProgress, } from './types.js'; import { LocalSubagentInvocation } from './local-invocation.js'; import { LocalAgentExecutor } from './local-executor.js'; import { AgentTerminateMode } from './types.js'; import { makeFakeConfig } from '../test-utils/config.js'; -import { ToolErrorType } from '../tools/tool-error.js'; import type { Config } from '../config/config.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { type z } from 'zod'; @@ -29,6 +37,7 @@ let mockConfig: Config; const testDefinition: LocalAgentDefinition = { kind: 'local', name: 'MockAgent', + displayName: 'Mock Agent', description: 'A mock agent.', inputConfig: { inputSchema: { @@ -70,6 +79,10 @@ describe('LocalSubagentInvocation', () => { ); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + it('should pass the messageBus to the parent constructor', () => { const params = { task: 'Analyze data' }; const invocation = new LocalSubagentInvocation( @@ -173,7 +186,12 @@ describe('LocalSubagentInvocation', () => { mockConfig, expect.any(Function), ); - expect(updateOutput).toHaveBeenCalledWith('Subagent starting...\n'); + expect(updateOutput).toHaveBeenCalledWith( + expect.objectContaining({ + isSubagentProgress: true, + agentName: 'MockAgent', + }), + ); expect(mockExecutorInstance.run).toHaveBeenCalledWith(params, signal); @@ -211,13 +229,17 @@ describe('LocalSubagentInvocation', () => { await invocation.execute(signal, updateOutput); - expect(updateOutput).toHaveBeenCalledWith('Subagent starting...\n'); - expect(updateOutput).toHaveBeenCalledWith('🤖💭 Analyzing...'); - expect(updateOutput).toHaveBeenCalledWith('🤖💭 Still thinking.'); - expect(updateOutput).toHaveBeenCalledTimes(3); // Initial message + 2 thoughts + expect(updateOutput).toHaveBeenCalledTimes(3); // Initial + 2 updates + const lastCall = updateOutput.mock.calls[2][0] as SubagentProgress; + expect(lastCall.recentActivity).toContainEqual( + expect.objectContaining({ + type: 'thought', + content: 'Analyzing... Still thinking.', + }), + ); }); - it('should NOT stream other activities (e.g., TOOL_CALL_START, ERROR)', async () => { + it('should stream other activities (e.g., TOOL_CALL_START, ERROR)', async () => { mockExecutorInstance.run.mockImplementation(async () => { const onActivity = MockLocalAgentExecutor.create.mock.calls[0][2]; @@ -226,7 +248,7 @@ describe('LocalSubagentInvocation', () => { isSubagentActivityEvent: true, agentName: 'MockAgent', type: 'TOOL_CALL_START', - data: { name: 'ls' }, + data: { name: 'ls', args: {} }, } as SubagentActivityEvent); onActivity({ isSubagentActivityEvent: true, @@ -240,9 +262,15 @@ describe('LocalSubagentInvocation', () => { await invocation.execute(signal, updateOutput); - // Should only contain the initial "Subagent starting..." message - expect(updateOutput).toHaveBeenCalledTimes(1); - expect(updateOutput).toHaveBeenCalledWith('Subagent starting...\n'); + expect(updateOutput).toHaveBeenCalledTimes(3); + const lastCall = updateOutput.mock.calls[2][0] as SubagentProgress; + expect(lastCall.recentActivity).toContainEqual( + expect.objectContaining({ + type: 'thought', + content: 'Error: Failed', + status: 'error', + }), + ); }); it('should run successfully without an updateOutput callback', async () => { @@ -272,16 +300,19 @@ describe('LocalSubagentInvocation', () => { const result = await invocation.execute(signal, updateOutput); - expect(result.error).toEqual({ - message: error.message, - type: ToolErrorType.EXECUTION_FAILED, - }); - expect(result.returnDisplay).toBe( - `Subagent Failed: MockAgent\nError: ${error.message}`, - ); + expect(result.error).toBeUndefined(); expect(result.llmContent).toBe( `Subagent 'MockAgent' failed. Error: ${error.message}`, ); + const display = result.returnDisplay as SubagentProgress; + expect(display.isSubagentProgress).toBe(true); + expect(display.recentActivity).toContainEqual( + expect.objectContaining({ + type: 'thought', + content: `Error: ${error.message}`, + status: 'error', + }), + ); }); it('should handle executor creation failure', async () => { @@ -291,19 +322,21 @@ describe('LocalSubagentInvocation', () => { const result = await invocation.execute(signal, updateOutput); expect(mockExecutorInstance.run).not.toHaveBeenCalled(); - expect(result.error).toEqual({ - message: creationError.message, - type: ToolErrorType.EXECUTION_FAILED, - }); - expect(result.returnDisplay).toContain(`Error: ${creationError.message}`); + expect(result.error).toBeUndefined(); + expect(result.llmContent).toContain(creationError.message); + + const display = result.returnDisplay as SubagentProgress; + expect(display.recentActivity).toContainEqual( + expect.objectContaining({ + content: `Error: ${creationError.message}`, + status: 'error', + }), + ); }); - /** - * This test verifies that the AbortSignal is correctly propagated and - * that a rejection from the executor due to abortion is handled gracefully. - */ it('should handle abortion signal during execution', async () => { const abortError = new Error('Aborted'); + abortError.name = 'AbortError'; mockExecutorInstance.run.mockRejectedValue(abortError); const controller = new AbortController(); @@ -312,14 +345,24 @@ describe('LocalSubagentInvocation', () => { updateOutput, ); controller.abort(); - const result = await executePromise; + await expect(executePromise).rejects.toThrow('Aborted'); expect(mockExecutorInstance.run).toHaveBeenCalledWith( params, controller.signal, ); - expect(result.error?.message).toBe('Aborted'); - expect(result.error?.type).toBe(ToolErrorType.EXECUTION_FAILED); + }); + + it('should throw an error and bubble cancellation when execution returns ABORTED', async () => { + const mockOutput = { + result: 'Cancelled by user', + terminate_reason: AgentTerminateMode.ABORTED, + }; + mockExecutorInstance.run.mockResolvedValue(mockOutput); + + await expect(invocation.execute(signal, updateOutput)).rejects.toThrow( + 'Operation cancelled by user', + ); }); }); }); diff --git a/packages/core/src/agents/local-invocation.ts b/packages/core/src/agents/local-invocation.ts index a75fa8a11a..4bd2bc171a 100644 --- a/packages/core/src/agents/local-invocation.ts +++ b/packages/core/src/agents/local-invocation.ts @@ -6,18 +6,25 @@ import type { Config } from '../config/config.js'; import { LocalAgentExecutor } from './local-executor.js'; -import type { AnsiOutput } from '../utils/terminalSerializer.js'; -import { BaseToolInvocation, type ToolResult } from '../tools/tools.js'; -import { ToolErrorType } from '../tools/tool-error.js'; -import type { - LocalAgentDefinition, - AgentInputs, - SubagentActivityEvent, +import { + BaseToolInvocation, + type ToolResult, + type ToolLiveOutput, +} from '../tools/tools.js'; +import { + type LocalAgentDefinition, + type AgentInputs, + type SubagentActivityEvent, + type SubagentProgress, + type SubagentActivityItem, + AgentTerminateMode, } from './types.js'; +import { randomUUID } from 'node:crypto'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; const INPUT_PREVIEW_MAX_LENGTH = 50; const DESCRIPTION_MAX_LENGTH = 200; +const MAX_RECENT_ACTIVITY = 3; /** * Represents a validated, executable instance of a subagent tool. @@ -81,11 +88,20 @@ export class LocalSubagentInvocation extends BaseToolInvocation< */ async execute( signal: AbortSignal, - updateOutput?: (output: string | AnsiOutput) => void, + updateOutput?: (output: ToolLiveOutput) => void, ): Promise { + let recentActivity: SubagentActivityItem[] = []; + try { if (updateOutput) { - updateOutput('Subagent starting...\n'); + // Send initial state + const initialProgress: SubagentProgress = { + isSubagentProgress: true, + agentName: this.definition.name, + recentActivity: [], + state: 'running', + }; + updateOutput(initialProgress); } // Create an activity callback to bridge the executor's events to the @@ -93,11 +109,114 @@ export class LocalSubagentInvocation extends BaseToolInvocation< const onActivity = (activity: SubagentActivityEvent): void => { if (!updateOutput) return; - if ( - activity.type === 'THOUGHT_CHUNK' && - typeof activity.data['text'] === 'string' - ) { - updateOutput(`🤖💭 ${activity.data['text']}`); + let updated = false; + + switch (activity.type) { + case 'THOUGHT_CHUNK': { + const text = String(activity.data['text']); + const lastItem = recentActivity[recentActivity.length - 1]; + if ( + lastItem && + lastItem.type === 'thought' && + lastItem.status === 'running' + ) { + lastItem.content += text; + } else { + recentActivity.push({ + id: randomUUID(), + type: 'thought', + content: text, + status: 'running', + }); + } + updated = true; + break; + } + case 'TOOL_CALL_START': { + const name = String(activity.data['name']); + const displayName = activity.data['displayName'] + ? String(activity.data['displayName']) + : undefined; + const description = activity.data['description'] + ? String(activity.data['description']) + : undefined; + const args = JSON.stringify(activity.data['args']); + recentActivity.push({ + id: randomUUID(), + type: 'tool_call', + content: name, + displayName, + description, + args, + status: 'running', + }); + updated = true; + break; + } + case 'TOOL_CALL_END': { + const name = String(activity.data['name']); + // Find the last running tool call with this name + for (let i = recentActivity.length - 1; i >= 0; i--) { + if ( + recentActivity[i].type === 'tool_call' && + recentActivity[i].content === name && + recentActivity[i].status === 'running' + ) { + recentActivity[i].status = 'completed'; + updated = true; + break; + } + } + break; + } + case 'ERROR': { + const error = String(activity.data['error']); + const isCancellation = error === 'Request cancelled.'; + const toolName = activity.data['name'] + ? String(activity.data['name']) + : undefined; + + if (toolName && isCancellation) { + for (let i = recentActivity.length - 1; i >= 0; i--) { + if ( + recentActivity[i].type === 'tool_call' && + recentActivity[i].content === toolName && + recentActivity[i].status === 'running' + ) { + recentActivity[i].status = 'cancelled'; + updated = true; + break; + } + } + } + + recentActivity.push({ + id: randomUUID(), + type: 'thought', // Treat errors as thoughts for now, or add an error type + content: isCancellation ? error : `Error: ${error}`, + status: isCancellation ? 'cancelled' : 'error', + }); + updated = true; + break; + } + default: + break; + } + + if (updated) { + // Keep only the last N items + if (recentActivity.length > MAX_RECENT_ACTIVITY) { + recentActivity = recentActivity.slice(-MAX_RECENT_ACTIVITY); + } + + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: this.definition.name, + recentActivity: [...recentActivity], // Copy to avoid mutation issues + state: 'running', + }; + + updateOutput(progress); } }; @@ -109,6 +228,23 @@ export class LocalSubagentInvocation extends BaseToolInvocation< const output = await executor.run(this.params, signal); + if (output.terminate_reason === AgentTerminateMode.ABORTED) { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: this.definition.name, + recentActivity: [...recentActivity], + state: 'cancelled', + }; + + if (updateOutput) { + updateOutput(progress); + } + + const cancelError = new Error('Operation cancelled by user'); + cancelError.name = 'AbortError'; + throw cancelError; + } + const resultContent = `Subagent '${this.definition.name}' finished. Termination Reason: ${output.terminate_reason} Result: @@ -131,13 +267,55 @@ ${output.result} const errorMessage = error instanceof Error ? error.message : String(error); + const isAbort = + (error instanceof Error && error.name === 'AbortError') || + errorMessage.includes('Aborted'); + + // Mark any running items as error/cancelled + for (const item of recentActivity) { + if (item.status === 'running') { + item.status = isAbort ? 'cancelled' : 'error'; + } + } + + // Ensure the error is reflected in the recent activity for display + // But only if it's NOT an abort, or if we want to show "Cancelled" as a thought + if (!isAbort) { + const lastActivity = recentActivity[recentActivity.length - 1]; + if (!lastActivity || lastActivity.status !== 'error') { + recentActivity.push({ + id: randomUUID(), + type: 'thought', + content: `Error: ${errorMessage}`, + status: 'error', + }); + // Maintain size limit + if (recentActivity.length > MAX_RECENT_ACTIVITY) { + recentActivity = recentActivity.slice(-MAX_RECENT_ACTIVITY); + } + } + } + + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: this.definition.name, + recentActivity: [...recentActivity], + state: isAbort ? 'cancelled' : 'error', + }; + + if (updateOutput) { + updateOutput(progress); + } + + if (isAbort) { + throw error; + } + return { llmContent: `Subagent '${this.definition.name}' failed. Error: ${errorMessage}`, - returnDisplay: `Subagent Failed: ${this.definition.name}\nError: ${errorMessage}`, - error: { - message: errorMessage, - type: ToolErrorType.EXECUTION_FAILED, - }, + returnDisplay: progress, + // We omit the 'error' property so that the UI renders our rich returnDisplay + // instead of the raw error message. The llmContent still informs the agent of the failure. }; } } diff --git a/packages/core/src/agents/subagent-tool.test.ts b/packages/core/src/agents/subagent-tool.test.ts index 74f0051351..c6e90ea198 100644 --- a/packages/core/src/agents/subagent-tool.test.ts +++ b/packages/core/src/agents/subagent-tool.test.ts @@ -120,6 +120,16 @@ describe('SubAgentInvocation', () => { ); }); + it('should return the correct description', () => { + const tool = new SubagentTool(testDefinition, mockConfig, mockMessageBus); + const params = {}; + // @ts-expect-error - accessing protected method for testing + const invocation = tool.createInvocation(params, mockMessageBus); + expect(invocation.getDescription()).toBe( + "Delegating to agent 'LocalAgent'", + ); + }); + it('should delegate shouldConfirmExecute to the inner sub-invocation (remote)', async () => { const tool = new SubagentTool( testRemoteDefinition, diff --git a/packages/core/src/agents/subagent-tool.ts b/packages/core/src/agents/subagent-tool.ts index 3ecff4e969..21a3864160 100644 --- a/packages/core/src/agents/subagent-tool.ts +++ b/packages/core/src/agents/subagent-tool.ts @@ -12,8 +12,8 @@ import { BaseToolInvocation, type ToolCallConfirmationDetails, isTool, + type ToolLiveOutput, } from '../tools/tools.js'; -import type { AnsiOutput } from '../utils/terminalSerializer.js'; import type { Config } from '../config/config.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import type { AgentDefinition, AgentInputs } from './types.js'; @@ -155,7 +155,7 @@ class SubAgentInvocation extends BaseToolInvocation { async execute( signal: AbortSignal, - updateOutput?: (output: string | AnsiOutput) => void, + updateOutput?: (output: ToolLiveOutput) => void, ): Promise { const validationError = SchemaValidator.validate( this.definition.inputConfig.inputSchema, diff --git a/packages/core/src/agents/types.ts b/packages/core/src/agents/types.ts index 3704746810..ceac0909df 100644 --- a/packages/core/src/agents/types.ts +++ b/packages/core/src/agents/types.ts @@ -71,6 +71,32 @@ export interface SubagentActivityEvent { data: Record; } +export interface SubagentActivityItem { + id: string; + type: 'thought' | 'tool_call'; + content: string; + displayName?: string; + description?: string; + args?: string; + status: 'running' | 'completed' | 'error' | 'cancelled'; +} + +export interface SubagentProgress { + isSubagentProgress: true; + agentName: string; + recentActivity: SubagentActivityItem[]; + state?: 'running' | 'completed' | 'error' | 'cancelled'; +} + +export function isSubagentProgress(obj: unknown): obj is SubagentProgress { + return ( + typeof obj === 'object' && + obj !== null && + 'isSubagentProgress' in obj && + obj.isSubagentProgress === true + ); +} + /** * The base definition for an agent. * @template TOutput The specific Zod schema for the agent's final output object. diff --git a/packages/core/src/core/coreToolHookTriggers.ts b/packages/core/src/core/coreToolHookTriggers.ts index 9c83253903..cbd90e8039 100644 --- a/packages/core/src/core/coreToolHookTriggers.ts +++ b/packages/core/src/core/coreToolHookTriggers.ts @@ -10,10 +10,11 @@ import type { ToolResult, AnyDeclarativeTool, AnyToolInvocation, + ToolLiveOutput, } from '../tools/tools.js'; import { ToolErrorType } from '../tools/tool-error.js'; import { debugLogger } from '../utils/debugLogger.js'; -import type { AnsiOutput, ShellExecutionConfig } from '../index.js'; +import type { ShellExecutionConfig } from '../index.js'; import { ShellToolInvocation } from '../tools/shell.js'; import { DiscoveredMCPToolInvocation } from '../tools/mcp-tool.js'; @@ -71,7 +72,7 @@ export async function executeToolWithHooks( toolName: string, signal: AbortSignal, tool: AnyDeclarativeTool, - liveOutputCallback?: (outputChunk: string | AnsiOutput) => void, + liveOutputCallback?: (outputChunk: ToolLiveOutput) => void, shellExecutionConfig?: ShellExecutionConfig, setPidCallback?: (pid: number) => void, config?: Config, diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 6814f31402..789ea73ff1 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -189,11 +189,16 @@ export class InvalidStreamError extends Error { readonly type: | 'NO_FINISH_REASON' | 'NO_RESPONSE_TEXT' - | 'MALFORMED_FUNCTION_CALL'; + | 'MALFORMED_FUNCTION_CALL' + | 'UNEXPECTED_TOOL_CALL'; constructor( message: string, - type: 'NO_FINISH_REASON' | 'NO_RESPONSE_TEXT' | 'MALFORMED_FUNCTION_CALL', + type: + | 'NO_FINISH_REASON' + | 'NO_RESPONSE_TEXT' + | 'MALFORMED_FUNCTION_CALL' + | 'UNEXPECTED_TOOL_CALL', ) { super(message); this.name = 'InvalidStreamError'; @@ -935,6 +940,12 @@ export class GeminiChat { 'MALFORMED_FUNCTION_CALL', ); } + if (finishReason === FinishReason.UNEXPECTED_TOOL_CALL) { + throw new InvalidStreamError( + 'Model stream ended with unexpected tool call.', + 'UNEXPECTED_TOOL_CALL', + ); + } if (!responseText) { throw new InvalidStreamError( 'Model stream ended with empty response text.', diff --git a/packages/core/src/scheduler/tool-executor.ts b/packages/core/src/scheduler/tool-executor.ts index d37c49624c..e358c53c8b 100644 --- a/packages/core/src/scheduler/tool-executor.ts +++ b/packages/core/src/scheduler/tool-executor.ts @@ -9,7 +9,8 @@ import type { ToolCallResponseInfo, ToolResult, Config, - AnsiOutput, + ToolResultDisplay, + ToolLiveOutput, } from '../index.js'; import { ToolErrorType, @@ -45,7 +46,7 @@ import { export interface ToolExecutionContext { call: ToolCall; signal: AbortSignal; - outputUpdateHandler?: (callId: string, output: string | AnsiOutput) => void; + outputUpdateHandler?: (callId: string, output: ToolLiveOutput) => void; onUpdateToolCall: (updatedCall: ToolCall) => void; } @@ -68,7 +69,7 @@ export class ToolExecutor { // Setup live output handling const liveOutputCallback = tool.canUpdateOutput && outputUpdateHandler - ? (outputChunk: string | AnsiOutput) => { + ? (outputChunk: ToolLiveOutput) => { outputUpdateHandler(callId, outputChunk); } : undefined; @@ -134,6 +135,7 @@ export class ToolExecutor { completedToolCall = this.createCancelledResult( call, 'User cancelled tool execution.', + toolResult.returnDisplay, ); } else if (toolResult.error === undefined) { completedToolCall = await this.createSuccessResult( @@ -155,7 +157,12 @@ export class ToolExecutor { } } catch (executionError: unknown) { spanMetadata.error = executionError; - if (signal.aborted) { + const isAbortError = + executionError instanceof Error && + (executionError.name === 'AbortError' || + executionError.message.includes('Operation cancelled by user')); + + if (signal.aborted || isAbortError) { completedToolCall = this.createCancelledResult( call, 'User cancelled tool execution.', @@ -182,6 +189,7 @@ export class ToolExecutor { private createCancelledResult( call: ToolCall, reason: string, + resultDisplay?: ToolResultDisplay, ): CancelledToolCall { const errorMessage = `[Operation Cancelled] ${reason}`; const startTime = 'startTime' in call ? call.startTime : undefined; @@ -206,7 +214,7 @@ export class ToolExecutor { }, }, ], - resultDisplay: undefined, + resultDisplay, error: undefined, errorType: undefined, contentLength: errorMessage.length, diff --git a/packages/core/src/scheduler/types.ts b/packages/core/src/scheduler/types.ts index 7eaf07e94e..9fedd48f41 100644 --- a/packages/core/src/scheduler/types.ts +++ b/packages/core/src/scheduler/types.ts @@ -11,8 +11,8 @@ import type { ToolCallConfirmationDetails, ToolConfirmationOutcome, ToolResultDisplay, + ToolLiveOutput, } from '../tools/tools.js'; -import type { AnsiOutput } from '../utils/terminalSerializer.js'; import type { ToolErrorType } from '../tools/tool-error.js'; import type { SerializableConfirmationDetails } from '../confirmation-bus/types.js'; import { type ApprovalMode } from '../policy/types.js'; @@ -125,7 +125,7 @@ export type ExecutingToolCall = { request: ToolCallRequestInfo; tool: AnyDeclarativeTool; invocation: AnyToolInvocation; - liveOutput?: string | AnsiOutput; + liveOutput?: ToolLiveOutput; progressMessage?: string; progressPercent?: number; progress?: number; @@ -197,7 +197,7 @@ export type ConfirmHandler = ( export type OutputUpdateHandler = ( toolCallId: string, - outputChunk: string | AnsiOutput, + outputChunk: ToolLiveOutput, ) => void; export type AllToolCallsCompleteHandler = ( diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 741272f555..6afded3faa 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -17,6 +17,7 @@ import type { ToolCallConfirmationDetails, ToolExecuteConfirmationDetails, PolicyUpdateOptions, + ToolLiveOutput, } from './tools.js'; import { BaseDeclarativeTool, @@ -149,7 +150,7 @@ export class ShellToolInvocation extends BaseToolInvocation< async execute( signal: AbortSignal, - updateOutput?: (output: string | AnsiOutput) => void, + updateOutput?: (output: ToolLiveOutput) => void, shellExecutionConfig?: ShellExecutionConfig, setPidCallback?: (pid: number) => void, ): Promise { diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 3c024168d4..0a82cc1510 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -19,6 +19,7 @@ import { type Question, } from '../confirmation-bus/types.js'; import { type ApprovalMode } from '../policy/types.js'; +import type { SubagentProgress } from '../agents/types.js'; /** * Represents a validated and ready-to-execute tool call. @@ -64,7 +65,7 @@ export interface ToolInvocation< */ execute( signal: AbortSignal, - updateOutput?: (output: string | AnsiOutput) => void, + updateOutput?: (output: ToolLiveOutput) => void, shellExecutionConfig?: ShellExecutionConfig, ): Promise; } @@ -276,7 +277,7 @@ export abstract class BaseToolInvocation< abstract execute( signal: AbortSignal, - updateOutput?: (output: string | AnsiOutput) => void, + updateOutput?: (output: ToolLiveOutput) => void, shellExecutionConfig?: ShellExecutionConfig, ): Promise; } @@ -422,7 +423,7 @@ export abstract class DeclarativeTool< async buildAndExecute( params: TParams, signal: AbortSignal, - updateOutput?: (output: string | AnsiOutput) => void, + updateOutput?: (output: ToolLiveOutput) => void, shellExecutionConfig?: ShellExecutionConfig, ): Promise { const invocation = this.build(params); @@ -688,7 +689,14 @@ export interface TodoList { todos: Todo[]; } -export type ToolResultDisplay = string | FileDiff | AnsiOutput | TodoList; +export type ToolLiveOutput = string | AnsiOutput | SubagentProgress; + +export type ToolResultDisplay = + | string + | FileDiff + | AnsiOutput + | TodoList + | SubagentProgress; export type TodoStatus = 'pending' | 'in_progress' | 'completed' | 'cancelled'; diff --git a/packages/core/src/utils/tool-utils.test.ts b/packages/core/src/utils/tool-utils.test.ts index 225889d53a..c007b37715 100644 --- a/packages/core/src/utils/tool-utils.test.ts +++ b/packages/core/src/utils/tool-utils.test.ts @@ -98,6 +98,17 @@ describe('shouldHideToolCall', () => { ).toBe(!visible); }, ); + + it('hides tool calls with a parentCallId', () => { + expect( + shouldHideToolCall({ + displayName: 'any_tool', + status: CoreToolCallStatus.Success, + hasResultDisplay: true, + parentCallId: 'some-parent', + }), + ).toBe(true); + }); }); describe('getToolSuggestion', () => { diff --git a/packages/core/src/utils/tool-utils.ts b/packages/core/src/utils/tool-utils.ts index b8e60fe4ce..17ccbda8d6 100644 --- a/packages/core/src/utils/tool-utils.ts +++ b/packages/core/src/utils/tool-utils.ts @@ -28,20 +28,28 @@ export interface ShouldHideToolCallParams { approvalMode?: ApprovalMode; /** Whether the tool has produced a result for display. */ hasResultDisplay: boolean; + /** The ID of the parent tool call, if any. */ + parentCallId?: string; } /** * Determines if a tool call should be hidden from the standard tool history UI. * * We hide tools in several cases: - * 1. Ask User tools that are in progress, displayed via specialized UI. - * 2. Ask User tools that errored without result display, typically param + * 1. Tool calls that have a parent, as they are "internal" to another tool (e.g. subagent). + * 2. Ask User tools that are in progress, displayed via specialized UI. + * 3. Ask User tools that errored without result display, typically param * validation errors that the agent automatically recovers from. - * 3. WriteFile and Edit tools when in Plan Mode, redundant because the + * 4. WriteFile and Edit tools when in Plan Mode, redundant because the * resulting plans are displayed separately upon exiting plan mode. */ export function shouldHideToolCall(params: ShouldHideToolCallParams): boolean { - const { displayName, status, approvalMode, hasResultDisplay } = params; + const { displayName, status, approvalMode, hasResultDisplay, parentCallId } = + params; + + if (parentCallId) { + return true; + } switch (displayName) { case ASK_USER_DISPLAY_NAME: