Fix #19: server-lifecycle & state-lock hardening

plugins/opencode/scripts/lib/server-lifecycle.mjs

@@ -6,7 +6,7 @@ import process from "node:process";
 import { spawn } from "node:child_process";
 
 import { OpencodeServerClient } from "./opencode-server.mjs";
-import { resolveStateDir } from "./state.mjs";
+import { atomicWriteFile, resolveStateDir } from "./state.mjs";
 
 export const SERVER_URL_ENV = "OPENCODE_COMPANION_SERVER_URL";
 export const PID_FILE_ENV = "OPENCODE_COMPANION_SERVER_PID_FILE";
@@ -57,7 +57,7 @@ export function loadServerSession(cwd) {
 export function saveServerSession(cwd, session) {
   const stateDir = resolveStateDir(cwd);
   fs.mkdirSync(stateDir, { recursive: true });
-  fs.writeFileSync(resolveServerStateFile(cwd), `${JSON.stringify(session, null, 2)}\n`, "utf8");
+  atomicWriteFile(resolveServerStateFile(cwd), `${JSON.stringify(session, null, 2)}\n`);
 }
 
 export function clearServerSession(cwd) {
@@ -112,7 +112,7 @@ function processIsAlive(pid) {
     process.kill(pid, 0);
     return true;
   } catch (error) {
-    return error?.code === "EPERM";
+    return error?.code === "EPERM" ? null : false;
   }
 }
 
@@ -212,8 +212,10 @@ function isLeaseActive(lease, nowMs = Date.now()) {
     return false;
   }
 
-  const alive = processIsAlive(Number(lease?.pid));
-  return alive !== false;
+  // Leases are created by this plugin, so an EPERM response cannot prove that
+  // the pid still belongs to the lease owner. Keep the long TTL for legitimate
+  // sessions, but do not let an inaccessible, reused foreign pid hold teardown.
+  return processIsAlive(Number(lease?.pid)) === true;
 }
 
 function pruneServerLeases(session) {
@@ -253,6 +255,8 @@ async function acquireServerLock(cwd, options = {}) {
   const lockDir = resolveServerLockDir(cwd);
   const staleMs = Math.max(1000, Number(options.lockStaleMs) || DEFAULT_LOCK_STALE_MS);
   const pollMs = Math.max(25, Number(options.lockPollMs) || DEFAULT_LOCK_POLL_MS);
+  const requestedTimeoutMs = options.lockAcquireTimeoutMs == null ? Number.NaN : Number(options.lockAcquireTimeoutMs);
+  const deadline = Number.isFinite(requestedTimeoutMs) && requestedTimeoutMs >= 0 ? Date.now() + requestedTimeoutMs : null;
   const token = `${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`;
 
   for (;;) {
@@ -268,7 +272,10 @@ async function acquireServerLock(cwd, options = {}) {
       if (isServerLockStale(lockDir, staleMs)) {
         stealStaleServerLock(lockDir);
       }
-      await sleep(pollMs);
+      if (deadline != null && Date.now() >= deadline) {
+        return null;
+      }
+      await sleep(deadline == null ? pollMs : Math.min(pollMs, Math.max(1, deadline - Date.now())));
       continue;
     }
 
@@ -366,6 +373,9 @@ export async function ensureServer(cwd, options = {}) {
   }
 
   const lock = await acquireServerLock(cwd, options);
+  if (!lock) {
+    throw new Error("Timed out acquiring the OpenCode server lock.");
+  }
 
   try {
     const lockedExisting = await loadHealthyServerSession(cwd, options.healthTimeoutMs ?? 500);
@@ -485,6 +495,9 @@ export async function teardownServerSession({
   cwd = null,
   force = false,
   ignoreCurrentProcessLease = false,
+  lockAcquireTimeoutMs = null,
+  lockPollMs = null,
+  lockStaleMs = null,
   url = null,
   pidFile = null,
   logFile = null,
@@ -505,7 +518,14 @@ export async function teardownServerSession({
     });
   }
 
-  const lock = await acquireServerLock(cwd);
+  const lock = await acquireServerLock(cwd, { lockAcquireTimeoutMs, lockPollMs, lockStaleMs });
+  if (!lock) {
+    return {
+      skipped: true,
+      reason: "lock-timeout",
+      diagnostic: "Timed out acquiring the OpenCode server lock for teardown."
+    };
+  }
   try {
     const current = loadServerSession(cwd);
     const currentUrl = normalizeUrl(current?.url);

plugins/opencode/scripts/lib/state.mjs

@@ -17,6 +17,10 @@ const DEFAULT_LOCK_STALE_MS = 30000;
 const DEFAULT_LOCK_POLL_MS = 25;
 const sleepBuffer = new Int32Array(new SharedArrayBuffer(4));
 
+function sleep(ms) {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
 function nowIso() {
   return new Date().toISOString();
 }
@@ -138,53 +142,108 @@ function releaseStateLock(lockDir, token) {
   removeStateLock(lockDir);
 }
 
-function acquireStateLock(cwd, options = {}) {
+function stateLockOptions(cwd, options = {}) {
   const stateDir = resolveStateDir(cwd);
   fs.mkdirSync(stateDir, { recursive: true });
 
-  const lockDir = resolveStateLockDir(cwd);
-  const staleMs = Math.max(1000, Number(options.lockStaleMs) || DEFAULT_LOCK_STALE_MS);
-  const pollMs = Math.max(10, Number(options.lockPollMs) || DEFAULT_LOCK_POLL_MS);
-  const token = `${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`;
+  return {
+    lockDir: resolveStateLockDir(cwd),
+    staleMs: Math.max(1000, Number(options.lockStaleMs) || DEFAULT_LOCK_STALE_MS),
+    pollMs: Math.max(10, Number(options.lockPollMs) || DEFAULT_LOCK_POLL_MS),
+    token: `${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`
+  };
+}
 
-  for (;;) {
-    try {
-      fs.mkdirSync(lockDir);
-    } catch (error) {
-      if (error?.code !== "EEXIST") {
-        throw error;
-      }
-      if (isStateLockStale(lockDir, staleMs)) {
-        stealStaleStateLock(lockDir);
-      }
-      Atomics.wait(sleepBuffer, 0, 0, pollMs);
-      continue;
-    }
+function lockAcquisitionDeadline(options) {
+  if (options.lockAcquireTimeoutMs == null) {
+    return null;
+  }
 
-    try {
-      fs.writeFileSync(
-        path.join(lockDir, LOCK_INFO_FILE),
-        `${JSON.stringify({ pid: process.pid, token, createdAt: new Date().toISOString() }, null, 2)}\n`,
-        "utf8"
-      );
-    } catch (error) {
-      removeStateLock(lockDir);
+  const timeoutMs = Number(options.lockAcquireTimeoutMs);
+  return Number.isFinite(timeoutMs) && timeoutMs >= 0 ? Date.now() + timeoutMs : null;
+}
+
+function tryAcquireStateLock(lockDir, staleMs, token) {
+  try {
+    fs.mkdirSync(lockDir);
+  } catch (error) {
+    if (error?.code !== "EEXIST") {
       throw error;
     }
+    if (isStateLockStale(lockDir, staleMs)) {
+      stealStaleStateLock(lockDir);
+    }
+    return null;
+  }
 
-    return () => releaseStateLock(lockDir, token);
+  try {
+    fs.writeFileSync(
+      path.join(lockDir, LOCK_INFO_FILE),
+      `${JSON.stringify({ pid: process.pid, token, createdAt: new Date().toISOString() }, null, 2)}\n`,
+      "utf8"
+    );
+  } catch (error) {
+    removeStateLock(lockDir);
+    throw error;
   }
+
+  return () => releaseStateLock(lockDir, token);
 }
 
-export function withStateLock(cwd, fn) {
-  const release = acquireStateLock(cwd);
+function acquireStateLock(cwd, options = {}) {
+  const { lockDir, staleMs, pollMs, token } = stateLockOptions(cwd, options);
+  const deadline = lockAcquisitionDeadline(options);
+
+  for (;;) {
+    const release = tryAcquireStateLock(lockDir, staleMs, token);
+    if (release) {
+      return release;
+    }
+    if (deadline != null && Date.now() >= deadline) {
+      throw new Error("Timed out acquiring the OpenCode state lock.");
+    }
+    Atomics.wait(sleepBuffer, 0, 0, deadline == null ? pollMs : Math.min(pollMs, Math.max(1, deadline - Date.now())));
+  }
+}
+
+async function acquireStateLockAsync(cwd, options = {}) {
+  const { lockDir, staleMs, pollMs, token } = stateLockOptions(cwd, options);
+  const deadline = lockAcquisitionDeadline(options);
+
+  for (;;) {
+    const release = tryAcquireStateLock(lockDir, staleMs, token);
+    if (release) {
+      return release;
+    }
+    if (deadline != null && Date.now() >= deadline) {
+      throw new Error("Timed out acquiring the OpenCode state lock.");
+    }
+    await sleep(deadline == null ? pollMs : Math.min(pollMs, Math.max(1, deadline - Date.now())));
+  }
+}
+
+// The synchronous API remains for callers that need a synchronous return value.
+// It waits for acquisition or stale-lock recovery unless callers opt into a
+// lockAcquireTimeoutMs deadline. Async command and hook paths should use
+// withStateLockAsync so lock contention yields the Node event loop.
+export function withStateLock(cwd, fn, options = {}) {
+  const release = acquireStateLock(cwd, options);
   try {
     return fn();
   } finally {
     release();
   }
 }
 
+export async function withStateLockAsync(cwd, fn, options = {}) {
+  const release = await acquireStateLockAsync(cwd, options);
+  try {
+    return await fn();
+  } finally {
+    release();
+  }
+}
+
 function loadStateUnlocked(cwd) {
   const stateFile = resolveStateFile(cwd);
   if (!fs.existsSync(stateFile)) {
@@ -223,7 +282,7 @@ function removeFileIfExists(filePath) {
   }
 }
 
-function atomicWriteFile(filePath, contents) {
+export function atomicWriteFile(filePath, contents) {
   const tempFile = `${filePath}.tmp-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`;
   try {
     fs.writeFileSync(tempFile, contents, "utf8");
@@ -296,6 +355,15 @@ export function updateState(cwd, mutate) {
   });
 }
 
+export function updateStateAsync(cwd, mutate, options = {}) {
+  return withStateLockAsync(cwd, () => {
+    const state = loadStateUnlocked(cwd);
+    const previousJobs = [...state.jobs];
+    mutate(state);
+    return saveStateUnlocked(cwd, state, previousJobs);
+  }, options);
+}
+
 export function generateJobId(prefix = "job") {
   const random = Math.random().toString(36).slice(2, 8);
   return `${prefix}-${Date.now().toString(36)}-${random}`;