diff --git a/README.md b/README.md index bb4919ed5..2159a21d1 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,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 `index_repository` stays available for manual reindexing. Both `watcher_enabled` and `auto_watch` are read at process startup, so a change takes effect the next time the server starts (restart or reconnect your MCP client). + ### Keeping Up to Date ```bash @@ -599,6 +601,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 c5cb96eb8..084d8ed3e 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -72,6 +72,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 ``` @@ -81,6 +82,19 @@ 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. +> +> Both keys are read **once at process startup**. Changing either takes effect on +> the next server start: restart the `codebase-memory-mcp` process or reconnect +> your MCP client. It does not affect an already-running server. Use +> `index_repository` to reindex manually while the watcher is disabled. ## 3. UI Settings diff --git a/scripts/test.sh b/scripts/test.sh index 1254419a6..5e7fc5e69 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -81,6 +81,13 @@ bash "$ROOT/tests/test_parent_watchdog.sh" echo "=== Step 5b: worker-mode watchdog regression (#845) ===" 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 the real MCP server with an isolated +# cache and proves watcher_enabled=false stops the watcher thread from starting +# and registering, while manual index_repository stays available. +echo "=== Step 5c: watcher_enabled kill-switch regression (#335) ===" +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 aec3cf102..92aac6284 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -5159,6 +5159,13 @@ int cbm_config_delete(cbm_config_t *cfg, const char *key) { return rc; } +/* Whether the background watcher subsystem should run (default true). main() + * gates watcher-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) { @@ -5178,6 +5185,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; } @@ -5207,6 +5216,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 de1ac04a2..6036321ad 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -365,6 +365,15 @@ 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, main() skips creating and starting the watcher entirely: the + * poll thread never starts and no projects are registered (#335). 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); /* ── Subcommands (wired from main.c) ─────────────────────────── */ diff --git a/src/main.c b/src/main.c index 01083e809..54dfa977e 100644 --- a/src/main.c +++ b/src/main.c @@ -793,10 +793,21 @@ int main(int argc, char **argv) { cbm_store_t *watch_store = NULL; if (!restricted_tool_profile) { - watch_store = cbm_store_open_memory(); - g_watcher = cbm_watcher_new(watch_store, watcher_index_fn, NULL); - /* Wire watcher + config into MCP server for session auto-index. */ - cbm_mcp_server_set_watcher(g_server, g_watcher); + /* #335: watcher_enabled (default true) is the master switch for the + * background watcher. When false, skip creating it entirely — the poll + * thread never starts (the thread-create below is gated on g_watcher) + * and no projects register (register_watcher_if_enabled early-returns + * on a NULL watcher). Manual index_repository is unaffected. The config + * wiring below stays unconditional so auto_index / auto_watch keep + * working even when the watcher is off. */ + if (cbm_config_watcher_enabled(runtime_config)) { + watch_store = cbm_store_open_memory(); + g_watcher = cbm_watcher_new(watch_store, watcher_index_fn, NULL); + /* Wire watcher into MCP server for session auto-index. */ + cbm_mcp_server_set_watcher(g_server, g_watcher); + } else { + cbm_log_info("watcher.disabled", "reason", "config"); + } cbm_mcp_server_set_config(g_server, runtime_config); } cbm_thread_t watcher_tid; diff --git a/tests/test_cli.c b/tests/test_cli.c index bb8a94b6f..93af2fa42 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -9408,6 +9408,53 @@ 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. cbm_config_watcher_enabled() is the seam + * main() gates watcher-thread startup on, so asserting it here proves the + * watcher is NOT initialized when disabled — without spinning up the real + * server (main() is not linked into the test binary). */ + 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) * ═══════════════════════════════════════════════════════════════════ */ @@ -9915,13 +9962,14 @@ SUITE(cli) { /* Skill directive descriptions (1 test — group E) */ RUN_TEST(cli_skill_descriptions_directive); - /* Config store (6 tests — group F) */ + /* Config store (7 tests — group F) */ RUN_TEST(cli_config_open_close); RUN_TEST(cli_config_get_set); RUN_TEST(cli_config_get_bool); 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..7e4e69c9d --- /dev/null +++ b/tests/test_watcher_disabled.sh @@ -0,0 +1,124 @@ +#!/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 main()'s gate were deleted. This drives the +# REAL server binary with an isolated cache/config 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. +# +# Skipped on Windows-like shells (uses POSIX process control + git fixture). +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +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)" +trap 'rm -rf "${work}"' 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 + +# Force in-process indexing so auto-index + registration logs land in THIS +# process's stderr (no supervised-worker subprocess timing to chase). +export CBM_INDEX_SUPERVISOR=0 + +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"}' + +# run_session ... +# Feeds newline-delimited JSON-RPC to the server (cwd = the fixture repo so the +# session root is derived from it), holds stdin open for so async +# auto-index / watcher-thread logging can land, then closes it (EOF → clean +# server shutdown). +run_session() { + local cache="$1" errf="$2" outf="$3" wait_s="$4"; shift 4 + local rpc="${work}/rpc" + : >"${rpc}" + local line + for line in "$@"; do printf '%s\n' "${line}" >>"${rpc}"; done + ( cat "${rpc}"; sleep "${wait_s}" ) \ + | ( cd "${repo}" && CBM_CACHE_DIR="${cache}" "${BINARY}" >"${outf}" 2>"${errf}" ) \ + || true +} + +fail() { + echo "FAIL: $*" >&2 + echo "----- off-manual stderr -----" >&2; cat "${work}/offm.err" 2>/dev/null | grep -E 'msg=watcher' >&2 || true + echo "----- off-auto stderr -----" >&2; cat "${work}/offa.err" 2>/dev/null | grep -E 'msg=watcher|autoindex' >&2 || true + echo "----- on-auto stderr -----" >&2; cat "${work}/on.err" 2>/dev/null | grep -E 'msg=watcher|autoindex' >&2 || true + exit 1 +} + +# ============================================================================= +# Run 1 — DISABLED + manual index_repository (auto_index off, no contention). +# Proves: gate fires, watcher never starts, and the manual MCP tool still +# serves + indexes with the watcher off. +# ============================================================================= +c_offm="${work}/cache-offm" +CBM_CACHE_DIR="${c_offm}" "${BINARY}" config set watcher_enabled false >/dev/null +run_session "${c_offm}" "${work}/offm.err" "${work}/offm.out" 6 \ + "${INIT}" "${INITED}" \ + "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/call\",\"params\":{\"name\":\"index_repository\",\"arguments\":{\"repo_path\":\"${repo}\",\"mode\":\"fast\"}}}" + +grep -qE 'msg=watcher\.disabled( |$)' "${work}/offm.err" \ + && grep -q 'reason=config' "${work}/offm.err" || fail "watcher.disabled not emitted when disabled" +if grep -qE 'msg=watcher\.start( |$)' "${work}/offm.err"; then fail "watcher.start present when disabled"; fi +if grep -qE 'msg=watcher\.watch( |$)' "${work}/offm.err"; then fail "watcher.watch (registration) present when disabled"; fi +grep -q '"id":2' "${work}/offm.out" || fail "no index_repository response (tool unavailable when watcher off)" +grep -qE 'msg=pass\.done|persist_hashes' "${work}/offm.err" || fail "manual index_repository did not run the index pipeline" +echo "ok: disabled — watcher.disabled emitted, no watcher.start/watch, manual index_repository served" + +# ============================================================================= +# Run 2 — DISABLED + auto_index on. Same-config apples-to-apples for the +# registration axis: indexing still runs, but NO registration happens. +# ============================================================================= +c_offa="${work}/cache-offa" +CBM_CACHE_DIR="${c_offa}" "${BINARY}" config set watcher_enabled false >/dev/null +CBM_CACHE_DIR="${c_offa}" "${BINARY}" config set auto_index true >/dev/null +run_session "${c_offa}" "${work}/offa.err" "${work}/offa.out" 8 "${INIT}" "${INITED}" + +grep -qE 'msg=watcher\.disabled( |$)' "${work}/offa.err" || fail "watcher.disabled not emitted (auto_index run)" +if grep -qE 'msg=watcher\.start( |$)' "${work}/offa.err"; then fail "watcher.start present when disabled (auto_index run)"; fi +if grep -qE 'msg=watcher\.watch( |$)' "${work}/offa.err"; then fail "project registered with watcher when disabled"; fi +grep -qE 'msg=autoindex\.(done|skip)' "${work}/offa.err" || fail "auto-index did not run with watcher off" +echo "ok: disabled+auto_index — indexing ran, no watcher.start, no registration" + +# ============================================================================= +# Run 3 — ENABLED positive control (auto_index on). Proves the signals above +# are real: the watcher starts AND registers the session project — so their +# absence in Runs 1-2 is meaningful, and removing the gate fails this test. +# ============================================================================= +c_on="${work}/cache-on" +CBM_CACHE_DIR="${c_on}" "${BINARY}" config set watcher_enabled true >/dev/null +CBM_CACHE_DIR="${c_on}" "${BINARY}" config set auto_index true >/dev/null +run_session "${c_on}" "${work}/on.err" "${work}/on.out" 8 "${INIT}" "${INITED}" + +grep -qE 'msg=watcher\.start( |$)' "${work}/on.err" || fail "watcher.start absent when enabled (watcher never started)" +if grep -qE 'msg=watcher\.disabled( |$)' "${work}/on.err"; then fail "watcher.disabled present when enabled"; fi +grep -qE 'msg=watcher\.watch( |$)' "${work}/on.err" || fail "no project registration when enabled (control failed)" +echo "ok: enabled — watcher.start emitted and session project registered" + +echo "PASS: watcher_enabled kill-switch process regression (#335)"