From 7d4dc779d02d85f8d72796c509982b6d738f5394 Mon Sep 17 00:00:00 2001 From: astandrik Date: Fri, 14 Aug 2026 18:44:04 +0300 Subject: [PATCH 1/3] fix(cli): explain Codex hook preflight refusals Signed-off-by: astandrik --- src/cli/cli.c | 78 ++++++++++++++++------ src/cli/config_toml_edit.c | 118 ++++++++++++++++++++++++++++------ src/cli/config_toml_edit.h | 18 ++++++ tests/test_cli.c | 80 +++++++++++++++++++++++ tests/test_config_toml_edit.c | 62 ++++++++++++++++++ 5 files changed, 318 insertions(+), 38 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index 7c0558c91..0a2af3fdc 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -3487,17 +3487,26 @@ bool cbm_optional_hook_supported_for_testing(const char *agent_name, bool window } #endif -static int cbm_reconcile_codex_hooks_command(const char *config_path, const char *command, - const char *command_windows, - cbm_toml_codex_hook_action_t action, bool check_only) { +static int cbm_reconcile_codex_hooks_command_detailed(const char *config_path, const char *command, + const char *command_windows, + cbm_toml_codex_hook_action_t action, + bool check_only, + cbm_toml_codex_hook_failure_t *failure) { if (!config_path || !command || !command_windows) { return CLI_ERR; } - return cbm_toml_reconcile_codex_hooks(config_path, CODEX_HOOK_BEGIN, CODEX_HOOK_END, command, - command_windows, action, check_only ? 1 : 0) == 0 + return cbm_toml_reconcile_codex_hooks_detailed(config_path, CODEX_HOOK_BEGIN, CODEX_HOOK_END, + command, command_windows, action, + check_only ? 1 : 0, failure) == 0 ? CLI_OK : CLI_ERR; } +static int cbm_reconcile_codex_hooks_command(const char *config_path, const char *command, + const char *command_windows, + cbm_toml_codex_hook_action_t action, bool check_only) { + return cbm_reconcile_codex_hooks_command_detailed(config_path, command, command_windows, action, + check_only, NULL); +} static int cbm_upsert_codex_hooks_command(const char *config_path, const char *command, const char *command_windows) { return cbm_reconcile_codex_hooks_command(config_path, command, command_windows, @@ -7362,15 +7371,25 @@ static void describe_agent_config_target(const char *path, char *out, size_t out (void)snprintf(out, out_size, " (target: %s, %lld bytes)", kind, (long long)info.size); } -static void record_agent_config_error(bool uninstalling, const char *agent, const char *operation, - const char *path) { +static void record_agent_config_error_with_reason(bool uninstalling, const char *agent, + const char *operation, const char *path, + const char *reason) { int *counter = uninstalling ? &g_agent_uninstall_errors : &g_agent_install_errors; (*counter)++; char detail[160]; describe_agent_config_target(path, detail, sizeof(detail)); - (void)fprintf(stderr, "error: agent_config agent=%s op=%s path=%s%s\n", - agent ? agent : "unknown", operation ? operation : "unknown", - path ? path : "unknown", detail); + (void)fprintf(stderr, "error: agent_config agent=%s op=%s path=%s", agent ? agent : "unknown", + operation ? operation : "unknown", path ? path : "unknown"); + if (reason && reason[0]) { + (void)fprintf(stderr, " reason=%s", reason); + } + (void)fputs(detail, stderr); + (void)fputc('\n', stderr); +} + +static void record_agent_config_error(bool uninstalling, const char *agent, const char *operation, + const char *path) { + record_agent_config_error_with_reason(uninstalling, agent, operation, path, NULL); } static bool prepare_config_parent(const char *path) { @@ -8507,10 +8526,22 @@ static void install_cli_agent_configs(const cbm_detected_agents_t *agents, const sizeof(command_windows)) == CLI_OK; cbm_toml_codex_hook_action_t preflight_action = use_hooks_json ? CBM_TOML_CODEX_HOOK_REMOVE : CBM_TOML_CODEX_HOOK_UPSERT; - if (!commands_ok || cbm_reconcile_codex_hooks_command(cp, command, command_windows, - preflight_action, true) != CLI_OK) { - record_agent_config_error(false, "Codex CLI", - commands_ok ? "hook_preflight" : "hook_command_build", cp); + cbm_toml_codex_hook_failure_t preflight_failure = CBM_TOML_CODEX_HOOK_FAILURE_NONE; + int preflight_result = commands_ok ? cbm_reconcile_codex_hooks_command_detailed( + cp, command, command_windows, preflight_action, + true, &preflight_failure) + : CLI_ERR; + if (preflight_result != CLI_OK) { + if (!g_install_plan) { + printf("Codex CLI:\n"); + fflush(stdout); + } + const char *reason = preflight_failure == CBM_TOML_CODEX_HOOK_FAILURE_NONE + ? NULL + : cbm_toml_codex_hook_failure_name(preflight_failure); + record_agent_config_error_with_reason( + false, "Codex CLI", commands_ok ? "hook_preflight" : "hook_command_build", cp, + reason); goto codex_install_done; } install_generic_agent_config("Codex CLI", binary_path, cp, ip, dry_run, @@ -10760,14 +10791,21 @@ static void uninstall_cli_agents(const cbm_detected_agents_t *agents, const char char hook_command_windows[CLI_BUF_8K]; bool hook_command_ok = cbm_build_augment_command(installed_binary, hook_command, sizeof(hook_command)) == CLI_OK; + bool hook_commands_ok = hook_command_ok && cbm_build_augment_command_windows( + installed_binary, hook_command_windows, + sizeof(hook_command_windows)) == CLI_OK; + cbm_toml_codex_hook_failure_t preflight_failure = CBM_TOML_CODEX_HOOK_FAILURE_NONE; bool hook_preflight_ok = - hook_command_ok && - cbm_build_augment_command_windows(installed_binary, hook_command_windows, - sizeof(hook_command_windows)) == CLI_OK && - cbm_reconcile_codex_hooks_command(cp, hook_command, hook_command_windows, - CBM_TOML_CODEX_HOOK_REMOVE, true) == CLI_OK; + hook_commands_ok && cbm_reconcile_codex_hooks_command_detailed( + cp, hook_command, hook_command_windows, + CBM_TOML_CODEX_HOOK_REMOVE, true, &preflight_failure) == CLI_OK; if (!hook_preflight_ok) { - record_agent_config_error(true, "Codex CLI", "hook_preflight", cp); + printf("Codex CLI:\n"); + fflush(stdout); + const char *reason = preflight_failure == CBM_TOML_CODEX_HOOK_FAILURE_NONE + ? NULL + : cbm_toml_codex_hook_failure_name(preflight_failure); + record_agent_config_error_with_reason(true, "Codex CLI", "hook_preflight", cp, reason); goto codex_toml_done; } uninstall_agent_mcp_instr((mcp_uninstall_args_t){"Codex CLI", cp, ip}, dry_run, diff --git a/src/cli/config_toml_edit.c b/src/cli/config_toml_edit.c index 6316e1371..ef73c66fc 100644 --- a/src/cli/config_toml_edit.c +++ b/src/cli/config_toml_edit.c @@ -2641,6 +2641,37 @@ int cbm_toml_remove_legacy_table(const char *file_path, const char *table_name, typedef struct { size_t start, end; } toml_codex_edit_t; +static void toml_codex_set_failure(cbm_toml_codex_hook_failure_t *failure, + cbm_toml_codex_hook_failure_t value) { + if (failure) { + *failure = value; + } +} +const char *cbm_toml_codex_hook_failure_name(cbm_toml_codex_hook_failure_t failure) { + switch (failure) { + case CBM_TOML_CODEX_HOOK_FAILURE_NONE: + return "none"; + case CBM_TOML_CODEX_HOOK_FAILURE_INVALID_ARGUMENT: + return "invalid_argument"; + case CBM_TOML_CODEX_HOOK_FAILURE_COMMAND_RENDER: + return "command_render"; + case CBM_TOML_CODEX_HOOK_FAILURE_CONFIG_READ: + return "config_read"; + case CBM_TOML_CODEX_HOOK_FAILURE_UNSAFE_CONTENT: + return "unsafe_content"; + case CBM_TOML_CODEX_HOOK_FAILURE_MALFORMED_CONFIG: + return "malformed_config"; + case CBM_TOML_CODEX_HOOK_FAILURE_AMBIGUOUS_OWNERSHIP: + return "ambiguous_hook_ownership"; + case CBM_TOML_CODEX_HOOK_FAILURE_CONFLICTING_HOOKS: + return "conflicting_hook_representations"; + case CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD: + return "edit_build"; + case CBM_TOML_CODEX_HOOK_FAILURE_CONFIG_WRITE: + return "config_write"; + } + return "unknown"; +} static size_t toml_codex_payload_start(const char *data, size_t len) { return len >= 3U && memcmp(data, "\xef\xbb\xbf", 3U) == 0 ? 3U : 0U; } @@ -2874,13 +2905,15 @@ static int toml_codex_add_edit(toml_codex_edit_t *edits, size_t *count, size_t s return TOML_EDIT_OK; } static int toml_codex_scan_inline(const char *data, size_t len, toml_codex_edit_t *edits, - size_t *edit_count, int found[2]) { + size_t *edit_count, int found[2], + cbm_toml_codex_hook_failure_t *failure) { toml_key_path_t wanted[2] = {{0}}; toml_key_path_t scope = {0}; if (toml_parse_key_path("hooks.SessionStart", 0U, strlen("hooks.SessionStart"), &wanted[0]) != TOML_EDIT_OK || toml_parse_key_path("hooks.SubagentStart", 0U, strlen("hooks.SubagentStart"), &wanted[1]) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD); goto error; } size_t cursor = 0U; @@ -2892,6 +2925,7 @@ static int toml_codex_scan_inline(const char *data, size_t len, toml_codex_edit_ if (!in_multiline) { toml_header_t header; if (toml_parse_header(data, &line, "", &header) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_MALFORMED_CONFIG); goto error; } header_present = header.present; @@ -2905,12 +2939,14 @@ static int toml_codex_scan_inline(const char *data, size_t len, toml_codex_edit_ if (!in_multiline && !header_present) { toml_assignment_t assignment; if (toml_parse_assignment(data, &line, &assignment) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_MALFORMED_CONFIG); goto error; } if (assignment.present) { toml_key_path_t full_key; if (toml_key_path_join(&scope, &assignment.key, &full_key) != TOML_EDIT_OK) { toml_assignment_dispose(&assignment); + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD); goto error; } int event = toml_key_path_equal(&full_key, &wanted[0]) @@ -2922,12 +2958,20 @@ static int toml_codex_scan_inline(const char *data, size_t len, toml_codex_edit_ toml_codex_skip_space(data, line.content_end, &tail); size_t payload_start = toml_codex_payload_start(data, len); size_t edit_start = line.start == 0U ? payload_start : line.start; - if (++found[event] > 1 || assignment.multiline_value || - tail != line.content_end || - !toml_codex_inline_is_owned(data, assignment.value_start, - assignment.value_end, event) || - toml_codex_add_edit(edits, edit_count, edit_start, line.full_end) != - TOML_EDIT_OK) { + cbm_toml_codex_hook_failure_t event_failure = CBM_TOML_CODEX_HOOK_FAILURE_NONE; + ++found[event]; + if (found[event] > 1) { + event_failure = CBM_TOML_CODEX_HOOK_FAILURE_CONFLICTING_HOOKS; + } else if (assignment.multiline_value || tail != line.content_end || + !toml_codex_inline_is_owned(data, assignment.value_start, + assignment.value_end, event)) { + event_failure = CBM_TOML_CODEX_HOOK_FAILURE_AMBIGUOUS_OWNERSHIP; + } else if (toml_codex_add_edit(edits, edit_count, edit_start, line.full_end) != + TOML_EDIT_OK) { + event_failure = CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD; + } + if (event_failure != CBM_TOML_CODEX_HOOK_FAILURE_NONE) { + toml_codex_set_failure(failure, event_failure); toml_assignment_dispose(&assignment); goto error; } @@ -2936,10 +2980,14 @@ static int toml_codex_scan_inline(const char *data, size_t len, toml_codex_edit_ } } if (toml_scan_line_strings(data, &line, &multiline_state) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_MALFORMED_CONFIG); goto error; } } int result = multiline_state == TOML_STRING_NONE ? TOML_EDIT_OK : TOML_EDIT_ERR; + if (result != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_MALFORMED_CONFIG); + } goto done; error: result = TOML_EDIT_ERR; @@ -2949,28 +2997,38 @@ static int toml_codex_scan_inline(const char *data, size_t len, toml_codex_edit_ toml_key_path_dispose(&wanted[1]); return result; } -int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_marker, - const char *end_marker, const char *command, - const char *command_windows, cbm_toml_codex_hook_action_t action, - int check_only) { +int cbm_toml_reconcile_codex_hooks_detailed(const char *file_path, const char *begin_marker, + const char *end_marker, const char *command, + const char *command_windows, + cbm_toml_codex_hook_action_t action, int check_only, + cbm_toml_codex_hook_failure_t *failure) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_NONE); if (!toml_valid_path(file_path) || !toml_valid_marker(begin_marker) || !toml_valid_marker(end_marker) || strcmp(begin_marker, end_marker) == 0 || !command || !command_windows || (action != CBM_TOML_CODEX_HOOK_UPSERT && action != CBM_TOML_CODEX_HOOK_REMOVE)) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_INVALID_ARGUMENT); return TOML_EDIT_ERR; } toml_buffer_t block = {0}; if (toml_codex_build_block(command, command_windows, &block) != TOML_EDIT_OK) { toml_buffer_dispose(&block); + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_COMMAND_RENDER); return TOML_EDIT_ERR; } char *existing = NULL; size_t existing_len = 0U; toml_file_snapshot_t snapshot; - if (toml_read_file(file_path, &existing, &existing_len, &snapshot) != TOML_EDIT_OK || - !toml_text_is_safe(existing, existing_len, 1)) { + if (toml_read_file(file_path, &existing, &existing_len, &snapshot) != TOML_EDIT_OK) { + toml_buffer_dispose(&block); + free(existing); + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_CONFIG_READ); + return TOML_EDIT_ERR; + } + if (!toml_text_is_safe(existing, existing_len, 1)) { toml_buffer_dispose(&block); free(existing); + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_UNSAFE_CONTENT); return TOML_EDIT_ERR; } const char *newline = toml_newline_style(existing, existing_len); @@ -2981,6 +3039,7 @@ int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_mark int has_pair = 0; if (toml_find_markers(existing, existing_len, begin_marker, end_marker, &begin_line, &end_line, &has_pair) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_CONFLICTING_HOOKS); goto error; } size_t pair_start = begin_line.start; @@ -2990,12 +3049,14 @@ int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_mark if (has_pair && toml_codex_owned_span(existing + pair_start, end_line.full_end - pair_start, begin_marker, end_marker, newline, 1) != end_line.full_end - pair_start) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_AMBIGUOUS_OWNERSHIP); goto error; } toml_codex_edit_t edits[4]; size_t edit_count = 0U; if (has_pair && toml_codex_add_edit(edits, &edit_count, pair_start, end_line.full_end) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD); goto error; } size_t payload_start = toml_codex_payload_start(existing, existing_len); @@ -3010,17 +3071,24 @@ int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_mark begin_marker, end_marker, newline, 0) : 0U; if (owned_len) { - if (++markerless_count > 1 || - toml_codex_add_edit(edits, &edit_count, pos, pos + owned_len) != TOML_EDIT_OK) { + if (++markerless_count > 1) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_CONFLICTING_HOOKS); + goto error; + } + if (toml_codex_add_edit(edits, &edit_count, pos, pos + owned_len) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD); goto error; } pos += owned_len - 1U; } } int inline_found[2] = {0}; - if (toml_codex_scan_inline(existing, existing_len, edits, &edit_count, inline_found) != - TOML_EDIT_OK || - (markerless_count && (has_pair || inline_found[0] || inline_found[1]))) { + if (toml_codex_scan_inline(existing, existing_len, edits, &edit_count, inline_found, failure) != + TOML_EDIT_OK) { + goto error; + } + if (markerless_count && (has_pair || inline_found[0] || inline_found[1])) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_CONFLICTING_HOOKS); goto error; } for (size_t i = 0U; i < edit_count; ++i) { @@ -3032,6 +3100,7 @@ int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_mark } } if (i > 0U && edits[i - 1U].end > edits[i].start) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_CONFLICTING_HOOKS); goto error; } } @@ -3039,11 +3108,13 @@ int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_mark for (size_t i = 0U; i < edit_count; ++i) { if (toml_buffer_append(&output, existing + cursor, edits[i].start - cursor) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD); goto error; } cursor = edits[i].end; } if (toml_buffer_append(&output, existing + cursor, existing_len - cursor) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD); goto error; } if (action == CBM_TOML_CODEX_HOOK_UPSERT) { @@ -3052,15 +3123,26 @@ int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_mark toml_buffer_append_cstr(&output, newline) != TOML_EDIT_OK) || toml_append_managed(&output, begin_marker, end_marker, block.data, newline) != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD); goto error; } } result = check_only ? TOML_EDIT_OK : toml_write_atomic(file_path, existing, existing_len, output.data, output.len, &snapshot); + if (result != TOML_EDIT_OK) { + toml_codex_set_failure(failure, CBM_TOML_CODEX_HOOK_FAILURE_CONFIG_WRITE); + } error: toml_buffer_dispose(&output); toml_buffer_dispose(&block); free(existing); return result; } +int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_marker, + const char *end_marker, const char *command, + const char *command_windows, cbm_toml_codex_hook_action_t action, + int check_only) { + return cbm_toml_reconcile_codex_hooks_detailed(file_path, begin_marker, end_marker, command, + command_windows, action, check_only, NULL); +} diff --git a/src/cli/config_toml_edit.h b/src/cli/config_toml_edit.h index ca73d4216..4fa989cf7 100644 --- a/src/cli/config_toml_edit.h +++ b/src/cli/config_toml_edit.h @@ -70,6 +70,24 @@ typedef enum { CBM_TOML_CODEX_HOOK_UPSERT = 0, CBM_TOML_CODEX_HOOK_REMOVE = 1 } cbm_toml_codex_hook_action_t; +typedef enum { + CBM_TOML_CODEX_HOOK_FAILURE_NONE = 0, + CBM_TOML_CODEX_HOOK_FAILURE_INVALID_ARGUMENT, + CBM_TOML_CODEX_HOOK_FAILURE_COMMAND_RENDER, + CBM_TOML_CODEX_HOOK_FAILURE_CONFIG_READ, + CBM_TOML_CODEX_HOOK_FAILURE_UNSAFE_CONTENT, + CBM_TOML_CODEX_HOOK_FAILURE_MALFORMED_CONFIG, + CBM_TOML_CODEX_HOOK_FAILURE_AMBIGUOUS_OWNERSHIP, + CBM_TOML_CODEX_HOOK_FAILURE_CONFLICTING_HOOKS, + CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD, + CBM_TOML_CODEX_HOOK_FAILURE_CONFIG_WRITE +} cbm_toml_codex_hook_failure_t; +const char *cbm_toml_codex_hook_failure_name(cbm_toml_codex_hook_failure_t failure); +int cbm_toml_reconcile_codex_hooks_detailed(const char *file_path, const char *begin_marker, + const char *end_marker, const char *command, + const char *command_windows, + cbm_toml_codex_hook_action_t action, int check_only, + cbm_toml_codex_hook_failure_t *failure); int cbm_toml_reconcile_codex_hooks(const char *file_path, const char *begin_marker, const char *end_marker, const char *command, const char *command_windows, cbm_toml_codex_hook_action_t action, diff --git a/tests/test_cli.c b/tests/test_cli.c index 2118b794f..785091263 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -9313,6 +9313,83 @@ TEST(cli_codex_migrates_to_single_hook_representation) { PASS(); } +#ifndef _WIN32 +TEST(cli_codex_preflight_reports_heading_and_reason) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-codex-preflight-reason-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + char codex_dir[512]; + char config_path[640]; + char agents_path[640]; + snprintf(codex_dir, sizeof(codex_dir), "%s/.codex", tmpdir); + snprintf(config_path, sizeof(config_path), "%s/config.toml", codex_dir); + snprintf(agents_path, sizeof(agents_path), "%s/AGENTS.md", codex_dir); + test_mkdirp(codex_dir); + const char *ambiguous = + "[hooks]\nSessionStart = [{ matcher = 'startup|resume|clear|compact', hooks = [" + "{ type = 'command', command = 'codebase-memory-mcp hook-augment' }, " + "{ type = 'command', command = 'foreign' }] }]\n"; + write_test_file(config_path, ambiguous); + + char *saved_home = save_test_env("HOME"); + char *saved_path = save_test_env("PATH"); + char *saved_codex = save_test_env("CODEX_HOME"); + cbm_setenv("HOME", tmpdir, 1); + cbm_setenv("PATH", tmpdir, 1); + cbm_unsetenv("CODEX_HOME"); + + FILE *capture = tmpfile(); + int saved_stdout = capture ? dup(STDOUT_FILENO) : -1; + int saved_stderr = capture ? dup(STDERR_FILENO) : -1; + bool redirected = false; + int install_rc = -1; + if (capture && saved_stdout >= 0 && saved_stderr >= 0) { + fflush(NULL); + redirected = + dup2(fileno(capture), STDOUT_FILENO) >= 0 && dup2(fileno(capture), STDERR_FILENO) >= 0; + if (redirected) { + install_rc = + cbm_install_agent_configs(tmpdir, "/opt/codebase-memory-mcp", false, false); + } + fflush(NULL); + (void)dup2(saved_stdout, STDOUT_FILENO); + (void)dup2(saved_stderr, STDERR_FILENO); + } + if (saved_stdout >= 0) { + close(saved_stdout); + } + if (saved_stderr >= 0) { + close(saved_stderr); + } + + char output[8192] = {0}; + if (capture) { + rewind(capture); + size_t count = fread(output, 1, sizeof(output) - 1U, capture); + output[count] = '\0'; + fclose(capture); + } + char *after = read_test_file_alloc(config_path); + struct stat state; + bool unchanged = after && strcmp(after, ambiguous) == 0 && stat(agents_path, &state) != 0; + bool diagnostic = + strstr(output, "Codex CLI:\nerror: agent_config agent=Codex CLI op=hook_preflight path=") != + NULL && + strstr(output, "reason=ambiguous_hook_ownership") != NULL; + free(after); + + restore_test_env("HOME", saved_home); + restore_test_env("PATH", saved_path); + restore_test_env("CODEX_HOME", saved_codex); + test_rmdir_r(tmpdir); + if (!redirected || install_rc == 0 || !unchanged || !diagnostic) + FAIL("Codex preflight refusal must retain its heading, reason, and fail-closed state"); + PASS(); +} +#endif + /* The PreToolUse augmenter parses search_graph's format:"json" payload to * build additionalContext. This test feeds it the REAL envelope from a live * in-memory server, so any drift between the response shape and the parser @@ -12879,6 +12956,9 @@ SUITE(cli) { RUN_TEST(cli_claude_hook_scripts_shell_quote_binary_path); RUN_TEST(cli_claude_hook_commands_shell_quote_custom_config_dir); RUN_TEST(cli_codex_migrates_to_single_hook_representation); +#ifndef _WIN32 + RUN_TEST(cli_codex_preflight_reports_heading_and_reason); +#endif RUN_TEST(cli_hook_augment_context_tracks_search_json_shape); RUN_TEST(cli_hook_augment_lifecycle_output_contract); RUN_TEST(cli_hook_augment_subagent_tier_router_contract); diff --git a/tests/test_config_toml_edit.c b/tests/test_config_toml_edit.c index ae133ff2c..b2ad00c92 100644 --- a/tests/test_config_toml_edit.c +++ b/tests/test_config_toml_edit.c @@ -50,6 +50,13 @@ static int cte_codex_edit(const char *path, cbm_toml_codex_hook_action_t action, CTE_CODEX_COMMAND, action, check_only); } +static int cte_codex_edit_detailed(const char *path, cbm_toml_codex_hook_action_t action, + int check_only, cbm_toml_codex_hook_failure_t *failure) { + return cbm_toml_reconcile_codex_hooks_detailed(path, CTE_CODEX_BEGIN, CTE_CODEX_END, + CTE_CODEX_COMMAND, CTE_CODEX_COMMAND, action, + check_only, failure); +} + static int cte_fixture(char *dir, size_t dir_size, char *path, size_t path_size) { char *created = th_mktempdir("cbm_toml_edit"); if (!created) { @@ -1129,6 +1136,60 @@ TEST(config_toml_codex_rejects_ambiguous_inline_byte_identically) { PASS(); } +TEST(config_toml_codex_reports_stable_failure_reasons) { + char dir[CTE_PATH_CAP]; + char path[CTE_PATH_CAP]; + char actual[CTE_FILE_CAP]; + static const struct { + const char *content; + cbm_toml_codex_hook_failure_t expected; + const char *name; + } cases[] = { + { + .content = + "[hooks]\nSessionStart = [{ matcher = 'startup|resume|clear|compact', hooks = [" + "{ type = 'command', command = 'codebase-memory-mcp hook-augment' }, " + "{ type = 'command', command = 'foreign' }] }]\n", + .expected = CBM_TOML_CODEX_HOOK_FAILURE_AMBIGUOUS_OWNERSHIP, + .name = "ambiguous_hook_ownership", + }, + { + .content = + "[hooks]\n" + "SessionStart = [{ matcher = 'startup|resume|clear|compact', hooks = [{ type = " + "'command', command = 'echo \"Code discovery: prefer codebase-memory-mcp\"' }] " + "}]\n" + "SessionStart = [{ matcher = 'startup|resume|clear|compact', hooks = [{ type = " + "'command', command = 'echo \"Code discovery: prefer codebase-memory-mcp\"' }] " + "}]\n", + .expected = CBM_TOML_CODEX_HOOK_FAILURE_CONFLICTING_HOOKS, + .name = "conflicting_hook_representations", + }, + { + .content = "[hooks\n", + .expected = CBM_TOML_CODEX_HOOK_FAILURE_MALFORMED_CONFIG, + .name = "malformed_config", + }, + }; + ASSERT_EQ(cte_fixture(dir, sizeof(dir), path, sizeof(path)), 0); + for (size_t i = 0U; i < sizeof(cases) / sizeof(cases[0]); ++i) { + cbm_toml_codex_hook_failure_t failure = CBM_TOML_CODEX_HOOK_FAILURE_NONE; + ASSERT_EQ(th_write_file(path, cases[i].content), 0); + ASSERT_EQ(cte_codex_edit_detailed(path, CBM_TOML_CODEX_HOOK_UPSERT, 1, &failure), -1); + ASSERT_EQ(failure, cases[i].expected); + ASSERT_STR_EQ(cbm_toml_codex_hook_failure_name(failure), cases[i].name); + ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0); + ASSERT_STR_EQ(actual, cases[i].content); + } + + cbm_toml_codex_hook_failure_t failure = CBM_TOML_CODEX_HOOK_FAILURE_INVALID_ARGUMENT; + ASSERT_EQ(th_write_file(path, "keep = true\n"), 0); + ASSERT_EQ(cte_codex_edit_detailed(path, CBM_TOML_CODEX_HOOK_UPSERT, 1, &failure), 0); + ASSERT_EQ(failure, CBM_TOML_CODEX_HOOK_FAILURE_NONE); + th_cleanup(dir); + PASS(); +} + TEST(config_toml_codex_preserves_bom_crlf_and_foreign_aot) { char dir[CTE_PATH_CAP]; char path[CTE_PATH_CAP]; @@ -1241,6 +1302,7 @@ SUITE(config_toml_edit) { RUN_TEST(config_toml_target_table_rejects_significant_nonassignments_byte_identically); RUN_TEST(config_toml_codex_reconciles_minimal_owned_forms); RUN_TEST(config_toml_codex_rejects_ambiguous_inline_byte_identically); + RUN_TEST(config_toml_codex_reports_stable_failure_reasons); RUN_TEST(config_toml_codex_preserves_bom_crlf_and_foreign_aot); RUN_TEST(config_toml_legacy_remove_reports_foreign_table_without_mutation); } From 00e0cf38d60fd8163a111d168d1118d04eaa529d Mon Sep 17 00:00:00 2001 From: astandrik Date: Fri, 14 Aug 2026 19:45:25 +0300 Subject: [PATCH 2/3] fix(cli): harden hook diagnostic contract Signed-off-by: astandrik --- src/cli/config_toml_edit.h | 3 +++ tests/test_cli.c | 10 ++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/cli/config_toml_edit.h b/src/cli/config_toml_edit.h index 4fa989cf7..4e41a4d9c 100644 --- a/src/cli/config_toml_edit.h +++ b/src/cli/config_toml_edit.h @@ -82,7 +82,10 @@ typedef enum { CBM_TOML_CODEX_HOOK_FAILURE_EDIT_BUILD, CBM_TOML_CODEX_HOOK_FAILURE_CONFIG_WRITE } cbm_toml_codex_hook_failure_t; +/* Returns a stable, content-free diagnostic name for a failure category. */ const char *cbm_toml_codex_hook_failure_name(cbm_toml_codex_hook_failure_t failure); +/* `failure` may be NULL. When non-NULL, it is reset to NONE on entry and remains + * NONE on success; failure categories never contain configuration content. */ int cbm_toml_reconcile_codex_hooks_detailed(const char *file_path, const char *begin_marker, const char *end_marker, const char *command, const char *command_windows, diff --git a/tests/test_cli.c b/tests/test_cli.c index 785091263..221eb749c 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -9326,12 +9326,18 @@ TEST(cli_codex_preflight_reports_heading_and_reason) { snprintf(codex_dir, sizeof(codex_dir), "%s/.codex", tmpdir); snprintf(config_path, sizeof(config_path), "%s/config.toml", codex_dir); snprintf(agents_path, sizeof(agents_path), "%s/AGENTS.md", codex_dir); - test_mkdirp(codex_dir); + if (test_mkdirp(codex_dir) != 0) { + test_rmdir_r(tmpdir); + FAIL("failed to create Codex preflight fixture directory"); + } const char *ambiguous = "[hooks]\nSessionStart = [{ matcher = 'startup|resume|clear|compact', hooks = [" "{ type = 'command', command = 'codebase-memory-mcp hook-augment' }, " "{ type = 'command', command = 'foreign' }] }]\n"; - write_test_file(config_path, ambiguous); + if (write_test_file(config_path, ambiguous) != 0) { + test_rmdir_r(tmpdir); + FAIL("failed to write Codex preflight fixture config"); + } char *saved_home = save_test_env("HOME"); char *saved_path = save_test_env("PATH"); From d0351fd99a8394d695213684ca1e2ea5093ce831 Mon Sep 17 00:00:00 2001 From: astandrik Date: Sat, 15 Aug 2026 09:06:39 +0300 Subject: [PATCH 3/3] test(cli): cover v0.10.2 Codex hook upgrades Signed-off-by: astandrik --- tests/test_config_toml_edit.c | 147 +++++++++++++++++++++++++++++++++- 1 file changed, 144 insertions(+), 3 deletions(-) diff --git a/tests/test_config_toml_edit.c b/tests/test_config_toml_edit.c index b2ad00c92..1266556e0 100644 --- a/tests/test_config_toml_edit.c +++ b/tests/test_config_toml_edit.c @@ -50,11 +50,18 @@ static int cte_codex_edit(const char *path, cbm_toml_codex_hook_action_t action, CTE_CODEX_COMMAND, action, check_only); } +static int cte_codex_edit_commands_detailed(const char *path, const char *command, + const char *command_windows, + cbm_toml_codex_hook_action_t action, int check_only, + cbm_toml_codex_hook_failure_t *failure) { + return cbm_toml_reconcile_codex_hooks_detailed(path, CTE_CODEX_BEGIN, CTE_CODEX_END, command, + command_windows, action, check_only, failure); +} + static int cte_codex_edit_detailed(const char *path, cbm_toml_codex_hook_action_t action, int check_only, cbm_toml_codex_hook_failure_t *failure) { - return cbm_toml_reconcile_codex_hooks_detailed(path, CTE_CODEX_BEGIN, CTE_CODEX_END, - CTE_CODEX_COMMAND, CTE_CODEX_COMMAND, action, - check_only, failure); + return cte_codex_edit_commands_detailed(path, CTE_CODEX_COMMAND, CTE_CODEX_COMMAND, action, + check_only, failure); } static int cte_fixture(char *dir, size_t dir_size, char *path, size_t path_size) { @@ -110,6 +117,25 @@ static int cte_occurrences(const char *text, const char *needle) { return count; } +static int cte_replace_once(const char *source, const char *needle, const char *replacement, + char *output, size_t output_size) { + const char *match = strstr(source, needle); + if (!match) { + return -1; + } + size_t prefix_len = (size_t)(match - source); + size_t needle_len = strlen(needle); + size_t replacement_len = strlen(replacement); + size_t suffix_len = strlen(match + needle_len); + if (prefix_len + replacement_len + suffix_len >= output_size) { + return -1; + } + memcpy(output, source, prefix_len); + memcpy(output + prefix_len, replacement, replacement_len); + memcpy(output + prefix_len + replacement_len, match + needle_len, suffix_len + 1U); + return 0; +} + static size_t cte_temp_count(const char *dir) { cbm_dir_t *directory = cbm_opendir(dir); if (!directory) { @@ -1106,6 +1132,120 @@ TEST(config_toml_codex_reconciles_minimal_owned_forms) { PASS(); } +/* #1633: keep this fixture literal. It is the managed AOT block written by the + * v0.10.2 release, with the release's POSIX and PowerShell command builders. */ +TEST(config_toml_codex_accepts_v0102_managed_windows_crlf) { + static const char *command = + "'C:\\Users\\Example\\AppData\\Local\\Programs\\codebase-memory-mcp\\" + "codebase-memory-mcp.exe' hook-augment"; + static const char *command_windows = + "& 'C:\\Users\\Example\\AppData\\Local\\Programs\\codebase-memory-mcp\\" + "codebase-memory-mcp.exe' hook-augment"; + static const char *original = + "\xEF\xBB\xBF[mcp_servers.other]\r\n" + "command = \"other\"\r\n" + "keep = true\r\n" + "# >>> codebase-memory-mcp SessionStart >>>\r\n" + "[[hooks.SessionStart]]\r\n" + "matcher = \"startup|resume|clear|compact\"\r\n" + "\r\n" + "[[hooks.SessionStart.hooks]]\r\n" + "type = \"command\"\r\n" + "command = \"'C:\\\\Users\\\\Example\\\\AppData\\\\Local\\\\Programs\\\\" + "codebase-memory-mcp\\\\codebase-memory-mcp.exe' hook-augment\"\r\n" + "command_windows = \"& 'C:\\\\Users\\\\Example\\\\AppData\\\\Local\\\\Programs\\\\" + "codebase-memory-mcp\\\\codebase-memory-mcp.exe' hook-augment\"\r\n" + "timeout = 5\r\n" + "\r\n" + "[[hooks.SubagentStart]]\r\n" + "matcher = \"*\"\r\n" + "\r\n" + "[[hooks.SubagentStart.hooks]]\r\n" + "type = \"command\"\r\n" + "command = \"'C:\\\\Users\\\\Example\\\\AppData\\\\Local\\\\Programs\\\\" + "codebase-memory-mcp\\\\codebase-memory-mcp.exe' hook-augment\"\r\n" + "command_windows = \"& 'C:\\\\Users\\\\Example\\\\AppData\\\\Local\\\\Programs\\\\" + "codebase-memory-mcp\\\\codebase-memory-mcp.exe' hook-augment\"\r\n" + "timeout = 5\r\n" + "# <<< codebase-memory-mcp SessionStart <<<\r\n"; + char dir[CTE_PATH_CAP]; + char path[CTE_PATH_CAP]; + char before[CTE_FILE_CAP]; + char actual[CTE_FILE_CAP]; + char invalid[2][CTE_FILE_CAP]; + ASSERT_EQ(cte_fixture(dir, sizeof(dir), path, sizeof(path)), 0); + ASSERT_EQ(th_write_file(path, original), 0); + ASSERT_EQ(cte_read(path, before, sizeof(before)), 0); + + cbm_toml_codex_hook_failure_t failure = CBM_TOML_CODEX_HOOK_FAILURE_INVALID_ARGUMENT; + int reconcile_rc = cte_codex_edit_commands_detailed(path, command, command_windows, + CBM_TOML_CODEX_HOOK_UPSERT, 1, &failure); + ASSERT_EQ(failure, CBM_TOML_CODEX_HOOK_FAILURE_NONE); + ASSERT_EQ(reconcile_rc, 0); + ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0); + ASSERT_STR_EQ(actual, before); + + ASSERT_EQ(cte_codex_edit_commands_detailed(path, command, command_windows, + CBM_TOML_CODEX_HOOK_UPSERT, 0, &failure), + 0); + ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0); + ASSERT_STR_EQ(actual, before); + ASSERT_EQ(cte_occurrences(actual, CTE_CODEX_BEGIN), 1); + ASSERT_EQ(cte_occurrences(actual, "[[hooks.SessionStart]]"), 1); + ASSERT_EQ(cte_occurrences(actual, "[[hooks.SubagentStart]]"), 1); + ASSERT_NOT_NULL(strstr(actual, "[mcp_servers.other]")); + ASSERT_NOT_NULL(strstr(actual, "keep = true")); + for (const char *cursor = actual; *cursor; ++cursor) { + if (*cursor == '\n') { + ASSERT(cursor > actual && cursor[-1] == '\r'); + } + } + + ASSERT_EQ(cte_codex_edit_commands_detailed(path, command, command_windows, + CBM_TOML_CODEX_HOOK_UPSERT, 0, &failure), + 0); + ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0); + ASSERT_STR_EQ(actual, before); + + ASSERT_EQ(cte_codex_edit_commands_detailed(path, command, command_windows, + CBM_TOML_CODEX_HOOK_REMOVE, 0, &failure), + 0); + ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0); + ASSERT_NOT_NULL(strstr(actual, "[mcp_servers.other]")); + ASSERT_NOT_NULL(strstr(actual, "keep = true")); + ASSERT_NULL(strstr(actual, CTE_CODEX_BEGIN)); + ASSERT_NULL(strstr(actual, "hook-augment")); + + static const char *owned_line = + "command = \"'C:\\\\Users\\\\Example\\\\AppData\\\\Local\\\\Programs\\\\" + "codebase-memory-mcp\\\\codebase-memory-mcp.exe' hook-augment\"\r\n"; + ASSERT_EQ(cte_replace_once(original, owned_line, "command = \"foreign\"\r\n", invalid[0], + sizeof(invalid[0])), + 0); + ASSERT_EQ(cte_replace_once(original, "type = \"command\"\r\n", + "type = \"command\"\r\nenabled = true\r\n", invalid[1], + sizeof(invalid[1])), + 0); + for (size_t i = 0U; i < sizeof(invalid) / sizeof(invalid[0]); ++i) { + ASSERT_EQ(th_write_file(path, invalid[i]), 0); + failure = CBM_TOML_CODEX_HOOK_FAILURE_NONE; + ASSERT_EQ(cte_codex_edit_commands_detailed(path, command, command_windows, + CBM_TOML_CODEX_HOOK_UPSERT, 1, &failure), + -1); + ASSERT_EQ(failure, CBM_TOML_CODEX_HOOK_FAILURE_AMBIGUOUS_OWNERSHIP); + ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0); + ASSERT_STR_EQ(actual, invalid[i]); + ASSERT_EQ(cte_codex_edit_commands_detailed(path, command, command_windows, + CBM_TOML_CODEX_HOOK_UPSERT, 0, &failure), + -1); + ASSERT_EQ(failure, CBM_TOML_CODEX_HOOK_FAILURE_AMBIGUOUS_OWNERSHIP); + ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0); + ASSERT_STR_EQ(actual, invalid[i]); + } + th_cleanup(dir); + PASS(); +} + TEST(config_toml_codex_rejects_ambiguous_inline_byte_identically) { char dir[CTE_PATH_CAP]; char path[CTE_PATH_CAP]; @@ -1301,6 +1441,7 @@ SUITE(config_toml_edit) { RUN_TEST(config_toml_vibe_ambiguous_target_fail_closed); RUN_TEST(config_toml_target_table_rejects_significant_nonassignments_byte_identically); RUN_TEST(config_toml_codex_reconciles_minimal_owned_forms); + RUN_TEST(config_toml_codex_accepts_v0102_managed_windows_crlf); RUN_TEST(config_toml_codex_rejects_ambiguous_inline_byte_identically); RUN_TEST(config_toml_codex_reports_stable_failure_reasons); RUN_TEST(config_toml_codex_preserves_bom_crlf_and_foreign_aot);