From 1c1d5422056e14ffd7ce86727d291b34f04da26d Mon Sep 17 00:00:00 2001 From: Scott Beck <296871+bline@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:16:12 -0600 Subject: [PATCH] fix(mcp): verify content for freshness and snippets; expose store generation (#2585) check_index_coverage judged freshness by mtime and size alone, although file_hashes records the SHA-256 of the indexed bytes. A same-size edit with the original mtime passed as metadata_match. Identical bytes rewritten with a new mtime stayed metadata_changed after re-indexing, because the no-op incremental route never rewrites the record. get_code_snippet sliced current disk bytes with indexed line numbers, so it could return another symbol's code under the requested name. No response carried an identity that changes on incremental re-index. - Freshness now compares the file's SHA-256 with the recorded hash. metadata_match additionally means the bytes match. content_changed (new) means same metadata but different bytes; content_match (new) means stale metadata but identical bytes. metadata_changed keeps its meaning. A per-path content_verified flag says whether the hash check ran. A path counts as current only when it did: without a usable hash or a readable file, the freshness label keeps its metadata value, but the path recommends read_source_and_reindex and is not exact-path verified. - get_code_snippet reads the file once, checks that buffer's hash, and slices the indexed range from the same buffer, so a concurrent write cannot produce a content_verified snippet of bytes that were never indexed. When the bytes are not the indexed bytes, it withholds the source and reports source_drift, freshness, and recommended_action "reindex_then_retry". Unlike a metadata gate, this also catches the same-size, preserved-mtime case. Unverifiable files keep the existing fallback, marked content_verified: false. - check_index_coverage metadata and get_code_snippet now carry store_generation ("ug", the identity query_graph cursors already use), which advances on every index run. It names the publication current when the response is built; it does not make the response one atomic read. metadata.generation is unchanged: it names the coverage-recording run. Tests (tests/test_mcp.c) reach each state through recorded hash rows instead of file times, so they behave the same on every platform. Without the fix they fail on the defects themselves: "metadata_match" != "content_changed" snippet source contains another function's code store_generation absent unverified metadata match recommended as current a write between check and slice is served as verified (test seam) The #1613 truncated-catalog test now records the file's real hash, as the indexer does, instead of an empty one. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Scott Beck <296871+bline@users.noreply.github.com> --- src/mcp/mcp.c | 267 +++++++++++++++++++++++++++++++++--- src/mcp/mcp_internal.h | 7 + tests/test_mcp.c | 298 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 555 insertions(+), 17 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 9466b7fa8e..287cb9166a 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1816,6 +1816,8 @@ struct cbm_mcp_server { void *auto_index_count_test_context; cbm_mcp_snapshot_read_test_hook_fn snapshot_read_test_hook; void *snapshot_read_test_context; + cbm_mcp_snippet_capture_test_hook_fn snippet_capture_test_hook; + void *snippet_capture_test_context; #endif size_t search_output_limit_override; const char *search_scan_command_override; @@ -2182,6 +2184,16 @@ void cbm_mcp_server_set_snapshot_read_test_hook(cbm_mcp_server_t *srv, srv->snapshot_read_test_hook = hook; srv->snapshot_read_test_context = context; } + +void cbm_mcp_server_set_snippet_capture_test_hook(cbm_mcp_server_t *srv, + cbm_mcp_snippet_capture_test_hook_fn hook, + void *context) { + if (!srv) { + return; + } + srv->snippet_capture_test_hook = hook; + srv->snippet_capture_test_context = context; +} #endif /* ── Cache dir + project DB path helpers ───────────────────────── */ @@ -6789,10 +6801,161 @@ static coverage_path_result_t coverage_normalize_rel(const char *input, bool all return written > 0U || allow_root ? COVERAGE_PATH_OK : COVERAGE_PATH_INVALID; } -static const char *coverage_path_freshness(cbm_store_t *store, const char *project, - const char *root_path, const char *rel_path, - bool *outside) { +/* #2585: the store generation ("ug") advances on every + * index run, incremental ones included, and on every publication. It is read + * when the response is built and names the publication current then; it does + * not make the response one atomic read of that publication. + * metadata.generation names the coverage-recording run and stays fixed across + * incremental re-indexes. Empty when unreadable. */ +static void mcp_add_store_generation(yyjson_mut_doc *doc, yyjson_mut_val *obj, cbm_store_t *store) { + char generation[QUERY_GRAPH_GENERATION_CAP] = ""; + if (!store || cbm_store_generation(store, generation, sizeof(generation)) != CBM_STORE_OK) { + generation[0] = '\0'; + } + yyjson_mut_obj_add_strcpy(doc, obj, "store_generation", generation); +} + +/* #2585: file_hashes.sha256 is the SHA-256 of the raw bytes the indexer read + * (pipeline_incremental.c semantic_manifest_hash_file). Recomputing it is the + * only way to say the bytes on disk are the bytes that were indexed: mtime and + * size miss a same-size edit whose timestamp was preserved (cp -p, rsync -t, + * archive extraction), and go stale when identical bytes are rewritten. */ +static bool mcp_file_sha256_hex(const char *abs_path, char out[CBM_SHA256_HEX_LEN + 1]) { + FILE *file = cbm_fopen(abs_path, "rb"); + if (!file) { + return false; + } + cbm_sha256_ctx sha; + cbm_sha256_init(&sha); + unsigned char buf[CBM_SZ_4K]; + size_t len; + while ((len = fread(buf, 1, sizeof(buf), file)) > 0) { + cbm_sha256_update(&sha, buf, len); + } + bool valid = !ferror(file); + if (fclose(file) != 0) { + valid = false; + } + if (!valid) { + return false; + } + uint8_t digest[CBM_SHA256_DIGEST_LEN]; + cbm_sha256_final(&sha, digest); + static const char hex[] = "0123456789abcdef"; + for (int i = 0; i < CBM_SHA256_DIGEST_LEN; i++) { + out[i * 2] = hex[digest[i] >> 4]; + out[i * 2 + 1] = hex[digest[i] & 0x0f]; + } + out[CBM_SHA256_HEX_LEN] = '\0'; + return true; +} + +/* True when the recorded content hash is a well-formed SHA-256 the current + * bytes can be compared against. */ +static bool mcp_recorded_sha256(const char *sha) { + if (!sha || strlen(sha) != CBM_SHA256_HEX_LEN) { + return false; + } + for (int i = 0; i < CBM_SHA256_HEX_LEN; i++) { + if (!((sha[i] >= '0' && sha[i] <= '9') || (sha[i] >= 'a' && sha[i] <= 'f'))) { + return false; + } + } + return true; +} + +/* #2585: read a whole file into *bytes (released with cbm_sb_free), so that + * the bytes a response serves are the bytes whose hash was checked. */ +static bool mcp_read_file_bytes(const char *abs_path, cbm_sb_t *bytes) { + cbm_sb_init(bytes); + FILE *file = cbm_fopen(abs_path, "rb"); + if (!file) { + return false; + } + char buf[CBM_SZ_4K]; + size_t len; + while ((len = fread(buf, 1, sizeof(buf), file)) > 0) { + cbm_sb_append_n(bytes, buf, len); + } + bool valid = !ferror(file) && !bytes->oom; + if (fclose(file) != 0) { + valid = false; + } + if (!valid) { + cbm_sb_free(bytes); + } + return valid; +} + +/* Lines [start, end] of an in-memory file, with read_file_lines' semantics + * (text-mode line endings, NULL when the range is empty). */ +static char *mcp_slice_lines(cbm_sb_t *bytes, int start, int end) { + size_t begin = bytes->len; + size_t stop = bytes->len; + int lineno = 1; + for (size_t i = 0; i < bytes->len; i++) { + if (lineno >= start && begin == bytes->len) { + begin = i; + } + if (bytes->buf[i] == '\n') { + if (lineno >= end) { + stop = i + 1; + break; + } + lineno++; + } + } + if (begin >= stop) { + return NULL; + } + char saved = bytes->buf[stop]; + bytes->buf[stop] = '\0'; + char *slice = heap_strdup(bytes->buf + begin); + bytes->buf[stop] = saved; +#ifdef _WIN32 + /* read_file_lines opens in text mode. */ + if (slice) { + char *out = slice; + for (const char *in = slice; *in; in++) { + if (!(in[0] == '\r' && in[1] == '\n')) { + *out++ = *in; + } + } + *out = '\0'; + } +#endif + return slice; +} + +/* A path is current when its bytes were verified to be the indexed bytes, + * whatever its recorded metadata says. A metadata match whose content could not + * be verified is not: it is the verdict the hash check exists to replace. */ +static bool coverage_freshness_current(const char *freshness, bool content_verified) { + return content_verified && freshness && + (strcmp(freshness, "metadata_match") == 0 || strcmp(freshness, "content_match") == 0); +} + +/* Freshness of one tracked path. With a recorded content hash the verdict is + * by content: "metadata_match" (metadata and bytes agree), "content_changed" + * (metadata agrees, bytes differ), "content_match" (metadata is stale, bytes + * are identical) or "metadata_changed" (both differ). Without one it falls + * back to metadata alone and *content_verified stays false. + * + * When captured is non-NULL, the file is read once into it (released with + * cbm_sb_free) and the verdict is about exactly those bytes, so a caller + * serving them cannot race a concurrent write. *captured_ok stays false when + * the path never reached the file (missing, outside, not tracked, + * unreadable). */ +static const char *coverage_path_freshness_capture(cbm_store_t *store, const char *project, + const char *root_path, const char *rel_path, + bool *outside, bool *content_verified, + cbm_sb_t *captured, bool *captured_ok) { *outside = false; + *content_verified = false; + if (captured) { + cbm_sb_init(captured); + *captured_ok = false; + } if (!root_path || !root_path[0]) { return "unavailable"; } @@ -6825,8 +6988,35 @@ static const char *coverage_path_freshness(cbm_store_t *store, const char *proje return "unavailable"; } bool matches = hash.mtime_ns == info.mtime_ns && hash.size == info.size; + char current[CBM_SHA256_HEX_LEN + 1] = ""; + bool hashed = false; + if (captured) { + *captured_ok = mcp_read_file_bytes(abs_path, captured); + if (*captured_ok) { + cbm_sha256_hex(captured->buf ? captured->buf : "", captured->len, current); + hashed = true; + } + } else { + hashed = mcp_recorded_sha256(hash.sha256) && mcp_file_sha256_hex(abs_path, current); + } + bool verifiable = mcp_recorded_sha256(hash.sha256) && hashed; + bool same_bytes = verifiable && strcmp(current, hash.sha256) == 0; cbm_store_clear_file_hash(&hash); - return matches ? "metadata_match" : "metadata_changed"; + if (!verifiable) { + return matches ? "metadata_match" : "metadata_changed"; + } + *content_verified = true; + if (matches) { + return same_bytes ? "metadata_match" : "content_changed"; + } + return same_bytes ? "content_match" : "metadata_changed"; +} + +static const char *coverage_path_freshness(cbm_store_t *store, const char *project, + const char *root_path, const char *rel_path, + bool *outside, bool *content_verified) { + return coverage_path_freshness_capture(store, project, root_path, rel_path, outside, + content_verified, NULL, NULL); } /* Read an "start-end,start-end,...[,+]" string into a JSON ranges array. @@ -6971,8 +7161,9 @@ static const char *coverage_status(const cbm_coverage_row_t *rows, int count, return "no_recorded_issue"; } -static const char *coverage_recommended_action(const char *status, const char *freshness) { - if (!freshness || strcmp(freshness, "metadata_match") != 0) { +static const char *coverage_recommended_action(const char *status, const char *freshness, + bool content_verified) { + if (!coverage_freshness_current(freshness, content_verified)) { return "read_source_and_reindex"; } if (strcmp(status, "partial") == 0) { @@ -7064,6 +7255,7 @@ static char *handle_check_index_coverage(cbm_mcp_server_t *srv, const char *args yyjson_mut_obj_add_int(doc, meta_obj, "coverage_version", have_meta ? meta.coverage_version : 0); yyjson_mut_obj_add_bool(doc, meta_obj, "generation_matches", generation_matches); + mcp_add_store_generation(doc, meta_obj, store); yyjson_mut_obj_add_val(doc, root, "metadata", meta_obj); yyjson_mut_val *path_results = yyjson_mut_arr(doc); @@ -7103,18 +7295,22 @@ static char *handle_check_index_coverage(cbm_mcp_server_t *srv, const char *args yyjson_mut_obj_add_str(doc, item, "coverage_lookup", "error"); } bool outside = false; - const char *freshness = coverage_path_freshness( - store, project, have_project ? proj.root_path : NULL, rel, &outside); - bool exact_path_verified = - have_meta && meta.hash_records_complete && strcmp(freshness, "metadata_match") == 0; + bool content_verified = false; + const char *freshness = + coverage_path_freshness(store, project, have_project ? proj.root_path : NULL, rel, + &outside, &content_verified); + bool exact_path_verified = have_meta && meta.hash_records_complete && + coverage_freshness_current(freshness, content_verified); const char *status = outside ? "outside_project" : coverage_status(rows, row_count, rel, recording_status, generation_matches, lookup_ok, exact_path_verified); yyjson_mut_obj_add_strcpy(doc, item, "status", status); yyjson_mut_obj_add_strcpy(doc, item, "freshness", freshness); - yyjson_mut_obj_add_strcpy(doc, item, "recommended_action", - coverage_recommended_action(status, freshness)); + yyjson_mut_obj_add_bool(doc, item, "content_verified", content_verified); + yyjson_mut_obj_add_strcpy( + doc, item, "recommended_action", + coverage_recommended_action(status, freshness, content_verified)); yyjson_mut_val *coverage = yyjson_mut_arr(doc); for (int i = 0; i < row_count; i++) { coverage_add_row_json(doc, coverage, &rows[i], rel, diagnostics_full); @@ -12612,8 +12808,10 @@ static char *snippet_pdf_text(const cbm_node_t *node) { return out; } +/* #2585: when captured is non-NULL the lines come from those already-read + * bytes instead of a fresh read of the file. */ static char *resolve_snippet_source(const char *root_path, const char *file_path, int start, - int end, char **out_abs_path) { + int end, cbm_sb_t *captured, char **out_abs_path) { *out_abs_path = NULL; if (!root_path || !file_path) { return NULL; @@ -12624,7 +12822,8 @@ static char *resolve_snippet_source(const char *root_path, const char *file_path *out_abs_path = abs_path; if (cbm_path_within_root(root_path, abs_path)) { - return read_file_lines(abs_path, start, end); + return captured ? mcp_slice_lines(captured, start, end) + : read_file_lines(abs_path, start, end); } return NULL; } @@ -13109,15 +13308,44 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node, if (!outline && end < original_end) { snippet_clipped = true; } + /* #2585: start/end are indexed line numbers. Slice the file only when its + * bytes are the indexed bytes; otherwise the same range can hold another + * symbol's code. The file is read once: the slice comes from the same + * bytes whose hash was checked, so a concurrent write cannot slip in + * between. Unverifiable files keep the previous behavior and say so + * through content_verified. */ + const char *source_freshness = NULL; + bool source_content_verified = false; + bool source_drift = false; + cbm_sb_t captured; + cbm_sb_init(&captured); + bool captured_ok = false; + if (!outline && !snippet_is_pdf(node->file_path) && root_path && node->file_path) { + bool outside = false; + source_freshness = coverage_path_freshness_capture( + srv->store, node->project, root_path, node->file_path, &outside, + &source_content_verified, &captured, &captured_ok); + source_drift = source_content_verified && + !coverage_freshness_current(source_freshness, source_content_verified); +#ifdef CBM_ENABLE_TEST_SEAMS + if (srv->snippet_capture_test_hook) { + srv->snippet_capture_test_hook(srv->snippet_capture_test_context, node->file_path); + } +#endif + } char *abs_path = NULL; char *source = NULL; - if (outline) { + if (outline || source_drift) { source = NULL; } else if (snippet_is_pdf(node->file_path)) { source = snippet_pdf_text(node); /* never the file's bytes */ } else { - source = resolve_snippet_source(root_path, node->file_path, start, end, &abs_path); + /* Without captured bytes (not tracked, unreadable) the file is read + * here, and the snippet is never content-verified. */ + source = resolve_snippet_source(root_path, node->file_path, start, end, + captured_ok ? &captured : NULL, &abs_path); } + cbm_sb_free(&captured); yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); yyjson_mut_val *root_obj = yyjson_mut_obj(doc); @@ -13137,6 +13365,13 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node, yyjson_mut_obj_add_str(doc, root_obj, "file_path", display_path); yyjson_mut_obj_add_int(doc, root_obj, "start_line", start); yyjson_mut_obj_add_int(doc, root_obj, "end_line", end); + mcp_add_store_generation(doc, root_obj, srv->store); + yyjson_mut_obj_add_bool(doc, root_obj, "content_verified", source_content_verified); + if (source_drift) { + yyjson_mut_obj_add_bool(doc, root_obj, "source_drift", true); + yyjson_mut_obj_add_strcpy(doc, root_obj, "freshness", source_freshness); + yyjson_mut_obj_add_str(doc, root_obj, "recommended_action", "reindex_then_retry"); + } if (snippet_clipped) { yyjson_mut_obj_add_bool(doc, root_obj, "source_truncated", true); yyjson_mut_obj_add_bool(doc, root_obj, "source_clipped", true); /* compatibility */ diff --git a/src/mcp/mcp_internal.h b/src/mcp/mcp_internal.h index ead1fca389..afbe380e5b 100644 --- a/src/mcp/mcp_internal.h +++ b/src/mcp/mcp_internal.h @@ -102,6 +102,13 @@ void cbm_mcp_server_set_snapshot_read_test_hook(cbm_mcp_server_t *srv, cbm_mcp_snapshot_read_test_hook_fn hook, void *context); +/* Runs after get_code_snippet has read and verified a file's bytes and before + * it slices them, so a test can write the file at exactly that point (#2585). */ +typedef void (*cbm_mcp_snippet_capture_test_hook_fn)(void *context, const char *rel_path); +void cbm_mcp_server_set_snippet_capture_test_hook(cbm_mcp_server_t *srv, + cbm_mcp_snippet_capture_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. */ diff --git a/tests/test_mcp.c b/tests/test_mcp.c index abd19c1242..55b9dd1634 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -10,6 +10,7 @@ #include "../src/foundation/log.h" #include "../src/foundation/platform.h" /* cbm_file_size */ #include "../src/foundation/mem.h" /* cbm_mem_set_budget_for_tests — over-budget seam */ +#include "../src/foundation/sha256.h" #include "../src/foundation/str_util.h" #include "../src/foundation/subprocess.h" #include "../src/foundation/workspace.h" @@ -6341,7 +6342,16 @@ TEST(tool_check_index_coverage_accepts_truncated_ignored_catalog_for_fresh_path_ * metadata_match contract below would fail. */ cbm_path_info_t path_info; ASSERT_EQ(cbm_path_info_utf8(source_path, &path_info), 0); - ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", path_info.mtime_ns, + /* The real content hash, as the indexer records it: a path counts as + * current only when its bytes are verified (#2585). */ + char content[4096]; + FILE *source = fopen(source_path, "rb"); + ASSERT_NOT_NULL(source); + size_t content_len = fread(content, 1, sizeof(content), source); + fclose(source); + char sha[CBM_SHA256_HEX_LEN + 1]; + cbm_sha256_hex(content, content_len, sha); + ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", sha, path_info.mtime_ns, path_info.size), CBM_STORE_OK); cbm_project_t project = {0}; @@ -6497,6 +6507,286 @@ TEST(tool_check_index_coverage_requires_source_when_file_metadata_changed) { PASS(); } +/* #2585 fixtures: record main.go's hash row with its real size and either its + * real or a shifted mtime, and either its real content hash or another + * file's. This reaches each freshness state without touching file times, which + * some platforms cannot set precisely. */ +static int record_main_go_hash(cbm_store_t *store, const char *tmp, bool real_content, + bool real_mtime) { + char path[512]; + snprintf(path, sizeof(path), "%s/project/main.go", tmp); + cbm_path_info_t info; + if (cbm_path_info_utf8(path, &info) != 0) { + return CBM_STORE_ERR; + } + FILE *fp = fopen(path, "rb"); + if (!fp) { + return CBM_STORE_ERR; + } + char bytes[4096]; + size_t len = fread(bytes, 1, sizeof(bytes), fp); + fclose(fp); + char sha[CBM_SHA256_HEX_LEN + 1]; + if (real_content) { + cbm_sha256_hex(bytes, len, sha); + } else { + cbm_sha256_hex("other bytes", strlen("other bytes"), sha); + } + int64_t mtime_ns = real_mtime ? info.mtime_ns : info.mtime_ns - (int64_t)CBM_NSEC_PER_SEC; + return cbm_store_upsert_file_hash(store, "test-project", "main.go", sha, mtime_ns, info.size); +} + +static yyjson_val *coverage_first_path(cbm_mcp_server_t *srv, yyjson_doc **doc, char **inner) { + char *response = cbm_mcp_handle_tool(srv, "check_index_coverage", + "{\"project\":\"test-project\",\"paths\":[\"main.go\"]," + "\"format\":\"json\"}"); + *inner = response ? extract_text_content(response) : NULL; + free(response); + *doc = *inner ? yyjson_read(*inner, strlen(*inner), 0) : NULL; + return *doc ? yyjson_arr_get(yyjson_obj_get(yyjson_doc_get_root(*doc), "paths"), 0) : NULL; +} + +/* #2585: freshness judged by mtime and size passed a same-size edit whose + * timestamp was preserved, and stayed stale after identical bytes were + * rewritten. With a recorded content hash it now reports the bytes. */ +TEST(tool_check_index_coverage_freshness_verifies_content_issue2585) { + char tmp[256]; + cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp)); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + cbm_project_t project = {0}; + ASSERT_EQ(cbm_store_get_project(store, "test-project", &project), CBM_STORE_OK); + ASSERT_EQ(write_coverage_meta(store, project.indexed_at, "complete"), CBM_STORE_OK); + cbm_project_free_fields(&project); + + static const struct { + bool real_content; + bool real_mtime; + const char *freshness; + const char *action; + } cases[] = { + /* Same metadata, different bytes: previously metadata_match. */ + {false, true, "content_changed", "read_source_and_reindex"}, + {true, true, "metadata_match", "use_graph_with_best_effort_caveat"}, + /* Stale metadata, identical bytes: previously metadata_changed. */ + {true, false, "content_match", "use_graph_with_best_effort_caveat"}, + {false, false, "metadata_changed", "read_source_and_reindex"}, + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + ASSERT_EQ(record_main_go_hash(store, tmp, cases[i].real_content, cases[i].real_mtime), + CBM_STORE_OK); + yyjson_doc *doc = NULL; + char *inner = NULL; + yyjson_val *path = coverage_first_path(srv, &doc, &inner); + ASSERT_NOT_NULL(path); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(path, "freshness")), cases[i].freshness); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(path, "recommended_action")), cases[i].action); + ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(path, "content_verified"))); + yyjson_doc_free(doc); + free(inner); + } + + /* A row without a usable content hash keeps the metadata verdict and says + * the content was not verified. */ + ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "fixture", 0, 0), + CBM_STORE_OK); + yyjson_doc *doc = NULL; + char *inner = NULL; + yyjson_val *path = coverage_first_path(srv, &doc, &inner); + ASSERT_NOT_NULL(path); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(path, "freshness")), "metadata_changed"); + ASSERT_FALSE(yyjson_get_bool(yyjson_obj_get(path, "content_verified"))); + yyjson_doc_free(doc); + free(inner); + + /* Matching metadata alone is not evidence the bytes are current: without + * a usable hash, the path must not be recommended as if it were. */ + char main_go[512]; + snprintf(main_go, sizeof(main_go), "%s/project/main.go", tmp); + cbm_path_info_t info; + ASSERT_EQ(cbm_path_info_utf8(main_go, &info), 0); + ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "fixture", info.mtime_ns, + info.size), + CBM_STORE_OK); + path = coverage_first_path(srv, &doc, &inner); + ASSERT_NOT_NULL(path); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(path, "freshness")), "metadata_match"); + ASSERT_FALSE(yyjson_get_bool(yyjson_obj_get(path, "content_verified"))); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(path, "recommended_action")), + "read_source_and_reindex"); + yyjson_doc_free(doc); + free(inner); + + cbm_mcp_server_free(srv); + cleanup_snippet_dir(tmp); + PASS(); +} + +static char *snippet_process_order(cbm_mcp_server_t *srv, yyjson_doc **doc) { + char *response = cbm_mcp_handle_tool( + srv, "get_code_snippet", + "{\"project\":\"test-project\"," + "\"qualified_name\":\"test-project.cmd.server.main.ProcessOrder\",\"format\":\"json\"}"); + char *inner = response ? extract_text_content(response) : NULL; + free(response); + *doc = inner ? yyjson_read(inner, strlen(inner), 0) : NULL; + return inner; +} + +/* #2585 (and #1750): the snippet's line range is the indexed range. When the + * file's bytes are not the indexed bytes, the same range can hold another + * symbol's code, so the source is withheld rather than misattributed. */ +TEST(tool_get_code_snippet_withholds_source_whose_content_drifted_issue2585) { + char tmp[256]; + cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp)); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + + /* Same metadata, different bytes: mtime/size cannot see this, and the + * indexed range would be sliced from bytes that were never indexed. */ + ASSERT_EQ(record_main_go_hash(store, tmp, false, true), CBM_STORE_OK); + yyjson_doc *doc = NULL; + char *inner = snippet_process_order(srv, &doc); + ASSERT_NOT_NULL(doc); + yyjson_val *root = yyjson_doc_get_root(doc); + ASSERT_NULL(strstr(yyjson_get_str(yyjson_obj_get(root, "source")), "func")); + ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(root, "source_drift"))); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(root, "freshness")), "content_changed"); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(root, "recommended_action")), "reindex_then_retry"); + ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(root, "content_verified"))); + yyjson_doc_free(doc); + free(inner); + + /* The indexed bytes, with current or stale metadata, are served. */ + for (int real_mtime = 1; real_mtime >= 0; real_mtime--) { + ASSERT_EQ(record_main_go_hash(store, tmp, true, real_mtime == 1), CBM_STORE_OK); + inner = snippet_process_order(srv, &doc); + ASSERT_NOT_NULL(doc); + root = yyjson_doc_get_root(doc); + ASSERT_NOT_NULL( + strstr(yyjson_get_str(yyjson_obj_get(root, "source")), "func ProcessOrder")); + ASSERT_NULL(yyjson_obj_get(root, "source_drift")); + ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(root, "content_verified"))); + yyjson_doc_free(doc); + free(inner); + } + + /* Without a usable hash the previous behavior holds, marked unverified. */ + ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", 0, 0), CBM_STORE_OK); + inner = snippet_process_order(srv, &doc); + ASSERT_NOT_NULL(doc); + root = yyjson_doc_get_root(doc); + ASSERT_NOT_NULL(strstr(yyjson_get_str(yyjson_obj_get(root, "source")), "func ProcessOrder")); + ASSERT_FALSE(yyjson_get_bool(yyjson_obj_get(root, "content_verified"))); + yyjson_doc_free(doc); + free(inner); + + cbm_mcp_server_free(srv); + cleanup_snippet_dir(tmp); + PASS(); +} + +#ifdef CBM_ENABLE_TEST_SEAMS +/* Rewrites main.go so that the indexed ProcessOrder range holds other code. */ +static void snippet_capture_rewrite(void *context, const char *rel_path) { + (void)rel_path; + FILE *fp = fopen((const char *)context, "w"); + if (fp) { + fputs("package main\n\n// a\n// b\n// c\n// d\nfunc Intruder() {\n}\n", fp); + fclose(fp); + } +} + +/* #2585: the bytes get_code_snippet serves must be the bytes it verified. A + * write landing after the hash check and before the slice must not produce a + * content_verified snippet holding code that was never indexed. */ +TEST(tool_get_code_snippet_serves_the_bytes_it_verified_issue2585) { + char tmp[256]; + cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp)); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + ASSERT_EQ(record_main_go_hash(store, tmp, true, true), CBM_STORE_OK); + char main_go[512]; + snprintf(main_go, sizeof(main_go), "%s/project/main.go", tmp); + + cbm_mcp_server_set_snippet_capture_test_hook(srv, snippet_capture_rewrite, main_go); + yyjson_doc *doc = NULL; + char *inner = snippet_process_order(srv, &doc); + cbm_mcp_server_set_snippet_capture_test_hook(srv, NULL, NULL); + ASSERT_NOT_NULL(doc); + yyjson_val *root = yyjson_doc_get_root(doc); + const char *source = yyjson_get_str(yyjson_obj_get(root, "source")); + ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(root, "content_verified"))); + ASSERT_NULL(strstr(source, "Intruder")); + ASSERT_NOT_NULL(strstr(source, "func ProcessOrder")); + yyjson_doc_free(doc); + free(inner); + + /* The write did land: the next call sees it and withholds the source. */ + inner = snippet_process_order(srv, &doc); + ASSERT_NOT_NULL(doc); + root = yyjson_doc_get_root(doc); + ASSERT_TRUE(yyjson_get_bool(yyjson_obj_get(root, "source_drift"))); + yyjson_doc_free(doc); + free(inner); + + cbm_mcp_server_free(srv); + cleanup_snippet_dir(tmp); + PASS(); +} +#endif + +/* #2585: metadata.generation names the coverage-recording run and does not + * change on incremental re-index, so it cannot tell two publications apart. + * store_generation does, and every response that carries it agrees. */ +TEST(tool_responses_carry_store_generation_issue2585) { + char tmp[256]; + cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp)); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + ASSERT_EQ(write_coverage_meta(store, "coverage-run-1", "complete"), CBM_STORE_OK); + ASSERT_EQ(record_main_go_hash(store, tmp, true, true), CBM_STORE_OK); + + char before[96]; + ASSERT_EQ(cbm_store_generation(store, before, sizeof(before)), CBM_STORE_OK); + yyjson_doc *cdoc = NULL; + char *cinner = NULL; + ASSERT_NOT_NULL(coverage_first_path(srv, &cdoc, &cinner)); + yyjson_val *meta = yyjson_obj_get(yyjson_doc_get_root(cdoc), "metadata"); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(meta, "store_generation")), before); + yyjson_doc *sdoc = NULL; + char *sinner = snippet_process_order(srv, &sdoc); + ASSERT_NOT_NULL(sdoc); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(yyjson_doc_get_root(sdoc), "store_generation")), + before); + yyjson_doc_free(cdoc); + free(cinner); + yyjson_doc_free(sdoc); + free(sinner); + + /* An index run advances the store generation, not the coverage one. */ + char project_dir[512]; + snprintf(project_dir, sizeof(project_dir), "%s/project", tmp); + ASSERT_EQ(cbm_store_upsert_project(store, "test-project", project_dir), CBM_STORE_OK); + char after[96]; + ASSERT_EQ(cbm_store_generation(store, after, sizeof(after)), CBM_STORE_OK); + ASSERT_STR_NEQ(after, before); + ASSERT_NOT_NULL(coverage_first_path(srv, &cdoc, &cinner)); + meta = yyjson_obj_get(yyjson_doc_get_root(cdoc), "metadata"); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(meta, "store_generation")), after); + ASSERT_STR_EQ(yyjson_get_str(yyjson_obj_get(meta, "generation")), "coverage-run-1"); + yyjson_doc_free(cdoc); + free(cinner); + + cbm_mcp_server_free(srv); + cleanup_snippet_dir(tmp); + PASS(); +} + TEST(tool_check_index_coverage_surfaces_lookup_errors) { char tmp[256]; cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp)); @@ -23929,6 +24219,12 @@ SUITE(mcp) { RUN_TEST(tool_check_index_coverage_rejects_stale_generation); RUN_TEST(tool_check_index_coverage_freshness_uses_indexer_mtime_source_issue1714); RUN_TEST(tool_check_index_coverage_requires_source_when_file_metadata_changed); + RUN_TEST(tool_check_index_coverage_freshness_verifies_content_issue2585); + RUN_TEST(tool_get_code_snippet_withholds_source_whose_content_drifted_issue2585); +#ifdef CBM_ENABLE_TEST_SEAMS + RUN_TEST(tool_get_code_snippet_serves_the_bytes_it_verified_issue2585); +#endif + RUN_TEST(tool_responses_carry_store_generation_issue2585); RUN_TEST(tool_check_index_coverage_surfaces_lookup_errors); RUN_TEST(tool_index_status_includes_git_metadata);