From d664dbc48600ca3720b26d7dc48c94ed4f5c2ee3 Mon Sep 17 00:00:00 2001 From: Andrew Hundt Date: Mon, 20 Jul 2026 02:58:08 -0400 Subject: [PATCH] fix(install): migrate owned Claude and Codex state src/cli/cli.c:3926 records the byte-exact SessionStart script emitted by the streamlined auto-index release, and cbm_install_session_reminder_script now accepts both released installer-owned variants while preserving edited files. src/cli/config_toml_edit.c:987 adds a narrowly selected managed-block path that permits TOML child-before-parent ordering. cbm_upsert_codex_mcp at src/cli/cli.c:2247 uses it to replace the owned mcp_servers.codebase-memory-mcp root without deleting user-owned tools.query_graph approval policy. tests/test_cli.c:4492 and tests/test_cli.c:5131 cover exact Claude hook migration plus idempotent Codex root migration with descendant policy preservation. Verified: 233/233 CLI ASan/UBSan tests; 34/34 config_toml_edit ASan/UBSan tests; scripts/check-source-safety.sh; git diff --check. Signed-off-by: Andrew Hundt --- src/cli/cli.c | 36 +++++++++++++++++--- src/cli/config_toml_edit.c | 49 +++++++++++++++++++-------- src/cli/config_toml_edit.h | 6 ++++ tests/test_cli.c | 69 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 140 insertions(+), 20 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index 9ccb2d5cd..cd5cb9769 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -2244,7 +2244,10 @@ int cbm_upsert_codex_mcp(const char *binary_path, const char *config_path) { cbm_remove_codex_legacy_mcp(config_path) != 0) { return CLI_ERR; } - return cbm_toml_upsert_managed_block(config_path, CODEX_MCP_BEGIN, CODEX_MCP_END, block) == 0 + /* Per-tool approval tables are user policy below the installer-owned MCP + * root. Preserve them across upgrades; TOML permits child-before-parent. */ + return cbm_toml_upsert_managed_block_preserve_descendants(config_path, CODEX_MCP_BEGIN, + CODEX_MCP_END, block) == 0 ? CLI_OK : CLI_ERR; } @@ -3920,6 +3923,29 @@ static const char cmm_released_session_script[] = "3. If a project is not indexed yet, run index_repository FIRST.\n" "REMINDER\n"; +/* Exact SessionStart document emitted by the streamlined auto-index release. + * Keep released installer bytes explicit so upgrades migrate only known-owned + * files; a user-edited near-match continues to fail closed. */ +static const char cmm_released_streamlined_session_script[] = + "#!/usr/bin/env bash\n" + "# SessionStart hook: remind agent to use codebase-memory-mcp tools.\n" + "# Installed by codebase-memory-mcp. Fires on startup/resume/clear/compact.\n" + "cat << 'REMINDER'\n" + "Code Discovery Protocol:\n" + "1. Prefer codebase-memory-mcp tools first for structural code exploration:\n" + " - search_graph(name_pattern/label/qn_pattern) to find functions/classes/routes\n" + " - trace_path(function_name, mode=calls|data_flow|cross_service) for call chains\n" + " - get_code(qualified_name) for exact symbol source in streamlined mode\n" + " - query_graph(query) for complex Cypher patterns\n" + " - search_code(pattern) for text/regex source search in an indexed project\n" + "2. Use Grep/Glob/Read freely for text, configs, non-code files, and\n" + " always Read a file before editing it.\n" + "3. Graph-backed tools auto-index the server CWD or explicit repo paths when\n" + " auto_index=true and under auto_index_limit. search_code needs an\n" + " indexed project. Use _hidden_tools\n" + " to reveal index_repository or get_architecture when explicit control is needed.\n" + "REMINDER\n"; + static const char cmm_released_subagent_script[] = "#!/usr/bin/env bash\n" "# SubagentStart hook: tell subagents to use codebase-memory-mcp tools.\n" @@ -4071,14 +4097,16 @@ static bool cbm_install_session_reminder_script(const char *home, const char *bi sizeof(script)) != CLI_OK) { return false; } - const char *const legacy[] = {cmm_released_session_script}; + const char *const legacy[] = {cmm_released_session_script, + cmm_released_streamlined_session_script}; #ifdef _WIN32 if (cbm_remove_owned_legacy_hook_script(hooks_dir, CMM_SESSION_REMINDER_SCRIPT_LEGACY, script, - legacy, 1U) != CLI_OK) { + legacy, sizeof(legacy) / sizeof(legacy[0])) != CLI_OK) { return false; } #endif - return cbm_write_owned_hook_script_with_legacy(script_path, script, legacy, 1U); + return cbm_write_owned_hook_script_with_legacy(script_path, script, legacy, + sizeof(legacy) / sizeof(legacy[0])); } static int cbm_upsert_session_hooks(const char *settings_path) { diff --git a/src/cli/config_toml_edit.c b/src/cli/config_toml_edit.c index 02ea991ec..528d97c78 100644 --- a/src/cli/config_toml_edit.c +++ b/src/cli/config_toml_edit.c @@ -143,7 +143,7 @@ typedef struct { static int toml_managed_block_conflicts(const char *existing, size_t existing_len, size_t exclude_start, size_t exclude_end, const char *block, - size_t block_len); + size_t block_len, int preserve_descendants); static void toml_buffer_dispose(toml_buffer_t *buffer) { if (!buffer) { @@ -984,8 +984,9 @@ int cbm_toml_escape_basic_string(const char *input, char *out, size_t out_size) return TOML_EDIT_OK; } -int cbm_toml_upsert_managed_block(const char *file_path, const char *begin_marker, - const char *end_marker, const char *block) { +static int toml_upsert_managed_block(const char *file_path, const char *begin_marker, + const char *end_marker, const char *block, + int preserve_descendants) { size_t block_len = 0U; if (!toml_valid_path(file_path) || !toml_valid_marker(begin_marker) || !toml_valid_marker(end_marker) || strcmp(begin_marker, end_marker) == 0 || !block || @@ -1016,7 +1017,7 @@ int cbm_toml_upsert_managed_block(const char *file_path, const char *begin_marke size_t exclude_start = has_pair ? begin_line.start : SIZE_MAX; size_t exclude_end = has_pair ? end_line.full_end : SIZE_MAX; if (toml_managed_block_conflicts(existing, existing_len, exclude_start, exclude_end, block, - block_len) != TOML_EDIT_OK) { + block_len, preserve_descendants) != TOML_EDIT_OK) { free(existing); return TOML_EDIT_ERR; } @@ -1051,6 +1052,17 @@ int cbm_toml_upsert_managed_block(const char *file_path, const char *begin_marke return result; } +int cbm_toml_upsert_managed_block(const char *file_path, const char *begin_marker, + const char *end_marker, const char *block) { + return toml_upsert_managed_block(file_path, begin_marker, end_marker, block, 0); +} + +int cbm_toml_upsert_managed_block_preserve_descendants(const char *file_path, + const char *begin_marker, + const char *end_marker, const char *block) { + return toml_upsert_managed_block(file_path, begin_marker, end_marker, block, 1); +} + int cbm_toml_remove_managed_block(const char *file_path, const char *begin_marker, const char *end_marker) { if (!toml_valid_path(file_path) || !toml_valid_marker(begin_marker) || @@ -1651,7 +1663,8 @@ static int toml_block_has_prior_table(const char *block, size_t block_len, size_ static int toml_existing_conflicts_with_table(const char *existing, size_t existing_len, size_t exclude_start, size_t exclude_end, - const toml_key_path_t *desired) { + const toml_key_path_t *desired, + int preserve_descendants) { size_t cursor = 0U; toml_line_t line; int multiline_state = TOML_STRING_NONE; @@ -1667,7 +1680,10 @@ static int toml_existing_conflicts_with_table(const char *existing, size_t exist return TOML_EDIT_ERR; } if (header.present) { - int conflict = !excluded && toml_key_path_has_prefix(&header.path, desired); + int descendant = header.path.count > desired->count && + toml_key_path_has_prefix(&header.path, desired); + int conflict = !excluded && toml_key_path_has_prefix(&header.path, desired) && + !(preserve_descendants && descendant); toml_key_path_dispose(&scope); scope = header.path; memset(&header.path, 0, sizeof(header.path)); @@ -1691,8 +1707,11 @@ static int toml_existing_conflicts_with_table(const char *existing, size_t exist toml_key_path_dispose(&scope); return TOML_EDIT_ERR; } - int conflict = toml_key_path_has_prefix(&full_key, desired) || - toml_key_path_has_prefix(desired, &full_key); + int descendant_scope = + scope.count > desired->count && toml_key_path_has_prefix(&scope, desired); + int conflict = !(preserve_descendants && descendant_scope) && + (toml_key_path_has_prefix(&full_key, desired) || + toml_key_path_has_prefix(desired, &full_key)); toml_key_path_dispose(&full_key); toml_assignment_dispose(&assignment); if (conflict) { @@ -1715,7 +1734,7 @@ static int toml_existing_conflicts_with_table(const char *existing, size_t exist static int toml_managed_block_conflicts(const char *existing, size_t existing_len, size_t exclude_start, size_t exclude_end, const char *block, - size_t block_len) { + size_t block_len, int preserve_descendants) { size_t cursor = 0U; toml_line_t line; int multiline_state = TOML_STRING_NONE; @@ -1730,7 +1749,8 @@ static int toml_managed_block_conflicts(const char *existing, size_t existing_le (toml_block_has_prior_table(block, block_len, line.start, &header.path) != TOML_EDIT_OK || toml_existing_conflicts_with_table(existing, existing_len, exclude_start, - exclude_end, &header.path) != TOML_EDIT_OK)) { + exclude_end, &header.path, + preserve_descendants) != TOML_EDIT_OK)) { toml_header_dispose(&header); return TOML_EDIT_ERR; } @@ -2483,8 +2503,6 @@ int cbm_toml_remove_legacy_table(const char *file_path, const char *table_name, if (header.present) { handled_header = 1; int exact = toml_key_path_equal(&header.path, &desired); - int descendant = header.path.count > desired.count && - toml_key_path_has_prefix(&header.path, &desired); if (exact) { if (header.array) { if (target_regular_seen) { @@ -2512,8 +2530,11 @@ int cbm_toml_remove_legacy_table(const char *file_path, const char *table_name, args_empty = 0; edit_start = header.edit_start; } else if (target_active) { - if (descendant || !toml_legacy_schema_is_owned(command_count, command_owned, - args_count, args_empty)) { + /* A descendant table (for example a Codex per-tool approval) + * is not part of the legacy root table's owned body. Preserve + * it while migrating the byte-exact owned command/args root. */ + if (!toml_legacy_schema_is_owned(command_count, command_owned, args_count, + args_empty)) { target_foreign = 1; } edit_end = header.edit_start; diff --git a/src/cli/config_toml_edit.h b/src/cli/config_toml_edit.h index fcfde1207..0d1baca10 100644 --- a/src/cli/config_toml_edit.h +++ b/src/cli/config_toml_edit.h @@ -27,6 +27,12 @@ int cbm_toml_escape_basic_string(const char *input, char *out, size_t out_size); * without changing the file. */ int cbm_toml_upsert_managed_block(const char *file_path, const char *begin_marker, const char *end_marker, const char *block); +/* Permit existing descendant tables of a table declared by block. TOML permits + * child-before-parent ordering; installers use this to own a parent table while + * preserving user-owned child policy. */ +int cbm_toml_upsert_managed_block_preserve_descendants(const char *file_path, + const char *begin_marker, + const char *end_marker, const char *block); int cbm_toml_remove_managed_block(const char *file_path, const char *begin_marker, const char *end_marker); diff --git a/tests/test_cli.c b/tests/test_cli.c index 2c83ce763..4e70d4bc6 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -4389,6 +4389,28 @@ static const char test_released_session_hook_script[] = "3. If a project is not indexed yet, run index_repository FIRST.\n" "REMINDER\n"; +/* Exact installer output from the streamlined auto-index guidance release. + * Upgrades must recognize their own prior bytes while preserving near-matches. */ +static const char test_intermediate_session_hook_script[] = + "#!/usr/bin/env bash\n" + "# SessionStart hook: remind agent to use codebase-memory-mcp tools.\n" + "# Installed by codebase-memory-mcp. Fires on startup/resume/clear/compact.\n" + "cat << 'REMINDER'\n" + "Code Discovery Protocol:\n" + "1. Prefer codebase-memory-mcp tools first for structural code exploration:\n" + " - search_graph(name_pattern/label/qn_pattern) to find functions/classes/routes\n" + " - trace_path(function_name, mode=calls|data_flow|cross_service) for call chains\n" + " - get_code(qualified_name) for exact symbol source in streamlined mode\n" + " - query_graph(query) for complex Cypher patterns\n" + " - search_code(pattern) for text/regex source search in an indexed project\n" + "2. Use Grep/Glob/Read freely for text, configs, non-code files, and\n" + " always Read a file before editing it.\n" + "3. Graph-backed tools auto-index the server CWD or explicit repo paths when\n" + " auto_index=true and under auto_index_limit. search_code needs an\n" + " indexed project. Use _hidden_tools\n" + " to reveal index_repository or get_architecture when explicit control is needed.\n" + "REMINDER\n"; + static const char test_released_subagent_hook_script[] = "#!/usr/bin/env bash\n" "# SubagentStart hook: tell subagents to use codebase-memory-mcp tools.\n" @@ -4466,6 +4488,17 @@ TEST(cli_upgrade_migrates_released_claude_hook_scripts) { strstr(settings, "cbm-code-discovery-gate") && strstr(settings, "cbm-session-reminder") && strstr(settings, "cbm-subagent-reminder"); + + /* A later installer emitted a second byte-exact session reminder before + * hooks delegated to the binary. It must migrate on an idempotent reinstall. */ + ASSERT_EQ(write_test_file(session_path, test_intermediate_session_hook_script), 0); + int intermediate_rc = + cbm_install_agent_configs(tmpdir, "/opt/codebase-memory-mcp", false, false); + char *intermediate_session = read_test_file_alloc(session_path); + bool intermediate_migrated = + intermediate_rc == 0 && intermediate_session && + strcmp(intermediate_session, test_intermediate_session_hook_script) != 0; + free(intermediate_session); free(gate); free(session); free(subagent); @@ -4475,8 +4508,8 @@ TEST(cli_upgrade_migrates_released_claude_hook_scripts) { restore_test_env("CLAUDE_CONFIG_DIR", saved_claude); restore_test_env("CODEX_HOME", saved_codex); test_rmdir_r(tmpdir); - if (!migrated) - FAIL("released Claude hook scripts must migrate byte-exactly and stay registered"); + if (!migrated || !intermediate_migrated) + FAIL("every released Claude hook script must migrate byte-exactly and stay registered"); PASS(); } @@ -5095,6 +5128,37 @@ TEST(cli_upsert_codex_mcp_replace) { PASS(); } +TEST(cli_upsert_codex_mcp_preserves_owned_descendant_tool_policy) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-codex-descendant-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + char configpath[512]; + snprintf(configpath, sizeof(configpath), "%s/config.toml", tmpdir); + const char *initial = "[mcp_servers.codebase-memory-mcp]\n" + "command = \"/old/path/codebase-memory-mcp\"\n" + "args = []\n\n" + "[mcp_servers.codebase-memory-mcp.tools.query_graph]\n" + "approval_mode = \"approve\"\n\n" + "[other]\nkeep = true\n"; + ASSERT_EQ(write_test_file(configpath, initial), 0); + + ASSERT_EQ(cbm_upsert_codex_mcp("/new/path/codebase-memory-mcp", configpath), 0); + ASSERT_EQ(cbm_upsert_codex_mcp("/new/path/codebase-memory-mcp", configpath), 0); + const char *data = read_test_file(configpath); + ASSERT_NOT_NULL(data); + ASSERT_NOT_NULL(strstr(data, "/new/path/codebase-memory-mcp")); + ASSERT_NULL(strstr(data, "/old/path/codebase-memory-mcp")); + ASSERT_NOT_NULL(strstr(data, "[mcp_servers.codebase-memory-mcp.tools.query_graph]\n" + "approval_mode = \"approve\"")); + ASSERT_NOT_NULL(strstr(data, "[other]\nkeep = true")); + ASSERT_EQ(count_substr(data, "# >>> codebase-memory-mcp MCP >>>"), 1); + + test_rmdir_r(tmpdir); + PASS(); +} + TEST(cli_codex_legacy_migration_ignores_header_text_in_multiline_string) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-codex-multiline-XXXXXX"); @@ -7198,6 +7262,7 @@ SUITE(cli) { RUN_TEST(cli_upsert_codex_mcp_escapes_windows_path); RUN_TEST(cli_upsert_codex_mcp_existing); RUN_TEST(cli_upsert_codex_mcp_replace); + RUN_TEST(cli_upsert_codex_mcp_preserves_owned_descendant_tool_policy); RUN_TEST(cli_codex_legacy_migration_ignores_header_text_in_multiline_string); /* Zed MCP format fix (1 test — group B) */