diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 21ee42e24c..9197018258 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -3203,10 +3203,7 @@ others) — this fix deliberately stayed scoped to the one file with direct, con starvation rather than a speculative sweep of every remaining occurrence. Worth revisiting each individually if queuing symptoms recur on them specifically. -**Separately found while validating this fix, not yet fixed:** `tests/test_pr_review_autofix_nvidia_nim_contract.py::test_review_fix_caller_runs_once_each_hour` -fails on a clean `origin/main` checkout, independent of this fix — `hourly-review-repair.yml` was renamed to -"Daily Review Recovery" and redesigned from one hourly cron to 17 staggered daily crons (one per target -repository), but this test still asserts the old single hourly `cron: "23 * * * *"`. Same bug class as the -`test_strix_quick_gate.sh` org-sweep-cron staleness found and fixed on `#1503` the same day: a test left -behind by a workflow redesign. Needs its own fix understanding the new staggered-daily design's actual -intended contract before rewriting the assertion — left for a dedicated follow-up rather than guessed at here. +**Resolved while validating this fix:** `hourly-review-repair.yml` is now the distributed `Daily Review +Recovery`; GitHub runs at `cron: "23 7 * * *"`, and the remaining repositories use their reviewed staggered +daily slots. `test_review_fix_caller_keeps_the_github_daily_recovery_slot` now enforces that current contract +and rejects the former hourly schedule instead of describing the repaired test as outstanding work. diff --git a/scripts/ci/audit_codeql_default_setup_rollout.py b/scripts/ci/audit_codeql_default_setup_rollout.py index 17eaa0146c..6637601593 100755 --- a/scripts/ci/audit_codeql_default_setup_rollout.py +++ b/scripts/ci/audit_codeql_default_setup_rollout.py @@ -269,6 +269,7 @@ def load_payload(path: Path | None, stdin: TextIO) -> list[dict[str, Any]]: def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + """Parse CLI arguments for either the file-payload or live-collection mode.""" parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("snapshots_json", nargs="?", type=Path) parser.add_argument("--repository") @@ -277,6 +278,7 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: def main(argv: list[str] | None = None) -> int: + """Audit CodeQL rollout state from file or live snapshots and print verdicts.""" args = parse_args(argv) try: live_mode = args.repository is not None or args.pr is not None diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 5ab7e830f3..797879642f 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -24,7 +24,6 @@ from scripts.ci.opencode_review_normalize_output import changed_file_is_material - PRIMARY_REVIEW_AUTHORS = { "opencode-agent[bot]", "opencode-agent", @@ -1636,7 +1635,10 @@ def call_llm( gateway_telemetry: dict[str, str | int] = {} if isinstance(exc, urllib.error.HTTPError): active_phase = "response_error" - gateway_telemetry = _extract_http_error_telemetry(exc) + try: + gateway_telemetry = _extract_http_error_telemetry(exc) + finally: + exc.close() model_value = gateway_telemetry.get("served_model") served_model = model_value if isinstance(model_value, str) else None elapsed = time.monotonic() - attempt_started diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index 33e58ed876..8a8aa524e5 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -304,6 +304,8 @@ def _github_open_json(url: str, token: str) -> object: with github_opener.open(request, timeout=30) as response: payload = response.read(MAX_RESPONSE_BYTES + 1) except (HTTPError, URLError, TimeoutError) as exc: + if isinstance(exc, HTTPError): + exc.close() raise PolicyError(f"GitHub API request failed for policy evidence: {type(exc).__name__}") from exc if len(payload) > MAX_RESPONSE_BYTES: raise PolicyError("GitHub API policy response exceeded the bounded response size") diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 04d26fac5a..633ab433f8 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -47,6 +47,7 @@ class SchedulerAdmissionGate: """Persist and bound review-worker leases for one scheduler execution.""" def __init__(self, state_path: Path, *, sequence: int, dispatch_budget: int) -> None: + """Bind this gate to one durable state file, run sequence, and worker budget.""" if sequence < 1: raise ValueError("admission sequence must be positive") if dispatch_budget < 0: @@ -68,6 +69,7 @@ def admit(self, component: str, repository: str, pr: dict[str, Any]) -> bool: selected: list[DispatchLease] = [] def lease(state): + """Apply this request to `state` and record any lease it wins.""" plan = plan_dispatches( state, [request], @@ -88,6 +90,7 @@ def reconcile(self, repository: str, prs: Sequence[dict[str, Any]]) -> None: live_prs = {int(pr["number"]): pr for pr in prs} def reconcile_state(state): + """Mark exact-head dispatched leases complete and superseded ones stale.""" records = dict(state.records) latest = dict(state.latest_sequences) for identity, record in tuple(records.items()): diff --git a/scripts/ci/reconcile_repository_metadata.py b/scripts/ci/reconcile_repository_metadata.py index 36a910ffa8..99d5ee9e95 100644 --- a/scripts/ci/reconcile_repository_metadata.py +++ b/scripts/ci/reconcile_repository_metadata.py @@ -16,10 +16,9 @@ import sys from pathlib import Path from typing import Any -from urllib.error import URLError +from urllib.error import HTTPError, URLError from urllib.request import HTTPRedirectHandler, Request, build_opener - ORGANIZATION = "ContextualWisdomLab" REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+$") TOPIC_RE = re.compile(r"^[a-z0-9][a-z0-9-]{0,49}$") @@ -247,6 +246,8 @@ def _pages_publication_ready(repository: str, current: dict[str, Any]) -> None: if not response.read(1): raise RuntimeError(f"GitHub Pages returned empty content for {repository}") except (URLError, TimeoutError, OSError) as exc: + if isinstance(exc, HTTPError): + exc.close() raise RuntimeError(f"GitHub Pages is not reachable for {repository}") from exc diff --git a/scripts/ci/review_admission_controller.py b/scripts/ci/review_admission_controller.py index dd26549a29..b8e7c208ab 100644 --- a/scripts/ci/review_admission_controller.py +++ b/scripts/ci/review_admission_controller.py @@ -20,12 +20,15 @@ @dataclass(frozen=True) class WorkerBoundary: + """The credential, permission set, and concurrency namespace one review worker runs under.""" + credential: str permissions: tuple[str, ...] concurrency_namespace: str cancel_in_progress: bool = True def concurrency_group(self, request: AdmissionRequest) -> str: + """Return this worker's `{namespace}-{repository}-{pull_request}` concurrency group.""" return ( f"{self.concurrency_namespace}-{request.repository}-{request.pull_request}" ) @@ -52,6 +55,8 @@ def concurrency_group(self, request: AdmissionRequest) -> str: @dataclass(frozen=True) class AdmissionRequest: + """One validated request to admit a review worker onto a specific PR head.""" + repository: str pull_request: int head_sha: str @@ -68,6 +73,7 @@ def create( component: str, sequence: int, ) -> AdmissionRequest: + """Validate and normalize raw fields into an `AdmissionRequest`.""" if isinstance(pull_request, bool) or not isinstance(pull_request, int): raise TypeError("pull request must be an integer") if isinstance(sequence, bool) or not isinstance(sequence, int): @@ -87,35 +93,45 @@ def create( @property def identity(self) -> str: + """Return the unique key identifying this exact request (including its sequence).""" return f"{self.repository}#{self.pull_request}@{self.head_sha}:{self.component}" @property def stream(self) -> str: + """Return the key identifying this request's PR+component stream across sequences.""" return f"{self.repository}#{self.pull_request}:{self.component}" @dataclass(frozen=True) class RequestRecord: + """An admission request paired with its current lifecycle status.""" + request: AdmissionRequest status: str @dataclass(frozen=True) class DispatchLease: + """A request that has been granted a worker boundary to run under.""" + request: AdmissionRequest boundary: WorkerBoundary @dataclass(frozen=True) class ControllerState: + """The durable admission controller's full state: known records and per-stream sequences.""" + records: dict[str, RequestRecord] latest_sequences: dict[str, int] @classmethod def empty(cls) -> ControllerState: + """Return the initial state with no records and no sequences observed yet.""" return cls({}, {}) def to_json(self) -> str: + """Serialize this state to its canonical, deterministically-ordered JSON form.""" payload = { "latest_sequences": self.latest_sequences, "records": { @@ -130,6 +146,7 @@ def to_json(self) -> str: @classmethod def from_json(cls, value: str) -> ControllerState: + """Parse and fully validate a state snapshot, rejecting any inconsistent JSON.""" payload = json.loads(value) if not isinstance(payload, dict): raise TypeError("durable admission state must be an object") @@ -204,6 +221,7 @@ def _open_regular_nofollow(path: Path, flags: int, mode: int = 0o600) -> int: def _read_state(path: Path) -> ControllerState: + """Read and parse one state file, rejecting a symlink and non-UTF-8 content.""" descriptor = _open_regular_nofollow(path, os.O_RDONLY) try: with os.fdopen(descriptor, encoding="utf-8") as stream: @@ -282,6 +300,8 @@ def update_state_file( @dataclass(frozen=True) class DispatchPlan: + """The result of one admission pass: the updated state, grants, and rejections.""" + state: ControllerState dispatches: tuple[DispatchLease, ...] rejections: dict[str, str] diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index b0376c0822..611361f00a 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -7,9 +7,9 @@ import json import os import platform -import signal -import shutil import shlex +import shutil +import signal import socket import subprocess import sys @@ -27,7 +27,6 @@ from scripts.ci import sandboxed_verify - RESULT_MARKER = "SANDBOXED_WEB_E2E_RESULT" SANDBOX_MOUNT = "/workspace" @@ -584,7 +583,9 @@ def wait_for_url(url: str, timeout: int, service: Service) -> bool: if 200 <= response.status < 500: return True time.sleep(1) - except (urllib.error.URLError, TimeoutError): + except (urllib.error.URLError, TimeoutError) as exc: + if isinstance(exc, urllib.error.HTTPError): + exc.close() time.sleep(1) return False diff --git a/tests/test_codeql_default_setup_rollout.py b/tests/test_codeql_default_setup_rollout.py index 665eed5aa3..8610a39f49 100644 --- a/tests/test_codeql_default_setup_rollout.py +++ b/tests/test_codeql_default_setup_rollout.py @@ -1,7 +1,12 @@ import base64 +import builtins import json +import runpy +import sys from io import StringIO +import pytest + from scripts.ci import audit_codeql_default_setup_rollout as rollout HEAD = "a" * 40 @@ -264,3 +269,192 @@ def request(self, path): assert "head changed" in str(exc) else: raise AssertionError("moving exact-head evidence must fail closed") + + +def test_pagination_rejects_malformed_and_unbounded_evidence(): + with pytest.raises(rollout.EvidenceError, match="malformed pagination"): + rollout._pages(FakeClient({"/items?per_page=100&page=1": {}}), "/items") + + pages = { + f"/items?per_page=100&page={page}": [{}] * 100 + for page in range(1, rollout.MAX_PAGES + 1) + } + with pytest.raises(rollout.EvidenceError, match="pagination exceeded"): + rollout._pages(FakeClient(pages), "/items") + + +@pytest.mark.parametrize( + ("source", "active"), + ( + ( + "steps:\n - name: disabled\n if: ${{ false }}\n" + " uses: github/codeql-action/analyze@pin\n", + False, + ), + ( + "steps:\n - name: disabled\n uses: github/codeql-action/analyze@pin\n" + " with:\n upload: 'never'\n - name: next\n run: true\n", + False, + ), + ( + "steps:\n - name: active\n uses: github/codeql-action/upload-sarif@pin\n", + True, + ), + ), +) +def test_advanced_uploader_detection_honors_only_local_disabling(source, active): + assert rollout._has_active_advanced_upload(source) is active + + +def test_live_snapshot_rejects_ambiguous_or_invalid_workflow_sources(): + repository = "ContextualWisdomLab/xtrmLLMBatchPython" + workflow_path = f"/repos/{repository}/actions/workflows?per_page=100&page=1" + source_path = f"/repos/{repository}/contents/.github/workflows/ci.yml?ref={HEAD}" + + cases = [] + duplicate = live_responses() + duplicate[workflow_path]["workflows"] *= 2 + cases.append((duplicate, "identity is ambiguous")) + + lookup_failure = live_responses() + lookup_failure[source_path] = rollout.GitHubError("HTTP 500") + cases.append((lookup_failure, "source lookup failed")) + + invalid_size = live_responses() + invalid_size[source_path]["size"] = -1 + cases.append((invalid_size, "invalid size")) + + invalid_base64 = live_responses() + invalid_base64[source_path]["content"] = "!" + cases.append((invalid_base64, "source is invalid")) + + size_mismatch = live_responses() + size_mismatch[source_path]["size"] += 1 + cases.append((size_mismatch, "size mismatch")) + + for responses, message in cases: + with pytest.raises(rollout.EvidenceError, match=message): + rollout.collect_live_snapshot(FakeClient(responses), repository, 292) + + +@pytest.mark.parametrize( + ("repository", "pr_number", "message"), + ( + ("Other/example", 1, "must belong"), + ("ContextualWisdomLab/example", 0, "must be positive"), + ), +) +def test_live_snapshot_rejects_invalid_identity(repository, pr_number, message): + with pytest.raises(rollout.EvidenceError, match=message): + rollout.collect_live_snapshot(FakeClient({}), repository, pr_number) + + +def test_live_snapshot_rejects_ambiguous_ruleset_owner_and_missing_states(): + repository = "ContextualWisdomLab/xtrmLLMBatchPython" + pull_path = f"/repos/{repository}/pulls/292" + rulesets_path = f"/repos/{repository}/rulesets?includes_parents=true&per_page=100&page=1" + detail_path = f"/repos/{repository}/rulesets/{rollout.RULESET_ID}?includes_parents=true" + setup_path = f"/repos/{repository}/code-scanning/default-setup" + runs_path = f"/repos/{repository}/actions/runs?head_sha={HEAD}&per_page=100&page=1" + + closed = live_responses() + closed[pull_path] = {"state": "closed", "head": {"sha": HEAD}} + ambiguous_ruleset = live_responses() + ambiguous_ruleset[rulesets_path] *= 2 + ambiguous_owner = live_responses() + ambiguous_owner[detail_path]["rules"][0]["parameters"]["workflows"] *= 2 + missing_setup = live_responses() + missing_setup[setup_path] = {"state": "new-state"} + missing_status = live_responses() + missing_status[runs_path]["workflow_runs"][0].update(status=None, conclusion=None) + + for responses, message in ( + (closed, "not open"), + (ambiguous_ruleset, "ruleset evidence is ambiguous"), + (ambiguous_owner, "ruleset owner is ambiguous"), + (missing_setup, "default-setup state is unavailable"), + (missing_status, "has no status"), + ): + with pytest.raises(rollout.EvidenceError, match=message): + rollout.collect_live_snapshot(FakeClient(responses), repository, 292) + + +def test_exempt_snapshot_revalidates_head_and_classification_edges(): + repository = "ContextualWisdomLab/noema" + pull_path = f"/repos/{repository}/pulls/7" + rulesets_path = f"/repos/{repository}/rulesets?includes_parents=true&per_page=100&page=1" + client = FakeClient( + { + pull_path: {"state": "open", "head": {"sha": HEAD}}, + rulesets_path: [], + } + ) + assert rollout.collect_live_snapshot(client, repository, 7) == { + "name": "noema", + "ruleset_applies": False, + } + assert rollout.classify(snapshot(default_setup_state="unsupported"))[0] == "BLOCK" + assert rollout.classify(snapshot(central_codeql_status="failure"))[0] == "ROLLBACK" + + class MovingExemptClient(FakeClient): + reads = 0 + + def request(self, path): + if path == pull_path: + self.reads += 1 + if self.reads == 2: + return {"state": "open", "head": {"sha": "b" * 40}} + return super().request(path) + + with pytest.raises(rollout.EvidenceError, match="head changed"): + rollout.collect_live_snapshot( + MovingExemptClient(client.responses), repository, 7 + ) + + +def test_payload_file_and_cli_error_paths(tmp_path, monkeypatch, capsys): + payload_path = tmp_path / "snapshots.json" + payload_path.write_text(json.dumps([snapshot()]), encoding="utf-8") + assert rollout.load_payload(payload_path, StringIO()) == [snapshot()] + with pytest.raises(ValueError, match="array of objects"): + rollout.load_payload(None, StringIO("{}")) + + assert rollout.main([str(payload_path), "--repository", "ContextualWisdomLab/x", "--pr", "1"]) == 2 + monkeypatch.setattr(rollout.sys, "stdin", StringIO("{")) + assert rollout.main([]) == 2 + assert "unable to load CodeQL rollout snapshots" in capsys.readouterr().err + + +def test_live_cli_collects_one_snapshot(monkeypatch, capsys): + fake_client = object() + calls = [] + monkeypatch.setattr( + rollout.GitHubClient, + "from_environment", + classmethod(lambda cls: fake_client), + ) + def collect_snapshot(client, repository, pr): + calls.append((client, repository, pr)) + return snapshot() + + monkeypatch.setattr(rollout, "collect_live_snapshot", collect_snapshot) + assert rollout.main( + ["--repository", "ContextualWisdomLab/example", "--pr", "7"] + ) == 0 + assert calls == [(fake_client, "ContextualWisdomLab/example", 7)] + assert "state=VERIFIED" in capsys.readouterr().out + + +def test_direct_script_import_falls_back_to_sibling_module(monkeypatch): + script_path = rollout.Path(rollout.__file__) + real_import = builtins.__import__ + + def import_with_package_missing(name, *args, **kwargs): + if name == "scripts.ci.organization_commercial_readiness_loop": + raise ModuleNotFoundError(name) + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", import_with_package_missing) + monkeypatch.setattr(sys, "path", [str(script_path.parent), *sys.path]) + namespace = runpy.run_path(str(script_path), run_name="rollout_direct_import_test") + assert namespace["GitHubClient"] is not None diff --git a/tests/test_docs_only_pr_runner_admission.py b/tests/test_docs_only_pr_runner_admission.py index 8b2e6e8ee2..3c155b4a15 100644 --- a/tests/test_docs_only_pr_runner_admission.py +++ b/tests/test_docs_only_pr_runner_admission.py @@ -85,19 +85,69 @@ def _on_block(workflow: str) -> str: return match.group(1) +def _strip_if_condition(block: str) -> str: + """Drop the `if:` line and, for a folded/literal scalar, its continuation lines. + + A workflow's `if:` condition can span multiple lines (``if: >-`` or ``if: |`` + followed by more-indented continuation lines) rather than a single line. + Comparing gate copies must ignore the whole condition, not just its first + line, since each copy is allowed its own admission condition independent + of how many source lines that condition takes. + """ + kept: list[str] = [] + skip_indent: int | None = None + for line in block.splitlines(): + if skip_indent is not None: + indent = len(line) - len(line.lstrip(" ")) + if not line.strip() or indent > skip_indent: + continue + skip_indent = None + if line.startswith(" if:"): + skip_indent = 4 + continue + kept.append(line) + return "\n".join(kept) + + def test_gate_job_is_byte_identical_across_the_five_workflows_apart_from_if(): """The `changed-scope` block must not drift between its five copies.""" normalized_blocks = set() for filename in GATE_WORKFLOWS: workflow = _read(filename) block = _top_level_job_block(workflow, "changed-scope") - normalized = "\n".join( - line for line in block.splitlines() if not line.strip().startswith("if:") - ) - normalized_blocks.add(normalized) + normalized_blocks.add(_strip_if_condition(block)) assert len(normalized_blocks) == 1, ( "changed-scope gate copies drifted; keep them byte-identical apart " - "from the single 'if:' line" + "from the 'if:' condition (which may itself span multiple lines, e.g. " + "a YAML block scalar)" + ) + + +def test_strip_if_condition_keeps_skipping_across_a_blank_continuation_line(): + """A blank line inside a folded/literal `if:` scalar must not end the skip. + + YAML's `if: >-`/`if: |` block scalars can carry a blank line as part of + the same condition; a blank line is not itself an "if:"-less, less- + indented line that should end the skip, and one falsely resetting + `skip_indent` would leave that scalar's later indented lines in the + normalized output, making an otherwise byte-identical body compare as + drifted. + """ + block = ( + " if: >-\n" + " first line ||\n" + "\n" + " second line after a blank\n" + " steps:\n" + " - name: example\n" + " if: success()\n" + " runs-on: ubuntu-24.04\n" + ) + assert _strip_if_condition(block) == ( + " steps:\n" + " - name: example\n" + " if: success()\n" + " runs-on: ubuntu-24.04" ) diff --git a/tests/test_noema_orchestrator_workflow_contract.py b/tests/test_noema_orchestrator_workflow_contract.py index 937cf6fe97..c43429e8bf 100644 --- a/tests/test_noema_orchestrator_workflow_contract.py +++ b/tests/test_noema_orchestrator_workflow_contract.py @@ -109,7 +109,10 @@ def test_noema_close_cleanup_selects_only_the_closed_pr_across_shared_display_ti fake_gh.write_text( """#!/usr/bin/env bash set -euo pipefail -if [[ "$*" == *"--paginate"* ]]; then +if [[ "$*" == *"/pulls/7"* ]]; then + printf '%s\n' "$*" >>"$FAKE_CALLS_FILE" + printf '%s\n' '{"state":"closed","draft":false,"head":{"sha":"'"$INACTIVE_PR_HEAD_SHA"'"}}' +elif [[ "$*" == *"--paginate"* ]]; then [[ "$*" != *"/actions/workflows/"* ]] || exit 99 printf '%s\n' "$*" >>"$FAKE_CALLS_FILE" url="$3" diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 6422ae5012..f7a2f3e452 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -1702,12 +1702,38 @@ def open(self, request): assert '{"error":' not in output +@pytest.mark.parametrize( + "attempts", + ( + [], + [None], + [ + { + "provider_name": "unsafe provider", + "phase": "unsafe phase", + "attempt_number": True, + "provider_status": 99, + } + ], + ), +) +def test_http_error_telemetry_omits_invalid_attempt_fields(attempts) -> None: + body = json.dumps({"error": {"detail": {"attempts": attempts}}}).encode() + error = noema.urllib.error.HTTPError( + "https://llm.example.test/chat", 502, "Bad Gateway", {}, io.BytesIO(body) + ) + try: + assert noema._extract_http_error_telemetry(error) == {} + finally: + error.close() + + def test_noema_redirect_handler_rejects_redirects(): """Noema must not follow redirects after validating the initial URL.""" handler = noema.NoRedirectHandler() request = noema.urllib.request.Request("https://llm.example.test/chat") - with pytest.raises(noema.urllib.error.HTTPError): + with pytest.raises(noema.urllib.error.HTTPError) as caught: handler.redirect_request( request, fp=None, @@ -1716,6 +1742,7 @@ def test_noema_redirect_handler_rejects_redirects(): headers={}, newurl="http://169.254.169.254/latest/meta-data/", ) + caught.value.close() def test_call_llm_rejects_control_character_scheme_evasion(monkeypatch): diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index c5d4e9d7a3..325c96dfaf 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -826,8 +826,9 @@ def test_github_open_json_rejects_nonapproved_origins(url: str) -> None: def test_github_opener_never_constructs_redirect_requests() -> None: """The policy opener refuses redirects rather than changing API origins.""" - with pytest.raises(HTTPError): + with pytest.raises(HTTPError) as caught: policy.NoRedirectHandler().redirect_request(policy.Request("https://example.com"), None, 302, "Found", {}, "https://evil.example") + caught.value.close() def test_annotation_escapes_workflow_command_fields() -> None: diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 8d4397c42d..54c1772886 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -30,6 +30,7 @@ def test_review_fix_caller_keeps_the_github_daily_recovery_slot() -> None: caller = _workflow_text(HOURLY_CALLER_WORKFLOW) assert 'cron: "23 7 * * *"' in caller assert 'cron: "23 * * * *"' not in caller + assert 'cron: "23 */2 * * *"' not in caller assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in caller diff --git a/tests/test_pr_review_fix_scheduler.py b/tests/test_pr_review_fix_scheduler.py index 667ced6148..2d4a2e7aa0 100644 --- a/tests/test_pr_review_fix_scheduler.py +++ b/tests/test_pr_review_fix_scheduler.py @@ -1467,6 +1467,8 @@ def test_fix_parse_args_and_self_test(monkeypatch): ["--repo", "owner/repo"], ["--repo", "owner/repo", "--base-branch", "main", "--pr-number", "-1"], ["--repo", "owner/repo", "--base-branch", "main", "--max-prs", "0"], + ["--repo", "owner/repo", "--base-branch", "main", "--scan-window-size", "51"], + ["--repo", "owner/repo", "--base-branch", "main", "--rotation-seed", "-1"], ["--repo", "owner/repo", "--base-branch", "main", "--max-dispatches", "0"], ["--repo", "owner/repo", "--base-branch", "main", "--retry-hours", "0"], ["--repo", "owner/repo", "--base-branch", "main", "--autofix-repository", "bad"], diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 71854724ce..baf9e2238a 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -8,7 +8,6 @@ from scripts.ci import pr_review_merge_scheduler as sched - TOKEN_SEPARATOR = "_" GITHUB_TOKEN_PREFIXES = { "classic": "g" + "hp", @@ -2350,6 +2349,15 @@ def test_central_coverage_retry_ignores_failed_required_workflow_placeholder( "same-head OpenCode re-dispatched" ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: "admission_deferred", + ) + deferred = inspect(coverage_request) + assert deferred.action == "wait" + assert deferred.reason == "bounded admission budget is exhausted" + def test_coverage_retry_disables_auto_merge_before_dispatch(monkeypatch): """A coverage retry must not leave an unsafe auto-merge request enabled.""" @@ -10228,3 +10236,282 @@ def test_reconcile_releases_strix_lease_when_no_run_was_created(tmp_path): record = next(iter(load_state_file(gate.state_path).records.values())) assert record.status == "stale" + + +def test_admission_gate_and_rotating_window_reject_invalid_bounds(tmp_path): + with pytest.raises(ValueError, match="sequence must be positive"): + sched.SchedulerAdmissionGate(tmp_path / "state.json", sequence=0, dispatch_budget=1) + with pytest.raises(ValueError, match="budget must not be negative"): + sched.SchedulerAdmissionGate(tmp_path / "state.json", sequence=1, dispatch_budget=-1) + with pytest.raises(ValueError, match="offset must be non-negative"): + sched.rotating_pr_window([{"number": 1}], offset=-1, window_size=1) + assert sched.rotating_pr_window([], offset=10, window_size=1) == [] + + +def test_admission_gate_reconcile_stales_moved_head(tmp_path): + gate = sched.SchedulerAdmissionGate( + tmp_path / "admission.json", sequence=91, dispatch_budget=1 + ) + old = make_pr(number=7, headRefOid="a" * 40) + assert gate.admit("opencode", "ContextualWisdomLab/example", old) + gate.reconcile( + "ContextualWisdomLab/example", + [make_pr(number=7, headRefOid="b" * 40)], + ) + + from scripts.ci.review_admission_controller import load_state_file + + record = next(iter(load_state_file(gate.state_path).records.values())) + assert record.status == "stale" + + +def test_admission_reconcile_scans_records_after_live_lease(monkeypatch, tmp_path): + gate = sched.SchedulerAdmissionGate( + tmp_path / "admission.json", sequence=92, dispatch_budget=2 + ) + pr = make_pr(number=7, headRefOid="a" * 40) + assert gate.admit("opencode", "ContextualWisdomLab/example", pr) + assert gate.admit("strix", "ContextualWisdomLab/example", pr) + monkeypatch.setattr( + sched, "opencode_progress_state", lambda *_args, **_kwargs: "running" + ) + gate.reconcile("ContextualWisdomLab/example", [pr]) + + from scripts.ci.review_admission_controller import load_state_file + + statuses = sorted( + record.status for record in load_state_file(gate.state_path).records.values() + ) + assert statuses == ["dispatched", "stale"] + + +def test_admission_deferred_decisions_remain_wait_states(monkeypatch): + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_args: None) + monkeypatch.setattr( + sched, "dispatch_strix_evidence", lambda *_args, **_kwargs: "admission_deferred" + ) + draft = inspect(make_pr(isDraft=True), allow_draft_review_dispatch=True) + assert draft.action == "wait" and "admission budget" in draft.reason + + monkeypatch.setattr( + sched, "dispatch_opencode_review", lambda *_args, **_kwargs: "admission_deferred" + ) + strix_complete = make_pr( + isDraft=True, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ) + draft_review = inspect(strix_complete, allow_draft_review_dispatch=True) + assert draft_review.action == "wait" and "admission budget" in draft_review.reason + + monkeypatch.setattr(sched, "opencode_progress_state", lambda *_args, **_kwargs: "absent") + stacked = inspect(make_pr(baseRefName="feature-base")) + assert stacked.action == "wait" and "admission budget" in stacked.reason + + +def test_empty_pr_close_continues_when_comment_fails(monkeypatch): + head_sha = "a" * 40 + candidate = make_pr(headRefOid=head_sha, files={"totalCount": 0, "nodes": []}) + monkeypatch.setattr( + sched, + "_fresh_open_pr_for_cancellation", + lambda *_args: {"draft": False, "changed_files": 0, "head": {"sha": head_sha}}, + ) + calls = [] + + def run(args): + calls.append(args) + if "comment" in args: + raise RuntimeError("comment unavailable") + return "" + + monkeypatch.setattr(sched, "run", run) + assert inspect(candidate, dry_run=False).action == "close_empty" + assert calls[-1][2] == "close" + calls.clear() + assert inspect(candidate, dry_run=True).action == "close_empty" + assert calls == [] + + +@pytest.mark.parametrize( + ("flag", "value", "message"), + ( + ("--admission-dispatch-budget", "-1", "must not be negative"), + ("--admission-sequence", "0", "must be positive"), + ), +) +def test_main_rejects_invalid_admission_bounds(flag, value, message): + with pytest.raises(SystemExit, match=message): + sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + flag, + value, + ] + ) + + +def test_post_update_followup_reports_admission_deferral(monkeypatch): + original = make_pr(headRefOid="a" * 40) + updated = make_pr(headRefOid="b" * 40) + monkeypatch.setattr(sched, "wait_for_updated_branch_head", lambda *_args: updated) + monkeypatch.setattr(sched, "dismiss_stale_opencode_approvals", lambda *_args, **_kwargs: (0, 0)) + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_args: None) + monkeypatch.setattr(sched, "strix_evidence_state", lambda _pr: "missing") + monkeypatch.setattr( + sched, "dispatch_strix_evidence", lambda *_args, **_kwargs: "admission_deferred" + ) + assert "admission budget" in sched.post_update_branch_followup( + "owner/repo", + original, + dry_run=False, + trigger_reviews=True, + review_dispatch_allowed=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + stale_opencode_minutes=30, + ) + + monkeypatch.setattr(sched, "strix_evidence_state", lambda _pr: "complete") + monkeypatch.setattr(sched, "opencode_progress_state", lambda *_args, **_kwargs: "absent") + monkeypatch.setattr( + sched, "dispatch_opencode_review", lambda *_args, **_kwargs: "admission_deferred" + ) + assert "admission budget" in sched.post_update_branch_followup( + "owner/repo", + original, + dry_run=False, + trigger_reviews=True, + review_dispatch_allowed=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + stale_opencode_minutes=30, + ) + + +def test_startup_failure_recovery_ignores_unusable_runs(monkeypatch): + head_sha = "a" * 40 + runs = [ + {"event": "schedule", "workflow_id": 1}, + {"event": "pull_request"}, + { + "event": "pull_request", + "workflow_id": 1, + "id": 1, + "created_at": "2026-01-01T00:00:00Z", + "head_sha": head_sha, + "status": "completed", + "conclusion": "success", + }, + { + "event": "pull_request", + "workflow_id": 1, + "id": 2, + "created_at": "2026-01-02T00:00:00Z", + "head_sha": head_sha, + "status": "completed", + "conclusion": "success", + }, + { + "event": "pull_request", + "workflow_id": 1, + "id": 1, + "created_at": "2026-01-01T00:00:00Z", + "head_sha": head_sha, + "status": "completed", + "conclusion": "success", + }, + {"event": "schedule", "workflow_id": 2}, + ] + monkeypatch.setattr( + sched, + "run_github_read", + lambda _args: json.dumps({"workflow_runs": runs}), + ) + assert sched.recover_current_head_startup_failures( + "owner/repo", make_pr(headRefOid=head_sha), dry_run=True + ) == [] + + +def test_main_materializes_admission_state(monkeypatch, tmp_path): + monkeypatch.setattr(sched, "fetch_open_prs", lambda *_args, **_kwargs: []) + assert sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + "--admission-state-path", + str(tmp_path / "admission.json"), + "--dry-run", + ] + ) == 0 + + +def test_strix_dispatch_checks_admission_and_live_head_before_rerun(monkeypatch): + pr = make_pr(headRefOid="a" * 40) + monkeypatch.setattr(sched, "matching_actions_job_id", lambda *_args: "7") + monkeypatch.setattr(sched, "review_dispatch_admitted", lambda *_args: False) + assert sched.dispatch_strix_evidence( + "owner/repo", "Strix Security Scan", pr, dry_run=False + ) == "admission_deferred" + + monkeypatch.setattr(sched, "review_dispatch_admitted", lambda *_args: True) + monkeypatch.setattr(sched, "live_dispatch_head_matches", lambda *_args: False) + assert sched.dispatch_strix_evidence( + "owner/repo", "Strix Security Scan", pr, dry_run=False + ) == "stale_head" + + +def test_strix_dispatch_checks_admission_and_live_head_before_new_run(monkeypatch): + pr = make_pr(headRefOid="a" * 40, baseRefOid="b" * 40) + monkeypatch.setattr(sched, "matching_actions_job_id", lambda *_args: None) + monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda *_args: None) + monkeypatch.setattr(sched, "active_review_run_refs", lambda *_args, **_kwargs: ([], [])) + monkeypatch.setattr( + sched, "_cancel_revalidated_review_run_refs", lambda *_args: ([], []) + ) + monkeypatch.setattr(sched, "repository_dispatch_target", lambda repo: repo) + monkeypatch.setattr(sched, "active_workflow_runs", lambda _repo: []) + monkeypatch.setattr(sched, "review_dispatch_admitted", lambda *_args: False) + assert sched.dispatch_strix_evidence( + "owner/repo", "Strix Security Scan", pr, dry_run=False + ) == "admission_deferred" + + monkeypatch.setattr(sched, "review_dispatch_admitted", lambda *_args: True) + monkeypatch.setattr(sched, "live_dispatch_head_matches", lambda *_args: False) + assert sched.dispatch_strix_evidence( + "owner/repo", "Strix Security Scan", pr, dry_run=False + ) == "stale_head" + + +@pytest.mark.parametrize("strix_state", ("missing", "complete")) +def test_ready_pr_admission_deferral_is_a_wait_state(monkeypatch, strix_state): + monkeypatch.setattr(sched, "opencode_progress_state", lambda *_args, **_kwargs: "absent") + monkeypatch.setattr(sched, "strix_evidence_state", lambda _pr: strix_state) + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda *_args: None) + monkeypatch.setattr( + sched, "dispatch_strix_evidence", lambda *_args, **_kwargs: "admission_deferred" + ) + monkeypatch.setattr( + sched, "dispatch_opencode_review", lambda *_args, **_kwargs: "admission_deferred" + ) + decision = inspect(make_pr()) + assert decision.action == "wait" + assert decision.reason == "bounded admission budget is exhausted" + + +def test_stale_opencode_admission_deferral_is_a_wait_state(monkeypatch): + monkeypatch.setattr(sched, "opencode_progress_state", lambda *_args, **_kwargs: "stale") + monkeypatch.setattr( + sched, "dispatch_opencode_review", lambda *_args, **_kwargs: "admission_deferred" + ) + decision = inspect(make_pr()) + assert decision.action == "wait" + assert decision.reason == "bounded admission budget is exhausted" diff --git a/tests/test_repository_branch_coverage_review_schedulers.py b/tests/test_repository_branch_coverage_review_schedulers.py index 04defb2d3f..5e93c3f8e4 100644 --- a/tests/test_repository_branch_coverage_review_schedulers.py +++ b/tests/test_repository_branch_coverage_review_schedulers.py @@ -3,7 +3,10 @@ from __future__ import annotations import argparse +import builtins import json +import runpy +from pathlib import Path from typing import Any import pytest @@ -17,6 +20,36 @@ from scripts.ci import pr_review_merge_scheduler as merge_scheduler +def test_merge_scheduler_core_direct_import_fallback(monkeypatch) -> None: + real_import = builtins.__import__ + + def import_without_package(name, *args, **kwargs): + if name == "scripts.ci.review_admission_controller": + raise ModuleNotFoundError(name) + return real_import(name, *args, **kwargs) + + monkeypatch.syspath_prepend(str(Path("scripts/ci").resolve())) + monkeypatch.setattr(builtins, "__import__", import_without_package) + namespace = runpy.run_path( + "scripts/ci/pr_review_merge_scheduler_core.py", + run_name="pr_review_merge_scheduler_core_direct_import_test", + ) + assert namespace["SchedulerAdmissionGate"].__name__ == "SchedulerAdmissionGate" + + +def test_merge_scheduler_self_test_direct_import_fallback(monkeypatch) -> None: + real_import = builtins.__import__ + + def import_without_package(name, *args, **kwargs): + if name == "scripts.ci.review_admission_controller": + raise ModuleNotFoundError(name) + return real_import(name, *args, **kwargs) + + monkeypatch.syspath_prepend(str(Path("scripts/ci").resolve())) + monkeypatch.setattr(builtins, "__import__", import_without_package) + merge_scheduler.self_test() + + def test_noema_public_dns_result_reaches_valid_model_response( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -208,3 +241,37 @@ def check(started: str, conclusion: str) -> dict[str, Any]: } } assert merge_scheduler.failed_status_checks(pr) == [] + + +def test_merge_scheduler_summary_loops_cover_optional_details() -> None: + conflicts = merge_scheduler.conflict_repair_summary( + [ + merge_scheduler.Decision( + 1, "wait", "merge conflict: DIRTY; base=main, head=feature-a" + ), + merge_scheduler.Decision( + 2, + "wait", + "merge conflict: DIRTY; base=main, head=feature-b; changed files to inspect first: app.py", + ), + ] + ) + assert "- `app.py`" in conflicts + + restamps = merge_scheduler.last_push_approval_restamp_summary( + [ + merge_scheduler.Decision( + 1, + "restamp_head", + "last-push approval head refresh requested", + ("unrelated note",), + ), + merge_scheduler.Decision( + 2, + "restamp_head", + "last-push approval head refresh requested", + ("last-push approval head refresh created same-tree head abc",), + ), + ] + ) + assert any("same-tree head abc" in line for line in restamps) diff --git a/tests/test_repository_metadata_live_verification.py b/tests/test_repository_metadata_live_verification.py index 6ae83d8c47..e363156663 100644 --- a/tests/test_repository_metadata_live_verification.py +++ b/tests/test_repository_metadata_live_verification.py @@ -5,11 +5,11 @@ import argparse import importlib.util import json +from io import BytesIO from pathlib import Path import pytest - ROOT = Path(__file__).resolve().parents[1] SCRIPT = ROOT / "scripts" / "ci" / "reconcile_repository_metadata.py" SPEC = importlib.util.spec_from_file_location("reconcile_repository_metadata", SCRIPT) @@ -63,6 +63,25 @@ def open(self, request, timeout): return self.response +def test_pages_transport_error_closes_response_body(monkeypatch) -> None: + body = BytesIO(b"redirect") + error = RECONCILER.HTTPError( + "https://contextualwisdomlab.github.io/Repo/", 302, "redirect", {}, body + ) + monkeypatch.setattr( + RECONCILER, "build_opener", lambda *_args: FakeOpener(error=error) + ) + with pytest.raises(RuntimeError, match="not reachable"): + RECONCILER._pages_publication_ready( + "Repo", + { + "status": "built", + "html_url": "https://contextualwisdomlab.github.io/Repo/", + }, + ) + assert body.closed + + def install_live_state( monkeypatch, *, @@ -145,10 +164,11 @@ def build_ok(handler): assert len(handlers) == 1 assert isinstance(handlers[0], RECONCILER._NoPagesRedirects) from urllib.error import HTTPError - with pytest.raises(HTTPError): + with pytest.raises(HTTPError) as caught: handlers[0].redirect_request( RECONCILER.Request("https://example.com"), None, 302, "redirect", {}, "http://127.0.0.1/" ) + caught.value.close() with pytest.raises(RuntimeError, match="not built"): RECONCILER._pages_publication_ready("Repo", {**ready, "status": "building"}) diff --git a/tests/test_review_admission_controller.py b/tests/test_review_admission_controller.py index fcb2885144..ce83f13918 100644 --- a/tests/test_review_admission_controller.py +++ b/tests/test_review_admission_controller.py @@ -1,9 +1,11 @@ import json +import os import threading from concurrent.futures import ThreadPoolExecutor import pytest +from scripts.ci import review_admission_controller as controller from scripts.ci.review_admission_controller import ( ADMISSION_PERMISSIONS, WORKER_BOUNDARIES, @@ -249,3 +251,190 @@ def test_budget_counts_active_leases_and_stale_heads_cannot_poison_sequence() -> dispatch_budget=1, ) assert retried.dispatches[0].request.sequence == 2 + + +@pytest.mark.parametrize( + ("changes", "error", "message"), + ( + ({"sequence": True}, TypeError, "sequence must be an integer"), + ({"pull_request": 0}, ValueError, "pull request must be positive"), + ({"head_sha": "short"}, ValueError, "head must be a full Git SHA"), + ({"sequence": 0}, ValueError, "sequence must be positive"), + ), +) +def test_request_rejects_each_invalid_scalar(changes, error, message) -> None: + values = { + "repository": "ContextualWisdomLab/example", + "pull_request": 7, + "head_sha": HEAD_1, + "component": "opencode", + "sequence": 1, + } + values.update(changes) + with pytest.raises(error, match=message): + AdmissionRequest.create(**values) + + +@pytest.mark.parametrize( + ("payload", "error", "message"), + ( + ([], TypeError, "must be an object"), + ({"records": []}, TypeError, "invalid collections"), + ( + {"records": {"bad": {"request": {}, "extra": 1}}, "latest_sequences": {}}, + ValueError, + "invalid durable admission record", + ), + ( + { + "records": { + "bad": { + "request": { + "repository": "ContextualWisdomLab/example", + "pull_request": 7, + }, + "status": "queued", + } + }, + "latest_sequences": {}, + }, + ValueError, + "invalid durable admission request", + ), + ), +) +def test_state_json_rejects_malformed_top_level_shapes(payload, error, message) -> None: + with pytest.raises(error, match=message): + ControllerState.from_json(json.dumps(payload)) + + +def test_state_json_rejects_non_string_record_identity(monkeypatch) -> None: + monkeypatch.setattr( + controller.json, + "loads", + lambda _serialized: {"records": {1: {}}, "latest_sequences": {}}, + ) + with pytest.raises(TypeError, match="invalid shape"): + ControllerState.from_json("ignored") + + +def test_state_json_rejects_identity_status_sequence_and_regression() -> None: + item = request("opencode", HEAD_1, 1) + + def encoded(identity=item.identity, status="queued", latest=None, record=item): + return json.dumps( + { + "records": { + identity: { + "request": { + "repository": record.repository, + "pull_request": record.pull_request, + "head_sha": record.head_sha, + "component": record.component, + "sequence": record.sequence, + }, + "status": status, + } + }, + "latest_sequences": latest + if latest is not None + else {item.stream: 1}, + } + ) + + with pytest.raises(ValueError, match="invalid durable admission record"): + ControllerState.from_json(encoded(identity="wrong")) + with pytest.raises(ValueError, match="invalid durable admission record"): + ControllerState.from_json(encoded(status="unknown")) + with pytest.raises(ValueError, match="invalid durable admission sequence"): + ControllerState.from_json(encoded(latest={item.stream: True})) + regressed = request("opencode", HEAD_1, 2) + with pytest.raises(ValueError, match="sequence regressed"): + ControllerState.from_json( + encoded( + identity=regressed.identity, + latest={regressed.stream: 1}, + record=regressed, + ) + ) + with pytest.raises(ValueError, match="sequence is inconsistent"): + ControllerState.from_json(encoded(latest={item.stream: 2})) + + +def test_state_file_rejects_corruption_symlinks_and_nonregular_paths(tmp_path) -> None: + corrupt = tmp_path / "corrupt.json" + corrupt.write_text("{", encoding="utf-8") + with pytest.raises(ValueError, match="corrupt and has no backup"): + load_state_file(corrupt) + + invalid_utf8 = tmp_path / "invalid.json" + invalid_utf8.write_bytes(b"\xff") + with pytest.raises(ValueError, match="not UTF-8"): + controller._read_state(invalid_utf8) + + with pytest.raises(ValueError, match="not a regular file"): + controller._open_regular_nofollow(tmp_path, os.O_RDONLY) + + state_path = tmp_path / "state.json" + backup = tmp_path / "state.json.bak" + backup.symlink_to(corrupt) + with pytest.raises(ValueError, match="backup must not be a symlink"): + load_state_file(state_path) + + atomic_link = tmp_path / "atomic.json" + atomic_link.symlink_to(corrupt) + with pytest.raises(ValueError, match="state path must not be a symlink"): + controller._atomic_write(atomic_link, "{}") + + lock_link = tmp_path / "locked.json.lock" + lock_link.symlink_to(corrupt) + with pytest.raises(ValueError, match="lock must not be a symlink"): + update_state_file(tmp_path / "locked.json", lambda state: state) + + +def test_update_and_dispatch_reject_invalid_transitions(tmp_path) -> None: + with pytest.raises(TypeError, match="must return ControllerState"): + update_state_file(tmp_path / "state.json", lambda state: object()) + with pytest.raises(ValueError, match="budget must not be negative"): + plan_dispatches(ControllerState.empty(), [], live_heads={}, dispatch_budget=-1) + + item = request("opencode", HEAD_2, 2) + lease = DispatchLease(item, WORKER_BOUNDARIES["opencode"]) + with pytest.raises(ValueError, match="active dispatch lease"): + complete_dispatch(ControllerState.empty(), lease, live_head=HEAD_2) + + +def test_new_head_stales_queued_predecessor_and_dispatch_rechecks_live_head() -> None: + old = request("opencode", HEAD_1, 1) + current = request("opencode", HEAD_2, 2) + state = ControllerState( + {old.identity: RequestRecord(old, "queued")}, + {old.stream: 1}, + ) + plan = plan_dispatches( + state, + [current], + live_heads={(current.repository, current.pull_request): HEAD_2}, + dispatch_budget=1, + ) + assert plan.state.records[old.identity].status == "stale" + + class MovingHeads(dict): + reads = 0 + + def get(self, key, default=None): + self.reads += 1 + return HEAD_2 if self.reads == 1 else HEAD_3 + + moved = plan_dispatches( + ControllerState.empty(), + [current], + live_heads=MovingHeads(), + dispatch_budget=1, + ) + assert moved.dispatches == () + assert moved.rejections[current.identity] == "stale_head" + + +def test_controller_self_test_executes_public_smoke_contract() -> None: + controller.self_test() diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index 1b1cdf3722..2ec7b32f37 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -6,6 +6,7 @@ import socket import subprocess import sys +from io import BytesIO from pathlib import Path import pytest @@ -671,6 +672,7 @@ def test_no_redirect_handler_raises_httperror_without_following(): sandboxed_web_e2e.NoRedirectHandler().redirect_request(request, None, 302, "Found", {}, "http://127.0.0.1") assert exc_info.value.code == 302 + exc_info.value.close() def test_wait_for_url_returns_false_after_timeout(monkeypatch, tmp_path): @@ -695,6 +697,37 @@ def open(self, url, timeout): assert sandboxed_web_e2e.wait_for_url("http://127.0.0.1:8000/health", 1, service) is False +def test_wait_for_url_closes_http_error_response(monkeypatch, tmp_path): + class RunningProcess: + def poll(self): + return None + + body = BytesIO(b"redirect") + error = sandboxed_web_e2e.urllib.error.HTTPError( + "http://127.0.0.1:8000/health", 302, "Found", {}, body + ) + ticks = iter([0, 0, 2]) + monkeypatch.setattr(sandboxed_web_e2e.time, "monotonic", lambda: next(ticks)) + monkeypatch.setattr(sandboxed_web_e2e.time, "sleep", lambda _seconds: None) + + class FailingOpener: + def open(self, _url, timeout): + raise error + + monkeypatch.setattr( + sandboxed_web_e2e.urllib.request, + "build_opener", + lambda *_args: FailingOpener(), + ) + service = sandboxed_web_e2e.Service( + "web", "serve", RunningProcess(), tmp_path / "web.log" + ) + assert not sandboxed_web_e2e.wait_for_url( + "http://127.0.0.1:8000/health", 1, service + ) + assert body.closed + + def test_main_runs_with_stubbed_services(monkeypatch, tmp_path, capsys): """Main records success evidence without requiring real POSIX services.""" repo = tmp_path / "repo"