From da7ffabf5aae12a6f3bcffa5b08513c14814a815 Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 9 Nov 2026 11:54:45 -0800 Subject: [PATCH 1/2] Retry custom plugin ZIP publication on Windows --- sdk/typescript/scripts/build-plugin.mjs | 21 +--- sdk/typescript/scripts/package-dist-files.mjs | 1 + sdk/typescript/src/retry-windows-rename.js | 25 ++++ sdk/typescript/src/runtime.ts | 31 ++++- sdk/typescript/tests-ts/build-plugin.test.ts | 49 +++----- sdk/typescript/tests-ts/runtime.test.ts | 108 ++++++++++++++++++ 6 files changed, 183 insertions(+), 52 deletions(-) create mode 100644 sdk/typescript/src/retry-windows-rename.js diff --git a/sdk/typescript/scripts/build-plugin.mjs b/sdk/typescript/scripts/build-plugin.mjs index 054e67e66a..9adde02a97 100644 --- a/sdk/typescript/scripts/build-plugin.mjs +++ b/sdk/typescript/scripts/build-plugin.mjs @@ -12,32 +12,13 @@ import { rm, } from "node:fs/promises"; import { dirname, join, posix, resolve } from "node:path"; -import { setTimeout as delay } from "node:timers/promises"; +import { retryWindowsRename } from "../src/retry-windows-rename.js"; import { isMain } from "./is-main.mjs"; import { pluginContractFiles } from "./plugin-contract.mjs"; const packageRoot = resolve(import.meta.dirname, ".."); const repositoryRoot = resolve(packageRoot, "../.."); -async function retryWindowsRename(operation, rollback) { - // Windows can temporarily deny a directory rename while a file inside is open. - for (let attempt = 0; ; attempt++) { - try { - return await operation(); - } catch (error) { - await rollback?.(); - if ( - process.platform !== "win32" || - !["EPERM", "EACCES", "EBUSY"].includes(error.code) || - attempt === 5 - ) { - throw error; - } - await delay(100 * (attempt + 1)); - } - } -} - function validatePath(path) { const parts = path.split("/"); if ( diff --git a/sdk/typescript/scripts/package-dist-files.mjs b/sdk/typescript/scripts/package-dist-files.mjs index a09a38418a..7e1a295461 100644 --- a/sdk/typescript/scripts/package-dist-files.mjs +++ b/sdk/typescript/scripts/package-dist-files.mjs @@ -129,6 +129,7 @@ const distModules = [ "record", "remediation", "request-metadata", + "retry-windows-rename", "runtime", "scan-activity", "scan-comparison", diff --git a/sdk/typescript/src/retry-windows-rename.js b/sdk/typescript/src/retry-windows-rename.js new file mode 100644 index 0000000000..9c8899375d --- /dev/null +++ b/sdk/typescript/src/retry-windows-rename.js @@ -0,0 +1,25 @@ +import { setTimeout as delay } from "node:timers/promises"; + +/** + * Windows can temporarily deny a directory rename while a descendant is open. + * @param {() => Promise} operation + * @param {() => Promise} [rollback] + * @param {AbortSignal} [signal] + */ +export async function retryWindowsRename(operation, rollback, signal) { + for (let attempt = 0; ; attempt++) { + try { + return await operation(); + } catch (error) { + await rollback?.(); + if ( + process.platform !== "win32" || + !["EPERM", "EACCES", "EBUSY"].includes(error?.code) || + attempt === 5 + ) { + throw error; + } + await delay(100 * (attempt + 1), undefined, { signal }); + } + } +} diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 5b4a8b2f8a..fe8d9fdb48 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -1,4 +1,5 @@ import { executablePathForSpawn } from "./executable-path.js"; +import { retryWindowsRename } from "./retry-windows-rename.js"; export { executablePathForSpawn } from "./executable-path.js"; import { resolveBundledCodexExecutable } from "./codex-sdk-environment.js"; import { gitProtectionRoots } from "./targets.js"; @@ -2642,6 +2643,7 @@ export async function extractPluginZip( const staging = await realpath( await mkdtemp(join(dirname(destination), ".codex-security-plugin-")), ); + let publishingOnWindows = false; try { throwIfSignalAborted(signal); let expandedSize = 0; @@ -2718,11 +2720,36 @@ export async function extractPluginZip( const pluginRoot = await discoverPluginRoot(staging); throwIfSignalAborted(signal); const relativeRoot = relative(staging, pluginRoot); - await rename(staging, destination); + if (process.platform === "win32") { + publishingOnWindows = true; + await retryWindowsRename( + async () => { + throwIfSignalAborted(signal); + if (await lstat(destination).catch(nullIfMissingFileError)) + throw new Error(`Destination already exists: ${destination}`); + await rename(staging, destination); + }, + undefined, + signal, + ); + publishingOnWindows = false; + } else { + await rename(staging, destination); + } return await validatePluginRoot(join(destination, relativeRoot)); } catch (error) { - await rm(staging, { recursive: true, force: true }).catch(() => undefined); + let cleanupDiagnostic = ""; + await rm(staging, { recursive: true, force: true }).catch( + (cleanupError) => { + cleanupDiagnostic = `\nCould not remove staging directory ${staging}: ${errorMessage(cleanupError)}`; + }, + ); throwIfSignalAborted(signal); + if (publishingOnWindows) + throw new PluginBootstrapError( + `Could not publish extracted plugin ZIP: ${errorMessage(error)}${cleanupDiagnostic}`, + { cause: error }, + ); if (error instanceof PluginBootstrapError) throw error; throw new PluginBootstrapError(`Invalid plugin ZIP: ${archivePath}`, { cause: error, diff --git a/sdk/typescript/tests-ts/build-plugin.test.ts b/sdk/typescript/tests-ts/build-plugin.test.ts index 0b82e255fd..97de0726b5 100644 --- a/sdk/typescript/tests-ts/build-plugin.test.ts +++ b/sdk/typescript/tests-ts/build-plugin.test.ts @@ -38,6 +38,23 @@ async function writeFixture( await writeFile(destination, contents); } +async function copyBuildEntrypoint(root: string): Promise { + for (const path of [ + "package.json", + "scripts/build-plugin.mjs", + "scripts/check-plugin-source.mjs", + "scripts/is-main.mjs", + "scripts/plugin-contract.mjs", + "src/retry-windows-rename.js", + ]) { + await writeFixture( + root, + path, + await readFile(new URL(`../${path}`, import.meta.url), "utf8"), + ); + } +} + async function files(root: string, prefix = ""): Promise { const entries = await readdir(join(root, prefix), { withFileTypes: true }); const paths = await Promise.all( @@ -435,16 +452,7 @@ describe("bundled plugin build", () => { const packageRoot = join(root, "sdk", "typescript"); const source = join(root, "plugins", "codex-security"); const destination = join(packageRoot, "_bundled_plugin"); - for (const script of ["build-plugin", "plugin-contract", "is-main"]) { - await writeFixture( - packageRoot, - `scripts/${script}.mjs`, - await readFile( - new URL(`../scripts/${script}.mjs`, import.meta.url), - "utf8", - ), - ); - } + await copyBuildEntrypoint(packageRoot); await writeFixture(source, ".codex-plugin/plugin.json", "{}\n"); await writeFixture( source, @@ -491,26 +499,7 @@ await writeFile(join(output, "unexpected.txt"), "undeclared output");\n`, const packageRoot = join(root, "sdk", "typescript"); const source = join(root, "plugins", "codex-security"); - await writeFixture( - packageRoot, - "package.json", - await readFile(new URL("../package.json", import.meta.url), "utf8"), - ); - for (const script of [ - "build-plugin", - "check-plugin-source", - "is-main", - "plugin-contract", - ]) { - await writeFixture( - packageRoot, - `scripts/${script}.mjs`, - await readFile( - new URL(`../scripts/${script}.mjs`, import.meta.url), - "utf8", - ), - ); - } + await copyBuildEntrypoint(packageRoot); await writeFixture( source, "plugin-files.json", diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index a996d793b8..31320962fb 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -1528,6 +1528,114 @@ for await (const line of createInterface({ input: process.stdin })) { } }); + test.skipIf(process.platform !== "win32").each(["released", "held"])( + "handles plugin ZIP publication when a Windows directory lock is %s", + async (lock) => { + const root = await temporaryDirectory(); + const archive = join(root, "plugin.zip"); + const destination = join(root, "extracted"); + const manifest = ".codex-plugin/plugin.json"; + const contents = JSON.stringify({ + name: "codex-security", + version: "1.2.3", + }); + await writeFile(archive, zipSync({ [manifest]: strToU8(contents) })); + const nativeRename = fsPromises.rename; + let blocked: NodeJS.ErrnoException | undefined; + let staging = ""; + let release: (() => Promise) | undefined; + using publication = spyOn(fsPromises, "rename").mockImplementation( + async (from, to) => { + if (to !== destination || staging) return nativeRename(from, to); + staging = String(from); + // Windows keeps a process's current directory open until the process exits. + const holder = childProcess.spawn( + "node", + ["-e", 'process.stdout.write("ready"); process.stdin.resume()'], + { + cwd: join(staging, ".codex-plugin"), + windowsHide: true, + }, + ); + const exited = once(holder, "exit"); + release = async () => { + holder.stdin.end(); + await exited; + }; + await once(holder.stdout, "data"); + try { + await nativeRename(from, to); + } catch (error) { + blocked = error as NodeJS.ErrnoException; + throw error; + } finally { + if (lock === "released") await release(); + } + }, + ); + try { + const result = extractPluginZip(archive, destination); + if (lock === "held") { + const failure = await result.catch((error: unknown) => error); + expect((failure as Error).message).toContain( + `Could not remove staging directory ${staging}:`, + ); + expect(failure).toMatchObject({ + message: expect.stringMatching(/publish.*ZIP.*rename/iu), + cause: { syscall: "rename", dest: destination }, + }); + expect(existsSync(staging)).toBe(true); + expect(existsSync(destination)).toBe(false); + } else { + expect(await result).toBe(destination); + expect(await readFile(join(destination, manifest), "utf8")).toBe( + contents, + ); + expect(existsSync(staging)).toBe(false); + } + expect(["EPERM", "EACCES", "EBUSY"]).toContain(blocked?.code ?? ""); + expect(publication.mock.calls.length).toBeGreaterThan(1); + } finally { + if (lock === "held") await release?.(); + } + }, + ); + + test.each(["empty", "nonempty"])( + "preserves platform behavior for an existing %s plugin ZIP destination", + async (kind) => { + const root = await temporaryDirectory(); + const archive = join(root, "plugin.zip"); + const destination = join(root, "extracted"); + await mkdir(destination); + const sentinel = join(destination, "previous"); + if (kind === "nonempty") await writeFile(sentinel, "keep"); + await writeFile( + archive, + zipSync({ + ".codex-plugin/plugin.json": strToU8( + JSON.stringify({ name: "codex-security", version: "1.2.3" }), + ), + }), + ); + const result = extractPluginZip(archive, destination); + if (process.platform === "win32" || kind === "nonempty") { + await expect(result).rejects.toThrow( + process.platform === "win32" + ? "Destination already exists" + : "Invalid plugin ZIP", + ); + if (kind === "nonempty") + expect(await readFile(sentinel, "utf8")).toBe("keep"); + else expect(await readdir(destination)).toEqual([]); + } else { + expect(await result).toBe(destination); + expect(await readdir(destination)).toEqual([".codex-plugin"]); + } + expect((await readdir(root)).sort()).toEqual(["extracted", "plugin.zip"]); + }, + ); + test("decodes flag-clear ZIP filenames with the legacy CP437 encoding", async () => { const root = await temporaryDirectory(); const archive = Buffer.from( From 4f4011aabb5ff1cd42fb9233979d8f26d731839a Mon Sep 17 00:00:00 2001 From: mldangelo-oai <269034524+mldangelo-oai@users.noreply.github.com> Date: Mon, 9 Nov 2026 12:13:54 -0800 Subject: [PATCH 2/2] fix(sdk): declare the Windows rename helper for source consumers Strict eval builds type-check the SDK source without JavaScript inference. Pair the shared helper with its source declaration and include its JavaScript explicitly so clean SDK builds still emit the implementation, declarations, and maps. --- sdk/typescript/src/retry-windows-rename.d.ts | 5 +++++ sdk/typescript/tsconfig.build.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 sdk/typescript/src/retry-windows-rename.d.ts diff --git a/sdk/typescript/src/retry-windows-rename.d.ts b/sdk/typescript/src/retry-windows-rename.d.ts new file mode 100644 index 0000000000..6628ea4f00 --- /dev/null +++ b/sdk/typescript/src/retry-windows-rename.d.ts @@ -0,0 +1,5 @@ +export function retryWindowsRename( + operation: () => Promise, + rollback?: () => Promise, + signal?: AbortSignal, +): Promise; diff --git a/sdk/typescript/tsconfig.build.json b/sdk/typescript/tsconfig.build.json index e36d09dd8d..ccd5ab1710 100644 --- a/sdk/typescript/tsconfig.build.json +++ b/sdk/typescript/tsconfig.build.json @@ -1,6 +1,6 @@ { "extends": "./tsconfig.json", - "include": ["src/**/*.ts", "src/**/*.tsx"], + "include": ["src/**/*.ts", "src/**/*.tsx", "src/retry-windows-rename.js"], "compilerOptions": { "rootDir": "src", "outDir": "dist",