mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-25 12:00:02 +08:00
fix(cli): index no longer blocks on a daemon lock naming a process that isn't CodeGraph (#2819)
codegraph index told you to run `codegraph daemon stop`, which the CLI
rejects ("too many arguments"), whenever it could not verify the process
named in .codegraph/daemon.pid. In that state `codegraph daemon` lists
nothing, because it lists only daemons that answer their socket. And
`codegraph unlock` kept an old-format lock whose process was alive (the
rule from issue #1834), so nothing the CLI offered got past it. The
writer-lock message gave the same wrong advice.
Root cause: a live pid in the lock was the only evidence, and a pid
proves nothing once the OS gives it to another program. Every CodeGraph
daemon since the first one (v0.9.5) runs
`<runtime> ... codegraph.js serve --mcp --path <root>`, so the holder's
command line settles the question.
Fix:
- New src/mcp/process-identity.ts reads a process's command line and
executable name from the OS: Win32_Process through PowerShell (tasklist
as fallback) on Windows, /proc on Linux, ps elsewhere. Each lookup is
capped at 5 s and never throws. A holder is a CodeGraph MCP server,
provably something else, or unknown, and unknown is never evidence.
- stopDaemonAt treats an unverified live holder that is provably not a
CodeGraph server like a dead one. Its lock is cleared, under the same
writer-slot claim, and the process is never signalled.
clearStaleDaemonArtifacts (codegraph unlock) does the same for an
old-format lock. A holder that may be a daemon keeps its lock, as
before (issues #1553 and #1834).
- daemonStopFailureMessage builds index's error from what remains: the
pid, what it runs, and a command that clears it (taskkill /PID N /F or
kill N), or which lock file to delete when only that will do.
- writerLockHeldMessage names `codegraph daemon` instead of the missing
`codegraph daemon stop`.
Validation (Windows 11):
- New daemon-lock-holder.test.ts (11 tests) and a daemon-registry test.
With main's daemon-registry.ts, writer-lock.ts and codegraph.ts put
back, 11 tests fail, including all 8 CLI cases and the writer-lock
wording; with the fix all pass. Four existing tests used generic
processes to stand in for a daemon that doesn't answer. Those stand-ins
now carry a CodeGraph server's command line, and pass on main and here.
- Real CLI with a private HOME, 4 cases: an old-format lock naming a live
PING.EXE; an old-format lock naming a live Node process that isn't
CodeGraph; a current-format lock naming that kind of Node process with
a pipe nobody listens on; an old-format lock naming a Node process whose
command line reads `codegraph.js serve --mcp`. On main all 4 index runs
exit 1 with the old advice, `daemon stop` is rejected, `daemon` lists
nothing, and unlock clears only the third. With the fix the first 3
rebuild (exit 0), each holder stays alive, and unlock clears the first
too. The 4th exits 1, naming the pid and `taskkill /PID <pid> /F`;
after that command, index exits 0.
Contributor PR #2373 (@danusha2345) changes the two strings only; I read
it for guidance.
Fixes #2770
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
b8c3c032be
commit
3f58e34383
@@ -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
|
||||
|
||||
@@ -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<boolean>
|
||||
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<boolean> {
|
||||
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<StopResult | null> {
|
||||
if (!await holderIsForeign(pid)) return null;
|
||||
const removed = cleanupDaemonArtifacts(root, lockContents);
|
||||
return { root, pid, outcome: removed ? 'not-running' : 'unverified' };
|
||||
}
|
||||
|
||||
const sleep = (ms: number): Promise<void> => 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<string> {
|
||||
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
|
||||
|
||||
@@ -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 `<runtime> … codegraph.js serve --mcp --path
|
||||
* <root>`. 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<string | null> {
|
||||
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<ProcessDescription> {
|
||||
// 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<ProcessDescription> {
|
||||
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<ProcessDescription> {
|
||||
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';
|
||||
}
|
||||
@@ -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
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user