mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-27 08:46:43 +00:00
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
2f78439ffd
commit
bdb678858b
+52
-13
@@ -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():
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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 <root>/<dot_dir>/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()
|
||||
Reference in New Issue
Block a user