fix(studio-server): undo keeps an outside change even when Studio's history claim arrives late (#4631)

* test(studio-server): a write landing between Studio's read and its patch survives undo * test(studio-server): the outside write also survives a claim that arrives after receipts expire * fix(studio-server): undo keeps an outside change even when Studio's history claim arrives late * fix(studio-server): history forgets replaced bytes on delete and frees unused bytes past budget Also recreates the history folder before storing replaced bytes, hears writes inside the queued start, and pins the agent-turn claim that arrives after the receipt expired.
2026-10-30 09:16:55 +00:00 · 2026-09-28 23:48:13 +00:00
parent 7e0711505a
commit 381fb0fd9a
4 changed files with 266 additions and 56 deletions
@@ -9,12 +9,13 @@ 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;
 }

+type OverwriteListener = (absPath: string, version: string, overwrote: string | Uint8Array) => void;
+
 const RECEIPT_TTL_MS = 10_000;
 const receipts = new Map<string, StoredReceipt[]>();
+const overwriteListeners = new Set<OverwriteListener>();

 /** Strong content version used as both the JSON version and HTTP ETag. */
 export function fileContentVersion(content: string | Uint8Array): string {
@@ -50,13 +51,20 @@ export function createWriteToken(requestToken?: string): string {
  return token && token.length <= 200 ? token : randomUUID();
 }

+/** Hears the bytes each API write replaced, so a project history can keep a save it never saw. */
+export function onFileOverwritten(listener: OverwriteListener): () => void {
+  overwriteListeners.add(listener);
+  return () => overwriteListeners.delete(listener);
+}
+
 export function recordFileWriteReceipt(
  filePath: string,
-  receipt: Omit<StoredReceipt, "recordedAt">,
+  { overwrote, ...receipt }: FileWriteReceipt & { overwrote?: string | Uint8Array },
 ): void {
  const absPath = realFilePath(filePath);
+  if (overwrote !== undefined)
+    for (const listener of overwriteListeners) listener(absPath, receipt.version, overwrote);
  const now = Date.now();
-  // 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);
@@ -92,20 +100,6 @@ export function identifyFileWrite(
  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(
@@ -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, recordFileWriteReceipt } from "../helpers/fileVersion";
+import { fileContentVersion, hashOfVersion, recordFileWriteReceipt } from "../helpers/fileVersion";
 import { HistoryBusyError } from "./ownerLock";
 import { HistoryIdError } from "./historyId";
 import { HistoryClosedError, openProjectHistory, type ProjectHistory } from "./projectHistory";
@@ -1099,7 +1099,10 @@ 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 () => {
+  it.each([
+    ["while the claim runs", Infinity],
+    ["before the claim comes", 1],
+  ])("keeps the bytes a claim cut at through a budget fold %s", async (_when, dragIdleMs) => {
    const saved = "B".repeat(3000);
    const { history, write, read, projectDir } = await project(
      { "index.html": "a", "other.html": "o" },
@@ -1108,7 +1111,7 @@ describe("openProjectHistory", () => {
    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" });
+    await history.claim(you, "Drag", ["other.html"], { coalesceKey: "drag", idleMs: dragIdleMs });
    // Studio's write of `edited` replaced a save it never read.
    const edited = `${saved}!`;
    write("index.html", edited);
@@ -1118,6 +1121,7 @@ describe("openProjectHistory", () => {
      writeToken: "studio",
      overwrote: saved,
    });
+    await new Promise((settle) => setTimeout(settle, 20));

    const edit = await history.claim(you, "Edit", ["index.html"], {
      overwrote: { "index.html": fileContentVersion("a") },
@@ -1126,6 +1130,66 @@ describe("openProjectHistory", () => {
    expect(read("index.html")).toBe(saved);
  });

+  it("keeps the bytes an API write replaced when its history folder was removed while open", async () => {
+    const { history, write, read, projectDir, historyRoot } = await project({ "index.html": "a" });
+    rmSync(historyRoot, { recursive: true, force: true });
+    write("index.html", "E");
+    recordFileWriteReceipt(join(projectDir, "index.html"), {
+      path: "index.html",
+      version: fileContentVersion("E"),
+      writeToken: "studio",
+      overwrote: "S",
+    });
+
+    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("S");
+  });
+
+  it("forgets what an API write replaced once the file is removed", async () => {
+    const { history, write, read, projectDir } = await project({ "index.html": "a" });
+    write("index.html", "E");
+    recordFileWriteReceipt(join(projectDir, "index.html"), {
+      path: "index.html",
+      version: fileContentVersion("E"),
+      writeToken: "studio",
+      overwrote: "S",
+    });
+    rmSync(join(projectDir, "index.html"));
+    await history.claim(you, "sweep", []);
+    // Re-created outside Studio with the bytes the API wrote: nothing Studio wrote is on disk now.
+    write("index.html", "E");
+
+    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("a");
+  });
+
+  it("frees unreferenced bytes past its budget even when nothing is left to fold", async () => {
+    const saved = "B".repeat(3000);
+    const { history, write, projectDir } = await project(
+      { "index.html": "a", "other.html": "o" },
+      { budgetBytes: 2000 },
+    );
+    history.pin((await change(history, you, "Pinned", () => write("other.html", "o2"))).id, true);
+    const edited = `${saved}!`;
+    write("index.html", edited);
+    recordFileWriteReceipt(join(projectDir, "index.html"), {
+      path: "index.html",
+      version: fileContentVersion(edited),
+      writeToken: "agent",
+      overwrote: saved,
+    });
+    write("index.html", "x");
+    await history.flush();
+
+    await expect(history.readBlob(hashOfVersion(fileContentVersion(saved))!)).rejects.toThrow();
+  });
+
  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) => {
@@ -1153,7 +1217,7 @@ describe("openProjectHistory", () => {
    expect(read("index.html")).toBe("X");
  });

-  it("keeps what a Studio write landing during a claim replaced, for the next claim", async () => {
+  it("keeps what a Studio write replaced over an unseen save, 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"), {
@@ -1167,16 +1231,91 @@ describe("openProjectHistory", () => {
    });
    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"));
+    // Studio's next write lands over an editor's save B that the history never saw.
    write("index.html", "Y");
+    receipt("Y", "B");

    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.each([
+    ["once its change is committed", true],
+    ["once an editor wrote over it", false],
+  ])(
+    "forgets what an API write replaced %s, so a later claim cannot cut at it",
+    async (_when, commit) => {
+      const { history, write, read, projectDir } = await project({ "index.html": "W" });
+      write("index.html", "X");
+      recordFileWriteReceipt(join(projectDir, "index.html"), {
+        path: "index.html",
+        version: fileContentVersion("X"),
+        writeToken: "agent",
+        overwrote: "W",
+      });
+      if (commit) await history.flush();
+      write("index.html", "C");
+      await history.flush();
+      // An editor, not the API, brings X back.
+      write("index.html", "X");
+
+      const edit = await history.claim(you, "Edit", ["index.html"], {
+        overwrote: { "index.html": fileContentVersion("C") },
+      });
+      expect((await history.undo(edit!.id, { who: you })).ok).toBe(true);
+      expect(read("index.html")).toBe("C");
+    },
+  );
+
+  it("cuts at an editor's save, not at what a rolled-back API write replaced", async () => {
+    const { history, write, read, projectDir } = await project({ "index.html": "W" });
+    const apiWrites = (content: string, overwrote?: string) => {
+      write("index.html", content);
+      recordFileWriteReceipt(join(projectDir, "index.html"), {
+        path: "index.html",
+        version: fileContentVersion(content),
+        writeToken: "api",
+        overwrote,
+      });
+    };
+    apiWrites("X", "W");
+    apiWrites("W");
+    apiWrites("K", "W");
+    await history.claim(you, "First", ["index.html"], {
+      overwrote: { "index.html": fileContentVersion("W") },
+    });
+    write("index.html", "X");
+    apiWrites("P", "X");
+
+    const second = await history.claim(you, "Second", ["index.html"], {
+      overwrote: { "index.html": fileContentVersion("K") },
+    });
+    expect((await history.undo(second!.id, { who: you })).ok).toBe(true);
+    expect(read("index.html")).toBe("X");
+  });
+
+  it("lets the budget free the bytes a committed API write replaced", async () => {
+    const saved = "B".repeat(3000);
+    const { history, write, projectDir } = await project(
+      { "index.html": "a", "other.html": "o" },
+      { budgetBytes: 2000 },
+    );
+    const edited = `${saved}!`;
+    write("index.html", edited);
+    recordFileWriteReceipt(join(projectDir, "index.html"), {
+      path: "index.html",
+      version: fileContentVersion(edited),
+      writeToken: "agent",
+      overwrote: saved,
+    });
+    await history.flush();
+
+    await change(history, you, "Other", () => write("other.html", "o2"));
+    expect(history.list().map((entry) => entry.label)).toContain("Other");
+  });
+
  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" },
@@ -1,5 +1,5 @@
 import { randomUUID } from "node:crypto";
-import { readdir, rm, rmdir, writeFile } from "node:fs/promises";
+import { mkdir, readdir, rm, rmdir, writeFile } from "node:fs/promises";
 import {
  type Dirent,
  type Stats,
@@ -16,14 +16,14 @@ 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,
+  onFileOverwritten,
  recordFileWriteReceipt,
 } from "../helpers/fileVersion.js";
+import { realFilePath } from "../helpers/safePath.js";
 import { affectsProjectSignature, listProjectFiles } from "../helpers/projectSignature.js";
 import { openBlobStore, type BlobStore } from "./blobStore.js";
 import { pruneGoneProjectHistoriesDaily } from "./pruneHistories.js";
@@ -260,6 +260,21 @@ const blocks = (removed: string, added: string) =>
 const takesWrite = (window: Group, at: number) =>
  window.idleMs === undefined || at - (window.lastWriteAt ?? at) <= window.idleMs;

+function chainBackFrom(
+  byAfter: Map<string, string> | undefined,
+  hash: string,
+): Map<string, string> {
+  const chain = new Map<string, string>();
+  let at = hash;
+  let prior = byAfter?.get(at);
+  while (prior !== undefined && !chain.has(at)) {
+    chain.set(at, prior);
+    at = prior;
+    prior = byAfter?.get(at);
+  }
+  return chain;
+}
+
 /** Files one change to a group; a later change to the same path keeps the group's first "before". */
 function addChange(group: Group, path: string, before: string | null, after: string | null): void {
  const earlier = group.changes.get(path);
@@ -279,6 +294,9 @@ class Engine {
  outside: Group | null = null;
  /** A coalescing claim, open until another key, its idle timer, an operation, a window, or another write. */
  claimed: { group: Group; key: string; timer?: NodeJS.Timeout } | null = null;
+  /** Per path and hash an API write left, the hash of the bytes it replaced, until walked or written past. */
+  overwritten = new Map<string, Map<string, string>>();
+  stopHearing: (() => void) | undefined;
  writeToken: string | undefined;
  quietTimer: NodeJS.Timeout | undefined;
  maxTimer: NodeJS.Timeout | undefined;
@@ -374,6 +392,46 @@ class Engine {
    await this.settleAll();
  }

+  hearWrites(): void {
+    const realDir = realFilePath(this.dir);
+    this.stopHearing = onFileOverwritten((absPath, version, bytes) => {
+      const after = hashOfVersion(version);
+      if (!after || !affectsProjectSignature(realDir, absPath)) return;
+      const replaced = hashOfVersion(fileContentVersion(bytes));
+      if (!replaced || replaced === after) return;
+      const path = relative(realDir, absPath).split(sep).join("/");
+      // One chain per file, a new Map per write (forgetWritesBefore checks identity); off-chain notes were overwritten.
+      this.overwritten.set(
+        path,
+        chainBackFrom(this.overwritten.get(path), replaced).set(after, replaced),
+      );
+      this.queue(async () => {
+        if (!this.blobs.has(replaced)) await this.storeBytes(bytes);
+      }).catch((error) => this.options.onError?.(error));
+    });
+  }
+
+  async storeBytes(bytes: string | Uint8Array): Promise<void> {
+    const staged = join(this.home, `overwrote-${randomUUID()}`);
+    await mkdir(this.home, { recursive: true });
+    await writeFile(staged, bytes);
+    try {
+      await this.blobs.put(staged);
+    } finally {
+      await rm(staged, { force: true });
+    }
+  }
+
+  /** Read at a state no API write left, with no write heard since `heard`: no walk passes it, so forget the rest. */
+  forgetWritesBefore(
+    path: string,
+    hash: string | null,
+    heard: Map<string, string> | undefined,
+  ): void {
+    if (this.overwritten.get(path) === heard && !(hash && heard?.has(hash)))
+      this.overwritten.delete(path);
+  }
+
  reopenClosedWindow(): void {
    const closed = this.options.closedWindow;
    if (!closed || this.log.entries.some((entry) => entry.id === closed.id)) return;
@@ -423,6 +481,7 @@ class Engine {
    const changedAt = (file: { mtimeMs: number; ctimeMs: number }) =>
      Math.min(sweptAt, Math.max(file.mtimeMs, file.ctimeMs));
    const seen = listProjectFiles(this.dir);
+    const heard = new Map(this.overwritten);
    const present = new Set(seen.map((file) => file.path));
    const removed = [...this.tracked.keys()]
      .filter((path) => !present.has(path))
@@ -450,6 +509,7 @@ class Engine {
      }
      const known = this.tracked.get(path)!;
      this.tracked.delete(path);
+      this.forgetWritesBefore(path, null, heard.get(path));
      await this.record(path, known.hash, null, at);
      changed = true;
    }
@@ -469,9 +529,11 @@ class Engine {
  async observe(path: string, stat: string, at: number): Promise<boolean> {
    const known = this.tracked.get(path) ?? { hash: null, stat: null };
    if (stat && known.stat === stat) return false;
+    const heard = this.overwritten.get(path);
    const hash = await this.storeIfPresent(path);
    if (hash === null || this.whereFolder() !== "here") return false;
    this.tracked.set(path, { hash, stat });
+    this.forgetWritesBefore(path, hash, heard);
    if (known.hash !== hash) await this.record(path, known.hash, hash, at);
    return known.hash !== hash || known.stat !== stat;
  }
@@ -575,39 +637,31 @@ class Engine {
    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);
+      cuts.push(this.overwrittenBy(change, told, walked[i]!) ?? told);
    }
-    hits.forEach(({ change }, i) =>
-      forgetOverwrittenBytes(
-        join(this.dir, change.path),
-        new Set([...walked[i]!].map(hashVersion)),
-      ),
-    );
+    hits.forEach(({ change }, i) => this.forgetOverwritten(change.path, walked[i]!));
    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;
+  /** Walks the server's writes back from `change.after` to the stored bytes they replaced. */
+  overwrittenBy(change: HistoryFileChange, told: string | undefined, seen: Set<string>) {
+    let found: string | 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));
+      const replaced = this.overwritten.get(change.path)?.get(hash);
      if (replaced === undefined) break;
-      bytes = replaced;
-      hash = hashOfVersion(fileContentVersion(replaced))!;
+      found = hash = 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 found;
+  }
+
+  forgetOverwritten(path: string, hashes: Iterable<string>): void {
+    const byAfter = this.overwritten.get(path);
+    for (const hash of hashes) byAfter?.delete(hash);
+    if (byAfter?.size === 0) this.overwritten.delete(path);
  }

  cutOut(group: Group, change: HistoryFileChange, cut: string | undefined) {
@@ -693,6 +747,11 @@ class Engine {
      throw error;
    }
    for (const listener of this.listeners) listener(entry);
+    for (const change of group.changes.values()) {
+      const walked = new Set<string>();
+      this.overwrittenBy(change, undefined, walked);
+      this.forgetOverwritten(change.path, walked);
+    }
    await this.keepWithinBudget();
    return entry;
  }
@@ -708,11 +767,16 @@ class Engine {
  async keepWithinBudget(): Promise<void> {
    const budget = this.options.budgetBytes ?? 2 * 1024 ** 3;
    let folded = false;
-    while (this.historyBytes() > budget && foldOldest(this.log)) {
-      folded = true;
+    while (this.historyBytes() > budget) {
      await this.blobs.prune(
-        new Set([...referencedHashes(this.log, this.manifest()), ...this.pendingHashes()]),
+        new Set([
+          ...referencedHashes(this.log, this.manifest()),
+          ...this.pendingHashes(),
+          ...[...this.overwritten.values()].flatMap((byAfter) => [...byAfter.values()]),
+        ]),
      );
+      if (this.historyBytes() <= budget || !foldOldest(this.log)) break;
+      folded = true;
    }
    if (folded) this.persistLog();
  }
@@ -1045,6 +1109,7 @@ class Engine {
      close: () =>
        (this.closing ??= (async () => {
          if (this.notedTimer) clearTimeout(this.notedTimer);
+          this.stopHearing?.();
          const settled = this.queue(() => this.settleAll());
          this.closed = true;
          await settled;
@@ -1086,7 +1151,10 @@ export async function openProjectHistory(options: ProjectHistoryOptions): Promis
  try {
    const blobs = await openBlobStore(join(options.historyRoot, projectId, "blobs"));
    const engine = new Engine(options, projectId, blobs);
-    await engine.queue(() => engine.start());
+    await engine.queue(async () => {
+      await engine.start();
+      engine.hearWrites();
+    });
    pruneGoneProjectHistoriesDaily(options.historyRoot, engine.now(), {
      onError: options.onError,
      budgetMs: options.pruneGoneProjectsBudgetMs,
@@ -49,9 +49,14 @@ async function demoProject({ quietMs }: { quietMs?: number } = {}) {
 }

 describe("history routes", () => {
-  it.each(["outside", "agent turn"])(
-    "keep the %s write that landed between Studio's read and its patch when Studio's edit is undone",
-    async (writer) => {
+  it.each([
+    ["outside", 0],
+    ["agent turn", 0],
+    ["outside", 11_000],
+    ["agent turn", 11_000],
+  ])(
+    "keep the %s write that landed between Studio's read and its patch when Studio's edit is undone (claim %d ms later)",
+    async (writer, claimDelayMs) => {
      const projectDir = tempDir("hf-history-outside-patch-");
      const file = join(projectDir, "index.html");
      writeFileSync(file, '<h1 id="title">A</h1>');
@@ -81,6 +86,10 @@ describe("history routes", () => {
        }),
      });
      expect(await patched.json()).toMatchObject({ ok: true, changed: true });
+      if (claimDelayMs) {
+        vi.useFakeTimers({ toFake: ["Date"], now: Date.now() + claimDelayMs });
+        cleanup.push(() => vi.useRealTimers());
+      }
      await call("/claim", {
        label: "Color",
        paths: ["index.html"],