mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix: CLI commands ignore runtime DATABASE_BACKEND when both features compiled (#740)
* fix: CLI commands ignore runtime DATABASE_BACKEND when both features compiled
`tool auth` and `mcp` CLI subcommands used compile-time `#[cfg]` gates
to select the database backend for secrets storage. When the binary is
compiled with both `postgres` and `libsql` features, the `#[cfg(feature
= "postgres")]` block always wins regardless of the runtime
`DATABASE_BACKEND` setting. This causes `tool auth` and `mcp auth` to
fail with a connection error for users running the libsql backend.
Switch both functions to `match config.database.backend { ... }` with
inner `#[cfg]` guards on each arm, matching the pattern already used in
`main.rs` and `app.rs`.
Also adds a top-level `auth` section to the Telegram channel
capabilities file so `ironclaw tool auth telegram` works for channels
(previously only tools had this section).
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: extract create_secrets_store factory into src/db, bump telegram version
- Move duplicated DB backend selection logic from cli/tool.rs and
cli/mcp.rs into a shared db::create_secrets_store() factory, following
the existing db::connect_from_config() pattern.
- Bump telegram channel version 0.2.0 → 0.2.1 to fix CI Version Bump Check.
- Add regression test for create_secrets_store with libsql backend.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* fix: address review feedback — wizard.rs pattern, formatting, version bump
- Convert setup/wizard.rs secrets store creation from tri-branch #[cfg]
to runtime match on selected_backend (same pattern as CLI fix).
- Fix formatting (assert! line wrapping caught by CI).
- Bump telegram version to 0.2.2 (main already has 0.2.1).
- Merge latest main.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* chore: fix regression test doc comment formatting
Co-Authored-By: Claude Opus 4.6 <[email protected]>
[skip-regression-check]
* fix: address Copilot review — wizard default backend, error chain preservation
- Fix wizard.rs: default selected_backend to "libsql" in libsql-only
builds so create_libsql_secrets_store is not skipped.
- Preserve error chain: replace .map_err(|e| anyhow!("{}", e)) with ?
in cli/tool.rs and cli/mcp.rs since DatabaseError implements
std::error::Error.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---------
Co-authored-by: Tiny Tim <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: firat.sertgoz <[email protected]>
This commit is contained in:
co-authored by
Tiny Tim
Claude Opus 4.6
firat.sertgoz
parent
652f30a826
commit
d8dcc34319
@@ -1,9 +1,17 @@
|
|||||||
{
|
{
|
||||||
"version": "0.2.0",
|
"version": "0.2.2",
|
||||||
"wit_version": "0.3.0",
|
"wit_version": "0.3.0",
|
||||||
"type": "channel",
|
"type": "channel",
|
||||||
"name": "telegram",
|
"name": "telegram",
|
||||||
"description": "Telegram Bot API channel for receiving and responding to Telegram messages",
|
"description": "Telegram Bot API channel for receiving and responding to Telegram messages",
|
||||||
|
"auth": {
|
||||||
|
"secret_name": "telegram_bot_token",
|
||||||
|
"display_name": "Telegram",
|
||||||
|
"instructions": "Get your bot token from @BotFather on Telegram (https://t.me/BotFather). Send /newbot or /token to get it.",
|
||||||
|
"setup_url": "https://t.me/BotFather",
|
||||||
|
"token_hint": "Looks like 123456789:AABBccDDeeFFgg...",
|
||||||
|
"env_var": "TELEGRAM_BOT_TOKEN"
|
||||||
|
},
|
||||||
"setup": {
|
"setup": {
|
||||||
"required_secrets": [
|
"required_secrets": [
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
"name": "telegram",
|
"name": "telegram",
|
||||||
"display_name": "Telegram Channel",
|
"display_name": "Telegram Channel",
|
||||||
"kind": "channel",
|
"kind": "channel",
|
||||||
"version": "0.2.1",
|
"version": "0.2.2",
|
||||||
"wit_version": "0.3.0",
|
"wit_version": "0.3.0",
|
||||||
"description": "Talk to your agent through a Telegram bot",
|
"description": "Talk to your agent through a Telegram bot",
|
||||||
"keywords": [
|
"keywords": [
|
||||||
|
|||||||
+2
-56
@@ -10,8 +10,6 @@ use clap::{Args, Subcommand};
|
|||||||
|
|
||||||
use crate::config::Config;
|
use crate::config::Config;
|
||||||
use crate::db::Database;
|
use crate::db::Database;
|
||||||
#[cfg(feature = "postgres")]
|
|
||||||
use crate::secrets::PostgresSecretsStore;
|
|
||||||
use crate::secrets::{SecretsCrypto, SecretsStore};
|
use crate::secrets::{SecretsCrypto, SecretsStore};
|
||||||
use crate::tools::mcp::{
|
use crate::tools::mcp::{
|
||||||
McpClient, McpServerConfig, McpSessionManager, OAuthConfig,
|
McpClient, McpServerConfig, McpSessionManager, OAuthConfig,
|
||||||
@@ -638,61 +636,9 @@ async fn get_secrets_store() -> anyhow::Result<Arc<dyn SecretsStore + Send + Syn
|
|||||||
)
|
)
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
let crypto = SecretsCrypto::new(master_key.clone())?;
|
let crypto = Arc::new(SecretsCrypto::new(master_key.clone())?);
|
||||||
|
|
||||||
#[cfg(feature = "postgres")]
|
Ok(crate::db::create_secrets_store(&config.database, crypto).await?)
|
||||||
{
|
|
||||||
let store = crate::history::Store::new(&config.database).await?;
|
|
||||||
store.run_migrations().await?;
|
|
||||||
Ok(Arc::new(PostgresSecretsStore::new(
|
|
||||||
store.pool(),
|
|
||||||
Arc::new(crypto),
|
|
||||||
)))
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(all(feature = "libsql", not(feature = "postgres")))]
|
|
||||||
{
|
|
||||||
use crate::db::Database as _;
|
|
||||||
use crate::db::libsql::LibSqlBackend;
|
|
||||||
use secrecy::ExposeSecret as _;
|
|
||||||
|
|
||||||
let default_path = crate::config::default_libsql_path();
|
|
||||||
let db_path = config
|
|
||||||
.database
|
|
||||||
.libsql_path
|
|
||||||
.as_deref()
|
|
||||||
.unwrap_or(&default_path);
|
|
||||||
|
|
||||||
let backend = if let Some(ref url) = config.database.libsql_url {
|
|
||||||
let token = config.database.libsql_auth_token.as_ref().ok_or_else(|| {
|
|
||||||
anyhow::anyhow!("LIBSQL_AUTH_TOKEN is required when LIBSQL_URL is set")
|
|
||||||
})?;
|
|
||||||
LibSqlBackend::new_remote_replica(db_path, url, token.expose_secret())
|
|
||||||
.await
|
|
||||||
.map_err(|e| anyhow::anyhow!("{}", e))?
|
|
||||||
} else {
|
|
||||||
LibSqlBackend::new_local(db_path)
|
|
||||||
.await
|
|
||||||
.map_err(|e| anyhow::anyhow!("{}", e))?
|
|
||||||
};
|
|
||||||
backend
|
|
||||||
.run_migrations()
|
|
||||||
.await
|
|
||||||
.map_err(|e| anyhow::anyhow!("{}", e))?;
|
|
||||||
|
|
||||||
Ok(Arc::new(crate::secrets::LibSqlSecretsStore::new(
|
|
||||||
backend.shared_db(),
|
|
||||||
Arc::new(crypto),
|
|
||||||
)))
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(not(any(feature = "postgres", feature = "libsql")))]
|
|
||||||
{
|
|
||||||
let _ = crypto;
|
|
||||||
anyhow::bail!(
|
|
||||||
"No database backend available for secrets. Enable 'postgres' or 'libsql' feature."
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
|
|||||||
+2
-56
@@ -11,10 +11,6 @@ use tokio::fs;
|
|||||||
|
|
||||||
use crate::bootstrap::ironclaw_base_dir;
|
use crate::bootstrap::ironclaw_base_dir;
|
||||||
use crate::config::Config;
|
use crate::config::Config;
|
||||||
#[allow(unused_imports)]
|
|
||||||
use crate::db::Database;
|
|
||||||
#[cfg(feature = "postgres")]
|
|
||||||
use crate::secrets::PostgresSecretsStore;
|
|
||||||
use crate::secrets::{CreateSecretParams, SecretsCrypto, SecretsStore};
|
use crate::secrets::{CreateSecretParams, SecretsCrypto, SecretsStore};
|
||||||
use crate::tools::wasm::{CapabilitiesFile, compute_binary_hash};
|
use crate::tools::wasm::{CapabilitiesFile, compute_binary_hash};
|
||||||
|
|
||||||
@@ -563,59 +559,9 @@ async fn init_secrets_store() -> anyhow::Result<Arc<dyn SecretsStore + Send + Sy
|
|||||||
)
|
)
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
let crypto = SecretsCrypto::new(master_key.clone())?;
|
let crypto = Arc::new(SecretsCrypto::new(master_key.clone())?);
|
||||||
|
|
||||||
let store: Arc<dyn SecretsStore + Send + Sync> = {
|
Ok(crate::db::create_secrets_store(&config.database, crypto).await?)
|
||||||
#[cfg(feature = "postgres")]
|
|
||||||
{
|
|
||||||
let store = crate::history::Store::new(&config.database).await?;
|
|
||||||
store.run_migrations().await?;
|
|
||||||
Arc::new(PostgresSecretsStore::new(store.pool(), Arc::new(crypto)))
|
|
||||||
}
|
|
||||||
#[cfg(all(feature = "libsql", not(feature = "postgres")))]
|
|
||||||
{
|
|
||||||
use crate::db::Database as _;
|
|
||||||
use crate::db::libsql::LibSqlBackend;
|
|
||||||
use secrecy::ExposeSecret as _;
|
|
||||||
|
|
||||||
let default_path = crate::config::default_libsql_path();
|
|
||||||
let db_path = config
|
|
||||||
.database
|
|
||||||
.libsql_path
|
|
||||||
.as_deref()
|
|
||||||
.unwrap_or(&default_path);
|
|
||||||
|
|
||||||
let backend = if let Some(ref url) = config.database.libsql_url {
|
|
||||||
let token = config.database.libsql_auth_token.as_ref().ok_or_else(|| {
|
|
||||||
anyhow::anyhow!("LIBSQL_AUTH_TOKEN is required when LIBSQL_URL is set")
|
|
||||||
})?;
|
|
||||||
LibSqlBackend::new_remote_replica(db_path, url, token.expose_secret())
|
|
||||||
.await
|
|
||||||
.map_err(|e| anyhow::anyhow!("{}", e))?
|
|
||||||
} else {
|
|
||||||
LibSqlBackend::new_local(db_path)
|
|
||||||
.await
|
|
||||||
.map_err(|e| anyhow::anyhow!("{}", e))?
|
|
||||||
};
|
|
||||||
backend
|
|
||||||
.run_migrations()
|
|
||||||
.await
|
|
||||||
.map_err(|e| anyhow::anyhow!("{}", e))?;
|
|
||||||
|
|
||||||
Arc::new(crate::secrets::LibSqlSecretsStore::new(
|
|
||||||
backend.shared_db(),
|
|
||||||
Arc::new(crypto),
|
|
||||||
))
|
|
||||||
}
|
|
||||||
#[cfg(not(any(feature = "postgres", feature = "libsql")))]
|
|
||||||
{
|
|
||||||
let _ = crypto;
|
|
||||||
anyhow::bail!(
|
|
||||||
"No database backend available for secrets. Enable 'postgres' or 'libsql' feature."
|
|
||||||
);
|
|
||||||
}
|
|
||||||
};
|
|
||||||
Ok(store)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Configure authentication for a tool.
|
/// Configure authentication for a tool.
|
||||||
|
|||||||
+101
@@ -91,6 +91,64 @@ pub async fn connect_from_config(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Create a secrets store from database and secrets configuration.
|
||||||
|
///
|
||||||
|
/// This is the shared factory for CLI commands and other call sites that need
|
||||||
|
/// a `SecretsStore` without going through the full `AppBuilder`. Mirrors the
|
||||||
|
/// pattern of [`connect_from_config`] but returns a secrets-specific store.
|
||||||
|
pub async fn create_secrets_store(
|
||||||
|
config: &crate::config::DatabaseConfig,
|
||||||
|
crypto: Arc<crate::secrets::SecretsCrypto>,
|
||||||
|
) -> Result<Arc<dyn crate::secrets::SecretsStore + Send + Sync>, DatabaseError> {
|
||||||
|
match config.backend {
|
||||||
|
#[cfg(feature = "libsql")]
|
||||||
|
crate::config::DatabaseBackend::LibSql => {
|
||||||
|
use secrecy::ExposeSecret as _;
|
||||||
|
|
||||||
|
let default_path = crate::config::default_libsql_path();
|
||||||
|
let db_path = config.libsql_path.as_deref().unwrap_or(&default_path);
|
||||||
|
|
||||||
|
let backend = if let Some(ref url) = config.libsql_url {
|
||||||
|
let token = config.libsql_auth_token.as_ref().ok_or_else(|| {
|
||||||
|
DatabaseError::Pool(
|
||||||
|
"LIBSQL_AUTH_TOKEN required when LIBSQL_URL is set".to_string(),
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
libsql::LibSqlBackend::new_remote_replica(db_path, url, token.expose_secret())
|
||||||
|
.await
|
||||||
|
.map_err(|e| DatabaseError::Pool(e.to_string()))?
|
||||||
|
} else {
|
||||||
|
libsql::LibSqlBackend::new_local(db_path)
|
||||||
|
.await
|
||||||
|
.map_err(|e| DatabaseError::Pool(e.to_string()))?
|
||||||
|
};
|
||||||
|
backend.run_migrations().await?;
|
||||||
|
|
||||||
|
Ok(Arc::new(crate::secrets::LibSqlSecretsStore::new(
|
||||||
|
backend.shared_db(),
|
||||||
|
crypto,
|
||||||
|
)))
|
||||||
|
}
|
||||||
|
#[cfg(feature = "postgres")]
|
||||||
|
_ => {
|
||||||
|
let pg = postgres::PgBackend::new(config)
|
||||||
|
.await
|
||||||
|
.map_err(|e| DatabaseError::Pool(e.to_string()))?;
|
||||||
|
pg.run_migrations().await?;
|
||||||
|
|
||||||
|
Ok(Arc::new(crate::secrets::PostgresSecretsStore::new(
|
||||||
|
pg.pool(),
|
||||||
|
crypto,
|
||||||
|
)))
|
||||||
|
}
|
||||||
|
#[cfg(not(feature = "postgres"))]
|
||||||
|
_ => Err(DatabaseError::Pool(
|
||||||
|
"No database backend available for secrets. Enable 'postgres' or 'libsql' feature."
|
||||||
|
.to_string(),
|
||||||
|
)),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ==================== Sub-traits ====================
|
// ==================== Sub-traits ====================
|
||||||
//
|
//
|
||||||
// Each sub-trait groups related persistence methods. The `Database` supertrait
|
// Each sub-trait groups related persistence methods. The `Database` supertrait
|
||||||
@@ -435,3 +493,46 @@ pub trait Database:
|
|||||||
/// Run schema migrations for this backend.
|
/// Run schema migrations for this backend.
|
||||||
async fn run_migrations(&self) -> Result<(), DatabaseError>;
|
async fn run_migrations(&self) -> Result<(), DatabaseError>;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
/// Regression test: `create_secrets_store` selects the correct backend at
|
||||||
|
/// runtime based on `DatabaseConfig`, not at compile time. Previously the
|
||||||
|
/// CLI duplicated this logic with compile-time `#[cfg]` gates that always
|
||||||
|
/// chose postgres when both features were enabled (PR #209).
|
||||||
|
#[cfg(feature = "libsql")]
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_create_secrets_store_libsql_backend() {
|
||||||
|
use secrecy::SecretString;
|
||||||
|
|
||||||
|
let tmp = tempfile::tempdir().unwrap();
|
||||||
|
let db_path = tmp.path().join("test.db");
|
||||||
|
|
||||||
|
let config = crate::config::DatabaseConfig {
|
||||||
|
backend: crate::config::DatabaseBackend::LibSql,
|
||||||
|
libsql_path: Some(db_path),
|
||||||
|
libsql_url: None,
|
||||||
|
libsql_auth_token: None,
|
||||||
|
url: SecretString::from("unused://libsql".to_string()),
|
||||||
|
pool_size: 1,
|
||||||
|
ssl_mode: crate::config::SslMode::default(),
|
||||||
|
};
|
||||||
|
|
||||||
|
let master_key = SecretString::from("a]".repeat(16));
|
||||||
|
let crypto = Arc::new(crate::secrets::SecretsCrypto::new(master_key).unwrap());
|
||||||
|
|
||||||
|
let store = create_secrets_store(&config, crypto).await;
|
||||||
|
assert!(
|
||||||
|
store.is_ok(),
|
||||||
|
"create_secrets_store should succeed for libsql backend"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Verify basic operation works
|
||||||
|
let store = store.unwrap();
|
||||||
|
let exists = store.exists("test_user", "nonexistent_secret").await;
|
||||||
|
assert!(exists.is_ok());
|
||||||
|
assert!(!exists.unwrap());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+26
-24
@@ -1612,48 +1612,50 @@ impl SetupWizard {
|
|||||||
};
|
};
|
||||||
|
|
||||||
// Create backend-appropriate secrets store.
|
// Create backend-appropriate secrets store.
|
||||||
// Respect the user's selected backend when both features are compiled,
|
// Use runtime dispatch based on the user's selected backend.
|
||||||
// so we don't accidentally use a postgres pool from DATABASE_URL when
|
// Default to whichever backend is compiled in. When only libsql is
|
||||||
// libsql was chosen (or vice versa).
|
// available, we must not default to "postgres" or we'd skip store creation.
|
||||||
|
let default_backend = {
|
||||||
|
#[cfg(feature = "postgres")]
|
||||||
|
{
|
||||||
|
"postgres"
|
||||||
|
}
|
||||||
|
#[cfg(not(feature = "postgres"))]
|
||||||
|
{
|
||||||
|
"libsql"
|
||||||
|
}
|
||||||
|
};
|
||||||
let selected_backend = self
|
let selected_backend = self
|
||||||
.settings
|
.settings
|
||||||
.database_backend
|
.database_backend
|
||||||
.as_deref()
|
.as_deref()
|
||||||
.unwrap_or("postgres");
|
.unwrap_or(default_backend);
|
||||||
|
|
||||||
#[cfg(all(feature = "libsql", feature = "postgres"))]
|
match selected_backend {
|
||||||
{
|
#[cfg(feature = "libsql")]
|
||||||
if selected_backend == "libsql" {
|
"libsql" | "turso" | "sqlite" => {
|
||||||
if let Some(store) = self.create_libsql_secrets_store(&crypto)? {
|
if let Some(store) = self.create_libsql_secrets_store(&crypto)? {
|
||||||
return Ok(SecretsContext::from_store(store, "default"));
|
return Ok(SecretsContext::from_store(store, "default"));
|
||||||
}
|
}
|
||||||
|
// Fallback to postgres if libsql store creation returned None
|
||||||
|
#[cfg(feature = "postgres")]
|
||||||
if let Some(store) = self.create_postgres_secrets_store(&crypto).await? {
|
if let Some(store) = self.create_postgres_secrets_store(&crypto).await? {
|
||||||
return Ok(SecretsContext::from_store(store, "default"));
|
return Ok(SecretsContext::from_store(store, "default"));
|
||||||
}
|
}
|
||||||
} else {
|
}
|
||||||
|
#[cfg(feature = "postgres")]
|
||||||
|
_ => {
|
||||||
if let Some(store) = self.create_postgres_secrets_store(&crypto).await? {
|
if let Some(store) = self.create_postgres_secrets_store(&crypto).await? {
|
||||||
return Ok(SecretsContext::from_store(store, "default"));
|
return Ok(SecretsContext::from_store(store, "default"));
|
||||||
}
|
}
|
||||||
|
// Fallback to libsql if postgres store creation returned None
|
||||||
|
#[cfg(feature = "libsql")]
|
||||||
if let Some(store) = self.create_libsql_secrets_store(&crypto)? {
|
if let Some(store) = self.create_libsql_secrets_store(&crypto)? {
|
||||||
return Ok(SecretsContext::from_store(store, "default"));
|
return Ok(SecretsContext::from_store(store, "default"));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
#[cfg(not(feature = "postgres"))]
|
||||||
|
_ => {}
|
||||||
#[cfg(all(feature = "postgres", not(feature = "libsql")))]
|
|
||||||
{
|
|
||||||
let _ = selected_backend;
|
|
||||||
if let Some(store) = self.create_postgres_secrets_store(&crypto).await? {
|
|
||||||
return Ok(SecretsContext::from_store(store, "default"));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(all(feature = "libsql", not(feature = "postgres")))]
|
|
||||||
{
|
|
||||||
let _ = selected_backend;
|
|
||||||
if let Some(store) = self.create_libsql_secrets_store(&crypto)? {
|
|
||||||
return Ok(SecretsContext::from_store(store, "default"));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Err(SetupError::Config(
|
Err(SetupError::Config(
|
||||||
|
|||||||
Reference in New Issue
Block a user