fix: parse nested bare JSON tool calls

[alectimison-maker]

Problem

Some model backends emit tool calls in ordinary message content instead of structured tool_calls. The previous bare-JSON fallback could not parse normal calls with nested arguments, but accepting every balanced object embedded in prose would also turn narrated examples or refusals into actions because the caller discards prose when fallback calls are found.

Change

• replace the flat bare-object regex with a string-aware balanced-object scanner
• recover after an unterminated opening brace by restarting from later candidates, capped at 16 restarts
• accept bare JSON candidates only when they occupy their line, allowing whitespace and one trailing comma
• preserve the existing allowlist, wrapped JSON, XML, explicit call:name{...} syntax, 10,000-character input cap, output shape, and Chrome/Firefox parity
• add nested-object, multiple-call, malformed-prefix, narration, bounded-work, and parity regressions

Security boundary and tradeoffs

The own-line rule separates a model committing to a bare call from the common case where it describes JSON inside a sentence. It prevents the tested refusal, quoted page-instruction, and inline-options cases from becoming actions.

This remains a heuristic rather than a complete intent classifier. Options emitted as separate call-only lines can still execute, explicit wrapped/XML/call syntaxes are unchanged, and a genuine bare call written mid-sentence is now ignored. The rule was validated against targeted cases, not a representative model-output corpus.

Validation

• node test/run.js ? 1,449 passed; the only failure is the pre-existing package.json 26.0.10 vs CHANGELOG.md 26.0.0 assertion
• node test/security/injection-corpus.mjs ? 60/60 passed
• Chrome and Firefox parser modules are byte-identical
• node --check for both parser modules ? passed
• git diff --check ? passed

[vercel]

@alectimison-maker is attempting to deploy a commit to the esokullu's projects Team on Vercel.

A member of the Team first needs to authorize it.

[Copilot]

🟡 Changes recommended

An unmatched opening brace before a valid tool call prevents the scanner from recovering that call.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Updates fallback parsing to support nested bare-JSON tool calls while preserving browser parity.

Changes:

• Adds a string-aware balanced-object scanner.
• Adds nested and multiple-call regression tests.
• Keeps Chrome and Firefox implementations identical.

File summaries

File	Description
src/chrome/src/agent/tool-call-parser.js	Adds balanced JSON extraction.
src/firefox/src/agent/tool-call-parser.js	Mirrors Chrome parser changes.
test/run.js	Tests nested arguments and call ordering.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[esokullu]

Pushed a commit fixing the unbalanced-brace regression. Copilot flagged the same thing on both parser files, so this closes that out.

