feat(core): resilient subagent tool rejection with contextual feedback (#22951)

This commit is contained in:
Abhi
2026-03-18 21:09:37 -04:00
committed by GitHub
parent 07140c690a
commit 35c933ab6a
5 changed files with 323 additions and 23 deletions
+177 -7
View File
@@ -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,
}),
}),
);
+61 -11
View File
@@ -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<TOutput extends z.ZodTypeAny> {
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<TOutput extends z.ZodTypeAny> {
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<TOutput extends z.ZodTypeAny> {
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<TOutput extends z.ZodTypeAny> {
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<TOutput extends z.ZodTypeAny> {
this.emitActivity('ERROR', {
error: finalResult,
context: 'protocol_violation',
errorType: SubagentActivityErrorType.GENERIC,
});
}
}
@@ -754,6 +766,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
this.emitActivity('ERROR', {
error: finalResult,
context: 'timeout',
errorType: SubagentActivityErrorType.GENERIC,
});
return {
result: finalResult,
@@ -761,7 +774,10 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
};
}
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<TOutput extends z.ZodTypeAny> {
context: 'tool_call',
name: toolName,
error,
errorType: SubagentActivityErrorType.GENERIC,
});
continue;
}
@@ -1057,6 +1074,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
context: 'tool_call',
name: toolName,
error,
errorType: SubagentActivityErrorType.GENERIC,
});
continue;
}
@@ -1099,6 +1117,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
name: toolName,
callId,
error,
errorType: SubagentActivityErrorType.GENERIC,
});
}
} else {
@@ -1142,6 +1161,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
name: toolName,
callId,
error,
errorType: SubagentActivityErrorType.GENERIC,
});
}
}
@@ -1166,6 +1186,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
name: toolName,
callId,
error,
errorType: SubagentActivityErrorType.GENERIC,
});
continue;
@@ -1213,18 +1234,46 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
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<TOutput extends z.ZodTypeAny> {
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 += `
@@ -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];
+29 -5
View File
@@ -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;
+12
View File
@@ -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;