From ed76cf8e88cfb1f26c82110e46a518c13577f788 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 28 Sep 2026 20:24:48 +0200 Subject: [PATCH] fix(cli): never stop a daemon whose cache is not confirmed as ours Why: the activation rendezvous is per OS account, so an install or update run under another HOME / CBM_CACHE_DIR (a sandbox, a second profile) meets the account's live daemon at the same endpoint. The daemon's activation-shutdown handler verifies only the requester's build, never its cache, so the CLI's scope decision is the only thing that tells "my daemon" from "someone else's". That decision drained the cohort unless the active cache was positively DIFFERENT: a busy session group (another activation or a session start held it, so the active identity was never read) and an unreadable active cache both fell through to "ours", printed "Stopping active CBM sessions" and shut the daemon and every session behind it down. Fix (user decision 2026-09-28, "unknown = foreign, keep it"): only a positive match may drain. cli_activation_resolve_scope returns CLI_ACTIVATION_SCOPE_ACTIVE only for an OK admission (exact identity, cache included, or an empty group) or for a conflict whose cache fingerprint equals this activation's. BUSY and a blank active cache now return the new CLI_ACTIVATION_SCOPE_UNCONFIRMED, handled like a foreign cohort: no lock is taken, no drain request is sent, the mutation proceeds, and the audit log records "quiesce_skipped: active cohort ownership unconfirmed (...)". A read-only status probe names the daemon that was left (pid, version) and the notice says how to stop it: close its sessions, then run `codebase-memory-mcp daemon stop`. The confirmed paths are unchanged: a normal self-upgrade still drains its own daemon, a clearly foreign cache is still left alone. Proof (product binaries; private CBM_RUNTIME_DIR; an own permanent test daemon started from the same build under a host HOME/cache; a sandbox HOME/cache on the same runtime dir; the cohort maintenance gate held by a second process, i.e. a busy session group): - origin/main: "Stopping active CBM sessions and operations for install...", the test daemon was stopped (daemon status: not running), and the install still failed (rc=1 after 23 s, "could not be stopped safely"). - this change: "Leaving the running CBM daemon untouched (pid 76713, version dev): this install could not confirm that it serves the cache this install targets (the session group was busy, so its cache identity could not be read)." The test daemon kept running with the same pid and the install completed (rc=0, 10 s). A lifetime record whose cache field is zeroed is refused (UNSAFE) by both builds with the daemon kept; that path never drained and is unchanged. Tests (cli suite; fixture daemon in a private runtime dir; sandbox HOME): - cli_install_foreign_home_unreadable_cohort_cache_keeps_host_daemon (the new seam cbm_cli_set_activation_scope_cache_unreadable_for_test blanks the observed active cache fingerprint) - cli_install_foreign_home_busy_cohort_keeps_host_daemon (the maintenance gate is held through the product's own lock-directory handle) Both were RED 3x on origin/main for the right reason (the host daemon was drained: ASSERT(host_serving)), are green with the fix, and turn RED again when the fix is reverted. Controls stay green: cli_install_into_host_namespace_still_drains_host_cohort (confirmed same cache: drained, as before) and cli_install_{skip_binary,binary}_into_foreign_home_never_drains_host_cohort (clearly different cache: kept, as before). Signed-off-by: Martin Vogel --- src/cli/cli.c | 72 ++++++++++++++++++++----- src/cli/cli.h | 6 +++ tests/test_cli.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+), 12 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index b868fdca2..4538080eb 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -226,6 +226,7 @@ typedef struct { static cbm_cli_activation_ops_t g_cli_activation_test_ops; static bool g_cli_activation_test_ops_set = false; static const char *g_cli_activation_runtime_parent_for_test = NULL; +static bool g_cli_activation_scope_cache_unreadable_for_test = false; static void cli_activation_diagnostic(const cbm_cli_activation_ops_t *ops, const char *message) { const char *diagnostic = message ? message : CLI_ACTIVATION_REFUSED_MESSAGE; @@ -335,6 +336,10 @@ const char *cbm_cli_activation_runtime_parent_for_test(void) { return g_cli_activation_runtime_parent_for_test; } +void cbm_cli_set_activation_scope_cache_unreadable_for_test(bool unreadable) { + g_cli_activation_scope_cache_unreadable_for_test = unreadable; +} + static const char *cli_activation_action_text(cbm_daemon_runtime_activation_action_t action) { switch (action) { case CBM_DAEMON_RUNTIME_ACTIVATION_INSTALL: @@ -541,6 +546,7 @@ typedef enum { CLI_ACTIVATION_SCOPE_ACTIVE = 0, CLI_ACTIVATION_SCOPE_NOTHING_TO_REPLACE, CLI_ACTIVATION_SCOPE_FOREIGN_COHORT, + CLI_ACTIVATION_SCOPE_UNCONFIRMED, CLI_ACTIVATION_SCOPE_ERROR, } cli_activation_scope_t; @@ -566,9 +572,43 @@ static void cli_activation_log_guard_decision(const cli_activation_production_co * anything at all (a --skip-binary install without an index reset publishes * nothing), and whose cohort is active. Admission with an immediate deadline * is the cheapest authoritative read of the active lifetime record: OK means - * the cohort is ours or empty, CONFLICT names the active identity (its cache - * fingerprint is filled for every conflict kind), BUSY means another - * activation holds maintenance and the barrier waits for it as before. */ + * the cohort shares this exact identity (cache included) or is empty, + * CONFLICT names the active identity (its cache fingerprint is filled for + * every conflict kind), BUSY means another activation or a session start held + * the group, so the active identity was never read. + * + * Only a POSITIVE match may drain (user decision 2026-09-28, "unknown = + * foreign, keep it"): the daemon's activation-shutdown handler verifies the + * requester's build, never its cache, so this decision is the only thing + * that keeps another HOME's install/update from stopping the account's live + * daemon. A busy group or an unreadable active cache used to fall through to + * "ours" and stop every session behind that daemon. */ +static cli_activation_scope_t cli_activation_scope_unconfirmed( + cli_activation_production_context_t *context, const char *scope, const char *reason) { + const char *action = cli_activation_action_text(context->action); + (void)snprintf(context->scope_detail, sizeof(context->scope_detail), + "active cohort ownership unconfirmed (%s); scope=%s; nothing stopped", reason, + scope); + cli_activation_log_guard_decision(context, "unconfirmed_cohort", NULL); + /* Name the daemon being left: a read-only status probe (no cohort, no + * HELLO, no admission) that works across build skew. */ + cbm_daemon_runtime_status_t daemon; + if (cbm_daemon_runtime_request_status(context->endpoint, &context->identity, + CLI_ACTIVATION_CONTROL_TIMEOUT_MS, &daemon)) { + printf("Leaving the running CBM daemon untouched (pid %lu, version %s): this %s could " + "not confirm that it serves the cache this %s targets (%s).\n", + (unsigned long)daemon.daemon_pid, daemon.semantic_version, action, action, reason); + } else { + printf("Leaving active CBM sessions untouched: this %s could not confirm that they " + "serve the cache it targets (%s).\n", + action, reason); + } + printf("If it is yours, close its CBM sessions and run `codebase-memory-mcp daemon stop` " + "to stop it yourself.\n"); + (void)fflush(stdout); + return CLI_ACTIVATION_SCOPE_UNCONFIRMED; +} + static cli_activation_scope_t cli_activation_resolve_scope( cli_activation_production_context_t *context) { const char *runtime_dir = cbm_daemon_ipc_endpoint_runtime_dir(context->endpoint); @@ -594,20 +634,27 @@ static cli_activation_scope_t cli_activation_resolve_scope( if (lease) { return CLI_ACTIVATION_SCOPE_ERROR; } + if (g_cli_activation_scope_cache_unreadable_for_test) { + conflict.active_cache_fingerprint[0] = '\0'; + } switch (status) { case CBM_VERSION_COHORT_OK: - case CBM_VERSION_COHORT_BUSY: return CLI_ACTIVATION_SCOPE_ACTIVE; + case CBM_VERSION_COHORT_BUSY: + return cli_activation_scope_unconfirmed( + context, scope, "the session group was busy, so its cache identity could not be read"); case CBM_VERSION_COHORT_CONFLICT: break; default: return CLI_ACTIVATION_SCOPE_ERROR; } - if (!conflict.active_cache_fingerprint[0] || - strcmp(conflict.active_cache_fingerprint, context->cache_fingerprint) == 0) { - /* Same cache namespace, another build or version: that IS the daemon - * this activation replaces. An unreadable active cache stays in scope - * rather than silently exempting a same-namespace daemon. */ + if (!conflict.active_cache_fingerprint[0]) { + return cli_activation_scope_unconfirmed(context, scope, + "its cache identity could not be read"); + } + if (strcmp(conflict.active_cache_fingerprint, context->cache_fingerprint) == 0) { + /* Positively confirmed: same cache namespace, another build or + * version. That IS the daemon this activation replaces. */ return CLI_ACTIVATION_SCOPE_ACTIVE; } (void)snprintf(context->scope_detail, sizeof(context->scope_detail), @@ -636,9 +683,10 @@ static int cli_activation_production_reserve(void *opaque, cbm_cli_activation_lo } if (scope != CLI_ACTIVATION_SCOPE_ACTIVE) { /* Nothing in the target namespace is being replaced, or the only - * active cohort serves another namespace: hold no maintenance, - * admission, lifetime or startup lock (each wakes or blocks the other - * namespace's sessions) and send no drain request. */ + * active cohort serves another namespace, or its ownership could not + * be confirmed: hold no maintenance, admission, lifetime or startup + * lock (each wakes or blocks the other namespace's sessions) and + * send no drain request. */ if (!cli_activation_log_event(context, "quiesce_skipped", context->scope_detail)) { return CLI_ERR; } diff --git a/src/cli/cli.h b/src/cli/cli.h index 5522edef0..2f6a4e7df 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -504,6 +504,12 @@ void cbm_cli_set_activation_ops_for_test(const cbm_cli_activation_ops_t *ops); void cbm_cli_set_activation_runtime_parent_for_test(const char *runtime_parent); const char *cbm_cli_activation_runtime_parent_for_test(void); +/* Internal integration-test seam: the activation scope read reports the active + * cohort's cache fingerprint as unreadable (blank), exactly what the scope + * decision sees when that field cannot be recovered. false restores the real + * read. Not a command-line or environment override. */ +void cbm_cli_set_activation_scope_cache_unreadable_for_test(bool unreadable); + /* ── Subcommands (wired from main.c) ─────────────────────────── */ /* install: copy binary, install skills, install editor MCP configs, ensure PATH. diff --git a/tests/test_cli.c b/tests/test_cli.c index 4ec96be53..a52633014 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -18,11 +18,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -2037,6 +2039,139 @@ TEST(cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing) { ASSERT_TRUE(nothing_drained); PASS(); } + +/* cli_scope_install with the activation's stdout captured into out. */ +static int cli_scope_install_captured(cli_scope_fixture_t *fixture, const char *home, + const char *cache, const char *bin_dir, bool skip_binary, + char *out, size_t out_size) { + out[0] = '\0'; + FILE *capture = tmpfile(); + int saved_stdout = capture ? dup(STDOUT_FILENO) : -1; + bool redirected = false; + if (capture && saved_stdout >= 0) { + fflush(stdout); + redirected = dup2(fileno(capture), STDOUT_FILENO) >= 0; + } + int rc = redirected ? cli_scope_install(fixture, home, cache, bin_dir, skip_binary) : -1; + if (redirected) { + fflush(stdout); + (void)dup2(saved_stdout, STDOUT_FILENO); + } + if (saved_stdout >= 0) { + close(saved_stdout); + } + if (capture) { + rewind(capture); + size_t count = fread(out, 1, out_size - 1U, capture); + out[count] = '\0'; + fclose(capture); + } + return rc; +} + +/* The notice an activation prints when it leaves a daemon it could not + * confirm as its own: names the daemon by pid and tells the user how to stop + * it themselves. Never the "Stopping ..." banner of a drain. */ +static bool cli_scope_kept_notice_names_host(const char *output, pid_t host) { + char pid_text[48]; + snprintf(pid_text, sizeof(pid_text), "(pid %ld, version %s)", (long)host, CBM_VERSION); + return strstr(output, "Leaving the running CBM daemon untouched") != NULL && + strstr(output, pid_text) != NULL && + strstr(output, "codebase-memory-mcp daemon stop") != NULL && + strstr(output, "Stopping active CBM sessions") == NULL; +} + +/* (e) USER DECISION 2026-09-28, "unknown = foreign, keep it": the scope read + * reaches the active cohort but cannot read its cache identity. Ownership is + * unconfirmed, so the sandbox install must leave the host daemon serving, + * stop no session, and say which daemon it left and how to stop it. The + * install publishes a binary (no --skip-binary): an activation that replaces + * nothing never consults the cohort at all (test d). */ +TEST(cli_install_foreign_home_unreadable_cohort_cache_keeps_host_daemon) { + cli_scope_fixture_t fixture; + bool ready = cli_scope_fixture_start(&fixture, "unread"); + char foreign_home[512]; + char foreign_cache[576]; + char foreign_bin[640]; + char activation_log[704]; + cli_scope_foreign_paths(&fixture, foreign_home, foreign_cache, foreign_bin, activation_log); + bool prepared = ready && cbm_mkdir_p(foreign_home, 0700); + char output[16384]; + output[0] = '\0'; + cbm_cli_set_activation_scope_cache_unreadable_for_test(true); + int install_rc = prepared + ? cli_scope_install_captured(&fixture, foreign_home, foreign_cache, + foreign_bin, false, output, sizeof(output)) + : -1; + cbm_cli_set_activation_scope_cache_unreadable_for_test(false); + bool host_serving = prepared && cli_scope_host_serving(&fixture); + bool notice = cli_scope_kept_notice_names_host(output, fixture.host); + const char *events = read_test_file(activation_log); + bool completed = events && strstr(events, "\"phase\":\"completed\"") != NULL; + bool nothing_drained = events && strstr(events, "cohort drained") == NULL; + int host_exit = cli_scope_fixture_finish(&fixture); + + ASSERT_TRUE(ready); + ASSERT_TRUE(host_serving); + ASSERT_EQ(host_exit, 0); + ASSERT_TRUE(nothing_drained); + ASSERT_TRUE(notice); + ASSERT_EQ(install_rc, 0); + ASSERT_TRUE(completed); + PASS(); +} + +/* (f) Same decision, busy group: another activation holds the cohort + * maintenance gate, so the scope read cannot reach the active identity at + * all. Unconfirmed ownership keeps the host daemon; the sandbox install + * completes without draining anyone. The gate is held through the product's + * own lock-directory handle on the name version_cohort.c uses. */ +TEST(cli_install_foreign_home_busy_cohort_keeps_host_daemon) { + cli_scope_fixture_t fixture; + bool ready = cli_scope_fixture_start(&fixture, "busy"); + char foreign_home[512]; + char foreign_cache[576]; + char foreign_bin[640]; + char activation_log[704]; + cli_scope_foreign_paths(&fixture, foreign_home, foreign_cache, foreign_bin, activation_log); + cbm_private_lock_directory_t *lock_directory = NULL; + cbm_private_file_lock_t *maintenance = NULL; + bool held = ready && + cbm_daemon_ipc_private_lock_directory_new(fixture.endpoint, &lock_directory) == + CBM_PRIVATE_FILE_LOCK_OK && + cbm_private_file_lock_try_acquire( + lock_directory, "cbm-version-cohort-maintenance-v1.lock", + CBM_PRIVATE_FILE_LOCK_EX, &maintenance) == CBM_PRIVATE_FILE_LOCK_OK; + bool prepared = held && cbm_mkdir_p(foreign_home, 0700); + char output[16384]; + output[0] = '\0'; + int install_rc = prepared + ? cli_scope_install_captured(&fixture, foreign_home, foreign_cache, + foreign_bin, false, output, sizeof(output)) + : -1; + bool host_serving = prepared && cli_scope_host_serving(&fixture); + bool released = + maintenance && cbm_private_file_lock_release(&maintenance) == CBM_PRIVATE_FILE_LOCK_OK; + if (lock_directory) { + cbm_private_lock_directory_close(lock_directory); + } + bool notice = cli_scope_kept_notice_names_host(output, fixture.host); + const char *events = read_test_file(activation_log); + bool completed = events && strstr(events, "\"phase\":\"completed\"") != NULL; + bool nothing_drained = events && strstr(events, "cohort drained") == NULL; + int host_exit = cli_scope_fixture_finish(&fixture); + + ASSERT_TRUE(ready); + ASSERT_TRUE(held); + ASSERT_TRUE(host_serving); + ASSERT_EQ(host_exit, 0); + ASSERT_TRUE(nothing_drained); + ASSERT_TRUE(notice); + ASSERT_EQ(install_rc, 0); + ASSERT_TRUE(completed); + ASSERT_TRUE(released); + PASS(); +} #endif TEST(cli_install_force_quiesces_active_cohort_before_replacing_binary) { @@ -16259,6 +16394,8 @@ SUITE(cli) { RUN_TEST(cli_install_binary_into_foreign_home_never_drains_host_cohort); RUN_TEST(cli_install_into_host_namespace_still_drains_host_cohort); RUN_TEST(cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing); + RUN_TEST(cli_install_foreign_home_unreadable_cohort_cache_keeps_host_daemon); + RUN_TEST(cli_install_foreign_home_busy_cohort_keeps_host_daemon); #endif RUN_TEST(cli_install_force_quiesces_active_cohort_before_replacing_binary); RUN_TEST(cli_install_dir_and_skip_config_stage_first_install_safely);