Skip to content

Commit d61811d

Browse files
nguiaSorennguiaSoren
committed
test(watcher): process-level regression for watcher_enabled + startup-read docs (DeusData#335)
Follow-up to the PR DeusData#1105 review: the unit test proves the config predicate but would still pass even if the daemon-side gate were removed. Add a process-level regression that drives the real binary and proves the gate actually prevents the watcher from being built, started or registered when disabled. - tests/test_watcher_disabled.sh (wired into scripts/test.sh as Step 5e) drives the binary against an isolated CBM_CACHE_DIR + tiny git fixture. With watcher_enabled=false it asserts: watcher.disabled reason=config emitted, watcher.start absent, no watcher.watch registration, auto_index still runs, and manual index_repository is served. A watcher_enabled=true positive control asserts watcher.start + watcher.watch appear, so the test fails if the gate is removed (verified by neutering the gate in host_state_prepare and observing the failure). No assertion is decided by a timeout. The watcher lives in the daemon, which logs to $CBM_CACHE_DIR/logs/cbm-daemon.log in a fixed order: watcher.disabled (host_state_prepare) -> daemon.start -> the thread's own watcher.start -> the join -> daemon.stop. Each run therefore retires its daemon and waits for that CLOSED lifecycle before asserting, so absence is a fact rather than a race. Every wait is a bounded poll on an asserted state following the soak harness's wait_for_daemon_stop doctrine, and an exhausted budget fails loudly with the reason instead of passing quietly; session stdin is held open through a FIFO until the last JSON-RPC response lands, not for an interval. Registration is asynchronous and unreachable until the project DB exists (application_refresh_watch_locked requires application_regular_db_exists), so a closed lifecycle alone would not carry the negative — the session could simply have ended first. The disabled+auto_index run waits for that DB, i.e. registration's own precondition; the enabled control waits for watcher.watch itself, so its absence in the disabled runs is evidence rather than noise. Honors CBM_TEST_BINARY like tests/test_worker_watchdog.sh, so the BUILD_DIR= sanitizer and container legs test the binary they built. - docs: CONFIGURATION.md + README.md document that watcher_enabled is read once when the daemon starts — the daemon outlives individual MCP sessions, so reconnecting a client is not enough; `daemon stop` retires it. This corrects the earlier claim that auto_watch is also startup-read: auto_watch is consulted per registration, and the two keys are now contrasted on read time as well as on scope (session registration vs whether the subsystem starts at all). No production code change in this commit: watcher.start and watcher.watch already exist as production logs. Signed-off-by: soren <nguiasoren@gmail.com>
1 parent 680bc72 commit d61811d

4 files changed

Lines changed: 303 additions & 1 deletion

File tree

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ When enabled, new projects are indexed automatically on first connection. Previo
153153

154154
Watcher registration is controlled separately by `auto_watch` (default `true`). Set `config set auto_watch false` to keep a session from registering its project with the background watcher — useful when working across many projects and you want each session contained to explicit indexing.
155155

156+
To turn the watcher off entirely, set `config set watcher_enabled false` (default `true`): the background poll thread never starts and no project is registered, while `auto_index` and manual `index_repository` keep working. Unlike `auto_watch` — which is consulted per session — `watcher_enabled` is read once when the background daemon starts, so run `codebase-memory-mcp daemon stop` after changing it; reconnecting your MCP client alone will not restart the daemon. See [docs/CONFIGURATION.md](docs/CONFIGURATION.md#2-cli-managed-runtime-settings).
157+
156158
### Keeping Up to Date
157159

158160
**Updates run from the install script on every platform, not from inside the running binary.** `codebase-memory-mcp update` validates your flags and then prints the exact command to run:

‎docs/CONFIGURATION.md‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,33 @@ Current keys:
8888
| `auto_index` | `false` | Automatically index new projects when an MCP session starts. |
8989
| `auto_index_limit` | `50000` | Maximum file count allowed for automatic indexing of a new project. |
9090
| `auto_watch` | `true` | Register the session's project with the background git watcher on connect. Set `false` to keep a session from registering its project (the watcher still runs for other projects). |
91-
| `watcher_enabled` | `true` | Run the background watcher thread that auto-reindexes projects when they change. Set `false` to stop the watcher from starting at all — no poll thread and no project registration. Reindex manually with `index_repository` when disabled. |
91+
| `watcher_enabled` | `true` | Master switch for the background watcher subsystem. Set `false` to stop the watcher from starting at all — no poll thread and no project registration. Reindex manually with `index_repository` when disabled. |
92+
93+
> **`watcher_enabled` vs `auto_watch`.** `watcher_enabled` controls whether the
94+
> watcher *subsystem* starts at all (the background poll thread). `auto_watch` is
95+
> narrower: it only controls whether a connecting session registers *its own*
96+
> project with an already-running watcher. When `watcher_enabled=false`,
97+
> `auto_watch` has no effect — there is no watcher to register with.
98+
>
99+
> They also differ in **when they are read**, which matters because the watcher
100+
> lives in the background daemon, not in your MCP client:
101+
>
102+
> - `auto_watch` is consulted each time a session would register its project, so
103+
> a change applies to sessions that connect afterwards.
104+
> - `watcher_enabled` is read **once, when the daemon starts**, because it decides
105+
> whether the watcher is built at all. The daemon is long-lived and outlives
106+
> individual MCP sessions, so **reconnecting your client is not enough** — retire
107+
> the daemon so the next one picks the new value up:
108+
>
109+
> ```bash
110+
> codebase-memory-mcp config set watcher_enabled false
111+
> codebase-memory-mcp daemon stop # next session starts a daemon without the watcher
112+
> codebase-memory-mcp daemon status # confirm
113+
> ```
114+
>
115+
> Disabling the watcher does not disable anything else: the daemon still starts,
116+
> `auto_index` still runs, and `index_repository` stays available for manual
117+
> reindexing.
92118
93119
## 3. UI Settings
94120

‎scripts/test.sh‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,15 @@ CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_worker_error_response.
331331
# skipping it silently would hide the gap. Run it by hand:
332332
# make -f Makefile.cbm cbm TEST_SEAMS=1 && bash tests/test_hook_conflict_notice.sh
333333

334+
# Step 5e: watcher_enabled kill-switch process regression (#335). Reuses the
335+
# prod binary built in Step 5; drives a real daemon against an isolated cache
336+
# and proves watcher_enabled=false stops the watcher from being built, started
337+
# or registered, while auto_index and manual index_repository keep working.
338+
# Every wait is a bounded poll on an asserted state (a closed daemon lifecycle),
339+
# never a fixed sleep — see the header of the test for why.
340+
echo "=== Step 5e: watcher_enabled kill-switch regression (#335) ==="
341+
CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_watcher_disabled.sh"
342+
334343
# Step 6: security-strings URL allow-list regression. The MSYS2 CLANG64 toolchain
335344
# bakes its package-tracker URL into the static Windows .exe; the binary string
336345
# audit must allow-list it (Windows-only — Linux smoke never saw it).

‎tests/test_watcher_disabled.sh‎

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,265 @@
1+
#!/usr/bin/env bash
2+
# test_watcher_disabled.sh — process-level regression for the watcher_enabled
3+
# kill-switch (#335). Requested by the maintainer on PR #1105: the unit test
4+
# (cli_config_watcher_enabled_default_and_persist) only proves the config
5+
# predicate and would still pass if the daemon-side gate were deleted. This
6+
# drives the REAL binary against an isolated cache and proves, at the process
7+
# level, that watcher_enabled=false actually prevents the watcher subsystem from
8+
# initializing:
9+
# - `watcher.disabled reason=config` IS emitted,
10+
# - `watcher.start` is ABSENT (the poll thread never runs),
11+
# - no project registration occurs (`watcher.watch` ABSENT), and
12+
# - manual index_repository remains available (the MCP tool still serves and
13+
# indexes with the watcher off).
14+
# A positive control (watcher_enabled=true) proves those signals are real —
15+
# watcher.start + watcher.watch DO appear and watcher.disabled does not — so
16+
# this test fails if the gate is removed.
17+
#
18+
# NO ASSERTION IS DECIDED BY A TIMEOUT. The watcher lives in the daemon
19+
# (src/daemon/host.c), which logs to $CBM_CACHE_DIR/logs/cbm-daemon.log in a
20+
# fixed order: watcher.disabled (host_state_prepare) -> daemon.start -> the
21+
# watcher thread's own watcher.start -> ... -> watcher thread joined ->
22+
# daemon.stop. Each run therefore retires its daemon and waits for that CLOSED
23+
# lifecycle (daemon.start ... daemon.stop) before asserting. Absence over a
24+
# closed lifecycle log is a fact, not a race: if watcher.start is not in a log
25+
# that already contains daemon.stop, the thread never ran. Every wait is a
26+
# bounded poll on an asserted state (the soak harness's wait_for_daemon_stop
27+
# doctrine, scripts/soak-test.sh), and exhausting a poll is a FAILURE with the
28+
# reason printed — never a silently-passing sleep.
29+
#
30+
# Skipped on Windows-like shells (uses POSIX process control + git fixture).
31+
set -euo pipefail
32+
33+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
34+
BINARY="${CBM_TEST_BINARY:-${ROOT}/build/c/codebase-memory-mcp}"
35+
36+
case "$(uname -s)" in
37+
MINGW*|MSYS*|CYGWIN*) echo "skipping watcher_disabled test on Windows"; exit 0 ;;
38+
esac
39+
[[ -x "${BINARY}" ]] || { echo "missing binary: ${BINARY}" >&2; exit 2; }
40+
command -v git >/dev/null 2>&1 || { echo "git required for fixture" >&2; exit 2; }
41+
42+
work="$(mktemp -d)"
43+
44+
# Every run gets its own CBM_CACHE_DIR, and the daemon endpoint is derived from
45+
# it, so these daemons are private to the test and can never touch a developer's
46+
# real one. Retire them on any exit path regardless.
47+
cleanup() {
48+
local cache
49+
for cache in "${work}"/cache-*; do
50+
[[ -d "${cache}" ]] || continue
51+
CBM_CACHE_DIR="${cache}" "${BINARY}" daemon stop >/dev/null 2>&1 || true
52+
done
53+
rm -rf "${work}"
54+
}
55+
trap cleanup EXIT
56+
57+
# --- tiny git fixture so indexing is fast + deterministic ---------------------
58+
repo="${work}/repo"
59+
mkdir -p "${repo}"
60+
cat >"${repo}/sample.c" <<'EOF'
61+
int add(int a, int b) { return a + b; }
62+
int main(void) { return add(1, 2); }
63+
EOF
64+
git -C "${repo}" init -q
65+
git -C "${repo}" -c user.email=t@example.com -c user.name=t add -A
66+
git -C "${repo}" -c user.email=t@example.com -c user.name=t commit -q -m init
67+
68+
INIT='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"watcher-test","version":"1.0"}}}'
69+
INITED='{"jsonrpc":"2.0","method":"notifications/initialized"}'
70+
71+
CURRENT_RUN="(setup)"
72+
73+
fail() {
74+
echo "FAIL [${CURRENT_RUN}]: $*" >&2
75+
local log
76+
for log in "${work}"/cache-*/logs/cbm-daemon.log; do
77+
[[ -f "${log}" ]] || continue
78+
echo "----- ${log} (watcher/daemon lines) -----" >&2
79+
grep -E 'msg=(watcher|daemon)\.' "${log}" >&2 || true
80+
done
81+
exit 1
82+
}
83+
84+
# wait_for <file> <extended-regex> <label>
85+
# Bounded poll on an asserted state: returns as soon as the state is observed,
86+
# and a exhausted budget is a failure with the reason — never a fixed sleep that
87+
# lets the assertion proceed on an unobserved state. 30s ceiling mirrors
88+
# scripts/soak-test.sh (attempts=300 x 0.1s): the wait sits above the worst case
89+
# so a slow, loaded CI runner is not mistaken for a broken gate.
90+
wait_for() {
91+
local file="$1" pattern="$2" label="$3"
92+
local attempts=300
93+
while [ "${attempts}" -gt 0 ]; do
94+
if [ -f "${file}" ] && grep -qE "${pattern}" "${file}"; then
95+
return 0
96+
fi
97+
attempts=$((attempts - 1))
98+
sleep 0.1
99+
done
100+
fail "timed out after 30s waiting for ${label} in ${file}"
101+
}
102+
103+
log_has() { grep -qE "$2" "$1"; }
104+
105+
# wait_for_project_db <cache_dir>
106+
# Registration is not reachable until the project DB exists: the daemon only
107+
# refreshes a session's watch once application_regular_db_exists(project) is
108+
# true (application.c application_refresh_watch_locked). Waiting on the DB is
109+
# therefore waiting on registration's own precondition — after this returns, a
110+
# missing watcher.watch is the gate's doing, not a session that ended too soon.
111+
wait_for_project_db() {
112+
local cache="$1" attempts=300 db
113+
while [ "${attempts}" -gt 0 ]; do
114+
for db in "${cache}"/*.db; do
115+
# An unmatched glob stays literal, and _config.db is the settings store,
116+
# not a project graph.
117+
case "${db}" in
118+
*/_config.db) continue ;;
119+
*'*.db') continue ;;
120+
esac
121+
if [ -f "${db}" ]; then
122+
return 0
123+
fi
124+
done
125+
attempts=$((attempts - 1))
126+
sleep 0.1
127+
done
128+
fail "timed out after 30s waiting for the project DB under ${cache}"
129+
}
130+
131+
# run_session <cache_dir> <outfile> [extra rpc line ...]
132+
# Drives the real binary in MCP stdio mode with cwd = the fixture repo (so the
133+
# session root is derived from it), holding stdin open via a FIFO until the LAST
134+
# request has been answered on stdout — an asserted state, not an interval —
135+
# then closing it (EOF -> clean shutdown). Sets DAEMON_LOG for the caller.
136+
#
137+
# Registration happens asynchronously, after indexing, so a closed lifecycle
138+
# alone would not carry a NEGATIVE registration assertion — the session could
139+
# simply have ended before registration was ever reachable. Two optional
140+
# pre-close waits close that gap, both observed while the session is still live:
141+
# PRE_CLOSE_DB=1 wait for the project DB, which is registration's own
142+
# precondition (see wait_for_project_db).
143+
# PRE_CLOSE_WAIT=<re> wait for a daemon-log pattern (with PRE_CLOSE_LABEL),
144+
# used by the positive control to observe registration
145+
# actually happening in this session shape.
146+
run_session() {
147+
local cache="$1" outf="$2"; shift 2
148+
local fifo="${work}/stdin.fifo"
149+
rm -f "${fifo}"; mkfifo "${fifo}"
150+
DAEMON_LOG="${cache}/logs/cbm-daemon.log"
151+
152+
( cd "${repo}" && CBM_CACHE_DIR="${cache}" "${BINARY}" <"${fifo}" \
153+
>"${outf}" 2>"${cache}.client.err" ) &
154+
local server_pid=$!
155+
156+
exec 3>"${fifo}"
157+
printf '%s\n' "${INIT}" >&3
158+
printf '%s\n' "${INITED}" >&3
159+
local line last_id=1
160+
for line in "$@"; do
161+
printf '%s\n' "${line}" >&3
162+
last_id=$((last_id + 1))
163+
done
164+
# Wait for the final response before closing stdin, so shutdown never races
165+
# the work under test.
166+
wait_for "${outf}" "\"id\":${last_id}[,}]" "the id=${last_id} JSON-RPC response"
167+
if [ -n "${PRE_CLOSE_DB:-}" ]; then
168+
wait_for_project_db "${cache}"
169+
fi
170+
if [ -n "${PRE_CLOSE_WAIT:-}" ]; then
171+
wait_for "${DAEMON_LOG}" "${PRE_CLOSE_WAIT}" "${PRE_CLOSE_LABEL:-${PRE_CLOSE_WAIT}}"
172+
fi
173+
exec 3>&-
174+
wait "${server_pid}" || true
175+
176+
# Retire the daemon and wait for the lifecycle to close, so the assertions
177+
# below read a complete log rather than a snapshot of one still being written.
178+
CBM_CACHE_DIR="${cache}" "${BINARY}" daemon stop >/dev/null 2>&1 || true
179+
wait_for "${DAEMON_LOG}" 'msg=daemon\.start( |$)' "the daemon to record its start"
180+
wait_for "${DAEMON_LOG}" 'msg=daemon\.stop( |$)' "the daemon lifecycle to close"
181+
}
182+
183+
# =============================================================================
184+
# Run 1 — DISABLED + manual index_repository (auto_index off, no contention).
185+
# Proves: gate fires, watcher never starts, and the manual MCP tool still
186+
# serves + indexes with the watcher off.
187+
# =============================================================================
188+
CURRENT_RUN="run 1: disabled + manual index_repository"
189+
c_offm="${work}/cache-offm"
190+
CBM_CACHE_DIR="${c_offm}" "${BINARY}" config set watcher_enabled false >/dev/null
191+
CBM_CACHE_DIR="${c_offm}" "${BINARY}" config set auto_index false >/dev/null
192+
run_session "${c_offm}" "${work}/offm.out" \
193+
"{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/call\",\"params\":{\"name\":\"index_repository\",\"arguments\":{\"repo_path\":\"${repo}\",\"mode\":\"fast\"}}}"
194+
195+
log_has "${DAEMON_LOG}" 'msg=watcher\.disabled( |$)' \
196+
|| fail "watcher.disabled not emitted when disabled"
197+
log_has "${DAEMON_LOG}" 'msg=watcher\.disabled .*reason=config' \
198+
|| fail "watcher.disabled emitted without reason=config"
199+
! log_has "${DAEMON_LOG}" 'msg=watcher\.start( |$)' \
200+
|| fail "watcher.start present when disabled (the poll thread ran)"
201+
! log_has "${DAEMON_LOG}" 'msg=watcher\.watch( |$)' \
202+
|| fail "watcher.watch (registration) present when disabled"
203+
204+
resp="$(grep '"id":2' "${work}/offm.out" | head -1)"
205+
[ -n "${resp}" ] || fail "no index_repository response (tool unavailable when watcher off)"
206+
case "${resp}" in
207+
*'"error"'*) fail "index_repository returned an error with the watcher off: ${resp}" ;;
208+
esac
209+
case "${resp}" in
210+
*nodes*) : ;;
211+
*) fail "index_repository response reports no indexed nodes: ${resp}" ;;
212+
esac
213+
echo "ok: disabled — watcher.disabled emitted, no watcher.start/watch, manual index_repository served"
214+
215+
# =============================================================================
216+
# Run 2 — DISABLED + auto_index on. Same-config apples-to-apples for the
217+
# registration axis: indexing still runs, but NO registration happens.
218+
# =============================================================================
219+
CURRENT_RUN="run 2: disabled + auto_index"
220+
c_offa="${work}/cache-offa"
221+
CBM_CACHE_DIR="${c_offa}" "${BINARY}" config set watcher_enabled false >/dev/null
222+
CBM_CACHE_DIR="${c_offa}" "${BINARY}" config set auto_index true >/dev/null
223+
# Hold the session open until auto-indexing has produced the project DB, so
224+
# registration's precondition is satisfied and the absence below is the gate.
225+
PRE_CLOSE_DB=1
226+
run_session "${c_offa}" "${work}/offa.out"
227+
PRE_CLOSE_DB=""
228+
229+
log_has "${DAEMON_LOG}" 'msg=watcher\.disabled( |$)' \
230+
|| fail "watcher.disabled not emitted (auto_index run)"
231+
# Contract point 3 is broader than the manual tool: turning the watcher off must
232+
# not turn auto-indexing off with it. The DB above proves the pipeline ran.
233+
log_has "${DAEMON_LOG}" 'msg=daemon\.start( |$)' \
234+
|| fail "daemon did not start with the watcher disabled"
235+
! log_has "${DAEMON_LOG}" 'msg=watcher\.start( |$)' \
236+
|| fail "watcher.start present when disabled (auto_index run)"
237+
! log_has "${DAEMON_LOG}" 'msg=watcher\.watch( |$)' \
238+
|| fail "project registered with the watcher when disabled"
239+
echo "ok: disabled+auto_index — auto-index still ran, no watcher.start, no registration"
240+
241+
# =============================================================================
242+
# Run 3 — ENABLED positive control (auto_index on). Proves the signals above
243+
# are real: the watcher starts AND registers the session project — so their
244+
# absence in Runs 1-2 is meaningful, and removing the gate fails this test.
245+
# =============================================================================
246+
CURRENT_RUN="run 3: enabled positive control"
247+
c_on="${work}/cache-on"
248+
CBM_CACHE_DIR="${c_on}" "${BINARY}" config set watcher_enabled true >/dev/null
249+
CBM_CACHE_DIR="${c_on}" "${BINARY}" config set auto_index true >/dev/null
250+
# The control must observe registration actually happening in this exact session
251+
# shape — that is what makes its absence in runs 1-2 evidence rather than noise.
252+
PRE_CLOSE_WAIT='msg=watcher\.watch( |$)'
253+
PRE_CLOSE_LABEL="the session project to register with the watcher"
254+
run_session "${c_on}" "${work}/on.out"
255+
PRE_CLOSE_WAIT=""
256+
257+
log_has "${DAEMON_LOG}" 'msg=watcher\.start( |$)' \
258+
|| fail "watcher.start absent when enabled (the watcher never started)"
259+
log_has "${DAEMON_LOG}" 'msg=watcher\.watch( |$)' \
260+
|| fail "no project registration when enabled (control failed)"
261+
! log_has "${DAEMON_LOG}" 'msg=watcher\.disabled( |$)' \
262+
|| fail "watcher.disabled present when enabled"
263+
echo "ok: enabled — watcher.start emitted and session project registered"
264+
265+
echo "PASS: watcher_enabled kill-switch process regression (#335)"

0 commit comments

Comments
 (0)