From 448a00f147276c65668dc6831e77f468c69b740e Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 19:00:36 -0700 Subject: [PATCH 1/5] fix(timeline): enforce locked track mutation invariants --- .../hooks/use-timeline-tracks.test.tsx | 40 ++ .../timeline/hooks/use-timeline-tracks.ts | 4 +- .../timeline/hooks/use-timeline-trim.ts | 22 +- src/features/timeline/hooks/use-track-drag.ts | 1 + .../timeline/hooks/use-track-push.test.tsx | 82 ++++ src/features/timeline/hooks/use-track-push.ts | 20 +- .../actions/edit/range-removal-actions.ts | 4 + .../stores/actions/edit/trim-actions.ts | 2 + .../item-actions.lock-invariants.test.ts | 268 ++++++++++++ .../actions/item-actions.track-push.test.ts | 58 ++- .../timeline/stores/actions/item-actions.ts | 359 +++++++++++----- .../stores/actions/sync-lock-ripple.test.ts | 125 ++++++ .../stores/actions/sync-lock-ripple.ts | 389 ++++++++++++------ .../timeline/utils/track-content-drag.test.ts | 74 ++++ .../timeline/utils/track-content-drag.ts | 45 +- .../timeline/utils/track-lock-invariants.ts | 74 ++++ 16 files changed, 1315 insertions(+), 252 deletions(-) create mode 100644 src/features/timeline/hooks/use-timeline-tracks.test.tsx create mode 100644 src/features/timeline/hooks/use-track-push.test.tsx create mode 100644 src/features/timeline/stores/actions/item-actions.lock-invariants.test.ts create mode 100644 src/features/timeline/utils/track-lock-invariants.ts diff --git a/src/features/timeline/hooks/use-timeline-tracks.test.tsx b/src/features/timeline/hooks/use-timeline-tracks.test.tsx new file mode 100644 index 000000000..07f271c88 --- /dev/null +++ b/src/features/timeline/hooks/use-timeline-tracks.test.tsx @@ -0,0 +1,40 @@ +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it } from 'vite-plus/test' +import { useItemsStore } from '../stores/items-store' +import { useTimelineCommandStore } from '../stores/timeline-command-store' +import { useTimelineSettingsStore } from '../stores/timeline-settings-store' +import { makeTimelineTrack } from '../test-helpers' +import { useTimelineTracks } from './use-timeline-tracks' + +describe('useTimelineTracks solo contract', () => { + beforeEach(() => { + useItemsStore.getState().setItems([]) + useItemsStore + .getState() + .setTracks([ + makeTimelineTrack({ id: 'v1', name: 'V1', kind: 'video', order: 0 }), + makeTimelineTrack({ id: 'a1', name: 'A1', kind: 'audio', order: 1 }), + ]) + useTimelineCommandStore.getState().clearHistory() + useTimelineSettingsStore.setState({ isDirty: false }) + }) + + it('keeps multiple stems soloed and toggles each track independently', () => { + const { result } = renderHook(() => useTimelineTracks()) + + act(() => result.current.toggleTrackSolo('v1')) + act(() => result.current.toggleTrackSolo('a1')) + + expect(useItemsStore.getState().tracks.map(({ id, solo }) => ({ id, solo }))).toEqual([ + { id: 'v1', solo: true }, + { id: 'a1', solo: true }, + ]) + + act(() => result.current.toggleTrackSolo('v1')) + + expect(useItemsStore.getState().tracks.map(({ id, solo }) => ({ id, solo }))).toEqual([ + { id: 'v1', solo: false }, + { id: 'a1', solo: true }, + ]) + }) +}) diff --git a/src/features/timeline/hooks/use-timeline-tracks.ts b/src/features/timeline/hooks/use-timeline-tracks.ts index 93d8bf60c..ad8d3edaa 100644 --- a/src/features/timeline/hooks/use-timeline-tracks.ts +++ b/src/features/timeline/hooks/use-timeline-tracks.ts @@ -232,8 +232,8 @@ export function useTimelineTracks() { ) /** - * Toggle track solo state - * Only one track can be soloed at a time - soloing a track will unsolo all others + * Toggle one track's solo state without changing any other soloed tracks. + * Multi-track solo is additive so editors can audition several stems together. * Reads latest state to avoid stale closure bugs */ const toggleTrackSolo = useCallback( diff --git a/src/features/timeline/hooks/use-timeline-trim.ts b/src/features/timeline/hooks/use-timeline-trim.ts index 75282d9f0..ca88a9d34 100644 --- a/src/features/timeline/hooks/use-timeline-trim.ts +++ b/src/features/timeline/hooks/use-timeline-trim.ts @@ -280,12 +280,7 @@ export function useTimelineTrim( let constraintLabel: string | null = null const trimConstraintItems = isRollingEdit || isRippleEdit ? [currentItem] : normalTrimItems for (const trimConstraintItem of trimConstraintItems) { - const { clampedAmount } = clampTrimAmount( - trimConstraintItem, - handle!, - deltaFrames, - fps, - ) + const { clampedAmount } = clampTrimAmount(trimConstraintItem, handle!, deltaFrames, fps) if (clampedAmount !== deltaFrames) { isConstrained = true constraintLabel = 'no handle' @@ -596,6 +591,10 @@ export function useTimelineTrim( items: allItems, tracks: useItemsStore.getState().tracks, editedTrackIds, + additionalAffectedIds: new Set([ + ...synchronizedItems.map((linkedItem) => linkedItem.id), + ...linkedPreviewUpdates.map((update) => update.id), + ]), intervals: [ { start: currentItem.from + currentItem.durationInFrames + rippleShift, @@ -607,6 +606,10 @@ export function useTimelineTrim( items: allItems, tracks: useItemsStore.getState().tracks, editedTrackIds, + additionalAffectedIds: new Set([ + ...synchronizedItems.map((linkedItem) => linkedItem.id), + ...linkedPreviewUpdates.map((update) => update.id), + ]), cutFrame: currentItem.from + currentItem.durationInFrames, amount: rippleShift, }) @@ -874,10 +877,9 @@ export function useTimelineTrim( handle === 'start' ? trimmedItem.from : trimmedItem.from + trimmedItem.durationInFrames return areTrimEdgesAligned(anchorTrimEdge, trimmedItemEdge) }) - const trimmedItemIds = - verticallyAlignedTrimItemIds.includes(currentItem.id) - ? verticallyAlignedTrimItemIds - : [currentItem.id] + const trimmedItemIds = verticallyAlignedTrimItemIds.includes(currentItem.id) + ? verticallyAlignedTrimItemIds + : [currentItem.id] magneticSnapTargetsRef.current = getMagneticSnapTargets() setDragState({ diff --git a/src/features/timeline/hooks/use-track-drag.ts b/src/features/timeline/hooks/use-track-drag.ts index a979f9d09..21449793a 100644 --- a/src/features/timeline/hooks/use-track-drag.ts +++ b/src/features/timeline/hooks/use-track-drag.ts @@ -375,6 +375,7 @@ export function useTrackDrag(track: TimelineTrack): UseTrackDragReturn { } } else { const updates = buildTrackContentMoveUpdates({ + tracks: allTracks, sectionTrackIds: dragState.sectionTrackIds, draggedTrackIds: draggedIds, items: itemsRef.current, diff --git a/src/features/timeline/hooks/use-track-push.test.tsx b/src/features/timeline/hooks/use-track-push.test.tsx new file mode 100644 index 000000000..7754a9177 --- /dev/null +++ b/src/features/timeline/hooks/use-track-push.test.tsx @@ -0,0 +1,82 @@ +import type { MouseEvent as ReactMouseEvent } from 'react' +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vite-plus/test' +import { useSelectionStore } from '@/shared/state/selection' +import { useItemsStore } from '../stores/items-store' +import { useTimelineSettingsStore } from '../stores/timeline-settings-store' +import { useTrackPushPreviewStore } from '../stores/track-push-preview-store' +import { makeTimelineAudioItem, makeTimelineTrack, makeTimelineVideoItem } from '../test-helpers' +import { useTrackPush } from './use-track-push' + +function makeMouseEvent(): ReactMouseEvent { + return { + button: 0, + clientX: 100, + preventDefault: vi.fn(), + stopPropagation: vi.fn(), + } as unknown as ReactMouseEvent +} + +describe('useTrackPush lock preview', () => { + beforeEach(() => { + useItemsStore.getState().setItems([]) + useItemsStore.getState().setTracks([]) + useTimelineSettingsStore.setState({ fps: 30, snapEnabled: false }) + useTrackPushPreviewStore.getState().clearPreview() + useSelectionStore.getState().setDragState(null) + useSelectionStore.getState().setActiveSnapTarget(null) + }) + + it('previews eligible unlocked items without moving standalone locked-track items', () => { + const video = makeTimelineVideoItem({ id: 'video', from: 30 }) + const lockedAudio = makeTimelineAudioItem({ id: 'audio', from: 30 }) + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'track-v1', name: 'V1', kind: 'video', order: 0 }), + makeTimelineTrack({ + id: 'track-a1', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }), + ]) + useItemsStore.getState().setItems([video, lockedAudio]) + const { result } = renderHook(() => useTrackPush(video, 10)) + + act(() => result.current.handleTrackPushStart(makeMouseEvent())) + + expect(result.current.isTrackPushActive).toBe(true) + expect([...useTrackPushPreviewStore.getState().shiftedItemIds]).toEqual([video.id]) + }) + + it('does not start or create a preview when the anchor has a locked linked companion', () => { + const video = makeTimelineVideoItem({ + id: 'video', + from: 30, + linkedGroupId: 'linked-av', + }) + const audio = makeTimelineAudioItem({ + id: 'audio', + from: 30, + linkedGroupId: 'linked-av', + }) + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'track-v1', name: 'V1', kind: 'video', order: 0 }), + makeTimelineTrack({ + id: 'track-a1', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }), + ]) + useItemsStore.getState().setItems([video, audio]) + const { result } = renderHook(() => useTrackPush(video, 10)) + + act(() => result.current.handleTrackPushStart(makeMouseEvent())) + + expect(result.current.isTrackPushActive).toBe(false) + expect(useTrackPushPreviewStore.getState().anchorItemId).toBeNull() + expect(useSelectionStore.getState().dragState).toBeNull() + }) +}) diff --git a/src/features/timeline/hooks/use-track-push.ts b/src/features/timeline/hooks/use-track-push.ts index 6219733c9..ce1456b52 100644 --- a/src/features/timeline/hooks/use-track-push.ts +++ b/src/features/timeline/hooks/use-track-push.ts @@ -10,6 +10,7 @@ import { useSnapCalculator } from './use-snap-calculator' import { trackPushItems } from '../stores/actions/item-actions' import type { SnapTarget } from '../types/drag' import { setActiveSnapTargetIfChanged } from '../utils/snap-target-state' +import { partitionItemMutationIdsByLock } from '../utils/track-lock-invariants' interface TrackPushState { isActive: boolean @@ -145,16 +146,19 @@ export function useTrackPush( e.preventDefault() commitPreviewFrameToCurrentFrame() - const { items: allItems, itemsByTrackId } = useItemsStore.getState() + const { items: allItems, itemsByTrackId, tracks } = useItemsStore.getState() const cutFrame = item.from - // Collect ALL items at or after the anchor's position, across every track - const shiftedIds = new Set() - for (const ti of allItems) { - if (ti.from >= cutFrame) { - shiftedIds.add(ti.id) - } - } + // Locked tracks stay fixed. If one proposed item belongs to a linked + // cohort with a locked companion, reject the gesture instead of + // previewing an A/V desync that the commit cannot accept. + const mutationPartition = partitionItemMutationIdsByLock({ + items: allItems, + tracks, + itemIds: allItems.filter((candidate) => candidate.from >= cutFrame).map(({ id }) => id), + }) + const shiftedIds = new Set(mutationPartition.allowedIds) + if (mutationPartition.blockedByLockedLinkedCohort || !shiftedIds.has(item.id)) return // Compute the tightest gap across all tracks. // Per track, find the first shifted item and the last non-shifted item diff --git a/src/features/timeline/stores/actions/edit/range-removal-actions.ts b/src/features/timeline/stores/actions/edit/range-removal-actions.ts index 30d0b54b3..6bd40556c 100644 --- a/src/features/timeline/stores/actions/edit/range-removal-actions.ts +++ b/src/features/timeline/stores/actions/edit/range-removal-actions.ts @@ -152,6 +152,10 @@ function applyRippleRemoval(ids: string[]): { removedIds: string[]; affectedIds: const syncLockResult = propagateRemovedIntervalsToSyncLockedTracks({ editedTrackIds, intervals: removedIntervals, + additionalAffectedIds: new Set([ + ...allRemoveIds, + ...filteredUpdates.map((update) => update.id), + ]), }) const cascadedRemoveIds = Array.from(new Set([...allRemoveIds, ...syncLockResult.removedIds])) diff --git a/src/features/timeline/stores/actions/edit/trim-actions.ts b/src/features/timeline/stores/actions/edit/trim-actions.ts index e2f4bd421..81391ec0b 100644 --- a/src/features/timeline/stores/actions/edit/trim-actions.ts +++ b/src/features/timeline/stores/actions/edit/trim-actions.ts @@ -415,6 +415,7 @@ export function rippleTrimItem(id: string, handle: 'start' | 'end', trimDelta: n const result = propagateRemovedIntervalsToSyncLockedTracks({ editedTrackIds: editedTracks, intervals: [interval], + additionalAffectedIds: new Set([...syncedIds, ...updates.map((update) => update.id)]), }) lockedAffected = result.affectedIds lockedRemoved = result.removedIds @@ -423,6 +424,7 @@ export function rippleTrimItem(id: string, handle: 'start' | 'end', trimDelta: n editedTrackIds: editedTracks, cutFrame: insertAt, amount: shift, + additionalAffectedIds: new Set([...syncedIds, ...updates.map((update) => update.id)]), }) lockedAffected = result.affectedIds } diff --git a/src/features/timeline/stores/actions/item-actions.lock-invariants.test.ts b/src/features/timeline/stores/actions/item-actions.lock-invariants.test.ts new file mode 100644 index 000000000..482115da9 --- /dev/null +++ b/src/features/timeline/stores/actions/item-actions.lock-invariants.test.ts @@ -0,0 +1,268 @@ +// @vitest-environment node + +import { beforeEach, describe, expect, it } from 'vite-plus/test' +import type { AudioItem, TimelineTrack, VideoItem } from '@/types/timeline' +import { useEditorStore } from '@/shared/state/editor' +import { useItemsStore } from '../items-store' +import { useKeyframesStore } from '../keyframes-store' +import { useTimelineCommandStore } from '../timeline-command-store' +import { useTimelineSettingsStore } from '../timeline-settings-store' +import { useTransitionsStore } from '../transitions-store' +import { + closeAllGapsOnTrack, + closeGapAtPosition, + moveItem, + moveItems, + removeItems, + rippleDeleteItems, + unlinkItems, + updateItem, +} from './item-actions' + +function makeTrack( + overrides: Partial & Pick, +): TimelineTrack { + return { + height: 80, + locked: false, + syncLock: true, + visible: true, + muted: false, + solo: false, + volume: 0, + items: [], + ...overrides, + } +} + +function makeVideoItem(overrides: Partial = {}): VideoItem { + return { + id: 'video-1', + type: 'video', + trackId: 'video-track', + from: 0, + durationInFrames: 60, + label: 'clip.mp4', + src: 'blob:video', + mediaId: 'media-1', + sourceStart: 10, + sourceEnd: 70, + sourceDuration: 120, + sourceFps: 30, + ...overrides, + } +} + +function makeAudioItem(overrides: Partial = {}): AudioItem { + return { + id: 'audio-1', + type: 'audio', + trackId: 'audio-track', + from: 0, + durationInFrames: 60, + label: 'clip.wav', + src: 'blob:audio', + mediaId: 'media-1', + sourceStart: 10, + sourceEnd: 70, + sourceDuration: 120, + sourceFps: 30, + ...overrides, + } +} + +function expectNoHistory(): void { + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(0) + expect(useTimelineSettingsStore.getState().isDirty).toBe(false) +} + +describe('track lock mutation invariants', () => { + beforeEach(() => { + useEditorStore.setState({ linkedSelectionEnabled: true }) + useItemsStore.getState().setItems([]) + useItemsStore.getState().setTracks([]) + useTransitionsStore.getState().setTransitions([]) + useKeyframesStore.getState().setKeyframes([]) + useTimelineCommandStore.getState().clearHistory() + useTimelineSettingsStore.setState({ fps: 30, isDirty: false }) + }) + + it('rejects direct timing, track, source-placement, and delete mutations on a locked item', () => { + const lockedTrack = makeTrack({ + id: 'video-track', + name: 'V1', + kind: 'video', + order: 0, + locked: true, + }) + const otherTrack = makeTrack({ + id: 'video-track-2', + name: 'V2', + kind: 'video', + order: 1, + }) + const original = makeVideoItem() + useItemsStore.getState().setTracks([lockedTrack, otherTrack]) + useItemsStore.getState().setItems([original]) + + updateItem(original.id, { + from: 20, + durationInFrames: 30, + trackId: otherTrack.id, + sourceStart: 40, + sourceEnd: 70, + }) + moveItem(original.id, 30, otherTrack.id) + removeItems([original.id]) + + expect(useItemsStore.getState().itemById[original.id]).toEqual(original) + expectNoHistory() + }) + + it('rejects plain and ripple delete atomically when a linked companion is locked', () => { + const videoTrack = makeTrack({ + id: 'video-track', + name: 'V1', + kind: 'video', + order: 0, + }) + const audioTrack = makeTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }) + const video = makeVideoItem({ linkedGroupId: 'linked-av' }) + const audio = makeAudioItem({ linkedGroupId: 'linked-av' }) + useItemsStore.getState().setTracks([videoTrack, audioTrack]) + useItemsStore.getState().setItems([video, audio]) + + removeItems([video.id]) + rippleDeleteItems([video.id]) + + expect(useItemsStore.getState().items).toEqual([video, audio]) + expectNoHistory() + }) + + it('requires explicit unlink before deleting away from a locked companion', () => { + useEditorStore.setState({ linkedSelectionEnabled: false }) + useItemsStore.getState().setTracks([ + makeTrack({ id: 'video-track', name: 'V1', kind: 'video', order: 0 }), + makeTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }), + ]) + const video = makeVideoItem({ linkedGroupId: 'linked-av' }) + const audio = makeAudioItem({ linkedGroupId: 'linked-av' }) + useItemsStore.getState().setItems([video, audio]) + + removeItems([video.id]) + expect(useItemsStore.getState().items).toHaveLength(2) + expectNoHistory() + + unlinkItems([video.id]) + removeItems([video.id]) + + expect(useItemsStore.getState().itemById[video.id]).toBeUndefined() + expect(useItemsStore.getState().itemById[audio.id]).toBeDefined() + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(2) + }) + + it('allows ripple delete on unlocked tracks while a locked sync-lock track stays byte-for-byte fixed', () => { + const videoTrack = makeTrack({ + id: 'video-track', + name: 'V1', + kind: 'video', + order: 0, + }) + const lockedAudioTrack = makeTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + syncLock: true, + }) + const deleted = makeVideoItem({ id: 'delete', durationInFrames: 30 }) + const downstream = makeVideoItem({ + id: 'downstream', + from: 50, + durationInFrames: 20, + mediaId: 'media-2', + }) + const lockedBed = makeAudioItem({ + id: 'locked-bed', + from: 0, + durationInFrames: 100, + sourceStart: 20, + sourceEnd: 120, + sourceDuration: 180, + }) + useItemsStore.getState().setTracks([videoTrack, lockedAudioTrack]) + useItemsStore.getState().setItems([deleted, downstream, lockedBed]) + + rippleDeleteItems([deleted.id]) + + expect(useItemsStore.getState().itemById[deleted.id]).toBeUndefined() + expect(useItemsStore.getState().itemById[downstream.id]).toMatchObject({ from: 20 }) + expect(useItemsStore.getState().itemById[lockedBed.id]).toEqual(lockedBed) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + }) + + it('rejects close-gap commands on a locked track without history', () => { + const lockedTrack = makeTrack({ + id: 'video-track', + name: 'V1', + kind: 'video', + order: 0, + locked: true, + }) + const first = makeVideoItem({ id: 'first', durationInFrames: 30 }) + const second = makeVideoItem({ id: 'second', from: 60, durationInFrames: 30 }) + useItemsStore.getState().setTracks([lockedTrack]) + useItemsStore.getState().setItems([first, second]) + + closeGapAtPosition(lockedTrack.id, 45) + closeAllGapsOnTrack(lockedTrack.id) + + expect(useItemsStore.getState().items).toEqual([first, second]) + expectNoHistory() + }) + + it('rejects close-gap and bulk-move plans that would peel away from a locked linked companion', () => { + const videoTrack = makeTrack({ + id: 'video-track', + name: 'V1', + kind: 'video', + order: 0, + }) + const audioTrack = makeTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }) + const anchor = makeVideoItem({ id: 'anchor', durationInFrames: 30 }) + const video = makeVideoItem({ id: 'linked-video', from: 60, linkedGroupId: 'linked-av' }) + const audio = makeAudioItem({ id: 'linked-audio', from: 60, linkedGroupId: 'linked-av' }) + useItemsStore.getState().setTracks([videoTrack, audioTrack]) + useItemsStore.getState().setItems([anchor, video, audio]) + + closeGapAtPosition(videoTrack.id, 45) + closeAllGapsOnTrack(videoTrack.id) + moveItems([ + { id: video.id, from: 10 }, + { id: audio.id, from: 10 }, + ]) + + expect(useItemsStore.getState().itemById[video.id]).toEqual(video) + expect(useItemsStore.getState().itemById[audio.id]).toEqual(audio) + expectNoHistory() + }) +}) diff --git a/src/features/timeline/stores/actions/item-actions.track-push.test.ts b/src/features/timeline/stores/actions/item-actions.track-push.test.ts index 150feec07..f57764bfe 100644 --- a/src/features/timeline/stores/actions/item-actions.track-push.test.ts +++ b/src/features/timeline/stores/actions/item-actions.track-push.test.ts @@ -1,7 +1,7 @@ // @vitest-environment node import { beforeEach, describe, expect, it } from 'vite-plus/test' -import type { AudioItem, VideoItem } from '@/types/timeline' +import type { AudioItem, TimelineTrack, VideoItem } from '@/types/timeline' import { useItemsStore } from '../items-store' import { useTimelineCommandStore } from '../timeline-command-store' import { useTimelineSettingsStore } from '../timeline-settings-store' @@ -36,6 +36,22 @@ function makeAudioItem(overrides: Partial = {}): AudioItem { } } +function makeTrack( + overrides: Partial & Pick, +): TimelineTrack { + return { + height: 80, + locked: false, + syncLock: true, + visible: true, + muted: false, + solo: false, + volume: 0, + items: [], + ...overrides, + } +} + describe('trackPushItems', () => { beforeEach(() => { useTimelineCommandStore.getState().clearHistory() @@ -112,4 +128,44 @@ describe('trackPushItems', () => { expect(reverted.find((i) => i.id === 'v2')).toMatchObject({ from: 100 }) expect(reverted.find((i) => i.id === 'a1')).toMatchObject({ from: 50 }) }) + + it('pushes eligible unlocked tracks while leaving standalone locked-track items fixed', () => { + useItemsStore + .getState() + .setTracks([ + makeTrack({ id: 'video-track', name: 'V1', order: 0, kind: 'video' }), + makeTrack({ id: 'audio-track', name: 'A1', order: 1, kind: 'audio', locked: true }), + ]) + const video = makeVideoItem({ id: 'v1', from: 50, durationInFrames: 30 }) + const lockedAudio = makeAudioItem({ id: 'a1', from: 50, durationInFrames: 30 }) + useItemsStore.getState().setItems([video, lockedAudio]) + + trackPushItems(video.id, 20) + + expect(useItemsStore.getState().itemById[video.id]).toMatchObject({ from: 70 }) + expect(useItemsStore.getState().itemById[lockedAudio.id]).toEqual(lockedAudio) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + + useTimelineCommandStore.getState().undo() + expect(useItemsStore.getState().itemById[video.id]).toMatchObject({ from: 50 }) + expect(useItemsStore.getState().itemById[lockedAudio.id]).toEqual(lockedAudio) + }) + + it('rejects a push atomically when the anchor has a locked linked companion', () => { + useItemsStore + .getState() + .setTracks([ + makeTrack({ id: 'video-track', name: 'V1', order: 0, kind: 'video' }), + makeTrack({ id: 'audio-track', name: 'A1', order: 1, kind: 'audio', locked: true }), + ]) + const video = makeVideoItem({ id: 'v1', from: 50, linkedGroupId: 'linked-av' }) + const audio = makeAudioItem({ id: 'a1', from: 50, linkedGroupId: 'linked-av' }) + useItemsStore.getState().setItems([video, audio]) + + trackPushItems(video.id, 20) + + expect(useItemsStore.getState().items).toEqual([video, audio]) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(0) + expect(useTimelineSettingsStore.getState().isDirty).toBe(false) + }) }) diff --git a/src/features/timeline/stores/actions/item-actions.ts b/src/features/timeline/stores/actions/item-actions.ts index 0f2e15210..1c1a28729 100644 --- a/src/features/timeline/stores/actions/item-actions.ts +++ b/src/features/timeline/stores/actions/item-actions.ts @@ -52,11 +52,63 @@ import { wouldCreateTransformParentCycle, } from '@/shared/utils/transform-parenting' import { createDefaultControllerItem } from '../../utils/generated-layer-items' +import { + isTimelineTrackLocked, + partitionItemMutationIdsByLock, +} from '../../utils/track-lock-invariants' + +const LOCK_PROTECTED_ITEM_FIELDS = new Set([ + 'from', + 'durationInFrames', + 'trackId', + 'trimStart', + 'trimEnd', + 'sourceStart', + 'sourceEnd', + 'sourceDuration', + 'sourceFps', + 'speed', + 'offset', + 'isReversed', + 'reverseConformLocalStart', +]) function isLinkedSelectionEnabled(): boolean { return useEditorStore.getState().linkedSelectionEnabled } +function changesLockedItemPlacement(item: TimelineItem, updates: Partial): boolean { + const updateRecord = updates as Record + const itemRecord = item as unknown as Record + return Object.keys(updateRecord).some( + (key) => LOCK_PROTECTED_ITEM_FIELDS.has(key) && updateRecord[key] !== itemRecord[key], + ) +} + +function areItemMutationsUnlocked(itemIds: Iterable): boolean { + const { items, tracks } = useItemsStore.getState() + return partitionItemMutationIdsByLock({ items, tracks, itemIds }).blockedIds.length === 0 +} + +function areMoveUpdatesUnlocked( + updates: Array<{ id: string; from: number; trackId?: string }>, + destinationTracks: TimelineTrack[] = useItemsStore.getState().tracks, +): boolean { + if (updates.length === 0) return false + + const { items, tracks } = useItemsStore.getState() + const partition = partitionItemMutationIdsByLock({ + items, + tracks, + itemIds: updates.map((update) => update.id), + }) + if (partition.blockedIds.length > 0) return false + + return updates.every( + (update) => !update.trackId || !isTimelineTrackLocked(destinationTracks, update.trackId), + ) +} + function pruneLayerGroupsAfterItemRemoval(): void { const store = useItemsStore.getState() const nextTracks = pruneEmptyLayerGroupHierarchy(store.tracks, store.items) @@ -106,18 +158,18 @@ function isInvalidTransformParentUpdate( if (!child || !canParticipateInTransformHierarchy(child)) return true return Boolean( context.parentItemId && - (wouldCreateTransformParentCycle( - childItemId, - context.parentItemId, - context.getItem, - context.getKeyframes, - ) || - hasRedundantTransformParentLink( - childItemId, - context.parentItemId, - context.getItem, - context.getKeyframes, - )), + (wouldCreateTransformParentCycle( + childItemId, + context.parentItemId, + context.getItem, + context.getKeyframes, + ) || + hasRedundantTransformParentLink( + childItemId, + context.parentItemId, + context.getItem, + context.getKeyframes, + )), ) } @@ -715,6 +767,17 @@ export function addItemsOnNewTracks(items: TimelineItem[], tracks: TimelineTrack } export function updateItem(id: string, updates: Partial): void { + const item = useItemsStore.getState().itemById[id] + if (!item) return + if (changesLockedItemPlacement(item, updates) && !areItemMutationsUnlocked([id])) return + if ( + updates.trackId && + updates.trackId !== item.trackId && + isTimelineTrackLocked(useItemsStore.getState().tracks, updates.trackId) + ) { + return + } + execute( 'UPDATE_ITEM', () => { @@ -829,6 +892,7 @@ export function reverseItems(ids: string[]): void { ) if (reversibleItems.length === 0) return + if (!areItemMutationsUnlocked(reversibleItems.map((item) => item.id))) return const shouldReverse = !reversibleItems.every((item) => item.isReversed === true) if (shouldReverse) { const videoItems = reversibleItems.filter((item) => item.type === 'video') @@ -875,6 +939,7 @@ export function commitPreparedReverseItems( results: ReverseConformResult[], ): void { if (items.length === 0) return + if (!areItemMutationsUnlocked(items.map((item) => item.id))) return const resultByItemId = new Map(results.map((result) => [result.itemId, result])) execute( @@ -913,159 +978,230 @@ export function commitPreparedReverseItems( } export function removeItems(ids: string[]): void { - const expandedIds = expandIdsWithLinkedItems( - useItemsStore.getState().items, - ids, - isLinkedSelectionEnabled(), - ) - if (expandedIds.length === 0) return + const { items, tracks } = useItemsStore.getState() + const expandedIds = expandIdsWithLinkedItems(items, ids, isLinkedSelectionEnabled()) + const { allowedIds } = partitionItemMutationIdsByLock({ + items, + tracks, + itemIds: expandedIds, + }) + if (allowedIds.length === 0) return execute( 'REMOVE_ITEMS', () => { // Remove items - useItemsStore.getState()._removeItems(expandedIds) + useItemsStore.getState()._removeItems(allowedIds) // Cascade: Remove transitions referencing deleted items - useTransitionsStore.getState()._removeTransitionsForItems(expandedIds) + useTransitionsStore.getState()._removeTransitionsForItems(allowedIds) // Cascade: Remove keyframes for deleted items - useKeyframesStore.getState()._removeKeyframesForItems(expandedIds) + useKeyframesStore.getState()._removeKeyframesForItems(allowedIds) pruneLayerGroupsAfterItemRemoval() useTimelineSettingsStore.getState().markDirty() }, - { ids: expandedIds }, + { ids: allowedIds }, ) emitUiSound('delete') } -export function rippleDeleteItems(ids: string[]): void { - const items = useItemsStore.getState().items - const linkedSelectionEnabled = isLinkedSelectionEnabled() - const expandedIds = expandIdsWithLinkedItems(items, ids, linkedSelectionEnabled) - if (expandedIds.length === 0) return +type RippleMoveUpdate = { id: string; from: number } + +interface RippleDeletePlan { + allRemoveIds: string[] + editedTrackIds: Set + filteredUpdates: RippleMoveUpdate[] + removedIntervals: Array<{ start: number; end: number }> + updates: RippleMoveUpdate[] +} - const idsToDelete = new Set(expandedIds) - const remainingItems = items.filter((item) => !idsToDelete.has(item.id)) +function buildBaseRippleShifts( + remainingItems: TimelineItem[], + deletedItems: TimelineItem[], +): Map { const baseShiftByItemId = new Map() - const editedTrackIds = new Set( - items.filter((item) => idsToDelete.has(item.id)).map((item) => item.trackId), - ) - const removedIntervals = items - .filter((item) => idsToDelete.has(item.id)) - .map((item) => ({ - start: item.from, - end: item.from + item.durationInFrames, - })) - - // Per-track: shift downstream items on the same track as each deleted item. - // Linked counterparts and attached captions on tracks that won't be handled - // by sync-lock ripple get shifted manually. Solo clips on unrelated tracks - // are left in place. - for (const item of remainingItems) { - const shiftAmount = items - .filter((candidate) => idsToDelete.has(candidate.id)) - .filter( - (deletedItem) => - deletedItem.trackId === item.trackId && - deletedItem.from + deletedItem.durationInFrames <= item.from, - ) - .reduce((sum, deletedItem) => sum + deletedItem.durationInFrames, 0) - if (shiftAmount > 0) { - baseShiftByItemId.set(item.id, shiftAmount) + for (const item of remainingItems) { + let shiftAmount = 0 + for (const deletedItem of deletedItems) { + if ( + deletedItem.trackId === item.trackId && + deletedItem.from + deletedItem.durationInFrames <= item.from + ) { + shiftAmount += deletedItem.durationInFrames + } } + if (shiftAmount > 0) baseShiftByItemId.set(item.id, shiftAmount) } - const trackById = new Map(useItemsStore.getState().tracks.map((track) => [track.id, track])) - const itemById = new Map(remainingItems.map((item) => [item.id, item])) - const shiftByItemId = new Map() + return baseShiftByItemId +} - for (const [itemId, shiftAmount] of baseShiftByItemId) { - if (shiftAmount <= 0) continue +function buildRippleMoveUpdates(params: { + remainingItems: TimelineItem[] + tracks: TimelineTrack[] + editedTrackIds: Set + baseShiftByItemId: ReadonlyMap + linkedSelectionEnabled: boolean +}): RippleMoveUpdate[] { + const trackById = new Map(params.tracks.map((track) => [track.id, track])) + const itemById = new Map(params.remainingItems.map((item) => [item.id, item])) + const shiftByItemId = new Map() - const relatedIds = expandIdsWithLinkedItems(remainingItems, [itemId], linkedSelectionEnabled) + for (const [itemId, shiftAmount] of params.baseShiftByItemId) { + const relatedIds = expandIdsWithLinkedItems( + params.remainingItems, + [itemId], + params.linkedSelectionEnabled, + ) for (const relatedId of relatedIds) { const relatedItem = itemById.get(relatedId) if (!relatedItem) continue const handledBySyncLock = - !editedTrackIds.has(relatedItem.trackId) && + !params.editedTrackIds.has(relatedItem.trackId) && isTrackSyncLockEnabled(trackById.get(relatedItem.trackId)) - if (handledBySyncLock) { - continue - } + if (handledBySyncLock) continue shiftByItemId.set(relatedId, Math.max(shiftByItemId.get(relatedId) ?? 0, shiftAmount)) } } - const updates = remainingItems.flatMap((item) => { + return params.remainingItems.flatMap((item) => { const shiftAmount = shiftByItemId.get(item.id) ?? 0 return shiftAmount > 0 ? [{ id: item.id, from: item.from - shiftAmount }] : [] }) +} - // Detect non-shifted items that would be overlapped by shifted items. - // These get deleted rather than creating overlaps. - const shiftedById = new Map(updates.map((u) => [u.id, u.from])) +function findItemsCoveredByRippleMove( + remainingItems: TimelineItem[], + updates: RippleMoveUpdate[], +): string[] { + const shiftedById = new Map(updates.map((update) => [update.id, update.from])) const coveredIds: string[] = [] + for (const item of remainingItems) { - if (shiftedById.has(item.id) || idsToDelete.has(item.id)) continue + if (shiftedById.has(item.id)) continue const itemEnd = item.from + item.durationInFrames - // Check if any shifted item on the same track would overlap this item - for (const other of remainingItems) { + + const isCovered = remainingItems.some((other) => { const newFrom = shiftedById.get(other.id) - if (newFrom === undefined || other.trackId !== item.trackId) continue - const newEnd = newFrom + other.durationInFrames - if (newFrom < itemEnd && newEnd > item.from) { - coveredIds.push(item.id) - break - } - } + if (newFrom === undefined || other.trackId !== item.trackId) return false + return newFrom < itemEnd && newFrom + other.durationInFrames > item.from + }) + if (isCovered) coveredIds.push(item.id) } - // Expand covered IDs with linked companions so we don't orphan them + return coveredIds +} + +function buildRippleDeletePlan(params: { + items: TimelineItem[] + tracks: TimelineTrack[] + deletionIds: string[] + linkedSelectionEnabled: boolean +}): RippleDeletePlan | null { + const idsToDelete = new Set(params.deletionIds) + const deletedItems = params.items.filter((item) => idsToDelete.has(item.id)) + const remainingItems = params.items.filter((item) => !idsToDelete.has(item.id)) + const editedTrackIds = new Set(deletedItems.map((item) => item.trackId)) + const removedIntervals = deletedItems.map((item) => ({ + start: item.from, + end: item.from + item.durationInFrames, + })) + const baseShiftByItemId = buildBaseRippleShifts(remainingItems, deletedItems) + const updates = buildRippleMoveUpdates({ + remainingItems, + tracks: params.tracks, + editedTrackIds, + baseShiftByItemId, + linkedSelectionEnabled: params.linkedSelectionEnabled, + }) + + const updatePartition = partitionItemMutationIdsByLock({ + items: remainingItems, + tracks: params.tracks, + itemIds: updates.map((update) => update.id), + }) + if (updatePartition.blockedIds.length > 0) return null + + const coveredIds = findItemsCoveredByRippleMove(remainingItems, updates) const expandedCoveredIds = expandIdsWithLinkedItems( remainingItems, coveredIds, - linkedSelectionEnabled, + params.linkedSelectionEnabled, ) - const allRemoveIds = [...expandedIds, ...expandedCoveredIds] + const coveredPartition = partitionItemMutationIdsByLock({ + items: remainingItems, + tracks: params.tracks, + itemIds: expandedCoveredIds, + }) + if (coveredPartition.blockedIds.length > 0) return null + + const coveredSet = new Set(coveredPartition.allowedIds) + return { + allRemoveIds: Array.from(new Set([...params.deletionIds, ...coveredSet])), + editedTrackIds, + filteredUpdates: updates.filter((update) => !coveredSet.has(update.id)), + removedIntervals, + updates, + } +} - // Filter out updates for items that were removed as covered (including their linked companions) - const coveredSet = new Set(expandedCoveredIds) - const filteredUpdates = - coveredSet.size > 0 ? updates.filter((u) => !coveredSet.has(u.id)) : updates +export function rippleDeleteItems(ids: string[]): void { + const { items, tracks } = useItemsStore.getState() + const linkedSelectionEnabled = isLinkedSelectionEnabled() + const expandedIds = expandIdsWithLinkedItems(items, ids, linkedSelectionEnabled) + const deletionPartition = partitionItemMutationIdsByLock({ + items, + tracks, + itemIds: expandedIds, + }) + if (deletionPartition.allowedIds.length === 0) return + const plan = buildRippleDeletePlan({ + items, + tracks, + deletionIds: deletionPartition.allowedIds, + linkedSelectionEnabled, + }) + if (!plan) return execute( 'RIPPLE_DELETE_ITEMS', () => { - useItemsStore.getState()._removeItems(allRemoveIds) - if (filteredUpdates.length > 0) { - useItemsStore.getState()._moveItems(filteredUpdates) + useItemsStore.getState()._removeItems(plan.allRemoveIds) + if (plan.filteredUpdates.length > 0) { + useItemsStore.getState()._moveItems(plan.filteredUpdates) } const syncLockResult = propagateRemovedIntervalsToSyncLockedTracks({ - editedTrackIds, - intervals: removedIntervals, + editedTrackIds: plan.editedTrackIds, + intervals: plan.removedIntervals, + additionalAffectedIds: new Set([ + ...plan.allRemoveIds, + ...plan.filteredUpdates.map((update) => update.id), + ]), }) // Cascade: Remove transitions and keyframes - const cascadedRemoveIds = Array.from(new Set([...allRemoveIds, ...syncLockResult.removedIds])) + const cascadedRemoveIds = Array.from( + new Set([...plan.allRemoveIds, ...syncLockResult.removedIds]), + ) useTransitionsStore.getState()._removeTransitionsForItems(cascadedRemoveIds) useKeyframesStore.getState()._removeKeyframesForItems(cascadedRemoveIds) // Repair transitions on moved clips (they may now overlap or gap differently) - if (filteredUpdates.length > 0) { - applyTransitionRepairs(filteredUpdates.map((u) => u.id)) + if (plan.filteredUpdates.length > 0) { + applyTransitionRepairs(plan.filteredUpdates.map((update) => update.id)) } // Repair transitions for surviving clips that were shifted const repairedClipIds = Array.from( - new Set([...updates.map((update) => update.id), ...syncLockResult.affectedIds]), + new Set([...plan.updates.map((update) => update.id), ...syncLockResult.affectedIds]), ) if (repairedClipIds.length > 0) { applyTransitionRepairs(repairedClipIds, new Set(cascadedRemoveIds)) @@ -1075,12 +1211,14 @@ export function rippleDeleteItems(ids: string[]): void { useTimelineSettingsStore.getState().markDirty() }, - { ids: allRemoveIds }, + { ids: plan.allRemoveIds }, ) } export function closeGapAtPosition(trackId: string, frame: number): void { - const items = useItemsStore.getState().items + const { items, tracks } = useItemsStore.getState() + if (isTimelineTrackLocked(tracks, trackId)) return + const targetFrame = Math.max(0, Math.round(frame)) const trackItems = items .filter((item) => item.trackId === trackId) @@ -1106,6 +1244,7 @@ export function closeGapAtPosition(trackId: string, frame: number): void { .filter((item) => item.trackId === trackId && item.from >= gapEnd) .map((item) => ({ id: item.id, from: item.from - gapSize })) if (updates.length === 0) return + if (!areMoveUpdatesUnlocked(updates)) return execute( 'CLOSE_GAP', @@ -1114,6 +1253,7 @@ export function closeGapAtPosition(trackId: string, frame: number): void { const syncLockResult = propagateRemovedIntervalsToSyncLockedTracks({ editedTrackIds: new Set([trackId]), intervals: [{ start: gapStart, end: gapEnd }], + additionalAffectedIds: new Set(updates.map((update) => update.id)), }) const removedIds = syncLockResult.removedIds @@ -1134,7 +1274,9 @@ export function closeGapAtPosition(trackId: string, frame: number): void { } export function closeAllGapsOnTrack(trackId: string): void { - const items = useItemsStore.getState().items + const { items, tracks } = useItemsStore.getState() + if (isTimelineTrackLocked(tracks, trackId)) return + const trackItems = items .filter((item) => item.trackId === trackId) .sort((left, right) => left.from - right.from) @@ -1154,6 +1296,7 @@ export function closeAllGapsOnTrack(trackId: string): void { const updates = buildLinkedLeftShiftUpdates(items, baseShiftByItemId, isLinkedSelectionEnabled()) if (updates.length === 0) return + if (!areMoveUpdatesUnlocked(updates)) return execute( 'CLOSE_ALL_GAPS', @@ -1175,16 +1318,26 @@ export function closeAllGapsOnTrack(trackId: string): void { export function trackPushItems(anchorId: string, delta: number): void { if (delta === 0) return - const items = useItemsStore.getState().items + const { items, tracks } = useItemsStore.getState() const anchor = items.find((i) => i.id === anchorId) if (!anchor) return + if (!areItemMutationsUnlocked([anchor.id])) return const cutFrame = anchor.from // Every item whose start is at or after the cut frame gets shifted + const candidateIds = items.filter((item) => item.from >= cutFrame).map((item) => item.id) + const mutationPartition = partitionItemMutationIdsByLock({ + items, + tracks, + itemIds: candidateIds, + }) + if (mutationPartition.blockedByLockedLinkedCohort) return + + const eligibleIds = new Set(mutationPartition.allowedIds) const updates: Array<{ id: string; from: number }> = [] for (const ti of items) { - if (ti.from >= cutFrame) { + if (eligibleIds.has(ti.id)) { updates.push({ id: ti.id, from: Math.max(0, ti.from + delta) }) } } @@ -1203,6 +1356,10 @@ export function trackPushItems(anchorId: string, delta: number): void { } export function moveItem(id: string, newFrom: number, newTrackId?: string): void { + const item = useItemsStore.getState().itemById[id] + if (!item) return + if (!areMoveUpdatesUnlocked([{ id, from: newFrom, trackId: newTrackId }])) return + execute( 'MOVE_ITEM', () => { @@ -1219,6 +1376,8 @@ export function moveItem(id: string, newFrom: number, newTrackId?: string): void } export function moveItems(updates: Array<{ id: string; from: number; trackId?: string }>): void { + if (!areMoveUpdatesUnlocked(updates)) return + execute( 'MOVE_ITEMS', () => { @@ -1260,6 +1419,8 @@ export function moveItemsWithTrackChanges( tracks: TimelineTrack[], updates: Array<{ id: string; from: number; trackId?: string }>, ): void { + if (!areMoveUpdatesUnlocked(updates, tracks)) return + execute( 'MOVE_ITEMS_WITH_TRACKS', () => { diff --git a/src/features/timeline/stores/actions/sync-lock-ripple.test.ts b/src/features/timeline/stores/actions/sync-lock-ripple.test.ts index 5bf70e31f..8d06086b9 100644 --- a/src/features/timeline/stores/actions/sync-lock-ripple.test.ts +++ b/src/features/timeline/stores/actions/sync-lock-ripple.test.ts @@ -253,4 +253,129 @@ describe('sync-lock ripple preview helpers', () => { linkedGroupId: undefined, }) }) + + it('lets track lock win over sync lock in previews and committed propagation', () => { + const tracks = [ + makeTrack({ id: 'edited-track', name: 'Edited', order: 0, kind: 'video' }), + makeTrack({ + id: 'audio-track', + name: 'A1', + order: 1, + kind: 'audio', + locked: true, + syncLock: true, + }), + ] + const lockedAudio = makeAudioItem({ + id: 'locked-audio', + trackId: 'audio-track', + from: 0, + durationInFrames: 100, + sourceStart: 20, + sourceEnd: 120, + sourceDuration: 180, + }) + + expect( + buildRemovedIntervalPreviewUpdatesForSyncLockedTracks({ + items: [lockedAudio], + tracks, + editedTrackIds: new Set(['edited-track']), + intervals: [{ start: 20, end: 40 }], + }), + ).toEqual([]) + + useItemsStore.getState().setTracks(tracks) + useItemsStore.getState().setItems([lockedAudio]) + const result = propagateRemovedIntervalsToSyncLockedTracks({ + editedTrackIds: new Set(['edited-track']), + intervals: [{ start: 20, end: 40 }], + }) + + expect(result).toEqual({ affectedIds: [], removedIds: [] }) + expect(useItemsStore.getState().itemById[lockedAudio.id]).toEqual(lockedAudio) + }) + + it('splits synchronized linked cohorts together and preserves per-side links', () => { + useItemsStore + .getState() + .setTracks([ + makeTrack({ id: 'edited-track', name: 'Edited', order: 0, kind: 'video' }), + makeTrack({ id: 'video-track', name: 'V1', order: 1, kind: 'video' }), + makeTrack({ id: 'audio-track', name: 'A1', order: 2, kind: 'audio' }), + ]) + useItemsStore.getState().setItems([ + makeVideoItem({ + id: 'linked-video', + trackId: 'video-track', + linkedGroupId: 'linked-av', + sourceStart: 0, + sourceEnd: 60, + sourceDuration: 120, + }), + makeAudioItem({ + id: 'linked-audio', + trackId: 'audio-track', + linkedGroupId: 'linked-av', + sourceStart: 0, + sourceEnd: 60, + sourceDuration: 120, + }), + ]) + + propagateInsertedGapToSyncLockedTracks({ + editedTrackIds: new Set(['edited-track']), + cutFrame: 20, + amount: 10, + }) + + const videos = useItemsStore + .getState() + .items.filter((item) => item.trackId === 'video-track') + .sort((left, right) => left.from - right.from) + const audios = useItemsStore + .getState() + .items.filter((item) => item.trackId === 'audio-track') + .sort((left, right) => left.from - right.from) + + expect(videos.map(({ from, durationInFrames }) => ({ from, durationInFrames }))).toEqual([ + { from: 0, durationInFrames: 20 }, + { from: 30, durationInFrames: 40 }, + ]) + expect(audios.map(({ from, durationInFrames }) => ({ from, durationInFrames }))).toEqual([ + { from: 0, durationInFrames: 20 }, + { from: 30, durationInFrames: 40 }, + ]) + expect(videos[0]?.linkedGroupId).toBe(audios[0]?.linkedGroupId) + expect(videos[1]?.linkedGroupId).toBe(audios[1]?.linkedGroupId) + expect(videos[0]?.linkedGroupId).not.toBe(videos[1]?.linkedGroupId) + }) + + it('rejects sync-lock mutation when a linked companion is locked', () => { + const video = makeVideoItem({ + id: 'linked-video', + trackId: 'video-track', + linkedGroupId: 'linked-av', + }) + const audio = makeAudioItem({ + id: 'linked-audio', + trackId: 'audio-track', + linkedGroupId: 'linked-av', + }) + useItemsStore + .getState() + .setTracks([ + makeTrack({ id: 'edited-track', name: 'Edited', order: 0, kind: 'video' }), + makeTrack({ id: 'video-track', name: 'V1', order: 1, kind: 'video' }), + makeTrack({ id: 'audio-track', name: 'A1', order: 2, kind: 'audio', locked: true }), + ]) + useItemsStore.getState().setItems([video, audio]) + + propagateRemovedIntervalsToSyncLockedTracks({ + editedTrackIds: new Set(['edited-track']), + intervals: [{ start: 20, end: 40 }], + }) + + expect(useItemsStore.getState().items).toEqual([video, audio]) + }) }) diff --git a/src/features/timeline/stores/actions/sync-lock-ripple.ts b/src/features/timeline/stores/actions/sync-lock-ripple.ts index 69be2e94d..8c347e9f2 100644 --- a/src/features/timeline/stores/actions/sync-lock-ripple.ts +++ b/src/features/timeline/stores/actions/sync-lock-ripple.ts @@ -2,7 +2,9 @@ import { useItemsStore } from '../items-store' import type { TimelineItem, TimelineTrack } from '@/types/timeline' import { isTrackSyncLockEnabled } from '../../utils/track-sync-lock' import type { PreviewItemUpdate } from '../../utils/item-edit-preview' -import { applySplitBookkeeping } from './split-bookkeeping' +import { applySplitBookkeeping, type SplitResultEntry } from './split-bookkeeping' +import { getLinkedItems } from '../../utils/linked-items' +import { isTimelineTrackLocked } from '../../utils/track-lock-invariants' export interface RipplePropagationResult { affectedIds: string[] @@ -52,30 +54,34 @@ function normalizeIntervals(intervals: TimeInterval[]): TimeInterval[] { return merged } +function canSyncLockRippleTrack( + tracks: TimelineTrack[], + track: TimelineTrack | undefined, + trackId: string, +): boolean { + return isTrackSyncLockEnabled(track) && !isTimelineTrackLocked(tracks, trackId) +} + function getCandidateTrackIdsFromState( items: TimelineItem[], tracks: TimelineTrack[], editedTrackIds: Set, ): string[] { - const trackIds = new Set() - - for (const track of tracks) { - if (!editedTrackIds.has(track.id) && isTrackSyncLockEnabled(track)) { - trackIds.add(track.id) - } - } - - for (const item of items) { - if (editedTrackIds.has(item.trackId)) continue - if (trackIds.has(item.trackId)) continue - - const track = tracks.find((candidate) => candidate.id === item.trackId) - if (isTrackSyncLockEnabled(track)) { - trackIds.add(item.trackId) - } - } + const trackById = new Map(tracks.map((track) => [track.id, track])) + const declaredCandidateIds = tracks + .filter( + (track) => !editedTrackIds.has(track.id) && canSyncLockRippleTrack(tracks, track, track.id), + ) + .map((track) => track.id) + const itemCandidateIds = items + .map((item) => item.trackId) + .filter( + (trackId) => + !editedTrackIds.has(trackId) && + canSyncLockRippleTrack(tracks, trackById.get(trackId), trackId), + ) - return [...trackIds] + return uniqueIds([...declaredCandidateIds, ...itemCandidateIds]) } function getCandidateTrackIds(editedTrackIds: Set): string[] { @@ -103,29 +109,41 @@ function setPreviewUpdate( }) } -function splitItemWithBookkeeping( - itemId: string, - splitFrame: number, -): { leftItem: TimelineItem; rightItem: TimelineItem } | null { - const current = useItemsStore.getState().itemById[itemId] - if (!current) { - return null +function applySplitBookkeepingByLinkedGroup(entries: SplitResultEntry[]): void { + const unlinkedEntries: SplitResultEntry[] = [] + const entriesByLinkedGroupId = new Map() + + for (const entry of entries) { + if (!entry.originalLinkedGroupId) { + unlinkedEntries.push(entry) + continue + } + + const groupEntries = entriesByLinkedGroupId.get(entry.originalLinkedGroupId) + if (groupEntries) groupEntries.push(entry) + else entriesByLinkedGroupId.set(entry.originalLinkedGroupId, [entry]) } - const result = useItemsStore.getState()._splitItem(itemId, splitFrame) - if (!result) { - return null + applySplitBookkeeping(unlinkedEntries) + for (const groupEntries of entriesByLinkedGroupId.values()) { + applySplitBookkeeping(groupEntries) } +} - applySplitBookkeeping([ - { - originalId: current.id, - originalLinkedGroupId: current.linkedGroupId, - result, - }, - ]) +function splitItemsWithBookkeeping(itemIds: string[], splitFrame: number): SplitResultEntry[] { + const store = useItemsStore.getState() + const entries = itemIds.flatMap((itemId) => { + const current = useItemsStore.getState().itemById[itemId] + if (!current) return [] + + const result = store._splitItem(itemId, splitFrame) + return result + ? [{ originalId: current.id, originalLinkedGroupId: current.linkedGroupId, result }] + : [] + }) - return result + applySplitBookkeepingByLinkedGroup(entries) + return entries } function buildRemovedIntervalPreviewUpdatesForTrack( @@ -242,11 +260,92 @@ function buildInsertedGapPreviewUpdatesForTrack( return [...updatesById.values()] } +function getAtomicCandidateTrackIds(params: { + items: TimelineItem[] + tracks: TimelineTrack[] + candidateTrackIds: string[] + updatesByTrackId: ReadonlyMap + additionalAffectedIds?: ReadonlySet +}): string[] { + const safeTrackIds = new Set(params.candidateTrackIds) + const itemById = new Map(params.items.map((item) => [item.id, item])) + + let changed = true + while (changed) { + changed = false + const affectedIds = new Set(params.additionalAffectedIds ?? []) + for (const trackId of safeTrackIds) { + for (const update of params.updatesByTrackId.get(trackId) ?? []) { + affectedIds.add(update.id) + } + } + + for (const trackId of [...safeTrackIds]) { + const trackUpdates = params.updatesByTrackId.get(trackId) ?? [] + const blocksTrack = trackUpdates.some((update) => { + const item = itemById.get(update.id) + if (!item) return true + + const linkedItems = getLinkedItems(params.items, item.id) + if (linkedItems.length <= 1) return false + + const hasLockedMember = linkedItems.some((linkedItem) => + isTimelineTrackLocked(params.tracks, linkedItem.trackId), + ) + const mutatesWholeCohort = linkedItems.every((linkedItem) => affectedIds.has(linkedItem.id)) + return hasLockedMember || !mutatesWholeCohort + }) + + if (blocksTrack) { + safeTrackIds.delete(trackId) + changed = true + } + } + } + + return params.candidateTrackIds.filter((trackId) => safeTrackIds.has(trackId)) +} + +function buildRemovedUpdatesByTrack(params: { + items: TimelineItem[] + candidateTrackIds: string[] + intervals: TimeInterval[] +}): Map { + return new Map( + params.candidateTrackIds.map((trackId) => [ + trackId, + buildRemovedIntervalPreviewUpdatesForTrack( + params.items.filter((item) => item.trackId === trackId), + params.intervals, + ), + ]), + ) +} + +function buildInsertedUpdatesByTrack(params: { + items: TimelineItem[] + candidateTrackIds: string[] + cutFrame: number + amount: number +}): Map { + return new Map( + params.candidateTrackIds.map((trackId) => [ + trackId, + buildInsertedGapPreviewUpdatesForTrack( + params.items.filter((item) => item.trackId === trackId), + params.cutFrame, + params.amount, + ), + ]), + ) +} + export function buildRemovedIntervalPreviewUpdatesForSyncLockedTracks(params: { items: TimelineItem[] tracks: TimelineTrack[] editedTrackIds: Set intervals: TimeInterval[] + additionalAffectedIds?: ReadonlySet }): PreviewItemUpdate[] { const intervals = normalizeIntervals(params.intervals) if (intervals.length === 0) { @@ -258,13 +357,20 @@ export function buildRemovedIntervalPreviewUpdatesForSyncLockedTracks(params: { params.tracks, params.editedTrackIds, ) + const updatesByTrackId = buildRemovedUpdatesByTrack({ + items: params.items, + candidateTrackIds, + intervals, + }) + const atomicTrackIds = getAtomicCandidateTrackIds({ + items: params.items, + tracks: params.tracks, + candidateTrackIds, + updatesByTrackId, + additionalAffectedIds: params.additionalAffectedIds, + }) - return candidateTrackIds.flatMap((trackId) => - buildRemovedIntervalPreviewUpdatesForTrack( - params.items.filter((item) => item.trackId === trackId), - intervals, - ), - ) + return atomicTrackIds.flatMap((trackId) => updatesByTrackId.get(trackId) ?? []) } export function buildInsertedGapPreviewUpdatesForSyncLockedTracks(params: { @@ -273,6 +379,7 @@ export function buildInsertedGapPreviewUpdatesForSyncLockedTracks(params: { editedTrackIds: Set cutFrame: number amount: number + additionalAffectedIds?: ReadonlySet }): PreviewItemUpdate[] { const cutFrame = Math.max(0, Math.round(params.cutFrame)) const amount = Math.max(0, Math.round(params.amount)) @@ -285,76 +392,73 @@ export function buildInsertedGapPreviewUpdatesForSyncLockedTracks(params: { params.tracks, params.editedTrackIds, ) + const updatesByTrackId = buildInsertedUpdatesByTrack({ + items: params.items, + candidateTrackIds, + cutFrame, + amount, + }) + const atomicTrackIds = getAtomicCandidateTrackIds({ + items: params.items, + tracks: params.tracks, + candidateTrackIds, + updatesByTrackId, + additionalAffectedIds: params.additionalAffectedIds, + }) - return candidateTrackIds.flatMap((trackId) => - buildInsertedGapPreviewUpdatesForTrack( - params.items.filter((item) => item.trackId === trackId), - cutFrame, - amount, - ), - ) + return atomicTrackIds.flatMap((trackId) => updatesByTrackId.get(trackId) ?? []) } -function removeItemsOnTrackInterval( - trackId: string, +function removeIntervalFromTracks( + trackIds: ReadonlySet, interval: TimeInterval, ): RipplePropagationResult { const store = useItemsStore.getState() const affectedIds: string[] = [] - const removedIds: string[] = [] const overlapping = useItemsStore .getState() .items.filter( (item) => - item.trackId === trackId && + trackIds.has(item.trackId) && item.from < interval.end && item.from + item.durationInFrames > interval.start, ) - .sort((left, right) => left.from - right.from) - - for (const overlappingItem of overlapping) { - const current = useItemsStore.getState().itemById[overlappingItem.id] - if (!current || current.trackId !== trackId) continue - - const itemEnd = current.from + current.durationInFrames - const startsBeforeInterval = current.from < interval.start - const endsAfterInterval = itemEnd > interval.end - - if (!startsBeforeInterval && !endsAfterInterval) { - store._removeItems([current.id]) - removedIds.push(current.id) - continue - } - - if (startsBeforeInterval && endsAfterInterval) { - const splitAtStart = splitItemWithBookkeeping(current.id, interval.start) - if (!splitAtStart) continue - affectedIds.push(splitAtStart.leftItem.id, splitAtStart.rightItem.id) - - const splitAtEnd = splitItemWithBookkeeping(splitAtStart.rightItem.id, interval.end) - if (!splitAtEnd) continue - store._removeItems([splitAtEnd.leftItem.id]) - removedIds.push(splitAtEnd.leftItem.id) - affectedIds.push(splitAtEnd.rightItem.id) - continue - } - if (startsBeforeInterval) { - const split = splitItemWithBookkeeping(current.id, interval.start) - if (!split) continue - store._removeItems([split.rightItem.id]) - removedIds.push(split.rightItem.id) - affectedIds.push(split.leftItem.id) - continue - } + const startSplitEntries = splitItemsWithBookkeeping( + overlapping.filter((item) => item.from < interval.start).map((item) => item.id), + interval.start, + ) + for (const entry of startSplitEntries) { + affectedIds.push(entry.result.leftItem.id, entry.result.rightItem.id) + } - const split = splitItemWithBookkeeping(current.id, interval.end) - if (!split) continue - store._removeItems([split.leftItem.id]) - removedIds.push(split.leftItem.id) - affectedIds.push(split.rightItem.id) + const endSplitEntries = splitItemsWithBookkeeping( + useItemsStore + .getState() + .items.filter( + (item) => + trackIds.has(item.trackId) && + item.from < interval.end && + item.from + item.durationInFrames > interval.end, + ) + .map((item) => item.id), + interval.end, + ) + for (const entry of endSplitEntries) { + affectedIds.push(entry.result.leftItem.id, entry.result.rightItem.id) } + const removedIds = useItemsStore + .getState() + .items.filter( + (item) => + trackIds.has(item.trackId) && + item.from >= interval.start && + item.from + item.durationInFrames <= interval.end, + ) + .map((item) => item.id) + if (removedIds.length > 0) store._removeItems(removedIds) + return { affectedIds: uniqueIds(affectedIds), removedIds: uniqueIds(removedIds), @@ -362,7 +466,7 @@ function removeItemsOnTrackInterval( } function shiftTrackItems( - trackId: string, + trackIds: ReadonlySet, predicate: (item: TimelineItem) => boolean, delta: number, ): string[] { @@ -373,7 +477,7 @@ function shiftTrackItems( const store = useItemsStore.getState() const updates = useItemsStore .getState() - .items.filter((item) => item.trackId === trackId && predicate(item)) + .items.filter((item) => trackIds.has(item.trackId) && predicate(item)) .map((item) => ({ id: item.id, from: Math.max(0, item.from + delta), @@ -389,35 +493,49 @@ function shiftTrackItems( export function propagateRemovedIntervalsToSyncLockedTracks(params: { editedTrackIds: Set intervals: TimeInterval[] + additionalAffectedIds?: ReadonlySet }): RipplePropagationResult { const intervals = normalizeIntervals(params.intervals) if (intervals.length === 0) { return { affectedIds: [], removedIds: [] } } + const { items, tracks } = useItemsStore.getState() const candidateTrackIds = getCandidateTrackIds(params.editedTrackIds) + const updatesByTrackId = buildRemovedUpdatesByTrack({ items, candidateTrackIds, intervals }) + const atomicTrackIds = new Set( + getAtomicCandidateTrackIds({ + items, + tracks, + candidateTrackIds, + updatesByTrackId, + additionalAffectedIds: params.additionalAffectedIds, + }), + ) const affectedIds: string[] = [] const removedIds: string[] = [] - for (const trackId of candidateTrackIds) { - let removedFrames = 0 - for (const interval of intervals) { - const currentInterval = { - start: interval.start - removedFrames, - end: interval.end - removedFrames, - } - const intervalLength = currentInterval.end - currentInterval.start - if (intervalLength <= 0) continue - - const overlapResult = removeItemsOnTrackInterval(trackId, currentInterval) - affectedIds.push(...overlapResult.affectedIds) - removedIds.push(...overlapResult.removedIds) - affectedIds.push( - ...shiftTrackItems(trackId, (item) => item.from >= currentInterval.end, -intervalLength), - ) - - removedFrames += intervalLength + let removedFrames = 0 + for (const interval of intervals) { + const currentInterval = { + start: interval.start - removedFrames, + end: interval.end - removedFrames, } + const intervalLength = currentInterval.end - currentInterval.start + if (intervalLength <= 0) continue + + const overlapResult = removeIntervalFromTracks(atomicTrackIds, currentInterval) + affectedIds.push(...overlapResult.affectedIds) + removedIds.push(...overlapResult.removedIds) + affectedIds.push( + ...shiftTrackItems( + atomicTrackIds, + (item) => item.from >= currentInterval.end, + -intervalLength, + ), + ) + + removedFrames += intervalLength } return { @@ -430,6 +548,7 @@ export function propagateInsertedGapToSyncLockedTracks(params: { editedTrackIds: Set cutFrame: number amount: number + additionalAffectedIds?: ReadonlySet }): RipplePropagationResult { const cutFrame = Math.max(0, Math.round(params.cutFrame)) const amount = Math.max(0, Math.round(params.amount)) @@ -437,31 +556,43 @@ export function propagateInsertedGapToSyncLockedTracks(params: { return { affectedIds: [], removedIds: [] } } + const { items, tracks } = useItemsStore.getState() const candidateTrackIds = getCandidateTrackIds(params.editedTrackIds) + const updatesByTrackId = buildInsertedUpdatesByTrack({ + items, + candidateTrackIds, + cutFrame, + amount, + }) + const atomicTrackIds = new Set( + getAtomicCandidateTrackIds({ + items, + tracks, + candidateTrackIds, + updatesByTrackId, + additionalAffectedIds: params.additionalAffectedIds, + }), + ) const affectedIds: string[] = [] - for (const trackId of candidateTrackIds) { - const straddledItems = useItemsStore + const splitEntries = splitItemsWithBookkeeping( + useItemsStore .getState() .items.filter( (item) => - item.trackId === trackId && + atomicTrackIds.has(item.trackId) && item.from < cutFrame && item.from + item.durationInFrames > cutFrame, ) - .sort((left, right) => left.from - right.from) - - for (const straddledItem of straddledItems) { - const current = useItemsStore.getState().itemById[straddledItem.id] - if (!current || current.trackId !== trackId) continue - const splitResult = splitItemWithBookkeeping(current.id, cutFrame) - if (!splitResult) continue - affectedIds.push(splitResult.leftItem.id, splitResult.rightItem.id) - } - - affectedIds.push(...shiftTrackItems(trackId, (item) => item.from >= cutFrame, amount)) + .map((item) => item.id), + cutFrame, + ) + for (const entry of splitEntries) { + affectedIds.push(entry.result.leftItem.id, entry.result.rightItem.id) } + affectedIds.push(...shiftTrackItems(atomicTrackIds, (item) => item.from >= cutFrame, amount)) + return { affectedIds: uniqueIds(affectedIds), removedIds: [], diff --git a/src/features/timeline/utils/track-content-drag.test.ts b/src/features/timeline/utils/track-content-drag.test.ts index 6a8213ab7..6f4549a8a 100644 --- a/src/features/timeline/utils/track-content-drag.test.ts +++ b/src/features/timeline/utils/track-content-drag.test.ts @@ -55,6 +55,10 @@ function makeAudioItem(id: string, trackId: string): AudioItem { } as AudioItem } +function makeVideoTracks(ids: string[]): TimelineTrack[] { + return ids.map((id, order) => makeTrack({ id, name: id.toUpperCase(), kind: 'video', order })) +} + describe('track content drag', () => { it('limits drag plans to the anchor section and ignores mixed A/V selections', () => { const tracks = [ @@ -100,6 +104,7 @@ describe('track content drag', () => { expect( buildTrackContentMoveUpdates({ + tracks: makeVideoTracks(['v3', 'v2', 'v1']), sectionTrackIds: ['v3', 'v2', 'v1'], draggedTrackIds: ['v1'], items, @@ -122,6 +127,7 @@ describe('track content drag', () => { expect( buildTrackContentMoveUpdates({ + tracks: makeVideoTracks(['v4', 'v3', 'v2', 'v1']), sectionTrackIds: ['v4', 'v3', 'v2', 'v1'], draggedTrackIds: ['v2', 'v1'], items, @@ -144,6 +150,7 @@ describe('track content drag', () => { expect( buildTrackContentMoveUpdates({ + tracks: makeVideoTracks(['v3', 'v2', 'v1']), sectionTrackIds: ['v3', 'v2', 'v1'], draggedTrackIds: ['v2'], items, @@ -218,4 +225,71 @@ describe('track content drag', () => { { id: 'clip-v1', from: 0, trackId: createdTracks?.[1]?.id }, ]) }) + + it('does not start a content reorder from a locked track', () => { + const tracks = [ + makeTrack({ id: 'v2', name: 'V2', kind: 'video', order: 0 }), + makeTrack({ id: 'v1', name: 'V1', kind: 'video', order: 1, locked: true }), + ] + + expect( + resolveTrackContentDragPlan({ + tracks, + anchorTrackId: 'v1', + selectedTrackIds: ['v1'], + }), + ).toBeNull() + }) + + it('rejects a fixed-lane reorder when any affected lane is locked', () => { + const tracks = [ + makeTrack({ id: 'v3', name: 'V3', kind: 'video', order: 0 }), + makeTrack({ id: 'v2', name: 'V2', kind: 'video', order: 1, locked: true }), + makeTrack({ id: 'v1', name: 'V1', kind: 'video', order: 2 }), + ] + const items = [ + makeVideoItem('clip-v3', 'v3'), + makeVideoItem('clip-v2', 'v2'), + makeVideoItem('clip-v1', 'v1'), + ] + + expect( + buildTrackContentMoveUpdates({ + tracks, + sectionTrackIds: ['v3', 'v2', 'v1'], + draggedTrackIds: ['v1'], + items, + insertIndex: 0, + }), + ).toEqual([]) + }) + + it('rejects track-header moves when a moved item has a locked linked companion', () => { + const tracks = [ + makeTrack({ id: 'v2', name: 'V2', kind: 'video', order: 0 }), + makeTrack({ id: 'v1', name: 'V1', kind: 'video', order: 1 }), + makeTrack({ id: 'a1', name: 'A1', kind: 'audio', order: 2, locked: true }), + ] + const video = { ...makeVideoItem('clip-v1', 'v1'), linkedGroupId: 'linked-av' } + const audio = { ...makeAudioItem('clip-a1', 'a1'), linkedGroupId: 'linked-av' } + const items = [video, audio] + + expect( + buildTrackContentMoveUpdates({ + tracks, + sectionTrackIds: ['v2', 'v1'], + draggedTrackIds: ['v1'], + items, + insertIndex: 0, + }), + ).toEqual([]) + expect( + buildTrackContentCreateTrackMovePlan({ + tracks, + items, + kind: 'video', + draggedTrackIds: ['v1'], + }), + ).toBeNull() + }) }) diff --git a/src/features/timeline/utils/track-content-drag.ts b/src/features/timeline/utils/track-content-drag.ts index 603f7da63..9de3f324c 100644 --- a/src/features/timeline/utils/track-content-drag.ts +++ b/src/features/timeline/utils/track-content-drag.ts @@ -5,6 +5,7 @@ import { getTrackKind, type TrackKind, } from './classic-tracks' +import { isTimelineTrackLocked, partitionItemMutationIdsByLock } from './track-lock-invariants' export interface TrackContentDragPlan { kind: TrackKind @@ -34,7 +35,7 @@ export function resolveTrackContentDragPlan(params: { selectedTrackIds: string[] }): TrackContentDragPlan | null { const anchorTrack = params.tracks.find((track) => track.id === params.anchorTrackId) - if (!anchorTrack) { + if (!anchorTrack || isTimelineTrackLocked(params.tracks, anchorTrack.id)) { return null } @@ -49,6 +50,14 @@ export function resolveTrackContentDragPlan(params: { } const selectedTrackIds = new Set(params.selectedTrackIds) + if ( + sectionTracks.some( + (track) => selectedTrackIds.has(track.id) && isTimelineTrackLocked(params.tracks, track.id), + ) + ) { + return null + } + const draggedTrackIds = sectionTracks .filter((track) => selectedTrackIds.has(track.id)) .map((track) => track.id) @@ -69,7 +78,10 @@ export function buildTrackContentCreateTrackMovePlan(params: { const sectionTracks = getKindTracks(params.tracks, params.kind) const draggedTrackIdsSet = new Set(params.draggedTrackIds) const draggedTracks = sectionTracks.filter((track) => draggedTrackIdsSet.has(track.id)) - if (draggedTracks.length === 0) { + if ( + draggedTracks.length === 0 || + draggedTracks.some((track) => isTimelineTrackLocked(params.tracks, track.id)) + ) { return null } @@ -121,6 +133,13 @@ export function buildTrackContentCreateTrackMovePlan(params: { ] }) + const mutationPartition = partitionItemMutationIdsByLock({ + items: params.items, + tracks: params.tracks, + itemIds: updates.map((update) => update.id), + }) + if (mutationPartition.blockedIds.length > 0) return null + return { tracks: nextTracks, updates, @@ -128,6 +147,7 @@ export function buildTrackContentCreateTrackMovePlan(params: { } export function buildTrackContentMoveUpdates(params: { + tracks: TimelineTrack[] sectionTrackIds: string[] draggedTrackIds: string[] items: TimelineItem[] @@ -174,7 +194,18 @@ export function buildTrackContentMoveUpdates(params: { } }) - return params.items.flatMap((item) => { + const affectedTrackIds = new Set() + for (const [sourceTrackId, destinationTrackId] of destinationTrackIdBySourceTrackId) { + affectedTrackIds.add(sourceTrackId) + affectedTrackIds.add(destinationTrackId) + } + if ( + Array.from(affectedTrackIds).some((trackId) => isTimelineTrackLocked(params.tracks, trackId)) + ) { + return [] + } + + const updates = params.items.flatMap((item) => { const destinationTrackId = destinationTrackIdBySourceTrackId.get(item.trackId) if (!destinationTrackId) { return [] @@ -188,4 +219,12 @@ export function buildTrackContentMoveUpdates(params: { }, ] }) + + const mutationPartition = partitionItemMutationIdsByLock({ + items: params.items, + tracks: params.tracks, + itemIds: updates.map((update) => update.id), + }) + + return mutationPartition.blockedIds.length > 0 ? [] : updates } diff --git a/src/features/timeline/utils/track-lock-invariants.ts b/src/features/timeline/utils/track-lock-invariants.ts new file mode 100644 index 000000000..434c7e7f5 --- /dev/null +++ b/src/features/timeline/utils/track-lock-invariants.ts @@ -0,0 +1,74 @@ +import type { TimelineItem, TimelineTrack } from '@/types/timeline' +import { resolveEffectiveTrackStates } from './group-utils' +import { getLinkedItems } from './linked-items' + +export interface ItemMutationLockPartition { + allowedIds: string[] + blockedIds: string[] + blockedByLockedLinkedCohort: boolean +} + +function getLockedTrackIds(tracks: TimelineTrack[]): Set { + const lockedTrackIds = new Set( + resolveEffectiveTrackStates(tracks) + .filter((track) => track.locked) + .map((track) => track.id), + ) + + for (const track of tracks) { + if (track.locked) lockedTrackIds.add(track.id) + } + + return lockedTrackIds +} + +export function isTimelineTrackLocked(tracks: TimelineTrack[], trackId: string): boolean { + return getLockedTrackIds(tracks).has(trackId) +} + +/** + * Partition a proposed item mutation without ever peeling an unlocked member + * away from a linked cohort that contains a locked member. + * + * Standalone items on locked tracks are simply ineligible. A linked cohort is + * stronger: if any companion is locked, every proposed mutation in that + * cohort is rejected so an A/V pair cannot be silently desynchronized. + */ +export function partitionItemMutationIdsByLock(params: { + items: TimelineItem[] + tracks: TimelineTrack[] + itemIds: Iterable +}): ItemMutationLockPartition { + const requestedIds = Array.from(new Set(params.itemIds)) + const requestedIdSet = new Set(requestedIds) + const lockedTrackIds = getLockedTrackIds(params.tracks) + const itemById = new Map(params.items.map((item) => [item.id, item])) + const blockedIds = new Set() + let blockedByLockedLinkedCohort = false + + for (const itemId of requestedIds) { + if (blockedIds.has(itemId)) continue + + const item = itemById.get(itemId) + if (!item) { + blockedIds.add(itemId) + continue + } + + const linkedItems = getLinkedItems(params.items, item.id) + const hasLockedMember = linkedItems.some((linkedItem) => lockedTrackIds.has(linkedItem.trackId)) + + if (!hasLockedMember) continue + + blockedByLockedLinkedCohort ||= linkedItems.length > 1 + for (const linkedItem of linkedItems) { + if (requestedIdSet.has(linkedItem.id)) blockedIds.add(linkedItem.id) + } + } + + return { + allowedIds: requestedIds.filter((itemId) => !blockedIds.has(itemId)), + blockedIds: requestedIds.filter((itemId) => blockedIds.has(itemId)), + blockedByLockedLinkedCohort, + } +} From af56cb4bb45523a8134ce16f6846718fb7ee8f8d Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 19:59:32 -0700 Subject: [PATCH 2/5] fix(timeline): make lock guards atomic across edit actions --- .../timeline/hooks/use-timeline-trim.test.tsx | 49 +-- .../actions/edit/freeze-frame-actions.ts | 61 +++- .../stores/actions/edit/join-actions.ts | 76 ++-- .../actions/edit/range-removal-actions.ts | 158 +++++++- .../actions/edit/rate-stretch-actions.ts | 210 ++++++++++- .../timeline/stores/actions/edit/shared.ts | 15 + .../stores/actions/edit/split-actions.ts | 105 +++--- .../stores/actions/edit/trim-actions.ts | 306 +++++++++++++++- .../timeline/stores/actions/item-actions.ts | 10 + .../item-edit-actions.lock-invariants.test.ts | 336 ++++++++++++++++++ .../actions/source-edit-actions.test.ts | 119 ++++++- .../stores/actions/source-edit-actions.ts | 61 +++- .../timeline/utils/source-edit-targeting.ts | 29 +- .../timeline/utils/track-lock-invariants.ts | 34 ++ 14 files changed, 1415 insertions(+), 154 deletions(-) create mode 100644 src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts diff --git a/src/features/timeline/hooks/use-timeline-trim.test.tsx b/src/features/timeline/hooks/use-timeline-trim.test.tsx index 8f831dce4..9eabc47f1 100644 --- a/src/features/timeline/hooks/use-timeline-trim.test.tsx +++ b/src/features/timeline/hooks/use-timeline-trim.test.tsx @@ -333,13 +333,7 @@ describe('useTimelineTrim', () => { .setItems([text, alignedVideo, alignedAudio, earlierVideo, earlierAudio]) useSelectionStore .getState() - .selectItems([ - 'text-1', - 'video-aligned', - 'audio-aligned', - 'video-earlier', - 'audio-earlier', - ]) + .selectItems(['text-1', 'video-aligned', 'audio-aligned', 'video-earlier', 'audio-earlier']) const { result } = renderTrimHook(text) startTrim(result, 'end') @@ -400,13 +394,7 @@ describe('useTimelineTrim', () => { .setItems([text, alignedVideo, alignedAudio, earlierVideo, earlierAudio]) useSelectionStore .getState() - .selectItems([ - 'text-1', - 'video-aligned', - 'audio-aligned', - 'video-earlier', - 'audio-earlier', - ]) + .selectItems(['text-1', 'video-aligned', 'audio-aligned', 'video-earlier', 'audio-earlier']) const { result } = renderTrimHook(text) startTrim(result, 'start') @@ -464,7 +452,7 @@ describe('useTimelineTrim', () => { expect(getItem('video-near').durationInFrames).toBe(60) }) - it('leaves a vertically aligned selected companion unchanged on a locked track', () => { + it('rejects a vertically aligned trim cohort containing a locked linked companion', () => { const text: TextItem = { id: 'text-1', type: 'text', @@ -477,21 +465,20 @@ describe('useTimelineTrim', () => { } const video = makeTimelineVideoItem({ id: 'video-1', linkedGroupId: 'lg-1' }) const audio = makeTimelineAudioItem({ id: 'audio-1', linkedGroupId: 'lg-1' }) - useItemsStore - .getState() - .setTracks([ - makeTimelineTrack({ id: 'track-v1', name: 'V1', kind: 'video', order: 0 }), - makeTimelineTrack({ id: 'track-v2', name: 'V2', kind: 'video', order: 1 }), - makeTimelineTrack({ - id: 'track-a1', - name: 'A1', - kind: 'audio', - order: 2, - locked: true, - }), - ]) + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'track-v1', name: 'V1', kind: 'video', order: 0 }), + makeTimelineTrack({ id: 'track-v2', name: 'V2', kind: 'video', order: 1 }), + makeTimelineTrack({ + id: 'track-a1', + name: 'A1', + kind: 'audio', + order: 2, + locked: true, + }), + ]) useItemsStore.getState().setItems([text, video, audio]) useSelectionStore.getState().selectItems(['text-1', 'video-1', 'audio-1']) + const undoDepthBefore = useTimelineCommandStore.getState().undoStack.length const { result } = renderTrimHook(text) startTrim(result, 'end') @@ -503,9 +490,11 @@ describe('useTimelineTrim', () => { releaseMouse() - expect(getItem('text-1').durationInFrames).toBe(50) - expect(getItem('video-1').durationInFrames).toBe(50) + expect(getItem('text-1').durationInFrames).toBe(60) + expect(getItem('video-1').durationInFrames).toBe(60) expect(getItem('audio-1').durationInFrames).toBe(60) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(undoDepthBefore) + expect(useTimelineSettingsStore.getState().isDirty).toBe(false) }) it('uses the tightest neighbor clamp across the vertical trim group', () => { diff --git a/src/features/timeline/stores/actions/edit/freeze-frame-actions.ts b/src/features/timeline/stores/actions/edit/freeze-frame-actions.ts index 6c8b7d7a6..01cc64671 100644 --- a/src/features/timeline/stores/actions/edit/freeze-frame-actions.ts +++ b/src/features/timeline/stores/actions/edit/freeze-frame-actions.ts @@ -8,7 +8,33 @@ import { importMediaLibraryService } from '@/features/timeline/deps/media-librar import { blobUrlManager } from '@/infrastructure/browser/blob-url-manager' import { execute, applyTransitionRepairs, getLogger } from '../shared' import { timelineToSourceFrames } from '../../../utils/source-calculations' -import { isInTransitionOverlap } from './shared' +import { canMutateTimelineItems, isInTransitionOverlap } from './shared' + +function canCommitFreezeFrame(itemId: string, playheadFrame: number): boolean { + const store = useItemsStore.getState() + const item = store.itemById[itemId] + if (!item || item.type !== 'video') return false + if ( + playheadFrame <= item.from || + playheadFrame >= item.from + item.durationInFrames || + isInTransitionOverlap(itemId, playheadFrame - item.from, item.durationInFrames) + ) { + return false + } + + const mutationIds = [ + itemId, + ...store.items + .filter( + (candidate) => + candidate.id !== itemId && + candidate.trackId === item.trackId && + candidate.from > playheadFrame, + ) + .map((candidate) => candidate.id), + ] + return canMutateTimelineItems(mutationIds, [item.trackId]) +} /** * Insert a freeze frame at the playhead position. @@ -33,6 +59,7 @@ export async function insertFreezeFrame(itemId: string, playheadFrame: number): if (isInTransitionOverlap(itemId, playheadFrame - itemStart, item.durationInFrames)) { return false } + if (!canCommitFreezeFrame(itemId, playheadFrame)) return false const fps = useTimelineSettingsStore.getState().fps const speed = item.speed ?? 1 @@ -144,6 +171,25 @@ export async function insertFreezeFrame(itemId: string, playheadFrame: number): const frameMediaId = mediaMetadata.id const frameBlobUrl = blobUrlManager.acquire(frameMediaId, frameBlob) + const rollbackPersistedFrame = async (): Promise => { + try { + await mediaLibraryService.deleteMediaFromProject(currentProjectId, frameMediaId) + } catch (cleanupError) { + getLogger().warn( + '[insertFreezeFrame] Failed to roll back persisted frame after rejected commit', + cleanupError, + ) + } + blobUrlManager.release(frameMediaId) + } + + // Locks can change while frame extraction and persistence are awaiting. + // Revalidate the complete split/shift cohort immediately before execute(). + if (!canCommitFreezeFrame(itemId, playheadFrame)) { + await rollbackPersistedFrame() + return false + } + // Step 4: Perform timeline mutations atomically (split + insert + shift). // Prepend the media item to the store only after execute() succeeds so a // failed _splitItem (e.g. the source clip was removed between validation @@ -229,18 +275,7 @@ export async function insertFreezeFrame(itemId: string, playheadFrame: number): // only by this project, so the reference-counted variant covers it // and preserves the global "delete everywhere" semantics for the // explicit user action. - try { - await mediaLibraryService.deleteMediaFromProject(currentProjectId, frameMediaId) - } catch (cleanupError) { - getLogger().warn( - '[insertFreezeFrame] Failed to roll back persisted frame after split failure', - cleanupError, - ) - } - // blobUrlManager.acquire above bumped the ref count for frameMediaId; - // matched release here revokes the underlying ObjectURL and frees the - // Blob so a failure path doesn't accumulate leaked frames over time. - blobUrlManager.release(frameMediaId) + await rollbackPersistedFrame() return false } diff --git a/src/features/timeline/stores/actions/edit/join-actions.ts b/src/features/timeline/stores/actions/edit/join-actions.ts index c5a60fd05..5288773dc 100644 --- a/src/features/timeline/stores/actions/edit/join-actions.ts +++ b/src/features/timeline/stores/actions/edit/join-actions.ts @@ -4,50 +4,50 @@ import { useKeyframesStore } from '../../keyframes-store' import { useTimelineSettingsStore } from '../../timeline-settings-store' import { execute, applyTransitionRepairs } from '../shared' import { getSynchronizedLinkedCounterpartPairForEdit } from '../linked-edit' -import { isLinkedSelectionEnabled } from './shared' +import { canMutateTimelineItems, isLinkedSelectionEnabled } from './shared' export function joinItems(itemIds: string[]): void { - execute( - 'JOIN_ITEMS', - () => { - const items = useItemsStore.getState().items - const itemsToJoin = items - .filter((item) => itemIds.includes(item.id)) - .toSorted((left, right) => left.from - right.from) - if (itemsToJoin.length < 2) return + const items = useItemsStore.getState().items + const itemsToJoin = items + .filter((item) => itemIds.includes(item.id)) + .toSorted((left, right) => left.from - right.from) + if (itemsToJoin.length < 2) return - const joinGroups = [itemIds] - if (itemsToJoin.length === 2) { - const [leftItem, rightItem] = itemsToJoin - if (leftItem && rightItem) { - const counterpartPair = getSynchronizedLinkedCounterpartPairForEdit( - items, - leftItem.id, - rightItem.id, - isLinkedSelectionEnabled(), - ) - if (counterpartPair) { - joinGroups.push([ - counterpartPair.leftCounterpart.id, - counterpartPair.rightCounterpart.id, - ]) - } - } + const joinGroups = [itemIds] + if (itemsToJoin.length === 2) { + const [leftItem, rightItem] = itemsToJoin + if (leftItem && rightItem) { + const counterpartPair = getSynchronizedLinkedCounterpartPairForEdit( + items, + leftItem.id, + rightItem.id, + isLinkedSelectionEnabled(), + ) + if (counterpartPair) { + joinGroups.push([counterpartPair.leftCounterpart.id, counterpartPair.rightCounterpart.id]) } + } + } - const groupDescriptors = joinGroups - .map((groupItemIds) => - items - .filter((item) => groupItemIds.includes(item.id)) - .toSorted((left, right) => left.from - right.from), - ) - .filter((groupItems) => groupItems.length >= 2) - .map((groupItems) => ({ - itemIds: groupItems.map((item) => item.id), - primaryId: groupItems[0]!.id, - removedIds: groupItems.slice(1).map((item) => item.id), - })) + const groupDescriptors = joinGroups + .map((groupItemIds) => + items + .filter((item) => groupItemIds.includes(item.id)) + .toSorted((left, right) => left.from - right.from), + ) + .filter((groupItems) => groupItems.length >= 2) + .map((groupItems) => ({ + itemIds: groupItems.map((item) => item.id), + primaryId: groupItems[0]!.id, + removedIds: groupItems.slice(1).map((item) => item.id), + })) + const mutationIds = groupDescriptors.flatMap((group) => group.itemIds) + if (groupDescriptors.length === 0 || !canMutateTimelineItems(mutationIds)) return + + execute( + 'JOIN_ITEMS', + () => { for (const group of groupDescriptors) { useItemsStore.getState()._joinItems(group.itemIds) } diff --git a/src/features/timeline/stores/actions/edit/range-removal-actions.ts b/src/features/timeline/stores/actions/edit/range-removal-actions.ts index 6bd40556c..81caab759 100644 --- a/src/features/timeline/stores/actions/edit/range-removal-actions.ts +++ b/src/features/timeline/stores/actions/edit/range-removal-actions.ts @@ -11,9 +11,13 @@ import { } from '../../../utils/media-item-frames' import { getUniqueLinkedItemAnchorIds } from '../../../utils/linked-items' import { isTrackSyncLockEnabled } from '../../../utils/track-sync-lock' -import { propagateRemovedIntervalsToSyncLockedTracks } from '../sync-lock-ripple' +import { + buildRemovedIntervalPreviewUpdatesForSyncLockedTracks, + propagateRemovedIntervalsToSyncLockedTracks, +} from '../sync-lock-ripple' import { applySplitBookkeeping, type SplitResultEntry } from '../split-bookkeeping' import { + canMutateTimelineItems, isLinkedSelectionEnabled, isInTransitionOverlap, requestPostEditWarmForItems, @@ -206,6 +210,148 @@ export function removeTranscriptRangesFromItems( return removeTimelineRangesFromItems('REMOVE_TRANSCRIPT_SELECTION', itemIds, rangesByMediaId) } +function getRangeRemovalAnchors( + itemIds: string[], + rangesByMediaId: Record, +): TimelineItem[] { + const store = useItemsStore.getState() + const anchorIds = getUniqueLinkedItemAnchorIds(store.items, itemIds) + return anchorIds + .map((id) => store.itemById[id]) + .filter( + (item): item is TimelineItem => + item !== undefined && + (item.type === 'video' || item.type === 'audio') && + !!item.mediaId && + (rangesByMediaId[item.mediaId]?.length ?? 0) > 0, + ) +} + +function getAnchorTimelineIntervals( + anchor: TimelineItem, + ranges: RemoveSilenceRange[], + timelineFps: number, +): RemoveSilenceRange[] { + return ranges.flatMap((range) => { + const firstFrame = sourceSecondsToTimelineFrame(anchor, range.start, timelineFps) + const secondFrame = sourceSecondsToTimelineFrame(anchor, range.end, timelineFps) + const start = Math.max(anchor.from, Math.min(firstFrame, secondFrame)) + const end = Math.min(anchor.from + anchor.durationInFrames, Math.max(firstFrame, secondFrame)) + return end > start ? [{ start, end }] : [] + }) +} + +interface RangeRemovalPreflightAccumulator { + mutationIds: Set + editedTrackIds: Set + earliestAffectedFrameByTrackId: Map + intervals: RemoveSilenceRange[] +} + +function addRangeAnchorPreflight(params: { + anchor: TimelineItem + ranges: RemoveSilenceRange[] + timelineFps: number + linkedSelectionEnabled: boolean + accumulator: RangeRemovalPreflightAccumulator +}): void { + const items = useItemsStore.getState().items + const splitItems = getLinkedItemsForEdit(items, params.anchor.id, params.linkedSelectionEnabled) + const anchorIntervals = getAnchorTimelineIntervals( + params.anchor, + params.ranges, + params.timelineFps, + ) + params.accumulator.intervals.push(...anchorIntervals) + + for (const splitItem of splitItems) { + params.accumulator.editedTrackIds.add(splitItem.trackId) + for (const relatedId of expandIdsWithLinkedItems( + items, + [splitItem.id], + params.linkedSelectionEnabled, + )) { + params.accumulator.mutationIds.add(relatedId) + } + + for (const interval of anchorIntervals) { + const previousStart = + params.accumulator.earliestAffectedFrameByTrackId.get(splitItem.trackId) ?? + Number.POSITIVE_INFINITY + params.accumulator.earliestAffectedFrameByTrackId.set( + splitItem.trackId, + Math.min(previousStart, interval.start), + ) + } + } +} + +function addRangeDownstreamPreflight(params: { + items: TimelineItem[] + linkedSelectionEnabled: boolean + accumulator: RangeRemovalPreflightAccumulator +}): void { + for (const item of params.items) { + const earliestAffectedFrame = params.accumulator.earliestAffectedFrameByTrackId.get( + item.trackId, + ) + if ( + earliestAffectedFrame === undefined || + item.from + item.durationInFrames <= earliestAffectedFrame + ) { + continue + } + for (const relatedId of expandIdsWithLinkedItems( + params.items, + [item.id], + params.linkedSelectionEnabled, + )) { + params.accumulator.mutationIds.add(relatedId) + } + } +} + +function buildRangeRemovalPreflight( + itemIds: string[], + rangesByMediaId: Record, +): { analyzedItemCount: number; mutationIds: string[] } { + const store = useItemsStore.getState() + const timelineFps = useTimelineSettingsStore.getState().fps + const anchors = getRangeRemovalAnchors(itemIds, rangesByMediaId) + if (anchors.length === 0) return { analyzedItemCount: 0, mutationIds: [] } + + const linkedSelectionEnabled = isLinkedSelectionEnabled() + const accumulator: RangeRemovalPreflightAccumulator = { + mutationIds: new Set(), + editedTrackIds: new Set(), + earliestAffectedFrameByTrackId: new Map(), + intervals: [], + } + + for (const anchor of anchors) { + addRangeAnchorPreflight({ + anchor, + ranges: rangesByMediaId[anchor.mediaId!] ?? [], + timelineFps, + linkedSelectionEnabled, + accumulator, + }) + } + + addRangeDownstreamPreflight({ items: store.items, linkedSelectionEnabled, accumulator }) + + const syncLockUpdates = buildRemovedIntervalPreviewUpdatesForSyncLockedTracks({ + items: store.items, + tracks: store.tracks, + editedTrackIds: accumulator.editedTrackIds, + intervals: accumulator.intervals, + additionalAffectedIds: accumulator.mutationIds, + }) + for (const update of syncLockUpdates) accumulator.mutationIds.add(update.id) + + return { analyzedItemCount: anchors.length, mutationIds: Array.from(accumulator.mutationIds) } +} + function removeTimelineRangesFromItems( commandType: 'REMOVE_SILENCE' | 'REMOVE_FILLER_WORDS' | 'REMOVE_TRANSCRIPT_SELECTION', itemIds: string[], @@ -215,6 +361,16 @@ function removeTimelineRangesFromItems( return { analyzedItemCount: 0, removedRangeCount: 0, removedItemCount: 0, splitCount: 0 } } + const preflight = buildRangeRemovalPreflight(itemIds, rangesByMediaId) + if (preflight.mutationIds.length === 0 || !canMutateTimelineItems(preflight.mutationIds)) { + return { + analyzedItemCount: preflight.analyzedItemCount, + removedRangeCount: 0, + removedItemCount: 0, + splitCount: 0, + } + } + return execute( commandType, () => { diff --git a/src/features/timeline/stores/actions/edit/rate-stretch-actions.ts b/src/features/timeline/stores/actions/edit/rate-stretch-actions.ts index 7d665bbb1..7d2cede93 100644 --- a/src/features/timeline/stores/actions/edit/rate-stretch-actions.ts +++ b/src/features/timeline/stores/actions/edit/rate-stretch-actions.ts @@ -2,11 +2,199 @@ import { useItemsStore } from '../../items-store' import { useTransitionsStore } from '../../transitions-store' import { useKeyframesStore } from '../../keyframes-store' import { useTimelineSettingsStore } from '../../timeline-settings-store' +import type { TimelineItem } from '@/types/timeline' import { execute, applyTransitionRepairs } from '../shared' import { getSynchronizedLinkedItemsForEdit } from '../linked-edit' import { timelineToSourceFrames, sourceToTimelineFrames } from '../../../utils/source-calculations' import { expandItemIdsWithAttachedCaptions, getLinkedItemIds } from '../../../utils/linked-items' -import { isLinkedSelectionEnabled, requestPostEditWarmForItems } from './shared' +import { + canMutateTimelineItems, + isLinkedSelectionEnabled, + requestPostEditWarmForItems, +} from './shared' +import { roundDuration, roundFrame } from '../../items-store-normalize' + +function addLinkedRippleCohort( + items: TimelineItem[], + itemId: string, + mutationIds: Set, +): void { + for (const relatedId of expandItemIdsWithAttachedCaptions( + items, + getLinkedItemIds(items, itemId), + )) { + mutationIds.add(relatedId) + } +} + +function collectRateStretchEndMutationIds(params: { + items: TimelineItem[] + synchronizedItems: TimelineItem[] + synchronizedIds: Set + touchedTrackIds: Set + oldEnd: number + mutationIds: Set +}): void { + for (const candidate of params.items) { + if ( + params.synchronizedIds.has(candidate.id) || + !params.touchedTrackIds.has(candidate.trackId) || + candidate.from < params.oldEnd + ) { + continue + } + addLinkedRippleCohort(params.items, candidate.id, params.mutationIds) + } + + const transitions = useTransitionsStore.getState().transitions + for (const synchronizedItem of params.synchronizedItems) { + for (const transition of transitions) { + if (transition.leftClipId !== synchronizedItem.id) continue + addLinkedRippleCohort(params.items, transition.rightClipId, params.mutationIds) + } + } +} + +function collectRateStretchStartMutationIds(params: { + items: TimelineItem[] + synchronizedItems: TimelineItem[] + synchronizedIds: Set + touchedTrackIds: Set + oldFrom: number + mutationIds: Set +}): void { + for (const candidate of params.items) { + if ( + params.synchronizedIds.has(candidate.id) || + !params.touchedTrackIds.has(candidate.trackId) || + candidate.from + candidate.durationInFrames > params.oldFrom + ) { + continue + } + addLinkedRippleCohort(params.items, candidate.id, params.mutationIds) + } + + const transitions = useTransitionsStore.getState().transitions + for (const synchronizedItem of params.synchronizedItems) { + for (const transition of transitions) { + if (transition.rightClipId !== synchronizedItem.id) continue + addLinkedRippleCohort(params.items, transition.leftClipId, params.mutationIds) + } + } +} + +function getRateStretchMutationIds(id: string, newFrom: number, newDuration: number): string[] { + const items = useItemsStore.getState().items + const synchronizedItems = getSynchronizedLinkedItemsForEdit(items, id, isLinkedSelectionEnabled()) + const anchor = synchronizedItems.find((item) => item.id === id) + if (!anchor) return [] + + const synchronizedIds = new Set(synchronizedItems.map((item) => item.id)) + const mutationIds = new Set(synchronizedIds) + const oldEnd = anchor.from + anchor.durationInFrames + const fromDelta = roundFrame(newFrom) - anchor.from + const endDelta = roundFrame(newFrom) + roundDuration(newDuration) - oldEnd + const touchedTrackIds = new Set(synchronizedItems.map((item) => item.trackId)) + + if (endDelta !== 0) { + collectRateStretchEndMutationIds({ + items, + synchronizedItems, + synchronizedIds, + touchedTrackIds, + oldEnd, + mutationIds, + }) + } + + if (fromDelta !== 0) { + collectRateStretchStartMutationIds({ + items, + synchronizedItems, + synchronizedIds, + touchedTrackIds, + oldFrom: anchor.from, + mutationIds, + }) + } + + return Array.from(mutationIds) +} + +interface ResetSpeedMutationPlan { + synchronizedItems: TimelineItem[] + oldEnd: number + growth: number +} + +function calculateResetSpeedDuration(item: TimelineItem, fps: number): number { + const currentSpeed = item.speed || 1 + const sourceFps = item.sourceFps ?? fps + const effectiveSourceFrames = + item.sourceEnd !== undefined && item.sourceStart !== undefined + ? item.sourceEnd - item.sourceStart + : timelineToSourceFrames(item.durationInFrames, currentSpeed, fps, sourceFps) + return Math.max(1, sourceToTimelineFrames(effectiveSourceFrames, 1, sourceFps, fps)) +} + +function getResetSpeedMutationPlan( + items: TimelineItem[], + id: string, + fps: number, +): ResetSpeedMutationPlan | null { + const item = items.find((candidate) => candidate.id === id) + if (!item || (item.type !== 'video' && item.type !== 'audio')) return null + const currentSpeed = item.speed || 1 + if (Math.abs(currentSpeed - 1) <= 0.01) return null + + const synchronizedItems = getSynchronizedLinkedItemsForEdit(items, id, isLinkedSelectionEnabled()) + const newDuration = calculateResetSpeedDuration(item, fps) + return { + synchronizedItems, + oldEnd: item.from + item.durationInFrames, + growth: roundDuration(newDuration) - item.durationInFrames, + } +} + +function addResetSpeedDownstreamMutationIds(params: { + items: TimelineItem[] + plan: ResetSpeedMutationPlan + processedIds: Set + mutationIds: Set +}): void { + if (params.plan.growth <= 0) return + const touchedTrackIds = new Set(params.plan.synchronizedItems.map((item) => item.trackId)) + for (const candidate of params.items) { + if ( + params.processedIds.has(candidate.id) || + !touchedTrackIds.has(candidate.trackId) || + candidate.from < params.plan.oldEnd + ) { + continue + } + addLinkedRippleCohort(params.items, candidate.id, params.mutationIds) + } +} + +function getResetSpeedMutationIds(itemIds: string[]): string[] { + const items = useItemsStore.getState().items + const fps = useTimelineSettingsStore.getState().fps + const mutationIds = new Set() + const processedIds = new Set() + + for (const id of itemIds) { + if (processedIds.has(id)) continue + const plan = getResetSpeedMutationPlan(items, id, fps) + if (!plan) continue + for (const synchronizedItem of plan.synchronizedItems) { + processedIds.add(synchronizedItem.id) + mutationIds.add(synchronizedItem.id) + } + addResetSpeedDownstreamMutationIds({ items, plan, processedIds, mutationIds }) + } + + return Array.from(mutationIds) +} export function rateStretchItemWithoutHistory( id: string, @@ -14,6 +202,9 @@ export function rateStretchItemWithoutHistory( newDuration: number, newSpeed: number, ): void { + const mutationIds = getRateStretchMutationIds(id, newFrom, newDuration) + if (mutationIds.length === 0 || !canMutateTimelineItems(mutationIds)) return + const itemsStore = useItemsStore.getState() const itemsBefore = itemsStore.items const synchronizedItems = getSynchronizedLinkedItemsForEdit( @@ -219,6 +410,9 @@ export function rateStretchItem( newDuration: number, newSpeed: number, ): void { + const mutationIds = getRateStretchMutationIds(id, newFrom, newDuration) + if (mutationIds.length === 0 || !canMutateTimelineItems(mutationIds)) return + execute( 'RATE_STRETCH_ITEM', () => { @@ -239,6 +433,9 @@ export function rateStretchItem( */ export function resetSpeedWithRipple(itemIds: string[]): void { const TOLERANCE = 0.01 + const mutationIds = getResetSpeedMutationIds(itemIds) + if (mutationIds.length === 0 || !canMutateTimelineItems(mutationIds)) return + execute( 'RESET_SPEED_WITH_RIPPLE', () => { @@ -270,16 +467,7 @@ export function resetSpeedWithRipple(itemIds: string[]): void { ) for (const si of synchronizedItems) processedIds.add(si.id) - const sourceFps = item.sourceFps ?? fps - const effectiveSourceFrames = - item.sourceEnd !== undefined && item.sourceStart !== undefined - ? item.sourceEnd - item.sourceStart - : timelineToSourceFrames(item.durationInFrames, currentSpeed, fps, sourceFps) - - const newDuration = Math.max( - 1, - sourceToTimelineFrames(effectiveSourceFrames, 1, sourceFps, fps), - ) + const newDuration = calculateResetSpeedDuration(item, fps) const oldEnd = item.from + item.durationInFrames stretchOps.push({ diff --git a/src/features/timeline/stores/actions/edit/shared.ts b/src/features/timeline/stores/actions/edit/shared.ts index f3c66d044..ce3a91136 100644 --- a/src/features/timeline/stores/actions/edit/shared.ts +++ b/src/features/timeline/stores/actions/edit/shared.ts @@ -10,11 +10,26 @@ import { usePreviewBridgeStore } from '@/shared/state/preview-bridge' import { useItemsStore } from '../../items-store' import { useTransitionsStore } from '../../transitions-store' import { calculateTransitionPortions } from '@/shared/timeline/transitions/transition-planner' +import { preflightTimelineMutation } from '../../../utils/track-lock-invariants' export function isLinkedSelectionEnabled(): boolean { return useEditorStore.getState().linkedSelectionEnabled } +/** Public compound actions call this after planning their complete cohort and before execute(). */ +export function canMutateTimelineItems( + itemIds: Iterable, + destinationTrackIds: Iterable = [], +): boolean { + const { items, tracks } = useItemsStore.getState() + return preflightTimelineMutation({ + items, + tracks, + itemIds, + destinationTrackIds, + }).allowed +} + const POST_EDIT_WARM_MAX_FRAMES = 32 function appendWarmFrame(target: number[], seen: Set, frame: number): void { diff --git a/src/features/timeline/stores/actions/edit/split-actions.ts b/src/features/timeline/stores/actions/edit/split-actions.ts index 73308a5f5..ed96dde73 100644 --- a/src/features/timeline/stores/actions/edit/split-actions.ts +++ b/src/features/timeline/stores/actions/edit/split-actions.ts @@ -7,7 +7,7 @@ import { execute, applyTransitionRepairs } from '../shared' import { getLinkedItemsForEdit } from '../linked-edit' import { getUniqueLinkedItemAnchorIds } from '../../../utils/linked-items' import { applySplitBookkeeping, type SplitResultEntry } from '../split-bookkeeping' -import { isLinkedSelectionEnabled, isInTransitionOverlap } from './shared' +import { canMutateTimelineItems, isLinkedSelectionEnabled, isInTransitionOverlap } from './shared' import { emitUiSound } from '@/shared/ui/ui-sound' export function splitItem( @@ -16,6 +16,9 @@ export function splitItem( ): { leftItem: TimelineItem; rightItem: TimelineItem } | null { const items = useItemsStore.getState().items const itemsToSplit = getLinkedItemsForEdit(items, id, isLinkedSelectionEnabled()) + if (itemsToSplit.length === 0 || !canMutateTimelineItems(itemsToSplit.map((item) => item.id))) { + return null + } for (const item of itemsToSplit) { // Bounds check first — out-of-range splits are a silent no-op (handled by _splitItem), @@ -77,53 +80,58 @@ export function splitAllItemsAtFrame(splitFrame: number): number { if (anchorIds.length === 0) return 0 - let splitCount = 0 + const splitPlans = anchorIds.flatMap((anchorId) => { + const itemsToSplit = getLinkedItemsForEdit(items, anchorId, isLinkedSelectionEnabled()) + if (itemsToSplit.length === 0) return [] - execute( - 'SPLIT_ALL_ITEMS_AT_FRAME', - () => { - for (const anchorId of anchorIds) { - const currentItems = useItemsStore.getState().items - const itemsToSplit = getLinkedItemsForEdit( - currentItems, - anchorId, - isLinkedSelectionEnabled(), - ) - if (itemsToSplit.length === 0) continue - - let blockedByTransition = false - const canSplitGroup = itemsToSplit.every((item) => { - if (splitFrame <= item.from || splitFrame >= item.from + item.durationInFrames) { - return false - } + let blockedByTransition = false + const canSplitGroup = itemsToSplit.every((item) => { + if (splitFrame <= item.from || splitFrame >= item.from + item.durationInFrames) { + return false + } - const relativeFrame = splitFrame - item.from - if (isInTransitionOverlap(item.id, relativeFrame, item.durationInFrames)) { - blockedByTransition = true - return false - } + const relativeFrame = splitFrame - item.from + if (isInTransitionOverlap(item.id, relativeFrame, item.durationInFrames)) { + blockedByTransition = true + return false + } - return true - }) + return true + }) - if (!canSplitGroup) { - if (blockedByTransition) { - toast.warning('Cannot split inside a transition zone') - emitUiSound('error') - } - continue - } + if (!canSplitGroup) { + if (blockedByTransition) { + toast.warning('Cannot split inside a transition zone') + emitUiSound('error') + } + return [] + } - const splitResults = itemsToSplit - .map((item) => ({ - originalId: item.id, - originalLinkedGroupId: item.linkedGroupId, - result: useItemsStore.getState()._splitItem(item.id, splitFrame), - })) + const itemIds = itemsToSplit.map((item) => item.id) + return canMutateTimelineItems(itemIds) ? [{ anchorId, itemIds }] : [] + }) + + if (splitPlans.length === 0) return 0 + + let splitCount = 0 + + execute( + 'SPLIT_ALL_ITEMS_AT_FRAME', + () => { + for (const plan of splitPlans) { + const splitResults = plan.itemIds + .map((itemId) => { + const item = useItemsStore.getState().itemById[itemId] + return { + originalId: itemId, + originalLinkedGroupId: item?.linkedGroupId, + result: useItemsStore.getState()._splitItem(itemId, splitFrame), + } + }) .filter((entry): entry is SplitResultEntry => entry.result !== null) const anchorResult = - splitResults.find((entry) => entry.originalId === anchorId)?.result ?? null + splitResults.find((entry) => entry.originalId === plan.anchorId)?.result ?? null if (!anchorResult) continue applySplitBookkeeping(splitResults) @@ -137,7 +145,7 @@ export function splitAllItemsAtFrame(splitFrame: number): number { useTimelineSettingsStore.getState().markDirty() } }, - { ids: anchorIds, splitFrame }, + { ids: splitPlans.map((plan) => plan.anchorId), splitFrame }, ) if (splitCount > 0) emitUiSound('confirm') @@ -154,18 +162,19 @@ export function splitItemAtFrames(id: string, splitFrames: number[]): number { if (splitFrames.length === 0) return 0 const sorted = [...splitFrames].sort((a, b) => b - a) + const itemsToSplit = getLinkedItemsForEdit( + useItemsStore.getState().items, + id, + isLinkedSelectionEnabled(), + ) + if (itemsToSplit.length === 0 || !canMutateTimelineItems(itemsToSplit.map((item) => item.id))) { + return 0 + } let splitCount = 0 execute( 'SPLIT_ITEM_MULTI', () => { - const itemsToSplit = getLinkedItemsForEdit( - useItemsStore.getState().items, - id, - isLinkedSelectionEnabled(), - ) - if (itemsToSplit.length === 0) return - const rightIdsByOriginalId = new Map(itemsToSplit.map((item) => [item.id, [] as string[]])) for (const frame of sorted) { diff --git a/src/features/timeline/stores/actions/edit/trim-actions.ts b/src/features/timeline/stores/actions/edit/trim-actions.ts index 81391ec0b..4410e8f21 100644 --- a/src/features/timeline/stores/actions/edit/trim-actions.ts +++ b/src/features/timeline/stores/actions/edit/trim-actions.ts @@ -8,7 +8,10 @@ import { getSynchronizedLinkedCounterpartPairForEdit, getSynchronizedLinkedItemsForEdit, } from '../linked-edit' -import { getAttachedCaptionItemIds } from '../../../utils/linked-items' +import { + expandItemIdsWithAttachedCaptions, + getAttachedCaptionItemIds, +} from '../../../utils/linked-items' import { computeClampedSlipDelta } from '../../../utils/slip-utils' import { computeSlideContinuitySourceDelta } from '../../../utils/slide-utils' import { clampSlideDeltaToPreserveKeyframes } from '../../../utils/slide-keyframe-constraints' @@ -24,11 +27,17 @@ import { clampSlipDeltaToPreserveTransitions, } from '../../../utils/transition-utils' import { + buildInsertedGapPreviewUpdatesForSyncLockedTracks, + buildRemovedIntervalPreviewUpdatesForSyncLockedTracks, propagateInsertedGapToSyncLockedTracks, propagateRemovedIntervalsToSyncLockedTracks, } from '../sync-lock-ripple' -import { isLinkedSelectionEnabled, requestPostEditWarmForItems } from './shared' -import type { TimelineItem } from '@/types/timeline' +import { + canMutateTimelineItems, + isLinkedSelectionEnabled, + requestPostEditWarmForItems, +} from './shared' +import type { TimelineItem, TimelineTrack } from '@/types/timeline' function keepTightestDelta(requested: number, candidate: number): number { return requested < 0 ? Math.max(requested, candidate) : Math.min(requested, candidate) @@ -73,6 +82,249 @@ function getSynchronizedTrimItems( return Array.from(synchronizedById.values()) } +function getSynchronizedTrimMutationIds( + id: string, + handle: 'start' | 'end', + trimAmount: number, + options: SynchronizedTrimOptions, +): string[] { + const items = useItemsStore.getState().items + const synchronizedItems = getSynchronizedTrimItems(items, id, options) + if (!synchronizedItems.some((item) => item.id === id)) return [] + + const synchronizedIds = synchronizedItems.map((item) => item.id) + const shrinksVisibleBounds = handle === 'start' ? trimAmount > 0 : trimAmount < 0 + return shrinksVisibleBounds + ? expandItemIdsWithAttachedCaptions(items, synchronizedIds) + : synchronizedIds +} + +function getClampedRippleTrimDelta(params: { + items: TimelineItem[] + synced: TimelineItem[] + syncedIds: Set + handle: 'start' | 'end' + trimDelta: number +}): number { + const transitions = useTransitionsStore.getState().transitions + const keyframesByItemId = useKeyframesStore.getState().keyframesByItemId + const timelineFps = useTimelineSettingsStore.getState().fps + let clampedTrimDelta = params.trimDelta + for (const syncedItem of params.synced) { + clampedTrimDelta = keepTightestDelta( + clampedTrimDelta, + clampRippleTrimDeltaToPreserveEditState( + syncedItem, + params.handle, + clampedTrimDelta, + params.items, + transitions, + keyframesByItemId, + timelineFps, + params.syncedIds, + false, + ), + ) + } + return clampedTrimDelta +} + +function addRippleTrimDownstreamMutationIds(params: { + items: TimelineItem[] + synced: TimelineItem[] + syncedIds: Set + mutationIds: Set +}): void { + const transitions = useTransitionsStore.getState().transitions + for (const syncedItem of params.synced) { + const oldSyncedEnd = syncedItem.from + syncedItem.durationInFrames + const transitionNeighbors = new Set( + transitions + .filter((transition) => transition.leftClipId === syncedItem.id) + .map((transition) => transition.rightClipId), + ) + for (const candidate of params.items) { + if (params.syncedIds.has(candidate.id) || candidate.trackId !== syncedItem.trackId) continue + if (candidate.from >= oldSyncedEnd || transitionNeighbors.has(candidate.id)) { + params.mutationIds.add(candidate.id) + } + } + } +} + +function addRippleTrimSyncLockMutationIds(params: { + items: TimelineItem[] + tracks: TimelineTrack[] + synced: TimelineItem[] + oldEnd: number + shift: number + mutationIds: Set +}): void { + const editedTrackIds = new Set(params.synced.map((candidate) => candidate.trackId)) + const additionalAffectedIds = new Set(params.mutationIds) + const previewUpdates = + params.shift < 0 + ? buildRemovedIntervalPreviewUpdatesForSyncLockedTracks({ + items: params.items, + tracks: params.tracks, + editedTrackIds, + intervals: [{ start: params.oldEnd + params.shift, end: params.oldEnd }], + additionalAffectedIds, + }) + : buildInsertedGapPreviewUpdatesForSyncLockedTracks({ + items: params.items, + tracks: params.tracks, + editedTrackIds, + cutFrame: params.oldEnd, + amount: params.shift, + additionalAffectedIds, + }) + for (const update of previewUpdates) params.mutationIds.add(update.id) +} + +function getRippleTrimMutationIds( + id: string, + handle: 'start' | 'end', + trimDelta: number, +): string[] { + const store = useItemsStore.getState() + const item = store.itemById[id] + if (!item) return [] + + const synced = getSynchronizedLinkedItemsForEdit(store.items, id, isLinkedSelectionEnabled()) + const syncedIds = new Set(synced.map((candidate) => candidate.id)) + const clampedTrimDelta = getClampedRippleTrimDelta({ + items: store.items, + synced, + syncedIds, + handle, + trimDelta, + }) + if (clampedTrimDelta === 0) return [] + + const mutationIds = new Set(syncedIds) + const shift = handle === 'end' ? clampedTrimDelta : -clampedTrimDelta + const oldEnd = item.from + item.durationInFrames + addRippleTrimDownstreamMutationIds({ items: store.items, synced, syncedIds, mutationIds }) + + const shrinksVisibleBounds = handle === 'start' ? clampedTrimDelta > 0 : clampedTrimDelta < 0 + if (shrinksVisibleBounds) { + for (const captionId of expandItemIdsWithAttachedCaptions(store.items, [...syncedIds])) { + mutationIds.add(captionId) + } + } + + if (shift !== 0) { + addRippleTrimSyncLockMutationIds({ + items: store.items, + tracks: store.tracks, + synced, + oldEnd, + shift, + mutationIds, + }) + } + + return Array.from(mutationIds) +} + +function getOptionalItem(items: TimelineItem[], itemId: string | null): TimelineItem | null { + return itemId ? (items.find((candidate) => candidate.id === itemId) ?? null) : null +} + +function getOptionalSynchronizedCounterpart(params: { + items: TimelineItem[] + neighborId: string | null + trackId: string + type: TimelineItem['type'] +}): TimelineItem | null { + if (!params.neighborId) return null + return getMatchingSynchronizedLinkedCounterpartForEdit( + params.items, + params.neighborId, + params.trackId, + params.type, + isLinkedSelectionEnabled(), + ) +} + +function getSlideCounterpartMutationIds(items: TimelineItem[], id: string): string[] { + const synchronizedCounterpart = + getSynchronizedLinkedItemsForEdit(items, id, isLinkedSelectionEnabled()).find( + (candidate) => candidate.id !== id, + ) ?? null + return synchronizedCounterpart ? [synchronizedCounterpart.id] : [] +} + +function getSlideCounterpartNeighborMutationIds(params: { + items: TimelineItem[] + id: string + leftNeighborId: string | null + rightNeighborId: string | null +}): string[] { + const synchronizedCounterpart = + getSynchronizedLinkedItemsForEdit(params.items, params.id, isLinkedSelectionEnabled()).find( + (candidate) => candidate.id !== params.id, + ) ?? null + if (!synchronizedCounterpart) return [] + + const counterpartEnd = synchronizedCounterpart.from + synchronizedCounterpart.durationInFrames + const leftCounterpart = getOptionalSynchronizedCounterpart({ + items: params.items, + neighborId: params.leftNeighborId, + trackId: synchronizedCounterpart.trackId, + type: synchronizedCounterpart.type, + }) + const rightCounterpart = getOptionalSynchronizedCounterpart({ + items: params.items, + neighborId: params.rightNeighborId, + trackId: synchronizedCounterpart.trackId, + type: synchronizedCounterpart.type, + }) + const cpLeftAdj = + params.items.find( + (candidate) => + candidate.trackId === synchronizedCounterpart.trackId && + candidate.id !== synchronizedCounterpart.id && + candidate.from + candidate.durationInFrames === synchronizedCounterpart.from, + ) ?? leftCounterpart + const cpRightAdj = + params.items.find( + (candidate) => + candidate.trackId === synchronizedCounterpart.trackId && + candidate.id !== synchronizedCounterpart.id && + candidate.from === counterpartEnd, + ) ?? rightCounterpart + return [cpLeftAdj?.id, cpRightAdj?.id].filter((itemId): itemId is string => !!itemId) +} + +function getSlideMutationIds( + id: string, + leftNeighborId: string | null, + rightNeighborId: string | null, +): string[] { + const items = useItemsStore.getState().items + if (!items.some((candidate) => candidate.id === id)) return [] + + const mutationIds = new Set([id]) + const leftNeighbor = getOptionalItem(items, leftNeighborId) + const rightNeighbor = getOptionalItem(items, rightNeighborId) + if (leftNeighbor) mutationIds.add(leftNeighbor.id) + if (rightNeighbor) mutationIds.add(rightNeighbor.id) + for (const counterpartId of getSlideCounterpartMutationIds(items, id)) { + mutationIds.add(counterpartId) + } + for (const neighborId of getSlideCounterpartNeighborMutationIds({ + items, + id, + leftNeighborId, + rightNeighborId, + })) { + mutationIds.add(neighborId) + } + return Array.from(mutationIds) +} + function clampSlideParticipantDelta( requestedDelta: number, item: TimelineItem, @@ -255,6 +507,9 @@ export function trimItemStart( trimAmount: number, options: SynchronizedTrimOptions = {}, ): void { + const mutationIds = getSynchronizedTrimMutationIds(id, 'start', trimAmount, options) + if (mutationIds.length === 0 || !canMutateTimelineItems(mutationIds)) return + execute( 'TRIM_ITEM_START', () => { @@ -269,6 +524,9 @@ export function trimItemEnd( trimAmount: number, options: SynchronizedTrimOptions = {}, ): void { + const mutationIds = getSynchronizedTrimMutationIds(id, 'end', trimAmount, options) + if (mutationIds.length === 0 || !canMutateTimelineItems(mutationIds)) return + execute( 'TRIM_ITEM_END', () => { @@ -285,6 +543,14 @@ export function trimItemBreakingTransition( transitionIdsToRemove: string[], options: Pick = {}, ): void { + const mutationIds = new Set(getSynchronizedTrimMutationIds(id, handle, trimAmount, options)) + for (const transition of useTransitionsStore.getState().transitions) { + if (!transitionIdsToRemove.includes(transition.id)) continue + mutationIds.add(transition.leftClipId) + mutationIds.add(transition.rightClipId) + } + if (mutationIds.size === 0 || !canMutateTimelineItems(mutationIds)) return + execute( handle === 'start' ? 'TRIM_ITEM_START' : 'TRIM_ITEM_END', () => { @@ -321,6 +587,9 @@ export function trimItemBreakingTransition( */ export function rippleTrimItem(id: string, handle: 'start' | 'end', trimDelta: number): void { if (trimDelta === 0) return + const mutationIds = getRippleTrimMutationIds(id, handle, trimDelta) + if (mutationIds.length === 0 || !canMutateTimelineItems(mutationIds)) return + execute( 'RIPPLE_EDIT', () => { @@ -473,6 +742,23 @@ export function rippleTrimItem(id: string, handle: 'start' | 'end', trimDelta: n export function rollingTrimItems(leftId: string, rightId: string, editPointDelta: number): void { if (editPointDelta === 0) return + const items = useItemsStore.getState().items + const leftItem = items.find((item) => item.id === leftId) + const rightItem = items.find((item) => item.id === rightId) + if (!leftItem || !rightItem) return + const mutationIds = new Set([leftId, rightId]) + const counterpartPair = getSynchronizedLinkedCounterpartPairForEdit( + items, + leftId, + rightId, + isLinkedSelectionEnabled(), + ) + if (counterpartPair) { + mutationIds.add(counterpartPair.leftCounterpart.id) + mutationIds.add(counterpartPair.rightCounterpart.id) + } + if (!canMutateTimelineItems(mutationIds)) return + execute( 'ROLLING_EDIT', () => { @@ -578,6 +864,18 @@ export function rollingTrimItems(leftId: string, rightId: string, editPointDelta export function slipItem(id: string, slipDelta: number): void { if (slipDelta === 0) return + const items = useItemsStore.getState().items + const item = items.find((candidate) => candidate.id === id) + if ( + !item || + (item.type !== 'video' && item.type !== 'audio' && item.type !== 'composition') || + item.sourceEnd === undefined + ) { + return + } + const synchronizedItems = getSynchronizedLinkedItemsForEdit(items, id, isLinkedSelectionEnabled()) + if (!canMutateTimelineItems(synchronizedItems.map((candidate) => candidate.id))) return + execute( 'SLIP_EDIT', () => { @@ -657,6 +955,8 @@ export function slideItem( rightNeighborId: string | null, ): void { if (slideDelta === 0) return + const mutationIds = getSlideMutationIds(id, leftNeighborId, rightNeighborId) + if (mutationIds.length === 0 || !canMutateTimelineItems(mutationIds)) return execute( 'SLIDE_EDIT', diff --git a/src/features/timeline/stores/actions/item-actions.ts b/src/features/timeline/stores/actions/item-actions.ts index 1c1a28729..ae4fa6b5c 100644 --- a/src/features/timeline/stores/actions/item-actions.ts +++ b/src/features/timeline/stores/actions/item-actions.ts @@ -71,6 +71,9 @@ const LOCK_PROTECTED_ITEM_FIELDS = new Set([ 'offset', 'isReversed', 'reverseConformLocalStart', + 'reversed', + 'segmentStart', + 'segmentEnd', ]) function isLinkedSelectionEnabled(): boolean { @@ -85,6 +88,12 @@ function changesLockedItemPlacement(item: TimelineItem, updates: Partial): boolean { + const updateRecord = updates as Record + const itemRecord = item as unknown as Record + return Object.keys(updateRecord).some((key) => updateRecord[key] !== itemRecord[key]) +} + function areItemMutationsUnlocked(itemIds: Iterable): boolean { const { items, tracks } = useItemsStore.getState() return partitionItemMutationIdsByLock({ items, tracks, itemIds }).blockedIds.length === 0 @@ -769,6 +778,7 @@ export function addItemsOnNewTracks(items: TimelineItem[], tracks: TimelineTrack export function updateItem(id: string, updates: Partial): void { const item = useItemsStore.getState().itemById[id] if (!item) return + if (!changesItem(item, updates)) return if (changesLockedItemPlacement(item, updates) && !areItemMutationsUnlocked([id])) return if ( updates.trackId && diff --git a/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts b/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts new file mode 100644 index 000000000..383764c72 --- /dev/null +++ b/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts @@ -0,0 +1,336 @@ +// @vitest-environment node + +import { beforeEach, describe, expect, it } from 'vite-plus/test' +import type { LottieItem, TextItem, TimelineItem, TimelineTrack } from '@/types/timeline' +import type { Transition } from '@/types/transition' +import { useEditorStore } from '@/shared/state/editor' +import { makeTimelineAudioItem, makeTimelineTrack, makeTimelineVideoItem } from '../../test-helpers' +import { useItemsStore } from '../items-store' +import { useKeyframesStore } from '../keyframes-store' +import { useTimelineCommandStore } from '../timeline-command-store' +import { useTimelineSettingsStore } from '../timeline-settings-store' +import { useTransitionsStore } from '../transitions-store' +import { + insertFreezeFrame, + joinItems, + rateStretchItem, + rateStretchItemWithoutHistory, + removeSilenceFromItems, + resetSpeedWithRipple, + rippleTrimItem, + rollingTrimItems, + slideItem, + slipItem, + splitAllItemsAtFrame, + splitItem, + splitItemAtFrames, + trimItemBreakingTransition, + trimItemEnd, + trimItemStart, +} from './item-edit-actions' +import { updateItem } from './item-actions' + +function tracks(overrides: Partial = {}): TimelineTrack[] { + return [ + makeTimelineTrack({ + id: 'video-track', + name: 'V1', + kind: 'video', + order: 0, + ...overrides, + }), + makeTimelineTrack({ id: 'audio-track', name: 'A1', kind: 'audio', order: 1 }), + makeTimelineTrack({ id: 'caption-track', name: 'Captions', order: 2 }), + ] +} + +function video(overrides: Partial> = {}) { + return makeTimelineVideoItem({ + id: 'middle', + trackId: 'video-track', + from: 60, + durationInFrames: 60, + sourceStart: 30, + sourceEnd: 90, + sourceDuration: 180, + ...overrides, + }) +} + +function transition(): Transition { + return { + id: 'transition-1', + type: 'crossfade', + presentation: 'fade', + timing: 'linear', + leftClipId: 'left', + rightClipId: 'middle', + trackId: 'video-track', + durationInFrames: 10, + } +} + +function snapshot() { + return { + items: structuredClone(useItemsStore.getState().items), + tracks: structuredClone(useItemsStore.getState().tracks), + transitions: structuredClone(useTransitionsStore.getState().transitions), + keyframes: structuredClone(useKeyframesStore.getState().keyframes), + undoDepth: useTimelineCommandStore.getState().undoStack.length, + redoDepth: useTimelineCommandStore.getState().redoStack.length, + dirty: useTimelineSettingsStore.getState().isDirty, + } +} + +function expectUnchanged(before: ReturnType): void { + expect(useItemsStore.getState().items).toEqual(before.items) + expect(useItemsStore.getState().tracks).toEqual(before.tracks) + expect(useTransitionsStore.getState().transitions).toEqual(before.transitions) + expect(useKeyframesStore.getState().keyframes).toEqual(before.keyframes) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(before.undoDepth) + expect(useTimelineCommandStore.getState().redoStack).toHaveLength(before.redoDepth) + expect(useTimelineSettingsStore.getState().isDirty).toBe(before.dirty) +} + +describe('public item edit lock preflights', () => { + beforeEach(() => { + useEditorStore.setState({ linkedSelectionEnabled: true }) + useItemsStore.getState().setTracks(tracks()) + useItemsStore.getState().setItems([]) + useTransitionsStore.getState().setTransitions([]) + useKeyframesStore.getState().setKeyframes([]) + useTimelineCommandStore.getState().clearHistory() + useTimelineSettingsStore.setState({ fps: 30, isDirty: false }) + }) + + it.each([ + ['normal trim start', () => trimItemStart('middle', 10)], + ['normal trim end', () => trimItemEnd('middle', -10)], + ['ripple trim', () => rippleTrimItem('middle', 'end', -10)], + ['rolling trim', () => rollingTrimItems('left', 'middle', 10)], + ['slip', () => slipItem('middle', 10)], + ['slide', () => slideItem('middle', 10, 'left', 'right')], + ])('rejects %s before any item, transition, dirty, or history change', (_name, action) => { + useItemsStore.getState().setTracks(tracks({ locked: true })) + useItemsStore + .getState() + .setItems([ + video({ id: 'left', from: 0, sourceStart: 0, sourceEnd: 60 }), + video(), + video({ id: 'right', from: 120, sourceStart: 60, sourceEnd: 120 }), + ]) + useTransitionsStore.getState().setTransitions([transition()]) + const before = snapshot() + + action() + + expectUnchanged(before) + }) + + it('preflights a transition-breaking trim before removing the transition', () => { + useItemsStore.getState().setTracks(tracks({ locked: true })) + useItemsStore + .getState() + .setItems([video({ id: 'left', from: 0, sourceStart: 0, sourceEnd: 60 }), video()]) + useTransitionsStore.getState().setTransitions([transition()]) + const before = snapshot() + + trimItemBreakingTransition('middle', 'start', 10, ['transition-1']) + + expectUnchanged(before) + }) + + it.each([true, false])( + 'rejects the live-QA linked A/V trim and split when linked selection is %s', + (linkedSelectionEnabled) => { + useEditorStore.setState({ linkedSelectionEnabled }) + useItemsStore.getState().setTracks([ + tracks()[0]!, + makeTimelineTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }), + ]) + useItemsStore.getState().setItems([ + video({ linkedGroupId: 'linked-av' }), + makeTimelineAudioItem({ + id: 'audio', + trackId: 'audio-track', + from: 60, + durationInFrames: 60, + sourceStart: 30, + sourceEnd: 90, + sourceDuration: 180, + linkedGroupId: 'linked-av', + }), + ]) + const before = snapshot() + + trimItemEnd('middle', -10) + expect(splitItem('middle', 90)).toBeNull() + + expectUnchanged(before) + }, + ) + + it('rejects every split entry point and join on an effectively locked group child', () => { + const group = makeTimelineTrack({ + id: 'group', + name: 'Locked Group', + order: 0, + isGroup: true, + locked: true, + }) + const child = makeTimelineTrack({ + id: 'video-track', + name: 'Layer', + order: 1, + kind: 'video', + parentTrackId: group.id, + }) + useItemsStore.getState().setTracks([group, child]) + useItemsStore + .getState() + .setItems([ + video({ id: 'left', from: 0, durationInFrames: 60 }), + video({ id: 'right', from: 60, durationInFrames: 60 }), + ]) + const before = snapshot() + + expect(splitItem('left', 30)).toBeNull() + expect(splitAllItemsAtFrame(30)).toBe(0) + expect(splitItemAtFrames('left', [20, 40])).toBe(0) + joinItems(['left', 'right']) + + expectUnchanged(before) + }) + + it('rejects rate stretch, reset-speed ripple, and freeze-frame insertion atomically', async () => { + useItemsStore.getState().setTracks(tracks({ locked: true })) + useItemsStore.getState().setItems([video({ speed: 2 }), video({ id: 'right', from: 120 })]) + const before = snapshot() + + rateStretchItem('middle', 60, 90, 1) + rateStretchItemWithoutHistory('middle', 60, 90, 1) + resetSpeedWithRipple(['middle']) + await expect(insertFreezeFrame('middle', 90)).resolves.toBe(false) + + expectUnchanged(before) + }) + + it('rejects range removal before its first split when a linked companion is locked', () => { + useEditorStore.setState({ linkedSelectionEnabled: false }) + useItemsStore.getState().setTracks([ + tracks()[0]!, + makeTimelineTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }), + ]) + useItemsStore.getState().setItems([ + video({ id: 'video', from: 0, linkedGroupId: 'linked-av' }), + makeTimelineAudioItem({ + id: 'audio', + trackId: 'audio-track', + linkedGroupId: 'linked-av', + sourceStart: 30, + sourceEnd: 90, + sourceDuration: 180, + }), + ]) + const before = snapshot() + + const result = removeSilenceFromItems(['video'], { + 'media-1': [{ start: 1.5, end: 2 }], + }) + + expect(result).toMatchObject({ removedItemCount: 0, splitCount: 0 }) + expectUnchanged(before) + }) + + it('rejects normal trim when attached caption repair would mutate a locked caption', () => { + const caption: TextItem = { + id: 'caption', + type: 'text', + trackId: 'caption-track', + from: 100, + durationInFrames: 30, + label: 'Caption', + text: 'Caption', + color: '#fff', + textRole: 'caption', + captionSource: { type: 'transcript', clipId: 'middle', mediaId: 'media-1' }, + } + useItemsStore.getState().setTracks([ + tracks()[0]!, + makeTimelineTrack({ + id: 'caption-track', + name: 'Captions', + order: 1, + locked: true, + }), + ]) + useItemsStore.getState().setItems([video(), caption]) + const before = snapshot() + + trimItemEnd('middle', -30) + + expectUnchanged(before) + }) + + it.each([ + ['reversed', { reversed: true }], + ['segmentStart', { segmentStart: 12 }], + ['segmentEnd', { segmentEnd: 48 }], + ] as const)('protects the Lottie %s field on locked tracks', (_field, updates) => { + const lottie: LottieItem = { + id: 'lottie', + type: 'lottie', + trackId: 'video-track', + from: 0, + durationInFrames: 60, + label: 'Animation', + src: 'blob:lottie', + frameRate: 30, + totalFrames: 60, + } + useItemsStore.getState().setTracks(tracks({ locked: true })) + useItemsStore.getState().setItems([lottie]) + const before = snapshot() + + updateItem('lottie', updates) + + expectUnchanged(before) + }) + + it('allows Lottie timing controls on an unlocked standalone item', () => { + const lottie: LottieItem = { + id: 'lottie', + type: 'lottie', + trackId: 'video-track', + from: 0, + durationInFrames: 60, + label: 'Animation', + src: 'blob:lottie', + frameRate: 30, + totalFrames: 60, + } + useItemsStore.getState().setItems([lottie]) + + updateItem('lottie', { reversed: true, segmentStart: 12, segmentEnd: 48 }) + + expect(useItemsStore.getState().itemById.lottie).toMatchObject({ + reversed: true, + segmentStart: 12, + segmentEnd: 48, + }) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + expect(useTimelineSettingsStore.getState().isDirty).toBe(true) + }) +}) diff --git a/src/features/timeline/stores/actions/source-edit-actions.test.ts b/src/features/timeline/stores/actions/source-edit-actions.test.ts index 80eabd236..d4b331e64 100644 --- a/src/features/timeline/stores/actions/source-edit-actions.test.ts +++ b/src/features/timeline/stores/actions/source-edit-actions.test.ts @@ -5,6 +5,7 @@ import type { TimelineItem } from '@/types/timeline' const mocks = vi.hoisted(() => ({ mediaById: {} as Record, + resolveMediaUrl: async (): Promise => 'blob:source-media', })) vi.mock('@/features/timeline/deps/media-library-store', () => ({ @@ -38,7 +39,7 @@ vi.mock('@/features/timeline/deps/media-library-resolver', () => ({ : mimeType.startsWith('image') ? 'image' : 'unknown', - resolveMediaUrl: async () => 'blob:source-media', + resolveMediaUrl: () => mocks.resolveMediaUrl(), })) import { makeTimelineTrack, makeTimelineVideoItem } from '../../test-helpers' @@ -98,6 +99,7 @@ describe('source edit actions', () => { }) useSourcePlayerStore.setState({ inPoint: 30, outPoint: 90 }) resetPlaybackPreviewState(0) + mocks.resolveMediaUrl = async () => 'blob:source-media' setSourceMedia() }) @@ -198,6 +200,68 @@ describe('source edit actions', () => { expect(audioItems[0]?.linkedGroupId).toBe(videoItems[0]?.linkedGroupId) expect(audioItems[0]).toMatchObject({ from: 0, durationInFrames: 60 }) }) + + it('treats an inherited group lock as a locked target lane', async () => { + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'group', + name: 'Locked Group', + order: 0, + isGroup: true, + locked: true, + }), + makeTimelineTrack({ + id: 'track-v1', + name: 'V1', + kind: 'video', + order: 1, + parentTrackId: 'group', + }), + ]) + usePlaybackStore.setState({ currentFrame: 0 }) + + await performInsertEdit() + + expect(trackItems('track-v1')).toHaveLength(0) + const inserted = useItemsStore.getState().items + expect(inserted).toHaveLength(1) + expect(inserted[0]?.trackId).not.toBe('track-v1') + }) + + it('revalidates target locks immediately before the async commit', async () => { + let releaseUrl!: (url: string) => void + let reportStarted!: () => void + const started = new Promise((resolve) => { + reportStarted = resolve + }) + mocks.resolveMediaUrl = () => + new Promise((resolve) => { + releaseUrl = resolve + reportStarted() + }) + usePlaybackStore.setState({ currentFrame: 0 }) + + const pendingEdit = performInsertEdit() + await started + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'track-v1', + name: 'V1', + kind: 'video', + order: 0, + locked: true, + }), + makeTimelineTrack({ id: 'track-a1', name: 'A1', kind: 'audio', order: 1 }), + ]) + releaseUrl('blob:source-media') + await pendingEdit + + expect(useItemsStore.getState().items).toHaveLength(0) + expect(useItemsStore.getState().tracks[0]?.locked).toBe(true) + expect(usePlaybackStore.getState().currentFrame).toBe(0) + expect(useTimelineSettingsStore.getState().isDirty).toBe(false) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(0) + }) }) describe('performOverwriteEdit', () => { @@ -256,4 +320,57 @@ describe('source edit actions', () => { expect(items[1]).toMatchObject({ from: 40, durationInFrames: 60, mediaId: 'media-1' }) }) }) + + it.each([ + ['insert', performInsertEdit], + ['overwrite', performOverwriteEdit], + ])( + 'rejects %s atomically when an overlapping target clip has a locked linked companion', + async (_name, action) => { + for (const linkedSelectionEnabled of [true, false]) { + useTimelineCommandStore.getState().clearHistory() + useTimelineSettingsStore.setState({ isDirty: false }) + useEditorStore.setState({ linkedSelectionEnabled }) + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'track-v1', name: 'V1', kind: 'video', order: 0 }), + makeTimelineTrack({ + id: 'track-a1', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }), + ]) + const linkedVideo = makeTimelineVideoItem({ + id: 'existing-video', + trackId: 'track-v1', + from: 0, + durationInFrames: 120, + sourceEnd: 120, + sourceDuration: 120, + linkedGroupId: 'linked-av', + }) + const linkedAudio: TimelineItem = { + ...linkedVideo, + id: 'existing-audio', + type: 'audio', + trackId: 'track-a1', + src: 'blob:audio', + } + useItemsStore.getState().setItems([linkedVideo, linkedAudio]) + usePlaybackStore.setState({ currentFrame: 30 }) + const itemsBefore = structuredClone(useItemsStore.getState().items) + const transitionsBefore = structuredClone(useTransitionsStore.getState().transitions) + const playheadBefore = usePlaybackStore.getState().currentFrame + + await action() + + expect(useItemsStore.getState().items).toEqual(itemsBefore) + expect(useTransitionsStore.getState().transitions).toEqual(transitionsBefore) + expect(usePlaybackStore.getState().currentFrame).toBe(playheadBefore) + expect(useTimelineSettingsStore.getState().isDirty).toBe(false) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(0) + } + }, + ) }) diff --git a/src/features/timeline/stores/actions/source-edit-actions.ts b/src/features/timeline/stores/actions/source-edit-actions.ts index a4642f22c..589dd4eb7 100644 --- a/src/features/timeline/stores/actions/source-edit-actions.ts +++ b/src/features/timeline/stores/actions/source-edit-actions.ts @@ -19,6 +19,7 @@ import { resolveSourceEditTrackTargets } from '../../utils/source-edit-targeting import { buildMediaTimelineItems } from '../../utils/media-timeline-item-builder' import { DEFAULT_TRACK_HEIGHT } from '../../constants' import { DEFAULT_PROJECT_HEIGHT, DEFAULT_PROJECT_WIDTH } from '@/shared/projects/defaults' +import { isTimelineTrackLocked, preflightTimelineMutation } from '../../utils/track-lock-invariants' interface SourceEditContext { sourceMediaId: string @@ -138,7 +139,9 @@ async function resolveSourceEditContext(): Promise { (trackId): trackId is string => !!trackId, ) const lockedTarget = resolvedTargets.tracks.find( - (timelineTrack) => targetTrackIds.includes(timelineTrack.id) && timelineTrack.locked, + (timelineTrack) => + targetTrackIds.includes(timelineTrack.id) && + isTimelineTrackLocked(resolvedTargets.tracks, timelineTrack.id), ) if (lockedTarget) { toast.warning(`Target track ${lockedTarget.name} is locked`) @@ -230,6 +233,40 @@ function createTimelineItems(ctx: SourceEditContext) { }) } +function getSourceEditPreflightTracks(resolvedTracks: TimelineTrack[]): TimelineTrack[] { + const currentTracks = useItemsStore.getState().tracks + const currentTrackIds = new Set(currentTracks.map((track) => track.id)) + return [...currentTracks, ...resolvedTracks.filter((track) => !currentTrackIds.has(track.id))] +} + +function canCommitSourceEdit(params: { + mode: 'insert' | 'overwrite' + targetTrackIds: string[] + resolvedTracks: TimelineTrack[] + start: number + end: number +}): boolean { + const { items } = useItemsStore.getState() + const targetTrackIdSet = new Set(params.targetTrackIds) + const mutationIds = items + .filter((item) => { + if (!targetTrackIdSet.has(item.trackId)) return false + const itemEnd = item.from + item.durationInFrames + return params.mode === 'insert' + ? (item.from < params.start && itemEnd > params.start) || item.from >= params.start + : item.from < params.end && itemEnd > params.start + }) + .map((item) => item.id) + const tracks = getSourceEditPreflightTracks(params.resolvedTracks) + + return preflightTimelineMutation({ + items, + tracks, + itemIds: mutationIds, + destinationTrackIds: params.targetTrackIds, + }).allowed +} + export async function performInsertEdit(): Promise { const ctx = await resolveSourceEditContext() if (!ctx) return @@ -241,6 +278,17 @@ export async function performInsertEdit(): Promise { toast.warning('Unable to resolve source patch targets') return } + if ( + !canCommitSourceEdit({ + mode: 'insert', + targetTrackIds, + resolvedTracks: ctx.resolvedTracks, + start: insertFrame, + end: insertFrame, + }) + ) { + return + } execute( 'INSERT_EDIT', @@ -306,6 +354,17 @@ export async function performOverwriteEdit(): Promise { toast.warning('Unable to resolve source patch targets') return } + if ( + !canCommitSourceEdit({ + mode: 'overwrite', + targetTrackIds, + resolvedTracks: ctx.resolvedTracks, + start: overwriteStart, + end: overwriteEnd, + }) + ) { + return + } execute( 'OVERWRITE_EDIT', diff --git a/src/features/timeline/utils/source-edit-targeting.ts b/src/features/timeline/utils/source-edit-targeting.ts index 6df59f51e..38c569385 100644 --- a/src/features/timeline/utils/source-edit-targeting.ts +++ b/src/features/timeline/utils/source-edit-targeting.ts @@ -6,6 +6,7 @@ import { renameTrackForKind, type TrackKind, } from './classic-tracks' +import { isTimelineTrackLocked } from './track-lock-invariants' interface EnsureTrackForKindParams { tracks: TimelineTrack[] @@ -28,7 +29,12 @@ function findFirstUnlockedTrackByKind( ): TimelineTrack | null { return ( [...tracks] - .filter((track) => !track.locked && !track.isGroup && getTrackKind(track) === kind) + .filter( + (track) => + !track.isGroup && + !isTimelineTrackLocked(tracks, track.id) && + getTrackKind(track) === kind, + ) .sort((a, b) => a.order - b.order)[0] ?? null ) } @@ -39,11 +45,15 @@ function findUnlockedTrackById( ): TimelineTrack | null { if (!trackId) return null const track = tracks.find((candidate) => candidate.id === trackId) - return track && !track.locked && !track.isGroup ? track : null + return track && !track.isGroup && !isTimelineTrackLocked(tracks, track.id) ? track : null } -function canUseTrackForKind(track: TimelineTrack | null, kind: TrackKind): track is TimelineTrack { - if (!track || track.locked || track.isGroup) { +function canUseTrackForKind( + tracks: TimelineTrack[], + track: TimelineTrack | null, + kind: TrackKind, +): track is TimelineTrack { + if (!track || track.isGroup || isTimelineTrackLocked(tracks, track.id)) { return false } @@ -93,7 +103,7 @@ function resolveTargetTrackForKind(params: { } = params const preferredTrack = findUnlockedTrackById(tracks, preferredTrackId) - if (canUseTrackForKind(preferredTrack, kind)) { + if (canUseTrackForKind(tracks, preferredTrack, kind)) { return ensureTrackForKind({ tracks, targetTrack: preferredTrack, @@ -104,7 +114,7 @@ function resolveTargetTrackForKind(params: { }) } - if (canUseTrackForKind(fallbackTrack, kind)) { + if (canUseTrackForKind(tracks, fallbackTrack, kind)) { return ensureTrackForKind({ tracks, targetTrack: fallbackTrack, @@ -145,7 +155,10 @@ function findNearestUnlockedTrackByKind( direction: 'above' | 'below', ): TimelineTrack | null { const candidates = tracks - .filter((track) => !track.locked && !track.isGroup && getTrackKind(track) === kind) + .filter( + (track) => + !track.isGroup && !isTimelineTrackLocked(tracks, track.id) && getTrackKind(track) === kind, + ) .filter((track) => direction === 'above' ? track.order < targetTrack.order : track.order > targetTrack.order, ) @@ -167,7 +180,7 @@ function ensureTrackForKind(params: EnsureTrackForKindParams): { preferTarget = false, } = params - if (targetTrack.locked) { + if (isTimelineTrackLocked(tracks, targetTrack.id)) { const existingTrack = findNearestUnlockedTrackByKind( tracks, targetTrack, diff --git a/src/features/timeline/utils/track-lock-invariants.ts b/src/features/timeline/utils/track-lock-invariants.ts index 434c7e7f5..b62355308 100644 --- a/src/features/timeline/utils/track-lock-invariants.ts +++ b/src/features/timeline/utils/track-lock-invariants.ts @@ -8,6 +8,11 @@ export interface ItemMutationLockPartition { blockedByLockedLinkedCohort: boolean } +export interface TimelineMutationPreflight extends ItemMutationLockPartition { + allowed: boolean + lockedDestinationTrackIds: string[] +} + function getLockedTrackIds(tracks: TimelineTrack[]): Set { const lockedTrackIds = new Set( resolveEffectiveTrackStates(tracks) @@ -72,3 +77,32 @@ export function partitionItemMutationIdsByLock(params: { blockedByLockedLinkedCohort, } } + +/** + * Validate an entire public-action mutation cohort before its first write. + * + * Callers must provide every existing item whose timing, source window, or + * existence the action can change. Linked cohorts are intentionally checked + * independent of the linked-selection preference: opting out of synchronized + * selection must never let an unlocked member peel away from a locked one. + * Destination lanes are checked separately so cross-track moves and source + * edits cannot write into an effectively locked Layer Group child. + */ +export function preflightTimelineMutation(params: { + items: TimelineItem[] + tracks: TimelineTrack[] + itemIds: Iterable + destinationTrackIds?: Iterable +}): TimelineMutationPreflight { + const partition = partitionItemMutationIdsByLock(params) + const lockedTrackIds = getLockedTrackIds(params.tracks) + const lockedDestinationTrackIds = Array.from(new Set(params.destinationTrackIds ?? [])).filter( + (trackId) => lockedTrackIds.has(trackId), + ) + + return { + ...partition, + allowed: partition.blockedIds.length === 0 && lockedDestinationTrackIds.length === 0, + lockedDestinationTrackIds, + } +} From 722cac7add3624c274d7cf360efbce7eb7027479 Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 20:47:41 -0700 Subject: [PATCH 3/5] fix(timeline): close remaining lock invariant gaps --- .../actions/edit/freeze-frame-actions.test.ts | 387 ++++++++++++++ .../actions/edit/freeze-frame-actions.ts | 479 +++++++++++------- .../stores/actions/edit/trim-actions.ts | 124 ++++- .../item-actions.lock-invariants.test.ts | 161 +++++- .../timeline/stores/actions/item-actions.ts | 17 +- .../item-edit-actions.lock-invariants.test.ts | 52 +- .../actions/source-edit-actions.test.ts | 232 +++++++-- .../stores/actions/source-edit-actions.ts | 416 +++++++++++---- .../timeline/utils/group-utils.test.ts | 102 ++++ src/features/timeline/utils/group-utils.ts | 124 ++++- 10 files changed, 1710 insertions(+), 384 deletions(-) create mode 100644 src/features/timeline/stores/actions/edit/freeze-frame-actions.test.ts diff --git a/src/features/timeline/stores/actions/edit/freeze-frame-actions.test.ts b/src/features/timeline/stores/actions/edit/freeze-frame-actions.test.ts new file mode 100644 index 000000000..a91d66c18 --- /dev/null +++ b/src/features/timeline/stores/actions/edit/freeze-frame-actions.test.ts @@ -0,0 +1,387 @@ +// @vitest-environment node + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' +import type { AudioItem, TimelineItem, TimelineTrack } from '@/types/timeline' + +const mocks = vi.hoisted(() => ({ + acquire: vi.fn<(mediaId: string, blob: Blob) => string>(), + release: vi.fn<(mediaId: string) => void>(), + getMediaFile: vi.fn<(mediaId: string) => Promise>(), + importGeneratedImage: vi.fn(), + deleteMediaFromProject: vi.fn<(projectId: string, mediaId: string) => Promise>(), + prependMediaItem: vi.fn(), + getPrimaryVideoTrack: vi.fn(), + getCanvas: vi.fn(), + disposeInput: vi.fn(), + disposeSink: vi.fn(), + mediaItems: [] as Array>, + mediaState: { + currentProjectId: 'project-1' as string | null, + mediaById: {} as Record>, + prependMediaItem: (media: Record) => { + mocks.prependMediaItem(media) + mocks.mediaItems.unshift(media) + }, + }, +})) + +vi.mock('@/features/timeline/deps/media-library-store', () => ({ + useMediaLibraryStore: { + getState: () => mocks.mediaState, + }, +})) + +vi.mock('@/features/timeline/deps/media-library-service', () => ({ + importMediaLibraryService: async () => ({ + mediaLibraryService: { + getMediaFile: mocks.getMediaFile, + importGeneratedImage: mocks.importGeneratedImage, + deleteMediaFromProject: mocks.deleteMediaFromProject, + }, + }), +})) + +vi.mock('@/infrastructure/browser/blob-url-manager', () => ({ + blobUrlManager: { + acquire: mocks.acquire, + release: mocks.release, + }, +})) + +vi.mock('mediabunny', () => { + class Input { + getPrimaryVideoTrack = mocks.getPrimaryVideoTrack + dispose = mocks.disposeInput + } + + class BlobSource {} + + class CanvasSink { + getCanvas = mocks.getCanvas + dispose = mocks.disposeSink + } + + return { Input, BlobSource, CanvasSink, ALL_FORMATS: [] } +}) + +import { useSelectionStore } from '@/shared/state/selection' +import { + makeTimelineAudioItem, + makeTimelineTrack, + makeTimelineVideoItem, +} from '../../../test-helpers' +import { useItemsStore } from '../../items-store' +import { useTimelineCommandStore } from '../../timeline-command-store' +import { useTimelineSettingsStore } from '../../timeline-settings-store' +import { useTransitionsStore } from '../../transitions-store' +import { insertFreezeFrame } from './freeze-frame-actions' + +const originalSplitItem = useItemsStore.getState()._splitItem +const originalAddItem = useItemsStore.getState()._addItem + +const generatedMedia = { + id: 'freeze-media', + fileName: 'freeze.png', + mimeType: 'image/png', + duration: 0, + createdAt: 1, + updatedAt: 1, +} + +function videoTrack(overrides: Partial = {}): TimelineTrack { + return makeTimelineTrack({ + id: 'video-track', + name: 'V1', + kind: 'video', + order: 0, + ...overrides, + }) +} + +function video(overrides: Partial> = {}) { + return makeTimelineVideoItem({ + id: 'video', + trackId: 'video-track', + from: 0, + durationInFrames: 120, + sourceStart: 0, + sourceEnd: 120, + sourceDuration: 120, + sourceFps: 30, + mediaId: 'media-1', + ...overrides, + }) +} + +function snapshot() { + return { + items: structuredClone(useItemsStore.getState().items), + tracks: structuredClone(useItemsStore.getState().tracks), + transitions: structuredClone(useTransitionsStore.getState().transitions), + selection: structuredClone(useSelectionStore.getState().selectedItemIds), + dirty: useTimelineSettingsStore.getState().isDirty, + undoDepth: useTimelineCommandStore.getState().undoStack.length, + redoDepth: useTimelineCommandStore.getState().redoStack.length, + mediaItems: structuredClone(mocks.mediaItems), + } +} + +function expectSnapshot(expected: ReturnType): void { + expect(useItemsStore.getState().items).toEqual(expected.items) + expect(useItemsStore.getState().tracks).toEqual(expected.tracks) + expect(useTransitionsStore.getState().transitions).toEqual(expected.transitions) + expect(useSelectionStore.getState().selectedItemIds).toEqual(expected.selection) + expect(useTimelineSettingsStore.getState().isDirty).toBe(expected.dirty) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(expected.undoDepth) + expect(useTimelineCommandStore.getState().redoStack).toHaveLength(expected.redoDepth) + expect(mocks.mediaItems).toEqual(expected.mediaItems) +} + +function deferGeneratedImageImport() { + let release!: (media: typeof generatedMedia) => void + let reportStarted!: () => void + const started = new Promise((resolve) => { + reportStarted = resolve + }) + mocks.importGeneratedImage.mockImplementation( + () => + new Promise((resolve) => { + release = resolve + reportStarted() + }), + ) + return { started, release: () => release(generatedMedia) } +} + +describe('freeze-frame async atomicity', () => { + beforeEach(() => { + vi.clearAllMocks() + useItemsStore.setState({ _splitItem: originalSplitItem, _addItem: originalAddItem }) + useItemsStore.getState().setTracks([videoTrack()]) + useItemsStore.getState().setItems([video()]) + useTransitionsStore.getState().setTransitions([]) + useSelectionStore.getState().clearSelection() + useSelectionStore.getState().selectItems(['sentinel-selection']) + useTimelineCommandStore.getState().clearHistory() + useTimelineSettingsStore.setState({ fps: 30, isDirty: false }) + + mocks.mediaItems = [{ id: 'media-1', fileName: 'source.mp4' }] + mocks.mediaState.currentProjectId = 'project-1' + mocks.mediaState.mediaById = { + 'media-1': { + id: 'media-1', + fileName: 'source.mp4', + mimeType: 'video/mp4', + duration: 4, + fps: 30, + }, + } + mocks.getMediaFile.mockResolvedValue(new Blob(['video'], { type: 'video/mp4' })) + mocks.getPrimaryVideoTrack.mockResolvedValue({ displayWidth: 1920, displayHeight: 1080 }) + mocks.getCanvas.mockResolvedValue({ + canvas: { + convertToBlob: async () => new Blob(['frame'], { type: 'image/png' }), + }, + }) + mocks.importGeneratedImage.mockResolvedValue(generatedMedia) + mocks.deleteMediaFromProject.mockResolvedValue(undefined) + mocks.acquire.mockReturnValue('blob:freeze-media') + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('keeps a successful output and undoes the timeline mutation in one step', async () => { + await expect(insertFreezeFrame('video', 60)).resolves.toBe(true) + + expect(mocks.deleteMediaFromProject).not.toHaveBeenCalled() + expect(mocks.release).not.toHaveBeenCalled() + expect(mocks.prependMediaItem).toHaveBeenCalledWith(generatedMedia) + expect(useItemsStore.getState().items).toHaveLength(3) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + + useTimelineCommandStore.getState().undo() + expect(useItemsStore.getState().items).toEqual([video()]) + }) + + it('does not write or shift the old lane after the source item moves lanes', async () => { + useItemsStore + .getState() + .setTracks([videoTrack(), videoTrack({ id: 'video-track-2', name: 'V2', order: 1 })]) + useItemsStore.getState().setItems([video(), video({ id: 'old-lane-downstream', from: 120 })]) + const deferred = deferGeneratedImageImport() + const pending = insertFreezeFrame('video', 60) + await deferred.started + + useItemsStore.getState()._moveItem('video', 0, 'video-track-2') + useItemsStore + .getState() + .setTracks([ + videoTrack({ locked: true }), + videoTrack({ id: 'video-track-2', name: 'V2', order: 1 }), + ]) + const before = snapshot() + deferred.release() + + await expect(pending).resolves.toBe(false) + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + }) + + it('rejects a target track lock that appears while persistence is awaiting', async () => { + const deferred = deferGeneratedImageImport() + const pending = insertFreezeFrame('video', 60) + await deferred.started + + useItemsStore.getState().setTracks([videoTrack({ locked: true })]) + const before = snapshot() + deferred.release() + + await expect(pending).resolves.toBe(false) + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + }) + + it.each([ + ['deletion', () => useItemsStore.getState()._removeItems(['video'])], + ['source change', () => useItemsStore.getState()._updateItem('video', { sourceStart: 12 })], + ])( + 'rejects source item %s after persistence and cleans media/blob state', + async (_case, drift) => { + const deferred = deferGeneratedImageImport() + const pending = insertFreezeFrame('video', 60) + await deferred.started + + drift() + const before = snapshot() + deferred.release() + + await expect(pending).resolves.toBe(false) + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + expect(mocks.prependMediaItem).not.toHaveBeenCalled() + }, + ) + + it('rejects source media deletion after persistence and cleans media/blob state', async () => { + const deferred = deferGeneratedImageImport() + const pending = insertFreezeFrame('video', 60) + await deferred.started + + delete mocks.mediaState.mediaById['media-1'] + const before = snapshot() + deferred.release() + + await expect(pending).resolves.toBe(false) + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + }) + + it('rejects downstream lane cohort drift after persistence', async () => { + const deferred = deferGeneratedImageImport() + const pending = insertFreezeFrame('video', 60) + await deferred.started + + useItemsStore.getState()._addItem(video({ id: 'late-item', from: 120 })) + const before = snapshot() + deferred.release() + + await expect(pending).resolves.toBe(false) + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + }) + + it('rejects linked companion drift after persistence', async () => { + const audioTrack = makeTimelineTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + }) + const linkedVideo = video({ linkedGroupId: 'linked-av' }) + const linkedAudio: AudioItem = makeTimelineAudioItem({ + id: 'audio', + trackId: 'audio-track', + linkedGroupId: 'linked-av', + from: 0, + durationInFrames: 120, + sourceStart: 0, + sourceEnd: 120, + sourceDuration: 120, + mediaId: 'media-1', + }) + useItemsStore.getState().setTracks([videoTrack(), audioTrack]) + useItemsStore.getState().setItems([linkedVideo, linkedAudio]) + const deferred = deferGeneratedImageImport() + const pending = insertFreezeFrame('video', 60) + await deferred.started + + useItemsStore.getState()._moveItem('audio', 12) + const before = snapshot() + deferred.release() + + await expect(pending).resolves.toBe(false) + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + }) + + it('cleans persisted media when blob URL acquisition throws', async () => { + mocks.acquire.mockImplementation(() => { + throw new Error('blob acquisition failed') + }) + const before = snapshot() + + await expect(insertFreezeFrame('video', 60)).resolves.toBe(false) + + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + }) + + it.each([ + ['returns false', () => vi.spyOn(useItemsStore.getState(), '_splitItem').mockReturnValue(null)], + [ + 'throws', + () => + vi.spyOn(useItemsStore.getState(), '_splitItem').mockImplementation(() => { + throw new Error('split mutation failed') + }), + ], + [ + 'throws after splitting', + () => + vi.spyOn(useItemsStore.getState(), '_addItem').mockImplementation(() => { + throw new Error('add mutation failed') + }), + ], + ])('cleans persisted media when execute %s', async (_case, mockSplit) => { + mockSplit() + const before = snapshot() + + await expect(insertFreezeFrame('video', 60)).resolves.toBe(false) + + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + expect(mocks.prependMediaItem).not.toHaveBeenCalled() + }) + + it('does not remove successful persisted output when the media UI prepend throws', async () => { + mocks.prependMediaItem.mockImplementation(() => { + throw new Error('media UI refresh failed') + }) + + await expect(insertFreezeFrame('video', 60)).resolves.toBe(true) + + expect(useItemsStore.getState().items).toHaveLength(3) + expect(mocks.deleteMediaFromProject).not.toHaveBeenCalled() + expect(mocks.release).not.toHaveBeenCalled() + }) +}) diff --git a/src/features/timeline/stores/actions/edit/freeze-frame-actions.ts b/src/features/timeline/stores/actions/edit/freeze-frame-actions.ts index 01cc64671..30eee8fff 100644 --- a/src/features/timeline/stores/actions/edit/freeze-frame-actions.ts +++ b/src/features/timeline/stores/actions/edit/freeze-frame-actions.ts @@ -1,4 +1,4 @@ -import type { ImageItem } from '@/types/timeline' +import type { ImageItem, TimelineItem, TimelineTrack, VideoItem } from '@/types/timeline' import { useItemsStore } from '../../items-store' import { useTransitionsStore } from '../../transitions-store' import { useTimelineSettingsStore } from '../../timeline-settings-store' @@ -9,31 +9,133 @@ import { blobUrlManager } from '@/infrastructure/browser/blob-url-manager' import { execute, applyTransitionRepairs, getLogger } from '../shared' import { timelineToSourceFrames } from '../../../utils/source-calculations' import { canMutateTimelineItems, isInTransitionOverlap } from './shared' +import { captureSnapshot, restoreSnapshot } from '../../commands/snapshot' + +interface FreezeFramePlan { + item: VideoItem + fps: number + media: { id: string; fps?: number } + projectId: string + downstreamItemIds: string[] + fingerprint: string +} + +function isFreezeFramePositionValid(item: VideoItem, playheadFrame: number): boolean { + if (playheadFrame <= item.from || playheadFrame >= item.from + item.durationInFrames) return false + return !isInTransitionOverlap(item.id, playheadFrame - item.from, item.durationInFrames) +} -function canCommitFreezeFrame(itemId: string, playheadFrame: number): boolean { +function getFreezeFrameDownstreamItems( + items: TimelineItem[], + item: VideoItem, + playheadFrame: number, +): TimelineItem[] { + return items.filter( + (candidate) => + candidate.id !== item.id && + candidate.trackId === item.trackId && + candidate.from >= playheadFrame, + ) +} + +function getFreezeFrameParticipants(itemId: string, playheadFrame: number) { const store = useItemsStore.getState() const item = store.itemById[itemId] - if (!item || item.type !== 'video') return false - if ( - playheadFrame <= item.from || - playheadFrame >= item.from + item.durationInFrames || - isInTransitionOverlap(itemId, playheadFrame - item.from, item.durationInFrames) - ) { - return false + if (!item || item.type !== 'video') return null + if (!isFreezeFramePositionValid(item, playheadFrame)) return null + + const downstreamItems = getFreezeFrameDownstreamItems(store.items, item, playheadFrame) + const mutationIds = [itemId, ...downstreamItems.map((candidate) => candidate.id)] + if (!canMutateTimelineItems(mutationIds, [item.trackId])) return null + return { store, item, downstreamItems, mutationIds } +} + +function getFreezeFrameScopeItems(items: TimelineItem[], mutationIds: string[]): TimelineItem[] { + const mutationIdSet = new Set(mutationIds) + const linkedGroupIds = new Set( + items + .filter((candidate) => mutationIdSet.has(candidate.id)) + .map((candidate) => candidate.linkedGroupId) + .filter((groupId): groupId is string => !!groupId), + ) + return items + .filter( + (candidate) => + mutationIdSet.has(candidate.id) || + (!!candidate.linkedGroupId && linkedGroupIds.has(candidate.linkedGroupId)), + ) + .toSorted((left, right) => left.id.localeCompare(right.id)) +} + +function getRelevantTrackStates(tracks: TimelineTrack[], trackIds: Set) { + const trackById = new Map(tracks.map((track) => [track.id, track] as const)) + const relevantTrackIds = new Set() + + for (const trackId of trackIds) { + const visited = new Set() + let currentId: string | undefined = trackId + while (currentId && !visited.has(currentId)) { + visited.add(currentId) + relevantTrackIds.add(currentId) + currentId = trackById.get(currentId)?.parentTrackId + } + if (currentId) relevantTrackIds.add(`cycle:${currentId}`) } - const mutationIds = [ - itemId, - ...store.items - .filter( - (candidate) => - candidate.id !== itemId && - candidate.trackId === item.trackId && - candidate.from > playheadFrame, - ) - .map((candidate) => candidate.id), - ] - return canMutateTimelineItems(mutationIds, [item.trackId]) + return [...relevantTrackIds].sort().map((trackId) => { + const track = trackById.get(trackId) + return track + ? { + id: track.id, + parentTrackId: track.parentTrackId, + kind: track.kind, + isGroup: track.isGroup, + locked: track.locked, + order: track.order, + height: track.height, + } + : { id: trackId, missing: true } + }) +} + +function buildFreezeFramePlan(itemId: string, playheadFrame: number): FreezeFramePlan | null { + const participants = getFreezeFrameParticipants(itemId, playheadFrame) + if (!participants) return null + const { store, item, downstreamItems, mutationIds } = participants + + const media = item.mediaId ? useMediaLibraryStore.getState().mediaById[item.mediaId] : undefined + if (!media) return null + const projectId = useMediaLibraryStore.getState().currentProjectId + if (!projectId) return null + + const scopeItems = getFreezeFrameScopeItems(store.items, mutationIds) + const scopeItemIds = new Set(scopeItems.map((candidate) => candidate.id)) + const relevantTransitions = useTransitionsStore + .getState() + .transitions.filter( + (transition) => + scopeItemIds.has(transition.leftClipId) || scopeItemIds.has(transition.rightClipId), + ) + .toSorted((left, right) => left.id.localeCompare(right.id)) + const trackIds = new Set(scopeItems.map((candidate) => candidate.trackId)) + const fps = useTimelineSettingsStore.getState().fps + + return { + item, + fps, + media: { id: media.id, fps: media.fps }, + projectId, + downstreamItemIds: downstreamItems.map((candidate) => candidate.id).toSorted(), + fingerprint: JSON.stringify({ + item, + scopeItems, + tracks: getRelevantTrackStates(store.tracks, trackIds), + transitions: relevantTransitions, + fps, + media: { id: media.id, fps: media.fps }, + projectId, + }), + } } /** @@ -46,46 +148,37 @@ function canCommitFreezeFrame(itemId: string, playheadFrame: number): boolean { * mutations are batched in a single command for undo/redo atomicity. */ export async function insertFreezeFrame(itemId: string, playheadFrame: number): Promise { - const items = useItemsStore.getState().items - const item = items.find((i) => i.id === itemId) - if (!item || item.type !== 'video') return false + const initialPlan = buildFreezeFramePlan(itemId, playheadFrame) + if (!initialPlan) return false - // Validate playhead is within item bounds (exclusive of edges — need room to split) - const itemStart = item.from - const itemEnd = item.from + item.durationInFrames - if (playheadFrame <= itemStart || playheadFrame >= itemEnd) return false - - // Block freeze frame insertion inside transition overlap zones - if (isInTransitionOverlap(itemId, playheadFrame - itemStart, item.durationInFrames)) { - return false - } - if (!canCommitFreezeFrame(itemId, playheadFrame)) return false - - const fps = useTimelineSettingsStore.getState().fps + const { item, fps } = initialPlan const speed = item.speed ?? 1 const sourceStart = item.sourceStart ?? 0 const sourceFps = item.sourceFps ?? fps // Calculate source frame at playhead in source-native FPS - const timelineOffset = playheadFrame - itemStart + const timelineOffset = playheadFrame - item.from const sourceFrame = sourceStart + timelineToSourceFrames(timelineOffset, speed, fps, sourceFps) - // Get media metadata for resolution and fps info - const media = item.mediaId ? useMediaLibraryStore.getState().mediaById[item.mediaId] : undefined - if (!media) { - getLogger().error('[insertFreezeFrame] Media not found for item:', item.mediaId) - return false - } - // Calculate timestamp in seconds for frame extraction - const mediaFps = media.fps || 30 + const mediaFps = initialPlan.media.fps || 30 const timestampSeconds = sourceFrame / mediaFps + let persistedFrame: + | { + mediaLibraryService: Awaited< + ReturnType + >['mediaLibraryService'] + projectId: string + mediaId: string + } + | undefined + let keepPersistedFrame = false try { const { mediaLibraryService } = await importMediaLibraryService() // Step 1: Get the media file blob - const blob = await mediaLibraryService.getMediaFile(media.id) + const blob = await mediaLibraryService.getMediaFile(initialPlan.media.id) if (!blob) { getLogger().error('[insertFreezeFrame] Could not access media file') return false @@ -97,47 +190,51 @@ export async function insertFreezeFrame(itemId: string, playheadFrame: number): source: new BlobSource(blob as File), formats: ALL_FORMATS, }) + let sink: InstanceType | undefined + let frameBlob: Blob + let frameWidth: number + let frameHeight: number + try { + const videoTrack = await input.getPrimaryVideoTrack() + if (!videoTrack) { + getLogger().error('[insertFreezeFrame] No video track found') + return false + } - const videoTrack = await input.getPrimaryVideoTrack() - if (!videoTrack) { - input.dispose() - getLogger().error('[insertFreezeFrame] No video track found') - return false - } - - const frameWidth = videoTrack.displayWidth - const frameHeight = videoTrack.displayHeight + frameWidth = videoTrack.displayWidth + frameHeight = videoTrack.displayHeight + sink = new CanvasSink(videoTrack, { + width: frameWidth, + height: frameHeight, + fit: 'fill', + }) - const sink = new CanvasSink(videoTrack, { - width: frameWidth, - height: frameHeight, - fit: 'fill', - }) + const wrapped = await sink.getCanvas(timestampSeconds) + if (!wrapped) { + getLogger().error('[insertFreezeFrame] Failed to extract frame') + return false + } - const wrapped = await sink.getCanvas(timestampSeconds) - if (!wrapped) { - ;(sink as unknown as { dispose?: () => void }).dispose?.() + const canvas = wrapped.canvas as OffscreenCanvas | HTMLCanvasElement + if ('convertToBlob' in canvas) { + frameBlob = await canvas.convertToBlob({ type: 'image/png' }) + } else { + frameBlob = await new Promise((resolve, reject) => { + canvas.toBlob( + (result) => (result ? resolve(result) : reject(new Error('Failed to create blob'))), + 'image/png', + ) + }) + } + } finally { + ;(sink as unknown as { dispose?: () => void } | undefined)?.dispose?.() input.dispose() - getLogger().error('[insertFreezeFrame] Failed to extract frame') - return false - } - - const canvas = wrapped.canvas as OffscreenCanvas | HTMLCanvasElement - let frameBlob: Blob - if ('convertToBlob' in canvas) { - frameBlob = await canvas.convertToBlob({ type: 'image/png' }) - } else { - frameBlob = await new Promise((resolve, reject) => { - canvas.toBlob( - (b) => (b ? resolve(b) : reject(new Error('Failed to create blob'))), - 'image/png', - ) - }) } - // Clean up mediabunny resources - ;(sink as unknown as { dispose?: () => void }).dispose?.() - input.dispose() + // Avoid persistence if extraction awaited across any relevant source, + // lane, linked-cohort, transition, track ancestry, or lock drift. + const prePersistPlan = buildFreezeFramePlan(itemId, playheadFrame) + if (!prePersistPlan || prePersistPlan.fingerprint !== initialPlan.fingerprint) return false // Step 3: Persist the frame as a media item. Delegates to the shared // import path (mediaLibraryService -> persistGeneratedMediaAsset) which @@ -146,12 +243,6 @@ export async function insertFreezeFrame(itemId: string, playheadFrame: number): // if any step throws. Hand-rolling this here previously skipped the // rollback and had to be patched repeatedly (createMedia-before-thumbnailId, // store-prepend-before-execute). - const currentProjectId = useMediaLibraryStore.getState().currentProjectId - if (!currentProjectId) { - getLogger().error('[insertFreezeFrame] No project context') - return false - } - const fileName = `freeze-frame-${item.label || 'video'}-${Math.round(timestampSeconds * 100) / 100}s.png` const frameFile = new File([frameBlob], fileName, { type: 'image/png', @@ -160,7 +251,7 @@ export async function insertFreezeFrame(itemId: string, playheadFrame: number): const mediaMetadata = await mediaLibraryService.importGeneratedImage( frameFile, - currentProjectId, + initialPlan.projectId, { width: frameWidth, height: frameHeight, @@ -169,120 +260,148 @@ export async function insertFreezeFrame(itemId: string, playheadFrame: number): }, ) const frameMediaId = mediaMetadata.id - const frameBlobUrl = blobUrlManager.acquire(frameMediaId, frameBlob) - - const rollbackPersistedFrame = async (): Promise => { - try { - await mediaLibraryService.deleteMediaFromProject(currentProjectId, frameMediaId) - } catch (cleanupError) { - getLogger().warn( - '[insertFreezeFrame] Failed to roll back persisted frame after rejected commit', - cleanupError, - ) - } - blobUrlManager.release(frameMediaId) + persistedFrame = { + mediaLibraryService, + projectId: initialPlan.projectId, + mediaId: frameMediaId, } + const frameBlobUrl = blobUrlManager.acquire(frameMediaId, frameBlob) - // Locks can change while frame extraction and persistence are awaiting. - // Revalidate the complete split/shift cohort immediately before execute(). - if (!canCommitFreezeFrame(itemId, playheadFrame)) { - await rollbackPersistedFrame() - return false - } + // Rebuild the exact plan after the final await. Any relevant drift rejects + // the operation; the finally block owns all persisted-media cleanup. + const commitPlan = buildFreezeFramePlan(itemId, playheadFrame) + if (!commitPlan || commitPlan.fingerprint !== initialPlan.fingerprint) return false // Step 4: Perform timeline mutations atomically (split + insert + shift). // Prepend the media item to the store only after execute() succeeds so a // failed _splitItem (e.g. the source clip was removed between validation // and execute) doesn't leave an orphaned entry in the media library UI. - const freezeDurationFrames = Math.round(fps * 2) // 2 seconds - - const success = execute( - 'INSERT_FREEZE_FRAME', - (): boolean => { - // Split the video at playhead - const splitResult = useItemsStore.getState()._splitItem(itemId, playheadFrame) - if (!splitResult) { - getLogger().error('[insertFreezeFrame] Split failed') - return false - } - - const { leftItem, rightItem } = splitResult + const freezeDurationFrames = Math.round(commitPlan.fps * 2) // 2 seconds + const beforeSnapshot = captureSnapshot() + const selectionBefore = useSelectionStore.getState() + const dirtyBefore = useTimelineSettingsStore.getState().isDirty + let success: boolean + try { + success = execute( + 'INSERT_FREEZE_FRAME', + (): boolean => { + // Split the video at playhead + const splitResult = useItemsStore.getState()._splitItem(itemId, playheadFrame) + if (!splitResult) { + getLogger().error('[insertFreezeFrame] Split failed') + return false + } - // Update transitions pointing to split item - const transitions = useTransitionsStore.getState().transitions - const updatedTransitions = transitions.map((t) => { - if (t.leftClipId === itemId) { - return { ...t, leftClipId: rightItem.id } + const { leftItem, rightItem } = splitResult + + // Update transitions pointing to split item + const transitions = useTransitionsStore.getState().transitions + const updatedTransitions = transitions.map((transition) => { + if (transition.leftClipId === itemId) { + return { ...transition, leftClipId: rightItem.id } + } + return transition + }) + useTransitionsStore.getState().setTransitions(updatedTransitions) + + // Create ImageItem for the freeze frame + const freezeFrameItem: ImageItem = { + id: crypto.randomUUID(), + type: 'image', + trackId: commitPlan.item.trackId, + from: playheadFrame, + durationInFrames: freezeDurationFrames, + label: fileName, + mediaId: frameMediaId, + src: frameBlobUrl, + sourceWidth: frameWidth, + sourceHeight: frameHeight, + transform: commitPlan.item.transform ? { ...commitPlan.item.transform } : undefined, } - return t - }) - useTransitionsStore.getState().setTransitions(updatedTransitions) - - // Create ImageItem for the freeze frame - const freezeFrameItem: ImageItem = { - id: crypto.randomUUID(), - type: 'image', - trackId: item.trackId, - from: playheadFrame, - durationInFrames: freezeDurationFrames, - label: fileName, - mediaId: frameMediaId, - src: frameBlobUrl, - sourceWidth: frameWidth, - sourceHeight: frameHeight, - transform: item.transform ? { ...item.transform } : undefined, - } - useItemsStore.getState()._addItem(freezeFrameItem) - - // Shift the right half forward by freeze frame duration - const newRightFrom = rightItem.from + freezeDurationFrames - useItemsStore.getState()._moveItem(rightItem.id, newRightFrom) - - // Also shift all items on same track that come after the right half - const allItems = useItemsStore.getState().items - const itemsToShift = allItems.filter( - (i) => - i.trackId === item.trackId && - i.id !== rightItem.id && - i.id !== leftItem.id && - i.id !== freezeFrameItem.id && - i.from > playheadFrame, - ) + useItemsStore.getState()._addItem(freezeFrameItem) - for (const shiftItem of itemsToShift) { - useItemsStore.getState()._moveItem(shiftItem.id, shiftItem.from + freezeDurationFrames) - } + // Shift the right half forward by freeze frame duration + const newRightFrom = rightItem.from + freezeDurationFrames + useItemsStore.getState()._moveItem(rightItem.id, newRightFrom) + + // Shift only the exact downstream cohort that was fingerprinted and + // lock-preflighted immediately before execute(). + for (const downstreamItemId of commitPlan.downstreamItemIds) { + const downstreamItem = useItemsStore.getState().itemById[downstreamItemId] + if (!downstreamItem) throw new Error('Freeze-frame downstream item drifted') + useItemsStore + .getState() + ._moveItem(downstreamItem.id, downstreamItem.from + freezeDurationFrames) + } - // Repair transitions - applyTransitionRepairs([leftItem.id, rightItem.id]) + // Repair transitions + applyTransitionRepairs([leftItem.id, rightItem.id]) - // Select the freeze frame item - useSelectionStore.getState().selectItems([freezeFrameItem.id]) + // Select the freeze frame item + useSelectionStore.getState().selectItems([freezeFrameItem.id]) - useTimelineSettingsStore.getState().markDirty() - return true - }, - { itemId, playheadFrame, freezeDurationFrames }, - ) + useTimelineSettingsStore.getState().markDirty() + return true + }, + { itemId, playheadFrame, freezeDurationFrames }, + ) + } catch (error) { + restoreSnapshot(beforeSnapshot) + useSelectionStore.setState({ + selectedItemIds: selectionBefore.selectedItemIds, + selectedItemIdSet: new Set(selectionBefore.selectedItemIds), + selectedMarkerId: selectionBefore.selectedMarkerId, + selectedTransitionId: selectionBefore.selectedTransitionId, + selectedTrackId: selectionBefore.selectedTrackId, + selectedTrackIds: selectionBefore.selectedTrackIds, + activeTrackId: selectionBefore.activeTrackId, + selectionType: selectionBefore.selectionType, + expandedKeyframeLanes: selectionBefore.expandedKeyframeLanes, + }) + useTimelineSettingsStore.setState({ isDirty: dirtyBefore }) + throw error + } if (!success) { - // Roll back the persisted media so a failed split (rare — only if the - // source clip was deleted between validation and execute) doesn't leave - // an orphan on disk or a dangling blob URL in memory. - // deleteMediaFromProject is the right call here (not deleteMedia): the - // frame was just associated with currentProjectId and is referenced - // only by this project, so the reference-counted variant covers it - // and preserves the global "delete everywhere" semantics for the - // explicit user action. - await rollbackPersistedFrame() return false } - useMediaLibraryStore.getState().prependMediaItem(mediaMetadata) + keepPersistedFrame = true + try { + useMediaLibraryStore.getState().prependMediaItem(mediaMetadata) + } catch (error) { + // Timeline and persistence already succeeded. Keep the referenced media + // instead of deleting a successful output because a UI-store refresh + // failed; the persisted entry will be rediscovered on the next reload. + getLogger().warn('[insertFreezeFrame] Failed to prepend persisted media item', error) + } return true } catch (error) { getLogger().error('[insertFreezeFrame] Failed:', error) return false + } finally { + if (persistedFrame && !keepPersistedFrame) { + try { + await persistedFrame.mediaLibraryService.deleteMediaFromProject( + persistedFrame.projectId, + persistedFrame.mediaId, + ) + } catch (cleanupError) { + getLogger().warn( + '[insertFreezeFrame] Failed to roll back persisted frame after rejected commit', + cleanupError, + ) + } finally { + try { + blobUrlManager.release(persistedFrame.mediaId) + } catch (cleanupError) { + getLogger().warn( + '[insertFreezeFrame] Failed to release persisted frame URL', + cleanupError, + ) + } + } + } } } diff --git a/src/features/timeline/stores/actions/edit/trim-actions.ts b/src/features/timeline/stores/actions/edit/trim-actions.ts index 4410e8f21..2951217d6 100644 --- a/src/features/timeline/stores/actions/edit/trim-actions.ts +++ b/src/features/timeline/stores/actions/edit/trim-actions.ts @@ -82,6 +82,85 @@ function getSynchronizedTrimItems( return Array.from(synchronizedById.values()) } +function getClampedSynchronizedTrimAmount( + synchronizedItems: TimelineItem[], + items: TimelineItem[], + handle: 'start' | 'end', + trimAmount: number, +): number { + const timelineFps = useTimelineSettingsStore.getState().fps + let synchronizedTrimAmount = trimAmount + for (const synchronizedItem of synchronizedItems) { + const sourceClampedAmount = clampTrimAmount( + synchronizedItem, + handle, + synchronizedTrimAmount, + timelineFps, + ).clampedAmount + synchronizedTrimAmount = keepTightestDelta( + synchronizedTrimAmount, + clampToAdjacentItems( + synchronizedItem, + handle, + sourceClampedAmount, + items, + getTransitionLinkedIds(synchronizedItem.id), + ), + ) + } + return synchronizedTrimAmount +} + +function getAttachedCaptionTrimMutationIds( + items: TimelineItem[], + synchronizedItems: TimelineItem[], + handle: 'start' | 'end', + trimAmount: number, +): string[] { + const captionMutationIds = new Set() + const itemById = new Map(items.map((item) => [item.id, item] as const)) + + for (const clip of synchronizedItems) { + if (clip.type === 'text') continue + const finalBounds = getFinalTrimmedClipBounds(clip, handle, trimAmount) + + for (const captionId of getAttachedCaptionItemIds(items, clip.id)) { + const caption = itemById.get(captionId) + if (caption?.type !== 'text') continue + if (captionChangesWithinBounds(caption, finalBounds)) captionMutationIds.add(caption.id) + } + } + + return Array.from(captionMutationIds) +} + +function getFinalTrimmedClipBounds( + clip: TimelineItem, + handle: 'start' | 'end', + trimAmount: number, +): { start: number; end: number } { + return { + start: handle === 'start' ? clip.from + trimAmount : clip.from, + end: + handle === 'start' + ? clip.from + clip.durationInFrames + : clip.from + clip.durationInFrames + trimAmount, + } +} + +function captionChangesWithinBounds( + caption: TimelineItem, + bounds: { start: number; end: number }, +): boolean { + const finalStart = Math.max(caption.from, bounds.start) + const finalEnd = Math.min(caption.from + caption.durationInFrames, bounds.end) + return ( + finalEnd <= finalStart || + finalStart !== caption.from || + finalEnd - finalStart !== caption.durationInFrames + ) +} + function getSynchronizedTrimMutationIds( id: string, handle: 'start' | 'end', @@ -93,9 +172,24 @@ function getSynchronizedTrimMutationIds( if (!synchronizedItems.some((item) => item.id === id)) return [] const synchronizedIds = synchronizedItems.map((item) => item.id) - const shrinksVisibleBounds = handle === 'start' ? trimAmount > 0 : trimAmount < 0 + const synchronizedTrimAmount = getClampedSynchronizedTrimAmount( + synchronizedItems, + items, + handle, + trimAmount, + ) + const shrinksVisibleBounds = + handle === 'start' ? synchronizedTrimAmount > 0 : synchronizedTrimAmount < 0 return shrinksVisibleBounds - ? expandItemIdsWithAttachedCaptions(items, synchronizedIds) + ? [ + ...synchronizedIds, + ...getAttachedCaptionTrimMutationIds( + items, + synchronizedItems, + handle, + synchronizedTrimAmount, + ), + ] : synchronizedIds } @@ -441,26 +535,12 @@ function applySynchronizedTrim( const anchorBefore = synchronizedItems.find((item) => item.id === id) if (!anchorBefore) return - const timelineFps = useTimelineSettingsStore.getState().fps - let synchronizedTrimAmount = trimAmount - for (const synchronizedItem of synchronizedItems) { - const sourceClampedAmount = clampTrimAmount( - synchronizedItem, - handle, - synchronizedTrimAmount, - timelineFps, - ).clampedAmount - synchronizedTrimAmount = keepTightestDelta( - synchronizedTrimAmount, - clampToAdjacentItems( - synchronizedItem, - handle, - sourceClampedAmount, - itemsBefore, - getTransitionLinkedIds(synchronizedItem.id), - ), - ) - } + const synchronizedTrimAmount = getClampedSynchronizedTrimAmount( + synchronizedItems, + itemsBefore, + handle, + trimAmount, + ) if (handle === 'start') { itemsStore._trimItemStart(id, synchronizedTrimAmount, { skipAdjacentClamp: true }) diff --git a/src/features/timeline/stores/actions/item-actions.lock-invariants.test.ts b/src/features/timeline/stores/actions/item-actions.lock-invariants.test.ts index 482115da9..90faa350a 100644 --- a/src/features/timeline/stores/actions/item-actions.lock-invariants.test.ts +++ b/src/features/timeline/stores/actions/item-actions.lock-invariants.test.ts @@ -3,18 +3,23 @@ import { beforeEach, describe, expect, it } from 'vite-plus/test' import type { AudioItem, TimelineTrack, VideoItem } from '@/types/timeline' import { useEditorStore } from '@/shared/state/editor' +import { useSelectionStore } from '@/shared/state/selection' import { useItemsStore } from '../items-store' import { useKeyframesStore } from '../keyframes-store' import { useTimelineCommandStore } from '../timeline-command-store' import { useTimelineSettingsStore } from '../timeline-settings-store' import { useTransitionsStore } from '../transitions-store' +import { useReverseConformDialogStore } from '../reverse-conform-dialog-store' import { closeAllGapsOnTrack, closeGapAtPosition, moveItem, moveItems, + linkItems, + commitPreparedReverseItems, removeItems, rippleDeleteItems, + reverseItems, unlinkItems, updateItem, } from './item-actions' @@ -73,18 +78,21 @@ function makeAudioItem(overrides: Partial = {}): AudioItem { function expectNoHistory(): void { expect(useTimelineCommandStore.getState().undoStack).toHaveLength(0) + expect(useTimelineCommandStore.getState().redoStack).toHaveLength(0) expect(useTimelineSettingsStore.getState().isDirty).toBe(false) } describe('track lock mutation invariants', () => { beforeEach(() => { useEditorStore.setState({ linkedSelectionEnabled: true }) + useSelectionStore.getState().clearSelection() useItemsStore.getState().setItems([]) useItemsStore.getState().setTracks([]) useTransitionsStore.getState().setTransitions([]) useKeyframesStore.getState().setKeyframes([]) useTimelineCommandStore.getState().clearHistory() useTimelineSettingsStore.setState({ fps: 30, isDirty: false }) + useReverseConformDialogStore.setState({ request: null }) }) it('rejects direct timing, track, source-placement, and delete mutations on a locked item', () => { @@ -145,33 +153,142 @@ describe('track lock mutation invariants', () => { expectNoHistory() }) - it('requires explicit unlink before deleting away from a locked companion', () => { - useEditorStore.setState({ linkedSelectionEnabled: false }) - useItemsStore.getState().setTracks([ - makeTrack({ id: 'video-track', name: 'V1', kind: 'video', order: 0 }), - makeTrack({ - id: 'audio-track', - name: 'A1', - kind: 'audio', - order: 1, - locked: true, - }), + it.each([true, false])( + 'rejects unlinking and deleting away from a locked companion when linked selection is %s', + (linkedSelectionEnabled) => { + useEditorStore.setState({ linkedSelectionEnabled }) + useItemsStore.getState().setTracks([ + makeTrack({ id: 'video-track', name: 'V1', kind: 'video', order: 0 }), + makeTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }), + ]) + const video = makeVideoItem({ linkedGroupId: 'linked-av' }) + const audio = makeAudioItem({ linkedGroupId: 'linked-av' }) + useItemsStore.getState().setItems([video, audio]) + + useSelectionStore.getState().selectItems([video.id]) + const selectionBefore = useSelectionStore.getState().selectedItemIds + + removeItems([video.id]) + unlinkItems([video.id]) + removeItems([video.id]) + + expect(useItemsStore.getState().items).toEqual([video, audio]) + expect(useSelectionStore.getState().selectedItemIds).toEqual(selectionBefore) + expectNoHistory() + }, + ) + + it('rejects a prepared reverse commit when a nested linked track locks during conforming', () => { + const group = makeTrack({ + id: 'group', + name: 'Group', + kind: 'audio', + order: 1, + isGroup: true, + }) + const videoTrack = makeTrack({ id: 'video-track', name: 'V1', kind: 'video', order: 0 }) + const audioTrack = makeTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 2, + parentTrackId: group.id, + }) + const linkedVideo = makeVideoItem({ linkedGroupId: 'linked-av' }) + const linkedAudio = makeAudioItem({ linkedGroupId: 'linked-av' }) + useItemsStore.getState().setTracks([videoTrack, group, audioTrack]) + useItemsStore.getState().setItems([linkedVideo, linkedAudio]) + + reverseItems([linkedVideo.id]) + const request = useReverseConformDialogStore.getState().request + expect(request).not.toBeNull() + useItemsStore.getState().setTracks([videoTrack, { ...group, locked: true }, audioTrack]) + const itemsBefore = structuredClone(useItemsStore.getState().items) + + commitPreparedReverseItems(request?.items ?? [], [ + { + itemId: linkedVideo.id, + src: 'blob:reverse', + path: 'reverse/video.mp4', + key: 'reverse-key', + quality: 'preview', + usesProxy: true, + isSourceLevel: true, + }, ]) - const video = makeVideoItem({ linkedGroupId: 'linked-av' }) - const audio = makeAudioItem({ linkedGroupId: 'linked-av' }) - useItemsStore.getState().setItems([video, audio]) - removeItems([video.id]) - expect(useItemsStore.getState().items).toHaveLength(2) + expect(useItemsStore.getState().items).toEqual(itemsBefore) expectNoHistory() + }) - unlinkItems([video.id]) - removeItems([video.id]) + it.each([true, false])( + 'rejects mixed locked/unlocked delete requests atomically when linked selection is %s', + (linkedSelectionEnabled) => { + useEditorStore.setState({ linkedSelectionEnabled }) + useItemsStore.getState().setTracks([ + makeTrack({ id: 'video-track', name: 'V1', kind: 'video', order: 0 }), + makeTrack({ + id: 'audio-track', + name: 'A1', + kind: 'audio', + order: 1, + locked: true, + }), + ]) + const unlocked = makeVideoItem({ id: 'unlocked' }) + const locked = makeAudioItem({ id: 'locked' }) + useItemsStore.getState().setItems([unlocked, locked]) + useSelectionStore.getState().selectItems([unlocked.id, locked.id]) - expect(useItemsStore.getState().itemById[video.id]).toBeUndefined() - expect(useItemsStore.getState().itemById[audio.id]).toBeDefined() - expect(useTimelineCommandStore.getState().undoStack).toHaveLength(2) - }) + for (const action of [removeItems, rippleDeleteItems]) { + action([unlocked.id, locked.id]) + expect(useItemsStore.getState().items).toEqual([unlocked, locked]) + expect(useSelectionStore.getState().selectedItemIds).toEqual([unlocked.id, locked.id]) + expectNoHistory() + } + }, + ) + + it.each([true, false])( + 'preflights every link membership mutation when linked selection is %s', + (linkedSelectionEnabled) => { + useEditorStore.setState({ linkedSelectionEnabled }) + useItemsStore.getState().setTracks([ + makeTrack({ + id: 'group', + name: 'Group', + kind: 'video', + order: 0, + isGroup: true, + locked: true, + }), + makeTrack({ + id: 'video-track', + name: 'V1', + kind: 'video', + order: 1, + parentTrackId: 'group', + }), + makeTrack({ id: 'audio-track', name: 'A1', kind: 'audio', order: 2 }), + ]) + const lockedVideo = makeVideoItem({ linkedGroupId: 'video-1' }) + const audio = makeAudioItem({ linkedGroupId: 'audio-1' }) + useItemsStore.getState().setItems([lockedVideo, audio]) + useSelectionStore.getState().selectItems([lockedVideo.id]) + + expect(linkItems([lockedVideo.id, audio.id])).toBe(false) + + expect(useItemsStore.getState().items).toEqual([lockedVideo, audio]) + expect(useSelectionStore.getState().selectedItemIds).toEqual([lockedVideo.id]) + expectNoHistory() + }, + ) it('allows ripple delete on unlocked tracks while a locked sync-lock track stays byte-for-byte fixed', () => { const videoTrack = makeTrack({ diff --git a/src/features/timeline/stores/actions/item-actions.ts b/src/features/timeline/stores/actions/item-actions.ts index ae4fa6b5c..245cea6b4 100644 --- a/src/features/timeline/stores/actions/item-actions.ts +++ b/src/features/timeline/stores/actions/item-actions.ts @@ -818,6 +818,7 @@ export function unlinkItems(ids: string[]): void { const linkedItems = items.filter((item) => unlinkIds.has(item.id) && item.linkedGroupId) if (linkedItems.length === 0) return + if (!areItemMutationsUnlocked(linkedItems.map((item) => item.id))) return // Detect video items that have a linked audio companion — their embedded audio // should be muted after unlinking so it doesn't start playing when the audio is deleted. @@ -858,6 +859,7 @@ export function linkItems(ids: string[]): boolean { if (!canLinkSelection(items, ids) || selectedItems.length < 2) { return false } + if (!areItemMutationsUnlocked(selectedItems.map((item) => item.id))) return false const linkedGroupId = crypto.randomUUID() execute( @@ -990,30 +992,31 @@ export function commitPreparedReverseItems( export function removeItems(ids: string[]): void { const { items, tracks } = useItemsStore.getState() const expandedIds = expandIdsWithLinkedItems(items, ids, isLinkedSelectionEnabled()) - const { allowedIds } = partitionItemMutationIdsByLock({ + const partition = partitionItemMutationIdsByLock({ items, tracks, itemIds: expandedIds, }) - if (allowedIds.length === 0) return + if (partition.allowedIds.length === 0 || partition.blockedIds.length > 0) return + const removalIds = partition.allowedIds execute( 'REMOVE_ITEMS', () => { // Remove items - useItemsStore.getState()._removeItems(allowedIds) + useItemsStore.getState()._removeItems(removalIds) // Cascade: Remove transitions referencing deleted items - useTransitionsStore.getState()._removeTransitionsForItems(allowedIds) + useTransitionsStore.getState()._removeTransitionsForItems(removalIds) // Cascade: Remove keyframes for deleted items - useKeyframesStore.getState()._removeKeyframesForItems(allowedIds) + useKeyframesStore.getState()._removeKeyframesForItems(removalIds) pruneLayerGroupsAfterItemRemoval() useTimelineSettingsStore.getState().markDirty() }, - { ids: allowedIds }, + { ids: removalIds }, ) emitUiSound('delete') @@ -1171,7 +1174,7 @@ export function rippleDeleteItems(ids: string[]): void { tracks, itemIds: expandedIds, }) - if (deletionPartition.allowedIds.length === 0) return + if (deletionPartition.allowedIds.length === 0 || deletionPartition.blockedIds.length > 0) return const plan = buildRippleDeletePlan({ items, tracks, diff --git a/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts b/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts index 383764c72..e0640f809 100644 --- a/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts +++ b/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts @@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it } from 'vite-plus/test' import type { LottieItem, TextItem, TimelineItem, TimelineTrack } from '@/types/timeline' import type { Transition } from '@/types/transition' import { useEditorStore } from '@/shared/state/editor' +import { useSelectionStore } from '@/shared/state/selection' import { makeTimelineAudioItem, makeTimelineTrack, makeTimelineVideoItem } from '../../test-helpers' import { useItemsStore } from '../items-store' import { useKeyframesStore } from '../keyframes-store' @@ -79,6 +80,7 @@ function snapshot() { undoDepth: useTimelineCommandStore.getState().undoStack.length, redoDepth: useTimelineCommandStore.getState().redoStack.length, dirty: useTimelineSettingsStore.getState().isDirty, + selection: structuredClone(useSelectionStore.getState().selectedItemIds), } } @@ -90,11 +92,13 @@ function expectUnchanged(before: ReturnType): void { expect(useTimelineCommandStore.getState().undoStack).toHaveLength(before.undoDepth) expect(useTimelineCommandStore.getState().redoStack).toHaveLength(before.redoDepth) expect(useTimelineSettingsStore.getState().isDirty).toBe(before.dirty) + expect(useSelectionStore.getState().selectedItemIds).toEqual(before.selection) } describe('public item edit lock preflights', () => { beforeEach(() => { useEditorStore.setState({ linkedSelectionEnabled: true }) + useSelectionStore.getState().clearSelection() useItemsStore.getState().setTracks(tracks()) useItemsStore.getState().setItems([]) useTransitionsStore.getState().setTransitions([]) @@ -254,13 +258,13 @@ describe('public item edit lock preflights', () => { expectUnchanged(before) }) - it('rejects normal trim when attached caption repair would mutate a locked caption', () => { + it('allows normal trim when a locked attached caption remains wholly within final bounds', () => { const caption: TextItem = { id: 'caption', type: 'text', trackId: 'caption-track', - from: 100, - durationInFrames: 30, + from: 70, + durationInFrames: 10, label: 'Caption', text: 'Caption', color: '#fff', @@ -277,13 +281,51 @@ describe('public item edit lock preflights', () => { }), ]) useItemsStore.getState().setItems([video(), caption]) - const before = snapshot() trimItemEnd('middle', -30) - expectUnchanged(before) + expect(useItemsStore.getState().itemById.middle).toMatchObject({ durationInFrames: 30 }) + expect(useItemsStore.getState().itemById.caption).toEqual(caption) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + expect(useTimelineSettingsStore.getState().isDirty).toBe(true) }) + it.each([ + ['crossing', 80, 20], + ['removed', 100, 30], + ] as const)( + 'rejects normal trim when a locked attached caption would be %s', + (_case, from, durationInFrames) => { + const caption: TextItem = { + id: 'caption', + type: 'text', + trackId: 'caption-track', + from, + durationInFrames, + label: 'Caption', + text: 'Caption', + color: '#fff', + textRole: 'caption', + captionSource: { type: 'transcript', clipId: 'middle', mediaId: 'media-1' }, + } + useItemsStore.getState().setTracks([ + tracks()[0]!, + makeTimelineTrack({ + id: 'caption-track', + name: 'Captions', + order: 1, + locked: true, + }), + ]) + useItemsStore.getState().setItems([video(), caption]) + const before = snapshot() + + trimItemEnd('middle', -30) + + expectUnchanged(before) + }, + ) + it.each([ ['reversed', { reversed: true }], ['segmentStart', { segmentStart: 12 }], diff --git a/src/features/timeline/stores/actions/source-edit-actions.test.ts b/src/features/timeline/stores/actions/source-edit-actions.test.ts index d4b331e64..aeaa2e8b2 100644 --- a/src/features/timeline/stores/actions/source-edit-actions.test.ts +++ b/src/features/timeline/stores/actions/source-edit-actions.test.ts @@ -77,6 +77,46 @@ function trackItems(trackId: string): TimelineItem[] { .sort((a, b) => a.from - b.from) } +function deferSourceUrl() { + let release!: (url: string) => void + let reportStarted!: () => void + const started = new Promise((resolve) => { + reportStarted = resolve + }) + mocks.resolveMediaUrl = () => + new Promise((resolve) => { + release = resolve + reportStarted() + }) + return { started, release: (url = 'blob:source-media') => release(url) } +} + +function rejectionSnapshot() { + return { + items: structuredClone(useItemsStore.getState().items), + tracks: structuredClone(useItemsStore.getState().tracks), + transitions: structuredClone(useTransitionsStore.getState().transitions), + selection: structuredClone(useSelectionStore.getState().selectedItemIds), + playhead: usePlaybackStore.getState().currentFrame, + dirty: useTimelineSettingsStore.getState().isDirty, + undoDepth: useTimelineCommandStore.getState().undoStack.length, + redoDepth: useTimelineCommandStore.getState().redoStack.length, + mediaById: structuredClone(mocks.mediaById), + } +} + +function expectRejectedEditToPreserve(snapshot: ReturnType): void { + expect(useItemsStore.getState().items).toEqual(snapshot.items) + expect(useItemsStore.getState().tracks).toEqual(snapshot.tracks) + expect(useTransitionsStore.getState().transitions).toEqual(snapshot.transitions) + expect(useSelectionStore.getState().selectedItemIds).toEqual(snapshot.selection) + expect(usePlaybackStore.getState().currentFrame).toBe(snapshot.playhead) + expect(useTimelineSettingsStore.getState().isDirty).toBe(snapshot.dirty) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(snapshot.undoDepth) + expect(useTimelineCommandStore.getState().redoStack).toHaveLength(snapshot.redoDepth) + expect(mocks.mediaById).toEqual(snapshot.mediaById) +} + describe('source edit actions', () => { beforeEach(() => { useTimelineCommandStore.getState().clearHistory() @@ -227,41 +267,6 @@ describe('source edit actions', () => { expect(inserted).toHaveLength(1) expect(inserted[0]?.trackId).not.toBe('track-v1') }) - - it('revalidates target locks immediately before the async commit', async () => { - let releaseUrl!: (url: string) => void - let reportStarted!: () => void - const started = new Promise((resolve) => { - reportStarted = resolve - }) - mocks.resolveMediaUrl = () => - new Promise((resolve) => { - releaseUrl = resolve - reportStarted() - }) - usePlaybackStore.setState({ currentFrame: 0 }) - - const pendingEdit = performInsertEdit() - await started - useItemsStore.getState().setTracks([ - makeTimelineTrack({ - id: 'track-v1', - name: 'V1', - kind: 'video', - order: 0, - locked: true, - }), - makeTimelineTrack({ id: 'track-a1', name: 'A1', kind: 'audio', order: 1 }), - ]) - releaseUrl('blob:source-media') - await pendingEdit - - expect(useItemsStore.getState().items).toHaveLength(0) - expect(useItemsStore.getState().tracks[0]?.locked).toBe(true) - expect(usePlaybackStore.getState().currentFrame).toBe(0) - expect(useTimelineSettingsStore.getState().isDirty).toBe(false) - expect(useTimelineCommandStore.getState().undoStack).toHaveLength(0) - }) }) describe('performOverwriteEdit', () => { @@ -321,6 +326,163 @@ describe('source edit actions', () => { }) }) + it.each([ + ['insert', performInsertEdit], + ['overwrite', performOverwriteEdit], + ])('rebuilds the %s item plan after async media resolution', async (mode, action) => { + usePlaybackStore.setState({ currentFrame: 0 }) + useSelectionStore.getState().selectItems(['sentinel-selection']) + const deferred = deferSourceUrl() + + const pendingEdit = action() + await deferred.started + const unrelatedTrack = makeTimelineTrack({ + id: 'track-v2', + name: 'V2', + kind: 'video', + order: 2, + }) + useItemsStore.getState().setTracks([...useItemsStore.getState().tracks, unrelatedTrack]) + useItemsStore.getState().setItems([ + makeTimelineVideoItem({ + id: 'late-item', + trackId: 'track-v1', + from: 20, + durationInFrames: 20, + sourceEnd: 20, + sourceDuration: 120, + }), + ]) + deferred.release() + await pendingEdit + + expect(useItemsStore.getState().tracks.some((track) => track.id === unrelatedTrack.id)).toBe( + true, + ) + if (mode === 'insert') { + expect(useItemsStore.getState().itemById['late-item']).toMatchObject({ from: 80 }) + } else { + expect(useItemsStore.getState().itemById['late-item']).toBeUndefined() + } + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + }) + + it.each([ + ['insert', performInsertEdit], + ['overwrite', performOverwriteEdit], + ])('does not resurrect an item removed during the %s await', async (_mode, action) => { + useItemsStore.getState().setItems([ + makeTimelineVideoItem({ + id: 'removed-during-await', + trackId: 'track-v1', + from: 0, + durationInFrames: 120, + sourceEnd: 120, + sourceDuration: 120, + }), + ]) + const deferred = deferSourceUrl() + const pendingEdit = action() + await deferred.started + + useItemsStore.getState()._removeItems(['removed-during-await']) + deferred.release() + await pendingEdit + + expect(useItemsStore.getState().itemById['removed-during-await']).toBeUndefined() + expect(useItemsStore.getState().items).toHaveLength(1) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + }) + + it.each([ + ['insert', performInsertEdit], + ['overwrite', performOverwriteEdit], + ])( + 'fails the %s edit closed when its target is removed during an await', + async (_mode, action) => { + const deferred = deferSourceUrl() + const pendingEdit = action() + await deferred.started + + useItemsStore + .getState() + .setTracks([makeTimelineTrack({ id: 'track-a1', name: 'A1', kind: 'audio', order: 1 })]) + const before = rejectionSnapshot() + deferred.release() + await pendingEdit + + expectRejectedEditToPreserve(before) + expect(useItemsStore.getState().tracks.some((track) => track.id === 'track-v1')).toBe(false) + }, + ) + + it.each([ + ['insert', performInsertEdit], + ['overwrite', performOverwriteEdit], + ])( + 'fails the %s edit closed when its target is nested under a newly locked ancestor', + async (_mode, action) => { + const deferred = deferSourceUrl() + const pendingEdit = action() + await deferred.started + + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'grandparent', + name: 'Locked Grandparent', + order: 0, + isGroup: true, + locked: true, + }), + makeTimelineTrack({ + id: 'parent', + name: 'Parent', + order: 1, + isGroup: true, + parentTrackId: 'grandparent', + }), + makeTimelineTrack({ + id: 'track-v1', + name: 'V1', + kind: 'video', + order: 2, + parentTrackId: 'parent', + }), + makeTimelineTrack({ id: 'track-a1', name: 'A1', kind: 'audio', order: 3 }), + ]) + const before = rejectionSnapshot() + deferred.release() + await pendingEdit + + expectRejectedEditToPreserve(before) + }, + ) + + it.each([ + ['insert', performInsertEdit], + ['overwrite', performOverwriteEdit], + ])('fails the %s edit closed when its target locks during an await', async (_mode, action) => { + const deferred = deferSourceUrl() + const pendingEdit = action() + await deferred.started + + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'track-v1', + name: 'V1', + kind: 'video', + order: 0, + locked: true, + }), + makeTimelineTrack({ id: 'track-a1', name: 'A1', kind: 'audio', order: 1 }), + ]) + const before = rejectionSnapshot() + deferred.release() + await pendingEdit + + expectRejectedEditToPreserve(before) + }) + it.each([ ['insert', performInsertEdit], ['overwrite', performOverwriteEdit], diff --git a/src/features/timeline/stores/actions/source-edit-actions.ts b/src/features/timeline/stores/actions/source-edit-actions.ts index 589dd4eb7..dc729fb71 100644 --- a/src/features/timeline/stores/actions/source-edit-actions.ts +++ b/src/features/timeline/stores/actions/source-edit-actions.ts @@ -15,7 +15,10 @@ import { importMediaLibraryService } from '@/features/timeline/deps/media-librar import { getMediaType, resolveMediaUrl } from '@/features/timeline/deps/media-library-resolver' import { toast } from 'sonner' import { execute, applyTransitionRepairs, getLogger } from './shared' -import { resolveSourceEditTrackTargets } from '../../utils/source-edit-targeting' +import { + resolveSourceEditTrackTargets, + type SourceEditTrackTargets, +} from '../../utils/source-edit-targeting' import { buildMediaTimelineItems } from '../../utils/media-timeline-item-builder' import { DEFAULT_TRACK_HEIGHT } from '../../constants' import { DEFAULT_PROJECT_HEIGHT, DEFAULT_PROJECT_WIDTH } from '@/shared/projects/defaults' @@ -47,67 +50,183 @@ interface SourceEditContext { resolvedTracks: TimelineTrack[] } -async function resolveSourceEditContext(): Promise { - const { - sourcePreviewMediaId: sourceMediaId, - sourcePatchVideoEnabled, - sourcePatchAudioEnabled, - sourcePatchVideoTrackId, - sourcePatchAudioTrackId, - } = useEditorStore.getState() - if (!sourceMediaId) { - toast.warning('Open a source in the source monitor first') - return null +function getSourceMediaFingerprint(media: { + duration: number + fps?: number + width?: number + height?: number + mimeType: string + fileName: string + audioCodec?: string +}): string { + return JSON.stringify([ + media.duration, + media.fps, + media.width, + media.height, + media.mimeType, + media.fileName, + media.audioCodec, + ]) +} + +function getUnchangedSourceMedia(sourceMediaId: string, mediaFingerprint: string) { + const media = useMediaLibraryStore.getState().mediaById[sourceMediaId] + if (!media) return null + return getSourceMediaFingerprint(media) === mediaFingerprint ? media : null +} + +function sourceMediaNeedsVideoPatch(mediaType: SourceEditContext['mediaType']): boolean { + return mediaType === 'video' || mediaType === 'image' || mediaType === 'lottie' +} + +function sourceMediaHasAudio( + mediaType: SourceEditContext['mediaType'], + audioCodec: string | undefined, +): boolean { + return mediaType === 'video' ? Boolean(audioCodec) : false +} + +function findTrackById(tracks: TimelineTrack[], trackId: string | null): TimelineTrack | null { + if (!trackId) return null + return tracks.find((track) => track.id === trackId) ?? null +} + +function getSourceEditTrackInputs(params: { + tracks: TimelineTrack[] + activeTrackId: string | null + preferredVideoTrackId: string | null + preferredAudioTrackId: string | null +}) { + const activeTrack = findTrackById(params.tracks, params.activeTrackId) + const preferredVideoTrack = findTrackById(params.tracks, params.preferredVideoTrackId) + const preferredAudioTrack = findTrackById(params.tracks, params.preferredAudioTrackId) + return { + activeTrack, + referenceTrack: activeTrack ?? preferredVideoTrack ?? preferredAudioTrack, } +} - const { inPoint, outPoint } = useSourcePlayerStore.getState() - const { activeTrackId } = useSelectionStore.getState() - const tracks = useItemsStore.getState().tracks - const activeTrack = activeTrackId - ? (tracks.find((track) => track.id === activeTrackId) ?? null) - : null - const preferredVideoTrack = sourcePatchVideoTrackId - ? (tracks.find((track) => track.id === sourcePatchVideoTrackId) ?? null) - : null - const preferredAudioTrack = sourcePatchAudioTrackId - ? (tracks.find((track) => track.id === sourcePatchAudioTrackId) ?? null) - : null - const referenceTrack = activeTrack ?? preferredVideoTrack ?? preferredAudioTrack ?? null +function getSourceEditTiming(params: { + mediaType: SourceEditContext['mediaType'] + media: { duration: number; fps?: number } + projectFps: number + inPoint: number | null + outPoint: number | null +}) { + const sourceFps = params.media.fps || 30 + const sourceDurationFrames = + params.mediaType === 'image' + ? params.projectFps * 3 + : Math.max(1, Math.round(params.media.duration * sourceFps)) + const effectiveIn = params.inPoint ?? 0 + const effectiveOut = params.outPoint ?? sourceDurationFrames + const sourceRangeFrames = effectiveOut - effectiveIn + const clipDurationFrames = + sourceFps === params.projectFps + ? sourceRangeFrames + : Math.max(1, Math.round((sourceRangeFrames * params.projectFps) / sourceFps)) + return { effectiveIn, effectiveOut, clipDurationFrames } +} - const media = useMediaLibraryStore.getState().mediaById[sourceMediaId] - if (!media) { - getLogger().warn('Source edit: Source media not found') - return null +function warnSourceEditTargetFailure(params: { + mediaType: SourceEditContext['mediaType'] + hasAudio: boolean + patchVideo: boolean + patchAudio: boolean +}): void { + if (!params.patchVideo && !params.patchAudio) { + toast.warning('Enable V and/or A source patch targets first') + return + } + if (params.mediaType === 'audio' && !params.patchAudio) { + toast.warning('Enable the A source patch target to edit audio') + return } + if (sourceMediaNeedsVideoPatch(params.mediaType) && !params.patchVideo && !params.hasAudio) { + toast.warning('Enable the V source patch target to edit this source') + return + } + toast.warning('Unable to resolve source patch targets') +} - const mediaType = getMediaType(media.mimeType) - if (mediaType === 'unknown') { - getLogger().warn('Source edit: Unknown media type') +function resolveCurrentSourceEditTargets(params: { + tracks: TimelineTrack[] + activeTrackId: string | null + preferredVideoTrackId: string | null + preferredAudioTrackId: string | null + mediaType: SourceEditContext['mediaType'] + hasAudio: boolean + patchVideo: boolean + patchAudio: boolean + preferredTrackHeight: number +}): SourceEditTrackTargets | null { + const resolvedTargets = resolveSourceEditTrackTargets({ + tracks: params.tracks, + activeTrackId: params.activeTrackId, + preferredVideoTrackId: params.preferredVideoTrackId, + preferredAudioTrackId: params.preferredAudioTrackId, + mediaType: params.mediaType, + hasAudio: params.hasAudio, + patchVideo: params.patchVideo, + patchAudio: params.patchAudio, + preferredTrackHeight: params.preferredTrackHeight, + }) + if (!resolvedTargets) { + warnSourceEditTargetFailure(params) return null } - const sourceFps = media.fps || 30 - const projectFps = useTimelineSettingsStore.getState().fps - const sourceDurationFrames = - mediaType === 'image' ? projectFps * 3 : Math.max(1, Math.round(media.duration * sourceFps)) + const targetTrackIds = new Set( + [resolvedTargets.videoTrackId, resolvedTargets.audioTrackId].filter( + (trackId): trackId is string => !!trackId, + ), + ) + const lockedTarget = resolvedTargets.tracks.find( + (track) => + targetTrackIds.has(track.id) && isTimelineTrackLocked(resolvedTargets.tracks, track.id), + ) + if (!lockedTarget) return resolvedTargets + toast.warning(`Target track ${lockedTarget.name} is locked`) + return null +} - const effectiveIn = inPoint ?? 0 - const effectiveOut = outPoint ?? sourceDurationFrames +function buildCurrentSourceEditContext(params: { + sourceMediaId: string + mediaFingerprint: string + blobUrl: string + thumbnailUrl?: string +}): SourceEditContext | null { + const { sourceMediaId, mediaFingerprint, blobUrl, thumbnailUrl } = params + const editorState = useEditorStore.getState() + if (editorState.sourcePreviewMediaId !== sourceMediaId) return null - // Convert source frames to project frames - const sourceRangeFrames = effectiveOut - effectiveIn - const clipDurationFrames = - sourceFps === projectFps - ? sourceRangeFrames - : Math.max(1, Math.round((sourceRangeFrames * projectFps) / sourceFps)) + const media = getUnchangedSourceMedia(sourceMediaId, mediaFingerprint) + if (!media) return null - const insertFrame = usePlaybackStore.getState().currentFrame + const mediaType = getMediaType(media.mimeType) + if (mediaType === 'unknown') return null + const { + sourcePatchVideoEnabled, + sourcePatchAudioEnabled, + sourcePatchVideoTrackId, + sourcePatchAudioTrackId, + } = editorState + const { inPoint, outPoint } = useSourcePlayerStore.getState() + const { activeTrackId } = useSelectionStore.getState() + const tracks = useItemsStore.getState().tracks + const projectFps = useTimelineSettingsStore.getState().fps + const { referenceTrack } = getSourceEditTrackInputs({ + tracks, + activeTrackId, + preferredVideoTrackId: sourcePatchVideoTrackId, + preferredAudioTrackId: sourcePatchAudioTrackId, + }) + const timing = getSourceEditTiming({ mediaType, media, projectFps, inPoint, outPoint }) const currentProject = useProjectStore.getState().currentProject - const canvasWidth = currentProject?.metadata.width ?? DEFAULT_PROJECT_WIDTH - const canvasHeight = currentProject?.metadata.height ?? DEFAULT_PROJECT_HEIGHT - const hasAudio = mediaType === 'video' && !!media.audioCodec - const resolvedTargets = resolveSourceEditTrackTargets({ + const hasAudio = sourceMediaHasAudio(mediaType, media.audioCodec) + const resolvedTargets = resolveCurrentSourceEditTargets({ tracks, activeTrackId, preferredVideoTrackId: sourcePatchVideoTrackId, @@ -118,53 +237,16 @@ async function resolveSourceEditContext(): Promise { patchAudio: sourcePatchAudioEnabled, preferredTrackHeight: referenceTrack?.height ?? DEFAULT_TRACK_HEIGHT, }) - if (!resolvedTargets) { - if (!sourcePatchVideoEnabled && !sourcePatchAudioEnabled) { - toast.warning('Enable V and/or A source patch targets first') - } else if (mediaType === 'audio' && !sourcePatchAudioEnabled) { - toast.warning('Enable the A source patch target to edit audio') - } else if ( - (mediaType === 'video' || mediaType === 'image' || mediaType === 'lottie') && - !sourcePatchVideoEnabled && - !hasAudio - ) { - toast.warning('Enable the V source patch target to edit this source') - } else { - toast.warning('Unable to resolve source patch targets') - } - return null - } - - const targetTrackIds = [resolvedTargets.videoTrackId, resolvedTargets.audioTrackId].filter( - (trackId): trackId is string => !!trackId, - ) - const lockedTarget = resolvedTargets.tracks.find( - (timelineTrack) => - targetTrackIds.includes(timelineTrack.id) && - isTimelineTrackLocked(resolvedTargets.tracks, timelineTrack.id), - ) - if (lockedTarget) { - toast.warning(`Target track ${lockedTarget.name} is locked`) - return null - } - - // Resolve blob URLs before execute (async not allowed inside execute) - const blobUrl = await resolveMediaUrl(sourceMediaId) - if (!blobUrl) { - toast.error('Failed to load source media') - return null - } - const { mediaLibraryService } = await importMediaLibraryService() - const thumbnailUrl = (await mediaLibraryService.getThumbnailBlobUrl(sourceMediaId)) || undefined + if (!resolvedTargets) return null return { sourceMediaId, videoTrackId: resolvedTargets.videoTrackId, audioTrackId: resolvedTargets.audioTrackId, - effectiveIn, - effectiveOut, - clipDurationFrames, - insertFrame, + effectiveIn: timing.effectiveIn, + effectiveOut: timing.effectiveOut, + clipDurationFrames: timing.clipDurationFrames, + insertFrame: usePlaybackStore.getState().currentFrame, blobUrl, thumbnailUrl, media: { @@ -177,13 +259,151 @@ async function resolveSourceEditContext(): Promise { }, mediaType, hasAudio, - canvasWidth, - canvasHeight, + canvasWidth: currentProject?.metadata.width ?? DEFAULT_PROJECT_WIDTH, + canvasHeight: currentProject?.metadata.height ?? DEFAULT_PROJECT_HEIGHT, projectFps, resolvedTracks: resolvedTargets.tracks, } } +function getTrackAncestryFingerprint(tracks: TimelineTrack[], trackId: string): string | null { + if (!tracks.some((track) => track.id === trackId)) return null + + const trackById = new Map(tracks.map((track) => [track.id, track] as const)) + const visited = new Set() + const ancestry: Array< + Pick + > = [] + let currentId: string | undefined = trackId + + while (currentId) { + if (visited.has(currentId)) { + ancestry.push({ + id: `cycle:${currentId}`, + locked: true, + order: 0, + height: 0, + }) + break + } + visited.add(currentId) + + const track = trackById.get(currentId) + if (!track) { + ancestry.push({ + id: `missing:${currentId}`, + locked: true, + order: 0, + height: 0, + }) + break + } + + ancestry.push({ + id: track.id, + parentTrackId: track.parentTrackId, + kind: track.kind, + isGroup: track.isGroup, + locked: track.locked, + order: track.order, + height: track.height, + }) + currentId = track.parentTrackId + } + + return JSON.stringify(ancestry) +} + +interface ExistingSourceTargetBaseline { + video?: { id: string; ancestryFingerprint: string } + audio?: { id: string; ancestryFingerprint: string } +} + +function captureExistingTargetBaseline( + context: SourceEditContext, + tracks: TimelineTrack[], +): ExistingSourceTargetBaseline { + const capture = (trackId: string | undefined) => { + if (!trackId) return undefined + const ancestryFingerprint = getTrackAncestryFingerprint(tracks, trackId) + return ancestryFingerprint ? { id: trackId, ancestryFingerprint } : undefined + } + + return { + video: capture(context.videoTrackId), + audio: capture(context.audioTrackId), + } +} + +function sourceTargetsDrifted( + baseline: ExistingSourceTargetBaseline, + context: SourceEditContext, + currentTracks: TimelineTrack[], +): boolean { + return ( + (!!baseline.video && + (context.videoTrackId !== baseline.video.id || + getTrackAncestryFingerprint(currentTracks, baseline.video.id) !== + baseline.video.ancestryFingerprint)) || + (!!baseline.audio && + (context.audioTrackId !== baseline.audio.id || + getTrackAncestryFingerprint(currentTracks, baseline.audio.id) !== + baseline.audio.ancestryFingerprint)) + ) +} + +async function resolveSourceEditContext(): Promise { + const sourceMediaId = useEditorStore.getState().sourcePreviewMediaId + if (!sourceMediaId) { + toast.warning('Open a source in the source monitor first') + return null + } + + const initialMedia = useMediaLibraryStore.getState().mediaById[sourceMediaId] + if (!initialMedia) { + getLogger().warn('Source edit: Source media not found') + return null + } + if (getMediaType(initialMedia.mimeType) === 'unknown') { + getLogger().warn('Source edit: Unknown media type') + return null + } + + const mediaFingerprint = getSourceMediaFingerprint(initialMedia) + const initialTracks = useItemsStore.getState().tracks + const initialContext = buildCurrentSourceEditContext({ + sourceMediaId, + mediaFingerprint, + blobUrl: '', + }) + if (!initialContext) return null + const targetBaseline = captureExistingTargetBaseline(initialContext, initialTracks) + + // Resolve every async asset first. The complete edit plan is deliberately + // rebuilt from live stores only after these awaits, so concurrent timeline + // changes cannot be overwritten by an earlier tracks/items snapshot. + const blobUrl = await resolveMediaUrl(sourceMediaId) + if (!blobUrl) { + toast.error('Failed to load source media') + return null + } + const { mediaLibraryService } = await importMediaLibraryService() + const thumbnailUrl = (await mediaLibraryService.getThumbnailBlobUrl(sourceMediaId)) || undefined + + const context = buildCurrentSourceEditContext({ + sourceMediaId, + mediaFingerprint, + blobUrl, + thumbnailUrl, + }) + if (!context) return null + + const currentTracks = useItemsStore.getState().tracks + if (sourceTargetsDrifted(targetBaseline, context, currentTracks)) return null + + return context +} + function createTimelineItems(ctx: SourceEditContext) { if (ctx.mediaType === 'audio' && !ctx.audioTrackId) { return [] @@ -233,12 +453,6 @@ function createTimelineItems(ctx: SourceEditContext) { }) } -function getSourceEditPreflightTracks(resolvedTracks: TimelineTrack[]): TimelineTrack[] { - const currentTracks = useItemsStore.getState().tracks - const currentTrackIds = new Set(currentTracks.map((track) => track.id)) - return [...currentTracks, ...resolvedTracks.filter((track) => !currentTrackIds.has(track.id))] -} - function canCommitSourceEdit(params: { mode: 'insert' | 'overwrite' targetTrackIds: string[] @@ -257,11 +471,9 @@ function canCommitSourceEdit(params: { : item.from < params.end && itemEnd > params.start }) .map((item) => item.id) - const tracks = getSourceEditPreflightTracks(params.resolvedTracks) - return preflightTimelineMutation({ items, - tracks, + tracks: params.resolvedTracks, itemIds: mutationIds, destinationTrackIds: params.targetTrackIds, }).allowed diff --git a/src/features/timeline/utils/group-utils.test.ts b/src/features/timeline/utils/group-utils.test.ts index c9ab55af6..578e716e8 100644 --- a/src/features/timeline/utils/group-utils.test.ts +++ b/src/features/timeline/utils/group-utils.test.ts @@ -61,6 +61,108 @@ describe('group-utils', () => { }) }) + it('propagates every effective state through a nested grandparent group', () => { + const [effectiveChild] = resolveEffectiveTrackStates([ + makeTrack({ + id: 'grandparent', + isGroup: true, + locked: true, + visible: false, + }), + makeTrack({ + id: 'parent', + isGroup: true, + parentTrackId: 'grandparent', + muted: true, + solo: true, + }), + makeTrack({ id: 'child', parentTrackId: 'parent' }), + ]) + + expect(effectiveChild).toMatchObject({ + id: 'child', + locked: true, + muted: true, + visible: false, + solo: true, + }) + }) + + it('resolves a deep tree without depending on input order', () => { + const depth = 1_000 + const groups = Array.from({ length: depth }, (_, index) => + makeTrack({ + id: `group-${index}`, + isGroup: true, + parentTrackId: index === 0 ? undefined : `group-${index - 1}`, + locked: index === 17, + muted: index === 217, + visible: index !== 617, + solo: index === 917, + }), + ) + const child = makeTrack({ id: 'deep-child', parentTrackId: `group-${depth - 1}` }) + + const [effectiveChild] = resolveEffectiveTrackStates([child, ...groups.toReversed()]) + + expect(effectiveChild).toMatchObject({ + id: 'deep-child', + locked: true, + muted: true, + visible: false, + solo: true, + }) + }) + + it('fails closed when a parent track is missing', () => { + const [effectiveChild] = resolveEffectiveTrackStates([ + makeTrack({ + id: 'orphan', + parentTrackId: 'missing-parent', + solo: true, + }), + ]) + + expect(effectiveChild).toMatchObject({ + locked: true, + muted: true, + visible: false, + solo: false, + }) + }) + + it('fails closed for a self-parent cycle', () => { + const [effectiveChild] = resolveEffectiveTrackStates([ + makeTrack({ id: 'self-cycle', parentTrackId: 'self-cycle', solo: true }), + ]) + + expect(effectiveChild).toMatchObject({ + locked: true, + muted: true, + visible: false, + solo: false, + }) + }) + + it('fails closed for every lane whose ancestry reaches a multi-node cycle', () => { + const effectiveTracks = resolveEffectiveTrackStates([ + makeTrack({ id: 'group-a', isGroup: true, parentTrackId: 'group-b' }), + makeTrack({ id: 'group-b', isGroup: true, parentTrackId: 'group-a' }), + makeTrack({ id: 'child-a', parentTrackId: 'group-a', solo: true }), + makeTrack({ id: 'child-b', parentTrackId: 'group-b' }), + ]) + + expect(effectiveTracks).toHaveLength(2) + for (const effectiveTrack of effectiveTracks) { + expect(effectiveTrack).toMatchObject({ + locked: true, + muted: true, + visible: false, + solo: false, + }) + } + }) + it('uses propagated visibility when collecting visible track ids', () => { const visibleTrackIds = getVisibleTrackIds([ makeTrack({ id: 'group-1', isGroup: true, visible: false }), diff --git a/src/features/timeline/utils/group-utils.ts b/src/features/timeline/utils/group-utils.ts index beab29495..1bc06192a 100644 --- a/src/features/timeline/utils/group-utils.ts +++ b/src/features/timeline/utils/group-utils.ts @@ -45,29 +45,131 @@ export function pruneEmptyLayerGroupHierarchy( return pruneEmptyLayerGroups(tracksWithPopulatedGroupChildren) } +type EffectiveTrackState = Pick & { + valid: boolean +} + +const ROOT_TRACK_STATE: EffectiveTrackState = { + locked: false, + muted: false, + visible: true, + solo: false, + valid: true, +} + +const MALFORMED_TRACK_STATE: EffectiveTrackState = { + locked: true, + muted: true, + visible: false, + solo: false, + valid: false, +} + +function inheritTrackState( + track: TimelineTrack, + inheritedState: EffectiveTrackState, +): EffectiveTrackState { + if (!inheritedState.valid) return MALFORMED_TRACK_STATE + return { + locked: track.locked || inheritedState.locked, + muted: track.muted || inheritedState.muted, + visible: track.visible !== false && inheritedState.visible, + solo: track.solo || inheritedState.solo, + valid: true, + } +} + +function collectTrackResolutionPath(params: { + track: TimelineTrack + trackById: Map + duplicateTrackIds: Set + stateById: Map +}): { path: TimelineTrack[]; inheritedState: EffectiveTrackState } { + const path: TimelineTrack[] = [] + const pathIds = new Set() + let cursor: TimelineTrack | undefined = params.track + + while (cursor) { + const cached = params.stateById.get(cursor.id) + if (cached) return { path, inheritedState: cached } + if (params.duplicateTrackIds.has(cursor.id)) { + return { path, inheritedState: MALFORMED_TRACK_STATE } + } + if (pathIds.has(cursor.id)) return { path, inheritedState: MALFORMED_TRACK_STATE } + + pathIds.add(cursor.id) + path.push(cursor) + if (!cursor.parentTrackId) return { path, inheritedState: ROOT_TRACK_STATE } + + cursor = params.trackById.get(cursor.parentTrackId) + if (!cursor) return { path, inheritedState: MALFORMED_TRACK_STATE } + } + + return { path, inheritedState: MALFORMED_TRACK_STATE } +} + +function resolveTrackState(params: { + track: TimelineTrack + trackById: Map + duplicateTrackIds: Set + stateById: Map +}): EffectiveTrackState { + const cached = params.stateById.get(params.track.id) + if (cached) return cached + if (params.duplicateTrackIds.has(params.track.id)) return MALFORMED_TRACK_STATE + + const { path, inheritedState } = collectTrackResolutionPath(params) + let state = inheritedState + for (let index = path.length - 1; index >= 0; index -= 1) { + const pathTrack = path[index]! + state = inheritTrackState(pathTrack, state) + params.stateById.set(pathTrack.id, state) + } + return params.stateById.get(params.track.id) ?? MALFORMED_TRACK_STATE +} + /** - * Return active timeline lanes with inherited Layer Group state and without - * the organizational container rows themselves. + * Return active timeline lanes with inherited state from their complete + * Layer Group ancestry and without the organizational container rows. + * + * Lock, mute, and solo are enabled by any ancestor; visibility must remain + * enabled at every level. Malformed ancestry (a missing parent or a cycle) is + * resolved fail-closed so every consumer sees the lane as locked, muted, and + * hidden rather than making a different partial guess. Solo is disabled for + * malformed ancestry because promoting an invalid lane into the solo set + * would make it more audible/visible, not less. */ export function resolveEffectiveTrackStates(tracks: TimelineTrack[]): TimelineTrack[] { - const groupsById = new Map( - tracks.filter((track) => track.isGroup).map((track) => [track.id, track] as const), - ) + const trackById = new Map() + const duplicateTrackIds = new Set() + for (const track of tracks) { + if (trackById.has(track.id)) { + duplicateTrackIds.add(track.id) + } else { + trackById.set(track.id, track) + } + } + const stateById = new Map() return tracks .filter((track) => !track.isGroup) .map((track) => { - const parentGroup = track.parentTrackId ? groupsById.get(track.parentTrackId) : undefined - if (!parentGroup) { + const state = resolveTrackState({ track, trackById, duplicateTrackIds, stateById }) + if ( + state.locked === track.locked && + state.muted === track.muted && + state.visible === track.visible && + state.solo === track.solo + ) { return track } return { ...track, - locked: track.locked || parentGroup.locked, - muted: track.muted || parentGroup.muted, - visible: track.visible !== false && parentGroup.visible !== false, - solo: track.solo || parentGroup.solo, + locked: state.locked, + muted: state.muted, + visible: state.visible, + solo: state.solo, } }) } From d731f64f4e573053edd831031649c5219d7a6a6d Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 21:13:55 -0700 Subject: [PATCH 4/5] fix(timeline): preserve nested group ancestry --- .../actions/edit/freeze-frame-actions.test.ts | 41 ++++ .../actions/source-edit-actions.test.ts | 105 ++++++++++ .../items-store.track-hierarchy.test.ts | 184 ++++++++++++++++++ .../timeline/utils/group-utils.test.ts | 50 +++++ src/features/timeline/utils/group-utils.ts | 52 ++++- 5 files changed, 423 insertions(+), 9 deletions(-) create mode 100644 src/features/timeline/stores/items-store.track-hierarchy.test.ts diff --git a/src/features/timeline/stores/actions/edit/freeze-frame-actions.test.ts b/src/features/timeline/stores/actions/edit/freeze-frame-actions.test.ts index a91d66c18..6465eedaf 100644 --- a/src/features/timeline/stores/actions/edit/freeze-frame-actions.test.ts +++ b/src/features/timeline/stores/actions/edit/freeze-frame-actions.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' import type { AudioItem, TimelineItem, TimelineTrack } from '@/types/timeline' +import type { Transition } from '@/types/transition' const mocks = vi.hoisted(() => ({ acquire: vi.fn<(mediaId: string, blob: Blob) => string>(), @@ -71,6 +72,7 @@ import { makeTimelineVideoItem, } from '../../../test-helpers' import { useItemsStore } from '../../items-store' +import { useKeyframesStore } from '../../keyframes-store' import { useTimelineCommandStore } from '../../timeline-command-store' import { useTimelineSettingsStore } from '../../timeline-settings-store' import { useTransitionsStore } from '../../transitions-store' @@ -118,6 +120,7 @@ function snapshot() { items: structuredClone(useItemsStore.getState().items), tracks: structuredClone(useItemsStore.getState().tracks), transitions: structuredClone(useTransitionsStore.getState().transitions), + keyframes: structuredClone(useKeyframesStore.getState().keyframes), selection: structuredClone(useSelectionStore.getState().selectedItemIds), dirty: useTimelineSettingsStore.getState().isDirty, undoDepth: useTimelineCommandStore.getState().undoStack.length, @@ -130,6 +133,7 @@ function expectSnapshot(expected: ReturnType): void { expect(useItemsStore.getState().items).toEqual(expected.items) expect(useItemsStore.getState().tracks).toEqual(expected.tracks) expect(useTransitionsStore.getState().transitions).toEqual(expected.transitions) + expect(useKeyframesStore.getState().keyframes).toEqual(expected.keyframes) expect(useSelectionStore.getState().selectedItemIds).toEqual(expected.selection) expect(useTimelineSettingsStore.getState().isDirty).toBe(expected.dirty) expect(useTimelineCommandStore.getState().undoStack).toHaveLength(expected.undoDepth) @@ -160,6 +164,17 @@ describe('freeze-frame async atomicity', () => { useItemsStore.getState().setTracks([videoTrack()]) useItemsStore.getState().setItems([video()]) useTransitionsStore.getState().setTransitions([]) + useKeyframesStore.getState().setKeyframes([ + { + itemId: 'video', + properties: [ + { + property: 'opacity', + keyframes: [{ id: 'sentinel-keyframe', frame: 10, value: 0.75, easing: 'linear' }], + }, + ], + }, + ]) useSelectionStore.getState().clearSelection() useSelectionStore.getState().selectItems(['sentinel-selection']) useTimelineCommandStore.getState().clearHistory() @@ -297,6 +312,32 @@ describe('freeze-frame async atomicity', () => { expect(mocks.release).toHaveBeenCalledWith('freeze-media') }) + it('rejects relevant transition drift after persistence without touching keyframes', async () => { + useItemsStore.getState().setItems([video(), video({ id: 'right', from: 120 })]) + const deferred = deferGeneratedImageImport() + const pending = insertFreezeFrame('video', 60) + await deferred.started + + const transition: Transition = { + id: 'late-transition', + type: 'crossfade', + presentation: 'fade', + timing: 'linear', + leftClipId: 'video', + rightClipId: 'right', + trackId: 'video-track', + durationInFrames: 10, + } + useTransitionsStore.getState().setTransitions([transition]) + const before = snapshot() + deferred.release() + + await expect(pending).resolves.toBe(false) + expectSnapshot(before) + expect(mocks.deleteMediaFromProject).toHaveBeenCalledWith('project-1', 'freeze-media') + expect(mocks.release).toHaveBeenCalledWith('freeze-media') + }) + it('rejects linked companion drift after persistence', async () => { const audioTrack = makeTimelineTrack({ id: 'audio-track', diff --git a/src/features/timeline/stores/actions/source-edit-actions.test.ts b/src/features/timeline/stores/actions/source-edit-actions.test.ts index aeaa2e8b2..148a5aac2 100644 --- a/src/features/timeline/stores/actions/source-edit-actions.test.ts +++ b/src/features/timeline/stores/actions/source-edit-actions.test.ts @@ -49,9 +49,11 @@ import { useSelectionStore } from '@/shared/state/selection' import { useSourcePlayerStore } from '@/shared/state/source-player' import { usePlaybackStore } from '@/shared/state/playback' import { useItemsStore } from '../items-store' +import { useKeyframesStore } from '../keyframes-store' import { useTransitionsStore } from '../transitions-store' import { useTimelineCommandStore } from '../timeline-command-store' import { useTimelineSettingsStore } from '../timeline-settings-store' +import { resolveEffectiveTrackStates } from '../../utils/group-utils' import { performInsertEdit, performOverwriteEdit } from './source-edit-actions' function setSourceMedia(overrides: Record = {}) { @@ -96,6 +98,7 @@ function rejectionSnapshot() { items: structuredClone(useItemsStore.getState().items), tracks: structuredClone(useItemsStore.getState().tracks), transitions: structuredClone(useTransitionsStore.getState().transitions), + keyframes: structuredClone(useKeyframesStore.getState().keyframes), selection: structuredClone(useSelectionStore.getState().selectedItemIds), playhead: usePlaybackStore.getState().currentFrame, dirty: useTimelineSettingsStore.getState().isDirty, @@ -109,6 +112,7 @@ function expectRejectedEditToPreserve(snapshot: ReturnType { ]) useItemsStore.getState().setItems([]) useTransitionsStore.getState().setTransitions([]) + useKeyframesStore.getState().setKeyframes([ + { + itemId: 'sentinel-keyframe-owner', + properties: [ + { + property: 'opacity', + keyframes: [{ id: 'sentinel-keyframe', frame: 0, value: 1, easing: 'linear' }], + }, + ], + }, + ]) useSelectionStore.getState().setActiveTrack(null) useEditorStore.setState({ sourcePreviewMediaId: 'media-1', @@ -367,6 +382,77 @@ describe('source edit actions', () => { expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) }) + it.each([ + ['insert', performInsertEdit], + ['overwrite', performOverwriteEdit], + ])('preserves an unrelated track removal during the %s await', async (_mode, action) => { + const unrelatedTrack = makeTimelineTrack({ + id: 'track-v2', + name: 'V2', + kind: 'video', + order: 2, + }) + useItemsStore.getState().setTracks([...useItemsStore.getState().tracks, unrelatedTrack]) + const deferred = deferSourceUrl() + const pendingEdit = action() + await deferred.started + + useItemsStore + .getState() + .setTracks(useItemsStore.getState().tracks.filter((track) => track.id !== unrelatedTrack.id)) + deferred.release() + await pendingEdit + + expect(useItemsStore.getState().tracks.some((track) => track.id === unrelatedTrack.id)).toBe( + false, + ) + expect(useItemsStore.getState().items).toHaveLength(1) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + }) + + it.each([ + ['insert', performInsertEdit], + ['overwrite', performOverwriteEdit], + ])('preserves an unrelated track reparent during the %s await', async (_mode, action) => { + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'old-group', name: 'Old group', order: 0, isGroup: true }), + makeTimelineTrack({ id: 'track-v1', name: 'V1', kind: 'video', order: 1 }), + makeTimelineTrack({ id: 'track-a1', name: 'A1', kind: 'audio', order: 2 }), + makeTimelineTrack({ + id: 'unrelated-lane', + name: 'V2', + kind: 'video', + order: 3, + parentTrackId: 'old-group', + }), + ]) + const deferred = deferSourceUrl() + const pendingEdit = action() + await deferred.started + + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'new-group', name: 'New group', order: 0, isGroup: true }), + makeTimelineTrack({ id: 'track-v1', name: 'V1', kind: 'video', order: 1 }), + makeTimelineTrack({ id: 'track-a1', name: 'A1', kind: 'audio', order: 2 }), + makeTimelineTrack({ + id: 'unrelated-lane', + name: 'V2', + kind: 'video', + order: 3, + parentTrackId: 'new-group', + }), + ]) + deferred.release() + await pendingEdit + + const tracks = useItemsStore.getState().tracks + expect(tracks.some((track) => track.id === 'old-group')).toBe(false) + expect(tracks.some((track) => track.id === 'new-group')).toBe(true) + expect(tracks.find((track) => track.id === 'unrelated-lane')?.parentTrackId).toBe('new-group') + expect(useItemsStore.getState().items).toHaveLength(1) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + }) + it.each([ ['insert', performInsertEdit], ['overwrite', performOverwriteEdit], @@ -450,6 +536,25 @@ describe('source edit actions', () => { }), makeTimelineTrack({ id: 'track-a1', name: 'A1', kind: 'audio', order: 3 }), ]) + const currentTracks = useItemsStore.getState().tracks + expect(currentTracks.map((track) => track.id)).toEqual([ + 'grandparent', + 'parent', + 'track-v1', + 'track-a1', + ]) + expect( + currentTracks.every( + (track) => + !track.parentTrackId || + currentTracks.some((parent) => parent.id === track.parentTrackId), + ), + ).toBe(true) + expect( + resolveEffectiveTrackStates(currentTracks).find((track) => track.id === 'track-v1'), + ).toMatchObject({ + locked: true, + }) const before = rejectionSnapshot() deferred.release() await pendingEdit diff --git a/src/features/timeline/stores/items-store.track-hierarchy.test.ts b/src/features/timeline/stores/items-store.track-hierarchy.test.ts new file mode 100644 index 000000000..317de4fd2 --- /dev/null +++ b/src/features/timeline/stores/items-store.track-hierarchy.test.ts @@ -0,0 +1,184 @@ +// @vitest-environment node + +import { beforeEach, describe, expect, it } from 'vite-plus/test' +import { makeTimelineTrack } from '../test-helpers' +import { useItemsStore } from './items-store' +import { resolveEffectiveTrackStates } from '../utils/group-utils' + +function storedTrackIds(): string[] { + return useItemsStore.getState().tracks.map((track) => track.id) +} + +describe('items-store track hierarchy normalization', () => { + beforeEach(() => { + useItemsStore.getState().setItems([]) + useItemsStore.getState().setTracks([]) + }) + + it('preserves every populated group ancestor and prunes an empty sibling branch', () => { + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'outer', name: 'Outer', order: 0, isGroup: true }), + makeTimelineTrack({ + id: 'middle', + name: 'Middle', + order: 1, + isGroup: true, + parentTrackId: 'outer', + }), + makeTimelineTrack({ + id: 'inner', + name: 'Inner', + order: 2, + isGroup: true, + parentTrackId: 'middle', + }), + makeTimelineTrack({ + id: 'empty-sibling', + name: 'Empty sibling', + order: 3, + isGroup: true, + parentTrackId: 'outer', + }), + makeTimelineTrack({ + id: 'lane', + name: 'Lane', + kind: 'video', + order: 4, + parentTrackId: 'inner', + }), + ]) + + expect(storedTrackIds()).toEqual(['outer', 'middle', 'inner', 'lane']) + }) + + it('preserves a valid nested ancestry so an outer lock is inherited after setTracks', () => { + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'locked-outer', + name: 'Locked outer', + order: 0, + isGroup: true, + locked: true, + }), + makeTimelineTrack({ + id: 'inner', + name: 'Inner', + order: 1, + isGroup: true, + parentTrackId: 'locked-outer', + }), + makeTimelineTrack({ + id: 'lane', + name: 'Lane', + kind: 'video', + order: 2, + parentTrackId: 'inner', + }), + ]) + + expect(storedTrackIds()).toEqual(['locked-outer', 'inner', 'lane']) + expect(resolveEffectiveTrackStates(useItemsStore.getState().tracks)).toEqual([ + expect.objectContaining({ id: 'lane', locked: true }), + ]) + }) + + it('retains orphan lanes and resolves their missing ancestry fail-closed', () => { + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'orphan', + name: 'Orphan', + kind: 'video', + order: 0, + parentTrackId: 'missing-group', + }), + ]) + + expect(storedTrackIds()).toEqual(['orphan']) + expect(resolveEffectiveTrackStates(useItemsStore.getState().tracks)).toEqual([ + expect.objectContaining({ + id: 'orphan', + locked: true, + muted: true, + visible: false, + solo: false, + }), + ]) + }) + + it('retains self and multi-node cycles reached by lanes and resolves them fail-closed', () => { + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'self-group', + name: 'Self group', + order: 0, + isGroup: true, + parentTrackId: 'self-group', + }), + makeTimelineTrack({ + id: 'self-lane', + name: 'Self lane', + kind: 'video', + order: 1, + parentTrackId: 'self-group', + }), + makeTimelineTrack({ + id: 'cycle-a', + name: 'Cycle A', + order: 2, + isGroup: true, + parentTrackId: 'cycle-b', + }), + makeTimelineTrack({ + id: 'cycle-b', + name: 'Cycle B', + order: 3, + isGroup: true, + parentTrackId: 'cycle-a', + }), + makeTimelineTrack({ + id: 'cycle-lane', + name: 'Cycle lane', + kind: 'video', + order: 4, + parentTrackId: 'cycle-a', + }), + ]) + + expect(storedTrackIds()).toEqual([ + 'self-group', + 'self-lane', + 'cycle-a', + 'cycle-b', + 'cycle-lane', + ]) + expect(resolveEffectiveTrackStates(useItemsStore.getState().tracks)).toEqual([ + expect.objectContaining({ id: 'self-lane', locked: true, visible: false }), + expect.objectContaining({ id: 'cycle-lane', locked: true, visible: false }), + ]) + }) + + it('retains duplicate parent definitions so ambiguous ancestry remains fail-closed', () => { + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'duplicate', name: 'Duplicate A', order: 0, isGroup: true }), + makeTimelineTrack({ id: 'duplicate', name: 'Duplicate B', order: 1, isGroup: true }), + makeTimelineTrack({ + id: 'lane', + name: 'Lane', + kind: 'video', + order: 2, + parentTrackId: 'duplicate', + }), + ]) + + expect(storedTrackIds()).toEqual(['duplicate', 'duplicate', 'lane']) + expect(resolveEffectiveTrackStates(useItemsStore.getState().tracks)).toEqual([ + expect.objectContaining({ + id: 'lane', + locked: true, + muted: true, + visible: false, + solo: false, + }), + ]) + }) +}) diff --git a/src/features/timeline/utils/group-utils.test.ts b/src/features/timeline/utils/group-utils.test.ts index 578e716e8..f1a957fc1 100644 --- a/src/features/timeline/utils/group-utils.test.ts +++ b/src/features/timeline/utils/group-utils.test.ts @@ -163,6 +163,22 @@ describe('group-utils', () => { } }) + it('fails closed when a lane reaches a duplicate track id', () => { + const [effectiveTrack] = resolveEffectiveTrackStates([ + makeTrack({ id: 'duplicate-parent', isGroup: true }), + makeTrack({ id: 'duplicate-parent', isGroup: true, locked: false }), + makeTrack({ id: 'child', parentTrackId: 'duplicate-parent', solo: true }), + ]) + + expect(effectiveTrack).toMatchObject({ + id: 'child', + locked: true, + muted: true, + visible: false, + solo: false, + }) + }) + it('uses propagated visibility when collecting visible track ids', () => { const visibleTrackIds = getVisibleTrackIds([ makeTrack({ id: 'group-1', isGroup: true, visible: false }), @@ -184,6 +200,40 @@ describe('group-utils', () => { ]) }) + it('retains every transitive group ancestor in input order and prunes empty branches', () => { + const outer = makeTrack({ id: 'outer', isGroup: true }) + const inner = makeTrack({ id: 'inner', isGroup: true, parentTrackId: outer.id }) + const emptySibling = makeTrack({ + id: 'empty-sibling', + isGroup: true, + parentTrackId: outer.id, + }) + const child = makeTrack({ id: 'child', parentTrackId: inner.id }) + + expect(pruneEmptyLayerGroups([child, emptySibling, inner, outer])).toEqual([ + child, + inner, + outer, + ]) + }) + + it('retains malformed group ancestry only when a lane reaches it', () => { + const cycleA = makeTrack({ id: 'cycle-a', isGroup: true, parentTrackId: 'cycle-b' }) + const cycleB = makeTrack({ id: 'cycle-b', isGroup: true, parentTrackId: 'cycle-a' }) + const unreferencedCycle = makeTrack({ + id: 'unreferenced-cycle', + isGroup: true, + parentTrackId: 'unreferenced-cycle', + }) + const child = makeTrack({ id: 'child', parentTrackId: cycleA.id }) + + expect(pruneEmptyLayerGroups([cycleA, cycleB, unreferencedCycle, child])).toEqual([ + cycleA, + cycleB, + child, + ]) + }) + it('prunes empty child lanes without removing empty top-level classic tracks', () => { const group = makeTrack({ id: 'group', isGroup: true }) const populatedChild = makeTrack({ id: 'child-populated', parentTrackId: group.id }) diff --git a/src/features/timeline/utils/group-utils.ts b/src/features/timeline/utils/group-utils.ts index 1bc06192a..605439b49 100644 --- a/src/features/timeline/utils/group-utils.ts +++ b/src/features/timeline/utils/group-utils.ts @@ -11,20 +11,54 @@ export function getVisibleTrackIds(tracks: TimelineTrack[]): Set { ) } +function indexLayerGroups(tracks: TimelineTrack[]): Map { + const groupsById = new Map() + for (const track of tracks) { + if (!track.isGroup) continue + const definitions = groupsById.get(track.id) + if (definitions) definitions.push(track) + else groupsById.set(track.id, [track]) + } + return groupsById +} + +function collectRetainedGroupIds( + tracks: TimelineTrack[], + groupsById: Map, +): Set { + const retainedGroupIds = new Set( + tracks.flatMap((track) => (!track.isGroup && track.parentTrackId ? [track.parentTrackId] : [])), + ) + const pendingGroupIds = [...retainedGroupIds] + + for (let index = 0; index < pendingGroupIds.length; index += 1) { + const definitions = groupsById.get(pendingGroupIds[index]!) ?? [] + for (const group of definitions) { + const parentId = group.parentTrackId + if (!parentId || retainedGroupIds.has(parentId)) continue + retainedGroupIds.add(parentId) + pendingGroupIds.push(parentId) + } + } + + return retainedGroupIds +} + /** - * Remove layer-group containers that no longer own any child tracks. + * Remove layer-group containers that no longer own a descendant lane. * * A layer group is an organizational timeline container, not an item lane of - * its own, so retaining an empty container only leaves an orphaned UI row. + * its own, so retaining a branch with no lane only leaves orphaned UI rows. + * Starting from every non-group lane makes the traversal independent of input + * order and retains its complete group ancestry. Missing parents terminate a + * branch, while visited IDs make self/multi-node cycles finite. If an ID has + * duplicate group definitions, all of them and all of their possible parents + * are retained so normalization does not erase the ambiguity that effective + * state resolution must handle fail-closed. */ export function pruneEmptyLayerGroups(tracks: TimelineTrack[]): TimelineTrack[] { - const populatedGroupIds = new Set( - tracks - .filter((track) => !track.isGroup && track.parentTrackId) - .map((track) => track.parentTrackId as string), - ) - - const nextTracks = tracks.filter((track) => !track.isGroup || populatedGroupIds.has(track.id)) + const retainedGroupIds = collectRetainedGroupIds(tracks, indexLayerGroups(tracks)) + const nextTracks = tracks.filter((track) => !track.isGroup || retainedGroupIds.has(track.id)) return nextTracks.length === tracks.length ? tracks : nextTracks } From 861d348f38886a02ba8982afea760e5a350c67cd Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 21:45:38 -0700 Subject: [PATCH 5/5] fix(timeline): fail closed on malformed track hierarchy --- .../item-edit-actions.lock-invariants.test.ts | 119 +++++++++++++++++ .../items-store.track-hierarchy.test.ts | 122 +++++++++++++++++- .../timeline/utils/group-utils.test.ts | 94 ++++++++++++++ src/features/timeline/utils/group-utils.ts | 114 ++++++++++------ 4 files changed, 406 insertions(+), 43 deletions(-) diff --git a/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts b/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts index e0640f809..467468bbd 100644 --- a/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts +++ b/src/features/timeline/stores/actions/item-edit-actions.lock-invariants.test.ts @@ -30,6 +30,7 @@ import { trimItemStart, } from './item-edit-actions' import { updateItem } from './item-actions' +import { preflightTimelineMutation } from '../../utils/track-lock-invariants' function tracks(overrides: Partial = {}): TimelineTrack[] { return [ @@ -144,6 +145,124 @@ describe('public item edit lock preflights', () => { expectUnchanged(before) }) + it.each([ + [ + 'mixed group/lane duplicate id', + [ + makeTimelineTrack({ + id: 'video-track', + name: 'Ambiguous group', + order: 0, + isGroup: true, + }), + makeTimelineTrack({ + id: 'video-track', + name: 'Ambiguous lane', + kind: 'video', + order: 1, + }), + ], + ], + [ + 'duplicate groups', + [ + makeTimelineTrack({ id: 'group', name: 'Group A', order: 0, isGroup: true }), + makeTimelineTrack({ id: 'group', name: 'Group B', order: 1, isGroup: true }), + makeTimelineTrack({ + id: 'video-track', + name: 'Lane', + kind: 'video', + order: 2, + parentTrackId: 'group', + }), + ], + ], + [ + 'duplicate lanes', + [ + makeTimelineTrack({ id: 'video-track', name: 'Lane A', kind: 'video', order: 0 }), + makeTimelineTrack({ id: 'video-track', name: 'Lane B', kind: 'video', order: 1 }), + ], + ], + [ + 'missing parent', + [ + makeTimelineTrack({ + id: 'video-track', + name: 'Lane', + kind: 'video', + order: 0, + parentTrackId: 'missing', + }), + ], + ], + [ + 'non-group parent', + [ + makeTimelineTrack({ id: 'ordinary-parent', name: 'V1', kind: 'video', order: 0 }), + makeTimelineTrack({ + id: 'video-track', + name: 'V2', + kind: 'video', + order: 1, + parentTrackId: 'ordinary-parent', + }), + ], + ], + [ + 'self-parent', + [ + makeTimelineTrack({ + id: 'video-track', + name: 'Lane', + kind: 'video', + order: 0, + parentTrackId: 'video-track', + }), + ], + ], + [ + 'multi-node cycle', + [ + makeTimelineTrack({ + id: 'group-a', + name: 'Group A', + order: 0, + isGroup: true, + parentTrackId: 'group-b', + }), + makeTimelineTrack({ + id: 'group-b', + name: 'Group B', + order: 1, + isGroup: true, + parentTrackId: 'group-a', + }), + makeTimelineTrack({ + id: 'video-track', + name: 'Lane', + kind: 'video', + order: 2, + parentTrackId: 'group-a', + }), + ], + ], + ] as const)('rejects trim atomically for malformed ancestry: %s', (_name, malformedTracks) => { + useItemsStore.getState().setTracks([...malformedTracks]) + useItemsStore.getState().setItems([video()]) + useSelectionStore.getState().selectItems(['middle']) + const before = snapshot() + const { items, tracks: storedTracks } = useItemsStore.getState() + + expect( + preflightTimelineMutation({ items, tracks: storedTracks, itemIds: ['middle'] }), + ).toMatchObject({ allowed: false, allowedIds: [], blockedIds: ['middle'] }) + + trimItemStart('middle', 10) + + expectUnchanged(before) + }) + it.each([true, false])( 'rejects the live-QA linked A/V trim and split when linked selection is %s', (linkedSelectionEnabled) => { diff --git a/src/features/timeline/stores/items-store.track-hierarchy.test.ts b/src/features/timeline/stores/items-store.track-hierarchy.test.ts index 317de4fd2..5085574a2 100644 --- a/src/features/timeline/stores/items-store.track-hierarchy.test.ts +++ b/src/features/timeline/stores/items-store.track-hierarchy.test.ts @@ -1,9 +1,10 @@ // @vitest-environment node import { beforeEach, describe, expect, it } from 'vite-plus/test' -import { makeTimelineTrack } from '../test-helpers' +import { makeTimelineTrack, makeTimelineVideoItem } from '../test-helpers' import { useItemsStore } from './items-store' import { resolveEffectiveTrackStates } from '../utils/group-utils' +import { preflightTimelineMutation } from '../utils/track-lock-invariants' function storedTrackIds(): string[] { return useItemsStore.getState().tracks.map((track) => track.id) @@ -82,6 +83,50 @@ describe('items-store track hierarchy normalization', () => { ]) }) + it('preserves a valid unlocked grandparent-to-group-to-lane ancestry after setTracks', () => { + const item = makeTimelineVideoItem({ id: 'item', trackId: 'lane' }) + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'grandparent', + name: 'Grandparent', + order: 0, + isGroup: true, + }), + makeTimelineTrack({ + id: 'group', + name: 'Group', + order: 1, + isGroup: true, + parentTrackId: 'grandparent', + }), + makeTimelineTrack({ + id: 'lane', + name: 'Lane', + kind: 'video', + order: 2, + parentTrackId: 'group', + }), + ]) + useItemsStore.getState().setItems([item]) + + const { items, tracks } = useItemsStore.getState() + expect(storedTrackIds()).toEqual(['grandparent', 'group', 'lane']) + expect(resolveEffectiveTrackStates(tracks)).toEqual([ + expect.objectContaining({ + id: 'lane', + locked: false, + muted: false, + visible: true, + solo: false, + }), + ]) + expect(preflightTimelineMutation({ items, tracks, itemIds: [item.id] })).toMatchObject({ + allowed: true, + allowedIds: [item.id], + blockedIds: [], + }) + }) + it('retains orphan lanes and resolves their missing ancestry fail-closed', () => { useItemsStore.getState().setTracks([ makeTimelineTrack({ @@ -181,4 +226,79 @@ describe('items-store track hierarchy normalization', () => { }), ]) }) + + it('retains a mixed group/lane duplicate id so setTracks cannot unlock the lane', () => { + const item = makeTimelineVideoItem({ id: 'item', trackId: 'mixed' }) + useItemsStore + .getState() + .setTracks([ + makeTimelineTrack({ id: 'mixed', name: 'Mixed group', order: 0, isGroup: true }), + makeTimelineTrack({ id: 'mixed', name: 'Mixed lane', kind: 'video', order: 1 }), + ]) + useItemsStore.getState().setItems([item]) + + const { items, tracks } = useItemsStore.getState() + expect(tracks.map((track) => ({ id: track.id, isGroup: track.isGroup === true }))).toEqual([ + { id: 'mixed', isGroup: true }, + { id: 'mixed', isGroup: false }, + ]) + expect(resolveEffectiveTrackStates(tracks)).toEqual([ + expect.objectContaining({ id: 'mixed', locked: true, muted: true, visible: false }), + ]) + expect(preflightTimelineMutation({ items, tracks, itemIds: [item.id] })).toMatchObject({ + allowed: false, + allowedIds: [], + blockedIds: [item.id], + }) + }) + + it('keeps duplicate lanes fail-closed through setTracks and mutation preflight', () => { + const item = makeTimelineVideoItem({ id: 'item', trackId: 'duplicate-lane' }) + useItemsStore.getState().setTracks([ + makeTimelineTrack({ + id: 'duplicate-lane', + name: 'Duplicate lane A', + kind: 'video', + order: 0, + }), + makeTimelineTrack({ + id: 'duplicate-lane', + name: 'Duplicate lane B', + kind: 'video', + order: 1, + }), + ]) + useItemsStore.getState().setItems([item]) + + const { items, tracks } = useItemsStore.getState() + expect(storedTrackIds()).toEqual(['duplicate-lane', 'duplicate-lane']) + expect(resolveEffectiveTrackStates(tracks)).toEqual([ + expect.objectContaining({ id: 'duplicate-lane', locked: true, visible: false }), + expect.objectContaining({ id: 'duplicate-lane', locked: true, visible: false }), + ]) + expect(preflightTimelineMutation({ items, tracks, itemIds: [item.id] }).allowed).toBe(false) + }) + + it('fails a child lane closed when its parent id belongs to a non-group lane', () => { + const item = makeTimelineVideoItem({ id: 'item', trackId: 'child' }) + useItemsStore.getState().setTracks([ + makeTimelineTrack({ id: 'ordinary-parent', name: 'V1', kind: 'video', order: 0 }), + makeTimelineTrack({ + id: 'child', + name: 'V2', + kind: 'video', + order: 1, + parentTrackId: 'ordinary-parent', + }), + ]) + useItemsStore.getState().setItems([item]) + + const { items, tracks } = useItemsStore.getState() + expect(storedTrackIds()).toEqual(['ordinary-parent', 'child']) + expect(resolveEffectiveTrackStates(tracks)).toEqual([ + expect.objectContaining({ id: 'ordinary-parent', locked: false, visible: true }), + expect.objectContaining({ id: 'child', locked: true, muted: true, visible: false }), + ]) + expect(preflightTimelineMutation({ items, tracks, itemIds: [item.id] }).allowed).toBe(false) + }) }) diff --git a/src/features/timeline/utils/group-utils.test.ts b/src/features/timeline/utils/group-utils.test.ts index f1a957fc1..8800e15b2 100644 --- a/src/features/timeline/utils/group-utils.test.ts +++ b/src/features/timeline/utils/group-utils.test.ts @@ -179,6 +179,46 @@ describe('group-utils', () => { }) }) + it('fails closed for every duplicate lane definition', () => { + const effectiveTracks = resolveEffectiveTrackStates([ + makeTrack({ id: 'duplicate-lane', name: 'Duplicate lane A', order: 0 }), + makeTrack({ id: 'duplicate-lane', name: 'Duplicate lane B', order: 1 }), + ]) + + expect(effectiveTracks).toHaveLength(2) + for (const effectiveTrack of effectiveTracks) { + expect(effectiveTrack).toMatchObject({ + id: 'duplicate-lane', + locked: true, + muted: true, + visible: false, + solo: false, + }) + } + }) + + it('fails only the child closed when its parent id resolves to an ordinary lane', () => { + const [ordinaryParent, malformedChild] = resolveEffectiveTrackStates([ + makeTrack({ id: 'ordinary-parent', order: 0 }), + makeTrack({ id: 'child', order: 1, parentTrackId: 'ordinary-parent', solo: true }), + ]) + + expect(ordinaryParent).toMatchObject({ + id: 'ordinary-parent', + locked: false, + muted: false, + visible: true, + solo: false, + }) + expect(malformedChild).toMatchObject({ + id: 'child', + locked: true, + muted: true, + visible: false, + solo: false, + }) + }) + it('uses propagated visibility when collecting visible track ids', () => { const visibleTrackIds = getVisibleTrackIds([ makeTrack({ id: 'group-1', isGroup: true, visible: false }), @@ -234,6 +274,60 @@ describe('group-utils', () => { ]) }) + it('retains duplicate definitions and their possible ancestors without reordering lanes', () => { + const ancestor = makeTrack({ id: 'ancestor', isGroup: true }) + const mixedGroup = makeTrack({ id: 'mixed', isGroup: true, parentTrackId: ancestor.id }) + const mixedLane = makeTrack({ id: 'mixed', order: 2 }) + const duplicateGroupA = makeTrack({ id: 'duplicate-group', isGroup: true, order: 3 }) + const duplicateGroupB = makeTrack({ id: 'duplicate-group', isGroup: true, order: 4 }) + const duplicateLaneA = makeTrack({ id: 'duplicate-lane', order: 5 }) + const emptyGroup = makeTrack({ id: 'empty-group', isGroup: true, order: 6 }) + const unrelatedLane = makeTrack({ id: 'unrelated-lane', order: 7 }) + const duplicateLaneB = makeTrack({ id: 'duplicate-lane', order: 8 }) + + expect( + pruneEmptyLayerGroups([ + mixedLane, + emptyGroup, + ancestor, + duplicateGroupA, + unrelatedLane, + mixedGroup, + duplicateLaneA, + duplicateGroupB, + duplicateLaneB, + ]), + ).toEqual([ + mixedLane, + ancestor, + duplicateGroupA, + unrelatedLane, + mixedGroup, + duplicateLaneA, + duplicateGroupB, + duplicateLaneB, + ]) + }) + + it('does not erase an empty duplicate lane before pruning its mixed-id hierarchy', () => { + const outer = makeTrack({ id: 'outer', isGroup: true, order: 0 }) + const mixedGroup = makeTrack({ + id: 'mixed', + isGroup: true, + order: 1, + parentTrackId: outer.id, + }) + const emptyMixedLane = makeTrack({ id: 'mixed', order: 2, parentTrackId: outer.id }) + const populatedChild = makeTrack({ id: 'child', order: 3, parentTrackId: mixedGroup.id }) + + expect( + pruneEmptyLayerGroupHierarchy( + [outer, mixedGroup, emptyMixedLane, populatedChild], + [{ trackId: populatedChild.id }], + ), + ).toEqual([outer, mixedGroup, emptyMixedLane, populatedChild]) + }) + it('prunes empty child lanes without removing empty top-level classic tracks', () => { const group = makeTrack({ id: 'group', isGroup: true }) const populatedChild = makeTrack({ id: 'child-populated', parentTrackId: group.id }) diff --git a/src/features/timeline/utils/group-utils.ts b/src/features/timeline/utils/group-utils.ts index 605439b49..378d874ff 100644 --- a/src/features/timeline/utils/group-utils.ts +++ b/src/features/timeline/utils/group-utils.ts @@ -11,33 +11,64 @@ export function getVisibleTrackIds(tracks: TimelineTrack[]): Set { ) } -function indexLayerGroups(tracks: TimelineTrack[]): Map { - const groupsById = new Map() +interface TrackHierarchyIndex { + tracksById: Map + trackById: Map + duplicateTrackIds: Set +} + +function indexTrackHierarchy(tracks: TimelineTrack[]): TrackHierarchyIndex { + const tracksById = new Map() + const trackById = new Map() + const duplicateTrackIds = new Set() + for (const track of tracks) { - if (!track.isGroup) continue - const definitions = groupsById.get(track.id) - if (definitions) definitions.push(track) - else groupsById.set(track.id, [track]) + const definitions = tracksById.get(track.id) + if (definitions) { + definitions.push(track) + duplicateTrackIds.add(track.id) + } else { + tracksById.set(track.id, [track]) + trackById.set(track.id, track) + } } - return groupsById + + return { tracksById, trackById, duplicateTrackIds } +} + +function collectRetainedGroupRootIds( + tracks: TimelineTrack[], + hierarchy: TrackHierarchyIndex, +): Set { + const retainedGroupIds = new Set(hierarchy.duplicateTrackIds) + for (const track of tracks) { + if (!track.isGroup && track.parentTrackId) retainedGroupIds.add(track.parentTrackId) + } + return retainedGroupIds +} + +function retainGroupParent( + track: TimelineTrack, + retainedGroupIds: Set, + pendingGroupIds: string[], +): void { + const parentId = track.isGroup ? track.parentTrackId : undefined + if (!parentId || retainedGroupIds.has(parentId)) return + retainedGroupIds.add(parentId) + pendingGroupIds.push(parentId) } function collectRetainedGroupIds( tracks: TimelineTrack[], - groupsById: Map, + hierarchy: TrackHierarchyIndex, ): Set { - const retainedGroupIds = new Set( - tracks.flatMap((track) => (!track.isGroup && track.parentTrackId ? [track.parentTrackId] : [])), - ) + const retainedGroupIds = collectRetainedGroupRootIds(tracks, hierarchy) const pendingGroupIds = [...retainedGroupIds] for (let index = 0; index < pendingGroupIds.length; index += 1) { - const definitions = groupsById.get(pendingGroupIds[index]!) ?? [] - for (const group of definitions) { - const parentId = group.parentTrackId - if (!parentId || retainedGroupIds.has(parentId)) continue - retainedGroupIds.add(parentId) - pendingGroupIds.push(parentId) + const definitions = hierarchy.tracksById.get(pendingGroupIds[index]!) ?? [] + for (const track of definitions) { + retainGroupParent(track, retainedGroupIds, pendingGroupIds) } } @@ -51,13 +82,14 @@ function collectRetainedGroupIds( * its own, so retaining a branch with no lane only leaves orphaned UI rows. * Starting from every non-group lane makes the traversal independent of input * order and retains its complete group ancestry. Missing parents terminate a - * branch, while visited IDs make self/multi-node cycles finite. If an ID has - * duplicate group definitions, all of them and all of their possible parents - * are retained so normalization does not erase the ambiguity that effective - * state resolution must handle fail-closed. + * branch, while visited IDs make self/multi-node cycles finite. Every duplicate + * ID is also a root: group/group and mixed group/lane definitions, plus every + * possible group ancestor, remain in place so normalization cannot sanitize an + * ambiguous topology into unlocked authorization. Lane/lane duplicates already + * survive because pruning never removes ordinary lanes. */ export function pruneEmptyLayerGroups(tracks: TimelineTrack[]): TimelineTrack[] { - const retainedGroupIds = collectRetainedGroupIds(tracks, indexLayerGroups(tracks)) + const retainedGroupIds = collectRetainedGroupIds(tracks, indexTrackHierarchy(tracks)) const nextTracks = tracks.filter((track) => !track.isGroup || retainedGroupIds.has(track.id)) return nextTracks.length === tracks.length ? tracks : nextTracks } @@ -71,9 +103,14 @@ export function pruneEmptyLayerGroupHierarchy( tracks: TimelineTrack[], items: ReadonlyArray>, ): TimelineTrack[] { + const hierarchy = indexTrackHierarchy(tracks) const populatedTrackIds = new Set(items.map((item) => item.trackId)) const tracksWithPopulatedGroupChildren = tracks.filter( - (track) => track.isGroup || !track.parentTrackId || populatedTrackIds.has(track.id), + (track) => + track.isGroup || + hierarchy.duplicateTrackIds.has(track.id) || + !track.parentTrackId || + populatedTrackIds.has(track.id), ) return pruneEmptyLayerGroups(tracksWithPopulatedGroupChildren) @@ -115,8 +152,7 @@ function inheritTrackState( function collectTrackResolutionPath(params: { track: TimelineTrack - trackById: Map - duplicateTrackIds: Set + hierarchy: TrackHierarchyIndex stateById: Map }): { path: TimelineTrack[]; inheritedState: EffectiveTrackState } { const path: TimelineTrack[] = [] @@ -126,7 +162,7 @@ function collectTrackResolutionPath(params: { while (cursor) { const cached = params.stateById.get(cursor.id) if (cached) return { path, inheritedState: cached } - if (params.duplicateTrackIds.has(cursor.id)) { + if (params.hierarchy.duplicateTrackIds.has(cursor.id)) { return { path, inheritedState: MALFORMED_TRACK_STATE } } if (pathIds.has(cursor.id)) return { path, inheritedState: MALFORMED_TRACK_STATE } @@ -135,8 +171,9 @@ function collectTrackResolutionPath(params: { path.push(cursor) if (!cursor.parentTrackId) return { path, inheritedState: ROOT_TRACK_STATE } - cursor = params.trackById.get(cursor.parentTrackId) - if (!cursor) return { path, inheritedState: MALFORMED_TRACK_STATE } + const parent = params.hierarchy.trackById.get(cursor.parentTrackId) + if (!parent?.isGroup) return { path, inheritedState: MALFORMED_TRACK_STATE } + cursor = parent } return { path, inheritedState: MALFORMED_TRACK_STATE } @@ -144,13 +181,12 @@ function collectTrackResolutionPath(params: { function resolveTrackState(params: { track: TimelineTrack - trackById: Map - duplicateTrackIds: Set + hierarchy: TrackHierarchyIndex stateById: Map }): EffectiveTrackState { const cached = params.stateById.get(params.track.id) if (cached) return cached - if (params.duplicateTrackIds.has(params.track.id)) return MALFORMED_TRACK_STATE + if (params.hierarchy.duplicateTrackIds.has(params.track.id)) return MALFORMED_TRACK_STATE const { path, inheritedState } = collectTrackResolutionPath(params) let state = inheritedState @@ -171,24 +207,18 @@ function resolveTrackState(params: { * resolved fail-closed so every consumer sees the lane as locked, muted, and * hidden rather than making a different partial guess. Solo is disabled for * malformed ancestry because promoting an invalid lane into the solo set - * would make it more audible/visible, not less. + * would make it more audible/visible, not less. Duplicate IDs of every shape, + * missing parents, non-group parents, self-parenting, and longer cycles are all + * malformed. Only a unique chain of Layer Group parents may authorize a lane. */ export function resolveEffectiveTrackStates(tracks: TimelineTrack[]): TimelineTrack[] { - const trackById = new Map() - const duplicateTrackIds = new Set() - for (const track of tracks) { - if (trackById.has(track.id)) { - duplicateTrackIds.add(track.id) - } else { - trackById.set(track.id, track) - } - } + const hierarchy = indexTrackHierarchy(tracks) const stateById = new Map() return tracks .filter((track) => !track.isGroup) .map((track) => { - const state = resolveTrackState({ track, trackById, duplicateTrackIds, stateById }) + const state = resolveTrackState({ track, hierarchy, stateById }) if ( state.locked === track.locked && state.muted === track.muted &&