mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 07:30:11 +00:00
fix(config): init_secrets no longer overwrites entire config (#726)
* fix(config): init_secrets no longer overwrites entire config init_secrets() was calling Config::from_db_with_toml() to re-resolve config after injecting credentials. This rebuilt the entire config from env/DB/defaults, nuking all other config fields (agent, safety, tools, etc.) even though only LlmConfig depends on injected credentials. This caused 5 CI test failures: the test rig's carefully chosen config values (max_tool_iterations, allow_local_tools, etc.) were silently overwritten with production defaults after secret injection. Fix: add Config::re_resolve_llm() that re-resolves only the LLM config after credential injection, leaving all other config fields untouched. Also fix TraceLlm::complete() to skip ToolCalls steps when called in force_text mode (iteration limit). [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(test): update test to match TraceLlm::complete() skip-tool-calls behavior [skip-regression-check] TraceLlm::complete() now skips ToolCalls steps (force_text mode) instead of erroring. Update the test to verify it skips past a ToolCalls step and returns the subsequent Text step. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Zaki <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
Zaki
parent
1c5117eded
commit
461d7712e8
+22
-21
@@ -255,15 +255,18 @@ impl AppBuilder {
|
||||
self.libsql_db.take();
|
||||
}
|
||||
|
||||
// Re-resolve config with OS credentials
|
||||
if let Some(ref db) = self.db {
|
||||
let toml_path = self.toml_path.as_deref();
|
||||
if let Ok(refreshed) =
|
||||
Config::from_db_with_toml(db.as_ref(), "default", toml_path).await
|
||||
{
|
||||
self.config = refreshed;
|
||||
tracing::debug!("LlmConfig re-resolved after OS credential injection");
|
||||
}
|
||||
// Re-resolve only the LLM config with OS credentials.
|
||||
let store: Option<&(dyn crate::db::SettingsStore + Sync)> =
|
||||
self.db.as_ref().map(|db| db.as_ref() as _);
|
||||
let toml_path = self.toml_path.as_deref();
|
||||
if let Err(e) = self
|
||||
.config
|
||||
.re_resolve_llm(store, "default", toml_path)
|
||||
.await
|
||||
{
|
||||
tracing::warn!(
|
||||
"Failed to re-resolve LLM config after OS credential injection: {e}"
|
||||
);
|
||||
}
|
||||
|
||||
return Ok(());
|
||||
@@ -308,18 +311,16 @@ impl AppBuilder {
|
||||
// Inject LLM API keys from encrypted storage
|
||||
crate::config::inject_llm_keys_from_secrets(secrets.as_ref(), "default").await;
|
||||
|
||||
// Re-resolve config with newly available keys
|
||||
if let Some(ref db) = self.db {
|
||||
let toml_path = self.toml_path.as_deref();
|
||||
match Config::from_db_with_toml(db.as_ref(), "default", toml_path).await {
|
||||
Ok(refreshed) => {
|
||||
self.config = refreshed;
|
||||
tracing::debug!("LlmConfig re-resolved after secret injection");
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::warn!("Failed to re-resolve config after secret injection: {}", e);
|
||||
}
|
||||
}
|
||||
// Re-resolve only the LLM config with newly available keys.
|
||||
let store: Option<&(dyn crate::db::SettingsStore + Sync)> =
|
||||
self.db.as_ref().map(|db| db.as_ref() as _);
|
||||
let toml_path = self.toml_path.as_deref();
|
||||
if let Err(e) = self
|
||||
.config
|
||||
.re_resolve_llm(store, "default", toml_path)
|
||||
.await
|
||||
{
|
||||
tracing::warn!("Failed to re-resolve LLM config after secret injection: {e}");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -257,6 +257,32 @@ impl Config {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Re-resolve only the LLM config after credential injection.
|
||||
///
|
||||
/// Called by `AppBuilder::init_secrets()` after injecting API keys into
|
||||
/// the env overlay. Only rebuilds `self.llm` — all other config fields
|
||||
/// are unaffected, preserving values from the initial config load (or
|
||||
/// from `Config::for_testing()` in test mode).
|
||||
pub async fn re_resolve_llm(
|
||||
&mut self,
|
||||
store: Option<&(dyn crate::db::SettingsStore + Sync)>,
|
||||
user_id: &str,
|
||||
toml_path: Option<&std::path::Path>,
|
||||
) -> Result<(), ConfigError> {
|
||||
let settings = if let Some(store) = store {
|
||||
let mut s = match store.get_all_settings(user_id).await {
|
||||
Ok(map) => Settings::from_db_map(&map),
|
||||
Err(_) => Settings::default(),
|
||||
};
|
||||
Self::apply_toml_overlay(&mut s, toml_path)?;
|
||||
s
|
||||
} else {
|
||||
Settings::default()
|
||||
};
|
||||
self.llm = LlmConfig::resolve(&settings)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Build config from settings (shared by from_env and from_db).
|
||||
async fn build(settings: &Settings) -> Result<Self, ConfigError> {
|
||||
Ok(Self {
|
||||
|
||||
Reference in New Issue
Block a user