mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
Notify about the conflict between gbrain serve (MCP) and CLI commands (#3243)
* Notify about gbrain serve and CLI conflict * Handle serve flags in PGLite lock notice --------- Co-authored-by: Francois de Fitte <4712833+fdefitte@users.noreply.github.com>
This commit is contained in:
co-authored by
Francois de Fitte
parent
4be9d112cb
commit
920aea5eb8
File diff suppressed because one or more lines are too long
+36
-7
@@ -16,6 +16,7 @@
|
||||
|
||||
import { mkdirSync, existsSync, readFileSync, writeFileSync, rmSync, statSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { parseGlobalFlags } from './cli-options.ts';
|
||||
|
||||
const LOCK_DIR_NAME = '.gbrain-lock';
|
||||
const LOCK_FILE = 'lock';
|
||||
@@ -24,6 +25,21 @@ const LOCK_FILE = 'lock';
|
||||
// LIVE holder (embed jobs run for many minutes) is never mistaken for stale.
|
||||
const HEARTBEAT_INTERVAL_MS = 30_000;
|
||||
|
||||
class LiveServeLockError extends Error {}
|
||||
|
||||
function isServeCommand(lockData: { subcommand?: unknown; command?: unknown }): boolean {
|
||||
// New lock files store the command after the same global-flag parsing used
|
||||
// by cli.ts. This survives paths with spaces and forms such as
|
||||
// `gbrain --quiet serve` without confusing `gbrain search serve`.
|
||||
if (typeof lockData.subcommand === 'string') return lockData.subcommand === 'serve';
|
||||
|
||||
const command = lockData.command;
|
||||
if (typeof command !== 'string') return false;
|
||||
const parts = command.trim().split(/\s+/);
|
||||
// Backward compatibility for locks created before `subcommand` was stored.
|
||||
return parts[0] === 'serve' || parts[1] === 'serve';
|
||||
}
|
||||
|
||||
// #2348: there is NO steal-on-stale-heartbeat anymore. A holder whose PID is
|
||||
// alive is NEVER reaped, regardless of how long its heartbeat has been stale.
|
||||
// PGLite/WASM is strictly single-writer; the heartbeat runs on the JS event
|
||||
@@ -32,9 +48,9 @@ const HEARTBEAT_INTERVAL_MS = 30_000;
|
||||
// Reaping it (the old #2058 grace window) let a second OS process open the same
|
||||
// data dir and corrupt the catalog + pgvector extension state (58P01 /
|
||||
// internal_load_library / `type "vector" does not exist`), recoverable only by
|
||||
// wipe+restore. Only a DEAD PID is reaped now; a wedged-but-alive or PID-reused
|
||||
// holder makes the acquire time out with a message naming the PID (the user
|
||||
// removes the lock explicitly) rather than risk corruption.
|
||||
// wipe+restore. Only a DEAD PID is reaped now. A live serve-tagged holder gets
|
||||
// the immediate process-conflict explanation below; other wedged-but-alive or
|
||||
// PID-reused holders time out. Neither path steals the lock.
|
||||
|
||||
export interface LockHandle {
|
||||
lockDir: string;
|
||||
@@ -145,13 +161,25 @@ export async function acquireLock(dataDir: string | undefined, opts?: { timeoutM
|
||||
// Holder process is gone — reap and try to acquire.
|
||||
try { rmSync(lockDir, { recursive: true, force: true }); } catch { /* race condition, try again */ }
|
||||
} else {
|
||||
// Live holder — wait and retry. If it is genuinely wedged (or its PID
|
||||
// was reused by an unrelated process), the acquire times out below
|
||||
// with a message naming the PID; we never force-steal a live holder.
|
||||
if (isServeCommand(lockData)) {
|
||||
throw new LiveServeLockError(
|
||||
`GBrain's local database is already open through \`gbrain serve\` (MCP, PID ${lockPid}). ` +
|
||||
`This brain uses PGLite, so a separate CLI process cannot open it at the same time. ` +
|
||||
`Stop \`gbrain serve\`, then retry this CLI command. ` +
|
||||
`Or keep it running and use its MCP tools instead. ` +
|
||||
`A process with the recorded PID is still running, so GBrain will not remove ${lockDir} automatically.`,
|
||||
);
|
||||
}
|
||||
// Other live holders may be short-lived, so wait and retry. If one is
|
||||
// genuinely wedged (or its PID was reused), the acquire times out;
|
||||
// we never force-steal a live holder.
|
||||
await new Promise(r => setTimeout(r, 1000));
|
||||
continue;
|
||||
}
|
||||
} catch {
|
||||
} catch (err) {
|
||||
// A live MCP server is not a stale or corrupt lock. Surface the useful
|
||||
// explanation without touching the lock it still owns.
|
||||
if (err instanceof LiveServeLockError) throw err;
|
||||
// Corrupt lock file — remove it
|
||||
try { rmSync(lockDir, { recursive: true, force: true }); } catch { /* race condition */ }
|
||||
}
|
||||
@@ -169,6 +197,7 @@ export async function acquireLock(dataDir: string | undefined, opts?: { timeoutM
|
||||
acquired_at: now,
|
||||
refreshed_at: now,
|
||||
command: process.argv.slice(1).join(' '),
|
||||
subcommand: parseGlobalFlags(process.argv.slice(2)).rest[0] ?? null,
|
||||
}), { mode: 0o644 });
|
||||
|
||||
const ownerToken = tokenOf({ pid: process.pid, acquired_at: now });
|
||||
|
||||
@@ -109,7 +109,13 @@ describe('pglite-lock #2058 heartbeat + steal-grace', () => {
|
||||
if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function writeHolder(fields: { pid: number; acquiredAgoMs: number; refreshedAgoMs: number }) {
|
||||
function writeHolder(fields: {
|
||||
pid: number;
|
||||
acquiredAgoMs: number;
|
||||
refreshedAgoMs: number;
|
||||
command?: string;
|
||||
subcommand?: string;
|
||||
}) {
|
||||
const lockDir = join(TEST_DIR, '.gbrain-lock');
|
||||
mkdirSync(lockDir, { recursive: true });
|
||||
const now = Date.now();
|
||||
@@ -117,10 +123,70 @@ describe('pglite-lock #2058 heartbeat + steal-grace', () => {
|
||||
pid: fields.pid,
|
||||
acquired_at: now - fields.acquiredAgoMs,
|
||||
refreshed_at: now - fields.refreshedAgoMs,
|
||||
command: 'test holder',
|
||||
command: fields.command ?? 'test holder',
|
||||
...(fields.subcommand === undefined ? {} : { subcommand: fields.subcommand }),
|
||||
}));
|
||||
}
|
||||
|
||||
test('a live gbrain serve owner with global flags fails fast with a clear explanation', async () => {
|
||||
writeHolder({
|
||||
pid: process.pid,
|
||||
acquiredAgoMs: 60_000,
|
||||
refreshedAgoMs: 0,
|
||||
command: '/path with spaces/gbrain/src/cli.ts --quiet serve',
|
||||
subcommand: 'serve',
|
||||
});
|
||||
|
||||
const startedAt = Date.now();
|
||||
await expect(acquireLock(TEST_DIR, { timeoutMs: 5_000 })).rejects.toThrow(
|
||||
/already open through `gbrain serve`.*Stop `gbrain serve`, then retry this CLI command.*use its MCP tools instead.*will not remove/s,
|
||||
);
|
||||
|
||||
expect(Date.now() - startedAt).toBeLessThan(1_000);
|
||||
expect(existsSync(join(TEST_DIR, '.gbrain-lock'))).toBe(true);
|
||||
});
|
||||
|
||||
test('legacy serve lock metadata is still recognized', async () => {
|
||||
writeHolder({
|
||||
pid: process.pid,
|
||||
acquiredAgoMs: 60_000,
|
||||
refreshedAgoMs: 0,
|
||||
command: '/path/to/gbrain/src/cli.ts serve',
|
||||
});
|
||||
|
||||
await expect(acquireLock(TEST_DIR, { timeoutMs: 5_000 })).rejects.toThrow(
|
||||
/already open through `gbrain serve`/,
|
||||
);
|
||||
expect(existsSync(join(TEST_DIR, '.gbrain-lock'))).toBe(true);
|
||||
});
|
||||
|
||||
test('a search for the word serve is not mistaken for the MCP server', async () => {
|
||||
writeHolder({
|
||||
pid: process.pid,
|
||||
acquiredAgoMs: 60_000,
|
||||
refreshedAgoMs: 0,
|
||||
command: '/compiled/gbrain search serve',
|
||||
subcommand: 'search',
|
||||
});
|
||||
|
||||
await expect(acquireLock(TEST_DIR, { timeoutMs: 100 })).rejects.toThrow(/Timed out/);
|
||||
expect(existsSync(join(TEST_DIR, '.gbrain-lock'))).toBe(true);
|
||||
});
|
||||
|
||||
test('a dead gbrain serve owner is still cleaned up automatically', async () => {
|
||||
writeHolder({
|
||||
pid: 999999999,
|
||||
acquiredAgoMs: 60_000,
|
||||
refreshedAgoMs: 0,
|
||||
command: '/path/to/gbrain/src/cli.ts serve',
|
||||
subcommand: 'serve',
|
||||
});
|
||||
|
||||
const lock = await acquireLock(TEST_DIR, { timeoutMs: 2_000 });
|
||||
expect(lock.acquired).toBe(true);
|
||||
await releaseLock(lock);
|
||||
});
|
||||
|
||||
test('[REGRESSION] a LIVE holder with a fresh heartbeat is NOT stolen even when the lock is old', async () => {
|
||||
// The WAL-corruption bug: a >5min embed used to get its lock force-removed.
|
||||
// Now an alive holder that heartbeated recently is left alone regardless of
|
||||
|
||||
Reference in New Issue
Block a user