diff --git a/plugins/opencode/scripts/lib/server-lifecycle.mjs b/plugins/opencode/scripts/lib/server-lifecycle.mjs index e072b99..c730a3f 100644 --- a/plugins/opencode/scripts/lib/server-lifecycle.mjs +++ b/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); diff --git a/plugins/opencode/scripts/lib/state.mjs b/plugins/opencode/scripts/lib/state.mjs index 389a9fc..e7484cc 100644 --- a/plugins/opencode/scripts/lib/state.mjs +++ b/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,46 +142,92 @@ 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 { @@ -185,6 +235,15 @@ export function withStateLock(cwd, fn) { } } +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}`; diff --git a/plugins/opencode/scripts/opencode-companion.mjs b/plugins/opencode/scripts/opencode-companion.mjs index e0daa50..8067227 100644 --- a/plugins/opencode/scripts/opencode-companion.mjs +++ b/plugins/opencode/scripts/opencode-companion.mjs @@ -33,9 +33,8 @@ import { listJobs, resolveJobLogFile, setConfig, - updateState, - upsertJob, - withStateLock, + updateStateAsync, + withStateLockAsync, writeJobFile } from "./lib/state.mjs"; import { @@ -618,12 +617,12 @@ function spawnDetachedTaskWorker(cwd, jobId) { return child; } -function markQueuedTaskSpawned(workspaceRoot, jobId, pid) { +async function markQueuedTaskSpawned(workspaceRoot, jobId, pid) { if (pid == null) { return; } - updateState(workspaceRoot, (state) => { + await updateStateAsync(workspaceRoot, (state) => { const storedJob = readStoredJob(workspaceRoot, jobId); if (storedJob?.status === "completed" || storedJob?.status === "failed" || storedJob?.status === "cancelled") { return; @@ -642,12 +641,12 @@ function markQueuedTaskSpawned(workspaceRoot, jobId, pid) { }); } -function markQueuedTaskSpawnFailed(workspaceRoot, jobId, logFile, error) { +async function markQueuedTaskSpawnFailed(workspaceRoot, jobId, logFile, error) { const errorMessage = error instanceof Error ? error.message : String(error); const completedAt = nowIso(); appendLogLine(logFile, `Failed to start background task worker: ${errorMessage}`); - updateState(workspaceRoot, (state) => { + await updateStateAsync(workspaceRoot, (state) => { const storedJob = readStoredJob(workspaceRoot, jobId); const failedJob = { ...(storedJob ?? { id: jobId }), @@ -669,7 +668,7 @@ function markQueuedTaskSpawnFailed(workspaceRoot, jobId, logFile, error) { }); } -function enqueueBackgroundTask(cwd, job, request) { +async function enqueueBackgroundTask(cwd, job, request) { const { logFile } = createTrackedProgress(job); appendLogLine(logFile, "Queued for background execution."); @@ -682,19 +681,21 @@ function enqueueBackgroundTask(cwd, job, request) { request }; writeJobFile(job.workspaceRoot, job.id, queuedRecord); - upsertJob(job.workspaceRoot, queuedRecord); + await updateStateAsync(job.workspaceRoot, (state) => { + applyJobPatch(state, queuedRecord); + }); let child; try { child = spawnDetachedTaskWorker(cwd, job.id); } catch (error) { - markQueuedTaskSpawnFailed(job.workspaceRoot, job.id, logFile, error); + await markQueuedTaskSpawnFailed(job.workspaceRoot, job.id, logFile, error); throw error; } child.once("error", (error) => { - markQueuedTaskSpawnFailed(job.workspaceRoot, job.id, logFile, error); + void markQueuedTaskSpawnFailed(job.workspaceRoot, job.id, logFile, error); }); - markQueuedTaskSpawned(job.workspaceRoot, job.id, child.pid ?? null); + await markQueuedTaskSpawned(job.workspaceRoot, job.id, child.pid ?? null); return { payload: { @@ -804,7 +805,7 @@ async function handleTask(argv) { stopReview, jobId: job.id }); - const { payload } = enqueueBackgroundTask(cwd, job, request); + const { payload } = await enqueueBackgroundTask(cwd, job, request); outputCommandResult(payload, renderQueuedTaskLaunch(payload), options.json); return; } @@ -984,8 +985,8 @@ function terminalJobIndexPatch(job) { ); } -function syncTerminalJobIndex(workspaceRoot, job) { - updateState(workspaceRoot, (state) => { +async function syncTerminalJobIndex(workspaceRoot, job) { + await updateStateAsync(workspaceRoot, (state) => { applyJobPatch(state, terminalJobIndexPatch(job)); }); return { @@ -994,9 +995,9 @@ function syncTerminalJobIndex(workspaceRoot, job) { }; } -function readCurrentCancelJob(workspaceRoot, job) { +async function readCurrentCancelJob(workspaceRoot, job) { let currentJob = job; - withStateLock(workspaceRoot, () => { + await withStateLockAsync(workspaceRoot, () => { const stateJob = listJobs(workspaceRoot).find((candidate) => candidate.id === job.id) ?? null; const storedJob = readStoredJob(workspaceRoot, job.id); currentJob = { @@ -1008,11 +1009,11 @@ function readCurrentCancelJob(workspaceRoot, job) { return currentJob; } -function cancelJobIfStillActive(workspaceRoot, job, completedAt) { +async function cancelJobIfStillActive(workspaceRoot, job, completedAt) { let cancelled = false; let nextJob = job; - updateState(workspaceRoot, (state) => { + await updateStateAsync(workspaceRoot, (state) => { const stateJob = state.jobs.find((candidate) => candidate.id === job.id) ?? null; const storedJob = readStoredJob(workspaceRoot, job.id); const currentJob = { @@ -1066,10 +1067,10 @@ async function handleCancel(argv) { const cwd = resolveCommandCwd(options); const reference = positionals[0] ?? ""; const { workspaceRoot, job } = resolveCancelableJob(cwd, reference, { env: process.env }); - const currentJob = readCurrentCancelJob(workspaceRoot, job); + const currentJob = await readCurrentCancelJob(workspaceRoot, job); if (isTerminalStatus(currentJob.status)) { - const syncedJob = syncTerminalJobIndex(workspaceRoot, currentJob); + const syncedJob = await syncTerminalJobIndex(workspaceRoot, currentJob); const payload = { jobId: syncedJob.id, status: syncedJob.status, @@ -1088,7 +1089,7 @@ async function handleCancel(argv) { const interrupt = await interruptServerTurn(cwd, { threadId, serverUrl }); const completedAt = nowIso(); - const cancelResult = cancelJobIfStillActive(workspaceRoot, currentJob, completedAt); + const cancelResult = await cancelJobIfStillActive(workspaceRoot, currentJob, completedAt); if (cancelResult.cancelled) { const cancelLogFile = cancelResult.job.logFile ?? currentJob.logFile ?? job.logFile ?? resolveJobLogFile(workspaceRoot, cancelResult.job.id); diff --git a/plugins/opencode/scripts/session-lifecycle-hook.mjs b/plugins/opencode/scripts/session-lifecycle-hook.mjs index 869dd7f..42882e1 100644 --- a/plugins/opencode/scripts/session-lifecycle-hook.mjs +++ b/plugins/opencode/scripts/session-lifecycle-hook.mjs @@ -12,12 +12,14 @@ import { loadServerSession, teardownServerSession } from "./lib/server-lifecycle.mjs"; -import { resolveStateFile, updateState } from "./lib/state.mjs"; +import { resolveStateFile, updateStateAsync } from "./lib/state.mjs"; import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; export const SESSION_ID_ENV = "OPENCODE_COMPANION_SESSION_ID"; const PLUGIN_DATA_ENV = "CLAUDE_PLUGIN_DATA"; +const SESSION_END_STATE_LOCK_ACQUIRE_TIMEOUT_MS = 500; +const SESSION_END_SERVER_LOCK_ACQUIRE_TIMEOUT_MS = 3000; function readHookInput() { const raw = fs.readFileSync(0, "utf8").trim(); @@ -38,7 +40,7 @@ function appendEnvVar(name, value) { fs.appendFileSync(process.env.CLAUDE_ENV_FILE, `export ${name}=${shellEscape(value)}\n`, "utf8"); } -function cleanupSessionJobs(cwd, sessionId) { +async function cleanupSessionJobs(cwd, sessionId) { if (!cwd || !sessionId) { return; } @@ -50,10 +52,14 @@ function cleanupSessionJobs(cwd, sessionId) { } let removedJobs = []; - updateState(workspaceRoot, (state) => { - removedJobs = state.jobs.filter((job) => job.sessionId === sessionId); - state.jobs = state.jobs.filter((job) => job.sessionId !== sessionId); - }); + await updateStateAsync( + workspaceRoot, + (state) => { + removedJobs = state.jobs.filter((job) => job.sessionId === sessionId); + state.jobs = state.jobs.filter((job) => job.sessionId !== sessionId); + }, + { lockAcquireTimeoutMs: SESSION_END_STATE_LOCK_ACQUIRE_TIMEOUT_MS } + ); if (removedJobs.length === 0) { return; } @@ -92,7 +98,7 @@ async function handleSessionEnd(input) { : null); try { - cleanupSessionJobs(cwd, input.session_id || process.env[SESSION_ID_ENV]); + await cleanupSessionJobs(cwd, input.session_id || process.env[SESSION_ID_ENV]); } catch (error) { process.stderr.write( `OpenCode session job cleanup failed: ${error instanceof Error ? error.message : String(error)}\n` @@ -106,8 +112,12 @@ async function handleSessionEnd(input) { sessionDir: serverSession?.sessionDir ?? null, pid: serverSession?.pid ?? null, external: Boolean(serverSession?.external), - killProcess: terminateProcessTree + killProcess: terminateProcessTree, + lockAcquireTimeoutMs: SESSION_END_SERVER_LOCK_ACQUIRE_TIMEOUT_MS }); + if (teardown?.diagnostic) { + process.stderr.write(`${teardown.diagnostic}\n`); + } if (!teardown?.skipped) { clearServerSession(cwd); } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 3107146..c98e185 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -8,7 +8,7 @@ import { fileURLToPath } from "node:url"; import { interruptServerTurn, runServerTurn } from "../plugins/opencode/scripts/lib/opencode.mjs"; import { loadServerSession, saveServerSession, SERVER_URL_ENV } from "../plugins/opencode/scripts/lib/server-lifecycle.mjs"; -import { resolveJobLogFile, resolveStateFile, saveState, writeJobFile } from "../plugins/opencode/scripts/lib/state.mjs"; +import { resolveJobLogFile, resolveStateDir, resolveStateFile, saveState, writeJobFile } from "../plugins/opencode/scripts/lib/state.mjs"; import { buildEnv, installFakeOpencode, readFakeState, readServerBootCount } from "./fake-opencode-fixture.mjs"; import { initGitRepo, makeTempDir, run, writeExecutable } from "./helpers.mjs"; @@ -88,6 +88,35 @@ function runServerTurnEnv(env) { }; } +function runWithTimeout(command, args, options = {}, timeoutMs) { + return new Promise((resolve) => { + const child = spawn(command, args, { + cwd: options.cwd, + env: options.env, + stdio: ["pipe", "pipe", "pipe"], + windowsHide: true + }); + let stdout = ""; + let stderr = ""; + let timedOut = false; + child.stdout.on("data", (chunk) => { + stdout += chunk; + }); + child.stderr.on("data", (chunk) => { + stderr += chunk; + }); + const timeout = setTimeout(() => { + timedOut = true; + child.kill(); + }, timeoutMs); + child.on("close", (status, signal) => { + clearTimeout(timeout); + resolve({ status, signal, stdout, stderr, timedOut }); + }); + child.stdin.end(options.input ?? ""); + }); +} + function jsonResponse(value, status = 200) { return new Response(JSON.stringify(value), { status, @@ -230,6 +259,54 @@ test("session end leaves server session when teardown is skipped for active leas }); }); +test("session end bounds a contended server teardown lock and reports a diagnostic", async () => { + const workspace = makeTempDir(); + const pluginDataDir = makeTempDir("opencode-plugin-data-"); + + await withProcessEnv({ CLAUDE_PLUGIN_DATA: pluginDataDir }, async () => { + saveServerSession(workspace, { + url: "http://127.0.0.1:1", + pid: null, + pidFile: null, + logFile: null, + sessionDir: null, + external: false, + leases: [] + }); + const lockDir = path.join(resolveStateDir(workspace), "server.lock"); + fs.mkdirSync(lockDir, { recursive: true }); + fs.writeFileSync( + path.join(lockDir, "owner.json"), + `${JSON.stringify({ pid: process.pid, token: "other-holder", createdAt: new Date().toISOString() })}\n`, + "utf8" + ); + + try { + const result = await runWithTimeout( + "node", + [SESSION_HOOK, "SessionEnd"], + { + cwd: workspace, + env: { + ...process.env, + CLAUDE_PLUGIN_DATA: pluginDataDir, + OPENCODE_COMPANION_SESSION_ID: "sess-server-lock-contention" + }, + input: JSON.stringify({ cwd: workspace, session_id: "sess-server-lock-contention" }) + }, + 4200 + ); + + assert.equal(result.timedOut, false, "SessionEnd should finish within its five-second hook budget"); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stderr, /Timed out acquiring the OpenCode server lock for teardown/); + assert.equal(loadServerSession(workspace).url, "http://127.0.0.1:1"); + } finally { + fs.rmSync(lockDir, { recursive: true, force: true }); + } + }); +}); + test("stop review gate blocks when the enabled OpenCode reviewer is unavailable", async () => { const workspace = makeTempDir(); const binDir = makeTempDir(); diff --git a/tests/server-lifecycle.test.mjs b/tests/server-lifecycle.test.mjs index a519b62..41e0e42 100644 --- a/tests/server-lifecycle.test.mjs +++ b/tests/server-lifecycle.test.mjs @@ -1,11 +1,14 @@ import http from "node:http"; import net from "node:net"; import { spawn } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; import test from "node:test"; import assert from "node:assert/strict"; import { makeTempDir } from "./helpers.mjs"; import { ensureServer, isServerHealthy, loadServerSession, saveServerSession, teardownServerSession } from "../plugins/opencode/scripts/lib/server-lifecycle.mjs"; +import { resolveStateDir } from "../plugins/opencode/scripts/lib/state.mjs"; async function canListenLocalhost() { return new Promise((resolve) => { @@ -256,3 +259,188 @@ test("teardownServerSession still skips ignored self lease when another process } } }); + +test("teardownServerSession returns a bounded diagnostic when the server lock is contended", async () => { + const workspace = makeTempDir(); + const pluginDataDir = makeTempDir(); + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = pluginDataDir; + const lockDir = path.join(resolveStateDir(workspace), "server.lock"); + fs.mkdirSync(lockDir, { recursive: true }); + fs.writeFileSync( + path.join(lockDir, "owner.json"), + `${JSON.stringify({ pid: process.pid, token: "other-holder", createdAt: new Date().toISOString() })}\n`, + "utf8" + ); + + let teardownPromise; + let testTimeout; + try { + const startedAt = Date.now(); + teardownPromise = teardownServerSession({ + cwd: workspace, + lockAcquireTimeoutMs: 100, + lockPollMs: 25 + }); + const result = await Promise.race([ + teardownPromise, + new Promise((resolve) => { + testTimeout = setTimeout(() => resolve({ testTimeout: true }), 500); + }) + ]); + + assert.equal(result.testTimeout, undefined, "teardown should not wait indefinitely for a live lock holder"); + assert.equal(result.skipped, true); + assert.equal(result.reason, "lock-timeout"); + assert.match(result.diagnostic, /Timed out acquiring the OpenCode server lock/); + assert.ok(Date.now() - startedAt < 500, "teardown should honor its acquisition deadline"); + } finally { + clearTimeout(testTimeout); + fs.rmSync(lockDir, { recursive: true, force: true }); + await teardownPromise?.catch(() => {}); + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } +}); + +test("ensureServer rejects when its bounded server lock acquisition times out", async () => { + const workspace = makeTempDir(); + const pluginDataDir = makeTempDir(); + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + const previousFetch = globalThis.fetch; + process.env.CLAUDE_PLUGIN_DATA = pluginDataDir; + const url = "http://127.0.0.1:1"; + const lockDir = path.join(resolveStateDir(workspace), "server.lock"); + saveServerSession(workspace, { + url, + pid: 123456, + pidFile: null, + logFile: null, + sessionDir: null, + external: false + }); + fs.mkdirSync(lockDir, { recursive: true }); + fs.writeFileSync( + path.join(lockDir, "owner.json"), + `${JSON.stringify({ pid: process.pid, token: "other-holder", createdAt: new Date().toISOString() })}\n`, + "utf8" + ); + globalThis.fetch = async () => + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { "content-type": "application/json" } + }); + + try { + await assert.rejects( + ensureServer(workspace, { lockAcquireTimeoutMs: 100, lockPollMs: 25 }), + /Timed out acquiring the OpenCode server lock/ + ); + assert.equal(loadServerSession(workspace).url, url); + } finally { + fs.rmSync(lockDir, { recursive: true, force: true }); + globalThis.fetch = previousFetch; + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } +}); + +test("teardownServerSession expires a lease whose pid reports EPERM", async () => { + const workspace = makeTempDir(); + const pluginDataDir = makeTempDir(); + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + const originalKill = process.kill; + process.env.CLAUDE_PLUGIN_DATA = pluginDataDir; + const session = { + url: "http://127.0.0.1:1", + pid: 123456, + pidFile: null, + logFile: null, + sessionDir: null, + external: false, + leases: [ + { + pid: process.pid, + token: "foreign-owner-pid", + createdAt: new Date().toISOString(), + expiresAt: new Date(Date.now() + 60 * 60 * 1000).toISOString() + } + ] + }; + saveServerSession(workspace, session); + process.kill = (pid, signal) => { + if (pid === process.pid && signal === 0) { + const error = new Error("operation not permitted"); + error.code = "EPERM"; + throw error; + } + return originalKill(pid, signal); + }; + + try { + let killedPid = null; + const result = await teardownServerSession({ + cwd: workspace, + url: session.url, + pid: session.pid, + killProcess: (pid) => { + killedPid = pid; + } + }); + + assert.equal(result.skipped, false); + assert.equal(killedPid, session.pid); + assert.equal(loadServerSession(workspace), null); + } finally { + process.kill = originalKill; + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } +}); + +test("saveServerSession preserves the existing session when its atomic rename fails", () => { + const workspace = makeTempDir(); + const pluginDataDir = makeTempDir(); + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = pluginDataDir; + const originalSession = { + url: "http://127.0.0.1:1", + pid: 123456, + pidFile: null, + logFile: null, + sessionDir: null, + external: false + }; + const replacementSession = { ...originalSession, url: "http://127.0.0.1:2" }; + const originalRename = fs.renameSync; + + try { + saveServerSession(workspace, originalSession); + fs.renameSync = () => { + throw new Error("simulated rename failure"); + }; + + assert.throws(() => saveServerSession(workspace, replacementSession), /simulated rename failure/); + assert.deepEqual(loadServerSession(workspace), originalSession); + assert.equal( + fs.readdirSync(resolveStateDir(workspace)).some((name) => name.startsWith("server.json.tmp-")), + false + ); + } finally { + fs.renameSync = originalRename; + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } +}); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index 03b52ae..9abd1aa 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -5,7 +5,15 @@ import test from "node:test"; import assert from "node:assert/strict"; import { makeTempDir } from "./helpers.mjs"; -import { resolveJobFile, resolveJobLogFile, resolveStateDir, resolveStateFile, saveState } from "../plugins/opencode/scripts/lib/state.mjs"; +import { + resolveJobFile, + resolveJobLogFile, + resolveStateDir, + resolveStateFile, + saveState, + withStateLock, + withStateLockAsync +} from "../plugins/opencode/scripts/lib/state.mjs"; function withoutPluginData(fn) { const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; @@ -121,3 +129,64 @@ test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", ); }); }); + +test("withStateLockAsync yields to timers while another process holds the lock", async () => { + const workspace = makeTempDir(); + const pluginDataDir = makeTempDir(); + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = pluginDataDir; + const lockDir = path.join(resolveStateDir(workspace), "state.lock"); + fs.mkdirSync(lockDir, { recursive: true }); + fs.writeFileSync( + path.join(lockDir, "owner.json"), + `${JSON.stringify({ pid: process.pid, token: "other-holder", createdAt: new Date().toISOString() })}\n`, + "utf8" + ); + + const startedAt = Date.now(); + const timerDelay = new Promise((resolve) => setTimeout(() => resolve(Date.now() - startedAt), 25)); + const releaseTimer = setTimeout(() => fs.rmSync(lockDir, { recursive: true, force: true }), 100); + + try { + const lockPromise = withStateLockAsync(workspace, () => {}, { lockPollMs: 1000 }); + assert.ok((await timerDelay) < 500, "the event loop should process timers before the next lock poll"); + await lockPromise; + } finally { + clearTimeout(releaseTimer); + fs.rmSync(lockDir, { recursive: true, force: true }); + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } +}); + +test("withStateLock waits for stale-lock takeover by default", () => { + const workspace = makeTempDir(); + const pluginDataDir = makeTempDir(); + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + process.env.CLAUDE_PLUGIN_DATA = pluginDataDir; + const lockDir = path.join(resolveStateDir(workspace), "state.lock"); + fs.mkdirSync(lockDir, { recursive: true }); + fs.writeFileSync( + path.join(lockDir, "owner.json"), + `${JSON.stringify({ pid: process.pid, token: "stuck-holder", createdAt: new Date().toISOString() })}\n`, + "utf8" + ); + + try { + const startedAt = Date.now(); + const acquired = withStateLock(workspace, () => true, { lockPollMs: 25, lockStaleMs: 5600 }); + + assert.equal(acquired, true); + assert.ok(Date.now() - startedAt >= 5500, "the lock should be acquired by stale takeover after the old five-second deadline"); + } finally { + fs.rmSync(lockDir, { recursive: true, force: true }); + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } +});