fix(windows): delete read-only Git trees instead of failing with WinError 5

shutil.rmtree stops at the first entry it cannot unlink, and Git leaves loose object files read-only on Windows (r--r--r-- trees on POSIX package installs behave the same). Checkpoint clear, MCP catalog uninstall/reinstall and git-installed plugin removal all deleted clones with a bare rmtree, so each aborted mid-tree and left partial state.

Add utils.rmtree_readonly: clear the write bit on the failing entry and its parent, retry that one operation, and keep rmtree semantics for every other failure (both the 3.11 onerror and 3.12+ onexc callback shapes).

hermes_cli/mcp_catalog.py

@@ -20,6 +20,7 @@ from hermes_cli._subprocess_compat import noninteractive_git_env
 from hermes_cli.colors import Colors, color
 from hermes_cli.config import load_config, save_config, get_env_value, save_env_value
 from hermes_cli.cli_output import prompt as _prompt_input
+from utils import rmtree_readonly
 
 _MANIFEST_VERSION = 1
 
@@ -413,7 +414,7 @@ def _do_git_install(entry: CatalogEntry) -> Path:
     if dest.exists():
         # Fresh checkout each install — the manifest ref is the source of truth.
         _say(f"  Removing existing install at {dest}", Colors.DIM)
-        shutil.rmtree(dest)
+        rmtree_readonly(dest)
     _say(f"  Cloning {install.url} ({install.ref}) → {dest}", Colors.CYAN)
 
     # `git clone --branch` only accepts branches/tags, NOT commit SHAs; detect SHA-shaped refs
@@ -433,7 +434,7 @@ def _do_git_install(entry: CatalogEntry) -> Path:
     if not is_sha_ref and _git("clone", "--depth", "1", "--branch", install.ref, install.url, str(dest)) != 0:
         # Branch/tag form failed (e.g. ref deleted upstream): fall through to full-clone path.
         if dest.exists():
-            shutil.rmtree(dest)
+            rmtree_readonly(dest)
         is_sha_ref = True
     if is_sha_ref:
         if _git("clone", install.url, str(dest)) != 0:
@@ -749,6 +750,6 @@ def uninstall_entry(name: str, *, purge_install_dir: bool = True) -> bool:
     if purge_install_dir:
         clone = _install_root() / name
         if clone.exists():
-            shutil.rmtree(clone)
+            rmtree_readonly(clone)
             removed = True
     return removed

hermes_cli/plugins_cmd.py

@@ -21,7 +21,7 @@ from hermes_cli.cli_output import line_input
 from hermes_cli.config import cfg_get
 from hermes_cli.plugin_capabilities import _child_dict
 from hermes_cli.secret_prompt import masked_secret_prompt
-from utils import atomic_write_text
+from utils import atomic_write_text, rmtree_readonly
 
 logger = logging.getLogger(__name__)
 
@@ -709,7 +709,7 @@ def _swap_in_plugin(tmp_target: Path, target: Path, backup: Path, old_metadata:
         _write_install_metadata(new_metadata)
     except Exception:
         if target.exists():
-            shutil.rmtree(target)
+            rmtree_readonly(target)
         if replaced_existing and backup.exists():
             os.replace(backup, target)
         if old_metadata:
@@ -984,7 +984,7 @@ def _remove_plugin_core(target: Path) -> None:
     """Remove one plugin and its metadata without splitting their state."""
     metadata = _read_install_metadata()
     if target.name not in metadata:
-        shutil.rmtree(target)
+        rmtree_readonly(target)
         return
     updated = {k: v for k, v in metadata.items() if k != target.name}
     staging = Path(tempfile.mkdtemp(prefix=f".{target.name}.remove-", dir=target.parent))
@@ -1000,9 +1000,9 @@ def _remove_plugin_core(target: Path) -> None:
                 f"Plugin metadata update failed and '{target.name}' could not be "
                 f"restored automatically; recovery copy remains at {backup}."
             ) from restore_exc
-        shutil.rmtree(staging, ignore_errors=True)
+        rmtree_readonly(staging, ignore_errors=True)
         raise
-    shutil.rmtree(staging)
+    rmtree_readonly(staging)
 
 
 def cmd_remove(name: str) -> None:

tests/hermes_cli/test_mcp_catalog.py

@@ -656,6 +656,22 @@ class TestUninstall:
 
         assert uninstall_entry("nonexistent") is False
 
+    def test_uninstall_removes_read_only_git_clone(self, monkeypatch):
+        """Loose objects are read-only in a clone: the purge must clear that, not abort (#117176)."""
+        import hermes_cli.mcp_catalog as mc
+
+        monkeypatch.setattr(mc, "remove_server", lambda name: False)
+        clone = mc._install_root() / "demo"
+        obj_dir = clone / ".git" / "objects" / "4b"
+        obj_dir.mkdir(parents=True)
+        obj = obj_dir / "825dc642cb6eb9a060e54bf8d69288fbee4904"
+        obj.write_text("blob", encoding="utf-8")
+        obj.chmod(0o444)
+        obj_dir.chmod(0o555)
+
+        assert mc.uninstall_entry("demo") is True
+        assert not clone.exists()
+
 
 # ---------------------------------------------------------------------------
 # Picker (non-TTY paths only — interactive curses is integration-tested)

