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 98f5542021.
This commit is contained in:
Matt Rubens 2025-06-01 22:05:20 -04:00 committed by GitHub
parent 7e125f86c6
commit 0f00982a2b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 114 additions and 108 deletions

View file

@ -9,7 +9,6 @@ import { Task } from "../task/Task"
import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } from "../../shared/tools"
import { formatResponse } from "../prompts/responses"
import { fileExistsAtPath } from "../../utils/fs"
import { addLineNumbers } from "../../integrations/misc/extract-text"
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
import { unescapeHtmlEntities } from "../../utils/text-normalization"
@ -163,7 +162,8 @@ export async function applyDiffTool(
return
}
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
// Call saveChanges to update the DiffViewProvider properties
await cline.diffViewProvider.saveChanges()
// Track file edit operation
if (relPath) {
@ -178,33 +178,13 @@ export async function applyDiffTool(
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`
}
if (userEdits) {
await cline.say(
"user_feedback_diff",
JSON.stringify({
tool: fileExists ? "editedExistingFile" : "newFileCreated",
path: getReadablePath(cline.cwd, relPath),
diff: userEdits,
} satisfies ClineSayTool),
)
// Get the formatted response message
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
pushToolResult(
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
partFailHint +
`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` +
`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(
finalContent || "",
)}\n</final_file_content>\n\n` +
`Please note:\n` +
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
`2. Proceed with the task using this updated file content as the new baseline.\n` +
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
`${newProblemsMessage}`,
)
if (partFailHint) {
pushToolResult(partFailHint + message)
} else {
pushToolResult(
`Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}\n` + partFailHint,
)
pushToolResult(message)
}
await cline.diffViewProvider.reset()

View file

@ -136,7 +136,8 @@ export async function insertContentTool(
return
}
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
// Call saveChanges to update the DiffViewProvider properties
await cline.diffViewProvider.saveChanges()
// Track file edit operation
if (relPath) {
@ -145,34 +146,14 @@ export async function insertContentTool(
cline.didEditFile = true
if (!userEdits) {
pushToolResult(
`The content was successfully inserted in ${relPath.toPosix()} at line ${lineNumber}.${newProblemsMessage}`,
)
await cline.diffViewProvider.reset()
return
}
await cline.say(
"user_feedback_diff",
JSON.stringify({
tool: "insertContent",
path: getReadablePath(cline.cwd, relPath),
diff: userEdits,
lineNumber: lineNumber,
} satisfies ClineSayTool),
// Get the formatted response message
const message = await cline.diffViewProvider.pushToolWriteResult(
cline,
cline.cwd,
false, // Always false for insert_content
)
pushToolResult(
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
`The updated content has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file:\n\n` +
`<final_file_content path="${relPath.toPosix()}">\n${finalContent}\n</final_file_content>\n\n` +
`Please note:\n` +
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
`2. Proceed with the task using this updated file content as the new baseline.\n` +
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
`${newProblemsMessage}`,
)
pushToolResult(message)
await cline.diffViewProvider.reset()
} catch (error) {

View file

@ -219,7 +219,8 @@ export async function searchAndReplaceTool(
return
}
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
// Call saveChanges to update the DiffViewProvider properties
await cline.diffViewProvider.saveChanges()
// Track file edit operation
if (relPath) {
@ -228,34 +229,14 @@ export async function searchAndReplaceTool(
cline.didEditFile = true
if (!userEdits) {
pushToolResult(`The content was successfully replaced in ${relPath}.${newProblemsMessage}`)
await cline.diffViewProvider.reset()
return
}
await cline.say(
"user_feedback_diff",
JSON.stringify({
tool: "appliedDiff",
path: getReadablePath(cline.cwd, relPath),
diff: userEdits,
} satisfies ClineSayTool),
// Get the formatted response message
const message = await cline.diffViewProvider.pushToolWriteResult(
cline,
cline.cwd,
false, // Always false for search_and_replace
)
// Format and send response with user's updates
const resultMessage = [
`The user made the following updates to your content:\n\n${userEdits}\n\n`,
`The updated content has been successfully saved to ${validRelPath.toPosix()}. Here is the full, updated content of the file:\n\n`,
`<final_file_content path="${validRelPath.toPosix()}">\n${finalContent}\n</final_file_content>\n\n`,
`Please note:\n`,
`1. You do not need to re-write the file with these changes, as they have already been applied.\n`,
`2. Proceed with the task using the updated file content as the new baseline.\n`,
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.`,
newProblemsMessage,
].join("")
pushToolResult(resultMessage)
pushToolResult(message)
// Record successful tool usage and cleanup
cline.recordToolUsage("search_and_replace")

View file

