fix(windows): retry publishing validated plugin ZIPs

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 (

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",

sdk/typescript/src/retry-windows-rename.d.ts

@@ -0,0 +1,5 @@
+export function retryWindowsRename(
+  operation: () => Promise<void>,
+  rollback?: () => Promise<void>,
+  signal?: AbortSignal,
+): Promise<void>;

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<void>} operation
+ * @param {() => Promise<void>} [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 });
+    }
+  }
+}

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,

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<void> {
+  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<string[]> {
   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",

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<void>) | 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(

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",