fix(sync): surface lock contention instead of reporting success (#1361)

Sync swallowed lock-acquisition failures and returned counters indistinguishable from a clean index.
Throw LockUnavailableError so the CLI reports failure and the watcher retains its retry behavior without inspecting counters.
Preserve quiet output and clean up resources when sync rejects.
Cover live locks, recovery, watcher retries, and CLI exit statuses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby McHenry
2026-09-27 08:03:04 -05:00
co-authored by Claude Opus 5.5
parent bd993b12fa
commit 55e2187090
4 changed files with 178 additions and 45 deletions
+80
View File
@@ -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');
});
});
+55 -2
View File
@@ -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(
+31 -31
View File
@@ -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)}`);
+12 -12
View File
@@ -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 };
},