summaryrefslogtreecommitdiff
path: root/packages/core/src
diff options
context:
space:
mode:
authorLeo <[email protected]>2025-06-28 19:02:44 +0100
committerGitHub <[email protected]>2025-06-28 18:02:44 +0000
commit601d9ba36d1c8f6d1e180a0c5cc1d5597906d33a (patch)
tree4f34dfee0c6391a321b5a259957854ae2743cb3f /packages/core/src
parent3518ff766345ba98d90495b46c84439e6fc1a61c (diff)
fix edit retrigger (#2306)
Diffstat (limited to 'packages/core/src')
-rw-r--r--packages/core/src/tools/edit.test.ts59
-rw-r--r--packages/core/src/tools/edit.ts23
-rw-r--r--packages/core/src/tools/write-file.test.ts44
-rw-r--r--packages/core/src/tools/write-file.ts25
4 files changed, 142 insertions, 9 deletions
diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts
index 50f68e2a..9143f3bd 100644
--- a/packages/core/src/tools/edit.test.ts
+++ b/packages/core/src/tools/edit.test.ts
@@ -549,6 +549,65 @@ describe('EditTool', () => {
/Attempted to create a file that already exists/,
);
});
+
+ it('should include modification message when proposed content is modified', async () => {
+ const initialContent = 'This is some old text.';
+ fs.writeFileSync(filePath, initialContent, 'utf8');
+ const params: EditToolParams = {
+ file_path: filePath,
+ old_string: 'old',
+ new_string: 'new',
+ modified_by_user: true,
+ };
+
+ (mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
+ ApprovalMode.AUTO_EDIT,
+ );
+ const result = await tool.execute(params, new AbortController().signal);
+
+ expect(result.llmContent).toMatch(
+ /User modified the `new_string` content/,
+ );
+ });
+
+ it('should not include modification message when proposed content is not modified', async () => {
+ const initialContent = 'This is some old text.';
+ fs.writeFileSync(filePath, initialContent, 'utf8');
+ const params: EditToolParams = {
+ file_path: filePath,
+ old_string: 'old',
+ new_string: 'new',
+ modified_by_user: false,
+ };
+
+ (mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
+ ApprovalMode.AUTO_EDIT,
+ );
+ const result = await tool.execute(params, new AbortController().signal);
+
+ expect(result.llmContent).not.toMatch(
+ /User modified the `new_string` content/,
+ );
+ });
+
+ it('should not include modification message when modified_by_user is not provided', async () => {
+ const initialContent = 'This is some old text.';
+ fs.writeFileSync(filePath, initialContent, 'utf8');
+ const params: EditToolParams = {
+ file_path: filePath,
+ old_string: 'old',
+ new_string: 'new',
+ };
+
+ (mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
+ ApprovalMode.AUTO_EDIT,
+ );
+ const result = await tool.execute(params, new AbortController().signal);
+
+ expect(result.llmContent).not.toMatch(
+ /User modified the `new_string` content/,
+ );
+ });
});
describe('getDescription', () => {
diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts
index ddb521be..2515261d 100644
--- a/packages/core/src/tools/edit.ts
+++ b/packages/core/src/tools/edit.ts
@@ -49,6 +49,11 @@ export interface EditToolParams {
* Use when you want to replace multiple occurrences.
*/
expected_replacements?: number;
+
+ /**
+ * Whether the edit was modified manually by the user.
+ */
+ modified_by_user?: boolean;
}
interface CalculatedEdit {
@@ -81,6 +86,8 @@ export class EditTool
'Edit',
`Replaces text within a file. By default, replaces a single occurrence, but can replace multiple occurrences when \`expected_replacements\` is specified. This tool requires providing significant context around the change to ensure precise targeting. Always use the ${ReadFileTool.Name} tool to examine the file's current content before attempting a text replacement.
+ The user has the ability to modify the \`new_string\` content. If modified, this will be stated in the response.
+
Expectation for required parameters:
1. \`file_path\` MUST be an absolute path; otherwise an error will be thrown.
2. \`old_string\` MUST be the exact literal text to replace (including all whitespace, indentation, newlines, and surrounding code etc.).
@@ -414,12 +421,19 @@ Expectation for required parameters:
displayResult = { fileDiff, fileName };
}
- const llmSuccessMessage = editData.isNewFile
- ? `Created new file: ${params.file_path} with provided content.`
- : `Successfully modified file: ${params.file_path} (${editData.occurrences} replacements).`;
+ const llmSuccessMessageParts = [
+ editData.isNewFile
+ ? `Created new file: ${params.file_path} with provided content.`
+ : `Successfully modified file: ${params.file_path} (${editData.occurrences} replacements).`,
+ ];
+ if (params.modified_by_user) {
+ llmSuccessMessageParts.push(
+ `User modified the \`new_string\` content to be: ${params.new_string}.`,
+ );
+ }
return {
- llmContent: llmSuccessMessage,
+ llmContent: llmSuccessMessageParts.join(' '),
returnDisplay: displayResult,
};
} catch (error) {
@@ -474,6 +488,7 @@ Expectation for required parameters:
...originalParams,
old_string: oldContent,
new_string: modifiedProposedContent,
+ modified_by_user: true,
}),
};
}
diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts
index 4646f30a..3f9825c6 100644
--- a/packages/core/src/tools/write-file.test.ts
+++ b/packages/core/src/tools/write-file.test.ts
@@ -567,5 +567,49 @@ describe('WriteFileTool', () => {
expect(fs.existsSync(filePath)).toBe(true);
expect(fs.readFileSync(filePath, 'utf8')).toBe(content);
});
+
+ it('should include modification message when proposed content is modified', async () => {
+ const filePath = path.join(rootDir, 'new_file_modified.txt');
+ const content = 'New file content modified by user';
+ mockEnsureCorrectFileContent.mockResolvedValue(content);
+
+ const params = {
+ file_path: filePath,
+ content,
+ modified_by_user: true,
+ };
+ const result = await tool.execute(params, abortSignal);
+
+ expect(result.llmContent).toMatch(/User modified the `content`/);
+ });
+
+ it('should not include modification message when proposed content is not modified', async () => {
+ const filePath = path.join(rootDir, 'new_file_unmodified.txt');
+ const content = 'New file content not modified';
+ mockEnsureCorrectFileContent.mockResolvedValue(content);
+
+ const params = {
+ file_path: filePath,
+ content,
+ modified_by_user: false,
+ };
+ const result = await tool.execute(params, abortSignal);
+
+ expect(result.llmContent).not.toMatch(/User modified the `content`/);
+ });
+
+ it('should not include modification message when modified_by_user is not provided', async () => {
+ const filePath = path.join(rootDir, 'new_file_unmodified.txt');
+ const content = 'New file content not modified';
+ mockEnsureCorrectFileContent.mockResolvedValue(content);
+
+ const params = {
+ file_path: filePath,
+ content,
+ };
+ const result = await tool.execute(params, abortSignal);
+
+ expect(result.llmContent).not.toMatch(/User modified the `content`/);
+ });
});
});
diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts
index 87f6e21e..2d5e85be 100644
--- a/packages/core/src/tools/write-file.ts
+++ b/packages/core/src/tools/write-file.ts
@@ -45,6 +45,11 @@ export interface WriteFileToolParams {
* The content to write to the file
*/
content: string;
+
+ /**
+ * Whether the proposed content was modified by the user.
+ */
+ modified_by_user?: boolean;
}
interface GetCorrectedFileContentResult {
@@ -68,7 +73,9 @@ export class WriteFileTool
super(
WriteFileTool.Name,
'WriteFile',
- 'Writes content to a specified file in the local filesystem.',
+ `Writes content to a specified file in the local filesystem.
+
+ The user has the ability to modify \`content\`. If modified, this will be stated in the response.`,
{
properties: {
file_path: {
@@ -270,9 +277,16 @@ export class WriteFileTool
DEFAULT_DIFF_OPTIONS,
);
- const llmSuccessMessage = isNewFile
- ? `Successfully created and wrote to new file: ${params.file_path}`
- : `Successfully overwrote file: ${params.file_path}`;
+ const llmSuccessMessageParts = [
+ isNewFile
+ ? `Successfully created and wrote to new file: ${params.file_path}.`
+ : `Successfully overwrote file: ${params.file_path}.`,
+ ];
+ if (params.modified_by_user) {
+ llmSuccessMessageParts.push(
+ `User modified the \`content\` to be: ${params.content}`,
+ );
+ }
const displayResult: FileDiff = { fileDiff, fileName };
@@ -298,7 +312,7 @@ export class WriteFileTool
}
return {
- llmContent: llmSuccessMessage,
+ llmContent: llmSuccessMessageParts.join(' '),
returnDisplay: displayResult,
};
} catch (error) {
@@ -395,6 +409,7 @@ export class WriteFileTool
) => ({
...originalParams,
content: modifiedProposedContent,
+ modified_by_user: true,
}),
};
}