Skip to content

Commit 381fb0f

Browse files
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.
1 parent 7e07115 commit 381fb0f

4 files changed

Lines changed: 266 additions & 56 deletions

File tree

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

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ export interface FileWriteReceipt {
99

1010
interface StoredReceipt extends FileWriteReceipt {
1111
recordedAt: number;
12-
/** The bytes this write replaced, so the project history can keep a save that landed just before it. */
13-
overwrote?: string | Uint8Array;
1412
}
1513

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

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

54+
/** Hears the bytes each API write replaced, so a project history can keep a save it never saw. */
55+
export function onFileOverwritten(listener: OverwriteListener): () => void {
56+
overwriteListeners.add(listener);
57+
return () => overwriteListeners.delete(listener);
58+
}
59+
5360
export function recordFileWriteReceipt(
5461
filePath: string,
55-
receipt: Omit<StoredReceipt, "recordedAt">,
62+
{ overwrote, ...receipt }: FileWriteReceipt & { overwrote?: string | Uint8Array },
5663
): void {
5764
const absPath = realFilePath(filePath);
65+
if (overwrote !== undefined)
66+
for (const listener of overwriteListeners) listener(absPath, receipt.version, overwrote);
5867
const now = Date.now();
59-
// Every path's expired receipts go, not just this one's: a receipt can hold a whole file's bytes.
6068
for (const [path, list] of receipts) {
6169
const live = list.filter((entry) => now - entry.recordedAt < RECEIPT_TTL_MS);
6270
if (live.length > 0) receipts.set(path, live);
@@ -92,20 +100,6 @@ export function identifyFileWrite(
92100
return { path, version, writeToken };
93101
}
94102

95-
/** The bytes the API write of `version` replaced, while its receipt lives. */
96-
export function bytesOverwrittenBy(
97-
filePath: string,
98-
version: string,
99-
): string | Uint8Array | undefined {
100-
return newestReceipt(realFilePath(filePath), version)?.overwrote;
101-
}
102-
103-
/** Drops the replaced bytes a claim walked through, so a later claim can't walk back through them. */
104-
export function forgetOverwrittenBytes(filePath: string, versions: ReadonlySet<string>): void {
105-
for (const receipt of receipts.get(realFilePath(filePath)) ?? [])
106-
if (versions.has(receipt.version)) delete receipt.overwrote;
107-
}
108-
109103
function newestReceipt(absPath: string, expectedVersion: string): StoredReceipt | undefined {
110104
const now = Date.now();
111105
const current = (receipts.get(absPath) ?? []).filter(

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

Lines changed: 145 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { spawnSync } from "node:child_process";
1818
import { tmpdir } from "node:os";
1919
import { basename, dirname, join, relative } from "node:path";
2020
import { afterEach, describe, expect, it, vi } from "vitest";
21-
import { fileContentVersion, recordFileWriteReceipt } from "../helpers/fileVersion";
21+
import { fileContentVersion, hashOfVersion, recordFileWriteReceipt } from "../helpers/fileVersion";
2222
import { HistoryBusyError } from "./ownerLock";
2323
import { HistoryIdError } from "./historyId";
2424
import { HistoryClosedError, openProjectHistory, type ProjectHistory } from "./projectHistory";
@@ -1099,7 +1099,10 @@ describe("openProjectHistory", () => {
10991099
expect((await window.close())?.id).toBe(window.id);
11001100
});
11011101

1102-
it("keeps the bytes a claim cut at through a budget fold that runs before they are logged", async () => {
1102+
it.each([
1103+
["while the claim runs", Infinity],
1104+
["before the claim comes", 1],
1105+
])("keeps the bytes a claim cut at through a budget fold %s", async (_when, dragIdleMs) => {
11031106
const saved = "B".repeat(3000);
11041107
const { history, write, read, projectDir } = await project(
11051108
{ "index.html": "a", "other.html": "o" },
@@ -1108,7 +1111,7 @@ describe("openProjectHistory", () => {
11081111
await change(history, you, "Old", () => write("other.html", "o2"));
11091112
history.pin((await change(history, you, "Pinned", () => write("other.html", "o3"))).id, true);
11101113
write("other.html", "o4");
1111-
await history.claim(you, "Drag", ["other.html"], { coalesceKey: "drag" });
1114+
await history.claim(you, "Drag", ["other.html"], { coalesceKey: "drag", idleMs: dragIdleMs });
11121115
// Studio's write of `edited` replaced a save it never read.
11131116
const edited = `${saved}!`;
11141117
write("index.html", edited);
@@ -1118,6 +1121,7 @@ describe("openProjectHistory", () => {
11181121
writeToken: "studio",
11191122
overwrote: saved,
11201123
});
1124+
await new Promise((settle) => setTimeout(settle, 20));
11211125

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

1133+
it("keeps the bytes an API write replaced when its history folder was removed while open", async () => {
1134+
const { history, write, read, projectDir, historyRoot } = await project({ "index.html": "a" });
1135+
rmSync(historyRoot, { recursive: true, force: true });
1136+
write("index.html", "E");
1137+
recordFileWriteReceipt(join(projectDir, "index.html"), {
1138+
path: "index.html",
1139+
version: fileContentVersion("E"),
1140+
writeToken: "studio",
1141+
overwrote: "S",
1142+
});
1143+
1144+
const edit = await history.claim(you, "Edit", ["index.html"], {
1145+
overwrote: { "index.html": fileContentVersion("a") },
1146+
});
1147+
expect((await history.undo(edit!.id, { who: you })).ok).toBe(true);
1148+
expect(read("index.html")).toBe("S");
1149+
});
1150+
1151+
it("forgets what an API write replaced once the file is removed", async () => {
1152+
const { history, write, read, projectDir } = await project({ "index.html": "a" });
1153+
write("index.html", "E");
1154+
recordFileWriteReceipt(join(projectDir, "index.html"), {
1155+
path: "index.html",
1156+
version: fileContentVersion("E"),
1157+
writeToken: "studio",
1158+
overwrote: "S",
1159+
});
1160+
rmSync(join(projectDir, "index.html"));
1161+
await history.claim(you, "sweep", []);
1162+
// Re-created outside Studio with the bytes the API wrote: nothing Studio wrote is on disk now.
1163+
write("index.html", "E");
1164+
1165+
const edit = await history.claim(you, "Edit", ["index.html"], {
1166+
overwrote: { "index.html": fileContentVersion("a") },
1167+
});
1168+
expect((await history.undo(edit!.id, { who: you })).ok).toBe(true);
1169+
expect(read("index.html")).toBe("a");
1170+
});
1171+
1172+
it("frees unreferenced bytes past its budget even when nothing is left to fold", async () => {
1173+
const saved = "B".repeat(3000);
1174+
const { history, write, projectDir } = await project(
1175+
{ "index.html": "a", "other.html": "o" },
1176+
{ budgetBytes: 2000 },
1177+
);
1178+
history.pin((await change(history, you, "Pinned", () => write("other.html", "o2"))).id, true);
1179+
const edited = `${saved}!`;
1180+
write("index.html", edited);
1181+
recordFileWriteReceipt(join(projectDir, "index.html"), {
1182+
path: "index.html",
1183+
version: fileContentVersion(edited),
1184+
writeToken: "agent",
1185+
overwrote: saved,
1186+
});
1187+
write("index.html", "x");
1188+
await history.flush();
1189+
1190+
await expect(history.readBlob(hashOfVersion(fileContentVersion(saved))!)).rejects.toThrow();
1191+
});
1192+
11291193
it("cuts a claim at a restored save, not at bytes an earlier claim already used", async () => {
11301194
const { history, write, read, projectDir } = await project({ "index.html": "W" });
11311195
const studioWrites = (content: string, overwrote: string) => {
@@ -1153,7 +1217,7 @@ describe("openProjectHistory", () => {
11531217
expect(read("index.html")).toBe("X");
11541218
});
11551219

1156-
it("keeps what a Studio write landing during a claim replaced, for the next claim", async () => {
1220+
it("keeps what a Studio write replaced over an unseen save, for the next claim", async () => {
11571221
const { history, write, read, projectDir } = await project({ "index.html": "W" });
11581222
const receipt = (content: string, overwrote: string) =>
11591223
recordFileWriteReceipt(join(projectDir, "index.html"), {
@@ -1167,16 +1231,91 @@ describe("openProjectHistory", () => {
11671231
});
11681232
write("index.html", "X");
11691233
receipt("X", "W");
1170-
// Studio's next write, over an editor's save B, lands after this claim's scan.
1171-
receipt("Y", "B");
11721234
await history.claim(you, "First", ["index.html"], told("W"));
1235+
// Studio's next write lands over an editor's save B that the history never saw.
11731236
write("index.html", "Y");
1237+
receipt("Y", "B");
11741238

11751239
const second = await history.claim(you, "Second", ["index.html"], told("X"));
11761240
expect((await history.undo(second!.id, { who: you })).ok).toBe(true);
11771241
expect(read("index.html")).toBe("B");
11781242
});
11791243

1244+
it.each([
1245+
["once its change is committed", true],
1246+
["once an editor wrote over it", false],
1247+
])(
1248+
"forgets what an API write replaced %s, so a later claim cannot cut at it",
1249+
async (_when, commit) => {
1250+
const { history, write, read, projectDir } = await project({ "index.html": "W" });
1251+
write("index.html", "X");
1252+
recordFileWriteReceipt(join(projectDir, "index.html"), {
1253+
path: "index.html",
1254+
version: fileContentVersion("X"),
1255+
writeToken: "agent",
1256+
overwrote: "W",
1257+
});
1258+
if (commit) await history.flush();
1259+
write("index.html", "C");
1260+
await history.flush();
1261+
// An editor, not the API, brings X back.
1262+
write("index.html", "X");
1263+
1264+
const edit = await history.claim(you, "Edit", ["index.html"], {
1265+
overwrote: { "index.html": fileContentVersion("C") },
1266+
});
1267+
expect((await history.undo(edit!.id, { who: you })).ok).toBe(true);
1268+
expect(read("index.html")).toBe("C");
1269+
},
1270+
);
1271+
1272+
it("cuts at an editor's save, not at what a rolled-back API write replaced", async () => {
1273+
const { history, write, read, projectDir } = await project({ "index.html": "W" });
1274+
const apiWrites = (content: string, overwrote?: string) => {
1275+
write("index.html", content);
1276+
recordFileWriteReceipt(join(projectDir, "index.html"), {
1277+
path: "index.html",
1278+
version: fileContentVersion(content),
1279+
writeToken: "api",
1280+
overwrote,
1281+
});
1282+
};
1283+
apiWrites("X", "W");
1284+
apiWrites("W");
1285+
apiWrites("K", "W");
1286+
await history.claim(you, "First", ["index.html"], {
1287+
overwrote: { "index.html": fileContentVersion("W") },
1288+
});
1289+
write("index.html", "X");
1290+
apiWrites("P", "X");
1291+
1292+
const second = await history.claim(you, "Second", ["index.html"], {
1293+
overwrote: { "index.html": fileContentVersion("K") },
1294+
});
1295+
expect((await history.undo(second!.id, { who: you })).ok).toBe(true);
1296+
expect(read("index.html")).toBe("X");
1297+
});
1298+
1299+
it("lets the budget free the bytes a committed API write replaced", async () => {
1300+
const saved = "B".repeat(3000);
1301+
const { history, write, projectDir } = await project(
1302+
{ "index.html": "a", "other.html": "o" },
1303+
{ budgetBytes: 2000 },
1304+
);
1305+
const edited = `${saved}!`;
1306+
write("index.html", edited);
1307+
recordFileWriteReceipt(join(projectDir, "index.html"), {
1308+
path: "index.html",
1309+
version: fileContentVersion(edited),
1310+
writeToken: "agent",
1311+
overwrote: saved,
1312+
});
1313+
await history.flush();
1314+
1315+
await change(history, you, "Other", () => write("other.html", "o2"));
1316+
expect(history.list().map((entry) => entry.label)).toContain("Other");
1317+
});
1318+
11801319
it("keeps the history of a small edit in a project larger than its budget", async () => {
11811320
const { history, write } = await project(
11821321
{ "media.bin": Buffer.alloc(4096, 1), "index.html": "a" },

0 commit comments

Comments
 (0)