Skip to content

Commit 72fb0aa

Browse files
authored
fix: prune stale tracking refs before pushing so re-split branches are not rejected as stale
1 parent 46453bb commit 72fb0aa

7 files changed

Lines changed: 236 additions & 6 deletions

File tree

pr_split/cli.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
fetch_fork_pr,
6464
is_worktree_clean,
6565
merge_base,
66+
prune_remote_tracking_refs,
6667
push_branch,
6768
remove_worktree,
6869
)
@@ -463,6 +464,11 @@ def _push_and_create_prs(
463464
record_map = {r.group_id: r for r in branch_records}
464465
errors: list[tuple[str, Exception]] = []
465466

467+
# Branch names are reused across runs; drop tracking refs left by an
468+
# earlier split whose remote branch was merged or deleted since, or
469+
# force-with-lease rejects every push as stale.
470+
prune_remote_tracking_refs()
471+
466472
# Children target parent branches, so every branch is pushed before any PR opens.
467473
with ThreadPoolExecutor(max_workers=_PUSH_MAX_WORKERS) as executor:
468474
push_futures = {

pr_split/git_ops/__init__.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
from .branches import (
2929
merge_base as merge_base,
3030
)
31+
from .branches import (
32+
prune_remote_tracking_refs as prune_remote_tracking_refs,
33+
)
3134
from .branches import (
3235
push_branch as push_branch,
3336
)

pr_split/git_ops/branches.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import contextlib
34
import os
45
import re
56
import subprocess
@@ -110,10 +111,36 @@ def delete_branch(branch: str, *, remote: bool = False) -> None:
110111
if _REMOTE_REF_MISSING not in str(exc):
111112
raise
112113
logger.info(logs.BRANCH_ALREADY_GONE.format(branch=branch, where=" on origin"))
114+
# Either way the remote branch is gone; drop the local tracking ref
115+
# too, or the next `push --force-with-lease` of a reused branch name
116+
# is rejected as "stale info" against a ref origin no longer has.
117+
forget_remote_tracking_ref(branch)
113118
if local_error is not None:
114119
raise local_error
115120

116121

122+
def forget_remote_tracking_ref(branch: str) -> None:
123+
with contextlib.suppress(GitOperationError):
124+
run_git("update-ref", "-d", f"refs/remotes/origin/{branch}")
125+
126+
127+
def prune_remote_tracking_refs() -> None:
128+
"""Drop tracking refs whose remote branch no longer exists.
129+
130+
Split branch names are reused across runs; after a merge or `clean` the
131+
remote branch is gone but `refs/remotes/origin/pr-split/...` may still
132+
point at the old head, and every `push --force-with-lease` would be
133+
rejected as stale. Only *gone* refs are dropped (`git remote prune`, no
134+
fetch): refreshing the refs that still exist would make the lease
135+
compare against origin's current tip and silently force over commits
136+
someone else pushed to a reused branch.
137+
"""
138+
try:
139+
run_git("remote", "prune", "origin")
140+
except GitOperationError as exc:
141+
logger.warning(logs.PRUNE_FAILED.format(error=exc))
142+
143+
117144
def merge_base(ref_a: str, ref_b: str) -> str:
118145
return run_git("merge-base", ref_a, ref_b)
119146

pr_split/logs.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,7 @@
7474
PR_SKIPPED_BASE_NOT_PUSHED = (
7575
"Skipping PR for group '{group}': its base branch '{base}' was not pushed"
7676
)
77+
PRUNE_FAILED = (
78+
"Could not prune origin's tracking refs before pushing ({error}); "
79+
"a reused branch name may be rejected as stale"
80+
)

tests/test_cli_helpers.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,3 +627,19 @@ def push(branch: str) -> None:
627627
with pytest.raises(PRSplitError):
628628
_push_and_create_prs(groups, records)
629629
assert mock_create.call_count == 0
630+
631+
632+
class TestPushPrunesFirst:
633+
@patch("pr_split.cli.create_pr", return_value=(1, "https://github.com/pr/1"))
634+
@patch("pr_split.cli.push_branch")
635+
@patch("pr_split.cli.prune_remote_tracking_refs")
636+
def test_tracking_refs_are_refreshed_before_any_push(
637+
self, mock_prune: MagicMock, mock_push: MagicMock, mock_create: MagicMock
638+
) -> None:
639+
order: list[str] = []
640+
mock_prune.side_effect = lambda: order.append("prune")
641+
mock_push.side_effect = lambda branch: order.append("push")
642+
_push_and_create_prs(
643+
[_group("pr-1", "feat: a")], [_branch_record("pr-1", "pr-split/ns/pr-1")]
644+
)
645+
assert order == ["prune", "push"]

tests/test_cli_new_features.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,7 @@ def test_close_failure_warning_includes_the_reason(
401401

402402

403403
class TestCleanupAfterMergeDeletedBranches:
404+
@patch("pr_split.git_ops.branches.forget_remote_tracking_ref")
404405
@patch("pr_split.cli.get_pr_state", return_value={"state": "MERGED"})
405406
@patch("pr_split.cli.shutil.rmtree")
406407
@patch("pr_split.cli.Path")
@@ -413,6 +414,7 @@ def test_fully_merged_split_cleans_up_completely(
413414
mock_path: MagicMock,
414415
mock_rmtree: MagicMock,
415416
mock_state: MagicMock,
417+
mock_forget: MagicMock,
416418
) -> None:
417419
mock_path.return_value.exists.return_value = True
418420
# merge --delete-branch already removed both local and remote branches

tests/test_git_branches.py

Lines changed: 178 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,18 @@ def test_local_only(self, mock_git: MagicMock) -> None:
125125
delete_branch("pr-split/pr-1")
126126
mock_git.assert_called_once_with("branch", "-D", "pr-split/pr-1")
127127

128+
@patch("pr_split.git_ops.branches.forget_remote_tracking_ref")
128129
@patch("pr_split.git_ops.branches.run_git")
129-
def test_with_remote(self, mock_git: MagicMock) -> None:
130+
def test_with_remote(self, mock_git: MagicMock, mock_forget: MagicMock) -> None:
130131
mock_git.return_value = ""
131132
delete_branch("pr-split/pr-1", remote=True)
132133
assert mock_git.call_count == 2
133134

135+
@patch("pr_split.git_ops.branches.forget_remote_tracking_ref")
134136
@patch("pr_split.git_ops.branches.run_git")
135-
def test_local_failure_still_deletes_remote(self, mock_git: MagicMock) -> None:
137+
def test_local_failure_still_deletes_remote(
138+
self, mock_git: MagicMock, mock_forget: MagicMock
139+
) -> None:
136140
mock_git.side_effect = [GitOperationError("checked out"), ""]
137141
with pytest.raises(GitOperationError, match="checked out"):
138142
delete_branch("pr-split/pr-1", remote=True)
@@ -145,15 +149,21 @@ def test_local_failure_without_remote_raises_immediately(self, mock_git: MagicMo
145149
delete_branch("pr-split/pr-1")
146150
mock_git.assert_called_once()
147151

152+
@patch("pr_split.git_ops.branches.forget_remote_tracking_ref")
148153
@patch("pr_split.git_ops.branches.run_git")
149-
def test_branch_already_deleted_locally_counts_as_deleted(self, mock_git: MagicMock) -> None:
154+
def test_branch_already_deleted_locally_counts_as_deleted(
155+
self, mock_git: MagicMock, mock_forget: MagicMock
156+
) -> None:
150157
# `pr-split merge` deletes the local branch; cleanup must not fail on it.
151158
mock_git.side_effect = [GitOperationError("error: branch 'pr-split/pr-1' not found."), ""]
152159
delete_branch("pr-split/pr-1", remote=True)
153160
mock_git.assert_any_call("push", "origin", "--delete", "pr-split/pr-1")
154161

162+
@patch("pr_split.git_ops.branches.forget_remote_tracking_ref")
155163
@patch("pr_split.git_ops.branches.run_git")
156-
def test_branch_already_deleted_on_origin_counts_as_deleted(self, mock_git: MagicMock) -> None:
164+
def test_branch_already_deleted_on_origin_counts_as_deleted(
165+
self, mock_git: MagicMock, mock_forget: MagicMock
166+
) -> None:
157167
mock_git.side_effect = [
158168
"",
159169
GitOperationError(
@@ -162,8 +172,11 @@ def test_branch_already_deleted_on_origin_counts_as_deleted(self, mock_git: Magi
162172
]
163173
delete_branch("pr-split/pr-1", remote=True)
164174

175+
@patch("pr_split.git_ops.branches.forget_remote_tracking_ref")
165176
@patch("pr_split.git_ops.branches.run_git")
166-
def test_branch_gone_everywhere_counts_as_deleted(self, mock_git: MagicMock) -> None:
177+
def test_branch_gone_everywhere_counts_as_deleted(
178+
self, mock_git: MagicMock, mock_forget: MagicMock
179+
) -> None:
167180
mock_git.side_effect = [
168181
GitOperationError("error: branch 'pr-split/pr-1' not found."),
169182
GitOperationError(
@@ -177,8 +190,11 @@ def test_missing_local_branch_without_remote_is_fine(self, mock_git: MagicMock)
177190
mock_git.side_effect = GitOperationError("error: branch 'pr-split/pr-1' not found.")
178191
delete_branch("pr-split/pr-1")
179192

193+
@patch("pr_split.git_ops.branches.forget_remote_tracking_ref")
180194
@patch("pr_split.git_ops.branches.run_git")
181-
def test_other_remote_failure_still_raises(self, mock_git: MagicMock) -> None:
195+
def test_other_remote_failure_still_raises(
196+
self, mock_git: MagicMock, mock_forget: MagicMock
197+
) -> None:
182198
mock_git.side_effect = ["", GitOperationError("fatal: could not read from remote")]
183199
with pytest.raises(GitOperationError, match="could not read from remote"):
184200
delete_branch("pr-split/pr-1", remote=True)
@@ -399,3 +415,159 @@ def test_localised_git_still_reports_already_deleted(
399415
monkeypatch.setenv("LANGUAGE", "de")
400416
# never existed: must be treated as already deleted, not as an error
401417
delete_branch("pr-split/ns/never")
418+
419+
420+
class TestStaleRemoteTrackingRefs:
421+
def _repo_with_origin(self, tmp_path: Path) -> Path:
422+
origin = tmp_path / "origin.git"
423+
subprocess.run(["git", "init", "-q", "--bare", str(origin)], check=True)
424+
repo = tmp_path / "repo"
425+
subprocess.run(["git", "clone", "-q", str(origin), str(repo)], check=True)
426+
subprocess.run(
427+
[
428+
"git",
429+
"-c",
430+
"user.name=t",
431+
"-c",
432+
"user.email=t@x",
433+
"commit",
434+
"-q",
435+
"--allow-empty",
436+
"-m",
437+
"base",
438+
],
439+
cwd=repo,
440+
check=True,
441+
)
442+
subprocess.run(["git", "push", "-q", "-u", "origin", "HEAD:main"], cwd=repo, check=True)
443+
return repo
444+
445+
def test_reused_branch_name_pushes_again_after_remote_deletion(
446+
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
447+
) -> None:
448+
from pr_split.git_ops.branches import prune_remote_tracking_refs, push_branch
449+
450+
repo = self._repo_with_origin(tmp_path)
451+
monkeypatch.chdir(repo)
452+
subprocess.run(["git", "branch", "pr-split/ns/pr-1"], cwd=repo, check=True)
453+
push_branch("pr-split/ns/pr-1")
454+
# Simulate `gh pr merge --delete-branch` / GitHub deleting the head
455+
# branch behind our back: the remote branch goes away but the local
456+
# tracking ref still points at the old head.
457+
subprocess.run(
458+
["git", "update-ref", "-d", "refs/heads/pr-split/ns/pr-1"],
459+
cwd=tmp_path / "origin.git",
460+
check=True,
461+
)
462+
subprocess.run(
463+
[
464+
"git",
465+
"-c",
466+
"user.name=t",
467+
"-c",
468+
"user.email=t@x",
469+
"commit",
470+
"-q",
471+
"--allow-empty",
472+
"-m",
473+
"v2",
474+
],
475+
cwd=repo,
476+
check=True,
477+
)
478+
subprocess.run(["git", "branch", "-f", "pr-split/ns/pr-1", "HEAD"], cwd=repo, check=True)
479+
with pytest.raises(GitOperationError, match="stale info"):
480+
push_branch("pr-split/ns/pr-1")
481+
482+
prune_remote_tracking_refs()
483+
push_branch("pr-split/ns/pr-1")
484+
485+
def test_prune_keeps_the_lease_against_third_party_pushes(
486+
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
487+
) -> None:
488+
from pr_split.git_ops.branches import prune_remote_tracking_refs, push_branch
489+
490+
repo = self._repo_with_origin(tmp_path)
491+
monkeypatch.chdir(repo)
492+
subprocess.run(["git", "branch", "pr-split/ns/pr-9"], cwd=repo, check=True)
493+
push_branch("pr-split/ns/pr-9")
494+
# A reviewer pushes a fixup to the split branch from another clone.
495+
other = tmp_path / "other"
496+
subprocess.run(
497+
["git", "clone", "-q", str(tmp_path / "origin.git"), str(other)], check=True
498+
)
499+
subprocess.run(["git", "checkout", "-q", "pr-split/ns/pr-9"], cwd=other, check=True)
500+
subprocess.run(
501+
[
502+
"git",
503+
"-c",
504+
"user.name=r",
505+
"-c",
506+
"user.email=r@x",
507+
"commit",
508+
"-q",
509+
"--allow-empty",
510+
"-m",
511+
"fixup",
512+
],
513+
cwd=other,
514+
check=True,
515+
)
516+
subprocess.run(["git", "push", "-q", "origin", "pr-split/ns/pr-9"], cwd=other, check=True)
517+
subprocess.run(
518+
[
519+
"git",
520+
"-c",
521+
"user.name=t",
522+
"-c",
523+
"user.email=t@x",
524+
"commit",
525+
"-q",
526+
"--allow-empty",
527+
"-m",
528+
"resplit",
529+
],
530+
cwd=repo,
531+
check=True,
532+
)
533+
subprocess.run(["git", "branch", "-f", "pr-split/ns/pr-9", "HEAD"], cwd=repo, check=True)
534+
535+
prune_remote_tracking_refs()
536+
537+
# The lease must still protect the reviewer's commit.
538+
with pytest.raises(GitOperationError, match="stale info"):
539+
push_branch("pr-split/ns/pr-9")
540+
541+
def test_remote_delete_drops_the_tracking_ref(
542+
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
543+
) -> None:
544+
from pr_split.git_ops.branches import push_branch
545+
546+
repo = self._repo_with_origin(tmp_path)
547+
monkeypatch.chdir(repo)
548+
subprocess.run(["git", "branch", "pr-split/ns/pr-2"], cwd=repo, check=True)
549+
push_branch("pr-split/ns/pr-2")
550+
assert branch_exists("refs/remotes/origin/pr-split/ns/pr-2")
551+
552+
delete_branch("pr-split/ns/pr-2", remote=True)
553+
554+
assert not branch_exists("refs/remotes/origin/pr-split/ns/pr-2")
555+
556+
def test_already_deleted_remote_still_drops_the_tracking_ref(
557+
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
558+
) -> None:
559+
from pr_split.git_ops.branches import push_branch
560+
561+
repo = self._repo_with_origin(tmp_path)
562+
monkeypatch.chdir(repo)
563+
subprocess.run(["git", "branch", "pr-split/ns/pr-3"], cwd=repo, check=True)
564+
push_branch("pr-split/ns/pr-3")
565+
subprocess.run(
566+
["git", "update-ref", "-d", "refs/heads/pr-split/ns/pr-3"],
567+
cwd=tmp_path / "origin.git",
568+
check=True,
569+
)
570+
571+
delete_branch("pr-split/ns/pr-3", remote=True)
572+
573+
assert not branch_exists("refs/remotes/origin/pr-split/ns/pr-3")

0 commit comments

Comments
 (0)