Skip to content

fix(agents): recover wrapped Pi JSON output - #195

Merged
kunchenguid merged 5 commits into
kunchenguid:mainfrom
jasonqlwilliams-alt:fm/gnhf-pi-json-recovery-p1
Aug 11, 2026
Merged

fix(agents): recover wrapped Pi JSON output#195
kunchenguid merged 5 commits into
kunchenguid:mainfrom
jasonqlwilliams-alt:fm/gnhf-pi-json-recovery-p1

Conversation

@jasonqlwilliams-alt

Copy link
Copy Markdown
Contributor

Intent

Resolve GNHF issues #154 and #185 by routing the Pi adapter through the repository's shared JSON-recovery path instead of calling JSON.parse(finalText) directly. Review community PR #164 in full, including discussion and its failing Windows check, and reuse its sound implementation with contributor attribution preserved, correcting remaining defects only on this task branch. Pi must accept schema-valid JSON wrapped in Markdown fences and a prose prefix followed by the final schema-valid JSON object, with adapter-level regression tests for both. Existing Copilot, OpenCode, and other shared JSON-recovery behavior must remain green. Understand the Windows failure and either fix it or show with concrete evidence that it is unrelated. Produce a review-ready PR through the no-mistakes pipeline. Do not merge, publish a package, change release policy, waive checks, or modify the contributor's branch or PR.

What Changed

  • Route Pi responses through the shared schema-aware JSON recovery path, accepting valid payloads wrapped in Markdown fences or preceded by prose.
  • Consolidate duplicate Copilot and OpenCode recovery logic into the shared parseAgentOutput helper.
  • Add Pi adapter regression coverage for prose-prefixed and fenced JSON responses.

Risk Assessment

✅ Low: The change reuses the existing recovery logic without altering sibling semantics, adds behavioral Pi regressions for both required cases, preserves contributor attribution, and PR #164's Windows failure was confined to unchanged timeout-prone tests while all affected adapter suites passed.

Testing

Inspected the full target and community PR context, ran focused Pi/Copilot/OpenCode/shared-parser plus ACP/Rovo recovery tests, exercised both required Pi wrappers through the bundled CLI, reproduced the prose failure on the base build, recorded reviewer-visible evidence, and cleaned all generated worktree artifacts; everything passed with no actionable findings.

Evidence: Pi recovery end-to-end transcript
GNHF Pi JSON-recovery end-to-end evidence
Target: c68a596f2c4312c2ef086960f209e065322a4586
Base:   3041614ba7f45fc758eb156716a1343942b4a052

The mock `pi` executable was invoked by the bundled GNHF CLI using Pi's real
JSONL message_end protocol. It edited WORKLOG.md and returned the requested
AgentOutput in the indicated wrapper shape.

TARGET BUILD - prose prefix followed by final JSON
Command:
  gnhf "exercise Pi prose recovery" --agent pi --max-iterations 1 --prevent-sleep off
Observed exit summary:
  pi ran for 0s before: max iterations reached (1)
  iterations      1 total       1 good       0 failed
  tokens          123 in        45 out
  branch diff     1 commit      +1           -0
  files           1 added       0 updated    0 deleted
Observed commit history:
  007bdf8 gnhf 1: Recovered prose-prefixed Pi output
  9d0c001 init
Observed WORKLOG.md:
  Recovered prose-prefixed Pi output

TARGET BUILD - Markdown-fenced JSON
Command:
  gnhf "exercise Pi fenced recovery" --agent pi --max-iterations 1 --prevent-sleep off
Observed exit summary:
  pi ran for 0s before: max iterations reached (1)
  iterations      1 total       1 good       0 failed
  tokens          123 in        45 out
  branch diff     1 commit      +1           -0
  files           1 added       0 updated    0 deleted
Observed commit history:
  4b7e9a3 gnhf 1: Recovered fenced Pi output
  76098d5 init
Observed WORKLOG.md:
  Recovered fenced Pi output

BASE BUILD - same prose prefix followed by final JSON
Command:
  gnhf "exercise Pi prose recovery" --agent pi --max-iterations 1 --prevent-sleep off
