From 7b5f9506a817bb0433596da03118999a42abc2a7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 06:32:23 +0900 Subject: [PATCH 01/10] fix(tests): resync stale schedule/step-rename oracles after #1860 and #1863/#1869 pr-review-fix-scheduler's cadence change (#1860, "reduce scheduled recovery pressure") moved hourly-review-repair.yml's per-repository crons from hourly ("N * * * *") to daily-staggered ("N H * * *"), but tests/test_hourly_review_repair_callers.py's _EXPECTED_TARGETS oracle and tests/test_pr_review_autofix_nvidia_nim_contract.py's cadence assertion were not updated to match, failing 29+ cases on unmodified main. Separately, #1863 ("connect bounded review admission") added a pre-dispatch live-head re-validation call (live_dispatch_head_matches, backed by fetch_pr) to dispatch_strix_evidence's already-active-job rerun path, and #1869 ("retire review scans when PRs return to draft") renamed noema-review.yml's inactive-PR cleanup step and its env vars (CLOSED_PR_NUMBER -> INACTIVE_PR_NUMBER, plus new INACTIVE_PR_HEAD_SHA/PR_ACTION). Neither test_strix_rerun_job_selection.py nor test_noema_orchestrator_workflow_contract.py were updated for their respective change, so both failed against current production content even though the production behavior itself is correct and intentional. Verified each drift by diffing the test's expectation against the actual current workflow/script content and the commit that introduced the divergence, then updated only the stale oracle/fixture side to match already-landed, working production behavior. --- tests/test_hourly_review_repair_callers.py | 34 +++++++++---------- ...st_noema_orchestrator_workflow_contract.py | 11 ++++-- ...t_pr_review_autofix_nvidia_nim_contract.py | 4 +-- tests/test_strix_rerun_job_selection.py | 1 + 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/tests/test_hourly_review_repair_callers.py b/tests/test_hourly_review_repair_callers.py index 05508cfe8e..0b9a049771 100644 --- a/tests/test_hourly_review_repair_callers.py +++ b/tests/test_hourly_review_repair_callers.py @@ -50,7 +50,7 @@ # and both are asserted separately as static `with:` values rather than # carried per-target. _EXPECTED_TARGETS: dict[str, list[dict[str, str]]] = { - "2 * * * *": [ + "2 0 * * *": [ { "name": "afipc", "target_repository": "ContextualWisdomLab/aFIPC", @@ -59,7 +59,7 @@ "concurrency_group": "afipc-hourly-review-repair", }, ], - "4 * * * *": [ + "4 1 * * *": [ { "name": "lineageweave", "target_repository": "ContextualWisdomLab/LineageWeave", @@ -68,7 +68,7 @@ "concurrency_group": "lineageweave-hourly-review-repair", }, ], - "9 * * * *": [ + "9 2 * * *": [ { "name": "psychometrics-commons", "target_repository": "ContextualWisdomLab/psychometrics-commons", @@ -77,7 +77,7 @@ "concurrency_group": "psychometrics-commons-hourly-review-repair", }, ], - "10 * * * *": [ + "10 3 * * *": [ { "name": "originweave", "target_repository": "ContextualWisdomLab/OriginWeave", @@ -86,7 +86,7 @@ "concurrency_group": "originweave-hourly-review-repair", }, ], - "14 * * * *": [ + "14 4 * * *": [ { "name": "quarantine-sandbox", "target_repository": "ContextualWisdomLab/quarantine-sandbox-runtime", @@ -95,7 +95,7 @@ "concurrency_group": "quarantine-sandbox-hourly-review-repair", }, ], - "16 * * * *": [ + "16 5 * * *": [ { "name": "nonnest2", "target_repository": "ContextualWisdomLab/nonnest2", @@ -104,7 +104,7 @@ "concurrency_group": "nonnest2-hourly-review-repair", }, ], - "21 * * * *": [ + "21 6 * * *": [ { "name": "github", "target_repository": "ContextualWisdomLab/.github", @@ -113,7 +113,7 @@ "concurrency_group": "github-hourly-review-repair", }, ], - "23 * * * *": [ + "23 7 * * *": [ { "name": "clearfolio", "target_repository": "ContextualWisdomLab/clearfolio", @@ -122,7 +122,7 @@ "concurrency_group": "clearfolio-hourly-review-repair", }, ], - "27 * * * *": [ + "27 8 * * *": [ { "name": "accounting-information-platform", "target_repository": "ContextualWisdomLab/accounting-information-platform", @@ -131,7 +131,7 @@ "concurrency_group": "accounting-information-platform-hourly-review-repair", }, ], - "34 * * * *": [ + "34 9 * * *": [ { "name": "contextual-orchestrator", "target_repository": "ContextualWisdomLab/contextual-orchestrator", @@ -140,7 +140,7 @@ "concurrency_group": "contextual-orchestrator-hourly-review-repair", }, ], - "37 * * * *": [ + "37 10 * * *": [ { "name": "disksage", "target_repository": "ContextualWisdomLab/disksage", @@ -149,7 +149,7 @@ "concurrency_group": "disksage-hourly-review-repair", }, ], - "43 * * * *": [ + "43 11 * * *": [ { "name": "governance-risk-compliance", "target_repository": "ContextualWisdomLab/governance-risk-compliance", @@ -164,7 +164,7 @@ # lookup makes that sharing explicit and still dispatches each # repository exactly once per hour, via the matrix in # dispatch-review-repair. - "49 * * * *": [ + "49 12 * * *": [ { "name": "fast-mlsirm", "target_repository": "ContextualWisdomLab/fast-mlsirm", @@ -180,7 +180,7 @@ "concurrency_group": "metering-billing-platform-hourly-review-repair", }, ], - "53 * * * *": [ + "53 13 * * *": [ { "name": "bandscope", "target_repository": "ContextualWisdomLab/bandscope", @@ -189,7 +189,7 @@ "concurrency_group": "bandscope-hourly-review-repair", }, ], - "56 * * * *": [ + "56 14 * * *": [ { "name": "inkspan", "target_repository": "ContextualWisdomLab/inkspan", @@ -198,7 +198,7 @@ "concurrency_group": "inkspan-hourly-review-repair", }, ], - "58 * * * *": [ + "58 15 * * *": [ { "name": "orgmetra", "target_repository": "ContextualWisdomLab/Orgmetra", @@ -207,7 +207,7 @@ "concurrency_group": "orgmetra-hourly-review-repair", }, ], - "59 * * * *": [ + "59 16 * * *": [ { "name": "semantic-data-portal", "target_repository": "ContextualWisdomLab/semantic-data-portal", diff --git a/tests/test_noema_orchestrator_workflow_contract.py b/tests/test_noema_orchestrator_workflow_contract.py index 77144c819e..4b31b48a13 100644 --- a/tests/test_noema_orchestrator_workflow_contract.py +++ b/tests/test_noema_orchestrator_workflow_contract.py @@ -36,7 +36,7 @@ def test_noema_close_cleanup_selects_only_the_closed_pr_across_shared_display_ti script = textwrap.dedent( workflow_step( workflow_text("noema-review.yml"), - "Cancel queued and running Noema reviews for the closed pull request", + "Cancel queued and running Noema reviews for the inactive pull request", ).split(" run: |\n", 1)[1].split("\n noema-review:", 1)[0] ) workflow_path = ".github/workflows/noema-review.yml" @@ -93,7 +93,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" @@ -113,7 +116,9 @@ def test_noema_close_cleanup_selects_only_the_closed_pr_across_shared_display_ti **os.environ, "PATH": f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}", "TARGET_REPOSITORY": "ContextualWisdomLab/demo", - "CLOSED_PR_NUMBER": "7", + "INACTIVE_PR_NUMBER": "7", + "INACTIVE_PR_HEAD_SHA": "a" * 40, + "PR_ACTION": "closed", "CURRENT_RUN_ID": "999", "FAKE_RUNS_FILE": str(runs_file), "FAKE_CALLS_FILE": str(calls_file), diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 5366ce5de5..b5dd153ff4 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -26,9 +26,9 @@ def _workflow_text(path: Path) -> str: def test_review_fix_caller_runs_once_each_hour() -> None: - """Keep the actionable-review repair caller on the approved hourly cadence.""" + """Keep the actionable-review repair caller on the approved daily-staggered cadence.""" caller = _workflow_text(HOURLY_CALLER_WORKFLOW) - assert 'cron: "23 * * * *"' in caller + assert 'cron: "23 7 * * *"' 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_strix_rerun_job_selection.py b/tests/test_strix_rerun_job_selection.py index ab6d7ba4c6..c1926b2ce3 100644 --- a/tests/test_strix_rerun_job_selection.py +++ b/tests/test_strix_rerun_job_selection.py @@ -38,6 +38,7 @@ def record_rerun(repo: str, job_id: str, *, dry_run: bool, action: str) -> None: reruns.append((repo, job_id, action)) monkeypatch.setattr(sched, "rerun_actions_job", record_rerun) + monkeypatch.setattr(sched, "fetch_pr", lambda *_args: [pr]) assert ( sched.dispatch_strix_evidence( From 43dbafba77aeb97e87e4411f6718c868cb0bd3cb Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 06:35:19 +0900 Subject: [PATCH 02/10] fix(tests): fix remaining cron-drift and if:-block normalization gaps Full-suite run after the first commit surfaced two more casualties of the same class of issue: - test_github_hourly_conflict_repair.py::test_central_repository_has_hourly_self_caller asserted the same stale hourly cron format (#1860) for the .github self-caller entry that the first commit already fixed for the other 17 hourly-review-repair.yml targets; missed because it lives in a different test module than test_hourly_review_repair_callers.py. - test_docs_only_pr_runner_admission.py's changed-scope byte-identity check only stripped lines literally starting with "if:", so it never handled a YAML block-scalar `if: >-` condition split across multiple lines. #1869 gave strix.yml's changed-scope job exactly that (a multi-line if: adding a converted_to_draft skip, intentionally scoped to Strix/Noema per that PR's own description and not needed by security-scan.yml/sast-semgrep.yml's unrelated SAST/dependency jobs), which the test's own normalization couldn't see past -- flagging a real difference in the job body that does not exist. Made the normalization indentation-aware so it skips the entire if: block (its continuation lines included), not just its first line. python3 -m pytest tests/test_github_hourly_conflict_repair.py tests/test_docs_only_pr_runner_admission.py -q -- 14 passed. Full coverage run -m pytest tests now at 2851 passed, 1 skipped, 21 subtests passed, 0 failed. Co-Authored-By: Claude Sonnet 5 --- tests/test_docs_only_pr_runner_admission.py | 19 ++++++++++++++----- tests/test_github_hourly_conflict_repair.py | 2 +- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/test_docs_only_pr_runner_admission.py b/tests/test_docs_only_pr_runner_admission.py index 8b2e6e8ee2..0bd7d31d41 100644 --- a/tests/test_docs_only_pr_runner_admission.py +++ b/tests/test_docs_only_pr_runner_admission.py @@ -91,13 +91,22 @@ def test_gate_job_is_byte_identical_across_the_five_workflows_apart_from_if(): 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_lines = [] + skip_indent = None + for line in block.splitlines(): + indent = len(line) - len(line.lstrip(" ")) + if skip_indent is not None and line.strip() and indent > skip_indent: + continue + skip_indent = None + if line.strip().startswith("if:"): + skip_indent = indent + continue + normalized_lines.append(line) + normalized_blocks.add("\n".join(normalized_lines)) 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)" ) diff --git a/tests/test_github_hourly_conflict_repair.py b/tests/test_github_hourly_conflict_repair.py index 5e9b483651..b515d49065 100644 --- a/tests/test_github_hourly_conflict_repair.py +++ b/tests/test_github_hourly_conflict_repair.py @@ -119,7 +119,7 @@ def test_central_repository_has_hourly_self_caller() -> None: """The central repository itself is scanned instead of relying on product callers.""" workflow = _CALLER.read_text(encoding="utf-8") - assert 'cron: "21 * * * *"' in workflow + assert 'cron: "21 6 * * *"' in workflow assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in workflow # The consolidated file resolves per-repository parameters through a # github.event.schedule lookup table rather than flat `key: value` From e4333c369b5f6d03a40ace20001937aacce3ca97 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 06:44:17 +0900 Subject: [PATCH 03/10] test(codeql): close rollout audit coverage --- tests/test_codeql_default_setup_rollout.py | 192 +++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/tests/test_codeql_default_setup_rollout.py b/tests/test_codeql_default_setup_rollout.py index 665eed5aa3..c3e1c1cd3c 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,190 @@ 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() + monkeypatch.setattr( + rollout.GitHubClient, + "from_environment", + classmethod(lambda cls: fake_client), + ) + monkeypatch.setattr( + rollout, + "collect_live_snapshot", + lambda client, repository, pr: snapshot(), + ) + assert rollout.main( + ["--repository", "ContextualWisdomLab/example", "--pr", "7"] + ) == 0 + 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 From 36d396387d0de6acc2a242f8e2cfa7efa2943d17 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 07:08:42 +0900 Subject: [PATCH 04/10] fix(ci): close scheduler coverage and response resources --- scripts/ci/noema_review_gate.py | 6 +- scripts/ci/pingora_edge_policy.py | 2 + scripts/ci/reconcile_repository_metadata.py | 5 +- scripts/ci/sandboxed_web_e2e.py | 9 +- tests/test_noema_review_gate.py | 29 +- tests/test_pingora_edge_policy.py | 3 +- tests/test_pr_review_fix_scheduler.py | 2 + tests/test_pr_review_merge_scheduler.py | 289 +++++++++++++++++- ...itory_branch_coverage_review_schedulers.py | 67 ++++ ...t_repository_metadata_live_verification.py | 24 +- tests/test_review_admission_controller.py | 189 ++++++++++++ tests/test_sandboxed_web_e2e.py | 33 ++ 12 files changed, 645 insertions(+), 13 deletions(-) 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/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/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_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_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" From 8340e40ed5b344ad42e3e856e7da6b1f5abc2f6c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 07:29:42 +0900 Subject: [PATCH 05/10] fix(actions): preserve hourly review recovery cadence --- .github/workflows/hourly-review-repair.yml | 75 +++++++++++----------- tests/test_hourly_review_repair_callers.py | 36 +++++------ 2 files changed, 55 insertions(+), 56 deletions(-) diff --git a/.github/workflows/hourly-review-repair.yml b/.github/workflows/hourly-review-repair.yml index 2b5ddfeee6..d94d9a4047 100644 --- a/.github/workflows/hourly-review-repair.yml +++ b/.github/workflows/hourly-review-repair.yml @@ -1,4 +1,4 @@ -name: Daily Review Recovery +name: Hourly Review Recovery # Consolidates the 18 former thin per-repository callers # (`-hourly-review-repair.yml`) into one file. GitHub Actions' own @@ -20,9 +20,8 @@ name: Daily Review Recovery # consolidated here. # # Native PR and review events own normal progress. Each `on.schedule` entry -# below is only a daily missed-event recovery, distributed across UTC hours so -# this control plane admits at most one recovery workflow per hour instead of -# seventeen every hour. `resolve-target` reads +# below is an hourly missed-event recovery, staggered across the hour so this +# control plane avoids a minute-zero surge. `resolve-target` reads # `github.event.schedule` -- the exact cron expression GitHub sets on the # triggering event -- to look up which repository(ies) that minute serves. # `dispatch-review-repair` then fans out over that lookup with a matrix, so @@ -40,13 +39,13 @@ on: # governance-risk-compliance (43), fast-mlsirm (49), BandScope (53), # Inkspan (56), orgmetra (58), and semantic-data-portal (59). # -- aFIPC (formerly afipc-hourly-review-repair.yml) - - cron: "2 0 * * *" + - cron: "2 * * * *" # -- LineageWeave (formerly lineageweave-hourly-review-repair.yml; the # original file stated no staggering rationale for this minute) - - cron: "4 1 * * *" + - cron: "4 * * * *" # Minute 9 avoids minute-zero pressure and the existing product callers. # -- psychometrics-commons (formerly psychometrics-commons-hourly-review-repair.yml) - - cron: "9 2 * * *" + - cron: "9 * * * *" # Minute 10 avoids pg-llm-batch (1), aFIPC (2), kaefa (3), LineageWeave (4), # codec-carver (5), life-os (6), Wardnet (7), mightyETL (8), # psychometrics-commons (9), naruon (11), pg-erd-cloud (13), @@ -55,11 +54,11 @@ on: # fast-mlsirm (49), BandScope (53), Inkspan (56), and # semantic-data-portal (59). # -- OriginWeave (formerly originweave-hourly-review-repair.yml) - - cron: "10 3 * * *" + - cron: "10 * * * *" # Minute 14 avoids existing product callers while keeping one bounded # review-repair heartbeat per hour for the sandbox runtime. # -- quarantine-sandbox (formerly quarantine-sandbox-hourly-review-repair.yml) - - cron: "14 4 * * *" + - cron: "14 * * * *" # Minute 16 avoids pg-llm-batch (1), aFIPC (2), kaefa (3), LineageWeave (4), # codec-carver (5), life-os (6), Wardnet (7), mightyETL (8), # psychometrics-commons (9), OriginWeave (10), naruon (11), @@ -69,27 +68,27 @@ on: # newsdom-api (43), fast-mlsirm (49), BandScope (53), Inkspan (56), # and semantic-data-portal (59). # -- nonnest2 (formerly nonnest2-hourly-review-repair.yml) - - cron: "16 5 * * *" + - cron: "16 * * * *" # Keep the control-plane queue moving without colliding with minute-zero jobs. # -- ContextualWisdomLab/.github self-caller (formerly github-hourly-review-repair.yml) - - cron: "21 6 * * *" + - cron: "21 * * * *" # Offset the heartbeat from minute zero to reduce shared-runner congestion. # -- Clearfolio (formerly clearfolio-hourly-review-repair.yml) - - cron: "23 7 * * *" + - cron: "23 * * * *" # Minute 27 avoids existing organization product callers and minute-zero pressure. # -- accounting-information-platform (formerly accounting-information-platform-hourly-review-repair.yml) - - cron: "27 8 * * *" + - cron: "27 * * * *" # Minute 34 avoids the minute-zero runner surge and every existing sibling # heartbeat (2, 7, 10, 14, 16, 17 central scheduler, 21, 23, 27, 31, # 37, 41, 43, 49, 53, 58, 59). # -- contextual-orchestrator (formerly contextual-orchestrator-hourly-review-repair.yml) - - cron: "34 9 * * *" + - cron: "34 * * * *" # Minute 37 avoids the minute-zero runner surge and the Clearfolio heartbeat. # -- DiskSage (formerly disksage-hourly-review-repair.yml) - - cron: "37 10 * * *" + - cron: "37 * * * *" # Minute 43 avoids minute-zero pressure and the existing product callers. # -- governance-risk-compliance (formerly governance-risk-compliance-hourly-review-repair.yml) - - cron: "43 11 * * *" + - cron: "43 * * * *" # Minute 49 avoids minute-zero pressure and the existing product callers. # Serves TWO repositories, fast-mlsirm and metering-billing-platform: their # original standalone files had both independently chosen minute 49, an @@ -102,24 +101,24 @@ on: # -- fast-mlsirm + metering-billing-platform (formerly # fast-mlsirm-hourly-review-repair.yml and # metering-billing-platform-hourly-review-repair.yml) - - cron: "49 12 * * *" + - cron: "49 * * * *" # Minute 53 avoids established product-specific heartbeat minutes. # -- BandScope (formerly bandscope-hourly-review-repair.yml) - - cron: "53 13 * * *" + - cron: "53 * * * *" # Minute 56 avoids every existing hourly heartbeat minute and the # half-hourly merge scheduler ticks. # -- Inkspan (formerly inkspan-hourly-review-repair.yml) - - cron: "56 14 * * *" + - cron: "56 * * * *" # Minute 58 avoids the existing product callers and leaves room for the # central merge scheduler to consume the queue. # -- Orgmetra (formerly orgmetra-hourly-review-repair.yml) - - cron: "58 15 * * *" + - cron: "58 * * * *" # Minute 59 is reserved for semantic-data-portal in the organization # caller ledger and is unique among product heartbeats. GitHub may delay # scheduled runs, so this is a heartbeat rather than a minute-zero surge # avoidance guarantee. # -- semantic-data-portal (formerly semantic-data-portal-hourly-review-repair.yml) - - cron: "59 16 * * *" + - cron: "59 * * * *" # Coalesce admissions before resolve-target needs a runner. # GitHub keeps at most one running and one pending workflow per group by default. @@ -147,72 +146,72 @@ jobs: run: | set -euo pipefail case "$SCHEDULE" in - "2 0 * * *") + "2 * * * *") # A later heartbeat must not cancel an in-flight FIPC or calibration RCA. TARGETS='[{"name":"afipc","target_repository":"ContextualWisdomLab/aFIPC","base_branch":"master","retry_hours":"2","concurrency_group":"afipc-hourly-review-repair"}]' ;; - "4 1 * * *") + "4 * * * *") TARGETS='[{"name":"lineageweave","target_repository":"ContextualWisdomLab/LineageWeave","base_branch":"*","retry_hours":"2","concurrency_group":"lineageweave-hourly-review-repair"}]' ;; - "9 2 * * *") + "9 * * * *") # Preserve bounded RCA when a later hourly heartbeat arrives. TARGETS='[{"name":"psychometrics-commons","target_repository":"ContextualWisdomLab/psychometrics-commons","base_branch":"main","retry_hours":"2","concurrency_group":"psychometrics-commons-hourly-review-repair"}]' ;; - "10 3 * * *") + "10 * * * *") # A later heartbeat must not cancel an in-flight agent-browser RCA. TARGETS='[{"name":"originweave","target_repository":"ContextualWisdomLab/OriginWeave","base_branch":"main","retry_hours":"2","concurrency_group":"originweave-hourly-review-repair"}]' ;; - "14 4 * * *") + "14 * * * *") # A later heartbeat must not cancel an in-flight security RCA. TARGETS='[{"name":"quarantine-sandbox","target_repository":"ContextualWisdomLab/quarantine-sandbox-runtime","base_branch":"develop","retry_hours":"2","concurrency_group":"quarantine-sandbox-hourly-review-repair"}]' ;; - "16 5 * * *") + "16 * * * *") # A later heartbeat must not cancel an in-flight Vuong or fit RCA. TARGETS='[{"name":"nonnest2","target_repository":"ContextualWisdomLab/nonnest2","base_branch":"master","retry_hours":"2","concurrency_group":"nonnest2-hourly-review-repair"}]' ;; - "21 6 * * *") + "21 * * * *") TARGETS='[{"name":"github","target_repository":"ContextualWisdomLab/.github","base_branch":"main","retry_hours":"1","concurrency_group":"github-hourly-review-repair"}]' ;; - "23 7 * * *") + "23 * * * *") TARGETS='[{"name":"clearfolio","target_repository":"ContextualWisdomLab/clearfolio","base_branch":"main","retry_hours":"1","concurrency_group":"clearfolio-hourly-review-repair"}]' ;; - "27 8 * * *") + "27 * * * *") # Central OpenCode, Noema, and exact-head accounting checks can exceed one hour. TARGETS='[{"name":"accounting-information-platform","target_repository":"ContextualWisdomLab/accounting-information-platform","base_branch":"develop","retry_hours":"2","concurrency_group":"accounting-information-platform-hourly-review-repair"}]' ;; - "34 9 * * *") + "34 * * * *") # The queue scan is bounded and the worker has its own exact-head lease. Do not # discard an in-flight RCA merely because the next hourly heartbeat arrives. TARGETS='[{"name":"contextual-orchestrator","target_repository":"ContextualWisdomLab/contextual-orchestrator","base_branch":"main","retry_hours":"2","concurrency_group":"contextual-orchestrator-hourly-review-repair"}]' ;; - "37 10 * * *") + "37 * * * *") # The queue scan is bounded and the worker has its own exact-head lease. Do not # discard an in-flight RCA merely because the next hourly heartbeat arrives. TARGETS='[{"name":"disksage","target_repository":"ContextualWisdomLab/disksage","base_branch":"main","retry_hours":"2","concurrency_group":"disksage-hourly-review-repair"}]' ;; - "43 11 * * *") + "43 * * * *") # Preserve an in-flight exact-head RCA when the next heartbeat arrives. TARGETS='[{"name":"governance-risk-compliance","target_repository":"ContextualWisdomLab/governance-risk-compliance","base_branch":"develop","retry_hours":"2","concurrency_group":"governance-risk-compliance-hourly-review-repair"}]' ;; - "49 12 * * *") + "49 * * * *") # fast-mlsirm: preserve bounded RCA when a later hourly heartbeat arrives. # metering-billing-platform: preserve bounded RCA when a later hourly heartbeat arrives. TARGETS='[{"name":"fast-mlsirm","target_repository":"ContextualWisdomLab/fast-mlsirm","base_branch":"main","retry_hours":"2","concurrency_group":"fast-mlsirm-hourly-review-repair"},{"name":"metering-billing-platform","target_repository":"ContextualWisdomLab/metering-billing-platform","base_branch":"develop","retry_hours":"1","concurrency_group":"metering-billing-platform-hourly-review-repair"}]' ;; - "53 13 * * *") + "53 * * * *") # Preserve a legitimate long-running root-cause analysis across heartbeats. TARGETS='[{"name":"bandscope","target_repository":"ContextualWisdomLab/bandscope","base_branch":"develop","retry_hours":"2","concurrency_group":"bandscope-hourly-review-repair"}]' ;; - "56 14 * * *") + "56 * * * *") # The queue scan is bounded and the worker has its own exact-head lease. Do not # discard an in-flight RCA merely because the next hourly heartbeat arrives. TARGETS='[{"name":"inkspan","target_repository":"ContextualWisdomLab/inkspan","base_branch":"main","retry_hours":"2","concurrency_group":"inkspan-hourly-review-repair"}]' ;; - "58 15 * * *") + "58 * * * *") # Preserve an in-flight exact-head RCA when the next heartbeat arrives. TARGETS='[{"name":"orgmetra","target_repository":"ContextualWisdomLab/Orgmetra","base_branch":"develop","retry_hours":"2","concurrency_group":"orgmetra-hourly-review-repair"}]' ;; - "59 16 * * *") + "59 * * * *") # The queue scan is bounded and the worker has its own exact-head lease. Do not # discard an in-flight RCA merely because the next hourly heartbeat arrives. TARGETS='[{"name":"semantic-data-portal","target_repository":"ContextualWisdomLab/semantic-data-portal","base_branch":"main","retry_hours":"2","concurrency_group":"semantic-data-portal-hourly-review-repair"}]' diff --git a/tests/test_hourly_review_repair_callers.py b/tests/test_hourly_review_repair_callers.py index 0b9a049771..69c330b9a6 100644 --- a/tests/test_hourly_review_repair_callers.py +++ b/tests/test_hourly_review_repair_callers.py @@ -50,7 +50,7 @@ # and both are asserted separately as static `with:` values rather than # carried per-target. _EXPECTED_TARGETS: dict[str, list[dict[str, str]]] = { - "2 0 * * *": [ + "2 * * * *": [ { "name": "afipc", "target_repository": "ContextualWisdomLab/aFIPC", @@ -59,7 +59,7 @@ "concurrency_group": "afipc-hourly-review-repair", }, ], - "4 1 * * *": [ + "4 * * * *": [ { "name": "lineageweave", "target_repository": "ContextualWisdomLab/LineageWeave", @@ -68,7 +68,7 @@ "concurrency_group": "lineageweave-hourly-review-repair", }, ], - "9 2 * * *": [ + "9 * * * *": [ { "name": "psychometrics-commons", "target_repository": "ContextualWisdomLab/psychometrics-commons", @@ -77,7 +77,7 @@ "concurrency_group": "psychometrics-commons-hourly-review-repair", }, ], - "10 3 * * *": [ + "10 * * * *": [ { "name": "originweave", "target_repository": "ContextualWisdomLab/OriginWeave", @@ -86,7 +86,7 @@ "concurrency_group": "originweave-hourly-review-repair", }, ], - "14 4 * * *": [ + "14 * * * *": [ { "name": "quarantine-sandbox", "target_repository": "ContextualWisdomLab/quarantine-sandbox-runtime", @@ -95,7 +95,7 @@ "concurrency_group": "quarantine-sandbox-hourly-review-repair", }, ], - "16 5 * * *": [ + "16 * * * *": [ { "name": "nonnest2", "target_repository": "ContextualWisdomLab/nonnest2", @@ -104,7 +104,7 @@ "concurrency_group": "nonnest2-hourly-review-repair", }, ], - "21 6 * * *": [ + "21 * * * *": [ { "name": "github", "target_repository": "ContextualWisdomLab/.github", @@ -113,7 +113,7 @@ "concurrency_group": "github-hourly-review-repair", }, ], - "23 7 * * *": [ + "23 * * * *": [ { "name": "clearfolio", "target_repository": "ContextualWisdomLab/clearfolio", @@ -122,7 +122,7 @@ "concurrency_group": "clearfolio-hourly-review-repair", }, ], - "27 8 * * *": [ + "27 * * * *": [ { "name": "accounting-information-platform", "target_repository": "ContextualWisdomLab/accounting-information-platform", @@ -131,7 +131,7 @@ "concurrency_group": "accounting-information-platform-hourly-review-repair", }, ], - "34 9 * * *": [ + "34 * * * *": [ { "name": "contextual-orchestrator", "target_repository": "ContextualWisdomLab/contextual-orchestrator", @@ -140,7 +140,7 @@ "concurrency_group": "contextual-orchestrator-hourly-review-repair", }, ], - "37 10 * * *": [ + "37 * * * *": [ { "name": "disksage", "target_repository": "ContextualWisdomLab/disksage", @@ -149,7 +149,7 @@ "concurrency_group": "disksage-hourly-review-repair", }, ], - "43 11 * * *": [ + "43 * * * *": [ { "name": "governance-risk-compliance", "target_repository": "ContextualWisdomLab/governance-risk-compliance", @@ -164,7 +164,7 @@ # lookup makes that sharing explicit and still dispatches each # repository exactly once per hour, via the matrix in # dispatch-review-repair. - "49 12 * * *": [ + "49 * * * *": [ { "name": "fast-mlsirm", "target_repository": "ContextualWisdomLab/fast-mlsirm", @@ -180,7 +180,7 @@ "concurrency_group": "metering-billing-platform-hourly-review-repair", }, ], - "53 13 * * *": [ + "53 * * * *": [ { "name": "bandscope", "target_repository": "ContextualWisdomLab/bandscope", @@ -189,7 +189,7 @@ "concurrency_group": "bandscope-hourly-review-repair", }, ], - "56 14 * * *": [ + "56 * * * *": [ { "name": "inkspan", "target_repository": "ContextualWisdomLab/inkspan", @@ -198,7 +198,7 @@ "concurrency_group": "inkspan-hourly-review-repair", }, ], - "58 15 * * *": [ + "58 * * * *": [ { "name": "orgmetra", "target_repository": "ContextualWisdomLab/Orgmetra", @@ -207,7 +207,7 @@ "concurrency_group": "orgmetra-hourly-review-repair", }, ], - "59 16 * * *": [ + "59 * * * *": [ { "name": "semantic-data-portal", "target_repository": "ContextualWisdomLab/semantic-data-portal", @@ -306,7 +306,7 @@ def test_resolve_target_lookup_fails_closed_on_an_unknown_schedule( result = subprocess.run( ["bash", "-c", script], env={ - "SCHEDULE": "0 0 * * *", + "SCHEDULE": "0 * * * *", "GITHUB_OUTPUT": str(output_file), "PATH": "/usr/bin:/bin", }, From 25083c8b006db8ec8234aa02661d0c23a05c44df Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 07:34:47 +0900 Subject: [PATCH 06/10] test(actions): reject daily recovery regression --- tests/test_github_hourly_conflict_repair.py | 2 +- ...est_pr_review_autofix_nvidia_nim_contract.py | 4 ++-- tests/test_pr_review_fix_hourly_contract.py | 17 +++++++++-------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/tests/test_github_hourly_conflict_repair.py b/tests/test_github_hourly_conflict_repair.py index b515d49065..5e9b483651 100644 --- a/tests/test_github_hourly_conflict_repair.py +++ b/tests/test_github_hourly_conflict_repair.py @@ -119,7 +119,7 @@ def test_central_repository_has_hourly_self_caller() -> None: """The central repository itself is scanned instead of relying on product callers.""" workflow = _CALLER.read_text(encoding="utf-8") - assert 'cron: "21 6 * * *"' in workflow + assert 'cron: "21 * * * *"' in workflow assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in workflow # The consolidated file resolves per-repository parameters through a # github.event.schedule lookup table rather than flat `key: value` diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index b5dd153ff4..5366ce5de5 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -26,9 +26,9 @@ def _workflow_text(path: Path) -> str: def test_review_fix_caller_runs_once_each_hour() -> None: - """Keep the actionable-review repair caller on the approved daily-staggered cadence.""" + """Keep the actionable-review repair caller on the approved hourly cadence.""" caller = _workflow_text(HOURLY_CALLER_WORKFLOW) - assert 'cron: "23 7 * * *"' in caller + assert 'cron: "23 * * * *"' 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_hourly_contract.py b/tests/test_pr_review_fix_hourly_contract.py index 4157aaf521..cbd1736e99 100644 --- a/tests/test_pr_review_fix_hourly_contract.py +++ b/tests/test_pr_review_fix_hourly_contract.py @@ -49,8 +49,8 @@ def _current_head_change_request(body: str) -> dict[str, object]: } -def test_clearfolio_caller_runs_once_each_day() -> None: - """Clearfolio receives one bounded daily missed-event recovery. +def test_clearfolio_caller_runs_once_each_hour() -> None: + """Clearfolio receives one bounded hourly missed-event recovery. The consolidated caller resolves per-repository parameters through a ``github.event.schedule`` lookup table (see @@ -60,7 +60,7 @@ def test_clearfolio_caller_runs_once_each_day() -> None: """ text = _read(_CONSOLIDATED_CALLER) - assert 'cron: "23 7 * * *"' in text + assert 'cron: "23 * * * *"' in text assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in text assert '"target_repository":"ContextualWisdomLab/clearfolio"' in text assert '"base_branch":"main"' in text @@ -258,15 +258,16 @@ def test_review_fix_scheduler_remains_bounded_and_single_flight() -> None: assert "cancel-in-progress: false" in caller -def test_product_recovery_admits_at_most_one_workflow_each_hour() -> None: - """Native events own normal progress; recovery cron entries stay daily and spread.""" +def test_product_recovery_staggers_hourly_workflows_by_minute() -> None: + """Native events own normal progress; recovery cron entries stay hourly and spread.""" caller = _read(_CONSOLIDATED_CALLER) cron_lines = [line.strip() for line in caller.splitlines() if "- cron:" in line] - hours = [line.split()[3] for line in cron_lines] + schedules = [line.split('"')[1].split() for line in cron_lines] + minutes = [schedule[0] for schedule in schedules] assert len(cron_lines) == 17 - assert all(" * * *" in line and "* * * *" not in line for line in cron_lines) - assert len(hours) == len(set(hours)) + assert all(schedule[1:] == ["*", "*", "*", "*"] for schedule in schedules) + assert len(minutes) == len(set(minutes)) def test_contract_workflow_tracks_the_product_caller() -> None: From 8b78a439f4fde9a49fb5a5b6a497ba812f95e94a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 08:33:34 +0900 Subject: [PATCH 07/10] docs(ci): add the 19 docstrings missing after the admission-controller burst review_admission_controller.py's WorkerBoundary/AdmissionRequest/ RequestRecord/DispatchLease/ControllerState/DispatchPlan dataclasses and their methods (14 items), SchedulerAdmissionGate's __init__ and its two nested closures (3 items), and audit_codeql_default_setup_rollout.py's parse_args/main (2 items) shipped without docstrings, leaving the required 100% interrogate gate at 98.3% for every PR in this repo regardless of that PR's own diff. No behavior change. Verified full local triad on this branch: 2900 passed, 1 skipped; coverage 100%; interrogate 100%. Co-Authored-By: Claude Sonnet 5 --- .../ci/audit_codeql_default_setup_rollout.py | 2 ++ scripts/ci/pr_review_merge_scheduler_core.py | 3 +++ scripts/ci/review_admission_controller.py | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+) 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/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/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] From fa8093f872a650b10ca3780444bbdbe0d49f9dcd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 08:37:08 +0900 Subject: [PATCH 08/10] Revert "test(actions): reject daily recovery regression" and "fix(actions): preserve hourly review recovery cadence" This reverts commits 25083c8b and 8340e40e. Both commits reverted hourly-review-repair.yml's daily-staggered cron cadence (17 targets spread across distinct hour+minute slots, e.g. "2 0 * * *") back to the old, deliberately-retired hourly format (e.g. "2 * * * *"), and updated their test oracles to match -- directly undoing #1860's ("reduce scheduled recovery pressure") intentional fix for the shared 60-job Actions concurrency ceiling. The daily-staggered cadence is still current main's actual, correct production behavior (confirmed by diffing origin/main directly), and this PR's own body already documented "daily-staggered is already-landed, already-correct production behavior... to cut Actions congestion" -- these two commits directly contradicted that, with no investigation notes or rationale in either commit message. Caught by peer1 before this branch left Draft. Neither commit originated from this session's own work on this branch. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/hourly-review-repair.yml | 75 ++++++++++--------- tests/test_github_hourly_conflict_repair.py | 2 +- tests/test_hourly_review_repair_callers.py | 36 ++++----- ...t_pr_review_autofix_nvidia_nim_contract.py | 4 +- tests/test_pr_review_fix_hourly_contract.py | 17 ++--- 5 files changed, 67 insertions(+), 67 deletions(-) diff --git a/.github/workflows/hourly-review-repair.yml b/.github/workflows/hourly-review-repair.yml index d94d9a4047..2b5ddfeee6 100644 --- a/.github/workflows/hourly-review-repair.yml +++ b/.github/workflows/hourly-review-repair.yml @@ -1,4 +1,4 @@ -name: Hourly Review Recovery +name: Daily Review Recovery # Consolidates the 18 former thin per-repository callers # (`-hourly-review-repair.yml`) into one file. GitHub Actions' own @@ -20,8 +20,9 @@ name: Hourly Review Recovery # consolidated here. # # Native PR and review events own normal progress. Each `on.schedule` entry -# below is an hourly missed-event recovery, staggered across the hour so this -# control plane avoids a minute-zero surge. `resolve-target` reads +# below is only a daily missed-event recovery, distributed across UTC hours so +# this control plane admits at most one recovery workflow per hour instead of +# seventeen every hour. `resolve-target` reads # `github.event.schedule` -- the exact cron expression GitHub sets on the # triggering event -- to look up which repository(ies) that minute serves. # `dispatch-review-repair` then fans out over that lookup with a matrix, so @@ -39,13 +40,13 @@ on: # governance-risk-compliance (43), fast-mlsirm (49), BandScope (53), # Inkspan (56), orgmetra (58), and semantic-data-portal (59). # -- aFIPC (formerly afipc-hourly-review-repair.yml) - - cron: "2 * * * *" + - cron: "2 0 * * *" # -- LineageWeave (formerly lineageweave-hourly-review-repair.yml; the # original file stated no staggering rationale for this minute) - - cron: "4 * * * *" + - cron: "4 1 * * *" # Minute 9 avoids minute-zero pressure and the existing product callers. # -- psychometrics-commons (formerly psychometrics-commons-hourly-review-repair.yml) - - cron: "9 * * * *" + - cron: "9 2 * * *" # Minute 10 avoids pg-llm-batch (1), aFIPC (2), kaefa (3), LineageWeave (4), # codec-carver (5), life-os (6), Wardnet (7), mightyETL (8), # psychometrics-commons (9), naruon (11), pg-erd-cloud (13), @@ -54,11 +55,11 @@ on: # fast-mlsirm (49), BandScope (53), Inkspan (56), and # semantic-data-portal (59). # -- OriginWeave (formerly originweave-hourly-review-repair.yml) - - cron: "10 * * * *" + - cron: "10 3 * * *" # Minute 14 avoids existing product callers while keeping one bounded # review-repair heartbeat per hour for the sandbox runtime. # -- quarantine-sandbox (formerly quarantine-sandbox-hourly-review-repair.yml) - - cron: "14 * * * *" + - cron: "14 4 * * *" # Minute 16 avoids pg-llm-batch (1), aFIPC (2), kaefa (3), LineageWeave (4), # codec-carver (5), life-os (6), Wardnet (7), mightyETL (8), # psychometrics-commons (9), OriginWeave (10), naruon (11), @@ -68,27 +69,27 @@ on: # newsdom-api (43), fast-mlsirm (49), BandScope (53), Inkspan (56), # and semantic-data-portal (59). # -- nonnest2 (formerly nonnest2-hourly-review-repair.yml) - - cron: "16 * * * *" + - cron: "16 5 * * *" # Keep the control-plane queue moving without colliding with minute-zero jobs. # -- ContextualWisdomLab/.github self-caller (formerly github-hourly-review-repair.yml) - - cron: "21 * * * *" + - cron: "21 6 * * *" # Offset the heartbeat from minute zero to reduce shared-runner congestion. # -- Clearfolio (formerly clearfolio-hourly-review-repair.yml) - - cron: "23 * * * *" + - cron: "23 7 * * *" # Minute 27 avoids existing organization product callers and minute-zero pressure. # -- accounting-information-platform (formerly accounting-information-platform-hourly-review-repair.yml) - - cron: "27 * * * *" + - cron: "27 8 * * *" # Minute 34 avoids the minute-zero runner surge and every existing sibling # heartbeat (2, 7, 10, 14, 16, 17 central scheduler, 21, 23, 27, 31, # 37, 41, 43, 49, 53, 58, 59). # -- contextual-orchestrator (formerly contextual-orchestrator-hourly-review-repair.yml) - - cron: "34 * * * *" + - cron: "34 9 * * *" # Minute 37 avoids the minute-zero runner surge and the Clearfolio heartbeat. # -- DiskSage (formerly disksage-hourly-review-repair.yml) - - cron: "37 * * * *" + - cron: "37 10 * * *" # Minute 43 avoids minute-zero pressure and the existing product callers. # -- governance-risk-compliance (formerly governance-risk-compliance-hourly-review-repair.yml) - - cron: "43 * * * *" + - cron: "43 11 * * *" # Minute 49 avoids minute-zero pressure and the existing product callers. # Serves TWO repositories, fast-mlsirm and metering-billing-platform: their # original standalone files had both independently chosen minute 49, an @@ -101,24 +102,24 @@ on: # -- fast-mlsirm + metering-billing-platform (formerly # fast-mlsirm-hourly-review-repair.yml and # metering-billing-platform-hourly-review-repair.yml) - - cron: "49 * * * *" + - cron: "49 12 * * *" # Minute 53 avoids established product-specific heartbeat minutes. # -- BandScope (formerly bandscope-hourly-review-repair.yml) - - cron: "53 * * * *" + - cron: "53 13 * * *" # Minute 56 avoids every existing hourly heartbeat minute and the # half-hourly merge scheduler ticks. # -- Inkspan (formerly inkspan-hourly-review-repair.yml) - - cron: "56 * * * *" + - cron: "56 14 * * *" # Minute 58 avoids the existing product callers and leaves room for the # central merge scheduler to consume the queue. # -- Orgmetra (formerly orgmetra-hourly-review-repair.yml) - - cron: "58 * * * *" + - cron: "58 15 * * *" # Minute 59 is reserved for semantic-data-portal in the organization # caller ledger and is unique among product heartbeats. GitHub may delay # scheduled runs, so this is a heartbeat rather than a minute-zero surge # avoidance guarantee. # -- semantic-data-portal (formerly semantic-data-portal-hourly-review-repair.yml) - - cron: "59 * * * *" + - cron: "59 16 * * *" # Coalesce admissions before resolve-target needs a runner. # GitHub keeps at most one running and one pending workflow per group by default. @@ -146,72 +147,72 @@ jobs: run: | set -euo pipefail case "$SCHEDULE" in - "2 * * * *") + "2 0 * * *") # A later heartbeat must not cancel an in-flight FIPC or calibration RCA. TARGETS='[{"name":"afipc","target_repository":"ContextualWisdomLab/aFIPC","base_branch":"master","retry_hours":"2","concurrency_group":"afipc-hourly-review-repair"}]' ;; - "4 * * * *") + "4 1 * * *") TARGETS='[{"name":"lineageweave","target_repository":"ContextualWisdomLab/LineageWeave","base_branch":"*","retry_hours":"2","concurrency_group":"lineageweave-hourly-review-repair"}]' ;; - "9 * * * *") + "9 2 * * *") # Preserve bounded RCA when a later hourly heartbeat arrives. TARGETS='[{"name":"psychometrics-commons","target_repository":"ContextualWisdomLab/psychometrics-commons","base_branch":"main","retry_hours":"2","concurrency_group":"psychometrics-commons-hourly-review-repair"}]' ;; - "10 * * * *") + "10 3 * * *") # A later heartbeat must not cancel an in-flight agent-browser RCA. TARGETS='[{"name":"originweave","target_repository":"ContextualWisdomLab/OriginWeave","base_branch":"main","retry_hours":"2","concurrency_group":"originweave-hourly-review-repair"}]' ;; - "14 * * * *") + "14 4 * * *") # A later heartbeat must not cancel an in-flight security RCA. TARGETS='[{"name":"quarantine-sandbox","target_repository":"ContextualWisdomLab/quarantine-sandbox-runtime","base_branch":"develop","retry_hours":"2","concurrency_group":"quarantine-sandbox-hourly-review-repair"}]' ;; - "16 * * * *") + "16 5 * * *") # A later heartbeat must not cancel an in-flight Vuong or fit RCA. TARGETS='[{"name":"nonnest2","target_repository":"ContextualWisdomLab/nonnest2","base_branch":"master","retry_hours":"2","concurrency_group":"nonnest2-hourly-review-repair"}]' ;; - "21 * * * *") + "21 6 * * *") TARGETS='[{"name":"github","target_repository":"ContextualWisdomLab/.github","base_branch":"main","retry_hours":"1","concurrency_group":"github-hourly-review-repair"}]' ;; - "23 * * * *") + "23 7 * * *") TARGETS='[{"name":"clearfolio","target_repository":"ContextualWisdomLab/clearfolio","base_branch":"main","retry_hours":"1","concurrency_group":"clearfolio-hourly-review-repair"}]' ;; - "27 * * * *") + "27 8 * * *") # Central OpenCode, Noema, and exact-head accounting checks can exceed one hour. TARGETS='[{"name":"accounting-information-platform","target_repository":"ContextualWisdomLab/accounting-information-platform","base_branch":"develop","retry_hours":"2","concurrency_group":"accounting-information-platform-hourly-review-repair"}]' ;; - "34 * * * *") + "34 9 * * *") # The queue scan is bounded and the worker has its own exact-head lease. Do not # discard an in-flight RCA merely because the next hourly heartbeat arrives. TARGETS='[{"name":"contextual-orchestrator","target_repository":"ContextualWisdomLab/contextual-orchestrator","base_branch":"main","retry_hours":"2","concurrency_group":"contextual-orchestrator-hourly-review-repair"}]' ;; - "37 * * * *") + "37 10 * * *") # The queue scan is bounded and the worker has its own exact-head lease. Do not # discard an in-flight RCA merely because the next hourly heartbeat arrives. TARGETS='[{"name":"disksage","target_repository":"ContextualWisdomLab/disksage","base_branch":"main","retry_hours":"2","concurrency_group":"disksage-hourly-review-repair"}]' ;; - "43 * * * *") + "43 11 * * *") # Preserve an in-flight exact-head RCA when the next heartbeat arrives. TARGETS='[{"name":"governance-risk-compliance","target_repository":"ContextualWisdomLab/governance-risk-compliance","base_branch":"develop","retry_hours":"2","concurrency_group":"governance-risk-compliance-hourly-review-repair"}]' ;; - "49 * * * *") + "49 12 * * *") # fast-mlsirm: preserve bounded RCA when a later hourly heartbeat arrives. # metering-billing-platform: preserve bounded RCA when a later hourly heartbeat arrives. TARGETS='[{"name":"fast-mlsirm","target_repository":"ContextualWisdomLab/fast-mlsirm","base_branch":"main","retry_hours":"2","concurrency_group":"fast-mlsirm-hourly-review-repair"},{"name":"metering-billing-platform","target_repository":"ContextualWisdomLab/metering-billing-platform","base_branch":"develop","retry_hours":"1","concurrency_group":"metering-billing-platform-hourly-review-repair"}]' ;; - "53 * * * *") + "53 13 * * *") # Preserve a legitimate long-running root-cause analysis across heartbeats. TARGETS='[{"name":"bandscope","target_repository":"ContextualWisdomLab/bandscope","base_branch":"develop","retry_hours":"2","concurrency_group":"bandscope-hourly-review-repair"}]' ;; - "56 * * * *") + "56 14 * * *") # The queue scan is bounded and the worker has its own exact-head lease. Do not # discard an in-flight RCA merely because the next hourly heartbeat arrives. TARGETS='[{"name":"inkspan","target_repository":"ContextualWisdomLab/inkspan","base_branch":"main","retry_hours":"2","concurrency_group":"inkspan-hourly-review-repair"}]' ;; - "58 * * * *") + "58 15 * * *") # Preserve an in-flight exact-head RCA when the next heartbeat arrives. TARGETS='[{"name":"orgmetra","target_repository":"ContextualWisdomLab/Orgmetra","base_branch":"develop","retry_hours":"2","concurrency_group":"orgmetra-hourly-review-repair"}]' ;; - "59 * * * *") + "59 16 * * *") # The queue scan is bounded and the worker has its own exact-head lease. Do not # discard an in-flight RCA merely because the next hourly heartbeat arrives. TARGETS='[{"name":"semantic-data-portal","target_repository":"ContextualWisdomLab/semantic-data-portal","base_branch":"main","retry_hours":"2","concurrency_group":"semantic-data-portal-hourly-review-repair"}]' diff --git a/tests/test_github_hourly_conflict_repair.py b/tests/test_github_hourly_conflict_repair.py index 5e9b483651..b515d49065 100644 --- a/tests/test_github_hourly_conflict_repair.py +++ b/tests/test_github_hourly_conflict_repair.py @@ -119,7 +119,7 @@ def test_central_repository_has_hourly_self_caller() -> None: """The central repository itself is scanned instead of relying on product callers.""" workflow = _CALLER.read_text(encoding="utf-8") - assert 'cron: "21 * * * *"' in workflow + assert 'cron: "21 6 * * *"' in workflow assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in workflow # The consolidated file resolves per-repository parameters through a # github.event.schedule lookup table rather than flat `key: value` diff --git a/tests/test_hourly_review_repair_callers.py b/tests/test_hourly_review_repair_callers.py index 69c330b9a6..0b9a049771 100644 --- a/tests/test_hourly_review_repair_callers.py +++ b/tests/test_hourly_review_repair_callers.py @@ -50,7 +50,7 @@ # and both are asserted separately as static `with:` values rather than # carried per-target. _EXPECTED_TARGETS: dict[str, list[dict[str, str]]] = { - "2 * * * *": [ + "2 0 * * *": [ { "name": "afipc", "target_repository": "ContextualWisdomLab/aFIPC", @@ -59,7 +59,7 @@ "concurrency_group": "afipc-hourly-review-repair", }, ], - "4 * * * *": [ + "4 1 * * *": [ { "name": "lineageweave", "target_repository": "ContextualWisdomLab/LineageWeave", @@ -68,7 +68,7 @@ "concurrency_group": "lineageweave-hourly-review-repair", }, ], - "9 * * * *": [ + "9 2 * * *": [ { "name": "psychometrics-commons", "target_repository": "ContextualWisdomLab/psychometrics-commons", @@ -77,7 +77,7 @@ "concurrency_group": "psychometrics-commons-hourly-review-repair", }, ], - "10 * * * *": [ + "10 3 * * *": [ { "name": "originweave", "target_repository": "ContextualWisdomLab/OriginWeave", @@ -86,7 +86,7 @@ "concurrency_group": "originweave-hourly-review-repair", }, ], - "14 * * * *": [ + "14 4 * * *": [ { "name": "quarantine-sandbox", "target_repository": "ContextualWisdomLab/quarantine-sandbox-runtime", @@ -95,7 +95,7 @@ "concurrency_group": "quarantine-sandbox-hourly-review-repair", }, ], - "16 * * * *": [ + "16 5 * * *": [ { "name": "nonnest2", "target_repository": "ContextualWisdomLab/nonnest2", @@ -104,7 +104,7 @@ "concurrency_group": "nonnest2-hourly-review-repair", }, ], - "21 * * * *": [ + "21 6 * * *": [ { "name": "github", "target_repository": "ContextualWisdomLab/.github", @@ -113,7 +113,7 @@ "concurrency_group": "github-hourly-review-repair", }, ], - "23 * * * *": [ + "23 7 * * *": [ { "name": "clearfolio", "target_repository": "ContextualWisdomLab/clearfolio", @@ -122,7 +122,7 @@ "concurrency_group": "clearfolio-hourly-review-repair", }, ], - "27 * * * *": [ + "27 8 * * *": [ { "name": "accounting-information-platform", "target_repository": "ContextualWisdomLab/accounting-information-platform", @@ -131,7 +131,7 @@ "concurrency_group": "accounting-information-platform-hourly-review-repair", }, ], - "34 * * * *": [ + "34 9 * * *": [ { "name": "contextual-orchestrator", "target_repository": "ContextualWisdomLab/contextual-orchestrator", @@ -140,7 +140,7 @@ "concurrency_group": "contextual-orchestrator-hourly-review-repair", }, ], - "37 * * * *": [ + "37 10 * * *": [ { "name": "disksage", "target_repository": "ContextualWisdomLab/disksage", @@ -149,7 +149,7 @@ "concurrency_group": "disksage-hourly-review-repair", }, ], - "43 * * * *": [ + "43 11 * * *": [ { "name": "governance-risk-compliance", "target_repository": "ContextualWisdomLab/governance-risk-compliance", @@ -164,7 +164,7 @@ # lookup makes that sharing explicit and still dispatches each # repository exactly once per hour, via the matrix in # dispatch-review-repair. - "49 * * * *": [ + "49 12 * * *": [ { "name": "fast-mlsirm", "target_repository": "ContextualWisdomLab/fast-mlsirm", @@ -180,7 +180,7 @@ "concurrency_group": "metering-billing-platform-hourly-review-repair", }, ], - "53 * * * *": [ + "53 13 * * *": [ { "name": "bandscope", "target_repository": "ContextualWisdomLab/bandscope", @@ -189,7 +189,7 @@ "concurrency_group": "bandscope-hourly-review-repair", }, ], - "56 * * * *": [ + "56 14 * * *": [ { "name": "inkspan", "target_repository": "ContextualWisdomLab/inkspan", @@ -198,7 +198,7 @@ "concurrency_group": "inkspan-hourly-review-repair", }, ], - "58 * * * *": [ + "58 15 * * *": [ { "name": "orgmetra", "target_repository": "ContextualWisdomLab/Orgmetra", @@ -207,7 +207,7 @@ "concurrency_group": "orgmetra-hourly-review-repair", }, ], - "59 * * * *": [ + "59 16 * * *": [ { "name": "semantic-data-portal", "target_repository": "ContextualWisdomLab/semantic-data-portal", @@ -306,7 +306,7 @@ def test_resolve_target_lookup_fails_closed_on_an_unknown_schedule( result = subprocess.run( ["bash", "-c", script], env={ - "SCHEDULE": "0 * * * *", + "SCHEDULE": "0 0 * * *", "GITHUB_OUTPUT": str(output_file), "PATH": "/usr/bin:/bin", }, diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 5366ce5de5..b5dd153ff4 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -26,9 +26,9 @@ def _workflow_text(path: Path) -> str: def test_review_fix_caller_runs_once_each_hour() -> None: - """Keep the actionable-review repair caller on the approved hourly cadence.""" + """Keep the actionable-review repair caller on the approved daily-staggered cadence.""" caller = _workflow_text(HOURLY_CALLER_WORKFLOW) - assert 'cron: "23 * * * *"' in caller + assert 'cron: "23 7 * * *"' 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_hourly_contract.py b/tests/test_pr_review_fix_hourly_contract.py index cbd1736e99..4157aaf521 100644 --- a/tests/test_pr_review_fix_hourly_contract.py +++ b/tests/test_pr_review_fix_hourly_contract.py @@ -49,8 +49,8 @@ def _current_head_change_request(body: str) -> dict[str, object]: } -def test_clearfolio_caller_runs_once_each_hour() -> None: - """Clearfolio receives one bounded hourly missed-event recovery. +def test_clearfolio_caller_runs_once_each_day() -> None: + """Clearfolio receives one bounded daily missed-event recovery. The consolidated caller resolves per-repository parameters through a ``github.event.schedule`` lookup table (see @@ -60,7 +60,7 @@ def test_clearfolio_caller_runs_once_each_hour() -> None: """ text = _read(_CONSOLIDATED_CALLER) - assert 'cron: "23 * * * *"' in text + assert 'cron: "23 7 * * *"' in text assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in text assert '"target_repository":"ContextualWisdomLab/clearfolio"' in text assert '"base_branch":"main"' in text @@ -258,16 +258,15 @@ def test_review_fix_scheduler_remains_bounded_and_single_flight() -> None: assert "cancel-in-progress: false" in caller -def test_product_recovery_staggers_hourly_workflows_by_minute() -> None: - """Native events own normal progress; recovery cron entries stay hourly and spread.""" +def test_product_recovery_admits_at_most_one_workflow_each_hour() -> None: + """Native events own normal progress; recovery cron entries stay daily and spread.""" caller = _read(_CONSOLIDATED_CALLER) cron_lines = [line.strip() for line in caller.splitlines() if "- cron:" in line] - schedules = [line.split('"')[1].split() for line in cron_lines] - minutes = [schedule[0] for schedule in schedules] + hours = [line.split()[3] for line in cron_lines] assert len(cron_lines) == 17 - assert all(schedule[1:] == ["*", "*", "*", "*"] for schedule in schedules) - assert len(minutes) == len(set(minutes)) + assert all(" * * *" in line and "* * * *" not in line for line in cron_lines) + assert len(hours) == len(set(hours)) def test_contract_workflow_tracks_the_product_caller() -> None: From 9eba818dfa6a110d16392baeab07754fe97e7c4e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 08:47:38 +0900 Subject: [PATCH 09/10] fix(tests): close the remaining if:-scalar bug and two stale hourly names The reverted commits left the changed-scope gate comparison's if:-condition stripping inlined with the same blank-line bug fixed upstream in #1812: the skip-state guard required line.strip() to be truthy before checking indentation, so a blank line inside a folded/literal `if:` scalar ended the skip early and leaked later indented lines into the "normalized" block. Extracted the corrected logic into `_strip_if_condition` (matching #1812) and added a regression test. Also renamed two tests whose names still said "hourly" even though their own assertions already check the approved daily-staggered cron values (test_central_repository_has_hourly_self_caller -> test_central_repository_has_daily_self_caller, test_review_fix_caller_runs_once_each_hour -> test_review_fix_caller_runs_once_each_day) -- no functional change. Verified full local triad: 2901 passed, 1 skipped; coverage 100%; interrogate 100%. Co-Authored-By: Claude Sonnet 5 --- tests/test_docs_only_pr_runner_admission.py | 63 +++++++++++++++---- tests/test_github_hourly_conflict_repair.py | 2 +- ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/tests/test_docs_only_pr_runner_admission.py b/tests/test_docs_only_pr_runner_admission.py index 0bd7d31d41..956e368016 100644 --- a/tests/test_docs_only_pr_runner_admission.py +++ b/tests/test_docs_only_pr_runner_admission.py @@ -85,24 +85,37 @@ 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.strip().startswith("if:"): + skip_indent = len(line) - len(line.lstrip(" ")) + 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_lines = [] - skip_indent = None - for line in block.splitlines(): - indent = len(line) - len(line.lstrip(" ")) - if skip_indent is not None and line.strip() and indent > skip_indent: - continue - skip_indent = None - if line.strip().startswith("if:"): - skip_indent = indent - continue - normalized_lines.append(line) - normalized_blocks.add("\n".join(normalized_lines)) + normalized_blocks.add(_strip_if_condition(block)) assert len(normalized_blocks) == 1, ( "changed-scope gate copies drifted; keep them byte-identical apart " "from the 'if:' condition (which may itself span multiple lines, e.g. " @@ -110,6 +123,32 @@ def test_gate_job_is_byte_identical_across_the_five_workflows_apart_from_if(): ) +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 = ( + " steps:\n" + " - name: example\n" + " if: >-\n" + " first line ||\n" + "\n" + " second line after a blank\n" + " runs-on: ubuntu-24.04\n" + ) + assert _strip_if_condition(block) == ( + " steps:\n" + " - name: example\n" + " runs-on: ubuntu-24.04" + ) + + def test_gate_job_and_codeql_scope_step_share_one_doc_pattern_line(): """The doc/image-only `case` line must be identical everywhere, and safe. diff --git a/tests/test_github_hourly_conflict_repair.py b/tests/test_github_hourly_conflict_repair.py index b515d49065..a0d9c7be06 100644 --- a/tests/test_github_hourly_conflict_repair.py +++ b/tests/test_github_hourly_conflict_repair.py @@ -115,7 +115,7 @@ def test_reusable_scheduler_enables_policy_for_hourly_callers() -> None: assert "--resolve-unreviewed-conflicts" in workflow -def test_central_repository_has_hourly_self_caller() -> None: +def test_central_repository_has_daily_self_caller() -> None: """The central repository itself is scanned instead of relying on product callers.""" workflow = _CALLER.read_text(encoding="utf-8") diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index b5dd153ff4..3befe2ef75 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -25,7 +25,7 @@ def _workflow_text(path: Path) -> str: return path.read_text(encoding="utf-8") -def test_review_fix_caller_runs_once_each_hour() -> None: +def test_review_fix_caller_runs_once_each_day() -> None: """Keep the actionable-review repair caller on the approved daily-staggered cadence.""" caller = _workflow_text(HOURLY_CALLER_WORKFLOW) assert 'cron: "23 7 * * *"' in caller From 7bc8506eb5735400707ba18ee1da9c90a465f699 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 09:12:25 +0900 Subject: [PATCH 10/10] test(ci): preserve exact review gate contracts Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae --- docs/product-technical-gap-baseline.md | 11 ++++------- tests/test_codeql_default_setup_rollout.py | 12 +++++++----- tests/test_docs_only_pr_runner_admission.py | 14 ++++++++------ 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index d1967deb04..02f31aa5fa 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/tests/test_codeql_default_setup_rollout.py b/tests/test_codeql_default_setup_rollout.py index c3e1c1cd3c..8610a39f49 100644 --- a/tests/test_codeql_default_setup_rollout.py +++ b/tests/test_codeql_default_setup_rollout.py @@ -427,19 +427,21 @@ def test_payload_file_and_cli_error_paths(tmp_path, monkeypatch, capsys): def test_live_cli_collects_one_snapshot(monkeypatch, capsys): fake_client = object() + calls = [] monkeypatch.setattr( rollout.GitHubClient, "from_environment", classmethod(lambda cls: fake_client), ) - monkeypatch.setattr( - rollout, - "collect_live_snapshot", - lambda client, repository, pr: snapshot(), - ) + 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 diff --git a/tests/test_docs_only_pr_runner_admission.py b/tests/test_docs_only_pr_runner_admission.py index 956e368016..3c155b4a15 100644 --- a/tests/test_docs_only_pr_runner_admission.py +++ b/tests/test_docs_only_pr_runner_admission.py @@ -102,8 +102,8 @@ def _strip_if_condition(block: str) -> str: if not line.strip() or indent > skip_indent: continue skip_indent = None - if line.strip().startswith("if:"): - skip_indent = len(line) - len(line.lstrip(" ")) + if line.startswith(" if:"): + skip_indent = 4 continue kept.append(line) return "\n".join(kept) @@ -134,17 +134,19 @@ def test_strip_if_condition_keeps_skipping_across_a_blank_continuation_line(): drifted. """ block = ( + " if: >-\n" + " first line ||\n" + "\n" + " second line after a blank\n" " steps:\n" " - name: example\n" - " if: >-\n" - " first line ||\n" - "\n" - " second line after a blank\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" )