Merge pull request #892 from SS-42/fix/adr-cross-process-lock

fix(mcp): keep ADR reads nonblocking during reindex

Author: DeusData

README.md

@@ -616,9 +616,11 @@ JSON arguments can also be piped on stdin. Inline JSON remains accepted for back
 | `get_code_snippet` | Read source code for a function by qualified name. |
 | `get_architecture` | Codebase overview: languages, packages, routes, hotspots, clusters, ADR. |
 | `search_code` | Grep-like text search within indexed project files. |
-| `manage_adr` | CRUD for Architecture Decision Records. |
+| `manage_adr` | CRUD for Architecture Decision Records. Query modes do not wait behind a same-project reindex; writes remain serialized. |
 | `ingest_traces` | Ingest runtime traces to validate HTTP_CALLS edges. |
 
+`manage_adr` query modes (`get` and `sections`) use the server's cached query store so they can proceed while a same-project reindex is running. If another process publishes a replacement store during reindexing, they can return the pre-publication ADR until idle eviction refreshes that cache. Updates remain serialized through the project mutation guard.
+
 ## Graph Data Model
 
 ### Node Labels

src/daemon/application.c

@@ -1009,6 +1009,12 @@ static bool application_session_mutation_begin(void *context, const char *projec
            application_mutation_begin_internal(session->application, session, project, true);
 }
 
