From 847c0481d01fff9b09be02bf499281bdffe93489 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 28 Mar 2026 17:28:37 -0400 Subject: [PATCH] Add file-based workflow imports --- lib/crates/fabro-validate/src/rules.rs | 97 +- .../fabro-workflows/src/pipeline/transform.rs | 104 +- .../src/transforms/graph_merge.rs | 234 ---- .../fabro-workflows/src/transforms/import.rs | 1227 +++++++++++++++++ .../fabro-workflows/src/transforms/mod.rs | 4 +- .../fabro-workflows/tests/integration.rs | 197 ++- 6 files changed, 1515 insertions(+), 348 deletions(-) delete mode 100644 lib/crates/fabro-workflows/src/transforms/graph_merge.rs create mode 100644 lib/crates/fabro-workflows/src/transforms/import.rs diff --git a/lib/crates/fabro-validate/src/rules.rs b/lib/crates/fabro-validate/src/rules.rs index ece6d5503..d3a64ca3c 100644 --- a/lib/crates/fabro-validate/src/rules.rs +++ b/lib/crates/fabro-validate/src/rules.rs @@ -32,6 +32,7 @@ pub fn built_in_rules() -> Vec> { Box::new(ScriptAbsoluteCdRule), Box::new(StylesheetModelKnownRule), Box::new(NodeModelKnownRule), + Box::new(ImportErrorRule), Box::new(UnresolvedFileRefRule), Box::new(ThreadIdRequiresFidelityFullRule), Box::new(SelectionValidRule), @@ -1033,7 +1034,50 @@ impl LintRule for NodeModelKnownRule { } } -// --- Rule 22: unresolved_file_ref (ERROR) --- +// --- Rule 22: import_error (ERROR) --- + +struct ImportErrorRule; + +impl LintRule for ImportErrorRule { + fn name(&self) -> &'static str { + "import_error" + } + + fn apply(&self, graph: &Graph) -> Vec { + let mut diagnostics = Vec::new(); + + for node in graph.nodes.values() { + if let Some(AttrValue::String(message)) = node.attrs.get("import_error") { + diagnostics.push(Diagnostic { + rule: self.name().to_string(), + severity: Severity::Error, + message: message.clone(), + node_id: Some(node.id.clone()), + edge: None, + fix: Some("Fix the imported workflow or import path".to_string()), + }); + } + + if node.attrs.contains_key("import") { + diagnostics.push(Diagnostic { + rule: self.name().to_string(), + severity: Severity::Error, + message: "unresolved import (no base directory available)".to_string(), + node_id: Some(node.id.clone()), + edge: None, + fix: Some( + "Load the workflow from a file so imports can resolve relative to it" + .to_string(), + ), + }); + } + } + + diagnostics + } +} + +// --- Rule 23: unresolved_file_ref (ERROR) --- struct UnresolvedFileRefRule; @@ -1080,7 +1124,7 @@ impl LintRule for UnresolvedFileRefRule { } } -// --- Rule 22: thread_id_requires_fidelity_full (WARNING) --- +// --- Rule 24: thread_id_requires_fidelity_full (WARNING) --- struct ThreadIdRequiresFidelityFullRule; @@ -3162,6 +3206,55 @@ mod tests { assert!(d.is_empty()); } + // import_error rule tests + + #[test] + fn import_error_rule_fires_on_import_error_attr() { + let mut g = minimal_graph(); + let mut node = Node::new("work"); + node.attrs.insert( + "import_error".to_string(), + AttrValue::String("file not found: ./missing.fabro".to_string()), + ); + g.nodes.insert("work".to_string(), node); + + let rule = ImportErrorRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Error); + assert_eq!(d[0].message, "file not found: ./missing.fabro"); + assert_eq!(d[0].node_id.as_deref(), Some("work")); + } + + #[test] + fn import_error_rule_fires_on_unresolved_import_attr() { + let mut g = minimal_graph(); + let mut node = Node::new("work"); + node.attrs.insert( + "import".to_string(), + AttrValue::String("./validate.fabro".to_string()), + ); + g.nodes.insert("work".to_string(), node); + + let rule = ImportErrorRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Error); + assert_eq!( + d[0].message, + "unresolved import (no base directory available)" + ); + assert_eq!(d[0].node_id.as_deref(), Some("work")); + } + + #[test] + fn import_error_rule_silent_for_clean_nodes() { + let g = minimal_graph(); + let rule = ImportErrorRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + // unresolved_file_ref rule tests #[test] diff --git a/lib/crates/fabro-workflows/src/pipeline/transform.rs b/lib/crates/fabro-workflows/src/pipeline/transform.rs index eb48e2482..44a85f540 100644 --- a/lib/crates/fabro-workflows/src/pipeline/transform.rs +++ b/lib/crates/fabro-workflows/src/pipeline/transform.rs @@ -1,6 +1,6 @@ use crate::transforms::{ - FileInliningTransform, ModelResolutionTransform, StylesheetApplicationTransform, Transform, - VariableExpansionTransform, + FileInliningTransform, ImportTransform, ModelResolutionTransform, + StylesheetApplicationTransform, Transform, VariableExpansionTransform, }; use super::types::{Parsed, TransformOptions, Transformed}; @@ -13,16 +13,20 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Transformed { let Parsed { mut graph, source } = parsed; // Built-in transforms (PreambleTransform moved to engine execution time) + if let Some(ref dir) = options.base_dir { + let fallback = dirs::home_dir().map(|home| home.join(".fabro")); + ImportTransform::new(dir.clone(), fallback).apply(&mut graph); + } + + if let Some(ref dir) = options.base_dir { + let fallback = dirs::home_dir().map(|home| home.join(".fabro")); + FileInliningTransform::new(dir.clone(), fallback).apply(&mut graph); + } + VariableExpansionTransform.apply(&mut graph); StylesheetApplicationTransform.apply(&mut graph); ModelResolutionTransform.apply(&mut graph); - // File inlining when base_dir is provided - if let Some(ref dir) = options.base_dir { - let fallback = dirs::home_dir().map(|h| h.join(".fabro")); - FileInliningTransform::new(dir.clone(), fallback).apply(&mut graph); - } - // Custom transforms for t in &options.custom_transforms { t.apply(&mut graph); @@ -33,10 +37,19 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Transformed { #[cfg(test)] mod tests { + use std::path::Path; + use super::*; use crate::pipeline::parse::parse; use fabro_graphviz::graph::AttrValue; + fn write_file(path: &Path, contents: &str) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, contents).unwrap(); + } + #[test] fn transform_applies_variable_expansion() { let dot = r#"digraph Test { @@ -84,4 +97,79 @@ mod tests { Some(&AttrValue::String("claude-sonnet-4-6".into())) ); } + + #[test] + fn transform_inlines_files_before_variable_expansion() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("goal.md"), "Expand $goal"); + + let parsed = parse( + r#"digraph Test { + graph [goal="Ship it"] + start [shape=Mdiamond] + work [prompt="@goal.md"] + exit [shape=Msquare] + start -> work -> exit + }"#, + ) + .unwrap(); + let transformed = transform( + parsed, + &TransformOptions { + base_dir: Some(dir.path().to_path_buf()), + custom_transforms: vec![], + }, + ); + + assert_eq!( + transformed.graph.nodes["work"] + .attrs + .get("prompt") + .and_then(AttrValue::as_str), + Some("Expand Ship it") + ); + } + + #[test] + fn transform_imports_before_variable_expansion_and_stylesheet() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("prompts/lint.md"), "Run checks for $goal"); + write_file( + &dir.path().join("validate.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="@prompts/lint.md"] + exit [shape=Msquare] + start -> lint -> exit + }"#, + ); + + let parsed = parse( + r#"digraph Test { + graph [goal="Launch", model_stylesheet=".validate { model: sonnet; }"] + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + ) + .unwrap(); + let transformed = transform( + parsed, + &TransformOptions { + base_dir: Some(dir.path().to_path_buf()), + custom_transforms: vec![], + }, + ); + + let lint = &transformed.graph.nodes["validate.lint"]; + assert_eq!( + lint.attrs.get("prompt").and_then(AttrValue::as_str), + Some("Run checks for Launch") + ); + assert_eq!( + lint.attrs.get("model"), + Some(&AttrValue::String("claude-sonnet-4-6".into())) + ); + } } diff --git a/lib/crates/fabro-workflows/src/transforms/graph_merge.rs b/lib/crates/fabro-workflows/src/transforms/graph_merge.rs deleted file mode 100644 index b582a7e92..000000000 --- a/lib/crates/fabro-workflows/src/transforms/graph_merge.rs +++ /dev/null @@ -1,234 +0,0 @@ -use fabro_graphviz::graph::{Edge, Graph, Node}; - -use super::Transform; - -/// Merges nodes and edges from secondary graphs into the primary graph. -/// Node IDs from secondary graphs are prefixed with a namespace to avoid collisions. -pub struct GraphMergeTransform { - secondary_graphs: Vec, -} - -impl GraphMergeTransform { - #[must_use] - pub const fn new(secondary_graphs: Vec) -> Self { - Self { secondary_graphs } - } -} - -impl Transform for GraphMergeTransform { - fn apply(&self, graph: &mut Graph) { - for secondary in &self.secondary_graphs { - let prefix = &secondary.name; - - for (id, node) in &secondary.nodes { - let prefixed_id = format!("{prefix}.{id}"); - let mut merged_node = Node::new(&prefixed_id); - merged_node.attrs.clone_from(&node.attrs); - merged_node.classes.clone_from(&node.classes); - graph.nodes.insert(prefixed_id, merged_node); - } - - for edge in &secondary.edges { - let mut merged_edge = Edge::new( - format!("{prefix}.{}", edge.from), - format!("{prefix}.{}", edge.to), - ); - merged_edge.attrs.clone_from(&edge.attrs); - graph.edges.push(merged_edge); - } - } - } -} - -#[cfg(test)] -mod tests { - use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; - - use super::*; - - #[test] - fn graph_merge_combines_nodes_and_edges() { - let mut primary = Graph::new("primary"); - primary.nodes.insert("a".to_string(), Node::new("a")); - primary.nodes.insert("b".to_string(), Node::new("b")); - primary.edges.push(Edge::new("a", "b")); - - let mut secondary = Graph::new("secondary"); - secondary.nodes.insert("x".to_string(), Node::new("x")); - secondary.nodes.insert("y".to_string(), Node::new("y")); - secondary.edges.push(Edge::new("x", "y")); - - let transform = GraphMergeTransform::new(vec![secondary]); - transform.apply(&mut primary); - - // Primary should now have 4 nodes: a, b, secondary.x, secondary.y - assert_eq!(primary.nodes.len(), 4); - assert!(primary.nodes.contains_key("secondary.x")); - assert!(primary.nodes.contains_key("secondary.y")); - // Should have 2 edges: a->b and secondary.x->secondary.y - assert_eq!(primary.edges.len(), 2); - } - - #[test] - fn graph_merge_prefixes_node_ids_to_avoid_collisions() { - let mut primary = Graph::new("primary"); - primary.nodes.insert("work".to_string(), Node::new("work")); - - let mut secondary = Graph::new("sub"); - secondary - .nodes - .insert("work".to_string(), Node::new("work")); - - let transform = GraphMergeTransform::new(vec![secondary]); - transform.apply(&mut primary); - - // Primary "work" is preserved, secondary "work" becomes "sub.work" - assert!(primary.nodes.contains_key("work")); - assert!(primary.nodes.contains_key("sub.work")); - assert_eq!(primary.nodes.len(), 2); - } - - #[test] - fn graph_merge_remaps_edges_to_prefixed_ids() { - let mut primary = Graph::new("primary"); - primary.nodes.insert("a".to_string(), Node::new("a")); - - let mut secondary = Graph::new("sub"); - secondary.nodes.insert("x".to_string(), Node::new("x")); - secondary.nodes.insert("y".to_string(), Node::new("y")); - secondary.edges.push(Edge::new("x", "y")); - - let transform = GraphMergeTransform::new(vec![secondary]); - transform.apply(&mut primary); - - // The edge from secondary should be remapped to sub.x -> sub.y - let merged_edge = primary - .edges - .iter() - .find(|e| e.from == "sub.x") - .expect("should have edge from sub.x"); - assert_eq!(merged_edge.to, "sub.y"); - } - - #[test] - fn graph_merge_preserves_primary_attributes() { - let mut primary = Graph::new("primary"); - primary.attrs.insert( - "goal".to_string(), - AttrValue::String("Build feature".to_string()), - ); - primary.attrs.insert( - "model_stylesheet".to_string(), - AttrValue::String("* { model: sonnet; }".to_string()), - ); - - let mut secondary = Graph::new("sub"); - secondary.attrs.insert( - "goal".to_string(), - AttrValue::String("Sub goal".to_string()), - ); - secondary.nodes.insert("x".to_string(), Node::new("x")); - - let transform = GraphMergeTransform::new(vec![secondary]); - transform.apply(&mut primary); - - assert_eq!(primary.goal(), "Build feature"); - assert_eq!(primary.model_stylesheet(), "* { model: sonnet; }"); - } - - #[test] - fn graph_merge_empty_secondary_is_noop() { - let mut primary = Graph::new("primary"); - primary.nodes.insert("a".to_string(), Node::new("a")); - primary.edges.push(Edge::new("a", "a")); - - let secondary = Graph::new("empty"); - - let transform = GraphMergeTransform::new(vec![secondary]); - transform.apply(&mut primary); - - assert_eq!(primary.nodes.len(), 1); - assert_eq!(primary.edges.len(), 1); - } - - #[test] - fn graph_merge_multiple_secondary_graphs() { - let mut primary = Graph::new("primary"); - primary.nodes.insert("a".to_string(), Node::new("a")); - - let mut sub1 = Graph::new("sub1"); - sub1.nodes.insert("n1".to_string(), Node::new("n1")); - - let mut sub2 = Graph::new("sub2"); - sub2.nodes.insert("n2".to_string(), Node::new("n2")); - - let transform = GraphMergeTransform::new(vec![sub1, sub2]); - transform.apply(&mut primary); - - assert_eq!(primary.nodes.len(), 3); - assert!(primary.nodes.contains_key("a")); - assert!(primary.nodes.contains_key("sub1.n1")); - assert!(primary.nodes.contains_key("sub2.n2")); - } - - #[test] - fn graph_merge_preserves_node_attributes() { - let mut primary = Graph::new("primary"); - - let mut secondary = Graph::new("sub"); - let mut node = Node::new("worker"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Do the work".to_string()), - ); - node.attrs - .insert("shape".to_string(), AttrValue::String("box".to_string())); - secondary.nodes.insert("worker".to_string(), node); - - let transform = GraphMergeTransform::new(vec![secondary]); - transform.apply(&mut primary); - - let merged = &primary.nodes["sub.worker"]; - assert_eq!(merged.id, "sub.worker"); - assert_eq!( - merged.attrs.get("prompt").and_then(AttrValue::as_str), - Some("Do the work") - ); - assert_eq!( - merged.attrs.get("shape").and_then(AttrValue::as_str), - Some("box") - ); - } - - #[test] - fn graph_merge_preserves_edge_attributes() { - let mut primary = Graph::new("primary"); - - let mut secondary = Graph::new("sub"); - secondary.nodes.insert("x".to_string(), Node::new("x")); - secondary.nodes.insert("y".to_string(), Node::new("y")); - let mut edge = Edge::new("x", "y"); - edge.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=success".to_string()), - ); - secondary.edges.push(edge); - - let transform = GraphMergeTransform::new(vec![secondary]); - transform.apply(&mut primary); - - let merged_edge = primary - .edges - .iter() - .find(|e| e.from == "sub.x") - .expect("should have merged edge"); - assert_eq!(merged_edge.to, "sub.y"); - assert_eq!( - merged_edge - .attrs - .get("condition") - .and_then(AttrValue::as_str), - Some("outcome=success") - ); - } -} diff --git a/lib/crates/fabro-workflows/src/transforms/import.rs b/lib/crates/fabro-workflows/src/transforms/import.rs new file mode 100644 index 000000000..3c3b7fa22 --- /dev/null +++ b/lib/crates/fabro-workflows/src/transforms/import.rs @@ -0,0 +1,1227 @@ +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; +use fabro_graphviz::parser; + +use super::{FileInliningTransform, Transform}; + +pub struct ImportTransform { + base_dir: PathBuf, + fallback_dir: Option, +} + +struct PlaceholderConfig { + default_attrs: HashMap, + class_names: Vec, + normalized_class: String, +} + +struct PreparedImport { + graph: Graph, + start_id: String, + exit_id: String, + entry_id: String, + exit_predecessor_id: String, +} + +impl ImportTransform { + #[must_use] + pub fn new(base_dir: PathBuf, fallback_dir: Option) -> Self { + Self { + base_dir, + fallback_dir, + } + } + + fn resolve_import_path( + import_path: &str, + base_dir: &Path, + fallback_dir: Option<&Path>, + ) -> Option { + let raw = Path::new(import_path); + let is_tilde = raw.starts_with("~"); + let expanded = if is_tilde { + match dirs::home_dir() { + Some(home) => home.join(raw.strip_prefix("~").unwrap()), + None => base_dir.join(import_path), + } + } else { + base_dir.join(import_path) + }; + + match expanded.canonicalize() { + Ok(path) if path.is_file() => Some(path), + _ if !is_tilde => fallback_dir.and_then(|fallback| { + let fallback_path = fallback.join(import_path); + match fallback_path.canonicalize() { + Ok(path) if path.is_file() => Some(path), + _ => None, + } + }), + _ => None, + } + } + + fn collect_import_nodes(graph: &Graph) -> Vec<(String, String)> { + graph + .nodes + .iter() + .filter_map(|(id, node)| { + node.attrs + .get("import") + .and_then(AttrValue::as_str) + .map(|path| (id.clone(), path.to_string())) + }) + .collect() + } + + fn expand_import( + &self, + graph: &mut Graph, + placeholder_id: &str, + import_path: &str, + current_base_dir: &Path, + import_stack: &mut Vec, + ) { + if !graph.nodes.contains_key(placeholder_id) { + return; + } + + let placeholder = match Self::placeholder_config(graph, placeholder_id) { + Ok(placeholder) => placeholder, + Err(message) => { + Self::poison_placeholder(graph, placeholder_id, &message); + return; + } + }; + + let Some(resolved_path) = + Self::resolve_import_path(import_path, current_base_dir, self.fallback_dir.as_deref()) + else { + Self::poison_placeholder( + graph, + placeholder_id, + &format!("file not found: {import_path}"), + ); + return; + }; + + if import_stack.contains(&resolved_path) { + let cycle = import_stack + .iter() + .chain(std::iter::once(&resolved_path)) + .map(|path| path.display().to_string()) + .collect::>() + .join(" -> "); + Self::poison_placeholder( + graph, + placeholder_id, + &format!("circular import detected: {cycle}"), + ); + return; + } + + let prepared = match self.prepare_import(&resolved_path, import_stack) { + Ok(prepared) => prepared, + Err(message) => { + Self::poison_placeholder(graph, placeholder_id, &message); + return; + } + }; + + if let Err(message) = Self::splice_import( + graph, + placeholder_id, + &resolved_path, + &placeholder, + prepared, + ) { + Self::poison_placeholder(graph, placeholder_id, &message); + } + } + + fn prepare_import( + &self, + resolved_path: &Path, + import_stack: &mut Vec, + ) -> Result { + Self::with_import_stack(import_stack, resolved_path.to_path_buf(), |import_stack| { + let source = std::fs::read_to_string(resolved_path) + .map_err(|error| format!("failed to read {}: {error}", resolved_path.display()))?; + let mut graph = parser::parse(&source) + .map_err(|error| format!("failed to parse {}: {error}", resolved_path.display()))?; + + let import_base_dir = resolved_path + .parent() + .map_or_else(|| PathBuf::from("."), Path::to_path_buf); + FileInliningTransform::new(import_base_dir.clone(), self.fallback_dir.clone()) + .apply(&mut graph); + + if let Some(message) = Self::unresolved_imported_prompt_error(&graph) { + return Err(message); + } + + let nested_imports = Self::collect_import_nodes(&graph); + for (placeholder_id, import_path) in nested_imports { + self.expand_import( + &mut graph, + &placeholder_id, + &import_path, + &import_base_dir, + import_stack, + ); + } + + Self::validate_imported_graph(graph) + }) + } + + fn splice_import( + graph: &mut Graph, + placeholder_id: &str, + resolved_path: &Path, + placeholder: &PlaceholderConfig, + prepared: PreparedImport, + ) -> Result<(), String> { + if graph + .edges + .iter() + .any(|edge| edge.from == placeholder_id && edge.to == placeholder_id) + { + return Err(format!( + "import placeholder '{placeholder_id}' cannot have a self-loop" + )); + } + + let incoming_edges = graph + .incoming_edges(placeholder_id) + .into_iter() + .cloned() + .collect::>(); + let outgoing_edges = graph + .outgoing_edges(placeholder_id) + .into_iter() + .cloned() + .collect::>(); + + if prepared.is_empty() { + if incoming_edges.iter().any(Self::has_semantic_edge_attrs) + || outgoing_edges.iter().any(Self::has_semantic_edge_attrs) + { + return Err(format!( + "empty import '{placeholder_id}' cannot bypass semantic edges" + )); + } + + graph.nodes.remove(placeholder_id); + graph + .edges + .retain(|edge| edge.from != placeholder_id && edge.to != placeholder_id); + + for incoming in &incoming_edges { + for outgoing in &outgoing_edges { + graph.edges.push(Edge::new(&incoming.from, &outgoing.to)); + } + } + + tracing::debug!( + node = %placeholder_id, + path = %resolved_path.display(), + "Expanded empty imported workflow via bypass" + ); + return Ok(()); + } + + graph.nodes.remove(placeholder_id); + graph + .edges + .retain(|edge| edge.from != placeholder_id && edge.to != placeholder_id); + + let PreparedImport { + graph: imported_graph, + start_id, + exit_id, + entry_id, + exit_predecessor_id, + } = prepared; + + for (node_id, node) in imported_graph.nodes { + if node_id == start_id || node_id == exit_id { + continue; + } + + let prefixed_id = format!("{placeholder_id}.{node_id}"); + let mut merged_node = Node::new(&prefixed_id); + merged_node.attrs.clone_from(&placeholder.default_attrs); + merged_node.attrs.extend(node.attrs); + Self::remap_retry_target(&mut merged_node.attrs, placeholder_id); + + merged_node.classes = node.classes; + for class_name in &placeholder.class_names { + Self::push_class(&mut merged_node.classes, class_name); + } + if !placeholder.normalized_class.is_empty() { + Self::push_class(&mut merged_node.classes, &placeholder.normalized_class); + } + + graph.nodes.insert(prefixed_id, merged_node); + } + + for edge in imported_graph.edges { + if edge.from == start_id + || edge.to == start_id + || edge.from == exit_id + || edge.to == exit_id + { + continue; + } + + let mut merged_edge = Edge::new( + format!("{placeholder_id}.{}", edge.from), + format!("{placeholder_id}.{}", edge.to), + ); + merged_edge.attrs = edge.attrs; + graph.edges.push(merged_edge); + } + + for edge in incoming_edges { + let mut rewired = Edge::new(edge.from, format!("{placeholder_id}.{entry_id}")); + rewired.attrs = edge.attrs; + graph.edges.push(rewired); + } + + for edge in outgoing_edges { + let mut rewired = Edge::new(format!("{placeholder_id}.{exit_predecessor_id}"), edge.to); + rewired.attrs = edge.attrs; + graph.edges.push(rewired); + } + + Ok(()) + } + + fn with_import_stack( + import_stack: &mut Vec, + resolved_path: PathBuf, + f: impl FnOnce(&mut Vec) -> T, + ) -> T { + import_stack.push(resolved_path); + let result = f(import_stack); + import_stack.pop(); + result + } + + fn placeholder_config( + graph: &Graph, + placeholder_id: &str, + ) -> Result { + let node = graph + .nodes + .get(placeholder_id) + .ok_or_else(|| format!("missing import placeholder '{placeholder_id}'"))?; + let mut default_attrs = HashMap::new(); + let mut class_names = Vec::new(); + + for (key, value) in &node.attrs { + if key == "import" { + continue; + } + + if key == "class" { + if let Some(class_attr) = value.as_str() { + for class_name in class_attr.split(',') { + let class_name = class_name.trim(); + if !class_name.is_empty() + && !class_names.iter().any(|value| value == class_name) + { + class_names.push(class_name.to_string()); + } + } + } + continue; + } + + if Self::allowed_placeholder_attr(key) { + default_attrs.insert(key.clone(), value.clone()); + continue; + } + + return Err(format!( + "import placeholder '{placeholder_id}' has unsupported attribute '{key}'" + )); + } + + Ok(PlaceholderConfig { + default_attrs, + class_names, + normalized_class: Self::normalize_class_name(placeholder_id), + }) + } + + fn validate_imported_graph(graph: Graph) -> Result { + let start_ids = graph + .nodes + .iter() + .filter_map(|(id, node)| Self::is_start_sentinel(id, node).then_some(id.clone())) + .collect::>(); + if start_ids.len() != 1 { + return Err(format!( + "imported workflow must have exactly one start node, found {}", + start_ids.len() + )); + } + + let exit_ids = graph + .nodes + .iter() + .filter_map(|(id, node)| Self::is_exit_sentinel(id, node).then_some(id.clone())) + .collect::>(); + if exit_ids.len() != 1 { + return Err(format!( + "imported workflow must have exactly one exit node, found {}", + exit_ids.len() + )); + } + + let start_id = start_ids[0].clone(); + let exit_id = exit_ids[0].clone(); + + if !graph.incoming_edges(&start_id).is_empty() { + return Err(format!( + "imported start node '{start_id}' must not have incoming edges" + )); + } + if !graph.outgoing_edges(&exit_id).is_empty() { + return Err(format!( + "imported exit node '{exit_id}' must not have outgoing edges" + )); + } + + let start_edges = graph.outgoing_edges(&start_id); + if start_edges.len() != 1 { + return Err(format!( + "imported start node '{start_id}' must have exactly one successor" + )); + } + if Self::has_semantic_edge_attrs(start_edges[0]) { + return Err(format!( + "imported edge '{} -> {}' must not carry semantic attributes", + start_edges[0].from, start_edges[0].to + )); + } + let entry_id = start_edges[0].to.clone(); + + let exit_edges = graph.incoming_edges(&exit_id); + if exit_edges.len() != 1 { + return Err(format!( + "imported exit node '{exit_id}' must have exactly one predecessor" + )); + } + if Self::has_semantic_edge_attrs(exit_edges[0]) { + return Err(format!( + "imported edge '{} -> {}' must not carry semantic attributes", + exit_edges[0].from, exit_edges[0].to + )); + } + let exit_predecessor_id = exit_edges[0].from.clone(); + + Ok(PreparedImport { + graph, + start_id, + exit_id, + entry_id, + exit_predecessor_id, + }) + } + + fn unresolved_imported_prompt_error(graph: &Graph) -> Option { + for (node_id, node) in &graph.nodes { + if Self::is_start_sentinel(node_id, node) || Self::is_exit_sentinel(node_id, node) { + continue; + } + + let Some(prompt) = node.attrs.get("prompt").and_then(AttrValue::as_str) else { + continue; + }; + if prompt.starts_with('@') { + return Some(format!( + "node '{node_id}' in imported workflow has unresolved file reference: {prompt}" + )); + } + } + + None + } + + fn remap_retry_target(attrs: &mut HashMap, placeholder_id: &str) { + for attr_name in ["retry_target", "fallback_retry_target"] { + let Some(target) = attrs + .get(attr_name) + .and_then(AttrValue::as_str) + .map(str::to_string) + else { + continue; + }; + attrs.insert( + attr_name.to_string(), + AttrValue::String(format!("{placeholder_id}.{target}")), + ); + } + } + + fn poison_placeholder(graph: &mut Graph, placeholder_id: &str, message: &str) { + if let Some(node) = graph.nodes.get_mut(placeholder_id) { + node.attrs.remove("import"); + node.attrs.insert( + "import_error".to_string(), + AttrValue::String(message.to_string()), + ); + } + + tracing::warn!(node = %placeholder_id, reason = %message, "Import expansion failed"); + } + + fn allowed_placeholder_attr(key: &str) -> bool { + matches!( + key, + "model" + | "provider" + | "reasoning_effort" + | "speed" + | "backend" + | "fidelity" + | "max_retries" + | "thread_id" + ) + } + + fn has_semantic_edge_attrs(edge: &Edge) -> bool { + [ + "condition", + "label", + "weight", + "fidelity", + "thread_id", + "loop_restart", + "freeform", + ] + .into_iter() + .any(|key| edge.attrs.contains_key(key)) + } + + fn push_class(classes: &mut Vec, class_name: &str) { + if !classes.iter().any(|value| value == class_name) { + classes.push(class_name.to_string()); + } + } + + fn normalize_class_name(label: &str) -> String { + label + .to_lowercase() + .chars() + .map(|char| if char == ' ' { '-' } else { char }) + .filter(|char| char.is_ascii_alphanumeric() || *char == '-') + .collect() + } + + fn is_start_sentinel(node_id: &str, node: &Node) -> bool { + node.shape() == "Mdiamond" || matches!(node_id, "start" | "Start") + } + + fn is_exit_sentinel(node_id: &str, node: &Node) -> bool { + node.shape() == "Msquare" || matches!(node_id, "exit" | "Exit" | "end" | "End") + } +} + +impl PreparedImport { + fn is_empty(&self) -> bool { + self.graph.nodes.iter().all(|(node_id, node)| { + ImportTransform::is_start_sentinel(node_id, node) + || ImportTransform::is_exit_sentinel(node_id, node) + }) + } +} + +impl Transform for ImportTransform { + fn apply(&self, graph: &mut Graph) { + let imports = Self::collect_import_nodes(graph); + let mut import_stack = Vec::new(); + + for (placeholder_id, import_path) in imports { + self.expand_import( + graph, + &placeholder_id, + &import_path, + &self.base_dir, + &mut import_stack, + ); + } + } +} + +#[cfg(test)] +mod tests { + use std::path::Path; + + use fabro_graphviz::graph::AttrValue; + use fabro_graphviz::parser; + + use super::*; + + fn parse_graph(source: &str) -> Graph { + parser::parse(source).unwrap() + } + + fn write_file(path: &Path, contents: &str) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, contents).unwrap(); + } + + fn apply_import(dot: &str, base_dir: &Path, fallback_dir: Option<&Path>) -> Graph { + let mut graph = parse_graph(dot); + ImportTransform::new(base_dir.to_path_buf(), fallback_dir.map(Path::to_path_buf)) + .apply(&mut graph); + graph + } + + fn basic_import_source() -> &'static str { + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="Run clippy", retry_target="test", class="code"] + test [prompt="Run tests"] + exit [shape=Msquare] + start -> lint -> test -> exit + }"# + } + + #[test] + fn basic_import_replaces_placeholder_and_rewires_edges() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("validate.fabro"), basic_import_source()); + + let graph = apply_import( + r#"digraph Deploy { + start [shape=Mdiamond] + validate [import="./validate.fabro"] + deploy [prompt="Deploy"] + exit [shape=Msquare] + start -> validate -> deploy -> exit + }"#, + dir.path(), + None, + ); + + assert!(!graph.nodes.contains_key("validate")); + assert!(graph.nodes.contains_key("validate.lint")); + assert!(graph.nodes.contains_key("validate.test")); + assert!(!graph.nodes.contains_key("validate.start")); + assert!(!graph.nodes.contains_key("validate.exit")); + + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "start" && edge.to == "validate.lint") + ); + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "validate.lint" && edge.to == "validate.test") + ); + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "validate.test" && edge.to == "deploy") + ); + assert_eq!( + graph.nodes["validate.lint"] + .attrs + .get("retry_target") + .and_then(AttrValue::as_str), + Some("validate.test") + ); + assert!( + graph.nodes["validate.lint"] + .classes + .iter() + .any(|class_name| class_name == "code") + ); + assert!( + graph.nodes["validate.lint"] + .classes + .iter() + .any(|class_name| class_name == "validate") + ); + } + + #[test] + fn placeholder_class_attr_and_defaults_propagate() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("validate.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="Run clippy", model="opus"] + test [prompt="Run tests"] + exit [shape=Msquare] + start -> lint -> test -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Deploy { + start [shape=Mdiamond] + validate [import="./validate.fabro", model="haiku", class="fast, shared"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert_eq!( + graph.nodes["validate.lint"] + .attrs + .get("model") + .and_then(AttrValue::as_str), + Some("opus") + ); + assert_eq!( + graph.nodes["validate.test"] + .attrs + .get("model") + .and_then(AttrValue::as_str), + Some("haiku") + ); + assert!( + graph.nodes["validate.test"] + .classes + .iter() + .any(|class_name| class_name == "fast") + ); + assert!( + graph.nodes["validate.test"] + .classes + .iter() + .any(|class_name| class_name == "shared") + ); + assert!( + graph.nodes["validate.test"] + .classes + .iter() + .any(|class_name| class_name == "validate") + ); + } + + #[test] + fn css_class_normalization_drops_underscores() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("mod.fabro"), basic_import_source()); + + let graph = apply_import( + r#"digraph Test { + start [shape=Mdiamond] + run_tests [import="./mod.fabro"] + exit [shape=Msquare] + start -> run_tests -> exit + }"#, + dir.path(), + None, + ); + + assert!( + graph.nodes["run_tests.lint"] + .classes + .iter() + .any(|class_name| class_name == "runtests") + ); + } + + #[test] + fn multiple_entry_nodes_poison_placeholder() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("validate.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="Run clippy"] + test [prompt="Run tests"] + exit [shape=Msquare] + start -> lint + start -> test + lint -> exit + test -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Deploy { + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert!(graph.nodes.contains_key("validate")); + assert_eq!( + graph.nodes["validate"] + .attrs + .get("import_error") + .and_then(AttrValue::as_str), + Some("imported start node 'start' must have exactly one successor") + ); + } + + #[test] + fn multiple_exit_predecessors_poison_placeholder() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("validate.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="Run clippy"] + test [prompt="Run tests"] + exit [shape=Msquare] + start -> lint + lint -> exit + test -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Deploy { + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert_eq!( + graph.nodes["validate"] + .attrs + .get("import_error") + .and_then(AttrValue::as_str), + Some("imported exit node 'exit' must have exactly one predecessor") + ); + } + + #[test] + fn missing_file_poison_keeps_placeholder_edges() { + let dir = tempfile::tempdir().unwrap(); + let graph = apply_import( + r#"digraph Deploy { + start [shape=Mdiamond] + validate [import="./missing.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert_eq!( + graph.nodes["validate"] + .attrs + .get("import_error") + .and_then(AttrValue::as_str), + Some("file not found: ./missing.fabro") + ); + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "start" && edge.to == "validate") + ); + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "validate" && edge.to == "exit") + ); + } + + #[test] + fn invalid_dot_poison_keeps_placeholder() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("broken.fabro"), "digraph broken {"); + + let graph = apply_import( + r#"digraph Deploy { + start [shape=Mdiamond] + broken [import="./broken.fabro"] + exit [shape=Msquare] + start -> broken -> exit + }"#, + dir.path(), + None, + ); + + assert!(graph.nodes["broken"].attrs.contains_key("import_error")); + } + + #[test] + fn circular_import_poison_only_inner_placeholder() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("a.fabro"), + r#"digraph a { + start [shape=Mdiamond] + b [import="./b.fabro"] + exit [shape=Msquare] + start -> b -> exit + }"#, + ); + write_file( + &dir.path().join("b.fabro"), + r#"digraph b { + start [shape=Mdiamond] + a [import="./a.fabro"] + exit [shape=Msquare] + start -> a -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + outer [import="./a.fabro"] + exit [shape=Msquare] + start -> outer -> exit + }"#, + dir.path(), + None, + ); + + assert!(!graph.nodes.contains_key("outer")); + assert!(graph.nodes.contains_key("outer.b.a")); + assert!(graph.nodes["outer.b.a"].attrs.contains_key("import_error")); + } + + #[test] + fn same_file_can_be_imported_twice() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("validate.fabro"), basic_import_source()); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + left [import="./validate.fabro"] + right [import="./validate.fabro"] + exit [shape=Msquare] + start -> left -> right -> exit + }"#, + dir.path(), + None, + ); + + assert!(graph.nodes.contains_key("left.lint")); + assert!(graph.nodes.contains_key("right.lint")); + } + + #[test] + fn nested_relative_imports_resolve_from_imported_file_dir() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("sub/a.fabro"), + r#"digraph a { + start [shape=Mdiamond] + b [import="./b.fabro"] + exit [shape=Msquare] + start -> b -> exit + }"#, + ); + write_file( + &dir.path().join("sub/b.fabro"), + r#"digraph b { + start [shape=Mdiamond] + work [prompt="Nested"] + exit [shape=Msquare] + start -> work -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + outer [import="./sub/a.fabro"] + exit [shape=Msquare] + start -> outer -> exit + }"#, + dir.path(), + None, + ); + + assert!(graph.nodes.contains_key("outer.b.work")); + } + + #[test] + fn imported_file_refs_resolve_from_imported_dir() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("sub/prompt.md"), "Run from subdir"); + write_file( + &dir.path().join("sub/validate.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="@prompt.md"] + exit [shape=Msquare] + start -> lint -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + validate [import="./sub/validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert_eq!( + graph.nodes["validate.lint"] + .attrs + .get("prompt") + .and_then(AttrValue::as_str), + Some("Run from subdir") + ); + } + + #[test] + fn unresolved_imported_file_ref_poison_placeholder() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("validate.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="@missing.md"] + exit [shape=Msquare] + start -> lint -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert_eq!( + graph.nodes["validate"] + .attrs + .get("import_error") + .and_then(AttrValue::as_str), + Some("node 'lint' in imported workflow has unresolved file reference: @missing.md") + ); + } + + #[test] + fn noop_fragment_bypasses_plain_edges() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("noop.fabro"), + r#"digraph noop { + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + a [prompt="A"] + middle [import="./noop.fabro"] + b [prompt="B"] + exit [shape=Msquare] + start -> a -> middle -> b -> exit + }"#, + dir.path(), + None, + ); + + assert!(!graph.nodes.contains_key("middle")); + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "a" && edge.to == "b" && edge.attrs.is_empty()) + ); + } + + #[test] + fn noop_fragment_with_semantic_host_edge_poison_placeholder() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("noop.fabro"), + r#"digraph noop { + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + middle [import="./noop.fabro"] + exit [shape=Msquare] + start -> middle [condition="outcome=success"] + middle -> exit + }"#, + dir.path(), + None, + ); + + assert!(graph.nodes["middle"].attrs.contains_key("import_error")); + } + + #[test] + fn edge_attributes_survive_normal_rewiring() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("validate.fabro"), basic_import_source()); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate [label="go", condition="outcome=success"] + validate -> exit [thread_id="session1"] + }"#, + dir.path(), + None, + ); + + let start_edge = graph + .edges + .iter() + .find(|edge| edge.from == "start" && edge.to == "validate.lint") + .unwrap(); + assert_eq!(start_edge.label(), Some("go")); + assert_eq!(start_edge.condition(), Some("outcome=success")); + + let exit_edge = graph + .edges + .iter() + .find(|edge| edge.from == "validate.test" && edge.to == "exit") + .unwrap(); + assert_eq!(exit_edge.thread_id(), Some("session1")); + } + + #[test] + fn disallowed_placeholder_attr_poison() { + let dir = tempfile::tempdir().unwrap(); + write_file(&dir.path().join("validate.fabro"), basic_import_source()); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + validate [import="./validate.fabro", selection="random"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert_eq!( + graph.nodes["validate"] + .attrs + .get("import_error") + .and_then(AttrValue::as_str), + Some("import placeholder 'validate' has unsupported attribute 'selection'") + ); + } + + #[test] + fn missing_sentinel_poison() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("validate.fabro"), + r#"digraph validate { + lint [prompt="Run clippy"] + }"#, + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert!(graph.nodes["validate"].attrs.contains_key("import_error")); + } + + #[test] + fn sentinel_semantic_edge_poison() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("validate.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="Run clippy"] + exit [shape=Msquare] + start -> lint [condition="outcome=success"] + lint -> exit + }"#, + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + dir.path(), + None, + ); + + assert!(graph.nodes["validate"].attrs.contains_key("import_error")); + } + + #[test] + fn import_can_resolve_from_fallback_dir() { + let base = tempfile::tempdir().unwrap(); + let fallback = tempfile::tempdir().unwrap(); + write_file( + &fallback.path().join("validate.fabro"), + basic_import_source(), + ); + + let graph = apply_import( + r#"digraph Host { + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + base.path(), + Some(fallback.path()), + ); + + assert!(graph.nodes.contains_key("validate.lint")); + } +} diff --git a/lib/crates/fabro-workflows/src/transforms/mod.rs b/lib/crates/fabro-workflows/src/transforms/mod.rs index 15ae8b670..befd3b037 100644 --- a/lib/crates/fabro-workflows/src/transforms/mod.rs +++ b/lib/crates/fabro-workflows/src/transforms/mod.rs @@ -6,7 +6,7 @@ pub trait Transform { } mod file_inlining; -mod graph_merge; +mod import; mod model_resolution; mod preamble; pub mod stylesheet; @@ -14,7 +14,7 @@ mod stylesheet_application; pub mod variable_expansion; pub use file_inlining::{FileInliningTransform, resolve_file_ref}; -pub use graph_merge::GraphMergeTransform; +pub use import::ImportTransform; pub use model_resolution::ModelResolutionTransform; pub use preamble::PreambleTransform; pub use stylesheet_application::StylesheetApplicationTransform; diff --git a/lib/crates/fabro-workflows/tests/integration.rs b/lib/crates/fabro-workflows/tests/integration.rs index d9eff5e33..d021413be 100644 --- a/lib/crates/fabro-workflows/tests/integration.rs +++ b/lib/crates/fabro-workflows/tests/integration.rs @@ -3892,95 +3892,84 @@ async fn manager_loop_child_dotfile_e2e() { } // =========================================================================== -// 19c. GraphMerge E2E (TS Scenario 11) +// 19c. ImportTransform E2E (TS Scenario 11) // =========================================================================== #[tokio::test] -async fn graph_merge_e2e_through_engine() { - use fabro_workflows::transform::GraphMergeTransform; - - // Module "val": lint -> test - let mut val_graph = Graph::new("val"); - let mut lint = Node::new("lint"); - lint.attrs - .insert("shape".to_string(), AttrValue::String("box".to_string())); - lint.attrs.insert( - "prompt".to_string(), - AttrValue::String("Lint the code".to_string()), - ); - val_graph.nodes.insert("lint".to_string(), lint); - - let mut test_node = Node::new("test"); - test_node - .attrs - .insert("shape".to_string(), AttrValue::String("box".to_string())); - test_node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Run tests".to_string()), - ); - val_graph.nodes.insert("test".to_string(), test_node); - val_graph.edges.push(Edge::new("lint", "test")); - - // Module "dep": stage -> release - let mut dep_graph = Graph::new("dep"); - let mut stage = Node::new("stage"); - stage - .attrs - .insert("shape".to_string(), AttrValue::String("box".to_string())); - stage.attrs.insert( - "prompt".to_string(), - AttrValue::String("Stage the release".to_string()), - ); - dep_graph.nodes.insert("stage".to_string(), stage); - - let mut release = Node::new("release"); - release - .attrs - .insert("shape".to_string(), AttrValue::String("box".to_string())); - release.attrs.insert( - "prompt".to_string(), - AttrValue::String("Release it".to_string()), - ); - dep_graph.nodes.insert("release".to_string(), release); - dep_graph.edges.push(Edge::new("stage", "release")); - - // Main graph: start, exit; edges connect modules - let mut main_graph = Graph::new("MergeE2E"); - main_graph.attrs.insert( - "goal".to_string(), - AttrValue::String("Test graph merge".to_string()), - ); - - let mut start = Node::new("start"); - start.attrs.insert( - "shape".to_string(), - AttrValue::String("Mdiamond".to_string()), - ); - main_graph.nodes.insert("start".to_string(), start); - - let mut exit = Node::new("exit"); - exit.attrs.insert( - "shape".to_string(), - AttrValue::String("Msquare".to_string()), - ); - main_graph.nodes.insert("exit".to_string(), exit); - - // Apply merge transform - let merge = GraphMergeTransform::new(vec![val_graph, dep_graph]); - merge.apply(&mut main_graph); - - // Add cross-module edges - main_graph.edges.push(Edge::new("start", "val.lint")); - main_graph.edges.push(Edge::new("val.test", "dep.stage")); - main_graph.edges.push(Edge::new("dep.release", "exit")); - - // Verify merged nodes exist - assert!(main_graph.nodes.contains_key("val.lint")); - assert!(main_graph.nodes.contains_key("val.test")); - assert!(main_graph.nodes.contains_key("dep.stage")); - assert!(main_graph.nodes.contains_key("dep.release")); +async fn import_e2e_through_engine() { + use fabro_workflows::pipeline::{TransformOptions, transform, validate}; let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("val.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="Lint the code"] + test [prompt="Run tests"] + exit [shape=Msquare] + start -> lint -> test -> exit + }"#, + ) + .unwrap(); + std::fs::write( + dir.path().join("dep.fabro"), + r#"digraph deploy { + start [shape=Mdiamond] + stage [prompt="Stage the release"] + release [prompt="Release it"] + exit [shape=Msquare] + start -> stage -> release -> exit + }"#, + ) + .unwrap(); + + let parsed = fabro_workflows::pipeline::parse( + r#"digraph MergeE2E { + graph [goal="Test file imports"] + start [shape=Mdiamond] + validate [import="./val.fabro"] + deploy [import="./dep.fabro"] + exit [shape=Msquare] + start -> validate -> deploy -> exit + }"#, + ) + .expect("parse should succeed"); + let transformed = transform( + parsed, + &TransformOptions { + base_dir: Some(dir.path().to_path_buf()), + custom_transforms: vec![], + }, + ); + let validated = validate(transformed, &[]); + validated + .raise_on_errors() + .expect("validation should pass after imports expand"); + let (graph, _, _) = validated.into_parts(); + + assert!(graph.nodes.contains_key("validate.lint")); + assert!(graph.nodes.contains_key("validate.test")); + assert!(graph.nodes.contains_key("deploy.stage")); + assert!(graph.nodes.contains_key("deploy.release")); + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "start" && edge.to == "validate.lint") + ); + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "validate.test" && edge.to == "deploy.stage") + ); + assert!( + graph + .edges + .iter() + .any(|edge| edge.from == "deploy.release" && edge.to == "exit") + ); + let engine = WorkflowRunner::new( make_linear_registry(), Arc::new(EventEmitter::new()), @@ -4001,47 +3990,51 @@ async fn graph_merge_e2e_through_engine() { git: None, }; let outcome = engine - .run(&main_graph, &run_options) + .run(&graph, &run_options) .await - .expect("graph merge E2E should succeed"); + .expect("import E2E should succeed"); assert_eq!(outcome.status, StageStatus::Success); let checkpoint = Checkpoint::load(&dir.path().join("checkpoint.json")).unwrap(); assert!( - checkpoint.completed_nodes.contains(&"val.lint".to_string()), - "val.lint should be completed" - ); - assert!( - checkpoint.completed_nodes.contains(&"val.test".to_string()), - "val.test should be completed" + checkpoint + .completed_nodes + .contains(&"validate.lint".to_string()), + "validate.lint should be completed" ); assert!( checkpoint .completed_nodes - .contains(&"dep.stage".to_string()), - "dep.stage should be completed" + .contains(&"validate.test".to_string()), + "validate.test should be completed" ); assert!( checkpoint .completed_nodes - .contains(&"dep.release".to_string()), - "dep.release should be completed" + .contains(&"deploy.stage".to_string()), + "deploy.stage should be completed" + ); + assert!( + checkpoint + .completed_nodes + .contains(&"deploy.release".to_string()), + "deploy.release should be completed" ); - // Verify ordering: val.test appears before dep.stage + // Verify ordering: validate.test appears before deploy.stage let val_test_pos = checkpoint .completed_nodes .iter() - .position(|n| n == "val.test") - .expect("val.test should be in completed_nodes"); + .position(|n| n == "validate.test") + .expect("validate.test should be in completed_nodes"); let dep_stage_pos = checkpoint .completed_nodes .iter() - .position(|n| n == "dep.stage") - .expect("dep.stage should be in completed_nodes"); + .position(|n| n == "deploy.stage") + .expect("deploy.stage should be in completed_nodes"); assert!( val_test_pos < dep_stage_pos, - "val.test ({val_test_pos}) should execute before dep.stage ({dep_stage_pos})" + "validate.test ({val_test_pos}) should execute before deploy.stage ({dep_stage_pos})" ); }