fix(ci): exempt draft PRs from the opencode-review current-head verdict gate

.github/workflows/opencode-review.yml

@@ -9,7 +9,7 @@ on:
   # content and never binds repository secrets. Privileged review execution is
   # isolated in opencode-review-dispatch.yml on repository_dispatch only.
   pull_request_target:
-    types: [opened, synchronize, reopened, ready_for_review, closed]
+    types: [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed]
 
 concurrency:
   group: >-
@@ -242,7 +242,7 @@ jobs:
       id-token: write
     steps:
       - name: Request current-head OpenCode review execution
-        if: github.event.action != 'closed'
+        if: github.event.action != 'closed' && !github.event.pull_request.draft
         env:
           OIDC_AUDIENCE: opencode-github-action
           OPENCODE_API_BASE_URL: https://api.opencode.ai
@@ -287,6 +287,10 @@ jobs:
             echo "PR closed; a current-head OpenCode verdict is not required."
             exit 0
           fi
+          if [ "${{ github.event.pull_request.draft }}" = "true" ]; then
+            echo "PR is a draft; a current-head OpenCode verdict is not required until it is marked ready for review."
+            exit 0
+          fi
           if [ -z "${PR_NUMBER:-}" ] || [ -z "${HEAD_SHA:-}" ]; then
             echo "::error::Missing PR number or head SHA; cannot verify a current-head OpenCode verdict."
             exit 1

CHANGELOG.md

@@ -5,6 +5,42 @@ this file. The format follows Keep a Changelog, and versioned releases follow
 Semantic Versioning where the repository publishes a release.
 
 ## [Unreleased]
