From bdb678858b61183c3cf820fd6f82eec9748b7134 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Mon, 27 Jul 2026 10:46:47 +0100 Subject: [PATCH] fix(install): scope uninstall to project_dir instead of always deleting the global skill (#2215) claude_uninstall/gemini_uninstall/codebuddy_uninstall accepted project_dir but, with the default project=False, still deleted the user-global skill tree, so a library/test caller passing a project_dir nuked ~/.claude et al (the API trap behind #2168). They now take remove_user_skill and treat a passed project_dir as authoritative; uninstall_all opts in explicitly to preserve 'graphify uninstall' behavior. Also fixes a live CLI bug where 'uninstall --project' deleted the global codebuddy skill. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/install.py | 65 +++++++++++++++---- tests/test_home_sandbox.py | 16 +++-- tests/test_uninstall_scope.py | 113 ++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 18 deletions(-) create mode 100644 tests/test_uninstall_scope.py diff --git a/graphify/install.py b/graphify/install.py index c3503c2e..8c0cfee6 100644 --- a/graphify/install.py +++ b/graphify/install.py @@ -792,10 +792,22 @@ def _uninstall_gemini_hook(project_dir: Path) -> None: settings["hooks"]["BeforeTool"] = filtered settings_path.write_text(json.dumps(settings, indent=2), encoding="utf-8") print(" .gemini/settings.json -> BeforeTool hook removed") -def gemini_uninstall(project_dir: Path | None = None, *, project: bool = False) -> None: - """Remove the graphify section from GEMINI.md, uninstall hook, and remove skill file.""" +def gemini_uninstall(project_dir: Path | None = None, *, project: bool = False, remove_user_skill: bool | None = None) -> None: + """Remove the graphify section from GEMINI.md, uninstall hook, and remove skill file. + + Scope rules (#2215): a bare call removes the user-global skill; passing + ``project_dir`` (or ``project=True``) scopes skill removal to that project + and leaves the global tree untouched, unless ``remove_user_skill=True`` + explicitly opts back into the global delete (as ``uninstall_all`` does). + """ + explicit_dir = project_dir is not None project_dir = project_dir or Path(".") - _remove_skill_file("gemini", project=project, project_dir=project_dir) + if remove_user_skill is None: + remove_user_skill = not project and not explicit_dir + if project or (explicit_dir and not remove_user_skill): + _remove_skill_file("gemini", project=True, project_dir=project_dir) + if remove_user_skill: + _remove_skill_file("gemini", project=False) target = project_dir / "GEMINI.md" if not target.exists(): @@ -1600,7 +1612,9 @@ def _project_uninstall(platform_name: str, project_dir: Path | None = None) -> N if not removed: print("nothing to remove") elif platform_name == "codebuddy": - codebuddy_uninstall(project_dir) + # project=True keeps `uninstall --project` project-scoped; previously + # this deleted the user-global codebuddy skill (#2215). + codebuddy_uninstall(project_dir, project=True) else: _remove_skill_file(platform_name, project=True, project_dir=project_dir) def _project_uninstall_all(project_dir: Path | None = None) -> None: @@ -1760,10 +1774,12 @@ def uninstall_all(project_dir: Path | None = None, purge: bool = False) -> None: pd = project_dir or Path(".") print("Uninstalling graphify from all detected platforms...\n") - # Skill-file / config-section uninstallers - claude_uninstall(pd) - codebuddy_uninstall(pd) - gemini_uninstall(pd) + # Skill-file / config-section uninstallers. remove_user_skill=True keeps the + # historical `graphify uninstall` behavior: global skill delete plus md/hook + # cleanup at the project dir (#2215). + claude_uninstall(pd, remove_user_skill=True) + codebuddy_uninstall(pd, remove_user_skill=True) + gemini_uninstall(pd, remove_user_skill=True) vscode_uninstall(pd) _cursor_uninstall(pd) _kiro_uninstall(pd) @@ -1798,7 +1814,7 @@ def uninstall_all(project_dir: Path | None = None, purge: bool = False) -> None: print(f"\n {_GRAPHIFY_OUT}/ -> not found (nothing to purge)") print("\nDone. Run 'pip uninstall graphifyy' to remove the package itself.") -def claude_uninstall(project_dir: Path | None = None, *, project: bool = False) -> None: +def claude_uninstall(project_dir: Path | None = None, *, project: bool = False, remove_user_skill: bool | None = None) -> None: """Remove the graphify skill tree (SKILL.md + references/) and the graphify section from CLAUDE.md and its local-only variants, plus the PreToolUse hook. @@ -1809,9 +1825,20 @@ def claude_uninstall(project_dir: Path | None = None, *, project: bool = False) A user may relocate the section/hook into the local-only files Claude Code supports so they are not committed to a shared repo, so uninstall also cleans CLAUDE.local.md, .claude/CLAUDE.local.md and .claude/settings.local.json (#1731). + + Scope rules (#2215): a bare call removes the user-global skill; passing + ``project_dir`` (or ``project=True``) scopes skill removal to that project + and leaves the global tree untouched, unless ``remove_user_skill=True`` + explicitly opts back into the global delete (as ``uninstall_all`` does). """ + explicit_dir = project_dir is not None project_dir = project_dir or Path(".") - _remove_skill_file("claude", project=project, project_dir=project_dir) + if remove_user_skill is None: + remove_user_skill = not project and not explicit_dir + if project or (explicit_dir and not remove_user_skill): + _remove_skill_file("claude", project=True, project_dir=project_dir) + if remove_user_skill: + _remove_skill_file("claude", project=False) md_targets = [ project_dir / "CLAUDE.md", @@ -1914,10 +1941,22 @@ def _uninstall_codebuddy_hook(project_dir: Path) -> None: settings["hooks"]["PreToolUse"] = filtered settings_path.write_text(json.dumps(settings, indent=2), encoding="utf-8") print(f" .codebuddy/settings.json -> PreToolUse hook removed") -def codebuddy_uninstall(project_dir: Path | None = None, *, project: bool = False) -> None: - """Remove the graphify skill tree (SKILL.md + references/) and the CODEBUDDY.md section.""" +def codebuddy_uninstall(project_dir: Path | None = None, *, project: bool = False, remove_user_skill: bool | None = None) -> None: + """Remove the graphify skill tree (SKILL.md + references/) and the CODEBUDDY.md section. + + Scope rules (#2215): a bare call removes the user-global skill; passing + ``project_dir`` (or ``project=True``) scopes skill removal to that project + and leaves the global tree untouched, unless ``remove_user_skill=True`` + explicitly opts back into the global delete (as ``uninstall_all`` does). + """ + explicit_dir = project_dir is not None project_dir = project_dir or Path(".") - _remove_skill_file("codebuddy", project=project, project_dir=project_dir) + if remove_user_skill is None: + remove_user_skill = not project and not explicit_dir + if project or (explicit_dir and not remove_user_skill): + _remove_skill_file("codebuddy", project=True, project_dir=project_dir) + if remove_user_skill: + _remove_skill_file("codebuddy", project=False) target = project_dir / "CODEBUDDY.md" if not target.exists(): diff --git a/tests/test_home_sandbox.py b/tests/test_home_sandbox.py index fc388adc..2ddb3ffb 100644 --- a/tests/test_home_sandbox.py +++ b/tests/test_home_sandbox.py @@ -39,11 +39,12 @@ def test_claude_config_dir_escape_hatch_is_cleared(): def test_global_uninstall_is_captured_by_sandbox(tmp_path, tmp_path_factory): - """claude_uninstall deletes the *global* ~/.claude/skills/graphify tree. + """Global skill deletes land inside the sandbox home, never the real one. - Plant that tree inside the sandbox home, run uninstall against an - unrelated project dir, and prove the delete landed in the sandbox - (and therefore not in the developer's real home). + Since #2215, `claude_uninstall(project_dir)` is project-scoped and must NOT + touch the global tree; the global delete now requires either a bare call or + an explicit `remove_user_skill=True`. Both scopes are exercised here so the + sandbox (#2168) is still proven to capture the global delete. """ skill = Path.home() / ".claude" / "skills" / "graphify" / "SKILL.md" skill.parent.mkdir(parents=True) @@ -51,8 +52,13 @@ def test_global_uninstall_is_captured_by_sandbox(tmp_path, tmp_path_factory): project_dir = tmp_path / "some-project" project_dir.mkdir() - claude_uninstall(project_dir) + # Project-scoped call: the global tree in the sandbox home must survive. + claude_uninstall(project_dir) + assert skill.exists(), "project-scoped uninstall deleted the global skill (#2215 trap)" + + # Explicit global opt-in: the delete happens, and lands in the sandbox. + claude_uninstall(project_dir, remove_user_skill=True) assert not skill.exists(), "global skill delete was not captured by the sandbox" # And the sandbox home itself is still inside pytest's tmp area. assert Path.home().is_relative_to(tmp_path_factory.getbasetemp()) diff --git a/tests/test_uninstall_scope.py b/tests/test_uninstall_scope.py new file mode 100644 index 00000000..9102cdfa --- /dev/null +++ b/tests/test_uninstall_scope.py @@ -0,0 +1,113 @@ +"""Scope regression tests for the uninstall API trap (issue #2215). + +`X_uninstall(project_dir)` used to delete the USER-GLOBAL skill tree because +`_platform_skill_destination` honors ``project_dir`` only when ``project=True``. +These tests pin the fixed contract: + +- bare call -> global skill removed (CLI behavior unchanged) +- fn(pd) -> project-scoped, global untouched (trap closed) +- fn(pd, project=True) -> project only +- fn(pd, remove_user_skill=True) -> global removed, project tree untouched +- `graphify uninstall --project` for codebuddy no longer nukes the global skill +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from graphify.install import ( + _project_uninstall, + claude_uninstall, + codebuddy_uninstall, + gemini_uninstall, +) + +PLATFORMS = [ + pytest.param(claude_uninstall, "claude", ".claude", id="claude"), + pytest.param(gemini_uninstall, "gemini", ".gemini", id="gemini"), + pytest.param(codebuddy_uninstall, "codebuddy", ".codebuddy", id="codebuddy"), +] + + +def _plant_skill_tree(root: Path, dot_dir: str) -> Path: + """Create //skills/graphify/{SKILL.md, references/x.md, .graphify_version}.""" + skill_dir = root / dot_dir / "skills" / "graphify" + (skill_dir / "references").mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("# graphify skill\n", encoding="utf-8") + (skill_dir / "references" / "x.md").write_text("ref\n", encoding="utf-8") + (skill_dir / ".graphify_version").write_text("0.0.0-test", encoding="utf-8") + return skill_dir + + +@pytest.mark.parametrize("uninstall_fn,platform,dot_dir", PLATFORMS) +def test_project_dir_call_never_touches_global(uninstall_fn, platform, dot_dir, tmp_path): + """fn(project_dir) removes only the project skill tree (#2215 trap closed).""" + global_tree = _plant_skill_tree(Path.home(), dot_dir) + proj_dir = tmp_path / "proj" + project_tree = _plant_skill_tree(proj_dir, dot_dir) + + uninstall_fn(proj_dir) + + assert (global_tree / "SKILL.md").exists(), "global skill deleted by project-scoped uninstall" + assert (global_tree / "references" / "x.md").exists() + assert (global_tree / ".graphify_version").exists() + assert not (project_tree / "SKILL.md").exists() + assert not project_tree.exists() + + +@pytest.mark.parametrize("uninstall_fn,platform,dot_dir", PLATFORMS) +def test_bare_call_still_removes_global(uninstall_fn, platform, dot_dir, tmp_path, monkeypatch): + """fn() with no args keeps the historical CLI behavior: global skill removed.""" + global_tree = _plant_skill_tree(Path.home(), dot_dir) + cwd = tmp_path / "empty-cwd" + cwd.mkdir() + monkeypatch.chdir(cwd) + + uninstall_fn() + + assert not (global_tree / "SKILL.md").exists() + assert not global_tree.exists() + + +@pytest.mark.parametrize("uninstall_fn,platform,dot_dir", PLATFORMS) +def test_remove_user_skill_opt_in_with_project_dir(uninstall_fn, platform, dot_dir, tmp_path): + """fn(pd, remove_user_skill=True) removes the global skill, leaves the project tree.""" + global_tree = _plant_skill_tree(Path.home(), dot_dir) + proj_dir = tmp_path / "proj" + project_tree = _plant_skill_tree(proj_dir, dot_dir) + + uninstall_fn(proj_dir, remove_user_skill=True) + + assert not (global_tree / "SKILL.md").exists() + assert not global_tree.exists() + assert (project_tree / "SKILL.md").exists() + assert (project_tree / "references" / "x.md").exists() + + +@pytest.mark.parametrize("uninstall_fn,platform,dot_dir", PLATFORMS) +def test_project_true_removes_only_project_tree(uninstall_fn, platform, dot_dir, tmp_path): + """fn(pd, project=True) removes only the project skill tree.""" + global_tree = _plant_skill_tree(Path.home(), dot_dir) + proj_dir = tmp_path / "proj" + project_tree = _plant_skill_tree(proj_dir, dot_dir) + + uninstall_fn(proj_dir, project=True) + + assert (global_tree / "SKILL.md").exists() + assert not (project_tree / "SKILL.md").exists() + assert not project_tree.exists() + + +def test_project_uninstall_codebuddy_spares_global(tmp_path): + """`graphify uninstall --project` (codebuddy branch) must not delete ~/.codebuddy (#2215).""" + global_tree = _plant_skill_tree(Path.home(), ".codebuddy") + proj_dir = tmp_path / "proj" + project_tree = _plant_skill_tree(proj_dir, ".codebuddy") + + _project_uninstall("codebuddy", proj_dir) + + assert (global_tree / "SKILL.md").exists(), "CLI --project uninstall deleted the global codebuddy skill" + assert (global_tree / ".graphify_version").exists() + assert not (project_tree / "SKILL.md").exists() + assert not project_tree.exists()