feat(server-auth): waveflow-server account binding (Phase 1.f.desktop.1)#189
Conversation
Foundational desktop wiring for the waveflow-server sync surface. Every
later sub-PR in 1.f.desktop.* reads its config through the module added
here.
* **Schema** β new migration extends the `auth_credential` provider
CHECK to include `'waveflow_server'`, mirroring the existing
Last.fm / Spotify rebuild pattern. Per-profile by design: each
desktop profile maps to one Better Auth account, so a profile
switch swaps the server identity along with the local library.
* **`src/server_client.rs`** β three responsibilities:
- read/write of `app_setting['app.waveflow_server_url']` (app-wide
base URL; validates parseability + http(s) scheme)
- read/write/clear of the per-profile JWT, with a structural sanity
check on three dot-separated segments before persistence
- `WaveflowServerClient::try_build` that returns an HTTP client
pre-baked with the base URL + Bearer header. Currently
`#[allow(dead_code)]` β sync (`1.f.desktop.2`+) is the first
consumer, kept here so the foundational PR ships the complete
auth-header attachment shape for review.
* **`commands/server_auth.rs`** β five `#[tauri::command]` entries:
`server_get_status`, `server_set_url`, `server_set_token`,
`server_sign_out`, `server_open_login_browser`. Every mutating
command returns a `ServerStatus` so the UI re-syncs on the same
round-trip.
* **Settings β IntΓ©grations β "Compte serveur WaveFlow"** β new card
with two inputs (server URL + JWT paste), an "Open login" button
that opens the configured URL in the default browser, an
emerald-tinted "Signed in" badge and a sign-out button. The
cancelled-flag pattern on the initial fetch keeps the React hooks
v7 `set-state-in-effect` rule happy.
* **i18n** β `settings.serverAccount.*` keys propagated to all 17
locales (`ar de en es fr hi id it ja ko nl pt pt-BR ru tr zh-CN zh-TW`)
per the convention in CLAUDE.md. Brand tokens (WaveFlow, JWT, URL,
Better Auth) stay verbatim across locales.
Sign-in flow today is **manual paste**: the user opens the server URL
in their browser, signs in via Better Auth, copies the issued JWT and
pastes it back into the card. A polished local-loopback OAuth flow
(mirroring `commands::spotify`'s tiny_http listener) ships in a
follow-up `1.f.desktop.1b` PR once the matching `/desktop-login` route
lands on `waveflow-web`.
OS-keyring storage and JWT refresh on 401 are explicitly out of scope
β documented in the `server_client` module docstring as 1.f.desktop.1b
/ 1.f.desktop.4 work.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
No actionable comments were generated in the recent review. π βΉοΈ Recent review infoβοΈ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: π Files selected for processing (4)
π WalkthroughWalkthroughCette PR ajoute une interface de configuration pour lier un compte serveur WaveFlow au profil desktop, exposant cinq commandes Tauri pour lire/Γ©crire l'URL du serveur et le token JWT, avec persistance en base de donnΓ©es, validation stricte et un composant React intΓ©grΓ© dans la section Integrations des ParamΓ¨tres. ChangesServer Account Configuration
Sequence DiagramsequenceDiagram
participant User
participant ServerAccountCard as ServerAccountCard (React)
participant serverAuthWrapper as serverAuth (TS)
participant Tauri as Tauri IPC
participant server_auth as server_auth (Rust commands)
participant server_client as server_client (persistence)
User->>ServerAccountCard: Enter URL & save
ServerAccountCard->>serverAuthWrapper: serverSetUrl(url)
serverAuthWrapper->>Tauri: invoke("server_set_url", {url})
Tauri->>server_auth: server_set_url
server_auth->>server_client: write_url + status
server_client->>server_auth: ServerStatus
server_auth->>Tauri: return ServerStatus
Tauri->>serverAuthWrapper: ServerStatus
serverAuthWrapper->>ServerAccountCard: Promise<ServerStatus>
ServerAccountCard->>User: Display updated status
Estimated code review effortπ― 4 (Complex) | β±οΈ ~45 minutes Possibly related PRs
Suggested labels
Poem
π₯ Pre-merge checks | β 5β Passed checks (5 passed)
βοΈ Tip: You can configure your own custom pre-merge checks in the settings. β¨ Finishing Touchesπ Generate docstrings
π§ͺ Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
π€ Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/views/settings/ServerAccountCard.tsx`:
- Around line 59-70: Les appels RPC doivent envoyer les valeurs trimmed: in
handleSaveUrl and the analogous JWT handler (e.g., handleSaveJwt), trim the
input drafts before calling serverSetUrl/serverSetJwt to avoid backend
validation failures from pasted whitespace/newlines; assign const trimmed =
urlDraft.trim() (or jwtDraft.trim()) and use that trimmed variable in the RPC
call and any subsequent state updates (e.g., setStatus), keeping existing
error/busy handling intact.
πͺ Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
βΉοΈ Review info
βοΈ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 30706853-5d23-4bcb-9a8c-e20881cb51aa
π Files selected for processing (25)
src-tauri/crates/app/src/commands/mod.rssrc-tauri/crates/app/src/commands/server_auth.rssrc-tauri/crates/app/src/lib.rssrc-tauri/crates/app/src/server_client.rssrc-tauri/migrations/profile/20260601000000_waveflow_server_auth_provider.sqlsrc/components/views/SettingsView.tsxsrc/components/views/settings/ServerAccountCard.tsxsrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh-CN.jsonsrc/i18n/locales/zh-TW.jsonsrc/lib/tauri/serverAuth.ts
@coderabbitai on PR #189 flagged that the handlers were sending the raw draft strings. The backend already trims, so this isn't a functional bug β but reflecting the normalised value in the input keeps the visible draft in sync with the persisted row without a refresh round-trip, and trimming the JWT pre-send guards against a trailing newline from a browser copy-paste tripping the three-segment structural check. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
β¦.f.desktop.2) (#191) * feat(sync): lamport clock + pending-ops queue infrastructure (Phase 1.f.desktop.2) Local-side foundation for the multi-device sync protocol from RFC-001 Β§6.6. Every sub-PR downstream (CRUD enqueue hooks 1.f.desktop.2b, Settings toggle + repo swap 1.f.desktop.3, WS subscriber + drain 1.f.desktop.4) reads the storage shape and the clock helpers shipped here. ## Schema - New per-profile migration `20260602000000_sync_pending_op.sql`. `sync_pending_op` is the local write-ahead log: BIGSERIAL-style `id` for FIFO ordering, UNIQUE on `operation_id` (server idempotency key) AND on `lamport_ts` (local defence in depth before the server's 23505 hits), CHECK on `op β {set, delete, insert, noop}`. Retry bookkeeping in `last_attempt_at` + `attempt_count` + `last_error`. ## Modules - `crate::sync::device` β stable per-install `device_id` (UUIDv4), lazy-generated on first read, persisted app-wide in `app_setting['sync.device_id']`. Reinstall = new id (correct behaviour; the previous install's Lamport history isn't recoverable). App-wide rather than per-profile so two profiles sharing a desktop emit ops under the same `device_id` β the server's `(user_id, device_id, lamport_ts)` UNIQUE already gives each profile its own Lamport space via the differing `user_id`. - `crate::sync::lamport` β per-profile monotonic clock. `next()` atomically increments + returns the new value via a single UPSERT + RETURNING (no SELECT-then-UPDATE race). `observe_remote(remote)` bumps the local floor past a remote `lamport_ts` so the next local op slots above it. Persisted in `profile_setting['sync.lamport_local_max']`. - `crate::sync::queue` β append-only local queue with `enqueue` / `list_pending` / `drop_acked` / `mark_failed` / `count_pending` / `clear`. `enqueue` assigns the `operation_id` itself so nothing leaks across layers; `list_pending` hydrates the row into a typed `PendingOp` with parsed JSON payload; `drop_acked` batches via `QueryBuilder::push_bind` (sqlx 0.9 refuses dynamically-built `&str` on the `query()` path). ## Diagnostics - `commands/sync.rs` exposes `sync_get_queue_state` (returns `{ device_id, lamport_local_max, pending_count }`) and `sync_clear_pending` (nuclear option for the Settings panel). Both registered in `lib.rs`. ## Tests - 15 unit tests on the helpers, all against in-memory sqlite pools with the production schema copy-pasted from the migration: - `device`: UUID generation + idempotence on re-read, `read` returns None on fresh DB. - `lamport`: monotonic increment, observe-remote bumps past local, observe-remote refuses to lower (the SQLite type- affinity gotcha: `max(int, text)` returns text > int, so both sides need a `CAST AS INTEGER` before the scalar `max` call), observe-remote handles 0 / negative as no-op, seeds clock on fresh profile. - `queue`: enqueue writes row + returns ids, FIFO order on list_pending, lamport UNIQUE rejects duplicates, drop_acked removes specified rows + handles empty input, mark_failed bumps counter + records error string, CHECK rejects unknown op string, count + clear observable. ## Not in this PR - CRUD enqueue hooks across `commands/{playlist,library,edit}`. ~15 command sites; cleaner to ship in a follow-up 1.f.desktop.2b so each hook reviews against the helpers already in tree. - Canonical-id mapping for cross-device entity identity. Today `entity_id` is the local i64 coerced to TEXT, which is fine for ops produced by THIS device. Cross-device requires a `local_id β canonical_id` mapping table that 1.f.desktop.4 introduces alongside the WS subscriber. - The drain task itself β 1.f.desktop.4. Helpers that aren't called yet (`ensure`, `next`, `observe_remote`, `enqueue`, `list_pending`, `drop_acked`, `mark_failed`) carry a module-level `#![allow(dead_code)]` with a justification comment pointing at the consuming sub-PR. Same pattern PR #189 used for `WaveflowServerClient`. Signed-off-by: InstaZDLL <github.105mh@8shield.net> * fix(sync): cr findings on pr #191 β log + guard + autoincrement @coderabbitai surfaced three findings; all valid, all applied with minimal blast radius: 1. **commands/sync.rs swallowed the `require_profile_pool` error.** `Err(_) => (0, 0)` silently rendered "0 pending / 0 lamport" for *every* failure mode, not just the legitimate "no active profile" path. Now binds `err` and emits a `tracing::warn!` with the cause before falling back to defaults β operator can correlate the UI surface with the real reason. 2. **`queue::list_pending` didn't guard against non-positive limit.** SQLite treats `LIMIT -1` as "no limit" and returns the entire table β a caller miscomputing the page size would pull every queued op into memory at once. Added an early return on `limit <= 0`. Test `list_pending_guards_against_non_positive_limit` pins both edge cases. 3. **`sync_pending_op.id INTEGER PRIMARY KEY` reused ids after deletes.** Without `AUTOINCREMENT`, the next insert picks `MAX(rowid) + 1` of the surviving rows β so once the queue is fully drained, the counter restarts at 1. A future drain task that tracks a `last_processed_id` high-water mark would silently miss every op appended past the reset. Bumped the column to `INTEGER PRIMARY KEY AUTOINCREMENT` (cost: one extra `sqlite_sequence` write per insert, lost in the JSON-payload serialise the same INSERT already does). Test `ids_stay_monotonic_after_clear` pins the regression. Migration safe to bump in place β the file is on a feature branch that hasn't shipped yet, so no production install has applied it. Signed-off-by: InstaZDLL <github.105mh@8shield.net> --------- Signed-off-by: InstaZDLL <github.105mh@8shield.net>
β¦.f.desktop.4a) (#196) * feat(sync): drain task pushes pending ops to waveflow-server (1.f.desktop.4a) Reveille the WaveflowServerClient struct that has been dormant since #189 and pipes the local sync_pending_op queue into POST /api/v1/sync/ops. One-way push for now; WebSocket subscriber + apply remote ops + canonical-id mapping all land in a follow-up 1.f.desktop.4b. ## sync::drain Background task spawned at boot from lib.rs. Loop alternates between a 30 s periodic tick and a tokio::sync::Notify wake fired by CRUD command sites after a successful tx.commit() β a chatty user's edits reach the server within ms instead of waiting the full poll interval. Two gates short-circuit the pass without an HTTP round-trip: 1. WaveflowServerClient::try_build β both the server URL and the active profile JWT must be configured. 2. SyncMode::Hybrid β Local-mode profiles keep their queue local. Either gate yields DrainOutcome::Skipped. ## Failure semantics - HTTP 200 β drop_acked removes the rows, loop continues to drain any newly-arrived ops. - HTTP 409 lamport_regression β lamport::observe_remote bumps the local floor past the server's view, mark_failed on the offending row, break the loop. The next pass retries with the bumped clock. - Other HTTP statuses + network errors β mark_failed the batch with the server reply for diagnostics, break. The periodic poll re-attempts later. ## Tauri surface - New command sync_drain_now for the Settings diagnostics "Push now" affordance β runs drain_once synchronously and returns the DrainOutcome. - sync_set_mode now notifies the drain task on the Hybrid switch so flipping the radio doesn't wait 30 s to fire the first push. ## Boot wiring - AppState gains drain: Arc<DrainHandle>, default-initialised so callers can notify() against it harmlessly before the task spawns. - lib.rs::run spawns the task right after app.manage(state) so app.state::<AppState>() resolves and the same Arc<Notify> is shared by command sites + the task. ## Notify-on-commit in playlist commands All 8 mutation sites in commands/playlist.rs gain a state.drain.notify() right after tx.commit(). No-op when the drain task isn't spawned yet (very brief window between state.manage and drain::spawn). ## Tests - 25 desktop sync unit tests still green (+3 new in sync::drain pinning DrainOutcome's snake_case discriminant tag, PushBatchRequest wire shape vs waveflow-server, and LamportRegression deserialisation from a server 409 body). ## Out of scope (1.f.desktop.4b) - WebSocket subscriber + apply remote ops on local SQLite. - Canonical-id mapping for cross-device entity identity. Today's entity_id is the local i64 coerced to TEXT, which is fine for the push direction since the server keys ops on (user_id, device_id, entity, entity_id) β different devices' ops live in disjoint namespaces. Cross-device replay needs the mapping table the WS subscriber will introduce. Signed-off-by: InstaZDLL <github.105mh@8shield.net> * fix(sync): serialise drain passes + mark batch failed on 409 parse fail @coderabbitai surfaced two valid issues on PR #196: 1. drain_once could be called concurrently by the background tick and the sync_drain_now Tauri command. Both would read the same sync_pending_op batch and POST it twice β the server absorbs the duplicates via the operation_id UNIQUE, but the wasted round-trip + duplicated total_sent accounting is avoidable. Added a shared Arc<tokio::sync::Mutex<()>> on AppState (drain_lock). Both call sites acquire it before drain_once and hold the guard across the await; a concurrent caller waits for the in-flight pass to finish rather than racing it. 2. When the 409 body failed to parse as LamportRegression, the loop just broke after a tracing::warn. The rows stayed at attempt_count=0, so the next pass would hit the same 409 forever without any diagnostic trail. Now mark_failed the whole batch with the parse error string before breaking, matching the shape of the other-status and network-error branches. 25/25 sync tests still green, clippy + fmt clean. Signed-off-by: InstaZDLL <github.105mh@8shield.net> --------- Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Foundational desktop wiring for the waveflow-server sync surface β Phase 1.f.desktop.1 of #133. Closes the auth gap so the next three sub-PRs can land on top:
1.f.desktop.2β Lamport clock + pending-ops queue1.f.desktop.3β Settings "Mode serveur" toggle + repo swap1.f.desktop.4β WebSocket subscribe + apply remote opsThis PR ships only the auth + HTTP-client foundation. No sync code paths consume
WaveflowServerClientyet β it's marked#[allow(dead_code)]with a comment explaining the next consumer.Summary
auth_credentialprovider'waveflow_server'via a profile migration (rebuild-table pattern matchingadd_spotify_auth_provider).src-tauri/crates/app/src/server_client.rsβ URL / JWT persistence +WaveflowServerClient(reqwest wrapper with Bearer auto-attach). Single source of truth for the server binding; documented surface for the upcoming sync sub-PRs.#[tauri::command]entries incommands/server_auth.rs:server_get_status,server_set_url,server_set_token,server_sign_out,server_open_login_browser. Every mutating command returns a freshServerStatusso the UI re-syncs on the same round-trip.ServerAccountCard.tsx) β URL input + JWT paste textarea + "Open login" button + emerald "Signed in" badge + Sign out. The cancelled-flag pattern on initial fetch keeps the React hooks v7set-state-in-effectrule happy without lifting state.settings.serverAccount.*keys propagated to all 17 locales per the CLAUDE.md convention. Brand tokens (WaveFlow, JWT, URL, Better Auth) verbatim across locales.Sign-in flow
Today: manual paste. The user clicks "Ouvrir la connexion", their default browser opens the configured server URL, they sign in via Better Auth (
waveflow-web), copy the issued JWT, and paste it back. Works against the currentwaveflow-webdeploy without any web-side change.Follow-up
1.f.desktop.1b: replace the paste step with a local-loopback OAuth listener (mirroringcommands::spotify'stiny_httppattern). That requires a small companion PR onwaveflow-webto add a/desktop-login?cb=β¦route that mints the JWT viaauth.api.getTokenand redirects to the localhost callback. Tracked separately so this PR stays focused.Out of scope (deferred, documented in module docstring)
keyringcrate ships fine on macOS / Windows; the Linux story relies on libsecret + a running secret service, which I want to validate against an AppImage build before committing to it. JWT lives in the per-profileauth_credential.token_encryptedblob today (same storage shape as Last.fm, ListenBrainz, Spotify).waveflow-webyet.1.f.desktop.4adds this once the server-fn lands.Test plan
Summary by CodeRabbit
New Features
Chores