diff --git a/src/specify_cli/events/__init__.py b/src/specify_cli/events/__init__.py index 0d1aa3a0a1..efdec79db5 100644 --- a/src/specify_cli/events/__init__.py +++ b/src/specify_cli/events/__init__.py @@ -32,6 +32,8 @@ from ..integrations._file_changes import unlink as _unlink_file, write_text as _write_text if TYPE_CHECKING: + from collections.abc import Iterable + from ..integrations.base import IntegrationBase from ..integrations.manifest import IntegrationManifest @@ -2153,13 +2155,15 @@ def _remove_opencode_entries(config_path: Path) -> bool: if existing is None: return False plugins = existing.get("plugin", []) - if isinstance(plugins, list): - ref = "./.opencode/plugin/speckit-events.ts" - plugins = [p for p in plugins if p != ref] - if plugins: - existing["plugin"] = plugins - else: - existing.pop("plugin", None) + ref = "./.opencode/plugin/speckit-events.ts" + # No Spec Kit plugin ref to strip: leave the user's file untouched. + if not isinstance(plugins, list) or ref not in plugins: + return False + plugins = [p for p in plugins if p != ref] + if plugins: + existing["plugin"] = plugins + else: + existing.pop("plugin", None) if not existing: _unlink_file(config_path, missing_ok=True) return True @@ -2376,8 +2380,9 @@ def _remove_copilot_entries(dst: Path) -> bool: if not isinstance(existing, dict): return False hooks = existing.get("hooks", {}) - if not isinstance(hooks, dict): - hooks = {} + # Nothing Specify-owned to strip: leave the user's file untouched. + if not isinstance(hooks, dict) or not _has_marked_entry(hooks.values()): + return False cleaned: dict[str, list] = {} for event, entries in hooks.items(): if not isinstance(entries, list): @@ -2505,7 +2510,8 @@ def _remove_json_root_entries(dst: Path) -> bool: existing = _load_user_json(dst) if existing is None: return False - if not isinstance(existing, dict): + # Nothing Specify-owned to strip: leave the user's file untouched. + if not isinstance(existing, dict) or not _has_marked_entry(existing.values()): return False cleaned: dict[str, Any] = {} for event, entries in existing.items(): @@ -2627,7 +2633,10 @@ def _remove_json_entries(dst: Path) -> bool: if existing is None: return False hooks = existing.get("hooks", {}) - if not isinstance(hooks, dict): + # Nothing Specify-owned to strip: leave the user's file untouched, so a + # no-op event refresh neither reformats nor deletes it (the JSON + # counterpart of the TOML guard added for #4563). + if not isinstance(hooks, dict) or not _has_marked_entry(hooks.values()): return False cleaned: dict[str, list] = {} for event, entries in hooks.items(): @@ -2671,6 +2680,16 @@ def _has_marker(entry: Any) -> bool: return False +def _has_marked_entry(groups: Iterable[Any]) -> bool: + """Return True if any list among *groups* holds a Specify-marked entry.""" + return any( + _has_marker(entry) + for entries in groups + if isinstance(entries, list) + for entry in entries + ) + + def register(app: typer.Typer) -> None: """Attach the event command group to the root application.""" from . import command_run # noqa: F401 -- registers handler diff --git a/tests/specify_cli/events/test_events.py b/tests/specify_cli/events/test_events.py index 787959f060..d3f714d15f 100644 --- a/tests/specify_cli/events/test_events.py +++ b/tests/specify_cli/events/test_events.py @@ -1171,6 +1171,98 @@ def test_owned_only_config_still_deleted_on_teardown(self, tmp_path): assert not config_path.exists() +class TestJsonNoOpRemoval: + """An empty resolved event map must leave a JSON native config that holds + no Specify-owned entries untouched, as #4563 did for Codex's TOML. + + Every ``specify extension add/remove/enable/disable/update`` refreshes + events. With no events configured, the refresh takes + ``install_integration_events``'s empty-map branch into the JSON removal + helpers, which rewrote the user's file even when there was nothing to + strip: re-indenting it, dropping empty lists, or deleting ``{}`` outright. + """ + + # Hand-formatted user configs with a user hook and an empty list that a + # rewrite would drop, and no Specify-owned entries. + USER_CONFIGS = { + "claude": ( + ".claude/settings.json", + b'{\n\t"hooks": {"Notification": [], "Stop": [{"hooks": ' + b'[{"type": "command", "command": "./user-hook.sh"}]}]}\n}', + ), + "copilot": ( + ".github/hooks/speckit.json", + b'{"version": 1, "hooks": {"agentStop": [], "sessionStart": ' + b'[{"type": "command", "bash": "./user-hook.sh"}]}}', + ), + "cursor-agent": ( + ".cursor/hooks.json", + b'{"version": 1, "hooks": {"afterFileEdit": [], "stop": ' + b'[{"command": "./user-hook.sh"}]}}', + ), + "devin": ( + ".devin/hooks.v1.json", + b'{"Stop": [], "SessionStart": [{"matcher": "", "hooks": ' + b'[{"type": "command", "command": "./user-hook.sh"}]}]}', + ), + "opencode": ( + "opencode.json", + b'{"instructions": [], "plugin": ["./user-hook.ts"]}', + ), + } + + def _write_user_config(self, tmp_path, key, content=None): + rel, original = self.USER_CONFIGS[key] + config_path = tmp_path / rel + config_path.parent.mkdir(parents=True, exist_ok=True) + config_path.write_bytes(original if content is None else content) + return config_path + + @pytest.mark.parametrize("key", sorted(USER_CONFIGS)) + def test_no_events_leaves_user_config_untouched(self, tmp_path, key): + config_path = self._write_user_config(tmp_path, key) + original = config_path.read_bytes() + mtime_before = config_path.stat().st_mtime_ns + + install_integration_events( + get_integration(key), tmp_path, _claude_manifest(tmp_path), {} + ) + + assert config_path.read_bytes() == original + assert config_path.stat().st_mtime_ns == mtime_before + + @pytest.mark.parametrize("key", sorted(USER_CONFIGS)) + def test_no_events_keeps_empty_user_config(self, tmp_path, key): + config_path = self._write_user_config(tmp_path, key, content=b"{}") + + install_integration_events( + get_integration(key), tmp_path, _claude_manifest(tmp_path), {} + ) + + assert config_path.read_bytes() == b"{}" + + @pytest.mark.parametrize("key", sorted(USER_CONFIGS)) + def test_no_events_still_strips_owned_entries(self, tmp_path, key): + """The guard must not defeat cleanup when owned entries exist.""" + integration = get_integration(key) + config_path = self._write_user_config(tmp_path, key) + install_integration_events( + integration, tmp_path, _claude_manifest(tmp_path), + {"session_start": [{"command": "speckit.boot"}]}, + ) + merged = config_path.read_text(encoding="utf-8") + assert "__speckit_event__" in merged or "speckit-events.ts" in merged + + install_integration_events( + integration, tmp_path, _claude_manifest(tmp_path), {} + ) + + cleaned = config_path.read_text(encoding="utf-8") + assert "__speckit_event__" not in cleaned + assert "speckit-events.ts" not in cleaned + assert "./user-hook." in cleaned + + # -- Opencode TS Plugin merging --------------------------------------------- class TestOpencodePluginMerging: