diff --git a/lib/crates/fabro-cli/src/commands/create.rs b/lib/crates/fabro-cli/src/commands/create.rs index d5db83934..a226f2c0e 100644 --- a/lib/crates/fabro-cli/src/commands/create.rs +++ b/lib/crates/fabro-cli/src/commands/create.rs @@ -1,19 +1,10 @@ use std::path::PathBuf; -use anyhow::bail; use chrono::Local; -use fabro_config::project as project_config; use fabro_config::run::RunDefaults; -use fabro_validate::Severity; use fabro_workflows::run_spec::RunSpec; -use fabro_workflows::sandbox_provider::SandboxProvider; -use fabro_workflows::workflow::WorkflowBuilder; -use super::run::{ - apply_goal_override, resolve_cli_goal, resolve_model_provider, resolve_sandbox_provider, - RunArgs, -}; -use super::shared::{print_diagnostics, read_workflow_file, relative_path}; +use super::run::{prepare_workflow, RunArgs}; use fabro_util::terminal::Styles; /// Create a workflow run: allocate run directory, persist spec, return (run_id, run_dir). @@ -21,7 +12,7 @@ use fabro_util::terminal::Styles; /// This does NOT execute the workflow — it only prepares the run directory. pub async fn create_run( args: &RunArgs, - mut run_defaults: RunDefaults, + run_defaults: RunDefaults, styles: &Styles, ) -> anyhow::Result<(String, PathBuf)> { let workflow_path = args @@ -29,112 +20,9 @@ pub async fn create_run( .as_ref() .ok_or_else(|| anyhow::anyhow!("--workflow is required"))?; - // Apply project-level config overrides - if let Ok(Some((_config_path, project_config))) = - project_config::discover_project_config(&std::env::current_dir().unwrap_or_default()) - { - tracing::debug!("Applying run defaults from fabro.toml"); - run_defaults.merge_overlay(project_config.into_run_defaults()); - } + let prep = prepare_workflow(args, run_defaults, styles)?; - // Resolve workflow arg, load run config if TOML - let (dot_path, run_cfg) = { - let (dot, cfg) = project_config::resolve_workflow(workflow_path)?; - match cfg { - Some(mut cfg) => { - cfg.apply_defaults(&run_defaults); - (dot, Some(cfg)) - } - None => (dot, None), - } - }; - - let directory = run_cfg - .as_ref() - .and_then(|c| c.work_dir.as_deref()) - .or(run_defaults.work_dir.as_deref()); - if let Some(dir) = directory { - std::env::set_current_dir(dir) - .map_err(|e| anyhow::anyhow!("Failed to set working directory to {dir}: {e}"))?; - } - - // Parse and validate workflow - let source = read_workflow_file(&dot_path)?; - let vars = run_cfg - .as_ref() - .and_then(|c| c.vars.as_ref()) - .or(run_defaults.vars.as_ref()); - let source = match vars { - Some(vars) => fabro_workflows::vars::expand_vars(&source, vars)?, - None => source, - }; - let dot_dir = dot_path.parent().unwrap_or(std::path::Path::new(".")); - let (mut graph, diagnostics) = - WorkflowBuilder::new().prepare_with_file_inlining(&source, dot_dir)?; - let cli_goal = resolve_cli_goal(&args.goal, &args.goal_file)?; - let toml_goal = run_cfg.as_ref().and_then(|c| c.goal.as_deref()); - apply_goal_override(&mut graph, cli_goal.as_deref(), toml_goal); - - // Inline @file references in the goal - if let Some(fabro_graphviz::graph::AttrValue::String(goal)) = graph.attrs.get("goal") { - let fallback = dirs::home_dir().map(|h| h.join(".fabro")); - let resolved = - fabro_workflows::transform::resolve_file_ref(goal, dot_dir, fallback.as_deref()); - if resolved != *goal { - graph.attrs.insert( - "goal".to_string(), - fabro_graphviz::graph::AttrValue::String(resolved), - ); - } - } - - eprintln!( - "{} {} {}", - styles.bold.apply_to("Workflow:"), - graph.name, - styles.dim.apply_to(format!( - "({} nodes, {} edges)", - graph.nodes.len(), - graph.edges.len() - )), - ); - eprintln!( - "{} {}", - styles.dim.apply_to("Graph:"), - styles.dim.apply_to(relative_path(&dot_path)), - ); - - let goal = graph.goal(); - if !goal.is_empty() { - let first_line = goal.lines().next().unwrap_or(goal); - eprintln!("{} {first_line}\n", styles.bold.apply_to("Goal:")); - } - - print_diagnostics(&diagnostics, styles); - - if diagnostics.iter().any(|d| d.severity == Severity::Error) { - bail!("Validation failed"); - } - - // Resolve sandbox provider - let sandbox_provider = if args.dry_run { - SandboxProvider::Local - } else { - resolve_sandbox_provider( - args.sandbox.map(Into::into), - run_cfg.as_ref(), - &run_defaults, - )? - }; - - // Resolve model and provider - let (model, provider) = resolve_model_provider( - args.model.as_deref(), - args.provider.as_deref(), - run_cfg.as_ref(), - &run_defaults, - &graph, - ); + let goal = prep.graph.goal(); // Create run directory let run_id = ulid::Ulid::new().to_string(); @@ -152,7 +40,7 @@ pub async fn create_run( tokio::fs::create_dir_all(&run_dir).await?; // Write essential files - tokio::fs::write(run_dir.join("graph.fabro"), &source).await?; + tokio::fs::write(run_dir.join("graph.fabro"), &prep.source).await?; tokio::fs::write(run_dir.join("id.txt"), &run_id).await?; std::fs::File::create(run_dir.join("progress.jsonl"))?; fabro_workflows::run_status::write_run_status( @@ -173,16 +61,16 @@ pub async fn create_run( let spec = RunSpec { run_id: run_id.clone(), workflow_path: std::fs::canonicalize(workflow_path).unwrap_or(workflow_path.clone()), - dot_source: source, + dot_source: prep.source, working_directory, goal: if goal.is_empty() { None } else { Some(goal.to_string()) }, - model, - provider, - sandbox_provider: sandbox_provider.to_string(), + model: prep.model, + provider: prep.provider, + sandbox_provider: prep.sandbox_provider.to_string(), labels: args .label .iter() diff --git a/lib/crates/fabro-cli/src/commands/run.rs b/lib/crates/fabro-cli/src/commands/run.rs index adf4260b0..effcb480f 100644 --- a/lib/crates/fabro-cli/src/commands/run.rs +++ b/lib/crates/fabro-cli/src/commands/run.rs @@ -433,30 +433,32 @@ struct CostAccumulator { has_pricing: bool, } -/// Execute a full workflow run. -/// -/// # Errors -/// -/// Returns an error if the workflow cannot be read, parsed, validated, or executed. -pub async fn run_command( - args: RunArgs, - mut run_defaults: RunDefaults, - styles: &'static Styles, - github_app: Option, - git_author: fabro_workflows::git::GitAuthor, -) -> anyhow::Result<()> { - // Handle --run-branch resume: read everything from git metadata - if let Some(branch) = args.run_branch.clone() { - return run_from_branch(args, &branch, styles, git_author, run_defaults, github_app).await; - } +/// Result of workflow preparation (shared between `create` and `run` commands). +pub(crate) struct PreparedWorkflow { + pub source: String, + pub graph: fabro_graphviz::graph::Graph, + pub run_cfg: Option, + pub sandbox_provider: SandboxProvider, + pub model: String, + pub provider: Option, + pub run_defaults: RunDefaults, +} +/// Resolve config, parse/validate the workflow graph, and resolve sandbox + model. +/// +/// Shared between `create_run` (which only persists the spec) and +/// `run_command` (which goes on to execute the workflow). +pub(crate) fn prepare_workflow( + args: &RunArgs, + mut run_defaults: RunDefaults, + styles: &Styles, +) -> anyhow::Result { let workflow_path = args .workflow .as_ref() - .ok_or_else(|| anyhow::anyhow!("--workflow is required unless --run-branch is provided"))?; + .ok_or_else(|| anyhow::anyhow!("--workflow is required"))?; // Apply project-level config overrides (fabro.toml) on top of CLI defaults. - // Precedence: workflow.toml > fabro.toml > cli.toml/server.toml if let Ok(Some((_config_path, project_config))) = project_config::discover_project_config(&std::env::current_dir().unwrap_or_default()) { @@ -464,7 +466,7 @@ pub async fn run_command( run_defaults.merge_overlay(project_config.into_run_defaults()); } - // 0. Resolve workflow arg, load run config if TOML, resolve DOT path, apply defaults + // Resolve workflow arg, load run config if TOML, apply defaults let (dot_path, run_cfg) = { let (dot, cfg) = project_config::resolve_workflow(workflow_path)?; match cfg { @@ -476,18 +478,6 @@ pub async fn run_command( } }; - // Extract workflow slug from the workflow path argument. - // If bare name (no extension, e.g. "smoke"), use it directly. - // Otherwise derive from the parent directory of the resolved .toml path. - let workflow_slug: Option = if workflow_path.extension().is_none() { - Some(workflow_path.to_string_lossy().into_owned()) - } else { - workflow_path - .parent() - .and_then(|p| p.file_name()) - .map(|n| n.to_string_lossy().into_owned()) - }; - let directory = run_cfg .as_ref() .and_then(|c| c.work_dir.as_deref()) @@ -497,15 +487,7 @@ pub async fn run_command( .map_err(|e| anyhow::anyhow!("Failed to set working directory to {dir}: {e}"))?; } - // Collect setup commands — they'll be run inside the sandbox - let setup_commands: Vec = run_cfg - .as_ref() - .and_then(|c| c.setup.as_ref()) - .or(run_defaults.setup.as_ref()) - .map(|s| s.commands.clone()) - .unwrap_or_default(); - - // 1. Parse and validate workflow + // Parse and validate workflow let source = read_workflow_file(&dot_path)?; let vars = run_cfg .as_ref() @@ -563,8 +545,7 @@ pub async fn run_command( bail!("Validation failed"); } - // 2. Pre-flight: check git cleanliness before creating any files - // (must happen before logs dir is created, which may be inside the repo) + // Resolve sandbox provider let sandbox_provider = if args.dry_run { SandboxProvider::Local } else { @@ -574,6 +555,76 @@ pub async fn run_command( &run_defaults, )? }; + + // Resolve model and provider + let (model, provider) = resolve_model_provider( + args.model.as_deref(), + args.provider.as_deref(), + run_cfg.as_ref(), + &run_defaults, + &graph, + ); + + Ok(PreparedWorkflow { + source, + graph, + run_cfg, + sandbox_provider, + model, + provider, + run_defaults, + }) +} + +/// Execute a full workflow run. +/// +/// # Errors +/// +/// Returns an error if the workflow cannot be read, parsed, validated, or executed. +pub async fn run_command( + args: RunArgs, + run_defaults: RunDefaults, + styles: &'static Styles, + github_app: Option, + git_author: fabro_workflows::git::GitAuthor, +) -> anyhow::Result<()> { + // Handle --run-branch resume: read everything from git metadata + if let Some(branch) = args.run_branch.clone() { + return run_from_branch(args, &branch, styles, git_author, run_defaults, github_app).await; + } + + let PreparedWorkflow { + source, + graph, + run_cfg, + sandbox_provider, + model, + provider, + run_defaults, + } = prepare_workflow(&args, run_defaults, styles)?; + + // Extract workflow slug from the workflow path argument. + // If bare name (no extension, e.g. "smoke"), use it directly. + // Otherwise derive from the parent directory of the resolved .toml path. + let workflow_path = args.workflow.as_ref().unwrap(); // safe: prepare_workflow validated + let workflow_slug: Option = if workflow_path.extension().is_none() { + Some(workflow_path.to_string_lossy().into_owned()) + } else { + workflow_path + .parent() + .and_then(|p| p.file_name()) + .map(|n| n.to_string_lossy().into_owned()) + }; + + // Collect setup commands — they'll be run inside the sandbox + let setup_commands: Vec = run_cfg + .as_ref() + .and_then(|c| c.setup.as_ref()) + .or(run_defaults.setup.as_ref()) + .map(|s| s.commands.clone()) + .unwrap_or_default(); + + // Pre-flight: check git cleanliness before creating any files let preserve_sandbox = resolve_preserve_sandbox(args.preserve_sandbox, run_cfg.as_ref(), &run_defaults); let original_cwd = std::env::current_dir()?; @@ -1171,14 +1222,6 @@ pub async fn run_command( } }; - let (model, provider) = resolve_model_provider( - args.model.as_deref(), - args.provider.as_deref(), - run_cfg.as_ref(), - &run_defaults, - &graph, - ); - // Parse provider string to enum (defaults to best available from env) let provider_enum: Provider = provider .as_deref()