From 63f864cdd79a0357ec1e0474586fb00a0ec7d91b Mon Sep 17 00:00:00 2001
From: Taylor Mullen <ntaylormullen@google.com>
Date: Sun, 20 Apr 2025 22:10:23 -0400
Subject: [PATCH] Fix read-file from exploding with path not found error.

- There were a few hiccups here. Somehow 2.5-flash wasn't actually abiding by our tool schema. Instead it was inferring `path`. To semi-combat this I've renamed `file_path` -> `path`.
- We weren't elevating errors that were created via schema validation. Instead both the `glob` and `read-file.ts` now surface this.
- In error scenarios (like failing schema) we were improperly surfacing these as success cases because we were overriding tool status.
---
 packages/cli/src/tools/read-file.tool.ts     |  5 ++---
 packages/cli/src/ui/hooks/useGeminiStream.ts | 13 +++++++++++--
 packages/server/src/tools/glob.ts            |  2 +-
 packages/server/src/tools/read-file.ts       | 14 +++++++-------
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/packages/cli/src/tools/read-file.tool.ts b/packages/cli/src/tools/read-file.tool.ts
index bbf9eafb..206267be 100644
--- a/packages/cli/src/tools/read-file.tool.ts
+++ b/packages/cli/src/tools/read-file.tool.ts
@@ -38,8 +38,7 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> {
    * Delegates validation to the core logic
    */
   validateToolParams(_params: ReadFileToolParams): string | null {
-    // Currently allowing any path. Add validation if needed.
-    return null;
+    return this.coreLogic.validateToolParams(_params);
   }
 
   /**
@@ -62,7 +61,7 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> {
   /**
    * Delegates execution to the core logic
    */
-  async execute(params: ReadFileToolParams): Promise<ToolResult> {
+  execute(params: ReadFileToolParams): Promise<ToolResult> {
     return this.coreLogic.execute(params);
   }
 }
diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts
index 6e3e8e84..21a9f508 100644
--- a/packages/cli/src/ui/hooks/useGeminiStream.ts
+++ b/packages/cli/src/ui/hooks/useGeminiStream.ts
@@ -330,7 +330,13 @@ export const useGeminiStream = (
                       ...item,
                       tools: item.tools.map((tool) =>
                         tool.callId === callId
-                          ? { ...tool, status: ToolCallStatus.Invoked }
+                          ? {
+                              ...tool,
+                              status:
+                                tool.status === ToolCallStatus.Error
+                                  ? ToolCallStatus.Error
+                                  : ToolCallStatus.Invoked,
+                            }
                           : tool,
                       ),
                     };
@@ -362,7 +368,10 @@ export const useGeminiStream = (
                         tool.callId === callId
                           ? {
                               ...tool,
-                              status: ToolCallStatus.Success,
+                              status:
+                                tool.status === ToolCallStatus.Error
+                                  ? ToolCallStatus.Error
+                                  : ToolCallStatus.Success,
                               resultDisplay: result.returnDisplay,
                             }
                           : tool,
diff --git a/packages/server/src/tools/glob.ts b/packages/server/src/tools/glob.ts
index b9a3143c..e81858c8 100644
--- a/packages/server/src/tools/glob.ts
+++ b/packages/server/src/tools/glob.ts
@@ -149,7 +149,7 @@ export class GlobLogic extends BaseTool<GlobToolParams, ToolResult> {
     if (validationError) {
       return {
         llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`,
-        returnDisplay: `Error: Failed to execute tool.`,
+        returnDisplay: validationError,
       };
     }
 
diff --git a/packages/server/src/tools/read-file.ts b/packages/server/src/tools/read-file.ts
index 9d053003..52856e42 100644
--- a/packages/server/src/tools/read-file.ts
+++ b/packages/server/src/tools/read-file.ts
@@ -17,7 +17,7 @@ export interface ReadFileToolParams {
   /**
    * The absolute path to the file to read
    */
-  file_path: string;
+  path: string;
 
   /**
    * The line number to start reading from (optional)
@@ -46,7 +46,7 @@ export class ReadFileLogic extends BaseTool<ReadFileToolParams, ToolResult> {
       '', // Description handled by CLI wrapper
       {
         properties: {
-          file_path: {
+          path: {
             description:
               "The absolute path to the file to read (e.g., '/home/user/project/file.txt'). Relative paths are not supported.",
             type: 'string',
@@ -62,7 +62,7 @@ export class ReadFileLogic extends BaseTool<ReadFileToolParams, ToolResult> {
             type: 'number',
           },
         },
-        required: ['file_path'],
+        required: ['path'],
         type: 'object',
       },
     );
@@ -101,7 +101,7 @@ export class ReadFileLogic extends BaseTool<ReadFileToolParams, ToolResult> {
     ) {
       return 'Parameters failed schema validation.';
     }
-    const filePath = params.file_path;
+    const filePath = params.path;
     if (!path.isAbsolute(filePath)) {
       return `File path must be absolute: ${filePath}`;
     }
@@ -185,7 +185,7 @@ export class ReadFileLogic extends BaseTool<ReadFileToolParams, ToolResult> {
    * @returns A string describing the file being read
    */
   getDescription(params: ReadFileToolParams): string {
-    const relativePath = makeRelative(params.file_path, this.rootDirectory);
+    const relativePath = makeRelative(params.path, this.rootDirectory);
     return shortenPath(relativePath);
   }
 
@@ -199,11 +199,11 @@ export class ReadFileLogic extends BaseTool<ReadFileToolParams, ToolResult> {
     if (validationError) {
       return {
         llmContent: `Error: Invalid parameters provided. Reason: ${validationError}`,
-        returnDisplay: '**Error:** Failed to execute tool.',
+        returnDisplay: validationError,
       };
     }
 
-    const filePath = params.file_path;
+    const filePath = params.path;
     try {
       if (!fs.existsSync(filePath)) {
         return {