diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b4b6115..e478835a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - VB.NET designer code such as `New System.Drawing.Point(…)` or `Me.Size = New System.Drawing.Size(…)` is no longer linked to a project type or member that only shares the name, and a class nested inside another is matched by its bare name only from inside that class or one that inherits it. - 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) - 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. diff --git a/__tests__/cli-unlock.test.ts b/__tests__/cli-unlock.test.ts index 56799f38..9dc7244d 100644 --- a/__tests__/cli-unlock.test.ts +++ b/__tests__/cli-unlock.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { execFile, execFileSync } from 'child_process'; +import { execFile, execFileSync, spawn } from 'child_process'; import * as fs from 'fs'; import * as net from 'net'; import * as os from 'os'; @@ -101,14 +101,22 @@ describe('codegraph unlock — daemon artifact recovery (#1553)', () => { }); it('preserves a live legacy lock whose daemon identity cannot be probed', () => { - const pidPath = getDaemonPidPath(tempDir); - fs.writeFileSync(pidPath, `${process.pid}\n`); + // The holder's command line reads like a CodeGraph MCP server's, so it may + // be the daemon. One that is plainly another program leaves a stale lock, + // which unlock clears (#2770). + const holder = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)', 'codegraph.js', 'serve', '--mcp'], { stdio: 'ignore' }); + try { + const pidPath = getDaemonPidPath(tempDir); + fs.writeFileSync(pidPath, `${holder.pid}\n`); - const output = runCodegraph(['unlock', tempDir], tempDir); + const output = runCodegraph(['unlock', tempDir], tempDir); - expect(output).toContain('No stale lock files found'); - expect(fs.readFileSync(pidPath, 'utf8')).toBe(`${process.pid}\n`); - expect(() => process.kill(process.pid, 0)).not.toThrow(); + expect(output).toContain('No stale lock files found'); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(`${holder.pid}\n`); + expect(() => process.kill(holder.pid!, 0)).not.toThrow(); + } finally { + holder.kill('SIGKILL'); + } }); it('removes a legacy lock whose PID is dead', () => { diff --git a/__tests__/daemon-lock-holder.test.ts b/__tests__/daemon-lock-holder.test.ts new file mode 100644 index 00000000..5ab11539 --- /dev/null +++ b/__tests__/daemon-lock-holder.test.ts @@ -0,0 +1,233 @@ +/** + * Issue #2770: `codegraph index` could not stop a project's daemon whose lock + * named a live process it couldn't verify, and told the user to run + * `codegraph daemon stop`, a command the CLI rejects. `codegraph daemon` lists + * nothing in that state and `codegraph unlock` kept the lock, so nothing the + * CLI offered got past it. + * + * A lock whose live holder the OS shows is not a CodeGraph server (a pid the + * OS gave to another program, or a lock written by hand) is now treated as + * stale, the way a dead holder's is: `index` clears it and rebuilds, and + * `unlock` clears it, without signalling the process. A holder that does run a + * CodeGraph server keeps its lock, and the message names that process and a + * command that stops it. + */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { spawn, spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { encodeLockInfo, getDaemonPidPath } from '../src/mcp/daemon-paths'; +import { isProcessAlive } from '../src/mcp/daemon-registry'; +import { + classifyLockHolder, + describeProcess, + isCodeGraphServerCommandLine, +} from '../src/mcp/process-identity'; +import { tryAcquireWriterLock, releaseWriterLock } from '../src/mcp/writer-lock'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); +const windows = process.platform === 'win32'; +const holders: number[] = []; +let root: string; + +/** + * Start a long-lived process to be named by the lock: `native` is no + * JavaScript runtime, `node` is Node running something else, and + * `codegraph-server` is Node with a CodeGraph MCP server's command line. + */ +function startHolder(kind: 'native' | 'node' | 'codegraph-server'): number { + const idle = ['-e', 'setInterval(() => {}, 1000)']; + const [command, args] = kind === 'native' + ? windows + ? [path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'PING.EXE'), ['-n', '600', '127.0.0.1']] + : ['sleep', ['600']] + : kind === 'node' + ? [process.execPath, idle] + : [process.execPath, [...idle, path.join(root, 'install', 'dist', 'bin', 'codegraph.js'), 'serve', '--mcp', '--path', root]]; + const child = spawn(command, args as string[], { detached: true, stdio: 'ignore', windowsHide: true }); + child.unref(); + if (!child.pid) throw new Error(`could not start a ${kind} holder`); + holders.push(child.pid); + return child.pid; +} + +function runCli(args: string[]) { + return spawnSync(process.execPath, [BIN, ...args], { + cwd: root, + encoding: 'utf8', + timeout: 60_000, + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_TELEMETRY: '0', DO_NOT_TRACK: '1', CODEGRAPH_NO_PROMPT_HOOK: '1' }, + }); +} + +function writeLegacyLock(pid: number): string { + const lockPath = getDaemonPidPath(root); + fs.writeFileSync(lockPath, `${pid}\n`); + return lockPath; +} + +/** A current-format lock whose socket no daemon listens on. */ +function writeLock(pid: number): string { + const lockPath = getDaemonPidPath(root); + fs.writeFileSync(lockPath, encodeLockInfo({ + pid, + version: '1.6.2', + socketPath: windows ? '\\\\.\\pipe\\codegraph-2770-nobody-listens' : path.join(root, '.codegraph', 'nobody-listens.sock'), + startedAt: Date.now() - 60_000, + })); + return lockPath; +} + +function stopCommand(pid: number): string { + return windows ? `taskkill /PID ${pid} /F` : `kill ${pid}`; +} + +beforeEach(async () => { + root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'cg-lock-holder-'))); + fs.writeFileSync(path.join(root, 'app.ts'), 'export function originalSymbol() { return 1; }\n'); + const cg = await CodeGraph.init(root); + await cg.indexAll(); + cg.close(); +}); + +afterEach(() => { + for (const pid of holders.splice(0)) { + try { process.kill(pid, 'SIGKILL'); } catch { /* already gone */ } + } + releaseWriterLock(root); + fs.rmSync(root, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); +}); + +describe('what a lock holder is (#2770)', () => { + it('recognizes a CodeGraph MCP server by its command line', () => { + expect(isCodeGraphServerCommandLine( + '"C:\\Program Files\\nodejs\\node.exe" --liftoff-only --disable-warning=ExperimentalWarning ' + + '"C:\\Users\\me\\AppData\\Roaming\\npm\\node_modules\\@colbymchenry\\codegraph\\lib\\dist\\bin\\codegraph.js" serve --mcp --path D:\\proj', + )).toBe(true); + expect(isCodeGraphServerCommandLine('/opt/codegraph/node /opt/codegraph/lib/dist/bin/codegraph.js serve --mcp --path /src/app')).toBe(true); + expect(isCodeGraphServerCommandLine('node /usr/lib/node_modules/@colbymchenry/codegraph/dist/bin/codegraph.js index')).toBe(false); + expect(isCodeGraphServerCommandLine('node server.js serve --mcp')).toBe(false); + expect(isCodeGraphServerCommandLine('C:\\Windows\\Explorer.EXE')).toBe(false); + }); + + it('counts only what the OS shows as evidence', () => { + expect(classifyLockHolder({ name: 'explorer.exe', commandLine: 'C:\\Windows\\Explorer.EXE' })).toBe('other'); + expect(classifyLockHolder({ name: 'node.exe', commandLine: 'node.exe -e "setInterval(() => {}, 1000)"' })).toBe('other'); + expect(classifyLockHolder({ name: 'node', commandLine: 'node codegraph.js serve --mcp --path /p' })).toBe('codegraph-server'); + // No command line (another user's or an elevated process): only an + // executable that is no JavaScript runtime rules the holder out. + expect(classifyLockHolder({ name: 'svchost.exe', commandLine: null })).toBe('other'); + expect(classifyLockHolder({ name: 'node.exe', commandLine: null })).toBe('unknown'); + expect(classifyLockHolder({ name: '/usr/local/bin/node', commandLine: null })).toBe('unknown'); + expect(classifyLockHolder({ name: null, commandLine: null })).toBe('unknown'); + }); + + it('reads a running process from the OS', async () => { + const native = startHolder('native'); + const server = startHolder('codegraph-server'); + const nativeInfo = await describeProcess(native); + expect(nativeInfo.commandLine ?? '').toMatch(windows ? /ping/i : /sleep/); + expect(classifyLockHolder(nativeInfo)).toBe('other'); + expect(classifyLockHolder(await describeProcess(server))).toBe('codegraph-server'); + process.kill(native, 'SIGKILL'); + const end = Date.now() + 10_000; + while (isProcessAlive(native) && Date.now() < end) await new Promise((r) => setTimeout(r, 50)); + expect(classifyLockHolder(await describeProcess(native))).toBe('unknown'); + }, 30_000); +}); + +describe('codegraph index and unlock with a daemon lock naming a live process (#2770)', () => { + it.each([ + ['an old-format lock naming a program that is not a JavaScript runtime', 'native', writeLegacyLock], + ['an old-format lock naming a Node process that is not CodeGraph', 'node', writeLegacyLock], + ['a lock naming a Node process that is not CodeGraph', 'node', writeLock], + ] as const)('index clears %s and rebuilds, leaving the process alone', (_, kind, write) => { + const holder = startHolder(kind); + const lockPath = write(holder); + fs.writeFileSync(path.join(root, 'app.ts'), 'export function replacementSymbol() { return 2; }\n'); + + const index = runCli(['index', '--quiet', root]); + + expect(index.stderr + index.stdout).not.toContain('daemon stop'); + expect(index.status, index.stderr).toBe(0); + expect(fs.existsSync(lockPath)).toBe(false); + expect(isProcessAlive(holder)).toBe(true); + const cg = CodeGraph.openSync(root); + try { expect(cg.searchNodes('replacementSymbol').length).toBeGreaterThan(0); } + finally { cg.close(); } + }, 60_000); + + it('unlock clears an old-format lock naming a process that is not CodeGraph, leaving the process alone', () => { + const holder = startHolder('native'); + const lockPath = writeLegacyLock(holder); + + const unlock = runCli(['unlock', root]); + + expect(unlock.status, unlock.stderr).toBe(0); + expect(unlock.stdout).toContain('Removed stale lock artifacts'); + expect(fs.existsSync(lockPath)).toBe(false); + expect(isProcessAlive(holder)).toBe(true); + }, 60_000); + + it.each([ + ['an old-format lock', writeLegacyLock], + ['a lock', writeLock], + ] as const)('keeps %s held by a CodeGraph server that does not answer, and names a command that clears it', async (_, write) => { + const holder = startHolder('codegraph-server'); + const lockPath = write(holder); + + const index = runCli(['index', '--quiet', root]); + + expect(index.status).toBe(1); + const message = index.stderr + index.stdout; + expect(message).not.toContain('daemon stop'); + expect(message).toContain(`Process ${holder} runs a CodeGraph server`); + expect(message).toContain(`\`${stopCommand(holder)}\``); + expect(isProcessAlive(holder)).toBe(true); + expect(fs.existsSync(lockPath)).toBe(true); + + // The advice: stop that process, then retry `codegraph index`. + process.kill(holder, 'SIGKILL'); + const end = Date.now() + 10_000; + while (isProcessAlive(holder) && Date.now() < end) await new Promise((r) => setTimeout(r, 50)); + const retry = runCli(['index', '--quiet', root]); + expect(retry.status, retry.stderr).toBe(0); + }, 90_000); + + it('names the stale writer lock when the same process that is not CodeGraph holds it too', () => { + const holder = startHolder('native'); + writeLock(holder); + // Both of a crashed daemon's lock files, once the OS has reused its pid. + const writerPath = path.join(root, '.codegraph', 'writer.pid'); + fs.writeFileSync(writerPath, JSON.stringify({ pid: holder, mode: 'daemon', startedAt: Date.now() - 60_000, ready: true }) + '\n'); + + const index = runCli(['index', '--quiet', root]); + + expect(index.status).toBe(1); + const message = index.stderr + index.stdout; + expect(message).toContain(`names process ${holder}, which is not a CodeGraph process, so it is stale: delete ${writerPath}`); + fs.rmSync(writerPath); + const retry = runCli(['index', '--quiet', root]); + expect(retry.status, retry.stderr).toBe(0); + expect(isProcessAlive(holder)).toBe(true); + }, 90_000); + + it('the writer-lock message names `codegraph daemon`, a command the CLI accepts', () => { + expect(tryAcquireWriterLock(root, 'daemon').kind).toBe('acquired'); + try { + const index = runCli(['index', '--quiet', root]); + expect(index.status).toBe(1); + const message = index.stderr + index.stdout; + expect(message).toContain('writer lock held'); + expect(message).not.toContain('daemon stop'); + expect(message).toContain('pick it in `codegraph daemon`'); + } finally { + releaseWriterLock(root); + } + const daemon = runCli(['daemon']); + expect(daemon.stderr).not.toContain('too many arguments'); + expect(daemon.status, daemon.stderr).toBe(0); + }, 60_000); +}); diff --git a/__tests__/daemon-registry.test.ts b/__tests__/daemon-registry.test.ts index af8c4639..49c2a4ed 100644 --- a/__tests__/daemon-registry.test.ts +++ b/__tests__/daemon-registry.test.ts @@ -37,10 +37,15 @@ function waitForExit(child: ReturnType): Promise { : new Promise((resolve) => child.once('exit', () => resolve())); } +/** + * A live process standing in for a daemon. Its command line reads like a + * CodeGraph MCP server's: one that is plainly another program would make its + * lock stale (#2770). + */ function startDetachedProcess(): number { const source = [ "const { spawn } = require('child_process');", - "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { detached: true, stdio: 'ignore' });", + "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)', 'codegraph.js', 'serve', '--mcp'], { detached: true, stdio: 'ignore' });", 'child.unref();', 'console.log(child.pid);', ].join(' '); @@ -101,6 +106,32 @@ describe('daemon-registry', () => { } }); + it('clears, without signalling it, the lock of a live process that is not a CodeGraph server (#2770)', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-foreign-holder-')); + fs.mkdirSync(path.join(root, '.codegraph')); + const holder = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' }); + const pid = holder.pid!; + try { + fs.writeFileSync( + getDaemonPidPath(root), + encodeLockInfo({ pid, version: 'test', socketPath: path.join(root, '.codegraph', 'missing.sock'), startedAt: Date.now() }), + ); + expect((await stopDaemonAt(root, { preserveUnverified: true })).outcome).toBe('not-running'); + expect(fs.existsSync(getDaemonPidPath(root))).toBe(false); + expect(isProcessAlive(pid)).toBe(true); + + // An old-format lock has no socket to ask; the command line still tells. + fs.writeFileSync(getDaemonPidPath(root), `${pid}\n`); + expect(await clearStaleDaemonArtifacts(root)).toBe(true); + expect(fs.existsSync(getDaemonPidPath(root))).toBe(false); + expect(isProcessAlive(pid)).toBe(true); + } finally { + holder.kill('SIGKILL'); + await waitForExit(holder); + fs.rmSync(root, { recursive: true, force: true }); + } + }, 30_000); + it.each([false, true])('confirms daemon termination (refused: %s)', async (refused) => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-daemon-identity-')); fs.mkdirSync(path.join(root, '.codegraph')); diff --git a/__tests__/index-command.test.ts b/__tests__/index-command.test.ts index b4b405f8..92d57d63 100644 --- a/__tests__/index-command.test.ts +++ b/__tests__/index-command.test.ts @@ -59,10 +59,16 @@ function isAlive(pid: number): boolean { } } +/** + * A live process standing in for a daemon that does not answer its socket. + * Its command line reads like a CodeGraph MCP server's: one that is plainly + * some other program would make the lock stale, and `index` would clear it + * (#2770). + */ function startDetachedProcess(): number { const source = [ "const { spawn } = require('child_process');", - "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { detached: true, stdio: 'ignore' });", + "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)', 'codegraph.js', 'serve', '--mcp'], { detached: true, stdio: 'ignore' });", 'child.unref();', 'console.log(child.pid);', ].join(' '); @@ -150,7 +156,7 @@ describe('codegraph index — full re-index keeps the graph populated (#874)', ( ); try { - expect(() => runCodegraph(['index', '--quiet'], tempDir)).toThrow(/could not verify.*daemon/i); + expect(() => runCodegraph(['index', '--quiet'], tempDir)).toThrow(/did not answer as its daemon, so it was left running/); expect(isAlive(daemonPid)).toBe(true); expect(fs.existsSync(lockPath)).toBe(true); } finally { diff --git a/__tests__/writer-lock.test.ts b/__tests__/writer-lock.test.ts index a13b1135..028f7f29 100644 --- a/__tests__/writer-lock.test.ts +++ b/__tests__/writer-lock.test.ts @@ -83,7 +83,10 @@ describe('writer lock (#1740)', () => { 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/); + // `codegraph daemon` is the picker that stops the shared daemon; the CLI + // rejects `codegraph daemon stop` (#2770). + expect(msg).toContain('pick it in `codegraph daemon`'); + expect(msg).not.toMatch(/daemon stop/); } }); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 119bd3c8..aeae56d6 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -903,10 +903,12 @@ program try { // A live MCP daemon keeps SQLite handles open. Verify it by its socket // before stopping it so a stale pidfile can never signal another process. - const { stopDaemonAt } = await import('../mcp/daemon-registry'); - const daemonStop = await stopDaemonAt(fs.realpathSync(projectPath), { preserveUnverified: true }); + // A lock left naming a process that isn't CodeGraph is cleared (#2770). + const { stopDaemonAt, daemonStopFailureMessage } = await import('../mcp/daemon-registry'); + const realRoot = fs.realpathSync(projectPath); + const daemonStop = await stopDaemonAt(realRoot, { preserveUnverified: true }); if (daemonStop.outcome === 'unverified' || daemonStop.outcome === 'still-running') { - throw new Error('Could not verify that the active CodeGraph daemon has stopped. Run `codegraph daemon stop` to stop it, then retry `codegraph index`.'); + throw new Error(await daemonStopFailureMessage(realRoot, daemonStop)); } // Keep the writer slot through recreation AND indexing. A reconnecting diff --git a/src/mcp/daemon-registry.ts b/src/mcp/daemon-registry.ts index a9e58601..707fbba2 100644 --- a/src/mcp/daemon-registry.ts +++ b/src/mcp/daemon-registry.ts @@ -32,12 +32,15 @@ import { type DaemonLockInfo, } from './daemon-paths'; import { + getWriterPidPath, readWriterLock, releaseWriterLock, swapWriterLock, tryAcquireWriterLock, + writerLockHeldMessage, type WriterLockInfo, } from './writer-lock'; +import { classifyLockHolder, describeProcess } from './process-identity'; import { isOlderRelease } from './version'; import { WORKER_START_SETTLE_MS } from '../worker-teardown'; @@ -217,13 +220,40 @@ export async function clearStaleDaemonArtifacts(root: string): Promise info = decodeLockInfo(lockContents); } catch { /* missing/corrupt */ } if (info && isProcessAlive(info.pid)) { - // A live legacy holder has no socket path to probe. That is inconclusive, - // not proof of PID reuse, so preserve its lock rather than risk two writers. - if (!canProbeDaemonIdentity(info) || await probeDaemonIdentity(info)) return false; + if (canProbeDaemonIdentity(info)) { + if (await probeDaemonIdentity(info)) return false; + } else if (!await holderIsForeign(info.pid)) { + // A live legacy holder has no socket path to probe. That is inconclusive, + // not proof of PID reuse, so preserve its lock rather than risk two + // writers, unless the OS shows the process isn't a CodeGraph server. + return false; + } } return cleanupDaemonArtifacts(root, lockContents); } +/** + * Whether the OS shows that the live process `pid` can't be a CodeGraph daemon + * (#2770): its command line is not a CodeGraph MCP server's. A lock naming it + * is stale, as one naming a dead process is. A process the OS won't describe + * is never counted, and neither is this one. + */ +async function holderIsForeign(pid: number): Promise { + if (pid === process.pid) return false; + return classifyLockHolder(await describeProcess(pid)) === 'other'; +} + +/** + * Clear the lock of a live holder that {@link holderIsForeign} rules out as a + * daemon, the way a dead holder's is cleared, never signalling it. Resolves + * null when the holder may be a daemon. + */ +async function clearForeignHolder(root: string, pid: number, lockContents: string | null): Promise { + if (!await holderIsForeign(pid)) return null; + const removed = cleanupDaemonArtifacts(root, lockContents); + return { root, pid, outcome: removed ? 'not-running' : 'unverified' }; +} + const sleep = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); /** How long `stopDaemonAt` gives a daemon to exit on SIGTERM before looking closer. */ @@ -297,18 +327,80 @@ export async function stopDaemonAt( return { root, pid, outcome: removed ? 'not-running' : 'unverified' }; } // Never signal a process merely because it reused a stale daemon PID. The - // daemon's immediate hello is the process-identity proof (#1553). + // daemon's immediate hello is the process-identity proof (#1553). A holder + // the OS shows is no CodeGraph server is no daemon either: its lock is + // stale, and is cleared without signalling it (#2770). if (!identity || !canProbeDaemonIdentity(identity)) { - return { root, pid, outcome: 'unverified' }; + return await clearForeignHolder(root, pid, lockContents) ?? { root, pid, outcome: 'unverified' }; } if (!await probeDaemonIdentity(identity)) { - if (options.preserveUnverified) return { root, pid, outcome: 'unverified' }; + if (options.preserveUnverified) { + return await clearForeignHolder(root, pid, lockContents) ?? { root, pid, outcome: 'unverified' }; + } const removed = cleanupDaemonArtifacts(root, lockContents); return { root, pid, outcome: removed ? 'not-running' : 'unverified' }; } return stopVerifiedDaemon(root, identity, lockContents, options.shutdownGraceMs); } +/** The OS command that stops process `pid`, for advice text. */ +function stopCommand(pid: number, force = false): string { + if (process.platform === 'win32') return `taskkill /PID ${pid} /F`; + return force ? `kill -9 ${pid}` : `kill ${pid}`; +} + +/** + * What `codegraph index` reports when {@link stopDaemonAt} left the project's + * daemon lock in place (outcome `unverified` or `still-running`): the process + * that holds it, what that process is, and a command that clears it (#2770). + */ +export async function daemonStopFailureMessage(root: string, result: StopResult): Promise { + const retry = 'then retry `codegraph index`'; + if (result.outcome === 'still-running' && result.pid !== null) { + return `The CodeGraph daemon serving this project (process ${result.pid}) did not exit when asked to stop. ` + + `Stop it with \`${stopCommand(result.pid, true)}\`, ${retry}.`; + } + const lockPath = getDaemonPidPath(root); + let holder: number | null = null; + try { + const current = decodeLockInfo(fs.readFileSync(lockPath, 'utf8')); + if (current && isProcessAlive(current.pid)) holder = current.pid; + } catch { /* no lock now */ } + if (holder !== null && result.pid !== null && holder !== result.pid) { + return `A new CodeGraph daemon (process ${holder}) started for this project while \`codegraph index\` was stopping the old one; ` + + 'retry `codegraph index`.'; + } + const kind = holder === null ? null : classifyLockHolder(await describeProcess(holder)); + if (holder !== null && kind === 'codegraph-server') { + return `Process ${holder} runs a CodeGraph server (\`codegraph serve --mcp\`) and holds this project's daemon lock, ` + + 'but did not answer as its daemon, so it was left running. It is probably an older or stuck CodeGraph daemon: ' + + `stop it with \`${stopCommand(holder)}\`, ${retry}.`; + } + if (holder !== null && kind === 'unknown') { + return `Process ${holder} holds this project's daemon lock (${lockPath}) but did not answer as a CodeGraph daemon, ` + + 'and CodeGraph could not tell what it is, so it was left running. ' + + `If it is a CodeGraph server, stop it with \`${stopCommand(holder)}\`; if it is not, delete ${lockPath}. ` + + 'Then retry `codegraph index`.'; + } + // The daemon lock names no daemon: no live process, or one that is not + // CodeGraph. Such a lock is only cleared while holding the project's writer + // lock, so that is what kept it. + const writer = readWriterLock(root); + const writerPath = getWriterPidPath(root); + if (writer && writer.pid > 0 && writer.pid !== process.pid && isProcessAlive(writer.pid)) { + if (writer.pid === holder) { + return `The project's writer lock (${writerPath}) names process ${holder}, which is not a CodeGraph process, ` + + `so it is stale: delete ${writerPath}, ${retry}.`; + } + return writerLockHeldMessage(writer, writerPath); + } + if (holder !== null) { + return `Process ${holder} is not a CodeGraph process, so this project's daemon lock is stale, but it could not be removed: ` + + `delete ${lockPath}, ${retry}.`; + } + return 'The project\'s CodeGraph daemon changed while `codegraph index` was stopping it; retry `codegraph index`.'; +} + /** * The signalling half of a stop, for a daemon whose socket hello just proved * `identity` against the lock read as `lockContents`: SIGTERM, wait, SIGKILL diff --git a/src/mcp/process-identity.ts b/src/mcp/process-identity.ts new file mode 100644 index 00000000..db8113bb --- /dev/null +++ b/src/mcp/process-identity.ts @@ -0,0 +1,135 @@ +/** + * What the OS says a running process is, for judging a lock file that names it + * (#2770). + * + * A pid in `.codegraph/daemon.pid` proves nothing once the OS has given that + * number to another process, and a daemon that does not answer its socket hello + * cannot vouch for itself (#1553). The process's command line is the evidence + * left: every CodeGraph daemon runs ` … codegraph.js serve --mcp --path + * `. A lock whose live holder runs anything else is as stale as one whose + * holder has exited. When the OS withholds the command line (another user's or + * an elevated process), the executable's name can still rule out a holder that + * is no JavaScript runtime at all. + * + * Lookups are bounded and best-effort. What the OS won't say is `null`, and + * `null` is never evidence either way: such a holder stays `unknown`, and its + * lock is kept. + */ + +import { execFile } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; + +export interface ProcessDescription { + /** The executable's file name, such as `node.exe` or `node`; null when the OS won't say. */ + name: string | null; + /** The whole command line; null when the OS won't say. */ + commandLine: string | null; +} + +/** + * A live lock holder is a CodeGraph MCP server (`codegraph-server`), provably + * something else (`other`), or a process the OS won't describe (`unknown`). + */ +export type LockHolderKind = 'codegraph-server' | 'other' | 'unknown'; + +const LOOKUP_TIMEOUT_MS = 5_000; + +/** Runs a short OS query and resolves its stdout, or null when it fails or times out. */ +function query(file: string, args: string[]): Promise { + return new Promise((resolve) => { + try { + execFile(file, args, { encoding: 'utf8', timeout: LOOKUP_TIMEOUT_MS, windowsHide: true, maxBuffer: 1 << 20 }, + (error, stdout) => resolve(error ? null : stdout)); + } catch { + resolve(null); + } + }); +} + +function windowsTool(...parts: string[]): string { + return path.join(process.env.SystemRoot || process.env.windir || 'C:\\Windows', 'System32', ...parts); +} + +async function describeWindowsProcess(pid: number): Promise { + // Win32_Process has the command line; tasklist, the fallback when PowerShell + // can't run, has only the image name. + const script = + "$ErrorActionPreference = 'Stop'; [Console]::OutputEncoding = [Text.Encoding]::UTF8; " + + `$p = Get-CimInstance Win32_Process -Filter 'ProcessId = ${pid}'; ` + + 'if ($p) { [Console]::Out.Write((ConvertTo-Json -Compress @{ name = $p.Name; commandLine = $p.CommandLine })) }'; + const json = await query(windowsTool('WindowsPowerShell', 'v1.0', 'powershell.exe'), + ['-NoProfile', '-NonInteractive', '-NoLogo', '-Command', script]); + if (json?.trim()) { + try { + const found = JSON.parse(json) as { name?: unknown; commandLine?: unknown }; + return { + name: typeof found.name === 'string' && found.name ? found.name : null, + commandLine: typeof found.commandLine === 'string' && found.commandLine.trim() ? found.commandLine : null, + }; + } catch { /* fall back to tasklist */ } + } + const csv = await query(windowsTool('tasklist.exe'), ['/FI', `PID eq ${pid}`, '/FO', 'CSV', '/NH']); + for (const line of csv?.split(/\r?\n/) ?? []) { + const fields = /^"([^"]*)","(\d+)"/.exec(line); + if (fields && Number(fields[2]) === pid) return { name: fields[1] || null, commandLine: null }; + } + return { name: null, commandLine: null }; +} + +function describeLinuxProcess(pid: number): ProcessDescription { + let commandLine: string | null = null; + let name: string | null = null; + try { + const args = fs.readFileSync(`/proc/${pid}/cmdline`, 'utf8').split('\0').filter(Boolean); + if (args.length > 0) commandLine = args.join(' '); + } catch { /* gone, or hidden (hidepid) */ } + try { + name = fs.readFileSync(`/proc/${pid}/comm`, 'utf8').trim() || null; + } catch { /* gone, or hidden */ } + return { name, commandLine }; +} + +async function describePsProcess(pid: number): Promise { + const [args, comm] = await Promise.all([ + query('ps', ['-ww', '-o', 'args=', '-p', String(pid)]), + query('ps', ['-o', 'comm=', '-p', String(pid)]), + ]); + return { name: comm?.trim() || null, commandLine: args?.trim() || null }; +} + +/** What the OS says process `pid` is running. Never throws. */ +export async function describeProcess(pid: number): Promise { + if (!Number.isSafeInteger(pid) || pid <= 0) return { name: null, commandLine: null }; + if (process.platform === 'win32') return describeWindowsProcess(pid); + if (process.platform === 'linux') return describeLinuxProcess(pid); + return describePsProcess(pid); +} + +/** + * Whether a command line is a CodeGraph MCP server's, the only kind of process + * that writes `daemon.pid`: it names CodeGraph and has `serve` and `--mcp` as + * arguments. + */ +export function isCodeGraphServerCommandLine(commandLine: string): boolean { + const args = commandLine.replace(/["']/g, ' ').split(/\s+/); + return /codegraph/i.test(commandLine) && args.includes('serve') && args.some((a) => a === '--mcp' || a.startsWith('--mcp=')); +} + +/** Whether an executable could be a JavaScript runtime that runs CodeGraph. */ +function mayRunCodeGraph(name: string): boolean { + const base = path.basename(name.replace(/\\/g, '/')).toLowerCase().replace(/\.exe$/, ''); + return /node|codegraph|bun|deno/.test(base); +} + +/** + * What a live lock holder is. The command line decides when the OS gives it; + * otherwise only an executable that is no JavaScript runtime counts as `other`. + */ +export function classifyLockHolder(description: ProcessDescription): LockHolderKind { + if (description.commandLine !== null) { + return isCodeGraphServerCommandLine(description.commandLine) ? 'codegraph-server' : 'other'; + } + if (description.name !== null && !mayRunCodeGraph(description.name)) return 'other'; + return 'unknown'; +} diff --git a/src/mcp/writer-lock.ts b/src/mcp/writer-lock.ts index 7785cde6..20f3e94e 100644 --- a/src/mcp/writer-lock.ts +++ b/src/mcp/writer-lock.ts @@ -238,7 +238,7 @@ export function writerLockHeldMessage( 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), ' + + 'Stop the other server (if it is the shared daemon, pick it in `codegraph daemon` to stop it; otherwise end the other MCP session), ' + 'or unset CODEGRAPH_NO_DAEMON so additional clients proxy to the shared daemon. ' + 'If this is stale, delete ' + pidPath );