From f2ce44708ab6108b17839b42799a97f7de765475 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 14 Nov 2026 11:16:18 -0500 Subject: [PATCH 1/2] fix(studio): a refused edit names the file and why, on every edit path --- .../studio/src/hooks/gsapMutationClient.ts | 8 ++++-- .../hooks/useDomEditCommitsHelpers.test.ts | 16 ++++++++++++ .../src/hooks/useDomEditCommitsHelpers.ts | 4 ++- .../src/hooks/useElementLifecycleOps.ts | 1 + packages/studio/src/hooks/useGroupCommits.ts | 3 +++ .../src/hooks/useTimelineDeleteOps.test.tsx | 26 +++++++++++++++++++ .../studio/src/hooks/useTimelineDeleteOps.ts | 4 ++- .../src/utils/studioSaveDiagnostics.test.ts | 10 +++++++ .../studio/src/utils/studioSaveDiagnostics.ts | 10 +++++++ .../src/utils/timelineCompositionInsert.ts | 4 ++- 10 files changed, 81 insertions(+), 5 deletions(-) diff --git a/packages/studio/src/hooks/gsapMutationClient.ts b/packages/studio/src/hooks/gsapMutationClient.ts index cbe53c3cd51..bd3564e426c 100644 --- a/packages/studio/src/hooks/gsapMutationClient.ts +++ b/packages/studio/src/hooks/gsapMutationClient.ts @@ -1,5 +1,6 @@ import { studioWriteHeaders } from "../utils/studioFileVersion"; import { studioApiFetch } from "../utils/studioApiFetch"; +import { heldFileWordsOf } from "../utils/studioSaveDiagnostics"; export class GsapPreviewConvergenceError extends Error {} export class GsapOwnershipProtocolError extends GsapPreviewConvergenceError {} @@ -41,7 +42,9 @@ export async function rollbackOwnedMutation( }, ); if (!response.ok) { - throw new Error(`Failed to restore ${targetPath}`); + throw new Error( + (await heldFileWordsOf(response, targetPath)) ?? `Failed to restore ${targetPath}`, + ); } const result = (await response.json()) as { restored?: unknown; conflict?: unknown }; if (result.restored === true && result.conflict === false) return "restored"; @@ -121,9 +124,10 @@ async function postOwnedGsapMutation( cause: error, }); } + const held = response.ok ? null : await heldFileWordsOf(response, filePath); const payload: unknown = await response.json().catch(() => null); if (!response.ok) { - throw new GsapPreviewConvergenceError(readMutationError(payload, fallback)); + throw new GsapPreviewConvergenceError(held ?? readMutationError(payload, fallback)); } return readMutationStatus(payload); } diff --git a/packages/studio/src/hooks/useDomEditCommitsHelpers.test.ts b/packages/studio/src/hooks/useDomEditCommitsHelpers.test.ts index f0242c2d6f3..08bd70ebcfb 100644 --- a/packages/studio/src/hooks/useDomEditCommitsHelpers.test.ts +++ b/packages/studio/src/hooks/useDomEditCommitsHelpers.test.ts @@ -82,6 +82,22 @@ describe("assignGsapTargetAutoIdIfNeeded", () => { expect(showToast).toHaveBeenCalledWith("Couldn't save edit: target not found", "error"); }); + it("names the file and why when the file is read-only", async () => { + stubPatch( + new Response(JSON.stringify({ error: "permission denied", why: "permission" }), { + status: 403, + headers: { "content-type": "application/json" }, + }), + ); + const { result, showToast } = assign(); + + await expect(result).resolves.toBeNull(); + expect(showToast).toHaveBeenCalledWith( + "index.html can't be changed: it is read-only or protected. Check its permissions, then try again.", + "error", + ); + }); + it("is null with a plain toast when the refusal has no JSON reason", async () => { stubPatch(new Response("boom", { status: 500, headers: { "content-type": "text/plain" } })); const { result, showToast } = assign(); diff --git a/packages/studio/src/hooks/useDomEditCommitsHelpers.ts b/packages/studio/src/hooks/useDomEditCommitsHelpers.ts index f2dfa12b342..d6bcb3a9a6c 100644 --- a/packages/studio/src/hooks/useDomEditCommitsHelpers.ts +++ b/packages/studio/src/hooks/useDomEditCommitsHelpers.ts @@ -1,5 +1,6 @@ import { createStudioSaveHttpError, + heldFileWordsOf, StudioSaveHttpError, trackStudioSaveFailure, } from "../utils/studioSaveDiagnostics"; @@ -195,7 +196,8 @@ export async function postPatchElement( }, ); if (!response.ok) { - showToast(formatPatchRejectionMessage(await readErrorResponseBody(response)), "error"); + const held = await heldFileWordsOf(response, targetPath); + showToast(held ?? formatPatchRejectionMessage(await readErrorResponseBody(response)), "error"); throw await createStudioSaveHttpError(response, `Failed to patch ${targetPath}`, { alreadyToasted: true, }); diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index a4567d9ea3f..d2dfe816e87 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -172,6 +172,7 @@ export function useElementLifecycleOps({ throw await createStudioSaveHttpError( removeResponse, `Failed to delete element from ${targetPath}`, + { path: targetPath }, ); } const removeData = (await removeResponse.json()) as { diff --git a/packages/studio/src/hooks/useGroupCommits.ts b/packages/studio/src/hooks/useGroupCommits.ts index 56324485b0d..0a520ba4cf4 100644 --- a/packages/studio/src/hooks/useGroupCommits.ts +++ b/packages/studio/src/hooks/useGroupCommits.ts @@ -17,6 +17,7 @@ import { } from "../utils/studioHelpers"; import type { TimelineElement } from "../player"; import { studioApiFetch } from "../utils/studioApiFetch"; +import { heldFileWordsOf } from "../utils/studioSaveDiagnostics"; interface UseGroupCommitsParams extends DomEditCommitBaseParams { /** Resync the SDK session after a server-side write (the wrapper/unwrap changes @@ -142,6 +143,8 @@ async function commitStructuralMutation( }, ); if (!mutateResponse.ok) { + const held = await heldFileWordsOf(mutateResponse, targetPath); + if (held) throw new Error(held); const errBody = (await mutateResponse.json().catch(() => null)) as { error?: string; } | null; diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx index fbc5b8bd717..f4d0357ace1 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx +++ b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx @@ -184,6 +184,32 @@ describe("useTimelineDeleteOps: ripple undo label", () => { ]); }); + it.each([ + [ + 403, + "permission", + "index.html can't be changed: it is read-only or protected. Check its permissions, then try again.", + ], + [423, "busy", "index.html is open in another program. Close it there, then try again."], + ])("a delete the server refuses with %i names the file and why", async (status, why, words) => { + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => + String(input).includes("/file-mutations/remove-elements/") + ? new Response(JSON.stringify({ error: "refused", why }), { status }) + : new Response(JSON.stringify({ content: html }), { status: 200 }), + ), + ); + const showToast = vi.fn(); + const { b, getHook } = mountDeleteHarness({ handleTimelineGroupMove: vi.fn(), showToast }); + + await act(async () => { + await getHook().handleTimelineElementDelete(b); + }); + + expect(showToast.mock.calls.map(([message]) => message)).toEqual([words]); + }); + it("shows exactly one toast when a ripple fails to persist after a committed delete", async () => { const handleTimelineGroupMove = vi.fn().mockRejectedValue(new Error("persist failed")); const showToast = vi.fn(); diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.ts b/packages/studio/src/hooks/useTimelineDeleteOps.ts index 4b513108595..67273a2a824 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.ts +++ b/packages/studio/src/hooks/useTimelineDeleteOps.ts @@ -26,6 +26,7 @@ import type { } from "./useTimelineGroupEditing"; import { studioApiFetch } from "../utils/studioApiFetch"; import { countOf } from "../utils/countOf"; +import { heldFileWordsOf } from "../utils/studioSaveDiagnostics"; /** Apply already-resolved ripple changes to the surviving elements for the * optimistic store update after a delete. Pure — no IO. */ @@ -141,7 +142,8 @@ export function useTimelineDeleteOps({ ); if (!removeResponse.ok) { throw new Error( - `Failed to delete ${countOf(sameFile.length, "clip")} from ${targetPath}`, + (await heldFileWordsOf(removeResponse, targetPath)) ?? + `Failed to delete ${countOf(sameFile.length, "clip")} from ${targetPath}`, ); } const removeData = (await removeResponse.json()) as { content?: string }; diff --git a/packages/studio/src/utils/studioSaveDiagnostics.test.ts b/packages/studio/src/utils/studioSaveDiagnostics.test.ts index 8c239d9900f..a329cb68d4e 100644 --- a/packages/studio/src/utils/studioSaveDiagnostics.test.ts +++ b/packages/studio/src/utils/studioSaveDiagnostics.test.ts @@ -8,6 +8,7 @@ import { StudioSaveHttpError, StudioSaveNetworkError, createStudioSaveHttpError, + heldFileWordsOf, buildStudioSaveFailureProperties, getStudioSaveStatusCode, retryStudioSave, @@ -32,6 +33,15 @@ describe("a file the server says is held or protected", () => { expect(error.statusCode).toBe(status); }); + it("reads a refused answer's words from a copy, leaving its body for the caller", async () => { + const response = new Response(JSON.stringify({ error: "x", why: "busy" }), { status: 423 }); + expect(await heldFileWordsOf(response, "a/index.html")).toBe( + "index.html is open in another program. Close it there, then try again.", + ); + expect(await response.json()).toEqual({ error: "x", why: "busy" }); + expect(await heldFileWordsOf(new Response("boom", { status: 500 }), "index.html")).toBeNull(); + }); + it("keeps the status and body for any other answer, and without a path", async () => { const other = await createStudioSaveHttpError( new Response("boom", { status: 500 }), diff --git a/packages/studio/src/utils/studioSaveDiagnostics.ts b/packages/studio/src/utils/studioSaveDiagnostics.ts index 2bbd6092cef..f60b7f0bdcd 100644 --- a/packages/studio/src/utils/studioSaveDiagnostics.ts +++ b/packages/studio/src/utils/studioSaveDiagnostics.ts @@ -211,6 +211,16 @@ function heldFileMessage(body: string, path: string | undefined): string | null } } +export async function heldFileWordsOf(response: Response, path: string): Promise { + return heldFileMessage( + await response + .clone() + .text() + .catch(() => ""), + path, + ); +} + export async function createStudioSaveHttpError( response: Response, fallbackMessage: string, diff --git a/packages/studio/src/utils/timelineCompositionInsert.ts b/packages/studio/src/utils/timelineCompositionInsert.ts index 1b5f6a1f783..312ff2eaed8 100644 --- a/packages/studio/src/utils/timelineCompositionInsert.ts +++ b/packages/studio/src/utils/timelineCompositionInsert.ts @@ -49,7 +49,9 @@ async function insertTimelineComposition(input: { }, ); if (!response.ok) { - throw await createStudioSaveHttpError(response, "Failed to add composition to timeline"); + throw await createStudioSaveHttpError(response, "Failed to add composition to timeline", { + path: input.targetPath, + }); } return (await response.json()) as TimelineCompositionInsertionResult; } From b34649fb7fa4258a6c25aa615200f32a4fd31c70 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 14 Nov 2026 12:00:59 -0500 Subject: [PATCH 2/2] fix(studio): freeze frame and caption save name a refused file too --- .../src/captions/hooks/useCaptionSync.test.ts | 45 +++++++++++++++++++ .../src/captions/hooks/useCaptionSync.ts | 11 +++-- .../studio/src/hooks/useFreezeFrame.test.ts | 9 ++++ packages/studio/src/hooks/useFreezeFrame.ts | 4 +- 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 packages/studio/src/captions/hooks/useCaptionSync.test.ts diff --git a/packages/studio/src/captions/hooks/useCaptionSync.test.ts b/packages/studio/src/captions/hooks/useCaptionSync.test.ts new file mode 100644 index 00000000000..907c9a238dd --- /dev/null +++ b/packages/studio/src/captions/hooks/useCaptionSync.test.ts @@ -0,0 +1,45 @@ +// @vitest-environment happy-dom +import { act, createElement } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useCaptionStore } from "../store"; +import type { CaptionModel } from "../types"; +import { useCaptionSync } from "./useCaptionSync"; + +afterEach(() => vi.unstubAllGlobals()); + +function Harness() { + useCaptionSync("p"); + return null; +} + +describe("useCaptionSync", () => { + it.each([ + [ + { why: "permission" }, + "caption-overrides.json can't be changed: it is read-only or protected. Check its permissions, then try again.", + ], + [{ error: "boom" }, "Caption changes couldn't be saved"], + ])("says why a refused save failed: %o", async (answer, words) => { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(JSON.stringify(answer), { status: 403 })), + ); + const root = createRoot(document.createElement("div")); + await act(async () => root.render(createElement(Harness))); + const model = { groupOrder: [], groups: new Map(), segments: new Map() }; + useCaptionStore.setState({ + model: model as unknown as CaptionModel, + sourceFilePath: "index.html", + }); + + await act(async () => { + useCaptionStore.getState().retrySave?.(); + await vi.waitFor(() => expect(useCaptionStore.getState().syncError).not.toBeNull()); + }); + + expect(useCaptionStore.getState().syncError).toBe(words); + await act(async () => root.unmount()); + useCaptionStore.setState({ model: null, sourceFilePath: null, syncError: null }); + }); +}); diff --git a/packages/studio/src/captions/hooks/useCaptionSync.ts b/packages/studio/src/captions/hooks/useCaptionSync.ts index 20424d953e6..8f22e5dd6ec 100644 --- a/packages/studio/src/captions/hooks/useCaptionSync.ts +++ b/packages/studio/src/captions/hooks/useCaptionSync.ts @@ -6,6 +6,7 @@ import { trackEvent } from "../../telemetry/client"; import type { CaptionStyle } from "../types"; import { studioWriteHeaders } from "../../utils/studioFileVersion"; import { studioApiFetch } from "../../utils/studioApiFetch"; +import { heldFileWordsOf } from "../../utils/studioSaveDiagnostics"; interface CaptionOverrideEntry { wordId?: string; @@ -93,6 +94,7 @@ export function useCaptionSync(projectId: string | null) { const seqAtSave = editSeqRef.current; const overrides = buildOverrides(state.model); + let held: string | null = null; studioApiFetch( buildProjectApiPath(pid, `/files/${encodeURIComponent("caption-overrides.json")}`), @@ -102,8 +104,11 @@ export function useCaptionSync(projectId: string | null) { body: JSON.stringify(overrides, null, 2), }, ) - .then((res) => { - if (!res.ok) throw new Error(`HTTP ${res.status}`); + .then(async (res) => { + if (!res.ok) { + held = await heldFileWordsOf(res, "caption-overrides.json"); + throw new Error(`HTTP ${res.status}`); + } // A newer edit may have re-armed the debounce while this PUT was in // flight — its beforeunload/unmount flush still needs pending=true. if (editSeqRef.current === seqAtSave) pendingRef.current = false; @@ -114,7 +119,7 @@ export function useCaptionSync(projectId: string | null) { // Caption auto-save is a data-loss path: surface it to the user, not // just telemetry. pendingRef stays true so beforeunload still warns. trackEvent("studio_caption_autosave_failed", { error: String(error) }); - useCaptionStore.getState().setSyncError("Caption changes couldn't be saved"); + useCaptionStore.getState().setSyncError(held ?? "Caption changes couldn't be saved"); }); }, []); diff --git a/packages/studio/src/hooks/useFreezeFrame.test.ts b/packages/studio/src/hooks/useFreezeFrame.test.ts index 2598c94f9b0..c6e779a7c30 100644 --- a/packages/studio/src/hooks/useFreezeFrame.test.ts +++ b/packages/studio/src/hooks/useFreezeFrame.test.ts @@ -57,6 +57,15 @@ describe("requestFreezeFrame", () => { requestFreezeFrame({ projectId: "p", path: "index.html", element, playhead: 7 }), ).rejects.toThrow("Move the playhead inside a video clip to freeze"); }); + + it("names the file and why when the file is read-only", async () => { + stubFetch({ status: 403, body: { error: "permission denied", why: "permission" } }); + await expect( + requestFreezeFrame({ projectId: "p", path: "scenes/intro.html", element, playhead: 7 }), + ).rejects.toThrow( + "intro.html can't be changed: it is read-only or protected. Check its permissions, then try again.", + ); + }); }); describe("useFreezeFrame", () => { diff --git a/packages/studio/src/hooks/useFreezeFrame.ts b/packages/studio/src/hooks/useFreezeFrame.ts index 70152479883..0f74beb4cbe 100644 --- a/packages/studio/src/hooks/useFreezeFrame.ts +++ b/packages/studio/src/hooks/useFreezeFrame.ts @@ -7,6 +7,7 @@ import { markStudioWriteToken } from "../utils/studioFileVersion"; import { serializeStudioFileMutations } from "../utils/studioFileMutationCoordinator"; import type { RecordEditInput } from "../utils/studioFileHistory"; import { studioApiFetch } from "../utils/studioApiFetch"; +import { heldFileWordsOf } from "../utils/studioSaveDiagnostics"; type ProjectFileWriter = (path: string, content: string, expectedContent?: string) => Promise; @@ -73,9 +74,10 @@ export async function requestFreezeFrame(input: { }), }, ); + const held = response.ok ? null : await heldFileWordsOf(response, input.path); const body: unknown = await response.json().catch(() => null); if (!response.ok || !isFreezeFrameResponse(body)) { - throw new Error(errorOf(body) ?? `Freeze frame failed (${response.status})`); + throw new Error(held ?? errorOf(body) ?? `Freeze frame failed (${response.status})`); } return body; }