diff --git a/CHANGELOG.md b/CHANGELOG.md index 66145dc939..c7b0d0cfac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,23 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Cache `active_workflow_runs` for the life of one `pr_review_merge_scheduler.py` + invocation.** `inspect_pr()` calls `cancel_stale_pr_runs()` unconditionally for + every non-draft PR before any eligibility gate, and several other call sites + (`active_review_run_refs`, `dispatch_strix_evidence`'s busy check) ask the + identical unfiltered `(repo, ("queued", "in_progress"))` question again -- + all against the one repository a scheduler invocation ever targets, with zero + caching anywhere in the file. At the default `MAX_PRS=100` this reissued the + same repository-wide, paginated `gh api .../actions/runs` fetch well over a + hundred times per run. `active_workflow_runs` now memoizes its result keyed on + the full `(repo, statuses, event, created, head_sha)` call shape for one + `main()` invocation, with explicit cache invalidation immediately after the + four places that mutate GitHub Actions run state + (`force_cancel_workflow_runs`, `rerun_actions_job`, `dispatch_opencode_review`, + `dispatch_strix_evidence`) so a later read in the same run can never replay a + pre-mutation snapshot. The four pre-existing `ThreadPoolExecutor` sites and the + correctly-sequential per-PR mutation-budget loop are untouched. See + ADR-0022. - **Consolidate the 18 per-repository hourly review-repair caller workflows into one file.** At the repository owner's request ("이런 Workflow는 단일 파일로 통합하라"), replaced `accounting-information-platform-`, `afipc-`, `bandscope-`, `clearfolio-`, diff --git a/docs/adr/0022-scheduler-active-workflow-runs-cache.md b/docs/adr/0022-scheduler-active-workflow-runs-cache.md new file mode 100644 index 0000000000..ca7e36946c --- /dev/null +++ b/docs/adr/0022-scheduler-active-workflow-runs-cache.md @@ -0,0 +1,174 @@ +# ADR-0022: Cache `active_workflow_runs` per scheduler invocation; stay Python + +- **Status:** Accepted +- **Date:** 2026-09-02 +- **Scope:** ContextualWisdomLab/.github `scripts/ci/pr_review_merge_scheduler.py` + (the `scan-pr-queue` job's PR-queue sweep) + +## Context + +`pr_review_merge_scheduler.py` is 5,428 lines and is invoked by `scan-pr-queue` +with `--max-prs "$MAX_PRS"` (workflow_call default `"100"`; +`.github/workflows/pr-review-merge-scheduler.yml`). Its call path is +`main()` → `fetch_open_prs()` (paginated GraphQL, one repository only -- +`fetch_open_prs(repo, max_prs)` takes a single `repo` string, never a set) → +`enrich_rest_mergeable_states()` (already a bounded `ThreadPoolExecutor`) → +a sequential `for pr in prs: inspect_pr(pr)`. That final loop is correctly +sequential by design, not a naive-parallelize target: `inspect_pr` consumes +stateful, order-dependent mutation-budget counters +(`review_dispatch_limit`/`branch_update_limit`, default `1`) that must be +spent in PR order across the whole sweep. + +`concurrent.futures.ThreadPoolExecutor` already exists at four sites -- +`fetch_open_prs_rest` (REST PR-list enrichment), `enrich_rest_mergeable_states` +(per-PR mergeable-state/compare-freshness enrichment), +`resolve_outdated_review_threads` (outdated-thread resolution), and +`force_cancel_workflow_runs` (batched run cancellation) -- so the "naive +sequential loop of independent reads" pattern this investigation went looking +for is already fixed everywhere it occurs for bulk reads. + +The real remaining inefficiency is different in kind: `inspect_pr()` calls +`cancel_stale_pr_runs(repo, pr, dry_run=dry_run)` **unconditionally** for +every non-draft PR, before any eligibility or budget gate. Non-dry-run, that +calls `active_workflow_runs(repo, ("queued", "in_progress"))` -- two +sequential, repository-wide, paginated `gh api repos/{repo}/actions/runs +--paginate --slurp` calls, unfiltered by PR and filtered client-side +afterward. Because the scheduler only ever targets the one repository passed +on its command line, this exact fetch is reissued from scratch for every PR +in the loop, and several other call sites (`active_review_run_refs`, +`dispatch_strix_evidence`'s busy check) ask the identical unfiltered question +again within the same invocation. There was no caching anywhere in the file +(`functools`/`lru_cache` was not even imported). Worst case at the default +`MAX_PRS=100` with mostly non-draft PRs: well over a hundred redundant +sequential `gh api` round-trips per scheduler invocation, each potentially +multi-page, for data that does not change unless the scheduler's own actions +change it. + +No prior ADR discusses this file's language choice (a repository-wide grep +across `docs/adr/*.md` and `docs/*.md` for the scheduler, scheduler +performance, GIL, or Python/Rust turned up nothing). `scripts/ci/` is 50 +files / 27,115 lines, 100% Python, with zero `.rs` files or `Cargo.toml` +anywhere in the repository -- Python-for-CI-glue is this repository's +existing, uniform convention. +`docs/product-technical-gap-baseline.md` §2.2 (Compute plane) scopes +mandatory Rust to math-science/psychometrics computation and CPU-bound hot +paths, and explicitly permits Python/JS for "orchestration/API adapter" +roles -- exactly what this scheduler is: `gh` CLI / GraphQL+REST glue with no +CPU-bound core. `docs/product-goal-directive.md` §6 separately carries a +narrower, already-authorized escape hatch for the concern this investigation +was chartered to check: if a Python web server hits GIL problems, support +multithreading or move to Python 3.14 -- not "rewrite in Rust." The measured +bottleneck here is redundant sequential I/O wait, not CPU/GIL-bound +computation; CPython threads already release the GIL during subprocess and +network I/O, so a Rust rewrite would not remove these round-trips -- only +avoiding the redundant reads does. + +## Decision + +1. **Cache, not a thread pool, for this hot path.** `active_workflow_runs` + now memoizes its result in a module-level dict keyed on the full call + shape `(repo, tuple(statuses), event, created, head_sha)`. This is a + caching fix in the same spirit as "stop repeating a blocking call that + could be done once" -- and is strictly better than thread-pooling the + redundant calls would have been, since caching also cuts GitHub API + rate-limit consumption instead of only wall clock. +2. **Cache lifetime is exactly one scheduler invocation.** + `reset_active_workflow_runs_cache()` clears the dict; `main()` calls it + once at the top of every run, so no state survives across separate + invocations sharing a process (relevant to tests, and to any future + long-lived caller). +3. **Explicit invalidation on every mutation, not a blind full-invocation + cache.** A blind cache is unsafe here: `dispatch_strix_evidence`'s + `busy_refs` check reads `active_workflow_runs` again immediately after + `force_cancel_workflow_run_refs` cancels stale runs for the same + repository, and a later PR's own `cancel_stale_pr_runs` can run after an + earlier PR's dispatch created a new run in the same repository within the + same invocation. Serving a pre-mutation snapshot to either of those reads + would let a just-cancelled run still look "busy," or let a same-invocation + dispatch go undetected by the repository-wide single-concurrency dispatch + guard. `reset_active_workflow_runs_cache()` is therefore called + immediately after the four places that change GitHub Actions run state: + `force_cancel_workflow_runs` (after a cancel), `rerun_actions_job` (after + a rerun), and `dispatch_opencode_review` / `dispatch_strix_evidence` + (after their dispatch `POST`) -- the complete set found by grepping for + every `force-cancel`, `/rerun`, and `/dispatches` call in the file. +4. **The four existing `ThreadPoolExecutor` sites and the sequential per-PR + mutation-budget loop are untouched.** They already convert independent, + read-only bulk lookups to bounded concurrency where that was safe; nothing + with ordering dependencies (merges, branch updates, review dispatches) was + touched, per this organization's standing rule against parallelizing + anything with side effects or ordering dependencies without strong + evidence. +5. **No Rust rewrite.** Per the gap-baseline and goal-directive citations in + Context above: this script's role and evidence do not meet the bar either + document sets for mandatory or motivated Rust. + +## Consequences + +- In the common case -- most PRs carry no stale old-head runs, so + `force_cancel_workflow_runs` is never called with a non-empty `run_ids` and + never invalidates -- the redundant unfiltered `(repo, ("queued", + "in_progress"))` fetches collapse from up to two per PR to two total for + the whole sweep, matching the investigation's own estimate. +- In the pathological case -- every single PR has a stale run to cancel, so + every iteration invalidates -- the cache provides no savings, but also no + regression: behavior degrades gracefully back to exactly today's + call-per-PR pattern, never worse. +- `tests/test_pr_review_merge_scheduler.py`: two existing call-index + assertions (`test_actions_call_gh_with_expected_arguments`, + `test_actions_control_uses_workflow_token_when_mutation_token_is_app`) + shifted because a busy-check read that used to issue two fresh `gh api` + calls is now a cache hit, and were updated (with an inline comment + explaining the shift) rather than the underlying call counts contorted to + preserve the old indices. Four new tests were added: + `test_active_workflow_runs_caches_repeated_identical_calls` (identical + results, one underlying fetch for many repeated calls), + `test_active_workflow_runs_cache_is_faster_than_repeated_fetches` (a + `time.sleep`-delayed fake `gh` proves a genuine wall-clock improvement, not + just fewer assertions), `test_active_workflow_runs_cache_keys_on_full_call_shape` + (distinct repo/statuses/event/created/head_sha combinations never share an + entry), and + `test_force_cancel_workflow_runs_invalidates_active_workflow_runs_cache` + (a cancellation is never masked by a stale pre-cancellation snapshot). A + new autouse fixture clears the cache between every test so the new + module-global state cannot leak across the file's ~250 other tests. +- `coverage run -m pytest tests && coverage report` remains 100% on + `scripts/ci` (`pr_review_merge_scheduler.py`: 2,208 statements / 940 + branches, zero missed); `interrogate` remains 100%. + +## Rejected alternatives + +- **A blind, never-invalidated full-invocation cache.** Rejected as unsafe: + it would let `dispatch_strix_evidence`'s busy check believe a run this same + invocation just cancelled is still occupying the repository's dispatch + capacity, or let one PR's dispatch go invisible to a later PR's read in the + same repository within the same run -- silently breaking the + "repository busy" single-concurrency dispatch guard the code depends on. +- **`functools.lru_cache` decorating `active_workflow_runs` directly.** + Rejected: `lru_cache` hashes its raw arguments before the function body + runs, so a caller passing `statuses` as a list (the parameter's declared + type is `Sequence[str]`, not specifically `tuple`) would raise + `TypeError: unhashable type` where today's implementation tolerates any + iterable. The manual cache normalizes to `tuple(statuses)` for the key + while still iterating the caller's original argument for the actual `gh` + calls. +- **Converting the unconditional `cancel_stale_pr_runs` call, or the per-PR + loop generally, into a `ThreadPoolExecutor` read-parallelization.** + Rejected: the loop is correctly sequential (the mutation-budget counters + must be consumed in PR order), and the actual inefficiency is a *duplicate* + read of identical data across iterations, not independent reads that could + usefully run concurrently. Caching is strictly better for this specific + shape of waste. +- **Rewrite this scheduler, or just its GitHub-API layer, in Rust.** + Rejected under `docs/product-technical-gap-baseline.md` §2.2's scoping + (mandatory Rust is reserved for CPU-bound math-science/psychometrics + compute; Python/JS is explicitly permitted for orchestration/API-adapter + roles) and `docs/product-goal-directive.md` §6's narrower, already-adopted + GIL escape hatch (multithreading or Python 3.14, not a rewrite). The + measured bottleneck is network I/O wait, which CPython already handles by + releasing the GIL during subprocess/socket calls; a Rust rewrite would not + remove the round-trips themselves, only the caching fix does. If a future + profile shows a genuinely CPU-bound hot path inside this file (none is + evidenced today), the removal/migration condition for revisiting this + decision is: a profiler-attributed CPU-bound function, not I/O-bound `gh` + invocation latency, consuming a measurable share of scheduler wall clock. diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 8c640b0b2d..8829652dbf 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2677,6 +2677,30 @@ def rerun_actions_job(repo: str, job_id: str, *, dry_run: bool, action: str) -> return require_github_actions_control_actor(action) run_github_actions(["gh", "api", "-X", "POST", f"repos/{repo}/actions/jobs/{job_id}/rerun"]) + # A rerun brings a completed run back to queued/in_progress; invalidate + # any cached active_workflow_runs snapshot so it is not read as stale. + reset_active_workflow_runs_cache() + + +_active_workflow_runs_cache: dict[ + tuple[str, tuple[str, ...], str | None, str | None, str | None], list[dict[str, Any]] +] = {} + + +def reset_active_workflow_runs_cache() -> None: + """Clear the per-invocation cache backing :func:`active_workflow_runs`. + + ``main`` calls this once at the top of every scheduler run so the cache + never survives across separate invocations sharing a process (tests + calling ``main`` more than once, most notably). It must also be called + immediately after anything that changes GitHub Actions run state -- + force-cancelling, rerunning, or dispatching a run -- so a later read in + the same run observes that mutation instead of a stale pre-mutation + snapshot; :func:`force_cancel_workflow_runs`, :func:`rerun_actions_job`, + :func:`dispatch_opencode_review`, and :func:`dispatch_strix_evidence` all + do this immediately after their mutating call. + """ + _active_workflow_runs_cache.clear() def active_workflow_runs( @@ -2699,7 +2723,20 @@ def active_workflow_runs( run history only grows, such as a same-head dispatch search, or one scoped to a single known commit -- should pass them to avoid paginating history it can never use. + + Results are memoized per exact ``(repo, statuses, event, created, + head_sha)`` combination for the life of the cache (cleared by + :func:`reset_active_workflow_runs_cache`). The scheduler's queue sweep + calls the unfiltered ``(repo, ("queued", "in_progress"))`` shape from + every non-draft PR's unconditional stale-run check plus every review + dispatch check, all against the one repository a scheduler invocation + ever targets -- without memoization that is up to two redundant, + repository-wide, paginated REST calls per PR for identical data. """ + cache_key = (repo, tuple(statuses), event, created, head_sha) + cached = _active_workflow_runs_cache.get(cache_key) + if cached is not None: + return list(cached) runs: list[dict[str, Any]] = [] for status in statuses: args = [ @@ -2725,7 +2762,8 @@ def active_workflow_runs( pages = payload if isinstance(payload, list) else [payload] for page in pages: runs.extend(page.get("workflow_runs") or []) - return runs + _active_workflow_runs_cache[cache_key] = runs + return list(runs) def workflow_run_mentions_pr(run_data: dict[str, Any], pr_number: int) -> bool: @@ -2956,6 +2994,12 @@ def cancel_one(run_id: str) -> tuple[str, str | None]: with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: results = list(executor.map(cancel_one, (str(run_id) for run_id in run_ids))) + # A cancelled run is no longer queued/in_progress; drop any cached + # active_workflow_runs snapshot so the next read (this same PR's later + # checks, or a later PR sharing this repository) sees the change instead + # of replaying it from before the cancellation. + reset_active_workflow_runs_cache() + failures = {run_id: reason for run_id, reason in results if reason is not None} for run_id, reason in failures.items(): print( @@ -3103,6 +3147,9 @@ def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dr } ), ) + # A dispatch queues a new run; invalidate any cached active_workflow_runs + # snapshot so a later busy/current-run check in this same invocation sees it. + reset_active_workflow_runs_cache() return "dispatched" @@ -3184,6 +3231,9 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry } ), ) + # A dispatch queues a new run; invalidate any cached active_workflow_runs + # snapshot so a later busy/current-run check in this same invocation sees it. + reset_active_workflow_runs_cache() return "dispatched" @@ -5335,6 +5385,10 @@ def parse_args(argv: list[str]) -> argparse.Namespace: def main(argv: list[str]) -> int: """Run the scheduler CLI.""" + # Each invocation is a fresh look at GitHub; never reuse another + # invocation's active_workflow_runs cache (relevant when a process + # calls main() more than once, tests included). + reset_active_workflow_runs_cache() args = parse_args(argv) if args.self_test: self_test() diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index d859b1730d..20f949353d 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1,6 +1,7 @@ import json import os import sys +import time from datetime import datetime, timezone import pytest @@ -35,6 +36,19 @@ def workflow_starting_mutation_credential(monkeypatch): monkeypatch.setenv("SCHEDULER_MUTATION_TOKEN_SOURCE", "PR_REVIEW_MERGE_TOKEN") +@pytest.fixture(autouse=True) +def reset_active_workflow_runs_cache(): + """Isolate ``active_workflow_runs``'s cache so tests never see a sibling's data. + + Different tests reuse the same ``owner/repo`` cache key with different + fake GitHub responses; without this the module-global cache from one test + would leak into the next. + """ + sched.reset_active_workflow_runs_cache() + yield + sched.reset_active_workflow_runs_cache() + + def fake_github_token(prefix, body): return f"{prefix}{TOKEN_SEPARATOR}{body}" @@ -4160,7 +4174,11 @@ def fake_run(args, stdin=None): assert calls[3][-1] == f"expected_head_sha={head_sha}" assert calls[4][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] assert calls[5][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] - assert calls[8] == [ + # dispatch_strix_evidence's busy_refs check re-reads the exact same + # (repo, ("queued", "in_progress")) shape calls[4:6] already fetched; + # active_workflow_runs's per-invocation cache serves it without a + # third/fourth GET, so its dispatch POST lands right after calls[4:6]. + assert calls[6] == [ "gh", "api", "-X", @@ -4169,17 +4187,20 @@ def fake_run(args, stdin=None): "--input", "-", ] - assert calls[9][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] - assert calls[10][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] - # calls[11:14]: the bounded discover_opencode_required_run_id fallback + # That dispatch invalidates the cache (it just queued a new run), so + # dispatch_opencode_review's own active_opencode_run_refs check below + # re-fetches fresh instead of reusing calls[4:6]'s now-stale snapshot. + assert calls[7][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] + assert calls[8][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] + # calls[9:12]: the bounded discover_opencode_required_run_id fallback # (matching_actions_run_id found nothing in this PR's empty rollup). for offset, status in enumerate(("queued", "in_progress", "completed")): - discover_call = calls[11 + offset] + discover_call = calls[9 + offset] assert discover_call[:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] assert f"status={status}" in discover_call assert "event=pull_request_target" in discover_call assert f"head_sha={head_sha}" in discover_call - assert calls[14] == [ + assert calls[12] == [ "gh", "api", "-X", @@ -4423,7 +4444,11 @@ def fake_run_with_env(args, *, stdin=None, env=None): assert calls[0][0] == ["gh", "api", "-X", "POST", "repos/owner/repo/actions/jobs/101/rerun"] assert calls[1][0][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] assert calls[2][0][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] - assert calls[5][0] == [ + # dispatch_strix_evidence's busy_refs check re-reads the exact same + # (repo, ("queued", "in_progress")) shape calls[1:3] already fetched; + # active_workflow_runs's per-invocation cache serves it without a + # third/fourth GET, so its dispatch POST lands right after calls[1:3]. + assert calls[3][0] == [ "gh", "api", "-X", @@ -4432,19 +4457,22 @@ def fake_run_with_env(args, *, stdin=None, env=None): "--input", "-", ] - assert calls[6][0][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] - assert calls[7][0][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] - # calls[8:11]: the bounded discover_opencode_required_run_id fallback + # That dispatch invalidates the cache (it just queued a new run), so + # dispatch_opencode_review's own active_opencode_run_refs check below + # re-fetches fresh instead of reusing calls[1:3]'s now-stale snapshot. + assert calls[4][0][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] + assert calls[5][0][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] + # calls[6:9]: the bounded discover_opencode_required_run_id fallback # (matching_actions_run_id found nothing in the empty rollup), scoped to # the exact head SHA across the three statuses that can hold the # required run. for offset, status in enumerate(("queued", "in_progress", "completed")): - discover_call = calls[8 + offset][0] + discover_call = calls[6 + offset][0] assert discover_call[:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] assert f"status={status}" in discover_call assert "event=pull_request_target" in discover_call assert f"head_sha={'a' * 40}" in discover_call - assert calls[11][0] == [ + assert calls[9][0] == [ "gh", "api", "-X", @@ -5095,6 +5123,121 @@ def fake_run(args, stdin=None): assert not any(str(arg).startswith("created=") for arg in args) +def test_active_workflow_runs_caches_repeated_identical_calls(monkeypatch): + """A repeated identical call is served from cache with the identical result. + + This is the scan-pr-queue win: every non-draft PR unconditionally asks + for the same (repo, ("queued", "in_progress")) shape via + ``cancel_stale_pr_runs``, and review dispatch re-asks the same shape + again -- all against the one repository a scheduler invocation ever + targets. Only the first call should reach the (faked) GitHub API; every + later call with the same arguments must return the same data without a + new call. + """ + calls = [] + + def fake_run(args, stdin=None): + del stdin + calls.append(args) + return json.dumps([{"workflow_runs": [{"id": 1}, {"id": 2}]}]) + + monkeypatch.setattr(sched, "run_github_actions", fake_run) + + first = sched.active_workflow_runs("owner/repo", ("queued", "in_progress")) + for _ in range(50): + repeated = sched.active_workflow_runs("owner/repo", ("queued", "in_progress")) + assert repeated == first + + # 2 calls total: one per status in the first, cache-populating call -- + # not 2 * 51 for 51 identical requests. + assert len(calls) == 2 + + +def test_active_workflow_runs_cache_is_faster_than_repeated_fetches(monkeypatch): + """Caching turns N redundant slow fetches into 1: wall clock reflects that.""" + delay = 0.02 + call_count = 0 + + def slow_fake_run(args, stdin=None): + del args, stdin + nonlocal call_count + call_count += 1 + time.sleep(delay) + return json.dumps([{"workflow_runs": []}]) + + monkeypatch.setattr(sched, "run_github_actions", slow_fake_run) + + repeats = 20 + start = time.monotonic() + for _ in range(repeats): + sched.active_workflow_runs("owner/repo", ("queued", "in_progress")) + elapsed = time.monotonic() - start + + # Uncached, 20 repeats * 2 statuses * 0.02s would take >= 0.8s; cached, + # only the first call's 2 statuses ever sleep. Generous bound keeps this + # robust on a loaded CI runner while still catching a caching regression. + assert call_count == 2 + assert elapsed < delay * 2 * repeats / 2 + + +def test_active_workflow_runs_cache_keys_on_full_call_shape(monkeypatch): + """Distinct repo/statuses/event/created/head_sha never share a cache entry.""" + calls = [] + + def fake_run(args, stdin=None): + del stdin + calls.append(args) + return json.dumps([{"workflow_runs": []}]) + + monkeypatch.setattr(sched, "run_github_actions", fake_run) + + sched.active_workflow_runs("owner/repo", ("queued",)) + sched.active_workflow_runs("owner/other-repo", ("queued",)) + sched.active_workflow_runs("owner/repo", ("in_progress",)) + sched.active_workflow_runs("owner/repo", ("queued",), event="repository_dispatch") + sched.active_workflow_runs("owner/repo", ("queued",), head_sha="a" * 40) + sched.active_workflow_runs("owner/repo", ("queued",)) # repeat of the first: cache hit + + assert len(calls) == 5 + + +def test_force_cancel_workflow_runs_invalidates_active_workflow_runs_cache(monkeypatch): + """A cancellation must not be masked by a stale pre-cancellation cache entry. + + ``dispatch_strix_evidence``'s busy_refs check runs right after + ``force_cancel_workflow_run_refs`` cancels stale runs for the same + repository; if the cache were not invalidated, that check could see a + run this very call just cancelled and wrongly report the repository + busy, or a later PR's cancel_stale_pr_runs could miss a run it should + force-cancel because a same-shape read from before an earlier + cancellation was replayed instead of re-fetched. + """ + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.setenv("GH_TOKEN", "workflow-token") + responses = [ + json.dumps([{"workflow_runs": [{"id": 9001}]}]), # queued, before cancel + json.dumps([{"workflow_runs": []}]), # in_progress, before cancel + "", # the force-cancel POST itself + json.dumps([{"workflow_runs": []}]), # queued, after cancel: must re-fetch + json.dumps([{"workflow_runs": []}]), # in_progress, after cancel + ] + + def fake_run(args, stdin=None): + del args, stdin + return responses.pop(0) + + monkeypatch.setattr(sched, "run", fake_run) + + before = sched.active_workflow_runs("owner/repo", ("queued", "in_progress")) + assert before == [{"id": 9001}] + + sched.force_cancel_workflow_runs("owner/repo", ["9001"]) + + after = sched.active_workflow_runs("owner/repo", ("queued", "in_progress")) + assert after == [] + assert responses == [] # every canned response was consumed: no call was skipped or reused + + def test_dispatch_strix_cancels_stale_central_run_and_keeps_current(monkeypatch, capsys): calls = [] head_sha = "a" * 40