From 48c5737bd5fd4872c6d88b08094196230c2348e5 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 10 Apr 2026 07:10:32 -0400 Subject: [PATCH] refactor(settings): resolve project settings --- lib/crates/fabro-cli/tests/it/cmd/config.rs | 6 ++- lib/crates/fabro-config/src/lib.rs | 4 +- lib/crates/fabro-config/src/project.rs | 40 ++++++----------- lib/crates/fabro-config/src/resolve/mod.rs | 19 +++++++- .../fabro-config/src/resolve/project.rs | 17 ++++++++ .../fabro-config/tests/resolve_project.rs | 43 +++++++++++++++++++ .../fabro-types/src/settings/accessors.rs | 13 ------ lib/crates/fabro-types/src/settings/mod.rs | 2 +- .../fabro-types/src/settings/project.rs | 9 ++++ 9 files changed, 109 insertions(+), 44 deletions(-) create mode 100644 lib/crates/fabro-config/src/resolve/project.rs create mode 100644 lib/crates/fabro-config/tests/resolve_project.rs diff --git a/lib/crates/fabro-cli/tests/it/cmd/config.rs b/lib/crates/fabro-cli/tests/it/cmd/config.rs index 847b8e2c5..4be3cfc92 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/config.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/config.rs @@ -39,6 +39,10 @@ fn resolve_cli(settings: &SettingsFile) -> fabro_types::settings::CliSettings { fabro_config::resolve_cli_from_file(settings).expect("cli settings should resolve") } +fn resolve_project(settings: &SettingsFile) -> fabro_types::settings::ProjectSettings { + fabro_config::resolve_project_from_file(settings).expect("project settings should resolve") +} + fn server_settings_fixture() -> SettingsFile { ConfigLayer::parse( r#" @@ -299,7 +303,7 @@ fn settings_local_merges_cli_and_project_defaults() { assert_eq!(cfg.run_model_name_str().as_deref(), Some("project-model")); assert_eq!(cfg.run_model_provider_str().as_deref(), Some("openai")); assert_eq!(cfg.run_goal_inline_str().as_deref(), None); - assert_eq!(cfg.project_directory(), Some("fabro")); + assert_eq!(resolve_project(&cfg).directory, "fabro"); // v2 R22: run.inputs replaces the inherited map wholesale rather than // merging by key, so the project layer wipes out the CLI layer's inputs. diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index c9987da8c..af96fa58d 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -15,8 +15,8 @@ pub use config::ConfigLayer; pub use fabro_util::path::expand_tilde; pub use home::Home; pub use resolve::{ - ResolveError, resolve_cli, resolve_cli_from_file, resolve_run, resolve_run_from_file, - resolve_server, resolve_server_from_file, + ResolveError, resolve_cli, resolve_cli_from_file, resolve_project, resolve_project_from_file, + resolve_run, resolve_run_from_file, resolve_server, resolve_server_from_file, }; pub use storage::{RunScratch, ServerState, Storage}; diff --git a/lib/crates/fabro-config/src/project.rs b/lib/crates/fabro-config/src/project.rs index 7a3c1149c..b0aeb5be4 100644 --- a/lib/crates/fabro-config/src/project.rs +++ b/lib/crates/fabro-config/src/project.rs @@ -12,12 +12,11 @@ use serde::Serialize; use crate::config::ConfigLayer; use crate::run; -use fabro_types::settings::{InterpString, SettingsFile}; +use crate::{resolve_project_from_file, resolve_run_from_file}; +use fabro_types::settings::SettingsFile; const CONFIG_FILENAME: &str = "fabro.toml"; const RUN_GRAPH_FILE: &str = "workflow.fabro"; -const DEFAULT_FABRO_DIRECTORY: &str = "fabro/"; - #[derive(Clone, Debug)] pub struct WorkflowPathResolution { pub resolved_workflow_path: PathBuf, @@ -38,12 +37,9 @@ pub fn parse_project_config(content: &str) -> anyhow::Result { /// paths are anchored at the directory of `path` at load time. pub fn load_project_config(path: &Path) -> anyhow::Result { let config = ConfigLayer::load(path).context("Failed to parse project config")?; - let root = config - .as_v2() - .project - .as_ref() - .and_then(|p| p.directory.as_deref()) - .unwrap_or(DEFAULT_FABRO_DIRECTORY); + let root = resolve_project_from_file(config.as_v2()) + .map_err(|errors| anyhow::anyhow!("Failed to resolve project settings: {errors:?}"))? + .directory; tracing::debug!(path = %path.display(), root = %root, "Loaded project config"); Ok(config) } @@ -125,11 +121,10 @@ pub fn resolve_workflow_path( } pub fn resolve_working_directory(settings: &SettingsFile, caller_cwd: &Path) -> PathBuf { - let Some(work_dir) = settings - .run - .as_ref() - .and_then(|run| run.working_dir.as_ref()) - .map(InterpString::as_source) + let Some(work_dir) = resolve_run_from_file(settings) + .ok() + .and_then(|settings| settings.working_dir) + .map(|value| value.as_source()) else { return caller_cwd.to_path_buf(); }; @@ -374,12 +369,9 @@ pub fn resolve_fabro_root(config_path: &Path, config: &ConfigLayer) -> PathBuf { let project_dir = config_path .parent() .expect("config_path should have a parent directory"); - let root = config - .as_v2() - .project - .as_ref() - .and_then(|p| p.directory.as_deref()) - .unwrap_or(DEFAULT_FABRO_DIRECTORY); + let root = resolve_project_from_file(config.as_v2()) + .expect("project settings should resolve") + .directory; project_dir.join(root) } @@ -408,12 +400,8 @@ directory = "fabro/" ) .unwrap(); assert_eq!( - config - .as_v2() - .project - .as_ref() - .and_then(|p| p.directory.as_deref()), - Some("fabro/") + resolve_project_from_file(config.as_v2()).unwrap().directory, + "fabro/" ); } diff --git a/lib/crates/fabro-config/src/resolve/mod.rs b/lib/crates/fabro-config/src/resolve/mod.rs index ff952aaf4..2df385ff0 100644 --- a/lib/crates/fabro-config/src/resolve/mod.rs +++ b/lib/crates/fabro-config/src/resolve/mod.rs @@ -1,12 +1,16 @@ mod cli; mod error; +mod project; mod run; mod server; -use fabro_types::settings::{CliSettings, RunSettings, ServerSettings, SettingsFile}; +use fabro_types::settings::{ + CliSettings, ProjectSettings, RunSettings, ServerSettings, SettingsFile, +}; pub use cli::resolve_cli; pub use error::ResolveError; +pub use project::resolve_project; pub use run::resolve_run; pub use server::resolve_server; @@ -32,6 +36,19 @@ pub fn resolve_server_from_file(file: &SettingsFile) -> Result Result> { + let mut errors = Vec::new(); + let layer = file.project.as_ref().cloned().unwrap_or_default(); + let resolved = resolve_project(&layer, &mut errors); + if errors.is_empty() { + Ok(resolved) + } else { + Err(errors) + } +} + pub fn resolve_run_from_file(file: &SettingsFile) -> Result> { let mut errors = Vec::new(); let layer = file.run.as_ref().cloned().unwrap_or_default(); diff --git a/lib/crates/fabro-config/src/resolve/project.rs b/lib/crates/fabro-config/src/resolve/project.rs new file mode 100644 index 000000000..fad600cdd --- /dev/null +++ b/lib/crates/fabro-config/src/resolve/project.rs @@ -0,0 +1,17 @@ +use fabro_types::settings::project::{ProjectLayer, ProjectSettings}; + +use super::ResolveError; + +const DEFAULT_PROJECT_DIRECTORY: &str = "fabro/"; + +pub fn resolve_project(layer: &ProjectLayer, _errors: &mut Vec) -> ProjectSettings { + ProjectSettings { + name: layer.name.clone(), + description: layer.description.clone(), + directory: layer + .directory + .clone() + .unwrap_or_else(|| DEFAULT_PROJECT_DIRECTORY.to_string()), + metadata: layer.metadata.clone(), + } +} diff --git a/lib/crates/fabro-config/tests/resolve_project.rs b/lib/crates/fabro-config/tests/resolve_project.rs new file mode 100644 index 000000000..ccc4ff281 --- /dev/null +++ b/lib/crates/fabro-config/tests/resolve_project.rs @@ -0,0 +1,43 @@ +use fabro_config::resolve_project_from_file; +use fabro_types::settings::SettingsFile; + +#[test] +fn resolves_project_defaults_from_empty_settings() { + let settings = SettingsFile::default(); + + let project = resolve_project_from_file(&settings).expect("empty settings should resolve"); + + assert_eq!(project.directory, "fabro/"); + assert!(project.name.is_none()); + assert!(project.description.is_none()); + assert!(project.metadata.is_empty()); +} + +#[test] +fn resolves_project_directory_and_metadata() { + let settings: SettingsFile = fabro_config::ConfigLayer::parse( + r#" +_version = 1 + +[project] +name = "Acme" +description = "Automation" +directory = ".fabro" + +[project.metadata] +team = "platform" +"#, + ) + .expect("fixture should parse") + .into(); + + let project = resolve_project_from_file(&settings).expect("project settings should resolve"); + + assert_eq!(project.name.as_deref(), Some("Acme")); + assert_eq!(project.description.as_deref(), Some("Automation")); + assert_eq!(project.directory, ".fabro"); + assert_eq!( + project.metadata.get("team").map(String::as_str), + Some("platform") + ); +} diff --git a/lib/crates/fabro-types/src/settings/accessors.rs b/lib/crates/fabro-types/src/settings/accessors.rs index 4923edb51..42a604fe5 100644 --- a/lib/crates/fabro-types/src/settings/accessors.rs +++ b/lib/crates/fabro-types/src/settings/accessors.rs @@ -9,7 +9,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use super::interp::InterpString; -use super::project::ProjectLayer; use super::run::{ ApprovalMode, GitAuthorLayer, HookEntry, McpEntryLayer, ResolvedGoalSource, ResolvedRunGoal, RunAgentLayer, RunArtifactsLayer, RunCheckpointLayer, RunExecutionLayer, RunGoalLayer, @@ -23,18 +22,6 @@ use super::server::{ use super::tree::SettingsFile; impl SettingsFile { - // ---------- project-scope ---------- - - #[must_use] - pub fn project_layer(&self) -> Option<&ProjectLayer> { - self.project.as_ref() - } - - #[must_use] - pub fn project_directory(&self) -> Option<&str> { - self.project.as_ref().and_then(|p| p.directory.as_deref()) - } - // ---------- run-scope ---------- #[must_use] diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index c9c4aac3f..9a5977d3d 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -35,7 +35,7 @@ pub use interp::{InterpString, Provenance, ResolveEnvError, Resolved}; pub use model_ref::{ AmbiguousModelRef, ModelRef, ModelRegistry, ParseModelRefError, ResolvedModelRef, }; -pub use project::ProjectLayer; +pub use project::{ProjectLayer, ProjectSettings}; pub use run::{ ArtifactsSettings, DaytonaSettings, DaytonaSnapshotSettings, DockerfileSource, GitAuthorSettings, HookDefinition, HookType, InterviewProviderSettings, McpServerSettings, diff --git a/lib/crates/fabro-types/src/settings/project.rs b/lib/crates/fabro-types/src/settings/project.rs index 3355559d2..24cd31d62 100644 --- a/lib/crates/fabro-types/src/settings/project.rs +++ b/lib/crates/fabro-types/src/settings/project.rs @@ -7,6 +7,15 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; +/// A structurally resolved `[project]` view for consumers. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct ProjectSettings { + pub name: Option, + pub description: Option, + pub directory: String, + pub metadata: HashMap, +} + /// A sparse `[project]` layer as it appears in a single settings file. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)]