From a1f9af3fa773ee8b7421d13d09b66059bd52058f Mon Sep 17 00:00:00 2001 From: Abhi <43648792+abhipatel12@users.noreply.github.com> Date: Mon, 23 Mar 2026 21:56:00 -0400 Subject: [PATCH] fix(core): accurately reflect subagent tool failure in UI (#23187) --- .../messages/SubagentProgressDisplay.test.tsx | 21 +++++++++++ .../SubagentProgressDisplay.test.tsx.snap | 7 ++++ .../agents/browser/browserAgentInvocation.ts | 8 +++-- packages/core/src/agents/local-executor.ts | 1 + .../core/src/agents/local-invocation.test.ts | 36 +++++++++++++++++++ packages/core/src/agents/local-invocation.ts | 7 ++-- packages/core/src/agents/types.ts | 12 +++++++ packages/core/src/tools/shell.ts | 4 +++ 8 files changed, 91 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/ui/components/messages/SubagentProgressDisplay.test.tsx b/packages/cli/src/ui/components/messages/SubagentProgressDisplay.test.tsx index 955c4a5f8a..caed091b2b 100644 --- a/packages/cli/src/ui/components/messages/SubagentProgressDisplay.test.tsx +++ b/packages/cli/src/ui/components/messages/SubagentProgressDisplay.test.tsx @@ -182,4 +182,25 @@ describe('', () => { ); expect(lastFrame()).toMatchSnapshot(); }); + + it('renders error tool status correctly', async () => { + const progress: SubagentProgress = { + isSubagentProgress: true, + agentName: 'TestAgent', + recentActivity: [ + { + id: '7', + type: 'tool_call', + content: 'run_shell_command', + args: '{"command": "echo hello"}', + status: 'error', + }, + ], + }; + + const { lastFrame } = await render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); }); 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 index 2d31c9c652..77a3ec001f 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/SubagentProgressDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/SubagentProgressDisplay.test.tsx.snap @@ -40,6 +40,13 @@ exports[` > renders correctly with file_path 1`] = ` " `; +exports[` > renders error tool status correctly 1`] = ` +"Running subagent TestAgent... + +x run_shell_command echo hello +" +`; + exports[` > renders thought bubbles correctly 1`] = ` "Running subagent TestAgent... diff --git a/packages/core/src/agents/browser/browserAgentInvocation.ts b/packages/core/src/agents/browser/browserAgentInvocation.ts index 60bd5201f0..0c96e1894c 100644 --- a/packages/core/src/agents/browser/browserAgentInvocation.ts +++ b/packages/core/src/agents/browser/browserAgentInvocation.ts @@ -30,6 +30,7 @@ import { type SubagentActivityEvent, type SubagentProgress, type SubagentActivityItem, + isToolActivityError, } from '../types.js'; import type { MessageBus } from '../../confirmation-bus/message-bus.js'; import { @@ -210,8 +211,9 @@ export class BrowserAgentInvocation extends BaseToolInvocation< const callId = activity.data['id'] ? String(activity.data['id']) : undefined; - // Find the tool call by ID - // Find the tool call by ID + const data = activity.data['data']; + const isError = isToolActivityError(data); + for (let i = recentActivity.length - 1; i >= 0; i--) { if ( recentActivity[i].type === 'tool_call' && @@ -219,7 +221,7 @@ export class BrowserAgentInvocation extends BaseToolInvocation< recentActivity[i].id === callId && recentActivity[i].status === 'running' ) { - recentActivity[i].status = 'completed'; + recentActivity[i].status = isError ? 'error' : 'completed'; updated = true; break; } diff --git a/packages/core/src/agents/local-executor.ts b/packages/core/src/agents/local-executor.ts index a860e1e597..ed26f634a0 100644 --- a/packages/core/src/agents/local-executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -1240,6 +1240,7 @@ export class LocalAgentExecutor { name: toolName, id: call.request.callId, output: call.response.resultDisplay, + data: call.response.data, }); } else if (call.status === 'error') { this.emitActivity('ERROR', { diff --git a/packages/core/src/agents/local-invocation.test.ts b/packages/core/src/agents/local-invocation.test.ts index 2153f538c9..478ceb9f34 100644 --- a/packages/core/src/agents/local-invocation.test.ts +++ b/packages/core/src/agents/local-invocation.test.ts @@ -338,6 +338,42 @@ describe('LocalSubagentInvocation', () => { ); }); + it('should mark tool call as error when TOOL_CALL_END contains isError: true', async () => { + mockExecutorInstance.run.mockImplementation(async () => { + const onActivity = MockLocalAgentExecutor.create.mock.calls[0][2]; + + if (onActivity) { + onActivity({ + isSubagentActivityEvent: true, + agentName: 'MockAgent', + type: 'TOOL_CALL_START', + data: { name: 'ls', args: {}, callId: 'call1' }, + } as SubagentActivityEvent); + onActivity({ + isSubagentActivityEvent: true, + agentName: 'MockAgent', + type: 'TOOL_CALL_END', + data: { name: 'ls', id: 'call1', data: { isError: true } }, + } as SubagentActivityEvent); + } + return { result: 'Done', terminate_reason: AgentTerminateMode.GOAL }; + }); + + await invocation.execute(signal, updateOutput); + + expect(updateOutput).toHaveBeenCalled(); + const lastCall = updateOutput.mock.calls[ + updateOutput.mock.calls.length - 1 + ][0] as SubagentProgress; + expect(lastCall.recentActivity).toContainEqual( + expect.objectContaining({ + type: 'tool_call', + content: 'ls', + status: 'error', + }), + ); + }); + it('should reflect tool rejections in the activity stream as cancelled but not abort the agent', async () => { mockExecutorInstance.run.mockImplementation(async () => { const onActivity = MockLocalAgentExecutor.create.mock.calls[0][2]; diff --git a/packages/core/src/agents/local-invocation.ts b/packages/core/src/agents/local-invocation.ts index 08a4aa8264..0d28dcbe64 100644 --- a/packages/core/src/agents/local-invocation.ts +++ b/packages/core/src/agents/local-invocation.ts @@ -21,6 +21,7 @@ import { SubagentActivityErrorType, SUBAGENT_REJECTED_ERROR_PREFIX, SUBAGENT_CANCELLED_ERROR_MESSAGE, + isToolActivityError, } from './types.js'; import { randomUUID } from 'node:crypto'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -166,14 +167,16 @@ export class LocalSubagentInvocation extends BaseToolInvocation< } case 'TOOL_CALL_END': { const name = String(activity.data['name']); - // Find the last running tool call with this name + const data = activity.data['data']; + const isError = isToolActivityError(data); + 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'; + recentActivity[i].status = isError ? 'error' : 'completed'; updated = true; break; } diff --git a/packages/core/src/agents/types.ts b/packages/core/src/agents/types.ts index 7f056c37ab..e36d8f0ccb 100644 --- a/packages/core/src/agents/types.ts +++ b/packages/core/src/agents/types.ts @@ -112,6 +112,18 @@ export function isSubagentProgress(obj: unknown): obj is SubagentProgress { ); } +/** + * Checks if the tool call data indicates an error. + */ +export function isToolActivityError(data: unknown): boolean { + return ( + data !== null && + typeof data === 'object' && + 'isError' in data && + data.isError === 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/tools/shell.ts b/packages/core/src/tools/shell.ts index b05badecf9..86e3a68bc5 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -381,6 +381,10 @@ export class ShellToolInvocation extends BaseToolInvocation< if (result.exitCode !== null && result.exitCode !== 0) { llmContentParts.push(`Exit Code: ${result.exitCode}`); + data = { + exitCode: result.exitCode, + isError: true, + }; } if (result.signal) {