@ -8,7 +8,7 @@ import { formatResponse } from "../prompts/responses"
import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools"
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
import { fileExistsAtPath } from "../../utils/fs"
import { addLineNumbers, stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
import { getReadablePath } from "../../utils/path"
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
import { detectCodeOmission } from "../../integrations/editor/detect-omission"
@ -208,7 +208,8 @@ export async function writeToFileTool(
return
}
const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges()
// Call saveChanges to update the DiffViewProvider properties
await cline.diffViewProvider.saveChanges()
// Track file edit operation
if (relPath) {
@ -217,31 +218,10 @@ export async function writeToFileTool(
cline.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request
if (userEdits) {
await cline.say(
"user_feedback_diff",
JSON.stringify({
tool: fileExists ? "editedExistingFile" : "newFileCreated",
path: getReadablePath(cline.cwd, relPath),
diff: userEdits,
} satisfies ClineSayTool),
)
// Get the formatted response message
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
pushToolResult(
`The user made the following updates to your content:\n\n${userEdits}\n\n` +
`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` +
`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(
finalContent || "",
)}\n</final_file_content>\n\n` +
`Please note:\n` +
`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
`2. Proceed with the task using this updated file content as the new baseline.\n` +
`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
`${newProblemsMessage}`,
)
} else {
pushToolResult(`The content was successfully saved to ${relPath.toPosix()}.${newProblemsMessage}`)
}
pushToolResult(message)
await cline.diffViewProvider.reset()

View file

@ -3,11 +3,14 @@ import * as path from "path"
import * as fs from "fs/promises"
import * as diff from "diff"
import stripBom from "strip-bom"
import { XMLBuilder } from "fast-xml-parser"
import { createDirectoriesForFile } from "../../utils/fs"
import { arePathsEqual } from "../../utils/path"
import { arePathsEqual, getReadablePath } from "../../utils/path"
import { formatResponse } from "../../core/prompts/responses"
import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
import { ClineSayTool } from "../../shared/ExtensionMessage"
import { Task } from "../../core/task/Task"
import { DecorationController } from "./DecorationController"
@ -15,6 +18,9 @@ export const DIFF_VIEW_URI_SCHEME = "cline-diff"
// TODO: https://github.com/cline/cline/pull/3354
export class DiffViewProvider {
// Properties to store the results of saveChanges
newProblemsMessage?: string
userEdits?: string
editType?: "create" | "modify"
isEditing = false
originalContent: string | undefined
@ -238,13 +244,91 @@ export class DiffViewProvider {
normalizedEditedContent,
)
// Store the results as class properties for formatFileWriteResponse to use
this.newProblemsMessage = newProblemsMessage
this.userEdits = userEdits
return { newProblemsMessage, userEdits, finalContent: normalizedEditedContent }
} else {
// No changes to Roo's edits.
// Store the results as class properties for formatFileWriteResponse to use
this.newProblemsMessage = newProblemsMessage
this.userEdits = undefined
return { newProblemsMessage, userEdits: undefined, finalContent: normalizedEditedContent }
}
}
/**
* Formats a standardized XML response for file write operations
*
* @param cwd Current working directory for path resolution
* @param isNewFile Whether this is a new file or an existing file being modified
* @returns Formatted message and say object for UI feedback
*/
async pushToolWriteResult(task: Task, cwd: string, isNewFile: boolean): Promise<string> {
if (!this.relPath) {
throw new Error("No file path available in DiffViewProvider")
}
// Only send user_feedback_diff if userEdits exists
if (this.userEdits) {
// Create say object for UI feedback
const say: ClineSayTool = {
tool: isNewFile ? "newFileCreated" : "editedExistingFile",
path: getReadablePath(cwd, this.relPath),
diff: this.userEdits,
}
// Send the user feedback
await task.say("user_feedback_diff", JSON.stringify(say))
}
// Build XML response
const xmlObj = {
file_write_result: {
path: this.relPath,
operation: isNewFile ? "created" : "modified",
user_edits: this.userEdits ? this.userEdits : undefined,
problems: this.newProblemsMessage || undefined,
notice: {
i: [
"You do not need to re-read the file, as you have seen all changes",
"Proceed with the task using these changes as the new baseline.",
...(this.userEdits
? [
"If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.",
]
: []),
],
},
},
}
const builder = new XMLBuilder({
format: true,
indentBy: "",
suppressEmptyNode: true,
processEntities: false,
tagValueProcessor: (name, value) => {
if (typeof value === "string") {
// Only escape <, >, and & characters
return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
}
return value
},
attributeValueProcessor: (name, value) => {
if (typeof value === "string") {
// Only escape <, >, and & characters
return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
}
return value
},
})
return builder.build(xmlObj)
}
async revertChanges(): Promise<void> {
if (!this.relPath || !this.activeDiffEditor) {
return