fix(llm): trust control escapes in JSON strings that escape backslashes#96949
fix(llm): trust control escapes in JSON strings that escape backslashes#96949Bartok9 wants to merge 1 commit into
Conversation
repairJson has a Windows-path heuristic: inside a string, a backslash before a
control-escape letter (b/f/n/r/t) whose decoded prefix looks like a Windows path
is treated as an unescaped path separator and doubled, so a model that emits a
raw path like {"path":"C:\new\file"} is repaired to the literal path instead of
a newline/formfeed. That bet is deliberate and test-pinned for unescaped paths.
But the heuristic inspected only the DECODED prefix, which is identical whether
the wire used escaped "\\" (valid) or raw "\" (malformed). So for a string that
properly escapes its path — e.g. JSON.stringify({text:"C:\\Users\\me\nDone."}),
where the path uses "\\" and the \n is a genuine newline — it still fired and
turned the real newline into a literal backslash-n. OpenAI/Azure/ChatGPT
Responses transports feed parseStreamingJson straight into tool calls with no
JSON.parse-first guard, so a model emitting a Bash/Write/Edit argument with a
properly escaped Windows path plus multi-line content ran a corrupted command.
Fix: once a string contains a properly escaped backslash ("\\"), the model
demonstrably escapes backslashes, so its later \n/\t/etc. are genuine control
escapes — skip the path heuristic for the rest of that string. Strings with no
escaped backslash (the pinned unescaped-path cases) repair exactly as before;
this only adds positive escaping evidence as a suppression signal. Add a
regression test asserting an escaped path followed by a real newline/tab
round-trips to JSON.parse.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
openclaw-barnacle
Bot
added
size: XS
triage: needs-pr-context
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. This PR should close as a duplicate: the same parser fix is already open in a proof-positive, clean canonical PR by the original author, so keeping this salvage branch open adds review noise without unique code or evidence. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Close this duplicate and review, land, or reject #96748 as the canonical fix path. So I’m closing this here and keeping the remaining discussion on #96748. Review detailsBest possible solution: Close this duplicate and review, land, or reject #96748 as the canonical fix path. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main and v2026.6.10 still run the Windows-path heuristic before valid escape preservation, and the canonical same-patch PR includes fail-before/pass-after terminal proof against the exported parser functions. Is this the best way to solve the issue? No for this PR as a landing path. The code change is a plausible shared-parser fix, but the same patch is already open with proof and maintainer-ready review at #96748. Security review: Security review cleared: No concrete security or supply-chain concern was found; the diff only changes local parser logic and colocated tests. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 4fc504d321b6. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
openclaw-barnacle
Bot
removed
the
triage: needs-pr-context
clawsweeper
Bot
added
proof: sufficient
|
Closing as a duplicate of the canonical #96748 — same two-file escaped-backslash control-escape patch by the original fix author, open and proof-positive. No unique behavior here beyond the re-submission, so deferring to #96748 as the landing path. Thanks @clawsweeper for the cross-reference. |
2 participants
What Problem This Solves
Salvages #96737 by @ly-wang19 onto current
main— clean single-commit rebase, original authorship preserved. The original was auto-closed byopenclaw-barnaclefor the author's >20-active-PR cap, not on merits; the bug is still live onmain.repairJson— the tolerant parser behindparseStreamingJson/parseJsonWithRepairused for model tool-call arguments — corrupts genuine control escapes (\n,\t, …) that legitimately follow an escaped Windows path inside a JSON string. A real newline in a multi-line tool-call argument becomes a literal backslash-n.Root Cause
Symptom — A tool-call argument like
{"text":"Saved to C:\\Users\\me\nDone."}(valid JSON:\\are escaped separators,\nis a real newline) comes back with the\nrewritten to a literal\\n, breaking multi-line shell commands and any value that mixes a Windows path with a real control char.Root cause — In
src/llm/utils/json-parse.ts,repairJson()has a heuristic that, when it sees a control-escape char after text thatlooksLikeWindowsPathPrefix, assumes the model forgot to escape a backslash and rewrites\→\\. But once a string has already contained a properly escaped backslash (\\), the model has demonstrated it escapes correctly, so subsequent\n/\tare genuine control escapes — the heuristic fires anyway and corrupts them.Evidence — On current
main, line 82 guards only onlooksLikeWindowsPathPrefix(stringValuePrefix), with no "already saw a correct escape" condition. The new regression test fails against thismainsource (see Real behavior proof).Fix + why this level — Track
sawEscapedBackslashper string; once a correctly escaped\\is seen, suppress the Windows-path rewrite for the remainder of that string. Reset on string open/close. This is the right level because the false positive is intrinsic to the repair heuristic's assumption — downstream consumers can't distinguish a real newline from a corrupted one after the fact.Scope / risk — Touches only the Windows-path repair branch in
repairJson. Strings that never contain a correct\\still get the original repair behavior (the genuinely-malformed case it was designed for). Reset on string boundaries prevents cross-string leakage.Evidence
src/llm/utils/json-parse.test.ts→ "preserves a real control escape after an escaped Windows path" — fails without the fix.WITH fix:
WITHOUT fix (test against current
mainsource forjson-parse.ts):Changes from original
main(clean single commit, no conflicts).Credit: Salvage of #96737 by @ly-wang19 — rebased onto current main. Original closed by the >20-active-PR queue cap, not on merits.