Skip to content

Commit 538459c

Browse files
fix(readFileTool): align legacy path failure flag with native path; drop temp warn
1 parent 4c6d443 commit 538459c

3 files changed

Lines changed: 8 additions & 9 deletions

File tree

packages/types/src/tool-params.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ export type ReadFileToolParams = ReadFileParams | LegacyReadFileParams
8989
* Type guard to check if params are in legacy format.
9090
*/
9191
export function isLegacyReadFileParams(params: ReadFileToolParams): params is LegacyReadFileParams {
92-
// Detect explicit flag (new code path) or bare `files` array (real legacy chat data)
92+
// `NativeToolCallParser` always tags freshly parsed legacy calls with `_legacyFormat: true`.
93+
// The bare-`files` fallback only matters for chat history persisted before that flag was
94+
// introduced (commit cc86049f1) and re-hydrated on a later run.
9395
const hasLegacyFlag = "_legacyFormat" in params && params._legacyFormat === true
9496
const hasFilesArray = "files" in params && Array.isArray((params as unknown as Record<string, unknown>).files)
9597
return hasLegacyFlag || hasFilesArray

src/core/tools/ReadFileTool.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -668,9 +668,6 @@ export class ReadFileTool extends BaseTool<"read_file"> {
668668
const { pushToolResult } = callbacks
669669
const modelInfo = task.api.getModel().info
670670

671-
// Temporary indicator for testing legacy format detection
672-
console.warn("[read_file] Legacy format detected - using backward compatibility path")
673-
674671
if (!fileEntries || fileEntries.length === 0) {
675672
task.consecutiveMistakeCount++
676673
task.recordToolError("read_file")
@@ -694,6 +691,8 @@ export class ReadFileTool extends BaseTool<"read_file"> {
694691
await task.say("rooignore_error", relPath)
695692
const errorMsg = formatResponse.rooIgnoreError(relPath)
696693
results.push(`File: ${relPath}\nError: ${errorMsg}`)
694+
// Mirror the native path: a blocked file marks the tool turn as failed.
695+
task.didToolFailInCurrentTurn = true
697696
continue
698697
}
699698

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -748,14 +748,10 @@ describe("ReadFileTool", () => {
748748
mockTask.ask.mockResolvedValue({ response: "yesButtonClicked", text: undefined, images: undefined })
749749
mockedFsReadFile.mockResolvedValue(Buffer.from("legacy content"))
750750

751-
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
752-
753751
await readFileTool.execute({ files: [{ path: "legacy.ts" }] } as any, mockTask as any, callbacks)
754752

755-
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Legacy format detected"))
753+
// The legacy path emits "File: <path>" entries — proof the backward-compat branch ran.
756754
expect(callbacks.pushToolResult).toHaveBeenCalledWith(expect.stringContaining("File: legacy.ts"))
757-
758-
consoleWarnSpy.mockRestore()
759755
})
760756

761757
it("should return error when legacy files array is empty", async () => {
@@ -816,6 +812,8 @@ describe("ReadFileTool", () => {
816812

817813
expect(mockTask.say).toHaveBeenCalledWith("rooignore_error", "secret.env")
818814
expect(callbacks.pushToolResult).toHaveBeenCalledWith(expect.stringContaining("blocked by the .rooignore"))
815+
// Consistent with the native path: a blocked file fails the tool turn.
816+
expect(mockTask.didToolFailInCurrentTurn).toBe(true)
819817
})
820818

821819
it("should deny legacy file read when user clicks no", async () => {

0 commit comments

Comments
 (0)