Skip to content

Commit 2a2037f

Browse files
Repair malformed tool call inputs (#578)
Co-authored-by: James Grugett <jahooma@gmail.com>
1 parent d11df24 commit 2a2037f

6 files changed

Lines changed: 391 additions & 69 deletions

File tree

common/src/tools/params/__tests__/coerce-to-array.test.ts

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import { describe, expect, it } from 'bun:test'
22
import z from 'zod/v4'
33

4-
import { coerceToArray } from '../utils'
4+
import { coerceToArray, normalizeReplacementAliases } from '../utils'
55

66
describe('coerceToArray', () => {
77
it('passes through arrays unchanged', () => {
88
expect(coerceToArray(['a', 'b'])).toEqual(['a', 'b'])
9-
expect(coerceToArray([{ old: 'x', new: 'y' }])).toEqual([{ old: 'x', new: 'y' }])
9+
expect(coerceToArray([{ old: 'x', new: 'y' }])).toEqual([
10+
{ old: 'x', new: 'y' },
11+
])
1012
expect(coerceToArray([])).toEqual([])
1113
})
1214

@@ -15,15 +17,20 @@ describe('coerceToArray', () => {
1517
})
1618

1719
it('wraps a single object in an array', () => {
18-
expect(coerceToArray({ old: 'x', new: 'y' })).toEqual([{ old: 'x', new: 'y' }])
20+
expect(coerceToArray({ old: 'x', new: 'y' })).toEqual([
21+
{ old: 'x', new: 'y' },
22+
])
1923
})
2024

2125
it('wraps a single number in an array', () => {
2226
expect(coerceToArray(42)).toEqual([42])
2327
})
2428

2529
it('parses a stringified JSON array', () => {
26-
expect(coerceToArray('["file1.ts", "file2.ts"]')).toEqual(['file1.ts', 'file2.ts'])
30+
expect(coerceToArray('["file1.ts", "file2.ts"]')).toEqual([
31+
'file1.ts',
32+
'file2.ts',
33+
])
2734
})
2835

2936
it('wraps a non-JSON string (does not parse as array)', () => {
@@ -116,3 +123,51 @@ describe('coerceToArray with Zod schemas', () => {
116123
expect(coercedSchema).toEqual(plainSchema)
117124
})
118125
})
126+
127+
describe('normalizeReplacementAliases', () => {
128+
it('maps old_str and new_str onto the documented replacement keys', () => {
129+
expect(
130+
normalizeReplacementAliases({
131+
old_str: 'before',
132+
new_str: 'after',
133+
allowMultiple: true,
134+
}),
135+
).toEqual({
136+
old_str: 'before',
137+
new_str: 'after',
138+
old: 'before',
139+
new: 'after',
140+
allowMultiple: true,
141+
})
142+
})
143+
144+
it('maps old_string and new_string onto the documented replacement keys', () => {
145+
expect(
146+
normalizeReplacementAliases({
147+
old_string: 'before',
148+
new_string: 'after',
149+
}),
150+
).toEqual({
151+
old_string: 'before',
152+
new_string: 'after',
153+
old: 'before',
154+
new: 'after',
155+
})
156+
})
157+
158+
it('does not overwrite documented replacement keys', () => {
159+
expect(
160+
normalizeReplacementAliases({
161+
old: 'before',
162+
new: 'after',
163+
old_str: 'ignored',
164+
new_str: 'ignored',
165+
}),
166+
).toEqual({
167+
old: 'before',
168+
new: 'after',
169+
old_str: 'ignored',
170+
new_str: 'ignored',
171+
})
172+
})
173+
})

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

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import z from 'zod/v4'
22

3-
import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils'
3+
import {
4+
$getNativeToolCallExampleString,
5+
coerceToArray,
6+
jsonToolResultSchema,
7+
normalizeReplacementAliases,
8+
} from '../utils'
49

510
import type { $ToolParams } from '../../constants'
611

@@ -30,33 +35,38 @@ const inputSchema = z
3035
z
3136
.array(
3237
z
33-
.object({
34-
old: z
35-
.string()
36-
.min(1, 'Old cannot be empty')
37-
.describe(
38-
`The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`,
39-
),
40-
new: z
41-
.string()
42-
.describe(
43-
`The string to replace the corresponding old string with. Can be empty to delete.`,
44-
),
45-
allowMultiple: z
46-
.boolean()
47-
.optional()
48-
.default(false)
49-
.describe(
50-
'Whether to allow multiple replacements of old string.',
51-
),
52-
})
38+
.preprocess(
39+
normalizeReplacementAliases,
40+
z.object({
41+
old: z
42+
.string()
43+
.min(1, 'Old cannot be empty')
44+
.describe(
45+
`The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`,
46+
),
47+
new: z
48+
.string()
49+
.describe(
50+
`The string to replace the corresponding old string with. Can be empty to delete.`,
51+
),
52+
allowMultiple: z
53+
.boolean()
54+
.optional()
55+
.default(false)
56+
.describe(
57+
'Whether to allow multiple replacements of old string.',
58+
),
59+
}),
60+
)
5361
.describe('Pair of old and new strings.'),
5462
)
5563
.min(1, 'Replacements cannot be empty'),
5664
)
5765
.describe('Array of replacements to make.'),
5866
})
59-
.describe(`Propose string replacements in a file without actually applying them.`)
67+
.describe(
68+
`Propose string replacements in a file without actually applying them.`,
69+
)
6070
const description = `
6171
Propose edits to a file without actually applying them. Use this tool when you want to draft changes that will be reviewed before being applied.
6272

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

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import z from 'zod/v4'
22

3-
import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils'
3+
import {
4+
$getNativeToolCallExampleString,
5+
coerceToArray,
6+
jsonToolResultSchema,
7+
normalizeReplacementAliases,
8+
} from '../utils'
49

510
import type { $ToolParams } from '../../constants'
611

@@ -31,26 +36,29 @@ const inputSchema = z
3136
z
3237
.array(
3338
z
34-
.object({
35-
old: z
36-
.string()
37-
.min(1, 'Old cannot be empty')
38-
.describe(
39-
`The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`,
40-
),
41-
new: z
42-
.string()
43-
.describe(
44-
`The string to replace the corresponding old string with. Can be empty to delete.`,
45-
),
46-
allowMultiple: z
47-
.boolean()
48-
.optional()
49-
.default(false)
50-
.describe(
51-
'Whether to allow multiple replacements of old string.',
52-
),
53-
})
39+
.preprocess(
40+
normalizeReplacementAliases,
41+
z.object({
42+
old: z
43+
.string()
44+
.min(1, 'Old cannot be empty')
45+
.describe(
46+
`The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`,
47+
),
48+
new: z
49+
.string()
50+
.describe(
51+
`The string to replace the corresponding old string with. Can be empty to delete.`,
52+
),
53+
allowMultiple: z
54+
.boolean()
55+
.optional()
56+
.default(false)
57+
.describe(
58+
'Whether to allow multiple replacements of old string.',
59+
),
60+
}),
61+
)
5462
.describe('Pair of old and new strings.'),
5563
)
5664
.min(1, 'Replacements cannot be empty'),

common/src/tools/params/utils.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,31 @@ export function coerceToArray(val: unknown): unknown {
3232
return val
3333
}
3434

35+
/**
36+
* Handles common replacement-key aliases emitted by some models while keeping
37+
* the documented schema stable.
38+
*/
39+
export function normalizeReplacementAliases(val: unknown): unknown {
40+
if (val === null || typeof val !== 'object' || Array.isArray(val)) {
41+
return val
42+
}
43+
44+
const replacement = { ...(val as Record<string, unknown>) }
45+
for (const [target, aliases] of [
46+
['old', ['old_str', 'old_string']],
47+
['new', ['new_str', 'new_string']],
48+
] as const) {
49+
if (replacement[target] !== undefined) {
50+
continue
51+
}
52+
const alias = aliases.find((key) => typeof replacement[key] === 'string')
53+
if (alias) {
54+
replacement[target] = replacement[alias]
55+
}
56+
}
57+
return replacement
58+
}
59+
3560
/** Only used for generating tool call strings before all tools are defined.
3661
*
3762
* @param toolName - The name of the tool to call

0 commit comments

Comments
 (0)