diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7a16181..f5f6951 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,9 +42,8 @@ jobs: run: npm ci working-directory: server - - name: Run tests - run: npm test - continue-on-error: true + - name: Run tests + run: npm test - - name: Run verification suite - run: npm run verify + - name: Run verification suite + run: npm run verify diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 588f701..cb16512 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -26,7 +26,7 @@ All notable changes to the KoalaSync browser extension and relay server. ### Changed - **Playback sync now follows the room's control setting** — When Host Control is enabled, only the host can drive room-wide playback changes; guests can still watch in sync without accidentally changing playback for everyone. - **Architecture: Modularized background.js** — Refactored 2410-line file into 11 focused modules for better maintainability. -- **Documentation: Consolidated Host Control Mode** — Unified documentation in host-control-mode.md with references to internal implementation details. +- **Documentation: Consolidated Host Control Mode** — Unified documentation in host-control-mode.md. - **README: Added protocol documentation** — Updated documentation links to include PROTOCOL.md reference. - **ARCHITECTURE: Added protocol reference** — Updated architecture documentation with link to PROTOCOL.md. diff --git a/docs/host-control-mode.md b/docs/host-control-mode.md index f4b066f..49bb6e3 100644 --- a/docs/host-control-mode.md +++ b/docs/host-control-mode.md @@ -123,9 +123,9 @@ When a guest's action is blocked: function getHostSyncTarget() { const host = currentRoom.peers.find(p => p.peerId === hostPeerId); if (!host) return null; - + let targetTime = host.currentTime; - + // Extrapolate if host is playing if (host.playbackState === 'playing' && host.lastHeartbeat) { const elapsedSec = (Date.now() - host.lastHeartbeat) / 1000; @@ -133,7 +133,7 @@ function getHostSyncTarget() { targetTime += elapsedSec; } } - + return { playbackState: host.playbackState, targetTime }; } ``` @@ -284,4 +284,3 @@ function getHostSyncTarget() { - [Protocol Specification](PROTOCOL.md) — Technical event details - [Architecture](ARCHITECTURE.md) — System overview -- [Internal Documentation](internal/host-control-mode-plan.md) — Original implementation plan (German) \ No newline at end of file diff --git a/docs/internal/host-control-mode-COHOST-PLAN.md b/docs/internal/host-control-mode-COHOST-PLAN.md deleted file mode 100644 index 854b9dc..0000000 --- a/docs/internal/host-control-mode-COHOST-PLAN.md +++ /dev/null @@ -1,118 +0,0 @@ -# Co-Host (Multi-Controller) — Implementation Plan - -Branch base: `feature/host-control-mode` (builds directly on it). -Goal: let the room owner grant **playback control to several peers** (co-hosts), not -just one — e.g. 4 of N people in a room may drive play/pause/seek, the rest are guests. - -This is the second server-gated feature the `capabilities` hook was designed for -(`CAPABILITIES.CO_HOST = 'co-host'`, already stubbed in `shared/constants.js`). - ---- - -## 1. Roles - -| Role | Can drive (play/pause/seek/force-sync/episode-lobby) | Can promote/demote + toggle mode | -|------|------|------| -| **Owner** (room creator, = today's "host") | yes (always a controller) | **yes** | -| **Controller** (co-host) | yes (in `host-only` mode) | no | -| **Guest** | only in `everyone` mode | no | - -The single-host feature is just the special case `controllers = { owner }`. - -## 2. Data model - -### Server (`room` object) -- `ownerPeerId` — the creator / manager. Keep `hostPeerId` as an **alias** (= ownerPeerId) - so older clients keep working. -- `controllers: Set` — peers allowed to drive. **Always contains ownerPeerId.** -- `controlMode: 'everyone' | 'host-only'` — unchanged wire values (`'host-only'` now means - "restricted to controllers", not "single host"). -- `MAX_CONTROLLERS` cap (e.g. 10) to bound the set + payload. - -### Shared constants -- `CAPABILITIES.CO_HOST = 'co-host'` (un-stub it) → add to `SERVER_CAPABILITIES`. -- New events: - - `SET_PEER_ROLE` (client→server): `{ peerId, controller: boolean }` — owner promotes/demotes. - - Extend `CONTROL_MODE` (server→client) payload: `{ controlMode, ownerPeerId, hostPeerId, controllers: [peerId...] }`. -- `ROOM_DATA` gains `ownerPeerId` + `controllers`. - -## 3. Gate generalization (the core change) - -Today the gate compares against a single `hostPeerId`. Generalize to set membership: - -- **Server relay gate** (`server/index.js`): `controlMode === 'host-only' && !room.controllers.has(mapping.peerId)` → drop. (Was `mapping.peerId !== room.hostPeerId`.) -- **Background gates** (sender + receiver): replace `amHost()` / `senderId !== hostPeerId` - with controller-set membership: `controllers.includes(myPeerId)` / `senderId ∈ controllers`. -- **Helpers:** split `amHost()` into `amOwner()` (manage rights) and `amController()` - (drive rights). The desync/snap-back path keys on `!amController()` instead of `!amHost()`. - -`SET_PEER_ROLE` handler (server): validate sender is owner, target is a current peer in the -room, enforce `MAX_CONTROLLERS`, always keep owner in the set, then broadcast `CONTROL_MODE` -with the new `controllers`. - -## 4. Client + UI - -- **Owner** sees the peer list with a per-peer **"Controller" toggle** (promote/demote) plus - the existing mode toggle. -- **Controllers** see a "Controller" badge and are NOT locked out of the remote-control buttons. -- **Guests** see "Guest" + the host-only notice (unchanged). -- The promote UI + co-host badges render only when the relay advertises the `co-host` - capability (feature detection, same pattern as `hostControlSupported`). -- i18n: new keys (`ROLE_CONTROLLER`, `BTN_PROMOTE`, `BTN_DEMOTE`, …) across all locales. - -## 5. Backwards compatibility - -- **New client + old server** (host-control only, no `co-host` capability): no co-host UI; - behaves as today's single-host. ✓ -- **Old client + new server**: ignores `controllers` / `SET_PEER_ROLE`. An old client that - the owner promotes still **gates itself** (its sender-gate only knows `!amHost`), so it - can't drive — it degrades to a guest. Co-host requires a client that understands - `controllers`. Document this; not a crash. ✓ -- No `PROTOCOL_VERSION` bump needed — purely additive, same as host-control. - -## 6. Edge cases -- **Controller leaves** → `removePeerFromRoom` also does `room.controllers.delete(peerId)`. -- **Owner leaves** → fallback: promote the earliest remaining **controller** to owner (prefer - a controller over a random peer); if none, earliest peer; keep the rest of the set. Reuse - the `peerJoinLocks` guard so a reconnect/second-tab doesn't demote (same fix as host). -- **Promote a peer not in the room** → server rejects (target must be a live peer). -- **Promote beyond `MAX_CONTROLLERS`** → server rejects, re-syncs the owner's UI. -- **`everyone` mode** → the `controllers` set is still maintained (so flipping to `host-only` - keeps the chosen co-hosts), it just isn't enforced while in `everyone`. -- **peerId spoofing** → unchanged accepted limitation (see `docs/KNOWN_LIMITATIONS.md`); - co-host doesn't widen it materially (still bounded to a temporary room). - -## 7. Scale: the "4 of 510 people" part — read this - -The role change above is moderate. **Putting 510 people in one room is a separate, larger -problem** and should be its own track: - -- `MAX_PEERS_PER_ROOM` is **25** today. 510 needs a large raise + load testing. -- **The real bottleneck at scale is heartbeat fan-out, not control events.** Every peer - heartbeats and the relay broadcasts each to all peers → O(N²) per interval. At 510 that's - ~510×509 / 15s ≈ **17k msg/s just for heartbeats** — the scaling wall. -- **Co-host actually *helps* the control-event side:** in `host-only` mode only the few - controllers emit play/pause/seek, so event *sources* drop from N to K (e.g. 4). Restricting - who can drive is synergistic with big rooms. -- Large rooms therefore need (independent of co-host): - - **Heartbeat fan-out reduction** — e.g. only relay controller/owner heartbeats to everyone, - relay guest heartbeats only to the owner/controllers (for the UI), or server-side - aggregation into periodic snapshots instead of per-peer relay. - - **`ROOM_DATA` payload trimming** — a 510-entry peer list is large; send counts + controller - details, lazy-load the full roster. - - Possibly the **socket.io Redis adapter** for horizontal scaling, and broadcast tuning. - -## 8. Effort estimate -- **Co-host roles** (server gate generalization + `SET_PEER_ROLE` + owner-leave fallback + - client gates + promote UI + i18n), at the current ≤25-peer scale: **~3–4 dev days** (same - shape as host-control itself — mostly generalizing host→controller-set). -- **Large-room scaling (510)**: a **separate ~1–2 week** track (heartbeat redesign + payload - trimming + cap raise + load testing), independent of co-host. Recommend shipping co-host at - the current cap first, then scaling rooms as its own project. - -## 9. Suggested sequencing -1. `CAPABILITIES.CO_HOST` + `controllers`/`ownerPeerId` in room state + `ROOM_DATA` (additive). -2. Server `SET_PEER_ROLE` + gate generalization + owner-leave fallback + WS tests. -3. Background: controller-set membership in both gates + `amOwner`/`amController`. -4. Popup: promote/demote toggles (owner) + Controller badge + i18n. -5. (Separate track) large-room scaling. diff --git a/docs/internal/host-control-mode-EDGECASES.md b/docs/internal/host-control-mode-EDGECASES.md deleted file mode 100644 index a6dfe8f..0000000 --- a/docs/internal/host-control-mode-EDGECASES.md +++ /dev/null @@ -1,283 +0,0 @@ -# Host Control Mode — Branch Overview, Goals & Edge Cases - -> **This is the canonical entry-point doc for branch `feature/host-control-mode`.** -> If you're an agent or contributor picking this up: read this file first, then the -> implementation plan in [`host-control-mode-plan.md`](./host-control-mode-plan.md). -> Temporary working doc — delete or fold into permanent docs before merging to `main`. -> -> Status legend: 🔴 open / unresolved · 🟡 idea, needs testing · 🟢 decided - ---- - -## 0. Implementation status - -All five layers are implemented & pushed (server + background + content + popup + i18n). -Automated checks green: ESLint, WS integration tests (incl. host-only gate, toggle -reject, host-leave fallback), content video-finder, locale consistency (15 langs), -full release verification. - -**Still needs real-device testing** — the EC test matrix in §7 (YouTube/Netflix/ -Twitch/Disney+/Jellyfin): involuntary-pause classification (EC-1/EC-5/EC-8), snap-back -reliability and fight-loops (EC-4), and the desync/resync flow across players. The -intent classifier (EC-9) and snap-back cooldown are first-pass heuristics tuned by -reading the code, not yet by watching them behave on each site. - -Deferred by decision (see §8): host grace period on disconnect (EC-10). - -### Capability detection (forward-compat hook) -The relay advertises `capabilities: ['host-control']` in `ROOM_DATA` -(`SERVER_CAPABILITIES` in server, `CAPABILITIES` in shared/constants). The client -enables host-control UI/behavior only when the flag is present, so the feature -degrades cleanly on an older relay (absent → off) and old clients ignore the -field. This is the extensible hook for the planned **co-host** feature (owner -promotes guests to additional controllers): it will add a `'co-host'` capability -+ events without a protocol bump or breaking older relays/clients. Add new flags -to `CAPABILITIES` / `SERVER_CAPABILITIES` as features land. - -### Pre-test self-audit (fixed) -- **Popup remote buttons froze for guests** — in host-only a guest's Play/Pause/SYNC - click was gated server-side but the button stuck on "Playing"/disabled with no - feedback. Now the remote controls are locked (disabled + tooltip) for guests, with - backstop guards in the handlers. (popup.js) -- **Desync dialog could break under strict CSP** — it used `innerHTML` with inline - `style=""` attributes, which Netflix/YouTube/Disney+ strip via `style-src`. Rebuilt - with the DOM API (CSSOM `.style` is CSP-safe) inside a **Shadow DOM** so page CSS - can't restyle/hide it. (content.js) -- **Live-DVR not detected (EC-15)** — `duration === Infinity` misses Twitch/YouTube - live-DVR (finite, sliding duration). Added a `seekable.start(0) > 1` sliding-window - heuristic in `hcmIsLive()`. (content.js) - -### Pre-test self-audit -- ~~**EC-4/EC-1 snap-back thrash:**~~ FIXED — implemented the **buffer-aware deferred - snap-back** (`hcmDeferredSnapBack`): on an involuntary event, if the player isn't - ready (`readyState<3` or seeking) we wait (poll, 8s cap) until it can play, then snap - ONCE to the host's re-queried position instead of repeatedly fighting the buffer. - Done defensively/player-agnostically — we can't enumerate every site, so this is safe - whether a player fires `pause()` or only `waiting`. Aborts if the user goes solo or is - no longer a gated guest; single pending poll (no stacking). -- ~~**Control-mode race at join:**~~ FIXED — `hcmHandleBlocked` now treats `HOST_BLOCKED` - as authoritative (adopts host-only/guest role) instead of re-checking local mode, - since background only sends it to gated guests. -- ~~**Dialog/badge text is English-only**~~ FIXED — background resolves the strings - via GET_HCM_STRINGS (it has the i18n loader); content fetches them on init with - English fallback. 6 new keys (HCM_DIALOG_*/HCM_BADGE_*) across all 15 locales. - ---- - -## 1. What this branch is for - -Origin: a GitHub feature request. When watching with larger groups, anyone can pause -or seek and disrupt everyone else. The requester wants the room creator to optionally -restrict control to a single **host**, the way Teleparty works — guests who try to -pause get asked whether they want to pause *only their own* player (and desync), and -otherwise get snapped back to the room's position. - -**Goal:** Add an optional per-room **Host Control Mode**. A room can be switched -between: -- **`everyone`** (default, current behavior): anyone can play/pause/seek for the room. -- **`host-only`**: only the host drives the room. A guest's deliberate play/pause/seek - is not broadcast; instead they're snapped back to the room position — unless they - explicitly choose to desync (go solo) with a "Resync" escape hatch. - -## 2. Trust model (read this before over-engineering) - -This is **client-side trust, by design**. It's a watch party, not a security boundary. -The point is preventing *accidental* and *casual* disruption, not stopping someone -determined to patch their own extension. We do **not** add auth, tokens, or -cryptographic host identity. `peerId` is unauthenticated and that's fine here. - -(We still gate server-side as the robust chokepoint — see plan — but that's about -killing spam reliably, not about defeating a hostile client.) - -## 3. Scope / non-goals - -In scope: -- Host designation (first joiner = host), mode toggle, host-only gating of all - room-moving events, guest snap-back, deliberate-desync flow + resync, host UI. - -Explicit non-goals (for this branch): -- Authenticated / spoof-proof host identity. -- Persistent host across server restarts (room state is in-memory). -- Syncing around personalized ad breaks. -- Host transfer UI (auto-fallback to `everyone` when host leaves; manual transfer - is a possible later add). - -## 4. Architecture summary - -Three-layer gate for room-moving events from a non-host in host-only mode -(`PLAY`, `PAUSE`, `SEEK`, `FORCE_SYNC_PREPARE`, `FORCE_SYNC_EXECUTE`, `EPISODE_LOBBY`): -1. **Server** — doesn't relay them (robust chokepoint, kills spam regardless of client). -2. **Sender (guest)** — doesn't emit; shows confirm dialog / disables host-only buttons. -3. **Receiver** — drops any that slip through (covers old/buggy/modified clients). - -Snap-back reuses the existing `_setSuppress` mechanism (content.js:442) so applying -the room state programmatically doesn't echo back as a new event. Target position is -extrapolated from the host's last known state (±1s). Full detail + code hooks in the -plan doc. - ---- - -## 5. The central challenge - -Everything hard about this feature reduces to **one question** (see EC-9): - -> How do we reliably tell a **deliberate** guest pause/seek from an **involuntary** -> player/browser event (buffering, ads, tab throttling, source swaps, DRM hiccups)? - -If we get this wrong, guests get spammed with desync dialogs and snap-back loops for -things they never did. The host/role plumbing is the easy part; this classifier is the -real work. **Design the intent-classifier before writing the gate.** - ---- - -## 6. Edge cases - -### EC-1 🔴 Buffering / loading fires a `pause` event -content.js listens to `play`/`pause`/`seeked`/`loadeddata` only (content.js:1000-1003), -not `waiting`/`stalled`. Pure HTML5 buffering fires `waiting` → harmless. But custom -players (Netflix/YouTube/Twitch/JW) often call `video.pause()` during buffering/ads → -real `pause` → guest gate would mis-classify as deliberate. Sub-cases: (a) initial load -sits paused, no event, fine; (b) mid-stream stall, player-dependent; (c) seek-induced -re-buffering may outlast the suppress window and leak. Mitigation: `isBuffering` flag -from `waiting`/`playing`, or grace window; in host-only the guest's own state is -irrelevant so just ignore involuntary pauses and let catch-up logic (content.js:489) -re-sync them. - -### EC-2 🟢 Force-Sync / Episode-Lobby abuse by guests -Guest could seek + spam Force-Sync to drag everyone, or spam Episode-Lobby to pause -everyone. Decision: host-only blocks guest *initiation* of `FORCE_SYNC_*` and -`EPISODE_LOBBY`; guests may only respond (`FORCE_SYNC_ACK`, `EPISODE_READY`). Guests' -legitimate path is the personal "Resync" button. - -### EC-3 🟢 Host leaves the room -Fall back to `controlMode = 'everyone'`, broadcast `CONTROL_MODE`. Never a stuck locked -room. (Auto-promote next peer deferred.) - -### EC-4 🔴 Snap-back fight loop (pause/play/skip back/pause/play) -Mashing controls or a janky player → each snap-back may emit events → ping-pong. -Mitigation: cooldown (~600ms) after snap-back; ensure snap-back runs fully under -suppress. Also: if target is unreachable (seek past buffered range), retry K times then -give up — no infinite loop. - -### EC-5 🔴 Ad breaks (YouTube/Twitch/…) -Mid-roll ads pause/swap the media element, differ per peer → desync is unavoidable and -must NOT spam the dialog. Probably covered by EC-1 buffering grace; flag for explicit -testing. - -### EC-6 🟡 Snap-back target accuracy -No continuous room clock; extrapolate from host's `currentTime` + `lastHeartbeat` -(±1s, worse if stale). The follow-up host correction must also be suppressed so it -doesn't read as guest input. - -### EC-7 🟢 Old / buggy / modified guest client -Covered by receiver-side + server-side gates. - -### EC-8 🔴 Tab throttling / background tab -Backgrounding throttles timers and may pause media. There's existing -`visibilityGraceUntil` handling for seeks (content.js:892). Confirm a -background-induced pause isn't treated as deliberate in host-only; reuse the grace flag. - -### EC-9 🔴 What counts as "deliberate" — the central unresolved question -Collapses EC-1/EC-5/EC-8. Candidate signals: `readyState`/`networkState`/`video.seeking` -at event time; recent `waiting`; recent user gesture (`navigator.userActivation`, -keydown/click); visibility/focus. Build one shared **intent-classifier** helper in -content.js that all host-only gating flows through. - -### EC-10 🔴 Host brief disconnect / reconnect (network blip) -Host's wifi drops for 3s and reconnects. With "host leaves → fallback to everyone", -a blip would silently unlock the room and demote the host (peerId persists in -chrome.storage so they rejoin with the same id, but the server already cleared -`hostPeerId`). Mitigation idea: short **host grace period** (e.g. keep `hostPeerId` -reserved for ~30s after disconnect; if the same peerId rejoins, restore host + mode). -Needs the server reaper (server:644) and `removePeerFromRoom` (server:168) to cooperate. - -### EC-11 🔴 New guest joins mid-session in host-only mode -On join they must (a) immediately sync to the host's current position without the host -doing anything, and (b) see they're a guest in the UI. ROOM_DATA already carries peers; -add `hostPeerId`/`controlMode` so a fresh client knows its role instantly. Verify the -existing "newcomer syncs without waiting" path (content.js:542) still fires. - -### EC-12 🔴 Desync semantics — what does "solo" actually mean? -When a guest chooses "pause only me", do they (a) fully ignore all subsequent host -events until they Resync, or (b) keep receiving but not auto-applying? Define clearly. -Proposed: full solo — ignore host play/pause/seek while desynced; Resync re-attaches and -snaps to current host position. Also: what state does Resync land them in if the host is -currently paused vs playing? - -### EC-13 🔴 Race: host flips to host-only exactly as a guest pauses -Event ordering between `SET_CONTROL_MODE`/`CONTROL_MODE` and an in-flight guest `PAUSE`. -The `seq` ordering helps, but define the tie-break. Likely: server is authoritative — -once it has `host-only`, it drops the guest event regardless of client-side timing. - -### EC-14 🟡 Volume / mute / audio-options must NOT be gated -Those are per-peer, not room control. The gate must target only play/pause/seek + -forcesync/episode — not `PEER_STATUS` volume/mute fields. Easy to over-block; add a test. - -### EC-15 🔴 Live streams (Twitch live, live DVR) -"Room timestamp" is fuzzy on live edge; seeking semantics differ. Decide whether -host-only even makes sense for live, or degrade gracefully. Low priority but log it. - -### EC-16 🟡 Host's own involuntary events still drive the room -If the host buffers and the player auto-pauses, that pause is "allowed" and pauses -everyone. That's existing behavior, but in host-only it means the host's buffering -stalls the whole room. Acceptable? Probably yes (host is authoritative), but note it. - -### EC-17 🟡 Server restart drops room state -`hostPeerId`/`controlMode` are in-memory. After a server restart, whoever rejoins first -becomes the new host and mode resets to `everyone`. Acceptable for now (non-goal), but -document so it's not a surprise. - -### EC-18 🟡 Dialog dismissed without choosing -Guest clicks away / presses Esc on the desync prompt. Default = treat as "No" → snap -back. Make sure an un-answered dialog can't leave them in limbo (paused + not desynced + -no dialog). - -### EC-19 🟡 Multiple video elements / element swap (SPA, ad → content) -Players that swap the `