Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ Shows a table with each sub-PR's ID, title, branch, PR number, live state (OPEN/
pr-split merge
```

Walks the dependency DAG and merges each PR in topological order. Skips already-merged, closed, draft, review-required, or changes-requested PRs, and every PR whose dependency was not merged in this run (independent subtrees still proceed). Stops if a merge fails. Exits 1 whenever a merge failed or any PR was left blocked, so re-run once the blocking PRs are ready.
Walks the dependency DAG and merges each PR in topological order. Skips already-merged, closed, draft, review-required, or changes-requested PRs, and every PR whose dependency was not merged in this run (independent subtrees still proceed). Stops if a merge fails. Exits 1 whenever a merge failed, any PR was left blocked, or a PR's state could not be fetched from GitHub (check `gh auth status`), so re-run once the cause is resolved.

Use `--auto` to queue merges behind CI checks (uses `gh pr merge --auto`):

Expand Down
28 changes: 24 additions & 4 deletions pr_split/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -2034,7 +2034,11 @@ def execute(
_AUTO_MERGE_POLL_TIMEOUT = 600


def _poll_for_merged(group_ids: list[str], pr_map: dict[str, PRRecord]) -> set[str]:
def _poll_for_merged(
group_ids: list[str],
pr_map: dict[str, PRRecord],
fetch_errors: list[str] | None = None,
) -> set[str]:
pending = set(group_ids)
actually_merged: set[str] = set()
deadline = time.monotonic() + _AUTO_MERGE_POLL_TIMEOUT
Expand All @@ -2053,6 +2057,8 @@ def _poll_for_merged(group_ids: list[str], pr_map: dict[str, PRRecord]) -> set[s
logger.warning(
f"PR #{pr_record.pr_number} ({gid}) {reason} while polling, aborting wait"
)
if state == "" and fetch_errors is not None:
fetch_errors.append(gid)
pending.discard(gid)
if pending:
remaining = ", ".join(pending)
Expand Down Expand Up @@ -2126,6 +2132,7 @@ def merge_all(
skipped: list[str] = []
skipped_ids: set[str] = set()
blocked: list[str] = []
fetch_errors: list[str] = []
failed: list[str] = []

stopped = False
Expand All @@ -2144,6 +2151,7 @@ def merge_all(
f"PR #{pr_record.pr_number} ({group_id}) state could not be fetched, skipping"
)
skipped_ids.add(group_id)
fetch_errors.append(group_id)
skipped.append(f"{group_id} (fetch error)")
continue

Expand Down Expand Up @@ -2203,8 +2211,13 @@ def merge_all(
queued = [gid for gid in batch if gid not in merged and gid not in skipped_ids]
if queued:
logger.info(f"Waiting for auto-merge to complete: {', '.join(queued)}")
actually_merged = _poll_for_merged(queued, pr_map)
poll_fetch_errors: list[str] = []
actually_merged = _poll_for_merged(queued, pr_map, poll_fetch_errors)
merged.extend(actually_merged)
for gid in poll_fetch_errors:
fetch_errors.append(gid)
skipped_ids.add(gid)
skipped.append(f"{gid} (fetch error)")

if stopped or any(gid not in merged and gid not in skipped_ids for gid in batch):
if not stopped:
Expand Down Expand Up @@ -2232,12 +2245,19 @@ def merge_all(
f"[yellow]Blocked by unmerged dependencies ({len(blocked)}): "
f"{', '.join(blocked)}. Re-run once those PRs are merged.[/yellow]"
)
if fetch_errors:
console.print(
f"[red]Could not fetch PR state for ({len(fetch_errors)}): "
f"{', '.join(fetch_errors)}. Check 'gh auth status' and re-run.[/red]"
)
if notify:
exit_reason = (
"merge_error"
if stopped
else "incomplete_batch"
if exited_early
else "fetch_error"
if fetch_errors
else "unmerged_dependency"
if blocked
else "success"
Expand All @@ -2250,12 +2270,12 @@ def merge_all(
"merged": merged,
"skipped": skipped_structured,
"failed": failed,
"success": not (failed or stopped or exited_early or blocked),
"success": not (failed or stopped or exited_early or blocked or fetch_errors),
"exit_reason": exit_reason,
},
)

if failed or stopped or exited_early or blocked:
if failed or stopped or exited_early or blocked or fetch_errors:
raise typer.Exit(1)
logger.success(f"Merge complete: {len(merged)} PRs merged")

Expand Down
90 changes: 90 additions & 0 deletions tests/test_cli_new_features.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,96 @@ def test_child_of_merged_parent_merges(
assert result.exit_code == 0


class TestMergeFetchErrors:
@patch("pr_split.cli._send_webhook")
@patch("pr_split.cli.merge_pr")
@patch("pr_split.cli.get_pr_state")
@patch("pr_split.cli.load_plan")
@patch("pr_split.cli.plan_exists", return_value=True)
def test_unfetchable_pr_state_fails_the_run(
self,
mock_exists: MagicMock,
mock_load: MagicMock,
mock_state: MagicMock,
mock_merge: MagicMock,
mock_webhook: MagicMock,
) -> None:
groups = [
_group("pr-1", "ok", files=["a.py"]),
_group("pr-2", "unfetchable", files=["b.py"]),
]
mock_load.return_value = _plan_with_prs(groups)
mock_state.side_effect = lambda n: (
{"state": "OPEN", "isDraft": False, "reviewDecision": None} if n == 1 else {}
)

result = runner.invoke(app, ["merge", "--notify", "https://example.invalid/hook"])

mock_merge.assert_called_once_with(1, auto=False)
assert result.exit_code == 1
assert "Could not fetch PR state" in result.output
assert "Merge complete" not in result.output
payload = mock_webhook.call_args[0][1]
assert payload["success"] is False
assert payload["exit_reason"] == "fetch_error"

@patch("pr_split.cli._send_webhook")
@patch("pr_split.cli.merge_pr")
@patch("pr_split.cli.get_pr_state")
@patch("pr_split.cli.load_plan")
@patch("pr_split.cli.plan_exists", return_value=True)
def test_fetch_error_on_parent_is_reported_as_root_cause(
self,
mock_exists: MagicMock,
mock_load: MagicMock,
mock_state: MagicMock,
mock_merge: MagicMock,
mock_webhook: MagicMock,
) -> None:
groups = [
_group("pr-1", "unfetchable", files=["a.py"]),
_group("pr-2", "child", depends_on=["pr-1"], files=["b.py"]),
]
mock_load.return_value = _plan_with_prs(groups)
mock_state.side_effect = lambda n: (
{} if n == 1 else {"state": "OPEN", "isDraft": False, "reviewDecision": None}
)

result = runner.invoke(app, ["merge", "--notify", "https://example.invalid/hook"])

assert result.exit_code == 1
assert mock_webhook.call_args[0][1]["exit_reason"] == "fetch_error"

@patch("pr_split.cli.time.sleep")
@patch("pr_split.cli._send_webhook")
@patch("pr_split.cli.merge_pr")
@patch("pr_split.cli.get_pr_state")
@patch("pr_split.cli.load_plan")
@patch("pr_split.cli.plan_exists", return_value=True)
def test_fetch_error_while_polling_auto_merge_is_reported(
self,
mock_exists: MagicMock,
mock_load: MagicMock,
mock_state: MagicMock,
mock_merge: MagicMock,
mock_webhook: MagicMock,
mock_sleep: MagicMock,
) -> None:
groups = [_group("pr-1", "only", files=["a.py"])]
mock_load.return_value = _plan_with_prs(groups)
mock_state.side_effect = [{"state": "OPEN", "isDraft": False, "reviewDecision": None}, {}]

result = runner.invoke(
app, ["merge", "--auto", "--notify", "https://example.invalid/hook"]
)

assert result.exit_code == 1
assert "Could not fetch PR state" in result.output
payload = mock_webhook.call_args[0][1]
assert payload["exit_reason"] == "fetch_error"
assert [s["id"] for s in payload["skipped"]] == ["pr-1"]


class TestMergeRejectsMalformedSavedPlan:
def _plan_file(self, groups: list[Group]) -> PlanFile:
from pr_split.constants import Priority
Expand Down
Loading