test(readFileTool): comprehensive coverage; align legacy failure flag with native path (#222)

Add unit coverage for ReadFileTool: input validation, rooignore blocking,
directory/binary/image handling, image memory limits, approval flow, slice and
indentation modes, output structure, and the legacy multi-file format.

Source changes:
- Drop the temporary "[read_file] Legacy format detected" debug console.warn.
- Mirror the native path in the legacy read path: set didToolFailInCurrentTurn
  on rooignore blocks, directory reads, and read errors so a failed legacy read
  fails the tool turn consistently.
- Recognize re-hydrated bare-`files` calls in isLegacyReadFileParams (history
  persisted before the _legacyFormat flag existed).

Co-authored-by: Armando Vaquera <263793884+proyectoauraorg@users.noreply.github.com>

Author: proyectoauraorg

packages/types/src/tool-params.ts

@@ -89,7 +89,14 @@ export type ReadFileToolParams = ReadFileParams | LegacyReadFileParams
  * Type guard to check if params are in legacy format.
  */
 export function isLegacyReadFileParams(params: ReadFileToolParams): params is LegacyReadFileParams {
-	return "_legacyFormat" in params && params._legacyFormat === true
+	// `NativeToolCallParser` always tags freshly parsed legacy calls with `_legacyFormat: true`.
+	// The bare-`files` fallback only matters for chat history persisted before that flag was
+	// introduced (commit cc86049f1) and re-hydrated on a later run. Note that params matched via
+	// that fallback narrow to `LegacyReadFileParams` but leave `_legacyFormat` `undefined`, so
+	// callers should branch on the presence of `files`, not on `_legacyFormat === true`.
+	const hasLegacyFlag = "_legacyFormat" in params && params._legacyFormat === true
+	const hasFilesArray = "files" in params && Array.isArray((params as unknown as Record<string, unknown>).files)
+	return hasLegacyFlag || hasFilesArray
 }
 
 export interface Coordinate {

src/core/tools/ReadFileTool.ts

@@ -668,9 +668,6 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 		const { pushToolResult } = callbacks
 		const modelInfo = task.api.getModel().info
 
-		// Temporary indicator for testing legacy format detection
-		console.warn("[read_file] Legacy format detected - using backward compatibility path")
-
 		if (!fileEntries || fileEntries.length === 0) {
 			task.consecutiveMistakeCount++
 			task.recordToolError("read_file")
@@ -694,6 +691,8 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 				await task.say("rooignore_error", relPath)
 				const errorMsg = formatResponse.rooIgnoreError(relPath)
 				results.push(`File: ${relPath}\nError: ${errorMsg}`)
+				// Mirror the native path: a blocked file marks the tool turn as failed.
+				task.didToolFailInCurrentTurn = true
 				continue
 			}
 
@@ -731,6 +730,8 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 					const errorMsg = `Cannot read '${relPath}' because it is a directory.`
 					results.push(`File: ${relPath}\nError: ${errorMsg}`)
 					await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
+					// Mirror the native path: a failed read marks the tool turn as failed.
+					task.didToolFailInCurrentTurn = true
 					continue
 				}
 
@@ -802,6 +803,8 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 				const errorMsg = error instanceof Error ? error.message : String(error)
 				results.push(`File: ${relPath}\nError: ${errorMsg}`)
 				await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
+				// Mirror the native path: a failed read marks the tool turn as failed.
+				task.didToolFailInCurrentTurn = true
 			}
 		}