fix(cli): accept the emptied hooks: {} mapping in the Hermes hook editor

src/cli/config_yaml_edit.c

@@ -2589,6 +2589,10 @@ typedef struct {
     bool identity_found;
     size_t identity_start;
     size_t identity_end;
+    /* The missing rest of the path begins under a `parent: {}` header. */
+    bool parent_inline_empty;
+    size_t parent_line;
+    size_t parent_colon;
 } yaml_mapping_sequence_target_t;
 
 static void yaml_sequence_key_vec_free(yaml_sequence_key_vec_t *keys) {
@@ -2962,6 +2966,37 @@ static int yaml_sequence_parse_block(const yaml_doc_t *doc, size_t begin_line, s
     return 0;
 }
 
+/* `section: {}` is how the mapping-entry editor writes a section it emptied
+ * (yaml_build_last_mapping_entry_removal), and its upsert accepts it back. On
+ * a PARENT of the sequence path it is that same empty mapping, so the rest of
+ * the path is missing beneath it: removal is a no-op and upsert expands the
+ * header in place. The Hermes remover leaves `hooks: {}` whenever our hook was
+ * the only entry. The sequence key itself holding `{}`, any other flow value,
+ * and content nested under the inline mapping stay refused. */
+static int yaml_sequence_target_empty_parent(const yaml_doc_t *doc, size_t key_line, size_t colon,
+                                             size_t child_end, size_t next_depth,
+                                             size_t sequence_path_len,
+                                             yaml_mapping_sequence_target_t *target) {
+    bool inline_empty = false;
+    if (next_depth >= sequence_path_len ||
+        yaml_tail_is_explicit_empty_mapping(doc->data, colon, doc->lines[key_line].text_end,
+                                            &inline_empty) != 0 ||
+        !inline_empty) {
+        return YAML_ERROR;
+    }
+    for (size_t i = key_line + YAML_UNIT; i < child_end; i++) {
+        if (!doc->lines[i].blank && !doc->lines[i].comment) {
+            return YAML_ERROR;
+        }
+    }
+    target->missing_index = next_depth;
+    target->insert_offset = yaml_sequence_line_offset(doc, child_end);
+    target->parent_inline_empty = true;
+    target->parent_line = key_line;
+    target->parent_colon = colon;
+    return 0;
+}
+
 static int yaml_sequence_analyze(const yaml_doc_t *doc, const char *const *sequence_path,
                                  const size_t *path_lengths, size_t sequence_path_len,
                                  const char *identity_key, const char *identity_value,
@@ -2993,10 +3028,11 @@ static int yaml_sequence_analyze(const yaml_doc_t *doc, const char *const *seque
             return 0;
         }
         const yaml_line_t *header = &doc->lines[key_line];
+        size_t child_end = yaml_sequence_nested_end(doc, key_line, parent_end);
         if (!yaml_tail_is_empty(doc->data, colon, header->text_end)) {
-            return YAML_ERROR;
+            return yaml_sequence_target_empty_parent(doc, key_line, colon, child_end,
+                                                     depth + YAML_UNIT, sequence_path_len, target);
         }
-        size_t child_end = yaml_sequence_nested_end(doc, key_line, parent_end);
         if (depth + YAML_UNIT == sequence_path_len) {
             target->sequence_found = true;
             target->missing_index = sequence_path_len;
@@ -3095,13 +3131,52 @@ static int yaml_sequence_validate_canonical(const char *canonical_item, size_t c
     return result;
 }
 
+/* Copies the document up to insert_offset with the `parent: {}` header turned
+ * into a block header `parent:`, exactly as
+ * yaml_build_inline_empty_mapping_section_add does for a mapping section: a
+ * trailing comment and the line ending are kept. */
+static int yaml_sequence_append_expanded_parent(yaml_buf_t *out, const yaml_doc_t *doc,
+                                                const yaml_mapping_sequence_target_t *target) {
+    if (target->parent_line >= doc->line_count) {
+        return YAML_ERROR;
+    }
+    const yaml_line_t *header = &doc->lines[target->parent_line];
+    size_t comment = 0U;
+    if (target->insert_offset < header->end ||
+        yaml_find_comment(doc->data, target->parent_colon + YAML_UNIT, header->text_end,
+                          &comment) != 0 ||
+        yaml_buf_append(out, doc->data, target->parent_colon + YAML_UNIT) != 0) {
+        return YAML_ERROR;
+    }
+    size_t kept_tail = header->text_end;
+    if (comment < header->text_end) {
+        if (yaml_buf_append_char(out, ' ') != 0) {
+            return YAML_ERROR;
+        }
+        kept_tail = comment;
+    }
+    if (yaml_buf_append(out, doc->data + kept_tail, header->end - kept_tail) != 0 ||
+        yaml_buf_append(out, doc->data + header->end, target->insert_offset - header->end) != 0 ||
+        (out->len > 0U && out->data[out->len - YAML_UNIT] != '\n' &&
+         yaml_buf_append(out, doc->eol, doc->eol_len) != 0)) {
+        return YAML_ERROR;
+    }
+    return 0;
+}
+
 static int yaml_sequence_build_insert(yaml_buf_t *out, const yaml_doc_t *doc,
                                       const yaml_mapping_sequence_target_t *target,
                                       const char *const *sequence_path, const size_t *path_lengths,
                                       size_t sequence_path_len, const yaml_buf_t *rendered_item) {
-    if (target->insert_offset > doc->len ||
-        yaml_buf_append(out, doc->data, target->insert_offset) != 0 ||
-        yaml_append_separator_if_needed(out, doc, target->insert_offset) != 0) {
+    if (target->insert_offset > doc->len) {
+        return YAML_ERROR;
+    }
+    if (target->parent_inline_empty) {
+        if (yaml_sequence_append_expanded_parent(out, doc, target) != 0) {
+            return YAML_ERROR;
+        }
+    } else if (yaml_buf_append(out, doc->data, target->insert_offset) != 0 ||
+               yaml_append_separator_if_needed(out, doc, target->insert_offset) != 0) {
         return YAML_ERROR;
     }
     if (!target->sequence_found) {

tests/test_cli.c

@@ -9450,6 +9450,143 @@ TEST(cli_hermes_stable_shell_context_contract) {
     PASS();
 }
 
+/* A fake HOME holding only ~/.hermes/config.yaml, so install and uninstall
+ * reach exactly the Hermes editors. */
+typedef struct {
+    char tmpdir[256];
+    char config_path[640];
+    char binary_path[640];
+    char *saved_home;
+    char *saved_path;
+    char *saved_hermes;
+} cli_hermes_cycle_t;
+
+static bool cli_hermes_cycle_open(cli_hermes_cycle_t *cycle, const char *initial_config) {
+    memset(cycle, 0, sizeof(*cycle));
+    snprintf(cycle->tmpdir, sizeof(cycle->tmpdir), "/tmp/cli-hermes-cycle-XXXXXX");
+    if (!cbm_mkdtemp(cycle->tmpdir)) {
+        cycle->tmpdir[0] = '\0';
+        return false;
+    }
+    cycle->saved_home = save_test_env("HOME");
+    cycle->saved_path = save_test_env("PATH");
+    cycle->saved_hermes = save_test_env("HERMES_HOME");
+    cbm_setenv("HOME", cycle->tmpdir, 1);
+    cbm_setenv("PATH", cycle->tmpdir, 1);
+    cbm_unsetenv("HERMES_HOME");
+
+    char hermes_dir[512];
+    snprintf(hermes_dir, sizeof(hermes_dir), "%s/.hermes", cycle->tmpdir);
+    snprintf(cycle->config_path, sizeof(cycle->config_path), "%s/config.yaml", hermes_dir);
+#ifdef _WIN32
+    snprintf(cycle->binary_path, sizeof(cycle->binary_path),
+             "%s/.local/bin/codebase-memory-mcp.exe", cycle->tmpdir);
+#else
+    snprintf(cycle->binary_path, sizeof(cycle->binary_path), "%s/.local/bin/codebase-memory-mcp",
+             cycle->tmpdir);
+#endif
+    test_mkdirp(hermes_dir);
+    return write_test_file(cycle->config_path, initial_config) == 0;
+}
+
+static void cli_hermes_cycle_close(cli_hermes_cycle_t *cycle) {
+    if (!cycle->tmpdir[0]) {
+        return;
+    }
+    restore_test_env("HOME", cycle->saved_home);
+    restore_test_env("PATH", cycle->saved_path);
+    restore_test_env("HERMES_HOME", cycle->saved_hermes);
+    test_rmdir_r(cycle->tmpdir);
+    cycle->tmpdir[0] = '\0';
+}
+
+/* Found while finishing #1862: when our hook was the only entry under
+ * `hooks:`, uninstall empties that mapping and the mapping editor writes it as
+ * `hooks: {}` (the same convention as `mcp_servers: {}`). The hook sequence
+ * editor then refused the file, so install -> uninstall -> install exited 1,
+ * and so did a repeated uninstall. The explicit empty mapping must be accepted:
+ * a repeated uninstall changes nothing and the reinstall restores the first
+ * install byte for byte, with exactly one owned hook. */
+TEST(cli_hermes_hook_reinstalls_after_uninstall_empties_hooks) {
+    cli_hermes_cycle_t cycle;
+    if (!cli_hermes_cycle_open(&cycle, "theme: solarized\n")) {
+        cli_hermes_cycle_close(&cycle);
+        FAIL("Hermes fake-HOME setup failed");
+    }
+    char *argv[] = {"uninstall", "--yes"};
+    int install_rc = cbm_install_agent_configs(cycle.tmpdir, cycle.binary_path, false, false);
+    char *installed = read_test_file_alloc(cycle.config_path);
+    int uninstall_rc = cli_test_cmd_uninstall(2, argv);
+    char *emptied = read_test_file_alloc(cycle.config_path);
+    int repeat_rc = cli_test_cmd_uninstall(2, argv);
+    char *repeated = read_test_file_alloc(cycle.config_path);
+    int reinstall_rc = cbm_install_agent_configs(cycle.tmpdir, cycle.binary_path, false, false);
+    char *reinstalled = read_test_file_alloc(cycle.config_path);
+    cli_hermes_cycle_close(&cycle);
+
+    bool installed_once = install_rc == 0 && installed &&
+                          test_count_substring(installed, "pre_llm_call:") == 1U &&
+                          test_count_substring(installed, "hook-augment") == 1U;
+    bool emptied_clean = uninstall_rc == 0 && emptied && strstr(emptied, "theme: solarized\n") &&
+                         test_count_substring(emptied, "hooks: {}\n") == 1U &&
+                         !strstr(emptied, "pre_llm_call") && !strstr(emptied, "hook-augment");
+    bool repeat_unchanged = repeat_rc == 0 && emptied && repeated && strcmp(repeated, emptied) == 0;
+    bool reinstalled_once =
+        reinstall_rc == 0 && installed && reinstalled && strcmp(reinstalled, installed) == 0;
+    free(installed);
+    free(emptied);
+    free(repeated);
+    free(reinstalled);
+    if (!installed_once)
+        FAIL("Hermes install must write exactly one owned pre_llm_call hook");
+    if (!emptied_clean)
+        FAIL("uninstall must remove the owned hook and leave only the empty hooks: {} mapping");
+    if (!repeat_unchanged)
+        FAIL("a repeated uninstall must accept hooks: {} and leave the file unchanged");
+    if (!reinstalled_once)
+        FAIL("reinstall must accept hooks: {} and restore exactly one owned hook");
+    PASS();
+}
+
+/* The same cycle with a user's own hook in the shared pre_llm_call sequence:
+ * the foreign hook survives untouched and uninstall restores the original
+ * file byte for byte. */
+TEST(cli_hermes_hook_cycle_keeps_foreign_pre_llm_hook) {
+    static const char original[] = "theme: solarized\n"
+                                   "mcp_servers: {}\n"
+                                   "hooks:\n"
+                                   "  pre_llm_call:\n"
+                                   "    - command: \"/usr/bin/user-hermes-hook\"\n";
+    cli_hermes_cycle_t cycle;
+    if (!cli_hermes_cycle_open(&cycle, original)) {
+        cli_hermes_cycle_close(&cycle);
+        FAIL("Hermes fake-HOME setup failed");
+    }
+    char *argv[] = {"uninstall", "--yes"};
+    int install_rc = cbm_install_agent_configs(cycle.tmpdir, cycle.binary_path, false, false);
+    char *installed = read_test_file_alloc(cycle.config_path);
+    int uninstall_rc = cli_test_cmd_uninstall(2, argv);
+    char *restored = read_test_file_alloc(cycle.config_path);
+    int reinstall_rc = cbm_install_agent_configs(cycle.tmpdir, cycle.binary_path, false, false);
+    char *reinstalled = read_test_file_alloc(cycle.config_path);
+    cli_hermes_cycle_close(&cycle);
+
+    bool merged = install_rc == 0 && installed &&
+                  test_count_substring(installed, "/usr/bin/user-hermes-hook") == 1U &&
+                  test_count_substring(installed, "pre_llm_call:") == 1U &&
+                  test_count_substring(installed, "hook-augment") == 1U;
+    bool restored_exactly = uninstall_rc == 0 && restored && strcmp(restored, original) == 0;
+    bool reinstalled_once =
+        reinstall_rc == 0 && installed && reinstalled && strcmp(reinstalled, installed) == 0;
+    free(installed);
+    free(restored);
+    free(reinstalled);
+    if (!merged || !restored_exactly || !reinstalled_once)
+        FAIL("a user's own pre_llm_call hook must survive install, uninstall and reinstall, and "
+             "uninstall must restore the original file byte for byte");
+    PASS();
+}
+
 #ifndef _WIN32
 TEST(cli_detected_agent_summary_includes_registry_clients) {
     char tmpdir[256];
@@ -20553,6 +20690,8 @@ SUITE(cli) {
     RUN_TEST(cli_junie_current_durable_context_contract);
     RUN_TEST(cli_rovo_installs_documented_global_memory);
     RUN_TEST(cli_hermes_stable_shell_context_contract);
+    RUN_TEST(cli_hermes_hook_reinstalls_after_uninstall_empties_hooks);
+    RUN_TEST(cli_hermes_hook_cycle_keeps_foreign_pre_llm_hook);
 #ifndef _WIN32
     RUN_TEST(cli_detected_agent_summary_includes_registry_clients);
 #endif