fix(json-parse): exclude code-context tails from Windows-path heuristic (#93139)#111523
fix(json-parse): exclude code-context tails from Windows-path heuristic (#93139)#111523jrusz wants to merge 2 commits into
Conversation
|
Codex review: needs changes before merge. Reviewed July 20, 2026, 3:32 PM ET / 19:32 UTC. Summary PR surface: Source +40, Tests +66. Total +106 across 2 files. Reproducibility: yes. from source: pass a JSON string containing Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Make the keyword test inspect the text ending immediately before the separator/drive-letter candidate, then add direct regressions for Do we have a high-confidence way to reproduce the issue? Yes, from source: pass a JSON string containing Is this the best way to solve the issue? No. The suffix-scoped strategy is the right boundary, but the current slice includes the separator after the keyword and therefore cannot recognize the intended code-context suffix. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e2bb04328f5c. Label changesLabel justifications:
Evidence reviewedPR surface: Source +40, Tests +66. Total +106 across 2 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (4 earlier review cycles)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Changes in this push:
All 22 tests pass. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
80f5078 to
feaa367
Compare
|
@clawsweeper re-review Changes in this push:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
feaa367 to
10c5b50
Compare
The repairJson helper's looksLikeWindowsPathPrefix() uses a regex that matches any tail ending in `<non-alphanum><letter>:`, which catches genuine drive letters like `C:` but also false-positives on Python/JS/shell block openers like `if x:`, `for x:`, `try:`, `def foo(x):`, `with open(...) as f:`. When triggered, the next \n escape is doubled to \\n, producing a literal backslash-n instead of a real newline in the parsed tool arguments. This corrupts write/edit tool calls carrying multi-line Python code. The corruption cascades: once the first \n is doubled, all subsequent \n escapes in the same JSON string are also corrupted. Fix: reject any tail containing code-context markers (parentheses, braces, equals, semicolons, single-quotes, commas, shell redirects, pipes) before the Windows path regex check. Real Windows path tails do not contain these characters. Adds 7 regression tests covering Python block openers, try/except, def signatures, while loops, the original openclaw#93139 reproducer, bash for-loops, and a mixed Windows-path-plus-code case. Validated on a live OpenClaw deployment (v2026.7.1 + this patch): - Without fix: 2/2 runs reproduced the bug (6-8 literal \n per file) - With fix: 2/2 runs clean (0 structural bugs) Closes openclaw#93139 Signed-off-by: Jakub Rusz <[email protected]>
10c5b50 to
1d654c4
Compare
Narrows the Windows-path heuristic to the matched drive candidate and the text ending before its separator. Whitespace-separated lowercase candidates after block-opening keywords are treated as code, while conventional uppercase drive letters and punctuation-delimited paths keep the existing Windows-path repair behavior. The casing rule is the bounded tie-break for malformed JSON that is otherwise ambiguous between one-letter code variables and drive paths. Adds direct regressions for if/as/in/match/case headers, the reported if-r case, uppercase batch paths, punctuation-wrapped paths, and an unescaped path after code-like command text. Refs openclaw#93139 Signed-off-by: Jakub Rusz <[email protected]>
1d654c4 to
35fde49
Compare
|
@clawsweeper re-review Updated final head
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Retrying unchanged exact head |
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(json-parse): exclude code-context tails from Windows-path heuristic (#93139) This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
What Problem This Solves
Fixes silent newline corruption when users write multi-line Python or shell code through agent tools and a block header ends in a lowercase one-letter identifier, such as
as f:,if r:,in d:,match x:, orcase x:.repairJson()could mistake that final<letter>:for a Windows drive prefix. It then doubled the following\nescape, soJSON.parseproduced a literal backslash plusninstead of a newline. Once triggered, later newlines in the same string could also be corrupted.Related: #93139 and #95982 (same root cause, but targeting source paths that moved on current
main).Closes #93139
Why This Change Was Made
The existing Windows-path heuristic matches tails ending in
<non-alphanumeric><letter>:so it can recover malformed JSON containing paths such asC:\new\file. That shape is inherently ambiguous with code such asif x:\n.The final rule keeps the repair localized to the shared parser and uses a bounded convention-based tie-break:
The keyword boundary recognizes prior JSON control escapes in their literal tracked form (
\\n,\\r, and\\t), becausestringValuePrefixstores those two-character escape spellings rather than decoded whitespace.This deliberately favors the reported lowercase code variables (
x,r,d,f) while preserving conventional uppercase paths such asif C:\new==C:\temp. The malformed input is otherwise ambiguous: uppercase code variables remain path-biased, while lowercase drive paths immediately following a guarded keyword remain code-biased.The guard is suffix-scoped, so punctuation or code-like text earlier in the same value does not disable repair for a later genuine path.
User Impact
Evidence
Byte-level proof before the fix
The corrupted file contained
5c 6e(backslash plusn) where0a(newline) was expected:Real OpenClaw verifier — before fix
OpenClaw v2026.7.1 stable, Claude Opus 4.6 through Anthropic Vertex:
Two unpatched runs reproduced the same structural corruption.
Real OpenClaw verifier — after applying the parser guard
Two patched runs completed with zero structural newline corruption. This live proof exercised the reported lowercase
as f:/ifclass. The final candidate-boundary and compatibility narrowing is covered by the exact-head parser tests below.Final-head focused tests
Final head:
35fde491743The 25 tests include 14 pre-existing parser cases plus 11 focused regressions covering both
parseJsonWithRepairandparseStreamingJson:if x:,as f:,in d:,match x:,case x:if r:sequence after a prior newlineif C:\new==C:\tempif,C:\new,(C:\new),{C:\new}C:\new\file.txtpath after code-like command textC:\bin,C:\temp,C:\new,D:\reports, andC:\userscompatibility fixturesFormatting and diff checks
Exact-head hosted CI is being refreshed by this push. The previous head's required
openclaw/ci-gatewas green.Scope
packages/ai/src/utils/json-parse.tssrc/llm/utils/json-parse.test.ts