From 5dd180f139c06bcd6ad63740e4b7aab10180155e Mon Sep 17 00:00:00 2001 From: gethin Date: Wed, 22 Jul 2026 08:31:25 +0100 Subject: [PATCH] fix(install): recognise junctions at skill-link paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A junction (mklink /J) at ~/.claude/skills/ reports is_symlink() == False, so the installer fell into the real-directory branch and called shutil.rmtree, which refuses links outright — crashing install_hooks before the JSON writes. Junctions are the natural workaround for the installer's own "enable Developer Mode" warning (they need no privilege), so the crash hits exactly the users the symlink fallback was written for. Treat junctions like symlinks: skip when already pointing at the source, otherwise remove the link itself (unlink, with os.rmdir fallback for dir junctions) and never its target. Co-Authored-By: Claude Fable 5 --- better_memory/cli/install_hooks.py | 16 +++++++++-- tests/e2e/test_install_hooks.py | 44 ++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) 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)