Skip to content

Commit eb3e571

Browse files
authored
Merge pull request #1441 from DeusData/fix/1388-hook-conflict-notice
fix(hook): surface daemon build conflicts to the hook caller (#1388)
2 parents ad96bf3 + 856ec69 commit eb3e571

6 files changed

Lines changed: 239 additions & 7 deletions

File tree

‎scripts/test.sh‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,19 @@ CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_worker_watchdog.sh"
282282
echo "=== Step 5c: worker error-response transport regression ==="
283283
bash "$ROOT/tests/test_worker_error_response.sh"
284284

285+
# Step 5d (#1388) is DELIBERATELY NOT GATING HERE — see
286+
# tests/test_hook_conflict_notice.sh for the full what-was-tried record.
287+
# Summary: the test forces a client/daemon build mismatch via the
288+
# CBM_TEST_HOOK_CLIENT_BUILD seam and asserts the stdout systemMessage. It is
289+
# reliably green locally against a seam-bearing binary, but on every CI leg the
290+
# forced mismatch raises no cohort conflict at all: the seam is present (the
291+
# test asserts that up front), the forced fingerprint is well-formed (64 hex),
292+
# and `daemon status` reports an active daemon on a DIFFERENT build - yet the
293+
# client joins silently. Until that local-vs-CI divergence in the cohort
294+
# admission path is understood, gating on it would make an unexplained red, and
295+
# skipping it silently would hide the gap. Run it by hand:
296+
# make -f Makefile.cbm cbm TEST_SEAMS=1 && bash tests/test_hook_conflict_notice.sh
297+
285298
# Step 6: security-strings URL allow-list regression. The MSYS2 CLANG64 toolchain
286299
# bakes its package-tracker URL into the static Windows .exe; the binary string
287300
# audit must allow-list it (Windows-only — Linux smoke never saw it).

‎src/cli/cli.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,20 @@ char *cbm_hook_augment_lifecycle_json_for(const char *input, const char *forced_
477477
void cbm_hook_augment_arm_deadline(void);
478478
char *cbm_hook_augment_read_stdin(void);
479479

480+
/* Why a hook client is not augmenting. The hook caller only ever sees stdout,
481+
* so each reason that is actionable by the user must have a stdout notice
482+
* (#1388: a build-conflicted daemon used to report on stderr alone, which is
483+
* invisible in-session and reads as silent skips). */
484+
typedef enum {
485+
CBM_HOOK_ADMISSION_DAEMON_ABSENT = 0, /* no daemon running: `daemon start` heals it */
486+
CBM_HOOK_ADMISSION_BUILD_CONFLICT /* daemon runs another build: needs `daemon stop` */
487+
} cbm_hook_admission_t;
488+
489+
/* The JSON systemMessage a hook client must print on stdout for `reason`, or
490+
* NULL when nothing should be printed. hook_dialect NULL = Claude Code, the
491+
* only dialect that surfaces a stdout systemMessage to the user. */
492+
const char *cbm_hook_admission_notice(cbm_hook_admission_t reason, const char *hook_dialect);
493+
480494
/* Process one already-read hook payload using a caller-owned MCP session.
481495
* Returns a malloc-owned hook output JSON string, or NULL for fail-open/no
482496
* augmentation. This is the daemon entry; it never arms a process-global

‎src/cli/hook_augment.c‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,3 +1437,24 @@ int cbm_cmd_hook_augment(int argc, char **argv) {
14371437
free(input);
14381438
return 0;
14391439
}
1440+
1441+
const char *cbm_hook_admission_notice(cbm_hook_admission_t reason, const char *hook_dialect) {
1442+
/* Only Claude Code (the NULL dialect) consumes a bare stdout JSON object;
1443+
* emitting it into another dialect's channel would corrupt that protocol. */
1444+
if (hook_dialect) {
1445+
return NULL;
1446+
}
1447+
switch (reason) {
1448+
case CBM_HOOK_ADMISSION_DAEMON_ABSENT:
1449+
return "{\"systemMessage\":\"codebase-memory-mcp: no CBM daemon is running, so graph "
1450+
"augmentation is currently skipped. Run `codebase-memory-mcp daemon start` (or "
1451+
"open an MCP session) to enable it.\"}";
1452+
case CBM_HOOK_ADMISSION_BUILD_CONFLICT:
1453+
return "{\"systemMessage\":\"codebase-memory-mcp: graph augmentation is skipped: the "
1454+
"active CBM daemon runs a different build than this binary (usually an update was "
1455+
"installed while the old daemon kept running). Run `codebase-memory-mcp daemon "
1456+
"stop`, then retry - the next command starts a matching daemon.\"}";
1457+
default:
1458+
return NULL;
1459+
}
1460+
}

‎src/main.c‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,12 +1483,26 @@ static void main_hook_report_absent_daemon(const char *hook_dialect) {
14831483
(void)fprintf(stderr, "codebase-memory-mcp: no CBM daemon is running, so graph "
14841484
"augmentation is skipped. Start an MCP session or run "
14851485
"`codebase-memory-mcp daemon start` to enable it.\n");
1486-
if (!hook_dialect) {
1487-
/* Claude hook output: a systemMessage is surfaced to the user. */
1488-
(void)fputs("{\"systemMessage\":\"codebase-memory-mcp: no CBM daemon is running, so "
1489-
"graph augmentation is currently skipped. Run `codebase-memory-mcp daemon "
1490-
"start` (or open an MCP session) to enable it.\"}",
1491-
stdout);
1486+
const char *notice = cbm_hook_admission_notice(CBM_HOOK_ADMISSION_DAEMON_ABSENT, hook_dialect);
1487+
if (notice) {
1488+
(void)fputs(notice, stdout);
1489+
(void)fflush(stdout);
1490+
}
1491+
}
1492+
1493+
/* #1388: a version-conflicted daemon is an actionable broken state, unlike a
1494+
* merely absent one - and stdout is the only hook channel Claude Code
1495+
* surfaces, so stderr-only reporting reads as eternal silence in-session.
1496+
* Emit a throttled systemMessage with restart guidance (the misleading
1497+
* "no daemon is running" text pointed at `daemon start`, which cannot heal a
1498+
* build conflict). */
1499+
static void main_hook_report_conflicted_daemon(const char *hook_dialect) {
1500+
if (hook_dialect || !main_hook_absent_notice_due()) {
1501+
return;
1502+
}
1503+
const char *notice = cbm_hook_admission_notice(CBM_HOOK_ADMISSION_BUILD_CONFLICT, hook_dialect);
1504+
if (notice) {
1505+
(void)fputs(notice, stdout);
14921506
(void)fflush(stdout);
14931507
}
14941508
}
@@ -2148,6 +2162,18 @@ int main(int argc, char **argv) {
21482162
return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
21492163
}
21502164

2165+
#ifdef CBM_ENABLE_TEST_SEAMS
2166+
/* #1388 test seam: let one binary present a foreign build fingerprint as a
2167+
* hook client, so the daemon-conflict reporting path is testable without
2168+
* building a second binary. */
2169+
static char seam_hook_build[CBM_DAEMON_BUILD_FINGERPRINT_SIZE];
2170+
const char *seam_forced_build = cbm_safe_getenv("CBM_TEST_HOOK_CLIENT_BUILD", seam_hook_build,
2171+
sizeof(seam_hook_build), NULL);
2172+
if (role == CBM_DAEMON_PROCESS_HOOK_CLIENT && seam_forced_build && seam_forced_build[0]) {
2173+
identity.build_fingerprint = seam_hook_build;
2174+
}
2175+
#endif
2176+
21512177
cbm_version_cohort_manager_t *client_cohort_manager = cbm_version_cohort_manager_new(endpoint);
21522178
cbm_version_cohort_lease_t *client_cohort_lease = NULL;
21532179
cbm_daemon_conflict_t client_cohort_conflict;
@@ -2169,6 +2195,10 @@ int main(int argc, char **argv) {
21692195
}
21702196
(void)fprintf(stderr, "codebase-memory-mcp: %s\n",
21712197
formatted ? message : "client exact-build admission failed");
2198+
if (role == CBM_DAEMON_PROCESS_HOOK_CLIENT &&
2199+
client_cohort_status == CBM_VERSION_COHORT_CONFLICT) {
2200+
main_hook_report_conflicted_daemon(hook_dialect);
2201+
}
21722202
(void)main_version_cohort_close(&client_cohort_lease, &client_cohort_manager);
21732203
cbm_daemon_ipc_endpoint_free(endpoint);
21742204
return role == CBM_DAEMON_PROCESS_HOOK_CLIENT ? EXIT_SUCCESS : EXIT_FAILURE;
@@ -2181,7 +2211,16 @@ int main(int argc, char **argv) {
21812211
endpoint, &identity, MAIN_HOOK_CONNECT_TIMEOUT_MS, &hook_connect);
21822212
cbm_daemon_ipc_endpoint_free(endpoint);
21832213
if (!hook_client) {
2184-
main_hook_report_absent_daemon(hook_dialect);
2214+
if (hook_connect.status == CBM_DAEMON_RUNTIME_CONNECT_CONFLICT) {
2215+
char conflict_detail[CBM_DAEMON_CONFLICT_MESSAGE_SIZE];
2216+
if (cbm_daemon_conflict_format(&hook_connect.conflict, conflict_detail,
2217+
sizeof(conflict_detail))) {
2218+
(void)fprintf(stderr, "codebase-memory-mcp: %s\n", conflict_detail);
2219+
}
2220+
main_hook_report_conflicted_daemon(hook_dialect);
2221+
} else {
2222+
main_hook_report_absent_daemon(hook_dialect);
2223+
}
21852224
(void)main_version_cohort_close(&client_cohort_lease, &client_cohort_manager);
21862225
return EXIT_SUCCESS;
21872226
}

‎tests/test_cli.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4706,6 +4706,32 @@ TEST(cli_agent_reinstall_preserves_foreign_policy_entries) {
47064706
PASS();
47074707
}
47084708

4709+
/* Regression for #1388: a hook client blocked by a daemon BUILD CONFLICT must
4710+
* emit a stdout systemMessage. stdout is the only channel a hook caller sees,
4711+
* so the pre-fix stderr-only reporting was indistinguishable from "no matches"
4712+
* and produced silent skips for the whole session. The absent-daemon notice
4713+
* must stay distinct: it points at `daemon start`, which cannot heal a build
4714+
* conflict. Non-Claude dialects take no bare stdout JSON at all. */
4715+
TEST(cli_hook_conflict_emits_stdout_notice_issue1388) {
4716+
const char *conflict = cbm_hook_admission_notice(CBM_HOOK_ADMISSION_BUILD_CONFLICT, NULL);
4717+
ASSERT_NOT_NULL(conflict);
4718+
ASSERT_NOT_NULL(strstr(conflict, "systemMessage"));
4719+
ASSERT_NOT_NULL(strstr(conflict, "different build"));
4720+
/* The actionable step: a conflicted daemon must be STOPPED, not started. */
4721+
ASSERT_NOT_NULL(strstr(conflict, "daemon stop"));
4722+
4723+
const char *absent = cbm_hook_admission_notice(CBM_HOOK_ADMISSION_DAEMON_ABSENT, NULL);
4724+
ASSERT_NOT_NULL(absent);
4725+
ASSERT_NOT_NULL(strstr(absent, "daemon start"));
4726+
/* Distinct diagnoses: the conflict notice must never claim no daemon runs. */
4727+
ASSERT_TRUE(strcmp(conflict, absent) != 0);
4728+
ASSERT_NULL(strstr(conflict, "no CBM daemon is running"));
4729+
4730+
/* Other dialects do not consume a bare stdout JSON object. */
4731+
ASSERT_NULL(cbm_hook_admission_notice(CBM_HOOK_ADMISSION_BUILD_CONFLICT, "codex"));
4732+
ASSERT_NULL(cbm_hook_admission_notice(CBM_HOOK_ADMISSION_DAEMON_ABSENT, "codex"));
4733+
PASS();
4734+
}
47094735
#ifndef _WIN32
47104736
/* Regression for #1387: installing over an existing setup whose hook scripts
47114737
* are not byte-owned (manual install with a custom binary location, or a
@@ -12075,6 +12101,7 @@ SUITE(cli) {
1207512101
RUN_TEST(cli_new_agent_install_plans_use_documented_paths);
1207612102
RUN_TEST(cli_new_agent_configs_use_documented_schemas);
1207712103
RUN_TEST(cli_agent_reinstall_preserves_foreign_policy_entries);
12104+
RUN_TEST(cli_hook_conflict_emits_stdout_notice_issue1388);
1207812105
#ifndef _WIN32
1207912106
RUN_TEST(cli_install_preserves_hook_entries_when_scripts_unowned_issue1387);
1208012107
#endif

‎tests/test_hook_conflict_notice.sh‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
#!/usr/bin/env bash
2+
# #1388: a hook client that cannot join because the active daemon runs a
3+
# DIFFERENT build must say so on stdout (systemMessage) - stdout is the only
4+
# hook channel Claude Code surfaces, so stderr-only reporting reads as eternal
5+
# silence in-session. Requires a TEST_SEAMS=1 binary (scripts/test.sh step 5
6+
# builds one): CBM_TEST_HOOK_CLIENT_BUILD forces the client fingerprint.
7+
#
8+
# STATUS: LOCAL-ONLY, NOT WIRED INTO scripts/test.sh (see the Step 5d comment
9+
# there). WHY: on every CI leg this test's forced client/daemon build mismatch
10+
# produces no cohort conflict, so the notice under test is never triggered and
11+
# the assertion fails for a reason unrelated to the fix.
12+
# WHAT WAS TRIED (2026-08-05, PR #1441):
13+
# - hardened the probe loop to wait for the asserted state with a bounded
14+
# backstop instead of a single post-start probe (cohort join is async);
15+
# - asserted the seam's presence in the binary up front, mirroring
16+
# tests/test_worker_watchdog.sh - the seam IS present on CI;
17+
# - replaced a `seq`-derived fingerprint with a length-checked literal, in
18+
# case `seq` was absent on a leg - it was not the cause;
19+
# - dumped stdout, the forced fingerprint, and `daemon status` on expiry:
20+
# the daemon is active on a DIFFERENT build fingerprint, yet the forced
21+
# client still joins without a conflict, which does not happen locally.
22+
# The remaining suspect is the cohort admission path behaving differently under
23+
# the CI environment; that is recorded as a follow-up rather than papered over.
24+
set -euo pipefail
25+
26+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
27+
BINARY="${ROOT}/build/c/codebase-memory-mcp"
28+
if [[ ! -x "${BINARY}" && -x "${BINARY}.exe" ]]; then
29+
BINARY="${BINARY}.exe"
30+
fi
31+
if [[ ! -x "${BINARY}" ]]; then
32+
echo "missing binary: ${BINARY}" >&2
33+
exit 2
34+
fi
35+
36+
# PRECONDITION, not a skip (same doctrine as tests/test_worker_watchdog.sh): this
37+
# test drives the hook-client build seam, which is compiled out of ordinary
38+
# builds. Against a seam-less binary no conflict can ever occur and the failure
39+
# would read as "the notice is missing" instead of "the capability is absent".
40+
if ! LC_ALL=C grep -a -q -F 'CBM_TEST_HOOK_CLIENT_BUILD' "${BINARY}"; then
41+
echo "binary lacks the hook-client build seam: ${BINARY}" >&2
42+
echo " rebuild it with: make -f Makefile.cbm cbm TEST_SEAMS=1" >&2
43+
echo " or run this test through scripts/test.sh, which does that for you." >&2
44+
exit 2
45+
fi
46+
47+
tmpdir="$(mktemp -d)"
48+
cleanup() {
49+
CBM_CACHE_DIR="${tmpdir}/cache" "${BINARY}" daemon stop >/dev/null 2>&1 || true
50+
rm -rf "${tmpdir}"
51+
}
52+
trap cleanup EXIT
53+
54+
if ! CBM_CACHE_DIR="${tmpdir}/cache" "${BINARY}" daemon start >"${tmpdir}/daemon-start.log" 2>&1; then
55+
echo "daemon start failed on this host - cannot exercise the conflict path" >&2
56+
cat "${tmpdir}/daemon-start.log" >&2
57+
exit 2
58+
fi
59+
payload='{"session_id":"probe","hook_event_name":"PreToolUse","tool_name":"Grep","tool_input":{"pattern":"x","path":"/tmp"},"cwd":"/tmp"}'
60+
forced_build="ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
61+
if [[ ${#forced_build} -ne 64 ]]; then
62+
echo "internal: forced fingerprint must be 64 hex chars, got ${#forced_build}" >&2
63+
exit 2
64+
fi
65+
66+
# The daemon's cohort join completes asynchronously after `daemon start`
67+
# returns. Wait for the STATE the assertion needs - a probe that observes the
68+
# build conflict - with a bounded liveness backstop; never a bare sleep. The
69+
# throttled notice marker is cleared before each probe so the stdout
70+
# assertion stays valid on whichever probe first observes the conflict.
71+
rc=0
72+
out=""
73+
for _attempt in $(seq 1 40); do
74+
rm -f "${tmpdir}/cache/.hook-daemon-absent-notice"
75+
set +e
76+
out="$(printf '%s' "${payload}" | CBM_CACHE_DIR="${tmpdir}/cache" \
77+
CBM_TEST_HOOK_CLIENT_BUILD="${forced_build}" \
78+
"${BINARY}" hook-augment 2>"${tmpdir}/probe.err")"
79+
rc=$?
80+
set -e
81+
if [[ ${rc} -ne 0 ]]; then
82+
echo "hook-augment must fail open (exit 0); got ${rc}" >&2
83+
cat "${tmpdir}/probe.err" >&2
84+
exit 1
85+
fi
86+
if grep -q "conflicting CBM process" "${tmpdir}/probe.err"; then
87+
break
88+
fi
89+
sleep 0.5
90+
done
91+
92+
if ! grep -q "conflicting CBM process" "${tmpdir}/probe.err"; then
93+
echo "no probe observed the build conflict within the backstop - daemon cohort" >&2
94+
echo "never became active, or this is not a TEST_SEAMS=1 binary" >&2
95+
echo "--- daemon-start.log ---" >&2
96+
cat "${tmpdir}/daemon-start.log" >&2
97+
echo "--- last probe stderr (empty means no conflict was raised) ---" >&2
98+
cat "${tmpdir}/probe.err" >&2
99+
echo "--- last probe stdout ---" >&2
100+
printf '%s\n' "${out}" >&2
101+
echo "--- diagnosis ---" >&2
102+
echo "forced_build=${forced_build} (len ${#forced_build})" >&2
103+
echo "seam present in binary: yes (asserted above)" >&2
104+
CBM_CACHE_DIR="${tmpdir}/cache" "${BINARY}" daemon status >&2 2>&1 || true
105+
exit 2
106+
fi
107+
if ! printf '%s' "${out}" | grep -q "systemMessage"; then
108+
echo "conflicted daemon must surface a stdout systemMessage to the hook caller (#1388)" >&2
109+
echo "stdout was: ${out}" >&2
110+
exit 1
111+
fi
112+
if ! printf '%s' "${out}" | grep -q "daemon stop"; then
113+
echo "the systemMessage must carry the daemon-restart guidance (#1388)" >&2
114+
echo "stdout was: ${out}" >&2
115+
exit 1
116+
fi
117+
118+
echo "ok: daemon build conflicts reach the hook caller as a systemMessage"

0 commit comments

Comments
 (0)