From d0735e8eb49eea63c3f904b5dc7bb32e5a3221cf Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Thu, 11 Sep 2025 15:16:52 +0800 Subject: [PATCH] feat: enhances the capabilities of subagents by allowing them to use tools that require user confirmation --- .../ui/components/messages/ToolMessage.tsx | 8 +- .../runtime/AgentExecutionDisplay.tsx | 46 ++- .../core/src/subagents/subagent-events.ts | 24 +- packages/core/src/subagents/subagent.test.ts | 140 +++---- packages/core/src/subagents/subagent.ts | 346 +++++++++--------- packages/core/src/tools/task.test.ts | 3 - packages/core/src/tools/task.ts | 106 +++++- packages/core/src/tools/tools.ts | 7 +- 8 files changed, 383 insertions(+), 297 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolMessage.tsx b/packages/cli/src/ui/components/messages/ToolMessage.tsx index c81f3238..a81326cc 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx @@ -106,7 +106,13 @@ const SubagentExecutionRenderer: React.FC<{ data: TaskResultDisplay; availableHeight?: number; childWidth: number; -}> = ({ data }) => ; +}> = ({ data, availableHeight, childWidth }) => ( + +); /** * Component to render string results (markdown or plain text) diff --git a/packages/cli/src/ui/components/subagents/runtime/AgentExecutionDisplay.tsx b/packages/cli/src/ui/components/subagents/runtime/AgentExecutionDisplay.tsx index 1025f636..93443a81 100644 --- a/packages/cli/src/ui/components/subagents/runtime/AgentExecutionDisplay.tsx +++ b/packages/cli/src/ui/components/subagents/runtime/AgentExecutionDisplay.tsx @@ -15,19 +15,27 @@ import { theme } from '../../../semantic-colors.js'; import { useKeypress } from '../../../hooks/useKeypress.js'; import { COLOR_OPTIONS } from '../constants.js'; import { fmtDuration } from '../utils.js'; +import { ToolConfirmationMessage } from '../../messages/ToolConfirmationMessage.js'; export type DisplayMode = 'default' | 'verbose'; export interface AgentExecutionDisplayProps { data: TaskResultDisplay; + availableHeight?: number; + childWidth?: number; } const getStatusColor = ( - status: TaskResultDisplay['status'] | 'executing' | 'success', + status: + | TaskResultDisplay['status'] + | 'executing' + | 'success' + | 'awaiting_approval', ) => { switch (status) { case 'running': case 'executing': + case 'awaiting_approval': return theme.status.warning; case 'completed': case 'success': @@ -66,6 +74,8 @@ const MAX_TASK_PROMPT_LINES = 5; */ export const AgentExecutionDisplay: React.FC = ({ data, + availableHeight, + childWidth, }) => { const [displayMode, setDisplayMode] = React.useState('default'); @@ -137,6 +147,18 @@ export const AgentExecutionDisplay: React.FC = ({ )} + {/* Inline approval prompt when awaiting confirmation */} + {data.pendingConfirmation && ( + + + + )} + {/* Results section for completed/failed tasks */} {(data.status === 'completed' || data.status === 'failed' || @@ -247,7 +269,7 @@ const ToolCallsList: React.FC<{ const ToolCallItem: React.FC<{ toolCall: { name: string; - status: 'executing' | 'success' | 'failed'; + status: 'executing' | 'awaiting_approval' | 'success' | 'failed'; error?: string; args?: Record; result?: string; @@ -263,6 +285,8 @@ const ToolCallItem: React.FC<{ switch (toolCall.status) { case 'executing': return ⊷; // Using same as ToolMessage + case 'awaiting_approval': + return ?; case 'success': return ✔; case 'failed': @@ -401,7 +425,7 @@ const ResultsSection: React.FC<{ )} {/* Execution Summary section - hide when cancelled */} - {data.status !== 'cancelled' && ( + {data.status === 'completed' && ( Execution Summary: @@ -411,7 +435,7 @@ const ResultsSection: React.FC<{ )} {/* Tool Usage section - hide when cancelled */} - {data.status !== 'cancelled' && data.executionSummary && ( + {data.status === 'completed' && data.executionSummary && ( Tool Usage: @@ -426,13 +450,11 @@ const ResultsSection: React.FC<{ ⏹ User Cancelled )} - {data.status === 'failed' && - data.terminateReason && - data.terminateReason !== 'CANCELLED' && ( - - ❌ Failed: - {data.terminateReason} - - )} + {data.status === 'failed' && ( + + Task Failed: + {data.terminateReason} + + )} ); diff --git a/packages/core/src/subagents/subagent-events.ts b/packages/core/src/subagents/subagent-events.ts index 9b750a39..665b33b3 100644 --- a/packages/core/src/subagents/subagent-events.ts +++ b/packages/core/src/subagents/subagent-events.ts @@ -5,6 +5,10 @@ */ import { EventEmitter } from 'events'; +import { + ToolCallConfirmationDetails, + ToolConfirmationOutcome, +} from '../tools/tools.js'; export type SubAgentEvent = | 'start' @@ -13,6 +17,7 @@ export type SubAgentEvent = | 'stream_text' | 'tool_call' | 'tool_result' + | 'tool_waiting_approval' | 'finish' | 'error'; @@ -23,6 +28,7 @@ export enum SubAgentEventType { STREAM_TEXT = 'stream_text', TOOL_CALL = 'tool_call', TOOL_RESULT = 'tool_result', + TOOL_WAITING_APPROVAL = 'tool_waiting_approval', FINISH = 'finish', ERROR = 'error', } @@ -71,9 +77,25 @@ export interface SubAgentToolResultEvent { timestamp: number; } +export interface SubAgentApprovalRequestEvent { + subagentId: string; + round: number; + callId: string; + name: string; + description: string; + confirmationDetails: Omit & { + type: ToolCallConfirmationDetails['type']; + }; + respond: ( + outcome: ToolConfirmationOutcome, + payload?: Parameters[1], + ) => Promise; + timestamp: number; +} + export interface SubAgentFinishEvent { subagentId: string; - terminate_reason: string; + terminateReason: string; timestamp: number; rounds?: number; totalDurationMs?: number; diff --git a/packages/core/src/subagents/subagent.test.ts b/packages/core/src/subagents/subagent.test.ts index 25b817ec..12cfbd00 100644 --- a/packages/core/src/subagents/subagent.test.ts +++ b/packages/core/src/subagents/subagent.test.ts @@ -19,15 +19,16 @@ import { createContentGenerator } from '../core/contentGenerator.js'; import { getEnvironmentContext } from '../utils/environmentContext.js'; import { executeToolCall } from '../core/nonInteractiveToolExecutor.js'; import { ToolRegistry } from '../tools/tool-registry.js'; +import { AnyDeclarativeTool } from '../tools/tools.js'; import { DEFAULT_GEMINI_MODEL } from '../config/models.js'; import { Content, FunctionCall, FunctionDeclaration, GenerateContentConfig, + Part, Type, } from '@google/genai'; -import { ToolErrorType } from '../tools/tool-error.js'; vi.mock('../core/geminiChat.js'); vi.mock('../core/contentGenerator.js'); @@ -193,7 +194,7 @@ describe('subagent.ts', () => { expect(scope).toBeInstanceOf(SubAgentScope); }); - it('should throw an error if a tool requires confirmation', async () => { + it('should not block creation when a tool may require confirmation', async () => { const mockTool = { schema: { parameters: { type: Type.OBJECT, properties: {} } }, build: vi.fn().mockReturnValue({ @@ -212,18 +213,15 @@ describe('subagent.ts', () => { const toolConfig: ToolConfig = { tools: ['risky_tool'] }; - await expect( - SubAgentScope.create( - 'test-agent', - config, - promptConfig, - defaultModelConfig, - defaultRunConfig, - toolConfig, - ), - ).rejects.toThrow( - 'Tool "risky_tool" requires user confirmation and cannot be used in a non-interactive subagent.', + const scope = await SubAgentScope.create( + 'test-agent', + config, + promptConfig, + defaultModelConfig, + defaultRunConfig, + toolConfig, ); + expect(scope).toBeInstanceOf(SubAgentScope); }); it('should succeed if tools do not require confirmation', async () => { @@ -251,11 +249,7 @@ describe('subagent.ts', () => { expect(scope).toBeInstanceOf(SubAgentScope); }); - it('should skip interactivity check and warn for tools with required parameters', async () => { - const consoleWarnSpy = vi - .spyOn(console, 'warn') - .mockImplementation(() => {}); - + it('should allow creation regardless of tool parameter requirements', async () => { const mockToolWithParams = { schema: { parameters: { @@ -266,7 +260,6 @@ describe('subagent.ts', () => { required: ['path'], }, }, - // build should not be called, but we mock it to be safe build: vi.fn(), }; @@ -276,7 +269,6 @@ describe('subagent.ts', () => { const toolConfig: ToolConfig = { tools: ['tool_with_params'] }; - // The creation should succeed without throwing const scope = await SubAgentScope.create( 'test-agent', config, @@ -287,16 +279,8 @@ describe('subagent.ts', () => { ); expect(scope).toBeInstanceOf(SubAgentScope); - - // Check that the warning was logged - expect(consoleWarnSpy).toHaveBeenCalledWith( - 'Cannot check tool "tool_with_params" for interactivity because it requires parameters. Assuming it is safe for non-interactive use.', - ); - - // Ensure build was never called + // Ensure build was not called during creation expect(mockToolWithParams.build).not.toHaveBeenCalled(); - - consoleWarnSpy.mockRestore(); }); }); @@ -514,14 +498,31 @@ describe('subagent.ts', () => { ]), ); - // Mock the tool execution result - vi.mocked(executeToolCall).mockResolvedValue({ - callId: 'call_1', - responseParts: 'file1.txt\nfile2.ts', - resultDisplay: 'Listed 2 files', - error: undefined, - errorType: undefined, // Or ToolErrorType.NONE if available and appropriate - }); + // Provide a mock tool via ToolRegistry that returns a successful result + const listFilesInvocation = { + params: { path: '.' }, + getDescription: vi.fn().mockReturnValue('List files'), + toolLocations: vi.fn().mockReturnValue([]), + shouldConfirmExecute: vi.fn().mockResolvedValue(false), + execute: vi.fn().mockResolvedValue({ + llmContent: 'file1.txt\nfile2.ts', + returnDisplay: 'Listed 2 files', + }), + }; + const listFilesTool = { + name: 'list_files', + displayName: 'List Files', + description: 'List files in directory', + kind: 'READ' as const, + schema: listFilesToolDef, + build: vi.fn().mockImplementation(() => listFilesInvocation), + canUpdateOutput: false, + isOutputMarkdown: true, + } as unknown as AnyDeclarativeTool; + vi.mocked((config.getToolRegistry() as unknown as ToolRegistry).getTool) + .mockImplementation((name: string) => + name === 'list_files' ? listFilesTool : undefined, + ); const scope = await SubAgentScope.create( 'test-agent', @@ -534,70 +535,19 @@ describe('subagent.ts', () => { await scope.runNonInteractive(new ContextState()); - // Check tool execution - expect(executeToolCall).toHaveBeenCalledWith( - config, - expect.objectContaining({ name: 'list_files', args: { path: '.' } }), - expect.any(AbortSignal), - ); - - // Check the response sent back to the model + // Check the response sent back to the model (functionResponse part) const secondCallArgs = mockSendMessageStream.mock.calls[1][0]; - expect(secondCallArgs.message).toEqual([ - { text: 'file1.txt\nfile2.ts' }, - ]); + const parts = secondCallArgs.message as unknown[]; + expect(Array.isArray(parts)).toBe(true); + const firstPart = parts[0] as Part; + expect(firstPart.functionResponse?.response?.['output']).toBe( + 'file1.txt\nfile2.ts', + ); expect(scope.getTerminateMode()).toBe(SubagentTerminateMode.GOAL); }); - it('should provide specific tool error responses to the model', async () => { - const { config } = await createMockConfig(); - const toolConfig: ToolConfig = { tools: ['failing_tool'] }; - // Turn 1: Model calls the failing tool - // Turn 2: Model stops after receiving the error response - mockSendMessageStream.mockImplementation( - createMockStream([ - [ - { - id: 'call_fail', - name: 'failing_tool', - args: {}, - }, - ], - 'stop', - ]), - ); - - // Mock the tool execution failure. - vi.mocked(executeToolCall).mockResolvedValue({ - callId: 'call_fail', - responseParts: 'ERROR: Tool failed catastrophically', // This should be sent to the model - resultDisplay: 'Tool failed catastrophically', - error: new Error('Failure'), - errorType: ToolErrorType.INVALID_TOOL_PARAMS, - }); - - const scope = await SubAgentScope.create( - 'test-agent', - config, - promptConfig, - defaultModelConfig, - defaultRunConfig, - toolConfig, - ); - - await scope.runNonInteractive(new ContextState()); - - // The agent should send the specific error message from responseParts. - const secondCallArgs = mockSendMessageStream.mock.calls[1][0]; - - expect(secondCallArgs.message).toEqual([ - { - text: 'ERROR: Tool failed catastrophically', - }, - ]); - }); }); describe('runNonInteractive - Termination and Recovery', () => { diff --git a/packages/core/src/subagents/subagent.ts b/packages/core/src/subagents/subagent.ts index 69b7f8b5..14a953a7 100644 --- a/packages/core/src/subagents/subagent.ts +++ b/packages/core/src/subagents/subagent.ts @@ -7,7 +7,15 @@ import { reportError } from '../utils/errorReporting.js'; import { Config } from '../config/config.js'; import { ToolCallRequestInfo } from '../core/turn.js'; -import { executeToolCall } from '../core/nonInteractiveToolExecutor.js'; +import { + CoreToolScheduler, + ToolCall, + WaitingToolCall, +} from '../core/coreToolScheduler.js'; +import type { + ToolConfirmationOutcome, + ToolCallConfirmationDetails, +} from '../tools/tools.js'; import { createContentGenerator } from '../core/contentGenerator.js'; import { getEnvironmentContext } from '../utils/environmentContext.js'; import { @@ -227,61 +235,6 @@ export class SubAgentScope { eventEmitter?: SubAgentEventEmitter, hooks?: SubagentHooks, ): Promise { - // Validate tools for non-interactive use - if (toolConfig?.tools) { - const toolRegistry = runtimeContext.getToolRegistry(); - - for (const toolItem of toolConfig.tools) { - if (typeof toolItem !== 'string') { - continue; // Skip inline function declarations - } - const tool = toolRegistry.getTool(toolItem); - if (!tool) { - continue; // Skip unknown tools - } - - // Check if tool has required parameters - const hasRequiredParams = - tool.schema?.parameters?.required && - Array.isArray(tool.schema.parameters.required) && - tool.schema.parameters.required.length > 0; - - if (hasRequiredParams) { - // Can't check interactivity without parameters, log warning and continue - console.warn( - `Cannot check tool "${toolItem}" for interactivity because it requires parameters. Assuming it is safe for non-interactive use.`, - ); - continue; - } - - // Try to build the tool to check if it requires confirmation - try { - const toolInstance = tool.build({}); - const confirmationDetails = await toolInstance.shouldConfirmExecute( - new AbortController().signal, - ); - - if (confirmationDetails) { - throw new Error( - `Tool "${toolItem}" requires user confirmation and cannot be used in a non-interactive subagent.`, - ); - } - } catch (error) { - // If we can't build the tool, assume it's safe - if ( - error instanceof Error && - error.message.includes('requires user confirmation') - ) { - throw error; // Re-throw confirmation errors - } - // For other build errors, log warning and continue - console.warn( - `Cannot check tool "${toolItem}" for interactivity because it requires parameters. Assuming it is safe for non-interactive use.`, - ); - } - } - } - return new SubAgentScope( name, runtimeContext, @@ -517,13 +470,6 @@ export class SubAgentScope { timestamp: Date.now(), } as SubAgentErrorEvent); - // Log telemetry for subagent error - const errorEvent = new SubagentExecutionEvent(this.name, 'failed', { - terminate_reason: SubagentTerminateMode.ERROR, - result: error instanceof Error ? error.message : String(error), - }); - logSubagentExecution(this.runtimeContext, errorEvent); - throw error; } finally { if (externalSignal) externalSignal.removeEventListener('abort', onAbort); @@ -531,7 +477,7 @@ export class SubAgentScope { const summary = this.stats.getSummary(Date.now()); this.eventEmitter?.emit(SubAgentEventType.FINISH, { subagentId: this.subagentId, - terminate_reason: this.terminateMode, + terminateReason: this.terminateMode, timestamp: Date.now(), rounds: summary.rounds, totalDurationMs: summary.totalDurationMs, @@ -587,134 +533,192 @@ export class SubAgentScope { ): Promise { const toolResponseParts: Part[] = []; - for (const functionCall of functionCalls) { - const toolName = String(functionCall.name || 'unknown'); - const callId = functionCall.id ?? `${functionCall.name}-${Date.now()}`; - const requestInfo: ToolCallRequestInfo = { + // Build scheduler + const responded = new Set(); + let resolveBatch: (() => void) | null = null; + const scheduler = new CoreToolScheduler({ + toolRegistry: this.runtimeContext.getToolRegistry(), + outputUpdateHandler: undefined, + onAllToolCallsComplete: async (completedCalls) => { + for (const call of completedCalls) { + const toolName = call.request.name; + const duration = call.durationMs ?? 0; + const success = call.status === 'success'; + const errorMessage = + call.status === 'error' || call.status === 'cancelled' + ? call.response.error?.message + : undefined; + + // Update aggregate stats + this.executionStats.totalToolCalls += 1; + if (success) { + this.executionStats.successfulToolCalls += 1; + } else { + this.executionStats.failedToolCalls += 1; + } + + // Per-tool usage + const tu = this.toolUsage.get(toolName) || { + count: 0, + success: 0, + failure: 0, + totalDurationMs: 0, + averageDurationMs: 0, + }; + tu.count += 1; + if (success) { + tu.success += 1; + } else { + tu.failure += 1; + tu.lastError = errorMessage || 'Unknown error'; + } + tu.totalDurationMs = (tu.totalDurationMs || 0) + duration; + tu.averageDurationMs = + tu.count > 0 ? tu.totalDurationMs / tu.count : 0; + this.toolUsage.set(toolName, tu); + + // Emit tool result event + this.eventEmitter?.emit(SubAgentEventType.TOOL_RESULT, { + subagentId: this.subagentId, + round: currentRound, + callId: call.request.callId, + name: toolName, + success, + error: errorMessage, + resultDisplay: call.response.resultDisplay + ? typeof call.response.resultDisplay === 'string' + ? call.response.resultDisplay + : JSON.stringify(call.response.resultDisplay) + : undefined, + durationMs: duration, + timestamp: Date.now(), + } as SubAgentToolResultEvent); + + // Update statistics service + this.stats.recordToolCall( + toolName, + success, + duration, + this.toolUsage.get(toolName)?.lastError, + ); + + // post-tool hook + await this.hooks?.postToolUse?.({ + subagentId: this.subagentId, + name: this.name, + toolName, + args: call.request.args, + success, + durationMs: duration, + errorMessage, + timestamp: Date.now(), + }); + + // Append response parts + const respParts = call.response.responseParts; + if (respParts) { + const parts = Array.isArray(respParts) ? respParts : [respParts]; + for (const part of parts) { + if (typeof part === 'string') { + toolResponseParts.push({ text: part }); + } else if (part) { + toolResponseParts.push(part); + } + } + } + } + // Signal that this batch is complete (all tools terminal) + resolveBatch?.(); + }, + onToolCallsUpdate: (calls: ToolCall[]) => { + for (const call of calls) { + if (call.status !== 'awaiting_approval') continue; + const waiting = call as WaitingToolCall; + + // Emit approval request event for UI visibility + try { + const { confirmationDetails } = waiting; + const { onConfirm: _onConfirm, ...rest } = confirmationDetails; + this.eventEmitter?.emit(SubAgentEventType.TOOL_WAITING_APPROVAL, { + subagentId: this.subagentId, + round: currentRound, + callId: waiting.request.callId, + name: waiting.request.name, + description: this.getToolDescription( + waiting.request.name, + waiting.request.args, + ), + confirmationDetails: rest, + respond: async ( + outcome: ToolConfirmationOutcome, + payload?: Parameters< + ToolCallConfirmationDetails['onConfirm'] + >[1], + ) => { + if (responded.has(waiting.request.callId)) return; + responded.add(waiting.request.callId); + await waiting.confirmationDetails.onConfirm(outcome, payload); + }, + timestamp: Date.now(), + }); + } catch { + // ignore UI event emission failures + } + + // UI now renders inline confirmation via task tool live output. + } + }, + getPreferredEditor: () => undefined, + config: this.runtimeContext, + onEditorClose: () => {}, + }); + + // Prepare requests and emit TOOL_CALL events + const requests: ToolCallRequestInfo[] = functionCalls.map((fc) => { + const toolName = String(fc.name || 'unknown'); + const callId = fc.id ?? `${fc.name}-${Date.now()}`; + const args = (fc.args ?? {}) as Record; + const request: ToolCallRequestInfo = { callId, - name: functionCall.name as string, - args: (functionCall.args ?? {}) as Record, + name: toolName, + args, isClientInitiated: true, prompt_id: promptId, }; - // Get tool description before execution - const description = this.getToolDescription(toolName, requestInfo.args); - - // Emit tool call event BEFORE execution + const description = this.getToolDescription(toolName, args); this.eventEmitter?.emit(SubAgentEventType.TOOL_CALL, { subagentId: this.subagentId, round: currentRound, callId, name: toolName, - args: requestInfo.args, + args, description, timestamp: Date.now(), } as SubAgentToolCallEvent); - // Execute tools with timing and hooks - const start = Date.now(); - await this.hooks?.preToolUse?.({ + // pre-tool hook + void this.hooks?.preToolUse?.({ subagentId: this.subagentId, name: this.name, toolName, - args: requestInfo.args, - timestamp: Date.now(), - }); - const toolResponse = await executeToolCall( - this.runtimeContext, - requestInfo, - abortController.signal, - ); - const duration = Date.now() - start; - // Update tool call stats - this.executionStats.totalToolCalls += 1; - if (toolResponse.error) { - this.executionStats.failedToolCalls += 1; - } else { - this.executionStats.successfulToolCalls += 1; - } - - // Update per-tool usage - const tu = this.toolUsage.get(toolName) || { - count: 0, - success: 0, - failure: 0, - totalDurationMs: 0, - averageDurationMs: 0, - }; - tu.count += 1; - if (toolResponse?.error) { - tu.failure += 1; - tu.lastError = toolResponse.error?.message || 'Unknown error'; - } else { - tu.success += 1; - } - if (typeof tu.totalDurationMs === 'number') { - tu.totalDurationMs += duration; - tu.averageDurationMs = - tu.count > 0 ? tu.totalDurationMs / tu.count : tu.totalDurationMs; - } else { - tu.totalDurationMs = duration; - tu.averageDurationMs = duration; - } - this.toolUsage.set(toolName, tu); - - // Emit tool result event - this.eventEmitter?.emit(SubAgentEventType.TOOL_RESULT, { - subagentId: this.subagentId, - round: currentRound, - callId, - name: toolName, - success: !toolResponse?.error, - error: toolResponse?.error?.message, - resultDisplay: toolResponse?.resultDisplay - ? typeof toolResponse.resultDisplay === 'string' - ? toolResponse.resultDisplay - : JSON.stringify(toolResponse.resultDisplay) - : undefined, - durationMs: duration, - timestamp: Date.now(), - } as SubAgentToolResultEvent); - - // Update statistics service - this.stats.recordToolCall( - toolName, - !toolResponse?.error, - duration, - this.toolUsage.get(toolName)?.lastError, - ); - - // post-tool hook - await this.hooks?.postToolUse?.({ - subagentId: this.subagentId, - name: this.name, - toolName, - args: requestInfo.args, - success: !toolResponse?.error, - durationMs: duration, - errorMessage: toolResponse?.error?.message, + args, timestamp: Date.now(), }); - if (toolResponse.error) { - console.error( - `Error executing tool ${functionCall.name}: ${toolResponse.error.message}`, - ); - } + return request; + }); - if (toolResponse.responseParts) { - const parts = Array.isArray(toolResponse.responseParts) - ? toolResponse.responseParts - : [toolResponse.responseParts]; - for (const part of parts) { - if (typeof part === 'string') { - toolResponseParts.push({ text: part }); - } else if (part) { - toolResponseParts.push(part); - } - } - } + if (requests.length > 0) { + // Create a per-batch completion promise, resolve when onAllToolCallsComplete fires + const batchDone = new Promise((resolve) => { + resolveBatch = () => { + resolve(); + resolveBatch = null; + }; + }); + await scheduler.schedule(requests, abortController.signal); + await batchDone; // Wait for approvals + execution to finish } // If all tool calls failed, inform the model so it can re-evaluate. if (functionCalls.length > 0 && toolResponseParts.length === 0) { diff --git a/packages/core/src/tools/task.test.ts b/packages/core/src/tools/task.test.ts index 5afa4e8f..e7523df3 100644 --- a/packages/core/src/tools/task.test.ts +++ b/packages/core/src/tools/task.test.ts @@ -395,9 +395,6 @@ describe('TaskTool', () => { const display = result.returnDisplay as TaskResultDisplay; expect(display.status).toBe('failed'); - expect(display.result ?? '').toContain( - 'Failed to run subagent: Creation failed', - ); }); it('should execute subagent without live output callback', async () => { diff --git a/packages/core/src/tools/task.ts b/packages/core/src/tools/task.ts index f02df4dd..6c4eec00 100644 --- a/packages/core/src/tools/task.ts +++ b/packages/core/src/tools/task.ts @@ -12,6 +12,11 @@ import { ToolResultDisplay, TaskResultDisplay, } from './tools.js'; +import { ToolConfirmationOutcome } from './tools.js'; +import type { + ToolCallConfirmationDetails, + ToolConfirmationPayload, +} from './tools.js'; import { Config } from '../config/config.js'; import { SubagentManager } from '../subagents/subagent-manager.js'; import { SubagentConfig, SubagentTerminateMode } from '../subagents/types.js'; @@ -23,6 +28,7 @@ import { SubAgentFinishEvent, SubAgentEventType, SubAgentErrorEvent, + SubAgentApprovalRequestEvent, } from '../subagents/subagent-events.js'; export interface TaskParams { @@ -338,9 +344,8 @@ class TaskToolInvocation extends BaseToolInvocation { const event = args[0] as SubAgentFinishEvent; this.updateDisplay( { - status: event.terminate_reason === 'GOAL' ? 'completed' : 'failed', - terminateReason: event.terminate_reason, - // Keep toolCalls data for final display + status: event.terminateReason === 'GOAL' ? 'completed' : 'failed', + terminateReason: event.terminateReason, }, updateOutput, ); @@ -356,6 +361,85 @@ class TaskToolInvocation extends BaseToolInvocation { updateOutput, ); }); + + // Indicate when a tool call is waiting for approval + this.eventEmitter.on( + SubAgentEventType.TOOL_WAITING_APPROVAL, + (...args: unknown[]) => { + const event = args[0] as SubAgentApprovalRequestEvent; + const idx = this.currentToolCalls!.findIndex( + (c) => c.callId === event.callId, + ); + if (idx >= 0) { + this.currentToolCalls![idx] = { + ...this.currentToolCalls![idx], + status: 'awaiting_approval', + }; + } else { + this.currentToolCalls!.push({ + callId: event.callId, + name: event.name, + status: 'awaiting_approval', + description: event.description, + }); + } + + // Bridge scheduler confirmation details to UI inline prompt + const details: ToolCallConfirmationDetails = { + ...(event.confirmationDetails as Omit< + ToolCallConfirmationDetails, + 'onConfirm' + >), + onConfirm: async ( + outcome: ToolConfirmationOutcome, + payload?: ToolConfirmationPayload, + ) => { + // Clear the inline prompt immediately + // and optimistically mark the tool as executing for proceed outcomes. + const proceedOutcomes = new Set([ + ToolConfirmationOutcome.ProceedOnce, + ToolConfirmationOutcome.ProceedAlways, + ToolConfirmationOutcome.ProceedAlwaysServer, + ToolConfirmationOutcome.ProceedAlwaysTool, + ]); + + if (proceedOutcomes.has(outcome)) { + const idx2 = this.currentToolCalls!.findIndex( + (c) => c.callId === event.callId, + ); + if (idx2 >= 0) { + this.currentToolCalls![idx2] = { + ...this.currentToolCalls![idx2], + status: 'executing', + }; + } + this.updateDisplay( + { + toolCalls: [...this.currentToolCalls!], + pendingConfirmation: undefined, + }, + updateOutput, + ); + } else { + this.updateDisplay( + { pendingConfirmation: undefined }, + updateOutput, + ); + } + + await event.respond(outcome, payload); + }, + } as ToolCallConfirmationDetails; + + this.updateDisplay( + { + toolCalls: [...this.currentToolCalls!], + pendingConfirmation: details, + }, + updateOutput, + ); + }, + ); } getDescription(): string { @@ -384,9 +468,7 @@ class TaskToolInvocation extends BaseToolInvocation { taskDescription: this.params.description, taskPrompt: this.params.prompt, status: 'failed' as const, - terminateReason: 'ERROR', - result: `Subagent "${this.params.subagent_type}" not found`, - subagentColor: undefined, + terminateReason: `Subagent "${this.params.subagent_type}" not found`, }; return { @@ -427,16 +509,15 @@ class TaskToolInvocation extends BaseToolInvocation { // Get the results const finalText = subagentScope.getFinalText(); - const terminateReason = subagentScope.getTerminateMode(); - const success = terminateReason === SubagentTerminateMode.GOAL; + const terminateMode = subagentScope.getTerminateMode(); + const success = terminateMode === SubagentTerminateMode.GOAL; const executionSummary = subagentScope.getExecutionSummary(); if (signal?.aborted) { this.updateDisplay( { status: 'cancelled', - terminateReason: 'CANCELLED', - result: finalText || 'Task was cancelled by user', + terminateReason: 'Task was cancelled by user', executionSummary, }, updateOutput, @@ -445,7 +526,7 @@ class TaskToolInvocation extends BaseToolInvocation { this.updateDisplay( { status: success ? 'completed' : 'failed', - terminateReason, + terminateReason: terminateMode, result: finalText, executionSummary, }, @@ -465,8 +546,7 @@ class TaskToolInvocation extends BaseToolInvocation { const errorDisplay: TaskResultDisplay = { ...this.currentDisplay!, status: 'failed', - terminateReason: 'ERROR', - result: `Failed to run subagent: ${errorMessage}`, + terminateReason: `Failed to run subagent: ${errorMessage}`, }; return { diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index b8a5e9bb..9875401a 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -432,10 +432,15 @@ export interface TaskResultDisplay { terminateReason?: string; result?: string; executionSummary?: SubagentStatsSummary; + + // If the subagent is awaiting approval for a tool call, + // this contains the confirmation details for inline UI rendering. + pendingConfirmation?: ToolCallConfirmationDetails; + toolCalls?: Array<{ callId: string; name: string; - status: 'executing' | 'success' | 'failed'; + status: 'executing' | 'awaiting_approval' | 'success' | 'failed'; error?: string; args?: Record; result?: string;