Skip to content

Commit 200cbdf

Browse files
committed
Handle some incorrect tool call schemas
1 parent 1ca6b47 commit 200cbdf

File tree

10 files changed

+261
-92
lines changed

10 files changed

+261
-92
lines changed
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { describe, expect, it } from 'bun:test'
2+
import z from 'zod/v4'
3+
4+
import { coerceToArray } from '../utils'
5+
6+
describe('coerceToArray', () => {
7+
it('passes through arrays unchanged', () => {
8+
expect(coerceToArray(['a', 'b'])).toEqual(['a', 'b'])
9+
expect(coerceToArray([{ old: 'x', new: 'y' }])).toEqual([{ old: 'x', new: 'y' }])
10+
expect(coerceToArray([])).toEqual([])
11+
})
12+
13+
it('wraps a single string in an array', () => {
14+
expect(coerceToArray('file.ts')).toEqual(['file.ts'])
15+
})
16+
17+
it('wraps a single object in an array', () => {
18+
expect(coerceToArray({ old: 'x', new: 'y' })).toEqual([{ old: 'x', new: 'y' }])
19+
})
20+
21+
it('wraps a single number in an array', () => {
22+
expect(coerceToArray(42)).toEqual([42])
23+
})
24+
25+
it('parses a stringified JSON array', () => {
26+
expect(coerceToArray('["file1.ts", "file2.ts"]')).toEqual(['file1.ts', 'file2.ts'])
27+
})
28+
29+
it('wraps a non-JSON string (does not parse as array)', () => {
30+
expect(coerceToArray('not-json')).toEqual(['not-json'])
31+
})
32+
33+
it('wraps a stringified JSON object (not an array) in an array', () => {
34+
expect(coerceToArray('{"key": "value"}')).toEqual(['{"key": "value"}'])
35+
})
36+
37+
it('passes through null', () => {
38+
expect(coerceToArray(null)).toBeNull()
39+
})
40+
41+
it('passes through undefined', () => {
42+
expect(coerceToArray(undefined)).toBeUndefined()
43+
})
44+
})
45+
46+
describe('coerceToArray with Zod schemas', () => {
47+
it('coerces a single string into an array for z.array(z.string())', () => {
48+
const schema = z.object({
49+
paths: z.preprocess(coerceToArray, z.array(z.string())),
50+
})
51+
const result = schema.safeParse({ paths: 'file.ts' })
52+
expect(result.success).toBe(true)
53+
if (result.success) {
54+
expect(result.data.paths).toEqual(['file.ts'])
55+
}
56+
})
57+
58+
it('coerces a single object into an array for z.array(z.object(...))', () => {
59+
const schema = z.object({
60+
replacements: z.preprocess(
61+
coerceToArray,
62+
z.array(z.object({ old: z.string(), new: z.string() })),
63+
),
64+
})
65+
const result = schema.safeParse({ replacements: { old: 'x', new: 'y' } })
66+
expect(result.success).toBe(true)
67+
if (result.success) {
68+
expect(result.data.replacements).toEqual([{ old: 'x', new: 'y' }])
69+
}
70+
})
71+
72+
it('still validates correctly when already an array', () => {
73+
const schema = z.object({
74+
paths: z.preprocess(coerceToArray, z.array(z.string())),
75+
})
76+
const result = schema.safeParse({ paths: ['a.ts', 'b.ts'] })
77+
expect(result.success).toBe(true)
78+
if (result.success) {
79+
expect(result.data.paths).toEqual(['a.ts', 'b.ts'])
80+
}
81+
})
82+
83+
it('still rejects invalid inner types after coercion', () => {
84+
const schema = z.object({
85+
paths: z.preprocess(coerceToArray, z.array(z.string())),
86+
})
87+
const result = schema.safeParse({ paths: 123 })
88+
expect(result.success).toBe(false)
89+
})
90+
91+
it('works with optional arrays', () => {
92+
const schema = z.object({
93+
paths: z.preprocess(coerceToArray, z.array(z.string())).optional(),
94+
})
95+
const withValue = schema.safeParse({ paths: 'file.ts' })
96+
expect(withValue.success).toBe(true)
97+
if (withValue.success) {
98+
expect(withValue.data.paths).toEqual(['file.ts'])
99+
}
100+
101+
const withoutValue = schema.safeParse({})
102+
expect(withoutValue.success).toBe(true)
103+
if (withoutValue.success) {
104+
expect(withoutValue.data.paths).toBeUndefined()
105+
}
106+
})
107+
108+
it('produces identical JSON schema with or without preprocess', () => {
109+
const plain = z.object({ paths: z.array(z.string()) })
110+
const coerced = z.object({
111+
paths: z.preprocess(coerceToArray, z.array(z.string())),
112+
})
113+
114+
const plainSchema = z.toJSONSchema(plain, { io: 'input' })
115+
const coercedSchema = z.toJSONSchema(coerced, { io: 'input' })
116+
expect(coercedSchema).toEqual(plainSchema)
117+
})
118+
})

