From 5ca234c2b24c27a8a484e744cc200b93e01f130b Mon Sep 17 00:00:00 2001 From: LeoLan Digital Date: Tue, 25 Aug 2026 15:15:56 +0200 Subject: [PATCH 1/9] fix: persist daemon watches across client disconnects Signed-off-by: LeoLan Digital Signed-off-by: LeoLan Digital Signed-off-by: LeoLan Digital --- src/daemon/application.c | 65 ++++++++++++++------ tests/test_daemon_application.c | 102 ++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 17 deletions(-) diff --git a/src/daemon/application.c b/src/daemon/application.c index 7e4a35be7..ce0cfdaf8 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -149,11 +149,11 @@ struct cbm_daemon_application_job { cbm_daemon_application_job_t *next; }; -/* A watcher-triggered physical job is owned by the exact live sessions that - * currently subscribe to its project/root watch. The callback waiting for the - * job is only a storage waiter; it is deliberately not an ownership - * subscription, so the worker is cancelled when the last matching session - * disconnects even while unrelated daemon sessions remain alive. */ +/* A watcher-triggered physical job is normally owned by the exact live + * sessions subscribed to its project/root watch. In permanent daemon mode the + * watcher itself additionally owns the physical job so background indexing can + * continue while no client session is connected. The callback remains a + * storage waiter in both modes and releases daemon ownership on completion. */ struct cbm_daemon_application_watch_job_subscription { cbm_daemon_application_session_t *session; cbm_daemon_application_job_t *job; @@ -424,7 +424,19 @@ static void application_release_session_watch_locked(cbm_daemon_application_sess if (watch->subscribers > 0) { watch->subscribers--; } - if (watch->subscribers == 0) { + + /* + * A permanent daemon owns the physical project watch independently of + * individual client sessions. Session disconnect releases only that + * session's logical subscription; the watcher remains available for + * background change detection while the daemon generation is alive. + * + * Non-permanent applications retain the historical session-scoped + * lifecycle and remove the physical watch after the final subscriber. + * Project deletion/pruning and daemon shutdown remove persistent watches + * through their dedicated lifecycle paths. + */ + if (!session->application->permanent && watch->subscribers == 0) { application_remove_watch_locked(session->application, watch); } } @@ -3375,10 +3387,13 @@ static int application_background_index(cbm_daemon_application_t *application, cbm_mutex_lock(&application->mutex); cbm_daemon_application_watch_t *watch = require_live_watch ? application_find_watch_locked(application, project_name) : NULL; - bool watch_live = !require_live_watch || - (watch && watch->subscribers > 0 && strcmp(watch->root, canonical_root) == 0); + bool watch_live = + !require_live_watch || + (watch && strcmp(watch->root, canonical_root) == 0 && + (watch->subscribers > 0 || application->permanent)); size_t watch_owner_count = 0; bool watch_subscriptions_ok = true; + bool watcher_owns_job = false; cbm_daemon_application_job_t *job = watch_live ? application_job_subscribe_locked(application, project_key, canonical_root, args, &subscribe_status) @@ -3386,16 +3401,28 @@ static int application_background_index(cbm_daemon_application_t *application, if (job && require_live_watch) { watch_subscriptions_ok = application_watch_job_subscribe_sessions_locked( application, watch, job, &watch_owner_count); - if (watch_subscriptions_ok && watch_owner_count > 0) { - job->watcher_waiters++; - } else if (!watch_subscriptions_ok) { + + if (!watch_subscriptions_ok) { subscribe_status = APPLICATION_JOB_SUBSCRIBE_ALLOCATION_FAILED; - } - /* application_job_subscribe_locked() lends the caller one ordinary - * subscriber. A watcher callback is only a storage waiter: exact live - * session subscriptions above own the physical work. */ - application_job_unsubscribe_locked(job); - if (!watch_subscriptions_ok || watch_owner_count == 0) { + application_job_unsubscribe_locked(job); + job = NULL; + } else if (application->permanent) { + /* + * The ordinary subscription acquired above is retained as daemon + * ownership. This allows the physical watcher to run indexing even + * while no client session currently owns the project watch. + */ + watcher_owns_job = true; + job->watcher_waiters++; + } else if (watch_owner_count > 0) { + /* + * Historical non-permanent behavior: exact live sessions own the + * physical work; the watcher callback itself is only a waiter. + */ + job->watcher_waiters++; + application_job_unsubscribe_locked(job); + } else { + application_job_unsubscribe_locked(job); job = NULL; } } @@ -3428,6 +3455,10 @@ static int application_background_index(cbm_daemon_application_t *application, if (job->watcher_waiters > 0) { job->watcher_waiters--; } + if (watcher_owns_job) { + application_job_unsubscribe_locked(job); + watcher_owns_job = false; + } } else { application_job_unsubscribe_locked(job); } diff --git a/tests/test_daemon_application.c b/tests/test_daemon_application.c index c639ace2f..b0b55e4f6 100644 --- a/tests/test_daemon_application.c +++ b/tests/test_daemon_application.c @@ -3621,6 +3621,107 @@ TEST(daemon_application_watcher_job_follows_exact_live_watch_owners) { PASS(); } + +TEST(daemon_application_permanent_watch_survives_disconnect_and_indexes_without_session) { + app_watch_race_fixture_t fixture; + bool fixture_ready = app_watch_race_fixture_init(&fixture, 47); + + if (fixture_ready) { + cbm_daemon_application_set_permanent(fixture.application, true); + } + + int watch_before_disconnect = + fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + + if (fixture.session) { + fixture.callbacks.session_cancel(fixture.callbacks.context, fixture.session); + } + + int watch_after_cancel = + fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + + if (fixture.session) { + fixture.callbacks.session_close(fixture.callbacks.context, fixture.session); + fixture.session = NULL; + } + + int watch_after_close = + fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + + /* + * No client session remains at this point. A permanent daemon-owned + * physical watch must nevertheless be allowed to launch its background + * indexing job. + */ + app_watcher_index_thread_t request = { + .application = fixture.application, + .project = fixture.project, + .root = fixture.root, + .pause_before_subscribe = false, + .result = -1, + }; + atomic_init(&request.ready, false); + atomic_init(&request.proceed, true); + atomic_init(&request.done, false); + + cbm_thread_t thread; + bool thread_started = + fixture_ready && + watch_after_close == 1 && + cbm_thread_create(&thread, 0, app_watcher_index_thread, &request) == 0; + + bool worker_started = + thread_started && app_wait_for_atomic_int(&fixture.fake.starts, 1); + + int watch_during_job = + fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + + /* + * The fake worker deliberately waits until the test releases it. + * Let the daemon-owned watcher job finish normally. + */ + if (worker_started) { + atomic_store(&fixture.fake.allow_completion, true); + } + + bool request_done = + thread_started && app_wait_for_atomic_bool(&request.done, true); + + bool thread_joined = + request_done && cbm_thread_join(&thread) == 0; + + int watch_after_job = + fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + + int destroys_before_cleanup = + fixture_ready ? atomic_load(&fixture.fake.destroys) : -1; + + bool cleaned = app_watch_race_fixture_finish(&fixture); + + ASSERT_TRUE(fixture_ready); + ASSERT_EQ(watch_before_disconnect, 1); + + /* Permanent mode: disconnect must not unwatch the project. */ + ASSERT_EQ(watch_after_cancel, 1); + ASSERT_EQ(watch_after_close, 1); + + /* No session exists, yet the watcher still owns background indexing. */ + ASSERT_TRUE(thread_started); + ASSERT_TRUE(worker_started); + ASSERT_EQ(watch_during_job, 1); + + ASSERT_TRUE(request_done); + ASSERT_TRUE(thread_joined); + ASSERT_EQ(request.result, 0); + ASSERT_EQ(destroys_before_cleanup, 1); + + /* Completing the background job must not consume the persistent watch. */ + ASSERT_EQ(watch_after_job, 1); + + ASSERT_TRUE(cleaned); + PASS(); +} + TEST(daemon_application_late_watcher_session_owns_active_watcher_job) { app_watch_race_fixture_t fixture; bool fixture_ready = app_watch_race_fixture_init(&fixture, 45); @@ -5116,6 +5217,7 @@ SUITE(daemon_application) { RUN_TEST(daemon_application_stale_watcher_callback_is_rejected_at_job_admission); RUN_TEST(daemon_application_final_cancel_drains_admitted_watcher_job); RUN_TEST(daemon_application_watcher_job_follows_exact_live_watch_owners); + RUN_TEST(daemon_application_permanent_watch_survives_disconnect_and_indexes_without_session); RUN_TEST(daemon_application_late_watcher_session_owns_active_watcher_job); RUN_TEST(daemon_application_serializes_adr_mutation_with_index_job); RUN_TEST(daemon_application_reserved_mutation_delays_worker_start); From b95b4033ea1a1befbd660e7a1917de059da8f705 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 20 Oct 2026 21:14:50 +0200 Subject: [PATCH 2/9] fix(mcp): rank every BM25 candidate when post-window filters would hide them file_pattern, kind and namespace are applied after the 2000-row BM25 candidate window: unlike the label they cannot go into the FTS5 MATCH. With a common query token and a narrow filter the window held none of the filtered rows, and search_graph answered 0 rows -- or a few rows of an under-counted total -- although matching nodes exist. An unlabelled query whose window held only File/Folder/Variable/Project rows had the same failure. When the window is saturated and such a filter is set, or the window held no row passing the filters, the query is now ranked over every candidate (inner LIMIT -1): the FTS-first subquery plan stays, the extra work is linear in the token's match count (the saturation probe already orders every match), and the total is exact. The decision depends only on the query, never on the page, so offset pages stay consistent. Unfiltered and label-only queries that find rows keep the window and are byte-identical. The filter binds, the count query and the saturation probe move into helpers shared by the ranked and the count query. Measured on Django, meilisearch and kubernetes indexes (9 / 9 / 24 filtered queries): zero results 6 -> 0, 6 -> 0, 23 -> 0; rows returned 52 -> 156, 3 -> 303, 19 -> 807; exact totals 0 -> 9, 0 -> 9, 0 -> 24. The graph is unchanged (search-only change). Refs #2386 Signed-off-by: Martin Vogel --- src/mcp/mcp.c | 226 +++++++++++++++++++++++++---------------------- tests/test_mcp.c | 152 +++++++++++++++++++++++++++++++ 2 files changed, 272 insertions(+), 106 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index ad9310186..869df5cc7 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -4019,6 +4019,8 @@ enum { * and then joining/filtering/re-ranking those, we bound all work to O(N) where * N = BM25_INNER_LIMIT rather than the full match set size. */ BM25_INNER_LIMIT = 2000, + /* SQLite reads a negative LIMIT as "no limit": rank every candidate. */ + BM25_INNER_ALL = -1, /* Longest label spliced into the MATCH pre-filter; real labels are short * identifiers, so anything longer simply skips the pre-filter. */ BM25_LABEL_MAX = 64, @@ -4374,6 +4376,96 @@ static char *bm25_render(const bm25_output_row_t *rows, int returned, int total, return json; } +/* The MATCH expression and the filters the ranked and the count query share. + * One binder for both keeps them on identical parameters. */ +typedef struct { + const char *fts_query; + const char *project; + const char *file_like; + const char *label; + const char *kind; + const char *ns; + double label_weight; +} bm25_filters_t; + +static void bm25_bind_opt_text(sqlite3_stmt *st, int idx, const char *value) { + if (value && value[0]) { + sqlite3_bind_text(st, idx, value, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); + } else { + sqlite3_bind_null(st, idx); + } +} + +static void bm25_bind_filters(sqlite3_stmt *st, const bm25_filters_t *f) { + sqlite3_bind_text(st, BM25_BIND_QUERY, f->fts_query, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); + sqlite3_bind_text(st, BM25_BIND_PROJECT, f->project, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); + bm25_bind_opt_text(st, BM25_BIND_FILE, f->file_like); + bm25_bind_opt_text(st, BM25_BIND_LABEL, f->label); + bm25_bind_opt_text(st, BM25_BIND_KIND, f->kind); + bm25_bind_opt_text(st, BM25_BIND_NS, f->ns); + sqlite3_bind_double(st, BM25_BIND_LABEL_WEIGHT, f->label_weight); +} + +/* The top-candidate window is a performance ceiling, not an exact-total + * boundary. Probe one candidate beyond it so a broad query never presents a + * window-local count as the complete match count. The probe shares the + * label-scoped MATCH but is otherwise global to the FTS table; saturation is + * therefore conservatively reported even when later project/path filters + * might discard the hidden candidates. */ +static bool bm25_window_saturated(sqlite3 *db, const char *fts_query) { + enum { PROBE_BIND_OFFSET = 2 }; + const char *probe_sql = "SELECT rowid FROM nodes_fts WHERE nodes_fts MATCH ?1 " + "ORDER BY bm25(nodes_fts), rowid LIMIT 1 OFFSET ?2"; + sqlite3_stmt *probe = NULL; + if (sqlite3_prepare_v2(db, probe_sql, BM25_SQL_AUTO_LEN, &probe, NULL) != SQLITE_OK) { + return true; + } + sqlite3_bind_text(probe, BM25_BIND_QUERY, fts_query, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); + sqlite3_bind_int(probe, PROBE_BIND_OFFSET, BM25_INNER_LIMIT); + bool saturated = sqlite3_step(probe) != SQLITE_DONE; + sqlite3_finalize(probe); + return saturated; +} + +/* Count the candidates that pass the filters among the top `inner_limit` + * (BM25_INNER_ALL: among all of them), with the ranked query's subquery shape + * so FTS5 drives the scan here too. */ +static int bm25_count_candidates(sqlite3 *db, const bm25_filters_t *f, int inner_limit) { + const char *count_sql = + "SELECT COUNT(*) FROM (" + " SELECT fts.rowid FROM (" + " SELECT rowid FROM nodes_fts WHERE nodes_fts MATCH ?1" + " ORDER BY " BM25_WEIGHTS " LIMIT ?3" + " ) fts " + " JOIN nodes n ON n.id = fts.rowid " + " WHERE n.project = ?2 " + /* MIRRORS the ranked query's filter verbatim — same weights, same + * label exclusions. Changing one alone reports a total that does + * not describe the rows returned. */ + " AND " BM25_UNLABELLED_EXCLUSION " " + " AND (?6 IS NULL OR n.file_path LIKE ?6)" + " AND (?7 IS NULL OR n.label = ?7)" + " AND (?9 IS NULL OR (CASE WHEN json_valid(n.properties) " + " THEN json_extract(n.properties,'$.kind') END) = ?9)" + " AND (?10 IS NULL OR (CASE WHEN json_valid(n.properties) " + " THEN json_extract(n.properties,'$.obj_key') END) " + " LIKE ('%/%/' || replace(replace(replace(?10,'\\','\\\\'),'%','\\%')," + " '_','\\_') || '/%') ESCAPE '\\')" + ")"; + int total = 0; + sqlite3_stmt *cs = NULL; + if (sqlite3_prepare_v2(db, count_sql, BM25_SQL_AUTO_LEN, &cs, NULL) != SQLITE_OK) { + return 0; + } + bm25_bind_filters(cs, f); + sqlite3_bind_int(cs, BM25_BIND_LIMIT, inner_limit); + if (sqlite3_step(cs) == SQLITE_ROW) { + total = sqlite3_column_int(cs, 0); + } + sqlite3_finalize(cs); + return total; +} + /* Run the BM25 full-text search path and return the JSON result string. * Returns NULL if FTS5 is unavailable or the query produced no usable tokens, * in which case the caller falls back to the regex-based search path. */ @@ -4461,117 +4553,39 @@ static char *bm25_search(cbm_store_t *store, const char *project, const char *qu free(file_like); return NULL; } - sqlite3_bind_text(stmt, BM25_BIND_QUERY, fts_query, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); - sqlite3_bind_text(stmt, BM25_BIND_PROJECT, project, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); + const bm25_filters_t filters = {.fts_query = fts_query, + .project = project, + .file_like = file_like, + .label = label, + .kind = kind, + .ns = ns, + .label_weight = label_weight}; + bm25_bind_filters(stmt, &filters); sqlite3_bind_int(stmt, BM25_BIND_LIMIT, limit > 0 ? limit : BM25_DEFAULT_LIMIT); sqlite3_bind_int(stmt, BM25_BIND_OFFSET, offset > 0 ? offset : 0); - sqlite3_bind_int(stmt, BM25_BIND_INNER, BM25_INNER_LIMIT); - if (file_like) { - sqlite3_bind_text(stmt, BM25_BIND_FILE, file_like, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); - } else { - sqlite3_bind_null(stmt, BM25_BIND_FILE); - } - if (label && label[0]) { - sqlite3_bind_text(stmt, BM25_BIND_LABEL, label, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); - } else { - sqlite3_bind_null(stmt, BM25_BIND_LABEL); - } sqlite3_bind_text(stmt, BM25_BIND_EXACT, query, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); - if (kind && kind[0]) { - sqlite3_bind_text(stmt, BM25_BIND_KIND, kind, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); - } else { - sqlite3_bind_null(stmt, BM25_BIND_KIND); - } - if (ns && ns[0]) { - sqlite3_bind_text(stmt, BM25_BIND_NS, ns, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); - } else { - sqlite3_bind_null(stmt, BM25_BIND_NS); - } - sqlite3_bind_double(stmt, BM25_BIND_LABEL_WEIGHT, label_weight); - /* Count hits within the same inner-limit window — capped at BM25_INNER_LIMIT. - * Uses the identical subquery structure so the FTS5 early-exit applies here too. */ - int total = 0; - { - const char *count_sql = - "SELECT COUNT(*) FROM (" - " SELECT fts.rowid FROM (" - " SELECT rowid FROM nodes_fts WHERE nodes_fts MATCH ?1" - " ORDER BY " BM25_WEIGHTS " LIMIT ?3" - " ) fts " - " JOIN nodes n ON n.id = fts.rowid " - " WHERE n.project = ?2 " - /* MIRRORS the ranked query's filter verbatim — same weights, same - * label exclusions. Changing one alone reports a total that does - * not describe the rows returned. */ - " AND " BM25_UNLABELLED_EXCLUSION " " - " AND (?6 IS NULL OR n.file_path LIKE ?6)" - " AND (?7 IS NULL OR n.label = ?7)" - " AND (?9 IS NULL OR (CASE WHEN json_valid(n.properties) " - " THEN json_extract(n.properties,'$.kind') END) = ?9)" - " AND (?10 IS NULL OR (CASE WHEN json_valid(n.properties) " - " THEN json_extract(n.properties,'$.obj_key') END) " - " LIKE ('%/%/' || replace(replace(replace(?10,'\\','\\\\'),'%','\\%')," - " '_','\\_') || '/%') ESCAPE '\\')" - ")"; - sqlite3_stmt *cs = NULL; - if (sqlite3_prepare_v2(db, count_sql, BM25_SQL_AUTO_LEN, &cs, NULL) == SQLITE_OK) { - sqlite3_bind_text(cs, BM25_BIND_QUERY, fts_query, BM25_SQL_AUTO_LEN, - MCP_SQLITE_TRANSIENT); - sqlite3_bind_text(cs, BM25_BIND_PROJECT, project, BM25_SQL_AUTO_LEN, - MCP_SQLITE_TRANSIENT); - sqlite3_bind_int(cs, BM25_BIND_LIMIT, BM25_INNER_LIMIT); - sqlite3_bind_double(cs, BM25_BIND_LABEL_WEIGHT, label_weight); - if (file_like) { - sqlite3_bind_text(cs, BM25_BIND_FILE, file_like, BM25_SQL_AUTO_LEN, - MCP_SQLITE_TRANSIENT); - } else { - sqlite3_bind_null(cs, BM25_BIND_FILE); - } - if (label && label[0]) { - sqlite3_bind_text(cs, BM25_BIND_LABEL, label, BM25_SQL_AUTO_LEN, - MCP_SQLITE_TRANSIENT); - } else { - sqlite3_bind_null(cs, BM25_BIND_LABEL); - } - if (kind && kind[0]) { - sqlite3_bind_text(cs, BM25_BIND_KIND, kind, BM25_SQL_AUTO_LEN, - MCP_SQLITE_TRANSIENT); - } else { - sqlite3_bind_null(cs, BM25_BIND_KIND); - } - if (ns && ns[0]) { - sqlite3_bind_text(cs, BM25_BIND_NS, ns, BM25_SQL_AUTO_LEN, MCP_SQLITE_TRANSIENT); - } else { - sqlite3_bind_null(cs, BM25_BIND_NS); - } - if (sqlite3_step(cs) == SQLITE_ROW) { - total = sqlite3_column_int(cs, 0); - } - sqlite3_finalize(cs); - } - } - - /* The top-candidate window is a performance ceiling, not an exact-total - * boundary. Probe one candidate beyond it so a broad query never presents - * a window-local count as the complete match count. The probe shares the - * label-scoped MATCH but is otherwise global to the FTS table; saturation - * is therefore conservatively reported even when later project/path - * filters might discard the hidden candidates. */ - bool candidate_window_saturated = true; - { - const char *probe_sql = "SELECT rowid FROM nodes_fts WHERE nodes_fts MATCH ?1 " - "ORDER BY bm25(nodes_fts), rowid LIMIT 1 OFFSET ?2"; - sqlite3_stmt *probe = NULL; - if (sqlite3_prepare_v2(db, probe_sql, BM25_SQL_AUTO_LEN, &probe, NULL) == SQLITE_OK) { - sqlite3_bind_text(probe, BM25_BIND_QUERY, fts_query, BM25_SQL_AUTO_LEN, - MCP_SQLITE_TRANSIENT); - sqlite3_bind_int(probe, BM25_BIND_PROJECT, BM25_INNER_LIMIT); - int probe_step = sqlite3_step(probe); - candidate_window_saturated = probe_step != SQLITE_DONE; - sqlite3_finalize(probe); - } + /* file_pattern, kind and namespace cannot be put into the MATCH the way + * the label is, so they filter AFTER the candidate window. When more + * candidates exist beyond it, a narrow filter can find none of its rows in + * the top BM25_INNER_LIMIT and the search answered 0 rows although + * matching nodes exist. A saturated window under such a filter -- or one + * that held no row passing the filters at all -- is therefore ranked over + * every candidate instead: the FTS-first subquery plan stays, the extra + * work is linear in the token's match count (the saturation probe already + * orders every match), and the total becomes exact. The decision depends + * only on the query, never on the page, so offset pages stay consistent. + * Unfiltered and label-only queries that find rows keep the window. */ + bool saturated = bm25_window_saturated(db, fts_query); + bool post_window_filter = file_like || (kind && kind[0]) || (ns && ns[0]); + int inner_limit = saturated && post_window_filter ? BM25_INNER_ALL : BM25_INNER_LIMIT; + int total = bm25_count_candidates(db, &filters, inner_limit); + if (saturated && total == 0 && inner_limit != BM25_INNER_ALL) { + inner_limit = BM25_INNER_ALL; + total = bm25_count_candidates(db, &filters, inner_limit); } + bool candidate_window_saturated = saturated && inner_limit != BM25_INNER_ALL; + sqlite3_bind_int(stmt, BM25_BIND_INNER, inner_limit); int row_cap = limit > 0 ? limit : BM25_DEFAULT_LIMIT; bm25_output_row_t *rows = diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 28d1946b7..2ef16fefe 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -4729,6 +4729,156 @@ TEST(tool_search_graph_bm25_zero_hint_names_the_label) { PASS(); } +/* file_pattern, kind and namespace filter after the BM25 candidate window + * (they cannot be put into the MATCH like the label). 2001 same-label nodes + * with a short path fill the window ahead of 3 that rank strictly lower (long + * path) and are the only ones passing each filter, so a windowed search + * answered 0 rows. A saturated window under such a filter is ranked over + * every candidate: all 3 rows, an exact total, consistent offset pages. */ +static void bm25_post_window_fixture(cbm_store_t *store, const char *project) { + cbm_store_upsert_project(store, project, "/tmp/bm25-post-window"); + for (int i = 0; i < 2004; i++) { + bool rare = i >= 2001; + char qn[64]; + snprintf(qn, sizeof(qn), "bm25-post-window.%s_%04d", rare ? "rare" : "fn", i); + cbm_node_t node = { + .project = project, + .label = "Function", + .name = "postneedle", + .qualified_name = qn, + .file_path = rare ? "deeply/nested/package/layout/registry.c" : "many.c", + .start_line = i + 1, + .end_line = i + 1, + .properties_json = + rare ? "{\"kind\":\"Deployment\",\"obj_key\":\"apps/Deployment/prod/web\"}" : NULL}; + cbm_store_upsert_node(store, &node); + } + cbm_store_exec(store, "INSERT INTO nodes_fts(nodes_fts) VALUES('delete-all');"); + cbm_store_exec(store, "INSERT INTO nodes_fts(rowid, name, qualified_name, label, " + "file_path) SELECT id, cbm_camel_split(name), qualified_name, " + "label, file_path FROM nodes;"); +} + +static char *bm25_post_window_search(cbm_mcp_server_t *srv, const char *filter_json, int limit, + int offset) { + char req[512]; + snprintf(req, sizeof(req), + "{\"jsonrpc\":\"2.0\",\"id\":2424,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_graph\",\"arguments\":{" + "\"project\":\"bm25-post-window\",\"query\":\"postneedle\"%s," + "\"limit\":%d,\"offset\":%d,\"format\":\"json\"}}}", + filter_json, limit, offset); + char *resp = cbm_mcp_server_handle(srv, req); + char *inner = resp ? extract_text_content(resp) : NULL; + free(resp); + return inner; +} + +TEST(tool_search_graph_bm25_post_window_filters_rank_every_candidate) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + cbm_mcp_server_set_project(srv, "bm25-post-window"); + bm25_post_window_fixture(store, "bm25-post-window"); + + static const char *const filters[] = { + ",\"file_pattern\":\"deeply/nested\"", + ",\"kind\":\"Deployment\"", + ",\"namespace\":\"prod\"", + }; + for (size_t i = 0; i < sizeof(filters) / sizeof(filters[0]); i++) { + char *inner = bm25_post_window_search(srv, filters[i], 10, 0); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "\"search_mode\":\"bm25\"")); + ASSERT_NOT_NULL(strstr(inner, "\"bm25-post-window.rare_2001\"")); + ASSERT_NOT_NULL(strstr(inner, "\"bm25-post-window.rare_2003\"")); + ASSERT_NULL(strstr(inner, "\"bm25-post-window.fn_")); + ASSERT_NOT_NULL(strstr(inner, "\"total\":3,")); + ASSERT_NOT_NULL(strstr(inner, "\"total_relation\":\"eq\"")); + ASSERT_NULL(strstr(inner, "candidate_window_saturated")); + free(inner); + } + + /* Offset pages over the exhaustive ranking are disjoint and complete. */ + char *page1 = bm25_post_window_search(srv, filters[0], 2, 0); + char *page2 = bm25_post_window_search(srv, filters[0], 2, 2); + ASSERT_NOT_NULL(page1); + ASSERT_NOT_NULL(page2); + ASSERT_NOT_NULL(strstr(page1, "\"returned\":2,")); + ASSERT_NOT_NULL(strstr(page2, "\"returned\":1,")); + ASSERT_NOT_NULL(strstr(page2, "\"total\":3,")); + for (int i = 2001; i < 2004; i++) { + char qn[64]; + snprintf(qn, sizeof(qn), "\"bm25-post-window.rare_%04d\"", i); + ASSERT_TRUE((strstr(page1, qn) != NULL) != (strstr(page2, qn) != NULL)); + } + free(page1); + free(page2); + + /* Without a post-window filter the window stays and says so. */ + char *inner = bm25_post_window_search(srv, "", 10, 0); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "\"total\":2000,")); + ASSERT_NOT_NULL(strstr(inner, "\"candidate_window_saturated\":true")); + free(inner); + + cbm_mcp_server_free(srv); + PASS(); +} + +/* An unlabelled query whose whole candidate window is File/Folder/Variable/ + * Project rows (excluded from unlabelled ranking) found no row at all, while + * a matching definition sat just beyond the window: 2001 short Folder rows + * outrank one Function with a long path. A window with no row passing the + * filters is ranked over every candidate instead. */ +TEST(tool_search_graph_bm25_empty_window_ranks_every_candidate) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + const char *project = "bm25-empty-window"; + cbm_mcp_server_set_project(srv, project); + ASSERT_EQ(cbm_store_upsert_project(store, project, "/tmp/bm25-empty-window"), CBM_STORE_OK); + for (int i = 0; i < 2002; i++) { + bool def = i == 2001; + char qn[64]; + snprintf(qn, sizeof(qn), "bm25-empty-window.%s_%04d", def ? "def" : "dir", i); + cbm_node_t node = {.project = project, + .label = def ? "Function" : "Folder", + .name = "foldneedle", + .qualified_name = qn, + .file_path = def ? "deeply/nested/package/layout/registry.c" : "d", + .start_line = i + 1, + .end_line = i + 1}; + ASSERT_GT(cbm_store_upsert_node(store, &node), 0); + } + ASSERT_EQ(cbm_store_exec(store, "INSERT INTO nodes_fts(nodes_fts) VALUES('delete-all');"), + CBM_STORE_OK); + ASSERT_EQ(cbm_store_exec(store, "INSERT INTO nodes_fts(rowid, name, qualified_name, label, " + "file_path) SELECT id, cbm_camel_split(name), qualified_name, " + "label, file_path FROM nodes;"), + CBM_STORE_OK); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":2425,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_graph\",\"arguments\":{" + "\"project\":\"bm25-empty-window\",\"query\":\"foldneedle\",\"limit\":10," + "\"format\":\"json\"}}}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + free(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "\"bm25-empty-window.def_2001\",\"Function\"")); + ASSERT_NULL(strstr(inner, "\"Folder\"")); + ASSERT_NOT_NULL(strstr(inner, "\"total\":1,")); + ASSERT_NOT_NULL(strstr(inner, "\"total_relation\":\"eq\"")); + free(inner); + + cbm_mcp_server_free(srv); + PASS(); +} + /* The definition whose NAME is the query ranks first: the `Table` class above * its own methods and above test methods that repeat "table" three times; the * `get_object_or_404` function above the test methods that contain it. */ @@ -25336,6 +25486,8 @@ SUITE(mcp) { RUN_TEST(tool_search_graph_bm25_label_scope_keeps_rank_order); RUN_TEST(tool_search_graph_bm25_serves_unranked_labels_when_named); RUN_TEST(tool_search_graph_bm25_zero_hint_names_the_label); + RUN_TEST(tool_search_graph_bm25_post_window_filters_rank_every_candidate); + RUN_TEST(tool_search_graph_bm25_empty_window_ranks_every_candidate); RUN_TEST(tool_search_graph_bm25_ranks_exact_name_first); RUN_TEST(tool_search_graph_rejects_bm25_and_semantic_query_together); RUN_TEST(tool_search_graph_semantic_ceiling_never_emits_unusable_continuation); From ad7899399bf4ec6f1dfcaf195bba99160908ccc7 Mon Sep 17 00:00:00 2001 From: Gaurav Dubey Date: Wed, 21 Oct 2026 13:23:06 +0000 Subject: [PATCH 3/9] fix(routes): read Go 1.22 ServeMux method and host patterns Since Go 1.22 a net/http ServeMux pattern may name a method and a host before the path: mux.HandleFunc("POST /v1/payments", h), "GET /v1/payments/{id}", "api.example.com/v1/x". Every route registration gate wanted the first argument to open with a slash (the extractor's handler capture, find_route_path_in_args in the parallel pass, and the three gates in the sequential pass), so such a registration minted no Route and no HANDLES edge, while the same file written with "/v1/payments" had both. cbm_service_pattern_mux_split reads the "[METHOD ][HOST]/[PATH]" grammar. For Go files only, the path of such a pattern now registers the route, the Route takes the pattern's method when the callee names none (Handle/HandleFunc are ANY), and a host is dropped from the path. Both passes share cbm_pipeline_mux_route_path and cbm_pipeline_route_reg_method so their Routes stay identical. Other languages and plain "/path" patterns take the unchanged path. Fixes #2537 Signed-off-by: Gaurav Dubey --- internal/cbm/extract_calls.c | 9 ++- internal/cbm/service_patterns.c | 44 +++++++++++ internal/cbm/service_patterns.h | 11 +++ src/pipeline/pass_calls.c | 30 ++++++-- src/pipeline/pass_parallel.c | 37 ++++++++- src/pipeline/pipeline_internal.h | 15 ++++ tests/test_extraction.c | 35 +++++++++ tests/test_infrascan.c | 34 +++++++++ tests/test_pipeline.c | 125 +++++++++++++++++++++++++++++++ 9 files changed, 327 insertions(+), 13 deletions(-) diff --git a/internal/cbm/extract_calls.c b/internal/cbm/extract_calls.c index 4eb02659c..f16b8ecf8 100644 --- a/internal/cbm/extract_calls.c +++ b/internal/cbm/extract_calls.c @@ -4553,8 +4553,13 @@ CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBML call.first_string_arg = route_path_with_group_prefix(ctx, node, call.callee_name, call.first_string_arg); /* A slashless registration's string action would only be - * guessed at (#1146): mint its Route without a handler. */ - if (call.first_string_arg && call.first_string_arg[0] == '/' && + * guessed at (#1146): mint its Route without a handler. A Go + * ServeMux pattern names its method or host before the slash + * ("POST /v1/x", #2537) and keeps its handler. */ + if (call.first_string_arg && + (call.first_string_arg[0] == '/' || + (ctx->language == CBM_LANG_GO && + cbm_service_pattern_mux_split(call.first_string_arg, NULL, 0) != NULL)) && !(laravel_uri && php_route_action_is_string(node))) { call.second_arg_name = extract_handler_arg(ctx, args); } diff --git a/internal/cbm/service_patterns.c b/internal/cbm/service_patterns.c index 7d428b8e3..1eaa50941 100644 --- a/internal/cbm/service_patterns.c +++ b/internal/cbm/service_patterns.c @@ -1183,6 +1183,50 @@ const char *cbm_service_pattern_route_method(const char *callee_name) { return NULL; } +static bool mux_is_blank(char ch) { + return ch == ' ' || ch == '\t'; +} + +const char *cbm_service_pattern_mux_split(const char *pattern, char *method, size_t method_sz) { + if (method && method_sz > 0) { + method[0] = '\0'; + } + if (!pattern || pattern[0] == '/') { + return NULL; + } + /* METHOD: Go cuts it at the first space or tab and trims the rest. */ + const char *rest = pattern; + size_t method_len = 0; + while (pattern[method_len] >= 'A' && pattern[method_len] <= 'Z') { + method_len++; + } + if (method_len > 0 && mux_is_blank(pattern[method_len])) { + rest = pattern + method_len; + while (mux_is_blank(*rest)) { + rest++; + } + } else { + method_len = 0; + } + /* HOST: everything before the path's slash, with no blank or line break. */ + const char *slash = rest; + while (*slash && *slash != '/' && !mux_is_blank(*slash) && *slash != '\n' && *slash != '\r') { + slash++; + } + if (*slash != '/' || (method_len == 0 && slash == rest)) { + return NULL; + } + if (method_len > 0 && method) { + if (method_len >= method_sz) { + method[0] = '\0'; + return NULL; + } + memcpy(method, pattern, method_len); + method[method_len] = '\0'; + } + return slash; +} + const char *cbm_service_pattern_broker(const char *resolved_qn) { if (!resolved_qn) { return NULL; diff --git a/internal/cbm/service_patterns.h b/internal/cbm/service_patterns.h index f06c85571..59f3ea0af 100644 --- a/internal/cbm/service_patterns.h +++ b/internal/cbm/service_patterns.h @@ -84,6 +84,17 @@ const char *cbm_service_pattern_http_method(const char *callee_name); * Returns NULL if not a known route registration method. */ const char *cbm_service_pattern_route_method(const char *callee_name); +/* Go 1.22+ net/http ServeMux pattern "[METHOD ][HOST]/[PATH]" (#2537): + * "POST /v1/x", "GET api.example.com/v1/x", "api.example.com/v1/x". Returns a + * pointer to PATH's leading slash inside `pattern`, or NULL when the pattern + * names neither a method nor a host before a slash (a plain "/v1/x" is NULL + * too: there is nothing to split). METHOD is a run of uppercase letters ended + * by spaces or tabs; it is copied to `method` (NUL-terminated, "" when the + * pattern names none) when `method` is non-NULL; a method that does not fit + * `method_sz` then makes the pattern NULL. The grammar is Go's: callers gate + * on the file language. */ +const char *cbm_service_pattern_mux_split(const char *pattern, char *method, size_t method_sz); + /* Get the broker name for an async QN (e.g., "pubsub" from a Pub/Sub QN). * Returns NULL if not an async pattern. */ const char *cbm_service_pattern_broker(const char *resolved_qn); diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index f3570904d..0e7069ab6 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -571,6 +571,21 @@ cbm_svc_kind_t cbm_pipeline_external_import_service(cbm_external_import_index_t return CBM_SVC_NONE; } +/* The path a route registration's first argument names: the argument itself + * when it opens with a slash, or in Go the path of a ServeMux pattern that + * names a method or host first ("POST /v1/x", #2537). NULL when it names no + * path. MUST match pass_parallel.c find_route_path_in_args. */ +static const char *route_reg_path(const CBMCall *call, CBMLanguage lang) { + const char *arg = call->first_string_arg; + if (!arg) { + return NULL; + } + if (arg[0] == '/') { + return arg; + } + return cbm_pipeline_mux_route_path(arg, lang); +} + /* Handle a route registration call: create Route node + HANDLES edge. * route_mount is the framework mount of the registering file ("/api" for a * Laravel 11+ `withRouting(api: ...)` file, #1146) or "". */ @@ -578,10 +593,11 @@ static void handle_route_registration(cbm_pipeline_ctx_t *ctx, const CBMCall *ca const cbm_gbuf_node_t *source_node, const char *module_qn, const char **imp_keys, const char **imp_vals, int imp_count, const char *route_mount, CBMLanguage lang) { - const char *method = cbm_service_pattern_route_method(call->callee_name); + char mux_method[CBM_SZ_16]; + const char *method = cbm_pipeline_route_reg_method(call, lang, mux_method, sizeof(mux_method)); char mounted[CBM_SZ_256]; const char *path = - cbm_laravel_mount_route(route_mount, call->first_string_arg, mounted, sizeof(mounted)); + cbm_laravel_mount_route(route_mount, route_reg_path(call, lang), mounted, sizeof(mounted)); char route_qn[CBM_ROUTE_QN_SIZE]; char cpath[CBM_SZ_256]; snprintf(route_qn, sizeof(route_qn), "__route__%s__%s", method ? method : "ANY", @@ -815,7 +831,7 @@ static void emit_classified_edge(cbm_pipeline_ctx_t *ctx, const CBMCall *call, bool suppress_plain_calls, const char *route_mount, CBMLanguage lang) { cbm_svc_kind_t svc = cbm_service_pattern_match(res->qualified_name); - if (svc == CBM_SVC_ROUTE_REG && call->first_string_arg && call->first_string_arg[0] == '/') { + if (svc == CBM_SVC_ROUTE_REG && route_reg_path(call, lang) != NULL) { handle_route_registration(ctx, call, source, module_qn, imp_keys, imp_vals, imp_count, route_mount, lang); return; @@ -1042,8 +1058,8 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, const CBM * apps. Classify by callee suffix + path-shaped first arg, exactly * like the parallel path's callee_suffix fallback; without this the * sequential path minted zero Route nodes for such files. */ - if (cbm_service_pattern_route_method(call->callee_name) != NULL && call->first_string_arg && - call->first_string_arg[0] == '/') { + if (cbm_service_pattern_route_method(call->callee_name) != NULL && + route_reg_path(call, lang) != NULL) { handle_route_registration(ctx, call, source_node, module_qn, imp_keys, imp_vals, imp_count, route_mount, lang); return SKIP_ONE; @@ -1220,8 +1236,8 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, const CBM * unresolved-callee fallback above mints, and nothing else. Dropping * the whole call lost every GET registration in a mixed-language repo * while POST (no `post` to collide with) survived. */ - if (cbm_service_pattern_route_method(call->callee_name) != NULL && call->first_string_arg && - call->first_string_arg[0] == '/') { + if (cbm_service_pattern_route_method(call->callee_name) != NULL && + route_reg_path(call, lang) != NULL) { handle_route_registration(ctx, call, source_node, module_qn, imp_keys, imp_vals, imp_count, route_mount, lang); return SKIP_ONE; diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index 73ce742ad..380110295 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -2263,13 +2263,41 @@ static bool is_route_path_shaped(const char *val) { return val && val[0] == '/' && !cbm_service_pattern_is_comment_text(val); } -static const char *find_route_path_in_args(const CBMCall *call, const char **out_handler) { +const char *cbm_pipeline_mux_route_path(const char *arg, CBMLanguage lang) { + if (lang != CBM_LANG_GO || !arg || cbm_service_pattern_is_comment_text(arg)) { + return NULL; + } + return cbm_service_pattern_mux_split(arg, NULL, 0); +} + +const char *cbm_pipeline_route_reg_method(const CBMCall *call, CBMLanguage lang, char *buf, + size_t buf_sz) { + const char *method = cbm_service_pattern_route_method(call->callee_name); + if (method && strcmp(method, "ANY") != 0) { + return method; + } + if (cbm_pipeline_mux_route_path(call->first_string_arg, lang) && + cbm_service_pattern_mux_split(call->first_string_arg, buf, buf_sz) && buf[0] != '\0') { + return buf; + } + return method; +} + +static const char *find_route_path_in_args(const CBMCall *call, const char **out_handler, + CBMLanguage lang) { *out_handler = NULL; /* 1. First string arg starting with / */ if (is_route_path_shaped(call->first_string_arg)) { *out_handler = call->second_arg_name; return call->first_string_arg; } + /* 1b. Go ServeMux pattern with a method or host before the path (#2537). + * MUST match pass_calls.c route_reg_path. */ + const char *mux_path = cbm_pipeline_mux_route_path(call->first_string_arg, lang); + if (mux_path) { + *out_handler = call->second_arg_name; + return mux_path; + } /* 2. Keyword args (prefix=, path=, route=, etc.) */ const char *found = NULL; for (int ai = 0; ai < call->arg_count && !found; ai++) { @@ -2428,7 +2456,8 @@ static void emit_route_registration(cbm_gbuf_t *gbuf, const cbm_gbuf_node_t *sou const cbm_registry_t *registry, const cbm_gbuf_t *main_gbuf, const char **ik, const char **iv, int ic, const char *route_mount, CBMLanguage lang) { - const char *method = cbm_service_pattern_route_method(call->callee_name); + char mux_method[CBM_SZ_16]; + const char *method = cbm_pipeline_route_reg_method(call, lang, mux_method, sizeof(mux_method)); char mounted[CBM_SZ_256]; route_path = cbm_laravel_mount_route(route_mount, route_path, mounted, sizeof(mounted)); char rqn[CBM_ROUTE_QN_SIZE]; @@ -2813,7 +2842,7 @@ static void emit_service_edge(cbm_gbuf_t *gbuf, const cbm_gbuf_node_t *source, if (svc == CBM_SVC_ROUTE_REG) { const char *handler_ref = NULL; - const char *route_path = find_route_path_in_args(call, &handler_ref); + const char *route_path = find_route_path_in_args(call, &handler_ref, lang); if (route_path) { emit_route_registration(gbuf, source, call, route_path, handler_ref, module_qn, registry, main_gbuf, imp_keys, imp_vals, imp_count, route_mount, @@ -2876,7 +2905,7 @@ static void emit_xlang_refused_route(cbm_gbuf_t *gbuf, const cbm_gbuf_node_t *so return; } const char *handler_ref = NULL; - const char *route_path = find_route_path_in_args(call, &handler_ref); + const char *route_path = find_route_path_in_args(call, &handler_ref, lang); if (route_path) { emit_route_registration(gbuf, source, call, route_path, handler_ref, module_qn, registry, main_gbuf, imp_keys, imp_vals, imp_count, route_mount, lang); diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index fb5b724d6..d6882067e 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -787,6 +787,21 @@ bool cbm_pipeline_http_client_call_url(const cbm_gbuf_t *gbuf, const char *proje * behind both passes' HTTP_CALLS / ASYNC_CALLS props (pass_parallel.c). */ void cbm_pipeline_close_call_props(char *props, int n, const CBMCall *call, bool with_line); +/* Go 1.22 ServeMux registrations may name a method and a host before the path + * ("POST /v1/x", "GET api.example.com/v1/x", #2537). For a Go file, the path + * inside such a route-registration argument; NULL for any other language and + * for an argument that is no such pattern (a plain "/v1/x" included). The one + * reader of that grammar behind both passes (pass_parallel.c). */ +const char *cbm_pipeline_mux_route_path(const char *arg, CBMLanguage lang); + +/* The method of a route registration's Route: the verb its callee names + * (`router.GET`), else the method its Go ServeMux pattern names + * (`mux.HandleFunc("POST /v1/x", h)`, #2537), else the callee's own answer + * (ANY or NULL). `buf` (CBM_SZ_16) holds a pattern method. Shared by both + * passes so their Routes stay byte-identical (pass_parallel.c). */ +const char *cbm_pipeline_route_reg_method(const CBMCall *call, CBMLanguage lang, char *buf, + size_t buf_sz); + /* Per-file index of the names a file's imports bind onto EXTERNAL modules: * local name -> the binding import. Built lazily, on the first call that * asks, from the file's extraction imports and its IMPORTS edges; one hash diff --git a/tests/test_extraction.c b/tests/test_extraction.c index e1c1e4491..e94aa73c4 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -6207,6 +6207,40 @@ TEST(extract_go_binary_concat_url_no_literal_suffix_issue1249) { PASS(); } +/* Issue #2537: a Go 1.22 ServeMux pattern may open with a method and/or a host + * ("POST /v1/payments", "GET api.example.com/v1/x"). The written pattern stays + * the first string argument, and its handler is captured exactly as for a + * plain "/v1/payments"; it was dropped because the pattern does not open with + * a slash. */ +TEST(extract_go_servemux_method_pattern_handler_issue2537) { + CBMFileResult *r = extract("package main\n" + "import \"net/http\"\n" + "func setup(mux *http.ServeMux) {\n" + " mux.HandleFunc(\"POST /v1/payments\", createPayment)\n" + " mux.Handle(\"GET api.example.com/v1/x\", hostHandler)\n" + "}\n", + CBM_LANG_GO, "t", "routes.go"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + + const CBMCall *method = find_call_by_callee(r, "mux.HandleFunc"); + ASSERT_NOT_NULL(method); + ASSERT_NOT_NULL(method->first_string_arg); + ASSERT_STR_EQ(method->first_string_arg, "POST /v1/payments"); + ASSERT_NOT_NULL(method->second_arg_name); + ASSERT_STR_EQ(method->second_arg_name, "createPayment"); + + const CBMCall *host = find_call_by_callee(r, "mux.Handle"); + ASSERT_NOT_NULL(host); + ASSERT_NOT_NULL(host->first_string_arg); + ASSERT_STR_EQ(host->first_string_arg, "GET api.example.com/v1/x"); + ASSERT_NOT_NULL(host->second_arg_name); + ASSERT_STR_EQ(host->second_arg_name, "hostHandler"); + + cbm_free_result(r); + PASS(); +} + /* Issue #2291: the JS/TS twin of #1249. A BFF that builds its backend URL as * `${process.env.BACKEND_URL}/wallet/topup` from a configurable base must * index the literal path after the base, exactly as `BACKEND_URL + "/x"` @@ -13442,6 +13476,7 @@ SUITE(extraction) { RUN_TEST(extract_ts_config_object_url_issue2235); RUN_TEST(extract_go_binary_concat_url_issue1249); RUN_TEST(extract_go_binary_concat_url_no_literal_suffix_issue1249); + RUN_TEST(extract_go_servemux_method_pattern_handler_issue2537); RUN_TEST(extract_ts_template_base_url_issue2291); RUN_TEST(extract_ts_template_base_url_no_literal_path_issue2291); RUN_TEST(extract_py_concat_const_url_issue706); diff --git a/tests/test_infrascan.c b/tests/test_infrascan.c index abfa8788c..2e33e5168 100644 --- a/tests/test_infrascan.c +++ b/tests/test_infrascan.c @@ -549,12 +549,46 @@ TEST(infrascan_data_flows_test_handlers_pair_only_within_their_file) { PASS(); } +/* Issue #2537: the Go 1.22 ServeMux grammar "[METHOD ][HOST]/[PATH]". The path + * is found inside the written pattern; a plain path, a lowercase word, a + * pattern without a path and a method too long for the buffer are no split. */ +TEST(infrascan_servemux_pattern_split_issue2537) { + char m[16]; + const char *pat = "POST /v1/payments"; + ASSERT_TRUE(cbm_service_pattern_mux_split(pat, m, sizeof(m)) == pat + 5); + ASSERT_STR_EQ(m, "POST"); + pat = "GET \t /v1/payments/{id}"; + ASSERT_TRUE(cbm_service_pattern_mux_split(pat, m, sizeof(m)) == pat + 6); + ASSERT_STR_EQ(m, "GET"); + pat = "GET api.example.com/v1/x"; + ASSERT_TRUE(cbm_service_pattern_mux_split(pat, m, sizeof(m)) == pat + 19); + ASSERT_STR_EQ(m, "GET"); + pat = "api.example.com:8443/v1/x"; + ASSERT_TRUE(cbm_service_pattern_mux_split(pat, m, sizeof(m)) == pat + 20); + ASSERT_STR_EQ(m, ""); + pat = "DELETE /"; + ASSERT_TRUE(cbm_service_pattern_mux_split(pat, NULL, 0) == pat + 7); + + ASSERT_NULL(cbm_service_pattern_mux_split("/v1/x", m, sizeof(m))); + ASSERT_NULL(cbm_service_pattern_mux_split("get /v1/x", m, sizeof(m))); + ASSERT_NULL(cbm_service_pattern_mux_split("POST", m, sizeof(m))); + ASSERT_NULL(cbm_service_pattern_mux_split("POST ", m, sizeof(m))); + ASSERT_NULL(cbm_service_pattern_mux_split("GET host", m, sizeof(m))); + ASSERT_NULL(cbm_service_pattern_mux_split("GET a b/x", m, sizeof(m))); + ASSERT_NULL(cbm_service_pattern_mux_split("", m, sizeof(m))); + ASSERT_NULL(cbm_service_pattern_mux_split(NULL, m, sizeof(m))); + ASSERT_NULL(cbm_service_pattern_mux_split("ABCDEFGHIJKLMNOP /x", m, sizeof(m))); + ASSERT_STR_EQ(m, ""); + PASS(); +} + SUITE(infrascan) { RUN_TEST(infrascan_http_route_literal_guard_rejects_filesystem_paths); RUN_TEST(infrascan_http_route_literal_guard_rejects_document_extensions); RUN_TEST(infrascan_client_navigation_is_js_family_only); RUN_TEST(infrascan_service_pattern_match_rejects_ids_inside_words); RUN_TEST(infrascan_service_pattern_match_keeps_real_library_qns); + RUN_TEST(infrascan_servemux_pattern_split_issue2537); RUN_TEST(infrascan_route_nodes_skip_bad_http_url_paths); RUN_TEST(infrascan_http_calls_join_matching_handler_route); RUN_TEST(infrascan_data_flows_link_every_caller_of_a_route); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index bd76775f3..e30d95a26 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -13376,6 +13376,130 @@ TEST(pipeline_route_prefix_bridge_is_order_free_and_root_bridges_nothing) { PASS(); } +/* Issue #2537: since Go 1.22 a ServeMux pattern may name a method and a host + * before the path ("POST /v1/payments", "GET api.example.com/v1/x"). Such a + * registration minted no Route and no HANDLES edge, because every registration + * gate wanted the first argument to open with a slash, while the same file + * written with "/v1/payments" had both. The method now comes from the pattern + * and a host is dropped from the path, in the sequential and the parallel pass + * alike. Only Go reads that grammar: a Python registration of "GET /py" is still + * no route. */ +TEST(pipeline_go_servemux_method_patterns_issue2537) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_servemux_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + write_temp_file(tmp, "go.mod", "module example.com/srv\n\ngo 1.22\n"); + write_temp_file(tmp, "main.go", + "package main\n" + "\n" + "import \"net/http\"\n" + "\n" + "func createPayment(w http.ResponseWriter, r *http.Request) {}\n" + "func getPayment(w http.ResponseWriter, r *http.Request) {}\n" + "func hostOnly(w http.ResponseWriter, r *http.Request) {}\n" + "func legacyList(w http.ResponseWriter, r *http.Request) {}\n" + "\n" + "func main() {\n" + "\tmux := http.NewServeMux()\n" + "\tmux.HandleFunc(\"POST /v1/payments\", createPayment)\n" + "\tmux.HandleFunc(\"GET /v1/payments/{id}\", getPayment)\n" + "\tmux.HandleFunc(\"api.example.com/v1/hosted\", hostOnly)\n" + "\tmux.HandleFunc(\"/v1/legacy\", legacyList)\n" + "\t_ = http.ListenAndServe(\":8080\", mux)\n" + "}\n"); + write_temp_file(tmp, "web/site.py", + "from flask import Flask\n" + "app = Flask(__name__)\n" + "\n" + "def py_view():\n" + " return \"ok\"\n" + "\n" + "app.add_url_rule(\"GET /py\", \"py_view\", py_view)\n"); + /* past MIN_FILES_FOR_PARALLEL, so the threaded run takes the parallel pass */ + for (int i = 0; i < 52; i++) { + char name[64]; + char body[128]; + snprintf(name, sizeof(name), "pad/padding%d.go", i); + snprintf(body, sizeof(body), "package pad\n\nfunc Padding%d() int { return %d }\n", i, i); + write_temp_file(tmp, name, body); + } + const char *old_single = getenv("CBM_INDEX_SINGLE_THREAD"); + char *saved_single = old_single ? strdup(old_single) : NULL; + const char *old_workers = getenv("CBM_WORKERS"); + char *saved_workers = old_workers ? strdup(old_workers) : NULL; + int run_rc[2] = {-1, -1}; + bool opened[2] = {false, false}; + int post[2] = {-1, -1}; + int get_id[2] = {-1, -1}; + int any_post[2] = {-1, -1}; + int host[2] = {-1, -1}; + int legacy[2] = {-1, -1}; + int py_any[2] = {-1, -1}; + int py_get[2] = {-1, -1}; + for (int mode = 0; mode < 2; mode++) { + if (mode == 0) { + cbm_setenv("CBM_INDEX_SINGLE_THREAD", "1", 1); + } else { + cbm_unsetenv("CBM_INDEX_SINGLE_THREAD"); + cbm_setenv("CBM_WORKERS", "4", 1); + } + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/servemux_%d.db", tmp, mode); + cbm_pipeline_t *p = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL); + run_rc[mode] = p ? cbm_pipeline_run(p) : -1; + const char *project = p ? cbm_pipeline_project_name(p) : NULL; + cbm_store_t *s = cbm_store_open_path(db_path); + opened[mode] = s != NULL; + if (s && project) { + post[mode] = + handles_to_route(s, project, "createPayment", "__route__POST__/v1/payments"); + get_id[mode] = + handles_to_route(s, project, "getPayment", "__route__GET__/v1/payments/{}"); + any_post[mode] = + handles_to_route(s, project, "createPayment", "__route__ANY__/v1/payments"); + host[mode] = handles_to_route(s, project, "hostOnly", "__route__ANY__/v1/hosted"); + legacy[mode] = handles_to_route(s, project, "legacyList", "__route__ANY__/v1/legacy"); + py_any[mode] = handles_to_route(s, project, "py_view", "__route__ANY__/py"); + py_get[mode] = handles_to_route(s, project, "py_view", "__route__GET__/py"); + } + if (s) { + cbm_store_close(s); + } + cbm_pipeline_free(p); + } + if (saved_single) { + cbm_setenv("CBM_INDEX_SINGLE_THREAD", saved_single, 1); + free(saved_single); + } else { + cbm_unsetenv("CBM_INDEX_SINGLE_THREAD"); + } + if (saved_workers) { + cbm_setenv("CBM_WORKERS", saved_workers, 1); + free(saved_workers); + } else { + cbm_unsetenv("CBM_WORKERS"); + } + th_rmtree(tmp); + for (int mode = 0; mode < 2; mode++) { + ASSERT_EQ(run_rc[mode], 0); + ASSERT_TRUE(opened[mode]); + /* the method is the pattern's, not ANY */ + ASSERT_EQ(post[mode], 1); + ASSERT_EQ(get_id[mode], 1); + ASSERT_EQ(any_post[mode], 0); + /* a host names no method and is not part of the path */ + ASSERT_EQ(host[mode], 1); + /* control: a plain path is unchanged */ + ASSERT_EQ(legacy[mode], 1); + /* another language's "GET /py" is not read as a ServeMux pattern */ + ASSERT_EQ(py_any[mode], 0); + ASSERT_EQ(py_get[mode], 0); + } + PASS(); +} + TEST(pipeline_swift_http_call_makes_route_issue1892) { char tmp[256]; snprintf(tmp, sizeof(tmp), "/tmp/cbm_swifthttp_XXXXXX"); @@ -27725,6 +27849,7 @@ SUITE(pipeline) { RUN_TEST(pipeline_swift_nested_url_makes_route_issue1892); RUN_TEST(pipeline_swift_http_call_makes_route_issue1892); RUN_TEST(pipeline_route_prefix_bridge_is_order_free_and_root_bridges_nothing); + RUN_TEST(pipeline_go_servemux_method_patterns_issue2537); RUN_TEST(pipeline_native_fetch_parallel_classified_as_http_calls); RUN_TEST(pipeline_local_fetch_shadow_not_classified_as_http); RUN_TEST(pipeline_typed_http_call_has_no_arg_url_twin); From 4d97d7819b80f96e2492569ae54b442af4406f98 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 26 Oct 2026 01:46:33 +0100 Subject: [PATCH 4/9] fix(routes): read a ServeMux pattern only for Handle and HandleFunc The Go 1.22 pattern split ran for every route-verb callee, so other Go calls whose name ends in Get/Post/Put/Delete/Patch became route registrations: st.Get("users/123") on a project method minted __route__GET__/123 and lost its CALLS edge to Store.Get, and c.Get("https://api.example.com/v1/x") split "https:" off as a host and minted __route__GET__//api.example.com/v1/x, in both passes. cbm_service_pattern_mux_route_path reads the pattern only for a callee ending ".Handle" or ".HandleFunc", and never for an argument containing "://". The extractor's handler capture and both passes (route_reg_path, find_route_path_in_args, cbm_pipeline_route_reg_method) go through it, so the two passes still mint identical Routes. Two negative cases extend pipeline_go_servemux_method_patterns_issue2537 in both passes, and the infrascan unit covers the gate. Refs #2537 Signed-off-by: Martin Vogel --- internal/cbm/extract_calls.c | 3 +- internal/cbm/service_patterns.c | 15 ++++++ internal/cbm/service_patterns.h | 8 ++++ src/pipeline/pass_calls.c | 7 +-- src/pipeline/pass_parallel.c | 13 +++--- src/pipeline/pipeline_internal.h | 11 +++-- tests/test_infrascan.c | 17 ++++++- tests/test_pipeline.c | 80 +++++++++++++++++++++++++++++++- 8 files changed, 137 insertions(+), 17 deletions(-) diff --git a/internal/cbm/extract_calls.c b/internal/cbm/extract_calls.c index f16b8ecf8..4beebc220 100644 --- a/internal/cbm/extract_calls.c +++ b/internal/cbm/extract_calls.c @@ -4559,7 +4559,8 @@ CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBML if (call.first_string_arg && (call.first_string_arg[0] == '/' || (ctx->language == CBM_LANG_GO && - cbm_service_pattern_mux_split(call.first_string_arg, NULL, 0) != NULL)) && + cbm_service_pattern_mux_route_path(call.callee_name, call.first_string_arg) != + NULL)) && !(laravel_uri && php_route_action_is_string(node))) { call.second_arg_name = extract_handler_arg(ctx, args); } diff --git a/internal/cbm/service_patterns.c b/internal/cbm/service_patterns.c index 1eaa50941..503d18538 100644 --- a/internal/cbm/service_patterns.c +++ b/internal/cbm/service_patterns.c @@ -1227,6 +1227,21 @@ const char *cbm_service_pattern_mux_split(const char *pattern, char *method, siz return slash; } +const char *cbm_service_pattern_mux_route_path(const char *callee_name, const char *pattern) { + static const char *const mux_callees[] = {".Handle", ".HandleFunc"}; + if (!callee_name || !pattern || strstr(pattern, "://") != NULL) { + return NULL; + } + size_t clen = strlen(callee_name); + for (size_t i = 0; i < sizeof(mux_callees) / sizeof(mux_callees[0]); i++) { + size_t slen = strlen(mux_callees[i]); + if (clen >= slen && strcmp(callee_name + clen - slen, mux_callees[i]) == 0) { + return cbm_service_pattern_mux_split(pattern, NULL, 0); + } + } + return NULL; +} + const char *cbm_service_pattern_broker(const char *resolved_qn) { if (!resolved_qn) { return NULL; diff --git a/internal/cbm/service_patterns.h b/internal/cbm/service_patterns.h index 59f3ea0af..0a182568e 100644 --- a/internal/cbm/service_patterns.h +++ b/internal/cbm/service_patterns.h @@ -95,6 +95,14 @@ const char *cbm_service_pattern_route_method(const char *callee_name); * on the file language. */ const char *cbm_service_pattern_mux_split(const char *pattern, char *method, size_t method_sz); +/* The path of a ServeMux registration's pattern (#2537): only a callee ending + * ".Handle" or ".HandleFunc" registers one, and never with a URL ("://"). + * Another route-verb call names no pattern: `store.Get("users/123")` is no + * host "users" + path "/123", `client.Get("https://...")` no host "https:". + * The split of cbm_service_pattern_mux_split for such a registration, else + * NULL. The grammar is Go's: callers gate on the file language. */ +const char *cbm_service_pattern_mux_route_path(const char *callee_name, const char *pattern); + /* Get the broker name for an async QN (e.g., "pubsub" from a Pub/Sub QN). * Returns NULL if not an async pattern. */ const char *cbm_service_pattern_broker(const char *resolved_qn); diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index 0e7069ab6..97c8e784e 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -573,8 +573,9 @@ cbm_svc_kind_t cbm_pipeline_external_import_service(cbm_external_import_index_t /* The path a route registration's first argument names: the argument itself * when it opens with a slash, or in Go the path of a ServeMux pattern that - * names a method or host first ("POST /v1/x", #2537). NULL when it names no - * path. MUST match pass_parallel.c find_route_path_in_args. */ + * names a method or host first ("POST /v1/x", #2537; Handle / HandleFunc + * only). NULL when it names no path. MUST match pass_parallel.c + * find_route_path_in_args. */ static const char *route_reg_path(const CBMCall *call, CBMLanguage lang) { const char *arg = call->first_string_arg; if (!arg) { @@ -583,7 +584,7 @@ static const char *route_reg_path(const CBMCall *call, CBMLanguage lang) { if (arg[0] == '/') { return arg; } - return cbm_pipeline_mux_route_path(arg, lang); + return cbm_pipeline_mux_route_path(call, lang); } /* Handle a route registration call: create Route node + HANDLES edge. diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index 380110295..feff58fdb 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -2263,11 +2263,12 @@ static bool is_route_path_shaped(const char *val) { return val && val[0] == '/' && !cbm_service_pattern_is_comment_text(val); } -const char *cbm_pipeline_mux_route_path(const char *arg, CBMLanguage lang) { +const char *cbm_pipeline_mux_route_path(const CBMCall *call, CBMLanguage lang) { + const char *arg = call->first_string_arg; if (lang != CBM_LANG_GO || !arg || cbm_service_pattern_is_comment_text(arg)) { return NULL; } - return cbm_service_pattern_mux_split(arg, NULL, 0); + return cbm_service_pattern_mux_route_path(call->callee_name, arg); } const char *cbm_pipeline_route_reg_method(const CBMCall *call, CBMLanguage lang, char *buf, @@ -2276,7 +2277,7 @@ const char *cbm_pipeline_route_reg_method(const CBMCall *call, CBMLanguage lang, if (method && strcmp(method, "ANY") != 0) { return method; } - if (cbm_pipeline_mux_route_path(call->first_string_arg, lang) && + if (cbm_pipeline_mux_route_path(call, lang) && cbm_service_pattern_mux_split(call->first_string_arg, buf, buf_sz) && buf[0] != '\0') { return buf; } @@ -2291,9 +2292,9 @@ static const char *find_route_path_in_args(const CBMCall *call, const char **out *out_handler = call->second_arg_name; return call->first_string_arg; } - /* 1b. Go ServeMux pattern with a method or host before the path (#2537). - * MUST match pass_calls.c route_reg_path. */ - const char *mux_path = cbm_pipeline_mux_route_path(call->first_string_arg, lang); + /* 1b. Go ServeMux pattern with a method or host before the path (#2537), + * read for Handle / HandleFunc only. MUST match pass_calls.c route_reg_path. */ + const char *mux_path = cbm_pipeline_mux_route_path(call, lang); if (mux_path) { *out_handler = call->second_arg_name; return mux_path; diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index d6882067e..d2fb1c736 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -788,11 +788,12 @@ bool cbm_pipeline_http_client_call_url(const cbm_gbuf_t *gbuf, const char *proje void cbm_pipeline_close_call_props(char *props, int n, const CBMCall *call, bool with_line); /* Go 1.22 ServeMux registrations may name a method and a host before the path - * ("POST /v1/x", "GET api.example.com/v1/x", #2537). For a Go file, the path - * inside such a route-registration argument; NULL for any other language and - * for an argument that is no such pattern (a plain "/v1/x" included). The one - * reader of that grammar behind both passes (pass_parallel.c). */ -const char *cbm_pipeline_mux_route_path(const char *arg, CBMLanguage lang); + * ("POST /v1/x", "GET api.example.com/v1/x", #2537). For a Go `Handle` / + * `HandleFunc` call, the path inside its first argument; NULL for any other + * language or callee (`store.Get("users/123")`), for a URL argument, and for an + * argument that is no such pattern (a plain "/v1/x" included). The one reader + * of that grammar behind both passes (pass_parallel.c). */ +const char *cbm_pipeline_mux_route_path(const CBMCall *call, CBMLanguage lang); /* The method of a route registration's Route: the verb its callee names * (`router.GET`), else the method its Go ServeMux pattern names diff --git a/tests/test_infrascan.c b/tests/test_infrascan.c index 2e33e5168..94ba522e1 100644 --- a/tests/test_infrascan.c +++ b/tests/test_infrascan.c @@ -551,7 +551,8 @@ TEST(infrascan_data_flows_test_handlers_pair_only_within_their_file) { /* Issue #2537: the Go 1.22 ServeMux grammar "[METHOD ][HOST]/[PATH]". The path * is found inside the written pattern; a plain path, a lowercase word, a - * pattern without a path and a method too long for the buffer are no split. */ + * pattern without a path and a method too long for the buffer are no split. + * Only a Handle / HandleFunc registration is read with it. */ TEST(infrascan_servemux_pattern_split_issue2537) { char m[16]; const char *pat = "POST /v1/payments"; @@ -579,6 +580,20 @@ TEST(infrascan_servemux_pattern_split_issue2537) { ASSERT_NULL(cbm_service_pattern_mux_split(NULL, m, sizeof(m))); ASSERT_NULL(cbm_service_pattern_mux_split("ABCDEFGHIJKLMNOP /x", m, sizeof(m))); ASSERT_STR_EQ(m, ""); + + /* only Handle / HandleFunc register a pattern, and never with a URL */ + pat = "POST /v1/payments"; + ASSERT_TRUE(cbm_service_pattern_mux_route_path("mux.HandleFunc", pat) == pat + 5); + pat = "api.example.com/v1/x"; + ASSERT_TRUE(cbm_service_pattern_mux_route_path("s.mux.Handle", pat) == pat + 15); + ASSERT_NULL(cbm_service_pattern_mux_route_path("st.Get", "users/123")); + ASSERT_NULL(cbm_service_pattern_mux_route_path("c.Get", "https://api.example.com/v1/x")); + ASSERT_NULL(cbm_service_pattern_mux_route_path("mux.Handle", "https://api.example.com/v1/x")); + ASSERT_NULL(cbm_service_pattern_mux_route_path("PatchCollector.Delete", "storage.k8s.io/v1")); + ASSERT_NULL(cbm_service_pattern_mux_route_path("r.Route", "GET /v1/x")); + ASSERT_NULL(cbm_service_pattern_mux_route_path("mux.HandleFunc", "/v1/x")); + ASSERT_NULL(cbm_service_pattern_mux_route_path(NULL, "GET /v1/x")); + ASSERT_NULL(cbm_service_pattern_mux_route_path("mux.HandleFunc", NULL)); PASS(); } diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index e30d95a26..e1e5e0f5d 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -13376,6 +13376,53 @@ TEST(pipeline_route_prefix_bridge_is_order_free_and_root_bridges_nothing) { PASS(); } +/* Route nodes whose qualified name contains `part`. */ +static int route_nodes_containing(cbm_store_t *s, const char *project, const char *part) { + cbm_node_t *routes = NULL; + int route_count = 0; + if (cbm_store_find_nodes_by_label(s, project, "Route", &routes, &route_count) != CBM_STORE_OK) { + return -1; + } + int matches = 0; + for (int i = 0; i < route_count; i++) { + if (routes[i].qualified_name && strstr(routes[i].qualified_name, part)) { + matches++; + } + } + if (routes) { + cbm_store_free_nodes(routes, route_count); + } + return matches; +} + +/* CALLS edges out of the function `source_name` into a Route node: the edge a + * route registration emits beside its Route (an HTTP client call's edge to a + * Route is HTTP_CALLS instead). */ +static int calls_into_route_count(cbm_store_t *s, const char *project, const char *source_name) { + cbm_edge_t *edges = NULL; + int edge_count = 0; + if (cbm_store_find_edges_by_type(s, project, "CALLS", &edges, &edge_count) != CBM_STORE_OK) { + return -1; + } + int matches = 0; + for (int i = 0; i < edge_count; i++) { + cbm_node_t source = {0}; + cbm_node_t target = {0}; + int source_ok = cbm_store_find_node_by_id(s, edges[i].source_id, &source) == CBM_STORE_OK; + int target_ok = cbm_store_find_node_by_id(s, edges[i].target_id, &target) == CBM_STORE_OK; + if (source_ok && target_ok && source.name && target.label && + strcmp(source.name, source_name) == 0 && strcmp(target.label, "Route") == 0) { + matches++; + } + cbm_node_free_fields(&source); + cbm_node_free_fields(&target); + } + if (edges) { + cbm_store_free_edges(edges, edge_count); + } + return matches; +} + /* Issue #2537: since Go 1.22 a ServeMux pattern may name a method and a host * before the path ("POST /v1/payments", "GET api.example.com/v1/x"). Such a * registration minted no Route and no HANDLES edge, because every registration @@ -13383,7 +13430,10 @@ TEST(pipeline_route_prefix_bridge_is_order_free_and_root_bridges_nothing) { * written with "/v1/payments" had both. The method now comes from the pattern * and a host is dropped from the path, in the sequential and the parallel pass * alike. Only Go reads that grammar: a Python registration of "GET /py" is still - * no route. */ + * no route. Only a ServeMux registration (Handle / HandleFunc) is read with it: + * another route-verb call names no pattern and mints no Route, neither + * `st.Get("users/123")` on a project method (which keeps its CALLS edge) nor + * `c.Get("https://api.example.com/v1/x")` on a client. */ TEST(pipeline_go_servemux_method_patterns_issue2537) { char tmp[256]; snprintf(tmp, sizeof(tmp), "/tmp/cbm_servemux_XXXXXX"); @@ -13401,6 +13451,16 @@ TEST(pipeline_go_servemux_method_patterns_issue2537) { "func hostOnly(w http.ResponseWriter, r *http.Request) {}\n" "func legacyList(w http.ResponseWriter, r *http.Request) {}\n" "\n" + "type Store struct{ m map[string]string }\n" + "\n" + "func (s *Store) Get(k string) string { return s.m[k] }\n" + "\n" + "func lookup(st *Store) string { return st.Get(\"users/123\") }\n" + "\n" + "func fetchRemote(c *http.Client) {\n" + "\t_, _ = c.Get(\"https://api.example.com/v1/x\")\n" + "}\n" + "\n" "func main() {\n" "\tmux := http.NewServeMux()\n" "\tmux.HandleFunc(\"POST /v1/payments\", createPayment)\n" @@ -13438,6 +13498,11 @@ TEST(pipeline_go_servemux_method_patterns_issue2537) { int legacy[2] = {-1, -1}; int py_any[2] = {-1, -1}; int py_get[2] = {-1, -1}; + int store_calls[2] = {-1, -1}; + int store_route[2] = {-1, -1}; + int lookup_reg[2] = {-1, -1}; + int remote_route[2] = {-1, -1}; + int remote_reg[2] = {-1, -1}; for (int mode = 0; mode < 2; mode++) { if (mode == 0) { cbm_setenv("CBM_INDEX_SINGLE_THREAD", "1", 1); @@ -13463,6 +13528,11 @@ TEST(pipeline_go_servemux_method_patterns_issue2537) { legacy[mode] = handles_to_route(s, project, "legacyList", "__route__ANY__/v1/legacy"); py_any[mode] = handles_to_route(s, project, "py_view", "__route__ANY__/py"); py_get[mode] = handles_to_route(s, project, "py_view", "__route__GET__/py"); + store_calls[mode] = named_edge_count(s, project, "CALLS", "lookup", "Get"); + store_route[mode] = route_nodes_containing(s, project, "__route__GET__/123"); + lookup_reg[mode] = calls_into_route_count(s, project, "lookup"); + remote_route[mode] = route_nodes_containing(s, project, "//api.example.com"); + remote_reg[mode] = calls_into_route_count(s, project, "fetchRemote"); } if (s) { cbm_store_close(s); @@ -13496,6 +13566,14 @@ TEST(pipeline_go_servemux_method_patterns_issue2537) { /* another language's "GET /py" is not read as a ServeMux pattern */ ASSERT_EQ(py_any[mode], 0); ASSERT_EQ(py_get[mode], 0); + /* a project method named Get keeps its CALLS edge; "users/123" is no + * host + path */ + ASSERT_EQ(store_calls[mode], 1); + ASSERT_EQ(store_route[mode], 0); + ASSERT_EQ(lookup_reg[mode], 0); + /* a client Get on a URL registers no route: "https:" is no host */ + ASSERT_EQ(remote_route[mode], 0); + ASSERT_EQ(remote_reg[mode], 0); } PASS(); } From 74e301d42e8a5ab32aa31d46e21637fd9d241a6f Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Wed, 28 Oct 2026 00:45:27 +0100 Subject: [PATCH 5/9] fix(daemon): end a permanent watch whose project was deleted Under `daemon start` a watch now outlives its last session. delete_project from another session or the CLI removes the project's DB but meets no owner of the watch: daemon MCP sessions carry no watcher pointer, so nothing unwatches it, and the next change in that root re-indexed the deleted project. When a permanent watch has no subscribers, no job for its project is running and its DB is gone, the watcher callback now removes the watch and skips, the rule a session refresh already applies to a deleted project. The running-job guard is needed because a full reindex unlinks the DB while it rebuilds it: a second change during that rebuild must join the job, not end the watch. Session-owned watches are unchanged. Tests: daemon_application_permanent_watch_ends_when_project_db_is_deleted (RED before: the deleted project was re-indexed) and daemon_application_permanent_watch_kept_while_its_reindex_runs (RED without the running-job guard: the second change ended the watch). Signed-off-by: Martin Vogel --- src/daemon/application.c | 21 +++++ tests/test_daemon_application.c | 147 ++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+) diff --git a/src/daemon/application.c b/src/daemon/application.c index a4e8f8fd1..804614e69 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -4268,6 +4268,21 @@ static void application_watch_encode_index_args_locked(const cbm_daemon_applicat *args = encoded ? yyjson_mut_write(document, 0, NULL) : NULL; } +/* Under `daemon start` a watch outlives its last session, so delete_project + * from another session or the CLI can remove the project's DB while nothing + * owns the watch any more: daemon MCP sessions carry no watcher pointer and + * never unwatch it. Re-indexing such an orphan would resurrect the deleted + * project. A full reindex also unlinks the DB while it rebuilds it, so a + * missing DB means "deleted" only while no job for the project is running. + * Caller holds application->mutex. */ +static bool application_permanent_watch_orphaned_locked(cbm_daemon_application_t *application, + const cbm_daemon_application_watch_t *watch, + const char *project_key) { + return application->permanent && watch->subscribers == 0 && + !application_find_active_job_locked(application, project_key) && + !application_regular_db_exists(watch->project); +} + static cbm_daemon_application_job_t *application_background_index_admit( cbm_daemon_application_t *application, const char *project_name, const char *canonical_root, const char *project_key, yyjson_mut_doc *document, yyjson_mut_val *root, char **args, @@ -4281,6 +4296,12 @@ static cbm_daemon_application_job_t *application_background_index_admit( } cbm_daemon_application_watch_t *watch = require_live_watch ? application_find_watch_locked(application, project_name) : NULL; + if (watch && application_permanent_watch_orphaned_locked(application, watch, project_key)) { + /* End the orphan the way a session refresh ends a deleted project's + * watch; the callback below becomes a harmless skip. */ + application_remove_watch_locked(application, watch); + watch = NULL; + } bool watch_live = !require_live_watch || (watch && application_watch_root_matches(watch->root, canonical_root) && (watch->subscribers > 0 || application->permanent)); diff --git a/tests/test_daemon_application.c b/tests/test_daemon_application.c index 67de9002f..b4c0d2316 100644 --- a/tests/test_daemon_application.c +++ b/tests/test_daemon_application.c @@ -4687,6 +4687,151 @@ TEST(daemon_application_permanent_watch_survives_disconnect_and_indexes_without_ PASS(); } +/* A permanent daemon keeps a project watched after its last session closes. + * delete_project from another session or the CLI removes the project's DB but + * owns no watch, so the watch itself must notice: a later change must end the + * orphaned watch, never re-create the deleted index. */ +TEST(daemon_application_permanent_watch_ends_when_project_db_is_deleted) { + app_watch_race_fixture_t fixture; + bool fixture_ready = app_watch_race_fixture_init(&fixture, 48); + if (fixture_ready) { + cbm_daemon_application_set_permanent(fixture.application, true); + /* Should a job be admitted, it completes at once: the verdict is + * read from the start counter, never from a timeout. */ + atomic_store(&fixture.fake.allow_completion, true); + } + if (fixture.session) { + fixture.callbacks.session_cancel(fixture.callbacks.context, fixture.session); + fixture.callbacks.session_close(fixture.callbacks.context, fixture.session); + fixture.session = NULL; + } + int watch_after_close = fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + bool db_removed = fixture_ready && cbm_unlink(fixture.db_path) == 0; + int result = db_removed ? cbm_daemon_application_watcher_index(fixture.project, fixture.root, + fixture.application) + : -2; + int starts = fixture_ready ? atomic_load(&fixture.fake.starts) : -1; + int watch_after_index = fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + bool cleaned = app_watch_race_fixture_finish(&fixture); + + ASSERT_TRUE(fixture_ready); + ASSERT_EQ(watch_after_close, 1); + ASSERT_TRUE(db_removed); + /* A harmless skip: nothing admitted, the deleted index stays deleted. */ + ASSERT_EQ(result, 1); + ASSERT_EQ(starts, 0); + /* The orphaned physical watch is torn down. */ + ASSERT_EQ(watch_after_index, 0); + ASSERT_TRUE(cleaned); + PASS(); +} + +/* The second watcher callback either joins the running job (its retained + * ownership is the job's second subscriber) or returns. Both states latch, so + * whichever happens decides the wait; the deadline only catches "neither". */ +static bool app_wait_for_watcher_join_or_return(cbm_daemon_application_t *application, + const char *project, atomic_bool *returned) { + uint64_t deadline = cbm_now_ms() + APP_TEST_TIMEOUT_MS; + while (cbm_now_ms() < deadline) { + if (cbm_daemon_application_job_subscribers(application, project) == 2 || + atomic_load(returned)) { + return true; + } + cbm_usleep(1000); + } + return false; +} + +/* A full reindex unlinks the project's DB while it rebuilds it, so a missing + * DB alone does not mean the project was deleted. While the permanent watch's + * own job runs, a second change in its root must join that job and keep the + * watch; once the job ends, both callbacks hand their ownership back. */ +TEST(daemon_application_permanent_watch_kept_while_its_reindex_runs) { + app_watch_race_fixture_t fixture; + bool fixture_ready = app_watch_race_fixture_init(&fixture, 49); + if (fixture_ready) { + cbm_daemon_application_set_permanent(fixture.application, true); + } + if (fixture.session) { + fixture.callbacks.session_cancel(fixture.callbacks.context, fixture.session); + fixture.callbacks.session_close(fixture.callbacks.context, fixture.session); + fixture.session = NULL; + } + int watch_after_close = fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + + app_watcher_index_thread_t first = { + .application = fixture.application, + .project = fixture.project, + .root = fixture.root, + .pause_before_subscribe = false, + .result = -1, + }; + atomic_init(&first.ready, false); + atomic_init(&first.proceed, true); + atomic_init(&first.done, false); + app_watcher_index_thread_t second = { + .application = fixture.application, + .project = fixture.project, + .root = fixture.root, + .pause_before_subscribe = false, + .result = -1, + }; + atomic_init(&second.ready, false); + atomic_init(&second.proceed, true); + atomic_init(&second.done, false); + + /* The fake worker keeps the first job running until allow_completion. */ + cbm_thread_t first_thread; + bool first_started = fixture_ready && watch_after_close == 1 && + cbm_thread_create(&first_thread, 0, app_watcher_index_thread, &first) == 0; + bool job_running = first_started && app_wait_for_atomic_int(&fixture.fake.starts, 1); + bool db_removed = job_running && cbm_unlink(fixture.db_path) == 0; + cbm_thread_t second_thread; + bool second_started = + db_removed && cbm_thread_create(&second_thread, 0, app_watcher_index_thread, &second) == 0; + bool settled = second_started && app_wait_for_watcher_join_or_return( + fixture.application, fixture.project, &second.done); + size_t subscribers_while_running = + fixture_ready ? cbm_daemon_application_job_subscribers(fixture.application, fixture.project) + : 0; + bool second_returned_while_running = atomic_load(&second.done); + int watch_while_running = fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + + if (fixture_ready) { + atomic_store(&fixture.fake.allow_completion, true); + } + bool first_joined = first_started && app_wait_for_atomic_bool(&first.done, true) && + cbm_thread_join(&first_thread) == 0; + bool second_joined = second_started && app_wait_for_atomic_bool(&second.done, true) && + cbm_thread_join(&second_thread) == 0; + int starts = fixture_ready ? atomic_load(&fixture.fake.starts) : -1; + size_t subscribers_after = + fixture_ready ? cbm_daemon_application_job_subscribers(fixture.application, fixture.project) + : SIZE_MAX; + int watch_after = fixture_ready ? cbm_watcher_watch_count(fixture.watcher) : -1; + bool cleaned = app_watch_race_fixture_finish(&fixture); + + ASSERT_TRUE(fixture_ready); + ASSERT_EQ(watch_after_close, 1); + ASSERT_TRUE(job_running); + ASSERT_TRUE(db_removed); + ASSERT_TRUE(settled); + /* The second change joined the running job instead of ending the watch. */ + ASSERT_EQ(subscribers_while_running, 2); + ASSERT_FALSE(second_returned_while_running); + ASSERT_EQ(watch_while_running, 1); + ASSERT_TRUE(first_joined); + ASSERT_TRUE(second_joined); + ASSERT_EQ(first.result, 0); + ASSERT_EQ(second.result, 0); + ASSERT_EQ(starts, 1); + /* Each callback released the ownership it retained at admission. */ + ASSERT_EQ(subscribers_after, 0); + ASSERT_EQ(watch_after, 1); + ASSERT_TRUE(cleaned); + PASS(); +} + TEST(daemon_application_late_watcher_session_owns_active_watcher_job) { app_watch_race_fixture_t fixture; bool fixture_ready = app_watch_race_fixture_init(&fixture, 45); @@ -7793,6 +7938,8 @@ SUITE(daemon_application) { RUN_TEST(daemon_application_final_cancel_drains_admitted_watcher_job); RUN_TEST(daemon_application_watcher_job_follows_exact_live_watch_owners); RUN_TEST(daemon_application_permanent_watch_survives_disconnect_and_indexes_without_session); + RUN_TEST(daemon_application_permanent_watch_ends_when_project_db_is_deleted); + RUN_TEST(daemon_application_permanent_watch_kept_while_its_reindex_runs); RUN_TEST(daemon_application_late_watcher_session_owns_active_watcher_job); RUN_TEST(daemon_application_watch_roots_match_windows_separators_only); RUN_TEST(daemon_application_async_completion_disables_all_watches); From 631418683e005850a3bce49a81f2cf655ce53e1c Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Wed, 28 Oct 2026 00:50:51 +0100 Subject: [PATCH 6/9] fix(daemon): keep the workspace-policy release under daemon start Under `daemon start` the session watch release kept every watch whose last subscriber left, so the daemon could keep watching after the session ended. The same release also runs when a session's root stops being allowed by workspace policy (an approval revoked, a boundary declared). That is not a disconnect: the session is still open and its root is refused, so keeping the watch there let a permanent daemon go on indexing a root the user had just withdrawn. application_release_session_watch_locked() now takes session_ending. The cancel and close paths pass true and keep the daemon-owned watch exactly as before; the workspace-policy branch of the watch refresh passes false and removes the watch in both modes, as it did before permanent watches. Test: daemon_application_permanent_workspace_release_still_ends_watch revokes the sensitive-root approval of a permanent session's root and sends one request; its synchronous refresh must end the watch (RED when the policy branch passes true). Signed-off-by: Martin Vogel --- src/daemon/application.c | 20 ++++-- tests/test_daemon_application.c | 117 ++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/src/daemon/application.c b/src/daemon/application.c index 804614e69..2383b0359 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -564,7 +564,8 @@ static bool application_watches_disabled_locked(cbm_daemon_application_t *applic return true; } -static void application_release_session_watch_locked(cbm_daemon_application_session_t *session) { +static void application_release_session_watch_locked(cbm_daemon_application_session_t *session, + bool session_ending) { application_watch_job_unsubscribe_session_locked(session, NULL); /* @@ -575,9 +576,14 @@ static void application_release_session_watch_locked(cbm_daemon_application_sess * * Non-permanent applications retain the historical session-scoped * lifecycle and remove the physical watch after the final subscriber. - * Project deletion/pruning and daemon shutdown remove persistent watches - * through their dedicated lifecycle paths. + * A release decided by workspace policy (session_ending false: the + * session's root is no longer allowed) is not a disconnect and removes + * the watch in both modes. Daemon shutdown and auto_watch=false remove + * persistent watches through their own paths; a deleted project's watch + * ends at its next watcher callback + * (application_permanent_watch_orphaned_locked). */ + bool daemon_keeps_watch = session_ending && session->application->permanent; while (session->watches) { application_watch_membership_t *membership = session->watches; cbm_daemon_application_watch_t *watch = membership->watch; @@ -585,7 +591,7 @@ static void application_release_session_watch_locked(cbm_daemon_application_sess cbm_free(CBM_MEM_CLASS_OTHER, membership); if (watch->subscribers > 0) { watch->subscribers--; - if (!session->application->permanent && watch->subscribers == 0) { + if (!daemon_keeps_watch && watch->subscribers == 0) { application_remove_watch_locked(session->application, watch); } } @@ -733,7 +739,7 @@ static void application_refresh_watch_locked(cbm_daemon_application_session_t *s return; } if (!application_session_workspace_allowed(session, "watch")) { - application_release_session_watch_locked(session); + application_release_session_watch_locked(session, false); return; } application_register_watch_locked(session, cbm_mcp_server_session_project(session->mcp), @@ -3661,7 +3667,7 @@ static void application_session_cancel(void *context, reap_update || (session->background_eligible && application->update_thread_started && application->update_owners == 0); application_index_watch_unsubscribe_locked(session, NULL); - application_release_session_watch_locked(session); + application_release_session_watch_locked(session, true); bool final_live_session = newly_cancelled; for (cbm_daemon_application_session_t *other = application->sessions; final_live_session && other; other = other->next) { @@ -3721,7 +3727,7 @@ static void application_session_close(void *context, reap_update || (session->background_eligible && application->update_thread_started && application->update_owners == 0); application_index_watch_unsubscribe_locked(session, NULL); - application_release_session_watch_locked(session); + application_release_session_watch_locked(session, true); cbm_mutex_unlock(&application->mutex); application_auto_index_cancel_join(application, join_auto_index); application_jobs_reap_completed(application); diff --git a/tests/test_daemon_application.c b/tests/test_daemon_application.c index b4c0d2316..6b8e91d9f 100644 --- a/tests/test_daemon_application.c +++ b/tests/test_daemon_application.c @@ -2820,6 +2820,122 @@ TEST(daemon_application_sensitive_root_blocks_watch_but_preserves_controls) { PASS(); } +/* Under `daemon start` a session that ENDS leaves its watch to the daemon, but + * a workspace-policy release is not a disconnect: once the session's root is + * no longer allowed (here its approval is revoked), the next request's + * refresh must still end the watch in a permanent generation too. */ +TEST(daemon_application_permanent_workspace_release_still_ends_watch) { + app_env_backup_t cache_environment; + app_env_backup_t home_environment; + bool environments_saved = app_env_backup_capture(&cache_environment, "CBM_CACHE_DIR") && + app_env_backup_capture(&home_environment, "HOME"); + char sensitive_raw[APP_TEST_PATH_CAP]; + char cache_raw[APP_TEST_PATH_CAP]; + (void)snprintf(sensitive_raw, sizeof(sensitive_raw), + "%s/cbm-app-permanent-release-home-XXXXXX", cbm_tmpdir()); + (void)snprintf(cache_raw, sizeof(cache_raw), "%s/cbm-app-permanent-release-cache-XXXXXX", + cbm_tmpdir()); + bool dirs_ok = cbm_mkdtemp(sensitive_raw) != NULL && cbm_mkdtemp(cache_raw) != NULL; + char sensitive[APP_TEST_PATH_CAP] = {0}; + char cache[APP_TEST_PATH_CAP] = {0}; + bool canonical = dirs_ok && cbm_canonical_path(sensitive_raw, sensitive, sizeof(sensitive)) && + cbm_canonical_path(cache_raw, cache, sizeof(cache)); + bool environments_set = environments_saved && canonical && + cbm_setenv("CBM_CACHE_DIR", cache, 1) == 0 && + cbm_setenv("HOME", sensitive, 1) == 0; + cbm_config_t *stored_config = environments_set ? cbm_config_open(cache) : NULL; + bool config_ready = stored_config && + cbm_config_set(stored_config, CBM_CONFIG_AUTO_INDEX, "false") == 0 && + cbm_config_set(stored_config, CBM_CONFIG_AUTO_WATCH, "true") == 0; + char *project = canonical ? cbm_project_name_from_path(sensitive) : NULL; + char db_path[APP_TEST_PATH_CAP] = {0}; + if (project) { + (void)snprintf(db_path, sizeof(db_path), "%s/%s.db", cache, project); + } + bool db_ready = config_ready && project && app_test_create_empty_file(db_path); + /* The root is $HOME, so only an explicit sensitive approval allows it. */ + char grant_error[APP_TEST_PATH_CAP]; + bool approved = db_ready && cbm_workspace_grant_add(cache, sensitive, sensitive, true, + grant_error, sizeof(grant_error)); + char grant_path[APP_TEST_PATH_CAP] = {0}; + bool grant_path_ok = + approved && cbm_workspace_grant_path(cache, grant_path, sizeof(grant_path)); + + cbm_store_t *store = cbm_store_open_memory(); + cbm_watcher_t *watcher = cbm_watcher_new(store, app_test_index_noop, NULL); + cbm_daemon_application_config_t config = { + .watcher = watcher, + .config = stored_config, + }; + cbm_daemon_application_t *application = + grant_path_ok && watcher ? cbm_daemon_application_new(&config) : NULL; + if (application) { + cbm_daemon_application_set_permanent(application, true); + } + cbm_daemon_runtime_application_callbacks_t callbacks = + cbm_daemon_application_runtime_callbacks(application); + cbm_daemon_runtime_application_session_t *session = + application ? app_test_open(&callbacks, 4036) : NULL; + + bool initialized = app_test_initialize_profile(&callbacks, session, sensitive, + CBM_MCP_TOOL_PROFILE_ALL, NULL, NULL); + int watch_while_approved = watcher ? cbm_watcher_watch_count(watcher) : -1; + + /* Revoke the approval. Every request ends with a synchronous watch + * refresh, so one ping is the point where the policy is re-read. */ + bool revoked = initialized && cbm_unlink(grant_path) == 0; + uint8_t *ping = NULL; + uint32_t ping_length = 0; + uint8_t *response = NULL; + uint32_t response_length = 0; + bool pinged = revoked && + app_test_text_request(CBM_DAEMON_APPLICATION_REQUEST_MCP, + "{\"jsonrpc\":\"2.0\",\"id\":4136,\"method\":\"ping\"}", + &ping, &ping_length) && + app_test_request(&callbacks, session, ping, ping_length, &response, + &response_length) == CBM_DAEMON_RUNTIME_APPLICATION_OK; + free(response); + free(ping); + int watch_after_revoke = watcher ? cbm_watcher_watch_count(watcher) : -1; + + if (session) { + callbacks.session_cancel(callbacks.context, session); + callbacks.session_close(callbacks.context, session); + } + bool stopped = application && cbm_daemon_application_shutdown(application, APP_TEST_TIMEOUT_MS); + cbm_daemon_application_free(application); + if (watcher) { + cbm_watcher_stop(watcher); + cbm_watcher_free(watcher); + } + cbm_store_close(store); + cbm_config_close(stored_config); + free(project); + (void)th_rmtree(sensitive); + (void)th_rmtree(cache); + bool cache_restored = app_env_backup_restore(&cache_environment); + bool home_restored = app_env_backup_restore(&home_environment); + + ASSERT_TRUE(environments_saved); + ASSERT_TRUE(dirs_ok); + ASSERT_TRUE(canonical); + ASSERT_TRUE(environments_set); + ASSERT_TRUE(config_ready); + ASSERT_TRUE(db_ready); + ASSERT_TRUE(approved); + ASSERT_TRUE(grant_path_ok); + ASSERT_TRUE(initialized); + ASSERT_EQ(watch_while_approved, 1); + ASSERT_TRUE(revoked); + ASSERT_TRUE(pinged); + /* A policy release, not a disconnect: the permanent watch ends as well. */ + ASSERT_EQ(watch_after_revoke, 0); + ASSERT_TRUE(stopped); + ASSERT_TRUE(cache_restored); + ASSERT_TRUE(home_restored); + PASS(); +} + TEST(daemon_application_programmatic_index_injects_resource_policy) { char *root = th_mktempdir("cbm_app_policy_root"); char *cache = th_mktempdir("cbm_app_policy_cache"); @@ -7904,6 +8020,7 @@ SUITE(daemon_application) { RUN_TEST(daemon_application_initialize_coalesces_auto_index_for_full_sessions); RUN_TEST(daemon_application_sensitive_root_blocks_auto_index_but_preserves_controls); RUN_TEST(daemon_application_sensitive_root_blocks_watch_but_preserves_controls); + RUN_TEST(daemon_application_permanent_workspace_release_still_ends_watch); RUN_TEST(daemon_application_ui_index_accepts_non_ascii_directory); RUN_TEST(daemon_application_index_args_compare_repo_path_separator_equivalently); RUN_TEST(daemon_application_async_index_returns_before_completion_and_status_polls); From 70d6d5017c741b57cd035b1ecc7927affb7908a2 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Wed, 28 Oct 2026 00:51:48 +0100 Subject: [PATCH 7/9] docs(watcher): describe the watch lifetime under daemon start A daemon started with `daemon start` now keeps a project watched after its last owning MCP session closes, until the daemon stops, the project is deleted, its directory disappears, or auto_watch is set to false. README (the watcher paragraph and the index_repository row), the watcher_enabled row of docs/CONFIGURATION.md and the comment above the index_status watch provider still said every watch ends with its last session. Signed-off-by: Martin Vogel --- README.md | 4 ++-- docs/CONFIGURATION.md | 2 +- src/daemon/application.c | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8fd5c68a9..4162ae371 100644 --- a/README.md +++ b/README.md @@ -255,7 +255,7 @@ codebase-memory-mcp config set auto_index true `auto_index` is off by default. When enabled, new projects are indexed automatically on first connection. The configurable file limit is `config set auto_index_limit 50000`. Previously indexed projects can be watched through `auto_watch` (on by default), subject to its global setting and session ownership. -Each initialized, long-lived MCP session can register its root project when it connects and any target from a successful `index_repository` call, including custom project names and roots in another worktree. While `auto_watch` is enabled, these watches belong to the registering sessions and remain while at least one owning session is connected; closing the last owner removes the watch. An async index registers its target only after success and while its requesting session remains connected. CLI indexing and failed MCP indexes do not register watches. Global `auto_watch` controls registration and removes all watches on the next refresh or registration request when set to `false`. `index_status` reports this in its `watch` object: `watched`, and either the poll cadence and last scan time or the `reason` it is not watched. +Each initialized, long-lived MCP session can register its root project when it connects and any target from a successful `index_repository` call, including custom project names and roots in another worktree. While `auto_watch` is enabled, these watches belong to the registering sessions and remain while at least one owning session is connected; closing the last owner removes the watch. Under a daemon started with `daemon start` a watch outlives its last owning session: the project stays watched and is re-indexed in the background when it changes, until that daemon stops, the project is deleted, its directory disappears, or `auto_watch` is set to `false`. An async index registers its target only after success and while its requesting session remains connected. CLI indexing and failed MCP indexes do not register watches. Global `auto_watch` controls registration and removes all watches on the next refresh or registration request when set to `false`. `index_status` reports this in its `watch` object: `watched`, and either the poll cadence and last scan time or the `reason` it is not watched. Watcher registration is controlled globally by `auto_watch` (default `true`). On the next refresh or registration request, `true` allows MCP sessions to register their root on connect and their successful `index_repository` targets; `false` removes all watches. CLI indexing and failed MCP indexes do not add watches. @@ -877,7 +877,7 @@ Tools requiring arguments accept JSON piped on stdin. Tools with only optional a | Tool | Description | |------|-------------| -| `index_repository` | Index a repository into the graph. A successful call from an active MCP session registers the target for auto-sync while that session remains connected, subject to the global `auto_watch` setting; CLI calls do not register watches. Waits for the whole index by default; pass `async: true` to start it in the daemon and return at once, then poll with `status: true` (see below). | +| `index_repository` | Index a repository into the graph. A successful call from an active MCP session registers the target for auto-sync while that session remains connected (under `daemon start`, also after it disconnects), subject to the global `auto_watch` setting; CLI calls do not register watches. Waits for the whole index by default; pass `async: true` to start it in the daemon and return at once, then poll with `status: true` (see below). | | `list_projects` | List indexed projects with paging; use `detail: "stats"` for node/edge counts and database sizes. | | `delete_project` | Remove a project and all its graph data. | | `index_status` | Check indexing status of a project. | diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 0981f6fc2..96a823752 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -89,7 +89,7 @@ Current keys: | `auto_index_limit` | `50000` | Maximum file count allowed for automatic indexing of a new project. | | `auto_watch` | `true` | Global switch for MCP session watch registration. On the next refresh or registration request, `true` allows session roots and successful `index_repository` targets to be watched; `false` removes all watches. The background watcher can still run with no registered projects. | | `doc_candidates` | `auto` | Score documentation sections against code to store doc↔code candidates (the possible targets of a section, and the sections that may describe a function or file, each with a judged probability). Unset (`auto`), the index mode decides: on for `full` indexes (the default mode), off for `moderate` and `fast`. Set `true` or `false` to choose for every mode; `config reset doc_candidates` returns to `auto`. The scoring grows with sections × functions: under a second up to ~100k functions, ~15 s at ~400k, ~3 minutes on the Linux kernel. Off stores no candidates and leaves the rest of the graph unchanged; on in `fast` mode stores candidates without adding `fast`'s skipped `SIMILAR_TO` / `SEMANTICALLY_RELATED` edges. `index_repository` also takes `doc_candidates` for a single index, which wins over this setting. Applies from the next index. | -| `watcher_enabled` | `true` | Master switch for the background watcher subsystem. Set `false` to stop the watcher from starting at all — no poll thread and no project registration. Reindex manually with `index_repository` when disabled. Active MCP sessions can register their root project and successful explicit index targets; `index_status` reports the current state in its `watch` object. | +| `watcher_enabled` | `true` | Master switch for the background watcher subsystem. Set `false` to stop the watcher from starting at all — no poll thread and no project registration. Reindex manually with `index_repository` when disabled. Active MCP sessions can register their root project and successful explicit index targets; under `daemon start` those watches outlive the session until the daemon stops, the project is deleted, its directory disappears, or `auto_watch` is set to `false`. `index_status` reports the current state in its `watch` object. | | `watch_non_git` | `false` | Also poll project roots that are **not git repositories**. By default the watcher only follows git projects, so a project indexed from a plain directory is never refreshed after its first index — reindex it manually with `index_repository`. Set `true` to poll such roots on the same adaptive cadence with a file-tree scan: the indexer's own discovery walk (same skip lists, `.gitignore` and `.cbmignore` rules) hashed over each file's path, size and mtime. Any change reindexes once; paths the indexer skips (including cbm's own `.codebase-memory/` output) never trigger. The first poll after the daemon starts reindexes each such project once, since nothing records which tree state the index holds. The scan walks the whole tree every poll, so it costs more than git polling on very large trees. Read once when the daemon starts, like `watcher_enabled`. | | `index_max_files` | `off` | Optional maximum number of accepted source files in one discovery run. | | `index_max_source_mb` | `off` | Optional maximum accepted source size in MiB in one discovery run. | diff --git a/src/daemon/application.c b/src/daemon/application.c index 2383b0359..eb9bf97f4 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -824,7 +824,9 @@ static const char *application_watch_strategy_name(cbm_watcher_strategy_t strate /* index_status watch visibility (#2167). The physical watcher is the truth for * "watched"; session roots and successful explicit targets stay watched while - * at least one owning MCP session remains open. Takes the + * at least one owning MCP session remains open (under `daemon start`, also + * after the last one closes, until the daemon stops, the project is deleted, + * its root disappears, or auto_watch is switched off). Takes the * application mutex and the watcher lock one after the other, never nested. */ static void application_watch_status(void *context, const char *project, cbm_mcp_watch_status_t *out) { From 55844c9da9b969eb7e80b60b50c0402f2952ba0c Mon Sep 17 00:00:00 2001 From: Divakant Pandey Date: Wed, 28 Oct 2026 17:26:47 +0530 Subject: [PATCH 8/9] fix(cli): use managed binary path for agent configs when externally managed (#2722) Signed-off-by: Divakant Pandey --- src/cli/cli.c | 74 +++++++++++++++++++++++++++++++++++------------- src/cli/cli.h | 2 ++ tests/test_cli.c | 60 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 19 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index ba4ae520a..79a08855c 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -10979,7 +10979,20 @@ void cbm_cli_set_client_selection_for_testing(const char *spec) { /* Detect the running binary's path at runtime. The return value distinguishes * OS-reported identity (safe ownership evidence) from the ~/.local/bin fallback * used only as an install copy source. */ +#ifdef CBM_CLI_ENABLE_TEST_API +static const char *g_cli_self_path_override_for_test = NULL; +void cbm_cli_set_self_path_override_for_test(const char *self_path) { + g_cli_self_path_override_for_test = self_path; +} +#endif + static bool cbm_detect_self_path(char *buf, size_t buf_sz, const char *home) { +#ifdef CBM_CLI_ENABLE_TEST_API + if (g_cli_self_path_override_for_test) { + snprintf(buf, buf_sz, "%s", g_cli_self_path_override_for_test); + return true; + } +#endif buf[0] = '\0'; bool exact = false; #ifdef _WIN32 @@ -11241,6 +11254,7 @@ char *cbm_build_install_plan_json(const char *home, const char *binary_path) { typedef struct { const char *bin_target; + const char *config_binary; const char *bin_dir; const char *home; const char *shell_rc; @@ -11265,6 +11279,8 @@ static int cli_install_activate(void *opaque) { !activation->shell_rc) { return CLI_TRUE; } + const char *config_binary = + activation->config_binary ? activation->config_binary : activation->bin_target; char previous_managed_binary[CLI_BUF_1K] = {0}; bool previous_managed_binary_exact = cbm_detect_self_path( previous_managed_binary, sizeof(previous_managed_binary), activation->home); @@ -11313,9 +11329,13 @@ static int cli_install_activate(void *opaque) { * including same-binary and non-force installs. */ int agent_config_rc = CLI_OK; if (!activation->skip_config) { + const char *prev = previous_managed_binary_exact ? previous_managed_binary : NULL; + if (config_binary != activation->bin_target && + (!prev || strcmp(prev, config_binary) == 0)) { + prev = activation->bin_target; + } agent_config_rc = cbm_install_agent_configs_with_previous( - activation->home, activation->bin_target, - previous_managed_binary_exact ? previous_managed_binary : NULL, activation->force, + activation->home, config_binary, prev, activation->force, activation->dry_run); } if (agent_config_rc != CLI_OK) { @@ -11463,11 +11483,40 @@ int cbm_cmd_install(int argc, char **argv) { return CLI_TRUE; } + char self_path[CLI_BUF_1K] = {0}; + bool self_path_exact = cbm_detect_self_path(self_path, sizeof(self_path), home); + + /* #1566: a binary owned by mise/Homebrew/nix is not ours to relocate. Infer + * it, and let either flag override the inference in either direction. */ + bool externally_managed = + !force_binary && cli_binary_is_externally_managed(self_path, self_path_exact, bin_dir); + bool explicit_skip_binary = skip_binary; + if (externally_managed) { + skip_binary = true; + } + + /* NOT stat(): on Windows it goes through the ANSI code page, so an + * extended-length or non-ASCII target reports "absent" and a non-force + * install silently overwrites bytes the user asked to keep. */ + cbm_path_info_t target_status; + bool target_exists = cbm_path_info_utf8(bin_target, &target_status) == 0; + bool same_binary = cbm_same_file(self_path, bin_target); + + /* #2722: when a binary is managed externally (mise, brew, nix, etc.), + * do not write the non-existent default target into agent configurations. + * Use the running managed binary path instead. */ + const char *config_binary = bin_target; + if (externally_managed && self_path_exact && self_path[0]) { + config_binary = self_path; + } else if (skip_binary && !target_exists && self_path_exact && self_path[0]) { + config_binary = self_path; + } + /* --plan: emit the machine-readable install receipt and exit WITHOUT * mutating anything (no config writes, no index deletion, no network) so * an agent can inspect exactly what install would touch first (#388). */ if (plan) { - char *json = cbm_build_install_plan_json_options(home, bin_target, skip_config); + char *json = cbm_build_install_plan_json_options(home, config_binary, skip_config); if (!json) { (void)fprintf(stderr, "error: failed to build install plan\n"); return CLI_TRUE; @@ -11489,16 +11538,9 @@ int cbm_cmd_install(int argc, char **argv) { printf("codebase-memory-mcp install %s\n\n", CBM_VERSION); - char self_path[CLI_BUF_1K] = {0}; - bool self_path_exact = cbm_detect_self_path(self_path, sizeof(self_path), home); - - /* #1566: a binary owned by mise/Homebrew/nix is not ours to relocate. Infer - * it, and let either flag override the inference in either direction. */ - bool externally_managed = - !force_binary && cli_binary_is_externally_managed(self_path, self_path_exact, bin_dir); - if (externally_managed || skip_binary) { + if (externally_managed || explicit_skip_binary) { const char *manager = cli_external_manager_name(self_path); - if (skip_binary) { + if (explicit_skip_binary) { printf("Skipping binary placement (--skip-binary); configuring agents only.\n\n"); } else { printf("Binary is managed elsewhere%s%s:\n %s\n" @@ -11506,15 +11548,8 @@ int cbm_cmd_install(int argc, char **argv) { " (use --force-binary to install a copy into %s anyway)\n\n", manager ? " by " : "", manager ? manager : "", self_path, bin_dir); } - skip_binary = true; } - /* NOT stat(): on Windows it goes through the ANSI code page, so an - * extended-length or non-ASCII target reports "absent" and a non-force - * install silently overwrites bytes the user asked to keep. */ - cbm_path_info_t target_status; - bool target_exists = cbm_path_info_utf8(bin_target, &target_status) == 0; - bool same_binary = cbm_same_file(self_path, bin_target); bool do_copy = !skip_binary && !same_binary && (!target_exists || force); /* (#607) Default: preserve existing indexes. `--reset-indexes` opts into @@ -11681,6 +11716,7 @@ int cbm_cmd_install(int argc, char **argv) { #endif cli_install_activation_t activation = { .bin_target = bin_target, + .config_binary = config_binary, .bin_dir = bin_dir, .home = home, .shell_rc = shell_rc, diff --git a/src/cli/cli.h b/src/cli/cli.h index 4d32873ca..da0e06517 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -250,6 +250,8 @@ void cbm_cli_set_client_selection_for_testing(const char *spec); * not recognisably foreign. Detection must be positive evidence — see the * implementation comment. */ const char *cbm_cli_external_manager_name_for_testing(const char *self_path); +/* #2722: override runtime binary self-path detection for testing. */ +void cbm_cli_set_self_path_override_for_test(const char *self_path); /* #1558: expose the config-key table so a test can prove a key is discoverable * (`config list`/`set` both walk it). */ size_t cbm_cli_config_key_count_for_testing(void); diff --git a/tests/test_cli.c b/tests/test_cli.c index 9853761cb..e39f2f7a9 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -18403,6 +18403,65 @@ TEST(cli_external_manager_detection_needs_positive_evidence_issue1566) { PASS(); } +/* #2722: when a binary is managed externally (e.g. mise, brew, nix), `install` + * skips placing a binary at ~/.local/bin, but must write the running managed + * binary path into agent configuration files instead of the non-existent + * default target. */ +TEST(cli_install_managed_binary_config_path_issue2722) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-install-managed-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) { + FAIL("cbm_mkdtemp failed"); + } + char *old_home = NULL; + char *old_cache = NULL; + cli_activation_save_env(&old_home, &old_cache); + cbm_setenv("HOME", tmpdir, 1); + char cache_dir[512]; + snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); + cbm_setenv("CBM_CACHE_DIR", cache_dir, 1); + + test_mark_claude_code_installed(tmpdir); + + const char *managed_path = + "/Users/x/.local/share/mise/installs/cbm/0.12.1/bin/codebase-memory-mcp"; + cbm_cli_set_self_path_override_for_test(managed_path); + + cli_activation_fake_t fake = { + .mutation_reserve_result = 1, + }; + cbm_cli_activation_ops_t ops = cli_activation_fake_ops(&fake); + cbm_cli_set_activation_ops_for_test(&ops); + + char *argv[] = {"--clients=claude"}; + int rc = cli_test_cmd_install(1, argv); + + cbm_cli_set_activation_ops_for_test(NULL); + cbm_cli_set_self_path_override_for_test(NULL); + + char bin_target[512]; + snprintf(bin_target, sizeof(bin_target), "%s/.local/bin/codebase-memory-mcp", tmpdir); + struct stat target_st; + bool target_created = stat(bin_target, &target_st) == 0; + + char claude_json_path[512]; + snprintf(claude_json_path, sizeof(claude_json_path), "%s/.claude.json", tmpdir); + char *claude_json = read_test_file_alloc(claude_json_path); + bool has_managed_path = claude_json && strstr(claude_json, managed_path) != NULL; + bool has_target_path = + claude_json && strstr(claude_json, ".local/bin/codebase-memory-mcp") != NULL; + + free(claude_json); + cli_activation_restore_env(old_home, old_cache); + test_rmdir_r(tmpdir); + + ASSERT_EQ(rc, 0); + ASSERT_FALSE(target_created); + ASSERT_TRUE(has_managed_path); + ASSERT_FALSE(has_target_path); + PASS(); +} + /* #1558: `install` configured EVERY detected client, so a user who wanted * Claude and Codex had to revert OpenCode and Cursor by hand — and the next * install silently recreated them. The selector restricts it. @@ -18820,6 +18879,7 @@ SUITE(cli) { RUN_TEST(cli_ui_config_keys_are_discoverable_and_settable_issue1558); RUN_TEST(cli_skill_frontmatter_scalars_with_colons_are_quoted_issue1554); RUN_TEST(cli_external_manager_detection_needs_positive_evidence_issue1566); + RUN_TEST(cli_install_managed_binary_config_path_issue2722); RUN_TEST(cli_clients_selector_vocabulary_is_complete_and_strict_issue1558); RUN_TEST(cli_clients_selector_filters_registry_installs_issue1798); RUN_TEST(cli_update_accepts_retired_variant_flags_issue1544); From 113f24faf12bee65cf9678cdbc5ef23612678ede Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Wed, 28 Oct 2026 18:11:31 +0100 Subject: [PATCH 9/9] style(cli): clang-format the managed-binary agent-config call (#2722) Signed-off-by: Martin Vogel --- src/cli/cli.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index 79a08855c..43fccf1bf 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -11335,8 +11335,7 @@ static int cli_install_activate(void *opaque) { prev = activation->bin_target; } agent_config_rc = cbm_install_agent_configs_with_previous( - activation->home, config_binary, prev, activation->force, - activation->dry_run); + activation->home, config_binary, prev, activation->force, activation->dry_run); } if (agent_config_rc != CLI_OK) { cli_activation_transaction_finalize_committed_or_fail_stop(