Skip to content

Commit 770c563

Browse files
yxsheeclaude
andauthored
fix(events): leave JSON hook configs untouched when nothing is owned (#5017)
`specify extension add`, `remove`, `enable`, `disable` and `update`, `specify init` with extensions, and integration setup all refresh runtime events. With no events configured, the refresh takes install_integration_events()'s empty-map branch into _remove_native_event_hooks(), and its JSON removal helpers rewrote the agent's native config even when it held no Specify-owned entries. In a plain Claude project (likewise Gemini, Qwen, Tabnine, Cursor, Copilot, Devin and opencode), adding an extension that declares no events re-indented the user's .claude/settings.json or opencode.json, dropped empty values such as "hooks": {"Notification": []} or "plugin": [], and deleted a config whose whole content was {}. #4563 fixed the same defect for Codex's TOML in _remove_toml_entries(). Give the JSON helpers the matching guard: _remove_json_entries(), _remove_copilot_entries() and _remove_json_root_entries() return early when no Specify-marked entry is present, and _remove_opencode_entries() returns early when opencode.json does not reference the Spec Kit plugin. Files that do hold owned entries are still stripped, rewritten or deleted as before. Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 244e752 commit 770c563

2 files changed

Lines changed: 122 additions & 11 deletions

File tree

‎src/specify_cli/events/__init__.py‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
from ..integrations._file_changes import unlink as _unlink_file, write_text as _write_text
3333

3434
if TYPE_CHECKING:
35+
from collections.abc import Iterable
36+
3537
from ..integrations.base import IntegrationBase
3638
from ..integrations.manifest import IntegrationManifest
3739

@@ -2153,13 +2155,15 @@ def _remove_opencode_entries(config_path: Path) -> bool:
21532155
if existing is None:
21542156
return False
21552157
plugins = existing.get("plugin", [])
2156-
if isinstance(plugins, list):
2157-
ref = "./.opencode/plugin/speckit-events.ts"
2158-
plugins = [p for p in plugins if p != ref]
2159-
if plugins:
2160-
existing["plugin"] = plugins
2161-
else:
2162-
existing.pop("plugin", None)
2158+
ref = "./.opencode/plugin/speckit-events.ts"
2159+
# No Spec Kit plugin ref to strip: leave the user's file untouched.
2160+
if not isinstance(plugins, list) or ref not in plugins:
2161+
return False
2162+
plugins = [p for p in plugins if p != ref]
2163+
if plugins:
2164+
existing["plugin"] = plugins
2165+
else:
2166+
existing.pop("plugin", None)
21632167
if not existing:
21642168
_unlink_file(config_path, missing_ok=True)
21652169
return True
@@ -2376,8 +2380,9 @@ def _remove_copilot_entries(dst: Path) -> bool:
23762380
if not isinstance(existing, dict):
23772381
return False
23782382
hooks = existing.get("hooks", {})
2379-
if not isinstance(hooks, dict):
2380-
hooks = {}
2383+
# Nothing Specify-owned to strip: leave the user's file untouched.
2384+
if not isinstance(hooks, dict) or not _has_marked_entry(hooks.values()):
2385+
return False
23812386
cleaned: dict[str, list] = {}
23822387
for event, entries in hooks.items():
23832388
if not isinstance(entries, list):
@@ -2505,7 +2510,8 @@ def _remove_json_root_entries(dst: Path) -> bool:
25052510
existing = _load_user_json(dst)
25062511
if existing is None:
25072512
return False
2508-
if not isinstance(existing, dict):
2513+
# Nothing Specify-owned to strip: leave the user's file untouched.
2514+
if not isinstance(existing, dict) or not _has_marked_entry(existing.values()):
25092515
return False
25102516
cleaned: dict[str, Any] = {}
25112517
for event, entries in existing.items():
@@ -2627,7 +2633,10 @@ def _remove_json_entries(dst: Path) -> bool:
26272633
if existing is None:
26282634
return False
26292635
hooks = existing.get("hooks", {})
2630-
if not isinstance(hooks, dict):
2636+
# Nothing Specify-owned to strip: leave the user's file untouched, so a
2637+
# no-op event refresh neither reformats nor deletes it (the JSON
2638+
# counterpart of the TOML guard added for #4563).
2639+
if not isinstance(hooks, dict) or not _has_marked_entry(hooks.values()):
26312640
return False
26322641
cleaned: dict[str, list] = {}
26332642
for event, entries in hooks.items():
@@ -2671,6 +2680,16 @@ def _has_marker(entry: Any) -> bool:
26712680
return False
26722681

26732682

2683+
def _has_marked_entry(groups: Iterable[Any]) -> bool:
2684+
"""Return True if any list among *groups* holds a Specify-marked entry."""
2685+
return any(
2686+
_has_marker(entry)
2687+
for entries in groups
2688+
if isinstance(entries, list)
2689+
for entry in entries
2690+
)
2691+
2692+
26742693
def register(app: typer.Typer) -> None:
26752694
"""Attach the event command group to the root application."""
26762695
from . import command_run # noqa: F401 -- registers handler

‎tests/specify_cli/events/test_events.py‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,6 +1171,98 @@ def test_owned_only_config_still_deleted_on_teardown(self, tmp_path):
11711171
assert not config_path.exists()
11721172

11731173

1174+
class TestJsonNoOpRemoval:
1175+
"""An empty resolved event map must leave a JSON native config that holds
1176+
no Specify-owned entries untouched, as #4563 did for Codex's TOML.
1177+
1178+
Every ``specify extension add/remove/enable/disable/update`` refreshes
1179+
events. With no events configured, the refresh takes
1180+
``install_integration_events``'s empty-map branch into the JSON removal
1181+
helpers, which rewrote the user's file even when there was nothing to
1182+
strip: re-indenting it, dropping empty lists, or deleting ``{}`` outright.
1183+
"""
1184+
1185+
# Hand-formatted user configs with a user hook and an empty list that a
1186+
# rewrite would drop, and no Specify-owned entries.
1187+
USER_CONFIGS = {
1188+
"claude": (
1189+
".claude/settings.json",
1190+
b'{\n\t"hooks": {"Notification": [], "Stop": [{"hooks": '
1191+
b'[{"type": "command", "command": "./user-hook.sh"}]}]}\n}',
1192+
),
1193+
"copilot": (
1194+
".github/hooks/speckit.json",
1195+
b'{"version": 1, "hooks": {"agentStop": [], "sessionStart": '
1196+
b'[{"type": "command", "bash": "./user-hook.sh"}]}}',
1197+
),
1198+
"cursor-agent": (
1199+
".cursor/hooks.json",
1200+
b'{"version": 1, "hooks": {"afterFileEdit": [], "stop": '
1201+
b'[{"command": "./user-hook.sh"}]}}',
1202+
),
1203+
"devin": (
1204+
".devin/hooks.v1.json",
1205+
b'{"Stop": [], "SessionStart": [{"matcher": "", "hooks": '
1206+
b'[{"type": "command", "command": "./user-hook.sh"}]}]}',
1207+
),
1208+
"opencode": (
1209+
"opencode.json",
1210+
b'{"instructions": [], "plugin": ["./user-hook.ts"]}',
1211+
),
1212+
}
1213+
1214+
def _write_user_config(self, tmp_path, key, content=None):
1215+
rel, original = self.USER_CONFIGS[key]
1216+
config_path = tmp_path / rel
1217+
config_path.parent.mkdir(parents=True, exist_ok=True)
1218+
config_path.write_bytes(original if content is None else content)
1219+
return config_path
1220+
1221+
@pytest.mark.parametrize("key", sorted(USER_CONFIGS))
1222+
def test_no_events_leaves_user_config_untouched(self, tmp_path, key):
1223+
config_path = self._write_user_config(tmp_path, key)
1224+
original = config_path.read_bytes()
1225+
mtime_before = config_path.stat().st_mtime_ns
1226+
1227+
install_integration_events(
1228+
get_integration(key), tmp_path, _claude_manifest(tmp_path), {}
1229+
)
1230+
1231+
assert config_path.read_bytes() == original
1232+
assert config_path.stat().st_mtime_ns == mtime_before
1233+
1234+
@pytest.mark.parametrize("key", sorted(USER_CONFIGS))
1235+
def test_no_events_keeps_empty_user_config(self, tmp_path, key):
1236+
config_path = self._write_user_config(tmp_path, key, content=b"{}")
1237+
1238+
install_integration_events(
1239+
get_integration(key), tmp_path, _claude_manifest(tmp_path), {}
1240+
)
1241+
1242+
assert config_path.read_bytes() == b"{}"
1243+
1244+
@pytest.mark.parametrize("key", sorted(USER_CONFIGS))
1245+
def test_no_events_still_strips_owned_entries(self, tmp_path, key):
1246+
"""The guard must not defeat cleanup when owned entries exist."""
1247+
integration = get_integration(key)
1248+
config_path = self._write_user_config(tmp_path, key)
1249+
install_integration_events(
1250+
integration, tmp_path, _claude_manifest(tmp_path),
1251+
{"session_start": [{"command": "speckit.boot"}]},
1252+
)
1253+
merged = config_path.read_text(encoding="utf-8")
1254+
assert "__speckit_event__" in merged or "speckit-events.ts" in merged
1255+
1256+
install_integration_events(
1257+
integration, tmp_path, _claude_manifest(tmp_path), {}
1258+
)
1259+
1260+
cleaned = config_path.read_text(encoding="utf-8")
1261+
assert "__speckit_event__" not in cleaned
1262+
assert "speckit-events.ts" not in cleaned
1263+
assert "./user-hook." in cleaned
1264+
1265+
11741266
# -- Opencode TS Plugin merging ---------------------------------------------
11751267

11761268
class TestOpencodePluginMerging:

0 commit comments

Comments
 (0)