Browse Source

fix(mcp): land #1624 opt-in explore dedup (#1788)

Land the #1624 approach by @danusha2345: re-serve source by default so
subagents and compacted contexts never inherit stale already-sent pointers.
Keep cross-call dedup available through explicit truthy
CODEGRAPH_EXPLORE_DEDUP values for durable contexts.

Preserve current Unreleased entries, credit the contribution, and align
the MCP server guidance with the safe default.

Validation on Linux / Node 22.23.2:
- Reproduced default-on failure before the fix; default-off now passes.
- Focused explore-cross-call-dedup suite: 26 passed.
- TypeScript: npx tsc -p tsconfig.json --noEmit passed.

Fixes #1620.

(cherry picked from commit 63992facabbbcef2797167dcdd90695d3802b533)

Co-authored-by: danusha2345 <ewidusoc498@gmail.com>
Colby Mchenry 2 hours ago
parent
commit
b8d46d13c7

+ 2 - 0
CHANGELOG.md

@@ -137,6 +137,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 #### MCP / indexing
 
+- `codegraph_explore` now re-serves source to fresh subagents and after context compaction, with cross-call dedup available only through an explicit `CODEGRAPH_EXPLORE_DEDUP=1` opt-in; thanks @danusha2345. (#1620, #1624)
+
 - **Watcher scope now matches `git ls-files --exclude-standard` (#1728).** `buildDefaultIgnore` / `buildScopeIgnore` read `.git/info/exclude` and `core.excludesFile` (not only the root `.gitignore`), and seed directories git reports as ignored-untracked so nested `.gitignore` effects prune the live watcher the same way the indexer skips them. Single-file auto-sync was already incremental (`pendingFiles` → scoped `sync({ paths })`); the remaining gap was watching trees git had excluded.
 
 - **Live sync no longer lets the write-ahead log grow without a bound when a reader is holding it open (#1539).** Incremental sync now uses the same writer pause that full indexing already used, and if checkpointing still cannot finish once the log is past its documented size limit — typically because the query pool is reading at the same time — sync stops with a clear error instead of keeping writing until the disk fills. The previous behaviour could leave a multi-tens-of-gigabyte log beside a few-gigabyte index on a large project. Close concurrent readers and retry, or raise `CODEGRAPH_WAL_VALVE_MB` if the limit is too tight for the project.

+ 44 - 0
__tests__/explore-cross-call-dedup.test.ts

@@ -28,6 +28,7 @@ import { ExploreSessionState, type ExploreProjectState } from '../src/mcp/explor
 import {
   EXPLORE_DEDUP,
   dedupeRange,
+  exploreDedupEnabled,
   fileFingerprint,
   formatBackReference,
   intersectRange,
@@ -41,6 +42,27 @@ const FIXTURE_SRC = path.join(__dirname, 'fixtures', 'payroll-go');
 const QUERY = 'how does payroll cycle create and calculate payslips?';
 const POINTER = 'Already sent earlier in this conversation';
 
+describe('dedup configuration', () => {
+  it('defaults off and requires an explicit truthy opt-in', () => {
+    const previous = process.env.CODEGRAPH_EXPLORE_DEDUP;
+    try {
+      delete process.env.CODEGRAPH_EXPLORE_DEDUP;
+      expect(exploreDedupEnabled()).toBe(false);
+      for (const enabled of ['1', 'true', 'on', 'yes', ' YES ']) {
+        process.env.CODEGRAPH_EXPLORE_DEDUP = enabled;
+        expect(exploreDedupEnabled()).toBe(true);
+      }
+      for (const disabled of ['0', 'false', 'off', 'no', 'unexpected']) {
+        process.env.CODEGRAPH_EXPLORE_DEDUP = disabled;
+        expect(exploreDedupEnabled()).toBe(false);
+      }
+    } finally {
+      if (previous === undefined) delete process.env.CODEGRAPH_EXPLORE_DEDUP;
+      else process.env.CODEGRAPH_EXPLORE_DEDUP = previous;
+    }
+  });
+});
+
 /** A prior-state shaped like the session tracker's, for the algebra tests. */
 function prior(files: Array<{ path: string; ranges: Array<[number, number]>; fingerprint?: string }>): ExploreProjectState {
   return {
@@ -183,8 +205,11 @@ describe('a second call against a real index', () => {
   let testDir: string;
   let cg: CodeGraph;
   let handler: ToolHandler;
+  let previousDedup: string | undefined;
 
   beforeAll(async () => {
+    previousDedup = process.env.CODEGRAPH_EXPLORE_DEDUP;
+    process.env.CODEGRAPH_EXPLORE_DEDUP = '1';
     testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cg18-'));
     fs.cpSync(FIXTURE_SRC, testDir, { recursive: true });
     fs.rmSync(path.join(testDir, '.codegraph'), { recursive: true, force: true });
@@ -194,6 +219,8 @@ describe('a second call against a real index', () => {
   }, 120_000);
 
   afterAll(() => {
+    if (previousDedup === undefined) delete process.env.CODEGRAPH_EXPLORE_DEDUP;
+    else process.env.CODEGRAPH_EXPLORE_DEDUP = previousDedup;
     if (cg) cg.destroy();
     if (testDir && fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true });
   });
@@ -328,6 +355,23 @@ describe('a second call against a real index', () => {
     }
   }, 120_000);
 
+  it('re-serves source by default when a connection may outlive the current context', async () => {
+    const session = new ExploreSessionState();
+    const previous = process.env.CODEGRAPH_EXPLORE_DEDUP;
+    delete process.env.CODEGRAPH_EXPLORE_DEDUP;
+    try {
+      const first = await explore(QUERY, session);
+      const second = await explore(QUERY, session);
+      expect(second).toBe(first);
+      expect(second).not.toContain(POINTER);
+      expect([...fencedLines(second).values()].reduce((sum, lines) => sum + lines.size, 0))
+        .toBeGreaterThan(20);
+    } finally {
+      if (previous === undefined) delete process.env.CODEGRAPH_EXPLORE_DEDUP;
+      else process.env.CODEGRAPH_EXPLORE_DEDUP = previous;
+    }
+  }, 120_000);
+
   it('reports the reclaimed bytes through the CG-4 diagnostic', async () => {
     const sidecar = path.join(testDir, 'cg18-diagnostic.jsonl');
     const session = new ExploreSessionState();

+ 8 - 5
src/mcp/explore-dedup.ts

@@ -68,16 +68,19 @@ export const EXPLORE_DEDUP = {
   MAX_SYMBOLS_IN_POINTER: 5,
 } as const;
 
-const OFF = new Set(['0', 'false', 'off', 'no']);
+const ON = new Set(['1', 'true', 'on', 'yes']);
 
 /**
- * Kill switch: `CODEGRAPH_EXPLORE_DEDUP=0` renders every call as if the session
- * had no history. Read per call (not memoized) so a test can toggle it.
+ * Cross-call source suppression is opt-in. An MCP connection is not a reliable
+ * conversation boundary: some hosts reuse it for subagents, and compaction can
+ * discard source while keeping the connection alive (#1620). Without a host-
+ * supplied context lifecycle, re-serving source is the only always-correct
+ * default. Read per call (not memoized) so tests and launchers can toggle it.
  */
 export function exploreDedupEnabled(): boolean {
   const raw = process.env.CODEGRAPH_EXPLORE_DEDUP;
-  if (raw === undefined) return true;
-  return !OFF.has(raw.trim().toLowerCase());
+  if (raw === undefined) return false;
+  return ON.has(raw.trim().toLowerCase());
 }
 
 /**

+ 1 - 1
src/mcp/server-instructions.ts

@@ -64,7 +64,7 @@ calls; a grep/read exploration is dozens.
 - **After editing, check the staleness banner.** When a tool response starts with "⚠️ Some files referenced below were edited since the last index sync…", the listed files are pending re-index — Read those specific files for accurate content. Every file NOT in that banner is fresh, so still trust codegraph. A different, rarer banner — "⚠️ CodeGraph auto-sync is DISABLED…" — means live watching stopped entirely (the whole index is frozen, not just a few files); until it's resolved, Read files directly to confirm anything that may have changed.
 - **A file flagged "⚠ changed on disk after the last index sync" drifted from its index** (most common on projects queried via \`projectPath\`, which have no live watcher). Codegraph never serves a possibly-mis-sliced body from such a file — it either shows the file's full CURRENT source (trust it as a Read) or omits the source with this flag. When the source was omitted, Read that specific file; line numbers referencing it elsewhere in the response may be shifted until that project's next sync. All unflagged files remain trustworthy.
 
-- **"Already sent earlier in this conversation" is a pointer, not a gap.** When a file's section carries that line instead of (or above) its source, an earlier \`codegraph_explore\` in THIS conversation already returned those exact lines and the file has not changed since — so the copy already in your context is current and exact. Scroll back to it; don't re-fetch it and don't Read the file. The bytes it freed went into source you have not seen yet, elsewhere in the same response.
+- **Source is re-served on every call by default**, including for fresh subagents and after context compaction. Cross-call dedup requires \`CODEGRAPH_EXPLORE_DEDUP=1\` and is only suitable for hosts that guarantee one durable context per connection. With that opt-in, **"Already sent earlier in this conversation"** points to exact, unchanged source returned by an earlier \`codegraph_explore\` in that context. Use that copy; don't re-fetch it and don't Read the file. The bytes it freed went into source you have not seen yet, elsewhere in the same response.
 
 ## Limitations