fix(studio): an effect save resolves with whether it landed, was refused or failed (#4531)

* fix(studio): an effect save resolves with whether it landed, was refused or failed

setElementFxAttribute.setQuiet and setAudioGroupAttribute.setQuiet, which
every timeline effect save goes through, resolved the same way whether the
write landed, the host's canEdit refused it, recording blocked it, or the
write failed; only Studio's toast told them apart. They now resolve with a
TimelineEditOutcome: saved, refused with the reason, or failed with the
error. The lock guard resolves with the refusal handler's result, and a
clip that cannot be addressed in its file reports failed instead of saved.

* fix(studio): the lock guard hands its reason to the refused effect save

The refusal reason comes from the gate's own canEdit pass instead of a
second call, a group save with nothing to address reports failed like the
clip save, and the failure message says the clip has no id to save it by.
This commit is contained in:
Miguel Ángel
2026-09-26 09:22:39 +00:00
committed by GitHub
parent 79144f914b
commit 5b034cab74
8 changed files with 200 additions and 74 deletions
@@ -1,3 +1,4 @@
import type { TimelineEditOutcome } from "../../hooks/timelineEditPermission";
import { useCallback, useMemo } from "react";
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
import type { TimelineElement } from "../../player";
@@ -40,7 +41,12 @@ export interface TimelineEditCallbackDeps {
handleToggleTrackHidden: (track: number, hidden: boolean) => Promise<void> | void;
setAudioGroupAttribute: {
setLive: (groupId: string, attr: string, value: string | null) => void;
setQuiet: (groupId: string, attr: string, value: string | null, label: string) => Promise<void>;
setQuiet: (
groupId: string,
attr: string,
value: string | null,
label: string,
) => Promise<TimelineEditOutcome | void>;
};
handleBlockedTimelineEdit: (element: TimelineElement, intent: BlockedTimelineEditIntent) => void;
handleTimelineElementSplit: (element: TimelineElement, splitTime: number) => Promise<void> | void;
@@ -60,7 +66,7 @@ export interface TimelineEditCallbackDeps {
attr: string,
value: string | null,
label: string,
) => Promise<void>;
) => Promise<TimelineEditOutcome | void>;
};
}
@@ -1,4 +1,5 @@
import { useCallback, useRef } from "react";
import { projectForTimelineSave, type TimelineEditOutcome } from "./timelineEditPermission";
import { HF_AUDIO_FX_ATTR } from "@hyperframes/core/audio-fx";
import { HF_AUDIO_AUTOMATION_ATTR } from "@hyperframes/core/audio-automation";
import { usePlayerStore } from "../player";
@@ -167,7 +168,7 @@ async function setAudioGroupAttribute({
writeProjectFile,
recordEdit,
pendingTimelineEditPathRef,
}: SetAudioGroupAttributeInput): Promise<string[]> {
}: SetAudioGroupAttributeInput): Promise<string[] | null> {
// The file that actually CONTAINS the group element, not just the active
// composition. A hand-authored sub-composition can declare both the members
// and their `<hf-audio-group>`, and until sub-comp children inherited
@@ -179,7 +180,7 @@ async function setAudioGroupAttribute({
const groupEl = previewIframe?.contentDocument?.getElementById(groupId) ?? null;
const targetPath = resolveGroupSourceFile(groupEl) || activeCompPath || "index.html";
const patchTarget = buildPatchTarget({ domId: groupId });
if (!patchTarget) return [];
if (!patchTarget) return null;
return persistElementAttribute({
projectId,
@@ -213,7 +214,12 @@ export function useSetAudioGroupAttribute({
isRecordingRef,
}: UseTimelineElementVisibilityEditingInput): {
setLive: (groupId: string, attr: string, value: string | null) => void;
setQuiet: (groupId: string, attr: string, value: string | null, label: string) => Promise<void>;
setQuiet: (
groupId: string,
attr: string,
value: string | null,
label: string,
) => Promise<TimelineEditOutcome>;
revertLive: (groupId: string, attr: string) => void;
} {
const liveBeforeRef = useRef(new Map<string, string | null>());
@@ -253,15 +259,16 @@ export function useSetAudioGroupAttribute({
[previewIframeRef],
);
const setQuiet = useCallback(
async (groupId: string, attr: string, value: string | null, label: string) => {
if (isRecordingRef?.current) {
showToast("Cannot edit timeline while recording", "error");
return;
}
const pid = projectIdRef.current;
if (!pid) return;
async (
groupId: string,
attr: string,
value: string | null,
label: string,
): Promise<TimelineEditOutcome> => {
const pid = projectForTimelineSave(isRecordingRef?.current, projectIdRef.current, showToast);
if (typeof pid !== "string") return pid;
try {
await setAudioGroupAttribute({
const written = await setAudioGroupAttribute({
projectId: pid,
activeCompPath,
groupId,
@@ -274,7 +281,9 @@ export function useSetAudioGroupAttribute({
pendingTimelineEditPathRef,
});
liveBeforeRef.current.delete(audioGroupAttributeLiveKey(groupId, attr));
if (!written) return { status: "failed", reason: "This group has no id to save it by" };
syncStoredGroupAttribute(groupId, attr, value);
return { status: "saved" };
} catch (error) {
// `persistElementAttribute` leaves the live DOM at the previous value
// however it failed — it unwinds a failed save, and an unresolvable
@@ -290,6 +299,7 @@ export function useSetAudioGroupAttribute({
const message = error instanceof Error ? error.message : "Failed to update group";
showToast(message);
liveBeforeRef.current.delete(audioGroupAttributeLiveKey(groupId, attr));
return { status: "failed", reason: message };
}
},
[
@@ -6,26 +6,60 @@ export type TimelineEditPermission = true | { blocked: true; reason: string };
export type CanEditTimelineElement = (element: TimelineElement) => TimelineEditPermission;
/** What an effect save did: landed, refused before writing, or failed while writing. */
export type TimelineEditOutcome =
| { status: "saved" }
| { status: "refused" | "failed"; reason: string };
/** The project to save into, or why the save stops before it writes. */
export function projectForTimelineSave(
isRecording: boolean | undefined,
projectId: string | null,
showToast: (message: string, tone?: "error" | "info") => void,
): string | TimelineEditOutcome {
if (isRecording) {
showToast("Cannot edit timeline while recording", "error");
return { status: "refused", reason: "Cannot edit timeline while recording" };
}
return projectId ?? { status: "failed", reason: "No project is open" };
}
function timelineEditRefusal(
canEdit: CanEditTimelineElement | undefined,
targets: readonly TimelineElement[],
): string | null {
for (const element of targets) {
const verdict = canEdit?.(element) ?? true;
if (verdict !== true) return verdict.reason;
}
return null;
}
/**
* Refuses a write when any target element is blocked, toasting the host's
* reason. Absent `canEdit` never refuses, so Studio itself is unchanged.
* The host's reason to refuse a write when any target element is blocked, toasted,
* or null. Absent `canEdit` never refuses, so Studio itself is unchanged.
*/
export function useTimelineEditGate(
export function useTimelineEditRefusal(
canEdit: CanEditTimelineElement | undefined,
showToast: (message: string, tone?: "error" | "info") => void,
) {
return useCallback(
(targets: readonly TimelineElement[]): boolean => {
if (!canEdit) return true;
for (const element of targets) {
const verdict = canEdit(element);
if (verdict !== true) {
showToast(verdict.reason, "error");
return false;
}
}
return true;
(targets: readonly TimelineElement[]): string | null => {
const reason = timelineEditRefusal(canEdit, targets);
if (reason !== null) showToast(reason, "error");
return reason;
},
[canEdit, showToast],
);
}
export function useTimelineEditGate(
canEdit: CanEditTimelineElement | undefined,
showToast: (message: string, tone?: "error" | "info") => void,
) {
const refuse = useTimelineEditRefusal(canEdit, showToast);
return useCallback(
(targets: readonly TimelineElement[]): boolean => refuse(targets) === null,
[refuse],
);
}
@@ -18,6 +18,7 @@ import type {
MutableRef,
UseTimelineElementVisibilityEditingInput,
} from "./timelineTrackVisibility";
import { projectForTimelineSave, type TimelineEditOutcome } from "./timelineEditPermission";
function patchLiveElementAttribute(
iframe: HTMLIFrameElement | null,
@@ -64,10 +65,10 @@ async function setElementAttribute({
writeProjectFile,
recordEdit,
pendingTimelineEditPathRef,
}: SetElementAttributeInput): Promise<string[]> {
}: SetElementAttributeInput): Promise<string[] | null> {
const targetPath = element.sourceFile || activeCompPath || "index.html";
const patchTarget = buildPatchTarget(element);
if (!patchTarget) return [];
if (!patchTarget) return null;
return persistElementAttribute({
projectId,
@@ -99,7 +100,7 @@ export function useSetElementAttribute({
attr: string,
value: string | null,
label: string,
) => Promise<void>;
) => Promise<TimelineEditOutcome>;
revertLive: (element: TimelineElement, attr: string) => void;
} {
const liveBeforeRef = useRef(new Map<string, string | null>());
@@ -130,15 +131,17 @@ export function useSetElementAttribute({
[previewIframeRef, activeCompPath],
);
const setQuiet = useCallback(
async (element: TimelineElement, attr: string, value: string | null, label: string) => {
if (isRecordingRef?.current) {
showToast("Cannot edit timeline while recording", "error");
return;
}
const pid = projectIdRef.current;
if (!pid) return;
async (
element: TimelineElement,
attr: string,
value: string | null,
label: string,
): Promise<TimelineEditOutcome> => {
const pid = projectForTimelineSave(isRecordingRef?.current, projectIdRef.current, showToast);
if (typeof pid !== "string") return pid;
const liveKey = elementAttributeLiveKey(element, activeCompPath, attr);
try {
await setElementAttribute({
const written = await setElementAttribute({
projectId: pid,
activeCompPath,
element,
@@ -150,12 +153,15 @@ export function useSetElementAttribute({
recordEdit,
pendingTimelineEditPathRef,
});
liveBeforeRef.current.delete(elementAttributeLiveKey(element, activeCompPath, attr));
liveBeforeRef.current.delete(liveKey);
if (written) return { status: "saved" };
return { status: "failed", reason: "This clip has no id to save it by" };
} catch (error) {
console.error("[Timeline] Failed to set element attribute", error);
const message = error instanceof Error ? error.message : "Failed to update effect";
showToast(message);
liveBeforeRef.current.delete(elementAttributeLiveKey(element, activeCompPath, attr));
liveBeforeRef.current.delete(liveKey);
return { status: "failed", reason: message };
}
},
[
@@ -131,6 +131,7 @@ function renderTimelineEditingHook(input: {
elementsDelete: ReturnType<typeof useTimelineEditing>["handleTimelineElementsDelete"];
handleAutoGroupCarveSources: ReturnType<typeof useTimelineEditing>["handleAutoGroupCarveSources"];
setAudioGroupAttribute: ReturnType<typeof useTimelineEditing>["setAudioGroupAttribute"];
setElementFxAttribute: ReturnType<typeof useTimelineEditing>["setElementFxAttribute"];
unmount: () => void;
} {
let move: ReturnType<typeof useTimelineEditing>["handleTimelineElementMove"] | null = null;
@@ -146,6 +147,7 @@ function renderTimelineEditingHook(input: {
let setAudioGroupAttribute:
| ReturnType<typeof useTimelineEditing>["setAudioGroupAttribute"]
| null = null;
let latest: ReturnType<typeof useTimelineEditing> | null = null;
function Harness() {
const commitRef = useRef(input.onZIndexCommit);
@@ -176,6 +178,7 @@ function renderTimelineEditingHook(input: {
elementsDelete = hook.handleTimelineElementsDelete;
handleAutoGroupCarveSources = hook.handleAutoGroupCarveSources;
setAudioGroupAttribute = hook.setAudioGroupAttribute;
latest = hook;
return null;
}
@@ -197,6 +200,8 @@ function renderTimelineEditingHook(input: {
elementsDelete,
handleAutoGroupCarveSources,
setAudioGroupAttribute,
setElementFxAttribute: (latest as unknown as ReturnType<typeof useTimelineEditing>)
.setElementFxAttribute,
unmount,
};
}
@@ -1910,3 +1915,57 @@ describe("useTimelineEditing: handler identity is stable across renders", () =>
unmount();
});
});
describe("useTimelineEditing effect saves report what happened", () => {
const LOCKED = { blocked: true as const, reason: "Reserved by an agent" };
it("resolves a saved clip effect as saved", async () => {
const { clip, setElementFxAttribute, writeProjectFile, unmount } = setupSingleClipHarness();
let outcome: unknown;
await act(async () => {
outcome = await setElementFxAttribute.setQuiet(clip, "data-hf-audio-fx", "echo", "Apply");
});
expect(outcome).toEqual({ status: "saved" });
expect(writeProjectFile).toHaveBeenCalled();
unmount();
});
it("resolves a clip effect on a locked clip as refused, with the host's reason", async () => {
const { clip, setElementFxAttribute, writeProjectFile, unmount } = setupSingleClipHarness({
canEdit: () => LOCKED,
});
let outcome: unknown;
await act(async () => {
outcome = await setElementFxAttribute.setQuiet(clip, "data-hf-audio-fx", "echo", "Apply");
});
expect(outcome).toEqual({ status: "refused", reason: "Reserved by an agent" });
expect(writeProjectFile).not.toHaveBeenCalled();
unmount();
});
it("resolves a clip effect whose write fails as failed", async () => {
const { clip, setElementFxAttribute, writeProjectFile, unmount } = setupSingleClipHarness();
writeProjectFile.mockRejectedValue(new Error("disk full"));
let outcome: unknown;
await act(async () => {
outcome = await setElementFxAttribute.setQuiet(clip, "data-hf-audio-fx", "echo", "Apply");
});
expect(outcome).toEqual({ status: "failed", reason: expect.stringContaining("disk full") });
unmount();
});
it("resolves a group audio effect on a locked group as refused", async () => {
const member = timelineElement({ id: "clip", track: 0, zIndex: 0 });
usePlayerStore.getState().setElements([{ ...member, audioGroup: "hf-group" }]);
const { setAudioGroupAttribute, writeProjectFile, unmount } = setupSingleClipHarness({
canEdit: () => LOCKED,
});
let outcome: unknown;
await act(async () => {
outcome = await setAudioGroupAttribute.setQuiet("hf-group", "data-volume", "0.5", "Volume");
});
expect(outcome).toEqual({ status: "refused", reason: "Reserved by an agent" });
expect(writeProjectFile).not.toHaveBeenCalled();
unmount();
});
});
+42 -33
View File
@@ -33,15 +33,19 @@ import {
} from "./timelineTrackVisibility";
import { useTimelineGroupEditing } from "./useTimelineGroupEditing";
import { useBlockedTimelineEditToast } from "./useBlockedTimelineEditToast";
import { useTimelineEditGate } from "./timelineEditPermission";
import {
useTimelineEditGate,
useTimelineEditRefusal,
type TimelineEditOutcome,
} from "./timelineEditPermission";
import { serializeZLaneGesture } from "../components/nle/zLaneGesture";
import { cutoverCommittedOrThrow, sdkTimingPersist } from "../utils/sdkCutover";
import type { TimelineMoveUpdates, UseTimelineEditingOptions } from "./useTimelineEditingTypes";
import { getStudioSaveErrorMessage } from "../utils/studioSaveDiagnostics";
type GuardedTimelineHandler = (...args: never[]) => Promise<void>;
type GuardedTimelineHandler = (...args: never[]) => Promise<unknown>;
type GuardedTimelineResolver = (...args: never[]) => readonly TimelineElement[];
type GuardedTimelineRefusal = (...args: never[]) => void;
type GuardedTimelineRefusal = (reason: string, ...args: never[]) => unknown;
interface GuardedTimelineEntry {
resolveTargets: GuardedTimelineResolver;
@@ -74,17 +78,18 @@ export function useTimelineEditing({
const editQueueRef = useRef(Promise.resolve());
const track = useTrackPendingTimelineEdit();
const checkEditable = useTimelineEditGate(canEdit, showToast);
const checkEditableRef = useRef(checkEditable);
checkEditableRef.current = checkEditable;
const refuseEdit = useTimelineEditRefusal(canEdit, showToast);
const refuseEditRef = useRef(refuseEdit);
refuseEditRef.current = refuseEdit;
const guardedRef = useRef(new WeakMap<GuardedTimelineHandler, GuardedTimelineEntry>());
// Refuses (no call, no write, no history entry) when any target is
// blocked; otherwise runs fn as before. Cached by fn identity — like
// track() — so a fresh closure here doesn't defeat track's own cache.
const guard = useCallback(
<H extends (...args: never[]) => Promise<void>>(
<H extends (...args: never[]) => Promise<unknown>>(
resolveTargets: (...args: Parameters<H>) => readonly TimelineElement[],
fn: H,
onRefused?: (...args: Parameters<H>) => void,
onRefused?: (reason: string, ...args: Parameters<H>) => Awaited<ReturnType<H>>,
): H => {
const key = fn as unknown as GuardedTimelineHandler;
const cached = guardedRef.current.get(key);
@@ -97,10 +102,9 @@ export function useTimelineEditing({
entry.resolveTargets = resolveTargets as unknown as GuardedTimelineResolver;
entry.onRefused = onRefused as unknown as GuardedTimelineRefusal | undefined;
entry.wrapped = ((...args: Parameters<H>) => {
if (!checkEditableRef.current(entry.resolveTargets(...(args as never[])))) {
entry.onRefused?.(...(args as never[]));
return Promise.resolve();
}
const reason = refuseEditRef.current(entry.resolveTargets(...(args as never[])));
if (reason !== null)
return Promise.resolve(entry.onRefused?.(reason, ...(args as never[])));
return fn(...args);
}) as H as unknown as GuardedTimelineHandler;
guardedRef.current.set(key, entry);
@@ -500,6 +504,25 @@ export function useTimelineEditing({
forceReloadSdkSession,
});
const refused = (reason: string): TimelineEditOutcome => ({ status: "refused", reason });
const audioGroupMembers = (groupId: string): TimelineElement[] => {
const state = usePlayerStore.getState();
const flatMembers = state.elements.filter((el) => el.audioGroup === groupId);
const domMembers = state.domClipChildren
.filter((child) => child.audioGroup === groupId)
.map(
(child): TimelineElement => ({
id: child.id,
domId: child.id,
tag: "div",
start: 0,
duration: 0,
track: -1,
}),
);
return [...flatMembers, ...domMembers];
};
// Every write-handler is tracked here, the one place all hand edits
// converge, so undo never races a write; canEdit gates the same point.
// Coverage boundary: see the PR body, not every kind resolves an element.
@@ -526,27 +549,10 @@ export function useTimelineEditing({
// (timelineAudioGroupVolume.ts): a sub-composition's group members have
// no flat twin, only a domClipChildren entry, so both are checked.
setQuiet: track(
guard(
(groupId) => {
const state = usePlayerStore.getState();
const flatMembers = state.elements.filter((el) => el.audioGroup === groupId);
const domMembers = state.domClipChildren
.filter((child) => child.audioGroup === groupId)
.map(
(child): TimelineElement => ({
id: child.id,
domId: child.id,
tag: "div",
start: 0,
duration: 0,
track: -1,
}),
);
return [...flatMembers, ...domMembers];
},
setAudioGroupAttribute.setQuiet,
(groupId, attr) => revertAudioGroupLive(groupId, attr),
),
guard(audioGroupMembers, setAudioGroupAttribute.setQuiet, (reason, groupId, attr) => {
revertAudioGroupLive(groupId, attr);
return refused(reason);
}),
),
},
setElementFxAttribute: {
@@ -555,7 +561,10 @@ export function useTimelineEditing({
guard(
(element) => [element],
setElementFxAttribute.setQuiet,
(element, attr) => revertElementFxLive(element, attr),
(reason, element, attr) => {
revertElementFxLive(element, attr);
return refused(reason);
},
),
),
},
+1
View File
@@ -162,6 +162,7 @@ export type { StudioPendingEditsDrainResult } from "./utils/studioPendingEdits";
export type {
CanEditTimelineElement,
TimelineEditPermission,
TimelineEditOutcome,
} from "./hooks/timelineEditPermission";
export { useEditHistoryActions } from "./hooks/useEditHistoryActions";
export type {
@@ -1,5 +1,6 @@
// fallow-ignore-file code-duplication
// fallow-ignore-file dead-code
import type { TimelineEditOutcome } from "../../hooks/timelineEditPermission";
import type { TimelineElement } from "../store/playerStore";
import type { TimelineMoveOperation } from "../../hooks/timelineMoveAdapter";
import type { BlockedTimelineEditIntent } from "./timelineEditing";
@@ -89,7 +90,7 @@ export interface TimelineEditCallbacks {
attr: string,
value: string | null,
label: string,
) => Promise<void>;
) => Promise<TimelineEditOutcome | void>;
/** C1's ungrouped-track FX pointer: "Group these clips" — write
* `data-audio-group` on every one of them, atomically. Same shape B6's
* carve auto-grouping uses. */
@@ -110,7 +111,7 @@ export interface TimelineEditCallbacks {
attr: string,
value: string | null,
label: string,
) => Promise<void>;
) => Promise<TimelineEditOutcome | void>;
onBlockedEditAttempt?: (element: TimelineElement, intent: BlockedTimelineEditIntent) => void;
onSplitElement?: (element: TimelineElement, splitTime: number) => Promise<void> | void;
onRazorSplit?: (element: TimelineElement, splitTime: number) => Promise<void> | void;