fix(studio-server): undo of a studio edit keeps a save that landed just before it (#4524)

* fix(studio-server): undo of a studio edit keeps a save that landed just before it

* fix(studio-server): undo keeps a save across several studio writes and a whole-file follow-up

* fix(studio-server): undo claims survive studio writes that loop back and stale write receipts

* fix(studio-server): a claim forgets only the write receipts it walked through

* chore(studio-server): drop a claim comment the comment ratchet counts against the stacked base

* docs(studio-server): keep the claim walk comment under the line limit

* refactor(studio-server): pull the pending-hash keep set out of the budget fold

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

@@ -105,6 +105,7 @@ function applyOneMutation(
     path: mutation.sourceFile,
     version,
     writeToken,
+    overwrote: before,
   });
   return {
     ...mutation,

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

@@ -9,6 +9,8 @@ export interface FileWriteReceipt {
 
 interface StoredReceipt extends FileWriteReceipt {
   recordedAt: number;
+  /** The bytes this write replaced, so the project history can keep a save that landed just before it. */
+  overwrote?: string | Uint8Array;
 }
 
 const RECEIPT_TTL_MS = 10_000;
@@ -48,14 +50,19 @@ export function createWriteToken(requestToken?: string): string {
   return token && token.length <= 200 ? token : randomUUID();
 }
 
-export function recordFileWriteReceipt(filePath: string, receipt: FileWriteReceipt): void {
+export function recordFileWriteReceipt(
+  filePath: string,
+  receipt: Omit<StoredReceipt, "recordedAt">,
+): void {
   const absPath = realFilePath(filePath);
   const now = Date.now();
-  const current = (receipts.get(absPath) ?? []).filter(
-    (entry) => now - entry.recordedAt < RECEIPT_TTL_MS,
-  );
-  current.push({ ...receipt, recordedAt: now });
-  receipts.set(absPath, current);
+  // Every path's expired receipts go, not just this one's: a receipt can hold a whole file's bytes.
+  for (const [path, list] of receipts) {
+    const live = list.filter((entry) => now - entry.recordedAt < RECEIPT_TTL_MS);
+    if (live.length > 0) receipts.set(path, live);
+    else receipts.delete(path);
+  }
+  receipts.set(absPath, [...(receipts.get(absPath) ?? []), { ...receipt, recordedAt: now }]);
 }
 
 export function clearFileWriteReceipt(filePath: string, version: string, writeToken: string): void {
@@ -79,7 +86,27 @@ export function identifyFileWrite(
   filePath: string,
   expectedVersion: string,
 ): FileWriteReceipt | null {
-  const absPath = realFilePath(filePath);
+  const receipt = newestReceipt(realFilePath(filePath), expectedVersion);
+  if (!receipt) return null;
+  const { path, version, writeToken } = receipt;
+  return { path, version, writeToken };
+}
+
+/** The bytes the API write of `version` replaced, while its receipt lives. */
+export function bytesOverwrittenBy(
+  filePath: string,
+  version: string,
+): string | Uint8Array | undefined {
+  return newestReceipt(realFilePath(filePath), version)?.overwrote;
+}
+
+/** Drops the replaced bytes a claim walked through, so a later claim can't walk back through them. */
+export function forgetOverwrittenBytes(filePath: string, versions: ReadonlySet<string>): void {
+  for (const receipt of receipts.get(realFilePath(filePath)) ?? [])
+    if (versions.has(receipt.version)) delete receipt.overwrote;
+}
+
+function newestReceipt(absPath: string, expectedVersion: string): StoredReceipt | undefined {
   const now = Date.now();
   const current = (receipts.get(absPath) ?? []).filter(
     (entry) => now - entry.recordedAt < RECEIPT_TTL_MS,
@@ -94,9 +121,7 @@ export function identifyFileWrite(
   for (let i = current.length - 1; i >= 0 && !receipt; i -= 1) {
     if (current[i]?.version === expectedVersion) receipt = current[i];
   }
-  if (!receipt) return null;
-  const { path, version, writeToken } = receipt;
-  return { path, version, writeToken };
+  return receipt;
 }
 
 /**

packages/studio-server/src/history/projectHistory.test.ts

@@ -18,7 +18,7 @@ import { spawnSync } from "node:child_process";
 import { tmpdir } from "node:os";
 import { basename, dirname, join, relative } from "node:path";
 import { afterEach, describe, expect, it, vi } from "vitest";
-import { fileContentVersion } from "../helpers/fileVersion";
+import { fileContentVersion, recordFileWriteReceipt } from "../helpers/fileVersion";
 import { HistoryBusyError } from "./ownerLock";
 import { openProjectHistory, type ProjectHistory } from "./projectHistory";
 import { START, type HistoryWho } from "./historyLog";
@@ -847,6 +847,84 @@ describe("openProjectHistory", () => {
     expect((await window.close())?.id).toBe(window.id);
   });
 
+  it("keeps the bytes a claim cut at through a budget fold that runs before they are logged", async () => {
+    const saved = "B".repeat(3000);
+    const { history, write, read, projectDir } = await project(
+      { "index.html": "a", "other.html": "o" },
+      { budgetBytes: 2000 },
+    );
+    await change(history, you, "Old", () => write("other.html", "o2"));
+    history.pin((await change(history, you, "Pinned", () => write("other.html", "o3"))).id, true);
+    write("other.html", "o4");
+    await history.claim(you, "Drag", ["other.html"], { coalesceKey: "drag" });
+    // Studio's write of `edited` replaced a save it never read.
+    const edited = `${saved}!`;
+    write("index.html", edited);
+    recordFileWriteReceipt(join(projectDir, "index.html"), {
+      path: "index.html",
+      version: fileContentVersion(edited),
+      writeToken: "studio",
+      overwrote: saved,
+    });
+
+    const edit = await history.claim(you, "Edit", ["index.html"], {
+      overwrote: { "index.html": fileContentVersion("a") },
+    });
+    expect((await history.undo(edit!.id, { who: you })).ok).toBe(true);
+    expect(read("index.html")).toBe(saved);
+  });
+
+  it("cuts a claim at a restored save, not at bytes an earlier claim already used", async () => {
+    const { history, write, read, projectDir } = await project({ "index.html": "W" });
+    const studioWrites = (content: string, overwrote: string) => {
+      write("index.html", content);
+      recordFileWriteReceipt(join(projectDir, "index.html"), {
+        path: "index.html",
+        version: fileContentVersion(content),
+        writeToken: "studio",
+        overwrote,
+      });
+    };
+    const told = (content: string) => ({
+      overwrote: { "index.html": fileContentVersion(content) },
+    });
+    studioWrites("X", "W");
+    await history.claim(you, "First", ["index.html"], told("W"));
+    studioWrites("Y", "X");
+    await history.claim(you, "Second", ["index.html"], told("X"));
+    // An editor restores X; Studio, still holding Y, patches it before the history sees X.
+    write("index.html", "X");
+    studioWrites("Z", "X");
+
+    const third = await history.claim(you, "Third", ["index.html"], told("Y"));
+    expect((await history.undo(third!.id, { who: you })).ok).toBe(true);
+    expect(read("index.html")).toBe("X");
+  });
+
+  it("keeps what a Studio write landing during a claim replaced, for the next claim", async () => {
+    const { history, write, read, projectDir } = await project({ "index.html": "W" });
+    const receipt = (content: string, overwrote: string) =>
+      recordFileWriteReceipt(join(projectDir, "index.html"), {
+        path: "index.html",
+        version: fileContentVersion(content),
+        writeToken: "studio",
+        overwrote,
+      });
+    const told = (content: string) => ({
+      overwrote: { "index.html": fileContentVersion(content) },
+    });
+    write("index.html", "X");
+    receipt("X", "W");
+    // Studio's next write, over an editor's save B, lands after this claim's scan.
+    receipt("Y", "B");
+    await history.claim(you, "First", ["index.html"], told("W"));
+    write("index.html", "Y");
+
+    const second = await history.claim(you, "Second", ["index.html"], told("X"));
+    expect((await history.undo(second!.id, { who: you })).ok).toBe(true);
+    expect(read("index.html")).toBe("B");
+  });
+
   it("keeps the history of a small edit in a project larger than its budget", async () => {
     const { history, write } = await project(
       { "media.bin": Buffer.alloc(4096, 1), "index.html": "a" },

packages/studio-server/src/history/projectHistory.ts

@@ -1,5 +1,5 @@
 import { randomUUID } from "node:crypto";
-import { readdir, rm, rmdir } from "node:fs/promises";
+import { readdir, rm, rmdir, writeFile } from "node:fs/promises";
 import {
   type Dirent,
   type Stats,
@@ -15,7 +15,10 @@ import { homedir } from "node:os";
 import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path";
 import { replaceFileAtomically } from "../helpers/atomicFile.js";
 import {
+  bytesOverwrittenBy,
   DELETED_VERSION,
+  fileContentVersion,
+  forgetOverwrittenBytes,
   hashOfVersion,
   hashVersion,
   recordFileWriteReceipt,
@@ -439,7 +442,7 @@ class Engine {
     { coalesceKey, idleMs, overwrote = {} }: ClaimOptions,
   ): Promise<{ id: string } | null> {
     await this.sweep();
-    const taken = this.takeClaimed(who, paths, overwrote);
+    const taken = await this.takeClaimed(who, paths, overwrote);
     if (!taken.length) return this.claimNothing(coalesceKey);
     const held = this.heldFor(coalesceKey, taken);
     if (!held) await this.commitClaim();
@@ -493,11 +496,11 @@ class Engine {
   }
 
   /** Takes `paths`' uncommitted changes from others, cut at what `who` overwrote. */
-  takeClaimed(
+  async takeClaimed(
     who: HistoryWho,
     paths: readonly string[],
     overwrote: Readonly<Record<string, string>>,
-  ): HistoryFileChange[] {
+  ): Promise<HistoryFileChange[]> {
     const wanted = new Set(paths.map((path) => this.logPath(path)));
     const at = new Map(
       Object.entries(overwrote).map(([path, version]) => [
@@ -507,15 +510,48 @@ class Engine {
     );
     const others = this.windows.filter((open) => !sameWho(open.who, who));
     const groups = this.outside ? [this.outside, ...others] : others;
-    const taken: HistoryFileChange[] = [];
-    for (const group of groups) {
-      for (const change of [...group.changes.values()]) {
-        if (!wanted.has(change.path)) continue;
-        const claimed = this.cutOut(group, change, at.get(change.path));
-        if (claimed) taken.push(claimed);
-      }
+    const hits = groups.flatMap((group) =>
+      [...group.changes.values()]
+        .filter((change) => wanted.has(change.path))
+        .map((change) => ({ group, change })),
+    );
+    const cuts: Array<string | undefined> = [];
+    const walked = hits.map(() => new Set<string>());
+    for (const [i, { change }] of hits.entries()) {
+      const told = at.get(change.path);
+      cuts.push((await this.overwrittenBy(change, told, walked[i]!)) ?? told);
+    }
+    hits.forEach(({ change }, i) =>
+      forgetOverwrittenBytes(
+        join(this.dir, change.path),
+        new Set([...walked[i]!].map(hashVersion)),
+      ),
+    );
+    return hits.flatMap(({ group, change }, i) => this.cutOut(group, change, cuts[i]) ?? []);
+  }
+
+  /** Walks the server's writes back from `change.after` to the bytes they replaced, stored so an undo restores them. */
+  async overwrittenBy(change: HistoryFileChange, told: string | undefined, seen: Set<string>) {
+    const absPath = join(this.dir, change.path);
+    let bytes: string | Uint8Array | undefined;
+    for (let hash = change.after; hash; ) {
+      // A loop (X, Y, back to X) says nothing about what was there first: the client's word stands.
+      if (seen.has(hash)) return undefined;
+      seen.add(hash);
+      const replaced = bytesOverwrittenBy(absPath, hashVersion(hash));
+      if (replaced === undefined) break;
+      bytes = replaced;
+      hash = hashOfVersion(fileContentVersion(replaced))!;
+      if (hash === change.before || hash === told) return hash;
+    }
+    if (bytes === undefined) return undefined;
+    const staged = join(this.home, `overwrote-${randomUUID()}`);
+    await writeFile(staged, bytes);
+    try {
+      return await this.blobs.put(staged);
+    } finally {
+      await rm(staged, { force: true });
     }
-    return taken;
   }
 
   cutOut(group: Group, change: HistoryFileChange, cut: string | undefined) {
@@ -615,11 +651,23 @@ class Engine {
     let folded = false;
     while (this.historyBytes() > budget && foldOldest(this.log)) {
       folded = true;
-      await this.blobs.prune(referencedHashes(this.log, this.manifest()));
+      await this.blobs.prune(
+        new Set([...referencedHashes(this.log, this.manifest()), ...this.pendingHashes()]),
+      );
     }
     if (folded) writeLog(this.logFile, this.log);
   }
 
+  /** Hashes only pending changes point at yet, such as a claim's stored cut. */
+  pendingHashes(): string[] {
+    const changes = [...this.windows, this.outside, this.claimed?.group].flatMap((group) => [
+      ...(group?.changes.values() ?? []),
+    ]);
+    return changes
+      .flatMap((change) => [change.before, change.after])
+      .filter((hash): hash is string => hash !== null);
+  }
+
   /** Before an operation or a window: every write is filed and every pending change committed, in write order. */
   async settle(): Promise<void> {
     await this.sweep();

packages/studio-server/src/routes/files.ts

@@ -490,27 +490,19 @@ function commitElementPatchBatchesWithReceipts(
  * the stage for a few hundred milliseconds right after the user typed. Every
  * mutation route records through here so no route can forget.
  */
-function recordMutationReceipt(
-  c: RouteContext,
-  filePath: string,
-  absPath: string,
-  html: string,
-): { version: string; writeToken: string } {
-  const version = fileContentVersion(html);
-  const writeToken = createWriteToken(c.req.header("X-Hyperframes-Write-Token"));
-  recordFileWriteReceipt(absPath, { path: filePath, version, writeToken });
-  return { version, writeToken };
-}
-
 function writeFileWithReceipt(
   c: RouteContext,
   filePath: string,
   absPath: string,
   html: string,
 ): { version: string; writeToken: string } {
+  const overwrote = readFileSync(absPath);
   replaceFileAtomically(absPath, html, statSync(absPath).mode);
   // The synchronous write cannot yield before its receipt is recorded; keep this block await-free.
-  return recordMutationReceipt(c, filePath, absPath, html);
+  const version = fileContentVersion(html);
+  const writeToken = createWriteToken(c.req.header("X-Hyperframes-Write-Token"));
+  recordFileWriteReceipt(absPath, { path: filePath, version, writeToken, overwrote });
+  return { version, writeToken };
 }
 
 function writeMutationResult(
@@ -2430,6 +2422,7 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
     }
 
     let backup: ReturnType<typeof snapshotBeforeWrite> = { backupPath: null };
+    let overwrote: Buffer | undefined;
     if (createOnly) {
       ensureDir(res.absPath);
       let fd: number;
@@ -2475,6 +2468,7 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
       }
       try {
         const currentContent = readFileSync(fd);
+        overwrote = currentContent;
         const currentVersion = fileContentVersion(currentContent);
         if (expectedVersion !== currentVersion) {
           return c.json(
@@ -2499,7 +2493,7 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
     }
     const version = fileContentVersion(body);
     const writeToken = createWriteToken(c.req.header("X-Hyperframes-Write-Token"));
-    recordFileWriteReceipt(res.absPath, { path: res.filePath, version, writeToken });
+    recordFileWriteReceipt(res.absPath, { path: res.filePath, version, writeToken, overwrote });
     c.header("ETag", version);
 
     return c.json({
@@ -2791,6 +2785,7 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
             path: file.path,
             version: fileContentVersion(file.after),
             writeToken,
+            overwrote: file.before,
           });
         }
       } catch (error) {

packages/studio-server/src/routes/history.test.ts

@@ -130,6 +130,66 @@ describe("history routes", () => {
     expect((await (await call("")).json()).back).toMatchObject({ label: "Dragged Title" });
   });
 
+  it.each([
+    ["a save that landed between Studio's read and its write", ["save B", "patch"], "B"],
+    ["its start, when Studio wrote the edit as two patches", ["patch", "patch"], "A"],
+    ["a save before Studio's patch and its whole-file follow-up", ["save B", "patch", "put"], "B"],
+    [
+      "its start, when Studio's patches came back to earlier bytes",
+      ["patch", "patch", "unpatch"],
+      "A",
+    ],
+  ])("undo Studio's element edit back to %s", async (_, steps, undoneTo) => {
+    const { projectDir, history } = await demoProject();
+    const index = join(projectDir, "index.html");
+    const saved = (text: string) => `<h1 id="title">${text}</h1>`;
+    writeFileSync(index, saved("A"));
+    await history.flush();
+    const api = createStudioApi({
+      listProjects: () => [],
+      resolveProject: (id: string) => (id === "demo" ? { id, dir: projectDir } : null),
+      history: () => history,
+    } as unknown as StudioApiAdapter);
+    const post = (path: string, body: object) =>
+      api.request(`/projects/demo${path}`, { method: "POST", body: JSON.stringify(body) });
+
+    let zIndex = 1;
+    for (const step of steps) {
+      if (step === "save B") writeFileSync(index, saved("B"));
+      const current = readFileSync(index, "utf-8");
+      const written =
+        step === "patch" || step === "unpatch"
+          ? await post("/file-mutations/patch-element/index.html", {
+              target: { id: "title" },
+              operations: [
+                {
+                  type: "inline-style",
+                  property: "z-index",
+                  value: String(step === "patch" ? ++zIndex : --zIndex),
+                },
+              ],
+            })
+          : step === "put"
+            ? await api.request("/projects/demo/files/index.html", {
+                method: "PUT",
+                headers: { "If-Match": fileContentVersion(current) },
+                body: `${current}<style>@font-face{}</style>`,
+              })
+            : null;
+      expect(written?.status ?? 200).toBe(200);
+    }
+    await post("/history/claim", {
+      label: "Moved Title",
+      paths: ["index.html"],
+      overwrote: { "index.html": fileContentVersion(saved("A")) },
+    });
+    expect(await (await post("/history/step", { direction: "back" })).json()).toMatchObject({
+      ok: true,
+      entry: { label: "Undid: Moved Title" },
+    });
+    expect(readFileSync(index, "utf-8")).toBe(saved(undoneTo));
+  });
+
   it("label an undo's writes with Studio's write token, so their echo reads as Studio's own", async () => {
     const { projectDir, call } = await demoProject();
     writeFileSync(join(projectDir, "index.html"), "B");