diff --git a/CHANGELOG.md b/CHANGELOG.md index ca81dcea1d..1e2469c69b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,7 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Bind stale-review run revalidation to repository-correct credentials.** Central `repository_dispatch` Actions evidence now uses the existing central dispatch read authority while direct target-repository runs retain target read authority. - **Pin `opencode-review-dispatch.yml` off the starved floating `ubuntu-latest` image.** The 2026-09-01 floating-image fix (see that entry below) pinned `strix.yml`, `opencode-review.yml`, and `noema-review.yml` -- the three required-check diff --git a/docs/doctoring/scheduler-central-run-read-authority.md b/docs/doctoring/scheduler-central-run-read-authority.md new file mode 100644 index 0000000000..9e4f3ac5f5 --- /dev/null +++ b/docs/doctoring/scheduler-central-run-read-authority.md @@ -0,0 +1,21 @@ +# Scheduler central run read authority + +## Problem + +The stale-review cancellation path introduced by `ContextualWisdomLab/.github#1669` revalidates an active workflow run immediately before force-cancellation. Direct pull-request runs live in the target repository, but organization-wide OpenCode and Strix `repository_dispatch` runs live in the configured central workflow repository. Reading both through the target-repository credential is therefore not a valid authority boundary: a target-only credential can be unable to read the central Actions run, causing fail-closed preservation of a genuinely stale central run and preventing a replacement current-head review from dispatching. + +## Constraint and decision + +The protected scheduler already separates target-repository reads (`gh_api_json`) from central-repository Actions reads (`gh_api_json_via_dispatch_token`, backed by `SCHEDULER_DISPATCH_TOKEN`). `_fresh_active_run_for_cancellation()` therefore selects the central reader only when `run_repo` exactly equals the validated configured `SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY`; all other repositories retain the target reader. When no central repository is configured, the helper does not invent elevated authority and continues through the target reader. Existing fail-closed handling remains unchanged: malformed, inaccessible, or non-active evidence preserves the candidate instead of authorizing cancellation. + +## Failure scenarios and evidence + +1. A stale central `repository_dispatch` run belongs to `ContextualWisdomLab/.github` while the inspected PR belongs to `ContextualWisdomLab/fast-mlsirm`. Revalidation must use central dispatch authority, otherwise a target-only token can strand the stale run and block replacement review dispatch. +2. A direct Actions run belongs to the target repository. Revalidation must continue to use target read authority; central dispatch credentials are not widened to target evidence. +3. Central ownership is absent or malformed. The scheduler does not guess a central repository or silently broaden credentials. + +`tests/test_scheduler_central_run_read_authority.py` binds these cases to the production helper. The repair is control-plane credential routing only: it does not change model selection, review semantics, cancellation criteria, merge authority, required checks, or leaf repository source. + +## Rollback and follow-up + +Rollback is the single helper-level reader selection plus this regression contract. After protected-main integration, re-evaluate affected leaf PRs for fresh current-head OpenCode/Strix evidence and confirm stale central runs no longer block replacement dispatch. Do not transfer predecessor review/check evidence. diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index c4e9d28ebd..4d69aefb67 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -3442,8 +3442,13 @@ def _fresh_open_pr_for_cancellation(repo: str, number: int) -> dict[str, Any]: def _fresh_active_run_for_cancellation(run_repo: str, run_id: str) -> dict[str, Any]: - """Return fresh active workflow-run evidence immediately before cancellation.""" - payload = gh_api_json(f"repos/{run_repo}/actions/runs/{run_id}") + """Return fresh active workflow-run evidence with repository-correct read authority.""" + central_repo = (os.environ.get("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY") or "").strip() + use_dispatch_authority = bool( + central_repo and run_repo == validate_github_repository(central_repo) + ) + reader = gh_api_json_via_dispatch_token if use_dispatch_authority else gh_api_json + payload = reader(f"repos/{run_repo}/actions/runs/{run_id}") if not isinstance(payload, dict) or str(payload.get("status") or "").lower() not in { "queued", "in_progress", diff --git a/tests/test_scheduler_central_run_read_authority.py b/tests/test_scheduler_central_run_read_authority.py new file mode 100644 index 0000000000..4e14b39da0 --- /dev/null +++ b/tests/test_scheduler_central_run_read_authority.py @@ -0,0 +1,76 @@ +"""Regression coverage for repository-correct stale-review run revalidation authority.""" + +from __future__ import annotations + +from scripts.ci import pr_review_merge_scheduler as sched + + +CENTRAL_REPO = "ContextualWisdomLab/.github" +TARGET_REPO = "ContextualWisdomLab/fast-mlsirm" + + +def test_central_repository_dispatch_run_uses_dispatch_read_authority(monkeypatch) -> None: + """Central Actions evidence must not be read through a target-repository credential.""" + calls: list[tuple[str, str]] = [] + monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", CENTRAL_REPO) + monkeypatch.setattr( + sched, + "gh_api_json", + lambda path: calls.append(("target", path)) or (_ for _ in ()).throw( + AssertionError("target credential must not read central Actions evidence") + ), + ) + monkeypatch.setattr( + sched, + "gh_api_json_via_dispatch_token", + lambda path: calls.append(("dispatch", path)) or {"status": "queued"}, + ) + + payload = sched._fresh_active_run_for_cancellation(CENTRAL_REPO, "95") + + assert payload == {"status": "queued"} + assert calls == [("dispatch", f"repos/{CENTRAL_REPO}/actions/runs/95")] + + +def test_target_repository_run_retains_target_read_authority(monkeypatch) -> None: + """Direct target Actions evidence must keep the target-repository read boundary.""" + calls: list[tuple[str, str]] = [] + monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", CENTRAL_REPO) + monkeypatch.setattr( + sched, + "gh_api_json", + lambda path: calls.append(("target", path)) or {"status": "in_progress"}, + ) + monkeypatch.setattr( + sched, + "gh_api_json_via_dispatch_token", + lambda path: calls.append(("dispatch", path)) or (_ for _ in ()).throw( + AssertionError("dispatch credential must not read target Actions evidence") + ), + ) + + payload = sched._fresh_active_run_for_cancellation(TARGET_REPO, "96") + + assert payload == {"status": "in_progress"} + assert calls == [("target", f"repos/{TARGET_REPO}/actions/runs/96")] + + +def test_unconfigured_central_repository_fails_closed_to_target_authority(monkeypatch) -> None: + """Without a configured central owner, the helper must not invent dispatch authority.""" + calls: list[tuple[str, str]] = [] + monkeypatch.delenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", raising=False) + monkeypatch.setattr( + sched, + "gh_api_json", + lambda path: calls.append(("target", path)) or {"status": "queued"}, + ) + monkeypatch.setattr( + sched, + "gh_api_json_via_dispatch_token", + lambda path: calls.append(("dispatch", path)) or {"status": "queued"}, + ) + + payload = sched._fresh_active_run_for_cancellation(TARGET_REPO, "97") + + assert payload == {"status": "queued"} + assert calls == [("target", f"repos/{TARGET_REPO}/actions/runs/97")]