Skip to content

Commit 98f5542

Browse files
mrubensmrubens
authored
Revert "fix: prevent dump of an entire file into the context on user edit " (#4211)
Revert "fix: prevent dump of an entire file into the context on user edit (#…" This reverts commit 5e50c55.
1 parent dca1076 commit 98f5542

File tree

5 files changed

+109
-115
lines changed

5 files changed

+109
-115
lines changed

‎src/core/tools/applyDiffTool.ts‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ 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"
1213
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
1314
import { unescapeHtmlEntities } from "../../utils/text-normalization"
1415

@@ -162,8 +163,7 @@ export async function applyDiffTool(
162163
return
163164
}
164165

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

168168
// Track file edit operation
169169
if (relPath) {
@@ -178,13 +178,33 @@ 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-
// Get the formatted response message
182-
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
183-
184-
if (partFailHint) {
185-
pushToolResult(partFailHint + message)
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+
)
186204
} else {
187-
pushToolResult(message)
205+
pushToolResult(
206+
`Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}\n` + partFailHint,
207+
)
188208
}
189209

190210
await cline.diffViewProvider.reset()

‎src/core/tools/insertContentTool.ts‎

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

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

142141
// Track file edit operation
143142
if (relPath) {
@@ -146,14 +145,34 @@ export async function insertContentTool(
146145

147146
cline.didEditFile = true
148147

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
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),
154164
)
155165

156-
pushToolResult(message)
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+
)
157176

158177
await cline.diffViewProvider.reset()
159178
} catch (error) {

‎src/core/tools/searchAndReplaceTool.ts‎

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

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

225224
// Track file edit operation
226225
if (relPath) {
@@ -229,14 +228,34 @@ export async function searchAndReplaceTool(
229228

230229
cline.didEditFile = true
231230

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
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),
237244
)
238245

239-
pushToolResult(message)
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)
240259

241260
// Record successful tool usage and cleanup
242261
cline.recordToolUsage("search_and_replace")

‎src/core/tools/writeToFileTool.ts‎

Lines changed: 26 additions & 6 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 { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
11+
import { addLineNumbers, 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,8 +208,7 @@ export async function writeToFileTool(
208208
return
209209
}
210210

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

214213
// Track file edit operation
215214
if (relPath) {
@@ -218,10 +217,31 @@ export async function writeToFileTool(
218217

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

221-
// Get the formatted response message
222-
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
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+
)
223229

224-
pushToolResult(message)
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+
}
225245

226246
await cline.diffViewProvider.reset()
227247

‎src/integrations/editor/DiffViewProvider.ts‎

Lines changed: 1 addition & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,18 @@ 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"
76

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

1512
import { DecorationController } from "./DecorationController"
1613

1714
export const DIFF_VIEW_URI_SCHEME = "cline-diff"
1815

1916
// TODO: https://github.com/cline/cline/pull/3354
2017
export class DiffViewProvider {
21-
// Properties to store the results of saveChanges
22-
newProblemsMessage?: string
23-
userEdits?: string
2418
editType?: "create" | "modify"
2519
isEditing = false
2620
originalContent: string | undefined
@@ -244,91 +238,13 @@ export class DiffViewProvider {
244238
normalizedEditedContent,
245239
)
246240

247-
// Store the results as class properties for formatFileWriteResponse to use
248-
this.newProblemsMessage = newProblemsMessage
249-
this.userEdits = userEdits
250-
251241
return { newProblemsMessage, userEdits, finalContent: normalizedEditedContent }
252242
} else {
253243
// 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-
258244
return { newProblemsMessage, userEdits: undefined, finalContent: normalizedEditedContent }
259245
}
260246
}
261247

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-
332248
async revertChanges(): Promise<void> {
333249
if (!this.relPath || !this.activeDiffEditor) {
334250
return

0 commit comments

Comments
 (0)