diff --git a/CHANGELOG.md b/CHANGELOG.md index 4120953dd4..98d62769e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,13 @@ ## Proposed +- Keep target-repository old-head Actions inventory and destructive-boundary + revalidation when review execution is centralized, while excluding only + bare or rendered OpenCode workflow names whose lifecycle belongs to the + dispatch repository. Target-owned CodeQL, security, and other direct + pull-request runs remain eligible for proven-old-head cancellation; + same-repository cleanup remains unfiltered. + - Run Python Security and Agent Review Runtime Quality CI for stacked pull requests by removing their pull-request base-branch filters. Extend the permanent stacked-workflow contract so all four owner review workflows diff --git a/docs/doctoring/central-review-target-inventory-suppression.md b/docs/doctoring/central-review-target-inventory-suppression.md new file mode 100644 index 0000000000..c16a38424e --- /dev/null +++ b/docs/doctoring/central-review-target-inventory-suppression.md @@ -0,0 +1,50 @@ +# Central review workflow-authority filtering + +Decision date: **2026-09-07** + +## Problem + +When the trusted reviewer is hosted centrally, target-repository OpenCode +workflow runs are not the authority for the central current-head verdict. +The first implementation therefore skipped the target repository's entire +old-head Actions inventory. That also preserved stale target-owned CodeQL, +Python Security, and other direct pull-request runs, whose lifecycle remains +the target repository's responsibility. + +## Decision + +Always retain target-repository stale-run inventory and the existing +destructive-boundary live PR/head revalidation. When the configured review +dispatch repository differs from the target, exclude only bare or rendered +OpenCode workflow names from the target cancellation candidates; the central +reviewer owns those runs in the dispatch repository. Target-owned CodeQL, +security, and other direct workflows remain eligible for proven-old-head +cancellation. When repository identities match case-insensitively, retain +unfiltered cleanup. + +## Failure scenes + +- Central review of a target repository: stale OpenCode names are excluded, but + stale target-owned CodeQL and security runs remain cancellable. +- Rendered GitHub run names such as + `OpenCode Review Dispatch owner/repo#1@` receive the same boundary as + their bare workflow names. +- Same-repository review: stale old-head cleanup remains unfiltered. +- Repository name casing differs: case-insensitive identity prevents accidental + cross-repository classification. + +## Evidence and follow-up + +The original RED commit +`08a16caa4fdb0d0d86c44bb8cd7aed611beaab7b` covered only the unsafe broad +suppression. Corrective RED commits +`234d98dec14ae7a91819857f561b78d0d424ec98` and +`32a0d66cd1210f6fae1cb675265ce4ce49f63167` prove the workflow-authority +filter, unrelated target-workflow preservation, central invocation, and +case-insensitive same-repository boundary. +Fresh exact-head hosted checks and independent review remain required. + +## Reference + +GitHub. (2026). *REST API endpoints for workflow runs*. +https://docs.github.com/en/rest/actions/workflow-runs diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 78793707d6..04750d0cea 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -3404,3 +3404,20 @@ same name in another file can carry the opposite safety property.** workflows at exact head `e2204eeb1ec2789ff791036140ba1672995d25f5`; RED commit `890bac2f69ff1a51f774ddf5d6c5d819afed4ac9`; fresh exact-head hosted checks remain required. + + +### Central review workflow-authority filtering + +- **Status:** Proposed +- **Owner:** `ContextualWisdomLab/.github` +- **Problem:** Broadly skipping target-repository Actions inventory for a + central reviewer also preserved stale target-owned CodeQL, security, and + other direct pull-request runs. +- **Action:** Retain target inventory and live PR/head revalidation. When the + dispatch repository differs, exclude only bare or rendered OpenCode workflow + names from target cancellation; keep same-repository cleanup unfiltered by + case-insensitive repository identity. +- **Evidence:** Corrective RED commits + `234d98dec14ae7a91819857f561b78d0d424ec98` and + `32a0d66cd1210f6fae1cb675265ce4ce49f63167`; focused local contract evidence + is not hosted authority, and fresh exact-head hosted checks remain required. diff --git a/scripts/ci/pr_review_merge_scheduler_core.py b/scripts/ci/pr_review_merge_scheduler_core.py index 9adcac3e37..08da1eecd9 100644 --- a/scripts/ci/pr_review_merge_scheduler_core.py +++ b/scripts/ci/pr_review_merge_scheduler_core.py @@ -3204,9 +3204,10 @@ def stale_pr_run_ids( pr: dict[str, Any], *, workflow: str | None = None, + excluded_workflows: frozenset[str] = frozenset(), statuses: Sequence[str] = ("queued", "in_progress"), ) -> list[str]: - """Return active run ids for older heads of the same pull request.""" + """Return older-head run ids except workflows owned by another repository.""" raw_head = pr.get("headRefOid") try: head = validate_git_sha(str(raw_head or "")).lower() @@ -3219,7 +3220,13 @@ 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: + run_name = str(run_data.get("name") or "") + if workflow is not None and run_name != workflow: + continue + if any( + run_name == candidate or run_name.startswith(f"{candidate} ") + for candidate in excluded_workflows + ): continue if str(run_data.get("head_sha") or "").lower() == head: continue @@ -3560,12 +3567,29 @@ def _review_run_still_superseded( def cancel_stale_pr_runs(repo: str, pr: dict[str, Any], *, dry_run: bool) -> list[str]: - """Force-cancel only direct-run candidates still proven stale at the destructive boundary.""" + """Cancel proven-stale direct runs except workflows owned by another repository.""" if dry_run: return [] require_github_actions_control_actor("force-cancel-stale-pr-runs") number = int(pr["number"]) - candidates = [str(run_id) for run_id in stale_pr_run_ids(repo, pr)] + dispatch_repo = repository_dispatch_target(repo) + excluded_workflows = ( + frozenset(OPENCODE_WORKFLOW_NAMES) + if dispatch_repo.casefold() != repo.casefold() + else frozenset() + ) + candidates = [ + str(run_id) + for run_id in ( + stale_pr_run_ids( + repo, + pr, + excluded_workflows=excluded_workflows, + ) + if excluded_workflows + else stale_pr_run_ids(repo, pr) + ) + ] def cancel_one(run_id: str) -> str | None: """Revalidate and cancel one direct workflow-run candidate when still stale.""" @@ -4270,6 +4294,8 @@ def inspect_pr( pass run(["gh", "pr", "close", str(number), "--repo", repo]) return Decision(number, "close_empty", "base 대비 실제 변경 0건") + # The target repository still owns CodeQL, security, and other direct PR + # runs. Only central-review names move to the dispatch repository. cancel_stale_pr_runs(repo, pr, dry_run=dry_run) if base_ref != base_branch: # Stacked/cascade PR (base is another feature branch). Org required diff --git a/tests/test_pr2005_central_inventory_boundary.py b/tests/test_pr2005_central_inventory_boundary.py new file mode 100644 index 0000000000..8d0340a326 --- /dev/null +++ b/tests/test_pr2005_central_inventory_boundary.py @@ -0,0 +1,164 @@ +"""Regression contracts for PR #2005's central review inventory boundary.""" + +from scripts.ci import pr_review_merge_scheduler_core as scheduler_core + + +def inspect_stacked_pull_request(repository, dispatch_repository, monkeypatch): + """Return the stale-run cleanup calls for one read-only stacked PR inspection.""" + cleanup_calls = [] + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + dispatch_repository, + ) + monkeypatch.setattr( + scheduler_core, + "cancel_stale_pr_runs", + lambda target_repository, pull_request, *, dry_run: cleanup_calls.append( + (target_repository, dry_run) + ), + ) + pull_request = { + "number": 1, + "isDraft": False, + "baseRefName": "feature-base", + "headRefOid": "a" * 40, + "files": {"totalCount": 1, "nodes": [{"path": "README.md"}]}, + "reviews": {"nodes": []}, + "reviewThreads": {"nodes": []}, + "statusCheckRollup": {"contexts": {"nodes": []}}, + "autoMergeRequest": None, + } + + scheduler_core.inspect_pr( + repository, + pull_request, + dry_run=True, + trigger_reviews=False, + enable_auto_merge_flag=False, + update_branches=False, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + return cleanup_calls + + +def test_central_dispatch_retains_target_cleanup(monkeypatch): + """Cross-repository dispatch must still invoke target-owned stale cleanup.""" + cleanup_calls = inspect_stacked_pull_request( + "owner/repo", + "ContextualWisdomLab/.github", + monkeypatch, + ) + + assert cleanup_calls == [("owner/repo", True)] + + +def test_same_repository_dispatch_keeps_unfiltered_cleanup_case_insensitively( + monkeypatch, +): + """Repository identity casing must not narrow same-repository cleanup.""" + cleanup_calls = inspect_stacked_pull_request( + "owner/repo", + "OWNER/REPO", + monkeypatch, + ) + + assert cleanup_calls == [("owner/repo", True)] + + +def test_cancel_stale_pr_runs_applies_central_review_filter_internally(monkeypatch): + """Existing callers keep their signature while cancellation scopes authority.""" + captured_exclusions = [] + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + monkeypatch.setattr( + scheduler_core, + "require_github_actions_control_actor", + lambda _action: None, + ) + + def stale_run_ids(_repo, _pull_request, *, excluded_workflows=frozenset()): + captured_exclusions.append(excluded_workflows) + return [] + + monkeypatch.setattr(scheduler_core, "stale_pr_run_ids", stale_run_ids) + + assert scheduler_core.cancel_stale_pr_runs( + "owner/repo", + {"number": 1, "headRefOid": "a" * 40}, + dry_run=False, + ) == [] + assert captured_exclusions == [ + frozenset(scheduler_core.OPENCODE_WORKFLOW_NAMES) + ] + + +def test_central_review_filter_preserves_target_security_runs(monkeypatch): + """Central review ownership must not preserve unrelated stale target runs.""" + current_head = "a" * 40 + stale_head = "b" * 40 + workflow_runs = [ + { + "id": 101, + "name": "Required OpenCode Review owner/repo#1@" + stale_head, + "head_sha": stale_head, + "pull_requests": [{"number": 1}], + }, + { + "id": 102, + "name": "Python Security", + "head_sha": stale_head, + "pull_requests": [{"number": 1}], + }, + { + "id": 103, + "name": "CodeQL", + "head_sha": stale_head, + "pull_requests": [{"number": 1}], + }, + ] + monkeypatch.setattr( + scheduler_core, + "active_workflow_runs", + lambda _repo, _statuses: workflow_runs, + ) + + assert scheduler_core.stale_pr_run_ids( + "owner/repo", + {"number": 1, "headRefOid": current_head}, + excluded_workflows=frozenset(scheduler_core.OPENCODE_WORKFLOW_NAMES), + ) == ["102", "103"] + + +def test_central_review_filter_matches_bare_and_rendered_names(monkeypatch): + """Both GitHub workflow names and rendered run names use central authority.""" + current_head = "a" * 40 + stale_head = "b" * 40 + workflow_runs = [ + { + "id": 201, + "name": "OpenCode Review", + "head_sha": stale_head, + "pull_requests": [{"number": 1}], + }, + { + "id": 202, + "name": "OpenCode Review Dispatch owner/repo#1@" + stale_head, + "head_sha": stale_head, + "pull_requests": [{"number": 1}], + }, + ] + monkeypatch.setattr( + scheduler_core, + "active_workflow_runs", + lambda _repo, _statuses: workflow_runs, + ) + + assert scheduler_core.stale_pr_run_ids( + "owner/repo", + {"number": 1, "headRefOid": current_head}, + excluded_workflows=frozenset(scheduler_core.OPENCODE_WORKFLOW_NAMES), + ) == []