diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 5bd02e4e3..e4b5783bf 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -12942,7 +12942,7 @@ static char *snippet_pdf_text(const cbm_node_t *node) { } static char *resolve_snippet_source(const char *root_path, const char *file_path, int start, - int end, char **out_abs_path) { + int end, bool read_allowed, char **out_abs_path) { *out_abs_path = NULL; if (!root_path || !file_path) { return NULL; @@ -12952,7 +12952,11 @@ static char *resolve_snippet_source(const char *root_path, const char *file_path snprintf(abs_path, apsz, "%s/%s", root_path, file_path); *out_abs_path = abs_path; - if (cbm_path_within_root(root_path, abs_path)) { + /* read_allowed=false: the indexed [start,end] coordinates are stale + * relative to the file on disk, so slicing the live file would return + * shifted text. The caller still gets abs_path for display and the + * caller reports the drift (#1750). */ + if (cbm_path_within_root(root_path, abs_path) && read_allowed) { return read_file_lines(abs_path, start, end); } return NULL; @@ -13440,12 +13444,25 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node, } char *abs_path = NULL; char *source = NULL; + /* #1750: the indexed [start,end] coordinates are only valid while the file + * on disk matches the recorded metadata. If the file was edited without + * re-indexing, slicing the live file would return shifted text (or a + * different function's body). Report the drift instead and keep the + * requested node's coordinates. */ + bool snippet_outside = false; + const char *freshness = NULL; + bool snippet_drifted = false; if (outline) { 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); + freshness = coverage_path_freshness(srv->store, node->project, root_path, node->file_path, + &snippet_outside); + snippet_drifted = freshness && (strcmp(freshness, "metadata_changed") == 0 || + strcmp(freshness, "missing") == 0); + source = resolve_snippet_source(root_path, node->file_path, start, end, !snippet_drifted, + &abs_path); } yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); @@ -13547,11 +13564,24 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node, } else { yyjson_mut_obj_add_str(doc, root_obj, "source", "(source not available)"); } + } else if (snippet_drifted && freshness) { + yyjson_mut_obj_add_str( + doc, root_obj, "source", + "(source not available: file changed after indexing; re-index the project for " + "accurate coordinates)"); } else { yyjson_mut_obj_add_str(doc, root_obj, "source_mode", "full"); yyjson_mut_obj_add_str(doc, root_obj, "source", "(source not available)"); } + /* #1750: the recorded start/end coordinates are stale relative to the file + * on disk. Explicitly flag the drift so consumers can tell "current source" + * from "index-time coordinates", instead of silently receiving shifted text. */ + if (snippet_drifted && freshness) { + yyjson_mut_obj_add_bool(doc, root_obj, "source_drift", true); + yyjson_mut_obj_add_str(doc, root_obj, "freshness", freshness); + } + /* match_method — omitted for exact matches */ if (match_method) { yyjson_mut_obj_add_str(doc, root_obj, "match_method", match_method); @@ -14087,6 +14117,7 @@ typedef struct { int match_count; /* exact total across the complete grep stream */ int match_stored_count; /* bounded evidence retained for rendering */ int match_capacity; + const char *freshness; /* #1750: static freshness label of the file, or NULL */ } search_result_t; typedef struct { @@ -14419,8 +14450,22 @@ static void attach_result_source(yyjson_mut_doc *doc, yyjson_mut_val *item, sear /* Containment: a search result whose indexed path resolves outside the * project root (a `..` segment, or a symlink/junction that discovery * followed) must not be read back into the response. Same guard the - * snippet path already uses. */ - if (!root_containment_check(containment, abs_path)) { + * snippet path already uses. + * + * #1750: the indexed [start,end] ranges only describe the file as it was + * at index time. If the file changed on disk, attaching source/context + * sliced with those ranges returns shifted text (or another function's + * body), and the symbol attribution itself is untrustworthy. Flag the + * drift and attach nothing. */ + bool inside = root_containment_check(containment, abs_path); + const char *freshness = r->freshness; + bool drifted = freshness && (strcmp(freshness, "metadata_changed") == 0 || + strcmp(freshness, "missing") == 0); + if (!inside || drifted) { + if (inside) { + yyjson_mut_obj_add_bool(doc, item, "source_drift", true); + yyjson_mut_obj_add_str(doc, item, "freshness", freshness); + } free(abs_path); return; } @@ -16976,6 +17021,17 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { free(out_degs); } + /* #1750: resolve each result file's freshness once per request. The indexed + * [start,end] ranges are only valid while the file matches its recorded + * metadata, so the renderers use this to withhold stale source/context. */ + for (int j = 0; j < sr_count; j++) { + bool outside = false; + sr[j].freshness = + (store && root_path && sr[j].file) + ? coverage_path_freshness(store, project, root_path, sr[j].file, &outside) + : NULL; + } + /* Compute scores and sort */ for (int j = 0; j < sr_count; j++) { sr[j].score = compute_search_score(&sr[j]); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 80a320ca5..d3cdcc3d4 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -3716,6 +3716,124 @@ TEST(tool_get_code_snippet_pages_outline_rows_to_exact_budget) { PASS(); } +/* #1750: get_code_snippet must detect index-to-disk drift before slicing the + * live file. Once the file is edited without re-indexing, the indexed + * [start,end] coordinates are stale; serving the live file with them returns + * shifted text (or another function's body). The response must flag the drift + * instead of silently serving stale coordinates. */ +TEST(tool_get_code_snippet_reports_drift_after_file_change) { + 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); + + char source_path[512]; + snprintf(source_path, sizeof(source_path), "%s/project/main.go", tmp); + /* Same mtime source the indexer and the freshness oracle use (#1714). */ + 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, + path_info.size), + CBM_STORE_OK); + + char *resp = + cbm_mcp_handle_tool(srv, "get_code_snippet", + "{\"project\":\"test-project\"," + "\"qualified_name\":\"test-project.cmd.server.main.HandleRequest\"," + "\"format\":\"json\"}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + free(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "func HandleRequest() error")); + ASSERT_NULL(strstr(inner, "source_drift")); + free(inner); + + /* Append lines without re-indexing: the recorded metadata no longer + * matches, so the indexed range is stale for the edited file. */ + FILE *fp = fopen(source_path, "a"); + ASSERT_NOT_NULL(fp); + for (int i = 0; i < 12; i++) { + fprintf(fp, "// padding after edit %d\n", i); + } + fclose(fp); + + resp = cbm_mcp_handle_tool(srv, "get_code_snippet", + "{\"project\":\"test-project\"," + "\"qualified_name\":\"test-project.cmd.server.main.HandleRequest\"," + "\"format\":\"json\"}"); + ASSERT_NOT_NULL(resp); + inner = extract_text_content(resp); + free(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "\"source_drift\":true")); + ASSERT_NOT_NULL(strstr(inner, "\"freshness\":\"metadata_changed\"")); + ASSERT_NOT_NULL(strstr(inner, "file changed after indexing")); + ASSERT_NULL(strstr(inner, "func HandleRequest() error")); + free(inner); + + cleanup_snippet_dir(tmp); + cbm_mcp_server_free(srv); + PASS(); +} + +/* #1750: search_code must not attach source sliced with stale indexed ranges + * when the file changed on disk after indexing. The result item is flagged + * source_drift instead, and the healthy path keeps attaching source. */ +TEST(tool_search_code_marks_drifted_file_without_source) { + 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); + + char source_path[512]; + snprintf(source_path, sizeof(source_path), "%s/project/main.go", tmp); + /* Same mtime source the indexer and the freshness oracle use (#1714). */ + 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, + path_info.size), + CBM_STORE_OK); + + char *resp = cbm_mcp_handle_tool(srv, "search_code", + "{\"pattern\":\"HandleRequest\",\"project\":\"test-project\"," + "\"mode\":\"full\",\"format\":\"json\"}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + free(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "func HandleRequest() error")); + ASSERT_NULL(strstr(inner, "source_drift")); + free(inner); + + /* Append lines without re-indexing: the recorded metadata no longer + * matches, so the indexed range is stale for the edited file. */ + FILE *fp = fopen(source_path, "a"); + ASSERT_NOT_NULL(fp); + for (int i = 0; i < 12; i++) { + fprintf(fp, "// padding after edit %d\n", i); + } + fclose(fp); + + resp = cbm_mcp_handle_tool(srv, "search_code", + "{\"pattern\":\"HandleRequest\",\"project\":\"test-project\"," + "\"mode\":\"full\",\"format\":\"json\"}"); + ASSERT_NOT_NULL(resp); + inner = extract_text_content(resp); + free(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "\"source_drift\":true")); + ASSERT_NOT_NULL(strstr(inner, "\"freshness\":\"metadata_changed\"")); + ASSERT_NULL(strstr(inner, "func HandleRequest() error")); + free(inner); + + cleanup_snippet_dir(tmp); + cbm_mcp_server_free(srv); + PASS(); +} + /* EVERY tool, not just the one that was reported. * * The duplication was invisible per-tool: each result looked reasonable on its @@ -25101,6 +25219,8 @@ SUITE(mcp) { RUN_TEST(tool_get_code_snippet_clips_whole_file_node); RUN_TEST(tool_get_code_snippet_omits_over_budget_whole_line); RUN_TEST(tool_get_code_snippet_pages_outline_rows_to_exact_budget); + RUN_TEST(tool_get_code_snippet_reports_drift_after_file_change); + RUN_TEST(tool_search_code_marks_drifted_file_without_source); RUN_TEST(tool_search_graph_includes_node_properties); RUN_TEST(tool_search_graph_toon_never_leaks_internal_fields); RUN_TEST(tool_lean_defaults_schema_and_status);