diff --git a/graphify/install.py b/graphify/install.py index 7075efa651..f3a788f33c 100644 --- a/graphify/install.py +++ b/graphify/install.py @@ -132,6 +132,22 @@ def _platform_skill_destination(platform_name: str, *, project: bool = False, pr if platform_name in ("claude", "windows") and os.environ.get("CLAUDE_CONFIG_DIR"): return Path(os.environ["CLAUDE_CONFIG_DIR"]) / "skills" / "graphify" / "SKILL.md" return Path.home() / cfg["skill_dst"] +def _global_claude_md() -> Path: + """The user-global ``CLAUDE.md`` the always-on block is registered in. + + ``CLAUDE_CONFIG_DIR`` relocates a user's whole Claude Code config, which is + how a second account is kept separate, so the install that WRITES this + block and the uninstall that STRIPS it must resolve the same file. They did + not: install read the variable (#2694) while uninstall only ever looked in + the current directory, so a user-global uninstall removed the skill and left + the registration pointing at it (#3572). One function now, so the two cannot + drift again. + """ + config_dir = os.environ.get("CLAUDE_CONFIG_DIR") + root = Path(config_dir) if config_dir else Path.home() / ".claude" + return root / "CLAUDE.md" + + def _packaged_skill_refs_dir(platform_name: str) -> Path | None: """Return the packaged references source dir for a progressive platform, else None. @@ -695,13 +711,13 @@ def install(platform: str = "claude", *, project: bool = False, project_dir: Pat if project: claude_md = project_dir / ".claude" / "CLAUDE.md" skill_ref = ".claude/skills/graphify/SKILL.md" - elif os.environ.get("CLAUDE_CONFIG_DIR"): - config_dir = Path(os.environ["CLAUDE_CONFIG_DIR"]) - claude_md = config_dir / "CLAUDE.md" - skill_ref = str(config_dir / "skills" / "graphify" / "SKILL.md") else: - claude_md = Path.home() / ".claude" / "CLAUDE.md" - skill_ref = "~/.claude/skills/graphify/SKILL.md" + claude_md = _global_claude_md() + skill_ref = ( + str(claude_md.parent / "skills" / "graphify" / "SKILL.md") + if os.environ.get("CLAUDE_CONFIG_DIR") + else "~/.claude/skills/graphify/SKILL.md" + ) _register_always_on_block( claude_md, " CLAUDE.md -> ", _skill_registration(skill_ref) ) @@ -735,6 +751,12 @@ def _print_install_usage() -> None: print(" --strict block the first raw file read per session until one " "`graphify query` runs (Claude Code project hook only; needs --project)") _CLAUDE_MD_MARKER = "## graphify" +# The user-global registration is written by `_skill_registration`, which opens +# its block with an H1 rather than the H2 `always_on/claude-md.md` uses for a +# project. Uninstall matched only the H2, so the global block was unremovable +# whatever directory it lived in -- the second half of #3572, and the reason +# adding the global file to the search list alone was not enough. +_GLOBAL_CLAUDE_MD_MARKER = "# graphify" _CODEBUDDY_MD_MARKER = "## graphify" _AGENTS_MD_MARKER = "## graphify" _GEMINI_MD_MARKER = "## graphify" @@ -1927,7 +1949,14 @@ def claude_uninstall(project_dir: Path | None = None, *, project: bool = False, project_dir / "CLAUDE.local.md", project_dir / ".claude" / "CLAUDE.local.md", ] - existing = [t for t in md_targets if t.exists()] + # A user-global uninstall has just deleted the global skill tree, so the + # block registering it goes with it, wherever CLAUDE_CONFIG_DIR put it. + # Appended rather than substituted: a bare `graphify uninstall` run inside a + # project still cleans that project's files too, which is what it did + # before (#3572). + if remove_user_skill: + md_targets.append(_global_claude_md()) + existing = [t for t in dict.fromkeys(md_targets) if t.exists()] removed_any = False for target in existing: # Not short-circuited: every present file must be cleaned, not just the first. @@ -1935,7 +1964,14 @@ def claude_uninstall(project_dir: Path | None = None, *, project: bool = False, removed_any = True if not existing: - print("No CLAUDE.md found in current directory - nothing to do") + # Naming the places looked in, because "not found in current directory" + # was printed even when the global file it had not looked at held the + # block it had just orphaned. + print( + "No CLAUDE.md found in " + + " or ".join(str(t) for t in md_targets) + + " - nothing to do" + ) elif not removed_any: print("graphify section not found in CLAUDE.md - nothing to do") @@ -1955,6 +1991,13 @@ def _strip_graphify_md_section(target: Path) -> bool: # Remove graphify's ## graphify section (heading matched exactly, never as a # substring of a user's ### graphify) from the marker to the next H2 or EOF. cleaned = _remove_marker_section(content, _CLAUDE_MD_MARKER) + if cleaned is None: + # The H1 form, ended at the next heading of any level rather than the + # next H2: the block has no subheadings of its own, so stopping at the + # first `#` line can only remove less, never a neighbour's content. + cleaned = _remove_marker_section( + content, _GLOBAL_CLAUDE_MD_MARKER, boundary_prefix="#" + ) if cleaned is None: return False if cleaned: diff --git a/tests/test_install.py b/tests/test_install.py index 72d4e4dd26..830fe1b4ef 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1454,3 +1454,145 @@ def test_project_uninstall_removes_the_bare_hook_command(tmp_path, monkeypatch): main() assert not [c for c in _hook_commands(settings.read_text(encoding="utf-8")) if "graphify" in c] + + +def _install_then_uninstall(tmp_path, monkeypatch, *, config_dir): + """Global install, then a bare `graphify uninstall` from an unrelated cwd. + + The cwd matters: a bare uninstall is the user-global one, and it used to + look for the registration in whatever directory it was run from. Running it + somewhere with no CLAUDE.md is what a user does, and what made the gap + invisible. + """ + from graphify.__main__ import install, claude_uninstall + + home = tmp_path / "home" + home.mkdir() + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + if config_dir is None: + monkeypatch.delenv("CLAUDE_CONFIG_DIR", raising=False) + registered = home / ".claude" / "CLAUDE.md" + else: + config_dir.mkdir() + monkeypatch.setenv("CLAUDE_CONFIG_DIR", str(config_dir)) + registered = config_dir / "CLAUDE.md" + + old = os.getcwd() + try: + os.chdir(tmp_path) + with patch("graphify.__main__.Path.home", return_value=home): + install(platform="claude") + assert registered.exists(), "precondition: install wrote the registration" + os.chdir(elsewhere) + claude_uninstall() + finally: + os.chdir(old) + return registered + + +def test_uninstall_removes_the_global_registration(tmp_path, monkeypatch): + """#3572: a user-global uninstall deleted the skill and left the block that + registers it, so Claude Code went on loading a `## graphify` entry pointing + at a SKILL.md that was no longer there.""" + registered = _install_then_uninstall(tmp_path, monkeypatch, config_dir=None) + + assert not registered.exists() or "graphify" not in registered.read_text() + + +def test_uninstall_removes_the_global_registration_under_claude_config_dir( + tmp_path, monkeypatch +): + """The relocated-profile case the report is about. Install already honours + the variable (#2694); uninstall has to read the same one, or the second + account keeps the orphaned block.""" + registered = _install_then_uninstall( + tmp_path, monkeypatch, config_dir=tmp_path / "cfg" + ) + + assert not registered.exists() or "graphify" not in registered.read_text() + + +def test_uninstall_keeps_the_users_own_notes(tmp_path, monkeypatch): + """The accept control, and the reason this is a section strip rather than a + delete: a global CLAUDE.md is where a user keeps their own standing + instructions, and the block is appended to whatever is already there.""" + from graphify.__main__ import install, claude_uninstall + + home = tmp_path / "home" + (home / ".claude").mkdir(parents=True) + notes = home / ".claude" / "CLAUDE.md" + notes.write_text("# My notes\n\nKeep me.\n", encoding="utf-8") + monkeypatch.delenv("CLAUDE_CONFIG_DIR", raising=False) + + old = os.getcwd() + try: + os.chdir(tmp_path) + with patch("graphify.__main__.Path.home", return_value=home): + install(platform="claude") + assert "graphify" in notes.read_text() + claude_uninstall() + finally: + os.chdir(old) + + survived = notes.read_text() + assert "graphify" not in survived + assert survived.rstrip() == "# My notes\n\nKeep me." + + +def test_a_project_uninstall_leaves_the_global_registration_alone( + tmp_path, monkeypatch +): + """The scope control. `project=True` is documented to leave the global tree + untouched (#2215), so widening the search must not widen the delete.""" + from graphify.__main__ import install, claude_uninstall + + home = tmp_path / "home" + home.mkdir() + project = tmp_path / "project" + project.mkdir() + monkeypatch.delenv("CLAUDE_CONFIG_DIR", raising=False) + + old = os.getcwd() + try: + os.chdir(tmp_path) + with patch("graphify.__main__.Path.home", return_value=home): + install(platform="claude") + registered = home / ".claude" / "CLAUDE.md" + assert "graphify" in registered.read_text() + claude_uninstall(project, project=True) + finally: + os.chdir(old) + + assert "graphify" in registered.read_text() + + +def test_uninstall_keeps_a_section_written_after_the_block(tmp_path, monkeypatch): + """The block is appended to the END of the file, so anything the user adds + later sits BELOW it. Ending the strip at the next H2 rather than the next + heading of any level would take an H1 section with it. + """ + from graphify.__main__ import install, claude_uninstall + + home = tmp_path / "home" + home.mkdir() + monkeypatch.delenv("CLAUDE_CONFIG_DIR", raising=False) + + old = os.getcwd() + try: + os.chdir(tmp_path) + with patch("graphify.__main__.Path.home", return_value=home): + install(platform="claude") + notes = home / ".claude" / "CLAUDE.md" + notes.write_text( + notes.read_text() + "\n# My own rules\n\nAlways run the tests.\n", + encoding="utf-8", + ) + claude_uninstall() + finally: + os.chdir(old) + + survived = notes.read_text() + assert "graphify" not in survived + assert "# My own rules" in survived + assert "Always run the tests." in survived