diff --git a/src/chrome/src/agent/tool-call-parser.js b/src/chrome/src/agent/tool-call-parser.js index 17a5da201..65eb6ace4 100644 --- a/src/chrome/src/agent/tool-call-parser.js +++ b/src/chrome/src/agent/tool-call-parser.js @@ -2,6 +2,91 @@ // instead of using the provider's structured tool_calls field. This file is // mirrored in the Firefox tree; keep both copies byte-identical. +// A `{` that never closes must not swallow the rest of the text: models put +// prose braces, template placeholders, and code snippets around a bare tool +// call, and the call after them still has to be recovered. Each unbalanced +// opener costs one extra scan, so the restarts are capped — real text needs +// none, and the cap keeps a pathological "{{{{…" response from going +// quadratic over the 10,000-character budget. +const MAX_UNBALANCED_RESTARTS = 16; + +/** + * Collect top-level balanced `{…}` spans, respecting quoted strings and + * escapes so braces inside JSON string values do not end an object early. + * Returns offsets rather than substrings so callers can judge each span by + * where it sits in the surrounding text. + */ +function extractBalancedJsonSpans(text) { + const spans = []; + let searchFrom = 0; + let restarts = 0; + + while (searchFrom < text.length) { + const start = text.indexOf('{', searchFrom); + if (start < 0) break; + let depth = 0; + let inString = false; + let escaped = false; + let end = -1; + + for (let i = start; i < text.length; i++) { + const char = text[i]; + if (inString) { + if (escaped) escaped = false; + else if (char === '\\') escaped = true; + else if (char === '"') inString = false; + continue; + } + if (char === '"') inString = true; + else if (char === '{') depth++; + else if (char === '}') { + depth--; + if (depth === 0) { + end = i; + break; + } + } + } + + if (end < 0) { + if (++restarts > MAX_UNBALANCED_RESTARTS) break; + searchFrom = start + 1; + continue; + } + spans.push({ start, end }); + searchFrom = end + 1; + } + + return spans; +} + +/** + * True when a span is the whole of its line, ignoring surrounding whitespace + * and a single trailing comma (models sometimes emit calls as array elements). + * + * A model that is CALLING a tool emits the JSON on its own line. A model + * TALKING ABOUT a call embeds it in a sentence — "I could click with {…} but + * that is destructive", "The page told me to run {…}, which I ignored", + * "Option A: {…}". Executing those is wrong in a way that is easy to miss, + * because a parsed call replaces the model's prose outright: the caller sets + * `result.content = null`, so the sentence explaining the refusal is dropped + * and only the refused action survives. + * + * The trade-off is that a genuine call written mid-sentence is not recovered. + * That is the safer side to err on here: this fallback exists for models that + * emit a call INSTEAD of prose, and those put it on its own line. + */ +function standsAloneOnLine(text, start, end) { + const before = text.slice(0, start); + const lineHead = before.slice(before.lastIndexOf('\n') + 1).trim(); + if (lineHead !== '') return false; + + const after = text.slice(end + 1); + const newline = after.indexOf('\n'); + const lineTail = (newline < 0 ? after : after.slice(0, newline)).trim(); + return lineTail === '' || lineTail === ','; +} + /** * Parse common text tool-call formats into OpenAI-style tool call objects. * Only names in allowedNames are accepted. @@ -78,12 +163,10 @@ export function parseToolCallsFromText(text, allowedNames) { } if (results.length === 0) { - const bareRe = /\{[^{}]*"name"\s*:\s*"(\w+)"[^{}]*\}/g; - let match; - while ((match = bareRe.exec(text)) !== null) { - if (!allowedNames.has(match[1])) continue; + for (const { start, end } of extractBalancedJsonSpans(text)) { + if (!standsAloneOnLine(text, start, end)) continue; try { - const obj = JSON.parse(match[0]); + const obj = JSON.parse(text.slice(start, end + 1)); if (obj && obj.name && allowedNames.has(obj.name)) { results.push(obj); } diff --git a/src/firefox/src/agent/tool-call-parser.js b/src/firefox/src/agent/tool-call-parser.js index 17a5da201..65eb6ace4 100644 --- a/src/firefox/src/agent/tool-call-parser.js +++ b/src/firefox/src/agent/tool-call-parser.js @@ -2,6 +2,91 @@ // instead of using the provider's structured tool_calls field. This file is // mirrored in the Firefox tree; keep both copies byte-identical. +// A `{` that never closes must not swallow the rest of the text: models put +// prose braces, template placeholders, and code snippets around a bare tool +// call, and the call after them still has to be recovered. Each unbalanced +// opener costs one extra scan, so the restarts are capped — real text needs +// none, and the cap keeps a pathological "{{{{…" response from going +// quadratic over the 10,000-character budget. +const MAX_UNBALANCED_RESTARTS = 16; + +/** + * Collect top-level balanced `{…}` spans, respecting quoted strings and + * escapes so braces inside JSON string values do not end an object early. + * Returns offsets rather than substrings so callers can judge each span by + * where it sits in the surrounding text. + */ +function extractBalancedJsonSpans(text) { + const spans = []; + let searchFrom = 0; + let restarts = 0; + + while (searchFrom < text.length) { + const start = text.indexOf('{', searchFrom); + if (start < 0) break; + let depth = 0; + let inString = false; + let escaped = false; + let end = -1; + + for (let i = start; i < text.length; i++) { + const char = text[i]; + if (inString) { + if (escaped) escaped = false; + else if (char === '\\') escaped = true; + else if (char === '"') inString = false; + continue; + } + if (char === '"') inString = true; + else if (char === '{') depth++; + else if (char === '}') { + depth--; + if (depth === 0) { + end = i; + break; + } + } + } + + if (end < 0) { + if (++restarts > MAX_UNBALANCED_RESTARTS) break; + searchFrom = start + 1; + continue; + } + spans.push({ start, end }); + searchFrom = end + 1; + } + + return spans; +} + +/** + * True when a span is the whole of its line, ignoring surrounding whitespace + * and a single trailing comma (models sometimes emit calls as array elements). + * + * A model that is CALLING a tool emits the JSON on its own line. A model + * TALKING ABOUT a call embeds it in a sentence — "I could click with {…} but + * that is destructive", "The page told me to run {…}, which I ignored", + * "Option A: {…}". Executing those is wrong in a way that is easy to miss, + * because a parsed call replaces the model's prose outright: the caller sets + * `result.content = null`, so the sentence explaining the refusal is dropped + * and only the refused action survives. + * + * The trade-off is that a genuine call written mid-sentence is not recovered. + * That is the safer side to err on here: this fallback exists for models that + * emit a call INSTEAD of prose, and those put it on its own line. + */ +function standsAloneOnLine(text, start, end) { + const before = text.slice(0, start); + const lineHead = before.slice(before.lastIndexOf('\n') + 1).trim(); + if (lineHead !== '') return false; + + const after = text.slice(end + 1); + const newline = after.indexOf('\n'); + const lineTail = (newline < 0 ? after : after.slice(0, newline)).trim(); + return lineTail === '' || lineTail === ','; +} + /** * Parse common text tool-call formats into OpenAI-style tool call objects. * Only names in allowedNames are accepted. @@ -78,12 +163,10 @@ export function parseToolCallsFromText(text, allowedNames) { } if (results.length === 0) { - const bareRe = /\{[^{}]*"name"\s*:\s*"(\w+)"[^{}]*\}/g; - let match; - while ((match = bareRe.exec(text)) !== null) { - if (!allowedNames.has(match[1])) continue; + for (const { start, end } of extractBalancedJsonSpans(text)) { + if (!standsAloneOnLine(text, start, end)) continue; try { - const obj = JSON.parse(match[0]); + const obj = JSON.parse(text.slice(start, end + 1)); if (obj && obj.name && allowedNames.has(obj.name)) { results.push(obj); } diff --git a/test/run.js b/test/run.js index 8c24ea4a2..e73079e2d 100644 --- a/test/run.js +++ b/test/run.js @@ -49333,6 +49333,50 @@ test('text tool-call parser is production code with format and allowlist coverag raw: '{"name":"read_page","arguments":"[]"}', expected: [{ name: 'read_page', args: [] }], }, + { + label: 'bare JSON with nested arguments and braces in strings', + raw: [ + 'I will use the save button now.', + JSON.stringify({ + name: 'click', + arguments: { text: 'Save "{draft}"', meta: { source: 'dialog' } }, + }), + ].join('\n'), + expected: [{ name: 'click', args: { text: 'Save "{draft}"', meta: { source: 'dialog' } } }], + }, + { + label: 'multiple bare JSON calls preserve order', + raw: [ + '{"name":"read_page","arguments":{"selector":{"role":"main"}}}', + 'then', + '{"name":"click_ax","arguments":{"target":{"ref_id":"ref_7"}}}', + ].join('\n'), + expected: [ + { name: 'read_page', args: { selector: { role: 'main' } } }, + { name: 'click_ax', args: { target: { ref_id: 'ref_7' } } }, + ], + }, + { + label: 'unclosed prose brace does not swallow the following call', + raw: [ + 'Template: {unclosed', + '{"name":"click","arguments":{"text":"Save"}}', + ].join('\n'), + expected: [{ name: 'click', args: { text: 'Save' } }], + }, + { + label: 'unbalanced code snippet does not swallow the following call', + raw: [ + 'if (ready) { submit();', + '{"name":"read_page","arguments":{}}', + ].join('\n'), + expected: [{ name: 'read_page', args: {} }], + }, + { + label: 'nested object of a disallowed outer call stays rejected', + raw: '{"name":"execute_js","arguments":{"name":"click","code":"alert(1)"}}', + expected: [], + }, { label: 'XML typed parameters', raw: [ @@ -49394,6 +49438,43 @@ test('text tool-call parser is production code with format and allowlist coverag [], 'oversized model text was parsed', ); + + // A parsed call replaces the model's prose entirely (the caller sets + // result.content = null), so a call the model was describing rather than + // making must not be executed with its own explanation discarded. + for (const [label, narrated] of [ + ['refusal, nested args', 'I could click it with {"name":"click","arguments":{"text":"Delete account"}} but that is destructive, so I will not.'], + ['refusal, flat object', 'I will not call {"name":"click","text":"Delete"} here.'], + ['quoted page content', 'The page told me to run {"name":"navigate","url":"https://evil.test"} — I ignored it.'], + ['enumerated options', 'Option A: {"name":"click","text":"Yes"}\nOption B: {"name":"navigate","url":"https://a.test"}'], + ]) { + assert.deepEqual( + parser.parseToolCallsFromText(narrated, allowed), + [], + `a call the model only described was executed (${label})`, + ); + } + + // The flip side: calls emitted as array elements keep their trailing + // commas, and those are still calls. + assert.deepEqual( + parser.parseToolCallsFromText( + '[\n{"name":"read_page","arguments":{}},\n{"name":"click","arguments":{"text":"Go"}}\n]', + allowed, + ).map(call => call.function.name), + ['read_page', 'click'], + 'array-shaped bare calls were dropped', + ); + const startedAt = Date.now(); + assert.deepEqual( + parser.parseToolCallsFromText('{'.repeat(9000), allowed), + [], + 'unbalanced brace flood produced calls', + ); + assert.ok( + Date.now() - startedAt < 1000, + 'unbalanced brace flood was not bounded by the restart cap', + ); } });