diff --git a/lib/crates/fabro-cli/src/commands/config/mod.rs b/lib/crates/fabro-cli/src/commands/config/mod.rs index f81fd3c7b..753897407 100644 --- a/lib/crates/fabro-cli/src/commands/config/mod.rs +++ b/lib/crates/fabro-cli/src/commands/config/mod.rs @@ -5,9 +5,9 @@ use crate::args::{GlobalArgs, SettingsArgs}; use crate::command_context::CommandContext; use crate::shared::print_json_pretty; use crate::user_config; -use fabro_config::ConfigLayer; use fabro_config::effective_settings; use fabro_config::effective_settings::{EffectiveSettingsLayers, EffectiveSettingsMode}; +use fabro_config::load_settings_project; use fabro_config::project; use fabro_types::settings::SettingsFile; @@ -18,14 +18,14 @@ fn config_layers( let cwd = ctx.cwd(); let (workflow_layer, project_layer) = match workflow { Some(path) => workflow_and_project_layers(path, cwd)?, - None => (ConfigLayer::default(), ConfigLayer::project(cwd)?), + None => (SettingsFile::default(), load_settings_project(cwd)?), }; let user_layer = user_config::settings_layer_with_config_and_storage_dir( Some(ctx.base_config_path()), None, )?; Ok(EffectiveSettingsLayers::new( - ConfigLayer::default(), + SettingsFile::default(), workflow_layer, project_layer, user_layer, @@ -35,7 +35,7 @@ fn config_layers( fn workflow_and_project_layers( path: &Path, cwd: &Path, -) -> anyhow::Result<(ConfigLayer, ConfigLayer)> { +) -> anyhow::Result<(SettingsFile, SettingsFile)> { let resolution = project::resolve_workflow_path(path, cwd)?; if resolution.workflow_config.is_none() && !resolution.resolved_workflow_path.is_file() { anyhow::bail!( diff --git a/lib/crates/fabro-cli/src/commands/exec.rs b/lib/crates/fabro-cli/src/commands/exec.rs index f2b8e6b69..a89fd5089 100644 --- a/lib/crates/fabro-cli/src/commands/exec.rs +++ b/lib/crates/fabro-cli/src/commands/exec.rs @@ -2,6 +2,7 @@ use anyhow::Result; use fabro_agent::cli::{OutputFormat, run_with_args, run_with_args_and_client}; use fabro_llm::client::Client; use fabro_llm::providers::FabroServerAdapter; +use fabro_mcp::config::{McpServerSettings, McpTransport}; use fabro_types::settings::InterpString; use fabro_types::settings::cli::OutputFormat as SettingsOutputFormat; use fabro_types::settings::run::McpEntryLayer; @@ -11,7 +12,7 @@ use std::sync::Arc; use crate::args::{ExecArgs, GlobalArgs}; use crate::user_config; -fn runtime_mcp_server(name: &str, entry: &McpEntryLayer) -> fabro_mcp::config::McpServerSettings { +fn runtime_mcp_server(name: &str, entry: &McpEntryLayer) -> McpServerSettings { let transport = match entry { McpEntryLayer::Stdio { script, @@ -27,7 +28,7 @@ fn runtime_mcp_server(name: &str, entry: &McpEntryLayer) -> fabro_mcp::config::M .map(|command| command.iter().map(InterpString::as_source).collect()) .unwrap_or_default() }; - fabro_mcp::config::McpTransport::Stdio { + McpTransport::Stdio { command, env: env .iter() @@ -35,7 +36,7 @@ fn runtime_mcp_server(name: &str, entry: &McpEntryLayer) -> fabro_mcp::config::M .collect(), } } - McpEntryLayer::Http { url, headers, .. } => fabro_mcp::config::McpTransport::Http { + McpEntryLayer::Http { url, headers, .. } => McpTransport::Http { url: url.as_source(), headers: headers .iter() @@ -57,7 +58,7 @@ fn runtime_mcp_server(name: &str, entry: &McpEntryLayer) -> fabro_mcp::config::M .map(|command| command.iter().map(InterpString::as_source).collect()) .unwrap_or_default() }; - fabro_mcp::config::McpTransport::Sandbox { + McpTransport::Sandbox { command, port: *port, env: env @@ -87,7 +88,7 @@ fn runtime_mcp_server(name: &str, entry: &McpEntryLayer) -> fabro_mcp::config::M tool_timeout.map_or(60, |duration| duration.as_std().as_secs()), ), }; - fabro_mcp::config::McpServerSettings { + McpServerSettings { name: name.to_string(), transport, startup_timeout_secs, @@ -137,48 +138,47 @@ pub(crate) async fn execute(mut args: ExecArgs, globals: &GlobalArgs) -> Result< // v2 MCPs live under `cli.exec.agent.mcps` (owner-specific) or // `run.agent.mcps`. For `fabro exec` we use the cli.exec path, falling // back to run.agent.mcps if unset. - let mcp_servers: Vec = - if !resolved_cli.exec.agent.mcps.is_empty() { - resolved_cli - .exec - .agent - .mcps - .values() - .map(|server| fabro_mcp::config::McpServerSettings { - name: server.name.clone(), - transport: server.transport.clone(), - startup_timeout_secs: server.startup_timeout_secs, - tool_timeout_secs: server.tool_timeout_secs, - }) - .collect() - } else if let Some(mcps) = cli_settings - .cli - .as_ref() - .and_then(|cli| cli.exec.as_ref()) - .and_then(|exec| exec.agent.as_ref()) - .map(|agent| &agent.mcps) - .filter(|mcps| !mcps.is_empty()) - { - mcps.iter() - .map(|(name, entry)| runtime_mcp_server(name, entry)) - .collect() - } else { - fabro_config::resolve_run_from_file(&cli_settings) - .map(|settings| { - settings - .agent - .mcps - .values() - .map(|server| fabro_mcp::config::McpServerSettings { - name: server.name.clone(), - transport: server.transport.clone(), - startup_timeout_secs: server.startup_timeout_secs, - tool_timeout_secs: server.tool_timeout_secs, - }) - .collect() - }) - .unwrap_or_default() - }; + let mcp_servers: Vec = if !resolved_cli.exec.agent.mcps.is_empty() { + resolved_cli + .exec + .agent + .mcps + .values() + .map(|server| McpServerSettings { + name: server.name.clone(), + transport: server.transport.clone(), + startup_timeout_secs: server.startup_timeout_secs, + tool_timeout_secs: server.tool_timeout_secs, + }) + .collect() + } else if let Some(mcps) = cli_settings + .cli + .as_ref() + .and_then(|cli| cli.exec.as_ref()) + .and_then(|exec| exec.agent.as_ref()) + .map(|agent| &agent.mcps) + .filter(|mcps| !mcps.is_empty()) + { + mcps.iter() + .map(|(name, entry)| runtime_mcp_server(name, entry)) + .collect() + } else { + fabro_config::resolve_run_from_file(&cli_settings) + .map(|settings| { + settings + .agent + .mcps + .values() + .map(|server| McpServerSettings { + name: server.name.clone(), + transport: server.transport.clone(), + startup_timeout_secs: server.startup_timeout_secs, + tool_timeout_secs: server.tool_timeout_secs, + }) + .collect() + }) + .unwrap_or_default() + }; if let Some(target) = server_target { tracing::info!(transport = "server", "Agent session starting"); let provider_name = args diff --git a/lib/crates/fabro-cli/src/commands/graph.rs b/lib/crates/fabro-cli/src/commands/graph.rs index 6487f5818..b03606845 100644 --- a/lib/crates/fabro-cli/src/commands/graph.rs +++ b/lib/crates/fabro-cli/src/commands/graph.rs @@ -2,7 +2,7 @@ use std::io::Write; use anyhow::bail; use fabro_api::types; -use fabro_config::ConfigLayer; +use fabro_types::settings::SettingsFile; use fabro_util::terminal::Styles; use tracing::debug; @@ -25,7 +25,7 @@ pub(crate) async fn run( let built = build_run_manifest(ManifestBuildInput { workflow: args.workflow.clone(), cwd: ctx.cwd().to_path_buf(), - args_layer: ConfigLayer::default(), + args_layer: SettingsFile::default(), args: None, run_id: None, })?; diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 0bd492329..fa4893b56 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -574,7 +574,7 @@ pub(crate) async fn run_install(args: &InstallArgs, globals: &GlobalArgs) -> Res let s = Styles::detect_stderr(); let emoji = console::Emoji("⚒️ ", ""); let cli_settings = user_config::load_settings_with_storage_dir(args.storage_dir.as_deref())?; - let storage_dir = cli_settings.storage_dir(); + let storage_dir = user_config::storage_dir(&cli_settings)?; let server_was_running = record::active_server_record(&storage_dir).is_some(); eprintln!(); @@ -1026,9 +1026,8 @@ mod tests { fn config_toml_roundtrips() { use fabro_types::settings::SettingsFile; let toml_str = format_config_toml("brynary"); - let cfg: SettingsFile = fabro_config::ConfigLayer::parse(&toml_str) - .expect("generated config should parse as v2") - .into(); + let cfg: SettingsFile = fabro_types::settings::parse_settings_file(&toml_str) + .expect("generated config should parse as v2"); let allowed = cfg .server .as_ref() @@ -1043,7 +1042,7 @@ mod tests { fn config_toml_has_auth_strategies() { use fabro_types::settings::SettingsFile; let toml_str = format_config_toml("alice"); - let cfg: SettingsFile = fabro_config::ConfigLayer::parse(&toml_str).unwrap().into(); + let cfg: SettingsFile = fabro_types::settings::parse_settings_file(&toml_str).unwrap(); let auth_api = cfg .server .as_ref() @@ -1069,7 +1068,7 @@ mod tests { use fabro_types::settings::SettingsFile; use fabro_types::settings::server::ServerListenLayer; let toml_str = format_config_toml("bob"); - let cfg: SettingsFile = fabro_config::ConfigLayer::parse(&toml_str).unwrap().into(); + let cfg: SettingsFile = fabro_types::settings::parse_settings_file(&toml_str).unwrap(); let listen = cfg .server .as_ref() diff --git a/lib/crates/fabro-cli/src/commands/pr/mod.rs b/lib/crates/fabro-cli/src/commands/pr/mod.rs index 5f55dc607..65b6d00c1 100644 --- a/lib/crates/fabro-cli/src/commands/pr/mod.rs +++ b/lib/crates/fabro-cli/src/commands/pr/mod.rs @@ -7,6 +7,7 @@ mod view; use anyhow::{Context, Result}; use fabro_types::PullRequestRecord; +use fabro_types::settings::InterpString; use crate::args::{GlobalArgs, PrCommand, PrNamespace, ServerTargetArgs}; use crate::command_context::CommandContext; @@ -15,8 +16,26 @@ use crate::shared::github::build_github_app_credentials; pub(crate) async fn dispatch(ns: PrNamespace, globals: &GlobalArgs) -> Result<()> { let ctx = CommandContext::base()?; - let github_app = - build_github_app_credentials(ctx.machine_settings().github_app_id_str().as_deref())?; + let server_settings = + fabro_config::resolve_server_from_file(ctx.machine_settings()).map_err(|errors| { + anyhow::anyhow!( + "failed to resolve server settings:\n{}", + errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("\n") + ) + })?; + let github_app = build_github_app_credentials( + server_settings + .integrations + .github + .app_id + .as_ref() + .map(InterpString::as_source) + .as_deref(), + )?; match ns.command { PrCommand::Create(args) => { Box::pin(create::create_command(args, github_app, globals)).await diff --git a/lib/crates/fabro-cli/src/commands/preflight.rs b/lib/crates/fabro-cli/src/commands/preflight.rs index 899f10046..3216d427e 100644 --- a/lib/crates/fabro-cli/src/commands/preflight.rs +++ b/lib/crates/fabro-cli/src/commands/preflight.rs @@ -1,5 +1,4 @@ use anyhow::bail; -use fabro_config::ConfigLayer; use fabro_types::settings::cli::OutputVerbosity; use fabro_util::terminal::Styles; @@ -8,6 +7,7 @@ use crate::command_context::CommandContext; use crate::commands::run::output::{ api_check_report_to_local, api_diagnostics_to_local, print_preflight_workflow_summary, }; +use crate::commands::run::overrides::preflight_args_layer; use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, preflight_manifest_args}; use crate::shared::print_json_pretty; @@ -19,7 +19,7 @@ pub(crate) async fn execute(mut args: PreflightArgs, globals: &GlobalArgs) -> an let manifest = build_run_manifest(ManifestBuildInput { workflow: args.workflow.clone(), cwd: ctx.cwd().to_path_buf(), - args_layer: ConfigLayer::try_from(&args)?, + args_layer: preflight_args_layer(&args)?, args: preflight_manifest_args(&args), run_id: None, })?; diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index 20b0bb5b7..5bc0c7159 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -13,6 +13,7 @@ use fabro_api::types; use fabro_interview::{AnswerValue, ConsoleInterviewer, Question, QuestionOption, QuestionType}; use fabro_store::EventEnvelope; use fabro_types::settings::cli::OutputVerbosity; +use fabro_types::settings::run::ApprovalMode; use fabro_util::json::normalize_json_value; use fabro_util::terminal::Styles; use fabro_workflow::outcome::StageStatus; @@ -65,9 +66,7 @@ pub(crate) async fn attach_run_with_client( let state = client.get_run_state(run_id).await?; let auto_approve = state.run.as_ref().is_some_and(|record| { fabro_config::resolve_run_from_file(&record.settings) - .map(|settings| { - settings.execution.approval == fabro_types::settings::run::ApprovalMode::Auto - }) + .map(|settings| settings.execution.approval == ApprovalMode::Auto) .unwrap_or(false) }); let verbose = state.run.as_ref().is_some_and(|record| { diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index b4a0a5ee4..a74950eae 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -2,13 +2,13 @@ use std::path::PathBuf; use crate::args::RunArgs; use crate::command_context::CommandContext; -use fabro_config::ConfigLayer; use fabro_config::Storage; use fabro_types::RunId; use fabro_types::settings::SettingsFile; use fabro_util::terminal::Styles; use super::output::{api_diagnostics_to_local, print_preflight_workflow_summary}; +use super::overrides::run_args_layer; use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, run_manifest_args}; use crate::user_config::{self, ServerTarget}; @@ -23,7 +23,7 @@ pub(crate) struct CreatedRun { pub(crate) async fn create_run( ctx: &CommandContext, args: &RunArgs, - cli_defaults: ConfigLayer, + _cli_defaults: SettingsFile, styles: &Styles, quiet: bool, ) -> anyhow::Result { @@ -31,13 +31,8 @@ pub(crate) async fn create_run( .workflow .as_ref() .ok_or_else(|| anyhow::anyhow!("--workflow is required"))?; - let cli_args_config = ConfigLayer::try_from(args)?; + let cli_args_config = run_args_layer(args)?; let cwd = ctx.cwd().to_path_buf(); - let _settings: SettingsFile = cli_args_config - .clone() - .combine(ConfigLayer::for_workflow(workflow_path, &cwd)?) - .combine(cli_defaults) - .into(); let run_id = args .run_id .as_deref() @@ -68,7 +63,7 @@ pub(crate) async fn create_run( let created_run_id = client.create_run_from_manifest(built.manifest).await?; let local_run_dir = match &target { ServerTarget::UnixSocket(_) => Some( - Storage::new(ctx.machine_settings().storage_dir()) + Storage::new(user_config::storage_dir(ctx.machine_settings())?) .run_scratch(&created_run_id) .root() .to_path_buf(), diff --git a/lib/crates/fabro-cli/src/commands/run/overrides.rs b/lib/crates/fabro-cli/src/commands/run/overrides.rs index d7b344ea4..407ed4a09 100644 --- a/lib/crates/fabro-cli/src/commands/run/overrides.rs +++ b/lib/crates/fabro-cli/src/commands/run/overrides.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use anyhow::{Result, anyhow}; -use fabro_config::ConfigLayer; use fabro_sandbox::SandboxProvider; use fabro_types::settings::SettingsFile; use fabro_types::settings::cli::{CliLayer, CliOutputLayer, OutputVerbosity}; @@ -117,67 +116,59 @@ fn current_dir_or_dot() -> PathBuf { std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")) } -impl TryFrom<&RunArgs> for ConfigLayer { - type Error = anyhow::Error; +pub(crate) fn run_args_layer(args: &RunArgs) -> Result { + let model = model_from_args(args.model.as_deref(), args.provider.as_deref()); + let sandbox = sandbox_layer( + args.sandbox.map(Into::into), + sparse_flag(args.preserve_sandbox), + ); + let execution = execution_layer( + sparse_flag(args.dry_run), + sparse_flag(args.auto_approve), + sparse_flag(args.no_retro), + ); - fn try_from(args: &RunArgs) -> Result { - let model = model_from_args(args.model.as_deref(), args.provider.as_deref()); - let sandbox = sandbox_layer( - args.sandbox.map(Into::into), - sparse_flag(args.preserve_sandbox), - ); - let execution = execution_layer( - sparse_flag(args.dry_run), - sparse_flag(args.auto_approve), - sparse_flag(args.no_retro), - ); + let cwd = current_dir_or_dot(); + let goal = goal_layer_from_args(args.goal.as_deref(), args.goal_file.as_deref(), &cwd)?; - let cwd = current_dir_or_dot(); - let goal = goal_layer_from_args(args.goal.as_deref(), args.goal_file.as_deref(), &cwd)?; + let run = RunLayer { + goal, + metadata: parse_labels(&args.label), + model, + sandbox, + execution, + ..RunLayer::default() + }; - let run = RunLayer { - goal, - metadata: parse_labels(&args.label), - model, - sandbox, - execution, - ..RunLayer::default() - }; - - Ok(Self::from(SettingsFile { - run: Some(run), - cli: cli_layer_for_verbose(args.verbose), - ..SettingsFile::default() - })) - } + Ok(SettingsFile { + run: Some(run), + cli: cli_layer_for_verbose(args.verbose), + ..SettingsFile::default() + }) } -impl TryFrom<&PreflightArgs> for ConfigLayer { - type Error = anyhow::Error; +pub(crate) fn preflight_args_layer(args: &PreflightArgs) -> Result { + let model = model_from_args(args.model.as_deref(), args.provider.as_deref()); + let sandbox = args.sandbox.map(|s| RunSandboxLayer { + provider: Some(SandboxProvider::from(s).to_string()), + ..RunSandboxLayer::default() + }); - fn try_from(args: &PreflightArgs) -> Result { - let model = model_from_args(args.model.as_deref(), args.provider.as_deref()); - let sandbox = args.sandbox.map(|s| RunSandboxLayer { - provider: Some(SandboxProvider::from(s).to_string()), - ..RunSandboxLayer::default() - }); + let cwd = current_dir_or_dot(); + let goal = goal_layer_from_args(args.goal.as_deref(), args.goal_file.as_deref(), &cwd)?; - let cwd = current_dir_or_dot(); - let goal = goal_layer_from_args(args.goal.as_deref(), args.goal_file.as_deref(), &cwd)?; + let run = RunLayer { + goal, + model, + sandbox, + ..RunLayer::default() + }; - let run = RunLayer { - goal, - model, - sandbox, - ..RunLayer::default() - }; - - Ok(Self::from(SettingsFile { - run: Some(run), - cli: cli_layer_for_verbose(args.verbose), - ..SettingsFile::default() - })) - } + Ok(SettingsFile { + run: Some(run), + cli: cli_layer_for_verbose(args.verbose), + ..SettingsFile::default() + }) } #[cfg(test)] diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index 330dc7aad..830561486 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -7,6 +7,7 @@ use anyhow::{Context, Result, anyhow}; use async_trait::async_trait; use fabro_interview::{ControlInterviewer, WorkerControlEnvelope, WorkerControlMessage}; use fabro_store::{EventEnvelope, EventPayload, RunProjection}; +use fabro_types::settings::InterpString; use fabro_types::settings::SettingsFile; use fabro_types::{EventBody, RunBlobId, RunEvent, RunId, StatusReason}; use fabro_workflow::artifact_snapshot::CapturedArtifactInfo; @@ -422,16 +423,25 @@ fn maybe_build_github_app_credentials( settings: &SettingsFile, ) -> Result> { let resolved_run = fabro_config::resolve_run_from_file(settings).ok(); + let resolved_server = fabro_config::resolve_server_from_file(settings).ok(); let needs_github_app = resolved_run .as_ref() .is_some_and(|settings| settings.sandbox.provider == "daytona") || resolved_run .as_ref() .is_some_and(|settings| settings.pull_request.is_some()) - || settings.github_permissions().is_some(); + || resolved_server + .as_ref() + .is_some_and(|settings| !settings.integrations.github.permissions.is_empty()); if needs_github_app { - build_github_app_credentials(settings.github_app_id_str().as_deref()) + build_github_app_credentials( + resolved_server + .as_ref() + .and_then(|settings| settings.integrations.github.app_id.as_ref()) + .map(InterpString::as_source) + .as_deref(), + ) } else { Ok(None) } diff --git a/lib/crates/fabro-cli/src/commands/server/foreground.rs b/lib/crates/fabro-cli/src/commands/server/foreground.rs index 5b76736cb..a429a084c 100644 --- a/lib/crates/fabro-cli/src/commands/server/foreground.rs +++ b/lib/crates/fabro-cli/src/commands/server/foreground.rs @@ -43,16 +43,21 @@ pub(crate) async fn execute( ); let pid = std::process::id(); - serve::serve_command(serve_args, styles, storage_dir, move |resolved_bind| { - record::write_server_record( - &record_path, - &record::ServerRecord { - pid, - bind: resolved_bind.clone(), - log_path: log_path.clone(), - started_at: Utc::now(), - }, - ) - }) + Box::pin(serve::serve_command( + serve_args, + styles, + storage_dir, + move |resolved_bind| { + record::write_server_record( + &record_path, + &record::ServerRecord { + pid, + bind: resolved_bind.clone(), + log_path: log_path.clone(), + started_at: Utc::now(), + }, + ) + }, + )) .await } diff --git a/lib/crates/fabro-cli/src/commands/server/mod.rs b/lib/crates/fabro-cli/src/commands/server/mod.rs index 7bebd3aac..01de6dc1d 100644 --- a/lib/crates/fabro-cli/src/commands/server/mod.rs +++ b/lib/crates/fabro-cli/src/commands/server/mod.rs @@ -28,26 +28,33 @@ pub(crate) async fn dispatch(command: ServerCommand, _globals: &GlobalArgs) -> R serve_args.config.as_deref(), storage_dir.as_deref(), )?; - let storage_dir = settings.storage_dir(); + let storage_dir = user_config::storage_dir(&settings)?; let bind_addr = match serve_args.bind.as_deref() { Some(s) => bind::parse_bind(s)?, None => BindRequest::Unix(user_config::default_socket_path()), }; let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); - start::execute(bind_addr, foreground, serve_args, storage_dir, styles).await + Box::pin(start::execute( + bind_addr, + foreground, + serve_args, + storage_dir, + styles, + )) + .await } ServerCommand::Stop(ServerStopArgs { storage_dir, timeout, }) => { let settings = user_config::load_settings_with_storage_dir(storage_dir.as_deref())?; - let storage_dir = settings.storage_dir(); + let storage_dir = user_config::storage_dir(&settings)?; stop::execute(&storage_dir, Duration::from_secs(timeout)); Ok(()) } ServerCommand::Status(ServerStatusArgs { storage_dir, json }) => { let settings = user_config::load_settings_with_storage_dir(storage_dir.as_deref())?; - let storage_dir = settings.storage_dir(); + let storage_dir = user_config::storage_dir(&settings)?; status::execute(&storage_dir, json) } ServerCommand::Serve(ServerServeArgs { @@ -69,7 +76,7 @@ pub(crate) async fn dispatch(command: ServerCommand, _globals: &GlobalArgs) -> R BindRequest::Unix(user_config::default_socket_path()) }; let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); - foreground::execute( + Box::pin(foreground::execute( record_path, ServeArgs { config: active_config_path, @@ -78,7 +85,7 @@ pub(crate) async fn dispatch(command: ServerCommand, _globals: &GlobalArgs) -> R bind_addr, storage_dir.clone_path(), styles, - ) + )) .await } } diff --git a/lib/crates/fabro-cli/src/commands/server/start.rs b/lib/crates/fabro-cli/src/commands/server/start.rs index 5f9c0ea09..00b40ac8d 100644 --- a/lib/crates/fabro-cli/src/commands/server/start.rs +++ b/lib/crates/fabro-cli/src/commands/server/start.rs @@ -24,7 +24,7 @@ pub(crate) async fn execute( serve_args.bind = Some(bind.to_string()); if foreground { - execute_foreground(bind, serve_args, storage_dir, styles).await + Box::pin(execute_foreground(bind, serve_args, storage_dir, styles)).await } else { execute_daemon(&bind, &serve_args, &storage_dir, true) } @@ -143,7 +143,7 @@ async fn execute_foreground( None }; - serve::serve_command( + Box::pin(serve::serve_command( serve_args, styles, Some(storage_dir), @@ -158,7 +158,7 @@ async fn execute_foreground( }, ) }, - ) + )) .await } diff --git a/lib/crates/fabro-cli/src/commands/store/dump.rs b/lib/crates/fabro-cli/src/commands/store/dump.rs index 20d9dd843..ad5064dac 100644 --- a/lib/crates/fabro-cli/src/commands/store/dump.rs +++ b/lib/crates/fabro-cli/src/commands/store/dump.rs @@ -15,11 +15,11 @@ use crate::args::{GlobalArgs, StoreDumpArgs}; use crate::server_client::ServerStoreClient; use crate::server_runs::ServerRunLookup; use crate::shared::{absolute_or_current, print_json_pretty}; -use crate::user_config::load_settings_with_storage_dir; +use crate::user_config::{load_settings_with_storage_dir, storage_dir}; pub(crate) async fn dump_command(args: &StoreDumpArgs, globals: &GlobalArgs) -> Result<()> { let cli_settings = load_settings_with_storage_dir(args.storage_dir.as_deref())?; - let lookup = ServerRunLookup::connect(&cli_settings.storage_dir()).await?; + let lookup = ServerRunLookup::connect(&storage_dir(&cli_settings)?).await?; let run = lookup.resolve(&args.run)?; let run_id = run.run_id(); let state = lookup.client().get_run_state(&run_id).await?; diff --git a/lib/crates/fabro-cli/src/commands/uninstall.rs b/lib/crates/fabro-cli/src/commands/uninstall.rs index 4e34f6e4e..f5ab53135 100644 --- a/lib/crates/fabro-cli/src/commands/uninstall.rs +++ b/lib/crates/fabro-cli/src/commands/uninstall.rs @@ -40,8 +40,10 @@ pub(crate) async fn run_uninstall(args: &UninstallArgs, globals: &GlobalArgs) -> return Ok(()); } - let storage_dir = - user_config::load_settings().map_or_else(|_| home.storage_dir(), |s| s.storage_dir()); + let storage_dir = user_config::load_settings().map_or_else( + |_| home.storage_dir(), + |settings| user_config::storage_dir(&settings).unwrap_or_else(|_| home.storage_dir()), + ); let inventory = build_inventory(&home_root, &storage_dir); diff --git a/lib/crates/fabro-cli/src/commands/validate.rs b/lib/crates/fabro-cli/src/commands/validate.rs index fe1a7fab6..eaa6ef3de 100644 --- a/lib/crates/fabro-cli/src/commands/validate.rs +++ b/lib/crates/fabro-cli/src/commands/validate.rs @@ -1,5 +1,5 @@ use anyhow::bail; -use fabro_config::ConfigLayer; +use fabro_types::settings::SettingsFile; use fabro_util::terminal::Styles; use crate::args::{GlobalArgs, ValidateArgs}; @@ -17,7 +17,7 @@ pub(crate) async fn run( let built = build_run_manifest(ManifestBuildInput { workflow: args.workflow.clone(), cwd: ctx.cwd().to_path_buf(), - args_layer: ConfigLayer::default(), + args_layer: SettingsFile::default(), args: None, run_id: None, })?; diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index bbafa8b70..1a2416d2e 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -133,12 +133,13 @@ async fn main_inner() -> (String, Result<()>) { { match load_settings_config(args.config.as_deref()) { Ok(layer) => { - use fabro_types::settings::SettingsFile; - let server_settings: SettingsFile = layer.into(); + let server_settings = layer; ( server_settings - .server_logging() - .and_then(|l| l.level.clone()), + .server + .as_ref() + .and_then(|server| server.logging.as_ref()) + .and_then(|logging| logging.level.clone()), false, ) } diff --git a/lib/crates/fabro-cli/src/manifest_builder.rs b/lib/crates/fabro-cli/src/manifest_builder.rs index d88eb2599..b004fa7f3 100644 --- a/lib/crates/fabro-cli/src/manifest_builder.rs +++ b/lib/crates/fabro-cli/src/manifest_builder.rs @@ -3,9 +3,10 @@ use std::path::{Component, Path, PathBuf}; use anyhow::{Context, Result, anyhow}; use fabro_api::types; -use fabro_config::ConfigLayer; +use fabro_config::load::{load_settings_for_workflow, load_settings_user}; +use fabro_config::merge::combine_files; use fabro_config::project::{self, discover_project_config, resolve_workflow_path}; -use fabro_config::run::parse_run_config; +use fabro_config::run::{parse_run_config, resolve_run_goal}; use fabro_config::user::active_settings_path; use fabro_graphviz::graph::AttrValue; use fabro_graphviz::parser; @@ -21,7 +22,7 @@ use crate::args::{PreflightArgs, RunArgs}; pub(crate) struct ManifestBuildInput { pub workflow: PathBuf, pub cwd: PathBuf, - pub args_layer: ConfigLayer, + pub args_layer: SettingsFile, pub args: Option, pub run_id: Option, } @@ -46,13 +47,12 @@ struct WorkflowScanInput { } pub(crate) fn build_run_manifest(input: ManifestBuildInput) -> Result { - let user_layer = ConfigLayer::settings()?; - let merged_settings: SettingsFile = input - .args_layer - .clone() - .combine(ConfigLayer::for_workflow(&input.workflow, &input.cwd)?) - .combine(user_layer.clone()) - .into(); + let user_layer = load_settings_user()?; + let workflow_layer = load_settings_for_workflow(&input.workflow, &input.cwd)?; + let merged_settings = combine_files( + combine_files(user_layer, workflow_layer), + input.args_layer.clone(), + ); let root_resolution = resolve_workflow_path(&input.workflow, &input.cwd)?; let target_path = root_resolution.dot_path.clone(); @@ -321,7 +321,6 @@ fn collect_workflow_config_files( ) -> Result<()> { let config_layer = parse_run_config(&config.source)?; let dockerfile = config_layer - .as_v2() .run .as_ref() .and_then(|run| run.sandbox.as_ref()) @@ -385,7 +384,7 @@ fn collect_bundled_file( } fn resolve_manifest_goal( - args_layer: &ConfigLayer, + args_layer: &SettingsFile, settings: &SettingsFile, root_source: &str, root_dot_path: &Path, @@ -395,19 +394,16 @@ fn resolve_manifest_goal( // Precedence 1: CLI args (`--goal` / `--goal-file`). These are already // resolved to absolute paths by `overrides::goal_layer_from_args`. - if let Some(resolved) = args_layer - .as_v2() - .resolve_run_goal(&working_directory) + if let Some(resolved) = resolve_run_goal(args_layer, &working_directory) .context("failed to resolve --goal-file contents")? { return Ok(Some(resolved_goal_to_manifest(resolved))); } // Precedence 2: merged config `run.goal`. Config-sourced `goal.file` - // paths were rewritten to absolute by `ConfigLayer::load` at the + // paths were rewritten to absolute by `load_settings_path` at the // directory of the config file that declared them. - if let Some(resolved) = settings - .resolve_run_goal(&working_directory) + if let Some(resolved) = resolve_run_goal(settings, &working_directory) .context("failed to resolve run.goal.file contents")? { return Ok(Some(resolved_goal_to_manifest(resolved))); @@ -608,7 +604,7 @@ mod tests { // Isolate from the developer's real ~/.fabro/settings.toml which may // still be in the legacy shape. Setting FABRO_CONFIG to a path inside - // the test tempdir forces the loader to produce an empty ConfigLayer. + // the test tempdir forces the loader to produce an empty user layer. let sandboxed_settings = temp.path().join("empty-settings.toml"); std::fs::write(&sandboxed_settings, "_version = 1\n").unwrap(); // SAFETY: single-threaded unit test body. @@ -619,7 +615,7 @@ mod tests { let built = build_run_manifest(ManifestBuildInput { workflow: PathBuf::from("fabro/workflows/demo/workflow.toml"), cwd: project.to_path_buf(), - args_layer: ConfigLayer::default(), + args_layer: SettingsFile::default(), args: None, run_id: None, }) @@ -706,7 +702,7 @@ file = "prompts/goal.md" let built = build_run_manifest(ManifestBuildInput { workflow: PathBuf::from("fabro/workflows/demo/workflow.toml"), cwd: project.to_path_buf(), - args_layer: ConfigLayer::default(), + args_layer: SettingsFile::default(), args: None, run_id: None, }) @@ -770,7 +766,7 @@ file = "prompts/goal.md" let built = build_run_manifest(ManifestBuildInput { workflow: PathBuf::from("fabro/workflows/demo/workflow.toml"), cwd: project.to_path_buf(), - args_layer: ConfigLayer::default(), + args_layer: SettingsFile::default(), args: None, run_id: None, }) diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index f75c2077e..15e78b863 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -106,7 +106,7 @@ pub(crate) async fn connect_server_with_settings( let target = user_config::resolve_server_target(args, settings)?; let runtime = LocalServerRuntime { active_config_path: base_config_path.to_path_buf(), - storage_dir: settings.storage_dir(), + storage_dir: user_config::storage_dir(settings)?, }; connect_target_api_client_bundle(&target, &runtime).await } diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index 561513752..23324c577 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -3,7 +3,6 @@ use std::path::{Path, PathBuf}; pub(crate) use fabro_config::user::*; use anyhow::{Result, bail}; -use fabro_config::ConfigLayer; use fabro_types::settings::cli::CliTargetSettings; use fabro_types::settings::{CliSettings, SettingsFile}; use fabro_util::version::FABRO_VERSION; @@ -27,28 +26,28 @@ pub(crate) fn load_settings() -> anyhow::Result { pub(crate) fn settings_layer_with_config_and_storage_dir( config_path: Option<&Path>, storage_dir: Option<&Path>, -) -> anyhow::Result { +) -> anyhow::Result { let layer = load_settings_config(config_path)?; Ok(apply_storage_dir_override(layer, storage_dir)) } pub(crate) fn settings_layer_with_storage_dir( storage_dir: Option<&Path>, -) -> anyhow::Result { +) -> anyhow::Result { settings_layer_with_config_and_storage_dir(None, storage_dir) } pub(crate) fn load_settings_with_storage_dir( storage_dir: Option<&Path>, ) -> anyhow::Result { - Ok(settings_layer_with_storage_dir(storage_dir)?.into()) + settings_layer_with_storage_dir(storage_dir) } pub(crate) fn load_settings_with_config_and_storage_dir( config_path: Option<&Path>, storage_dir: Option<&Path>, ) -> anyhow::Result { - Ok(settings_layer_with_config_and_storage_dir(config_path, storage_dir)?.into()) + settings_layer_with_config_and_storage_dir(config_path, storage_dir) } fn render_resolve_errors(errors: Vec) -> anyhow::Error { @@ -67,14 +66,13 @@ pub(crate) fn resolve_cli_settings(file: &SettingsFile) -> anyhow::Result, -) -> ConfigLayer { +) -> SettingsFile { use fabro_types::settings::interp::InterpString; use fabro_types::settings::server::{ServerLayer, ServerStorageLayer}; if let Some(dir) = storage_dir { - let file = layer.as_v2_mut(); - let server = file.server.get_or_insert_with(ServerLayer::default); + let server = layer.server.get_or_insert_with(ServerLayer::default); let storage = server .storage .get_or_insert_with(ServerStorageLayer::default); @@ -100,12 +98,10 @@ fn cli_target_from_settings(settings: &CliSettings) -> Option<(String, Option { - let tls_settings = tls.as_ref().and_then(|tls| { - Some(ClientTlsSettings { - cert: PathBuf::from(tls.cert.as_source()), - key: PathBuf::from(tls.key.as_source()), - ca: PathBuf::from(tls.ca.as_source()), - }) + let tls_settings = tls.as_ref().map(|tls| ClientTlsSettings { + cert: PathBuf::from(tls.cert.as_source()), + key: PathBuf::from(tls.key.as_source()), + ca: PathBuf::from(tls.ca.as_source()), }); Some((url.as_source(), tls_settings)) } @@ -125,6 +121,30 @@ pub(crate) fn default_server_target() -> ServerTarget { ServerTarget::UnixSocket(default_socket_path()) } +pub(crate) fn storage_dir(settings: &SettingsFile) -> anyhow::Result { + let resolved = fabro_config::resolve_server_from_file(settings).map_err(|errors| { + anyhow::anyhow!( + "failed to resolve server settings:\n{}", + errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("\n") + ) + })?; + let resolved_root = resolved + .storage + .root + .resolve(|name| std::env::var(name).ok()) + .map_err(|err| { + anyhow::anyhow!( + "failed to resolve {}: {err}", + resolved.storage.root.as_source() + ) + })?; + Ok(PathBuf::from(resolved_root.value)) +} + fn parse_server_target(value: &str, tls: Option) -> Result { if value.starts_with("http://") || value.starts_with("https://") { return Ok(ServerTarget::HttpUrl { @@ -213,6 +233,7 @@ pub(crate) fn build_server_client( mod tests { use super::*; use crate::args::ServerTargetArgs; + use fabro_types::settings::parse_settings_file; fn server_target_args(value: Option<&str>) -> ServerTargetArgs { ServerTargetArgs { @@ -221,9 +242,7 @@ mod tests { } fn parse_v2(source: &str) -> SettingsFile { - fabro_config::ConfigLayer::parse(source) - .expect("fixture should parse") - .into() + parse_settings_file(source).expect("fixture should parse") } #[test] diff --git a/lib/crates/fabro-cli/tests/it/cmd/config.rs b/lib/crates/fabro-cli/tests/it/cmd/config.rs index 4be3cfc92..b26c32f2e 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/config.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/config.rs @@ -1,8 +1,7 @@ use std::path::PathBuf; -use fabro_config::ConfigLayer; use fabro_test::{fabro_snapshot, test_context}; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{SettingsFile, parse_settings_file}; use httpmock::MockServer; use predicates::prelude::*; @@ -43,8 +42,94 @@ fn resolve_project(settings: &SettingsFile) -> fabro_types::settings::ProjectSet fabro_config::resolve_project_from_file(settings).expect("project settings should resolve") } +fn resolve_run(settings: &SettingsFile) -> fabro_types::settings::RunSettings { + fabro_config::resolve_run_from_file(settings).expect("run settings should resolve") +} + +fn resolve_server(settings: &SettingsFile) -> fabro_types::settings::ServerSettings { + fabro_config::resolve_server_from_file(settings).expect("server settings should resolve") +} + +fn run_goal_inline(settings: &SettingsFile) -> Option { + match resolve_run(settings).goal { + Some(fabro_types::settings::run::RunGoal::Inline(value)) => Some(value.as_source()), + _ => None, + } +} + +fn run_model_name(settings: &SettingsFile) -> Option { + resolve_run(settings) + .model + .name + .as_ref() + .map(|value| value.as_source()) +} + +fn run_model_provider(settings: &SettingsFile) -> Option { + resolve_run(settings) + .model + .provider + .as_ref() + .map(|value| value.as_source()) +} + +fn run_inputs(settings: &SettingsFile) -> &std::collections::HashMap { + settings + .run + .as_ref() + .and_then(|run| run.inputs.as_ref()) + .expect("run.inputs") +} + +fn run_sandbox(settings: &SettingsFile) -> &fabro_types::settings::run::RunSandboxLayer { + settings + .run + .as_ref() + .and_then(|run| run.sandbox.as_ref()) + .expect("run.sandbox") +} + +fn run_checkpoint(settings: &SettingsFile) -> &fabro_types::settings::run::RunCheckpointLayer { + settings + .run + .as_ref() + .and_then(|run| run.checkpoint.as_ref()) + .expect("run.checkpoint") +} + +fn run_hooks(settings: &SettingsFile) -> &[fabro_types::settings::run::HookEntry] { + settings + .run + .as_ref() + .map(|run| run.hooks.as_slice()) + .unwrap_or(&[]) +} + +fn run_agent_mcps( + settings: &SettingsFile, +) -> &std::collections::HashMap { + settings + .run + .as_ref() + .and_then(|run| run.agent.as_ref()) + .map(|agent| &agent.mcps) + .expect("run.agent.mcps") +} + +fn auto_approve_enabled(settings: &SettingsFile) -> bool { + resolve_run(settings).execution.approval == fabro_types::settings::run::ApprovalMode::Auto +} + +fn run_prepare_commands(settings: &SettingsFile) -> Vec { + resolve_run(settings).prepare.commands +} + +fn server_storage_root(settings: &SettingsFile) -> String { + resolve_server(settings).storage.root.as_source() +} + fn server_settings_fixture() -> SettingsFile { - ConfigLayer::parse( + parse_settings_file( r#" _version = 1 @@ -61,7 +146,6 @@ shared = "server" "#, ) .expect("server settings fixture should parse") - .into() } fn server_settings_body(settings: &SettingsFile) -> String { @@ -300,14 +384,14 @@ fn settings_local_merges_cli_and_project_defaults() { .clone(); let cfg = parse_settings(&output); - 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!(run_model_name(&cfg).as_deref(), Some("project-model")); + assert_eq!(run_model_provider(&cfg).as_deref(), Some("openai")); + assert_eq!(run_goal_inline(&cfg).as_deref(), None); 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. - let vars = cfg.run_inputs().expect("run.inputs"); + let vars = run_inputs(&cfg); assert_eq!(vars.get("project_only").and_then(|v| v.as_str()), Some("1")); assert_eq!(vars.get("shared").and_then(|v| v.as_str()), Some("project")); assert!( @@ -317,7 +401,7 @@ fn settings_local_merges_cli_and_project_defaults() { // v2 R71: provider-native maps such as run.sandbox.daytona.labels remain // sticky merge-by-key, so CLI labels persist under the project layer. - let sandbox = cfg.run_sandbox().expect("run.sandbox"); + let sandbox = run_sandbox(&cfg); let labels = &sandbox.daytona.as_ref().expect("daytona").labels; assert_eq!(labels.get("cli_only").map(String::as_str), Some("1")); assert_eq!(labels.get("shared").map(String::as_str), Some("cli")); @@ -341,18 +425,18 @@ fn settings_local_workflow_name_applies_run_overlay_and_deep_merges() { use fabro_types::settings::run::McpEntryLayer; let cfg = parse_settings(&output); - assert_eq!(cfg.run_goal_inline_str().as_deref(), Some("demo goal")); - assert_eq!(cfg.run_model_name_str().as_deref(), Some("run-model")); - assert_eq!(cfg.run_model_provider_str().as_deref(), Some("anthropic")); + assert_eq!(run_goal_inline(&cfg).as_deref(), Some("demo goal")); + assert_eq!(run_model_name(&cfg).as_deref(), Some("run-model")); + assert_eq!(run_model_provider(&cfg).as_deref(), Some("anthropic")); // v2 R22: run.inputs replaces wholesale, so the workflow layer wins // over project and cli. - let vars = cfg.run_inputs().expect("run.inputs"); + let vars = run_inputs(&cfg); assert_eq!(vars.get("run_only").and_then(|v| v.as_str()), Some("1")); assert_eq!(vars.get("shared").and_then(|v| v.as_str()), Some("run")); // checkpoint.exclude_globs is a security/policy list: replace by default. - let checkpoint = cfg.run_checkpoint().expect("run.checkpoint"); + let checkpoint = run_checkpoint(&cfg); assert_eq!( checkpoint.exclude_globs, vec!["run-only".to_string(), "shared".to_string()] @@ -361,7 +445,7 @@ fn settings_local_workflow_name_applies_run_overlay_and_deep_merges() { // Hooks: id-based replacement. The "shared" hook appears in both cli and // workflow layers and resolves to the workflow entry; project and run-only // contribute the other two ids. - let hooks = cfg.run_hooks(); + let hooks = run_hooks(&cfg); assert!(hooks.len() >= 2); let shared_hook = hooks .iter() @@ -381,7 +465,7 @@ fn settings_local_workflow_name_applies_run_overlay_and_deep_merges() { .any(|hook| hook.name.as_deref() == Some("run-only")) ); - let mcps = cfg.run_agent_mcps().expect("run.agent.mcps"); + let mcps = run_agent_mcps(&cfg); match mcps.get("shared").expect("shared mcp") { McpEntryLayer::Stdio { command, .. } => { let command = command.as_ref().expect("command"); @@ -393,7 +477,7 @@ fn settings_local_workflow_name_applies_run_overlay_and_deep_merges() { assert!(mcps.contains_key("run_only")); // run.sandbox.daytona.labels stays sticky merge-by-key per R71. - let sandbox = cfg.run_sandbox().expect("run.sandbox"); + let sandbox = run_sandbox(&cfg); let labels = &sandbox.daytona.as_ref().expect("daytona").labels; assert_eq!(labels.get("run_only").map(String::as_str), Some("1")); assert_eq!(labels.get("shared").map(String::as_str), Some("run")); @@ -434,14 +518,14 @@ fn settings_local_explicit_workflow_path_uses_workflow_project_layers() { .clone(); let cfg = parse_settings(&output); - assert!(cfg.auto_approve_enabled()); + assert!(auto_approve_enabled(&cfg)); // v2 R30: run.prepare.steps replaces the whole ordered list across layers. // The highest-precedence layer (workflow) wins. assert_eq!( - cfg.run_prepare_commands(), + run_prepare_commands(&cfg), vec!["workflow-setup".to_string()] ); - assert_eq!(cfg.run_sandbox().and_then(|sb| sb.preserve), Some(true)); + assert_eq!(run_sandbox(&cfg).preserve, Some(true)); } #[test] @@ -594,7 +678,12 @@ name = "legacy-model" resolve_cli(&cfg).output.verbosity, fabro_types::settings::cli::OutputVerbosity::Normal ); - assert!(cfg.run_model().is_none()); + assert!( + cfg.run + .as_ref() + .and_then(|run| run.model.as_ref()) + .is_none() + ); } #[test] @@ -623,13 +712,9 @@ shared = "legacy" .stderr(predicate::str::contains("ignoring legacy config file")); let cfg = parse_settings(&assert.get_output().stdout); - assert_eq!(cfg.run_model_name_str().as_deref(), Some("project-model")); - assert_eq!( - cfg.run_inputs() - .and_then(|vars| vars.get("shared")) - .and_then(|v| v.as_str()), - Some("project") - ); + assert_eq!(run_model_name(&cfg).as_deref(), Some("project-model")); + let vars = run_inputs(&cfg); + assert_eq!(vars.get("shared").and_then(|v| v.as_str()), Some("project")); } #[test] @@ -761,12 +846,9 @@ shared = "cli" mock.assert(); let cfg = parse_settings(&output); - 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.server_storage_root_str().as_deref(), - Some("/srv/fabro-server") - ); + assert_eq!(run_model_name(&cfg).as_deref(), Some("project-model")); + assert_eq!(run_model_provider(&cfg).as_deref(), Some("openai")); + assert_eq!(server_storage_root(&cfg), "/srv/fabro-server"); assert_eq!( resolve_cli(&cfg).output.verbosity, fabro_types::settings::cli::OutputVerbosity::Verbose @@ -775,7 +857,7 @@ shared = "cli" // R22: run.inputs replaces wholesale across layers. Project is the // highest-precedence layer that sets inputs, so project's vars win // and server-side vars are discarded rather than merged. - let vars = cfg.run_inputs().expect("run.inputs"); + let vars = run_inputs(&cfg); assert_eq!(vars.get("project_only").and_then(|v| v.as_str()), Some("1")); assert_eq!(vars.get("shared").and_then(|v| v.as_str()), Some("project")); assert!( @@ -832,10 +914,7 @@ verbosity = "verbose" cli_mock.assert(); configured_mock.assert_calls(0); let cfg = parse_settings(&output); - assert_eq!( - cfg.server_storage_root_str().as_deref(), - Some("/srv/fabro-server") - ); + assert_eq!(server_storage_root(&cfg), "/srv/fabro-server"); } #[test] diff --git a/lib/crates/fabro-cli/tests/it/cmd/create.rs b/lib/crates/fabro-cli/tests/it/cmd/create.rs index 825ec7c2a..09565db30 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/create.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/create.rs @@ -8,6 +8,12 @@ use crate::support::{fabro_json_snapshot, unique_run_id}; use super::support::{fixture, output_stdout, resolve_run, run_count_for_test_case, run_state}; +fn resolved_run( + settings: &fabro_types::settings::SettingsFile, +) -> fabro_types::settings::RunSettings { + fabro_config::resolve_run_from_file(settings).expect("run settings should resolve") +} + fn run_status_response(run_id: &str, status: &str) -> serde_json::Value { serde_json::json!({ "id": run_id, @@ -353,22 +359,26 @@ fn create_persists_requested_overrides_into_store() { "team": run_record.labels.get("team"), }); let settings = &run_record.settings; + let resolved_run = resolved_run(settings); let cli_settings = fabro_config::resolve_cli_from_file(settings).expect("cli settings"); let compact = json!({ "workflow_slug": run_record.workflow_slug, "settings": { - "goal": settings.run_goal_inline_str(), - "dry_run": settings.dry_run_enabled(), - "auto_approve": settings.auto_approve_enabled(), - "no_retro": settings.no_retro_enabled(), + "goal": match resolved_run.goal.as_ref() { + Some(fabro_types::settings::run::RunGoal::Inline(value)) => Some(value.as_source()), + _ => None, + }, + "dry_run": resolved_run.execution.mode == fabro_types::settings::run::RunMode::DryRun, + "auto_approve": resolved_run.execution.approval == fabro_types::settings::run::ApprovalMode::Auto, + "no_retro": !resolved_run.execution.retros, "verbose": cli_settings.output.verbosity == fabro_types::settings::cli::OutputVerbosity::Verbose, "llm": { - "model": settings.run_model_name_str(), - "provider": settings.run_model_provider_str(), + "model": resolved_run.model.name.as_ref().map(|value| value.as_source()), + "provider": resolved_run.model.provider.as_ref().map(|value| value.as_source()), }, "sandbox": { - "provider": settings.run_sandbox().and_then(|sb| sb.provider.clone()), - "preserve": settings.preserve_sandbox_enabled(), + "provider": resolved_run.sandbox.provider, + "preserve": resolved_run.sandbox.preserve, }, }, "labels": labels, @@ -425,12 +435,16 @@ fn create_json_implies_auto_approve() { let run = resolve_run(&context, run_id); assert!( - run_state(&run.run_dir) - .run - .as_ref() - .expect("run record should exist") - .settings - .auto_approve_enabled() + resolved_run( + &run_state(&run.run_dir) + .run + .as_ref() + .expect("run record should exist") + .settings, + ) + .execution + .approval + == fabro_types::settings::run::ApprovalMode::Auto ); } diff --git a/lib/crates/fabro-cli/tests/it/cmd/runner.rs b/lib/crates/fabro-cli/tests/it/cmd/runner.rs index 823205294..36a049425 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/runner.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/runner.rs @@ -210,10 +210,11 @@ digraph GitHubApp { let run_dir = context.find_run_dir(&run_id); let state = run_state(&run_dir); let run = state.run.as_ref().expect("run record should exist"); + let resolved_server = fabro_config::resolve_server_from_file(&run.settings).unwrap(); fabro_json_snapshot!( context, serde_json::json!({ - "app_id": run.settings.github_app_id_str(), + "app_id": resolved_server.integrations.github.app_id.map(|value| value.as_source()), }), @r#" { diff --git a/lib/crates/fabro-config/src/config.rs b/lib/crates/fabro-config/src/config.rs deleted file mode 100644 index b9c066ccb..000000000 --- a/lib/crates/fabro-config/src/config.rs +++ /dev/null @@ -1,376 +0,0 @@ -//! v2-backed configuration layer. -//! -//! `ConfigLayer` is a newtype over [`SettingsFile`] — the v2 namespaced -//! parse tree in `fabro_types::settings::v2`. Loading functions (`parse`, -//! `load`, `for_workflow`, `project`, `settings`) all hard-fail on legacy -//! top-level keys with targeted rename hints. `ConfigLayer::combine` walks -//! the v2 merge matrix from [`crate::merge`]. -//! -//! Consumers that need the inner tree call [`ConfigLayer::as_v2`] (borrow) -//! or `.into()` to move out an owned `SettingsFile`. The legacy flat -//! `Settings` shape is no longer reachable from this layer. - -use std::path::Path; - -use anyhow::Context; -use fabro_types::settings::accessors::resolve_goal_file_path; -use fabro_types::settings::interp::InterpString; -use fabro_types::settings::run::RunGoalLayer; -use fabro_types::settings::{SettingsFile, parse_settings_file as parse_v2_settings_file}; -use serde::{Deserialize, Serialize}; - -use crate::merge::combine_files; -use crate::project::{self}; -use crate::user; - -/// Rewrite any relative `run.goal = { file = "..." }` path in `file` to an -/// absolute path anchored at `base_dir`. -/// -/// Called from `ConfigLayer::load` so that layers coming from different -/// config files can be merged without losing the "relative to my source -/// file" context. Paths that contain `${env.NAME}` interpolation are left -/// alone (they get resolved against the run's working directory at consume -/// time via [`SettingsFile::resolve_run_goal`]). -fn resolve_goal_file_paths(file: &mut SettingsFile, base_dir: &Path) { - let Some(run) = file.run.as_mut() else { - return; - }; - let Some(RunGoalLayer::File { file: goal_file }) = run.goal.as_mut() else { - return; - }; - if !goal_file.is_literal() { - // Env-tokenized paths stay unresolved until consume time. - return; - } - let literal = goal_file.as_source(); - if Path::new(&literal).is_absolute() { - return; - } - let absolute = resolve_goal_file_path(&literal, base_dir); - *goal_file = InterpString::parse(&absolute.to_string_lossy()); -} - -/// A parsed settings file layer. -/// -/// Thin newtype around the v2 [`SettingsFile`] parse tree. The newtype -/// exists so fabro-config can attach helper methods and evolve the -/// internal representation without forcing every caller to import v2 -/// types. -#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] -#[serde(transparent)] -pub struct ConfigLayer { - pub file: SettingsFile, -} - -impl From for ConfigLayer { - fn from(file: SettingsFile) -> Self { - Self { file } - } -} - -impl From for SettingsFile { - fn from(layer: ConfigLayer) -> Self { - layer.file - } -} - -impl ConfigLayer { - /// Combine two layers using the v2 merge matrix. - #[must_use] - pub fn combine(self, other: Self) -> Self { - // In the legacy contract `self.combine(other)` means `self` is the - // higher-precedence layer and `other` is the lower-precedence one. - // The merge matrix walker takes (lower, higher). - Self { - file: combine_files(other.file, self.file), - } - } - - /// Parse a v2 TOML settings file into a layer. - pub fn parse(content: &str) -> anyhow::Result { - let file = parse_v2_settings_file(content) - .map_err(|e| anyhow::anyhow!("{e}")) - .context("Failed to parse settings file")?; - Ok(Self { file }) - } - - /// Load a v2 TOML settings file from disk. - /// - /// Relative `run.goal = { file = "..." }` paths are resolved against - /// the directory of `path` at load time. Subsequent merging with other - /// layers can then safely treat the path as self-contained. - pub fn load(path: &Path) -> anyhow::Result { - let content = std::fs::read_to_string(path) - .with_context(|| format!("Failed to read {}", path.display()))?; - let mut layer = Self::parse(&content)?; - let base_dir = path.parent().unwrap_or_else(|| Path::new(".")); - resolve_goal_file_paths(&mut layer.file, base_dir); - Ok(layer) - } - - /// Load workflow config + project config for a workflow path. - /// - /// Resolves the workflow path, loads its config, discovers project config - /// (`fabro.toml`) from the resolved workflow's parent directory, and - /// combines them (workflow takes precedence over project). - pub fn for_workflow(path: &Path, cwd: &Path) -> anyhow::Result { - let resolution = project::resolve_workflow_path(path, cwd)?; - if resolution.workflow_config.is_none() && !resolution.resolved_workflow_path.is_file() { - anyhow::bail!( - "Workflow not found: {}", - resolution.resolved_workflow_path.display() - ); - } - - let workflow_config = resolution.workflow_config.unwrap_or_default(); - let project_config = project::discover_project_config( - resolution - .resolved_workflow_path - .parent() - .unwrap_or_else(|| Path::new(".")), - )? - .map(|(_, config)| config) - .unwrap_or_default(); - - Ok(workflow_config.combine(project_config)) - } - - /// Discover project config (`fabro.toml`) by walking ancestors from `start`. - pub fn project(start: &Path) -> anyhow::Result { - Ok(project::discover_project_config(start)? - .map(|(_, config)| config) - .unwrap_or_default()) - } - - /// Load machine-level defaults from `~/.fabro/settings.toml`. - pub fn settings() -> anyhow::Result { - user::load_settings_config(None) - } - - /// Borrow the inner v2 settings file for direct access. - #[must_use] - pub fn as_v2(&self) -> &SettingsFile { - &self.file - } - - /// Mutably borrow the inner v2 settings file. - pub fn as_v2_mut(&mut self) -> &mut SettingsFile { - &mut self.file - } -} - -#[cfg(test)] -mod tests { - use fabro_types::settings::run::RunGoalLayer; - - use super::*; - - #[test] - fn parse_rejects_legacy_flat_keys() { - let err = ConfigLayer::parse("[llm]\nprovider = \"openai\"").unwrap_err(); - let text = format!("{err:#}"); - assert!( - text.contains("run.model") || text.contains("llm"), - "expected rename hint in error: {text}" - ); - } - - #[test] - fn parse_accepts_inline_goal() { - let layer = ConfigLayer::parse( - r#" -_version = 1 -[run] -goal = "Do things" -"#, - ) - .unwrap(); - assert_eq!( - layer.file.run_goal_inline_str().as_deref(), - Some("Do things") - ); - } - - #[test] - fn parse_accepts_file_variant() { - let layer = ConfigLayer::parse( - r#" -_version = 1 -[run.goal] -file = "prompts/goal.md" -"#, - ) - .unwrap(); - let Some(RunGoalLayer::File { file }) = layer.file.run_goal_layer() else { - panic!("expected run.goal.file variant"); - }; - assert_eq!(file.as_source(), "prompts/goal.md"); - } - - #[test] - fn parse_rejects_goal_with_unknown_sibling_fields() { - // The untagged enum should reject any `{ file = ..., extra = ... }` - // shape because neither the inline nor the file variant matches. - let err = ConfigLayer::parse( - r#" -_version = 1 -[run.goal] -file = "prompts/goal.md" -extra = "boom" -"#, - ) - .unwrap_err(); - let text = format!("{err:#}"); - assert!(text.to_lowercase().contains("run.goal") || text.contains("extra")); - } - - #[test] - fn combine_prefers_higher_precedence_self() { - let higher = ConfigLayer::parse( - r#" -_version = 1 -[run] -goal = "higher goal" -"#, - ) - .unwrap(); - let lower = ConfigLayer::parse( - r#" -_version = 1 -[run] -goal = "lower goal" -"#, - ) - .unwrap(); - let merged = higher.combine(lower); - assert_eq!( - merged.file.run_goal_inline_str().as_deref(), - Some("higher goal") - ); - } - - #[test] - fn combine_replaces_file_goal_with_inline_from_higher_layer() { - // A higher-precedence `run.goal = "inline"` must fully override a - // lower layer's `run.goal = { file = "..." }` — the scalar merge - // treats `goal` as one field regardless of which variant each - // layer picked. - let higher = ConfigLayer::parse( - r#" -_version = 1 -[run] -goal = "inline override" -"#, - ) - .unwrap(); - let lower = ConfigLayer::parse( - r#" -_version = 1 -[run.goal] -file = "/tmp/goal.md" -"#, - ) - .unwrap(); - let merged = higher.combine(lower); - assert_eq!( - merged.file.run_goal_inline_str().as_deref(), - Some("inline override") - ); - } - - #[test] - fn combine_replaces_inline_goal_with_file_from_higher_layer() { - let higher = ConfigLayer::parse( - r#" -_version = 1 -[run.goal] -file = "/tmp/goal.md" -"#, - ) - .unwrap(); - let lower = ConfigLayer::parse( - r#" -_version = 1 -[run] -goal = "inline loser" -"#, - ) - .unwrap(); - let merged = higher.combine(lower); - assert!(matches!( - merged.file.run_goal_layer(), - Some(RunGoalLayer::File { .. }) - )); - } - - #[test] - fn load_rewrites_relative_goal_file_to_absolute() { - let tmp = tempfile::tempdir().unwrap(); - let config_path = tmp.path().join("fabro.toml"); - std::fs::write( - &config_path, - r#" -_version = 1 -[run.goal] -file = "prompts/goal.md" -"#, - ) - .unwrap(); - - let layer = ConfigLayer::load(&config_path).unwrap(); - let Some(RunGoalLayer::File { file }) = layer.file.run_goal_layer() else { - panic!("expected file variant"); - }; - let resolved = file.as_source(); - let expected = tmp.path().join("prompts").join("goal.md"); - assert_eq!(resolved, expected.to_string_lossy()); - } - - #[test] - fn load_leaves_absolute_goal_file_untouched() { - let tmp = tempfile::tempdir().unwrap(); - let config_path = tmp.path().join("fabro.toml"); - let abs_goal = "/etc/fabro/goal.md"; - std::fs::write( - &config_path, - format!( - r#" -_version = 1 -[run.goal] -file = "{abs_goal}" -"# - ), - ) - .unwrap(); - - let layer = ConfigLayer::load(&config_path).unwrap(); - let Some(RunGoalLayer::File { file }) = layer.file.run_goal_layer() else { - panic!("expected file variant"); - }; - assert_eq!(file.as_source(), abs_goal); - } - - #[test] - fn load_leaves_env_interpolated_goal_file_untouched() { - // InterpString paths aren't resolved at load time because env - // lookups happen at consume time. The loader should leave them - // alone. - let tmp = tempfile::tempdir().unwrap(); - let config_path = tmp.path().join("fabro.toml"); - std::fs::write( - &config_path, - r#" -_version = 1 -[run.goal] -file = "${env.GOALS_DIR}/goal.md" -"#, - ) - .unwrap(); - - let layer = ConfigLayer::load(&config_path).unwrap(); - let Some(RunGoalLayer::File { file }) = layer.file.run_goal_layer() else { - panic!("expected file variant"); - }; - assert_eq!(file.as_source(), "${env.GOALS_DIR}/goal.md"); - } -} diff --git a/lib/crates/fabro-config/src/effective_settings.rs b/lib/crates/fabro-config/src/effective_settings.rs index c9f59fde6..a8d4a2a86 100644 --- a/lib/crates/fabro-config/src/effective_settings.rs +++ b/lib/crates/fabro-config/src/effective_settings.rs @@ -11,7 +11,6 @@ use fabro_types::settings::SettingsFile; use fabro_types::settings::run::{RunExecutionLayer, RunLayer}; use fabro_types::settings::server::ServerLayer; -use crate::ConfigLayer; use crate::merge::combine_files; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -23,19 +22,19 @@ pub enum EffectiveSettingsMode { #[derive(Clone, Debug, Default)] pub struct EffectiveSettingsLayers { - pub args: ConfigLayer, - pub workflow: ConfigLayer, - pub project: ConfigLayer, - pub user: ConfigLayer, + pub args: SettingsFile, + pub workflow: SettingsFile, + pub project: SettingsFile, + pub user: SettingsFile, } impl EffectiveSettingsLayers { #[must_use] pub fn new( - args: ConfigLayer, - workflow: ConfigLayer, - project: ConfigLayer, - user: ConfigLayer, + args: SettingsFile, + workflow: SettingsFile, + project: SettingsFile, + user: SettingsFile, ) -> Self { Self { args, @@ -60,9 +59,10 @@ pub fn resolve_settings( } = layers; match mode { - EffectiveSettingsMode::LocalOnly => { - Ok(args.combine(workflow).combine(project).combine(user).into()) - } + EffectiveSettingsMode::LocalOnly => Ok(combine_files( + combine_files(combine_files(user, project), workflow), + args, + )), EffectiveSettingsMode::RemoteServer | EffectiveSettingsMode::LocalDaemon => { let server_settings = server_settings.ok_or_else(|| { anyhow!("server settings are required for server-targeted settings resolution") @@ -70,13 +70,13 @@ pub fn resolve_settings( // Owner-specific domains (cli, server) may only come from the // local ~/.fabro/settings.toml, never from fabro.toml or // workflow.toml. The user layer keeps its cli/server fields. - strip_owner_domains(workflow.as_v2_mut()); - strip_owner_domains(project.as_v2_mut()); + strip_owner_domains(&mut workflow); + strip_owner_domains(&mut project); let server_defaults = server_defaults_file(server_settings); - let combined: SettingsFile = - args.combine(workflow).combine(project).combine(user).into(); + let combined = + combine_files(combine_files(combine_files(user, project), workflow), args); let mut settings = match mode { EffectiveSettingsMode::RemoteServer => { @@ -179,20 +179,20 @@ fn apply_local_daemon_overrides(mut settings: SettingsFile, server: &SettingsFil mod tests { use fabro_types::settings::InterpString; use fabro_types::settings::server::{ServerLayer, ServerSchedulerLayer, ServerStorageLayer}; + use fabro_types::settings::{SettingsFile, parse_settings_file}; use super::{EffectiveSettingsLayers, EffectiveSettingsMode, resolve_settings}; - use crate::ConfigLayer; - fn layer(source: &str) -> ConfigLayer { - ConfigLayer::parse(source).expect("v2 fixture should parse") + fn layer(source: &str) -> SettingsFile { + parse_settings_file(source).expect("v2 fixture should parse") } #[test] fn local_only_merges_project_and_user_layers() { let settings = resolve_settings( EffectiveSettingsLayers::new( - ConfigLayer::default(), - ConfigLayer::default(), + SettingsFile::default(), + SettingsFile::default(), layer( r#" _version = 1 @@ -227,13 +227,23 @@ shared = "user" .unwrap(); assert_eq!( - settings.run_model_name_str().as_deref(), + settings + .run + .as_ref() + .and_then(|run| run.model.as_ref()) + .and_then(|model| model.name.as_ref()) + .map(|value| value.as_source()) + .as_deref(), Some("project-model") ); // Per R22, run.inputs replaces wholesale — the winning layer is the // highest-precedence layer that sets `inputs` (project here, since it // wins over user). - let inputs = settings.run_inputs().unwrap(); + let inputs = settings + .run + .as_ref() + .and_then(|run| run.inputs.as_ref()) + .unwrap(); assert!(inputs.contains_key("project_only")); assert_eq!( inputs.get("shared").and_then(|v| v.as_str()), @@ -249,7 +259,7 @@ shared = "user" fn local_only_merges_workflow_project_user() { let settings = resolve_settings( EffectiveSettingsLayers::new( - ConfigLayer::default(), + SettingsFile::default(), layer( r#" _version = 1 @@ -284,14 +294,35 @@ provider = "openai" .unwrap(); assert_eq!( - settings.run_goal_inline_str().as_deref(), + match settings.run.as_ref().and_then(|run| run.goal.as_ref()) { + Some(fabro_types::settings::run::RunGoalLayer::Inline(value)) => { + Some(value.as_source()) + } + _ => None, + } + .as_deref(), Some("workflow goal") ); assert_eq!( - settings.run_model_name_str().as_deref(), + settings + .run + .as_ref() + .and_then(|run| run.model.as_ref()) + .and_then(|model| model.name.as_ref()) + .map(|value| value.as_source()) + .as_deref(), Some("workflow-model") ); - assert_eq!(settings.run_model_provider_str().as_deref(), Some("openai")); + assert_eq!( + settings + .run + .as_ref() + .and_then(|run| run.model.as_ref()) + .and_then(|model| model.provider.as_ref()) + .map(|value| value.as_source()) + .as_deref(), + Some("openai") + ); } #[test] @@ -321,10 +352,10 @@ root = "/tmp/should-be-inert" let settings = resolve_settings( EffectiveSettingsLayers::new( - ConfigLayer::default(), - ConfigLayer::default(), + SettingsFile::default(), + SettingsFile::default(), project_with_server, - ConfigLayer::default(), + SettingsFile::default(), ), Some(&server_settings), EffectiveSettingsMode::RemoteServer, @@ -332,11 +363,23 @@ root = "/tmp/should-be-inert" .unwrap(); assert_eq!( - settings.server_storage_root_str().as_deref(), + settings + .server + .as_ref() + .and_then(|server| server.storage.as_ref()) + .and_then(|storage| storage.root.as_ref()) + .map(|value| value.as_source()) + .as_deref(), Some("/srv/fabro") ); assert_eq!( - settings.run_goal_inline_str().as_deref(), + match settings.run.as_ref().and_then(|run| run.goal.as_ref()) { + Some(fabro_types::settings::run::RunGoalLayer::Inline(value)) => { + Some(value.as_source()) + } + _ => None, + } + .as_deref(), Some("project goal") ); } @@ -362,9 +405,22 @@ root = "/tmp/should-be-inert" .unwrap(); assert_eq!( - settings.server_storage_root_str().as_deref(), + settings + .server + .as_ref() + .and_then(|server| server.storage.as_ref()) + .and_then(|storage| storage.root.as_ref()) + .map(|value| value.as_source()) + .as_deref(), Some("/srv/fabro") ); - assert_eq!(settings.max_concurrent_runs(), Some(7)); + assert_eq!( + settings + .server + .as_ref() + .and_then(|server| server.scheduler.as_ref()) + .and_then(|scheduler| scheduler.max_concurrent_runs), + Some(7) + ); } } diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index efe3d6c9a..e40bdfe2e 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -1,9 +1,9 @@ extern crate self as fabro_config; -pub mod config; pub mod effective_settings; pub mod home; pub mod legacy_env; +pub mod load; pub mod merge; pub mod project; pub mod resolve; @@ -11,25 +11,40 @@ pub mod run; pub mod storage; pub mod user; -pub use config::ConfigLayer; pub use fabro_util::path::expand_tilde; pub use home::Home; +pub use load::{ + load_settings_for_workflow, load_settings_path, load_settings_project, load_settings_user, +}; pub use resolve::{ - ResolveError, resolve_cli, resolve_cli_from_file, resolve_features, resolve_features_from_file, - resolve_project, resolve_project_from_file, resolve_run, resolve_run_from_file, resolve_server, - resolve_server_from_file, resolve_workflow, resolve_workflow_from_file, + ResolveError, resolve, resolve_cli, resolve_cli_from_file, resolve_features, + resolve_features_from_file, resolve_project, resolve_project_from_file, resolve_run, + resolve_run_from_file, resolve_server, resolve_server_from_file, resolve_workflow, + resolve_workflow_from_file, }; pub use storage::{RunScratch, ServerState, Storage}; -use std::path::{Path, PathBuf}; +use std::path::Path; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{Settings, SettingsFile}; use serde::de::DeserializeOwned; -/// Resolve the storage directory: v2 `server.storage.root` > home default. -#[must_use] -pub fn resolve_storage_dir(settings: &SettingsFile) -> PathBuf { - settings.storage_dir() +pub fn load_and_resolve( + layers: effective_settings::EffectiveSettingsLayers, + server_settings: Option<&SettingsFile>, + mode: effective_settings::EffectiveSettingsMode, +) -> anyhow::Result { + let layer = effective_settings::resolve_settings(layers, server_settings, mode)?; + resolve(&layer).map_err(|errors| { + anyhow::anyhow!( + "failed to resolve settings:\n{}", + errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("\n") + ) + }) } /// Load a TOML config from an explicit path or `~/.fabro/{filename}`. diff --git a/lib/crates/fabro-config/src/load.rs b/lib/crates/fabro-config/src/load.rs new file mode 100644 index 000000000..5667ae7a4 --- /dev/null +++ b/lib/crates/fabro-config/src/load.rs @@ -0,0 +1,79 @@ +use std::path::{Path, PathBuf}; + +use anyhow::Context; +use fabro_types::settings::run::RunGoalLayer; +use fabro_types::settings::{InterpString, SettingsFile, parse_settings_file}; + +use crate::merge::combine_files; +use crate::project; +use crate::user; + +pub fn load_settings_path(path: &Path) -> anyhow::Result { + let content = std::fs::read_to_string(path) + .with_context(|| format!("Failed to read {}", path.display()))?; + let mut layer = parse_settings_file(&content) + .map_err(|err| anyhow::anyhow!("{err}")) + .context("Failed to parse settings file")?; + let base_dir = path.parent().unwrap_or_else(|| Path::new(".")); + resolve_goal_file_paths(&mut layer, base_dir); + Ok(layer) +} + +pub fn load_settings_for_workflow(path: &Path, cwd: &Path) -> anyhow::Result { + let resolution = project::resolve_workflow_path(path, cwd)?; + if resolution.workflow_config.is_none() && !resolution.resolved_workflow_path.is_file() { + anyhow::bail!( + "Workflow not found: {}", + resolution.resolved_workflow_path.display() + ); + } + + let workflow_config = resolution.workflow_config.unwrap_or_default(); + let project_config = project::discover_project_config( + resolution + .resolved_workflow_path + .parent() + .unwrap_or_else(|| Path::new(".")), + )? + .map(|(_, config)| config) + .unwrap_or_default(); + + Ok(combine_files(project_config, workflow_config)) +} + +pub fn load_settings_project(start: &Path) -> anyhow::Result { + Ok(project::discover_project_config(start)? + .map(|(_, config)| config) + .unwrap_or_default()) +} + +pub fn load_settings_user() -> anyhow::Result { + user::load_settings_config(None) +} + +pub(crate) fn resolve_goal_file_paths(file: &mut SettingsFile, base_dir: &Path) { + let Some(run) = file.run.as_mut() else { + return; + }; + let Some(RunGoalLayer::File { file: goal_file }) = run.goal.as_mut() else { + return; + }; + if !goal_file.is_literal() { + return; + } + let literal = goal_file.as_source(); + if Path::new(&literal).is_absolute() { + return; + } + let absolute = resolve_goal_file_path(&literal, base_dir); + *goal_file = InterpString::parse(&absolute.to_string_lossy()); +} + +pub(crate) fn resolve_goal_file_path(path_str: &str, base_dir: &Path) -> PathBuf { + let path = Path::new(path_str); + if path.is_absolute() { + path.to_path_buf() + } else { + base_dir.join(path) + } +} diff --git a/lib/crates/fabro-config/src/project.rs b/lib/crates/fabro-config/src/project.rs index d6015a5e8..fcdcab7e9 100644 --- a/lib/crates/fabro-config/src/project.rs +++ b/lib/crates/fabro-config/src/project.rs @@ -10,33 +10,35 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, bail}; use serde::Serialize; -use crate::config::ConfigLayer; +use crate::load::load_settings_path; use crate::run; use crate::{resolve_project_from_file, resolve_run_from_file, resolve_workflow_from_file}; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{SettingsFile, parse_settings_file}; const CONFIG_FILENAME: &str = "fabro.toml"; #[derive(Clone, Debug)] pub struct WorkflowPathResolution { pub resolved_workflow_path: PathBuf, pub dot_path: PathBuf, - pub workflow_config: Option, + pub workflow_config: Option, pub workflow_toml_path: Option, pub workflow_slug: Option, } /// Parse a project config from a TOML string. -pub fn parse_project_config(content: &str) -> anyhow::Result { - ConfigLayer::parse(content).context("Failed to parse project config") +pub fn parse_project_config(content: &str) -> anyhow::Result { + parse_settings_file(content) + .map_err(|err| anyhow::anyhow!("{err}")) + .context("Failed to parse project config") } /// Load a project config from a file path. /// -/// Goes through [`ConfigLayer::load`] so that relative `run.goal.file` +/// Goes through [`load_settings_path`] so that relative `run.goal.file` /// 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 = resolve_project_from_file(config.as_v2()) +pub fn load_project_config(path: &Path) -> anyhow::Result { + let config = load_settings_path(path).context("Failed to parse project config")?; + let root = resolve_project_from_file(&config) .map_err(|errors| anyhow::anyhow!("Failed to resolve project settings: {errors:?}"))? .directory; tracing::debug!(path = %path.display(), root = %root, "Loaded project config"); @@ -45,7 +47,7 @@ pub fn load_project_config(path: &Path) -> anyhow::Result { /// Walk ancestor directories from `start` looking for `fabro.toml`. /// Returns the config file path and parsed config, or `None` if not found. -pub fn discover_project_config(start: &Path) -> anyhow::Result> { +pub fn discover_project_config(start: &Path) -> anyhow::Result> { for ancestor in start.ancestors() { let candidate = ancestor.join(CONFIG_FILENAME); if candidate.is_file() { @@ -90,7 +92,7 @@ pub fn resolve_workflow_path( if path.extension().is_some_and(|ext| ext == "toml") { match run::load_run_config(&path) { Ok(cfg) => { - let workflow = resolve_workflow_from_file(cfg.as_v2()).map_err(|errors| { + let workflow = resolve_workflow_from_file(&cfg).map_err(|errors| { anyhow::anyhow!("Failed to resolve workflow settings: {errors:?}") })?; let dot_path = run::resolve_graph_path(&path, &workflow.graph); @@ -336,7 +338,7 @@ fn find_closest_match(input: &str, candidates: &[String]) -> Option { } /// Resolve a workflow argument to a DOT path and optional run config. -pub fn resolve_workflow(arg: &Path) -> anyhow::Result<(PathBuf, Option)> { +pub fn resolve_workflow(arg: &Path) -> anyhow::Result<(PathBuf, Option)> { let start = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let resolution = resolve_workflow_path(arg, &start)?; Ok((resolution.dot_path, resolution.workflow_config)) @@ -348,7 +350,6 @@ pub fn is_retro_enabled() -> bool { let start = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); match discover_project_config(&start) { Ok(Some((_path, config))) => config - .as_v2() .run .as_ref() .and_then(|r| r.execution.as_ref()) @@ -361,11 +362,11 @@ pub fn is_retro_enabled() -> bool { /// Resolve the fabro root directory from a config file path and its config. /// The returned path is the directory containing `fabro.toml` joined with the /// `project.directory` value (default: `fabro/`). -pub fn resolve_fabro_root(config_path: &Path, config: &ConfigLayer) -> PathBuf { +pub fn resolve_fabro_root(config_path: &Path, config: &SettingsFile) -> PathBuf { let project_dir = config_path .parent() .expect("config_path should have a parent directory"); - let root = resolve_project_from_file(config.as_v2()) + let root = resolve_project_from_file(config) .expect("project settings should resolve") .directory; project_dir.join(root) @@ -380,8 +381,8 @@ mod tests { #[test] fn parse_minimal_config() { let config = parse_project_config("_version = 1\n").unwrap(); - assert_eq!(config.as_v2().version, Some(1)); - assert!(config.as_v2().project.is_none()); + assert_eq!(config.version, Some(1)); + assert!(config.project.is_none()); } #[test] @@ -396,7 +397,7 @@ directory = "fabro/" ) .unwrap(); assert_eq!( - resolve_project_from_file(config.as_v2()).unwrap().directory, + resolve_project_from_file(&config).unwrap().directory, "fabro/" ); } @@ -414,7 +415,6 @@ retros = true .unwrap(); assert_eq!( config - .as_v2() .run .as_ref() .and_then(|r| r.execution.as_ref()) @@ -453,7 +453,7 @@ retros = true let path = tmp.path().join("fabro.toml"); fs::write(&path, "_version = 1\n").unwrap(); let config = load_project_config(&path).unwrap(); - assert_eq!(config.as_v2().version, Some(1)); + assert_eq!(config.version, Some(1)); } #[test] @@ -465,7 +465,7 @@ retros = true let (found_path, config) = discover_project_config(&sub).unwrap().unwrap(); assert_eq!(found_path, tmp.path().join("fabro.toml")); - assert_eq!(config.as_v2().version, Some(1)); + assert_eq!(config.version, Some(1)); } #[test] @@ -485,7 +485,9 @@ file = "prompts/goal.md" .unwrap(); let config = load_project_config(&path).unwrap(); - let Some(RunGoalLayer::File { file }) = config.as_v2().run_goal_layer() else { + let Some(RunGoalLayer::File { file }) = + config.run.as_ref().and_then(|run| run.goal.as_ref()) + else { panic!("expected file variant"); }; let expected = tmp.path().join("prompts").join("goal.md"); diff --git a/lib/crates/fabro-config/src/resolve/mod.rs b/lib/crates/fabro-config/src/resolve/mod.rs index 7fa33ade5..e4f2ce0e3 100644 --- a/lib/crates/fabro-config/src/resolve/mod.rs +++ b/lib/crates/fabro-config/src/resolve/mod.rs @@ -7,8 +7,8 @@ mod server; mod workflow; use fabro_types::settings::{ - CliSettings, FeaturesSettings, ProjectSettings, RunSettings, ServerSettings, SettingsFile, - WorkflowSettings, + CliSettings, FeaturesSettings, InterpString, ProjectSettings, RunSettings, ServerSettings, + Settings, SettingsFile, WorkflowSettings, }; pub use cli::resolve_cli; @@ -19,93 +19,76 @@ pub use run::resolve_run; pub use server::resolve_server; pub use workflow::resolve_workflow; -pub fn resolve_cli_from_file(file: &SettingsFile) -> Result> { +pub fn resolve(file: &SettingsFile) -> Result> { let mut errors = Vec::new(); - let layer = file.cli.as_ref().cloned().unwrap_or_default(); - let resolved = resolve_cli(&layer, &mut errors); + let project_layer = file.project.clone().unwrap_or_default(); + let workflow_layer = file.workflow.clone().unwrap_or_default(); + let run_layer = file.run.clone().unwrap_or_default(); + let cli_layer = file.cli.clone().unwrap_or_default(); + let server_layer = file.server.clone().unwrap_or_default(); + let features_layer = file.features.clone().unwrap_or_default(); + + let settings = Settings { + project: resolve_project(&project_layer, &mut errors), + workflow: resolve_workflow(&workflow_layer, &mut errors), + run: resolve_run(&run_layer, &mut errors), + cli: resolve_cli(&cli_layer, &mut errors), + server: resolve_server(&server_layer, &mut errors), + features: resolve_features(&features_layer, &mut errors), + }; + if errors.is_empty() { - Ok(resolved) + Ok(settings) } else { Err(errors) } } +pub fn resolve_cli_from_file(file: &SettingsFile) -> Result> { + resolve(file).map(|settings| settings.cli) +} + pub fn resolve_server_from_file(file: &SettingsFile) -> Result> { - let mut errors = Vec::new(); - let layer = file.server.as_ref().cloned().unwrap_or_default(); - let resolved = resolve_server(&layer, &mut errors); - if errors.is_empty() { - Ok(resolved) - } else { - Err(errors) - } + resolve(file).map(|settings| settings.server) } pub fn resolve_project_from_file( file: &SettingsFile, ) -> 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) - } + resolve(file).map(|settings| settings.project) } pub fn resolve_features_from_file( file: &SettingsFile, ) -> Result> { - let mut errors = Vec::new(); - let layer = file.features.as_ref().cloned().unwrap_or_default(); - let resolved = resolve_features(&layer, &mut errors); - if errors.is_empty() { - Ok(resolved) - } else { - Err(errors) - } + resolve(file).map(|settings| settings.features) } pub fn resolve_run_from_file(file: &SettingsFile) -> Result> { - let mut errors = Vec::new(); - let layer = file.run.as_ref().cloned().unwrap_or_default(); - let resolved = resolve_run(&layer, &mut errors); - if errors.is_empty() { - Ok(resolved) - } else { - Err(errors) - } + resolve(file).map(|settings| settings.run) } pub fn resolve_workflow_from_file( file: &SettingsFile, ) -> Result> { - let mut errors = Vec::new(); - let layer = file.workflow.as_ref().cloned().unwrap_or_default(); - let resolved = resolve_workflow(&layer, &mut errors); - if errors.is_empty() { - Ok(resolved) - } else { - Err(errors) - } + resolve(file).map(|settings| settings.workflow) } pub(crate) fn require_interp( - value: Option<&fabro_types::settings::InterpString>, + value: Option<&InterpString>, path: &str, errors: &mut Vec, -) -> fabro_types::settings::InterpString { +) -> InterpString { value.cloned().unwrap_or_else(|| { errors.push(ResolveError::Missing { path: path.to_string(), }); - fabro_types::settings::InterpString::parse("") + InterpString::parse("") }) } pub(crate) fn parse_socket_addr( - value: &fabro_types::settings::InterpString, + value: &InterpString, path: &str, errors: &mut Vec, ) -> std::net::SocketAddr { @@ -122,8 +105,6 @@ pub(crate) fn parse_socket_addr( } } -pub(crate) fn default_interp( - path: impl AsRef, -) -> fabro_types::settings::InterpString { - fabro_types::settings::InterpString::parse(&path.as_ref().to_string_lossy()) +pub(crate) fn default_interp(path: impl AsRef) -> InterpString { + InterpString::parse(&path.as_ref().to_string_lossy()) } diff --git a/lib/crates/fabro-config/src/resolve/run.rs b/lib/crates/fabro-config/src/resolve/run.rs index 264298f72..d228b69fe 100644 --- a/lib/crates/fabro-config/src/resolve/run.rs +++ b/lib/crates/fabro-config/src/resolve/run.rs @@ -1,15 +1,16 @@ use fabro_types::settings::InterpString; use fabro_types::settings::run::{ - ApprovalMode, ArtifactsSettings, DaytonaDockerfileLayer, DaytonaSettings, + ApprovalMode, ArtifactsSettings, DaytonaDockerfileLayer, DaytonaSandboxLayer, DaytonaSettings, DaytonaSnapshotSettings, DockerfileSource, GitAuthorSettings, HookAgentMarker, HookDefinition, - HookTlsMode, HookType, InterviewProviderSettings, McpEntryLayer, McpServerSettings, - McpTransport, MergeStrategy, ModelRefOrSplice, NotificationProviderSettings, + HookEntry, HookTlsMode, HookType, InterviewProviderLayer, InterviewProviderSettings, + InterviewsLayer, LocalSandboxSettings, McpEntryLayer, McpServerSettings, McpTransport, + MergeStrategy, ModelRefOrSplice, NotificationProviderLayer, NotificationProviderSettings, NotificationRouteLayer, NotificationRouteSettings, PullRequestSettings, RunAgentLayer, RunAgentSettings, RunArtifactsLayer, RunCheckpointLayer, RunCheckpointSettings, RunExecutionLayer, RunExecutionSettings, RunGitLayer, RunGitSettings, RunGoal, RunGoalLayer, RunInterviewsSettings, RunLayer, RunMode, RunModelLayer, RunModelSettings, RunPrepareLayer, - RunPrepareSettings, RunSandboxLayer, RunSandboxSettings, RunScmLayer, RunScmSettings, - RunSettings, ScmGitHubSettings, StringOrSplice, TlsMode, + RunPrepareSettings, RunPullRequestLayer, RunSandboxLayer, RunSandboxSettings, RunScmLayer, + RunScmSettings, RunSettings, ScmGitHubSettings, StringOrSplice, TlsMode, }; use super::ResolveError; @@ -165,10 +166,8 @@ fn resolve_sandbox( } } -fn resolve_local_sandbox( - sandbox: &RunSandboxLayer, -) -> fabro_types::settings::run::LocalSandboxSettings { - fabro_types::settings::run::LocalSandboxSettings { +fn resolve_local_sandbox(sandbox: &RunSandboxLayer) -> LocalSandboxSettings { + LocalSandboxSettings { worktree_mode: sandbox .local .as_ref() @@ -177,7 +176,7 @@ fn resolve_local_sandbox( } } -fn resolve_daytona(daytona: &fabro_types::settings::run::DaytonaSandboxLayer) -> DaytonaSettings { +fn resolve_daytona(daytona: &DaytonaSandboxLayer) -> DaytonaSettings { DaytonaSettings { auto_stop_interval: daytona.auto_stop_interval, labels: daytona.labels.clone(), @@ -224,16 +223,14 @@ fn resolve_notification_route(route: &NotificationRouteLayer) -> NotificationRou } fn resolve_notification_provider( - provider: &fabro_types::settings::run::NotificationProviderLayer, + provider: &NotificationProviderLayer, ) -> NotificationProviderSettings { NotificationProviderSettings { channel: provider.channel.clone(), } } -fn resolve_interviews( - interviews: Option<&fabro_types::settings::run::InterviewsLayer>, -) -> RunInterviewsSettings { +fn resolve_interviews(interviews: Option<&InterviewsLayer>) -> RunInterviewsSettings { let Some(interviews) = interviews else { return RunInterviewsSettings::default(); }; @@ -246,9 +243,7 @@ fn resolve_interviews( } } -fn resolve_interview_provider( - provider: &fabro_types::settings::run::InterviewProviderLayer, -) -> InterviewProviderSettings { +fn resolve_interview_provider(provider: &InterviewProviderLayer) -> InterviewProviderSettings { InterviewProviderSettings { channel: provider.channel.clone(), } @@ -347,11 +342,7 @@ fn resolve_mcp_command( .unwrap_or_default() } -fn resolve_hook( - hook: &fabro_types::settings::run::HookEntry, - index: usize, - errors: &mut Vec, -) -> HookDefinition { +fn resolve_hook(hook: &HookEntry, index: usize, errors: &mut Vec) -> HookDefinition { let variants = [ hook.script.is_some() || hook.command.is_some(), hook.url.is_some(), @@ -396,7 +387,7 @@ fn resolve_hook( } } -fn resolve_hook_type(hook: &fabro_types::settings::run::HookEntry) -> Option { +fn resolve_hook_type(hook: &HookEntry) -> Option { if hook.script.is_some() || hook.command.is_some() { return None; } @@ -457,9 +448,7 @@ fn resolve_scm(scm: Option<&RunScmLayer>) -> RunScmSettings { } } -fn resolve_pull_request( - pull_request: Option<&fabro_types::settings::run::RunPullRequestLayer>, -) -> Option { +fn resolve_pull_request(pull_request: Option<&RunPullRequestLayer>) -> Option { let pull_request = pull_request?; if !pull_request.enabled.unwrap_or(false) { return None; diff --git a/lib/crates/fabro-config/src/resolve/server.rs b/lib/crates/fabro-config/src/resolve/server.rs index 7db2d8947..4ca844f74 100644 --- a/lib/crates/fabro-config/src/resolve/server.rs +++ b/lib/crates/fabro-config/src/resolve/server.rs @@ -3,14 +3,16 @@ use std::time::Duration; use fabro_types::settings::InterpString; use fabro_types::settings::server::{ DiscordIntegrationSettings, GithubIntegrationSettings, GithubOauthSettings, - IntegrationWebhooksSettings, ObjectStoreProvider, ObjectStoreSettings, ServerApiLayer, - ServerApiSettings, ServerArtifactsLayer, ServerArtifactsSettings, ServerAuthApiJwtSettings, - ServerAuthApiMtlsSettings, ServerAuthApiSettings, ServerAuthLayer, ServerAuthSettings, - ServerAuthWebGithubLayer, ServerAuthWebProvidersSettings, ServerAuthWebSettings, - ServerIntegrationsLayer, ServerIntegrationsSettings, ServerLayer, ServerListenLayer, - ServerListenSettings, ServerListenTlsLayer, ServerLoggingSettings, ServerSchedulerSettings, - ServerSettings, ServerSlateDbLayer, ServerSlateDbSettings, ServerStorageSettings, - ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, TlsConfig, + IntegrationWebhooksSettings, ObjectStoreLocalLayer, ObjectStoreProvider, ObjectStoreS3Layer, + ObjectStoreSettings, ServerApiLayer, ServerApiSettings, ServerArtifactsLayer, + ServerArtifactsSettings, ServerAuthApiJwtSettings, ServerAuthApiMtlsSettings, + ServerAuthApiSettings, ServerAuthLayer, ServerAuthSettings, ServerAuthWebGithubLayer, + ServerAuthWebProvidersSettings, ServerAuthWebSettings, ServerIntegrationsLayer, + ServerIntegrationsSettings, ServerLayer, ServerListenLayer, ServerListenSettings, + ServerListenTlsLayer, ServerLoggingSettings, ServerSchedulerSettings, ServerSettings, + ServerSlateDbLayer, ServerSlateDbSettings, ServerStorageLayer, ServerStorageSettings, + ServerWebLayer, ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, + TlsConfig, }; use fabro_util::Home; @@ -49,9 +51,7 @@ pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> Se } } -fn resolve_storage( - layer: Option<&fabro_types::settings::server::ServerStorageLayer>, -) -> ServerStorageSettings { +fn resolve_storage(layer: Option<&ServerStorageLayer>) -> ServerStorageSettings { ServerStorageSettings { root: layer .and_then(|storage| storage.root.clone()) @@ -106,10 +106,7 @@ fn resolve_tls( (Some(TlsConfig { cert, key, ca }), valid) } -fn resolve_web( - _api: Option<&ServerApiLayer>, - layer: Option<&fabro_types::settings::server::ServerWebLayer>, -) -> ServerWebSettings { +fn resolve_web(_api: Option<&ServerApiLayer>, layer: Option<&ServerWebLayer>) -> ServerWebSettings { ServerWebSettings { enabled: layer.and_then(|web| web.enabled).unwrap_or(true), url: layer @@ -217,15 +214,14 @@ fn resolve_slatedb( ), flush_interval: layer .and_then(|slatedb| slatedb.flush_interval) - .map(|duration| duration.as_std()) - .unwrap_or_else(|| Duration::from_millis(1)), + .map_or_else(|| Duration::from_millis(1), |duration| duration.as_std()), } } fn resolve_object_store( provider: ObjectStoreProvider, - local: Option<&fabro_types::settings::server::ObjectStoreLocalLayer>, - s3: Option<&fabro_types::settings::server::ObjectStoreS3Layer>, + local: Option<&ObjectStoreLocalLayer>, + s3: Option<&ObjectStoreS3Layer>, storage_root: &InterpString, path_prefix: &str, errors: &mut Vec, diff --git a/lib/crates/fabro-config/src/run.rs b/lib/crates/fabro-config/src/run.rs index 33fac41c4..5ec9a3e18 100644 --- a/lib/crates/fabro-config/src/run.rs +++ b/lib/crates/fabro-config/src/run.rs @@ -1,6 +1,6 @@ //! Workflow / run config loading helpers. //! -//! Thin wrappers around `ConfigLayer::parse` / `ConfigLayer::load` plus +//! Thin wrappers around `parse_settings_file` / `load_settings_path` plus //! path resolution for the `[workflow] graph` override. Runtime types //! that used to be re-exported from here live under //! `fabro_types::settings::run` now. @@ -9,19 +9,23 @@ use std::path::{Path, PathBuf}; use anyhow::Context; -use crate::config::ConfigLayer; +use crate::load::{load_settings_path, resolve_goal_file_path}; +use fabro_types::settings::run::{ResolvedGoalSource, ResolvedRunGoal, RunGoalLayer}; +use fabro_types::settings::{SettingsFile, parse_settings_file}; /// Load and parse a run config from a TOML file. -pub fn parse_run_config(contents: &str) -> anyhow::Result { - ConfigLayer::parse(contents).context("Failed to parse run config TOML") +pub fn parse_run_config(contents: &str) -> anyhow::Result { + parse_settings_file(contents) + .map_err(|err| anyhow::anyhow!("{err}")) + .context("Failed to parse run config TOML") } /// Load and parse a run config from a TOML file. /// -/// Goes through [`ConfigLayer::load`] so that relative `run.goal.file` +/// Goes through [`load_settings_path`] so that relative `run.goal.file` /// paths are anchored at the directory of `path` at load time. -pub fn load_run_config(path: &Path) -> anyhow::Result { - ConfigLayer::load(path) +pub fn load_run_config(path: &Path) -> anyhow::Result { + load_settings_path(path) .with_context(|| format!("Failed to parse workflow config at {}", path.display())) } @@ -34,6 +38,71 @@ pub fn resolve_graph_path(workflow_toml: &Path, graph_relative: &str) -> PathBuf .join(graph_relative) } +#[derive(Debug)] +pub enum ResolveRunGoalError { + EnvLookup { + var: String, + }, + Io { + path: PathBuf, + source: std::io::Error, + }, +} + +impl std::fmt::Display for ResolveRunGoalError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::EnvLookup { var } => write!( + f, + "run.goal.file references env var `{var}` which is not set" + ), + Self::Io { path, source } => { + write!(f, "failed to read goal file {}: {source}", path.display()) + } + } + } +} + +impl std::error::Error for ResolveRunGoalError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::EnvLookup { .. } => None, + Self::Io { source, .. } => Some(source), + } + } +} + +pub fn resolve_run_goal( + settings: &SettingsFile, + base_dir: &Path, +) -> Result, ResolveRunGoalError> { + let Some(goal) = settings.run.as_ref().and_then(|run| run.goal.as_ref()) else { + return Ok(None); + }; + + match goal { + RunGoalLayer::Inline(text) => Ok(Some(ResolvedRunGoal { + text: text.as_source(), + source: ResolvedGoalSource::Inline, + })), + RunGoalLayer::File { file } => { + let resolved = file + .resolve(|name| std::env::var(name).ok()) + .map_err(|err| ResolveRunGoalError::EnvLookup { var: err.name })?; + let path = resolve_goal_file_path(&resolved.value, base_dir); + let text = + std::fs::read_to_string(&path).map_err(|source| ResolveRunGoalError::Io { + path: path.clone(), + source, + })?; + Ok(Some(ResolvedRunGoal { + text, + source: ResolvedGoalSource::File { path }, + })) + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -56,7 +125,9 @@ file = "prompts/goal.md" .unwrap(); let config = load_run_config(&workflow_toml).unwrap(); - let Some(RunGoalLayer::File { file }) = config.as_v2().run_goal_layer() else { + let Some(RunGoalLayer::File { file }) = + config.run.as_ref().and_then(|run| run.goal.as_ref()) + else { panic!("expected file variant"); }; let expected = workflow_dir.join("prompts").join("goal.md"); @@ -78,7 +149,9 @@ file = "/etc/fabro/goal.md" .unwrap(); let config = load_run_config(&workflow_toml).unwrap(); - let Some(RunGoalLayer::File { file }) = config.as_v2().run_goal_layer() else { + let Some(RunGoalLayer::File { file }) = + config.run.as_ref().and_then(|run| run.goal.as_ref()) + else { panic!("expected file variant"); }; assert_eq!(file.as_source(), "/etc/fabro/goal.md"); diff --git a/lib/crates/fabro-config/src/user.rs b/lib/crates/fabro-config/src/user.rs index a979cf93d..b48d90fea 100644 --- a/lib/crates/fabro-config/src/user.rs +++ b/lib/crates/fabro-config/src/user.rs @@ -8,8 +8,9 @@ use std::collections::HashSet; use std::path::{Path, PathBuf}; use std::sync::{Mutex, OnceLock}; -use crate::config::ConfigLayer; use crate::home::Home; +use crate::load::load_settings_path; +use fabro_types::settings::SettingsFile; pub const SETTINGS_CONFIG_FILENAME: &str = "settings.toml"; pub const LEGACY_USER_CONFIG_FILENAME: &str = "cli.toml"; @@ -68,7 +69,7 @@ fn should_warn_about_legacy_user_config(path: &Path) -> bool { /// returning defaults if the default file doesn't exist. An explicit path that /// doesn't exist is an error. #[allow(clippy::print_stderr)] -pub fn load_settings_config(path: Option<&Path>) -> anyhow::Result { +pub fn load_settings_config(path: Option<&Path>) -> anyhow::Result { if let Some(explicit) = path .map(Path::to_path_buf) .or_else(|| std::env::var_os(FABRO_CONFIG_ENV).map(PathBuf::from)) @@ -98,12 +99,12 @@ pub fn load_settings_config(path: Option<&Path>) -> anyhow::Result if default.is_file() { load_v2_layer_from_path(&default) } else { - Ok(ConfigLayer::default()) + Ok(SettingsFile::default()) } } -fn load_v2_layer_from_path(path: &Path) -> anyhow::Result { - ConfigLayer::load(path) +fn load_v2_layer_from_path(path: &Path) -> anyhow::Result { + load_settings_path(path) } #[cfg(test)] diff --git a/lib/crates/fabro-config/tests/resolve_cli.rs b/lib/crates/fabro-config/tests/resolve_cli.rs index 107e5bd89..6fd7b11c0 100644 --- a/lib/crates/fabro-config/tests/resolve_cli.rs +++ b/lib/crates/fabro-config/tests/resolve_cli.rs @@ -1,7 +1,7 @@ use fabro_config::resolve_cli_from_file; -use fabro_types::settings::SettingsFile; use fabro_types::settings::cli::{CliTargetSettings, OutputFormat, OutputVerbosity}; use fabro_types::settings::run::AgentPermissions; +use fabro_types::settings::{SettingsFile, parse_settings_file}; #[test] fn resolves_cli_defaults_from_empty_settings() { @@ -19,7 +19,7 @@ fn resolves_cli_defaults_from_empty_settings() { #[test] fn resolves_cli_target_exec_and_output_settings() { - let settings: SettingsFile = fabro_config::ConfigLayer::parse( + let settings: SettingsFile = parse_settings_file( r#" _version = 1 @@ -57,8 +57,7 @@ check = false level = "debug" "#, ) - .expect("fixture should parse") - .into(); + .expect("fixture should parse"); let cli = resolve_cli_from_file(&settings).expect("cli settings should resolve"); diff --git a/lib/crates/fabro-config/tests/resolve_features.rs b/lib/crates/fabro-config/tests/resolve_features.rs index 9afb4853a..4d7a5f596 100644 --- a/lib/crates/fabro-config/tests/resolve_features.rs +++ b/lib/crates/fabro-config/tests/resolve_features.rs @@ -1,5 +1,5 @@ use fabro_config::resolve_features_from_file; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{SettingsFile, parse_settings_file}; #[test] fn resolves_features_defaults_from_empty_settings() { @@ -12,7 +12,7 @@ fn resolves_features_defaults_from_empty_settings() { #[test] fn resolves_session_sandboxes_flag() { - let settings: SettingsFile = fabro_config::ConfigLayer::parse( + let settings: SettingsFile = parse_settings_file( r#" _version = 1 @@ -20,8 +20,7 @@ _version = 1 session_sandboxes = true "#, ) - .expect("fixture should parse") - .into(); + .expect("fixture should parse"); let features = resolve_features_from_file(&settings).expect("features should resolve"); diff --git a/lib/crates/fabro-config/tests/resolve_project.rs b/lib/crates/fabro-config/tests/resolve_project.rs index ccc4ff281..975777173 100644 --- a/lib/crates/fabro-config/tests/resolve_project.rs +++ b/lib/crates/fabro-config/tests/resolve_project.rs @@ -1,5 +1,5 @@ use fabro_config::resolve_project_from_file; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{SettingsFile, parse_settings_file}; #[test] fn resolves_project_defaults_from_empty_settings() { @@ -15,7 +15,7 @@ fn resolves_project_defaults_from_empty_settings() { #[test] fn resolves_project_directory_and_metadata() { - let settings: SettingsFile = fabro_config::ConfigLayer::parse( + let settings: SettingsFile = parse_settings_file( r#" _version = 1 @@ -28,8 +28,7 @@ directory = ".fabro" team = "platform" "#, ) - .expect("fixture should parse") - .into(); + .expect("fixture should parse"); let project = resolve_project_from_file(&settings).expect("project settings should resolve"); diff --git a/lib/crates/fabro-config/tests/resolve_root.rs b/lib/crates/fabro-config/tests/resolve_root.rs new file mode 100644 index 000000000..139b835a7 --- /dev/null +++ b/lib/crates/fabro-config/tests/resolve_root.rs @@ -0,0 +1,200 @@ +use fabro_config::effective_settings::{EffectiveSettingsLayers, EffectiveSettingsMode}; +use fabro_types::run::RunRecord; +use fabro_types::run_event::run::RunCreatedProps; +use fabro_types::settings::{SettingsFile, parse_settings_file}; + +fn parse(source: &str) -> SettingsFile { + parse_settings_file(source).expect("fixture should parse") +} + +#[test] +fn resolves_root_settings_defaults() { + let settings = + fabro_config::resolve(&SettingsFile::default()).expect("empty settings should resolve"); + + assert_eq!(settings.project.directory, "fabro/"); + assert_eq!(settings.workflow.graph, "workflow.fabro"); + assert_eq!(settings.run.execution.retros, true); + assert_eq!(settings.cli.updates.check, true); + assert_eq!(settings.server.scheduler.max_concurrent_runs, 5); + assert_eq!(settings.features.session_sandboxes, false); +} + +#[test] +fn resolve_accumulates_errors_across_namespaces() { + let settings = parse( + r#" +_version = 1 + +[server.listen] +type = "tcp" +address = "127.0.0.1:3000" + +[server.listen.tls] +cert = "/tmp/server.pem" + +[server.auth.api.mtls] +enabled = true + +[run.sandbox] +provider = "not-a-provider" +"#, + ); + + let errors = fabro_config::resolve(&settings).expect_err("invalid shape should fail"); + let rendered = errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("\n"); + + assert!(rendered.contains("server.listen.tls.key")); + assert!(rendered.contains("server.listen.tls.ca")); + assert!(rendered.contains("run.sandbox.provider")); +} + +#[test] +fn load_and_resolve_merges_layers_before_resolution() { + let settings = fabro_config::load_and_resolve( + EffectiveSettingsLayers::new( + SettingsFile::default(), + parse( + r#" +_version = 1 + +[workflow] +graph = "graphs/workflow.dot" +"#, + ), + parse( + r#" +_version = 1 + +[project] +directory = ".fabro" +"#, + ), + parse( + r#" +_version = 1 + +[server.storage] +root = "/srv/fabro" + +[run.model] +provider = "openai" +name = "gpt-5" +"#, + ), + ), + None, + EffectiveSettingsMode::LocalOnly, + ) + .expect("layers should load and resolve"); + + assert_eq!(settings.project.directory, ".fabro"); + assert_eq!(settings.workflow.graph, "graphs/workflow.dot"); + assert_eq!(settings.server.storage.root.as_source(), "/srv/fabro"); + assert_eq!( + settings + .run + .model + .provider + .as_ref() + .map(|value| value.as_source()), + Some("openai".to_string()) + ); + assert_eq!( + settings + .run + .model + .name + .as_ref() + .map(|value| value.as_source()), + Some("gpt-5".to_string()) + ); +} + +#[test] +fn run_record_round_trips_templated_settings() { + let settings = parse( + r#" +_version = 1 + +[server.storage] +root = "${env.FABRO_STORAGE}" +"#, + ); + let record = RunRecord { + run_id: fabro_types::fixtures::RUN_1, + settings, + graph: fabro_types::graph::Graph::new("test"), + workflow_slug: Some("demo".to_string()), + working_directory: std::path::PathBuf::from("/tmp/project"), + host_repo_path: Some("/tmp/project".to_string()), + repo_origin_url: None, + base_branch: Some("main".to_string()), + labels: std::collections::HashMap::new(), + provenance: None, + manifest_blob: None, + definition_blob: None, + }; + + let json = serde_json::to_value(&record).expect("record should serialize"); + let round_trip: RunRecord = serde_json::from_value(json).expect("record should deserialize"); + + assert_eq!( + round_trip + .settings + .server + .as_ref() + .and_then(|server| server.storage.as_ref()) + .and_then(|storage| storage.root.as_ref()) + .map(|value| value.as_source()), + Some("${env.FABRO_STORAGE}".to_string()) + ); +} + +#[test] +fn run_created_props_round_trips_templated_settings() { + let settings = parse( + r#" +_version = 1 + +[server.integrations.github] +app_id = "${env.GITHUB_APP_ID}" +"#, + ); + let event = RunCreatedProps { + settings, + graph: fabro_types::graph::Graph::new("test"), + workflow_source: Some("digraph test { start -> exit }".to_string()), + workflow_config: None, + labels: std::collections::BTreeMap::new(), + run_dir: "/tmp/run".to_string(), + working_directory: "/tmp/project".to_string(), + host_repo_path: Some("/tmp/project".to_string()), + repo_origin_url: None, + base_branch: Some("main".to_string()), + workflow_slug: Some("demo".to_string()), + db_prefix: None, + provenance: None, + manifest_blob: None, + }; + + let json = serde_json::to_value(&event).expect("event should serialize"); + let round_trip: RunCreatedProps = + serde_json::from_value(json).expect("event should deserialize"); + + assert_eq!( + round_trip + .settings + .server + .as_ref() + .and_then(|server| server.integrations.as_ref()) + .and_then(|integrations| integrations.github.as_ref()) + .and_then(|github| github.app_id.as_ref()) + .map(|value| value.as_source()), + Some("${env.GITHUB_APP_ID}".to_string()) + ); +} diff --git a/lib/crates/fabro-config/tests/resolve_run.rs b/lib/crates/fabro-config/tests/resolve_run.rs index 49648ebc9..ed0aa7029 100644 --- a/lib/crates/fabro-config/tests/resolve_run.rs +++ b/lib/crates/fabro-config/tests/resolve_run.rs @@ -1,11 +1,8 @@ -use fabro_config::ConfigLayer; use fabro_types::settings::run::{ApprovalMode, RunGoal, RunMode, WorktreeMode}; -use fabro_types::settings::{InterpString, SettingsFile}; +use fabro_types::settings::{InterpString, SettingsFile, parse_settings_file}; fn parse(source: &str) -> SettingsFile { - ConfigLayer::parse(source) - .expect("fixture should parse") - .into() + parse_settings_file(source).expect("fixture should parse") } #[test] diff --git a/lib/crates/fabro-config/tests/resolve_server.rs b/lib/crates/fabro-config/tests/resolve_server.rs index f4ec7833f..6ef2a563a 100644 --- a/lib/crates/fabro-config/tests/resolve_server.rs +++ b/lib/crates/fabro-config/tests/resolve_server.rs @@ -1,12 +1,9 @@ -use fabro_config::ConfigLayer; use fabro_types::settings::server::{ObjectStoreSettings, ServerListenSettings}; -use fabro_types::settings::{InterpString, SettingsFile}; +use fabro_types::settings::{InterpString, SettingsFile, parse_settings_file}; use fabro_util::Home; fn parse(source: &str) -> SettingsFile { - ConfigLayer::parse(source) - .expect("fixture should parse") - .into() + parse_settings_file(source).expect("fixture should parse") } #[test] diff --git a/lib/crates/fabro-config/tests/resolve_workflow.rs b/lib/crates/fabro-config/tests/resolve_workflow.rs index 0ece928d8..bf84b6bb1 100644 --- a/lib/crates/fabro-config/tests/resolve_workflow.rs +++ b/lib/crates/fabro-config/tests/resolve_workflow.rs @@ -1,5 +1,5 @@ use fabro_config::resolve_workflow_from_file; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{SettingsFile, parse_settings_file}; #[test] fn resolves_workflow_defaults_from_empty_settings() { @@ -15,7 +15,7 @@ fn resolves_workflow_defaults_from_empty_settings() { #[test] fn resolves_workflow_graph_and_metadata() { - let settings: SettingsFile = fabro_config::ConfigLayer::parse( + let settings: SettingsFile = parse_settings_file( r#" _version = 1 @@ -28,8 +28,7 @@ graph = "graphs/ship.dot" tier = "gold" "#, ) - .expect("fixture should parse") - .into(); + .expect("fixture should parse"); let workflow = resolve_workflow_from_file(&settings).expect("workflow settings should resolve"); diff --git a/lib/crates/fabro-server/src/jwt_auth.rs b/lib/crates/fabro-server/src/jwt_auth.rs index 01c880fb0..fdc91f2c2 100644 --- a/lib/crates/fabro-server/src/jwt_auth.rs +++ b/lib/crates/fabro-server/src/jwt_auth.rs @@ -452,8 +452,8 @@ mod tests { use axum::http::{Request, StatusCode}; use axum::response::IntoResponse; use axum::routing::get; - use fabro_config::ConfigLayer; use fabro_config::resolve_server_from_file; + use fabro_types::settings::parse_settings_file; use tower::ServiceExt; use crate::web_auth::SessionCookie; @@ -461,9 +461,7 @@ mod tests { // --- Fail-closed resolver tests (R52/R53) ----------------------------------- fn settings(source: &str) -> ResolvedServerSettings { - let file = ConfigLayer::parse(source) - .expect("fixture should parse") - .into(); + let file = parse_settings_file(source).expect("fixture should parse"); resolve_server_from_file(&file).expect("fixture should resolve") } diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index adb39ba48..25cd73260 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -4,25 +4,30 @@ use std::sync::Arc; use anyhow::{Result, anyhow, bail}; use fabro_api::types; -use fabro_config::ConfigLayer; use fabro_config::effective_settings; use fabro_config::effective_settings::{EffectiveSettingsLayers, EffectiveSettingsMode}; +use fabro_config::merge::combine_files; use fabro_config::project::resolve_working_directory; use fabro_config::run::parse_run_config; use fabro_graphviz::graph::{Graph, is_llm_handler_type}; use fabro_graphviz::render::apply_direction; use fabro_llm::Provider; use fabro_model::Catalog; +use fabro_sandbox::config::{ + DaytonaNetwork, DaytonaSnapshotSettings, DockerfileSource as SandboxDockerfileSource, +}; use fabro_sandbox::daytona::DaytonaConfig; use fabro_sandbox::{DockerSandboxOptions, Sandbox, SandboxProvider, SandboxSpec}; use fabro_types::RunId; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::ServerSettings; use fabro_types::settings::cli::{CliLayer, CliOutputLayer, OutputVerbosity}; use fabro_types::settings::interp::InterpString; use fabro_types::settings::run::{ - ApprovalMode, DaytonaDockerfileLayer, RunExecutionLayer, RunGoalLayer, RunLayer, RunMode, - RunModelLayer, RunSandboxLayer, + ApprovalMode, DaytonaDockerfileLayer, DaytonaNetworkLayer, DaytonaSettings, DockerfileSource, + RunExecutionLayer, RunGoalLayer, RunLayer, RunMode, RunModelLayer, RunSandboxLayer, + RunSettings, }; +use fabro_types::settings::{SettingsFile, parse_settings_file}; use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSection, CheckStatus}; use fabro_validate::Severity; use fabro_workflow::error::FabroError; @@ -70,15 +75,15 @@ pub(crate) fn prepare_manifest_with_mode( .configs .iter() .filter(|config| config.type_ == types::ManifestConfigType::Project) - .try_fold(ConfigLayer::default(), |layer, config| { - Ok::<_, anyhow::Error>(parse_manifest_config(config)?.combine(layer)) + .try_fold(SettingsFile::default(), |layer, config| { + Ok::<_, anyhow::Error>(combine_files(layer, parse_manifest_config(config)?)) })?; let user_layer = manifest .configs .iter() .filter(|config| config.type_ == types::ManifestConfigType::User) - .try_fold(ConfigLayer::default(), |layer, config| { - Ok::<_, anyhow::Error>(parse_manifest_config(config)?.combine(layer)) + .try_fold(SettingsFile::default(), |layer, config| { + Ok::<_, anyhow::Error>(combine_files(layer, parse_manifest_config(config)?)) })?; let mut settings = effective_settings::resolve_settings( EffectiveSettingsLayers::new(args_layer, workflow_layer, project_layer, user_layer), @@ -193,12 +198,12 @@ fn workflow_bundle_from_manifest( fn root_workflow_config_layer( manifest: &types::RunManifest, workflow: &BundledWorkflow, -) -> Result { +) -> Result { let Some(root) = manifest.workflows.get(&manifest.target.path) else { bail!("manifest target path is missing from workflows map"); }; let Some(config) = root.config.as_ref() else { - return Ok(ConfigLayer::default()); + return Ok(SettingsFile::default()); }; let mut layer = parse_run_config(&config.source)?; @@ -206,16 +211,16 @@ fn root_workflow_config_layer( Ok(layer) } -fn parse_manifest_config(config: &types::ManifestConfig) -> Result { +fn parse_manifest_config(config: &types::ManifestConfig) -> Result { let Some(source) = config.source.as_deref() else { - return Ok(ConfigLayer::default()); + return Ok(SettingsFile::default()); }; - ConfigLayer::parse(source) + parse_settings_file(source).map_err(|err| anyhow!("Failed to parse settings file: {err}")) } -fn manifest_args_layer(args: Option<&types::ManifestArgs>) -> ConfigLayer { +fn manifest_args_layer(args: Option<&types::ManifestArgs>) -> SettingsFile { let Some(args) = args else { - return ConfigLayer::default(); + return SettingsFile::default(); }; let model = (args.model.is_some() || args.provider.is_some()).then(|| RunModelLayer { @@ -268,11 +273,11 @@ fn manifest_args_layer(args: Option<&types::ManifestArgs>) -> ConfigLayer { }) }); - ConfigLayer::from(SettingsFile { + SettingsFile { run, cli, ..SettingsFile::default() - }) + } } fn parse_labels(labels: &[String]) -> HashMap { @@ -284,12 +289,11 @@ fn parse_labels(labels: &[String]) -> HashMap { } fn resolve_manifest_dockerfile( - layer: &mut ConfigLayer, + layer: &mut SettingsFile, config_path: &Path, files: &HashMap, ) -> Result<()> { let source = layer - .as_v2_mut() .run .as_mut() .and_then(|run| run.sandbox.as_mut()) @@ -340,12 +344,22 @@ async fn build_preflight_report( ) -> Result<(CheckReport, bool)> { let graph = validated.graph(); let settings = &prepared.settings; - let materialized = materialize_run(settings.clone(), graph, &Catalog::builtin()); + let materialized = materialize_run(settings.clone(), graph, Catalog::builtin()); let resolved_run = fabro_config::resolve_run_from_file(&materialized) .map_err(|errors| anyhow!(render_resolve_errors(&errors)))?; + let resolved_server = fabro_config::resolve_server_from_file(settings) + .map_err(|errors| anyhow!(render_resolve_errors(&errors)))?; let sandbox_provider = resolve_sandbox_provider(&resolved_run)?; let github_app = state - .github_app_credentials(settings.github_app_id_str().as_deref()) + .github_app_credentials( + resolved_server + .integrations + .github + .app_id + .as_ref() + .map(InterpString::as_source) + .as_deref(), + ) .await .map_err(|err| anyhow!(err))?; let mut checks = Vec::new(); @@ -402,7 +416,7 @@ async fn build_preflight_report( ) .await; let llm_ok = run_llm_check(state, &mut checks, graph, &resolved_run).await; - run_github_token_check(&mut checks, prepared, settings, github_app).await; + run_github_token_check(&mut checks, prepared, &resolved_server, github_app).await; let checks_ok = sandbox_ok && llm_ok; @@ -418,19 +432,16 @@ async fn build_preflight_report( )) } -fn resolve_sandbox_provider( - settings: &fabro_types::settings::run::RunSettings, -) -> Result { - Ok(Some(settings.sandbox.provider.as_str()) - .map(str::parse::) - .transpose() - .map_err(|err| anyhow!("Invalid sandbox provider: {err}"))? - .unwrap_or_default()) +fn resolve_sandbox_provider(settings: &RunSettings) -> Result { + Ok(Some(str::parse::( + settings.sandbox.provider.as_str(), + )) + .transpose() + .map_err(|err| anyhow!("Invalid sandbox provider: {err}"))? + .unwrap_or_default()) } -fn resolve_daytona_config( - settings: &fabro_types::settings::run::RunSettings, -) -> Option { +fn resolve_daytona_config(settings: &RunSettings) -> Option { settings .sandbox .daytona @@ -442,7 +453,7 @@ async fn run_sandbox_check( checks: &mut Vec, sandbox_provider: SandboxProvider, prepared: &PreparedManifest, - resolved_run: &fabro_types::settings::run::RunSettings, + resolved_run: &RunSettings, github_app: Option, ) -> bool { let daytona_config = resolve_daytona_config(resolved_run); @@ -515,7 +526,7 @@ async fn run_llm_check( state: &AppState, checks: &mut Vec, graph: &Graph, - settings: &fabro_types::settings::run::RunSettings, + settings: &RunSettings, ) -> bool { let (model, provider) = resolve_model_provider(settings, graph); let default_provider = provider.as_deref().unwrap_or("anthropic"); @@ -606,21 +617,16 @@ async fn run_llm_check( } } -fn resolve_model_provider( - settings: &fabro_types::settings::run::RunSettings, - _graph: &Graph, -) -> (String, Option) { +fn resolve_model_provider(settings: &RunSettings, _graph: &Graph) -> (String, Option) { let provider = settings .model .provider .as_ref() .map(InterpString::as_source); - let model = settings - .model - .name - .as_ref() - .map(InterpString::as_source) - .unwrap_or_else(|| Catalog::builtin().default_from_env().id.clone()); + let model = settings.model.name.as_ref().map_or_else( + || Catalog::builtin().default_from_env().id.clone(), + InterpString::as_source, + ); match Catalog::builtin().get(&model) { Some(info) => ( @@ -639,12 +645,14 @@ fn render_resolve_errors(errors: &[fabro_config::ResolveError]) -> String { .join("; ") } -fn runtime_daytona_config(settings: &fabro_types::settings::run::DaytonaSettings) -> DaytonaConfig { +fn runtime_daytona_config(settings: &DaytonaSettings) -> DaytonaConfig { DaytonaConfig { auto_stop_interval: settings.auto_stop_interval, labels: (!settings.labels.is_empty()).then_some(settings.labels.clone()), - snapshot: settings.snapshot.as_ref().map(|snapshot| { - fabro_sandbox::config::DaytonaSnapshotSettings { + snapshot: settings + .snapshot + .as_ref() + .map(|snapshot| DaytonaSnapshotSettings { name: snapshot.name.clone(), cpu: snapshot.cpu, memory: snapshot.memory_gb, @@ -653,24 +661,19 @@ fn runtime_daytona_config(settings: &fabro_types::settings::run::DaytonaSettings .dockerfile .as_ref() .map(|dockerfile| match dockerfile { - fabro_types::settings::run::DockerfileSource::Inline(text) => { - fabro_sandbox::config::DockerfileSource::Inline(text.clone()) + DockerfileSource::Inline(text) => { + SandboxDockerfileSource::Inline(text.clone()) } - fabro_types::settings::run::DockerfileSource::Path { path } => { - fabro_sandbox::config::DockerfileSource::Path { path: path.clone() } + DockerfileSource::Path { path } => { + SandboxDockerfileSource::Path { path: path.clone() } } }), - } - }), + }), network: settings.network.as_ref().map(|network| match network { - fabro_types::settings::run::DaytonaNetworkLayer::Block => { - fabro_sandbox::config::DaytonaNetwork::Block - } - fabro_types::settings::run::DaytonaNetworkLayer::AllowAll => { - fabro_sandbox::config::DaytonaNetwork::AllowAll - } - fabro_types::settings::run::DaytonaNetworkLayer::AllowList { allow_list } => { - fabro_sandbox::config::DaytonaNetwork::AllowList(allow_list.clone()) + DaytonaNetworkLayer::Block => DaytonaNetwork::Block, + DaytonaNetworkLayer::AllowAll => DaytonaNetwork::AllowAll, + DaytonaNetworkLayer::AllowList { allow_list } => { + DaytonaNetwork::AllowList(allow_list.clone()) } }), skip_clone: settings.skip_clone, @@ -680,19 +683,19 @@ fn runtime_daytona_config(settings: &fabro_types::settings::run::DaytonaSettings async fn run_github_token_check( checks: &mut Vec, prepared: &PreparedManifest, - settings: &SettingsFile, + settings: &ServerSettings, github_app: Option, ) { - let Some(v2_permissions) = settings.github_permissions() else { - return; - }; - if v2_permissions.is_empty() { + if settings.integrations.github.permissions.is_empty() { return; } // Resolve InterpString permission values eagerly for token minting and // for display in the preflight report. - let github_permissions: HashMap = v2_permissions + let github_permissions: HashMap = settings + .integrations + .github + .permissions .iter() .map(|(k, v)| (k.clone(), v.as_source())) .collect(); @@ -863,9 +866,7 @@ mod tests { } fn server_settings_fixture(source: &str) -> SettingsFile { - fabro_config::ConfigLayer::parse(source) - .expect("v2 fixture should parse") - .into() + fabro_types::settings::parse_settings_file(source).expect("v2 fixture should parse") } #[test] @@ -885,11 +886,10 @@ root = "/srv/fabro" let prepared = prepare_manifest_with_mode(&server_settings, &minimal_manifest(), false).unwrap(); - assert!(!prepared.settings.dry_run_enabled()); - assert_eq!( - prepared.settings.server_storage_root_str().as_deref(), - Some("/srv/fabro"), - ); + let resolved_run = fabro_config::resolve_run_from_file(&prepared.settings).unwrap(); + let resolved_server = fabro_config::resolve_server_from_file(&prepared.settings).unwrap(); + assert!(resolved_run.execution.mode != fabro_types::settings::run::RunMode::DryRun); + assert_eq!(resolved_server.storage.root.as_source(), "/srv/fabro"); } #[test] @@ -920,7 +920,13 @@ root = "/srv/fabro" let prepared = prepare_manifest_with_mode(&server_settings, &manifest, false).unwrap(); - assert!(prepared.settings.dry_run_enabled()); + assert_eq!( + fabro_config::resolve_run_from_file(&prepared.settings) + .unwrap() + .execution + .mode, + fabro_types::settings::run::RunMode::DryRun + ); } #[test] @@ -970,20 +976,25 @@ app_id = "snapshotted-app-id" }); let prepared = prepare_manifest_with_mode(&server_settings, &manifest, true).unwrap(); + let resolved_run = fabro_config::resolve_run_from_file(&prepared.settings).unwrap(); + let resolved_server = fabro_config::resolve_server_from_file(&prepared.settings).unwrap(); // v2 merge matrix: run.prepare.steps replaces the whole list across // layers, so the higher-precedence workflow layer wins over cli. assert_eq!( - prepared.settings.run_prepare_commands(), + resolved_run.prepare.commands, vec!["workflow-setup".to_string()] ); assert_eq!( - prepared.settings.github_app_id_str().as_deref(), + resolved_server + .integrations + .github + .app_id + .as_ref() + .map(|value| value.as_source()) + .as_deref(), Some("snapshotted-app-id") ); - assert_eq!( - prepared.settings.server_storage_root_str().as_deref(), - Some("/srv/fabro"), - ); + assert_eq!(resolved_server.storage.root.as_source(), "/srv/fabro"); } } diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 42d161693..441764343 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -85,7 +85,7 @@ pub struct ServeArgs { } fn load_settings(path: Option<&Path>) -> anyhow::Result { - Ok(load_settings_config(path)?.into()) + load_settings_config(path) } fn resolved_config_path(path: Option<&Path>) -> PathBuf { @@ -363,7 +363,7 @@ where .github .webhooks .as_ref() - .and_then(|_| resolved_server_settings.integrations.github.app_id.as_ref()) + .and(resolved_server_settings.integrations.github.app_id.as_ref()) .map(resolve_interp) .transpose()?; let webhook_manager = match webhook_app_id { @@ -695,13 +695,10 @@ mod tests { build_object_store_with_preference, server_bind_title, server_title, }; use crate::bind::Bind; - use fabro_config::ConfigLayer; - use fabro_types::settings::SettingsFile; + use fabro_types::settings::{SettingsFile, parse_settings_file}; fn parse_settings(source: &str) -> SettingsFile { - ConfigLayer::parse(source) - .expect("v2 fixture should parse") - .into() + parse_settings_file(source).expect("v2 fixture should parse") } #[test] @@ -722,10 +719,13 @@ mod tests { let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro-storage")); - assert_eq!( - resolved.server_storage_root_str().as_deref(), - Some("/srv/fabro-storage") - ); + let storage_root = resolved + .server + .as_ref() + .and_then(|server| server.storage.as_ref()) + .and_then(|storage| storage.root.as_ref()) + .map(|value| value.as_source()); + assert_eq!(storage_root.as_deref(), Some("/srv/fabro-storage")); } #[test] @@ -752,7 +752,14 @@ enabled = false let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro")); - assert_eq!(resolved.server_web().and_then(|w| w.enabled), Some(true)); + assert_eq!( + resolved + .server + .as_ref() + .and_then(|server| server.web.as_ref()) + .and_then(|web| web.enabled), + Some(true) + ); } #[test] @@ -772,7 +779,14 @@ enabled = false let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro")); - assert_eq!(resolved.server_web().and_then(|w| w.enabled), Some(false)); + assert_eq!( + resolved + .server + .as_ref() + .and_then(|server| server.web.as_ref()) + .and_then(|web| web.enabled), + Some(false) + ); } #[test] diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 930c19206..9adae4f30 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -34,6 +34,7 @@ use fabro_model::{BilledModelUsage, BilledTokenCounts}; use fabro_store::{ ArtifactStore, Database, EventEnvelope, EventPayload, PendingInterviewRecord, StageId, }; +use fabro_types::settings::run::RunMode; use fabro_types::settings::{InterpString, ServerSettings as ResolvedServerSettings, SettingsFile}; use fabro_types::{ ActorRef, EventBody, InterviewQuestionRecord, InterviewQuestionType, RunBlobId, @@ -595,7 +596,7 @@ impl AppState { pub(crate) fn dry_run(&self) -> bool { fabro_config::resolve_run_from_file(&self.settings.read().unwrap()) - .map(|settings| settings.execution.mode == fabro_types::settings::run::RunMode::DryRun) + .map(|settings| settings.execution.mode == RunMode::DryRun) .unwrap_or(false) } @@ -710,7 +711,7 @@ impl AppState { } pub(crate) fn reload_settings_from_disk(&self) -> anyhow::Result<()> { - let reloaded: SettingsFile = fabro_config::ConfigLayer::load(&self.config_path)?.into(); + let reloaded = fabro_config::load_settings_path(&self.config_path)?; self.replace_settings(reloaded) } } @@ -1467,9 +1468,45 @@ fn build_prune_plan( } fn system_sandbox_provider(settings: &SettingsFile) -> String { - fabro_config::resolve_run_from_file(settings) - .map(|settings| settings.sandbox.provider) - .unwrap_or_else(|_| SandboxProvider::default().to_string()) + fabro_config::resolve_run_from_file(settings).map_or_else( + |_| SandboxProvider::default().to_string(), + |settings| settings.sandbox.provider, + ) +} + +fn render_resolve_errors(errors: &[fabro_config::ResolveError]) -> String { + errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("; ") +} + +fn resolved_storage_dir(settings: &SettingsFile) -> Result { + let resolved = + resolve_server_from_file(settings).map_err(|errors| render_resolve_errors(&errors))?; + resolved + .storage + .root + .resolve(|name| std::env::var(name).ok()) + .map(|value| PathBuf::from(value.value)) + .map_err(|err| { + format!( + "failed to resolve {}: {err}", + resolved.storage.root.as_source() + ) + }) +} + +fn resolved_github_app_id(settings: &SettingsFile) -> Result, String> { + let resolved = + resolve_server_from_file(settings).map_err(|errors| render_resolve_errors(&errors))?; + Ok(resolved + .integrations + .github + .app_id + .as_ref() + .map(InterpString::as_source)) } fn parse_system_duration(raw: &str) -> anyhow::Result { @@ -3488,10 +3525,19 @@ async fn start_run( ) .into_response(); }; - let run_dir = Storage::new(run_record.settings.storage_dir()) - .run_scratch(&id) - .root() - .to_path_buf(); + let run_dir = match resolved_storage_dir(&run_record.settings) { + Ok(storage_dir) => Storage::new(storage_dir) + .run_scratch(&id) + .root() + .to_path_buf(), + Err(err) => { + return ApiError::new( + StatusCode::INTERNAL_SERVER_ERROR, + format!("invalid persisted server storage settings: {err}"), + ) + .into_response(); + } + }; let dot_source = run_state.graph_source.unwrap_or_default(); { @@ -3649,16 +3695,21 @@ async fn execute_run_in_process(state: Arc, run_id: RunId) { return; } }; - let github_app = match state - .github_app_credentials( - persisted - .run_record() - .settings - .github_app_id_str() - .as_deref(), - ) - .await - { + let github_app_id = match resolved_github_app_id(&persisted.run_record().settings) { + Ok(app_id) => app_id, + Err(err) => { + tracing::error!(run_id = %run_id, error = %err, "Invalid GitHub App config"); + let mut runs = state.runs.lock().expect("runs lock poisoned"); + if let Some(managed_run) = runs.get_mut(&run_id) { + managed_run.status = RunStatus::Failed; + managed_run.error = Some(format!("Invalid GitHub App config: {err}")); + clear_live_run_state(managed_run); + } + state.scheduler_notify.notify_one(); + return; + } + }; + let github_app = match state.github_app_credentials(github_app_id.as_deref()).await { Ok(github_app) => github_app, Err(e) => { tracing::error!(run_id = %run_id, error = %e, "Invalid GitHub App credentials"); @@ -6299,7 +6350,7 @@ mod tests { #[tokio::test] async fn auth_login_github_redirects_to_github() { - let settings: SettingsFile = fabro_config::ConfigLayer::parse( + let settings: SettingsFile = fabro_types::settings::parse_settings_file( r#" _version = 1 @@ -6313,8 +6364,7 @@ client_id = "Iv1.testclient" slug = "fabro" "#, ) - .expect("fixture should parse") - .into(); + .expect("fixture should parse"); let app = build_router( create_app_state_with_options(settings, 5), AuthMode::Disabled, @@ -7440,7 +7490,7 @@ slug = "fabro" #[tokio::test] async fn start_run_persists_full_settings_snapshot() { - let settings: SettingsFile = fabro_config::ConfigLayer::parse( + let settings: SettingsFile = fabro_types::settings::parse_settings_file( r#" _version = 1 @@ -7479,8 +7529,7 @@ url = "http://api.example.test" level = "debug" "#, ) - .expect("fixture should parse") - .into(); + .expect("fixture should parse"); let state = create_app_state_with_options(settings, 5); let app = build_router(Arc::clone(&state), AuthMode::Disabled); @@ -7512,24 +7561,41 @@ level = "debug" .unwrap() .run .expect("run record should exist"); + let resolved_run = fabro_config::resolve_run_from_file(&run_record.settings).unwrap(); + let resolved_server = fabro_config::resolve_server_from_file(&run_record.settings).unwrap(); // Server-side `dry_run` default must not override the manifest's intent. // Verify a sampling of the persisted v2 settings. assert_eq!( - run_record.settings.run_goal_inline_str().as_deref(), + match resolved_run.goal { + Some(fabro_types::settings::run::RunGoal::Inline(value)) => Some(value.as_source()), + _ => None, + } + .as_deref(), Some("Test"), "goal should be persisted from the manifest" ); assert!( - !run_record.settings.dry_run_enabled(), + resolved_run.execution.mode != fabro_types::settings::run::RunMode::DryRun, "server-local dry_run fallback must not override manifest intent" ); assert_eq!( - run_record.settings.run_model_name_str().as_deref(), + resolved_run + .model + .name + .as_ref() + .map(|value| value.as_source()) + .as_deref(), Some("claude-sonnet-4-5"), ); assert_eq!( - run_record.settings.github_app_id_str().as_deref(), + resolved_server + .integrations + .github + .app_id + .as_ref() + .map(|value| value.as_source()) + .as_deref(), Some("12345"), ); } @@ -7894,7 +7960,7 @@ level = "debug" #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn cancel_during_startup_persists_cancelled_reason() { - let settings: SettingsFile = fabro_config::ConfigLayer::parse( + let settings: SettingsFile = fabro_types::settings::parse_settings_file( r#" _version = 1 @@ -7905,8 +7971,7 @@ script = "sleep 5" timeout = "30s" "#, ) - .expect("fixture should parse") - .into(); + .expect("fixture should parse"); let state = create_app_state_with_settings_and_registry_factory(settings, |interviewer| { fabro_workflow::handler::default_registry(interviewer, || None) }); diff --git a/lib/crates/fabro-server/src/settings_view.rs b/lib/crates/fabro-server/src/settings_view.rs index 4fbbfb753..3f5c6dba3 100644 --- a/lib/crates/fabro-server/src/settings_view.rs +++ b/lib/crates/fabro-server/src/settings_view.rs @@ -73,12 +73,10 @@ pub(crate) fn redact_for_api(settings: &SettingsFile) -> SettingsFile { #[cfg(test)] mod tests { use super::*; - use fabro_config::ConfigLayer; + use fabro_types::settings::parse_settings_file; fn parse(source: &str) -> SettingsFile { - ConfigLayer::parse(source) - .expect("fixture should parse") - .into() + parse_settings_file(source).expect("fixture should parse") } #[test] diff --git a/lib/crates/fabro-server/src/web_auth.rs b/lib/crates/fabro-server/src/web_auth.rs index d2ef3f352..d0211f16a 100644 --- a/lib/crates/fabro-server/src/web_auth.rs +++ b/lib/crates/fabro-server/src/web_auth.rs @@ -781,13 +781,9 @@ mod tests { // Re-parse the emitted document to prove it round-trips into a // valid v2 `SettingsFile`. let emitted = doc.to_string(); - let file = fabro_config::ConfigLayer::parse(&emitted) + let file = fabro_types::settings::parse_settings_file(&emitted) .expect("merged output should parse as a v2 SettingsFile"); - let server = file - .as_v2() - .server - .as_ref() - .expect("[server] should be present"); + let server = file.server.as_ref().expect("[server] should be present"); let integrations = server .integrations .as_ref() @@ -869,7 +865,7 @@ name = "claude-sonnet" // Finally, the whole thing must still parse as a valid v2 // SettingsFile. - fabro_config::ConfigLayer::parse(&emitted) + fabro_types::settings::parse_settings_file(&emitted) .expect("merged output should still parse as v2 after the edit"); } } diff --git a/lib/crates/fabro-server/tests/it/api/routing.rs b/lib/crates/fabro-server/tests/it/api/routing.rs index 5b8276b06..3cdb79388 100644 --- a/lib/crates/fabro-server/tests/it/api/routing.rs +++ b/lib/crates/fabro-server/tests/it/api/routing.rs @@ -1,12 +1,11 @@ use axum::body::{Body, to_bytes}; use axum::http::{Method, Request, StatusCode}; -use fabro_config::ConfigLayer; use fabro_server::jwt_auth::AuthMode; use fabro_server::server::{ RouterOptions, build_router, build_router_with_options, create_app_state, create_app_state_with_options, }; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{SettingsFile, parse_settings_file}; use tower::ServiceExt; use crate::helpers::body_json; @@ -121,7 +120,7 @@ async fn web_enabled_serves_web_only_routes() { #[tokio::test] async fn web_disabled_returns_404_for_web_routes_and_keeps_machine_api() { - let settings: SettingsFile = ConfigLayer::parse( + let settings: SettingsFile = parse_settings_file( r#" _version = 1 @@ -129,8 +128,7 @@ _version = 1 enabled = false "#, ) - .expect("settings fixture should parse") - .into(); + .expect("settings fixture should parse"); let app = build_router_with_options( create_app_state_with_options(settings, 5), AuthMode::Disabled, @@ -179,7 +177,7 @@ enabled = false #[tokio::test] async fn web_disabled_ignores_demo_header_dispatch() { - let settings: SettingsFile = ConfigLayer::parse( + let settings: SettingsFile = parse_settings_file( r#" _version = 1 @@ -187,8 +185,7 @@ _version = 1 enabled = false "#, ) - .expect("settings fixture should parse") - .into(); + .expect("settings fixture should parse"); let app = build_router_with_options( create_app_state_with_options(settings, 5), AuthMode::Disabled, diff --git a/lib/crates/fabro-server/tests/it/api/settings.rs b/lib/crates/fabro-server/tests/it/api/settings.rs index fe7e82ee5..61243ec11 100644 --- a/lib/crates/fabro-server/tests/it/api/settings.rs +++ b/lib/crates/fabro-server/tests/it/api/settings.rs @@ -1,16 +1,15 @@ use axum::body::Body; use axum::http::{Request, StatusCode}; -use fabro_config::ConfigLayer; use fabro_server::jwt_auth::AuthMode; use fabro_server::server::{build_router, create_app_state_with_options}; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{SettingsFile, parse_settings_file}; use tower::ServiceExt; use crate::helpers::body_json; #[tokio::test] async fn retrieve_server_settings_returns_runtime_settings() { - let settings: SettingsFile = ConfigLayer::parse( + let settings: SettingsFile = parse_settings_file( r#" _version = 1 @@ -27,8 +26,7 @@ verbosity = "verbose" server_only = "1" "#, ) - .expect("settings fixture should parse") - .into(); + .expect("settings fixture should parse"); let app = build_router( create_app_state_with_options(settings, 5), AuthMode::Disabled, diff --git a/lib/crates/fabro-types/src/settings/accessors.rs b/lib/crates/fabro-types/src/settings/accessors.rs deleted file mode 100644 index 42a604fe5..000000000 --- a/lib/crates/fabro-types/src/settings/accessors.rs +++ /dev/null @@ -1,593 +0,0 @@ -//! Convenience accessors on [`SettingsFile`]. -//! -//! These methods provide ergonomic, flat-shaped views into the v2 parse -//! tree. They exist so that consumers don't have to chain `.as_ref()` -//! through every Option layer when reading common fields. Each accessor -//! walks the real v2 structure — there is no transitional state here. - -use std::collections::HashMap; -use std::path::{Path, PathBuf}; - -use super::interp::InterpString; -use super::run::{ - ApprovalMode, GitAuthorLayer, HookEntry, McpEntryLayer, ResolvedGoalSource, ResolvedRunGoal, - RunAgentLayer, RunArtifactsLayer, RunCheckpointLayer, RunExecutionLayer, RunGoalLayer, - RunLayer, RunMode, RunModelLayer, RunPrepareLayer, RunPullRequestLayer, RunSandboxLayer, -}; -use super::server::{ - GithubIntegrationLayer, ServerApiLayer, ServerArtifactsLayer, ServerIntegrationsLayer, - ServerLayer, ServerLoggingLayer, ServerSchedulerLayer, ServerStorageLayer, ServerWebLayer, - SlackIntegrationLayer, -}; -use super::tree::SettingsFile; - -impl SettingsFile { - // ---------- run-scope ---------- - - #[must_use] - pub fn run_layer(&self) -> Option<&RunLayer> { - self.run.as_ref() - } - - /// Raw access to the `run.goal` variant (inline or file). - #[must_use] - pub fn run_goal_layer(&self) -> Option<&RunGoalLayer> { - self.run.as_ref().and_then(|r| r.goal.as_ref()) - } - - /// Inline goal text only. Returns `None` when `run.goal` is unset **or** - /// when it's a file-sourced goal — callers that need the file contents - /// should use [`SettingsFile::resolve_run_goal`]. - #[must_use] - pub fn run_goal_inline_str(&self) -> Option { - match self.run_goal_layer()? { - RunGoalLayer::Inline(s) => Some(s.as_source()), - RunGoalLayer::File { .. } => None, - } - } - - /// Resolve the `run.goal` layer to its final text, reading a file from - /// disk if necessary. - /// - /// Path resolution: - /// - /// - Absolute paths in the `file` variant are used as-is. - /// - Literal relative paths should already have been rewritten to - /// absolute at config-load time by - /// `fabro_config::resolve_goal_file_paths`. If one reaches this point - /// it will be resolved against `base_dir` as a fallback. - /// - `${env.NAME}` interpolation is resolved via `std::env::var` at - /// call time. Relative paths that survive interpolation are also - /// resolved against `base_dir`. - /// - /// Returns `Ok(None)` when `run.goal` is unset. Returns `Err` when the - /// file variant points at a path that can't be read or has an - /// unresolved env token. - pub fn resolve_run_goal( - &self, - base_dir: &Path, - ) -> Result, ResolveGoalError> { - let Some(layer) = self.run_goal_layer() else { - return Ok(None); - }; - match layer { - RunGoalLayer::Inline(s) => Ok(Some(ResolvedRunGoal { - text: s.as_source(), - source: ResolvedGoalSource::Inline, - })), - RunGoalLayer::File { file } => { - let resolved = file - .resolve(|name| std::env::var(name).ok()) - .map_err(|err| ResolveGoalError::EnvLookup { var: err.name })?; - let path = resolve_goal_file_path(&resolved.value, base_dir); - let text = std::fs::read_to_string(&path).map_err(|err| ResolveGoalError::Io { - path: path.clone(), - source: err, - })?; - Ok(Some(ResolvedRunGoal { - text, - source: ResolvedGoalSource::File { path }, - })) - } - } - } - - #[must_use] - pub fn run_working_dir(&self) -> Option<&InterpString> { - self.run.as_ref().and_then(|r| r.working_dir.as_ref()) - } - - #[must_use] - pub fn run_working_dir_str(&self) -> Option { - self.run_working_dir().map(InterpString::as_source) - } - - #[must_use] - pub fn run_model(&self) -> Option<&RunModelLayer> { - self.run.as_ref().and_then(|r| r.model.as_ref()) - } - - #[must_use] - pub fn run_model_name_str(&self) -> Option { - self.run_model() - .and_then(|m| m.name.as_ref()) - .map(InterpString::as_source) - } - - #[must_use] - pub fn run_model_provider_str(&self) -> Option { - self.run_model() - .and_then(|m| m.provider.as_ref()) - .map(InterpString::as_source) - } - - #[must_use] - pub fn run_sandbox(&self) -> Option<&RunSandboxLayer> { - self.run.as_ref().and_then(|r| r.sandbox.as_ref()) - } - - #[must_use] - pub fn run_prepare(&self) -> Option<&RunPrepareLayer> { - self.run.as_ref().and_then(|r| r.prepare.as_ref()) - } - - /// Flattened prepare-step commands: each `script` is kept as-is, and - /// `command` argv is joined with spaces. Env-interpolation tokens are - /// emitted verbatim via [`InterpString::as_source`]. - #[must_use] - pub fn run_prepare_commands(&self) -> Vec { - let Some(prepare) = self.run_prepare() else { - return Vec::new(); - }; - prepare - .steps - .iter() - .filter_map(|step| { - if let Some(script) = &step.script { - Some(script.as_source()) - } else { - step.command.as_ref().map(|argv| { - argv.iter() - .map(InterpString::as_source) - .collect::>() - .join(" ") - }) - } - }) - .collect() - } - - /// Prepare-step timeout in milliseconds. - #[must_use] - pub fn run_prepare_timeout_ms(&self) -> Option { - self.run_prepare() - .and_then(|p| p.timeout) - .map(|d| u64::try_from(d.as_std().as_millis()).unwrap_or(u64::MAX)) - } - - #[must_use] - pub fn run_checkpoint(&self) -> Option<&RunCheckpointLayer> { - self.run.as_ref().and_then(|r| r.checkpoint.as_ref()) - } - - #[must_use] - pub fn run_hooks(&self) -> &[HookEntry] { - self.run.as_ref().map_or(&[], |r| r.hooks.as_slice()) - } - - #[must_use] - pub fn run_pull_request(&self) -> Option<&RunPullRequestLayer> { - self.run.as_ref().and_then(|r| r.pull_request.as_ref()) - } - - #[must_use] - pub fn run_artifacts(&self) -> Option<&RunArtifactsLayer> { - self.run.as_ref().and_then(|r| r.artifacts.as_ref()) - } - - #[must_use] - pub fn run_execution(&self) -> Option<&RunExecutionLayer> { - self.run.as_ref().and_then(|r| r.execution.as_ref()) - } - - #[must_use] - pub fn run_agent(&self) -> Option<&RunAgentLayer> { - self.run.as_ref().and_then(|r| r.agent.as_ref()) - } - - #[must_use] - pub fn run_agent_mcps(&self) -> Option<&HashMap> { - self.run_agent().map(|a| &a.mcps) - } - - #[must_use] - pub fn run_inputs(&self) -> Option<&HashMap> { - self.run.as_ref().and_then(|r| r.inputs.as_ref()) - } - - /// Stringified view of `run.inputs`: non-string TOML values are rendered - /// via their canonical TOML representation (integers, booleans, and - /// arrays are flattened through `Display`). Returns `None` when no - /// inputs are set. - #[must_use] - pub fn run_inputs_as_strings(&self) -> Option> { - self.run_inputs().map(|inputs| { - inputs - .iter() - .map(|(k, v)| { - let stringified = match v { - toml::Value::String(s) => s.clone(), - other => other.to_string(), - }; - (k.clone(), stringified) - }) - .collect() - }) - } - - #[must_use] - pub fn run_metadata(&self) -> Option<&HashMap> { - self.run.as_ref().map(|r| &r.metadata) - } - - #[must_use] - pub fn run_git_author(&self) -> Option<&GitAuthorLayer> { - self.run - .as_ref() - .and_then(|r| r.git.as_ref()) - .and_then(|g| g.author.as_ref()) - } - - // ---------- execution-posture booleans ---------- - - #[must_use] - pub fn dry_run_enabled(&self) -> bool { - matches!( - self.run_execution().and_then(|e| e.mode), - Some(RunMode::DryRun) - ) - } - - #[must_use] - pub fn auto_approve_enabled(&self) -> bool { - matches!( - self.run_execution().and_then(|e| e.approval), - Some(ApprovalMode::Auto) - ) - } - - /// Returns `true` when retros are explicitly disabled. Defaults to - /// `false` (retros enabled) when not set. - #[must_use] - pub fn no_retro_enabled(&self) -> bool { - matches!(self.run_execution().and_then(|e| e.retros), Some(false)) - } - - #[must_use] - pub fn preserve_sandbox_enabled(&self) -> bool { - self.run_sandbox() - .and_then(|sb| sb.preserve) - .unwrap_or(false) - } - - // ---------- server-scope ---------- - - #[must_use] - pub fn server_layer(&self) -> Option<&ServerLayer> { - self.server.as_ref() - } - - #[must_use] - pub fn server_api(&self) -> Option<&ServerApiLayer> { - self.server.as_ref().and_then(|s| s.api.as_ref()) - } - - #[must_use] - pub fn server_web(&self) -> Option<&ServerWebLayer> { - self.server.as_ref().and_then(|s| s.web.as_ref()) - } - - #[must_use] - pub fn server_storage(&self) -> Option<&ServerStorageLayer> { - self.server.as_ref().and_then(|s| s.storage.as_ref()) - } - - #[must_use] - pub fn server_storage_root_str(&self) -> Option { - self.server_storage() - .and_then(|s| s.root.as_ref()) - .map(InterpString::as_source) - } - - #[must_use] - pub fn server_artifacts(&self) -> Option<&ServerArtifactsLayer> { - self.server.as_ref().and_then(|s| s.artifacts.as_ref()) - } - - #[must_use] - pub fn server_scheduler(&self) -> Option<&ServerSchedulerLayer> { - self.server.as_ref().and_then(|s| s.scheduler.as_ref()) - } - - #[must_use] - pub fn max_concurrent_runs(&self) -> Option { - self.server_scheduler().and_then(|s| s.max_concurrent_runs) - } - - #[must_use] - pub fn server_logging(&self) -> Option<&ServerLoggingLayer> { - self.server.as_ref().and_then(|s| s.logging.as_ref()) - } - - #[must_use] - pub fn server_integrations(&self) -> Option<&ServerIntegrationsLayer> { - self.server.as_ref().and_then(|s| s.integrations.as_ref()) - } - - #[must_use] - pub fn server_integrations_github(&self) -> Option<&GithubIntegrationLayer> { - self.server_integrations().and_then(|i| i.github.as_ref()) - } - - #[must_use] - pub fn server_integrations_slack(&self) -> Option<&SlackIntegrationLayer> { - self.server_integrations().and_then(|i| i.slack.as_ref()) - } - - #[must_use] - pub fn github_app_id_str(&self) -> Option { - self.server_integrations_github() - .and_then(|g| g.app_id.as_ref()) - .map(InterpString::as_source) - } - - #[must_use] - pub fn github_client_id_str(&self) -> Option { - self.server_integrations_github() - .and_then(|g| g.client_id.as_ref()) - .map(InterpString::as_source) - } - - #[must_use] - pub fn github_slug_str(&self) -> Option { - self.server_integrations_github() - .and_then(|g| g.slug.as_ref()) - .map(InterpString::as_source) - } - - #[must_use] - pub fn github_permissions(&self) -> Option<&HashMap> { - self.server_integrations_github() - .map(|g| &g.permissions) - .filter(|m| !m.is_empty()) - } - - // ---------- storage path with home-dir default ---------- - - /// Returns the configured server storage root, or the home-dir default - /// when unset. Env interpolation is resolved at read time against the - /// process environment. - #[must_use] - pub fn storage_dir(&self) -> PathBuf { - self.server_storage() - .and_then(|s| s.root.as_ref()) - .and_then(|interp| { - interp - .resolve(|name| std::env::var(name).ok()) - .ok() - .map(|resolved| resolved.value) - }) - .map_or_else(|| fabro_util::Home::from_env().storage_dir(), PathBuf::from) - } - - // ---------- labels / metadata aggregation ---------- - - /// Combined metadata labels from project, workflow, and run layers. - /// Later layers overwrite earlier ones (project < workflow < run). - #[must_use] - pub fn all_labels(&self) -> HashMap { - let mut out = HashMap::new(); - if let Some(project) = &self.project { - for (k, v) in &project.metadata { - out.insert(k.clone(), v.clone()); - } - } - if let Some(workflow) = &self.workflow { - for (k, v) in &workflow.metadata { - out.insert(k.clone(), v.clone()); - } - } - if let Some(run) = &self.run { - for (k, v) in &run.metadata { - out.insert(k.clone(), v.clone()); - } - } - out - } -} - -/// Resolve a goal-file path string against `base_dir`. Absolute paths are -/// used as-is; relative paths are joined onto `base_dir`. -#[must_use] -pub fn resolve_goal_file_path(path_str: &str, base_dir: &Path) -> PathBuf { - let path = Path::new(path_str); - if path.is_absolute() { - path.to_path_buf() - } else { - base_dir.join(path) - } -} - -/// Error returned by [`SettingsFile::resolve_run_goal`]. -#[derive(Debug)] -pub enum ResolveGoalError { - /// The `run.goal.file` InterpString referenced an env var that wasn't - /// set at consume time. - EnvLookup { var: String }, - /// The goal file exists in config but could not be read. - Io { - path: PathBuf, - source: std::io::Error, - }, -} - -impl std::fmt::Display for ResolveGoalError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::EnvLookup { var } => write!( - f, - "failed to resolve run.goal.file: env var {var:?} referenced by ${{env.{var}}} is not set" - ), - Self::Io { path, .. } => { - write!(f, "failed to read run.goal.file at {}", path.display()) - } - } - } -} - -impl std::error::Error for ResolveGoalError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::EnvLookup { .. } => None, - Self::Io { source, .. } => Some(source), - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::settings::run::{RunLayer, RunModelLayer}; - - #[test] - fn run_goal_inline_str_returns_source_value() { - let file = SettingsFile { - run: Some(RunLayer { - goal: Some(RunGoalLayer::Inline(InterpString::parse("Implement OAuth"))), - ..RunLayer::default() - }), - ..SettingsFile::default() - }; - assert_eq!( - file.run_goal_inline_str().as_deref(), - Some("Implement OAuth") - ); - } - - #[test] - fn run_goal_inline_str_is_none_for_file_variant() { - let file = SettingsFile { - run: Some(RunLayer { - goal: Some(RunGoalLayer::File { - file: InterpString::parse("/abs/goal.md"), - }), - ..RunLayer::default() - }), - ..SettingsFile::default() - }; - assert_eq!(file.run_goal_inline_str(), None); - assert!(matches!( - file.run_goal_layer(), - Some(RunGoalLayer::File { .. }) - )); - } - - #[test] - fn resolve_run_goal_reads_file_variant_from_disk() { - let tmp = tempfile::tempdir().unwrap(); - let goal_path = tmp.path().join("goal.md"); - std::fs::write(&goal_path, "ship the thing").unwrap(); - - let file = SettingsFile { - run: Some(RunLayer { - goal: Some(RunGoalLayer::File { - file: InterpString::parse(goal_path.to_str().unwrap()), - }), - ..RunLayer::default() - }), - ..SettingsFile::default() - }; - - let resolved = file - .resolve_run_goal(tmp.path()) - .expect("goal file should resolve") - .expect("goal should be set"); - assert_eq!(resolved.text, "ship the thing"); - assert!(matches!( - resolved.source, - ResolvedGoalSource::File { ref path } if path == &goal_path - )); - } - - #[test] - fn resolve_run_goal_inline_passes_text_through() { - let file = SettingsFile { - run: Some(RunLayer { - goal: Some(RunGoalLayer::Inline(InterpString::parse("literal goal"))), - ..RunLayer::default() - }), - ..SettingsFile::default() - }; - let resolved = file - .resolve_run_goal(std::path::Path::new("/")) - .unwrap() - .unwrap(); - assert_eq!(resolved.text, "literal goal"); - assert_eq!(resolved.source, ResolvedGoalSource::Inline); - } - - #[test] - fn run_model_name_str_walks_tree() { - let file = SettingsFile { - run: Some(RunLayer { - model: Some(RunModelLayer { - name: Some(InterpString::parse("claude-sonnet-4-6")), - ..RunModelLayer::default() - }), - ..RunLayer::default() - }), - ..SettingsFile::default() - }; - assert_eq!( - file.run_model_name_str().as_deref(), - Some("claude-sonnet-4-6") - ); - } - - #[test] - fn all_labels_merges_project_workflow_run() { - use crate::settings::project::ProjectLayer; - use crate::settings::workflow::WorkflowLayer; - - let mut project_metadata = HashMap::new(); - project_metadata.insert("env".into(), "project".into()); - project_metadata.insert("team".into(), "core".into()); - - let mut workflow_metadata = HashMap::new(); - workflow_metadata.insert("env".into(), "workflow".into()); - - let mut run_metadata = HashMap::new(); - run_metadata.insert("priority".into(), "high".into()); - - let file = SettingsFile { - project: Some(ProjectLayer { - metadata: project_metadata, - ..ProjectLayer::default() - }), - workflow: Some(WorkflowLayer { - metadata: workflow_metadata, - ..WorkflowLayer::default() - }), - run: Some(RunLayer { - metadata: run_metadata, - ..RunLayer::default() - }), - ..SettingsFile::default() - }; - - let labels = file.all_labels(); - assert_eq!(labels.get("env").map(String::as_str), Some("workflow")); - assert_eq!(labels.get("team").map(String::as_str), Some("core")); - assert_eq!(labels.get("priority").map(String::as_str), Some("high")); - } -} diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index 0b93d3bef..a0ad9d935 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -9,13 +9,13 @@ //! `settings/v2/` subdirectory, so the `::v2::` path prefix no longer //! exists. -pub mod accessors; pub mod cli; pub mod duration; pub mod features; pub mod interp; pub mod model_ref; pub mod project; +pub mod resolved; pub mod run; pub mod server; pub mod size; @@ -36,6 +36,7 @@ pub use model_ref::{ AmbiguousModelRef, ModelRef, ModelRegistry, ParseModelRefError, ResolvedModelRef, }; pub use project::{ProjectLayer, ProjectSettings}; +pub use resolved::Settings; pub use run::{ ArtifactsSettings, DaytonaSettings, DaytonaSnapshotSettings, DockerfileSource, GitAuthorSettings, HookDefinition, HookType, InterviewProviderSettings, McpServerSettings, diff --git a/lib/crates/fabro-types/src/settings/resolved.rs b/lib/crates/fabro-types/src/settings/resolved.rs new file mode 100644 index 000000000..3382ff034 --- /dev/null +++ b/lib/crates/fabro-types/src/settings/resolved.rs @@ -0,0 +1,14 @@ +use super::{ + CliSettings, FeaturesSettings, ProjectSettings, RunSettings, ServerSettings, WorkflowSettings, +}; + +/// A fully resolved settings view across all namespaces. +#[derive(Debug, Clone, Default, PartialEq)] +pub struct Settings { + pub project: ProjectSettings, + pub workflow: WorkflowSettings, + pub run: RunSettings, + pub cli: CliSettings, + pub server: ServerSettings, + pub features: FeaturesSettings, +} diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index c2f072b36..51d25459d 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -16,7 +16,7 @@ use super::interp::InterpString; use super::model_ref::ModelRef; /// A structurally resolved `[run]` view for consumers. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, Default, PartialEq)] pub struct RunSettings { pub goal: Option, pub working_dir: Option, @@ -37,30 +37,6 @@ pub struct RunSettings { pub artifacts: ArtifactsSettings, } -impl Default for RunSettings { - fn default() -> Self { - Self { - goal: None, - working_dir: None, - metadata: HashMap::new(), - inputs: HashMap::new(), - model: RunModelSettings::default(), - git: RunGitSettings::default(), - prepare: RunPrepareSettings::default(), - execution: RunExecutionSettings::default(), - checkpoint: RunCheckpointSettings::default(), - sandbox: RunSandboxSettings::default(), - notifications: HashMap::new(), - interviews: RunInterviewsSettings::default(), - agent: RunAgentSettings::default(), - hooks: Vec::new(), - scm: RunScmSettings::default(), - pull_request: None, - artifacts: ArtifactsSettings::default(), - } - } -} - /// The resolved source of a run goal. #[derive(Debug, Clone, PartialEq)] pub enum RunGoal { @@ -322,7 +298,7 @@ impl HookDefinition { #[must_use] pub fn is_blocking(&self) -> bool { - self.blocking.unwrap_or_else(|| { + self.blocking.unwrap_or({ matches!( self.event, HookEvent::RunStart diff --git a/lib/crates/fabro-types/src/settings/server.rs b/lib/crates/fabro-types/src/settings/server.rs index 34b4aff5e..f7cfc0729 100644 --- a/lib/crates/fabro-types/src/settings/server.rs +++ b/lib/crates/fabro-types/src/settings/server.rs @@ -15,7 +15,7 @@ use super::duration::Duration as DurationLayer; use super::interp::InterpString; /// A structurally resolved `[server]` view for consumers. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct ServerSettings { pub listen: ServerListenSettings, pub api: ServerApiSettings, @@ -29,23 +29,6 @@ pub struct ServerSettings { pub integrations: ServerIntegrationsSettings, } -impl Default for ServerSettings { - fn default() -> Self { - Self { - listen: ServerListenSettings::default(), - api: ServerApiSettings::default(), - web: ServerWebSettings::default(), - auth: ServerAuthSettings::default(), - storage: ServerStorageSettings::default(), - artifacts: ServerArtifactsSettings::default(), - slatedb: ServerSlateDbSettings::default(), - scheduler: ServerSchedulerSettings::default(), - logging: ServerLoggingSettings::default(), - integrations: ServerIntegrationsSettings::default(), - } - } -} - #[derive(Debug, Clone, PartialEq, Eq)] pub enum ServerListenSettings { Tcp { diff --git a/lib/crates/fabro-workflow/src/operations/create.rs b/lib/crates/fabro-workflow/src/operations/create.rs index 1eb5b2c78..bcdcd0eec 100644 --- a/lib/crates/fabro-workflow/src/operations/create.rs +++ b/lib/crates/fabro-workflow/src/operations/create.rs @@ -3,7 +3,8 @@ use fabro_graphviz::graph::{AttrValue, Graph}; use fabro_model::Catalog; use fabro_sandbox::SandboxProvider; use fabro_store::Database; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::run::RunMode; +use fabro_types::settings::{Settings, SettingsFile}; use fabro_types::{RunId, RunProvenance}; use std::collections::BTreeMap; use std::collections::HashMap; @@ -72,7 +73,7 @@ pub async fn create(store: &Database, request: CreateRunInput) -> Result Result Result FabroError { FabroError::engine(err.to_string()) } +fn render_resolve_errors(errors: &[fabro_config::ResolveError]) -> String { + errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("; ") +} + +fn resolve_settings_tree(settings: &SettingsFile) -> Result { + fabro_config::resolve(settings) + .map_err(|errors| FabroError::Precondition(render_resolve_errors(&errors))) +} + +fn combined_labels(settings: &Settings) -> HashMap { + let mut labels = settings.project.metadata.clone(); + labels.extend(settings.workflow.metadata.clone()); + labels.extend(settings.run.metadata.clone()); + labels +} + fn validate_sandbox_provider(settings: &SettingsFile) -> Result<(), FabroError> { - let resolved = fabro_config::resolve_run_from_file(settings).map_err(|errors| { - FabroError::Precondition( - errors - .iter() - .map(ToString::to_string) - .collect::>() - .join("; "), - ) - })?; + let resolved = fabro_config::resolve_run_from_file(settings) + .map_err(|errors| FabroError::Precondition(render_resolve_errors(&errors)))?; resolved .sandbox .provider @@ -304,7 +330,7 @@ pub(super) fn preprocess_and_validate( settings: Option<&SettingsFile>, goal_override: Option<&str>, ) -> Result { - let source = match settings.and_then(SettingsFile::run_inputs_as_strings) { + let source = match run_inputs_as_strings(settings) { Some(mut vars) => { vars.insert("goal".to_string(), "$goal".to_string()); expand_vars(dot_source, &vars) @@ -327,6 +353,23 @@ pub(super) fn preprocess_and_validate( Ok(pipeline::validate(transformed, &[])) } +fn run_inputs_as_strings(settings: Option<&SettingsFile>) -> Option> { + settings + .and_then(|settings| settings.run.as_ref()) + .and_then(|run| run.inputs.as_ref()) + .map(|inputs| { + inputs + .iter() + .map(|(key, value)| { + let stringified = value + .as_str() + .map_or_else(|| value.to_string(), ToString::to_string); + (key.clone(), stringified) + }) + .collect() + }) +} + fn apply_goal_override(graph: &mut Graph, goal_override: Option<&str>) { if let Some(goal_override) = goal_override { graph.attrs.insert( @@ -353,7 +396,7 @@ fn persist_validated( provenance, } = options; - let settings = materialize_run(settings, validated.graph(), &Catalog::builtin()); + let settings = materialize_run(settings, validated.graph(), Catalog::builtin()); let run_id = run_id.unwrap_or_else(RunId::new); let run_dir = run_dir.unwrap_or_else(|| default_run_dir(&run_id)); @@ -762,38 +805,42 @@ mod tests { assert_eq!(created.run_id, fixtures::RUN_1); assert_eq!(created.persisted.run_record().graph.goal(), "override goal"); assert_eq!( - created - .persisted - .run_record() - .settings - .run_model_name_str() + fabro_config::resolve_run_from_file(&created.persisted.run_record().settings) + .unwrap() + .model + .name + .as_ref() + .map(|value| value.as_source()) .as_deref(), Some("claude-sonnet-4-6") ); assert_eq!( - created - .persisted - .run_record() - .settings - .run_model_provider_str() + fabro_config::resolve_run_from_file(&created.persisted.run_record().settings) + .unwrap() + .model + .provider + .as_ref() + .map(|value| value.as_source()) .as_deref(), Some("anthropic") ); assert_eq!( - created - .persisted - .run_record() - .settings - .run_goal_inline_str() - .as_deref(), + match fabro_config::resolve_run_from_file(&created.persisted.run_record().settings) + .unwrap() + .goal + { + Some(fabro_types::settings::run::RunGoal::Inline(value)) => { + Some(value.as_source()) + } + _ => None, + } + .as_deref(), Some("override goal") ); assert!( - created - .persisted - .run_record() - .settings - .run_pull_request() + fabro_config::resolve_run_from_file(&created.persisted.run_record().settings) + .unwrap() + .pull_request .is_none() ); assert_eq!( diff --git a/lib/crates/fabro-workflow/src/operations/source.rs b/lib/crates/fabro-workflow/src/operations/source.rs index 93c1e5321..aa126c75e 100644 --- a/lib/crates/fabro-workflow/src/operations/source.rs +++ b/lib/crates/fabro-workflow/src/operations/source.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use anyhow::Context; use fabro_config::project as project_config; +use fabro_config::run::resolve_run_goal; use fabro_types::settings::SettingsFile; use crate::file_resolver::{FileResolver, FilesystemFileResolver}; @@ -137,8 +138,7 @@ fn resolve_goal_override( settings: &SettingsFile, working_directory: &Path, ) -> anyhow::Result> { - settings - .resolve_run_goal(working_directory) + resolve_run_goal(settings, working_directory) .map(|opt| opt.map(|resolved| resolved.text)) .map_err(anyhow::Error::from) } diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index 44f61eac1..ea9fa6357 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -6,16 +6,21 @@ use std::time::{Duration, Instant}; use fabro_config::project as project_config; use fabro_interview::{AutoApproveInterviewer, Interviewer}; +use fabro_mcp::config::{McpServerSettings, McpTransport}; use fabro_model::{Catalog, FallbackTarget, Provider}; use fabro_sandbox::config::{self as sandbox_config, WorktreeMode, bridge_worktree_mode}; +use fabro_sandbox::config::{ + DaytonaNetwork, DaytonaSnapshotSettings, DockerfileSource as SandboxDockerfileSource, +}; use fabro_sandbox::{SandboxProvider, SandboxSpec}; use fabro_types::RunId; use fabro_types::settings::InterpString; use fabro_types::settings::run::{ + ApprovalMode, DaytonaNetworkLayer, DaytonaSettings, DockerfileSource as ResolvedDockerfileSource, HookDefinition as ResolvedHookDefinition, HookEvent as ResolvedHookEvent, HookType as ResolvedHookType, McpServerSettings as ResolvedMcpServerSettings, McpTransport as ResolvedMcpTransport, - PullRequestSettings, RunModelSettings as ResolvedRunModelSettings, + PullRequestSettings, RunMode, RunModelSettings as ResolvedRunModelSettings, RunSettings as ResolvedRunSettings, TlsMode as ResolvedTlsMode, }; @@ -302,20 +307,16 @@ impl RunSession { .map_err(|errors| FabroError::Precondition(render_resolve_errors(&errors)))?; let sandbox_provider = resolve_sandbox_provider(&resolved)?; - let sandbox_provider = if resolved.execution.mode - == fabro_types::settings::run::RunMode::DryRun - && !sandbox_provider.is_local() - { - SandboxProvider::Local - } else { - sandbox_provider - }; - let model = resolved - .model - .name - .as_ref() - .map(InterpString::as_source) - .unwrap_or_else(|| Catalog::builtin().default_from_env().id.clone()); + let sandbox_provider = + if resolved.execution.mode == RunMode::DryRun && !sandbox_provider.is_local() { + SandboxProvider::Local + } else { + sandbox_provider + }; + let model = resolved.model.name.as_ref().map_or_else( + || Catalog::builtin().default_from_env().id.clone(), + InterpString::as_source, + ); let provider = resolved .model .provider @@ -362,9 +363,14 @@ impl RunSession { .iter() .map(|(k, v)| (k.clone(), resolve_interp(v))) .collect(); + let resolved_server = fabro_config::resolve_server_from_file(settings) + .map_err(|errors| FabroError::Precondition(render_resolve_errors(&errors)))?; let github_permissions: Option> = - settings.github_permissions().map(|perms| { - perms + (!resolved_server.integrations.github.permissions.is_empty()).then(|| { + resolved_server + .integrations + .github + .permissions .iter() .map(|(k, v)| (k.clone(), resolve_interp(v))) .collect() @@ -381,12 +387,12 @@ impl RunSession { resolve_dir: working_directory.clone(), }); - let interviewer: Arc = - if resolved.execution.approval == fabro_types::settings::run::ApprovalMode::Auto { - Arc::new(AutoApproveInterviewer) - } else { - services.interviewer - }; + let interviewer: Arc = if resolved.execution.approval == ApprovalMode::Auto + { + Arc::new(AutoApproveInterviewer) + } else { + services.interviewer + }; let pr_config = resolved.pull_request.clone(); @@ -401,7 +407,7 @@ impl RunSession { provider: provider_enum, fallback_chain, mcp_servers, - dry_run: resolved.execution.mode == fabro_types::settings::run::RunMode::DryRun, + dry_run: resolved.execution.mode == RunMode::DryRun, }, interviewer, on_node: services.on_node, @@ -457,11 +463,12 @@ async fn load_accepted_run_definition( } fn resolve_sandbox_provider(settings: &ResolvedRunSettings) -> Result { - Some(settings.sandbox.provider.as_str()) - .map(str::parse::) - .transpose() - .map_err(|err| FabroError::Precondition(format!("Invalid sandbox provider: {err}")))? - .map_or_else(|| Ok(SandboxProvider::default()), Ok) + Some(str::parse::( + settings.sandbox.provider.as_str(), + )) + .transpose() + .map_err(|err| FabroError::Precondition(format!("Invalid sandbox provider: {err}")))? + .map_or_else(|| Ok(SandboxProvider::default()), Ok) } fn resolve_worktree_mode(settings: &ResolvedRunSettings) -> sandbox_config::WorktreeMode { @@ -509,41 +516,37 @@ fn render_resolve_errors(errors: &[fabro_config::ResolveError]) -> String { .join("; ") } -fn runtime_mcp_server( - settings: &ResolvedMcpServerSettings, -) -> fabro_mcp::config::McpServerSettings { - fabro_mcp::config::McpServerSettings { +fn runtime_mcp_server(settings: &ResolvedMcpServerSettings) -> McpServerSettings { + McpServerSettings { name: settings.name.clone(), transport: match &settings.transport { - ResolvedMcpTransport::Stdio { command, env } => { - fabro_mcp::config::McpTransport::Stdio { - command: command.clone(), - env: env.clone(), - } - } - ResolvedMcpTransport::Http { url, headers } => fabro_mcp::config::McpTransport::Http { + ResolvedMcpTransport::Stdio { command, env } => McpTransport::Stdio { + command: command.clone(), + env: env.clone(), + }, + ResolvedMcpTransport::Http { url, headers } => McpTransport::Http { url: url.clone(), headers: headers.clone(), }, - ResolvedMcpTransport::Sandbox { command, port, env } => { - fabro_mcp::config::McpTransport::Sandbox { - command: command.clone(), - port: *port, - env: env.clone(), - } - } + ResolvedMcpTransport::Sandbox { command, port, env } => McpTransport::Sandbox { + command: command.clone(), + port: *port, + env: env.clone(), + }, }, startup_timeout_secs: settings.startup_timeout_secs, tool_timeout_secs: settings.tool_timeout_secs, } } -fn runtime_daytona_config(settings: &fabro_types::settings::run::DaytonaSettings) -> DaytonaConfig { +fn runtime_daytona_config(settings: &DaytonaSettings) -> DaytonaConfig { DaytonaConfig { auto_stop_interval: settings.auto_stop_interval, labels: (!settings.labels.is_empty()).then_some(settings.labels.clone()), - snapshot: settings.snapshot.as_ref().map(|snapshot| { - fabro_sandbox::config::DaytonaSnapshotSettings { + snapshot: settings + .snapshot + .as_ref() + .map(|snapshot| DaytonaSnapshotSettings { name: snapshot.name.clone(), cpu: snapshot.cpu, memory: snapshot.memory_gb, @@ -553,23 +556,18 @@ fn runtime_daytona_config(settings: &fabro_types::settings::run::DaytonaSettings .as_ref() .map(|dockerfile| match dockerfile { ResolvedDockerfileSource::Inline(text) => { - fabro_sandbox::config::DockerfileSource::Inline(text.clone()) + SandboxDockerfileSource::Inline(text.clone()) } ResolvedDockerfileSource::Path { path } => { - fabro_sandbox::config::DockerfileSource::Path { path: path.clone() } + SandboxDockerfileSource::Path { path: path.clone() } } }), - } - }), + }), network: settings.network.as_ref().map(|network| match network { - fabro_types::settings::run::DaytonaNetworkLayer::Block => { - fabro_sandbox::config::DaytonaNetwork::Block - } - fabro_types::settings::run::DaytonaNetworkLayer::AllowAll => { - fabro_sandbox::config::DaytonaNetwork::AllowAll - } - fabro_types::settings::run::DaytonaNetworkLayer::AllowList { allow_list } => { - fabro_sandbox::config::DaytonaNetwork::AllowList(allow_list.clone()) + DaytonaNetworkLayer::Block => DaytonaNetwork::Block, + DaytonaNetworkLayer::AllowAll => DaytonaNetwork::AllowAll, + DaytonaNetworkLayer::AllowList { allow_list } => { + DaytonaNetwork::AllowList(allow_list.clone()) } }), skip_clone: settings.skip_clone, diff --git a/lib/crates/fabro-workflow/src/run_lookup.rs b/lib/crates/fabro-workflow/src/run_lookup.rs index ed0546c2c..e6d393ced 100644 --- a/lib/crates/fabro-workflow/src/run_lookup.rs +++ b/lib/crates/fabro-workflow/src/run_lookup.rs @@ -6,7 +6,6 @@ use chrono::{DateTime, Utc}; use fabro_config::Storage; use fabro_store::{Database, RunSummary}; use fabro_types::RunId; -use fabro_types::settings::SettingsFile; use serde::Serialize; use crate::operations::make_run_dir; @@ -142,7 +141,7 @@ pub fn scratch_base(storage_dir: &Path) -> PathBuf { } pub fn default_scratch_base() -> PathBuf { - scratch_base(&SettingsFile::default().storage_dir()) + scratch_base(&fabro_util::Home::from_env().storage_dir()) } fn scan_orphan_runs(base: &Path) -> Result> { diff --git a/lib/crates/fabro-workflow/src/run_options.rs b/lib/crates/fabro-workflow/src/run_options.rs index a1ca65c7a..6f3dbe044 100644 --- a/lib/crates/fabro-workflow/src/run_options.rs +++ b/lib/crates/fabro-workflow/src/run_options.rs @@ -5,6 +5,7 @@ use std::sync::atomic::AtomicBool; use fabro_types::RunId; use fabro_types::settings::SettingsFile; +use fabro_types::settings::run::RunMode; use crate::git::{GitAuthor, git_author_from_settings}; @@ -43,7 +44,7 @@ pub struct RunOptions { impl RunOptions { pub fn dry_run_enabled(&self) -> bool { fabro_config::resolve_run_from_file(&self.settings) - .map(|settings| settings.execution.mode == fabro_types::settings::run::RunMode::DryRun) + .map(|settings| settings.execution.mode == RunMode::DryRun) .unwrap_or(false) } diff --git a/lib/crates/fabro-workflow/tests/materialize_run.rs b/lib/crates/fabro-workflow/tests/materialize_run.rs index 1b2889899..f86f2152b 100644 --- a/lib/crates/fabro-workflow/tests/materialize_run.rs +++ b/lib/crates/fabro-workflow/tests/materialize_run.rs @@ -33,18 +33,29 @@ fn materialize_run_applies_graph_and_catalog_defaults() { }; let materialized = materialize_run(settings, &graph(source), &Catalog::builtin()); + let resolved = fabro_config::resolve_run_from_file(&materialized).unwrap(); assert_eq!( - materialized.run_model_name_str().as_deref(), + resolved + .model + .name + .as_ref() + .map(|value| value.as_source()) + .as_deref(), Some("claude-sonnet-4-6") ); assert_eq!( - materialized.run_model_provider_str().as_deref(), + resolved + .model + .provider + .as_ref() + .map(|value| value.as_source()) + .as_deref(), Some("anthropic") ); assert_eq!( - materialized.run_goal_layer(), + materialized.run.as_ref().and_then(|run| run.goal.as_ref()), Some(&RunGoalLayer::Inline(InterpString::parse("Build feature"))) ); - assert!(materialized.run_pull_request().is_none()); + assert!(resolved.pull_request.is_none()); }