You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#2168 reported two things: the test suite silently deleting the developer's installed skill, and the API behaviour underneath that makes it possible. It closed with fix option (a) — v0.9.27 adds an autouse _sandbox_home fixture in tests/conftest.py that sandboxes HOME/USERPROFILE/LOCALAPPDATA, clears CLAUDE_CONFIG_DIR/XDG_CONFIG_HOME, and monkeypatches Path.home. That is a solid fix and it does close the contributor-facing damage — running pytest tests/ no longer touches a real ~/.claude.
This issue tracks fix option (b), which did not ship: the root cause is still present in install.py.
project still defaults to False. _remove_skill_file (install.py:232) forwards both arguments to _platform_skill_destination (install.py:68), where the project branch is the only one that consults project_dir:
cfg=_PLATFORM_CONFIG[platform_name]
ifproject:
return (project_dirorPath(".")) /cfg["skill_dst"] # line 117ifplatform_namein ("claude", "windows") andos.environ.get("CLAUDE_CONFIG_DIR"):
returnPath(os.environ["CLAUDE_CONFIG_DIR"]) /"skills"/"graphify"/"SKILL.md"returnPath.home() /cfg["skill_dst"] # line 121
With project=False, the project_dir that was passed in is accepted, defaulted, and forwarded — then ignored. Resolution falls through to Path.home() / cfg["skill_dst"], i.e. ~/.claude/skills/graphify/SKILL.md (_PLATFORM_CONFIG["claude"]["skill_dst"], install.py:328). _remove_skill_file then unlinks SKILL.md (:237) and .graphify_version (:242), shutil.rmtrees references/ (:246), and rmdirs upward through the parents (:250).
For what it's worth, git diff v0.9.25 v0.9.27 -- graphify/install.py contains no claude_uninstall, project_dir, or _platform_skill_destination changes — the install.py delta in that release is the #2167 settings-clobber work.
Why it still matters
The suite is sandboxed now, so the CI/contributor path is closed. The path reachable from real usage is not:
graphify claude uninstall run from inside a project directory, without --project, removes the global skill rather than that project's. From the user's side it reads as uninstalling the project's copy; the global tree is what disappears, and it fails silently in the sense that nothing indicates the wrong scope was hit.
A future test that legitimately sidesteps the fixture, or any caller outside the suite, re-opens the original hole. The fixture protects the suite, not the function.
Possible fixes — maintainers' call
Either direction closes it; it's a design decision:
Honour project_dir when it is passed — treat a non-Noneproject_dir as implying project scope. Matches what the signature reads like today.
Keep the current semantics but make them explicit — either drop project_dir from the signature when project is the scope selector, or raise on the project_dir is not None and not project combination instead of silently ignoring it.
(2) is the smaller change and removes the ambiguity outright; (1) is closer to what callers appear to expect. Same offer as last time — happy to submit a PR for whichever you prefer, I didn't want to presume the design call.
Context
#2168 reported two things: the test suite silently deleting the developer's installed skill, and the API behaviour underneath that makes it possible. It closed with fix option (a) — v0.9.27 adds an autouse
_sandbox_homefixture intests/conftest.pythat sandboxesHOME/USERPROFILE/LOCALAPPDATA, clearsCLAUDE_CONFIG_DIR/XDG_CONFIG_HOME, and monkeypatchesPath.home. That is a solid fix and it does close the contributor-facing damage — runningpytest tests/no longer touches a real~/.claude.This issue tracks fix option (b), which did not ship: the root cause is still present in
install.py.Still present at v0.9.27
Verified against the
v0.9.27tag.claude_uninstall—graphify/install.py:1801:projectstill defaults toFalse._remove_skill_file(install.py:232) forwards both arguments to_platform_skill_destination(install.py:68), where theprojectbranch is the only one that consultsproject_dir:With
project=False, theproject_dirthat was passed in is accepted, defaulted, and forwarded — then ignored. Resolution falls through toPath.home() / cfg["skill_dst"], i.e.~/.claude/skills/graphify/SKILL.md(_PLATFORM_CONFIG["claude"]["skill_dst"],install.py:328)._remove_skill_filethen unlinksSKILL.md(:237) and.graphify_version(:242),shutil.rmtreesreferences/(:246), andrmdirs upward through the parents (:250).For what it's worth,
git diff v0.9.25 v0.9.27 -- graphify/install.pycontains noclaude_uninstall,project_dir, or_platform_skill_destinationchanges — theinstall.pydelta in that release is the #2167 settings-clobber work.Why it still matters
The suite is sandboxed now, so the CI/contributor path is closed. The path reachable from real usage is not:
graphify claude uninstallrun from inside a project directory, without--project, removes the global skill rather than that project's. From the user's side it reads as uninstalling the project's copy; the global tree is what disappears, and it fails silently in the sense that nothing indicates the wrong scope was hit.claude_uninstall(some_path)call behaves the same way. The signature invites the assumption that passing a path scopes the operation — that assumption is exactly what produced Runningpytest tests/silently deletes the user's installed Claude skill (test_claude_md.py + claude_uninstall project_dir leak) #2168.Possible fixes — maintainers' call
Either direction closes it; it's a design decision:
project_dirwhen it is passed — treat a non-Noneproject_diras implying project scope. Matches what the signature reads like today.project_dirfrom the signature whenprojectis the scope selector, or raise on theproject_dir is not None and not projectcombination instead of silently ignoring it.(2) is the smaller change and removes the ambiguity outright; (1) is closer to what callers appear to expect. Same offer as last time — happy to submit a PR for whichever you prefer, I didn't want to presume the design call.
Note
Unrelated to #2162.