Simplify entry clone back to a spread, cover it with two generic tests

The per-type clone functions from the previous commit listed every field of
every entry type, which was a lot of code for a change meant to be a drop-in
swap. They are replaced by the spread they were written to guard, keeping both
review fixes and the exhaustiveness guard.

The explicit literals were also weaker than they looked. They forced a new
field to be named, but nothing stopped it being named as a plain alias, which
is the actual bug we care about. The guarantee now comes from a test that
walks a clone against its source and fails on any shared nested object or
array. That covers a field added later without the clone enumerating fields,
and it names the path it found:

  entry.triggers.0 is shared with the source

Paired with a field-for-field comparison against structuredClone, the two tests
pin both halves of the contract - same value, no shared references - and both
were confirmed to fail when the corresponding fix is reverted.

Dropping the literals also removes the `parent: undefined` divergence they
introduced on entries that omit the key, since a spread copies exactly the keys
that are present.

Net: the diff for this PR goes from 261 to 159 added lines, with
rundown.utils.ts down from 132 to 52.

apps/server/src/api-data/rundown/__tests__/rundown.utils.test.ts

@@ -1,5 +1,6 @@
 import {
   EndAction,
+  OntimeEntry,
   OntimeEvent,
   OntimeGroup,
   OntimeMilestone,
@@ -8,13 +9,21 @@ import {
   TimerType,
   Trigger,
 } from 'ontime-types';
-import { MILLIS_PER_HOUR, MILLIS_PER_MINUTE, createEvent } from 'ontime-utils';
+import {
+  MILLIS_PER_HOUR,
+  MILLIS_PER_MINUTE,
+  createDelay,
+  createEvent,
+  createGroup,
+  createMilestone,
+} from 'ontime-utils';
 import { assertType } from 'vitest';
 
 import { makeOntimeEvent, makeOntimeGroup, makeOntimeMilestone, makeRundown } from '../__mocks__/rundown.mocks.js';
 import { parseRundown } from '../rundown.parser.js';
 import {
   calculateDayOffset,
+  cloneEntryData,
   deleteById,
   doesInvalidateMetadata,
   getIntegerAndFraction,
@@ -715,3 +724,60 @@ describe('eventDurationMatchGroupTarget()', () => {
     expect(result).toStrictEqual(null);
   });
 });
+
+describe('cloneEntryData()', () => {
+  const trigger: Trigger = {
+    id: 'trigger-1',
+    title: 'Go on air',
+    trigger: TimerLifeCycle.onStart,
+    automationId: 'automation-1',
+  };
+
+  // the real factories, so these are complete entries exactly as the rundown holds them
+  const entries: [string, OntimeEntry][] = [
+    ['event', createEvent({ custom: { sponsor: 'a value' }, triggers: [trigger] }, 'cue-1') as OntimeEvent],
+    ['group', createGroup({ id: 'group-1', entries: ['a', 'b'], custom: { sponsor: 'a value' } })],
+    ['milestone', createMilestone({ id: 'milestone-1', custom: { sponsor: 'a value' } })],
+    ['delay', createDelay({ id: 'delay-1', duration: 10 })],
+  ];
+
+  /**
+   * Fails if any nested object or array in the clone is the same reference as the source,
+   * so a field added later that needs a copy of its own is caught here without the clone
+   * having to enumerate fields.
+   */
+  function expectNoSharedReferences(clone: unknown, source: unknown, path: string) {
+    if (typeof source !== 'object' || source === null) return;
+    expect(clone, `${path} is shared with the source`).not.toBe(source);
+    const cloneRecord = clone as Record<string, unknown>;
+    const sourceRecord = source as Record<string, unknown>;
+    for (const key of Object.keys(sourceRecord)) {
+      expectNoSharedReferences(cloneRecord[key], sourceRecord[key], `${path}.${key}`);
+    }
+  }
+
+  /** the two halves of the structuredClone contract: same value, no shared references */
+  it.each(entries)('clones a %s to the same value structuredClone would produce', (_type, entry) => {
+    expect(cloneEntryData(entry)).toStrictEqual(structuredClone(entry));
+  });
+
+  it.each(entries)('shares no nested object or array with the source %s', (_type, entry) => {
+    expectNoSharedReferences(cloneEntryData(entry), entry, 'entry');
+  });
+
+  /**
+   * Regression: normalising an absent container to an empty one makes deepEqual report a
+   * change on every comparison, which would have the runtime re-broadcast and re-save the
+   * restore point on every tick. See PR #2178.
+   */
+  it('leaves an absent triggers or entries value absent', () => {
+    expect(cloneEntryData(makeOntimeEvent({ id: 'partial' })).triggers).toBeUndefined();
+    expect(cloneEntryData(makeOntimeGroup({ id: 'partial', entries: undefined })).entries).toBeUndefined();
+  });
+
+  it('throws on an entry type it does not know how to clone', () => {
+    expect(() => cloneEntryData({ id: 'x', type: 'unknown' } as unknown as OntimeEvent)).toThrow(
+      'Unsupported entry type for cloning',
+    );
+  });
+});

