mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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 <roo-code@z.ewheeler.org> * 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 <roo-code@z.ewheeler.org> --------- Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org> Co-authored-by: Eric Wheeler <roo-code@z.ewheeler.org>
This commit is contained in:
KJ7LNW
GitHub
parent
e40a4c96ed
commit
5e50c5543b
5 changed files with 114 additions and 108 deletions
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
|
|
@ -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, "&").replace(/</g, "<").replace(/>/g, ">")
|
||||
}
|
||||
return value
|
||||
},
|
||||
attributeValueProcessor: (name, value) => {
|
||||
if (typeof value === "string") {
|
||||
// Only escape <, >, and & characters
|
||||
return value.replace(/&/g, "&").replace(/</g, "<").replace(/>/g, ">")
|
||||
}
|
||||
return value
|
||||
},
|
||||
})
|
||||
|
||||
return builder.build(xmlObj)
|
||||
}
|
||||
|
||||
async revertChanges(): Promise<void> {
|
||||
if (!this.relPath || !this.activeDiffEditor) {
|
||||
return
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue