From 36482d7c803c0106fe12d3b19c7f852522676a6f Mon Sep 17 00:00:00 2001 From: Thierry Date: Mon, 11 May 2026 21:01:59 +0200 Subject: [PATCH 1/2] test(db): cover accounts CRUD + settings_db round-trips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two modules with zero coverage flagged by the coverage audit (2026-05-11). Both are foundational — accounts handles OAuth-linked identities + Product Truth (security-adjacent), settings_db backs the active_provider / active_model persistence the AI flow depends on. Tests use the same fresh in-memory pool pattern as history and groups. ## db::accounts (+10 tests) - upsert_inserts_new_account_and_returns_it - upsert_on_existing_account_updates_in_place - list_returns_all_accounts_ordered_by_created_at - get_by_id_returns_account_and_404s_on_missing - update_product_truth_round_trips_and_clears - update_branding_sets_both_colors_atomically (partial-clear case) - update_display_handle_overrides_and_clears (migration 016 regression guard for LinkedIn full-name vs brand handle) - update_visual_profile_round_trips_and_clears - delete_removes_account_and_nulls_post_references (manual cascade enforced in app code because SQLite forbids FK on ALTER ADD) - delete_is_idempotent_on_missing_account (race between publish flow and disconnect must not panic) ## db::settings_db (+5 tests) - get_returns_none_for_unknown_key - set_then_get_round_trips - set_on_existing_key_upserts_the_value (refresh-model flow) - migrations_seed_default_provider_and_model (consumer-POV smoke for the migrations chain 001/005/006/008/010) - set_accepts_empty_string_value (intentional — keeps the function pure key-value, no business-rule pollution) Total: Rust 221/221 (was 206), clippy + fmt clean. Co-Authored-By: Claude Opus 4.7 --- src-tauri/src/db/accounts.rs | 270 ++++++++++++++++++++++++++++++++ src-tauri/src/db/settings_db.rs | 89 +++++++++++ 2 files changed, 359 insertions(+) diff --git a/src-tauri/src/db/accounts.rs b/src-tauri/src/db/accounts.rs index d52063c..62553df 100644 --- a/src-tauri/src/db/accounts.rs +++ b/src-tauri/src/db/accounts.rs @@ -205,3 +205,273 @@ pub async fn delete(pool: &SqlitePool, provider: &str, user_id: &str) -> Result< .map_err(|e| e.to_string())?; Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use sqlx::sqlite::SqlitePoolOptions; + + /// Fresh in-memory pool with all migrations applied — same pattern as + /// `db::history::tests` and `db::groups::tests`. Each test gets its + /// own isolated DB because `:memory:` is connection-private and we + /// cap at `max_connections = 1`. + async fn fresh_pool() -> SqlitePool { + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect("sqlite::memory:") + .await + .expect("connect in-memory sqlite"); + sqlx::migrate!("src/db/migrations") + .run(&pool) + .await + .expect("migrations apply cleanly"); + pool + } + + async fn seed_account(pool: &SqlitePool, provider: &str, user_id: &str) -> Account { + upsert_and_get( + pool, + provider, + user_id, + &format!("user_{user_id}"), + Some(&format!("Display {user_id}")), + &format!("{provider}:{user_id}"), + Some("2099-12-31T23:59:59Z"), + ) + .await + .expect("seed upsert") + } + + #[tokio::test] + async fn upsert_inserts_new_account_and_returns_it() { + // First call with a brand-new (provider, user_id) tuple must + // INSERT and return the freshly-created row including the + // server-side id allocation. + let pool = fresh_pool().await; + let acc = seed_account(&pool, "instagram", "12345").await; + + assert_eq!(acc.provider, "instagram"); + assert_eq!(acc.user_id, "12345"); + assert_eq!(acc.username, "user_12345"); + assert_eq!(acc.display_name.as_deref(), Some("Display 12345")); + assert_eq!(acc.token_key, "instagram:12345"); + assert_eq!( + acc.token_expires_at.as_deref(), + Some("2099-12-31T23:59:59Z") + ); + // Nullable columns left blank on first insert. + assert!(acc.product_truth.is_none()); + assert!(acc.brand_color.is_none()); + assert!(acc.visual_profile.is_none()); + assert!(acc.display_handle.is_none()); + } + + #[tokio::test] + async fn upsert_on_existing_account_updates_in_place() { + // The ON CONFLICT(provider, user_id) DO UPDATE branch must + // refresh mutable fields without changing the id. This is the + // OAuth re-connection flow — same Instagram account, new token, + // possibly new display name. + let pool = fresh_pool().await; + let first = seed_account(&pool, "instagram", "12345").await; + + let updated = upsert_and_get( + &pool, + "instagram", + "12345", + "new_username", + Some("New Display"), + "instagram:12345_new_token_key", + Some("2100-01-01T00:00:00Z"), + ) + .await + .expect("second upsert"); + + // Same row (no id rotation), refreshed fields. + assert_eq!(updated.id, first.id); + assert_eq!(updated.username, "new_username"); + assert_eq!(updated.display_name.as_deref(), Some("New Display")); + assert_eq!(updated.token_key, "instagram:12345_new_token_key"); + assert_eq!( + updated.token_expires_at.as_deref(), + Some("2100-01-01T00:00:00Z") + ); + } + + #[tokio::test] + async fn list_returns_all_accounts_ordered_by_created_at() { + let pool = fresh_pool().await; + let a = seed_account(&pool, "instagram", "111").await; + let b = seed_account(&pool, "linkedin", "222").await; + let c = seed_account(&pool, "instagram", "333").await; + + let all = list(&pool).await.expect("list"); + assert_eq!(all.len(), 3); + // Insertion order = creation order = list order. + assert_eq!(all[0].id, a.id); + assert_eq!(all[1].id, b.id); + assert_eq!(all[2].id, c.id); + } + + #[tokio::test] + async fn get_by_id_returns_account_and_404s_on_missing() { + let pool = fresh_pool().await; + let acc = seed_account(&pool, "instagram", "12345").await; + + let fetched = get_by_id(&pool, acc.id).await.expect("get_by_id hit"); + assert_eq!(fetched.id, acc.id); + assert_eq!(fetched.user_id, "12345"); + + let err = get_by_id(&pool, 99999) + .await + .expect_err("missing id must return Err"); + assert!( + err.contains("99999"), + "error should surface the id queried, got: {err}" + ); + } + + #[tokio::test] + async fn update_product_truth_round_trips_and_clears() { + // SECURITY-ADJACENT: product_truth is content the AI prompts + // get conditioned on. A failed set or a silent revert would + // affect every future generation for that account. Round-trip + // verify both set + clear. + let pool = fresh_pool().await; + let acc = seed_account(&pool, "instagram", "12345").await; + + update_product_truth(&pool, acc.id, Some("Brand X: minimalist DevOps")) + .await + .expect("set"); + let after_set = get_by_id(&pool, acc.id).await.expect("get"); + assert_eq!( + after_set.product_truth.as_deref(), + Some("Brand X: minimalist DevOps") + ); + + update_product_truth(&pool, acc.id, None) + .await + .expect("clear"); + let after_clear = get_by_id(&pool, acc.id).await.expect("get"); + assert!(after_clear.product_truth.is_none()); + } + + #[tokio::test] + async fn update_branding_sets_both_colors_atomically() { + let pool = fresh_pool().await; + let acc = seed_account(&pool, "instagram", "12345").await; + + update_branding(&pool, acc.id, Some("#0d1117"), Some("#3ddc84")) + .await + .expect("set both"); + let after = get_by_id(&pool, acc.id).await.expect("get"); + assert_eq!(after.brand_color.as_deref(), Some("#0d1117")); + assert_eq!(after.accent_color.as_deref(), Some("#3ddc84")); + + // Clearing one but keeping the other — both Option<&str> are + // bound independently, so a single-clear is valid. + update_branding(&pool, acc.id, None, Some("#ff6b6b")) + .await + .expect("partial clear"); + let after_partial = get_by_id(&pool, acc.id).await.expect("get"); + assert!(after_partial.brand_color.is_none()); + assert_eq!(after_partial.accent_color.as_deref(), Some("#ff6b6b")); + } + + #[tokio::test] + async fn update_display_handle_overrides_and_clears() { + // Regression guard for migration 016: LinkedIn accounts get + // `username` populated with the owner's full personal name. The + // brand stamp on rendered visuals needs a separate handle. + // None clears, falling back to `username` at render time. + let pool = fresh_pool().await; + let acc = seed_account(&pool, "linkedin", "abc-xyz").await; + + update_display_handle(&pool, acc.id, Some("terminallearning")) + .await + .expect("set"); + let after_set = get_by_id(&pool, acc.id).await.expect("get"); + assert_eq!( + after_set.display_handle.as_deref(), + Some("terminallearning") + ); + + update_display_handle(&pool, acc.id, None) + .await + .expect("clear"); + let after_clear = get_by_id(&pool, acc.id).await.expect("get"); + assert!(after_clear.display_handle.is_none()); + } + + #[tokio::test] + async fn update_visual_profile_round_trips_and_clears() { + let pool = fresh_pool().await; + let acc = seed_account(&pool, "instagram", "12345").await; + + let profile_json = r##"{"colors":["#0d1117","#3ddc84"],"mood":["tech"]}"##; + update_visual_profile(&pool, acc.id, Some(profile_json)) + .await + .expect("set"); + let after = get_by_id(&pool, acc.id).await.expect("get"); + assert_eq!(after.visual_profile.as_deref(), Some(profile_json)); + + update_visual_profile(&pool, acc.id, None) + .await + .expect("clear"); + let after_clear = get_by_id(&pool, acc.id).await.expect("get"); + assert!(after_clear.visual_profile.is_none()); + } + + #[tokio::test] + async fn delete_removes_account_and_nulls_post_references() { + // SECURITY-ADJACENT: deleting an account must not leave + // dangling foreign keys in post_history. SQLite can't enforce + // ON DELETE SET NULL via ALTER TABLE ADD COLUMN (migration 013 + // hotfix), so the cascade lives in this function. Verify it + // actually fires. + let pool = fresh_pool().await; + let acc = seed_account(&pool, "instagram", "12345").await; + + // Insert a post pinned to this account. + sqlx::query( + "INSERT INTO post_history (network, caption, hashtags, status, created_at, \ + images, account_id) VALUES ('instagram', 'test', '[]', 'draft', \ + '2026-01-01T00:00:00Z', '[]', ?)", + ) + .bind(acc.id) + .execute(&pool) + .await + .expect("insert post"); + + delete(&pool, "instagram", "12345").await.expect("delete"); + + // Account gone. + let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM accounts") + .fetch_one(&pool) + .await + .expect("count"); + assert_eq!(count, 0); + + // Post still there but unpinned. + let post_account_id: Option = + sqlx::query_scalar("SELECT account_id FROM post_history WHERE caption = 'test'") + .fetch_one(&pool) + .await + .expect("get post"); + assert!( + post_account_id.is_none(), + "post_history.account_id must be NULL after the source account is deleted" + ); + } + + #[tokio::test] + async fn delete_is_idempotent_on_missing_account() { + // Calling delete for an account that doesn't exist must not + // error — the publish flow may race with a user clicking + // disconnect, and we'd rather no-op than panic. + let pool = fresh_pool().await; + delete(&pool, "instagram", "ghost-id") + .await + .expect("delete on missing must be Ok"); + } +} diff --git a/src-tauri/src/db/settings_db.rs b/src-tauri/src/db/settings_db.rs index 55b0e8b..d920204 100644 --- a/src-tauri/src/db/settings_db.rs +++ b/src-tauri/src/db/settings_db.rs @@ -21,3 +21,92 @@ pub async fn set(pool: &SqlitePool, key: &str, value: &str) -> Result<(), String .map(|_| ()) .map_err(|e| e.to_string()) } + +#[cfg(test)] +mod tests { + use super::*; + use sqlx::sqlite::SqlitePoolOptions; + + /// Fresh in-memory pool with all migrations applied. The `settings` + /// table is created and seeded with default rows (active_provider, + /// active_model) by the migration chain — see `migration_tests`. + async fn fresh_pool() -> SqlitePool { + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect("sqlite::memory:") + .await + .expect("connect in-memory sqlite"); + sqlx::migrate!("src/db/migrations") + .run(&pool) + .await + .expect("migrations apply cleanly"); + pool + } + + #[tokio::test] + async fn get_returns_none_for_unknown_key() { + let pool = fresh_pool().await; + let v = get(&pool, "completely-unknown-key").await; + assert!(v.is_none(), "unknown key must yield None, got: {v:?}"); + } + + #[tokio::test] + async fn set_then_get_round_trips() { + let pool = fresh_pool().await; + set(&pool, "my-key", "my-value").await.expect("set"); + let v = get(&pool, "my-key").await; + assert_eq!(v.as_deref(), Some("my-value")); + } + + #[tokio::test] + async fn set_on_existing_key_upserts_the_value() { + // The ON CONFLICT(key) DO UPDATE branch must replace the old + // value in place. Used at startup to refresh active_provider / + // active_model when the user picks a new model in Settings. + let pool = fresh_pool().await; + set(&pool, "active_model", "anthropic/claude-haiku-latest") + .await + .expect("first set"); + set(&pool, "active_model", "anthropic/claude-sonnet-4.6") + .await + .expect("second set"); + let v = get(&pool, "active_model").await; + assert_eq!( + v.as_deref(), + Some("anthropic/claude-sonnet-4.6"), + "second set must override the first" + ); + } + + #[tokio::test] + async fn migrations_seed_default_provider_and_model() { + // Smoke: the migration chain seeds default active_provider and + // active_model so the first AI call doesn't crash on a fresh + // install. Migrations 001 / 005 / 006 / 008 / 010 are jointly + // responsible. Re-asserted here from the consumer's POV so a + // future migration that breaks the seed gets caught. + let pool = fresh_pool().await; + let provider = get(&pool, "active_provider").await; + let model = get(&pool, "active_model").await; + assert!( + provider.as_deref().is_some_and(|s| !s.trim().is_empty()), + "active_provider must be seeded, got: {provider:?}" + ); + assert!( + model.as_deref().is_some_and(|s| !s.trim().is_empty()), + "active_model must be seeded, got: {model:?}" + ); + } + + #[tokio::test] + async fn set_accepts_empty_string_value() { + // The setter doesn't enforce non-empty values — that's a UI + // concern. Allowing empty strings keeps the function a pure + // key-value store and lets the UI clear settings by writing + // "" rather than needing a separate delete op. + let pool = fresh_pool().await; + set(&pool, "my-key", "").await.expect("set empty"); + let v = get(&pool, "my-key").await; + assert_eq!(v.as_deref(), Some("")); + } +} From 1839eda62f4b7eb257fb4036a273d34e0c208267 Mon Sep 17 00:00:00 2001 From: Thierry Date: Mon, 11 May 2026 21:05:15 +0200 Subject: [PATCH 2/2] test(sidecar): lock IPC serde contract (5 tests) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continuing the coverage audit started on this branch. `sidecar.rs` has 3 public functions but they're all process-spawn / async IO — true integration territory. What IS testable cleanly without a real Python install is the SERDE contract of the IPC types, and that's exactly the layer where silent cross-language drift hits the user the hardest. - sidecar_request_serialises_with_all_fields — every Python-side field present in the wire JSON. Renaming a Rust field without updating the sidecar would silently break generation. - sidecar_request_serialises_api_key_none_as_null — SECURITY: Ollama / local flows pass None; serde must emit `null`, not omit. Skipping would let the sidecar fall back to an env-var read on the Python side. - sidecar_data_deserialises_with_usage_field — cost tracker contract for the `usage` token counts. - sidecar_data_deserialises_without_usage_for_legacy_sidecars — `#[serde(default)] Option` keeps stale sidecars parseable instead of breaking every BYOK user on update lag. - token_usage_defaults_to_zero_when_field_missing — verify the `#[serde(default)]` on TokenUsage fields so a future refactor that drops the attribute is caught. Total: Rust 226/226 (was 221), clippy + fmt clean. Process-spawn coverage stays in the integration tier — out of scope for `cargo test --lib` and tracked for a future end-to-end suite once we have Python available in CI. Co-Authored-By: Claude Opus 4.7 --- src-tauri/src/sidecar.rs | 127 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/src-tauri/src/sidecar.rs b/src-tauri/src/sidecar.rs index 7197459..dd75381 100644 --- a/src-tauri/src/sidecar.rs +++ b/src-tauri/src/sidecar.rs @@ -385,3 +385,130 @@ pub async fn call_render_sidecar( .unwrap_or_else(|| "Unknown render sidecar error".to_string())) } } + +#[cfg(test)] +mod tests { + use super::*; + + // ── IPC contract tests ─────────────────────────────────────────────────── + // + // The sidecar protocol is text JSON over stdin/stdout. Tests in this + // module lock the SERDE shape of every public type so accidental + // additions / renames / type changes are caught before they break + // the Python sidecar at runtime. We do NOT spawn a real Python + // process here — those would be integration tests and require a + // working Python install in CI. The serialisation contract is + // exactly the surface that matters for cross-language stability. + + #[test] + fn sidecar_request_serialises_with_all_fields() { + // Lock the wire format: every Python-side field must be present + // in the JSON we send. Renaming a field on the Rust side without + // updating the sidecar would silently break generation calls. + let req = SidecarRequest { + action: "generate_content".to_string(), + provider: "openrouter".to_string(), + api_key: Some("sk-or-test".to_string()), + model: "anthropic/claude-sonnet-4.6".to_string(), + base_url: None, + brief: "Test brief".to_string(), + network: "instagram".to_string(), + system_prompt: "You are an IG expert.".to_string(), + }; + let json = serde_json::to_string(&req).expect("serialise"); + // Single contract assertion per field — duplicating the JSON + // string verbatim makes the test verbose and brittle against + // formatting changes. + for needle in [ + r#""action":"generate_content""#, + r#""provider":"openrouter""#, + r#""api_key":"sk-or-test""#, + r#""model":"anthropic/claude-sonnet-4.6""#, + r#""base_url":null"#, + r#""brief":"Test brief""#, + r#""network":"instagram""#, + r#""system_prompt":"You are an IG expert.""#, + ] { + assert!( + json.contains(needle), + "wire JSON missing {needle}, got: {json}" + ); + } + } + + #[test] + fn sidecar_request_serialises_api_key_none_as_null() { + // SECURITY: Ollama / local-only flows pass `None` for api_key. + // serde must emit `null`, not omit the field — the sidecar + // distinguishes "key not provided" from "key absent" via the + // explicit null. Skipping the field could silently look up an + // env-var fallback on the Python side, which is not what we + // want. + let req = SidecarRequest { + action: "generate_content".to_string(), + provider: "ollama".to_string(), + api_key: None, + model: "llama3".to_string(), + base_url: Some("http://localhost:11434/v1".to_string()), + brief: "x".repeat(10), + network: "instagram".to_string(), + system_prompt: "p".to_string(), + }; + let json = serde_json::to_string(&req).expect("serialise"); + assert!( + json.contains(r#""api_key":null"#), + "api_key=None must serialise as `null`, got: {json}" + ); + } + + #[test] + fn sidecar_data_deserialises_with_usage_field() { + // Cost tracker contract: the sidecar's generate_content + // response carries token counts under `usage`. The Rust side + // persists those into ai_usage. Schema drift here breaks the + // cost panel silently (no error, just zero usage logged). + let payload = r#"{ + "caption": "Hello world", + "hashtags": ["tag1","tag2"], + "usage": {"input_tokens": 123, "output_tokens": 456} + }"#; + let data: SidecarData = serde_json::from_str(payload).expect("deserialise"); + assert_eq!(data.caption, "Hello world"); + assert_eq!(data.hashtags, vec!["tag1".to_string(), "tag2".to_string()]); + let usage = data.usage.expect("usage present"); + assert_eq!(usage.input_tokens, 123); + assert_eq!(usage.output_tokens, 456); + } + + #[test] + fn sidecar_data_deserialises_without_usage_for_legacy_sidecars() { + // Older sidecar builds don't emit `usage`. The Rust side has + // `#[serde(default)] Option` — those calls must + // still parse cleanly. If serde started rejecting missing + // usage, every BYOK user on a stale sidecar would see all AI + // calls fail until they update — silent breakage class. + let payload = r#"{"caption":"hi","hashtags":[]}"#; + let data: SidecarData = serde_json::from_str(payload).expect("deserialise legacy"); + assert_eq!(data.caption, "hi"); + assert!(data.hashtags.is_empty()); + assert!(data.usage.is_none()); + } + + #[test] + fn token_usage_defaults_to_zero_when_field_missing() { + // serde `#[serde(default)]` on TokenUsage fields means a + // half-populated `usage` (e.g. provider returns only input + // tokens) parses with the missing field at 0 rather than + // erroring out. Verify the default explicitly so a future + // refactor that drops the attribute is caught. + let only_input = r#"{"input_tokens": 100}"#; + let usage: TokenUsage = serde_json::from_str(only_input).expect("partial usage"); + assert_eq!(usage.input_tokens, 100); + assert_eq!(usage.output_tokens, 0); + + let empty = r#"{}"#; + let usage: TokenUsage = serde_json::from_str(empty).expect("empty usage"); + assert_eq!(usage.input_tokens, 0); + assert_eq!(usage.output_tokens, 0); + } +}