diff --git a/src/cli/cli.c b/src/cli/cli.c index d85ebf0b6..a224310e7 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -10802,6 +10802,30 @@ int cbm_cmd_install(int argc, char **argv) { /* ── Subcommand: uninstall ────────────────────────────────────── */ +/* One wording for every line the uninstall report prints. + * + * The report used to be written in the past tense whatever happened: a + * --dry-run said "removed" for files it never opened, and a step that had just + * recorded an error said "removed" on the next line (#1954). A reader could + * only conclude that a dozen configuration files had been rewritten. + * + * Every report line now goes through this verb, and the caller prints the line + * only when the step really ran and worked. A failed step prints nothing here + * because record_agent_config_error has already named the agent, the operation + * and the path on stderr. The shape copies the Qoder and Devin blocks below, + * which already said "planned" for a dry run and "failed" for a failure. */ +static const char *uninstall_verb(bool dry_run) { + return dry_run ? "would remove" : "removed"; +} + +/* True when nothing has recorded a failure since `errors_before` was read from + * g_agent_uninstall_errors. Several report lines below cover a run of steps at + * once ("removed MCP config + hooks + instructions"), and this ties such a line + * to whether every one of those steps worked. */ +static bool uninstall_steps_succeeded(int errors_before) { + return g_agent_uninstall_errors == errors_before; +} + /* Remove Claude Code agent configs. */ static void uninstall_claude_code(const char *home, const char *installed_binary, bool dry_run) { char config_dir[CLI_BUF_1K]; @@ -10812,7 +10836,7 @@ static void uninstall_claude_code(const char *home, const char *installed_binary char skills_dir[CLI_BUF_1K]; snprintf(skills_dir, sizeof(skills_dir), "%s/skills", config_dir); int removed = cbm_remove_skills(skills_dir, dry_run); - printf("Claude Code: removed %d skill(s)\n", removed); + printf("Claude Code: %s %d skill(s)\n", uninstall_verb(dry_run), removed); char agent_path[CLI_BUF_1K]; snprintf(agent_path, sizeof(agent_path), "%s/agents/codebase-memory.md", config_dir); uninstall_tiered_agent_profiles( @@ -10827,10 +10851,14 @@ static void uninstall_claude_code(const char *home, const char *installed_binary char mcp_path[CLI_BUF_1K]; snprintf(mcp_path, sizeof(mcp_path), "%s/.mcp.json", config_dir); + bool legacy_mcp_removed = true; if (!dry_run && cbm_remove_editor_mcp_owned(installed_binary, mcp_path) != CLI_OK) { record_agent_config_error(true, "Claude Code", "legacy_mcp_uninstall", mcp_path); + legacy_mcp_removed = false; + } + if (legacy_mcp_removed) { + printf(" %s MCP config entry\n", uninstall_verb(dry_run)); } - printf(" removed MCP config entry\n"); char mcp_path2[CLI_BUF_1K]; snprintf(mcp_path2, sizeof(mcp_path2), "%s/.claude.json", user_root); @@ -10840,6 +10868,7 @@ static void uninstall_claude_code(const char *home, const char *installed_binary char settings_path[CLI_BUF_1K]; snprintf(settings_path, sizeof(settings_path), "%s/settings.json", config_dir); + int hook_errors_before = g_agent_uninstall_errors; if (!dry_run) { if (cbm_remove_claude_hooks_with_binary(settings_path, installed_binary) != CLI_OK) { record_agent_config_error(true, "Claude Code", "pretool_hook_uninstall", settings_path); @@ -10933,7 +10962,9 @@ static void uninstall_claude_code(const char *home, const char *installed_binary #endif } } - printf(" removed PreToolUse + SessionStart + SubagentStart hooks\n"); + if (uninstall_steps_succeeded(hook_errors_before)) { + printf(" %s PreToolUse + SessionStart + SubagentStart hooks\n", uninstall_verb(dry_run)); + } } /* Remove MCP + instructions for a generic agent. */ @@ -10943,28 +10974,41 @@ typedef struct { const char *config_path; const char *instr_path; } mcp_uninstall_args_t; +/* Remove MCP + instructions for a generic agent, and report only what the run + * really did. Both lines below used to print every time: a config entry that + * was preserved because it had been modified said "preserved" and "removed" one + * after the other, a config the run had just failed to edit still said + * "removed", and a --dry-run said "removed" for files it never opened + * (#1954). */ static void uninstall_agent_mcp_instr(mcp_uninstall_args_t paths, bool dry_run, int (*remove_fn)(const char *, const char *)) { const char *name = paths.name; const char *instr_path = paths.instr_path; + bool mcp_removed = true; if (!dry_run) { char binary_path[CLI_BUF_1K]; cbm_agent_installed_binary_path(cbm_get_home_dir(), binary_path, sizeof(binary_path)); int remove_result = remove_fn(binary_path, paths.config_path); if (remove_result < CLI_OK) { record_agent_config_error(true, name, "mcp_uninstall", paths.config_path); + mcp_removed = false; } else if (remove_result > CLI_OK) { printf("%s: preserved modified or foreign MCP entry\n", name); + mcp_removed = false; } } - printf("%s: removed MCP config entry\n", name); + if (mcp_removed) { + printf("%s: %s MCP config entry\n", name, uninstall_verb(dry_run)); + } if (instr_path) { - if (!dry_run) { - if (cbm_remove_instructions(instr_path) != CLI_OK) { - record_agent_config_error(true, name, "instructions_uninstall", instr_path); - } + bool instructions_removed = true; + if (!dry_run && cbm_remove_instructions(instr_path) != CLI_OK) { + record_agent_config_error(true, name, "instructions_uninstall", instr_path); + instructions_removed = false; + } + if (instructions_removed) { + printf(" %s instructions\n", uninstall_verb(dry_run)); } - printf(" removed instructions\n"); } } @@ -10972,16 +11016,18 @@ static bool uninstall_codex_activation_pointer(const char *path, bool dry_run) { return path && (dry_run || cbm_remove_instructions(path) == CLI_OK); } -static void report_codex_activation_pointer_uninstall(const char *path, bool removed) { - printf(" instructions: removed managed activation pointer\n"); - if (!removed) { +static void report_codex_activation_pointer_uninstall(const char *path, bool removed, + bool dry_run) { + if (removed) { + printf(" instructions: %s managed activation pointer\n", uninstall_verb(dry_run)); + } else { record_agent_config_error(true, "Codex CLI", "instructions_uninstall", path); } } static void uninstall_agent_skill(const char *label, const char *skills_dir, bool dry_run) { int removed = cbm_remove_skills(skills_dir, dry_run); - printf(" %s skill: %d removed\n", label, removed); + printf(" %s skill: %d %s\n", label, removed, dry_run ? "would be removed" : "removed"); } static void uninstall_copilot_durable_context(const char *home, bool dry_run) { @@ -10995,8 +11041,10 @@ static void uninstall_copilot_durable_context(const char *home, bool dry_run) { snprintf(skills_dir, sizeof(skills_dir), "%s/skills", config_dir); snprintf(agent_path, sizeof(agent_path), "%s/agents/codebase-memory.agent.md", config_dir); cbm_agent_installed_binary_path(home, binary_path, sizeof(binary_path)); + bool copilot_hooks_removed = true; if (!dry_run && cbm_remove_copilot_hooks(hook_path, binary_path) != CLI_OK) { record_agent_config_error(true, "Copilot", "lifecycle_hook_uninstall", hook_path); + copilot_hooks_removed = false; } uninstall_agent_skill("Copilot", skills_dir, dry_run); uninstall_tiered_agent_profiles( @@ -11008,7 +11056,9 @@ static void uninstall_copilot_durable_context(const char *home, bool dry_run) { .dialect = CBM_GRAPH_DIALECT_COPILOT, }, dry_run); - printf(" removed SessionStart + SubagentStart hooks\n"); + if (copilot_hooks_removed) { + printf(" %s SessionStart + SubagentStart hooks\n", uninstall_verb(dry_run)); + } } static int cbm_remove_managed_instructions(const char *instructions_path) { @@ -11072,8 +11122,9 @@ static void uninstall_gitlab_durable_context(const cbm_agent_registry_context_t } if (!dry_run && cbm_remove_gitlab_session_hook(hooks_path, binary_path) != CLI_OK) { record_agent_config_error(true, "GitLab Duo CLI", "session_hook_uninstall", hooks_path); + } else { + printf(" hook: %s canonical SessionStart entry\n", uninstall_verb(dry_run)); } - printf(" hook: removed canonical SessionStart entry\n"); } static void uninstall_devin_durable_context(const cbm_agent_registry_context_t *registry, @@ -11113,8 +11164,9 @@ static void uninstall_pi_durable_context(const char *home, bool dry_run) { snprintf(skills_dir, sizeof(skills_dir), "%s/.pi/agent/skills", home); if (!dry_run && cbm_remove_managed_instructions(instructions_path) != CLI_OK) { record_agent_config_error(true, "Pi", "instructions_uninstall", instructions_path); + } else { + printf(" instructions: %s managed context\n", uninstall_verb(dry_run)); } - printf(" instructions: removed managed context\n"); uninstall_agent_skill("Pi", skills_dir, dry_run); char extension_path[CLI_BUF_1K]; snprintf(extension_path, sizeof(extension_path), "%s/.pi/agent/extensions/cbmem.ts", home); @@ -11125,8 +11177,9 @@ static void uninstall_managed_agent_instructions(const char *label, const char * bool dry_run) { if (!dry_run && cbm_remove_managed_instructions(instructions_path) != CLI_OK) { record_agent_config_error(true, label, "instructions_uninstall", instructions_path); + } else { + printf(" instructions: %s managed context\n", uninstall_verb(dry_run)); } - printf(" instructions: removed managed context\n"); } static bool remove_cline_context_hooks(const char *cline_root, const char *binary_path, @@ -11178,8 +11231,9 @@ static void uninstall_kimi_durable_context(const cbm_agent_registry_context_t *r snprintf(config_path, sizeof(config_path), "%s/config.toml", kimi_home); if (!dry_run && cbm_remove_kimi_context_hook(config_path) != CLI_OK) { record_agent_config_error(true, "Kimi Code CLI", "prompt_hook_uninstall", config_path); + } else { + printf(" hook: %s managed UserPromptSubmit entry\n", uninstall_verb(dry_run)); } - printf(" hook: removed managed UserPromptSubmit entry\n"); uninstall_managed_agent_instructions("Kimi Code CLI", instructions_path, dry_run); uninstall_agent_skill("Kimi Code CLI", skills_dir, dry_run); } @@ -11318,11 +11372,18 @@ static void uninstall_agent_client_registry(const char *home, bool dry_run) { : profile->remove_mcp(profile->id, config_path, binary_path); if (edit_result == CBM_AGENT_EDIT_FOREIGN) { printf(" mcp: preserved modified or foreign entry in %s\n", config_path); + } else if (edit_result == CBM_AGENT_EDIT_NOT_APPLICABLE) { + /* The client has no MCP editor for this config shape, so + * there was never an entry of ours to take out. Counting + * that as a failure is what made a whole uninstall refuse + * over a file it had never written to (#1954). */ + printf(" mcp: no managed entry to remove in %s\n", config_path); } else if (edit_result != CBM_AGENT_EDIT_OK) { record_agent_config_error(true, profile->display_name, "mcp_uninstall", config_path); } else { - printf(" mcp: removed canonical entry from %s\n", config_path); + printf(" mcp: %s canonical entry from %s\n", uninstall_verb(dry_run), + config_path); } } } @@ -11368,6 +11429,7 @@ static void uninstall_gemini_config(const char *home, bool dry_run) { snprintf(cp, sizeof(cp), "%s/.gemini/settings.json", home); snprintf(ip, sizeof(ip), "%s/.gemini/GEMINI.md", home); snprintf(ap, sizeof(ap), "%s/.gemini/agents/codebase-memory.md", home); + int gemini_errors_before = g_agent_uninstall_errors; if (!dry_run) { if (cbm_remove_editor_mcp_owned(installed_binary, cp) != CLI_OK) { record_agent_config_error(true, "Gemini CLI", "mcp_uninstall", cp); @@ -11396,7 +11458,10 @@ static void uninstall_gemini_config(const char *home, bool dry_run) { .dialect = CBM_GRAPH_DIALECT_GEMINI, }, dry_run); - printf("Gemini CLI: removed MCP config + hooks + instructions + tiered subagents\n"); + if (uninstall_steps_succeeded(gemini_errors_before)) { + printf("Gemini CLI: %s MCP config + hooks + instructions + tiered subagents\n", + uninstall_verb(dry_run)); + } } static void uninstall_cli_agents(const cbm_detected_agents_t *agents, const char *home, @@ -11434,12 +11499,12 @@ static void uninstall_cli_agents(const cbm_detected_agents_t *agents, const char ? NULL : cbm_toml_codex_hook_failure_name(preflight_failure); record_agent_config_error_with_reason(true, "Codex CLI", "hook_preflight", cp, reason); - report_codex_activation_pointer_uninstall(ip, pointer_removed); + report_codex_activation_pointer_uninstall(ip, pointer_removed, dry_run); goto codex_toml_done; } uninstall_agent_mcp_instr((mcp_uninstall_args_t){"Codex CLI", cp, NULL}, dry_run, cbm_remove_codex_mcp_owned); - report_codex_activation_pointer_uninstall(ip, pointer_removed); + report_codex_activation_pointer_uninstall(ip, pointer_removed, dry_run); if (!dry_run && cbm_reconcile_codex_hooks_command(cp, hook_command, hook_command_windows, CBM_TOML_CODEX_HOOK_REMOVE, false) != CLI_OK) { @@ -11514,6 +11579,7 @@ static void uninstall_cli_agents(const cbm_detected_agents_t *agents, const char char ip[CLI_BUF_1K]; snprintf(cp, sizeof(cp), "%s/.aider.conf.yml", home); snprintf(ip, sizeof(ip), "%s/CONVENTIONS.md", home); + int aider_errors_before = g_agent_uninstall_errors; if (!dry_run) { if (cbm_yaml_remove_string_list_item(cp, "read", ip) != CLI_OK) { record_agent_config_error(true, "Aider", "loader_uninstall", cp); @@ -11522,7 +11588,9 @@ static void uninstall_cli_agents(const cbm_detected_agents_t *agents, const char record_agent_config_error(true, "Aider", "instructions_uninstall", ip); } } - printf("Aider: removed instructions + loader reference\n"); + if (uninstall_steps_succeeded(aider_errors_before)) { + printf("Aider: %s instructions + loader reference\n", uninstall_verb(dry_run)); + } } } @@ -11551,6 +11619,7 @@ static void uninstall_editor_agents(const cbm_detected_agents_t *agents, const c snprintf(cp, sizeof(cp), "%s/.config/kilo/kilo.jsonc", home); snprintf(ip, sizeof(ip), "%s/.config/kilo/rules/codebase-memory-mcp.md", home); snprintf(ap, sizeof(ap), "%s/.config/kilo/agents/codebase-memory.md", home); + int kilo_errors_before = g_agent_uninstall_errors; if (!dry_run) { if (cbm_remove_kilo_mcp_owned(installed_binary, cp) != CLI_OK) { record_agent_config_error(true, "KiloCode", "mcp_uninstall", cp); @@ -11598,7 +11667,9 @@ static void uninstall_editor_agents(const cbm_detected_agents_t *agents, const c .dialect = CBM_GRAPH_DIALECT_KILO, }, dry_run); - printf("KiloCode: removed MCP config + instruction reference\n"); + if (uninstall_steps_succeeded(kilo_errors_before)) { + printf("KiloCode: %s MCP config + instruction reference\n", uninstall_verb(dry_run)); + } } if (agents->vscode) { char code_user[CLI_BUF_1K]; @@ -11647,6 +11718,7 @@ static void uninstall_editor_agents(const cbm_detected_agents_t *agents, const c bool workspace_ok = cbm_openclaw_workspace_path(home, cp, workspace, sizeof(workspace)); uninstall_agent_mcp_instr((mcp_uninstall_args_t){"OpenClaw", cp, NULL}, dry_run, cbm_remove_openclaw_mcp_owned); + int openclaw_errors_before = g_agent_uninstall_errors; if (!dry_run && cbm_remove_openclaw_compaction(cp) != CLI_OK) { record_agent_config_error(true, "OpenClaw", "compaction_uninstall", cp); } @@ -11665,9 +11737,13 @@ static void uninstall_editor_agents(const cbm_detected_agents_t *agents, const c tools_path); } } - printf(" removed workspace instructions + compaction augmentation\n"); - } else { - printf(" removed compaction augmentation; workspace instructions unresolved\n"); + if (uninstall_steps_succeeded(openclaw_errors_before)) { + printf(" %s workspace instructions + compaction augmentation\n", + uninstall_verb(dry_run)); + } + } else if (uninstall_steps_succeeded(openclaw_errors_before)) { + printf(" %s compaction augmentation; workspace instructions unresolved\n", + uninstall_verb(dry_run)); } } } @@ -11741,7 +11817,7 @@ static void uninstall_additional_agents(const cbm_detected_agents_t *agents, con } else if (hook_result != CBM_YAML_IDENTITY_EDIT_OK) { record_agent_config_error(true, "Hermes", "pre_llm_hook_uninstall", cp); } else { - printf(" hook: removed canonical pre_llm_call entry\n"); + printf(" hook: %s canonical pre_llm_call entry\n", uninstall_verb(dry_run)); } uninstall_agent_mcp_instr((mcp_uninstall_args_t){"Hermes", cp, NULL}, dry_run, cbm_remove_hermes_mcp_owned); @@ -11754,7 +11830,8 @@ static void uninstall_additional_agents(const cbm_detected_agents_t *agents, con snprintf(skills_dir, sizeof(skills_dir), "%s/.agents/skills", home); uninstall_agent_mcp_instr((mcp_uninstall_args_t){"OpenHands", cp, NULL}, dry_run, cbm_remove_editor_mcp_owned); - printf(" removed %d skill(s)\n", cbm_remove_skills(skills_dir, dry_run)); + printf(" %d skill(s) %s\n", cbm_remove_skills(skills_dir, dry_run), + dry_run ? "would be removed" : "removed"); } if (agents->augment) { char cp[CLI_BUF_1K]; @@ -11776,6 +11853,7 @@ static void uninstall_additional_agents(const cbm_detected_agents_t *agents, con cbm_agent_installed_binary_path(home, binary_path, sizeof(binary_path)); uninstall_agent_mcp_instr((mcp_uninstall_args_t){"Augment/Auggie", cp, ip}, dry_run, cbm_remove_editor_mcp_owned); + int augment_errors_before = g_agent_uninstall_errors; uninstall_tiered_agent_profiles( (cbm_tiered_profile_set_t){ .label = "Augment/Auggie", @@ -11817,7 +11895,10 @@ static void uninstall_additional_agents(const cbm_detected_agents_t *agents, con } } } - printf(" removed SessionStart + PostToolUse hooks + dedicated subagent\n"); + if (uninstall_steps_succeeded(augment_errors_before)) { + printf(" %s SessionStart + PostToolUse hooks + dedicated subagent\n", + uninstall_verb(dry_run)); + } } if (agents->cline) { char cline_root[CLI_BUF_1K]; @@ -11903,6 +11984,7 @@ static void uninstall_additional_agents(const cbm_detected_agents_t *agents, con snprintf(skills_dir, sizeof(skills_dir), "%s/.factory/skills", home); uninstall_agent_mcp_instr((mcp_uninstall_args_t){"Factory Droid", cp, ip}, dry_run, cbm_remove_factory_mcp_owned); + int factory_errors_before = g_agent_uninstall_errors; if (!dry_run && cbm_remove_factory_hooks(hp, installed_binary) != CLI_OK) { record_agent_config_error(true, "Factory Droid", "context_hook_uninstall", hp); } @@ -11915,7 +11997,9 @@ static void uninstall_additional_agents(const cbm_detected_agents_t *agents, con .dialect = CBM_GRAPH_DIALECT_FACTORY, }, dry_run); - printf(" removed SessionStart + PostToolUse hooks\n"); + if (uninstall_steps_succeeded(factory_errors_before)) { + printf(" %s SessionStart + PostToolUse hooks\n", uninstall_verb(dry_run)); + } } if (agents->crush) { char cp[CLI_BUF_1K]; @@ -12073,13 +12157,18 @@ static int cli_uninstall_activate(void *opaque) { uninstall_additional_agents(&activation->agents, activation->home, activation->dry_run); uninstall_agent_client_registry(activation->home, activation->dry_run); - if (g_agent_uninstall_errors != 0) { - cli_activation_transaction_abort_or_fail_stop(&activation->binary_transaction, - "uninstall_transaction_config_cleanup_abort"); - (void)fprintf(stderr, "error: one or more agent cleanup operations failed; executable " - "and index removal were not started\n"); - return CLI_ACTIVATION_PARTIAL; - } + /* An agent configuration that could not be cleaned up used to stop the run + * right here, before the executable and the indexes were touched. One + * unwritable file — a Cursor config symlinked into a dotfiles repository, + * or an agent detected by its binary on PATH whose config this HOME never + * had — then left a 282 MB executable and a 104 MB index cache behind after + * a run that printed "Uninstall complete" (#1954). + * + * Cleaning up an agent's configuration and removing this tool's own files + * are separate jobs. A failure in the first no longer cancels the second. + * Every failure is still named on stderr as it happens, counted, summarised + * at the end of the run, and still makes the exit code non-zero, so nothing + * about the failure becomes quieter — only the blast radius shrinks. */ if (activation->delete_indexes && !activation->dry_run) { int expected = count_db_indexes(activation->home); @@ -12252,8 +12341,21 @@ int cbm_cmd_uninstall(int argc, char **argv) { return CLI_TRUE; } - printf("\nUninstall complete. Please restart your coding-agent sessions " - "to properly take this into account.\n"); + if (g_agent_uninstall_errors == 0) { + printf("\nUninstall complete. Please restart your coding-agent sessions " + "to properly take this into account.\n"); + } else { + /* Say the count out loud. The per-failure lines are on stderr, which a + * reader who only watches stdout never sees, and the run is no longer + * stopped by them — so this is the one place that tells the reader some + * configuration files still hold entries of ours. */ + printf("\nUninstall finished, and %d agent configuration cleanup step(s) failed.\n" + "Look for the \"error: agent_config\" lines above: those files still need an\n" + "edit by hand. Removal of the executable and the indexes went ahead anyway.\n" + "Please restart your coding-agent sessions to properly take this into " + "account.\n", + g_agent_uninstall_errors); + } if (dry_run) { printf("(dry-run — no files were modified)\n"); } diff --git a/tests/test_cli.c b/tests/test_cli.c index 976280868..fb331d90a 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -2277,6 +2277,198 @@ TEST(cli_uninstall_preserves_binary_and_index_when_cohort_does_not_drain) { PASS(); } +/* #1954: one agent config that cannot be cleaned up must not keep the + * executable and the indexes on disk. + * + * The fixture is the reporter's own setup: ~/.cursor/mcp.json is a symlink into + * a dotfiles directory, which the JSON editors refuse to write through. Before + * the fix that single refusal returned early, so a run that printed + * "Uninstall complete" left a 282 MB executable and the whole index cache + * behind. PATH moves with HOME so a real agent binary on the developer's + * machine cannot add a second, unrelated failure. */ +#ifndef _WIN32 +TEST(cli_uninstall_removes_binary_and_index_despite_agent_config_failure) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-uninstall-linked-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) { + FAIL("cbm_mkdtemp failed"); + } + char *old_home = NULL; + char *old_cache = NULL; + cli_activation_save_env(&old_home, &old_cache); + cbm_setenv("HOME", tmpdir, 1); + char *old_path = save_test_env("PATH"); + cbm_setenv("PATH", tmpdir, 1); + + char dotfiles_dir[512]; + char cursor_dir[512]; + char link_target[640]; + char cursor_config[640]; + snprintf(dotfiles_dir, sizeof(dotfiles_dir), "%s/dotfiles", tmpdir); + snprintf(cursor_dir, sizeof(cursor_dir), "%s/.cursor", tmpdir); + test_mkdirp(dotfiles_dir); + test_mkdirp(cursor_dir); + snprintf(link_target, sizeof(link_target), "%s/mcp.json", dotfiles_dir); + snprintf(cursor_config, sizeof(cursor_config), "%s/mcp.json", cursor_dir); + write_test_file(link_target, "{\"mcpServers\":{}}\n"); + if (symlink(link_target, cursor_config) != 0) { + cli_activation_restore_env(old_home, old_cache); + restore_test_env("PATH", old_path); + test_rmdir_r(tmpdir); + FAIL("failed to create the linked Cursor config fixture"); + } + + char cache_dir[512]; + char index_path[640]; + snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); + cbm_setenv("CBM_CACHE_DIR", cache_dir, 1); + test_mkdirp(cache_dir); + snprintf(index_path, sizeof(index_path), "%s/project.db", cache_dir); + write_test_file(index_path, "index must not survive a linked agent config"); + + char bin_dir[512]; + char bin_target[640]; + snprintf(bin_dir, sizeof(bin_dir), "%s/.local/bin", tmpdir); + test_mkdirp(bin_dir); + snprintf(bin_target, sizeof(bin_target), "%s/codebase-memory-mcp", bin_dir); + write_test_file(bin_target, "binary must not survive a linked agent config"); + + cli_activation_fake_t fake = { + .participants_active = true, + .mutation_reserve_result = 1, + }; + cbm_cli_activation_ops_t ops = cli_activation_fake_ops(&fake); + cbm_cli_set_activation_ops_for_test(&ops); + + FILE *capture = tmpfile(); + int saved_stdout = capture ? dup(STDOUT_FILENO) : -1; + int saved_stderr = capture ? dup(STDERR_FILENO) : -1; + int rc = -1; + if (capture && saved_stdout >= 0 && saved_stderr >= 0) { + fflush(NULL); + if (dup2(fileno(capture), STDOUT_FILENO) >= 0 && + dup2(fileno(capture), STDERR_FILENO) >= 0) { + char *argv[] = {"--yes"}; + rc = cli_test_cmd_uninstall(1, argv); + } + 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); + } + cbm_cli_set_activation_ops_for_test(NULL); + cbm_set_auto_answer_for_test(0); + + bool index_left = read_test_file(index_path) != NULL; + bool binary_left = read_test_file(bin_target) != NULL; + bool link_intact = read_test_file(link_target) != NULL; + bool cleanup_failure_reported = + strstr(output, "agent configuration cleanup step(s) failed") != NULL; + bool old_abort_message = + strstr(output, "executable and index removal were not started") != NULL; + cli_activation_restore_env(old_home, old_cache); + restore_test_env("PATH", old_path); + test_rmdir_r(tmpdir); + + /* The failed config cleanup still fails the run. */ + ASSERT_EQ(rc, 1); + ASSERT_FALSE(index_left); + ASSERT_FALSE(binary_left); + /* The link's target was never written through. */ + ASSERT_TRUE(link_intact); + ASSERT_TRUE(cleanup_failure_reported); + ASSERT_FALSE(old_abort_message); + PASS(); +} +#endif + +/* #1954, second half: a --dry-run must not describe removals it did not make. + * Every per-client line used to be written in the past tense, so a dry run read + * as a report of a dozen rewritten configuration files. */ +TEST(cli_uninstall_dry_run_report_uses_the_future_tense) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-uninstall-tense-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) { + FAIL("cbm_mkdtemp failed"); + } + char *old_home = NULL; + char *old_cache = NULL; + cli_activation_save_env(&old_home, &old_cache); + cbm_setenv("HOME", tmpdir, 1); + char *old_path = save_test_env("PATH"); + cbm_setenv("PATH", tmpdir, 1); + + char cursor_dir[512]; + char cursor_config[640]; + snprintf(cursor_dir, sizeof(cursor_dir), "%s/.cursor", tmpdir); + test_mkdirp(cursor_dir); + snprintf(cursor_config, sizeof(cursor_config), "%s/mcp.json", cursor_dir); + const char *before = "{\"mcpServers\":{}}\n"; + write_test_file(cursor_config, before); + + char cache_dir[512]; + snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); + cbm_setenv("CBM_CACHE_DIR", cache_dir, 1); + test_mkdirp(cache_dir); + + FILE *capture = tmpfile(); + int saved_stdout = capture ? dup(STDOUT_FILENO) : -1; + int saved_stderr = capture ? dup(STDERR_FILENO) : -1; + int rc = -1; + if (capture && saved_stdout >= 0 && saved_stderr >= 0) { + fflush(NULL); + if (dup2(fileno(capture), STDOUT_FILENO) >= 0 && + dup2(fileno(capture), STDERR_FILENO) >= 0) { + char *argv[] = {"--dry-run", "--yes"}; + rc = cli_test_cmd_uninstall(2, argv); + } + 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); + } + cbm_set_auto_answer_for_test(0); + + char *after = read_test_file_alloc(cursor_config); + bool config_untouched = after && strcmp(after, before) == 0; + free(after); + bool says_would_remove = strstr(output, "Cursor: would remove MCP config entry") != NULL; + bool claims_a_past_removal = strstr(output, "Cursor: removed MCP config entry") != NULL; + cli_activation_restore_env(old_home, old_cache); + restore_test_env("PATH", old_path); + test_rmdir_r(tmpdir); + + ASSERT_EQ(rc, 0); + ASSERT_TRUE(config_untouched); + ASSERT_TRUE(says_would_remove); + ASSERT_FALSE(claims_a_past_removal); + PASS(); +} + TEST(cli_activation_guard_is_bypassed_for_dry_run_and_plan) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-daemon-stateless-XXXXXX"); @@ -4613,6 +4805,11 @@ TEST(cli_agent_install_reports_safe_editor_refusal) { PASS(); } +/* A safe-editor refusal still fails the run, and it no longer keeps the + * executable. Removing an agent's configuration and removing this tool's own + * files are separate jobs, so one malformed OpenClaw config does not cancel the + * other (#1954). The two claims that mattered before are unchanged: the exit + * code is non-zero, and the malformed file is never written through. */ TEST(cli_agent_uninstall_reports_safe_editor_refusal) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-uninstall-refusal-XXXXXX"); @@ -4635,7 +4832,7 @@ TEST(cli_agent_uninstall_reports_safe_editor_refusal) { #else snprintf(bin_path, sizeof(bin_path), "%s/codebase-memory-mcp", bin_dir); #endif - write_test_file(bin_path, "installed binary must remain live\n"); + write_test_file(bin_path, "installed binary must not outlive the uninstall\n"); char *saved_home = save_test_env("HOME"); char *saved_path = save_test_env("PATH"); @@ -4659,9 +4856,10 @@ TEST(cli_agent_uninstall_reports_safe_editor_refusal) { restore_test_env("HOME", saved_home); restore_test_env("PATH", saved_path); test_rmdir_r(tmpdir); - if (rc == 0 || !preserved || !binary_preserved || fake.mutation_reserve_count != 1 || - fake.mutation_lease_release_count != 1 || !strstr(fake.diagnostic, "executable was kept")) - FAIL("agent uninstall refusal must fail before removing the live binary"); + if (rc == 0 || !preserved || binary_preserved || fake.mutation_reserve_count != 1 || + fake.mutation_lease_release_count != 1 || fake.diagnostic[0] != '\0') + FAIL("agent uninstall refusal must fail the run, keep the malformed config untouched, " + "and still remove the executable"); PASS(); } @@ -10618,7 +10816,8 @@ TEST(cli_codex_migrates_to_single_hook_representation) { #else snprintf(binary_path, sizeof(binary_path), "%s/codebase-memory-mcp", binary_dir); #endif - write_test_file(binary_path, "installed binary must survive failed cleanup\n"); + /* #1954: a failed agent-config cleanup no longer keeps the executable. */ + write_test_file(binary_path, "installed binary must not survive a failed cleanup\n"); char *saved_home = save_test_env("HOME"); char *saved_path = save_test_env("PATH"); @@ -10673,7 +10872,7 @@ TEST(cli_codex_migrates_to_single_hook_representation) { struct stat state; hooks = read_test_file_alloc(hooks_path); char *agents_after_uninstall = read_test_file_alloc(agents_path); - bool independent_cleanup = uninstall_rc != 0 && stat(binary_path, &state) == 0 && + bool independent_cleanup = uninstall_rc != 0 && stat(binary_path, &state) != 0 && stat(skill_path, &state) != 0 && stat(agent_path, &state) != 0 && hooks && !strstr(hooks, "hook-augment") && agents_after_uninstall && agents_after_uninstall[0] == '\0'; @@ -14793,6 +14992,10 @@ SUITE(cli) { RUN_TEST(cli_update_agent_configs_finish_before_guard_release); RUN_TEST(cli_uninstall_quiesces_active_cohort_before_removing_binary_and_index); RUN_TEST(cli_uninstall_preserves_binary_and_index_when_cohort_does_not_drain); +#ifndef _WIN32 + RUN_TEST(cli_uninstall_removes_binary_and_index_despite_agent_config_failure); +#endif + RUN_TEST(cli_uninstall_dry_run_report_uses_the_future_tense); RUN_TEST(cli_activation_guard_is_bypassed_for_dry_run_and_plan); #ifdef _WIN32 RUN_TEST(cli_windows_update_hands_off_to_install_script);