diff --git a/README.md b/README.md index 9118f5c..9f29e83 100644 --- a/README.md +++ b/README.md @@ -244,7 +244,7 @@ ignore = ["*.tmp", "node_modules/"] *Focus: Turning the tool from a blind script into a helpful partner that negotiates with you.* -- [ ] **Smart Restore:** Replace hard failures on "dirty" files with a negotiation menu (Overwrite / View Diff / Cancel). +- [x] **Smart Restore:** Replace hard failures on "dirty" files with a negotiation menu (Overwrite / View Diff / Cancel). - [ ] **Pre-Flight Checklists:** Display a summary table of incoming changes (machines, timestamps, file counts) before running destructive commands like `finalize`. - [x] **Active Doctor:** Upgrade `git pulsar doctor` to not just diagnose issues (like stopped daemons), but offer to auto-fix them interactively. diff --git a/src/README.md b/src/README.md index eb6fb44..28d44de 100644 --- a/src/README.md +++ b/src/README.md @@ -12,7 +12,7 @@ The `src/` directory contains the package source code. The architecture strictly - **Safety:** Implements `GIT_INDEX_FILE` isolation to ensure it never locks or corrupts the user's active git index. - **`git_pulsar/ops.py`**: High-level Business Logic. - **Role:** The "Controller." It orchestrates complex multi-step operations like `finalize` (Octopus Merges), `restore`, and drift detection. - - **Logic:** Calculates the "Zipper Graph" topology to merge shadow commits back into the main branch, manages atomic file I/O for cross-process state tracking, and evaluates pipeline blockers (e.g., oversized files). + - **Logic:** Calculates the "Zipper Graph" topology to merge shadow commits back into the main branch, manages atomic file I/O for cross-process state tracking, evaluates pipeline blockers (e.g., oversized files), and handles interactive state machines for file restorations. - **`git_pulsar/config.py`**: Configuration Engine. - **Role:** The "Source of Truth." - **Logic:** Implements a cascading hierarchy (Defaults → Global → Local) to merge settings from `~/.config/git-pulsar/config.toml` and project-level `pulsar.toml` or `pyproject.toml`. @@ -21,7 +21,7 @@ The `src/` directory contains the package source code. The architecture strictly - **`git_pulsar/git_wrapper.py`**: The Git Interface. - **Role:** A strict wrapper around `subprocess`. - - **Philosophy:** **No Porcelain.** This module primarily uses git *plumbing* commands (`write-tree`, `commit-tree`, `update-ref`) rather than user-facing commands (`commit`, `add`) to ensure deterministic behavior. + - **Philosophy:** **No Porcelain.** This module primarily uses git *plumbing* commands (`write-tree`, `commit-tree`, `update-ref`) rather than user-facing commands (`commit`, `add`) to ensure deterministic behavior. It also handles dynamic command construction, such as safe boundary markers (`--`) for file-level diff targeting. - **`git_pulsar/system.py`**: OS Abstraction. - **Role:** Identity & Environment. - **Logic:** Handles the chaos of cross-platform identity (mapping `IOPlatformUUID` on macOS vs `/etc/machine-id` on Linux) to ensure stable "Roaming Profiles." @@ -47,4 +47,4 @@ The `src/` directory contains the package source code. The architecture strictly 3. **Identity Stability:** The `system` module guarantees that a Machine ID persists across reboots, preventing "Split Brain" backup histories. 4. **Configuration Precedence:** Local project configuration MUST always override global user settings to ensure repo-specific constraints (e.g., large file limits) are respected. 5. **State Over Events (Zero-Latency):** The diagnostic engine (`cli.py`) MUST prioritize current repository state and local caches (e.g., `.git/pulsar_drift_state`) over historical log events or live network calls, ensuring the CLI never blocks the user's terminal while evaluating system health. -6. **Interactive Safety:** The diagnostic engine's interactive resolution queue MUST explicitly prompt the user for confirmation before executing any state-altering auto-fixes (e.g., deleting locks or modifying the registry). +6. **Interactive Safety:** Interactive control loops MUST explicitly prompt the user for confirmation before executing any destructive or state-altering operations (e.g., overwriting dirty files during restore, deleting locks, or modifying the registry). diff --git a/src/git_pulsar/git_wrapper.py b/src/git_pulsar/git_wrapper.py index 24fd170..5224762 100644 --- a/src/git_pulsar/git_wrapper.py +++ b/src/git_pulsar/git_wrapper.py @@ -266,10 +266,14 @@ def get_untracked_files(self) -> list[str]: output = self._run(["ls-files", "--others", "--exclude-standard"]) return output.splitlines() if output else [] - def run_diff(self, target: str) -> None: + def run_diff(self, target: str, file: str | None = None) -> None: """Executes a git diff operation, outputting directly to stdout. Args: - target (str): The target revision or file to diff against. + target (str): The target revision or branch to diff against. + file (str | None, optional): A specific file path to diff. Defaults to None. """ - self._run(["diff", target], capture=False) + cmd = ["diff", target] + if file: + cmd.extend(["--", file]) + self._run(cmd, capture=False) diff --git a/src/git_pulsar/ops.py b/src/git_pulsar/ops.py index d5927e0..618e8f3 100644 --- a/src/git_pulsar/ops.py +++ b/src/git_pulsar/ops.py @@ -11,6 +11,7 @@ from rich.console import Console from rich.panel import Panel +from rich.prompt import Prompt from . import system from .config import Config @@ -302,10 +303,24 @@ def restore_file(path_str: str, force: bool = False) -> None: # 1. Safety Check: Verify if the file is dirty. if not force and path.exists() and repo.status_porcelain(path_str): console.print( - f"[bold red]ABORTED:[/bold red] '{path_str}' has uncommitted changes." + f"[bold yellow]WARNING:[/bold yellow] '{path_str}' has uncommitted changes." ) - console.print(" Use --force to overwrite them.") - sys.exit(1) + + while True: + choice = Prompt.ask( + " [O]verwrite / [V]iew Diff / [C]ancel", + choices=["o", "v", "c"], + default="c", + ) + + if choice == "v": + repo.run_diff(backup_ref, file=path_str) + continue + elif choice == "c": + console.print("[bold red]ABORTED.[/bold red]") + sys.exit(0) + elif choice == "o": + break # 2. Restore file from backup ref. console.print( diff --git a/tests/README.md b/tests/README.md index 956af18..aaa1fb2 100644 --- a/tests/README.md +++ b/tests/README.md @@ -35,6 +35,7 @@ Verifies the "State Reconciliation" engine and primitive operations. - **State Management:** Verifies atomic file I/O operations (`set_drift_state`) to ensure cross-process thread safety between the background daemon and foreground CLI. - **Drift Detection:** Tests the core logic for identifying when remote sessions leapfrog local ones, simulating various network failures and detached HEAD states. - **Pipeline Blockers:** Validates decoupled checks for oversized files (`has_large_files`), ensuring they safely abort operations and trigger system notifications without polluting the daemon's event loop. +- **Interactive State Machines:** Validates the `Prompt.ask` control loop during dirty file restorations, ensuring branching paths (Overwrite, View Diff, Cancel) execute the correct `GitRepo` methods and exit gracefully. ### 5. Configuration Hierarchy (`test_config.py`) @@ -53,6 +54,13 @@ Validates the state-aware diagnostic engine and user-facing CLI commands. - **Environment Simulation & Guidance:** Uses `tmp_path` and `mocker` to synthesize restrictive `.git/hooks`, offline networks, and Linux `systemd` configurations (`loginctl`) without executing side effects on the host, verifying exact stdout formatting for manual interventions. - **UI Determinism:** Ensures commands like `status` and `config` parse timestamps and route to standard system editors (`$EDITOR`, `nano`) correctly. +### 7. Git Abstraction Layer (`test_git_wrapper.py`) + +Ensures the Python-to-Git subprocess boundary remains secure and predictable. + +- **Command Construction:** Verifies that dynamic arguments—such as file-level diff targeting—correctly append necessary boundary markers (`--`) to prevent Git from misinterpreting file paths as revision hashes. +- **Error Handling:** Ensures low-level subprocess failures are caught and logged rather than causing silent upstream crashes. + --- ## Running Tests diff --git a/tests/test_git_wrapper.py b/tests/test_git_wrapper.py index 2b33937..37a5dc0 100644 --- a/tests/test_git_wrapper.py +++ b/tests/test_git_wrapper.py @@ -23,3 +23,20 @@ def test_list_refs_logs_error_on_failure( # Assert it logged the warning assert "Git error listing refs" in caplog.text + + +def test_run_diff_with_file_targeting(mocker: MagicMock, tmp_path: Path) -> None: + """Verifies that run_diff correctly appends the file boundary double-dash.""" + (tmp_path / ".git").mkdir() + repo = GitRepo(tmp_path) + mock_run = mocker.patch.object(repo, "_run") + + # Diff against target without file + repo.run_diff("HEAD") + mock_run.assert_called_with(["diff", "HEAD"], capture=False) + + # Diff against target with specific file + repo.run_diff("refs/backup/main", file="src/main.py") + mock_run.assert_called_with( + ["diff", "refs/backup/main", "--", "src/main.py"], capture=False + ) diff --git a/tests/test_ops.py b/tests/test_ops.py index aff68b0..6a5b7eb 100644 --- a/tests/test_ops.py +++ b/tests/test_ops.py @@ -97,8 +97,8 @@ def test_restore_clean(mocker: MagicMock) -> None: mock_repo.checkout.assert_called_with(expected_ref, file="script.py") -def test_restore_dirty_fails(tmp_path: Path, mocker: MagicMock) -> None: - """Verifies that `restore_file` aborts if the target file has uncommitted changes. +def test_restore_dirty_cancels(tmp_path: Path, mocker: MagicMock) -> None: + """Verifies that selecting [C]ancel exits cleanly with code 0. Args: tmp_path (Path): Pytest fixture for a temporary directory. @@ -110,13 +110,68 @@ def test_restore_dirty_fails(tmp_path: Path, mocker: MagicMock) -> None: mock_cls = mocker.patch("git_pulsar.ops.GitRepo") mock_repo = mock_cls.return_value mock_repo.status_porcelain.return_value = ["M script.py"] - mock_repo.current_branch.return_value = "main" + mocker.patch("git_pulsar.ops.get_backup_ref", return_value="refs/backup") + mocker.patch("git_pulsar.ops.console") + + # Mock the prompt to return 'c' for cancel + mocker.patch("git_pulsar.ops.Prompt.ask", return_value="c") + + with pytest.raises(SystemExit) as excinfo: + ops.restore_file("script.py") + + assert excinfo.value.code == 0 + mock_repo.checkout.assert_not_called() + + +def test_restore_dirty_overwrites(tmp_path: Path, mocker: MagicMock) -> None: + """Verifies that selecting [O]verwrite breaks the loop and restores the file. + + Args: + tmp_path (Path): Pytest fixture for a temporary directory. + mocker (MagicMock): Pytest fixture for mocking. + """ + os.chdir(tmp_path) + (tmp_path / "script.py").touch() + + mock_cls = mocker.patch("git_pulsar.ops.GitRepo") + mock_repo = mock_cls.return_value + mock_repo.status_porcelain.return_value = ["M script.py"] + mocker.patch("git_pulsar.ops.get_backup_ref", return_value="refs/backup") + mocker.patch("git_pulsar.ops.console") + + # Mock the prompt to return 'o' for overwrite + mocker.patch("git_pulsar.ops.Prompt.ask", return_value="o") + + ops.restore_file("script.py") + + mock_repo.checkout.assert_called_once_with("refs/backup", file="script.py") + +def test_restore_dirty_views_diff(tmp_path: Path, mocker: MagicMock) -> None: + """Verifies that selecting [V]iew Diff executes run_diff and re-prompts. + + Args: + tmp_path (Path): Pytest fixture for a temporary directory. + mocker (MagicMock): Pytest fixture for mocking. + """ + os.chdir(tmp_path) + (tmp_path / "script.py").touch() + + mock_cls = mocker.patch("git_pulsar.ops.GitRepo") + mock_repo = mock_cls.return_value + mock_repo.status_porcelain.return_value = ["M script.py"] + mocker.patch("git_pulsar.ops.get_backup_ref", return_value="refs/backup") mocker.patch("git_pulsar.ops.console") + # Mock the prompt to return 'v' (view), then 'c' (cancel) on the second pass + mocker.patch("git_pulsar.ops.Prompt.ask", side_effect=["v", "c"]) + with pytest.raises(SystemExit): ops.restore_file("script.py") + mock_repo.run_diff.assert_called_once_with("refs/backup", file="script.py") + mock_repo.checkout.assert_not_called() + def test_sync_session_success(mocker: MagicMock) -> None: """