mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(autopilot): verify lock holder process before exiting (#477)
A lock file can outlive its autopilot process after a crash or forced termination. The previous mtime-only check treated that file as proof that another instance was running, so supervisor restarts could exit repeatedly until the file aged out. Read the holder PID and probe it with signal 0. Keep the lock whenever that process is alive; take over only dead, malformed, empty, or self-owned locks. EPERM remains conservative and counts as alive, preventing two autopilot instances from running concurrently. Add hermetic tests for missing, live, dead, malformed, and empty lock states.
This commit is contained in:
@@ -130,6 +130,37 @@ export function shouldSpawnAutopilotWorker(args: string[]): boolean {
|
||||
return !args.includes('--no-worker');
|
||||
}
|
||||
|
||||
export function isPidAlive(pid: number): boolean {
|
||||
if (!Number.isFinite(pid) || pid <= 0) return false;
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
return true;
|
||||
} catch (error: unknown) {
|
||||
return (error as NodeJS.ErrnoException).code === 'EPERM';
|
||||
}
|
||||
}
|
||||
|
||||
export function decideLockAcquisition(
|
||||
lockPath: string,
|
||||
currentPid: number,
|
||||
): { action: 'acquire' } | { action: 'exit'; holderPid: number } | { action: 'takeover'; reason: string } {
|
||||
if (!existsSync(lockPath)) return { action: 'acquire' };
|
||||
|
||||
let raw = '';
|
||||
try {
|
||||
raw = readFileSync(lockPath, 'utf-8').trim();
|
||||
} catch {
|
||||
// An unreadable lock cannot prove another process is alive.
|
||||
}
|
||||
|
||||
const holderPid = Number.parseInt(raw, 10);
|
||||
const sameProcess = Number.isFinite(holderPid) && holderPid === currentPid;
|
||||
const alive = !sameProcess && isPidAlive(holderPid);
|
||||
|
||||
if (alive) return { action: 'exit', holderPid };
|
||||
return { action: 'takeover', reason: `dead pid ${raw || '<empty>'}` };
|
||||
}
|
||||
|
||||
// ── Self-upgrade silent channel (v0.42; opt-in, supervisor-relaunch) ─────────
|
||||
|
||||
/**
|
||||
@@ -351,14 +382,13 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
|
||||
const lockPath = gbrainHomePath('autopilot.lock');
|
||||
try {
|
||||
mkdirSync(gbrainHomePath(), { recursive: true });
|
||||
if (existsSync(lockPath)) {
|
||||
const stat = require('fs').statSync(lockPath);
|
||||
const ageMinutes = (Date.now() - stat.mtimeMs) / 60000;
|
||||
if (ageMinutes < 10) {
|
||||
console.error('Another autopilot instance is running (lock file is fresh). Exiting.');
|
||||
process.exit(0);
|
||||
}
|
||||
console.log('Stale lock file found (>10 min). Taking over.');
|
||||
const decision = decideLockAcquisition(lockPath, process.pid);
|
||||
if (decision.action === 'exit') {
|
||||
console.error(`Another autopilot instance is running (pid ${decision.holderPid}). Exiting.`);
|
||||
process.exit(0);
|
||||
}
|
||||
if (decision.action === 'takeover') {
|
||||
console.log(`Stale autopilot lock found (${decision.reason}). Taking over.`);
|
||||
}
|
||||
writeFileSync(lockPath, String(process.pid));
|
||||
} catch { /* best-effort */ }
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { decideLockAcquisition, isPidAlive } from '../src/commands/autopilot.ts';
|
||||
|
||||
let tmp: string;
|
||||
let lockPath: string;
|
||||
beforeEach(() => {
|
||||
tmp = mkdtempSync(join(tmpdir(), 'gbrain-autopilot-lock-'));
|
||||
lockPath = join(tmp, 'autopilot.lock');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('isPidAlive', () => {
|
||||
test('returns true for the current process', () => {
|
||||
expect(isPidAlive(process.pid)).toBe(true);
|
||||
});
|
||||
|
||||
test('returns false for invalid process ids', () => {
|
||||
expect(isPidAlive(0)).toBe(false);
|
||||
expect(isPidAlive(-1)).toBe(false);
|
||||
expect(isPidAlive(Number.NaN)).toBe(false);
|
||||
expect(isPidAlive(Number.POSITIVE_INFINITY)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('decideLockAcquisition', () => {
|
||||
test('acquires when no lock exists', () => {
|
||||
expect(decideLockAcquisition(lockPath, process.pid)).toEqual({ action: 'acquire' });
|
||||
});
|
||||
|
||||
test('takes over a lock whose holder is dead', () => {
|
||||
writeFileSync(lockPath, '4194303');
|
||||
expect(decideLockAcquisition(lockPath, process.pid)).toEqual({
|
||||
action: 'takeover',
|
||||
reason: 'dead pid 4194303',
|
||||
});
|
||||
});
|
||||
|
||||
test('keeps a lock whose holder is alive regardless of age', () => {
|
||||
writeFileSync(lockPath, String(process.pid));
|
||||
expect(decideLockAcquisition(lockPath, process.pid + 100_000)).toEqual({
|
||||
action: 'exit',
|
||||
holderPid: process.pid,
|
||||
});
|
||||
});
|
||||
|
||||
test('takes over malformed and empty locks', () => {
|
||||
writeFileSync(lockPath, 'not-a-pid');
|
||||
expect(decideLockAcquisition(lockPath, process.pid).action).toBe('takeover');
|
||||
writeFileSync(lockPath, '');
|
||||
expect(decideLockAcquisition(lockPath, process.pid).action).toBe('takeover');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user