+- Fix `opencode-review.yml`'s required `opencode-review-target` check
+  (`Fail closed without a current-head OpenCode verdict`) reporting a hard
+  `exit 1` failure on every push to a draft PR, forever, until the PR is
+  marked ready for review. Root cause: `scripts/ci/pr_review_merge_scheduler.py`
+  deliberately never dispatches an OpenCode review request for a draft PR
+  (`if pr.get("isDraft"): return Decision(number, "skip", "draft PR")`), but
+  the required check had no draft handling and unconditionally demanded a
+  current-head verdict on every `opened`/`synchronize`/`reopened` event
+  regardless. Adds a `github.event.pull_request.draft` early-exit mirroring
+  the existing `closed` early-exit in style and placement — the job still
+  always runs and reports a status, it just reports success instead of a
+  false-alarm failure for a state that was never going to get a verdict in
+  the first place. The gate is unchanged once a PR is marked ready for
+  review (`ready_for_review` and subsequent `synchronize` events carry
+  `draft: false`). See `tests/test_opencode_required_verdict_regression.py`
+  for shell-level regression coverage executing the production step body.
+  Follow-up: add `converted_to_draft` to the same workflow's
+  `pull_request_target` trigger types. Without it, a ready PR converted
+  back to draft with no new commit never gets a fresh required-workflow
+  run, so its previously failed `opencode-review` check keeps showing
+  failure indefinitely even though the draft exemption above would have
+  passed it.
+- Also gate `opencode-review-target`'s earlier `Request current-head
+  OpenCode review execution` step on `!github.event.pull_request.draft`,
+  not only the later `Fail closed without a current-head OpenCode verdict`
+  step. That earlier step performs its own OIDC token exchange, OpenCode
+  app-token exchange, and `repository_dispatch` call under
+  `set -euo pipefail`; any transient failure there (an OIDC hiccup, the
+  OpenCode token endpoint erroring, a `gh api` dispatch failure) stopped
+  the job with `exit 1` before the later step's draft exemption ever ran,
+  so a genuinely infrastructure-caused outage could still turn a draft
+  PR's required check red even after the fix above. The dispatch step now
+  short-circuits for drafts exactly like the verdict step does, while
+  still running for every non-draft, non-`closed` event. See the new
+  `test_request_review_execution_step_is_also_gated_on_draft` regression
+  test asserting the exact `if:` condition string on this step.
 - Harden the review sidecar's per-account catalog cap against silent drift:
   `contextual_orchestrator_review_launcher.py`'s two
   `build_zdr_prioritized_catalog` call sites now source their
@@ -549,7 +585,8 @@ Semantic Versioning where the repository publishes a release.
   Informational, no change: the gap-baseline's repeated review-round
   narrative is this repo's own documented, intentional convention
   (ADR-0002: the baseline is "an operational snapshot," not a duplicate of
-  the ADR's design record), not accidental redundancy.- Raise `contextual_orchestrator_review_sidecar.sh`'s
+  the ADR's design record), not accidental redundancy.
+- Raise `contextual_orchestrator_review_sidecar.sh`'s
   `ORCHESTRATOR_CATALOG_FAMILY_CAP` default from 4 to 8: root-caused the
   live "no provider route passed the Strix plain-chat preflight" outage
   blocking `noema-review`/`opencode-review`/`strix` org-wide to

scripts/ci/test_strix_quick_gate.sh

@@ -502,7 +502,7 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() {
 	local opencode_config="$REPO_ROOT/opencode.jsonc"
 
 	assert_file_contains "$bootstrap_file" "pull_request_target:" "opencode required workflow loads its metadata-only bootstrap from the protected base ref"
-	assert_file_contains "$bootstrap_file" "types: [opened, synchronize, reopened, ready_for_review, closed]" "opencode required workflow reacts to current PR head changes and closed-PR cleanup"
+	assert_file_contains "$bootstrap_file" "types: [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed]" "opencode required workflow reacts to current PR head changes, draft reconversion, and closed-PR cleanup"
 	assert_file_contains "$bootstrap_file" "required-workflow-bootstrap:" "opencode required workflow materializes at least one job for pull_request ruleset runs"
 	assert_file_contains "$bootstrap_file" "Required OpenCode workflow materialized without checking out or" "opencode required workflow bootstrap documents its data-only trust boundary"
 	assert_file_contains "$bootstrap_file" "coverage-source-tree:" "opencode required workflow preserves the stable coverage-source-tree branch-protection context"

tests/test_opencode_required_verdict_regression.py

@@ -3,7 +3,9 @@
 from __future__ import annotations
 
 import json
+import os
 import shutil
+import stat
 import subprocess
 from pathlib import Path
 
@@ -13,6 +15,7 @@
 HEAD = "a" * 40
 WORKFLOW = Path(".github/workflows/opencode-review.yml")
 STATUS_HELPER = Path("scripts/ci/opencode_dispatch_status.py")
+STEP_NAME = "Fail closed without a current-head OpenCode verdict"
 
 
 def review(*, state: str, commit_id: str = HEAD, body: str = "") -> dict[str, object]:
@@ -117,3 +120,245 @@ def test_required_workflow_cannot_succeed_with_an_echo_only_placeholder() -> Non
         "Review approval remains a separate current-head PR review requirement"
         not in workflow
     )
+
+
+def test_required_workflow_reruns_on_draft_reconversion() -> None:
+    """A ready PR converted back to draft must get a fresh required-workflow run.
+
+    Without ``converted_to_draft`` in the trigger list, a PR that goes
+    ready -> draft with no new commit keeps its previously failed
+    ``opencode-review`` check forever: no event refires the job that could
+    apply the draft exemption below.
+    """
+    workflow = WORKFLOW.read_text(encoding="utf-8")
+    assert (
+        "types: [opened, synchronize, reopened, ready_for_review, "
+        "converted_to_draft, closed]"
+    ) in workflow
+
+
+def test_request_review_execution_step_is_also_gated_on_draft() -> None:
+    """The dispatch step must not run for drafts either.
+
+    ``Fail closed without a current-head OpenCode verdict`` exempts drafts,
+    but the earlier ``Request current-head OpenCode review execution`` step
+    performs its own OIDC token exchange, OpenCode app-token exchange, and
+    ``repository_dispatch`` call under ``set -euo pipefail``, any of which
+    can fail on infrastructure trouble unrelated to draft status. Without a
+    matching draft guard on this step's own ``if:``, that failure happens
+    before the later exemption ever gets a chance to run, keeping the
+    required check red on every draft PR whenever OIDC or the dispatch API
+    has trouble.
+    """
+    workflow = WORKFLOW.read_text(encoding="utf-8")
+    lines = workflow.splitlines()
+    step_index = next(
+        index
+        for index, line in enumerate(lines)
+        if line.strip() == "- name: Request current-head OpenCode review execution"
+    )
+    if_line = next(
+        line.strip()
+        for line in lines[step_index + 1 :]
+        if line.strip().startswith("if:")
+    )
+    assert (
+        if_line
+        == "if: github.event.action != 'closed' && !github.event.pull_request.draft"
+    )
+
+
+def _extract_run_block(workflow_text: str, step_name: str) -> str:
+    """Return the literal bash text of one workflow step's ``run: |`` block."""
+    lines = workflow_text.splitlines()
+    step_index = next(
+        index for index, line in enumerate(lines) if line.strip() == f"- name: {step_name}"
+    )
+    run_index = next(
+        index
+        for index in range(step_index + 1, len(lines))
+        if lines[index].strip() == "run: |"
+    )
+    run_indent = len(lines[run_index]) - len(lines[run_index].lstrip())
+    block_lines = []
+    for line in lines[run_index + 1 :]:
+        if line.strip() and len(line) - len(line.lstrip()) <= run_indent:
+            break
+        block_lines.append(line[run_indent + 2 :] if len(line) >= run_indent + 2 else "")
+    return "\n".join(block_lines) + "\n"
+
+
+def _render_step(script: str, *, event_action: str, draft: str) -> str:
+    """Substitute the two inline ``${{ github.* }}`` expressions GitHub Actions
+    would resolve before invoking bash, so the raw step body becomes directly
+    executable outside of Actions."""
+    rendered = script.replace("${{ github.event.action }}", event_action)
+    rendered = rendered.replace("${{ github.event.pull_request.draft }}", draft)
+    assert "${{" not in rendered, "unresolved GitHub Actions expression remains"
+    return rendered
+
+
+def _write_refusing_gh(bin_dir: Path) -> None:
+    """Install a fake ``gh`` on PATH that fails loudly if it is ever invoked.
+
+    Used to prove an early-exit branch never reaches the Reviews API call.
+    """
+    fake_gh = bin_dir / "gh"
+    fake_gh.write_text(
+        "#!/usr/bin/env bash\n"
+        "echo 'unexpected gh invocation: the early-exit should have short-circuited' >&2\n"
+        "exit 17\n",
+        encoding="utf-8",
+    )
+    fake_gh.chmod(fake_gh.stat().st_mode | stat.S_IEXEC)
+
+
+def _write_reviews_gh(bin_dir: Path, reviews: list[dict[str, object]]) -> Path:
+    """Install a fake ``gh`` on PATH that serves a fixed Reviews API page."""
+    fake_gh = bin_dir / "gh"
+    fixture = bin_dir / "reviews.json"
+    fixture.write_text(json.dumps(reviews), encoding="utf-8")
+    fake_gh.write_text(
+        "#!/usr/bin/env bash\n"
+        "set -euo pipefail\n"
+        'test "$1" = api\n'
+        f"cat {fixture}\n",
+        encoding="utf-8",
+    )
+    fake_gh.chmod(fake_gh.stat().st_mode | stat.S_IEXEC)
+    return fake_gh
+
+
+def _run_step(
+    tmp_path: Path,
+    *,
+    event_action: str,
+    draft: str,
+    pr_number: str = "",
+    head_sha: str = "",
+    gh_fixture: str = "refuse",
+    reviews: list[dict[str, object]] | None = None,
+) -> subprocess.CompletedProcess[str]:
+    """Execute the production step body with the given event shape.
+
+    ``gh_fixture`` selects a fake ``gh`` on PATH: ``"refuse"`` fails loudly if
+    invoked (proving an early exit never reaches the Reviews API call), and
+    ``"reviews"`` serves ``reviews`` back from ``gh api``.
+    """
+    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")
+
+    workflow = WORKFLOW.read_text(encoding="utf-8")
+    script = _render_step(
+        _extract_run_block(workflow, STEP_NAME),
+        event_action=event_action,
+        draft=draft,
+    )
+
+    bin_dir = tmp_path / "bin"
+    bin_dir.mkdir()
+    if gh_fixture == "refuse":
+        _write_refusing_gh(bin_dir)
+    else:
+        _write_reviews_gh(bin_dir, reviews or [])
+
+    env = {
+        **os.environ,
+        "PATH": f"{bin_dir}:{os.environ['PATH']}",
+        "GH_TOKEN": "fake-token",
+        "TARGET_REPOSITORY": "ContextualWisdomLab/example",
+        "PR_NUMBER": pr_number,
+        "HEAD_SHA": head_sha,
+    }
+    return subprocess.run(
+        [bash],
+        input=script,
+        text=True,
+        capture_output=True,
+        check=False,
+        env=env,
+    )
+
+
+def test_draft_pr_short_circuits_before_the_reviews_api_call(tmp_path: Path) -> None:
+    """A draft PR passes without ever calling the Reviews API.
+
+    The merge scheduler (``scripts/ci/pr_review_merge_scheduler.py``) never
+    dispatches a review request for a draft PR, so this required check must
+    not demand a verdict that was never going to be requested. ``PR_NUMBER``
+    and ``HEAD_SHA`` are deliberately left unset here to prove the draft
+    early-exit runs before the "missing PR number or head SHA" fail-closed
+    check that follows it.
+    """
+    result = _run_step(
+        tmp_path,
+        event_action="synchronize",
+        draft="true",
+        gh_fixture="refuse",
+    )
+    assert result.returncode == 0, result.stderr
+    assert "PR is a draft" in result.stdout
+
+
+@pytest.mark.parametrize(
+    "event_action", ("opened", "synchronize", "reopened", "converted_to_draft")
+)
+def test_draft_pr_short_circuits_on_every_non_closed_event_type(
+    tmp_path: Path, event_action: str
+) -> None:
+    """The draft exemption applies uniformly across opened/synchronize/reopened."""
+    result = _run_step(
+        tmp_path,
+        event_action=event_action,
+        draft="true",
+        gh_fixture="refuse",
+    )
+    assert result.returncode == 0, result.stderr
+    assert "PR is a draft" in result.stdout
+
+
+def test_ready_for_review_pr_still_requires_a_current_head_verdict(
+    tmp_path: Path,
+) -> None:
+    """Once a PR is not a draft, the real gate still runs unchanged."""
+    result = _run_step(
+        tmp_path,
+        event_action="ready_for_review",
+        draft="false",
+        pr_number="1437",
+        head_sha=HEAD,
+        gh_fixture="reviews",
+        reviews=[review(state="APPROVED")],
+    )
+    assert result.returncode == 0, result.stderr
+    assert "Current-head OpenCode verdict: APPROVED." in result.stdout
+
+
+def test_non_draft_pr_without_a_verdict_still_fails_closed(tmp_path: Path) -> None:
+    """A non-draft PR with no matching review still fails closed as before."""
+    result = _run_step(
+        tmp_path,
+        event_action="synchronize",
+        draft="false",
+        pr_number="1437",
+        head_sha=HEAD,
+        gh_fixture="reviews",
+        reviews=[],
+    )
+    assert result.returncode == 1
+    assert "No APPROVED or CHANGES_REQUESTED from opencode-agent" in result.stdout
+
+
+def test_closed_pr_short_circuits_before_the_draft_check(tmp_path: Path) -> None:
+    """The pre-existing ``closed`` early-exit still takes precedence over draft."""
+    result = _run_step(
+        tmp_path,
+        event_action="closed",
+        draft="true",
+        gh_fixture="refuse",
+    )
+    assert result.returncode == 0, result.stderr
+    assert "PR closed; a current-head OpenCode verdict is not required." in result.stdout
+    assert "PR is a draft" not in result.stdout

tests/test_required_workflow_queue_contract.py

@@ -428,8 +428,9 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() -
         assert "github.event.action != 'closed'" in workflow
 
     opencode_bootstrap = workflow_text("opencode-review.yml")
-    assert "types: [opened, synchronize, reopened, ready_for_review, closed]" in (
-        opencode_bootstrap
+    assert (
+        "types: [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed]"
+        in opencode_bootstrap
     )
     assert "actions/checkout" not in opencode_bootstrap
     assert "${{ secrets." not in opencode_bootstrap