From 8a704486ed3528f6fe5aaf6ce34cad1b153ed40d Mon Sep 17 00:00:00 2001 From: Ulises Millan Guerrero Date: Fri, 21 Aug 2026 13:56:41 -0600 Subject: [PATCH] fix(mcp): flag source drift in get_code_snippet and search_code The snippet and full-mode search tools slice the live file on disk using line coordinates recorded at index time. After the file is edited without re-indexing, those ranges are stale: the tools returned shifted source text, or a grep hit attributed to an adjacent function, with no signal that the answer was drifting (everything after a small edit looked like current source). Both tools now consult the same freshness oracle check_index_coverage already exposes. When the file no longer matches the recorded metadata (metadata_changed/missing) they skip the stale live slice and report source_drift/freshness instead; the healthy path (metadata_match) is byte-identical to before. No index-format change: the recorded metadata is unchanged, only the read tools consult it. Fixes #1750 Signed-off-by: Ulises Millan Guerrero --- .../.openspec.yaml | 2 + .../design.md | 52 ++++++++ .../proposal.md | 25 ++++ .../mcp-tools-code-snippet-drift/spec.md | 30 +++++ .../tasks.md | 23 ++++ openspec/config.yaml | 32 +++++ .../mcp-tools-code-snippet-drift/spec.md | 32 +++++ src/mcp/mcp.c | 66 +++++++-- tests/test_mcp.c | 125 ++++++++++++++++++ 9 files changed, 377 insertions(+), 10 deletions(-) create mode 100644 openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/.openspec.yaml create mode 100644 openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/design.md create mode 100644 openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/proposal.md create mode 100644 openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/specs/mcp-tools-code-snippet-drift/spec.md create mode 100644 openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/tasks.md create mode 100644 openspec/config.yaml create mode 100644 openspec/specs/mcp-tools-code-snippet-drift/spec.md diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/.openspec.yaml b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/.openspec.yaml new file mode 100644 index 000000000..d160e09cf --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-21 diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/design.md b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/design.md new file mode 100644 index 000000000..32e331783 --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/design.md @@ -0,0 +1,52 @@ +## Context + +See proposal.md for motivation. The relevant current state (verified against source): + +- `get_code_snippet` → `build_snippet_response` (src/mcp/mcp.c:8639) → `resolve_snippet_source` (mcp.c:8506) → `read_file_lines(abs_path, start, end)` slices the LIVE file with `node->start_line/end_line` taken from the graph index. No freshness check anywhere on that path. +- `search_code` → `classify_all_grep_hits` (mcp.c:9576) attributes grep hits to symbols by `find_tightest_node` over indexed node ranges; `attach_result_source` (mcp.c:9071) then reads the live file with `r->start_line/end_line` (mode full, mcp.c:9107) or around `r->match_lines` (context mode, mcp.c:9126). +- The drift signal already exists: `coverage_path_freshness` (mcp.c:4218) compares the stored `mtime_ns`/`size` (from `cbm_store_get_file_hash`) with the file on disk. It is only consumed by `handle_check_index_coverage`; the read tools never consult it. +- Both tools already guard containment via `cbm_path_within_root`; the missing guard is metadata drift, not traversal. + +## Goals / Non-Goals + +**Goals:** +- `get_code_snippet` reports drift explicitly instead of slicing stale coordinates. +- `search_code` never attaches `source`/`context` sliced from a drifted file using stale ranges, and marks such rows. +- Drift reporting reuses `coverage_path_freshness` so the signal is identical to `check_index_coverage`. +- Healthy path (`metadata_match`) is byte-identical to today. + +**Non-Goals:** +- Re-attributing grep hits for drifted files to different symbols (impossible without re-index; the drift flag makes the row explicitly untrustworthy). Re-indexing remains the user's action. +- Changing grep-hit classification rules, result ordering, or the TOON (text) output. TOON already carries only file:line:ranges and never reads source. +- Any index-format or DB change; no new metadata is recorded. + +## Decisions + +**D1 — Reuse `coverage_path_freshness` as the single drift oracle.** +`freshness == "metadata_changed" || "missing"` ⇒ drifted → skip the live read, emit `source_drift: true` + `freshness: `. +`"metadata_match"` ⇒ current behavior unchanged. +Other states (`unavailable`, `outside_project`, `not_tracked`) ⇒ keep current behavior (no new refusal paths; `outside_project` is already handled by the containment guards). + +**D2 — get_code_snippet: gate the read at `resolve_snippet_source`.** +`resolve_snippet_source` gains a `bool read_allowed` parameter (same path-building/containment logic; only the `read_file_lines` call is skipped). `build_snippet_response` computes freshness first and passes the flag; on drift it adds `source_drift`, `freshness`, and a `source` string stating no source is available because the file changed after indexing. `file_path`/`start_line`/`end_line`/`source_clipped` remain reported (they describe the indexed node the user asked about). + +**D3 — search_code: gate each item's read in `attach_result_source`.** +`attach_result_source` gains `cbm_store_t *store, const char *project` (threaded from `handle_search_code` via `assemble_search_output`). When the item's file is drifted, neither `source` (full mode) nor `context` (context mode) is attached, and `source_drift: true` + `freshness: ` are added to the item. Raw (un-attributed) hits keep their grep-verified content — those lines come from the live grep output, not from stale index coordinates. + +**D4 — Drift check per item, not per response.** +A search result spans multiple files; per-item checks keep a single drifted file from suppressing healthy results. Cost: one `stat`+hash lookup per distinct result file, which is proportional to the result set the tool already touches. + +## Risks / Trade-offs + +- [Extra `stat`+DB lookup per snippet/search item] → Bound by result set size; `coverage_path_freshness` already runs once per path in `check_index_coverage`; the lookup is a single indexed row read. +- [Agents relying on `source` in drifted worktrees get placeholder text] → This is the intended behavior change (issue #1750); the explicit `source_drift` marker is the machine-readable signal, and the message states the remedy (re-index). +- [`not_tracked`/`unavailable` files keep today's behavior] → A file without a hash record cannot be judged; refusing would regress read-only tool behavior for foreign files. Flagged in specs as out of contract scope. +- [Implementer must not widen classification semantics] → Guard is additive-only; healthy paths unchanged (tested). + +## Migration Plan + +No data migration. No configuration change. Release notes: read tools now flag source-drift on stale coordinates instead of serving them. Rollback = revert PR; no persistent state. + +## Open Questions + +None. diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/proposal.md b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/proposal.md new file mode 100644 index 000000000..5bde389b0 --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/proposal.md @@ -0,0 +1,25 @@ +## Why + +`get_code_snippet` and `search_code` read the live file from disk but slice it using line coordinates captured at index time. After the file is edited on disk without re-indexing, those coordinates are stale: the tools return shifted/corrupted source text, or a grep hit is misattributed to an adjacent function. There is no signal that the answer is drifting — the tools present stale slices as though they were the current source. + +## What Changes + +- `get_code_snippet` detects index-to-disk drift before slicing the live file and, instead of returning corrupted source text, reports the drift explicitly (no `source` payload; a `source_drift` marker and a human-readable reason with the freshness state). +- `search_code` applies the same guard in `MODE_FULL` (and `MODE_COMPACT` where result rows carry source lines) so matches are not attached to unrelated source text. +- Drift detection reuses the existing freshness machinery (`coverage_path_freshness`, the same signal `check_index_coverage` already exposes as `metadata_match` / `metadata_changed` / `missing`) — no new metadata is written by the indexer. +- Behavior on `metadata_match` (clean) is unchanged, so a current graph keeps returning full snippets. +- No index format or API surface changes on the write side: this only changes what the two read tools return when the file drifted. + +## Capabilities + +### New Capabilities +- `mcp-tools-code-snippet-drift`: behavior of `get_code_snippet` and `search_code` when disk content no longer matches the indexed metadata (drift detection and honest reporting). + +### Modified Capabilities +- None (no existing specs; this repo has no committed specs yet — `openspec/specs/` is empty). + +## Impact + +- `src/mcp/mcp.c` — `build_snippet_response`, `resolve_snippet_source`, `attach_result_source`, and the two tool handlers (drift guard + response shape). +- `tests/test_mcp.c` — new regression tests: snippet after drift reports drift instead of stale text; search results after drift do not attach drifted source; clean-metadata path keeps returning full source. +- Response contract of the two MCP tools gains an optional `source_drift` field on the drifted path only. No tool is added/removed/renamed; the healthy path is byte-compatible. diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/specs/mcp-tools-code-snippet-drift/spec.md b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/specs/mcp-tools-code-snippet-drift/spec.md new file mode 100644 index 000000000..2ba5a54ff --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/specs/mcp-tools-code-snippet-drift/spec.md @@ -0,0 +1,30 @@ +## Purpose +Defines how the MCP source-reading tools behave when a file on disk no longer matches the metadata recorded at index time: they must detect the drift and report it explicitly instead of silently serving source text sliced with stale line coordinates. + +## ADDED Requirements + +### Requirement: get_code_snippet detects index-to-disk drift before serving source +When a resolved node's file has changed on disk since indexing (freshness `metadata_changed`) or is missing on disk, `get_code_snippet` SHALL NOT serve source text read from the live file using the node's indexed `[start_line, end_line]` coordinates. The response SHALL instead mark the result as drifted with a `source_drift` boolean, a `freshness` field naming the state (`metadata_changed` or `missing`), and a `source` value that states no source is available because the file changed after indexing. When the file matches the index (`metadata_match`) the tool SHALL return the full source slice exactly as before. + +#### Scenario: snippet requested for a file edited after indexing +- **WHEN** a file was indexed, then its content is modified on disk (mtime or size changes) without re-indexing, and `get_code_snippet` resolves a symbol in that file +- **THEN** the response contains `source_drift: true`, `freshness: "metadata_changed"` or `"missing"`, and no source text sliced from the live file with the stale indexed coordinates + +#### Scenario: snippet requested for a file whose metadata matches the index +- **WHEN** `get_code_snippet` resolves a symbol in a file whose recorded mtime/size equal the file on disk +- **THEN** the response is unchanged from today: `source` carries the live slice of the file's current content at the indexed coordinates + +### Requirement: search_code does not attach drifted source text to results +`search_code` SHALL NOT attach `source` or `context` text read from a file whose on-disk metadata differs from the index (`metadata_changed` or `missing`) using the stale indexed `[start_line, end_line]` ranges. Such result items SHALL carry a `source_drift` marker and a `freshness` field naming the state. Items for files whose metadata matches the index SHALL keep today's `source`/`context` attachment. + +#### Scenario: full-mode search hit in a file edited after indexing +- **WHEN** `search_code` (mode `full`) finds a match in a file whose metadata no longer matches the index +- **THEN** the result item carries `source_drift: true` and `freshness: "metadata_changed"` (or `"missing"`) and no `source` text sliced from the live file with stale coordinates + +#### Scenario: full-mode search hit in a file matching the index +- **WHEN** `search_code` (mode `full`) finds a match in a file whose recorded metadata equals the file on disk +- **THEN** the result item carries the `source` window around the match exactly as today + +#### Scenario: context-mode search hit in a drifted file +- **WHEN** `search_code` is called with `context` lines against a file whose metadata no longer matches the index +- **THEN** the result item carries `source_drift: true` and `freshness: "metadata_changed"`, and does not attach `context` text for a symbol attribution that cannot be trusted diff --git a/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/tasks.md b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/tasks.md new file mode 100644 index 000000000..d6164557a --- /dev/null +++ b/openspec/changes/archive/2026-08-21-oss-codebase-memory-mcp-issue-1750/tasks.md @@ -0,0 +1,23 @@ +## 1. Drift guard in get_code_snippet + +- [x] 1.1 Change `resolve_snippet_source` (src/mcp/mcp.c:8506) to accept a `bool read_allowed` parameter and skip `read_file_lines` when false, keeping the path/containment logic and `out_abs_path` intact +- [x] 1.2 In `build_snippet_response` (src/mcp/mcp.c:8639), compute `coverage_path_freshness(srv->store, node->project, root_path, node->file_path, &outside)` before resolving the source; when the state is `metadata_changed` or `missing`, call `resolve_snippet_source` with `read_allowed=false` and add `source_drift: true`, `freshness: `, and a `source` string stating the file changed after indexing and re-indexing is required +- [ ] 1.3 Verify: `make -f Makefile.cbm test-focused TEST_SUITES=mcp` passes (existing snippet tests green; no drift test yet) + +## 2. Drift guard in search_code + +- [x] 2.1 Thread `cbm_store_t *store` and `const char *project` into `assemble_search_output` (src/mcp/mcp.c:9287) and its call site (mcp.c:10208), then into `attach_result_source` (mcp.c:9071) +- [x] 2.2 In `attach_result_source`, when `coverage_path_freshness` reports `metadata_changed` or `missing` for `r->file`, skip `source`/`context` attachment and add `source_drift: true` + `freshness: ` to the item object +- [ ] 2.3 Verify: `make -f Makefile.cbm test-focused TEST_SUITES=mcp` passes + +## 3. Regression tests + +- [x] 3.1 Add `TEST(tool_get_code_snippet_reports_sourceless_drift_after_file_change)` in tests/test_mcp.c: index a small fixture via the test store helpers, rewrite the file on disk (changing mtime/size), call the snippet handler, and assert `source_drift: true`, `freshness: "metadata_changed"`, and no stale source text; then restore mtime/size and assert the response carries the full source again (metadata_match path unchanged) +- [x] 3.2 Add `TEST(tool_search_code_marks_drifted_file_results)` in tests/test_mcp.c: index a fixture, edit the file on disk, run a search matching that file (mode full), and assert the result item carries `source_drift: true` and no `source` text; a second non-drifted file in the same index still gets `source` attached +- [x] 3.3 Verify: `make -f Makefile.cbm test-focused TEST_SUITES=mcp` passes with the new tests + +## 4. Final validation + +- [ ] 4.1 Run `make -f Makefile.cbm test` (full suite) and `make -f Makefile.cbm lint-ci`; both pass +- [ ] 4.2 Run `OPENSPEC_NO_UPDATE_CHECK=1 openspec validate oss-codebase-memory-mcp-issue-1750 --json`; clean +- [ ] 4.3 Archive the change: `OPENSPEC_NO_UPDATE_CHECK=1 openspec archive oss-codebase-memory-mcp-issue-1750 --yes` diff --git a/openspec/config.yaml b/openspec/config.yaml new file mode 100644 index 000000000..c4d34acea --- /dev/null +++ b/openspec/config.yaml @@ -0,0 +1,32 @@ +schema: spec-driven + +# Project context (optional) +# This is shown to AI when creating artifacts. +# Add your tech stack, conventions, style guides, domain knowledge, etc. +# Example: +# context: | +# Tech stack: TypeScript, React, Node.js +# We use conventional commits +# Domain: e-commerce platform + +# Per-artifact rules (optional) +# Add custom rules for specific artifacts. +# Example: +# rules: +# proposal: +# - Keep proposals under 500 words +# - Always include a "Non-goals" section +# tasks: +# - Break tasks into chunks of max 2 hours + +# Per-operation guidance (optional) +# Add advisory guidance for how apply and archive work should be conducted. +# This is separate from artifact rules above. +# Example: +# operations: +# apply: +# guidance: +# - Keep test summaries concise +# archive: +# guidance: +# - Summarize the archive outcome before finishing diff --git a/openspec/specs/mcp-tools-code-snippet-drift/spec.md b/openspec/specs/mcp-tools-code-snippet-drift/spec.md new file mode 100644 index 000000000..2152470a8 --- /dev/null +++ b/openspec/specs/mcp-tools-code-snippet-drift/spec.md @@ -0,0 +1,32 @@ +# mcp-tools-code-snippet-drift Specification + +## Purpose +Defines how the MCP source-reading tools behave when a file on disk no longer matches the metadata recorded at index time: they must detect the drift and report it explicitly instead of silently serving source text sliced with stale line coordinates. + +## Requirements + +### Requirement: get_code_snippet detects index-to-disk drift before serving source +When a resolved node's file has changed on disk since indexing (freshness `metadata_changed`) or is missing on disk, `get_code_snippet` SHALL NOT serve source text read from the live file using the node's indexed `[start_line, end_line]` coordinates. The response SHALL instead mark the result as drifted with a `source_drift` boolean, a `freshness` field naming the state (`metadata_changed` or `missing`), and a `source` value that states no source is available because the file changed after indexing. When the file matches the index (`metadata_match`) the tool SHALL return the full source slice exactly as before. + +#### Scenario: snippet requested for a file edited after indexing +- **WHEN** a file was indexed, then its content is modified on disk (mtime or size changes) without re-indexing, and `get_code_snippet` resolves a symbol in that file +- **THEN** the response contains `source_drift: true`, `freshness: "metadata_changed"` or `"missing"`, and no source text sliced from the live file with the stale indexed coordinates + +#### Scenario: snippet requested for a file whose metadata matches the index +- **WHEN** `get_code_snippet` resolves a symbol in a file whose recorded mtime/size equal the file on disk +- **THEN** the response is unchanged from today: `source` carries the live slice of the file's current content at the indexed coordinates + +### Requirement: search_code does not attach drifted source text to results +`search_code` SHALL NOT attach `source` or `context` text read from a file whose on-disk metadata differs from the index (`metadata_changed` or `missing`) using the stale indexed `[start_line, end_line]` ranges. Such result items SHALL carry a `source_drift` marker and a `freshness` field naming the state. Items for files whose metadata matches the index SHALL keep today's `source`/`context` attachment. + +#### Scenario: full-mode search hit in a file edited after indexing +- **WHEN** `search_code` (mode `full`) finds a match in a file whose metadata no longer matches the index +- **THEN** the result item carries `source_drift: true` and `freshness: "metadata_changed"` (or `"missing"`) and no `source` text sliced from the live file with stale coordinates + +#### Scenario: full-mode search hit in a file matching the index +- **WHEN** `search_code` (mode `full`) finds a match in a file whose recorded metadata equals the file on disk +- **THEN** the result item carries the `source` window around the match exactly as today + +#### Scenario: context-mode search hit in a drifted file +- **WHEN** `search_code` is called with `context` lines against a file whose metadata no longer matches the index +- **THEN** the result item carries `source_drift: true` and `freshness: "metadata_changed"`, and does not attach `context` text for a symbol attribution that cannot be trusted diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 04316627b..75e569ad2 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -8504,7 +8504,7 @@ bool cbm_path_within_root(const char *root_path, const char *abs_path) { } 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; @@ -8514,7 +8514,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; @@ -8654,7 +8658,18 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node, snippet_clipped = true; } char *abs_path = NULL; - char *source = resolve_snippet_source(root_path, node->file_path, start, end, &abs_path); + /* #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 = coverage_path_freshness(srv->store, node->project, root_path, + node->file_path, &snippet_outside); + bool snippet_drifted = freshness && (strcmp(freshness, "metadata_changed") == 0 || + strcmp(freshness, "missing") == 0); + char *source = + resolve_snippet_source(root_path, node->file_path, start, end, !snippet_drifted, &abs_path); yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); yyjson_mut_val *root_obj = yyjson_mut_obj(doc); @@ -8687,10 +8702,23 @@ 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", "(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); @@ -9069,7 +9097,8 @@ static yyjson_mut_val *build_dedup_files_array(yyjson_mut_doc *doc, search_resul /* Attach source or context lines to a search result JSON item. */ static void attach_result_source(yyjson_mut_doc *doc, yyjson_mut_val *item, search_result_t *r, - int mode, int context_lines, const char *root_path) { + int mode, int context_lines, const char *root_path, + cbm_store_t *store, const char *project) { enum { MODE_FULL = 1 }; if (r->start_line <= 0 || r->end_line <= 0) { return; @@ -9085,6 +9114,22 @@ static void attach_result_source(yyjson_mut_doc *doc, yyjson_mut_val *item, sear return; } + /* #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 outside = false; + const char *freshness = + store ? coverage_path_freshness(store, project, root_path, r->file, &outside) : NULL; + bool drifted = freshness && (strcmp(freshness, "metadata_changed") == 0 || + strcmp(freshness, "missing") == 0); + if (drifted && freshness) { + yyjson_mut_obj_add_bool(doc, item, "source_drift", true); + yyjson_mut_obj_add_str(doc, item, "freshness", freshness); + return; + } + if (mode == MODE_FULL) { /* Cap each hit's source at a match-anchored window: uncapped * whole-symbol dumps ran to 5.7KB × N hits (142KB responses). The @@ -9286,8 +9331,9 @@ static char *assemble_search_output_toon(search_result_t *sr, int sr_count, grep /* Phase 4: assemble JSON output from search results */ static char *assemble_search_output(search_result_t *sr, int sr_count, grep_match_t *raw, int raw_count, int gm_count, int limit, int mode, - int context_lines, const char *root_path, - bool warn_literal_pipe, const search_metrics_t *metrics) { + int context_lines, const char *root_path, cbm_store_t *store, + const char *project, bool warn_literal_pipe, + const search_metrics_t *metrics) { enum { MODE_COMPACT = 0, MODE_FULL = 1, MODE_FILES = 2, SEARCH_SLOW_MS = 5000 }; yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); @@ -9339,7 +9385,7 @@ static char *assemble_search_output(search_result_t *sr, int sr_count, grep_matc yyjson_mut_arr_add_int(doc, row, r->out_degree); if (mode == MODE_FULL || attach_context) { yyjson_mut_val *src = yyjson_mut_obj(doc); - attach_result_source(doc, src, r, mode, context_lines, root_path); + attach_result_source(doc, src, r, mode, context_lines, root_path, store, project); yyjson_mut_arr_add_val(row, src); } yyjson_mut_arr_add_val(results_arr, row); @@ -10205,9 +10251,9 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { result = cbm_mcp_text_result(toon_text ? toon_text : "out of memory", toon_text == NULL); free(toon_text); } else { - result = - assemble_search_output(sr, sr_count, raw, raw_count, gm_count, limit, mode, - context_lines, root_path, pat_has_pipe && !use_regex, &metrics); + result = assemble_search_output(sr, sr_count, raw, raw_count, gm_count, limit, mode, + context_lines, root_path, store, project, + pat_has_pipe && !use_regex, &metrics); } free(gm); free(sr); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 278d33e68..a6fd1d856 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -2003,6 +2003,129 @@ TEST(tool_get_code_snippet_clips_whole_file_node) { 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); + struct stat source_stat; + ASSERT_EQ(stat(source_path, &source_stat), 0); +#ifdef __APPLE__ + int64_t source_mtime_ns = + ((int64_t)source_stat.st_mtimespec.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + + (int64_t)source_stat.st_mtimespec.tv_nsec; +#elif defined(_WIN32) + int64_t source_mtime_ns = (int64_t)source_stat.st_mtime * (int64_t)CBM_NSEC_PER_SEC; +#else + int64_t source_mtime_ns = ((int64_t)source_stat.st_mtim.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + + (int64_t)source_stat.st_mtim.tv_nsec; +#endif + ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", source_mtime_ns, + source_stat.st_size), + CBM_STORE_OK); + + /* Fresh metadata: full source served, no drift marker. */ + char *resp = + cbm_mcp_handle_tool(srv, "get_code_snippet", + "{\"project\":\"test-project\"," + "\"qualified_name\":\"test-project.cmd.server.main.HandleRequest\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "func HandleRequest() error")); + ASSERT_NULL(strstr(resp, "source_drift")); + free(resp); + + /* Append lines: the file gains content, so the recorded metadata no longer + * matches. The indexed 3-5 range is now stale for edited source. */ + 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\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "\"source_drift\":true")); + ASSERT_NOT_NULL(strstr(resp, "\"freshness\":\"metadata_changed\"")); + ASSERT_NOT_NULL(strstr(resp, "not available")); + free(resp); + + 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); + struct stat source_stat; + ASSERT_EQ(stat(source_path, &source_stat), 0); +#ifdef __APPLE__ + int64_t source_mtime_ns = + ((int64_t)source_stat.st_mtimespec.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + + (int64_t)source_stat.st_mtimespec.tv_nsec; +#elif defined(_WIN32) + int64_t source_mtime_ns = (int64_t)source_stat.st_mtime * (int64_t)CBM_NSEC_PER_SEC; +#else + int64_t source_mtime_ns = ((int64_t)source_stat.st_mtim.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + + (int64_t)source_stat.st_mtim.tv_nsec; +#endif + ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", source_mtime_ns, + source_stat.st_size), + CBM_STORE_OK); + + /* Fresh metadata: source is attached to the result item. */ + char *resp = cbm_mcp_handle_tool( + srv, "search_code", + "{\"pattern\":\"HandleRequest\",\"project\":\"test-project\",\"mode\":\"full\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "func HandleRequest() error")); + ASSERT_NULL(strstr(resp, "source_drift")); + free(resp); + + /* Append lines: metadata drifts; the stale indexed range must not be sliced + * into the response. */ + 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\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_NOT_NULL(strstr(resp, "\"source_drift\":true")); + ASSERT_NOT_NULL(strstr(resp, "\"freshness\":\"metadata_changed\"")); + free(resp); + + 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 @@ -11318,6 +11441,8 @@ SUITE(mcp) { RUN_TEST(tool_trace_totals_respect_test_filter_tests_root_subtree_issue1294); RUN_TEST(tool_get_architecture_cycles_detects_scc); RUN_TEST(tool_get_code_snippet_clips_whole_file_node); + 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);