From 9f3655dd31b5b199d4a8b608d59234cd4771c339 Mon Sep 17 00:00:00 2001 From: WANG Cong <115451386+congw729@users.noreply.github.com> Date: Sat, 8 Aug 2026 02:36:18 +0800 Subject: [PATCH] Bind direct review evidence to the frozen head SHA 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. --- README.md | 4 +- docs/codex/README.md | 5 +- integrations/cursor/imreview.md | 10 ++-- .../skills/imreview/SKILL.md | 10 ++-- src/infermatrix_copilot/thin_mcp_server.py | 33 +++++++++++-- test/test_imreview_output_contract.py | 4 ++ test/test_thin_mcp_server.py | 49 +++++++++++++++++-- 7 files changed, 99 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 3576800..42b4822 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,9 @@ release_baseline.yaml 的 audited_sha Direct 根据 PR 描述返回至多 3 个精确知识路由和审查门禁；Strict 返回 `run_id`。`repo_path` 可临时指定 Strict 使用的本地 checkout。 - `validate_direct_review(subtraction_signal, subtraction?, - minimality_proof?, final_comment_count=1)`：Direct 最终输出前的完成检查。 + minimality_proof?, final_comment_count=1, evidence_head_sha)`：Direct 最终 + 输出前的完成检查。`evidence_head_sha` 必须是本次审查固定的 head 提交， + 证明引用的源码和验证结果都读取自该版本而非本地工作区的其他分支。 普通小修改传 `subtraction_signal="none"`；存在新增或扩张结构时传 `"triggered"`，并提供减法项或最小性证明。 - `get_review_status(run_id)`：查看 Strict 后台任务的步骤和进度。 diff --git a/docs/codex/README.md b/docs/codex/README.md index 58de0a5..2766d16 100644 --- a/docs/codex/README.md +++ b/docs/codex/README.md @@ -159,7 +159,10 @@ Restart Codex after installation. Use `/mcp` or `codex mcp list` to confirm that Strict starts the packaged workflow and accepts `review_depth` plus an optional local checkout override through `repo_path`. - `validate_direct_review(subtraction_signal, subtraction?, minimality_proof?, - final_comment_count=1)`: `none` completes an ordinary small fix without a + final_comment_count=1, evidence_head_sha)`: `evidence_head_sha` must be the + frozen head commit every cited source file and validation result was read + at; evidence read from another local revision does not complete the review. + `none` completes an ordinary small fix without a minimality proof. `triggered` requires anchored subtraction actions or concrete evidence that the inspected scope is already minimal. - `update_knowledge(repo?)`: keeps `repo` only for call compatibility and diff --git a/integrations/cursor/imreview.md b/integrations/cursor/imreview.md index a325ae7..f3d37d5 100644 --- a/integrations/cursor/imreview.md +++ b/integrations/cursor/imreview.md @@ -10,8 +10,11 @@ call `review` once with `quick_map` in each returned `knowledge_routes` item. Do not open the full route file unless a concrete ambiguity blocks source review, and do not walk `AGENTS.md`, `CLAUDE.md`, repository indexes, or model catalogs. Inspect the -live target and return only evidence-backed findings with file and line -references. +target at the pinned head SHA and return only evidence-backed findings with +file and line references. Read every file cited as evidence at that commit; +when the local checkout does not contain it, fetch the PR head ref or read +files by ref, and never cite the working tree as evidence for a different +revision. Follow the returned `execution_budget`. Extend it once only when a concrete unresolved P1/high-risk contract remains, and state that question before extending. @@ -35,7 +38,8 @@ Do not wait for CI completion or resolved mergeability before the progress update. Mark early findings as preliminary and continue the review. This update is not a GitHub comment; do not post an interim review. -Before finalizing, classify `subtraction_signal`. Use `none` without a +Before finalizing, call `validate_direct_review` with the pinned head SHA as +`evidence_head_sha` and classify `subtraction_signal`. Use `none` without a minimality proof when the diff does not add or expand a helper, class, fallback, compatibility branch, or public behavior. Use `triggered` for those changes and only then provide subtraction evidence. diff --git a/plugins/infermatrix-copilot/skills/imreview/SKILL.md b/plugins/infermatrix-copilot/skills/imreview/SKILL.md index a0f2166..82bd8a2 100644 --- a/plugins/infermatrix-copilot/skills/imreview/SKILL.md +++ b/plugins/infermatrix-copilot/skills/imreview/SKILL.md @@ -13,8 +13,11 @@ with `mode="direct"` plus the collected `title`, `body`, and `changed_files`. Use the embedded `quick_map` in each returned `knowledge_routes` item. Do not open the full route file unless a concrete ambiguity blocks source review, and do not reopen `AGENTS.md`, -`CLAUDE.md`, repo indexes, or model catalogs. Inspect the live code and return -only evidence-backed findings with file/line references. +`CLAUDE.md`, repo indexes, or model catalogs. Inspect the code at the pinned +head SHA and return only evidence-backed findings with file/line references. +Read every file cited as evidence at that commit; when the local checkout does +not contain it, fetch the PR head ref or read files by ref, and never cite the +working tree as evidence for a different revision. Treat the returned `execution_budget` as a hard ceiling. At the limit, return the supported verdict plus any remaining validation gap. Extend it once by the returned allowance only when a concrete unresolved P1/high-risk contract @@ -36,7 +39,8 @@ no-issue conclusion; do not add searches only for confidence. Do not wait for CI completion or resolved mergeability before the progress update. Mark early findings as preliminary and continue the review. This update is not a GitHub comment; do not post an interim review. -Before finalizing, classify `subtraction_signal`. Use `none` without a +Before finalizing, call `validate_direct_review` with the pinned head SHA as +`evidence_head_sha` and classify `subtraction_signal`. Use `none` without a minimality proof when the diff does not add or expand a helper, class, fallback, compatibility branch, or public behavior. Use `triggered` for those changes and only then provide subtraction evidence. diff --git a/src/infermatrix_copilot/thin_mcp_server.py b/src/infermatrix_copilot/thin_mcp_server.py index 5c12c1d..a33c422 100644 --- a/src/infermatrix_copilot/thin_mcp_server.py +++ b/src/infermatrix_copilot/thin_mcp_server.py @@ -172,6 +172,7 @@ def _knowledge_root() -> Path: ) _DIRECT_REVIEW_CHECKLIST = [ "Freeze one base/head snapshot and collect PR intent, diff, mergeability, and CI once.", + "Read every source file cited as evidence at the frozen head SHA; when the local checkout does not contain that commit, fetch the PR head ref or read files by ref instead of trusting the working tree.", "Immediately after snapshot metadata returns, report head SHA, CI, mergeability, and preliminary findings in the host conversation; do this before reading knowledge, searching source, or running tests.", "Call Direct once with the collected title, body, and changed_files; read only the returned knowledge_routes and stop knowledge navigation.", "After the progress update, run independent knowledge/source and validation tracks concurrently.", @@ -208,6 +209,7 @@ def _knowledge_root() -> Path: } _SUBTRACTION_ACTIONS = {"DELETE", "DEFER", "INLINE", "MERGE", "MOVE"} _SUBTRACTION_SIGNALS = {"none", "triggered"} +_EVIDENCE_HEAD_SHA = re.compile(r"[0-9a-f]{7,40}") def _normalize_repo(repo: str) -> str: @@ -559,6 +561,7 @@ def _direct_completion_result( subtraction: list[dict[str, str]] | None = None, minimality_proof: dict[str, str] | None = None, final_comment_count: int = 1, + evidence_head_sha: str = "", ) -> dict: """Mechanically gate Direct completion on subtraction classification. @@ -566,10 +569,14 @@ def _direct_completion_result( true. A diff without a subtraction trigger can finish after explicitly declaring ``none``. Triggered diffs still need actionable subtraction or concrete evidence that the inspected scope is already minimal. + ``evidence_head_sha`` forces an explicit declaration that every cited + source file and validation result was read at the frozen head commit, + not at whatever revision the local working tree happened to hold. """ subtraction_signal = str(subtraction_signal).strip().casefold() subtraction = subtraction or [] minimality_proof = minimality_proof or {} + evidence_head_sha = str(evidence_head_sha).strip().casefold() missing: list[str] = [] if final_comment_count != 1: @@ -578,6 +585,13 @@ def _direct_completion_result( if subtraction_signal not in _SUBTRACTION_SIGNALS: missing.append("subtraction_signal must be 'none' or 'triggered'") + if not _EVIDENCE_HEAD_SHA.fullmatch(evidence_head_sha): + missing.append( + "evidence_head_sha must be the frozen head commit SHA " + "(7-40 hex characters) that every cited source file and " + "validation result was read at" + ) + malformed_subtractions: list[int] = [] for index, item in enumerate(subtraction): if not isinstance(item, dict): @@ -633,6 +647,7 @@ def _direct_completion_result( "status": "complete" if complete else "partial_review", "publish_ready": complete, "final_comment_count": final_comment_count, + "evidence_head_sha": evidence_head_sha, "subtraction_signal": subtraction_signal, "subtraction_required": subtraction_signal == "triggered", "subtraction_items": len(subtraction), @@ -712,9 +727,12 @@ def build_mcp( "verdict at the limit. Extend it once by the returned allowance " "only for a stated unresolved P1/high-risk contract. Continue " "reviewing without " - "posting an early GitHub comment. Before treating a Direct review as " - "complete or posting its only final comment, call " - "validate_direct_review. Mark subtraction_signal=none when the diff " + "posting an early GitHub comment. Read every source file cited as " + "evidence at the frozen head SHA; fetch the PR head ref when the " + "local checkout holds another revision. Before treating a Direct " + "review as complete or posting its only final comment, call " + "validate_direct_review with that evidence_head_sha. Mark " + "subtraction_signal=none when the diff " "does not add or expand a helper, class, fallback, compatibility " "branch, or public behavior. Only subtraction_signal=triggered " "requires subtraction evidence. " @@ -799,6 +817,7 @@ def run() -> dict: }, "completion_gate": { "tool": "validate_direct_review", + "evidence_head_sha": "Required: the frozen head commit SHA every cited source file and validation result was read at; fetch the PR head ref when the local checkout holds another revision.", "subtraction_signal": { "none": "No helper/class/fallback/compatibility/public-behavior expansion; no subtraction evidence required.", "triggered": "Require subtraction items or minimality_proof.", @@ -856,14 +875,17 @@ def validate_direct_review( subtraction: list[dict[str, str]] | None = None, minimality_proof: dict[str, str] | None = None, final_comment_count: int = 1, + evidence_head_sha: str = "", ) -> dict: """Validate the Direct completion gate before the only final comment. Use ``subtraction_signal='none'`` when the diff does not add or expand a helper, class, fallback, compatibility branch, or public behavior. Use ``'triggered'`` for those diffs, then supply actionable subtraction - items or concrete minimality evidence. A ``partial_review`` result is - not a completed Direct review. + items or concrete minimality evidence. Pass ``evidence_head_sha`` as + the frozen head commit every cited source file and validation result + was read at; a review whose evidence came from another revision is not + complete. A ``partial_review`` result is not a completed Direct review. """ started = time.perf_counter() result = _direct_completion_result( @@ -871,6 +893,7 @@ def validate_direct_review( subtraction=subtraction, minimality_proof=minimality_proof, final_comment_count=final_comment_count, + evidence_head_sha=evidence_head_sha, ) result.setdefault("diagnostics", {})["timing_ms"] = { "validate_direct_review": int( diff --git a/test/test_imreview_output_contract.py b/test/test_imreview_output_contract.py index ed53d7b..4cd29ba 100644 --- a/test/test_imreview_output_contract.py +++ b/test/test_imreview_output_contract.py @@ -36,3 +36,7 @@ def test_imreview_returns_github_style_findings(prompt_path: Path) -> None: assert "bounded `rg` searches" in prompt assert "import/version compatibility preflight" in prompt assert "head SHA and an environment fingerprint" in prompt + assert "at the pinned head SHA" in prompt + assert "fetch the PR head ref" in prompt + assert "never cite the working tree as evidence" in prompt + assert "`evidence_head_sha`" in prompt diff --git a/test/test_thin_mcp_server.py b/test/test_thin_mcp_server.py index 7f5e912..719d6f4 100644 --- a/test/test_thin_mcp_server.py +++ b/test/test_thin_mcp_server.py @@ -127,6 +127,11 @@ def test_direct_entrypoints_do_not_resolve_repo(monkeypatch): and "environment fingerprint" in item for item in review["first_review_checklist"] ) + assert any( + "at the frozen head SHA" in item + and "fetch the PR head ref" in item + for item in review["first_review_checklist"] + ) assert review["progress_update"] == { "deadline_seconds": 60, "channel": "host_conversation", @@ -151,6 +156,7 @@ def test_direct_entrypoints_do_not_resolve_repo(monkeypatch): } assert review["completion_gate"] == { "tool": "validate_direct_review", + "evidence_head_sha": "Required: the frozen head commit SHA every cited source file and validation result was read at; fetch the PR head ref when the local checkout holds another revision.", "subtraction_signal": { "none": "No helper/class/fallback/compatibility/public-behavior expansion; no subtraction evidence required.", "triggered": "Require subtraction items or minimality_proof.", @@ -294,8 +300,11 @@ def test_direct_routes_model_rules_without_index_navigation(): assert all(route["quick_map"] for route in routing["routes"]) +_HEAD_SHA = "1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b" + + def test_direct_completion_requires_subtraction_signal_classification(): - result = _direct_completion_result() + result = _direct_completion_result(evidence_head_sha=_HEAD_SHA) assert result["status"] == "partial_review" assert result["publish_ready"] is False @@ -304,17 +313,46 @@ def test_direct_completion_requires_subtraction_signal_classification(): ] -def test_small_fix_accepts_no_subtraction_signal_without_full_proof(): +def test_direct_completion_requires_evidence_head_sha(): result = _direct_completion_result(subtraction_signal="none") + assert result["status"] == "partial_review" + assert result["publish_ready"] is False + assert result["missing"] == [ + "evidence_head_sha must be the frozen head commit SHA " + "(7-40 hex characters) that every cited source file and " + "validation result was read at" + ] + + +def test_direct_completion_rejects_non_commit_evidence_reference(): + result = _direct_completion_result( + subtraction_signal="none", + evidence_head_sha="main", + ) + + assert result["status"] == "partial_review" + assert result["publish_ready"] is False + + +def test_small_fix_accepts_no_subtraction_signal_without_full_proof(): + result = _direct_completion_result( + subtraction_signal="none", + evidence_head_sha=_HEAD_SHA, + ) + assert result["status"] == "complete" assert result["publish_ready"] is True assert result["subtraction_required"] is False assert result["subtraction_items"] == 0 + assert result["evidence_head_sha"] == _HEAD_SHA def test_triggered_subtraction_requires_evidence(): - result = _direct_completion_result(subtraction_signal="triggered") + result = _direct_completion_result( + subtraction_signal="triggered", + evidence_head_sha=_HEAD_SHA, + ) assert result["status"] == "partial_review" assert result["publish_ready"] is False @@ -329,6 +367,7 @@ def test_triggered_subtraction_requires_evidence(): def test_issue_5559_trigger_accepts_single_comment_with_subtractions(): result = _direct_completion_result( subtraction_signal="triggered", + evidence_head_sha=_HEAD_SHA, subtraction=[ { "anchor": "examples/offline_inference/text_to_image/text_to_image.py:358", @@ -353,6 +392,7 @@ def test_issue_5559_trigger_accepts_single_comment_with_subtractions(): def test_direct_completion_accepts_concrete_minimality_proof(): result = _direct_completion_result( subtraction_signal="triggered", + evidence_head_sha=_HEAD_SHA, minimality_proof={ "scope_ledger": "Every changed production file maps to the requested API fix.", "abstraction_census": "No new helper, class, projection, fallback, or compatibility branch.", @@ -368,6 +408,7 @@ def test_direct_completion_accepts_concrete_minimality_proof(): def test_direct_completion_rejects_malformed_subtraction_and_two_comments(): result = _direct_completion_result( subtraction_signal="triggered", + evidence_head_sha=_HEAD_SHA, subtraction=[{ "anchor": "examples/task.py:120", "action": "", @@ -384,6 +425,7 @@ def test_direct_completion_rejects_malformed_subtraction_and_two_comments(): def test_direct_completion_does_not_count_bug_fixes_as_subtraction(): result = _direct_completion_result( subtraction_signal="triggered", + evidence_head_sha=_HEAD_SHA, subtraction=[{ "anchor": "src/adapter.py:42", "action": "FIX the incorrect default value", @@ -398,6 +440,7 @@ def test_direct_completion_does_not_count_bug_fixes_as_subtraction(): def test_no_signal_rejects_contradictory_subtraction_evidence(): result = _direct_completion_result( subtraction_signal="none", + evidence_head_sha=_HEAD_SHA, subtraction=[{ "anchor": "src/adapter.py:42", "action": "DELETE unused compatibility branch",