diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 4ab0c1b8..50d3cbca 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -4,7 +4,10 @@ on: push: branches: [main, develop, master] pull_request_target: - types: [opened, synchronize, reopened, ready_for_review, auto_merge_enabled, closed] + # Include retarget/auto-merge state changes so stacked PR chains are + # re-evaluated immediately when merge readiness shifts without a new head push. + # `enqueued` and `dequeued` are not valid pull_request_target activity types. + types: [opened, synchronize, reopened, edited, ready_for_review, auto_merge_enabled, auto_merge_disabled, closed] pull_request_review: types: [submitted, dismissed] workflow_run: diff --git a/scripts/ci/adversarial_evidence.py b/scripts/ci/adversarial_evidence.py index 4fbc4ce5..9f53ff54 100644 --- a/scripts/ci/adversarial_evidence.py +++ b/scripts/ci/adversarial_evidence.py @@ -58,8 +58,16 @@ def adversarial_evidence_rejection_reason( ) else: escaped_line = re.escape(str(line)) + # Accept "path:N", "path#LN", "path line N", "path, line N" (path-first), + # or "line N of path" / "line N in path" (natural English phrasing). + # The path may be followed by sentence punctuation (e.g., "... in path."). path_citation = re.search( - rf"{escaped_path}(?::|#L|\s+line\s+){escaped_line}\b", + rf"{escaped_path}(?::|#L|,?\s+line\s+){escaped_line}\b", + cleaned, + re.IGNORECASE, + ) or re.search( + rf"\bline\s+{escaped_line}\b\s+(?:of|in)\s+{escaped_path}" + r"(?:(?![A-Za-z0-9_./-])|(?=\.(?:$|[\s)\]}\",;:!?])))", cleaned, re.IGNORECASE, ) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index c13adfe9..86a33d2f 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1901,16 +1901,15 @@ def active_review_run_refs( for run_repo in repositories: for run_data in active_workflow_runs(run_repo, statuses): - run_name = str(run_data.get("name") or "") - if run_name != workflow and run_name not in workflow_aliases: - continue run_id = run_data.get("id") if not run_id: continue run_ref = (run_repo, str(run_id)) + run_event = str(run_data.get("event") or "").lower() display_title = str(run_data.get("display_title") or "") if ( - run_data.get("event") == "repository_dispatch" + run_repo == dispatch_repo + and run_event == "repository_dispatch" and display_title.startswith(dispatch_title_prefix) ): dispatched_head = display_title.removeprefix(dispatch_title_prefix).lower() @@ -1918,6 +1917,9 @@ def active_review_run_refs( continue (current if dispatched_head == head else stale).append(run_ref) continue + run_name = str(run_data.get("name") or "") + if run_name != workflow and run_name not in workflow_aliases: + continue if run_repo != target_repo: continue run_head = str(run_data.get("head_sha") or "").lower() diff --git a/tests/test_adversarial_evidence.py b/tests/test_adversarial_evidence.py index d5849012..7e0ffdec 100644 --- a/tests/test_adversarial_evidence.py +++ b/tests/test_adversarial_evidence.py @@ -10,6 +10,13 @@ def test_rejects_circular_adversarial_evidence(): ) +def test_rejects_negated_execution_evidence(): + assert "explicitly denies" in evidence.adversarial_evidence_rejection_reason( + "No test was run. .github/workflows/review.yml passed.", + ".github/workflows/review.yml", + ) + + def test_accepts_independent_proof_anchor_and_rejects_path_only(): assert ( evidence.adversarial_evidence_rejection_reason( @@ -86,6 +93,55 @@ def test_requires_the_exact_probe_path_and_line_when_line_is_supplied(): ) +def test_accepts_natural_english_line_of_path_citation(): + """'line N of path' and 'line N in path' are valid citation formats.""" + # "line N of path" form + assert ( + evidence.adversarial_evidence_rejection_reason( + f"Source trace at line 42 of .github/workflows/review.yml confirmed the branch was rejected. {SOURCE_RECEIPT}", + ".github/workflows/review.yml", + 42, + ) + is None + ), "should accept 'line N of path' citation" + # "line N in path" form + assert ( + evidence.adversarial_evidence_rejection_reason( + f"Checked line 42 in .github/workflows/review.yml and observed it was rejected. {SOURCE_RECEIPT}", + ".github/workflows/review.yml", + 42, + ) + is None + ), "should accept 'line N in path' citation" + # sentence punctuation after path should still be accepted + assert ( + evidence.adversarial_evidence_rejection_reason( + f"Checked line 42 in .github/workflows/review.yml. observed it was rejected. {SOURCE_RECEIPT}", + ".github/workflows/review.yml", + 42, + ) + is None + ), "should accept trailing sentence punctuation after path" + # wrong line number still rejected + assert "must cite" in evidence.adversarial_evidence_rejection_reason( + f"Line 99 of .github/workflows/review.yml confirmed. {SOURCE_RECEIPT}", + ".github/workflows/review.yml", + 42, + ), "wrong line number must still be rejected" + # prefix boundary still enforced + assert "must cite" in evidence.adversarial_evidence_rejection_reason( + f"line 42 of prefix.github/workflows/review.yml confirmed. {SOURCE_RECEIPT}", + ".github/workflows/review.yml", + 42, + ), "leading path boundary must still be enforced" + # longer suffix paths remain invalid + assert "must cite" in evidence.adversarial_evidence_rejection_reason( + f"line 42 in .github/workflows/review.yml.backup confirmed. {SOURCE_RECEIPT}", + ".github/workflows/review.yml", + 42, + ), "path suffix continuation must still be rejected" + + def test_path_only_citation_rejects_longer_path_substrings(): """A filename embedded inside another path is not an exact citation.""" assert "exact probe path" in evidence.adversarial_evidence_rejection_reason( diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 1fe10da7..3083328c 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -2078,7 +2078,7 @@ def test_dispatch_opencode_review_deduplicates_current_head_repository_dispatch( head_sha = "a" * 40 current_dispatch = { "id": 9100, - "name": "Required OpenCode Review", + "name": f"Required OpenCode Review owner/repo#1@{head_sha}", "event": "repository_dispatch", "head_sha": "default-branch-sha", "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", @@ -2132,7 +2132,7 @@ def test_dispatch_strix_cancels_stale_central_run_and_keeps_current(monkeypatch, central_runs = [ { "id": 9300, - "name": "Strix Security Scan", + "name": f"Strix Security Scan owner/repo#1@{stale_sha}", "event": "repository_dispatch", "head_sha": "default-branch-sha", "display_title": f"Strix Security Scan owner/repo#1@{stale_sha}", @@ -2140,7 +2140,7 @@ def test_dispatch_strix_cancels_stale_central_run_and_keeps_current(monkeypatch, }, { "id": 9301, - "name": "Strix Security Scan", + "name": f"Strix Security Scan owner/repo#1@{head_sha}", "event": "repository_dispatch", "head_sha": "default-branch-sha", "display_title": f"Strix Security Scan owner/repo#1@{head_sha}", @@ -2200,7 +2200,7 @@ def test_central_run_filter_ignores_malformed_and_non_dispatch_titles(monkeypatc central_runs = [ { "id": 9400, - "name": "Required OpenCode Review", + "name": "Required OpenCode Review owner/repo#1@not-a-sha", "event": "repository_dispatch", "display_title": "Required OpenCode Review owner/repo#1@not-a-sha", "pull_requests": [], @@ -2232,6 +2232,93 @@ def fake_active_runs(repo, statuses=("queued", "in_progress")): assert sched.force_cancel_workflow_runs("owner/repo", []) == {} +def test_active_review_run_refs_match_dynamic_run_names_from_actions_api(monkeypatch): + head_sha = "a" * 40 + current_run = { + "id": 9500, + "name": f"Required OpenCode Review owner/repo#1@{head_sha}", + "event": "repository_dispatch", + "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", + "head_sha": "default-branch-sha", + "pull_requests": [], + } + + def fake_active_runs(repo, statuses=("queued", "in_progress")): + del statuses + if repo == "ContextualWisdomLab/.github": + return [current_run] + return [] + + monkeypatch.setattr( + sched, + "active_workflow_runs", + fake_active_runs, + ) + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + + assert sched.active_opencode_run_refs( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid=head_sha), + ) == ([("ContextualWisdomLab/.github", "9500")], []) + + +def test_active_review_run_refs_ignore_dynamic_titles_from_pull_request_target(monkeypatch): + head_sha = "a" * 40 + current_run = { + "id": 9501, + "name": f"Required OpenCode Review owner/repo#1@{head_sha}", + "event": "pull_request_target", + "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", + "head_sha": head_sha, + "pull_requests": [{"number": 1}], + } + monkeypatch.setattr( + sched, + "active_workflow_runs", + lambda repo, statuses=("queued", "in_progress"): [current_run], + ) + + assert sched.active_opencode_run_refs( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid=head_sha), + ) == ([], []) + + +def test_active_review_run_refs_ignore_target_repo_dispatch_when_central_repo_is_configured(monkeypatch): + head_sha = "a" * 40 + target_repo_dispatch_run = { + "id": 9502, + "name": f"Required OpenCode Review owner/repo#1@{head_sha}", + "event": "repository_dispatch", + "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", + "head_sha": "target-default-branch-sha", + "pull_requests": [], + } + + def fake_active_runs(repo, statuses=("queued", "in_progress")): + del statuses + if repo == "owner/repo": + return [target_repo_dispatch_run] + return [] + + monkeypatch.setattr(sched, "active_workflow_runs", fake_active_runs) + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + + assert sched.active_opencode_run_refs( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid=head_sha), + ) == ([], []) + + def test_active_run_filters_and_stale_opencode_dry_run(monkeypatch): runs = [ { diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 8e66277f..1d070bba 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -38,6 +38,42 @@ def test_merge_scheduler_dispatches_one_review_by_default() -> None: ) +def test_merge_scheduler_pull_request_target_includes_retarget_and_automerge_events() -> None: + import re # noqa: PLC0415 + + workflow = workflow_text("pr-review-merge-scheduler.yml") + # Match the inline types list under pull_request_target (DOTALL to skip comment lines). + m = re.search( + r"pull_request_target:.*?types:\s*\[([^\]]+)\]", + workflow, + re.DOTALL, + ) + assert m is not None, "pull_request_target.types list not found in scheduler workflow" + trigger_types = {t.strip() for t in m.group(1).split(",") if t.strip()} + + # Stacked-PR retarget and auto-merge transitions that must fire the scheduler. + required = { + "opened", + "synchronize", + "reopened", + "edited", # retarget: base branch change fires this + "ready_for_review", + "auto_merge_enabled", + "auto_merge_disabled", + "closed", + } + assert required.issubset(trigger_types), ( + f"Missing pull_request_target trigger types: {required - trigger_types}" + ) + # enqueued/dequeued are not valid pull_request_target activity types. + assert "enqueued" not in trigger_types, ( + "`enqueued` is not a valid pull_request_target activity type" + ) + assert "dequeued" not in trigger_types, ( + "`dequeued` is not a valid pull_request_target activity type" + ) + + def test_merge_scheduler_provides_same_repository_dispatch_credential() -> None: """Guard the runner-token dispatch credential for central review workflows.