diff --git a/src/components/ai-edition/NewEditorShell.tsx b/src/components/ai-edition/NewEditorShell.tsx index 68931e66c..b28f8558e 100644 --- a/src/components/ai-edition/NewEditorShell.tsx +++ b/src/components/ai-edition/NewEditorShell.tsx @@ -302,15 +302,9 @@ export function NewEditorShell() { // 3. Handle request-save-before-close from Electron const unsubSaveBeforeClose = window.electronAPI.onRequestSaveBeforeClose(async () => { const doc = useProjectStore.getState().document; - if (doc) { - try { - await saveDocument(doc); - return true; - } catch { - toast.error("Failed to save before closing"); - return false; - } - } + // The store already toasted the reason; answering false is what keeps the + // window open on top of it. + if (doc) return await saveDocument(doc); return true; }); @@ -618,14 +612,7 @@ export function NewEditorShell() { const handleSave = useCallback(async () => { const doc = useProjectStore.getState().document; if (!doc) return; - try { - await saveDocument(doc); - toast.success("Project saved"); - } catch (err) { - toast.error("Save failed", { - description: err instanceof Error ? err.message : String(err), - }); - } + if (await saveDocument(doc)) toast.success("Project saved"); }, [saveDocument]); // Native File menu (electron/main.ts) → v4 actions. The menu is shown via @@ -653,13 +640,7 @@ export function NewEditorShell() { const doc = useProjectStore.getState().document; if (!doc) return; if (title === doc.project.title) return; - try { - await saveDocument({ ...doc, project: { ...doc.project, title } }); - } catch (err) { - toast.error("Rename failed", { - description: err instanceof Error ? err.message : String(err), - }); - } + await saveDocument({ ...doc, project: { ...doc.project, title } }); }, [saveDocument], ); @@ -680,13 +661,12 @@ export function NewEditorShell() { } if (choice === "save") { const doc = useProjectStore.getState().document; - if (doc) { - try { - await saveDocument(doc); - } catch { - resolve("cancel"); - return; - } + // A failed save cancels the action that prompted this dialog. The store has + // already said why -- which is what the bare `catch {}` here used to swallow, + // leaving the window refusing to close with nothing on screen explaining it. + if (doc && !(await saveDocument(doc))) { + resolve("cancel"); + return; } } if (action === "record") { @@ -845,13 +825,8 @@ export function NewEditorShell() { if (choice === "cancel") return; if (choice === "save") { const doc = useProjectStore.getState().document; - if (doc) { - try { - await saveDocument(doc); - } catch { - return; - } - } + // Stay put if the save did not land -- the store has already said why. + if (doc && !(await saveDocument(doc))) return; } setNewProjectOpen(true); })(); @@ -864,13 +839,8 @@ export function NewEditorShell() { if (choice === "cancel") return; if (choice === "save") { const doc = useProjectStore.getState().document; - if (doc) { - try { - await saveDocument(doc); - } catch { - return; - } - } + // Stay put if the save did not land -- the store has already said why. + if (doc && !(await saveDocument(doc))) return; } setOpenProjectOpen(true); })(); diff --git a/src/components/ai-edition/v4/V4Timeline.tsx b/src/components/ai-edition/v4/V4Timeline.tsx index 22b79aa8d..ba549180f 100644 --- a/src/components/ai-edition/v4/V4Timeline.tsx +++ b/src/components/ai-edition/v4/V4Timeline.tsx @@ -1059,6 +1059,10 @@ export function V4Timeline({ return; } const added = await tl.addZoomsBulk(suggestions); + // A failed write returns 0 and has already toasted why. Without this the user + // got "Added 0 automatic zooms" stacked on top of "Failed to save project", + // with no zoom anywhere -- a success message for something that did not happen. + if (added === 0) return; toast.success( t(added === 1 ? "toolbar.addedAutoZoom" : "toolbar.addedAutoZoomPlural", { count: added }), ); diff --git a/src/i18n/toastText.ts b/src/i18n/toastText.ts new file mode 100644 index 000000000..9b20d57e9 --- /dev/null +++ b/src/i18n/toastText.ts @@ -0,0 +1,25 @@ +// Translation for toasts fired OUTSIDE React -- stores, and anything else that reports +// to the user without a component around it to call `useScopedT`. + +import { DEFAULT_LOCALE, type I18nNamespace, LOCALE_STORAGE_KEY, type Locale } from "./config"; +import { getAvailableLocales, translate } from "./loader"; + +/** + * Toasts fired outside React still have to speak the user's language. Same source as + * `I18nProvider` (stored preference, else the default), validated so a stale value + * cannot push `translate` onto a locale it does not have. + */ +export function toastText( + namespace: I18nNamespace, + key: string, + vars?: Record, +): string { + let locale: Locale = DEFAULT_LOCALE; + try { + const stored = localStorage.getItem(LOCALE_STORAGE_KEY); + if (stored && getAvailableLocales().includes(stored as Locale)) locale = stored as Locale; + } catch { + // localStorage may be unavailable -- the default locale is a fine answer. + } + return translate(locale, namespace, key, vars); +} diff --git a/src/lib/ai-edition/store/projectStore.test.ts b/src/lib/ai-edition/store/projectStore.test.ts index 5483f2b88..7435eb733 100644 --- a/src/lib/ai-edition/store/projectStore.test.ts +++ b/src/lib/ai-edition/store/projectStore.test.ts @@ -297,6 +297,58 @@ describe("useProjectStore", () => { expect(toastMocks.error.mock.calls[0][0]).toContain("video.mp4"); }); + // The save boundary. Every write in the app funnels through `saveDocument`, and + // almost every caller `void`s it from a click handler, so what this function does + // with a failure IS what the user sees. + describe("saveDocument reports a failed write instead of rejecting", () => { + it("resolves false, tells the user, and leaves the document alone", async () => { + useProjectStore.setState({ + projectId: "proj_test", + document: sampleDoc, + revision: 3, + status: "ready", + dirty: true, + }); + bridgeMocks.save.mockResolvedValue({ success: false, error: "EACCES" }); + + const edited = { ...sampleDoc, project: { ...sampleDoc.project, title: "Edited" } }; + await expect(useProjectStore.getState().saveDocument(edited)).resolves.toBe(false); + + expect(toastMocks.error).toHaveBeenCalledWith("Failed to save project", { + description: "EACCES", + }); + const state = useProjectStore.getState(); + expect(state.document?.project.title).toBe("Test"); + expect(state.revision).toBe(3); + // Still dirty: `dirty` is the only input to the beforeunload guard and to + // `setHasUnsavedChanges`, so a failed write is the last moment to claim clean. + expect(state.dirty).toBe(true); + }); + + it("never rejects, so a detached caller cannot leak an unhandled rejection", async () => { + useProjectStore.setState({ projectId: "proj_test", document: sampleDoc, dirty: true }); + bridgeMocks.save.mockRejectedValue(new Error("bridge is gone")); + + await expect(useProjectStore.getState().saveDocument(sampleDoc)).resolves.toBe(false); + expect(toastMocks.error).toHaveBeenCalledWith("Failed to save project", { + description: "bridge is gone", + }); + }); + + it("resolves true and commits on success", async () => { + const saved = { ...sampleDoc, project: { ...sampleDoc.project, title: "Saved" } }; + useProjectStore.setState({ projectId: "proj_test", document: sampleDoc, dirty: true }); + bridgeMocks.save.mockResolvedValue({ success: true, document: saved }); + + await expect(useProjectStore.getState().saveDocument(saved)).resolves.toBe(true); + + expect(toastMocks.error).not.toHaveBeenCalled(); + const state = useProjectStore.getState(); + expect(state.document?.project.title).toBe("Saved"); + expect(state.dirty).toBe(false); + }); + }); + it("removeAsset requires a loaded project", async () => { await expect(useProjectStore.getState().removeAsset("asset_x")).rejects.toThrow( "No project loaded", diff --git a/src/lib/ai-edition/store/projectStore.ts b/src/lib/ai-edition/store/projectStore.ts index 539449dab..60b42e767 100644 --- a/src/lib/ai-edition/store/projectStore.ts +++ b/src/lib/ai-edition/store/projectStore.ts @@ -1,4 +1,6 @@ +import { toast } from "sonner"; import { create } from "zustand"; +import { toastText } from "@/i18n/toastText"; import { nativeBridgeClient } from "@/native/client"; import { type Interval, @@ -36,7 +38,17 @@ export interface ProjectState { refresh: () => Promise; addAsset: (path: string, label?: string) => Promise; removeAsset: (assetId: string) => Promise; - saveDocument: (document: AxcutDocument) => Promise; + /** + * Write the document to disk. Resolves `true` when it landed, `false` when it did + * not -- and a `false` has ALREADY been reported to the user and logged, so a + * caller that ignores it is choosing not to react, not choosing to stay silent. + * + * It never rejects, by design. Every save in the app funnels through here and + * almost all of them are `void`-ed from a click handler, so a rejection here was + * an unhandled rejection in the renderer with no toast, no log and no clue -- + * change a caption font on a read-only project and the edit was simply gone. + */ + saveDocument: (document: AxcutDocument) => Promise; setDocument: (document: AxcutDocument) => void; replaceTimeline: (intervals: Interval[], reason: string) => Promise; restoreFullTimeline: () => Promise; @@ -169,8 +181,10 @@ export const useProjectStore = create((set, get) => ({ a.id === addedAsset.id ? { ...a, cameraTrack: linked } : a, ), }; - await get().saveDocument(next); - document = parseDocument(next); + // Only adopt the linked document if it actually reached disk -- otherwise + // the caller is handed a document claiming a camera link the file does not + // have. The store has already told the user the write failed. + if (await get().saveDocument(next)) document = parseDocument(next); } // success:false just means no camera was found for this asset — // the normal case for a plain imported video. Nothing to surface. @@ -212,17 +226,32 @@ export const useProjectStore = create((set, get) => ({ }, async saveDocument(document) { - const result = await nativeBridgeClient.aiEdition.save(document); - if (!result.success || !result.document) { - throw new Error(result.error ?? "Failed to save project"); + try { + const result = await nativeBridgeClient.aiEdition.save(document); + if (!result.success || !result.document) { + throw new Error(result.error ?? "Failed to save project"); + } + const parsed = parseDocument(result.document); + set({ + document: parsed, + revision: get().revision + 1, + dirty: false, + lastSavedAt: new Date(), + }); + return true; + } catch (error) { + // Logged as well as toasted: a toast is gone in five seconds, and "my edit + // disappeared" gets reported much later than that. + console.error("[project] failed to save document:", error); + toast.error(toastText("editor", "project.failedToSave"), { + description: error instanceof Error ? error.message : String(error), + }); + // `dirty` is deliberately left alone. It is the only input to the + // `beforeunload` guard and to `setHasUnsavedChanges`, so clearing it here + // would let the window close without a prompt on the one path where there is + // definitely something unsaved. + return false; } - const parsed = parseDocument(result.document); - set({ - document: parsed, - revision: get().revision + 1, - dirty: false, - lastSavedAt: new Date(), - }); }, setDocument(document) { diff --git a/src/lib/ai-edition/store/transcriptionStore.ts b/src/lib/ai-edition/store/transcriptionStore.ts index 1cee92d11..3c3c1139f 100644 --- a/src/lib/ai-edition/store/transcriptionStore.ts +++ b/src/lib/ai-edition/store/transcriptionStore.ts @@ -25,8 +25,7 @@ import { useEffect, useMemo } from "react"; import { toast } from "sonner"; import { create } from "zustand"; -import { DEFAULT_LOCALE, LOCALE_STORAGE_KEY, type Locale } from "@/i18n/config"; -import { getAvailableLocales, translate } from "@/i18n/loader"; +import { toastText as translateToast } from "@/i18n/toastText"; import { transcribeAsset, withTranscript } from "../document/transcribe"; import type { AxcutDocument } from "../schema"; import { @@ -85,21 +84,9 @@ function hasLocalSttEngine(): boolean { return typeof window.electronAPI?.stt?.transcribe === "function"; } -/** - * Toasts fired outside React still have to speak the user's language. Same - * source as `I18nProvider` (stored preference, else the default), validated so - * a stale value can't push `translate` onto a locale it doesn't have. - */ -function toastText(key: string, vars?: Record): string { - let locale: Locale = DEFAULT_LOCALE; - try { - const stored = localStorage.getItem(LOCALE_STORAGE_KEY); - if (stored && getAvailableLocales().includes(stored as Locale)) locale = stored as Locale; - } catch { - // localStorage may be unavailable — the default locale is a fine answer. - } - return translate(locale, "editor", key, vars); -} +/** This store's toasts all live in the `editor` namespace. */ +const toastText = (key: string, vars?: Record) => + translateToast("editor", key, vars); export const useTranscriptionStore = create((set, get) => ({ projectId: null, @@ -321,24 +308,26 @@ async function persistPermanentFailure( const doc = project.document; if (!doc || doc.project.id !== projectId) return; if (!doc.assets.some((a) => a.id === assetId)) return; - try { - await project.saveDocument({ - ...doc, - assets: doc.assets.map((a) => - a.id === assetId - ? { - ...a, - transcriptionFailure: { - kind, - message: failure.message, - at: new Date().toISOString(), - }, - } - : a, - ), - }); - } catch (error) { - console.warn("[transcription] could not persist the failure on the asset:", error); + // Best-effort bookkeeping: `saveDocument` reports its own failures and resolves + // false rather than throwing, and a note on the asset is not worth a second + // message on top of the one the user already got. + const persisted = await project.saveDocument({ + ...doc, + assets: doc.assets.map((a) => + a.id === assetId + ? { + ...a, + transcriptionFailure: { + kind, + message: failure.message, + at: new Date().toISOString(), + }, + } + : a, + ), + }); + if (!persisted) { + console.warn("[transcription] could not persist the failure on the asset"); } } diff --git a/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts b/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts index f86088bcf..91f69aac5 100644 --- a/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts +++ b/src/lib/ai-edition/store/useSequentialTimelineOps.test.ts @@ -69,10 +69,11 @@ describe("useSequentialTimelineOps", () => { const callOrder: string[] = []; const saveDocument = vi.fn(async (doc: AxcutDocument) => { // Mirror the real store: write the saved doc back so the next - // call in the queue sees the latest committed state. + // call in the queue sees the latest committed state, and report + // success the way `projectStore.saveDocument` does. useProjectStore.getState().setDocument(doc); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion callOrder.push(doc.timeline.trimRanges[0]?.startSec.toString() ?? "empty"); + return true; }); const { result } = renderHook(() => @@ -112,15 +113,20 @@ describe("useSequentialTimelineOps", () => { expect(doc2.timeline.trimRanges.map((t) => t.startSec).sort()).toEqual([1, 5]); }); - it("swallows save errors so the next call can still proceed", async () => { + it("keeps the queue healthy when a save reports failure", async () => { + // `projectStore.saveDocument` resolves false rather than rejecting -- it has + // already told the user why. What this hook owes is that the failed op resolves + // to null and the NEXT op still runs, off a document that never took the failed + // edit. const seed = makeDocWithAsset(); useProjectStore.setState({ document: seed }); const saveDocument = vi - .fn<(doc: AxcutDocument) => Promise>() - .mockRejectedValueOnce(new Error("save failed")) + .fn<(doc: AxcutDocument) => Promise>() + .mockResolvedValueOnce(false) .mockImplementationOnce(async (doc) => { useProjectStore.getState().setDocument(doc); + return true; }); const { result } = renderHook(() => @@ -140,28 +146,24 @@ describe("useSequentialTimelineOps", () => { reason: "second", }; - let firstSettled = false; - let secondSettled = false; + let firstResult: AxcutDocument | null | undefined; + let secondResult: AxcutDocument | null | undefined; await act(async () => { const p1 = result.current.apply(op1); const p2 = result.current.apply(op2); - await p1.catch((err: unknown) => { - firstSettled = true; - expect((err as Error).message).toBe("save failed"); - }); - await p2.then(() => { - secondSettled = true; - }); + firstResult = await p1; + secondResult = await p2; }); - expect(firstSettled).toBe(true); - expect(secondSettled).toBe(true); + expect(firstResult).toBeNull(); + expect(secondResult).not.toBeNull(); // The queue survived the first failure — both saves were attempted. expect(saveDocument).toHaveBeenCalledTimes(2); + expect(saveDocument).toHaveBeenNthCalledWith(2, secondResult); }); it("returns null when the store has no document and no fallback is supplied", async () => { - const saveDocument = vi.fn(async () => undefined); + const saveDocument = vi.fn(async () => true); const { result } = renderHook(() => useSequentialTimelineOps({ fallbackDocument: null, saveDocument }), ); @@ -190,6 +192,7 @@ describe("useSequentialTimelineOps", () => { const saveDocument = vi.fn(async (doc: AxcutDocument) => { useProjectStore.getState().setDocument(doc); + return true; }); const { result } = renderHook(() => diff --git a/src/lib/ai-edition/store/useSequentialTimelineOps.ts b/src/lib/ai-edition/store/useSequentialTimelineOps.ts index 642c5a435..0bcdd842f 100644 --- a/src/lib/ai-edition/store/useSequentialTimelineOps.ts +++ b/src/lib/ai-edition/store/useSequentialTimelineOps.ts @@ -7,10 +7,11 @@ // read the doc INSIDE the chain, after awaiting the previous save, so // every call sees the doc state the previous call committed. // -// Errors are swallowed when advancing the queue ref so a failed save -// doesn't poison the queue (the next call still has a resolved promise -// to chain off). The original promise returned to the caller is NOT -// swallowed — the caller can await it and observe the rejection. +// A failed save resolves to null rather than rejecting -- `projectStore.saveDocument` +// reports it to the user and returns false. Operation and dynamic-import errors still +// reject the promise handed to the caller; both call sites `void` it, so those remain +// unhandled rejections. They are also the two failures that mean the code is broken +// rather than the disk, so they belong in the console. import { useCallback, useRef } from "react"; import type { AxcutTimelineOperation } from "@/lib/ai-edition/document/operations"; @@ -24,8 +25,11 @@ export interface SequentialTimelineOps { * previous op's save has resolved), and the resulting document is * saved. Calls are serialised — op N+1 reads the doc op N wrote. * - * Returns the saved document, or `null` if no project document is - * loaded (store empty AND no fallback supplied). + * Returns the saved document, or `null` for either of two different things: + * no project document is loaded (store empty AND no fallback supplied), which + * is a silent no-op, or the save failed, which the user has already been told + * about. Both call sites `void` the result, so they are not distinguished; a + * caller that needs to tell them apart has to widen this return type first. */ apply: (op: AxcutTimelineOperation) => Promise; } @@ -33,8 +37,9 @@ export interface SequentialTimelineOps { export function useSequentialTimelineOps(options: { /** Used only when the project store has no document yet. */ fallbackDocument: AxcutDocument | null; - /** Persist a document. The hook awaits this before unblocking the queue. */ - saveDocument: (doc: AxcutDocument) => Promise; + /** Persist a document, resolving false if the write failed (already reported). + * The hook awaits this before unblocking the queue. */ + saveDocument: (doc: AxcutDocument) => Promise; }): SequentialTimelineOps { const { fallbackDocument, saveDocument } = options; const saveQueueRef = useRef>(Promise.resolve()); @@ -43,7 +48,7 @@ export function useSequentialTimelineOps(options: { (op: AxcutTimelineOperation): Promise => { const queued = saveQueueRef.current .then(() => import("@/lib/ai-edition/document/operations")) - .then(({ applyTimelineOperation }) => { + .then(async ({ applyTimelineOperation }) => { // Read the doc inside the chain. The store holds the // latest committed state because the previous call's // save has already resolved by the time this .then @@ -51,13 +56,10 @@ export function useSequentialTimelineOps(options: { const doc = useProjectStore.getState().document ?? fallbackDocument; if (!doc) return null; const applied = applyTimelineOperation(doc, op); - return saveDocument(applied.document).then(() => applied.document); + return (await saveDocument(applied.document)) ? applied.document : null; }); - // Swallow rejection when advancing the queue so a failed save - // doesn't poison the queue — the next call still has a - // resolved promise to chain off. The original `queued` is - // returned to the caller, who can await it and observe the - // rejection. + // Keep operation/import errors from poisoning the queue -- the next call still + // needs a resolved promise to chain off. Save failures already resolve to null. saveQueueRef.current = queued.then( () => undefined, () => undefined, diff --git a/src/lib/ai-edition/store/useTimeline.test.ts b/src/lib/ai-edition/store/useTimeline.test.ts index 7f4541768..cc51b3a01 100644 --- a/src/lib/ai-edition/store/useTimeline.test.ts +++ b/src/lib/ai-edition/store/useTimeline.test.ts @@ -18,6 +18,9 @@ const probeVideoDurationMock = vi.hoisted(() => vi.fn()); const probeVideoDimensionsMock = vi.hoisted(() => vi.fn().mockResolvedValue({ width: 1920, height: 1080 }), ); +const toastErrorMock = vi.hoisted(() => vi.fn()); + +vi.mock("sonner", () => ({ toast: { error: toastErrorMock } })); vi.mock("../timeline/duration", async (importOriginal) => { const actual = await importOriginal(); @@ -546,6 +549,63 @@ describe("useTimeline zoom modifiers (rotation + focus mode)", () => { focusMode: "auto", }); }); + + it("rolls a live focus edit back when its commit cannot be saved", async () => { + bridgeMocks.save.mockResolvedValueOnce({ success: false, error: "project file locked" }); + const { result } = renderTimeline(); + + act(() => result.current.updateZoomFocusLive("zoom_a", { cx: 0.8, cy: 0.2 })); + expect(useProjectStore.getState().document?.zoomRanges[0]?.focus).toEqual({ + cx: 0.8, + cy: 0.2, + }); + + await act(async () => { + await result.current.commitZoomFocus(); + }); + + expect(useProjectStore.getState().document?.zoomRanges[0]?.focus).toEqual({ + cx: 0.5, + cy: 0.5, + }); + // Still dirty, deliberately. The rollback target is the document this drag + // started from, not the last SAVED one, so claiming "clean" would let the window + // close on unsaved work without a prompt. + expect(useProjectStore.getState().dirty).toBe(true); + // The live edit advanced revision once; restoring a different document + // advances it again so async work cannot mistake the rollback for the + // optimistic document it replaced. + expect(useProjectStore.getState().revision).toBe(3); + expect(toastErrorMock).toHaveBeenCalledWith("Failed to save project", { + description: "project file locked", + }); + }); + + it("does not restore another project's document after the project changed", async () => { + // A drag does not always end in a commit: `ZoomFocusOverlay` unmounts the instant + // `focusMode` flips to "auto", so `endDrag` never runs and the snapshot outlives + // the project. Restoring it into the NEXT project put project A's document in + // project B, and the following successful save wrote A over B on disk. + const { result, rerender } = renderTimeline(); + + act(() => result.current.updateZoomFocusLive("zoom_a", { cx: 0.8, cy: 0.2 })); + + const otherProjectDoc: AxcutDocument = { + ...docWithZoom, + project: { ...docWithZoom.project, id: "proj_other", title: "Other" }, + }; + act(() => { + useProjectStore.setState({ projectId: "proj_other", document: otherProjectDoc }); + }); + rerender(); + + bridgeMocks.save.mockResolvedValueOnce({ success: false, error: "project file locked" }); + await act(async () => { + await result.current.commitZoomFocus(); + }); + + expect(useProjectStore.getState().document?.project.id).toBe("proj_other"); + }); }); // Regression guard for the playhead-stutter fix. `currentTimeSec` is rewritten on @@ -669,3 +729,55 @@ describe("useTimeline selection", () => { expect(result.current.clipSelection).toBeNull(); }); }); + +describe("useTimeline save failures", () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + beforeEach(() => { + useProjectStore.getState().clear(); + for (const mock of Object.values(bridgeMocks)) mock.mockReset(); + toastErrorMock.mockReset(); + bridgeMocks.save.mockResolvedValue({ success: false, error: "disk full" }); + useProjectStore.setState({ + projectId: "proj_test", + document: sampleDoc, + revision: 1, + status: "ready", + error: null, + }); + }); + + it("surfaces a rejected mutation without applying it or leaking the rejection", async () => { + const { result } = renderTimeline(); + + await act(async () => { + await expect(result.current.removeClip("clip_a")).resolves.toBeUndefined(); + }); + + expect(toastErrorMock).toHaveBeenCalledWith("Failed to save project", { + description: "disk full", + }); + expect(useProjectStore.getState().document?.timeline.clips).toHaveLength(1); + expect(useProjectStore.getState().document?.timeline.clips[0]?.id).toBe("clip_a"); + }); + + it("reports 0 zooms added when the bulk write fails", async () => { + // The count is what the Auto-enhance caller shows in its success toast, so a + // failed write returning `suggestions.length` produced "Added 3 automatic zooms" + // stacked on "Failed to save project", with no zoom anywhere. The caller guards + // on this 0 (`V4Timeline` runAutoZooms). + const { result } = renderTimeline(); + + let added: number | undefined; + await act(async () => { + added = await result.current.addZoomsBulk([ + { span: { start: 1000, end: 2000 }, focus: { cx: 0.5, cy: 0.5 } }, + ]); + }); + + expect(added).toBe(0); + expect(useProjectStore.getState().document?.zoomRanges).toHaveLength(0); + }); +}); diff --git a/src/lib/ai-edition/store/useTimeline.ts b/src/lib/ai-edition/store/useTimeline.ts index cd763bae1..f921a9917 100644 --- a/src/lib/ai-edition/store/useTimeline.ts +++ b/src/lib/ai-edition/store/useTimeline.ts @@ -104,6 +104,27 @@ export function useTimeline() { // the Delete key operates on. const [multiSelection, setMultiSelection] = useState([]); const [clipSelection, setClipSelection] = useState(null); + // Pre-drag snapshots for the two optimistic paths (zoom focus, annotations), so a + // failed commit can put the document back instead of leaving an edit on screen that + // was never written. + const zoomFocusRollbackRef = useRef(null); + const zoomFocusLiveRef = useRef(null); + const annotationRollbackRef = useRef(null); + const annotationLiveRef = useRef(null); + + // A drag does not always end in a commit: `ZoomFocusOverlay` unmounts the moment + // `focusMode` flips to "auto", so `endDrag` never runs and the snapshot outlives the + // project. Left alone, resetting focus in project B and failing that save restored + // project A's document into B -- the next successful save then wrote A over B. It + // also pinned two whole documents per hook instance, and annotations can carry + // base64 image data URLs. + // biome-ignore lint/correctness/useExhaustiveDependencies: projectId is the trigger, not a read — the body only clears refs. + useEffect(() => { + zoomFocusRollbackRef.current = null; + zoomFocusLiveRef.current = null; + annotationRollbackRef.current = null; + annotationLiveRef.current = null; + }, [projectId]); const hasDoc = document !== null && projectId !== null; @@ -217,7 +238,7 @@ export function useTimeline() { ...document, zoomRanges: [...document.zoomRanges, ...anchored] as AxcutDocument["zoomRanges"], }; - await saveDocument(next); + if (!(await saveDocument(next))) return 0; return suggestions.length; }, [document, saveDocument], @@ -305,7 +326,7 @@ export function useTimeline() { ...created, ] as unknown as AxcutDocument["annotations"], }; - await saveDocument(next); + if (!(await saveDocument(next))) return; // Select the freshly added annotation so its inspector opens and it shows a // selection box on the canvas, ready to be retyped over. const newId = created[0]?.id ?? ann.id; @@ -487,6 +508,7 @@ export function useTimeline() { (id: string, focus: { cx: number; cy: number }) => { const doc = useProjectStore.getState().document; if (!doc) return; + if (zoomFocusLiveRef.current !== doc) zoomFocusRollbackRef.current = doc; const next: AxcutDocument = { ...doc, zoomRanges: patchPillById(doc.zoomRanges, id, { @@ -494,6 +516,7 @@ export function useTimeline() { }) as AxcutDocument["zoomRanges"], }; setDocument(next); + zoomFocusLiveRef.current = next; }, [setDocument], ); @@ -501,7 +524,19 @@ export function useTimeline() { const commitZoomFocus = useCallback(async () => { const doc = useProjectStore.getState().document; if (!doc) return; - await saveDocument(doc); + const rollback = zoomFocusRollbackRef.current; + zoomFocusRollbackRef.current = null; + zoomFocusLiveRef.current = null; + if (!(await saveDocument(doc)) && rollback) { + useProjectStore.setState((state) => + // `dirty` is deliberately NOT cleared. The rollback target is the last document + // this drag started from, which is not the same as the last SAVED one: with two + // commits in flight the first one's unsaved document is what we restore. Saying + // "clean" there tells `beforeunload` and `setHasUnsavedChanges` there is nothing + // to save, and the window closes on real work without prompting. + state.document === doc ? { document: rollback, revision: state.revision + 1 } : {}, + ); + } }, [saveDocument]); // Zoom-level control for the region-settings panel (1-6, matches @@ -590,11 +625,13 @@ export function useTimeline() { (id: string, patch: Partial) => { const doc = useProjectStore.getState().document; if (!doc) return; + if (annotationLiveRef.current !== doc) annotationRollbackRef.current = doc; const next: AxcutDocument = { ...doc, annotations: patchPillById(doc.annotations, id, patch), }; setDocument(next); + annotationLiveRef.current = next; }, [setDocument], ); @@ -602,7 +639,19 @@ export function useTimeline() { const commitAnnotationChange = useCallback(async () => { const doc = useProjectStore.getState().document; if (!doc) return; - await saveDocument(doc); + const rollback = annotationRollbackRef.current; + annotationRollbackRef.current = null; + annotationLiveRef.current = null; + if (!(await saveDocument(doc)) && rollback) { + useProjectStore.setState((state) => + // `dirty` is deliberately NOT cleared. The rollback target is the last document + // this drag started from, which is not the same as the last SAVED one: with two + // commits in flight the first one's unsaved document is what we restore. Saying + // "clean" there tells `beforeunload` and `setHasUnsavedChanges` there is nothing + // to save, and the window closes on real work without prompting. + state.document === doc ? { document: rollback, revision: state.revision + 1 } : {}, + ); + } }, [saveDocument]); const updateSpeedSpan = useCallback( @@ -692,7 +741,7 @@ export function useTimeline() { async (kind: RegionKind, id: string) => { if (!document) return; // One shared mutator with the agent's removeTrim / removeModifier tools. - await saveDocument(removeRegionInDocument(document, kind, id)); + if (!(await saveDocument(removeRegionInDocument(document, kind, id)))) return; if (selection?.id === id) setSelection(null); setMultiSelection((prev) => prev.filter((h) => h.id !== id)); }, @@ -743,7 +792,7 @@ export function useTimeline() { ? { ...legacy, speedRegions: prevSpeed, cameraFullscreenRegions: prevCameraFullscreen } : document.legacyEditor, }; - await saveDocument(next); + if (!(await saveDocument(next))) return; setSelection(null); setMultiSelection([]); }, @@ -926,7 +975,7 @@ export function useTimeline() { timeline: { ...currentDoc.timeline, clips: newClips }, }; const finalDoc = rederiveRegionMs(next, newClips); - await saveDocument(finalDoc); + if (!(await saveDocument(finalDoc))) return; setClipSelection(newClip.id); // If we used the placeholder, kick off the probe in the background. @@ -971,7 +1020,7 @@ export function useTimeline() { // original, so its index in the result is the original's index + 1. const insertedIndex = document.timeline.clips.findIndex((c) => c.id === clipId) + 1; const next = duplicateClipInDocument(document, clipId, "user", "Duplicated clip"); - await saveDocument(next); + if (!(await saveDocument(next))) return; setClipSelection(next.timeline.clips[insertedIndex]?.id ?? null); }, [document, saveDocument], @@ -981,7 +1030,7 @@ export function useTimeline() { async (clipId: string) => { if (!document) return; // One shared mutator with the agent's removeClip tool: reflow survivors + rederive pills. - await saveDocument(removeClipInDocument(document, clipId)); + if (!(await saveDocument(removeClipInDocument(document, clipId)))) return; if (clipSelection === clipId) setClipSelection(null); }, [document, clipSelection, saveDocument],