diff --git a/.changeset/2106-windows-durable-casting.md b/.changeset/2106-windows-durable-casting.md new file mode 100644 index 000000000..2b99df228 --- /dev/null +++ b/.changeset/2106-windows-durable-casting.md @@ -0,0 +1,5 @@ +--- +"@bradygaster/squad-sdk": patch +--- + +Fix `squad init` and team casting failing on Windows with `EPERM: operation not permitted, fsync`. The durable casting registry fsynced directory handles after each rename, which Windows rejects; directory flushes are now skipped on `win32`, where the file-level fsync is the only flush available. Also fixed the casting writer lock crashing with `EPERM ... rename` instead of waiting when another process already holds it on Windows, which reports a rename onto an existing directory as `EPERM` rather than POSIX `ENOTEMPTY`/`EEXIST`. Lock timeouts now name the last publish error, and releasing the lock retries briefly when Windows transiently rejects the directory rename (e.g. antivirus holding a freshly written file), instead of leaving a recovery guard that blocks later casting writers. If Windows still blocks deleting the released lock copy, release succeeds and a later release sweeps the leftover out of `.squad/casting/`. diff --git a/packages/squad-sdk/src/casting/durable-registry.ts b/packages/squad-sdk/src/casting/durable-registry.ts index de9ee9fe9..339db9f6e 100644 --- a/packages/squad-sdk/src/casting/durable-registry.ts +++ b/packages/squad-sdk/src/casting/durable-registry.ts @@ -190,6 +190,10 @@ function readText(filePath: string): string | undefined { } function flushDirectory(directoryPath: string): void { + // Directory fsync is a POSIX durability step for rename. Windows rejects + // FlushFileBuffers on a directory handle with EPERM and has no equivalent; + // the file-level fsync before rename is the only flush available there. + if (process.platform === 'win32') return; const descriptor = openSync(directoryPath, 'r'); try { fsyncSync(descriptor); @@ -301,6 +305,76 @@ function ownerPath(lockDirectory: string): string { return path.join(lockDirectory, 'owner.json'); } +const WIN32_TRANSIENT_FS_CODES = ['EPERM', 'EACCES', 'EBUSY']; +const WIN32_RENAME_RETRY_MS = 1_000; + +// Windows fails a directory rename while another process (antivirus, indexer) +// briefly holds a handle to a freshly written file inside it. Quarantine +// targets are unique paths, so these codes cannot mean "target exists" here. +function renameWithTransientRetry(from: string, to: string): void { + const deadline = now() + WIN32_RENAME_RETRY_MS; + while (true) { + try { + renameSync(from, to); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code ?? ''; + if ( + process.platform !== 'win32' + || !WIN32_TRANSIENT_FS_CODES.includes(code) + || now() >= deadline + ) { + throw error; + } + wait(LOCK_RETRY_MS); + } + } +} + +// The quarantine rename already moved ownership off the lock path, so this +// delete is cleanup. On win32 a lingering handle must not fail release; the +// uniquely named leftover is ignored by every reader of the casting dir. +function removeQuarantine(quarantinePath: string): void { + if (process.platform !== 'win32') { + rmSync(quarantinePath, { recursive: true }); + return; + } + try { + rmSync(quarantinePath, { recursive: true, maxRetries: 3 }); + } catch (error) { + if (!WIN32_TRANSIENT_FS_CODES.includes((error as NodeJS.ErrnoException).code ?? '')) throw error; + } +} + +const QUARANTINE_NAME = /^registry\.lock(\.recovery)?\.(?:released|stale)-/; + +// Best-effort cleanup of quarantines whose delete was abandoned, which would +// otherwise accumulate in the committed casting directory. Call only while +// holding the recovery guard: every lock quarantine happens under it, but a +// guard quarantines itself after giving it up, so a guard leftover is removed +// only once it is older than any live guard. +function sweepAbandonedQuarantines(castingDir: string): void { + try { + const staleAge = hooks?.staleLockAgeMs ?? DEFAULT_STALE_LOCK_AGE_MS; + for (const name of readdirSync(castingDir)) { + const match = QUARANTINE_NAME.exec(name); + if (!match) continue; + const entry = path.join(castingDir, name); + try { + if (match[1]) { + const owner = parseOwner(readText(ownerPath(entry))); + if (!owner || now() - Date.parse(owner.created_at) < staleAge) continue; + } + rmSync(entry, { recursive: true, force: true }); + } catch { + // Still held; a later release retries. + } + } + } catch { + // Cleanup must never fail a release that already succeeded. + } +} + function createOwnedDirectory(directoryPath: string, token: string): LockOwner { mkdirSync(directoryPath); const owner: LockOwner = { @@ -341,7 +415,7 @@ function quarantineOwnedDirectory( directoryPath, ); try { - renameSync(directoryPath, quarantinePath); + renameWithTransientRetry(directoryPath, quarantinePath); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') return false; throw error; @@ -357,7 +431,7 @@ function quarantineOwnedDirectory( if (!existsSync(directoryPath)) renameSync(quarantinePath, directoryPath); throw new Error(`Casting lock ownership changed while quarantining ${directoryPath}`); } - rmSync(quarantinePath, { recursive: true }); + removeQuarantine(quarantinePath); flushDirectory(path.dirname(directoryPath)); return true; } @@ -382,6 +456,25 @@ function acquireRecoveryGuard(lockPath: string): { token: string; release: () => }; } +/** + * Whether publishing the candidate lock directory failed because another + * writer already holds the lock. POSIX reports a non-empty rename target as + * EEXIST/ENOTEMPTY; Windows (MoveFileEx onto an existing directory) reports + * EPERM. + */ +function isLockPublishContention(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException).code ?? ''; + if (['EEXIST', 'ENOTEMPTY', 'ENOTDIR'].includes(code)) return true; + return process.platform === 'win32' && code === 'EPERM'; +} + +// Win32 EPERM is ambiguous (held lock vs. a real permission problem), so the +// timeout names the last publish error instead of only blaming a writer. +function lockTimeoutError(purpose: string, lockPath: string, lastPublishCode: string | undefined): Error { + const detail = lastPublishCode ? ` (last publish error: ${lastPublishCode})` : ''; + return new Error(`Timed out waiting for concurrent ${purpose} lock: ${lockPath}${detail}`); +} + function tryRecoverStaleLock(lockPath: string): boolean { const guard = acquireRecoveryGuard(lockPath); if (!guard) return false; @@ -415,6 +508,7 @@ export function acquireCastingRegistryLock( createOwnedDirectory(candidatePath, token); const deadline = now() + (hooks?.lockTimeoutMs ?? DEFAULT_LOCK_TIMEOUT_MS); let published = false; + let lastPublishCode: string | undefined; try { while (true) { if (!existsSync(guardPath)) { @@ -434,13 +528,13 @@ export function acquireCastingRegistryLock( } break; } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (!['EEXIST', 'ENOTEMPTY', 'ENOTDIR'].includes(code ?? '')) throw error; + if (!isLockPublishContention(error)) throw error; + lastPublishCode = (error as NodeJS.ErrnoException).code; } } if (tryRecoverStaleLock(lockPath)) continue; if (now() >= deadline) { - throw new Error(`Timed out waiting for concurrent ${purpose} lock: ${lockPath}`); + throw lockTimeoutError(purpose, lockPath, lastPublishCode); } wait(LOCK_RETRY_MS); } @@ -463,6 +557,7 @@ export function acquireCastingRegistryLock( if (!quarantineOwnedDirectory(lockPath, token, 'released')) { throw new Error(`Casting lock ownership changed before release: ${lockPath}`); } + sweepAbandonedQuarantines(castingDir); } finally { guard.release(); } @@ -482,6 +577,7 @@ export async function acquireCastingRegistryLockAsync( createOwnedDirectory(candidatePath, token); const deadline = now() + (hooks?.lockTimeoutMs ?? DEFAULT_LOCK_TIMEOUT_MS); let published = false; + let lastPublishCode: string | undefined; const waitAsync = async (): Promise => { if (hooks?.wait) hooks.wait(LOCK_RETRY_MS); else await new Promise(resolve => setTimeout(resolve, LOCK_RETRY_MS)); @@ -505,13 +601,13 @@ export async function acquireCastingRegistryLockAsync( } break; } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (!['EEXIST', 'ENOTEMPTY', 'ENOTDIR'].includes(code ?? '')) throw error; + if (!isLockPublishContention(error)) throw error; + lastPublishCode = (error as NodeJS.ErrnoException).code; } } if (tryRecoverStaleLock(lockPath)) continue; if (now() >= deadline) { - throw new Error(`Timed out waiting for concurrent ${purpose} lock: ${lockPath}`); + throw lockTimeoutError(purpose, lockPath, lastPublishCode); } await waitAsync(); } @@ -534,6 +630,7 @@ export async function acquireCastingRegistryLockAsync( if (!quarantineOwnedDirectory(lockPath, token, 'released')) { throw new Error(`Casting lock ownership changed before release: ${lockPath}`); } + sweepAbandonedQuarantines(castingDir); } finally { guard.release(); } diff --git a/test/casting-durable-registry-windows.test.ts b/test/casting-durable-registry-windows.test.ts new file mode 100644 index 000000000..60039c043 --- /dev/null +++ b/test/casting-durable-registry-windows.test.ts @@ -0,0 +1,270 @@ +import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync } from 'node:fs'; +import { basename, join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +// Emulate Windows filesystem semantics on any host (issue #2106): +// - FlushFileBuffers on a directory handle fails with EPERM (file handles flush normally). +// - MoveFileEx onto an existing directory fails with EPERM instead of POSIX ENOTEMPTY/EEXIST. +// - Optionally, a directory rename into release quarantine fails with a transient code a set +// number of times per source path, as when antivirus briefly holds a freshly written owner.json. +// - Optionally, deleting a release quarantine fails with a given code. +const fsFaults = vi.hoisted(() => ({ + releaseRenameFailuresPerPath: 0, + releaseRenameCode: 'EPERM', + injected: new Map(), + quarantineRemoveCode: null as string | null, + directoryFsyncEperm: true, +})); + +const WIN32_TRANSIENT_CODES = ['EPERM', 'EACCES', 'EBUSY'] as const; + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + const fsError = (code: string, syscall: string): Error => + Object.assign(new Error(`${code}: simulated failure, ${syscall}`), { code, syscall }); + const eperm = (syscall: string): Error => + Object.assign(new Error(`EPERM: operation not permitted, ${syscall}`), { code: 'EPERM', syscall }); + const fsyncSync = vi.fn((descriptor: number) => { + if (actual.fstatSync(descriptor).isDirectory()) { + // With the fault off, emulate a POSIX host where directory fsync succeeds. + if (fsFaults.directoryFsyncEperm) throw eperm('fsync'); + return undefined; + } + return actual.fsyncSync(descriptor); + }); + const rmSync = vi.fn((target: string, options?: import('node:fs').RmOptions) => { + if (fsFaults.quarantineRemoveCode && String(target).includes('.released-')) { + throw fsError(fsFaults.quarantineRemoveCode, 'rmdir'); + } + return actual.rmSync(target, options); + }); + const renameSync = vi.fn((from: string, to: string) => { + if (process.platform === 'win32' && String(to).includes('.released-')) { + const seen = fsFaults.injected.get(String(from)) ?? 0; + if (seen < fsFaults.releaseRenameFailuresPerPath) { + fsFaults.injected.set(String(from), seen + 1); + throw fsError(fsFaults.releaseRenameCode, 'rename'); + } + } + try { + return actual.renameSync(from, to); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (process.platform === 'win32' && (code === 'ENOTEMPTY' || code === 'EEXIST')) { + throw eperm('rename'); + } + throw error; + } + }); + const overrides = { fsyncSync, renameSync, rmSync }; + return { ...actual, ...overrides, default: { ...actual, ...overrides } }; +}); + +const { + _setCastingDurabilityHooksForTesting, + acquireCastingRegistryLock, + acquireCastingRegistryLockAsync, + ensureCastingRegistryPair, +} = await import('../packages/squad-sdk/src/casting/durable-registry.js'); + +const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')!; +const roots: string[] = []; + +function setPlatform(platform: NodeJS.Platform): void { + Object.defineProperty(process, 'platform', { ...originalPlatform, value: platform }); +} + +function castingDir(name: string): string { + const root = join(tmpdir(), `squad-durable-win-${name}-${process.pid}-${Date.now()}`); + roots.push(root); + const dir = join(root, '.squad', 'casting'); + mkdirSync(dir, { recursive: true }); + return dir; +} + +function useFastLockClock(): { advance: (milliseconds: number) => void } { + let clock = Date.now(); + _setCastingDurabilityHooksForTesting({ + now: () => clock, + wait: milliseconds => { clock += milliseconds; }, + lockTimeoutMs: 50, + }); + return { advance: milliseconds => { clock += milliseconds; } }; +} + +const registryRaw = JSON.stringify({ + schema: 'squad-agent-provenance/v1', + schema_version: 1, + revision: 1, + generated_at: '2026-09-20T00:00:00.000Z', + agents: {}, +}, null, 2) + '\n'; +const historyRaw = JSON.stringify({ + assignment_cast_snapshots: {}, + universe_usage_history: [], +}, null, 2) + '\n'; + +afterEach(() => { + fsFaults.releaseRenameFailuresPerPath = 0; + fsFaults.releaseRenameCode = 'EPERM'; + fsFaults.injected.clear(); + fsFaults.quarantineRemoveCode = null; + fsFaults.directoryFsyncEperm = true; + _setCastingDurabilityHooksForTesting(null); + Object.defineProperty(process, 'platform', originalPlatform); + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +describe('casting registry durability on Windows filesystem semantics (#2106)', () => { + it('initializes the casting pair on win32 when directory fsync returns EPERM', () => { + setPlatform('win32'); + const dir = castingDir('init'); + + expect( + () => ensureCastingRegistryPair(dir, registryRaw, historyRaw), + 'ensureCastingRegistryPair must not fsync directory handles on win32 ' + + '(Windows rejects FlushFileBuffers on a directory with EPERM)', + ).not.toThrow(); + expect(JSON.parse(readFileSync(join(dir, 'registry.json'), 'utf8'))).toMatchObject({ revision: 1, agents: {} }); + expect(JSON.parse(readFileSync(join(dir, 'history.json'), 'utf8'))).toMatchObject({ universe_usage_history: [] }); + }); + + it('still flushes directories on POSIX and surfaces a directory fsync failure', () => { + setPlatform('linux'); + const dir = castingDir('posix'); + + expect( + () => ensureCastingRegistryPair(dir, registryRaw, historyRaw), + 'on POSIX the directory fsync is a required durability step; its failure must not be swallowed', + ).toThrow(/EPERM: operation not permitted, fsync/); + }); + + it('waits on a held lock when win32 reports the publish rename as EPERM (sync)', () => { + setPlatform('win32'); + const dir = castingDir('lock-sync'); + const release = acquireCastingRegistryLock(dir); + useFastLockClock(); + + expect( + () => acquireCastingRegistryLock(dir), + 'a second acquirer on win32 must treat EPERM from renaming onto the held lock ' + + 'directory as contention and time out, not crash with EPERM', + ).toThrow(/Timed out waiting for concurrent casting writer lock: .*\(last publish error: EPERM\)/); + release(); + expect(existsSync(join(dir, 'registry.lock'))).toBe(false); + }); + + it('waits on a held lock when win32 reports the publish rename as EPERM (async)', async () => { + setPlatform('win32'); + const dir = castingDir('lock-async'); + const release = acquireCastingRegistryLock(dir); + useFastLockClock(); + + await expect( + acquireCastingRegistryLockAsync(dir), + 'the async acquirer on win32 must treat EPERM from renaming onto the held lock ' + + 'directory as contention and time out, not crash with EPERM', + ).rejects.toThrow(/Timed out waiting for concurrent casting writer lock: .*\(last publish error: EPERM\)/); + release(); + expect(existsSync(join(dir, 'registry.lock'))).toBe(false); + }); + + it.each(WIN32_TRANSIENT_CODES)( + 'retries a transient win32 %s when releasing the lock and its recovery guard', + code => { + setPlatform('win32'); + const dir = castingDir(`release-transient-${code}`); + const release = acquireCastingRegistryLock(dir); + useFastLockClock(); + fsFaults.releaseRenameCode = code; + fsFaults.releaseRenameFailuresPerPath = 2; + + expect( + () => release(), + `release on win32 must retry a transient ${code} from the quarantine rename; ` + + 'a leftover registry.lock.recovery wedges every later casting writer', + ).not.toThrow(); + expect([...fsFaults.injected.keys()].map(p => basename(p)).sort()).toEqual([ + 'registry.lock', + 'registry.lock.recovery', + ]); + expect(existsSync(join(dir, 'registry.lock'))).toBe(false); + expect(existsSync(join(dir, 'registry.lock.recovery'))).toBe(false); + }, + ); + + it('gives up on a persistent win32 EPERM when releasing after the retry window', () => { + setPlatform('win32'); + const dir = castingDir('release-persistent'); + const release = acquireCastingRegistryLock(dir); + useFastLockClock(); + fsFaults.releaseRenameFailuresPerPath = Number.POSITIVE_INFINITY; + + expect( + () => release(), + 'a persistent EPERM on release must surface after a bounded retry, not loop forever', + ).toThrow(/EPERM: simulated failure, rename/); + }); + + it.each(WIN32_TRANSIENT_CODES)( + 'releases on win32 when deleting the quarantined lock hits %s', + code => { + setPlatform('win32'); + const dir = castingDir(`release-rm-win32-${code}`); + const release = acquireCastingRegistryLock(dir); + fsFaults.quarantineRemoveCode = code; + + expect( + () => release(), + `on win32 the lock is released once the quarantine rename succeeds; a ${code} ` + + 'deleting the quarantined copy (antivirus holding owner.json) must not fail release', + ).not.toThrow(); + expect(existsSync(join(dir, 'registry.lock'))).toBe(false); + expect(existsSync(join(dir, 'registry.lock.recovery'))).toBe(false); + expect(readdirSync(dir).filter(name => name.includes('.released-'))).toHaveLength(2); + }, + ); + + it('sweeps abandoned quarantines on a later release, keeping guard quarantines until stale', () => { + setPlatform('win32'); + const dir = castingDir('sweep'); + const clock = useFastLockClock(); + const leftovers = (): string[] => + readdirSync(dir).filter(name => /\.(released|stale)-/.test(name)).sort(); + + fsFaults.quarantineRemoveCode = 'EPERM'; + acquireCastingRegistryLock(dir)(); + expect(leftovers(), 'setup: an abandoned delete leaves a lock and a guard quarantine').toHaveLength(2); + fsFaults.quarantineRemoveCode = null; + mkdirSync(join(dir, 'registry.lock.stale-abandoned')); + + acquireCastingRegistryLock(dir)(); + const young = leftovers(); + expect( + young.map(name => name.replace(/-.*/, '-*')), + 'a later release must sweep abandoned lock quarantines (.released-*, .stale-*) under the ' + + `recovery guard, but keep a guard quarantine younger than the stale age; left: ${young.join(', ')}`, + ).toEqual(['registry.lock.recovery.released-*']); + + clock.advance(5_001); + acquireCastingRegistryLock(dir)(); + expect( + leftovers(), + 'a guard quarantine older than the stale-lock age must be swept on the next release', + ).toEqual([]); + }); + + it('still surfaces a quarantine delete failure on POSIX', () => { + setPlatform('linux'); + fsFaults.directoryFsyncEperm = false; + const dir = castingDir('release-rm-posix'); + const release = acquireCastingRegistryLock(dir); + fsFaults.quarantineRemoveCode = 'EPERM'; + + expect( + () => release(), + 'the win32 best-effort delete must not swallow POSIX delete failures', + ).toThrow(/EPERM: simulated failure, rmdir/); + }); +}); diff --git a/test/casting-durable-registry.test.ts b/test/casting-durable-registry.test.ts index cb09264a4..74824a172 100644 --- a/test/casting-durable-registry.test.ts +++ b/test/casting-durable-registry.test.ts @@ -10,6 +10,7 @@ import { import { hostname } from 'node:os'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { pathToFileURL } from 'node:url'; import { spawn, spawnSync } from 'node:child_process'; import { afterEach, describe, expect, it } from 'vitest'; import { @@ -264,7 +265,7 @@ describe('casting registry directory lock', () => { 'packages/squad-sdk/src/casting/durable-registry.ts', ); const script = ` - import { acquireCastingRegistryLock } from ${JSON.stringify(modulePath)}; + import { acquireCastingRegistryLock } from ${JSON.stringify(pathToFileURL(modulePath).href)}; import { appendFileSync } from 'node:fs'; const castingDir = process.argv[1]; const logPath = process.argv[2]; @@ -469,7 +470,7 @@ describe('casting registry/history roll-forward transaction', () => { import { _setCastingDurabilityHooksForTesting, commitCastingRegistryPair, - } from ${JSON.stringify(modulePath)}; + } from ${JSON.stringify(pathToFileURL(modulePath).href)}; import { readFileSync } from 'node:fs'; import { join } from 'node:path'; const failure = process.argv[1];