From ff6d92d55e35c9932b40f4de2398e681d224dfcf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 23:20:41 +0000 Subject: [PATCH] fix(tests): repoint 3 hourly-review-repair test files at the daily-cadence redesign hourly-review-repair.yml was redesigned from a single hourly cron shared by 18 thin per-repository callers into one file with 17 distinct daily cron entries (one * * * per repository, staggered across UTC hours 0-16). Three test files were never updated to match and were failing on a clean origin/main checkout: - tests/test_hourly_review_repair_callers.py: _EXPECTED_TARGETS' 17 schedule keys and the schedule-list assertion still used the old hourly " * * * *" shape instead of the actual daily " * * *" cron strings (1 + 16 failing cases). - tests/test_github_hourly_conflict_repair.py::test_central_repository_has_hourly_self_caller asserted cron: "21 * * * *" instead of the central .github self-caller's actual cron: "21 6 * * *". - tests/test_pr_review_autofix_nvidia_nim_contract.py::test_review_fix_caller_runs_once_each_hour asserted a single hourly cron: "23 * * * *" that no longer exists; renamed to test_review_fix_caller_runs_once_each_day and repointed at Clearfolio's actual cron: "23 7 * * *", keeping the original protective intent (the caller still dispatches to the reusable, product-neutral pr-review-fix-scheduler.yml engine). Found while investigating a live required-check failure on ContextualWisdomLab/.github#1870: agent-review-runtime-quality-ci.yml's "Verify scheduler and contextual-orchestrator review-repair contracts" step runs pytest with no positional test path (full default tests/ discovery), so it was picking up these 19 stale assertions. With them fixed, the exact unscoped pytest command the workflow runs passes cleanly (100% branch coverage across pr_review_conflict_scope, pr_review_autofix_context, zdr_policy, and contextual_orchestrator_review_policy) -- no change to agent-review-runtime-quality-ci.yml itself was needed. Full suite (tests --ignore test_queue_cancellation_{open_pr_revalidation,revalidation}) went from 23 failed/2828 passed to 3 failed/2848 passed; the 3 remaining failures are pre-existing and unrelated (2 real test/workflow-drift bugs in other files, 1 local gh-CLI-missing sandbox limitation), confirmed identical before and after on a stash-based A/B. scripts/ci coverage (99%) and interrogate (98.3%) are unchanged from baseline, confirming this is a test-only change. Also updates CHANGELOG.md (first Unreleased section) and docs/product-technical-gap-baseline.md with a new dated entry recording the fix; the specific "opencode-review-dispatch.yml ... starved floating image" entry this was expected to append to was not found verbatim in the current file, so a new standalone entry was added instead. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX --- CHANGELOG.md | 1 + docs/product-technical-gap-baseline.md | 29 ++++++++++ tests/test_github_hourly_conflict_repair.py | 12 +++- tests/test_hourly_review_repair_callers.py | 57 ++++++++++++------- ...t_pr_review_autofix_nvidia_nim_contract.py | 25 +++++++- 5 files changed, 100 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2adc63c787..cff56e53db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- **Fix three test files left stale by the hourly-review-repair daily-cadence redesign, found while investigating a live required-check failure on `.github#1870`.** `hourly-review-repair.yml`'s consolidation into 17 distinct daily cron entries (one minute-hour per repository, staggered across UTC hours 0-16, rather than one minute shared by an hourly cron) was never reflected in `tests/test_hourly_review_repair_callers.py` (`_EXPECTED_TARGETS`' 17 schedule keys, and the schedule-list assertion, still used the old `" * * * *"` hourly shape), `tests/test_github_hourly_conflict_repair.py::test_central_repository_has_hourly_self_caller` (still asserted `cron: "21 * * * *"` instead of the central `.github` self-caller's actual `cron: "21 6 * * *"`), and `tests/test_pr_review_autofix_nvidia_nim_contract.py::test_review_fix_caller_runs_once_each_hour` (asserted a single hourly `cron: "23 * * * *"` that no longer exists; renamed to `test_review_fix_caller_runs_once_each_day` and repointed at Clearfolio's actual `cron: "23 7 * * *"` entry, keeping its original protective intent -- dispatch reaches the reusable, product-neutral `pr-review-fix-scheduler.yml` engine). `agent-review-runtime-quality-ci.yml`'s unscoped `pytest` coverage step (no positional test path, so it runs full default `tests/` discovery) was picking up these 19 stale-assertion failures on every PR; with the assertions corrected, that step's coverage gate (100% branch coverage across `pr_review_conflict_scope`, `pr_review_autofix_context`, `zdr_policy`, and `contextual_orchestrator_review_policy`) passes cleanly again with no workflow-level change needed. - Align current-main workflow contract tests with native auto-merge completion, validated dispatch concurrency keys, rotating queue pagination, globbed watch paths, admission jobs, and the reviewed OpenCode dispatch blob. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 6dbb08b3d3..e90e2dc3e7 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -3184,3 +3184,32 @@ inspecting a deterministic rotating window of 50, then stopping after the single doctoring doc's 2026-09-03 follow-up section for the full before/after and updated tests. A comment was left on `#1397` pointing at the replacement fix rather than closing it (closure is a merge-only action per this repo's governance model). + +## `hourly-review-repair.yml` daily-cadence redesign left three test files stale — 2026-09-04 + +**Status:** Root-caused and fixed. Note on provenance: this entry documents a stale-cron test bug found +while investigating a live required-check failure on `ContextualWisdomLab/.github#1870`; a prior search of +this document for an earlier same-day note describing it as a deferred follow-up (expected near an entry +about `opencode-review-dispatch.yml` and the starved floating runner image) did not find one in the current +file, so this is recorded as a new entry rather than an edit to existing prose. + +`agent-review-runtime-quality-ci.yml`'s "Verify scheduler and contextual-orchestrator review-repair +contracts" step runs `pytest` with no positional test path, so it falls back to full default `tests/` +discovery. That discovery was picking up 19 failing assertions (1 schedule-list + 16 parametrized in +`tests/test_hourly_review_repair_callers.py`, 1 in `tests/test_github_hourly_conflict_repair.py`, 1 in +`tests/test_pr_review_autofix_nvidia_nim_contract.py`) left over from `hourly-review-repair.yml`'s +consolidation into 17 distinct **daily** cron entries (one ` * * *` per repository, staggered +across UTC hours 0-16 — see `docs/doctoring/hourly-review-repair-single-file-consolidation.md` and +`docs/adr/0021-hourly-review-repair-single-file-consolidation.md`), which the three test files still checked +against the old hourly ` * * * *` shape. Fixed by updating `_EXPECTED_TARGETS`' 17 schedule keys and +the schedule-list assertion, repointing `test_central_repository_has_hourly_self_caller` at the central +`.github` self-caller's actual `cron: "21 6 * * *"`, and renaming/repointing +`test_review_fix_caller_runs_once_each_hour` to `test_review_fix_caller_runs_once_each_day` against +Clearfolio's actual `cron: "23 7 * * *"` — preserving each test's original protective intent (every distinct +schedule maps to the right repository exactly once; the caller still dispatches to the reusable, +product-neutral `pr-review-fix-scheduler.yml` engine) rather than weakening it. With the assertions +corrected, the exact unscoped `pytest` command the workflow runs passes cleanly (100% branch coverage across +`pr_review_conflict_scope`, `pr_review_autofix_context`, `zdr_policy`, and +`contextual_orchestrator_review_policy`; the only remaining local failures are two pre-existing, unrelated +test/workflow-drift bugs and one `gh`-CLI-missing sandbox limitation, none introduced or resolved by this +change) — so `agent-review-runtime-quality-ci.yml` itself needed no workflow-level change. diff --git a/tests/test_github_hourly_conflict_repair.py b/tests/test_github_hourly_conflict_repair.py index 5e9b483651..df6179b1d4 100644 --- a/tests/test_github_hourly_conflict_repair.py +++ b/tests/test_github_hourly_conflict_repair.py @@ -116,10 +116,18 @@ def test_reusable_scheduler_enables_policy_for_hourly_callers() -> None: def test_central_repository_has_hourly_self_caller() -> None: - """The central repository itself is scanned instead of relying on product callers.""" + """The central repository itself is scanned instead of relying on product callers. + + ``hourly-review-repair.yml`` was redesigned from a single hourly cron + shared by 18 thin per-repository callers into one file with 17 distinct + *daily* cron entries, each pinned to a distinct UTC hour (see + docs/adr/0021-hourly-review-repair-single-file-consolidation.md and the + workflow's own top-of-file comment). The central `.github` self-caller's + entry kept its original minute (21) and was assigned hour 6. + """ 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..08b233bfb2 100644 --- a/tests/test_hourly_review_repair_callers.py +++ b/tests/test_hourly_review_repair_callers.py @@ -49,8 +49,19 @@ # was raised to "200" here -- see test_max_prs_and_max_dispatches_stay_uniform_static_values) # and both are asserted separately as static `with:` values rather than # carried per-target. +# +# Each schedule key is a full 5-field daily cron string (minute, then a +# distinct hour 0-16), not the hourly `" * * * *"` shape the original +# 18 per-repository files used. The redesign (see +# docs/doctoring/hourly-review-repair-single-file-consolidation.md and +# docs/adr/0021-hourly-review-repair-single-file-consolidation.md) turned the +# single shared hourly cron into 17 distinct once-per-day schedules, +# staggered across UTC hours 0-16, so this control plane admits at most one +# recovery workflow per hour instead of seventeen every hour. Each +# repository's original minute is preserved verbatim as the new cron's +# minute field; only the hour field is new. _EXPECTED_TARGETS: dict[str, list[dict[str, str]]] = { - "2 * * * *": [ + "2 0 * * *": [ { "name": "afipc", "target_repository": "ContextualWisdomLab/aFIPC", @@ -59,7 +70,7 @@ "concurrency_group": "afipc-hourly-review-repair", }, ], - "4 * * * *": [ + "4 1 * * *": [ { "name": "lineageweave", "target_repository": "ContextualWisdomLab/LineageWeave", @@ -68,7 +79,7 @@ "concurrency_group": "lineageweave-hourly-review-repair", }, ], - "9 * * * *": [ + "9 2 * * *": [ { "name": "psychometrics-commons", "target_repository": "ContextualWisdomLab/psychometrics-commons", @@ -77,7 +88,7 @@ "concurrency_group": "psychometrics-commons-hourly-review-repair", }, ], - "10 * * * *": [ + "10 3 * * *": [ { "name": "originweave", "target_repository": "ContextualWisdomLab/OriginWeave", @@ -86,7 +97,7 @@ "concurrency_group": "originweave-hourly-review-repair", }, ], - "14 * * * *": [ + "14 4 * * *": [ { "name": "quarantine-sandbox", "target_repository": "ContextualWisdomLab/quarantine-sandbox-runtime", @@ -95,7 +106,7 @@ "concurrency_group": "quarantine-sandbox-hourly-review-repair", }, ], - "16 * * * *": [ + "16 5 * * *": [ { "name": "nonnest2", "target_repository": "ContextualWisdomLab/nonnest2", @@ -104,7 +115,7 @@ "concurrency_group": "nonnest2-hourly-review-repair", }, ], - "21 * * * *": [ + "21 6 * * *": [ { "name": "github", "target_repository": "ContextualWisdomLab/.github", @@ -113,7 +124,7 @@ "concurrency_group": "github-hourly-review-repair", }, ], - "23 * * * *": [ + "23 7 * * *": [ { "name": "clearfolio", "target_repository": "ContextualWisdomLab/clearfolio", @@ -122,7 +133,7 @@ "concurrency_group": "clearfolio-hourly-review-repair", }, ], - "27 * * * *": [ + "27 8 * * *": [ { "name": "accounting-information-platform", "target_repository": "ContextualWisdomLab/accounting-information-platform", @@ -131,7 +142,7 @@ "concurrency_group": "accounting-information-platform-hourly-review-repair", }, ], - "34 * * * *": [ + "34 9 * * *": [ { "name": "contextual-orchestrator", "target_repository": "ContextualWisdomLab/contextual-orchestrator", @@ -140,7 +151,7 @@ "concurrency_group": "contextual-orchestrator-hourly-review-repair", }, ], - "37 * * * *": [ + "37 10 * * *": [ { "name": "disksage", "target_repository": "ContextualWisdomLab/disksage", @@ -149,7 +160,7 @@ "concurrency_group": "disksage-hourly-review-repair", }, ], - "43 * * * *": [ + "43 11 * * *": [ { "name": "governance-risk-compliance", "target_repository": "ContextualWisdomLab/governance-risk-compliance", @@ -162,9 +173,9 @@ # independent files (fast-mlsirm, metering-billing-platform) had each # chosen minute 49 without knowing about the other. The consolidated # lookup makes that sharing explicit and still dispatches each - # repository exactly once per hour, via the matrix in + # repository exactly once per day (hour 12 UTC), via the matrix in # dispatch-review-repair. - "49 * * * *": [ + "49 12 * * *": [ { "name": "fast-mlsirm", "target_repository": "ContextualWisdomLab/fast-mlsirm", @@ -180,7 +191,7 @@ "concurrency_group": "metering-billing-platform-hourly-review-repair", }, ], - "53 * * * *": [ + "53 13 * * *": [ { "name": "bandscope", "target_repository": "ContextualWisdomLab/bandscope", @@ -189,7 +200,7 @@ "concurrency_group": "bandscope-hourly-review-repair", }, ], - "56 * * * *": [ + "56 14 * * *": [ { "name": "inkspan", "target_repository": "ContextualWisdomLab/inkspan", @@ -198,7 +209,7 @@ "concurrency_group": "inkspan-hourly-review-repair", }, ], - "58 * * * *": [ + "58 15 * * *": [ { "name": "orgmetra", "target_repository": "ContextualWisdomLab/Orgmetra", @@ -207,7 +218,7 @@ "concurrency_group": "orgmetra-hourly-review-repair", }, ], - "59 * * * *": [ + "59 16 * * *": [ { "name": "semantic-data-portal", "target_repository": "ContextualWisdomLab/semantic-data-portal", @@ -277,7 +288,15 @@ def test_all_eighteen_former_callers_are_deleted() -> None: def test_schedule_list_has_every_distinct_minute_exactly_once() -> None: - """The 17 distinct minutes (49 is intentionally shared) each appear once.""" + """The 17 distinct daily cron entries (minute 49 is intentionally shared + by two repositories at hour 12) each appear exactly once. + + Each entry is a full `` * * *`` daily schedule, not an + hourly `` * * * *`` one: the redesign spreads the 17 (18 + repository) recoveries across distinct UTC hours 0-16 so this control + plane admits at most one recovery workflow per hour instead of + seventeen every hour. + """ text = _read(_CALLER) cron_lines = re.findall(r'- cron: "([^"]+)"', text) diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 5366ce5de5..388648321f 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -25,12 +25,31 @@ def _workflow_text(path: Path) -> str: return path.read_text(encoding="utf-8") -def test_review_fix_caller_runs_once_each_hour() -> None: - """Keep the actionable-review repair caller on the approved hourly cadence.""" +def test_review_fix_caller_runs_once_each_day() -> None: + """Keep the actionable-review repair caller on the approved daily cadence. + + ``hourly-review-repair.yml`` (its `name:` is now "Daily Review Recovery") + was redesigned from a single cron shared by up to 18 thin per-repository + callers, firing every hour, into one file with 17 distinct daily cron + entries staggered across UTC hours -- see + docs/adr/0021-hourly-review-repair-single-file-consolidation.md. Each + resolved target, including this file's representative example + (Clearfolio, minute 23, hour 7), now runs once per day rather than once + per hour. The protected invariant this test originally guarded -- + dispatch reaches the reusable, product-neutral `pr-review-fix-scheduler.yml` + engine rather than reimplementing review-fix logic in the caller -- is + unchanged and reasserted here. + """ caller = _workflow_text(HOURLY_CALLER_WORKFLOW) - assert 'cron: "23 * * * *"' in caller + assert 'cron: "23 7 * * *"' in caller + assert 'cron: "23 * * * *"' not in caller assert 'cron: "23 */2 * * *"' not in caller assert "uses: ./.github/workflows/pr-review-fix-scheduler.yml" in caller + # Product-hourly-caller-neutral per CLAUDE.md/AGENTS.md: no target + # repository dispatched by the caller is hard-coded into the reusable + # engine itself. + reusable = _workflow_text(FIX_SCHEDULER_WORKFLOW) + assert "ContextualWisdomLab/clearfolio" not in reusable def test_scheduled_autofix_routes_through_contextual_orchestrator() -> None: