diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 4bbff770..15331fbc 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -3920,6 +3920,8 @@ jobs: body="$(printf '%s\n' \ "## Pull request overview" \ "" \ + "OpenCode reviewed the current-head bounded evidence and found no blocking issues." \ + "" \ "$model_summary" \ "" \ "## Findings" \ @@ -7291,6 +7293,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 permissions: + actions: write contents: read pull-requests: read env: @@ -7299,12 +7302,12 @@ jobs: - name: Dispatch deferred same-head review retry after model-pool exhaustion env: GH_TOKEN: ${{ github.token }} - # Cross-repository dispatch of the central workflow needs a PAT; the - # target-repository runner token cannot dispatch workflows that live - # in the central .github repository, and the OpenCode app token has - # no Actions permission. - RETRY_DISPATCH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN }} - RETRY_DISPATCH_TOKEN_SOURCE: ${{ secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || 'none' }} + # Cross-repository dispatch needs a PAT. Inside the central repository + # itself, its runner token can dispatch the same trusted workflow. + # This token is only passed to `gh workflow run`; review writes remain + # exclusively authenticated by the OpenCode App token. + RETRY_DISPATCH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || (github.repository == 'ContextualWisdomLab/.github' && github.token) || '' }} + RETRY_DISPATCH_TOKEN_SOURCE: ${{ secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || github.repository == 'ContextualWisdomLab/.github' && 'github-token' || 'none' }} # One fixed backoff window before the single deferred retry. GitHub # Models per-minute throttles recover well inside this window; daily # quota exhaustion outlives any in-workflow delay and is the org diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index ae348732..ad7af29f 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -4,7 +4,7 @@ on: push: branches: [main, develop, master] pull_request_target: - types: [opened, synchronize, reopened, ready_for_review, auto_merge_enabled, closed] + types: [opened, synchronize, reopened, ready_for_review, closed] pull_request_review: types: [submitted, dismissed] workflow_run: diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index ab3c85cc..5e9ce835 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1349,23 +1349,15 @@ def dismiss_stale_opencode_change_requests(repo: str, pr: dict[str, Any], *, dry f"expected {expected_head}, observed {live_head or ''}" ) + dismissed = 0 for review_id in review_ids: message = ( "Superseded automated OpenCode change request from a previous head; " f"exact current head {expected_head} has a later OpenCode approval." ) - run( - [ - "gh", - "api", - "-X", - "PUT", - f"repos/{repo}/pulls/{number}/reviews/{review_id}/dismissals", - "-f", - f"message={message}", - ] - ) - return len(review_ids) + if dismiss_pull_request_review(repo, number, review_id, message=message): + dismissed += 1 + return dismissed def failed_status_checks(pr: dict[str, Any]) -> list[str]: @@ -1452,16 +1444,23 @@ def run_head_guarded_merge( auto: bool, ) -> None: """Run a head-guarded merge using an allowed repository merge method.""" + merge_flag = repository_auto_merge_flag(repo) if auto else "--squash" args = ["gh", "pr", "merge", number, "--repo", repo] if auto: args.append("--auto") - args.extend(["--squash", "--match-head-commit", head]) + print( + f"PR #{number}: enabling auto-merge with repository-enabled method " + f"{merge_flag.removeprefix('--')} at guarded head {head}." + ) + args.extend([merge_flag, "--match-head-commit", head]) try: run(args) return except RuntimeError as exc: detail = str(exc).lower() - if not any(marker in detail for marker in SQUASH_MERGE_DISABLED_MARKERS): + if merge_flag != "--squash" or not any( + marker in detail for marker in SQUASH_MERGE_DISABLED_MARKERS + ): raise reason = str(exc).splitlines()[-1][:400] @@ -1477,6 +1476,35 @@ def run_head_guarded_merge( run(merge_args) +def repository_auto_merge_flag(repo: str) -> str: + """Return the first merge method enabled in repository settings.""" + raw = run_github_read(["gh", "api", f"repos/{repo}"]) + try: + settings = json.loads(raw) + except json.JSONDecodeError as exc: + raise RuntimeError( + f"Could not select an auto-merge method for {repo}: " + "repository settings were not valid JSON." + ) from exc + if not isinstance(settings, dict): + raise RuntimeError( + f"Could not select an auto-merge method for {repo}: " + "repository settings were not an object." + ) + + for setting, flag in ( + ("allow_squash_merge", "--squash"), + ("allow_merge_commit", "--merge"), + ("allow_rebase_merge", "--rebase"), + ): + if settings.get(setting) is True: + return flag + raise RuntimeError( + f"Could not enable auto-merge for {repo}: repository settings expose " + "no enabled merge method." + ) + + def enable_auto_merge(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: """Enable auto-merge for a PR at its current head using an allowed method.""" number = str(pr["number"]) diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 0497bcec..e5a39ad5 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -8,6 +8,8 @@ import pytest +from scripts.ci.opencode_existing_approval_gate import PRIMARY_APPROVAL_MARKER + def test_code_reviewer_subagent_contract_is_configured(): """Guard the read-only code-reviewer subagent contract.""" @@ -1233,6 +1235,11 @@ def test_merge_scheduler_uses_escalating_mutation_credentials(): assert '--branch-update-limit "$ORG_SWEEP_BRANCH_UPDATE_LIMIT"' in workflow assert "pull_request_review:" in workflow assert "types: [submitted, dismissed]" in workflow + assert ( + "types: [opened, synchronize, reopened, ready_for_review, closed]" + in workflow + ) + assert "auto_merge_enabled" not in workflow assert ( "github.event_name == 'pull_request_review' && " "format('pr-{0}', github.event.pull_request.number)" in workflow @@ -1495,6 +1502,7 @@ def test_opencode_approve_review_publication_failure_keeps_gate_result(): " - name: Publish central OpenCode fast approval", 1 )[1].split(" - name: Publish OpenCode review outcome", 1)[0] assert "continue-on-error: true" in fast_approval + assert PRIMARY_APPROVAL_MARKER in fast_approval assert "def latest_peer_checks:" in fast_approval assert 'group_by([.app.slug // "", .name // ""])' in fast_approval assert fast_approval.count("latest_peer_checks") == 3 diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 530c273c..225a4974 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1557,6 +1557,10 @@ def test_actions_call_gh_with_expected_arguments(monkeypatch): def fake_run(args, stdin=None): calls.append(args) + if args == ["gh", "api", "repos/owner/repo"]: + return json.dumps( + {"allow_squash_merge": False, "allow_merge_commit": True} + ) if args[:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"]: return '{"workflow_runs": []}' return "" @@ -1580,10 +1584,12 @@ def fake_run(args, stdin=None): sched.update_branch("owner/repo", pr, dry_run=False) sched.dispatch_strix_evidence("owner/repo", "Strix Security Scan", pr, dry_run=False) sched.dispatch_opencode_review("owner/repo", "OpenCode Review", pr, dry_run=False) - assert calls[0][:4] == ["gh", "pr", "merge", "1"] - assert "--squash" in calls[0] - assert calls[0][-2:] == ["--match-head-commit", head_sha] - assert calls[1] == [ + assert calls[0] == ["gh", "api", "repos/owner/repo"] + assert calls[1][:4] == ["gh", "pr", "merge", "1"] + assert "--auto" in calls[1] + assert "--merge" in calls[1] + assert calls[1][-2:] == ["--match-head-commit", head_sha] + assert calls[2] == [ "gh", "pr", "merge", @@ -1594,13 +1600,13 @@ def fake_run(args, stdin=None): "--match-head-commit", head_sha, ] - assert calls[2] == ["gh", "pr", "merge", "1", "--repo", "owner/repo", "--disable-auto"] - assert calls[3][:4] == ["gh", "api", "-X", "PUT"] - assert calls[3][-1] == f"expected_head_sha={head_sha}" - assert calls[4][:5] == ["gh", "workflow", "run", "Strix Security Scan", "--repo"] - assert calls[5][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] + assert calls[3] == ["gh", "pr", "merge", "1", "--repo", "owner/repo", "--disable-auto"] + assert calls[4][:4] == ["gh", "api", "-X", "PUT"] + assert calls[4][-1] == f"expected_head_sha={head_sha}" + assert calls[5][:5] == ["gh", "workflow", "run", "Strix Security Scan", "--repo"] assert calls[6][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] - assert calls[7][:5] == ["gh", "workflow", "run", "OpenCode Review", "--repo"] + assert calls[7][:5] == ["gh", "api", "--method", "GET", "repos/owner/repo/actions/runs"] + assert calls[8][:5] == ["gh", "workflow", "run", "OpenCode Review", "--repo"] calls.clear() required_workflow_pr = make_pr( @@ -1702,6 +1708,11 @@ def fake_run(args, stdin=None): return "" monkeypatch.setattr(sched, "run", fake_run) + monkeypatch.setattr( + sched, + "run_github_read", + lambda _args: json.dumps({"allow_squash_merge": True}), + ) sched.run_head_guarded_merge( "owner/repo", @@ -1713,12 +1724,73 @@ def fake_run(args, stdin=None): assert len(calls) == 2 assert "--squash" in calls[0] assert "--merge" in calls[1] + assert ("--auto" in calls[0]) is auto assert ("--auto" in calls[1]) is auto assert calls[0][-2:] == ["--match-head-commit", head_sha] assert calls[1][-2:] == ["--match-head-commit", head_sha] assert "Squash merges are not allowed" in capsys.readouterr().out +@pytest.mark.parametrize( + ("settings", "expected_flag"), + [ + ({"allow_squash_merge": True, "allow_merge_commit": True}, "--squash"), + ({"allow_squash_merge": False, "allow_merge_commit": True}, "--merge"), + ( + { + "allow_squash_merge": False, + "allow_merge_commit": False, + "allow_rebase_merge": True, + }, + "--rebase", + ), + ], +) +def test_head_guarded_auto_merge_uses_repository_enabled_method( + monkeypatch, capsys, settings, expected_flag +): + calls = [] + head_sha = "a" * 40 + + monkeypatch.setattr(sched, "run_github_read", lambda _args: json.dumps(settings)) + monkeypatch.setattr(sched, "run", lambda args, stdin=None: calls.append(args) or "") + + sched.run_head_guarded_merge("owner/repo", "7", head_sha, auto=True) + + assert calls == [ + [ + "gh", + "pr", + "merge", + "7", + "--repo", + "owner/repo", + "--auto", + expected_flag, + "--match-head-commit", + head_sha, + ] + ] + assert expected_flag.removeprefix("--") in capsys.readouterr().out + + +@pytest.mark.parametrize( + ("payload", "message"), + [ + ("not-json", "not valid JSON"), + ("[]", "not an object"), + ("{}", "no enabled merge method"), + ], +) +def test_repository_auto_merge_flag_explains_invalid_settings( + monkeypatch, payload, message +): + monkeypatch.setattr(sched, "run_github_read", lambda _args: payload) + + with pytest.raises(RuntimeError, match=message): + sched.repository_auto_merge_flag("owner/repo") + + def test_head_guarded_merge_does_not_mask_unrelated_failure(monkeypatch): calls = [] @@ -2227,7 +2299,7 @@ def fake_graphql(query, **fields): assert all(query == sched.RESOLVE_REVIEW_THREAD_MUTATION for query, _ in calls) -def test_dismiss_stale_opencode_change_requests_is_current_head_guarded(monkeypatch): +def test_dismiss_stale_opencode_change_requests_is_current_head_guarded(monkeypatch, capsys): exact_head = "a" * 40 unapproved = make_pr( headRefOid=exact_head, @@ -2274,11 +2346,26 @@ def test_dismiss_stale_opencode_change_requests_is_current_head_guarded(monkeypa monkeypatch.setenv("GITHUB_ACTIONS", "true") monkeypatch.setenv("GH_TOKEN", "workflow-token") + states = iter([exact_head, "DISMISSED", "DISMISSED"]) + monkeypatch.setattr(sched, "run_github_read", lambda args, stdin=None: calls.append(args) or next(states)) assert sched.dismiss_stale_opencode_change_requests("owner/repo", pr, dry_run=False) == 2 assert calls[0] == ["gh", "api", "repos/owner/repo/pulls/1", "--jq", ".head.sha"] assert calls[1][:5] == ["gh", "api", "-X", "PUT", "repos/owner/repo/pulls/1/reviews/201/dismissals"] - assert calls[2][:5] == ["gh", "api", "-X", "PUT", "repos/owner/repo/pulls/1/reviews/202/dismissals"] - assert all(call[-2] == "-f" and call[-1].startswith("message=") for call in calls[1:]) + assert calls[2] == ["gh", "api", "repos/owner/repo/pulls/1/reviews/201", "--jq", ".state"] + assert calls[3][:5] == ["gh", "api", "-X", "PUT", "repos/owner/repo/pulls/1/reviews/202/dismissals"] + assert calls[4] == ["gh", "api", "repos/owner/repo/pulls/1/reviews/202", "--jq", ".state"] + assert all(call[-2] == "-f" and call[-1].startswith("message=") for call in (calls[1], calls[3])) + + calls.clear() + monkeypatch.setattr(sched, "run_github_read", lambda args, stdin=None: calls.append(args) or exact_head) + + def reject_dismissal(args, stdin=None): + calls.append(args) + raise RuntimeError("Branch protections do not permit dismissing this review (HTTP 403)") + + monkeypatch.setattr(sched, "run", reject_dismissal) + assert sched.dismiss_stale_opencode_change_requests("owner/repo", pr, dry_run=False) == 0 + assert "Branch protections do not permit dismissing this review (HTTP 403)" in capsys.readouterr().out calls.clear() monkeypatch.setattr(sched, "run_github_read", lambda args, stdin=None: calls.append(args) or ("d" * 40)) diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 089c73b2..ffbbdd78 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -53,6 +53,24 @@ def test_merge_scheduler_provides_same_repository_dispatch_credential() -> None: assert workflow.count("SCHEDULER_DISPATCH_TOKEN: ${{ github.token }}") == 2 +def test_opencode_exhausted_retry_uses_runner_token_only_for_central_dispatch() -> None: + """Keep same-repository retry live without weakening App-only review writes.""" + workflow = workflow_text("opencode-review.yml") + retry_job = workflow.split(" opencode-exhausted-retry:\n", 1)[1] + + assert "actions: write" in retry_job.split(" env:\n", 1)[0] + assert ( + "github.repository == 'ContextualWisdomLab/.github' && github.token" + in retry_job + ) + assert ( + "github.repository == 'ContextualWisdomLab/.github' && 'github-token'" + in retry_job + ) + assert "review writes remain" in retry_job + assert "GH_TOKEN=\"$RETRY_DISPATCH_TOKEN\" gh workflow run" in retry_job + + def test_required_pull_request_workflows_cancel_superseded_runs() -> None: for filename in ( "close-empty-pr.yml",