feat(review): support reviewing GitHub PRs via --pr / focus auto-detect#2
Merged
JohnnyVicious merged 1 commit intomainfrom Apr 12, 2026
Merged
feat(review): support reviewing GitHub PRs via --pr / focus auto-detect#2JohnnyVicious merged 1 commit intomainfrom
JohnnyVicious merged 1 commit intomainfrom
Conversation
4 tasks
Previously, /opencode:review and /opencode:adversarial-review could only review the local working tree or a base-branch diff. A request like "/opencode:adversarial-review on PR #390" was silently misrouted: the companion still ran git diff against the local working tree, and the "on PR #390" string only entered the prompt as focus text. The reviewer ended up commenting on whatever happened to be in the local checkout. This change adds a real PR-fetch path: * `lib/git.mjs` gains `detectPrReference`, `getPrInfo`, and `getPrDiff` helpers that shell out to `gh pr view --json` and `gh pr diff`. They surface a clear error if `gh` is missing, unauthenticated, or pointed at the wrong remote. * `lib/prompts.mjs#buildReviewPrompt` accepts `opts.pr`. When set, it fetches PR metadata + diff via `gh` instead of reading the local working tree, swaps the target label to "Pull request #N (title)", and emits a `<pr_metadata>` block in the review context so the reviewer knows the base/head refs and stat counts. * `handleReview` and `handleAdversarialReview` accept `--pr <N>`. The adversarial handler additionally auto-detects "PR #N" / "PR N" / "pr #N" / "pr N" inside the focus text (case-insensitive) and strips the matched substring so it does not pollute the prompt. Explicit `--pr` always wins over auto-detection. * Both command markdown files document the new flag, mention the `gh` requirement, and add a PR-size estimation rule for the orchestrator. Apache License 2.0 §4(b) modification notices added to all three modified script files. Note: this branch and `fix/model-passthrough` both touch `valueOptions` in `handleReview`/`handleAdversarialReview`, so the second-merged branch will need a trivial conflict resolution (merge the two arrays).
JohnnyVicious
force-pushed
the
fix/pr-fetch
branch
from
7dbcaf4 to
3018652
Compare
JohnnyVicious
added a commit
that referenced
this pull request
Apr 12, 2026
`/opencode:adversarial-review on PR #389` was silently misrouted to a working-tree review because bash strips unquoted `#389` tokens as comments before they reach the companion script. The fix in PR #2 (detectPrReference + focus auto-detect in handleAdversarialReview) was correct in isolation but never ran — by the time the companion script saw argv, the `#389` had already been removed at the shell boundary, so the regex had nothing to match. The fix has to live at the slash-command markdown level, because that is where Claude reads the instructions about how to construct the bash command. Two complementary defenses: 1. **PR reference extraction (REQUIRED)**. New rule in both adversarial-review.md and review.md telling Claude to recognise `PR #N` / `pr #N` / `PR N` patterns in the user's input, extract the number, pass it as `--pr N`, and strip the matched phrase from any focus text. Includes a worked transformation example showing the wrong (bash-eats-the-comment) vs right (--pr extracted) form. 2. **Focus text quoting (REQUIRED, adversarial only)**. New rule in adversarial-review.md telling Claude to wrap focus text in single quotes when invoking bash, so other shell metacharacters (`*`, `$`, `;`, `&`, `|`, `<`, `>`, parens, backticks) survive. Includes guidance on the `'\''` escape for apostrophes. 3. Updated the foreground/background flow examples to stop showing `$ARGUMENTS` literally (which trained Claude to invoke bash without any of the above transformations) and instead show the illustrative shape `<flags> [--pr N] ['<quoted focus>']` with an explicit "transform $ARGUMENTS first" instruction. The companion script's focus-text PR auto-detect (detectPrReference) is intentionally kept as a fallback for cases where focus text reaches the script intact (e.g. someone calling the companion directly via bash with proper quoting, bypassing the slash command). No code or test changes — these are slash-command instructions read by Claude, not executed by node.
4 tasks
JohnnyVicious
added a commit
that referenced
this pull request
Apr 12, 2026
) `/opencode:adversarial-review on PR #389` was silently misrouted to a working-tree review because bash strips unquoted `#389` tokens as comments before they reach the companion script. The fix in PR #2 (detectPrReference + focus auto-detect in handleAdversarialReview) was correct in isolation but never ran — by the time the companion script saw argv, the `#389` had already been removed at the shell boundary, so the regex had nothing to match. The fix has to live at the slash-command markdown level, because that is where Claude reads the instructions about how to construct the bash command. Two complementary defenses: 1. **PR reference extraction (REQUIRED)**. New rule in both adversarial-review.md and review.md telling Claude to recognise `PR #N` / `pr #N` / `PR N` patterns in the user's input, extract the number, pass it as `--pr N`, and strip the matched phrase from any focus text. Includes a worked transformation example showing the wrong (bash-eats-the-comment) vs right (--pr extracted) form. 2. **Focus text quoting (REQUIRED, adversarial only)**. New rule in adversarial-review.md telling Claude to wrap focus text in single quotes when invoking bash, so other shell metacharacters (`*`, `$`, `;`, `&`, `|`, `<`, `>`, parens, backticks) survive. Includes guidance on the `'\''` escape for apostrophes. 3. Updated the foreground/background flow examples to stop showing `$ARGUMENTS` literally (which trained Claude to invoke bash without any of the above transformations) and instead show the illustrative shape `<flags> [--pr N] ['<quoted focus>']` with an explicit "transform $ARGUMENTS first" instruction. The companion script's focus-text PR auto-detect (detectPrReference) is intentionally kept as a fallback for cases where focus text reaches the script intact (e.g. someone calling the companion directly via bash with proper quoting, bypassing the slash command). No code or test changes — these are slash-command instructions read by Claude, not executed by node.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/opencode:reviewand/opencode:adversarial-review. Before this change, asking the reviewer to "review PR #390" silently fell back to a local working-tree review and treated the PR reference as focus text.lib/git.mjshelpersgetPrInfoandgetPrDiffshell out togh pr view --jsonandgh pr diff, with an actionable error ifghis missing, unauthenticated, or pointed at the wrong remote.buildReviewPromptacceptsopts.pr; when set it sources the diff and changed-file list fromghinstead ofgit, swaps the target label toPull request #N "title" (head -> base), and emits a<pr_metadata>block so the reviewer sees branch refs, URL, and stat counts.--pr <N>. The adversarial handler additionally auto-detectsPR #N/pr N(case-insensitive) inside the focus text viadetectPrReferenceand strips the matched substring so it doesn't end up in the prompt. Explicit--pralways wins over auto-detection.ghrequirement, and a PR-size estimation rule (gh pr view --json additions,deletions,changedFiles) for the orchestrator.Conflict note
This branch and #1 (
fix/model-passthrough) both touch thevalueOptionsarray inhandleReview/handleAdversarialReview. The second-merged branch will need a trivial conflict resolution: merge"model"and"pr"into the same array.Test plan
node --checkpasses foropencode-companion.mjs,lib/git.mjs,lib/prompts.mjsdetectPrReferencesmoke-tested against 10 input variants (positive:PR #390,pr 390, embedded references; negative:fix #123, plain text, empty/null)"on PR #390"->pr=390, focus"on";"review PR #42 for security issues"->pr=42, focus"review for security issues"/opencode:adversarial-review --wait --pr 390against a repo whoseoriginis on GitHub returns a review of the PR diff (not the local working tree)/opencode:adversarial-review --wait on PR #390triggers the same PR fetch via auto-detection/opencode:adversarial-review --wait --pr 999999(nonexistent PR) surfaces thegh pr viewerror cleanly/opencode:adversarial-review --wait some focus text(no PR ref) still reviews the working tree (regression check)