From 9ee9a07d852a498718d1fc81b67ab69fa0eca359 Mon Sep 17 00:00:00 2001 From: metehanulusoy Date: Sun, 12 Jul 2026 12:14:13 +0300 Subject: [PATCH] fix(cli): print help and require confirmation before destructive uninstall `uninstall --help` performed a real uninstall: unknown flags were silently ignored by the mutating subcommands, and the agent-config removal and binary deletion ran without any confirmation gate (the only prompt guarded index deletion). - install/uninstall/update now recognize --help/-h and print usage without touching anything - uninstall rejects unknown flags with an error + usage (a typo like `--dryrun` no longer triggers a real uninstall) - uninstall asks for confirmation before removing agent configs and the binary; -y auto-confirms (unchanged for scripts), -n and non-TTY stdin abort. --dry-run skips the prompt as it mutates nothing, keeping `uninstall --dry-run -y` (smoke-test phase 6b) working as before. Fixes #1038 Signed-off-by: metehanulusoy Co-Authored-By: Claude Fable 5 --- src/cli/cli.c | 71 +++++++++++++++++++++++++ tests/test_cli.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 205 insertions(+) diff --git a/src/cli/cli.c b/src/cli/cli.c index 8ffc679f0..5f2328d57 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -2964,6 +2964,49 @@ static bool prompt_yn(const char *question) { return (buf[0] == 'y' || buf[0] == 'Y') ? true : false; } +/* ── Subcommand help ──────────────────────────────────────────── */ + +/* Mutating subcommands (install/uninstall/update) must never run when the + * user asked for usage: `uninstall --help` used to perform a real uninstall + * because unknown flags were silently ignored (#1038). */ +static bool cmd_wants_help(int argc, char **argv) { + for (int i = 0; i < argc; i++) { + if (strcmp(argv[i], "--help") == 0 || strcmp(argv[i], "-h") == 0) { + return true; + } + } + return false; +} + +static void print_install_usage(void) { + printf("Usage: codebase-memory-mcp install [-y|-n] [--force] [--dry-run]\n"); + printf(" [--plan] [--reset-indexes]\n\n"); + printf(" -y, --yes Answer yes to all prompts\n"); + printf(" -n, --no Answer no to all prompts\n"); + printf(" --force Overwrite existing skills and config entries\n"); + printf(" --dry-run Show what would change without modifying files\n"); + printf(" --plan Print machine-readable install plan (no changes)\n"); + printf(" --reset-indexes Delete existing indexes during install\n"); +} + +static void print_uninstall_usage(void) { + printf("Usage: codebase-memory-mcp uninstall [-y|-n] [--dry-run]\n\n"); + printf(" -y, --yes Answer yes to all prompts (required without a terminal)\n"); + printf(" -n, --no Answer no to all prompts (aborts the uninstall)\n"); + printf(" --dry-run Show what would be removed without modifying files\n"); +} + +static void print_update_usage(void) { + printf("Usage: codebase-memory-mcp update [-y|-n] [--force] [--dry-run]\n"); + printf(" [--standard|--ui]\n\n"); + printf(" -y, --yes Answer yes to all prompts\n"); + printf(" -n, --no Answer no to all prompts\n"); + printf(" --force Update even if already on the latest version\n"); + printf(" --dry-run Show what would change without modifying files\n"); + printf(" --standard Select the standard variant\n"); + printf(" --ui Select the UI variant\n"); +} + /* ── SHA-256 checksum verification ─────────────────────────────── */ /* SHA-256 hex digest: 64 hex chars + NUL */ @@ -3748,6 +3791,10 @@ char *cbm_build_install_plan_json(const char *home, const char *binary_path) { } int cbm_cmd_install(int argc, char **argv) { + if (cmd_wants_help(argc, argv)) { + print_install_usage(); + return 0; + } parse_auto_answer(argc, argv); bool dry_run = false; bool force = false; @@ -4085,11 +4132,22 @@ static void uninstall_editor_agents(const cbm_detected_agents_t *agents, const c } int cbm_cmd_uninstall(int argc, char **argv) { + if (cmd_wants_help(argc, argv)) { + print_uninstall_usage(); + return 0; + } parse_auto_answer(argc, argv); bool dry_run = false; for (int i = 0; i < argc; i++) { if (strcmp(argv[i], "--dry-run") == 0) { dry_run = true; + } else if (strcmp(argv[i], "-y") != 0 && strcmp(argv[i], "--yes") != 0 && + strcmp(argv[i], "-n") != 0 && strcmp(argv[i], "--no") != 0) { + /* Unknown flags must not fall through to a destructive uninstall: + * `uninstall --dryrun` (typo) used to perform a REAL uninstall (#1038). */ + (void)fprintf(stderr, "error: unknown uninstall option: %s\n\n", argv[i]); + print_uninstall_usage(); + return CLI_TRUE; } } @@ -4101,6 +4159,15 @@ int cbm_cmd_uninstall(int argc, char **argv) { printf("codebase-memory-mcp uninstall\n\n"); + /* Removing agent configs and the binary is destructive: gate it on + * explicit confirmation. Non-interactive callers must pass -y; + * prompt_yn() fails closed without a terminal (#1038). */ + if (!dry_run && + !prompt_yn("Remove codebase-memory-mcp from all detected agents and delete the binary?")) { + printf("Uninstall cancelled. No files were modified.\n"); + return CLI_TRUE; + } + cbm_detected_agents_t agents = cbm_detect_agents(home); if (agents.claude_code) { uninstall_claude_code(home, dry_run); @@ -4382,6 +4449,10 @@ static bool check_already_latest(void) { } int cbm_cmd_update(int argc, char **argv) { + if (cmd_wants_help(argc, argv)) { + print_update_usage(); + return 0; + } parse_auto_answer(argc, argv); bool dry_run = false; diff --git a/tests/test_cli.c b/tests/test_cli.c index 09ff3ea06..be6120e61 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -519,6 +519,137 @@ TEST(cli_uninstall_removes_skills) { PASS(); } +/* ── uninstall command safety (#1038) ─────────────────────────── */ + +/* Test seam from cli.c: force prompt_yn() auto-answer (1=yes, -1=no, 0=prompt). */ +extern void cbm_set_auto_answer_for_test(int value); + +static bool test_path_exists(const char *path) { + struct stat st; + return stat(path, &st) == 0; +} + +/* Create a fake install under home: binary + Claude MCP config. + * Writes the binary path into bin_path. Returns 0 on success. */ +static int setup_fake_install(const char *home, char *bin_path, size_t bin_sz) { +#ifdef _WIN32 + snprintf(bin_path, bin_sz, "%s/.local/bin/codebase-memory-mcp.exe", home); +#else + snprintf(bin_path, bin_sz, "%s/.local/bin/codebase-memory-mcp", home); +#endif + if (th_write_file(bin_path, "fake-binary") != 0) + return -1; + char mcp[1024]; + snprintf(mcp, sizeof(mcp), "%s/.claude/.mcp.json", home); + return th_write_file(mcp, "{\"mcpServers\":{\"codebase-memory-mcp\":{\"command\":\"x\"}}}"); +} + +TEST(cli_uninstall_help_not_destructive) { + /* Repro of #1038: `uninstall --help` used to run a full uninstall. */ + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-unhelp-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + const char *raw = getenv("HOME"); + char *old_home = raw ? strdup(raw) : NULL; + cbm_setenv("HOME", tmpdir, 1); + + char bin_path[1024]; + int setup_rc = setup_fake_install(tmpdir, bin_path, sizeof(bin_path)); + + char help_flag[] = "--help"; + char *argv_help[] = {help_flag}; + int rc = cbm_cmd_uninstall(1, argv_help); + bool bin_kept = test_path_exists(bin_path); + + if (old_home) { + cbm_setenv("HOME", old_home, 1); + free(old_home); + } else + cbm_unsetenv("HOME"); + test_rmdir_r(tmpdir); + + ASSERT_EQ(setup_rc, 0); + ASSERT_EQ(rc, 0); + ASSERT(bin_kept); + PASS(); +} + +TEST(cli_uninstall_unknown_flag_rejected) { + /* A typo like `--dryrun` must error out, not run a real uninstall (#1038). */ + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-unflag-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + const char *raw = getenv("HOME"); + char *old_home = raw ? strdup(raw) : NULL; + cbm_setenv("HOME", tmpdir, 1); + + char bin_path[1024]; + int setup_rc = setup_fake_install(tmpdir, bin_path, sizeof(bin_path)); + + char typo_flag[] = "--dryrun"; + char *argv_typo[] = {typo_flag}; + int rc = cbm_cmd_uninstall(1, argv_typo); + bool bin_kept = test_path_exists(bin_path); + + if (old_home) { + cbm_setenv("HOME", old_home, 1); + free(old_home); + } else + cbm_unsetenv("HOME"); + test_rmdir_r(tmpdir); + + ASSERT_EQ(setup_rc, 0); + ASSERT_EQ(rc, 1); + ASSERT(bin_kept); + PASS(); +} + +TEST(cli_uninstall_confirmation_gate) { + /* Declined prompt aborts; -y proceeds and removes the binary (#1038). */ + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-unconf-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + const char *raw = getenv("HOME"); + char *old_home = raw ? strdup(raw) : NULL; + cbm_setenv("HOME", tmpdir, 1); + + char bin_path[1024]; + int setup_rc = setup_fake_install(tmpdir, bin_path, sizeof(bin_path)); + + /* Auto-answer "no": uninstall must cancel without touching files */ + cbm_set_auto_answer_for_test(-1); + int rc_no = cbm_cmd_uninstall(0, NULL); + bool bin_after_no = test_path_exists(bin_path); + + /* -y: proceeds and removes the fake binary */ + cbm_set_auto_answer_for_test(0); + char yes_flag[] = "-y"; + char *argv_yes[] = {yes_flag}; + int rc_yes = cbm_cmd_uninstall(1, argv_yes); + bool bin_after_yes = test_path_exists(bin_path); + cbm_set_auto_answer_for_test(0); + + if (old_home) { + cbm_setenv("HOME", old_home, 1); + free(old_home); + } else + cbm_unsetenv("HOME"); + test_rmdir_r(tmpdir); + + ASSERT_EQ(setup_rc, 0); + ASSERT_EQ(rc_no, 1); + ASSERT(bin_after_no); + ASSERT_EQ(rc_yes, 0); + ASSERT(!bin_after_yes); + PASS(); +} + TEST(cli_remove_old_monolithic_skill) { /* Port of TestRemoveOldMonolithicSkill */ char tmpdir[256]; @@ -3101,6 +3232,9 @@ SUITE(cli) { RUN_TEST(cli_skill_idempotent); RUN_TEST(cli_skill_force_overwrite); RUN_TEST(cli_uninstall_removes_skills); + RUN_TEST(cli_uninstall_help_not_destructive); + RUN_TEST(cli_uninstall_unknown_flag_rejected); + RUN_TEST(cli_uninstall_confirmation_gate); RUN_TEST(cli_remove_old_monolithic_skill); RUN_TEST(cli_skill_files_content); RUN_TEST(cli_codex_instructions);