From e0a6133293556d87de6c2ca43de674e3a643e339 Mon Sep 17 00:00:00 2001 From: Vincent Koc <25068+vincentkoc@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:26:17 +0200 Subject: [PATCH 1/3] fix(daemon): preserve disabled UI on manual start Signed-off-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com> --- docs/CONFIGURATION.md | 2 + src/main.c | 22 ++++++--- tests/test_daemon_open_readiness.py | 77 +++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 6 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 862d591c3f..cd28ced1c2 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -136,6 +136,8 @@ Current format: Notes: - If a UI-enabled binary finds its verified external asset pack and no UI config file exists yet, the UI auto-enables on first run. Missing or invalid assets leave the MCP/daemon service available and keep the UI disabled. +- A bare `daemon start` honors the saved `ui_enabled` value, including `false`. Enable the UI with `codebase-memory-mcp config set ui_enabled true`. +- On a cold start, `daemon start --open` or `daemon start --port=N` explicitly enables the UI and persists that choice. An already-active daemon keeps its current settings; stop it first to change ports with `--port=N`. - `CBM_CACHE_DIR` changes both the UI config location and the runtime settings database location. - CBM resolves `CBM_CACHE_DIR` to one canonical per-account cache root. A process configured with a different root fails while any CBM session or command is active; close them before switching roots. diff --git a/src/main.c b/src/main.c index 284a899cbf..0e65ab5f26 100644 --- a/src/main.c +++ b/src/main.c @@ -2333,7 +2333,7 @@ static void main_daemon_ctl_print_ui_configuration(void) { "`daemon start --open` verifies it)\n", ui_config.ui_port); } else { - printf(" ui: disabled (enable with `daemon start` in a UI build)\n"); + printf(" ui: disabled (enable with `config set ui_enabled true`)\n"); } } @@ -2556,8 +2556,12 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo cbm_ui_config_t ui_config; cbm_ui_config_load(&ui_config); if (!ui_config.ui_enabled) { - (void)fprintf(stderr, "error: UI is disabled for the active daemon; browser was not " - "opened\n"); + main_daemon_ctl_print_ui_configuration(); + if (open_browser) { + (void)fprintf(stderr, + "error: UI is disabled for the active daemon; browser was not " + "opened\n"); + } return open_browser ? EXIT_FAILURE : EXIT_SUCCESS; } if (requested_port > 0 && requested_port != ui_config.ui_port) { @@ -2612,9 +2616,12 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo * startup window; configure the UI before departing. */ int ui_port = 0; bool ui_configured = false; - if ((CBM_EMBEDDED_FILE_COUNT > 0)) { - cbm_ui_config_t ui_config; - cbm_ui_config_load(&ui_config); + cbm_ui_config_t ui_config; + cbm_ui_config_load(&ui_config); + /* A bare start inherits the saved choice, just like automatic startup. + * Only an explicit UI request may re-enable a deliberately disabled UI. */ + bool ui_requested = ui_config.ui_enabled || requested_port > 0 || open_browser; + if (CBM_EMBEDDED_FILE_COUNT > 0 && ui_requested) { ui_port = requested_port > 0 ? requested_port : ui_config.ui_port; uint8_t update_mask = 0x03U; /* enabled + port */ bool context_set = @@ -2660,6 +2667,9 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo } printf("It survives idle periods and session ends; `codebase-memory-mcp daemon stop` " "retires it.\n"); + if (CBM_EMBEDDED_FILE_COUNT > 0 && !ui_requested) { + main_daemon_ctl_print_ui_configuration(); + } /* Skipped when the handshake was refused: that path announces the port as * warming, which would be a promise nothing is keeping. */ int ui_result = diff --git a/tests/test_daemon_open_readiness.py b/tests/test_daemon_open_readiness.py index 08f7d851ab..b3177eb545 100644 --- a/tests/test_daemon_open_readiness.py +++ b/tests/test_daemon_open_readiness.py @@ -6,6 +6,8 @@ This guard uses a real UI build and isolated daemon generations: +* a saved disabled UI survives cold and repeated bare starts, while explicit + UI requests and saved enabled settings still start the listener; * a temporarily occupied port proves the command waits and then succeeds once the verified UI listener becomes available; * a foreign service returning the exact formerly accepted HTML markers proves @@ -22,6 +24,7 @@ Exit code: 0 == green, 1 == behavior regression, 2 == fixture/setup error. """ +import json import os import re import shutil @@ -327,6 +330,76 @@ def assert_active_daemon_open(binary, work): stop_daemon(binary, env, daemon_pid) +def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): + name = "enabled" if enabled else "disabled" + name += "-" + ("-".join(flag.lstrip("-") for flag in flags) or "bare") + cache = os.path.join(work, "cache-" + name) + marker = os.path.join(work, "browser-" + name + ".txt") + probe, port = occupied_loopback_port() + probe.close() + os.makedirs(cache, exist_ok=True) + env = fixture_environment(work, cache, marker, 10000) + config_path = os.path.join(cache, "config.json") + daemon_pid = 0 + try: + for key, value in (("ui_enabled", str(enabled).lower()), ("ui_port", str(port))): + configured = subprocess.run([binary, "config", "set", key, value], + capture_output=True, timeout=10, env=env, cwd=work) + if configured.returncode != 0: + print("SETUP FAIL: could not persist UI setting:\n%s" % output_text(configured)) + return False + with open(config_path, "rb") as handle: + original_config = handle.read() + arguments = ["--port=%d" % port if flag == "--port" else flag for flag in flags] + first = subprocess.run([binary, "daemon", "start", *arguments], capture_output=True, + timeout=30, env=env, cwd=work) + first_text = output_text(first) + daemon_pid = pid_from(first_text) + if first.returncode != 0 or "daemon: started" not in first_text: + print("RED: daemon start failed with saved UI setting (%s):\n%s" % + (name, first_text[:700])) + return False + with open(config_path, "rb") as handle: + current_config = handle.read() + expected_enabled = enabled or bool(flags) + if (json.loads(current_config).get("ui_enabled") != expected_enabled or + (not expected_enabled and current_config != original_config)): + print("RED: daemon start rewrote the saved UI setting (%s): %r" % + (name, current_config)) + return False + if expected_enabled: + opened = subprocess.run([binary, "daemon", "start", "--open"], capture_output=True, + timeout=20, env=env, cwd=work) + if opened.returncode != 0 or not os.path.exists(marker): + print("RED: requested UI did not become ready (%s):\n%s" % + (name, output_text(opened)[:700])) + return False + else: + for arguments in (("start",), ("status",), ("start", "--open")): + result = subprocess.run([binary, "daemon", *arguments], capture_output=True, + timeout=10, env=env, cwd=work) + text = output_text(result) + expected_result = 1 if "--open" in arguments else 0 + if (result.returncode != expected_result or "disabled" not in text or + "warming" in text or os.path.exists(marker)): + print("RED: disabled UI was not preserved by %r:\n%s" % + (arguments, text[:700])) + return False + with open(config_path, "rb") as handle: + if handle.read() != original_config: + print("RED: repeated daemon start rewrote disabled UI config") + return False + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as connection: + connection.settimeout(0.5) + if connection.connect_ex(("127.0.0.1", port)) == 0: + print("RED: disabled UI opened a listener") + return False + print("PASS: daemon start respects saved settings and explicit UI requests (%s)" % name) + return True + finally: + stop_daemon(binary, env, daemon_pid) + + def main(): if len(sys.argv) != 2: print("usage: python3 test_daemon_open_readiness.py ") @@ -346,6 +419,10 @@ def main(): # Darwin temporary root so the product endpoint itself remains valid. short_temp_root = "/private/tmp" if sys.platform == "darwin" else tempfile.gettempdir() with tempfile.TemporaryDirectory(prefix="cbm_uiopen_", dir=short_temp_root) as work: + for enabled, flags in ((False, ()), (True, ()), + (False, ("--port",)), (False, ("--open",))): + if not assert_persisted_ui_setting(binary, work, enabled, flags): + return 1 if not assert_delayed_success(binary, work): return 1 if not assert_bounded_foreign_port_failure(binary, work): From 65dd99d7e64da39155b3dbcdc10dc0db1a83299e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 00:56:08 +0100 Subject: [PATCH 2/3] refactor(daemon): extract the cold start UI rule into a tested helper The cold daemon start rule this PR introduces (a bare start honours a saved ui_enabled=false; only an explicit --port=N or --open turns the UI on and persists that choice) was covered only by tests/test_daemon_open_readiness.py, a real-process guard that no CI leg runs. Move the rule into a pure helper, cbm_ui_config_daemon_start_ui() in src/ui/config.c, and call it from the cold start path in main.c; the CBM_EMBEDDED_FILE_COUNT gate stays in main.c and the behaviour is unchanged. Add config_daemon_start_honours_saved_ui_choice to the ui suite, which CI runs. It fails if the old always-configure rule comes back (a bare start with a saved disabled UI) and if --port=N or --open stop re-enabling a disabled UI. Refs #2114 Signed-off-by: Martin Vogel --- src/main.c | 7 ++++--- src/ui/config.c | 16 ++++++++++++++++ src/ui/config.h | 9 +++++++++ tests/test_ui.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/main.c b/src/main.c index 402ee5f640..6c2eea7583 100644 --- a/src/main.c +++ b/src/main.c @@ -2735,10 +2735,11 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo cbm_ui_config_t ui_config; cbm_ui_config_load(&ui_config); /* A bare start inherits the saved choice, just like automatic startup. - * Only an explicit UI request may re-enable a deliberately disabled UI. */ - bool ui_requested = ui_config.ui_enabled || requested_port > 0 || open_browser; + * Only an explicit UI request may re-enable a deliberately disabled UI + * (cbm_ui_config_daemon_start_ui; the ui suite pins the rule). */ + bool ui_requested = + cbm_ui_config_daemon_start_ui(&ui_config, requested_port, open_browser, &ui_port); if (CBM_EMBEDDED_FILE_COUNT > 0 && ui_requested) { - ui_port = requested_port > 0 ? requested_port : ui_config.ui_port; uint8_t update_mask = 0x03U; /* enabled + port */ bool context_set = main_set_client_context(start_result.client, ".", CBM_MCP_TOOL_PROFILE_ALL, NULL, NULL, diff --git a/src/ui/config.c b/src/ui/config.c index 8fc1bbda4e..9c1e6b9690 100644 --- a/src/ui/config.c +++ b/src/ui/config.c @@ -168,6 +168,22 @@ void cbm_ui_config_load(cbm_ui_config_t *cfg) { cbm_runtime_settings_load_ui(&cfg->ui_enabled, &cfg->ui_port); } +/* ── Daemon start rule ───────────────────────────────────────── */ + +bool cbm_ui_config_daemon_start_ui(const cbm_ui_config_t *saved, int requested_port, + bool open_browser, int *port_out) { + if (port_out) { + *port_out = 0; + } + if (!saved || !(saved->ui_enabled || requested_port > 0 || open_browser)) { + return false; + } + if (port_out) { + *port_out = requested_port > 0 ? requested_port : saved->ui_port; + } + return true; +} + /* ── Save ────────────────────────────────────────────────────── */ static bool config_parent_directory(const char *path, char *directory, size_t directory_size) { diff --git a/src/ui/config.h b/src/ui/config.h index 90d4ceabe7..36b62222f5 100644 --- a/src/ui/config.h +++ b/src/ui/config.h @@ -31,4 +31,13 @@ bool cbm_ui_config_save(const cbm_ui_config_t *cfg); * Exposed for testing. */ void cbm_ui_config_path(char *buf, int bufsz); +/* The cold `daemon start` UI rule (#2114). A bare start honours the saved + * choice, including a deliberately disabled UI; an explicit --port=N + * (requested_port > 0) or --open requests the UI, and the caller persists that + * choice. Returns whether the start configures the UI. *port_out (optional) + * receives the port to use, the requested port winning over the saved one, or + * 0 when no UI is requested. A NULL saved configuration requests nothing. */ +bool cbm_ui_config_daemon_start_ui(const cbm_ui_config_t *saved, int requested_port, + bool open_browser, int *port_out); + #endif /* CBM_UI_CONFIG_H */ diff --git a/tests/test_ui.c b/tests/test_ui.c index 8c866ff526..922363b069 100644 --- a/tests/test_ui.c +++ b/tests/test_ui.c @@ -267,6 +267,52 @@ TEST(config_missing_fields) { PASS(); } +/* #2114: a cold `daemon start` used to configure (and so re-enable) the UI on + * every start, overwriting a saved ui_enabled=false. The rule now: a bare start + * honours the saved choice; only an explicit --port=N or --open requests the UI + * (and the caller persists that choice). The real-process guard + * tests/test_daemon_open_readiness.py covers the same contract end to end, but + * no CI leg runs it, so this pure rule is what CI checks. */ +TEST(config_daemon_start_honours_saved_ui_choice) { + const cbm_ui_config_t disabled = {.ui_enabled = false, .ui_port = 9749}; + const cbm_ui_config_t enabled = {.ui_enabled = true, .ui_port = 9749}; + int port = -1; + + /* Bare start, saved disabled: no UI and nothing to persist (#2114). */ + ASSERT_FALSE(cbm_ui_config_daemon_start_ui(&disabled, 0, false, &port)); + ASSERT_EQ(port, 0); + + /* Bare start, saved enabled: the UI on the saved port. */ + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&enabled, 0, false, &port)); + ASSERT_EQ(port, 9749); + + /* --port=N re-enables a disabled UI on N. */ + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 9800, false, &port)); + ASSERT_EQ(port, 9800); + + /* --open re-enables a disabled UI on the saved port. */ + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 0, true, &port)); + ASSERT_EQ(port, 9749); + + /* An explicit --port=N wins over the saved port. */ + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&enabled, 9800, false, &port)); + ASSERT_EQ(port, 9800); + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 9800, true, &port)); + ASSERT_EQ(port, 9800); + + /* No saved configuration requests nothing. */ + port = -1; + ASSERT_FALSE(cbm_ui_config_daemon_start_ui(NULL, 9800, true, &port)); + ASSERT_EQ(port, 0); + + PASS(); +} + /* ── Embedded asset tests ─────────────────────────────────────── */ TEST(embedded_lookup_not_found) { @@ -2734,6 +2780,7 @@ SUITE(ui) { RUN_TEST(config_overwrite); RUN_TEST(config_corrupt_file); RUN_TEST(config_missing_fields); + RUN_TEST(config_daemon_start_honours_saved_ui_choice); /* Embedded assets (stub) */ RUN_TEST(embedded_lookup_not_found); From f31d7e82a0437fd11cb3f86c3769996d29cb289e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 00:56:08 +0100 Subject: [PATCH 3/3] test(daemon): read the saved UI setting through config get config set now stores ui_enabled and ui_port in the shared settings database (_config.db), so the readiness guard read of config.json raised FileNotFoundError before its first assertion. Read the pair back with config get ui_enabled and config get ui_port instead; they load the same value every start path uses. The guard compares it after config set, after the first start, and, for a disabled UI, after the repeated start, status and start --open commands. Drop the now unused json import. Signed-off-by: Martin Vogel --- tests/test_daemon_open_readiness.py | 46 +++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/tests/test_daemon_open_readiness.py b/tests/test_daemon_open_readiness.py index b3177eb545..f0ef1d6e24 100644 --- a/tests/test_daemon_open_readiness.py +++ b/tests/test_daemon_open_readiness.py @@ -24,7 +24,6 @@ Exit code: 0 == green, 1 == behavior regression, 2 == fixture/setup error. """ -import json import os import re import shutil @@ -330,6 +329,25 @@ def assert_active_daemon_open(binary, work): stop_daemon(binary, env, daemon_pid) +def saved_ui_setting(binary, env, work): + """Return the persisted (ui_enabled, ui_port) pair as ``config get`` reports it. + + ``config set`` keeps the UI keys in the shared settings database + (``_config.db``), not in ``config.json``. Reading them back through the + product's own command checks the value every start path loads, whatever the + storage format. Returns None when a read fails. + """ + values = [] + for key in ("ui_enabled", "ui_port"): + result = subprocess.run([binary, "config", "get", key], capture_output=True, + timeout=10, env=env, cwd=work) + if result.returncode != 0: + print("diagnostic: config get %s failed:\n%s" % (key, output_text(result)[:700])) + return None + values.append(result.stdout.decode("utf-8", "replace").strip()) + return tuple(values) + + def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): name = "enabled" if enabled else "disabled" name += "-" + ("-".join(flag.lstrip("-") for flag in flags) or "bare") @@ -339,7 +357,6 @@ def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): probe.close() os.makedirs(cache, exist_ok=True) env = fixture_environment(work, cache, marker, 10000) - config_path = os.path.join(cache, "config.json") daemon_pid = 0 try: for key, value in (("ui_enabled", str(enabled).lower()), ("ui_port", str(port))): @@ -348,8 +365,11 @@ def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): if configured.returncode != 0: print("SETUP FAIL: could not persist UI setting:\n%s" % output_text(configured)) return False - with open(config_path, "rb") as handle: - original_config = handle.read() + original_setting = saved_ui_setting(binary, env, work) + if original_setting != (str(enabled).lower(), str(port)): + print("SETUP FAIL: config get does not report the saved UI setting (%s): %r" % + (name, original_setting)) + return False arguments = ["--port=%d" % port if flag == "--port" else flag for flag in flags] first = subprocess.run([binary, "daemon", "start", *arguments], capture_output=True, timeout=30, env=env, cwd=work) @@ -359,13 +379,12 @@ def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): print("RED: daemon start failed with saved UI setting (%s):\n%s" % (name, first_text[:700])) return False - with open(config_path, "rb") as handle: - current_config = handle.read() + current_setting = saved_ui_setting(binary, env, work) expected_enabled = enabled or bool(flags) - if (json.loads(current_config).get("ui_enabled") != expected_enabled or - (not expected_enabled and current_config != original_config)): + if (current_setting is None or current_setting[0] != str(expected_enabled).lower() or + (not expected_enabled and current_setting != original_setting)): print("RED: daemon start rewrote the saved UI setting (%s): %r" % - (name, current_config)) + (name, current_setting)) return False if expected_enabled: opened = subprocess.run([binary, "daemon", "start", "--open"], capture_output=True, @@ -385,10 +404,11 @@ def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): print("RED: disabled UI was not preserved by %r:\n%s" % (arguments, text[:700])) return False - with open(config_path, "rb") as handle: - if handle.read() != original_config: - print("RED: repeated daemon start rewrote disabled UI config") - return False + repeated_setting = saved_ui_setting(binary, env, work) + if repeated_setting != original_setting: + print("RED: repeated daemon start rewrote disabled UI config: %r" % + (repeated_setting,)) + return False with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as connection: connection.settimeout(0.5) if connection.connect_ex(("127.0.0.1", port)) == 0: