Skip to content

Commit 5e50c55

Browse files
KJ7LNWEric Wheeler
KJ7LNW
and
Eric Wheeler
authored
fix: prevent dump of an entire file into the context on user edit (#3654)
* refactor: Add pushToolWriteResult method to DiffViewProvider Previously, each tool file contained duplicate code for formatting file write responses and conditionally sending user_feedback_diff messages. This led to inconsistent implementations and made changes difficult to maintain. This refactoring centralizes the response formatting and messaging logic in the DiffViewProvider class, which now: - Stores results from saveChanges() in class properties - Only sends user_feedback_diff when user edits exist - Configures XMLBuilder with no indentation for cleaner output Tool files now make a single method call instead of duplicating logic. Fixes: #3647 Signed-off-by: Eric Wheeler <[email protected]> * fix: conditionally show user edits message in file write response Make the 'If the user's edits have addressed part of the task...' message conditional based on whether there are actual user edits. This prevents showing irrelevant guidance when no user edits were made to the file. Signed-off-by: Eric Wheeler <[email protected]> --------- Signed-off-by: Eric Wheeler <[email protected]> Co-authored-by: Eric Wheeler <[email protected]>
1 parent e40a4c9 commit 5e50c55

File tree

5 files changed

+115
-109
lines changed

5 files changed

+115
-109
lines changed

‎src/core/tools/applyDiffTool.ts‎

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { Task } from "../task/Task"
99
import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } from "../../shared/tools"
1010
import { formatResponse } from "../prompts/responses"
1111
import { fileExistsAtPath } from "../../utils/fs"
12-
import { addLineNumbers } from "../../integrations/misc/extract-text"
1312
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1413
import { unescapeHtmlEntities } from "../../utils/text-normalization"
1514

@@ -163,7 +162,8 @@ export async function applyDiffTool(
163162
return
164163
}
165164

166-
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
165+
// Call saveChanges to update the DiffViewProvider properties
166+
await cline.diffViewProvider.saveChanges()
167167

