diff --git a/.github/workflows/open-downstream-qpk-pin-prs.yml b/.github/workflows/open-downstream-qpk-pin-prs.yml index 4287b88..e28900b 100644 --- a/.github/workflows/open-downstream-qpk-pin-prs.yml +++ b/.github/workflows/open-downstream-qpk-pin-prs.yml @@ -79,7 +79,7 @@ jobs: run: | set -euo pipefail qpk_sha=$(tr -d '[:space:]' < QPK_PIN) - python3 scripts/merge_verified_strategy_qpk_pin_prs.py --qpk-sha "$qpk_sha" + python3 scripts/merge_verified_strategy_qpk_pin_prs.py --qpk-sha "$qpk_sha" --close-superseded - name: Summarize missing token if: steps.sync.outputs.missing_token == 'true' diff --git a/.github/workflows/update-qpk-pin.yml b/.github/workflows/update-qpk-pin.yml index 9bcac25..feccbc2 100644 --- a/.github/workflows/update-qpk-pin.yml +++ b/.github/workflows/update-qpk-pin.yml @@ -12,7 +12,9 @@ on: - ".github/workflows/update-qpk-pin.yml" - "scripts/check_qpk_pin_consistency.py" - "scripts/open_downstream_qpk_pin_prs.py" + - "scripts/merge_verified_strategy_qpk_pin_prs.py" - "tests/test_qpk_pin_consistency.py" + - "tests/test_merge_verified_strategy_qpk_pin_prs.py" - "tests/test_update_qpk_pin_workflow.py" - "docs/**" - "**.md" diff --git a/scripts/merge_verified_strategy_qpk_pin_prs.py b/scripts/merge_verified_strategy_qpk_pin_prs.py index 0ec32e6..bd23239 100644 --- a/scripts/merge_verified_strategy_qpk_pin_prs.py +++ b/scripts/merge_verified_strategy_qpk_pin_prs.py @@ -40,6 +40,8 @@ re.IGNORECASE, ) AUTOMATION_AUTHOR = "Pigbibi" +GENERATED_BRANCH_PREFIX = "auto/qpk-pin-sync-" +GENERATED_TITLE_PREFIX = "chore(deps): align QPK pin to " def run(command: list[str], *, env: dict[str, str]) -> subprocess.CompletedProcess[str]: @@ -63,7 +65,7 @@ def candidate_reason( ) -> str | None: """Return a fail-closed reason when a generated PR is not mergeable.""" - expected_title = f"chore(deps): align QPK pin to {qpk_sha[:12]}" + expected_title = f"{GENERATED_TITLE_PREFIX}{qpk_sha[:12]}" author = (pr.get("author") or {}).get("login") if author != AUTOMATION_AUTHOR: return "unexpected_author" @@ -86,6 +88,24 @@ def candidate_reason( return None +def superseded_pr_reason(*, pr: dict[str, Any], current_branch: str) -> str | None: + """Return a reason unless this is a safely recognizable obsolete PR.""" + + author = (pr.get("author") or {}).get("login") + if author != AUTOMATION_AUTHOR: + return "unexpected_author" + if pr.get("baseRefName") != "main" or pr.get("isCrossRepository") or pr.get("isDraft"): + return "unexpected_pr_target" + head_ref = pr.get("headRefName") or "" + if head_ref == current_branch: + return "current_branch" + if not head_ref.startswith(GENERATED_BRANCH_PREFIX): + return "unexpected_branch" + if not (pr.get("title") or "").startswith(GENERATED_TITLE_PREFIX): + return "unexpected_title" + return None + + def open_pr_payload(repo: RepoSpec, branch: str, *, env: dict[str, str]) -> dict[str, Any] | None: payload = _json( [ @@ -109,18 +129,45 @@ def open_pr_payload(repo: RepoSpec, branch: str, *, env: dict[str, str]) -> dict raise RuntimeError(f"ambiguous_generated_prs:count={len(payload)}") pr_number = str(payload[0]["number"]) return _json( + pr_view_command(repo, pr_number), + env=env, + ) + + +def pr_view_command(repo: RepoSpec, pr_number: str) -> list[str]: + return [ + "gh", + "pr", + "view", + pr_number, + "--repo", + f"QuantStrategyLab/{repo.name}", + "--json", + "author,baseRefName,headRefName,headRefOid,isCrossRepository,isDraft,number,statusCheckRollup,title,url", + ] + + +def open_generated_prs(repo: RepoSpec, *, env: dict[str, str]) -> list[dict[str, Any]]: + payload = _json( [ "gh", "pr", - "view", - pr_number, + "list", "--repo", f"QuantStrategyLab/{repo.name}", + "--state", + "open", + "--limit", + "100", "--json", - "author,baseRefName,headRefName,headRefOid,isCrossRepository,isDraft,number,statusCheckRollup,title,url", + "number", ], env=env, ) + return [ + _json(pr_view_command(repo, str(item["number"])), env=env) + for item in payload + ] def changed_files(repo: RepoSpec, pr_number: int, *, env: dict[str, str]) -> list[str]: @@ -171,10 +218,71 @@ def merge_candidate(repo: RepoSpec, pr: dict[str, Any], *, env: dict[str, str]) ) +def qpk_ref_from_pyproject(text: str) -> str | None: + refs = set(QPK_REF_RE.findall(text)) + return next(iter(refs)) if len(refs) == 1 else None + + +def is_main_history_ancestor(*, candidate_sha: str, qpk_sha: str, env: dict[str, str]) -> bool: + status = run( + [ + "gh", + "api", + f"repos/QuantStrategyLab/QuantPlatformKit/compare/{candidate_sha}...{qpk_sha}", + "--jq", + ".status", + ], + env=env, + ).stdout.strip() + return status == "ahead" + + +def close_superseded_candidates( + repo: RepoSpec, + *, + current_branch: str, + qpk_sha: str, + env: dict[str, str], +) -> list[str]: + """Close only fully recognizable older pins that lead to the current pin.""" + + results: list[str] = [] + for pr in open_generated_prs(repo, env=env): + reason = superseded_pr_reason(pr=pr, current_branch=current_branch) + if reason is not None: + continue + old_qpk_sha = qpk_ref_from_pyproject(pyproject_text(repo, pr["headRefOid"], env=env)) + if old_qpk_sha is None or not is_main_history_ancestor( + candidate_sha=old_qpk_sha, + qpk_sha=qpk_sha, + env=env, + ): + continue + run( + [ + "gh", + "pr", + "close", + str(pr["number"]), + "--repo", + f"QuantStrategyLab/{repo.name}", + "--delete-branch", + ], + env=env, + ) + results.append(str(pr["number"])) + return results + + def parse_args() -> argparse.Namespace: parser = argparse.ArgumentParser(description="Merge verified generated strategy QPK pin PRs.") parser.add_argument("--qpk-sha", required=True) parser.add_argument("--token-env", default="QSL_REPO_SYNC_TOKEN") + parser.add_argument( + "--close-superseded", + action="store_true", + help="Close only verified older generated strategy pin PRs after a current PR is green.", + ) return parser.parse_args() @@ -207,6 +315,15 @@ def main() -> int: continue merge_candidate(repo, pr, env=env) print(f"{repo.name}: queued:{pr['url']}") + if args.close_superseded: + closed = close_superseded_candidates( + repo, + current_branch=branch, + qpk_sha=qpk_sha, + env=env, + ) + if closed: + print(f"{repo.name}: closed_superseded:{','.join(closed)}") except (RuntimeError, subprocess.CalledProcessError, ValueError, UnicodeDecodeError) as exc: failures += 1 if isinstance(exc, subprocess.CalledProcessError): diff --git a/tests/test_merge_verified_strategy_qpk_pin_prs.py b/tests/test_merge_verified_strategy_qpk_pin_prs.py index b9557b8..bdf0ba8 100644 --- a/tests/test_merge_verified_strategy_qpk_pin_prs.py +++ b/tests/test_merge_verified_strategy_qpk_pin_prs.py @@ -1,6 +1,11 @@ from __future__ import annotations -from scripts.merge_verified_strategy_qpk_pin_prs import ALLOWED_CHANGED_FILES, candidate_reason, expected_branch +from scripts.merge_verified_strategy_qpk_pin_prs import ( + ALLOWED_CHANGED_FILES, + candidate_reason, + expected_branch, + superseded_pr_reason, +) from scripts.open_downstream_qpk_pin_prs import RepoSpec @@ -13,6 +18,7 @@ def _pr(*, checks: list[dict[str, str]] | None = None) -> dict[str, object]: "baseRefName": "main", "isCrossRepository": False, "isDraft": False, + "headRefName": "auto/qpk-pin-sync-8378e939d932-usequitystrategies", "title": f"chore(deps): align QPK pin to {TARGET[:12]}", "statusCheckRollup": checks if checks is not None @@ -61,3 +67,15 @@ def test_strategy_pin_pr_fails_closed_for_unexpected_changes_or_ci() -> None: pyproject_text=_pyproject("37c81901160c5b31127a27dba1c63944933fb6bf"), qpk_sha=TARGET, ) == "qpk_pin_mismatch" + + +def test_only_a_recognized_older_generated_pr_can_be_closed() -> None: + current_branch = "auto/qpk-pin-sync-8378e939d932-usequitystrategies" + older = _pr() + older["headRefName"] = "auto/qpk-pin-sync-37c81901160c-usequitystrategies" + assert superseded_pr_reason(pr=older, current_branch=current_branch) is None + + assert superseded_pr_reason(pr=_pr(), current_branch=current_branch) == "current_branch" + manual = _pr() + manual["headRefName"] = "codex/manual-dependency-change" + assert superseded_pr_reason(pr=manual, current_branch=current_branch) == "unexpected_branch" diff --git a/tests/test_update_qpk_pin_workflow.py b/tests/test_update_qpk_pin_workflow.py index 403f54a..3a88c50 100644 --- a/tests/test_update_qpk_pin_workflow.py +++ b/tests/test_update_qpk_pin_workflow.py @@ -237,7 +237,9 @@ def test_dependency_success_reaches_only_guarded_pr_step(tmp_path: Path) -> None ) in workflow assert ' - ".github/workflows/update-qpk-pin.yml"' in workflow assert ' - "scripts/open_downstream_qpk_pin_prs.py"' in workflow + assert ' - "scripts/merge_verified_strategy_qpk_pin_prs.py"' in workflow assert ' - "tests/test_qpk_pin_consistency.py"' in workflow + assert ' - "tests/test_merge_verified_strategy_qpk_pin_prs.py"' in workflow assert ' - "tests/test_update_qpk_pin_workflow.py"' in workflow assert "workflow_dispatch:" not in workflow @@ -255,7 +257,7 @@ def test_downstream_rollout_is_scheduled_and_phase_gated() -> None: assert "peter-evans/create-pull-request@22a9089034f40e5a961c8808d113e2c98fb63676" in workflow assert "peter-evans/create-pull-request@v7" not in workflow assert "Queue verified strategy pin PRs" in workflow - assert "merge_verified_strategy_qpk_pin_prs.py --qpk-sha" in workflow + assert "merge_verified_strategy_qpk_pin_prs.py --qpk-sha \"$qpk_sha\" --close-superseded" in workflow def test_staged_pin_auto_advance_is_limited_to_verified_machine_prs() -> None: