Skip to content

Commit 5b0963d

Browse files
samhvw8samhvw8
committed
feat: enhance readFileTool with XML parsing and file processing state tracking
1 parent bc00c16 commit 5b0963d

File tree

2 files changed

+144
-67
lines changed

2 files changed

+144
-67
lines changed

‎src/core/Cline.ts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ import { EXPERIMENT_IDS, experiments as Experiments, ExperimentId } from "../sha
3838
import { formatLanguage } from "../shared/language"
3939
import { ToolParamName, ToolResponse, DiffStrategy } from "../shared/tools"
4040

41+
// utils
42+
import { parseXml } from "../utils/xml"
43+
4144
// services
4245
import { UrlContentFetcher } from "../services/browser/UrlContentFetcher"
4346
import { listFiles } from "../services/glob/list-files"
@@ -1249,8 +1252,37 @@ export class Cline extends EventEmitter<ClineEvents> {
12491252
switch (block.name) {
12501253
case "execute_command":
12511254
return `[${block.name} for '${block.params.command}']`
1252-
case "read_file":
1253-
return `[${block.name} for '${block.params.path}']`
1255+
case "read_file": {
1256+
// Handle both single path and multiple files via args
1257+
if (block.params.args) {
1258+
try {
1259+
const parsed = parseXml(block.params.args) as any
1260+
const files = Array.isArray(parsed.file)
1261+
? parsed.file
1262+
: [parsed.file].filter(Boolean)
1263+
const paths = files.map((f: any) => f?.path).filter(Boolean) as string[]
1264+
1265+
if (paths.length === 0) {
1266+
return `[${block.name} with no valid paths]`
1267+
} else if (paths.length === 1) {
1268+
return `[${block.name} for '${paths[0]}']`
1269+
} else if (paths.length <= 3) {
1270+
const pathList = paths.map((p) => `'${p}'`).join(", ")
1271+
return `[${block.name} for ${pathList}]`
1272+
} else {
1273+
return `[${block.name} for ${paths.length} files]`
1274+
}
1275+
} catch (error) {
1276+
console.error("Failed to parse read_file args XML for description:", error)
1277+
return `[${block.name} with unparseable args]`
1278+
}
1279+
} else if (block.params.path) {
1280+
// Fallback for legacy single-path usage
1281+
return `[${block.name} for '${block.params.path}']`
1282+
} else {
1283+
return `[${block.name} with missing path/args]`
1284+
}
1285+
}
12541286
case "fetch_instructions":
12551287
return `[${block.name} for '${block.params.task}']`
12561288
case "write_to_file":

‎src/core/tools/readFileTool.ts‎

Lines changed: 110 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,17 @@ interface FileEntry {
2626
lineRanges?: LineRange[]
2727
}
2828

29+
// New interface to track file processing state
30+
interface FileResult {
31+
path: string
32+
status: "approved" | "denied" | "blocked" | "error" | "pending"
33+
content?: string
34+
error?: string
35+
notice?: string
36+
lineRanges?: LineRange[]
37+
xmlContent?: string // Final XML content for this file
38+
}
39+
2940
export async function readFileTool(
3041
cline: Cline,
3142
block: ToolUse,
@@ -74,8 +85,6 @@ export async function readFileTool(
7485
const parsed = parseXml(argsXmlTag) as any
7586
const files = Array.isArray(parsed.file) ? parsed.file : [parsed.file].filter(Boolean)
7687

77-
console.log("Parsed files:", files)
78-
7988
for (const file of files) {
8089
if (!file.path) continue
8190

@@ -112,53 +121,68 @@ export async function readFileTool(
112121
return
113122
}
114123

115-
const results: string[] = []
124+
// Create an array to track the state of each file
125+
const fileResults: FileResult[] = fileEntries.map((entry) => ({
126+
path: entry.path || "",
127+
status: "pending",
128+
lineRanges: entry.lineRanges,
129+
}))
130+
131+
// Function to update file result status
132+
const updateFileResult = (path: string, updates: Partial<FileResult>) => {
133+
const index = fileResults.findIndex((result) => result.path === path)
134+
if (index !== -1) {
135+
fileResults[index] = { ...fileResults[index], ...updates }
136+
}
137+
}
116138

117139
try {
118140
// First validate all files and get approvals
119-
const blockedFiles = new Set<string>()
120-
const approvedFiles = new Set<string>()
121-
122-
for (const entry of fileEntries) {
123-
const relPath = entry.path || ""
141+
for (const fileResult of fileResults) {
142+
const relPath = fileResult.path
124143
const fullPath = path.resolve(cline.cwd, relPath)
125144

126145
// Validate line ranges first
127-
if (entry.lineRanges) {
128-
for (const range of entry.lineRanges) {
146+
if (fileResult.lineRanges) {
147+
let hasRangeError = false
148+
for (const range of fileResult.lineRanges) {
129149
if (range.start > range.end) {
130-
await handleFileError(
131-
new Error("Invalid line range: end line cannot be less than start line"),
132-
relPath,
133-
fileEntries.length === 1,
134-
results,
135-
handleError,
136-
)
137-
blockedFiles.add(relPath)
150+
const errorMsg = "Invalid line range: end line cannot be less than start line"
151+
updateFileResult(relPath, {
152+
status: "blocked",
153+
error: errorMsg,
154+
xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
155+
})
156+
await handleError(`reading file ${relPath}`, new Error(errorMsg))
157+
hasRangeError = true
138158
break
139159
}
140160
if (isNaN(range.start) || isNaN(range.end)) {
141-
await handleFileError(
142-
new Error("Invalid line range values"),
143-
relPath,
144-
fileEntries.length === 1,
145-
results,
146-
handleError,
147-
)
148-
blockedFiles.add(relPath)
161+
const errorMsg = "Invalid line range values"
162+
updateFileResult(relPath, {
163+
status: "blocked",
164+
error: errorMsg,
165+
xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
166+
})
167+
await handleError(`reading file ${relPath}`, new Error(errorMsg))
168+
hasRangeError = true
149169
break
150170
}
151171
}
172+
if (hasRangeError) continue
152173
}
153174

154175
// Then check RooIgnore validation
155-
if (!blockedFiles.has(relPath)) {
176+
if (fileResult.status === "pending") {
156177
const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath)
157178
if (!accessAllowed) {
158179
await cline.say("rooignore_error", relPath)
159180
const errorMsg = formatResponse.rooIgnoreError(relPath)
160-
results.push(`<file><path>${relPath}</path><error>${errorMsg}</error></file>`)
161-
blockedFiles.add(relPath)
181+
updateFileResult(relPath, {
182+
status: "blocked",
183+
error: errorMsg,
184+
xmlContent: `<file><path>${relPath}</path><error>${errorMsg}</error></file>`,
185+
})
162186
continue
163187
}
164188

@@ -168,8 +192,8 @@ export async function readFileTool(
168192

169193
// Create line snippet for approval message
170194
let lineSnippet = ""
171-
if (entry.lineRanges && entry.lineRanges.length > 0) {
172-
const ranges = entry.lineRanges.map((range) =>
195+
if (fileResult.lineRanges && fileResult.lineRanges.length > 0) {
196+
const ranges = fileResult.lineRanges.map((range) =>
173197
t("tools:readFile.linesRange", { start: range.start, end: range.end }),
174198
)
175199
lineSnippet = ranges.join(", ")
@@ -189,23 +213,25 @@ export async function readFileTool(
189213

190214
const didApprove = await askApproval("tool", completeMessage)
191215
if (!didApprove) {
192-
blockedFiles.add(relPath)
216+
updateFileResult(relPath, {
217+
status: "denied",
218+
xmlContent: `<file><path>${relPath}</path><status>Denied by user</status></file>`,
219+
})
193220
} else {
194-
approvedFiles.add(relPath)
221+
updateFileResult(relPath, { status: "approved" })
195222
}
196223
}
197224
}
198225

199226
// Then process only approved files
200-
for (const entry of fileEntries) {
201-
const relPath = entry.path || ""
202-
const fullPath = path.resolve(cline.cwd, relPath)
203-
227+
for (const fileResult of fileResults) {
204228
// Skip files that weren't approved
205-
if (!approvedFiles.has(relPath)) {
229+
if (fileResult.status !== "approved") {
206230
continue
207231
}
208232

233+
const relPath = fileResult.path
234+
const fullPath = path.resolve(cline.cwd, relPath)
209235
const { maxReadFileLine = 500 } = (await cline.providerRef.deref()?.getState()) ?? {}
210236

211237
// Process approved files
@@ -214,32 +240,37 @@ export async function readFileTool(
214240

215241
// Handle binary files
216242
if (isBinary) {
217-
results.push(`<file><path>${relPath}</path>\n<notice>Binary file</notice>\n</file>`)
243+
updateFileResult(relPath, {
244+
notice: "Binary file",
245+
xmlContent: `<file><path>${relPath}</path>\n<notice>Binary file</notice>\n</file>`,
246+
})
218247
continue
219248
}
220249

221250
// Handle range reads (bypass maxReadFileLine)
222-
if (entry.lineRanges && entry.lineRanges.length > 0) {
251+
if (fileResult.lineRanges && fileResult.lineRanges.length > 0) {
223252
const rangeResults: string[] = []
224-
for (const range of entry.lineRanges) {
253+
for (const range of fileResult.lineRanges) {
225254
const content = addLineNumbers(
226255
await readLines(fullPath, range.end - 1, range.start - 1),
227256
range.start,
228257
)
229258
const lineRangeAttr = ` lines="${range.start}-${range.end}"`
230259
rangeResults.push(`<content${lineRangeAttr}>\n${content}</content>`)
231260
}
232-
results.push(`<file><path>${relPath}</path>\n${rangeResults.join("\n")}\n</file>`)
261+
updateFileResult(relPath, {
262+
xmlContent: `<file><path>${relPath}</path>\n${rangeResults.join("\n")}\n</file>`,
263+
})
233264
continue
234265
}
235266

236267
// Handle definitions-only mode
237268
if (maxReadFileLine === 0) {
238269
const defResult = await parseSourceCodeDefinitionsForFile(fullPath, cline.rooIgnoreController)
239270
if (defResult) {
240-
results.push(
241-
`<file><path>${relPath}</path>\n<list_code_definition_names>${defResult}</list_code_definition_names>\n</file>`,
242-
)
271+
updateFileResult(relPath, {
272+
xmlContent: `<file><path>${relPath}</path>\n<list_code_definition_names>${defResult}</list_code_definition_names>\n</file>`,
273+
})
243274
}
244275
continue
245276
}
@@ -255,7 +286,9 @@ export async function readFileTool(
255286
xmlInfo += `<list_code_definition_names>${defResult}</list_code_definition_names>\n`
256287
}
257288
xmlInfo += `<notice>Showing only ${maxReadFileLine} of ${totalLines} total lines. Use line_range if you need to read more lines</notice>\n`
258-
results.push(`<file><path>${relPath}</path>\n${xmlInfo}</file>`)
289+
updateFileResult(relPath, {
290+
xmlContent: `<file><path>${relPath}</path>\n${xmlInfo}</file>`,
291+
})
259292
continue
260293
}
261294

@@ -271,32 +304,44 @@ export async function readFileTool(
271304
// Track file read
272305
await cline.getFileContextTracker().trackFileContext(relPath, "read_tool" as RecordSource)
273306

274-
results.push(`<file><path>${relPath}</path>\n${xmlInfo}</file>`)
307+
updateFileResult(relPath, {
308+
xmlContent: `<file><path>${relPath}</path>\n${xmlInfo}</file>`,
309+
})
275310
} catch (error) {
276-
await handleFileError(error, relPath, fileEntries.length === 1, results, handleError)
311+
const errorMsg = error instanceof Error ? error.message : String(error)
312+
updateFileResult(relPath, {
313+
status: "error",
314+
error: `Error reading file: ${errorMsg}`,
315+
xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
316+
})
317+
await handleError(`reading file ${relPath}`, error instanceof Error ? error : new Error(errorMsg))
277318
}
278319
}
279320

321+
// Generate final XML result from all file results
322+
const xmlResults = fileResults.filter((result) => result.xmlContent).map((result) => result.xmlContent)
323+
280324
// Push combined results
281-
pushToolResult(`<files>\n${results.join("\n")}\n</files>`)
325+
pushToolResult(`<files>\n${xmlResults.join("\n")}\n</files>`)
282326
} catch (error) {
283327
// Handle all errors using per-file format for consistency
284328
const relPath = fileEntries[0]?.path || "unknown"
285-
await handleFileError(error, relPath, false, results, handleError)
286-
pushToolResult(`<files>\n${results.join("\n")}\n</files>`)
287-
}
288-
}
329+
const errorMsg = error instanceof Error ? error.message : String(error)
330+
331+
// If we have file results, update the first one with the error
332+
if (fileResults.length > 0) {
333+
updateFileResult(relPath, {
334+
status: "error",
335+
error: `Error reading file: ${errorMsg}`,
336+
xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
337+
})
338+
}
289339

290-
// Error handling function
291-
async function handleFileError(
292-
error: unknown,
293-
relPath: string,
294-
isOnlyFile: boolean,
295-
results: string[],
296-
handleError: HandleError,
297-
): Promise<void> {
298-
const errorMsg = error instanceof Error ? error.message : String(error)
299-
// Always use per-file error format for consistency
300-
results.push(`<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`)
301-
await handleError(`reading file ${relPath}`, error instanceof Error ? error : new Error(errorMsg))
340+
await handleError(`reading file ${relPath}`, error instanceof Error ? error : new Error(errorMsg))
341+
342+
// Generate final XML result from all file results
343+
const xmlResults = fileResults.filter((result) => result.xmlContent).map((result) => result.xmlContent)
344+
345+
pushToolResult(`<files>\n${xmlResults.join("\n")}\n</files>`)
346+
}
302347
}

0 commit comments

Comments
 (0)