diff --git a/.planning/phases/03-skip-system-session-persistence-settings/03-RESEARCH.md b/.planning/phases/03-skip-system-session-persistence-settings/03-RESEARCH.md new file mode 100644 index 0000000..1613257 --- /dev/null +++ b/.planning/phases/03-skip-system-session-persistence-settings/03-RESEARCH.md @@ -0,0 +1,771 @@ +# Phase 3: Skip System + Session Persistence + Settings — Research + +**Researched:** 2026-05-10 +**Domain:** Democratic vote-to-skip, session save/load, host settings configuration +**Confidence:** HIGH + +## Summary + +Phase 3 adds three tightly-interleaved features to the existing Jam Session codebase: a democratic skip vote system mirroring the Phase 2 vote toggle pattern, JSON-based session persistence in XDG-compliant directories, and a host settings panel synced via WebSocket. All three features rely entirely on Python stdlib (json, asyncio, pathlib, signal, os) and the already-installed runtime dependencies (mpv, yt-dlp, uvicorn/FastAPI). No new external packages are required. + +The skip system uses the same `asyncio.Lock`-protected `set[str]` pattern as `SessionManager._votes`, with a deterministic threshold formula (`floor(count * pct/100) + 1` with small-group exceptions). Cooldown enforcement uses `asyncio.get_running_loop().call_later()` — stdlib, no timer library needed. Session persistence uses `json.dumps()` + atomic `os.replace()` for corruption-resistant writes to `~/.local/share/jam-session/sessions/.json`. Settings use the same atomic pattern to `~/.config/jam-session/settings.json`. Shutdown handling uses `loop.add_signal_handler()` for SIGTERM/SIGINT interception with `loop.run_in_executor()` for blocking stdin prompts — fully covered by stdlib. + +**Primary recommendation:** Extend `SessionManager` with `_skip_votes: set[str]` and `_skip_cooldown_until: float`, extend `ConnectionManager.send_sync()` with skip state fields, add `client.skip_vote` / `client.host.settings` handlers to `handle_message()`, implement `_save_session()` / `_load_session()` / `_save_settings()` as plain `json.dumps()` + `os.replace()` atomic writes, and wire `loop.add_signal_handler(signal.SIGINT, ...)` / `loop.add_signal_handler(signal.SIGTERM, ...)` in lifespan startup. + +## Architectural Responsibility Map + +| Capability | Primary Tier | Secondary Tier | Rationale | +|------------|-------------|----------------|-----------| +| Skip vote counting/tracking | API / Backend (SessionManager) | — | Server is the authoritative source for vote state — never the client | +| Skip threshold computation | API / Backend (SessionManager) | — | Formula depends on server-side nickname count, threshold_pct from settings | +| Skip vote cooldown enforcement | API / Backend (server.py + SessionManager) | — | Server-enforced timer prevents race conditions from client clocks | +| mpv track advancement on skip | API / Backend (MusicController) | — | music.skip() already exists; skip trigger calls the same method | +| Session save (serialize queue to JSON) | API / Backend (server.py) | — | Serializes URLs+metadata; stream URLs are never persisted | +| Session load (deserialize + resolve) | API / Backend (server.py) | — | Resolves YouTube URLs fresh via yt-dlp on load | +| Settings read/write (JSON config) | API / Backend (server.py) | — | Settings are server-side truth; UI pushes changes, never reads independently | +| Settings UI rendering | Browser / Client (HTML/CSS) | — | Collapsible section, host-only visibility, slider+number+tooggle controls | +| Skip vote UI toggle + badge | Browser / Client (JS) | — | Optimistic UI updates, server.skip_update confirms/reconciles | +| Session save/load UI controls | Browser / Client (JS) | — | Buttons, name input, session picker list, stop-server modal | +| Shutdown signal handling (SIGTERM/Ctrl+C) | API / Backend (server.py lifespan) | — | asyncio signal handlers intercept OS signals, block on stdin prompt | +| Auto-backup (last-session.json) | API / Backend (server.py) | — | Fires on all shutdown paths; survives unexpected exits | +| Startup restore prompt | API / Backend (server.py lifespan) | — | Terminal stdin prompt before accepting connections | + +## User Constraints (from CONTEXT.md) + +### Locked Decisions + +- **D-01:** Skip vote toggle button in Now Playing — visible to all guests. Badge "N/M to skip". Filled icon (#e94560) when voted, hollow (#666) when not. 150ms scale pulse on toggle. Reuses Phase 2 vote toggle pattern. +- **D-02:** `client.skip_vote { action: "cast"|"remove" }` — server validates, broadcasts `server.skip_update { vote_count, threshold, triggered, cooldown_remaining }`. Threshold met → music.skip() + server.playback_update. +- **D-03:** Host instant skip and guest skip vote coexist — host sees instant skip AND skip vote toggle. +- **D-04:** Skip votes reset to 0 on every track change. Cooldown default 3s, configurable. Client shows disabled button + countdown during cooldown. +- **D-05:** Host controls row remains above skip vote toggle in Now Playing. +- **D-06:** Denominator = all connected guests. Not voting = implied "no." Guests joining mid-track don't affect denominator. +- **D-07:** Skip votes persist until track change — survive WebSocket drops. +- **D-08:** Threshold formula: 1 guest→1, 2→2, 3→2, 4+→`floor(count*threshold_pct/100)+1`. Default_pct=50. +- **D-09:** Host counts as a guest. Instant skip always works regardless. +- **D-10:** Saved session = JSON `{saved_at, tracks: [{url, title, source_id}]}`. No stream_urls. Resolve fresh on load via yt-dlp, reuse cache by source_id. +- **D-11:** Location: `~/.local/share/jam-session/sessions/.json`. +- **D-12:** Vote counts and skip votes reset on session load. +- **D-13:** Host Settings collapsible section at bottom. Visible only to host. Contains skip threshold slider (10-100%, default 50), auto-persist toggle (default off), skip cooldown slider (0-10s, default 3). +- **D-14:** Config file: `~/.config/jam-session/settings.json`. Read on startup. Created on first save. No hot-reload. +- **D-15:** Settings sync via WebSocket. Host sends `client.host.settings`, server validates, broadcasts `server.settings_update`. Async file write. +- **D-16:** Server in-memory settings = live truth. Config file = persistence only. +- **D-17:** Save/Load/Stop Server in Host Settings section. +- **D-18:** Three shutdown paths — browser button + modal, SIGTERM/Ctrl+C prompt, auto-backup last-session.json on all. +- **D-19:** Auto-persist writes last-session.json on every queue mutation. +- **D-20:** Loading session appends tracks (non-destructive). +- **D-21–D-23:** New WebSocket messages and sync extension. + +### the agent's Discretion + +None — all decisions were explicitly made by user. + +### Deferred Ideas (OUT OF SCOPE) + +None — discussion stayed within phase scope. + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|------------------| +| SK-01 | Guest can cast a vote-to-skip on the currently playing song | `_skip_votes: set[str]` in SessionManager, `client.skip_vote` handler, toggle button in Now Playing UI | +| SK-02 | Skip threshold defaults to 50%+1 of connected guests, configurable | Threshold formula in SessionManager, settings in `settings.json`, `server.settings_update` broadcast | +| SK-03 | When skip threshold is met, mpv advances to the next track | music.skip() (existing), threshold check in skip_vote handler | +| SK-04 | One skip vote per guest per track | Set-based deduplication in `_skip_votes`, reset on track change | +| SS-01 | Session queue and nicknames reset when the host closes the app | Shutdown handler clears session state, auto-backup writes last-session.json | +| SS-02 | Host is prompted to save before closing if queue is non-empty | SIGTERM handler with stdin prompt (run_in_executor), browser modal with Save/Stop/Cancel | +| SS-03 | Host can load a previously saved session | `_load_session()` reads JSON, resolves URLs via yt-dlp, appends tracks | +| SS-04 | Settings option to auto-persist queue across restarts | Auto-persist toggle in settings, writes last-session.json on queue mutations | +| ST-01 | Host can configure skip vote threshold (default 50%+1) | Range slider 10-100% in Host Settings, `skip_threshold_pct` in settings.json | +| ST-02 | Host can enable/disable auto-persist (default off) | Toggle switch in Host Settings, `auto_persist` in settings.json | +| ST-03 | Settings persist across server restarts | Atomic write to `~/.config/jam-session/settings.json`, read on lifespan startup | + +## Standard Stack + +### Core (all already installed — zero new dependencies) + +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| Python stdlib `json` | 3.14 built-in | Session file + settings file serialization | No third-party JSON library needed; stdlib handles the simple structures | +| Python stdlib `asyncio` | 3.14 built-in | Cooldown timer (`call_later`), signal handling (`add_signal_handler`), executor for stdin | All async primitives needed are in stdlib | +| Python stdlib `pathlib` | 3.14 built-in | XDG path construction, directory creation | Standard for filesystem paths | +| Python stdlib `os` | 3.14 built-in | Atomic file replacement (`os.replace`), directory creation, signal constants | Atomic writes prevent corruption | +| Python stdlib `signal` | 3.14 built-in | SIGTERM/SIGINT constants for handler registration | Required for graceful shutdown | + +### Runtime Dependencies (already installed on host) + +| Tool | Version | Purpose | Notes | +|------|---------|---------|-------| +| mpv | present | Audio playback, skip via `playlist_next` | Already managed by MusicController | +| yt-dlp | 2026.3.17 | YouTube URL resolution on session load | Already used in `resolve_youtube()` | +| ffmpeg | 7.1.2 | Audio processing (used internally by yt-dlp) | Already present | + +### Alternatives Considered + +| Instead of | Could Use | Tradeoff | +|------------|-----------|----------| +| stdlib `json` | `orjson` / `ujson` | Faster but adds a dependency for a dataset of <100 tracks | +| `asyncio.call_later` for cooldown | `asyncio.sleep` in a task | call_later is lighter — no task lifecycle to manage | +| `os.replace` for atomic writes | Write to tempfile + rename | Same thing; os.replace is equivalent | +| `loop.add_signal_handler` | `signal.signal` + `call_soon_threadsafe` | Signal module approach is thread-unsafe with asyncio; loop.add_signal_handler is the blessed path | + +**Installation:** +```bash +# No new packages required. Everything is stdlib or already installed. +``` + +**Version verification:** [VERIFIED: runtime environment] +- Python 3.14.3 (≥3.12 required) ✓ +- mpv (present on system) ✓ +- yt-dlp 2026.3.17 (present) ✓ +- ffmpeg 7.1.2 (present) ✓ + +## Architecture Patterns + +### System Architecture Diagram — Skip Vote Flow + +``` +┌─────────────────────────────────────────────────────────────────────────┐ +│ Guest Browser Host Browser │ +│ ┌────────────────┐ ┌──────────────────────────┐ │ +│ │ Skip Vote │ client.skip_vote │ Skip Vote Toggle + Host │ │ +│ │ Toggle (⏭) │────────────────────→│ Instant Skip (⏭) │ │ +│ │ + Badge (N/M) │ │ │ │ +│ └────────────────┘ └──────────────────────────┘ │ +│ ↑ ↑ │ +│ │ server.skip_update │ server.skip_update │ +│ │ (vote_count, threshold, │ + server.role │ +│ │ cooldown_remaining) │ │ +├─────────┼────────────────────────────────────────┼──────────────────────┤ +│ │ HOST MACHINE │ │ +│ │ │ │ +│ ┌──────┴────────────────────────────────────────┴─────────────────┐ │ +│ │ handle_message() (server.py) │ │ +│ │ ┌─────────────────────┐ ┌──────────────────────────────┐ │ │ +│ │ │ client.skip_vote │ │ client.host.settings │ │ │ +│ │ │ → toggle_skip_vote()│ │ → update_settings() │ │ │ +│ │ └─────────┬───────────┘ └──────────────┬───────────────┘ │ │ +│ └────────────┼──────────────────────────────┼─────────────────────┘ │ +│ │ │ │ +│ ┌────────────┴──────────────────────────────┴─────────────────────┐ │ +│ │ SessionManager │ │ +│ │ _skip_votes: set[str] _nicknames: dict[str, str] │ │ +│ │ _skip_cooldown_until: float _queue: list[Track] │ │ +│ │ │ │ +│ │ toggle_skip_vote(conn_id): │ │ +│ │ 1. Lock │ │ +│ │ 2. Validate cooldown not active │ │ +│ │ 3. Toggle conn_id in _skip_votes │ │ +│ │ 4. Compute threshold from nicknames + threshold_pct │ │ +│ │ 5. Check if len(_skip_votes) >= threshold │ │ +│ │ 6. Return (vote_count, threshold, triggered: bool) │ │ +│ └────────────────────────┬────────────────────────────────────────┘ │ +│ │ triggered === true │ +│ ┌────────────────────────┴────────────────────────────────────────┐ │ +│ │ MusicController │ │ +│ │ skip() → send_command(["playlist_next"]) → mpv IPC │ │ +│ └────────────────────────┬────────────────────────────────────────┘ │ +│ │ │ +│ ┌────────────────────────┴────────────────────────────────────────┐ │ +│ │ mpv process │ │ +│ │ Advances to next track → fires end-file event │ │ +│ │ → on_mpv_event("track_finished") │ │ +│ │ → _handle_track_finished() │ │ +│ │ → reset skip votes + start cooldown │ │ +│ └─────────────────────────────────────────────────────────────────┘ │ +└─────────────────────────────────────────────────────────────────────────┘ +``` + +### System Architecture Diagram — Session Save/Load + Settings Flow + +``` +┌─────────────────────────────────────────────────────────────────────────┐ +│ HOST MACHINE │ +│ │ +│ ┌──────────────────────────────────────────────────────────────────┐ │ +│ │ lifespan (server.py) │ │ +│ │ │ │ +│ │ STARTUP: │ │ +│ │ _load_settings() → ~/.config/jam-session/settings.json │ │ +│ │ _prompt_load_last() → ~/.local/share/.../last-session.json │ │ +│ │ add_signal_handler(SIGINT, _shutdown_handler) │ │ +│ │ add_signal_handler(SIGTERM, _shutdown_handler) │ │ +│ │ │ │ +│ │ SHUTDOWN (yield → after): │ │ +│ │ _auto_backup() → always write last-session.json │ │ +│ │ _prompt_save() → if queue non-empty & !auto_persist │ │ +│ │ stop_mdns(), music.stop_mpv() │ │ +│ └──────────────────────────────────────────────────────────────────┘ │ +│ │ +│ SAVE FLOW (any time during runtime): │ +│ Host clicks "Save Session" │ +│ → client.host.save_session { name } │ +│ → _save_session(name): │ +│ tracks = [{url, title, source_id} for t in queue] │ +│ data = {saved_at: ISO, tracks} │ +│ write atomic: tmp → os.replace(tmp, final) │ +│ → ~/.local/share/jam-session/sessions/.json │ +│ → server.session_saved { name, track_count } → toast │ +│ │ +│ LOAD FLOW: │ +│ Host clicks session in picker list │ +│ → client.host.load_session { name } │ +│ → _load_session(name): │ +│ Read JSON, for each track: │ +│ resolve_youtube(url) → reuse cache by source_id │ +│ Track(title, duration, stream_url, source_id, ...) │ +│ session.add_track() for each resolved track │ +│ broadcast queue_update for each │ +│ → server.session_loaded { name, track_count } → toast │ +│ │ +│ SETTINGS FLOW: │ +│ Host changes slider/toggle │ +│ → 300ms debounce │ +│ → client.host.settings { skip_threshold_pct, auto_persist, │ +│ skip_cooldown_seconds } │ +│ → validate, update in-memory config │ +│ → _save_settings(config): atomic write json │ +│ → ~/.config/jam-session/settings.json │ +│ → broadcast server.settings_update to ALL clients │ +│ │ +│ AUTO-PERSIST FLOW: │ +│ On every queue mutation (add/remove/reorder): │ +│ if auto_persist: _auto_backup() → last-session.json │ +│ Survives crash — last write is on disk │ +└─────────────────────────────────────────────────────────────────────────┘ +``` + +### Recommended Project Structure (additions only — no changes to existing files' locations) + +``` +jam_session/ +├── server.py # ADD: _load_settings(), _save_settings(), +│ # _save_session(), _load_session(), +│ # _auto_backup(), _prompt_save(), +│ # _prompt_load_last(), +│ # _shutdown_handler(), +│ # signal handlers in lifespan, +│ # skip_vote + settings cases in handle_message() +├── session.py # ADD: _skip_votes: set[str], +│ # _skip_cooldown_until: float, +│ # _skip_denominator: int, +│ # toggle_skip_vote(conn_id), +│ # get_skip_state(), +│ # reset_skip_votes(), +│ # start_skip_cooldown(seconds) +├── ws.py # ADD: broadcast_skip_update(), +│ # broadcast_settings_update(), +│ # EXTEND: send_sync() with skip state + +│ # settings fields +├── models.py # ADD: SettingsConfig model (may be dict, +│ # not worth a full model — see analysis) +└── static/ + ├── index.html # ADD: skip vote button in #now-playing-info, + │ #
after #share, + │ # stop-server modal overlay + ├── app.js # ADD: skipVoted flag, settings state, + │ # skip vote toggle handler, + │ # settings change handlers (debounced), + │ # save/load/stop server handlers, + │ # session picker render, + │ # cases in handleWsMessage switch, + │ # EXTEND: renderFullState for skip+settings + └── style.css # ADD: skip vote button styles (reuse .vote-btn), + │ # skip badge styles, + │ # host-settings section styles, + │ # slider + number input styles, + │ # toggle switch styles, + │ # modal overlay + card styles, + │ # session picker item styles +``` + +### Pattern 1: Set-Based Skip Voting (mirrors existing `_votes` dict) + +**What:** Track skip votes as `set[str]` of conn_ids who have voted to skip. Toggle membership. Check `len(set) >= threshold`. Reset set to empty on track change. + +**When to use:** Every skip vote cast, removal, threshold check, and track change. + +**Example:** +```python +# Source: Extending existing SessionManager._votes pattern (session.py:42) +# Verified against codebase + +class SessionManager: + def __init__(self): + # ... existing fields ... + self._skip_votes: set[str] = set() # conn_ids that voted to skip + self._skip_cooldown_until: float = 0.0 # loop.time() when cooldown ends + self._skip_denominator: int = 0 # snapshot at track start + + async def toggle_skip_vote(self, conn_id: str) -> dict: + """Returns skip state for broadcast. Must be called under lock.""" + now = asyncio.get_running_loop().time() + if now < self._skip_cooldown_until: + return { + "vote_count": len(self._skip_votes), + "threshold": self._compute_threshold(), + "triggered": False, + "cooldown_remaining": int(self._skip_cooldown_until - now), + } + + if conn_id in self._skip_votes: + self._skip_votes.discard(conn_id) + else: + self._skip_votes.add(conn_id) + + threshold = self._compute_threshold() + triggered = len(self._skip_votes) >= threshold + + return { + "vote_count": len(self._skip_votes), + "threshold": threshold, + "triggered": triggered, + "cooldown_remaining": 0, + } +``` + +### Pattern 2: Atomic JSON Write + +**What:** Serialize to temp file, then `os.replace(tmp, final)` for atomic replacement on same filesystem. Never truncate-then-write (corruption window). + +**When to use:** Saving sessions, saving settings, auto-backup writes. + +**Example:** +```python +# Source: Python stdlib os.replace() docs +# [VERIFIED: runtime test — os.replace works on same filesystem] + +import json +import os +from pathlib import Path + +def atomic_write_json(path: Path, data: dict) -> None: + """Atomically write JSON data to path.""" + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_suffix(path.suffix + ".tmp") + tmp.write_text(json.dumps(data, indent=2, default=str)) + os.replace(tmp, path) # atomic on same filesystem +``` + +### Pattern 3: Signal-Based Graceful Shutdown with Stdin Prompt + +**What:** Register `loop.add_signal_handler(signal.SIGINT, handler)` and `loop.add_signal_handler(signal.SIGTERM, handler)` in lifespan startup. Handler flags shutdown, which lifespan's post-yield cleanup detects and runs save prompts + auto-backup. + +**When to use:** SIGTERM/Ctrl+C shutdown path (D-18 path 2). + +**Example:** +```python +# Source: Python 3.14 asyncio docs — loop.add_signal_handler +# [VERIFIED: signal.SIGTERM, signal.SIGINT available in Python 3.14.3] + +import asyncio +import signal +import sys + +_shutdown_requested = False + +def _signal_handler(): + global _shutdown_requested + _shutdown_requested = True + +async def lifespan(app: FastAPI): + loop = asyncio.get_running_loop() + loop.add_signal_handler(signal.SIGINT, _signal_handler) + loop.add_signal_handler(signal.SIGTERM, _signal_handler) + + # ... startup ... + + yield + + # Shutdown: auto-backup always + _auto_backup() + + # Prompt if queue non-empty and auto-persist OFF + if session._queue and not config.get("auto_persist"): + await _prompt_save_terminal() + + music.stop_mpv() +``` + +**Stdin prompt during shutdown:** +```python +# Blocking stdin read must run in executor (doesn't block event loop): +async def _prompt_save_terminal(): + loop = asyncio.get_running_loop() + n = len(session._queue) + prompt = f"Queue has {n} tracks. Save session before exit? (y/N/name): " + response = await loop.run_in_executor(None, input, prompt) + # ... handle response ... +``` + +### Pattern 4: Debounced Settings Broadcast + +**What:** Frontend accumulates settings changes for 300ms, then sends `client.host.settings` with full settings state. Server validates, updates in-memory config, writes file async, broadcasts to all. + +**When to use:** Every settings slider/toggle change from host. + +**Anti-Patterns to Avoid** + +- **Anti-pattern: Client-side threshold computation.** Threshold must be computed server-side using `SessionManager._nicknames` (authoritative). The badge shows the server-computed values. Clients never compute M in "N/M to skip." +- **Anti-pattern: Truncate-then-write for JSON files.** If the process crashes mid-write, the file is corrupted. Always use write-to-tmp + atomic rename. +- **Anti-pattern: Using `time.sleep` for cooldown.** This blocks the entire event loop. Use `loop.call_later()` to schedule a callback that re-enables skip votes after the cooldown period. +- **Anti-pattern: `signal.signal()` directly with asyncio.** The C-level signal handler runs outside the asyncio event loop and cannot safely call loop methods. Use `loop.add_signal_handler()` exclusively. + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Cooldown timer | Custom timer class or thread | `asyncio.get_running_loop().call_later(delay, callback)` | Stdlib, integrates with event loop, no thread overhead | +| Atomic file writes | Open-write-close (corruption window) | write to `.tmp` + `os.replace(tmp, final)` | Atomic on same filesystem, no partial writes survive | +| Signal handling for graceful exit | `signal.signal()` + `call_soon_threadsafe` or polling flag | `loop.add_signal_handler(signum, callback)` | Thread-safe, integrates with event loop, no polling | +| Blocking stdin during shutdown | `input()` directly in async context | `loop.run_in_executor(None, input, prompt)` | Doesn't block event loop; mpv keeps playing during prompt | +| JSON serialization | Custom serializer | `json.dumps(data, indent=2, default=str)` | Handles datetime, Track dicts via default=str | +| Relative time for session picker | Custom date math | Stdlib `datetime` arithmetic, manual "X ago" string builder | Simple enough — no library justified for this scale | +| Settings slider+number sync | Custom binding logic | Vanilla JS `input` event + bidirectional DOM update | Trivial; 30 lines of JS | + +**Key insight:** All timer/signal/I/O needs are fully covered by Python stdlib. The codebase already uses `asyncio.Lock`, `subprocess`, and `threading` — no new patterns to introduce. + +## Runtime State Inventory + +> Phase 3 is a greenfield features phase (new capabilities, not rename/refactor). No runtime state migration needed. Existing mpv process, in-memory queue, and cache index persist as-is. New filesystem directories created on first use. + +| Category | Items Found | Action Required | +|----------|-------------|------------------| +| Stored data | AudioCacheManager cache_index.json at `~/.cache/jam-session/cache_index.json` — carries source_id keys | None — session load reuses cache by source_id; no migration | +| Live service config | None — no external services configured in UI/database | None | +| OS-registered state | None — no systemd/pm2/launchd registrations | None | +| Secrets/env vars | None — host_token is generated per-session, never persisted | None | +| Build artifacts | None — no compiled/extracted packages | None | + +**Nothing found in category:** Verified by checking ~/.local/share/jam-session, ~/.config/jam-session, and known locations from existing code (cache_dir, socket path). No runtime state carries old names or needs migration. + +## Common Pitfalls + +### Pitfall 1: Skip Vote Race Condition — Threshold Met Twice + +**What goes wrong:** Two guests vote to skip near-simultaneously. Both votes push the count past the threshold. music.skip() gets called twice, skipping two tracks instead of one. + +**Why it happens:** The threshold check and skip call are not atomic. Between "check count >= threshold" and "call skip()", another vote arrives and also triggers. + +**How to avoid:** Use a `_skip_triggered: bool` flag set to `True` before calling `music.skip()`. Threshold check includes `and not _skip_triggered`. Reset the flag on track change in `_handle_track_finished()`. + +**Warning signs:** +- Two tracks advance after a skip threshold is met +- "Skipping…" appears in UI, then next track also skips immediately + +**Prevention code pattern:** +```python +# In SessionManager.toggle_skip_vote(): +if vote_count >= threshold and not self._skip_triggered: + self._skip_triggered = True + result["triggered"] = True +# In reset_skip_votes(): +self._skip_triggered = False +``` + +### Pitfall 2: Cooldown Timer Leaks — Zombie call_later Callbacks + +**What goes wrong:** A cooldown timer is scheduled via `call_later`, but before it fires, a new track change occurs and starts a new cooldown. The old callback fires and resets cooldown state, clobbering the new cooldown. + +**Why it happens:** `call_later` returns a `TimerHandle` but it's not stored/cancelled. Multiple concurrent timers can exist. + +**How to avoid:** Store the `TimerHandle` returned by `call_later()`. Before scheduling a new cooldown, call `.cancel()` on the previous handle. After the callback fires, set the handle to `None`. + +**Warning signs:** +- Cooldown ends early (stale callback fires) +- Cooldown countdown jumps to 0 unexpectedly +- "Skip in 0s" appears briefly then goes back to countdown + +### Pitfall 3: Session File Corruption on Crash During Write + +**What goes wrong:** Server writes session JSON directly to the target file. Crashes mid-write. File contains truncated JSON. On next load, json.loads() raises JSONDecodeError. + +**Why it happens:** Direct `open(path, 'w').write(data)` has a corruption window. If the process dies between open and close, partial data is on disk. + +**How to avoid:** Never write directly to the target file. Write to `.json.tmp`, then `os.replace(tmp, target)`. If the crash happens during the tmp write, the target file is untouched. If it happens during replace, the OS guarantees atomicity (either old or new file exists, never partial). + +**Warning signs:** +- `JSONDecodeError` on startup when loading sessions +- "Could not load session: file may be damaged" toasts + +### Pitfall 4: Skip Denominator Changing Mid-Track (Guests Joining/Leaving) + +**What goes wrong:** A guest joins mid-track, expanding the denominator and making skip harder. Or guests disconnect, shrinking denominator and making skip easier — potentially triggering a skip when the count hasn't changed. + +**Why it happens:** Denominator computed from live `_nicknames` count rather than a snapshot taken at track start. + +**How to avoid:** Store `_skip_denominator = len(self._nicknames)` at the moment `_handle_track_finished()` runs (track change). Use this snapshot for all threshold computations for the duration of that track. Only update on the next track change. + +**Warning signs:** +- Badge shows "2/3 to skip" then changes to "2/4 to skip" after someone joins +- Song unexpectedly skips when a guest disconnects (denominator shrinks) + +### Pitfall 5: Auto-Persist Flooding Disk on Queue Mutations + +**What goes wrong:** Auto-persist writes `last-session.json` on every queue mutation. If someone adds 20 songs rapidly, that's 20 disk writes in rapid succession. + +**Why it happens:** No debounce on auto-persist writes. Each add/remove/reorder triggers immediately. + +**How to avoid:** Implement a write debounce for auto-persist only: schedule a write for 500ms in the future via `call_later`, cancel and reschedule if a new mutation arrives within that window. + +**Warning signs:** +- Disk I/O spikes during bulk song additions +- Multiple `last-session.json` writes logged within 1 second + +## Code Examples + +### Skip Vote Toggle Handler (server.py handle_message) + +```python +# Source: Verified against existing handle_message pattern (server.py:125) +elif msg_type == "client.skip_vote": + payload = msg.get("payload", {}) + action = payload.get("action") + + if action not in ("cast", "remove"): + await manager.send_error( + manager.active_connections.get(conn_id), + "Invalid action for skip_vote" + ) + return + + # Get nickname (required — server enforces nicknames first) + nickname = session.get_nickname(conn_id) + if not nickname: + await manager.send_error( + manager.active_connections.get(conn_id), + "Set a nickname before voting to skip" + ) + return + + async with session._lock: + # Reject if during cooldown + now = asyncio.get_running_loop().time() + if now < session._skip_cooldown_until: + await manager.send_error( + manager.active_connections.get(conn_id), + "Skip voting is in cooldown" + ) + return + + if action == "cast": + if conn_id in session._skip_votes: + return # idempotent — already voted + session._skip_votes.add(conn_id) + else: # remove + if conn_id not in session._skip_votes: + return # idempotent — not voted + session._skip_votes.discard(conn_id) + + threshold = session._compute_skip_threshold() + triggered = len(session._skip_votes) >= threshold and not session._skip_triggered + + if triggered: + session._skip_triggered = True + music.skip() # advances mpv — track_finished event will reset votes + + await manager.broadcast({ + "type": "server.skip_update", + "payload": { + "vote_count": len(session._skip_votes), + "threshold": threshold, + "triggered": triggered, + "cooldown_remaining": 0, + } + }) +``` + +### Atomic Settings Save + +```python +# Source: Python stdlib os.replace() — verified working in Python 3.14.3 + +SETTINGS_PATH = Path.home() / ".config" / "jam-session" / "settings.json" + +def _save_settings(config: dict) -> None: + """Atomically write settings to XDG config directory.""" + SETTINGS_PATH.parent.mkdir(parents=True, exist_ok=True) + tmp = SETTINGS_PATH.with_suffix(".json.tmp") + tmp.write_text(json.dumps(config, indent=2)) + os.replace(tmp, SETTINGS_PATH) + +def _load_settings() -> dict: + """Load settings with defaults. Creates file if doesn't exist.""" + defaults = { + "skip_threshold_pct": 50, + "auto_persist": False, + "skip_cooldown_seconds": 3, + } + if SETTINGS_PATH.exists(): + try: + data = json.loads(SETTINGS_PATH.read_text()) + # Merge with defaults (handles missing keys) + return {**defaults, **data} + except (json.JSONDecodeError, OSError): + pass + # Create file with defaults on first run + _save_settings(defaults) + return defaults +``` + +### Cooldown with call_later (SessionManager) + +```python +# Source: asyncio docs — loop.call_later() +# [VERIFIED: asyncio available in Python 3.14.3] + +class SessionManager: + def __init__(self): + # ... + self._cooldown_handle: asyncio.TimerHandle | None = None + + def start_skip_cooldown(self, seconds: int): + """Start cooldown after track change. Cancels any existing.""" + if self._cooldown_handle is not None: + self._cooldown_handle.cancel() + self._cooldown_handle = None + + loop = asyncio.get_running_loop() + self._skip_cooldown_until = loop.time() + seconds + + def _on_cooldown_end(): + self._cooldown_handle = None + + self._cooldown_handle = loop.call_later(seconds, _on_cooldown_end) + + def reset_skip_votes(self): + """Reset skip state on track change.""" + self._skip_votes.clear() + self._skip_triggered = False + self._skip_denominator = len(self._nicknames) +``` + +### Threshold Formula (server-side only) + +```python +# Source: 03-CONTEXT.md D-08 (user decision) +def _compute_skip_threshold(nickname_count: int, threshold_pct: int) -> int: + """Compute skip vote threshold from connected guest count and percentage.""" + if nickname_count == 1: + return 1 + if nickname_count == 2: + return 2 + if nickname_count == 3: + return 2 + # 4+ guests: floor(count * pct / 100) + 1 + return int(nickname_count * threshold_pct / 100) + 1 +``` + +## State of the Art + +| Old Approach | Current Approach | When Changed | Impact | +|--------------|------------------|--------------|--------| +| skip vote tracked manually | `set[str]` with conn_id (matches existing `_votes` dict) | Always been the plan | Consistent pattern; easy auditing | +| No session persistence | JSON with atomic writes via `os.replace` | This phase | Zero-dependency persistence; corruption-resistant | +| Hardcoded settings | JSON config file with WebSocket sync | This phase | Survives restarts; UI is source of truth | +| No graceful shutdown | `loop.add_signal_handler` + executor stdin prompts | This phase | Stdlib only; no new deps | + +**Deprecated/outdated:** +- **`signal.signal()` for asyncio apps:** Deprecated by asyncio community consensus. `loop.add_signal_handler` is the correct API. +- **Direct file writes without atomic swap:** The truncate-then-write pattern has been the cause of countless corrupted config files. Always use temp+rename. + +## Assumptions Log + +| # | Claim | Section | Risk if Wrong | +|---|-------|---------|---------------| +| A1 | `asyncio.get_running_loop()` is available in the signal handler callback context [ASSUMED based on asyncio docs pattern] | Shutdown handling | Loop not running → RuntimeError; mitigated by checking loop.is_running() | +| A2 | `os.replace()` is atomic on the host's filesystem (likely ext4/btrfs) [ASSUMED] | Atomic writes | Non-atomic FS (rare) → partial writes survive; mitigated by tmp+rename pattern being safe even on non-atomic FS for the common case | +| A3 | `loop.run_in_executor(None, input, prompt)` works for blocking stdin during shutdown [ASSUMED] | Terminal prompt | If executor pool is exhausted or shutdown race occurs → prompt hangs; mitigated by timeout wrapper | + +## Open Questions + +1. **Session name collision behavior** + - What we know: Save overwrites existing file with same name (atomic replace handles this safely) + - What's unclear: Should server warn "Session 'jam-session-2026-05-10' already exists. Overwrite?" or silently overwrite? + - Recommendation: Silent overwrite for v1 (host is the only user, unlikely to conflict accidentally). Can add confirmation in Phase 4 if needed. + +2. **Settings model — dedicated Pydantic class or plain dict?** + - What we know: Settings has 3 fields (threshold_pct, auto_persist, cooldown_seconds). QueueState is a Pydantic model. Settings are simpler and read/written as raw JSON. + - What's unclear: Is a dedicated Pydantic `SettingsConfig` model worth the boilerplate for 3 fields? + - Recommendation: Use a plain `dict` with defaults, validated in `client.host.settings` handler via manual checks (range assertions, type checks). A Pydantic model is overkill for 3 values that change infrequently. If settings grow beyond 5 fields, refactor to a model then. + +3. **`last-session.json` overwrite on fast successive shutdowns** + - What we know: Auto-backup writes on every shutdown. If shutdown is triggered, then cancelled mid-way, a stale `last-session.json` may exist. + - What's unclear: Should `last-session.json` include a `server_pid` or generation counter to detect stale backups? + - Recommendation: Not needed for v1. The startup prompt asks "Load last session? (y/N):" — host can say no if the backup is clearly stale. Add generation counter in v1.x if this becomes confusing. + +## Environment Availability + +| Dependency | Required By | Available | Version | Fallback | +|------------|------------|-----------|---------|----------| +| Python 3.12+ | Everything | ✓ | 3.14.3 | — | +| mpv | Audio skip (playlist_next) | ✓ | present | — | +| yt-dlp | Session load URL resolution | ✓ | 2026.3.17 | — | +| ffmpeg | Audio processing (yt-dlp internal) | ✓ | 7.1.2 | — | +| uvicorn / FastAPI | Web server (already running) | ✓ | installed | — | +| json (stdlib) | Session + settings serialization | ✓ | built-in | — | +| asyncio (stdlib) | Cooldown timer, signal handling | ✓ | built-in | — | +| pathlib (stdlib) | XDG path construction | ✓ | built-in | — | +| signal (stdlib) | SIGTERM/SIGINT constants | ✓ | built-in | — | +| ~/.local/share/jam-session/ | Session file storage | ✗ | — | Auto-created with `mkdir(parents=True, exist_ok=True)` | +| ~/.config/jam-session/ | Settings file storage | ✗ | — | Auto-created with `mkdir(parents=True, exist_ok=True)` | + +**Missing dependencies with no fallback:** +- None — all runtime dependencies are present. Filesystem directories are created on first use. + +**Missing dependencies with fallback:** +- XDG directories → auto-created by `Path.mkdir(parents=True, exist_ok=True)` on first save/load. + +## Security Domain + +> `security_enforcement` is not explicitly disabled in config.json → treat as enabled. + +### Applicable ASVS Categories + +| ASVS Category | Applies | Standard Control | +|---------------|---------|-----------------| +| V2 Authentication | no | No authentication — ephemeral nicknames, no accounts | +| V3 Session Management | no | Sessions are ephemeral; no login sessions to manage | +| V4 Access Control | yes (partial) | Host token validation (existing), settings mutation restricted to host conn_id | +| V5 Input Validation | yes | Validate all WebSocket payloads: action enum, threshold range (10-100), cooldown range (0-10), session name length, JSON structure | +| V6 Cryptography | no | No cryptographic operations in this phase | + +### Known Threat Patterns for FastAPI + WebSocket + JSON Files + +| Pattern | STRIDE | Standard Mitigation | +|---------|--------|---------------------| +| Malformed JSON in session file | Tampering | `try/except JSONDecodeError` on load, fall back to empty/default | +| Session name path traversal ("../../etc/passwd") | Tampering / Elevation | Sanitize session name: allow only `[A-Za-z0-9_-]`, reject paths with `/` or `..` | +| Non-host sending `client.host.settings` | Spoofing | Validate `conn_id == session.get_host_conn_id()` before processing settings mutations | +| Settings value out of range (threshold 999%) | Tampering | Server-side range validation (10 ≤ threshold_pct ≤ 100, 0 ≤ cooldown ≤ 10) | +| Overlarge session name DoS (1MB name) | Denial of Service | Cap session name length at 100 characters | +| Duplicate skip vote spam (rapid cast/remove) | Denial of Service | Single-vote-per-track enforced by set membership; rate-limit not needed | + +## Sources + +### Primary (HIGH confidence) +- Codebase audit (`jam_session/session.py`, `server.py`, `ws.py`, `music.py`, `models.py`, `cache.py`, `static/app.js`, `static/index.html`, `static/style.css`) — existing patterns, integration points, reusable assets [VERIFIED: codebase grep + read] +- Python 3.14.3 stdlib — `json`, `asyncio`, `pathlib`, `os`, `signal` availability [VERIFIED: runtime probes] +- FastAPI lifespan events docs (https://fastapi.tiangolo.com/advanced/events/) — lifespan pattern already in use [VERIFIED: codebase match] +- Python asyncio signal handling docs — `loop.add_signal_handler` API [VERIFIED: runtime availability check] +- 03-CONTEXT.md — all 23 user decisions (D-01 through D-23) [CITED: .planning/phases/03-skip-system-session-persistence-settings/03-CONTEXT.md] +- 03-UI-SPEC.md — design system, component inventory, interaction contracts, copywriting [CITED: .planning/phases/03-skip-system-session-persistence-settings/03-UI-SPEC.md] + +### Secondary (MEDIUM confidence) +- .planning/research/ARCHITECTURE.md — system overview, component responsibilities, data flow patterns [CITED: existing research artifact] +- .planning/research/PITFALLS.md — race conditions, vote state loss, session corruption patterns [CITED: existing research artifact] +- Phase 2 UI-SPEC (02-UI-SPEC.md) — design system and interaction patterns carried forward [CITED: phase 2 artifact] + +### Tertiary (LOW confidence) +- None — research based entirely on verified codebase state, runtime probes, and locked user decisions. + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH — no new dependencies; all tools verified present via runtime probes +- Architecture: HIGH — all patterns map directly to existing code; all integration points identified and cross-referenced with current source +- Pitfalls: HIGH — identified from existing PITFALLS.md patterns applied to skip/session/settings context, plus first-principles analysis of the new code paths +- Environment: HIGH — all tools probed and versions confirmed; missing dirs auto-created + +**Research date:** 2026-05-10 +**Valid until:** 2026-06-09 (30 days — stable domain, no fast-moving dependencies)