From 418af220fef27cee86087c6ea40758007c9282ba Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 22 Sep 2026 04:51:31 +0900 Subject: [PATCH] ci(opencode): fallback reviews never satisfy the required verdict The required opencode-review bootstrap accepted any opencode-agent CHANGES_REQUESTED, so a model-unavailable fallback blocker review turned the check green with no model verdict, and the receipt gate treated the same review as a formal receipt, suppressing the retry wake. Treat any fallback-marked review as MODEL_OUTPUT_UNAVAILABLE in both gates. Refs #2335 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LeLbwEgYXzCzpaFMTHDcBS --- .github/workflows/opencode-review.yml | 29 +++++--- CHANGELOG.md | 4 ++ scripts/ci/opencode_review_receipt_gate.py | 11 ++-- ...st_opencode_required_verdict_regression.py | 66 ++++++++++++++++++- tests/test_opencode_review_receipt_gate.py | 29 ++++++++ 5 files changed, 124 insertions(+), 15 deletions(-) diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index ec94e6d24e..2c8eb85bc1 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -572,21 +572,32 @@ jobs: ] | (last // {}) as $review | ($review.body // "" | ascii_downcase) as $body - | if $review.state == "CHANGES_REQUESTED" then + # A review written by the model-unavailable fallback (deterministic + # blockers or a fallback approval) is not a model verdict. Counting + # its CHANGES_REQUESTED made this required check green while no model + # had reviewed anything; it now surfaces as MODEL_OUTPUT_UNAVAILABLE. + | ([ + "deterministic current-head evidence", + "deterministic fallback approval", + "model-unavailable evidence fallback", + "did not emit a usable current-head control block", + "scope: `unsupported`", + "model-pool outcome: `unknown`" + ] | any(. as $marker | $body | contains($marker))) as $fallback + | if ($review.state == "CHANGES_REQUESTED" or $review.state == "APPROVED") and $fallback then + "MODEL_OUTPUT_UNAVAILABLE" + elif $review.state == "CHANGES_REQUESTED" then "CHANGES_REQUESTED" - elif $review.state == "APPROVED" - and ($body | contains("deterministic current-head evidence") | not) - and ($body | contains("deterministic fallback approval") | not) - and ($body | contains("model-unavailable evidence fallback") | not) - and ($body | contains("did not emit a usable current-head control block") | not) - and ($body | contains("scope: `unsupported`") | not) - and ($body | contains("model-pool outcome: `unknown`") | not) - then + elif $review.state == "APPROVED" then "APPROVED" else empty end ')" + if [ "$verdict" = "MODEL_OUTPUT_UNAVAILABLE" ]; then + echo "::error title=MODEL_OUTPUT_UNAVAILABLE::The latest opencode-agent review on the current head came from the model-unavailable evidence fallback, not from a model verdict. The required OpenCode review stays unsatisfied until a model-backed APPROVED or CHANGES_REQUESTED review lands on this head." + exit 1 + fi if [ -z "$verdict" ]; then echo "::error::No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. The dispatch workflow will rerun this failed job after publishing an authenticated exact-head verdict." exit 1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 34281625cb..454cdf34f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### A model-unavailable fallback review never satisfies the required OpenCode check + +- When the OpenCode model pool was exhausted, the fallback posted a deterministic CHANGES_REQUESTED blocker review. The required `opencode-review` bootstrap accepted any opencode-agent CHANGES_REQUESTED, so the check went green with no model verdict: from the 2026-08-27 gateway switch until the /v1 fix (#2333), no model verdict ran while required checks passed (for example run 34931908846, model=none). `opencode_review_receipt_gate.is_formal_receipt` likewise filtered fallback markers only for APPROVED, so the fallback review suppressed the scheduler wake that retries a real review. Both gates now treat a review carrying any fallback marker, in either state, as not a model verdict. The required step fails with an explicit `MODEL_OUTPUT_UNAVAILABLE` error, and the receipt gate keeps the retry wake alive. A later model-backed review on the same head still passes. Merges were never opened by the fallback; this fixes the check hiding the outage. Refs #2335. + ### Noema transport capacity schedules a bounded continuation re-dispatch - After gateway failover, HTTP 429/5xx no longer end only as a permanent required-check failure with `caller attempts=1`. ADR-0031 classifies that class as `provider_capacity_unavailable`, keeps the single gateway request per job, surfaces `provider_attempt_count` from the orchestrator error envelope, and authorizes at most two same-head `repository_dispatch` retries after a capped `Retry-After` or deterministic 60–180 s jitter. Review is never skipped. Refs #2165. diff --git a/scripts/ci/opencode_review_receipt_gate.py b/scripts/ci/opencode_review_receipt_gate.py index 4dcb24af88..88bea9d869 100644 --- a/scripts/ci/opencode_review_receipt_gate.py +++ b/scripts/ci/opencode_review_receipt_gate.py @@ -130,10 +130,11 @@ def is_formal_receipt( body = str(review.get("body") or "") if is_mention_or_malformed(body): return False, "mention, status-only, or malformed payload is not a formal review" - if state == "APPROVED" and any( - marker in body.casefold() for marker in FALLBACK_APPROVAL_MARKERS - ): - return False, "fallback approval is not a substantive formal review" + if any(marker in body.casefold() for marker in FALLBACK_APPROVAL_MARKERS): + # The model-unavailable fallback also writes CHANGES_REQUESTED blocker + # reviews. Neither state is a model verdict, so neither may suppress the + # scheduler wake that retries a real review once a model is available. + return False, "model-unavailable fallback review is not a model verdict" if is_draft and state == "APPROVED": return False, "draft must never receive bot APPROVE" return True, "current-head formal review" @@ -161,7 +162,7 @@ def evaluate_receipts( return review, reason if "never receive bot APPROVE" in reason: return None, reason - if "fallback approval" in reason: + if "fallback" in reason: return None, reason if reason.startswith("stale"): stale_hits += 1 diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index c764ad0ad2..718fe84109 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -138,7 +138,6 @@ def test_runtime_required_verdict_accepts_only_formal_current_head_states( [], [review(state="COMMENTED")], [review(state="APPROVED", commit_id="b" * 40)], - [review(state="APPROVED", body="deterministic fallback approval")], ), ) def test_runtime_required_verdict_rejects_nonpassing_evidence( @@ -148,6 +147,71 @@ def test_runtime_required_verdict_rejects_nonpassing_evidence( assert runtime_verdict(reviews) == "" +FALLBACK_CHANGES_REQUESTED_BODY = ( + "## Pull request overview\n\nOpenCode could not approve from deterministic current-head evidence " + "because GitHub Checks have failed.\n\n- Root cause: The model-unavailable evidence fallback is " + "allowed only when peer GitHub Checks are complete and clean." +) + + +@pytest.mark.parametrize( + "fallback", + ( + review(state="CHANGES_REQUESTED", body=FALLBACK_CHANGES_REQUESTED_BODY), + review(state="APPROVED", body="deterministic fallback approval"), + review(state="CHANGES_REQUESTED", body="Model-pool outcome: `unknown`"), + ), +) +def test_runtime_required_verdict_never_counts_a_model_unavailable_fallback(fallback) -> None: + """RED on main: a fallback CHANGES_REQUESTED made the required check green with no model verdict. + + Run 34931908846 ended with the model pool exhausted (model=none), yet the + fallback's deterministic blocker review satisfied this check. Any review + carrying a fallback marker now yields MODEL_OUTPUT_UNAVAILABLE instead. + """ + assert runtime_verdict([fallback]) == "MODEL_OUTPUT_UNAVAILABLE" + # A later real model verdict on the same head still passes. + assert runtime_verdict([fallback, review(state="CHANGES_REQUESTED", body="## Verdict\nFix the race.")]) == "CHANGES_REQUESTED" + + +def test_fail_closed_step_reports_model_unavailable_explicitly(tmp_path: Path) -> None: + """The step exits 1 with a MODEL_OUTPUT_UNAVAILABLE error, never success, for a fallback-only head.""" + bash = shutil.which("bash") + jq = shutil.which("jq") + if bash is None or jq is None: + pytest.skip("bash and jq are required to execute the production step body") + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + gh = bin_dir / "gh" + gh.write_text( + "#!/usr/bin/env bash\n" + 'if [[ "$*" == *"/reviews"* ]]; then printf \'%s\' "$FAKE_REVIEWS"; else printf \'%s\' "$LIVE_PR_JSON"; fi\n', + encoding="utf-8", + ) + gh.chmod(0o755) + result = subprocess.run( + [bash, "-c", fail_closed_script()], + env={ + **os.environ, + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "GH_TOKEN": "fake-token", + "TARGET_REPOSITORY": "ContextualWisdomLab/example", + "PR_NUMBER": "7", + "HEAD_SHA": HEAD, + "PR_ACTION": "synchronize", + "PR_DRAFT": "false", + "LIVE_PR_JSON": json.dumps({"draft": False, "head": {"sha": HEAD}, "state": "open"}), + "FAKE_REVIEWS": json.dumps([review(state="CHANGES_REQUESTED", body=FALLBACK_CHANGES_REQUESTED_BODY)]), + }, + text=True, + capture_output=True, + check=False, + ) + assert result.returncode == 1, result.stdout + result.stderr + assert "MODEL_OUTPUT_UNAVAILABLE" in result.stdout + assert "Current-head OpenCode verdict" not in result.stdout + + @pytest.mark.parametrize("state", ("APPROVED", "CHANGES_REQUESTED")) def test_runtime_required_verdict_ignores_later_nonformal_current_head_comment( state: str, diff --git a/tests/test_opencode_review_receipt_gate.py b/tests/test_opencode_review_receipt_gate.py index c971e2128a..1737880491 100644 --- a/tests/test_opencode_review_receipt_gate.py +++ b/tests/test_opencode_review_receipt_gate.py @@ -342,3 +342,32 @@ def unexpected_run(args, **kwargs): )(), ) assert receipt.load_reviews("-")[0]["commit_id"] == receipt.AFIPC_230_HEAD + + +def test_fallback_changes_requested_is_not_a_receipt_so_review_retries() -> None: + """RED on main: a fallback CHANGES_REQUESTED counted as a receipt and suppressed the retry wake. + + With the model pool exhausted, the fallback posts deterministic blocker + reviews. Treating them as a formal receipt stopped the scheduler from ever + retrying a real model review on that head. + """ + fallback = review( + commit=receipt.AFIPC_230_HEAD, + state="CHANGES_REQUESTED", + body=( + "## Pull request overview\n\nOpenCode could not approve from deterministic current-head " + "evidence because GitHub Checks have failed.\n\n- Root cause: The model-unavailable " + "evidence fallback is allowed only when peer GitHub Checks are complete and clean." + ), + ) + found, reason = receipt.evaluate_receipts([fallback], receipt.AFIPC_230_HEAD) + assert found is None + assert "fallback" in reason + real = review( + commit=receipt.AFIPC_230_HEAD, + state="CHANGES_REQUESTED", + body="## Verdict\nRequest changes: the race is unguarded.", + review_id=2, + ) + found, _reason = receipt.evaluate_receipts([fallback, real], receipt.AFIPC_230_HEAD) + assert found is not None