diff --git a/README.md b/README.md index 7a32ce401..925f4929e 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,8 @@ When enabled, new projects are indexed automatically on first connection. Previo 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. +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). + ### Keeping Up to Date **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: @@ -660,6 +662,7 @@ codebase-memory-mcp config list # show all settings codebase-memory-mcp config set auto_index true # auto-index on session start codebase-memory-mcp config set auto_index_limit 50000 # max files for auto-index codebase-memory-mcp config set auto_watch false # don't register background git watcher (default: true) +codebase-memory-mcp config set watcher_enabled false # stop the watcher thread entirely (default: true) codebase-memory-mcp config reset auto_index # reset to default ``` diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index e8dc608c3..d97cb0310 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -77,6 +77,7 @@ codebase-memory-mcp config list codebase-memory-mcp config get auto_index codebase-memory-mcp config set auto_index true codebase-memory-mcp config set auto_index_limit 50000 +codebase-memory-mcp config set watcher_enabled false codebase-memory-mcp config reset auto_index ``` @@ -86,6 +87,34 @@ Current keys: |---|---|---| | `auto_index` | `false` | Automatically index new projects when an MCP session starts. | | `auto_index_limit` | `50000` | Maximum file count allowed for automatic indexing of a new project. | +| `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). | +| `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. | + +> **`watcher_enabled` vs `auto_watch`.** `watcher_enabled` controls whether the +> watcher *subsystem* starts at all (the background poll thread). `auto_watch` is +> narrower: it only controls whether a connecting session registers *its own* +> project with an already-running watcher. When `watcher_enabled=false`, +> `auto_watch` has no effect — there is no watcher to register with. +> +> They also differ in **when they are read**, which matters because the watcher +> lives in the background daemon, not in your MCP client: +> +> - `auto_watch` is consulted each time a session would register its project, so +> a change applies to sessions that connect afterwards. +> - `watcher_enabled` is read **once, when the daemon starts**, because it decides +> whether the watcher is built at all. The daemon is long-lived and outlives +> individual MCP sessions, so **reconnecting your client is not enough** — retire +> the daemon so the next one picks the new value up: +> +> ```bash +> codebase-memory-mcp config set watcher_enabled false +> codebase-memory-mcp daemon stop # next session starts a daemon without the watcher +> codebase-memory-mcp daemon status # confirm +> ``` +> +> Disabling the watcher does not disable anything else: the daemon still starts, +> `auto_index` still runs, and `index_repository` stays available for manual +> reindexing. ## 3. UI Settings diff --git a/scripts/test.sh b/scripts/test.sh index 3df2c3431..fc20f0103 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -276,6 +276,15 @@ CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_parent_watchdog.sh" echo "=== Step 5b: worker-mode watchdog regression (#845) ===" CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_worker_watchdog.sh" +# Step 5c: watcher_enabled kill-switch process regression (#335). Reuses the +# prod binary built in Step 5; drives a real daemon against an isolated cache +# and proves watcher_enabled=false stops the watcher from being built, started +# or registered, while auto_index and manual index_repository keep working. +# Every wait is a bounded poll on an asserted state (a closed daemon lifecycle), +# never a fixed sleep — see the header of the test for why. +echo "=== Step 5c: watcher_enabled kill-switch regression (#335) ===" +CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_watcher_disabled.sh" + # Step 6: security-strings URL allow-list regression. The MSYS2 CLANG64 toolchain # bakes its package-tracker URL into the static Windows .exe; the binary string # audit must allow-list it (Windows-only — Linux smoke never saw it). diff --git a/src/cli/cli.c b/src/cli/cli.c index 8010e2258..5605574f5 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -6442,6 +6442,14 @@ int cbm_config_delete(cbm_config_t *cfg, const char *key) { return rc; } +/* Whether the background watcher subsystem should run (default true). The + * daemon host gates watcher construction and thread startup on this; see + * cbm_config_watcher_enabled in cli.h. NULL-safe via cbm_config_get_bool (a + * NULL cfg returns the default). */ +bool cbm_config_watcher_enabled(cbm_config_t *cfg) { + return cbm_config_get_bool(cfg, CBM_CONFIG_WATCHER_ENABLED, true); +} + /* ── Config CLI subcommand ────────────────────────────────────── */ int cbm_cmd_config(int argc, char **argv) { @@ -6461,6 +6469,8 @@ int cbm_cmd_config(int argc, char **argv) { "Register background git watcher on session connect"); printf(" %-25s default=%-10s %s\n", CBM_CONFIG_UI_LANG, "auto", "Pin graph UI language: en, zh, or auto"); + printf(" %-25s default=%-10s %s\n", CBM_CONFIG_WATCHER_ENABLED, "true", + "Run the background watcher thread (auto-reindex); false to disable"); return 0; } @@ -6490,6 +6500,8 @@ int cbm_cmd_config(int argc, char **argv) { cbm_config_get(cfg, CBM_CONFIG_AUTO_WATCH, "true")); printf(" %-25s = %-10s\n", CBM_CONFIG_UI_LANG, cbm_config_get(cfg, CBM_CONFIG_UI_LANG, "auto")); + printf(" %-25s = %-10s\n", CBM_CONFIG_WATCHER_ENABLED, + cbm_config_get(cfg, CBM_CONFIG_WATCHER_ENABLED, "true")); } else if (strcmp(argv[0], "get") == 0) { if (argc < MIN_ARGC_GET) { (void)fprintf(stderr, "Usage: config get \n"); diff --git a/src/cli/cli.h b/src/cli/cli.h index 27ef4e3ec..a8b25b842 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -395,6 +395,16 @@ int cbm_config_delete(cbm_config_t *cfg, const char *key); #define CBM_CONFIG_AUTO_INDEX_LIMIT "auto_index_limit" #define CBM_CONFIG_AUTO_WATCH "auto_watch" #define CBM_CONFIG_UI_LANG "ui-lang" +#define CBM_CONFIG_WATCHER_ENABLED "watcher_enabled" + +/* Whether the background watcher subsystem should run at all (default true). + * When false, the daemon host skips building and starting the watcher entirely: + * the poll thread never starts and no projects are registered (#335). Read once + * at daemon startup. Distinct from auto_watch, which only gates per-session + * registration while the watcher IS running. NULL-safe — a NULL cfg returns the + * default (true), so a failure to open the config store never silently disables + * the watcher. */ +bool cbm_config_watcher_enabled(cbm_config_t *cfg); /* ── Binary activation safety ─────────────────────────────────── */ diff --git a/src/daemon/host.c b/src/daemon/host.c index 61dcf14af..4aaf6adfd 100644 --- a/src/daemon/host.c +++ b/src/daemon/host.c @@ -573,9 +573,22 @@ static bool host_state_prepare(host_state_t *host, const cbm_daemon_ipc_endpoint cbm_log_error("daemon.runtime_config_open_failed", "reason", "config_db_unavailable"); return false; } - host->watch_store = cbm_store_open_memory(); host->project_locks = cbm_project_lock_manager_new(endpoint); - host->watcher = cbm_watcher_new(host->watch_store, host_watcher_index, host); + /* #335: watcher_enabled (default true) is the master switch for the + * background watcher. When false the daemon never builds the watcher or the + * in-memory store backing it, so the poll thread never starts + * (host_background_start has nothing to run) and no project ever registers + * (register_watcher_if_enabled early-returns on a NULL watcher). The daemon + * still starts and still owns everything else: IPC, HTTP/UI, project locks, + * and manual index_repository. Read once here at daemon startup, so a change + * takes effect when the daemon next starts — see docs/CONFIGURATION.md. */ + bool watcher_enabled = cbm_config_watcher_enabled(host->runtime_config); + if (watcher_enabled) { + host->watch_store = cbm_store_open_memory(); + host->watcher = cbm_watcher_new(host->watch_store, host_watcher_index, host); + } else { + cbm_log_info("watcher.disabled", "reason", "config"); + } cbm_daemon_application_config_t application_config = { .watcher = host->watcher, .config = host->runtime_config, @@ -586,7 +599,12 @@ static bool host_state_prepare(host_state_t *host, const cbm_daemon_ipc_endpoint if (host->application && host->permanent) { cbm_daemon_application_set_permanent(host->application, true); } - if (!host->watch_store || !host->watcher || !host->project_locks || !host->application) { + if (!host->project_locks || !host->application) { + return false; + } + /* A watcher deliberately left unbuilt by watcher_enabled=false is not a + * startup failure; an allocation failure while it is enabled still is. */ + if (watcher_enabled && (!host->watch_store || !host->watcher)) { return false; } return true; @@ -812,10 +830,14 @@ bool cbm_daemon_host_http_thread_create_failure_lifecycle_for_test(void) { } static bool host_background_start(host_state_t *host) { - if (cbm_thread_create(&host->watcher_thread, 0, host_watcher_thread, host->watcher) != 0) { - return false; + /* No watcher object when watcher_enabled=false (#335) — nothing to run, and + * the daemon must still come up with its remaining subsystems. */ + if (host->watcher) { + if (cbm_thread_create(&host->watcher_thread, 0, host_watcher_thread, host->watcher) != 0) { + return false; + } + host->watcher_started = true; } - host->watcher_started = true; host_http_reconcile_at(host, cbm_now_ms(), true); return true; diff --git a/tests/test_cli.c b/tests/test_cli.c index 3edb18c84..1e7fda33c 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -11379,6 +11379,55 @@ TEST(cli_config_persists) { PASS(); } +TEST(cli_config_watcher_enabled_default_and_persist) { + /* #335: the background watcher is on by default and can be disabled via the + * persisted `watcher_enabled` key. This pins the config layer only — default + * preservation, the accepted spellings, and persistence across reopen. That + * the daemon host actually honours the key (watcher never built, thread + * never started, no registration) is proven separately by the process-level + * regression in tests/test_watcher_disabled.sh, which fails if the gate in + * host_state_prepare() is removed. */ + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-cfg-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + /* A NULL config (store failed to open) defaults to ON — a config failure + * must never silently disable the watcher. */ + ASSERT_TRUE(cbm_config_watcher_enabled(NULL)); + + cbm_config_t *cfg = cbm_config_open(tmpdir); + ASSERT_NOT_NULL(cfg); + + /* Default: absent key → watcher runs. */ + ASSERT_TRUE(cbm_config_watcher_enabled(cfg)); + + /* Disable: false / 0 / off all turn the watcher off. */ + cbm_config_set(cfg, CBM_CONFIG_WATCHER_ENABLED, "false"); + ASSERT_FALSE(cbm_config_watcher_enabled(cfg)); + cbm_config_set(cfg, CBM_CONFIG_WATCHER_ENABLED, "0"); + ASSERT_FALSE(cbm_config_watcher_enabled(cfg)); + cbm_config_set(cfg, CBM_CONFIG_WATCHER_ENABLED, "off"); + ASSERT_FALSE(cbm_config_watcher_enabled(cfg)); + + /* Re-enable: true / 1 / on all turn it back on. */ + cbm_config_set(cfg, CBM_CONFIG_WATCHER_ENABLED, "on"); + ASSERT_TRUE(cbm_config_watcher_enabled(cfg)); + cbm_config_set(cfg, CBM_CONFIG_WATCHER_ENABLED, "1"); + ASSERT_TRUE(cbm_config_watcher_enabled(cfg)); + + /* Persistence: the disabled state survives close + reopen. */ + cbm_config_set(cfg, CBM_CONFIG_WATCHER_ENABLED, "false"); + cbm_config_close(cfg); + cfg = cbm_config_open(tmpdir); + ASSERT_NOT_NULL(cfg); + ASSERT_FALSE(cbm_config_watcher_enabled(cfg)); + + cbm_config_close(cfg); + test_rmdir_r(tmpdir); + PASS(); +} + /* ═══════════════════════════════════════════════════════════════════ * Group H: cbm_replace_binary (update command helper) * ═══════════════════════════════════════════════════════════════════ */ @@ -12080,6 +12129,7 @@ SUITE(cli) { RUN_TEST(cli_config_get_int); RUN_TEST(cli_config_delete); RUN_TEST(cli_config_persists); + RUN_TEST(cli_config_watcher_enabled_default_and_persist); /* Replace binary (update command helper — group H) */ #ifndef _WIN32 diff --git a/tests/test_watcher_disabled.sh b/tests/test_watcher_disabled.sh new file mode 100755 index 000000000..23d91ae74 --- /dev/null +++ b/tests/test_watcher_disabled.sh @@ -0,0 +1,265 @@ +#!/usr/bin/env bash +# test_watcher_disabled.sh — process-level regression for the watcher_enabled +# kill-switch (#335). Requested by the maintainer on PR #1105: the unit test +# (cli_config_watcher_enabled_default_and_persist) only proves the config +# predicate and would still pass if the daemon-side gate were deleted. This +# drives the REAL binary against an isolated cache and proves, at the process +# level, that watcher_enabled=false actually prevents the watcher subsystem from +# initializing: +# - `watcher.disabled reason=config` IS emitted, +# - `watcher.start` is ABSENT (the poll thread never runs), +# - no project registration occurs (`watcher.watch` ABSENT), and +# - manual index_repository remains available (the MCP tool still serves and +# indexes with the watcher off). +# A positive control (watcher_enabled=true) proves those signals are real — +# watcher.start + watcher.watch DO appear and watcher.disabled does not — so +# this test fails if the gate is removed. +# +# NO ASSERTION IS DECIDED BY A TIMEOUT. The watcher lives in the daemon +# (src/daemon/host.c), which logs to $CBM_CACHE_DIR/logs/cbm-daemon.log in a +# fixed order: watcher.disabled (host_state_prepare) -> daemon.start -> the +# watcher thread's own watcher.start -> ... -> watcher thread joined -> +# daemon.stop. Each run therefore retires its daemon and waits for that CLOSED +# lifecycle (daemon.start ... daemon.stop) before asserting. Absence over a +# closed lifecycle log is a fact, not a race: if watcher.start is not in a log +# that already contains daemon.stop, the thread never ran. Every wait is a +# bounded poll on an asserted state (the soak harness's wait_for_daemon_stop +# doctrine, scripts/soak-test.sh), and exhausting a poll is a FAILURE with the +# reason printed — never a silently-passing sleep. +# +# Skipped on Windows-like shells (uses POSIX process control + git fixture). +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +BINARY="${CBM_TEST_BINARY:-${ROOT}/build/c/codebase-memory-mcp}" + +case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) echo "skipping watcher_disabled test on Windows"; exit 0 ;; +esac +[[ -x "${BINARY}" ]] || { echo "missing binary: ${BINARY}" >&2; exit 2; } +command -v git >/dev/null 2>&1 || { echo "git required for fixture" >&2; exit 2; } + +work="$(mktemp -d)" + +# Every run gets its own CBM_CACHE_DIR, and the daemon endpoint is derived from +# it, so these daemons are private to the test and can never touch a developer's +# real one. Retire them on any exit path regardless. +cleanup() { + local cache + for cache in "${work}"/cache-*; do + [[ -d "${cache}" ]] || continue + CBM_CACHE_DIR="${cache}" "${BINARY}" daemon stop >/dev/null 2>&1 || true + done + rm -rf "${work}" +} +trap cleanup EXIT + +# --- tiny git fixture so indexing is fast + deterministic --------------------- +repo="${work}/repo" +mkdir -p "${repo}" +cat >"${repo}/sample.c" <<'EOF' +int add(int a, int b) { return a + b; } +int main(void) { return add(1, 2); } +EOF +git -C "${repo}" init -q +git -C "${repo}" -c user.email=t@example.com -c user.name=t add -A +git -C "${repo}" -c user.email=t@example.com -c user.name=t commit -q -m init + +INIT='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"watcher-test","version":"1.0"}}}' +INITED='{"jsonrpc":"2.0","method":"notifications/initialized"}' + +CURRENT_RUN="(setup)" + +fail() { + echo "FAIL [${CURRENT_RUN}]: $*" >&2 + local log + for log in "${work}"/cache-*/logs/cbm-daemon.log; do + [[ -f "${log}" ]] || continue + echo "----- ${log} (watcher/daemon lines) -----" >&2 + grep -E 'msg=(watcher|daemon)\.' "${log}" >&2 || true + done + exit 1 +} + +# wait_for