mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): retry transient Windows EPERM when acquiring auth lock
A just-removed auth.json.lock directory can stay delete-pending on Windows while another process holds a handle, making mkdir fail with EPERM. Retry this briefly (capped separately from ELOCKED) so real permission errors still fail fast. fixes #10801
This commit is contained in:
parent
441f5767c4
commit
e6fb2581df
3 changed files with 96 additions and 18 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -16,6 +16,18 @@ import { isCommandConfigValue, resolveConfigValue } from "./resolve-config-value
|
|||
|
||||
type AuthStorageData = Record<string, Credential>;
|
||||
|
||||
// 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<T> = {
|
||||
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<T>(fn: (current: string | undefined) => LockResult<T>): 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<void>) | 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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue