fix(library): indexFiles keeps other writers out until its files are stored (#2845)

CodeGraph.indexFiles returned the orchestrator's indexing promise from
inside the try whose finally releases the cross-process write lock. A
returned promise leaves the try at once, so the finally ran as soon as
indexing started, and the lock was gone while the files were still being
extracted and stored. Another process's sync or index of the same
project, such as a git hook, the CLI or an MCP server, could then write to
the index at the same time. indexAll and sync await inside their try and
hold the lock to the end; the in-process mutex already covered the whole
call.

Fix: await the indexing inside the try, so the lock is let go only once
the files are stored, or when indexing fails.

Validation:
- New __tests__/index-files-write-lock.test.ts (2 cases). A second handle
  on the project stands in for another process, and the real indexing
  waits at a gate the test opens. While indexFiles runs, the other
  handle's sync must be refused with LockUnavailableError. With main's
  src/index.ts it ran to completion instead (red); with the fix it is
  refused, and admitted once the files are stored (green). When indexing
  fails, the lock is let go (passes on both).
- The lock and sync test files pass alongside: sync.test.ts,
  cli-sync.test.ts, cli-unlock.test.ts, orphaned-refs-sweep.test.ts
  and sync-rebuild-convergence.test.ts, the last two of which call
  indexFiles.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-23 13:30:13 +00:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 23eb392915
commit 865ddf4ada
3 changed files with 85 additions and 2 deletions
+1
View File
@@ -52,6 +52,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- Upgrading CodeGraph while an agent session is open no longer leaves the old version's background server in charge of your project: the first session started from the new install stops it and starts a current one in its place, even while sessions opened before the upgrade are still running. That old server could no longer load the language parsers the upgrade removed, so it saved every file it re-indexed with no symbols, while new sessions could only read the index beside it without keeping it up to date. A background server from a newer install is never stopped, and sessions opened before the upgrade keep the old version until you restart them. Thanks @lipchey for the report. (#2335)
- A file is no longer saved with no symbols when its language parser can't be loaded, which is what happened to every file a background server re-indexed after an upgrade removed its install: the file keeps what it had and is indexed again once the parser loads, and files an earlier version emptied this way are re-indexed by the next sync. A background server also exits on its own once its install is upgraded or removed, so the next session starts one from the current install. Thanks @lipchey for the report. (#2335)
- `codegraph index` no longer stops with advice to run `codegraph daemon stop`, a command that doesn't exist, when the project's background-server lock names a process that isn't CodeGraph, such as a lock left by a crash whose process number the system has since given to another program: it now clears that stale lock, leaving the process alone, and rebuilds, and `codegraph unlock` clears it too. When a CodeGraph process that doesn't answer holds the lock, such as an older background server, the message names that process and the command that stops it, and the message about another server already serving a project now points to `codegraph daemon`. Thanks @inth3shadows for the report and @danusha2345 for the first fix. (#2770)
- A program that uses CodeGraph as a library and indexes chosen files with `indexFiles()` now keeps other CodeGraph processes, like a `codegraph sync` run by a git hook, out of the index until those files are stored. Before, it let go of the write lock that keeps two writers apart as soon as indexing started, so another process could write to the index at the same time.
- The shared background server is no longer killed with "Main thread unresponsive" while it brings another project's index up to date, such as one a session names with `projectPath`: only its own project's index counted as progress, so a long update of any other looked like a hang, and the server was stopped and restarted again and again. Every index the server keeps up to date now counts, and so does the one a session updates on its own when the background server is unavailable. Thanks @mradwankhalil for the report and the first fix. (#2404)
- An index left incomplete by an indexing run that was stopped partway, by a crash, an out-of-memory kill or the server's own hang check, no longer gets the background server killed with "Main thread unresponsive" every time it starts: on a large project, restoring the database indexes the run had set aside took longer than the hang check allows, so each restart died at the same point and the index was never repaired. They are now restored in the background, and the project is served once they are back. Thanks @cbeaulieu-gt for the report and the first fix. (#1887)
- A new agent session no longer waits up to about 12 minutes for its first answer when the background server is stuck: a server whose process still holds its connection open but never responds made the session wait out a long series of retries. The session now gives up within seconds and answers on its own, and switches back to the background server once that has been restarted.
+79
View File
@@ -0,0 +1,79 @@
import { afterEach, describe, expect, it, vi } from 'vitest';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import CodeGraph, { LockUnavailableError } from '../src';
import type { ExtractionOrchestrator } from '../src/extraction';
/**
* `CodeGraph.indexFiles` holds the cross-process write lock until its files
* are stored, as `indexAll` and `sync` do: another process's sync or index of
* the same project is refused while it runs, and admitted after. It used to
* return the indexing promise from inside the `try` whose `finally` releases
* the lock, so the lock was gone as soon as indexing started. A second handle
* on the project stands in for the other process: the lock file names a live
* PID, this one, so its acquire refuses as another process's would.
*/
describe('indexFiles holds the write lock while it indexes', () => {
const dirs: string[] = [];
const graphs: CodeGraph[] = [];
afterEach(() => {
vi.restoreAllMocks();
for (const cg of graphs.splice(0)) cg.close();
for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true, maxRetries: 5 });
});
/**
* An indexed project whose one file was then edited, a gate its next
* indexFiles waits at before indexing, and a second handle on it.
*/
async function editedProjectWithGatedIndexing(fail = false) {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-index-files-lock-'));
dirs.push(dir);
fs.mkdirSync(path.join(dir, 'src'));
fs.writeFileSync(path.join(dir, 'src', 'a.ts'), 'export function before() { return 1; }\n');
const cg = CodeGraph.initSync(dir);
graphs.push(cg);
await cg.indexAll();
fs.writeFileSync(path.join(dir, 'src', 'a.ts'), 'export function after() { return 2; }\n');
const orchestrator = (cg as unknown as { orchestrator: ExtractionOrchestrator }).orchestrator;
const real = orchestrator.indexFiles.bind(orchestrator);
let open!: () => void;
const gate = new Promise<void>((resolve) => { open = resolve; });
const indexing = vi.spyOn(orchestrator, 'indexFiles').mockImplementation(async (filePaths) => {
await gate;
if (fail) throw new Error('extraction failed');
return real(filePaths);
});
const other = CodeGraph.openSync(dir);
graphs.push(other);
return { cg, other, indexing, open };
}
it('refuses another writer until the files are indexed', async () => {
const { cg, other, indexing, open } = await editedProjectWithGatedIndexing();
const run = cg.indexFiles(['src/a.ts']);
await vi.waitFor(() => expect(indexing).toHaveBeenCalled());
// Indexing is under way: another process's sync must wait its turn.
await expect(other.sync()).rejects.toBeInstanceOf(LockUnavailableError);
open();
const result = await run;
expect(result.errors).toEqual([]);
expect(result.success).toBe(true);
expect(cg.getNodesByName('after')).toHaveLength(1);
// Done: the next writer gets in.
await expect(other.sync()).resolves.toBeDefined();
});
it('lets the lock go when indexing fails', async () => {
const { cg, other, open } = await editedProjectWithGatedIndexing(true);
const run = cg.indexFiles(['src/a.ts']);
open();
await expect(run).rejects.toThrow('extraction failed');
await expect(other.sync()).resolves.toBeDefined();
});
});
+5 -2
View File
@@ -887,8 +887,11 @@ export class CodeGraph {
// The files' nodes are new to the resolver: a resolution after this
// reloads its caches, the set of node names included, as indexAll and
// sync do before they resolve, instead of reading the graph as it was
// before (#2829).
return this.orchestrator.indexFiles(filePaths).finally(() => this.resolver.clearCaches());
// before (#2829). Awaited, so the write lock is held until the files
// are stored, as indexAll and sync hold it: returned unawaited, the
// promise left the `try` at once and the `finally` let the lock go as
// soon as indexing started, so another process could write meanwhile.
return await this.orchestrator.indexFiles(filePaths).finally(() => this.resolver.clearCaches());
} finally {
this.fileLock.release();
}