tests/hermes_cli/test_plugins_cmd.py

@@ -361,7 +361,7 @@ class TestCmdInstall:
 
     @patch("hermes_cli.plugins_cmd._display_after_install")
     @patch("hermes_cli.plugins_cmd.shutil.move")
-    @patch("hermes_cli.plugins_cmd.shutil.rmtree")
+    @patch("hermes_cli.plugins_cmd.rmtree_readonly")
     @patch("hermes_cli.plugins_cmd._plugins_dir")
     @patch("hermes_cli.plugins_cmd._read_manifest")
     @patch("hermes_cli.plugins_cmd.subprocess.run")
@@ -449,7 +449,7 @@ class TestCmdRemove:
 
     @patch("hermes_cli.plugins_cmd._sanitize_plugin_name")
     @patch("hermes_cli.plugins_cmd._plugins_dir")
-    @patch("hermes_cli.plugins_cmd.shutil.rmtree")
+    @patch("hermes_cli.plugins_cmd.rmtree_readonly")
     def test_remove_deletes_plugin(self, mock_rmtree, mock_plugins_dir, mock_sanitize):
         from hermes_cli.plugins_cmd import cmd_remove
 
@@ -479,6 +479,22 @@ class TestCmdRemove:
 
         assert exc_info.value.code == 1
 
+    def test_remove_plugin_core_deletes_read_only_git_tree(self, tmp_path):
+        """Git leaves loose objects read-only: removal must clear that, not abort (#117179)."""
+        from hermes_cli.plugins_cmd import _remove_plugin_core
+
+        target = tmp_path / "plugins" / "demo"
+        obj_dir = target / ".git" / "objects" / "4b"
+        obj_dir.mkdir(parents=True)
+        obj = obj_dir / "825dc642cb6eb9a060e54bf8d69288fbee4904"
+        obj.write_text("blob", encoding="utf-8")
+        obj.chmod(0o444)
+        obj_dir.chmod(0o555)
+
+        _remove_plugin_core(target)
+
+        assert not target.exists()
+
 
 # ── cmd_list tests ─────────────────────────────────────────────────────────
 

tests/test_utils_rmtree_readonly.py

@@ -0,0 +1,88 @@
+"""``utils.rmtree_readonly`` removes trees that ``shutil.rmtree`` refuses.
+
+Git marks loose object files read-only on Windows (``WinError 5``), and package
+installs arrive as read-only trees on POSIX, so every cleanup path that deletes a
+clone needs the retry.  Regression coverage for #117170, #117176 and #117179.
+"""
+
+from __future__ import annotations
+
+import os
+import stat
+from pathlib import Path
+
+import pytest
+
+import utils
+from utils import rmtree_readonly
+
+
+def _read_only_object_dir(root: Path) -> Path:
+    """A clone-shaped tree whose loose object and its directory are read-only."""
+    obj_dir = root / ".git" / "objects" / "4b"
+    obj_dir.mkdir(parents=True)
+    obj = obj_dir / "825dc642cb6eb9a060e54bf8d69288fbee4904"
+    obj.write_text("blob", encoding="utf-8")
+    obj.chmod(0o444)
+    obj_dir.chmod(0o555)  # POSIX unlink needs a writable parent
+    return obj_dir
+
+
+def test_removes_tree_with_read_only_object(tmp_path):
+    root = tmp_path / "plugins" / "demo"
+    obj_dir = _read_only_object_dir(root)
+    assert not (obj_dir / "825dc642cb6eb9a060e54bf8d69288fbee4904").stat().st_mode & stat.S_IWUSR
+
+    rmtree_readonly(root)
+
+    assert not root.exists()
+
+
+@pytest.mark.windows_only
+def test_removes_read_only_file_in_writable_directory(tmp_path):
+    """The Git-for-Windows shape: the file is read-only, its directory is writable."""
+    root = tmp_path / "plugins" / "demo"
+    obj_dir = root / ".git" / "objects" / "4b"
+    obj_dir.mkdir(parents=True)
+    (obj_dir / "825dc642cb6eb9a060e54bf8d69288fbee4904").write_text("blob", encoding="utf-8")
+    (obj_dir / "825dc642cb6eb9a060e54bf8d69288fbee4904").chmod(stat.S_IREAD)
+
+    rmtree_readonly(root)
+
+    assert not root.exists()
+
+
+def _stub_rmtree(monkeypatch, exc: OSError) -> list:
+    """Replace ``shutil.rmtree`` so the wrapper sees *exc* for every attempt."""
+    attempts: list = []
+
+    def _fake(path, **kwargs):
+        attempts.append((path, kwargs))
+        raise exc
+
+    monkeypatch.setattr(utils.shutil, "rmtree", _fake)
+    return attempts
+
+
+def test_ignore_errors_swallows_a_persistent_permission_failure(tmp_path, monkeypatch):
+    _stub_rmtree(monkeypatch, PermissionError(13, "Access is denied", str(tmp_path)))
+
+    rmtree_readonly(tmp_path, ignore_errors=True)
+
+
+def test_permission_failure_still_raises_without_ignore_errors(tmp_path, monkeypatch):
+    _stub_rmtree(monkeypatch, PermissionError(13, "Access is denied", str(tmp_path)))
+
+    with pytest.raises(PermissionError):
+        rmtree_readonly(tmp_path)
+
+
+def test_non_permission_failures_propagate(tmp_path, monkeypatch):
+    """Only ``PermissionError`` is retried — everything else keeps rmtree semantics."""
+    attempts = _stub_rmtree(monkeypatch, OSError(39, "Directory not empty", str(tmp_path)))
+
+    with pytest.raises(OSError) as excinfo:
+        rmtree_readonly(tmp_path)
+
+    assert excinfo.value.errno == 39
+    assert len(attempts) == 1  # no second attempt for a non-permission failure

