route fabro-config parsing through settings fromstr

This commit is contained in:
Bryan Helmkamp 2026-04-23 19:20:02 -04:00
parent e17bd789dd
commit 941c6e83f9
No known key found for this signature in database
11 changed files with 62 additions and 56 deletions

View file

@ -6,7 +6,6 @@ use fabro_types::{ServerSettings, UserSettings, WorkflowSettings};
use crate::defaults::DEFAULTS_LAYER;
use crate::load::load_settings_path;
use crate::parse::parse_settings_layer;
use crate::resolve::{
ResolveError, resolve_cli, resolve_features, resolve_project, resolve_run, resolve_server,
resolve_workflow,
@ -73,7 +72,8 @@ impl ServerSettingsBuilder {
}
pub fn from_toml(source: &str) -> Result<ServerSettings> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
Self::from_layer(&layer)
}
@ -115,13 +115,15 @@ impl UserSettingsBuilder {
}
pub fn from_toml(source: &str) -> Result<UserSettings> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
Self::from_layer(&layer)
}
pub fn from_toml_with_cli_overrides(source: &str, cli: &CliLayer) -> Result<UserSettings> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
Self::from_layer_with_cli_overrides(&layer, cli)
}
@ -166,7 +168,8 @@ impl RunSettingsBuilder {
}
pub fn from_toml(source: &str) -> Result<RunNamespace> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
Self::from_layer(&layer)
}
@ -211,7 +214,8 @@ pub fn server_runtime_settings_from_toml(
run_overrides: Option<RunLayer>,
server_overrides: Option<ServerLayer>,
) -> Result<ServerRuntimeSettings> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
resolve_server_runtime_settings(layer, run_overrides, server_overrides)
}
@ -261,7 +265,8 @@ impl WorkflowSettingsBuilder {
}
pub fn from_toml(source: &str) -> Result<WorkflowSettings> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
Self::from_layer(&layer)
.map_err(|errors| Error::resolve("failed to resolve workflow settings", errors.into()))
@ -288,7 +293,8 @@ impl WorkflowSettingsBuilder {
}
pub fn workflow_toml(self, source: &str) -> Result<Self> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
Ok(self.workflow_layer(layer))
}
@ -304,7 +310,8 @@ impl WorkflowSettingsBuilder {
}
pub fn project_toml(self, source: &str) -> Result<Self> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
Ok(self.project_layer(layer))
}
@ -320,7 +327,8 @@ impl WorkflowSettingsBuilder {
}
pub fn user_toml(self, source: &str) -> Result<Self> {
let layer = parse_settings_layer(source)
let layer = source
.parse::<SettingsLayer>()
.map_err(|err| Error::parse("Failed to parse settings file", err))?;
Ok(self.user_layer(layer))
}

View file

@ -1,8 +1,9 @@
use std::sync::LazyLock;
use crate::{SettingsLayer, parse_settings_layer};
use crate::SettingsLayer;
pub(crate) static DEFAULTS_LAYER: LazyLock<SettingsLayer> = LazyLock::new(|| {
parse_settings_layer(include_str!("defaults.toml"))
include_str!("defaults.toml")
.parse::<SettingsLayer>()
.expect("embedded defaults.toml must parse as a valid SettingsLayer")
});

View file

