diff --git a/.github/workflows/central-review.yml b/.github/workflows/central-review.yml index 2544fbf..2cc4374 100644 --- a/.github/workflows/central-review.yml +++ b/.github/workflows/central-review.yml @@ -160,7 +160,7 @@ jobs: printf 'Using trusted CodeGraph CLI %s.\n' "$("$codegraph_bin" --version)" echo "$trusted_root/node_modules/.bin" >>"$GITHUB_PATH" - - name: Wait for non-OpenCode current-head checks + - name: Wait for review-independent current-head checks env: GH_TOKEN: ${{ steps.noema_app.outputs.token }} run: | @@ -172,16 +172,19 @@ jobs: "$EXPECTED_HEAD_SHA" "$live" exit 1 fi + # These exact checks consume review evidence themselves. Waiting on + # either one here creates a cycle: Noema waits for the governance + # check while the governance check waits for Noema/OpenCode. pending="$(gh api "repos/${TARGET_REPOSITORY}/commits/${EXPECTED_HEAD_SHA}/check-runs?per_page=100" \ - --jq '[.check_runs[] | select(.name != "opencode-review" and .status != "completed") | .name] | unique | join(", ")')" + --jq '[.check_runs[] | select((.name != "opencode-review" and .name != "metadata-only gate evaluation") and .status != "completed") | .name] | unique | join(", ")')" if [ -z "$pending" ]; then - echo "All non-OpenCode current-head checks are complete." + echo "All review-independent current-head checks are complete." exit 0 fi printf 'Noema current-head check wait %s/90: %s\n' "$attempt" "$pending" sleep 60 done - echo "::error::Noema waited 90 minutes but non-OpenCode current-head checks are still incomplete." + echo "::error::Noema waited 90 minutes but review-independent current-head checks are still incomplete." exit 1 - name: Run independent PydanticAI review and publish current-head verdict diff --git a/reviewer/noema_reviewer/gating.py b/reviewer/noema_reviewer/gating.py index 3d47ed7..5933dcb 100644 --- a/reviewer/noema_reviewer/gating.py +++ b/reviewer/noema_reviewer/gating.py @@ -26,9 +26,12 @@ # Noema is an independent reviewer. Treating the primary OpenCode review check # as a deterministic finding would make each reviewer wait on the other and -# deadlock the two-reviewer rule. Every other failed current-head check remains -# blocking. -INDEPENDENT_PRIMARY_CHECK_NAMES = frozenset({"opencode-review"}) +# deadlock the two-reviewer rule. The metadata-only gate is also downstream of +# review evidence, so it cannot be used as evidence against an independent +# review. Every other failed current-head check remains blocking. +REVIEW_DEPENDENT_CHECK_NAMES = frozenset( + {"opencode-review", "metadata-only gate evaluation"} +) def missing_evidence(manifest: ReviewManifest) -> list[str]: @@ -117,7 +120,7 @@ def failed_checks_as_review(manifest: ReviewManifest) -> list[Finding]: recommendation="Fix the logged root cause and rerun the check on the current head.", ) for check in manifest.check_conclusions - if check.name not in INDEPENDENT_PRIMARY_CHECK_NAMES + if check.name not in REVIEW_DEPENDENT_CHECK_NAMES and check.conclusion.lower() in blocking_conclusions ] diff --git a/reviewer/tests/test_central_review_workflow.py b/reviewer/tests/test_central_review_workflow.py new file mode 100644 index 0000000..8819e92 --- /dev/null +++ b/reviewer/tests/test_central_review_workflow.py @@ -0,0 +1,17 @@ +"""Contracts for the trusted central Noema review workflow.""" + +from pathlib import Path + + +def test_review_wait_excludes_only_exact_review_dependent_checks() -> None: + """The independent reviewer must not wait on checks that consume its verdict.""" + repo_root = Path(__file__).resolve().parents[2] + workflow = (repo_root / ".github/workflows/central-review.yml").read_text( + encoding="utf-8" + ) + + assert "Wait for review-independent current-head checks" in workflow + assert '.name != "opencode-review"' in workflow + assert '.name != "metadata-only gate evaluation"' in workflow + assert "All review-independent current-head checks are complete." in workflow + assert "non-OpenCode current-head checks" not in workflow diff --git a/reviewer/tests/test_gating.py b/reviewer/tests/test_gating.py index a0c5129..798f605 100644 --- a/reviewer/tests/test_gating.py +++ b/reviewer/tests/test_gating.py @@ -106,6 +106,19 @@ def test_primary_opencode_check_does_not_deadlock_independent_noema() -> None: assert enforce_security_and_check_gates(manifest, verdict).verdict is Verdict.APPROVE +def test_review_dependent_metadata_gate_does_not_deadlock_independent_noema() -> None: + """A downstream metadata controller cannot be a prerequisite for its reviewer.""" + manifest = _full_manifest( + check_conclusions=[ + CheckConclusion(name="metadata-only gate evaluation", conclusion="failure"), + CheckConclusion(name="build", conclusion="success"), + ] + ) + assert failed_checks_as_review(manifest) == [] + verdict = ReviewVerdict(verdict=Verdict.APPROVE, summary="independent evidence passed") + assert enforce_security_and_check_gates(manifest, verdict).verdict is Verdict.APPROVE + + def test_similarly_named_failed_check_remains_blocking() -> None: """The independence exception cannot hide a similarly named failed check.""" manifest = _full_manifest( @@ -114,6 +127,16 @@ def test_similarly_named_failed_check_remains_blocking() -> None: assert failed_checks_as_review(manifest) +def test_similarly_named_metadata_check_remains_blocking() -> None: + """Only the exact downstream metadata gate receives the cycle exception.""" + manifest = _full_manifest( + check_conclusions=[ + CheckConclusion(name="metadata-only gate evaluation copy", conclusion="failure") + ] + ) + assert failed_checks_as_review(manifest) + + def test_unresolved_current_thread_downgrades_approval() -> None: """An unresolved non-outdated inline thread is a deterministic blocker.""" manifest = _full_manifest(