Merge pull request #1435 from DeusData/fix/1387-install-preserves-hooks

fix(install): never remove existing hook entries on a refused script rewrite (#1387)

Author: DeusData

src/cli/cli.c

@@ -7177,9 +7177,13 @@ static void install_claude_code_config(const char *home, const char *binary_path
         char hook_path[CLI_BUF_1K];
         gate_ok = cbm_install_hook_gate_script(home, binary_path);
         snprintf(hook_path, sizeof(hook_path), "%s/hooks/%s", config_dir, CMM_HOOK_GATE_SCRIPT);
+        /* #1387: a failed script (re)write must never remove existing hook
+         * entries. The common failure is TEXT_UNOWNED - a script the user
+         * modified or a manual install wrote with another binary path - and
+         * that script still works; deleting the registration turns a skipped
+         * update into config loss. Entry removal belongs to uninstall only. */
         if (!gate_ok) {
             record_agent_config_error(false, "Claude Code", "hook_script_install", hook_path);
-            (void)cbm_remove_claude_hooks(settings_path);
         } else if (cbm_upsert_claude_hooks(settings_path) != CLI_OK) {
             gate_ok = false;
             record_agent_config_error(false, "Claude Code", "hook_register", settings_path);
@@ -7190,7 +7194,6 @@ static void install_claude_code_config(const char *home, const char *binary_path
                  CMM_SESSION_REMINDER_SCRIPT);
         if (!session_ok) {
             record_agent_config_error(false, "Claude Code", "hook_script_install", hook_path);
-            (void)cbm_remove_session_hooks(settings_path);
         } else if (cbm_upsert_session_hooks(settings_path) != CLI_OK) {
             session_ok = false;
             record_agent_config_error(false, "Claude Code", "hook_register", settings_path);
@@ -7201,7 +7204,6 @@ static void install_claude_code_config(const char *home, const char *binary_path
                  CMM_SUBAGENT_REMINDER_SCRIPT);
         if (!subagent_ok) {
             record_agent_config_error(false, "Claude Code", "hook_script_install", hook_path);
-            (void)cbm_remove_claude_subagent_hooks(settings_path);
         } else if (cbm_upsert_claude_subagent_hooks(settings_path) != CLI_OK) {
             subagent_ok = false;
             record_agent_config_error(false, "Claude Code", "hook_register", settings_path);

tests/test_cli.c

@@ -4670,6 +4670,108 @@ TEST(cli_agent_reinstall_preserves_foreign_policy_entries) {
     PASS();
 }
 
+#ifndef _WIN32
+/* Regression for #1387: installing over an existing setup whose hook scripts
+ * are not byte-owned (manual install with a custom binary location, or a
+ * user-modified reminder) must NOT remove the existing, working hook entries
+ * from settings.json. The refused script rewrite is reported as an error and
+ * both the scripts and the entries survive byte-identically. */
+TEST(cli_install_preserves_hook_entries_when_scripts_unowned_issue1387) {
+    char tmpdir[256];
+    snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-hooks-preserve-XXXXXX");
+    if (!cbm_mkdtemp(tmpdir))
+        FAIL("cbm_mkdtemp failed");
+
+    char hooks_dir[512];
+    snprintf(hooks_dir, sizeof(hooks_dir), "%s/.claude/hooks", tmpdir);
+    if (!cbm_mkdir_p(hooks_dir, 0755))
+        FAIL("mkdir hooks_dir failed");
+
+    /* Gate script written by a manual install: BIN outside the managed target,
+     * so its bytes match no current or released installer-owned shape. */
+    static const char unowned_gate[] =
+        "#!/usr/bin/env bash\n"
+        "# codebase-memory-mcp search augmenter (Claude Code PreToolUse).\n"
+        "BIN=\"/opt/tools/cbm/codebase-memory-mcp\"\n"
+        "[ -x \"$BIN\" ] || exit 0\n"
+        "\"$BIN\" hook-augment 2>/dev/null\n"
+        "exit 0\n";
+    char gate_path[768];
+    snprintf(gate_path, sizeof(gate_path), "%s/cbm-code-discovery-gate", hooks_dir);
+    write_test_file(gate_path, unowned_gate);
+
+    /* Session reminder carrying a user-added line. */
+    static const char unowned_session[] =
+        "#!/usr/bin/env bash\n"
+        "# SessionStart hook: remind agent to use codebase-memory-mcp tools.\n"
+        "echo my-extra-team-reminder\n";
+    char session_path[768];
+    snprintf(session_path, sizeof(session_path), "%s/cbm-session-reminder", hooks_dir);
+    write_test_file(session_path, unowned_session);
+
+    static const char settings_before[] =
+        "{\n"
+        "  \"hooks\": {\n"
+        "    \"PreToolUse\": [\n"
+        "      {\"matcher\": \"Grep|Glob\", \"hooks\": [{\"type\": \"command\", "
+        "\"command\": \"~/.claude/hooks/cbm-code-discovery-gate\", \"timeout\": 5}]},\n"
+        "      {\"matcher\": \"Bash\", \"hooks\": [{\"type\": \"command\", "
+        "\"command\": \"/usr/local/bin/my-own-guard\"}]}\n"
+        "    ],\n"
+        "    \"SessionStart\": [\n"
+        "      {\"matcher\": \"startup\", \"hooks\": [{\"type\": \"command\", "
+        "\"command\": \"~/.claude/hooks/cbm-session-reminder\"}]},\n"
+        "      {\"matcher\": \"resume\", \"hooks\": [{\"type\": \"command\", "
+        "\"command\": \"~/.claude/hooks/cbm-session-reminder\"}]},\n"
+        "      {\"matcher\": \"clear\", \"hooks\": [{\"type\": \"command\", "
+        "\"command\": \"~/.claude/hooks/cbm-session-reminder\"}]},\n"
+        "      {\"matcher\": \"compact\", \"hooks\": [{\"type\": \"command\", "
+        "\"command\": \"~/.claude/hooks/cbm-session-reminder\"}]}\n"
+        "    ]\n"
+        "  }\n"
+        "}\n";
+    char settings_path[768];
+    snprintf(settings_path, sizeof(settings_path), "%s/.claude/settings.json", tmpdir);
+    write_test_file(settings_path, settings_before);
+
+    const char *const env_names[] = {"HOME", "PATH", "CLAUDE_CONFIG_DIR"};
+    char *saved[sizeof(env_names) / sizeof(env_names[0])];
+    for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) {
+        saved[i] = save_test_env(env_names[i]);
+        cbm_unsetenv(env_names[i]);
+    }
+    cbm_setenv("HOME", tmpdir, 1);
+    cbm_setenv("PATH", tmpdir, 1);
+
+    int install_rc =
+        cbm_install_agent_configs(tmpdir, "/opt/other/codebase-memory-mcp", false, false);
+
+    char *settings = read_test_file_alloc(settings_path);
+    char *gate = read_test_file_alloc(gate_path);
+    char *session = read_test_file_alloc(session_path);
+    bool preserved =
+        install_rc != 0 /* the refused rewrites are reported, not silent */
+        && settings && strstr(settings, "~/.claude/hooks/cbm-code-discovery-gate") &&
+        strstr(settings, "Grep|Glob") && strstr(settings, "/usr/local/bin/my-own-guard") &&
+        strstr(settings, "\"startup\"") && strstr(settings, "\"resume\"") &&
+        strstr(settings, "\"clear\"") && strstr(settings, "\"compact\"") &&
+        strstr(settings, "~/.claude/hooks/cbm-session-reminder") && gate &&
+        strcmp(gate, unowned_gate) == 0 && session && strcmp(session, unowned_session) == 0;
+    free(settings);
+    free(gate);
+    free(session);
+
+    for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) {
+        restore_test_env(env_names[i], saved[i]);
+    }
+    test_rmdir_r(tmpdir);
+    if (!preserved)
+        FAIL("install must preserve existing hook entries and scripts when the on-disk "
+             "scripts are unowned (#1387)");
+    PASS();
+}
+#endif
+
 TEST(cli_existing_agents_install_durable_child_context) {
     char tmpdir[256];
     snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-durable-agents-XXXXXX");
@@ -11936,6 +12038,9 @@ SUITE(cli) {
     RUN_TEST(cli_new_agent_install_plans_use_documented_paths);
     RUN_TEST(cli_new_agent_configs_use_documented_schemas);
     RUN_TEST(cli_agent_reinstall_preserves_foreign_policy_entries);
+#ifndef _WIN32
+    RUN_TEST(cli_install_preserves_hook_entries_when_scripts_unowned_issue1387);
+#endif
     RUN_TEST(cli_existing_agents_install_durable_child_context);
     RUN_TEST(cli_durable_profiles_follow_current_vendor_paths);
     RUN_TEST(cli_cline_data_dir_only_redirects_data_state);