common/src/tools/params/tool/ask-user.ts

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

3-
import { $getNativeToolCallExampleString, jsonToolResultSchema } from '../utils'
3+
import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils'
44

55
import type { $ToolParams } from '../../constants'
66

@@ -15,17 +15,21 @@ export const questionSchema = z.object({
1515
'Short label (max 12 chars) displayed as a chip/tag. Example: "Auth method"',
1616
),
1717
options: z
18-
.object({
19-
label: z.string().describe('The display text for this option'),
20-
description: z
21-
.string()
22-
.optional()
23-
.describe('Explanation shown when option is focused'),
24-
})
25-
.array()
26-
.refine((opts) => opts.length >= 2, {
27-
message: 'Each question must have at least 2 options',
28-
})
18+
.preprocess(
19+
coerceToArray,
20+
z
21+
.object({
22+
label: z.string().describe('The display text for this option'),
23+
description: z
24+
.string()
25+
.optional()
26+
.describe('Explanation shown when option is focused'),
27+
})
28+
.array()
29+
.refine((opts) => opts.length >= 2, {
30+
message: 'Each question must have at least 2 options',
31+
}),
32+
)
2933
.describe('Array of answer options with label and optional description.'),
3034

3135
multiSelect: z
@@ -64,8 +68,12 @@ const endsAgentStep = true
6468
const inputSchema = z
6569
.object({
6670
questions: z
67-
.array(questionSchema)
68-
.min(1, 'Must provide at least one question')
71+
.preprocess(
72+
coerceToArray,
73+
z
74+
.array(questionSchema)
75+
.min(1, 'Must provide at least one question'),
76+
)
6977
.describe('List of multiple choice questions to ask the user'),
7078
})
7179
.describe(

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

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

3-
import { $getNativeToolCallExampleString, jsonToolResultSchema } from '../utils'
3+
import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils'
44

55
import type { $ToolParams } from '../../constants'
66

@@ -25,31 +25,35 @@ const inputSchema = z
2525
.min(1, 'Path cannot be empty')
2626
.describe(`The path to the file to edit.`),
2727
replacements: z
28-
.array(
28+
.preprocess(
29+
coerceToArray,
2930
z
30-
.object({
31-
old: z
32-
.string()
33-
.min(1, 'Old cannot be empty')
34-
.describe(
35-
`The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`,
36-
),
37-
new: z
38-
.string()
39-
.describe(
40-
`The string to replace the corresponding old string with. Can be empty to delete.`,
41-
),
42-
allowMultiple: z
43-
.boolean()
44-
.optional()
45-
.default(false)
46-
.describe(
47-
'Whether to allow multiple replacements of old string.',
48-
),
49-
})
50-
.describe('Pair of old and new strings.'),
31+
.array(
32+
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+
})
53+
.describe('Pair of old and new strings.'),
54+
)
55+
.min(1, 'Replacements cannot be empty'),
5156
)
52-
.min(1, 'Replacements cannot be empty')
5357
.describe('Array of replacements to make.'),
5458
})
5559
.describe(`Propose string replacements in a file without actually applying them.`)

common/src/tools/params/tool/read-files.ts

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

3-
import { $getNativeToolCallExampleString, jsonToolResultSchema } from '../utils'
3+
import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils'
44

55
import type { $ToolParams } from '../../constants'
66

