feat: improve error message when read_file is used on directory

- Add early directory detection in ReadFileTool before attempting file read
- Provide clear, actionable error message suggesting use of list_files tool
- Add test case for directory reading error scenario
- Fix fsPromises.stat mocking in all test suites to support directory checks

Fixes ROO-319

Author: roomote

src/core/tools/ReadFileTool.ts

@@ -1,4 +1,5 @@
 import path from "path"
+import * as fs from "fs/promises"
 import { isBinaryFile } from "isbinaryfile"
 import type { FileEntry, LineRange } from "@roo-code/types"
 import { isNativeProtocol, ANTHROPIC_DEFAULT_MAX_TOKENS } from "@roo-code/types"
@@ -350,6 +351,20 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 				const fullPath = path.resolve(task.cwd, relPath)
 
 				try {
+					// Check if the path is a directory before attempting to read it
+					const stats = await fs.stat(fullPath)
+					if (stats.isDirectory()) {
+						const errorMsg = `Cannot read '${relPath}' because it is a directory. To view the contents of a directory, use the list_files tool instead.`
+						updateFileResult(relPath, {
+							status: "error",
+							error: errorMsg,
+							xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
+							nativeContent: `File: ${relPath}\nError: Error reading file: ${errorMsg}`,
+						})
+						await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
+						continue
+					}
+
 					const [totalLines, isBinary] = await Promise.all([countFileLines(fullPath), isBinaryFile(fullPath)])
 
 					if (isBinary) {

src/core/tools/__tests__/readFileTool.spec.ts

@@ -305,6 +305,13 @@ describe("read_file tool with maxReadFileLine setting", () => {
 		mockedPathResolve.mockReturnValue(absoluteFilePath)
 		mockedIsBinaryFile.mockResolvedValue(false)
 
+		// Mock fsPromises.stat to return a file (not directory) by default
+		fsPromises.stat.mockResolvedValue({
+			isDirectory: () => false,
+			isFile: () => true,
+			isSymbolicLink: () => false,
+		} as any)
+
 		mockInputContent = fileContent
 
 		// Setup the extractTextFromFile mock implementation with the current mockInputContent
@@ -612,7 +619,12 @@ describe("read_file tool output structure", () => {
 
 		// CRITICAL: Reset fsPromises mocks to prevent cross-test contamination
 		fsPromises.stat.mockClear()
-		fsPromises.stat.mockResolvedValue({ size: 1024 })
+		fsPromises.stat.mockResolvedValue({
+			size: 1024,
+			isDirectory: () => false,
+			isFile: () => true,
+			isSymbolicLink: () => false,
+		} as any)
 		fsPromises.readFile.mockClear()
 
 		// Use shared mock setup function
@@ -1429,6 +1441,37 @@ describe("read_file tool output structure", () => {
 				`File: ${testFilePath}\nError: Access to ${testFilePath} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.`,
 			)
 		})
+
+		it("should provide helpful error when trying to read a directory", async () => {
+			// Setup - mock fsPromises.stat to indicate the path is a directory
+			const dirPath = "test/my-directory"
+			const absoluteDirPath = "/test/my-directory"
+
+			mockedPathResolve.mockReturnValue(absoluteDirPath)
+
+			// Mock fs/promises stat to return directory
+			fsPromises.stat.mockResolvedValue({
+				isDirectory: () => true,
+				isFile: () => false,
+				isSymbolicLink: () => false,
+			} as any)
+
+			// Mock isBinaryFile won't be called since we check directory first
+			mockedIsBinaryFile.mockResolvedValue(false)
+
+			// Execute
+			const result = await executeReadFileTool({ args: `<file><path>${dirPath}</path></file>` })
+
+			// Verify - native format for error
+			expect(result).toContain(`File: ${dirPath}`)
+			expect(result).toContain(`Error: Error reading file: Cannot read '${dirPath}' because it is a directory`)
+			expect(result).toContain("use the list_files tool instead")
+
+			// Verify that task.say was called with the error
+			expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Cannot read"))
+			expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("is a directory"))
+			expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("list_files tool"))
+		})
 	})
 })
 
@@ -1460,7 +1503,12 @@ describe("read_file tool with image support", () => {
 
 		// CRITICAL: Reset fsPromises.stat to prevent cross-test contamination
 		fsPromises.stat.mockClear()
-		fsPromises.stat.mockResolvedValue({ size: 1024 })
+		fsPromises.stat.mockResolvedValue({
+			size: 1024,
+			isDirectory: () => false,
+			isFile: () => true,
+			isSymbolicLink: () => false,
+		} as any)
 
 		// Use shared mock setup function with local variables
 		const mocks = createMockCline()
@@ -1801,6 +1849,13 @@ describe("read_file tool concurrent file reads limit", () => {
 		mockedIsBinaryFile.mockResolvedValue(false)
 		mockedCountFileLines.mockResolvedValue(10)
 
+		// Mock fsPromises.stat to return a file (not directory) by default
+		fsPromises.stat.mockResolvedValue({
+			isDirectory: () => false,
+			isFile: () => true,
+			isSymbolicLink: () => false,
+		} as any)
+
 		toolResult = undefined
 	})