diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 19ea58003f..b8b7833712 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -565,11 +565,22 @@ jobs: return 0 fi local run_ids + # Workflow identity comes from `.path`, not `.name` -- the same + # defect strix.yml's cleanup job carried. This workflow declares + # `run-name:`, so a run's `name` is the rendered title + # "Required OpenCode Review #@", never the bare + # workflow name, and the former `.name ==` equality selected + # nothing: sampled 2026-09-07, 0 of 100 runs carried the bare name + # and 100 of 100 carried the rendered one. `.path` is the signal + # noema-review.yml already adopted here, stable for both native and + # ruleset-injected runs; the `startswith` check keeps the second, + # independent signal the equality was reaching for. if ! run_ids="$(jq -r --arg pr "$TARGET_PR_NUMBER" --arg head_sha "$TARGET_PR_HEAD_SHA" \ --arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" ' .workflow_runs[] | select((.id | tostring) != $current) - | select(.name == "Required OpenCode Review") + | select(.path == ".github/workflows/opencode-review.yml") + | select((.name // "") | startswith("Required OpenCode Review")) | select(.event == "pull_request_target") | ((.display_title // "") | startswith("Required OpenCode Review " + $repo + "#" + $pr + "@")) as $title_matches | ((.pull_requests // []) | any((.number | tostring) == $pr)) as $metadata_matches diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 58ed3dab8d..b3f4cd5776 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -292,11 +292,32 @@ jobs: return 0 fi local run_ids + # Workflow identity comes from `.path`, not `.name`. This workflow + # declares `run-name:`, and GitHub reports the *rendered* run-name + # in a run's `name` -- "Strix Security Scan #@", + # never the bare workflow name. The former `.name ==` equality + # therefore matched nothing and this whole cleanup job was a silent + # no-op: sampled 2026-09-07, 0 of 100 strix runs carried the bare + # name while 100 of 100 carried the rendered one. `.path` is the + # signal noema-review.yml already adopted for this same defect, and + # it holds in both contexts -- verified the same day as + # ".github/workflows/strix.yml" on native `.github` runs and on the + # 9 ruleset-injected runs in bandscope, which carries no local + # strix.yml. The `startswith` name check is kept as the second, + # independent signal the original equality was reaching for. + # + # The workflow-level `concurrency` group above hides this on the + # `synchronize` path (it cancels the previous head's run itself), + # so what was actually lost is every case no successor run + # supersedes: `closed` and `converted_to_draft` left their in-flight + # scans running to completion, holding admission slots under the + # shared 60-job ceiling for a PR nobody is waiting on. if ! run_ids="$(jq -r --arg pr "$TARGET_PR_NUMBER" --arg head_sha "$TARGET_PR_HEAD_SHA" \ --arg action "$PR_ACTION" --arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" ' .workflow_runs[] | select((.id | tostring) != $current) - | select(.name == "Strix Security Scan") + | select(.path == ".github/workflows/strix.yml") + | select((.name // "") | startswith("Strix Security Scan")) | select(.event == "pull_request_target") | ((.display_title // "") | startswith("Strix Security Scan " + $repo + "#" + $pr + "@")) as $title_matches | ((.pull_requests // []) | any((.number | tostring) == $pr)) as $metadata_matches diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 4df4dac3de..5b30dab6b0 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -3175,6 +3175,42 @@ def workflow_run_mentions_pr(run_data: dict[str, Any], pr_number: int) -> bool: return any(pr.get("number") == pr_number for pr in run_data.get("pull_requests") or []) +def run_name_identifies_workflow(run_name: str, *workflow_names: str) -> bool: + """Return whether a run's reported ``name`` identifies one of these workflows. + + GitHub reports the *rendered* ``run-name:`` in a run's ``name`` field, not + the workflow's declared ``name:``. Every central review workflow here + declares one, so ``name`` arrives as ``" #@"`` + and an equality test against the declared name matches nothing in + production at all: sampled 2026-09-07, 0 of 100 ``strix.yml`` runs and 0 of + 100 ``opencode-review.yml`` runs carried the bare form while 100 of 100 + carried the rendered one. + + Both forms are accepted because both occur. The bare name is what a + workflow declaring no ``run-name:`` sends, and what GitHub can fall back to + for an organization-required-workflow run materialized in a sibling + repository (recorded on ``noema-review.yml``'s cleanup job). + + The space this requires after a candidate is a word boundary, and that is + the whole of what it buys: a workflow named "Strix Security Scanner" does + not answer for the candidate "Strix Security Scan". It deliberately does + *not* separate a longer workflow name that begins with the candidate and a + space -- a hypothetical "Strix Security Scan Extended" would be accepted. + Two things make that safe rather than latent. No central workflow name + prefixes another (verified 0 of 35 on 2026-09-07, and pinned by + :mod:`tests.test_stale_run_cleanup_workflow_identity`), and every call site + pins identity a second time -- by ``display_title`` prefix, the run's + ``path``, or pull-request metadata -- so this predicate narrows a candidate + set rather than deciding identity alone. Accepting a prefix is also + required, not merely tolerated: callers pass short aliases ("Strix") for + the same workflow on purpose. + """ + return any( + run_name == candidate or run_name.startswith(f"{candidate} ") + for candidate in workflow_names + ) + + def stale_pr_run_ids( repo: str, pr: dict[str, Any], @@ -3195,7 +3231,9 @@ def stale_pr_run_ids( number = int(pr["number"]) stale: list[str] = [] for run_data in active_workflow_runs(repo, statuses): - if workflow is not None and run_data.get("name") != workflow: + if workflow is not None and not run_name_identifies_workflow( + str(run_data.get("name") or ""), workflow + ): continue if str(run_data.get("head_sha") or "").lower() == head: continue @@ -3266,10 +3304,7 @@ def active_review_run_refs( # never cancelled. Sampled 2026-09-07: 100 of 100 # opencode-review-dispatch runs carry the rendered form, 0 bare. # Accept it -- the workflow name, then a space, then the suffix. - if not any( - run_name == candidate or run_name.startswith(f"{candidate} ") - for candidate in (workflow, *workflow_aliases) - ): + if not run_name_identifies_workflow(run_name, workflow, *workflow_aliases): continue run_id = run_data.get("id") if not run_id: @@ -3777,7 +3812,7 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry run_title="Strix Security Scan", workflow_aliases=frozenset({"Strix Security Scan"}), ) - preserved_run_refs, cancelled_refs = _cancel_revalidated_review_run_refs( + preserved_run_refs, _cancelled_refs = _cancel_revalidated_review_run_refs( repo, workflow, pr, stale_run_refs ) current_run_refs = [*current_run_refs, *preserved_run_refs] @@ -3791,24 +3826,6 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry return "already_running" target_repo = validate_github_repository(repo) dispatch_repo = repository_dispatch_target(target_repo) - cancelled_ids = {run_id for _, run_id in cancelled_refs} - busy_refs = [ - (dispatch_repo, str(run_data["id"])) - for run_data in active_workflow_runs(dispatch_repo) - if run_data.get("id") - and str(run_data["id"]) not in cancelled_ids - and run_data.get("name") == workflow - and run_data.get("event") == "repository_dispatch" - and str(run_data.get("display_title") or "").startswith( - f"Strix Security Scan {target_repo}#" - ) - ] - if busy_refs: - print( - "Strix evidence dispatch skipped: target repository already has active run(s) " - + ", ".join(f"{run_repo}@{run_id}" for run_repo, run_id in busy_refs) - ) - return "repository_busy" if not review_dispatch_admitted("strix", repo, pr): return "admission_deferred" base_ref, base_sha, head_sha = validated_pr_dispatch_fields(pr) diff --git a/tests/test_opencode_required_verdict_regression.py b/tests/test_opencode_required_verdict_regression.py index 5c5325d1aa..7be9ec1481 100644 --- a/tests/test_opencode_required_verdict_regression.py +++ b/tests/test_opencode_required_verdict_regression.py @@ -215,12 +215,22 @@ def _cleanup_run( *, run_id: int, head_sha: str = HEAD, - name: str = "Required OpenCode Review", + name: str | None = None, + path: str = ".github/workflows/opencode-review.yml", event: str = "pull_request_target", display_title: str | None = None, pr_number: int = 1437, ) -> dict[str, object]: - """Build one synthetic workflow-run record for the cleanup filter.""" + """Build one synthetic workflow-run record for the cleanup filter. + + ``name`` defaults to the same string as ``display_title`` because that is + what GitHub sends: this workflow declares ``run-name:``, and a run's + ``name`` is the rendered result, never the declared workflow name. The + earlier default paired a bare ``name`` with a rendered ``display_title``, a + combination the API cannot produce, which is how the selector's former + ``.name ==`` equality passed here while matching 0 of 100 live runs. + ``path`` carries the workflow identity the selector now reads. + """ title = ( display_title if display_title is not None @@ -228,7 +238,8 @@ def _cleanup_run( ) return { "id": run_id, - "name": name, + "name": title if name is None else name, + "path": path, "event": event, "display_title": title, "pull_requests": [{"number": pr_number, "head": {"sha": head_sha}}], @@ -261,7 +272,12 @@ def test_cleanup_excludes_a_different_pull_request() -> None: def test_cleanup_excludes_a_differently_named_or_triggered_run() -> None: """A same-PR run for another workflow or trigger is left untouched.""" - other_workflow = _cleanup_run(run_id=1, head_sha="b" * 40, name="Strix Security Scan") + other_workflow = _cleanup_run( + run_id=1, + head_sha="b" * 40, + name="Strix Security Scan", + path=".github/workflows/strix.yml", + ) other_event = _cleanup_run(run_id=2, head_sha="b" * 40, event="workflow_dispatch") assert ( cleanup_candidate_run_ids([other_workflow, other_event], current_run_id="999") diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index ba47b89c8d..06cf737780 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -6077,38 +6077,6 @@ def fake_run(args, stdin=None): ) -def test_dispatch_strix_waits_for_active_target_repository_run(monkeypatch, capsys): - calls = [] - active_run = { - "id": 9350, - "name": "Strix Security Scan", - "event": "repository_dispatch", - "display_title": f"Strix Security Scan owner/repo#2@{'c' * 40}", - "pull_requests": [], - } - - monkeypatch.setattr( - sched, - "active_workflow_runs", - lambda repo, statuses=("queued", "in_progress"): [active_run], - ) - monkeypatch.setattr(sched, "run_github_dispatch", lambda args, stdin=None: calls.append(args)) - monkeypatch.setenv("GITHUB_ACTIONS", "true") - monkeypatch.setenv("GH_TOKEN", "workflow-token") - monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", "ContextualWisdomLab/.github") - - result = sched.dispatch_strix_evidence( - "owner/repo", - "Strix Security Scan", - make_pr(headRefOid="a" * 40), - dry_run=False, - ) - - assert result == "repository_busy" - assert calls == [] - assert "target repository already has active run(s) ContextualWisdomLab/.github@9350" in capsys.readouterr().out - - def test_central_run_filter_accepts_the_run_name_github_actually_sends(monkeypatch): """A ``run-name:`` workflow reports the rendered title in ``name``. diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 4ab09b1d0c..9b6518fef6 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -781,13 +781,19 @@ def test_strix_cleanup_uses_pr_metadata_when_custom_title_is_absent() -> None: marker = '--arg action "$PR_ACTION" --arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" \'\n' start = workflow.index(marker) + len(marker) end = workflow.index('\n \' <<<"$runs_json"', start) + # Every row carries ``path``: the selector reads workflow identity there + # now, because a run's ``name`` is the rendered ``run-name:`` and the old + # ``.name ==`` equality matched 0 of 100 live runs. The bare ``name`` here + # is deliberate and still accepted -- it is the required-workflow-ruleset + # shape this test exists for, where GitHub renders no run-name at all. + strix_path = ".github/workflows/strix.yml" runs = { "workflow_runs": [ - {"id": 1, "name": "Strix Security Scan", "event": "pull_request_target", "pull_requests": [{"number": 7, "head": {"sha": "old"}}]}, - {"id": 2, "name": "Strix Security Scan", "event": "pull_request_target", "pull_requests": [{"number": 7, "head": {"sha": "current"}}]}, - {"id": 3, "name": "Strix Security Scan", "event": "pull_request_target", "pull_requests": [{"number": 7}]}, - {"id": 4, "name": "Strix Security Scan", "event": "pull_request_target", "display_title": "Strix Security Scan owner/repo#7@old", "pull_requests": [{"number": 7, "head": {"sha": "current"}}]}, - {"id": 5, "name": "Strix Security Scan", "event": "pull_request_target", "pull_requests": [{"number": 8, "head": {"sha": "old"}}]}, + {"id": 1, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "pull_requests": [{"number": 7, "head": {"sha": "old"}}]}, + {"id": 2, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "pull_requests": [{"number": 7, "head": {"sha": "current"}}]}, + {"id": 3, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "pull_requests": [{"number": 7}]}, + {"id": 4, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "display_title": "Strix Security Scan owner/repo#7@old", "pull_requests": [{"number": 7, "head": {"sha": "current"}}]}, + {"id": 5, "name": "Strix Security Scan", "path": strix_path, "event": "pull_request_target", "pull_requests": [{"number": 8, "head": {"sha": "old"}}]}, ] } result = subprocess.run( @@ -836,7 +842,7 @@ def _run_strix_cleanup( exit 0 fi if [[ "$*" == *"actions/runs?status=queued"* ]]; then - printf '%s\n' '{"workflow_runs":[{"id":100,"name":"Strix Security Scan","event":"pull_request_target","pull_requests":[{"number":7,"head":{"sha":"old"}}]}]}' + printf '%s\n' '{"workflow_runs":[{"id":100,"name":"Strix Security Scan","path":".github/workflows/strix.yml","event":"pull_request_target","pull_requests":[{"number":7,"head":{"sha":"old"}}]}]}' exit 0 fi if [[ "$*" == *"actions/runs?status="* ]]; then diff --git a/tests/test_stale_run_cleanup_workflow_identity.py b/tests/test_stale_run_cleanup_workflow_identity.py new file mode 100644 index 0000000000..0f34e76ebc --- /dev/null +++ b/tests/test_stale_run_cleanup_workflow_identity.py @@ -0,0 +1,356 @@ +"""Pin the run-identity signal both stale-run cleanup jobs select on. + +``strix.yml`` and ``opencode-review.yml`` each carry a cleanup job whose whole +purpose is retiring superseded runs of their own workflow. Both selected those +runs with ``select(.name == "")``. Both workflows +declare ``run-name:``, and GitHub reports the *rendered* run-name in a run's +``name`` field -- so the equality matched nothing and each job was a silent +no-op. Measured 2026-09-07 against the live API: of the 100 most recent runs of +each workflow, 0 carried the bare declared name and 100 carried the rendered +``" #@"`` form. + +The repair adopts the signal ``noema-review.yml``'s cleanup job already uses +for the identical defect: ``.path``, which the same measurement confirmed +stable in both contexts -- ``.github/workflows/strix.yml`` on native runs in +``.github`` and on the nine ruleset-injected runs in ``bandscope``, which +carries no local copy of that workflow. + +These tests execute the production jq against fixtures in the rendered shape, +so a revert to equality fails here rather than shipping green. Each selector +also gets a negative control: a foreign workflow's run attached to the same +pull request must not be selected, because the surviving ``$metadata_matches`` +branch would otherwise let this job cancel other workflows' runs. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +import scripts.ci.pr_review_merge_scheduler_core as sched + +STRIX_WORKFLOW = Path(".github/workflows/strix.yml") +OPENCODE_WORKFLOW = Path(".github/workflows/opencode-review.yml") + + +def _selector(workflow: Path, start_marker: str) -> str: + """Return the jq program the named cleanup job runs, read from the workflow.""" + text = workflow.read_text(encoding="utf-8") + start = text.index(start_marker) + len(start_marker) + end = text.index('\n \' <<<"$runs_json"', start) + return text[start:end] + + +def _run_jq(selector: str, args: list[str], runs: dict[str, object]) -> list[str]: + """Execute a selector against a runs payload, returning the selected ids.""" + jq = shutil.which("jq") + if jq is None: # pragma: no cover - environment without jq + pytest.skip("jq is required to execute the production cleanup selector") + result = subprocess.run( + [jq, "-r", *args, selector], + input=json.dumps(runs), + text=True, + capture_output=True, + check=True, + ) + return result.stdout.split() + + +def _strix_selector() -> str: + """Return the Strix cleanup job's jq selector.""" + return _selector( + STRIX_WORKFLOW, + '--arg action "$PR_ACTION" --arg repo "$TARGET_REPOSITORY"' + ' --arg current "$CURRENT_RUN_ID" \'\n', + ) + + +def _opencode_selector() -> str: + """Return the OpenCode review cleanup job's jq selector.""" + return _selector( + OPENCODE_WORKFLOW, + '--arg repo "$TARGET_REPOSITORY" --arg current "$CURRENT_RUN_ID" \'\n', + ) + + +def test_strix_cleanup_selects_a_superseded_run_whose_name_is_rendered() -> None: + """The Strix cleanup selector matches the only run shape GitHub sends. + + ``name`` and ``display_title`` are identical here because that is what the + API returns for a workflow declaring ``run-name:``; a fixture pairing a + bare ``name`` with a rendered ``display_title`` describes no real run and + is what let the equality survive. + """ + old = f"Strix Security Scan owner/repo#7@{'a' * 40}" + current = f"Strix Security Scan owner/repo#7@{'b' * 40}" + runs = { + "workflow_runs": [ + { + "id": 4001, + "path": ".github/workflows/strix.yml", + "name": old, + "display_title": old, + "event": "pull_request_target", + "pull_requests": [{"number": 7, "head": {"sha": "a" * 40}}], + }, + { + "id": 4002, + "path": ".github/workflows/strix.yml", + "name": current, + "display_title": current, + "event": "pull_request_target", + "pull_requests": [{"number": 7, "head": {"sha": "b" * 40}}], + }, + ] + } + selected = _run_jq( + _strix_selector(), + [ + "--arg", "pr", "7", + "--arg", "head_sha", "b" * 40, + "--arg", "action", "synchronize", + "--arg", "repo", "owner/repo", + "--arg", "current", "4002", + ], + runs, + ) + assert selected == ["4001"] + + +def test_strix_cleanup_retires_a_run_of_a_closed_pull_request() -> None: + """A closed pull request's in-flight scan is retired with no successor run. + + This is the case the workflow-level ``concurrency`` group cannot reach: on + ``synchronize`` a newer run supersedes the old one and GitHub cancels it, + but ``closed`` and ``converted_to_draft`` start no successor, so the dead + selector left those scans running to completion against the shared job + ceiling for a pull request nobody is waiting on. + """ + title = f"Strix Security Scan owner/repo#7@{'a' * 40}" + runs = { + "workflow_runs": [ + { + "id": 4010, + "path": ".github/workflows/strix.yml", + "name": title, + "display_title": title, + "event": "pull_request_target", + "pull_requests": [{"number": 7, "head": {"sha": "a" * 40}}], + } + ] + } + selected = _run_jq( + _strix_selector(), + [ + "--arg", "pr", "7", + "--arg", "head_sha", "a" * 40, + "--arg", "action", "closed", + "--arg", "repo", "owner/repo", + "--arg", "current", "4099", + ], + runs, + ) + assert selected == ["4010"] + + +def test_strix_cleanup_leaves_another_workflows_run_alone() -> None: + """A foreign workflow's run on the same pull request is never selected. + + ``$metadata_matches`` accepts any run whose ``pull_requests[]`` names this + pull request, so without a workflow-identity filter this job would cancel + every other central workflow's runs. ``.path`` is that filter. + """ + foreign = f"Required OpenCode Review owner/repo#7@{'a' * 40}" + runs = { + "workflow_runs": [ + { + "id": 4020, + "path": ".github/workflows/opencode-review.yml", + "name": foreign, + "display_title": foreign, + "event": "pull_request_target", + "pull_requests": [{"number": 7, "head": {"sha": "a" * 40}}], + } + ] + } + selected = _run_jq( + _strix_selector(), + [ + "--arg", "pr", "7", + "--arg", "head_sha", "b" * 40, + "--arg", "action", "closed", + "--arg", "repo", "owner/repo", + "--arg", "current", "4099", + ], + runs, + ) + assert selected == [] + + +def test_opencode_cleanup_selects_a_superseded_run_whose_name_is_rendered() -> None: + """The OpenCode review cleanup selector matches the rendered run shape.""" + old = f"Required OpenCode Review owner/repo#9@{'c' * 40}" + current = f"Required OpenCode Review owner/repo#9@{'d' * 40}" + runs = { + "workflow_runs": [ + { + "id": 5001, + "path": ".github/workflows/opencode-review.yml", + "name": old, + "display_title": old, + "event": "pull_request_target", + "pull_requests": [{"number": 9, "head": {"sha": "c" * 40}}], + }, + { + "id": 5002, + "path": ".github/workflows/opencode-review.yml", + "name": current, + "display_title": current, + "event": "pull_request_target", + "pull_requests": [{"number": 9, "head": {"sha": "d" * 40}}], + }, + ] + } + selected = _run_jq( + _opencode_selector(), + [ + "--arg", "pr", "9", + "--arg", "head_sha", "d" * 40, + "--arg", "repo", "owner/repo", + "--arg", "current", "5002", + ], + runs, + ) + assert selected == ["5001"] + + +def test_opencode_cleanup_leaves_another_workflows_run_alone() -> None: + """A Strix run on the same pull request is not cancelled by OpenCode cleanup.""" + foreign = f"Strix Security Scan owner/repo#9@{'c' * 40}" + runs = { + "workflow_runs": [ + { + "id": 5010, + "path": ".github/workflows/strix.yml", + "name": foreign, + "display_title": foreign, + "event": "pull_request_target", + "pull_requests": [{"number": 9, "head": {"sha": "c" * 40}}], + } + ] + } + selected = _run_jq( + _opencode_selector(), + [ + "--arg", "pr", "9", + "--arg", "head_sha", "d" * 40, + "--arg", "repo", "owner/repo", + "--arg", "current", "5099", + ], + runs, + ) + assert selected == [] + + +def test_run_name_identifies_workflow_accepts_both_forms_github_sends() -> None: + """Both the rendered and the bare run name identify their workflow.""" + assert sched.run_name_identifies_workflow( + f"Strix Security Scan owner/repo#7@{'a' * 40}", "Strix Security Scan" + ) + assert sched.run_name_identifies_workflow("Strix Security Scan", "Strix Security Scan") + + +def test_run_name_identifies_workflow_requires_a_word_boundary() -> None: + """A name that extends the candidate without a separator is rejected. + + This is the whole of what the required space buys, and the assertion is + written to say so rather than to imply the predicate resolves identity by + itself: "Strix Security Scan Extended" *is* accepted, and is safe only + because no central workflow name prefixes another and every call site pins + identity again by ``.path``, ``display_title``, or pull-request metadata. + """ + assert not sched.run_name_identifies_workflow( + "Strix Security Scanner owner/repo#7@abc", "Strix Security Scan" + ) + assert not sched.run_name_identifies_workflow("", "Strix Security Scan") + assert sched.run_name_identifies_workflow( + "Strix Security Scan Extended", "Strix Security Scan" + ) + + +def test_no_central_workflow_name_prefixes_another() -> None: + """Pin the premise that makes a prefix accept safe for these workflows. + + :func:`scripts.ci.pr_review_merge_scheduler_core.run_name_identifies_workflow` + accepts a candidate followed by a space, so a new workflow named as an + extension of an existing one ("Strix Security Scan Extended") would start + answering for it. Nothing else in the repository would notice; this test is + the notice. + """ + # Both extensions, though every workflow here is currently ``.yml``. A + # single ``*.yml`` glob would drop a future ``.yaml`` workflow out of the + # premise silently, and the count guard below would not notice either -- + # 35 files minus one is still comfortably over the floor. A test whose + # whole purpose is announcing a change nobody would otherwise see must not + # be bypassable by a file extension. + workflows = sorted( + { + *Path(".github/workflows").glob("*.yml"), + *Path(".github/workflows").glob("*.yaml"), + } + ) + names = sorted( + { + line.split(":", 1)[1].strip().strip("\"'") + for workflow in workflows + for line in workflow.read_text(encoding="utf-8").splitlines() + if line.startswith("name:") + } + ) + # Refuse a vacuous pass: an empty or mis-rooted glob makes the collision + # check below trivially true, which is the exact shape this repository has + # shipped before (an audit reporting "PASS: all 0 repositories"). + assert len(names) >= 30 + # Every workflow file must contribute a name, or a file could drop out of + # the premise by losing its top-level ``name:`` rather than its extension. + assert len(names) == len(workflows) + collisions = [ + (shorter, longer) + for shorter in names + for longer in names + if shorter != longer and longer.startswith(f"{shorter} ") + ] + assert collisions == [] + + +def test_stale_pr_run_ids_matches_a_rendered_workflow_run_name(monkeypatch) -> None: + """``stale_pr_run_ids`` narrows by workflow using the name GitHub sends. + + Its only production caller passes no ``workflow``, so the equality this + replaces never ran; the parameter stayed a trap that silently returned no + runs for any caller that did supply one. + """ + monkeypatch.setattr(sched, "validate_git_sha", lambda value: str(value)) + rendered = f"Required OpenCode Review owner/repo#1@{'a' * 40}" + runs = [ + {"name": rendered, "id": 61, "head_sha": "old", "pull_requests": [{"number": 1}]}, + {"name": rendered, "id": 62, "head_sha": "head", "pull_requests": [{"number": 1}]}, + { + "name": f"Strix Security Scan owner/repo#1@{'a' * 40}", + "id": 63, + "head_sha": "old", + "pull_requests": [{"number": 1}], + }, + ] + monkeypatch.setattr( + sched, "active_workflow_runs", lambda repo, statuses=("queued", "in_progress"): runs + ) + pr = {"number": 1, "headRefOid": "head"} + + assert sched.stale_pr_run_ids( + "owner/repo", pr, workflow="Required OpenCode Review" + ) == ["61"]