diff --git a/src/commands/autopilot.ts b/src/commands/autopilot.ts index 82979fb12..475b33745 100644 --- a/src/commands/autopilot.ts +++ b/src/commands/autopilot.ts @@ -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 || ''}` }; } diff --git a/src/commands/status.ts b/src/commands/status.ts index de223cf16..bdeea0283 100644 --- a/src/commands/status.ts +++ b/src/commands/status.ts @@ -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`.'); } diff --git a/test/autopilot-lock.test.ts b/test/autopilot-lock.test.ts index f3795ec5a..062722031 100644 --- a/test/autopilot-lock.test.ts +++ b/test/autopilot-lock.test.ts @@ -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(); + } + }); +});