Skip to content

fix(review): hand off --post to Claude Code via stderr trailer#66

Merged
JohnnyVicious merged 1 commit intomainfrom
fix/pr-post-trailer-handoff
Apr 14, 2026
Merged

fix(review): hand off --post to Claude Code via stderr trailer#66
JohnnyVicious merged 1 commit intomainfrom
fix/pr-post-trailer-handoff

Conversation

@JohnnyVicious
Copy link
Copy Markdown
Owner

Symptom

Real use of `/opencode:adversarial-review --pr 412 --post` failed with:

[opencode-companion] Failed to post review to PR #412: Unterminated string in JSON at position 689 (line 1 column 690)

Root cause

v1.0.9 fetched PR files with `gh api --paginate`, which returns concatenated per-page JSON arrays. My reassembly heuristic split on `][` to find page boundaries, but that same byte pair can appear inside a legitimate `patch` field (any JS/Go/TS code with `foo][bar`). Splitting there cut the JSON mid-string and the re-parsed chunks were invalid. Position 689 is exactly where a `][` inside a patch happened to land.

Fix (two pieces)

1. Drop `--paginate` entirely.

Use `per_page=100`. No stream reassembly, no split heuristics. On a 100+ file PR, findings in the tail files degrade to summary-only via `splitFindings` — the same graceful fallback that already handles findings outside the diff.

2. Stop executing the POST from Node (user's pivot).

The companion now builds the payload, writes it to `$TMPDIR/opencode-plugin-cc/post-pr---.json`, and emits a tagged stderr trailer:

```
<opencode_post_instructions>
412
<inline_count>3</inline_count>
<summary_only_count>2</summary_only_count>
<payload_path>/tmp/opencode-plugin-cc/post-pr-412-….json</payload_path>
gh api -X POST "repos/{owner}/{repo}/pulls/412/reviews" --input '/tmp/…json'
rm -f '/tmp/…json'
</opencode_post_instructions>
```

The slash-command runner (Claude Code) parses the block, runs `` verbatim via its Bash tool, appends a one-line status like `Review posted to PR #412: <html_url>` to the review output, then runs ``. Companion never touches HTTP for posts.

Benefits:

  • No more JSON-stream reassembly bugs.
  • No more duplicated `gh` plumbing in Node (auth, pagination, error shapes).
  • Claude can show/confirm the payload before firing.
  • Failures surface in the chat where they're debuggable, not buried in a stderr line.

Slash command docs in `review.md` and `adversarial-review.md` spell the handoff out step-by-step so Claude treats it mechanically (parse → run → status line → cleanup), and explicitly forbid inventing a post when the block is absent.

Test plan

  • 30 unit tests in `tests/pr-comments.test.mjs`, including:
    • `shQuote` escapes embedded single quotes via `'\''`
    • `writePayloadFile` round-trips JSON and produces unique paths
    • `formatPostTrailer` emits all six expected tags
    • `preparePostInstructions` with an injected `prData` option that bypasses `gh` entirely — verifies the end-to-end payload contents (commit_id, event, summary body, inline comments array with correct line anchors) without any real network
    • Graceful `{ prepared: false, error }` when gh would fail
  • Full suite: `npm test` → 252/252 pass.
  • Manual smoke test on a real PR (next session).

…iler

v1.0.9 shipped a companion-internal `gh api` POST path that fetched the
PR file list with `gh api --paginate` and re-assembled the concatenated
page JSONs by string-splitting on `][`. Any patch whose content
legitimately contains `][` (very common in JS/Go/TS code) corrupted the
reassembled JSON mid-string and the post failed with:

    Failed to post review to PR #412: Unterminated string in JSON at
    position 689

Fix the root cause and the architectural mistake together:

1. Drop `gh api --paginate` entirely. Use `per_page=100`, which covers
   the vast majority of real PRs in a single request — no stream
   reassembly, no split heuristics. On a 100+ file PR, findings whose
   files are past the first page degrade to summary-only via the
   existing `splitFindings` logic, which is already the graceful
   fallback path.

2. Stop executing the `POST /reviews` call from Node. The companion
   now builds the full review payload (summary body + inline comments
   for high-confidence findings addressable on the PR diff), writes it
   to a temp file under `$TMPDIR/opencode-plugin-cc/`, and emits a
   structured trailer on stderr:

       <opencode_post_instructions>
         <pr>412</pr>
         <inline_count>3</inline_count>
         <summary_only_count>2</summary_only_count>
         <payload_path>…</payload_path>
         <command>gh api -X POST "repos/{owner}/{repo}/pulls/412/reviews" --input '…'</command>
         <cleanup>rm -f '…'</cleanup>
       </opencode_post_instructions>

   The slash-command runner (Claude Code) reads the trailer, runs the
   exact `<command>` via its Bash tool, parses the GitHub response for
   `html_url`, appends a one-line status to the review output, and
   runs `<cleanup>`. The companion never touches HTTP for posts.

   Benefits: no more JSON-stream-reassembly bugs, no more duplicated
   gh plumbing in Node, Claude can show/confirm the payload before it
   fires, failures land in the chat where they're debuggable instead
   of buried in a stderr line.

Slash-command docs in `review.md` and `adversarial-review.md` now
describe the handoff mechanically: parse stderr for the block, run the
command verbatim, append a status line, run cleanup. The flow works
identically for foreground and background reviews (on `run_in_background:
true`, Claude just sees the trailer later when it reads `BashOutput`).

Tests: all 30 pr-comments cases pass, including new coverage for
`shQuote`, `writePayloadFile`, `formatPostTrailer`, and
`preparePostInstructions` (which accepts an injected `prData` option so
tests bypass `gh` entirely). Full suite 252/252.
@JohnnyVicious JohnnyVicious merged commit e96d7e3 into main Apr 14, 2026
1 check passed
@JohnnyVicious JohnnyVicious deleted the fix/pr-post-trailer-handoff branch April 14, 2026 18:39
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.

1 participant