mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
feat(bootstrap): auto-detect libsql when ironclaw.db exists (#399)
* feat(bootstrap): auto-detect libsql when ironclaw.db exists If DATABASE_BACKEND is unset after loading all env files and ~/.ironclaw/ironclaw.db exists, default to libsql automatically. Fixes the chicken-and-egg problem on cloud instances where no DATABASE_URL is configured: users no longer need to prefix every ironclaw command with DATABASE_BACKEND=libsql. Priority order: explicit env var > .env > ~/.ironclaw/.env > auto-detect Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(bootstrap): move env loading to sync main() before tokio runtime - Fix cargo fmt: wrap three long assert! lines in new tests - Address set_var data race: load_ironclaw_env() is now called from a synchronous fn main() wrapper before the Tokio runtime starts, making the set_var call provably safe (no worker threads exist yet) - Remove the redundant dotenvy::dotenv() + load_ironclaw_env() calls from inside command handlers and agent startup (already done pre-tokio) - Update SAFETY comment to reflect the actual invariant Addresses Gemini code review comment and cargo fmt CI failure on PR #399. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --------- Co-authored-by: Claude Sonnet 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
9ce09f71b0
commit
9b25e7566c
+86
-1
@@ -84,11 +84,16 @@ pub fn ironclaw_env_path() -> PathBuf {
|
||||
/// takes priority over `~/.ironclaw/.env`. dotenvy never overwrites
|
||||
/// existing env vars, so the effective priority is:
|
||||
///
|
||||
/// explicit env vars > `./.env` > `~/.ironclaw/.env`
|
||||
/// explicit env vars > `./.env` > `~/.ironclaw/.env` > auto-detect
|
||||
///
|
||||
/// If `~/.ironclaw/.env` doesn't exist but the legacy `bootstrap.json` does,
|
||||
/// extracts `DATABASE_URL` from it and writes the `.env` file (one-time
|
||||
/// upgrade from the old config format).
|
||||
///
|
||||
/// After loading the `.env` file, auto-detects the libsql backend: if
|
||||
/// `DATABASE_BACKEND` is still unset and `~/.ironclaw/ironclaw.db` exists,
|
||||
/// defaults to `libsql` so cloud instances work out of the box without any
|
||||
/// manual configuration.
|
||||
pub fn load_ironclaw_env() {
|
||||
let path = ironclaw_env_path();
|
||||
|
||||
@@ -100,6 +105,22 @@ pub fn load_ironclaw_env() {
|
||||
if path.exists() {
|
||||
let _ = dotenvy::from_path(&path);
|
||||
}
|
||||
|
||||
// Auto-detect libsql: if DATABASE_BACKEND is still unset after loading
|
||||
// all env files, and the local SQLite DB exists, default to libsql.
|
||||
// This avoids the chicken-and-egg problem on cloud instances where no
|
||||
// DATABASE_URL is configured but ironclaw.db is already present.
|
||||
if std::env::var("DATABASE_BACKEND").is_err() {
|
||||
let default_db = dirs::home_dir()
|
||||
.unwrap_or_default()
|
||||
.join(".ironclaw")
|
||||
.join("ironclaw.db");
|
||||
if default_db.exists() {
|
||||
// SAFETY: `load_ironclaw_env` is called from a synchronous `fn main()`
|
||||
// before the Tokio runtime is started, so no other threads exist yet.
|
||||
unsafe { std::env::set_var("DATABASE_BACKEND", "libsql") };
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// If `bootstrap.json` exists, pull `database_url` out of it and write `.env`.
|
||||
@@ -659,6 +680,42 @@ INJECTED="pwned"#;
|
||||
assert_eq!(onboard.unwrap().1, "true");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_libsql_autodetect_sets_backend_when_db_exists() {
|
||||
let _guard = ENV_MUTEX.lock().unwrap();
|
||||
let old_val = std::env::var("DATABASE_BACKEND").ok();
|
||||
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
|
||||
unsafe { std::env::remove_var("DATABASE_BACKEND") };
|
||||
|
||||
let dir = tempdir().unwrap();
|
||||
let db_path = dir.path().join("ironclaw.db");
|
||||
|
||||
// No DB file — auto-detect guard should not trigger.
|
||||
assert!(!db_path.exists());
|
||||
let would_trigger = std::env::var("DATABASE_BACKEND").is_err() && db_path.exists();
|
||||
assert!(
|
||||
!would_trigger,
|
||||
"should not auto-detect when db file is absent"
|
||||
);
|
||||
|
||||
// Create the DB file — guard should now trigger.
|
||||
std::fs::write(&db_path, "").unwrap();
|
||||
assert!(db_path.exists());
|
||||
|
||||
// Simulate the detection logic (DATABASE_BACKEND unset + db exists).
|
||||
let detected = std::env::var("DATABASE_BACKEND").is_err() && db_path.exists();
|
||||
assert!(
|
||||
detected,
|
||||
"should detect libsql when db file is present and backend unset"
|
||||
);
|
||||
|
||||
// Restore.
|
||||
if let Some(val) = old_val {
|
||||
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
|
||||
unsafe { std::env::set_var("DATABASE_BACKEND", val) };
|
||||
}
|
||||
}
|
||||
|
||||
// === QA Plan P1 - 1.2: Bootstrap .env round-trip tests ===
|
||||
|
||||
#[test]
|
||||
@@ -694,6 +751,34 @@ INJECTED="pwned"#;
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_libsql_autodetect_does_not_override_explicit_backend() {
|
||||
let _guard = ENV_MUTEX.lock().unwrap();
|
||||
let old_val = std::env::var("DATABASE_BACKEND").ok();
|
||||
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
|
||||
unsafe { std::env::set_var("DATABASE_BACKEND", "postgres") };
|
||||
|
||||
let dir = tempdir().unwrap();
|
||||
let db_path = dir.path().join("ironclaw.db");
|
||||
std::fs::write(&db_path, "").unwrap();
|
||||
|
||||
// The guard: only sets libsql if DATABASE_BACKEND is NOT already set.
|
||||
let would_override = std::env::var("DATABASE_BACKEND").is_err() && db_path.exists();
|
||||
assert!(
|
||||
!would_override,
|
||||
"must not override an explicitly set DATABASE_BACKEND"
|
||||
);
|
||||
|
||||
// Restore.
|
||||
if let Some(val) = old_val {
|
||||
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
|
||||
unsafe { std::env::set_var("DATABASE_BACKEND", val) };
|
||||
} else {
|
||||
// SAFETY: ENV_MUTEX ensures single-threaded access to env vars in tests
|
||||
unsafe { std::env::remove_var("DATABASE_BACKEND") };
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bootstrap_env_special_chars_in_url() {
|
||||
let dir = tempdir().unwrap();
|
||||
|
||||
+13
-14
@@ -44,8 +44,19 @@ fn init_cli_tracing() {
|
||||
.init();
|
||||
}
|
||||
|
||||
#[tokio::main]
|
||||
async fn main() -> anyhow::Result<()> {
|
||||
/// Synchronous entry point. Loads `.env` files before the Tokio runtime
|
||||
/// starts so that `std::env::set_var` is safe (no worker threads yet).
|
||||
fn main() -> anyhow::Result<()> {
|
||||
let _ = dotenvy::dotenv();
|
||||
ironclaw::bootstrap::load_ironclaw_env();
|
||||
|
||||
tokio::runtime::Builder::new_multi_thread()
|
||||
.enable_all()
|
||||
.build()?
|
||||
.block_on(async_main())
|
||||
}
|
||||
|
||||
async fn async_main() -> anyhow::Result<()> {
|
||||
let cli = Cli::parse();
|
||||
|
||||
// Handle non-agent commands first (they don't need full setup)
|
||||
@@ -80,14 +91,10 @@ async fn main() -> anyhow::Result<()> {
|
||||
}
|
||||
Some(Command::Doctor) => {
|
||||
init_cli_tracing();
|
||||
let _ = dotenvy::dotenv();
|
||||
ironclaw::bootstrap::load_ironclaw_env();
|
||||
return ironclaw::cli::run_doctor_command().await;
|
||||
}
|
||||
Some(Command::Status) => {
|
||||
init_cli_tracing();
|
||||
let _ = dotenvy::dotenv();
|
||||
ironclaw::bootstrap::load_ironclaw_env();
|
||||
return run_status_command().await;
|
||||
}
|
||||
Some(Command::Completion(completion)) => {
|
||||
@@ -115,9 +122,6 @@ async fn main() -> anyhow::Result<()> {
|
||||
skip_auth,
|
||||
channels_only,
|
||||
}) => {
|
||||
let _ = dotenvy::dotenv();
|
||||
ironclaw::bootstrap::load_ironclaw_env();
|
||||
|
||||
#[cfg(any(feature = "postgres", feature = "libsql"))]
|
||||
{
|
||||
let config = SetupConfig {
|
||||
@@ -141,11 +145,6 @@ async fn main() -> anyhow::Result<()> {
|
||||
|
||||
// ── Agent startup ──────────────────────────────────────────────────
|
||||
|
||||
// Load .env files early so DATABASE_URL (and any other vars) are
|
||||
// available to all subsequent env-based config resolution.
|
||||
let _ = dotenvy::dotenv();
|
||||
ironclaw::bootstrap::load_ironclaw_env();
|
||||
|
||||
// Enhanced first-run detection
|
||||
#[cfg(any(feature = "postgres", feature = "libsql"))]
|
||||
if !cli.no_onboard
|
||||
|
||||
Reference in New Issue
Block a user