diff --git a/docs/doctoring/opencode-peer-check-reevaluation.md b/docs/doctoring/opencode-peer-check-reevaluation.md new file mode 100644 index 0000000000..6d851eb188 --- /dev/null +++ b/docs/doctoring/opencode-peer-check-reevaluation.md @@ -0,0 +1,170 @@ +# OpenCode peer-check fallback reevaluation + +Status: Proposed. Owner issue: #2125. + +Protected main `fb17ef556f94f673234aa557254ae52779e9a7b0` accepted a +current-head CHANGES_REQUESTED review carrying its canonical model-unavailable +fallback marker as a substantive receipt. The required caller consequently +skipped another review after peer CodeQL checks recovered (#2113, +job 103581933696). + +The receipt helper matches the complete canonical failed-peer-check payload: +fixed producer prose, the current head SHA, failed-check rows, and an optional +generated Mermaid evidence map. Extra prose, mixed findings, and unknown formats +remain formal blockers; a heading or fallback marker cannot remove a product +finding. A diagram heading alone cannot hide additional prose. A newer fallback blocks reuse of an older same-head +receipt; a later substantive product finding still deduplicates normally. +Approval eligibility and downstream gates are unchanged. This does not approve +any PR or establish model availability. PR #1706 edits a separate verdict lookup +in the same caller; this repair changes only the shared receipt helper. + +Regression: six fallback-marker cases failed before the change; a substantive +finding case already passed. The receipt, live-draft and required-verdict tests +exercise the shared boundary and existing caller behavior. Actual fresh receiver +execution and a formal current-head review remain deployment acceptance gates. + +## Concurrent owner integration + +Ordinary merge preserves both aa6150c and 96704bc7 histories. The remote +substantive-finding repair's original mixed-review case is retained as an +additional regression. Its broad heading regex is superseded by the exact +producer envelope because unknown headings must remain blockers, not silently +qualify for reevaluation. No approval criterion is relaxed. + +### Historical remote-branch validation (not merged-head proof) + +Regression evidence is bound to exact commits. Commit +`9cd835df682b74ead1e5306b92280da941e45040` is RED because a mixed fallback +and authorization finding returns no receipt. Commits +`7b528279720af353ba91fbff5d43384ad466bb8b` and +`f5be0fb8f21d9fad64c8576b5e980d5eeb9a1b1e` introduce and correct the +minimal finding-heading classifier. On the corrected source blob +`310f05d956382d4ba525907ccba64c633c1c2a6c` and test blob +`05955d2e2181aefe957f239d8aae8012a0ad04d9`, the receipt, +live-Draft, required-verdict, and coverage-publication suites report 86 passed; +Ruff and compileall are green. Hosted exact-head checks, independent review, and +a fresh #2113 receiver/formal review remain deployment acceptance gates. + +## Unstructured finding regression + +The db8058e9 test exposes a product finding appended as ordinary prose beneath +the canonical heading. The former heading-only classifier discarded it. The +replacement matches the full producer envelope and fails closed on added prose, +including text after the diagram. Tests execute the actual producer printf block +and the graph emitter rather than reproducing shortened synthetic envelopes. +Receipt-module validation: 25 passed, 100% statements and branches; hosted +review and deployment evidence remain outstanding. + +## Exact historical coverage-only refresh (#2125 / #2126) + +The authenticated public review `5301203359` is preserved byte-for-byte in +`tests/fixtures/opencode_legacy_coverage_review5301203359.json`. Its historical +producer is `e6334e2` (`opencode_review_surfaces.py` and the dispatch publisher). +It contains the complete changed-file walkthrough, generated behavior graph, +fixed no-product-finding prose, exact head/run/attempt identity, failed coverage, +fixed review outcome, and appended evidence map. Protected production now posts +this fallback as COMMENT; this repair does not alter publication or workflows. + +`opencode_legacy_coverage_fallback.py` shares exact classification between the +receipt helper and scheduler retry predicate. It requires an existing trusted +OpenCode login, matching full commit/body/current head, and positive decimal run +and attempt. It reconstructs file-role labels and both graphs from bounded paths +using the producer, then compares the entire envelope. The appended map may use +the producer's ordinary or merge-conflict graph; arbitrary Mermaid nodes, changed +labels, extra prose, mixed findings, malformed identities, unsupported format or +source-root-dependent API diagrams are not exempted. No repository/head/run is +hardcoded as an exception. A bare coverage substring or no-finding sentence is +not refresh permission; the old substring-only scheduler predicate is removed. + +A recognized fallback is non-substantive, not APPROVED. A newer fallback, +including COMMENTED or an incorrectly posted APPROVED envelope, prevents the +receipt gate from resurrecting an older same-head approval. A later substantive +source finding still deduplicates as a formal blocker. Existing peer-check-only +fallback recognition and its regressions are preserved. + +Coverage retry still requires complete current-head coverage and Strix evidence, +no failed peers, elapsed retry floor, available history, ordinary deduplication, +admission, actor and live-head checks. Failed CodeQL stays BLOCK/WAIT without +review dispatch or merge. Independent approval, last-push approval, repository +protection and exact-head revalidation are unchanged. There is no privileged +refresh override or approve/merge authority in the classifier. + +### Security Notes + +Review objects and body text are untrusted input. Recognition crosses only a +classification boundary, never a network/mutation boundary: exact actor and +identity validation plus full producer reconstruction fail closed on unknown +content. Changed paths are bounded to 200 rows / 512 characters and normalized +relative components; they are labels only, never opened from the review body. +No source root is passed to the renderer. Producer validation errors return +non-classification, not permission or a gate crash. No secrets, new dependencies, +permissions, logs containing private inputs, workflow changes, dispatch/rerun, +commit/push or merge were introduced. Tests cover identity mismatch, unknown +actors, extra/mixed prose and graph injection, malformed run/attempt, stale +approval resurrection, and failed peer checks. + +### Local verification scope + +Initial actual-fixture RED: both receipt and scheduler cases failed (2 failed). +The first minimal GREEN passed both cases (2 passed). Additional RED cases caught +a producer-validation exception and older-approval resurrection by COMMENTED; +both were repaired fail closed. Existing scheduler gate tests now use the actual +canonical fixture and full synthetic SHA rather than abbreviated coverage prose; +no gate assertions were removed. Final targeted test counts and strict coverage +are reported by the implementing lane, not by this historical record. + +This is local classification/regression evidence only. Remote integration, +independent review, hosted exact-head required checks, fresh receiver execution, +substantive current-head review and protected merge remain uncompleted gates. + +## Current-head CodeRabbit graph-boundary repair (2026-10-04) + +The `76ab3eb5515afdc3f1d61560ad951e34d7776e3f` receipt classifier accepted +arbitrary two-space-indented lines in a Mermaid fence, including source findings. +The shell publisher preserved any body containing the evidence-map heading. +Both defects were reproduced before changing production code: four indented +finding cases failed, and three heading/arbitrary-map preservation cases failed. +A separate non-repository-cwd fixture regression failed with FileNotFoundError +before its workflow path was anchored to `Path(__file__).resolve().parents[1]`. + +Receipt recognition now reconstructs the optional fixed no-path graph with +`emit_mermaid([])` and compares it exactly, alongside the unchanged full peer +fallback envelope. The receipt API has no independently trusted changed-file, +source-root or merge-state binding. Therefore even genuine path/source-dependent +flowcharts, sequence diagrams and class diagrams remain formal blockers here; +recognizing them would require that additional trusted contract, not parsing +self-asserted labels as evidence. The no-graph canonical fallback still works. +Unknown/mixed graph findings return the newer CHANGES_REQUESTED receipt instead +of discarding it or resurrecting an older approval. The historical coverage-only +classifier and its exact-envelope tests are unchanged. + +The shell publisher compares the complete terminal map against a freshly emitted +graph using its existing changed-files, source-root and merge-state inputs. +Duplicate maps, wrong bindings, bare headings, extra prose and arbitrary graphs +retain all original body content and receive an explicit noncanonical-evidence +notice plus the trusted graph. Nothing is stripped or replaced to launder a +finding into a canonical fallback. Renderer errors propagate after temporary +file cleanup; a dedicated RED regression caught the former cleanup-masked error. +Source-backed canonical class graphs and ordinary/conflict maps are preserved +only when their complete producer output matches. + +### Security Notes and verification boundary + +Bodies and review descriptions are data, not authority. No paths from receipt +text are opened and no source-dependent trust is inferred from Mermaid syntax. +Shell comparisons quote the literal trusted graph, preserving source prose and +findings on mismatch. No new private-body logging, secrets, dependencies, +workflow/scheduler, actor, credential, permission or protection changes occur. +The escaped fixed prefix is not labeled a ReDoS vulnerability without a repro. + +Local focused production/receipt/surfaces/shell/gate suites: **649 passed** in +both ordinary and `GITHUB_ACTIONS=true` environments. The receipt suite also +reported **31 passed** when launched from a real non-repository scratch cwd. +Receipt statements and branches: **100%** (160 statements, 66 branches, zero +misses). Ruff, interrogate 100%, bash syntax, Python compilation and diff checks +passed; the added-line static secret/injection/eval/deserialization scan found +no matches. These are bounded local results, not hosted review or merge approval. +The full 5,000-plus suite is reserved for the parent lane. Independent approval, +exact-head required hosted checks, fresh receiver/formal review and protected +merge remain outstanding; no commit, push, thread resolution or remote mutation +was performed by this repair lane. diff --git a/scripts/ci/opencode_legacy_coverage_fallback.py b/scripts/ci/opencode_legacy_coverage_fallback.py new file mode 100644 index 0000000000..0ca7d9c0a3 --- /dev/null +++ b/scripts/ci/opencode_legacy_coverage_fallback.py @@ -0,0 +1,86 @@ +"""Recognize only the complete historical coverage-failure publisher envelope. + +This is refresh evidence, never approval or merge authority. Unknown content, +including unrecognized diagrams, remains a substantive review blocker. +""" + +from __future__ import annotations + +import re +from collections.abc import Mapping +from typing import Any + +try: + from scripts.ci.opencode_review_surfaces import build_fallback_review, emit_mermaid +except ModuleNotFoundError: + from opencode_review_surfaces import build_fallback_review, emit_mermaid + + +_PATH_ROW = re.compile(r"- `([A-Za-z0-9_.@+/-]{1,512})` — [^\n]+") +_IDENTITY = re.compile( + r"- Head SHA: `([0-9a-fA-F]{40})`\n" + r"- Workflow run: ([1-9][0-9]{0,19})\n" + r"- Workflow attempt: ([1-9][0-9]{0,19})\n" + r"- Coverage gate: `failure`\n" +) +_OUTCOME = ( + "\n## Review outcome\n\n" + "Coverage is a gate, not the review. This body reviews the changed product files." +) + + +def is_legacy_coverage_only_review(review: Mapping[str, Any], head_sha: str) -> bool: + """Validate identity and reconstruct the entire bounded producer body exactly.""" + author = review.get("user") or review.get("author") or {} + commit = review.get("commit") or {} + commit_sha = review.get("commit_id") or ( + commit.get("oid") or commit.get("sha") if isinstance(commit, Mapping) else "" + ) + if ( + not isinstance(author, Mapping) + or author.get("login") not in {"opencode-agent", "opencode-agent[bot]"} + or review.get("state") not in {"CHANGES_REQUESTED", "APPROVED", "COMMENTED"} + or not re.fullmatch(r"[0-9a-fA-F]{40}", head_sha) + or str(commit_sha).lower() != head_sha.lower() + ): + return False + body = str(review.get("body") or "") + identities = list(_IDENTITY.finditer(body)) + if len(identities) != 1 or identities[0].group(1).lower() != head_sha.lower(): + return False + identity = identities[0] + paths = [match.group(1) for match in _PATH_ROW.finditer(body)] + if ( + not paths + or len(paths) > 200 + or any( + path.startswith("/") + or any(part in {"", ".", ".."} for part in path.split("/")) + for path in paths + ) + ): + return False + try: + expected = ( + build_fallback_review( + changed_files=paths, + head_sha=identity.group(1), + run_id=identity.group(2), + run_attempt=identity.group(3), + coverage_result="failure", + ) + + _OUTCOME + ) + except ValueError: + return False + # The publisher always appended the evidence map. Its graph is reconstructed + # from paths, not accepted as arbitrary Mermaid text that could hide findings. + for merge_state in ("UNKNOWN", "DIRTY"): + candidate = ( + expected + + "\n\n## Changed-File Evidence Map\n\n" + + emit_mermaid(paths, merge_state=merge_state).rstrip("\n") + ) + if body == candidate or body == candidate + "\n": + return True + return False diff --git a/scripts/ci/opencode_review_comment_helpers.sh b/scripts/ci/opencode_review_comment_helpers.sh index 6caab6f963..84156ed6bf 100644 --- a/scripts/ci/opencode_review_comment_helpers.sh +++ b/scripts/ci/opencode_review_comment_helpers.sh @@ -30,7 +30,7 @@ opencode_review_surfaces_py() { emit_change_flow_mermaid_graph() { local merge_state="${1:-UNKNOWN}" - local changed_files_file + local changed_files_file render_status=0 changed_files_file="$(mktemp)" if ! timeout "${REVIEW_PUBLISH_GH_API_TIMEOUT_SECONDS:-120}s" \ @@ -44,13 +44,14 @@ emit_change_flow_mermaid_graph() { python3 "$(opencode_review_surfaces_py)" emit-mermaid \ --changed-files-file "$changed_files_file" \ --source-root "$OPENCODE_SOURCE_WORKDIR" \ - --merge-state "$merge_state" + --merge-state "$merge_state" || render_status=$? else python3 "$(opencode_review_surfaces_py)" emit-mermaid \ --changed-files-file "$changed_files_file" \ - --merge-state "$merge_state" + --merge-state "$merge_state" || render_status=$? fi rm -f "$changed_files_file" + return "$render_status" } append_mermaid_review_graph() { @@ -63,12 +64,22 @@ append_mermaid_review_graph() { } ensure_review_body_has_change_graph() { - local body="$1" + local body="$1" trusted_graph prefix printf '%s\n' "$body" + trusted_graph="$(append_mermaid_review_graph)" || return 1 if grep -Fq "## Changed-File Evidence Map" <<<"$body"; then - return 0 + # Compare the entire terminal map with the freshly bound producer output. + # Never strip model/source content to manufacture a safe fallback envelope. + if [[ "$body" == *"$trusted_graph" ]]; then + prefix="${body%"$trusted_graph"}" + if [[ "$prefix" != *"## Changed-File Evidence Map"* ]]; then + return 0 + fi + fi + printf '\n## Noncanonical evidence map\n\n' + printf '%s\n' 'The existing evidence map is not the trusted current producer graph; original content is retained for substantive review.' fi - append_mermaid_review_graph + printf '%s\n' "$trusted_graph" } append_merge_conflict_guidance() { diff --git a/scripts/ci/opencode_review_receipt_gate.py b/scripts/ci/opencode_review_receipt_gate.py index 4dcb24af88..f04f4f61fb 100644 --- a/scripts/ci/opencode_review_receipt_gate.py +++ b/scripts/ci/opencode_review_receipt_gate.py @@ -13,6 +13,13 @@ from pathlib import Path from typing import Any +try: + from scripts.ci.opencode_legacy_coverage_fallback import is_legacy_coverage_only_review + from scripts.ci.opencode_review_surfaces import emit_mermaid +except ModuleNotFoundError: + from opencode_legacy_coverage_fallback import is_legacy_coverage_only_review + from opencode_review_surfaces import emit_mermaid + SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") REPO_RE = re.compile( @@ -110,6 +117,38 @@ def is_mention_or_malformed(body: str) -> bool: return not any(marker in stripped for marker in PRODUCT_MARKERS) +def is_peer_check_only_fallback(body: str, head_sha: str) -> bool: + """Match the complete producer payload; retain unknown prose as a blocker.""" + prefix = ( + "## Pull request overview\n\n" + "OpenCode could not approve from deterministic current-head evidence " + "because GitHub Checks have failed.\n\n" + "## Findings\n\n" + "### 1. HIGH Current-head GitHub Checks - Fix failed required checks before approval\n" + f"- Problem: Failed same-head checks remain for `{head_sha}`.\n" + "- Root cause: The model-unavailable evidence fallback is allowed only " + "when peer GitHub Checks are complete and clean.\n" + "- Fix: Read and fix the failed check logs below, then rerun the current-head checks.\n" + "- Regression test: Keep the model-unavailable fallback gated on an empty " + "failed-check rollup.\n\nFailed checks:\n" + ) + check_line = ( + r"- [^\n\r]+: (?i:FAILURE|ERROR|TIMED_OUT|ACTION_REQUIRED|CANCELLED|STARTUP_FAILURE)" + r"(?: \(https://[^\s()]+\))?" + ) + # This receipt has no changed-file/source-root/merge-state binding. Only the + # fixed no-path graph is independently reconstructible; source-dependent + # diagrams remain formal blockers rather than trusting labels from the body. + graph = re.escape( + "\n\n## Changed-File Evidence Map\n\n" + emit_mermaid([]).rstrip("\n") + ) + return re.fullmatch( + re.escape(prefix) + check_line + r"(?:\n" + check_line + r")*" + + r"(?:" + graph + r")?\n?", + body, + ) is not None + + def is_formal_receipt( review: Mapping[str, Any], head_sha: str, @@ -123,6 +162,9 @@ def is_formal_receipt( if author not in FORMAL_AUTHORS: return False, f"author {author or ''} is not an OpenCode publisher" state = str(review.get("state") or "").upper() + if is_legacy_coverage_only_review(review, head_sha): + verdict = "approval" if state == "APPROVED" else "changes request" + return False, f"fallback {verdict} requires fresh substantive review" if state not in FORMAL_STATES: return False, f"state {state or ''} is not a formal review verdict" if not review.get("id"): @@ -134,6 +176,10 @@ def is_formal_receipt( marker in body.casefold() for marker in FALLBACK_APPROVAL_MARKERS ): return False, "fallback approval is not a substantive formal review" + if state == "CHANGES_REQUESTED" and is_peer_check_only_fallback(body, head_sha) and any( + marker in body.casefold() for marker in FALLBACK_APPROVAL_MARKERS + ): + return False, "fallback changes request requires fresh substantive review" if is_draft and state == "APPROVED": return False, "draft must never receive bot APPROVE" return True, "current-head formal review" @@ -161,7 +207,7 @@ def evaluate_receipts( return review, reason if "never receive bot APPROVE" in reason: return None, reason - if "fallback approval" in reason: + if "fallback approval" in reason or "fallback changes request" in reason: return None, reason if reason.startswith("stale"): stale_hits += 1 diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 5a86bd24c8..689f64d508 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -22,6 +22,7 @@ from urllib.parse import quote try: + from scripts.ci.opencode_legacy_coverage_fallback import is_legacy_coverage_only_review from scripts.ci.review_admission_controller import ( WORKER_BOUNDARIES, AdmissionRequest, @@ -32,6 +33,7 @@ update_state_file, ) except ModuleNotFoundError: # pragma: no cover - package import path + from opencode_legacy_coverage_fallback import is_legacy_coverage_only_review from review_admission_controller import ( WORKER_BOUNDARIES, AdmissionRequest, @@ -2216,7 +2218,9 @@ def is_deterministic_fallback_approval(review: dict[str, Any]) -> bool: if (review.get("state") or "").upper() != "APPROVED": return False body = (review.get("body") or "").lower() - return any(marker in body for marker in DETERMINISTIC_APPROVAL_MARKERS) + return any(marker in body for marker in DETERMINISTIC_APPROVAL_MARKERS) or ( + is_legacy_coverage_only_review(review, str((review.get("commit") or {}).get("oid") or "")) + ) def has_current_head_deterministic_fallback_approval(pr: dict[str, Any]) -> bool: @@ -2305,8 +2309,9 @@ def latest_current_head_coverage_change_request( continue if (review.get("state") or "").upper() != "CHANGES_REQUESTED": return None - body = (review.get("body") or "").lower() - return review if all(marker in body for marker in COVERAGE_REVIEW_MARKERS) else None + return review if is_legacy_coverage_only_review( + review, str(pr.get("headRefOid") or "") + ) else None return None diff --git a/tests/fixtures/opencode_legacy_coverage_review5301203359.json b/tests/fixtures/opencode_legacy_coverage_review5301203359.json new file mode 100644 index 0000000000..871459fa5f --- /dev/null +++ b/tests/fixtures/opencode_legacy_coverage_review5301203359.json @@ -0,0 +1,13 @@ +{ + "databaseId": 5301203359, + "id": "PRR_kwDORjvEXs8AAAABO_nxnw", + "author": { + "login": "opencode-agent" + }, + "state": "CHANGES_REQUESTED", + "body": "## Pull request overview\n\nOpenCode reviewed the current-head product diff. Coverage is a separate gate.\n\n## Changed files\n\n- `services/analysis-engine/tests/test_supply_chain_policy.py` — regression suite\n\n## Changed behavior\n\n```mermaid\nflowchart LR\n PR[\"PR changed files\"] --> Evidence[\"OpenCode bounded evidence\"]\n Evidence --> S1[\"Test: test_supply_chain_policy.py\"]\n S1 --> I1[\"regression suite\"]\n I1 --> R1[\"Review risk: Test: test_supply_chain_policy.py\"]\n R1 --> V1[\"targeted test run\"]\n```\n\n## Findings\n\nNo source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.\n\n- Head SHA: `8fe6b6d99c009527ef0bcba419e6f6debdb23c23`\n- Workflow run: 35908696823\n- Workflow attempt: 1\n- Coverage gate: `failure`\n\n## Review outcome\n\nCoverage is a gate, not the review. This body reviews the changed product files.\n\n## Changed-File Evidence Map\n\n```mermaid\nflowchart LR\n PR[\"PR changed files\"] --> Evidence[\"OpenCode bounded evidence\"]\n Evidence --> S1[\"Test: test_supply_chain_policy.py\"]\n S1 --> I1[\"regression suite\"]\n I1 --> R1[\"Review risk: Test: test_supply_chain_policy.py\"]\n R1 --> V1[\"targeted test run\"]\n```", + "commit": { + "oid": "8fe6b6d99c009527ef0bcba419e6f6debdb23c23" + }, + "submittedAt": "2026-09-24T07:31:28Z" +} \ No newline at end of file diff --git a/tests/test_opencode_legacy_coverage_refresh.py b/tests/test_opencode_legacy_coverage_refresh.py new file mode 100644 index 0000000000..1347d9d25a --- /dev/null +++ b/tests/test_opencode_legacy_coverage_refresh.py @@ -0,0 +1,408 @@ +"""Exact historical coverage-only reviews require refresh, never approval.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from scripts.ci import opencode_review_receipt_gate as receipt +from scripts.ci import pr_review_merge_scheduler as scheduler + +FIXTURE = ( + Path(__file__).parent + / "fixtures" + / "opencode_legacy_coverage_review5301203359.json" +) + + +def actual_review(): + """Read the authenticated public GraphQL review without altering its body.""" + return json.loads(FIXTURE.read_text(encoding="utf-8")) + + +def review_pr(review): + """Bind one review to its exact current PR head.""" + return {"headRefOid": review["commit"]["oid"], "reviews": {"nodes": [review]}} + + +@pytest.mark.parametrize("surface", ["receipt", "scheduler"]) +def test_actual_legacy_review_is_refresh_only(surface): + """Review 5301203359 is non-substantive on both consumers, not an approval.""" + review = actual_review() + pr = review_pr(review) + if surface == "receipt": + found, reason = receipt.evaluate_receipts([review], pr["headRefOid"]) + assert found is None + assert "fallback changes request" in reason + else: + assert scheduler.current_head_coverage_change_request(pr) is True + assert scheduler.has_current_head_approval(pr) is False + + +@pytest.mark.parametrize( + "mutation", + [ + "commit", + "body_sha", + "actor", + "extra", + "mixed", + "unknown", + "run_zero", + "run_text", + "attempt_zero", + "attempt_text", + "graph_finding", + "graph_label", + "path_role", + "missing_map", + "missing_identity", + "duplicate_identity", + "no_paths", + "traversal", + "absolute", + "double_slash", + "dot_path", + "too_many_paths", + "malformed_author", + "malformed_commit", + "state", + "head", + "missing_commit", + "coverage_markers", + "coverage_success", + ], +) +def test_unknown_or_mixed_legacy_review_stays_blocking(mutation): + """No substring, malformed identity, or arbitrary graph can authorize refresh.""" + from scripts.ci.opencode_legacy_coverage_fallback import ( + is_legacy_coverage_only_review, + ) + + review = actual_review() + head = review["commit"]["oid"] + body = review["body"] + if mutation == "commit": + review["commit"]["oid"] = "b" * 40 + elif mutation == "body_sha": + body = body.replace(head, "b" * 40) + elif mutation == "actor": + review["author"]["login"] = "github-actions[bot]" + elif mutation == "extra": + body += "\nThe changed endpoint allows anonymous writes." + elif mutation == "mixed": + body = body.replace( + "## Findings\n\n", "## Findings\n\n### HIGH Missing authorization\n" + ) + elif mutation == "unknown": + body = body.replace("## Changed behavior", "## Unknown behavior") + elif mutation.startswith("run_"): + body = body.replace( + "Workflow run: 35908696823", + "Workflow run: " + ("0" if mutation == "run_zero" else "not-a-run"), + ) + elif mutation.startswith("attempt_"): + body = body.replace( + "Workflow attempt: 1", + "Workflow attempt: " + ("0" if mutation == "attempt_zero" else "1.0"), + ) + elif mutation == "graph_finding": + body = body.replace( + "flowchart LR\n", + "flowchart LR\n Missing authorization allows anonymous writes\n", + ) + elif mutation == "graph_label": + body = body.replace('I1["regression suite"]', 'I1["anonymous writes"]') + elif mutation == "path_role": + body = body.replace("` — regression suite", "` — anonymous writes") + elif mutation == "missing_map": + body = body.split("\n\n## Changed-File Evidence Map")[0] + elif mutation == "missing_identity": + body = body.replace(f"- Head SHA: `{head}`\n", "") + elif mutation == "duplicate_identity": + start = body.index("- Head SHA:") + end = body.index("\n\n## Review outcome") + body += "\n" + body[start:end] + "\n" + elif mutation == "no_paths": + body = body.replace( + "- `services/analysis-engine/tests/test_supply_chain_policy.py` — regression suite\n", + "", + ) + elif mutation in {"traversal", "absolute", "double_slash", "dot_path"}: + path = { + "traversal": "../tests/test_policy.py", + "absolute": "/tests/test_policy.py", + "double_slash": "tests//test_policy.py", + "dot_path": "./tests/test_policy.py", + }[mutation] + body = body.replace( + "services/analysis-engine/tests/test_supply_chain_policy.py", path + ) + elif mutation == "too_many_paths": + body = body.replace( + "## Changed files\n\n", + "## Changed files\n\n" + + "- `tests/test_extra.py` — regression suite\n" * 200, + ) + elif mutation == "malformed_author": + review["author"] = "unknown" + elif mutation == "malformed_commit": + review["commit"] = "unknown" + elif mutation == "state": + review["state"] = "PENDING" + elif mutation == "head": + head = "short" + elif mutation == "missing_commit": + review.pop("commit") + elif mutation == "coverage_markers": + body = "## Pull request overview\ncoverage evidence did not pass; coverage-evidence; required test/docstring evidence" + else: + body = body.replace("Coverage gate: `failure`", "Coverage gate: `success`") + review["body"] = body + assert is_legacy_coverage_only_review(review, head) is False + pr = {"headRefOid": head, "reviews": {"nodes": [review]}} + # Malformed API object shape is tested at the shared trust boundary; the + # scheduler API contract itself still requires GraphQL mapping-shaped nodes. + if mutation not in {"malformed_author", "malformed_commit"}: + assert scheduler.current_head_coverage_change_request(pr) is False + if mutation in { + "extra", + "mixed", + "unknown", + "run_zero", + "run_text", + "attempt_zero", + "attempt_text", + "graph_finding", + "graph_label", + "path_role", + "missing_map", + "missing_identity", + "duplicate_identity", + "no_paths", + "traversal", + "absolute", + "double_slash", + "dot_path", + "too_many_paths", + "coverage_markers", + "coverage_success", + }: + assert receipt.evaluate_receipts([review], head)[0] is review + + +@pytest.mark.parametrize("state", ["CHANGES_REQUESTED", "APPROVED", "COMMENTED"]) +def test_newer_fallback_never_resurrects_older_approval(state): + """A canonical fallback cannot authorize approval even when misposted APPROVED.""" + review = actual_review() + head = review["commit"]["oid"] + older = { + **review, + "id": "older", + "state": "APPROVED", + "body": "## Pull request overview\nSubstantive review.", + } + review["state"] = state + assert receipt.evaluate_receipts([older, review], head)[0] is None + assert ( + scheduler.has_current_head_approval( + {"headRefOid": head, "reviews": {"nodes": [older, review]}} + ) + is False + ) + + +@pytest.mark.parametrize("login", ["opencode-agent", "opencode-agent[bot]"]) +@pytest.mark.parametrize("merge_state", ["UNKNOWN", "DIRTY"]) +def test_rest_and_canonical_graph_variants(login, merge_state): + """Exact reconstruction is generic in repository, paths, head, run, and attempt.""" + from scripts.ci.opencode_review_surfaces import build_fallback_review, emit_mermaid + from scripts.ci.opencode_legacy_coverage_fallback import ( + is_legacy_coverage_only_review, + ) + + head = "a" * 40 + paths = ["backend/api.py", "frontend/view.tsx", "tests/test_api.py"] + body = build_fallback_review( + changed_files=paths, + head_sha=head, + run_id="99", + run_attempt="3", + coverage_result="failure", + ) + body += "\n## Review outcome\n\nCoverage is a gate, not the review. This body reviews the changed product files." + body += "\n\n## Changed-File Evidence Map\n\n" + emit_mermaid( + paths, merge_state=merge_state + ) + review = { + "id": 1, + "user": {"login": login}, + "commit_id": head, + "state": "CHANGES_REQUESTED", + "body": body, + } + assert is_legacy_coverage_only_review(review, head) is True + assert receipt.evaluate_receipts([review], head)[0] is None + + +def test_producer_rejection_stays_a_formal_blocker(): + """A normalized path that violates producer anchor policy cannot crash the gate.""" + from scripts.ci.opencode_legacy_coverage_fallback import ( + is_legacy_coverage_only_review, + ) + + review = actual_review() + head = review["commit"]["oid"] + review["body"] = review["body"].replace( + "services/analysis-engine/tests/test_supply_chain_policy.py", + "docs/.github/workflows/opencode-review.yml", + ) + assert is_legacy_coverage_only_review(review, head) is False + assert receipt.evaluate_receipts([review], head)[0] is review + + +def test_direct_script_imports_fail_closed(monkeypatch): + """Direct CLI loading uses the same classifier without package import paths.""" + import builtins + import importlib + import sys + + original = builtins.__import__ + + def direct_import(name, *args, **kwargs): + """Exercise direct-script fallback imports with no package namespace.""" + if name.startswith("scripts.ci.opencode_"): + raise ModuleNotFoundError(name) + return original(name, *args, **kwargs) + + from scripts.ci import opencode_legacy_coverage_fallback as classifier + from scripts.ci import opencode_review_surfaces as surfaces + + with monkeypatch.context() as context: + context.syspath_prepend(str(Path("scripts/ci").resolve())) + context.setitem(sys.modules, "opencode_review_surfaces", surfaces) + context.setitem(sys.modules, "opencode_legacy_coverage_fallback", classifier) + context.setattr(builtins, "__import__", direct_import) + importlib.reload(classifier) + importlib.reload(receipt) + review = actual_review() + head = review["commit"]["oid"] + assert classifier.is_legacy_coverage_only_review(review, head) + assert receipt.evaluate_receipts([review], head)[0] is None + importlib.reload(classifier) + importlib.reload(receipt) + + +@pytest.mark.parametrize("peer", ["CodeQL", "strix", "coverage-evidence"]) +@pytest.mark.parametrize("conclusion", ["FAILURE", "CANCELLED", "TIMED_OUT"]) +def test_actual_legacy_failed_peer_never_dispatches_or_merges( + monkeypatch, peer, conclusion +): + """Actual fallback plus any failed peer must retain WAIT/BLOCK, no mutations.""" + review = actual_review() + head = review["commit"]["oid"] + checks = [ + { + "__typename": "CheckRun", + "name": name, + "status": "COMPLETED", + "conclusion": conclusion if name == peer else "SUCCESS", + } + for name in ("coverage-evidence", "strix", "CodeQL") + ] + pr = { + **review_pr(review), + "number": 1176, + "isDraft": False, + "mergeable": "MERGEABLE", + "mergeStateStatus": "CLEAN", + "baseRefName": "main", + "baseRefOid": "b" * 40, + "headRefName": "feature", + "headRepository": {"nameWithOwner": "owner/repo"}, + "author": {"login": "pull-request-author"}, + "reviewDecision": "CHANGES_REQUESTED", + "reviewThreads": {"nodes": []}, + "statusCheckRollup": {"contexts": {"nodes": checks}}, + "commits": {"nodes": [{"commit": {"oid": head}}]}, + } + + def forbidden(*args, **kwargs): + """Fail the test before any live dispatch, merge, or other GitHub mutation.""" + raise AssertionError("mutation not authorized by fallback") + + for name in ( + "dispatch_opencode_review", + "dispatch_strix_evidence", + "merge_pr", + "enable_auto_merge", + "run", + "run_github_dispatch", + "run_github_actions", + ): + monkeypatch.setattr(scheduler, name, forbidden) + decision = scheduler.inspect_pr( + "owner/repo", + pr, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + merge_mode="auto", + ) + assert decision.action in {"block", "wait"} + assert scheduler.contract_decision(decision) not in { + "MERGE", + "AUTO_MERGE", + "REVIEW_DISPATCH", + } + assert not scheduler.has_current_head_approval(pr) + + +def test_substantive_review_after_legacy_still_deduplicates(): + """Fresh actual source findings remain formal blockers after an old fallback.""" + review = actual_review() + head = review["commit"]["oid"] + substantive = { + **review, + "id": "newer", + "body": "## Pull request overview\n## Findings\n### HIGH Missing authorization\nAnonymous writes.", + } + assert receipt.evaluate_receipts([review, substantive], head)[0] is substantive + assert ( + scheduler.current_head_coverage_change_request( + {"headRefOid": head, "reviews": {"nodes": [review, substantive]}} + ) + is False + ) + + +def test_existing_scheduler_protection_summary_boundaries(): + """Conflict and last-push guidance remain blockers, never refresh authority.""" + assert scheduler.is_strix_scan_check_run( + {"__typename": "CheckRun", "name": "strix"} + ) + assert not scheduler.is_strix_scan_check_run( + {"__typename": "StatusContext", "name": "strix"} + ) + assert "reviewDecision is REVIEW_REQUIRED" in scheduler.auto_merge_wait_reason( + "BLOCKED", {"reviewDecision": "REVIEW_REQUIRED"} + ) + conflict = scheduler.Decision( + 1, + "block", + "merge conflict: DIRTY; base=main, head=feature; resolve conflict markers", + ) + assert scheduler.conflict_repair_summary([conflict]) + restamp = scheduler.Decision( + 1, + "restamp_head", + "last-push approval head refresh created same-tree head ffffffffffff", + ("unrelated note",), + ) + assert scheduler.last_push_approval_restamp_summary([restamp]) diff --git a/tests/test_opencode_review_comment_helpers.py b/tests/test_opencode_review_comment_helpers.py index 9f0d0ca5c1..b38aa029a0 100644 --- a/tests/test_opencode_review_comment_helpers.py +++ b/tests/test_opencode_review_comment_helpers.py @@ -7,6 +7,12 @@ import subprocess from pathlib import Path +import pytest + +from scripts.ci.opencode_review_surfaces import emit_mermaid +from scripts.ci import opencode_review_receipt_gate as receipt +from tests.test_opencode_review_receipt_gate import canonical_peer_fallback, review + REPO_ROOT = Path(__file__).resolve().parents[1] HELPER = REPO_ROOT / "scripts/ci/opencode_review_comment_helpers.sh" @@ -52,3 +58,132 @@ def test_helper_sources_python_surfaces_module() -> None: text = HELPER.read_text(encoding="utf-8") assert "opencode_review_surfaces.py" in text assert 'add("other", "Changed file"' not in text + + +def render_with_trusted_graph(tmp_path, body, paths, merge_state="UNKNOWN", source_root=None): + """Exercise the real shell/CLI publisher with bounded local GitHub responses.""" + changed = tmp_path / "changed.txt" + changed.write_text("\n".join(paths) + "\n") + fake_bin = tmp_path / "bin" + fake_bin.mkdir(exist_ok=True) + gh = fake_bin / "gh" + gh.write_text( + '#!/bin/sh\ncase "$2" in\n' + 'diff) cat "$OPENCODE_CHANGED_FILES_FILE" ;;\n' + 'view) printf \'{"mergeStateStatus":"%s"}\\n\' "$TEST_MERGE_STATE" ;;\n' + '*) exit 1 ;;\nesac\n' + ) + gh.chmod(0o755) + result = subprocess.run( + ["bash", "-euo", "pipefail", "-c", + '. "$1"; body="$(cat)"; ensure_review_body_has_change_graph "$body"', + "test", str(HELPER)], + input=body, text=True, capture_output=True, check=False, + cwd=tmp_path, + env={**os.environ, "PATH": f"{fake_bin}:{os.environ['PATH']}", + "PR_NUMBER": "47", "GH_REPOSITORY": "ContextualWisdomLab/OriginWeave", + "OPENCODE_CHANGED_FILES_FILE": str(changed), + "OPENCODE_SOURCE_WORKDIR": str(source_root or ""), + "TEST_MERGE_STATE": merge_state}, + ) + assert result.returncode == 0, result.stderr + return result.stdout + + +@pytest.mark.parametrize("existing", [ + "## Changed-File Evidence Map", + "## Changed-File Evidence Map\n\n```mermaid\nflowchart LR\n Anonymous writes are allowed\n```", + "## Changed-File Evidence Map\n\n```mermaid\nclassDiagram\n class MissingAuthorization\n```", +]) +def test_noncanonical_existing_graph_is_not_preserved_as_trusted(tmp_path, existing): + """Keep source findings intact while marking a noncanonical map fail closed.""" + head = receipt.AFIPC_230_HEAD + body = canonical_peer_fallback(head) + "\n" + existing + output = render_with_trusted_graph(tmp_path, body, ["docs/guide.md"]) + assert output.startswith(body + "\n") + assert "## Noncanonical evidence map" in output + assert emit_mermaid(["docs/guide.md"]).strip() in output + candidate = review(commit=head, body=output) + older = review(commit=head, state="APPROVED", review_id=2) + assert receipt.evaluate_receipts([older, candidate], head)[0] is candidate + + +@pytest.mark.parametrize("paths,merge_state", [ + ([], "UNKNOWN"), (["docs/guide.md"], "UNKNOWN"), + (["docs/guide.md"], "DIRTY"), (["crates/demo/src/lib.rs"], "CONFLICTING"), +]) +def test_exact_trusted_graph_is_preserved(tmp_path, paths, merge_state): + """Preservation requires the complete graph from the same producer binding.""" + body = "## Pull request overview\nSource prose.\n\n## Changed-File Evidence Map\n\n" + body += emit_mermaid(paths, merge_state=merge_state).rstrip("\n") + assert render_with_trusted_graph(tmp_path, body, paths, merge_state) == body + "\n" + + +@pytest.mark.parametrize("kind", ["source", "changed-files", "merge-state", "duplicate", "extra-prose"]) +def test_graph_preservation_is_bound_to_current_producer_inputs(tmp_path, kind): + """A valid Mermaid shape alone cannot bypass source or live-state comparison.""" + paths = ["crates/demo/src/lib.rs"] + source = tmp_path / paths[0] + source.parent.mkdir(parents=True) + source.write_text("pub struct CurrentApi;\n") + old_graph = emit_mermaid(paths) + merge_state = "UNKNOWN" + if kind == "changed-files": + old_graph = emit_mermaid(["docs/old.md"]) + elif kind == "merge-state": + old_graph = emit_mermaid(paths, source_root=tmp_path) + merge_state = "DIRTY" + elif kind in {"duplicate", "extra-prose"}: + old_graph = emit_mermaid(paths, source_root=tmp_path) + body = "## Pull request overview\nSource finding.\n\n## Changed-File Evidence Map\n\n" + old_graph.rstrip("\n") + if kind == "duplicate": + body = body + "\n\n## Changed-File Evidence Map\n\n" + old_graph.rstrip("\n") + elif kind == "extra-prose": + body += "\nAnonymous writes are allowed." + output = render_with_trusted_graph(tmp_path, body, paths, merge_state, tmp_path) + assert output.startswith(body + "\n") + assert "## Noncanonical evidence map" in output + assert emit_mermaid(paths, merge_state, tmp_path).strip() in output + + +def test_bound_source_class_graph_is_preserved(tmp_path): + """The exact source-root-backed graph is preserved without invented edges.""" + source = tmp_path / "lib.rs" + source.write_text("pub struct CurrentApi;\n") + body = "## Pull request overview\n\n## Changed-File Evidence Map\n\n" + body += emit_mermaid(["lib.rs"], source_root=tmp_path).rstrip("\n") + assert render_with_trusted_graph(tmp_path, body, ["lib.rs"], source_root=tmp_path) == body + "\n" + + +def test_noncanonical_map_cannot_be_laundered_with_empty_trusted_graph(tmp_path): + """Appending a canonical graph must never erase an arbitrary source finding.""" + head = receipt.AFIPC_230_HEAD + body = canonical_peer_fallback(head) + "\n## Changed-File Evidence Map\n\n```mermaid\nflowchart LR\n Missing authorization\n```" + output = render_with_trusted_graph(tmp_path, body, []) + assert output.startswith(body + "\n") + assert emit_mermaid([]).strip() in output + candidate = review(commit=head, body=output) + assert receipt.evaluate_receipts([review(commit=head, state="APPROVED"), candidate], head)[0] is candidate + + +def test_no_graph_canonical_fallback_still_triggers_fresh_review(tmp_path): + """The publisher's ordinary no-map fallback remains non-substantive.""" + head = receipt.AFIPC_230_HEAD + output = render_with_trusted_graph(tmp_path, canonical_peer_fallback(head), []) + assert receipt.is_peer_check_only_fallback(output, head) + assert receipt.evaluate_receipts([review(commit=head, body=output)], head)[0] is None + + +def test_graph_emitter_failure_is_not_a_canonical_map(tmp_path): + """A failed trusted renderer must propagate instead of legitimizing a heading.""" + result = subprocess.run( + ["bash", "-euo", "pipefail", "-c", + '. "$1"; timeout() { printf \'{"mergeStateStatus":"UNKNOWN"}\\n\'; }; ' + 'python3() { return 1; }; ' + 'ensure_review_body_has_change_graph "## Changed-File Evidence Map"', + "test", str(HELPER)], + text=True, capture_output=True, check=False, + env={**os.environ, "PR_NUMBER": "47", "GH_REPOSITORY": "ContextualWisdomLab/OriginWeave"}, + cwd=tmp_path, + ) + assert result.returncode != 0 diff --git a/tests/test_opencode_review_receipt_gate.py b/tests/test_opencode_review_receipt_gate.py index c971e2128a..de29fd7ddd 100644 --- a/tests/test_opencode_review_receipt_gate.py +++ b/tests/test_opencode_review_receipt_gate.py @@ -342,3 +342,199 @@ def unexpected_run(args, **kwargs): )(), ) assert receipt.load_reviews("-")[0]["commit_id"] == receipt.AFIPC_230_HEAD + + +def canonical_peer_fallback(head): + """Execute the producer's printf block with a realistic failed-check row.""" + import subprocess + source = (Path(__file__).resolve().parents[1] / ".github/workflows/opencode-review-dispatch.yml").read_text() + # Locate the unique canonical producer directly; other fallbacks stay independent. + marker = source.index("printf 'OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.") + start = source.rfind(" printf '## Pull request overview", 0, marker) + end = source.index(' cat "$failed_checks_file"', marker) + program = source[start:end] + "printf '%s\\n' '- CodeQL PR/CodeQL compatibility analysis (python): FAILURE (https://github.com/ContextualWisdomLab/.github/actions/runs/1)'" + return subprocess.check_output(["bash", "-c", program], env={"HEAD_SHA": head}, text=True) + + +def test_canonical_peer_fallback_is_independent_of_cwd(tmp_path, monkeypatch): + """The real producer fixture is loaded from this test's repository, not cwd.""" + monkeypatch.chdir(tmp_path) + body = canonical_peer_fallback(receipt.AFIPC_230_HEAD) + assert receipt.is_peer_check_only_fallback(body, receipt.AFIPC_230_HEAD) + + +def test_fallback_changes_request_requires_fresh_review(): + """The complete producer envelope triggers review without reusing approval.""" + head = receipt.AFIPC_230_HEAD + fallback = review(commit=head, body=canonical_peer_fallback(head)) + older_approval = review(commit=head, state="APPROVED") + found, reason = receipt.evaluate_receipts([older_approval, fallback], head) + assert found is None + assert "fallback" in reason + + +def test_substantive_changes_request_still_deduplicates(): + """Actual product findings remain a receipt even after a prior fallback.""" + head = receipt.AFIPC_230_HEAD + fallback = review(commit=head, body="## Pull request overview\nmodel-unavailable evidence fallback") + substantive = review(commit=head, body=( + "## Pull request overview\n## Findings\n" + "### 1. HIGH Missing authorization\n" + "The changed endpoint allows anonymous writes.\n" + )) + found, _ = receipt.evaluate_receipts([fallback, substantive], head) + assert found == substantive + + + +def test_fallback_marker_does_not_hide_substantive_finding() -> None: + """A fallback marker cannot suppress a real product blocker in the same review.""" + head = receipt.AFIPC_230_HEAD + mixed = review( + commit=head, + body=( + "## Pull request overview\n" + "model-unavailable evidence fallback\n" + "OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.\n## Findings\n" + "### 1. HIGH Current-head GitHub Checks - Fix failed required checks before approval\n" + "### 2. HIGH Missing authorization\n" + "The changed endpoint allows anonymous writes.\n" + ), + ) + found, reason = receipt.evaluate_receipts([mixed], head) + assert found == mixed + assert reason == "current-head formal review" + + +@pytest.mark.parametrize("suffix", [ + "\n### 2. HIGH Missing authorization\nAnonymous writes are allowed.\n", + "\n### Unexpected finding shape\n", + "\n#### Missing authorization\n", + "\n###Missing authorization\n", +]) +def test_unknown_or_mixed_finding_is_retained(suffix): + """Only an exact peer-check-only finding list may trigger reevaluation.""" + head = receipt.AFIPC_230_HEAD + body = ("## Pull request overview\nmodel-unavailable evidence fallback\n" + "OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.\n## Findings\n" + "### 1. HIGH Current-head GitHub Checks - Fix failed required checks before approval\n") + candidate = review(commit=head, body=body + suffix) + assert receipt.evaluate_receipts([candidate], head)[0] == candidate + + +def test_evidence_map_headings_are_not_product_findings(): + """Only the generated fenced diagram may follow the complete finding.""" + from scripts.ci.opencode_review_surfaces import emit_mermaid + head = receipt.AFIPC_230_HEAD + body = canonical_peer_fallback(head) + "\n## Changed-File Evidence Map\n\n" + emit_mermaid([]) + assert receipt.evaluate_receipts([review(commit=head, body=body)], head)[0] is None + for extra in ("\nThe endpoint allows anonymous writes.", "\n## Findings\nOther finding"): + candidate = review(commit=head, body=body + extra) + assert receipt.evaluate_receipts([candidate], head)[0] == candidate + + +@pytest.mark.parametrize("graph", [ + "```mermaid\nflowchart LR\n Anonymous writes are allowed\n```\n", + "```mermaid\nclassDiagram\n class MissingAuthorization\n```\n", + "```mermaid\nsequenceDiagram\n participant Crate as Anonymous writes are allowed\n```\n", + "```mermaid\nflowchart LR\n Evidence[\"OpenCode evidence\"] --> Review[\"Current PR review path\"]\n Review --> Verify[\"Required checks\"]\n %% HIGH missing authorization\n```\n", +]) +def test_arbitrary_indented_graph_findings_remain_formal_blockers(graph): + """An indented graph line is not proof of canonical non-product evidence.""" + head = receipt.AFIPC_230_HEAD + body = canonical_peer_fallback(head) + "\n## Changed-File Evidence Map\n\n" + graph + candidate = review(commit=head, body=body) + older = review(commit=head, state="APPROVED", review_id=2) + found, reason = receipt.evaluate_receipts([older, candidate], head) + assert found is candidate + assert reason == "current-head formal review" + + +def test_source_dependent_canonical_graph_without_binding_remains_blocking(tmp_path): + """A genuine graph is not receipt proof without trusted paths/source inputs.""" + from scripts.ci.opencode_review_surfaces import emit_mermaid + source = tmp_path / "lib.rs" + source.write_text("pub struct CurrentApi;\n") + head = receipt.AFIPC_230_HEAD + for graph in (emit_mermaid(["docs/guide.md"]), emit_mermaid(["lib.rs"]), + emit_mermaid(["lib.rs"], source_root=tmp_path)): + body = canonical_peer_fallback(head) + "\n## Changed-File Evidence Map\n\n" + graph + candidate = review(commit=head, body=body) + assert receipt.evaluate_receipts([review(commit=head, state="APPROVED"), candidate], head)[0] is candidate + + +def test_unknown_fallback_format_is_retained(): + """A fallback marker alone cannot classify an unknown review as peer-only.""" + head = receipt.AFIPC_230_HEAD + candidate = review(commit=head, body="## Pull request overview\nmodel-unavailable evidence fallback") + assert receipt.evaluate_receipts([candidate], head)[0] == candidate + + +def test_peer_fallback_literals_remain_bound_to_canonical_producer(): + """Producer wording drift requires an explicit receipt-contract update.""" + source = (Path(__file__).resolve().parents[1] / ".github/workflows/opencode-review-dispatch.yml").read_text() + assert "OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed." in source + assert "### 1. HIGH Current-head GitHub Checks - Fix failed required checks before approval" in source + + +def test_mixed_finding_without_canonical_overview_remains_blocking(): + """Preserve the concurrent owner's original mixed-review counterexample.""" + head = receipt.AFIPC_230_HEAD + candidate = review(commit=head, body=( + "## Pull request overview\nmodel-unavailable evidence fallback\n" + "## Findings\n" + "### 1. HIGH Current-head GitHub Checks - Fix failed required checks before approval\n" + "### 2. HIGH Missing authorization\nThe changed endpoint allows anonymous writes.\n" + )) + assert receipt.evaluate_receipts([candidate], head)[0] == candidate + + +@pytest.mark.parametrize("sections", ["", "## Findings\n### 1. HIGH Missing authorization\n## Findings\n### 2. HIGH Missing authorization\n"]) +def test_missing_or_duplicate_findings_remain_blocking(sections): + """Ambiguous section structure must never discard an active change request.""" + head = receipt.AFIPC_230_HEAD + candidate = review(commit=head, body=( + "## Pull request overview\nmodel-unavailable evidence fallback\n" + "OpenCode could not approve from deterministic current-head evidence " + "because GitHub Checks have failed.\n" + sections + )) + assert receipt.evaluate_receipts([candidate], head)[0] == candidate + + +def test_peer_fallback_with_unstructured_product_finding_remains_blocking() -> None: + """Substantive prose cannot hide under the canonical peer-check heading.""" + head = receipt.AFIPC_230_HEAD + candidate = review( + commit=head, + body=( + "## Pull request overview\n\n" + "OpenCode could not approve from deterministic current-head evidence " + "because GitHub Checks have failed.\n\n" + "model-unavailable evidence fallback\n\n" + "## Findings\n\n" + "### 1. HIGH Current-head GitHub Checks - Fix failed required checks before approval\n" + f"- Problem: Failed same-head checks remain for `{head}`.\n" + "- Root cause: The model-unavailable evidence fallback is allowed only " + "when peer GitHub Checks are complete and clean.\n" + "- Fix: Read and fix the failed check logs below, then rerun the current-head checks.\n" + "- Regression test: Keep the model-unavailable fallback gated on an empty " + "failed-check rollup.\n\n" + "Failed checks:\n" + "- CodeQL PR/CodeQL compatibility analysis (python): FAILURE " + "(https://github.com/ContextualWisdomLab/.github/actions/runs/1)\n\n" + "The changed endpoint also allows anonymous writes.\n" + ), + ) + + found, _ = receipt.evaluate_receipts([candidate], head) + + assert found == candidate + + +@pytest.mark.parametrize("extra", ["The endpoint allows anonymous writes.\n", "- Missing authorization\n", "## Other finding\n"]) +def test_canonical_payload_with_extra_prose_is_retained(extra): + """Unknown prose anywhere in an otherwise valid payload remains blocking.""" + head = receipt.AFIPC_230_HEAD + for body in (extra + canonical_peer_fallback(head), canonical_peer_fallback(head) + extra): + candidate = review(commit=head, body=body) + assert receipt.evaluate_receipts([candidate], head)[0] == candidate diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index abe5a411c7..42e1270a9d 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -8,6 +8,17 @@ from scripts.ci import pr_review_merge_scheduler as sched + +def canonical_coverage_body(): + """Bind scheduler gate regressions to the actual historical producer envelope.""" + from pathlib import Path + review = json.loads( + (Path(__file__).parent / "fixtures" / "opencode_legacy_coverage_review5301203359.json") + .read_text(encoding="utf-8") + ) + return review["body"].replace(review["commit"]["oid"], "a" * 40) + + TOKEN_SEPARATOR = "_" GITHUB_TOKEN_PREFIXES = { "classic": "g" + "hp", @@ -2654,14 +2665,13 @@ def test_central_coverage_retry_ignores_failed_required_workflow_placeholder( lambda *args, **kwargs: "dispatched", ) coverage_request = make_pr( + headRefOid="a" * 40, reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), + **opencode_review("CHANGES_REQUESTED", "a" * 40), "body": ( - "OpenCode cannot approve yet because required coverage evidence " - "did not pass. The coverage-evidence gate reported that required " - "test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -2713,15 +2723,14 @@ def test_coverage_retry_disables_auto_merge_before_dispatch(monkeypatch): lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", ) coverage_request = make_pr( + headRefOid="a" * 40, autoMergeRequest={"enabledAt": "now"}, reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), + **opencode_review("CHANGES_REQUESTED", "a" * 40), "body": ( - "OpenCode cannot approve yet because required coverage evidence " - "did not pass. The coverage-evidence gate reported that required " - "test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -2763,14 +2772,13 @@ def test_coverage_retry_waits_for_visible_opencode_run(monkeypatch): lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", ) coverage_request = make_pr( + headRefOid="a" * 40, reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), + **opencode_review("CHANGES_REQUESTED", "a" * 40), "body": ( - "OpenCode cannot approve yet because required coverage evidence " - "did not pass. The coverage-evidence gate reported that required " - "test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -2813,14 +2821,14 @@ def test_coverage_retry_disables_auto_merge_for_visible_opencode_run(monkeypatch lambda repo, pr, dry_run: disabled.append((repo, pr["number"], dry_run)), ) coverage_request = make_pr( + headRefOid="a" * 40, autoMergeRequest={"enabledAt": "now"}, reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), + **opencode_review("CHANGES_REQUESTED", "a" * 40), "body": ( - "coverage evidence did not pass; coverage-evidence reported that " - "required test/docstring evidence was not proven" + canonical_coverage_body() ), } ] @@ -2870,19 +2878,18 @@ def test_coverage_retry_waits_for_same_head_retry_floor(monkeypatch): lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", ) coverage_request = make_pr( + headRefOid="a" * 40, autoMergeRequest={"enabledAt": "now"}, reviews={ "nodes": [ { **opencode_review( "CHANGES_REQUESTED", - "head", + "a" * 40, submitted_at="2999-01-01T00:00:00Z", ), "body": ( - "OpenCode cannot approve yet because required coverage evidence " - "did not pass. The coverage-evidence gate reported that required " - "test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -2917,15 +2924,14 @@ def test_coverage_retry_floor_uses_latest_dispatch_timestamp(monkeypatch): coverage_review = { **opencode_review( "CHANGES_REQUESTED", - "head", + "a" * 40, submitted_at="2026-08-24T00:00:00Z", ), "body": ( - "OpenCode cannot approve yet because required coverage evidence did not pass. " - "The coverage-evidence gate reported that required test/docstring evidence was not proven." + canonical_coverage_body() ), } - coverage_request = make_pr(reviews={"nodes": [coverage_review]}) + coverage_request = make_pr(headRefOid="a" * 40, reviews={"nodes": [coverage_review]}) monkeypatch.setattr( sched, "latest_opencode_dispatch_started_at", @@ -2956,17 +2962,17 @@ def test_coverage_retry_wait_reason_fails_closed_when_dispatch_history_is_unavai ): """Unavailable dispatch history prevents an unbounded same-head retry.""" coverage_request = make_pr( + headRefOid="a" * 40, reviews={ "nodes": [ { **opencode_review( "CHANGES_REQUESTED", - "head", + "a" * 40, submitted_at="2026-08-24T00:00:00Z", ), "body": ( - "coverage evidence did not pass; coverage-evidence reported that " - "required test/docstring evidence was not proven" + canonical_coverage_body() ), } ] @@ -2990,17 +2996,17 @@ def test_coverage_retry_wait_reason_fails_closed_when_dispatch_history_is_unavai def test_coverage_retry_floor_keeps_newer_review_timestamp(monkeypatch): """An older dispatch cannot extend or replace the newer review timestamp.""" coverage_request = make_pr( + headRefOid="a" * 40, reviews={ "nodes": [ { **opencode_review( "CHANGES_REQUESTED", - "head", + "a" * 40, submitted_at="2026-08-24T02:00:00Z", ), "body": ( - "coverage evidence did not pass; coverage-evidence reported that " - "required test/docstring evidence was not proven" + canonical_coverage_body() ), } ] @@ -3027,17 +3033,16 @@ def test_coverage_retry_without_timestamp_fails_closed(monkeypatch): "ContextualWisdomLab/.github", ) monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_: None) - coverage_review = opencode_review("CHANGES_REQUESTED", "head") + coverage_review = opencode_review("CHANGES_REQUESTED", "a" * 40) coverage_review.pop("submittedAt") coverage_request = make_pr( + headRefOid="a" * 40, reviews={ "nodes": [ { **coverage_review, "body": ( - "OpenCode cannot approve yet because required coverage evidence " - "did not pass. The coverage-evidence gate reported that required " - "test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -3080,14 +3085,13 @@ def test_coverage_retry_keeps_failed_opencode_workflow_siblings_fail_closed( lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", ) coverage_request = make_pr( + headRefOid="a" * 40, reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), + **opencode_review("CHANGES_REQUESTED", "a" * 40), "body": ( - "OpenCode cannot approve yet because required coverage evidence " - "did not pass. The coverage-evidence gate reported that required " - "test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -3142,16 +3146,15 @@ def test_conflicting_coverage_retry_blocks_with_conflict_guidance(monkeypatch): lambda *args, **kwargs: dispatched.append((args, kwargs)) or "dispatched", ) coverage_request = make_pr( + headRefOid="a" * 40, mergeStateStatus="CONFLICTING", restMergeableState="CONFLICTING", reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), + **opencode_review("CHANGES_REQUESTED", "a" * 40), "body": ( - "OpenCode cannot approve yet because required coverage evidence " - "did not pass. The coverage-evidence gate reported that required " - "test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -3341,13 +3344,13 @@ def test_review_state_and_failed_checks(): assert not sched.has_current_head_changes_requested(superseded) coverage_request = make_pr( + headRefOid="a" * 40, reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), + **opencode_review("CHANGES_REQUESTED", "a" * 40), "body": ( - "OpenCode cannot approve yet because required coverage evidence did not pass. " - "The coverage-evidence gate reported that required test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -7322,13 +7325,13 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert merge_state in conflict_with_stale_review.reason assert update_calls == [] coverage_request = make_pr( + headRefOid="a" * 40, reviews={ "nodes": [ { - **opencode_review("CHANGES_REQUESTED", "head"), + **opencode_review("CHANGES_REQUESTED", "a" * 40), "body": ( - "OpenCode cannot approve yet because required coverage evidence did not pass. " - "The coverage-evidence gate reported that required test/docstring evidence was not proven." + canonical_coverage_body() ), } ] @@ -7416,7 +7419,7 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert coverage_decision.reason == ( "current-head OpenCode coverage blocker is cleared; same-head OpenCode re-dispatched" ) - assert dispatched == [("owner/repo", "OpenCode Review", "head", True)] + assert dispatched == [("owner/repo", "OpenCode Review", "a" * 40, True)] coverage_request["statusCheckRollup"]["contexts"]["nodes"].append( {