From f0e709e11f03cbf596c06f7845d6fb442706732c Mon Sep 17 00:00:00 2001 From: James Sesler Date: Fri, 25 Sep 2026 03:26:04 -0400 Subject: [PATCH 1/5] fix: make stale lock ticket reaping single-winner on Windows Co-Authored-By: Paperclip --- core/status.ts | 61 +++++++++++++++++++++++++----------- tests/state-store.test.mjs | 64 +++++++++++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/core/status.ts b/core/status.ts index 0eefa29..886511c 100644 --- a/core/status.ts +++ b/core/status.ts @@ -2,7 +2,7 @@ // framework logic shared by every wrapper. import { randomBytes } from "node:crypto"; -import { open, readdir, readFile, rm, stat, unlink, utimes } from "node:fs/promises"; +import { open, readdir, readFile, rename, rm, stat, unlink, utimes } from "node:fs/promises"; import { basename, dirname, join } from "node:path"; import type { CarryForwardEntry, @@ -457,6 +457,8 @@ export interface AcquireLockOptions { * hung; default {@link STALE_LOCK_MS}. A dead holder is removed at once. */ staleMs?: number; + /** @internal File operations seam for deterministic lock race tests. */ + fsOps?: { stat: typeof stat; rename: typeof rename; unlink: typeof unlink }; } /** @@ -490,6 +492,7 @@ export interface AcquireLockOptions { export async function acquireLock(lockPath: string, options: AcquireLockOptions = {}): Promise { const timeoutMs = options.timeoutMs ?? LOCK_TIMEOUT_MS; const staleMs = options.staleMs ?? STALE_LOCK_MS; + const fsOps = options.fsOps ?? { stat, rename, unlink }; const startedAt = Date.now(); const dir = dirname(lockPath); const base = basename(lockPath); @@ -536,11 +539,14 @@ export async function acquireLock(lockPath: string, options: AcquireLockOptions // A pre-#355 lock file: honour it while fresh, remove it when stale. if (names.includes(base)) { - const legacyStat = await stat(lockPath).catch(() => null); + const legacyStat = await fsOps.stat(lockPath).catch((error: NodeJS.ErrnoException) => { + if (error.code !== "ENOENT") blocked = true; + return null; + }); if (legacyStat && Date.now() - legacyStat.mtimeMs <= staleMs) blocked = true; else if (legacyStat) { const holder = await describeLockHolder(lockPath); - if (await removeIfPresent(lockPath)) brokeStale = holder; + if (await removeIfPresent(lockPath, lockPath, fsOps)) brokeStale = holder; } } @@ -549,7 +555,7 @@ export async function acquireLock(lockPath: string, options: AcquireLockOptions for (const name of names) { if (!name.startsWith(`${base}.c.`) || name === basename(choosingPath)) continue; const path = join(dir, name); - if (await ownerIsGone(path, staleMs)) { + if (await ownerIsGone(path, staleMs, fsOps)) { await rm(path, { force: true }).catch(() => undefined); continue; } @@ -562,11 +568,11 @@ export async function acquireLock(lockPath: string, options: AcquireLockOptions const ahead = names.filter((name) => name.startsWith(`${base}.t.`) && name < ticketName).sort(); for (const name of ahead) { const path = join(dir, name); - if (await ownerIsGone(path, staleMs)) { - // Recorded only by the waiter whose rm actually removed it; the - // others saw the same dead ticket and removed nothing. + if (await ownerIsGone(path, staleMs, fsOps)) { + // Only the waiter that claims the dead ticket records its holder; + // other waiters cannot claim the same path. const holder = await describeLockHolder(path); - if (await removeIfPresent(path)) brokeStale = holder; + if (await removeIfPresent(path, lockPath, fsOps)) brokeStale = holder; continue; } blocked = true; @@ -593,19 +599,36 @@ export async function acquireLock(lockPath: string, options: AcquireLockOptions } /** - * Remove `path`; true when this call removed it, false when it was already - * gone. `unlink`, not `rm`: `fs.promises.rm` reports success to every one - * of several concurrent callers, and the point here is to know which one - * actually took the file away. + * Atomically move a stale claim out of the queue. Only the waiter whose + * rename succeeds reports the break. The tombstone is outside the ticket + * namespace, so even a Windows unlink failure cannot revive the claim. */ -async function removeIfPresent(path: string): Promise { +async function removeIfPresent( + path: string, + lockPath: string, + fsOps: NonNullable, +): Promise { + const tombstone = `${lockPath}.reaped.${randomBytes(8).toString("hex")}`; try { - await unlink(path); - return true; + await fsOps.rename(path, tombstone); } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; + // Windows can report a losing rename as EPERM or EBUSY. Confirm + // absence; a real permission/sharing error must still surface. + if (["EPERM", "EBUSY"].includes((error as NodeJS.ErrnoException).code ?? "")) { + try { + await fsOps.stat(path); + } catch (statError) { + if ((statError as NodeJS.ErrnoException).code === "ENOENT") return false; + throw statError; + } + } throw error; } + // Cleanup is best effort: the original claim is already gone. A leftover + // tombstone cannot be mistaken for a ticket or legacy lock. + await fsOps.unlink(tombstone).catch(() => undefined); + return true; } /** Create `path` exclusively with `content`; the descriptor is closed either way (#131). */ @@ -624,12 +647,12 @@ async function writeExclusive(path: string, content: string): Promise { * cannot be signalled (another user's process) counts as alive. A file that * vanished while we looked is gone, and so is its owner's claim. */ -async function ownerIsGone(path: string, staleMs: number): Promise { +async function ownerIsGone(path: string, staleMs: number, fsOps: NonNullable): Promise { let fileStat; try { - fileStat = await stat(path); - } catch { - return true; + fileStat = await fsOps.stat(path); + } catch (error) { + return (error as NodeJS.ErrnoException).code === "ENOENT"; } if (Date.now() - fileStat.mtimeMs > staleMs) return true; const { pid } = await describeLockHolder(path); diff --git a/tests/state-store.test.mjs b/tests/state-store.test.mjs index 4c03f97..869bddc 100644 --- a/tests/state-store.test.mjs +++ b/tests/state-store.test.mjs @@ -6,7 +6,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { existsSync } from "node:fs"; -import { mkdir, mkdtemp, readFile, readdir, rm, utimes, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, readdir, rename, rm, stat, unlink, utimes, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; @@ -253,6 +253,68 @@ test("many waiters, one dead ticket ahead of them: one holder at a time, every w } }); +test("Windows unlink errors after a stale-ticket claim cannot give two waiters the break", async () => { + for (const code of ["EPERM", "EBUSY"]) { + const { dir, cleanup } = await tempDir("cc-lock-windows-"); + try { + const lockPath = join(dir, "status.yaml.lock"); + await deadTicket(dir); + let claimed = 0; + let errors = 0; + const fsOps = { + stat, + rename: async (from, to) => { + await rename(from, to); + if (from.includes(".t.")) claimed += 1; + }, + unlink: async (path) => { + if (path.includes(".reaped.")) { + const error = new Error("Windows unlink race"); + error.code = code; + errors += 1; + throw error; + } + await unlink(path); + }, + }; + const handles = await Promise.all(Array.from({ length: 2 }, async () => { + const handle = await acquireLock(lockPath, { fsOps }); + await handle.release(); + return handle; + })); + assert.equal(claimed, 1, "one rename claimed the dead ticket"); + assert.equal(handles.filter((handle) => handle.brokeStale).length, 1); + assert.equal(errors, 1, `the winning claim encountered ${code} on cleanup`); + } finally { + await cleanup(); + } + } +}); + +test("a non-ENOENT ticket stat error is not evidence that its owner is gone", async () => { + const { dir, cleanup } = await tempDir("cc-lock-stat-"); + try { + const lockPath = join(dir, "status.yaml.lock"); + const dead = await deadTicket(dir); + const fsOps = { + stat: async (path) => { + if (path === dead) { + const error = new Error("Windows stat denied"); + error.code = "EPERM"; + throw error; + } + return stat(path); + }, + rename, + unlink, + }; + await assert.rejects(acquireLock(lockPath, { fsOps, timeoutMs: 150 }), /Timed out waiting for lock/); + assert.equal(existsSync(dead), true, "an unreadable ticket is not reaped"); + } finally { + await cleanup(); + } +}); + test("a waiter that died in the doorway does not block the queue; a live one does until it has its ticket", async () => { const { dir, cleanup } = await tempDir("cc-lock-door-"); try { From 62e4e0e91ea77eaccd95cb735ab2852883391d13 Mon Sep 17 00:00:00 2001 From: James Sesler Date: Fri, 25 Sep 2026 03:38:01 -0400 Subject: [PATCH 2/5] fix: retry contended stale claims and clean up failed waiters Co-Authored-By: Paperclip --- core/status.ts | 144 +++++++++++++++++++++---------------- tests/state-store.test.mjs | 95 +++++++++++++++++++++++- 2 files changed, 178 insertions(+), 61 deletions(-) diff --git a/core/status.ts b/core/status.ts index 886511c..d9b721f 100644 --- a/core/status.ts +++ b/core/status.ts @@ -527,74 +527,97 @@ export async function acquireLock(lockPath: string, options: AcquireLockOptions }, Math.max(50, Math.floor(staleMs / 4))); heartbeat.unref?.(); - const giveUp = async (): Promise => { - clearInterval(heartbeat); - await rm(ticketPath, { force: true }).catch(() => undefined); + const giveUp = (): never => { throw new Error(`Timed out waiting for lock: ${lockPath}`); }; - while (true) { - const names = await readdir(dir).catch(() => [] as string[]); - let blocked = false; - - // A pre-#355 lock file: honour it while fresh, remove it when stale. - if (names.includes(base)) { - const legacyStat = await fsOps.stat(lockPath).catch((error: NodeJS.ErrnoException) => { - if (error.code !== "ENOENT") blocked = true; - return null; - }); - if (legacyStat && Date.now() - legacyStat.mtimeMs <= staleMs) blocked = true; - else if (legacyStat) { - const holder = await describeLockHolder(lockPath); - if (await removeIfPresent(lockPath, lockPath, fsOps)) brokeStale = holder; + let acquired = false; + try { + while (true) { + const names = await readdir(dir).catch(() => [] as string[]); + let blocked = false; + // Failed tombstone cleanup must not leave permanent files in a repo. + for (const name of names) { + if (!name.startsWith(`${base}.reaped.`)) continue; + const path = join(dir, name); + const fileStat = await fsOps.stat(path).catch(() => null); + if (fileStat && Date.now() - fileStat.mtimeMs > staleMs) { + await rm(path, { force: true }).catch(() => undefined); + } } - } - // Someone is between taking a number and writing their ticket: their - // number may be earlier than ours. Wait, unless they died in the door. - for (const name of names) { - if (!name.startsWith(`${base}.c.`) || name === basename(choosingPath)) continue; - const path = join(dir, name); - if (await ownerIsGone(path, staleMs, fsOps)) { - await rm(path, { force: true }).catch(() => undefined); - continue; + // A pre-#355 lock file: honour it while fresh, remove it when stale. + if (names.includes(base)) { + const legacyStat = await fsOps.stat(lockPath).catch((error: NodeJS.ErrnoException) => { + if (error.code !== "ENOENT") blocked = true; + return null; + }); + if (legacyStat && Date.now() - legacyStat.mtimeMs <= staleMs) blocked = true; + else if (legacyStat) { + const holder = await describeLockHolder(lockPath); + const claim = await removeIfPresent(lockPath, lockPath, fsOps); + if (claim === "claimed") brokeStale = holder; + if (claim === "busy") blocked = true; + } } - blocked = true; - } - // Every ticket ahead of ours whose owner is alive blocks us; a dead or - // hung owner's ticket is removed by its own name. - if (!blocked) { - const ahead = names.filter((name) => name.startsWith(`${base}.t.`) && name < ticketName).sort(); - for (const name of ahead) { + // Someone is between taking a number and writing their ticket: their + // number may be earlier than ours. Wait, unless they died in the door. + for (const name of names) { + if (!name.startsWith(`${base}.c.`) || name === basename(choosingPath)) continue; const path = join(dir, name); if (await ownerIsGone(path, staleMs, fsOps)) { - // Only the waiter that claims the dead ticket records its holder; - // other waiters cannot claim the same path. - const holder = await describeLockHolder(path); - if (await removeIfPresent(path, lockPath, fsOps)) brokeStale = holder; + await rm(path, { force: true }).catch(() => undefined); continue; } blocked = true; - break; } - } - if (!blocked) { - // Our own ticket must still be there: a waiter that judged us hung - // (the machine slept past staleMs) has already let someone in. - if (!(await pathExists(ticketPath))) return giveUp(); - return { - release: async () => { - clearInterval(heartbeat); - await rm(ticketPath, { force: true }).catch(() => undefined); - }, - ...(brokeStale && { brokeStale }), - }; - } + // Every ticket ahead of ours whose owner is alive blocks us; a dead or + // hung owner's ticket is removed by its own name. + if (!blocked) { + const ahead = names.filter((name) => name.startsWith(`${base}.t.`) && name < ticketName).sort(); + for (const name of ahead) { + const path = join(dir, name); + if (await ownerIsGone(path, staleMs, fsOps)) { + // Only the waiter that claims the dead ticket records its holder; + // other waiters cannot claim the same path. + const holder = await describeLockHolder(path); + const claim = await removeIfPresent(path, lockPath, fsOps); + if (claim === "claimed") brokeStale = holder; + if (claim === "busy") { + blocked = true; + break; + } + continue; + } + blocked = true; + break; + } + } + + if (!blocked) { + // Our own ticket must still be there: a waiter that judged us hung + // (the machine slept past staleMs) has already let someone in. + if (!(await pathExists(ticketPath))) giveUp(); + acquired = true; + return { + release: async () => { + clearInterval(heartbeat); + await rm(ticketPath, { force: true }).catch(() => undefined); + }, + ...(brokeStale && { brokeStale }), + }; + } - if (Date.now() - startedAt > timeoutMs) return giveUp(); - await sleep(LOCK_RETRY_MS); + if (Date.now() - startedAt > timeoutMs) giveUp(); + await sleep(LOCK_RETRY_MS); + } + } finally { + if (!acquired) { + clearInterval(heartbeat); + await rm(ticketPath, { force: true }).catch(() => undefined); + } } } @@ -607,28 +630,29 @@ async function removeIfPresent( path: string, lockPath: string, fsOps: NonNullable, -): Promise { +): Promise<"claimed" | "gone" | "busy"> { const tombstone = `${lockPath}.reaped.${randomBytes(8).toString("hex")}`; try { await fsOps.rename(path, tombstone); } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return false; - // Windows can report a losing rename as EPERM or EBUSY. Confirm - // absence; a real permission/sharing error must still surface. - if (["EPERM", "EBUSY"].includes((error as NodeJS.ErrnoException).code ?? "")) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return "gone"; + // Windows can report contention as a permission or sharing error. + // An existing source blocks this round; a missing one lost the race. + if (["EPERM", "EBUSY", "EACCES"].includes((error as NodeJS.ErrnoException).code ?? "")) { try { await fsOps.stat(path); } catch (statError) { - if ((statError as NodeJS.ErrnoException).code === "ENOENT") return false; + if ((statError as NodeJS.ErrnoException).code === "ENOENT") return "gone"; throw statError; } + return "busy"; } throw error; } // Cleanup is best effort: the original claim is already gone. A leftover // tombstone cannot be mistaken for a ticket or legacy lock. await fsOps.unlink(tombstone).catch(() => undefined); - return true; + return "claimed"; } /** Create `path` exclusively with `content`; the descriptor is closed either way (#131). */ diff --git a/tests/state-store.test.mjs b/tests/state-store.test.mjs index 869bddc..905b73d 100644 --- a/tests/state-store.test.mjs +++ b/tests/state-store.test.mjs @@ -8,7 +8,7 @@ import assert from "node:assert/strict"; import { existsSync } from "node:fs"; import { mkdir, mkdtemp, readFile, readdir, rename, rm, stat, unlink, utimes, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { dirname, join, resolve } from "node:path"; +import { basename, dirname, join, resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); @@ -285,12 +285,105 @@ test("Windows unlink errors after a stale-ticket claim cannot give two waiters t assert.equal(claimed, 1, "one rename claimed the dead ticket"); assert.equal(handles.filter((handle) => handle.brokeStale).length, 1); assert.equal(errors, 1, `the winning claim encountered ${code} on cleanup`); + const [tombstone] = (await readdir(dir)).filter((name) => name.includes(".reaped.")); + assert.ok(tombstone, "failed cleanup left a tombstone"); + const old = new Date(Date.now() - 1_000); + await utimes(join(dir, tombstone), old, old); + const next = await acquireLock(lockPath, { fsOps, staleMs: 100 }); + await next.release(); + assert.deepEqual(await readdir(dir), [], "the next waiter swept the old tombstone"); } finally { await cleanup(); } } }); +test("Windows claim contention retries while the stale ticket still exists", async () => { + for (const code of ["EPERM", "EBUSY", "EACCES"]) { + const { dir, cleanup } = await tempDir("cc-lock-claim-busy-"); + try { + const lockPath = join(dir, "status.yaml.lock"); + const dead = await deadTicket(dir); + let attempts = 0; + const fsOps = { + stat, + rename: async (from, to) => { + if (from === dead && attempts++ === 0) { + const error = new Error("ticket held open by another waiter"); + error.code = code; + throw error; + } + await rename(from, to); + }, + unlink, + }; + const handle = await acquireLock(lockPath, { fsOps, timeoutMs: 1_000 }); + assert.equal(attempts, 2, `${code} retried the claim`); + assert.equal(handle.brokeStale?.pid, 4194303); + await handle.release(); + assert.deepEqual(await readdir(dir), [], "no ticket remains after release"); + } finally { + await cleanup(); + } + } +}); + +test("Windows claim errors after another waiter removed the ticket do not report a break", async () => { + for (const code of ["EPERM", "EBUSY", "EACCES"]) { + const { dir, cleanup } = await tempDir("cc-lock-claim-gone-"); + try { + const lockPath = join(dir, "status.yaml.lock"); + const dead = await deadTicket(dir); + let attempts = 0; + const fsOps = { + stat, + rename: async (from, to) => { + if (from === dead) { + attempts += 1; + await unlink(from); + const error = new Error("losing Windows claim"); + error.code = code; + throw error; + } + await rename(from, to); + }, + unlink, + }; + const handle = await acquireLock(lockPath, { fsOps }); + assert.equal(attempts, 1); + assert.equal(handle.brokeStale, undefined, `${code} was a lost race`); + await handle.release(); + assert.deepEqual(await readdir(dir), []); + } finally { + await cleanup(); + } + } +}); + +test("a claim error removes this waiter's ticket before rejecting", async () => { + const { dir, cleanup } = await tempDir("cc-lock-claim-error-"); + try { + const lockPath = join(dir, "status.yaml.lock"); + const dead = await deadTicket(dir); + const fsOps = { + stat, + rename: async () => { + const error = new Error("claim failed"); + error.code = "EIO"; + throw error; + }, + unlink, + }; + await assert.rejects(acquireLock(lockPath, { fsOps }), /claim failed/); + assert.deepEqual(await readdir(dir), [basename(dead)], "the failed waiter's ticket was removed"); + const handle = await acquireLock(lockPath); + await handle.release(); + assert.deepEqual(await readdir(dir), []); + } finally { + await cleanup(); + } +}); + test("a non-ENOENT ticket stat error is not evidence that its owner is gone", async () => { const { dir, cleanup } = await tempDir("cc-lock-stat-"); try { From d2387b04b030449e72877a0f64af51ae474e88da Mon Sep 17 00:00:00 2001 From: James Sesler Date: Fri, 25 Sep 2026 03:42:05 -0400 Subject: [PATCH 3/5] test: identify ticket paths claimed in Windows race Co-Authored-By: Paperclip --- tests/state-store.test.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/state-store.test.mjs b/tests/state-store.test.mjs index 905b73d..4572383 100644 --- a/tests/state-store.test.mjs +++ b/tests/state-store.test.mjs @@ -258,14 +258,14 @@ test("Windows unlink errors after a stale-ticket claim cannot give two waiters t const { dir, cleanup } = await tempDir("cc-lock-windows-"); try { const lockPath = join(dir, "status.yaml.lock"); - await deadTicket(dir); - let claimed = 0; + const dead = await deadTicket(dir); + const claimedPaths = []; let errors = 0; const fsOps = { stat, rename: async (from, to) => { await rename(from, to); - if (from.includes(".t.")) claimed += 1; + if (from.includes(".t.")) claimedPaths.push(from); }, unlink: async (path) => { if (path.includes(".reaped.")) { @@ -282,7 +282,7 @@ test("Windows unlink errors after a stale-ticket claim cannot give two waiters t await handle.release(); return handle; })); - assert.equal(claimed, 1, "one rename claimed the dead ticket"); + assert.deepEqual(claimedPaths, [dead], "only the planted dead ticket was claimed"); assert.equal(handles.filter((handle) => handle.brokeStale).length, 1); assert.equal(errors, 1, `the winning claim encountered ${code} on cleanup`); const [tombstone] = (await readdir(dir)).filter((name) => name.includes(".reaped.")); From 229aa8dd7514d6ff7dd4aa2b8f47818e8d98ec4b Mon Sep 17 00:00:00 2001 From: James Sesler Date: Fri, 25 Sep 2026 03:47:09 -0400 Subject: [PATCH 4/5] fix: arbitrate stale ticket claims with exclusive marker Co-Authored-By: Paperclip --- core/status.ts | 30 +++++++++++++++++++++++++----- tests/state-store.test.mjs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/core/status.ts b/core/status.ts index d9b721f..17fb7c7 100644 --- a/core/status.ts +++ b/core/status.ts @@ -536,12 +536,15 @@ export async function acquireLock(lockPath: string, options: AcquireLockOptions while (true) { const names = await readdir(dir).catch(() => [] as string[]); let blocked = false; - // Failed tombstone cleanup must not leave permanent files in a repo. + // Failed cleanup must not leave permanent tombstones or claim markers. for (const name of names) { - if (!name.startsWith(`${base}.reaped.`)) continue; + const claimPrefix = `${base}.claim.`; + if (!name.startsWith(`${base}.reaped.`) && !name.startsWith(claimPrefix)) continue; const path = join(dir, name); const fileStat = await fsOps.stat(path).catch(() => null); - if (fileStat && Date.now() - fileStat.mtimeMs > staleMs) { + const claimSourceGone = name.startsWith(claimPrefix) + && !(await pathExists(join(dir, name.slice(claimPrefix.length)))); + if (claimSourceGone || (fileStat && Date.now() - fileStat.mtimeMs > staleMs)) { await rm(path, { force: true }).catch(() => undefined); } } @@ -632,9 +635,19 @@ async function removeIfPresent( fsOps: NonNullable, ): Promise<"claimed" | "gone" | "busy"> { const tombstone = `${lockPath}.reaped.${randomBytes(8).toString("hex")}`; + // Windows can let concurrent renames of the same source both report + // success. This stable, exclusive marker decides who may claim that name. + const claimPath = `${lockPath}.claim.${basename(path)}`; + try { + await writeExclusive(claimPath, `${process.pid}\n`); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") return "busy"; + throw error; + } try { await fsOps.rename(path, tombstone); } catch (error) { + await rm(claimPath, { force: true }).catch(() => undefined); if ((error as NodeJS.ErrnoException).code === "ENOENT") return "gone"; // Windows can report contention as a permission or sharing error. // An existing source blocks this round; a missing one lost the race. @@ -649,9 +662,16 @@ async function removeIfPresent( } throw error; } - // Cleanup is best effort: the original claim is already gone. A leftover - // tombstone cannot be mistaken for a ticket or legacy lock. + // Keep the claim marker until stale cleanup: Windows can briefly expose + // the source path after rename reports success. Neither file is a ticket. await fsOps.unlink(tombstone).catch(() => undefined); + try { + await fsOps.stat(path); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + await rm(claimPath, { force: true }).catch(() => undefined); + } + } return "claimed"; } diff --git a/tests/state-store.test.mjs b/tests/state-store.test.mjs index 4572383..b0b1779 100644 --- a/tests/state-store.test.mjs +++ b/tests/state-store.test.mjs @@ -328,6 +328,40 @@ test("Windows claim contention retries while the stale ticket still exists", asy } }); +test("only one waiter reports a break when Windows exposes the source after rename succeeds", async () => { + const { dir, cleanup } = await tempDir("cc-lock-claim-visible-"); + try { + const lockPath = join(dir, "status.yaml.lock"); + const dead = await deadTicket(dir); + let renameCalls = 0; + const fsOps = { + stat, + rename: async (from, to) => { + if (from === dead) { + renameCalls += 1; + return; // Model Windows reporting success while the source is still visible. + } + await rename(from, to); + }, + unlink, + }; + const first = await acquireLock(lockPath, { fsOps, timeoutMs: 1_000 }); + assert.equal(first.brokeStale?.pid, 4194303); + await first.release(); + const secondPromise = acquireLock(lockPath, { fsOps, timeoutMs: 1_000 }); + await wait(100); + assert.equal(await settled(secondPromise), false, "the claim marker blocks a second break"); + await unlink(dead); + const second = await secondPromise; + assert.equal(second.brokeStale, undefined); + assert.equal(renameCalls, 1); + await second.release(); + assert.deepEqual(await readdir(dir), []); + } finally { + await cleanup(); + } +}); + test("Windows claim errors after another waiter removed the ticket do not report a break", async () => { for (const code of ["EPERM", "EBUSY", "EACCES"]) { const { dir, cleanup } = await tempDir("cc-lock-claim-gone-"); From ffba2610e2c542d3d159f730b30993ac128733f4 Mon Sep 17 00:00:00 2001 From: James Sesler Date: Sat, 26 Sep 2026 02:49:00 -0400 Subject: [PATCH 5/5] build: keep @internal test seams out of the published declarations (#444 review) AcquireLockOptions is re-exported through core/index.ts, so the fsOps test seam was emitted into dist/core/status.d.ts and offered to every consumer of the published types. stripInternal drops members marked @internal from declarations; runtime code is unchanged. fsOps is the only @internal in core/, mcp-server/ or extensions/. --- tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tsconfig.json b/tsconfig.json index 7e4b2d1..d63c89a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -10,6 +10,7 @@ "outDir": "dist", "rootDir": ".", "declaration": true, + "stripInternal": true, "isolatedModules": true, "allowJs": true, "strict": false,