fix: retry lock reads during Windows owner handoff

CHANGELOG.md

@@ -17,8 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Retry a denied lock-directory scan during Windows publication handoff within
-  the existing wait bound. Retain the native error when a denial persists and
-  preserve the owner. Avoid rejecting contenders during normal lock handoff.
+  the existing wait bound. Also retry denied owner-record reads and lock
+  observations while a prior owner removes them. Retain the native error when
+  denial persists and preserve the owner. Avoid rejecting contenders during
+  normal lock handoff.
 
 - Prepare file and directory locks with unique owner records before publishing
   them. Recover only the observed dead owner so delayed stale-lock recovery

docs/0.5.0-api.md

@@ -54,7 +54,7 @@ File transactions and directory publication now share one lock protocol: prepare
 
 The executed matrix covers two simultaneous stale-owner observations for file and directory operations, exact stale-write rejection, unrelated lock contents, junctions, fresh malformed-record timeout, aged legacy recovery, partial owner writes, failed preparation cleanup and an exclusive preparation collision. It also verifies successor ownership during release, denied process probes and filesystem errors during acquisition. The original six minimal cases failed before the fix. Separate child-process cases terminate after preparing an owner but before publishing the lock: data is unchanged, no incomplete live lock is exposed, retry succeeds and the abandoned private preparation remains available for manual cleanup. Existing file/directory termination and recovery cases also run with the shared protocol. These results qualify the tested process checkpoints and cooperating current CLI commands, not power-loss durability or concurrent use of older lock protocols.
 
-On Windows, scanning a lock directory while its previous owner removes it can return `EPERM`. The acquisition loop retries that specific scan failure within its existing ten-second bound and retains the native cause if it persists; other filesystem errors still propagate. A repeated four-writer generator probe reproduced the native failure before the fix. Deterministic file/directory cases verify transient recovery, permanent-denial timeout, exact bytes and owner preservation.
+On Windows, inspecting a lock directory or opening its owner record while its previous owner removes it can return `EPERM`. The acquisition loop retries read-only lock observations (`lstat`, `scandir`, `open`, `read`) within its existing ten-second bound and retains the native cause if denial persists; write/deletion failures and other filesystem errors still propagate. A repeated four-writer generator probe reproduced native scan and owner-open failures on separate source heads. Deterministic file/directory cases verify transient recovery, permanent-denial timeout, exact bytes and owner preservation; separate deletion-denial cases verify those errors propagate without entering a transaction.
 
 ## Release boundaries
 

src/filesystem-lock.ts

@@ -7,6 +7,7 @@ const LOCK_TIMEOUT_MS = 10_000;
 const ORPHANED_LOCK_AGE_MS = 30_000;
 const OWNER_FILE = /^owner-[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}\.json$/i;
 const RENAME_CONTENTION_CODES = new Set(["EEXIST", "ENOTEMPTY", "EPERM", "EBUSY", "EACCES"]);
+const LOCK_READ_SYSCALLS = new Set(["lstat", "scandir", "open", "read"]);
 
 function isNodeError(error: unknown, code: string): error is NodeJS.ErrnoException {
   return error instanceof Error && "code" in error && error.code === code;
@@ -112,11 +113,11 @@ export async function acquireFilesystemLock(
           { cause: lastError },
         );
       }
