Merge branch 'main' into fix/cp-sat-feasible-warning

Author: vitali87

pr_split/cli.py

@@ -193,6 +193,14 @@ def _validate_inputs(
         _require_gh_stack()
 
 
+def _load_plan_or_exit() -> PlanFile:
+    try:
+        return load_plan()
+    except PRSplitError as exc:
+        console.print(f"[red]{exc}[/red]")
+        raise typer.Exit(1) from exc
+
+
 def _handle_loc_bound_warnings(warnings: list[str], *, strict_loc_bounds: bool) -> None:
     if strict_loc_bounds and warnings:
         console.print(f"[red]{ErrorMsg.LOC_BOUNDS_STRICT_FAILED()}[/red]")
@@ -839,7 +847,7 @@ def split(
     _validate_inputs(dev_branch, base, dry_run=dry_run, stacked=stack)
 
     if plan_exists():
-        existing = load_plan()
+        existing = _load_plan_or_exit()
         has_git_state = existing.git_state.branches or existing.git_state.prs
         if has_git_state:
             console.print("[yellow]An existing split plan with branches/PRs was found.[/yellow]")
@@ -982,7 +990,7 @@ def status() -> None:
         console.print(ErrorMsg.NO_PLAN())
         raise typer.Exit(0)
 
-    plan_file = load_plan()
+    plan_file = _load_plan_or_exit()
     plan = plan_file.plan
     git_state = plan_file.git_state
 
@@ -1078,7 +1086,7 @@ def clean() -> None:
         console.print(ErrorMsg.NO_PLAN())
         raise typer.Exit(0)
 
-    plan_file = load_plan()
+    plan_file = _load_plan_or_exit()
     git_state = plan_file.git_state
 
     typer.confirm("Delete all pr-split branches and close PRs?", abort=True)
@@ -1115,7 +1123,7 @@ def execute(
         console.print(ErrorMsg.NO_PLAN())
         raise typer.Exit(1)
 
-    plan_file = load_plan()
+    plan_file = _load_plan_or_exit()
     plan = plan_file.plan
     if stack and not plan.stacked:
         plan = plan.model_copy(update={"stacked": True})
@@ -1282,7 +1290,7 @@ def merge_all(
         console.print(ErrorMsg.NO_PLAN())
         raise typer.Exit(0)
 
-    plan_file = load_plan()
+    plan_file = _load_plan_or_exit()
     plan = plan_file.plan
     git_state = plan_file.git_state
     pr_map = {r.group_id: r for r in git_state.prs}

pr_split/exceptions.py

@@ -25,6 +25,9 @@ class ErrorMsg(StrEnum):
     LOC_MISMATCH = "Total LOC {actual} does not match diff LOC {expected}"
     MERGE_CONFLICT = "Groups '{a}' and '{b}' modify overlapping regions in '{file}'"
     NO_PLAN = "No split plan found; run 'pr-split split' first"
+    PLAN_LOAD_FAILED = (
+        "Cannot load split plan from '{path}': {detail}; delete it and run 'pr-split split' again"
+    )
     LLM_PARSE_ERROR = "Failed to parse LLM response: {detail}"
     LLM_OUTPUT_TRUNCATED = (
         "LLM response was cut off before the plan was complete ({detail});"

pr_split/plan_store.py

@@ -1,6 +1,7 @@
 from pathlib import Path
 
 from loguru import logger
+from pydantic import ValidationError
 
 from . import logs
 from .constants import PLAN_DIR, PLAN_FILE
@@ -20,7 +21,10 @@ def load_plan() -> PlanFile:
     plan_path = Path(PLAN_FILE)
     if not plan_path.exists():
         raise PRSplitError(ErrorMsg.NO_PLAN())
-    plan_file = PlanFile.model_validate_json(plan_path.read_text())
+    try:
+        plan_file = PlanFile.model_validate_json(plan_path.read_text())
+    except (OSError, UnicodeDecodeError, ValidationError) as exc:
+        raise PRSplitError(ErrorMsg.PLAN_LOAD_FAILED(path=plan_path, detail=exc)) from exc
     logger.info(logs.PLAN_LOADED.format(count=len(plan_file.plan.groups), path=plan_path))
     return plan_file
 

tests/test_cli_new_features.py

@@ -450,6 +450,44 @@ def test_split_revalidates_min_loc_after_interactive_edit(
         mock_save_plan.assert_not_called()
 
 
+class TestCorruptPlanFile:
+    @pytest.mark.parametrize("command", ["status", "clean", "execute", "merge"])
+    def test_commands_report_corrupt_plan(
+        self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, command: str
+    ) -> None:
+        monkeypatch.chdir(tmp_path)
+        (tmp_path / ".pr-split").mkdir()
+        (tmp_path / ".pr-split" / "plan.json").write_text("{not json")
+
+        result = runner.invoke(app, [command])
+
+        assert result.exit_code == 1
+        assert "Cannot load split plan" in result.output
+        assert not isinstance(result.exception, Exception) or isinstance(
+            result.exception, SystemExit
+        )
+
+    @patch("pr_split.cli._validate_inputs")
+    @patch("pr_split.cli.branch_exists", return_value=True)
+    def test_split_reports_corrupt_plan(
+        self,
+        mock_branch_exists: MagicMock,
+        mock_validate_inputs: MagicMock,
+        tmp_path: Path,
+        monkeypatch: pytest.MonkeyPatch,
+    ) -> None:
+        monkeypatch.chdir(tmp_path)
+        (tmp_path / ".pr-split").mkdir()
+        (tmp_path / ".pr-split" / "plan.json").write_text("")
+
+        result = runner.invoke(
+            app, ["split", "feature-branch", "--dry-run"], env={"ANTHROPIC_API_KEY": "sk-test"}
+        )
+
+        assert result.exit_code == 1
+        assert "Cannot load split plan" in result.output
+
+
 class TestCleanupSkipsFinishedPrs:
     @patch("pr_split.cli.get_pr_state")
     @patch("pr_split.cli.shutil.rmtree")

tests/test_plan_store.py

@@ -106,3 +106,43 @@ def test_saved_file_is_valid_json(
         assert raw["plan"]["priority"] == "logical"
         assert raw["plan"]["min_loc"] == 25
         assert raw["plan"]["strict_loc_bounds"] is True
+
+
+class TestPlanStoreCorruptFiles:
+    @pytest.mark.parametrize(
+        "content",
+        [
+            pytest.param("", id="empty"),
+            pytest.param("{not json", id="not-json"),
+            pytest.param('{"plan": {"dev_branch": "x"}}', id="missing-fields"),
+            pytest.param('{"plan": {"groups": []}, "git_state": {}}', id="old-schema"),
+        ],
+    )
+    def test_unreadable_plan_raises_pr_split_error(
+        self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, content: str
+    ) -> None:
+        monkeypatch.chdir(tmp_path)
+        plan_dir = tmp_path / ".pr-split"
+        plan_dir.mkdir()
+        (plan_dir / "plan.json").write_text(content)
+        with pytest.raises(PRSplitError, match="Cannot load split plan") as excinfo:
+            load_plan()
+        assert "pr-split split" in str(excinfo.value)
+
+    def test_invalid_utf8_raises_pr_split_error(
+        self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        monkeypatch.chdir(tmp_path)
+        (tmp_path / ".pr-split").mkdir()
+        (tmp_path / ".pr-split" / "plan.json").write_bytes(b'{"plan": "\xff\xfe"}')
+        with pytest.raises(PRSplitError, match="Cannot load split plan"):
+            load_plan()
+
+    def test_read_error_raises_pr_split_error(
+        self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        monkeypatch.chdir(tmp_path)
+        (tmp_path / ".pr-split").mkdir()
+        (tmp_path / ".pr-split" / "plan.json").mkdir()
+        with pytest.raises(PRSplitError, match="Cannot load split plan"):
+            load_plan()