From cddb6ac63dd63e40f496f81bf7284a31ab7419da Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:50:17 +0800 Subject: [PATCH] fix: retire stale strategy QPK pin proposals Co-Authored-By: Codex --- .../merge_verified_strategy_qpk_pin_prs.py | 69 ++++++++++++------- ...est_merge_verified_strategy_qpk_pin_prs.py | 39 +++++++++++ 2 files changed, 85 insertions(+), 23 deletions(-) diff --git a/scripts/merge_verified_strategy_qpk_pin_prs.py b/scripts/merge_verified_strategy_qpk_pin_prs.py index bd23239..321a664 100644 --- a/scripts/merge_verified_strategy_qpk_pin_prs.py +++ b/scripts/merge_verified_strategy_qpk_pin_prs.py @@ -286,6 +286,48 @@ def parse_args() -> argparse.Namespace: return parser.parse_args() +def process_repo( + repo: RepoSpec, + *, + qpk_sha: str, + close_superseded: bool, + env: dict[str, str], +) -> None: + """Queue the current pin when eligible, then retire verified older pins. + + A current generated PR may already have merged by the time the hourly + workflow runs again. Cleanup must therefore be independent of finding an + open current PR; otherwise historical proposal branches accumulate forever. + """ + + branch = expected_branch(repo, qpk_sha) + pr = open_pr_payload(repo, branch, env=env) + if pr is None: + print(f"{repo.name}: no current generated PR") + else: + reason = candidate_reason( + pr=pr, + changed_files=changed_files(repo, int(pr["number"]), env=env), + pyproject_text=pyproject_text(repo, pr["headRefOid"], env=env), + qpk_sha=qpk_sha, + ) + if reason is not None: + print(f"{repo.name}: skipped:{reason}") + else: + merge_candidate(repo, pr, env=env) + print(f"{repo.name}: queued:{pr['url']}") + + if 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)}") + + def main() -> int: args = parse_args() qpk_sha = args.qpk_sha.strip() @@ -299,31 +341,12 @@ def main() -> int: failures = 0 for repo in STRATEGY_REPOS: try: - branch = expected_branch(repo, qpk_sha) - pr = open_pr_payload(repo, branch, env=env) - if pr is None: - print(f"{repo.name}: no current generated PR") - continue - reason = candidate_reason( - pr=pr, - changed_files=changed_files(repo, int(pr["number"]), env=env), - pyproject_text=pyproject_text(repo, pr["headRefOid"], env=env), + process_repo( + repo, qpk_sha=qpk_sha, + close_superseded=args.close_superseded, + env=env, ) - if reason is not None: - print(f"{repo.name}: skipped:{reason}") - 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 bdf0ba8..1399a2c 100644 --- a/tests/test_merge_verified_strategy_qpk_pin_prs.py +++ b/tests/test_merge_verified_strategy_qpk_pin_prs.py @@ -4,6 +4,7 @@ ALLOWED_CHANGED_FILES, candidate_reason, expected_branch, + process_repo, superseded_pr_reason, ) from scripts.open_downstream_qpk_pin_prs import RepoSpec @@ -79,3 +80,41 @@ def test_only_a_recognized_older_generated_pr_can_be_closed() -> None: manual = _pr() manual["headRefName"] = "codex/manual-dependency-change" assert superseded_pr_reason(pr=manual, current_branch=current_branch) == "unexpected_branch" + + +def test_cleanup_runs_after_current_pin_has_already_merged(monkeypatch, capsys) -> None: + repo = RepoSpec("UsEquityStrategies") + monkeypatch.setattr( + "scripts.merge_verified_strategy_qpk_pin_prs.open_pr_payload", + lambda *_args, **_kwargs: None, + ) + observed: dict[str, object] = {} + + def close(repo_arg, *, current_branch, qpk_sha, env): + observed.update( + repo=repo_arg, + current_branch=current_branch, + qpk_sha=qpk_sha, + env=env, + ) + return ["123"] + + monkeypatch.setattr( + "scripts.merge_verified_strategy_qpk_pin_prs.close_superseded_candidates", + close, + ) + + process_repo( + repo, + qpk_sha=TARGET, + close_superseded=True, + env={"GH_TOKEN": "test"}, + ) + + assert observed["repo"] == repo + assert observed["current_branch"] == expected_branch(repo, TARGET) + assert observed["qpk_sha"] == TARGET + assert capsys.readouterr().out.splitlines() == [ + "UsEquityStrategies: no current generated PR", + "UsEquityStrategies: closed_superseded:123", + ]