diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 82345c81f..25ed6c735 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -35,6 +35,7 @@ - Fixed `~` not being expanded in `!pattern`, `+path`, and `-path` entries of the `extensions`, `skills`, `prompts`, and `themes` settings, so entries like `!~/skills/pdf` matched nothing ([#10765](https://github.com/earendil-works/pi/issues/10765)) - Fixed MCP servers with resources registering no tools until every resource was listed; Pi no longer lists resources when connecting, so `/mcp` and `pi mcp list` show that a server offers resources instead of counting them ([#10526](https://github.com/earendil-works/pi/issues/10526)) - Fixed MCP tool calls failing when the server returns structured content that is not an object (an array, `null`, or a primitive, as MCP 2026-07-28 allows); codemode declarations of MCP tools without an output schema now type `structuredContent` as `unknown` instead of an object ([#10416](https://github.com/earendil-works/pi/issues/10416)) +- Fixed reading or writing `auth.json` on Windows failing with `EPERM` when the previous lock folder was still being deleted; Pi now retries briefly before reporting the error ([#10801](https://github.com/earendil-works/pi/issues/10801)) ### Removed diff --git a/packages/coding-agent/src/core/auth-storage.ts b/packages/coding-agent/src/core/auth-storage.ts index 9d6495919..5a8fa417a 100644 --- a/packages/coding-agent/src/core/auth-storage.ts +++ b/packages/coding-agent/src/core/auth-storage.ts @@ -16,6 +16,18 @@ import { isCommandConfigValue, resolveConfigValue } from "./resolve-config-value type AuthStorageData = Record; +// On Windows, a just-removed lock directory can stay "delete pending" while another process +// (antivirus, indexer, another pi checking the lock) still has it open. mkdir then fails with +// EPERM until that handle closes. Retry this briefly so real permission errors still fail fast. +const MAX_EPERM_RETRIES = 5; + +function getRetryableLockErrorCode(error: unknown): "ELOCKED" | "EPERM" | undefined { + const code = error instanceof Error && "code" in error ? error.code : undefined; + if (code === "ELOCKED") return code; + if (code === "EPERM" && process.platform === "win32") return code; + return undefined; +} + type LockResult = { result: T; next?: string; @@ -69,28 +81,17 @@ export class FileAuthStorageBackend implements AuthStorageBackend { private acquireLockSyncWithRetry(path: string): () => void { const maxAttempts = 10; const delayMs = 20; - let lastError: unknown; - - for (let attempt = 1; attempt <= maxAttempts; attempt++) { + for (let attempt = 1; ; attempt++) { try { return lockfile.lockSync(path, { realpath: false }); } catch (error) { - const code = - typeof error === "object" && error !== null && "code" in error - ? String((error as { code?: unknown }).code) - : undefined; - if (code !== "ELOCKED" || attempt === maxAttempts) { - throw error; - } - lastError = error; + if (!getRetryableLockErrorCode(error) || attempt === maxAttempts) throw error; const start = Date.now(); while (Date.now() - start < delayMs) { // Sleep synchronously to avoid changing callers to async. } } } - - throw (lastError as Error) ?? new Error("Failed to acquire auth storage lock"); } withLock(fn: (current: string | undefined) => LockResult): T { @@ -121,6 +122,7 @@ export class FileAuthStorageBackend implements AuthStorageBackend { const maxDelayMs = 2_000; const deadline = Date.now() + staleMs; let retry = 0; + let epermRetries = 0; while (true) { signal?.throwIfAborted(); let release: (() => Promise) | undefined; @@ -133,12 +135,11 @@ export class FileAuthStorageBackend implements AuthStorageBackend { }); } catch (error) { signal?.throwIfAborted(); - const code = - typeof error === "object" && error !== null && "code" in error - ? String((error as { code?: unknown }).code) - : undefined; + const code = getRetryableLockErrorCode(error); const remainingMs = deadline - Date.now(); - if (code !== "ELOCKED" || remainingMs <= 0) throw error; + if (!code || remainingMs <= 0) throw error; + // The 30s deadline is for waiting on another holder; cap EPERM retries separately. + if (code === "EPERM" && ++epermRetries > MAX_EPERM_RETRIES) throw error; const baseDelayMs = Math.min(10 * 2 ** retry, maxDelayMs / 2); retry++; const delayMs = Math.min(Math.round(baseDelayMs * (1 + Math.random())), remainingMs); diff --git a/packages/coding-agent/test/auth-storage.test.ts b/packages/coding-agent/test/auth-storage.test.ts index 61cdbefba..8005bf978 100644 --- a/packages/coding-agent/test/auth-storage.test.ts +++ b/packages/coding-agent/test/auth-storage.test.ts @@ -260,6 +260,82 @@ describe("AuthStorage", () => { expect(release).toHaveBeenCalledTimes(1); }); + describe("Windows EPERM on lock acquisition", () => { + // #10801: a delete-pending lock directory on Windows makes mkdir fail with EPERM briefly. + const eperm = () => Object.assign(new Error("operation not permitted"), { code: "EPERM" }); + const originalPlatform = process.platform; + const setPlatform = (platform: NodeJS.Platform) => + Object.defineProperty(process, "platform", { value: platform, configurable: true }); + + afterEach(() => { + setPlatform(originalPlatform); + }); + + test("retries transient EPERM on Windows (async)", async () => { + setPlatform("win32"); + writeAuthJson({}); + const backend = new FileAuthStorageBackend(authJsonPath); + const release = vi.fn(async () => {}); + const lockSpy = vi.spyOn(lockfile, "lock").mockRejectedValueOnce(eperm()).mockResolvedValueOnce(release); + vi.spyOn(Math, "random").mockReturnValue(0); + const update = vi.fn(async () => ({ result: undefined })); + + await backend.withLockAsync(update); + + expect(lockSpy).toHaveBeenCalledTimes(2); + expect(update).toHaveBeenCalledTimes(1); + expect(release).toHaveBeenCalledTimes(1); + }); + + test("surfaces persistent EPERM on Windows after a bounded number of retries (async)", async () => { + setPlatform("win32"); + writeAuthJson({}); + const backend = new FileAuthStorageBackend(authJsonPath); + const lockSpy = vi.spyOn(lockfile, "lock").mockImplementation(async () => { + throw eperm(); + }); + vi.spyOn(Math, "random").mockReturnValue(0); + const update = vi.fn(async () => ({ result: undefined })); + + await expect(backend.withLockAsync(update)).rejects.toMatchObject({ code: "EPERM" }); + + expect(lockSpy).toHaveBeenCalledTimes(6); + expect(update).not.toHaveBeenCalled(); + }); + + test("does not retry EPERM on other platforms (async)", async () => { + setPlatform("linux"); + writeAuthJson({}); + const backend = new FileAuthStorageBackend(authJsonPath); + const lockSpy = vi.spyOn(lockfile, "lock").mockRejectedValueOnce(eperm()); + + await expect(backend.withLockAsync(async () => ({ result: undefined }))).rejects.toMatchObject({ + code: "EPERM", + }); + expect(lockSpy).toHaveBeenCalledTimes(1); + }); + + test("retries transient EPERM on Windows (sync)", () => { + setPlatform("win32"); + writeAuthJson({}); + const backend = new FileAuthStorageBackend(authJsonPath); + const release = vi.fn(); + const lockSpy = vi + .spyOn(lockfile, "lockSync") + .mockImplementationOnce(() => { + throw eperm(); + }) + .mockReturnValueOnce(release); + const update = vi.fn(() => ({ result: undefined })); + + backend.withLock(update); + + expect(lockSpy).toHaveBeenCalledTimes(2); + expect(update).toHaveBeenCalledTimes(1); + expect(release).toHaveBeenCalledTimes(1); + }); + }); + test("surfaces a compromised file storage lock", async () => { writeAuthJson({ anthropic: { type: "api_key", key: "stored" } }); const backend = new FileAuthStorageBackend(authJsonPath);