diff --git a/README.md b/README.md index 7309cad..586a006 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ Linear chains in the plan are registered as [native GitHub stacks](https://githu pr-split status ``` -Shows a table with each sub-PR's ID, title, branch, PR number, live state (OPEN/CLOSED/MERGED), and review decision (Approved, Changes Requested, etc.) queried directly from GitHub. +Shows a table with each sub-PR's ID, title, branch, PR number, live state (OPEN/CLOSED/MERGED), and review decision (Approved, Changes Requested, etc.) queried directly from GitHub. A PR whose state could not be fetched is shown as UNKNOWN with a warning, never as a stale OPEN. ### Merge split PRs in dependency order diff --git a/pr_split/cli.py b/pr_split/cli.py index 4ad2ea6..874fc5e 100644 --- a/pr_split/cli.py +++ b/pr_split/cli.py @@ -949,6 +949,7 @@ def status() -> None: table.add_column("State") table.add_column("Review") + unverified: list[int] = [] for group in plan.groups: branch_name = branch_map.get(group.id, "") pr_record = pr_map.get(group.id) @@ -956,12 +957,23 @@ def status() -> None: pr_state = "" review = "" if pr_record: - live = live_states.get(pr_record.pr_number, {}) - pr_state = live.get("state", pr_record.state.value).upper() - review = (live.get("reviewDecision") or "").replace("_", " ").title() + live = live_states.get(pr_record.pr_number) or {} + if live: + pr_state = str(live.get("state") or "").upper() + review = str(live.get("reviewDecision") or "").replace("_", " ").title() + else: + # The recorded state is never written back after merge/close, + # so showing it would claim OPEN for a PR that may be gone. + pr_state = "UNKNOWN" + unverified.append(pr_record.pr_number) table.add_row(group.id, group.title, branch_name, pr_info, pr_state, review) console.print(table) + if unverified: + console.print( + f"[yellow]Could not fetch live state for {len(unverified)} PR(s): " + f"{', '.join(f'#{n}' for n in unverified)}. Check 'gh auth status'.[/yellow]" + ) def _cleanup_git_state(git_state: GitState) -> tuple[int, int]: diff --git a/tests/test_cli_coverage.py b/tests/test_cli_coverage.py index e6d7b0a..8f64d2d 100644 --- a/tests/test_cli_coverage.py +++ b/tests/test_cli_coverage.py @@ -25,9 +25,17 @@ _validate_inputs, app, ) -from pr_split.constants import AssignmentType +from pr_split.constants import AssignmentType, Priority from pr_split.exceptions import GitOperationError, PRSplitError -from pr_split.schemas import Group, GroupAssignment +from pr_split.schemas import ( + BranchRecord, + GitState, + Group, + GroupAssignment, + PlanFile, + PRRecord, + SplitPlan, +) from pr_split.types_defs import ForkPRInfo runner = CliRunner() @@ -659,6 +667,57 @@ def test_status_no_plan(self, mock_pe: MagicMock) -> None: result = runner.invoke(app, ["status"]) assert result.exit_code == 0 + def _plan_file(self) -> PlanFile: + plan = SplitPlan( + dev_branch="feature", + base_branch="main", + max_loc=400, + priority=Priority.ORTHOGONAL, + groups=[_group("pr-1", "one"), _group("pr-2", "two")], + ) + git_state = GitState( + branches=[ + BranchRecord(group_id="pr-1", branch_name="b1", base_branch="main"), + BranchRecord(group_id="pr-2", branch_name="b2", base_branch="main"), + ], + prs=[ + PRRecord(group_id="pr-1", pr_number=7, pr_url="u"), + PRRecord(group_id="pr-2", pr_number=8, pr_url="u"), + ], + ) + return PlanFile(plan=plan, git_state=git_state) + + @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_state_is_unknown_not_open( + self, mock_pe: MagicMock, mock_load: MagicMock, mock_state: MagicMock + ) -> None: + mock_load.return_value = self._plan_file() + mock_state.side_effect = lambda n: {} if n == 7 else {"state": "MERGED"} + + result = runner.invoke(app, ["status"]) + + assert result.exit_code == 0 + assert "UNKNOWN" in result.output + assert "MERGED" in result.output + assert "OPEN" not in result.output + assert "Could not fetch live state for 1 PR(s): #7" in result.output + + @patch( + "pr_split.cli.get_pr_state", return_value={"state": "OPEN", "reviewDecision": "APPROVED"} + ) + @patch("pr_split.cli.load_plan") + @patch("pr_split.cli.plan_exists", return_value=True) + def test_live_state_and_review_are_shown( + self, mock_pe: MagicMock, mock_load: MagicMock, mock_state: MagicMock + ) -> None: + mock_load.return_value = self._plan_file() + result = runner.invoke(app, ["status"]) + assert result.exit_code == 0 + assert "Approved" in result.output + assert "Could not fetch" not in result.output + @patch("pr_split.cli.plan_exists", return_value=False) def test_clean_no_plan(self, mock_pe: MagicMock) -> None: result = runner.invoke(app, ["clean"])