refactor(cli): throw tool error instead of returning and matching error strings (#8432)

* fix: prevent string error matching throwing errors

- added ToolResultWithStatus

* add throwing of errors in search code tool

* throw errors in cli fetch tool

* throw errors in writeFile tool

* throw error in status tool

* throw errors in listFiles tool

* throw error in readfile tool

* fix no matches for pattern searchCode

* update tests

* use continueerror writeFile

---------

Co-authored-by: Nate <sestinj@gmail.com>

Author: uinstinct

extensions/cli/src/stream/handleToolCalls.ts

@@ -123,26 +123,18 @@ export async function handleToolCalls(
     return true; // Signal early return needed
   }
 
-  // Convert tool results and add them to the chat history with per-result status
+  // Convert tool results and add them to the chat history with status from execution
   toolResults.forEach((toolResult) => {
     const resultContent =
       typeof toolResult.content === "string" ? toolResult.content : "";
 
-    // Derive per-result status instead of applying batch-wide hasRejection
-    let status: ToolStatus = "done";
-    const lower = resultContent.toLowerCase();
-    if (
-      lower.includes("permission denied by user") ||
-      lower.includes("cancelled due to previous tool rejection") ||
-      lower.includes("canceled due to previous tool rejection")
-    ) {
-      status = "canceled";
-    } else if (
-      lower.startsWith("error executing tool") ||
-      lower.startsWith("error:")
-    ) {
-      status = "errored" as ToolStatus;
-    }
+    // Use the status from the tool execution result instead of text matching
+    const status = toolResult.status;
+
+    logger.debug("Tool result status", {
+      status,
+      toolCallId: toolResult.tool_call_id,
+    });
 
     if (useService) {
       chatHistorySvc.addToolResult(

extensions/cli/src/stream/streamChatResponse.helpers.ts

@@ -1,5 +1,6 @@
 // Helper functions extracted from streamChatResponse.ts to reduce file size
 
+import type { ToolStatus } from "core/index.js";
 import { ContinueError, ContinueErrorReason } from "core/util/errors.js";
 import { ChatCompletionToolMessageParam } from "openai/resources/chat/completions.mjs";
 
@@ -27,6 +28,10 @@ import { logger } from "../util/logger.js";
 
 import { StreamCallbacks } from "./streamChatResponse.types.js";
 
+export interface ToolResultWithStatus extends ChatCompletionToolMessageParam {
+  status: ToolStatus;
+}
+
 // Helper function to handle permission denied
 export function handlePermissionDenied(
   toolCall: PreprocessedToolCall,
@@ -389,15 +394,15 @@ export async function preprocessStreamedToolCalls(
  * Executes preprocessed tool calls, handling permissions and results
  * @param preprocessedCalls - The preprocessed tool calls ready for execution
  * @param callbacks - Optional callbacks for notifying of events
- * @returns - Chat history entries with tool results
+ * @returns - Chat history entries with tool results and status information
  */
 export async function executeStreamedToolCalls(
   preprocessedCalls: PreprocessedToolCall[],
   callbacks?: StreamCallbacks,
   isHeadless?: boolean,
 ): Promise<{
   hasRejection: boolean;
-  chatHistoryEntries: ChatCompletionToolMessageParam[];
+  chatHistoryEntries: ToolResultWithStatus[];
 }> {
   // Strategy: queue permissions (preserve order), then run approved tools in parallel.
   // If any permission is rejected, cancel the remaining tools in this batch.
@@ -408,7 +413,7 @@ export async function executeStreamedToolCalls(
     call,
   }));
 
-  const entriesByIndex = new Map<number, ChatCompletionToolMessageParam>();
+  const entriesByIndex = new Map<number, ToolResultWithStatus>();
   const execPromises: Promise<void>[] = [];
 
   let hasRejection = false;
@@ -439,17 +444,18 @@ export async function executeStreamedToolCalls(
       );
 
       if (!permissionResult.approved) {
-        // Permission denied: record and mark rejection
+        // Permission denied: create entry with canceled status
         const denialReason = permissionResult.denialReason || "user";
         const deniedMessage =
           denialReason === "policy"
             ? `Command blocked by security policy`
             : `Permission denied by user`;
 
-        const deniedEntry: ChatCompletionToolMessageParam = {
+        const deniedEntry: ToolResultWithStatus = {
           role: "tool",
           tool_call_id: call.id,
           content: deniedMessage,
+          status: "canceled",
         };
         entriesByIndex.set(index, deniedEntry);
         callbacks?.onToolResult?.(
@@ -484,10 +490,11 @@ export async function executeStreamedToolCalls(
               arguments: call.arguments,
             });
             const toolResult = await executeToolCall(call);
-            const entry: ChatCompletionToolMessageParam = {
+            const entry: ToolResultWithStatus = {
               role: "tool",
               tool_call_id: call.id,
               content: toolResult,
+              status: "done",
             };
             entriesByIndex.set(index, entry);
             callbacks?.onToolResult?.(toolResult, call.name, "done");
@@ -511,6 +518,7 @@ export async function executeStreamedToolCalls(
               role: "tool",
               tool_call_id: call.id,
               content: errorMessage,
+              status: "errored",
             });
             callbacks?.onToolError?.(errorMessage, call.name);
             // Immediate service update for UI feedback
@@ -536,6 +544,7 @@ export async function executeStreamedToolCalls(
         role: "tool",
         tool_call_id: call.id,
         content: errorMessage,
+        status: "errored",
       });
       callbacks?.onToolError?.(errorMessage, call.name);
       // Treat permission errors like execution errors but do not stop the batch
@@ -552,9 +561,9 @@ export async function executeStreamedToolCalls(
   await Promise.all(execPromises);
 
   // Assemble final entries in original order
-  const chatHistoryEntries: ChatCompletionToolMessageParam[] = preprocessedCalls
+  const chatHistoryEntries: ToolResultWithStatus[] = preprocessedCalls
     .map((_, index) => entriesByIndex.get(index))
-    .filter((e): e is ChatCompletionToolMessageParam => !!e);
+    .filter((e): e is ToolResultWithStatus => !!e);
 
   return {
     hasRejection,

extensions/cli/src/tools/fetch.test.ts

@@ -96,23 +96,21 @@ describe("fetchTool", () => {
     expect(result).toBe("Content from page 1\n\nContent from page 2");
   });
 
-  it("should return error message when no content items returned", async () => {
+  it("should throw error when no content items returned", async () => {
     mockFetchUrlContentImpl.mockResolvedValue([]);
 
-    const result = await fetchTool.run({ url: "https://example.com" });
-
-    expect(result).toBe(
-      "Error: Could not fetch content from https://example.com",
+    await expect(fetchTool.run({ url: "https://example.com" })).rejects.toThrow(
+      "Could not fetch content from https://example.com",
     );
   });
 
-  it("should handle errors from core implementation", async () => {
+  it("should throw errors from core implementation", async () => {
     const error = new Error("Network error");
     mockFetchUrlContentImpl.mockRejectedValue(error);
 
-    const result = await fetchTool.run({ url: "https://example.com" });
-
-    expect(result).toBe("Error: Network error");
+    await expect(fetchTool.run({ url: "https://example.com" })).rejects.toThrow(
+      "Error: Network error",
+    );
   });
 
   it("should call fetchUrlContentImpl with correct arguments", async () => {

extensions/cli/src/tools/fetch.ts

@@ -1,5 +1,6 @@
 import type { ContextItem } from "core/index.js";
 import { fetchUrlContentImpl } from "core/tools/implementations/fetchUrlContent.js";
+import { ContinueError, ContinueErrorReason } from "core/util/errors.js";
 
 import { Tool } from "./types.js";
 
@@ -46,7 +47,10 @@ export const fetchTool: Tool = {
       console.error = originalConsoleError;
 
       if (contextItems.length === 0) {
-        return `Error: Could not fetch content from ${url}`;
+        throw new ContinueError(
+          ContinueErrorReason.Unspecified,
+          `Could not fetch content from ${url}`,
+        );
       }
 
       // Format the results for CLI display
@@ -59,7 +63,12 @@ export const fetchTool: Tool = {
         })
         .join("\n\n");
     } catch (error) {
-      return `Error: ${error instanceof Error ? error.message : String(error)}`;
+      if (error instanceof ContinueError) {
+        throw error;
+      }
+      throw new Error(
+        `Error: ${error instanceof Error ? error.message : String(error)}`,
+      );
     }
   },
 };

extensions/cli/src/tools/index.tsx

@@ -228,7 +228,7 @@ export async function executeToolCall(
       errorReason,
     });
 
-    return `Error executing tool "${toolCall.name}": ${errorMessage}`;
+    throw error;
   }
 }
 

extensions/cli/src/tools/listFiles.ts

@@ -61,9 +61,11 @@ export const listFilesTool: Tool = {
 
       return `Files in ${args.dirpath}:\n${fileDetails.join("\n")}`;
     } catch (error) {
-      return `Error listing files: ${
-        error instanceof Error ? error.message : String(error)
-      }`;
+      throw new Error(
+        `Error listing files: ${
+          error instanceof Error ? error.message : String(error)
+        }`,
+      );
     }
   },
 };

extensions/cli/src/tools/readFile.ts

@@ -1,6 +1,7 @@
 import * as fs from "fs";
 
 import { throwIfFileIsSecurityConcern } from "core/indexing/ignore.js";
+import { ContinueError, ContinueErrorReason } from "core/util/errors.js";
 
 import { formatToolArgument } from "./formatters.js";
 import { Tool } from "./types.js";
@@ -51,7 +52,10 @@ export const readFileTool: Tool = {
       }
 
       if (!fs.existsSync(filepath)) {
-        return `Error: File does not exist: ${filepath}`;
+        throw new ContinueError(
+          ContinueErrorReason.Unspecified,
+          `File does not exist: ${filepath}`,
+        );
       }
       const realPath = fs.realpathSync(filepath);
       const content = fs.readFileSync(realPath, "utf-8");
@@ -66,9 +70,14 @@ export const readFileTool: Tool = {
 
       return `Content of ${filepath}:\n${content}`;
     } catch (error) {
-      return `Error reading file: ${
-        error instanceof Error ? error.message : String(error)
-      }`;
+      if (error instanceof ContinueError) {
+        throw error;
+      }
+      throw new Error(
+        `Error reading file: ${
+          error instanceof Error ? error.message : String(error)
+        }`,
+      );
     }
   },
 };

extensions/cli/src/tools/searchCode.ts

@@ -2,6 +2,8 @@ import * as child_process from "child_process";
 import * as fs from "fs";
 import * as util from "util";
 
+import { ContinueError, ContinueErrorReason } from "core/util/errors.js";
+
 import { Tool } from "./types.js";
 
 const execPromise = util.promisify(child_process.exec);
@@ -53,64 +55,66 @@ export const searchCodeTool: Tool = {
     path?: string;
     file_pattern?: string;
   }): Promise<string> => {
-    try {
-      const searchPath = args.path || process.cwd();
-      if (!fs.existsSync(searchPath)) {
-        return `Error: Path does not exist: ${searchPath}`;
-      }
+    const searchPath = args.path || process.cwd();
+    if (!fs.existsSync(searchPath)) {
+      throw new ContinueError(
+        ContinueErrorReason.Unspecified,
+        `Path does not exist: ${searchPath}`,
+      );
+    }
 
-      let command = `rg --line-number --with-filename --color never "${args.pattern}"`;
+    let command = `rg --line-number --with-filename --color never "${args.pattern}"`;
 
-      if (args.file_pattern) {
-        command += ` -g "${args.file_pattern}"`;
-      }
+    if (args.file_pattern) {
+      command += ` -g "${args.file_pattern}"`;
+    }
 
-      command += ` "${searchPath}"`;
-      try {
-        const { stdout, stderr } = await execPromise(command);
+    command += ` "${searchPath}"`;
+    try {
+      const { stdout, stderr } = await execPromise(command);
 
-        if (stderr) {
-          return `Warning during search: ${stderr}\n\n${stdout}`;
-        }
+      if (stderr) {
+        return `Warning during search: ${stderr}\n\n${stdout}`;
+      }
 
-        if (!stdout.trim()) {
-          return `No matches found for pattern "${args.pattern}"${
-            args.file_pattern ? ` in files matching "${args.file_pattern}"` : ""
-          }.`;
-        }
+      if (!stdout.trim()) {
+        return `No matches found for pattern "${args.pattern}"${
+          args.file_pattern ? ` in files matching "${args.file_pattern}"` : ""
+        }.`;
+      }
 
-        // Split the results into lines and limit the number of results
-        const lines = stdout.split("\n");
-        const truncated = lines.length > DEFAULT_MAX_RESULTS;
-        const limitedLines = lines.slice(0, DEFAULT_MAX_RESULTS);
-        const resultText = limitedLines.join("\n");
+      // Split the results into lines and limit the number of results
+      const lines = stdout.split("\n");
+      const truncated = lines.length > DEFAULT_MAX_RESULTS;
+      const limitedLines = lines.slice(0, DEFAULT_MAX_RESULTS);
+      const resultText = limitedLines.join("\n");
 
-        const truncationMessage = truncated
-          ? `\n\n[Results truncated: showing ${DEFAULT_MAX_RESULTS} of ${lines.length} matches]`
-          : "";
+      const truncationMessage = truncated
+        ? `\n\n[Results truncated: showing ${DEFAULT_MAX_RESULTS} of ${lines.length} matches]`
+        : "";
 
-        return `Search results for pattern "${args.pattern}"${
+      return `Search results for pattern "${args.pattern}"${
+        args.file_pattern ? ` in files matching "${args.file_pattern}"` : ""
+      }:\n\n${resultText}${truncationMessage}`;
+    } catch (error: any) {
+      if (error instanceof ContinueError) {
+        throw error;
+      }
+      if (error.code === 1) {
+        return `No matches found for pattern "${args.pattern}"${
           args.file_pattern ? ` in files matching "${args.file_pattern}"` : ""
-        }:\n\n${resultText}${truncationMessage}`;
-      } catch (error: any) {
-        if (error.code === 1) {
-          return `No matches found for pattern "${args.pattern}"${
-            args.file_pattern ? ` in files matching "${args.file_pattern}"` : ""
-          }.`;
-        }
-        if (error instanceof Error) {
-          if (error.message.includes("command not found")) {
-            return `Error: ripgrep is not installed.`;
-          }
+        }.`;
+      }
+      if (error instanceof Error) {
+        if (error.message.includes("command not found")) {
+          throw new Error(`ripgrep is not installed.`);
         }
-        return `Error executing ripgrep: ${
-          error instanceof Error ? error.message : String(error)
-        }`;
       }
-    } catch (error) {
-      return `Error searching code: ${
-        error instanceof Error ? error.message : String(error)
-      }`;
+      throw new Error(
+        `Error executing ripgrep: ${
+          error instanceof Error ? error.message : String(error)
+        }`,
+      );
     }
   },
 };