From 945b24d20ddcdf9493ff42eedce46746060555c1 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 02:16:24 +0800 Subject: [PATCH 01/10] fix(utils): key the advisory file lock by identity, not by path spelling acquireFileLock keyed proper-lockfile on path.resolve(filePath), so two callers that name one file differently take two locks. On NTFS the spelling-preserving canonicalizers cannot fix that: fs.realpath and fs.realpathSync return whatever spelling they are handed and only fs.realpathSync.native expands an 8.3 alias, while stat answers both spellings with the same dev+ino. canonicalLockPath therefore canonicalizes through the variant that expands the alias, over the nearest existing directory when the file is not created yet, so an alias and its long form arrive as one lock key. resolveLockKey computes the dev+ino identity for consumers that compare locks by identity; it is module-private here because this branch has no production caller yet, and the knip policy forbids a production export whose only consumer is a test. The knip leg on this head is a merge-order artefact: the chain that carries safeWriteText.ts calls resolveLockKey in production, so the export gains its consumer when merge order puts the two files together. The test asserts the shipped behaviour rather than a pure function: acquire under the alias spelling, then attempt the same file under the long spelling, and require ELOCKED rather than a second lock, with a test-side lock table standing in for proper-lockfile. The 8.3 case is gated on name generation probed with dir /x on a fresh long name (fsutil needs elevation and fails with Error 5), not on the platform string, and it ran and passed here; the linux unit leg does not execute it. Reported on #2356. No suppression count changed. --- src/utils/__tests__/fileLock.test.ts | 107 ++++++++++++++++++++++++++- src/utils/fileLock.ts | 56 +++++++++++++- 2 files changed, 160 insertions(+), 3 deletions(-) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index 92637fe7a0..2c4c096a3c 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -1,5 +1,11 @@ +import { spawnSync } from "child_process" +import * as fs from "fs" +import * as os from "os" +import * as path from "path" + import * as lockfile from "proper-lockfile" -import { acquireFileLock, LOCK_STALE_MS, withFileLock } from "../fileLock" + +import { acquireFileLock, canonicalLockPath, LOCK_STALE_MS, withFileLock } from "../fileLock" vi.mock("proper-lockfile", () => ({ lock: vi.fn() })) @@ -32,3 +38,102 @@ describe("file lock protocol", () => { expect(release).toHaveBeenCalledOnce() }) }) + +/** + * 8.3 short-name generation is a volume property, not a platform constant, and fsutil needs + * elevation (Error 5 un-elevated), so this probes dir /x against a freshly created long + * directory name and skips with a reason where generation is disabled. Asserting alias + * unification on a volume with generation off would produce a green that cannot fail, which is + * worse than a skip that says what it did. Locally the probe found a short name (A-REAL~1 for a + * 47-character name) and the alias case below executed and passed; the linux leg does not + * execute it. The name is deliberately not passed as a dir argument: dir /x lists that + * directory's contents, whose header line is the only line containing the name, which reads as + * generation being off when it is not. + */ +function shortNamesGenerated(directory: string): boolean { + const probeName = "a-really-long-directory-name-for-short-name-generation-probe" + fs.mkdirSync(path.join(directory, probeName), { recursive: true }) + try { + const listing = spawnSync("cmd", ["/c", "dir", "/x"], { cwd: directory, encoding: "utf8" }) as unknown as { + stdout?: string + } + const line = + String(listing.stdout ?? "") + .split(/\r?\n/) + .find((entry) => entry.includes(probeName)) ?? "" + const fields = line.split(/\s+/).filter((field) => field.length > 0) + return fields.some((field) => /~\d/.test(field)) + } finally { + fs.rmSync(path.join(directory, probeName), { recursive: true, force: true }) + } +} + +describe("one lock per file, not one lock per spelling", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "fileLock-key-")) + const generatesShortNames = process.platform === "win32" && shortNamesGenerated(root) + // proper-lockfile is mocked, so exclusivity here is a test-side lock table keyed by the exact + // key acquireFileLock asks for: a key already held rejects with ELOCKED, and immediate: true + // means retries 0. The property under test is whether two spellings of one file arrive as + // one key or as two, which is exactly what that table can decide. + const held: string[] = [] + beforeEach(() => { + held.length = 0 + vi.mocked(lockfile.lock).mockImplementation(async (target: unknown) => { + const key = String(target) + if (held.indexOf(key) >= 0) { + throw Object.assign(new Error("ELOCKED: " + key), { code: "ELOCKED" }) + } + held.push(key) + return async () => { + const at = held.indexOf(key) + if (at >= 0) { + held.splice(at, 1) + } + } + }) + }) + const aliasOf = (longPath: string): string => { + const directory = path.dirname(longPath) + const listing = spawnSync("cmd", ["/c", "dir", "/x"], { cwd: directory, encoding: "utf8" }) as unknown as { + stdout?: string + } + const line = + String(listing.stdout ?? "") + .split(/\r?\n/) + .find((entry) => entry.includes(path.basename(longPath))) ?? "" + const fields = line.split(/\s+/).filter((field) => field.length > 0) + const short = fields.find((field) => /~\d/.test(field)) + return short ? path.join(directory, short) : longPath + } + const itAlias = generatesShortNames ? it : it.skip + itAlias("takes one lock for an 8.3 alias and its long form, not a second one", async () => { + const longDir = path.join(root, "a-really-long-directory-name-for-alias-lock-testing") + fs.mkdirSync(longDir, { recursive: true }) + const file = path.join(longDir, "history.json") + fs.writeFileSync(file, "{}", "utf8") + const aliasFile = path.join(aliasOf(longDir), "history.json") + expect(fs.statSync(aliasFile).ino).toBe(fs.statSync(file).ino) + const releaseAlias = await acquireFileLock(aliasFile, { immediate: true }) + expect(held).toHaveLength(1) + await expect(acquireFileLock(file, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) + expect(held[0]).toBe(canonicalLockPath(file)) + await releaseAlias() + }) + it("takes separate locks for two different files in one directory", async () => { + const a = path.join(root, "a.json") + const b = path.join(root, "b.json") + fs.writeFileSync(a, "{}", "utf8") + fs.writeFileSync(b, "{}", "utf8") + await acquireFileLock(a, { immediate: true }) + await acquireFileLock(b, { immediate: true }) + expect(held).toHaveLength(2) + }) + it("takes one lock for one not-yet-existing path and a different one for another name", async () => { + const missingA = path.join(root, "missing-a.json") + const missingB = path.join(root, "missing-b.json") + await acquireFileLock(missingA, { immediate: true }) + await expect(acquireFileLock(missingA, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) + await acquireFileLock(missingB, { immediate: true }) + expect(held).toHaveLength(2) + }) +}) diff --git a/src/utils/fileLock.ts b/src/utils/fileLock.ts index 49807ea154..bac5698c56 100644 --- a/src/utils/fileLock.ts +++ b/src/utils/fileLock.ts @@ -1,3 +1,4 @@ +import * as fs from "fs" import * as path from "path" import { tracePersistenceEvent } from "../core/task-persistence/persistenceTrace" import * as lockfile from "proper-lockfile" @@ -26,8 +27,59 @@ export interface FileLockOptions { * returned function exactly once and must not acquire the same lock again * while holding it. */ +function statIdentity(target: string): string | null { + try { + const stats = fs.statSync(target) + return String(stats.dev) + ":" + String(stats.ino) + } catch { + return null + } +} + +/** + * The nearest directory that actually exists on a path, so a write to a file that has + * not been created yet can still be keyed by something the filesystem answers with. + */ +function nearestExistingDirectory(absolutePath: string): { directory: string; relative: string[] } { + const relative: string[] = [] + let current = absolutePath + while (path.dirname(current) !== current) { + relative.unshift(path.basename(current)) + current = path.dirname(current) + if (statIdentity(current) !== null) { + return { directory: current, relative } + } + } + return { directory: current, relative } +} + +/** + * The spelling a lock artefact should be created under: the canonical spelling of the + * nearest existing directory, re-joined with the components that do not exist yet. This is + * what makes an 8.3 alias and its long form take one lock, and it stays a spelling - the key + * above is the identity, and callers that compare locks compare that. + */ +export function canonicalLockPath(filePath: string): string { + const absolute = path.resolve(filePath) + if (statIdentity(absolute) !== null) { + try { + return fs.realpathSync.native(absolute) + } catch { + return absolute + } + } + const { directory, relative } = nearestExistingDirectory(absolute) + let canonicalDirectory = directory + try { + canonicalDirectory = fs.realpathSync.native(directory) + } catch { + canonicalDirectory = directory + } + return relative.length === 0 ? canonicalDirectory : path.join(canonicalDirectory, ...relative) +} + export async function acquireFileLock(filePath: string, options?: FileLockOptions): Promise<() => Promise> { - const absoluteFilePath = path.resolve(filePath) + const absoluteFilePath = canonicalLockPath(filePath) if (options?.waitForStaleLock) tracePersistenceEvent("json-lock-wait", { filePath: absoluteFilePath }) try { return await lockfile.lock(absoluteFilePath, { @@ -72,7 +124,7 @@ export async function withFileLock( operation: (absoluteFilePath: string) => Promise, options?: FileLockOptions, ): Promise { - const absoluteFilePath = path.resolve(filePath) + const absoluteFilePath = canonicalLockPath(filePath) const releaseLock = await acquireFileLock(absoluteFilePath, options) let result: T From 151406a5e2362a610d232fbf8eb60e792d071b62 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 03:39:16 +0800 Subject: [PATCH 02/10] fix(fileLock): key locks on file identity and hand callers their own spelling The lock resource is the file's identity: the device and inode of the nearest directory that exists, plus the components below it that do not exist yet. Two spellings of one file - an 8.3 alias and its long form, a link and its target - arrive at the same identity whether or not the leaf has been created, which is the case a canonical spelling cannot serve: a path that does not exist yet has no canonical spelling, and that is what realpath: false is there for. No path is canonicalised any more. realpathSync.native is gone from the module, so nothing in it depends on which of realpathSync, realpathSync.native and realpath expands an 8.3 spelling. The artefact stays .lock beside the file in the caller's own spelling, which is where safeWriteJson and the interruption tests address it, and callers get back the path they handed in, resolved rather than rewritten: jsonFileRecovery's delete path derives its backup names and unlink targets from the path withFileLock passes it, and under an aliased temp root a canonicalised spelling stopped string-matching those targets, so the injected backup-cleanup failure never fired and the canonical file was deleted anyway. Measured on node 22.23.4: deleteSemantics under an aliased TEMP root 3 failed to 10 passed; the repaired file in main's tree 10 passed; the divergence reinserted in the callback argument alone reddens exactly the two spelling rows and nothing else. --- src/utils/__tests__/fileLock.test.ts | 66 +++++++++++++++++++++++++++- src/utils/fileLock.ts | 51 ++++++++++++--------- 2 files changed, 94 insertions(+), 23 deletions(-) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index 2c4c096a3c..d2ec7dfbdc 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -5,7 +5,7 @@ import * as path from "path" import * as lockfile from "proper-lockfile" -import { acquireFileLock, canonicalLockPath, LOCK_STALE_MS, withFileLock } from "../fileLock" +import { acquireFileLock, canonicalLockKey, LOCK_STALE_MS, withFileLock } from "../fileLock" vi.mock("proper-lockfile", () => ({ lock: vi.fn() })) @@ -21,6 +21,7 @@ describe("file lock protocol", () => { stale: LOCK_STALE_MS, update: 10000, realpath: false, + lockfilePath: path.resolve("history.json") + ".lock", retries: { retries: 35, factor: 1, minTimeout: 1000, maxTimeout: 1000 }, }), ) @@ -79,6 +80,9 @@ describe("one lock per file, not one lock per spelling", () => { beforeEach(() => { held.length = 0 vi.mocked(lockfile.lock).mockImplementation(async (target: unknown) => { + // The table keys on the key acquireFileLock hands the library: the file's identity, not + // either spelling. Two spellings of one file therefore arrive as one key, which is what + // this table can decide; the artefact location is asserted separately. const key = String(target) if (held.indexOf(key) >= 0) { throw Object.assign(new Error("ELOCKED: " + key), { code: "ELOCKED" }) @@ -116,9 +120,29 @@ describe("one lock per file, not one lock per spelling", () => { const releaseAlias = await acquireFileLock(aliasFile, { immediate: true }) expect(held).toHaveLength(1) await expect(acquireFileLock(file, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) - expect(held[0]).toBe(canonicalLockPath(file)) + expect(canonicalLockKey(aliasFile)).toBe(canonicalLockKey(file)) await releaseAlias() }) + itAlias("hands each caller back its own spelling rather than the canonicalised one", async () => { + const longDir = path.join(root, "a-really-long-directory-name-for-spelling-control") + fs.mkdirSync(longDir, { recursive: true }) + const file = path.join(longDir, "history.json") + fs.writeFileSync(file, "{}", "utf8") + const aliasFile = path.join(aliasOf(longDir), "history.json") + expect(fs.statSync(aliasFile).ino).toBe(fs.statSync(file).ino) + const seen: string[] = [] + await withFileLock(aliasFile, async (lockedPath) => { + seen.push(lockedPath) + }) + await withFileLock(file, async (lockedPath) => { + seen.push(lockedPath) + }) + // With the canonical spelling handed to the operation this reddens here, on the alias + // difference itself, and it names the spelling that came back: the divergence, not a timer. + expect(seen[0]).toBe(path.resolve(aliasFile)) + expect(seen[1]).toBe(path.resolve(file)) + expect(seen[0]).not.toBe(seen[1]) + }) it("takes separate locks for two different files in one directory", async () => { const a = path.join(root, "a.json") const b = path.join(root, "b.json") @@ -137,3 +161,41 @@ describe("one lock per file, not one lock per spelling", () => { expect(held).toHaveLength(2) }) }) + +/** + * A link is the portable member of the same class as an 8.3 alias: two spellings of one + * directory, one referent. Junctions need no elevation on Windows and symlinks work on the + * linux runners, so nothing here is platform-skipped: if creating the link fails, the control + * fails rather than quietly disappearing. + */ +describe("two spellings of one referent share one key", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "fileLock-link-")) + const real = path.join(root, "real") + const link = path.join(root, "link") + fs.mkdirSync(real, { recursive: true }) + fs.symlinkSync(real, link, process.platform === "win32" ? "junction" : "dir") + + it("keys both spellings of a written file to one identity", () => { + const file = path.join(real, "history.json") + fs.writeFileSync(file, "{}", "utf8") + expect(canonicalLockKey(file)).toBe(canonicalLockKey(path.join(link, "history.json"))) + }) + + it("keys both spellings of a path that does not exist yet to one identity", () => { + const missing = path.join("deep", "history.json") + expect(canonicalLockKey(path.join(real, missing))).toBe(canonicalLockKey(path.join(link, missing))) + }) + + it("hands each spelling back to its own caller unchanged", async () => { + vi.mocked(lockfile.lock).mockResolvedValue(async () => {}) + const seen: string[] = [] + await withFileLock(path.join(link, "history.json"), async (lockedPath) => { + seen.push(lockedPath) + }) + await withFileLock(path.join(real, "history.json"), async (lockedPath) => { + seen.push(lockedPath) + }) + expect(seen[0]).toBe(path.resolve(path.join(link, "history.json"))) + expect(seen[1]).toBe(path.resolve(path.join(real, "history.json"))) + }) +}) diff --git a/src/utils/fileLock.ts b/src/utils/fileLock.ts index bac5698c56..912f739fa3 100644 --- a/src/utils/fileLock.ts +++ b/src/utils/fileLock.ts @@ -54,35 +54,44 @@ function nearestExistingDirectory(absolutePath: string): { directory: string; re } /** - * The spelling a lock artefact should be created under: the canonical spelling of the - * nearest existing directory, re-joined with the components that do not exist yet. This is - * what makes an 8.3 alias and its long form take one lock, and it stays a spelling - the key - * above is the identity, and callers that compare locks compare that. + * The identity a lock is keyed on: the device and inode of the nearest directory that actually + * exists, plus the components below it that do not exist yet. An 8.3 alias and its long form, a + * link and its target, and a case variant on a case-insensitive volume all arrive at the same + * identity, whether or not the leaf has been created. That is the case a canonical spelling + * cannot serve: a leaf that does not exist yet has no canonical spelling, which is what the + * `realpath: false` in `acquireFileLock` is there for. And nothing here rewrites a caller's + * path - the key is for comparing locks, never for opening files - so the strings callers hand + * to unlink and readFile stay the strings those callers built. */ -export function canonicalLockPath(filePath: string): string { +export function canonicalLockKey(filePath: string): string { const absolute = path.resolve(filePath) - if (statIdentity(absolute) !== null) { - try { - return fs.realpathSync.native(absolute) - } catch { - return absolute - } - } const { directory, relative } = nearestExistingDirectory(absolute) - let canonicalDirectory = directory - try { - canonicalDirectory = fs.realpathSync.native(directory) - } catch { - canonicalDirectory = directory + const identity = statIdentity(directory) + if (identity === null) { + // Not even a root of the path exists yet, so there is no identity to key on and the + // resolved path is the one thing every caller of it computes alike. + return absolute } - return relative.length === 0 ? canonicalDirectory : path.join(canonicalDirectory, ...relative) + return identity + ":" + relative.join("/") +} + +/** + * Where the artefact lives: beside the file in the caller's own spelling. The key above is the + * identity, and the artefact stays a path, because `safeWriteJson` and the interruption tests + * address `.lock` directly - moving it is what made a green suite red, so the location is + * pinned by a test rather than left to taste. + */ +function lockArtefactPath(absolutePath: string): string { + return absolutePath + ".lock" } export async function acquireFileLock(filePath: string, options?: FileLockOptions): Promise<() => Promise> { - const absoluteFilePath = canonicalLockPath(filePath) + const absoluteFilePath = path.resolve(filePath) + const lockfilePath = lockArtefactPath(absoluteFilePath) if (options?.waitForStaleLock) tracePersistenceEvent("json-lock-wait", { filePath: absoluteFilePath }) try { - return await lockfile.lock(absoluteFilePath, { + return await lockfile.lock(canonicalLockKey(absoluteFilePath), { + lockfilePath, stale: LOCK_STALE_MS, update: 10000, // Update mtime every 10 seconds to prevent staleness if operation is long realpath: false, // the file may not exist yet, which is acceptable @@ -124,7 +133,7 @@ export async function withFileLock( operation: (absoluteFilePath: string) => Promise, options?: FileLockOptions, ): Promise { - const absoluteFilePath = canonicalLockPath(filePath) + const absoluteFilePath = path.resolve(filePath) const releaseLock = await acquireFileLock(absoluteFilePath, options) let result: T From 812ba359b624ccc86264b35b27369de7e40c2e00 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 14:25:00 +0800 Subject: [PATCH 03/10] test(utils): decide cross-spelling exclusion with the real library The mocked suite keys its lock table on the string acquireFileLock hands the library and never looks at lockfilePath, so a change that gave two spellings different on-disk artefacts would have left it green. Delegate the mock to the real proper-lockfile for one suite and assert, for a junction and for an 8.3 alias, that the two spellings resolve to one artefact inode and that the second acquire rejects ELOCKED while the first holds it. Placing the artefact by the caller's spelling reddens exactly those two rows and leaves the key-equality rows green, which is the coverage the review asked for. Create each suite's temporary root in beforeAll and remove it in afterAll: the roots were made during collection and survived every run of the file, including runs where the alias rows were skipped. --- src/utils/__tests__/fileLock.test.ts | 166 ++++++++++++++++++++++----- 1 file changed, 136 insertions(+), 30 deletions(-) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index d2ec7dfbdc..58e1dbad2c 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -46,8 +46,8 @@ describe("file lock protocol", () => { * directory name and skips with a reason where generation is disabled. Asserting alias * unification on a volume with generation off would produce a green that cannot fail, which is * worse than a skip that says what it did. Locally the probe found a short name (A-REAL~1 for a - * 47-character name) and the alias case below executed and passed; the linux leg does not - * execute it. The name is deliberately not passed as a dir argument: dir /x lists that + * 47-character name) and the alias cases below executed and passed; the linux leg does not + * execute them. The name is deliberately not passed as a dir argument: dir /x lists that * directory's contents, whose header line is the only line containing the name, which reads as * generation being off when it is not. */ @@ -69,20 +69,63 @@ function shortNamesGenerated(directory: string): boolean { } } +/** + * The 8.3 alias of a path, read from the dir /x listing of its directory. Used by both the + * key-equality suite and the real-lock suite below, so it lives at module scope: a volume with + * generation off returns the long path unchanged, which is why every caller gates on + * shortNamesGenerated first. + */ +function aliasOf(longPath: string): string { + const directory = path.dirname(longPath) + const listing = spawnSync("cmd", ["/c", "dir", "/x"], { cwd: directory, encoding: "utf8" }) as unknown as { + stdout?: string + } + const line = + String(listing.stdout ?? "") + .split(/\r?\n/) + .find((entry) => entry.includes(path.basename(longPath))) ?? "" + const fields = line.split(/\s+/).filter((field) => field.length > 0) + const short = fields.find((field) => /~\d/.test(field)) + return short ? path.join(directory, short) : longPath +} + +/** + * A temporary root for one suite, created when that suite runs and removed when it ends, + * including when a test inside it fails. Creating the root in the describe body left one root + * and its fixtures on disk for every collection of this file, including runs where the suite's + * tests were skipped. + */ +function useTemporaryRoot(prefix: string): () => string { + let root = "" + beforeAll(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), prefix)) + }) + afterAll(() => { + if (root) { + fs.rmSync(root, { recursive: true, force: true }) + } + }) + return () => root +} + describe("one lock per file, not one lock per spelling", () => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), "fileLock-key-")) - const generatesShortNames = process.platform === "win32" && shortNamesGenerated(root) + const root = useTemporaryRoot("fileLock-key-") + let generatesShortNames = false // proper-lockfile is mocked, so exclusivity here is a test-side lock table keyed by the exact // key acquireFileLock asks for: a key already held rejects with ELOCKED, and immediate: true // means retries 0. The property under test is whether two spellings of one file arrive as // one key or as two, which is exactly what that table can decide. const held: string[] = [] + beforeAll(() => { + generatesShortNames = process.platform === "win32" && shortNamesGenerated(root()) + }) beforeEach(() => { held.length = 0 vi.mocked(lockfile.lock).mockImplementation(async (target: unknown) => { // The table keys on the key acquireFileLock hands the library: the file's identity, not // either spelling. Two spellings of one file therefore arrive as one key, which is what - // this table can decide; the artefact location is asserted separately. + // this table can decide; whether they also land on one on-disk artefact is asserted + // separately, against the real library. const key = String(target) if (held.indexOf(key) >= 0) { throw Object.assign(new Error("ELOCKED: " + key), { code: "ELOCKED" }) @@ -96,22 +139,17 @@ describe("one lock per file, not one lock per spelling", () => { } }) }) - const aliasOf = (longPath: string): string => { - const directory = path.dirname(longPath) - const listing = spawnSync("cmd", ["/c", "dir", "/x"], { cwd: directory, encoding: "utf8" }) as unknown as { - stdout?: string - } - const line = - String(listing.stdout ?? "") - .split(/\r?\n/) - .find((entry) => entry.includes(path.basename(longPath))) ?? "" - const fields = line.split(/\s+/).filter((field) => field.length > 0) - const short = fields.find((field) => /~\d/.test(field)) - return short ? path.join(directory, short) : longPath - } - const itAlias = generatesShortNames ? it : it.skip + // The volume probe runs in beforeAll, so the gate is decided when the suite runs rather than + // when it is collected, and the alias tests skip with a reason instead of disappearing. + const itAlias = (name: string, fn: () => Promise) => + it(name, async (context) => { + if (!generatesShortNames) { + return context.skip() + } + await fn() + }) itAlias("takes one lock for an 8.3 alias and its long form, not a second one", async () => { - const longDir = path.join(root, "a-really-long-directory-name-for-alias-lock-testing") + const longDir = path.join(root(), "a-really-long-directory-name-for-alias-lock-testing") fs.mkdirSync(longDir, { recursive: true }) const file = path.join(longDir, "history.json") fs.writeFileSync(file, "{}", "utf8") @@ -124,7 +162,7 @@ describe("one lock per file, not one lock per spelling", () => { await releaseAlias() }) itAlias("hands each caller back its own spelling rather than the canonicalised one", async () => { - const longDir = path.join(root, "a-really-long-directory-name-for-spelling-control") + const longDir = path.join(root(), "a-really-long-directory-name-for-spelling-control") fs.mkdirSync(longDir, { recursive: true }) const file = path.join(longDir, "history.json") fs.writeFileSync(file, "{}", "utf8") @@ -144,8 +182,8 @@ describe("one lock per file, not one lock per spelling", () => { expect(seen[0]).not.toBe(seen[1]) }) it("takes separate locks for two different files in one directory", async () => { - const a = path.join(root, "a.json") - const b = path.join(root, "b.json") + const a = path.join(root(), "a.json") + const b = path.join(root(), "b.json") fs.writeFileSync(a, "{}", "utf8") fs.writeFileSync(b, "{}", "utf8") await acquireFileLock(a, { immediate: true }) @@ -153,8 +191,8 @@ describe("one lock per file, not one lock per spelling", () => { expect(held).toHaveLength(2) }) it("takes one lock for one not-yet-existing path and a different one for another name", async () => { - const missingA = path.join(root, "missing-a.json") - const missingB = path.join(root, "missing-b.json") + const missingA = path.join(root(), "missing-a.json") + const missingB = path.join(root(), "missing-b.json") await acquireFileLock(missingA, { immediate: true }) await expect(acquireFileLock(missingA, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) await acquireFileLock(missingB, { immediate: true }) @@ -169,11 +207,15 @@ describe("one lock per file, not one lock per spelling", () => { * fails rather than quietly disappearing. */ describe("two spellings of one referent share one key", () => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), "fileLock-link-")) - const real = path.join(root, "real") - const link = path.join(root, "link") - fs.mkdirSync(real, { recursive: true }) - fs.symlinkSync(real, link, process.platform === "win32" ? "junction" : "dir") + const root = useTemporaryRoot("fileLock-link-") + let real = "" + let link = "" + beforeAll(() => { + real = path.join(root(), "real") + link = path.join(root(), "link") + fs.mkdirSync(real, { recursive: true }) + fs.symlinkSync(real, link, process.platform === "win32" ? "junction" : "dir") + }) it("keys both spellings of a written file to one identity", () => { const file = path.join(real, "history.json") @@ -199,3 +241,67 @@ describe("two spellings of one referent share one key", () => { expect(seen[1]).toBe(path.resolve(path.join(real, "history.json"))) }) }) + +/** + * The mocked suites above decide key equality and nothing else: the mock keys on the string + * acquireFileLock hands it and never looks at lockfilePath, so a change that gave two spellings + * two different on-disk artefacts would have left them green. These delegate the mock to the + * real library, the way the McpHub settings-creation integration spec does, so the outcome is + * decided by the artefact the filesystem resolves - which is the mutual-exclusion claim the + * alias-keyed fix actually makes. + */ +describe("two spellings contend with the real library", () => { + const root = useTemporaryRoot("fileLock-real-") + let generatesShortNames = false + let actualLock: typeof lockfile.lock + beforeAll(async () => { + actualLock = (await vi.importActual("proper-lockfile")).lock + generatesShortNames = process.platform === "win32" && shortNamesGenerated(root()) + }) + beforeEach(() => { + vi.mocked(lockfile.lock).mockImplementation((target, options) => actualLock(target, options)) + }) + + // Both spellings name one directory once the filesystem resolves them, so the artefacts are + // one directory and the library refuses the second caller. Asserting that the artefacts + // share an inode is what pins this down: a placement keyed on the spelling would land two + // directories here, the second acquire would succeed, and the suite above would stay green. + async function expectOneLockAcrossSpellings(firstPath: string, secondPath: string): Promise { + fs.mkdirSync(path.dirname(firstPath), { recursive: true }) + fs.writeFileSync(firstPath, "{}", "utf8") + const release = await acquireFileLock(firstPath, { immediate: true }) + try { + expect(fs.existsSync(secondPath + ".lock")).toBe(true) + expect(fs.statSync(secondPath + ".lock").ino).toBe(fs.statSync(firstPath + ".lock").ino) + await expect(acquireFileLock(secondPath, { immediate: true })).rejects.toMatchObject({ + code: "ELOCKED", + }) + } finally { + await release() + } + const released = await acquireFileLock(secondPath, { immediate: true }) + await released() + expect(fs.existsSync(firstPath + ".lock")).toBe(false) + } + + it("blocks the other spelling of a junction path while one holds the real lock", async () => { + const real = path.join(root(), "real") + const link = path.join(root(), "link") + fs.mkdirSync(real, { recursive: true }) + fs.symlinkSync(real, link, process.platform === "win32" ? "junction" : "dir") + await expectOneLockAcrossSpellings(path.join(link, "history.json"), path.join(real, "history.json")) + }) + + it("blocks the other spelling of an 8.3 alias while one holds the real lock", async (context) => { + if (!generatesShortNames) { + return context.skip() + } + const longDir = path.join(root(), "a-really-long-directory-name-for-real-lock-alias-testing") + fs.mkdirSync(longDir, { recursive: true }) + const file = path.join(longDir, "history.json") + fs.writeFileSync(file, "{}", "utf8") + const aliasFile = path.join(aliasOf(longDir), "history.json") + expect(fs.statSync(aliasFile).ino).toBe(fs.statSync(file).ino) + await expectOneLockAcrossSpellings(aliasFile, file) + }) +}) From 20343f818a7af671c285e77818c7c7c27ff9bd2d Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 15:12:59 +0800 Subject: [PATCH 04/10] fix(utils): read the lock identity in bigint so large inodes stay distinct fs.statSync hands back dev and ino as plain numbers unless BigInts are asked for, and above 2^53 an inode stops being a distinct number: 17451448568457636 and 17451448568457637 are adjacent integers and one double. Stringifying that number for the lock key therefore loses information silently and can key two neighbouring directories alike. Read the identity with { bigint: true }, which is lossless, and add a regression row whose statSync double answers plain stats with rounded numbers and BigInts only when asked - a double that always returned BigInts would be green before the fix. Co-Authored-By: Claude --- src/utils/__tests__/fileLock.test.ts | 74 ++++++++++++++++++++++++++++ src/utils/fileLock.ts | 5 +- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index 58e1dbad2c..6f3d2fb432 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -9,6 +9,27 @@ import { acquireFileLock, canonicalLockKey, LOCK_STALE_MS, withFileLock } from " vi.mock("proper-lockfile", () => ({ lock: vi.fn() })) +type StatSyncOverride = ( + target: string, + options?: { bigint?: boolean }, +) => { dev: number | bigint; ino: number | bigint } + +/** + * A delegating filesystem double, armed only by the inode-precision suite below. Everything else + * in this file keeps using the real filesystem, because the wrapper forwards to the actual module + * whenever no override is installed. + */ +let statSyncOverride: StatSyncOverride | null = null + +vi.mock("fs", async (importOriginal) => { + const actual = await importOriginal() + const statSync = ((target: string, options?: unknown) => + statSyncOverride === null + ? (actual.statSync as (path: string, opts?: unknown) => unknown)(target, options) + : statSyncOverride(target, options as { bigint?: boolean } | undefined)) as typeof actual.statSync + return { ...actual, statSync } +}) + describe("file lock protocol", () => { beforeEach(() => vi.clearAllMocks()) @@ -305,3 +326,56 @@ describe("two spellings contend with the real library", () => { await expectOneLockAcrossSpellings(aliasFile, file) }) }) + +/** + * The identity is a string built from device and inode, and node hands those over as plain numbers + * unless asked otherwise. Above 2^53 an inode is no longer a distinct number - 17451448568457636 + * and 17451448568457637 are adjacent integers and one double - so the identity has to be read with + * `{ bigint: true }` or two neighbouring directories collapse into one key and one lock. + * + * The double is the row: it answers a plain stat with **rounded numbers**, the way node does, and + * hands back BigInts only when the caller asks for them. A double that returned BigInts + * unconditionally would read green before the fix and prove nothing about the numeric path. + */ +describe("identity keeps inodes above 2^53 distinct", () => { + const inodeLow = 17451448568457636n + const inodeHigh = 17451448568457637n + const dev = 7n + // Two existing directories one inode apart, each holding a file of the same name, so the only + // thing that can separate the two keys is the inode the identity is built from. These paths are + // synthetic - the fake statSync answers for them - so nothing is created on disk. + const precisionRoot = path.resolve("zoo-identity-precision") + const directoryLow = path.join(precisionRoot, "a") + const directoryHigh = path.join(precisionRoot, "b") + + let statCalls: Array<{ target: string; bigintRequested: boolean }> = [] + beforeEach(() => { + statCalls = [] + statSyncOverride = (target, options) => { + const bigintRequested = options?.bigint === true + statCalls.push({ target, bigintRequested }) + const ino = target === directoryLow ? inodeLow : target === directoryHigh ? inodeHigh : 1n + // Fidelity: BigInts only when they were asked for, rounded numbers otherwise. + return bigintRequested ? { dev, ino } : { dev: Number(dev), ino: Number(ino) } + } + }) + afterEach(() => { + statSyncOverride = null + }) + + it("pairs the adjacent inodes named by the precision loss", () => { + expect(Number(inodeLow)).toBe(Number(inodeHigh)) + expect(String(inodeLow)).not.toBe(String(inodeHigh)) + }) + + it("keys two directories one inode apart above 2^53 by different identities", () => { + const low = canonicalLockKey(path.join(directoryLow, "history.json")) + const high = canonicalLockKey(path.join(directoryHigh, "history.json")) + expect(low).not.toBe(high) + }) + + it("asks the filesystem for the identity in bigint form", () => { + canonicalLockKey(path.join(directoryLow, "history.json")) + expect(statCalls.some((call) => call.bigintRequested)).toBe(true) + }) +}) diff --git a/src/utils/fileLock.ts b/src/utils/fileLock.ts index 912f739fa3..0ea669e157 100644 --- a/src/utils/fileLock.ts +++ b/src/utils/fileLock.ts @@ -29,7 +29,10 @@ export interface FileLockOptions { */ function statIdentity(target: string): string | null { try { - const stats = fs.statSync(target) + // BigInt stats, because the identity is a string and a plain number is not lossless: two + // adjacent inodes above 2^53 are the same Number, so the numeric form can key two + // neighbouring directories alike and let them share a lock nobody asked for. + const stats = fs.statSync(target, { bigint: true }) return String(stats.dev) + ":" + String(stats.ino) } catch { return null From 2d75cda5ec99f543789cdc63d9a2cfc05b1ef2d8 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 20:28:00 +0800 Subject: [PATCH 05/10] docs(utils): state that the lock identity converges aliasing above the leaf and not at it --- src/utils/fileLock.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/utils/fileLock.ts b/src/utils/fileLock.ts index 0ea669e157..821518f214 100644 --- a/src/utils/fileLock.ts +++ b/src/utils/fileLock.ts @@ -58,13 +58,18 @@ function nearestExistingDirectory(absolutePath: string): { directory: string; re /** * The identity a lock is keyed on: the device and inode of the nearest directory that actually - * exists, plus the components below it that do not exist yet. An 8.3 alias and its long form, a - * link and its target, and a case variant on a case-insensitive volume all arrive at the same - * identity, whether or not the leaf has been created. That is the case a canonical spelling - * cannot serve: a leaf that does not exist yet has no canonical spelling, which is what the - * `realpath: false` in `acquireFileLock` is there for. And nothing here rewrites a caller's - * path - the key is for comparing locks, never for opening files - so the strings callers hand - * to unlink and readFile stay the strings those callers built. + * exists, plus the components below it that do not exist yet. Aliasing at or above that directory + * converges - an 8.3 alias of it, a link to it, and a case variant of it on a case-insensitive + * volume all arrive at the same identity, because the identity is the directory's and only the + * surviving components are compared. Aliasing OF THE FINAL COMPONENT does not: those components + * are compared as the strings the caller wrote, so an 8.3 alias of the file itself, or a case + * variant of it, arrives as a second key - and lockArtefactPath names the artefact by + * concatenating that same spelling, so it arrives as a second artefact beside it, and two writers + * that believe they hold the same lock do not. A canonical spelling cannot serve the general + * case here: a leaf that does not exist yet has no canonical spelling, which is what the + * `realpath: false` in `acquireFileLock` is there for. And nothing here rewrites a caller's path + * - the key is for comparing locks, never for opening files - so the strings callers hand to + * unlink and readFile stay the strings those callers built. */ export function canonicalLockKey(filePath: string): string { const absolute = path.resolve(filePath) From ff5fbc58709b4beef4182c9139095e3aac62772a Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 20:31:13 +0800 Subject: [PATCH 06/10] fix(utils): name the lock artefact by the spelling the volume holds for the leaf, and contend the two spellings against the real library --- src/utils/__tests__/fileLock.test.ts | 31 ++++++++++++++++++++++++++++ src/utils/fileLock.ts | 14 ++++++++----- src/utils/lockLeafSpelling.ts | 29 ++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 src/utils/lockLeafSpelling.ts diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index 6f3d2fb432..dd3c8980ed 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -325,6 +325,37 @@ describe("two spellings contend with the real library", () => { expect(fs.statSync(aliasFile).ino).toBe(fs.statSync(file).ino) await expectOneLockAcrossSpellings(aliasFile, file) }) + + // The two rows above alias a directory: once the filesystem resolves the two spellings the + // artefacts are one directory no matter what the artefact is named by. These two spellings differ + // in the final component only, so the spelling the artefact name is built from is what decides + // whether the callers contend. A placement that concatenates the caller's own leaf spelling lands + // two artefacts here, lets the second caller through, and leaves every mocked suite above green - + // which is why this one delegates to the real library and asserts the artefact directory too. + it("blocks the other spelling of an 8.3 alias of the leaf while one holds the real lock", async (context) => { + if (!generatesShortNames) { + return context.skip() + } + const directory = path.join(root(), "leaf-alias-lock") + fs.mkdirSync(directory, { recursive: true }) + const longLeaf = path.join(directory, "task-history-leaf-for-alias-lock-testing.json") + fs.writeFileSync(longLeaf, "{}", "utf8") + const aliasLeaf = aliasOf(longLeaf) + expect(aliasLeaf).not.toBe(longLeaf) + expect(fs.statSync(aliasLeaf).ino).toBe(fs.statSync(longLeaf).ino) + const release = await acquireFileLock(aliasLeaf, { immediate: true }) + try { + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([ + "task-history-leaf-for-alias-lock-testing.json.lock", + ]) + await expect(acquireFileLock(longLeaf, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) + } finally { + await release() + } + const released = await acquireFileLock(longLeaf, { immediate: true }) + await released() + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([]) + }) }) /** diff --git a/src/utils/fileLock.ts b/src/utils/fileLock.ts index 821518f214..cdd0ad1b28 100644 --- a/src/utils/fileLock.ts +++ b/src/utils/fileLock.ts @@ -2,6 +2,7 @@ import * as fs from "fs" import * as path from "path" import { tracePersistenceEvent } from "../core/task-persistence/persistenceTrace" import * as lockfile from "proper-lockfile" +import { canonicalLeafSpelling } from "./lockLeafSpelling" /** * Shared staleness window for per-file advisory locks. This module owns the @@ -84,13 +85,16 @@ export function canonicalLockKey(filePath: string): string { } /** - * Where the artefact lives: beside the file in the caller's own spelling. The key above is the - * identity, and the artefact stays a path, because `safeWriteJson` and the interruption tests - * address `.lock` directly - moving it is what made a green suite red, so the location is - * pinned by a test rather than left to taste. + * Where the artefact lives: beside the file, named by the spelling the volume holds for that leaf, + * so two spellings of one file contend with one another instead of each taking its own lock. The + * key above is the identity, and the artefact stays a path, because `safeWriteJson` and the + * interruption tests address `.lock` directly - moving it is what made a green suite red, so + * the location is pinned by a test rather than left to taste. Only the final component is + * re-spelled: a caller that reaches the file through a junction or an 8.3 alias of a DIRECTORY + * still gets the artefact inside that directory, because the re-spelling never moves the file. */ function lockArtefactPath(absolutePath: string): string { - return absolutePath + ".lock" + return canonicalLeafSpelling(absolutePath) + ".lock" } export async function acquireFileLock(filePath: string, options?: FileLockOptions): Promise<() => Promise> { diff --git a/src/utils/lockLeafSpelling.ts b/src/utils/lockLeafSpelling.ts new file mode 100644 index 0000000000..c132e823c5 --- /dev/null +++ b/src/utils/lockLeafSpelling.ts @@ -0,0 +1,29 @@ +import * as fs from "fs" +import * as path from "path" + +/** + * The spelling an artefact should be named by when one file is spelled more than one way. An 8.3 + * alias, a case variant on a case-insensitive volume, and the long form all name one directory + * entry, and only one of those spellings can be the name of the artefact if two writers are to + * contend at all. The native realpath is the call that answers this: it reports the spelling the + * volume itself holds, which plain `fs.realpathSync` does not - on a Windows volume with 8.3 + * generation on, `fs.realpathSync` hands an alias straight back, so it converges nothing, while + * the native call returns the long form for both the alias and a case variant of it. Only the + * final component is re-spelled, never the directory: `fileLock` deliberately passes + * `realpath: false` to the lock library because the file may not exist yet, and the artefact has + * to stay beside the file the caller named. Where the volume has no spelling to report - the leaf + * is not created yet, or it is a link whose target is gone - the call throws and the caller's own + * spelling is returned unchanged, which is the behaviour today. + */ +export function canonicalLeafSpelling(absolutePath: string): string { + try { + const volumeSpelling = path.basename(fs.realpathSync.native(absolutePath)) + return volumeSpelling === path.basename(absolutePath) + ? absolutePath + : path.join(path.dirname(absolutePath), volumeSpelling) + } catch { + // Nothing to converge: a leaf that does not exist yet has no volume spelling, and a dangling + // link has no target to name. Refusing the write for either would be worse than today. + return absolutePath + } +} From 7feb8a12112bcddeeedf2315a08cca493acdd977 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 20:49:13 +0800 Subject: [PATCH 07/10] test(utils): cover the link-named leaf on the linux arm, where it is the only spelling the volume can hand back --- src/utils/__tests__/fileLock.test.ts | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index dd3c8980ed..51bad9eecf 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -356,6 +356,65 @@ describe("two spellings contend with the real library", () => { await released() expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([]) }) + + // The rows above alias a directory, and on ext4 a case variant is a different file and an 8.3 alias + // does not exist, so the only way the final component is spelled more than one way on the linux arm + // is a leaf that is itself a link. These two rows are the linux arm's version of the two rows above, + // gated the other way round: they run where the 8.3 rows skip and skip where they run. + const itPosixLeaf = (name: string, fn: () => Promise) => + it(name, async (context) => { + if (process.platform === "win32") { + return context.skip() + } + await fn() + }) + + itPosixLeaf( + "blocks the target of a link while the link holds the real lock, on one artefact named by the target", + async () => { + const directory = path.join(root(), "link-leaf-lock") + fs.mkdirSync(directory, { recursive: true }) + const target = path.join(directory, "history.json") + fs.writeFileSync(target, "{}", "utf8") + const link = path.join(directory, "history-link.json") + fs.symlinkSync("history.json", link) + expect(fs.statSync(link).ino).toBe(fs.statSync(target).ino) + const release = await acquireFileLock(link, { immediate: true }) + try { + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([ + "history.json.lock", + ]) + await expect(acquireFileLock(target, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) + } finally { + await release() + } + const released = await acquireFileLock(target, { immediate: true }) + await released() + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([]) + }, + ) + + itPosixLeaf("keeps a link's artefact in the caller's directory when its target lives elsewhere", async () => { + const directory = path.join(root(), "far-link-lock") + const elsewhere = path.join(root(), "far-link-target") + fs.mkdirSync(directory, { recursive: true }) + fs.mkdirSync(elsewhere, { recursive: true }) + const target = path.join(elsewhere, "history.json") + fs.writeFileSync(target, "{}", "utf8") + const link = path.join(directory, "history-link.json") + fs.symlinkSync(target, link) + const release = await acquireFileLock(link, { immediate: true }) + try { + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual(["history.json.lock"]) + expect(fs.readdirSync(elsewhere).filter((name) => name.endsWith(".lock"))).toEqual([]) + const besideTarget = await acquireFileLock(target, { immediate: true }) + await besideTarget() + } finally { + await release() + } + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([]) + expect(fs.readdirSync(elsewhere).filter((name) => name.endsWith(".lock"))).toEqual([]) + }) }) /** From 685ddda9a13310f79590c990cae08033d84b300a Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 21:34:52 +0800 Subject: [PATCH 08/10] test(utils): pin the leaf substitution on a stubbed volume spelling, so it is proved where no volume would hand back two spellings --- src/utils/__tests__/fileLock.test.ts | 39 ++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index 51bad9eecf..33ba1cb0b9 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -182,6 +182,45 @@ describe("one lock per file, not one lock per spelling", () => { expect(canonicalLockKey(aliasFile)).toBe(canonicalLockKey(file)) await releaseAlias() }) + + // The portable half of the leaf-spelling coverage. The 8.3 row above needs a volume that hands + // back two spellings of one entry and the real-library link rows below need a filesystem that + // lets a leaf be a link; this one needs neither. It substitutes the spelling the volume reports + // and asserts the artefact acquireFileLock asks the library to create, so dropping the + // substitution fails here on every platform, not only where a second spelling exists by luck. + it("names the artefact by the spelling the volume reports for the leaf, not by the caller's", async () => { + const directory = path.join(root(), "substituted-leaf-lock") + fs.mkdirSync(directory, { recursive: true }) + const callerPath = path.join(directory, "history-item.json") + const volumePath = path.join(directory, "HISTORY-ITEM.JSON") + fs.writeFileSync(callerPath, "{}", "utf8") + // fs's namespace types these properties read-only, and the module double forwards to the real + // realpathSync object, so the substitution is installed by reading the callable through a + // mutable view of it and restoring the original function in the finally below. + const realpathModule: { native: (target: string) => string } = fs.realpathSync + const actualNative = realpathModule.native + try { + realpathModule.native = (target: string) => + target === path.resolve(callerPath) ? volumePath : actualNative(target) + const release = await acquireFileLock(callerPath, { immediate: true }) + try { + const call = vi.mocked(lockfile.lock).mock.calls.at(-1) + expect(call?.[1]?.lockfilePath).toBe(path.join(directory, "HISTORY-ITEM.JSON.lock")) + expect(String(call?.[0])).toBe(canonicalLockKey(path.resolve(callerPath))) + expect(fs.existsSync(path.join(directory, "HISTORY-ITEM.JSON.lock"))).toBe(false) + expect(fs.existsSync(path.join(directory, "history-item.json.lock"))).toBe(false) + } finally { + await release() + } + } finally { + realpathModule.native = actualNative + } + const after = await acquireFileLock(callerPath, { immediate: true }) + await after() + expect(vi.mocked(lockfile.lock).mock.calls.at(-1)?.[1]?.lockfilePath).toBe( + path.join(directory, "history-item.json.lock"), + ) + }) itAlias("hands each caller back its own spelling rather than the canonicalised one", async () => { const longDir = path.join(root(), "a-really-long-directory-name-for-spelling-control") fs.mkdirSync(longDir, { recursive: true }) From d76bbfd4f9f7a81cf517a9727c7cbc3bf5926677 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Tue, 10 Nov 2026 00:48:02 +0800 Subject: [PATCH 09/10] utils: pin the shared artefact, the queued writer, and the release order Three rows in src/utils/__tests__/fileLock.test.ts, run against the real library: - contends an unrelated sibling named like the link target against the link's lock - keeps a waiting writer at the caller-named artefact across a leaf-link replacement, and merges into the referent that answers when it proceeds - refuses a second key on a held artefact, and a late release from the writer that let go plus two identity rows: keys whose digits concatenate alike, and the no-ancestor fallback. Verified on Linux node v22.23.4: pristine 19 passed | 4 skipped (23); with the leaf-spelling call removed 5 failed; with the artefact composed from the resolved target 7 failed; restored green. Test-only commit: no executable line changes, so the priced 33 counted lines at 5319fe91fb is unchanged and no local Stryker preflight was run - this box cannot spawn .bin/stryker. --- src/utils/__tests__/fileLock.test.ts | 178 +++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index 33ba1cb0b9..bd54acfe41 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -454,6 +454,143 @@ describe("two spellings contend with the real library", () => { expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([]) expect(fs.readdirSync(elsewhere).filter((name) => name.endsWith(".lock"))).toEqual([]) }) + + // The row above proves the artefact lands in the caller's directory. It does not prove what + // happens when the caller's own directory already holds a file with the target's name, and that + // collision is not hypothetical - it is what the lock does. The sibling is a different inode with + // its own content and its own canonical key, and it still contends on the one artefact name the + // volume spelling hands both of them. + itPosixLeaf("contends an unrelated sibling named like the link target against the link's lock", async () => { + const directory = path.join(root(), "sibling-collision") + const elsewhere = path.join(root(), "sibling-target") + fs.mkdirSync(directory, { recursive: true }) + fs.mkdirSync(elsewhere, { recursive: true }) + const target = path.join(elsewhere, "history.json") + fs.writeFileSync(target, JSON.stringify({ writer: "target" }), "utf8") + const link = path.join(directory, "history-link.json") + fs.symlinkSync(target, link) + const sibling = path.join(directory, "history.json") + fs.writeFileSync(sibling, JSON.stringify({ writer: "sibling" }), "utf8") + expect(fs.realpathSync.native(link)).toBe(fs.realpathSync.native(target)) + expect(fs.statSync(sibling).ino).not.toBe(fs.statSync(link).ino) + expect(canonicalLockKey(link)).not.toBe(canonicalLockKey(sibling)) + const release = await acquireFileLock(link, { immediate: true }) + try { + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual(["history.json.lock"]) + await expect(acquireFileLock(sibling, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) + } finally { + await release() + } + const afterRelease = await acquireFileLock(sibling, { immediate: true }) + await afterRelease() + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([]) + expect(fs.readdirSync(elsewhere).filter((name) => name.endsWith(".lock"))).toEqual([]) + }) + + // A leaf link replaced while a second writer queues is the case the persistence-integrity check + // asked for. The contract it has to assert is the one the filesystem offers: convergence happens + // inside one directory, so the artefact directory does not follow the target. Both artefacts are + // named here, the queue is observed at the caller-named one, and the merged content is read back + // from both coexisting targets - constructed as two live files with two live caller paths, and the + // replacement is an atomic rename of a freshly created link over the old one, so no recycled inode + // is load-bearing. + itPosixLeaf( + "keeps a waiting writer at the caller-named artefact across a leaf-link replacement, and merges into the referent that answers when it proceeds", + async () => { + const readMerged = (target: string): { merge: string; writer?: string } => + JSON.parse(fs.readFileSync(target, "utf8")) as { merge: string; writer?: string } + const callerDir = path.join(root(), "retarget-caller") + const targetDir = path.join(root(), "retarget-target") + fs.mkdirSync(callerDir, { recursive: true }) + fs.mkdirSync(targetDir, { recursive: true }) + const firstTarget = path.join(targetDir, "history.json") + fs.writeFileSync(firstTarget, JSON.stringify({ merge: "first" }), "utf8") + const secondTarget = path.join(callerDir, "history-content.json") + fs.writeFileSync(secondTarget, JSON.stringify({ merge: "second" }), "utf8") + const callerPath = path.join(callerDir, "history.json") + fs.symlinkSync(firstTarget, callerPath) + const callerArtefact = path.join(callerDir, "history.json.lock") + const targetArtefact = path.join(targetDir, "history.json.lock") + let releaseFirst: (() => Promise) | null = null + let releaseSecond: (() => Promise) | null = null + let waitingSeen = "not started" + try { + releaseFirst = await acquireFileLock(callerPath, { immediate: true }) + const firstBase = readMerged(callerPath) + fs.writeFileSync(callerPath, JSON.stringify({ ...firstBase, writer: "first" }), "utf8") + expect(fs.existsSync(callerArtefact)).toBe(true) + const throughTarget = await acquireFileLock(firstTarget, { immediate: true }) + expect(fs.existsSync(targetArtefact)).toBe(true) + await throughTarget() + const waitingWriter = acquireFileLock(callerPath) + waitingSeen = await Promise.race([ + waitingWriter.then(() => "settled while the first writer still held"), + new Promise((resolve) => { + setTimeout(() => resolve("still queued"), 300) + }), + ]) + const replacement = path.join(callerDir, "history.json.next") + fs.symlinkSync(secondTarget, replacement) + fs.renameSync(replacement, callerPath) + await releaseFirst() + releaseFirst = null + releaseSecond = await waitingWriter + expect(fs.readdirSync(callerDir).filter((name) => name.endsWith(".lock"))).toEqual([ + "history.json.lock", + ]) + const secondBase = readMerged(callerPath) + fs.writeFileSync(callerPath, JSON.stringify({ ...secondBase, writer: "second" }), "utf8") + } finally { + if (releaseFirst !== null) { + await releaseFirst() + } + if (releaseSecond !== null) { + await releaseSecond() + } + } + expect(waitingSeen).toBe("still queued") + expect(fs.existsSync(callerArtefact)).toBe(false) + expect(fs.existsSync(targetArtefact)).toBe(false) + expect(readMerged(firstTarget)).toEqual({ merge: "first", writer: "first" }) + expect(readMerged(secondTarget)).toEqual({ merge: "second", writer: "second" }) + }, + ) + + // The third pre-merge row asks about overlapping acquisitions and releases, and about one library key + // owning two different artefacts at once. Measured before asserting: an acquisition that arrives at an + // artefact another key already holds rejects ELOCKED - on this volume the artefact carries the lock, so + // a second key cannot fork a second lock onto it - and a release called a second time, after another + // writer has taken the artefact, rejects ERELEASED and leaves that writer's artefact where it is. Each + // writer then releases through the function it was handed, and neither directory is left holding one. + itPosixLeaf("refuses a second key on a held artefact, and a late release from the writer that let go", async () => { + const directory = path.join(root(), "release-order") + const elsewhere = path.join(root(), "release-target") + fs.mkdirSync(directory, { recursive: true }) + fs.mkdirSync(elsewhere, { recursive: true }) + const target = path.join(elsewhere, "history.json") + fs.writeFileSync(target, JSON.stringify({ writer: "target" }), "utf8") + const link = path.join(directory, "history-link.json") + fs.symlinkSync(target, link) + const sibling = path.join(directory, "history.json") + fs.writeFileSync(sibling, JSON.stringify({ writer: "sibling" }), "utf8") + expect(canonicalLockKey(link)).not.toBe(canonicalLockKey(sibling)) + const releaseLink = await acquireFileLock(link, { immediate: true }) + let releaseSibling: (() => Promise) | null = null + try { + await expect(acquireFileLock(sibling, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) + const queued = acquireFileLock(sibling) + await releaseLink() + releaseSibling = await queued + await expect(releaseLink()).rejects.toMatchObject({ code: "ERELEASED" }) + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual(["history.json.lock"]) + } finally { + if (releaseSibling !== null) { + await releaseSibling() + } + } + expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([]) + expect(fs.readdirSync(elsewhere).filter((name) => name.endsWith(".lock"))).toEqual([]) + }) }) /** @@ -507,4 +644,45 @@ describe("identity keeps inodes above 2^53 distinct", () => { canonicalLockKey(path.join(directoryLow, "history.json")) expect(statCalls.some((call) => call.bigintRequested)).toBe(true) }) + + // The identity and the surviving components are joined with a separator, and a separator is only + // decoration until two widths concatenate alike: device 1 inode 23 and device 12 inode 3 build the + // same digit run, and device 1 inode 2 followed by a file named 3history.json builds the same run as + // device 1 inode 23 followed by history.json. These paths are synthetic - the fake statSync answers + // for them - and the doubles return plain numbers for both spellings because the row is about widths, + // not about the bigint read the row above already pins, so the only thing that can keep the three + // keys apart is the separator the unit composes them with. + it("keeps identities whose digits concatenate alike apart", () => { + const directoryWide = path.join(precisionRoot, "wide") + const directoryWideConcat = path.join(precisionRoot, "wide-concat") + const directoryShift = path.join(precisionRoot, "shift") + statSyncOverride = (target) => { + const fabricated: Record = { + [directoryWide]: { dev: 1, ino: 23 }, + [directoryWideConcat]: { dev: 12, ino: 3 }, + [directoryShift]: { dev: 1, ino: 2 }, + } + return fabricated[path.resolve(target)] ?? { dev: 1, ino: 99 } + } + const wide = canonicalLockKey(path.join(directoryWide, "history.json")) + const sameConcat = canonicalLockKey(path.join(directoryWideConcat, "history.json")) + const shifted = canonicalLockKey(path.join(directoryShift, "3history.json")) + expect(wide).not.toBe(sameConcat) + expect(wide).not.toBe(shifted) + expect(sameConcat).not.toBe(shifted) + }) + + // Every ancestor missing is the fall-through at the end of nearestExistingDirectory, and that is the + // only way to reach the fallback that hands back the caller's own path: with no identity anywhere the + // key has to be the resolved path, because nothing on the filesystem has answered with an identity to + // build one from. Mutating that fall-through's return to an empty object is equivalent here - an + // undefined directory statSyncs to null and lands on the same fallback - so this row covers the line + // without killing that mutant, and says so rather than claiming a kill. + it("falls back to the caller's own path when no ancestor has an identity", () => { + statSyncOverride = () => { + throw Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" }) + } + const uncreated = path.resolve(path.join(precisionRoot, "uncreated", "nested", "history.json")) + expect(canonicalLockKey(uncreated)).toBe(uncreated) + }) }) From c90647c7a1699c3623943b1d145b8ac4653d667a Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Tue, 10 Nov 2026 04:09:36 +0800 Subject: [PATCH 10/10] test(utils): release the fileLock link lock from finally when a row fails before release The release-order row took the link lock outside its try/finally, so an assertion failure between acquisition and release left the renewal timer armed while teardown removed the lock directory, and the next stat in the same process raised ECOMPROMISED inside an unrelated row. Ownership is tracked before the acquisition and the link is released in finally unless the row already released it, which the row itself asserts with ERELEASED. --- src/utils/__tests__/fileLock.test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/utils/__tests__/fileLock.test.ts b/src/utils/__tests__/fileLock.test.ts index bd54acfe41..869f96c0e9 100644 --- a/src/utils/__tests__/fileLock.test.ts +++ b/src/utils/__tests__/fileLock.test.ts @@ -574,12 +574,19 @@ describe("two spellings contend with the real library", () => { const sibling = path.join(directory, "history.json") fs.writeFileSync(sibling, JSON.stringify({ writer: "sibling" }), "utf8") expect(canonicalLockKey(link)).not.toBe(canonicalLockKey(sibling)) - const releaseLink = await acquireFileLock(link, { immediate: true }) + // Ownership is tracked before anything is acquired: if the assertion below fails between + // taking the link lock and releasing it, the release still has to run, because teardown + // removes the lock directory and a renewal timer still armed on it raises ECOMPROMISED + // inside some later row. + let releaseLink: (() => Promise) | null = null + let linkReleased = false let releaseSibling: (() => Promise) | null = null try { + releaseLink = await acquireFileLock(link, { immediate: true }) await expect(acquireFileLock(sibling, { immediate: true })).rejects.toMatchObject({ code: "ELOCKED" }) const queued = acquireFileLock(sibling) await releaseLink() + linkReleased = true releaseSibling = await queued await expect(releaseLink()).rejects.toMatchObject({ code: "ERELEASED" }) expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual(["history.json.lock"]) @@ -587,6 +594,9 @@ describe("two spellings contend with the real library", () => { if (releaseSibling !== null) { await releaseSibling() } + if (releaseLink !== null && !linkReleased) { + await releaseLink() + } } expect(fs.readdirSync(directory).filter((name) => name.endsWith(".lock"))).toEqual([]) expect(fs.readdirSync(elsewhere).filter((name) => name.endsWith(".lock"))).toEqual([])