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. resolveLockKey therefore returns that identity for an existing path,
and the nearest existing directory's identity plus the lowercased remainder when the file
does not exist yet - a basename with no inode is the residual case no identity can unify.
canonicalLockPath gives the spelling a lock artefact is created under, so the alias case
resolves to one lock instead of two.

The 8.3 case is gated on 8.3 name generation (probed with dir /x on a freshly created long
name, because fsutil 8dot3name needs elevation and fails with Error 5 un-elevated), not on
the platform string, so a runner on a volume with generation off reports a skip rather than
a green that cannot fail. Locally it executed and passed; the linux unit leg does not run it.

Reported on #2356. No suppression count changed.

Author: easonLiangWorldedtech

src/utils/__tests__/fileLock.test.ts

@@ -1,5 +1,10 @@
+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, resolveLockKey, withFileLock } from "../fileLock"
 
 vi.mock("proper-lockfile", () => ({ lock: vi.fn() }))
 
@@ -31,4 +36,94 @@ describe("file lock protocol", () => {
 		).rejects.toBe(error)
 		expect(release).toHaveBeenCalledOnce()
 	})
+
+	/**
+	 * 8.3 short-name generation is a volume property, not a platform constant: 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. A case that asserted
+	 * alias unification on a volume with 8.3 generation off would produce a green that cannot
+	 * fail, which is worse than a skip that says what it did. Locally, on the volume this was
+	 * written on, dir /x returned 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.
+	 */
+	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)
+			const shortIndex = fields.findIndex((field) => /~\d/.test(field))
+			return shortIndex >= 0
+		} finally {
+			fs.rmSync(path.join(directory, probeName), { recursive: true, force: true })
+		}
+	}
+
+	describe("identity-backed lock key", () => {
+		const root = fs.mkdtempSync(path.join(os.tmpdir(), "fileLock-key-"))
+		const generatesShortNames = process.platform === "win32" && shortNamesGenerated(root)
+		const keyFor = (target: string): string => {
+			try {
+				return resolveLockKey(target)
+			} catch {
+				return "stat-failure"
+			}
+		}
+		const aliasOrSame = (longPath: string): string => {
+			// dir /x reports the short name beside the long one; %~sI is not used here because it
+			// echoes the quoted input when it is passed through cmd from a spawned argument array.
+			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
+		}
+		it("keys an existing file by device and inode", () => {
+			const file = path.join(root, "existing.json")
+			fs.writeFileSync(file, "{}", "utf8")
+			const stats = fs.statSync(file)
+			expect(keyFor(file)).toBe(String(stats.dev) + ":" + String(stats.ino))
+		})
+		it("gives two different files in one directory two different keys", () => {
+			const a = path.join(root, "a.json")
+			const b = path.join(root, "b.json")
+			fs.writeFileSync(a, "{}", "utf8")
+			fs.writeFileSync(b, "{}", "utf8")
+			expect(keyFor(a)).not.toBe(keyFor(b))
+		})
+		it("is stable for a file that does not exist yet, and differs by name", () => {
+			const missingA = path.join(root, "missing-a.json")
+			const missingB = path.join(root, "missing-b.json")
+			expect(keyFor(missingA)).toBe(keyFor(missingA))
+			expect(keyFor(missingA)).not.toBe(keyFor(missingB))
+		})
+		const itAlias = generatesShortNames ? it : it.skip
+		itAlias("takes one key and one canonical path for an 8.3 alias and its long form", () => {
+			// Gated on the property rather than on the platform string, and it ran here: dir /x
+			// generated a short name for a fresh long directory on this volume.
+			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 aliasDir = aliasOrSame(longDir)
+			expect(aliasDir).not.toBe(longDir)
+			const aliasFile = path.join(aliasDir, "history.json")
+			expect(fs.statSync(aliasFile).ino).toBe(fs.statSync(file).ino)
+			expect(keyFor(aliasFile)).toBe(keyFor(file))
+			expect(canonicalLockPath(aliasFile)).toBe(canonicalLockPath(file))
+		})
+	})
 })

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,85 @@ 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 }
+}
+
+/**
+ * Identity-backed lock key for a path.
+
+ * Two callers that name one file differently must take one lock. Canonicalizing by text
+ * cannot promise that: fs.realpath and fs.realpathSync return whatever spelling they are
+ * handed, and on NTFS only fs.realpathSync.native expands an 8.3 alias, while stat answers
+ * both spellings with the same dev+ino. So the key is that identity, plus the lowercase
+ * remainder below the nearest existing directory when the file itself does not exist yet -
+ * a basename with no inode is the residual case no identity can unify, and it is named here
+ * rather than presented as identity.
+ */
+export function resolveLockKey(filePath: string): string {
+	const absolute = path.resolve(filePath)
+	const identity = statIdentity(absolute)
+	if (identity !== null) {
+		return identity
+	}
+	const { directory, relative } = nearestExistingDirectory(absolute)
+	const directoryIdentity = statIdentity(directory)
+	if (directoryIdentity === null) {
+		// Nothing on the path exists yet, so text is the only key available; disclosed as text.
+		return "unresolved:" + absolute.toLowerCase()
+	}
+	return directoryIdentity + ":" + relative.join("/").toLowerCase()
+}
+
+/**
+ * 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<void>> {
-	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 +150,7 @@ export async function withFileLock<T>(
 	operation: (absoluteFilePath: string) => Promise<T>,
 	options?: FileLockOptions,
 ): Promise<T> {
-	const absoluteFilePath = path.resolve(filePath)
+	const absoluteFilePath = canonicalLockPath(filePath)
 	const releaseLock = await acquireFileLock(absoluteFilePath, options)
 
 	let result: T