Bind direct review evidence to the frozen head SHA - #56
Merged
zuiho-kai merged 1 commit intoAug 9, 2026
Conversation
Direct reviews froze one head snapshot but never required the source files cited as evidence to be read at that commit. A reviewer holding a local checkout on another branch could quote stale code and still pass the completion gate, producing false findings against the PR head. Require the evidence binding at both ends: the review checklist and the imreview prompts now direct reading every cited file at the pinned head SHA (fetching the PR head ref when the checkout holds another revision), and validate_direct_review now takes a mandatory evidence_head_sha and returns partial_review without a plausible commit reference.
congw729
force-pushed
the
fix/imreview-evidence-head-sha
branch
from
August 7, 2026 18:42
8d0168d to
9f3655d
Compare
Contributor
Author
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
2 participants
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.
Fixes #55.
Problem
Direct reviews freeze one head snapshot, but nothing requires the source files cited as evidence to be read at that commit. With
REPO_PATHSpointing at a long-lived local checkout, a review of a PR that targets another branch can quote stale code and still passvalidate_direct_review. This produced a concrete false-positive P1 against vllm-project/vllm-omni#5890: the local checkout (onmain) showednum_cards=2for the duplex tests while the PR head (baseminicpm-challenge) declaresnum_cards=1, and the resulting finding asked for a hardware change the PR does not need. Details in #55.Changes
imreviewprompts (plugin skill and Cursor integration): replace the ambiguous "inspect the live code/target" with reading every cited file at the pinned head SHA, fetching the PR head ref (or reading files by ref) when the local checkout does not contain that commit, and never citing the working tree as evidence for a different revision. The final gate call now passes the pinned head SHA asevidence_head_sha.first_review_checklist: add the same evidence-binding rule right after the snapshot-freeze item, mirroring the existing rule that binds validation commands to the head SHA.validate_direct_review/_direct_completion_result: add a requiredevidence_head_shaargument (7-40 hex characters). A missing or implausible value returnspartial_review, same fail-closed shape as the subtraction gate. Like that gate, this checks structure rather than truth: it forces an explicit "which commit did I read?" declaration before the only final comment.completion_gatepayload and server instructions mention the new requirement; interface docs inREADME.mdanddocs/codex/README.mdupdated.Validation
pytest test/test_thin_mcp_server.py test/test_imreview_output_contract.py— 22 passed, including new cases for the missing/implausibleevidence_head_shapaths and the updated checklist/gate payloads.test_provider_selection_and_gapsandtest_repo_neutral_core, which fail identically on cleanmain(02e16c25) and are unrelated to this change.