diff --git a/src/lib/state/mcp-lifecycle-lock-acquisition.test.ts b/src/lib/state/mcp-lifecycle-lock-acquisition.test.ts index d53edcec57c..bb72667d5d1 100644 --- a/src/lib/state/mcp-lifecycle-lock-acquisition.test.ts +++ b/src/lib/state/mcp-lifecycle-lock-acquisition.test.ts @@ -30,7 +30,7 @@ function options() { return { stateDir, pollIntervalMs: 1, - timeoutMs: 20, + timeoutMs: 1_000, corruptLockGraceMs: 1, }; } @@ -129,9 +129,12 @@ describe("MCP lifecycle lock acquisition", () => { it("does not strand asynchronous recovery behind an expired legacy marker", async () => { writeTimerMarker(undefined, new Date(Date.now() - 1_000).toISOString()); - await expect(withMcpLifecycleLock(SANDBOX_NAME, () => "entered", options())).resolves.toBe( - "entered", - ); + await expect( + withMcpLifecycleLock(SANDBOX_NAME, () => "entered", { + ...options(), + monotonicNow: () => 0, + }), + ).resolves.toBe("entered"); }); it("does not strand synchronous recovery behind an expired legacy short-token marker", () => { diff --git a/src/lib/state/mcp-lifecycle-lock-acquisition.ts b/src/lib/state/mcp-lifecycle-lock-acquisition.ts index 4408b32849f..14292054740 100644 --- a/src/lib/state/mcp-lifecycle-lock-acquisition.ts +++ b/src/lib/state/mcp-lifecycle-lock-acquisition.ts @@ -79,6 +79,8 @@ export interface McpLifecycleLockOptions { pollIntervalMs?: number; timeoutMs?: number; corruptLockGraceMs?: number; + /** Monotonic clock override used only by deterministic deadline tests. */ + monotonicNow?: () => number; } interface HeldLockLease { @@ -170,6 +172,7 @@ function ensureDurableContainmentForStaleGenerationSync( stateDir: string, observation: LockObservation, reason: string, + assertAuthority?: () => void, ): void { const containmentPath = committedContainmentPath(lockPath); if (mcpLifecycleLockPathExistsSync(containmentPath)) return; @@ -182,6 +185,7 @@ function ensureDurableContainmentForStaleGenerationSync( sandboxName, readShieldsTimerTakeoverToken(sandboxName, stateDir), `${reason}; contained generation ${generation}`, + assertAuthority, ); } catch (error) { if (mcpLifecycleLockPathExistsSync(containmentPath)) return; @@ -219,10 +223,13 @@ function classifyObservedMcpLifecycleLock( sandboxName: string, corruptLockGraceMs: number, corruptTracker: CorruptGenerationTracker, + now: number, ): McpLifecycleLockDisposition { - if (!observation.owner || observation.owner.sandboxName !== sandboxName) { + if ( + (!observation.owner || observation.owner.sandboxName !== sandboxName) && + observation.reclaimable + ) { const generation = `${observation.dev}:${observation.ino}:${observation.mtimeMs}`; - const now = performance.now(); if (corruptTracker.generation !== generation) { corruptTracker.generation = generation; corruptTracker.firstSeenAt = now; @@ -232,12 +239,9 @@ function classifyObservedMcpLifecycleLock( } resetCorruptGenerationTracker(corruptTracker); // The wall-clock arguments are irrelevant for a structurally valid owner. - return classifyMcpLifecycleLock( - observation, - sandboxName, - observation.mtimeMs, - corruptLockGraceMs, - ); + return observation.owner === null + ? "wait" + : classifyMcpLifecycleLock(observation, sandboxName, observation.mtimeMs, corruptLockGraceMs); } function isValidMainOwnerForSandbox(observation: LockObservation, sandboxName: string): boolean { @@ -261,6 +265,8 @@ async function tryReapStaleMainLock( stateDir: string, corruptLockGraceMs: number, corruptTracker: CorruptGenerationTracker, + monotonicNow: () => number, + assertBeforeDeadline: () => void, ): Promise { const containmentPath = committedContainmentPath(lockPath); const deadlinePath = `${lockPath}.deadline`; @@ -275,9 +281,11 @@ async function tryReapStaleMainLock( const reaperPath = `${lockPath}.reaper`; const reaperToken = crypto.randomUUID(); const reaperOwner = createMcpLifecycleLockOwner(sandboxName, reaperToken, takeoverToken); + assertBeforeDeadline(); if (!(await writeMcpLifecycleLockCandidateAndLink(reaperPath, reaperOwner))) return false; try { + assertBeforeDeadline(); if ( (await mcpLifecycleLockPathExists(containmentPath)) || (await mcpLifecycleLockPathExists(deadlinePath)) || @@ -289,8 +297,13 @@ async function tryReapStaleMainLock( if (!latest) return true; if ( !isValidMainOwnerForSandbox(latest, sandboxName) || - classifyObservedMcpLifecycleLock(latest, sandboxName, corruptLockGraceMs, corruptTracker) !== - "stale" + classifyObservedMcpLifecycleLock( + latest, + sandboxName, + corruptLockGraceMs, + corruptTracker, + monotonicNow(), + ) !== "stale" ) { return false; } @@ -302,16 +315,20 @@ async function tryReapStaleMainLock( return false; } if (latest.owner?.shieldsTakeoverToken || currentTakeoverToken) { + assertBeforeDeadline(); ensureDurableContainmentForStaleGenerationSync( lockPath, sandboxName, stateDir, latest, "A timer-bound sandbox mutation owner exited before durable containment was committed", + assertBeforeDeadline, ); return false; } - return reclaimStaleMcpLifecycleLockGeneration(lockPath, latest); + assertBeforeDeadline(); + // Await reclamation so the finally block releases the reaper generation after reclamation or restoration completes. + return await reclaimStaleMcpLifecycleLockGeneration(lockPath, latest, assertBeforeDeadline); } finally { await safelyReleaseMcpLifecycleLock(reaperPath, reaperToken); } @@ -323,6 +340,8 @@ function tryReapStaleMainLockSync( stateDir: string, corruptLockGraceMs: number, corruptTracker: CorruptGenerationTracker, + monotonicNow: () => number, + assertBeforeDeadline: () => void, ): boolean { const containmentPath = committedContainmentPath(lockPath); const deadlinePath = `${lockPath}.deadline`; @@ -337,9 +356,11 @@ function tryReapStaleMainLockSync( const reaperPath = `${lockPath}.reaper`; const reaperToken = crypto.randomUUID(); const reaperOwner = createMcpLifecycleLockOwner(sandboxName, reaperToken, takeoverToken); + assertBeforeDeadline(); if (!writeMcpLifecycleLockCandidateAndLinkSync(reaperPath, reaperOwner)) return false; try { + assertBeforeDeadline(); if ( mcpLifecycleLockPathExistsSync(containmentPath) || mcpLifecycleLockPathExistsSync(deadlinePath) || @@ -351,8 +372,13 @@ function tryReapStaleMainLockSync( if (!latest) return true; if ( !isValidMainOwnerForSandbox(latest, sandboxName) || - classifyObservedMcpLifecycleLock(latest, sandboxName, corruptLockGraceMs, corruptTracker) !== - "stale" + classifyObservedMcpLifecycleLock( + latest, + sandboxName, + corruptLockGraceMs, + corruptTracker, + monotonicNow(), + ) !== "stale" ) { return false; } @@ -364,16 +390,19 @@ function tryReapStaleMainLockSync( return false; } if (latest.owner?.shieldsTakeoverToken || currentTakeoverToken) { + assertBeforeDeadline(); ensureDurableContainmentForStaleGenerationSync( lockPath, sandboxName, stateDir, latest, "A timer-bound sandbox mutation owner exited before durable containment was committed", + assertBeforeDeadline, ); return false; } - return reclaimStaleMcpLifecycleLockGenerationSync(lockPath, latest); + assertBeforeDeadline(); + return reclaimStaleMcpLifecycleLockGenerationSync(lockPath, latest, assertBeforeDeadline); } finally { safelyReleaseMcpLifecycleLockSync(reaperPath, reaperToken); } @@ -382,13 +411,14 @@ function tryReapStaleMainLockSync( async function acquireMcpLifecycleLock( sandboxName: string, options: McpLifecycleLockOptions, -): Promise { +): Promise void }> { const pollIntervalMs = positiveInteger(options.pollIntervalMs, DEFAULT_POLL_INTERVAL_MS); const timeoutMs = positiveInteger(options.timeoutMs, DEFAULT_TIMEOUT_MS); const corruptLockGraceMs = positiveInteger( options.corruptLockGraceMs, DEFAULT_CORRUPT_LOCK_GRACE_MS, ); + const monotonicNow = options.monotonicNow ?? (() => performance.now()); const stateDir = options.stateDir ?? resolveNemoclawStateDir(); const lockPath = getMcpLifecycleLockPath(sandboxName, stateDir); await fs.promises.mkdir(path.dirname(lockPath), { @@ -396,23 +426,25 @@ async function acquireMcpLifecycleLock( mode: 0o700, }); - const startedAt = performance.now(); + const deadline = monotonicNow() + timeoutMs; const corruptMainTracker: CorruptGenerationTracker = { generation: null, firstSeenAt: 0 }; const corruptReaperTracker: CorruptGenerationTracker = { generation: null, firstSeenAt: 0 }; const corruptDeadlineTracker: CorruptGenerationTracker = { generation: null, firstSeenAt: 0 }; let lastOwnerPid: number | null = null; + const assertBeforeDeadline = () => { + if (monotonicNow() < deadline) return; + const ownerSuffix = lastOwnerPid ? ` (owner pid ${lastOwnerPid})` : ""; + throw new Error( + `Timed out waiting for the sandbox mutation lock for '${sandboxName}'${ownerSuffix}. Another lifecycle, policy, channel, shields, or snapshot operation is still running.`, + ); + }; for (;;) { const containmentPath = committedContainmentPath(lockPath); const containment = await readMcpLifecycleLockObservation(containmentPath); if (containment) { throw committedContainmentActiveError(sandboxName, lockPath, containment); } - if (performance.now() - startedAt >= timeoutMs) { - const ownerSuffix = lastOwnerPid ? ` (owner pid ${lastOwnerPid})` : ""; - throw new Error( - `Timed out waiting for the sandbox mutation lock for '${sandboxName}'${ownerSuffix}. Another lifecycle, policy, channel, shields, or snapshot operation is still running.`, - ); - } + assertBeforeDeadline(); const deadlinePath = `${lockPath}.deadline`; const deadlineObservation = await readMcpLifecycleLockObservation(deadlinePath); @@ -422,14 +454,17 @@ async function acquireMcpLifecycleLock( sandboxName, corruptLockGraceMs, corruptDeadlineTracker, + monotonicNow(), ); if (deadlineDisposition === "stale") { + assertBeforeDeadline(); ensureDurableContainmentForStaleGenerationSync( lockPath, sandboxName, stateDir, deadlineObservation, "An auto-restore deadline owner exited before its recovery operation completed", + assertBeforeDeadline, ); continue; } @@ -446,14 +481,17 @@ async function acquireMcpLifecycleLock( sandboxName, corruptLockGraceMs, corruptReaperTracker, + monotonicNow(), ); if (reaperDisposition === "stale") { + assertBeforeDeadline(); ensureDurableContainmentForStaleGenerationSync( lockPath, sandboxName, stateDir, reaperObservation, "A stale-lock reaper exited before cleanup completed", + assertBeforeDeadline, ); continue; } @@ -480,6 +518,7 @@ async function acquireMcpLifecycleLock( const token = crypto.randomUUID(); const shieldsTakeoverToken = readShieldsTimerTakeoverToken(sandboxName, stateDir); const owner = createMcpLifecycleLockOwner(sandboxName, token, shieldsTakeoverToken); + assertBeforeDeadline(); if (await writeMcpLifecycleLockCandidateAndLink(lockPath, owner)) { // A stale-lock reaper may have appeared between our pre-check and the // atomic link. Do not enter the critical section until that generation @@ -491,9 +530,16 @@ async function acquireMcpLifecycleLock( !isShieldsTimerDeadlineExpired(sandboxName, stateDir) && readShieldsTimerTakeoverToken(sandboxName, stateDir) === shieldsTakeoverToken ) { + try { + assertBeforeDeadline(); + } catch (error) { + await safelyReleaseMcpLifecycleLock(lockPath, token); + throw error; + } return { lockPath, token, + assertBeforeDeadline, ...(shieldsTakeoverToken ? { shieldsTakeoverToken } : {}), }; } @@ -510,24 +556,30 @@ async function acquireMcpLifecycleLock( sandboxName, corruptLockGraceMs, corruptMainTracker, + monotonicNow(), ) === "stale" ) { if (isValidMainOwnerForSandbox(observation, sandboxName)) { + assertBeforeDeadline(); await tryReapStaleMainLock( lockPath, sandboxName, stateDir, corruptLockGraceMs, corruptMainTracker, + monotonicNow, + assertBeforeDeadline, ); continue; } + assertBeforeDeadline(); ensureDurableContainmentForStaleGenerationSync( lockPath, sandboxName, stateDir, observation, "A sandbox mutation owner exited before its descendants could be proven contained", + assertBeforeDeadline, ); continue; } @@ -541,7 +593,7 @@ async function acquireMcpLifecycleLock( function acquireMcpLifecycleLockSync( sandboxName: string, options: McpLifecycleLockOptions & { stateDir: string }, -): AcquiredMcpLifecycleLock { +): AcquiredMcpLifecycleLock & { assertBeforeDeadline: () => void } { const pollIntervalMs = positiveInteger(options.pollIntervalMs, DEFAULT_POLL_INTERVAL_MS); const timeoutMs = positiveInteger(options.timeoutMs, DEFAULT_TIMEOUT_MS); const corruptLockGraceMs = positiveInteger( @@ -551,24 +603,27 @@ function acquireMcpLifecycleLockSync( const lockPath = getMcpLifecycleLockPath(sandboxName, options.stateDir); fs.mkdirSync(path.dirname(lockPath), { recursive: true, mode: 0o700 }); - const startedAt = performance.now(); + const monotonicNow = options.monotonicNow ?? (() => performance.now()); + const deadline = monotonicNow() + timeoutMs; const corruptMainTracker: CorruptGenerationTracker = { generation: null, firstSeenAt: 0 }; const corruptReaperTracker: CorruptGenerationTracker = { generation: null, firstSeenAt: 0 }; const corruptDeadlineTracker: CorruptGenerationTracker = { generation: null, firstSeenAt: 0 }; let lastOwnerPid: number | null = null; + const assertBeforeDeadline = () => { + if (monotonicNow() < deadline) return; + throw new Error( + `Timed out waiting for sandbox mutation lock for '${sandboxName}'${ + lastOwnerPid ? ` (owner PID ${lastOwnerPid})` : "" + }`, + ); + }; for (;;) { const containmentPath = committedContainmentPath(lockPath); const containment = readMcpLifecycleLockObservationSync(containmentPath); if (containment) { throw committedContainmentActiveError(sandboxName, lockPath, containment); } - if (performance.now() - startedAt >= timeoutMs) { - throw new Error( - `Timed out waiting for sandbox mutation lock for '${sandboxName}'${ - lastOwnerPid ? ` (owner PID ${lastOwnerPid})` : "" - }`, - ); - } + assertBeforeDeadline(); const deadlinePath = `${lockPath}.deadline`; const deadlineObservation = readMcpLifecycleLockObservationSync(deadlinePath); @@ -578,14 +633,17 @@ function acquireMcpLifecycleLockSync( sandboxName, corruptLockGraceMs, corruptDeadlineTracker, + monotonicNow(), ); if (deadlineDisposition === "stale") { + assertBeforeDeadline(); ensureDurableContainmentForStaleGenerationSync( lockPath, sandboxName, options.stateDir, deadlineObservation, "An auto-restore deadline owner exited before its recovery operation completed", + assertBeforeDeadline, ); continue; } @@ -602,14 +660,17 @@ function acquireMcpLifecycleLockSync( sandboxName, corruptLockGraceMs, corruptReaperTracker, + monotonicNow(), ); if (reaperDisposition === "stale") { + assertBeforeDeadline(); ensureDurableContainmentForStaleGenerationSync( lockPath, sandboxName, options.stateDir, reaperObservation, "A stale-lock reaper exited before cleanup completed", + assertBeforeDeadline, ); continue; } @@ -631,6 +692,7 @@ function acquireMcpLifecycleLockSync( const token = crypto.randomUUID(); const shieldsTakeoverToken = readShieldsTimerTakeoverToken(sandboxName, options.stateDir); const owner = createMcpLifecycleLockOwner(sandboxName, token, shieldsTakeoverToken); + assertBeforeDeadline(); if (writeMcpLifecycleLockCandidateAndLinkSync(lockPath, owner)) { if ( !mcpLifecycleLockPathExistsSync(containmentPath) && @@ -639,9 +701,16 @@ function acquireMcpLifecycleLockSync( !isShieldsTimerDeadlineExpired(sandboxName, options.stateDir) && readShieldsTimerTakeoverToken(sandboxName, options.stateDir) === shieldsTakeoverToken ) { + try { + assertBeforeDeadline(); + } catch (error) { + safelyReleaseMcpLifecycleLockSync(lockPath, token); + throw error; + } return { lockPath, token, + assertBeforeDeadline, ...(shieldsTakeoverToken ? { shieldsTakeoverToken } : {}), }; } @@ -658,24 +727,30 @@ function acquireMcpLifecycleLockSync( sandboxName, corruptLockGraceMs, corruptMainTracker, + monotonicNow(), ) === "stale" ) { if (isValidMainOwnerForSandbox(observation, sandboxName)) { + assertBeforeDeadline(); tryReapStaleMainLockSync( lockPath, sandboxName, options.stateDir, corruptLockGraceMs, corruptMainTracker, + monotonicNow, + assertBeforeDeadline, ); continue; } + assertBeforeDeadline(); ensureDurableContainmentForStaleGenerationSync( lockPath, sandboxName, options.stateDir, observation, "A sandbox mutation owner exited before its descendants could be proven contained", + assertBeforeDeadline, ); continue; } @@ -885,6 +960,7 @@ async function acquireDeadlineFence( sandboxName, corruptLockGraceMs, corruptTracker, + performance.now(), ) === "stale" ) { ensureDurableContainmentForStaleGenerationSync( @@ -1000,6 +1076,7 @@ function acquireDeadlineFenceSync( sandboxName, corruptLockGraceMs, corruptTracker, + performance.now(), ) === "stale" ) { ensureDurableContainmentForStaleGenerationSync( @@ -1053,7 +1130,13 @@ async function clearDeadlineProtectedPath( const observed = await readMcpLifecycleLockObservation(targetPath); if (!observed) return; - const disposition = classifyObservedMcpLifecycleLock(observed, sandboxName, 0, corruptTracker); + const disposition = classifyObservedMcpLifecycleLock( + observed, + sandboxName, + 0, + corruptTracker, + performance.now(), + ); const owner = observed.owner; const exactLocalOwner = owner?.sandboxName === sandboxName && @@ -1148,7 +1231,13 @@ function clearDeadlineProtectedPathSync( const observed = readMcpLifecycleLockObservationSync(targetPath); if (!observed) return; - const disposition = classifyObservedMcpLifecycleLock(observed, sandboxName, 0, corruptTracker); + const disposition = classifyObservedMcpLifecycleLock( + observed, + sandboxName, + 0, + corruptTracker, + performance.now(), + ); const owner = observed.owner; const exactLocalOwner = owner?.sandboxName === sandboxName && @@ -1584,7 +1673,10 @@ export function withMcpLifecycleLockSync( context.set(lockPath, lease); let retainOwnedGate = false; try { - return heldLocks.run(context, operation); + return heldLocks.run(context, () => { + acquired.assertBeforeDeadline(); + return operation(); + }); } catch (error) { retainOwnedGate = Boolean(acquired.shieldsTakeoverToken) && @@ -1637,6 +1729,7 @@ export async function withMcpLifecycleLock( return heldLocks.run(context, async () => { let retainOwnedGate = false; try { + acquired.assertBeforeDeadline(); return await operation(); } catch (error) { retainOwnedGate = diff --git a/src/lib/state/mcp-lifecycle-lock-identity.test.ts b/src/lib/state/mcp-lifecycle-lock-identity.test.ts index 79f359a2bab..5d336ba0d4f 100644 --- a/src/lib/state/mcp-lifecycle-lock-identity.test.ts +++ b/src/lib/state/mcp-lifecycle-lock-identity.test.ts @@ -73,7 +73,7 @@ function owner( } function observation(lockOwner: McpLifecycleLockOwner | null, mtimeMs = 0): LockObservation { - return { owner: lockOwner, mtimeMs, dev: 10, ino: 20 }; + return { owner: lockOwner, mtimeMs, dev: 10, ino: 20, reclaimable: true }; } function probes( diff --git a/src/lib/state/mcp-lifecycle-lock-identity.ts b/src/lib/state/mcp-lifecycle-lock-identity.ts index 5b0690efcb5..3b49bb2dba5 100644 --- a/src/lib/state/mcp-lifecycle-lock-identity.ts +++ b/src/lib/state/mcp-lifecycle-lock-identity.ts @@ -32,6 +32,8 @@ export interface LockObservation { mtimeMs: number; dev: number; ino: number; + /** A directory cannot be restored with a hard link. */ + reclaimable: boolean; } export type McpLifecycleLockDisposition = "active" | "stale" | "wait"; @@ -205,7 +207,9 @@ export function classifyMcpLifecycleLock( ): McpLifecycleLockDisposition { const { owner } = observation; if (!owner || owner.sandboxName !== sandboxName) { - return nowMs - observation.mtimeMs >= corruptLockGraceMs ? "stale" : "wait"; + return observation.reclaimable && nowMs - observation.mtimeMs >= corruptLockGraceMs + ? "stale" + : "wait"; } // The lock coordinates local CLI processes, not independent hosts or PID // namespaces. Never use this process's PID table to reap a foreign owner; diff --git a/src/lib/state/mcp-lifecycle-lock-storage.ts b/src/lib/state/mcp-lifecycle-lock-storage.ts index d75caf4ac4e..43c0599e381 100644 --- a/src/lib/state/mcp-lifecycle-lock-storage.ts +++ b/src/lib/state/mcp-lifecycle-lock-storage.ts @@ -46,7 +46,13 @@ export async function readMcpLifecycleLockObservation( try { const stat = await fs.promises.lstat(lockPath); if (!stat.isFile() || stat.isSymbolicLink()) { - return { owner: null, mtimeMs: stat.mtimeMs, dev: stat.dev, ino: stat.ino }; + return { + owner: null, + mtimeMs: stat.mtimeMs, + dev: stat.dev, + ino: stat.ino, + reclaimable: !stat.isDirectory(), + }; } } catch (statError) { if (isErrnoException(statError) && statError.code === "ENOENT") return null; @@ -58,7 +64,13 @@ export async function readMcpLifecycleLockObservation( try { const stat = await handle.stat(); if (!stat.isFile()) { - return { owner: null, mtimeMs: stat.mtimeMs, dev: stat.dev, ino: stat.ino }; + return { + owner: null, + mtimeMs: stat.mtimeMs, + dev: stat.dev, + ino: stat.ino, + reclaimable: !stat.isDirectory(), + }; } try { const parsed: unknown = JSON.parse(await handle.readFile("utf8")); @@ -67,9 +79,16 @@ export async function readMcpLifecycleLockObservation( mtimeMs: stat.mtimeMs, dev: stat.dev, ino: stat.ino, + reclaimable: true, }; } catch { - return { owner: null, mtimeMs: stat.mtimeMs, dev: stat.dev, ino: stat.ino }; + return { + owner: null, + mtimeMs: stat.mtimeMs, + dev: stat.dev, + ino: stat.ino, + reclaimable: true, + }; } } finally { await handle.close(); @@ -88,7 +107,13 @@ export function readMcpLifecycleLockObservationSync(lockPath: string): LockObser try { const stat = fs.lstatSync(lockPath); if (!stat.isFile() || stat.isSymbolicLink()) { - return { owner: null, mtimeMs: stat.mtimeMs, dev: stat.dev, ino: stat.ino }; + return { + owner: null, + mtimeMs: stat.mtimeMs, + dev: stat.dev, + ino: stat.ino, + reclaimable: !stat.isDirectory(), + }; } } catch (statError) { if (isErrnoException(statError) && statError.code === "ENOENT") return null; @@ -100,7 +125,13 @@ export function readMcpLifecycleLockObservationSync(lockPath: string): LockObser try { const stat = fs.fstatSync(fd); if (!stat.isFile()) { - return { owner: null, mtimeMs: stat.mtimeMs, dev: stat.dev, ino: stat.ino }; + return { + owner: null, + mtimeMs: stat.mtimeMs, + dev: stat.dev, + ino: stat.ino, + reclaimable: !stat.isDirectory(), + }; } try { const parsed: unknown = JSON.parse(fs.readFileSync(fd, "utf8")); @@ -109,9 +140,16 @@ export function readMcpLifecycleLockObservationSync(lockPath: string): LockObser mtimeMs: stat.mtimeMs, dev: stat.dev, ino: stat.ino, + reclaimable: true, }; } catch { - return { owner: null, mtimeMs: stat.mtimeMs, dev: stat.dev, ino: stat.ino }; + return { + owner: null, + mtimeMs: stat.mtimeMs, + dev: stat.dev, + ino: stat.ino, + reclaimable: true, + }; } } finally { fs.closeSync(fd); @@ -155,9 +193,34 @@ export function safelyReleaseMcpLifecycleLockSync(lockPath: string, token: strin reclaimStaleMcpLifecycleLockGenerationSync(lockPath, observation); } +async function restoreClaimedMcpLifecycleLockGeneration( + targetPath: string, + quarantinePath: string, +): Promise { + try { + await fs.promises.link(quarantinePath, targetPath); + await fs.promises.rm(quarantinePath, { force: true }); + } catch (error) { + if (!isErrnoException(error) || error.code !== "EEXIST") throw error; + } +} + +function restoreClaimedMcpLifecycleLockGenerationSync( + targetPath: string, + quarantinePath: string, +): void { + try { + fs.linkSync(quarantinePath, targetPath); + fs.rmSync(quarantinePath, { force: true }); + } catch (error) { + if (!isErrnoException(error) || error.code !== "EEXIST") throw error; + } +} + export async function reclaimStaleMcpLifecycleLockGeneration( targetPath: string, expected: LockObservation, + assertAfterClaim?: () => void, ): Promise { const quarantinePath = `${targetPath}.reclaim-${process.pid}-${crypto.randomUUID()}`; try { @@ -180,6 +243,12 @@ export async function reclaimStaleMcpLifecycleLockGeneration( claimed.ino === expected.ino : claimed?.owner?.token === expectedToken; if (claimedExpectedGeneration) { + try { + assertAfterClaim?.(); + } catch (error) { + await restoreClaimedMcpLifecycleLockGeneration(targetPath, quarantinePath); + throw error; + } await fs.promises.rm(quarantinePath, { force: true, recursive: true }); return true; } @@ -189,18 +258,14 @@ export async function reclaimStaleMcpLifecycleLockGeneration( // quarantine name. If another generation already occupies the canonical // path, preserve the displaced owner record for diagnosis rather than ever // deleting an owner we did not claim. - try { - await fs.promises.link(quarantinePath, targetPath); - await fs.promises.rm(quarantinePath, { force: true }); - } catch (error) { - if (!isErrnoException(error) || error.code !== "EEXIST") throw error; - } + await restoreClaimedMcpLifecycleLockGeneration(targetPath, quarantinePath); return false; } export function reclaimStaleMcpLifecycleLockGenerationSync( targetPath: string, expected: LockObservation, + assertAfterClaim?: () => void, ): boolean { const quarantinePath = `${targetPath}.reclaim-${process.pid}-${crypto.randomUUID()}`; try { @@ -220,16 +285,17 @@ export function reclaimStaleMcpLifecycleLockGenerationSync( claimed.ino === expected.ino : claimed?.owner?.token === expectedToken; if (claimedExpectedGeneration) { + try { + assertAfterClaim?.(); + } catch (error) { + restoreClaimedMcpLifecycleLockGenerationSync(targetPath, quarantinePath); + throw error; + } fs.rmSync(quarantinePath, { force: true, recursive: true }); return true; } - try { - fs.linkSync(quarantinePath, targetPath); - fs.rmSync(quarantinePath, { force: true }); - } catch (error) { - if (!isErrnoException(error) || error.code !== "EEXIST") throw error; - } + restoreClaimedMcpLifecycleLockGenerationSync(targetPath, quarantinePath); return false; } diff --git a/test/e2e/support/messaging-compatible-endpoint-helpers.test.ts b/test/e2e/support/messaging-compatible-endpoint-helpers.test.ts index faa1072a067..e251dce637c 100644 --- a/test/e2e/support/messaging-compatible-endpoint-helpers.test.ts +++ b/test/e2e/support/messaging-compatible-endpoint-helpers.test.ts @@ -67,11 +67,17 @@ describe("messaging compatible endpoint helper coverage", () => { "signals only a start-time-matched owned gateway process (#6352)", async () => { const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-owned-gateway-pid-")); - const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { - argv0: "openshell-gateway[nemoclaw=nemoclaw;port=8080]", - stdio: "ignore", - }); + const child = spawn( + process.execPath, + ["-e", "process.stdout.write('ready\\n'); setInterval(() => {}, 1000)"], + { + argv0: "openshell-gateway[nemoclaw=nemoclaw;port=8080]", + stdio: ["ignore", "pipe", "ignore"], + }, + ); + const childReady = once(child.stdout, "data"); await once(child, "spawn"); + await childReady; const childExit = once(child, "exit"); const pid = child.pid; expect(pid).toBeTypeOf("number"); diff --git a/test/helpers/mcp-lifecycle-lock-deadline-clock.ts b/test/helpers/mcp-lifecycle-lock-deadline-clock.ts new file mode 100644 index 00000000000..d7b01e560b9 --- /dev/null +++ b/test/helpers/mcp-lifecycle-lock-deadline-clock.ts @@ -0,0 +1,68 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; + +type AsynchronousReplacementClock = { + readonly handoffScheduled: () => boolean; + readonly monotonicNow: () => number; +}; + +type SynchronousReplacementClock = { + readonly acquisitionPublished: () => boolean; + readonly monotonicNow: () => number; + readonly replacementPublished: () => boolean; +}; + +function replaceLockGeneration(lockPath: string, token: string): void { + const owner = JSON.parse(fs.readFileSync(lockPath, "utf8")) as Record; + fs.unlinkSync(lockPath); + fs.writeFileSync(lockPath, `${JSON.stringify({ ...owner, token })}\n`); +} + +export function createAsynchronousLockReplacementClock( + lockPath: string, + replacementToken: string, +): AsynchronousReplacementClock { + let now = 0; + let handoffScheduled = false; + + return { + handoffScheduled: () => handoffScheduled, + monotonicNow: () => { + if (!handoffScheduled && fs.existsSync(lockPath)) { + handoffScheduled = true; + queueMicrotask(() => { + replaceLockGeneration(lockPath, replacementToken); + now = 100; + }); + } + return now; + }, + }; +} + +export function createSynchronousLockReplacementClock( + lockPath: string, + replacementToken: string, +): SynchronousReplacementClock { + let acquisitionPublished = false; + let replacementPublished = false; + + return { + acquisitionPublished: () => acquisitionPublished, + monotonicNow: () => { + if (!fs.existsSync(lockPath)) return 0; + if (!acquisitionPublished) { + acquisitionPublished = true; + return 0; + } + if (!replacementPublished) { + replaceLockGeneration(lockPath, replacementToken); + replacementPublished = true; + } + return 100; + }, + replacementPublished: () => replacementPublished, + }; +} diff --git a/test/helpers/mcp-lifecycle-lock-properties.ts b/test/helpers/mcp-lifecycle-lock-properties.ts index 693932cc864..83961b2a041 100644 --- a/test/helpers/mcp-lifecycle-lock-properties.ts +++ b/test/helpers/mcp-lifecycle-lock-properties.ts @@ -42,7 +42,7 @@ function owner( } function observation(lockOwner: McpLifecycleLockOwner | null, mtimeMs = 0): LockObservation { - return { owner: lockOwner, mtimeMs, dev: 1, ino: 1 }; + return { owner: lockOwner, mtimeMs, dev: 1, ino: 1, reclaimable: true }; } function probes( diff --git a/test/mcp-lifecycle-lock.test.ts b/test/mcp-lifecycle-lock.test.ts index 2fddf6a91c7..0cf90e0d54e 100644 --- a/test/mcp-lifecycle-lock.test.ts +++ b/test/mcp-lifecycle-lock.test.ts @@ -11,6 +11,10 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as lifecycleLock from "../src/lib/state/mcp-lifecycle-lock"; +import { + createAsynchronousLockReplacementClock, + createSynchronousLockReplacementClock, +} from "./helpers/mcp-lifecycle-lock-deadline-clock"; import "./helpers/mcp-lifecycle-lock-properties"; const requireDist = createRequire(import.meta.url); @@ -27,7 +31,7 @@ const currentPidNamespaceIdentity = lifecycleLock.readMcpLockPidNamespaceIdentit let stateDir: string; const children = new Set(); -function options(overrides: Record = {}) { +function options(overrides: Partial = {}) { return { stateDir, pollIntervalMs: 5, @@ -145,9 +149,18 @@ describe("MCP lifecycle lock", () => { fs.mkdirSync(path.dirname(lockPath), { recursive: true }); fs.writeFileSync(targetPath, target); fs.symlinkSync(targetPath, lockPath); + const nowValues = [0, 0, 0, 0, 11]; + let nowCalls = 0; await expect( - lifecycleLock.withMcpLifecycleLock("alpha", () => "acquired", options({ timeoutMs: 50 })), + lifecycleLock.withMcpLifecycleLock( + "alpha", + () => "acquired", + options({ + timeoutMs: 50, + monotonicNow: () => nowValues[Math.min(nowCalls++, nowValues.length - 1)], + }), + ), ).rejects.toThrow(/containment is active/); expect(fs.readFileSync(targetPath, "utf8")).toBe(target); expect(fs.lstatSync(lockPath).isSymbolicLink()).toBe(true); @@ -170,13 +183,15 @@ describe("MCP lifecycle lock", () => { server.listen(lockPath, resolve); }); expect(fs.lstatSync(lockPath).isSocket()).toBe(true); + let monotonicNow = 0; try { await expect( lifecycleLock.withMcpLifecycleLock("alpha", () => "acquired", { ...options(), stateDir: shortStateDir, - timeoutMs: 50, + corruptLockGraceMs: 1, + monotonicNow: () => monotonicNow++, }), ).rejects.toThrow(/containment is active/); expect(fs.lstatSync(lockPath).isSocket()).toBe(true); @@ -616,6 +631,327 @@ const releasePath = process.argv[3]; expect(fs.existsSync(containmentPath)).toBe(true); }); + it("preserves a corrupt lock when observation crosses the acquisition deadline (#7858)", async () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync(lockPath, '{"version":1,"sandboxName":"alpha"'); + const nowValues = [0, 0, 0, 0, 10, 10, 200]; + let nowCalls = 0; + + await expect( + lifecycleLock.withMcpLifecycleLock( + "alpha", + () => undefined, + options({ + timeoutMs: 30, + corruptLockGraceMs: 100, + monotonicNow: () => nowValues[Math.min(nowCalls++, nowValues.length - 1)], + }), + ), + ).rejects.toThrow("Timed out waiting for the sandbox mutation lock"); + + expect(fs.readFileSync(lockPath, "utf8")).toContain('"sandboxName":"alpha"'); + expect(fs.existsSync(`${lockPath}.containment`)).toBe(false); + }); + + it("rolls back containment when publication crosses the acquisition deadline in the asynchronous path (#7858)", async () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + const deadlinePath = `${lockPath}.deadline`; + const containmentPath = `${lockPath}.containment`; + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync( + deadlinePath, + `${JSON.stringify({ + version: 1, + sandboxName: "alpha", + pid: 2_147_483_647, + processIdentity: "dead-deadline", + hostIdentity: currentHostIdentity, + pidNamespaceIdentity: currentPidNamespaceIdentity, + token: "stale-deadline-token", + acquiredAt: "2026-01-01T00:00:00.000Z", + })}\n`, + ); + const linkSync = fs.linkSync.bind(fs); + let now = 0; + const linkSpy = vi.spyOn(fs, "linkSync").mockImplementation((from, to) => { + linkSync(from, to); + now = String(to) === containmentPath ? 100 : now; + }); + const operation = vi.fn(); + + try { + await expect( + lifecycleLock.withMcpLifecycleLock("alpha", operation, { + ...options({ timeoutMs: 30 }), + monotonicNow: () => now, + }), + ).rejects.toThrow("Timed out waiting for the sandbox mutation lock"); + } finally { + linkSpy.mockRestore(); + } + + expect(operation).not.toHaveBeenCalled(); + expect(JSON.parse(fs.readFileSync(deadlinePath, "utf8")).token).toBe("stale-deadline-token"); + expect(fs.existsSync(containmentPath)).toBe(false); + }); + + it("rolls back containment when publication crosses the acquisition deadline in the synchronous path (#7858)", () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + const deadlinePath = `${lockPath}.deadline`; + const containmentPath = `${lockPath}.containment`; + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync( + deadlinePath, + `${JSON.stringify({ + version: 1, + sandboxName: "alpha", + pid: 2_147_483_647, + processIdentity: "dead-deadline", + hostIdentity: currentHostIdentity, + pidNamespaceIdentity: currentPidNamespaceIdentity, + token: "stale-deadline-token", + acquiredAt: "2026-01-01T00:00:00.000Z", + })}\n`, + ); + const linkSync = fs.linkSync.bind(fs); + let now = 0; + const linkSpy = vi.spyOn(fs, "linkSync").mockImplementation((from, to) => { + linkSync(from, to); + now = String(to) === containmentPath ? 100 : now; + }); + const operation = vi.fn(); + + try { + expect(() => + lifecycleLock.withMcpLifecycleLockSync("alpha", operation, { + ...options({ timeoutMs: 30 }), + monotonicNow: () => now, + }), + ).toThrow("Timed out waiting for sandbox mutation lock"); + } finally { + linkSpy.mockRestore(); + } + + expect(operation).not.toHaveBeenCalled(); + expect(JSON.parse(fs.readFileSync(deadlinePath, "utf8")).token).toBe("stale-deadline-token"); + expect(fs.existsSync(containmentPath)).toBe(false); + }); + + it("does not enter the critical section when lock publication crosses the acquisition deadline (#7858)", async () => { + let nowCalls = 0; + let entered = false; + + await expect( + lifecycleLock.withMcpLifecycleLock( + "alpha", + () => { + entered = true; + }, + options({ + timeoutMs: 30, + monotonicNow: () => (nowCalls++ < 3 ? 0 : 100), + }), + ), + ).rejects.toThrow("Timed out waiting for the sandbox mutation lock"); + + expect(entered).toBe(false); + expect(fs.existsSync(lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir))).toBe(false); + }); + + it("does not invoke the callback after the acquisition deadline in the asynchronous path (#7858)", async () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + const clock = createAsynchronousLockReplacementClock(lockPath, "async-replacement-token"); + const operation = vi.fn(); + + await expect( + lifecycleLock.withMcpLifecycleLock("alpha", operation, { + ...options({ timeoutMs: 30 }), + monotonicNow: clock.monotonicNow, + }), + ).rejects.toThrow("Timed out waiting for the sandbox mutation lock"); + + expect(clock.handoffScheduled()).toBe(true); + expect(operation).not.toHaveBeenCalled(); + expect(JSON.parse(fs.readFileSync(lockPath, "utf8")).token).toBe("async-replacement-token"); + }); + + it("does not invoke the callback after the acquisition deadline in the synchronous path (#7858)", () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + const clock = createSynchronousLockReplacementClock(lockPath, "sync-replacement-token"); + const operation = vi.fn(); + + expect(() => + lifecycleLock.withMcpLifecycleLockSync("alpha", operation, { + ...options({ timeoutMs: 30 }), + monotonicNow: clock.monotonicNow, + }), + ).toThrow("Timed out waiting for sandbox mutation lock"); + + expect(clock.acquisitionPublished()).toBe(true); + expect(clock.replacementPublished()).toBe(true); + expect(operation).not.toHaveBeenCalled(); + expect(JSON.parse(fs.readFileSync(lockPath, "utf8")).token).toBe("sync-replacement-token"); + }); + + it("does not enter the synchronous critical section when lock publication crosses the acquisition deadline (#7858)", () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + const linkSync = fs.linkSync.bind(fs); + let now = 0; + const linkSpy = vi.spyOn(fs, "linkSync").mockImplementation((from, to) => { + linkSync(from, to); + now = String(to) === lockPath ? 100 : now; + }); + const operation = vi.fn(); + + try { + expect(() => + lifecycleLock.withMcpLifecycleLockSync("alpha", operation, { + ...options({ timeoutMs: 30 }), + monotonicNow: () => now, + }), + ).toThrow("Timed out waiting for sandbox mutation lock"); + } finally { + linkSpy.mockRestore(); + } + + expect(operation).not.toHaveBeenCalled(); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it("restores a stale main lock when reclamation crosses the acquisition deadline (#7858)", async () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync( + lockPath, + `${JSON.stringify({ + version: 1, + sandboxName: "alpha", + pid: 2_147_483_647, + processIdentity: "dead-process", + hostIdentity: currentHostIdentity, + pidNamespaceIdentity: currentPidNamespaceIdentity, + token: "stale-main-token", + acquiredAt: "2026-01-01T00:00:00.000Z", + })}\n`, + ); + const rename = fs.promises.rename.bind(fs.promises); + let now = 0; + const renameSpy = vi.spyOn(fs.promises, "rename").mockImplementation(async (from, to) => { + await rename(from, to); + now = String(from) === lockPath ? 100 : now; + }); + let entered = false; + + try { + await expect( + lifecycleLock.withMcpLifecycleLock( + "alpha", + () => { + entered = true; + }, + options({ timeoutMs: 30, monotonicNow: () => now }), + ), + ).rejects.toThrow("Timed out waiting for the sandbox mutation lock"); + } finally { + renameSpy.mockRestore(); + } + + expect(entered).toBe(false); + expect(JSON.parse(fs.readFileSync(lockPath, "utf8")).token).toBe("stale-main-token"); + }); + + it("restores a stale main lock when synchronous reclamation crosses the acquisition deadline (#7858)", () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync( + lockPath, + `${JSON.stringify({ + version: 1, + sandboxName: "alpha", + pid: 2_147_483_647, + processIdentity: "dead-process", + hostIdentity: currentHostIdentity, + pidNamespaceIdentity: currentPidNamespaceIdentity, + token: "stale-main-token", + acquiredAt: "2026-01-01T00:00:00.000Z", + })}\n`, + ); + const renameSync = fs.renameSync.bind(fs); + let now = 0; + const renameSpy = vi.spyOn(fs, "renameSync").mockImplementation((from, to) => { + renameSync(from, to); + now = String(from) === lockPath ? 100 : now; + }); + const operation = vi.fn(); + + try { + expect(() => + lifecycleLock.withMcpLifecycleLockSync("alpha", operation, { + ...options({ timeoutMs: 30 }), + monotonicNow: () => now, + }), + ).toThrow("Timed out waiting for sandbox mutation lock"); + } finally { + renameSpy.mockRestore(); + } + + expect(operation).not.toHaveBeenCalled(); + expect(JSON.parse(fs.readFileSync(lockPath, "utf8")).token).toBe("stale-main-token"); + }); + + it("preserves a stale reaper when observation crosses the acquisition deadline (#7858)", async () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + const reaperPath = `${lockPath}.reaper`; + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync( + reaperPath, + `${JSON.stringify({ + version: 1, + sandboxName: "alpha", + pid: 2_147_483_647, + processIdentity: "dead-reaper", + hostIdentity: currentHostIdentity, + pidNamespaceIdentity: currentPidNamespaceIdentity, + token: "stale-reaper-token", + acquiredAt: "2026-01-01T00:00:00.000Z", + })}\n`, + ); + let nowCalls = 0; + + await expect( + lifecycleLock.withMcpLifecycleLock( + "alpha", + () => undefined, + options({ timeoutMs: 30, monotonicNow: () => (nowCalls++ < 2 ? 0 : 100) }), + ), + ).rejects.toThrow("Timed out waiting for the sandbox mutation lock"); + + expect(JSON.parse(fs.readFileSync(reaperPath, "utf8")).token).toBe("stale-reaper-token"); + expect(fs.existsSync(`${lockPath}.containment`)).toBe(false); + }); + + it("does not reclaim a corrupt directory at the lock path (#7858)", async () => { + const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); + fs.mkdirSync(lockPath, { recursive: true }); + const nowValues = [0, 0, 0, 0, 100]; + let nowCalls = 0; + + await expect( + lifecycleLock.withMcpLifecycleLock( + "alpha", + () => undefined, + options({ + timeoutMs: 30, + corruptLockGraceMs: 1, + monotonicNow: () => nowValues[Math.min(nowCalls++, nowValues.length - 1)], + }), + ), + ).rejects.toThrow("Timed out waiting for the sandbox mutation lock"); + + expect(fs.lstatSync(lockPath).isDirectory()).toBe(true); + }); + it("commits durable containment for a reaper whose owner died during stale-lock cleanup", async () => { const lockPath = lifecycleLock.getMcpLifecycleLockPath("alpha", stateDir); const reaperPath = `${lockPath}.reaper`;