refactor(sync): atomic playlist write + outbox enqueue in one tx

[InstaZDLL]

Closes #193. Closes the drift window #192 documented where the playlist write committed in one SQLite transaction and the matching `sync_pending_op` row landed in a separate await — a crash or DB hiccup between the two could leave the local state ahead of the queue and the server would never hear about the user's edit.

Summary

• waveflow-core — every write method on `SqlitePlaylistRepository` now has a sibling free function in the same module taking `&mut SqliteConnection` (`insert_custom_conn`, `update_conn`, `delete_conn`, `append_track_conn`, `append_tracks_conn`, `remove_track_conn`, `reorder_track_conn`). The trait impls become thin wrappers that acquire `pool.begin()` and delegate, so every existing caller keeps working unchanged.
• `reorder_track_conn` returns `Option` (effective position post-clamp) instead of `bool`, so the caller can stamp the sync payload with the row's actual new state — closes the divergence the feat(sync): wire playlist CRUD enqueue hooks (Phase 1.f.desktop.2b) #192 readback patch worked around with a post-write SELECT. The trait method maps the Option to a bool for back-compat.
• waveflow desktop — `lamport::next_conn`, `queue::enqueue_conn`, `mode::read_conn`, `server_client::read_token_conn` are sibling variants that take `&mut SqliteConnection` so they compose in a caller's open tx.
• `sync::hooks::enqueue_op_in_tx(conn, draft)` — atomic outbox path. Returns `AppResult` so the caller can `?` the failure and let the surrounding tx roll back the entity write too. The fire-and-forget `enqueue_op` variant is gone — playlist commands all use the atomic path now, and a future non-playlist hook would too.
• `commands/playlist.rs` — the 8 mutation commands (create / update / delete / add_track / add_tracks / add_source / remove_track / reorder) all wrap their write + enqueue in a single `pool.begin()` → `..._conn(&mut tx)` → ... → `enqueue_op_in_tx(&mut tx)` → `tx.commit()` block. Filesystem-level side effects (`playlist_cover::maybe_regen_auto_cover`) run OUTSIDE the tx as before.

Module docstring

`crate::sync::hooks` rewritten — atomicity is no longer a documented known-gap, it's the guaranteed shape. Skip path (no JWT or `SyncMode::Local`) still returns `Ok(false)` without writing.

Test plan