@ -9,6 +9,8 @@ use std::str::FromStr;
use serde::{Deserialize, Serialize};
use crate::parse::{ParseError, parse_settings};
use super::cli::CliLayer;
use super::features::FeaturesLayer;
use super::project::ProjectLayer;
@ -36,10 +38,10 @@ pub(crate) struct SettingsLayer {
}
impl FromStr for SettingsLayer {
type Err = toml::de::Error;
type Err = ParseError;
fn from_str(source: &str) -> Result<Self, Self::Err> {
toml::from_str(source)
parse_settings(source)
}
}
@ -103,7 +105,7 @@ impl SettingsLayer {
/// with `["dev-token"]`. Use anywhere a test needs a starter
/// `SettingsLayer` that the strict resolver will accept.
#[must_use]
pub fn test_default() -> Self {
pub(crate) fn test_default() -> Self {
let mut layer = Self::default();
layer.ensure_test_auth_methods();
layer
@ -112,7 +114,7 @@ impl SettingsLayer {
/// If `server.auth.methods` is unset, populate it with `["dev-token"]`.
/// Existing methods (set by a fixture) are preserved. Use to make a
/// parsed-from-TOML layer resolve cleanly without overriding test intent.
pub fn ensure_test_auth_methods(&mut self) {
pub(crate) fn ensure_test_auth_methods(&mut self) {
use fabro_types::settings::ServerAuthMethod;
use super::server::{ServerAuthLayer, ServerLayer as ServerLayerTy};

View file

@ -52,7 +52,6 @@ pub use layers::{
};
pub(crate) use layers::{Combine, SettingsLayer};
pub use parse::ParseError;
pub(crate) use parse::parse_settings_layer;
pub use resolve::{
ResolveError, resolve_cli, resolve_features, resolve_project, resolve_run, resolve_server,
resolve_workflow,

View file

@ -7,12 +7,12 @@ use std::path::{Path, PathBuf};
use fabro_types::settings::InterpString;
use crate::parse::parse_settings_layer;
use crate::{Error, Result, RunGoalLayer, SettingsLayer};
pub(crate) fn load_settings_path(path: &Path) -> Result<SettingsLayer> {
let content = std::fs::read_to_string(path).map_err(|source| Error::read_file(path, source))?;
let mut layer = parse_settings_layer(&content)
let mut layer = content
.parse::<SettingsLayer>()
.map_err(|err| Error::parse_file("Failed to parse settings file", path, err))?;
let base_dir = path.parent().unwrap_or_else(|| Path::new("."));
resolve_goal_file_paths(&mut layer, base_dir);

View file

@ -58,7 +58,7 @@ impl fmt::Display for VersionError {
impl std::error::Error for VersionError {}
pub(crate) fn parse_settings_layer(input: &str) -> Result<SettingsLayer, ParseError> {
pub(crate) fn parse_settings(input: &str) -> Result<SettingsLayer, ParseError> {
let raw: toml::Value = toml::from_str(input).map_err(|e| ParseError::Toml(e.to_string()))?;
validate_version(&raw).map_err(ParseError::Version)?;
@ -135,19 +135,19 @@ mod tests {
#[test]
fn parses_empty_file() {
let file = parse_settings_layer("").unwrap();
let file = "".parse::<SettingsLayer>().unwrap();
assert_eq!(file, SettingsLayer::default());
}
#[test]
fn parses_minimal_valid_file() {
let file = parse_settings_layer("_version = 1\n").unwrap();
let file = "_version = 1\n".parse::<SettingsLayer>().unwrap();
assert_eq!(file.version, Some(1));
}
#[test]
fn rejects_legacy_version_key_with_rename_hint() {
let err = parse_settings_layer("version = 1").unwrap_err();
let err = "version = 1".parse::<SettingsLayer>().unwrap_err();
assert!(matches!(
err,
ParseError::Version(VersionError::LegacyVersionKey)
@ -157,13 +157,13 @@ mod tests {
#[test]
fn rejects_unknown_top_level_key() {
let err = parse_settings_layer("unknown_key = 1").unwrap_err();
let err = "unknown_key = 1".parse::<SettingsLayer>().unwrap_err();
assert!(matches!(err, ParseError::UnknownTopLevelKey { .. }));
}
#[test]
fn higher_version_rejected_with_upgrade_hint() {
let err = parse_settings_layer("_version = 99").unwrap_err();
let err = "_version = 99".parse::<SettingsLayer>().unwrap_err();
assert!(err.to_string().contains("Upgrade"));
}
}

View file

@ -404,7 +404,7 @@ mod tests {
#[test]
fn parse_minimal_config() {
let config = crate::parse_settings_layer("_version = 1\n").unwrap();
let config = "_version = 1\n".parse::<crate::SettingsLayer>().unwrap();
assert_eq!(config.version, Some(1));
assert!(config.project.is_none());
}
@ -429,14 +429,13 @@ directory = "custom/"
#[test]
fn parse_with_run_execution_retros() {
let config = crate::parse_settings_layer(
"
let config = "
_version = 1
[run.execution]
retros = true
",
)
"
.parse::<crate::SettingsLayer>()
.unwrap();
assert_eq!(
config
@ -450,7 +449,8 @@ retros = true
#[test]
fn parse_rejects_legacy_llm_section() {
let err = crate::parse_settings_layer("_version = 1\n[llm]\nprovider = \"openai\"\n")
let err = "_version = 1\n[llm]\nprovider = \"openai\"\n"
.parse::<crate::SettingsLayer>()
.unwrap_err();
let text = format!("{err:#}");
assert!(
@ -461,7 +461,7 @@ retros = true
#[test]
fn parse_higher_version_errors() {
let err = crate::parse_settings_layer("_version = 2\n").unwrap_err();
let err = "_version = 2\n".parse::<crate::SettingsLayer>().unwrap_err();
let chain = format!("{err:#}");
assert!(
chain.contains("Upgrade") || chain.to_lowercase().contains("version"),

View file

@ -58,12 +58,11 @@ mod tests {
use fabro_types::settings::run::{HookType, McpTransport, TlsMode};
use crate::{WorkflowSettingsBuilder, parse_settings_layer};
use crate::{SettingsLayer, WorkflowSettingsBuilder};
#[test]
fn resolve_preserves_source_templates_for_mcp_and_hook_strings() {
let settings = parse_settings_layer(
r#"
let settings = r#"
_version = 1
[server.auth]
@ -98,8 +97,8 @@ url = "https://hooks.example.com"
[run.hooks.headers]
Authorization = "Bearer {{ env.HOOK_TOKEN }}"
"#,
)
"#
.parse::<SettingsLayer>()
.expect("settings fixture should parse");
let resolved = WorkflowSettingsBuilder::from_layer(&settings)

View file

@ -4,7 +4,7 @@ use fabro_types::settings::cli::{OutputFormat, OutputVerbosity};
use crate::{Combine, SettingsLayer, StringOrSplice};
fn parse(input: &str) -> SettingsLayer {
crate::parse_settings_layer(input).expect("fixture should parse")
input.parse::<SettingsLayer>().expect("fixture should parse")
}
#[test]

View file

@ -2,12 +2,10 @@ use fabro_types::settings::cli::OutputFormat;
use fabro_types::settings::run::{ApprovalMode, RunMode, WorktreeMode};
use fabro_types::settings::server::ObjectStoreProvider;
use crate::{
Combine, ServerSettingsBuilder, SettingsLayer, WorkflowSettingsBuilder, parse_settings_layer,
};
use crate::{Combine, ServerSettingsBuilder, SettingsLayer, WorkflowSettingsBuilder};
fn parse(source: &str) -> SettingsLayer {
parse_settings_layer(source).expect("fixture should parse")
source.parse::<SettingsLayer>().expect("fixture should parse")
}
fn embedded_defaults() -> SettingsLayer {

View file

@ -12,10 +12,10 @@ use temp_env::with_var;
use crate::resolve::dev_token_auth_enabled;
use crate::user::default_storage_dir;
use crate::{ServerSettingsBuilder, SettingsLayer, parse_settings_layer};
use crate::{ServerSettingsBuilder, SettingsLayer};
fn parse(source: &str) -> SettingsLayer {
let mut layer = parse_settings_layer(source).expect("fixture should parse");
let mut layer = source.parse::<SettingsLayer>().expect("fixture should parse");
layer.ensure_test_auth_methods();
layer
}
@ -37,7 +37,7 @@ fn resolve_errors(error: fabro_config::Error) -> Vec<fabro_config::ResolveError>
}
}
fn render_resolve_errors(error: fabro_config::Error) -> String {
fn render_resolve_error_lines(error: fabro_config::Error) -> String {
resolve_errors(error)
.into_iter()
.map(|error| error.to_string())
@ -154,8 +154,7 @@ session_sandboxes = true
#[test]
fn parsing_rejects_inbound_listener_tls_configuration() {
let err = parse_settings_layer(
r#"
let err = r#"
_version = 1
[server.listen]
@ -164,8 +163,8 @@ address = "127.0.0.1:32276"
[server.listen.tls]
cert = "/etc/fabro/server.pem"
"#,
)
"#
.parse::<SettingsLayer>()
.expect_err("listener TLS should be rejected at parse time");
assert!(err.to_string().contains("unknown field `tls`"));
@ -185,7 +184,7 @@ endpoint = "{{ env.S3_ENDPOINT }}"
"#,
);
let rendered = render_resolve_errors(
let rendered = render_resolve_error_lines(
ServerSettingsBuilder::from_layer(&file)
.expect_err("s3 config without bucket/region should fail"),
);
@ -391,7 +390,7 @@ strategy = "server_url"
"#,
);
let rendered = render_resolve_errors(
let rendered = render_resolve_error_lines(
ServerSettingsBuilder::from_layer(&file)
.expect_err("server_url webhook strategy should require server.api.url"),
);
@ -413,7 +412,7 @@ strategy = "tailscale_funnel"
"#,
);
let rendered = render_resolve_errors(ServerSettingsBuilder::from_layer(&file).expect_err(
let rendered = render_resolve_error_lines(ServerSettingsBuilder::from_layer(&file).expect_err(
"configured webhook strategy should require server.integrations.github.app_id",
));
@ -431,7 +430,7 @@ entries = ["10.0.0.0/33"]
"#,
);
let rendered = render_resolve_errors(
let rendered = render_resolve_error_lines(
ServerSettingsBuilder::from_layer(&file).expect_err("invalid CIDR should fail"),
);
@ -449,7 +448,7 @@ entries = ["github_meta_hooks"]
"#,
);
let rendered = render_resolve_errors(
let rendered = render_resolve_error_lines(
ServerSettingsBuilder::from_layer(&file)
.expect_err("github_meta_hooks should be rejected outside github webhooks"),
);
@ -472,7 +471,7 @@ entries = ["10.0.0.0/8"]
"#,
);
let rendered = render_resolve_errors(
let rendered = render_resolve_error_lines(
ServerSettingsBuilder::from_layer(&file)
.expect_err("unix allowlist without trusted proxies should fail"),
);
@ -495,7 +494,7 @@ entries = ["github_meta_hooks"]
"#,
);
let rendered = render_resolve_errors(
let rendered = render_resolve_error_lines(
ServerSettingsBuilder::from_layer(&file)
.expect_err("unix github webhook allowlist without trusted proxies should fail"),
);