diff --git a/better_memory/cli/install_hooks.py b/better_memory/cli/install_hooks.py index f5c6579..e669731 100644 --- a/better_memory/cli/install_hooks.py +++ b/better_memory/cli/install_hooks.py @@ -244,10 +244,22 @@ def install_skill_symlinks() -> None: if not source.is_dir(): continue link = user_skills_dir / skill_name - if link.is_symlink(): + # Junctions (mklink /J — creatable without Developer Mode, so a + # plausible manual workaround for the symlink-privilege WARN below) + # report is_symlink() == False but ARE links: resolve() follows them + # and shutil.rmtree refuses them outright ("Cannot call rmtree on a + # symbolic link"), which used to crash the installer before the JSON + # writes — a total install failure. Treat them exactly like symlinks. + if link.is_symlink() or os.path.isjunction(link): if link.resolve() == source.resolve(): continue # Already correct — nothing to do. - link.unlink() + try: + link.unlink() + except (OSError, PermissionError): + # Directory junctions on some Python/Windows combinations + # reject unlink(); rmdir removes the link itself and can + # never touch the target's contents. + os.rmdir(link) elif link.exists(): if link.is_file(): link.unlink() diff --git a/tests/e2e/test_install_hooks.py b/tests/e2e/test_install_hooks.py index 7802909..fb61d01 100644 --- a/tests/e2e/test_install_hooks.py +++ b/tests/e2e/test_install_hooks.py @@ -713,3 +713,47 @@ def test_symlink_already_correct_is_noop(harness: InstallHarness) -> None: assert (after.st_ino, after.st_ctime_ns) == before_identity, ( "already-correct symlink was recreated instead of no-op'd" ) + + +# --------------------------------- A7: junction at the skill-link path + + +def test_junction_skill_link_is_recognised_and_skipped( + harness: InstallHarness, +) -> None: + """A junction (mklink /J) pointing at the right skill source must be + treated as an already-correct link, not a real directory. + + Junctions report ``is_symlink() == False`` but ``shutil.rmtree`` refuses + them, so the old dir-branch crashed the installer BEFORE the JSON writes + — a total install failure triggered by the very workaround (junctions + need no Developer Mode) a user on locked-down Windows would reach for. + """ + if not hasattr(os.path, "isjunction"): + pytest.skip("os.path.isjunction requires Python 3.12+") + + repo_skills = Path(__file__).resolve().parents[2] / ".claude" / "skills" + source = repo_skills / SKILL_NAMES[0] + skills_dir = harness.home / ".claude" / "skills" + skills_dir.mkdir(parents=True, exist_ok=True) + junction = skills_dir / SKILL_NAMES[0] + + proc = subprocess.run( # noqa: S603 - fixed argv, test harness + ["cmd", "/c", "mklink", "/J", str(junction), str(source)], + capture_output=True, text=True, + ) + if proc.returncode != 0: + pytest.skip(f"cannot create junction here: {proc.stderr.strip()}") + assert os.path.isjunction(junction) + + result = harness.run() + assert result.returncode == 0, result.stderr + assert "Cannot call rmtree" not in result.stderr + + # Both configs written despite the pre-existing junction. + settings = json.loads( + (harness.home / ".claude" / "settings.json").read_text(encoding="utf-8") + ) + assert_managed_shapes(settings) + # The junction survived untouched (it already pointed at the source). + assert os.path.isjunction(junction)