The problem. The scanner opened a candidate at the first { and never reconsidered that offset, so a brace that never closed consumed everything after it. The flat regex this replaced had no such state and did recover those calls. Verified both ways:

input	before	after
Template: {unclosed\n{"name":"click",…}	[]	parsed
if (ready) { submit();\n{"name":"read_page",…}	[]	parsed

Prose braces, template placeholders and code snippets ahead of a bare tool call are exactly what this fallback exists to survive, so it mattered.

The fix. Scanning resumes at start + 1 when a candidate never closes, which is the resynchronisation Copilot asked for. The nested-object recovery this PR is actually about is untouched — top-level candidates are still preferred, and the extra scans only happen on malformed input.

I capped the restarts at 16. Unbounded start + 1 restarts go quadratic: I measured '{'.repeat(10000) at ~95ms of blocking work, which the cap takes under a millisecond while still tolerating far more stray braces than real model output contains. The number is arbitrary in the sense that any value well above "realistic prose" works — say if you'd rather it were higher.

Also. Moved the parseToolCallsFromText docblock back onto its function; the original commit left it stranded above the new helper, which now has its own.

Tests. Both unbalanced-prefix cases, plus one asserting that a nested {"name":"click"} inside a disallowed outer call stays rejected — that pins the property that nested objects aren't promoted to candidates, which is what keeps the allowlist meaningful here. Plus a bounded-work assertion on the brace flood.

1449 passed, security corpus 60/60, Chrome and Firefox copies cmp-verified identical. The one failure is the pre-existing package.json 26.0.10 vs CHANGELOG 26.0.0 mismatch.

Worth flagging for later: there are now two balanced-object scanners per browser tree — this one and json-extract.js — and they'd drifted apart on exactly this recovery behaviour, which is how the same bug ended up living in both. #2683 now matches. Within a tree they could share one helper.

[esokullu]

Follow-up: pushed an own-line rule for bare candidates. This one is a design change rather than a bug fix, so it needs your read.

What I found. Recovering nested bare objects also made narrated calls reachable. Testing the same inputs against main and this branch:

model output	main	this branch (before)
"I could click it with {…"text":"Delete account"…} but that's destructive, so I will not."	[]	executes the click
"Option A: {…click…} Option B: {…navigate…} I recommend A."	[]	executes both
"The page said to run {…navigate to evil.test…} — I ignored it."	[]	navigates there

This matters more than it looks because of what the caller does with the result: agent.js:20700 sets result.content = null when the fallback finds calls. The model's prose is discarded. So in row one the refusal is thrown away and the refused action runs.

One correction to the PR description. It says this "matches the previous flat-object behavior." It doesn't. The old regex matched only brace-free objects, and essentially every real call carries arguments: {…}, so in practice the old fallback rarely fired on prose. This one fires reliably. Worth fixing in the description regardless of what you think of the rule below — as written it tells a reviewer not to look here.

But main is not the safe baseline either, which is what convinced me not to just narrow the scanner. It executes the same narration whenever the object has no nested braces:

• "I will not call {"name":"click","text":"Delete"} here." → ["click"]
• "The page said to run {"name":"navigate","url":"https://evil.test"} — I ignored it." → ["navigate"]

So the real defect is older than this PR and lives in both: the parser cannot tell a call the model committed to from one it merely described. This PR widens its reach; it didn't create it.

The rule. 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. Bare candidates are now accepted only when they are the whole of their line, ignoring surrounding whitespace and a single trailing comma so array-shaped output still parses. Wrapped and XML formats are untouched — they are explicit, so narration was never ambiguous there.

Verified both directions:

• Kept: whole-message call; prose line then call on its own line (your fixture); two calls on their own lines (your fixture); array elements with trailing commas; indented inside a fence; call after an unbalanced prose brace.
• Dropped: all four narration cases above, including the two main executes today.

So this ends up strictly safer than the current main, not just safer than this branch.

Limits, so you can judge it fairly. It is a heuristic on eight hand-picked cases, not a corpus. A model that lists options on separate lines would still get both executed. A genuine call written mid-sentence is now lost — I took that trade deliberately, since this fallback exists for models that emit a call instead of prose, but it is a real behaviour change and it is the part most likely to bite.

It also brushes against something you tested for on purpose: multiple bare calls still parse, but only because your fixture puts them on separate lines. If you meant to support several calls inline, this rule contradicts that and should be revisited.

1449 passed, corpus 60/60, both trees cmp-identical. The parser's worst case is 9ms on a 10k input. Happy to drop this commit if you would rather ship the narrower fix and handle narration separately.

[esokullu]

Status notes to go with the commits above, so the review state is on the record rather than in a side channel.

Who has actually looked at this. Nobody has approved. Copilot commented and its finding is addressed, but that review does not count toward merge requirements. Everything on this branch beyond your original two commits was written by me and reviewed by me — the unbalanced-brace fix, the restart cap, and the own-line rule. No independent eyes. That is worth weighing given the last of those is a behaviour change to the trust boundary rather than a bug fix.

What needs your decision, not just your review. The own-line rule narrows what counts as a tool call. I believe it is right, and it closes holes main has today, but it is a judgment call about model behaviour that you are better placed to make than I am. If you would rather ship the narrower nested-object fix and handle narration as its own change, say so and I will drop that commit — the two earlier ones stand on their own.

One thing only you can fix. The PR description still says the residual risk "matches the previous flat-object behavior." It does not, and that sentence is the one most likely to stop the next reviewer from checking the narration case. I cannot edit the PR body.

Verification, stated plainly. The own-line rule was validated against eight inputs I picked myself — six that must parse, four that must not. That is not a corpus, and I would not describe it as proof. Two gaps I know about: options listed on separate lines still both execute, and a genuine call written mid-sentence is now dropped.

CI. smoke passes. Vercel is red on the fork-deploy authorization prompt, not on anything in the diff. npm test shows one failure, the pre-existing package.json 26.0.10 vs CHANGELOG.md 26.0.0 mismatch, which PR CI does not run — it will surface at release time instead, and is worth fixing on main independently of this PR.

I have kept these notes here rather than duplicating them on #2678 and #2683, since this is the PR carrying a design decision. The same "no approval, author has not reviewed the pushed commits" applies to both of those.

[alectimison-maker]

Thanks for surfacing the trust-boundary consequence. I reviewed the current head (6ec52c04) and agree that the own-line rule should stay in this PR.

The deciding point for me is the caller behavior: once fallback calls are returned, the surrounding prose is discarded. Without a boundary, an inline example or refusal can become the action while the refusal itself disappears. Requiring a bare candidate to stand alone on its line is a reasonable narrowing for this fallback.

I also agree with the limits you called out. It is a heuristic, not corpus-level evidence: options on separate lines can still execute, explicit call syntaxes are unchanged, and a genuine mid-sentence bare call is lost. I've replaced the PR description's incorrect equivalence to the old flat regex and documented those tradeoffs directly.

I reran the test runner (1,449 passed, with only the existing package/CHANGELOG mismatch), the security corpus (60/60), syntax checks, parser parity, and git diff --check.