diff --git a/.github/labeler.yml b/.github/labeler.yml index faf72f9f..10d00f56 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -35,40 +35,39 @@ 'scope: api': - changed-files: - any-glob-to-any-file: - - 'src/http.rs' - - 'src/services.rs' + - 'src/api/**' + - 'src/app/**' + - 'src/http/**' + - 'src/services/**' - 'src/lib.rs' 'scope: db': - changed-files: - any-glob-to-any-file: - - 'src/database.rs' - - 'src/catalog.rs' + - 'src/storage/**' - 'migrations-v2/**' 'scope: auth': - changed-files: - any-glob-to-any-file: - - 'src/authentication.rs' - - 'src/oauth.rs' - - 'src/security.rs' + - 'src/auth/**' 'scope: subsonic': - changed-files: - any-glob-to-any-file: - - 'src/subsonic.rs' + - 'src/subsonic/**' - 'docs/subsonic-compatibility.md' 'scope: streaming': - changed-files: - any-glob-to-any-file: - - 'src/media.rs' - - 'src/stream_ticket.rs' + - 'src/media/**' + - 'src/auth/stream_ticket/**' 'scope: scanner': - changed-files: - any-glob-to-any-file: - - 'src/scanner.rs' + - 'src/library/**' # ───────────────────────────────────────────────────────────── # Web fine scopes (inside webapp/src/). diff --git a/.github/workflows/ci-rust.yml b/.github/workflows/ci-rust.yml index 2b73276c..1265cda2 100644 --- a/.github/workflows/ci-rust.yml +++ b/.github/workflows/ci-rust.yml @@ -12,6 +12,7 @@ on: - "Cargo.toml" - "Cargo.lock" - "build.rs" + - "scripts/check-rust-layout.sh" - "src/**" - "migrations-v2/**" - "tests/**" @@ -24,6 +25,7 @@ on: - "Cargo.toml" - "Cargo.lock" - "build.rs" + - "scripts/check-rust-layout.sh" - "src/**" - "migrations-v2/**" - "tests/**" @@ -79,6 +81,12 @@ jobs: - name: Install FFmpeg run: sudo apt-get update && sudo apt-get install -y --no-install-recommends ffmpeg + # The shape the restructuring gave the tree: no source file past 800 + # lines, no test file past 1,000, no unit test inline in the code. The + # limits and the one allowed exception live in the script, with why. + - name: Rust layout + run: sh scripts/check-rust-layout.sh + - name: cargo fmt run: cargo fmt --all --check @@ -88,6 +96,14 @@ jobs: - name: cargo check run: cargo check --all-targets --all-features + # An intra-doc link that resolves nowhere is a warning rustdoc prints and + # nobody reads; here it fails the build, so a renamed item cannot leave + # its links behind. + - name: cargo doc + env: + RUSTDOCFLAGS: -D warnings + run: cargo doc --no-deps --all-features + - name: cargo test run: cargo test --all-features diff --git a/AGENTS.md b/AGENTS.md index a64d4851..45e6c54a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,11 +20,14 @@ cargo test --all-features Tests are hermetic and use temporary SQLite databases. No PostgreSQL or Better Auth process is required. +Unit tests live in `tests.rs` beside their module (`foo/mod.rs` declares `#[cfg(test)] mod tests;`), never inline. Integration targets are declared in `Cargo.toml`; a large one is a directory whose `main.rs` declares one file per subject. CI runs `sh scripts/check-rust-layout.sh`: no source file under `src/` past 800 lines, no `tests.rs` or file under `tests/` past 1,000, no inline `mod tests {`. It also runs `cargo doc --no-deps --all-features` with `RUSTDOCFLAGS="-D warnings"`. + ## v2 conventions - `src/main.rs` only loads config, initializes state, dispatches CLI commands and serves the router. Testable runtime behaviour belongs behind library functions. -- All environment access lives in `src/config.rs`. -- SQL stays in `src/database.rs` or later repository modules. Handlers orchestrate HTTP only. +- `src/` is filed by domain; only `main.rs` and `lib.rs` sit at its root. `app/` (boot, state, router), `cli/`, `config/`; the surfaces `api/` and `subsonic/` over the one domain `services/`; `auth/` (sessions, sign-in guard, OAuth, setup token, instance-key cryptography, stream tickets); `library/` (scanner, watcher, tags, PIDs, credit splitting, playlist files, lyrics parsing); `storage/` (database, catalogue, sync journal, collation, pre-migration copy); `remote/` (outbound client, enrichment, portraits, LRCLIB, podcast feeds, scrobblers); `http/` (security headers, public share, `/reference`, web client); `media/`; `quality/`. A new module goes under the parent whose domain it serves, never at the root. +- All environment access lives in `src/config/`. +- SQL stays in `src/storage/database/`, `src/storage/catalog/` or `src/services/`. Handlers orchestrate HTTP only. - SQLite connections enable WAL, foreign keys and `busy_timeout`. All mutations acquire the single process-wide writer gate; libraries may scan concurrently but never create independent SQLite writers. - Migrations under `migrations-v2/` are immutable once merged. Schema changes always add a new dated migration. - Tenant isolation is enforced in repository queries through account plus library membership, not only in handlers. diff --git a/CLAUDE.md b/CLAUDE.md index 89eeca4e..c142d13d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co The accepted design is [RFC-002](docs/rfcs/RFC-002-waveflow-server-v2.md). **Read it before adding a module or a public route.** [`AGENTS.md`](AGENTS.md) holds the short form of the same rules plus the milestone sequence; this file is the deeper reference. -**An RFC's `Status` field never flips.** Every RFC under `docs/rfcs/` says `Proposed` whether it shipped a year ago or not at all, so the header is not evidence about the code — a `Status` line has misled a reader into holding shipped work as pending. What each one carries instead is an **`Implemented by`** line naming the pull requests, which can actually be checked. (Both keys were `Statut` and `Implémentée par` in the five French RFCs until #222 moved the series to English; a document quoting the old spellings is dated, not wrong.) And when it matters, read the code rather than either: the routes registered in `src/lib.rs` and the dated files under `migrations-v2/` are the only account of what exists. +**An RFC's `Status` field never flips.** Every RFC under `docs/rfcs/` says `Proposed` whether it shipped a year ago or not at all, so the header is not evidence about the code — a `Status` line has misled a reader into holding shipped work as pending. What each one carries instead is an **`Implemented by`** line naming the pull requests, which can actually be checked. (Both keys were `Statut` and `Implémentée par` in the five French RFCs until #222 moved the series to English; a document quoting the old spellings is dated, not wrong.) And when it matters, read the code rather than either: the routes registered from `src/app/router.rs` and the dated files under `migrations-v2/` are the only account of what exists. **Documentation is written in English.** Every RFC, analysis, plan, guide and handoff under `docs/`, and these instruction files with them. The design @@ -22,7 +22,15 @@ English. Layout: -- `/` — the server crate (`waveflow-server`). +- `/` — the server crate (`waveflow-server`). Under `src/`, only `main.rs` and `lib.rs` sit at the root; every module is filed by domain: + - `app/` — boot, `AppState`, the router and the OpenAPI document; `cli/` — the command line; `config/` — every setting. + - `api/` — `/api/v2`; `subsonic/` — the OpenSubsonic façade; `services/` — the domain both call (see below). + - `auth/` — accounts and sessions (`authentication`), the sign-in guard (`sign_in`), native-client OAuth (`oauth`), the setup token, the instance key's cryptography (`security`) and stream tickets. + - `library/` — reading the music on disk: `scanner/`, `watcher/`, the tag mapper (`tags`), derived identifiers (`pid`), `credit_split`, `link_roots`, `playlist_file` and `lyrics` (the LRC/TTML parsers and the sheets they make). + - `storage/` — SQLite: `database/` (the pool and the foundation repositories), `catalog/` (the catalogue repository), `sync` (the user-data change journal), `natural_order` (the collation registered on the pool) and `migration_backup`. + - `remote/` — everything that talks to another host: the outbound client (`outbound`), `enrichment/`, `portrait`, `lrclib`, `podcast` (the feed fetcher) and `scrobblers/`. + - `http/` — HTTP outside the two API surfaces: `security_headers`, `public_share`, `reference` (the `/reference` page) and `webui`. + - `media/` — streaming, transcoding, artwork, canvases and `replay_gain`; `quality/` — the audio analysis. - `webapp/` — the embedded React client, built to `webapp/dist` and compiled in by `rust_embed`. Under `webapp/src/`, only `main.tsx` (the router and the providers) sits at the root; everything else is filed by what it is: `api/` (the HTTP client, one file per resource over the session and transport in `client.ts`, re-exported by `index.ts` so the rest of the app imports `../api`; then the query keys and the query client), `pages/` (one screen per file), `components/` (pieces shared between screens), `player/`, `state/` (contexts: i18n, preferences, library scope), `lib/` (logic with no React in it) and `styles/` (the sheet and the design tokens). A test sits beside the file it tests. The desktop app at [`InstaZDLL/WaveFlow`](https://github.com/InstaZDLL/WaveFlow) is a separate repository. It consumes this API but is not part of this codebase. @@ -37,6 +45,8 @@ cargo fmt --all --check # CI gate cargo clippy --all-targets --all-features -- -D warnings # CI gate; warnings are errors cargo test --all-features # hermetic; temporary SQLite databases cargo test --all-features --test subsonic_contract # one test, in one target +RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features # CI gate; a link that resolves nowhere fails +sh scripts/check-rust-layout.sh # CI gate; file sizes and no inline unit tests bun --cwd=webapp install bun run build # webapp first, then cargo — the order matters @@ -46,27 +56,29 @@ Tests need no service container: `test_app()` builds a `Config` over a `TempDir` The integration suite is twenty-three targets under `tests/`, one per subject — `auth`, `scanner`, `identity`, `catalog`, `subsonic_contract`, `subsonic_browse`, `subsonic_fields`, `subsonic_methods`, `native_api`, `sync`, `media`, `service`, `cli`, `scrobbling`, `enrichment`, `credits`, `quality`, `uploads`, `canvas`, `discovery`, `smart_playlists`, `podcasts`, `migrations` — nearly all of them sharing `tests/support/mod.rs`. `migrations` is the exception and deliberately so: every other target starts from `test_app()`, which builds its database from nothing, and that is exactly the path an upgrade does not take. It seeds a database at each **released** schema, boots the server on it, and reads the rows back through the services — and it is where anything else that needs a database *behind* this binary belongs, such as proving that a key change reads without migrating and copies the pair aside before it does. `autotests = false`, so each is a target only because `Cargo.toml` declares it: a new file under `tests/` is compiled by nothing until a `[[test]]` entry names it, and the count above is worth checking against that list rather than trusted. `support/mod.rs` is the exception that proves it — shared module code, declared nowhere, a target never. A target too large for one file becomes a directory and stays one target: its `[[test]]` entry points at `tests//main.rs`, which reaches the shared module through `#[path = "../support/mod.rs"]`, keeps the imports and helpers its files share, and declares one file per surface with `mod`; each of those starts with `use super::*;`. The target's name does not change, so `cargo test --test native_api ` still finds a test, now under its module (`playlists::…`) — and a new file in such a directory is compiled by nothing until `main.rs` declares it, the same trap one level down. File a new test by the surface it exercises, and check that by what it calls rather than by what its name says. +Unit tests never sit inline in the code. A module with unit tests is a directory: the code in `foo/mod.rs`, which declares `#[cfg(test)] mod tests;`, and the tests in `foo/tests.rs` beside it, reaching private items through `use super::*;` — never a `foo.rs` with a `foo/` beside it. CI holds the sizes too (`scripts/check-rust-layout.sh`): a source file under `src/` stays within 800 lines, a `tests.rs` under `src/` or any file under `tests/` within 1,000, and no file under `src/` opens `mod tests {`. Past a limit, split along the file's seams into a directory, as the restructuring did, rather than raising it; the script's allowlist holds one file, a single test that cannot be cut without changing it, and says why. + `cargo build` works without a client build — `build.rs` embeds a placeholder page when `webapp/dist` is absent. A real `vite build` overwrites it. ## Architecture & conventions -- **Library/binary split.** `src/main.rs` only loads config, dispatches CLI commands and serves. The router is built by `waveflow_server::app(&config, state)` in `src/lib.rs`, so tests spawn the same app in-process. Put logic behind `app()`, not in `main`. -- **`AppState`** (`src/lib.rs`) holds the shared singletons: `db`, `auth`, `secret_box`, `scanner`, `media`, `services`, plus config values copied in (`artwork_dir`, `public_url`, `stream_ticket_ttl`). Add new singletons there. -- **Three directories carry the surfaces.** `src/api/` is `/api/v2`, one module per resource plus `error`, `access` and `web_session`; `src/subsonic/` is the façade, one module per method family; `src/services/` is the domain, several `impl DomainServices` blocks split by domain. Each `mod.rs` keeps only what its children share — the router or dispatch, the cross-cutting types, and for `services` the SQL projection macros, which sit ahead of the `mod` declarations so their textual scope reaches every child. File a new handler under the resource it serves, not in `mod.rs`. -- **All environment access lives in `src/config.rs`.** Every tunable is a field on `Config` with its env var documented on it. -- **No SQL in handlers.** SQL belongs in `src/database.rs`, `src/catalog.rs` or `src/services/`; handlers orchestrate HTTP only. +- **Library/binary split.** `src/main.rs` only loads config, dispatches CLI commands and serves. The router is built by `waveflow_server::app(&config, state)` in `src/app/router.rs`, re-exported by `src/lib.rs`, so tests spawn the same app in-process. Put logic behind `app()`, not in `main`. +- **`AppState`** (`src/app/state.rs`) holds the shared singletons: `db`, `auth`, `secret_box`, `scanner`, `media`, `services`, plus config values copied in (`artwork_dir`, `public_url`, `stream_ticket_ttl`). Add new singletons there. +- **Three directories carry the surfaces.** `src/api/` is `/api/v2`, one module per resource plus `error`, `access` and `web_session`; `src/subsonic/` is the façade, one module per method family; `src/services/` is the domain, several `impl DomainServices` blocks split by domain. Each `mod.rs` keeps only what its children share — the router or dispatch, the cross-cutting types, and for `services` the SQL projection macros, which sit ahead of the `mod` declarations so their textual scope reaches every child. File a new handler under the resource it serves, not in `mod.rs`. Everything else in `src/` is filed by domain (see Layout): a new module goes under the parent whose domain it serves — `auth/`, `library/`, `storage/`, `remote/`, `http/`, `media/` — never at the root, and each parent's `mod.rs` holds only its `//!` and the `pub mod` lines. +- **All environment access lives in `src/config/`.** Every tunable is a field on `Config` with its env var documented on it. +- **No SQL in handlers.** SQL belongs in `src/storage/database/`, `src/storage/catalog/` or `src/services/`; handlers orchestrate HTTP only. - **One set of domain services.** `DomainServices` (`src/services/`) is the single implementation behind the native API, the Subsonic façade and the web client. A mutation reachable from two surfaces must call the same method — that convergence is the point of M4, and duplicating logic per surface is how the two drift. - **Tenancy is enforced in the queries**, through `library_member`, not in handlers. The shared projections are `song_select!` / `album_select!` / `artist_select!` macros that `concat!` into literals: sqlx only accepts static SQL, so composing them stays injection-proof by construction. The first bind is always the user id. - **404 blurs everything.** A resource that is missing, and one that belongs to another account, answer identically. `ServiceError::Forbidden` maps onto 404 for that reason. Never confirm existence to someone not entitled to it. - **SQLite discipline.** WAL, foreign keys, `busy_timeout`. Every mutation takes the process-wide writer gate (`db.writer_guard()`); libraries may scan concurrently but never open independent writers. `NULL` sorts first in SQLite — order explicitly with `NULLS LAST` wherever a missing tag would otherwise jump the queue. - **Migrations are immutable once merged.** They are embedded at compile time and checksummed, so editing an applied migration makes the server refuse to start. Schema changes are new dated files under `migrations-v2/`. -- **Identifiers and time.** Public ids are UUIDs; timestamps are Unix epoch milliseconds. Album and artist ids are *derived* UUIDs (version 8, RFC 9562 §5.8): the identity engine in `src/pid.rs` hashes the tags that name the entity, under the configurable spec `WAVEFLOW_PID_ALBUM`/`WAVEFLOW_PID_ARTIST`, so the same files answer the same ids on a fresh install. Track ids stay drawn at random — six tables cascade off them, and a scan matches a track by path, then by content hash, then by the relocation hint in `track.pid`. That hint is the track spec evaluated over the file's tags, and it stays a hint: it is asked only when the first two have nothing to say — a file both moved and re-encoded — and it moves nothing unless it names exactly one track whose file has genuinely disappeared. Changing `WAVEFLOW_PID_TRACK` drops every stored hint on the next boot: a hint written under one spec must never be compared against a lookup computed under another, because two specs can evaluate to the same string and the hint would then hand a new file an unrelated track's identity. The next ordinary scan writes them back from its skip path, so clearing them costs no rescan — and a track spec change still re-identifies nothing. Changing a spec re-identifies everything it governs: the next boot notices, schedules a full rescan of every library, and album favourites do not survive it. A credit decision (RFC-015) is narrower: splitting a joined credit such as `Zedd, Alessia Cara` re-identifies only the artists it names. The album display string and the track hint read the tag mapper's names, before any decision, so no album or track id moves, and favourites on the joined artist are carried onto its parts through the journal. +- **Identifiers and time.** Public ids are UUIDs; timestamps are Unix epoch milliseconds. Album and artist ids are *derived* UUIDs (version 8, RFC 9562 §5.8): the identity engine in `src/library/pid/` hashes the tags that name the entity, under the configurable spec `WAVEFLOW_PID_ALBUM`/`WAVEFLOW_PID_ARTIST`, so the same files answer the same ids on a fresh install. Track ids stay drawn at random — six tables cascade off them, and a scan matches a track by path, then by content hash, then by the relocation hint in `track.pid`. That hint is the track spec evaluated over the file's tags, and it stays a hint: it is asked only when the first two have nothing to say — a file both moved and re-encoded — and it moves nothing unless it names exactly one track whose file has genuinely disappeared. Changing `WAVEFLOW_PID_TRACK` drops every stored hint on the next boot: a hint written under one spec must never be compared against a lookup computed under another, because two specs can evaluate to the same string and the hint would then hand a new file an unrelated track's identity. The next ordinary scan writes them back from its skip path, so clearing them costs no rescan — and a track spec change still re-identifies nothing. Changing a spec re-identifies everything it governs: the next boot notices, schedules a full rescan of every library, and album favourites do not survive it. A credit decision (RFC-015) is narrower: splitting a joined credit such as `Zedd, Alessia Cara` re-identifies only the artists it names. The album display string and the track hint read the tag mapper's names, before any decision, so no album or track id moves, and favourites on the joined artist are carried onto its parts through the journal. - **Credentials.** Web passwords use Argon2id. Access, refresh, API and OAuth codes are stored as SHA-256 hashes only. The dedicated Subsonic password is encrypted with ChaCha20-Poly1305 under the instance key. Back up `data/waveflow.db` and `data/instance.key` together — encrypted values cannot be recovered with one without the other. -- **Browser playback uses stream tickets** (`src/stream_ticket.rs`). `