Skip to content

Commit 7fc36fd

Browse files
authored
fix: repair malformed JSON in model-written tool arguments (#62)
## Summary Model-written tool arguments sometimes fail strict `JSON.parse` and kill the tool call at preflight. Two failure classes were observed in real Dynamic Workflow launches: - **Invalid escape sequences** — markdown-style escapes such as `\*` inside JSON string values (`Bad escaped character at position 3458`). - **Unescaped inner quotes** — a literal `"` inside a string value that terminates it early (`Expected ',' or ']' after array element in JSON at position 2832`). Both now repair instead of failing the call. The repair runs only as a fallback after `JSON.parse` rejects the input, and the original parse error is still reported when the repaired text also fails to parse. ## Approach A single scanner pass rewrites invalid escapes to a literal backslash plus character, and rewrites an in-string `"` to `\"` unless the next non-whitespace character is structural (`,` `:` `}` `]`) or end of input. A content quote immediately followed by a structural character stays ambiguous and still closes the string; that limit is documented in the function comment. ## Test plan - `packages/agent-core/test/loop/tool-call.e2e.test.ts` — 45 tests, including a reproduction of the observed items-array failure, mixed-class repair, whitespace before the terminator, and preservation of the original error on still-broken input. - Full suite green locally. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of malformed tool arguments, including invalid escape sequences and unescaped quotation marks. * Preserves valid escapes while repairing recoverable formatting issues. * Provides clearer errors when arguments remain structurally invalid. * **Tests** * Added coverage for malformed escapes, embedded quotes, combined errors, valid escapes, and invalid JSON structures. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 065bf2e commit 7fc36fd

3 files changed

Lines changed: 146 additions & 0 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pythoughts/pythinker-code": patch
3+
---
4+
5+
Repair invalid escape sequences and unescaped quotes in model-written tool arguments instead of failing the tool call.

packages/agent-core/src/loop/tool-call.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,10 +300,76 @@ export function parseToolCallArguments(
300300
try {
301301
return { success: true, data: JSON.parse(raw) as unknown };
302302
} catch (error) {
303+
const repaired = repairInvalidStringEscapes(raw);
304+
if (repaired !== null) {
305+
try {
306+
return { success: true, data: JSON.parse(repaired) as unknown };
307+
} catch {
308+
// Report the original parse error below.
309+
}
310+
}
303311
return { success: false, error: errorMessage(error) };
304312
}
305313
}
306314

315+
/**
316+
* Models sometimes emit invalid escapes (\* \_ \[) or unescaped quotes inside
317+
* JSON string values. Rewrite invalid escapes to a literal backslash +
318+
* character and quotes that cannot terminate the string to escaped quotes.
319+
* A content quote followed by a structural character is ambiguous and still
320+
* closes the string; if reparsing fails, the original parse error is reported.
321+
* Returns null when nothing was repaired.
322+
*/
323+
function repairInvalidStringEscapes(raw: string): string | null {
324+
let result = '';
325+
let inString = false;
326+
let repaired = false;
327+
328+
for (let index = 0; index < raw.length; index += 1) {
329+
const character = raw[index];
330+
if (character === '"') {
331+
if (!inString) {
332+
inString = true;
333+
result += character;
334+
continue;
335+
}
336+
337+
let lookahead = index + 1;
338+
while (lookahead < raw.length && ' \t\n\r'.includes(raw[lookahead]!)) lookahead += 1;
339+
const next = raw[lookahead];
340+
if (next === undefined || ',:}]'.includes(next)) {
341+
inString = false;
342+
result += character;
343+
} else {
344+
result += '\\"';
345+
repaired = true;
346+
}
347+
continue;
348+
}
349+
if (!inString || character !== '\\') {
350+
result += character;
351+
continue;
352+
}
353+
354+
const next = raw[index + 1];
355+
if (next !== undefined && '"\\/bfnrt'.includes(next)) {
356+
result += character + next;
357+
index += 1;
358+
continue;
359+
}
360+
if (next === 'u' && /^[0-9a-fA-F]{4}$/u.test(raw.slice(index + 2, index + 6))) {
361+
result += raw.slice(index, index + 6);
362+
index += 5;
363+
continue;
364+
}
365+
366+
result += '\\\\';
367+
repaired = true;
368+
}
369+
370+
return repaired ? result : null;
371+
}
372+
307373
function validateExecutableToolArgs(tool: ExecutableTool, args: unknown): string | null {
308374
let validator = validators.get(tool);
309375
if (validator === undefined) {

packages/agent-core/test/loop/tool-call.e2e.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type { ContentPart } from '@pythoughts/kosong';
1212
import { describe, expect, it } from 'vitest';
1313

1414
import { createLoopEventDispatcher, runTurn as runTurnImpl, ToolAccesses } from '../../src/loop';
15+
import { parseToolCallArguments } from '../../src/loop/tool-call';
1516
import type { Logger } from '../../src/logging';
1617
import type {
1718
ExecutableTool,
@@ -48,6 +49,15 @@ function expectTextOutput(output: unknown): string {
4849
return output as string;
4950
}
5051

52+
function parseErrorMessage(raw: string): string {
53+
try {
54+
JSON.parse(raw);
55+
} catch (error) {
56+
return error instanceof Error ? error.message : String(error);
57+
}
58+
throw new Error(`expected ${raw} to fail JSON.parse`);
59+
}
60+
5161
async function contentBlockOutput(output: ContentPart[]): Promise<ContentPart[]> {
5262
const blocks = new ContentBlocksTool({ output });
5363
const { context } = await runTurn({
@@ -118,6 +128,71 @@ function makeTestLogger(): {
118128
return { log, entries };
119129
}
120130

131+
describe('parseToolCallArguments', () => {
132+
it('repairs markdown-style escapes inside string values', () => {
133+
const result = parseToolCallArguments('{"a":"bold \\*text\\* and \\_x"}');
134+
135+
expect(result).toEqual({ success: true, data: { a: 'bold \\*text\\* and \\_x' } });
136+
});
137+
138+
it('leaves valid escapes unchanged', () => {
139+
const raw = '{"a":"line\\nquote\\" uA slash\\\\/"}';
140+
141+
expect(parseToolCallArguments(raw)).toEqual({ success: true, data: JSON.parse(raw) });
142+
});
143+
144+
it('repairs a bad unicode escape inside a string value', () => {
145+
const result = parseToolCallArguments('{"a":"\\u12ZZ"}');
146+
147+
expect(result).toEqual({ success: true, data: { a: '\\u12ZZ' } });
148+
});
149+
150+
it('repairs unescaped quotes inside items-array string values', () => {
151+
const result = parseToolCallArguments(
152+
'{"items":[{"prompt":"Review the "config" module carefully","i":"review config"}]}',
153+
);
154+
155+
expect(result).toEqual({
156+
success: true,
157+
data: { items: [{ prompt: 'Review the "config" module carefully', i: 'review config' }] },
158+
});
159+
});
160+
161+
it('leaves a quote before a structural character unchanged', () => {
162+
const raw = '{"a":"done","b":1}';
163+
164+
expect(parseToolCallArguments(raw)).toEqual({ success: true, data: JSON.parse(raw) });
165+
});
166+
167+
it('repairs invalid escapes and unescaped quotes together', () => {
168+
const result = parseToolCallArguments('{"a":"bold \\*x and a "quoted" word"}');
169+
170+
expect(result).toEqual({ success: true, data: { a: 'bold \\*x and a "quoted" word' } });
171+
});
172+
173+
it('recognizes a string terminator separated from structure by whitespace', () => {
174+
const result = parseToolCallArguments('{"a":"text" , "b":"x "y" z"}');
175+
176+
expect(result).toEqual({ success: true, data: { a: 'text', b: 'x "y" z' } });
177+
});
178+
179+
it('returns the original parse error after quote repair still fails', () => {
180+
const raw = String.raw`{"a":"bad \*","b":[}`;
181+
182+
expect(parseToolCallArguments(raw)).toEqual({ success: false, error: parseErrorMessage(raw) });
183+
});
184+
185+
it('returns the original parse error for structurally broken input', () => {
186+
const raw = '{"a":"truncated';
187+
188+
expect(parseToolCallArguments(raw)).toEqual({ success: false, error: parseErrorMessage(raw) });
189+
});
190+
191+
it('does not repair a backslash outside a string', () => {
192+
expect(parseToolCallArguments('{\\*"a":1}').success).toBe(false);
193+
});
194+
});
195+
121196
describe('runTurn — tool-call behaviour', () => {
122197
it('strips enabled intent before hooks, validation, execution, and persistence', async () => {
123198
const hookArgs: unknown[] = [];

0 commit comments

Comments
 (0)