refactor: consolidate gateway secrets into agentflare.db (closes #138)

src/cli/gateway.rs

@@ -34,11 +34,10 @@ impl GatewayArgs {
 }
 
 fn run_secret(action: GatewaySecretAction) {
-    let db_path = crate::paths::home().join(".agentflare").join("gateway.db");
-    let conn = match crate::gateway_secrets::open_db(&db_path) {
+    let conn = match crate::db::open() {
         Ok(c) => c,
         Err(e) => {
-            eprintln!("failed to open gateway secrets db: {e}");
+            eprintln!("failed to open agentflare.db: {e}");
             std::process::exit(1);
         }
     };

src/db.rs

@@ -3,14 +3,18 @@
 //! (see #138 — gateway secrets fold in later). The rebuildable caches under
 //! `~/.local/share/agentflare/` (skills index, gateway tool-index) stay
 //! separate: they belong in the data dir, not next to source-of-truth state.
-use rusqlite::Connection;
+use rusqlite::{params, Connection};
 use std::path::PathBuf;
 use std::time::Duration;
 
 pub fn agentflare_db_path() -> PathBuf {
     crate::paths::home().join(".agentflare").join("agentflare.db")
 }
 
+fn old_gateway_db_path() -> PathBuf {
+    crate::paths::home().join(".agentflare").join("gateway.db")
+}
+
 /// Opens (creating if absent) `agentflare.db` and applies every table's
 /// migration. Each subsystem owns its own `CREATE TABLE IF NOT EXISTS` so
 /// this stays a thin dispatcher.
@@ -27,9 +31,59 @@ pub fn open() -> rusqlite::Result<Connection> {
     tune(&conn)?;
     crate::claims::migrate(&conn)?;
     crate::review::migrate(&conn)?;
+    crate::gateway_secrets::migrate(&conn)?;
+    // One-time migration: copy secrets from old gateway.db
+    // (pre-#138 separate file) into agentflare.db.
+    migrate_old_gateway_db(&conn)?;
     Ok(conn)
 }
 
+fn migrate_old_gateway_db(conn: &Connection) -> rusqlite::Result<()> {
+    import_legacy_secrets(conn, &old_gateway_db_path())
+}
+
+/// Copy secrets from a legacy `gateway.db` (pre-#138 separate file) into the
+/// shared db, then rename the legacy file so it's imported exactly once.
+///
+/// Reads from the legacy db are best-effort: a missing file, an unopenable
+/// db, or an incompatible/malformed `gateway_secrets` schema all skip the
+/// import rather than failing `open()` — otherwise one bad legacy file would
+/// brick unrelated claims access too. Only writes into our own (healthy)
+/// shared db are fatal.
+///
+/// Renaming to `gateway.db.migrated` on success is the migration-complete
+/// marker: without it every `open()` re-imports via `INSERT OR IGNORE`, so a
+/// secret the user deleted would resurrect on the next run.
+fn import_legacy_secrets(conn: &Connection, old_path: &std::path::Path) -> rusqlite::Result<()> {
+    if !old_path.exists() {
+        return Ok(());
+    }
+    let Ok(old) = Connection::open(old_path) else {
+        return Ok(());
+    };
+    let Ok(mut stmt) = old.prepare("SELECT name, ciphertext FROM gateway_secrets") else {
+        return Ok(()); // incompatible legacy schema — skip, don't brick open()
+    };
+    let Ok(rows) = stmt.query_map([], |r| {
+        Ok((r.get::<_, String>(0)?, r.get::<_, Vec<u8>>(1)?))
+    }) else {
+        return Ok(());
+    };
+    // `flatten` drops rows that fail to decode (e.g. malformed ciphertext)
+    // rather than propagating — one bad row must not abort the migration.
+    for (name, ciphertext) in rows.flatten() {
+        conn.execute(
+            "INSERT OR IGNORE INTO gateway_secrets (name, ciphertext) VALUES (?1, ?2)",
+            params![name, ciphertext],
+        )?;
+    }
+    // Best-effort: if the rename fails we just re-import idempotently next time.
+    drop(stmt);
+    drop(old);
+    let _ = std::fs::rename(old_path, old_path.with_extension("db.migrated"));
+    Ok(())
+}
+
 /// Best-effort owner-only permissions (no-op off Unix).
 #[cfg(unix)]
 fn restrict(path: &std::path::Path, mode: u32) {
@@ -53,3 +107,78 @@ fn tune(conn: &Connection) -> rusqlite::Result<()> {
     let _: String = conn.query_row("PRAGMA journal_mode=WAL", [], |r| r.get(0))?;
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn new_db() -> Connection {
+        let conn = Connection::open_in_memory().unwrap();
+        crate::gateway_secrets::migrate(&conn).unwrap();
+        conn
+    }
+
+    fn write_legacy(path: &std::path::Path, ddl: &str, rows: &[(&str, &[u8])]) {
+        let old = Connection::open(path).unwrap();
+        old.execute_batch(ddl).unwrap();
+        for (name, ct) in rows {
+            old.execute(
+                "INSERT INTO gateway_secrets (name, ciphertext) VALUES (?1, ?2)",
+                params![name, ct],
+            )
+            .unwrap();
+        }
+    }
+
+    const GOOD_DDL: &str =
+        "CREATE TABLE gateway_secrets (name TEXT PRIMARY KEY, ciphertext BLOB NOT NULL);";
+
+    #[test]
+    fn deleted_secret_stays_deleted_after_reopen() {
+        let dir = tempfile::tempdir().unwrap();
+        let old_path = dir.path().join("gateway.db");
+        write_legacy(&old_path, GOOD_DDL, &[("github_pat", b"cipher")]);
+
+        let conn = new_db();
+        import_legacy_secrets(&conn, &old_path).unwrap();
+        let present: i64 = conn
+            .query_row("SELECT COUNT(*) FROM gateway_secrets WHERE name='github_pat'", [], |r| r.get(0))
+            .unwrap();
+        assert_eq!(present, 1, "secret should import on first open");
+
+        // Legacy file is renamed once migrated, so a re-import can't resurrect.
+        assert!(!old_path.exists(), "legacy file should be renamed after migration");
+        conn.execute("DELETE FROM gateway_secrets WHERE name='github_pat'", []).unwrap();
+        import_legacy_secrets(&conn, &old_path).unwrap();
+        let after: i64 = conn
+            .query_row("SELECT COUNT(*) FROM gateway_secrets", [], |r| r.get(0))
+            .unwrap();
+        assert_eq!(after, 0, "deleted secret must not resurrect on reopen");
+    }
+
+    #[test]
+    fn incompatible_legacy_schema_does_not_brick_open() {
+        let dir = tempfile::tempdir().unwrap();
+        let old_path = dir.path().join("gateway.db");
+        // gateway_secrets exists but lacks the `ciphertext` column.
+        let old = Connection::open(&old_path).unwrap();
+        old.execute_batch("CREATE TABLE gateway_secrets (name TEXT, blob BLOB);").unwrap();
+        old.execute("INSERT INTO gateway_secrets (name, blob) VALUES ('x', X'00')", []).unwrap();
+        drop(old);
+
+        let conn = new_db();
+        // Must be Ok — a malformed legacy schema is skipped, not propagated.
+        import_legacy_secrets(&conn, &old_path).unwrap();
+        let count: i64 = conn
+            .query_row("SELECT COUNT(*) FROM gateway_secrets", [], |r| r.get(0))
+            .unwrap();
+        assert_eq!(count, 0);
+    }
+
+    #[test]
+    fn missing_legacy_file_is_noop() {
+        let dir = tempfile::tempdir().unwrap();
+        let conn = new_db();
+        import_legacy_secrets(&conn, &dir.path().join("nope.db")).unwrap();
+    }
+}

src/gateway_secrets.rs

@@ -8,18 +8,15 @@ use crate::auth_crypt;
 use rusqlite::{params, Connection, OptionalExtension};
 use std::path::Path;
 
-pub fn open_db(path: &Path) -> rusqlite::Result<Connection> {
-    if let Some(parent) = path.parent() {
-        let _ = std::fs::create_dir_all(parent);
-    }
-    let conn = Connection::open(path)?;
+/// Creates the `gateway_secrets` table in an existing connection (called
+/// from `db::open()` so the table lives in `agentflare.db`).
+pub fn migrate(conn: &Connection) -> rusqlite::Result<()> {
     conn.execute_batch(
         "CREATE TABLE IF NOT EXISTS gateway_secrets (
             name TEXT PRIMARY KEY,
             ciphertext BLOB NOT NULL
         );",
-    )?;
-    Ok(conn)
+    )
 }
 
 #[derive(Debug)]
@@ -112,12 +109,17 @@ mod tests {
         unsafe { std::env::remove_var("AGENTFLARE_VAULT_PASSPHRASE") };
     }
 
+    fn mem_migrated() -> Connection {
+        let conn = Connection::open_in_memory().unwrap();
+        migrate(&conn).unwrap();
+        conn
+    }
+
     #[test]
     fn roundtrip_set_get() {
         let _guard = GLOBAL_STATE_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         set_passphrase("test-pass");
-        let conn = Connection::open_in_memory().unwrap();
-        conn.execute_batch("CREATE TABLE gateway_secrets (name TEXT PRIMARY KEY, ciphertext BLOB NOT NULL);").unwrap();
+        let conn = mem_migrated();
         set_secret(&conn, "github_pat", "ghp_abc123").unwrap();
         assert_eq!(get_secret(&conn, "github_pat").unwrap(), Some("ghp_abc123".to_string()));
         clear_passphrase();
@@ -126,17 +128,15 @@ mod tests {
     #[test]
     fn get_missing_secret_returns_none() {
         let _guard = GLOBAL_STATE_LOCK.lock().unwrap_or_else(|e| e.into_inner());
-        let conn = Connection::open_in_memory().unwrap();
-        conn.execute_batch("CREATE TABLE gateway_secrets (name TEXT PRIMARY KEY, ciphertext BLOB NOT NULL);").unwrap();
+        let conn = mem_migrated();
         assert_eq!(get_secret(&conn, "missing").unwrap(), None);
     }
 
     #[test]
     fn wrong_passphrase_is_a_clear_error_not_silent_empty_string() {
         let _guard = GLOBAL_STATE_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         set_passphrase("right-pass");
-        let conn = Connection::open_in_memory().unwrap();
-        conn.execute_batch("CREATE TABLE gateway_secrets (name TEXT PRIMARY KEY, ciphertext BLOB NOT NULL);").unwrap();
+        let conn = mem_migrated();
         set_secret(&conn, "s", "value").unwrap();
         set_passphrase("wrong-pass");
         let err = get_secret(&conn, "s").unwrap_err();
@@ -148,8 +148,7 @@ mod tests {
     fn list_and_remove() {
         let _guard = GLOBAL_STATE_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         set_passphrase("test-pass");
-        let conn = Connection::open_in_memory().unwrap();
-        conn.execute_batch("CREATE TABLE gateway_secrets (name TEXT PRIMARY KEY, ciphertext BLOB NOT NULL);").unwrap();
+        let conn = mem_migrated();
         set_secret(&conn, "a", "1").unwrap();
         set_secret(&conn, "b", "2").unwrap();
         assert_eq!(list_secrets(&conn).unwrap(), vec!["a".to_string(), "b".to_string()]);

src/mcp_server.rs

@@ -930,10 +930,6 @@ impl AgentflareMcp {
         dirs::data_local_dir().unwrap_or_else(std::env::temp_dir).join("agentflare").join("gateway.db")
     }
 
-    fn gateway_secrets_db_path() -> std::path::PathBuf {
-        crate::paths::home().join(".agentflare").join("gateway.db")
-    }
-
     fn load_gateway_config() -> gateway_registry::GatewayConfig {
         let path = crate::paths::home().join(".agentflare").join("gateway.toml");
         match std::fs::read_to_string(&path) {
@@ -962,14 +958,10 @@ impl AgentflareMcp {
     }
 
     fn resolve_gateway_secrets() -> std::collections::HashMap<String, String> {
-        let db_path = Self::gateway_secrets_db_path();
-        let conn = match crate::gateway_secrets::open_db(&db_path) {
+        let conn = match crate::db::open() {
             Ok(conn) => conn,
             Err(e) => {
-                eprintln!(
-                    "agentflare: failed to open gateway secrets db at {}: {e}",
-                    db_path.display()
-                );
+                eprintln!("agentflare: failed to open agentflare.db for gateway secrets: {e}");
                 return std::collections::HashMap::new();
             }
         };