diff --git a/__tests__/malformed-recovery.test.ts b/__tests__/malformed-recovery.test.ts new file mode 100644 index 00000000..f38409cf --- /dev/null +++ b/__tests__/malformed-recovery.test.ts @@ -0,0 +1,169 @@ +/** + * Issue #679: MCP SQLite stale connection recovery (lazy detection) + * + * When CLI `zcodegraph index` rebuilds the database, the MCP server's + * long-lived CodeGraph instance holds a stale SQLite handle. Subsequent + * tool calls fail with "database disk image is malformed". The fix: + * detect the corruption error in ToolHandler.execute()'s catch block, + * reopen the database connection, and retry the tool call once. + * + * These tests follow TDD vertical slices — each test drives one piece + * of implementation through the public interface. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { isSqliteCorruptionError } from '../src/db/error-detection'; +import CodeGraph from '../src/index'; +import { ToolHandler } from '../src/mcp/tools'; + +describe('isSqliteCorruptionError', () => { + it('matches "database disk image is malformed"', () => { + const err = new Error('database disk image is malformed'); + expect(isSqliteCorruptionError(err)).toBe(true); + }); + + it('matches "file is not a database"', () => { + const err = new Error('file is not a database'); + expect(isSqliteCorruptionError(err)).toBe(true); + }); + + it('matches "SQLITE_CORRUPT" in error message', () => { + const err = new Error('SQLITE_CORRUPT: some internal detail'); + expect(isSqliteCorruptionError(err)).toBe(true); + }); + + it('does not match non-corruption errors', () => { + expect(isSqliteCorruptionError(new Error('database is locked'))).toBe(false); + expect(isSqliteCorruptionError(new Error('no such table: nodes'))).toBe(false); + expect(isSqliteCorruptionError(new Error('connection timeout'))).toBe(false); + }); + + it('handles non-Error values gracefully', () => { + expect(isSqliteCorruptionError(null)).toBe(false); + expect(isSqliteCorruptionError(undefined)).toBe(false); + expect(isSqliteCorruptionError('database disk image is malformed')).toBe(true); + expect(isSqliteCorruptionError(42)).toBe(false); + }); +}); + +describe('CodeGraph.reopen()', () => { + let testDir: string; + let cg: CodeGraph; + + beforeEach(() => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'zcodegraph-reopen-')); + fs.mkdirSync(path.join(testDir, 'src')); + fs.writeFileSync( + path.join(testDir, 'src', 'survivor.ts'), + 'export function survivor() { return 1; }\n', + ); + cg = CodeGraph.initSync(testDir, { config: { include: ['**/*.ts'], exclude: [] } } as any); + }); + + afterEach(() => { + try { cg.unwatch(); } catch { /* ignore */ } + try { cg.close(); } catch { /* ignore */ } + if (fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true }); + }); + + it('serves queries normally after reopen', async () => { + await cg.indexAll({ engine: 'typescript' }); + + // Verify search works before reopen + const before = cg.searchNodes('survivor'); + expect(before.length).toBeGreaterThan(0); + + // Reopen — closes the old DB handle, opens a fresh one + cg.reopen(); + + // Search must still work with the new connection + const after = cg.searchNodes('survivor'); + expect(after.length).toBeGreaterThan(0); + expect(after[0].node.name).toBe('survivor'); + }); +}); + +describe('ToolHandler lazy recovery', () => { + let testDir: string; + let cg: CodeGraph; + let handler: ToolHandler; + + beforeEach(() => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'zcodegraph-lazy-recovery-')); + fs.mkdirSync(path.join(testDir, 'src')); + fs.writeFileSync( + path.join(testDir, 'src', 'survivor.ts'), + 'export function survivor() { return 1; }\n', + ); + cg = CodeGraph.initSync(testDir, { config: { include: ['**/*.ts'], exclude: [] } } as any); + handler = new ToolHandler(cg); + }); + + afterEach(() => { + try { cg.unwatch(); } catch { /* ignore */ } + try { cg.close(); } catch { /* ignore */ } + if (fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true }); + }); + + it('reopens and retries when a SQLite corruption error occurs', async () => { + await cg.indexAll({ engine: 'typescript' }); + + // Spy on searchNodes: throw once with corruption error, then work + let searchCallCount = 0; + const realSearchNodes = cg.searchNodes.bind(cg); + cg.searchNodes = (...args: Parameters) => { + searchCallCount++; + if (searchCallCount === 1) { + throw new Error('database disk image is malformed'); + } + return realSearchNodes(...args); + }; + + // Spy on reopen to verify it's called + const reopenSpy = vi.spyOn(cg, 'reopen'); + + const res = await handler.execute('zcodegraph_search', { query: 'survivor' }); + + expect(reopenSpy).toHaveBeenCalledTimes(1); + expect(res.isError).toBeFalsy(); + expect(res.content[0].text).toMatch(/survivor/); + }); + + it('does not retry for non-corruption errors', async () => { + await cg.indexAll({ engine: 'typescript' }); + + // Spy on searchNodes: throw a non-corruption error + cg.searchNodes = () => { + throw new Error('no such table: nodes'); + }; + + const reopenSpy = vi.spyOn(cg, 'reopen'); + + const res = await handler.execute('zcodegraph_search', { query: 'survivor' }); + + expect(reopenSpy).not.toHaveBeenCalled(); + expect(res.isError).toBe(true); + expect(res.content[0].text).toMatch(/no such table/); + }); + + it('does not retry more than once', async () => { + await cg.indexAll({ engine: 'typescript' }); + + // searchNodes always throws corruption error, even after reopen + cg.searchNodes = () => { + throw new Error('database disk image is malformed'); + }; + + const reopenSpy = vi.spyOn(cg, 'reopen'); + + const res = await handler.execute('zcodegraph_search', { query: 'survivor' }); + + // reopen called once, but error persists → return error, no second retry + expect(reopenSpy).toHaveBeenCalledTimes(1); + expect(res.isError).toBe(true); + expect(res.content[0].text).toMatch(/malformed/); + }); +}); diff --git a/docs/plans/issue-679-malformed-recovery.json b/docs/plans/issue-679-malformed-recovery.json new file mode 100644 index 00000000..7c3b3902 --- /dev/null +++ b/docs/plans/issue-679-malformed-recovery.json @@ -0,0 +1,84 @@ +{ + "title": "Issue #679: MCP SQLite stale connection recovery (lazy detection)", + "description": "CLI zcodegraph index rebuilds DB → MCP holds stale SQLite handle → database disk image is malformed. Fix: lazy detection in ToolHandler.execute() catch block, match malformed/SQLITE_CORRUPT, reopen+retry once.", + "version": 1, + "nodes": { + "1": { + "id": "1", + "label": "Issue #679: MCP SQLite stale connection recovery (lazy detection)", + "status": "in_progress", + "mode": "explore", + "parent": null, + "children": [ + "1-1", + "1-2", + "1-3", + "1-4", + "1-5" + ], + "decisions": [ + { + "q": "检测策略:MCP 应该何时发现数据库已损坏?", + "answer": "A. 惰性检测 — 在 ToolHandler.execute() catch 中匹配 malformed/SQLITE_CORRUPT,触发 reopen+retry", + "note": "DB 损坏是低频事件,不值得每次请求加 integrity_check。惰性检测在 catch 中匹配后触发一次 reopen+retry,对用户透明。" + } + ], + "notes": "" + }, + "1-1": { + "id": "1-1", + "label": "添加 SQLite 损坏错误检测工具函数", + "status": "completed", + "mode": "exploit", + "parent": "1", + "children": [], + "decisions": [], + "notes": "位置:src/utils.ts 或新建 src/db/error-detection.ts。匹配模式:'database disk image is malformed' | 'SQLITE_CORRUPT' | 'file is not a database'。返回 boolean。导出 isSqliteCorruptionError(err: unknown): boolean。" + }, + "1-2": { + "id": "1-2", + "label": "CodeGraph 添加 reopen() 公开方法关闭旧连接并重建内部对象", + "status": "completed", + "mode": "exploit", + "parent": "1", + "children": [], + "decisions": [], + "notes": "CodeGraph 已有私有方法 closeDatabaseForExternalIndex() + reopenDatabaseAfterExternalIndex()(src/index.ts:1145-1165),用于 Rust indexer 路径。提取为公开 reopen() 方法:unwatch → close db → reopen db → 重建 queries/orchestrator/resolver/graphManager/traverser/contextBuilder → restart watcher。复用现有逻辑,避免重复。" + }, + "1-3": { + "id": "1-3", + "label": "ToolHandler.execute 添加惰性恢复与一次性重试逻辑", + "status": "completed", + "mode": "exploit", + "parent": "1", + "children": [], + "decisions": [], + "notes": "在 execute() catch 块(tools.ts:974)中:1) 检测 isSqliteCorruptionError(err) 2) 若匹配,调用 getCodeGraph(path).reopen() 3) 标记已重试(防止无限循环)4) 重新执行 switch 分支。需要将 switch 逻辑提取为 executeOnce() 内部方法,execute() 包装重试。" + }, + "1-4": { + "id": "1-4", + "label": "TDD 测试验证 malformed 错误触发恢复与重试", + "status": "completed", + "mode": "exploit", + "parent": "1", + "children": [], + "decisions": [], + "notes": "测试文件:__tests__/malformed-recovery.test.ts。场景:1) mock CodeGraph 抛出 malformed 错误 → 验证 reopen 被调用 + 重试成功 2) 非 malformed 错误 → 不触发 reopen 3) reopen 后仍失败 → 返回错误(不二次重试)4) isSqliteCorruptionError 单元测试覆盖各种错误消息变体。" + }, + "1-5": { + "id": "1-5", + "label": "端到端验证与 PR 提交", + "status": "in_progress", + "mode": "explore", + "parent": "1", + "children": [], + "decisions": [], + "notes": "" + } + }, + "metadata": { + "created": "2026-07-13 23:04:04", + "updated": "2026-07-13 23:19:04", + "md_file": "D:/workspace/github/jununfly/ZCodeGraph/docs/plans/issue-679-malformed-recovery.md" + } +} \ No newline at end of file diff --git a/docs/plans/issue-679-malformed-recovery.md b/docs/plans/issue-679-malformed-recovery.md new file mode 100644 index 00000000..e771ecd1 --- /dev/null +++ b/docs/plans/issue-679-malformed-recovery.md @@ -0,0 +1,14 @@ + +## ZJ Roadmap + +> 数据文件: `issue-679-malformed-recovery.json` | 最后更新: 2026-07-13 23:19:04 + +[~][X+] 1. Issue #679: MCP SQLite stale connection recovery (lazy detection) +├── [x][Y+] 1-1. 添加 SQLite 损坏错误检测工具函数 +├── [x][Y+] 1-2. CodeGraph 添加 reopen() 公开方法关闭旧连接并重建内部对象 +├── [x][Y+] 1-3. ToolHandler.execute 添加惰性恢复与一次性重试逻辑 +├── [x][Y+] 1-4. TDD 测试验证 malformed 错误触发恢复与重试 +└── [~][X+] 1-5. 端到端验证与 PR 提交 + +### 当前施工:1-5. 端到端验证与 PR 提交 + diff --git a/src/db/error-detection.ts b/src/db/error-detection.ts new file mode 100644 index 00000000..d3b3642a --- /dev/null +++ b/src/db/error-detection.ts @@ -0,0 +1,41 @@ +/** + * SQLite corruption error detection + * + * Used by {@link ToolHandler.execute} to detect stale-connection errors + * after CLI `zcodegraph index` rebuilds the database file out from under + * the MCP server's long-lived CodeGraph instance. When such an error is + * detected, the handler can reopen the connection and retry the tool call. + */ + +/** + * Error message substrings that indicate SQLite database corruption or + * a stale file handle — the DB file was replaced/rebuilt while the + * connection was still open. + * + * Sources: + * - `database disk image is malformed` — SQLite SQLITE_CORRUPT + * - `file is not a database` — SQLite SQLITE_NOTADB + * - `SQLITE_CORRUPT` — raw SQLite error code name (some Node bindings) + */ +const CORRUPTION_PATTERNS: readonly string[] = [ + 'database disk image is malformed', + 'file is not a database', + 'SQLITE_CORRUPT', +]; + +/** + * Returns `true` when the given error looks like a SQLite corruption or + * stale-handle error — the kind that happens when `zcodegraph index` + * rebuilds the `.codegraph/codegraph.db` file while the MCP server's + * `CodeGraph` instance still holds the old connection. + * + * @param err — any caught error (may be non-Error) + */ +export function isSqliteCorruptionError(err: unknown): boolean { + if (err instanceof Error) { + const msg = err.message.toLowerCase(); + return CORRUPTION_PATTERNS.some((p) => msg.includes(p.toLowerCase())); + } + const str = String(err ?? '').toLowerCase(); + return CORRUPTION_PATTERNS.some((p) => str.includes(p.toLowerCase())); +} diff --git a/src/index.ts b/src/index.ts index 15b74fcf..7d283734 100644 --- a/src/index.ts +++ b/src/index.ts @@ -944,6 +944,28 @@ export class CodeGraph { this.db.close(); } + /** + * Close the stale SQLite connection and reopen a fresh one to the same + * database file, rebuilding all internal objects that hold references to + * the old connection. + * + * Used by {@link ToolHandler.execute} when it detects a SQLite corruption + * error (Issue #679): after CLI `zcodegraph index` rebuilds the `.codegraph/ + * codegraph.db` file out from under the MCP server's long-lived + * `CodeGraph` instance, the old connection is stale. Calling `reopen()` + * picks up the new file without restarting the process. + * + * The file watcher is stopped before closing (so it doesn't try to write + * to a closing handle) and NOT restarted — the caller is responsible for + * restarting it if needed. The file lock is NOT released — the MCP server + * still owns it. + */ + reopen(): void { + this.unwatch(); + this.db.close(); + this.reopenDatabaseAfterExternalIndex(); + } + /** * Get the project root directory */ diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index ff0d27d1..685ad4ae 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -26,6 +26,7 @@ import { existsSync, } from 'fs'; import { clamp, validateProjectPath } from '../utils'; +import { isSqliteCorruptionError } from '../db/error-detection'; import { isGeneratedFile } from '../extraction/generated-detection'; import { resolve as resolvePath } from 'path'; import type { ExploreOutputBudget } from './explore-types.js'; @@ -897,83 +898,114 @@ export class ToolHandler { } /** - * Execute a tool by name + * Execute a tool by name. + * + * Wraps {@link executeOnce} with lazy SQLite corruption recovery (Issue #679): + * if the first attempt throws a corruption error — typically because CLI + * `zcodegraph index` rebuilt the DB file under the MCP server's long-lived + * connection — the handler reopens the database and retries exactly once. + * Non-corruption errors and second-attempt failures are returned as-is. */ async execute(toolName: string, args: Record): Promise { try { - // Block the first tool call on the engine's post-open reconcile so we - // never serve rows for files deleted/edited while no MCP server was - // running. The gate is cleared after first await — subsequent calls - // pay nothing. Catch-up failures are logged by the engine; we - // proceed regardless so a transient sync error never breaks tools. - // - // Only gate the DEFAULT project (no explicit projectPath). Cross-project - // queries open CodeGraph on demand without a watcher or catch-up sync, - // so there is no gate to wait for (Issue #5: isolate MCP project state). - if (!args.projectPath && this.catchUpGate) { - const gate = this.catchUpGate; - this.catchUpGate = null; - try { await gate; } catch { /* engine already logged */ } - } - // Honor the optional tool allowlist (CODEGRAPH_MCP_TOOLS): a trimmed - // surface rejects ablated tools defensively even if a client cached them. - if (!this.isToolAllowed(toolName)) { - return this.errorResult(`Tool ${toolName} is disabled via CODEGRAPH_MCP_TOOLS`); - } - // Cross-cutting input validation. All tools accept an optional - // `projectPath` and most accept either `query`, `task`, or - // `symbol` — bound their lengths centrally so individual handlers - // can stay focused on tool-specific logic. - const pathCheck = this.validateOptionalPath(args.projectPath, 'projectPath'); - if (typeof pathCheck === 'object' && pathCheck !== undefined) { - return pathCheck; + return await this.executeOnce(toolName, args); + } catch (err) { + if (!isSqliteCorruptionError(err)) { + return this.errorResult(`Tool execution failed: ${err instanceof Error ? err.message : String(err)}`); } - // The `path` and `pattern` properties used by zcodegraph_files are - // also path-shaped — apply the same cap. - if (args.path !== undefined) { - const check = this.validateOptionalPath(args.path, 'path'); - if (typeof check === 'object' && check !== undefined) return check; + // Corruption detected — reopen the DB connection and retry once. + let cg: CodeGraph; + try { + cg = this.getCodeGraph(args.projectPath as string | undefined); + } catch { + return this.errorResult(`Tool execution failed: ${err instanceof Error ? err.message : String(err)}`); } - if (args.pattern !== undefined) { - const check = this.validateOptionalPath(args.pattern, 'pattern'); - if (typeof check === 'object' && check !== undefined) return check; + try { + cg.reopen(); + return await this.executeOnce(toolName, args); + } catch (retryErr) { + const msg = retryErr instanceof Error ? retryErr.message : String(retryErr); + return this.errorResult(`Tool execution failed: ${msg}`); } + } + } - // Read tools resolve through a single result variable so cross-cutting - // notices — worktree-index mismatch (issue #155) and per-file - // staleness (issue #403) — can be applied in one place. status embeds - // its own verbose worktree warning but still flows through the - // staleness wrapper so its pending-files section stays consistent - // with what the read tools surface. - let result: ToolResult; - switch (toolName) { - case 'zcodegraph_search': - result = await this.handleSearch(args); break; - case 'zcodegraph_callers': - result = await this.handleCallers(args); break; - case 'zcodegraph_callees': - result = await this.handleCallees(args); break; - case 'zcodegraph_impact': - result = await this.handleImpact(args); break; - case 'zcodegraph_explore': - result = await this.handleExplore(args); break; - case 'zcodegraph_node': - result = await this.handleNode(args); break; - case 'zcodegraph_status': - // status embeds the pending-files list as a first-class section - // (see handleStatus), so we skip the auto-banner wrapper here to - // avoid duplicating the same info at the top of the response. - return await this.handleStatus(args); - case 'zcodegraph_files': - result = await this.handleFiles(args); break; - default: - return this.errorResult(`Unknown tool: ${toolName}`); - } - const withWorktree = this.withWorktreeNotice(result, args.projectPath as string | undefined); - return this.withStalenessNotice(withWorktree, args.projectPath as string | undefined); - } catch (err) { - return this.errorResult(`Tool execution failed: ${err instanceof Error ? err.message : String(err)}`); + /** + * Single-attempt tool execution — gate, validation, dispatch, and + * cross-cutting notice wrappers. Called by {@link execute}; not intended + * for direct external use. + */ + private async executeOnce(toolName: string, args: Record): Promise { + // Block the first tool call on the engine's post-open reconcile so we + // never serve rows for files deleted/edited while no MCP server was + // running. The gate is cleared after first await — subsequent calls + // pay nothing. Catch-up failures are logged by the engine; we + // proceed regardless so a transient sync error never breaks tools. + // + // Only gate the DEFAULT project (no explicit projectPath). Cross-project + // queries open CodeGraph on demand without a watcher or catch-up sync, + // so there is no gate to wait for (Issue #5: isolate MCP project state). + if (!args.projectPath && this.catchUpGate) { + const gate = this.catchUpGate; + this.catchUpGate = null; + try { await gate; } catch { /* engine already logged */ } + } + // Honor the optional tool allowlist (CODEGRAPH_MCP_TOOLS): a trimmed + // surface rejects ablated tools defensively even if a client cached them. + if (!this.isToolAllowed(toolName)) { + return this.errorResult(`Tool ${toolName} is disabled via CODEGRAPH_MCP_TOOLS`); + } + // Cross-cutting input validation. All tools accept an optional + // `projectPath` and most accept either `query`, `task`, or + // `symbol` — bound their lengths centrally so individual handlers + // can stay focused on tool-specific logic. + const pathCheck = this.validateOptionalPath(args.projectPath, 'projectPath'); + if (typeof pathCheck === 'object' && pathCheck !== undefined) { + return pathCheck; + } + // The `path` and `pattern` properties used by zcodegraph_files are + // also path-shaped — apply the same cap. + if (args.path !== undefined) { + const check = this.validateOptionalPath(args.path, 'path'); + if (typeof check === 'object' && check !== undefined) return check; + } + if (args.pattern !== undefined) { + const check = this.validateOptionalPath(args.pattern, 'pattern'); + if (typeof check === 'object' && check !== undefined) return check; + } + + // Read tools resolve through a single result variable so cross-cutting + // notices — worktree-index mismatch (issue #155) and per-file + // staleness (issue #403) — can be applied in one place. status embeds + // its own verbose worktree warning but still flows through the + // staleness wrapper so its pending-files section stays consistent + // with what the read tools surface. + let result: ToolResult; + switch (toolName) { + case 'zcodegraph_search': + result = await this.handleSearch(args); break; + case 'zcodegraph_callers': + result = await this.handleCallers(args); break; + case 'zcodegraph_callees': + result = await this.handleCallees(args); break; + case 'zcodegraph_impact': + result = await this.handleImpact(args); break; + case 'zcodegraph_explore': + result = await this.handleExplore(args); break; + case 'zcodegraph_node': + result = await this.handleNode(args); break; + case 'zcodegraph_status': + // status embeds the pending-files list as a first-class section + // (see handleStatus), so we skip the auto-banner wrapper here to + // avoid duplicating the same info at the top of the response. + return await this.handleStatus(args); + case 'zcodegraph_files': + result = await this.handleFiles(args); break; + default: + return this.errorResult(`Unknown tool: ${toolName}`); } + const withWorktree = this.withWorktreeNotice(result, args.projectPath as string | undefined); + return this.withStalenessNotice(withWorktree, args.projectPath as string | undefined); } /**