diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b84e19dd..80d8106e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -105,6 +105,21 @@ jobs: --no-default-features --features postgres + - name: cargo check + test (sync_v2 — off by default, so unchecked otherwise) + # `crate::remote` (RFC-005) lives behind an off-by-default feature + # while its slices land, which means the step above never compiles + # a line of it. Without this step the whole tree would rot + # silently until the day it is switched on. Linux only: it adds no + # platform-specific code, and the Windows slot cannot run the app + # crate's test binary anyway (see below). + if: runner.os == 'Linux' + run: >- + cargo test + --manifest-path src-tauri/Cargo.toml + -p waveflow + --features sync_v2 + --all-targets + - name: cargo test (full workspace) if: runner.os == 'Linux' run: cargo test --manifest-path src-tauri/Cargo.toml --workspace diff --git a/CLAUDE.md b/CLAUDE.md index 91a7d0a3..f890b22f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,7 +56,7 @@ Inside `crates/app/src/`: - **`commands/`** — one module per domain (`library`, `playlist`, `smart_playlists`, `track`, `browse`, `player`, `scan`, `edit`, `profile`, `analysis`, `deezer`, `similar`, `lyrics`, `stats`, `wrapped`, `maintenance`, `radio`, `duplicates`, `preferences`, `plugins`, `canvas`, …), all registered in `lib.rs::generate_handler![]`. CRUD delegates to `waveflow_core::repository::sqlite::*`; IPC + state + filesystem + emit glue stays in the command. - **`audio/`** — 3-thread lock-free engine: `decoder.rs` (symphonia + rubato), `output.rs` (cpal callback on its own thread, SPSC `rtrb` ring), `state.rs` (`SharedPlayback` atomics), `analytics.rs`, `crossfade.rs`, `eq.rs`, `spectrum.rs`, `wasapi_exclusive.rs`. Topology: [`docs/architecture/audio.md`](docs/architecture/audio.md). -- **`dlna/`** (axum + SSDP, opt-in) · **`mpd/`** (TCP MPD protocol, opt-in) · **`media_controls.rs`** (souvlaki → SMTC / MPRIS / MediaRemote) · **`discord_presence.rs`** · **`queue.rs`** · **`player_actions.rs`** (shared control sequence) · **`sync/`** (outbound op emit) · **`backup.rs`** · **`db/`** (pool wiring + `migration_heal`). +- **`dlna/`** (axum + SSDP, opt-in) · **`mpd/`** (TCP MPD protocol, opt-in) · **`media_controls.rs`** (souvlaki → SMTC / MPRIS / MediaRemote) · **`discord_presence.rs`** · **`queue.rs`** · **`player_actions.rs`** (shared control sequence) · **`remote/`** (remote source + sync v2, feature `sync_v2`) · **`sync/`** (retired v1 protocol, feature `sync_v1` — both features off by default) · **`backup.rs`** · **`db/`** (pool wiring + `migration_heal`). - **Scanner** — the orchestrator `scan_folder_inner` stays app-side (it emits `scan:progress`); every pure helper lives in `waveflow_core::scanner::{extract, upserts}`. - **Database** — per-profile SQLite via sqlx + a global `app.db` for the profile list and app-wide settings. Migrations at `src-tauri/migrations/{app,profile}/`, compiled in via `sqlx::migrate!`. Layout: [`docs/architecture/storage.md`](docs/architecture/storage.md). diff --git a/docs/README.md b/docs/README.md index 06608d6f..9d7ee367 100644 --- a/docs/README.md +++ b/docs/README.md @@ -34,8 +34,9 @@ Long-form design documents that lock in cross-cutting architectural decisions be | ------------------------------------------------------------------- | -------- | -------------------------------------------------------------------------------------------------------- | | [RFC-001 — WaveFlow Server](rfcs/RFC-001-waveflow-server.md) | Accepted | Server, web, auth, sync, streaming, Phase 1 delivery plan | | [RFC-002 — Plugin SDK](rfcs/RFC-002-plugin-sdk.md) | Draft | WASM Component Model plugins for sources / metadata / UI, sideload distribution, desktop + server parity | -| [RFC-003 — Sync architecture v2](rfcs/RFC-003-sync-architecture.md) | Draft | Backfill, HLC ordering, per-entity CRDT conflict resolution, status UI. Supersedes RFC-001 §1.f. | +| [RFC-003 — Sync architecture v2](rfcs/RFC-003-sync-architecture.md) | Superseded by RFC-005 | Backfill, HLC ordering, per-entity CRDT conflict resolution. **Not** the server's RFC-003 — see [RFC-005](rfcs/RFC-005-remote-source-and-sync-v2.md#the-rfc-003-naming-trap). | | [RFC-004 — Community-DB](rfcs/RFC-004-community-database.md) | Draft | Opt-in shared metadata pool (lyrics, bios, BPM, etc.), LRCLIB pattern. Schema + endpoints + privacy. | +| [RFC-005 — Remote source + sync v2](rfcs/RFC-005-remote-source-and-sync-v2.md) | Accepted | The server catalogue as a separate remote source, `MusicServer` / `SyncProvider` seam, PKCE, journal-based user-data sync. | ## Contributing diff --git a/docs/architecture/invariants.md b/docs/architecture/invariants.md index 17093417..b2e58af4 100644 --- a/docs/architecture/invariants.md +++ b/docs/architecture/invariants.md @@ -152,6 +152,20 @@ Default it into the overflow ("⋯") menu via [`MoreActionsMenu`](../../src/comp Every outbound HTTP path (Deezer, Last.fm, similar, LRCLIB, the plugin registry) checks `offline::is_offline()` first and short-circuits to an empty payload or the cache. Persisted in `app_setting['network.offline_mode']`. **Treat new HTTP code paths the same way.** +### Remote user data never lands in the local tables + +[RFC-005](../rfcs/RFC-005-remote-source-and-sync-v2.md). Synchronized state describes the **server's** playlists, favourites, ratings, history, queue and shares, and those reference the **server's** tracks — which have no local counterpart. Writing them into `playlist` / `liked_track` / `track.rating` would leave two options, both wrong: fabricate local track rows for content that only exists on the server, or silently drop every entry. The first corrupts the library, the second makes sync look broken while reporting success. + +The projection therefore lives in its own `remote_*` tables and is **reconstructible**: dropping it and re-fetching `GET /api/v2/sync/snapshot` is always a valid recovery, and is what the apply path does when it meets a known event it cannot apply. `remote_mutation` is the one exception — it holds writes the server has not seen yet, so it must survive a projection reset. + +Matching a local file to a server track is deliberately out of scope and needs its own RFC. + +**Two RFCs are numbered 003.** The desktop's [RFC-003](../rfcs/RFC-003-sync-architecture.md) (hybrid logical clocks, superseded) has nothing to do with the server's RFC-003 (sync v2, accepted). Any instruction naming "RFC-003" must name the repository too, or it will be read as the wrong document. On the desktop side the accepted design is **RFC-005**. + +### The three sections below describe the retired v1 protocol + +They are accurate for `crate::sync` under the `sync_v1` feature, which is off by default and talks to a server generation that no longer exists. They stay until the v2 snapshot bootstrap is proven, because they are the only documented recovery path from a divergence. **Do not use them as a model for new work** — see the section above. + ### Outbound `playlist + field: "tracks"` ops carry a snapshot map Phase 1.j.b. Every command in [`commands/playlist.rs`](../../src-tauri/crates/app/src/commands/playlist.rs) that inserts tracks (`add_track_to_playlist`, `add_tracks_to_playlist`, `add_source_to_playlist`) calls [`sync::track_snapshots::build_snapshots(conn, &track_ids)`](../../src-tauri/crates/app/src/sync/track_snapshots.rs) inside the same SQLite transaction and folds the result into the outbound payload as `snapshots: { "": { title, artist?, duration_ms? } }`. diff --git a/docs/rfcs/RFC-003-sync-architecture.md b/docs/rfcs/RFC-003-sync-architecture.md index 4fcc1a5d..2457b58c 100644 --- a/docs/rfcs/RFC-003-sync-architecture.md +++ b/docs/rfcs/RFC-003-sync-architecture.md @@ -1,6 +1,16 @@ # RFC-003 — Sync architecture v2 -- **Status**: Draft +> **Superseded on 2026-08-10 by [RFC-005](RFC-005-remote-source-and-sync-v2.md).** +> The server it was designed against no longer exists: hybrid logical clocks, +> per-entity CRDT arbitration and digest reconciliation are all dropped in favour +> of a server-authoritative ordered journal. Kept for the problem statement +> below, which is still an accurate account of why the v1 protocol failed. +> +> **This is not the server's RFC-003.** `waveflow-server` has its own document +> with that number, describing the accepted v2 protocol. Any instruction +> mentioning "RFC-003" must name the repository. + +- **Status**: Superseded by RFC-005 - **Date**: 2026-06-12 - **Authors**: @InstaZDLL - **Supersedes**: RFC-001 §Phase 1.f sync (the practical parts — apply pipeline + ops log stay; semantics and protocol are redesigned). diff --git a/docs/rfcs/RFC-005-remote-source-and-sync-v2.md b/docs/rfcs/RFC-005-remote-source-and-sync-v2.md new file mode 100644 index 00000000..c4efb77a --- /dev/null +++ b/docs/rfcs/RFC-005-remote-source-and-sync-v2.md @@ -0,0 +1,428 @@ +# RFC-005 — Remote music source and user-data sync v2 + +- **Status**: Accepted +- **Date**: 2026-08-10 +- **Authors**: @InstaZDLL +- **Supersedes**: [RFC-003](RFC-003-sync-architecture.md) (desktop) — hybrid logical clocks, per-entity CRDT arbitration and digest reconciliation are all dropped, see [Why the v1 design retires](#why-the-v1-design-retires). +- **Server-side counterpart**: `waveflow-server` `docs/rfcs/RFC-003-waveflow-sync-v2.md` (accepted 2026-08-09) — **a different document with the same number**, see [the naming trap](#the-rfc-003-naming-trap). +- **Implementation**: `crate::remote` behind the `sync_v2` Cargo feature. + +--- + +## The situation this RFC answers + +The desktop's synchronization layer talks to a server protocol that no longer +exists. All six routes it consumes have zero occurrences in the server's current +source, and its sign-in flow depends on a web front-end that was removed. The +server is now authoritative over an ordered journal; the desktop was written +against a peer-to-peer model where clients arbitrated concurrent writes among +themselves. + +This is not an adaptation. It is a replacement of the protocol, and — more +consequentially — a change in **what synchronization means for the user**. + +## Decision 1 — the remote catalogue is a separate source, never merged + +The single most important consequence, and the one that reshapes the UI: + +> Synchronized state describes the **server's** playlists, favourites, ratings, +> history, queue and shares. Those reference the **server's** tracks. A server +> track has no local counterpart, and this protocol never invents one. + +So the incoming projection cannot be written into `playlist`, `liked_track` or +`track.rating`. Doing so would either fabricate local tracks for rows that only +exist on the server, or silently drop every entry — the first corrupts the local +library, the second makes sync look broken. The projection therefore lands in +its **own tables** (`remote_*`), is presented as a distinct source in the +sidebar, and is reconstructible: dropping it and re-fetching a snapshot is +always a valid recovery. + +Matching a local file to a server track is **out of scope** and needs its own +RFC. When it comes, the only automatic link allowed is an exact, unique +content-hash match; a MusicBrainz identifier is a suggestion the user confirms; +matching by title/artist/duration is explicitly forbidden. + +**What this costs.** Local playlists no longer travel between machines. That +capability existed in the v1 design and is genuinely lost. It cannot be +recovered without the matching layer above, because a local playlist is a list +of local files and nothing in the protocol can name those on another install. + +## Decision 2 — two seams, so sync stays a capability + +The desktop must be able to connect to any server speaking the Subsonic +protocol, not only to WaveFlow. That requirement drives the shape: one +mandatory interface for what every server does, one optional interface for what +only WaveFlow offers. + +```text + MusicServer (mandatory) SyncProvider (optional) + catalogue, search, playback, snapshot, changes, ack, socket + user-data per capability + │ │ + ┌──────┴────────┐ │ +SubsonicSource WaveflowSource ─────────────────┘ + /rest/* /api/v2/* (native end to end) +``` + +Between WaveFlow Desktop and WaveFlow Server we go through `/api/v2` +**always** — catalogue and playback included, not just synchronization. Routing +our own traffic through the compatibility façade would forfeit three things we +already have: mutation idempotency (only the v2 routes read the operation-id +header), full-text search (the façade still filters in memory), and native +pagination with typed projections. + +`WaveflowSource` is therefore an independent implementation, not a +`SubsonicSource` with sync bolted on. + +**Detection.** A Subsonic `ping` against WaveFlow answers `type="waveflow"`. +That field — not the extension list — decides whether `SyncProvider` is +available. + +> **Verified trap.** `getOpenSubsonicExtensions` returns an *empty* container +> today. A client that probed capabilities that way would conclude the server +> offers nothing, while it in fact offers the entire v2 API. + +## Decision 3 — remote identity is polymorphic + +```rust +enum RemoteIdentity { + Waveflow { account_id: Uuid, device_id: Uuid, cursor: i64 }, + Subsonic { username: String }, +} +``` + +A third-party server has no account UUID, no device notion and no cursor. +Putting those three fields in a shared struct would make them optional +everywhere and spread `unwrap` over cases that cannot occur. + +One desktop profile binds to one server account. A profile stays the local unit +of identity and session; a library is a content resource, so an account exposing +several libraries selects one (`active_library_id`) rather than spawning +artificial profiles. + +## Decision 4 — remote identifiers are opaque strings + +WaveFlow serializes its UUIDs, which makes its two surfaces interchangeable +without a translation table. Other servers emit textual identifiers of another +shape. So: never parse a remote identifier into a `Uuid`, index on the composite +key `(profile_id, remote_id)` — two servers can legitimately emit the same +string — and keep the catalogue cache separate from synchronized state, since +one is reconstructible and the other is not. + +## Decision 5 — Authorization Code + PKCE on loopback + +The desktop is a public client. It opens `/authorize` in the system +browser with `client_id`, `redirect_uri`, `code_challenge` (S256), `state` and +`device_name`; the consent screen posts them back with the browser session +attached and follows the redirect the server computes. The loopback listener +then exchanges `code` + `code_verifier` at `/api/v2/oauth/token`. + +The loopback listener and the random generator already exist for another +provider; only the protocol changes. + +Three details, all measured against a live server rather than inferred: + +> **A code is spent on first presentation**, whatever the outcome. Redeeming +> with a wrong verifier burns it — presenting the *correct* verifier afterwards +> still answers 401. Retrying a code is not a recovery path; the flow restarts +> from the beginning. + +> **The redirect URI is compared as a string at redemption.** Shape validation +> ignores the port, as RFC 8252 §7.3 asks, but the token endpoint compares the +> grant's URI with the presented one byte for byte — changing only the port +> answers 401. Binding the loopback listener *first* and building the URI once +> from the port obtained makes the two identical by construction. + +> **The refresh token rotates and the device survives it.** A refresh returns a +> new refresh token, the old one answers 401, and `device_id` is echoed back +> unchanged. So a rotation never invalidates queued mutations, and the +> device-adoption branch in the client is defensive, not routine. + +Worth recording as an observation rather than a decision: `/oauth/authorize` +authenticates with a **Bearer header**, not a browser cookie, so the consent +step is technically reachable without a browser at all. We still go through the +browser — it is what keeps the account password out of the desktop process — +but that is a deliberate choice, not a constraint the server imposes. + +A third-party server authenticates by username/password, token/salt or API key +instead. That is a second authentication shape to carry, not a degraded first. + +## Decision 6 — playback carries a Bearer header + +`GET /api/v2/tracks/{id}/stream` with `Authorization: Bearer`, accepting +`format`, `bitrate` and `offset_ms`, answering 206/416 on ranges. Sealed tickets +exist for consumers that cannot set a header — a browser `