mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-29 19:01:39 +00:00
fix(autopilot): don't treat a recycled lock PID as a live autopilot (#2503)
PID liveness alone let a reused PID (observed: AirPlayUIAgent after reboot) block every new autopilot with 'another instance is running' while status reported running=true. decideLockAcquisition now verifies the holder's command line mentions gbrain (ps -p <pid> -o command=, fail-open when ps can't judge) and takes over foreign holders with an explicit reason; gbrain status mirrors the check (running=false + foreign_lock_holder flag + human-readable stale-lock hint). Fixes #2503 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
221238deed
commit
caf341c76d
@@ -161,9 +161,35 @@ export function isPidAlive(pid: number): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Issue #2503: PID liveness alone is not identity. After a reboot (or plain
|
||||
* PID-counter wraparound) the lock's PID can be reused by an unrelated
|
||||
* process (observed: Apple `AirPlayUIAgent`), which made every new autopilot
|
||||
* exit with "another instance is running" while maintenance silently stopped.
|
||||
*
|
||||
* Cheap identity check: does the holder's command line mention gbrain?
|
||||
* Covers the compiled binary (`/usr/local/bin/gbrain autopilot`) and dev runs
|
||||
* (`bun …/gbrain/src/cli.ts autopilot`). Fail-open: if `ps` is unavailable or
|
||||
* unreadable we return true (keep the pre-#2503 "alive = holder" behavior and
|
||||
* never steal a lock we can't judge).
|
||||
*/
|
||||
export function pidLooksLikeGbrain(pid: number): boolean {
|
||||
try {
|
||||
const cmd = execSync(`ps -p ${pid} -o command=`, {
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'ignore'],
|
||||
}).trim();
|
||||
if (!cmd) return true; // no command visible — can't judge, don't steal
|
||||
return /gbrain/i.test(cmd);
|
||||
} catch {
|
||||
return true; // ps failed (no ps, permissions, exotic OS) — fail-open
|
||||
}
|
||||
}
|
||||
|
||||
export function decideLockAcquisition(
|
||||
lockPath: string,
|
||||
currentPid: number,
|
||||
holderIsGbrain: (pid: number) => boolean = pidLooksLikeGbrain,
|
||||
): { action: 'acquire' } | { action: 'exit'; holderPid: number } | { action: 'takeover'; reason: string } {
|
||||
if (!existsSync(lockPath)) return { action: 'acquire' };
|
||||
|
||||
@@ -178,7 +204,12 @@ export function decideLockAcquisition(
|
||||
const sameProcess = Number.isFinite(holderPid) && holderPid === currentPid;
|
||||
const alive = !sameProcess && isPidAlive(holderPid);
|
||||
|
||||
if (alive) return { action: 'exit', holderPid };
|
||||
if (alive) {
|
||||
// #2503: a live PID that is not gbrain is a recycled PID, not a rival
|
||||
// autopilot — take the lock over instead of exiting forever.
|
||||
if (holderIsGbrain(holderPid)) return { action: 'exit', holderPid };
|
||||
return { action: 'takeover', reason: `foreign process holds stale lock (pid ${holderPid})` };
|
||||
}
|
||||
return { action: 'takeover', reason: `dead pid ${raw || '<empty>'}` };
|
||||
}
|
||||
|
||||
|
||||
+15
-1
@@ -39,6 +39,7 @@ import type { BrainEngine } from '../core/engine.ts';
|
||||
import { existsSync, readFileSync } from 'node:fs';
|
||||
import { gbrainPath, loadConfig, isThinClient } from '../core/config.ts';
|
||||
import { callRemoteTool, unpackToolResult } from '../core/mcp-client.ts';
|
||||
import { pidLooksLikeGbrain } from './autopilot.ts';
|
||||
import { VERSION } from '../version.ts';
|
||||
import {
|
||||
buildSyncStatusReport,
|
||||
@@ -101,6 +102,8 @@ export interface AutopilotStatus {
|
||||
lockfile_present: boolean;
|
||||
pid: number | null;
|
||||
running: boolean;
|
||||
/** #2503: lock PID is alive but belongs to a non-gbrain process (recycled PID). */
|
||||
foreign_lock_holder?: boolean;
|
||||
}
|
||||
|
||||
export interface StatusReport {
|
||||
@@ -302,6 +305,7 @@ function buildAutopilotStatus(): AutopilotStatus {
|
||||
const lockfile_present = existsSync(lockPath);
|
||||
let pid: number | null = null;
|
||||
let running = false;
|
||||
let foreign_lock_holder = false;
|
||||
if (lockfile_present) {
|
||||
try {
|
||||
const raw = readFileSync(lockPath, 'utf-8').trim();
|
||||
@@ -318,6 +322,12 @@ function buildAutopilotStatus(): AutopilotStatus {
|
||||
const code = (err as NodeJS.ErrnoException).code;
|
||||
running = code === 'EPERM';
|
||||
}
|
||||
// #2503: liveness is not identity — a recycled PID owned by an
|
||||
// unrelated process must not report autopilot as running.
|
||||
if (running && !pidLooksLikeGbrain(parsed)) {
|
||||
running = false;
|
||||
foreign_lock_holder = true;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
/* unreadable lockfile, leave pid=null/running=false */
|
||||
@@ -328,6 +338,7 @@ function buildAutopilotStatus(): AutopilotStatus {
|
||||
lockfile_present,
|
||||
pid,
|
||||
running,
|
||||
...(foreign_lock_holder ? { foreign_lock_holder } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -592,7 +603,10 @@ function renderHuman(report: StatusReport): string {
|
||||
if (a.running) {
|
||||
lines.push(` running (PID ${a.pid})`);
|
||||
} else if (a.lockfile_present) {
|
||||
lines.push(` stale lockfile (PID ${a.pid ?? '?'} not alive). Run \`gbrain autopilot --install\` to restart.`);
|
||||
const why = a.foreign_lock_holder
|
||||
? `PID ${a.pid ?? '?'} is held by a non-gbrain process`
|
||||
: `PID ${a.pid ?? '?'} not alive`;
|
||||
lines.push(` stale lockfile (${why}). Run \`gbrain autopilot --install\` to restart.`);
|
||||
} else {
|
||||
lines.push(' not running. Install with `gbrain autopilot --install`.');
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@ 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';
|
||||
import { decideLockAcquisition, isPidAlive, pidLooksLikeGbrain } from '../src/commands/autopilot.ts';
|
||||
|
||||
let tmp: string;
|
||||
let lockPath: string;
|
||||
@@ -43,12 +43,22 @@ describe('decideLockAcquisition', () => {
|
||||
|
||||
test('keeps a lock whose holder is alive regardless of age', () => {
|
||||
writeFileSync(lockPath, String(process.pid));
|
||||
expect(decideLockAcquisition(lockPath, process.pid + 100_000)).toEqual({
|
||||
expect(decideLockAcquisition(lockPath, process.pid + 100_000, () => true)).toEqual({
|
||||
action: 'exit',
|
||||
holderPid: process.pid,
|
||||
});
|
||||
});
|
||||
|
||||
// #2503: a live PID recycled by an unrelated process (e.g. AirPlayUIAgent)
|
||||
// must not block autopilot forever — identity mismatch means takeover.
|
||||
test('takes over a live lock held by a foreign (non-gbrain) process', () => {
|
||||
writeFileSync(lockPath, String(process.pid));
|
||||
expect(decideLockAcquisition(lockPath, process.pid + 100_000, () => false)).toEqual({
|
||||
action: 'takeover',
|
||||
reason: `foreign process holds stale lock (pid ${process.pid})`,
|
||||
});
|
||||
});
|
||||
|
||||
test('takes over malformed and empty locks', () => {
|
||||
writeFileSync(lockPath, 'not-a-pid');
|
||||
expect(decideLockAcquisition(lockPath, process.pid).action).toBe('takeover');
|
||||
@@ -56,3 +66,18 @@ describe('decideLockAcquisition', () => {
|
||||
expect(decideLockAcquisition(lockPath, process.pid).action).toBe('takeover');
|
||||
});
|
||||
});
|
||||
|
||||
describe('pidLooksLikeGbrain', () => {
|
||||
test('fails open on a dead pid (ps errors → true, never steal blind)', () => {
|
||||
expect(pidLooksLikeGbrain(4194303)).toBe(true);
|
||||
});
|
||||
|
||||
test('recognizes a live non-gbrain process as foreign', () => {
|
||||
const child = Bun.spawn(['sleep', '30']);
|
||||
try {
|
||||
expect(pidLooksLikeGbrain(child.pid)).toBe(false);
|
||||
} finally {
|
||||
child.kill();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user