From e1ea66a833913cc5c3966e43782812565e1c6f09 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 10 Apr 2026 06:54:31 -0400 Subject: [PATCH] refactor(settings): resolve run settings and materialize defaults Add the resolved run namespace, materialize persisted run defaults at create time, and migrate the main workflow/server/CLI runtime paths off the old run bridges. --- lib/crates/fabro-checkpoint/src/author.rs | 11 +- lib/crates/fabro-cli/src/commands/exec.rs | 143 ++++- .../fabro-cli/src/commands/run/attach.rs | 11 +- .../fabro-cli/src/commands/run/runner.rs | 14 +- lib/crates/fabro-config/src/lib.rs | 4 +- lib/crates/fabro-config/src/project.rs | 7 +- lib/crates/fabro-config/src/resolve/mod.rs | 15 +- lib/crates/fabro-config/src/resolve/run.rs | 487 ++++++++++++++++++ lib/crates/fabro-config/tests/resolve_run.rs | 60 +++ lib/crates/fabro-mcp/src/config.rs | 282 +--------- lib/crates/fabro-server/src/run_manifest.rs | 134 +++-- lib/crates/fabro-server/src/server.rs | 11 +- lib/crates/fabro-server/src/web_auth.rs | 7 +- lib/crates/fabro-types/src/settings/mod.rs | 9 +- lib/crates/fabro-types/src/settings/run.rs | 393 ++++++++++++++ lib/crates/fabro-workflow/src/config.rs | 71 --- lib/crates/fabro-workflow/src/git.rs | 7 +- lib/crates/fabro-workflow/src/lib.rs | 2 +- .../fabro-workflow/src/lifecycle/git.rs | 2 +- .../fabro-workflow/src/lifecycle/mod.rs | 2 +- .../fabro-workflow/src/operations/create.rs | 95 +--- .../fabro-workflow/src/operations/start.rs | 301 ++++++++--- .../fabro-workflow/src/pipeline/execute.rs | 2 +- .../src/pipeline/pull_request.rs | 2 +- .../fabro-workflow/src/pipeline/types.rs | 2 +- .../fabro-workflow/src/run_materialization.rs | 70 +++ lib/crates/fabro-workflow/src/run_options.rs | 26 +- .../fabro-workflow/tests/materialize_run.rs | 50 ++ 28 files changed, 1607 insertions(+), 613 deletions(-) create mode 100644 lib/crates/fabro-config/src/resolve/run.rs create mode 100644 lib/crates/fabro-config/tests/resolve_run.rs delete mode 100644 lib/crates/fabro-workflow/src/config.rs create mode 100644 lib/crates/fabro-workflow/src/run_materialization.rs create mode 100644 lib/crates/fabro-workflow/tests/materialize_run.rs diff --git a/lib/crates/fabro-checkpoint/src/author.rs b/lib/crates/fabro-checkpoint/src/author.rs index a386bc898..9cd815406 100644 --- a/lib/crates/fabro-checkpoint/src/author.rs +++ b/lib/crates/fabro-checkpoint/src/author.rs @@ -1,7 +1,7 @@ use std::fmt::Write; use fabro_types::settings::InterpString; -use fabro_types::settings::run::GitAuthorLayer; +use fabro_types::settings::run::{GitAuthorLayer, GitAuthorSettings}; /// Resolved git author identity for checkpoint commits. #[derive(Debug, Clone, PartialEq)] @@ -58,3 +58,12 @@ impl From<&GitAuthorLayer> for GitAuthor { ) } } + +impl From<&GitAuthorSettings> for GitAuthor { + fn from(value: &GitAuthorSettings) -> Self { + Self::from_options( + value.name.as_ref().map(InterpString::as_source), + value.email.as_ref().map(InterpString::as_source), + ) + } +} diff --git a/lib/crates/fabro-cli/src/commands/exec.rs b/lib/crates/fabro-cli/src/commands/exec.rs index 1d54dc177..9511e1a9d 100644 --- a/lib/crates/fabro-cli/src/commands/exec.rs +++ b/lib/crates/fabro-cli/src/commands/exec.rs @@ -2,14 +2,98 @@ 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, bridge_mcp_entry}; use fabro_types::settings::InterpString; +use fabro_types::settings::run::McpEntryLayer; use std::collections::HashMap; 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 { + let transport = match entry { + McpEntryLayer::Stdio { + script, + command, + env, + .. + } => { + let command = if let Some(script) = script { + vec!["sh".to_string(), "-c".to_string(), script.as_source()] + } else { + command + .as_ref() + .map(|command| command.iter().map(InterpString::as_source).collect()) + .unwrap_or_default() + }; + fabro_mcp::config::McpTransport::Stdio { + command, + env: env + .iter() + .map(|(key, value)| (key.clone(), value.as_source())) + .collect(), + } + } + McpEntryLayer::Http { url, headers, .. } => fabro_mcp::config::McpTransport::Http { + url: url.as_source(), + headers: headers + .iter() + .map(|(key, value)| (key.clone(), value.as_source())) + .collect(), + }, + McpEntryLayer::Sandbox { + script, + command, + port, + env, + .. + } => { + let command = if let Some(script) = script { + vec!["sh".to_string(), "-c".to_string(), script.as_source()] + } else { + command + .as_ref() + .map(|command| command.iter().map(InterpString::as_source).collect()) + .unwrap_or_default() + }; + fabro_mcp::config::McpTransport::Sandbox { + command, + port: *port, + env: env + .iter() + .map(|(key, value)| (key.clone(), value.as_source())) + .collect(), + } + } + }; + let (startup_timeout_secs, tool_timeout_secs) = match entry { + McpEntryLayer::Http { + startup_timeout, + tool_timeout, + .. + } + | McpEntryLayer::Stdio { + startup_timeout, + tool_timeout, + .. + } + | McpEntryLayer::Sandbox { + startup_timeout, + tool_timeout, + .. + } => ( + startup_timeout.map_or(10, |duration| duration.as_std().as_secs()), + tool_timeout.map_or(60, |duration| duration.as_std().as_secs()), + ), + }; + fabro_mcp::config::McpServerSettings { + name: name.to_string(), + transport, + startup_timeout_secs, + tool_timeout_secs, + } +} + pub(crate) async fn execute(mut args: ExecArgs, globals: &GlobalArgs) -> Result<()> { use fabro_agent::cli::PermissionLevel as AgentPermissionLevel; use fabro_types::settings::run::AgentPermissions; @@ -46,17 +130,52 @@ 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 mcps_iter = exec_agent - .map(|a| &a.mcps) - .filter(|m| !m.is_empty()) - .or_else(|| cli_settings.run_agent_mcps()); - let mcp_servers: Vec = mcps_iter - .map(|mcps| { - mcps.iter() - .map(|(name, entry)| bridge_mcp_entry(entry).into_config(name.clone())) - .collect() - }) - .unwrap_or_default(); + let mcp_servers: Vec = if let Some(mcps) = exec_agent + .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: match &server.transport { + fabro_types::settings::run::McpTransport::Stdio { command, env } => { + fabro_mcp::config::McpTransport::Stdio { + command: command.clone(), + env: env.clone(), + } + } + fabro_types::settings::run::McpTransport::Http { url, headers } => { + fabro_mcp::config::McpTransport::Http { + url: url.clone(), + headers: headers.clone(), + } + } + fabro_types::settings::run::McpTransport::Sandbox { + command, + port, + env, + } => fabro_mcp::config::McpTransport::Sandbox { + command: command.clone(), + port: *port, + env: env.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/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index 6009f64c1..221b20e1e 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -62,10 +62,13 @@ pub(crate) async fn attach_run_with_client( json_output: bool, ) -> Result { let state = client.get_run_state(run_id).await?; - let auto_approve = state - .run - .as_ref() - .is_some_and(|record| record.settings.auto_approve_enabled()); + 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 + }) + .unwrap_or(false) + }); let verbose = state .run .as_ref() diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index 62e6e8c34..330dc7aad 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -421,13 +421,13 @@ fn update_worker_title_from_event(event: &RunEvent) { fn maybe_build_github_app_credentials( settings: &SettingsFile, ) -> Result> { - let needs_github_app = settings - .run_sandbox() - .and_then(|sandbox| sandbox.provider.as_deref()) - .is_some_and(|provider| provider == "daytona") - || settings - .run_pull_request() - .is_some_and(|pr| pr.enabled.unwrap_or(false)) + let resolved_run = fabro_config::resolve_run_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(); if needs_github_app { diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index f9a60f070..cf3e0d949 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -14,7 +14,9 @@ pub mod user; pub use config::ConfigLayer; pub use fabro_util::path::expand_tilde; pub use home::Home; -pub use resolve::{ResolveError, resolve_server, resolve_server_from_file}; +pub use resolve::{ + ResolveError, resolve_run, resolve_run_from_file, resolve_server, resolve_server_from_file, +}; pub use storage::{RunScratch, ServerState, Storage}; use std::path::{Path, PathBuf}; diff --git a/lib/crates/fabro-config/src/project.rs b/lib/crates/fabro-config/src/project.rs index 2c49cf426..7a3c1149c 100644 --- a/lib/crates/fabro-config/src/project.rs +++ b/lib/crates/fabro-config/src/project.rs @@ -125,7 +125,12 @@ pub fn resolve_workflow_path( } pub fn resolve_working_directory(settings: &SettingsFile, caller_cwd: &Path) -> PathBuf { - let Some(work_dir) = settings.run_working_dir().map(InterpString::as_source) else { + let Some(work_dir) = settings + .run + .as_ref() + .and_then(|run| run.working_dir.as_ref()) + .map(InterpString::as_source) + else { return caller_cwd.to_path_buf(); }; let path = PathBuf::from(&work_dir); diff --git a/lib/crates/fabro-config/src/resolve/mod.rs b/lib/crates/fabro-config/src/resolve/mod.rs index ea00dbe8a..9e8ad7692 100644 --- a/lib/crates/fabro-config/src/resolve/mod.rs +++ b/lib/crates/fabro-config/src/resolve/mod.rs @@ -1,9 +1,11 @@ mod error; +mod run; mod server; -use fabro_types::settings::{ServerSettings, SettingsFile}; +use fabro_types::settings::{RunSettings, ServerSettings, SettingsFile}; pub use error::ResolveError; +pub use run::resolve_run; pub use server::resolve_server; pub fn resolve_server_from_file(file: &SettingsFile) -> Result> { @@ -17,6 +19,17 @@ pub fn resolve_server_from_file(file: &SettingsFile) -> Result 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) + } +} + pub(crate) fn require_interp( value: Option<&fabro_types::settings::InterpString>, path: &str, diff --git a/lib/crates/fabro-config/src/resolve/run.rs b/lib/crates/fabro-config/src/resolve/run.rs new file mode 100644 index 000000000..d9c13ecdc --- /dev/null +++ b/lib/crates/fabro-config/src/resolve/run.rs @@ -0,0 +1,487 @@ +use fabro_types::settings::InterpString; +use fabro_types::settings::run::{ + ApprovalMode, ArtifactsSettings, DaytonaDockerfileLayer, DaytonaSettings, + DaytonaSnapshotSettings, DockerfileSource, GitAuthorSettings, HookAgentMarker, HookDefinition, + HookTlsMode, HookType, InterviewProviderSettings, McpEntryLayer, McpServerSettings, + McpTransport, MergeStrategy, ModelRefOrSplice, 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, +}; + +use super::ResolveError; + +pub fn resolve_run(layer: &RunLayer, errors: &mut Vec) -> RunSettings { + RunSettings { + goal: resolve_goal(layer.goal.as_ref()), + working_dir: layer.working_dir.clone(), + metadata: layer.metadata.clone(), + inputs: layer.inputs.clone().unwrap_or_default(), + model: resolve_model(layer.model.as_ref()), + git: resolve_git(layer.git.as_ref()), + prepare: resolve_prepare(layer.prepare.as_ref(), errors), + execution: resolve_execution(layer.execution.as_ref()), + checkpoint: resolve_checkpoint(layer.checkpoint.as_ref()), + sandbox: resolve_sandbox(layer.sandbox.as_ref(), errors), + notifications: layer + .notifications + .iter() + .map(|(name, route)| (name.clone(), resolve_notification_route(route))) + .collect(), + interviews: resolve_interviews(layer.interviews.as_ref()), + agent: resolve_agent(layer.agent.as_ref()), + hooks: layer + .hooks + .iter() + .enumerate() + .map(|(index, hook)| resolve_hook(hook, index, errors)) + .collect(), + scm: resolve_scm(layer.scm.as_ref()), + pull_request: resolve_pull_request(layer.pull_request.as_ref()), + artifacts: resolve_artifacts(layer.artifacts.as_ref()), + } +} + +fn resolve_goal(goal: Option<&RunGoalLayer>) -> Option { + match goal? { + RunGoalLayer::Inline(value) => Some(RunGoal::Inline(value.clone())), + RunGoalLayer::File { file } => Some(RunGoal::File(file.clone())), + } +} + +fn resolve_model(model: Option<&RunModelLayer>) -> RunModelSettings { + let Some(model) = model else { + return RunModelSettings::default(); + }; + + RunModelSettings { + provider: model.provider.clone(), + name: model.name.clone(), + fallbacks: model + .fallbacks + .iter() + .filter_map(|entry| match entry { + ModelRefOrSplice::ModelRef(model_ref) => Some(model_ref.clone()), + ModelRefOrSplice::Splice => None, + }) + .collect(), + } +} + +fn resolve_git(git: Option<&RunGitLayer>) -> RunGitSettings { + RunGitSettings { + author: git.and_then(|git| { + git.author.as_ref().map(|author| GitAuthorSettings { + name: author.name.clone(), + email: author.email.clone(), + }) + }), + } +} + +fn resolve_prepare( + prepare: Option<&RunPrepareLayer>, + errors: &mut Vec, +) -> RunPrepareSettings { + let Some(prepare) = prepare else { + return RunPrepareSettings::default(); + }; + + let mut commands = Vec::new(); + for (index, step) in prepare.steps.iter().enumerate() { + match (&step.script, &step.command) { + (Some(script), None) => commands.push(script.as_source()), + (None, Some(argv)) => commands.push( + argv.iter() + .map(InterpString::as_source) + .collect::>() + .join(" "), + ), + (Some(_), Some(_)) | (None, None) => errors.push(ResolveError::Invalid { + path: format!("run.prepare.steps[{index}]"), + reason: "exactly one of script or command must be set".to_string(), + }), + } + } + + RunPrepareSettings { + commands, + timeout_ms: prepare.timeout.map_or(300_000, |timeout| { + u64::try_from(timeout.as_std().as_millis()).unwrap_or(u64::MAX) + }), + } +} + +fn resolve_execution(execution: Option<&RunExecutionLayer>) -> RunExecutionSettings { + let Some(execution) = execution else { + return RunExecutionSettings::default(); + }; + + RunExecutionSettings { + mode: execution.mode.unwrap_or(RunMode::Normal), + approval: execution.approval.unwrap_or(ApprovalMode::Prompt), + retros: execution.retros.unwrap_or(true), + } +} + +fn resolve_checkpoint(checkpoint: Option<&RunCheckpointLayer>) -> RunCheckpointSettings { + RunCheckpointSettings { + exclude_globs: checkpoint + .map(|checkpoint| checkpoint.exclude_globs.clone()) + .unwrap_or_default(), + } +} + +fn resolve_sandbox( + sandbox: Option<&RunSandboxLayer>, + errors: &mut Vec, +) -> RunSandboxSettings { + let Some(sandbox) = sandbox else { + return RunSandboxSettings::default(); + }; + + let provider = sandbox + .provider + .clone() + .unwrap_or_else(|| "local".to_string()); + match provider.as_str() { + "local" | "docker" | "daytona" => {} + other => errors.push(ResolveError::Invalid { + path: "run.sandbox.provider".to_string(), + reason: format!("unknown sandbox provider: {other}"), + }), + } + + RunSandboxSettings { + provider, + preserve: sandbox.preserve.unwrap_or(false), + devcontainer: sandbox.devcontainer.unwrap_or(false), + env: sandbox.env.clone(), + local: resolve_local_sandbox(sandbox), + daytona: sandbox.daytona.as_ref().map(resolve_daytona), + } +} + +fn resolve_local_sandbox( + sandbox: &RunSandboxLayer, +) -> fabro_types::settings::run::LocalSandboxSettings { + fabro_types::settings::run::LocalSandboxSettings { + worktree_mode: sandbox + .local + .as_ref() + .and_then(|local| local.worktree_mode) + .unwrap_or_default(), + } +} + +fn resolve_daytona(daytona: &fabro_types::settings::run::DaytonaSandboxLayer) -> DaytonaSettings { + DaytonaSettings { + auto_stop_interval: daytona.auto_stop_interval, + labels: daytona.labels.clone(), + snapshot: daytona.snapshot.as_ref().and_then(|snapshot| { + snapshot.name.as_ref().map(|name| DaytonaSnapshotSettings { + name: name.clone(), + cpu: snapshot.cpu, + memory_gb: snapshot.memory.map(|size| size_to_gb_i32(size.as_bytes())), + disk_gb: snapshot.disk.map(|size| size_to_gb_i32(size.as_bytes())), + dockerfile: snapshot + .dockerfile + .as_ref() + .map(|dockerfile| match dockerfile { + DaytonaDockerfileLayer::Inline(text) => { + DockerfileSource::Inline(text.clone()) + } + DaytonaDockerfileLayer::Path { path } => { + DockerfileSource::Path { path: path.clone() } + } + }), + }) + }), + network: daytona.network.clone(), + skip_clone: daytona.skip_clone.unwrap_or(false), + } +} + +fn resolve_notification_route(route: &NotificationRouteLayer) -> NotificationRouteSettings { + NotificationRouteSettings { + enabled: route.enabled.unwrap_or(false), + provider: route.provider.clone(), + events: route + .events + .iter() + .filter_map(|event| match event { + StringOrSplice::Value(value) => Some(value.clone()), + StringOrSplice::Splice => None, + }) + .collect(), + slack: route.slack.as_ref().map(resolve_notification_provider), + discord: route.discord.as_ref().map(resolve_notification_provider), + teams: route.teams.as_ref().map(resolve_notification_provider), + } +} + +fn resolve_notification_provider( + provider: &fabro_types::settings::run::NotificationProviderLayer, +) -> NotificationProviderSettings { + NotificationProviderSettings { + channel: provider.channel.clone(), + } +} + +fn resolve_interviews( + interviews: Option<&fabro_types::settings::run::InterviewsLayer>, +) -> RunInterviewsSettings { + let Some(interviews) = interviews else { + return RunInterviewsSettings::default(); + }; + + RunInterviewsSettings { + provider: interviews.provider.clone(), + slack: interviews.slack.as_ref().map(resolve_interview_provider), + discord: interviews.discord.as_ref().map(resolve_interview_provider), + teams: interviews.teams.as_ref().map(resolve_interview_provider), + } +} + +fn resolve_interview_provider( + provider: &fabro_types::settings::run::InterviewProviderLayer, +) -> InterviewProviderSettings { + InterviewProviderSettings { + channel: provider.channel.clone(), + } +} + +fn resolve_agent(agent: Option<&RunAgentLayer>) -> RunAgentSettings { + let Some(agent) = agent else { + return RunAgentSettings::default(); + }; + + RunAgentSettings { + permissions: agent.permissions, + mcps: agent + .mcps + .iter() + .map(|(name, entry)| (name.clone(), resolve_mcp_entry(name, entry))) + .collect(), + } +} + +fn resolve_mcp_entry(name: &str, entry: &McpEntryLayer) -> McpServerSettings { + let transport = match entry { + McpEntryLayer::Stdio { + script, + command, + env, + .. + } => McpTransport::Stdio { + command: resolve_mcp_command(script.as_ref(), command.as_ref()), + env: env + .iter() + .map(|(key, value)| (key.clone(), value.as_source())) + .collect(), + }, + McpEntryLayer::Http { url, headers, .. } => McpTransport::Http { + url: url.as_source(), + headers: headers + .iter() + .map(|(key, value)| (key.clone(), value.as_source())) + .collect(), + }, + McpEntryLayer::Sandbox { + script, + command, + port, + env, + .. + } => McpTransport::Sandbox { + command: resolve_mcp_command(script.as_ref(), command.as_ref()), + port: *port, + env: env + .iter() + .map(|(key, value)| (key.clone(), value.as_source())) + .collect(), + }, + }; + + let (startup_timeout_secs, tool_timeout_secs) = match entry { + McpEntryLayer::Http { + startup_timeout, + tool_timeout, + .. + } + | McpEntryLayer::Stdio { + startup_timeout, + tool_timeout, + .. + } + | McpEntryLayer::Sandbox { + startup_timeout, + tool_timeout, + .. + } => ( + startup_timeout.map_or(10, |timeout| timeout.as_std().as_secs()), + tool_timeout.map_or(60, |timeout| timeout.as_std().as_secs()), + ), + }; + + McpServerSettings { + name: name.to_string(), + transport, + startup_timeout_secs, + tool_timeout_secs, + } +} + +fn resolve_mcp_command( + script: Option<&InterpString>, + command: Option<&Vec>, +) -> Vec { + if let Some(script) = script { + return vec!["sh".to_string(), "-c".to_string(), script.as_source()]; + } + command + .map(|command| command.iter().map(InterpString::as_source).collect()) + .unwrap_or_default() +} + +fn resolve_hook( + hook: &fabro_types::settings::run::HookEntry, + index: usize, + errors: &mut Vec, +) -> HookDefinition { + let variants = [ + hook.script.is_some() || hook.command.is_some(), + hook.url.is_some(), + hook.prompt.is_some() && hook.agent.is_none(), + hook.agent == Some(HookAgentMarker::Enabled), + ] + .into_iter() + .filter(|flag| *flag) + .count(); + + if variants != 1 { + errors.push(ResolveError::Invalid { + path: format!("run.hooks[{index}]"), + reason: "exactly one hook transport must be configured".to_string(), + }); + } + + let hook_type = resolve_hook_type(hook); + let command = if let Some(script) = &hook.script { + Some(script.as_source()) + } else { + hook.command.as_ref().map(|command| { + command + .iter() + .map(InterpString::as_source) + .collect::>() + .join(" ") + }) + }; + + HookDefinition { + name: hook.name.clone().or_else(|| hook.id.clone()), + event: hook.event, + command, + hook_type, + matcher: hook.matcher.clone(), + blocking: hook.blocking, + timeout_ms: hook + .timeout + .map(|timeout| u64::try_from(timeout.as_std().as_millis()).unwrap_or(u64::MAX)), + sandbox: hook.sandbox, + } +} + +fn resolve_hook_type(hook: &fabro_types::settings::run::HookEntry) -> Option { + if hook.script.is_some() || hook.command.is_some() { + return None; + } + + if let Some(url) = &hook.url { + let headers = if hook.headers.is_empty() { + None + } else { + Some( + hook.headers + .iter() + .map(|(key, value)| (key.clone(), value.as_source())) + .collect(), + ) + }; + let tls = match hook.tls { + Some(HookTlsMode::Verify) => TlsMode::Verify, + Some(HookTlsMode::NoVerify) => TlsMode::NoVerify, + Some(HookTlsMode::Off) => TlsMode::Off, + None => TlsMode::default(), + }; + return Some(HookType::Http { + url: url.as_source(), + headers, + allowed_env_vars: hook.allowed_env_vars.clone(), + tls, + }); + } + + if hook.agent == Some(HookAgentMarker::Enabled) { + return Some(HookType::Agent { + prompt: hook + .prompt + .as_ref() + .map(InterpString::as_source) + .unwrap_or_default(), + model: hook.model.as_ref().map(InterpString::as_source), + max_tool_rounds: hook.max_tool_rounds, + }); + } + + hook.prompt.as_ref().map(|prompt| HookType::Prompt { + prompt: prompt.as_source(), + model: hook.model.as_ref().map(InterpString::as_source), + }) +} + +fn resolve_scm(scm: Option<&RunScmLayer>) -> RunScmSettings { + let Some(scm) = scm else { + return RunScmSettings::default(); + }; + + RunScmSettings { + provider: scm.provider.clone(), + owner: scm.owner.clone(), + repository: scm.repository.clone(), + github: scm.github.as_ref().map(|_| ScmGitHubSettings), + } +} + +fn resolve_pull_request( + pull_request: Option<&fabro_types::settings::run::RunPullRequestLayer>, +) -> Option { + let pull_request = pull_request?; + if !pull_request.enabled.unwrap_or(false) { + return None; + } + + Some(PullRequestSettings { + enabled: true, + draft: pull_request.draft.unwrap_or(true), + auto_merge: pull_request.auto_merge.unwrap_or(false), + merge_strategy: pull_request.merge_strategy.unwrap_or(MergeStrategy::Squash), + }) +} + +fn resolve_artifacts(artifacts: Option<&RunArtifactsLayer>) -> ArtifactsSettings { + ArtifactsSettings { + include: artifacts + .map(|artifacts| artifacts.include.clone()) + .unwrap_or_default(), + } +} + +fn size_to_gb_i32(bytes: u64) -> i32 { + let gb = bytes / 1_000_000_000; + i32::try_from(gb).unwrap_or(i32::MAX) +} diff --git a/lib/crates/fabro-config/tests/resolve_run.rs b/lib/crates/fabro-config/tests/resolve_run.rs new file mode 100644 index 000000000..49648ebc9 --- /dev/null +++ b/lib/crates/fabro-config/tests/resolve_run.rs @@ -0,0 +1,60 @@ +use fabro_config::ConfigLayer; +use fabro_types::settings::run::{ApprovalMode, RunGoal, RunMode, WorktreeMode}; +use fabro_types::settings::{InterpString, SettingsFile}; + +fn parse(source: &str) -> SettingsFile { + ConfigLayer::parse(source) + .expect("fixture should parse") + .into() +} + +#[test] +fn resolves_run_defaults_from_empty_settings() { + let settings = fabro_config::resolve_run_from_file(&SettingsFile::default()) + .expect("empty settings should resolve"); + + assert_eq!(settings.execution.mode, RunMode::Normal); + assert_eq!(settings.execution.approval, ApprovalMode::Prompt); + assert!(settings.execution.retros); + assert_eq!(settings.prepare.timeout_ms, 300_000); + assert_eq!(settings.sandbox.provider, "local"); + assert_eq!(settings.sandbox.local.worktree_mode, WorktreeMode::Clean); + assert!(settings.pull_request.is_none()); +} + +#[test] +fn preserves_goal_variants_and_model_sources() { + let file = parse( + r#" +_version = 1 + +[run] +working_dir = "${env.FABRO_WORKDIR}" + +[run.goal] +file = "${env.GOAL_FILE}" + +[run.model] +provider = "anthropic" +name = "sonnet" +"#, + ); + + let settings = fabro_config::resolve_run_from_file(&file).expect("run settings should resolve"); + + match settings.goal { + Some(RunGoal::File(path)) => { + assert_eq!(path, InterpString::parse("${env.GOAL_FILE}")); + } + other => panic!("expected file goal, got {other:?}"), + } + assert_eq!( + settings.working_dir, + Some(InterpString::parse("${env.FABRO_WORKDIR}")) + ); + assert_eq!( + settings.model.provider, + Some(InterpString::parse("anthropic")) + ); + assert_eq!(settings.model.name, Some(InterpString::parse("sonnet"))); +} diff --git a/lib/crates/fabro-mcp/src/config.rs b/lib/crates/fabro-mcp/src/config.rs index 6c555b818..0ac19144f 100644 --- a/lib/crates/fabro-mcp/src/config.rs +++ b/lib/crates/fabro-mcp/src/config.rs @@ -1,281 +1 @@ -//! MCP server configuration runtime types. -//! -//! The v2 parse tree lives in `fabro_types::settings::run::McpEntryLayer`. -//! This module owns the runtime shape (flattened, with timeout helpers) that -//! the MCP client consumes at execution time. Conversion from the v2 shape -//! lives in [`bridge_mcp_entry`] / [`bridge_mcps`]. - -use std::collections::HashMap; -use std::time::Duration; - -use fabro_types::settings::InterpString; -use fabro_types::settings::run::McpEntryLayer; -use serde::{Deserialize, Serialize}; - -#[must_use] -pub fn default_startup_timeout_secs() -> u64 { - 10 -} - -#[must_use] -pub fn default_tool_timeout_secs() -> u64 { - 60 -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct McpServerSettings { - pub name: String, - pub transport: McpTransport, - #[serde(default = "default_startup_timeout_secs")] - pub startup_timeout_secs: u64, - #[serde(default = "default_tool_timeout_secs")] - pub tool_timeout_secs: u64, -} - -impl McpServerSettings { - #[must_use] - pub fn startup_timeout(&self) -> Duration { - Duration::from_secs(self.startup_timeout_secs) - } - - #[must_use] - pub fn tool_timeout(&self) -> Duration { - Duration::from_secs(self.tool_timeout_secs) - } -} - -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] -pub enum McpTransport { - Stdio { - command: Vec, - #[serde(default)] - env: HashMap, - }, - Http { - url: String, - #[serde(default)] - headers: HashMap, - }, - /// MCP server that runs inside a sandbox and is accessed via HTTP preview URL. - /// During session init, the server is started inside the sandbox and this - /// variant is resolved into an `Http` transport using the sandbox's preview URL. - Sandbox { - command: Vec, - port: u16, - #[serde(default)] - env: HashMap, - }, -} - -/// MCP server entry as it appears in TOML config files (without a `name` field). -/// -/// Converted to [`McpServerSettings`] via [`McpServerEntry::into_config`]. -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -pub struct McpServerEntry { - #[serde(flatten)] - pub transport: McpTransport, - #[serde(default = "default_startup_timeout_secs")] - pub startup_timeout_secs: u64, - #[serde(default = "default_tool_timeout_secs")] - pub tool_timeout_secs: u64, -} - -impl McpServerEntry { - #[must_use] - pub fn into_config(self, name: String) -> McpServerSettings { - McpServerSettings { - name, - transport: self.transport, - startup_timeout_secs: self.startup_timeout_secs, - tool_timeout_secs: self.tool_timeout_secs, - } - } -} - -/// Convert a map of v2 `McpEntryLayer` entries into runtime `McpServerEntry`s. -#[must_use] -pub fn bridge_mcps(mcps: &HashMap) -> HashMap { - mcps.iter() - .map(|(name, entry)| (name.clone(), bridge_mcp_entry(entry))) - .collect() -} - -/// Convert a single v2 `McpEntryLayer` into the runtime `McpServerEntry`. -#[must_use] -pub fn bridge_mcp_entry(entry: &McpEntryLayer) -> McpServerEntry { - let transport = match entry { - McpEntryLayer::Stdio { - script, - command, - env, - .. - } => { - let command_vec: Vec = if let Some(script) = script { - vec!["sh".into(), "-c".into(), interp_to_string(script)] - } else if let Some(command) = command { - command.iter().map(interp_to_string).collect() - } else { - Vec::new() - }; - McpTransport::Stdio { - command: command_vec, - env: env - .iter() - .map(|(k, v)| (k.clone(), interp_to_string(v))) - .collect(), - } - } - McpEntryLayer::Http { url, headers, .. } => McpTransport::Http { - url: interp_to_string(url), - headers: headers - .iter() - .map(|(k, v)| (k.clone(), interp_to_string(v))) - .collect(), - }, - McpEntryLayer::Sandbox { - script, - command, - port, - env, - .. - } => { - let command_vec: Vec = if let Some(script) = script { - vec!["sh".into(), "-c".into(), interp_to_string(script)] - } else if let Some(command) = command { - command.iter().map(interp_to_string).collect() - } else { - Vec::new() - }; - McpTransport::Sandbox { - command: command_vec, - port: *port, - env: env - .iter() - .map(|(k, v)| (k.clone(), interp_to_string(v))) - .collect(), - } - } - }; - - let (startup_secs, tool_secs) = match entry { - McpEntryLayer::Http { - startup_timeout, - tool_timeout, - .. - } - | McpEntryLayer::Stdio { - startup_timeout, - tool_timeout, - .. - } - | McpEntryLayer::Sandbox { - startup_timeout, - tool_timeout, - .. - } => ( - startup_timeout.map_or(default_startup_timeout_secs(), |d| d.as_std().as_secs()), - tool_timeout.map_or(default_tool_timeout_secs(), |d| d.as_std().as_secs()), - ), - }; - - McpServerEntry { - transport, - startup_timeout_secs: startup_secs, - tool_timeout_secs: tool_secs, - } -} - -fn interp_to_string(value: &InterpString) -> String { - value.as_source() -} - -#[cfg(test)] -mod tests { - use super::*; - use std::collections::HashMap; - - #[test] - fn stdio_config_construction() { - let config = McpServerSettings { - name: "test-server".into(), - transport: McpTransport::Stdio { - command: vec![ - "npx".into(), - "-y".into(), - "@modelcontextprotocol/server-filesystem".into(), - ], - env: HashMap::new(), - }, - startup_timeout_secs: 10, - tool_timeout_secs: 60, - }; - assert_eq!(config.name, "test-server"); - assert_eq!(config.startup_timeout(), Duration::from_secs(10)); - assert_eq!(config.tool_timeout(), Duration::from_secs(60)); - } - - #[test] - fn http_config_construction() { - let config = McpServerSettings { - name: "remote-server".into(), - transport: McpTransport::Http { - url: "https://example.com/mcp".into(), - headers: HashMap::from([("Authorization".into(), "Bearer token".into())]), - }, - startup_timeout_secs: 30, - tool_timeout_secs: 60, - }; - assert_eq!(config.name, "remote-server"); - assert_eq!(config.startup_timeout(), Duration::from_secs(30)); - assert_eq!(config.tool_timeout(), Duration::from_secs(60)); - } - - #[test] - fn serde_round_trip_stdio() { - let config = McpServerSettings { - name: "fs".into(), - transport: McpTransport::Stdio { - command: vec!["node".into(), "server.js".into()], - env: HashMap::from([("NODE_ENV".into(), "production".into())]), - }, - startup_timeout_secs: 15, - tool_timeout_secs: 90, - }; - let json = serde_json::to_string(&config).unwrap(); - let deserialized: McpServerSettings = serde_json::from_str(&json).unwrap(); - assert_eq!(deserialized.name, "fs"); - assert_eq!(deserialized.startup_timeout_secs, 15); - assert_eq!(deserialized.tool_timeout_secs, 90); - assert!( - matches!(deserialized.transport, McpTransport::Stdio { command, .. } if command == vec!["node", "server.js"]) - ); - } - - #[test] - fn serde_round_trip_http() { - let config = McpServerSettings { - name: "remote".into(), - transport: McpTransport::Http { - url: "https://mcp.example.com".into(), - headers: HashMap::new(), - }, - startup_timeout_secs: 10, - tool_timeout_secs: 60, - }; - let json = serde_json::to_string(&config).unwrap(); - let deserialized: McpServerSettings = serde_json::from_str(&json).unwrap(); - assert_eq!(deserialized.name, "remote"); - assert!( - matches!(deserialized.transport, McpTransport::Http { url, .. } if url == "https://mcp.example.com") - ); - } - - #[test] - fn serde_defaults_applied() { - let json = r#"{"name":"minimal","transport":{"type":"stdio","command":["echo"]}}"#; - let config: McpServerSettings = serde_json::from_str(json).unwrap(); - assert_eq!(config.startup_timeout_secs, 10); - assert_eq!(config.tool_timeout_secs, 60); - } -} +pub use fabro_types::settings::run::{McpServerSettings, McpTransport}; diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index d2b6aea35..adb39ba48 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -13,7 +13,6 @@ 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::bridge_sandbox; use fabro_sandbox::daytona::DaytonaConfig; use fabro_sandbox::{DockerSandboxOptions, Sandbox, SandboxProvider, SandboxSpec}; use fabro_types::RunId; @@ -29,6 +28,7 @@ use fabro_validate::Severity; use fabro_workflow::error::FabroError; use fabro_workflow::operations::{CreateRunInput, ValidateInput, WorkflowInput, validate}; use fabro_workflow::pipeline::Validated; +use fabro_workflow::run_materialization::materialize_run; use fabro_workflow::workflow_bundle::{BundledWorkflow, WorkflowBundle}; use crate::server::AppState; @@ -340,14 +340,17 @@ async fn build_preflight_report( ) -> Result<(CheckReport, bool)> { let graph = validated.graph(); let settings = &prepared.settings; - let sandbox_provider = resolve_sandbox_provider(settings)?; + 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 sandbox_provider = resolve_sandbox_provider(&resolved_run)?; let github_app = state .github_app_credentials(settings.github_app_id_str().as_deref()) .await .map_err(|err| anyhow!(err))?; let mut checks = Vec::new(); - let setup_command_count = settings.run_prepare_commands().len(); + let setup_command_count = resolved_run.prepare.commands.len(); let repo_summary = prepared.git.as_ref().map_or_else( || "unknown".to_string(), |git| { @@ -390,9 +393,15 @@ async fn build_preflight_report( remediation: None, }); - let sandbox_ok = - run_sandbox_check(&mut checks, sandbox_provider, prepared, github_app.clone()).await; - let llm_ok = run_llm_check(state, &mut checks, graph, settings).await; + let sandbox_ok = run_sandbox_check( + &mut checks, + sandbox_provider, + prepared, + &resolved_run, + github_app.clone(), + ) + .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; let checks_ok = sandbox_ok && llm_ok; @@ -409,28 +418,34 @@ async fn build_preflight_report( )) } -fn resolve_sandbox_provider(settings: &SettingsFile) -> Result { - Ok(settings - .run_sandbox() - .and_then(|sb| sb.provider.as_deref()) +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_daytona_config(settings: &SettingsFile) -> Option { - let sandbox = settings.run_sandbox()?; - bridge_sandbox(sandbox).daytona +fn resolve_daytona_config( + settings: &fabro_types::settings::run::RunSettings, +) -> Option { + settings + .sandbox + .daytona + .as_ref() + .map(runtime_daytona_config) } async fn run_sandbox_check( checks: &mut Vec, sandbox_provider: SandboxProvider, prepared: &PreparedManifest, + resolved_run: &fabro_types::settings::run::RunSettings, github_app: Option, ) -> bool { - let daytona_config = resolve_daytona_config(&prepared.settings); + let daytona_config = resolve_daytona_config(resolved_run); let sandbox_result: Result, String> = match sandbox_provider { SandboxProvider::Local => SandboxSpec::Local { working_directory: prepared.working_directory.clone(), @@ -500,7 +515,7 @@ async fn run_llm_check( state: &AppState, checks: &mut Vec, graph: &Graph, - settings: &SettingsFile, + settings: &fabro_types::settings::run::RunSettings, ) -> bool { let (model, provider) = resolve_model_provider(settings, graph); let default_provider = provider.as_deref().unwrap_or("anthropic"); @@ -591,34 +606,21 @@ async fn run_llm_check( } } -fn resolve_model_provider(settings: &SettingsFile, graph: &Graph) -> (String, Option) { - let configured_model = settings.run_model_name_str(); - let configured_provider = settings.run_model_provider_str(); - - let provider = configured_provider.or_else(|| { - graph - .attrs - .get("default_provider") - .and_then(|value| value.as_str()) - .map(String::from) - }); - let model = configured_model - .or_else(|| { - graph - .attrs - .get("default_model") - .and_then(|value| value.as_str()) - .map(String::from) - }) - .unwrap_or_else(|| { - let catalog = Catalog::builtin(); - let info = provider - .as_deref() - .and_then(|value| value.parse::().ok()) - .and_then(|provider| catalog.default_for_provider(provider)) - .unwrap_or_else(|| catalog.default_from_env()); - info.id.clone() - }); +fn resolve_model_provider( + settings: &fabro_types::settings::run::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()); match Catalog::builtin().get(&model) { Some(info) => ( @@ -629,6 +631,52 @@ fn resolve_model_provider(settings: &SettingsFile, graph: &Graph) -> (String, Op } } +fn render_resolve_errors(errors: &[fabro_config::ResolveError]) -> String { + errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("; ") +} + +fn runtime_daytona_config(settings: &fabro_types::settings::run::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 { + name: snapshot.name.clone(), + cpu: snapshot.cpu, + memory: snapshot.memory_gb, + disk: snapshot.disk_gb, + dockerfile: snapshot + .dockerfile + .as_ref() + .map(|dockerfile| match dockerfile { + fabro_types::settings::run::DockerfileSource::Inline(text) => { + fabro_sandbox::config::DockerfileSource::Inline(text.clone()) + } + fabro_types::settings::run::DockerfileSource::Path { path } => { + fabro_sandbox::config::DockerfileSource::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()) + } + }), + skip_clone: settings.skip_clone, + } +} + async fn run_github_token_check( checks: &mut Vec, prepared: &PreparedManifest, diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 58b020839..930c19206 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -594,7 +594,9 @@ impl AppState { } pub(crate) fn dry_run(&self) -> bool { - self.settings.read().unwrap().dry_run_enabled() + fabro_config::resolve_run_from_file(&self.settings.read().unwrap()) + .map(|settings| settings.execution.mode == fabro_types::settings::run::RunMode::DryRun) + .unwrap_or(false) } pub(crate) async fn build_llm_client(&self) -> Result { @@ -1465,10 +1467,9 @@ fn build_prune_plan( } fn system_sandbox_provider(settings: &SettingsFile) -> String { - settings - .run_sandbox() - .and_then(|sb| sb.provider.clone()) - .unwrap_or_else(|| SandboxProvider::default().to_string()) + fabro_config::resolve_run_from_file(settings) + .map(|settings| settings.sandbox.provider) + .unwrap_or_else(|_| SandboxProvider::default().to_string()) } fn parse_system_duration(raw: &str) -> anyhow::Result { diff --git a/lib/crates/fabro-server/src/web_auth.rs b/lib/crates/fabro-server/src/web_auth.rs index d522de14e..7dc6a8f01 100644 --- a/lib/crates/fabro-server/src/web_auth.rs +++ b/lib/crates/fabro-server/src/web_auth.rs @@ -149,11 +149,8 @@ fn json_response(status: StatusCode, body: serde_json::Value) -> Response { fn features_json(settings: &SettingsFile) -> serde_json::Value { let features = settings.features.as_ref(); let session_sandboxes = features.and_then(|f| f.session_sandboxes).unwrap_or(false); - // Retros in v2 live under `run.execution.retros` (positive form) rather - // than the top-level features stanza. - let retros = settings - .run_execution() - .and_then(|e| e.retros) + let retros = fabro_config::resolve_run_from_file(settings) + .map(|settings| settings.execution.retros) .unwrap_or(false); json!({ "session_sandboxes": session_sandboxes, diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index ac4d584ab..fa1786637 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -32,7 +32,14 @@ pub use model_ref::{ AmbiguousModelRef, ModelRef, ModelRegistry, ParseModelRefError, ResolvedModelRef, }; pub use project::ProjectLayer; -pub use run::RunLayer; +pub use run::{ + ArtifactsSettings, DaytonaSettings, DaytonaSnapshotSettings, DockerfileSource, + GitAuthorSettings, HookDefinition, HookType, InterviewProviderSettings, McpServerSettings, + McpTransport, NotificationProviderSettings, NotificationRouteSettings, PullRequestSettings, + RunAgentSettings, RunCheckpointSettings, RunExecutionSettings, RunGitSettings, RunGoal, + RunInterviewsSettings, RunLayer, RunModelSettings, RunPrepareSettings, RunSandboxSettings, + RunScmSettings, RunSettings, ScmGitHubSettings, TlsMode, +}; pub use server::{ DiscordIntegrationSettings, GithubIntegrationSettings, GithubOauthSettings, IntegrationWebhooksSettings, ObjectStoreSettings, ServerApiSettings, ServerArtifactsSettings, diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index 04246fb30..c2f072b36 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -7,6 +7,7 @@ //! behavior, and artifact collection. use std::collections::HashMap; +use std::time::Duration as StdDuration; use serde::{Deserialize, Serialize}; @@ -14,6 +15,398 @@ use super::duration::Duration; use super::interp::InterpString; use super::model_ref::ModelRef; +/// A structurally resolved `[run]` view for consumers. +#[derive(Debug, Clone, PartialEq)] +pub struct RunSettings { + pub goal: Option, + pub working_dir: Option, + pub metadata: HashMap, + pub inputs: HashMap, + pub model: RunModelSettings, + pub git: RunGitSettings, + pub prepare: RunPrepareSettings, + pub execution: RunExecutionSettings, + pub checkpoint: RunCheckpointSettings, + pub sandbox: RunSandboxSettings, + pub notifications: HashMap, + pub interviews: RunInterviewsSettings, + pub agent: RunAgentSettings, + pub hooks: Vec, + pub scm: RunScmSettings, + pub pull_request: Option, + 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 { + Inline(InterpString), + File(InterpString), +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct RunModelSettings { + pub provider: Option, + pub name: Option, + pub fallbacks: Vec, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct RunGitSettings { + pub author: Option, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct GitAuthorSettings { + pub name: Option, + pub email: Option, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct RunPrepareSettings { + pub commands: Vec, + pub timeout_ms: u64, +} + +impl Default for RunPrepareSettings { + fn default() -> Self { + Self { + commands: Vec::new(), + timeout_ms: 300_000, + } + } +} + +#[derive(Debug, Clone, PartialEq)] +pub struct RunExecutionSettings { + pub mode: RunMode, + pub approval: ApprovalMode, + pub retros: bool, +} + +impl Default for RunExecutionSettings { + fn default() -> Self { + Self { + mode: RunMode::Normal, + approval: ApprovalMode::Prompt, + retros: true, + } + } +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct RunCheckpointSettings { + pub exclude_globs: Vec, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct RunSandboxSettings { + pub provider: String, + pub preserve: bool, + pub devcontainer: bool, + pub env: HashMap, + pub local: LocalSandboxSettings, + pub daytona: Option, +} + +impl Default for RunSandboxSettings { + fn default() -> Self { + Self { + provider: "local".to_string(), + preserve: false, + devcontainer: false, + env: HashMap::new(), + local: LocalSandboxSettings::default(), + daytona: None, + } + } +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct LocalSandboxSettings { + pub worktree_mode: WorktreeMode, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct DaytonaSettings { + pub auto_stop_interval: Option, + pub labels: HashMap, + pub snapshot: Option, + pub network: Option, + pub skip_clone: bool, +} + +#[derive(Debug, Clone, PartialEq)] +pub enum DockerfileSource { + Inline(String), + Path { path: String }, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct DaytonaSnapshotSettings { + pub name: String, + pub cpu: Option, + pub memory_gb: Option, + pub disk_gb: Option, + pub dockerfile: Option, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct NotificationRouteSettings { + pub enabled: bool, + pub provider: Option, + pub events: Vec, + pub slack: Option, + pub discord: Option, + pub teams: Option, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct NotificationProviderSettings { + pub channel: Option, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct RunInterviewsSettings { + pub provider: Option, + pub slack: Option, + pub discord: Option, + pub teams: Option, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct InterviewProviderSettings { + pub channel: Option, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct RunAgentSettings { + pub permissions: Option, + pub mcps: HashMap, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct McpServerSettings { + pub name: String, + pub transport: McpTransport, + pub startup_timeout_secs: u64, + pub tool_timeout_secs: u64, +} + +impl Default for McpServerSettings { + fn default() -> Self { + Self { + name: String::new(), + transport: McpTransport::Stdio { + command: Vec::new(), + env: HashMap::new(), + }, + startup_timeout_secs: 10, + tool_timeout_secs: 60, + } + } +} + +impl McpServerSettings { + #[must_use] + pub fn startup_timeout(&self) -> StdDuration { + StdDuration::from_secs(self.startup_timeout_secs) + } + + #[must_use] + pub fn tool_timeout(&self) -> StdDuration { + StdDuration::from_secs(self.tool_timeout_secs) + } +} + +#[derive(Debug, Clone, PartialEq)] +pub enum McpTransport { + Stdio { + command: Vec, + env: HashMap, + }, + Http { + url: String, + headers: HashMap, + }, + Sandbox { + command: Vec, + port: u16, + env: HashMap, + }, +} + +#[derive(Debug, Clone, Copy, Deserialize, PartialEq, Eq, Default, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum TlsMode { + #[default] + Verify, + NoVerify, + Off, +} + +#[derive(Debug, Clone, Deserialize, PartialEq, Serialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum HookType { + Command { + command: String, + }, + Http { + url: String, + headers: Option>, + #[serde(default)] + allowed_env_vars: Vec, + #[serde(default)] + tls: TlsMode, + }, + Prompt { + prompt: String, + model: Option, + }, + Agent { + prompt: String, + model: Option, + max_tool_rounds: Option, + }, +} + +#[derive(Debug, Clone, Deserialize, PartialEq, Serialize)] +pub struct HookDefinition { + pub name: Option, + pub event: HookEvent, + #[serde(default)] + pub command: Option, + #[serde(flatten)] + pub hook_type: Option, + pub matcher: Option, + pub blocking: Option, + pub timeout_ms: Option, + pub sandbox: Option, +} + +impl HookDefinition { + pub fn resolved_hook_type(&self) -> Option> { + if let Some(ref hook_type) = self.hook_type { + return Some(std::borrow::Cow::Borrowed(hook_type)); + } + self.command.as_ref().map(|command| { + std::borrow::Cow::Owned(HookType::Command { + command: command.clone(), + }) + }) + } + + #[must_use] + pub fn is_blocking(&self) -> bool { + self.blocking.unwrap_or_else(|| { + matches!( + self.event, + HookEvent::RunStart + | HookEvent::StageStart + | HookEvent::EdgeSelected + | HookEvent::PreToolUse + | HookEvent::SandboxReady + ) + }) + } + + #[must_use] + pub fn timeout(&self) -> StdDuration { + if let Some(ms) = self.timeout_ms { + return StdDuration::from_millis(ms); + } + let default_ms = match self.resolved_hook_type().as_deref() { + Some(HookType::Prompt { .. }) => 30_000, + _ => 60_000, + }; + StdDuration::from_millis(default_ms) + } + + #[must_use] + pub fn runs_in_sandbox(&self) -> bool { + self.sandbox.unwrap_or(true) + } + + #[must_use] + pub fn effective_name(&self) -> String { + if let Some(ref name) = self.name { + return name.clone(); + } + let event = format!("{:?}", self.event).to_lowercase(); + match self.resolved_hook_type().as_deref() { + Some(HookType::Command { command }) => { + let short = &command[..command.floor_char_boundary(20)]; + format!("{event}:{short}") + } + Some(HookType::Http { url, .. }) => format!("{event}:{url}"), + Some(HookType::Prompt { prompt, .. } | HookType::Agent { prompt, .. }) => { + let short = &prompt[..prompt.floor_char_boundary(20)]; + format!("{event}:{short}") + } + None => event, + } + } +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct RunScmSettings { + pub provider: Option, + pub owner: Option, + pub repository: Option, + pub github: Option, +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct ScmGitHubSettings; + +#[derive(Debug, Clone, PartialEq)] +pub struct PullRequestSettings { + pub enabled: bool, + pub draft: bool, + pub auto_merge: bool, + pub merge_strategy: MergeStrategy, +} + +impl Default for PullRequestSettings { + fn default() -> Self { + Self { + enabled: false, + draft: true, + auto_merge: false, + merge_strategy: MergeStrategy::Squash, + } + } +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct ArtifactsSettings { + pub include: Vec, +} + /// A sparse `[run]` layer as it appears in a single settings file. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] diff --git a/lib/crates/fabro-workflow/src/config.rs b/lib/crates/fabro-workflow/src/config.rs deleted file mode 100644 index c11478d16..000000000 --- a/lib/crates/fabro-workflow/src/config.rs +++ /dev/null @@ -1,71 +0,0 @@ -//! Workflow runtime configuration shapes. -//! -//! Runtime-side types consumed by the pipeline. The v2 parse tree lives in -//! `fabro_types::settings::run::{RunPullRequestLayer, MergeStrategy, -//! RunArtifactsLayer}`. Conversion from v2 lives in [`bridge_pull_request`] -//! / [`bridge_run_artifacts`]. - -use fabro_types::settings::run::{ - MergeStrategy as V2MergeStrategy, RunArtifactsLayer, RunPullRequestLayer, -}; -use serde::{Deserialize, Serialize}; - -fn default_true() -> bool { - true -} - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct PullRequestSettings { - #[serde(default)] - pub enabled: bool, - #[serde(default = "default_true")] - pub draft: bool, - #[serde(default)] - pub auto_merge: bool, - #[serde(default)] - pub merge_strategy: MergeStrategy, -} - -#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize)] -#[serde(rename_all = "lowercase")] -pub enum MergeStrategy { - #[default] - Squash, - Merge, - Rebase, -} - -#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] -pub struct ArtifactsSettings { - #[serde(default)] - pub include: Vec, -} - -#[must_use] -pub fn bridge_merge_strategy(m: V2MergeStrategy) -> MergeStrategy { - match m { - V2MergeStrategy::Squash => MergeStrategy::Squash, - V2MergeStrategy::Merge => MergeStrategy::Merge, - V2MergeStrategy::Rebase => MergeStrategy::Rebase, - } -} - -#[must_use] -pub fn bridge_pull_request(pr: &RunPullRequestLayer) -> PullRequestSettings { - PullRequestSettings { - enabled: pr.enabled.unwrap_or(false), - draft: pr.draft.unwrap_or(true), - auto_merge: pr.auto_merge.unwrap_or(false), - merge_strategy: pr - .merge_strategy - .map(bridge_merge_strategy) - .unwrap_or_default(), - } -} - -#[must_use] -pub fn bridge_run_artifacts(artifacts: &RunArtifactsLayer) -> ArtifactsSettings { - ArtifactsSettings { - include: artifacts.include.clone(), - } -} diff --git a/lib/crates/fabro-workflow/src/git.rs b/lib/crates/fabro-workflow/src/git.rs index 153a38228..02f746c1c 100644 --- a/lib/crates/fabro-workflow/src/git.rs +++ b/lib/crates/fabro-workflow/src/git.rs @@ -16,9 +16,10 @@ pub use fabro_checkpoint::metadata::MetadataStore; pub const RUN_BRANCH_PREFIX: &str = "fabro/run/"; pub fn git_author_from_settings(settings: &SettingsFile) -> GitAuthor { - settings - .run_git_author() - .map(GitAuthor::from) + fabro_config::resolve_run_from_file(settings) + .ok() + .and_then(|settings| settings.git.author) + .map(|author| GitAuthor::from(&author)) .unwrap_or_default() } diff --git a/lib/crates/fabro-workflow/src/lib.rs b/lib/crates/fabro-workflow/src/lib.rs index 270ce6d9d..edabf6af2 100644 --- a/lib/crates/fabro-workflow/src/lib.rs +++ b/lib/crates/fabro-workflow/src/lib.rs @@ -116,7 +116,6 @@ pub mod artifact; pub mod artifact_snapshot; pub mod artifact_upload; pub(crate) mod condition; -pub mod config; pub mod context; pub mod devcontainer_bridge; pub mod error; @@ -139,6 +138,7 @@ pub mod run_control; pub(crate) mod run_dir; pub mod run_dump; pub mod run_lookup; +pub mod run_materialization; pub mod run_options; pub mod run_status; pub mod runtime_store; diff --git a/lib/crates/fabro-workflow/src/lifecycle/git.rs b/lib/crates/fabro-workflow/src/lifecycle/git.rs index f2d38ee6d..eb9af86f9 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/git.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/git.rs @@ -192,7 +192,7 @@ impl RunLifecycle for GitLifecycle { &result.outcome.status.to_string(), completed_count, shadow_sha, - self.run_options.checkpoint_exclude_globs(), + &self.run_options.checkpoint_exclude_globs(), &git_author, ) .await; diff --git a/lib/crates/fabro-workflow/src/lifecycle/mod.rs b/lib/crates/fabro-workflow/src/lifecycle/mod.rs index 7e0106e79..df6c1d052 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/mod.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/mod.rs @@ -165,7 +165,7 @@ impl WorkflowLifecycle { run_store.clone(), Arc::clone(emitter), run_options.run_id, - run_options.artifact_globs().to_vec(), + run_options.artifact_globs(), artifact_sink, captured_artifact_count, ); diff --git a/lib/crates/fabro-workflow/src/operations/create.rs b/lib/crates/fabro-workflow/src/operations/create.rs index 00fc3a266..1eb5b2c78 100644 --- a/lib/crates/fabro-workflow/src/operations/create.rs +++ b/lib/crates/fabro-workflow/src/operations/create.rs @@ -1,10 +1,9 @@ use fabro_config::Storage; use fabro_graphviz::graph::{AttrValue, Graph}; -use fabro_model::{Catalog, Provider}; +use fabro_model::Catalog; use fabro_sandbox::SandboxProvider; use fabro_store::Database; -use fabro_types::settings::run::{RunGoalLayer, RunLayer, RunModelLayer}; -use fabro_types::settings::{InterpString, SettingsFile}; +use fabro_types::settings::SettingsFile; use fabro_types::{RunId, RunProvenance}; use std::collections::BTreeMap; use std::collections::HashMap; @@ -17,6 +16,7 @@ use crate::pipeline::types::PersistOptions; use crate::pipeline::{self, Persisted, TransformOptions, Validated}; use crate::records::RunRecord; use crate::run_lookup::default_scratch_base; +use crate::run_materialization::materialize_run; use crate::transforms::{Transform, expand_vars}; use crate::workflow_bundle::{RunDefinition, WorkflowBundle}; use fabro_sandbox::daytona::detect_repo_info; @@ -71,7 +71,10 @@ pub async fn create(store: &Database, request: CreateRunInput) -> Result FabroError { } fn validate_sandbox_provider(settings: &SettingsFile) -> Result<(), FabroError> { - if let Some(provider) = settings - .run_sandbox() - .and_then(|sandbox| sandbox.provider.as_deref()) - { - provider - .parse::() - .map_err(|err| FabroError::Precondition(format!("Invalid sandbox provider: {err}")))?; - } + let resolved = fabro_config::resolve_run_from_file(settings).map_err(|errors| { + FabroError::Precondition( + errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("; "), + ) + })?; + resolved + .sandbox + .provider + .parse::() + .map_err(|err| FabroError::Precondition(format!("Invalid sandbox provider: {err}")))?; Ok(()) } @@ -344,7 +353,7 @@ fn persist_validated( provenance, } = options; - let settings = resolve_run_settings(settings, validated.graph()); + 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)); @@ -373,65 +382,6 @@ fn persist_validated( ) } -pub(crate) fn resolve_run_settings(mut settings: SettingsFile, graph: &Graph) -> SettingsFile { - let configured_model = settings.run_model_name_str(); - let configured_provider = settings.run_model_provider_str(); - let graph_provider = graph - .attrs - .get("default_provider") - .and_then(|v| v.as_str()) - .map(str::to_string); - let graph_model = graph - .attrs - .get("default_model") - .and_then(|v| v.as_str()) - .map(str::to_string); - - let provider = configured_provider.or(graph_provider); - - let model = configured_model.or(graph_model).unwrap_or_else(|| { - let catalog = Catalog::builtin(); - provider - .as_deref() - .and_then(|value| value.parse::().ok()) - .and_then(|provider| catalog.default_for_provider(provider)) - .unwrap_or_else(|| catalog.default_from_env()) - .id - .clone() - }); - - let (resolved_model, resolved_provider) = match Catalog::builtin().get(&model) { - Some(info) => ( - info.id.clone(), - provider.or(Some(info.provider.to_string())), - ), - None => (model, provider), - }; - - let run = settings.run.get_or_insert_with(RunLayer::default); - let model_layer = run.model.get_or_insert_with(RunModelLayer::default); - model_layer.name = Some(InterpString::parse(&resolved_model)); - model_layer.provider = resolved_provider.as_deref().map(InterpString::parse); - - let goal = graph.goal().to_string(); - run.goal = if goal.is_empty() { - None - } else { - Some(RunGoalLayer::Inline(InterpString::parse(&goal))) - }; - // Strip disabled pull_request entries so downstream consumers can - // treat `Some(_)` as "PR creation is on". - if run - .pull_request - .as_ref() - .is_some_and(|pr| !pr.enabled.unwrap_or(false)) - { - run.pull_request = None; - } - - settings -} - pub(crate) fn default_run_dir(run_id: &RunId) -> PathBuf { make_run_dir(&default_scratch_base(), run_id) } @@ -449,6 +399,7 @@ mod tests { use fabro_graphviz::graph::AttrValue; use fabro_store::Database; use fabro_types::fixtures; + use fabro_types::settings::InterpString; use object_store::local::LocalFileSystem; use object_store::memory::InMemory; use std::sync::Arc; diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index 9fefa5ff0..44f61eac1 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -5,19 +5,19 @@ use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use fabro_config::project as project_config; -use fabro_hooks::config::bridge_hook; use fabro_interview::{AutoApproveInterviewer, Interviewer}; -use fabro_mcp::config::bridge_mcp_entry; use fabro_model::{Catalog, FallbackTarget, Provider}; -use fabro_sandbox::config::{ - self as sandbox_config, WorktreeMode, bridge_sandbox, bridge_worktree_mode, -}; +use fabro_sandbox::config::{self as sandbox_config, WorktreeMode, bridge_worktree_mode}; use fabro_sandbox::{SandboxProvider, SandboxSpec}; use fabro_types::RunId; -use fabro_types::settings::run::ModelRefOrSplice; -use fabro_types::settings::{InterpString, SettingsFile}; - -use crate::config::{PullRequestSettings, bridge_pull_request}; +use fabro_types::settings::InterpString; +use fabro_types::settings::run::{ + DockerfileSource as ResolvedDockerfileSource, HookDefinition as ResolvedHookDefinition, + HookEvent as ResolvedHookEvent, HookType as ResolvedHookType, + McpServerSettings as ResolvedMcpServerSettings, McpTransport as ResolvedMcpTransport, + PullRequestSettings, RunModelSettings as ResolvedRunModelSettings, + RunSettings as ResolvedRunSettings, TlsMode as ResolvedTlsMode, +}; use crate::artifact_upload::ArtifactSink; use crate::context::Context; @@ -298,17 +298,29 @@ impl RunSession { .map(|(url, branch)| (Some(url), branch)) .unwrap_or((None, None)); - let sandbox_provider = resolve_sandbox_provider(settings)?; - let sandbox_provider = if settings.dry_run_enabled() && !sandbox_provider.is_local() { + let resolved = fabro_config::resolve_run_from_file(settings) + .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 = settings - .run_model_name_str() + let model = resolved + .model + .name + .as_ref() + .map(InterpString::as_source) .unwrap_or_else(|| Catalog::builtin().default_from_env().id.clone()); - let provider = settings - .run_model_provider_str() + let provider = resolved + .model + .provider + .as_ref() + .map(InterpString::as_source) .filter(|value| !value.is_empty()); let provider_enum: Provider = provider @@ -318,15 +330,13 @@ impl RunSession { .map_err(|err| FabroError::Precondition(err.clone()))? .unwrap_or_else(Provider::default_from_env); - let fallback_chain = resolve_fallback_chain(provider_enum, &model, settings); - let mcp_servers = settings - .run_agent_mcps() - .map(|mcps| { - mcps.iter() - .map(|(name, entry)| bridge_mcp_entry(entry).into_config(name.clone())) - .collect() - }) - .unwrap_or_default(); + let fallback_chain = resolve_fallback_chain(provider_enum, &model, &resolved.model); + let mcp_servers = resolved + .agent + .mcps + .values() + .map(runtime_mcp_server) + .collect(); let sandbox = match sandbox_provider { SandboxProvider::Local => SandboxSpec::Local { @@ -339,22 +349,19 @@ impl RunSession { }, }, SandboxProvider::Daytona => SandboxSpec::Daytona { - config: resolve_daytona_config(settings).unwrap_or_default(), + config: resolve_daytona_config(&resolved).unwrap_or_default(), github_app: services.github_app.clone(), run_id: Some(record.run_id), clone_branch: detected_base_branch.or_else(|| record.base_branch.clone()), }, }; - let toml_env: HashMap = settings - .run_sandbox() - .map(|sb| { - sb.env - .iter() - .map(|(k, v)| (k.clone(), resolve_interp(v))) - .collect() - }) - .unwrap_or_default(); + let toml_env: HashMap = resolved + .sandbox + .env + .iter() + .map(|(k, v)| (k.clone(), resolve_interp(v))) + .collect(); let github_permissions: Option> = settings.github_permissions().map(|perms| { perms @@ -369,22 +376,19 @@ impl RunSession { origin_url: origin_url.clone(), }; - let devcontainer = settings - .run_sandbox() - .and_then(|sb| sb.devcontainer) - .unwrap_or(false) - .then(|| DevcontainerSpec { - enabled: true, - resolve_dir: working_directory.clone(), - }); + let devcontainer = resolved.sandbox.devcontainer.then(|| DevcontainerSpec { + enabled: true, + resolve_dir: working_directory.clone(), + }); - let interviewer: Arc = if settings.auto_approve_enabled() { - Arc::new(AutoApproveInterviewer) - } else { - services.interviewer - }; + let interviewer: Arc = + if resolved.execution.approval == fabro_types::settings::run::ApprovalMode::Auto { + Arc::new(AutoApproveInterviewer) + } else { + services.interviewer + }; - let pr_config = settings.run_pull_request().map(bridge_pull_request); + let pr_config = resolved.pull_request.clone(); Ok(Self { cancel_token: services.cancel_token, @@ -397,17 +401,17 @@ impl RunSession { provider: provider_enum, fallback_chain, mcp_servers, - dry_run: settings.dry_run_enabled(), + dry_run: resolved.execution.mode == fabro_types::settings::run::RunMode::DryRun, }, interviewer, on_node: services.on_node, lifecycle: LifecycleOptions { - setup_commands: settings.run_prepare_commands(), - setup_command_timeout_ms: settings.run_prepare_timeout_ms().unwrap_or(300_000), + setup_commands: resolved.prepare.commands.clone(), + setup_command_timeout_ms: resolved.prepare.timeout_ms, devcontainer_phases: Vec::new(), }, hooks: fabro_hooks::HookSettings { - hooks: settings.run_hooks().iter().map(bridge_hook).collect(), + hooks: resolved.hooks.iter().map(runtime_hook_definition).collect(), }, sandbox_env, devcontainer, @@ -416,10 +420,10 @@ impl RunSession { artifact_sink: services.artifact_sink, git, github_app: services.github_app.clone(), - worktree_mode: Some(resolve_worktree_mode(settings)), + worktree_mode: Some(resolve_worktree_mode(&resolved)), registry_override: services.registry_override, - retro_enabled: !settings.no_retro_enabled() && project_config::is_retro_enabled(), - preserve_sandbox: resolve_preserve_sandbox(settings), + retro_enabled: resolved.execution.retros && project_config::is_retro_enabled(), + preserve_sandbox: resolved.sandbox.preserve, pr_config, pr_github_app: services.github_app, pr_origin_url: origin_url, @@ -452,43 +456,32 @@ async fn load_accepted_run_definition( serde_json::from_slice(&bytes).map_err(|err| FabroError::Parse(err.to_string())) } -fn resolve_sandbox_provider(settings: &SettingsFile) -> Result { - settings - .run_sandbox() - .and_then(|sb| sb.provider.as_deref()) +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) } -fn resolve_preserve_sandbox(settings: &SettingsFile) -> bool { - settings.preserve_sandbox_enabled() +fn resolve_worktree_mode(settings: &ResolvedRunSettings) -> sandbox_config::WorktreeMode { + bridge_worktree_mode(settings.sandbox.local.worktree_mode) } -fn resolve_worktree_mode(settings: &SettingsFile) -> sandbox_config::WorktreeMode { +fn resolve_daytona_config(settings: &ResolvedRunSettings) -> Option { settings - .run_sandbox() - .and_then(|sb| sb.local.as_ref()) - .and_then(|local| local.worktree_mode) - .map(bridge_worktree_mode) - .unwrap_or_default() -} - -fn resolve_daytona_config(settings: &SettingsFile) -> Option { - let sandbox = settings.run_sandbox()?; - bridge_sandbox(sandbox).daytona + .sandbox + .daytona + .as_ref() + .map(runtime_daytona_config) } fn resolve_fallback_chain( provider: Provider, model: &str, - settings: &SettingsFile, + settings: &ResolvedRunModelSettings, ) -> Vec { - let Some(model_layer) = settings.run_model() else { - return Vec::new(); - }; - if model_layer.fallbacks.is_empty() { + if settings.fallbacks.is_empty() { return Vec::new(); } // Group v2 ModelRef entries by provider name, preserving the legacy @@ -499,17 +492,156 @@ fn resolve_fallback_chain( // provider-keyed fallbacks. A proper provider-aware fallback chain // is a follow-up along with the model registry work. let mut by_provider: HashMap> = HashMap::new(); - for entry in &model_layer.fallbacks { - if let ModelRefOrSplice::ModelRef(model_ref) = entry { - by_provider - .entry(String::new()) - .or_default() - .push(model_ref.to_string()); - } + for model_ref in &settings.fallbacks { + by_provider + .entry(String::new()) + .or_default() + .push(model_ref.to_string()); } Catalog::builtin().build_fallback_chain(provider, model, &by_provider) } +fn render_resolve_errors(errors: &[fabro_config::ResolveError]) -> String { + errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("; ") +} + +fn runtime_mcp_server( + settings: &ResolvedMcpServerSettings, +) -> fabro_mcp::config::McpServerSettings { + fabro_mcp::config::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 { + url: url.clone(), + headers: headers.clone(), + }, + ResolvedMcpTransport::Sandbox { command, port, env } => { + fabro_mcp::config::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 { + 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 { + name: snapshot.name.clone(), + cpu: snapshot.cpu, + memory: snapshot.memory_gb, + disk: snapshot.disk_gb, + dockerfile: snapshot + .dockerfile + .as_ref() + .map(|dockerfile| match dockerfile { + ResolvedDockerfileSource::Inline(text) => { + fabro_sandbox::config::DockerfileSource::Inline(text.clone()) + } + ResolvedDockerfileSource::Path { path } => { + fabro_sandbox::config::DockerfileSource::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()) + } + }), + skip_clone: settings.skip_clone, + } +} + +fn runtime_hook_definition(definition: &ResolvedHookDefinition) -> fabro_hooks::HookDefinition { + fabro_hooks::HookDefinition { + name: definition.name.clone(), + event: match definition.event { + ResolvedHookEvent::RunStart => fabro_hooks::HookEvent::RunStart, + ResolvedHookEvent::RunComplete => fabro_hooks::HookEvent::RunComplete, + ResolvedHookEvent::RunFailed => fabro_hooks::HookEvent::RunFailed, + ResolvedHookEvent::StageStart => fabro_hooks::HookEvent::StageStart, + ResolvedHookEvent::StageComplete => fabro_hooks::HookEvent::StageComplete, + ResolvedHookEvent::StageFailed => fabro_hooks::HookEvent::StageFailed, + ResolvedHookEvent::StageRetrying => fabro_hooks::HookEvent::StageRetrying, + ResolvedHookEvent::EdgeSelected => fabro_hooks::HookEvent::EdgeSelected, + ResolvedHookEvent::ParallelStart => fabro_hooks::HookEvent::ParallelStart, + ResolvedHookEvent::ParallelComplete => fabro_hooks::HookEvent::ParallelComplete, + ResolvedHookEvent::SandboxReady => fabro_hooks::HookEvent::SandboxReady, + ResolvedHookEvent::SandboxCleanup => fabro_hooks::HookEvent::SandboxCleanup, + ResolvedHookEvent::CheckpointSaved => fabro_hooks::HookEvent::CheckpointSaved, + ResolvedHookEvent::PreToolUse => fabro_hooks::HookEvent::PreToolUse, + ResolvedHookEvent::PostToolUse => fabro_hooks::HookEvent::PostToolUse, + ResolvedHookEvent::PostToolUseFailure => fabro_hooks::HookEvent::PostToolUseFailure, + }, + command: definition.command.clone(), + hook_type: definition.hook_type.as_ref().map(runtime_hook_type), + matcher: definition.matcher.clone(), + blocking: definition.blocking, + timeout_ms: definition.timeout_ms, + sandbox: definition.sandbox, + } +} + +fn runtime_hook_type(hook_type: &ResolvedHookType) -> fabro_hooks::HookType { + match hook_type { + ResolvedHookType::Command { command } => fabro_hooks::HookType::Command { + command: command.clone(), + }, + ResolvedHookType::Http { + url, + headers, + allowed_env_vars, + tls, + } => fabro_hooks::HookType::Http { + url: url.clone(), + headers: headers.clone(), + allowed_env_vars: allowed_env_vars.clone(), + tls: match tls { + ResolvedTlsMode::Verify => fabro_hooks::TlsMode::Verify, + ResolvedTlsMode::NoVerify => fabro_hooks::TlsMode::NoVerify, + ResolvedTlsMode::Off => fabro_hooks::TlsMode::Off, + }, + }, + ResolvedHookType::Prompt { prompt, model } => fabro_hooks::HookType::Prompt { + prompt: prompt.clone(), + model: model.clone(), + }, + ResolvedHookType::Agent { + prompt, + model, + max_tool_rounds, + } => fabro_hooks::HookType::Agent { + prompt: prompt.clone(), + model: model.clone(), + max_tool_rounds: *max_tool_rounds, + }, + } +} + impl RunSession { /// Shared engine: initialize, execute, retro, finalize, pull_request. async fn run( @@ -852,6 +984,7 @@ mod tests { use chrono::Utc; use fabro_store::Database; use fabro_types::fixtures; + use fabro_types::settings::SettingsFile; use fabro_types::settings::run::{RunExecutionLayer, RunLayer, RunMode}; use object_store::memory::InMemory; diff --git a/lib/crates/fabro-workflow/src/pipeline/execute.rs b/lib/crates/fabro-workflow/src/pipeline/execute.rs index 902228342..d375a829c 100644 --- a/lib/crates/fabro-workflow/src/pipeline/execute.rs +++ b/lib/crates/fabro-workflow/src/pipeline/execute.rs @@ -74,7 +74,7 @@ pub async fn execute(init: Initialized) -> Executed { base_sha, run_branch: git.run_branch.clone(), meta_branch: git.meta_branch.clone(), - checkpoint_exclude_globs: run_options.checkpoint_exclude_globs().to_vec(), + checkpoint_exclude_globs: run_options.checkpoint_exclude_globs(), git_author: run_options.git_author(), })) }); diff --git a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs index d00093cb3..8bb601aac 100644 --- a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs @@ -1,6 +1,6 @@ -use crate::config::MergeStrategy; use fabro_store::RunProjection; use fabro_types::PullRequestRecord; +use fabro_types::settings::run::MergeStrategy; use tracing::{debug, info}; use fabro_github::{self as github_app, GitHubAppCredentials, ssh_url_to_https}; diff --git a/lib/crates/fabro-workflow/src/pipeline/types.rs b/lib/crates/fabro-workflow/src/pipeline/types.rs index 653a9fc12..61cd9396e 100644 --- a/lib/crates/fabro-workflow/src/pipeline/types.rs +++ b/lib/crates/fabro-workflow/src/pipeline/types.rs @@ -12,10 +12,10 @@ use fabro_model::FallbackTarget; use fabro_sandbox::SandboxSpec; use fabro_sandbox::config::WorktreeMode; use fabro_types::RunId; +use fabro_types::settings::run::PullRequestSettings; use fabro_validate::Diagnostic; use crate::artifact_upload::ArtifactSink; -use crate::config::PullRequestSettings; use crate::context::Context; use crate::error::FabroError; use crate::event::Emitter; diff --git a/lib/crates/fabro-workflow/src/run_materialization.rs b/lib/crates/fabro-workflow/src/run_materialization.rs new file mode 100644 index 000000000..765a18776 --- /dev/null +++ b/lib/crates/fabro-workflow/src/run_materialization.rs @@ -0,0 +1,70 @@ +use fabro_graphviz::graph::Graph; +use fabro_model::{Catalog, Provider}; +use fabro_types::settings::run::{RunGoalLayer, RunLayer, RunModelLayer}; +use fabro_types::settings::{InterpString, SettingsFile}; + +pub fn materialize_run(mut layer: SettingsFile, graph: &Graph, catalog: &Catalog) -> SettingsFile { + let configured_model = layer + .run + .as_ref() + .and_then(|run| run.model.as_ref()) + .and_then(|model| model.name.as_ref()) + .map(InterpString::as_source); + let configured_provider = layer + .run + .as_ref() + .and_then(|run| run.model.as_ref()) + .and_then(|model| model.provider.as_ref()) + .map(InterpString::as_source); + let graph_provider = graph + .attrs + .get("default_provider") + .and_then(|value| value.as_str()) + .map(str::to_string); + let graph_model = graph + .attrs + .get("default_model") + .and_then(|value| value.as_str()) + .map(str::to_string); + + let provider = configured_provider.or(graph_provider); + let model = configured_model.or(graph_model).unwrap_or_else(|| { + provider + .as_deref() + .and_then(|value| value.parse::().ok()) + .and_then(|provider| catalog.default_for_provider(provider)) + .unwrap_or_else(|| catalog.default_from_env()) + .id + .clone() + }); + + let (resolved_model, resolved_provider) = match catalog.get(&model) { + Some(info) => ( + info.id.clone(), + provider.or(Some(info.provider.to_string())), + ), + None => (model, provider), + }; + + let run = layer.run.get_or_insert_with(RunLayer::default); + let model_layer = run.model.get_or_insert_with(RunModelLayer::default); + model_layer.name = Some(InterpString::parse(&resolved_model)); + model_layer.provider = resolved_provider.as_deref().map(InterpString::parse); + + let goal = graph.goal().to_string(); + run.goal = if goal.is_empty() { + None + } else { + Some(RunGoalLayer::Inline(InterpString::parse(&goal))) + }; + + if run + .pull_request + .as_ref() + .is_some_and(|pull_request| !pull_request.enabled.unwrap_or(false)) + { + run.pull_request = None; + } + + layer +} diff --git a/lib/crates/fabro-workflow/src/run_options.rs b/lib/crates/fabro-workflow/src/run_options.rs index ddcf93cb2..a1ca65c7a 100644 --- a/lib/crates/fabro-workflow/src/run_options.rs +++ b/lib/crates/fabro-workflow/src/run_options.rs @@ -5,7 +5,6 @@ use std::sync::atomic::AtomicBool; use fabro_types::RunId; use fabro_types::settings::SettingsFile; -use fabro_types::settings::run::RunPullRequestLayer; use crate::git::{GitAuthor, git_author_from_settings}; @@ -43,28 +42,25 @@ pub struct RunOptions { impl RunOptions { pub fn dry_run_enabled(&self) -> bool { - self.settings.dry_run_enabled() + fabro_config::resolve_run_from_file(&self.settings) + .map(|settings| settings.execution.mode == fabro_types::settings::run::RunMode::DryRun) + .unwrap_or(false) } - pub fn checkpoint_exclude_globs(&self) -> &[String] { - self.settings - .run_checkpoint() - .map_or(&[], |cp| cp.exclude_globs.as_slice()) + pub fn checkpoint_exclude_globs(&self) -> Vec { + fabro_config::resolve_run_from_file(&self.settings) + .map(|settings| settings.checkpoint.exclude_globs) + .unwrap_or_default() } pub fn git_author(&self) -> GitAuthor { git_author_from_settings(&self.settings) } - /// PR config, if present in the v2 run layer. - pub fn pull_request(&self) -> Option<&RunPullRequestLayer> { - self.settings.run_pull_request() - } - - pub fn artifact_globs(&self) -> &[String] { - self.settings - .run_artifacts() - .map_or(&[], |a| a.include.as_slice()) + pub fn artifact_globs(&self) -> Vec { + fabro_config::resolve_run_from_file(&self.settings) + .map(|settings| settings.artifacts.include) + .unwrap_or_default() } } diff --git a/lib/crates/fabro-workflow/tests/materialize_run.rs b/lib/crates/fabro-workflow/tests/materialize_run.rs new file mode 100644 index 000000000..1b2889899 --- /dev/null +++ b/lib/crates/fabro-workflow/tests/materialize_run.rs @@ -0,0 +1,50 @@ +use fabro_graphviz::graph::Graph; +use fabro_model::Catalog; +use fabro_types::settings::run::{RunGoalLayer, RunLayer, RunModelLayer, RunPullRequestLayer}; +use fabro_types::settings::{InterpString, SettingsFile}; +use fabro_workflow::run_materialization::materialize_run; + +fn graph(source: &str) -> Graph { + fabro_graphviz::parser::parse(source).expect("graph should parse") +} + +#[test] +fn materialize_run_applies_graph_and_catalog_defaults() { + let source = r#"digraph Test { + graph [goal="Build feature"] + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#; + + let settings = SettingsFile { + run: Some(RunLayer { + model: Some(RunModelLayer { + name: Some(InterpString::parse("sonnet")), + ..RunModelLayer::default() + }), + pull_request: Some(RunPullRequestLayer { + enabled: Some(false), + ..RunPullRequestLayer::default() + }), + ..RunLayer::default() + }), + ..SettingsFile::default() + }; + + let materialized = materialize_run(settings, &graph(source), &Catalog::builtin()); + + assert_eq!( + materialized.run_model_name_str().as_deref(), + Some("claude-sonnet-4-6") + ); + assert_eq!( + materialized.run_model_provider_str().as_deref(), + Some("anthropic") + ); + assert_eq!( + materialized.run_goal_layer(), + Some(&RunGoalLayer::Inline(InterpString::parse("Build feature"))) + ); + assert!(materialized.run_pull_request().is_none()); +}