From 767f6d5fec533df9a1a75b8c00ab93734ef0ebeb Mon Sep 17 00:00:00 2001 From: L4XB Date: Tue, 15 Sep 2026 15:59:06 +0200 Subject: [PATCH] fix(install): remove the user-global CLAUDE.md registration on uninstall Reported as an install problem (#3572). Install is fine on `v8`: with `CLAUDE_CONFIG_DIR` set and a sandbox HOME, the skill tree, its references and the registration all land under the config dir and nothing touches HOME. What does not work is the other direction, in two independent ways that stack. First, `claude_uninstall`'s `md_targets` were all `project_dir`-relative, so a user-global uninstall deleted the global skill tree and then looked for the registration in whatever directory it was run from. Measured: the skill goes, `$CLAUDE_CONFIG_DIR/CLAUDE.md` keeps its three graphify lines, and the command prints "No CLAUDE.md found in current directory - nothing to do". Second, adding the global file to that list was not enough. The project block comes from `always_on/claude-md.md` and opens with `## graphify`; the global block comes from `_skill_registration` and opens with `# graphify`. The strip matched only the H2, so the global block was unremovable wherever it lived. So: one `_global_claude_md()` resolver that install and uninstall both read, so they cannot drift on `CLAUDE_CONFIG_DIR` again; the global file appended to the uninstall targets only when the global skill is being removed, so `project=True` still leaves it alone (#2215); and the strip falls through to the H1 form, ending at the next heading of any level rather than the next H2. Six cells. Both locations, the user's own notes surviving, a project uninstall leaving the global block alone, and a section the user wrote BELOW the block surviving, which is reachable because install appends to the end of the file. Five mutations, all killed; the last of those cells exists because the next-H2 boundary survived the first run of the battery. --- graphify/install.py | 59 +++++++++++++++--- tests/test_install.py | 142 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 8 deletions(-) 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