Merge pull request #1796 from liuchong/fix/daemon-start-survives-ui-config

fix(cli): a refused UI configuration is not a failed daemon start

Author: DeusData

src/main.c

@@ -2177,6 +2177,26 @@ static void main_daemon_ctl_open_browser(int port) {
     }
 }
 
+/* Ask the daemon to enable the UI on `port`; true when it accepted.
+ *
+ * The seam below forces the refusal that the caller must survive. Reproducing it
+ * for real needs a machine loaded enough to miss a bounded handshake, which is
+ * not a state a test can ask for; it is COMPILED OUT of ordinary builds (see
+ * TEST_SEAMS in Makefile.cbm). */
+static bool main_daemon_ctl_apply_ui_config(cbm_daemon_runtime_client_t *client,
+                                            uint8_t update_mask, int port) {
+#ifdef CBM_ENABLE_TEST_SEAMS
+    char forced[8];
+    if (cbm_safe_getenv("CBM_TEST_DAEMON_UI_CONFIG_REFUSED", forced, sizeof(forced), NULL) &&
+        forced[0] == '1') {
+        return false;
+    }
+#endif
+    return cbm_daemon_application_client_set_ui_config(client, update_mask, true, port,
+                                                       MAIN_CONNECT_TIMEOUT_MS) ==
+           CBM_DAEMON_RUNTIME_APPLICATION_OK;
+}
+
 static int main_daemon_ctl_finish_ui_open(cbm_daemon_runtime_client_t **client_io, int port,
                                           bool open_browser) {
     if (!open_browser) {
@@ -2395,6 +2415,7 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo
     /* The committed control connection satisfied the daemon's no-client
      * 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);
@@ -2403,14 +2424,30 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo
         bool context_set =
             main_set_client_context(start_result.client, ".", CBM_MCP_TOOL_PROFILE_ALL, NULL, NULL,
                                     MAIN_CONNECT_TIMEOUT_MS);
-        if (!context_set || cbm_daemon_application_client_set_ui_config(
-                                start_result.client, update_mask, true, ui_port,
-                                MAIN_CONNECT_TIMEOUT_MS) != CBM_DAEMON_RUNTIME_APPLICATION_OK) {
+        ui_configured = context_set &&
+                        main_daemon_ctl_apply_ui_config(start_result.client, update_mask, ui_port);
+        if (!ui_configured) {
+            /* Reaching here means the daemon is up: the control connection above
+             * already satisfied its startup window. Only the UI handshake — two
+             * requests bounded by MAIN_CONNECT_TIMEOUT_MS — came back short.
+             *
+             * Whether that is fatal depends on what was asked for. `--port`/`--open`
+             * make the UI the point of the command, so failing to configure it is a
+             * failed command. A bare `daemon start` asks for a daemon, and it got
+             * one; reporting failure there sent operators hunting for a daemon that
+             * was in fact running and healthy. It also made a loaded machine look
+             * like a broken one, because a second of scheduling delay after an
+             * abrupt shutdown is enough to miss this handshake. */
+            if (requested_port > 0 || open_browser) {
+                (void)fprintf(stderr,
+                              "error: the daemon did not accept the UI configuration; browser was "
+                              "not opened\n");
+                (void)cbm_daemon_runtime_client_close(start_result.client, MAIN_CLOSE_TIMEOUT_MS);
+                return EXIT_FAILURE;
+            }
             (void)fprintf(stderr,
-                          "error: the daemon did not accept the UI configuration; browser was "
-                          "not opened\n");
-            (void)cbm_daemon_runtime_client_close(start_result.client, MAIN_CLOSE_TIMEOUT_MS);
-            return EXIT_FAILURE;
+                          "warning: the daemon started but did not accept the UI configuration; "
+                          "the UI is unavailable until the next `daemon start`\n");
         }
     } else if (requested_port > 0 || open_browser) {
         (void)fprintf(stderr, "warning: this binary was built without UI support; "
@@ -2427,8 +2464,10 @@ 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");
+    /* Skipped when the handshake was refused: that path announces the port as
+     * warming, which would be a promise nothing is keeping. */
     int ui_result =
-        (CBM_EMBEDDED_FILE_COUNT > 0)
+        (CBM_EMBEDDED_FILE_COUNT > 0 && ui_configured)
             ? main_daemon_ctl_finish_ui_open(&start_result.client, ui_port, open_browser)
             : EXIT_SUCCESS;
     if (start_result.client) {

tests/windows/test_daemon_lifecycle.py

@@ -24,9 +24,11 @@
 import tempfile
 
 
-def run_cli(binary, cache, args, timeout=60):
+def run_cli(binary, cache, args, timeout=60, extra_env=None):
     env = dict(os.environ)
     env["CBM_CACHE_DIR"] = cache
+    if extra_env:
+        env.update(extra_env)
     return subprocess.run([binary] + args, capture_output=True, timeout=timeout, env=env)
 
 
@@ -81,6 +83,8 @@ def main():
                   % start_text[:400])
             return 1
         print("PASS: daemon start reported permanent pid %d" % daemon_pid)
+        # Only a UI build configures a UI, and only it can refuse to.
+        ui_build = "ui:" in start_text
 
         warm = run_cli(binary, cache, ["cli", "list_projects", "{}"])
         warm_text = output_text(warm)
@@ -107,6 +111,58 @@ def main():
                   % output_text(stop_again)[:300])
             return 1
         print("PASS: stop retired the idle daemon; second stop was idempotent")
+        daemon_pid = 0
+
+        if not ui_build:
+            print("SKIP: non-UI binary cannot exercise the UI-configuration refusal")
+        else:
+            refused_env = {"CBM_TEST_DAEMON_UI_CONFIG_REFUSED": "1"}
+            # A refused UI handshake must not be reported as a failed start. The
+            # daemon is already up by then, so a bare `daemon start` got exactly
+            # what it asked for; only `--port`/`--open` make the UI the point of
+            # the command. Before the fix a busy machine — one second of delay
+            # after an abrupt shutdown was enough — turned a healthy start into a
+            # nonzero exit, which is how this reached CI as a phantom failure.
+            refused = run_cli(binary, cache, ["daemon", "start"], extra_env=refused_env)
+            refused_text = output_text(refused)
+            pid_match = re.search(r"pid (\d+)", refused_text)
+            daemon_pid = int(pid_match.group(1)) if pid_match else 0
+            if refused.returncode != 0 or "permanent" not in refused_text:
+                print("RED: a refused UI configuration must not fail `daemon start`; "
+                      "the daemon is already running by then:\n%s" % refused_text[:400])
+                return 1
+            if "warning" not in refused_text:
+                print("RED: a refused UI configuration must still be reported, as a "
+                      "warning:\n%s" % refused_text[:400])
+                return 1
+            alive = run_cli(binary, cache, ["daemon", "status"])
+            if alive.returncode != 0 or "permanent" not in output_text(alive):
+                print("RED: the daemon that survived a refused UI configuration should "
+                      "be reachable:\n%s" % output_text(alive)[:400])
+                return 1
+            print("PASS: a refused UI configuration warns and leaves the daemon running")
+
+            stopped = run_cli(binary, cache, ["daemon", "stop"])
+            if stopped.returncode != 0:
+                print("RED: could not retire the daemon started without a UI:\n%s"
+                      % output_text(stopped)[:300])
+                return 1
+            daemon_pid = 0
+
+            # The other half of the contract: when the UI *was* asked for, failing
+            # to configure it is still a failed command.
+            demanded = run_cli(binary, cache, ["daemon", "start", "--port=45871"],
+                               extra_env=refused_env)
+            demanded_text = output_text(demanded)
+            pid_match = re.search(r"pid (\d+)", demanded_text)
+            daemon_pid = int(pid_match.group(1)) if pid_match else 0
+            if demanded.returncode == 0:
+                print("RED: `daemon start --port=N` must fail when the UI it asked for "
+                      "could not be configured:\n%s" % demanded_text[:400])
+                return 1
+            print("PASS: an explicitly requested UI still fails the command when refused")
+            run_cli(binary, cache, ["daemon", "stop"])
+            daemon_pid = 0
 
         print("\nGREEN: daemon lifecycle (status/start/recycle/stop) behaves.")
         return 0