Skip to content

Commit e5342b3

Browse files
committed
feat: Enhance read_file tool to support multiple files via setting
Implements the ability for the `read_file` tool to accept a JSON array string of file paths in its `path` parameter, allowing the AI to request multiple files in a single call. Key changes: - Added `maxConcurrentFileReads` setting (default: 1) to control the maximum number of files read per request. - Updated setting schema, types, UI (slider in Context Management), state management, and persistence logic for the new setting. - Modified `readFileTool.ts` to parse the `path` parameter (handling both single strings and JSON arrays), enforce the `maxConcurrentFileReads` limit, and aggregate results/errors. - Updated the `read_file` tool prompt description to inform the AI about the new capability and usage. - Includes fixes for UI slider display/persistence and backend state handling. - Updated relevant unit tests.
1 parent a3f1a3f commit e5342b3

File tree

17 files changed

+964
-291
lines changed

17 files changed

+964
-291
lines changed

src/core/__tests__/read-file-tool.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,105 @@ describe("read_file tool with maxReadFileLine setting", () => {
138138
expect(addLineNumbers).toHaveBeenCalled()
139139
})
140140
})
141+
142+
// Test suite for path parameter parsing
143+
describe("read_file tool path parameter parsing", () => {
144+
// Variable to hold the imported function
145+
let readFileTool: any;
146+
// Variable to hold the mock t function for this suite - DECLARE HERE
147+
let localMockT: jest.Mock;
148+
const mockCline = {
149+
consecutiveMistakeCount: 0,
150+
recordToolError: jest.fn(),
151+
say: jest.fn(),
152+
providerRef: { // Mock providerRef and deref
153+
deref: () => ({
154+
log: jest.fn(), // Mock the log function
155+
getState: jest.fn().mockResolvedValue({ // Mock getState
156+
maxReadFileLine: 500,
157+
maxConcurrentFileReads: 1,
158+
alwaysAllowReadOnly: false,
159+
alwaysAllowReadOnlyOutsideWorkspace: false,
160+
}),
161+
}),
162+
},
163+
cwd: "/test/workspace", // Mock cwd
164+
rooIgnoreController: { // Mock rooIgnoreController
165+
validateAccess: jest.fn().mockReturnValue(true),
166+
},
167+
getFileContextTracker: jest.fn(() => ({ // Mock getFileContextTracker
168+
trackFileContext: jest.fn().mockResolvedValue(undefined),
169+
})),
170+
} as any // Use 'any' for simplicity in mocking
171+
172+
const mockAskApproval = jest.fn().mockResolvedValue(true)
173+
const mockHandleError = jest.fn()
174+
const mockPushToolResult = jest.fn()
175+
const mockRemoveClosingTag = jest.fn(); // Define mock inside describe
176+
177+
beforeEach(() => {
178+
// Reset modules to ensure fresh mocks for this suite
179+
jest.resetModules();
180+
181+
// ASSIGN mock implementation for t here
182+
localMockT = jest.fn((key, params) => {
183+
if (key === "tools:readFile.error.incompleteJsonArray") {
184+
return `Incomplete or malformed file path array: ${params?.value}. It looks like a JSON array but is missing the closing bracket or is otherwise invalid.`
185+
}
186+
return key
187+
});
188+
189+
// Apply the mock for i18n specifically for this suite
190+
jest.doMock("../../i18n", () => ({
191+
t: localMockT,
192+
}));
193+
194+
// Require the module *after* setting up the mock
195+
readFileTool = require("../tools/readFileTool").readFileTool;
196+
197+
// Reset other mocks before each test
198+
jest.clearAllMocks()
199+
mockCline.consecutiveMistakeCount = 0
200+
mockCline.recordToolError.mockClear();
201+
mockCline.say.mockClear();
202+
mockAskApproval.mockClear();
203+
mockHandleError.mockClear();
204+
mockPushToolResult.mockClear();
205+
mockRemoveClosingTag.mockClear();
206+
mockRemoveClosingTag.mockImplementation((_tag: string, value: string | undefined): string | undefined => value);
207+
})
208+
209+
it("should return incompleteJsonArray error for malformed JSON array string", async () => {
210+
const incompleteJsonPath = '["file1.txt", "file2.txt"' // Missing closing bracket
211+
212+
const block = {
213+
tool_name: "read_file",
214+
tool_id: "1",
215+
params: {
216+
path: incompleteJsonPath,
217+
},
218+
}
219+
220+
await readFileTool(
221+
mockCline,
222+
block,
223+
mockAskApproval,
224+
mockHandleError,
225+
mockPushToolResult,
226+
mockRemoveClosingTag, // Pass the mock function as argument
227+
)
228+
229+
expect(mockCline.recordToolError).toHaveBeenCalledWith("read_file")
230+
expect(localMockT).toHaveBeenCalledWith("tools:readFile.error.incompleteJsonArray", { value: incompleteJsonPath })
231+
expect(mockCline.say).toHaveBeenCalledWith(
232+
"error",
233+
// Use the mock function directly to get the expected string
234+
localMockT("tools:readFile.error.incompleteJsonArray", { value: incompleteJsonPath }),
235+
)
236+
expect(mockPushToolResult).toHaveBeenCalledWith(
237+
`<tool_error tool_name="read_file">Incomplete or malformed file path array: ${incompleteJsonPath}. It looks like a JSON array but is missing the closing bracket or is otherwise invalid.</tool_error>`,
238+
)
239+
})
240+
241+
// Add more tests for other parsing scenarios (valid JSON, single path, invalid format, etc.) if needed
242+
})

0 commit comments

Comments
 (0)