diff --git a/__tests__/cli-sync.test.ts b/__tests__/cli-sync.test.ts new file mode 100644 index 000000000..ed998a339 --- /dev/null +++ b/__tests__/cli-sync.test.ts @@ -0,0 +1,80 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +describe('codegraph sync lock contention (#1361)', () => { + let testDir: string; + + beforeEach(async () => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cli-sync-')); + fs.writeFileSync(path.join(testDir, 'index.ts'), 'export function original() { return 1; }'); + const cg = CodeGraph.initSync(testDir); + try { + await cg.indexAll(); + } finally { + cg.destroy(); + } + }); + + afterEach(() => { + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + function sync(quiet: boolean) { + return spawnSync(process.execPath, [BIN, 'sync', testDir, ...(quiet ? ['--quiet'] : [])], { + encoding: 'utf8', + timeout: 30_000, + env: { + ...process.env, + CODEGRAPH_TELEMETRY: '0', DO_NOT_TRACK: '1', + CODEGRAPH_NO_PROMPT_HOOK: '1', CODEGRAPH_NO_DAEMON: '1', + NODE_NO_WARNINGS: '1', NO_COLOR: '1', + }, + }); + } + + it.each([false, true])('reports contention and recovers after release (quiet=%s)', (quiet) => { + fs.writeFileSync(path.join(testDir, 'index.ts'), 'export function changedUnderLock() { return 2; }'); + const lockPath = path.join(testDir, '.codegraph', 'codegraph.lock'); + fs.writeFileSync(lockPath, String(process.pid)); + + const locked = sync(quiet); + expect(locked.error).toBeUndefined(); + expect(locked.status).toBe(1); + expect(locked.stdout + locked.stderr).not.toContain('Already up to date'); + if (quiet) { + expect(locked.stdout + locked.stderr).toBe(''); + } else { + expect(locked.stdout + locked.stderr).toMatch(/busy|lock/i); + expect(locked.stdout + locked.stderr).toMatch(/retry/i); + } + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(process.pid)); + const before = CodeGraph.openSync(testDir); + try { + expect(before.searchNodes('changedUnderLock')).toHaveLength(0); + } finally { + before.destroy(); + } + + fs.unlinkSync(lockPath); + const unlocked = sync(quiet); + expect(unlocked.error).toBeUndefined(); + expect(unlocked.status).toBe(0); + const after = CodeGraph.openSync(testDir); + try { + expect(after.searchNodes('changedUnderLock')).toHaveLength(1); + } finally { + after.destroy(); + } + + const unchanged = sync(quiet); + expect(unchanged.status).toBe(0); + if (quiet) expect(unchanged.stdout + unchanged.stderr).toBe(''); + else expect(unchanged.stdout).toContain('Already up to date'); + }); +}); diff --git a/__tests__/sync.test.ts b/__tests__/sync.test.ts index 455c2eecf..c3aa7936a 100644 --- a/__tests__/sync.test.ts +++ b/__tests__/sync.test.ts @@ -6,12 +6,13 @@ * Claude Code hooks integration. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +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 { execFileSync } from 'child_process'; -import CodeGraph from '../src/index'; +import CodeGraph, { LockUnavailableError } from '../src/index'; +import { __emitWatchEventForTests } from '../src/sync/watcher'; describe('Sync Module', () => { describe('Sync Functionality', () => { @@ -90,6 +91,58 @@ describe('Sync Module', () => { }); describe('sync()', () => { + it('rejects a live lock and applies the pending edit after release (#1361)', async () => { + const lockPath = path.join(testDir, '.codegraph', 'codegraph.lock'); + fs.writeFileSync(path.join(testDir, 'src', 'index.ts'), + 'export function changedUnderLock() { return 2; }'); + fs.writeFileSync(lockPath, String(process.pid)); + + await expect(cg.sync()).rejects.toBeInstanceOf(LockUnavailableError); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(process.pid)); + expect(cg.searchNodes('changedUnderLock')).toHaveLength(0); + + fs.unlinkSync(lockPath); + const result = await cg.sync(); + expect(result.filesModified).toBe(1); + expect(cg.searchNodes('changedUnderLock')).toHaveLength(1); + }); + + it('open with sync rejects contention without removing the held lock (#1361)', async () => { + const lockPath = path.join(testDir, '.codegraph', 'codegraph.lock'); + fs.writeFileSync(lockPath, String(process.pid)); + await expect(CodeGraph.open(testDir, { sync: true })).rejects.toBeInstanceOf(LockUnavailableError); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(process.pid)); + }); + + it('watch retains pending edits under a live lock and retries after release (#1361)', async () => { + const lockPath = path.join(testDir, '.codegraph', 'codegraph.lock'); + const sync = vi.spyOn(cg, 'sync'); + const onSyncComplete = vi.fn(); + const onSyncError = vi.fn(); + try { + cg.watch({ inertForTests: true, debounceMs: 20, onSyncComplete, onSyncError }); + fs.writeFileSync(path.join(testDir, 'src', 'index.ts'), + 'export function changedUnderLock() { return 2; }'); + fs.writeFileSync(lockPath, String(process.pid)); + __emitWatchEventForTests(testDir, 'src/index.ts'); + + await vi.waitFor(() => expect(sync).toHaveBeenCalled()); + await expect(sync.mock.results[0].value).rejects.toBeInstanceOf(LockUnavailableError); + expect(cg.getPendingFiles().map((f) => f.path)).toContain('src/index.ts'); + expect(onSyncComplete).not.toHaveBeenCalled(); + expect(onSyncError).not.toHaveBeenCalled(); + + fs.unlinkSync(lockPath); + await vi.waitFor(() => expect(onSyncComplete).toHaveBeenCalled(), { timeout: 5000 }); + expect(cg.getPendingFiles()).toHaveLength(0); + expect(cg.searchNodes('changedUnderLock')).toHaveLength(1); + expect(onSyncError).not.toHaveBeenCalled(); + } finally { + cg.unwatch(); + sync.mockRestore(); + } + }); + it('should reindex added files', async () => { // Add a new file fs.writeFileSync( diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 83fb2310a..29869e008 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -955,39 +955,39 @@ program const { default: CodeGraph } = await loadCodeGraph(); const cg = await CodeGraph.open(projectPath); - if (options.quiet) { - await cg.sync(); + try { + if (options.quiet) { + await cg.sync(); + return; + } + + const clack = await importESM('@clack/prompts'); + clack.intro('Syncing CodeGraph'); + + process.stdout.write(`${colors.dim}${getGlyphs().rail}${colors.reset}\n`); + const progress = createShimmerProgress(); + + const result = await cg.sync({ + onProgress: progress.onProgress, + }).finally(() => progress.stop()); + + const totalChanges = result.filesAdded + result.filesModified + result.filesRemoved; + + if (totalChanges === 0) { + clack.log.info('Already up to date'); + } else { + clack.log.success(`Synced ${formatNumber(totalChanges)} changed files`); + const details: string[] = []; + if (result.filesAdded > 0) details.push(`Added: ${result.filesAdded}`); + if (result.filesModified > 0) details.push(`Modified: ${result.filesModified}`); + if (result.filesRemoved > 0) details.push(`Removed: ${result.filesRemoved}`); + clack.log.info(`${details.join(', ')} ${getGlyphs().dash} ${formatNumber(result.nodesUpdated)} nodes in ${formatDuration(result.durationMs)}`); + } + + clack.outro('Done'); + } finally { cg.destroy(); - return; } - - const clack = await importESM('@clack/prompts'); - clack.intro('Syncing CodeGraph'); - - process.stdout.write(`${colors.dim}${getGlyphs().rail}${colors.reset}\n`); - const progress = createShimmerProgress(); - - const result = await cg.sync({ - onProgress: progress.onProgress, - }); - - await progress.stop(); - - const totalChanges = result.filesAdded + result.filesModified + result.filesRemoved; - - if (totalChanges === 0) { - clack.log.info('Already up to date'); - } else { - clack.log.success(`Synced ${formatNumber(totalChanges)} changed files`); - const details: string[] = []; - if (result.filesAdded > 0) details.push(`Added: ${result.filesAdded}`); - if (result.filesModified > 0) details.push(`Modified: ${result.filesModified}`); - if (result.filesRemoved > 0) details.push(`Removed: ${result.filesRemoved}`); - clack.log.info(`${details.join(', ')} ${getGlyphs().dash} ${formatNumber(result.nodesUpdated)} nodes in ${formatDuration(result.durationMs)}`); - } - - clack.outro('Done'); - cg.destroy(); } catch (err) { if (!options.quiet) { error(`Failed to sync: ${err instanceof Error ? err.message : String(err)}`); diff --git a/src/index.ts b/src/index.ts index c1226fc7f..ee5d88322 100644 --- a/src/index.ts +++ b/src/index.ts @@ -357,7 +357,12 @@ export class CodeGraph { // Sync if requested if (options.sync) { - await instance.sync(); + try { + await instance.sync(); + } catch (err) { + instance.destroy(); + throw err; + } } return instance; @@ -777,7 +782,8 @@ export class CodeGraph { } /** - * Sync with current file state (incremental update) + * Sync with current file state (incremental update). + * Throws LockUnavailableError if the cross-process write lock is unavailable. * * Uses a mutex to prevent concurrent indexing operations. */ @@ -785,8 +791,10 @@ export class CodeGraph { return this.indexMutex.withLock(async () => { try { this.fileLock.acquire(); - } catch { - return { filesChecked: 0, filesAdded: 0, filesModified: 0, filesRemoved: 0, nodesUpdated: 0, durationMs: 0 }; + } catch (err) { + throw new LockUnavailableError( + `Sync could not acquire the file lock; retry when the index is available. ${err instanceof Error ? err.message : String(err)}` + ); } // Defer WAL auto-checkpointing for the whole incremental run, exactly // as indexAll does for the bulk path (#1231): sync's store loop and its @@ -1080,14 +1088,6 @@ export class CodeGraph { this.projectRoot, async (paths?: string[]) => { const result = await this.sync({ paths }); - // sync() returns this exact zero-shape iff it failed to acquire the - // file lock (a real empty sync always has filesChecked > 0 because - // scanDirectory ran). Surface that to the watcher as a typed error - // so it keeps pendingFiles + reschedules instead of clearing them - // (#449). - if (result.filesChecked === 0 && result.durationMs === 0) { - throw new LockUnavailableError(); - } const filesChanged = result.filesAdded + result.filesModified + result.filesRemoved; return { filesChanged, durationMs: result.durationMs }; },