feat: Postgres pool + first migration + /ready (Phase 1.b.2b)#3
Merged
Conversation
Wire the server's storage layer end-to-end:
- `waveflow-core` consumed via git dep pinned at the merge SHA of
`feat: postgres feature + PostgresProfileRepository` on the main
repo (rev 3ab43f0). `default-features = false` + `features =
["postgres"]` pulls the Postgres impl without the sqlite drag.
- `Config` gains `DATABASE_URL` (required, no fallback) and
`WAVEFLOW_DB_MAX_CONNECTIONS` (default 20). Both fail fast at boot
on invalid values.
- `db::connect` opens a sqlx `PgPool` with sane defaults (5 s acquire
timeout, 10 min idle timeout, capped by config). `db::run_migrations`
applies the compile-time-embedded `MIGRATOR` — the same immutable-
checksum discipline the desktop uses (`CLAUDE.md`).
- First migration `20260529000000_profile.sql` mirrors the SQLite
`profile` table shape (BIGSERIAL id, epoch-ms timestamps, indexed
`last_used_at DESC`) so `PostgresProfileRepository` round-trips
identical-shaped rows.
- `lib::app` now takes an `AppState { db: PgPool }` threaded into the
routers via `with_state`. `main.rs` connects + migrates before
binding the listener — a schema failure aborts startup with a
non-zero exit so the orchestrator backs off.
- New `GET /ready` handler probes the pool via `SELECT 1`. 200 when
healthy, 503 when degraded — the unhappy path is a warn-level log,
not a panic, so a transient Postgres blip self-heals.
CI: ubuntu-latest gets a Postgres 17 service container + DATABASE_URL
env so the full suite (incl. `#[sqlx::test]` integration tests) runs
on every PR. windows-latest stays as a compile guard — service
containers are Linux-only and waveflow-server is a Linux/macOS
daemon by design.
Tests: `health` + `ready` integration suites use `#[sqlx::test(migrator
= "waveflow_server::db::MIGRATOR")]` to get a fresh per-test database
provisioned by sqlx (no manual fixture cleanup). A schema canary
verifies the `profile` table lands so 1.b.4's CRUD work doesn't trip
on a silent migration drop.
Signed-off-by: InstaZDLL <github.105mh@8shield.net>
📝 WalkthroughWalkthroughCe PR intègre Postgres via sqlx : variables d'environnement et Config, module DB avec migrateur embarqué et ping, endpoint /ready, AppState partagé, harness et tests d'intégration, plus adaptations Cargo/CI/README. ChangesIntégration de base de données Postgres
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/ci.yml:
- Around line 26-27: Le service Postgres utilise le tag flottant "postgres:17";
remplacez-le par l'image épinglée par digest (sha256) pour garantir des builds
reproductibles, par exemple remplacer "postgres:17" par "postgres@sha256:..." et
ajoutez un commentaire à côté "# 17.x" pour indiquer la version majeure suivie
(conserver ainsi la trace de la version tout en fixant le digest).
In `@Cargo.toml`:
- Line 27: The README MSRV and Cargo metadata are out of sync with the sqlx
dependency: sqlx = "0.9" requires Rust 1.94+, so update README.md to state MSRV
1.94+ (replace the 1.84+ mention) and add rust-version = "1.94" under the
[package] section of Cargo.toml to provide a clear compiler error for older
toolchains; also remove or clarify any references to "0.9.0-alpha.1" in the
README if they imply the crate is still alpha since version "0.9" targets the
stable 0.9.0 release.
In `@src/api/ready.rs`:
- Around line 42-55: The readiness handler currently returns the raw DB error
via db: err.to_string() in the Err(err) branch; instead keep the detailed error
only in the tracing::warn! log and return a generic value to the client (e.g.
db: "unavailable" or db: None). Edit the Err(err) arm in the readiness handler
(the Err(err) branch that constructs ReadyResponse) to replace err.to_string()
with a non-sensitive sentinel or None, and if ReadyResponse.db is a String
change it to Option<String> (or adjust its type) so the public /ready response
never exposes internal DB error details while preserving the existing
tracing::warn!(error = %err, ...) logging.
In `@src/db.rs`:
- Around line 3-8: The doc comments in src/db.rs reference a non-existent symbol
`MIGRATIONS_DIR`, causing broken intra-doc links; update the doc comments to
reference the actual exported symbol `MIGRATOR` (i.e., change occurrences of
[`MIGRATIONS_DIR`] to [`MIGRATOR`]) or alternatively spell out the literal path
(`./migrations`) in the text so rustdoc links resolve and warnings go away,
ensuring references match the public `pub static MIGRATOR: Migrator =
sqlx::migrate!("./migrations");`.
🪄 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: d3e537d2-b221-4c9d-9569-8f2ebd90e46d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock,!*.lock
📒 Files selected for processing (14)
.env.example.github/workflows/ci.ymlCargo.tomlREADME.mdmigrations/20260529000000_profile.sqlsrc/api/mod.rssrc/api/ready.rssrc/config.rssrc/db.rssrc/lib.rssrc/main.rstests/health.rstests/ready.rstests/support.rs
Three valid issues, one skipped with rationale. 1. APPLIED — Declare MSRV explicitly. sqlx 0.9 documents `rust-version = "1.94.0"`; add the same to our package metadata so an older toolchain fails at dependency resolution with one clear line instead of a wall of rustc errors. README bumped from "1.84+" to "1.94+ (inherited from sqlx 0.9)". 2. APPLIED — `/ready` no longer leaks sqlx error detail. A sqlx error message can echo the connection URL host / user, and the typical /ready consumer is an unauthenticated load balancer. Keep the detail in `tracing::warn!` only; the body returns a fixed "unavailable" sentinel. `ReadyResponse.db` is now `&'static str` for both arms — the response shape becomes a tight enum on the wire. 3. APPLIED — `db.rs` module doc referenced `[`MIGRATIONS_DIR`]`, a symbol that doesn't exist. Rewrote to point at the actual public `MIGRATOR` and to spell out `./migrations` so rustdoc resolves cleanly. 4. SKIPPED — Pinning `postgres:17` to a sha256 digest for the CI service container. For ephemeral test infra (not a production deploy), the floating major tag picks up security patches automatically and the test surface — `SELECT 1` against a healthy pool — doesn't change between Postgres minor versions. Pinning would mean manual digest bumps with no security upside, just for the test runner. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
33-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCorriger le contrat documenté de
/readyen 503.La doc indique encore
db: <error>, alors que ce PR retire l’exposition des détails SQL côté HTTP. Mets à jour la réponse décrite en Line 33 pour refléter la nouvelle valeur stable (&'static str) et éviter une fausse API contract côté clients/tests.🤖 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 `@README.md` at line 33, Met à jour le contrat documenté pour l'endpoint GET /ready: remplacer la description actuelle qui expose l'erreur SQL par une valeur constante &'static str; documente la réponse d'échec comme 503 {status: "not_ready", db: "error"} (et conserve 200 {status: "ready", db: "ok"} pour le succès) afin d'aligner README.md avec la suppression de l'exposition des détails SQL.src/api/ready.rs (1)
9-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDoc obsolète : le corps ne contient plus
<error>.La doc annonce
503 {status: "not_ready", db: <error>}, mais le handler renvoie désormais le sentinel fixe"unavailable"(le détail reste danstracing::warn!). Mets la doc à jour pour refléter le contrat réel et éviter d'induire en erreur les consommateurs de/ready.📝 Correctif proposé
-//! - `503 {status: "not_ready", db: <error>}` otherwise. A 503 here +//! - `503 {status: "not_ready", db: "unavailable"}` otherwise. A 503 here //! tells a Kubernetes / systemd-style probe to keep waiting / stop //! routing traffic; it doesn't crash the process so a transient //! Postgres blip self-heals.🤖 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/api/ready.rs` around lines 9 - 15, Update the doc comment in src/api/ready.rs to reflect the current response contract: change the 503 description from `db: <error>` to the fixed sentinel `db: "unavailable"` (keeping the 200 case as `db: "ok"`), and note that detailed error information is emitted to tracing::warn! rather than returned in the body; locate the ready handler comment at the top of the file and replace the outdated line so consumers see the actual JSON shape returned by the ready endpoint.
🤖 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.
Outside diff comments:
In `@README.md`:
- Line 33: Met à jour le contrat documenté pour l'endpoint GET /ready: remplacer
la description actuelle qui expose l'erreur SQL par une valeur constante
&'static str; documente la réponse d'échec comme 503 {status: "not_ready", db:
"error"} (et conserve 200 {status: "ready", db: "ok"} pour le succès) afin
d'aligner README.md avec la suppression de l'exposition des détails SQL.
In `@src/api/ready.rs`:
- Around line 9-15: Update the doc comment in src/api/ready.rs to reflect the
current response contract: change the 503 description from `db: <error>` to the
fixed sentinel `db: "unavailable"` (keeping the 200 case as `db: "ok"`), and
note that detailed error information is emitted to tracing::warn! rather than
returned in the body; locate the ready handler comment at the top of the file
and replace the outdated line so consumers see the actual JSON shape returned by
the ready endpoint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4fdc78e0-cb95-4304-ae80-5f1e92c31d92
📒 Files selected for processing (4)
Cargo.tomlREADME.mdsrc/api/ready.rssrc/db.rs
Follow-up on the earlier `/ready` body change (CR #3 round 2). The handler now returns the fixed `"unavailable"` sentinel for the 503 case, but the README's published contract and the `api/ready.rs` module doc still described the old `db: <error>` body — risk of clients keying off a string that never appears. Both docs now match the wire shape and note that sqlx error detail stays in `tracing::warn!` only (rationale: unauthenticated probes like load balancers shouldn't see connection URLs / credentials). Signed-off-by: InstaZDLL <github.105mh@8shield.net>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/ready.rs (1)
65-73: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDéplace cette requête SQL hors du handler API.
probe_dbest défini danssrc/api/ready.rsavec une requête SQL brute. Pour rester aligné avec les frontières du projet, mets ce probe dans la couche DB/repository (p. ex.src/db.rsou via un traitwaveflow-core::repository) et garde le handler limité à l’orchestration HTTP.As per coding guidelines « Privilégie les traits de
waveflow-core::repositoryplutôt que des requêtes SQL en dur dans les handlers. »🤖 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/api/ready.rs` around lines 65 - 73, La fonction probe_db dans src/api/ready.rs contient une requête SQL brute (sqlx::query_scalar::<_, i32>("SELECT 1").fetch_one(pool).await) et doit être déplacée vers la couche DB/repository; extractez cette logic dans src/db.rs (ou implémentez un trait dans waveflow-core::repository, p.ex. Repository::probe or HealthRepository::ping) que prend &PgPool et retourne Result<(), sqlx::Error>, puis appelez cette méthode depuis le handler readiness pour que probe_db (ou son équivalent) réside dans la couche de persistance tandis que le handler HTTP reste uniquement orchestrateur.
🤖 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.
Outside diff comments:
In `@src/api/ready.rs`:
- Around line 65-73: La fonction probe_db dans src/api/ready.rs contient une
requête SQL brute (sqlx::query_scalar::<_, i32>("SELECT
1").fetch_one(pool).await) et doit être déplacée vers la couche DB/repository;
extractez cette logic dans src/db.rs (ou implémentez un trait dans
waveflow-core::repository, p.ex. Repository::probe or HealthRepository::ping)
que prend &PgPool et retourne Result<(), sqlx::Error>, puis appelez cette
méthode depuis le handler readiness pour que probe_db (ou son équivalent) réside
dans la couche de persistance tandis que le handler HTTP reste uniquement
orchestrateur.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f8f4dc6e-50b6-495f-8111-6fbd297b0c9d
📒 Files selected for processing (2)
README.mdsrc/api/ready.rs
The handler had a raw `SELECT 1` query inline, which violates the project's no-SQL-in-handlers rule (the same boundary the desktop side enforces between Tauri commands and waveflow-core). A repository trait in waveflow-core would be over-engineering for a schema-agnostic ping; instead expose it as `db::ping(&PgPool)` so future infrastructure probes (e.g. Redis, plugin host) can land alongside. `/ready` handler now reads as pure HTTP orchestration: call `db::ping`, map the Result to (status, body). Wire shape unchanged (`db: "ok"` / `db: "unavailable"`), tests are still green. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
This was referenced May 30, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wire the server's storage layer end-to-end. The pool is online, migrations apply at boot, and the orchestrator can probe both liveness (`/health`) and readiness (`/ready`).
What's wired
Dependency: `waveflow-core` (RFC-001 §6.5)
Pinned by rev for reproducibility — bump in tree when picking up a new core release. Cargo's git resolver finds the workspace member `waveflow-core` inside the desktop repo's `src-tauri/crates/core/` without needing a workspace manifest at the repo root.
Configuration
Pool + migrations (`src/db.rs`)
First migration (`20260529000000_profile.sql`)
Mirrors the SQLite `profile` table shape (BIGSERIAL id, epoch-ms timestamps, indexed `last_used_at DESC`) so `PostgresProfileRepository` round-trips identical-shaped rows. Schema parity with the desktop's `src-tauri/migrations/app/` is what makes the repository trait work cross-backend.
`/ready` (`src/api/ready.rs`)
CI: Postgres service container
Test harness
`#[sqlx::test(migrator = "waveflow_server::db::MIGRATOR")]` on every integration test:
No manual fixture cleanup. `tests/support.rs` spawns the real `app(config, state)` on a kernel-assigned port.
Three integration tests: `/health` smoke + request-id propagation, `/ready` happy path, schema canary that asserts the `profile` table lands after migration.
Out of scope
Test plan
Signed-off-by: InstaZDLL github.105mh@8shield.net
Summary by CodeRabbit
Notes de version
Nouvelles fonctionnalités
Documentation
Tests
Chores