Skip to content

fix(json-parse): exclude code-context tails from Windows-path heuristic (#93139)#111523

Open
jrusz wants to merge 2 commits into
openclaw:mainfrom
jrusz:fix/repairjson-windows-path-false-positive
Open

fix(json-parse): exclude code-context tails from Windows-path heuristic (#93139)#111523
jrusz wants to merge 2 commits into
openclaw:mainfrom
jrusz:fix/repairjson-windows-path-false-positive

Conversation

@jrusz

@jrusz jrusz commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

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:, or case x:.

repairJson() could mistake that final <letter>: for a Windows drive prefix. It then doubled the following \n escape, so JSON.parse produced a literal backslash plus n instead 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 as C:\new\file. That shape is inherently ambiguous with code such as if x:\n.

The final rule keeps the repair localized to the shared parser and uses a bounded convention-based tie-break:

  1. Match only the candidate drive-path suffix.
  2. End the keyword subject before the candidate separator.
  3. Treat a whitespace-separated lowercase candidate after a block-opening keyword as code.
  4. Preserve conventional uppercase drive letters and punctuation-delimited paths as Windows paths.

The keyword boundary recognizes prior JSON control escapes in their literal tracked form (\\n, \\r, and \\t), because stringValuePrefix stores 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 as if 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

  • Multi-line code containing the reported one-letter block headers now retains real newlines through tool-call JSON repair.
  • Existing malformed Windows-path recovery remains intact for uppercase drives, punctuation-wrapped paths, and paths following code-like command text.
  • No public API, configuration, dependency, or export changes.

Evidence

Byte-level proof before the fix

The corrupted file contained 5c 6e (backslash plus n) where 0a (newline) was expected:

# Line 35 of test_write.py, at the "as f:" boundary
Bytes: 61 73 20 66 3a 5c 6e 20 20 20 20 20 20 20 20 20 20 20 20 64 61 74 61
ASCII: as f:\n            data
#            ^^ ^^ literal 0x5c 0x6e instead of 0x0a

Real OpenClaw verifier — before fix

OpenClaw v2026.7.1 stable, Claude Opus 4.6 through Anthropic Vertex:

Run 1 — Version 2026.7.1-2 (stable), NO FIX

🐛 BUG DETECTED: test_write.py
  Real newlines: 61, Literal \n: 6
  Suspicious lines:
      Line 8: with open(f"/tmp/{item['name']}.log", 'w') as f:\n                f.write(...)
      Line 35: with open(self.config['db_path']) as f:\n            data = json.load(f)\n        if key in data:

🐛 BUG DETECTED: test_write2.py
  Real newlines: 24, Literal \n: 2

RESULT: 🐛 Bug reproduced — literal \n found in tool output

Two unpatched runs reproduced the same structural corruption.

Real OpenClaw verifier — after applying the parser guard

Run 3 — Version 2026.7.1, parser guard deployed

✅ Clean: test_write.py
  Real newlines: 66, Structural bugs: 0, Total literal \n: 1
  (the remaining literal \n was inside a string literal and expected)

✅ Clean: test_write2.py
  Real newlines: 26, Structural bugs: 0, Total literal \n: 0

RESULT: ✅ No bug detected in this run

Two patched runs completed with zero structural newline corruption. This live proof exercised the reported lowercase as f:/if class. The final candidate-boundary and compatibility narrowing is covered by the exact-head parser tests below.

Final-head focused tests

Final head: 35fde491743

$ node scripts/run-vitest.mjs src/llm/utils/json-parse.test.ts

 Test Files  1 passed (1)
      Tests  25 passed (25)
   Duration  351ms

[test] passed 1 Vitest shard in 4.49s

The 25 tests include 14 pre-existing parser cases plus 11 focused regressions covering both parseJsonWithRepair and parseStreamingJson:

  • direct lowercase code candidates: if x:, as f:, in d:, match x:, case x:
  • the reported if r: sequence after a prior newline
  • uppercase batch paths after a guarded keyword: if C:\new==C:\temp
  • punctuation-wrapped malformed paths: if,C:\new, (C:\new), {C:\new}
  • a malformed C:\new\file.txt path after code-like command text
  • the existing C:\bin, C:\temp, C:\new, D:\reports, and C:\users compatibility fixtures

Formatting and diff checks

$ ./node_modules/.bin/oxfmt --check packages/ai/src/utils/json-parse.ts src/llm/utils/json-parse.test.ts
All matched files use the correct format.

$ git diff --check upstream/main...HEAD
git diff --check: PASS

Exact-head hosted CI is being refreshed by this push. The previous head's required openclaw/ci-gate was green.

Scope

  • packages/ai/src/utils/json-parse.ts
  • src/llm/utils/json-parse.test.ts
  • No changelog change, per contributor policy.
  • Allow edits by maintainers remains enabled.
  • AI-assisted: yes; the implementation and regressions were manually reviewed and validated as described above.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed July 20, 2026, 3:32 PM ET / 19:32 UTC.

Summary
The branch narrows looksLikeWindowsPathPrefix() in the JSON repair parser and adds regression cases for Python and shell block headers plus Windows-path preservation.

PR surface: Source +40, Tests +66. Total +106 across 2 files.

Reproducibility: yes. from source: pass a JSON string containing if x:\\n or as f:\\n through parseJsonWithRepair; the candidate match starts at the separator and the present keyword test receives a trailing space, so it does not reject the false positive.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #93139
Summary: This PR is a candidate fix for the canonical JSON-repair newline-corruption report; the older open parser PR addresses the same root cause but is not a safe superseding landing path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦞 diamond lobster
Patch quality: 🦪 silver shellfish
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Exclude the separator from the keyword-suffix subject and add direct parser assertions for one-letter block headers.
  • Rerun the focused JSON-parser test file and provide the resulting output after the correction.

Risk before merge

  • [P1] Merging as written leaves the central silent-corruption case active for common block headers ending in a one-letter identifier, despite the added regression suite and claimed real-run proof.

Maintainer options:

  1. Correct the candidate-boundary slice (recommended)
    Exclude the separator from the keyword-suffix subject and rerun direct parser regressions before merge so code headers stop being classified as Windows paths.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Repair the keyword-boundary slice so it ends before the candidate separator, add direct `if x:` and `as f:` regressions plus Windows-path preservation coverage, and provide focused test output.

Next step before merge

  • A narrow source-and-regression correction can resolve the remaining P1 defect without a product or API decision.

Security
Cleared: The two-file parser-and-test diff adds no dependency, permission, secret, CI, artifact, or supply-chain surface.

Review findings

  • [P1] Exclude the separator from the keyword suffix — packages/ai/src/utils/json-parse.ts:147-150
Review details

Best possible solution:

Make the keyword test inspect the text ending immediately before the separator/drive-letter candidate, then add direct regressions for if x:, as f:, match x:, and case x: alongside a genuine Windows-path preservation case.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: pass a JSON string containing if x:\\n or as f:\\n through parseJsonWithRepair; the candidate match starts at the separator and the present keyword test receives a trailing space, so it does not reject the false positive.

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:

  • [P1] Exclude the separator from the keyword suffix — packages/ai/src/utils/json-parse.ts:147-150
    beforeCandidate includes sep, so if x: produces if and cannot satisfy the keyword regex ending at $. The false-positive path remains active for the reported one-letter block headers; slice through matchIndex instead, or otherwise allow the separator after the keyword, and add direct assertions for this shape.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e2bb04328f5c.

Label changes

Label justifications:

  • P1: The PR targets silent corruption of code written through agent tools, but the current guard still misses the representative one-letter block-header form.
  • merge-risk: 🚨 compatibility: Changing the JSON-repair path can alter recovery of malformed Windows-style paths, so the final guard needs focused preservation coverage.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦞 diamond lobster and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body contains before/after terminal verifier output from a real v2026.7.1 write workflow; it demonstrates the original defect and an improved observed result, though the source correction still needs repair.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains before/after terminal verifier output from a real v2026.7.1 write workflow; it demonstrates the original defect and an improved observed result, though the source correction still needs repair.
Evidence reviewed

PR surface:

Source +40, Tests +66. Total +106 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 41 1 +40
Tests 1 66 0 +66
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 107 1 +106

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/llm/utils/json-parse.test.ts.
  • [P1] git diff --check.

What I checked:

  • Central guard still misses ordinary headers: The candidate match for if x: begins at the space before x; beforeCandidate slices through sep.length, producing if rather than if, while the keyword regex requires the keyword at the end of its input. The guard therefore returns true and preserves the false-positive Windows-path classification. (packages/ai/src/utils/json-parse.ts:147, 1d654c4f0e39)
  • Regression scope is the reported root cause: The linked canonical report describes the same false positive: a one-letter identifier immediately before a colon makes the prior Windows-path heuristic treat the following escaped newline as path content. This PR explicitly targets that report but does not yet reject its representative if x:/as f: shape. (packages/ai/src/utils/json-parse.ts:112, 1d654c4f0e39)
  • Previous review cycle was substantively addressed but not sufficient: The current head adds match and case to the keyword list after the earlier review requested them; the remaining flaw is the construction of the string tested by that list, not the omission of those two keywords. (packages/ai/src/utils/json-parse.ts:149, 1d654c4f0e39)
  • Real behavior proof exists for the original defect: The PR body includes before/after terminal verifier output on v2026.7.1 showing structural literal-\\n corruption before the patch and clean writes after it. That proof is meaningful for the reported workflow, but it does not overcome the source-level defect in the current guard. (1d654c4f0e39)

Likely related people:

  • louria: Authored the earlier open PR that identified the same repairJson Windows-path false-positive mechanism and proposed regression coverage around this parser path. (role: adjacent implementation contributor; confidence: low; commits: bf74d346f3f5; files: src/llm/utils/json-parse.test.ts)
  • Haderach-Ram: Opened the canonical report with the user-visible write/heredoc corruption scenario that this parser change is intended to fix. (role: reproduction reporter; confidence: low; files: packages/ai/src/utils/json-parse.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (4 earlier review cycles)
  • reviewed 2026-07-19T19:50:54.900Z sha 3761af6 :: needs real behavior proof before merge. :: [P1] Preserve Windows paths after code-like command text
  • reviewed 2026-07-19T20:02:35.193Z sha 3761af6 :: needs changes before merge. :: [P1] Scope the code-context rejection to the candidate path suffix
  • reviewed 2026-07-19T20:26:22.256Z sha 80f5078 :: needs changes before merge. :: [P1] Reject loop headers whose final token masks the keyword
  • reviewed 2026-07-19T21:09:10.587Z sha 80f5078 :: needs changes before merge. :: [P1] Cover match and case one-letter headers

@jrusz

jrusz commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 19, 2026
@jrusz

jrusz commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Changes in this push:

  • Replaced the full-tail punctuation rejection with a suffix-scoped guard that checks only the matched candidate drive-path and its immediate predecessor
  • Added regression test for code-like text before a Windows path (python -c 'x'; C:\new\file.txt)
  • Keyword boundary uses (?:^|[^A-Za-z0-9]|\\[nrt]) to handle stringValuePrefix escape tracking (prior \n stored as literal \ + n)

All 22 tests pass.

@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 19, 2026
@jrusz
jrusz force-pushed the fix/repairjson-windows-path-false-positive branch from 80f5078 to feaa367 Compare July 20, 2026 17:19
@jrusz

jrusz commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Changes in this push:

  • Fixed TS18048: const sep = m[1] ?? "" (strict null handling for regex capture group)
  • Added match and case to the keyword guard (Python 3.10+ structural pattern matching)
  • Added regression tests for match x:\n and case x:\n patterns
  • All 24 tests pass, TypeScript compiles clean, formatting verified with oxfmt

@clawsweeper

clawsweeper Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@jrusz
jrusz force-pushed the fix/repairjson-windows-path-false-positive branch from feaa367 to 10c5b50 Compare July 20, 2026 17:30
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]>
@jrusz
jrusz force-pushed the fix/repairjson-windows-path-false-positive branch from 10c5b50 to 1d654c4 Compare July 20, 2026 19:27
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 21, 2026
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]>
@jrusz
jrusz force-pushed the fix/repairjson-windows-path-false-positive branch from 1d654c4 to 35fde49 Compare July 21, 2026 18:18
@jrusz

jrusz commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated final head 35fde491743 to make the candidate boundary explicit:

  • the keyword subject now ends before the candidate separator
  • code rejection requires a whitespace separator and lowercase candidate
  • conventional uppercase drives and punctuation-delimited paths remain path-biased
  • added direct if x:, as f:, in d:, match x:, case x:, and reported if r: assertions
  • added genuine single-backslash preservation coverage for if C:\new, if,C:\new, wrapped paths, and a path after code-like text
  • refreshed the PR body with final-head focused output: 25/25 tests pass, formatting and git diff --check pass

@clawsweeper

clawsweeper Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@jrusz

jrusz commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Retrying unchanged exact head 35fde491743: the prior review generation run completed successfully and uploaded its artifact, but the durable review was not published during the ClawSweeper state-publication outage. The candidate-boundary correction, direct one-letter header regressions, Windows-path preservation coverage, PR body, and 25/25 focused test proof remain current.

@clawsweeper

clawsweeper Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: write tool and exec heredocs insert literal \n instead of newlines in string content

2 participants