tools/checkpoint_manager.py

@@ -27,7 +27,7 @@ from typing import Dict, Iterator, List, NamedTuple, Optional, Set, Tuple
 from hermes_constants import get_hermes_home
 from hermes_cli._subprocess_compat import windows_hide_flags
 from hermes_cli.gitlock import clear_stale_tmp_packs
-from utils import env_int
+from utils import env_int, rmtree_readonly
 
 logger = logging.getLogger(__name__)
 
@@ -1050,7 +1050,7 @@ def _rmtree_counted(child: Path, result: Dict[str, int], key: str, fail_fmt: str
     """rmtree ``child``, crediting bytes + ``result[key]``; failures count as ``errors`` when tracked."""
     try:
         size = _dir_size_bytes(child)
-        shutil.rmtree(child)
+        rmtree_readonly(child)
         result["bytes_freed"] += size
         result[key] += 1
     except OSError as exc:
@@ -1271,7 +1271,7 @@ def clear_all(checkpoint_base: Optional[Path] = None) -> Dict[str, int]:
         return out
     size = _dir_size_bytes(base)
     try:
-        shutil.rmtree(base)
+        rmtree_readonly(base)
         out.update(bytes_freed=size, deleted=True)
     except OSError as exc:
         logger.warning("Could not clear checkpoint base %s: %s", base, exc)

utils.py

@@ -225,6 +225,41 @@ def fsync_directory(path: Union[str, Path]) -> None:
         os.close(fd)
 
 
+def rmtree_readonly(path: Union[str, Path], *, ignore_errors: bool = False) -> None:
+    """``shutil.rmtree`` that can also delete read-only trees.
+
+    ``shutil.rmtree`` stops at the first entry it cannot unlink.  Git marks
+    loose object files read-only on Windows (``WinError 5``), and package
+    installs (Nix store, deb/rpm) are copied ``r--r--r--`` into ``0555``
+    directories on POSIX, where unlinking needs a writable *parent*.  Clear the
+    write bit on the failing path and on its parent, then retry the exact
+    operation that failed.  Only ``PermissionError`` is retried: every other
+    failure keeps ``shutil.rmtree``'s semantics (and ``ignore_errors``).
+    """
+
+    def _on_error(func, fpath, exc_info):
+        # ``onerror`` (3.11) passes ``exc_info``, ``onexc`` (3.12+) the exception.
+        exc = exc_info[1] if isinstance(exc_info, tuple) else exc_info
+        if not isinstance(exc, PermissionError):
+            raise exc
+        for candidate in (os.path.dirname(fpath), fpath):
+            if candidate:
+                with suppress(OSError):
+                    os.chmod(candidate, os.stat(candidate).st_mode | stat.S_IWUSR | stat.S_IXUSR)
+        func(fpath)
+
+    try:
+        try:
+            shutil.rmtree(path, onexc=_on_error)
+        except TypeError:  # ``onexc`` is 3.12+; 3.11 only knows ``onerror``
+            shutil.rmtree(path, onerror=_on_error)
+    except OSError:
+        # ``ignore_errors`` still gets the read-only recovery; it only swallows
+        # whatever is left after the retry.
+        if not ignore_errors:
+            raise
+
+
 def _atomic_write(path: Path, write, *, prefix: str, encoding: str = "utf-8", mode: "int | None" = None,
                   preserve_owner: bool = True, binary: bool = False, fsync_dir: bool = False) -> None:
     """Temp file + fsync + :func:`atomic_replace`, then re-apply owner/mode.