Break-glass merge PR #2650

src/cli/cli.c

@@ -14,6 +14,7 @@
 #include "cli/config_toml_edit.h"
 #include "ui/config.h"
 #include "cli/config_yaml_edit.h"
+#include "cli/runtime_settings.h"
 #include "daemon/bootstrap.h"
 #include "daemon/ipc.h"
 #include "daemon/runtime.h"
@@ -7409,6 +7410,9 @@ cbm_config_t *cbm_config_open(const char *cache_dir) {
         }
         return NULL;
     }
+    /* Another process mid-commit is waited out; failing here failed every
+     * settings write behind it, the daemon's UI save included. */
+    cbm_runtime_settings_wait_for_writers(db);
 
     /* Create table if not exists */
     const char *sql = "CREATE TABLE IF NOT EXISTS config (key TEXT PRIMARY KEY, value TEXT)";

src/cli/runtime_settings.c

@@ -203,6 +203,32 @@ void cbm_runtime_settings_note_http_port(int port) {
     atomic_store_explicit(&active_http_port, port, memory_order_relaxed);
 }
 
+/* A commit holds _config.db's lock for milliseconds, so its release is an event
+ * that always comes; waiting for it is not a race. Failing on the lock instead
+ * (SQLite's default, or startup's former 50 ms window) let one process's routine
+ * commit fail another: a daemon saving its UI configuration made unrelated CLI
+ * starts exit with "could not load saved runtime settings", and a concurrent
+ * commit made the daemon's own UI save fail. The bound only catches a wedged
+ * holder. Each busy wait is logged once (SQLite restarts the attempt count for
+ * every lock attempt), so a test that holds the lock can observe the waiter
+ * instead of guessing a duration. */
+enum { SETTINGS_BUSY_TIMEOUT_MS = 3000, SETTINGS_BUSY_STEP_MS = 10 };
+
+static int settings_busy_wait(void *context, int attempts) {
+    (void)context;
+    if (attempts == 0)
+        cbm_log_warn("config_db.busy_wait", "action", "waiting for a settings writer");
+    if ((long long)attempts * SETTINGS_BUSY_STEP_MS >= SETTINGS_BUSY_TIMEOUT_MS)
+        return 0;
+    sqlite3_sleep(SETTINGS_BUSY_STEP_MS);
+    return 1;
+}
+
+void cbm_runtime_settings_wait_for_writers(sqlite3 *db) {
+    if (db)
+        sqlite3_busy_handler(db, settings_busy_wait, NULL);
+}
+
 static bool canonical_cache_path(const char *cache, char out[SETTINGS_PATH_CAP], bool create) {
     if (!cache || !cache[0])
         return false;
@@ -234,7 +260,7 @@ static sqlite3 *settings_open(const char *cache) {
             sqlite3_close(db);
         return NULL;
     }
-    sqlite3_busy_timeout(db, 3000);
+    cbm_runtime_settings_wait_for_writers(db);
     const char *schema =
         "BEGIN IMMEDIATE;"
         "CREATE TABLE IF NOT EXISTS runtime_settings(key TEXT PRIMARY KEY,value TEXT NOT NULL);"
@@ -442,6 +468,65 @@ void cbm_runtime_settings_reset_for_tests(void) {
 }
 #endif
 
+/* A process killed mid-write leaves a hot rollback journal, and a read-only
+ * connection cannot roll it back (SQLITE_READONLY_ROLLBACK): every later start
+ * failed with "could not load saved runtime settings" until something opened
+ * the database read-write, and nothing did. One read-write open that reads the
+ * schema makes SQLite roll the interrupted write back. A journal is hot only
+ * while no live writer holds its lock, so this never undoes a write in
+ * progress; a live writer takes the busy path instead. */
+static bool settings_recover_hot_journal(const char *path) {
+    sqlite3 *db = NULL;
+    bool recovered = sqlite3_open_v2(path, &db, SQLITE_OPEN_READWRITE, NULL) == SQLITE_OK;
+    if (recovered) {
+        cbm_runtime_settings_wait_for_writers(db);
+        recovered =
+            sqlite3_exec(db, "SELECT count(*) FROM sqlite_master", NULL, NULL, NULL) == SQLITE_OK;
+    }
+    if (db)
+        sqlite3_close(db);
+    char journal[SETTINGS_PATH_CAP + 16];
+    (void)snprintf(journal, sizeof(journal), "%s-journal", path);
+    if (recovered)
+        cbm_log_warn("config_db.hot_journal_recovered", "journal", journal, "action",
+                     "rolled back an interrupted settings write");
+    else
+        /* Still fatal for the caller: without the rollback the saved settings
+         * cannot be read consistently, and guessing them is worse. */
+        cbm_log_error("config_db.hot_journal_unrecovered", "journal", journal, "hint",
+                      "an interrupted settings write could not be rolled back; make the "
+                      "settings directory writable and retry");
+    return recovered;
+}
+
+/* Read-only connection holding one read snapshot (BEGIN + a schema read), or
+ * NULL. A writer mid-commit is waited out; a database still locked after the
+ * bound is an error, not an absent override. */
+static sqlite3 *settings_open_snapshot(const char *path, bool *hot_journal) {
+    *hot_journal = false;
+    sqlite3 *db = NULL;
+    if (sqlite3_open_v2(path, &db, SQLITE_OPEN_READONLY, NULL) != SQLITE_OK) {
+        if (db)
+            sqlite3_close(db);
+        return NULL;
+    }
+    cbm_runtime_settings_wait_for_writers(db);
+    if (sqlite3_exec(db, "BEGIN; SELECT count(*) FROM sqlite_master", NULL, NULL, NULL) ==
+        SQLITE_OK)
+        return db;
+    *hot_journal = sqlite3_extended_errcode(db) == SQLITE_READONLY_ROLLBACK;
+    sqlite3_close(db);
+    return NULL;
+}
+
+static sqlite3 *settings_read_snapshot(const char *path) {
+    bool hot_journal = false;
+    sqlite3 *db = settings_open_snapshot(path, &hot_journal);
+    if (!db && hot_journal && settings_recover_hot_journal(path))
+        db = settings_open_snapshot(path, &hot_journal);
+    return db;
+}
+
 bool cbm_runtime_settings_init(const char *cache) {
     char canonical[SETTINGS_PATH_CAP];
     if (!canonical_cache_path(cache, canonical, true))
@@ -453,20 +538,12 @@ bool cbm_runtime_settings_init(const char *cache) {
         return false;
     sqlite3 *db = NULL;
     if (cbm_file_exists(path)) {
-        if (sqlite3_open_v2(path, &db, SQLITE_OPEN_READONLY, NULL) != SQLITE_OK) {
-            if (db)
-                sqlite3_close(db);
+        /* The snapshot read happens before individual optional tables are
+         * queried; failing here exits the whole process. Hook clients skip
+         * this init. */
+        db = settings_read_snapshot(path);
+        if (!db)
             return false;
-        }
-        /* Startup, especially hook clients, must never wait on a schema writer.
-         * This read forces the shared snapshot before individual optional tables
-         * are queried; a busy database is an error, not an absent override. */
-        sqlite3_busy_timeout(db, 50);
-        if (sqlite3_exec(db, "BEGIN; SELECT count(*) FROM sqlite_master", NULL, NULL, NULL) !=
-            SQLITE_OK) {
-            sqlite3_close(db);
-            return false;
-        }
     }
     for (size_t i = 0; i < SETTINGS_COUNT; i++) {
         bool saved = false;
@@ -640,15 +717,18 @@ static char *error_json(const char *message, int code, int *status) {
     return json;
 }
 
+/* An unchanged value is not rewritten: the UPDATE would still fire the revision
+ * trigger, so every `daemon start` on a UI build committed (journal + fsync) and
+ * locked out concurrent readers although nothing changed. */
 static bool write_value(sqlite3 *db, const setting_t *setting, const char *value) {
     const char *sql;
     if (setting->storage == SET_CONFIG)
         sql = value ? "INSERT INTO config(key,value) VALUES(?,?) ON CONFLICT(key) DO UPDATE SET "
-                      "value=excluded.value"
+                      "value=excluded.value WHERE value IS NOT excluded.value"
                     : "DELETE FROM config WHERE key=?";
     else
         sql = value ? "INSERT INTO runtime_settings(key,value) VALUES(?,?) ON CONFLICT(key) DO "
-                      "UPDATE SET value=excluded.value"
+                      "UPDATE SET value=excluded.value WHERE value IS NOT excluded.value"
                     : "DELETE FROM runtime_settings WHERE key=?";
     sqlite3_stmt *statement = NULL;
     if (sqlite3_prepare_v2(db, sql, -1, &statement, NULL) != SQLITE_OK)
@@ -759,13 +839,12 @@ void cbm_runtime_settings_load_ui(bool *enabled, int *port) {
     char path[SETTINGS_PATH_CAP];
     if (snprintf(path, sizeof(path), "%s/_config.db", canonical) >= (int)sizeof(path))
         return;
-    sqlite3 *db = NULL;
-    if (sqlite3_open_v2(path, &db, SQLITE_OPEN_READONLY, NULL) != SQLITE_OK) {
-        if (db)
-            sqlite3_close(db);
+    /* Without the wait and the hot-journal recovery, a concurrent commit or an
+     * interrupted write made every read fail and the caller silently kept its
+     * defaults. */
+    sqlite3 *db = settings_read_snapshot(path);
+    if (!db)
         return;
-    }
-    sqlite3_exec(db, "BEGIN", NULL, NULL, NULL);
     for (size_t i = 0; i < SETTINGS_COUNT; i++)
         if (settings[i].storage == SET_UI) {
             char value[SETTINGS_VALUE_CAP];

src/cli/runtime_settings.h

@@ -25,6 +25,11 @@ bool cbm_runtime_settings_save_ui(bool enabled, int port);
 /* The HTTP handler supplies its actual bound port for truthful listener state. */
 void cbm_runtime_settings_note_http_port(int port);
 
+/* Every connection to _config.db waits for a committing writer instead of
+ * failing on its lock. Logs config_db.busy_wait once per busy wait. */
+struct sqlite3;
+void cbm_runtime_settings_wait_for_writers(struct sqlite3 *db);
+
 #ifdef CBM_ENABLE_TEST_SEAMS
 /* Single-threaded test teardown only; never exposed in production builds. */
 void cbm_runtime_settings_reset_for_tests(void);

src/main.c

@@ -2483,7 +2483,7 @@ static void main_daemon_ctl_open_browser(int port) {
  * 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) {
+                                            uint8_t update_mask, int port, uint32_t timeout_ms) {
 #ifdef CBM_ENABLE_TEST_SEAMS
     char forced[8];
     if (cbm_safe_getenv("CBM_TEST_DAEMON_UI_CONFIG_REFUSED", forced, sizeof(forced), NULL) &&
@@ -2491,9 +2491,8 @@ static bool main_daemon_ctl_apply_ui_config(cbm_daemon_runtime_client_t *client,
         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;
+    return cbm_daemon_application_client_set_ui_config(
+               client, update_mask, true, port, timeout_ms) == CBM_DAEMON_RUNTIME_APPLICATION_OK;
 }
 
 static int main_daemon_ctl_finish_ui_open(cbm_daemon_runtime_client_t **client_io, int port,
@@ -2712,7 +2711,13 @@ 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. */
+     * startup window; configure the UI before departing.
+     *
+     * Both requests wait for the daemon's reply within the startup budget. The
+     * connection is up, so the reply is an event that comes unless the daemon
+     * dies; the old 1 s bound (MAIN_CONNECT_TIMEOUT_MS, a probe budget) decided
+     * the command whenever saving the UI configuration (journaled commits to
+     * _config.db) ran long on a loaded runner. */
     int ui_port = 0;
     bool ui_configured = false;
     if ((CBM_EMBEDDED_FILE_COUNT > 0)) {
@@ -2722,13 +2727,14 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo
         uint8_t update_mask = 0x03U; /* enabled + port */
         bool context_set =
             main_set_client_context(start_result.client, ".", CBM_MCP_TOOL_PROFILE_ALL, NULL, NULL,
-                                    MAIN_CONNECT_TIMEOUT_MS);
+                                    MAIN_DAEMON_CTL_START_TIMEOUT_MS);
         ui_configured = context_set &&
-                        main_daemon_ctl_apply_ui_config(start_result.client, update_mask, ui_port);
+                        main_daemon_ctl_apply_ui_config(start_result.client, update_mask, ui_port,
+                                                        MAIN_DAEMON_CTL_START_TIMEOUT_MS);
         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.
+             * requests bounded by the startup budget — 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
@@ -3371,9 +3377,13 @@ int main(int argc, char **argv) {
 
     g_daemon_client = bootstrap_result.client;
 
+    /* Startup requests on the connected client wait within the startup budget,
+     * like the bootstrap before them: a 1 s bound on the reply (a probe budget)
+     * ended the server whenever the daemon's UI save ran long, which the smoke's
+     * `--ui=true --port` run saw as a binary without UI assets. */
     if (role == CBM_DAEMON_PROCESS_MCP_CLIENT &&
         !main_set_client_context(g_daemon_client, NULL, tool_profile, NULL, NULL,
-                                 MAIN_CONNECT_TIMEOUT_MS)) {
+                                 bootstrap_config.startup_timeout_ms)) {
         (void)fprintf(stderr, "codebase-memory-mcp: daemon session context was rejected\n");
         (void)cbm_daemon_runtime_client_close(g_daemon_client, MAIN_CLOSE_TIMEOUT_MS);
         g_daemon_client = NULL;
@@ -3391,9 +3401,10 @@ int main(int argc, char **argv) {
         bool explicitly_enabled = false;
         uint8_t update_mask =
             parse_ui_flags(argc, argv, &ui_enabled, &ui_port, &explicitly_enabled);
-        if (update_mask != 0 && cbm_daemon_application_client_set_ui_config(
-                                    g_daemon_client, update_mask, ui_enabled, ui_port,
-                                    MAIN_CONNECT_TIMEOUT_MS) != CBM_DAEMON_RUNTIME_APPLICATION_OK) {
+        if (update_mask != 0 &&
+            cbm_daemon_application_client_set_ui_config(
+                g_daemon_client, update_mask, ui_enabled, ui_port,
+                bootstrap_config.startup_timeout_ms) != CBM_DAEMON_RUNTIME_APPLICATION_OK) {
             (void)fprintf(stderr, "codebase-memory-mcp: daemon UI configuration update failed\n");
             (void)cbm_daemon_runtime_client_close(g_daemon_client, MAIN_CLOSE_TIMEOUT_MS);
             g_daemon_client = NULL;

tests/test_runtime_settings.c

@@ -4,6 +4,7 @@
 #include "foundation/compat_fs.h"
 #include "foundation/platform.h"
 #include "foundation/limits.h"
+#include "foundation/log.h"
 #include "cli/cli.h"
 #include "cli/runtime_settings.h"
 #include "ui/config.h"
@@ -397,7 +398,240 @@ TEST(runtime_settings_response_memory_returns_to_caller_baseline) {
     PASS();
 }
 
+/* A second connection holds _config.db's lock, as another process mid-commit
+ * would. The sink commits the holder the moment the waiter logs
+ * config_db.busy_wait, so the wait is observed as an event, single-threaded and
+ * without a duration. A call that fails on the lock instead never logs, so the
+ * holder stays held through it and the call reports the failure. Results are
+ * captured before any ASSERT so a failure never leaks the lock or the sink. */
+static sqlite3 *g_lock_holder;
+static int g_lock_waits;
+static CBMLogLevel g_lock_saved_level;
+
+static void release_holder_on_wait(const char *line) {
+    if (!g_lock_holder || !strstr(line, "config_db.busy_wait"))
+        return;
+    g_lock_waits++;
+    (void)sqlite3_exec(g_lock_holder, "COMMIT", NULL, NULL, NULL);
+}
+
+static bool hold_config_lock(const char *cache, const char *lock_sql) {
+    char path[512];
+    snprintf(path, sizeof(path), "%s/_config.db", cache);
+    g_lock_waits = 0;
+    if (sqlite3_open(path, &g_lock_holder) != SQLITE_OK ||
+        sqlite3_exec(g_lock_holder, lock_sql, NULL, NULL, NULL) != SQLITE_OK) {
+        sqlite3_close(g_lock_holder);
+        g_lock_holder = NULL;
+        return false;
+    }
+    g_lock_saved_level = cbm_log_get_level();
+    if (g_lock_saved_level > CBM_LOG_WARN)
+        cbm_log_set_level(CBM_LOG_WARN);
+    cbm_log_set_sink(release_holder_on_wait);
+    return true;
+}
+
+static void release_config_lock(void) {
+    cbm_log_set_sink(NULL);
+    cbm_log_set_level(g_lock_saved_level);
+    if (g_lock_holder) {
+        if (!sqlite3_get_autocommit(g_lock_holder))
+            (void)sqlite3_exec(g_lock_holder, "COMMIT", NULL, NULL, NULL);
+        sqlite3_close(g_lock_holder);
+        g_lock_holder = NULL;
+    }
+}
+
+static long long settings_revision(const char *cache) {
+    yyjson_doc *snapshot = read_snapshot(cache);
+    const char *text =
+        snapshot ? yyjson_get_str(yyjson_obj_get(yyjson_doc_get_root(snapshot), "revision")) : NULL;
+    long long revision = text ? strtoll(text, NULL, 10) : -1;
+    yyjson_doc_free(snapshot);
+    return revision;
+}
+
+/* A test that fails between init and fixture_close leaves the settings bound
+ * to its own cache; the lock tests reset first so each one fails on its own
+ * defect, never on a predecessor's leftover state. */
+static bool fresh_fixture_open(settings_fixture_t *fixture) {
+    cbm_runtime_settings_reset_for_tests();
+    return fixture_open(fixture);
+}
+
+/* CI churn: a daemon committing _config.db made an unrelated CLI start exit
+ * with "could not load saved runtime settings" (50 ms window). */
+TEST(runtime_settings_startup_waits_out_a_committing_writer) {
+    settings_fixture_t fixture;
+    ASSERT_TRUE(fresh_fixture_open(&fixture));
+    int status = 0;
+    char *result = apply(fixture.path, "{\"CBM_WORKERS\":\"4\"}", &status);
+    ASSERT_EQ(status, 200);
+    cbm_free(CBM_MEM_CLASS_OTHER, result);
+    ASSERT_TRUE(hold_config_lock(fixture.path, "BEGIN EXCLUSIVE"));
+    bool initialized = cbm_runtime_settings_init(fixture.path);
+    int waits = g_lock_waits;
+    release_config_lock();
+    ASSERT_TRUE(initialized);
+    ASSERT_EQ(waits, 1);
+    ASSERT_STR_EQ(cbm_runtime_getenv("CBM_WORKERS"), "4");
+    fixture_close(&fixture);
+    PASS();
+}
+
+/* The daemon's set_ui_config handler: a concurrent commit failed the save at
+ * once (cbm_config_open had no wait) and made the load silently return the
+ * legacy defaults instead of the saved port. */
+TEST(runtime_settings_ui_save_and_load_wait_out_a_committing_writer) {
+    settings_fixture_t fixture;
+    ASSERT_TRUE(fresh_fixture_open(&fixture));
+    ASSERT_TRUE(cbm_runtime_settings_init(fixture.path));
+    cbm_ui_config_t config = {.ui_enabled = true, .ui_port = 8222};
+    ASSERT_TRUE(cbm_ui_config_save(&config));
+
+    ASSERT_TRUE(hold_config_lock(fixture.path, "BEGIN EXCLUSIVE"));
+    config.ui_port = 8333;
+    bool saved = cbm_ui_config_save(&config);
+    int save_waits = g_lock_waits;
+    release_config_lock();
+    ASSERT_TRUE(saved);
+    ASSERT_TRUE(save_waits >= 1);
+
+    ASSERT_TRUE(hold_config_lock(fixture.path, "BEGIN EXCLUSIVE"));
+    cbm_ui_config_t loaded = {0};
+    cbm_ui_config_load(&loaded);
+    int load_waits = g_lock_waits;
+    release_config_lock();
+    ASSERT_EQ(load_waits, 1);
+    ASSERT_TRUE(loaded.ui_enabled);
+    ASSERT_EQ(loaded.ui_port, 8333);
+    fixture_close(&fixture);
+    PASS();
+}
+
+/* Every `daemon start` on a UI build saved the same two values; the upsert
+ * still updated the rows, fired the revision trigger and committed. */
+TEST(runtime_settings_unchanged_ui_save_does_not_rewrite) {
+    settings_fixture_t fixture;
+    ASSERT_TRUE(fresh_fixture_open(&fixture));
+    ASSERT_TRUE(cbm_runtime_settings_init(fixture.path));
+    cbm_ui_config_t config = {.ui_enabled = true, .ui_port = 8222};
+    ASSERT_TRUE(cbm_ui_config_save(&config));
+    long long first = settings_revision(fixture.path);
+    ASSERT_TRUE(first > 0);
+    ASSERT_TRUE(cbm_ui_config_save(&config));
+    ASSERT_TRUE(cbm_ui_config_save(&config));
+    ASSERT_EQ(settings_revision(fixture.path), first);
+    config.ui_port = 8223;
+    ASSERT_TRUE(cbm_ui_config_save(&config));
+    ASSERT_EQ(settings_revision(fixture.path), first + 1);
+    fixture_close(&fixture);
+    PASS();
+}
+
+/* A process killed mid-write (crash, taskkill /F, power loss) leaves _config.db
+ * with a hot rollback journal. Made portably and deterministically: snapshot the
+ * database and its journal while a spilled write transaction is open, roll the
+ * transaction back, then put the snapshot back. The journal is checked to be
+ * hot (SQLite's magic and a nonzero record count) before the code under test
+ * runs, so these tests cannot pass on a journal that was never hot. */
+static bool copy_file(const char *from, const char *to) {
+    FILE *in = cbm_fopen(from, "rb");
+    FILE *out = in ? cbm_fopen(to, "wb") : NULL;
+    bool ok = in && out;
+    char buffer[8192];
+    size_t got = 0;
+    while (ok && (got = fread(buffer, 1, sizeof(buffer), in)) > 0)
+        ok = fwrite(buffer, 1, got, out) == got;
+    if (in)
+        fclose(in);
+    if (out && fclose(out) != 0)
+        ok = false;
+    return ok;
+}
+
+static bool journal_is_hot(const char *path) {
+    static const unsigned char magic[8] = {0xd9, 0xd5, 0x05, 0xf9, 0x20, 0xa1, 0x63, 0xd7};
+    unsigned char header[12] = {0};
+    FILE *file = cbm_fopen(path, "rb");
+    bool read = file && fread(header, 1, sizeof(header), file) == sizeof(header);
+    if (file)
+        fclose(file);
+    unsigned records = ((unsigned)header[8] << 24) | ((unsigned)header[9] << 16) |
+                       ((unsigned)header[10] << 8) | (unsigned)header[11];
+    return read && memcmp(header, magic, sizeof(magic)) == 0 && records > 0;
+}
+
+static bool leave_hot_journal(const char *cache, const char *update_sql) {
+    char db_path[512], journal_path[512], db_copy[512], journal_copy[512];
+    snprintf(db_path, sizeof(db_path), "%s/_config.db", cache);
+    snprintf(journal_path, sizeof(journal_path), "%s/_config.db-journal", cache);
+    snprintf(db_copy, sizeof(db_copy), "%s/snapshot.db", cache);
+    snprintf(journal_copy, sizeof(journal_copy), "%s/snapshot.journal", cache);
+    sqlite3 *writer = NULL;
+    bool ok = sqlite3_open(db_path, &writer) == SQLITE_OK &&
+              sqlite3_exec(writer, "PRAGMA cache_size=1; BEGIN IMMEDIATE; CREATE TABLE spill(x BLOB)",
+                           NULL, NULL, NULL) == SQLITE_OK &&
+              sqlite3_exec(writer, update_sql, NULL, NULL, NULL) == SQLITE_OK;
+    for (int i = 0; ok && i < 64; i++)
+        ok = sqlite3_exec(writer, "INSERT INTO spill VALUES(randomblob(8192))", NULL, NULL, NULL) ==
+             SQLITE_OK;
+    ok = ok && copy_file(db_path, db_copy) && copy_file(journal_path, journal_copy);
+    if (writer) {
+        (void)sqlite3_exec(writer, "ROLLBACK", NULL, NULL, NULL);
+        sqlite3_close(writer);
+    }
+    ok = ok && journal_is_hot(journal_copy) && copy_file(db_copy, db_path) &&
+         copy_file(journal_copy, journal_path);
+    remove(db_copy);
+    remove(journal_copy);
+    return ok;
+}
+
+/* The hot journal made every later start fail with "could not load saved
+ * runtime settings", permanently: a read-only connection cannot roll it back. */
+TEST(runtime_settings_startup_recovers_a_hot_journal) {
+    settings_fixture_t fixture;
+    ASSERT_TRUE(fresh_fixture_open(&fixture));
+    int status = 0;
+    char *result = apply(fixture.path, "{\"CBM_WORKERS\":\"4\"}", &status);
+    ASSERT_EQ(status, 200);
+    cbm_free(CBM_MEM_CLASS_OTHER, result);
+    ASSERT_TRUE(leave_hot_journal(fixture.path,
+                                  "UPDATE runtime_settings SET value='9' WHERE key='CBM_WORKERS'"));
+    ASSERT_TRUE(cbm_runtime_settings_init(fixture.path));
+    ASSERT_STR_EQ(cbm_runtime_getenv("CBM_WORKERS"), "4");
+    char journal[512];
+    snprintf(journal, sizeof(journal), "%s/_config.db-journal", fixture.path);
+    ASSERT_FALSE(cbm_file_exists(journal));
+    fixture_close(&fixture);
+    PASS();
+}
+
+/* The same journal made the UI load silently fall back to the legacy defaults. */
+TEST(runtime_settings_ui_load_recovers_a_hot_journal) {
+    settings_fixture_t fixture;
+    ASSERT_TRUE(fresh_fixture_open(&fixture));
+    ASSERT_TRUE(cbm_runtime_settings_init(fixture.path));
+    cbm_ui_config_t config = {.ui_enabled = true, .ui_port = 8222};
+    ASSERT_TRUE(cbm_ui_config_save(&config));
+    ASSERT_TRUE(leave_hot_journal(fixture.path,
+                                  "UPDATE runtime_settings SET value='9999' WHERE key='ui_port'"));
+    cbm_ui_config_t loaded = {0};
+    cbm_ui_config_load(&loaded);
+    ASSERT_TRUE(loaded.ui_enabled);
+    ASSERT_EQ(loaded.ui_port, 8222);
+    fixture_close(&fixture);
+    PASS();
+}
+
 SUITE(runtime_settings) {
+    RUN_TEST(runtime_settings_startup_recovers_a_hot_journal);
+    RUN_TEST(runtime_settings_ui_load_recovers_a_hot_journal);
+    RUN_TEST(runtime_settings_startup_waits_out_a_committing_writer);
+    RUN_TEST(runtime_settings_ui_save_and_load_wait_out_a_committing_writer);
+    RUN_TEST(runtime_settings_unchanged_ui_save_does_not_rewrite);
     RUN_TEST(runtime_settings_response_memory_returns_to_caller_baseline);
     RUN_TEST(runtime_settings_preserves_unknown_inherited_values_and_reset);
     RUN_TEST(runtime_settings_canonical_alias_and_actual_listener);

tests/windows/test_daemon_stability.py

@@ -12,6 +12,8 @@ that guard the daemon's PRODUCT contract under stress and misuse:
   with the SAME pid (no second daemon, no restart); ``daemon status`` reports
   the active pid; a ``--port=N`` pointing at an occupied port must not block
   the start (the UI bind is retried in the background by design).
+* ``daemon start --port=N`` while another process holds ``_config.db``'s
+  write lock waits for the daemon's UI save instead of failing the handshake.
 * ``daemon stop`` REFUSES while an MCP session is attached, lists the blocking
   client, and succeeds once the session closes.
 * Crash recovery: after a kill -9 the stale daemon state must clear, a cold
@@ -49,6 +51,7 @@ import os
 import re
 import shutil
 import socket
+import sqlite3
 import subprocess
 import sys
 import tempfile
@@ -247,6 +250,97 @@ def section_start_status_port(binary, work):
         kill_pid(daemon_pid)
 
 
+def daemon_log_contains(cache, needle):
+    try:
+        with open(os.path.join(cache, "logs", "cbm-daemon.log"), "rb") as handle:
+            return needle in handle.read()
+    except OSError:
+        return False
+
+
+def section_start_port_waits_for_settings_writer(binary, work):
+    """`daemon start --port` while another process holds _config.db's write lock.
+
+    section_start_status_port went red on CI runners: the client gave the UI
+    handshake a 1 s reply budget while the daemon was still committing the UI
+    configuration. Holding the lock makes that slow save a state the test can
+    ask for. The holder is released on EVENTS: when the client exits (it gave
+    up under the lock), or WRITER_HOLD_S after the daemon LOGGED that it waits
+    for the writer. WRITER_HOLD_S outlasts the old 1 s budget and stays inside
+    the daemon's 3 s bound for a wedged holder.
+    """
+    writer_hold_s = 1.5
+    cache = section_dirs(work, "writer")
+    daemon_pid = 0
+    holder = None
+    client = None
+    try:
+        warm = run_cli(binary, cache, ["daemon", "start"], timeout=60)
+        if warm.returncode != 0 or not pid_from(out_text(warm)):
+            raise SetupFailure("permanent daemon did not start to create _config.db "
+                               "(rc=%s):\n%s" % (warm.returncode, out_text(warm)))
+        stop = run_cli(binary, cache, ["daemon", "stop"])
+        if stop.returncode != 0 or not wait_status_not_running(binary, cache, 45):
+            print("RED: the warm-up daemon did not stop cleanly")
+            return False
+        holder = sqlite3.connect(os.path.join(cache, "_config.db"), isolation_level=None,
+                                 timeout=0)
+        holder.execute("BEGIN IMMEDIATE")
+        probe = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        probe.bind(("127.0.0.1", 0))
+        port = probe.getsockname()[1]
+        probe.close()
+        client = subprocess.Popen([binary, "daemon", "start", "--port=%d" % port],
+                                  env=cli_env(cache), stdout=subprocess.PIPE,
+                                  stderr=subprocess.STDOUT)
+        waited_at = None
+        deadline = time.monotonic() + 90
+        while client.poll() is None and time.monotonic() < deadline:
+            now = time.monotonic()
+            if waited_at is None and daemon_log_contains(cache, b"config_db.busy_wait"):
+                waited_at = now
+            if waited_at is not None and now - waited_at >= writer_hold_s:
+                break
+            time.sleep(0.05)
+        exited_under_lock = client.poll() is not None
+        # Release by CLOSING the holder: the OS drops a handle's locks when it
+        # closes, so the release cannot be refused. On Windows a COMMIT of this
+        # change-free transaction returned plain SQLITE_BUSY (code 5, no journal,
+        # client still waiting) while the daemon retried the lock: 3 of 5 full
+        # guard runs, on the test's own connection, never on the product's side.
+        holder.close()
+        holder = None
+        output, _ = client.communicate(timeout=90)
+        text = output.decode("utf-8", "replace")
+        daemon_pid = pid_from(text)
+        if exited_under_lock or client.returncode != 0 or "permanent" not in text:
+            print("RED: `daemon start --port` must wait for a settings writer, not give up "
+                  "on the UI handshake (exited_under_lock=%s rc=%s daemon_waited=%s):\n%s"
+                  % (exited_under_lock, client.returncode, waited_at is not None,
+                     excerpt(text)))
+            return False
+        stop = run_cli(binary, cache, ["daemon", "stop"])
+        if stop.returncode != 0 or not wait_status_not_running(binary, cache, 45):
+            print("RED: the daemon did not stop cleanly after the held-writer start")
+            return False
+        daemon_pid = 0
+        client = None
+        print("PASS: `daemon start --port` waits out a held settings writer and starts")
+        return True
+    finally:
+        if holder is not None:
+            holder.close()
+        if client is not None:
+            if client.poll() is None:
+                client.kill()
+                client.wait()
+            # The client may have started a daemon whose pid was never read (a
+            # failure before its output); a survivor locked the guard root's
+            # binary and failed the next run's residue cleanup.
+            run_cli(binary, cache, ["daemon", "stop"], timeout=60)
+        kill_pid(daemon_pid)
+
+
 def section_stop_refuses_busy(binary, work):
     cache = section_dirs(work, "busy")
     daemon_pid = 0
@@ -328,8 +422,13 @@ def section_crash_recovery(binary, work):
                                "(rc=%s):\n%s" % (start.returncode, out_text(start)))
         kill_pid(daemon_pid)
         if not wait_status_not_running(binary, cache, 60):
-            print("RED: after kill -9 of pid %d the stale daemon state never cleared "
-                  "(`daemon status` kept reporting it)" % daemon_pid)
+            # Print what status said: a hot settings journal left by the kill
+            # made every command fail with "could not load saved runtime
+            # settings", which a bare "kept reporting it" hid.
+            last = run_cli(binary, cache, ["daemon", "status"], timeout=30)
+            print("RED: after kill -9 of pid %d the stale daemon state never cleared; "
+                  "last `daemon status` rc=%s:\n%s"
+                  % (daemon_pid, last.returncode, excerpt(out_text(last))))
             return False
         cold = run_cli(binary, cache, ["cli", "list_projects", "{}"], timeout=90)
         if cold.returncode != 0 or "daemon start" in out_text(cold):
@@ -455,6 +554,7 @@ def main():
         section_params,
         section_hook_fail_open,
         section_start_status_port,
+        section_start_port_waits_for_settings_writer,
         section_stop_refuses_busy,
         section_crash_recovery,
         section_churn_stability,