+static bool application_session_mutation_try_begin(void *context, const char *project) {
+    cbm_daemon_application_session_t *session = context;
+    return session &&
+           application_mutation_begin_internal(session->application, session, project, false);
+}
+
 static void application_session_mutation_end(void *context, const char *project) {
     cbm_daemon_application_session_t *session = context;
     if (session) {
@@ -2246,6 +2252,8 @@ static cbm_daemon_runtime_application_session_t *application_session_open(
     cbm_mcp_server_set_index_executor(session->mcp, application_index_execute, session);
     cbm_mcp_server_set_project_mutation_guard(session->mcp, application_session_mutation_begin,
                                               application_session_mutation_end, session);
+    cbm_mcp_server_set_project_mutation_try_guard(session->mcp,
+                                                  application_session_mutation_try_begin);
     session->tool_profile = CBM_MCP_TOOL_PROFILE_ALL;
     session->application = application;
     session->client_id = client_id;

src/main.c

@@ -251,17 +251,21 @@ static bool main_test_worker_project_lock_marker(const main_local_cli_mutation_t
 }
 #endif
 
-static bool main_local_cli_mutation_begin(void *context, const char *project) {
+static bool main_local_cli_mutation_begin_internal(void *context, const char *project, bool wait) {
     main_local_cli_mutation_t *mutation = context;
     if (!mutation || !mutation->manager || !project || !project[0]) {
         return false;
     }
     for (;;) {
-        uint64_t now = cbm_now_ms();
-        uint64_t deadline = now > UINT64_MAX - 100U ? UINT64_MAX : now + 100U;
         cbm_project_lock_lease_t *lease = NULL;
-        cbm_private_file_lock_status_t status =
-            cbm_project_lock_acquire(mutation->manager, project, deadline, NULL, &lease);
+        cbm_private_file_lock_status_t status;
+        if (wait) {
+            uint64_t now = cbm_now_ms();
+            uint64_t deadline = now > UINT64_MAX - 100U ? UINT64_MAX : now + 100U;
+            status = cbm_project_lock_acquire(mutation->manager, project, deadline, NULL, &lease);
+        } else {
+            status = cbm_project_lock_try_acquire(mutation->manager, project, &lease);
+        }
         if (status == CBM_PRIVATE_FILE_LOCK_OK && lease) {
             main_local_cli_lease_t *held = calloc(1, sizeof(*held));
             if (held) {
@@ -292,6 +296,9 @@ static bool main_local_cli_mutation_begin(void *context, const char *project) {
                           "refuse_mutation");
             return false;
         }
+        if (!wait) {
+            return false;
+        }
         if (mutation->feedback && !mutation->waiting_reported) {
             (void)fprintf(mutation->feedback, "Waiting for another CBM mutation of %s...\n",
                           project);
@@ -301,6 +308,14 @@ static bool main_local_cli_mutation_begin(void *context, const char *project) {
     }
 }
 
+static bool main_local_cli_mutation_begin(void *context, const char *project) {
+    return main_local_cli_mutation_begin_internal(context, project, true);
+}
+
+static bool main_local_cli_mutation_try_begin(void *context, const char *project) {
+    return main_local_cli_mutation_begin_internal(context, project, false);
+}
+
 static void main_local_cli_mutation_end(void *context, const char *project) {
     main_local_cli_mutation_t *mutation = context;
     if (!mutation || !project) {
@@ -775,6 +790,8 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo
             if (project_locks) {
                 cbm_mcp_server_set_project_mutation_guard(srv, main_local_cli_mutation_begin,
                                                           main_local_cli_mutation_end, &mutation);
+                cbm_mcp_server_set_project_mutation_try_guard(srv,
+                                                              main_local_cli_mutation_try_begin);
             }
         }
         maintenance_binding_failed = srv && !maintenance_context;

src/mcp/mcp.c

@@ -1532,6 +1532,7 @@ struct cbm_mcp_server {
     cbm_proc_log_cb index_log_callback;
     void *index_log_context;
     cbm_mcp_project_mutation_begin_fn mutation_begin;
+    cbm_mcp_project_mutation_try_begin_fn mutation_try_begin;
     cbm_mcp_project_mutation_end_fn mutation_end;
     void *mutation_context;
     cbm_mcp_quarantine_test_hook_fn quarantine_test_hook;
@@ -1682,14 +1683,27 @@ void cbm_mcp_server_set_project_mutation_guard(cbm_mcp_server_t *srv,
         return;
     }
     srv->mutation_begin = begin;
+    srv->mutation_try_begin = NULL;
     srv->mutation_end = end;
     srv->mutation_context = begin ? context : NULL;
 }
 
+void cbm_mcp_server_set_project_mutation_try_guard(
+    cbm_mcp_server_t *srv, cbm_mcp_project_mutation_try_begin_fn try_begin) {
+    if (srv && srv->mutation_begin) {
+        srv->mutation_try_begin = try_begin;
+    }
+}
+
 static bool mcp_project_mutation_begin(cbm_mcp_server_t *srv, const char *project) {
     return !srv->mutation_begin || srv->mutation_begin(srv->mutation_context, project);
 }
 
+static bool mcp_project_mutation_try_begin(cbm_mcp_server_t *srv, const char *project) {
+    return !srv->mutation_begin ||
+           (srv->mutation_try_begin && srv->mutation_try_begin(srv->mutation_context, project));
+}
+
 static void mcp_project_mutation_end(cbm_mcp_server_t *srv, const char *project) {
     if (srv->mutation_end) {
         srv->mutation_end(srv->mutation_context, project);
@@ -2045,8 +2059,18 @@ static bool quarantine_corrupt_store(cbm_mcp_server_t *srv, const char *project,
 /* Open the right project's .db file for query tools.
  * Caches the connection — reopens only when project changes.
  * Tracks last-access time so the event loop can evict idle stores. */
+typedef enum {
+    STORE_RECOVERY_NONE,
+    STORE_RECOVERY_BUSY,
+    STORE_RECOVERY_TRY_GUARD_UNAVAILABLE,
+} store_recovery_status_t;
+
 static cbm_store_t *resolve_store_internal(cbm_mcp_server_t *srv, const char *project,
-                                           bool mutation_already_held) {
+                                           bool mutation_already_held, bool nonblocking_recovery,
+                                           store_recovery_status_t *recovery_status) {
+    if (recovery_status) {
+        *recovery_status = STORE_RECOVERY_NONE;
+    }
     if (!project) {
         return NULL; /* project is required — no implicit fallback */
     }
@@ -2076,9 +2100,16 @@ static cbm_store_t *resolve_store_internal(cbm_mcp_server_t *srv, const char *pr
             srv->store = NULL;
             bool mutation_acquired = mutation_already_held;
             if (!mutation_acquired) {
-                mutation_acquired = mcp_project_mutation_begin(srv, project);
+                mutation_acquired = nonblocking_recovery
+                                        ? mcp_project_mutation_try_begin(srv, project)
+                                        : mcp_project_mutation_begin(srv, project);
             }
             if (!mutation_acquired) {
+                if (nonblocking_recovery && recovery_status) {
+                    *recovery_status = srv->mutation_try_begin
+                                           ? STORE_RECOVERY_BUSY
+                                           : STORE_RECOVERY_TRY_GUARD_UNAVAILABLE;
+                }
                 return NULL;
             }
 
@@ -2142,7 +2173,7 @@ static cbm_store_t *resolve_store_internal(cbm_mcp_server_t *srv, const char *pr
 }
 
 static cbm_store_t *resolve_store(cbm_mcp_server_t *srv, const char *project) {
-    return resolve_store_internal(srv, project, false);
+    return resolve_store_internal(srv, project, false, false, NULL);
 }
 
 /* Forward decl — definition lives below alongside list_projects. */
@@ -10486,6 +10517,28 @@ static char *adr_read_legacy_file(const char *root_path) {
     "then draft and store. Sections: PURPOSE, STACK, ARCHITECTURE, "               \
     "PATTERNS, TRADEOFFS, PHILOSOPHY."
 
+/* resolve_store opens file-backed projects query-only. A mutation must release
+ * that reader before opening a dedicated writer because atomic publication uses
+ * a self-contained DELETE-mode database that switches back to WAL on write. */
+static cbm_store_t *open_adr_store_for_write(cbm_mcp_server_t *srv, cbm_store_t *resolved,
+                                             cbm_store_t **owned_rw) {
+    if (!srv || !resolved || !owned_rw) {
+        return NULL;
+    }
+    const char *resolved_db_path = cbm_store_db_path(resolved);
+    if (!resolved_db_path) {
+        return resolved;
+    }
+    char *rw_path = heap_strdup(resolved_db_path);
+    if (!rw_path) {
+        return NULL;
+    }
+    invalidate_cached_store(srv);
+    *owned_rw = cbm_store_open_path(rw_path);
+    free(rw_path);
+    return *owned_rw;
+}
+
 static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
     char *project = get_project_arg(args);
     char *mode_str = cbm_mcp_get_string_arg(args, "mode");
@@ -10517,8 +10570,10 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
             true);
     }
 
+    bool write_request =
+        content && (strcmp(mode_str, "update") == 0 || strcmp(mode_str, "store") == 0);
     bool mutation_held = false;
-    if (project) {
+    if (write_request && project) {
         mutation_held = mcp_project_mutation_begin(srv, project);
         if (!mutation_held) {
             free(project);
@@ -10539,11 +10594,21 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
     /* ADRs are stored in the SQLite store (project_summaries), the SAME
      * backend the UI /api/adr endpoints use — so writes via the MCP tool and
      * the UI are visible to each other (#256). */
-    cbm_store_t *resolved = resolve_store_internal(srv, project, mutation_held);
+    store_recovery_status_t recovery_status = STORE_RECOVERY_NONE;
+    cbm_store_t *resolved =
+        resolve_store_internal(srv, project, mutation_held, !write_request, &recovery_status);
     if (!resolved) {
-        char *err = build_no_store_error(project);
-        char *res = cbm_mcp_text_result(err, true);
-        free(err);
+        char *res = NULL;
+        if (recovery_status == STORE_RECOVERY_BUSY) {
+            res = cbm_mcp_text_result("project is busy; retry after indexing", true);
+        } else if (recovery_status == STORE_RECOVERY_TRY_GUARD_UNAVAILABLE) {
+            res =
+                cbm_mcp_text_result("project recovery requires a nonblocking mutation guard", true);
+        } else {
+            char *err = build_no_store_error(project);
+            res = cbm_mcp_text_result(err, true);
+            free(err);
+        }
         if (mutation_held) {
             mcp_project_mutation_end(srv, project);
         }
@@ -10553,66 +10618,57 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
         return res;
     }
 
-    /* resolve_store opens file-backed projects READ-ONLY (query stores must
-     * not mutate the DB). manage_adr is the only resolve_store caller that
-     * WRITES, so it needs a writable handle. For a file-backed project open a
-     * dedicated read-write handle to the same DB file (the project is verified
-     * to exist via resolve_store, so cbm_store_open_path won't create a ghost
-     * DB). For an in-memory / embedded store (db_path == NULL) the resolved
-     * store is already writable — use it directly. */
     cbm_store_t *store = resolved;
     cbm_store_t *owned_rw = NULL;
-    const char *resolved_db_path = cbm_store_db_path(resolved);
-    if (resolved_db_path) {
-        /* Atomic publication leaves a self-contained DELETE-mode database.
-         * Opening the writer switches it back to WAL, which requires an
-         * exclusive lock and therefore fails while resolve_store's read-only
-         * handle is still open. Copy the ACTUAL resolved path first (it may be
-         * a drifted-filename fallback), then release that cached reader before
-         * opening the writer. Request-scoped queries reopen it when needed. */
-        char *rw_path = heap_strdup(resolved_db_path);
-        if (!rw_path) {
-            if (mutation_held) {
-                mcp_project_mutation_end(srv, project);
-            }
-            free(project);
-            free(mode_str);
-            free(content);
-            return cbm_mcp_text_result("failed to allocate ADR store path", true);
-        }
-        invalidate_cached_store(srv);
-        owned_rw = cbm_store_open_path(rw_path);
-        free(rw_path);
-        if (!owned_rw) {
-            char *err = build_no_store_error(project);
-            char *res = cbm_mcp_text_result(err, true);
-            free(err);
+    if (write_request) {
+        store = open_adr_store_for_write(srv, resolved, &owned_rw);
+        if (!store) {
             if (mutation_held) {
                 mcp_project_mutation_end(srv, project);
             }
             free(project);
             free(mode_str);
             free(content);
-            return res;
+            return cbm_mcp_text_result("failed to open writable ADR store", true);
         }
-        store = owned_rw;
     }
 
     /* One-time migration: older versions wrote ADRs to a file at
-     * <root>/.codebase-memory/adr.md. If the store has no ADR yet but that
-     * legacy file exists, import it so nothing is lost on upgrade. */
+     * <root>/.codebase-memory/adr.md. A read never waits for the project lease:
+     * it returns the legacy content immediately and attempts migration only if
+     * a nonblocking acquire succeeds. */
     cbm_adr_t adr;
     memset(&adr, 0, sizeof(adr));
     bool have_adr = (cbm_store_adr_get(store, project, &adr) == CBM_STORE_OK);
-    if (!have_adr) {
+    char *legacy = NULL;
+    if (!have_adr && !write_request) {
         char *root_path = project_root_from_store(store, project);
-        char *legacy = adr_read_legacy_file(root_path);
+        legacy = adr_read_legacy_file(root_path);
         free(root_path);
-        if (legacy) {
-            if (cbm_store_adr_store(store, project, legacy) == CBM_STORE_OK) {
-                have_adr = (cbm_store_adr_get(store, project, &adr) == CBM_STORE_OK);
+        if (legacy && mcp_project_mutation_try_begin(srv, project)) {
+            if (!mcp_request_cancelled(srv)) {
+                /* A publisher may have completed before the lease was granted.
+                 * File-backed stores must reopen after acquisition and trust
+                 * only that generation. Embedded stores have no publication
+                 * boundary, so retain their live handle. */
+                if (cbm_store_db_path(resolved)) {
+                    invalidate_cached_store(srv);
+                    resolved = NULL;
+                    store = NULL;
+                    resolved = resolve_store_internal(srv, project, true, false, NULL);
+                }
+                if (resolved) {
+                    store = open_adr_store_for_write(srv, resolved, &owned_rw);
+                    if (store) {
+                        have_adr = (cbm_store_adr_get(store, project, &adr) == CBM_STORE_OK);
+                        if (!have_adr &&
+                            cbm_store_adr_store(store, project, legacy) == CBM_STORE_OK) {
+                            have_adr = (cbm_store_adr_get(store, project, &adr) == CBM_STORE_OK);
+                        }
+                    }
+                }
             }
-            free(legacy);
+            mcp_project_mutation_end(srv, project);
         }
     }
 
@@ -10621,7 +10677,8 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
     yyjson_mut_doc_set_root(doc, root_obj);
 
     bool is_error = false;
-    if ((strcmp(mode_str, "update") == 0 || strcmp(mode_str, "store") == 0) && content) {
+    const char *adr_content = have_adr ? adr.content : legacy;
+    if (write_request) {
         if (cbm_store_adr_store(store, project, content) == CBM_STORE_OK) {
             yyjson_mut_obj_add_str(doc, root_obj, "status", "updated");
             yyjson_mut_obj_add_str(doc, root_obj, "semantics", "whole_document_replaced");
@@ -10630,10 +10687,10 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
             is_error = true;
         }
     } else if (strcmp(mode_str, "sections") == 0) {
-        adr_list_sections_from_content(doc, root_obj, have_adr ? adr.content : NULL);
+        adr_list_sections_from_content(doc, root_obj, adr_content);
     } else { /* get */
-        if (have_adr && adr.content) {
-            yyjson_mut_obj_add_strcpy(doc, root_obj, "content", adr.content);
+        if (adr_content) {
+            yyjson_mut_obj_add_strcpy(doc, root_obj, "content", adr_content);
         } else {
             yyjson_mut_obj_add_str(doc, root_obj, "content", "");
             yyjson_mut_obj_add_str(doc, root_obj, "status", "no_adr");
@@ -10652,6 +10709,7 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
     if (mutation_held) {
         mcp_project_mutation_end(srv, project);
     }
+    free(legacy);
     free(project);
     free(mode_str);
     free(content);