-      const lockStat = await statIfPresent(lock);
-      if (lockStat && (!lockStat.isDirectory() || lockStat.isSymbolicLink())) {
-        throw ambiguousLock(lock);
-      }
       try {
+        const lockStat = await statIfPresent(lock);
+        if (lockStat && (!lockStat.isDirectory() || lockStat.isSymbolicLink())) {
+          throw ambiguousLock(lock);
+        }
         await fs.rename(stage, lock);
         return async () => {
           await fs.unlink(path.join(lock, ownerName));
@@ -131,9 +132,12 @@ export async function acquireFilesystemLock(
         try {
           if (await removeOrphanedLock(lock)) continue;
         } catch (inspectionError) {
-          // Windows can reject a scan while another owner removes this lock.
+          // Windows can deny reads while another owner removes its lock record.
           // Retry within the same deadline; a permanent denial retains its cause.
-          if (!isNodeError(inspectionError, "EPERM") || inspectionError.syscall !== "scandir") {
+          if (
+            !isNodeError(inspectionError, "EPERM") ||
+            !LOCK_READ_SYSCALLS.has(inspectionError.syscall ?? "")
+          ) {
             throw inspectionError;
           }
           lastError = inspectionError;

tests/filesystem-locks.test.ts

@@ -44,29 +44,56 @@ afterEach(async () => {
   await Promise.all(roots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true })));
 });
 
+const observationCases = ["directory", "file"].flatMap((kind) =>
+  (["scandir", "open", "read", "lstat"] as const).map((syscall) => ({ kind, syscall })),
+);
+function denyLockObservation(
+  syscall: "scandir" | "open" | "read" | "lstat",
+  lock: string,
+  fail: () => void,
+) {
+  if (syscall === "scandir") {
+    const readdir = fs.readdir.bind(fs);
+    vi.spyOn(fs, "readdir").mockImplementation((async (...args: Parameters<typeof fs.readdir>) => {
+      if (String(args[0]) === lock) fail();
+      return readdir(...args);
+    }) as typeof fs.readdir);
+  } else if (syscall === "open" || syscall === "read") {
+    const readFile = fs.readFile.bind(fs);
+    vi.spyOn(fs, "readFile").mockImplementation((async (
+      ...args: Parameters<typeof fs.readFile>
+    ) => {
+      if (String(args[0]) === path.join(lock, "owner.json")) fail();
+      return readFile(...args);
+    }) as typeof fs.readFile);
+  } else {
+    const lstat = fs.lstat.bind(fs);
+    vi.spyOn(fs, "lstat").mockImplementation((async (...args: Parameters<typeof fs.lstat>) => {
+      if (String(args[0]) === lock) fail();
+      return lstat(...args);
+    }) as typeof fs.lstat);
+  }
+}
+
 describe("filesystem lock ownership", () => {
-  it.each(["directory", "file"])(
-    "retries a transient Windows-style scandir denial while the %s lock is handed off",
-    async (kind) => {
+  it.each(observationCases)(
+    "retries a transient Windows-style $syscall denial while the $kind lock is handed off",
+    async ({ kind, syscall }) => {
       const { root, target, lock } = await fixture(kind);
       await fs.mkdir(lock);
       await fs.writeFile(path.join(lock, "owner.json"), '{"pid":2147483647}');
-      const readdir = fs.readdir.bind(fs);
-      const denied = Object.assign(new Error("injected pending-deletion directory scan"), {
+      const denied = Object.assign(new Error("injected pending-deletion lock observation"), {
         code: "EPERM",
-        syscall: "scandir",
-        path: lock,
+        syscall,
+        path: syscall === "open" || syscall === "read" ? path.join(lock, "owner.json") : lock,
       });
       let failed = false;
-      vi.spyOn(fs, "readdir").mockImplementation((async (
-        ...args: Parameters<typeof fs.readdir>
-      ) => {
-        if (String(args[0]) === lock && !failed) {
+      denyLockObservation(syscall, lock, () => {
+        if (!failed) {
           failed = true;
           throw denied;
         }
-        return readdir(...args);
-      }) as typeof fs.readdir);
+      });
       let entered = 0;
       await expect(
         operate(kind, target, async () => {
@@ -80,30 +107,24 @@ describe("filesystem lock ownership", () => {
     },
   );
 
-  it.each(["directory", "file"])(
-    "bounds a permanent %s lock scan denial and retains the native cause without deleting its owner",
-    async (kind) => {
+  it.each(observationCases)(
+    "bounds a permanent $kind lock $syscall denial and retains the native cause without deleting its owner",
+    async ({ kind, syscall }) => {
       const { root, target, lock } = await fixture(kind);
       await fs.mkdir(lock);
       const owner = path.join(lock, "owner.json");
       await fs.writeFile(owner, '{"pid":2147483647}');
-      const denied = Object.assign(new Error("injected permanent directory scan denial"), {
+      const denied = Object.assign(new Error("injected permanent lock observation denial"), {
         code: "EPERM",
-        syscall: "scandir",
-        path: lock,
+        syscall,
+        path: syscall === "open" || syscall === "read" ? owner : lock,
       });
       let now = Date.now();
       vi.spyOn(Date, "now").mockImplementation(() => now);
-      const readdir = fs.readdir.bind(fs);
-      vi.spyOn(fs, "readdir").mockImplementation((async (
-        ...args: Parameters<typeof fs.readdir>
-      ) => {
-        if (String(args[0]) === lock) {
-          now += 10_001;
-          throw denied;
-        }
-        return readdir(...args);
-      }) as typeof fs.readdir);
+      denyLockObservation(syscall, lock, () => {
+        now += 10_001;
+        throw denied;
+      });
       await expect(
         operate(kind, target, async () => {
           throw new Error("must not enter");
@@ -112,6 +133,7 @@ describe("filesystem lock ownership", () => {
         message: expect.stringContaining("Timed out waiting"),
         cause: denied,
       });
+      vi.restoreAllMocks();
       expect(await fs.readFile(owner, "utf8")).toBe('{"pid":2147483647}');
       if (kind === "file") expect(await fs.readFile(target, "utf8")).toBe("old");
       expect((await fs.readdir(root)).sort()).toEqual(
@@ -122,6 +144,46 @@ describe("filesystem lock ownership", () => {
     },
   );
 
+  it.each(["unlink", "rmdir"] as const)(
+    "propagates an EPERM %s failure instead of treating a deletion failure as observation contention",
+    async (method) => {
+      const { root, target, lock } = await fixture("file");
+      await fs.mkdir(lock);
+      const owner = path.join(lock, "owner.json");
+      await fs.writeFile(owner, '{"pid":2147483647}');
+      const failure = Object.assign(new Error("injected lock deletion denial"), {
+        code: "EPERM",
+        syscall: method,
+        path: method === "unlink" ? owner : lock,
+      });
+      if (method === "unlink") {
+        const unlink = fs.unlink.bind(fs);
+        vi.spyOn(fs, "unlink").mockImplementation(async (...args) => {
+          if (String(args[0]) === owner) throw failure;
+          return unlink(...args);
+        });
+      } else {
+        const rmdir = fs.rmdir.bind(fs);
+        vi.spyOn(fs, "rmdir").mockImplementation(async (...args) => {
+          if (String(args[0]) === lock) throw failure;
+          return rmdir(...args);
+        });
+      }
+      await expect(
+        operate("file", target, async () => {
+          throw new Error("must not enter");
+        }),
+      ).rejects.toBe(failure);
+      vi.restoreAllMocks();
+      expect(await fs.readFile(target, "utf8")).toBe("old");
+      expect((await fs.readdir(root)).sort()).toEqual(
+        [path.basename(lock), "manifest.json"].sort(),
+      );
+      if (method === "unlink") expect(await fs.readFile(owner, "utf8")).toBe('{"pid":2147483647}');
+      else expect(await fs.readdir(lock)).toEqual([]);
+    },
+  );
+
   it.each(["directory", "file"])(
     "does not remove a successor's %s owner during release",
     async (kind) => {