Add JSON repair + review ground-truth worktree diff

[inhaq]

This pull request was created by @kiro-agent on behalf of @inhaq 👻

Comment with /kiro fix to address specific feedback or /kiro all to address everything.
_{Learn about Kiro Web}

---

Two self-contained robustness/correctness improvements to the actor-critic engine, derived from studying the earendil-works/pi agent harness. Both are additive — no engine, scheduler, or role-interface changes.

1. Robust JSON repair (borrowed from pi)

Ports pi's conservative JSON string-literal repair (MIT, from @earendil-works/pi packages/ai/src/utils/json-parse.ts, trimmed to the non-streaming path — no partial-json dependency added) into src/engine/jsonExtract.ts, and applies it per candidate in parseLastValidJson.

Model output is frequently almost valid JSON — a raw newline/tab inside a string, or an invalid backslash escape (e.g. a Windows path). Stock JSON.parse rejects these, which previously cost a full repair-retry round-trip (the PARSE_REPAIR_NUDGE in runnerRoles.ts and the critic retry in loop.ts) and often ended in NEEDS_HUMAN. repairJson only rewrites characters stock JSON would reject, so already-valid input is returned byte-for-byte.

2. Review the ground-truth worktree diff, not the model's self-reported diff

The critic reviewed built.diff (what the model claimed it changed), but the worktree contents on disk are what actually get committed and integrated — so a file-editing actor's reviewed artifact could diverge from the integrated artifact.

