scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-05 08:10:39 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

Extract shared prepare_workflow to eliminate create/run duplication

Browse source 
The workflow preparation logic (config discovery, graph parsing, goal
resolution, sandbox/model resolution) was duplicated between create.rs
and run.rs. Extract into a shared prepare_workflow() function, reducing
create.rs from 204 to 91 lines.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-20 12:52:02 -04:00
parent 9c22bb8834
commit 89ecff48cb
2 changed files with 102 additions and 171 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

130
lib/crates/fabro-cli/src/commands/create.rs
View file

@ -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()

143
lib/crates/fabro-cli/src/commands/run.rs
View file

@ -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<fabro_github::GitHubAppCredentials>,
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<WorkflowRunConfig>,
pub sandbox_provider: SandboxProvider,
pub model: String,
pub provider: Option<String>,
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<PreparedWorkflow> {
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<String> = 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<String> = 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<fabro_github::GitHubAppCredentials>,
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<String> = 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<String> = 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()