From a259c638f5f352e9d58872981cb10d9c0059b406 Mon Sep 17 00:00:00 2001 From: AI Assistant Date: Sat, 1 Aug 2026 09:43:49 -0300 Subject: [PATCH] fix: protect externally managed skills --- README.md | 6 +++ graphify/__main__.py | 3 ++ graphify/install.py | 81 ++++++++++++++++++++++++++++- tests/test_install_roundtrip.py | 50 ++++++++++++++++++ tests/test_skill_version_warning.py | 12 +++++ 5 files changed, 150 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 36a2235ba..36bbe097f 100644 --- a/README.md +++ b/README.md @@ -621,6 +621,12 @@ uv tool upgrade graphifyy graphify install # overwrites the skill file ``` +If another configuration manager owns the skill instructions, place a +`.graphify_externally_managed` marker beside `SKILL.md` instead. Graphify will +then suppress its version warning and refuse to overwrite or remove that skill, +or mutate that platform's hooks and instruction files. Remove the marker only +when you want Graphify's installer to take ownership. + **Claude Code prompt cache invalidated after every `graphify extract`** Graphify writes output files (`graph.json`, `graphify-out/`) into the workspace. If those paths aren't ignored, every write invalidates Claude Code's prompt cache, forcing a full re-upload at cache-write rates on the next turn. Add them to `.claudeignore`: ```text diff --git a/graphify/__main__.py b/graphify/__main__.py index 924ae986d..4baeece3d 100644 --- a/graphify/__main__.py +++ b/graphify/__main__.py @@ -54,6 +54,7 @@ _install_kilo_plugin, _install_opencode_plugin, _install_skill_references, + _is_externally_managed, _kilo_config_path, _kilo_config_write_path, _kilo_install, @@ -163,6 +164,8 @@ def __getattr__(name: str) -> str: def _check_skill_version(skill_dst: Path) -> None: """Warn if the installed skill is from an older graphify version.""" + if _is_externally_managed(skill_dst): + return version_file = skill_dst.parent / ".graphify_version" try: if not version_file.exists(): diff --git a/graphify/install.py b/graphify/install.py index dd8e6a820..2986697ca 100644 --- a/graphify/install.py +++ b/graphify/install.py @@ -30,6 +30,63 @@ from graphify.paths import GRAPHIFY_OUT as _GRAPHIFY_OUT +_EXTERNALLY_MANAGED_MARKER = ".graphify_externally_managed" + + +def _is_externally_managed(skill_dst: Path) -> bool: + """Return whether another tool owns this skill destination. + + The marker follows the same ownership boundary as Python's + ``EXTERNALLY-MANAGED`` convention: Graphify may use the installed package, + but must not overwrite or remove the assistant instructions maintained by + another system. + """ + try: + return (skill_dst.parent / _EXTERNALLY_MANAGED_MARKER).exists() + except OSError: + return False + + +def _refuse_externally_managed_cli_action(platform_name: str, action: str) -> None: + """Stop a platform installer before it mutates assistant configuration.""" + platform_name = _canonical_platform(platform_name) + if platform_name not in _PLATFORM_CONFIG: + return + skill_dst = _platform_skill_destination(platform_name) + if not _is_externally_managed(skill_dst): + return + print( + f"error: {platform_name} integration is externally managed; Graphify will not " + f"{action} its skill, hooks, or instruction files. Remove " + f"{skill_dst.parent / _EXTERNALLY_MANAGED_MARKER} only if Graphify should " + "take ownership.", + file=sys.stderr, + ) + raise SystemExit(1) + + +def _refuse_bulk_uninstall_when_externally_managed() -> None: + """Prevent a bulk uninstall from partially deleting managed integrations.""" + managed = sorted( + { + skill_dst.parent / _EXTERNALLY_MANAGED_MARKER + for skill_dst in { + _platform_skill_destination(name) for name in _PLATFORM_CONFIG + } + if _is_externally_managed(skill_dst) + }, + key=str, + ) + if not managed: + return + markers = ", ".join(str(path) for path in managed) + print( + "error: bulk uninstall refused because externally managed Graphify " + f"integrations are present: {markers}", + file=sys.stderr, + ) + raise SystemExit(1) + @functools.lru_cache(maxsize=None) def _always_on(basename: str) -> str: @@ -63,7 +120,7 @@ def _refresh_all_version_stamps() -> None: for name in _PLATFORM_CONFIG: skill_dst = _platform_skill_destination(name) vf = skill_dst.parent / ".graphify_version" - if skill_dst.exists(): + if skill_dst.exists() and not _is_externally_managed(skill_dst): vf.write_text(__version__, encoding="utf-8") def _platform_skill_destination(platform_name: str, *, project: bool = False, project_dir: Path | None = None) -> Path: """Return the skill destination for a platform and scope.""" @@ -181,6 +238,18 @@ def _copy_skill_file(platform_name: str, *, project: bool = False, project_dir: ``skill_refs``), any orphan ``references/`` left by a prior progressive install is removed so the on-disk layout matches the package. """ + skill_dst = _platform_skill_destination( + platform_name, project=project, project_dir=project_dir + ) + if _is_externally_managed(skill_dst): + print( + f"error: {skill_dst} is externally managed; Graphify will not overwrite it. " + f"Remove {skill_dst.parent / _EXTERNALLY_MANAGED_MARKER} only if Graphify " + "should take ownership.", + file=sys.stderr, + ) + raise SystemExit(1) + skill_file = "skill.md" if platform_name == "gemini" else _PLATFORM_CONFIG[platform_name]["skill_file"] skill_src = Path(__file__).parent / skill_file if not skill_src.exists(): @@ -198,7 +267,6 @@ def _copy_skill_file(platform_name: str, *, project: bool = False, project_dir: ) sys.exit(1) - skill_dst = _platform_skill_destination(platform_name, project=project, project_dir=project_dir) skill_dst.parent.mkdir(parents=True, exist_ok=True) # Install the references/ sidecar (or clear an orphan one) BEFORE writing @@ -232,6 +300,9 @@ def _copy_skill_file(platform_name: str, *, project: bool = False, project_dir: def _remove_skill_file(platform_name: str, *, project: bool = False, project_dir: Path | None = None) -> bool: """Remove a platform skill file and its version stamp without touching other scopes.""" skill_dst = _platform_skill_destination(platform_name, project=project, project_dir=project_dir) + if _is_externally_managed(skill_dst): + print(f" skill externally managed; left untouched -> {skill_dst}") + return False removed = False if skill_dst.exists(): skill_dst.unlink() @@ -2018,6 +2089,10 @@ def dispatch_install_cli(cmd: str) -> bool: """ if cmd not in _CLI_INSTALL_COMMANDS: return False + if cmd not in ("install", "uninstall"): + subcmd = sys.argv[2] if len(sys.argv) > 2 else "" + if subcmd in ("install", "uninstall"): + _refuse_externally_managed_cli_action(cmd, subcmd) if cmd == "install": # Default to windows platform on Windows, claude elsewhere default_platform = "windows" if platform.system() == "Windows" else "claude" @@ -2105,6 +2180,8 @@ def dispatch_install_cli(cmd: str) -> bool: else: _project_uninstall_all(Path(".")) else: + if selected_platform is None: + _refuse_bulk_uninstall_when_externally_managed() uninstall_all(purge=purge) elif cmd == "claude": subcmd = sys.argv[2] if len(sys.argv) > 2 else "" diff --git a/tests/test_install_roundtrip.py b/tests/test_install_roundtrip.py index 68216e6b3..d817f3868 100644 --- a/tests/test_install_roundtrip.py +++ b/tests/test_install_roundtrip.py @@ -17,6 +17,7 @@ import os import shutil +import sys import tempfile from pathlib import Path from unittest.mock import patch @@ -192,6 +193,55 @@ def test_install_entrypoint_roundtrip_for_progressive_and_monolith(tmp_path): assert not (skill_dir / "SKILL.md").exists() +def test_externally_managed_skill_is_not_overwritten_or_removed(tmp_path): + skill_dir = tmp_path / ".claude" / "skills" / "graphify" + skill_dir.mkdir(parents=True) + skill = skill_dir / "SKILL.md" + skill.write_text("host-managed instructions\n", encoding="utf-8") + marker = skill_dir / ".graphify_externally_managed" + marker.write_text("managed by external configuration manager\n", encoding="utf-8") + + with patch("graphify.__main__.Path.home", return_value=tmp_path): + with pytest.raises(SystemExit) as exc_info: + mainmod._copy_skill_file("claude") + assert exc_info.value.code == 1 + assert mainmod._remove_skill_file("claude") is False + + assert skill.read_text(encoding="utf-8") == "host-managed instructions\n" + assert marker.exists() + + +@pytest.mark.parametrize( + ("platform", "mutated_paths"), + [ + ("claude", ("CLAUDE.md", ".claude/settings.json")), + ("codex", ("AGENTS.md", ".codex/hooks.json")), + ], +) +def test_named_platform_installer_cannot_mutate_externally_managed_runtime( + platform, mutated_paths, tmp_path, monkeypatch +): + home = tmp_path / "home" + project = tmp_path / "project" + home.mkdir() + project.mkdir() + monkeypatch.chdir(project) + + with patch("graphify.__main__.Path.home", return_value=home): + skill_dst = mainmod._platform_skill_destination(platform) + skill_dst.parent.mkdir(parents=True) + (skill_dst.parent / ".graphify_externally_managed").write_text( + "managed by external configuration manager\n", encoding="utf-8" + ) + monkeypatch.setattr(sys, "argv", ["graphify", platform, "install"]) + with pytest.raises(SystemExit) as exc_info: + mainmod.dispatch_install_cli(platform) + + assert exc_info.value.code == 1 + for relative in mutated_paths: + assert not (project / relative).exists() + + # --- monolith -> progressive upgrade path -------------------------------------- diff --git a/tests/test_skill_version_warning.py b/tests/test_skill_version_warning.py index 1cca987dd..ba629d9ba 100644 --- a/tests/test_skill_version_warning.py +++ b/tests/test_skill_version_warning.py @@ -58,3 +58,15 @@ def test_matching_version_is_silent(tmp_path, monkeypatch, capsys): skill_dst = _make_skill(tmp_path, "0.9.3") mainmod._check_skill_version(skill_dst) assert capsys.readouterr().err == "" + + +def test_externally_managed_skill_is_silent_even_when_stale(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(mainmod, "__version__", "0.9.31") + skill_dst = _make_skill(tmp_path, "0.9.20") + (skill_dst.parent / ".graphify_externally_managed").write_text( + "managed by host configuration\n", encoding="utf-8" + ) + + mainmod._check_skill_version(skill_dst) + + assert capsys.readouterr().err == ""