diff --git a/src/cli/App.tsx b/src/cli/App.tsx index 6b64a88..f66521b 100644 --- a/src/cli/App.tsx +++ b/src/cli/App.tsx @@ -94,7 +94,13 @@ import { toLines, } from "./helpers/accumulator"; import { backfillBuffers } from "./helpers/backfill"; +import { + type AdvancedDiffSuccess, + computeAdvancedDiff, + parsePatchToAdvancedDiff, +} from "./helpers/diff"; import { formatErrorDetails } from "./helpers/errorFormatter"; +import { parsePatchOperations } from "./helpers/formatArgsDisplay"; import { buildMemoryReminder, parseMemoryPreference, @@ -117,6 +123,11 @@ import { clearSubagentsByIds, } from "./helpers/subagentState"; import { getRandomThinkingVerb } from "./helpers/thinkingMessages"; +import { + isFileEditTool, + isFileWriteTool, + isPatchTool, +} from "./helpers/toolNameMapping"; import { isFancyUITool, isTaskTool } from "./helpers/toolNameMapping.js"; import { useSuspend } from "./hooks/useSuspend/useSuspend.ts"; import { useSyncedState } from "./hooks/useSyncedState"; @@ -682,6 +693,10 @@ export default function App({ if ("phase" in ln && ln.phase === "finished") { emittedIdsRef.current.add(id); newlyCommitted.push({ ...ln }); + // Note: We intentionally don't cleanup precomputedDiffs here because + // the Static area renders AFTER this function returns (on next React tick), + // and the diff needs to be available for ToolCallMessage to render. + // The diffs will be cleaned up when the session ends or on next session start. } } @@ -719,6 +734,12 @@ export default function App({ // Track whether we've already backfilled history (should only happen once) const hasBackfilledRef = useRef(false); + // Cache precomputed diffs from approval dialogs for tool return rendering + // Key: toolCallId or "toolCallId:filePath" for Patch operations + const precomputedDiffsRef = useRef>( + new Map(), + ); + // Recompute UI state from buffers after chunks (micro-batched) const refreshDerived = useCallback(() => { const b = buffersRef.current; @@ -1255,6 +1276,77 @@ export default function App({ } } + // Precompute diffs for auto-allowed file edit tools before execution + for (const ac of autoAllowed) { + const toolName = ac.approval.toolName; + const toolCallId = ac.approval.toolCallId; + try { + const args = JSON.parse(ac.approval.toolArgs || "{}"); + + if (isFileWriteTool(toolName)) { + const filePath = args.file_path as string | undefined; + if (filePath) { + const result = computeAdvancedDiff({ + kind: "write", + filePath, + content: (args.content as string) || "", + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } + } else if (isFileEditTool(toolName)) { + const filePath = args.file_path as string | undefined; + if (filePath) { + // Check if it's a multi-edit (has edits array) or single edit + if (args.edits && Array.isArray(args.edits)) { + const result = computeAdvancedDiff({ + kind: "multi_edit", + filePath, + edits: args.edits as Array<{ + old_string: string; + new_string: string; + replace_all?: boolean; + }>, + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } else { + const result = computeAdvancedDiff({ + kind: "edit", + filePath, + oldString: (args.old_string as string) || "", + newString: (args.new_string as string) || "", + replaceAll: args.replace_all as boolean | undefined, + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } + } + } else if (isPatchTool(toolName) && args.input) { + // Patch tools - parse hunks directly (patches ARE diffs) + const operations = parsePatchOperations(args.input as string); + for (const op of operations) { + const key = `${toolCallId}:${op.path}`; + if (op.kind === "add" || op.kind === "update") { + const result = parsePatchToAdvancedDiff( + op.patchLines, + op.path, + ); + if (result) { + precomputedDiffsRef.current.set(key, result); + } + } + // Delete operations don't need diffs + } + } + } catch { + // Ignore errors in diff computation for auto-allowed tools + } + } + // Execute auto-allowed tools (sequential for writes, parallel for reads) const autoAllowedResults = await executeAutoAllowedTools( autoAllowed, @@ -3711,6 +3803,79 @@ DO NOT respond to these messages or otherwise consider them in your response unl // If all approvals can be auto-handled (yolo mode), process them immediately if (needsUserInput.length === 0) { + // Precompute diffs for auto-allowed file edit tools before execution + for (const ac of autoAllowed) { + const toolName = ac.approval.toolName; + const toolCallId = ac.approval.toolCallId; + try { + const args = JSON.parse(ac.approval.toolArgs || "{}"); + + if (isFileWriteTool(toolName)) { + const filePath = args.file_path as string | undefined; + if (filePath) { + const result = computeAdvancedDiff({ + kind: "write", + filePath, + content: (args.content as string) || "", + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } + } else if (isFileEditTool(toolName)) { + const filePath = args.file_path as string | undefined; + if (filePath) { + // Check if it's a multi-edit (has edits array) or single edit + if (args.edits && Array.isArray(args.edits)) { + const result = computeAdvancedDiff({ + kind: "multi_edit", + filePath, + edits: args.edits as Array<{ + old_string: string; + new_string: string; + replace_all?: boolean; + }>, + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } else { + const result = computeAdvancedDiff({ + kind: "edit", + filePath, + oldString: (args.old_string as string) || "", + newString: (args.new_string as string) || "", + replaceAll: args.replace_all as boolean | undefined, + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } + } + } else if (isPatchTool(toolName) && args.input) { + // Patch tools - parse hunks directly (patches ARE diffs) + const operations = parsePatchOperations( + args.input as string, + ); + for (const op of operations) { + const key = `${toolCallId}:${op.path}`; + if (op.kind === "add" || op.kind === "update") { + const result = parsePatchToAdvancedDiff( + op.patchLines, + op.path, + ); + if (result) { + precomputedDiffsRef.current.set(key, result); + } + } + // Delete operations don't need diffs + } + } + } catch { + // Ignore errors in diff computation for auto-allowed tools + } + } + // Execute auto-allowed tools (sequential for writes, parallel for reads) const autoAllowedResults = await executeAutoAllowedTools( autoAllowed, @@ -3794,6 +3959,79 @@ DO NOT respond to these messages or otherwise consider them in your response unl .filter(Boolean) as ApprovalContext[], ); + // Precompute diffs for auto-allowed file edit tools before execution + for (const ac of autoAllowed) { + const toolName = ac.approval.toolName; + const toolCallId = ac.approval.toolCallId; + try { + const args = JSON.parse(ac.approval.toolArgs || "{}"); + + if (isFileWriteTool(toolName)) { + const filePath = args.file_path as string | undefined; + if (filePath) { + const result = computeAdvancedDiff({ + kind: "write", + filePath, + content: (args.content as string) || "", + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } + } else if (isFileEditTool(toolName)) { + const filePath = args.file_path as string | undefined; + if (filePath) { + // Check if it's a multi-edit (has edits array) or single edit + if (args.edits && Array.isArray(args.edits)) { + const result = computeAdvancedDiff({ + kind: "multi_edit", + filePath, + edits: args.edits as Array<{ + old_string: string; + new_string: string; + replace_all?: boolean; + }>, + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } else { + const result = computeAdvancedDiff({ + kind: "edit", + filePath, + oldString: (args.old_string as string) || "", + newString: (args.new_string as string) || "", + replaceAll: args.replace_all as boolean | undefined, + }); + if (result.mode === "advanced") { + precomputedDiffsRef.current.set(toolCallId, result); + } + } + } + } else if (isPatchTool(toolName) && args.input) { + // Patch tools - parse hunks directly (patches ARE diffs) + const operations = parsePatchOperations( + args.input as string, + ); + for (const op of operations) { + const key = `${toolCallId}:${op.path}`; + if (op.kind === "add" || op.kind === "update") { + const result = parsePatchToAdvancedDiff( + op.patchLines, + op.path, + ); + if (result) { + precomputedDiffsRef.current.set(key, result); + } + } + // Delete operations don't need diffs + } + } + } catch { + // Ignore errors in diff computation for auto-allowed tools + } + } + // Execute auto-allowed tools (sequential for writes, parallel for reads) const autoAllowedWithResults = await executeAutoAllowedTools( autoAllowed, @@ -4083,51 +4321,64 @@ DO NOT respond to these messages or otherwise consider them in your response unl ); // Handle approval callbacks - sequential review - const handleApproveCurrent = useCallback(async () => { - if (isExecutingTool) return; + const handleApproveCurrent = useCallback( + async (diffs?: Map) => { + if (isExecutingTool) return; - const currentIndex = approvalResults.length; - const currentApproval = pendingApprovals[currentIndex]; + const currentIndex = approvalResults.length; + const currentApproval = pendingApprovals[currentIndex]; - if (!currentApproval) return; + if (!currentApproval) return; - setIsExecutingTool(true); + // Store precomputed diffs before execution + if (diffs) { + for (const [key, diff] of diffs) { + precomputedDiffsRef.current.set(key, diff); + } + } - try { - // Store approval decision (don't execute yet - batch execute after all approvals) - const decision = { - type: "approve" as const, - approval: currentApproval, - }; + setIsExecutingTool(true); - // Check if we're done with all approvals - if (currentIndex + 1 >= pendingApprovals.length) { - // All approvals collected, execute and send to backend - // sendAllResults owns the lock release via its finally block - await sendAllResults(decision); - } else { - // Not done yet, store decision and show next approval - setApprovalResults((prev) => [...prev, decision]); + try { + // Store approval decision (don't execute yet - batch execute after all approvals) + const decision = { + type: "approve" as const, + approval: currentApproval, + }; + + // Check if we're done with all approvals + if (currentIndex + 1 >= pendingApprovals.length) { + // All approvals collected, execute and send to backend + // sendAllResults owns the lock release via its finally block + await sendAllResults(decision); + } else { + // Not done yet, store decision and show next approval + setApprovalResults((prev) => [...prev, decision]); + setIsExecutingTool(false); + } + } catch (e) { + const errorDetails = formatErrorDetails(e, agentId); + appendError(errorDetails); + setStreaming(false); setIsExecutingTool(false); } - } catch (e) { - const errorDetails = formatErrorDetails(e, agentId); - appendError(errorDetails); - setStreaming(false); - setIsExecutingTool(false); - } - }, [ - agentId, - pendingApprovals, - approvalResults, - sendAllResults, - appendError, - isExecutingTool, - setStreaming, - ]); + }, + [ + agentId, + pendingApprovals, + approvalResults, + sendAllResults, + appendError, + isExecutingTool, + setStreaming, + ], + ); const handleApproveAlways = useCallback( - async (scope?: "project" | "session") => { + async ( + scope?: "project" | "session", + diffs?: Map, + ) => { if (isExecutingTool) return; // For now, just handle the first approval with approve-always @@ -4159,7 +4410,8 @@ DO NOT respond to these messages or otherwise consider them in your response unl refreshDerived(); // Approve current tool (handleApproveCurrent manages the execution guard) - await handleApproveCurrent(); + // Pass diffs through to store them before execution + await handleApproveCurrent(diffs); }, [ approvalResults, @@ -5011,7 +5263,10 @@ Plan file path: ${planFilePath}`; ) : item.kind === "assistant" ? ( ) : item.kind === "tool_call" ? ( - + ) : item.kind === "subagent_group" ? ( ) : item.kind === "error" ? ( @@ -5053,7 +5308,10 @@ Plan file path: ${planFilePath}`; ) : ln.kind === "assistant" ? ( ) : ln.kind === "tool_call" ? ( - + ) : ln.kind === "error" ? ( ) : ln.kind === "status" ? ( diff --git a/src/cli/components/AdvancedDiffRenderer.tsx b/src/cli/components/AdvancedDiffRenderer.tsx index 065223c..2e245df 100644 --- a/src/cli/components/AdvancedDiffRenderer.tsx +++ b/src/cli/components/AdvancedDiffRenderer.tsx @@ -60,7 +60,7 @@ function Line({ text, pairText, gutterWidth, - contentWidth, + columns, enableWord, }: { kind: "context" | "remove" | "add"; @@ -68,7 +68,7 @@ function Line({ text: string; pairText?: string; // when '-' followed by '+' to highlight words gutterWidth: number; - contentWidth: number; + columns: number; enableWord: boolean; }) { const symbol = kind === "add" ? "+" : kind === "remove" ? "-" : " "; @@ -106,21 +106,22 @@ function Line({ : Diff.diffChars(text, pairText) : null; - // Compute remaining width for the text area within this row - const textWidth = Math.max(0, contentWidth - gutterWidth - 2); + // Build prefix: " 1 + " (line number + symbol) + const linePrefix = `${padLeft(displayNo, gutterWidth)} ${symbol} `; + const prefixWidth = linePrefix.length; + const contentWidth = Math.max(0, columns - prefixWidth); return ( - - - {padLeft(displayNo, gutterWidth)} + + + + {padLeft(displayNo, gutterWidth)}{" "} + {symbol}{" "} + - - {symbol} - - - + {charParts ? ( - + {charParts.map((p, i) => { // For '-' lines: render removed + unchanged; drop added if (kind === "remove") { @@ -138,7 +139,6 @@ function Line({ return ( {p.value} @@ -162,7 +162,6 @@ function Line({ return ( {p.value} @@ -172,10 +171,7 @@ function Line({ } // Context (should not occur with charParts), fall back to full line return ( - + {p.value} ); @@ -183,6 +179,7 @@ function Line({ ) : ( @@ -370,31 +367,101 @@ export function AdvancedDiffRenderer( ? `Wrote changes to ${relative}` : `Updated ${relative}`; - // Best-effort width clamp for rendering inside approval panel (border + padding + indent ~ 8 cols) - const panelInnerWidth = Math.max(20, columns - 8); // keep a reasonable minimum + // If no changes (empty diff), show a message with filepath + if (rows.length === 0) { + const noChangesGutter = 4; + return ( + + {showHeader ? ( + + + + {" "} + ⎿ + + + + {header} + + + ) : null} + + + {" "} + + + + No changes to {relative} (file content + identical) + + + + + ); + } + + // Gutter width for " ⎿" prefix (4 chars: 2 spaces + ⎿ + space) + const toolResultGutter = 4; return ( - + {showHeader ? ( <> - {header} - {`Showing ~${ADV_DIFF_CONTEXT_LINES} context line${ADV_DIFF_CONTEXT_LINES === 1 ? "" : "s"}`} + + + + {" "} + ⎿ + + + + {header} + + + + + {" "} + + + {`Showing ~${ADV_DIFF_CONTEXT_LINES} context line${ADV_DIFF_CONTEXT_LINES === 1 ? "" : "s"}`} + + ) : null} - {rows.map((r, idx) => ( - - ))} + {rows.map((r, idx) => + showHeader ? ( + + + {" "} + + + + ) : ( + + ), + )} ); } diff --git a/src/cli/components/ApprovalDialogRich.tsx b/src/cli/components/ApprovalDialogRich.tsx index ff20ce3..b7225e2 100644 --- a/src/cli/components/ApprovalDialogRich.tsx +++ b/src/cli/components/ApprovalDialogRich.tsx @@ -3,7 +3,11 @@ import { Box, Text, useInput } from "ink"; import type React from "react"; import { memo, useEffect, useMemo, useState } from "react"; import type { ApprovalContext } from "../../permissions/analyzer"; -import { type AdvancedDiffSuccess, computeAdvancedDiff } from "../helpers/diff"; +import { + type AdvancedDiffSuccess, + computeAdvancedDiff, + parsePatchToAdvancedDiff, +} from "../helpers/diff"; import { parsePatchOperations } from "../helpers/formatArgsDisplay"; import { resolvePlaceholders } from "../helpers/pasteRegistry"; import type { ApprovalRequest } from "../helpers/stream"; @@ -17,8 +21,11 @@ type Props = { progress?: { current: number; total: number }; totalTools?: number; isExecuting?: boolean; - onApproveAll: () => void; - onApproveAlways: (scope?: "project" | "session") => void; + onApproveAll: (diffs?: Map) => void; + onApproveAlways: ( + scope?: "project" | "session", + diffs?: Map, + ) => void; onDenyAll: (reason: string) => void; onCancel?: () => void; // Cancel all approvals without sending to server }; @@ -28,6 +35,8 @@ type DynamicPreviewProps = { toolArgs: string; parsedArgs: Record | null; precomputedDiff: AdvancedDiffSuccess | null; + allDiffs: Map; + toolCallId: string | undefined; }; // Options renderer - memoized to prevent unnecessary re-renders @@ -70,6 +79,8 @@ const DynamicPreview: React.FC = ({ toolArgs, parsedArgs, precomputedDiff, + allDiffs, + toolCallId, }) => { const t = toolName.toLowerCase(); @@ -189,39 +200,56 @@ const DynamicPreview: React.FC = ({ if (typeof inputVal === "string") { const operations = parsePatchOperations(inputVal); if (operations.length > 0) { + const { relative } = require("node:path"); + const cwd = process.cwd(); return ( - {operations.map((op) => { + {operations.map((op, idx) => { + const relPath = relative(cwd, op.path); + const displayPath = relPath.startsWith("..") ? op.path : relPath; + // Look up precomputed diff from allDiffs using toolCallId:path key + const diffKey = toolCallId + ? `${toolCallId}:${op.path}` + : undefined; + const opDiff = diffKey ? allDiffs.get(diffKey) : undefined; if (op.kind === "add") { return ( - + + {idx > 0 && } + {displayPath} + + ); } if (op.kind === "update") { return ( - + + {idx > 0 && } + {displayPath} + + ); } if (op.kind === "delete") { return ( - - Delete file: {op.path} - + + {idx > 0 && } + {displayPath} + File will be deleted + ); } return null; @@ -517,40 +545,6 @@ export const ApprovalDialog = memo(function ApprovalDialog({ setDenyReason(""); }, [progress?.current]); - // Build options based on approval context - const options = useMemo(() => { - const approvalLabel = - progress && progress.total > 1 - ? "Yes, approve this tool" - : "Yes, just this once"; - const opts = [{ label: approvalLabel, action: onApproveAll }]; - - // Add context-aware approval option if available (only for single approvals) - if (approvalContext?.allowPersistence) { - opts.push({ - label: approvalContext.approveAlwaysText, - action: () => - onApproveAlways( - approvalContext.defaultScope === "user" - ? "session" - : approvalContext.defaultScope, - ), - }); - } - - // Add deny option - const denyLabel = - progress && progress.total > 1 - ? "No, deny this tool (esc)" - : "No, and tell Letta what to do differently (esc)"; - opts.push({ - label: denyLabel, - action: () => {}, // Handled separately via setIsEnteringReason - }); - - return opts; - }, [progress, approvalContext, onApproveAll, onApproveAlways]); - useInput((_input, key) => { if (isExecuting) return; @@ -669,6 +663,79 @@ export const ApprovalDialog = memo(function ApprovalDialog({ return null; }, [approvalRequest, parsedArgs]); + // Build map of all diffs (for Edit/Write AND Patch operations) + const allDiffs = useMemo((): Map => { + const diffs = new Map(); + const toolCallId = approvalRequest?.toolCallId; + if (!toolCallId) return diffs; + + // For Edit/Write/MultiEdit - single file diff + if (precomputedDiff) { + diffs.set(toolCallId, precomputedDiff); + return diffs; + } + + // For Patch tools - parse hunks directly (patches ARE diffs, no need to recompute) + const t = approvalRequest.toolName.toLowerCase(); + if ((t === "apply_patch" || t === "applypatch") && parsedArgs?.input) { + const operations = parsePatchOperations(parsedArgs.input as string); + for (const op of operations) { + const key = `${toolCallId}:${op.path}`; + + if (op.kind === "add" || op.kind === "update") { + // Parse patch hunks directly instead of trying to find oldString in file + const result = parsePatchToAdvancedDiff(op.patchLines, op.path); + if (result) { + diffs.set(key, result); + } + } + // Delete operations don't need diffs + } + } + + return diffs; + }, [approvalRequest, parsedArgs, precomputedDiff]); + + // Build options based on approval context + const options = useMemo(() => { + const approvalLabel = + progress && progress.total > 1 + ? "Yes, approve this tool" + : "Yes, just this once"; + const opts = [ + { + label: approvalLabel, + action: () => onApproveAll(allDiffs.size > 0 ? allDiffs : undefined), + }, + ]; + + // Add context-aware approval option if available (only for single approvals) + if (approvalContext?.allowPersistence) { + opts.push({ + label: approvalContext.approveAlwaysText, + action: () => + onApproveAlways( + approvalContext.defaultScope === "user" + ? "session" + : approvalContext.defaultScope, + allDiffs.size > 0 ? allDiffs : undefined, + ), + }); + } + + // Add deny option + const denyLabel = + progress && progress.total > 1 + ? "No, deny this tool (esc)" + : "No, and tell Letta Code what to do differently (esc)"; + opts.push({ + label: denyLabel, + action: () => {}, // Handled separately via setIsEnteringReason + }); + + return opts; + }, [progress, approvalContext, onApproveAll, onApproveAlways, allDiffs]); + // Get the human-readable header label const headerLabel = useMemo(() => { if (!approvalRequest) return ""; @@ -677,18 +744,101 @@ export const ApprovalDialog = memo(function ApprovalDialog({ if (t === "apply_patch" || t === "applypatch") { if (parsedArgs?.input && typeof parsedArgs.input === "string") { const operations = parsePatchOperations(parsedArgs.input); - const firstOp = operations[0]; - if (firstOp) { - if (firstOp.kind === "add") return "Write File"; - if (firstOp.kind === "update") return "Edit File"; - if (firstOp.kind === "delete") return "Delete File"; + if (operations.length > 0) { + const isMulti = operations.length > 1; + const firstOp = operations[0]; + if (firstOp?.kind === "add") + return isMulti ? "Write Files" : "Write File"; + if (firstOp?.kind === "update") + return isMulti ? "Edit Files" : "Edit File"; + if (firstOp?.kind === "delete") + return isMulti ? "Delete Files" : "Delete File"; } } return "Apply Patch"; // Fallback } + // For write tools, check if file exists to show "Overwrite File" vs "Write File" + if ( + t === "write" || + t === "write_file" || + t === "writefile" || + t === "write_file_gemini" || + t === "writefilegemini" + ) { + const filePath = parsedArgs?.file_path as string | undefined; + if (filePath) { + try { + const { existsSync } = require("node:fs"); + if (existsSync(filePath)) { + return "Overwrite File"; + } + } catch { + // Ignore errors, fall through to default + } + } + return "Write File"; + } return getHeaderLabel(approvalRequest.toolName); }, [approvalRequest, parsedArgs]); + // Compute the question text (customized for write tools to show filepath) + const questionText = useMemo((): { text: string; boldPath?: string } => { + if (!approvalRequest || !parsedArgs) { + return { text: "Do you want to proceed?" }; + } + const t = approvalRequest.toolName.toLowerCase(); + // For write tools, show "Write to {path}?" or "Overwrite {path}?" + if ( + t === "write" || + t === "write_file" || + t === "writefile" || + t === "write_file_gemini" || + t === "writefilegemini" + ) { + const filePath = parsedArgs.file_path as string | undefined; + if (filePath) { + const { existsSync } = require("node:fs"); + const { relative } = require("node:path"); + const cwd = process.cwd(); + const relPath = relative(cwd, filePath); + const displayPath = relPath.startsWith("..") ? filePath : relPath; + try { + if (existsSync(filePath)) { + return { text: "Overwrite", boldPath: `${displayPath}?` }; + } + } catch { + // Ignore errors + } + return { text: "Write to", boldPath: `${displayPath}?` }; + } + } + // For patch tools, show file path(s) being modified + if ((t === "apply_patch" || t === "applypatch") && parsedArgs.input) { + const operations = parsePatchOperations(parsedArgs.input as string); + if (operations.length > 0) { + const { relative } = require("node:path"); + const cwd = process.cwd(); + const paths = operations.map((op) => { + const relPath = relative(cwd, op.path); + return relPath.startsWith("..") ? op.path : relPath; + }); + if (paths.length === 1) { + const op = operations[0]; + if (op?.kind === "add") { + return { text: "Write to", boldPath: `${paths[0]}?` }; + } else if (op?.kind === "update") { + return { text: "Update", boldPath: `${paths[0]}?` }; + } else if (op?.kind === "delete") { + return { text: "Delete", boldPath: `${paths[0]}?` }; + } + } else { + return { text: "Apply patch to", boldPath: `${paths.length} files?` }; + } + } + } + return { text: "Do you want to proceed?" }; + }, [approvalRequest, parsedArgs]); + // Guard: should never happen as parent checks length, but satisfies TypeScript if (!approvalRequest) { return null; @@ -748,11 +898,21 @@ export const ApprovalDialog = memo(function ApprovalDialog({ toolArgs={approvalRequest.toolArgs} parsedArgs={parsedArgs} precomputedDiff={precomputedDiff} + allDiffs={allDiffs} + toolCallId={approvalRequest.toolCallId} /> {/* Prompt */} - Do you want to proceed? + + {questionText.text} + {questionText.boldPath ? ( + <> + {" "} + {questionText.boldPath} + + ) : null} + {/* Options selector (single line per option) */} diff --git a/src/cli/components/ToolCallMessageRich.tsx b/src/cli/components/ToolCallMessageRich.tsx index 9c22216..b26c556 100644 --- a/src/cli/components/ToolCallMessageRich.tsx +++ b/src/cli/components/ToolCallMessageRich.tsx @@ -2,6 +2,7 @@ import { Box, Text } from "ink"; import { memo } from "react"; import { INTERRUPTED_BY_USER } from "../../constants"; import { clipToolReturn } from "../../tools/manager.js"; +import type { AdvancedDiffSuccess } from "../helpers/diff"; import { formatArgsDisplay, parsePatchInput, @@ -18,6 +19,7 @@ import { isTodoTool, } from "../helpers/toolNameMapping.js"; import { useTerminalWidth } from "../hooks/useTerminalWidth"; +import { AdvancedDiffRenderer } from "./AdvancedDiffRenderer"; import { BlinkDot } from "./BlinkDot.js"; import { colors } from "./colors.js"; import { @@ -51,417 +53,509 @@ type ToolCallLine = { * - Blinking dots for pending/running states * - Result shown with ⎿ prefix underneath */ -export const ToolCallMessage = memo(({ line }: { line: ToolCallLine }) => { - const columns = useTerminalWidth(); +export const ToolCallMessage = memo( + ({ + line, + precomputedDiffs, + }: { + line: ToolCallLine; + precomputedDiffs?: Map; + }) => { + const columns = useTerminalWidth(); - // Parse and format the tool call - const rawName = line.name ?? "?"; - const argsText = line.argsText ?? "..."; + // Parse and format the tool call + const rawName = line.name ?? "?"; + const argsText = line.argsText ?? "..."; - // Task tool - handled by SubagentGroupDisplay, don't render here - // Exception: Cancelled/rejected Task tools should be rendered inline - // since they won't appear in SubagentGroupDisplay - if (isTaskTool(rawName)) { - const isCancelledOrRejected = - line.phase === "finished" && line.resultOk === false; - if (!isCancelledOrRejected) { - return null; - } - } - - // Apply tool name remapping - let displayName = getDisplayToolName(rawName); - - // For Patch tools, override display name based on patch content - // (Add → Write, Update → Update, Delete → Delete) - if (isPatchTool(rawName)) { - try { - const parsedArgs = JSON.parse(argsText); - if (parsedArgs.input) { - const patchInfo = parsePatchInput(parsedArgs.input); - if (patchInfo) { - if (patchInfo.kind === "add") displayName = "Write"; - else if (patchInfo.kind === "update") displayName = "Update"; - else if (patchInfo.kind === "delete") displayName = "Delete"; - } + // Task tool - handled by SubagentGroupDisplay, don't render here + // Exception: Cancelled/rejected Task tools should be rendered inline + // since they won't appear in SubagentGroupDisplay + if (isTaskTool(rawName)) { + const isCancelledOrRejected = + line.phase === "finished" && line.resultOk === false; + if (!isCancelledOrRejected) { + return null; } - } catch { - // Keep default "Patch" name if parsing fails - } - } - - // Format arguments for display using the old formatting logic - // Pass rawName to enable special formatting for file tools - const formatted = formatArgsDisplay(argsText, rawName); - const args = `(${formatted.display})`; - - const rightWidth = Math.max(0, columns - 2); // gutter is 2 cols - - // If name exceeds available width, fall back to simple wrapped rendering - const fallback = displayName.length >= rightWidth; - - // Determine dot state based on phase - const getDotElement = () => { - switch (line.phase) { - case "streaming": - return ●; - case "ready": - return ; - case "running": - return ; - case "finished": - if (line.resultOk === false) { - return ●; - } - return ●; - default: - return ●; - } - }; - - // Format result for display - const getResultElement = () => { - if (!line.resultText) return null; - - const prefix = ` ⎿ `; // Match old format: 2 spaces, glyph, 2 spaces - const prefixWidth = 5; // Total width of prefix - const contentWidth = Math.max(0, columns - prefixWidth); - - // Special cases from old ToolReturnBlock (check before truncation) - if (line.resultText === "Running...") { - return ( - - - {prefix} - - - Running... - - - ); } - if (line.resultText === INTERRUPTED_BY_USER) { - return ( - - - {prefix} - - - {INTERRUPTED_BY_USER} - - - ); - } + // Apply tool name remapping + let displayName = getDisplayToolName(rawName); - // Truncate the result text for display (UI only, API gets full response) - // Strip trailing newlines to avoid extra visual spacing (e.g., from bash echo) - const displayResultText = clipToolReturn(line.resultText).replace( - /\n+$/, - "", - ); - - // Helper to check if a value is a record - const isRecord = (v: unknown): v is Record => - typeof v === "object" && v !== null; - - // Check if this is a todo_write tool with successful result - if ( - isTodoTool(rawName, displayName) && - line.resultOk !== false && - line.argsText - ) { + // For Patch tools, override display name based on patch content + // (Add → Write, Update → Update, Delete → Delete) + if (isPatchTool(rawName)) { try { - const parsedArgs = JSON.parse(line.argsText); - if (parsedArgs.todos && Array.isArray(parsedArgs.todos)) { - // Convert todos to safe format for TodoRenderer - // Note: Anthropic/Codex use "content", Gemini uses "description" - const safeTodos = parsedArgs.todos.map((t: unknown, i: number) => { - const rec = isRecord(t) ? t : {}; - const status: "pending" | "in_progress" | "completed" = - rec.status === "completed" - ? "completed" - : rec.status === "in_progress" - ? "in_progress" - : "pending"; - const id = typeof rec.id === "string" ? rec.id : String(i); - // Handle both "content" (Anthropic/Codex) and "description" (Gemini) fields - const content = - typeof rec.content === "string" - ? rec.content - : typeof rec.description === "string" - ? rec.description - : JSON.stringify(t); - const priority: "high" | "medium" | "low" | undefined = - rec.priority === "high" - ? "high" - : rec.priority === "medium" - ? "medium" - : rec.priority === "low" - ? "low" - : undefined; - return { content, status, id, priority }; - }); - - // Return TodoRenderer directly - it has its own prefix - return ; - } - } catch { - // If parsing fails, fall through to regular handling - } - } - - // Check if this is an update_plan tool with successful result - if ( - isPlanTool(rawName, displayName) && - line.resultOk !== false && - line.argsText - ) { - try { - const parsedArgs = JSON.parse(line.argsText); - if (parsedArgs.plan && Array.isArray(parsedArgs.plan)) { - // Convert plan items to safe format for PlanRenderer - const safePlan = parsedArgs.plan.map((item: unknown) => { - const rec = isRecord(item) ? item : {}; - const status: "pending" | "in_progress" | "completed" = - rec.status === "completed" - ? "completed" - : rec.status === "in_progress" - ? "in_progress" - : "pending"; - const step = - typeof rec.step === "string" ? rec.step : JSON.stringify(item); - return { step, status }; - }); - - const explanation = - typeof parsedArgs.explanation === "string" - ? parsedArgs.explanation - : undefined; - - // Return PlanRenderer directly - it has its own prefix - return ; - } - } catch { - // If parsing fails, fall through to regular handling - } - } - - // Check if this is a memory tool - show diff instead of raw result - if (isMemoryTool(rawName) && line.resultOk !== false && line.argsText) { - const memoryDiff = ( - - ); - if (memoryDiff) { - return memoryDiff; - } - // If MemoryDiffRenderer returns null, fall through to regular handling - } - - // Check if this is a file edit tool - show diff instead of success message - if (isFileEditTool(rawName) && line.resultOk !== false && line.argsText) { - try { - const parsedArgs = JSON.parse(line.argsText); - const filePath = parsedArgs.file_path || ""; - - // Multi-edit: has edits array - if (parsedArgs.edits && Array.isArray(parsedArgs.edits)) { - const edits = parsedArgs.edits.map( - (e: { old_string?: string; new_string?: string }) => ({ - old_string: e.old_string || "", - new_string: e.new_string || "", - }), - ); - return ( - - ); - } - - // Single edit: has old_string/new_string - if (parsedArgs.old_string !== undefined) { - return ( - - ); - } - } catch { - // If parsing fails, fall through to regular handling - } - } - - // Check if this is a file write tool - show written content - if (isFileWriteTool(rawName) && line.resultOk !== false && line.argsText) { - try { - const parsedArgs = JSON.parse(line.argsText); - const filePath = parsedArgs.file_path || ""; - const content = parsedArgs.content || ""; - - if (filePath && content) { - return ; - } - } catch { - // If parsing fails, fall through to regular handling - } - } - - // Check if this is a patch tool - show diff/content based on operation type - if (isPatchTool(rawName) && line.resultOk !== false && line.argsText) { - try { - const parsedArgs = JSON.parse(line.argsText); + const parsedArgs = JSON.parse(argsText); if (parsedArgs.input) { - const operations = parsePatchOperations(parsedArgs.input); - - if (operations.length > 0) { - return ( - - {operations.map((op) => { - if (op.kind === "add") { - return ( - - ); - } - if (op.kind === "update") { - return ( - - ); - } - if (op.kind === "delete") { - const gutterWidth = 4; - return ( - - - - {" "} - ⎿ - - - - - Deleted {op.path} - - - - ); - } - return null; - })} - - ); + const patchInfo = parsePatchInput(parsedArgs.input); + if (patchInfo) { + if (patchInfo.kind === "add") displayName = "Write"; + else if (patchInfo.kind === "update") displayName = "Update"; + else if (patchInfo.kind === "delete") displayName = "Delete"; } } } catch { - // If parsing fails, fall through to regular handling + // Keep default "Patch" name if parsing fails } } - // Regular result handling - const isError = line.resultOk === false; + // Format arguments for display using the old formatting logic + // Pass rawName to enable special formatting for file tools + const formatted = formatArgsDisplay(argsText, rawName); + const args = `(${formatted.display})`; - // Try to parse JSON for cleaner error display - let displayText = displayResultText; - try { - const parsed = JSON.parse(displayResultText); - if (parsed.error && typeof parsed.error === "string") { - displayText = parsed.error; + const rightWidth = Math.max(0, columns - 2); // gutter is 2 cols + + // If name exceeds available width, fall back to simple wrapped rendering + const fallback = displayName.length >= rightWidth; + + // Determine dot state based on phase + const getDotElement = () => { + switch (line.phase) { + case "streaming": + return ●; + case "ready": + return ; + case "running": + return ; + case "finished": + if (line.resultOk === false) { + return ●; + } + return ●; + default: + return ●; } - } catch { - // Not JSON, use raw text - } + }; - // Format tool denial errors more user-friendly - if (isError && displayText.includes("request to call tool denied")) { - // Use [\s\S]+ to match multiline reasons - const match = displayText.match(/User reason: ([\s\S]+)$/); - const reason = match?.[1]?.trim() || "(empty)"; - displayText = `User rejected the tool call with reason: ${reason}`; - } + // Format result for display + const getResultElement = () => { + if (!line.resultText) return null; + + const prefix = ` ⎿ `; // Match old format: 2 spaces, glyph, 2 spaces + const prefixWidth = 5; // Total width of prefix + const contentWidth = Math.max(0, columns - prefixWidth); + + // Special cases from old ToolReturnBlock (check before truncation) + if (line.resultText === "Running...") { + return ( + + + {prefix} + + + Running... + + + ); + } + + if (line.resultText === INTERRUPTED_BY_USER) { + return ( + + + {prefix} + + + {INTERRUPTED_BY_USER} + + + ); + } + + // Truncate the result text for display (UI only, API gets full response) + // Strip trailing newlines to avoid extra visual spacing (e.g., from bash echo) + const displayResultText = clipToolReturn(line.resultText).replace( + /\n+$/, + "", + ); + + // Helper to check if a value is a record + const isRecord = (v: unknown): v is Record => + typeof v === "object" && v !== null; + + // Check if this is a todo_write tool with successful result + if ( + isTodoTool(rawName, displayName) && + line.resultOk !== false && + line.argsText + ) { + try { + const parsedArgs = JSON.parse(line.argsText); + if (parsedArgs.todos && Array.isArray(parsedArgs.todos)) { + // Convert todos to safe format for TodoRenderer + // Note: Anthropic/Codex use "content", Gemini uses "description" + const safeTodos = parsedArgs.todos.map((t: unknown, i: number) => { + const rec = isRecord(t) ? t : {}; + const status: "pending" | "in_progress" | "completed" = + rec.status === "completed" + ? "completed" + : rec.status === "in_progress" + ? "in_progress" + : "pending"; + const id = typeof rec.id === "string" ? rec.id : String(i); + // Handle both "content" (Anthropic/Codex) and "description" (Gemini) fields + const content = + typeof rec.content === "string" + ? rec.content + : typeof rec.description === "string" + ? rec.description + : JSON.stringify(t); + const priority: "high" | "medium" | "low" | undefined = + rec.priority === "high" + ? "high" + : rec.priority === "medium" + ? "medium" + : rec.priority === "low" + ? "low" + : undefined; + return { content, status, id, priority }; + }); + + // Return TodoRenderer directly - it has its own prefix + return ; + } + } catch { + // If parsing fails, fall through to regular handling + } + } + + // Check if this is an update_plan tool with successful result + if ( + isPlanTool(rawName, displayName) && + line.resultOk !== false && + line.argsText + ) { + try { + const parsedArgs = JSON.parse(line.argsText); + if (parsedArgs.plan && Array.isArray(parsedArgs.plan)) { + // Convert plan items to safe format for PlanRenderer + const safePlan = parsedArgs.plan.map((item: unknown) => { + const rec = isRecord(item) ? item : {}; + const status: "pending" | "in_progress" | "completed" = + rec.status === "completed" + ? "completed" + : rec.status === "in_progress" + ? "in_progress" + : "pending"; + const step = + typeof rec.step === "string" ? rec.step : JSON.stringify(item); + return { step, status }; + }); + + const explanation = + typeof parsedArgs.explanation === "string" + ? parsedArgs.explanation + : undefined; + + // Return PlanRenderer directly - it has its own prefix + return ; + } + } catch { + // If parsing fails, fall through to regular handling + } + } + + // Check if this is a memory tool - show diff instead of raw result + if (isMemoryTool(rawName) && line.resultOk !== false && line.argsText) { + const memoryDiff = ( + + ); + if (memoryDiff) { + return memoryDiff; + } + // If MemoryDiffRenderer returns null, fall through to regular handling + } + + // Check if this is a file edit tool - show diff instead of success message + if (isFileEditTool(rawName) && line.resultOk !== false && line.argsText) { + const diff = line.toolCallId + ? precomputedDiffs?.get(line.toolCallId) + : undefined; + + try { + const parsedArgs = JSON.parse(line.argsText); + const filePath = parsedArgs.file_path || ""; + + // Use AdvancedDiffRenderer if we have a precomputed diff + if (diff) { + // Multi-edit: has edits array + if (parsedArgs.edits && Array.isArray(parsedArgs.edits)) { + const edits = parsedArgs.edits.map( + (e: { + old_string?: string; + new_string?: string; + replace_all?: boolean; + }) => ({ + old_string: e.old_string || "", + new_string: e.new_string || "", + replace_all: e.replace_all, + }), + ); + return ( + + ); + } + // Single edit + return ( + + ); + } + + // Fallback to simple renderers when no precomputed diff + // Multi-edit: has edits array + if (parsedArgs.edits && Array.isArray(parsedArgs.edits)) { + const edits = parsedArgs.edits.map( + (e: { old_string?: string; new_string?: string }) => ({ + old_string: e.old_string || "", + new_string: e.new_string || "", + }), + ); + return ( + + ); + } + + // Single edit: has old_string/new_string + if (parsedArgs.old_string !== undefined) { + return ( + + ); + } + } catch { + // If parsing fails, fall through to regular handling + } + } + + // Check if this is a file write tool - show written content + if ( + isFileWriteTool(rawName) && + line.resultOk !== false && + line.argsText + ) { + const diff = line.toolCallId + ? precomputedDiffs?.get(line.toolCallId) + : undefined; + + try { + const parsedArgs = JSON.parse(line.argsText); + const filePath = parsedArgs.file_path || ""; + const content = parsedArgs.content || ""; + + if (filePath && content) { + if (diff) { + return ( + + ); + } + return ; + } + } catch { + // If parsing fails, fall through to regular handling + } + } + + // Check if this is a patch tool - show diff/content based on operation type + if (isPatchTool(rawName) && line.resultOk !== false && line.argsText) { + try { + const parsedArgs = JSON.parse(line.argsText); + if (parsedArgs.input) { + const operations = parsePatchOperations(parsedArgs.input); + + if (operations.length > 0) { + return ( + + {operations.map((op) => { + // Look up precomputed diff using compound key + const key = `${line.toolCallId}:${op.path}`; + const diff = precomputedDiffs?.get(key); + + if (op.kind === "add") { + return diff ? ( + + ) : ( + + ); + } + if (op.kind === "update") { + return diff ? ( + + ) : ( + + ); + } + if (op.kind === "delete") { + const gutterWidth = 4; + return ( + + + + {" "} + ⎿ + + + + + Deleted {op.path} + + + + ); + } + return null; + })} + + ); + } + } + } catch { + // If parsing fails, fall through to regular handling + } + } + + // Regular result handling + const isError = line.resultOk === false; + + // Try to parse JSON for cleaner error display + let displayText = displayResultText; + try { + const parsed = JSON.parse(displayResultText); + if (parsed.error && typeof parsed.error === "string") { + displayText = parsed.error; + } + } catch { + // Not JSON, use raw text + } + + // Format tool denial errors more user-friendly + if (isError && displayText.includes("request to call tool denied")) { + // Use [\s\S]+ to match multiline reasons + const match = displayText.match(/User reason: ([\s\S]+)$/); + const reason = match?.[1]?.trim() || "(empty)"; + displayText = `User rejected the tool call with reason: ${reason}`; + } + + return ( + + + {prefix} + + + {isError ? ( + {displayText} + ) : ( + + )} + + + ); + }; return ( - - - {prefix} - - - {isError ? ( - {displayText} - ) : ( - - )} + + {/* Tool call with exact wrapping logic from old codebase */} + + + {getDotElement()} + + + + {fallback ? ( + + {isMemoryTool(rawName) ? ( + <> + + {displayName} + + {args} + + ) : ( + <> + {displayName} + {args} + + )} + + ) : ( + + + {displayName} + + {args ? ( + + {args} + + ) : null} + + )} + + + {/* Tool result (if present) */} + {getResultElement()} ); - }; - - return ( - - {/* Tool call with exact wrapping logic from old codebase */} - - - {getDotElement()} - - - - {fallback ? ( - - {isMemoryTool(rawName) ? ( - <> - - {displayName} - - {args} - - ) : ( - <> - {displayName} - {args} - - )} - - ) : ( - - - {displayName} - - {args ? ( - - {args} - - ) : null} - - )} - - - - {/* Tool result (if present) */} - {getResultElement()} - - ); -}); + }, +); ToolCallMessage.displayName = "ToolCallMessage"; diff --git a/src/cli/helpers/diff.ts b/src/cli/helpers/diff.ts index 408ad2f..66e2a5e 100644 --- a/src/cli/helpers/diff.ts +++ b/src/cli/helpers/diff.ts @@ -189,3 +189,73 @@ export function computeAdvancedDiff( return { mode: "advanced", fileName, oldStr, newStr, hunks }; } + +/** + * Parse a patch operation's hunks directly into AdvancedDiffSuccess format. + * This bypasses the "read file -> find oldString" flow since the patch IS the diff. + * Used for ApplyPatch tool previews where multi-hunk patches can't be found as + * contiguous blocks in the file. + */ +export function parsePatchToAdvancedDiff( + patchLines: string[], // Lines for this file operation (after "*** Update File:" or "*** Add File:") + filePath: string, +): AdvancedDiffSuccess | null { + const fileName = basename(filePath); + const hunks: AdvancedHunk[] = []; + + let currentHunk: AdvancedHunk | null = null; + let oldLine = 1; + let newLine = 1; + + for (const line of patchLines) { + if (line.startsWith("@@")) { + // Start new hunk - try to parse line numbers from @@ -old,count +new,count @@ + if (currentHunk && currentHunk.lines.length > 0) { + hunks.push(currentHunk); + } + + // Try standard unified diff format: @@ -10,5 +10,7 @@ + const match = line.match(/@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/); + currentHunk = { + oldStart: match?.[1] ? parseInt(match[1], 10) : oldLine, + newStart: match?.[2] ? parseInt(match[2], 10) : newLine, + lines: [], + }; + continue; + } + + if (!currentHunk) { + // Create implicit first hunk if no @@ header seen yet + currentHunk = { oldStart: 1, newStart: 1, lines: [] }; + } + + // Parse diff line (prefix + content) + if (line.length === 0) { + // Empty line - treat as context + currentHunk.lines.push({ raw: " " }); + oldLine++; + newLine++; + } else { + const prefix = line[0]; + if (prefix === " " || prefix === "-" || prefix === "+") { + currentHunk.lines.push({ raw: line }); + if (prefix === " " || prefix === "-") oldLine++; + if (prefix === " " || prefix === "+") newLine++; + } + } + } + + if (currentHunk && currentHunk.lines.length > 0) { + hunks.push(currentHunk); + } + + if (hunks.length === 0) return null; + + return { + mode: "advanced", + fileName, + oldStr: "", // Not needed for rendering when hunks are provided + newStr: "", // Not needed for rendering when hunks are provided + hunks, + }; +} diff --git a/src/cli/helpers/formatArgsDisplay.ts b/src/cli/helpers/formatArgsDisplay.ts index 3a0945c..d24a589 100644 --- a/src/cli/helpers/formatArgsDisplay.ts +++ b/src/cli/helpers/formatArgsDisplay.ts @@ -61,8 +61,14 @@ export function parsePatchInput( * Patch operation types for result rendering */ export type PatchOperation = - | { kind: "add"; path: string; content: string } - | { kind: "update"; path: string; oldString: string; newString: string } + | { kind: "add"; path: string; content: string; patchLines: string[] } + | { + kind: "update"; + path: string; + oldString: string; + newString: string; + patchLines: string[]; + } | { kind: "delete"; path: string }; /** @@ -96,15 +102,22 @@ export function parsePatchOperations(input: string): PatchOperation[] { const path = line.replace("*** Add File:", "").trim(); i++; const contentLines: string[] = []; + const patchLines: string[] = []; while (i < stopIdx) { const raw = lines[i]; if (raw === undefined || raw.startsWith("*** ")) break; + patchLines.push(raw); // Store raw patch line for direct hunk parsing if (raw.startsWith("+")) { contentLines.push(raw.slice(1)); } i++; } - operations.push({ kind: "add", path, content: contentLines.join("\n") }); + operations.push({ + kind: "add", + path, + content: contentLines.join("\n"), + patchLines, + }); continue; } @@ -121,13 +134,16 @@ export function parsePatchOperations(input: string): PatchOperation[] { // Collect all hunk lines const oldParts: string[] = []; const newParts: string[] = []; + const patchLines: string[] = []; // Store raw lines for direct hunk parsing while (i < stopIdx) { const hLine = lines[i]; if (hLine === undefined || hLine.startsWith("*** ")) break; + patchLines.push(hLine); // Store raw patch line + if (hLine.startsWith("@@")) { - // Skip hunk header + // Hunk header - don't parse for oldParts/newParts, just store in patchLines i++; continue; } @@ -161,6 +177,7 @@ export function parsePatchOperations(input: string): PatchOperation[] { path, oldString: oldParts.join("\n"), newString: newParts.join("\n"), + patchLines, }); continue; }