From 4895183790bbdb4af203e7bb2981a123534abcef Mon Sep 17 00:00:00 2001 From: ram/haidar Date: Thu, 2 Apr 2026 23:24:23 +0700 Subject: [PATCH 1/4] fix(core): add directory lock fallback - Use a bundled directory-based lock when proper-lockfile cannot be resolved so cache locking still works in packaged/runtime environments. - Target: core --- lib/cache-lock.ts | 153 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 150 insertions(+), 3 deletions(-) diff --git a/lib/cache-lock.ts b/lib/cache-lock.ts index 79c949a..af10783 100644 --- a/lib/cache-lock.ts +++ b/lib/cache-lock.ts @@ -1,6 +1,151 @@ import fs from "node:fs/promises" import path from "node:path" -import lockfile from "proper-lockfile" + +type LockFunction = (file: string, options?: Record) => Promise<() => Promise> + +type RetryOptions = { + retries: number + minTimeout: number + maxTimeout: number +} + +let cachedLockFunction: LockFunction | undefined + +function resolveLockFunction(value: unknown, visited = new Set()): LockFunction | undefined { + if (typeof value === "function") { + return value as LockFunction + } + if (!value || typeof value !== "object") { + return undefined + } + if (visited.has(value)) { + return undefined + } + visited.add(value) + + const record = value as Record + const direct = resolveLockFunction(record.lock, visited) + if (direct) return direct + + const viaDefault = resolveLockFunction(record.default, visited) + if (viaDefault) return viaDefault + + const viaModule = resolveLockFunction(record.module, visited) + if (viaModule) return viaModule + + const viaExports = resolveLockFunction(record.exports, visited) + if (viaExports) return viaExports + + return undefined +} + +function isFsErrorCode(error: unknown, code: string): boolean { + return typeof error === "object" && error !== null && "code" in error && error.code === code +} + +function toRetryOptions(value: unknown): RetryOptions { + const defaultOptions: RetryOptions = { + retries: 0, + minTimeout: 50, + maxTimeout: 200 + } + if (!value || typeof value !== "object") { + return defaultOptions + } + + const record = value as Record + const retries = + typeof record.retries === "number" && Number.isFinite(record.retries) + ? Math.max(0, Math.floor(record.retries)) + : defaultOptions.retries + const minTimeout = + typeof record.minTimeout === "number" && Number.isFinite(record.minTimeout) + ? Math.max(1, Math.floor(record.minTimeout)) + : defaultOptions.minTimeout + const maxTimeout = + typeof record.maxTimeout === "number" && Number.isFinite(record.maxTimeout) + ? Math.max(minTimeout, Math.floor(record.maxTimeout)) + : defaultOptions.maxTimeout + + return { + retries, + minTimeout, + maxTimeout + } +} + +async function sleep(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)) +} + +async function lockWithDirectoryFallback( + targetPath: string, + options?: Record +): Promise<() => Promise> { + const lockDir = `${targetPath}.lock` + const retry = toRetryOptions(options?.retries) + const staleMs = typeof options?.stale === "number" && Number.isFinite(options.stale) ? Math.max(1, options.stale) : 0 + + for (let attempt = 0; ; attempt += 1) { + try { + await fs.mkdir(lockDir) + return async () => { + try { + await fs.rm(lockDir, { recursive: true, force: true }) + } catch (error) { + if (!isFsErrorCode(error, "ENOENT")) { + throw error + } + } + } + } catch (error) { + if (!isFsErrorCode(error, "EEXIST")) { + throw error + } + + if (staleMs > 0) { + try { + const stat = await fs.stat(lockDir) + if (Date.now() - stat.mtimeMs > staleMs) { + await fs.rm(lockDir, { recursive: true, force: true }) + continue + } + } catch (staleError) { + if (!isFsErrorCode(staleError, "ENOENT")) { + throw staleError + } + continue + } + } + + if (attempt >= retry.retries) { + throw error + } + + const timeout = Math.min(retry.maxTimeout, retry.minTimeout * 2 ** attempt) + await sleep(timeout) + } + } +} + +async function resolveImportedLockFunction(specifier: string): Promise { + try { + return resolveLockFunction(await import(specifier)) + } catch { + return undefined + } +} + +async function getLockFunction(): Promise { + if (cachedLockFunction) return cachedLockFunction + + cachedLockFunction = + (await resolveImportedLockFunction("proper-lockfile")) ?? + (await resolveImportedLockFunction("proper-lockfile/index.js")) ?? + lockWithDirectoryFallback + + return cachedLockFunction +} const LOCK_RETRIES = { retries: 20, @@ -41,7 +186,8 @@ export async function withLockedDirectory( options: LockTargetOptions = {} ): Promise { await fs.mkdir(directoryPath, { recursive: true }) - const release = await lockfile.lock(directoryPath, resolveLockOptions(options)) + const lock = await getLockFunction() + const release = await lock(directoryPath, resolveLockOptions(options)) try { return await fn() } finally { @@ -57,7 +203,8 @@ export async function withLockedFile( await fs.mkdir(path.dirname(filePath), { recursive: true }) const lockTargetPath = lockTargetPathForFile(filePath) await ensureLockTargetFile(lockTargetPath) - const release = await lockfile.lock(lockTargetPath, resolveLockOptions(options)) + const lock = await getLockFunction() + const release = await lock(lockTargetPath, resolveLockOptions(options)) try { return await fn() } finally { From a0e126fa6119cda9aaf1a8c6377db68a595ab2d7 Mon Sep 17 00:00:00 2001 From: Bryan Font Date: Mon, 6 Apr 2026 16:00:09 -0400 Subject: [PATCH 2/4] test(core): cover lockfile import compatibility --- scripts/test-mocking-allowlist.json | 5 ++ test/cache-lock.test.ts | 78 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 test/cache-lock.test.ts diff --git a/scripts/test-mocking-allowlist.json b/scripts/test-mocking-allowlist.json index 37adee5..624ddde 100644 --- a/scripts/test-mocking-allowlist.json +++ b/scripts/test-mocking-allowlist.json @@ -31,6 +31,11 @@ "mock": 0, "stubGlobal": 0 }, + "test/cache-lock.test.ts": { + "doMock": 3, + "mock": 0, + "stubGlobal": 0 + }, "test/codex-native-auth-menu-wiring.test.ts": { "doMock": 7, "mock": 0, diff --git a/test/cache-lock.test.ts b/test/cache-lock.test.ts new file mode 100644 index 0000000..7e5b9c7 --- /dev/null +++ b/test/cache-lock.test.ts @@ -0,0 +1,78 @@ +import fs from "node:fs/promises" +import os from "node:os" +import path from "node:path" + +import { afterEach, describe, expect, it, vi } from "vitest" + +const createdTempDirs = new Set() + +async function createTempDir(prefix: string): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)) + createdTempDirs.add(dir) + return dir +} + +afterEach(async () => { + vi.doUnmock("proper-lockfile") + vi.doUnmock("proper-lockfile/index.js") + vi.resetModules() + + const dirs = [...createdTempDirs] + createdTempDirs.clear() + await Promise.all(dirs.map((dir) => fs.rm(dir, { recursive: true, force: true }))) +}) + +describe("cache lock", () => { + it("uses a CommonJS-style lock export without requiring a default export", async () => { + const lock = vi.fn(async () => async () => {}) + vi.doMock("proper-lockfile", () => ({ lock })) + + const { lockTargetPathForFile, withLockedFile } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const filePath = path.join(root, "cache.json") + + let entered = false + const result = await withLockedFile(filePath, async () => { + entered = true + return "ok" + }) + + expect(result).toBe("ok") + expect(entered).toBe(true) + expect(lock).toHaveBeenCalledTimes(1) + expect(lock).toHaveBeenCalledWith( + lockTargetPathForFile(filePath), + expect.objectContaining({ + realpath: true, + retries: { + retries: 20, + minTimeout: 10, + maxTimeout: 100 + } + }) + ) + }) + + it("falls back to directory locks when proper-lockfile imports are unavailable", async () => { + vi.doMock("proper-lockfile", () => { + throw new Error("module unavailable") + }) + vi.doMock("proper-lockfile/index.js", () => { + throw new Error("subpath unavailable") + }) + + const { lockTargetPathForFile, withLockedFile } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const filePath = path.join(root, "snapshots.json") + const lockDir = `${lockTargetPathForFile(filePath)}.lock` + + let sawLockDir = false + await withLockedFile(filePath, async () => { + await expect(fs.access(lockDir)).resolves.toBeUndefined() + sawLockDir = true + }) + + expect(sawLockDir).toBe(true) + await expect(fs.access(lockDir)).rejects.toMatchObject({ code: "ENOENT" }) + }) +}) From 71b7b90144afa01c3e34279a8d95a3462b94d077 Mon Sep 17 00:00:00 2001 From: Bryan Font Date: Mon, 6 Apr 2026 16:28:17 -0400 Subject: [PATCH 3/4] fix(core): keep directory lock fallback alive under stale windows --- lib/cache-lock.ts | 92 ++++- scripts/test-mocking-allowlist.json | 2 +- test/cache-lock.test.ts | 517 +++++++++++++++++++++++++++- 3 files changed, 600 insertions(+), 11 deletions(-) diff --git a/lib/cache-lock.ts b/lib/cache-lock.ts index af10783..377bfe6 100644 --- a/lib/cache-lock.ts +++ b/lib/cache-lock.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "node:crypto" import fs from "node:fs/promises" import path from "node:path" @@ -10,6 +11,7 @@ type RetryOptions = { } let cachedLockFunction: LockFunction | undefined +const DIRECTORY_LOCK_OWNER_FILE = ".owner" function resolveLockFunction(value: unknown, visited = new Set()): LockFunction | undefined { if (typeof value === "function") { @@ -78,6 +80,26 @@ async function sleep(ms: number): Promise { await new Promise((resolve) => setTimeout(resolve, ms)) } +function ownerFilePathForLockDirectory(lockDir: string): string { + return path.join(lockDir, DIRECTORY_LOCK_OWNER_FILE) +} + +async function writeDirectoryLockOwner(lockDir: string, ownerToken: string): Promise { + await fs.writeFile(ownerFilePathForLockDirectory(lockDir), ownerToken, { mode: 0o600 }) +} + +async function hasDirectoryLockOwner(lockDir: string, ownerToken: string): Promise { + try { + const value = await fs.readFile(ownerFilePathForLockDirectory(lockDir), "utf8") + return value === ownerToken + } catch (error) { + if (isFsErrorCode(error, "ENOENT")) { + return false + } + throw error + } +} + async function lockWithDirectoryFallback( targetPath: string, options?: Record @@ -85,11 +107,62 @@ async function lockWithDirectoryFallback( const lockDir = `${targetPath}.lock` const retry = toRetryOptions(options?.retries) const staleMs = typeof options?.stale === "number" && Number.isFinite(options.stale) ? Math.max(1, options.stale) : 0 + const heartbeatMs = staleMs > 0 ? Math.max(1, Math.min(Math.floor(staleMs / 2), 1_000)) : 0 for (let attempt = 0; ; attempt += 1) { try { await fs.mkdir(lockDir) + const ownerToken = randomUUID() + try { + await writeDirectoryLockOwner(lockDir, ownerToken) + } catch (error) { + await fs.rm(lockDir, { recursive: true, force: true }) + throw error + } + + let released = false + let timer: ReturnType | undefined + let heartbeat = Promise.resolve() + let heartbeatError: unknown + + const scheduleHeartbeat = () => { + if (released || heartbeatMs <= 0) return + timer = setTimeout(() => { + heartbeat = heartbeat.finally(async () => { + if (released) return + try { + if (!(await hasDirectoryLockOwner(lockDir, ownerToken))) { + released = true + return + } + const now = new Date() + await fs.utimes(lockDir, now, now) + } catch (error) { + if (!isFsErrorCode(error, "ENOENT")) { + heartbeatError = error + released = true + return + } + released = true + return + } + scheduleHeartbeat() + }) + }, heartbeatMs) + } + + scheduleHeartbeat() + return async () => { + released = true + if (timer) clearTimeout(timer) + await heartbeat + if (heartbeatError) { + throw heartbeatError + } + if (!(await hasDirectoryLockOwner(lockDir, ownerToken))) { + return + } try { await fs.rm(lockDir, { recursive: true, force: true }) } catch (error) { @@ -139,10 +212,7 @@ async function resolveImportedLockFunction(specifier: string): Promise { if (cachedLockFunction) return cachedLockFunction - cachedLockFunction = - (await resolveImportedLockFunction("proper-lockfile")) ?? - (await resolveImportedLockFunction("proper-lockfile/index.js")) ?? - lockWithDirectoryFallback + cachedLockFunction = (await resolveImportedLockFunction("proper-lockfile")) ?? lockWithDirectoryFallback return cachedLockFunction } @@ -211,3 +281,17 @@ export async function withLockedFile( await release() } } + +// Test-only hooks for focused fallback coverage without widening the runtime behavior surface. +export const __cacheLockTest = { + DIRECTORY_LOCK_OWNER_FILE, + getLockFunction, + hasDirectoryLockOwner, + lockWithDirectoryFallback, + ownerFilePathForLockDirectory, + resolveImportedLockFunction, + resolveLockFunction, + resolveLockOptions, + toRetryOptions, + writeDirectoryLockOwner +} diff --git a/scripts/test-mocking-allowlist.json b/scripts/test-mocking-allowlist.json index 624ddde..0a2f6d6 100644 --- a/scripts/test-mocking-allowlist.json +++ b/scripts/test-mocking-allowlist.json @@ -32,7 +32,7 @@ "stubGlobal": 0 }, "test/cache-lock.test.ts": { - "doMock": 3, + "doMock": 4, "mock": 0, "stubGlobal": 0 }, diff --git a/test/cache-lock.test.ts b/test/cache-lock.test.ts index 7e5b9c7..19d2690 100644 --- a/test/cache-lock.test.ts +++ b/test/cache-lock.test.ts @@ -12,9 +12,25 @@ async function createTempDir(prefix: string): Promise { return dir } +function mockUnavailableProperLockfile() { + vi.doMock("proper-lockfile", () => { + throw new Error("module unavailable") + }) + vi.doMock("proper-lockfile/index.js", () => { + throw new Error("subpath unavailable") + }) +} + +async function sleep(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)) +} + afterEach(async () => { + vi.restoreAllMocks() vi.doUnmock("proper-lockfile") vi.doUnmock("proper-lockfile/index.js") + vi.doUnmock("node:fs/promises") + vi.useRealTimers() vi.resetModules() const dirs = [...createdTempDirs] @@ -23,7 +39,70 @@ afterEach(async () => { }) describe("cache lock", () => { + it("resolves lock functions from direct and nested module shapes", async () => { + const { __cacheLockTest } = await import("../lib/cache-lock") + const lock = vi.fn(async () => async () => {}) + + expect(__cacheLockTest.resolveLockFunction(lock)).toBe(lock) + expect(__cacheLockTest.resolveLockFunction({ default: { lock } })).toBe(lock) + expect(__cacheLockTest.resolveLockFunction({ module: { lock } })).toBe(lock) + expect(__cacheLockTest.resolveLockFunction({ exports: { lock } })).toBe(lock) + expect(__cacheLockTest.resolveLockFunction(null)).toBeUndefined() + + const cyclic: Record = {} + cyclic.default = cyclic + expect(__cacheLockTest.resolveLockFunction(cyclic)).toBeUndefined() + }) + + it("normalizes retry and stale option values defensively", async () => { + const { __cacheLockTest } = await import("../lib/cache-lock") + + expect(__cacheLockTest.toRetryOptions(undefined)).toEqual({ + retries: 0, + minTimeout: 50, + maxTimeout: 200 + }) + expect(__cacheLockTest.toRetryOptions({})).toEqual({ + retries: 0, + minTimeout: 50, + maxTimeout: 200 + }) + expect( + __cacheLockTest.toRetryOptions({ + retries: -3.4, + minTimeout: 0, + maxTimeout: 0 + }) + ).toEqual({ + retries: 0, + minTimeout: 1, + maxTimeout: 1 + }) + expect( + __cacheLockTest.toRetryOptions({ + retries: "bad", + minTimeout: "bad", + maxTimeout: "bad" + }) + ).toEqual({ + retries: 0, + minTimeout: 50, + maxTimeout: 200 + }) + expect(__cacheLockTest.resolveLockOptions({ staleMs: 3.9 })).toMatchObject({ stale: 3 }) + expect(__cacheLockTest.resolveLockOptions()).toMatchObject({ + realpath: true, + retries: { + retries: 20, + minTimeout: 10, + maxTimeout: 100 + } + }) + expect(__cacheLockTest.resolveLockOptions()).not.toHaveProperty("stale") + }) + it("uses a CommonJS-style lock export without requiring a default export", async () => { + vi.resetModules() const lock = vi.fn(async () => async () => {}) vi.doMock("proper-lockfile", () => ({ lock })) @@ -53,13 +132,48 @@ describe("cache lock", () => { ) }) - it("falls back to directory locks when proper-lockfile imports are unavailable", async () => { - vi.doMock("proper-lockfile", () => { - throw new Error("module unavailable") - }) - vi.doMock("proper-lockfile/index.js", () => { - throw new Error("subpath unavailable") + it("locks directories with a resolved lock function", async () => { + vi.resetModules() + const release = vi.fn(async () => {}) + const lock = vi.fn(async () => release) + vi.doMock("proper-lockfile", () => ({ lock })) + + const { withLockedDirectory } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-dir-") + const directoryPath = path.join(root, "nested") + + await expect(withLockedDirectory(directoryPath, async () => "dir-ok", { staleMs: 9 })).resolves.toBe("dir-ok") + expect(lock).toHaveBeenCalledWith( + directoryPath, + expect.objectContaining({ + stale: 9 + }) + ) + expect(release).toHaveBeenCalledTimes(1) + }) + + it("returns undefined when a lock module cannot be imported", async () => { + const { __cacheLockTest } = await import("../lib/cache-lock") + + expect(await __cacheLockTest.resolveImportedLockFunction("missing-lock-module")).toBeUndefined() + }) + + it("rethrows non-ENOENT owner reads", async () => { + const readFileSpy = vi + .spyOn(fs, "readFile") + .mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) + + const { __cacheLockTest } = await import("../lib/cache-lock") + + await expect(__cacheLockTest.hasDirectoryLockOwner("/tmp/missing-lock", "owner")).rejects.toMatchObject({ + code: "EPERM" }) + expect(readFileSpy).toHaveBeenCalledTimes(1) + }) + + it("falls back to directory locks when proper-lockfile imports are unavailable", async () => { + vi.resetModules() + mockUnavailableProperLockfile() const { lockTargetPathForFile, withLockedFile } = await import("../lib/cache-lock") const root = await createTempDir("opencode-cache-lock-") @@ -75,4 +189,395 @@ describe("cache lock", () => { expect(sawLockDir).toBe(true) await expect(fs.access(lockDir)).rejects.toMatchObject({ code: "ENOENT" }) }) + + it("swallows ENOENT when a fallback lock directory is already gone during release", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "release-target") + const lockDir = `${targetPath}.lock` + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + await fs.rm(lockDir, { recursive: true, force: true }) + await expect(release()).resolves.toBeUndefined() + }) + + it("rethrows non-ENOENT release errors from fallback locks", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const rmSpy = vi.spyOn(fs, "rm").mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "release-error-target") + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + await expect(release()).rejects.toMatchObject({ code: "EPERM" }) + expect(rmSpy).toHaveBeenCalledTimes(1) + }) + + it("removes a fallback lock directory when writing owner metadata fails", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const writeFileSpy = vi + .spyOn(fs, "writeFile") + .mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) + const rmSpy = vi.spyOn(fs, "rm") + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "owner-write-error-target") + const lockDir = `${targetPath}.lock` + + await expect( + __cacheLockTest.lockWithDirectoryFallback(targetPath, { + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + ).rejects.toMatchObject({ code: "EPERM" }) + expect(writeFileSpy).toHaveBeenCalledTimes(1) + expect(rmSpy).toHaveBeenCalledWith(lockDir, { recursive: true, force: true }) + await expect(fs.access(lockDir)).rejects.toMatchObject({ code: "ENOENT" }) + }) + + it("updates the lock heartbeat while holding a fallback lock", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + let heartbeatCallback: (() => void) | undefined + vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { + heartbeatCallback = callback as () => void + return 1 as unknown as ReturnType + }) as unknown as typeof setTimeout) + vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) + const utimesSpy = vi.spyOn(fs, "utimes").mockResolvedValueOnce(undefined) + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "heartbeat-target") + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 20, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + heartbeatCallback?.() + await vi.waitFor(() => { + expect(utimesSpy).toHaveBeenCalledTimes(1) + }) + await expect(release()).resolves.toBeUndefined() + }) + + it("stops heartbeat writes when utimes sees ENOENT", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const heartbeatCallbacks: Array<() => void> = [] + vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { + heartbeatCallbacks.push(callback as () => void) + return heartbeatCallbacks.length as unknown as ReturnType + }) as unknown as typeof setTimeout) + vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) + const utimesSpy = vi.spyOn(fs, "utimes").mockRejectedValueOnce(Object.assign(new Error("gone"), { code: "ENOENT" })) + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "heartbeat-enoent-target") + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 20, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + heartbeatCallbacks[0]?.() + await vi.waitFor(() => { + expect(utimesSpy).toHaveBeenCalledTimes(1) + }) + expect(heartbeatCallbacks).toHaveLength(1) + await expect(release()).resolves.toBeUndefined() + }) + + it("stops heartbeat writes after fallback lock ownership changes", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const heartbeatCallbacks: Array<() => void> = [] + vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { + heartbeatCallbacks.push(callback as () => void) + return heartbeatCallbacks.length as unknown as ReturnType + }) as unknown as typeof setTimeout) + vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) + const readFileSpy = vi.spyOn(fs, "readFile") + const utimesSpy = vi.spyOn(fs, "utimes") + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "heartbeat-owner-target") + const lockDir = `${targetPath}.lock` + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 20, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + await __cacheLockTest.writeDirectoryLockOwner(lockDir, "other-owner") + heartbeatCallbacks[0]?.() + await vi.waitFor(() => { + expect(readFileSpy).toHaveBeenCalled() + }) + expect(utimesSpy).not.toHaveBeenCalled() + await expect(release()).resolves.toBeUndefined() + await expect(fs.access(lockDir)).resolves.toBeUndefined() + }) + + it("surfaces non-ENOENT heartbeat failures", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + let heartbeatCallback: (() => void) | undefined + vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { + heartbeatCallback = callback as () => void + return 1 as unknown as ReturnType + }) as unknown as typeof setTimeout) + vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) + const utimesSpy = vi + .spyOn(fs, "utimes") + .mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "heartbeat-error-target") + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 20, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + heartbeatCallback?.() + await vi.waitFor(() => { + expect(utimesSpy).toHaveBeenCalledTimes(1) + }) + await expect(release()).rejects.toMatchObject({ code: "EPERM" }) + }) + + it("skips heartbeat work after a fallback lock has already been released", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + let heartbeatCallback: (() => void) | undefined + vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { + heartbeatCallback = callback as () => void + return 1 as unknown as ReturnType + }) as unknown as typeof setTimeout) + vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) + const utimesSpy = vi.spyOn(fs, "utimes") + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "heartbeat-released-target") + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 20, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + const releasePromise = release() + heartbeatCallback?.() + await expect(releasePromise).resolves.toBeUndefined() + expect(utimesSpy).not.toHaveBeenCalled() + }) + + it("skips heartbeat writes after the fallback lock has already been released", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + let heartbeatCallback: (() => void) | undefined + vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { + heartbeatCallback = callback as () => void + return 1 as unknown as ReturnType + }) as unknown as typeof setTimeout) + vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) + const utimesSpy = vi.spyOn(fs, "utimes") + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "heartbeat-skip-target") + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 20, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + await expect(release()).resolves.toBeUndefined() + heartbeatCallback?.() + await Promise.resolve() + expect(utimesSpy).not.toHaveBeenCalled() + }) + + it("reclaims stale fallback lock directories before retrying", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "stale-target") + const lockDir = `${targetPath}.lock` + + await fs.mkdir(lockDir) + const staleDate = new Date(Date.now() - 5_000) + await fs.utimes(lockDir, staleDate, staleDate) + + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 1, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + await expect(fs.access(lockDir)).resolves.toBeUndefined() + await release() + await expect(fs.access(lockDir)).rejects.toMatchObject({ code: "ENOENT" }) + }) + + it("retries fallback locking when the stale lock disappears during inspection", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const actualMkdir = fs.mkdir.bind(fs) + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "vanished-target") + const mkdirSpy = vi + .spyOn(fs, "mkdir") + .mockRejectedValueOnce(Object.assign(new Error("busy"), { code: "EEXIST" })) + .mockImplementationOnce(actualMkdir) + const statSpy = vi.spyOn(fs, "stat").mockRejectedValueOnce(Object.assign(new Error("gone"), { code: "ENOENT" })) + + const { __cacheLockTest } = await import("../lib/cache-lock") + + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 1, + retries: { retries: 1, minTimeout: 1, maxTimeout: 1 } + }) + + expect(mkdirSpy).toHaveBeenCalledTimes(2) + expect(statSpy).toHaveBeenCalledTimes(1) + await release() + }) + + it("does not remove a fallback lock after ownership has been replaced", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "replaced-owner-target") + const lockDir = `${targetPath}.lock` + const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 20, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + + await fs.rm(lockDir, { recursive: true, force: true }) + await fs.mkdir(lockDir) + await __cacheLockTest.writeDirectoryLockOwner(lockDir, "other-owner") + + await expect(release()).resolves.toBeUndefined() + await expect(fs.access(lockDir)).resolves.toBeUndefined() + }) + + it("rethrows non-ENOENT stale inspection errors", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "stat-error-target") + await fs.mkdir(`${targetPath}.lock`) + + const statSpy = vi.spyOn(fs, "stat").mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) + + const { __cacheLockTest } = await import("../lib/cache-lock") + + await expect( + __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 1, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + ).rejects.toMatchObject({ code: "EPERM" }) + expect(statSpy).toHaveBeenCalledTimes(1) + }) + + it("does not reclaim an active fallback lock after the stale window elapses", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const { withLockedFile } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const filePath = path.join(root, "active-target.json") + + let releaseFirst!: () => void + const firstDone = new Promise((resolve) => { + releaseFirst = resolve + }) + + let firstEntered = false + const firstLock = withLockedFile( + filePath, + async () => { + firstEntered = true + await firstDone + }, + { staleMs: 50 } + ) + + while (!firstEntered) { + await sleep(5) + } + + await sleep(120) + + let secondEntered = false + const secondLock = withLockedFile( + filePath, + async () => { + secondEntered = true + }, + { staleMs: 50 } + ) + + await sleep(80) + expect(secondEntered).toBe(false) + + releaseFirst() + await firstLock + await secondLock + expect(secondEntered).toBe(true) + }) + + it("throws fallback errors that are not lock contention", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const { __cacheLockTest } = await import("../lib/cache-lock") + await expect( + __cacheLockTest.lockWithDirectoryFallback(path.join("/dev/null", "child"), { + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + ).rejects.toMatchObject({ code: "ENOTDIR" }) + }) + + it("fails fallback locking once retries are exhausted", async () => { + vi.resetModules() + mockUnavailableProperLockfile() + + const { __cacheLockTest } = await import("../lib/cache-lock") + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "exhausted-target") + await fs.mkdir(`${targetPath}.lock`) + + await expect( + __cacheLockTest.lockWithDirectoryFallback(targetPath, { + stale: 100_000, + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + ).rejects.toMatchObject({ code: "EEXIST" }) + }) }) From b345a987247e95636d6c3dfa1d69c988220a9342 Mon Sep 17 00:00:00 2001 From: Bryan Font Date: Mon, 6 Apr 2026 16:45:25 -0400 Subject: [PATCH 4/4] fix(core): simplify directory lock fallback safety --- lib/cache-lock.ts | 57 ------ test/cache-lock.test.ts | 387 +++++++++------------------------------- 2 files changed, 83 insertions(+), 361 deletions(-) diff --git a/lib/cache-lock.ts b/lib/cache-lock.ts index 377bfe6..fab4a2d 100644 --- a/lib/cache-lock.ts +++ b/lib/cache-lock.ts @@ -106,8 +106,6 @@ async function lockWithDirectoryFallback( ): Promise<() => Promise> { const lockDir = `${targetPath}.lock` const retry = toRetryOptions(options?.retries) - const staleMs = typeof options?.stale === "number" && Number.isFinite(options.stale) ? Math.max(1, options.stale) : 0 - const heartbeatMs = staleMs > 0 ? Math.max(1, Math.min(Math.floor(staleMs / 2), 1_000)) : 0 for (let attempt = 0; ; attempt += 1) { try { @@ -119,47 +117,7 @@ async function lockWithDirectoryFallback( await fs.rm(lockDir, { recursive: true, force: true }) throw error } - - let released = false - let timer: ReturnType | undefined - let heartbeat = Promise.resolve() - let heartbeatError: unknown - - const scheduleHeartbeat = () => { - if (released || heartbeatMs <= 0) return - timer = setTimeout(() => { - heartbeat = heartbeat.finally(async () => { - if (released) return - try { - if (!(await hasDirectoryLockOwner(lockDir, ownerToken))) { - released = true - return - } - const now = new Date() - await fs.utimes(lockDir, now, now) - } catch (error) { - if (!isFsErrorCode(error, "ENOENT")) { - heartbeatError = error - released = true - return - } - released = true - return - } - scheduleHeartbeat() - }) - }, heartbeatMs) - } - - scheduleHeartbeat() - return async () => { - released = true - if (timer) clearTimeout(timer) - await heartbeat - if (heartbeatError) { - throw heartbeatError - } if (!(await hasDirectoryLockOwner(lockDir, ownerToken))) { return } @@ -176,21 +134,6 @@ async function lockWithDirectoryFallback( throw error } - if (staleMs > 0) { - try { - const stat = await fs.stat(lockDir) - if (Date.now() - stat.mtimeMs > staleMs) { - await fs.rm(lockDir, { recursive: true, force: true }) - continue - } - } catch (staleError) { - if (!isFsErrorCode(staleError, "ENOENT")) { - throw staleError - } - continue - } - } - if (attempt >= retry.retries) { throw error } diff --git a/test/cache-lock.test.ts b/test/cache-lock.test.ts index 19d2690..e8f9c79 100644 --- a/test/cache-lock.test.ts +++ b/test/cache-lock.test.ts @@ -21,6 +21,10 @@ function mockUnavailableProperLockfile() { }) } +function mockProperLockfileNamespace(lock: (...args: Array) => Promise<() => Promise>) { + vi.doMock("proper-lockfile", () => ({ lock })) +} + async function sleep(ms: number): Promise { await new Promise((resolve) => setTimeout(resolve, ms)) } @@ -104,21 +108,13 @@ describe("cache lock", () => { it("uses a CommonJS-style lock export without requiring a default export", async () => { vi.resetModules() const lock = vi.fn(async () => async () => {}) - vi.doMock("proper-lockfile", () => ({ lock })) + mockProperLockfileNamespace(lock) const { lockTargetPathForFile, withLockedFile } = await import("../lib/cache-lock") const root = await createTempDir("opencode-cache-lock-") const filePath = path.join(root, "cache.json") - let entered = false - const result = await withLockedFile(filePath, async () => { - entered = true - return "ok" - }) - - expect(result).toBe("ok") - expect(entered).toBe(true) - expect(lock).toHaveBeenCalledTimes(1) + await expect(withLockedFile(filePath, async () => "ok")).resolves.toBe("ok") expect(lock).toHaveBeenCalledWith( lockTargetPathForFile(filePath), expect.objectContaining({ @@ -132,23 +128,24 @@ describe("cache lock", () => { ) }) + it("loads the plugin entry when proper-lockfile resolves to a CommonJS-style namespace", async () => { + vi.resetModules() + mockProperLockfileNamespace(vi.fn(async () => async () => {})) + + const pluginEntry = await import("../index") + expect(typeof pluginEntry.OpenAIMultiAuthPlugin).toBe("function") + }) + it("locks directories with a resolved lock function", async () => { vi.resetModules() const release = vi.fn(async () => {}) - const lock = vi.fn(async () => release) - vi.doMock("proper-lockfile", () => ({ lock })) + mockProperLockfileNamespace(vi.fn(async () => release)) const { withLockedDirectory } = await import("../lib/cache-lock") const root = await createTempDir("opencode-cache-lock-dir-") const directoryPath = path.join(root, "nested") await expect(withLockedDirectory(directoryPath, async () => "dir-ok", { staleMs: 9 })).resolves.toBe("dir-ok") - expect(lock).toHaveBeenCalledWith( - directoryPath, - expect.objectContaining({ - stale: 9 - }) - ) expect(release).toHaveBeenCalledTimes(1) }) @@ -165,7 +162,7 @@ describe("cache lock", () => { const { __cacheLockTest } = await import("../lib/cache-lock") - await expect(__cacheLockTest.hasDirectoryLockOwner("/tmp/missing-lock", "owner")).rejects.toMatchObject({ + await expect(__cacheLockTest.hasDirectoryLockOwner("/tmp/missing-lock", "owner-token")).rejects.toMatchObject({ code: "EPERM" }) expect(readFileSpy).toHaveBeenCalledTimes(1) @@ -190,289 +187,140 @@ describe("cache lock", () => { await expect(fs.access(lockDir)).rejects.toMatchObject({ code: "ENOENT" }) }) - it("swallows ENOENT when a fallback lock directory is already gone during release", async () => { + it("keeps exclusivity instead of reaping a long-held fallback lock", async () => { vi.resetModules() mockUnavailableProperLockfile() - const { __cacheLockTest } = await import("../lib/cache-lock") + const { withLockedFile } = await import("../lib/cache-lock") const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "release-target") - const lockDir = `${targetPath}.lock` - const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } - }) - - await fs.rm(lockDir, { recursive: true, force: true }) - await expect(release()).resolves.toBeUndefined() - }) - - it("rethrows non-ENOENT release errors from fallback locks", async () => { - vi.resetModules() - mockUnavailableProperLockfile() - - const rmSpy = vi.spyOn(fs, "rm").mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) + const filePath = path.join(root, "active-target.json") - const { __cacheLockTest } = await import("../lib/cache-lock") - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "release-error-target") - const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + let releaseFirst!: () => void + const firstDone = new Promise((resolve) => { + releaseFirst = resolve }) - await expect(release()).rejects.toMatchObject({ code: "EPERM" }) - expect(rmSpy).toHaveBeenCalledTimes(1) - }) - - it("removes a fallback lock directory when writing owner metadata fails", async () => { - vi.resetModules() - mockUnavailableProperLockfile() - - const writeFileSpy = vi - .spyOn(fs, "writeFile") - .mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) - const rmSpy = vi.spyOn(fs, "rm") - - const { __cacheLockTest } = await import("../lib/cache-lock") - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "owner-write-error-target") - const lockDir = `${targetPath}.lock` + let firstEntered = false + const firstLock = withLockedFile( + filePath, + async () => { + firstEntered = true + await firstDone + }, + { staleMs: 20 } + ) - await expect( - __cacheLockTest.lockWithDirectoryFallback(targetPath, { - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } - }) - ).rejects.toMatchObject({ code: "EPERM" }) - expect(writeFileSpy).toHaveBeenCalledTimes(1) - expect(rmSpy).toHaveBeenCalledWith(lockDir, { recursive: true, force: true }) - await expect(fs.access(lockDir)).rejects.toMatchObject({ code: "ENOENT" }) - }) + while (!firstEntered) { + await sleep(5) + } - it("updates the lock heartbeat while holding a fallback lock", async () => { - vi.resetModules() - mockUnavailableProperLockfile() + await sleep(60) - let heartbeatCallback: (() => void) | undefined - vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { - heartbeatCallback = callback as () => void - return 1 as unknown as ReturnType - }) as unknown as typeof setTimeout) - vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) - const utimesSpy = vi.spyOn(fs, "utimes").mockResolvedValueOnce(undefined) + let secondEntered = false + const secondLock = withLockedFile( + filePath, + async () => { + secondEntered = true + }, + { staleMs: 20 } + ) - const { __cacheLockTest } = await import("../lib/cache-lock") - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "heartbeat-target") - const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 20, - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } - }) + await sleep(60) + expect(secondEntered).toBe(false) - heartbeatCallback?.() - await vi.waitFor(() => { - expect(utimesSpy).toHaveBeenCalledTimes(1) - }) - await expect(release()).resolves.toBeUndefined() + releaseFirst() + await firstLock + await secondLock + expect(secondEntered).toBe(true) }) - it("stops heartbeat writes when utimes sees ENOENT", async () => { + it("retries fallback locking until the directory becomes available", async () => { vi.resetModules() mockUnavailableProperLockfile() - const heartbeatCallbacks: Array<() => void> = [] - vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { - heartbeatCallbacks.push(callback as () => void) - return heartbeatCallbacks.length as unknown as ReturnType - }) as unknown as typeof setTimeout) - vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) - const utimesSpy = vi.spyOn(fs, "utimes").mockRejectedValueOnce(Object.assign(new Error("gone"), { code: "ENOENT" })) + const actualMkdir = fs.mkdir.bind(fs) + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "retry-target") + const mkdirSpy = vi + .spyOn(fs, "mkdir") + .mockRejectedValueOnce(Object.assign(new Error("busy"), { code: "EEXIST" })) + .mockImplementationOnce(actualMkdir) const { __cacheLockTest } = await import("../lib/cache-lock") - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "heartbeat-enoent-target") const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 20, - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + retries: { retries: 1, minTimeout: 1, maxTimeout: 1 } }) - heartbeatCallbacks[0]?.() - await vi.waitFor(() => { - expect(utimesSpy).toHaveBeenCalledTimes(1) - }) - expect(heartbeatCallbacks).toHaveLength(1) + expect(mkdirSpy).toHaveBeenCalledTimes(2) await expect(release()).resolves.toBeUndefined() }) - it("stops heartbeat writes after fallback lock ownership changes", async () => { + it("swallows ENOENT when a fallback lock directory is already gone during release", async () => { vi.resetModules() mockUnavailableProperLockfile() - const heartbeatCallbacks: Array<() => void> = [] - vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { - heartbeatCallbacks.push(callback as () => void) - return heartbeatCallbacks.length as unknown as ReturnType - }) as unknown as typeof setTimeout) - vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) - const readFileSpy = vi.spyOn(fs, "readFile") - const utimesSpy = vi.spyOn(fs, "utimes") - const { __cacheLockTest } = await import("../lib/cache-lock") const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "heartbeat-owner-target") + const targetPath = path.join(root, "release-target") const lockDir = `${targetPath}.lock` const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 20, retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } }) - await __cacheLockTest.writeDirectoryLockOwner(lockDir, "other-owner") - heartbeatCallbacks[0]?.() - await vi.waitFor(() => { - expect(readFileSpy).toHaveBeenCalled() - }) - expect(utimesSpy).not.toHaveBeenCalled() + await fs.rm(lockDir, { recursive: true, force: true }) await expect(release()).resolves.toBeUndefined() - await expect(fs.access(lockDir)).resolves.toBeUndefined() }) - it("surfaces non-ENOENT heartbeat failures", async () => { + it("removes a fallback lock directory when writing owner metadata fails", async () => { vi.resetModules() mockUnavailableProperLockfile() - let heartbeatCallback: (() => void) | undefined - vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { - heartbeatCallback = callback as () => void - return 1 as unknown as ReturnType - }) as unknown as typeof setTimeout) - vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) - const utimesSpy = vi - .spyOn(fs, "utimes") + const writeFileSpy = vi + .spyOn(fs, "writeFile") .mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) + const rmSpy = vi.spyOn(fs, "rm") const { __cacheLockTest } = await import("../lib/cache-lock") const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "heartbeat-error-target") - const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 20, - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } - }) - - heartbeatCallback?.() - await vi.waitFor(() => { - expect(utimesSpy).toHaveBeenCalledTimes(1) - }) - await expect(release()).rejects.toMatchObject({ code: "EPERM" }) - }) - - it("skips heartbeat work after a fallback lock has already been released", async () => { - vi.resetModules() - mockUnavailableProperLockfile() - - let heartbeatCallback: (() => void) | undefined - vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { - heartbeatCallback = callback as () => void - return 1 as unknown as ReturnType - }) as unknown as typeof setTimeout) - vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) - const utimesSpy = vi.spyOn(fs, "utimes") - - const { __cacheLockTest } = await import("../lib/cache-lock") - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "heartbeat-released-target") - const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 20, - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } - }) - - const releasePromise = release() - heartbeatCallback?.() - await expect(releasePromise).resolves.toBeUndefined() - expect(utimesSpy).not.toHaveBeenCalled() - }) - - it("skips heartbeat writes after the fallback lock has already been released", async () => { - vi.resetModules() - mockUnavailableProperLockfile() - - let heartbeatCallback: (() => void) | undefined - vi.spyOn(globalThis, "setTimeout").mockImplementation(((callback: TimerHandler) => { - heartbeatCallback = callback as () => void - return 1 as unknown as ReturnType - }) as unknown as typeof setTimeout) - vi.spyOn(globalThis, "clearTimeout").mockImplementation(() => {}) - const utimesSpy = vi.spyOn(fs, "utimes") - - const { __cacheLockTest } = await import("../lib/cache-lock") - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "heartbeat-skip-target") - const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 20, - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } - }) - - await expect(release()).resolves.toBeUndefined() - heartbeatCallback?.() - await Promise.resolve() - expect(utimesSpy).not.toHaveBeenCalled() - }) - - it("reclaims stale fallback lock directories before retrying", async () => { - vi.resetModules() - mockUnavailableProperLockfile() - - const { __cacheLockTest } = await import("../lib/cache-lock") - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "stale-target") + const targetPath = path.join(root, "owner-write-error-target") const lockDir = `${targetPath}.lock` - await fs.mkdir(lockDir) - const staleDate = new Date(Date.now() - 5_000) - await fs.utimes(lockDir, staleDate, staleDate) - - const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 1, - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } - }) - - await expect(fs.access(lockDir)).resolves.toBeUndefined() - await release() + await expect( + __cacheLockTest.lockWithDirectoryFallback(targetPath, { + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } + }) + ).rejects.toMatchObject({ code: "EPERM" }) + expect(writeFileSpy).toHaveBeenCalledTimes(1) + expect(rmSpy).toHaveBeenCalledWith(lockDir, { recursive: true, force: true }) await expect(fs.access(lockDir)).rejects.toMatchObject({ code: "ENOENT" }) }) - it("retries fallback locking when the stale lock disappears during inspection", async () => { + it("rethrows non-ENOENT release errors from fallback locks", async () => { vi.resetModules() mockUnavailableProperLockfile() - const actualMkdir = fs.mkdir.bind(fs) - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "vanished-target") - const mkdirSpy = vi - .spyOn(fs, "mkdir") - .mockRejectedValueOnce(Object.assign(new Error("busy"), { code: "EEXIST" })) - .mockImplementationOnce(actualMkdir) - const statSpy = vi.spyOn(fs, "stat").mockRejectedValueOnce(Object.assign(new Error("gone"), { code: "ENOENT" })) + const rmSpy = vi.spyOn(fs, "rm").mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) const { __cacheLockTest } = await import("../lib/cache-lock") - + const root = await createTempDir("opencode-cache-lock-") + const targetPath = path.join(root, "release-error-target") const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 1, - retries: { retries: 1, minTimeout: 1, maxTimeout: 1 } + retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } }) - expect(mkdirSpy).toHaveBeenCalledTimes(2) - expect(statSpy).toHaveBeenCalledTimes(1) - await release() + await expect(release()).rejects.toMatchObject({ code: "EPERM" }) + expect(rmSpy).toHaveBeenCalledTimes(1) }) - it("does not remove a fallback lock after ownership has been replaced", async () => { + it("does not remove a replacement fallback lock owned by someone else", async () => { vi.resetModules() mockUnavailableProperLockfile() const { __cacheLockTest } = await import("../lib/cache-lock") const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "replaced-owner-target") + const targetPath = path.join(root, "replacement-owner-target") const lockDir = `${targetPath}.lock` const release = await __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 20, retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } }) @@ -484,74 +332,6 @@ describe("cache lock", () => { await expect(fs.access(lockDir)).resolves.toBeUndefined() }) - it("rethrows non-ENOENT stale inspection errors", async () => { - vi.resetModules() - mockUnavailableProperLockfile() - - const root = await createTempDir("opencode-cache-lock-") - const targetPath = path.join(root, "stat-error-target") - await fs.mkdir(`${targetPath}.lock`) - - const statSpy = vi.spyOn(fs, "stat").mockRejectedValueOnce(Object.assign(new Error("denied"), { code: "EPERM" })) - - const { __cacheLockTest } = await import("../lib/cache-lock") - - await expect( - __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 1, - retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } - }) - ).rejects.toMatchObject({ code: "EPERM" }) - expect(statSpy).toHaveBeenCalledTimes(1) - }) - - it("does not reclaim an active fallback lock after the stale window elapses", async () => { - vi.resetModules() - mockUnavailableProperLockfile() - - const { withLockedFile } = await import("../lib/cache-lock") - const root = await createTempDir("opencode-cache-lock-") - const filePath = path.join(root, "active-target.json") - - let releaseFirst!: () => void - const firstDone = new Promise((resolve) => { - releaseFirst = resolve - }) - - let firstEntered = false - const firstLock = withLockedFile( - filePath, - async () => { - firstEntered = true - await firstDone - }, - { staleMs: 50 } - ) - - while (!firstEntered) { - await sleep(5) - } - - await sleep(120) - - let secondEntered = false - const secondLock = withLockedFile( - filePath, - async () => { - secondEntered = true - }, - { staleMs: 50 } - ) - - await sleep(80) - expect(secondEntered).toBe(false) - - releaseFirst() - await firstLock - await secondLock - expect(secondEntered).toBe(true) - }) - it("throws fallback errors that are not lock contention", async () => { vi.resetModules() mockUnavailableProperLockfile() @@ -575,7 +355,6 @@ describe("cache lock", () => { await expect( __cacheLockTest.lockWithDirectoryFallback(targetPath, { - stale: 100_000, retries: { retries: 0, minTimeout: 1, maxTimeout: 1 } }) ).rejects.toMatchObject({ code: "EEXIST" })