Skip to content

Commit c4f8e8f

Browse files
committed
only allow single instance of old in str_replace
1 parent 0042ef7 commit c4f8e8f

File tree

3 files changed

+42
-18
lines changed

3 files changed

+42
-18
lines changed

backend/src/process-str-replace.ts

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { tryToDoStringReplacementWithExtraIndentation } from './generate-diffs-p
55

66
export async function processStrReplace(
77
path: string,
8-
replacements: { old: string; new: string }[],
8+
replacements: { old: string; new: string; allowMultiple: boolean }[],
99
initialContentPromise: Promise<string | null>,
1010
): Promise<
1111
| {
@@ -24,7 +24,7 @@ export async function processStrReplace(
2424
let allPatches: string[] = []
2525
let messages: string[] = []
2626

27-
for (const { old: oldStr, new: newStr } of replacements) {
27+
for (const { old: oldStr, new: newStr, allowMultiple } of replacements) {
2828
// Regular case: require oldStr for replacements
2929
if (!oldStr) {
3030
messages.push(
@@ -44,15 +44,19 @@ export async function processStrReplace(
4444
const normalizedCurrentContent = normalizeLineEndings(currentContent)
4545
const normalizedOldStr = normalizeLineEndings(oldStr)
4646

47-
const updatedOldStr = tryMatchOldStr(
47+
const match = tryMatchOldStr(
4848
normalizedCurrentContent,
4949
normalizedOldStr,
5050
newStr,
51+
allowMultiple,
5152
)
52-
if (updatedOldStr === null) {
53-
messages.push(
54-
`The old string ${JSON.stringify(oldStr)} was not found in the file, skipping. Please try again with a different old string that matches the file content exactly.`,
55-
)
53+
let updatedOldStr: string | null
54+
55+
if (match.success) {
56+
updatedOldStr = match.oldStr
57+
} else {
58+
messages.push(match.error)
59+
updatedOldStr = null
5660
}
5761

5862
const updatedContent =
@@ -114,9 +118,18 @@ const tryMatchOldStr = (
114118
initialContent: string,
115119
oldStr: string,
116120
newStr: string,
117-
) => {
118-
if (initialContent.includes(oldStr)) {
119-
return oldStr
121+
allowMultiple: boolean,
122+
): { success: true; oldStr: string } | { success: false; error: string } => {
123+
// count the number of occurrences of oldStr in initialContent
124+
const count = initialContent.split(oldStr).length - 1
125+
if (count === 1) {
126+
return { success: true, oldStr }
127+
}
128+
if (!allowMultiple && count > 1) {
129+
return {
130+
success: false,
131+
error: `Found ${count} occurrences of ${JSON.stringify(oldStr)} in the file. Please try again with a longer (more specified) old string or set allowMultiple to true.`,
132+
}
120133
}
121134

122135
const newChange = tryToDoStringReplacementWithExtraIndentation(
@@ -126,7 +139,7 @@ const tryMatchOldStr = (
126139
)
127140
if (newChange) {
128141
logger.debug('Matched with indentation modification')
129-
return newChange.searchContent
142+
return { success: true, oldStr: newChange.searchContent }
130143
} else {
131144
// Try matching without any whitespace as a last resort
132145
const noWhitespaceSearch = oldStr.replace(/\s+/g, '')
@@ -164,9 +177,12 @@ const tryMatchOldStr = (
164177
)
165178
if (initialContent.includes(actualContent)) {
166179
logger.debug('Matched with whitespace removed')
167-
return actualContent
180+
return { success: true, oldStr: actualContent }
168181
}
169182
}
170183
}
171-
return null
184+
return {
185+
success: false,
186+
error: `The old string ${JSON.stringify(oldStr)} was not found in the file, skipping. Please try again with a different old string that matches the file content exactly.`,
187+
}
172188
}

backend/src/tools/definitions/tool/str-replace.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,15 @@
11
import { getToolCallString } from '@codebuff/common/tools/utils'
2-
import { closeXml } from '@codebuff/common/util/xml'
32

43
import type { ToolDescription } from '../tool-def-type'
54

65
const toolName = 'str_replace'
7-
const endsAgentStep = false
86
export const strReplaceTool = {
97
toolName,
108
description: `
119
Use this tool to make edits within existing files. Prefer this tool over the write_file tool for existing files, unless you need to make major changes throughout the file, in which case use write_file.
1210
1311
Important:
14-
If you are making multiple edits in a row to a file, use only one <str_replace> call with multiple replacements instead of multiple str_replace tool calls.
15-
16-
Don't forget to close the <str_replace> tag with ${closeXml('str_replace')} after you have finished making all the replacements.
12+
If you are making multiple edits in a row to a file, use only one str_replace call with multiple replacements instead of multiple str_replace tool calls.
1713
1814
Example:
1915
${getToolCallString(toolName, {
@@ -24,6 +20,11 @@ ${getToolCallString(toolName, {
2420
old: '\n\t\t// @codebuff delete this log line please\n\t\tconsole.log("Hello, world!");\n',
2521
new: '\n',
2622
},
23+
{
24+
old: '\nfoo:',
25+
new: '\nbar:',
26+
allowMultiple: true,
27+
},
2728
],
2829
})}
2930
`.trim(),

common/src/tools/params/tool/str-replace.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ export const strReplaceParams = {
2828
.describe(
2929
`The string to replace the corresponding old string with. Can be empty to delete.`,
3030
),
31+
allowMultiple: z
32+
.boolean()
33+
.optional()
34+
.default(false)
35+
.describe(
36+
'Whether to allow multiple replacements of old string.',
37+
),
3138
})
3239
.describe('Pair of old and new strings.'),
3340
)

0 commit comments

Comments
 (0)