route cli settings loads through dense config

This commit is contained in:
Bryan Helmkamp 2026-04-23 17:09:21 -04:00
parent 96b904c24a
commit 83fc1602ea
No known key found for this signature in database
3 changed files with 185 additions and 146 deletions

View file

@ -2,9 +2,8 @@ use std::path::{Path, PathBuf};
use std::sync::Arc;
use anyhow::{Context as _, Result, bail};
use fabro_config::{RunSettingsBuilder, ServerSettingsBuilder, UserSettingsBuilder};
use fabro_types::settings::RunNamespace;
use fabro_types::settings::cli::{CliLayer, OutputFormat, OutputVerbosity};
use fabro_types::settings::{Combine, RunNamespace, SettingsLayer};
use fabro_types::{ServerSettings, UserSettings};
use fabro_util::printer::Printer;
use tokio::sync::OnceCell;
@ -13,6 +12,7 @@ use crate::args::{
ServerConnectionArgs, ServerTargetArgs, printer_from_verbosity, require_no_json_override,
};
use crate::server_client::Client;
use crate::user_config::LoadedSettings;
use crate::{server_client, user_config};
#[derive(Clone, Debug)]
@ -185,42 +185,28 @@ fn load_merged_settings(
cli_layer: &CliLayer,
server_mode: &ServerMode,
) -> Result<ResolvedCommandSettings> {
let disk_settings = match server_mode {
ServerMode::None | ServerMode::ByTarget { .. } => user_config::load_settings()?,
let loaded_settings = match server_mode {
ServerMode::None | ServerMode::ByTarget { .. } => {
user_config::load_resolved_settings(None, None, Some(cli_layer))?
}
ServerMode::ByStorageDir {
storage_dir_override,
..
} => user_config::load_settings_with_storage_dir(storage_dir_override.as_deref())?,
} => user_config::load_resolved_settings(
None,
storage_dir_override.as_deref(),
Some(cli_layer),
)?,
};
merge_settings_layer(disk_settings, cli_layer)
resolve_command_settings(loaded_settings)
}
fn merge_settings_layer(
disk_settings: SettingsLayer,
cli_layer: &CliLayer,
) -> Result<ResolvedCommandSettings> {
let storage_dir = crate::local_server::storage_dir(&disk_settings)?;
let run_settings = RunSettingsBuilder::from_layer(&disk_settings).map_err(|err| {
// Keep command context tolerant even when unrelated run defaults
// do not resolve cleanly.
err.to_string()
});
let server_settings = ServerSettingsBuilder::from_layer(&disk_settings).map_err(|err| {
// Keep storage-dir and CLI-target resolution tolerant even when full
// server resolution would reject a partial local settings file.
err.to_string()
});
let merged_settings = SettingsLayer {
cli: Some(cli_layer.clone()),
..SettingsLayer::default()
}
.combine(disk_settings);
let user_settings = UserSettingsBuilder::from_layer(&merged_settings)?;
fn resolve_command_settings(loaded_settings: LoadedSettings) -> Result<ResolvedCommandSettings> {
Ok(ResolvedCommandSettings {
storage_dir,
run_settings,
server_settings,
user_settings,
storage_dir: loaded_settings.storage_dir,
run_settings: loaded_settings.run_settings,
server_settings: loaded_settings.server_settings,
user_settings: loaded_settings.user_settings,
})
}
@ -228,14 +214,12 @@ fn merge_settings_layer(
mod tests {
use std::path::PathBuf;
use fabro_config::parse_settings_layer;
use fabro_types::settings::InterpString;
use fabro_types::settings::cli::{CliLayer, CliOutputLayer, OutputFormat, OutputVerbosity};
use fabro_types::settings::server::{ServerLayer, ServerStorageLayer};
use fabro_util::printer::Printer;
use tokio::sync::OnceCell;
use super::{CommandContext, ServerMode, merge_settings_layer};
use super::{CommandContext, ServerMode, resolve_command_settings};
use crate::user_config;
fn cli_layer_with_json_and_verbose() -> CliLayer {
CliLayer {
@ -249,9 +233,11 @@ mod tests {
fn synthetic_context(process_local_json: bool, printer: Printer) -> CommandContext {
let cli_layer = cli_layer_with_json_and_verbose();
let resolved_settings =
merge_settings_layer(parse_settings_layer("_version = 1\n").unwrap(), &cli_layer)
.expect("settings should merge");
let resolved_settings = resolve_command_settings(
user_config::load_resolved_settings_from_toml("_version = 1\n", None, Some(&cli_layer))
.expect("settings should resolve"),
)
.expect("settings should merge");
CommandContext {
printer,
process_local_json,
@ -267,18 +253,6 @@ mod tests {
}
}
fn with_storage_dir_override(
mut layer: fabro_types::settings::SettingsLayer,
path: &std::path::Path,
) -> fabro_types::settings::SettingsLayer {
let server = layer.server.get_or_insert_with(ServerLayer::default);
let storage = server
.storage
.get_or_insert_with(ServerStorageLayer::default);
storage.root = Some(InterpString::parse(&path.display().to_string()));
layer
}
#[test]
fn context_exposes_resolved_output_and_explicit_json_state() {
let ctx = synthetic_context(true, Printer::Default);
@ -295,24 +269,34 @@ mod tests {
#[test]
fn storage_dir_override_only_changes_storage_root_in_merged_settings() {
let cli_layer = cli_layer_with_json_and_verbose();
let base_disk_settings = parse_settings_layer(
r#"
let base_settings = resolve_command_settings(
user_config::load_resolved_settings_from_toml(
r#"
_version = 1
[server.storage]
root = "/srv/fabro/default"
"#,
None,
Some(&cli_layer),
)
.expect("base settings should resolve"),
)
.expect("settings fixture should parse");
let override_disk_settings = with_storage_dir_override(
base_disk_settings.clone(),
std::path::Path::new("/srv/fabro/override"),
);
.expect("base settings should merge");
let connection_settings = resolve_command_settings(
user_config::load_resolved_settings_from_toml(
r#"
_version = 1
let base_settings = merge_settings_layer(base_disk_settings, &cli_layer)
.expect("base settings should merge");
let connection_settings = merge_settings_layer(override_disk_settings, &cli_layer)
.expect("connection settings should merge");
[server.storage]
root = "/srv/fabro/default"
"#,
Some(std::path::Path::new("/srv/fabro/override")),
Some(&cli_layer),
)
.expect("connection settings should resolve"),
)
.expect("connection settings should merge");
assert_eq!(
base_settings.user_settings,
@ -341,17 +325,18 @@ root = "/srv/fabro/default"
#[test]
fn storage_dir_stays_available_when_server_settings_do_not_resolve() {
let resolved = merge_settings_layer(
parse_settings_layer(
let resolved = resolve_command_settings(
user_config::load_resolved_settings_from_toml(
r#"
_version = 1
[server.storage]
root = "/srv/fabro"
"#,
None,
Some(&CliLayer::default()),
)
.expect("settings fixture should parse"),
&CliLayer::default(),
.expect("settings should resolve"),
)
.expect("settings should merge");
@ -362,8 +347,8 @@ root = "/srv/fabro"
#[test]
fn run_settings_include_run_agent_mcps() {
let resolved = merge_settings_layer(
parse_settings_layer(
let resolved = resolve_command_settings(
user_config::load_resolved_settings_from_toml(
r#"
_version = 1
@ -371,9 +356,10 @@ _version = 1
type = "stdio"
command = ["demo-mcp"]
"#,
None,
Some(&CliLayer::default()),
)
.expect("settings fixture should parse"),
&CliLayer::default(),
.expect("settings should resolve"),
)
.expect("settings should merge");

View file

@ -1,14 +1,9 @@
//! Helpers for CLI code that manages the local Fabro server on this host.
//!
//! This module is the only generic CLI lifecycle surface allowed to read
//! `[server.*]` settings. User-facing CLI commands outside same-host server
//! lifecycle should not call into it.
use std::path::{Path, PathBuf};
use anyhow::Result;
use fabro_config::bind::BindRequest;
use fabro_config::{ServerSettingsBuilder, parse_settings_layer};
use fabro_types::ServerSettings;
use fabro_types::settings::{ServerAuthMethod, SettingsLayer};
@ -23,34 +18,27 @@ pub(crate) struct LocalServerConfig {
impl LocalServerConfig {
pub(crate) fn load(config_path: Option<&Path>, storage_dir: Option<&Path>) -> Result<Self> {
let settings =
user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?;
Self::from_layer(&settings)
let settings = user_config::load_resolved_settings(config_path, storage_dir, None)?;
Ok(Self::from_loaded_settings(settings))
}
pub(crate) fn load_with_storage_dir(storage_dir: Option<&Path>) -> Result<Self> {
let settings = user_config::load_settings_with_storage_dir(storage_dir)?;
Self::from_layer(&settings)
let settings = user_config::load_resolved_settings(None, storage_dir, None)?;
Ok(Self::from_loaded_settings(settings))
}
fn from_layer(settings: &SettingsLayer) -> Result<Self> {
let storage_dir = storage_dir(settings)?;
let config_log_level = settings
.server
.as_ref()
.and_then(|server| server.logging.as_ref())
.and_then(|logging| logging.level.clone());
let server_settings = resolved_server_settings(settings).map_err(|err| err.to_string());
fn from_loaded_settings(settings: user_config::LoadedSettings) -> Self {
let server_settings = settings.server_settings;
let auth_methods = server_settings
.as_ref()
.map(|resolved| resolved.server.auth.methods.clone())
.unwrap_or_default();
Ok(Self {
storage_dir,
Self {
storage_dir: settings.storage_dir,
auth_methods,
config_log_level,
config_log_level: settings.config_log_level,
server_settings,
})
}
}
pub(crate) fn storage_dir(&self) -> &Path {
@ -75,20 +63,16 @@ impl LocalServerConfig {
}
pub(crate) fn storage_dir_from_toml(source: &str) -> Result<PathBuf> {
let settings = parse_settings_layer(source)
.map_err(|err| anyhow::anyhow!("failed to parse settings file: {err}"))?;
storage_dir(&settings)
storage_dir_from_toml_with_lookup(source, &|name| std::env::var(name).ok())
}
pub(crate) fn storage_dir(settings: &SettingsLayer) -> Result<PathBuf> {
storage_dir_with_lookup(settings, &|name| std::env::var(name).ok())
}
pub(crate) fn storage_dir_with_lookup(
settings: &SettingsLayer,
fn storage_dir_from_toml_with_lookup(
source: &str,
lookup: &dyn Fn(&str) -> Option<String>,
) -> Result<PathBuf> {
let storage_root = settings
let layer: SettingsLayer = toml::from_str(source)
.map_err(|err| anyhow::anyhow!("failed to parse settings file: {err}"))?;
let storage_root = layer
.server
.as_ref()
.and_then(|server| server.storage.as_ref())
@ -104,15 +88,11 @@ pub(crate) fn storage_dir_with_lookup(
Ok(PathBuf::from(resolved_root.value))
}
fn resolved_server_settings(settings: &SettingsLayer) -> Result<ServerSettings> {
ServerSettingsBuilder::from_layer(settings).map_err(Into::into)
}
#[cfg(test)]
mod tests {
use std::path::PathBuf;
use super::storage_dir_from_toml;
use super::{storage_dir_from_toml, storage_dir_from_toml_with_lookup};
#[test]
fn storage_dir_from_toml_reads_explicit_root_without_full_server_resolution() {
@ -135,4 +115,20 @@ root = "/srv/fabro"
assert_eq!(path, fabro_config::user::default_storage_dir());
}
#[test]
fn storage_dir_from_toml_resolves_env_interpolation() {
let path = storage_dir_from_toml_with_lookup(
r#"
_version = 1
[server.storage]
root = "{{ env.FABRO_STORAGE_ROOT }}"
"#,
&|name| (name == "FABRO_STORAGE_ROOT").then_some("/srv/fabro".to_string()),
)
.expect("storage root should resolve");
assert_eq!(path, PathBuf::from("/srv/fabro"));
}
}

View file

@ -1,52 +1,101 @@
use std::path::Path;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use anyhow::Result;
pub(crate) use fabro_client::ServerTarget;
pub(crate) use fabro_config::user::{active_settings_path, default_storage_dir};
use fabro_config::user::{default_socket_path, load_settings_config};
use fabro_types::UserSettings;
use fabro_types::settings::cli::CliTargetSettings;
use fabro_types::settings::{CliNamespace, SettingsLayer};
use fabro_config::{RunSettingsBuilder, ServerSettingsBuilder, UserSettingsBuilder};
use fabro_types::settings::cli::{CliLayer, CliTargetSettings};
use fabro_types::settings::{CliNamespace, Combine, RunNamespace, SettingsLayer};
use fabro_types::{ServerSettings, UserSettings};
use fabro_util::version::FABRO_VERSION;
use tracing::debug;
use crate::args::ServerTargetArgs;
pub(crate) fn load_settings() -> anyhow::Result<SettingsLayer> {
load_settings_with_config_and_storage_dir(None, None)
pub(crate) struct LoadedSettings {
pub(crate) storage_dir: PathBuf,
pub(crate) config_log_level: Option<String>,
pub(crate) run_settings: std::result::Result<RunNamespace, String>,
pub(crate) server_settings: std::result::Result<ServerSettings, String>,
pub(crate) user_settings: UserSettings,
}
pub(crate) fn load_settings_with_storage_dir(
storage_dir: Option<&Path>,
) -> anyhow::Result<SettingsLayer> {
load_settings_with_config_and_storage_dir(None, storage_dir)
}
pub(crate) fn load_settings_with_config_and_storage_dir(
pub(crate) fn load_resolved_settings(
config_path: Option<&Path>,
storage_dir: Option<&Path>,
) -> anyhow::Result<SettingsLayer> {
cli_layer: Option<&CliLayer>,
) -> anyhow::Result<LoadedSettings> {
let layer = load_settings_config(config_path)?;
Ok(apply_storage_dir_override(layer, storage_dir))
resolve_loaded_settings(layer, storage_dir, cli_layer)
}
fn apply_storage_dir_override(
mut layer: SettingsLayer,
fn resolve_loaded_settings(
layer: SettingsLayer,
storage_dir: Option<&Path>,
) -> SettingsLayer {
use fabro_types::settings::InterpString;
use fabro_types::settings::server::{ServerLayer, ServerStorageLayer};
cli_layer: Option<&CliLayer>,
) -> anyhow::Result<LoadedSettings> {
let storage_override = storage_dir.map(Path::to_path_buf);
let storage_dir = storage_dir_from_layer(&layer, storage_dir)?;
let config_log_level = layer
.server
.as_ref()
.and_then(|server| server.logging.as_ref())
.and_then(|logging| logging.level.clone());
let run_settings = RunSettingsBuilder::from_layer(&layer).map_err(|err| err.to_string());
let server_settings = ServerSettingsBuilder::from_layer(&layer)
.map(|settings| match storage_override.as_deref() {
Some(dir) => settings.with_storage_override(dir),
None => settings,
})
.map_err(|err| err.to_string());
let user_settings_layer = if let Some(cli_layer) = cli_layer {
SettingsLayer {
cli: Some(cli_layer.clone()),
..SettingsLayer::default()
}
.combine(layer.clone())
} else {
layer.clone()
};
let user_settings = UserSettingsBuilder::from_layer(&user_settings_layer)?;
Ok(LoadedSettings {
storage_dir,
config_log_level,
run_settings,
server_settings,
user_settings,
})
}
fn storage_dir_from_layer(
layer: &SettingsLayer,
storage_dir: Option<&Path>,
) -> anyhow::Result<PathBuf> {
storage_dir_from_layer_with_lookup(layer, storage_dir, &|name| std::env::var(name).ok())
}
fn storage_dir_from_layer_with_lookup(
layer: &SettingsLayer,
storage_dir: Option<&Path>,
lookup: &dyn Fn(&str) -> Option<String>,
) -> anyhow::Result<PathBuf> {
if let Some(dir) = storage_dir {
let server = layer.server.get_or_insert_with(ServerLayer::default);
let storage = server
.storage
.get_or_insert_with(ServerStorageLayer::default);
storage.root = Some(InterpString::parse(&dir.display().to_string()));
return Ok(dir.to_path_buf());
}
layer
let storage_root = layer
.server
.as_ref()
.and_then(|server| server.storage.as_ref())
.and_then(|storage| storage.root.clone())
.unwrap_or_else(|| {
fabro_types::settings::InterpString::parse(&default_storage_dir().to_string_lossy())
});
let resolved_root = storage_root.resolve(lookup)?;
Ok(PathBuf::from(resolved_root.value))
}
/// Pull the resolved CLI target configuration out of `[cli.target]`.
@ -102,17 +151,27 @@ pub(crate) fn cli_http_client_builder() -> fabro_http::HttpClientBuilder {
fabro_http::HttpClientBuilder::new().user_agent(format!("fabro-cli/{FABRO_VERSION}"))
}
#[cfg(test)]
pub(crate) fn load_resolved_settings_from_toml(
source: &str,
storage_dir: Option<&Path>,
cli_layer: Option<&CliLayer>,
) -> anyhow::Result<LoadedSettings> {
let layer: SettingsLayer = toml::from_str(source)
.map_err(|err| anyhow::anyhow!("failed to parse settings file: {err}"))?;
resolve_loaded_settings(layer, storage_dir, cli_layer)
}
#[cfg(test)]
mod tests {
use std::path::PathBuf;
use fabro_config::UserSettingsBuilder;
use fabro_config::user::default_storage_dir;
use fabro_config::{UserSettingsBuilder, parse_settings_layer};
use fabro_types::UserSettings;
use super::*;
use crate::args::ServerTargetArgs;
use crate::local_server;
fn server_target_args(value: Option<&str>) -> ServerTargetArgs {
ServerTargetArgs {
@ -124,10 +183,6 @@ mod tests {
UserSettingsBuilder::from_toml(source).expect("fixture should resolve")
}
fn parse_layer(source: &str) -> SettingsLayer {
parse_settings_layer(source).expect("fixture should parse")
}
#[test]
fn exec_has_no_server_target_by_default() {
assert_eq!(exec_server_target(&server_target_args(None)).unwrap(), None);
@ -234,45 +289,47 @@ url = "https://config.example.com"
#[test]
fn storage_dir_defaults_without_server_auth_methods() {
let settings = SettingsLayer::default();
let layer = SettingsLayer::default();
assert_eq!(
local_server::storage_dir(&settings).unwrap(),
storage_dir_from_layer(&layer, None).unwrap(),
default_storage_dir()
);
}
#[test]
fn storage_dir_uses_explicit_server_storage_root() {
let settings = parse_layer(
let layer: SettingsLayer = toml::from_str(
r#"
_version = 1
[server.storage]
root = "/srv/fabro"
"#,
);
)
.expect("fixture should parse");
assert_eq!(
local_server::storage_dir(&settings).unwrap(),
storage_dir_from_layer(&layer, None).unwrap(),
PathBuf::from("/srv/fabro")
);
}
#[test]
fn storage_dir_resolves_env_interpolated_root() {
let settings = parse_layer(
let layer: SettingsLayer = toml::from_str(
r#"
_version = 1
[server.storage]
root = "{{ env.FABRO_STORAGE_ROOT }}"
"#,
);
)
.expect("fixture should parse");
let temp = tempfile::tempdir().unwrap();
assert_eq!(
local_server::storage_dir_with_lookup(&settings, &|name| {
storage_dir_from_layer_with_lookup(&layer, None, &|name| {
(name == "FABRO_STORAGE_ROOT").then(|| temp.path().display().to_string())
})
.unwrap(),