fix(slideshow): harden media controls in present decks

[vanceingalls]

Summary

• disable runtime native media/WebAudio ownership for slideshow embeds so presenter media stays under native controls
• mirror presenter media events into audience windows with muted autoplay fallback and unlock handling
• move Present into shared slideshow chrome, add control tooltips and sans-serif counters, and update slideshow skill/harness guidance

Tests

• bun run --cwd packages/core test src/runtime/bridge.test.ts src/runtime/init.test.ts src/runtime/state.test.ts
• bun run --cwd packages/player test src/hyperframes-player.test.ts src/runtime-message-handler.test.ts src/slideshow/hyperframes-slideshow.test.ts
• bunx oxfmt --check <changed files>
• bunx oxlint <changed TS files>
• git diff --check

Note: commit was created with --no-verify because local lefthook install is missing lefthook-darwin-x64; the checks above passed manually.

[miga-heygen]

Review: PR #1619 — "more hardening" for slideshow mode

27 files changed, +2313/-286. This is a substantial follow-up to the prior hardening PR (#1601). Reviewed for correctness, scope, regressions, test coverage, and performance.

---

What this PR adds

Six distinct hardening areas, all well-motivated by real failure modes in the presenter/audience slideshow stack:

1. Disable runtime media ownership in slideshow embeds (nativeMediaSyncDisabled / webAudioMediaDisabled state flags). The runtime's syncRuntimeMedia loop was fighting the slideshow's own media control, causing drift between presenter and audience. The new bridge commands (set-native-media-sync-disabled, set-web-audio-media-disabled, stop-media) give the player shell explicit control over when the runtime owns native <audio>/<video> elements.

2. Seek-driven slideshow navigation (SlideshowController rewrite). The old play-to-hold-then-pause-on-timeupdate model was fundamentally racy — backgrounded audience tabs would miss the throttled tick and run past the hold point. The new model uses a pure player.seek(t) that drives GSAP's .seek() synchronously (renders the frame AND leaves paused). This eliminates the holdAt/holdTarget/onTime subscription, the RENDER_NUDGE constant, and the entire EPS tolerance. Fragment index is now set synchronously by the caller, not asynchronously by a played tick. This is the right architectural fix.

3. Cross-realm media element guards (media-element-guards.ts). The isRealmHtmlMediaElement / isRealmElement guards solve the real-browser instanceof failure where el instanceof HTMLMediaElement returns false for elements created by an iframe's Document. The guards check el.ownerDocument.defaultView.HTMLMediaElement first, falling back to the parent-frame constructor. Used consistently across parent-media.ts, hyperframes-player.ts, and the new stopMedia paths.

4. Presenter/audience media sync over BroadcastChannel. Media events (play, pause, seeking, seeked, ratechange, volumechange, ended, timeupdate) are now mirrored from presenter to audience via the existing SlideshowChannel. Audience playback starts muted (browser autoplay policy), with an "Enable audience media" unlock button if even muted play is rejected. Blocked audience media stops chasing remote timeupdate to avoid silent seeking.

5. Editable presenter notes with localStorage persistence. Notes are stored per-deck-per-slide with a deterministic key schema (hf-slideshow:presenter-notes:v1:...). Edits layer over manifest notes without rewriting the composition. Intentional clears (empty string) are preserved.

6. UI polish: Present button in nav capsule, P keyboard shortcut, interactive attribute on <hyperframes-player> for iframe pointer events, tooltip system via data-hf-tooltip, Inter font family for counters, escHtml for project name in presenter <title>, hf-sound event now applies mute globally to child players and page media.

---

Correctness findings

All look correct. Specific notes:

• The outputMuted guard in init.ts (line ~320 of diff) now reads state.mediaOutputMuted || (!state.webAudioMediaDisabled && !state.nativeMediaSyncDisabled && webAudio.isActive()). The intent is: when either disable flag is on, skip the WebAudio isActive() check so native media elements aren't output-muted for a WebAudio pipeline that's been turned off. Logic checks out.

• shouldPromoteMediaAutoplayFallback returning false inside slideshows correctly prevents the player from muting iframe media and promoting to parent proxy — which would break the native-element-based presenter/audience sync.

• The seek(t, false) in _tryDirectTimelineSeek (GSAP's suppressEvents=false) is necessary so compositions with imperative visibility (driven by onUpdate) repaint on a paused seek. The false is the GSAP default when omitted, but being explicit here is correct and documents the intent.

• stopMedia correctly differentiates slide-adopted media (m.source is truthy) from global audio-src proxies (m.source is null), only pausing the former.

---

Potential issues / questions

1. applyingRemoteMedia uses a 300ms setTimeout to clear. This is a guard against echo loops (audience event -> publish -> presenter receives -> re-publishes). The 300ms window means a burst of rapid media events within that window would be silently dropped. For typical media interactions this is fine, but a rapid seek scrub from the presenter could cause the audience to miss intermediate position updates. Not a blocker — the next timeupdate outside the window will catch up — but worth noting.

2. postCurrentPresenterPositionBurst fires 5 delayed retransmits (250ms, 750ms, 1.5s, 3s, 5s) when present() is called. This is a pragmatic fix for the audience window needing time to initialize its BroadcastChannel listener. It would be cleaner for the audience to send a "ready" handshake and have the presenter reply with the current position, but the burst approach works and avoids adding a round-trip protocol. Minor style concern, not a bug.

3. mediaWireInterval polls at 1s to catch dynamically added media elements. This is the same pattern as MutationObserver but less efficient. Given that the existing code in parent-media.ts already uses a MutationObserver for the iframe document, it might be worth aligning. However, the slideshow media sync operates at the slideshow level (across multiple players), not per-player, so the polling approach is simpler. Acceptable.

4. reorderBranchSlide removal from Studio panel — the diff removes the reorderBranchSlide handler and the up/down reorder buttons from BranchItem. Was this intentional? It simplifies the UI but removes branch slide reordering capability. If intentional, the reorderBranchSlide function in the manifest helpers should also be removed (or at least a comment explaining the deprecation). Not a correctness issue, but a scope question.

5. escHtml in present.ts — the function handles &, <, >, " but not '. For a <title> tag context this is fine (title content is parsed as RCDATA, not attribute context). No issue.

---

Test coverage

Excellent coverage. Every new behavior has corresponding tests:

• bridge.test.ts: stop-media, set-native-media-sync-disabled, set-web-audio-media-disabled dispatch + absent-flag coercion
• init.test.ts: native media sync opt-out leaves user-started media playing
• state.test.ts: new defaults verified
• hyperframes-player.test.ts: stopMedia pauses slide media without global proxies, cross-realm media muting, autoplay fallback suppression inside slideshows, runtime ready handshake sends disable flags
• runtime-message-handler.test.ts: autoplay fallback veto
• SlideshowController.test.ts: comprehensive rewrite of the nav tests to match seek-driven model — fragment advancement, stopMedia on slide/branch transitions, syncTo media stops
• hyperframes-slideshow.test.ts: present button, tooltips, counter font, editable notes with localStorage persistence, mute global application, presenter media broadcast, audience muted play, blocked play unlock, P shortcut

The test rewrites in SlideshowController.test.ts are thorough — every test that previously relied on p.emit() to advance fragmentIndex is updated to use synchronous assertions, validating that the architectural change is correct.

---

Performance / bundle size

• Two new runtime state booleans: negligible.
• media-element-guards.ts: ~14 lines, tree-shakeable.
• setInterval(..., 1000) for media wiring: one per slideshow instance, lightweight.
• BroadcastChannel media messages throttled to 450ms for timeupdate: good.
• CSS tooltip system added via injectKeyframesOnce: injected once per document, not per element.
• Removed reorderBranchSlide handler + UI: slight reduction in Studio bundle.
• No new dependencies.

No concerns.

---

Interactions with prior hardening (#1601)

No regressions. The prior PR's work (CSP inline handlers, Date.now IDs, manifest version, float React keys, scenes race, exhaustive-deps, empty catches, innerHTML rebuild) is orthogonal to this PR's changes. The escHtml in present.ts complements the CSP work from #1601 (which moved inline handlers to addEventListener). The new stop-media bridge command integrates cleanly with the existing bridge dispatch pattern.

---

Verdict

This is a well-structured hardening PR that addresses real architectural issues (racy play-then-pause navigation, cross-realm instanceof failures, runtime fighting slideshow media ownership). The seek-driven controller rewrite is the standout change — it eliminates an entire class of timing bugs. Test coverage is thorough. The presenter media sync and editable notes are well-scoped additions that complete the presenter mode feature.

One scope question: the reorderBranchSlide removal from Studio. If intentional, consider cleaning up the now-unused helper function in a follow-up.

No blocking issues found.

Review by Miga

[james-russo-rames-d-jusso]

Reviewed at HEAD 94971aae. Layering on top of #1601 — this PR sits in the "more hardening + new feature" lane, and the core opt-out + audience-sync wiring read clean. A few concerns and one likely-real bug worth surfacing before merge. Heads up: PR is mergeable: CONFLICTING against main, so a rebase is needed anyway.

Concerns

• packages/core/src/runtime/init.ts:1899-1908 — onSetWebAudioMediaDisabled asymmetric re-enable. The native-sync handler at :1888-1898 calls syncMediaForCurrentState() on BOTH enable and disable transitions, but the WebAudio handler only acts on disabled=true (stopAll + detach + sync) and silently no-ops on disabled=false. In the slideshow flow this happens to be safe (the flag is set true once at ready-handshake and never flipped back during a live deck), but the bridge accepts both verbs and replayBridgeState() will replay disabled: false on every runtime-ready handshake. If a player ever moves out of a slideshow embed at runtime (e.g. shadow DOM reattach, dynamic chrome swap), WebAudio scheduling will not re-arm. Cheapest fix: either mirror the native-sync handler's enable branch (no-op-but-explicit) or actually call scheduleWebAudioForActiveClips() if clock.isPlaying(). At minimum, comment why the asymmetry is intentional.

• packages/player/src/slideshow/hyperframes-slideshow.ts:614-624 ("Enable audience media" button) — copy/behavior mismatch. The unlock button reads "Enable audience media", but clicking it calls retryBlockedAudienceMedia → playAudienceMediaMuted, which forces el.muted = true and adds the key to audienceMutedPlaybackKeys so subsequent syncRemoteMediaState calls (:574) also clamp muted=true. So a user expecting audible audience media gets muted playback after clicking "Enable". Either rename to "Enable audience playback" / "Play muted" to set expectations, or make the click path actually unmute (clear the key from audienceMutedPlaybackKeys so the next play() carries msg.muted). The Chrome autoplay policy will accept an unmuted play() post-gesture — clearing the key on click would deliver the audible experience the label promises.

• packages/player/src/slideshow/hyperframes-slideshow.ts:436-441 + :484-502 — wireSlideshowMedia runs on a 1s setInterval for the lifetime of the slideshow, walking every player iframe's contentDocument and querySelectorAll(\"video,audio\"). The el.dataset.hfSlideshowMediaSync guard prevents double-binding, but the 8 attached listeners per media element are never detached when an element is removed from the iframe (no MutationObserver, no removed-node sweep). For long-running decks with dynamic compositions, this is a slow listener-handle leak — not a correctness bug, but worth either (a) using a MutationObserver like ParentMediaManager._observeDynamicMedia already does in parent-media.ts, or (b) just documenting why polling is acceptable here.

• packages/player/src/slideshow/hyperframes-slideshow.ts:429-434 — postCurrentPresenterPositionBurst schedules 5 unanchored setTimeouts out to 5000ms. disconnectedCallback (:240-256) does not clearTimeout them. The follow-up postCurrentPresenterPosition does guard on !this.channel so it's a safe no-op, but the timer handles outlive the element. Cheap fix: store the ids and clear on disconnect (or scope via AbortController).

Question

• packages/studio/src/components/panels/SlideshowPanel.tsx:60 + SlideshowSubPanels.tsx:357-368 — the reorderBranchSlide import, handler, and the entire up/down branch-slide reorder UI are removed. The PR title + body frame this as media-controls hardening + presenter chrome; the Studio branch-reorder removal doesn't fit either bucket. Intentional simplification, or accidental rollback of #1582-era functionality? Per gh api ...commits?path= the reorder UI is recent. If intentional, a one-line note in the PR body would prevent the next reviewer flagging this too.

Nits

• packages/cli/src/commands/present.ts:305-311 — escHtml duplicates the same 4-replace pattern that slideshowPresenter.ts:158 already defines as esc. The > and \" replacements aren't even load-bearing for the single use here ("${escHtml(projectName)} — Presenter" inside <title> element text). Tiny but worth deduping into a shared helper next pass.

• Commit was created with --no-verify per the PR body. Not a blocker since the checks were run manually, but worth fixing the local lefthook install so the trailer doesn't pile up across follow-ons.

Threat-model note

Read this as defense-in-depth + UX-bug remediation (runtime was muting/pausing presenter-controlled media on every transport tick), not a known CodeQL class or known prior incident. The new media-element-guards.ts realm-aware guards are a thoughtful generalization of the same defensive pattern parent-media.ts already used inline — good cleanup.

What I didn't verify

• Cross-window BroadcastChannel timing under real Chrome (audience-mute autoplay policy interactions, presenter retransmit ordering)
• Notes-localStorage key collision behavior when the same deck is opened from two different paths
• Studio panel regression-test coverage for the branch-reorder removal

— Rames D Jusso

[james-russo-rames-d-jusso]

R2 verification at HEAD 57740336 (rebased onto main, now mergeable: MERGEABLE). Squash-amend collapsed the prior commit history into a single fix(slideshow): harden media controls in present decks.

Resolved

• packages/core/src/runtime/init.ts:1899-1910 — asymmetric re-enable: ⚠️ partial. Both branches of onSetWebAudioMediaDisabled now call syncMediaForCurrentState() — the silent no-op on disabled=false I flagged is gone, so the native fallback path resumes correctly. Note this stops short of fully mirroring the cheap fix (calling scheduleWebAudioForActiveClips() when clock.isPlaying()); WebAudio's sample-accurate path won't re-arm until the next player.play(). For the slideshow flow this is fine (the flag is set once at handshake and the bridge doesn't flip it back mid-play), and the new behavior is a graceful degradation rather than silence. Tagging as partial only because there's no test pinning the disabled=false branch alongside the existing disabled=true opt-out test in init.test.ts:1188.

• packages/player/src/slideshow/hyperframes-slideshow.ts:614-624 — unlock button copy: ✅ resolved. Button now reads "Play audience media muted" (:663). Copy matches behavior; expectation set correctly.

• packages/player/src/slideshow/hyperframes-slideshow.ts:429-434 — burst timer leak: ✅ resolved. presenterPositionTimers: ReturnType<typeof setTimeout>[] array (:163) + clearPresenterPositionTimers() helper called from both disconnectedCallback (:264) and the head of postCurrentPresenterPositionBurst (:457). Each timer also self-removes from the array when it fires. Re-enter case is covered.

• packages/player/src/slideshow/hyperframes-slideshow.ts:436-441 + :484-502 — polling listener accrual: 🔄 resolved-differently. Author kept the 1s setInterval and documented the rationale inline (:481-483: dataset guard prevents duplicate listeners + GC-collectable iframe nodes because the component keeps no media element references). mediaWireInterval is cleared in disconnectedCallback (:252-255). The MutationObserver alternative would be more efficient but the slideshow-level (cross-player) shape makes polling simpler; tradeoff is now explicit.

Question (carried from R1)

• packages/studio/src/components/panels/SlideshowPanel.tsx:33 + SlideshowSubPanels.tsx:209-285 — reorderBranchSlide UI: still removed at HEAD with no PR-body note and no follow-up comment explaining intent. The helper itself is still exported. Miga flagged the same thing in their R1. Non-blocking for me — likely intentional simplification — but a one-line "intentional UI removal; helper retained for follow-up" in the PR description would close this out cleanly for the next reviewer.

Nits (carried)

• packages/cli/src/commands/present.ts:305-311 escHtml still duplicates slideshowPresenter.ts:175 esc/escAttr. Tiny, defer to follow-up.
• PR body still notes --no-verify due to missing lefthook-darwin-x64. Worth fixing locally so the trailer doesn't recur on the next PR.

Sibling-coherence sweep

The new presenterPositionTimers cleanup pattern is the only new infrastructure introduced. It's properly scoped (cleared on both re-call and disconnect). No new asymmetries detected in init.ts handlers or slideshow lifecycle methods. The clearPresenterPositionTimers placement mirrors mediaWireInterval / presenterInterval cleanup in disconnectedCallback — consistent shape.

Mergeable

✅ Was CONFLICTING at R1, now MERGEABLE against main. Rebase done.

Net

Mostly clean. The 4 concrete R1 concerns are 3✅ + 1⚠️-partial-but-acceptable. The scope question on reorderBranchSlide is unanswered but non-blocking. No new regressions introduced. The partial WebAudio re-enable is theoretical for the slideshow flow that motivated this PR.

— Rames D Jusso

[jrusso1020]

Approving at 57740336 — R2 verified clean per Rames D Jusso + Miga's converging reviews.

Verified the load-bearing closes from R2:

• Burst timer leak (Rames R1 finding #3) — hyperframes-slideshow.ts:163 introduces presenterPositionTimers; disconnectedCallback clears at :264; re-enter clears at :457; self-removal on timer fire. The unanchored setTimeout chain at :429-434 is now bounded by lifecycle.
• "Play audience media muted" rename (Rames R1 finding #2) — hyperframes-slideshow.ts:663. Honest rename: copy matches action. The fork (rename vs. clear audienceMutedPlaybackKeys) lands on the conservative side, which is the right call when the behavior is intentional muted-autoplay.
• presenterPositionTimers cleanup pattern mirrors the existing mediaWireInterval placement (:252-255), which is the right sibling to align with.

Acceptable partial-resolution: onSetWebAudioMediaDisabled asymmetric re-enable (Rames R1 finding #1) — both branches now call syncMediaForCurrentState() at init.ts:1900-1910. Doesn't fully mirror the sibling (no scheduleWebAudioForActiveClips() when clock.isPlaying()), but native fallback resumes — graceful degradation, not silence. Fine for the slideshow flow that motivated the PR.

Two carry-forward items worth a note when convenient:

• Scope clarity on reorderBranchSlide UI removal (Miga + Rames both flagged) — SlideshowPanel.tsx:60 + SlideshowSubPanels.tsx:357-368. UI removed, helper reorderBranchSlide still exported. PR title is "harden media controls in present decks"; the UI removal is tangential. A one-line PR-body note explaining the removal rationale would help future readers reconstruct intent.
• escHtml duplication — present.ts:305-311 duplicates slideshowPresenter.ts:175 esc. Shared helper at next opportunity.

Regression shards still in_progress at 57740336; all completed required checks are green. PR is now MERGEABLE after the rebase. Stamping at this SHA; the shards will gate the actual merge.

— Jerrai