Merge pull request #1588 from DeusData/fix/refusal-detail-propagation

fix(cli): show the check that refused, instead of pointing at nothing (#1537)
This commit is contained in:
Martin Vogel authored and GitHub committed 2026-08-13 16:47:54 +02:00
commit 9fbcd726fd
4 files changed
+61

No files matched your search

+17
View File
@@ -242,6 +242,23 @@ static void cli_activation_diagnostic(const cbm_cli_activation_ops_t *ops, const
"or use an owner-private directory for --dir/CBM_CACHE_DIR, then retry.",
note);
diagnostic = attributed;
} else if (diagnostic == CLI_ACTIVATION_REFUSED_MESSAGE) {
/* #1537/#1416: the reservation-failure message told the reader to
* "check the errors above" — and nothing was above. The detail that
* names the failing component is recorded on the daemon side and was
* only ever surfaced by `daemon status`, so the CLI replaced a message
* that blamed the wrong thing with one that blamed nothing. Two
* reporters were left with no way forward. Print it here. */
const char *detail = cbm_daemon_ipc_validation_detail();
if (detail && detail[0]) {
(void)snprintf(attributed, sizeof(attributed),
"error: activation could not reserve exclusive access; no activation "
"was committed.\n"
"error: this is NOT a running-session problem — nothing needs to be "
"closed. The check that refused was: %s",
detail);
diagnostic = attributed;
}
}
if (ops && ops->visible_diagnostic) {
ops->visible_diagnostic(ops->context, diagnostic);
+7
View File
@@ -42,6 +42,13 @@ static void ipc_validation_detail_set(const char *format, ...) {
va_end(arguments);
}
#ifdef CBM_ENABLE_TEST_SEAMS
void cbm_daemon_ipc_set_validation_detail_for_testing(const char *detail) {
(void)snprintf(ipc_validation_detail_buffer, sizeof(ipc_validation_detail_buffer), "%s",
detail ? detail : "");
}
#endif
const char *cbm_daemon_ipc_validation_detail(void) {
return ipc_validation_detail_buffer;
}
+4
View File
@@ -91,6 +91,10 @@ bool cbm_daemon_ipc_private_directory_secure(const char *directory_path);
* validation refusal (empty string when none). Diagnostic only — callers
* append it to their error messages; policy decisions never read it. */
const char *cbm_daemon_ipc_validation_detail(void);
#ifdef CBM_ENABLE_TEST_SEAMS
/* #1537: seed the detail so a test can prove the CLI refusal surfaces it. */
void cbm_daemon_ipc_set_validation_detail_for_testing(const char *detail);
#endif
/* Create/validate an owner-only directory and securely open one regular
* owner-only append log within it. User-controlled path components may not be
+33
View File
@@ -687,6 +687,38 @@ TEST(cli_activation_refusal_note_reaches_diagnostic_issue1416) {
* CBM sessions could not be stopped" sent a reporter hunting processes that a
* reboot proved did not exist — and the remedy we printed told them to run a
* cbm binary they had just uninstalled. Each condition now names itself. */
/* #1537/#1416: the reservation-failure message told readers to "check the
* errors above" — and nothing was above. The detail naming the failing
* component is recorded on the daemon side and was only surfaced by
* `daemon status`, so the CLI replaced a message that blamed the WRONG thing
* with one that blamed NOTHING. Two reporters were left with no way forward.
*
* The assertion is the property, not the wording: the refusal must never point
* at evidence it does not show. */
TEST(cli_activation_refusal_shows_the_detail_it_points_at_issue1537) {
cbm_activation_transaction_note_refusal_for_testing(NULL, 0UL);
cbm_daemon_ipc_set_validation_detail_for_testing(
"/home/u/.cache/codebase-memory-mcp: ancestor '.cache' is not a usable "
"private-directory parent");
cli_activation_fake_t failed = {
.mutation_reserve_result = -1,
};
cbm_cli_activation_ops_t ops = {
.context = &failed,
.reserve_for_mutation = cli_activation_fake_reserve_mutation,
.mutation_lease_release = cli_activation_fake_release_mutation,
.visible_diagnostic = cli_activation_fake_diagnostic,
};
ASSERT_EQ(cbm_cli_activation_guard_with_ops(&ops, cli_activation_fake_mutation, &failed), 1);
/* The named component must appear, and the dangling pointer must not. */
ASSERT_NOT_NULL(strstr(failed.diagnostic, "is not a usable private-directory parent"));
ASSERT_NULL(strstr(failed.diagnostic, "Check the errors above"));
cbm_daemon_ipc_set_validation_detail_for_testing("");
PASS();
}
TEST(cli_activation_distinguishes_busy_from_reservation_failure_issue1537) {
cbm_activation_transaction_note_refusal_for_testing(NULL, 0UL);
@@ -12421,6 +12453,7 @@ SUITE(cli) {
RUN_TEST(cli_activation_quiesces_active_cohort_before_mutation);
RUN_TEST(cli_activation_refuses_when_cohort_does_not_drain);
RUN_TEST(cli_activation_refusal_note_reaches_diagnostic_issue1416);
RUN_TEST(cli_activation_refusal_shows_the_detail_it_points_at_issue1537);
RUN_TEST(cli_activation_distinguishes_busy_from_reservation_failure_issue1537);
RUN_TEST(cli_activation_refuses_unsafe_cohort_reservation);
RUN_TEST(cli_activation_releases_maintenance_lease_after_success);