diff --git a/CHANGELOG.md b/CHANGELOG.md index 220298c..146c20b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +#### MCP / indexing + +- **A second `codegraph serve --mcp` on the same project no longer silently kills auto-sync (#1740).** Direct mode (`CODEGRAPH_NO_DAEMON=1` or proxy→in-process fallback) now takes an exclusive `.codegraph/writer.pid` lock; a second writer exits immediately with guidance to stop the other server or unset `CODEGRAPH_NO_DAEMON` so clients share the daemon. The shared daemon already multiplexes N clients onto one watcher — this closes the same-OS dual-direct gap the docs warned about for Windows/WSL but did not guard. + #### Screens, links and navigation - **Where the app goes after login is a fork, not two always-es.** A navigation whose destination comes back from a helper — `router.replace(await resolvePostLoginRoute())` over `return (await hasSeenWelcome(…)) ? '/home/' : '/welcome/'` — drew both screens with no condition, reading as if the welcome screen always shows. The two arms share a line, and only a column can tell them apart; each synthesized edge now carries its literal's own position, so the guard reader says which arm it is: `WHEN await hasSeenWelcome(…)` → home, and its negation → welcome. And the scan starts at the helper's body, so a literal-union return type — `Promise<'/welcome/' | '/home/'>`, whose routes are string literals too, written first — no longer stands in for the navigation itself. Re-index after upgrading to pick the positions up. diff --git a/README.md b/README.md index a73d3b2..80fa506 100644 --- a/README.md +++ b/README.md @@ -944,6 +944,8 @@ Framework routing is validated the same way, on a canonical app per framework: E **MCP server not connecting** — Your agent starts the server itself, so you don't launch it by hand. Make sure the project is initialized and indexed (`codegraph status`) and that the path in your MCP config is correct. If it still won't connect, re-run `codegraph install` to rewrite the config. +**Two `codegraph serve --mcp` on one project fight over the index / auto-sync stops** — CodeGraph allows one live MCP *writer* per project (the shared background daemon, or a single direct-mode process). Extra clients should proxy to that daemon. If you set `CODEGRAPH_NO_DAEMON=1`, run only one `serve --mcp` for that project; a second instance exits with a clear writer-lock error (see `writer.pid` under `.codegraph/`). Prefer leaving the daemon enabled so multiple MCP hosts share one watcher. + **MCP tool calls fail with `Transport closed` while `codegraph status`/`sync` are healthy** — almost always WSL2 with the project on a Windows drive (a `/mnt/c` or `/mnt/d` path), where the local socket CodeGraph uses to share one background server across sessions is unreliable. CodeGraph now falls back to serving the session in-process instead of dropping the connection, but if you still hit it, set `CODEGRAPH_NO_DAEMON=1` in your MCP server's environment to skip the shared server entirely (each session runs in its own process). Moving the project onto the Linux-native filesystem (e.g. under `~/` instead of `/mnt/`) restores the shared server. **Missing symbols** — The MCP server auto-syncs on save (wait a couple seconds). Run `codegraph sync` manually if needed. Check that the file's language is supported and isn't inside a `.gitignore`d or default-excluded directory (e.g. `node_modules`, `dist`). diff --git a/__tests__/mcp-writer-lock.test.ts b/__tests__/mcp-writer-lock.test.ts new file mode 100644 index 0000000..be68f94 --- /dev/null +++ b/__tests__/mcp-writer-lock.test.ts @@ -0,0 +1,128 @@ +/** + * Issue #1740 — concurrent direct-mode serve --mcp must fail fast on the + * second writer instead of silently degrading auto-sync. + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { ChildProcessWithoutNullStreams, spawn } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { getWriterPidPath } from '../src/mcp/writer-lock'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +function sleep(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); +} + +function spawnMcp( + cwd: string, + env: NodeJS.ProcessEnv, +): { child: ChildProcessWithoutNullStreams; getStderr: () => string } { + const child = spawn(process.execPath, [BIN, 'serve', '--mcp'], { + cwd, + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env, ...env }, + }) as ChildProcessWithoutNullStreams; + child.on('error', () => {}); + child.stdin.on('error', () => {}); + let stderr = ''; + child.stderr.on('data', (c: Buffer) => { stderr += c.toString('utf8'); }); + child.stdout.on('data', () => {}); + return { child, getStderr: () => stderr }; +} + +describe('issue #1740 — direct-mode writer lock', () => { + let tempDir: string; + let realRoot: string; + const children: ChildProcessWithoutNullStreams[] = []; + + beforeEach(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg1740-mcp-')); + realRoot = fs.realpathSync(tempDir); + fs.mkdirSync(path.join(realRoot, 'src')); + fs.writeFileSync(path.join(realRoot, 'src/a.ts'), 'export function a() { return 1; }\n'); + const cg = await CodeGraph.init(realRoot); + await cg.indexAll(); + cg.close(); + }); + + afterEach(async () => { + for (const c of children) { + try { c.kill('SIGTERM'); } catch { /* ignore */ } + } + children.length = 0; + await sleep(300); + try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('second CODEGRAPH_NO_DAEMON serve --mcp exits with writer-lock error', async () => { + const env = { + CODEGRAPH_NO_DAEMON: '1', + CODEGRAPH_MCP_DEBUG: '1', + CODEGRAPH_NO_WATCHDOG: '1', + CODEGRAPH_STARTUP_HANDSHAKE_TIMEOUT_MS: '0', + // Avoid wasm --liftoff-only re-exec so lock.pid matches the spawned pid. + CODEGRAPH_NO_RELAUNCH: '1', + CODEGRAPH_WASM_RELAUNCHED: '1', + }; + const first = spawnMcp(realRoot, env); + children.push(first.child); + + const lockPath = getWriterPidPath(realRoot); + const deadline = Date.now() + 10000; + while (Date.now() < deadline && !fs.existsSync(lockPath)) { + await sleep(50); + } + expect(fs.existsSync(lockPath)).toBe(true); + expect(first.child.exitCode).toBeNull(); + + const second = spawnMcp(realRoot, env); + children.push(second.child); + + const code = await new Promise((resolve) => { + const timer = setTimeout(() => resolve(second.child.exitCode), 10000); + second.child.on('close', (c) => { + clearTimeout(timer); + resolve(c); + }); + }); + + expect(code).toBe(1); + expect(second.getStderr()).toMatch(/writer lock held/i); + expect(second.getStderr()).toMatch(/CODEGRAPH_NO_DAEMON/); + expect(first.child.exitCode).toBeNull(); + const lock = JSON.parse(fs.readFileSync(lockPath, 'utf8')) as { pid: number }; + expect(lock.pid).toBe(first.child.pid); + }, 20000); + + it('default daemon mode still allows two proxies to share one writer', async () => { + const env = { + CODEGRAPH_MCP_LOG_ATTACH: '1', + CODEGRAPH_NO_WATCHDOG: '1', + CODEGRAPH_STARTUP_HANDSHAKE_TIMEOUT_MS: '0', + CODEGRAPH_NO_RELAUNCH: '1', + CODEGRAPH_WASM_RELAUNCHED: '1', + }; + const a = spawnMcp(realRoot, env); + const b = spawnMcp(realRoot, env); + children.push(a.child, b.child); + + const lockPath = getWriterPidPath(realRoot); + const deadline = Date.now() + 15000; + while (Date.now() < deadline && !fs.existsSync(lockPath)) { + await sleep(50); + } + expect(fs.existsSync(lockPath)).toBe(true); + await sleep(1000); + expect(a.child.exitCode).toBeNull(); + expect(b.child.exitCode).toBeNull(); + + const lock = JSON.parse(fs.readFileSync(lockPath, 'utf8')) as { pid: number; mode: string }; + expect(lock.mode).toBe('daemon'); + expect(lock.pid).not.toBe(a.child.pid); + expect(lock.pid).not.toBe(b.child.pid); + }, 25000); +}); diff --git a/__tests__/writer-lock.test.ts b/__tests__/writer-lock.test.ts new file mode 100644 index 0000000..c3ba447 --- /dev/null +++ b/__tests__/writer-lock.test.ts @@ -0,0 +1,87 @@ +/** + * Project writer lock (#1740) — unit coverage for acquire / re-entrant / + * stale-dead-pid / live-holder refusal. + */ + +import { afterEach, describe, expect, it } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { + decodeWriterLockInfo, + getWriterPidPath, + releaseWriterLock, + tryAcquireWriterLock, + writerLockHeldMessage, +} from '../src/mcp/writer-lock'; + +describe('writer lock (#1740)', () => { + let dir: string; + + afterEach(() => { + if (dir) { + releaseWriterLock(dir); + try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + } + }); + + function makeProject(): string { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg1740-lock-')); + fs.mkdirSync(path.join(dir, '.codegraph'), { recursive: true }); + return dir; + } + + it('acquires and releases writer.pid', () => { + const root = makeProject(); + const r = tryAcquireWriterLock(root, 'direct'); + expect(r.kind).toBe('acquired'); + expect(fs.existsSync(getWriterPidPath(root))).toBe(true); + const info = decodeWriterLockInfo(fs.readFileSync(getWriterPidPath(root), 'utf8')); + expect(info?.pid).toBe(process.pid); + expect(info?.mode).toBe('direct'); + releaseWriterLock(root); + expect(fs.existsSync(getWriterPidPath(root))).toBe(false); + }); + + it('is re-entrant for the same pid', () => { + const root = makeProject(); + expect(tryAcquireWriterLock(root, 'daemon').kind).toBe('acquired'); + const again = tryAcquireWriterLock(root, 'fallback'); + expect(again.kind).toBe('acquired'); + releaseWriterLock(root); + }); + + it('reports taken when a live foreign pid holds the lock', () => { + const root = makeProject(); + // Use our own pid first, then overwrite with a fake live-looking pid by + // writing a pid that is alive: process.pid of this test — simulate foreign + // by writing a different alive pid. On Linux, PID 1 is almost always alive. + fs.writeFileSync( + getWriterPidPath(root), + JSON.stringify({ pid: 1, mode: 'direct', startedAt: Date.now() }) + '\n', + { flag: 'wx' }, + ); + const r = tryAcquireWriterLock(root, 'direct'); + expect(r.kind).toBe('taken'); + if (r.kind === 'taken') { + expect(r.existing?.pid).toBe(1); + const msg = writerLockHeldMessage(r.existing, r.pidPath); + expect(msg).toMatch(/writer lock held/i); + expect(msg).toMatch(/CODEGRAPH_NO_DAEMON/); + expect(msg).toMatch(/daemon stop/); + } + }); + + it('clears a stale dead-pid lock and acquires', () => { + const root = makeProject(); + // Pick a pid that is extremely unlikely to be alive. + const deadPid = 2147483646; + fs.writeFileSync( + getWriterPidPath(root), + JSON.stringify({ pid: deadPid, mode: 'direct', startedAt: Date.now() }) + '\n', + ); + const r = tryAcquireWriterLock(root, 'direct'); + expect(r.kind).toBe('acquired'); + releaseWriterLock(root); + }); +}); diff --git a/src/mcp/daemon.ts b/src/mcp/daemon.ts index 500c48a..a5735d1 100644 --- a/src/mcp/daemon.ts +++ b/src/mcp/daemon.ts @@ -55,6 +55,7 @@ import { getDaemonSocketPath, } from './daemon-paths'; import { CodeGraphPackageVersion } from './version'; +import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock'; import { registerDaemon, deregisterDaemon } from './daemon-registry'; /** Default idle linger after the last client disconnects. */ @@ -204,6 +205,16 @@ export class Daemon { * listening — the daemon then sticks around until idle/shutdown. */ async start(): Promise { + // #1740: claim the project writer lock before opening/watching so a + // concurrent direct-mode serve --mcp cannot start a second watcher. + const writer = tryAcquireWriterLock(this.projectRoot, 'daemon'); + if (writer.kind === 'taken') { + const msg = writerLockHeldMessage(writer.existing, writer.pidPath); + process.stderr.write(`[CodeGraph daemon] ${msg}\n`); + this.cleanupLockfile(); + throw new Error(msg); + } + // Engine init is deliberately backgrounded — see #172. The first session // to land waits on `ensureInitialized` either way, and unloaded sessions // (cross-project tool calls only) shouldn't pay any open cost. @@ -498,6 +509,7 @@ export class Daemon { } private cleanupLockfile(): void { + releaseWriterLock(this.projectRoot); try { if (fs.existsSync(this.pidPath)) { // Only remove if it still belongs to us — another daemon may have diff --git a/src/mcp/engine.ts b/src/mcp/engine.ts index 8f2e5b6..734fafa 100644 --- a/src/mcp/engine.ts +++ b/src/mcp/engine.ts @@ -16,6 +16,7 @@ import type CodeGraph from '../index'; import { resolveServerRoot } from '../directory'; import { watchDisabledReason } from '../sync'; import { ToolHandler } from './tools'; +import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock'; import { QueryPool, resolvePoolSize } from './query-pool'; // Lazy-load the heavy CodeGraph chain (sqlite + query/graph/context layers) OFF @@ -67,6 +68,8 @@ export class MCPEngine { // bounded but shouldn't run on every tool call in the no-default state. private lastRetrySubScanAt = 0; private watcherStarted = false; + /** Set when this engine holds writer.pid (#1740). */ + private writerLockRoot: string | null = null; private opts: Required; private closed = false; // Off-loop read-tool pool (daemon mode only). Created lazily once the default @@ -203,6 +206,10 @@ export class MCPEngine { stop(): void { if (this.closed) return; this.closed = true; + if (this.writerLockRoot) { + releaseWriterLock(this.writerLockRoot); + this.writerLockRoot = null; + } // Detach + terminate the worker pool first so no tool call routes to a // worker mid-teardown; outstanding pool calls resolve with graceful guidance. this.toolHandler.setQueryPool(null); @@ -278,6 +285,24 @@ export class MCPEngine { private startWatching(): void { if (!this.cg || this.watcherStarted || !this.opts.watch) return; + // #1740: only one live watcher/writer per project. Daemon and startDirect + // usually already hold writer.pid (re-entrant for this pid). Proxy + // in-process fallback acquires here; if another writer holds it, skip the + // watcher so we never contend on codegraph.lock until auto-sync degrades. + const lockRoot = this.projectPath; + if (lockRoot) { + const writer = tryAcquireWriterLock(lockRoot, 'fallback'); + if (writer.kind === 'taken') { + const msg = writerLockHeldMessage(writer.existing, writer.pidPath); + process.stderr.write( + `[CodeGraph MCP] File watcher not started — ${msg}\n` + ); + this.watcherStarted = true; + return; + } + this.writerLockRoot = lockRoot; + } + const disabledReason = watchDisabledReason(this.projectPath ?? process.cwd()); if (disabledReason) { process.stderr.write( diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 3f57024..e4a14a4 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -48,6 +48,7 @@ import { tryAcquireDaemonLock, } from './daemon'; import { connectWithHello, runLocalHandshakeProxy } from './proxy'; +import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock'; import { getDaemonSocketCandidates, probeDaemonIdentity } from './daemon-paths'; import { getTelemetry } from '../telemetry'; import { checkForUpdateInBackground } from '../upgrade/update-check'; @@ -252,6 +253,8 @@ export class MCPServer { // Idempotency guard for stop(). private stopped = false; private mode: 'unstarted' | 'direct' | 'proxy' | 'daemon' = 'unstarted'; + /** Project root whose writer.pid we hold in direct mode (#1740); released on stop. */ + private writerLockRoot: string | null = null; constructor(projectPath?: string) { this.projectPath = projectPath || null; @@ -329,6 +332,10 @@ export class MCPServer { stop(): void { if (this.stopped) return; this.stopped = true; + if (this.writerLockRoot) { + releaseWriterLock(this.writerLockRoot); + this.writerLockRoot = null; + } if (this.ppidWatchdog) { clearInterval(this.ppidWatchdog); this.ppidWatchdog = null; @@ -358,6 +365,20 @@ export class MCPServer { if (reason && process.env.CODEGRAPH_MCP_DEBUG) { process.stderr.write(`[CodeGraph MCP] Direct mode: ${reason}.\n`); } + + // #1740: refuse a second direct writer on an initialized project. Daemon + // mode multiplexes clients; direct mode is single-writer-per-project. + const writerRoot = resolveDaemonRoot(this.projectPath); + if (writerRoot) { + const writer = tryAcquireWriterLock(writerRoot, 'direct'); + if (writer.kind === 'taken') { + const msg = writerLockHeldMessage(writer.existing, writer.pidPath); + process.stderr.write(`[CodeGraph MCP] ${msg}\n`); + process.exit(1); + } + this.writerLockRoot = writerRoot; + } + this.engine = new MCPEngine(); const transport = new StdioTransport(); this.session = new MCPSession(transport, this.engine, { diff --git a/src/mcp/writer-lock.ts b/src/mcp/writer-lock.ts new file mode 100644 index 0000000..919e630 --- /dev/null +++ b/src/mcp/writer-lock.ts @@ -0,0 +1,188 @@ +/** + * Project writer lock (#1740). + * + * At most one long-lived MCP *writer* (shared daemon OR direct-mode / + * in-process engine that owns the FileWatcher) may serve a given project. + * The shared daemon already multiplexes N stdio proxies onto one writer; this + * lock closes the same-OS gap where two direct-mode `serve --mcp` processes + * (via `CODEGRAPH_NO_DAEMON=1` or proxy→in-process fallback) each start a + * watcher, contend on `codegraph.lock`, and degrade auto-sync. + * + * Deliberately separate from `daemon.pid`: proxies probe the daemon socket + * and may clear a live pid that has no socket. A direct-mode holder must not + * look like a daemon. `writer.pid` is only about "who owns live auto-sync". + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import { getCodeGraphDir } from '../directory'; +/** Signal-0 liveness (EPERM ⇒ alive). Local copy to avoid a daemon↔writer cycle. */ +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (err: unknown) { + const e = err as NodeJS.ErrnoException; + if (e.code === 'EPERM') return true; + return false; + } +} + + +/** Absolute path to the writer pid lockfile for `projectRoot`. */ +export function getWriterPidPath(projectRoot: string): string { + let root = projectRoot; + try { root = fs.realpathSync(projectRoot); } catch { /* keep lexical */ } + return path.join(getCodeGraphDir(root), 'writer.pid'); +} + +/** Structured contents of the writer pidfile. */ +export interface WriterLockInfo { + pid: number; + /** `direct` | `daemon` | `fallback` — for actionable error text only. */ + mode: string; + startedAt: number; +} + +export type WriterAcquireResult = + | { kind: 'acquired'; pidPath: string; info: WriterLockInfo } + | { kind: 'taken'; existing: WriterLockInfo | null; pidPath: string }; + +function encode(info: WriterLockInfo): string { + return JSON.stringify(info) + '\n'; +} + +export function decodeWriterLockInfo(raw: string): WriterLockInfo | null { + try { + const parsed = JSON.parse(raw.trim()) as Partial; + if (typeof parsed.pid !== 'number' || typeof parsed.mode !== 'string') return null; + return { + pid: parsed.pid, + mode: parsed.mode, + startedAt: typeof parsed.startedAt === 'number' ? parsed.startedAt : 0, + }; + } catch { + return null; + } +} + +/** + * Atomically create `writer.pid` (link-into-place, O_EXCL fallback). If held + * by a dead PID, clear and retry once. Does not steal from a live holder. + */ +export function tryAcquireWriterLock( + projectRoot: string, + mode: string, +): WriterAcquireResult { + const pidPath = getWriterPidPath(projectRoot); + fs.mkdirSync(path.dirname(pidPath), { recursive: true }); + + const info: WriterLockInfo = { + pid: process.pid, + mode, + startedAt: Date.now(), + }; + + const attempt = (): WriterAcquireResult => { + const tmp = `${pidPath}.${process.pid}.tmp`; + let acquired = false; + try { + fs.writeFileSync(tmp, encode(info), { mode: 0o600 }); + try { + fs.linkSync(tmp, pidPath); + acquired = true; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') { + // taken + } else { + // No hard links — O_EXCL create. + try { + const fd = fs.openSync(pidPath, 'wx', 0o600); + try { + fs.writeSync(fd, encode(info)); + acquired = true; + } finally { + fs.closeSync(fd); + } + } catch (e2: unknown) { + if ((e2 as NodeJS.ErrnoException).code !== 'EEXIST') throw e2; + } + } + } + } finally { + try { fs.unlinkSync(tmp); } catch { /* ignore */ } + } + + if (acquired) return { kind: 'acquired', pidPath, info }; + + let existing: WriterLockInfo | null = null; + try { + existing = decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); + } catch { /* unreadable */ } + return { kind: 'taken', existing, pidPath }; + }; + + let result = attempt(); + if (result.kind === 'taken' && result.existing && result.existing.pid === process.pid) { + // Same process already holds it (daemon acquired before engine watch). + return { kind: 'acquired', pidPath: result.pidPath, info: result.existing }; + } + if (result.kind === 'taken') { + const existing = result.existing; + if (!existing || existing.pid <= 0 || !isProcessAlive(existing.pid)) { + // Stale — clear (pid-verified) and retry once. + try { + const raw = fs.readFileSync(pidPath, 'utf8'); + const cur = decodeWriterLockInfo(raw); + if (!cur || cur.pid === existing?.pid) { + if (!cur || cur.pid <= 0 || !isProcessAlive(cur.pid)) { + fs.unlinkSync(pidPath); + } + } + } catch { /* ENOENT ok */ } + result = attempt(); + } + } + return result; +} + +/** Release if we still own the lock (pid match). */ +export function releaseWriterLock(projectRoot: string): void { + const pidPath = getWriterPidPath(projectRoot); + try { + if (!fs.existsSync(pidPath)) return; + const info = decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); + if (info && info.pid === process.pid) { + fs.unlinkSync(pidPath); + } + } catch { /* best-effort */ } +} + +/** Read current lock without acquiring. */ +export function readWriterLock(projectRoot: string): WriterLockInfo | null { + const pidPath = getWriterPidPath(projectRoot); + try { + return decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); + } catch { + return null; + } +} + +/** + * Actionable message when another live process owns the writer lock (#1740). + */ +export function writerLockHeldMessage( + existing: WriterLockInfo | null, + pidPath: string, +): string { + const who = existing && existing.pid > 0 + ? `PID ${existing.pid} (${existing.mode || 'unknown'} mode)` + : 'another process'; + return ( + 'CodeGraph writer lock held by ' + who + '. ' + + 'Only one live MCP writer may serve a project (auto-sync / index). ' + + 'Stop the other server (codegraph daemon stop if a shared daemon, or end the other MCP session), ' + + 'or unset CODEGRAPH_NO_DAEMON so additional clients proxy to the shared daemon. ' + + 'If this is stale, delete ' + pidPath + ); +}