Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-21
Original file line number Diff line number Diff line change
@@ -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: <state>`.
`"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: <state>` 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.
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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: <state>`, 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: <state>` 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`
32 changes: 32 additions & 0 deletions openspec/config.yaml
Original file line number Diff line number Diff line change
@@ -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
32 changes: 32 additions & 0 deletions openspec/specs/mcp-tools-code-snippet-drift/spec.md
Original file line number Diff line number Diff line change
@@ -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
Loading
Loading