@@ -21,13 +21,16 @@ const endsAgentStep = true
2121
const inputSchema = z
2222
.object({
2323
paths: z
24-
.array(
25-
z
26-
.string()
27-
.min(1, 'Paths cannot be empty')
28-
.describe(
29-
`File path to read relative to the **project root**. Absolute file paths will not work.`,
30-
),
24+
.preprocess(
25+
coerceToArray,
26+
z.array(
27+
z
28+
.string()
29+
.min(1, 'Paths cannot be empty')
30+
.describe(
31+
`File path to read relative to the **project root**. Absolute file paths will not work.`,
32+
),
33+
),
3134
)
3235
.describe('List of file paths to read.'),
3336
})

common/src/tools/params/tool/read-subtree.ts

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

3-
import { $getNativeToolCallExampleString, jsonToolResultSchema } from '../utils'
3+
import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils'
44

55
import type { $ToolParams } from '../../constants'
66

@@ -9,7 +9,7 @@ const endsAgentStep = true
99
const inputSchema = z
1010
.object({
1111
paths: z
12-
.array(z.string())
12+
.preprocess(coerceToArray, z.array(z.string()))
1313
.optional()
1414
.describe(
1515
`List of paths to directories or files. Relative to the project root. If omitted, the entire project tree is used.`,

common/src/tools/params/tool/spawn-agents.ts

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

33
import { jsonObjectSchema } from '../../../types/json'
4-
import { $getNativeToolCallExampleString, jsonToolResultSchema } from '../utils'
4+
import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils'
55

66
import type { $ToolParams } from '../../constants'
77

@@ -16,16 +16,19 @@ const toolName = 'spawn_agents'
1616
const endsAgentStep = true
1717
const inputSchema = z
1818
.object({
19-
agents: z
20-
.object({
21-
agent_type: z.string().describe('Agent to spawn'),
22-
prompt: z.string().optional().describe('Prompt to send to the agent'),
23-
params: z
24-
.record(z.string(), z.any())
25-
.optional()
26-
.describe('Parameters object for the agent (if any)'),
27-
})
28-
.array(),
19+
agents: z.preprocess(
20+
coerceToArray,
21+
z
22+
.object({
23+
agent_type: z.string().describe('Agent to spawn'),
24+
prompt: z.string().optional().describe('Prompt to send to the agent'),
25+
params: z
26+
.record(z.string(), z.any())
27+
.optional()
28+
.describe('Parameters object for the agent (if any)'),
29+
})
30+
.array(),
31+
),
2932
})
3033
.describe(
3134
`Spawn multiple agents and send a prompt and/or parameters to each of them. These agents will run in parallel. Note that that means they will run independently. If you need to run agents sequentially, use spawn_agents with one agent at a time instead.`,

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

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

3-
import { $getNativeToolCallExampleString, jsonToolResultSchema } from '../utils'
3+
import { $getNativeToolCallExampleString, coerceToArray, jsonToolResultSchema } from '../utils'
44

55
import type { $ToolParams } from '../../constants'
66

@@ -26,31 +26,35 @@ const inputSchema = z
2626
.min(1, 'Path cannot be empty')
2727
.describe(`The path to the file to edit.`),
2828
replacements: z
29-
.array(
29+
.preprocess(
30+
coerceToArray,
3031
z
31-
.object({
32-
old: z
33-
.string()
34-
.min(1, 'Old cannot be empty')
35-
.describe(
36-
`The string to replace. This must be an *exact match* of the string you want to replace, including whitespace and punctuation.`,
37-
),
38-
new: z
39-
.string()
40-
.describe(
41-
`The string to replace the corresponding old string with. Can be empty to delete.`,
42-
),
43-
allowMultiple: z
44-
.boolean()
45-
.optional()
46-
.default(false)
47-
.describe(
48-
'Whether to allow multiple replacements of old string.',
49-
),
50-
})
51-
.describe('Pair of old and new strings.'),
32+
.array(
33+
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+
})
54+
.describe('Pair of old and new strings.'),
55+
)
56+
.min(1, 'Replacements cannot be empty'),
5257
)
53-
.min(1, 'Replacements cannot be empty')
5458
.describe('Array of replacements to make.'),
5559
})
5660
.describe(`Replace strings in a file with new strings.`)

0 commit comments

Comments
 (0)