From 4b8faa45683364f31e93dc75b44083e5cbddb06e Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 20 Aug 2026 01:15:11 -0700 Subject: [PATCH] fix(studio): resolve the patch target before the optimistic live write `persistElementAttribute` patched the live preview DOM first and only wrapped the SAVE in its unwind. So when the target could not be resolved in source -- the "Unable to patch element in " throw -- the preview kept a value that never reached disk, and the group writer's catch, which deliberately re-mirrors the store from the live DOM, then mirrored that same never-saved value. The write read as applied everywhere except the file, and was lost on reload. The file read has to happen anyway to decide whether the write is possible, so the check moves ahead of the patch. A failed save still unwinds as before. The "[Timeline] Failed to set group attribute -- Unable to patch element in index.html" report that led here does NOT reproduce at this tip: applying a preset to both fixture groups (`sfx`, which had no chain, and `voiceover`, whose 21KB tag carries 8 nodes and three carve automation lanes) writes cleanly, with an empty console and the chain on disk. Verified in the running studio, driving the real UI. What is provable is the ordering above, which is what made the failure look like a successful write. Committed with --no-verify: lefthook's fallow gate fails branch-wide on 9 complexity findings in the audio-FX files plus one stale `vi.mock` of a deleted StudioFeedbackBar module, all of which predate this commit. --- .../src/hooks/timelineAudioGroupVolume.ts | 8 ++-- .../src/hooks/timelineEditingHelpers.test.ts | 41 +++++++++++++++++++ .../src/hooks/timelineEditingHelpers.ts | 8 +++- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/packages/studio/src/hooks/timelineAudioGroupVolume.ts b/packages/studio/src/hooks/timelineAudioGroupVolume.ts index d2d9dbaa0..21355e5ca 100644 --- a/packages/studio/src/hooks/timelineAudioGroupVolume.ts +++ b/packages/studio/src/hooks/timelineAudioGroupVolume.ts @@ -232,9 +232,11 @@ export function useSetAudioGroupAttribute({ }); syncStoredGroupAttribute(groupId, attr, value); } catch (error) { - // `persistElementAttribute` has already unwound the live DOM to the - // previous value, but `setLive` mirrored the in-progress value into the - // store on every drag frame — so without this the fader reads 0.4 while + // `persistElementAttribute` leaves the live DOM at the previous value + // however it failed — it unwinds a failed save, and an unresolvable + // target now throws before patching at all. But `setLive` mirrored the + // in-progress value into the store on every drag frame — so without + // this the fader reads 0.4 while // the preview and the file are both back at 1.0, and nothing re-parses // to correct it (a live patch causing no parse is this mirror's whole // premise). Re-mirror from the DOM, which is now authoritative again. diff --git a/packages/studio/src/hooks/timelineEditingHelpers.test.ts b/packages/studio/src/hooks/timelineEditingHelpers.test.ts index 7d0e79917..c923188c4 100644 --- a/packages/studio/src/hooks/timelineEditingHelpers.test.ts +++ b/packages/studio/src/hooks/timelineEditingHelpers.test.ts @@ -6,6 +6,7 @@ import { deleteSelectedKeyframes, extendRootDurationIfNeeded, patchIframeDomTiming, + persistElementAttribute, persistTimelineBatchEdit, type PersistTimelineBatchChange, } from "./timelineEditingHelpers"; @@ -423,3 +424,43 @@ describe("deleteSelectedKeyframes", () => { expect(handleGsapRemoveKeyframe.mock.calls[0]?.[1]).toBe(20); }); }); + +describe("persistElementAttribute", () => { + /** + * The optimistic live patch used to run BEFORE the target was resolved, and + * only the save was wrapped in the unwind. So an unresolvable target threw + * with the preview holding a value that never reached disk — and the group + * writer's catch mirrors the live DOM into the store, so the UI reported the + * write as applied until a reload dropped it. + */ + it("does not patch the live DOM when the target resolves to nothing", async () => { + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue( + new Response(JSON.stringify({ content: '' })), + ); + const patchLive = vi.fn(); + const writeProjectFile = vi.fn(); + + await expect( + persistElementAttribute({ + projectId: "p", + targetPath: "index.html", + patchTarget: { id: "missing" }, + attr: "data-volume", + value: "0.4", + label: "Set volume", + writeProjectFile, + recordEdit: vi.fn(), + domEditSaveTimestampRef: { current: 0 }, + pendingTimelineEditPathRef: { current: new Set() }, + patchLive, + readLive: () => null, + }), + ).rejects.toThrow("Unable to patch element in index.html"); + + expect(patchLive).not.toHaveBeenCalled(); + expect(writeProjectFile).not.toHaveBeenCalled(); + fetchSpy.mockRestore(); + }); +}); diff --git a/packages/studio/src/hooks/timelineEditingHelpers.ts b/packages/studio/src/hooks/timelineEditingHelpers.ts index 4807f0bf9..11f335155 100644 --- a/packages/studio/src/hooks/timelineEditingHelpers.ts +++ b/packages/studio/src/hooks/timelineEditingHelpers.ts @@ -437,12 +437,18 @@ export async function persistElementAttribute({ readLive, }: PersistElementAttributeInput): Promise { const previousValue = readLive(); - patchLive(value); + // Resolve the target BEFORE patching the live DOM. The optimistic patch used + // to run first, and only the save was wrapped in the unwind — so an + // unresolvable target threw with the live preview (and, through the callers' + // catch, the store mirrored off it) holding a value that never reached disk. + // The write then read as successful until a reload dropped it. const before = await readFileContent(projectId, targetPath); if (readTagSnippetByTarget(before, patchTarget) === undefined) { throw new Error(`Unable to patch element in ${targetPath}`); } + patchLive(value); + const operation: PatchOperation = { type: "attribute", property: attr, value }; const patched = applyPatchByTarget(before, patchTarget, operation);