mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 23:50:17 +00:00
feat(setup): quick onboarding, preserve .env vars, clean up boot logging (#821)
* feat(setup): quick onboarding, preserve .env vars, clean up boot logging (#751, #674) - Add upsert_bootstrap_vars() to preserve user-added .env vars on re-onboarding - Add --quick mode: auto-defaults DB + security, asks only LLM provider (2 steps) - Auto-triggered onboarding uses quick mode for near-instant first run - Fix NEAR AI model fetch to use cloud-api.near.ai when API key is set - Handle missing WASM tools/channels directories gracefully - Downgrade all boot/shutdown tracing::info! to debug (boot screen shows user output) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(setup): gate env_backend variable behind postgres feature flag [skip-regression-check] Clippy lint fix — not a behavioral change, just moving a variable declaration inside the cfg(feature = "postgres") block where it's used. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(review): address PR review comments - WASM loaders: use tokio::fs::metadata, only treat NotFound as empty, propagate other IO errors, handle TOCTOU in read_dir - bootstrap: only ignore NotFound in read_to_string, propagate other errors - wizard: restore print_info/print_success for migrations in interactive mode (gated by !config.quick), keep tracing::debug for diagnostics - tests: use shared crate::config::helpers::ENV_MUTEX instead of separate NEARAI_ENV_MUTEX to prevent cross-test env var races - README: fix quick mode description to mention model selection, clarify auto_setup_database may prompt when DATABASE_URL is set Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(setup): skip prompts for DATABASE_URL in quick mode [skip-regression-check] auto_setup_database() now uses DATABASE_URL directly without calling step_database_postgres() (which prompts for confirmation). Quick mode should be fully non-interactive when env vars are already set. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(cli): update --quick help text to mention model selection [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
94d101924e
commit
3a2989d009
+40
-3
@@ -10,7 +10,7 @@ file first, then adjust the code to match.
|
||||
## Entry Points
|
||||
|
||||
```
|
||||
ironclaw onboard [--skip-auth] [--channels-only]
|
||||
ironclaw onboard [--skip-auth] [--channels-only] [--provider-only] [--quick]
|
||||
```
|
||||
|
||||
Explicit invocation. Loads `.env` files, runs the wizard, exits.
|
||||
@@ -26,6 +26,8 @@ the wizard). Otherwise triggers when no database is configured:
|
||||
- `LIBSQL_PATH` env var is set
|
||||
- `~/.ironclaw/ironclaw.db` exists on disk
|
||||
|
||||
Auto-triggered onboarding uses **quick mode** by default.
|
||||
|
||||
The `--no-onboard` CLI flag suppresses auto-detection.
|
||||
|
||||
---
|
||||
@@ -50,7 +52,41 @@ The `--no-onboard` CLI flag suppresses auto-detection.
|
||||
|
||||
---
|
||||
|
||||
## The 8-Step Wizard
|
||||
## Quick Mode
|
||||
|
||||
Quick mode (`--quick` flag, or auto-triggered on first run) provides a
|
||||
near-instant onboarding experience by auto-defaulting everything except
|
||||
the LLM provider and model selection.
|
||||
|
||||
```
|
||||
auto_setup_database() → libsql at ~/.ironclaw/ironclaw.db (zero prompts)
|
||||
auto_setup_security() → keychain or env var (zero prompts)
|
||||
Step 1/2: Inference Provider ← only interactive step
|
||||
Step 2/2: Model Selection ← only interactive step
|
||||
↓
|
||||
save_and_summarize() → includes tip to run `ironclaw onboard`
|
||||
```
|
||||
|
||||
**`auto_setup_database()`:** Uses existing env vars if set (`DATABASE_URL`
|
||||
for postgres, `LIBSQL_PATH` for libsql) without prompting. Otherwise
|
||||
defaults to libsql at `~/.ironclaw/ironclaw.db`, creates the database,
|
||||
and runs migrations silently. Falls back to interactive mode only when
|
||||
just the postgres feature is compiled and no `DATABASE_URL` is set.
|
||||
|
||||
**`auto_setup_security()`:** Checks for existing `SECRETS_MASTER_KEY`
|
||||
env var or OS keychain key. If neither exists, generates a new key and
|
||||
stores it in the keychain (macOS) or env var (Linux/other). Zero prompts
|
||||
except unavoidable macOS keychain dialogs.
|
||||
|
||||
**`.env` preservation (fix for #751):** `write_bootstrap_env()` now uses
|
||||
`upsert_bootstrap_vars()` instead of `save_bootstrap_env()`, preserving
|
||||
user-added variables like `HTTP_HOST` across re-onboarding.
|
||||
|
||||
The full 9-step wizard remains available via `ironclaw onboard`.
|
||||
|
||||
---
|
||||
|
||||
## The 9-Step Wizard
|
||||
|
||||
### Overview
|
||||
|
||||
@@ -62,7 +98,8 @@ Step 4: Model Selection
|
||||
Step 5: Embeddings
|
||||
Step 6: Channel Configuration
|
||||
Step 7: Extensions (tools)
|
||||
Step 8: Background Tasks (heartbeat)
|
||||
Step 8: Docker Sandbox
|
||||
Step 9: Background Tasks (heartbeat)
|
||||
↓
|
||||
save_and_summarize()
|
||||
```
|
||||
|
||||
+208
-10
@@ -76,6 +76,8 @@ pub struct SetupConfig {
|
||||
pub channels_only: bool,
|
||||
/// Only reconfigure LLM provider and model selection.
|
||||
pub provider_only: bool,
|
||||
/// Quick setup: auto-defaults everything except LLM provider and model.
|
||||
pub quick: bool,
|
||||
}
|
||||
|
||||
/// Interactive setup wizard for IronClaw.
|
||||
@@ -154,6 +156,26 @@ impl SetupWizard {
|
||||
print_step(1, 2, "Inference Provider");
|
||||
self.step_inference_provider().await?;
|
||||
self.persist_after_step().await;
|
||||
print_step(2, 2, "Model Selection");
|
||||
self.step_model_selection().await?;
|
||||
self.persist_after_step().await;
|
||||
} else if self.config.quick {
|
||||
// Quick mode: auto-default database + security, only ask for
|
||||
// LLM provider + model. Designed for first-run experience.
|
||||
self.auto_setup_database().await?;
|
||||
|
||||
// Load existing settings from DB (if any prior partial run)
|
||||
let step1_settings = self.settings.clone();
|
||||
self.try_load_existing_settings().await;
|
||||
self.settings.merge_from(&step1_settings);
|
||||
|
||||
self.auto_setup_security().await?;
|
||||
self.persist_after_step().await;
|
||||
|
||||
print_step(1, 2, "Inference Provider");
|
||||
self.step_inference_provider().await?;
|
||||
self.persist_after_step().await;
|
||||
|
||||
print_step(2, 2, "Model Selection");
|
||||
self.step_model_selection().await?;
|
||||
self.persist_after_step().await;
|
||||
@@ -659,7 +681,10 @@ impl SetupWizard {
|
||||
use refinery::embed_migrations;
|
||||
embed_migrations!("migrations");
|
||||
|
||||
print_info("Running migrations...");
|
||||
if !self.config.quick {
|
||||
print_info("Running migrations...");
|
||||
}
|
||||
tracing::debug!("Running PostgreSQL migrations...");
|
||||
|
||||
let mut client = pool
|
||||
.get()
|
||||
@@ -671,7 +696,10 @@ impl SetupWizard {
|
||||
.await
|
||||
.map_err(|e| SetupError::Database(format!("Migration failed: {}", e)))?;
|
||||
|
||||
print_success("Migrations applied");
|
||||
if !self.config.quick {
|
||||
print_success("Migrations applied");
|
||||
}
|
||||
tracing::debug!("PostgreSQL migrations applied");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -682,14 +710,20 @@ impl SetupWizard {
|
||||
if let Some(ref backend) = self.db_backend {
|
||||
use crate::db::Database;
|
||||
|
||||
print_info("Running migrations...");
|
||||
if !self.config.quick {
|
||||
print_info("Running migrations...");
|
||||
}
|
||||
tracing::debug!("Running libSQL migrations...");
|
||||
|
||||
backend
|
||||
.run_migrations()
|
||||
.await
|
||||
.map_err(|e| SetupError::Database(format!("Migration failed: {}", e)))?;
|
||||
|
||||
print_success("Migrations applied");
|
||||
if !self.config.quick {
|
||||
print_success("Migrations applied");
|
||||
}
|
||||
tracing::debug!("libSQL migrations applied");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -804,6 +838,140 @@ impl SetupWizard {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Auto-setup database with zero prompts (quick mode).
|
||||
///
|
||||
/// Uses existing env vars if present, otherwise defaults to libsql at the
|
||||
/// standard path. Falls back to the interactive `step_database()` only when
|
||||
/// just the postgres feature is compiled (can't auto-default postgres).
|
||||
async fn auto_setup_database(&mut self) -> Result<(), SetupError> {
|
||||
// If DATABASE_URL or LIBSQL_PATH already set, respect existing config
|
||||
#[cfg(feature = "postgres")]
|
||||
let env_backend = std::env::var("DATABASE_BACKEND").ok();
|
||||
|
||||
#[cfg(feature = "postgres")]
|
||||
if let Some(ref backend) = env_backend
|
||||
&& (backend == "postgres" || backend == "postgresql")
|
||||
{
|
||||
if let Ok(url) = std::env::var("DATABASE_URL") {
|
||||
print_info("Using existing PostgreSQL configuration");
|
||||
self.settings.database_backend = Some("postgres".to_string());
|
||||
self.settings.database_url = Some(url);
|
||||
return Ok(());
|
||||
}
|
||||
// Postgres configured but no URL — fall through to interactive
|
||||
return self.step_database().await;
|
||||
}
|
||||
|
||||
#[cfg(feature = "postgres")]
|
||||
if let Ok(url) = std::env::var("DATABASE_URL") {
|
||||
print_info("Using existing PostgreSQL configuration");
|
||||
self.settings.database_backend = Some("postgres".to_string());
|
||||
self.settings.database_url = Some(url);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Auto-default to libsql if the feature is compiled
|
||||
#[cfg(feature = "libsql")]
|
||||
{
|
||||
self.settings.database_backend = Some("libsql".to_string());
|
||||
|
||||
let existing_path = std::env::var("LIBSQL_PATH")
|
||||
.ok()
|
||||
.or_else(|| self.settings.libsql_path.clone());
|
||||
|
||||
let db_path = existing_path.unwrap_or_else(|| {
|
||||
crate::config::default_libsql_path()
|
||||
.to_string_lossy()
|
||||
.to_string()
|
||||
});
|
||||
|
||||
let turso_url = std::env::var("LIBSQL_URL").ok();
|
||||
let turso_token = std::env::var("LIBSQL_AUTH_TOKEN").ok();
|
||||
|
||||
self.test_database_connection_libsql(
|
||||
&db_path,
|
||||
turso_url.as_deref(),
|
||||
turso_token.as_deref(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
self.run_migrations_libsql().await?;
|
||||
|
||||
self.settings.libsql_path = Some(db_path.clone());
|
||||
if let Some(url) = turso_url {
|
||||
self.settings.libsql_url = Some(url);
|
||||
}
|
||||
|
||||
print_success(&format!("Using embedded database at {}", db_path));
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Only postgres feature compiled — can't auto-default, use interactive
|
||||
#[allow(unreachable_code)]
|
||||
{
|
||||
self.step_database().await
|
||||
}
|
||||
}
|
||||
|
||||
/// Auto-setup security with zero prompts (quick mode).
|
||||
///
|
||||
/// Silently configures the master key: uses existing env var or keychain
|
||||
/// key if available, otherwise generates and stores one automatically
|
||||
/// (keychain on macOS, env var fallback).
|
||||
async fn auto_setup_security(&mut self) -> Result<(), SetupError> {
|
||||
// Check env var first
|
||||
if std::env::var("SECRETS_MASTER_KEY").is_ok() {
|
||||
self.settings.secrets_master_key_source = KeySource::Env;
|
||||
print_success("Security configured (env var)");
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Try existing keychain key (no prompts — get_master_key may show
|
||||
// OS dialogs on macOS, but that's unavoidable for keychain access)
|
||||
if let Ok(keychain_key_bytes) = crate::secrets::keychain::get_master_key().await {
|
||||
let key_hex: String = keychain_key_bytes
|
||||
.iter()
|
||||
.map(|b| format!("{:02x}", b))
|
||||
.collect();
|
||||
self.secrets_crypto = Some(Arc::new(
|
||||
SecretsCrypto::new(SecretString::from(key_hex))
|
||||
.map_err(|e| SetupError::Config(e.to_string()))?,
|
||||
));
|
||||
self.settings.secrets_master_key_source = KeySource::Keychain;
|
||||
print_success("Security configured (keychain)");
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// No existing key — generate one
|
||||
// Try keychain first (preferred on macOS)
|
||||
let key = crate::secrets::keychain::generate_master_key();
|
||||
if crate::secrets::keychain::store_master_key(&key)
|
||||
.await
|
||||
.is_ok()
|
||||
{
|
||||
let key_hex: String = key.iter().map(|b| format!("{:02x}", b)).collect();
|
||||
self.secrets_crypto = Some(Arc::new(
|
||||
SecretsCrypto::new(SecretString::from(key_hex))
|
||||
.map_err(|e| SetupError::Config(e.to_string()))?,
|
||||
));
|
||||
self.settings.secrets_master_key_source = KeySource::Keychain;
|
||||
print_success("Master key stored in OS keychain");
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Keychain unavailable — fall back to env var mode
|
||||
let key_hex = crate::secrets::keychain::generate_master_key_hex();
|
||||
self.secrets_crypto = Some(Arc::new(
|
||||
SecretsCrypto::new(SecretString::from(key_hex.clone()))
|
||||
.map_err(|e| SetupError::Config(e.to_string()))?,
|
||||
));
|
||||
crate::config::inject_single_var("SECRETS_MASTER_KEY", &key_hex);
|
||||
self.settings.secrets_master_key_hex = Some(key_hex);
|
||||
self.settings.secrets_master_key_source = KeySource::Env;
|
||||
print_success("Master key stored in ~/.ironclaw/.env");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Step 3: Inference provider selection.
|
||||
///
|
||||
/// Uses the provider registry to dynamically build the selection menu.
|
||||
@@ -2506,7 +2674,7 @@ impl SetupWizard {
|
||||
.iter()
|
||||
.map(|(k, v)| (k.as_str(), v.as_str()))
|
||||
.collect();
|
||||
crate::bootstrap::save_bootstrap_env(&pairs).map_err(|e| {
|
||||
crate::bootstrap::upsert_bootstrap_vars(&pairs).map_err(|e| {
|
||||
SetupError::Io(std::io::Error::other(format!(
|
||||
"Failed to save bootstrap env to .env: {}",
|
||||
e
|
||||
@@ -2778,6 +2946,13 @@ impl SetupWizard {
|
||||
println!(" ironclaw onboard");
|
||||
println!();
|
||||
|
||||
if self.config.quick {
|
||||
print_info(
|
||||
"Tip: Run `ironclaw onboard` to configure channels, extensions, embeddings, and more.",
|
||||
);
|
||||
println!();
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
@@ -3217,11 +3392,6 @@ async fn discover_wasm_channels(dir: &std::path::Path) -> Vec<(String, ChannelCa
|
||||
/// Reads `NEARAI_API_KEY` from the environment so that users who authenticated
|
||||
/// via Cloud API key (option 4) don't get re-prompted during model selection.
|
||||
fn build_nearai_model_fetch_config() -> crate::config::LlmConfig {
|
||||
let base_url =
|
||||
std::env::var("NEARAI_BASE_URL").unwrap_or_else(|_| "https://private.near.ai".to_string());
|
||||
let auth_base_url =
|
||||
std::env::var("NEARAI_AUTH_URL").unwrap_or_else(|_| "https://private.near.ai".to_string());
|
||||
|
||||
// If the user authenticated via API key (option 4), the key is stored
|
||||
// as an env var. Pass it through so `resolve_bearer_token()` doesn't
|
||||
// re-trigger the interactive auth prompt.
|
||||
@@ -3230,6 +3400,17 @@ fn build_nearai_model_fetch_config() -> crate::config::LlmConfig {
|
||||
.filter(|k| !k.is_empty())
|
||||
.map(secrecy::SecretString::from);
|
||||
|
||||
// Match the same base_url logic as LlmConfig::resolve(): use cloud-api
|
||||
// when an API key is present, private.near.ai for session-token auth.
|
||||
let default_base = if api_key.is_some() {
|
||||
"https://cloud-api.near.ai"
|
||||
} else {
|
||||
"https://private.near.ai"
|
||||
};
|
||||
let base_url = std::env::var("NEARAI_BASE_URL").unwrap_or_else(|_| default_base.to_string());
|
||||
let auth_base_url =
|
||||
std::env::var("NEARAI_AUTH_URL").unwrap_or_else(|_| "https://private.near.ai".to_string());
|
||||
|
||||
crate::config::LlmConfig {
|
||||
backend: "nearai".to_string(),
|
||||
session: crate::llm::session::SessionConfig {
|
||||
@@ -3466,6 +3647,7 @@ mod tests {
|
||||
use tempfile::tempdir;
|
||||
|
||||
use super::*;
|
||||
use crate::config::helpers::ENV_MUTEX;
|
||||
|
||||
#[test]
|
||||
fn test_wizard_creation() {
|
||||
@@ -3480,6 +3662,7 @@ mod tests {
|
||||
skip_auth: true,
|
||||
channels_only: false,
|
||||
provider_only: false,
|
||||
quick: false,
|
||||
};
|
||||
let wizard = SetupWizard::with_config(config);
|
||||
assert!(wizard.config.skip_auth);
|
||||
@@ -3860,7 +4043,9 @@ mod tests {
|
||||
fn test_build_nearai_model_fetch_config_picks_up_api_key_env() {
|
||||
use secrecy::ExposeSecret;
|
||||
|
||||
let _lock = ENV_MUTEX.lock().unwrap();
|
||||
let _guard = EnvGuard::set("NEARAI_API_KEY", "test-cloud-api-key-12345");
|
||||
let _guard2 = EnvGuard::clear("NEARAI_BASE_URL");
|
||||
|
||||
let config = build_nearai_model_fetch_config();
|
||||
assert!(
|
||||
@@ -3871,24 +4056,37 @@ mod tests {
|
||||
config.nearai.api_key.as_ref().unwrap().expose_secret(),
|
||||
"test-cloud-api-key-12345"
|
||||
);
|
||||
// With API key, base_url must point to cloud-api (not private.near.ai)
|
||||
assert_eq!(
|
||||
config.nearai.base_url, "https://cloud-api.near.ai",
|
||||
"API key auth must use cloud-api base URL for model fetching"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression test for #799: when NEARAI_API_KEY is absent or empty,
|
||||
/// the config should have `api_key: None` (session token path).
|
||||
#[test]
|
||||
fn test_build_nearai_model_fetch_config_none_when_no_api_key() {
|
||||
let _lock = ENV_MUTEX.lock().unwrap();
|
||||
let _guard = EnvGuard::clear("NEARAI_API_KEY");
|
||||
let _guard2 = EnvGuard::clear("NEARAI_BASE_URL");
|
||||
|
||||
let config = build_nearai_model_fetch_config();
|
||||
assert!(
|
||||
config.nearai.api_key.is_none(),
|
||||
"config should have no api_key when env var is absent"
|
||||
);
|
||||
// Without API key, base_url must point to private.near.ai (session token)
|
||||
assert_eq!(
|
||||
config.nearai.base_url, "https://private.near.ai",
|
||||
"session-token auth must use private.near.ai base URL"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression test for #799: empty NEARAI_API_KEY should be treated as absent.
|
||||
#[test]
|
||||
fn test_build_nearai_model_fetch_config_none_when_empty_api_key() {
|
||||
let _lock = ENV_MUTEX.lock().unwrap();
|
||||
let _guard = EnvGuard::set("NEARAI_API_KEY", "");
|
||||
|
||||
let config = build_nearai_model_fetch_config();
|
||||
|
||||
Reference in New Issue
Block a user