From 53ec9e593762fdefbb01c66a681e2141dc9ec741 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Wed, 16 Sep 2026 15:58:17 +0300 Subject: [PATCH 1/3] fix(cli): sync says why it did nothing and exits non-zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When another process holds `.codegraph/codegraph.lock` (an MCP server or another CLI mid-index), `CodeGraph.sync()` returned all-zero counts and the CLI printed "Already up to date" and exited 0 — or, with `--quiet`, printed nothing — without having synced anything. `SyncResult` now carries `skippedReason: 'locked'` (plus the holder's PID when readable) and the CLI maps it to one stderr line with the reason and the remedy, exiting 1 in both normal and `--quiet` mode. Quiet suppresses progress, never a failure. The stale-extraction half of the same symptom is covered by #1842 and is deliberately not duplicated here. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 1 + __tests__/cli-sync-reports-reason.test.ts | 73 +++++++++++++++++++++++ src/bin/codegraph.ts | 24 +++++++- src/extraction/index.ts | 8 +++ src/index.ts | 7 ++- src/utils.ts | 10 ++++ 6 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 __tests__/cli-sync-reports-reason.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 75535a0bb..7e1f6d672 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- `codegraph sync` no longer reports success when another process (an MCP server or another CLI mid-index) holds the index lock: it now exits with status 1 and prints one line on stderr naming the lock holder and asking you to retry — also with `--quiet`, so a git hook that fails on the exit code shows why. - Rust calls on `self` now stay with the enclosing type instead of linking to an unrelated type’s same-named method. Thanks @L4XB. (#1861) - Turning telemetry off now resets its identity and stops running processes from recording, sending, or restoring unsent data. (#1869) diff --git a/__tests__/cli-sync-reports-reason.test.ts b/__tests__/cli-sync-reports-reason.test.ts new file mode 100644 index 000000000..d5732a641 --- /dev/null +++ b/__tests__/cli-sync-reports-reason.test.ts @@ -0,0 +1,73 @@ +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'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); +let root: string; + +function run(...args: string[]) { + return spawnSync(process.execPath, [BIN, ...args], { + cwd: root, + encoding: 'utf8', + timeout: 30_000, + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1', NO_COLOR: '1' }, + }); +} + +const lockPath = () => path.join(root, '.codegraph', 'codegraph.lock'); + +beforeEach(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-sync-locked-')); + fs.writeFileSync(path.join(root, 'original.ts'), 'export function original() { return 1; }\n'); + const cg = CodeGraph.initSync(root); + try { await cg.indexAll(); } finally { cg.close(); } +}); + +afterEach(() => fs.rmSync(root, { recursive: true, force: true })); + +describe('codegraph sync while another process holds the index lock', () => { + it('sync() reports the lock instead of all-zero success counts', async () => { + // The test process itself is the live holder — the same thing FileLock.acquire writes. + fs.writeFileSync(lockPath(), String(process.pid)); + const cg = CodeGraph.openSync(root); + try { + const result = await cg.sync(); + expect(result.skippedReason).toBe('locked'); + expect(result.lockHolderPid).toBe(process.pid); + expect(result.filesChecked).toBe(0); + } finally { cg.close(); } + }); + + it.each([false, true])('CLI exits 1 with the reason on stderr, quiet=%s', (quiet) => { + fs.writeFileSync(path.join(root, 'added.ts'), 'export const added = 2;\n'); + fs.writeFileSync(lockPath(), String(process.pid)); + + const result = run('sync', ...(quiet ? ['--quiet'] : [])); + expect(result.status).toBe(1); + expect(result.stderr).toContain('another process holds the index lock'); + expect(result.stderr).toContain(`PID ${process.pid}`); + expect(result.stderr).toContain('codegraph sync'); + expect(result.stdout + result.stderr).not.toContain('Already up to date'); + if (quiet) { + expect(result.stdout).toBe(''); + expect(result.stderr.trim().split('\n')).toHaveLength(1); + } + + // Nothing was synced and the foreign lock was left alone. + expect(fs.readFileSync(lockPath(), 'utf8')).toBe(String(process.pid)); + const cg = CodeGraph.openSync(root); + try { expect(cg.getStats().fileCount).toBe(1); } finally { cg.close(); } + }); + + it.each([false, true])('syncs normally once the lock is released, quiet=%s', (quiet) => { + fs.writeFileSync(path.join(root, 'added.ts'), 'export const added = 2;\n'); + const result = run('sync', ...(quiet ? ['--quiet'] : [])); + expect(result.status).toBe(0); + expect(result.stderr).not.toContain('index lock'); + const cg = CodeGraph.openSync(root); + try { expect(cg.getStats().fileCount).toBe(2); } finally { cg.close(); } + }); +}); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index a6fe54c98..4739778c4 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -61,6 +61,7 @@ import { BROWSER_ENV, DEFAULT_UI_PORT } from '../ui-server/constants'; import type { UiServerHandle } from '../ui-server'; import { lookupSymbolNodes, describeSymbolNode, groupDefinitions } from '../graph/symbol-lookup'; import type { Node, Edge } from '../types'; +import type { SyncResult } from '../extraction'; import { isTestPath } from '../search/query-utils'; // Decided once, before `--color`/`--no-color` are stripped from argv below @@ -955,9 +956,24 @@ program const { default: CodeGraph } = await loadCodeGraph(); const cg = await CodeGraph.open(projectPath); + // sync() returns all-zero counts when another process (an MCP server + // or another CLI mid-index) holds the index lock. That is not "up to + // date": say why, and exit 1 even under --quiet, which suppresses + // progress but never a failure. + const lockedMessage = (result: SyncResult): string | null => { + if (result.skippedReason !== 'locked') return null; + const holder = result.lockHolderPid != null ? ` (PID ${result.lockHolderPid})` : ''; + return `Nothing synced: another process holds the index lock${holder}. Retry, or run "codegraph sync" when it exits.`; + }; + if (options.quiet) { - await cg.sync(); + const result = await cg.sync(); cg.destroy(); + const locked = lockedMessage(result); + if (locked) { + process.stderr.write(`codegraph sync: ${locked}\n`); + process.exit(1); + } return; } @@ -973,6 +989,12 @@ program await progress.stop(); + const locked = lockedMessage(result); + if (locked) { + cg.destroy(); + throw new Error(locked); + } + const totalChanges = result.filesAdded + result.filesModified + result.filesRemoved; if (totalChanges === 0) { diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 743f5f42e..957de2639 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -125,6 +125,14 @@ export interface SyncResult { filesRemoved: number; nodesUpdated: number; durationMs: number; + /** + * Set when NO reconciliation ran because another process holds the index + * lock (`.codegraph/codegraph.lock`, e.g. an MCP server mid-sync). The + * counts above are then all zero and must not be read as "up to date". + */ + skippedReason?: 'locked'; + /** PID of the process holding the lock, when it could be read. */ + lockHolderPid?: number; changedFilePaths?: string[]; /** Paths not absorbed because reading or extraction failed; retain for status/retry. */ failedFilePaths?: string[]; diff --git a/src/index.ts b/src/index.ts index 7b8ef2ca7..2879c7d39 100644 --- a/src/index.ts +++ b/src/index.ts @@ -786,7 +786,12 @@ export class CodeGraph { try { this.fileLock.acquire(); } catch { - return { filesChecked: 0, filesAdded: 0, filesModified: 0, filesRemoved: 0, nodesUpdated: 0, durationMs: 0 }; + const lockHolderPid = this.fileLock.readHolderPid(); + return { + skippedReason: 'locked', + ...(lockHolderPid != null ? { lockHolderPid } : {}), + filesChecked: 0, filesAdded: 0, filesModified: 0, filesRemoved: 0, nodesUpdated: 0, durationMs: 0, + }; } // Defer WAL auto-checkpointing for the whole incremental run, exactly // as indexAll does for the bulk path (#1231): sync's store loop and its diff --git a/src/utils.ts b/src/utils.ts index dc860a3f4..6afc66c0f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -295,6 +295,16 @@ export class FileLock { } } + /** PID written in the lock file, or null when absent/unreadable. */ + readHolderPid(): number | null { + try { + const pid = parseInt(fs.readFileSync(this.lockPath, 'utf-8').trim(), 10); + return Number.isNaN(pid) ? null : pid; + } catch { + return null; + } + } + /** * Release the lock */ From 93f4d5cd9c4154d2b0169d53c57bebae53b0185e Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Thu, 24 Sep 2026 11:23:31 +0300 Subject: [PATCH 2/3] test(cli): keep Node's experimental warning out of the quiet-sync stderr count The test spawns the CLI with CODEGRAPH_WASM_RELAUNCHED=1, which skips the relaunch and with it --disable-warning=ExperimentalWarning. On Node 22 the node:sqlite ExperimentalWarning then lands on stderr as two extra lines and the quiet case, which expects exactly one line, fails (seen in CI on Node 22; Node 24 no longer warns). NODE_NO_WARNINGS=1 for the spawned CLI. Co-Authored-By: Claude Opus 5.5 --- __tests__/cli-sync-reports-reason.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/__tests__/cli-sync-reports-reason.test.ts b/__tests__/cli-sync-reports-reason.test.ts index d5732a641..d2314fe07 100644 --- a/__tests__/cli-sync-reports-reason.test.ts +++ b/__tests__/cli-sync-reports-reason.test.ts @@ -13,7 +13,10 @@ function run(...args: string[]) { cwd: root, encoding: 'utf8', timeout: 30_000, - env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1', NO_COLOR: '1' }, + // NODE_NO_WARNINGS: skipping the relaunch also skips its + // --disable-warning=ExperimentalWarning, and Node 22 then prints the + // node:sqlite warning to stderr, which the quiet case counts line by line. + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1', NO_COLOR: '1', NODE_NO_WARNINGS: '1' }, }); } From bd957c42927ee2ff7d9029a5f8ecb89df2866090 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Sun, 27 Sep 2026 11:04:58 +0300 Subject: [PATCH 3/3] fix(cli): only a live lock holder is contention; keep the unlock hint otherwise Review on #1889: sync() reported any failure to take the index lock as contention. A directory where codegraph.lock should be made sync print "another process holds the index lock" and exit 1 when nothing held it. Contention is now a live process named in the lock file (`locked`, with its PID); any other failure is `lock-failed` and carries the lock's own message, with its `codegraph unlock` hint. The changelog no longer says a failing git hook shows why: the hooks codegraph installs discard both output and exit status. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- __tests__/cli-sync-reports-reason.test.ts | 30 +++++++++++++++++++++++ src/bin/codegraph.ts | 19 ++++++++------ src/extraction/index.ts | 12 ++++++--- src/index.ts | 12 ++++++--- src/utils.ts | 9 ++++--- 6 files changed, 65 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e1f6d672..f40f20567 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,7 +145,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes -- `codegraph sync` no longer reports success when another process (an MCP server or another CLI mid-index) holds the index lock: it now exits with status 1 and prints one line on stderr naming the lock holder and asking you to retry — also with `--quiet`, so a git hook that fails on the exit code shows why. +- `codegraph sync` no longer reports success when it could not take the index lock: it exits with status 1 and prints one line on stderr, also with `--quiet`. When another process (an MCP server or another CLI mid-index) holds the lock, the line names it and asks you to retry; any other lock problem keeps its `codegraph unlock` hint. A script of your own that checks the exit code can now tell a skipped sync from an up-to-date one. - Rust calls on `self` now stay with the enclosing type instead of linking to an unrelated type’s same-named method. Thanks @L4XB. (#1861) - Turning telemetry off now resets its identity and stops running processes from recording, sending, or restoring unsent data. (#1869) diff --git a/__tests__/cli-sync-reports-reason.test.ts b/__tests__/cli-sync-reports-reason.test.ts index d2314fe07..c746b24ba 100644 --- a/__tests__/cli-sync-reports-reason.test.ts +++ b/__tests__/cli-sync-reports-reason.test.ts @@ -65,6 +65,36 @@ describe('codegraph sync while another process holds the index lock', () => { try { expect(cg.getStats().fileCount).toBe(1); } finally { cg.close(); } }); + it('does not call a lock nobody holds contention, and keeps its unlock hint', async () => { + // A directory where the lock file should be: acquire fails, but no process holds it. + fs.mkdirSync(lockPath()); + const cg = CodeGraph.openSync(root); + try { + const result = await cg.sync(); + expect(result.skippedReason).toBe('lock-failed'); + expect(result.lockHolderPid).toBeUndefined(); + expect(result.lockError).toContain('codegraph unlock'); + } finally { cg.close(); } + + const cli = run('sync', '--quiet'); + expect(cli.status).toBe(1); + expect(cli.stderr).not.toContain('another process holds the index lock'); + expect(cli.stderr).toContain('the index lock could not be taken'); + expect(cli.stderr).toContain('codegraph unlock'); + }); + + it('does not call a lock left by a dead process contention', async () => { + // A PID that cannot be alive: sync reclaims the stale lock and runs. + fs.writeFileSync(lockPath(), '2147483646'); + fs.writeFileSync(path.join(root, 'added.ts'), 'export const added = 2;\n'); + const cg = CodeGraph.openSync(root); + try { + const result = await cg.sync(); + expect(result.skippedReason).toBeUndefined(); + expect(result.filesAdded).toBe(1); + } finally { cg.close(); } + }); + it.each([false, true])('syncs normally once the lock is released, quiet=%s', (quiet) => { fs.writeFileSync(path.join(root, 'added.ts'), 'export const added = 2;\n'); const result = run('sync', ...(quiet ? ['--quiet'] : [])); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 4739778c4..8aaa81b15 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -956,14 +956,19 @@ program const { default: CodeGraph } = await loadCodeGraph(); const cg = await CodeGraph.open(projectPath); - // sync() returns all-zero counts when another process (an MCP server - // or another CLI mid-index) holds the index lock. That is not "up to - // date": say why, and exit 1 even under --quiet, which suppresses - // progress but never a failure. + // sync() returns all-zero counts when it could not take the index lock. + // That is not "up to date": say why, and exit 1 even under --quiet, + // which suppresses progress but never a failure. Only a live holder is + // contention; any other lock failure keeps the lock's own message. const lockedMessage = (result: SyncResult): string | null => { - if (result.skippedReason !== 'locked') return null; - const holder = result.lockHolderPid != null ? ` (PID ${result.lockHolderPid})` : ''; - return `Nothing synced: another process holds the index lock${holder}. Retry, or run "codegraph sync" when it exits.`; + if (result.skippedReason === 'locked') { + const holder = result.lockHolderPid != null ? ` (PID ${result.lockHolderPid})` : ''; + return `Nothing synced: another process holds the index lock${holder}. Retry, or run "codegraph sync" when it exits.`; + } + if (result.skippedReason === 'lock-failed') { + return `Nothing synced: the index lock could not be taken. ${result.lockError ?? ''}`.trimEnd(); + } + return null; }; if (options.quiet) { diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 957de2639..23abe1d76 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -126,13 +126,17 @@ export interface SyncResult { nodesUpdated: number; durationMs: number; /** - * Set when NO reconciliation ran because another process holds the index - * lock (`.codegraph/codegraph.lock`, e.g. an MCP server mid-sync). The + * Set when NO reconciliation ran because the index lock + * (`.codegraph/codegraph.lock`) could not be taken: `locked` when a live + * process holds it (an MCP server or another CLI mid-sync), `lock-failed` + * for anything else (an unreadable lock, a directory in its place). The * counts above are then all zero and must not be read as "up to date". */ - skippedReason?: 'locked'; - /** PID of the process holding the lock, when it could be read. */ + skippedReason?: 'locked' | 'lock-failed'; + /** PID of the live process holding the lock (`locked`). */ lockHolderPid?: number; + /** The lock's own error message, with its `codegraph unlock` hint (`lock-failed`). */ + lockError?: string; changedFilePaths?: string[]; /** Paths not absorbed because reading or extraction failed; retain for status/retry. */ failedFilePaths?: string[]; diff --git a/src/index.ts b/src/index.ts index 2879c7d39..5f15415d5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -785,11 +785,15 @@ export class CodeGraph { return this.indexMutex.withLock(async () => { try { this.fileLock.acquire(); - } catch { - const lockHolderPid = this.fileLock.readHolderPid(); + } catch (err) { + // Contention is a live process named in the lock file. Anything else + // (an unreadable lock, a directory where the file should be) is not + // contention, and keeps the lock's own message and its unlock hint. + const lockHolderPid = this.fileLock.readLiveHolderPid(); return { - skippedReason: 'locked', - ...(lockHolderPid != null ? { lockHolderPid } : {}), + ...(lockHolderPid != null + ? { skippedReason: 'locked' as const, lockHolderPid } + : { skippedReason: 'lock-failed' as const, lockError: err instanceof Error ? err.message : String(err) }), filesChecked: 0, filesAdded: 0, filesModified: 0, filesRemoved: 0, nodesUpdated: 0, durationMs: 0, }; } diff --git a/src/utils.ts b/src/utils.ts index 6afc66c0f..37627e573 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -295,11 +295,14 @@ export class FileLock { } } - /** PID written in the lock file, or null when absent/unreadable. */ - readHolderPid(): number | null { + /** + * PID written in the lock file when that process is alive — real contention — + * or null when the file is absent, unreadable, or names a dead process. + */ + readLiveHolderPid(): number | null { try { const pid = parseInt(fs.readFileSync(this.lockPath, 'utf-8').trim(), 10); - return Number.isNaN(pid) ? null : pid; + return !Number.isNaN(pid) && this.isProcessAlive(pid) ? pid : null; } catch { return null; }