jellytau/docs/specs/player-facade-enforcement.md

Spec: Enforce the unified player boundary

Status: Proposed Requirements: DR-095 (new); relates to UR-005 and the unified-player-boundary principle in CLAUDE.md and 02-svelte-frontend.md UX spec: n/a — refactor, no user-visible change. Supersedes / revises: n/a

Summary

The stated principle is that UI controls playback only through playerController (src/lib/player/index.ts), never by calling commands.player* directly. There are 52 direct call sites outside that facade. This spec routes the genuine playback-control calls through the facade, narrows the principle's wording so it stops forbidding things it never meant to forbid, and adds the lint rule that keeps it true — because this rule is the one design principle in the audit with no automated check at all, and it is also the one that drifted furthest.

Motivation

Direct commands.player* usage outside src/lib/player/, by file:

File	Sites
queue.ts	10
player/[id]/+page.svelte	9
VideoPlayer.svelte	8
settings/+page.svelte	5
sleepTimer.ts / auth.ts / autoplay.ts	4 each
preload.ts	3
library/[id], playerEvents.ts, playbackMode.ts	1–2 each

These are not equivalent violations, and treating them as one number is why the rule has been easy to ignore. Three distinct groups:

(a) Genuine violations — playback control with a facade method that already exists. playerStop ×6, playerPlayTracks ×4, playerSeek ×2, playerPlayAlbumTrack ×2, playerNext, playerPrevious, playerSkipTo, playerToggleShuffle, playerCycleRepeat, playerRemoveFromQueue, playerMoveInQueue, playerAddTrackById, playerAddTracksByIds, playerSetSubtitleTrack, playerPlayItem. The facade exposes stop(), seek(), next(), previous(), skipTo(), toggleShuffle(), cycleRepeat(), removeFromQueue(), moveInQueue(), addTrackById(), addTracksByIds(), setSubtitleTrack(), playTracks(), playAlbumTrack(), playItem() — every one of these has a facade equivalent that is simply not being called. queue.ts is the starkest case: it imports commands directly and re-implements ten methods the facade already provides.

(b) Playback control with no facade method. playerPlayQueue, playerGetQueue, playerGetStatus, playerEnterBackgroundAudio, playerExitBackgroundAudio, playerSetSleepTimer, playerCancelSleepTimer, playerPlayNextEpisode, playerCancelAutoplayCountdown. In scope for the principle, but currently impossible to comply with — the facade has no surface for them. A rule that cannot be followed is not being broken so much as it is unfinished.

(c) Not playback control. playerConfigureJellyfin ×3, playerDisableJellyfin, playerGet/SetAudioSettings, playerGet/SetVideoSettings, playerGetEqPresets, playerGet/SetAutoplaySettings, playerGet/SetCacheConfig, playerPreloadUpcoming. These are configuration and lifecycle calls that happen to live under the player_ command prefix. The principle is about who is authoritative for playback state — settings CRUD isn't that.

The audit's read: the rule as written is violated 52 times, which makes real drift indistinguishable from acceptable usage, and that ambiguity is what lets group (a) persist. Note also that the principle is well-honoured where it matters most — the read side is clean, with UI reading state exclusively from the facade's re-exported stores. The write side is what drifted.

Layer assignment

Frontend-internal refactor. No domain logic moves and nothing new crosses IPC — the same Rust commands are called, through one module instead of many.

Logic / responsibility	Layer	Why it belongs there
Playback command dispatch (adapter routing: native vs HTML5)	Frontend — src/lib/player/ only	Presentation-layer plumbing, but must be centralised: the facade picks between the native backend and the HTML5 <video> adapter. A caller bypassing it silently skips that routing.
Playback authority (position, pause, rate, track changes)	Rust / the player	Unchanged. The player is authoritative; UI is a consumer. This spec does not touch that direction.
Queue mutation commands	Frontend facade → Rust	Rust owns queue state; the facade is the single call path to it.
Player settings CRUD (EQ, video, autoplay, cache)	Frontend, outside the facade	Configuration, not playback control — read/written on a settings page with no adapter routing. Explicitly carved out below.
Backend→frontend event handling	playerEvents.ts	Already correct. It is the facade's own plumbing, not a bypassing consumer.

