From 459af04cb6624e7903e944b409ce4940707d441f Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 11 Oct 2026 09:00:23 +0200 Subject: [PATCH 1/2] fix(cli): name the failing update step and its OS error On macOS the signed-candidate preparation folded every step into one boolean, so each failure printed the same line: "signed update candidate preparation failed". A full /tmp read exactly like a broken codesign. That is what a cli test hit on 2026-10-09, while a multi-GB index was filling a nearly full disk. Each step now records why it failed, and the update prints, for example: error: signed update candidate preparation failed while writing the unsigned candidate: No space left on device The staging functions in activation_transaction.c keep the errno of the failing write (or sync, close, read) through their cleanup, so a caller can name it. The non-macOS path adds the OS error to its existing "failed to stage verified update" line (POSIX only; Windows reports through GetLastError). A failed fingerprint of the staged file now gets its own message instead of "...: activation transaction completed". A test seam makes the next staging write fail with a chosen errno. The new test cli_update_names_the_failing_staging_step_and_os_error fails without the cli.c change and passes with it, and checks that the old executable stays in place. Also checked on real input: with /tmp on a small volume that fills between two updates, the update prints the line above. Signed-off-by: Martin Vogel --- src/cli/activation_transaction.c | 69 ++++++++++++-- src/cli/activation_transaction.h | 7 +- src/cli/cli.c | 135 +++++++++++++++++++++++----- tests/test_cli.c | 148 +++++++++++++++++++++++++++++++ 4 files changed, 329 insertions(+), 30 deletions(-) diff --git a/src/cli/activation_transaction.c b/src/cli/activation_transaction.c index a56052775..18ed6dd14 100644 --- a/src/cli/activation_transaction.c +++ b/src/cli/activation_transaction.c @@ -138,6 +138,16 @@ void cbm_activation_transaction_note_refusal_for_testing(const char *predicate, activation_note_refusal(predicate, os_error); } } + +/* Test seam: the next `count` staging writes fail with errno = os_error, so a + * caller's report of a full disk is testable without filling one. */ +static unsigned int activation_write_failures_for_testing; +static int activation_write_failure_errno_for_testing; + +void cbm_activation_transaction_write_failures_set_for_testing(unsigned int count, int os_error) { + activation_write_failures_for_testing = count; + activation_write_failure_errno_for_testing = os_error; +} #endif #ifdef _WIN32 @@ -1134,6 +1144,13 @@ static bool activation_native_sync(activation_native_file_t file) { static bool activation_native_write_all(activation_native_file_t file, const void *data, size_t length) { +#ifdef CBM_ENABLE_TEST_SEAMS + if (activation_write_failures_for_testing > 0) { + activation_write_failures_for_testing--; + errno = activation_write_failure_errno_for_testing; + return false; + } +#endif const unsigned char *cursor = data; while (length > 0) { #ifdef _WIN32 @@ -1618,6 +1635,16 @@ static void activation_failed_stage_cleanup(cbm_activation_transaction_t *transa } } +/* A failed stage write reports IO with errno restored, after the cleanup, to + * the error of the call that failed (0 when none applies), so the caller can + * name it. */ +static cbm_activation_transaction_status_t activation_stage_io_failure( + cbm_activation_transaction_t *transaction, int os_error) { + activation_failed_stage_cleanup(transaction); + errno = os_error; + return CBM_ACTIVATION_TRANSACTION_IO; +} + cbm_activation_transaction_status_t cbm_activation_transaction_stage_bytes( const char *target_path, const void *candidate, size_t candidate_size, cbm_activation_transaction_t **transaction_out) { @@ -1643,11 +1670,20 @@ cbm_activation_transaction_status_t cbm_activation_transaction_stage_bytes( } transaction->staged_exists = true; bool written = activation_native_write_all(staged, candidate, candidate_size); + int os_error = written ? 0 : errno; bool durable = written && activation_native_sync(staged); + if (written && !durable) { + os_error = errno; + } bool closed = activation_native_close(staged); - if (!written || !durable || !closed || !activation_sync_directory(transaction)) { - activation_failed_stage_cleanup(transaction); - return CBM_ACTIVATION_TRANSACTION_IO; + if (!closed && os_error == 0) { + os_error = errno; + } + if (!written || !durable || !closed) { + return activation_stage_io_failure(transaction, os_error); + } + if (!activation_sync_directory(transaction)) { + return activation_stage_io_failure(transaction, errno); } *transaction_out = transaction; return CBM_ACTIVATION_TRANSACTION_OK; @@ -1797,28 +1833,45 @@ cbm_activation_transaction_status_t cbm_activation_transaction_stage_file( unsigned char buffer[64U * 1024U]; size_t total = 0; bool copied = true; + int os_error = 0; for (;;) { size_t amount = 0; if (!activation_native_read(source, buffer, sizeof(buffer), &amount)) { copied = false; + os_error = errno; break; } if (amount == 0) { break; } - if (SIZE_MAX - total < amount || !activation_native_write_all(staged, buffer, amount)) { + if (SIZE_MAX - total < amount) { copied = false; break; } + if (!activation_native_write_all(staged, buffer, amount)) { + copied = false; + os_error = errno; + break; + } total += amount; } bool durable = copied && total > 0 && activation_native_sync(staged); + if (copied && total > 0 && !durable) { + os_error = errno; + } bool source_closed = activation_native_close(source); + if (!source_closed && os_error == 0) { + os_error = errno; + } bool staged_closed = activation_native_close(staged); - if (!copied || total == 0 || !durable || !source_closed || !staged_closed || - !activation_sync_directory(transaction)) { - activation_failed_stage_cleanup(transaction); - return CBM_ACTIVATION_TRANSACTION_IO; + if (!staged_closed && os_error == 0) { + os_error = errno; + } + if (!copied || total == 0 || !durable || !source_closed || !staged_closed) { + return activation_stage_io_failure(transaction, os_error); + } + if (!activation_sync_directory(transaction)) { + return activation_stage_io_failure(transaction, errno); } *transaction_out = transaction; return CBM_ACTIVATION_TRANSACTION_OK; diff --git a/src/cli/activation_transaction.h b/src/cli/activation_transaction.h index c0dd89bfc..3aa37e1e4 100644 --- a/src/cli/activation_transaction.h +++ b/src/cli/activation_transaction.h @@ -45,7 +45,11 @@ void cbm_activation_transaction_set_before_absent_publish_for_test( void cbm_activation_transaction_rename_failures_set_for_test(unsigned int count); /* Stage a candidate beside target_path (therefore on the same filesystem). - * The staged file is private to the current account and executable. */ + * The staged file is private to the current account and executable. + * On POSIX, an IO result leaves errno at the error of the failing system call + * (0 when none applies, e.g. a short read), through the stage cleanup, so a + * caller can name it: a full disk reads "No space left on device". The same + * holds for stage_file below. */ cbm_activation_transaction_status_t cbm_activation_transaction_stage_bytes( const char *target_path, const void *candidate, size_t candidate_size, cbm_activation_transaction_t **transaction_out); @@ -100,6 +104,7 @@ const char *cbm_activation_transaction_refusal_note(void); #ifdef CBM_ENABLE_TEST_SEAMS void cbm_activation_transaction_note_refusal_for_testing(const char *predicate, unsigned long os_error); +void cbm_activation_transaction_write_failures_set_for_testing(unsigned int count, int os_error); #endif #endif /* CBM_ACTIVATION_TRANSACTION_H */ diff --git a/src/cli/cli.c b/src/cli/cli.c index c99034847..172657edd 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -13202,6 +13202,45 @@ static int cli_update_activate_binary(void *opaque) { return CLI_OK; } +#ifdef __APPLE__ +/* The first step of the signed-candidate preparation that failed. One boolean + * used to cover all of them, so a full disk read exactly like a broken + * codesign: "candidate preparation failed", and nothing else. */ +typedef struct { + const char *step; + int os_error; /* errno left by an OS-level step; 0 when none applies */ + const char *detail; /* the reason when no OS error applies, or NULL */ +} cli_update_prep_failure_t; + +/* Records the first failing step. An OS-level step keeps the errno it left; a + * step that succeeds clears errno so the next one starts clean. Returns ok, so + * the steps chain with &&. */ +static bool cli_update_prep_step(bool ok, const char *step, bool os_level, const char *detail, + cli_update_prep_failure_t *failure) { + if (ok) { + errno = 0; + return true; + } + if (!failure->step) { + failure->step = step; + failure->os_error = os_level ? errno : 0; + failure->detail = detail; + } + return false; +} + +static void cli_update_prep_report(const cli_update_prep_failure_t *failure) { + const char *step = failure->step ? failure->step : "an unrecorded step"; + const char *reason = failure->os_error != 0 ? strerror(failure->os_error) : failure->detail; + if (reason) { + (void)fprintf(stderr, "error: signed update candidate preparation failed while %s: %s\n", + step, reason); + } else { + (void)fprintf(stderr, "error: signed update candidate preparation failed while %s\n", step); + } +} +#endif + static int extract_and_install_binary(extract_install_args_t args) { const char *tmp_archive = args.tmp_archive; const char *ext = args.ext; @@ -13259,42 +13298,79 @@ static int extract_and_install_binary(extract_install_args_t args) { #ifdef __APPLE__ /* codesign replaces the signed file on current macOS releases. Publish and * sign a disposable private copy first, then stage the resulting immutable - * bytes into the final transaction. */ + * bytes into the final transaction. Every step records why it failed, so a + * full disk is reported as one and not as a signing failure. */ + cli_update_prep_failure_t failure = {0}; char prepared_dir[CLI_BUF_1K]; char prepared_candidate[CLI_BUF_1K]; int prepared_dir_length = snprintf(prepared_dir, sizeof(prepared_dir), "%s/cbm-update-sign-XXXXXX", cbm_tmpdir()); - bool prepared = prepared_dir_length > 0 && (size_t)prepared_dir_length < sizeof(prepared_dir) && - cbm_mkdtemp(prepared_dir) != NULL; + errno = 0; + bool prepared = cli_update_prep_step(prepared_dir_length > 0 && + (size_t)prepared_dir_length < sizeof(prepared_dir) && + cbm_mkdtemp(prepared_dir) != NULL, + "creating its private directory", true, NULL, &failure); int prepared_candidate_length = prepared ? snprintf(prepared_candidate, sizeof(prepared_candidate), "%s/codebase-memory-mcp", prepared_dir) : CLI_ERR; - prepared = prepared && prepared_candidate_length > 0 && - (size_t)prepared_candidate_length < sizeof(prepared_candidate); + prepared = prepared && cli_update_prep_step(prepared_candidate_length > 0 && + (size_t)prepared_candidate_length < + sizeof(prepared_candidate), + "naming the candidate file", false, NULL, &failure); cbm_activation_transaction_t *preparation = NULL; stage_status = prepared ? cbm_activation_transaction_stage_bytes(prepared_candidate, bin_data, (size_t)bin_len, &preparation) : CBM_ACTIVATION_TRANSACTION_IO; + prepared = prepared && + cli_update_prep_step( + stage_status == CBM_ACTIVATION_TRANSACTION_OK && preparation, + "writing the unsigned candidate", stage_status == CBM_ACTIVATION_TRANSACTION_IO, + cbm_activation_transaction_status_message(stage_status), &failure); free(bin_data); cli_binary_validator_t unsigned_validator = {{0}}; - prepared = stage_status == CBM_ACTIVATION_TRANSACTION_OK && preparation && - cli_activation_transaction_expected_build(preparation, &unsigned_validator) && - cli_activation_transaction_commit_validated(preparation, &unsigned_validator, - CLI_OCTAL_PERM) == CLI_OK && - cli_activation_transaction_finalize_close(&preparation) == CLI_OK && - cbm_macos_adhoc_sign(prepared_candidate) == CLI_OK && - cbm_daemon_build_fingerprint_file(prepared_candidate, validator.fingerprint); + prepared = prepared && + cli_update_prep_step( + cli_activation_transaction_expected_build(preparation, &unsigned_validator), + "fingerprinting the unsigned candidate", true, NULL, &failure) && + cli_update_prep_step(cli_activation_transaction_commit_validated( + preparation, &unsigned_validator, CLI_OCTAL_PERM) == CLI_OK, + "publishing the unsigned candidate", false, NULL, &failure) && + cli_update_prep_step( + cli_activation_transaction_finalize_close(&preparation) == CLI_OK, + "closing the unsigned candidate's transaction", false, NULL, &failure) && + cli_update_prep_step(cbm_macos_adhoc_sign(prepared_candidate) == CLI_OK, + "signing the candidate ad hoc", false, NULL, &failure) && + cli_update_prep_step( + cbm_daemon_build_fingerprint_file(prepared_candidate, validator.fingerprint), + "fingerprinting the signed candidate", true, NULL, &failure); const char *prepared_argv[] = {prepared_candidate, "--version", NULL}; - prepared = prepared && cbm_exec_no_shell(prepared_argv) == CLI_OK; + int version_rc = prepared ? cbm_exec_no_shell(prepared_argv) : CLI_OK; + char version_detail[80]; + if (version_rc == CBM_NOT_FOUND) { + snprintf(version_detail, sizeof(version_detail), "it could not run or was killed"); + } else { + snprintf(version_detail, sizeof(version_detail), "--version exited with status %d", + version_rc); + } + prepared = + prepared && cli_update_prep_step(version_rc == CLI_OK, "running the signed candidate", + false, version_detail, &failure); if (prepared) { stage_status = cbm_activation_transaction_stage_file(bin_dest, prepared_candidate, &binary_transaction); cli_binary_validator_t staged_validator = {{0}}; prepared = - stage_status == CBM_ACTIVATION_TRANSACTION_OK && binary_transaction && - cli_activation_transaction_expected_build(binary_transaction, &staged_validator) && - strcmp(staged_validator.fingerprint, validator.fingerprint) == 0; + cli_update_prep_step( + stage_status == CBM_ACTIVATION_TRANSACTION_OK && binary_transaction, + "staging the signed candidate", stage_status == CBM_ACTIVATION_TRANSACTION_IO, + cbm_activation_transaction_status_message(stage_status), &failure) && + cli_update_prep_step( + cli_activation_transaction_expected_build(binary_transaction, &staged_validator), + "fingerprinting the staged candidate", true, NULL, &failure) && + cli_update_prep_step(strcmp(staged_validator.fingerprint, validator.fingerprint) == 0, + "comparing the staged and signed candidates", false, + "their fingerprints differ", &failure); if (prepared) { validator = staged_validator; } @@ -13308,18 +13384,35 @@ static int extract_and_install_binary(extract_install_args_t args) { (void)cbm_rmdir(prepared_dir); } if (!prepared) { - (void)fprintf(stderr, "error: signed update candidate preparation failed\n"); + cli_update_prep_report(&failure); (void)cli_activation_transaction_abort(&binary_transaction); return CLI_TRUE; } #else + errno = 0; stage_status = cbm_activation_transaction_stage_bytes(bin_dest, bin_data, (size_t)bin_len, &binary_transaction); + /* POSIX only: Windows staging reports through GetLastError, not errno. */ +#ifdef _WIN32 + int stage_error = 0; +#else + int stage_error = stage_status == CBM_ACTIVATION_TRANSACTION_IO ? errno : 0; +#endif free(bin_data); - if (stage_status != CBM_ACTIVATION_TRANSACTION_OK || !binary_transaction || - !cli_activation_transaction_expected_build(binary_transaction, &validator)) { - (void)fprintf(stderr, "error: failed to stage verified update: %s\n", - cbm_activation_transaction_status_message(stage_status)); + if (stage_status != CBM_ACTIVATION_TRANSACTION_OK || !binary_transaction) { + if (stage_error != 0) { + (void)fprintf(stderr, "error: failed to stage verified update: %s: %s\n", + cbm_activation_transaction_status_message(stage_status), + strerror(stage_error)); + } else { + (void)fprintf(stderr, "error: failed to stage verified update: %s\n", + cbm_activation_transaction_status_message(stage_status)); + } + (void)cli_activation_transaction_abort(&binary_transaction); + return CLI_TRUE; + } + if (!cli_activation_transaction_expected_build(binary_transaction, &validator)) { + (void)fprintf(stderr, "error: failed to fingerprint the staged update\n"); (void)cli_activation_transaction_abort(&binary_transaction); return CLI_TRUE; } diff --git a/tests/test_cli.c b/tests/test_cli.c index a3fda1d5f..33340328c 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -2969,6 +2969,153 @@ TEST(cli_update_agent_configs_finish_before_guard_release) { #endif } +/* 2026-10-09: /tmp filled up mid-update, and the update said only "signed + * update candidate preparation failed", although the cause was a full disk. + * The failing staging write must be reported by step and OS error, through the + * staging cleanup, and the old executable must stay. */ +TEST(cli_update_names_the_failing_staging_step_and_os_error) { +#ifdef _WIN32 + SKIP_PLATFORM("Windows staging reports through GetLastError, not errno"); +#else + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-update-enospc-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); + const char *download_env = getenv("CBM_DOWNLOAD_URL"); + char *old_download = download_env ? strdup(download_env) : NULL; + cbm_setenv("HOME", tmpdir, 1); + char cache_dir[512]; + snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); + cbm_setenv("CBM_CACHE_DIR", cache_dir, 1); + char release_dir[512]; + char bin_dir[512]; + char bin_target[640]; + snprintf(release_dir, sizeof(release_dir), "%s/release", tmpdir); + snprintf(bin_dir, sizeof(bin_dir), "%s/.local/bin", tmpdir); + snprintf(bin_target, sizeof(bin_target), "%s/codebase-memory-mcp", bin_dir); + test_mkdirp(release_dir); + test_mkdirp(bin_dir); + static const char old_binary[] = "old binary before a failed update"; + write_test_file(bin_target, old_binary); + +#if defined(__APPLE__) +#if defined(__aarch64__) || defined(__arm64__) + const char *asset_name = "codebase-memory-mcp-darwin-arm64.tar.gz"; +#else + const char *asset_name = "codebase-memory-mcp-darwin-amd64.tar.gz"; +#endif + char native_fixture_path[CBM_SZ_4K] = {0}; + uint32_t native_fixture_size = (uint32_t)sizeof(native_fixture_path); + ASSERT_EQ(_NSGetExecutablePath(native_fixture_path, &native_fixture_size), 0); + const char *native_fixture = native_fixture_path; +#else +#if defined(__aarch64__) + const char *asset_name = "codebase-memory-mcp-linux-arm64-portable.tar.gz"; +#else + const char *asset_name = "codebase-memory-mcp-linux-amd64-portable.tar.gz"; +#endif + const char *native_fixture = "/bin/true"; +#endif + FILE *native_file = fopen(native_fixture, "rb"); + ASSERT_NOT_NULL(native_file); + ASSERT_EQ(fseek(native_file, 0, SEEK_END), 0); + long native_size = ftell(native_file); + ASSERT_GT(native_size, 0); + ASSERT_LTE(native_size, INT_MAX); + ASSERT_EQ(fseek(native_file, 0, SEEK_SET), 0); + unsigned char *replacement = malloc((size_t)native_size); + ASSERT_NOT_NULL(replacement); + ASSERT_EQ(fread(replacement, 1, (size_t)native_size, native_file), (size_t)native_size); + ASSERT_EQ(fclose(native_file), 0); + int archive_len = 0; + unsigned char *archive = + create_test_targz("codebase-memory-mcp", replacement, (int)native_size, &archive_len); + free(replacement); + ASSERT_NOT_NULL(archive); + char archive_path[768]; + snprintf(archive_path, sizeof(archive_path), "%s/%s", release_dir, asset_name); + FILE *archive_file = fopen(archive_path, "wb"); + ASSERT_NOT_NULL(archive_file); + ASSERT_EQ(fwrite(archive, 1, (size_t)archive_len, archive_file), (size_t)archive_len); + ASSERT_EQ(fclose(archive_file), 0); + free(archive); + char digest[65]; + ASSERT_EQ(cbm_cli_sha256_file(archive_path, digest, sizeof(digest)), 0); + char checksum_path[640]; + snprintf(checksum_path, sizeof(checksum_path), "%s/checksums.txt", release_dir); + FILE *checksum_file = fopen(checksum_path, "w"); + ASSERT_NOT_NULL(checksum_file); + ASSERT_TRUE(fprintf(checksum_file, "%s %s\n", digest, asset_name) > 0); + ASSERT_EQ(fclose(checksum_file), 0); + char download_url[640]; + snprintf(download_url, sizeof(download_url), "file://%s", release_dir); + cbm_setenv("CBM_DOWNLOAD_URL", download_url, 1); + + cli_activation_fake_t fake = { + .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) { + /* The first staging write of the update fails as on a full disk. */ + cbm_activation_transaction_write_failures_set_for_testing(1U, ENOSPC); + char *argv[] = {"--force"}; + rc = cli_test_cmd_update(1, argv); + cbm_activation_transaction_write_failures_set_for_testing(0U, 0); + } + 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); + } + cbm_cli_set_activation_ops_for_test(NULL); + char output[16384] = {0}; + if (capture) { + rewind(capture); + size_t count = fread(output, 1, sizeof(output) - 1U, capture); + output[count] = '\0'; + fclose(capture); + } + const char *kept = read_test_file(bin_target); + bool old_kept = kept && strcmp(kept, old_binary) == 0; + + if (old_download) { + cbm_setenv("CBM_DOWNLOAD_URL", old_download, 1); + } else { + cbm_unsetenv("CBM_DOWNLOAD_URL"); + } + free(old_download); + cli_activation_restore_env(old_home, old_cache); + test_rmdir_r(tmpdir); + + ASSERT_NEQ(rc, 0); + ASSERT_NOT_NULL(strstr(output, strerror(ENOSPC))); +#ifdef __APPLE__ + ASSERT_NOT_NULL(strstr(output, "failed while writing the unsigned candidate")); +#else + ASSERT_NOT_NULL(strstr(output, "failed to stage verified update")); +#endif + ASSERT_TRUE(old_kept); + PASS(); +#endif +} + TEST(cli_uninstall_quiesces_active_cohort_before_removing_binary_and_index) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-daemon-uninstall-XXXXXX"); @@ -17776,6 +17923,7 @@ SUITE(cli) { RUN_TEST(cli_clients_selector_filters_registry_installs_issue1798); RUN_TEST(cli_update_accepts_retired_variant_flags_issue1544); RUN_TEST(cli_update_agent_configs_finish_before_guard_release); + RUN_TEST(cli_update_names_the_failing_staging_step_and_os_error); 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); RUN_TEST(cli_uninstall_auto_answers_keep_indexes_without_delete_indexes); From 7547801a41bb7089869bb03deb3689dcafbd4989 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 11 Oct 2026 09:00:23 +0200 Subject: [PATCH 2/2] test(cli): the scope fixture names its failing step; no timeout decides readiness cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing failed once on 2026-10-09 at ASSERT(ready), with nothing else in the log. cli_scope_fixture_start returned false silently from any of its setup steps, and a 90 s deadline decided the wait for the host child's readiness. The fixture now records and prints the step that failed: the fixture directory, the host directories, the runner's own image fingerprint, the pipes, the fork, the host's start, or the client connect, with the OS error where one applies. The host child prints which of its own steps failed before it answers 'E'. The readiness wait has no deadline any more, because no timeout may decide a test. The child answers 'R' or 'E', or the pipe reaches EOF when it dies. A child that wedges is left to the harness's per-suite ceiling. New test: cli_scope_fixture_names_its_failing_setup_step. Signed-off-by: Martin Vogel --- tests/test_cli.c | 122 +++++++++++++++++++++++++++++++---------------- 1 file changed, 80 insertions(+), 42 deletions(-) diff --git a/tests/test_cli.c b/tests/test_cli.c index 33340328c..c325f399e 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -1572,14 +1572,13 @@ 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 +/* The host child's readiness has NO deadline (USER RULE: no timeout decides a + * test). The child answers 'R' or 'E', or the pipe reaches EOF when it dies, + * and the fixture names the failing step in every case. A child that wedges + * before answering is left to the harness's per-suite ceiling. A 90 s bound + * used to decide this wait, and on a starved machine (2026-10-09, during a + * kernel index) ASSERT(ready) failed with nothing in the log to say why. + * Generous, bounded waits remain for reaping and status probes below. */ #define CLI_SCOPE_HOST_REAP_TIMEOUT_MS 60000U #define CLI_SCOPE_HOST_SERVING_TIMEOUT_MS 15000U /* Teardown budget for a child the activation ALREADY drained: its service is @@ -1621,6 +1620,7 @@ typedef struct { char *old_home; char *old_cache; char *old_shell; + char failed_step[160]; /* the setup step that failed, "" when ready */ } cli_scope_fixture_t; static _Noreturn void cli_scope_host_child(const cli_scope_fixture_t *fixture, int ready_fd, @@ -1632,9 +1632,12 @@ static _Noreturn void cli_scope_host_child(const cli_scope_fixture_t *fixture, i 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; + int acquire_status = -1; + if (endpoint && manager) { + acquire_status = (int)cbm_version_cohort_acquire(manager, &fixture->identity, + cbm_now_ms() + 45000U, &lease, &conflict); + } + bool admitted = acquire_status == (int)CBM_VERSION_COHORT_OK; if (admitted) { cbm_daemon_runtime_service_config_t config = { .endpoint = endpoint, @@ -1654,6 +1657,16 @@ static _Noreturn void cli_scope_host_child(const cli_scope_fixture_t *fixture, i } bool ready_ok = service && cbm_daemon_runtime_service_state(service) == CBM_DAEMON_RUNTIME_SERVICE_RUNNING; + if (!ready_ok) { + /* The parent sees only 'E'; say here which step it was. */ + const char *step = !endpoint ? "creating the endpoint" + : !manager ? "creating the cohort manager" + : !admitted ? "joining the version cohort" + : !service ? "starting the runtime service" + : "reaching the running state"; + (void)fprintf(stderr, "cli scope host: setup failed at %s (cohort status %d)\n", step, + acquire_status); + } char ready = ready_ok ? 'R' : 'E'; (void)write(ready_fd, &ready, 1); close(ready_fd); @@ -1701,30 +1714,11 @@ 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; +/* Read the host child's one-byte readiness signal, with no deadline: the + * answer is 'R', 'E', or EOF when the child is gone (see + * CLI_SCOPE_TIMEOUT_MS). Returns the byte, or 0 on EOF or a read error. */ +static char cli_scope_wait_ready(int fd) { 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) { @@ -1737,6 +1731,20 @@ static char cli_scope_wait_ready(int fd, uint32_t timeout_ms) { } } +/* Records which setup step failed and says so on stderr, so the caller's + * ASSERT_TRUE(ready) is never a silent failure. Returns false. */ +static bool cli_scope_setup_failed(cli_scope_fixture_t *fixture, const char *tag, const char *step, + int os_error) { + snprintf(fixture->failed_step, sizeof(fixture->failed_step), "%s", step); + if (os_error != 0) { + (void)fprintf(stderr, "cli scope fixture %s: setup failed at %s: %s\n", tag, step, + strerror(os_error)); + } else { + (void)fprintf(stderr, "cli scope fixture %s: setup failed at %s\n", tag, step); + } + return false; +} + /* 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 @@ -1769,7 +1777,7 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta fixture->release_fd = -1; snprintf(fixture->tmpdir, sizeof(fixture->tmpdir), "/tmp/cli-guard-scope-%s-XXXXXX", tag); if (!cbm_mkdtemp(fixture->tmpdir)) { - return false; + return cli_scope_setup_failed(fixture, tag, "creating the fixture directory", errno); } snprintf(fixture->runtime_parent, sizeof(fixture->runtime_parent), "%s/runtime", fixture->tmpdir); @@ -1782,12 +1790,14 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta 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; + return cli_scope_setup_failed(fixture, tag, "creating the host directories", errno); } 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; + /* Fails when the runner's file no longer matches its running image, + * e.g. a rebuild replaced it mid-run. */ + return cli_scope_setup_failed(fixture, tag, "fingerprinting the runner's own image", 0); } /* The guard claims the supervisor's captured build; the runner stubs that * capture while a runtime service must carry the real image hash. Align @@ -1807,14 +1817,16 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta int ready_pipe[2] = {-1, -1}; int release_pipe[2] = {-1, -1}; if (pipe(ready_pipe) != 0) { - return false; + return cli_scope_setup_failed(fixture, tag, "creating the readiness pipe", errno); } if (pipe(release_pipe) != 0) { + int pipe_error = errno; close(ready_pipe[0]); close(ready_pipe[1]); - return false; + return cli_scope_setup_failed(fixture, tag, "creating the release pipe", pipe_error); } pid_t child = fork(); + int fork_error = child < 0 ? errno : 0; if (child == 0) { close(ready_pipe[0]); close(release_pipe[1]); @@ -1822,7 +1834,7 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta } close(ready_pipe[1]); close(release_pipe[0]); - char ready = child > 0 ? cli_scope_wait_ready(ready_pipe[0], CLI_SCOPE_READY_TIMEOUT_MS) : 0; + char ready = child > 0 ? cli_scope_wait_ready(ready_pipe[0]) : 0; bool host_ready = ready == 'R'; close(ready_pipe[0]); fixture->host = child; @@ -1831,8 +1843,15 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta const char *shell = getenv("SHELL"); fixture->old_shell = shell ? strdup(shell) : NULL; cbm_setenv("SHELL", "/bin/zsh", 1); + if (child < 0) { + return cli_scope_setup_failed(fixture, tag, "forking the host", fork_error); + } if (!host_ready) { - return false; + return cli_scope_setup_failed(fixture, tag, + ready == 'E' + ? "starting the host (its own line names the step)" + : "starting the host, which exited before answering", + 0); } fixture->endpoint = cbm_daemon_bootstrap_endpoint_new(fixture->runtime_parent); cbm_daemon_runtime_connect_result_t connect_result = {0}; @@ -1840,7 +1859,13 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta ? cbm_daemon_runtime_client_connect(fixture->endpoint, &fixture->identity, CLI_SCOPE_TIMEOUT_MS, &connect_result) : NULL; - return fixture->client != NULL; + if (!fixture->client) { + return cli_scope_setup_failed(fixture, tag, + fixture->endpoint ? "connecting the parent's client" + : "creating the parent's endpoint", + 0); + } + return true; } static bool cli_scope_host_serving_within(const cli_scope_fixture_t *fixture, uint32_t timeout_ms) { @@ -2043,6 +2068,18 @@ TEST(cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing) { PASS(); } +/* 2026-10-09: ASSERT(ready) failed with nothing in the log, so which setup + * step broke was unknowable. The fixture now names the step it stopped at. A + * tag naming a missing directory makes its own mkdtemp fail deterministically; + * nothing is created, so there is nothing to finish. */ +TEST(cli_scope_fixture_names_its_failing_setup_step) { + cli_scope_fixture_t fixture; + bool ready = cli_scope_fixture_start(&fixture, "no-such-parent/x"); + ASSERT_FALSE(ready); + ASSERT_STR_EQ(fixture.failed_step, "creating the fixture directory"); + 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, @@ -17902,6 +17939,7 @@ 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_scope_fixture_names_its_failing_setup_step); RUN_TEST(cli_install_foreign_home_unreadable_cohort_cache_keeps_host_daemon); RUN_TEST(cli_install_foreign_home_busy_cohort_keeps_host_daemon); #endif