Skip to content

Commit 2658a18

Browse files
authored
Merge pull request #2600 from bline/fix/snapshot-coherence
fix(mcp): verify content for freshness and snippets; expose store generation (#2585)
2 parents beddaaa + 95279bd commit 2658a18

3 files changed

Lines changed: 555 additions & 17 deletions

File tree

‎src/mcp/mcp.c‎

Lines changed: 251 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1902,6 +1902,8 @@ struct cbm_mcp_server {
19021902
void *auto_index_count_test_context;
19031903
cbm_mcp_snapshot_read_test_hook_fn snapshot_read_test_hook;
19041904
void *snapshot_read_test_context;
1905+
cbm_mcp_snippet_capture_test_hook_fn snippet_capture_test_hook;
1906+
void *snippet_capture_test_context;
19051907
#endif
19061908
bool read_batch_tool; /* this request's store reads run in a read batch */
19071909
size_t search_output_limit_override;
@@ -2272,6 +2274,16 @@ void cbm_mcp_server_set_snapshot_read_test_hook(cbm_mcp_server_t *srv,
22722274
srv->snapshot_read_test_hook = hook;
22732275
srv->snapshot_read_test_context = context;
22742276
}
2277+
2278+
void cbm_mcp_server_set_snippet_capture_test_hook(cbm_mcp_server_t *srv,
2279+
cbm_mcp_snippet_capture_test_hook_fn hook,
2280+
void *context) {
2281+
if (!srv) {
2282+
return;
2283+
}
2284+
srv->snippet_capture_test_hook = hook;
2285+
srv->snippet_capture_test_context = context;
2286+
}
22752287
#endif
22762288

22772289
/* ── Cache dir + project DB path helpers ───────────────────────── */
@@ -7063,10 +7075,161 @@ static coverage_path_result_t coverage_normalize_rel(const char *input, bool all
70637075
return written > 0U || allow_root ? COVERAGE_PATH_OK : COVERAGE_PATH_INVALID;
70647076
}
70657077

7066-
static const char *coverage_path_freshness(cbm_store_t *store, const char *project,
7067-
const char *root_path, const char *rel_path,
7068-
bool *outside) {
7078+
/* #2585: the store generation ("u<db_uid>g<mutation_gen>") advances on every
7079+
* index run, incremental ones included, and on every publication. It is read
7080+
* when the response is built and names the publication current then; it does
7081+
* not make the response one atomic read of that publication.
7082+
* metadata.generation names the coverage-recording run and stays fixed across
7083+
* incremental re-indexes. Empty when unreadable. */
7084+
static void mcp_add_store_generation(yyjson_mut_doc *doc, yyjson_mut_val *obj, cbm_store_t *store) {
7085+
char generation[QUERY_GRAPH_GENERATION_CAP] = "";
7086+
if (!store || cbm_store_generation(store, generation, sizeof(generation)) != CBM_STORE_OK) {
7087+
generation[0] = '\0';
7088+
}
7089+
yyjson_mut_obj_add_strcpy(doc, obj, "store_generation", generation);
7090+
}
7091+
7092+
/* #2585: file_hashes.sha256 is the SHA-256 of the raw bytes the indexer read
7093+
* (pipeline_incremental.c semantic_manifest_hash_file). Recomputing it is the
7094+
* only way to say the bytes on disk are the bytes that were indexed: mtime and
7095+
* size miss a same-size edit whose timestamp was preserved (cp -p, rsync -t,
7096+
* archive extraction), and go stale when identical bytes are rewritten. */
7097+
static bool mcp_file_sha256_hex(const char *abs_path, char out[CBM_SHA256_HEX_LEN + 1]) {
7098+
FILE *file = cbm_fopen(abs_path, "rb");
7099+
if (!file) {
7100+
return false;
7101+
}
7102+
cbm_sha256_ctx sha;
7103+
cbm_sha256_init(&sha);
7104+
unsigned char buf[CBM_SZ_4K];
7105+
size_t len;
7106+
while ((len = fread(buf, 1, sizeof(buf), file)) > 0) {
7107+
cbm_sha256_update(&sha, buf, len);
7108+
}
7109+
bool valid = !ferror(file);
7110+
if (fclose(file) != 0) {
7111+
valid = false;
7112+
}
7113+
if (!valid) {
7114+
return false;
7115+
}
7116+
uint8_t digest[CBM_SHA256_DIGEST_LEN];
7117+
cbm_sha256_final(&sha, digest);
7118+
static const char hex[] = "0123456789abcdef";
7119+
for (int i = 0; i < CBM_SHA256_DIGEST_LEN; i++) {
7120+
out[i * 2] = hex[digest[i] >> 4];
7121+
out[i * 2 + 1] = hex[digest[i] & 0x0f];
7122+
}
7123+
out[CBM_SHA256_HEX_LEN] = '\0';
7124+
return true;
7125+
}
7126+
7127+
/* True when the recorded content hash is a well-formed SHA-256 the current
7128+
* bytes can be compared against. */
7129+
static bool mcp_recorded_sha256(const char *sha) {
7130+
if (!sha || strlen(sha) != CBM_SHA256_HEX_LEN) {
7131+
return false;
7132+
}
7133+
for (int i = 0; i < CBM_SHA256_HEX_LEN; i++) {
7134+
if (!((sha[i] >= '0' && sha[i] <= '9') || (sha[i] >= 'a' && sha[i] <= 'f'))) {
7135+
return false;
7136+
}
7137+
}
7138+
return true;
7139+
}
7140+
7141+
/* #2585: read a whole file into *bytes (released with cbm_sb_free), so that
7142+
* the bytes a response serves are the bytes whose hash was checked. */
7143+
static bool mcp_read_file_bytes(const char *abs_path, cbm_sb_t *bytes) {
7144+
cbm_sb_init(bytes);
7145+
FILE *file = cbm_fopen(abs_path, "rb");
7146+
if (!file) {
7147+
return false;
7148+
}
7149+
char buf[CBM_SZ_4K];
7150+
size_t len;
7151+
while ((len = fread(buf, 1, sizeof(buf), file)) > 0) {
7152+
cbm_sb_append_n(bytes, buf, len);
7153+
}
7154+
bool valid = !ferror(file) && !bytes->oom;
7155+
if (fclose(file) != 0) {
7156+
valid = false;
7157+
}
7158+
if (!valid) {
7159+
cbm_sb_free(bytes);
7160+
}
7161+
return valid;
7162+
}
7163+
7164+
/* Lines [start, end] of an in-memory file, with read_file_lines' semantics
7165+
* (text-mode line endings, NULL when the range is empty). */
7166+
static char *mcp_slice_lines(cbm_sb_t *bytes, int start, int end) {
7167+
size_t begin = bytes->len;
7168+
size_t stop = bytes->len;
7169+
int lineno = 1;
7170+
for (size_t i = 0; i < bytes->len; i++) {
7171+
if (lineno >= start && begin == bytes->len) {
7172+
begin = i;
7173+
}
7174+
if (bytes->buf[i] == '\n') {
7175+
if (lineno >= end) {
7176+
stop = i + 1;
7177+
break;
7178+
}
7179+
lineno++;
7180+
}
7181+
}
7182+
if (begin >= stop) {
7183+
return NULL;
7184+
}
7185+
char saved = bytes->buf[stop];
7186+
bytes->buf[stop] = '\0';
7187+
char *slice = heap_strdup(bytes->buf + begin);
7188+
bytes->buf[stop] = saved;
7189+
#ifdef _WIN32
7190+
/* read_file_lines opens in text mode. */
7191+
if (slice) {
7192+
char *out = slice;
7193+
for (const char *in = slice; *in; in++) {
7194+
if (!(in[0] == '\r' && in[1] == '\n')) {
7195+
*out++ = *in;
7196+
}
7197+
}
7198+
*out = '\0';
7199+
}
7200+
#endif
7201+
return slice;
7202+
}
7203+
7204+
/* A path is current when its bytes were verified to be the indexed bytes,
7205+
* whatever its recorded metadata says. A metadata match whose content could not
7206+
* be verified is not: it is the verdict the hash check exists to replace. */
7207+
static bool coverage_freshness_current(const char *freshness, bool content_verified) {
7208+
return content_verified && freshness &&
7209+
(strcmp(freshness, "metadata_match") == 0 || strcmp(freshness, "content_match") == 0);
7210+
}
7211+
7212+
/* Freshness of one tracked path. With a recorded content hash the verdict is
7213+
* by content: "metadata_match" (metadata and bytes agree), "content_changed"
7214+
* (metadata agrees, bytes differ), "content_match" (metadata is stale, bytes
7215+
* are identical) or "metadata_changed" (both differ). Without one it falls
7216+
* back to metadata alone and *content_verified stays false.
7217+
*
7218+
* When captured is non-NULL, the file is read once into it (released with
7219+
* cbm_sb_free) and the verdict is about exactly those bytes, so a caller
7220+
* serving them cannot race a concurrent write. *captured_ok stays false when
7221+
* the path never reached the file (missing, outside, not tracked,
7222+
* unreadable). */
7223+
static const char *coverage_path_freshness_capture(cbm_store_t *store, const char *project,
7224+
const char *root_path, const char *rel_path,
7225+
bool *outside, bool *content_verified,
7226+
cbm_sb_t *captured, bool *captured_ok) {
70697227
*outside = false;
7228+
*content_verified = false;
7229+
if (captured) {
7230+
cbm_sb_init(captured);
7231+
*captured_ok = false;
7232+
}
70707233
if (!root_path || !root_path[0]) {
70717234
return "unavailable";
70727235
}
@@ -7099,8 +7262,35 @@ static const char *coverage_path_freshness(cbm_store_t *store, const char *proje
70997262
return "unavailable";
71007263
}
71017264
bool matches = hash.mtime_ns == info.mtime_ns && hash.size == info.size;
7265+
char current[CBM_SHA256_HEX_LEN + 1] = "";
7266+
bool hashed = false;
7267+
if (captured) {
7268+
*captured_ok = mcp_read_file_bytes(abs_path, captured);
7269+
if (*captured_ok) {
7270+
cbm_sha256_hex(captured->buf ? captured->buf : "", captured->len, current);
7271+
hashed = true;
7272+
}
7273+
} else {
7274+
hashed = mcp_recorded_sha256(hash.sha256) && mcp_file_sha256_hex(abs_path, current);
7275+
}
7276+
bool verifiable = mcp_recorded_sha256(hash.sha256) && hashed;
7277+
bool same_bytes = verifiable && strcmp(current, hash.sha256) == 0;
71027278
cbm_store_clear_file_hash(&hash);
7103-
return matches ? "metadata_match" : "metadata_changed";
7279+
if (!verifiable) {
7280+
return matches ? "metadata_match" : "metadata_changed";
7281+
}
7282+
*content_verified = true;
7283+
if (matches) {
7284+
return same_bytes ? "metadata_match" : "content_changed";
7285+
}
7286+
return same_bytes ? "content_match" : "metadata_changed";
7287+
}
7288+
7289+
static const char *coverage_path_freshness(cbm_store_t *store, const char *project,
7290+
const char *root_path, const char *rel_path,
7291+
bool *outside, bool *content_verified) {
7292+
return coverage_path_freshness_capture(store, project, root_path, rel_path, outside,
7293+
content_verified, NULL, NULL);
71047294
}
71057295

71067296
/* Read an "start-end,start-end,...[,+<N>]" string into a JSON ranges array.
@@ -7270,8 +7460,9 @@ static const char *coverage_status(const cbm_coverage_row_t *rows, int count,
72707460
return "no_recorded_issue";
72717461
}
72727462

7273-
static const char *coverage_recommended_action(const char *status, const char *freshness) {
7274-
if (!freshness || strcmp(freshness, "metadata_match") != 0) {
7463+
static const char *coverage_recommended_action(const char *status, const char *freshness,
7464+
bool content_verified) {
7465+
if (!coverage_freshness_current(freshness, content_verified)) {
72757466
return "read_source_and_reindex";
72767467
}
72777468
if (strcmp(status, "partial") == 0) {
@@ -7368,6 +7559,7 @@ static char *handle_check_index_coverage(cbm_mcp_server_t *srv, const char *args
73687559
yyjson_mut_obj_add_int(doc, meta_obj, "coverage_version",
73697560
have_meta ? meta.coverage_version : 0);
73707561
yyjson_mut_obj_add_bool(doc, meta_obj, "generation_matches", generation_matches);
7562+
mcp_add_store_generation(doc, meta_obj, store);
73717563
yyjson_mut_obj_add_val(doc, root, "metadata", meta_obj);
73727564

73737565
yyjson_mut_val *path_results = yyjson_mut_arr(doc);
@@ -7407,18 +7599,22 @@ static char *handle_check_index_coverage(cbm_mcp_server_t *srv, const char *args
74077599
yyjson_mut_obj_add_str(doc, item, "coverage_lookup", "error");
74087600
}
74097601
bool outside = false;
7410-
const char *freshness = coverage_path_freshness(
7411-
store, project, have_project ? proj.root_path : NULL, rel, &outside);
7412-
bool exact_path_verified =
7413-
have_meta && meta.hash_records_complete && strcmp(freshness, "metadata_match") == 0;
7602+
bool content_verified = false;
7603+
const char *freshness =
7604+
coverage_path_freshness(store, project, have_project ? proj.root_path : NULL, rel,
7605+
&outside, &content_verified);
7606+
bool exact_path_verified = have_meta && meta.hash_records_complete &&
7607+
coverage_freshness_current(freshness, content_verified);
74147608
const char *status =
74157609
outside ? "outside_project"
74167610
: coverage_status(rows, row_count, rel, recording_status,
74177611
generation_matches, lookup_ok, exact_path_verified);
74187612
yyjson_mut_obj_add_strcpy(doc, item, "status", status);
74197613
yyjson_mut_obj_add_strcpy(doc, item, "freshness", freshness);
7420-
yyjson_mut_obj_add_strcpy(doc, item, "recommended_action",
7421-
coverage_recommended_action(status, freshness));
7614+
yyjson_mut_obj_add_bool(doc, item, "content_verified", content_verified);
7615+
yyjson_mut_obj_add_strcpy(
7616+
doc, item, "recommended_action",
7617+
coverage_recommended_action(status, freshness, content_verified));
74227618
yyjson_mut_val *coverage = yyjson_mut_arr(doc);
74237619
for (int i = 0; i < row_count; i++) {
74247620
coverage_add_row_json(doc, coverage, &rows[i], rel, diagnostics_full);
@@ -13327,8 +13523,10 @@ static char *snippet_pdf_text(const cbm_node_t *node) {
1332713523
return out;
1332813524
}
1332913525

13526+
/* #2585: when captured is non-NULL the lines come from those already-read
13527+
* bytes instead of a fresh read of the file. */
1333013528
static char *resolve_snippet_source(const char *root_path, const char *file_path, int start,
13331-
int end, char **out_abs_path) {
13529+
int end, cbm_sb_t *captured, char **out_abs_path) {
1333213530
*out_abs_path = NULL;
1333313531
if (!root_path || !file_path) {
1333413532
return NULL;
@@ -13339,7 +13537,8 @@ static char *resolve_snippet_source(const char *root_path, const char *file_path
1333913537

1334013538
*out_abs_path = abs_path;
1334113539
if (cbm_path_within_root(root_path, abs_path)) {
13342-
return read_file_lines(abs_path, start, end);
13540+
return captured ? mcp_slice_lines(captured, start, end)
13541+
: read_file_lines(abs_path, start, end);
1334313542
}
1334413543
return NULL;
1334513544
}
@@ -13824,15 +14023,44 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node,
1382414023
if (!outline && end < original_end) {
1382514024
snippet_clipped = true;
1382614025
}
14026+
/* #2585: start/end are indexed line numbers. Slice the file only when its
14027+
* bytes are the indexed bytes; otherwise the same range can hold another
14028+
* symbol's code. The file is read once: the slice comes from the same
14029+
* bytes whose hash was checked, so a concurrent write cannot slip in
14030+
* between. Unverifiable files keep the previous behavior and say so
14031+
* through content_verified. */
14032+
const char *source_freshness = NULL;
14033+
bool source_content_verified = false;
14034+
bool source_drift = false;
14035+
cbm_sb_t captured;
14036+
cbm_sb_init(&captured);
14037+
bool captured_ok = false;
14038+
if (!outline && !snippet_is_pdf(node->file_path) && root_path && node->file_path) {
14039+
bool outside = false;
14040+
source_freshness = coverage_path_freshness_capture(
14041+
srv->store, node->project, root_path, node->file_path, &outside,
14042+
&source_content_verified, &captured, &captured_ok);
14043+
source_drift = source_content_verified &&
14044+
!coverage_freshness_current(source_freshness, source_content_verified);
14045+
#ifdef CBM_ENABLE_TEST_SEAMS
14046+
if (srv->snippet_capture_test_hook) {
14047+
srv->snippet_capture_test_hook(srv->snippet_capture_test_context, node->file_path);
14048+
}
14049+
#endif
14050+
}
1382714051
char *abs_path = NULL;
1382814052
char *source = NULL;
13829-
if (outline) {
14053+
if (outline || source_drift) {
1383014054
source = NULL;
1383114055
} else if (snippet_is_pdf(node->file_path)) {
1383214056
source = snippet_pdf_text(node); /* never the file's bytes */
1383314057
} else {
13834-
source = resolve_snippet_source(root_path, node->file_path, start, end, &abs_path);
14058+
/* Without captured bytes (not tracked, unreadable) the file is read
14059+
* here, and the snippet is never content-verified. */
14060+
source = resolve_snippet_source(root_path, node->file_path, start, end,
14061+
captured_ok ? &captured : NULL, &abs_path);
1383514062
}
14063+
cbm_sb_free(&captured);
1383614064

1383714065
yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL);
1383814066
yyjson_mut_val *root_obj = yyjson_mut_obj(doc);
@@ -13852,6 +14080,13 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node,
1385214080
yyjson_mut_obj_add_str(doc, root_obj, "file_path", display_path);
1385314081
yyjson_mut_obj_add_int(doc, root_obj, "start_line", start);
1385414082
yyjson_mut_obj_add_int(doc, root_obj, "end_line", end);
14083+
mcp_add_store_generation(doc, root_obj, srv->store);
14084+
yyjson_mut_obj_add_bool(doc, root_obj, "content_verified", source_content_verified);
14085+
if (source_drift) {
14086+
yyjson_mut_obj_add_bool(doc, root_obj, "source_drift", true);
14087+
yyjson_mut_obj_add_strcpy(doc, root_obj, "freshness", source_freshness);
14088+
yyjson_mut_obj_add_str(doc, root_obj, "recommended_action", "reindex_then_retry");
14089+
}
1385514090
if (snippet_clipped) {
1385614091
yyjson_mut_obj_add_bool(doc, root_obj, "source_truncated", true);
1385714092
yyjson_mut_obj_add_bool(doc, root_obj, "source_clipped", true); /* compatibility */

‎src/mcp/mcp_internal.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,13 @@ void cbm_mcp_server_set_snapshot_read_test_hook(cbm_mcp_server_t *srv,
115115
cbm_mcp_snapshot_read_test_hook_fn hook,
116116
void *context);
117117

118+
/* Runs after get_code_snippet has read and verified a file's bytes and before
119+
* it slices them, so a test can write the file at exactly that point (#2585). */
120+
typedef void (*cbm_mcp_snippet_capture_test_hook_fn)(void *context, const char *rel_path);
121+
void cbm_mcp_server_set_snippet_capture_test_hook(cbm_mcp_server_t *srv,
122+
cbm_mcp_snippet_capture_test_hook_fn hook,
123+
void *context);
124+
118125
/* Filesystem PATH_MAX prevents a portable end-to-end fixture for multi-KiB
119126
* stored paths. This seam drives the normal search result ownership,
120127
* directory aggregation, and renderer pipeline with synthetic identities. */

0 commit comments

Comments
 (0)