No Jellyfin taxonomy is involved, so no boundary-leak risk.

Design

1. Narrow the principle to what it actually means

Amend CLAUDE.md and 02-svelte-frontend.md:

Unified player boundary. UI controls playback — transport, queue mutation, track selection, playback initiation — only through playerController. Player configuration commands (player_*_settings, player_configure_jellyfin, player_*_cache_config, player_preload_upcoming) are ordinary IPC and may be called directly from settings surfaces.

This is a clarification, not a relaxation: it makes group (c) explicitly fine so that a violation count means something. A rule with 52 nominal violations, most of them acceptable, provides no signal.

2. Fill the facade gaps (group b)

Add to playerController, each a thin pass-through preserving current behaviour:

playQueue, getQueue, getStatus,
enterBackgroundAudio, exitBackgroundAudio,
setSleepTimer, cancelSleepTimer,
playNextEpisode, cancelAutoplayCountdown,

Do this first — group (a) cannot be fully migrated while callers still need a direct import for a neighbouring call, and a file that imports commands for one reason will keep using it for others.

3. Migrate group (a)

Mechanical: replace commands.playerX(...) with playerController.x(...). Highest-value first: queue.ts (10 sites, all direct facade equivalents), then player/[id]/+page.svelte, VideoPlayer.svelte, sleepTimer.ts, playbackMode.ts, library/[id]/+page.svelte.

Two sites need care rather than substitution:

• playerEvents.ts (playerOnPlaybackEnded, playerStop in the error path). This module is the facade's event plumbing — the counterpart to index.ts, inside the boundary conceptually though not by directory. Treat src/lib/services/playerEvents.ts as inside the boundary and exempt it, rather than making it call the facade that calls back into it. Record this in the lint config with the reason.
• VideoPlayer.svelte — registers its own adapter via setActiveAdapter. Its playerStop/playerPlayItem calls interact with adapter lifecycle, and CLAUDE.md's gotcha ("no lifecycle calls after an await in onMount") applies. Migrate this file last and on its own, so an Android seek regression is bisectable to one commit.

4. Add the lint rule (the part that makes it stick)

The audit's finding was that principles with working checks held up and principles without them drifted. This principle has no check. Add scripts/check-player-boundary.sh, wired as bun run check:player-boundary and into test-all.sh:

# Playback-control commands that MUST go through the facade.
CONTROL='player(Play|Pause|Toggle|Stop|Seek|Next|Previous|SkipTo|ToggleShuffle|CycleRepeat|RemoveFromQueue|MoveInQueue|SetVolume|ToggleMute|SetSubtitleTrack|SeekVideo|SwitchAudioTrack|PlayTracks|PlayAlbumTrack|PlayItem|PlayQueue|AddTrackById|AddTracksByIds|GetQueue|GetStatus|EnterBackgroundAudio|ExitBackgroundAudio|SetSleepTimer|CancelSleepTimer|PlayNextEpisode|CancelAutoplayCountdown|OnPlaybackEnded)'

# Inside the boundary: the facade and its event plumbing.
EXEMPT='^src/lib/player/|^src/lib/services/playerEvents\.ts$'

Flag commands.$CONTROL in non-test src/ files outside EXEMPT. Config commands are deliberately absent from the list, matching §1 — so the check encodes the narrowed rule rather than the aspirational one.

An ESLint no-restricted-syntax rule would give better editor feedback, but the project has no ESLint config; a shell check matches the existing check:boundary precedent and adds no dependency.

Out of scope

