diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index 5f22beab7..9999da69b 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -11,7 +11,7 @@ use fabro_config::{ use fabro_static::EnvVars; use fabro_types::settings::cli::CliTargetSettings; use fabro_types::settings::server::LogDestination; -use fabro_types::settings::{CliNamespace, InterpString, RunNamespace}; +use fabro_types::settings::{InterpString, RunNamespace}; use fabro_types::{ServerSettings, UserSettings}; use fabro_util::error::SharedError; use fabro_util::version::FABRO_VERSION; @@ -220,21 +220,12 @@ fn value_at_path<'a>(document: &'a toml::Value, path: &[&str]) -> Option<&'a tom Some(current) } -/// Pull the resolved CLI target configuration out of `[cli.target]`. -/// Returns either an http(s) URL or a unix socket path. -fn cli_target_from_settings(settings: &CliNamespace) -> Option { - let target = settings.target.as_ref()?; - match target { - CliTargetSettings::Http { url } => Some(url.clone()), - CliTargetSettings::Unix { path } => Some(path.clone()), - } -} - fn configured_server_target(settings: &UserSettings) -> Result> { - let Some(value) = cli_target_from_settings(&settings.cli) else { - return Ok(None); - }; - parse_server_target(&value).map(Some) + match settings.cli.target.as_ref() { + Some(CliTargetSettings::Http { url }) => ServerTarget::http_url(url).map(Some), + Some(CliTargetSettings::Unix { path }) => ServerTarget::unix_socket_path(path).map(Some), + None => Ok(None), + } } pub(crate) fn default_server_target() -> ServerTarget { diff --git a/lib/crates/fabro-config/src/resolve/mod.rs b/lib/crates/fabro-config/src/resolve/mod.rs index fd7f1e01a..0432ca766 100644 --- a/lib/crates/fabro-config/src/resolve/mod.rs +++ b/lib/crates/fabro-config/src/resolve/mod.rs @@ -21,12 +21,7 @@ pub(crate) fn require_interp( path: &str, errors: &mut Vec, ) -> InterpString { - value.cloned().unwrap_or_else(|| { - errors.push(ResolveError::Missing { - path: path.to_string(), - }); - InterpString::parse("") - }) + require_value(value, path, errors, || InterpString::parse("")) } pub(crate) fn require_string( @@ -34,11 +29,20 @@ pub(crate) fn require_string( path: &str, errors: &mut Vec, ) -> String { + require_value(value, path, errors, String::new) +} + +fn require_value( + value: Option<&T>, + path: &str, + errors: &mut Vec, + missing: impl FnOnce() -> T, +) -> T { value.cloned().unwrap_or_else(|| { errors.push(ResolveError::Missing { path: path.to_string(), }); - String::new() + missing() }) } @@ -77,13 +81,20 @@ pub(crate) fn default_interp(path: impl AsRef) -> InterpString /// String pass itself is retired in a later slice. Unclaimed `{{ ... }}` text /// (jq programs, Go templates) never interpolated and does not warn. pub(crate) fn warn_if_demoted_template(field: &str, value: Option<&str>) { - if value.is_some_and(|value| !InterpString::parse(value).is_literal()) { - tracing::warn!( - field = %field, - "this field no longer interpolates template tokens and uses the value literally; it \ - was demoted to a plain string in the interpolation unification" - ); + let Some(value) = value else { + return; + }; + if !tracing::enabled!(tracing::Level::WARN) || !value.contains("{{") { + return; } + if InterpString::parse(value).is_literal() { + return; + } + tracing::warn!( + field = %field, + "this field no longer interpolates template tokens and uses the value literally; it \ + was demoted to a plain string in the interpolation unification" + ); } #[cfg(test)] diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index 9c9786706..d6ea6252a 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -527,7 +527,6 @@ async fn build_preflight_report( &mut checks, graph, &resolved_run, - &configured_providers, catalog.as_ref(), llm_result, ) @@ -987,12 +986,16 @@ async fn run_llm_check( checks: &mut Vec, graph: &Graph, settings: &RunNamespace, - configured_providers: &[ProviderId], catalog: &Catalog, llm_result: Result, ) -> bool { - let (model, provider) = resolve_model_provider(settings, graph, configured_providers, catalog); - let default_provider = provider.as_deref().unwrap_or("anthropic"); + let model = settings + .model + .name + .as_deref() + .unwrap_or_else(|| catalog.default_for_configured_ids(&[]).id.as_str()); + let provider = settings.model.provider.as_deref(); + let default_provider = provider.unwrap_or("anthropic"); let mut model_providers = std::collections::BTreeSet::new(); let mut has_llm_nodes = false; @@ -1001,7 +1004,7 @@ async fn run_llm_check( continue; } has_llm_nodes = true; - let node_model = node.model().unwrap_or(&model); + let node_model = node.model().unwrap_or(model); let node_provider = node.provider().unwrap_or(default_provider); let (resolved_model, resolved_provider) = if let Some(info) = catalog.get(node_model) { (info.id.clone(), info.provider.to_string()) @@ -1142,29 +1145,6 @@ fn canonical_provider_id(catalog: &Catalog, provider_name: &str) -> ProviderId { .map_or(provider_id, |provider| provider.id.clone()) } -fn resolve_model_provider( - settings: &RunNamespace, - _graph: &Graph, - configured_providers: &[ProviderId], - catalog: &Catalog, -) -> (String, Option) { - let provider = settings.model.provider.clone(); - let model = settings.model.name.clone().unwrap_or_else(|| { - catalog - .default_for_configured_ids(configured_providers) - .id - .clone() - }); - - match catalog.get(&model) { - Some(info) => ( - info.id.clone(), - provider.or(Some(info.provider.to_string())), - ), - None => (model, provider), - } -} - async fn run_github_token_check( checks: &mut Vec, prepared: &PreparedManifest, diff --git a/lib/crates/fabro-types/src/settings/interp.rs b/lib/crates/fabro-types/src/settings/interp.rs index 664ec583c..e5b00dc06 100644 --- a/lib/crates/fabro-types/src/settings/interp.rs +++ b/lib/crates/fabro-types/src/settings/interp.rs @@ -15,6 +15,7 @@ //! the value, via [`InterpString::resolve_with`]. Provenance tracking lets //! outward-facing renderers redact env- and secret-sourced values uniformly. +use std::borrow::Cow; use std::fmt; use serde::de::{self, Visitor}; @@ -388,19 +389,29 @@ impl InterpString { /// [`InterpString::substitute_variables`] for settings fields stored as /// `String`; it keeps the raw-source round-trip in one audited place. pub fn substitute_variables_in_str(value: &str, lookup: F) -> Result + where + F: FnMut(&str) -> Option, + { + Self::substitute_variables_in_str_cow(value, lookup).map(Cow::into_owned) + } + + pub(crate) fn substitute_variables_in_str_cow( + value: &str, + lookup: F, + ) -> Result, ResolveError> where F: FnMut(&str) -> Option, { let parsed = Self::parse(value); if !parsed.references(Namespace::Vars) { - return Ok(value.to_owned()); + return Ok(Cow::Borrowed(value)); } #[expect( clippy::disallowed_methods, reason = "canonical raw-source round-trip for String-typed settings fields whose \ remaining tokens resolve downstream" )] - Ok(parsed.substitute_variables(lookup)?.as_source()) + Ok(Cow::Owned(parsed.substitute_variables(lookup)?.as_source())) } } diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index cd5e30949..d25bc61a5 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -178,7 +178,11 @@ where if !may_reference_variable(value) { return Ok(()); } - *value = InterpString::substitute_variables_in_str(value, lookup)?; + if let std::borrow::Cow::Owned(substituted) = + InterpString::substitute_variables_in_str_cow(value, lookup)? + { + *value = substituted; + } Ok(()) }