Browse Source

fix: add PID file singleton guard to prevent duplicate companion spawns

Each OpenCode process calls spawnIfAvailable() independently with no
cross-process coordination, causing 68 companion processes to pile up.

Introduce a companion.pid file check: read the stored PID, verify the
process is alive with process.kill(pid, 0), skip spawn if running, or
clean up stale PID files from crashes. Write the PID after a successful
spawn and remove it on exit.

Fixes #650
Michael Henke 1 month ago
parent
commit
387bba90e0
2 changed files with 107 additions and 1 deletions
  1. 56 1
      src/companion/manager.test.ts
  2. 51 0
      src/companion/manager.ts

+ 56 - 1
src/companion/manager.test.ts

@@ -1,5 +1,11 @@
 import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
-import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
+import {
+  existsSync,
+  mkdirSync,
+  readFileSync,
+  rmSync,
+  writeFileSync,
+} from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
 import {
@@ -430,6 +436,55 @@ describe('CompanionManager', () => {
     expect(state.config.enabled).toBe(true);
   });
 
+  it('skips spawn when PID file points to a live process', () => {
+    // Write a PID file with our own PID (which is alive)
+    mkdirSync(path.dirname(stateFilePath()), { recursive: true });
+    const pidFile = path.join(path.dirname(stateFilePath()), 'companion.pid');
+    writeFileSync(pidFile, String(process.pid));
+
+    const m = make('test-pid-guard');
+    m.onLoad();
+
+    // Should not have spawned — PID file guard prevented it
+    // The session should still be written to state
+    const state = readState();
+    expect(state.sessions[0].session_id).toBe('test-pid-guard');
+  });
+
+  it('spawns when PID file contains a dead process', () => {
+    mkdirSync(path.dirname(stateFilePath()), { recursive: true });
+    const pidFile = path.join(path.dirname(stateFilePath()), 'companion.pid');
+    // Use an impossibly high PID that no kernel will ever assign
+    writeFileSync(pidFile, '999999999');
+
+    const m = make('test-stale-pid');
+    m.onLoad();
+
+    // Stale PID file should have been cleaned up
+    expect(existsSync(pidFile)).toBe(false);
+    const state = readState();
+    expect(state.sessions[0].session_id).toBe('test-stale-pid');
+  });
+
+  it('spawns when no PID file exists', () => {
+    const m = make('test-no-pid');
+    m.onLoad();
+    const state = readState();
+    expect(state.sessions[0].session_id).toBe('test-no-pid');
+  });
+
+  it('cleans up PID file on exit', () => {
+    mkdirSync(path.dirname(stateFilePath()), { recursive: true });
+    const pidFile = path.join(path.dirname(stateFilePath()), 'companion.pid');
+    writeFileSync(pidFile, String(process.pid));
+
+    const m = make('test-pid-cleanup');
+    m.onLoad();
+    m.onExit();
+
+    expect(existsSync(pidFile)).toBe(false);
+  });
+
   it('removes disabled session entries on load', () => {
     mkdirSync(path.dirname(stateFilePath()), { recursive: true });
     writeFileSync(

+ 51 - 0
src/companion/manager.ts

@@ -61,6 +61,30 @@ export function stateFilePath(): string {
   );
 }
 
+function pidFilePath(): string {
+  const xdg = process.env.XDG_DATA_HOME?.trim();
+  const base =
+    xdg && path.isAbsolute(xdg)
+      ? xdg
+      : path.join(os.homedir(), '.local', 'share');
+  return path.join(
+    base,
+    'opencode',
+    'storage',
+    'oh-my-opencode-slim',
+    'companion.pid',
+  );
+}
+
+function isProcessAlive(pid: number): boolean {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
 function defaultBinaryPath(): string {
   const xdg = process.env.XDG_DATA_HOME?.trim();
   const base =
@@ -260,6 +284,12 @@ export class CompanionManager {
   }
 
   onExit(): void {
+    if (this.wasSpawner) {
+      try {
+        const pf = pidFilePath();
+        if (existsSync(pf)) rmSync(pf, { force: true });
+      } catch {}
+    }
     activeManagers.delete(this);
     if (this.companionProcess) {
       try {
@@ -335,6 +365,23 @@ export class CompanionManager {
 
   private spawnIfAvailable(): void {
     if (this.config?.enabled !== true) return;
+    // Check if another process already spawned the companion
+    const pidFile = pidFilePath();
+    try {
+      if (existsSync(pidFile)) {
+        const existingPid = Number(readFileSync(pidFile, 'utf8').trim());
+        if (isProcessAlive(existingPid)) {
+          log('[companion] another instance already running, skipping spawn');
+          return;
+        }
+        // Stale PID file — process died without cleanup
+        log(
+          '[companion] removing stale PID file for dead process',
+          String(existingPid),
+        );
+        rmSync(pidFile, { force: true });
+      }
+    } catch {}
     const bin = resolveCompanionBinaryPath(this.config);
     if (!bin) {
       const expected = this.config.binaryPath?.trim() || defaultBinaryPath();
@@ -365,6 +412,10 @@ export class CompanionManager {
           debug: this.config.debug === true,
         }),
       );
+      try {
+        mkdirSync(path.dirname(pidFile), { recursive: true });
+        writeFileSync(pidFile, String(process.pid));
+      } catch {}
     } catch (err) {
       log('[companion] spawn failed', String(err));
     }