fix(review): pin PR code reads to the head SHA, not the default branch - #2100
Conversation
The review panel read PR files with `github_read_file`, whose `ref` is optional and defaults to the repo's DEFAULT BRANCH. Omit it and you read the code as it was BEFORE the PR — so the verifier "confirms" that symbols the PR ADDS don't exist, and a correct PR collects blocker findings. Observed on protoAgent#2088: the verifier reported `goalDetailQuery` missing from apps/web/src/lib/queries.ts; it is exported at line 90 of the reviewed head. #2094 drew 8 → 9 → 11 findings across three re-reviews about code the PR never touched (recalled prior_findings re-seed each panel, so a wrong-ref finding compounds). Prompt-level discipline was ALREADY here and did not hold: both the review-finder and verifier prompts said "pass ref=<head SHA>", the recipe passed the SHA to every step, and the reads still went to main. A rule the model has to remember on every call is not a guarantee. Both review seats now use github-plugin's `github_read_pr_file` (v0.3.0), which takes no ref at all — it resolves the head SHA server-side from the PR number. `github_read_file` stays for policy docs, which must still be read at the base (a PR must not rewrite the rules it is judged by), and the verifier now fails closed: an unreadable head means "uncertain", never "confirmed". Refs pr-reviewer-plugin#20, protoLabsAI/github-plugin#22. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![protoreview[bot]](https://avatars.githubusercontent.com/in/4233969?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA panel review — PASS
code-review · head 04ab9ffffcd2 · formal
[review-synthesizer completed: workflow code-review:report]
The review panel identified no defects across the provided findings lists, and the verifier confirmed no issues. The overall risk is low as no correctness, security, or behavioral regressions were detected. There are no items to fix.
[]
…, section order (#2108) Release prep for the 25 commits sitting on main since v0.105.2. CHANGELOG [Unreleased] was missing four user-facing changes and had **two separate `### Fixed` sections**, which would have rolled into a malformed release section. Merged them, reordered to Keep a Changelog's Added/Changed/Fixed, and added entries for: - the watch-tool gating (#2099) — a DEFAULT-OFF behavior change, so it gets an explicit "existing configs must set `watches.enabled: true`" note - the PR-review head-SHA fix (#2100) - the Tools-panel Filesystem group chips (#2084) - the PWA theme-color fix (#2083) ADRs and CI-internal commits are deliberately not listed — this repo cites ADRs inline in the entry they belong to rather than as standalone lines. `watches.enabled` shipped with NO documentation outside a commented block in the example config, while simultaneously flipping the tools to default-off. Added `## watches` to the configuration reference and a "Turn it on first" section to the watches guide, both stating the three things the flag deliberately does not do — it never deletes stored watches, the background poller keeps running, and `goal.enabled` must ALSO be true because the tools ride inside the goal tool group. Wording checked against the schema comment in `graph/config.py` and the example config so the three don't drift. Co-authored-by: GitHub CI <ci@example.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Fixes the panel half of pr-reviewer-plugin#20. Pairs with github-plugin#22 (merged, v0.3.0), which adds the tool this uses.
The bug
The review panel reads PR files with
github_read_file, whoserefis optional and defaults to the repo's default branch. Omit it and you read the code as it was before the PR — so the verifier "confirms" that symbols the PR adds don't exist, and a correct PR collects blocker findings.Reproduced on #2088: the verifier reported
goalDetailQuerymissing fromapps/web/src/lib/queries.ts. At the reviewed head19c37f7ait is exported at line 90 — it readmain.#2094 shows it compounds: three re-reviews produced 8 → 9 → 11 findings about code the PR never touched. Recalled
prior_findingsre-seed each panel, so a wrong-ref finding breeds.Why not just fix the prompt
Because that was already the fix, and it didn't hold:
review-finder: "Ref discipline: a plaingithub_read_fileshows the DEFAULT branch … passref=<head SHA>"verifier: the same rulehead_shato all four finders and the verify stepEvery layer was correct and the reads still went to
main. A rule the model must remember on each of ~40 tool calls isn't a guarantee — so this removes the choice.The change
Both review seats now use
github_read_pr_file(PR number + path), which takes no ref: the head SHA is resolved server-side from the PR, and a failed pinned read errors rather than falling back.github_read_filestays available and is still the right tool for policy docs (CLAUDE.md/PROTO.md), which must be read at the base — a PR must not rewrite the rules it's judged by. The verifier also now fails closed: if it can't read a file at the PR head, the verdict isuncertain, neverconfirmed.Test
27 passed (
test_code_review_workflow.py,test_sdk_subagent_tools.py).The old test asserted the prompt contained
ref=<head SHA>— that assertion was passing while production emitted false blockers, which is the whole lesson. Replaced with assertions on the enforced behaviour: both seats holdgithub_read_pr_file, the finder is told never to usegithub_read_filefor code context, and the verifier carries the fail-closed rule.Deploy note
Vera (the QA reviewer) is pinned to base
0.104.5, so she picks this up on the next protoAgent release. Her re-gate is currently disabled (regate: false, pr-reviewer-plugin v0.7.1) so these false FAILs can't block merges in the meantime.🤖 Generated with Claude Code