Skip to content

Commit bd892b7

Browse files
committed
fix(studio): expect what deleting a clip already wrote, not what it read
Deleting a clip refused its own save. The flow reads the file, POSTs `remove-element` — which rewrites the file server-side — then saves the duration shrink on top. That second write carried the content read at the start as its optimistic-concurrency expectation, but the server had already moved the file on, so it came back 409 "file conflict". The save queue pauses on a conflict rather than retry stale work, so the error surfaced and nothing persisted afterwards; the clip stayed on the timeline until a reload, since the store update runs after the save and the throw skipped it. The undo baseline and "what is on disk" were one value in saveProjectFilesWithHistory, which is right until a server-side mutation has already written part of the edit. They are now separable: `diskContent` says what to expect on disk, `readFile` still says what undo restores. Both delete paths pass what remove-element left behind. Measured in the studio, before and after: PUT /files/index.html 409 -> 200, no error toast, the clip leaves the timeline immediately, and undo still brings it back. Pre-existing — reproduced identically on the pre-session studio source.
1 parent 33ccc0a commit bd892b7

4 files changed

Lines changed: 72 additions & 1 deletion

File tree

packages/studio/src/hooks/useElementLifecycleOps.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ export function useElementLifecycleOps({
141141
kind: "timeline",
142142
files: { [targetPath]: patchedContent },
143143
readFile: async () => originalContent,
144+
// remove-element already wrote the removal, so disk holds THAT — not
145+
// the content read at the top. Undo still goes back to the original.
146+
diskContent: { [targetPath]: patchedContent },
144147
writeFile: writeProjectFile,
145148
recordEdit: editHistory.recordEdit,
146149
});

packages/studio/src/hooks/useTimelineEditing.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,9 @@ export function useTimelineEditing({
448448
kind: "timeline",
449449
files: { [targetPath]: patchedContent },
450450
readFile: async () => originalContent,
451+
// remove-element already wrote the removal, so disk holds THAT — not the
452+
// content read at the top. Undo still goes back to the original.
453+
diskContent: { [targetPath]: removedContent },
451454
writeFile: writeProjectFile,
452455
recordEdit,
453456
});

packages/studio/src/utils/studioFileHistory.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,55 @@ describe("saveProjectFilesWithHistory", () => {
2929
});
3030
});
3131

32+
/**
33+
* Deleting a clip POSTs `remove-element`, which rewrites the file server-side,
34+
* and only then saves the duration shrink. Expecting the content read before
35+
* the mutation made the server refuse that write as a conflict: the save queue
36+
* paused on the 409 and the clip stayed on the timeline until a reload.
37+
*/
38+
it("expects what is on disk, not the undo baseline, when they differ", async () => {
39+
const expectations: Record<string, string | undefined> = {};
40+
const recordEdit = vi.fn();
41+
42+
await saveProjectFilesWithHistory({
43+
projectId: "project-1",
44+
label: "Delete timeline clip",
45+
kind: "timeline",
46+
files: { "index.html": "removed+shrunk" },
47+
readFile: async () => "original",
48+
diskContent: { "index.html": "removed" },
49+
writeFile: async (path, _content, expectedContent) => {
50+
expectations[path] = expectedContent;
51+
},
52+
recordEdit,
53+
});
54+
55+
expect(expectations["index.html"]).toBe("removed");
56+
// Undo still goes all the way back, which is the whole reason the two are
57+
// allowed to differ.
58+
expect(recordEdit).toHaveBeenCalledWith(
59+
expect.objectContaining({
60+
files: { "index.html": { before: "original", after: "removed+shrunk" } },
61+
}),
62+
);
63+
});
64+
65+
it("still expects the undo baseline when nothing says otherwise", async () => {
66+
const expectations: Record<string, string | undefined> = {};
67+
await saveProjectFilesWithHistory({
68+
projectId: "project-1",
69+
label: "Move layer",
70+
kind: "manual",
71+
files: { "index.html": "after" },
72+
readFile: async () => "before",
73+
writeFile: async (path, _content, expectedContent) => {
74+
expectations[path] = expectedContent;
75+
},
76+
recordEdit: vi.fn(),
77+
});
78+
expect(expectations["index.html"]).toBe("before");
79+
});
80+
3281
it("skips writes and history for unchanged content", async () => {
3382
const writeFile = vi.fn();
3483
const recordEdit = vi.fn();

packages/studio/src/utils/studioFileHistory.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,21 @@ interface SaveProjectFilesWithHistoryInput {
3434
readFile: (path: string) => Promise<string>;
3535
writeFile: ProjectFileWriter;
3636
recordEdit: (entry: RecordEditInput) => Promise<void>;
37+
/**
38+
* What a path holds ON DISK right now, when that is not the same as the
39+
* history's "before".
40+
*
41+
* The two are normally one value, so the write's optimistic-concurrency
42+
* expectation was taken straight from the undo baseline. They come apart when
43+
* a server-side mutation has already written part of the edit: deleting a clip
44+
* POSTs `remove-element`, which rewrites the file, and only then saves the
45+
* duration shrink — expecting the pre-delete content it read at the start. The
46+
* server had moved the file on, so the write was refused as a conflict, the
47+
* save queue paused, and the clip stayed on the timeline until a reload.
48+
*
49+
* Undo still restores `before`; this only says what to expect on disk.
50+
*/
51+
diskContent?: Record<string, string>;
3752
}
3853

3954
export async function readProjectFileContent(pid: string, path: string): Promise<string> {
@@ -57,6 +72,7 @@ export async function saveProjectFilesWithHistory({
5772
readFile,
5873
writeFile,
5974
recordEdit,
75+
diskContent,
6076
}: SaveProjectFilesWithHistoryInput): Promise<string[]> {
6177
return serializeStudioFileMutations(writeFile, Object.keys(files), async () => {
6278
const snapshots: Record<string, { before: string; after: string }> = {};
@@ -73,7 +89,7 @@ export async function saveProjectFilesWithHistory({
7389
const writtenPaths: string[] = [];
7490
try {
7591
for (const path of changedPaths) {
76-
await writeFile(path, snapshots[path].after, snapshots[path].before);
92+
await writeFile(path, snapshots[path].after, diskContent?.[path] ?? snapshots[path].before);
7793
writtenPaths.push(path);
7894
}
7995

0 commit comments

Comments
 (0)