From 93d2ee59eaec4999ca666a8be214de8d9f2f2bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E6=96=87=E7=91=84?= Date: Fri, 28 Aug 2026 18:55:48 +0800 Subject: [PATCH 1/4] fix(mcp): non-mutating query-only store resolve; annotate the ten read-only tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit search_graph, query_graph, trace_path, get_code_snippet, get_graph_schema, get_architecture, search_code, index_status, check_index_coverage and detect_changes were annotated readOnlyHint=false / destructiveHint=true because resolve_store()'s corrupt-store recovery quarantines database files - a mutation. That conservatism locked every plan-mode client out of the read-only analysis surface (#1100 calls the same tools "eleven read-only analysis tools"), since spec-compliant plan gates refuse tools that are not declared read-only. This closes the gap the annotation comment pointed at. The query-only resolve now classifies a failed integrity check without mutating: no mutation lease, no quarantine. A confirmed-corrupt database is reported (a new STORE_RECOVERY_CORRUPT status; the tool reply names the corruption and points at index_repository instead of a misleading "project not found") and left in place for a write-side open to quarantine and rebuild, exactly as before. Transient verdicts stay retryable. Write-side opens (index_repository, manage_adr updates) keep the existing blocking recovery, quarantine, and generation re-check semantics unchanged. The ten query tools' annotations flip to readOnlyHint=true / destructiveHint=false, making them callable under plan mode in Claude Code, Codex, ZCode, and every other client that gates on the hints. Tests: the annotations expectation table is flipped; the query branch of the guard-balance and guard-denial tests now assert zero leases taken and byte-identical files; the ghost-db test (#704) expects the corruption message; the four write-side recovery tests (generation re-check after lease wait, unique backup naming, publish failure, published-WAL completeness) are driven through manage_adr update - the write path where that recovery now lives. Signed-off-by: 周文瑄 --- src/mcp/mcp.c | 119 ++++++++++++++++++++++++++++++++++++----------- tests/test_mcp.c | 92 ++++++++++++++++++++---------------- 2 files changed, 145 insertions(+), 66 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index af2092a82..ec4871e31 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -754,23 +754,30 @@ typedef struct { } tool_annotation_def_t; /* Tool annotations are deliberately explicit. All tools operate on the local - * repository/index domain, so none cross an open-world trust boundary. */ + * repository/index domain, so none cross an open-world trust boundary. + * + * The ten pure query tools resolve their store through resolve_store(), whose + * query-only path is strictly non-mutating: a corrupt database is reported and + * left in place, never quarantined or rebuilt — quarantine/rebuild is reserved + * for write-side opens (index_repository, manage_adr writes). That is what + * makes readOnlyHint=true honest for them and lets plan-mode clients expose + * them (the "read-only analysis tools" surface described in #1100). */ static const tool_annotation_def_t TOOL_ANNOTATIONS[] = { {"index_repository", false, false, true, false}, - {"search_graph", false, true, true, false}, - {"query_graph", false, true, true, false}, - {"trace_path", false, true, true, false}, - {"get_code_snippet", false, true, true, false}, + {"search_graph", true, false, true, false}, + {"query_graph", true, false, true, false}, + {"trace_path", true, false, true, false}, + {"get_code_snippet", true, false, true, false}, {"get_file_outline", false, true, true, false}, - {"get_graph_schema", false, true, true, false}, + {"get_graph_schema", true, false, true, false}, {"compare_graphs", true, false, true, false}, - {"get_architecture", false, true, true, false}, - {"search_code", false, true, true, false}, + {"get_architecture", true, false, true, false}, + {"search_code", true, false, true, false}, {"list_projects", true, false, true, false}, {"delete_project", false, true, true, false}, - {"index_status", false, true, true, false}, - {"check_index_coverage", false, true, true, false}, - {"detect_changes", false, true, true, false}, + {"index_status", true, false, true, false}, + {"check_index_coverage", true, false, true, false}, + {"detect_changes", true, false, true, false}, {"manage_adr", false, true, false, false}, {"ingest_traces", false, false, false, false}, }; @@ -1624,6 +1631,12 @@ struct cbm_mcp_server { bool owns_store; /* true if we opened the store */ char *current_project; /* which project store is open for (heap) */ time_t store_last_used; /* last time resolve_store was called for a named project */ + /* Set by a query-only resolve that hit a confirmed-corrupt database and + * left it in place (no quarantine, no rebuild). Error builders read this + * so the tool reply names the corruption instead of a misleading + * "project not found". */ + bool readonly_resolve_hit_corrupt; + char readonly_corrupt_project[CBM_SZ_1K]; /* Session + auto-index state */ char session_root[CBM_SZ_1K]; /* detected project root path */ @@ -2217,14 +2230,21 @@ typedef enum { STORE_RECOVERY_NONE, STORE_RECOVERY_BUSY, STORE_RECOVERY_TRY_GUARD_UNAVAILABLE, + /* Query-only resolve: a confirmed-corrupt database was reported and left + * in place. Not an error state of the recovery machinery — the caller is + * expected to answer with the corruption, not retry. */ + STORE_RECOVERY_CORRUPT, } store_recovery_status_t; static cbm_store_t *resolve_store_internal(cbm_mcp_server_t *srv, const char *project, bool mutation_already_held, bool nonblocking_recovery, - store_recovery_status_t *recovery_status) { + store_recovery_status_t *recovery_status, + bool allow_autorecovery) { if (recovery_status) { *recovery_status = STORE_RECOVERY_NONE; } + srv->readonly_resolve_hit_corrupt = false; + srv->readonly_corrupt_project[0] = '\0'; if (!project) { return NULL; /* project is required — no implicit fallback */ } @@ -2254,6 +2274,38 @@ static cbm_store_t *resolve_store_internal(cbm_mcp_server_t *srv, const char *pr project_db_path(project, path, sizeof(path)); srv->store = path[0] ? cbm_store_open_path_query(path) : NULL; if (srv->store) { + /* Query-only resolve: classify a failed integrity check without any + * mutation — no lease, no quarantine. A corrupt database is reported + * and left in place for a write-side open (index_repository) to + * quarantine and rebuild; that is what keeps the ten read-only + * tools' readOnlyHint=true honest. A transient failure (lock, IO, or + * the shallow check's own prepare-error-as-corrupt blind spot) is + * answered as busy and retried by the next resolve. */ + if (!allow_autorecovery && !cbm_store_check_integrity(srv->store)) { + cbm_store_close(srv->store); + srv->store = NULL; + srv->store = cbm_store_open_path_query(path); + cbm_integrity_verdict_t verdict = + srv->store ? cbm_store_check_integrity_verdict(srv->store) + : CBM_INTEGRITY_TRANSIENT; + if (srv->store) { + cbm_store_close(srv->store); + srv->store = NULL; + } + if (verdict == CBM_INTEGRITY_CORRUPT) { + srv->readonly_resolve_hit_corrupt = true; + snprintf(srv->readonly_corrupt_project, + sizeof(srv->readonly_corrupt_project), "%s", project); + cbm_log_warn("store.corrupt_readonly", "project", project, "path", path, + "action", "left in place for a write-side rebuild"); + if (recovery_status) { + *recovery_status = STORE_RECOVERY_CORRUPT; + } + } else if (recovery_status) { + *recovery_status = STORE_RECOVERY_BUSY; + } + return NULL; + } /* Check DB integrity — back up (never silently delete) a corrupt DB */ if (!cbm_store_check_integrity(srv->store)) { cbm_store_close(srv->store); @@ -2358,7 +2410,8 @@ static cbm_store_t *resolve_store_internal(cbm_mcp_server_t *srv, const char *pr } static cbm_store_t *resolve_store(cbm_mcp_server_t *srv, const char *project) { - return resolve_store_internal(srv, project, false, false, NULL); + /* Query-only callers (every read tool): strictly non-mutating resolve. */ + return resolve_store_internal(srv, project, false, false, NULL, false); } /* Forward decl — definition lives below alongside list_projects. */ @@ -2489,16 +2542,28 @@ static char *build_no_store_error(const char *project) { : build_missing_project_error(); } +/* Same contract as build_no_store_error, but when the query-only resolve + * confirmed a corrupt database and left it in place, name that instead of a + * misleading "project not found". */ +static char *build_no_store_error_checked(cbm_mcp_server_t *srv, const char *project) { + if (srv->readonly_resolve_hit_corrupt && project && + strcmp(project, srv->readonly_corrupt_project) == 0) { + return build_project_list_error( + "project store is corrupt (left untouched); run index_repository to rebuild it"); + } + return build_no_store_error(project); +} + /* Bail with the right error when no store is available. */ -#define REQUIRE_STORE(store, project) \ - do { \ - if (!(store)) { \ - char *_err = build_no_store_error(project); \ - char *_res = cbm_mcp_text_result(_err, true); \ - free(_err); \ - free(project); \ - return _res; \ - } \ +#define REQUIRE_STORE(store, project) \ + do { \ + if (!(store)) { \ + char *_err = build_no_store_error_checked(srv, project); \ + char *_res = cbm_mcp_text_result(_err, true); \ + free(_err); \ + free(project); \ + return _res; \ + } \ } while (0) static bool project_has_adr(cbm_store_t *store, const char *project, const char *root_path) { @@ -11627,7 +11692,7 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) { char *root_path = get_project_root(srv, project); if (!root_path) { - char *err = build_no_store_error(project); + char *err = build_no_store_error_checked(srv, project); char *res = cbm_mcp_text_result(err, true); free(err); free(project); @@ -12396,8 +12461,8 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) { * backend the UI /api/adr endpoints use — so writes via the MCP tool and * the UI are visible to each other (#256). */ store_recovery_status_t recovery_status = STORE_RECOVERY_NONE; - cbm_store_t *resolved = - resolve_store_internal(srv, project, mutation_held, !write_request, &recovery_status); + cbm_store_t *resolved = resolve_store_internal(srv, project, mutation_held, !write_request, + &recovery_status, true); if (!resolved) { char *res = NULL; if (recovery_status == STORE_RECOVERY_BUSY) { @@ -12406,7 +12471,7 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) { res = cbm_mcp_text_result("project recovery requires a nonblocking mutation guard", true); } else { - char *err = build_no_store_error(project); + char *err = build_no_store_error_checked(srv, project); res = cbm_mcp_text_result(err, true); free(err); } @@ -12458,7 +12523,7 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) { invalidate_cached_store(srv); resolved = NULL; store = NULL; - resolved = resolve_store_internal(srv, project, true, false, NULL); + resolved = resolve_store_internal(srv, project, true, false, NULL, true); } if (resolved) { store = open_adr_store_for_write(srv, resolved, &owned_rw); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 8db63b1ed..e7959bdee 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -934,23 +934,27 @@ TEST(mcp_tools_have_behavior_annotations) { bool open_world; } expected[] = { {"index_repository", false, false, true, false}, - /* These query tools can reach resolve_store(), whose corrupt-store - * recovery quarantines/removes database files. Keep the annotations - * conservative until query resolution is strictly non-mutating. */ - {"search_graph", false, true, true, false}, - {"query_graph", false, true, true, false}, - {"trace_path", false, true, true, false}, - {"get_code_snippet", false, true, true, false}, + /* The ten query tools resolve their store through the strictly + * non-mutating query-only path: a corrupt database is reported and + * left in place, never quarantined or rebuilt. Quarantine/rebuild is + * a write-side job (index_repository, manage_adr writes), so the + * read-only annotations are honest and plan-mode clients can expose + * these tools. get_file_outline arrived after this split and keeps + * its upstream conservative annotation. */ + {"search_graph", true, false, true, false}, + {"query_graph", true, false, true, false}, + {"trace_path", true, false, true, false}, + {"get_code_snippet", true, false, true, false}, {"get_file_outline", false, true, true, false}, - {"get_graph_schema", false, true, true, false}, + {"get_graph_schema", true, false, true, false}, {"compare_graphs", true, false, true, false}, - {"get_architecture", false, true, true, false}, - {"search_code", false, true, true, false}, + {"get_architecture", true, false, true, false}, + {"search_code", true, false, true, false}, {"list_projects", true, false, true, false}, {"delete_project", false, true, true, false}, - {"index_status", false, true, true, false}, - {"check_index_coverage", false, true, true, false}, - {"detect_changes", false, true, true, false}, + {"index_status", true, false, true, false}, + {"check_index_coverage", true, false, true, false}, + {"detect_changes", true, false, true, false}, {"manage_adr", false, true, false, false}, {"ingest_traces", false, false, false, false}, }; @@ -7553,9 +7557,11 @@ TEST(tool_cross_repo_honors_source_name_override) { } /* Corrupt-store quarantine renames/unlinks the project DB and sidecars, so it - * is a mutation even when resolve_store() was reached by a query tool. Generic - * queries use a blocking guard for that recovery, while manage_adr reads must - * use one nonblocking acquisition and never nest a blocking lease. */ + * is a mutation — and exactly the mutation a query-only resolve must never + * perform. A query tool that meets a corrupt store reports the corruption and + * leaves every file in place, taking no mutation lease; the quarantine path + * belongs to write-side opens, where manage_adr reads must use one nonblocking + * acquisition and never nest a blocking lease. */ TEST(tool_corrupt_store_cleanup_guard_is_balanced_and_not_nested) { char cache[256]; snprintf(cache, sizeof(cache), "%s/cbm-mcp-corrupt-guard-XXXXXX", cbm_tmpdir()); @@ -7581,6 +7587,7 @@ TEST(tool_corrupt_store_cleanup_guard_is_balanced_and_not_nested) { char *resp = cbm_mcp_handle_tool(query_srv, "search_graph", "{\"project\":\"guard-corrupt-project\",\"name_pattern\":\".*\"}"); + bool query_reports_corruption = resp && strstr(resp, "corrupt") != NULL; free(resp); cbm_mcp_server_free(query_srv); char query_backup_path[CBM_SZ_1K]; @@ -7617,13 +7624,14 @@ TEST(tool_corrupt_store_cleanup_guard_is_balanced_and_not_nested) { free(saved_cache_copy); cbm_rmdir(cache); - ASSERT_TRUE(query_quarantined); - ASSERT_EQ(query_probe.begin_count, 1); - ASSERT_EQ(query_probe.end_count, 1); - ASSERT_STR_EQ(query_probe.begin_projects[0], project); - ASSERT_STR_EQ(query_probe.end_projects[0], project); - ASSERT_TRUE(query_probe.db_exists_at_begin); - ASSERT_FALSE(query_probe.db_exists_at_end); + /* A query-only resolve never repairs: no lease is taken, the corrupt DB + * and its sidecars stay in place, and the reply names the corruption + * instead of "project not found". */ + ASSERT_FALSE(query_quarantined); + ASSERT_TRUE(cbm_file_exists(db_path)); + ASSERT_TRUE(query_reports_corruption); + ASSERT_EQ(query_probe.begin_count, 0); + ASSERT_EQ(query_probe.end_count, 0); ASSERT_TRUE(adr_quarantined); ASSERT_EQ(adr_probe.begin_count, 0); ASSERT_EQ(adr_probe.try_begin_count, 1); @@ -7636,8 +7644,9 @@ TEST(tool_corrupt_store_cleanup_guard_is_balanced_and_not_nested) { } /* Integrity is checked before the lease is requested, but quarantine itself - * must fail closed when that lease is denied. In particular, a rejected query - * may not remove either a recoverable DB generation or its committed WAL. */ + * must fail closed when that lease is denied. A query-only resolve never + * reaches that point at all: it asks for no lease, touches no file, and + * leaves both a recoverable DB generation and its committed WAL untouched. */ TEST(tool_corrupt_store_cleanup_guard_denial_preserves_db_and_wal) { char cache[256]; snprintf(cache, sizeof(cache), "%s/cbm-mcp-corrupt-denied-XXXXXX", cbm_tmpdir()); @@ -7695,9 +7704,9 @@ TEST(tool_corrupt_store_cleanup_guard_denial_preserves_db_and_wal) { free(saved_cache_copy); cbm_rmdir(cache); - ASSERT_EQ(begin_count, 1); + ASSERT_EQ(begin_count, 0); ASSERT_EQ(end_count, 0); - ASSERT_TRUE(guarded_project); + ASSERT_FALSE(guarded_project); ASSERT_TRUE(db_unchanged); ASSERT_TRUE(wal_unchanged); ASSERT_EQ(backup_count, 0); @@ -7801,10 +7810,11 @@ TEST(tool_manage_adr_corrupt_store_missing_try_guard_reports_configuration) { PASS(); } -/* Another session may publish a good generation while this query waits for - * the mutation lease. Cleanup must re-open and re-check the path after lease - * acquisition; quarantining based on the stale pre-wait handle loses the new - * generation and returns a false "not indexed" result. */ +/* A write-side open must trust only the generation that is current after its + * mutation lease is held: another session may have published a good + * generation while this request waited for the lease. Quarantining based on + * a stale pre-wait handle loses the new generation and returns a false + * "not indexed" result. */ TEST(tool_corrupt_store_cleanup_rechecks_generation_after_guard_wait) { char cache[256]; snprintf(cache, sizeof(cache), "%s/cbm-mcp-corrupt-recheck-XXXXXX", cbm_tmpdir()); @@ -7832,7 +7842,8 @@ TEST(tool_corrupt_store_cleanup_rechecks_generation_after_guard_wait) { cbm_mcp_server_set_project_mutation_guard(srv, mcp_replacing_mutation_guard_begin, mcp_replacing_mutation_guard_end, &replacement); char *resp = cbm_mcp_handle_tool( - srv, "search_graph", "{\"project\":\"guard-corrupt-recheck\",\"name_pattern\":\".*\"}"); + srv, "manage_adr", + "{\"project\":\"guard-corrupt-recheck\",\"mode\":\"update\",\"content\":\"# ADR\\n\\nPending replacement.\"}"); bool response_used_replacement = resp && !response_contains_json_fragment(resp, "\"isError\":true"); free(resp); @@ -7908,7 +7919,8 @@ TEST(tool_corrupt_store_cleanup_preserves_existing_backup_and_uses_unique_name) cbm_mcp_server_set_project_mutation_guard(srv, mcp_mutation_guard_probe_begin, mcp_mutation_guard_probe_end, &probe); char *resp = cbm_mcp_handle_tool( - srv, "search_graph", "{\"project\":\"guard-corrupt-unique\",\"name_pattern\":\".*\"}"); + srv, "manage_adr", + "{\"project\":\"guard-corrupt-unique\",\"mode\":\"update\",\"content\":\"# ADR\\n\\nPending quarantine.\"}"); free(resp); cbm_mcp_server_free(srv); @@ -7981,9 +7993,9 @@ TEST(tool_corrupt_store_cleanup_publish_failure_preserves_db_and_wal) { mcp_mutation_guard_probe_end, &guard); mcp_quarantine_hook_probe_t hook = {.deny_step = "before_snapshot_publish"}; cbm_mcp_server_set_quarantine_test_hook(srv, mcp_quarantine_hook_probe, &hook); - char *resp = - cbm_mcp_handle_tool(srv, "search_graph", - "{\"project\":\"guard-corrupt-publish-fail\",\"name_pattern\":\".*\"}"); + char *resp = cbm_mcp_handle_tool( + srv, "manage_adr", + "{\"project\":\"guard-corrupt-publish-fail\",\"mode\":\"update\",\"content\":\"# ADR\\n\\nPending publish.\"}"); bool db_unchanged = mcp_file_matches_snapshot(db_path, db_before, db_len); bool wal_unchanged = mcp_file_matches_snapshot(wal_path, wal_before, wal_len); @@ -8056,8 +8068,8 @@ TEST(tool_corrupt_store_cleanup_publishes_complete_wal_snapshot_before_delete) { mcp_quarantine_hook_probe_t hook = {.deny_step = "after_snapshot_publish"}; cbm_mcp_server_set_quarantine_test_hook(srv, mcp_quarantine_hook_probe, &hook); char *resp = cbm_mcp_handle_tool( - srv, "search_graph", - "{\"project\":\"guard-corrupt-after-publish\",\"name_pattern\":\".*\"}"); + srv, "manage_adr", + "{\"project\":\"guard-corrupt-after-publish\",\"mode\":\"update\",\"content\":\"# ADR\\n\\nPending publish.\"}"); bool db_unchanged = mcp_file_matches_snapshot(db_path, db_before, db_len); bool wal_unchanged = mcp_file_matches_snapshot(wal_path, wal_before, wal_len); @@ -11071,12 +11083,14 @@ TEST(tool_resolve_store_by_internal_name_issue704) { free(q_alpha); /* ── D: the 0-byte ghost is NOT resolvable ─────────────────────── */ + /* A query-only resolve reports the corrupt generation and leaves it in + * place — the reply names the corruption instead of "not found". */ char *q_ghost = cbm_mcp_server_handle( srv, "{\"jsonrpc\":\"2.0\",\"id\":4,\"method\":\"tools/call\"," "\"params\":{\"name\":\"search_graph\",\"arguments\":{" "\"project\":\"ghost704\",\"name_pattern\":\".*\",\"limit\":5}}}"); ASSERT_NOT_NULL(q_ghost); - ASSERT_NOT_NULL(strstr(q_ghost, "not found")); + ASSERT_NOT_NULL(strstr(q_ghost, "corrupt")); free(q_ghost); /* ── E: addressing the drifted db by its FILENAME stays not-found ── */ From 2d8721a60360df5804dfcce37640a2ca5fa0eac0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E6=96=87=E7=91=84?= Date: Fri, 28 Aug 2026 20:26:08 +0800 Subject: [PATCH 2/4] style(mcp): clang-format the query-only resolve and error builder changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI lint gate flagged 18 clang-format violations in the lines the previous commit touched (the read-only verdict branch inside resolve_store_internal, the REQUIRE_STORE macro, and the two call sites that switched to build_no_store_error_checked). No semantic change: whitespace and line wrapping only; the mcp suite still passes 202/0/7. Signed-off-by: 周文瑄 --- src/mcp/mcp.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index ec4871e31..aefc810cd 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -2285,19 +2285,19 @@ static cbm_store_t *resolve_store_internal(cbm_mcp_server_t *srv, const char *pr cbm_store_close(srv->store); srv->store = NULL; srv->store = cbm_store_open_path_query(path); - cbm_integrity_verdict_t verdict = - srv->store ? cbm_store_check_integrity_verdict(srv->store) - : CBM_INTEGRITY_TRANSIENT; + cbm_integrity_verdict_t verdict = srv->store + ? cbm_store_check_integrity_verdict(srv->store) + : CBM_INTEGRITY_TRANSIENT; if (srv->store) { cbm_store_close(srv->store); srv->store = NULL; } if (verdict == CBM_INTEGRITY_CORRUPT) { srv->readonly_resolve_hit_corrupt = true; - snprintf(srv->readonly_corrupt_project, - sizeof(srv->readonly_corrupt_project), "%s", project); - cbm_log_warn("store.corrupt_readonly", "project", project, "path", path, - "action", "left in place for a write-side rebuild"); + snprintf(srv->readonly_corrupt_project, sizeof(srv->readonly_corrupt_project), "%s", + project); + cbm_log_warn("store.corrupt_readonly", "project", project, "path", path, "action", + "left in place for a write-side rebuild"); if (recovery_status) { *recovery_status = STORE_RECOVERY_CORRUPT; } @@ -2555,15 +2555,15 @@ static char *build_no_store_error_checked(cbm_mcp_server_t *srv, const char *pro } /* Bail with the right error when no store is available. */ -#define REQUIRE_STORE(store, project) \ - do { \ - if (!(store)) { \ - char *_err = build_no_store_error_checked(srv, project); \ - char *_res = cbm_mcp_text_result(_err, true); \ - free(_err); \ - free(project); \ - return _res; \ - } \ +#define REQUIRE_STORE(store, project) \ + do { \ + if (!(store)) { \ + char *_err = build_no_store_error_checked(srv, project); \ + char *_res = cbm_mcp_text_result(_err, true); \ + free(_err); \ + free(project); \ + return _res; \ + } \ } while (0) static bool project_has_adr(cbm_store_t *store, const char *project, const char *root_path) { @@ -12034,7 +12034,7 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) { /* module rollup: a quotient view of the blast radius */ if (impact.visited_count > 0) { cbm_sb_append(&sb, "impacted_modules: (rows: module count)\n"); - char (*mods)[CBM_SZ_128] = malloc(DETECT_MODCAP * CBM_SZ_128); + char(*mods)[CBM_SZ_128] = malloc(DETECT_MODCAP * CBM_SZ_128); int *mcnt = malloc(DETECT_MODCAP * sizeof(int)); if (mods && mcnt) { int overflow = 0; @@ -12097,7 +12097,7 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) { yyjson_mut_obj_add_val(doc, root_obj, "impacted", imp); /* Model parity with the tree encoding: the complete module rollup. */ if (impact.visited_count > 0) { - char (*mods)[CBM_SZ_128] = malloc(DETECT_MODCAP * CBM_SZ_128); + char(*mods)[CBM_SZ_128] = malloc(DETECT_MODCAP * CBM_SZ_128); int *mcnt = malloc(DETECT_MODCAP * sizeof(int)); if (mods && mcnt) { int overflow = 0; @@ -12461,8 +12461,8 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) { * backend the UI /api/adr endpoints use — so writes via the MCP tool and * the UI are visible to each other (#256). */ store_recovery_status_t recovery_status = STORE_RECOVERY_NONE; - cbm_store_t *resolved = resolve_store_internal(srv, project, mutation_held, !write_request, - &recovery_status, true); + cbm_store_t *resolved = + resolve_store_internal(srv, project, mutation_held, !write_request, &recovery_status, true); if (!resolved) { char *res = NULL; if (recovery_status == STORE_RECOVERY_BUSY) { From 240dc8156ba3ce9be92e2142e8506fef727710b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E6=96=87=E7=91=84?= Date: Fri, 28 Aug 2026 20:46:03 +0800 Subject: [PATCH 3/4] style(mcp): keep the upstream pointer-to-array declaration spacing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The local clang-format 15 that produced the previous commit removes the space in 'char (*mods)[CBM_SZ_128]' (pointer-to-array declaration), while the CI formatter keeps it; 15 also rewrote those two pre-existing detect_changes lines even though they were untouched by this PR. Restore the upstream spelling so the CI format gate passes and the PR diff no longer touches lines it has no reason to touch. Signed-off-by: 周文瑄 --- src/mcp/mcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index aefc810cd..4eeebf45a 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -12034,7 +12034,7 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) { /* module rollup: a quotient view of the blast radius */ if (impact.visited_count > 0) { cbm_sb_append(&sb, "impacted_modules: (rows: module count)\n"); - char(*mods)[CBM_SZ_128] = malloc(DETECT_MODCAP * CBM_SZ_128); + char (*mods)[CBM_SZ_128] = malloc(DETECT_MODCAP * CBM_SZ_128); int *mcnt = malloc(DETECT_MODCAP * sizeof(int)); if (mods && mcnt) { int overflow = 0; @@ -12097,7 +12097,7 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) { yyjson_mut_obj_add_val(doc, root_obj, "impacted", imp); /* Model parity with the tree encoding: the complete module rollup. */ if (impact.visited_count > 0) { - char(*mods)[CBM_SZ_128] = malloc(DETECT_MODCAP * CBM_SZ_128); + char (*mods)[CBM_SZ_128] = malloc(DETECT_MODCAP * CBM_SZ_128); int *mcnt = malloc(DETECT_MODCAP * sizeof(int)); if (mods && mcnt) { int overflow = 0; From adb49b3a28826e6eef871b1777b63668199c0463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E6=96=87=E7=91=84?= Date: Fri, 28 Aug 2026 23:25:47 +0800 Subject: [PATCH 4/4] test(mcp): snapshot the query-branch DB state before suite teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's tsan and lsan-macos runs failed the mutation-guard suite on ASSERT(cbm_file_exists(db_path)) at the guard-balance test, while the behavior suite passed everywhere. The assertion ran after the test's own teardown: cleanup_project_db() unlinks the same db path before the assertion block, and the manage_adr branch above replants and quarantines that very path, so the late file check observed the test's cleanup, not the query-only resolve it was meant to pin. Whether it tripped depended on unlink/visibility timing, which is why it surfaced only under some schedulers and never in the single-suite local runs. Capture cbm_file_exists() into query_db_left_in_place right after the query branch completes, before anything replants or removes the file, and assert on that snapshot. Signed-off-by: 周文瑄 --- tests/test_mcp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_mcp.c b/tests/test_mcp.c index e7959bdee..dc7716b14 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -7595,6 +7595,11 @@ TEST(tool_corrupt_store_cleanup_guard_is_balanced_and_not_nested) { mcp_find_corrupt_backups(cache, project, query_backup_path, sizeof(query_backup_path)); bool query_quarantined = !cbm_file_exists(db_path) && query_backup_count == 1 && query_backup_path[0] != '\0'; + /* Snapshot the on-disk state HERE: the manage_adr branch below replants + * and then quarantines this same path, and the suite's cleanup unlinks + * it before the assertions run — a later cbm_file_exists() would observe + * that teardown, not the query-only resolve this check pins. */ + bool query_db_left_in_place = cbm_file_exists(db_path); /* Replant the same deterministic corruption to exercise manage_adr's * already-held lease independently from the query server above. */ @@ -7628,7 +7633,7 @@ TEST(tool_corrupt_store_cleanup_guard_is_balanced_and_not_nested) { * and its sidecars stay in place, and the reply names the corruption * instead of "project not found". */ ASSERT_FALSE(query_quarantined); - ASSERT_TRUE(cbm_file_exists(db_path)); + ASSERT_TRUE(query_db_left_in_place); ASSERT_TRUE(query_reports_corruption); ASSERT_EQ(query_probe.begin_count, 0); ASSERT_EQ(query_probe.end_count, 0);