fix(editor): host bridge round-trip stability + host-mode timeline shortcuts - #13
Merged
Merged
Conversation
stableSerialize treated a present-but-undefined key as different from a missing key, and frameItemToNativeComparable unconditionally emitted volume/speed/opacity/transform keys (plus an always-present text style object). Host snapshots that omit those optional fields failed metadataUnchanged after a simple drag or trim, landing in the unsupported-edit branch, and untouched clips leaked into changed[] on multi-clip timelines. - stableSerialize now skips undefined-valued object entries. - frameItemToNativeComparable emits volume/speed/opacity/transform only when set, and the plain-text style key only when non-empty; an opacity-only native transform round-trips as the top-level opacity field the host sent. - commandIdsForChanges compares clips with synthesized source bounds on both sides so host items without sourceStart/sourceEnd do not appear changed after the native bridge fills its defaults. - deriveSupportedHostEdit batches one remove_item command per removed item (bounded by MAX_COMMANDS_PER_OPERATION) instead of only handling exactly one removal. Round-trip tests install host snapshots through the real runtime, perform store move/trim/remove operations, and assert the derived host command batches.
TimelineShortcutsController was only rendered outside host mode, so Delete/Backspace/Space/J/K/L and friends did nothing in the host-embedded surface. Add useHostTimelineShortcuts (composed in use-timeline-shortcuts.ts) and mount it via a HostTimelineShortcutsController when hostRuntime is present. The composition mounts only bindings that never mutate the host-owned document outside the bridge: - Playback/navigation and tool switching mount as-is (local UI state; Shift+C split crosses the bridge as a supported split_item command). - Delete/Backspace are extracted into a shared useDeleteShortcuts hook; item removal flows through the bridge as remove_item commands, now batched for multi-select. - useUIShortcuts gains an enableHistory option (default true) so host mode mounts zoom/snap without undo/redo, which would mutate the temporal store without emitting host commands. Ripple delete, clipboard, markers, in/out points, nudges, join, freeze frame, and clear-keyframes stay unmounted in host mode. The editor -> timeline edge stays within budget by exporting the new hook from the existing use-timeline-shortcuts module.
nativeTransformToFrame fills every key, so the ?? 0 fallbacks were dead branches that tripped the fallow changed-health complexity gate.
Author
|
@codepress /judge-verification can you judge this verification? Canonical QA ReportHost bridge round-trip stability fix (drag/trim/delete rejection toasts) + host-mode timeline shortcuts (Space/J/K/L/arrows/Delete/tools/zoom; undo/redo, ripple delete, clipboard, markers, nudges gated off in host mode). Three commits: 503c3cc round-trip fix, 01a80ab shortcut mount, c679db1 fallow-gate cleanup. Verifier revision bindingbase: a188efa (origin/staging) Gate results
Inherited baseline failures
Environment blockers
Visual / browser artifacts
Red/green evidence for the fix
Caveats
Overall✅ PASS — PENDING JUDGE No merge until trusted judge PASS. |
|
👋 Hey @lightninglu10! Spawning a verification judge session to help with your request. View session |
|
Dispatched by judge-verification. This judgment is READY TO MERGE at the current head. If the PR is still a draft, transition it to ready for review now with the guarded readiness transaction: Verification Judgment — PR #13 (local)Verdict: ✅ READY TO MERGE PR Head SHA: Summary: The report is bound to the live PR head, the changed host bridge paths are covered by runtime-backed focused tests, and the host shortcut composition is wired to the real host editor mount. The three reported baseline failures are immaterial to this diff and are explicitly waived; the browser artifact path has a publication caveat only. Revision binding
Diff trigger inventory
Per-item
Overall status check
What stood up
|
MediaElementAudioSourceNode silences cross-origin media served without CORS approval (HTML spec), so host-mode playback through the Web Audio clip graph was muted. Route cross-origin sources around the graph: applyVideoElementAudioState and NativePitchCorrectedAudio now drive the media element's volume/muted directly when the source is not WebAudio-safe (isWebAudioSafeMediaSource: same-origin, blob:, data:), with reactive volume/mute propagation on the direct path. EQ remains graph-only and is skipped on the direct path. Host runtime hardening: reset the persisted playback mute/volume on mount (the monitor volume UI is hidden in host mode, so a persisted mute silently zeroed embedded audio), and keep resilient pointerdown/keydown listeners for the whole host session that resume the shared preview AudioContext when it starts suspended.
Author
|
@codepress /judge-verification can you judge this verification? Canonical QA ReportRe-bind of the canonical QA to the new head. Previous report (comment 5377278393) covered c679db1: host bridge round-trip stability fix + host-mode timeline shortcuts. Delta c679db1..bf0f298 is ONE commit, bf0f298 Verifier revision bindingbase: a188efa (origin/staging) Gate results
Delta disclosure: c679db1..bf0f298 (audio fix)
Inherited baseline failures
Environment blockers
Visual / browser artifacts
Caveats
Overall✅ PASS — PENDING JUDGE No merge until trusted judge PASS. |
|
👋 Hey @lightninglu10! Spawning a verification judge session to help with your request. View session |
|
Dispatched by judge-verification. This judgment is READY TO MERGE at the current head. If the PR is still a draft, transition it to ready for review now with the guarded readiness transaction: Verification Judgment — PR #13 (local)Verdict: ✅ READY TO MERGE PR Head SHA: Summary: The current PR head is safe to merge based on the bound verification evidence and the audited diff. The host bridge and host-mode shortcut behavior remain covered by runtime-backed focused tests, and the new cross-origin audio paths are covered at both the video-element and native audio-component boundaries. The three reported full-suite health failures reproduce at the base and do not touch the changed surfaces; the browser artifact citation has a publication-only caveat. Revision binding
Diff trigger inventory
Per-item
Overall status check
What stood up
|
Author
|
@codepress /judge-verification can you judge this verification? Local VerificationPR Head SHA: Scope: FreeCut host-embedding hardening across four commits on Diff Trigger Inventory
Blast radius: preview audio path (video + audio elements) and host-mode store mounting. Same-origin and blob:/data: sources keep the existing Web Audio graph behavior unchanged (guarded by Verification Contract ResultsCanonical 20-gate set from
Red/green evidence for the delta fixes:
Head/tree binding captured after all gates: Disclosed caveats (inherited, environment-limited)
Browser artifacts: Overall: PASS |
|
👋 Hey @lightninglu10! Spawning a verification judge session to help with your request. View session |
|
Dispatched by judge-verification. This judgment is READY TO MERGE at the current head. If the PR is still a draft, transition it to ready for review now with the guarded readiness transaction: Verification Judgment — PR #13 (local)Verdict: ✅ READY TO MERGE PR Head SHA: Summary: The current PR head is safe to merge based on the exact-head verification report and the audited diff. The host bridge and host-mode shortcut behavior remain covered by runtime-backed focused tests, and the new cross-origin audio paths are covered at both the video-element and native audio-component boundaries. The inherited unit/fallow findings are reproduced at base and do not touch the changed surfaces; the browser artifact citation carries a publication-only caveat. Revision binding
Diff trigger inventory
Per-item
Overall status check
What stood up
|
lightninglu10
marked this pull request as ready for review
August 22, 2026 03:19
|
Starting initial review of |
![codepress-reviewer-dev[bot]](https://avatars.githubusercontent.com/in/3963614?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved.
The host remains the authoritative timeline owner: local timeline changes are still reduced to bounded host commands and unsupported mutations restore the authoritative snapshot. The new host shortcut slice respects that boundary by leaving local-only history and unsupported editing families unmounted.
The round-trip normalization, batched removals, and cross-origin direct-audio fallback are coherent with that model. I found no actionable correctness, security, or production-risk issues in the supplied diff.
Reviewed `a188efa2`...`bf0f2986`. Diff collection and GitHub posting were handled by the automation.
|
Review head |
lightninglu10
added a commit
that referenced
this pull request
Aug 22, 2026
* chore(release): freecut-editor-surface 0.3.2 Version bump so the 0.3.2 npmjs publish is reproducible from a real repo commit (provenance gate). Content delta vs 0.3.1 is PR #13: host bridge round-trip stability, host-mode timeline shortcuts, and the host-mode cross-origin audio fix. * chore(release): document and script the manual npmjs release path The package manifest keeps publishConfig pointed at GitHub Packages (the CI tag/dispatch target, enforced by package-editor-surface.mjs), but CodePress consumes @quantfive/freecut-editor-surface from npmjs and the repo has no NPM_TOKEN secret, so npmjs releases are a manual maintainer step. Add scripts/publish-editor-surface-npmjs.mjs (root script publish:editor-surface:npmjs) that runs the provenance + deterministic pack + fresh-consumer smoke preflight and then publishes the exact tarball to https://registry.npmjs.org with --access=public; --dry-run validates without publishing. Document both release targets in the package README and refresh the provenance package.json checksums. * chore(release): make public npmjs the canonical release target Per maintainer direction the editor surface is a public package now. publishConfig targets https://registry.npmjs.org with public access and package-editor-surface.mjs enforces exactly that; the tag/dispatch workflow publishes to npmjs with NODE_AUTH_TOKEN from the NPM_TOKEN repo secret (a repo admin must add it before tag publishes work). The manual maintainer path (publish:editor-surface:npmjs) stays as the fallback and drops its registry/access overrides now that the manifest is canonical. README release docs updated; GitHub Packages is no longer a target. * chore(release): guard npmjs publish to the merged staging revision Reviewer finding: the manual publish script rebuilt and preflighted without checking the worktree or release ref, so uncommitted source could ship as the official public version. The script now requires an explicit --ref <sha> (or --ref HEAD) and refuses before any build step — in dry-run mode too — unless the tree is clean, HEAD equals --ref, and --ref is an ancestor of origin/staging. Guard behavior is covered by scripts/publish-editor-surface-npmjs.test.mjs (node --test, 7 cases: clean/match/ancestor acceptance, dirty tree, HEAD mismatch, non-staging ref, missing ref, unresolvable ref) wired as test:publish-editor-surface-guards. README documents the --ref flow; provenance checksums refreshed for the root manifest change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two user-facing bug fixes for the CodePress host-embedded surface, plus the host-mode keyboard shortcut mount they depend on.
Commit 1 —
fix(editor): keep host bridge round-trip stable for minimal clip items(503c3cc)Fixes the "Property, effect, or animation edits are unsupported by the host slice" toast on simple clip drags/trims. Root cause: serialization asymmetry in the host bridge round trip —
stableSerializetreated a present-but-undefinedkey as different from a missing key, andframeItemToNativeComparableunconditionally emittedvolume/speed/opacity/transformkeys (plus an always-present textstyle: {}). Host snapshots that omit those optional fields failedmetadataUnchangedafter a drag, and synthesizedsourceStart/sourceEndkeys made untouched clips leak intochanged[]on multi-clip timelines ("Multiple or ambiguous timeline changes").stableSerializeskipsundefined-valued object entries.frameItemToNativeComparableemits optional keys only when set; plain-textstyleonly when non-empty; opacity-only native transforms round-trip as the top-levelopacitythe host sent.commandIdsForChangescompares clips with synthesized source bounds on both sides.deriveSupportedHostEditbatches oneremove_itemper removed item (bounded byMAX_COMMANDS_PER_OPERATION= 64) instead of only handling exactly one removal — enables multi-select delete.Commit 2 —
feat(editor): mount host-safe timeline shortcuts in host mode(01a80ab)TimelineShortcutsControllerwas only rendered outside host mode, so Delete/Backspace/Space/J/K/L/arrows did nothing in the embed. AddsuseHostTimelineShortcutsmounted via a newHostTimelineShortcutsControllerwhenhostRuntimeis present:split_item), Delete/Backspace (flows through the bridge asremove_item, now batched for multi-select), zoom/snap (S, Shift+S, Mod+=/−,\).Implementation: Delete/Backspace extracted into a shared
useDeleteShortcuts;useUIShortcutsgains anenableHistoryoption (default true). The editor→timeline edge stays within budget (73/73) by exporting from the existinguse-timeline-shortcutsmodule.User-reported bugs addressed
Test evidence
src/features/editor/host/controller.test.ts: 17/17 (7 new round-trip tests: store-level drag →move_item, trim →trim_item, multi-clip timeline with untouched clips excluded, default-color text item, opacity-carrying clip, multi-remove batch, >64-removal bounded rejection). Red/green verified: all new tests fail without the fix.src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx: 5/5 (real keydown dispatch: Space toggles playback; Delete removes selected items incl. multi-select; Mod+Z does not undo in host mode; control test proves Mod+Z undoes with full shortcuts).use-editing-shortcuts.test.tsxsuite passes against the extracted-hook refactor (verified with an environment workaround for the inherited localStorage issue below).Inherited baseline failures (disclosed, reproduced at base)
Full
test:runon this machine (Node 26) has widespread jsdomlocalStoragefailures (TypeError: Cannot read properties of undefined (reading 'setItem')from zustand persist middleware) — the failing-test set is byte-identical at basea188efa2; zero new failures introduced. Details in the QA report.