• Changing playback behaviour — pure refactor.
• The one-directional state principle (audited clean; UI reads from facade stores only).
• Moving settings CRUD behind the facade (§1 explicitly carves it out).
• Introducing ESLint.
• Refactoring VideoPlayer.svelte's 2079 lines generally, beyond its facade call sites.
• The commands.player* calls inside src/lib/player/ — that is the facade doing its job.

Acceptance criteria

• playerController exposes the group-(b) methods listed in §2.
• grep -rn "commands\.player" src/ --include='*.ts' --include='*.svelte' | grep -v '^src/lib/player/' | grep -v 'playerEvents\.ts' | grep -v '\.test\.' | grep -v bindings.ts returns only configuration commands per §1 — no transport, queue, or playback-initiation call.
• queue.ts no longer imports commands from bindings.
• bun run check:player-boundary exists, is wired into test-all.sh, and passes.
• The check fails when a commands.playerStop() is added to a non-exempt file — verify explicitly, as with the other gates in this batch.
• The check does not fail on commands.playerSetAudioSettings() in settings/+page.svelte (the §1 carve-out works).
• CLAUDE.md and 02-svelte-frontend.md carry the narrowed wording, including the config carve-out and the playerEvents.ts exemption with its reason.
• No behavioural change: audio and video playback, queue reorder, shuffle/repeat, sleep timer, background audio, and autoplay all behave as before on both Linux and Android.
• Android seek and onMount lifecycle still correct after the VideoPlayer.svelte migration (the known-fragile path).
• bun run check and bun run test pass.
• bun run check:boundary passes.
• Changed code carries // TRACES: comments.
• No Rust change, so no bindings.ts regeneration.

Testing

Frontend (bun run test):

• Extend the existing facade tests to cover each new group-(b) method: it forwards to the right command with the right arguments, and routes to the active adapter where applicable.
• queue.ts tests: assert calls land on playerController, not commands. Mock the facade — a test that mocks commands would pass either way and guard nothing.
• Keep tauriIntegration.test.ts and the other IPC param-naming tests green; they cover the camelCase rule this refactor must not disturb.

Manual (no automated coverage for these paths):

• Linux: play/pause/seek/next/prev, queue reorder, shuffle, repeat, sleep timer, transcoded video (HLS), background audio enter/exit.
• Android: the same, plus lockscreen/MediaSession controls, and seek after entering the player — the specific regression CLAUDE.md warns about.

Because this is a pure refactor, the strongest signal is that no test changes expectation. A test needing its assertions rewritten means behaviour moved — investigate rather than update it.

TRACES

Allocate in requirements.md:

• DR-095 — "UI playback control is routed exclusively through the playerController facade (src/lib/player/), with playerEvents.ts inside the boundary as its event plumbing and player configuration commands explicitly outside it; enforced by scripts/check-player-boundary.sh." Category: Player. Traces to UR-005. Status: Done on merge.

// src/lib/player/index.ts
// TRACES: UR-005 | DR-095

New facade tests take @req-test: UT-089 onward (next free UT is UT-089; coordinate if landing alongside the sibling specs, which draw from the same pool).

Notes for the implementer

• A parallel Claude session may be active in this repo — git diff before "repairing" unexpected changes (CLAUDE.md §Gotchas).
• Order matters: §2 (fill gaps) → §3 (migrate, VideoPlayer.svelte last and alone) → §4 (add the check). Adding the check first turns master red.
• 🔴 VideoPlayer.svelte: no lifecycle calls after an await in onMount — it flips to HTML5 mode and breaks Android seek. Do not let a mechanical substitution introduce an await before a lifecycle call.
• The facade's requireHandle() may throw where a raw commands call did not. Check each migrated call site's error handling rather than assuming the try/catch still covers the same cases.
• playbackMode.ts interacts with remote-mode routing (play_on_session vs local MPV). Verify remote casting still works after migrating its playerPlayTracks call.
• This spec is deliberately the lowest priority of the audit batch: it is the largest diff and the only one carrying real regression risk, while the traceability gate is a few lines and restores a dead safety net.