Skip to content

Commit cb70c75

Browse files
roomoteroomote
committed
fix: allow partial args through parseToolCall finalization for write_to_file and other tools
When the model response gets truncated (e.g., due to output token limits), required parameters like `content` for write_to_file arrive as `undefined`. The finalization path in parseToolCall() required ALL params via AND conditions, causing nativeArgs to stay undefined and the tool call to be silently dropped or throw a generic error -- bypassing the tool's own user-friendly retry logic. The streaming/partial path already uses OR conditions to allow partial args through. This change aligns the finalization path with the same pattern, so that partially-constructed args flow through to the tool's execute() method, which can produce specific "missing parameter X, retrying..." messages that get sent back to the model. Fixes #12079
1 parent eafed97 commit cb70c75

2 files changed

Lines changed: 100 additions & 15 deletions

File tree

‎src/core/assistant-message/NativeToolCallParser.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,7 @@ export class NativeToolCallParser {
794794
break
795795

796796
case "apply_diff":
797-
if (args.path !== undefined && args.diff !== undefined) {
797+
if (args.path !== undefined || args.diff !== undefined) {
798798
nativeArgs = {
799799
path: args.path,
800800
diff: args.diff,
@@ -805,8 +805,8 @@ export class NativeToolCallParser {
805805
case "edit":
806806
case "search_and_replace":
807807
if (
808-
args.file_path !== undefined &&
809-
args.old_string !== undefined &&
808+
args.file_path !== undefined ||
809+
args.old_string !== undefined ||
810810
args.new_string !== undefined
811811
) {
812812
nativeArgs = {
@@ -819,7 +819,7 @@ export class NativeToolCallParser {
819819
break
820820

821821
case "ask_followup_question":
822-
if (args.question !== undefined && args.follow_up !== undefined) {
822+
if (args.question !== undefined || args.follow_up !== undefined) {
823823
nativeArgs = {
824824
question: args.question,
825825
follow_up: args.follow_up,
@@ -837,7 +837,7 @@ export class NativeToolCallParser {
837837
break
838838

839839
case "generate_image":
840-
if (args.prompt !== undefined && args.path !== undefined) {
840+
if (args.prompt !== undefined || args.path !== undefined) {
841841
nativeArgs = {
842842
prompt: args.prompt,
843843
path: args.path,
@@ -865,7 +865,7 @@ export class NativeToolCallParser {
865865
break
866866

867867
case "search_files":
868-
if (args.path !== undefined && args.regex !== undefined) {
868+
if (args.path !== undefined || args.regex !== undefined) {
869869
nativeArgs = {
870870
path: args.path,
871871
regex: args.regex,
@@ -875,7 +875,7 @@ export class NativeToolCallParser {
875875
break
876876

877877
case "switch_mode":
878-
if (args.mode_slug !== undefined && args.reason !== undefined) {
878+
if (args.mode_slug !== undefined || args.reason !== undefined) {
879879
nativeArgs = {
880880
mode_slug: args.mode_slug,
881881
reason: args.reason,
@@ -903,7 +903,7 @@ export class NativeToolCallParser {
903903
break
904904

905905
case "write_to_file":
906-
if (args.path !== undefined && args.content !== undefined) {
906+
if (args.path !== undefined || args.content !== undefined) {
907907
nativeArgs = {
908908
path: args.path,
909909
content: args.content,
@@ -912,7 +912,7 @@ export class NativeToolCallParser {
912912
break
913913

914914
case "use_mcp_tool":
915-
if (args.server_name !== undefined && args.tool_name !== undefined) {
915+
if (args.server_name !== undefined || args.tool_name !== undefined) {
916916
nativeArgs = {
917917
server_name: args.server_name,
918918
tool_name: args.tool_name,
@@ -922,7 +922,7 @@ export class NativeToolCallParser {
922922
break
923923

924924
case "access_mcp_resource":
925-
if (args.server_name !== undefined && args.uri !== undefined) {
925+
if (args.server_name !== undefined || args.uri !== undefined) {
926926
nativeArgs = {
927927
server_name: args.server_name,
928928
uri: args.uri,
@@ -940,8 +940,8 @@ export class NativeToolCallParser {
940940

941941
case "search_replace":
942942
if (
943-
args.file_path !== undefined &&
944-
args.old_string !== undefined &&
943+
args.file_path !== undefined ||
944+
args.old_string !== undefined ||
945945
args.new_string !== undefined
946946
) {
947947
nativeArgs = {
@@ -954,8 +954,8 @@ export class NativeToolCallParser {
954954

955955
case "edit_file":
956956
if (
957-
args.file_path !== undefined &&
958-
args.old_string !== undefined &&
957+
args.file_path !== undefined ||
958+
args.old_string !== undefined ||
959959
args.new_string !== undefined
960960
) {
961961
nativeArgs = {
@@ -977,7 +977,7 @@ export class NativeToolCallParser {
977977
break
978978

979979
case "new_task":
980-
if (args.mode !== undefined && args.message !== undefined) {
980+
if (args.mode !== undefined || args.message !== undefined) {
981981
nativeArgs = {
982982
mode: args.mode,
983983
message: args.message,

‎src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,91 @@ describe("NativeToolCallParser", () => {
313313
})
314314
})
315315

316+
describe("write_to_file tool", () => {
317+
it("should parse write_to_file with both path and content", () => {
318+
const toolCall = {
319+
id: "toolu_wtf_1",
320+
name: "write_to_file" as const,
321+
arguments: JSON.stringify({
322+
path: "src/test.ts",
323+
content: "console.log('hello')",
324+
}),
325+
}
326+
327+
const result = NativeToolCallParser.parseToolCall(toolCall)
328+
329+
expect(result).not.toBeNull()
330+
expect(result?.type).toBe("tool_use")
331+
if (result?.type === "tool_use") {
332+
const nativeArgs = result.nativeArgs as { path: string; content: string }
333+
expect(nativeArgs.path).toBe("src/test.ts")
334+
expect(nativeArgs.content).toBe("console.log('hello')")
335+
}
336+
})
337+
338+
it("should parse write_to_file with missing content (truncated response)", () => {
339+
const toolCall = {
340+
id: "toolu_wtf_2",
341+
name: "write_to_file" as const,
342+
arguments: JSON.stringify({
343+
path: "src/test.ts",
344+
}),
345+
}
346+
347+
const result = NativeToolCallParser.parseToolCall(toolCall)
348+
349+
expect(result).not.toBeNull()
350+
expect(result?.type).toBe("tool_use")
351+
if (result?.type === "tool_use") {
352+
const nativeArgs = result.nativeArgs as { path: string; content?: string }
353+
expect(nativeArgs.path).toBe("src/test.ts")
354+
expect(nativeArgs.content).toBeUndefined()
355+
}
356+
})
357+
358+
it("should parse write_to_file with missing path", () => {
359+
const toolCall = {
360+
id: "toolu_wtf_3",
361+
name: "write_to_file" as const,
362+
arguments: JSON.stringify({
363+
content: "console.log('hello')",
364+
}),
365+
}
366+
367+
const result = NativeToolCallParser.parseToolCall(toolCall)
368+
369+
expect(result).not.toBeNull()
370+
expect(result?.type).toBe("tool_use")
371+
if (result?.type === "tool_use") {
372+
const nativeArgs = result.nativeArgs as { path?: string; content: string }
373+
expect(nativeArgs.path).toBeUndefined()
374+
expect(nativeArgs.content).toBe("console.log('hello')")
375+
}
376+
})
377+
})
378+
379+
describe("apply_diff tool with missing params", () => {
380+
it("should parse apply_diff with missing diff (truncated response)", () => {
381+
const toolCall = {
382+
id: "toolu_ad_1",
383+
name: "apply_diff" as const,
384+
arguments: JSON.stringify({
385+
path: "src/test.ts",
386+
}),
387+
}
388+
389+
const result = NativeToolCallParser.parseToolCall(toolCall)
390+
391+
expect(result).not.toBeNull()
392+
expect(result?.type).toBe("tool_use")
393+
if (result?.type === "tool_use") {
394+
const nativeArgs = result.nativeArgs as { path: string; diff?: string }
395+
expect(nativeArgs.path).toBe("src/test.ts")
396+
expect(nativeArgs.diff).toBeUndefined()
397+
}
398+
})
399+
})
400+
316401
describe("finalizeStreamingToolCall", () => {
317402
describe("read_file tool", () => {
318403
it("should parse read_file args on finalize", () => {

0 commit comments

Comments
 (0)