apps/server/src/api-data/rundown/rundown.dao.ts

@@ -27,6 +27,7 @@ import {
   isPlayableEvent,
 } from 'ontime-types';
 import { addToRundown, createGroup, customFieldLabelToKey, getInsertAfterId, insertAtIndex } from 'ontime-utils';
+import type { DeepReadonly } from 'ts-essentials';
 
 import { getDataProvider } from '../../classes/data-provider/DataProvider.js';
 import { consoleError } from '../../utils/console.js';
@@ -34,6 +35,7 @@ import { ProcessedRundownMetadata, makeRundownMetadata } from './rundown.parser.
 import type { RundownMetadata } from './rundown.types.js';
 import {
   applyPatchToEntry,
+  cloneRundown,
   cloneSimpleRundownEntry,
   deleteById,
   doesInvalidateMetadata,
@@ -79,9 +81,16 @@ export const getRundownMetadata = (): Readonly<RundownMetadata> => rundownMetada
 export const getProjectCustomFields = (): Readonly<CustomFields> => projectCustomFields;
 export const getEntryWithId = (entryId: EntryId): OntimeEntry | undefined => cachedRundown.entries[entryId];
 
-type Transaction = {
+/**
+ * @param R the type callers see for `rundown` - a plain, mutable `Rundown` when the
+ * transaction was opened with `mutableRundown: true`, otherwise a `DeepReadonly<Rundown>`
+ * so that accidentally mutating an entry (or an order array) on a non-mutable transaction
+ * - which would silently corrupt the live cache without going through commit() - is a
+ * compile-time error instead of a runtime bug.
+ */
+type Transaction<R> = {
   customFields: CustomFields;
-  rundown: Rundown;
+  rundown: R;
 
   commit: (shouldProcess?: boolean) => Promise<{
     rundown: Readonly<Rundown>;
@@ -102,11 +111,17 @@ type TransactionOptions = {
   rundownId?: string;
 };
 
-export function createTransaction(options: TransactionOptions): Transaction {
+export function createTransaction(options: TransactionOptions & { mutableRundown: true }): Transaction<Rundown>;
+export function createTransaction(
+  options: TransactionOptions & { mutableRundown?: false },
+): Transaction<DeepReadonly<Rundown>>;
+export function createTransaction(
+  options: TransactionOptions,
+): Transaction<Rundown> | Transaction<DeepReadonly<Rundown>> {
   const targetId = options.rundownId ?? cachedRundown.id;
   const isLoaded = targetId === cachedRundown.id;
   const sourceRundown: Rundown = isLoaded ? cachedRundown : (getDataProvider().getRundown(targetId) as Rundown);
-  const rundown = options.mutableRundown ? structuredClone(sourceRundown) : sourceRundown;
+  const rundown = options.mutableRundown ? cloneRundown(sourceRundown) : sourceRundown;
   const customFields = options.mutableCustomFields ? structuredClone(projectCustomFields) : projectCustomFields;
 
   /**
@@ -707,7 +722,7 @@ export const customFieldMutation = {
  * Expose function to add an initial rundown to the system
  */
 export function init(initialRundown: Readonly<Rundown>, initialCustomFields: Readonly<CustomFields>) {
-  const rundown = structuredClone(initialRundown);
+  const rundown = cloneRundown(initialRundown);
   const customFields = structuredClone(initialCustomFields);
   const processedData = processRundown(rundown, customFields, { mutate: true });
 

apps/server/src/api-data/rundown/rundown.parser.ts

@@ -33,7 +33,7 @@ import {
 import { makeNewRundown } from '../../models/dataModel.js';
 import type { ErrorEmitter } from '../../utils/parserUtils.js';
 import { RundownMetadata } from './rundown.types.js';
-import { calculateDayOffset, cleanupCustomFields } from './rundown.utils.js';
+import { calculateDayOffset, cleanupCustomFields, cloneEntryData } from './rundown.utils.js';
 
 /**
  * Parse a rundowns object along with the project custom fields
@@ -234,7 +234,7 @@ export function makeRundownMetadata(customFields: CustomFields, options?: { muta
   };
 
   function process<T extends OntimeEntry>(entry: T, childOfGroup: EntryId | null): T {
-    return processEntry(rundownMeta, customFields, mutate ? entry : structuredClone(entry), childOfGroup);
+    return processEntry(rundownMeta, customFields, mutate ? entry : cloneEntryData(entry), childOfGroup);
   }
 
   function getMetadata(): ProcessedRundownMetadata {

apps/server/src/api-data/rundown/rundown.service.ts

@@ -39,6 +39,7 @@ import {
 import { parseRundown } from './rundown.parser.js';
 import type { RundownMetadata } from './rundown.types.js';
 import {
+  cloneRundown,
   generateEvent,
   getFirstInsertId,
   getIntegerAndFraction,
@@ -626,7 +627,7 @@ export async function editCustomField(
     // ... reassign references in the background rundowns
     for (const rundownId of Object.keys(projectRundowns)) {
       if (rundownId !== rundown.id) {
-        const backgroundRundown = structuredClone(projectRundowns[rundownId]);
+        const backgroundRundown = cloneRundown(projectRundowns[rundownId]);
         customFieldMutation.renameUsages(backgroundRundown, oldKey, newKey);
         await updateBackgroundRundown(rundownId, backgroundRundown);
       }
@@ -666,7 +667,7 @@ export async function deleteCustomField(key: CustomFieldKey, projectRundowns: Pr
   // remove references in the background rundowns
   for (const rundownId of Object.keys(projectRundowns)) {
     if (rundownId !== rundown.id) {
-      const backgroundRundown = structuredClone(projectRundowns[rundownId]);
+      const backgroundRundown = cloneRundown(projectRundowns[rundownId]);
       customFieldMutation.removeUsages(backgroundRundown, key);
       await updateBackgroundRundown(rundownId, backgroundRundown);
     }
@@ -846,7 +847,7 @@ export async function duplicateExistingRundown(id: string) {
   const dataProvider = getDataProvider();
   const rundown = dataProvider.getRundown(id);
 
-  const duplicatedRundown: Rundown = structuredClone(rundown);
+  const duplicatedRundown: Rundown = cloneRundown(rundown);
   duplicatedRundown.id = generateId();
   duplicatedRundown.title = `Copy of ${rundown.title}`;
   duplicatedRundown.revision = 0;

apps/server/src/api-data/rundown/rundown.utils.ts

@@ -329,7 +329,7 @@ export function mergeRundownPreservingFields(
     const structure = isOntimeGroup(incomingEntry)
       ? { entries: incomingEntry.entries }
       : { parent: incomingEntry.parent };
-    entries[id] = structuredClone({ ...merged, ...structure });
+    entries[id] = cloneEntryData({ ...merged, ...structure });
   }
 
   return {
@@ -499,6 +499,56 @@ export function cloneSimpleRundownEntry(entry: OntimeEntry, newId: EntryId): Ont
   throw new Error(`Unsupported entry type for cloning: ${entry}`);
 }
 
+/**
+ * Fast, shape-aware clone of a single entry, preserving its identity (id, revision, etc).
+ * Drop-in replacement for `structuredClone(entry)`, at a fraction of the cost: the spread
+ * already copies every primitive field by value, so only the nested containers need work.
+ * `custom` and `entries` hold primitives, so copying the container is enough. `triggers`
+ * holds objects, which are copied too - otherwise a mutable transaction could edit a trigger
+ * of the cached rundown before commit.
+ * An absent container is left absent rather than normalised to an empty one, so that a clone
+ * still compares equal to the entry it came from.
+ */
+export function cloneEntryData<T extends OntimeEntry>(entry: T): T {
+  switch (entry.type) {
+    case SupportedEntry.Event:
+      return { ...entry, custom: { ...entry.custom }, triggers: entry.triggers?.map((t) => ({ ...t })) } as T;
+    case SupportedEntry.Group:
+      return { ...entry, custom: { ...entry.custom }, entries: entry.entries?.slice() } as T;
+    case SupportedEntry.Milestone:
+      return { ...entry, custom: { ...entry.custom } } as T;
+    case SupportedEntry.Delay:
+      return { ...entry } as T;
+    default: {
+      // exhaustiveness guard: a new member of `SupportedEntry` is named in the error here
+      const unhandled: never = entry;
+      throw new Error(`Unsupported entry type for cloning: ${(unhandled as OntimeEntry).type}`);
+    }
+  }
+}
+
+/**
+ * Fast, shape-aware clone of a whole rundown.
+ * Drop-in replacement for `structuredClone(rundown)`: every entry (and its nested
+ * `custom` / `triggers` / `entries` containers) gets its own copy, so callers can mutate
+ * the result freely without touching the source - same contract as structuredClone,
+ * at a fraction of the cost since we skip the generic serialization algorithm.
+ */
+export function cloneRundown(rundown: Readonly<Rundown>): Rundown {
+  const entries: RundownEntries = {};
+  for (const id in rundown.entries) {
+    entries[id] = cloneEntryData(rundown.entries[id]);
+  }
+  return {
+    id: rundown.id,
+    title: rundown.title,
+    revision: rundown.revision,
+    order: rundown.order.slice(),
+    flatOrder: rundown.flatOrder.slice(),
+    entries,
+  };
+}
+
 /**
  * Utility for calculating if the current events should have a day offset
  * @param current the current event under test

apps/server/src/api-data/sheets/sheets.service.ts

@@ -464,9 +464,8 @@ export async function upload(sheetId: string, options: ImportMap) {
     sheetOrder.forEach((entryId, index) => {
       const isGroupEnd = entryId.startsWith('group-end-');
       const id = isGroupEnd ? entryId.split('group-end-')[1] : entryId;
-      const entry = isGroupEnd
+      // cellRequestFromEvent only reads the entry to build a cell request, no clone is needed
-        ? ({ id: entryId, type: SupportedEntry.Group } as OntimeGroup)
+      const entry = isGroupEnd ? ({ id: entryId, type: SupportedEntry.Group } as OntimeGroup) : rundown.entries[id];
-        : structuredClone(rundown.entries[id]);
       updateRundown.push(cellRequestFromEvent(entry, index, worksheetId, sheetMetadata));
     });
   } catch (e) {

apps/server/src/classes/data-provider/DataProvider.ts

@@ -12,6 +12,7 @@ import {
   ViewSettings,
 } from 'ontime-types';
 
+import { cloneRundown } from '../../api-data/rundown/rundown.utils.js';
 import { isTest } from '../../setup/environment.js';
 import { shouldCrashDev } from '../../utils/development.js';
 import { isPath } from '../../utils/fileManagement.js';
@@ -102,7 +103,7 @@ function getCustomFields(): Readonly<CustomFields> {
 }
 
 async function setRundown(rundownKey: string, newData: Rundown): ReadonlyPromise<ProjectRundowns> {
-  db.data.rundowns[rundownKey] = structuredClone(newData);
+  db.data.rundowns[rundownKey] = cloneRundown(newData);
   await persist();
   return db.data.rundowns;
 }

apps/server/src/classes/data-provider/DataProvider.utils.ts

@@ -4,12 +4,17 @@ import { DatabaseModel } from 'ontime-types';
  * Merges a partial ontime project into a given ontime project
  */
 export function safeMerge(existing: DatabaseModel, newData: Partial<DatabaseModel>): DatabaseModel {
-  const deepExisting = structuredClone(existing);
+  // rundowns are merged separately below by reference (only the top-level map is copied,
-  const deepNewData = structuredClone(newData);
+  // same as the other properties here) - deep-cloning them here would be wasted work,
+  // since a project's rundowns are by far the largest part of this object
+  const { rundowns: existingRundowns, ...existingRest } = existing;
+  const { rundowns: newRundowns = {}, ...newDataRest } = newData;
+
+  const deepExisting = structuredClone(existingRest);
+  const deepNewData = structuredClone(newDataRest);
 
   // destructure each property to simplify merging not provided ie: ...{} has no effect
   const {
-    rundowns = {},
     project = {},
     settings = {},
     viewSettings = {},
@@ -19,7 +24,7 @@ export function safeMerge(existing: DatabaseModel, newData: Partial<DatabaseMode
   } = deepNewData;
 
   return {
-    rundowns: { ...existing.rundowns, ...rundowns },
+    rundowns: { ...existingRundowns, ...newRundowns },
     project: { ...deepExisting.project, ...project },
     settings: { ...deepExisting.settings, ...settings },
     viewSettings: { ...deepExisting.viewSettings, ...viewSettings },

apps/server/src/services/runtime-service/runtime.service.ts

@@ -20,6 +20,7 @@ import { triggerAutomations } from '../../api-data/automation/automation.service
 import { triggerReportEntry } from '../../api-data/report/report.service.js';
 import { getCurrentRundown, getEntryWithId, getRundownMetadata } from '../../api-data/rundown/rundown.dao.js';
 import { RundownMetadata } from '../../api-data/rundown/rundown.types.js';
+import { cloneEntryData } from '../../api-data/rundown/rundown.utils.js';
 import { logger } from '../../classes/Logger.js';
 import { timerConfig } from '../../setup/config.js';
 import { eventStore } from '../../stores/EventStore.js';
@@ -754,7 +755,7 @@ function broadcastResult(_target: any, _propertyKey: string, descriptor: Propert
       }
       // at this point we know that either the id or the contents has changed
       batch.add(key, currentEntry as RuntimeStore[K]); // we know that there is the necessary overlap in the types to cast this
-      RuntimeService.previousState[key] = structuredClone(currentEntry);
+      RuntimeService.previousState[key] = currentEntry ? cloneEntryData(currentEntry) : null;
       return true;
     }