From 231447984d0f4f65c96f95dea9ad4fd22424450a Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Tue, 27 Oct 2026 10:54:18 -0500 Subject: [PATCH] fix(cli): codegraph status opens the index read-only, and says when a rebuild is running Status opened the database read-write, so its open ran the writer's repairs: migrations, the FTS-trigger and secondary-index recovery an interrupted bulk load leaves behind, and WAL maintenance. During a `codegraph index`, that recovery recreated the indexes the indexer had dropped for its bulk load, and the indexer's next write, whose transaction had already read, failed with SQLITE_BUSY_SNAPSHOT (517): "Failed to index: database is locked". Status now opens with `readOnly: true` and leaves every repair to the writer, as the MCP reader connections already do. When another live process holds `rebuild.pid`, it says so: a warning in the text output and `rebuildInProgress` in `--json`, the only new key. `isRebuildInProgress` is the check `assertNoRebuild` already made. Added when landing it: - A read-only open migrates nothing, so status on an index built by 1.4.0 or older (schema 7, no `unresolved_refs.status`) failed with "no such column: status". The pending-reference count counts every row when the column is missing, as the v8 migration marks them pending. - While a rebuild runs, status skips the advice meant for an index at rest: the interrupted-run references, "run sync", "re-index", and the oversized-WAL warning, which mid-rebuild all describe the live run. - Status no longer reclaims a killed session's oversized WAL, so the warning names the next `codegraph sync` instead of status itself. - Tests: the issue's exact interleaving (status inside the writer's edge transaction, as in PR #2916's test), a pre-v8 index, and the mid-rebuild advice. Validation (Windows 11, Node 22.22): - The issue's reproduction with our build of main 69af19ac: status inside the transaction exits 0, the writer fails with errcode 517 and the five dropped edge indexes come back. With this change the insert succeeds and the indexes stay dropped. - Real rebuilds of excalidraw 95307fa1 (716 files) with `codegraph status` looping beside them, 3 with default settings and 3 with the resolution window forced: main failed 6 of 6, each with "store worker: database is locked" and an empty index; this change failed 0 of 6 over 46 status runs, with identical complete graphs (14,236 nodes, 61,210 edges). - status-during-rebuild.test.ts: 7/7 pass. With main's sources 6 of 7 fail; without `readOnly` 3 fail, one with "database is locked"; with the PR as written the pre-v8 and mid-rebuild cases fail. - Indexes built by v1.6.2 (schema 11) and v1.4.0 (schema 7): the same numbers as main, and the schema is left as it is. Interrupted-index warnings (indexing, partial, failed) are unchanged. Original PR #2921 by @danusha2345. Fixes #2913 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/status-during-rebuild.test.ts | 240 ++++++++++++++++++++++++ src/bin/codegraph.ts | 39 +++- src/db/queries.ts | 8 + src/mcp/writer-lock.ts | 9 +- 5 files changed, 287 insertions(+), 10 deletions(-) create mode 100644 __tests__/status-during-rebuild.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b4dfbb8af..c555c2ada8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - For Hermes Agent, `codegraph install` and `upgrade` no longer add `mcp-codegraph` to `platform_toolsets.cli`. Hermes up to 0.22.2 reported that name as an unknown toolset on every start, and Hermes 0.22.4 reads it as the only MCP server to load, hiding your others from CLI sessions; an entry an earlier install added is taken out again. A refresh also no longer resets your own `mcp_servers.codegraph` settings, such as an absolute command path, timeouts or `enabled: false`, and a config indented or written differently from Hermes's default no longer gets a duplicate or unreadable entry. Thanks @Huy3ko for the report and @conifercone for confirming it. (#2485) - `codegraph upgrade` now upgrades a global npm install when run from your home folder, which holds npm's global folder on Windows and with tools like nvm: before, it ran `npm install` without `-g` there, which left the global copy on the old version and put a second copy in your home folder. A copy installed in a project is now upgraded in that project wherever you run the command, with a warning if npm left it on the old version, and a copy installed with pnpm, yarn or bun gets the command that upgrades it instead of npm. Thanks @timlincool for the report and @maxmilian for the first fix. (#2477) - `codegraph status` no longer says the index is up to date while indexed files are missing their symbols: it now names files the parser couldn't read and files stored without their symbols (which `codegraph sync` repairs), `status --json` counts both, and `codegraph files --json` lists each file's recorded errors. Thanks @lipchey for the report. (#2336) +- Running `codegraph status` while `codegraph index` rebuilds a project no longer makes the rebuild fail with "database is locked": status now only reads the index, and it says when a rebuild is running in another process, in its output and as `rebuildInProgress` in `status --json`. Thanks @sealad886 for the report, and @danusha2345 and @KissFeng for the fixes. (#2913) - In CFML, a `` or `` block the parser can't read no longer marks its whole file as contributing nothing to the graph: `codegraph init` no longer warns about the file, and `codegraph status` no longer lists it among files whose symbols are missing, since its component, functions and calls are all indexed. The file's recorded errors now name the block and its line instead. Thanks @HarryMuc for the report. (#2654) - In Erlang, a comment inside an argument or parameter list, like the one in `add_to_includes( % give access to .hrl in app's src/` before its two arguments, no longer counts as an argument. The call now links to the function of its real arity, and a function whose parameter list holds a comment keeps its own arity. Before, the comment raised the arity by one, so the call linked to nothing. Re-index Erlang projects after upgrading. Thanks @Dshuishui. (#1619) - Indexing large Python projects is much faster again and needs less memory: since 1.6.2, resolving Python references re-read source files over and over, so a project the size of CPython took several times as long to index. The graph it builds is unchanged. Thanks @bompus for the report. (#2332) diff --git a/__tests__/status-during-rebuild.test.ts b/__tests__/status-during-rebuild.test.ts new file mode 100644 index 0000000000..95b45ebe45 --- /dev/null +++ b/__tests__/status-during-rebuild.test.ts @@ -0,0 +1,240 @@ +/** + * `codegraph status` while another process rebuilds the index (#2913). + * + * Status opened the database read-write, and the open ran the writer's + * repairs: during a rebuild's bulk load it recreated the secondary edge + * indexes the indexer had dropped, so the indexer's next edge write failed + * with SQLITE_BUSY_SNAPSHOT ("database is locked"). Status now opens read-only + * and leaves the dropped indexes alone, and it says that a rebuild is running. + * + * The CLI runs against the built binary, as the issue's reproduction did. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { DatabaseConnection } from '../src/db'; +import { QueryBuilder } from '../src/db/queries'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +function runStatus(cwd: string, json: boolean): string { + return execFileSync(process.execPath, [BIN, 'status', ...(json ? ['--json'] : [])], { + cwd, + encoding: 'utf-8', + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1' }, + stdio: ['ignore', 'pipe', 'pipe'], + }); +} + +function statusJson(cwd: string): Record { + const line = runStatus(cwd, true).trim().split('\n').filter(Boolean).pop()!; + return JSON.parse(line); +} + +/** The secondary indexes on `edges`, the ones a bulk edge load drops. */ +function edgeIndexes(conn: DatabaseConnection): string[] { + return (conn.getDb().prepare( + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'edges' AND sql IS NOT NULL" + ).all() as Array<{ name: string }>).map((r) => r.name).sort(); +} + +describe('codegraph status during an index rebuild (#2913)', () => { + let dir: string; + let dbPath: string; + + beforeEach(async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-status-rebuild-')); + fs.writeFileSync(path.join(dir, 'a.ts'), 'export function a() { return b(); }\nexport function b() { return 1; }\n'); + const cg = CodeGraph.initSync(dir); + await cg.indexAll(); + cg.close(); + dbPath = path.join(dir, '.codegraph', 'codegraph.db'); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + /** Both locks name this process, as `codegraph index` writes them. */ + function holdRebuildLocks(pid: number): void { + const lock = JSON.stringify({ pid, mode: 'rebuild', startedAt: Date.now(), ready: false }); + fs.writeFileSync(path.join(dir, '.codegraph', 'rebuild.pid'), lock); + fs.writeFileSync(path.join(dir, '.codegraph', 'writer.pid'), lock); + } + + it('leaves the indexes a bulk load dropped alone, so the writer goes on writing', async () => { + const writer = DatabaseConnection.open(dbPath); + try { + const complete = edgeIndexes(writer); + writer.beginBulkEdgeLoad(); + const dropped = edgeIndexes(writer); + expect(dropped.length).toBeLessThan(complete.length); + holdRebuildLocks(process.pid); + + const status = statusJson(dir); + expect(status.initialized).toBe(true); + expect(status.nodeCount).toBeGreaterThan(0); + expect(status.rebuildInProgress).toBe(true); + // Status repaired nothing: the bulk load's indexes are still down. + expect(edgeIndexes(writer)).toEqual(dropped); + + // The indexer's next edge write, as it happens after status returned. + const ids = (writer.getDb().prepare('SELECT id FROM nodes WHERE kind = ? ORDER BY start_line').all('function') as Array<{ id: string }>).map((r) => r.id); + expect(ids.length).toBe(2); + const queries = new QueryBuilder(writer.getDb()); + expect(() => queries.insertEdges([{ source: ids[0]!, target: ids[1]!, kind: 'references' }])).not.toThrow(); + await writer.endBulkEdgeLoad(); + expect(edgeIndexes(writer)).toEqual(complete); + } finally { + writer.close(); + } + }); + + it('a read-write open still recreates them: the repair stays with the writer', () => { + const writer = DatabaseConnection.open(dbPath); + try { + const complete = edgeIndexes(writer); + writer.beginBulkEdgeLoad(); + expect(edgeIndexes(writer).length).toBeLessThan(complete.length); + const other = DatabaseConnection.open(dbPath); + other.close(); + expect(edgeIndexes(writer)).toEqual(complete); + } finally { + writer.close(); + } + }); + + it('says a rebuild is running, and not when its lock belongs to a dead process', () => { + holdRebuildLocks(process.pid); + expect(runStatus(dir, false)).toContain('An index rebuild ("codegraph index") is running in another process'); + + // A pid no process has: the lock is stale, not a rebuild. + holdRebuildLocks(2_147_483_000); + expect(statusJson(dir).rebuildInProgress).toBe(false); + expect(runStatus(dir, false)).not.toContain('is running in another process'); + }); + + it('reports a project with no rebuild as before', () => { + const status = statusJson(dir); + expect(status.rebuildInProgress).toBe(false); + expect(status.nodeCount).toBeGreaterThan(0); + expect(runStatus(dir, false)).toContain('Index Statistics'); + }); + + it('runs inside the writer\'s edge transaction without failing its insert', () => { + // The issue's interleaving: the writer reads its endpoints inside the + // transaction, status runs in another process, then the writer inserts. + // Read-write, status recreated the dropped indexes in between and the + // insert failed with SQLITE_BUSY_SNAPSHOT (517), "database is locked". + const writer = DatabaseConnection.open(dbPath); + try { + writer.beginBulkEdgeLoad(); + holdRebuildLocks(process.pid); + const db = writer.getDb(); + const ids = (db.prepare('SELECT id FROM nodes WHERE kind = ? ORDER BY start_line').all('function') as Array<{ id: string }>).map((r) => r.id); + const edgeCount = (): number => (db.prepare('SELECT COUNT(*) AS n FROM edges').get() as { n: number }).n; + const before = edgeCount(); + const queries = new QueryBuilder(db); + // The endpoint read is private; hook it as the issue's reproduction did. + const hooked = queries as unknown as { getExistingNodeIds: (ids: readonly string[]) => Set }; + const readEndpoints = hooked.getExistingNodeIds.bind(queries); + let status: Record | undefined; + hooked.getExistingNodeIds = (endpointIds) => { + const existing = readEndpoints(endpointIds); + status = statusJson(dir); + return existing; + }; + expect(() => queries.insertEdges([{ source: ids[0]!, target: ids[1]!, kind: 'references' }])).not.toThrow(); + expect(status?.rebuildInProgress).toBe(true); + expect(edgeCount()).toBe(before + 1); + } finally { + writer.close(); + } + }); + + it('gives no advice meant for an index at rest while a rebuild runs', () => { + // Mid-rebuild, pending references are the run's own work and the version + // stamps are not written yet: "run sync" or "re-index" would be advice + // against the rebuild itself. + const conn = DatabaseConnection.open(dbPath); + try { + const db = conn.getDb(); + const from = (db.prepare("SELECT id FROM nodes WHERE kind = 'function' LIMIT 1").get() as { id: string }).id; + db.prepare( + "INSERT INTO unresolved_refs (from_node_id, reference_name, reference_kind, line, col, file_path, language) VALUES (?, 'later', 'calls', 1, 0, 'a.ts', 'typescript')" + ).run(from); + db.exec("DELETE FROM project_metadata WHERE key IN ('indexed_with_version', 'indexed_with_extraction_version')"); + } finally { + conn.close(); + } + + holdRebuildLocks(process.pid); + const during = runStatus(dir, false); + expect(during).toContain('is running in another process'); + expect(during).toContain('Index Statistics'); + expect(during).not.toContain('from an interrupted run'); + expect(during).not.toContain('Run "codegraph'); + + // The same index at rest, its rebuild lock left by a dead process. + holdRebuildLocks(2_147_483_000); + const atRest = runStatus(dir, false); + expect(atRest).toContain('1 references from an interrupted run are awaiting resolution'); + expect(atRest).toContain('re-index to pick up'); + }); +}); + +describe('codegraph status on an index an older release built (#2913)', () => { + let dir: string; + let dbPath: string; + + beforeEach(async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-status-legacy-')); + fs.writeFileSync(path.join(dir, 'a.ts'), 'export function a() { return b(); }\nexport function b() { return 1; }\n'); + const cg = CodeGraph.initSync(dir); + await cg.indexAll(); + cg.close(); + dbPath = path.join(dir, '.codegraph', 'codegraph.db'); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('reads one from before schema v8 as it is, without upgrading it', () => { + // Up to 1.4.0 a reference carried no status: the v8 migration marks every + // row pending. Status opens read-only, so it reads the index unmigrated. + const conn = DatabaseConnection.open(dbPath); + try { + const db = conn.getDb(); + const from = (db.prepare("SELECT id FROM nodes WHERE kind = 'function' LIMIT 1").get() as { id: string }).id; + const onStatus = db.prepare( + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'unresolved_refs' AND (sql LIKE '%status%' OR sql LIKE '%name_tail%')" + ).all() as Array<{ name: string }>; + for (const { name } of onStatus) db.exec(`DROP INDEX ${name}`); + db.exec('ALTER TABLE unresolved_refs DROP COLUMN status'); + db.exec('ALTER TABLE unresolved_refs DROP COLUMN name_tail'); + // A reference an interrupted run left behind. + db.prepare( + "INSERT INTO unresolved_refs (from_node_id, reference_name, reference_kind, line, col, file_path, language) VALUES (?, 'missing', 'calls', 1, 0, 'a.ts', 'typescript')" + ).run(from); + db.exec("DELETE FROM schema_versions; INSERT INTO schema_versions (version, applied_at, description) VALUES (7, 0, 'legacy')"); + } finally { + conn.close(); + } + + const status = statusJson(dir); + expect(status.nodeCount).toBeGreaterThan(0); + expect((status.index as { pendingRefs: number }).pendingRefs).toBe(1); + expect(runStatus(dir, false)).toContain('1 references from an interrupted run are awaiting resolution'); + + const after = DatabaseConnection.open(dbPath, { readOnly: true }); + try { + expect(after.getSchemaVersion()?.version).toBe(7); + } finally { + after.close(); + } + }); +}); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 95a291a565..7719497eff 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -1123,7 +1123,17 @@ program } const { default: CodeGraph } = await loadCodeGraph(); - const cg = await CodeGraph.open(projectPath); + // Status only reads. Opened read-write, it ran the open-time repairs + // meant for the writer: during a rebuild's bulk load it recreated the + // secondary indexes the indexer had dropped, and the indexer's next + // edge write failed with SQLITE_BUSY_SNAPSHOT, "database is locked" + // (#2913). Read-only, it leaves migrations, index repair and WAL + // maintenance to the writer. + const cg = await CodeGraph.open(projectPath, { readOnly: true }); + const { isRebuildInProgress } = await import('../mcp/writer-lock'); + // A `codegraph index` in another process owns the database: the + // numbers below are a snapshot of a graph being rebuilt. + const rebuildInProgress = isRebuildInProgress(projectPath); const stats = cg.getStats(); const changes = cg.getChangedFiles(); const backend = cg.getBackend(); @@ -1165,6 +1175,9 @@ program worktreeMismatch: worktreeMismatch ? { worktreeRoot: worktreeMismatch.worktreeRoot, indexRoot: worktreeMismatch.indexRoot } : null, + // True while another process runs `codegraph index` on this + // project: the statistics are a snapshot of the rebuild (#2913). + rebuildInProgress, index: { builtWithVersion: buildInfo.version, builtWithExtractionVersion: buildInfo.extractionVersion, @@ -1197,14 +1210,18 @@ program if (worktreeMismatch) { warn(worktreeMismatchWarning(worktreeMismatch)); } - if (indexState === 'indexing') { + if (rebuildInProgress) { + warn('An index rebuild ("codegraph index") is running in another process — the statistics below are a snapshot of it.'); + } else if (indexState === 'indexing') { warn('The last index run never finished (killed mid-index?) — the index is truncated. Re-run "codegraph index".'); } else if (indexState === 'partial') { warn('The last index run silently dropped files — the index is partial. Re-run "codegraph index".'); } else if (indexState === 'failed') { warn('The last index run failed — results may be incomplete. Re-run "codegraph index".'); } - if (pendingRefs > 0) { + // A running rebuild's references are its own work in progress, not an + // interrupted run's. + if (pendingRefs > 0 && !rebuildInProgress) { warn(`${formatNumber(pendingRefs)} references from an interrupted run are awaiting resolution — some callers/impact edges are missing. Run "codegraph sync" to resolve them.`); } console.log(); @@ -1217,15 +1234,16 @@ program console.log(` DB Size: ${(stats.dbSizeBytes / 1024 / 1024).toFixed(2)} MB`); // Surface the WAL sidecar (#1431): a WAL that dwarfs the DB at rest is // the killed-session leak — invisible before this line, it only showed - // up as a mysteriously full disk. open() above already kicked off the - // automatic heal for the oversized case. + // up as a mysteriously full disk. The read-only open above leaves the + // heal to the next writer's open, such as a sync (#2913). if (stats.walSizeBytes > 0) { const { WAL_HEAL_THRESHOLD_BYTES } = await import('../db/index'); const oversized = stats.walSizeBytes > Math.max(WAL_HEAL_THRESHOLD_BYTES, stats.dbSizeBytes); const walLabel = `${(stats.walSizeBytes / 1024 / 1024).toFixed(2)} MB`; console.log(` WAL Size: ${oversized ? chalk.yellow(walLabel) : walLabel}`); - if (oversized) { - warn('The write-ahead log is larger than the database — killed sessions left it behind. It is reclaimed automatically on open; if it persists across runs, another live CodeGraph process is holding it.'); + // A running rebuild's WAL is its own, not a killed session's. + if (oversized && !rebuildInProgress) { + warn('The write-ahead log is larger than the database — killed sessions left it behind. The next "codegraph sync" reclaims it; if it persists after that, another live CodeGraph process is holding it.'); } } // Surface the active SQLite backend (node:sqlite — Node's built-in real @@ -1262,6 +1280,13 @@ program } console.log(); + // What follows is advice for an index at rest: sync its pending changes, + // re-index an older build. A running rebuild is replacing this one. + if (rebuildInProgress) { + cg.destroy(); + return; + } + // Pending changes const totalChanges = changes.added.length + changes.modified.length + changes.removed.length; if (totalChanges > 0) { diff --git a/src/db/queries.ts b/src/db/queries.ts index fa237656e5..3377e288ed 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -4134,6 +4134,14 @@ export class QueryBuilder { */ getUnresolvedReferencesCount(): number { if (!this.stmts.getUnresolvedCount) { + // An index built before schema v8 (1.4.0 and older) has no `status`, + // and the v8 migration marks every one of its rows pending. A read-only + // open migrates nothing (`codegraph status`, #2913), so count them all + // until a writer's open has migrated it. + const columns = this.db.prepare('PRAGMA table_info(unresolved_refs)').all() as Array<{ name: string }>; + if (!columns.some((c) => c.name === 'status')) { + return (this.db.prepare('SELECT COUNT(*) as count FROM unresolved_refs').get() as { count: number }).count; + } this.stmts.getUnresolvedCount = this.db.prepare( "SELECT COUNT(*) as count FROM unresolved_refs WHERE status = 'pending'" ); diff --git a/src/mcp/writer-lock.ts b/src/mcp/writer-lock.ts index 20f3e94e2c..d7b78f2351 100644 --- a/src/mcp/writer-lock.ts +++ b/src/mcp/writer-lock.ts @@ -258,8 +258,11 @@ export class RebuildInProgressError extends Error { } export function assertNoRebuild(root: string): void { + if (isRebuildInProgress(root)) throw new RebuildInProgressError(); +} + +/** Whether another live process holds the project's rebuild lock (`codegraph index`). */ +export function isRebuildInProgress(root: string): boolean { const lock = readWriterLock(root, 'rebuild.pid'); - if (lock && lock.pid !== process.pid && isProcessAlive(lock.pid)) { - throw new RebuildInProgressError(); - } + return !!lock && lock.pid !== process.pid && isProcessAlive(lock.pid); }