fix(runtime): add setter to delegated __player properties

[miguel-heygen]

Summary

• Adds a set accessor to the Object.defineProperty delegation on window.__player methods
• Adds regression test verifying __player.renderSeek can be reassigned after init

Problem

The property delegation on window.__player (init.ts:1956) used Object.defineProperty with only a get accessor. When Studio's motion-wrapping code tried to reassign __player.renderSeek with a wrapped version, it threw:

Uncaught TypeError: Cannot set property renderSeek of [object Object] which has only a getter

This cascaded into an infinite error loop — the timeline kept "dancing" (being modified without user input), the preview stuck on "Loading composition", and the console filled with hundreds of repeated errors.

Root cause

// Before — getter only, no setter
Object.defineProperty(playerApi, key, {
  get: () => player[key],
  configurable: true,
});

Studio's motion wrapper reads __player.renderSeek, wraps it with additional behavior, then tries to SET the wrapped version back. The missing setter caused this assignment to throw in strict mode.

Fix

// After — getter + setter proxy
Object.defineProperty(playerApi, key, {
  get: () => player[key],
  set: (v) => { player[key] = v; },
  configurable: true,
});

The setter proxies assignments back to the internal player object, so the delegation pattern still works (reads go through player, writes update player).

Test plan

• New regression test: allows external code to reassign delegated __player methods
• All 6 init tests pass
• All 29 player tests pass
• Lint + typecheck + commitlint pass

🤖 Generated with Claude Code

[jrusso1020]

Tiny clean fix at the right architectural layer. The Object.defineProperty getter-only pattern was a runtime trap waiting for any external code to try the natural JavaScript reassignment pattern (window.__player.renderSeek = wrapped). Studio's motion wrapper triggered it. Approving.

What I verified

The delegation list is uniform — all 7 methods get the new setter:

const delegated = [
  "play",
  "pause",
  "seek",
  "renderSeek",
  "getTime",
  "getDuration",
  "isPlaying",
] as const;
for (const key of delegated) {
  Object.defineProperty(playerApi, key, {
    get: () => player[key],
    set: (v: unknown) => {
      (player as Record<string, unknown>)[key] = v;
    },
    configurable: true,
  });
}

So external code can wrap any of play, pause, seek, renderSeek, getTime, getDuration, isPlaying. Studio's motion wrap pattern (read original, wrap with side-effect, reassign) now works for all of them.

The fix preserves the delegation invariant. Reads go through player[key], writes also mutate player[key]. So:

• External read of playerApi.renderSeek returns whatever's currently on player.renderSeek (wrapped or original)
• External write to playerApi.renderSeek = wrapped makes ALL future reads (external AND internal) see the wrapped version

That's the right behavior for the wrap-once-on-init pattern Studio's installStudioMotionSeekReapply uses — once wrapped, all callers (the runtime's own seek paths, the studio UI's scrub path, the postMessage bridge) see the wrap. Single mutation point, all consumers consistent.

Test pins the regression case:

const original = player.renderSeek;
expect(() => {
  player.renderSeek = (t: number) => original(t);
}).not.toThrow();

That's exactly the studio motion-wrap pattern (read → close over original → reassign). The "doesn't throw" assertion catches the missing-setter regression specifically. ✓

Type-safety trade-off is acceptable. The (player as Record<string, unknown>)[key] = v cast bypasses property-type checking on the assignment. Same shape as the existing get: () => player[key] (which similarly punts on per-key types). Since delegated is as const, TypeScript could in principle enforce per-key types — but the cost-benefit isn't worth it for an init-time setup. Acceptable.

CI: Lint, Test, Typecheck, Build, Test: runtime contract, all Perf:* jobs (drift, parity, scrub, fps, load), preview-regression, Preview parity, CodeQL, player-perf all ✓ green at 711ac22f. Tests on windows-latest, Render on windows-latest, regression-shards (visual), Smoke: global install, CLI smoke (required) still in progress — visual regression shards are slow-running and unrelated to runtime/property-descriptor changes. No blockers visible.

Connection to prior PRs

This is the runtime fix that makes hf#673's installStudioMotionSeekReapply (in packages/core/src/components/editor/studioMotion.ts:626) actually work at runtime:

const wrapPlayerMethod = (key: "renderSeek" | "seek") => {
    const original = player[key];
    if (typeof original !== "function") return;
    player[key] = (time: number) => {  // ← would throw without this PR's setter
      original.call(player, time);
      studioWin.__hfStudioMotionApply?.();
    };
};

