From 1ee3051d90e35cbe79cc6c0cb9d6972be17be879 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 07:10:02 +0900 Subject: [PATCH 1/2] fix(tests): repair 23 stale assertions left by the admission-controller burst Reproduced fresh on unmodified main before fixing: 23 tests failing across 6 files, all pre-existing tests broken by the same-day admission-controller feature work (#1859-#1869), none caused by a production regression. - tests/test_hourly_review_repair_callers.py (19 parametrized cases): #1860 ("reduce scheduled recovery pressure") consolidated hourly-review-repair.yml's 17 per-repo callers from truly-hourly ("M * * * *") to daily-staggered ("M H * * *") cron expressions, same 17 minutes, one new hour component each. Updated the test's _EXPECTED_TARGETS oracle keys to match; the target-repo mapping values are unchanged. - tests/test_github_hourly_conflict_repair.py, tests/test_pr_review_autofix_nvidia_nim_contract.py: same #1860 cadence change, one stale cron literal each (central .github self-caller at minute 21, Clearfolio at minute 23). - tests/test_noema_orchestrator_workflow_contract.py: noema-review.yml's "closed pull request" cleanup step and its CLOSED_PR_NUMBER env var were renamed to "inactive pull request" / INACTIVE_PR_NUMBER (broadened to also cover converted-to-draft, alongside two new env vars the step now needs -- INACTIVE_PR_HEAD_SHA, PR_ACTION). Updated the test's step-name lookup, env dict, and fake-gh fixture to supply a live PR response for the new live_target_matches() re-check. - tests/test_docs_only_pr_runner_admission.py: not a real drift -- the test's changed-scope byte-identity check only stripped a single-line `if:` condition. strix.yml legitimately needs a two-line folded `if: >-` condition (also skipping converted_to_draft), which made its copy compare unequal to the other two workflows' single-line conditions even though the underlying job body is identical. Fixed the test's own normalization to strip an `if:` condition's continuation lines too, regardless of how many source lines it spans. - tests/test_strix_rerun_job_selection.py: #1862/#1863 added a live_dispatch_head_matches() freshness re-check before any Strix rerun, called via a real `gh` lookup this unit test does not exercise. Stubbed it to the happy path so the test stays focused on its actual subject (job selection: the "strix" scan job, never its "publish-manual-pr-evidence-status" sibling) rather than the separately covered freshness gate. Verified full local triad: coverage run -m pytest tests -> 2851 passed, 1 skipped, 21 subtests passed. Coverage/docstring gates are a separate, larger gap left by the same admission-controller work (review_admission_controller.py at 85%/39%, pr_review_merge_scheduler_core.py and audit_codeql_default_setup_rollout.py also short of 100%) -- tracked separately, not fixed in this PR. Co-Authored-By: Claude Sonnet 5 --- tests/test_docs_only_pr_runner_admission.py | 29 +++++++++++++--- tests/test_github_hourly_conflict_repair.py | 2 +- tests/test_hourly_review_repair_callers.py | 34 +++++++++---------- ...st_noema_orchestrator_workflow_contract.py | 9 +++-- ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- tests/test_strix_rerun_job_selection.py | 6 ++++ 6 files changed, 57 insertions(+), 25 deletions(-) diff --git a/tests/test_docs_only_pr_runner_admission.py b/tests/test_docs_only_pr_runner_admission.py index 8b2e6e8ee2..ae6318c7b8 100644 --- a/tests/test_docs_only_pr_runner_admission.py +++ b/tests/test_docs_only_pr_runner_admission.py @@ -85,19 +85,40 @@ 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(): + 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 + kept.append(line) + return "\n".join(kept) + + def test_gate_job_is_byte_identical_across_the_five_workflows_apart_from_if(): """The `changed-scope` block must not drift between its five copies.""" normalized_blocks = set() for filename in GATE_WORKFLOWS: workflow = _read(filename) block = _top_level_job_block(workflow, "changed-scope") - normalized = "\n".join( - line for line in block.splitlines() if not line.strip().startswith("if:") - ) + normalized = _strip_if_condition(block) normalized_blocks.add(normalized) assert len(normalized_blocks) == 1, ( "changed-scope gate copies drifted; keep them byte-identical apart " - "from the single 'if:' line" + "from the 'if:' condition" ) 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 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..a1fd12fa67 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" @@ -100,6 +100,9 @@ def test_noema_close_cleanup_selects_only_the_closed_pr_across_shared_display_ti status="$(printf '%s' "$url" | sed -E 's/.*status=([a-z_]+)&.*/\\1/')" jq --arg status "$status" '{workflow_runs: [.workflow_runs[] | select(.status == $status)]}' \\ "$FAKE_RUNS_FILE" +elif [[ "$*" == *"/pulls/"* ]]; then + printf '%s\n' "$*" >>"$FAKE_CALLS_FILE" + printf '{"state": "closed", "draft": false, "head": {"sha": "%s"}}\n' "$(printf 'a%.0s' {1..40})" else printf '%s\n' "$*" >>"$FAKE_CALLS_FILE" fi @@ -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..65beed77f0 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -28,7 +28,7 @@ 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.""" 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..83fa62ba91 100644 --- a/tests/test_strix_rerun_job_selection.py +++ b/tests/test_strix_rerun_job_selection.py @@ -38,6 +38,12 @@ 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) + # This test's own concern is job selection (the "strix" scan job, not its + # "publish-manual-pr-evidence-status" sibling) -- not the separate live + # head-freshness re-check `dispatch_strix_evidence` now performs before + # any rerun, which needs a real `gh` call and has its own dedicated + # coverage. Stub it to the happy path so this test stays focused. + monkeypatch.setattr(sched, "live_dispatch_head_matches", lambda repo, pr: True) assert ( sched.dispatch_strix_evidence( From 8887c2bee036ba64cb9b5a37c163f69d02754e16 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 07:16:17 +0900 Subject: [PATCH 2/2] docs(ci): add missing docstrings on audit_codeql_default_setup_rollout's CLI entry points parse_args/main lacked docstrings, failing the repo's 100% interrogate gate (98.3% actual). No behavior change. Co-Authored-By: Claude Sonnet 5 --- scripts/ci/audit_codeql_default_setup_rollout.py | 2 ++ 1 file changed, 2 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