From 013e1f7ec7d4b981a1cd28bf35d5767c195f8447 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 8 Sep 2026 20:28:58 +0200 Subject: [PATCH 1/2] fix(cli): scope the activation guard to the install-target HOME (amendment) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rendezvous directory the CLI's activation guard talks to is per OS account (service.h), not per HOME/CBM_CACHE_DIR: every daemon owned by one user meets at the same endpoint, and only the cache fingerprint carried in the cohort identity separates one HOME/CBM_CACHE_DIR namespace from another. `cli_activation_production_reserve` used `cbm_version_cohort_reserve_for_mutation`, which treats any lifetime- lock holder at that shared endpoint as the daemon this activation must drain, with no cache-fingerprint awareness at all. An `install` run against a second HOME (a sandbox, a second profile, `--skip-binary` into an alternate CBM_CACHE_DIR) therefore reached the LIVE host daemon at the shared endpoint and drained it — disconnecting every one of the host's real MCP clients — even though nothing the sandboxed install touched belonged to the host's cache namespace. The fix adds `cli_activation_resolve_scope`, a non-blocking probe via the already cache-fingerprint-aware `cbm_version_cohort_acquire`, run before the blind drain path. It distinguishes three cases from the resolved target of THIS activation: - nothing to quiesce at all: a `--skip-binary` install that also resets no index publishes nothing, so no session anywhere needs to stop (`quiesce_required` on `cli_activation_guard_scoped`, plumbed from `cbm_cmd_install`'s own binary/index-reset decision); - the active cohort's cache fingerprint doesn't match this activation's target cache: a foreign namespace, left untouched; - same cache fingerprint (or no active cohort): this activation really does replace that daemon, so the existing drain/quiesce path runs unchanged, including the barrier wait when another activation already holds maintenance. Verified against a real forked daemon (a live runtime service with one committed client) standing in for the host: an install into a foreign HOME/CBM_CACHE_DIR (`--skip-binary` and a full binary install) leaves that daemon serving with its client still attached, while an install that targets the host's own namespace still drains it and the activation audit still names the client it disconnected. New/updated tests in tests/test_cli.c (cli suite): - cli_install_skip_binary_into_foreign_home_never_drains_host_cohort - cli_install_binary_into_foreign_home_never_drains_host_cohort - cli_install_into_host_namespace_still_drains_host_cohort (regression) - cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing - cli_activation_quiesce_does_not_wait_on_bootstrap_startup updated to carry a real cache fingerprint for its participant, now that scope is fingerprint-gated src/mcp/index_supervisor.{c,h}: CBM_CLI_ENABLE_TEST_API-gated setter so the fixture's forked daemon can carry the same build fingerprint the guard captures from the supervisor. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Martin Vogel --- src/cli/cli.c | 163 +++++++++++++-- src/mcp/index_supervisor.c | 10 + src/mcp/index_supervisor.h | 10 + tests/test_cli.c | 407 ++++++++++++++++++++++++++++++++++++- 4 files changed, 576 insertions(+), 14 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index d85ebf0b6..6cf9ddca8 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -214,6 +214,12 @@ typedef struct { bool cleanup_ok; bool original_cache_environment_present; bool cache_environment_overridden; + /* Scope: what THIS activation replaces decides whether any session must + * be quiesced at all, and the target cache namespace decides WHICH + * cohort. A skipped coordination holds no lock and drains nobody. */ + bool quiesce_required; + bool coordination_skipped; + char scope_detail[CBM_SZ_1K]; } cli_activation_production_context_t; static cbm_cli_activation_ops_t g_cli_activation_test_ops; @@ -528,6 +534,91 @@ static void cli_activation_release_cleanup_lease(cli_activation_production_conte } } +typedef enum { + CLI_ACTIVATION_SCOPE_ACTIVE = 0, + CLI_ACTIVATION_SCOPE_NOTHING_TO_REPLACE, + CLI_ACTIVATION_SCOPE_FOREIGN_COHORT, + CLI_ACTIVATION_SCOPE_ERROR, +} cli_activation_scope_t; + +static void cli_activation_log_guard_decision(const cli_activation_production_context_t *context, + const char *decision, const char *active_cache) { + char clients[32]; + (void)snprintf(clients, sizeof(clients), "%llu", + (unsigned long long)context->daemon_result.active_clients); + const char *scope = cbm_daemon_ipc_endpoint_runtime_dir(context->endpoint); + cbm_log_info("activation.guard", "scope", scope ? scope : "", "cache", + context->cache_fingerprint, "clients", clients, "decision", decision, + "active_cache", active_cache ? active_cache : ""); +} + +/* Whose sessions does this activation have to stop? The rendezvous directory + * is per OS account (service.h), so the host daemon of another HOME / + * CBM_CACHE_DIR meets this activation at the very same endpoint; only the + * cache fingerprint in the cohort identity separates the namespaces. An + * `install` into a sandbox HOME used to drain the host daemon and every MCP + * client behind it although nothing it touched belonged to the host. + * + * Two questions, answered from the activation's own target: does it replace + * 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. */ +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); + const char *scope = runtime_dir ? runtime_dir : ""; + const char *action = cli_activation_action_text(context->action); + if (!context->quiesce_required) { + (void)snprintf(context->scope_detail, sizeof(context->scope_detail), + "nothing to quiesce: published binary and indexes unchanged; scope=%s", + scope); + cli_activation_log_guard_decision(context, "nothing_to_replace", NULL); + printf("No CBM session needs to stop for %s: the published binary and indexes are " + "unchanged.\n", + action); + (void)fflush(stdout); + return CLI_ACTIVATION_SCOPE_NOTHING_TO_REPLACE; + } + cbm_version_cohort_lease_t *lease = NULL; + cbm_daemon_conflict_t conflict; + memset(&conflict, 0, sizeof(conflict)); + cbm_version_cohort_status_t status = cbm_version_cohort_acquire( + context->cohort_manager, &context->identity, cbm_now_ms(), &lease, &conflict); + cli_activation_release_cleanup_lease(context, &lease); + if (lease) { + return CLI_ACTIVATION_SCOPE_ERROR; + } + switch (status) { + case CBM_VERSION_COHORT_OK: + case CBM_VERSION_COHORT_BUSY: + return CLI_ACTIVATION_SCOPE_ACTIVE; + 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. */ + return CLI_ACTIVATION_SCOPE_ACTIVE; + } + (void)snprintf(context->scope_detail, sizeof(context->scope_detail), + "active cohort serves another cache namespace (%.12s), this %s targets " + "%.12s; scope=%s; nothing stopped", + conflict.active_cache_fingerprint, action, context->cache_fingerprint, scope); + cli_activation_log_guard_decision(context, "foreign_cohort", conflict.active_cache_fingerprint); + printf("Leaving active CBM sessions untouched: they serve another cache namespace than " + "this %s targets.\n", + action); + (void)fflush(stdout); + return CLI_ACTIVATION_SCOPE_FOREIGN_COHORT; +} + static int cli_activation_production_reserve(void *opaque, cbm_cli_activation_lock_t *lease_out) { cli_activation_production_context_t *context = opaque; if (lease_out) { @@ -536,6 +627,26 @@ static int cli_activation_production_reserve(void *opaque, cbm_cli_activation_lo if (!context || !context->cohort_manager || !lease_out) { return CLI_ERR; } + cli_activation_scope_t scope = cli_activation_resolve_scope(context); + if (scope == CLI_ACTIVATION_SCOPE_ERROR) { + return CLI_ERR; + } + 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. */ + if (!cli_activation_log_event(context, "quiesce_skipped", context->scope_detail)) { + return CLI_ERR; + } + context->coordination_skipped = true; + context->mutation_authorized = true; + *lease_out = context; + return 1; + } + printf("Stopping active CBM sessions and operations for %s...\n", + cli_activation_action_text(context->action)); + (void)fflush(stdout); cbm_version_cohort_quiesce_result_t quiesce = CBM_VERSION_COHORT_QUIESCE_NOT_NEEDED; cbm_version_cohort_lease_t *lease = NULL; context->control_deadline_ms = cli_activation_deadline_after(CLI_ACTIVATION_CONTROL_TIMEOUT_MS); @@ -582,6 +693,8 @@ static int cli_activation_production_reserve(void *opaque, cbm_cli_activation_lo } context->cohort_lease = lease; + cli_activation_log_guard_decision( + context, context->shutdown_requested ? "cohort_drained" : "no_active_cohort", NULL); if (!cli_activation_log_event(context, "daemon_stopped", context->shutdown_requested ? "cohort drained" : "no active cohort")) { @@ -600,6 +713,13 @@ static void cli_activation_production_release(void *opaque, cbm_cli_activation_l if (!context) { return; } + if (context->coordination_skipped) { + /* Nothing was held: the token is the context itself. */ + if (lease != (cbm_cli_activation_lock_t)context) { + context->cleanup_ok = false; + } + return; + } /* Global release order is the inverse of acquisition: startup first, * then lifetime/admission/maintenance through the cohort lease. */ if (context->startup_lock) { @@ -626,9 +746,11 @@ static void cli_activation_production_diagnostic(void *opaque, const char *messa static bool cli_activation_production_context_init(cli_activation_production_context_t *context, cbm_daemon_runtime_activation_action_t action, const char *target_version, - const char *target_build) { + const char *target_build, + bool quiesce_required) { memset(context, 0, sizeof(*context)); context->action = action; + context->quiesce_required = quiesce_required; context->target_version = target_version; context->target_build = target_build; context->cleanup_ok = true; @@ -735,23 +857,26 @@ static void cli_activation_production_context_close(cli_activation_production_co context->original_cache_environment = NULL; } -static int cli_activation_guard(cbm_daemon_runtime_activation_action_t action, - const char *target_version, const char *target_build, - cbm_cli_activation_mutation_fn mutation, void *mutation_context) { +/* quiesce_required: false when the activation publishes no binary and resets + * no index (a config-only install) — nothing running is then replaced, and + * no session is stopped for it. */ +static int cli_activation_guard_scoped(cbm_daemon_runtime_activation_action_t action, + const char *target_version, const char *target_build, + bool quiesce_required, + cbm_cli_activation_mutation_fn mutation, + void *mutation_context) { if (g_cli_activation_test_ops_set) { return cbm_cli_activation_guard_with_ops(&g_cli_activation_test_ops, mutation, mutation_context); } cli_activation_production_context_t context; - if (!cli_activation_production_context_init(&context, action, target_version, target_build)) { + if (!cli_activation_production_context_init(&context, action, target_version, target_build, + quiesce_required)) { cli_activation_production_context_close(&context); cli_activation_production_diagnostic(NULL, CLI_ACTIVATION_REFUSED_MESSAGE); return CLI_TRUE; } - printf("Stopping active CBM sessions and operations for %s...\n", - cli_activation_action_text(action)); - (void)fflush(stdout); if (!cli_activation_log_event(&context, "requested", NULL)) { cli_activation_production_context_close(&context); (void)fprintf(stderr, "error: activation request could not be recorded safely; " @@ -793,6 +918,13 @@ static int cli_activation_guard(cbm_daemon_runtime_activation_action_t action, return rc; } +static int cli_activation_guard(cbm_daemon_runtime_activation_action_t action, + const char *target_version, const char *target_build, + cbm_cli_activation_mutation_fn mutation, void *mutation_context) { + return cli_activation_guard_scoped(action, target_version, target_build, true, mutation, + mutation_context); +} + /* Tar header field offsets */ #define TAR_NAME_LEN 101 /* filename field: bytes 0-99 + NUL */ #define TAR_SIZE_OFFSET 124 /* octal size field offset */ @@ -10756,11 +10888,16 @@ int cbm_cmd_install(int argc, char **argv) { .force = force, .dry_run = dry_run, }; - int activation_rc = - dry_run ? cli_install_activate(&activation) - : cli_activation_guard(CBM_DAEMON_RUNTIME_ACTIVATION_INSTALL, CBM_VERSION, - has_binary_validator ? binary_validator.fingerprint : NULL, - cli_install_activate, &activation); + /* What this install replaces decides whether any session must stop: with + * the published binary untouched (--skip-binary, or an externally managed + * binary) and no index reset, agent configs are refreshed while every + * session stays up. */ + bool quiesce_required = has_binary_validator || delete_indexes; + int activation_rc = dry_run ? cli_install_activate(&activation) + : cli_activation_guard_scoped( + CBM_DAEMON_RUNTIME_ACTIVATION_INSTALL, CBM_VERSION, + has_binary_validator ? binary_validator.fingerprint : NULL, + quiesce_required, cli_install_activate, &activation); if (activation.binary_transaction) { (void)cli_activation_transaction_abort(&activation.binary_transaction); } diff --git a/src/mcp/index_supervisor.c b/src/mcp/index_supervisor.c index eed78f418..a035e3d6d 100644 --- a/src/mcp/index_supervisor.c +++ b/src/mcp/index_supervisor.c @@ -149,6 +149,16 @@ const char *cbm_index_supervisor_build_fingerprint(void) { return g_build_fingerprint[0] ? g_build_fingerprint : NULL; } +#if defined(CBM_CLI_ENABLE_TEST_API) +void cbm_index_supervisor_set_build_fingerprint_for_test(const char *fingerprint) { + if (!fingerprint || !worker_fingerprint_valid(fingerprint)) { + return; + } + g_build_fingerprint_capture_attempted = true; + (void)snprintf(g_build_fingerprint, sizeof(g_build_fingerprint), "%s", fingerprint); +} +#endif + static bool worker_fingerprint_valid(const char *fingerprint) { if (!fingerprint || strlen(fingerprint) != CBM_INDEX_WORKER_BUILD_FINGERPRINT_LENGTH) { return false; diff --git a/src/mcp/index_supervisor.h b/src/mcp/index_supervisor.h index 31259df90..cbc38f696 100644 --- a/src/mcp/index_supervisor.h +++ b/src/mcp/index_supervisor.h @@ -76,6 +76,16 @@ void cbm_index_worker_log_begin(const char *args_json, const char *repo_path); bool cbm_index_supervisor_capture_build_fingerprint(void); const char *cbm_index_supervisor_build_fingerprint(void); +#if defined(CBM_CLI_ENABLE_TEST_API) +/* Test-only: the runner captures a synthetic stub (see the capture seam) so + * spawned workers start instantly, but a daemon runtime service can only carry + * the REAL image hash. A fixture that runs a real daemon and drives the + * production activation guard against it aligns the captured value with that + * hash for its duration and restores the previous value afterwards. Invalid + * input is ignored. Never compiled into production. */ +void cbm_index_supervisor_set_build_fingerprint_for_test(const char *fingerprint); +#endif + typedef struct { const char *expected_build_fingerprint; const char *args_json; diff --git a/tests/test_cli.c b/tests/test_cli.c index 976280868..3a4b4a851 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -23,7 +23,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -39,6 +41,7 @@ #include #include #include +#include #endif #ifdef __APPLE__ #include @@ -47,6 +50,12 @@ #include #include +/* Same guarded fallback every product TU carries; CI injects the real value + * through CFLAGS_EXTRA for product objects and test objects alike. */ +#ifndef CBM_VERSION +#define CBM_VERSION "dev" +#endif + /* Internal prompt seam used to restore process-global state after command * tests that exercise --yes. */ void cbm_set_auto_answer_for_test(int value); @@ -1216,6 +1225,23 @@ TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup) { test_rmdir_r(tmpdir); FAIL("runtime parent setup failed"); } + /* The participant models the daemon of THIS namespace: its cohort + * identity carries the fingerprint of the cache the install below + * targets, derived as main.c derives it. A foreign cache would (rightly) + * be left alone by the scoped guard. */ + char participant_cache[512]; + char participant_cache_canonical[1024]; + char participant_cache_fingerprint[CBM_SHA256_HEX_LEN + 1] = {0}; + snprintf(participant_cache, sizeof(participant_cache), "%s/cache", tmpdir); + if (!cbm_mkdir_p(participant_cache, 0700) || + !cbm_canonical_path(participant_cache, participant_cache_canonical, + sizeof(participant_cache_canonical))) { + test_rmdir_r(tmpdir); + FAIL("participant cache setup failed"); + } + cbm_normalize_path_sep(participant_cache_canonical); + cbm_sha256_hex(participant_cache_canonical, strlen(participant_cache_canonical), + participant_cache_fingerprint); int ready_pipe[2] = {-1, -1}; if (pipe(ready_pipe) != 0) { test_rmdir_r(tmpdir); @@ -1231,7 +1257,7 @@ TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup) { cbm_daemon_build_identity_t identity = { .semantic_version = "cli-activation-test", .build_fingerprint = fingerprint, - .cache_fingerprint = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + .cache_fingerprint = participant_cache_fingerprint, .protocol_abi = CBM_DAEMON_RUNTIME_WIRE_ABI, .store_abi = 1, .feature_abi = 1, @@ -1467,6 +1493,381 @@ TEST(cli_install_recovers_markerless_stale_rendezvous) { ASSERT_TRUE(anchor_removed); PASS(); } +/* ── Activation-guard namespace scope (2026-09-08) ────────────────── + * The rendezvous directory is per OS ACCOUNT, never per HOME (service.h): + * every daemon of one user meets at one endpoint, and the cohort identity's + * cache fingerprint is what separates one HOME / CBM_CACHE_DIR namespace + * from another. An `install` into a second HOME (a sandbox, a second + * profile) therefore reaches the LIVE host daemon at the shared endpoint — + * and used to drain it, disconnecting every MCP client, although nothing + * that install touched belonged to the host namespace. + * + * The fixture models the host the way host.c builds it: a forked process + * that admits itself to the cohort with the host cache fingerprint, runs a + * real runtime service at the shared endpoint, and exits (releasing the + * cohort lease LAST, the real teardown order) once that service has been + * drained. The parent keeps one committed client connected to it across the + * install under test and asks the daemon itself afterwards. */ +#define CLI_SCOPE_HOST_DRAINED 3 +#define CLI_SCOPE_TIMEOUT_MS 5000U + +typedef struct { + char tmpdir[256]; + char runtime_parent[512]; + char host_home[512]; + char host_cache[512]; + char host_cache_fingerprint[CBM_SHA256_HEX_LEN + 1]; + char self_build[CBM_DAEMON_BUILD_FINGERPRINT_SIZE]; + char previous_supervisor_build[CBM_DAEMON_BUILD_FINGERPRINT_SIZE]; + char conflict_log[640]; + cbm_daemon_build_identity_t identity; + cbm_daemon_ipc_endpoint_t *endpoint; + cbm_daemon_runtime_client_t *client; + pid_t host; + int release_fd; + char *old_home; + char *old_cache; + char *old_shell; +} cli_scope_fixture_t; + +static _Noreturn void cli_scope_host_child(const cli_scope_fixture_t *fixture, int ready_fd, + int release_fd) { + cbm_daemon_ipc_endpoint_t *endpoint = + cbm_daemon_bootstrap_endpoint_new(fixture->runtime_parent); + cbm_version_cohort_manager_t *manager = + endpoint ? cbm_version_cohort_manager_new(endpoint) : NULL; + cbm_version_cohort_lease_t *lease = NULL; + cbm_daemon_conflict_t conflict; + cbm_daemon_runtime_service_t *service = NULL; + bool admitted = endpoint && manager && + cbm_version_cohort_acquire(manager, &fixture->identity, cbm_now_ms() + 45000U, + &lease, &conflict) == CBM_VERSION_COHORT_OK; + if (admitted) { + cbm_daemon_runtime_service_config_t config = { + .endpoint = endpoint, + .identity = fixture->identity, + .conflict_log_path = fixture->conflict_log, + .conflict_log_cap_bytes = 64U * 1024U, + .max_clients = 8, + .lease_timeout_ms = CLI_SCOPE_TIMEOUT_MS, + .request_timeout_ms = CLI_SCOPE_TIMEOUT_MS, + .shutdown_timeout_ms = CLI_SCOPE_TIMEOUT_MS, + /* Born permanent: only a drain/stop ends it, never the parent's + * client leaving, so "still alive" is a statement about the + * drain alone. */ + .permanent = true, + }; + service = cbm_daemon_runtime_service_start(&config); + } + bool ready_ok = + service && cbm_daemon_runtime_service_state(service) == CBM_DAEMON_RUNTIME_SERVICE_RUNNING; + char ready = ready_ok ? 'R' : 'E'; + (void)write(ready_fd, &ready, 1); + close(ready_fd); + bool drained = false; + uint64_t deadline = cbm_now_ms() + 120000U; + while (ready_ok && cbm_now_ms() < deadline) { + if (cbm_daemon_runtime_service_wait_exited(service, 50U)) { + drained = true; + break; + } + struct pollfd release_poll = {.fd = release_fd, .events = POLLIN, .revents = 0}; + if (poll(&release_poll, 1, 0) > 0) { + break; + } + } + uint64_t cleanup_deadline = cbm_now_ms() + 2U * CLI_SCOPE_TIMEOUT_MS; + if (service) { + if (!drained) { + (void)cbm_daemon_runtime_service_stop(service, CLI_SCOPE_TIMEOUT_MS); + } + while (!cbm_daemon_runtime_service_free(service) && cbm_now_ms() < cleanup_deadline) { + cbm_usleep(1000); + } + } + while (lease && cbm_version_cohort_lease_release(&lease) != CBM_PRIVATE_FILE_LOCK_OK && + cbm_now_ms() < cleanup_deadline) { + cbm_usleep(1000); + } + while (manager && cbm_version_cohort_manager_free(&manager) != CBM_PRIVATE_FILE_LOCK_OK && + cbm_now_ms() < cleanup_deadline) { + cbm_usleep(1000); + } + cbm_daemon_ipc_endpoint_free(endpoint); + close(release_fd); + _exit(!ready_ok ? 1 : drained ? CLI_SCOPE_HOST_DRAINED : 0); +} + +static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *tag) { + memset(fixture, 0, sizeof(*fixture)); + fixture->host = -1; + fixture->release_fd = -1; + snprintf(fixture->tmpdir, sizeof(fixture->tmpdir), "/tmp/cli-guard-scope-%s-XXXXXX", tag); + if (!cbm_mkdtemp(fixture->tmpdir)) { + return false; + } + snprintf(fixture->runtime_parent, sizeof(fixture->runtime_parent), "%s/runtime", + fixture->tmpdir); + snprintf(fixture->host_home, sizeof(fixture->host_home), "%s/host", fixture->tmpdir); + snprintf(fixture->host_cache, sizeof(fixture->host_cache), "%s/cache", fixture->host_home); + snprintf(fixture->conflict_log, sizeof(fixture->conflict_log), "%s/conflicts.ndjson", + fixture->host_home); + /* The host identity's cache fingerprint is derived exactly as main.c does + * for a real daemon: resolved dir -> canonical path -> SHA-256. */ + char canonical_cache[1024]; + if (test_mkdirp(fixture->runtime_parent) != 0 || !cbm_mkdir_p(fixture->host_cache, 0700) || + !cbm_canonical_path(fixture->host_cache, canonical_cache, sizeof(canonical_cache))) { + return false; + } + cbm_normalize_path_sep(canonical_cache); + cbm_sha256_hex(canonical_cache, strlen(canonical_cache), fixture->host_cache_fingerprint); + if (!cbm_daemon_runtime_process_build_fingerprint((uint64_t)getpid(), fixture->self_build)) { + return false; + } + /* The guard claims the supervisor's captured build; the runner stubs that + * capture while a runtime service must carry the real image hash. Align + * the two for this fixture exactly as one production process has them. */ + const char *previous_supervisor_build = cbm_index_supervisor_build_fingerprint(); + snprintf(fixture->previous_supervisor_build, sizeof(fixture->previous_supervisor_build), "%s", + previous_supervisor_build ? previous_supervisor_build : ""); + cbm_index_supervisor_set_build_fingerprint_for_test(fixture->self_build); + fixture->identity = (cbm_daemon_build_identity_t){ + .semantic_version = CBM_VERSION, + .build_fingerprint = fixture->self_build, + .cache_fingerprint = fixture->host_cache_fingerprint, + .protocol_abi = CBM_DAEMON_RUNTIME_WIRE_ABI, + .store_abi = 1, + .feature_abi = 1, + }; + int ready_pipe[2] = {-1, -1}; + int release_pipe[2] = {-1, -1}; + if (pipe(ready_pipe) != 0) { + return false; + } + if (pipe(release_pipe) != 0) { + close(ready_pipe[0]); + close(ready_pipe[1]); + return false; + } + pid_t child = fork(); + if (child == 0) { + close(ready_pipe[0]); + close(release_pipe[1]); + cli_scope_host_child(fixture, ready_pipe[1], release_pipe[0]); + } + close(ready_pipe[1]); + close(release_pipe[0]); + char ready = 0; + bool host_ready = child > 0 && read(ready_pipe[0], &ready, 1) == 1 && ready == 'R'; + close(ready_pipe[0]); + fixture->host = child; + fixture->release_fd = release_pipe[1]; + cli_activation_save_env(&fixture->old_home, &fixture->old_cache); + const char *shell = getenv("SHELL"); + fixture->old_shell = shell ? strdup(shell) : NULL; + cbm_setenv("SHELL", "/bin/zsh", 1); + if (!host_ready) { + return false; + } + fixture->endpoint = cbm_daemon_bootstrap_endpoint_new(fixture->runtime_parent); + cbm_daemon_runtime_connect_result_t connect_result = {0}; + fixture->client = fixture->endpoint + ? cbm_daemon_runtime_client_connect(fixture->endpoint, &fixture->identity, + CLI_SCOPE_TIMEOUT_MS, &connect_result) + : NULL; + return fixture->client != NULL; +} + +/* Ask the host daemon itself: still running, not stopping, and the parent's + * committed client still admitted. */ +static bool cli_scope_host_serving(const cli_scope_fixture_t *fixture) { + cbm_daemon_runtime_status_t status = {0}; + return fixture->endpoint && + cbm_daemon_runtime_request_status(fixture->endpoint, &fixture->identity, + CLI_SCOPE_TIMEOUT_MS, &status) && + !status.stopping && status.committed_clients == 1; +} + +static int cli_scope_install(cli_scope_fixture_t *fixture, const char *home, const char *cache, + const char *bin_dir, bool skip_binary) { + cbm_setenv("HOME", home, 1); + cbm_setenv("CBM_CACHE_DIR", cache, 1); + cbm_cli_set_activation_runtime_parent_for_test(fixture->runtime_parent); + char dir_arg[704]; + snprintf(dir_arg, sizeof(dir_arg), "--dir=%s", bin_dir); + char *install_argv[] = {skip_binary ? "--skip-binary" : "--force", "--skip-config", "--yes", + dir_arg}; + int rc = cli_test_cmd_install(4, install_argv); + cbm_cli_set_activation_runtime_parent_for_test(g_cli_suite_runtime_parent); + cbm_set_auto_answer_for_test(0); + return rc; +} + +/* Returns the host child's exit status: 0 released intact, + * CLI_SCOPE_HOST_DRAINED when an activation drained it, -1 unknown. */ +static int cli_scope_fixture_finish(cli_scope_fixture_t *fixture) { + if (fixture->client) { + (void)cbm_daemon_runtime_client_close(fixture->client, CLI_SCOPE_TIMEOUT_MS); + fixture->client = NULL; + } + if (fixture->release_fd >= 0) { + /* Closing the write end is the signal (the child's poll sees POLLHUP); + * a write would raise SIGPIPE once a drained child is already gone. */ + close(fixture->release_fd); + fixture->release_fd = -1; + } + int host_exit = -1; + if (fixture->host > 0) { + int status = 0; + if (waitpid(fixture->host, &status, 0) == fixture->host && WIFEXITED(status)) { + host_exit = WEXITSTATUS(status); + } + fixture->host = -1; + } + cbm_daemon_ipc_endpoint_free(fixture->endpoint); + fixture->endpoint = NULL; + if (fixture->previous_supervisor_build[0]) { + cbm_index_supervisor_set_build_fingerprint_for_test(fixture->previous_supervisor_build); + } + if (fixture->old_shell) { + cbm_setenv("SHELL", fixture->old_shell, 1); + } else { + cbm_unsetenv("SHELL"); + } + free(fixture->old_shell); + fixture->old_shell = NULL; + cli_activation_restore_env(fixture->old_home, fixture->old_cache); + fixture->old_home = NULL; + fixture->old_cache = NULL; + test_rmdir_r(fixture->tmpdir); + return host_exit; +} + +static void cli_scope_foreign_paths(const cli_scope_fixture_t *fixture, char home[512], + char cache[576], char bin_dir[640], char activation_log[704]) { + snprintf(home, 512, "%s/sandbox", fixture->tmpdir); + snprintf(cache, 576, "%s/cache", home); + snprintf(bin_dir, 640, "%s/custom/bin", home); + snprintf(activation_log, 704, "%s/logs/activation-events.ndjson", cache); +} + +/* (a) `HOME= install --skip-binary` — the observed incident shape: + * the sandbox shares the account rendezvous, its cache namespace differs. */ +TEST(cli_install_skip_binary_into_foreign_home_never_drains_host_cohort) { + cli_scope_fixture_t fixture; + bool ready = cli_scope_fixture_start(&fixture, "skipbin"); + 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); + int install_rc = + prepared ? cli_scope_install(&fixture, foreign_home, foreign_cache, foreign_bin, true) : -1; + bool host_serving = prepared && cli_scope_host_serving(&fixture); + 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_EQ(install_rc, 0); + ASSERT_TRUE(host_serving); + ASSERT_EQ(host_exit, 0); + ASSERT_TRUE(completed); + ASSERT_TRUE(nothing_drained); + PASS(); +} + +/* (b) A full install (binary published into the sandbox bin dir) is scoped + * the same way: the host namespace is not what is being replaced. */ +TEST(cli_install_binary_into_foreign_home_never_drains_host_cohort) { + cli_scope_fixture_t fixture; + bool ready = cli_scope_fixture_start(&fixture, "binary"); + 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); + char target_path[704]; + snprintf(target_path, sizeof(target_path), "%s/codebase-memory-mcp", foreign_bin); + bool prepared = ready && cbm_mkdir_p(foreign_home, 0700); + int install_rc = + prepared ? cli_scope_install(&fixture, foreign_home, foreign_cache, foreign_bin, false) + : -1; + bool host_serving = prepared && cli_scope_host_serving(&fixture); + struct stat target_status; + bool target_exists = stat(target_path, &target_status) == 0; + 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_EQ(install_rc, 0); + ASSERT_TRUE(target_exists); + ASSERT_TRUE(host_serving); + ASSERT_EQ(host_exit, 0); + ASSERT_TRUE(completed); + ASSERT_TRUE(nothing_drained); + PASS(); +} + +/* (c) Regression: an install that replaces the binary inside the host's own + * namespace still drains that cohort — and the audit names its client. */ +TEST(cli_install_into_host_namespace_still_drains_host_cohort) { + cli_scope_fixture_t fixture; + bool ready = cli_scope_fixture_start(&fixture, "host"); + char host_bin[640]; + char activation_log[704]; + snprintf(host_bin, sizeof(host_bin), "%s/custom/bin", fixture.host_home); + snprintf(activation_log, sizeof(activation_log), "%s/logs/activation-events.ndjson", + fixture.host_cache); + int install_rc = + ready ? cli_scope_install(&fixture, fixture.host_home, fixture.host_cache, host_bin, false) + : -1; + bool host_serving = ready && cli_scope_host_serving(&fixture); + const char *events = read_test_file(activation_log); + bool drained_in_log = events && strstr(events, "cohort drained") != NULL && + strstr(events, "\"daemon_active_clients\":1") != NULL; + int host_exit = cli_scope_fixture_finish(&fixture); + + ASSERT_TRUE(ready); + ASSERT_EQ(install_rc, 0); + ASSERT_FALSE(host_serving); + ASSERT_EQ(host_exit, CLI_SCOPE_HOST_DRAINED); + ASSERT_TRUE(drained_in_log); + PASS(); +} + +/* (d) `--skip-binary` with nothing to publish (no binary at the target, no + * index reset) replaces nothing, so even the host's own namespace has + * nothing to quiesce: agent configs are refreshed, sessions stay up. */ +TEST(cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing) { + cli_scope_fixture_t fixture; + bool ready = cli_scope_fixture_start(&fixture, "unchanged"); + char absent_bin[640]; + char activation_log[704]; + snprintf(absent_bin, sizeof(absent_bin), "%s/absent/bin", fixture.host_home); + snprintf(activation_log, sizeof(activation_log), "%s/logs/activation-events.ndjson", + fixture.host_cache); + int install_rc = + ready ? cli_scope_install(&fixture, fixture.host_home, fixture.host_cache, absent_bin, true) + : -1; + bool host_serving = ready && cli_scope_host_serving(&fixture); + 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_EQ(install_rc, 0); + ASSERT_TRUE(host_serving); + ASSERT_EQ(host_exit, 0); + ASSERT_TRUE(completed); + ASSERT_TRUE(nothing_drained); + PASS(); +} #endif TEST(cli_install_force_quiesces_active_cohort_before_replacing_binary) { @@ -14774,6 +15175,10 @@ SUITE(cli) { RUN_TEST(cli_activation_cleanup_failure_fail_stops_before_lease_release); RUN_TEST(cli_activation_quiesce_does_not_wait_on_bootstrap_startup); RUN_TEST(cli_install_recovers_markerless_stale_rendezvous); + RUN_TEST(cli_install_skip_binary_into_foreign_home_never_drains_host_cohort); + 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); #endif RUN_TEST(cli_install_force_quiesces_active_cohort_before_replacing_binary); RUN_TEST(cli_install_dir_and_skip_config_stage_first_install_safely); From 92cb3b9a4bebbc79bd283e447104669ca068ea64 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Wed, 9 Sep 2026 00:56:55 +0200 Subject: [PATCH 2/2] fix(test): bound the cli activation-guard scope fixture's host waits (#2115) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four namespace-scope tests fork a real host daemon and, in the parent, waited on it with two UNBOUNDED calls: read() for the child's one-byte readiness signal in cli_scope_fixture_start, and waitpid(host, 0) in cli_scope_fixture_finish. If the forked child stalls before it can signal — a fork-time sanitizer allocator stall is the classic cause, the same hazard the earlier posix_spawn work addressed — the parent blocks forever and the WHOLE cli suite hangs to the CI wall-clock kill (the observed rc=124 at 900 s on the ubuntu-24.04-arm gcc shard, which passed on every other #2115 shard). Attribution: the production drain path is already bounded (DRAIN 15 s, CONTROL 2 s) and a foreign-namespace install takes the non-blocking scope probe and skips draining entirely, so this is not a production deadlock — it is a test-fixture determinism gap, amplified by ASan on a loaded arm runner. Fix, test-only: - cli_scope_wait_ready(): poll the ready pipe to a generous, bounded deadline (90 s, comfortably past the child's own 45 s cohort-admission deadline plus service start), so a wedged child yields a clean ASSERT_TRUE(ready) failure instead of an unbounded read(). - cli_scope_reap_host(): reap with WNOHANG to a bound (60 s), then SIGKILL and reap, so a wedged child is force-killed rather than hanging finish(); a killed child leaves host_exit == -1, which fails the caller's assertion cleanly. This also prevents a stuck child from holding cohort locks that would poison later tests. - cli_scope_host_serving(): raise the status probe deadline from 5 s to 15 s so a slow-but-alive daemon on a loaded runner is not misread as "drained" (the flaky ASSERT(host_serving) failure this fixture showed); it still fails cleanly when the daemon is genuinely gone or reports stopping. None of these decides a healthy test — the child signals in well under a second on a healthy runner; the bounds only convert an otherwise unbounded hang into a deterministic pass/fail. The scoping contract still binds: with cli_activation_resolve_scope forced to always return ACTIVE, the three "must-not-drain" tests go RED at ASSERT(host_serving) and the host-namespace "must-drain" test stays GREEN. cli suite: 313 passed locally (macOS, twice); 30-round pre-fix and 8-round post-fix stress of the four tests were clean; a fault-injected never-ready child fails all four cleanly in ~37 s with no suite hang. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Martin Vogel --- tests/test_cli.c | 89 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 7 deletions(-) diff --git a/tests/test_cli.c b/tests/test_cli.c index 3a4b4a851..43779e3bd 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -42,6 +42,7 @@ #include #include #include +#include #endif #ifdef __APPLE__ #include @@ -1510,6 +1511,16 @@ TEST(cli_install_recovers_markerless_stale_rendezvous) { * install under test and asks the daemon itself afterwards. */ #define CLI_SCOPE_HOST_DRAINED 3 #define CLI_SCOPE_TIMEOUT_MS 5000U +/* Generous, BOUNDED waits so a wedged host child (a fork-time sanitizer + * allocator stall is the classic cause — see the posix_spawn fix history) + * fails the test cleanly instead of hanging the whole suite to the CI + * wall-clock kill. Each comfortably exceeds the child's own 45 s cohort + * admission deadline plus a multi-second service start/teardown, so none trips + * for a merely slow-but-healthy runner; they only convert an otherwise + * unbounded hang into a deterministic pass/fail. */ +#define CLI_SCOPE_READY_TIMEOUT_MS 90000U +#define CLI_SCOPE_HOST_REAP_TIMEOUT_MS 60000U +#define CLI_SCOPE_HOST_SERVING_TIMEOUT_MS 15000U typedef struct { char tmpdir[256]; @@ -1598,6 +1609,68 @@ static _Noreturn void cli_scope_host_child(const cli_scope_fixture_t *fixture, i _exit(!ready_ok ? 1 : drained ? CLI_SCOPE_HOST_DRAINED : 0); } +/* Bounded read of the host child's one-byte readiness signal. A child that + * deadlocks before it can write (a fork-time allocator stall under a sanitizer + * is the classic cause) must never hang the whole suite on an unbounded read: + * poll to a generous deadline, then let the caller's ASSERT_TRUE(ready) fail + * cleanly. Returns the byte, or 0 when the child died, closed the pipe, or + * never answered in time. */ +static char cli_scope_wait_ready(int fd, uint32_t timeout_ms) { + uint64_t deadline = cbm_now_ms() + timeout_ms; + for (;;) { + int64_t remaining = (int64_t)deadline - (int64_t)cbm_now_ms(); + if (remaining <= 0) { + return 0; + } + struct pollfd pfd = {.fd = fd, .events = POLLIN, .revents = 0}; + int r = poll(&pfd, 1, (int)remaining); + if (r < 0) { + if (errno == EINTR) { + continue; + } + return 0; + } + if (r == 0) { + return 0; /* deadline reached with no signal */ + } + char ready = 0; + ssize_t got = read(fd, &ready, 1); + if (got == 1) { + return ready; + } + if (got < 0 && errno == EINTR) { + continue; + } + return 0; /* EOF (child gone) or error */ + } +} + +/* Reap the host child within a bound: the release signal makes a healthy child + * break within ~50 ms and finish teardown in a few seconds, so a child still + * alive past the deadline is wedged — SIGKILL it and reap so the suite always + * makes progress. Returns the child's exit code, or -1 when it had to be + * killed or did not exit cleanly; a -1 fails the caller's host_exit assertion + * cleanly rather than hanging. */ +static int cli_scope_reap_host(pid_t host, uint32_t timeout_ms) { + uint64_t deadline = cbm_now_ms() + timeout_ms; + int status = 0; + for (;;) { + pid_t reaped = waitpid(host, &status, WNOHANG); + if (reaped == host) { + return WIFEXITED(status) ? WEXITSTATUS(status) : -1; + } + if (reaped < 0 && errno != EINTR) { + return -1; + } + if (cbm_now_ms() >= deadline) { + (void)kill(host, SIGKILL); + (void)waitpid(host, &status, 0); + return -1; + } + cbm_usleep(2000); + } +} + static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *tag) { memset(fixture, 0, sizeof(*fixture)); fixture->host = -1; @@ -1657,8 +1730,8 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta } close(ready_pipe[1]); close(release_pipe[0]); - char ready = 0; - bool host_ready = child > 0 && read(ready_pipe[0], &ready, 1) == 1 && ready == 'R'; + char ready = child > 0 ? cli_scope_wait_ready(ready_pipe[0], CLI_SCOPE_READY_TIMEOUT_MS) : 0; + bool host_ready = ready == 'R'; close(ready_pipe[0]); fixture->host = child; fixture->release_fd = release_pipe[1]; @@ -1682,9 +1755,14 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta * committed client still admitted. */ static bool cli_scope_host_serving(const cli_scope_fixture_t *fixture) { cbm_daemon_runtime_status_t status = {0}; + /* A generous, bounded status deadline: a foreign-namespace install leaves + * this daemon serving, so a slow response on a loaded runner must not be + * misread as "drained" (the flaky failure this fixture showed). The call + * still fails cleanly — a genuinely drained daemon is unreachable or + * reports stopping — it just no longer decides survival on a 5 s budget. */ return fixture->endpoint && cbm_daemon_runtime_request_status(fixture->endpoint, &fixture->identity, - CLI_SCOPE_TIMEOUT_MS, &status) && + CLI_SCOPE_HOST_SERVING_TIMEOUT_MS, &status) && !status.stopping && status.committed_clients == 1; } @@ -1718,10 +1796,7 @@ static int cli_scope_fixture_finish(cli_scope_fixture_t *fixture) { } int host_exit = -1; if (fixture->host > 0) { - int status = 0; - if (waitpid(fixture->host, &status, 0) == fixture->host && WIFEXITED(status)) { - host_exit = WEXITSTATUS(status); - } + host_exit = cli_scope_reap_host(fixture->host, CLI_SCOPE_HOST_REAP_TIMEOUT_MS); fixture->host = -1; } cbm_daemon_ipc_endpoint_free(fixture->endpoint);