I approved hf#673 round 1 without tracing this call path back to the runtime's property descriptor — that was the kind of cross-package miss my own audit-paths discipline warns against. Vai caught the seek-while-playing pause-coherency bug on that PR, but neither of us flagged that the underlying wrap mechanism would throw. This PR is the downstream fix for that miss.

Worth noting for the trail: any future "studio code wraps a runtime method" pattern needs the property to have a setter — the runtime's delegation list is the source of truth for which methods are wrappable.

Smaller observation (non-blocking)

The error message in the PR description (Cannot set property renderSeek of [object Object] which has only a getter) is V8's strict-mode message. In non-strict execution contexts the assignment would have silently failed instead of throwing. The cascading "infinite error loop" — timeline dancing, preview stuck on "Loading composition", console floods — implies the studio iframe runs in strict mode (which it should, given ES modules default to strict). Worth a one-line check that the runtime always evaluates in strict mode so this kind of silent-fail can't replace the loud throw on a future config change. Probably implicit via TypeScript module emission, just noting.

Praise

• Two-line fix at the source. The right shape — fix the property descriptor, don't add escape hatches in callers. Once this lands, every current and future caller of the wrap pattern works without coordination.
• Test is the exact reproducer of the failure mode. Not a synthetic "test that property descriptors can be set" — but the actual studio-shape "read original, wrap, reassign" pattern that triggered the bug. Future regressions in the same shape get caught immediately.
• Uniform fix across the entire delegation list. Not just renderSeek (the one that surfaced the bug) but all 7 methods. Studio (or anyone else) can wrap any of them with the same pattern.
• PR description traces the cascade clearly: missing setter → throw on assignment → studio's motion wrap fails → timeline state corrupt → preview stuck → error loop. That's the kind of root-cause analysis that gives the next debugger of "preview stuck on Loading composition" a fast trail.

Summary

Two-line fix at the right layer, regression test pins the studio-shape pattern, CI green on all the non-visual-regression jobs. Approved at 711ac22f. Ship it.

(Note: this fixes a downstream miss from hf#673 that I should have caught — my approval there didn't trace installStudioMotionSeekReapply's wrap mechanism back to the runtime's property descriptor.)

— Review by Rames Jusso (pr-review)

[vanceingalls]

Verdict: Approve. Surgical, well-justified fix with a regression test. Root cause analysis is correct and the implementation matches it.

Blockers

None.

Important

None.

Nits

• packages/core/src/runtime/init.test.ts:286-313 — the test only asserts that assignment doesn't throw. Worth one extra assertion to lock in the delegation contract: after player.renderSeek = wrapped, a subsequent read of player.renderSeek should return wrapped (proves the setter actually wrote to the underlying player object and the getter sees it), and ideally that internal callers route through the wrapped version. The current test would still pass even if the setter was a no-op set: () => {}. Not blocking — the immediate regression is covered.
• packages/core/src/runtime/init.ts:1958-1960 — the setter accepts unknown with no validation. If a caller assigns a non-function (__player.renderSeek = "foo"), the failure shifts from synchronous-on-set (old behavior) to deferred-on-invocation (new behavior). Acceptable trade-off for a delegation proxy targeted at a known consumer pattern (Studio motion wrapper), but worth noting.
• The PR body doesn't call out a behavioral side effect: because internal call sites (player.play(), player.pause(), player.seek(time), player.renderSeek(...) at init.ts:1546-1555 and elsewhere) read through the same player object the setter writes to, they will now also pick up the Studio-wrapped versions. This is almost certainly the intended behavior (you want the wrapped logic to fire whenever the function is called, regardless of caller), but it's a behavior change worth documenting.

Praise

• Property-delegation rationale (lines 1940-1943) explains the why clearly — read/write proxy keeps the live player overrides visible while letting external code wrap.
• Re-init safety preserved: configurable: true + teardown-on-reinit means the descriptor can be redefined cleanly across sessions.
• Audit-all-sites: only one defineProperty-with-getter site exists for __player (this one). The fallback runtime in packages/producer/src/services/fileServer.ts:259 uses plain data props which already accept assignment, so no parallel fix needed there.
• Repro + test land in the same PR. Minimal blast radius (3 lines of prod code). All required CI green.

— Vai