Observed exit summary:
  pi ran for 0s before: Failed to parse pi output: Unexpected token 'I', "Iteration "... is not valid JSON
  iterations      1 total       0 good       1 failed
  tokens          123 in        45 out
  branch diff     0 commits     +0           -0
  files           0 added       0 updated    0 deleted

PR #164 Windows failure inspection
  Failed job: build-and-test (windows-latest), job 84188300892
  Failure shape: four timeouts in e2e/e2e-cli.test.ts and
  src/core/git.injection.test.ts.
  Pi adapter/parser tests did not fail in that job. Ubuntu and macOS passed.
  None of the timed-out test files were changed by the Pi JSON-recovery work.
- Evidence: [PR #164 failed Windows job](https://github.com/kunchenguid/gnhf/actions/runs/28320260845/job/84188300892)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • Inspected the target with git diff 3041614ba7f45fc758eb156716a1343942b4a052 c68a596f2c4312c2ef086960f209e065322a4586 and verified contributor-authored commits using git show -s --format=fuller.
  • Reviewed PR #164 using gh-axi pr view 164 --full --comments --reviews, gh-axi pr diff 164 --full, gh-axi pr checks 164, and gh-axi run view 28320260845 --job 84188300892 --log-failed.
  • pnpm exec vitest run src/core/agents/pi.test.ts src/core/agents/copilot.test.ts src/core/agents/opencode.test.ts src/core/agents/types.test.ts src/core/agents/json-extract.test.ts
  • pnpm exec vitest run src/core/agents/rovodev.test.ts src/core/agents/acp.test.ts -t "recovers JSON when rovodev|strips a leading|extracts a JSON object"
  • Built the target with pnpm run build.
  • Ran the bundled target CLI with a mock Pi message_end containing prose followed by schema-valid JSON; GNHF reported 1 good / 0 failed and committed Recovered prose-prefixed Pi output.
  • Ran the bundled target CLI with a mock Pi message_end containing Markdown-fenced schema-valid JSON; GNHF reported 1 good / 0 failed and committed Recovered fenced Pi output.
  • Archived and built base commit 3041614ba7f45fc758eb156716a1343942b4a052, then ran the identical prose-prefixed mock Pi stream; GNHF reproduced Failed to parse pi output, 0 good / 1 failed, and no commit.
  • Removed generated node_modules, dist, and the temporary base-build source, then verified the worktree remained clean.
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

cjunxiang and others added 5 commits August 10, 2026 12:08
PiAgent parsed its final assistant message with raw JSON.parse, unlike
every other adapter (acp, copilot, opencode, rovodev) which route through
parseAgentJson. When the pi agent prefixes a short prose summary before
the JSON object (e.g. "Iteration 3 complete. ..."), JSON.parse throws on
the first non-JSON token, every iteration fails, and gnhf hits its
maxConsecutiveFailures limit and aborts.

Route PiAgent through parseAgentJson with the schema validator as the
accept predicate and a no-predicate fallback (mirroring opencode), so
schema-invalid JSON still surfaces as "Invalid pi output" instead of
"Failed to parse pi output". Add a regression test for prose-then-JSON.

Fixes kunchenguid#154
The parseAgentJson + validateAgentOutput fallback shape was duplicated
identically in parsePiOutput, parseOpenCodeOutput, and parseCopilotOutput
(only the function name and the no-JSON error string differed). Extract a
single parseAgentOutput(text, schema, agentLabel) next to
validateAgentOutput in types.ts and route all three adapters through it.

Behavior is preserved: the per-agent SyntaxError message is retained via
agentLabel, and each adapter keeps its own caller-side error formatting
(pi distinguishes Failed vs Invalid; opencode/copilot use a single message).
@kunchenguid
kunchenguid merged commit f47d916 into kunchenguid:main Aug 11, 2026
5 checks passed
@kunchenguid

Copy link
Copy Markdown
Owner

Thanks @jasonqlwilliams-alt - merged! Really appreciate the contribution.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants