fix(studio-server): a failed save reports its real error when a later file was never written (#5364)

* fix(studio-server): a failed save reports its real error when a later file was never written

* refactor(studio-server): share the file writer type and harden rollback test cleanup

.fallowrc.jsonc

@@ -538,6 +538,7 @@
       "packages/engine/src/services/extractionCache.test.ts",
       "packages/studio-server/src/routes/preview.test.ts",
       "packages/studio-server/src/routes/projects.test.ts",
+      "packages/studio-server/src/helpers/applyFileMutations.test.ts",
       "packages/studio-server/src/helpers/backupJournal.test.ts",
       "packages/studio-server/src/helpers/finiteMutation.test.ts",
       "packages/studio-server/src/helpers/hfIdPersist.test.ts",

packages/studio-server/src/helpers/applyFileMutations.test.ts

@@ -1,4 +1,12 @@
-import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import {
+  chmodSync,
+  existsSync,
+  mkdirSync,
+  mkdtempSync,
+  readFileSync,
+  rmSync,
+  writeFileSync,
+} from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
 import { describe, expect, it } from "vitest";
@@ -171,4 +179,69 @@ describe("applyFileMutations", () => {
       rmSync(projectDir, { recursive: true, force: true });
     }
   });
+
+  it.skipIf(process.platform === "win32" || process.getuid?.() === 0)(
+    "reports the write error, not a failed rollback, when a later file was never written",
+    () => {
+      resetFileWriteReceipts();
+      const projectDir = mkdtempSync(join(tmpdir(), "hf-mutation-readonly-"));
+      const first = join(projectDir, "first.html");
+      const lockedDir = join(projectDir, "locked");
+      const second = join(lockedDir, "second.html");
+      try {
+        mkdirSync(lockedDir);
+        writeFileSync(first, "first-before", "utf8");
+        writeFileSync(second, "second-before", "utf8");
+        chmodSync(lockedDir, 0o555);
+        let thrown: unknown;
+        try {
+          applyFileMutations(projectDir, [
+            { sourceFile: "first.html", absPath: first, after: "first-after" },
+            { sourceFile: "locked/second.html", absPath: second, after: "second-after" },
+          ]);
+        } catch (error) {
+          thrown = error;
+        }
+        expect(thrown).not.toBeInstanceOf(AggregateError);
+        expect(thrown).toMatchObject({ code: "EACCES" });
+        expect(readFileSync(first, "utf8")).toBe("first-before");
+        expect(readFileSync(second, "utf8")).toBe("second-before");
+        expect(identifyFileWrite(first, fileContentVersion("first-after"))).toBeNull();
+      } finally {
+        if (existsSync(lockedDir)) chmodSync(lockedDir, 0o755);
+        resetFileWriteReceipts();
+        rmSync(projectDir, { recursive: true, force: true });
+      }
+    },
+  );
+
+  it("still reports an incomplete rollback when a written file cannot be restored", () => {
+    const projectDir = mkdtempSync(join(tmpdir(), "hf-mutation-restore-"));
+    try {
+      for (const name of ["first", "second"])
+        writeFileSync(join(projectDir, `${name}.html`), `${name}-before`, "utf8");
+      let writes = 0;
+      expect(() =>
+        applyFileMutations(
+          projectDir,
+          ["first", "second"].map((name) => ({
+            sourceFile: `${name}.html`,
+            absPath: join(projectDir, `${name}.html`),
+            after: `${name}-after`,
+          })),
+          undefined,
+          (path, content, encoding) => {
+            writes += 1;
+            if (writes === 2) throw new Error("second write failed");
+            if (writes === 3) throw new Error("first restore failed");
+            writeFileSync(path, content, encoding);
+          },
+        ),
+      ).toThrow("File mutation failed and rollback did not complete");
+      expect(readFileSync(join(projectDir, "first.html"), "utf8")).toBe("first-after");
+    } finally {
+      resetFileWriteReceipts();
+      rmSync(projectDir, { recursive: true, force: true });
+    }
+  });
 });

packages/studio-server/src/helpers/applyFileMutations.ts

@@ -31,7 +31,7 @@ export function applyFileMutations(
   projectDir: string,
   mutations: readonly FileMutationInput[],
   requestToken?: string,
-  writeFile: (path: string, content: string, encoding: "utf-8") => void = (path, content) =>
+  writeFile: WriteFile = (path, content) =>
     replaceFileAtomically(path, content, statSync(path).mode),
 ): AppliedFileMutation[] {
   const prepared = mutations.map((mutation) => ({
@@ -39,9 +39,7 @@ export function applyFileMutations(
     before: mutation.before ?? readFileSync(mutation.absPath, "utf-8"),
   }));
   const results: AppliedFileMutation[] = [];
-  const attempted: Array<
-    PreparedMutation & { version: string; writeToken: string; written: boolean }
-  > = [];
+  const attempted: AttemptedMutation[] = [];
   for (const mutation of prepared)
     assertExpectedVersion(mutation, readFileSync(mutation.absPath, "utf-8"));
   try {
@@ -50,16 +48,7 @@ export function applyFileMutations(
     }
     return results;
   } catch (error) {
-    const rollbackErrors: unknown[] = [];
-    for (const mutation of attempted.reverse()) {
-      try {
-        if (!mutation.written || readFileSync(mutation.absPath, "utf-8") === mutation.after)
-          writeFile(mutation.absPath, mutation.before, "utf-8");
-        clearFileWriteReceipt(mutation.absPath, mutation.version, mutation.writeToken);
-      } catch (rollbackError) {
-        rollbackErrors.push(rollbackError);
-      }
-    }
+    const rollbackErrors = rollBack(attempted, writeFile);
     if (rollbackErrors.length > 0) {
       throw new AggregateError(
         [error, ...rollbackErrors],
@@ -71,13 +60,34 @@ export function applyFileMutations(
 }
 
 type PreparedMutation = FileMutationInput & { before: string };
+type WriteFile = (path: string, content: string, encoding: "utf-8") => void;
+type AttemptedMutation = PreparedMutation & {
+  version: string;
+  writeToken: string;
+  written: boolean;
+};
+
+function rollBack(attempted: AttemptedMutation[], writeFile: WriteFile): unknown[] {
+  const errors: unknown[] = [];
+  for (const mutation of attempted.reverse()) {
+    try {
+      const current = readFileSync(mutation.absPath, "utf-8");
+      if (mutation.written ? current === mutation.after : current !== mutation.before)
+        writeFile(mutation.absPath, mutation.before, "utf-8");
+      clearFileWriteReceipt(mutation.absPath, mutation.version, mutation.writeToken);
+    } catch (rollbackError) {
+      errors.push(rollbackError);
+    }
+  }
+  return errors;
+}
 
 function applyOneMutation(
   projectDir: string,
   mutation: PreparedMutation,
   requestToken: string | undefined,
-  writeFile: (path: string, content: string, encoding: "utf-8") => void,
-  attempted: Array<PreparedMutation & { version: string; writeToken: string; written: boolean }>,
+  writeFile: WriteFile,
+  attempted: AttemptedMutation[],
 ): AppliedFileMutation {
   const current = readFileSync(mutation.absPath, "utf-8");
   assertExpectedVersion(mutation, current);