mirror of
https://github.com/tobi/qmd.git
synced 2026-09-14 20:27:06 +08:00
fix(mcp): verify pidfile PID belongs to qmd before stop/start
Recycled PIDs after reboot made `qmd mcp stop` SIGTERM unrelated processes and blocked `--daemon` with a false "Already running". Identity-check via process cmdline before signalling or refusing. Fixes #806
This commit is contained in:
+1
-1
@@ -3,11 +3,11 @@
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- `qmd doctor` no longer false-positives `.etag` HTTP sidecars (written by
|
||||
`qmd pull` next to each download) as invalid GGUF models. The model-cache
|
||||
check now only inspects real `.gguf` files, so a sidecar that happens to
|
||||
sort before the blob no longer poisons the report. #812
|
||||
- `qmd mcp stop` and `qmd mcp --http --daemon` now verify that a pidfile PID still belongs to a qmd process before signalling it or refusing to start. Recycled PIDs (common after reboot) are treated as stale: the pidfile is unlinked instead of SIGTERM'ing an unrelated process or blocking daemon start with a false "Already running" error (#806).
|
||||
|
||||
- `qmd collection add` now rejects missing paths and regular files before
|
||||
creating collection configuration or index state. The error reports both the
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
/**
|
||||
* MCP daemon pidfile identity helpers.
|
||||
*
|
||||
* Pidfiles alone are unsafe after PID reuse (e.g. post-reboot). Callers must
|
||||
* confirm a recorded PID still belongs to a qmd process before signalling it
|
||||
* or treating it as "already running".
|
||||
*/
|
||||
|
||||
import { existsSync, readFileSync } from "node:fs";
|
||||
import { execFileSync } from "node:child_process";
|
||||
|
||||
/** True if a process command line looks like a qmd CLI invocation. */
|
||||
export function looksLikeQmdMcpCommand(cmdline: string): boolean {
|
||||
const s = cmdline.trim();
|
||||
if (!s) return false;
|
||||
// Match bare `qmd`, `qmd.ts`/`qmd.js`, or a path ending in /qmd(.ts|.js)
|
||||
return /(?:^|[\s/\\])qmd(?:\.(?:ts|js))?(?:[\s]|$)/i.test(s);
|
||||
}
|
||||
|
||||
/** Read process cmdline (Linux /proc preferred; ps fallback for macOS). */
|
||||
export function readProcessCmdline(pid: number): string | null {
|
||||
if (!Number.isInteger(pid) || pid <= 0) return null;
|
||||
|
||||
const procPath = `/proc/${pid}/cmdline`;
|
||||
if (existsSync(procPath)) {
|
||||
try {
|
||||
const raw = readFileSync(procPath, "utf-8");
|
||||
const cmdline = raw.replace(/\0/g, " ").trim();
|
||||
if (cmdline) return cmdline;
|
||||
} catch {
|
||||
// fall through to ps
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
let cmdline = "";
|
||||
try {
|
||||
cmdline = execFileSync("ps", ["-p", String(pid), "-o", "args="], {
|
||||
encoding: "utf-8",
|
||||
timeout: 2000,
|
||||
stdio: ["ignore", "pipe", "ignore"],
|
||||
});
|
||||
} catch {
|
||||
cmdline = execFileSync("ps", ["-p", String(pid), "-o", "command="], {
|
||||
encoding: "utf-8",
|
||||
timeout: 2000,
|
||||
stdio: ["ignore", "pipe", "ignore"],
|
||||
});
|
||||
}
|
||||
const trimmed = cmdline.trim();
|
||||
return trimmed || null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true only if `pid` is alive AND its command line looks like qmd.
|
||||
* If cmdline cannot be read or does not match, returns false (treat as stale).
|
||||
*/
|
||||
export function isQmdMcpPid(pid: number): boolean {
|
||||
if (!Number.isInteger(pid) || pid <= 0) return false;
|
||||
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
||||
const cmdline = readProcessCmdline(pid);
|
||||
if (!cmdline) return false;
|
||||
return looksLikeQmdMcpCommand(cmdline);
|
||||
}
|
||||
+25
-12
@@ -2,6 +2,7 @@ import { isBun, openDatabase } from "../db.js";
|
||||
import type { Database, SQLiteValue } from "../db.js";
|
||||
import fastGlob from "fast-glob";
|
||||
import { execSync, spawn as nodeSpawn } from "child_process";
|
||||
import { isQmdMcpPid } from "./mcp-pid.js";
|
||||
import { fileURLToPath } from "url";
|
||||
import { basename, dirname, join as pathJoin, relative as relativePath, resolve as pathResolve } from "path";
|
||||
import { parseArgs } from "util";
|
||||
@@ -502,12 +503,11 @@ async function showStatus(): Promise<void> {
|
||||
const mcpPidPath = resolve(mcpCacheDir, "mcp.pid");
|
||||
if (existsSync(mcpPidPath)) {
|
||||
const mcpPid = parseInt(readFileSync(mcpPidPath, "utf-8").trim());
|
||||
try {
|
||||
process.kill(mcpPid, 0);
|
||||
if (isQmdMcpPid(mcpPid)) {
|
||||
console.log(`MCP: ${c.green}running${c.reset} (PID ${mcpPid})`);
|
||||
} catch {
|
||||
unlinkSync(mcpPidPath);
|
||||
// Stale PID file cleaned up silently
|
||||
} else {
|
||||
try { unlinkSync(mcpPidPath); } catch { /* ignore */ }
|
||||
// Stale / recycled PID file cleaned up silently
|
||||
}
|
||||
}
|
||||
console.log("");
|
||||
@@ -4434,13 +4434,17 @@ if (isMain) {
|
||||
process.exit(0);
|
||||
}
|
||||
const pid = parseInt(readFileSync(pidPath, "utf-8").trim());
|
||||
if (!isQmdMcpPid(pid)) {
|
||||
try { unlinkSync(pidPath); } catch { /* ignore */ }
|
||||
console.log("Cleaned up stale PID file (server was not running).");
|
||||
process.exit(0);
|
||||
}
|
||||
try {
|
||||
process.kill(pid, 0); // alive?
|
||||
process.kill(pid, "SIGTERM");
|
||||
unlinkSync(pidPath);
|
||||
console.log(`Stopped QMD MCP server (PID ${pid}).`);
|
||||
} catch {
|
||||
unlinkSync(pidPath);
|
||||
try { unlinkSync(pidPath); } catch { /* ignore */ }
|
||||
console.log("Cleaned up stale PID file (server was not running).");
|
||||
}
|
||||
process.exit(0);
|
||||
@@ -4454,16 +4458,15 @@ if (isMain) {
|
||||
const host = cli.values.host ? String(cli.values.host) : undefined;
|
||||
|
||||
if (cli.values.daemon) {
|
||||
// Guard: check if already running
|
||||
// Guard: check if already running (identity-checked — recycled PIDs are stale)
|
||||
if (existsSync(pidPath)) {
|
||||
const existingPid = parseInt(readFileSync(pidPath, "utf-8").trim());
|
||||
try {
|
||||
process.kill(existingPid, 0); // alive?
|
||||
if (isQmdMcpPid(existingPid)) {
|
||||
console.error(`Already running (PID ${existingPid}). Run 'qmd mcp stop' first.`);
|
||||
process.exit(1);
|
||||
} catch {
|
||||
// Stale PID file — continue
|
||||
}
|
||||
// Stale or recycled PID file — remove and continue
|
||||
try { unlinkSync(pidPath); } catch { /* ignore */ }
|
||||
}
|
||||
|
||||
mkdirSync(cacheDir, { recursive: true });
|
||||
@@ -4492,6 +4495,16 @@ if (isMain) {
|
||||
// async cleanup handlers in startMcpHttpServer actually run.
|
||||
process.removeAllListeners("SIGTERM");
|
||||
process.removeAllListeners("SIGINT");
|
||||
// Best-effort: if this process owns the daemon pidfile, unlink on exit
|
||||
// (covers SIGTERM/SIGINT via startMcpHttpServer's process.exit).
|
||||
const unlinkOwnPidfile = () => {
|
||||
try {
|
||||
if (!existsSync(pidPath)) return;
|
||||
const written = parseInt(readFileSync(pidPath, "utf-8").trim());
|
||||
if (written === process.pid) unlinkSync(pidPath);
|
||||
} catch { /* ignore */ }
|
||||
};
|
||||
process.on("exit", unlinkOwnPidfile);
|
||||
const { startMcpHttpServer } = await import("../mcp/server.js");
|
||||
try {
|
||||
await startMcpHttpServer(port, { dbPath: getDbPath(), host });
|
||||
|
||||
@@ -2550,6 +2550,60 @@ describe("mcp http daemon", () => {
|
||||
await sleep(500);
|
||||
try { unlinkSync(pidPath()); } catch {}
|
||||
});
|
||||
|
||||
test("stop does not SIGTERM a live non-qmd PID from a recycled pidfile (#806)", async () => {
|
||||
// Stand-in for a recycled PID owner (must not be killed)
|
||||
const decoy = spawn("sleep", ["1000000"], { stdio: "ignore" });
|
||||
expect(decoy.pid).toBeTruthy();
|
||||
spawnedPids.push(decoy.pid!);
|
||||
writeFileSync(pidPath(), String(decoy.pid));
|
||||
|
||||
try {
|
||||
const { stdout, exitCode } = await runDaemonQmd(["mcp", "stop"]);
|
||||
expect(exitCode).toBe(0);
|
||||
expect(stdout).toContain("stale");
|
||||
expect(existsSync(pidPath())).toBe(false);
|
||||
|
||||
// Decoy must still be alive
|
||||
expect(() => process.kill(decoy.pid!, 0)).not.toThrow();
|
||||
} finally {
|
||||
decoy.kill("SIGTERM");
|
||||
await new Promise<void>((resolve) => decoy.once("close", () => resolve()));
|
||||
}
|
||||
});
|
||||
|
||||
test("--daemon treats live non-qmd pidfile PID as stale and starts (#806)", async () => {
|
||||
const decoy = spawn("sleep", ["1000000"], { stdio: "ignore" });
|
||||
expect(decoy.pid).toBeTruthy();
|
||||
spawnedPids.push(decoy.pid!);
|
||||
writeFileSync(pidPath(), String(decoy.pid));
|
||||
|
||||
const port = randomPort();
|
||||
try {
|
||||
const { stdout, stderr, exitCode } = await runDaemonQmd([
|
||||
"mcp", "--http", "--daemon", "--port", String(port),
|
||||
]);
|
||||
expect(exitCode).toBe(0);
|
||||
expect(stderr).not.toContain("Already running");
|
||||
expect(stdout).toContain(`http://localhost:${port}/mcp`);
|
||||
|
||||
const pid = parseInt(readFileSync(pidPath(), "utf-8").trim());
|
||||
spawnedPids.push(pid);
|
||||
expect(pid).not.toBe(decoy.pid);
|
||||
|
||||
// Decoy must still be alive
|
||||
expect(() => process.kill(decoy.pid!, 0)).not.toThrow();
|
||||
|
||||
const ready = await waitForServer(port);
|
||||
expect(ready).toBe(true);
|
||||
process.kill(pid, "SIGTERM");
|
||||
await sleep(500);
|
||||
try { unlinkSync(pidPath()); } catch {}
|
||||
} finally {
|
||||
decoy.kill("SIGTERM");
|
||||
await new Promise<void>((resolve) => decoy.once("close", () => resolve()));
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// =============================================================================
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
/**
|
||||
* Unit tests for MCP pidfile identity helpers (#806).
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from "vitest";
|
||||
import { looksLikeQmdMcpCommand, isQmdMcpPid } from "../src/cli/mcp-pid.ts";
|
||||
|
||||
describe("looksLikeQmdMcpCommand", () => {
|
||||
test("matches bare qmd and common CLI script paths", () => {
|
||||
expect(looksLikeQmdMcpCommand("qmd mcp --http --port 8181")).toBe(true);
|
||||
expect(looksLikeQmdMcpCommand("/usr/local/bin/qmd mcp --http")).toBe(true);
|
||||
expect(looksLikeQmdMcpCommand("node /home/me/qmd/src/cli/qmd.ts mcp --http")).toBe(true);
|
||||
expect(looksLikeQmdMcpCommand("node /home/me/qmd/dist/cli/qmd.js mcp --http")).toBe(true);
|
||||
expect(looksLikeQmdMcpCommand("tsx src/cli/qmd.ts mcp --http --daemon")).toBe(true);
|
||||
});
|
||||
|
||||
test("rejects empty / whitespace and unrelated processes", () => {
|
||||
expect(looksLikeQmdMcpCommand("")).toBe(false);
|
||||
expect(looksLikeQmdMcpCommand(" ")).toBe(false);
|
||||
expect(looksLikeQmdMcpCommand(
|
||||
"/System/Library/PrivateFrameworks/GenerativeExperiencesRuntime.framework/Versions/A/generativeexperiencesd",
|
||||
)).toBe(false);
|
||||
expect(looksLikeQmdMcpCommand("sleep 1000000")).toBe(false);
|
||||
expect(looksLikeQmdMcpCommand("node server.js")).toBe(false);
|
||||
});
|
||||
|
||||
test("does not match qmd as a substring of another token", () => {
|
||||
expect(looksLikeQmdMcpCommand("myqmdtool serve")).toBe(false);
|
||||
expect(looksLikeQmdMcpCommand("qmdfoo")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isQmdMcpPid", () => {
|
||||
test("returns false for invalid / dead PIDs", () => {
|
||||
expect(isQmdMcpPid(0)).toBe(false);
|
||||
expect(isQmdMcpPid(-1)).toBe(false);
|
||||
expect(isQmdMcpPid(1.5)).toBe(false);
|
||||
expect(isQmdMcpPid(999999999)).toBe(false);
|
||||
});
|
||||
|
||||
test("returns true for the current process when it looks like qmd", () => {
|
||||
// Vitest/tsx argv typically includes the test file, not qmd — so this
|
||||
// process itself usually fails the cmdline check. Assert the live+match
|
||||
// path using our own PID only when argv happens to include qmd; otherwise
|
||||
// just confirm a clearly-alive non-qmd PID (self) returns false.
|
||||
const self = process.pid;
|
||||
const argvJoined = process.argv.join(" ");
|
||||
if (looksLikeQmdMcpCommand(argvJoined)) {
|
||||
expect(isQmdMcpPid(self)).toBe(true);
|
||||
} else {
|
||||
expect(isQmdMcpPid(self)).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user