168168
// Track file edit operation
169169
if (relPath) {
@@ -178,33 +178,13 @@ export async function applyDiffTool(
178178
partFailHint = `But unable to apply all diff parts to file: ${absolutePath}, silently use <read_file> tool to check newest file version and re-apply diffs\n`
179179
}
180180

181-
if (userEdits) {
182-
await cline.say(
183-
"user_feedback_diff",
184-
JSON.stringify({
185-
tool: fileExists ? "editedExistingFile" : "newFileCreated",
186-
path: getReadablePath(cline.cwd, relPath),
187-
diff: userEdits,
188-
} satisfies ClineSayTool),
189-
)
190-
191-
pushToolResult(
192-
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
193-
partFailHint +
194-
`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` +
195-
`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(
196-
finalContent || "",
197-
)}\n</final_file_content>\n\n` +
198-
`Please note:\n` +
199-
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
200-
`2. Proceed with the task using this updated file content as the new baseline.\n` +
201-
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
202-
`${newProblemsMessage}`,
203-
)
181+
// Get the formatted response message
182+
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
183+
184+
if (partFailHint) {
185+
pushToolResult(partFailHint + message)
204186
} else {
205-
pushToolResult(
206-
`Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}\n` + partFailHint,
207-
)
187+
pushToolResult(message)
208188
}
209189

210190
await cline.diffViewProvider.reset()

‎src/core/tools/insertContentTool.ts‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ export async function insertContentTool(
136136
return
137137
}
138138

139-
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
139+
// Call saveChanges to update the DiffViewProvider properties
140+
await cline.diffViewProvider.saveChanges()
140141

141142
// Track file edit operation
142143
if (relPath) {
@@ -145,34 +146,14 @@ export async function insertContentTool(
145146

146147
cline.didEditFile = true
147148

148-
if (!userEdits) {
149-
pushToolResult(
150-
`The content was successfully inserted in ${relPath.toPosix()} at line ${lineNumber}.${newProblemsMessage}`,
151-
)
152-
await cline.diffViewProvider.reset()
153-
return
154-
}
155-
156-
await cline.say(
157-
"user_feedback_diff",
158-
JSON.stringify({
159-
tool: "insertContent",
160-
path: getReadablePath(cline.cwd, relPath),
161-
diff: userEdits,
162-
lineNumber: lineNumber,
163-
} satisfies ClineSayTool),
149+
// Get the formatted response message
150+
const message = await cline.diffViewProvider.pushToolWriteResult(
151+
cline,
152+
cline.cwd,
153+
false, // Always false for insert_content
164154
)
165155

166-
pushToolResult(
167-
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
168-
`The updated content has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file:\n\n` +
169-
`<final_file_content path="${relPath.toPosix()}">\n${finalContent}\n</final_file_content>\n\n` +
170-
`Please note:\n` +
171-
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
172-
`2. Proceed with the task using this updated file content as the new baseline.\n` +
173-
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
174-
`${newProblemsMessage}`,
175-
)
156+
pushToolResult(message)
176157

177158
await cline.diffViewProvider.reset()
178159
} catch (error) {

‎src/core/tools/searchAndReplaceTool.ts‎

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,8 @@ export async function searchAndReplaceTool(
219219
return
220220
}
221221

222-
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
222+
// Call saveChanges to update the DiffViewProvider properties
223+
await cline.diffViewProvider.saveChanges()
223224

224225
// Track file edit operation
225226
if (relPath) {
@@ -228,34 +229,14 @@ export async function searchAndReplaceTool(
228229

229230
cline.didEditFile = true
230231

231-
if (!userEdits) {
232-
pushToolResult(`The content was successfully replaced in ${relPath}.${newProblemsMessage}`)
233-
await cline.diffViewProvider.reset()
234-
return
235-
}
236-
237-
await cline.say(
238-
"user_feedback_diff",
239-
JSON.stringify({
240-
tool: "appliedDiff",
241-
path: getReadablePath(cline.cwd, relPath),
242-
diff: userEdits,
243-
} satisfies ClineSayTool),
232+
// Get the formatted response message
233+
const message = await cline.diffViewProvider.pushToolWriteResult(
234+
cline,
235+
cline.cwd,
236+
false, // Always false for search_and_replace
244237
)
245238

246-
// Format and send response with user's updates
247-
const resultMessage = [
248-
`The user made the following updates to your content:\n\n${userEdits}\n\n`,
249-
`The updated content has been successfully saved to ${validRelPath.toPosix()}. Here is the full, updated content of the file:\n\n`,
250-
`<final_file_content path="${validRelPath.toPosix()}">\n${finalContent}\n</final_file_content>\n\n`,
251-
`Please note:\n`,
252-
`1. You do not need to re-write the file with these changes, as they have already been applied.\n`,
253-
`2. Proceed with the task using the updated file content as the new baseline.\n`,
254-
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.`,
255-
newProblemsMessage,
256-
].join("")
257-
258-
pushToolResult(resultMessage)
239+
pushToolResult(message)
259240

260241
// Record successful tool usage and cleanup
261242
cline.recordToolUsage("search_and_replace")

‎src/core/tools/writeToFileTool.ts‎

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { formatResponse } from "../prompts/responses"
88
import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools"
99
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1010
import { fileExistsAtPath } from "../../utils/fs"
11-
import { addLineNumbers, stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
11+
import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
1212
import { getReadablePath } from "../../utils/path"
1313
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
1414
import { detectCodeOmission } from "../../integrations/editor/detect-omission"
@@ -208,7 +208,8 @@ export async function writeToFileTool(
208208
return
209209
}
210210

211-
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
211+
// Call saveChanges to update the DiffViewProvider properties
212+
await cline.diffViewProvider.saveChanges()
212213

213214
// Track file edit operation
214215
if (relPath) {
@@ -217,31 +218,10 @@ export async function writeToFileTool(
217218

218219
cline.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request
219220

220-
if (userEdits) {
221-
await cline.say(
222-
"user_feedback_diff",
223-
JSON.stringify({
224-
tool: fileExists ? "editedExistingFile" : "newFileCreated",
225-
path: getReadablePath(cline.cwd, relPath),
226-
diff: userEdits,
227-
} satisfies ClineSayTool),
228-
)
221+
// Get the formatted response message
222+
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
229223

230-
pushToolResult(
231-
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
232-
`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` +
233-
`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(
234-
finalContent || "",
235-
)}\n</final_file_content>\n\n` +
236-
`Please note:\n` +
237-
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
238-
`2. Proceed with the task using this updated file content as the new baseline.\n` +
239-
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
240-
`${newProblemsMessage}`,
241-
)
242-
} else {
243-
pushToolResult(`The content was successfully saved to ${relPath.toPosix()}.${newProblemsMessage}`)
244-
}
224+
pushToolResult(message)
245225

246226
await cline.diffViewProvider.reset()
247227

‎src/integrations/editor/DiffViewProvider.ts‎

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,24 @@ import * as path from "path"
33
import * as fs from "fs/promises"
44
import * as diff from "diff"
55
import stripBom from "strip-bom"
6+
import { XMLBuilder } from "fast-xml-parser"
67

78
import { createDirectoriesForFile } from "../../utils/fs"
8-
import { arePathsEqual } from "../../utils/path"
9+
import { arePathsEqual, getReadablePath } from "../../utils/path"
910
import { formatResponse } from "../../core/prompts/responses"
1011
import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
12+
import { ClineSayTool } from "../../shared/ExtensionMessage"
13+
import { Task } from "../../core/task/Task"
1114

1215
import { DecorationController } from "./DecorationController"
1316

1417
export const DIFF_VIEW_URI_SCHEME = "cline-diff"
1518

1619
// TODO: https://github.com/cline/cline/pull/3354
1720
export class DiffViewProvider {
21+
// Properties to store the results of saveChanges
22+
newProblemsMessage?: string
23+
userEdits?: string
1824
editType?: "create" | "modify"
1925
isEditing = false
2026
originalContent: string | undefined
@@ -238,13 +244,91 @@ export class DiffViewProvider {
238244
normalizedEditedContent,
239245
)
240246

247+
// Store the results as class properties for formatFileWriteResponse to use
248+
this.newProblemsMessage = newProblemsMessage
249+
this.userEdits = userEdits
250+
241251
return { newProblemsMessage, userEdits, finalContent: normalizedEditedContent }
242252
} else {
243253
// No changes to Roo's edits.
254+
// Store the results as class properties for formatFileWriteResponse to use
255+
this.newProblemsMessage = newProblemsMessage
256+
this.userEdits = undefined
257+
244258
return { newProblemsMessage, userEdits: undefined, finalContent: normalizedEditedContent }
245259
}
246260
}
247261

262+
/**
263+
* Formats a standardized XML response for file write operations
264+
*
265+
* @param cwd Current working directory for path resolution
266+
* @param isNewFile Whether this is a new file or an existing file being modified
267+
* @returns Formatted message and say object for UI feedback
268+
*/
269+
async pushToolWriteResult(task: Task, cwd: string, isNewFile: boolean): Promise<string> {
270+
if (!this.relPath) {
271+
throw new Error("No file path available in DiffViewProvider")
272+
}
273+
274+
// Only send user_feedback_diff if userEdits exists
275+
if (this.userEdits) {
276+
// Create say object for UI feedback
277+
const say: ClineSayTool = {
278+
tool: isNewFile ? "newFileCreated" : "editedExistingFile",
279+
path: getReadablePath(cwd, this.relPath),
280+
diff: this.userEdits,
281+
}
282+
283+
// Send the user feedback
284+
await task.say("user_feedback_diff", JSON.stringify(say))
285+
}
286+
287+
// Build XML response
288+
const xmlObj = {
289+
file_write_result: {
290+
path: this.relPath,
291+
operation: isNewFile ? "created" : "modified",
292+
user_edits: this.userEdits ? this.userEdits : undefined,
293+
problems: this.newProblemsMessage || undefined,
294+
notice: {
295+
i: [
296+
"You do not need to re-read the file, as you have seen all changes",
297+
"Proceed with the task using these changes as the new baseline.",
298+
...(this.userEdits
299+
? [
300+
"If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.",
301+
]
302+
: []),
303+
],
304+
},
305+
},
306+
}
307+
308+
const builder = new XMLBuilder({
309+
format: true,
310+
indentBy: "",
311+
suppressEmptyNode: true,
312+
processEntities: false,
313+
tagValueProcessor: (name, value) => {
314+
if (typeof value === "string") {
315+
// Only escape <, >, and & characters
316+
return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
317+
}
318+
return value
319+
},
320+
attributeValueProcessor: (name, value) => {
321+
if (typeof value === "string") {
322+
// Only escape <, >, and & characters
323+
return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
324+
}
325+
return value
326+
},
327+
})
328+
329+
return builder.build(xmlObj)
330+
}
331+
248332
async revertChanges(): Promise<void> {
249333
if (!this.relPath || !this.activeDiffEditor) {
250334
return

0 commit comments

Comments
 (0)