Skip to content

Commit 429fb9f

Browse files
committed
feat(error-interception): add INVALID_JSON_ARGUMENTS pattern for concatenated JSON objects
When AI models concatenate multiple JSON objects into a single tool call's arguments string (e.g., {path:file1}{path:file2}), JSON.parse throws and the tool call is silently discarded. The model receives no feedback and repeats the mistake. This commit adds: - INVALID_JSON_ARGUMENTS error category in types.ts - EI/INVALID_JSON_ARGUMENTS/001 pattern in errorPatterns.ts (priority 63) - parseErrors static Map in NativeToolCallParser to store parse error messages - consumeParseError()/hasParseError() methods for error retrieval - presentAssistantMessage routes parse errors to INVALID_JSON_ARGUMENTS pattern - 3 unit tests + 2 integration tests (144 total tests pass) Closes #1000 (addresses edelauna's comment on concatenated JSON objects)
1 parent 767d428 commit 429fb9f

6 files changed

Lines changed: 208 additions & 13 deletions

File tree

src/core/assistant-message/NativeToolCallParser.ts

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,35 @@ export class NativeToolCallParser {
7373
}
7474
>()
7575

76+
/**
77+
* Stores JSON.parse error messages keyed by tool call ID.
78+
* When parseToolCall() catches a JSON.parse failure, it records the error
79+
* here so presentAssistantMessage can retrieve it and route the signal to
80+
* the INVALID_JSON_ARGUMENTS error-interception pattern instead of the
81+
* generic PARAM_MISSING path.
82+
*/
83+
private static parseErrors = new Map<string, string>()
84+
85+
/**
86+
* Retrieve and remove the parse error for a given tool call ID.
87+
* Returns undefined if no parse error was recorded.
88+
*/
89+
public static consumeParseError(toolCallId: string): string | undefined {
90+
const error = NativeToolCallParser.parseErrors.get(toolCallId)
91+
if (error !== undefined) {
92+
NativeToolCallParser.parseErrors.delete(toolCallId)
93+
}
94+
return error
95+
}
96+
97+
/**
98+
* Check whether a parse error was recorded for a given tool call ID
99+
* without consuming it.
100+
*/
101+
public static hasParseError(toolCallId: string): boolean {
102+
return NativeToolCallParser.parseErrors.has(toolCallId)
103+
}
104+
76105
private static coerceOptionalBoolean(value: unknown): boolean | undefined {
77106
if (typeof value === "boolean") {
78107
return value
@@ -1030,14 +1059,22 @@ export class NativeToolCallParser {
10301059

10311060
return result
10321061
} catch (error) {
1033-
console.error(
1034-
`Failed to parse tool call arguments: ${error instanceof Error ? error.message : String(error)}`,
1035-
)
1036-
1037-
console.error(`Tool call: ${JSON.stringify(toolCall, null, 2)}`)
1038-
return null
1062+
const errorMessage = error instanceof Error ? error.message : String(error)
1063+
1064+
console.error(
1065+
`Failed to parse tool call arguments: ${errorMessage}`,
1066+
)
1067+
1068+
console.error(`Tool call: ${JSON.stringify(toolCall, null, 2)}`)
1069+
1070+
// Store the parse error so presentAssistantMessage can route it
1071+
// to the INVALID_JSON_ARGUMENTS error-interception pattern
1072+
// instead of the generic PARAM_MISSING path.
1073+
NativeToolCallParser.parseErrors.set(toolCall.id, errorMessage)
1074+
1075+
return null
1076+
}
10391077
}
1040-
}
10411078

