test(artifacts): handle blocked open-parent renames (#7904)

* test(artifacts): handle blocked open-parent renames

* test(artifacts): exercise blocked parent rename cleanup

CONTRIBUTING.md

@@ -274,7 +274,10 @@ The general source suite (`npm run test:fast`, `npm run test:changed`, and
 On a Windows checkout, some source tests fail because fixtures use POSIX paths,
 `#!/bin/sh` tool stubs, or POSIX rename semantics. Compare failures against the
 base revision, or run the suite on Linux or WSL, before attributing them to a
-change. See [testing](docs/testing.md#developer-commands) for command scopes
+change. The embedded-artifact parent-swap tests exercise the selected-path
+replacement when the rename succeeds; if Windows refuses it while the admitted
+file is open, they instead verify the refusal and cleanup. See
+[testing](docs/testing.md#developer-commands) for command scopes
 and lane selection.
 
 `git submodule update --init --recursive` fails at the pinned Unblob submodule

src/artifacts/GodotPckArtifactReader.test.ts

@@ -7,7 +7,7 @@ import {
   pckU64,
   type GodotPckMember,
 } from "../../tests/fixtures/godotPck.js";
-import { mkdir, rename, writeFile } from "node:fs/promises";
+import { mkdir, writeFile } from "node:fs/promises";
 import { join } from "node:path";
 import { text } from "node:stream/consumers";
 import { once } from "node:events";
@@ -15,6 +15,7 @@ import { once } from "node:events";
 import { describe, expect, it } from "vitest";
 
 import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";
+import { renameOpenParent } from "../../tests/fixtures/renameOpenParent.js";
 import { openRegularFile } from "../filesystem/RegularFile.js";
 
 import type { ArtifactEntry } from "./ArtifactReader.js";
@@ -381,7 +382,7 @@ describe("GodotPckArtifactReader member streams", () => {
   });
 
   it.each([false, true])(
-    "reads located pack facts from the admitted descriptor after path replacement (embedded: %s)",
+    "reads located pack facts from the admitted descriptor after an attempted path replacement (embedded: %s)",
     async (embedded) => {
       const root = await createTestTempDirectory("rea-godot-borrowed-source-");
       const selected = join(root, "selected");
@@ -399,12 +400,14 @@ describe("GodotPckArtifactReader member streams", () => {
         if (location === undefined) throw new Error("expected a pack location");
         // Replace the selected directory without changing the admitted inode's
         // metadata. Moving the file itself would change its validated ctime.
-        await rename(selected, join(root, "original"));
-        await mkdir(selected);
-        await writeFile(
-          path,
-          buildGodotPck(3, [{ path: "other.gd", data: "" }]),
-        );
+        const swap = await renameOpenParent(selected, join(root, "original"));
+        if (swap.kind === "renamed") {
+          await mkdir(selected);
+          await writeFile(
+            path,
+            buildGodotPck(3, [{ path: "other.gd", data: "" }]),
+          );
+        }
         const reader = new GodotPckArtifactReader(path, {
           kind: "godot-pck",
           source: descriptor,

src/artifacts/extraction/EmbeddedArtifactExtraction.test.ts

@@ -1,7 +1,6 @@
 import {
   appendFile,
   mkdir,
-  rename,
   readFile,
   stat,
   symlink,
@@ -12,6 +11,7 @@ import { createHash } from "node:crypto";
 import { join } from "node:path";
 import { expect, it, vi } from "vitest";
 import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js";
+import { renameOpenParent } from "../../../tests/fixtures/renameOpenParent.js";
 import { buildGodotPck } from "../../../tests/fixtures/godotPck.js";
 import { ArtifactResourceScope } from "../ArtifactResourceScope.js";
 import { GodotPckArtifactReader } from "../GodotPckArtifactReader.js";
@@ -301,9 +301,14 @@ it.each(cases)(
   },
 );
 
-it.each(cases)(
-  "rejects a replacement at the admitted $format pathname",
-  async ({ inputFormat, reader, bytes }) => {
+it.each(
+  cases.flatMap((item) => [
+    { ...item, simulateWindowsRefusal: false },
+    { ...item, simulateWindowsRefusal: true },
+  ]),
+)(
+  "handles attempted replacement at the admitted $format pathname (simulated refusal: $simulateWindowsRefusal)",
+  async ({ inputFormat, reader, bytes, simulateWindowsRefusal }) => {
     const directory = await createTestTempDirectory(
       "rea-embedded-extraction-replacement-",
     );
@@ -311,34 +316,66 @@ it.each(cases)(
     await mkdir(selectedDirectory);
     const path = join(selectedDirectory, "source");
     const output = join(directory, "out");
-    await writeFile(path, bytes());
+    const originalBytes = bytes();
+    await writeFile(path, originalBytes);
     const scope = new ArtifactResourceScope();
     const originalClose = reader.prototype.close;
     let closes = 0;
+    let deniedRename: unknown;
     const close = vi
       .spyOn(reader.prototype, "close")
       .mockImplementation(async function (this: { close(): Promise<void> }) {
         await originalClose.call(this);
         if (++closes === 1) {
           // Moving the parent leaves the admitted regular file's metadata intact.
-          await rename(selectedDirectory, join(directory, "admitted"));
+          const swap = await renameOpenParent(
+            selectedDirectory,
+            join(directory, "admitted"),
+            simulateWindowsRefusal
+              ? {
+                  platform: "win32",
+                  rename: async () => {
+                    throw Object.assign(new Error("injected EPERM"), {
+                      code: "EPERM",
+                    });
+                  },
+                }
+              : {},
+          );
+          if (swap.kind === "blocked") {
+            deniedRename = swap.cause;
+            throw swap.cause;
+          }
           await mkdir(selectedDirectory);
           await writeFile(path, Buffer.from("replacement"));
         }
       });
     try {
-      await expect(
-        extractArtifact({
-          inputPath: path,
-          inputFormat,
-          outputRoot: output,
-          environment: {},
-          integrityPolicy: "fail",
-          resourceScope: scope,
-        }),
-      ).rejects.toMatchObject({ reason: "integrity" });
+      const failure = await extractArtifact({
+        inputPath: path,
+        inputFormat,
+        outputRoot: output,
+        environment: {},
+        integrityPolicy: "fail",
+        resourceScope: scope,
+      }).then(
+        () => undefined,
+        (cause: unknown) => cause,
+      );
+      if (deniedRename === undefined)
+        expect(failure).toMatchObject({ reason: "integrity" });
+      else
+        expect(failure).toMatchObject({
+          reason: "io",
+          message: expect.stringContaining("EPERM"),
+        });
       await expect(stat(output)).rejects.toMatchObject({ code: "ENOENT" });
       expect(closes).toBe(1);
+      if (deniedRename !== undefined) {
+        expect((await readFile(path)).equals(originalBytes)).toBe(true);
+        expect(scope.hasPending).toBe(true);
+        await scope.close();
+      }
       expect(scope.hasPending).toBe(false);
     } finally {
       await scope.close();

tests/fixtures/renameOpenParent.ts

@@ -0,0 +1,25 @@
+import { rename } from "node:fs/promises";
+
+/** Attempt the inode-preserving parent swap; Windows may refuse it while the child is open. */
+export const renameOpenParent = async (
+  from: string,
+  to: string,
+  options: {
+    readonly platform?: NodeJS.Platform;
+    readonly rename?: typeof rename;
+  } = {},
+): Promise<{ kind: "renamed" } | { kind: "blocked"; cause: unknown }> => {
+  try {
+    await (options.rename ?? rename)(from, to);
+    return { kind: "renamed" };
+  } catch (cause: unknown) {
+    if (
+      (options.platform ?? process.platform) === "win32" &&
+      cause instanceof Error &&
+      "code" in cause &&
+      cause.code === "EPERM"
+    )
+      return { kind: "blocked", cause };
+    throw cause;
+  }
+};