collective

Browse / github.com/storytold/artcraft / patch

fix(video-editor): finish pending saves before project exit

fix(video-editor): finish pending saves before project exit ## What changed Finish an existing autosave before flushing the latest dirty snapshot, and make concurrent flush callers wait for the active write. If exit saving fails, keep the editor and its active project available, show the existing error toast, and re-enable exit so the user can retry. Navigate only after saving and closing succeeds. ## Why The editor header awaits `project.prepareExit()`, which awaits `save.flush()`. Previously `saveNow()` returned immediately when an autosave was in flight, allowing the editor to close before a later edit was persisted. The header also navigated in

Status
open
Score
0.00
Adopters
0 other operators
Author
rudycelekli (imported from pull request #2077, open)
Base
b5614ad333839649d8f4e779d955b89b3c22f376
Size
+245 −3 in 4 files
Review
pending
Shared
2026-10-28
Id
9591ca7a5873980f65ed0518

How it went

No other operator has reported using it yet.

Diff

raw · frontend/libs/components/video-editor/src/lib/components/editor/editor-header.spec.tsx frontend/libs/components/video-editor/src/lib/components/editor/editor-header.tsx frontend/libs/components/video-editor/src/lib/core/managers/save-manager.spec.ts frontend/libs/components/video-editor/src/lib/core/managers/save-manager.ts

diff --git a/frontend/libs/components/video-editor/src/lib/components/editor/editor-header.spec.tsx b/frontend/libs/components/video-editor/src/lib/components/editor/editor-header.spec.tsxnew file mode 100644--- /dev/null+++ b/frontend/libs/components/video-editor/src/lib/components/editor/editor-header.spec.tsx@@ -0,0 +1,111 @@+import { act, type ReactNode } from "react";+import { createRoot, type Root } from "react-dom/client";+import { EditorHeader } from "./editor-header";+import { SaveManager } from "../../core/managers/save-manager";+import { ProjectManager } from "../../core/managers/project-manager";+import type { EditorCore } from "../../core";+import type { TProject } from "../../project/types";++const host = vi.hoisted(() => ({+  editor: null as unknown as EditorCore,+  navigate: vi.fn(),+  error: vi.fn(),+}));++vi.mock("react-router-dom", () => ({ useNavigate: () => host.navigate }));+vi.mock("../../editor/use-editor", () => ({ useEditor: (selector?: (editor: EditorCore) => unknown) => selector ? selector(host.editor) : host.editor }));+vi.mock("../../EditorProvider", () => ({ useEditorAdapters: () => ({ toast: { error: host.error }, authUser: { currentUser: () => null } }) }));+vi.mock("../../commands/project", () => ({ UpdateProjectSettingsCommand: class {} }));+vi.mock("../../media/rehydrate", () => ({ rehydrateProjectMedia: vi.fn() }));+vi.mock("../ui/dropdown-menu", () => ({+  DropdownMenu: ({ children }: { children: ReactNode }) => <div>{children}</div>,+  DropdownMenuContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,+  DropdownMenuTrigger: ({ children }: { children: ReactNode }) => <div>{children}</div>,+  DropdownMenuItem: ({ children, onClick, disabled }: { children: ReactNode; onClick: () => void; disabled?: boolean }) => <button onClick={onClick} disabled={disabled}>{children}</button>,+  DropdownMenuSeparator: () => null,+}));+vi.mock("../ui/button", () => ({ Button: ({ children }: { children: ReactNode }) => <button>{children}</button> }));+vi.mock("../../utils/ui", () => ({ cn: (...values: string[]) => values.filter(Boolean).join(" ") }));+vi.mock("./export-button", () => ({ ExportButton: () => null }));+vi.mock("../../actions/shortcuts-dialog", () => ({ ShortcutsDialog: () => null }));+vi.mock("../../project/components/rename-project-dialog", () => ({ RenameProjectDialog: () => null }));+vi.mock("../../project/components/delete-project-dialog", () => ({ DeleteProjectDialog: () => null }));++let root: Root;+let container: HTMLDivElement;+let writes: Array<{ resolve: () => void; reject: (error: Error) => void }>;++beforeEach(async () => {+  vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true);+  vi.spyOn(console, "error").mockImplementation(() => {});+  host.navigate.mockReset();+  host.error.mockReset();+  writes = [];+  const editor = {+    adapters: { projectStorage: { saveProject: () => new Promise<void>((resolve, reject) => writes.push({ resolve, reject })) } },+    media: { getAssets: () => [], clearAllAssets: vi.fn() },+    scenes: { clearScenes: vi.fn() }, command: { clear: vi.fn() },+  } as unknown as EditorCore;+  editor.project = new ProjectManager(editor);+  editor.save = new SaveManager({ editor });+  editor.project.setActiveProject({ project: {+    metadata: { id: "draft", name: "Draft", duration: 0, createdAt: new Date(0), updatedAt: new Date(0) },+    scenes: [], currentSceneId: "", version: 1,+    settings: { fps: { numerator: 30, denominator: 1 }, canvasSize: { width: 1920, height: 1080 }, background: { type: "color", color: "#000" } },+  } as TProject });+  host.editor = editor;+  container = document.createElement("div");+  document.body.append(container);+  root = createRoot(container);+  await act(async () => root.render(<EditorHeader exitTo="/create" />));+});++afterEach(async () => {+  await act(async () => root.unmount());+  host.editor.save.stop();+  container.remove();+  vi.restoreAllMocks();+  vi.unstubAllGlobals();+});++it("retains an unsaved project and lets the user retry after storage fails", async () => {+  await clickExit();+  await act(async () => writes[0].reject(new Error("disk full")));+  const abandoned = host.navigate.mock.calls.length;+  const retryDisabled = exitButton().disabled;+  expect(host.editor.project.getActive()?.metadata.id).toBe("draft");+  expect(abandoned).toBe(0);+  expect(retryDisabled).toBe(false);+  expect(host.error).toHaveBeenCalledWith("Failed to save project", { description: "disk full" });+  await clickExit();+  expect(writes).toHaveLength(2);+  await act(async () => writes[1].resolve());+  expect(host.editor.project.getActive()).toBeNull();+  expect(host.navigate).toHaveBeenCalledExactlyOnceWith("/create");+});++it("waits for an ordinary successful save before closing and navigating", async () => {+  await clickExit();+  expect(host.navigate).not.toHaveBeenCalled();+  expect(host.editor.project.getActive()?.metadata.id).toBe("draft");+  await act(async () => writes[0].resolve());+  expect(host.editor.project.getActive()).toBeNull();+  expect(host.navigate).toHaveBeenCalledExactlyOnceWith("/create");+  expect(host.error).not.toHaveBeenCalled();+});++it("blocks another exit click while saving", async () => {+  await clickExit();+  expect(exitButton().disabled).toBe(true);+  await clickExit();+  expect(writes).toHaveLength(1);+  await act(async () => writes[0].resolve());+});++function exitButton() {+  return Array.from(container.querySelectorAll("button")).find((button) => button.textContent === "Exit project")!;+}++async function clickExit() {+  await act(async () => exitButton().click());+}diff --git a/frontend/libs/components/video-editor/src/lib/components/editor/editor-header.tsx b/frontend/libs/components/video-editor/src/lib/components/editor/editor-header.tsx--- a/frontend/libs/components/video-editor/src/lib/components/editor/editor-header.tsx+++ b/frontend/libs/components/video-editor/src/lib/components/editor/editor-header.tsx@@ -81,9 +81,15 @@ function ProjectDropdown({ exitTo }: { exitTo: string }) {       editor.project.closeProject();     } catch (error) {       console.error("Failed to prepare project exit:", error);+      toast.error("Failed to save project", {+        description:+          error instanceof Error ? error.message : "Please try again",+      });+      return;     } finally {-      navigate(exitTo);+      setIsExiting(false);     }+    navigate(exitTo);   };    const handleSaveProjectName = async (newName: string) => {diff --git a/frontend/libs/components/video-editor/src/lib/core/managers/save-manager.spec.ts b/frontend/libs/components/video-editor/src/lib/core/managers/save-manager.spec.tsnew file mode 100644--- /dev/null+++ b/frontend/libs/components/video-editor/src/lib/core/managers/save-manager.spec.ts@@ -0,0 +1,116 @@+import { SaveManager } from "./save-manager";+import { ProjectManager } from "./project-manager";+import type { EditorCore } from "../index";+import type { TProject } from "../../project/types";++// Commands and media downloads are not invoked by save/exit; exercise the+// real managers and document serialization against the host storage adapter.+vi.mock("../../commands/project", () => ({ UpdateProjectSettingsCommand: class {} }));+vi.mock("../../media/rehydrate", () => ({ rehydrateProjectMedia: vi.fn() }));++beforeEach(() => vi.useFakeTimers());+afterEach(() => vi.useRealTimers());++it("waits for an in-flight autosave and the newest edit before exit closes the project", async () => {+  const fixture = buildFixture();+  fixture.save.markDirty();+  await vi.advanceTimersByTimeAsync(20);+  fixture.project.setActiveProject({ project: sampleProject("Newest edit") });+  fixture.save.markDirty();++  let exited = false;+  const exit = fixture.project.prepareExit().then(() => {+    exited = true;+    fixture.project.closeProject();+  });+  await drainMicrotasks();+  const exitedBeforeWriteFinished = exited;+  fixture.releaseFirstWrite();+  await exit;+  await vi.advanceTimersByTimeAsync(20);++  expect(exitedBeforeWriteFinished).toBe(false);+  expect(fixture.savedNames).toEqual(["Original", "Newest edit"]);+  expect(fixture.project.getActive()).toBeNull();+});++it("makes concurrent flush callers wait for the final write", async () => {+  const fixture = buildFixture();+  fixture.save.markDirty();+  await vi.advanceTimersByTimeAsync(20);+  fixture.project.setActiveProject({ project: sampleProject("Newest edit") });+  const completions: number[] = [];+  const flushes = [1, 2].map((id) => fixture.save.flush().then(() => completions.push(id)));+  await drainMicrotasks();+  const earlyCompletions = [...completions];+  fixture.releaseFirstWrite();+  await Promise.all(flushes);++  expect(earlyCompletions).toEqual([]);+  expect(fixture.savedNames).toEqual(["Original", "Newest edit"]);+  expect(fixture.save.getIsDirty()).toBe(false);+});++it("propagates an in-flight write failure to a waiting flush", async () => {+  const fixture = buildFixture();+  const first = fixture.save.flush();+  const second = fixture.save.flush();+  const results = Promise.allSettled([first, second]);+  fixture.rejectFirstWrite(new Error("disk unavailable"));+  const settled = await results;++  expect(settled.map((result) => result.status)).toEqual(["rejected", "rejected"]);+  expect(fixture.project.getActive()?.metadata.name).toBe("Original");+  fixture.save.stop();+});++it("preserves the ordinary flush and prepare-exit path", async () => {+  const fixture = buildFixture(false);+  fixture.project.setActiveProject({ project: sampleProject("Newest edit") });+  await fixture.project.prepareExit();+  expect(fixture.savedNames).toEqual(["Newest edit"]);+  expect(fixture.save.getIsDirty()).toBe(false);+  fixture.project.closeProject();+});++function buildFixture(blockFirstWrite = true) {+  let releaseFirstWrite!: () => void;+  let rejectFirstWrite!: (reason: Error) => void;+  const firstWrite = new Promise<void>((resolve, reject) => {+    releaseFirstWrite = resolve;+    rejectFirstWrite = reject;+  });+  const savedNames: string[] = [];+  let writes = 0;+  const editor = {+    adapters: { projectStorage: {+      async saveProject(document: { data: { project: { metadata: { name: string } } } }) {+        writes++;+        if (blockFirstWrite && writes === 1) await firstWrite;+        savedNames.push(document.data.project.metadata.name);+      },+    } },+    scenes: { subscribe: () => () => {}, clearScenes: vi.fn() },+    timeline: { subscribe: () => () => {} },+    command: { clear: vi.fn() },+    media: { getAssets: () => [], clearAllAssets: vi.fn() },+  } as unknown as EditorCore;+  const project = new ProjectManager(editor);+  const save = new SaveManager({ editor, debounceMs: 20 });+  Object.assign(editor, { project, save });+  project.setActiveProject({ project: sampleProject("Original") });+  save.start();+  return { project, save, savedNames, releaseFirstWrite, rejectFirstWrite };+}++function sampleProject(name: string): TProject {+  return {+    metadata: { id: "project-a", name, createdAt: new Date(0), updatedAt: new Date(0), duration: 0 },+    scenes: [], currentSceneId: "", settings: { fps: { numerator: 30, denominator: 1 }, canvasSize: { width: 1920, height: 1080 }, background: { type: "color", color: "#000" } },+    version: 1,+  } as TProject;+}++async function drainMicrotasks() {+  for (let i = 0; i < 5; i++) await Promise.resolve();+}diff --git a/frontend/libs/components/video-editor/src/lib/core/managers/save-manager.ts b/frontend/libs/components/video-editor/src/lib/core/managers/save-manager.ts--- a/frontend/libs/components/video-editor/src/lib/core/managers/save-manager.ts+++ b/frontend/libs/components/video-editor/src/lib/core/managers/save-manager.ts@@ -8,6 +8,7 @@ export class SaveManager {   private debounceMs: number;   private isPaused = false;   private isSaving = false;+  private savePromise: Promise<void> | null = null;   private hasPendingSave = false;   private saveTimer: ReturnType<typeof setTimeout> | null = null;   private unsubscribeHandlers: Array<() => void> = [];@@ -64,6 +65,9 @@ export class SaveManager {    async flush(): Promise<void> {     this.hasPendingSave = true;+    // Exit callers must wait for the current snapshot, then persist any edit+    // queued while that write was in flight before closing the project.+    if (this.savePromise) await this.savePromise;     await this.saveNow();   } @@ -82,7 +86,10 @@ export class SaveManager {   }    private async saveNow(): Promise<void> {-    if (this.isSaving) return;+    if (this.isSaving) {+      await this.savePromise;+      return;+    }     if (!this.hasPendingSave) return;      const activeProject = this.editor.project.getActive();@@ -95,8 +102,10 @@ export class SaveManager {     this.clearTimer();      try {-      await this.editor.project.saveCurrentProject();+      this.savePromise = this.editor.project.saveCurrentProject();+      await this.savePromise;     } finally {+      this.savePromise = null;       this.isSaving = false;       if (this.hasPendingSave) {         this.queueSave();