10421079
/**
10431080
* Parse dynamic MCP tools (named mcp--serverName--toolName).

src/core/assistant-message/__tests__/presentAssistantMessage-error-interception.spec.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { describe, it, expect, beforeEach, vi } from "vitest"
44
import { presentAssistantMessage } from "../presentAssistantMessage"
55
import { getTaskErrorState } from "../../tools/error-interception"
6+
import { NativeToolCallParser } from "../NativeToolCallParser"
67

78
// Mock heavy dependencies that are not relevant to error interception paths.
89
vi.mock("../../task/Task")
@@ -38,6 +39,15 @@ vi.mock("../../i18n", () => ({
3839
}),
3940
}))
4041

42+
// Mock NativeToolCallParser so we can simulate JSON.parse failures
43+
// (concatenated JSON objects in tool call arguments).
44+
vi.mock("../NativeToolCallParser", () => ({
45+
NativeToolCallParser: {
46+
consumeParseError: vi.fn(() => undefined),
47+
hasParseError: vi.fn(() => false),
48+
},
49+
}))
50+
4151
function createMockTask() {
4252
const mockTask: any = {
4353
taskId: "ei-task-id",
@@ -99,6 +109,8 @@ describe("presentAssistantMessage - Error Interception Integration", () => {
99109
// Reset validateToolUse mock to prevent cross-test contamination from mockImplementationOnce
100110
const { validateToolUse } = await import("../../tools/validateToolUse")
101111
;(validateToolUse as any).mockReset()
112+
// Reset NativeToolCallParser.consumeParseError mock to default (no parse error)
113+
vi.mocked(NativeToolCallParser.consumeParseError).mockReturnValue(undefined)
102114
})
103115

104116
describe("XML_NATIVE_DUAL_PROTOCOL detection", () => {
@@ -622,5 +634,76 @@ describe("presentAssistantMessage - Error Interception Integration", () => {
622634
)
623635
expect(mockTask.didAlreadyUseTool).toBe(false)
624636
})
637+
638+
describe("INVALID_JSON_ARGUMENTS - concatenated JSON tool call", () => {
639+
it("produces guided tool_result with INVALID_JSON_ARGUMENTS when parse error is recorded", async () => {
640+
const toolCallId = "call_concat_json_001"
641+
642+
// Simulate NativeToolCallParser having recorded a JSON.parse
643+
// failure for this tool call (e.g. concatenated JSON objects).
644+
vi.mocked(NativeToolCallParser.consumeParseError).mockReturnValue(
645+
"Unexpected non-whitespace character after JSON at position 42",
646+
)
647+
648+
mockTask.assistantMessageContent = [
649+
{
650+
type: "tool_use",
651+
id: toolCallId,
652+
name: "read_file",
653+
params: {},
654+
// nativeArgs is absent because JSON.parse failed
655+
partial: false,
656+
},
657+
]
658+
659+
await presentAssistantMessage(mockTask)
660+
661+
const toolResult = mockTask.userMessageContent.find(
662+
(item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId,
663+
)
664+
expect(toolResult).toBeDefined()
665+
expect(toolResult.is_error).toBe(true)
666+
// The guided payload should reference the INVALID_JSON_ARGUMENTS category
667+
expect(String(toolResult.content)).toContain("INVALID_JSON_ARGUMENTS")
668+
// The guidance should mention concatenation
669+
expect(String(toolResult.content)).toContain("concatenated")
670+
// Should mention one tool call per file
671+
expect(String(toolResult.content)).toContain("one tool call per file")
672+
// Should NOT contain the generic PARAM_MISSING message
673+
expect(String(toolResult.content)).not.toContain("PARAM_MISSING")
674+
// consecutiveMistakeCount should increment
675+
expect(mockTask.consecutiveMistakeCount).toBe(1)
676+
// Should NOT have set didAlreadyUseTool
677+
expect(mockTask.didAlreadyUseTool).toBe(false)
678+
})
679+
680+
it("falls back to PARAM_MISSING when no parse error is recorded", async () => {
681+
const toolCallId = "call_missing_args_002"
682+
683+
// No parse error recorded — simulate the original missing-args path
684+
vi.mocked(NativeToolCallParser.consumeParseError).mockReturnValue(undefined)
685+
686+
mockTask.assistantMessageContent = [
687+
{
688+
type: "tool_use",
689+
id: toolCallId,
690+
name: "read_file",
691+
params: {},
692+
partial: false,
693+
},
694+
]
695+
696+
await presentAssistantMessage(mockTask)
697+
698+
const toolResult = mockTask.userMessageContent.find(
699+
(item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId,
700+
)
701+
expect(toolResult).toBeDefined()
702+
expect(toolResult.is_error).toBe(true)
703+
// Should contain PARAM_MISSING, not INVALID_JSON_ARGUMENTS
704+
expect(String(toolResult.content)).toContain("PARAM_MISSING")
705+
expect(String(toolResult.content)).not.toContain("INVALID_JSON_ARGUMENTS")
706+
})
707+
})
625708
})
626709
})

src/core/assistant-message/presentAssistantMessage.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type { ToolParamName, ToolResponse, ToolUse, McpToolUse } from "../../sha
2020

2121
import { AskIgnoredError } from "../task/AskIgnoredError"
2222
import { Task } from "../task/Task"
23+
import { NativeToolCallParser } from "./NativeToolCallParser"
2324

2425
import { listFilesTool } from "../tools/ListFilesTool"
2526
import { readFileTool } from "../tools/ReadFileTool"
@@ -509,25 +510,39 @@ export async function presentAssistantMessage(cline: Task) {
509510
const customTool = stateExperiments?.customTools ? customToolRegistry.get(block.name) : undefined
510511
const isKnownTool = isValidToolName(String(block.name), stateExperiments)
511512
if (isKnownTool && !block.nativeArgs && !customTool) {
512-
const errorMessage =
513-
`Invalid tool call for '${block.name}': missing nativeArgs. ` +
514-
`This usually means the model streamed invalid or incomplete arguments and the call could not be finalized.`
515-
513+
// Check whether NativeToolCallParser recorded a JSON.parse
514+
// failure for this tool call ID. If so, the arguments were
515+
// present but malformed (e.g. concatenated JSON objects),
516+
// which is a distinct failure pattern from missing args.
517+
const parseErrorMessage = NativeToolCallParser.consumeParseError(toolCallId)
518+
const isInvalidJson = parseErrorMessage !== undefined
519+
520+
const errorMessage = isInvalidJson
521+
? `Invalid tool call for '${block.name}': arguments could not be parsed as JSON. ` +
522+
`This usually means multiple JSON objects were concatenated into a single arguments string.`
523+
: `Invalid tool call for '${block.name}': missing nativeArgs. ` +
524+
`This usually means the model streamed invalid or incomplete arguments and the call could not be finalized.`
525+
516526
cline.consecutiveMistakeCount++
517527
try {
518528
cline.recordToolError(block.name as ToolName, errorMessage)
519529
} catch {
520530
// Best-effort only
521531
}
522-
532+
523533
// Convert missing nativeArgs into a structured guided payload.
534+
// When a JSON.parse error was recorded, route to the
535+
// INVALID_JSON_ARGUMENTS pattern; otherwise fall back to
536+
// the generic PARAM_MISSING pattern.
524537
const guided = interceptor.transformError(cline, {
525538
source: "parser",
526539
stage: "parse",
527540
taskId: cline.taskId,
528541
toolCallId,
529542
toolName: block.name,
530-
metadata: { missingNativeArgs: true },
543+
metadata: isInvalidJson
544+
? { invalidJsonArguments: true }
545+
: { missingNativeArgs: true },
531546
})
532547

533548
// Push tool_result directly without setting didAlreadyUseTool so streaming can

src/core/tools/error-interception/__tests__/ErrorClassifier.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,41 @@ describe("classifyError", () => {
194194
})
195195
})
196196

197+
it("classifies invalid JSON arguments from parser as INVALID_JSON_ARGUMENTS", () => {
198+
const signal = baseSignal({
199+
source: "parser",
200+
stage: "parse",
201+
metadata: { invalidJsonArguments: true },
202+
})
203+
const result = classifyError(signal)
204+
expect(result.category).toBe("INVALID_JSON_ARGUMENTS")
205+
expect(result.patternId).toBe("EI/INVALID_JSON_ARGUMENTS/001")
206+
expect(result.confidence).toBe("exact")
207+
expect(result.retryPolicy).toBe("correct-and-retry")
208+
})
209+
210+
it("does not classify INVALID_JSON_ARGUMENTS without tool context", () => {
211+
const signal = baseSignal({
212+
source: "parser",
213+
stage: "parse",
214+
toolName: undefined,
215+
metadata: { invalidJsonArguments: true },
216+
})
217+
const result = classifyError(signal)
218+
expect(result.category).not.toBe("INVALID_JSON_ARGUMENTS")
219+
})
220+
221+
it("does not classify INVALID_JSON_ARGUMENTS for missing native args", () => {
222+
const signal = baseSignal({
223+
source: "parser",
224+
stage: "parse",
225+
metadata: { missingNativeArgs: true },
226+
})
227+
const result = classifyError(signal)
228+
expect(result.category).toBe("PARAM_MISSING")
229+
expect(result.category).not.toBe("INVALID_JSON_ARGUMENTS")
230+
})
231+
197232
describe("fallback heuristic matches", () => {
198233
it("classifies shell integration message when name is missing", () => {
199234
const signal = baseSignal({
@@ -369,6 +404,7 @@ describe("classifyError", () => {
369404
"SHELL_INTEGRATION",
370405
"MCP_TOOL_MISSING",
371406
"INVALID_TOOL_PROTOCOL",
407+
"INVALID_JSON_ARGUMENTS",
372408
"CONTEXT_OVERFLOW",
373409
"UNCLASSIFIED",
374410
]

src/core/tools/error-interception/errorPatterns.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,29 @@ export const ERROR_PATTERNS: readonly ErrorPattern[] = [
350350
},
351351
},
352352

353+
// -------------------------------------------------------------------------
354+
// 63 INVALID_JSON_ARGUMENTS
355+
// -------------------------------------------------------------------------
356+
{
357+
id: "EI/INVALID_JSON_ARGUMENTS/001",
358+
category: "INVALID_JSON_ARGUMENTS",
359+
priority: 63,
360+
severity: "error",
361+
retryPolicy: "correct-and-retry",
362+
requiresToolContext: true,
363+
matches: (signal) =>
364+
signal.source === "parser" &&
365+
signal.stage === "parse" &&
366+
metadataIs(signal, "invalidJsonArguments", true),
367+
template: {
368+
what: "Tool call arguments could not be parsed as JSON.",
369+
why: "You concatenated multiple JSON objects into a single arguments string. Each tool call must contain exactly one valid JSON object.",
370+
next: [
371+
"Issue one tool call per file. Use parallel tool calls if you need multiple files simultaneously.",
372+
],
373+
},
374+
},
375+
353376
// -------------------------------------------------------------------------
354377
// 60 CONTEXT_OVERFLOW
355378
// -------------------------------------------------------------------------

src/core/tools/error-interception/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export type ErrorCategory =
1414
| "DIFF_MATCH_FAILED"
1515
| "DUPLICATE_CALL"
1616
| "FILE_NOT_FOUND"
17+
| "INVALID_JSON_ARGUMENTS"
1718
| "INVALID_TOOL_PROTOCOL"
1819
| "MCP_TOOL_MISSING"
1920
| "PARAM_MISSING"

0 commit comments

Comments
 (0)