Skip to content

Commit 0f00982

Browse files
mrubensmrubens
authored
Revert "Revert "fix: prevent dump of an entire file into the context on user edit "" (#4219)
Revert "Revert "fix: prevent dump of an entire file into the context on user …" This reverts commit 98f5542.
1 parent 7e125f8 commit 0f00982

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)