• `cargo test -p waveflow --lib sync::` — 22/22 green locally
• `cargo test -p waveflow-core --lib` — 35/35 green locally
• `cargo clippy --workspace --all-targets -- -D warnings`
• `cargo fmt --check`
• Manual smoke (requires feat(server-auth): oauth-loopback browser handshake (Phase 1.f.desktop.1b) #190 + waveflow-web feat(player-bar): redesign right cluster (Spotify-style) + overflow defaults #18 deployed):
  • Sign in to a profile, mode = Hybrid
  • Create a playlist → `sync_get_queue_state` shows `pending_count: 1` AND `SELECT COUNT(*) FROM playlist` shows the row
  • Force an enqueue failure (e.g. by manually corrupting `auth_credential.token_encrypted`) → next CRUD operation aborts, NEITHER the playlist write NOR the queue row land
  • Reorder a track to position 999 in a 10-track playlist → `sync_get_queue_state` payload shows `position: 9` (the clamped value), not 999

Out of scope

• library + edit hooks. They were never actually wired in feat(sync): wire playlist CRUD enqueue hooks (Phase 1.f.desktop.2b) #192 (deferred for design reasons). When they get hooked, they'll use `enqueue_op_in_tx` like playlist.

Summary by CodeRabbit

Notes de version

• Refactor
  • Opérations sur les playlists (création, édition, suppression, réordonnancement, ajout/retrait de pistes) désormais exécutées de façon atomique pour plus de fiabilité.
  • Enregistrement des opérations de synchronisation couplé à l’écriture locale pour garantir cohérence entre l’état écrit et le payload envoyé.
  • Régénération automatique de la couverture exécutée après la persistance pour éviter d’impacter la transaction.
• Correction
  • Échec contrôlé du réordonnancement si l’entrée n’existe plus, évitant des mismatches de synchronisation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Ce changement rend atomiques les mutations playlist et l'enqueue des opérations de sync : le repository expose helpers *_conn, la pile de sync expose *_conn et enqueue_op_in_tx, puis les commandes playlist utilisent une transaction unique pour mutation + outbox, commit puis travail post-commit.

Changes

Atomicité de la mutation playlist et sync outbox

Layer / File(s)	Summary
Playlist repository transaction-aware layer src-tauri/crates/core/src/repository/sqlite/playlist.rs	Les helpers insert_custom_conn, update_conn, delete_conn, append_track_conn, append_tracks_conn, remove_track_conn, reorder_track_conn et begin_tx acceptent une &mut SqliteConnection. Les méthodes existantes délèguent ou utilisent ces helpers dans des transactions.
Sync infrastructure transaction-aware APIs src-tauri/crates/app/src/server_client.rs, src-tauri/crates/app/src/sync/lamport.rs, src-tauri/crates/app/src/sync/mode.rs, src-tauri/crates/app/src/sync/queue.rs, src-tauri/crates/app/src/sync/hooks.rs	Ajout de read_token_conn, next_conn, read_conn, enqueue_conn et de enqueue_op_in_tx qui lisent/incrémentent/enfilent sur la connexion fournie, avec propagation d'erreurs et logique de skip (No JWT / SyncMode::Local).
Playlist commands transactional integration src-tauri/crates/app/src/commands/playlist.rs	create_playlist, update_playlist, delete_playlist, add_track_to_playlist, add_tracks_to_playlist, remove_track_from_playlist, reorder_playlist_track, add_source_to_playlist ouvrent une tx, appellent *_conn, enfilent via enqueue_op_in_tx, commitent, puis font les tâches post-commit (ex. régénération de couverture) hors tx. Les positions effectives renvoyées par reorder_track_conn sont utilisées pour composer le payload de sync.

---

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

---

Possibly related PRs

• InstaZDLL/WaveFlow#191: Base sync (Lamport + queue) sur laquelle les variantes *_conn s'appuient.
• InstaZDLL/WaveFlow#192: PR antérieure traitant l'atomicité outbox/playlist ; ce PR complète en remplaçant enqueue_op par enqueue_op_in_tx.

---

Suggested labels

type: fix

---

Poem

🎶 Transactions enlacées, playlist en paix,
Un seul commit scelle ce qui était séparé.
L'outbox et la piste désormais enlignées,
Plus d'ombre entre stockage et vérité.
🚀 Bravo, cohérence livrée !

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	Le titre suit Conventional Commits avec scope valide (sync) et décrit l'objectif principal : atomicité de l'écriture playlist + enqueue en une seule transaction.
Linked Issues check	✅ Passed	Les changements couvrent intégralement l'objectif #193 : atomic write + enqueue en une seule transaction SQLite, avec enqueue_op_in_tx(conn, draft) retournant AppResult, removal du fire-and-forget enqueue_op, et toutes les 8 commandes playlist utilisant le chemin atomique.
Out of Scope Changes check	✅ Passed	Les changements demeurent strictement limités aux playlists et à l'infrastructure transactionnelle (lamport, queue, mode, server_client). Aucune mutation library/edit hooks qui étaient délibérément hors scope et déferred pour des raisons de design.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check	✅ Passed	La description suit la structure requise avec un titre en Conventional Commits, un résumé détaillé des modifications, un plan de test explicite, et l'indication des problèmes fermés.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/1-f-desktop-atomicity-followup

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src-tauri/crates/app/src/commands/playlist.rs (1)

193-221: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Place le contrôle d’existence dans la même transaction.

repo.exists() est fait avant pool.begin(). Si une autre écriture supprime la playlist entre les deux, update_conn devient un no-op silencieux et la boucle enfile quand même des ops "set". On recrée donc une divergence local/outbox malgré ce chemin atomique. Il faut faire remonter rows_affected == 0 depuis update_conn, ou refaire la vérification dans tx avant d’enqueue.

🤖 Prompt for 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.

In `@src-tauri/crates/app/src/commands/playlist.rs` around lines 193 - 221, The
existence check must happen inside the same transaction to avoid enqueuing ops
for a deleted playlist; modify update_conn (or its caller) to surface
rows_affected and treat 0 as an error, or perform an existence SELECT using the
same transaction before calling enqueue_op_in_tx. Concretely: change update_conn
to return the number of rows updated (or a Result<Option<()>>), call it within
the tx begun by pool.begin(), check the returned rows_affected == 0 and return
an error (aborting before the enqueue loop) so enqueue_op_in_tx and tx.commit()
only run for actually-updated playlists.

🤖 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-tauri/crates/app/src/commands/playlist.rs`:
- Around line 391-404: The code enqueues a "delete" pending op even when
remove_track_conn returned false (nothing removed locally); change the flow in
the block around remove_track_conn, enqueue_op_in_tx and tx.commit so you
capture remove_track_conn(...)'s boolean result and only call
crate::sync::hooks::enqueue_op_in_tx with the PendingOpDraft (and then commit
the transaction) when remove_track_conn returned true; if it returns false, roll
back or drop the transaction without enqueuing the delete op to avoid removing
server-side tracks that weren't removed locally.

---

Outside diff comments:
In `@src-tauri/crates/app/src/commands/playlist.rs`:
- Around line 193-221: The existence check must happen inside the same
transaction to avoid enqueuing ops for a deleted playlist; modify update_conn
(or its caller) to surface rows_affected and treat 0 as an error, or perform an
existence SELECT using the same transaction before calling enqueue_op_in_tx.
Concretely: change update_conn to return the number of rows updated (or a
Result<Option<()>>), call it within the tx begun by pool.begin(), check the
returned rows_affected == 0 and return an error (aborting before the enqueue
loop) so enqueue_op_in_tx and tx.commit() only run for actually-updated
playlists.

🪄 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: c59a2bc0-c9e5-44c9-a093-f4c13a80a0d5

📥 Commits

Reviewing files that changed from the base of the PR and between 1e274fb and ef3ecd2.

📒 Files selected for processing (7)

• src-tauri/crates/app/src/commands/playlist.rs
• src-tauri/crates/app/src/server_client.rs
• src-tauri/crates/app/src/sync/hooks.rs
• src-tauri/crates/app/src/sync/lamport.rs
• src-tauri/crates/app/src/sync/mode.rs
• src-tauri/crates/app/src/sync/queue.rs
• src-tauri/crates/core/src/repository/sqlite/playlist.rs