From bf9a04fb687e067ebac07142fd081c78f02552fe Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 10 Aug 2026 15:23:45 +0200 Subject: [PATCH 01/14] docs(rfcs): accept RFC-005, the remote source and user-data sync v2 The desktop talks a protocol the server no longer implements. RFC-005 records the replacement and, more importantly, the decision that was written nowhere: synchronized state describes the server's playlists, favourites and ratings, which reference the server's tracks. Those have no local counterpart, and matching one is deliberately out of scope. Writing that projection into the local tables would leave two options, both wrong -- fabricate local rows for content that only exists on the server, or silently drop every entry. The first corrupts the library, the second makes a broken sync report success. So the projection gets its own reconstructible tables, and local playlists stop travelling between machines: a real capability lost, and one that cannot come back without the matching layer. Numbered 005 on purpose. Two documents were numbered RFC-003, one per repository, describing unrelated designs; the desktop's is now marked superseded and carries the warning in its header. Claude-Session: https://claude.ai/code/session_01N9WXrurarkoiT2jgimvyj1 --- docs/README.md | 3 +- docs/architecture/invariants.md | 14 + docs/rfcs/RFC-003-sync-architecture.md | 12 +- .../rfcs/RFC-005-remote-source-and-sync-v2.md | 261 ++++++++++++++++++ 4 files changed, 288 insertions(+), 2 deletions(-) create mode 100644 docs/rfcs/RFC-005-remote-source-and-sync-v2.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..b7d857c1 --- /dev/null +++ b/docs/rfcs/RFC-005-remote-source-and-sync-v2.md @@ -0,0 +1,261 @@ +# 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. + +> **Verified trap.** A code is **spent on first presentation**, whatever the +> outcome. A wrong verifier burns it. Retrying the same code is not a recovery +> path — the flow restarts from the beginning. + +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 `