fix(scheduler): don't report a rejected run cancellation as cancelled

scripts/ci/pr_review_merge_scheduler.py

@@ -2966,13 +2966,24 @@ def cancel_one(run_id: str) -> tuple[str, str | None]:
     return failures
 
 
-def force_cancel_workflow_run_refs(run_refs: Sequence[tuple[str, str]]) -> None:
-    """Force-cancel repository-qualified runs while retaining bounded batches."""
+def force_cancel_workflow_run_refs(run_refs: Sequence[tuple[str, str]]) -> list[tuple[str, str]]:
+    """Force-cancel repository-qualified runs and return the ones actually cancelled.
+
+    ``force_cancel_workflow_runs`` reports GitHub's per-run cancellation rejections
+    as a ``{run_id: failure_reason}`` dict rather than raising. A caller that treats
+    every requested ref as gone once this returns would misclassify a run GitHub
+    refused to cancel as cancelled -- letting a duplicate review dispatch alongside
+    a run that is, in fact, still active. Exclude rejected refs from the result so
+    every caller can tell the difference.
+    """
     runs_by_repo: dict[str, list[str]] = {}
     for run_repo, run_id in run_refs:
         runs_by_repo.setdefault(run_repo, []).append(run_id)
+    cancelled: list[tuple[str, str]] = []
     for run_repo, run_ids in runs_by_repo.items():
-        force_cancel_workflow_runs(run_repo, run_ids)
+        failures = force_cancel_workflow_runs(run_repo, run_ids)
+        cancelled.extend((run_repo, run_id) for run_id in run_ids if run_id not in failures)
+    return cancelled
 
 
 def cancel_stale_pr_runs(repo: str, pr: dict[str, Any], *, dry_run: bool) -> list[str]:
@@ -2981,8 +2992,8 @@ def cancel_stale_pr_runs(repo: str, pr: dict[str, Any], *, dry_run: bool) -> lis
         return []
     require_github_actions_control_actor("force-cancel-stale-pr-runs")
     run_ids = stale_pr_run_ids(repo, pr)
-    force_cancel_workflow_runs(repo, run_ids)
-    return run_ids
+    failures = force_cancel_workflow_runs(repo, run_ids)
+    return [run_id for run_id in run_ids if run_id not in failures]
 
 
 def cancel_stale_opencode_runs(repo: str, workflow: str, pr: dict[str, Any], *, dry_run: bool) -> list[str]:
@@ -2991,8 +3002,8 @@ def cancel_stale_opencode_runs(repo: str, workflow: str, pr: dict[str, Any], *,
         return []
     require_github_actions_control_actor("force-cancel-stale-opencode-review")
     _, stale_refs = active_opencode_run_refs(repo, workflow, pr)
-    force_cancel_workflow_run_refs(stale_refs)
-    return [run_id for _, run_id in stale_refs]
+    cancelled_refs = force_cancel_workflow_run_refs(stale_refs)
+    return [run_id for _, run_id in cancelled_refs]
 
 
 def discover_opencode_required_run_id(repo: str, head_sha: str) -> int | None:
@@ -3131,7 +3142,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"}),
     )
-    force_cancel_workflow_run_refs(stale_run_refs)
+    cancelled_refs = force_cancel_workflow_run_refs(stale_run_refs)
     if current_run_refs:
         print(
             "Strix evidence dispatch skipped: active same-head workflow run(s) "
@@ -3142,12 +3153,12 @@ 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)
-    stale_ids = {run_id for _, run_id in stale_run_refs}
+    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 stale_ids
+        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(

tests/test_pr_review_merge_scheduler.py

@@ -1486,6 +1486,55 @@ def maybe_fail(args):
     }
 
 
+def test_cancel_revalidated_review_run_refs_preserves_failed_cancellation(monkeypatch):
+    """Keep a review ref busy when GitHub rejects its destructive cancellation.
+
+    Discovered mid-flight during PR #1669's development (the naruon headRefOid
+    incident fix) and intentionally scoped out of that PR; landing fresh here per
+    docs/doctoring/scheduler-stale-headrefoid-cancellation.md. That branch's
+    prototype named this cancellation path ``_cancel_revalidated_review_run_refs``;
+    current main's actual shared choke point for cancelling a revalidated batch of
+    stale/superseded review run refs -- used by both ``dispatch_opencode_review``
+    and ``dispatch_strix_evidence`` -- is :func:`force_cancel_workflow_run_refs`,
+    so this test (kept under the established name) targets that real function.
+    """
+    stale_refs = [("owner/repo", "101"), ("owner/repo", "202")]
+
+    def cancel(_repo, run_ids):
+        run_id = str(run_ids[0])
+        return {run_id: "GitHub rejected cancellation"} if run_id == "101" else {}
+
+    monkeypatch.setattr(sched, "force_cancel_workflow_runs", cancel)
+
+    cancelled = sched.force_cancel_workflow_run_refs(stale_refs)
+
+    assert ("owner/repo", "101") not in cancelled
+    assert ("owner/repo", "202") in cancelled
+
+
+def test_cancel_stale_opencode_runs_preserves_failed_cancellation(monkeypatch):
+    """Keep a stale review active when GitHub rejects its cancellation."""
+    stale_refs = [("owner/repo", "101"), ("owner/repo", "202")]
+    monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda _action: None)
+    monkeypatch.setattr(
+        sched,
+        "active_opencode_run_refs",
+        lambda _repo, _workflow, _pr: ([], stale_refs),
+    )
+
+    def cancel(_repo, run_ids):
+        run_id = str(run_ids[0])
+        return {run_id: "GitHub rejected cancellation"} if run_id == "101" else {}
+
+    monkeypatch.setattr(sched, "force_cancel_workflow_runs", cancel)
+
+    run_ids = sched.cancel_stale_opencode_runs(
+        "owner/repo", "OpenCode Review", make_pr(), dry_run=False
+    )
+
+    assert run_ids == ["202"]
+
+
 def test_cancel_stale_opencode_runs_dry_run_skips_lookup_and_mutation(monkeypatch):
     calls = []
     monkeypatch.setattr(sched, "stale_opencode_run_ids", lambda *args: calls.append(args) or ["1"])
@@ -5387,6 +5436,22 @@ def fake_run(args, stdin=None):
     assert any("status=in_progress" in " ".join(call) for call in calls)
 
 
+def test_cancel_stale_pr_runs_preserves_failed_cancellation(monkeypatch):
+    """Do not report a stale run cancelled when GitHub rejected the API call."""
+    monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda _action: None)
+    monkeypatch.setattr(sched, "stale_pr_run_ids", lambda _repo, _pr: ["101", "202"])
+
+    def cancel(_repo, run_ids):
+        run_id = str(run_ids[0])
+        return {run_id: "GitHub rejected cancellation"} if run_id == "101" else {}
+
+    monkeypatch.setattr(sched, "force_cancel_workflow_runs", cancel)
+
+    run_ids = sched.cancel_stale_pr_runs("owner/repo", make_pr(), dry_run=False)
+
+    assert run_ids == ["202"]
+
+
 def test_mutations_refuse_local_credentials(monkeypatch):
     calls = []
     monkeypatch.setattr(sched, "run", lambda args: calls.append(args) or "")