From 6e4be522085af1e5ab0031bd9132a62d2caf6887 Mon Sep 17 00:00:00 2001 From: Ilan Lidovski Date: Wed, 2 Sep 2026 12:06:15 +0300 Subject: [PATCH 1/2] CM-71886: Keep the hooks file on uninstall instead of unlinking it When Cycode's hooks were the only hooks left, uninstall deleted the whole file and with it any unrelated top-level keys (mcpServers, permissions, ...) in a shared settings file such as Claude Code's settings.json. Drop the emptied hooks key and write the remainder back instead. Co-Authored-By: Claude Fable 5.1 --- .../cli/apps/ai_guardrails/hooks_manager.py | 8 ++---- .../ai_guardrails/test_hooks_manager.py | 26 ++++++++++++++++--- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/cycode/cli/apps/ai_guardrails/hooks_manager.py b/cycode/cli/apps/ai_guardrails/hooks_manager.py index fa8e095e..73867475 100644 --- a/cycode/cli/apps/ai_guardrails/hooks_manager.py +++ b/cycode/cli/apps/ai_guardrails/hooks_manager.py @@ -190,13 +190,9 @@ def _persist_uninstall(hooks_path: Path, existing: dict, modified: bool) -> tupl """Apply the uninstall result to disk and return ``(success, message)``.""" if not modified: return True, 'No Cycode hooks found to remove' + # Never unlink: a shared settings file (Claude Code's settings.json) holds unrelated keys. if not existing.get('hooks'): - try: - hooks_path.unlink() - except Exception as e: - logger.debug('Failed to delete hooks file', exc_info=e) - return False, f'Failed to remove hooks file: {hooks_path}' - return True, f'Removed hooks file: {hooks_path}' + existing.pop('hooks', None) if not _save_hooks_file(hooks_path, existing): return False, f'Failed to update hooks file: {hooks_path}' return True, f'Cycode hooks removed from: {hooks_path}' diff --git a/tests/cli/commands/ai_guardrails/test_hooks_manager.py b/tests/cli/commands/ai_guardrails/test_hooks_manager.py index 67dfbeb1..ed1142ab 100644 --- a/tests/cli/commands/ai_guardrails/test_hooks_manager.py +++ b/tests/cli/commands/ai_guardrails/test_hooks_manager.py @@ -261,9 +261,30 @@ def test_uninstall_preserves_user_hook_colocated_with_cycode( assert not any('cycode ai-guardrails' in c for c in commands) +def test_uninstall_keeps_shared_settings_file_when_cycode_hooks_were_the_only_hooks(fs: FakeFilesystem) -> None: + """Claude Code's settings.json is a general-purpose file: when Cycode's hooks were the + only hooks in it, uninstall must write back the unrelated top-level keys, not unlink.""" + claude_code = ClaudeCode() + hooks_path = claude_code.settings_path('user') + mcp_servers = {'my-server': {'command': 'npx', 'args': ['-y', 'my-mcp-server']}} + fs.create_file(hooks_path, contents=json.dumps({'mcpServers': mcp_servers, 'permissions': {'allow': ['Bash']}})) + + success, _ = install_hooks(claude_code) + assert success is True + + success, _ = uninstall_hooks(claude_code) + assert success is True + + assert hooks_path.exists(), 'uninstall deleted a settings file it does not own' + saved = json.loads(hooks_path.read_text()) + assert saved['mcpServers'] == mcp_servers + assert saved['permissions'] == {'allow': ['Bash']} + assert not saved.get('hooks') + + def test_copilot_dedicated_file_install_uninstall_lifecycle(fs: FakeFilesystem) -> None: """Copilot uses a dedicated Cycode-owned file: install creates it from - scratch, reinstall is idempotent, uninstall removes the file entirely.""" + scratch, reinstall is idempotent, uninstall drops the emptied hooks key.""" copilot = Copilot() hooks_path = copilot.settings_path('user') @@ -281,10 +302,9 @@ def test_copilot_dedicated_file_install_uninstall_lifecycle(fs: FakeFilesystem) assert all(len(entries) == 1 for entries in saved['hooks'].values()) assert saved['hooks']['PreToolUse'][0]['command'] == 'cycode ai-guardrails scan --ide copilot' - # Uninstall deletes the emptied dedicated file rather than leaving a husk. success, _ = uninstall_hooks(copilot) assert success is True - assert not hooks_path.exists() + assert json.loads(hooks_path.read_text()) == {'version': 1} def test_create_policy_file_repo_scope(fs: FakeFilesystem) -> None: From 2205dc646065b63b4b61c0feb3ee85345ae39be6 Mon Sep 17 00:00:00 2001 From: Ilan Lidovski Date: Wed, 2 Sep 2026 12:19:06 +0300 Subject: [PATCH 2/2] CM-71886: Use repo scope in the shared-settings uninstall test The user-scope Claude Code path is built from Path.home() at import time, which pyfakefs does not intercept on Python 3.9, so the test only passed where a real ~/.claude/settings.json happened to exist. Co-Authored-By: Claude Fable 5.1 --- tests/cli/commands/ai_guardrails/test_hooks_manager.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/cli/commands/ai_guardrails/test_hooks_manager.py b/tests/cli/commands/ai_guardrails/test_hooks_manager.py index ed1142ab..2212e798 100644 --- a/tests/cli/commands/ai_guardrails/test_hooks_manager.py +++ b/tests/cli/commands/ai_guardrails/test_hooks_manager.py @@ -264,15 +264,19 @@ def test_uninstall_preserves_user_hook_colocated_with_cycode( def test_uninstall_keeps_shared_settings_file_when_cycode_hooks_were_the_only_hooks(fs: FakeFilesystem) -> None: """Claude Code's settings.json is a general-purpose file: when Cycode's hooks were the only hooks in it, uninstall must write back the unrelated top-level keys, not unlink.""" + # Repo scope: the user-scope path is built from Path.home() at import time, which + # pyfakefs does not intercept on Python 3.9. + repo = Path('/repo') + fs.create_dir(repo) claude_code = ClaudeCode() - hooks_path = claude_code.settings_path('user') + hooks_path = claude_code.settings_path('repo', repo) mcp_servers = {'my-server': {'command': 'npx', 'args': ['-y', 'my-mcp-server']}} fs.create_file(hooks_path, contents=json.dumps({'mcpServers': mcp_servers, 'permissions': {'allow': ['Bash']}})) - success, _ = install_hooks(claude_code) + success, _ = install_hooks(claude_code, scope='repo', repo_path=repo) assert success is True - success, _ = uninstall_hooks(claude_code) + success, _ = uninstall_hooks(claude_code, scope='repo', repo_path=repo) assert success is True assert hooks_path.exists(), 'uninstall deleted a settings file it does not own'