diff --git a/CHANGELOG.md b/CHANGELOG.md index 75535a0bbd..f40f20567c 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 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 new file mode 100644 index 0000000000..c746b24ba9 --- /dev/null +++ b/__tests__/cli-sync-reports-reason.test.ts @@ -0,0 +1,106 @@ +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, + // 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' }, + }); +} + +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('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'] : [])); + 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 a6fe54c98e..8aaa81b15a 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,29 @@ program const { default: CodeGraph } = await loadCodeGraph(); const cg = await CodeGraph.open(projectPath); + // 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') { + 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) { - 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 +994,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 743f5f42ed..23abe1d76c 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -125,6 +125,18 @@ export interface SyncResult { filesRemoved: number; nodesUpdated: number; durationMs: number; + /** + * 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' | '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 7b8ef2ca78..5f15415d5c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -785,8 +785,17 @@ 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) { + // 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 { + ...(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, + }; } // 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 dc860a3f49..37627e573b 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -295,6 +295,19 @@ export class FileLock { } } + /** + * 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) && this.isProcessAlive(pid) ? pid : null; + } catch { + return null; + } + } + /** * Release the lock */