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 <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel committed 2026-10-11 09:00:23 +02:00
1 parent 83edbd3a12
commit 459af04cb6
4 files changed
+329 -30

No files matched your search

+61 -8
View File
@@ -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;
+6 -1
View File
@@ -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 */
+114 -21
View File
@@ -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;
}
+148
View File
@@ -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);