fix(tests): stub sleep in OpenCode poll regression tests, salvage #1706

tests/test_opencode_live_draft_state_regression.py

@@ -34,6 +34,14 @@ def _write_live_state_gh(
     for exercising a missing/null/non-string/unexpected ``state`` field that
     the convenience ``live_draft``/``live_head``/``live_state`` parameters
     cannot express.
+
+    Also stubs ``sleep`` to return instantly: ``fail_closed_script()``'s
+    transport-failure retry path really does ``sleep "$poll_interval_seconds"``
+    (60s) between attempts, and this fixture's later-call sentinel exit code
+    drives that path to its 3-failure fail-closed threshold in
+    ``test_stale_draft_verdict_event_does_not_exempt_live_ready_pr`` -- without
+    this stub that test performs two genuine 60s sleeps (~120s real
+    wall-clock time per run) instead of running fast.
     """
     payload = json.dumps(
         live_payload_override
@@ -70,6 +78,9 @@ def evaluate_receipts(reviews, head_sha, *, is_draft):
         encoding="utf-8",
     )
     fake_gh.chmod(fake_gh.stat().st_mode | 0o111)
+    fake_sleep = bin_dir / "sleep"
+    fake_sleep.write_text("#!/usr/bin/env bash\nexit 0\n", encoding="utf-8")
+    fake_sleep.chmod(fake_sleep.stat().st_mode | 0o111)
 
 
 def _run_step(
@@ -138,12 +149,13 @@ def test_stale_draft_verdict_event_does_not_exempt_live_ready_pr(
 
     Unlike ``request_review_script()``'s single unguarded live-PR fetch, this
     step's post-draft-check Reviews API poll retries a transport failure up
-    to ``max_poll_transport_failures`` times (with a real backoff sleep
-    between attempts) before failing closed with its own exit 1 and
-    diagnostic -- so the fixture's synthetic unmocked-call sentinel exit code
-    never reaches this script's own exit status, unlike the sibling test
-    above. The "stale" continuation message is still emitted first, proving
-    the step did not silently exempt the live-ready PR from verdict polling.
+    to ``max_poll_transport_failures`` times (with a stubbed, instant backoff
+    "sleep" between attempts -- see ``_write_live_state_gh``) before failing
+    closed with its own exit 1 and diagnostic -- so the fixture's synthetic
+    unmocked-call sentinel exit code never reaches this script's own exit
+    status, unlike the sibling test above. The "stale" continuation message
+    is still emitted first, proving the step did not silently exempt the
+    live-ready PR from verdict polling.
     """
     result = _run_step(tmp_path, fail_closed_script(), live_draft=False)
 
@@ -186,15 +198,29 @@ def test_stale_draft_request_reuses_live_ready_approval(tmp_path: Path) -> None:
 
 
 @pytest.mark.parametrize("script", (request_review_script(), fail_closed_script()))
-def test_draft_exemption_fails_closed_when_live_head_moved(
+def test_draft_exemption_applies_even_when_live_head_has_moved(
     tmp_path: Path,
     script: str,
 ) -> None:
-    """The event cannot exempt a different live head even when it is still draft."""
+    """A still-draft PR exempts before the head-match check ever runs.
+
+    #1697 reordered the live-state checks so closed/draft admission is
+    evaluated before the head-SHA-match check (a draft PR whose live head
+    moved between the event snapshot and this step's own live re-fetch must
+    not fail closed with red-X noise -- see
+    ``ContextualWisdomLab/contextual-orchestrator`` PR #1000). The
+    head-moved branch is therefore unreachable while still draft: this
+    exercise now exempts via the draft check, not the head-match check.
+    Equivalent direct coverage of the production step lives in
+    ``test_opencode_required_verdict_regression.py``'s
+    ``test_request_review_step_exempts_a_draft_pr_whose_live_head_has_moved``
+    and ``test_fail_closed_step_exempts_a_draft_pr_whose_live_head_has_moved``.
+    """
     result = _run_step(tmp_path, script, live_draft=True, live_head="b" * 40)
 
-    assert result.returncode == 1
-    assert "head moved while validating live" in result.stdout
+    assert result.returncode == 0, result.stderr
+    assert "still a draft on the live exact head" in result.stdout
+    assert "head moved" not in result.stdout
 
 
 @pytest.mark.parametrize("script", (request_review_script(), fail_closed_script()))

tests/test_opencode_poll_self_retirement.py

@@ -35,8 +35,16 @@ def _run_poll_loop(
     reviews: list[dict[str, object]] | None = None,
     fail_live_pr_attempts: int = 0,
     fail_review_attempts: int = 0,
+    date_epochs: list[int] | None = None,
 ) -> tuple[subprocess.CompletedProcess[str], list[str]]:
-    """Execute the production poll body against a deterministic fake ``gh``."""
+    """Execute the production poll body against a deterministic fake ``gh``.
+
+    ``date_epochs``, when given, stubs ``date`` to return each listed epoch
+    in turn (clamped to the last entry once exhausted) instead of the real
+    clock -- letting a test fast-forward past the real
+    ``poll_deadline_epoch`` wall-clock deadline after a chosen number of
+    genuinely-executed loop iterations, without ever sleeping for real time.
+    """
     call_log = tmp_path / "gh-calls.log"
     live_fail_counter = tmp_path / "live-pr-failures"
     review_fail_counter = tmp_path / "review-failures"
@@ -84,6 +92,35 @@ def _run_poll_loop(
     )
     fake_timeout.chmod(0o755)
 
+    env_overrides: dict[str, str] = {}
+    if date_epochs is not None:
+        date_epochs_file = tmp_path / "date-epochs"
+        date_epochs_file.write_text(
+            "\n".join(str(epoch) for epoch in date_epochs) + "\n", encoding="utf-8"
+        )
+        date_counter = tmp_path / "date-calls"
+        fake_date = tmp_path / "date"
+        fake_date.write_text(
+            """#!/bin/sh
+set -eu
+count=0
+if [ -e "$FAKE_DATE_COUNTER" ]; then
+  count="$(cat "$FAKE_DATE_COUNTER")"
+fi
+count=$((count + 1))
+printf '%s\\n' "$count" > "$FAKE_DATE_COUNTER"
+line="$(sed -n "${count}p" "$FAKE_DATE_EPOCHS")"
+if [ -z "$line" ]; then
+  line="$(tail -n1 "$FAKE_DATE_EPOCHS")"
+fi
+printf '%s\\n' "$line"
+""",
+            encoding="utf-8",
+        )
+        fake_date.chmod(0o755)
+        env_overrides["FAKE_DATE_EPOCHS"] = str(date_epochs_file)
+        env_overrides["FAKE_DATE_COUNTER"] = str(date_counter)
+
     script = "\n".join(
         (
             "set -euo pipefail",
@@ -92,6 +129,7 @@ def _run_poll_loop(
             'review_poll_failures=0',
             'max_poll_transport_failures=3',
             'poll_interval_seconds=60',
+            'poll_deadline_epoch=$(( $(date +%s) + 10800 ))',
             "while :; do",
             _poll_loop(),
             "done",
@@ -111,6 +149,7 @@ def _run_poll_loop(
             "GH_REVIEW_FAIL_COUNTER": str(review_fail_counter),
             "GH_LIVE_PR": json.dumps(live_pr),
             "GH_REVIEWS": json.dumps(reviews or []),
+            **env_overrides,
         }
     )
     result = subprocess.run(
@@ -324,3 +363,98 @@ def test_self_retirement_does_not_replace_semantic_review_with_a_short_timeout()
     assert "while :; do" in target_job
     assert "poll_interval_seconds=60" in target_job
     assert 'sleep "$poll_interval_seconds"' in target_job
+
+
+def test_poll_fails_closed_after_wall_clock_deadline_with_every_gh_call_succeeding(
+    tmp_path: Path,
+) -> None:
+    """The zombie scenario: no transport failure ever occurs, yet no verdict posts.
+
+    `max_poll_transport_failures` cannot catch this -- every `gh` call
+    below succeeds -- so only a genuinely distinct wall-clock deadline
+    (`poll_deadline_epoch`, computed once before the loop) can release the
+    runner. A fake `date` fast-forwards past the real production 10800s
+    (180-minute) bound only after two full, genuinely-executed fast
+    iterations (proving the check is a real per-iteration wall-clock
+    comparison, not a check that fires before any work happens), without
+    this test ever sleeping for real time.
+    """
+    head_sha = "5" * 40
+    result, calls = _run_poll_loop(
+        tmp_path,
+        head_sha=head_sha,
+        live_pr={"head": {"sha": head_sha}, "draft": False, "state": "open"},
+        reviews=[],  # opencode-agent never posts a review on this head
+        date_epochs=[1000, 1000, 1000, 999999999999],
+    )
+
+    assert result.returncode == 1
+    assert (
+        "::error::No current-head OpenCode verdict after 180 minutes of "
+        "polling; failing closed and releasing the runner." in result.stdout
+    )
+    # Distinct diagnostic from the transport-failure path: nothing here failed.
+    assert "consecutive times" not in result.stdout
+    assert calls == [
+        "api repos/ContextualWisdomLab/example/pulls/42",
+        "api --paginate repos/ContextualWisdomLab/example/pulls/42/reviews?per_page=100",
+        "api repos/ContextualWisdomLab/example/pulls/42",
+        "api --paginate repos/ContextualWisdomLab/example/pulls/42/reviews?per_page=100",
+    ]
+
+
+def test_poll_wall_clock_deadline_does_not_interfere_with_a_fast_verdict(
+    tmp_path: Path,
+) -> None:
+    """A verdict arriving on the first poll is unaffected by the new bound."""
+    head_sha = "6" * 40
+    result, calls = _run_poll_loop(
+        tmp_path,
+        head_sha=head_sha,
+        live_pr={"head": {"sha": head_sha}, "draft": False, "state": "open"},
+        reviews=[
+            {
+                "user": {"login": "opencode-agent[bot]"},
+                "commit_id": head_sha,
+                "state": "APPROVED",
+                "body": "Source-backed current-head semantic review.",
+            }
+        ],
+        date_epochs=[1000, 1000],  # baseline call, then one in-bounds iteration check
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert calls == [
+        "api repos/ContextualWisdomLab/example/pulls/42",
+        "api --paginate repos/ContextualWisdomLab/example/pulls/42/reviews?per_page=100",
+    ]
+    assert "No current-head OpenCode verdict after" not in result.stdout
+
+
+def test_wall_clock_deadline_is_distinct_from_and_additional_to_transport_counter() -> None:
+    """The new bound sits alongside, not in place of, the transport-failure counter.
+
+    Pins the production shape so a future edit cannot quietly collapse the
+    two into one, or drop the wall-clock bound back to unbounded: both
+    `max_poll_transport_failures` (existing) and `poll_deadline_epoch`
+    (computed once before the loop) must be present, and the wall-clock
+    check must live inside the `while :; do` loop body -- not as a
+    job-level `timeout-minutes:`, which would kill the runner mid-request
+    instead of failing closed with a clear diagnostic.
+    """
+    target_job = WORKFLOW.read_text(encoding="utf-8").split(
+        "  opencode-review-target:\n", 1
+    )[1].split("\n  cancel-superseded-opencode-review-runs:\n", 1)[0]
+    assert "max_poll_transport_failures=3" in target_job
+    assert "poll_deadline_epoch=$(( $(date -u +%s) + 10800 ))" in target_job
+    loop = _poll_loop()
+    assert 'if [ "$(date -u +%s)" -ge "$poll_deadline_epoch" ]; then' in loop
+    assert (
+        "::error::No current-head OpenCode verdict after 180 minutes of "
+        "polling; failing closed and releasing the runner." in loop
+    )
+    # The deadline check must precede this iteration's gh calls so an
+    # already-expired deadline never spends another API request.
+    assert loop.index('-ge "$poll_deadline_epoch"') < loop.index(
+        'live_poll_pr="$(timeout 30s gh api'
+    )

tests/test_opencode_required_verdict_regression.py

@@ -280,7 +280,16 @@ def test_required_workflow_cannot_succeed_with_an_echo_only_placeholder() -> Non
 
 
 def _write_live_pr_then_refusing_gh(bin_dir: Path) -> None:
-    """Serve the authoritative live PR lookup, then reject downstream GitHub I/O."""
+    """Serve the authoritative live PR lookup, then reject downstream GitHub I/O.
+
+    Also stubs ``sleep`` to return instantly. The production poll loop's
+    transport-failure path really does ``sleep "$poll_interval_seconds"``
+    (60s) between retries -- without this stub, a test that drives that path
+    to its 3-failure fail-closed threshold performs two genuine 60s sleeps
+    (observed directly: this exact gap made
+    ``test_fail_closed_step_still_polls_for_a_non_draft_pr`` take ~120s of
+    real wall-clock time per run instead of running fast).
+    """
     fake_gh = bin_dir / "gh"
     fake_gh.write_text(
         "#!/usr/bin/env bash\n"
@@ -294,6 +303,9 @@ def _write_live_pr_then_refusing_gh(bin_dir: Path) -> None:
         encoding="utf-8",
     )
     fake_gh.chmod(fake_gh.stat().st_mode | 0o111)
+    fake_sleep = bin_dir / "sleep"
+    fake_sleep.write_text("#!/usr/bin/env bash\nexit 0\n", encoding="utf-8")
+    fake_sleep.chmod(fake_sleep.stat().st_mode | 0o111)
 
 
 def _run_fail_closed_step(
@@ -623,9 +635,10 @@ def test_fail_closed_step_still_polls_for_a_non_draft_pr(tmp_path: Path) -> None
     """A non-draft PR must still reach the Reviews API call (not exempted).
 
     Unlike the request-review step's single unguarded call, the Reviews API
-    fetch here retries a transport failure up to three times (with a real
-    backoff sleep between attempts) before failing closed with its own exit
-    1, so the fixture's synthetic unmocked-call sentinel exit code (17)
+    fetch here retries a transport failure up to three times (with a
+    stubbed, instant backoff "sleep" between attempts -- see
+    ``_write_live_pr_then_refusing_gh``) before failing closed with its own
+    exit 1, so the fixture's synthetic unmocked-call sentinel exit code (17)
     never reaches this script's own exit status -- it is absorbed by the
     retry loop instead, which still logs the sentinel's stderr diagnostic on
     every attempt.