From 35c933ab6a6939d53aad0884a2264cc82f942fa8 Mon Sep 17 00:00:00 2001 From: Abhi <43648792+abhipatel12@users.noreply.github.com> Date: Wed, 18 Mar 2026 21:09:37 -0400 Subject: [PATCH] feat(core): resilient subagent tool rejection with contextual feedback (#22951) --- .../core/src/agents/local-executor.test.ts | 184 +++++++++++++++++- packages/core/src/agents/local-executor.ts | 72 +++++-- .../core/src/agents/local-invocation.test.ts | 44 +++++ packages/core/src/agents/local-invocation.ts | 34 +++- packages/core/src/agents/types.ts | 12 ++ 5 files changed, 323 insertions(+), 23 deletions(-) diff --git a/packages/core/src/agents/local-executor.test.ts b/packages/core/src/agents/local-executor.test.ts index 8fc189e961..e0a21e01e3 100644 --- a/packages/core/src/agents/local-executor.test.ts +++ b/packages/core/src/agents/local-executor.test.ts @@ -91,9 +91,18 @@ import { type LocalAgentDefinition, type SubagentActivityEvent, type OutputConfig, + SubagentActivityErrorType, } from './types.js'; -import type { AnyDeclarativeTool, AnyToolInvocation } from '../tools/tools.js'; -import type { ToolCallRequestInfo } from '../scheduler/types.js'; +import { + ToolConfirmationOutcome, + type AnyDeclarativeTool, + type AnyToolInvocation, +} from '../tools/tools.js'; +import { + type ToolCallRequestInfo, + CoreToolCallStatus, +} from '../scheduler/types.js'; + import { CompressionStatus } from '../core/turn.js'; import { ChatCompressionService } from '../services/chatCompressionService.js'; import type { @@ -1013,6 +1022,7 @@ describe('LocalAgentExecutor', () => { data: expect.objectContaining({ context: 'protocol_violation', error: expectedError, + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -1058,6 +1068,7 @@ describe('LocalAgentExecutor', () => { context: 'tool_call', name: TASK_COMPLETE_TOOL_NAME, error: expectedError, + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -1161,7 +1172,7 @@ describe('LocalAgentExecutor', () => { if (callsStarted === 2) resolveCalls(); await vi.advanceTimersByTimeAsync(100); return { - status: 'success', + status: CoreToolCallStatus.Success, request: reqInfo, tool: {} as AnyDeclarativeTool, invocation: {} as AnyToolInvocation, @@ -1179,7 +1190,7 @@ describe('LocalAgentExecutor', () => { ], error: undefined, errorType: undefined, - contentLength: undefined, + contentLength: 0, }, }; }), @@ -1217,10 +1228,10 @@ describe('LocalAgentExecutor', () => { expect(parts).toEqual( expect.arrayContaining([ expect.objectContaining({ - functionResponse: expect.objectContaining({ id: 'c1' }), + functionResponse: expect.objectContaining({ name: LS_TOOL_NAME }), }), expect.objectContaining({ - functionResponse: expect.objectContaining({ id: 'c2' }), + functionResponse: expect.objectContaining({ name: LS_TOOL_NAME }), }), ]), ); @@ -1291,6 +1302,7 @@ describe('LocalAgentExecutor', () => { data: expect.objectContaining({ context: 'tool_call_unauthorized', name: READ_FILE_TOOL_NAME, + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -1344,6 +1356,7 @@ describe('LocalAgentExecutor', () => { context: 'tool_call', name: TASK_COMPLETE_TOOL_NAME, error: expect.stringContaining('Output validation failed'), + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -1390,6 +1403,7 @@ describe('LocalAgentExecutor', () => { type: 'ERROR', data: expect.objectContaining({ error: `Error: Failed to create chat object: ${getErrorMessage(initError)}`, + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -1418,7 +1432,7 @@ describe('LocalAgentExecutor', () => { ]); mockScheduleAgentTools.mockResolvedValueOnce([ { - status: 'error', + status: CoreToolCallStatus.Error, request: { callId: 'call1', name: LS_TOOL_NAME, @@ -1469,6 +1483,7 @@ describe('LocalAgentExecutor', () => { context: 'tool_call', name: LS_TOOL_NAME, error: toolErrorMessage, + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -1491,6 +1506,157 @@ describe('LocalAgentExecutor', () => { expect(output.terminate_reason).toBe(AgentTerminateMode.GOAL); expect(output.result).toBe('Aborted due to tool failure.'); }); + + it('should handle a soft tool rejection (outcome: Cancel) and provide direct instructions to the model', async () => { + const definition = createTestDefinition([LS_TOOL_NAME]); + const executor = await LocalAgentExecutor.create( + definition, + mockConfig, + onActivity, + ); + + // Turn 1: Model calls a tool that will be rejected + mockModelResponse([ + { name: LS_TOOL_NAME, args: { path: '/secret' }, id: 'call1' }, + ]); + mockScheduleAgentTools.mockResolvedValueOnce([ + { + status: 'cancelled', + request: { + callId: 'call1', + name: LS_TOOL_NAME, + args: { path: '/secret' }, + isClientInitiated: false, + prompt_id: 'test-prompt', + }, + tool: {} as AnyDeclarativeTool, + invocation: {} as AnyToolInvocation, + outcome: ToolConfirmationOutcome.Cancel, // Soft rejection + response: { + callId: 'call1', + resultDisplay: '', + responseParts: [ + { + functionResponse: { + name: LS_TOOL_NAME, + response: { + error: + '[Operation Cancelled] Reason: User denied execution.', + }, + id: 'call1', + }, + }, + ], + error: undefined, + errorType: undefined, + contentLength: 0, + }, + }, + ]); + + // Turn 2: Model sees the rejection + consolidated instructions and completes + mockModelResponse([ + { + name: TASK_COMPLETE_TOOL_NAME, + args: { finalResult: 'User rejected access to /secret.' }, + id: 'call2', + }, + ]); + + const output = await executor.run( + { goal: 'Soft rejection test' }, + signal, + ); + + // Verify the activity stream reported the consolidated instruction + expect(activities).toContainEqual( + expect.objectContaining({ + type: 'ERROR', + data: expect.objectContaining({ + context: 'tool_call', + name: LS_TOOL_NAME, + error: expect.stringContaining('User rejected this operation'), + errorType: SubagentActivityErrorType.REJECTED, + }), + }), + ); + + // Verify the instruction was sent back to the model as the tool error + const turn2Params = getMockMessageParams(1); + const parts = turn2Params.message as Part[]; + const errorMsg = parts[0].functionResponse?.response?.['error']; + expect(typeof errorMsg).toBe('string'); + if (typeof errorMsg === 'string') { + expect(errorMsg).toContain('User rejected this operation'); + expect(errorMsg).toContain('acknowledge this, rethink your strategy'); + } + + expect(output.terminate_reason).toBe(AgentTerminateMode.GOAL); + expect(output.result).toBe('User rejected access to /secret.'); + }); + + it('should handle a hard tool abort (cancelled with no outcome) and terminate the agent', async () => { + const definition = createTestDefinition([LS_TOOL_NAME]); + const executor = await LocalAgentExecutor.create( + definition, + mockConfig, + onActivity, + ); + + // Turn 1: Model calls a tool that will be aborted (e.g. Ctrl+C) + mockModelResponse([ + { name: LS_TOOL_NAME, args: { path: '/secret' }, id: 'call1' }, + ]); + mockScheduleAgentTools.mockResolvedValueOnce([ + { + status: 'cancelled', + request: { + callId: 'call1', + name: LS_TOOL_NAME, + args: { path: '/secret' }, + isClientInitiated: false, + prompt_id: 'test-prompt', + }, + tool: {} as AnyDeclarativeTool, + invocation: {} as AnyToolInvocation, + outcome: undefined, // Hard abort + response: { + callId: 'call1', + resultDisplay: '', + responseParts: [ + { + functionResponse: { + name: LS_TOOL_NAME, + response: { error: 'Request cancelled.' }, + id: 'call1', + }, + }, + ], + error: undefined, + errorType: undefined, + contentLength: 0, + }, + }, + ]); + + const output = await executor.run({ goal: 'Hard abort test' }, signal); + + // Verify the activity stream reported the cancellation + expect(activities).toContainEqual( + expect.objectContaining({ + type: 'ERROR', + data: expect.objectContaining({ + context: 'tool_call', + name: LS_TOOL_NAME, + error: 'Request cancelled.', + errorType: SubagentActivityErrorType.CANCELLED, + }), + }), + ); + + // Agent should terminate with ABORTED status + expect(output.terminate_reason).toBe(AgentTerminateMode.ABORTED); + }); }); describe('Model Routing', () => { @@ -1685,6 +1851,7 @@ describe('LocalAgentExecutor', () => { data: expect.objectContaining({ context: 'timeout', error: 'Agent timed out after 0.5 minutes.', + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -1873,6 +2040,7 @@ describe('LocalAgentExecutor', () => { data: expect.objectContaining({ context: 'recovery_turn', error: 'Graceful recovery attempt failed. Reason: stop', + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -1956,6 +2124,7 @@ describe('LocalAgentExecutor', () => { data: expect.objectContaining({ context: 'recovery_turn', error: 'Graceful recovery attempt failed. Reason: stop', + errorType: SubagentActivityErrorType.GENERIC, }), }), ); @@ -2077,6 +2246,7 @@ describe('LocalAgentExecutor', () => { data: expect.objectContaining({ context: 'recovery_turn', error: 'Graceful recovery attempt failed. Reason: stop', + errorType: SubagentActivityErrorType.GENERIC, }), }), ); diff --git a/packages/core/src/agents/local-executor.ts b/packages/core/src/agents/local-executor.ts index c41ae801c4..9dc92d1321 100644 --- a/packages/core/src/agents/local-executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -18,7 +18,10 @@ import { import { ToolRegistry } from '../tools/tool-registry.js'; import { PromptRegistry } from '../prompts/prompt-registry.js'; import { ResourceRegistry } from '../resources/resource-registry.js'; -import { type AnyDeclarativeTool } from '../tools/tools.js'; +import { + type AnyDeclarativeTool, + ToolConfirmationOutcome, +} from '../tools/tools.js'; import { DiscoveredMCPTool, isMcpToolName, @@ -46,6 +49,9 @@ import { DEFAULT_QUERY_STRING, DEFAULT_MAX_TURNS, DEFAULT_MAX_TIME_MINUTES, + SubagentActivityErrorType, + SUBAGENT_REJECTED_ERROR_PREFIX, + SUBAGENT_CANCELLED_ERROR_MESSAGE, type LocalAgentDefinition, type AgentInputs, type OutputObject, @@ -338,6 +344,7 @@ export class LocalAgentExecutor { this.emitActivity('ERROR', { error: `Agent stopped calling tools but did not call '${TASK_COMPLETE_TOOL_NAME}' to finalize the session.`, context: 'protocol_violation', + errorType: SubagentActivityErrorType.GENERIC, }); return { status: 'stop', @@ -471,6 +478,7 @@ export class LocalAgentExecutor { this.emitActivity('ERROR', { error: `Graceful recovery attempt failed. Reason: ${turnResult.status}`, context: 'recovery_turn', + errorType: SubagentActivityErrorType.GENERIC, }); return null; } catch (error) { @@ -478,6 +486,7 @@ export class LocalAgentExecutor { this.emitActivity('ERROR', { error: `Graceful recovery attempt failed: ${String(error)}`, context: 'recovery_turn', + errorType: SubagentActivityErrorType.GENERIC, }); return null; } finally { @@ -683,12 +692,14 @@ export class LocalAgentExecutor { this.emitActivity('ERROR', { error: finalResult, context: 'timeout', + errorType: SubagentActivityErrorType.GENERIC, }); } else if (terminateReason === AgentTerminateMode.MAX_TURNS) { finalResult = `Agent reached max turns limit (${maxTurns}).`; this.emitActivity('ERROR', { error: finalResult, context: 'max_turns', + errorType: SubagentActivityErrorType.GENERIC, }); } else if ( terminateReason === AgentTerminateMode.ERROR_NO_COMPLETE_TASK_CALL @@ -700,6 +711,7 @@ export class LocalAgentExecutor { this.emitActivity('ERROR', { error: finalResult, context: 'protocol_violation', + errorType: SubagentActivityErrorType.GENERIC, }); } } @@ -754,6 +766,7 @@ export class LocalAgentExecutor { this.emitActivity('ERROR', { error: finalResult, context: 'timeout', + errorType: SubagentActivityErrorType.GENERIC, }); return { result: finalResult, @@ -761,7 +774,10 @@ export class LocalAgentExecutor { }; } - this.emitActivity('ERROR', { error: String(error) }); + this.emitActivity('ERROR', { + error: String(error), + errorType: SubagentActivityErrorType.GENERIC, + }); throw error; // Re-throw other errors or external aborts. } finally { deadlineTimer.abort(); @@ -1030,6 +1046,7 @@ export class LocalAgentExecutor { context: 'tool_call', name: toolName, error, + errorType: SubagentActivityErrorType.GENERIC, }); continue; } @@ -1057,6 +1074,7 @@ export class LocalAgentExecutor { context: 'tool_call', name: toolName, error, + errorType: SubagentActivityErrorType.GENERIC, }); continue; } @@ -1099,6 +1117,7 @@ export class LocalAgentExecutor { name: toolName, callId, error, + errorType: SubagentActivityErrorType.GENERIC, }); } } else { @@ -1142,6 +1161,7 @@ export class LocalAgentExecutor { name: toolName, callId, error, + errorType: SubagentActivityErrorType.GENERIC, }); } } @@ -1166,6 +1186,7 @@ export class LocalAgentExecutor { name: toolName, callId, error, + errorType: SubagentActivityErrorType.GENERIC, }); continue; @@ -1213,18 +1234,46 @@ export class LocalAgentExecutor { name: toolName, callId: call.request.callId, error: call.response.error?.message || 'Unknown error', + errorType: SubagentActivityErrorType.GENERIC, }); } else if (call.status === 'cancelled') { - this.emitActivity('ERROR', { - context: 'tool_call', - name: toolName, - callId: call.request.callId, - error: 'Request cancelled.', - }); - aborted = true; + const isSoftRejection = + call.outcome === ToolConfirmationOutcome.Cancel; + + if (isSoftRejection) { + const error = `${SUBAGENT_REJECTED_ERROR_PREFIX} Please acknowledge this, rethink your strategy, and try a different approach. If you cannot proceed without the rejected operation, summarize the issue and use \`${TASK_COMPLETE_TOOL_NAME}\` to report your findings and the blocker.`; + this.emitActivity('ERROR', { + context: 'tool_call', + name: toolName, + callId: call.request.callId, + error, + errorType: SubagentActivityErrorType.REJECTED, + }); + // Soft rejection: we do NOT set aborted=true, allowing the agent to rethink. + + // Provide the direct instruction to the model as the tool error response. + syncResults.set(call.request.callId, { + functionResponse: { + name: toolName, + id: call.request.callId, + response: { error }, + }, + }); + continue; // Skip the generic syncResults.set below + } else { + // Hard abort (Ctrl+C) + this.emitActivity('ERROR', { + context: 'tool_call', + name: toolName, + callId: call.request.callId, + error: SUBAGENT_CANCELLED_ERROR_MESSAGE, + errorType: SubagentActivityErrorType.CANCELLED, + }); + aborted = true; + } } - // Add result to syncResults to preserve order later + // Add result to syncResults for other statuses (success, error, hard abort) syncResults.set(call.request.callId, call.response.responseParts[0]); } } @@ -1335,7 +1384,8 @@ export class LocalAgentExecutor { Important Rules: * You are running in a non-interactive mode. You CANNOT ask the user for input or clarification. * Work systematically using available tools to complete your task. -* Always use absolute paths for file operations. Construct them using the provided "Environment Context".`; +* Always use absolute paths for file operations. Construct them using the provided "Environment Context". +* If a tool call is rejected by the user, acknowledge the rejection, rethink your strategy, and try a different approach. Do not repeatedly attempt the same rejected operation.`; if (this.definition.outputConfig) { finalPrompt += ` diff --git a/packages/core/src/agents/local-invocation.test.ts b/packages/core/src/agents/local-invocation.test.ts index 39c3ea1fe5..34df9844c9 100644 --- a/packages/core/src/agents/local-invocation.test.ts +++ b/packages/core/src/agents/local-invocation.test.ts @@ -19,6 +19,8 @@ import { type SubagentActivityEvent, type AgentInputs, type SubagentProgress, + SubagentActivityErrorType, + SUBAGENT_REJECTED_ERROR_PREFIX, } from './types.js'; import { LocalSubagentInvocation } from './local-invocation.js'; import { LocalAgentExecutor } from './local-executor.js'; @@ -303,6 +305,48 @@ describe('LocalSubagentInvocation', () => { ); }); + 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]; + + 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: 'ERROR', + data: { + name: 'ls', + callId: 'call1', + error: `${SUBAGENT_REJECTED_ERROR_PREFIX} Please acknowledge this, rethink your strategy, and try a different approach. If you cannot proceed without the rejected operation, summarize the issue and use \`complete_task\` to report your findings and the blocker.`, + errorType: SubagentActivityErrorType.REJECTED, + }, + } as SubagentActivityEvent); + } + return { + result: 'Rethinking...', + terminate_reason: AgentTerminateMode.GOAL, + }; + }); + + await invocation.execute(signal, updateOutput); + + expect(updateOutput).toHaveBeenCalledTimes(4); + const lastCall = updateOutput.mock.calls[3][0] as SubagentProgress; + expect(lastCall.recentActivity).toContainEqual( + expect.objectContaining({ + type: 'tool_call', + content: 'ls', + status: 'cancelled', + }), + ); + }); + it('should run successfully without an updateOutput callback', 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 f78faf32c0..e8b98d4744 100644 --- a/packages/core/src/agents/local-invocation.ts +++ b/packages/core/src/agents/local-invocation.ts @@ -18,6 +18,9 @@ import { type SubagentProgress, type SubagentActivityItem, AgentTerminateMode, + SubagentActivityErrorType, + SUBAGENT_REJECTED_ERROR_PREFIX, + SUBAGENT_CANCELLED_ERROR_MESSAGE, } from './types.js'; import { randomUUID } from 'node:crypto'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -171,12 +174,19 @@ export class LocalSubagentInvocation extends BaseToolInvocation< } case 'ERROR': { const error = String(activity.data['error']); - const isCancellation = error === 'Request cancelled.'; + const errorType = activity.data['errorType']; + const isCancellation = + errorType === SubagentActivityErrorType.CANCELLED || + error === SUBAGENT_CANCELLED_ERROR_MESSAGE; + const isRejection = + errorType === SubagentActivityErrorType.REJECTED || + error.startsWith(SUBAGENT_REJECTED_ERROR_PREFIX); + const toolName = activity.data['name'] ? String(activity.data['name']) : undefined; - if (toolName && isCancellation) { + if (toolName && (isCancellation || isRejection)) { for (let i = recentActivity.length - 1; i >= 0; i--) { if ( recentActivity[i].type === 'tool_call' && @@ -188,13 +198,27 @@ export class LocalSubagentInvocation extends BaseToolInvocation< break; } } + } else if (toolName) { + // Mark non-rejection/non-cancellation errors as 'error' + 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 = 'error'; + 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', + type: 'thought', + content: + isCancellation || isRejection ? error : `Error: ${error}`, + status: isCancellation || isRejection ? 'cancelled' : 'error', }); updated = true; break; diff --git a/packages/core/src/agents/types.ts b/packages/core/src/agents/types.ts index 2c703f90fd..7f056c37ab 100644 --- a/packages/core/src/agents/types.ts +++ b/packages/core/src/agents/types.ts @@ -65,6 +65,18 @@ export type RemoteAgentInputs = { query: string }; /** * Structured events emitted during subagent execution for user observability. */ +export enum SubagentActivityErrorType { + REJECTED = 'REJECTED', + CANCELLED = 'CANCELLED', + GENERIC = 'GENERIC', +} + +/** + * Standard error messages for subagent activities. + */ +export const SUBAGENT_REJECTED_ERROR_PREFIX = 'User rejected this operation.'; +export const SUBAGENT_CANCELLED_ERROR_MESSAGE = 'Request cancelled.'; + export interface SubagentActivityEvent { isSubagentActivityEvent: true; agentName: string;