From d762859963acccac4eacef86eb6ab81850619bf4 Mon Sep 17 00:00:00 2001 From: Charles Packer Date: Fri, 7 Nov 2025 18:23:16 -0800 Subject: [PATCH] refactor: extract shared approval batch execution logic (#82) Co-authored-by: Letta --- src/agent/approval-execution.ts | 125 ++++++++++++++++++++++++++++++ src/cli/App.tsx | 106 +++++--------------------- src/headless.ts | 130 ++++++-------------------------- 3 files changed, 166 insertions(+), 195 deletions(-) create mode 100644 src/agent/approval-execution.ts diff --git a/src/agent/approval-execution.ts b/src/agent/approval-execution.ts new file mode 100644 index 0000000..0650255 --- /dev/null +++ b/src/agent/approval-execution.ts @@ -0,0 +1,125 @@ +// src/agent/approval-execution.ts +// Shared logic for executing approval batches (used by both interactive and headless modes) + +import type { ApprovalRequest } from "../cli/helpers/stream"; +import { executeTool } from "../tools/manager"; + +export type ApprovalDecision = + | { type: "approve"; approval: ApprovalRequest } + | { type: "deny"; approval: ApprovalRequest; reason: string }; + +export type ApprovalResult = { + type: "tool" | "approval"; + tool_call_id: string; + tool_return?: string; + status?: "success" | "error"; + stdout?: string[]; + stderr?: string[]; + approve?: boolean; + reason?: string; +}; + +/** + * Execute a batch of approval decisions and format results for the backend. + * + * This function handles: + * - Executing approved tools (with error handling) + * - Formatting denials + * - Combining all results into a single batch + * + * Used by both interactive (App.tsx) and headless (headless.ts) modes. + * + * @param decisions - Array of approve/deny decisions for each tool + * @param onChunk - Optional callback to update UI with tool results (for interactive mode) + * @returns Array of formatted results ready to send to backend + */ +export async function executeApprovalBatch( + decisions: ApprovalDecision[], + onChunk?: (chunk: any) => void, +): Promise { + const results: ApprovalResult[] = []; + + for (const decision of decisions) { + if (decision.type === "approve") { + // Execute the approved tool + try { + const parsedArgs = + typeof decision.approval.toolArgs === "string" + ? JSON.parse(decision.approval.toolArgs) + : decision.approval.toolArgs || {}; + + const toolResult = await executeTool( + decision.approval.toolName, + parsedArgs, + ); + + // Update UI if callback provided (interactive mode) + if (onChunk) { + onChunk({ + message_type: "tool_return_message", + id: "dummy", + date: new Date().toISOString(), + tool_call_id: decision.approval.toolCallId, + tool_return: toolResult.toolReturn, + status: toolResult.status, + stdout: toolResult.stdout, + stderr: toolResult.stderr, + }); + } + + results.push({ + type: "tool", + tool_call_id: decision.approval.toolCallId, + tool_return: toolResult.toolReturn, + status: toolResult.status, + stdout: toolResult.stdout, + stderr: toolResult.stderr, + }); + } catch (e) { + // Still need to send error result to backend for this tool + const errorMessage = `Error executing tool: ${String(e)}`; + + // Update UI if callback provided + if (onChunk) { + onChunk({ + message_type: "tool_return_message", + id: "dummy", + date: new Date().toISOString(), + tool_call_id: decision.approval.toolCallId, + tool_return: errorMessage, + status: "error", + }); + } + + results.push({ + type: "tool", + tool_call_id: decision.approval.toolCallId, + tool_return: errorMessage, + status: "error", + }); + } + } else { + // Format denial for backend + // Update UI if callback provided + if (onChunk) { + onChunk({ + message_type: "tool_return_message", + id: "dummy", + date: new Date().toISOString(), + tool_call_id: decision.approval.toolCallId, + tool_return: `Error: request to call tool denied. User reason: ${decision.reason}`, + status: "error", + }); + } + + results.push({ + type: "approval", + tool_call_id: decision.approval.toolCallId, + approve: false, + reason: decision.reason, + }); + } + } + + return results; +} diff --git a/src/cli/App.tsx b/src/cli/App.tsx index af33fb9..b75d983 100644 --- a/src/cli/App.tsx +++ b/src/cli/App.tsx @@ -1169,94 +1169,28 @@ export default function App({ ...(additionalDecision ? [additionalDecision] : []), ]; - // Execute approved tools and format results - const executedResults: Array<{ - type: "tool" | "approval"; - tool_call_id: string; - tool_return?: string; - status?: "success" | "error"; - stdout?: string[]; - stderr?: string[]; - approve?: boolean; - reason?: string; - }> = []; - - for (const decision of allDecisions) { - if (decision.type === "approve") { - // Execute the approved tool - try { - const parsedArgs = safeJsonParseOr>( - decision.approval.toolArgs, - {}, + // Execute approved tools and format results using shared function + const { executeApprovalBatch } = await import( + "../agent/approval-execution" + ); + const executedResults = await executeApprovalBatch( + allDecisions, + (chunk) => { + onChunk(buffersRef.current, chunk); + // Also log errors to the UI error display + if ( + chunk.status === "error" && + chunk.message_type === "tool_return_message" + ) { + const isToolError = chunk.tool_return?.startsWith( + "Error executing tool:", ); - const toolResult = await executeTool( - decision.approval.toolName, - parsedArgs, - ); - - // Update buffers with tool return for UI - onChunk(buffersRef.current, { - message_type: "tool_return_message", - id: "dummy", - date: new Date().toISOString(), - tool_call_id: decision.approval.toolCallId, - tool_return: toolResult.toolReturn, - status: toolResult.status, - stdout: toolResult.stdout, - stderr: toolResult.stderr, - }); - - executedResults.push({ - type: "tool", - tool_call_id: decision.approval.toolCallId, - tool_return: toolResult.toolReturn, - status: toolResult.status, - stdout: toolResult.stdout, - stderr: toolResult.stderr, - }); - } catch (e) { - appendError(String(e)); - - // Still need to send error result to backend for this tool - const errorMessage = `Error executing tool: ${String(e)}`; - - // Update buffers with error for UI - onChunk(buffersRef.current, { - message_type: "tool_return_message", - id: "dummy", - date: new Date().toISOString(), - tool_call_id: decision.approval.toolCallId, - tool_return: errorMessage, - status: "error", - }); - - executedResults.push({ - type: "tool", - tool_call_id: decision.approval.toolCallId, - tool_return: errorMessage, - status: "error", - }); + if (isToolError) { + appendError(chunk.tool_return); + } } - } else { - // Format denial for backend - // Update buffers with denial for UI - onChunk(buffersRef.current, { - message_type: "tool_return_message", - id: "dummy", - date: new Date().toISOString(), - tool_call_id: decision.approval.toolCallId, - tool_return: `Error: request to call tool denied. User reason: ${decision.reason}`, - status: "error", - }); - - executedResults.push({ - type: "approval", - tool_call_id: decision.approval.toolCallId, - approve: false, - reason: decision.reason, - }); - } - } + }, + ); // Combine with auto-handled and auto-denied results const allResults = [ diff --git a/src/headless.ts b/src/headless.ts index 248ce8c..cd418d2 100644 --- a/src/headless.ts +++ b/src/headless.ts @@ -258,68 +258,28 @@ export async function handleHeadlessCommand( }); } - // Phase 2: Execute approved tools and format results - const executedResults: Array<{ - type: "tool" | "approval"; - tool_call_id: string; - tool_return?: string; - status?: "success" | "error"; - stdout?: string[]; - stderr?: string[]; - approve?: boolean; - reason?: string; - }> = []; + // Phase 2: Execute approved tools and format results using shared function + const { executeApprovalBatch } = await import( + "./agent/approval-execution" + ); - for (const decision of decisions) { - if (decision.type === "approve") { - try { - const parsedArgs = safeJsonParseOr>( - decision.approval.toolArgs || "{}", - {}, + // Emit auto_approval events for stream-json format + if (outputFormat === "stream-json") { + for (const decision of decisions) { + if (decision.type === "approve") { + console.log( + JSON.stringify({ + type: "auto_approval", + tool_name: decision.approval.toolName, + tool_call_id: decision.approval.toolCallId, + }), ); - const toolResult = await executeTool( - decision.approval.toolName, - parsedArgs, - ); - - // Emit auto_approval event for stream-json for visibility - if (outputFormat === "stream-json") { - console.log( - JSON.stringify({ - type: "auto_approval", - tool_name: decision.approval.toolName, - tool_call_id: decision.approval.toolCallId, - }), - ); - } - - executedResults.push({ - type: "tool", - tool_call_id: decision.approval.toolCallId, - tool_return: toolResult.toolReturn, - status: toolResult.status, - stdout: toolResult.stdout, - stderr: toolResult.stderr, - }); - } catch (e) { - const errorMessage = `Error executing tool: ${String(e)}`; - executedResults.push({ - type: "tool", - tool_call_id: decision.approval.toolCallId, - tool_return: errorMessage, - status: "error", - }); } - } else { - executedResults.push({ - type: "approval", - tool_call_id: decision.approval.toolCallId, - approve: false, - reason: decision.reason, - }); } } + const executedResults = await executeApprovalBatch(decisions); + // Send all results in one batch const approvalInput: ApprovalCreate = { type: "approval", @@ -683,59 +643,11 @@ export async function handleHeadlessCommand( }); } - // Phase 2: Execute all approved tools and format results - const executedResults: Array<{ - type: "tool" | "approval"; - tool_call_id: string; - tool_return?: string; - status?: "success" | "error"; - stdout?: string[]; - stderr?: string[]; - approve?: boolean; - reason?: string; - }> = []; - - for (const decision of decisions) { - if (decision.type === "approve") { - // Execute the approved tool - try { - const parsedArgs = safeJsonParseOr>( - decision.approval.toolArgs, - {}, - ); - const toolResult = await executeTool( - decision.approval.toolName, - parsedArgs, - ); - - executedResults.push({ - type: "tool", - tool_call_id: decision.approval.toolCallId, - tool_return: toolResult.toolReturn, - status: toolResult.status, - stdout: toolResult.stdout, - stderr: toolResult.stderr, - }); - } catch (e) { - // Still need to send error result to backend for this tool - const errorMessage = `Error executing tool: ${String(e)}`; - executedResults.push({ - type: "tool", - tool_call_id: decision.approval.toolCallId, - tool_return: errorMessage, - status: "error", - }); - } - } else { - // Format denial for backend - executedResults.push({ - type: "approval", - tool_call_id: decision.approval.toolCallId, - approve: false, - reason: decision.reason, - }); - } - } + // Phase 2: Execute all approved tools and format results using shared function + const { executeApprovalBatch } = await import( + "./agent/approval-execution" + ); + const executedResults = await executeApprovalBatch(decisions); // Send all results in one batch currentInput = [