From 6e959ed0c99b6efaec3bfd84ff17688c50b1cb53 Mon Sep 17 00:00:00 2001 From: jake Date: Tue, 17 Mar 2026 00:23:30 +0000 Subject: [PATCH] =?UTF-8?q?refactor(kon):=20clean=20up=20storage=20and=20c?= =?UTF-8?q?loud-providers=20=E2=80=94=20remove=20unused=20deps,=20add=20sa?= =?UTF-8?q?fety=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused chrono and uuid dependencies from kon-storage - Add schema versioning TODO to run_migrations() for pre-release safety - Add doc comment and usage example to log_error() (uncalled public API) - Add TODO to consolidate app_data_dir() with model_manager::dirs_path() - Add safety comments and #[allow(deprecated)] to keystore set_var usage - Add TODO for keyring crate migration in cloud-providers keystore - Apply cargo fmt Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/cloud-providers/src/keystore.rs | 27 +++++++++++++---- crates/storage/Cargo.toml | 6 ---- crates/storage/src/database.rs | 42 +++++++++++++++++--------- crates/storage/src/file_storage.rs | 10 +++--- crates/storage/src/lib.rs | 6 ++-- 5 files changed, 57 insertions(+), 34 deletions(-) diff --git a/crates/cloud-providers/src/keystore.rs b/crates/cloud-providers/src/keystore.rs index f171878..871565c 100644 --- a/crates/cloud-providers/src/keystore.rs +++ b/crates/cloud-providers/src/keystore.rs @@ -1,14 +1,29 @@ /// Store an API key in the OS keychain. -/// Stub implementation using environment variables until keyring crate is added. +/// +/// Stub implementation using environment variables until the `keyring` crate is +/// added. Keys are only held in-process and lost on exit. +/// +/// # Safety note +/// `std::env::set_var` is deprecated in Rust 2024 edition and is **not** +/// thread-safe — mutating the environment while other threads read it is +/// undefined behaviour. This is acceptable during single-threaded app init +/// but must not be called from async/multi-threaded contexts. +/// +/// TODO: Replace with the `keyring` crate (or platform-native credential +/// storage) so keys persist across sessions and are accessed safely. +#[allow(deprecated)] // set_var deprecated in Rust 2024 edition pub fn store_api_key(provider: &str, key: &str) { - std::env::set_var( - format!("KON_API_KEY_{}", provider.to_uppercase()), - key, - ); + // SAFETY: Only safe when called from a single-threaded context (e.g. app + // initialisation). See doc comment above. + std::env::set_var(format!("KON_API_KEY_{}", provider.to_uppercase()), key); } /// Retrieve an API key from the OS keychain. -/// Stub implementation using environment variables until keyring crate is added. +/// +/// Stub implementation using environment variables until the `keyring` crate is +/// added. Returns `None` if no key has been stored this session. +/// +/// TODO: Replace with the `keyring` crate alongside `store_api_key`. pub fn retrieve_api_key(provider: &str) -> Option { std::env::var(format!("KON_API_KEY_{}", provider.to_uppercase())).ok() } diff --git a/crates/storage/Cargo.toml b/crates/storage/Cargo.toml index 500ff79..a386ca8 100644 --- a/crates/storage/Cargo.toml +++ b/crates/storage/Cargo.toml @@ -12,9 +12,3 @@ sqlx = { version = "0.8", features = ["sqlite", "runtime-tokio"] } # Async runtime tokio = { version = "1", features = ["rt", "sync", "macros"] } - -# UUIDs for record IDs -uuid = { version = "1", features = ["v4"] } - -# Timestamps -chrono = { version = "0.4", features = ["serde"] } diff --git a/crates/storage/src/database.rs b/crates/storage/src/database.rs index 027fbce..ef23166 100644 --- a/crates/storage/src/database.rs +++ b/crates/storage/src/database.rs @@ -27,6 +27,11 @@ pub async fn init(db_path: &Path) -> Result { } /// Run schema migrations. +/// +/// TODO(pre-release): Implement schema versioning for safe upgrades. +/// Currently uses `CREATE TABLE IF NOT EXISTS` which silently ignores column +/// additions/removals. Before shipping, add a `schema_version` table and +/// sequential migration steps so existing user databases upgrade cleanly. async fn run_migrations(pool: &SqlitePool) -> Result<()> { sqlx::query( "CREATE TABLE IF NOT EXISTS transcripts ( @@ -186,10 +191,7 @@ pub async fn insert_transcript( Ok(()) } -pub async fn get_transcript( - pool: &SqlitePool, - id: &str, -) -> Result> { +pub async fn get_transcript(pool: &SqlitePool, id: &str) -> Result> { let row = sqlx::query( "SELECT id, text, source, title, audio_path, duration, engine, model_id, inference_ms, sample_rate, audio_channels, format_mode, remove_fillers, british_english, anti_hallucination, created_at FROM transcripts WHERE id = ?", ) @@ -231,16 +233,14 @@ pub async fn insert_task( bucket: &str, source_transcript_id: Option<&str>, ) -> Result<()> { - sqlx::query( - "INSERT INTO tasks (id, text, bucket, source_transcript_id) VALUES (?, ?, ?, ?)", - ) - .bind(id) - .bind(text) - .bind(bucket) - .bind(source_transcript_id) - .execute(pool) - .await - .map_err(|e| KonError::StorageError(format!("Insert task failed: {e}")))?; + sqlx::query("INSERT INTO tasks (id, text, bucket, source_transcript_id) VALUES (?, ?, ?, ?)") + .bind(id) + .bind(text) + .bind(bucket) + .bind(source_transcript_id) + .execute(pool) + .await + .map_err(|e| KonError::StorageError(format!("Insert task failed: {e}")))?; Ok(()) } @@ -364,7 +364,19 @@ fn transcript_row_from(r: &sqlx::sqlite::SqliteRow) -> TranscriptRow { // --- Error Logging --- -/// Log a structured error to the error_log table. +/// Log a structured error to the `error_log` table. +/// +/// Available for Tauri command handlers to persist errors for diagnostics. +/// Each entry records context (which subsystem), an optional error code, +/// the human-readable message, and optional JSON metadata. +/// +/// # Example +/// ```ignore +/// log_error(&pool, "transcription", Some("WHISPER_INIT"), "Model load failed", None).await?; +/// ``` +/// +/// TODO: Wire this into Tauri command error paths so runtime failures are +/// persisted for user-facing error history and crash diagnostics. pub async fn log_error( pool: &SqlitePool, context: &str, diff --git a/crates/storage/src/file_storage.rs b/crates/storage/src/file_storage.rs index 6d320f8..1b1000e 100644 --- a/crates/storage/src/file_storage.rs +++ b/crates/storage/src/file_storage.rs @@ -3,14 +3,16 @@ use std::path::PathBuf; /// Resolve the app data directory. /// Windows: %LOCALAPPDATA%/kon /// Unix: ~/.kon +/// +/// TODO: Consolidate with `crates/transcription/src/model_manager.rs::dirs_path()` +/// into a shared helper in `crates/core/` to avoid duplicating platform-specific +/// path logic across crates. pub fn app_data_dir() -> PathBuf { if cfg!(target_os = "windows") { - let local_app_data = - std::env::var("LOCALAPPDATA").unwrap_or_else(|_| ".".to_string()); + let local_app_data = std::env::var("LOCALAPPDATA").unwrap_or_else(|_| ".".to_string()); PathBuf::from(local_app_data).join("kon") } else { - let home = - std::env::var("HOME").unwrap_or_else(|_| "/tmp".to_string()); + let home = std::env::var("HOME").unwrap_or_else(|_| "/tmp".to_string()); PathBuf::from(home).join(".kon") } } diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 69c42bd..5703f39 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -2,8 +2,8 @@ pub mod database; pub mod file_storage; pub use database::{ - complete_task, delete_task, delete_transcript, get_setting, get_transcript, - init, insert_task, insert_transcript, list_tasks, list_transcripts, - log_error, set_setting, InsertTranscriptParams, TaskRow, TranscriptRow, + complete_task, delete_task, delete_transcript, get_setting, get_transcript, init, insert_task, + insert_transcript, list_tasks, list_transcripts, log_error, set_setting, + InsertTranscriptParams, TaskRow, TranscriptRow, }; pub use file_storage::{app_data_dir, database_path, recordings_dir};