Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
liuchong wants to merge 1 commit into
base: main
Choose a base branch
from
+104 −9
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 47 additions & 8 deletions src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2175,6 +2175,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) {
Expand Down Expand Up @@ -2380,6 +2400,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);
Expand All @@ -2388,14 +2409,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; "
Expand All @@ -2412,8 +2449,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) {
Expand Down
58 changes: 57 additions & 1 deletion tests/windows/test_daemon_lifecycle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
Loading