trending/codebase-memory-mcp
1
0
Fork 0
mirror of https://github.com/DeusData/codebase-memory-mcp.git synced 2026-10-29 07:13:44 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(mcp): fail closed on incomplete snapshots

Browse Source 
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel
2026-09-05 06:05:08 +02:00
parent 14c229b158
commit 5630f11f21
3 changed files with 166 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/mcp/mcp.c
+99 -32
View File
@@ -1702,6 +1702,8 @@ struct cbm_mcp_server {
#ifdef CBM_ENABLE_TEST_SEAMS
cbm_mcp_auto_index_count_test_hook_fn auto_index_count_test_hook;
void *auto_index_count_test_context;
cbm_mcp_snapshot_read_test_hook_fn snapshot_read_test_hook;
void *snapshot_read_test_context;
#endif
size_t search_output_limit_override;
const char *search_scan_command_override;
@@ -2035,6 +2037,17 @@ void cbm_mcp_server_set_search_scan_timeout_for_test(cbm_mcp_server_t *srv, uint
srv->search_scan_timeout_override_set = override_set;
}
}
#ifdef CBM_ENABLE_TEST_SEAMS
void cbm_mcp_server_set_snapshot_read_test_hook(cbm_mcp_server_t *srv,
cbm_mcp_snapshot_read_test_hook_fn hook,
void *context) {
if (!srv) {
return;
}
srv->snapshot_read_test_hook = hook;
srv->snapshot_read_test_context = context;
}
#endif
/* ── Cache dir + project DB path helpers ───────────────────────── */
@@ -14889,19 +14902,19 @@ static void detect_snapshot_add_field(cbm_sha256_ctx *hash, const char *value) {
cbm_sha256_update(hash, "|", 1);
}
static void detect_snapshot_add_changed_file(cbm_sha256_ctx *hash, const char *root_path,
const char *relative_path) {
static bool detect_snapshot_add_changed_file(cbm_mcp_server_t *srv, cbm_sha256_ctx *hash,
const char *root_path, const char *relative_path) {
detect_snapshot_add_field(hash, relative_path);
size_t root_len = strlen(root_path);
size_t relative_len = strlen(relative_path);
if (root_len > (size_t)-1 - relative_len - 2U) {
detect_snapshot_add_field(hash, "path_overflow");
return;
return false;
}
char *absolute = malloc(root_len + relative_len + 2U);
if (!absolute) {
detect_snapshot_add_field(hash, "path_oom");
return;
return false;
}
memcpy(absolute, root_path, root_len);
absolute[root_len] = '/';
@@ -14909,33 +14922,59 @@ static void detect_snapshot_add_changed_file(cbm_sha256_ctx *hash, const char *r
cbm_path_info_t info = {0};
if (cbm_path_info_utf8(absolute, &info) != 0) {
detect_snapshot_add_field(hash, "absent");
/* A missing path can be a legitimate deletion, but the portable
* metadata API cannot distinguish it from permission and transient
* inspection failures on every supported platform. Hash the observed
* state for diagnostics, but do not issue a cursor from incomplete
* evidence. Offset paging remains available. */
detect_snapshot_add_field(hash, "metadata_unavailable");
free(absolute);
return;
return false;
}
char metadata[160];
snprintf(metadata, sizeof(metadata), "r%d:d%d:l%d:s%lld:m%lld", info.is_regular ? 1 : 0,
info.is_directory ? 1 : 0, info.is_symlink ? 1 : 0, (long long)info.size,
(long long)info.mtime_ns);
detect_snapshot_add_field(hash, metadata);
bool complete = true;
if (info.is_regular) {
FILE *file = cbm_fopen(absolute, "rb");
bool allow_open = true;
#ifdef CBM_ENABLE_TEST_SEAMS
if (srv && srv->snapshot_read_test_hook) {
allow_open = srv->snapshot_read_test_hook(srv->snapshot_read_test_context, absolute);
}
#else
(void)srv;
#endif
FILE *file = allow_open ? cbm_fopen(absolute, "rb") : NULL;
if (file) {
unsigned char buffer[64 * 1024];
size_t count;
while ((count = fread(buffer, 1, sizeof(buffer), file)) > 0) {
cbm_sha256_update(hash, buffer, count);
}
detect_snapshot_add_field(hash, ferror(file) ? "read_error" : "read_complete");
bool read_error = ferror(file) != 0;
detect_snapshot_add_field(hash, read_error ? "read_error" : "read_complete");
complete = !read_error;
(void)fclose(file);
} else {
detect_snapshot_add_field(hash, "open_error");
complete = false;
}
} else if (info.is_symlink) {
/* lstat-style metadata identifies the link object but not its target
* bytes. The cross-platform metadata API intentionally does not expose
* a readlink/reparse payload, so fail closed instead of pretending the
* cursor is bound to the changed link identity. */
detect_snapshot_add_field(hash, "link_identity_unavailable");
complete = false;
}
free(absolute);
return complete;
}
static void detect_snapshot_fingerprint(const char *root_path, const char *head_oid,
static bool detect_snapshot_fingerprint(cbm_mcp_server_t *srv, const char *root_path,
const char *head_oid,
const char *base_oid, const char *merge_base,
const char *generation, char **files, int file_count,
const cbm_traverse_result_t *impact,
@@ -14948,8 +14987,11 @@ static void detect_snapshot_fingerprint(const char *root_path, const char *head_
detect_snapshot_add_field(&hash, base_oid);
detect_snapshot_add_field(&hash, merge_base);
detect_snapshot_add_field(&hash, generation);
bool complete = true;
for (int i = 0; i < file_count; i++) {
detect_snapshot_add_changed_file(&hash, root_path, files[i]);
if (!detect_snapshot_add_changed_file(srv, &hash, root_path, files[i])) {
complete = false;
}
}
if (impact) {
for (int i = 0; i < impact->visited_count; i++) {
@@ -14980,6 +15022,7 @@ static void detect_snapshot_fingerprint(const char *root_path, const char *head_
out[i * 2 + 1] = hex[digest[i] & 0x0f];
}
out[32] = '\0';
return complete;
}
static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) {
@@ -15522,13 +15565,20 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) {
char generation[96] = "unknown";
(void)cbm_store_generation(store, generation, sizeof(generation));
char detect_snapshot[33];
detect_snapshot_fingerprint(root_path, head_oid, base_oid, merge_base, generation, files,
file_count, &impact, modules, nmods, module_overflow,
detect_snapshot);
bool detect_snapshot_complete = detect_snapshot_fingerprint(
srv, root_path, head_oid, base_oid, merge_base, generation, files, file_count, &impact,
modules, nmods, module_overflow, detect_snapshot);
const char *cursor_error = NULL;
detect_cursor_t decoded_cursor = {0};
if (changed_cursor_arg && changed_cursor_arg[0]) {
bool cursor_supplied = (changed_cursor_arg && changed_cursor_arg[0]) ||
(impact_cursor_arg && impact_cursor_arg[0]) ||
(module_cursor_arg && module_cursor_arg[0]);
if (cursor_supplied && !detect_snapshot_complete) {
cursor_error = "snapshot_unavailable: changed file bytes could not be fingerprinted — "
"rerun without the cursor after the files are readable";
}
if (!cursor_error && changed_cursor_arg && changed_cursor_arg[0]) {
if (changed_offset != 0) {
cursor_error = "cursor_params_mismatch: changed_cursor cannot be combined with a "
"nonzero changed_offset";
@@ -15621,6 +15671,9 @@ render_detect_output:
if (output_budget_floor_exceeded) {
cbm_tree_scalar_bool(&sb, "output_budget_floor_exceeded", true);
}
if (!detect_snapshot_complete) {
cbm_tree_scalar_bool(&sb, "snapshot_cursor_unavailable", true);
}
/* Changed files are independently pageable: traversal still used every
* file, so paging affects presentation only, never the graph answer. */
cbm_tree_scalar_int(&sb, "changed_total", file_count);
@@ -15630,9 +15683,11 @@ render_detect_output:
if (changed_has_more && changed_returned > 0) {
cbm_tree_scalar_int(&sb, "changed_next_offset", changed_start + changed_returned);
char cursor[80];
detect_cursor_encode('c', detect_snapshot, detect_qhash,
changed_start + changed_returned, cursor);
cbm_tree_scalar_str(&sb, "changed_next_cursor", cursor);
if (detect_snapshot_complete) {
detect_cursor_encode('c', detect_snapshot, detect_qhash,
changed_start + changed_returned, cursor);
cbm_tree_scalar_str(&sb, "changed_next_cursor", cursor);
}
} else if (changed_has_more) {
cbm_tree_scalar_bool(&sb,
changed_limit == 0 ? "changed_continuation_requires_positive_limit"
@@ -15659,7 +15714,8 @@ render_detect_output:
cbm_tree_scalar_int(&sb, "seed_symbols", seed_count);
if (want_symbols) {
detect_emit_impacted_tree(&sb, &impact, imp_start, imp_returned, truncated);
if (imp_start + imp_returned < impact.visited_count && imp_returned > 0) {
if (detect_snapshot_complete && imp_start + imp_returned < impact.visited_count &&
imp_returned > 0) {
char cursor[80];
detect_cursor_encode('i', detect_snapshot, detect_qhash, imp_start + imp_returned,
cursor);
@@ -15675,9 +15731,11 @@ render_detect_output:
if (module_has_more && module_returned > 0) {
cbm_tree_scalar_int(&sb, "module_next_offset", module_start + module_returned);
char cursor[80];
detect_cursor_encode('m', detect_snapshot, detect_qhash,
module_start + module_returned, cursor);
cbm_tree_scalar_str(&sb, "module_next_cursor", cursor);
if (detect_snapshot_complete) {
detect_cursor_encode('m', detect_snapshot, detect_qhash,
module_start + module_returned, cursor);
cbm_tree_scalar_str(&sb, "module_next_cursor", cursor);
}
} else if (module_has_more) {
cbm_tree_scalar_bool(&sb,
module_limit == 0
@@ -15735,6 +15793,9 @@ render_detect_output:
if (output_budget_floor_exceeded) {
yyjson_mut_obj_add_bool(doc, root_obj, "output_budget_floor_exceeded", true);
}
if (!detect_snapshot_complete) {
yyjson_mut_obj_add_bool(doc, root_obj, "snapshot_cursor_unavailable", true);
}
yyjson_mut_obj_add_int(doc, root_obj, "changed_total", file_count);
yyjson_mut_obj_add_int(doc, root_obj, "changed_returned", changed_returned);
bool changed_has_more = changed_start + changed_returned < file_count;
@@ -15743,9 +15804,11 @@ render_detect_output:
yyjson_mut_obj_add_int(doc, root_obj, "changed_next_offset",
changed_start + changed_returned);
char cursor[80];
detect_cursor_encode('c', detect_snapshot, detect_qhash,
changed_start + changed_returned, cursor);
yyjson_mut_obj_add_strcpy(doc, root_obj, "changed_next_cursor", cursor);
if (detect_snapshot_complete) {
detect_cursor_encode('c', detect_snapshot, detect_qhash,
changed_start + changed_returned, cursor);
yyjson_mut_obj_add_strcpy(doc, root_obj, "changed_next_cursor", cursor);
}
} else if (changed_has_more) {
yyjson_mut_obj_add_bool(doc, root_obj,
changed_limit == 0
@@ -15781,10 +15844,12 @@ render_detect_output:
yyjson_mut_obj_add_bool(doc, root_obj, "impacted_has_more", impacted_has_more);
if (impacted_has_more && imp_returned > 0) {
yyjson_mut_obj_add_int(doc, root_obj, "impacted_next_offset", imp_start + imp_returned);
char cursor[80];
detect_cursor_encode('i', detect_snapshot, detect_qhash, imp_start + imp_returned,
cursor);
yyjson_mut_obj_add_strcpy(doc, root_obj, "impacted_next_cursor", cursor);
if (detect_snapshot_complete) {
char cursor[80];
detect_cursor_encode('i', detect_snapshot, detect_qhash, imp_start + imp_returned,
cursor);
yyjson_mut_obj_add_strcpy(doc, root_obj, "impacted_next_cursor", cursor);
}
} else if (impacted_has_more) {
yyjson_mut_obj_add_bool(doc, root_obj, "impacted_continuation_requires_higher_budget",
true);
@@ -15800,10 +15865,12 @@ render_detect_output:
if (module_has_more && module_returned > 0) {
yyjson_mut_obj_add_int(doc, root_obj, "module_next_offset",
module_start + module_returned);
char cursor[80];
detect_cursor_encode('m', detect_snapshot, detect_qhash,
module_start + module_returned, cursor);
yyjson_mut_obj_add_strcpy(doc, root_obj, "module_next_cursor", cursor);
if (detect_snapshot_complete) {
char cursor[80];
detect_cursor_encode('m', detect_snapshot, detect_qhash,
module_start + module_returned, cursor);
yyjson_mut_obj_add_strcpy(doc, root_obj, "module_next_cursor", cursor);
}
} else if (module_has_more) {
yyjson_mut_obj_add_bool(doc, root_obj,
module_limit == 0
src/mcp/mcp_internal.h
+8
View File
@@ -76,6 +76,14 @@ void cbm_search_code_build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bo
const char *file_pattern, const char *tmpfile,
const char *filelist, const char *root_path);
#ifdef CBM_ENABLE_TEST_SEAMS
/* Reject opening one changed file while detect_changes fingerprints its live
* snapshot. This makes otherwise platform-specific permission/read races
* deterministic without changing production filesystem behavior. */
typedef bool (*cbm_mcp_snapshot_read_test_hook_fn)(void *context, const char *absolute_path);
void cbm_mcp_server_set_snapshot_read_test_hook(cbm_mcp_server_t *srv,
cbm_mcp_snapshot_read_test_hook_fn hook,
void *context);
/* Filesystem PATH_MAX prevents a portable end-to-end fixture for multi-KiB
* stored paths. This seam drives the normal search result ownership,
* directory aggregation, and renderer pipeline with synthetic identities. */
tests/test_mcp.c
+59
View File
@@ -266,6 +266,10 @@ static bool mcp_search_cache_close(mcp_search_cache_t *cache) {
cache->saved_cache = NULL;
return th_rmtree(cache->path) == 0;
}
static bool mcp_snapshot_read_hook_probe(void *context, const char *absolute_path) {
const char *rejected_fragment = context;
return !rejected_fragment || !absolute_path || !strstr(absolute_path, rejected_fragment);
}
typedef struct {
const char *name;
@@ -13327,6 +13331,37 @@ TEST(tool_detect_changes_pages_changed_files_and_honors_semantic_budget) {
free(inner);
free(response);
/* A cursor is only safe when every changed regular file contributed its
* bytes. Force a portable open failure after Git has listed the file: the
* first page must stay usable but cannot advertise snapshot-bound cursors,
* and an already-issued cursor must fail closed while the snapshot cannot
* be re-established. */
cbm_mcp_server_set_snapshot_read_test_hook(srv, mcp_snapshot_read_hook_probe, seed_file);
response = cbm_mcp_handle_tool(
srv, "detect_changes",
"{\"project\":\"detect-pages-project\",\"base_branch\":\"HEAD\","
"\"scope\":\"files\",\"format\":\"json\"}");
ASSERT_NOT_NULL(response);
inner = extract_text_content(response);
ASSERT_NOT_NULL(inner);
doc = yyjson_read(inner, strlen(inner), 0);
ASSERT_NOT_NULL(doc);
root = yyjson_doc_get_root(doc);
ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(root, "snapshot_cursor_unavailable")));
ASSERT_NULL(yyjson_obj_get(root, "changed_next_cursor"));
yyjson_doc_free(doc);
free(inner);
free(response);
response = cbm_mcp_handle_tool(srv, "detect_changes", cursor_args);
ASSERT_NOT_NULL(response);
inner = extract_text_content(response);
ASSERT_NOT_NULL(inner);
ASSERT_NOT_NULL(strstr(inner, "snapshot_unavailable"));
free(inner);
free(response);
cbm_mcp_server_set_snapshot_read_test_hook(srv, NULL, NULL);
/* The same path set with changed bytes is a different live snapshot. The
* old opaque cursor must fail loudly instead of applying offset 20 to the
* new ordering/impact model. */
@@ -13340,6 +13375,30 @@ TEST(tool_detect_changes_pages_changed_files_and_honors_semantic_budget) {
ASSERT_NOT_NULL(strstr(inner, "snapshot_changed"));
free(inner);
free(response);
#ifndef _WIN32
/* lstat metadata is not the symlink's identity: different target bytes can
* have the same length and timestamp. Until those bytes are fingerprinted,
* keep the otherwise complete answer but withhold snapshot cursors. */
char changed_link[CBM_SZ_4K];
snprintf(changed_link, sizeof(changed_link), "%s/changed-link.c", repo);
ASSERT_EQ(symlink("first-target", changed_link), 0);
response = cbm_mcp_handle_tool(
srv, "detect_changes",
"{\"project\":\"detect-pages-project\",\"base_branch\":\"HEAD\","
"\"scope\":\"files\",\"format\":\"json\"}");
ASSERT_NOT_NULL(response);
inner = extract_text_content(response);
ASSERT_NOT_NULL(inner);
doc = yyjson_read(inner, strlen(inner), 0);
ASSERT_NOT_NULL(doc);
root = yyjson_doc_get_root(doc);
ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(root, "snapshot_cursor_unavailable")));
ASSERT_NULL(yyjson_obj_get(root, "changed_next_cursor"));
yyjson_doc_free(doc);
free(inner);
free(response);
#endif
free(changed_cursor);
cbm_mcp_server_free(srv);