• Adds worktreeDiff() to src/workspace/git.ts: captures the real unified diff including newly created files (via add -A --intent-to-add then git diff HEAD, with a git diff fallback). Never throws.
• Adds an optional captureDiff seam to LoopDeps (src/engine/loop.ts), read right after build and before the mechanical gate (so build/test artifacts don't pollute the diff). When it returns a non-empty diff, that becomes the artifact the critic reviews; otherwise it falls back to the model-reported diff.
• Wires captureDiff in src/session.ts for worktree runs.

Testing

• New test/jsonExtract.test.ts (10 tests): repair of raw control chars, invalid escapes, preserved \uXXXX, byte-for-byte no-op on valid JSON, last-valid-object preference, and original-error rethrow.
• test/git.test.ts: worktreeDiff captures modified + newly created files and returns empty on a non-repo.
• test/loop.test.ts: captureDiff overrides the model diff (critic sees ground truth) and falls back on empty/throw.
• npm run typecheck clean; full suite 188 tests pass.

Limitations / follow-ups

• captureDiff overrides the reviewed diff only; touchedFiles is still model-reported (informational in the bundle). Deriving it from git diff --name-only is a small follow-up.
• Non-editing runners (HTTP/API models that return a diff but don't write files) are unchanged here — they keep surfacing their claimed diff. The natural next step is a native kind:"agent" runner (pi agent-core + ai) that actually edits the worktree.

Summary by CodeRabbit

• New Features

  • Added worktree-based diff capture to improve change reporting accuracy.
  • Introduced an optional hook to capture ground-truth diffs during task execution when available.

• Bug Fixes

  • Improved JSON extraction robustness by automatically repairing common malformed JSON before parsing and schema validation.

• Tests

  • Added coverage for JSON repair/parsing behavior and verified worktree diff capture, including fallback scenarios when capture fails or returns empty results.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 334bb3b6-125f-49c8-948d-7581e5d82095

📥 Commits

Reviewing files that changed from the base of the PR and between fffa516 and 561ee1f.

📒 Files selected for processing (1)

• src/engine/jsonExtract.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/engine/jsonExtract.ts

---

📝 Walkthrough

Walkthrough

The PR adds two independent improvements: a conservative JSON repair/parse fallback (repairJson, parseJsonWithRepair) in the extraction pipeline to recover "almost valid" JSON emitted by models, and a worktreeDiff/captureDiff pipeline that replaces the actor's self-reported diff with the real on-disk git worktree diff before the critic evaluates it.

Changes

JSON Repair and Parse Fallback

Layer / File(s)	Summary
repairJson/parseJsonWithRepair implementation, integration, and tests src/engine/jsonExtract.ts, test/jsonExtract.test.ts	Adds character-level repair helpers, repairJson (escapes raw control characters and corrects invalid escape sequences), and parseJsonWithRepair (strict parse first, repair-and-retry second). parseLastValidJson is updated to use parseJsonWithRepair. File-level docs and a full Vitest suite covering valid passthrough, repair cases, and parseLastValidJson multi-candidate selection are also added.

Ground-Truth Diff Capture Pipeline

Layer / File(s)	Summary
worktreeDiff git helper and tests src/workspace/git.ts, test/git.test.ts	Adds worktreeDiff(dir, exec?) which stages untracked files with --intent-to-add, runs git diff HEAD, falls back to git diff on unborn branch, and returns an empty string on any error. Two new tests cover the tracked+untracked file case and non-repo safety.
captureDiff hook in LoopDeps and BUILDING state src/engine/loop.ts, test/loop.test.ts	Adds optional `captureDiff?: (cwd) => Promise
Session wiring src/session.ts	Imports worktreeDiff and assigns schedulerExtra.captureDiff in the worktree-enabled branch of runGoal to supply real on-disk diffs to the task loop.

Sequence Diagram(s)

sequenceDiagram
  participant Session as session.ts runGoal
  participant RunTask as runTask BUILDING
  participant WorktreeDiff as worktreeDiff
  participant MechanicalGate
  participant Critic

  Session->>RunTask: runTask(deps) with captureDiff wired
  RunTask->>RunTask: actor produces built.diff
  RunTask->>WorktreeDiff: worktreeDiff(taskCwd, gitExec)
  alt non-empty ground-truth diff
    WorktreeDiff-->>RunTask: real diff string
    RunTask->>RunTask: lastDiff = captured diff
  else empty or error
    WorktreeDiff-->>RunTask: empty or throws
    RunTask->>RunTask: lastDiff = actor diff unchanged
  end
  RunTask->>MechanicalGate: evaluate lastDiff
  RunTask->>Critic: review(lastDiff)
  Critic-->>RunTask: GREEN or RED

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• inhaq/loopwright#1: Modifies LoopDeps and src/engine/loop.ts loop plumbing — directly connected to this PR's captureDiff extension of the same type.
• inhaq/loopwright#4: Introduces parseLastValidJson in src/engine/jsonExtract.ts — the exact function this PR extends with repairJson/parseJsonWithRepair.
• inhaq/loopwright#7: Implements the worktree-based scheduler in src/session.ts/runGoal — the same branch this PR extends to wire in captureDiff.

Poem

🐇 A rabbit once parsed a tangled JSON mess,
raw newlines in strings — what a terrible stress!
I snipped and I patched with a conservative hand,
then checked the git diff — the ground truth so grand.
Now models can fib, but the worktree won't lie,
the critic sees truth as the diffs flutter by! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely captures the two main changes: JSON repair functionality and ground-truth worktree diff review integration.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pi-harness-json-repair-and-diff-soundness

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/engine/jsonExtract.ts`:
- Around line 119-128: When the unicode escape validation at lines 120-126 fails
(nextChar is "u" but not followed by exactly four hexadecimal digits), the code
currently falls through to the VALID_JSON_ESCAPES check on line 127, which
incorrectly treats the invalid escape as valid. Add an else block after the
successful unicode escape handling to catch failed unicode escapes and
literalize them by escaping the backslash (add "\\\\" to repaired instead of
"\\") so that invalid sequences like "\uZZZZ" are properly converted to
"\\uZZZZ" in the output, preventing JSON.parse from failing.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 40e25e12-3c27-4708-994c-e3d66a7bf11f

📥 Commits

Reviewing files that changed from the base of the PR and between e9052a7 and fffa516.

📒 Files selected for processing (7)

• src/engine/jsonExtract.ts
• src/engine/loop.ts
• src/session.ts
• src/workspace/git.ts
• test/git.test.ts
• test/jsonExtract.test.ts
• test/loop.test.ts