Skip to content

Commit 65dd99d

Browse files
DeusDataDeusData
committed
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 <[email protected]>
1 parent 95fc0d3 commit 65dd99d

4 files changed

Lines changed: 76 additions & 3 deletions

File tree

‎src/main.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2735,10 +2735,11 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo
27352735
cbm_ui_config_t ui_config;
27362736
cbm_ui_config_load(&ui_config);
27372737
/* A bare start inherits the saved choice, just like automatic startup.
2738-
* Only an explicit UI request may re-enable a deliberately disabled UI. */
2739-
bool ui_requested = ui_config.ui_enabled || requested_port > 0 || open_browser;
2738+
* Only an explicit UI request may re-enable a deliberately disabled UI
2739+
* (cbm_ui_config_daemon_start_ui; the ui suite pins the rule). */
2740+
bool ui_requested =
2741+
cbm_ui_config_daemon_start_ui(&ui_config, requested_port, open_browser, &ui_port);
27402742
if (CBM_EMBEDDED_FILE_COUNT > 0 && ui_requested) {
2741-
ui_port = requested_port > 0 ? requested_port : ui_config.ui_port;
27422743
uint8_t update_mask = 0x03U; /* enabled + port */
27432744
bool context_set =
27442745
main_set_client_context(start_result.client, ".", CBM_MCP_TOOL_PROFILE_ALL, NULL, NULL,

‎src/ui/config.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,22 @@ void cbm_ui_config_load(cbm_ui_config_t *cfg) {
168168
cbm_runtime_settings_load_ui(&cfg->ui_enabled, &cfg->ui_port);
169169
}
170170

171+
/* ── Daemon start rule ───────────────────────────────────────── */
172+
173+
bool cbm_ui_config_daemon_start_ui(const cbm_ui_config_t *saved, int requested_port,
174+
bool open_browser, int *port_out) {
175+
if (port_out) {
176+
*port_out = 0;
177+
}
178+
if (!saved || !(saved->ui_enabled || requested_port > 0 || open_browser)) {
179+
return false;
180+
}
181+
if (port_out) {
182+
*port_out = requested_port > 0 ? requested_port : saved->ui_port;
183+
}
184+
return true;
185+
}
186+
171187
/* ── Save ────────────────────────────────────────────────────── */
172188

173189
static bool config_parent_directory(const char *path, char *directory, size_t directory_size) {

‎src/ui/config.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,13 @@ bool cbm_ui_config_save(const cbm_ui_config_t *cfg);
3131
* Exposed for testing. */
3232
void cbm_ui_config_path(char *buf, int bufsz);
3333

34+
/* The cold `daemon start` UI rule (#2114). A bare start honours the saved
35+
* choice, including a deliberately disabled UI; an explicit --port=N
36+
* (requested_port > 0) or --open requests the UI, and the caller persists that
37+
* choice. Returns whether the start configures the UI. *port_out (optional)
38+
* receives the port to use, the requested port winning over the saved one, or
39+
* 0 when no UI is requested. A NULL saved configuration requests nothing. */
40+
bool cbm_ui_config_daemon_start_ui(const cbm_ui_config_t *saved, int requested_port,
41+
bool open_browser, int *port_out);
42+
3443
#endif /* CBM_UI_CONFIG_H */

‎tests/test_ui.c‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,52 @@ TEST(config_missing_fields) {
267267
PASS();
268268
}
269269

270+
/* #2114: a cold `daemon start` used to configure (and so re-enable) the UI on
271+
* every start, overwriting a saved ui_enabled=false. The rule now: a bare start
272+
* honours the saved choice; only an explicit --port=N or --open requests the UI
273+
* (and the caller persists that choice). The real-process guard
274+
* tests/test_daemon_open_readiness.py covers the same contract end to end, but
275+
* no CI leg runs it, so this pure rule is what CI checks. */
276+
TEST(config_daemon_start_honours_saved_ui_choice) {
277+
const cbm_ui_config_t disabled = {.ui_enabled = false, .ui_port = 9749};
278+
const cbm_ui_config_t enabled = {.ui_enabled = true, .ui_port = 9749};
279+
int port = -1;
280+
281+
/* Bare start, saved disabled: no UI and nothing to persist (#2114). */
282+
ASSERT_FALSE(cbm_ui_config_daemon_start_ui(&disabled, 0, false, &port));
283+
ASSERT_EQ(port, 0);
284+
285+
/* Bare start, saved enabled: the UI on the saved port. */
286+
port = -1;
287+
ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&enabled, 0, false, &port));
288+
ASSERT_EQ(port, 9749);
289+
290+
/* --port=N re-enables a disabled UI on N. */
291+
port = -1;
292+
ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 9800, false, &port));
293+
ASSERT_EQ(port, 9800);
294+
295+
/* --open re-enables a disabled UI on the saved port. */
296+
port = -1;
297+
ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 0, true, &port));
298+
ASSERT_EQ(port, 9749);
299+
300+
/* An explicit --port=N wins over the saved port. */
301+
port = -1;
302+
ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&enabled, 9800, false, &port));
303+
ASSERT_EQ(port, 9800);
304+
port = -1;
305+
ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 9800, true, &port));
306+
ASSERT_EQ(port, 9800);
307+
308+
/* No saved configuration requests nothing. */
309+
port = -1;
310+
ASSERT_FALSE(cbm_ui_config_daemon_start_ui(NULL, 9800, true, &port));
311+
ASSERT_EQ(port, 0);
312+
313+
PASS();
314+
}
315+
270316
/* ── Embedded asset tests ─────────────────────────────────────── */
271317

272318
TEST(embedded_lookup_not_found) {
@@ -2734,6 +2780,7 @@ SUITE(ui) {
27342780
RUN_TEST(config_overwrite);
27352781
RUN_TEST(config_corrupt_file);
27362782
RUN_TEST(config_missing_fields);
2783+
RUN_TEST(config_daemon_start_honours_saved_ui_choice);
27372784

27382785
/* Embedded assets (stub) */
27392786
RUN_TEST(embedded_lookup_not_found);

0 commit comments

Comments
 (0)