diff --git a/tests/helpers/isolated-codex-home.ts b/tests/helpers/isolated-codex-home.ts index 975e8e81c..17273755e 100644 --- a/tests/helpers/isolated-codex-home.ts +++ b/tests/helpers/isolated-codex-home.ts @@ -1,6 +1,7 @@ -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { removeTreeWithRetry } from "./remove-tree"; export interface IsolatedCodexHome { path: string; @@ -18,7 +19,7 @@ export function installIsolatedCodexHome(prefix = "ocx-codex-home-"): IsolatedCo restore() { if (previousCodexHome === undefined) delete process.env.CODEX_HOME; else process.env.CODEX_HOME = previousCodexHome; - rmSync(path, { recursive: true, force: true }); + removeTreeWithRetry(path); }, }; } diff --git a/tests/helpers/remove-tree.ts b/tests/helpers/remove-tree.ts new file mode 100644 index 000000000..53e36a584 --- /dev/null +++ b/tests/helpers/remove-tree.ts @@ -0,0 +1,32 @@ +import { rmSync } from "node:fs"; + +const TRANSIENT_REMOVE_CODES = new Set(["EPERM", "EBUSY", "ENOTEMPTY"]); +const REMOVE_ATTEMPTS = 50; +const REMOVE_RETRY_DELAY_MS = 50; + +type RemoveTreeWithRetryOptions = Readonly<{ + remove?: (path: string) => void; + sleep?: (milliseconds: number) => void; +}>; + +/** Retry only Windows filesystem-release races; preserve every other cleanup failure. */ +export function removeTreeWithRetry( + path: string, + options: RemoveTreeWithRetryOptions = {}, +): void { + const remove = options.remove ?? (target => rmSync(target, { recursive: true, force: true })); + const sleep = options.sleep ?? Bun.sleepSync; + + for (let attempt = 1; attempt <= REMOVE_ATTEMPTS; attempt += 1) { + try { + remove(path); + return; + } catch (error) { + const code = error && typeof error === "object" && "code" in error + ? String(error.code) + : ""; + if (!TRANSIENT_REMOVE_CODES.has(code) || attempt === REMOVE_ATTEMPTS) throw error; + sleep(REMOVE_RETRY_DELAY_MS); + } + } +} diff --git a/tests/remove-tree-helper.test.ts b/tests/remove-tree-helper.test.ts new file mode 100644 index 000000000..f7c4cb4b1 --- /dev/null +++ b/tests/remove-tree-helper.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "bun:test"; +import { removeTreeWithRetry } from "./helpers/remove-tree"; + +function codedError(code: string, message = code): Error & { code: string } { + return Object.assign(new Error(message), { code }); +} + +describe("removeTreeWithRetry", () => { + test.each(["EPERM", "EBUSY", "ENOTEMPTY"])("retries transient %s failures", code => { + let removeCalls = 0; + const sleeps: number[] = []; + + removeTreeWithRetry("ignored", { + remove: () => { + removeCalls += 1; + if (removeCalls < 3) throw codedError(code); + }, + sleep: milliseconds => sleeps.push(milliseconds), + }); + + expect(removeCalls).toBe(3); + expect(sleeps).toEqual([50, 50]); + }); + + test("rethrows non-transient failures immediately", () => { + const error = codedError("EACCES", "denied"); + let sleeps = 0; + + expect(() => removeTreeWithRetry("ignored", { + remove: () => { throw error; }, + sleep: () => { sleeps += 1; }, + })).toThrow(error); + expect(sleeps).toBe(0); + }); + + test("rethrows the final transient failure without an extra sleep", () => { + const error = codedError("EBUSY", "still locked"); + let removeCalls = 0; + let sleeps = 0; + + expect(() => removeTreeWithRetry("ignored", { + remove: () => { + removeCalls += 1; + throw error; + }, + sleep: () => { sleeps += 1; }, + })).toThrow(error); + expect(removeCalls).toBe(50); + expect(sleeps).toBe(49); + }); +}); diff --git a/tests/server-rate-limit-retry-e2e.test.ts b/tests/server-rate-limit-retry-e2e.test.ts index 6a6d7f4c6..467f6ca52 100644 --- a/tests/server-rate-limit-retry-e2e.test.ts +++ b/tests/server-rate-limit-retry-e2e.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { saveConfig } from "../src/config"; @@ -7,6 +7,7 @@ import { clearKeyCooldowns } from "../src/providers/key-failover"; import { startServer } from "../src/server"; import type { OcxConfig } from "../src/types"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; +import { removeTreeWithRetry } from "./helpers/remove-tree"; let testDir = ""; let previousHome: string | undefined; @@ -25,7 +26,7 @@ afterEach(() => { else process.env.OPENCODEX_HOME = previousHome; isolatedCodexHome?.restore(); isolatedCodexHome = null; - if (testDir) rmSync(testDir, { recursive: true, force: true }); + if (testDir) removeTreeWithRetry(testDir); clearKeyCooldowns(); }); @@ -100,8 +101,11 @@ describe("server same-target 429 retry (end-to-end)", () => { expect(seenHeaders[0]).toEqual(seenHeaders[1]); expect(seenHeaders[1]).toEqual(seenHeaders[2]); } finally { - server?.stop(true); - globalThis.fetch = originalFetch; + try { + await server?.stop(true); + } finally { + globalThis.fetch = originalFetch; + } } }); @@ -144,8 +148,11 @@ describe("server same-target 429 retry (end-to-end)", () => { expect(json.error?.type).toBe("rate_limit_error"); expect(sends).toBe(1); } finally { - server?.stop(true); - globalThis.fetch = originalFetch; + try { + await server?.stop(true); + } finally { + globalThis.fetch = originalFetch; + } } }); @@ -186,8 +193,11 @@ describe("server same-target 429 retry (end-to-end)", () => { expect(res.status).toBe(429); expect(sends).toBe(2); } finally { - server?.stop(true); - globalThis.fetch = originalFetch; + try { + await server?.stop(true); + } finally { + globalThis.fetch = originalFetch; + } } }); @@ -240,8 +250,11 @@ describe("server same-target 429 retry (end-to-end)", () => { "Bearer key-beta-444555666777", ]); } finally { - server?.stop(true); - globalThis.fetch = originalFetch; + try { + await server?.stop(true); + } finally { + globalThis.fetch = originalFetch; + } } }); @@ -303,8 +316,11 @@ describe("server same-target 429 retry (end-to-end)", () => { expect(seenBodies).toHaveLength(2); expect(seenBodies[0]).toBe(seenBodies[1]); } finally { - server?.stop(true); - globalThis.fetch = originalFetch; + try { + await server?.stop(true); + } finally { + globalThis.fetch = originalFetch; + } } }); @@ -351,8 +367,11 @@ describe("server same-target 429 retry (end-to-end)", () => { // have replayed on the second key too (4+ sends). expect(sends).toBe(3); } finally { - server?.stop(true); - globalThis.fetch = originalFetch; + try { + await server?.stop(true); + } finally { + globalThis.fetch = originalFetch; + } } }); });