diff --git a/crates/arc-workflows/src/cli/run.rs b/crates/arc-workflows/src/cli/run.rs index ca90943e9..3d4230cf2 100644 --- a/crates/arc-workflows/src/cli/run.rs +++ b/crates/arc-workflows/src/cli/run.rs @@ -249,11 +249,8 @@ pub async fn run_command( None => source, }; let dot_dir = dot_path.parent().unwrap_or(std::path::Path::new(".")); - let mut builder = WorkflowBuilder::new(); - builder.register_transform(Box::new(crate::transform::FileInliningTransform::new( - dot_dir.to_path_buf(), - ))); - let (mut graph, diagnostics) = builder.prepare(&source)?; + let (mut graph, diagnostics) = + WorkflowBuilder::new().prepare_with_file_inlining(&source, dot_dir)?; let toml_goal = run_cfg.as_ref().and_then(|c| c.goal.as_deref()); apply_goal_override(&mut graph, args.goal.as_deref(), toml_goal); @@ -1009,13 +1006,13 @@ async fn run_from_branch( .ok_or_else(|| anyhow::anyhow!("no graph.dot found on metadata branch for run {run_id}"))?; // If --pipeline was also provided, use it instead (allows overriding) - let source = if let Some(ref workflow_path) = args.workflow { - super::read_dot_file(workflow_path)? + let (mut graph, diagnostics) = if let Some(ref workflow_path) = args.workflow { + let source = super::read_dot_file(workflow_path)?; + let dot_dir = workflow_path.parent().unwrap_or(std::path::Path::new(".")); + crate::workflow::WorkflowBuilder::new().prepare_with_file_inlining(&source, dot_dir)? } else { - source + crate::workflow::WorkflowBuilder::new().prepare(&source)? }; - - let (mut graph, diagnostics) = crate::workflow::WorkflowBuilder::new().prepare(&source)?; apply_goal_override(&mut graph, args.goal.as_deref(), None); eprintln!( diff --git a/crates/arc-workflows/src/cli/validate.rs b/crates/arc-workflows/src/cli/validate.rs index 3a2be370b..8be0487d3 100644 --- a/crates/arc-workflows/src/cli/validate.rs +++ b/crates/arc-workflows/src/cli/validate.rs @@ -2,9 +2,9 @@ use anyhow::bail; use arc_util::terminal::Styles; use crate::validation::Severity; -use crate::workflow::WorkflowBuilder; +use crate::workflow::prepare_from_file; -use super::{print_diagnostics, read_dot_file, ValidateArgs}; +use super::{print_diagnostics, ValidateArgs}; /// Parse and validate a workflow file without executing it. /// @@ -12,13 +12,7 @@ use super::{print_diagnostics, read_dot_file, ValidateArgs}; /// /// Returns an error if the file cannot be read, parsed, or has validation errors. pub fn validate_command(args: &ValidateArgs, styles: &Styles) -> anyhow::Result<()> { - let source = read_dot_file(&args.workflow)?; - let dot_dir = args.workflow.parent().unwrap_or(std::path::Path::new(".")); - let mut builder = WorkflowBuilder::new(); - builder.register_transform(Box::new(crate::transform::FileInliningTransform::new( - dot_dir.to_path_buf(), - ))); - let (graph, diagnostics) = builder.prepare(&source)?; + let (graph, diagnostics) = prepare_from_file(&args.workflow)?; eprintln!( "{} ({} nodes, {} edges)", diff --git a/crates/arc-workflows/src/handler/manager_loop.rs b/crates/arc-workflows/src/handler/manager_loop.rs index 27a587833..477a9c76b 100644 --- a/crates/arc-workflows/src/handler/manager_loop.rs +++ b/crates/arc-workflows/src/handler/manager_loop.rs @@ -13,7 +13,7 @@ use crate::engine::{RunConfig, WorkflowRunEngine}; use crate::error::ArcError; use crate::graph::{Graph, Node}; use crate::outcome::{Outcome, StageStatus}; -use crate::workflow::prepare_workflow; +use crate::workflow::{prepare_from_file, prepare_from_source}; use super::{EngineServices, Handler}; @@ -42,23 +42,31 @@ fn parse_duration_str(s: &str) -> Duration { Duration::from_secs(45) } -/// Read DOT source from node attributes: inline `stack.child_dot_source` or -/// file path `stack.child_dotfile`. -fn read_child_dot(node: &Node) -> Result { +/// Parse a child workflow graph from node attributes: inline `stack.child_dot_source` +/// (no file inlining) or file path `stack.child_dotfile` (with file inlining). +fn parse_child_graph(node: &Node) -> Result { if let Some(dot) = node .attrs .get("stack.child_dot_source") .and_then(|v| v.as_str()) { - return Ok(dot.to_string()); + return prepare_from_source(dot); } if let Some(path) = node .attrs .get("stack.child_dotfile") .and_then(|v| v.as_str()) { - return std::fs::read_to_string(path) - .map_err(|e| ArcError::handler(format!("Failed to read child dotfile {path}: {e}"))); + let (graph, diagnostics) = prepare_from_file(std::path::Path::new(path))?; + let errors: Vec<&crate::validation::Diagnostic> = diagnostics + .iter() + .filter(|d| d.severity == crate::validation::Severity::Error) + .collect(); + if !errors.is_empty() { + let messages: Vec = errors.iter().map(|d| d.message.clone()).collect(); + return Err(ArcError::Validation(messages.join("; "))); + } + return Ok(graph); } Err(ArcError::handler("No child DOT source".to_string())) } @@ -113,13 +121,8 @@ impl Handler for SubWorkflowHandler { .and_then(|v| v.as_str()) .unwrap_or(""); - // Read and parse child DOT source - let dot_source = match read_child_dot(node) { - Ok(s) => s, - Err(e) => return Ok(Outcome::fail_classify(e.to_string())), - }; - - let child_graph = match prepare_workflow(&dot_source) { + // Read and parse child workflow graph + let child_graph = match parse_child_graph(node) { Ok(g) => g, Err(e) => { return Ok(Outcome::fail_classify(format!( diff --git a/crates/arc-workflows/src/workflow.rs b/crates/arc-workflows/src/workflow.rs index fa526ff4c..a8350faec 100644 --- a/crates/arc-workflows/src/workflow.rs +++ b/crates/arc-workflows/src/workflow.rs @@ -1,6 +1,10 @@ +use std::path::Path; + use crate::error::ArcError; use crate::graph::Graph; -use crate::transform::{StylesheetApplicationTransform, Transform, VariableExpansionTransform}; +use crate::transform::{ + FileInliningTransform, StylesheetApplicationTransform, Transform, VariableExpansionTransform, +}; use crate::validation::{self, Diagnostic}; /// Builder for configuring and executing a workflow preparation. @@ -29,12 +33,39 @@ impl WorkflowBuilder { /// /// Returns an error if parsing or validation fails. pub fn prepare(&self, dot_source: &str) -> Result<(Graph, Vec), ArcError> { + self.prepare_inner(dot_source, None) + } + + /// Prepare a workflow with file inlining: parse DOT, apply built-in transforms + /// including `FileInliningTransform`, then custom transforms, then validate. + /// + /// # Errors + /// + /// Returns an error if parsing or validation fails. + pub fn prepare_with_file_inlining( + &self, + dot_source: &str, + base_dir: &Path, + ) -> Result<(Graph, Vec), ArcError> { + self.prepare_inner(dot_source, Some(base_dir)) + } + + fn prepare_inner( + &self, + dot_source: &str, + base_dir: Option<&Path>, + ) -> Result<(Graph, Vec), ArcError> { let mut graph = crate::parser::parse(dot_source)?; // Built-in transforms (PreambleTransform moved to engine execution time) VariableExpansionTransform.apply(&mut graph); StylesheetApplicationTransform.apply(&mut graph); + // File inlining when base_dir is provided + if let Some(dir) = base_dir { + FileInliningTransform::new(dir.to_path_buf()).apply(&mut graph); + } + // Custom transforms for transform in &self.transforms { transform.apply(&mut graph); @@ -51,12 +82,25 @@ impl Default for WorkflowBuilder { } } -/// Convenience function: parse DOT, apply built-in transforms, validate, return graph. +/// Convenience: read a DOT file, apply built-in transforms including file inlining, validate. +/// +/// # Errors +/// +/// Returns an error if the file cannot be read, parsed, or validated. +pub fn prepare_from_file(path: &Path) -> Result<(Graph, Vec), ArcError> { + let source = std::fs::read_to_string(path) + .map_err(|e| ArcError::Parse(format!("Failed to read {}: {e}", path.display())))?; + let dot_dir = path.parent().unwrap_or(Path::new(".")); + WorkflowBuilder::new().prepare_with_file_inlining(&source, dot_dir) +} + +/// Convenience: parse DOT source (no file inlining), apply built-in transforms, validate. +/// Returns the graph or an error if validation produces Error-severity diagnostics. /// /// # Errors /// /// Returns an error if parsing fails or if validation produces Error-severity diagnostics. -pub fn prepare_workflow(dot_source: &str) -> Result { +pub fn prepare_from_source(dot_source: &str) -> Result { let builder = WorkflowBuilder::new(); let (graph, diagnostics) = builder.prepare(dot_source)?; @@ -85,15 +129,15 @@ mod tests { }"#; #[test] - fn prepare_workflow_minimal() { - let graph = prepare_workflow(MINIMAL_DOT).unwrap(); + fn prepare_from_source_minimal() { + let graph = prepare_from_source(MINIMAL_DOT).unwrap(); assert_eq!(graph.name, "Test"); assert!(graph.find_start_node().is_some()); assert!(graph.find_exit_node().is_some()); } #[test] - fn prepare_workflow_applies_variable_expansion() { + fn prepare_from_source_applies_variable_expansion() { let dot = r#"digraph Test { graph [goal="Fix bugs"] start [shape=Mdiamond] @@ -101,7 +145,7 @@ mod tests { exit [shape=Msquare] start -> work -> exit }"#; - let graph = prepare_workflow(dot).unwrap(); + let graph = prepare_from_source(dot).unwrap(); let prompt = graph.nodes["work"] .attrs .get("prompt") @@ -111,7 +155,7 @@ mod tests { } #[test] - fn prepare_workflow_applies_stylesheet() { + fn prepare_from_source_applies_stylesheet() { let dot = r#"digraph Test { graph [goal="Test", model_stylesheet="* { llm_model: sonnet; }"] start [shape=Mdiamond] @@ -119,7 +163,7 @@ mod tests { exit [shape=Msquare] start -> work -> exit }"#; - let graph = prepare_workflow(dot).unwrap(); + let graph = prepare_from_source(dot).unwrap(); assert_eq!( graph.nodes["work"].attrs.get("llm_model"), Some(&AttrValue::String("sonnet".into())) @@ -127,18 +171,18 @@ mod tests { } #[test] - fn prepare_workflow_returns_error_on_invalid_dot() { - let result = prepare_workflow("not a graph"); + fn prepare_from_source_returns_error_on_invalid_dot() { + let result = prepare_from_source("not a graph"); assert!(result.is_err()); } #[test] - fn prepare_workflow_returns_error_on_validation_failure() { + fn prepare_from_source_returns_error_on_validation_failure() { let dot = r#"digraph Test { graph [goal="Test"] work [label="Work"] }"#; - let result = prepare_workflow(dot); + let result = prepare_from_source(dot); assert!(result.is_err()); }