From 52aed8c64278f7ffb661cc16020f5376aaac7d7c Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 19 Sep 2026 11:41:38 -0400 Subject: [PATCH] Read the run's display graph off Petri's admitted graph Every run is admitted by Petri, whose check lowers imports, file references, templates and the model stylesheet, lints the workflow and pins its models. Fabro then re-parsed the same workflow through its own legacy pipeline (parse, transforms, structural validation) only to fill `RunSpec.graph` for the read side. That second pass is gone: the run's display graph is `fabro_types::RunGraph`, built once in `fabro-petri` from the admitted graph's metadata (the workflow name and goal from the graph params; each declared stage's label and handler kind; one edge per routing arm as written, lowering artifacts left out), and stored on the spec at create beside the DOT as `graph_source`. Deleted: `fabro-workflow`'s `pipeline`, `transforms`, `file_resolver`, `operations::{source, validate}`, `run_materialization` and the legacy `compile_admitted_run`; the server's `compile_admitted`, the structural manifest pass, `preflight_model` and the model probe `run_llm_check` (Petri's admission raises `attractor.model.unknown`); `fabro-graphviz`'s stylesheet parser; most of `fabro_types::graph` (the DOT model keeps what the bundler, version registration and template walker read). The DOT parser stays for the bundler and the SVG render. `POST /validate`, `POST /preflight`, `POST /graph/render`, `fabro validate` and `fabro preflight` run on Petri's check alone, so their diagnostics carry Petri's codes (`attractor.unbound_input`, `unsupported.template.unbound_input`) where Fabro's `template_undefined_variable` and `goal_self_reference` were. A refused workflow's summary still names the DOT as written. The OpenAPI `RunSpec` schema gains `RunGraph`, `RunGraphNode` and `RunGraphEdge`, reused from `fabro-types` with parity tests; the TS client is regenerated. Co-Authored-By: Claude Fable 5.1 --- .../tests/manifest_path_round_trip.rs | 65 - .../fabro-graphviz/src/stylesheet.rs | 342 --- .../fabro-workflow/src/file_resolver.rs | 289 --- .../fabro-workflow/src/operations/source.rs | 196 -- .../fabro-workflow/src/operations/validate.rs | 60 - .../fabro-workflow/src/pipeline/mod.rs | 13 - .../fabro-workflow/src/pipeline/parse.rs | 43 - .../fabro-workflow/src/pipeline/persist.rs | 185 -- .../fabro-workflow/src/pipeline/transform.rs | 628 ------ .../fabro-workflow/src/pipeline/types.rs | 221 -- .../fabro-workflow/src/pipeline/validate.rs | 97 - .../fabro-workflow/src/run_materialization.rs | 24 - .../src/transforms/file_inlining.rs | 721 ------- .../fabro-workflow/src/transforms/import.rs | 1877 ----------------- .../src/transforms/importable_field.rs | 131 -- .../fabro-workflow/src/transforms/mod.rs | 23 - .../transforms/model_stylesheet_template.rs | 225 -- .../src/transforms/stylesheet.rs | 258 --- .../src/transforms/stylesheet_application.rs | 41 - .../src/transforms/variable_expansion.rs | 1416 ------------- 20 files changed, 6855 deletions(-) delete mode 100644 lib/apps/fabro-cli/tests/manifest_path_round_trip.rs delete mode 100644 lib/components/fabro-graphviz/src/stylesheet.rs delete mode 100644 lib/components/fabro-workflow/src/file_resolver.rs delete mode 100644 lib/components/fabro-workflow/src/operations/source.rs delete mode 100644 lib/components/fabro-workflow/src/operations/validate.rs delete mode 100644 lib/components/fabro-workflow/src/pipeline/mod.rs delete mode 100644 lib/components/fabro-workflow/src/pipeline/parse.rs delete mode 100644 lib/components/fabro-workflow/src/pipeline/persist.rs delete mode 100644 lib/components/fabro-workflow/src/pipeline/transform.rs delete mode 100644 lib/components/fabro-workflow/src/pipeline/types.rs delete mode 100644 lib/components/fabro-workflow/src/pipeline/validate.rs delete mode 100644 lib/components/fabro-workflow/src/run_materialization.rs delete mode 100644 lib/components/fabro-workflow/src/transforms/file_inlining.rs delete mode 100644 lib/components/fabro-workflow/src/transforms/import.rs delete mode 100644 lib/components/fabro-workflow/src/transforms/importable_field.rs delete mode 100644 lib/components/fabro-workflow/src/transforms/mod.rs delete mode 100644 lib/components/fabro-workflow/src/transforms/model_stylesheet_template.rs delete mode 100644 lib/components/fabro-workflow/src/transforms/stylesheet.rs delete mode 100644 lib/components/fabro-workflow/src/transforms/stylesheet_application.rs delete mode 100644 lib/components/fabro-workflow/src/transforms/variable_expansion.rs diff --git a/lib/apps/fabro-cli/tests/manifest_path_round_trip.rs b/lib/apps/fabro-cli/tests/manifest_path_round_trip.rs deleted file mode 100644 index a85fafb7d..000000000 --- a/lib/apps/fabro-cli/tests/manifest_path_round_trip.rs +++ /dev/null @@ -1,65 +0,0 @@ -#![expect( - clippy::disallowed_methods, - reason = "Sync temp fixture writes keep this manifest round-trip test simple and isolated." -)] - -use std::path::PathBuf; - -use fabro_config::{EnvironmentLayer, MergeMap}; -use fabro_manifest::{ManifestBuildInput, build_run_manifest}; -use fabro_types::ManifestPath; - -fn test_environment_defaults() -> MergeMap { - MergeMap::from(std::collections::HashMap::from([( - "default".to_string(), - EnvironmentLayer { - provider: Some("local".to_string()), - ..EnvironmentLayer::default() - }, - )])) -} - -#[test] -fn cli_built_manifest_resolves_user_global_at_path() { - let temp = tempfile::tempdir().unwrap(); - let workflow_dir = temp.path().join(".fabro/workflows/demo"); - let project = temp.path().join("project"); - std::fs::create_dir_all(workflow_dir.join("prompts")).unwrap(); - std::fs::create_dir_all(&project).unwrap(); - std::fs::write( - workflow_dir.join("workflow.fabro"), - r#"digraph Demo { - graph [goal="Demo"] - start [shape=Mdiamond] - prompt [prompt="@prompts/hello.md"] - exit [shape=Msquare] - start -> prompt -> exit - }"#, - ) - .unwrap(); - std::fs::write(workflow_dir.join("prompts/hello.md"), "hello from bundle").unwrap(); - - let built = build_run_manifest(ManifestBuildInput { - workflow: workflow_dir.join("workflow.fabro"), - cwd: project, - environment_defaults: test_environment_defaults(), - ..Default::default() - }) - .unwrap(); - - let bundle = fabro_server::workflow_bundle_from_manifest(&built.manifest.workflows).unwrap(); - let target_path = ManifestPath::from_wire(&built.manifest.target.path).unwrap(); - let workflow = bundle - .workflow(&target_path) - .expect("root workflow should be present"); - let resolved = workflow - .file_resolver() - .resolve(&workflow.current_dir(), "prompts/hello.md") - .expect("prompt should resolve from bundle"); - - assert_eq!(resolved.content, "hello from bundle"); - assert_eq!( - resolved.path, - PathBuf::from("../.fabro/workflows/demo/prompts/hello.md") - ); -} diff --git a/lib/components/fabro-graphviz/src/stylesheet.rs b/lib/components/fabro-graphviz/src/stylesheet.rs deleted file mode 100644 index 874a6106d..000000000 --- a/lib/components/fabro-graphviz/src/stylesheet.rs +++ /dev/null @@ -1,342 +0,0 @@ -use crate::error::Error; - -/// A parsed stylesheet selector. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Selector { - /// `*` -- matches all nodes, specificity 0. - Universal, - /// Bare word -- matches nodes by shape name, specificity 1. - Shape(String), - /// `.classname` -- matches nodes with that class, specificity 2. - Class(String), - /// `#nodeid` -- matches a specific node, specificity 3. - Id(String), -} - -impl Selector { - #[must_use] - pub const fn specificity(&self) -> u8 { - match self { - Self::Universal => 0, - Self::Shape(_) => 1, - Self::Class(_) => 2, - Self::Id(_) => 3, - } - } -} - -/// A single CSS-like declaration: `property: value`. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Declaration { - pub property: String, - pub value: String, -} - -/// A stylesheet rule: selector + declarations. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Rule { - pub selector: Selector, - pub declarations: Vec, -} - -/// A parsed stylesheet containing multiple rules. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Stylesheet { - pub rules: Vec, -} - -/// Parse a stylesheet string into a `Stylesheet`. -/// -/// # Errors -/// -/// Returns an error if the input contains invalid stylesheet syntax. -pub fn parse_stylesheet(input: &str) -> Result { - let input = strip_css_comments(input)?; - let input = input.trim(); - if input.is_empty() { - return Ok(Stylesheet { rules: Vec::new() }); - } - - let mut rules = Vec::new(); - let mut remaining = input; - - while !remaining.trim().is_empty() { - remaining = remaining.trim(); - - let selector = parse_selector(&mut remaining)?; - if !remaining.starts_with('{') { - return Err(Error::Stylesheet(format!( - "expected '{{' after selector, got: {:?}", - excerpt(remaining) - ))); - } - remaining = remaining[1..].trim(); - - let declarations = parse_declarations(&mut remaining)?; - remaining = remaining[1..].trim(); // skip '}' - - rules.push(Rule { - selector, - declarations, - }); - } - - Ok(Stylesheet { rules }) -} - -fn strip_css_comments(input: &str) -> Result { - let mut output = String::with_capacity(input.len()); - let mut remaining = input; - - while let Some(start) = remaining.find("/*") { - let body = &remaining[start + 2..]; - let Some(end) = body.find("*/") else { - return Err(Error::Stylesheet(format!( - "unterminated CSS comment: {:?}", - excerpt(&remaining[start..]) - ))); - }; - - output.push_str(&remaining[..start]); - // CSS comments do not join the tokens on either side. Preserve - // that boundary for this parser with one whitespace character. - output.push(' '); - remaining = &body[end + 2..]; - } - - output.push_str(remaining); - Ok(output) -} - -/// A short excerpt of `input` for error messages, cut on a character boundary. -fn excerpt(input: &str) -> String { - input.chars().take(20).collect() -} - -fn parse_selector(remaining: &mut &str) -> Result { - if remaining.starts_with('*') { - *remaining = remaining[1..].trim(); - Ok(Selector::Universal) - } else if remaining.starts_with('#') { - *remaining = remaining[1..].trim(); - let end = remaining - .find(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-') - .unwrap_or(remaining.len()); - if end == 0 { - return Err(Error::Stylesheet("expected identifier after '#'".into())); - } - let id = remaining[..end].to_string(); - *remaining = remaining[end..].trim(); - Ok(Selector::Id(id)) - } else if remaining.starts_with('.') { - *remaining = remaining[1..].trim(); - let end = remaining - .find(|c: char| !c.is_ascii_lowercase() && !c.is_ascii_digit() && c != '-') - .unwrap_or(remaining.len()); - if end == 0 { - return Err(Error::Stylesheet("expected class name after '.'".into())); - } - let class = remaining[..end].to_string(); - *remaining = remaining[end..].trim(); - Ok(Selector::Class(class)) - } else { - // Bare word: shape selector - let end = remaining - .find(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-') - .unwrap_or(remaining.len()); - if end == 0 { - return Err(Error::Stylesheet(format!( - "expected selector ('*', '#id', '.class', or shape name), got: {:?}", - excerpt(remaining) - ))); - } - let shape = remaining[..end].to_string(); - *remaining = remaining[end..].trim(); - Ok(Selector::Shape(shape)) - } -} - -fn parse_declarations(remaining: &mut &str) -> Result, Error> { - let mut declarations = Vec::new(); - while !remaining.starts_with('}') { - if remaining.is_empty() { - return Err(Error::Stylesheet( - "unexpected end of stylesheet, expected '}'".into(), - )); - } - if remaining.starts_with(';') { - *remaining = remaining[1..].trim(); - continue; - } - - let prop_end = remaining - .find(|c: char| c == ':' || c.is_whitespace()) - .unwrap_or(remaining.len()); - let property = remaining[..prop_end].to_string(); - *remaining = remaining[prop_end..].trim(); - - if !remaining.starts_with(':') { - return Err(Error::Stylesheet(format!( - "expected ':' after property name '{property}'" - ))); - } - *remaining = remaining[1..].trim(); - - let val_end = remaining.find([';', '}']).unwrap_or(remaining.len()); - let value = remaining[..val_end].trim().to_string(); - *remaining = remaining[val_end..].trim(); - - if value.is_empty() { - return Err(Error::Stylesheet(format!( - "empty value for property '{property}'" - ))); - } - - declarations.push(Declaration { property, value }); - - if remaining.starts_with(';') { - *remaining = remaining[1..].trim(); - } - } - Ok(declarations) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn parse_empty_stylesheet() { - let ss = parse_stylesheet("").unwrap(); - assert!(ss.rules.is_empty()); - } - - #[test] - fn parse_universal_rule() { - let ss = parse_stylesheet("* { model: claude-sonnet-4-5; provider: anthropic; }").unwrap(); - assert_eq!(ss.rules.len(), 1); - assert_eq!(ss.rules[0].selector, Selector::Universal); - assert_eq!(ss.rules[0].declarations.len(), 2); - assert_eq!(ss.rules[0].declarations[0].property, "model"); - assert_eq!(ss.rules[0].declarations[0].value, "claude-sonnet-4-5"); - } - - #[test] - fn parse_class_rule() { - let ss = parse_stylesheet(".code { model: claude-opus-4-6; }").unwrap(); - assert_eq!(ss.rules[0].selector, Selector::Class("code".into())); - } - - #[test] - fn parse_id_rule() { - let ss = parse_stylesheet("#critical_review { model: gpt-5.2; reasoning_effort: high; }") - .unwrap(); - assert_eq!(ss.rules[0].selector, Selector::Id("critical_review".into())); - assert_eq!(ss.rules[0].declarations.len(), 2); - } - - #[test] - fn parse_multiple_rules() { - let input = r" - * { model: claude-sonnet-4-5; provider: anthropic; } - .code { model: claude-opus-4-6; provider: anthropic; } - #critical_review { model: gpt-5.2; provider: openai; reasoning_effort: high; } - "; - let ss = parse_stylesheet(input).unwrap(); - assert_eq!(ss.rules.len(), 3); - } - - #[test] - fn parse_css_comments_between_tokens() { - let input = r" - /* Defaults apply to every node. */ - */* selector */{/* before property */ - model/* before colon */:/* before value */claude-sonnet-4-5/* after value */; - /* before closing brace */} - /* Use Opus for coding nodes. */ - .code { model: claude-opus-4-6; } - "; - - let ss = parse_stylesheet(input).unwrap(); - assert_eq!(ss.rules.len(), 2); - assert_eq!(ss.rules[0].selector, Selector::Universal); - assert_eq!(ss.rules[0].declarations[0].property, "model"); - assert_eq!(ss.rules[0].declarations[0].value, "claude-sonnet-4-5"); - assert_eq!(ss.rules[1].selector, Selector::Class("code".into())); - } - - #[test] - fn parse_comment_only_stylesheet() { - let ss = parse_stylesheet("/* no rules */").unwrap(); - assert!(ss.rules.is_empty()); - } - - #[test] - fn comments_do_not_join_tokens() { - let result = parse_stylesheet("* { mo/**/del: sonnet; }"); - assert!(result.is_err()); - } - - #[test] - fn comments_close_at_first_terminator() { - let ss = parse_stylesheet("/* outer /* inner */ * { model: sonnet; }").unwrap(); - assert_eq!(ss.rules.len(), 1); - assert_eq!(ss.rules[0].selector, Selector::Universal); - } - - #[test] - fn parse_error_unterminated_comment() { - let error = parse_stylesheet("/* no terminator").unwrap_err(); - assert_eq!( - error.to_string(), - r#"Stylesheet error: unterminated CSS comment: "/* no terminator""# - ); - } - - #[test] - fn parse_error_line_comment() { - let error = parse_stylesheet("// not a CSS comment\n* { model: sonnet; }").unwrap_err(); - assert!( - error.to_string().contains("expected selector"), - "`//` should be reported as a bad selector, got: {error}" - ); - } - - #[test] - fn parse_error_excerpt_splits_on_character_boundary() { - // A multi-byte character straddling the excerpt cutoff must not panic. - let error = parse_stylesheet("/* ünterminated cömment, well over 20 bytes").unwrap_err(); - assert_eq!( - error.to_string(), - r#"Stylesheet error: unterminated CSS comment: "/* ünterminated cömm""# - ); - } - - #[test] - fn parse_error_missing_brace() { - let result = parse_stylesheet("* model: test; }"); - assert!(result.is_err()); - } - - #[test] - fn parse_error_missing_selector() { - let result = parse_stylesheet("{ model: test; }"); - assert!(result.is_err()); - } - - #[test] - fn parse_shape_selector() { - let ss = parse_stylesheet("box { model: opus; }").unwrap(); - assert_eq!(ss.rules.len(), 1); - assert_eq!(ss.rules[0].selector, Selector::Shape("box".into())); - assert_eq!(ss.rules[0].declarations[0].value, "opus"); - } - - #[test] - fn selector_specificity_values() { - assert_eq!(Selector::Universal.specificity(), 0); - assert_eq!(Selector::Shape("box".into()).specificity(), 1); - assert_eq!(Selector::Class("x".into()).specificity(), 2); - assert_eq!(Selector::Id("x".into()).specificity(), 3); - } -} diff --git a/lib/components/fabro-workflow/src/file_resolver.rs b/lib/components/fabro-workflow/src/file_resolver.rs deleted file mode 100644 index 53485addf..000000000 --- a/lib/components/fabro-workflow/src/file_resolver.rs +++ /dev/null @@ -1,289 +0,0 @@ -#![expect( - clippy::disallowed_methods, - reason = "sync workflow file resolver invoked at stage setup; not on a Tokio hot path" -)] - -use std::collections::HashMap; -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use fabro_template::{TemplateIncludeResolver, TemplateLoadError, TemplateSource, TemplateStore}; -use fabro_types::ManifestPath; - -pub trait FileResolver: Send + Sync { - fn resolve(&self, current_dir: &Path, reference: &str) -> Option; -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct ResolvedFile { - pub path: PathBuf, - pub content: String, -} - -#[derive(Clone)] -pub struct FileResolverTemplateStore { - base_dir: PathBuf, - resolver: Arc, -} - -impl FileResolverTemplateStore { - #[must_use] - pub fn new(base_dir: PathBuf, resolver: Arc) -> Self { - Self { base_dir, resolver } - } -} - -impl TemplateStore for FileResolverTemplateStore { - fn load( - &self, - parent: &TemplateSource, - reference: &str, - ) -> Result, TemplateLoadError> { - let path = - TemplateIncludeResolver::new(parent.root.clone()).resolve(&parent.path, reference)?; - Ok(self - .resolver - .resolve(&self.base_dir, &path.to_string()) - .map(|resolved| TemplateSource::new(path, parent.root.clone(), resolved.content))) - } -} - -#[derive(Clone, Debug, Default)] -pub struct BundleFileResolver { - files: HashMap, -} - -impl BundleFileResolver { - #[must_use] - pub fn new(files: HashMap) -> Self { - Self { files } - } -} - -impl FileResolver for BundleFileResolver { - fn resolve(&self, current_dir: &Path, reference: &str) -> Option { - let path = ManifestPath::from_reference(current_dir, reference)?; - let content = self.files.get(&path)?.clone(); - Some(ResolvedFile { - path: path.into(), - content, - }) - } -} - -#[derive(Clone, Debug, Default)] -pub struct FilesystemFileResolver { - fallback_dir: Option, -} - -impl FilesystemFileResolver { - #[must_use] - pub fn new(fallback_dir: Option) -> Self { - Self { fallback_dir } - } -} - -impl FileResolver for FilesystemFileResolver { - fn resolve(&self, current_dir: &Path, reference: &str) -> Option { - let raw = Path::new(reference); - let is_tilde = reference.starts_with('~'); - let expanded = if is_tilde { - match dirs::home_dir() { - Some(home) => home.join(raw.strip_prefix("~").unwrap_or_else(|_| Path::new(""))), - None => current_dir.join(reference), - } - } else { - current_dir.join(reference) - }; - - let resolved_path = match expanded.canonicalize() { - Ok(path) if path.is_file() => Some(path), - _ if !is_tilde => self.fallback_dir.as_ref().and_then(|fallback_dir| { - let fallback_path = fallback_dir.join(reference); - match fallback_path.canonicalize() { - Ok(path) if path.is_file() => Some(path), - _ => None, - } - }), - _ => None, - }?; - - match std::fs::read_to_string(&resolved_path) { - Ok(content) => Some(ResolvedFile { - path: resolved_path, - content, - }), - Err(error) => { - tracing::warn!( - path = %resolved_path.display(), - %error, - "Failed to read file reference" - ); - None - } - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - fn manifest_path(value: &str) -> ManifestPath { - ManifestPath::from_wire(value).expect("path should parse") - } - - #[test] - fn bundle_resolver_returns_exact_match() { - let resolver = BundleFileResolver::new(HashMap::from([( - manifest_path("prompts/review.md"), - "check it".to_string(), - )])); - - let resolved = resolver - .resolve(Path::new("."), "prompts/review.md") - .expect("file should resolve"); - - assert_eq!(resolved.path, PathBuf::from("prompts/review.md")); - assert_eq!(resolved.content, "check it"); - } - - #[test] - fn bundle_resolver_normalizes_relative_segments() { - let resolver = BundleFileResolver::new(HashMap::from([( - manifest_path("prompts/review.md"), - "check it".to_string(), - )])); - - let resolved = resolver - .resolve(Path::new("subflows"), "../prompts/review.md") - .expect("file should resolve"); - - assert_eq!(resolved.path, PathBuf::from("prompts/review.md")); - } - - #[test] - fn bundle_resolver_returns_none_for_missing_path() { - let resolver = BundleFileResolver::new(HashMap::new()); - assert!(resolver.resolve(Path::new("."), "missing.md").is_none()); - } - - #[test] - fn bundle_resolver_resolves_outside_cwd_paths() { - let resolver = BundleFileResolver::new(HashMap::from([( - manifest_path("../.fabro/workflows/demo/prompts/hello.md"), - "prompt content".to_string(), - )])); - - let resolved = resolver - .resolve(Path::new("../.fabro/workflows/demo"), "prompts/hello.md") - .expect("file should resolve for out-of-CWD workflow"); - - assert_eq!(resolved.content, "prompt content"); - } - - #[test] - fn filesystem_resolver_reads_existing_file() { - let dir = tempfile::tempdir().unwrap(); - std::fs::write(dir.path().join("prompt.md"), "inlined content").unwrap(); - - let resolved = FilesystemFileResolver::new(None) - .resolve(dir.path(), "prompt.md") - .expect("file should resolve"); - - assert_eq!(resolved.content, "inlined content"); - } - - #[test] - fn filesystem_resolver_returns_none_for_missing_file() { - let dir = tempfile::tempdir().unwrap(); - - assert!( - FilesystemFileResolver::new(None) - .resolve(dir.path(), "nonexistent.md") - .is_none() - ); - } - - #[test] - fn filesystem_resolver_expands_tilde() { - let home = dirs::home_dir().expect("home dir must exist"); - let test_file = home.join(".fabro_test_tilde_tmp"); - std::fs::write(&test_file, "tilde content").unwrap(); - let _cleanup = scopeguard::guard((), |()| { - let _ = std::fs::remove_file(&test_file); - }); - - let dir = tempfile::tempdir().unwrap(); - let resolved = FilesystemFileResolver::new(None) - .resolve(dir.path(), "~/.fabro_test_tilde_tmp") - .expect("tilde path should resolve"); - - assert_eq!(resolved.content, "tilde content"); - } - - #[test] - fn filesystem_resolver_resolves_dotdot() { - let dir = tempfile::tempdir().unwrap(); - std::fs::write(dir.path().join("file.md"), "dotdot content").unwrap(); - std::fs::create_dir(dir.path().join("subdir")).unwrap(); - - let resolved = FilesystemFileResolver::new(None) - .resolve(dir.path(), "subdir/../file.md") - .expect("dotdot path should resolve"); - - assert_eq!(resolved.content, "dotdot content"); - } - - #[test] - fn filesystem_resolver_falls_back_to_fallback_dir() { - let base = tempfile::tempdir().unwrap(); - let fallback = tempfile::tempdir().unwrap(); - std::fs::write(fallback.path().join("shared.md"), "shared content").unwrap(); - - let resolved = FilesystemFileResolver::new(Some(fallback.path().to_path_buf())) - .resolve(base.path(), "shared.md") - .expect("file should resolve from the fallback dir"); - - assert_eq!(resolved.content, "shared content"); - } - - #[test] - fn filesystem_resolver_base_dir_takes_precedence_over_fallback() { - let base = tempfile::tempdir().unwrap(); - let fallback = tempfile::tempdir().unwrap(); - std::fs::write(base.path().join("prompt.md"), "base content").unwrap(); - std::fs::write(fallback.path().join("prompt.md"), "fallback content").unwrap(); - - let resolved = FilesystemFileResolver::new(Some(fallback.path().to_path_buf())) - .resolve(base.path(), "prompt.md") - .expect("file should resolve from the base dir"); - - assert_eq!(resolved.content, "base content"); - } - - #[test] - fn filesystem_resolver_no_fallback_for_tilde_path() { - let base = tempfile::tempdir().unwrap(); - let fallback = tempfile::tempdir().unwrap(); - std::fs::write(fallback.path().join("file.md"), "fallback").unwrap(); - - // A tilde path to a nonexistent file does not fall back to the fallback dir. - assert!( - FilesystemFileResolver::new(Some(fallback.path().to_path_buf())) - .resolve(base.path(), "~/nonexistent_fabro_test.md") - .is_none() - ); - } - - #[test] - fn filesystem_resolver_returns_none_without_fallback() { - let base = tempfile::tempdir().unwrap(); - - assert!( - FilesystemFileResolver::new(None) - .resolve(base.path(), "missing.md") - .is_none() - ); - } -} diff --git a/lib/components/fabro-workflow/src/operations/source.rs b/lib/components/fabro-workflow/src/operations/source.rs deleted file mode 100644 index 195566b7a..000000000 --- a/lib/components/fabro-workflow/src/operations/source.rs +++ /dev/null @@ -1,196 +0,0 @@ -#![expect( - clippy::disallowed_methods, - reason = "sync workflow operation loader; runs at workflow-load time" -)] - -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use anyhow::Context; -use fabro_config::project::{ - WorkflowLocation, resolve_working_directory_from_run, workflow_slug_from_path, -}; -use fabro_config::run::resolve_run_goal_from_namespace; -use fabro_types::WorkflowSettings; - -use crate::file_resolver::{FileResolver, FilesystemFileResolver}; -use crate::workflow_bundle::BundledWorkflow; - -#[derive(Clone, Debug)] -pub enum WorkflowInput { - Path(PathBuf), - DotSource { - source: String, - base_dir: Option, - }, - Bundled(BundledWorkflow), -} - -#[derive(Clone, Debug)] -pub(crate) struct ResolveWorkflowInput { - pub workflow: WorkflowInput, - pub settings: WorkflowSettings, - pub cwd: PathBuf, -} - -#[derive(Clone)] -pub(crate) struct ResolvedWorkflow { - pub raw_source: String, - pub settings: WorkflowSettings, - pub workflow_slug: Option, - pub dot_path: Option, - pub current_dir: Option, - pub file_resolver: Option>, - pub goal_override: Option, - pub working_directory: PathBuf, -} - -pub(crate) fn resolve_workflow(request: ResolveWorkflowInput) -> anyhow::Result { - match request.workflow { - WorkflowInput::Path(workflow_path) => { - let location = WorkflowLocation::resolve(&workflow_path, &request.cwd)?; - let settings = request.settings; - let raw_source = std::fs::read_to_string(&location.graph) - .with_context(|| format!("Failed to read {}", location.graph.display()))?; - let working_directory = resolve_working_directory_from_run(&settings.run, &request.cwd); - let goal_override = resolve_goal_override(&settings, &working_directory)?; - - Ok(ResolvedWorkflow { - raw_source, - settings, - workflow_slug: location.slug, - dot_path: Some(location.graph), - current_dir: Some(location.dir), - file_resolver: Some(Arc::new(FilesystemFileResolver::new(Some( - fabro_util::Home::from_env().root().to_path_buf(), - )))), - goal_override, - working_directory, - }) - } - WorkflowInput::DotSource { source, base_dir } => { - let settings = request.settings; - let working_directory = resolve_working_directory_from_run(&settings.run, &request.cwd); - let goal_override = resolve_goal_override(&settings, &working_directory)?; - let has_base_dir = base_dir.is_some(); - Ok(ResolvedWorkflow { - raw_source: source, - settings, - workflow_slug: None, - dot_path: None, - current_dir: base_dir, - file_resolver: has_base_dir.then(|| { - Arc::new(FilesystemFileResolver::new(Some( - fabro_util::Home::from_env().root().to_path_buf(), - ))) as Arc - }), - goal_override, - working_directory, - }) - } - WorkflowInput::Bundled(workflow) => { - let settings = request.settings; - let working_directory = resolve_working_directory_from_run(&settings.run, &request.cwd); - let goal_override = resolve_goal_override(&settings, &working_directory)?; - - Ok(ResolvedWorkflow { - raw_source: workflow.source.clone(), - settings, - workflow_slug: workflow_slug_from_path(workflow.path.as_path()), - dot_path: Some(workflow.path.as_path().to_path_buf()), - current_dir: Some(workflow.current_dir()), - file_resolver: Some(workflow.file_resolver()), - goal_override, - working_directory, - }) - } - } -} - -/// Resolve the `run.goal` override for a direct (non-manifest) workflow -/// run. Reads the file from disk if the goal layer is the `file` variant. -/// Relative paths that survived config load are anchored at -/// `working_directory`. -fn resolve_goal_override( - settings: &WorkflowSettings, - working_directory: &Path, -) -> anyhow::Result> { - resolve_run_goal_from_namespace(&settings.run, working_directory) - .map(|opt| opt.map(|resolved| resolved.text)) - .map_err(anyhow::Error::from) -} - -#[cfg(test)] -mod tests { - use fabro_types::settings::InterpString; - - use super::*; - - #[test] - fn resolve_workflow_uses_explicit_cwd_for_relative_work_dir() { - use fabro_types::settings::run::RunNamespace; - - let dir = tempfile::tempdir().unwrap(); - let resolved = resolve_workflow(ResolveWorkflowInput { - workflow: WorkflowInput::DotSource { - source: "digraph Test { start -> exit }".to_string(), - base_dir: None, - }, - settings: WorkflowSettings { - run: RunNamespace { - working_dir: Some("workspace".to_string()), - ..RunNamespace::default() - }, - ..WorkflowSettings::default() - }, - cwd: dir.path().to_path_buf(), - }) - .unwrap(); - - assert_eq!(resolved.working_directory, dir.path().join("workspace")); - } - - #[test] - fn resolve_workflow_reads_goal_override_from_dense_run_settings() { - use fabro_types::settings::run::{RunGoal, RunNamespace}; - - let dir = tempfile::tempdir().unwrap(); - let goal_path = dir.path().join("goal.md"); - std::fs::write(&goal_path, "dense goal").unwrap(); - let resolved = resolve_workflow(ResolveWorkflowInput { - workflow: WorkflowInput::DotSource { - source: "digraph Test { start -> exit }".to_string(), - base_dir: None, - }, - settings: WorkflowSettings { - run: RunNamespace { - goal: Some(RunGoal::File(InterpString::parse( - &goal_path.display().to_string(), - ))), - ..RunNamespace::default() - }, - ..WorkflowSettings::default() - }, - cwd: dir.path().to_path_buf(), - }) - .unwrap(); - - assert_eq!(resolved.goal_override.as_deref(), Some("dense goal")); - } - - #[test] - fn resolve_workflow_uses_dense_settings_without_re_resolution() { - let dir = tempfile::tempdir().unwrap(); - let resolved = resolve_workflow(ResolveWorkflowInput { - workflow: WorkflowInput::DotSource { - source: "digraph Test { start -> exit }".to_string(), - base_dir: None, - }, - settings: WorkflowSettings::default(), - cwd: dir.path().to_path_buf(), - }) - .unwrap(); - - assert_eq!(resolved.settings, WorkflowSettings::default()); - } -} diff --git a/lib/components/fabro-workflow/src/operations/validate.rs b/lib/components/fabro-workflow/src/operations/validate.rs deleted file mode 100644 index 7cb55aef9..000000000 --- a/lib/components/fabro-workflow/src/operations/validate.rs +++ /dev/null @@ -1,60 +0,0 @@ -use std::collections::HashMap; -use std::path::PathBuf; - -use fabro_types::WorkflowSettings; - -use super::create::{preprocess_and_validate, template_context}; -use super::source::{ResolveWorkflowInput, WorkflowInput, resolve_workflow}; -use crate::error::Error; -use crate::operations::RenderMode; -use crate::pipeline::{TransformOptions, Validated}; -use crate::transforms::Transform; - -pub struct ValidateInput { - pub workflow: WorkflowInput, - pub settings: WorkflowSettings, - /// Run-scoped variables (`{{ vars.* }}`) available to prompts and goals. - /// Empty for offline/CLI validation. - pub vars: HashMap, - pub cwd: PathBuf, - pub custom_transforms: Vec>, -} - -/// Parse and transform a DOT source string: the structural validation Fabro -/// does itself. Its diagnostics are the transforms' (an unbound template -/// variable, a missing file); the workflow's rules and its models are -/// Petri's to judge at admission. -/// -/// Returns `Validated` even when validation produced errors. Call -/// `validated.raise_on_errors()` if the caller wants to fail fast. -pub fn validate(input: ValidateInput) -> Result { - let ValidateInput { - workflow, - settings, - vars, - cwd, - custom_transforms, - } = input; - let resolved = resolve_workflow(ResolveWorkflowInput { - workflow, - settings, - cwd, - }) - .map_err(|err| Error::Parse(err.to_string()))?; - - preprocess_and_validate( - &resolved.raw_source, - resolved.goal_override.as_deref(), - &TransformOptions { - current_dir: resolved.current_dir, - file_resolver: resolved.file_resolver, - template_context: template_context(Some(&resolved.settings), vars), - source_name: resolved - .dot_path - .as_ref() - .map(|path| path.display().to_string()), - render_mode: RenderMode::Structural, - custom_transforms, - }, - ) -} diff --git a/lib/components/fabro-workflow/src/pipeline/mod.rs b/lib/components/fabro-workflow/src/pipeline/mod.rs deleted file mode 100644 index 15180842b..000000000 --- a/lib/components/fabro-workflow/src/pipeline/mod.rs +++ /dev/null @@ -1,13 +0,0 @@ -mod parse; -mod persist; -mod transform; -pub(crate) mod types; -mod validate; - -pub use parse::parse; -pub(crate) use persist::persist; -pub use transform::transform; -pub use types::{ - Parsed, Persisted, TEMPLATE_UNDEFINED_VARIABLE_RULE, TransformOptions, Transformed, Validated, -}; -pub use validate::validate; diff --git a/lib/components/fabro-workflow/src/pipeline/parse.rs b/lib/components/fabro-workflow/src/pipeline/parse.rs deleted file mode 100644 index 95a8665b0..000000000 --- a/lib/components/fabro-workflow/src/pipeline/parse.rs +++ /dev/null @@ -1,43 +0,0 @@ -use fabro_graphviz::parser; - -use super::types::Parsed; -use crate::error::Error; - -/// PARSE phase: parse DOT source into a `Parsed` graph. -/// -/// # Errors -/// -/// Returns `Error::Parse` if the DOT source is invalid. -pub fn parse(dot_source: &str) -> Result { - let graph = parser::parse(dot_source)?; - Ok(Parsed { - graph, - source: dot_source.to_string(), - }) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn parse_minimal_dot() { - let dot = r#"digraph Test { - graph [goal="Build feature"] - start [shape=Mdiamond] - exit [shape=Msquare] - start -> exit - }"#; - let parsed = parse(dot).unwrap(); - assert_eq!(parsed.graph.name, "Test"); - assert!(parsed.graph.find_start_node().is_some()); - assert!(parsed.graph.find_exit_node().is_some()); - assert_eq!(parsed.source, dot); - } - - #[test] - fn parse_invalid_dot() { - let result = parse("not a graph"); - assert!(result.is_err()); - } -} diff --git a/lib/components/fabro-workflow/src/pipeline/persist.rs b/lib/components/fabro-workflow/src/pipeline/persist.rs deleted file mode 100644 index 3f1cdadb9..000000000 --- a/lib/components/fabro-workflow/src/pipeline/persist.rs +++ /dev/null @@ -1,185 +0,0 @@ -use super::types::{PersistOptions, Persisted, Validated}; -use crate::error::Error; - -/// PERSIST phase: create the run directory and return durable metadata for -/// store persistence. -pub(crate) fn persist( - validated: Validated, - mut options: PersistOptions, -) -> Result { - let (graph, source, diagnostics) = validated.into_parts(); - options.run_spec.graph = graph.clone(); - - std::fs::create_dir_all(&options.run_dir).map_err(|err| { - Error::Io(format!( - "creating run directory {}: {err}", - options.run_dir.display() - )) - })?; - - Ok(Persisted::new( - graph, - source, - diagnostics, - options.run_dir, - options.run_spec, - )) -} - -#[cfg(test)] -#[expect(clippy::disallowed_methods, reason = "tests stage pipeline fixtures")] -mod tests { - use std::collections::HashMap; - - use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; - use fabro_types::{PetriAdmission, RunSpec, fixtures, test_support}; - - use super::*; - - fn graph_and_source() -> (Graph, String) { - let source = r#"digraph test { - graph [goal="Ship feature"]; - start [shape=Mdiamond]; - exit [shape=Msquare]; - start -> exit; -}"# - .to_string(); - - let mut graph = Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - AttrValue::String("Ship feature".to_string()), - ); - - let mut start = Node::new("start"); - start.attrs.insert( - "shape".to_string(), - AttrValue::String("Mdiamond".to_string()), - ); - graph.nodes.insert("start".to_string(), start); - - let mut exit = Node::new("exit"); - exit.attrs.insert( - "shape".to_string(), - AttrValue::String("Msquare".to_string()), - ); - graph.nodes.insert("exit".to_string(), exit); - - graph.edges.push(Edge::new("start", "exit")); - (graph, source) - } - - fn different_graph() -> Graph { - let mut graph = Graph::new("different"); - let mut start = Node::new("start"); - start.attrs.insert( - "shape".to_string(), - AttrValue::String("Mdiamond".to_string()), - ); - graph.nodes.insert("start".to_string(), start); - graph - } - - fn sample_record(graph: Graph) -> RunSpec { - RunSpec { - run_id: fixtures::RUN_1, - settings: fabro_types::WorkflowSettings { - run: fabro_types::settings::RunNamespace { - execution: fabro_types::settings::run::RunExecutionSettings { - mode: fabro_types::settings::run::RunMode::DryRun, - ..fabro_types::settings::run::RunExecutionSettings::default() - }, - ..fabro_types::settings::RunNamespace::default() - }, - ..fabro_types::WorkflowSettings::default() - }, - graph, - graph_source: None, - workflow_slug: Some("ship".to_string()), - workflow_version_id: None, - target: None, - automation: None, - source_directory: Some("/tmp/project".to_string()), - git: Some(fabro_types::GitContext { - origin_url: String::new(), - branch: "main".to_string(), - sha: None, - dirty: fabro_types::DirtyStatus::Clean, - }), - labels: HashMap::from([ - ("env".to_string(), "test".to_string()), - ("team".to_string(), "workflow".to_string()), - ]), - provenance: test_support::test_run_provenance(), - definition_blob: None, - spec_blob: None, - fork_source_ref: None, - admission: PetriAdmission::default(), - } - } - - #[test] - fn persist_creates_run_dir_without_writing_legacy_files() { - let temp = tempfile::tempdir().unwrap(); - let run_dir = temp.path().join("run"); - let (graph, source) = graph_and_source(); - let persisted = persist( - Validated::new(graph.clone(), source, vec![]), - PersistOptions { - run_dir: run_dir.clone(), - run_spec: sample_record(different_graph()), - }, - ) - .unwrap(); - - assert!(run_dir.is_dir()); - assert!( - std::fs::read_dir(&run_dir).unwrap().next().is_none(), - "persist should not project files into the scratch dir" - ); - assert_eq!(persisted.run_dir(), run_dir.as_path()); - assert_eq!( - serde_json::to_value(persisted.run_spec().graph.clone()).unwrap(), - serde_json::to_value(graph).unwrap() - ); - } - - #[test] - fn persist_overwrites_run_spec_graph_with_validated_graph() { - let temp = tempfile::tempdir().unwrap(); - let run_dir = temp.path().join("run"); - let (graph, source) = graph_and_source(); - - let persisted = persist( - Validated::new(graph.clone(), source, vec![]), - PersistOptions { - run_dir: run_dir.clone(), - run_spec: sample_record(different_graph()), - }, - ) - .unwrap(); - - assert_eq!(persisted.run_spec().graph.name, graph.name); - assert!(persisted.run_spec().graph.nodes.contains_key("exit")); - assert_eq!( - serde_json::to_value(persisted.run_spec().graph.clone()).unwrap(), - serde_json::to_value(graph).unwrap() - ); - } - - #[test] - fn persist_returns_error_on_io_failure() { - let temp = tempfile::tempdir().unwrap(); - let run_dir = temp.path().join("run"); - std::fs::write(&run_dir, "not a directory").unwrap(); - let (graph, source) = graph_and_source(); - - let err = persist(Validated::new(graph, source, vec![]), PersistOptions { - run_dir, - run_spec: sample_record(different_graph()), - }) - .unwrap_err(); - - assert!(matches!(err, Error::Io(_))); - } -} diff --git a/lib/components/fabro-workflow/src/pipeline/transform.rs b/lib/components/fabro-workflow/src/pipeline/transform.rs deleted file mode 100644 index 01c40193e..000000000 --- a/lib/components/fabro-workflow/src/pipeline/transform.rs +++ /dev/null @@ -1,628 +0,0 @@ -use std::sync::Arc; - -use super::types::{Parsed, TransformOptions, Transformed}; -use crate::error::Error; -use crate::transforms::{ - FileInliningTransform, ImportTransform, ModelStylesheetTemplateTransform, - ScriptInterpolationTransform, StylesheetApplicationTransform, TemplateTransform, Transform, -}; - -/// TRANSFORM phase: apply built-in and custom transforms to a parsed graph. -/// -/// Returns `Transformed` with a graph for post-transform adjustments -/// (e.g. goal override) before validation. -pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result { - let Parsed { graph, source } = parsed; - let mut diagnostics = Vec::new(); - - // Built-in transforms (PreambleTransform moved to engine execution time) - let graph = if let (Some(current_dir), Some(file_resolver)) = - (&options.current_dir, &options.file_resolver) - { - let (graph, transform_diagnostics) = ImportTransform::new( - current_dir.clone(), - Arc::clone(file_resolver), - options.template_context.clone(), - ) - .with_template_options( - options.source_name.clone(), - Some(source.clone()), - options.render_mode, - ) - .apply_with_diagnostics(graph)?; - diagnostics.extend(transform_diagnostics); - graph - } else { - graph - }; - - let graph = if let (Some(current_dir), Some(file_resolver)) = - (&options.current_dir, &options.file_resolver) - { - let (graph, transform_diagnostics) = - FileInliningTransform::new(current_dir.clone(), Arc::clone(file_resolver)) - .with_template_options( - options.template_context.clone(), - options.source_name.clone(), - Some(source.clone()), - options.render_mode, - ) - .apply_with_diagnostics(graph)?; - diagnostics.extend(transform_diagnostics); - graph - } else { - graph - }; - - let (graph, transform_diagnostics) = TemplateTransform { - context: options.template_context.clone(), - source_name: options.source_name.clone(), - source_text: Some(source.clone()), - render_mode: options.render_mode, - } - .apply_with_diagnostics(graph)?; - diagnostics.extend(transform_diagnostics); - let graph = if graph.model_stylesheet().is_empty() { - graph - } else { - let (graph, transform_diagnostics) = ModelStylesheetTemplateTransform { - context: options.template_context.clone(), - source_name: options.source_name.clone(), - source_text: Some(source.clone()), - render_mode: options.render_mode, - file_resolution: options - .current_dir - .clone() - .zip(options.file_resolver.clone()), - } - .apply_with_diagnostics(graph)?; - diagnostics.extend(transform_diagnostics); - graph - }; - let (graph, transform_diagnostics) = ScriptInterpolationTransform { - context: options.template_context.clone(), - source_name: options.source_name.clone(), - render_mode: options.render_mode, - } - .apply_with_diagnostics(graph)?; - diagnostics.extend(transform_diagnostics); - let graph = StylesheetApplicationTransform.apply(graph)?; - - // Custom transforms - let graph = options - .custom_transforms - .iter() - .try_fold(graph, |graph, transform| transform.apply(graph))?; - - Ok(Transformed { - graph, - source, - diagnostics, - }) -} - -#[cfg(test)] -#[expect(clippy::disallowed_methods, reason = "tests stage pipeline fixtures")] -mod tests { - use std::collections::HashMap; - use std::path::Path; - use std::sync::Arc; - - use fabro_graphviz::graph::AttrValue; - - use super::*; - use crate::file_resolver::FilesystemFileResolver; - use crate::pipeline::parse::parse; - use crate::pipeline::types::{GOAL_SELF_REFERENCE_RULE, TEMPLATE_UNDEFINED_VARIABLE_RULE}; - - 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 transform_options() -> TransformOptions { - TransformOptions { - current_dir: None, - file_resolver: None, - template_context: fabro_template::TemplateContext::new(), - source_name: None, - render_mode: crate::operations::RenderMode::Strict, - custom_transforms: vec![], - } - } - - #[test] - fn transform_applies_variable_expansion() { - let dot = r#"digraph Test { - graph [goal="Fix bugs"] - start [shape=Mdiamond] - work [prompt="Goal: {{ goal }}"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &transform_options()).unwrap(); - let prompt = transformed.graph.nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str) - .unwrap(); - assert_eq!(prompt, "Goal: Fix bugs"); - } - - #[test] - fn transform_applies_stylesheet() { - let dot = r#"digraph Test { - graph [goal="Test", model_stylesheet="* { model: sonnet; }"] - start [shape=Mdiamond] - work [label="Work"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &transform_options()).unwrap(); - assert_eq!( - transformed.graph.nodes["work"].attrs.get("model"), - Some(&AttrValue::String("sonnet".into())) - ); - } - - #[test] - fn transform_renders_model_stylesheet_before_applying_and_resolving_it() { - let dot = r#"digraph Test { - graph [ - goal="Test", - model_stylesheet=" - * { reasoning_effort: low; } - {# MiniJinja comments can sit beside CSS braces. #} - {% if inputs.effort == 'deep' %} - .variable { model: sonnet; reasoning_effort: high; } - {% endif %} - " - ] - start [shape=Mdiamond] - baseline [prompt="Baseline"] - selected [prompt="Selected", class="variable"] - explicit [prompt="Explicit", class="variable", reasoning_effort="medium"] - exit [shape=Msquare] - start -> baseline -> selected -> explicit -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &TransformOptions { - template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::from([ - ( - "effort".to_string(), - toml::Value::String("deep".to_string()), - ), - ])), - ..transform_options() - }) - .unwrap(); - - assert_eq!( - transformed.graph.nodes["baseline"] - .attrs - .get("reasoning_effort") - .and_then(AttrValue::as_str), - Some("low") - ); - assert_eq!( - transformed.graph.nodes["selected"] - .attrs - .get("reasoning_effort") - .and_then(AttrValue::as_str), - Some("high") - ); - assert_eq!( - transformed.graph.nodes["selected"] - .attrs - .get("model") - .and_then(AttrValue::as_str), - Some("sonnet") - ); - assert_eq!( - transformed.graph.nodes["explicit"] - .attrs - .get("reasoning_effort") - .and_then(AttrValue::as_str), - Some("medium") - ); - assert!( - transformed - .diagnostics - .iter() - .all(|diagnostic| diagnostic.rule != "detemplated_attribute"), - "{:?}", - transformed.diagnostics - ); - } - - #[test] - fn transform_renders_model_stylesheet_static_include() { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("styles.partial"), - ".selected { model: sonnet; }", - ); - let source_name = dir.path().join("workflow.fabro"); - let dot = r#"digraph Test { - graph [model_stylesheet="{% include 'styles.partial' %}"] - start [shape=Mdiamond] - selected [prompt="Selected", class="selected"] - exit [shape=Msquare] - start -> selected -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &TransformOptions { - current_dir: Some(dir.path().to_path_buf()), - file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))), - source_name: Some(source_name.display().to_string()), - ..transform_options() - }) - .unwrap(); - - assert_eq!( - transformed.graph.nodes["selected"] - .attrs - .get("model") - .and_then(AttrValue::as_str), - Some("sonnet") - ); - } - - #[test] - fn structural_model_stylesheet_undefined_value_skips_stylesheet_parsing() { - let dot = r#"digraph Test { - graph [model_stylesheet="* { reasoning_effort: {{ inputs.effort }}; }"] - start [shape=Mdiamond] - work [prompt="Work"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &TransformOptions { - render_mode: crate::operations::RenderMode::Structural, - ..transform_options() - }) - .unwrap(); - - assert_eq!(transformed.graph.model_stylesheet(), ""); - assert_eq!( - transformed - .diagnostics - .iter() - .filter(|diagnostic| diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .count(), - 1 - ); - assert!( - transformed - .diagnostics - .iter() - .all(|diagnostic| diagnostic.rule != "detemplated_attribute") - ); - } - - #[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 { - current_dir: Some(dir.path().to_path_buf()), - file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))), - ..transform_options() - }) - .unwrap(); - - 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 {{ inputs.task }}", - ); - 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 { - current_dir: Some(dir.path().to_path_buf()), - file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))), - template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::from([ - ( - "task".to_string(), - toml::Value::String("Launch".to_string()), - ), - ])), - ..transform_options() - }) - .unwrap(); - - 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("sonnet".into())) - ); - } - - #[test] - fn imported_script_does_not_rescan_substituted_token_syntax() { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("child.fabro"), - r#"digraph child { - start [shape=Mdiamond] - run [shape=parallelogram, script="printf '%s' {{ inputs.payload }}"] - exit [shape=Msquare] - start -> run -> exit - }"#, - ); - let parsed = parse( - r#"digraph Test { - graph [goal="Test"] - start [shape=Mdiamond] - child [import="./child.fabro"] - exit [shape=Msquare] - start -> child -> exit - }"#, - ) - .unwrap(); - let transformed = transform(parsed, &TransformOptions { - current_dir: Some(dir.path().to_path_buf()), - file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))), - template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::from([ - ( - "payload".to_string(), - toml::Value::String("{{ secrets.API_KEY }}".to_string()), - ), - ])), - ..transform_options() - }) - .unwrap(); - - assert_eq!( - transformed.graph.nodes["child.run"] - .attrs - .get("script") - .and_then(AttrValue::as_str), - Some("printf '%s' '{{ secrets.API_KEY }}'") - ); - } - - #[test] - fn imported_script_reports_a_missing_value_once() { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("child.fabro"), - r#"digraph child { - start [shape=Mdiamond] - run [shape=parallelogram, script="echo {{ inputs.missing }}"] - exit [shape=Msquare] - start -> run -> exit - }"#, - ); - let parsed = parse( - r#"digraph Test { - graph [goal="Test"] - start [shape=Mdiamond] - child [import="./child.fabro"] - exit [shape=Msquare] - start -> child -> exit - }"#, - ) - .unwrap(); - let transformed = transform(parsed, &TransformOptions { - current_dir: Some(dir.path().to_path_buf()), - file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))), - render_mode: crate::operations::RenderMode::Structural, - ..transform_options() - }) - .unwrap(); - - let missing_value_diagnostics = transformed - .diagnostics - .iter() - .filter(|diagnostic| diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .count(); - assert_eq!( - missing_value_diagnostics, 1, - "{:?}", - transformed.diagnostics - ); - } - - #[test] - fn transform_interpolates_vars_in_node_prompt() { - let dot = r#"digraph Test { - graph [goal="Fix bugs"] - start [shape=Mdiamond] - work [prompt="Service: {{ vars.SERVICE }}"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &TransformOptions { - template_context: fabro_template::TemplateContext::new().with_vars(HashMap::from([( - "SERVICE".to_string(), - "billing".to_string(), - )])), - ..transform_options() - }) - .unwrap(); - let prompt = transformed.graph.nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str) - .unwrap(); - assert_eq!(prompt, "Service: billing"); - } - - #[test] - fn transform_interpolates_vars_in_graph_goal_and_through_prompt() { - // The goal interpolates `{{ vars.* }}`, and a prompt that embeds the - // goal sees the vars-resolved text. - let dot = r#"digraph Test { - graph [goal="Ship {{ vars.SERVICE }}"] - start [shape=Mdiamond] - work [prompt="Goal: {{ goal }}"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &TransformOptions { - template_context: fabro_template::TemplateContext::new().with_vars(HashMap::from([( - "SERVICE".to_string(), - "billing".to_string(), - )])), - ..transform_options() - }) - .unwrap(); - assert_eq!( - transformed - .graph - .attrs - .get("goal") - .and_then(AttrValue::as_str), - Some("Ship billing") - ); - assert_eq!( - transformed.graph.nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Goal: Ship billing") - ); - } - - #[test] - fn transform_with_empty_vars_warns_on_unknown_var() { - // Offline / no variable store: `{{ vars.* }}` is undefined, surfacing a - // structural-mode warning (promoted to a hard error at run-create). - let dot = r#"digraph Test { - graph [goal="Fix bugs"] - start [shape=Mdiamond] - work [prompt="Service: {{ vars.MISSING }}"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &TransformOptions { - template_context: fabro_template::TemplateContext::new(), - render_mode: crate::operations::RenderMode::Structural, - ..transform_options() - }) - .unwrap(); - let diag = transformed - .diagnostics - .iter() - .find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .expect("expected a template_undefined_variable diagnostic for vars.MISSING"); - assert!( - diag.message.contains("vars.MISSING"), - "message: {}", - diag.message - ); - } - - #[test] - fn structural_transform_preserves_catalog_owned_model_selection() { - let dot = r#"digraph Test { - graph [goal="Test"] - start [shape=Mdiamond] - work [prompt="Do work", model="private-model", provider="server-only"] - exit [shape=Msquare] - start -> work -> exit - }"#; - let parsed = parse(dot).unwrap(); - let transformed = transform(parsed, &TransformOptions { - ..transform_options() - }) - .unwrap(); - let work = &transformed.graph.nodes["work"]; - - assert_eq!( - work.attrs.get("model").and_then(AttrValue::as_str), - Some("private-model") - ); - assert_eq!( - work.attrs.get("provider").and_then(AttrValue::as_str), - Some("server-only") - ); - } - - #[test] - fn transform_reports_goal_self_reference_once_across_passes() { - // FileInlining renders the goal for prompt context, but TemplateTransform - // is the only pass that should emit the self-reference diagnostic. - let dir = tempfile::tempdir().unwrap(); - let parsed = parse( - r#"digraph Test { - graph [goal="Improve on {{ goal }}"] - start [shape=Mdiamond] - work [prompt="Do the work"] - exit [shape=Msquare] - start -> work -> exit - }"#, - ) - .unwrap(); - let transformed = transform(parsed, &TransformOptions { - current_dir: Some(dir.path().to_path_buf()), - file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))), - render_mode: crate::operations::RenderMode::Structural, - ..transform_options() - }) - .unwrap(); - - let self_ref = transformed - .diagnostics - .iter() - .filter(|d| d.rule == GOAL_SELF_REFERENCE_RULE) - .count(); - assert_eq!( - self_ref, 1, - "goal self-reference should be reported exactly once across transform passes" - ); - } -} diff --git a/lib/components/fabro-workflow/src/pipeline/types.rs b/lib/components/fabro-workflow/src/pipeline/types.rs deleted file mode 100644 index 6952dbe2f..000000000 --- a/lib/components/fabro-workflow/src/pipeline/types.rs +++ /dev/null @@ -1,221 +0,0 @@ -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use fabro_graphviz::graph::Graph; -use fabro_template::TemplateContext; -use fabro_types::RunSpec; -use fabro_types::diagnostic::{Diagnostic, Severity}; - -use crate::error::Error; -use crate::file_resolver::FileResolver; -use crate::transforms::{RenderMode, Transform}; - -/// Output of the PARSE phase. -#[non_exhaustive] -pub struct Parsed { - pub graph: Graph, - pub source: String, -} - -/// Output of the TRANSFORM phase. Graph is mutable — callers may apply -/// post-transform adjustments (e.g. goal override) before validation. -#[non_exhaustive] -pub struct Transformed { - pub graph: Graph, - pub source: String, - /// Diagnostics produced during the transform pass. Prepended to the - /// validation diagnostics so users see them before lint output. - pub diagnostics: Vec, -} - -/// Lint rule name attached to diagnostics for undefined template variables. -pub const TEMPLATE_UNDEFINED_VARIABLE_RULE: &str = "template_undefined_variable"; - -/// Lint rule name attached to diagnostics for graph goal self-references. -pub(crate) const GOAL_SELF_REFERENCE_RULE: &str = "goal_self_reference"; - -/// Output of the VALIDATE phase. Always produced (even with errors). -/// Caller inspects diagnostics and decides whether to proceed. -/// Graph is read-only — use accessors, not direct field access. -#[non_exhaustive] -pub struct Validated { - graph: Graph, - source: String, - diagnostics: Vec, -} - -impl Validated { - /// Create a new `Validated` from its parts. - pub(crate) fn new(graph: Graph, source: String, diagnostics: Vec) -> Self { - Self { - graph, - source, - diagnostics, - } - } - - pub fn graph(&self) -> &Graph { - &self.graph - } - - pub fn source(&self) -> &str { - &self.source - } - - pub fn diagnostics(&self) -> &[Diagnostic] { - &self.diagnostics - } - - /// Promote diagnostics for one rule from warnings to errors. Rendering is - /// intentionally lenient; callers decide whether a diagnostic should block - /// the operation they are about to perform. - pub fn promote_rule_to_error(&mut self, rule: &str) { - for diagnostic in &mut self.diagnostics { - if diagnostic.rule == rule { - diagnostic.severity = Severity::Error; - } - } - } - - pub fn promote_template_undefined_variables_to_errors(&mut self) { - self.promote_rule_to_error(TEMPLATE_UNDEFINED_VARIABLE_RULE); - } - - /// Add diagnostics from another judge of the workflow (Petri's check), - /// after the transforms' own. - pub fn extend_diagnostics(&mut self, diagnostics: impl IntoIterator) { - self.diagnostics.extend(diagnostics); - } - - /// True if any diagnostic has Error severity. - #[must_use] - pub fn has_errors(&self) -> bool { - self.diagnostics - .iter() - .any(|d| d.severity == Severity::Error) - } - - /// Returns `Err(Error::Validation)` if any Error-severity diagnostics - /// exist. Diagnostics remain accessible via `diagnostics()` for - /// printing before this call. - pub fn raise_on_errors(&self) -> Result<(), Error> { - if self.has_errors() { - let message = self - .diagnostics - .iter() - .filter(|d| d.severity == Severity::Error) - .map(|d| d.message.as_str()) - .collect::>() - .join("; "); - return Err(Error::Validation(message)); - } - Ok(()) - } - - /// Consume into owned graph, source, and diagnostics (used by initialize). - pub fn into_parts(self) -> (Graph, String, Vec) { - (self.graph, self.source, self.diagnostics) - } -} - -/// Options for the PERSIST phase. -pub(crate) struct PersistOptions { - pub run_dir: PathBuf, - pub run_spec: RunSpec, -} - -/// Output of the PERSIST phase. Run directory created and the validated -/// workflow is persisted into the durable run spec. -#[derive(Debug)] -#[non_exhaustive] -pub struct Persisted { - graph: Graph, - source: String, - diagnostics: Vec, - run_dir: PathBuf, - run_spec: RunSpec, -} - -impl Persisted { - /// Create a new `Persisted` from its parts. - pub(crate) fn new( - graph: Graph, - source: String, - diagnostics: Vec, - run_dir: PathBuf, - run_spec: RunSpec, - ) -> Self { - Self { - graph, - source, - diagnostics, - run_dir, - run_spec, - } - } - - pub fn graph(&self) -> &Graph { - &self.graph - } - - pub fn source(&self) -> &str { - &self.source - } - - pub fn diagnostics(&self) -> &[Diagnostic] { - &self.diagnostics - } - - pub fn run_dir(&self) -> &Path { - &self.run_dir - } - - pub fn run_spec(&self) -> &RunSpec { - &self.run_spec - } - - /// True if any diagnostic has Error severity. - #[must_use] - pub fn has_errors(&self) -> bool { - self.diagnostics - .iter() - .any(|d| d.severity == Severity::Error) - } - - /// Returns `Err(Error::Validation)` if any Error-severity diagnostics - /// exist. - pub fn raise_on_errors(&self) -> Result<(), Error> { - if self.has_errors() { - let message = self - .diagnostics - .iter() - .filter(|d| d.severity == Severity::Error) - .map(|d| d.message.as_str()) - .collect::>() - .join("; "); - return Err(Error::Validation(message)); - } - Ok(()) - } - - /// Consume into owned graph, source, diagnostics, run dir, and run spec. - pub fn into_parts(self) -> (Graph, String, Vec, PathBuf, RunSpec) { - ( - self.graph, - self.source, - self.diagnostics, - self.run_dir, - self.run_spec, - ) - } -} - -/// Options for the TRANSFORM phase. -pub struct TransformOptions { - pub current_dir: Option, - pub file_resolver: Option>, - pub template_context: TemplateContext, - pub source_name: Option, - pub render_mode: RenderMode, - pub custom_transforms: Vec>, -} diff --git a/lib/components/fabro-workflow/src/pipeline/validate.rs b/lib/components/fabro-workflow/src/pipeline/validate.rs deleted file mode 100644 index 9e557ecf0..000000000 --- a/lib/components/fabro-workflow/src/pipeline/validate.rs +++ /dev/null @@ -1,97 +0,0 @@ -use super::types::{Transformed, Validated}; - -/// VALIDATE phase: the transformed graph with the transforms' diagnostics. -/// Fabro's lint rules went with the legacy executor; the workflow's rules -/// and its models are Petri's to judge at admission. -/// -/// **Infallible.** Always returns `Validated` with diagnostics. Caller decides -/// whether to fail via `validated.raise_on_errors()`. -#[must_use] -pub fn validate(transformed: Transformed) -> Validated { - let Transformed { - graph, - source, - diagnostics, - } = transformed; - Validated::new(graph, source, diagnostics) -} - -#[cfg(test)] -mod tests { - use std::collections::HashMap; - - use fabro_types::diagnostic::Severity; - - use super::*; - use crate::pipeline::parse::parse; - use crate::pipeline::transform; - use crate::pipeline::types::TransformOptions; - - fn transform_options() -> TransformOptions { - TransformOptions { - current_dir: None, - file_resolver: None, - template_context: fabro_template::TemplateContext::new(), - source_name: None, - render_mode: crate::operations::RenderMode::Strict, - custom_transforms: vec![], - } - } - - fn run_pipeline(dot: &str) -> Validated { - let parsed = parse(dot).unwrap(); - let transformed = transform::transform(parsed, &transform_options()).unwrap(); - validate(transformed) - } - - #[test] - fn validate_valid_graph() { - let dot = r#"digraph Test { - graph [goal="Build feature"] - start [shape=Mdiamond] - exit [shape=Msquare] - start -> exit - }"#; - let validated = run_pipeline(dot); - assert!(!validated.has_errors()); - assert!(validated.raise_on_errors().is_ok()); - } - - #[test] - fn validate_into_parts() { - let dot = r#"digraph Test { - graph [goal="Build feature"] - start [shape=Mdiamond] - exit [shape=Msquare] - start -> exit - }"#; - let validated = run_pipeline(dot); - let (graph, source, diagnostics) = validated.into_parts(); - assert_eq!(graph.name, "Test"); - assert_eq!(source, dot); - assert!(diagnostics.iter().all(|d| d.severity != Severity::Error)); - } - - #[test] - fn unresolved_template_variables_are_the_transforms_diagnostics() { - let dot = r#"digraph Test { - graph [model_stylesheet="* { model: {{ vars.MODEL }}; }"] - start [shape=Mdiamond] - exit [shape=Msquare] - start -> exit - }"#; - let transformed = transform::transform(parse(dot).unwrap(), &TransformOptions { - template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::new()), - source_name: Some("workflow.fabro".to_string()), - render_mode: crate::operations::RenderMode::Structural, - ..transform_options() - }) - .unwrap(); - let validated = validate(transformed); - - assert!(validated.diagnostics().iter().any(|diagnostic| { - diagnostic.rule == "template_undefined_variable" - && diagnostic.message.contains("vars.MODEL") - })); - } -} diff --git a/lib/components/fabro-workflow/src/run_materialization.rs b/lib/components/fabro-workflow/src/run_materialization.rs deleted file mode 100644 index 89e6a58e1..000000000 --- a/lib/components/fabro-workflow/src/run_materialization.rs +++ /dev/null @@ -1,24 +0,0 @@ -use fabro_graphviz::graph::Graph; -use fabro_types::WorkflowSettings; -use fabro_types::settings::InterpString; -use fabro_types::settings::run::RunGoal; - -/// The graph's goal becomes the run's inline goal (none when the graph has -/// none), and a pull request block the settings disable is dropped. -pub fn materialize_goal_and_pull_request(settings: &mut WorkflowSettings, graph: &Graph) { - let goal = graph.goal().to_string(); - settings.run.goal = if goal.is_empty() { - None - } else { - Some(RunGoal::Inline(InterpString::parse(&goal))) - }; - - if settings - .run - .pull_request - .as_ref() - .is_some_and(|pull_request| !pull_request.enabled) - { - settings.run.pull_request = None; - } -} diff --git a/lib/components/fabro-workflow/src/transforms/file_inlining.rs b/lib/components/fabro-workflow/src/transforms/file_inlining.rs deleted file mode 100644 index d1bf7f832..000000000 --- a/lib/components/fabro-workflow/src/transforms/file_inlining.rs +++ /dev/null @@ -1,721 +0,0 @@ -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use fabro_graphviz::graph::{AttrValue, Graph}; -use fabro_template::{TemplateContext, TemplateSource, TemplateStore}; -use fabro_types::ManifestPath; -use fabro_types::diagnostic::Diagnostic; - -use super::Transform; -use super::importable_field::ImportableField; -use crate::error::Error; -use crate::file_resolver::{FileResolver, FileResolverTemplateStore, ResolvedFile}; -use crate::transforms::variable_expansion::{ - RenderMode, TemplateRenderStore, TemplateRenderTarget, render_template_for_target, -}; - -fn parent_dir_or_dot(path: &Path) -> PathBuf { - path.parent() - .filter(|parent| !parent.as_os_str().is_empty()) - .map_or_else(|| PathBuf::from("."), Path::to_path_buf) -} - -pub(crate) fn template_render_store( - current_dir: &Path, - resolver: Arc, - source_name: Option<&str>, -) -> Result { - let root = template_root_for_current_dir(current_dir)?; - let source_path = template_source_path_for_current_dir(current_dir, source_name, &root)?; - let base_dir = template_store_base_dir(current_dir); - // The store's render substitutes the text being rendered, so the source - // carries only its path and template root. - Ok(TemplateRenderStore::new( - TemplateSource::new(source_path, root, String::new()), - Arc::new(FileResolverTemplateStore::new(base_dir, resolver)), - )) -} - -fn template_store_base_dir(current_dir: &Path) -> PathBuf { - if current_dir.is_absolute() { - current_dir.to_path_buf() - } else { - PathBuf::from(".") - } -} - -fn template_root_for_current_dir(current_dir: &Path) -> Result { - if current_dir.is_absolute() { - return manifest_path("."); - } - manifest_path_from_path(current_dir) -} - -fn template_source_path_for_current_dir( - current_dir: &Path, - source_name: Option<&str>, - root: &ManifestPath, -) -> Result { - if let Some(source_name) = source_name { - let source_path = Path::new(source_name); - if source_path.is_absolute() { - if let Some(path) = ManifestPath::from_absolute(source_path, current_dir) { - return Ok(path); - } - } else if let Some(path) = ManifestPath::from_wire(source_name) { - if root.as_path().as_os_str().is_empty() || path.starts_with(root) { - return Ok(path); - } - if let Some(path) = ManifestPath::from_reference(root.as_path(), source_name) { - return Ok(path); - } - } - } - ManifestPath::from_reference(root.as_path(), "workflow.fabro") - .ok_or_else(|| Error::Validation("invalid workflow template source path".to_string())) -} - -fn manifest_path(value: &str) -> Result { - ManifestPath::from_wire(value) - .ok_or_else(|| Error::Validation(format!("invalid manifest path: {value}"))) -} - -fn manifest_path_from_path(path: &Path) -> Result { - let value = path - .to_str() - .ok_or_else(|| Error::Validation(format!("invalid UTF-8 path: {}", path.display())))?; - manifest_path(value) -} - -fn manifest_parent_or_dot(path: &ManifestPath) -> Result { - manifest_path_from_path(path.parent_or_dot()) -} - -fn manifest_path_is_within_root(path: &ManifestPath, root: &ManifestPath) -> bool { - if root.as_path().as_os_str().is_empty() { - return !path - .as_path() - .components() - .next() - .is_some_and(|component| matches!(component, std::path::Component::ParentDir)); - } - path.starts_with(root) -} - -/// Inlines `@file` references in node prompts and the graph-level goal. -pub struct FileInliningTransform { - current_dir: PathBuf, - resolver: Arc, - context: TemplateContext, - source_name: Option, - source_text: Option, - goal_override: Option, - render_mode: RenderMode, -} - -impl FileInliningTransform { - #[must_use] - pub fn new(current_dir: PathBuf, resolver: Arc) -> Self { - Self { - current_dir, - resolver, - context: TemplateContext::new(), - source_name: None, - source_text: None, - goal_override: None, - render_mode: RenderMode::Strict, - } - } - - #[must_use] - pub fn with_template_options( - mut self, - context: TemplateContext, - source_name: Option, - source_text: Option, - render_mode: RenderMode, - ) -> Self { - self.context = context; - self.source_name = source_name; - self.source_text = source_text; - self.render_mode = render_mode; - self - } - - #[must_use] - pub fn with_goal_override(mut self, goal: Option) -> Self { - self.goal_override = goal; - self - } - - pub(crate) fn apply_with_diagnostics( - &self, - graph: Graph, - ) -> Result<(Graph, Vec), Error> { - let mut graph = graph; - let mut diagnostics = Vec::new(); - self.inline_graph_goal(&mut graph, &mut diagnostics)?; - - let resolved_goal = match &self.goal_override { - Some(goal) => goal.clone(), - // `inline_graph_goal` has already rendered inputs and inlined any - // goal file reference for prompt context. The later TemplateTransform - // pass owns canonical goal validation diagnostics. - None => graph.goal().to_string(), - }; - let ctx = self.context.clone().with_goal(resolved_goal); - - for (node_id, node) in &mut graph.nodes { - // `prompt` is an importable template: MiniJinja-render the value, - // then inline any `@file` reference (whose contents are rendered - // too). Clone up front so the immutable borrow ends before we - // re-insert. - let prompt = match node.attrs.get("prompt") { - Some(AttrValue::String(value)) => Some(value.clone()), - _ => None, - }; - if let Some(attr_value) = prompt { - let target = TemplateRenderTarget::node_attr( - self.source_name.clone(), - node_id.clone(), - "prompt", - ) - .with_source_origin(self.source_text.as_deref(), &attr_value) - .with_template_store(template_render_store( - &self.current_dir, - Arc::clone(&self.resolver), - self.source_name.as_deref(), - )?); - let rendered = render_template_for_target( - &attr_value, - &ctx, - self.render_mode, - &target, - &mut diagnostics, - )?; - let value = self - .render_import(&rendered, &ctx, target, &mut diagnostics)? - .unwrap_or(rendered); - node.attrs - .insert("prompt".to_string(), AttrValue::String(value)); - } - - // `output_schema` is NOT a template: an inline JSON string is used - // verbatim, and an `@file` reference is loaded verbatim. Neither the - // value nor the loaded contents are MiniJinja-rendered. - let output_schema = match node.attrs.get("output_schema") { - Some(AttrValue::String(value)) => Some(value.clone()), - _ => None, - }; - if let Some(attr_value) = output_schema { - let value = self.resolve_output_schema_ref(node_id, &attr_value)?; - node.attrs - .insert("output_schema".to_string(), AttrValue::String(value)); - } - } - - Ok((graph, diagnostics)) - } - - fn inline_graph_goal( - &self, - graph: &mut Graph, - diagnostics: &mut Vec, - ) -> Result<(), Error> { - let Some(AttrValue::String(goal)) = graph.attrs.get("goal") else { - return Ok(()); - }; - let ctx = self.context.clone().with_goal("{{ goal }}"); - let target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal") - .with_source_origin(self.source_text.as_deref(), goal) - .with_template_store(template_render_store( - &self.current_dir, - Arc::clone(&self.resolver), - self.source_name.as_deref(), - )?); - let rendered = - render_template_for_target(goal, &ctx, self.render_mode, &target, diagnostics)?; - let value = self - .render_import(&rendered, &ctx, target, diagnostics)? - .unwrap_or(rendered); - graph - .attrs - .insert("goal".to_string(), AttrValue::String(value)); - Ok(()) - } - - /// Resolve a node `prompt` / graph `goal` value to its final text. The - /// `rendered` value is the already-MiniJinja-rendered inline content; when - /// it is an `@path` import, the file is loaded and its contents rendered - /// too. Returns `Ok(None)` for inline content or a missing file, so the - /// caller falls back to the rendered inline value. - fn render_import( - &self, - rendered: &str, - ctx: &TemplateContext, - owner_target: TemplateRenderTarget, - diagnostics: &mut Vec, - ) -> Result, Error> { - let Some(path) = ImportableField::parse(rendered).import_path()? else { - return Ok(None); - }; - let Some(resolved) = self.resolver.resolve(&self.current_dir, path) else { - return Ok(None); - }; - let (source, store) = self.template_source_for_resolved_file(&resolved)?; - let target = owner_target - .with_source_name(resolved.path.display().to_string()) - .with_source_origin(Some(&resolved.content), &resolved.content) - .with_template_store(TemplateRenderStore::new(source, store)); - Ok(Some(render_template_for_target( - &resolved.content, - ctx, - self.render_mode, - &target, - diagnostics, - )?)) - } - - /// Resolve an `output_schema` value. An inline JSON string is returned - /// as-is; an `@file` import is loaded verbatim. Unlike `prompt`, - /// `output_schema` is not a template, so neither the value nor the loaded - /// file contents are MiniJinja-rendered. - fn resolve_output_schema_ref(&self, node_id: &str, value: &str) -> Result { - let Some(path) = ImportableField::parse(value).import_path()? else { - return Ok(value.to_string()); - }; - let Some(resolved) = self.resolver.resolve(&self.current_dir, path) else { - return Err(Error::Validation(format!( - "node '{node_id}' output_schema has unresolved file reference: {value}" - ))); - }; - Ok(resolved.content) - } - - fn template_root_for_resolved_file(&self, path: &Path) -> PathBuf { - let parent = parent_dir_or_dot(path); - if self.current_dir.is_absolute() && path.starts_with(&self.current_dir) { - self.current_dir.clone() - } else { - parent - } - } - - fn template_source_for_resolved_file( - &self, - resolved: &ResolvedFile, - ) -> Result<(TemplateSource, Arc), Error> { - if resolved.path.is_absolute() { - let root_dir = self.template_root_for_resolved_file(&resolved.path); - let path = ManifestPath::from_absolute(&resolved.path, &root_dir).ok_or_else(|| { - Error::Validation(format!( - "invalid resolved template path: {}", - resolved.path.display() - )) - })?; - return Ok(( - TemplateSource::new(path, manifest_path(".")?, resolved.content.clone()), - Arc::new(FileResolverTemplateStore::new( - root_dir, - Arc::clone(&self.resolver), - )), - )); - } - - let path = manifest_path_from_path(&resolved.path)?; - let current_root = template_root_for_current_dir(&self.current_dir)?; - let root = if manifest_path_is_within_root(&path, ¤t_root) { - current_root - } else { - manifest_parent_or_dot(&path)? - }; - Ok(( - TemplateSource::new(path, root, resolved.content.clone()), - Arc::new(FileResolverTemplateStore::new( - PathBuf::from("."), - Arc::clone(&self.resolver), - )), - )) - } -} - -impl Transform for FileInliningTransform { - fn apply(&self, graph: Graph) -> Result { - let (graph, diagnostics) = self.apply_with_diagnostics(graph)?; - if !diagnostics.is_empty() { - return Err(Error::ValidationFailed { diagnostics }); - } - Ok(graph) - } -} - -#[cfg(test)] -mod tests { - #![expect( - clippy::disallowed_methods, - reason = "These unit tests use the real git CLI to build repositories for file-inlining transform coverage." - )] - - use std::collections::HashMap; - use std::sync::Arc; - - use fabro_graphviz::graph::{AttrValue, Graph, Node}; - use fabro_template::{TemplateRenderMode, TemplateSource, render_source}; - use fabro_types::ManifestPath; - - use super::*; - use crate::file_resolver::{BundleFileResolver, FilesystemFileResolver}; - - fn manifest_path(value: &str) -> ManifestPath { - ManifestPath::from_wire(value).expect("path should parse") - } - - #[test] - fn file_inlining_transform_inlines_prompt_and_goal() { - let dir = tempfile::tempdir().unwrap(); - // Init repo - std::process::Command::new("git") - .args(["init"]) - .current_dir(dir.path()) - .output() - .unwrap(); - std::process::Command::new("git") - .args([ - "-c", - "user.name=test", - "-c", - "user.email=test@test", - "commit", - "--allow-empty", - "-m", - "init", - ]) - .current_dir(dir.path()) - .output() - .unwrap(); - - std::fs::write(dir.path().join("prompt.md"), "Do the work").unwrap(); - std::fs::write(dir.path().join("goal.md"), "Ship feature").unwrap(); - - let mut graph = Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - AttrValue::String("@goal.md".to_string()), - ); - let mut node = Node::new("work"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("@prompt.md".to_string()), - ); - graph.nodes.insert("work".to_string(), node); - - let transform = FileInliningTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - ); - let graph = transform.apply(graph).unwrap(); - - assert_eq!( - graph.nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Do the work") - ); - assert_eq!( - graph.attrs.get("goal").and_then(AttrValue::as_str), - Some("Ship feature") - ); - } - - #[test] - fn file_inlining_transform_inlines_output_schema_reference() { - let dir = tempfile::tempdir().unwrap(); - std::fs::create_dir_all(dir.path().join("schemas")).unwrap(); - std::fs::write( - dir.path().join("schemas/audit-result.schema.json"), - r#"{"type":"object","required":["passed"]}"#, - ) - .unwrap(); - - let mut graph = Graph::new("test"); - let mut node = Node::new("audit"); - node.attrs.insert( - "output_schema".to_string(), - AttrValue::String("@schemas/audit-result.schema.json".to_string()), - ); - graph.nodes.insert("audit".to_string(), node); - - let transform = FileInliningTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - ); - let graph = transform.apply(graph).unwrap(); - - assert_eq!( - graph.nodes["audit"] - .attrs - .get("output_schema") - .and_then(AttrValue::as_str), - Some(r#"{"type":"object","required":["passed"]}"#) - ); - } - - #[test] - fn file_inlining_transform_leaves_routing_output_schema_keyword_unchanged() { - let dir = tempfile::tempdir().unwrap(); - let mut graph = Graph::new("test"); - let mut node = Node::new("route"); - node.attrs.insert( - "output_schema".to_string(), - AttrValue::String("routing".to_string()), - ); - graph.nodes.insert("route".to_string(), node); - - let transform = FileInliningTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - ); - let graph = transform.apply(graph).unwrap(); - - assert_eq!( - graph.nodes["route"] - .attrs - .get("output_schema") - .and_then(AttrValue::as_str), - Some("routing") - ); - } - - #[test] - fn file_inlining_transform_does_not_render_templates_in_output_schema() { - let dir = tempfile::tempdir().unwrap(); - let mut graph = Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - AttrValue::String("Fix bugs".to_string()), - ); - let mut node = Node::new("emit"); - // `output_schema` is not a template: `{{ goal }}` must stay literal. - node.attrs.insert( - "output_schema".to_string(), - AttrValue::String(r#"{"title": "{{ goal }}"}"#.to_string()), - ); - graph.nodes.insert("emit".to_string(), node); - - let transform = FileInliningTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - ); - let graph = transform.apply(graph).unwrap(); - - assert_eq!( - graph.nodes["emit"] - .attrs - .get("output_schema") - .and_then(AttrValue::as_str), - Some(r#"{"title": "{{ goal }}"}"#) - ); - } - - #[test] - fn file_inlining_transform_loads_output_schema_file_verbatim() { - let dir = tempfile::tempdir().unwrap(); - // File contents contain template syntax that must NOT be rendered. - std::fs::write( - dir.path().join("schema.json"), - r#"{"kind": "{{ inputs.kind }}"}"#, - ) - .unwrap(); - let mut graph = Graph::new("test"); - let mut node = Node::new("emit"); - node.attrs.insert( - "output_schema".to_string(), - AttrValue::String("@schema.json".to_string()), - ); - graph.nodes.insert("emit".to_string(), node); - - let transform = FileInliningTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - ); - let graph = transform.apply(graph).unwrap(); - - assert_eq!( - graph.nodes["emit"] - .attrs - .get("output_schema") - .and_then(AttrValue::as_str), - Some(r#"{"kind": "{{ inputs.kind }}"}"#) - ); - } - - #[test] - fn file_inlining_transform_reports_unresolved_output_schema_reference() { - let dir = tempfile::tempdir().unwrap(); - let mut graph = Graph::new("test"); - let mut node = Node::new("audit"); - node.attrs.insert( - "output_schema".to_string(), - AttrValue::String("@schemas/missing.schema.json".to_string()), - ); - graph.nodes.insert("audit".to_string(), node); - - let transform = FileInliningTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - ); - let error = transform.apply(graph).unwrap_err(); - - assert!( - error.to_string().contains( - "node 'audit' output_schema has unresolved file reference: @schemas/missing.schema.json" - ), - "unexpected error: {error}", - ); - } - - #[test] - fn file_inlining_transform_resolves_minijinja_includes_for_prompts_and_goal() { - let dir = tempfile::tempdir().unwrap(); - std::fs::create_dir_all(dir.path().join("prompts")).unwrap(); - std::fs::create_dir_all(dir.path().join("goals")).unwrap(); - std::fs::write( - dir.path().join("prompts/work.md"), - r#"{% include "work.tpl.md" %}"#, - ) - .unwrap(); - std::fs::write(dir.path().join("prompts/work.tpl.md"), "file prompt").unwrap(); - std::fs::write(dir.path().join("inline.tpl.md"), "inline prompt").unwrap(); - std::fs::write( - dir.path().join("goals/goal.md"), - r#"{% include "goal.tpl.md" %}"#, - ) - .unwrap(); - std::fs::write(dir.path().join("goals/goal.tpl.md"), "included goal").unwrap(); - - let mut graph = Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - AttrValue::String("@goals/goal.md".to_string()), - ); - let mut file_prompt = Node::new("file_prompt"); - file_prompt.attrs.insert( - "prompt".to_string(), - AttrValue::String("@prompts/work.md".to_string()), - ); - graph.nodes.insert("file_prompt".to_string(), file_prompt); - let mut inline_prompt = Node::new("inline_prompt"); - inline_prompt.attrs.insert( - "prompt".to_string(), - AttrValue::String(r#"{% include "inline.tpl.md" %}"#.to_string()), - ); - graph - .nodes - .insert("inline_prompt".to_string(), inline_prompt); - - let transform = FileInliningTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - ); - let graph = transform.apply(graph).unwrap(); - - assert_eq!( - graph.nodes["file_prompt"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("file prompt") - ); - assert_eq!( - graph.nodes["inline_prompt"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("inline prompt") - ); - assert_eq!( - graph.attrs.get("goal").and_then(AttrValue::as_str), - Some("included goal") - ); - } - - #[test] - fn file_resolver_template_store_renders_sibling_partial_under_root() { - let resolver = Arc::new(BundleFileResolver::new(HashMap::from([( - manifest_path("prompts/partials/audit.partial.tpl"), - "shared partial".to_string(), - )]))); - let store = FileResolverTemplateStore::new(PathBuf::from("."), resolver); - let source = TemplateSource::new( - manifest_path("prompts/audits/audit.prompt.md"), - manifest_path("prompts"), - r#"{% include "../partials/audit.partial.tpl" %}"#, - ); - - let rendered = render_source( - &source, - &TemplateContext::new(), - Arc::new(store), - TemplateRenderMode::Strict, - ) - .unwrap(); - - assert_eq!(rendered, "shared partial"); - } - - #[test] - fn file_resolver_template_store_rejects_escaping_include() { - let resolver = Arc::new(BundleFileResolver::new(HashMap::from([( - manifest_path("outside.md"), - "outside".to_string(), - )]))); - let store = FileResolverTemplateStore::new(PathBuf::from("."), resolver); - let source = TemplateSource::new( - manifest_path("prompts/audits/audit.prompt.md"), - manifest_path("prompts"), - r#"{% include "../../outside.md" %}"#, - ); - - let err = render_source( - &source, - &TemplateContext::new(), - Arc::new(store), - TemplateRenderMode::Strict, - ) - .unwrap_err(); - - assert!(matches!(err, fabro_template::TemplateError::Load { .. })); - } - - #[test] - fn file_inlining_transform_falls_back_to_fallback_dir() { - let base = tempfile::tempdir().unwrap(); - let fallback = tempfile::tempdir().unwrap(); - std::fs::write(fallback.path().join("shared.md"), "shared prompt").unwrap(); - - let mut graph = Graph::new("test"); - let mut node = Node::new("work"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("@shared.md".to_string()), - ); - graph.nodes.insert("work".to_string(), node); - - let transform = FileInliningTransform::new( - base.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(Some( - fallback.path().to_path_buf(), - ))), - ); - let graph = transform.apply(graph).unwrap(); - - assert_eq!( - graph.nodes["work"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("shared prompt") - ); - } -} diff --git a/lib/components/fabro-workflow/src/transforms/import.rs b/lib/components/fabro-workflow/src/transforms/import.rs deleted file mode 100644 index c53be71cd..000000000 --- a/lib/components/fabro-workflow/src/transforms/import.rs +++ /dev/null @@ -1,1877 +0,0 @@ -use std::collections::HashMap; -use std::path::{Path, PathBuf}; -use std::sync::Arc; - -use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; -use fabro_graphviz::parser; -use fabro_template::{TemplateContext, validate_static_reference}; -use fabro_types::diagnostic::{Diagnostic, Severity}; -use fabro_types::graph::ReferenceKind; - -use super::file_inlining::template_render_store; -use super::{FileInliningTransform, Transform}; -use crate::error::Error; -use crate::file_resolver::{FileResolver, ResolvedFile}; -use crate::transforms::variable_expansion::{ - RenderMode, TemplateRenderTarget, TemplateTransform, render_template_for_target, -}; - -pub struct ImportTransform { - current_dir: PathBuf, - resolver: Arc, - context: TemplateContext, - source_name: Option, - source_text: Option, - render_mode: RenderMode, -} - -struct PlaceholderOptions { - 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, - diagnostics: Vec, -} - -enum ImportPrepareError { - Hard(Error), - Soft(String), -} - -const IMPORTED_MODEL_STYLESHEET_IGNORED_RULE: &str = "imported_model_stylesheet_ignored"; - -fn imported_model_stylesheet_ignored_diagnostic(source_name: &str) -> Diagnostic { - Diagnostic { - rule: IMPORTED_MODEL_STYLESHEET_IGNORED_RULE.to_string(), - severity: Severity::Warning, - message: "imported graph attribute `model_stylesheet` is ignored; only the root graph stylesheet is applied" - .to_string(), - fix: Some( - "move the stylesheet to the root graph; it can target imported nodes by ID, class, or shape" - .to_string(), - ), - source_path: Some(source_name.to_string()), - ..Diagnostic::default() - } -} - -impl From for ImportPrepareError { - fn from(error: Error) -> Self { - Self::Hard(error) - } -} - -impl ImportTransform { - #[must_use] - pub fn new( - current_dir: PathBuf, - resolver: Arc, - context: TemplateContext, - ) -> Self { - Self { - current_dir, - resolver, - context, - source_name: None, - source_text: None, - render_mode: RenderMode::Structural, - } - } - - #[must_use] - pub fn with_template_options( - mut self, - source_name: Option, - source_text: Option, - render_mode: RenderMode, - ) -> Self { - self.source_name = source_name; - self.source_text = source_text; - self.render_mode = render_mode; - self - } - - 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, - parent_goal: &str, - current_base_dir: &Path, - import_stack: &mut Vec, - ) -> Result, Error> { - if !graph.nodes.contains_key(placeholder_id) { - return Ok(Vec::new()); - } - - if graph - .edges - .iter() - .any(|edge| edge.from == placeholder_id && edge.to == placeholder_id) - { - Self::poison_placeholder( - graph, - placeholder_id, - &format!("import placeholder '{placeholder_id}' cannot have a self-loop"), - ); - return Ok(Vec::new()); - } - - let placeholder = match Self::placeholder_config(graph, placeholder_id) { - Ok(placeholder) => placeholder, - Err(message) => { - Self::poison_placeholder(graph, placeholder_id, &message); - return Ok(Vec::new()); - } - }; - - if let Err(error) = validate_static_reference(import_path, ReferenceKind::Import) { - Self::poison_placeholder(graph, placeholder_id, &error.to_string()); - return Ok(Vec::new()); - } - - let Some(resolved_file) = self.resolver.resolve(current_base_dir, import_path) else { - Self::poison_placeholder( - graph, - placeholder_id, - &format!("file not found: {import_path}"), - ); - return Ok(Vec::new()); - }; - - if import_stack.contains(&resolved_file.path) { - let cycle = import_stack - .iter() - .chain(std::iter::once(&resolved_file.path)) - .map(|path| path.display().to_string()) - .collect::>() - .join(" -> "); - Self::poison_placeholder( - graph, - placeholder_id, - &format!("circular import detected: {cycle}"), - ); - return Ok(Vec::new()); - } - - let prepared = match self.prepare_import(&resolved_file, parent_goal, import_stack) { - Ok(prepared) => prepared, - Err(ImportPrepareError::Hard(error)) => return Err(error), - Err(ImportPrepareError::Soft(message)) => { - Self::poison_placeholder(graph, placeholder_id, &message); - return Ok(Vec::new()); - } - }; - let diagnostics = prepared.diagnostics.clone(); - - if let Err(message) = Self::splice_import( - graph, - placeholder_id, - &resolved_file.path, - &placeholder, - prepared, - ) { - Self::poison_placeholder(graph, placeholder_id, &message); - } - - Ok(diagnostics) - } - - fn prepare_import( - &self, - resolved_file: &ResolvedFile, - parent_goal: &str, - import_stack: &mut Vec, - ) -> Result { - Self::with_import_stack(import_stack, resolved_file.path.clone(), |import_stack| { - let mut diagnostics = Vec::new(); - let source_name = resolved_file.path.display().to_string(); - let source_text = resolved_file.content.clone(); - - let mut graph = parser::parse(&resolved_file.content).map_err(|error| { - ImportPrepareError::Soft(format!( - "failed to parse {}: {error}", - resolved_file.path.display() - )) - })?; - - if !graph.model_stylesheet().is_empty() { - diagnostics.push(imported_model_stylesheet_ignored_diagnostic(&source_name)); - } - - let import_base_dir = resolved_file - .path - .parent() - .map_or_else(|| PathBuf::from("."), Path::to_path_buf); - let (inlined_graph, file_diagnostics) = - FileInliningTransform::new(import_base_dir.clone(), Arc::clone(&self.resolver)) - .with_template_options( - self.context.clone(), - Some(source_name.clone()), - Some(source_text.clone()), - self.render_mode, - ) - .with_goal_override(Some(parent_goal.to_string())) - .apply_with_diagnostics(graph) - .map_err(ImportPrepareError::Hard)?; - graph = inlined_graph; - diagnostics.extend(file_diagnostics); - - graph.attrs.insert( - "goal".to_string(), - AttrValue::String(parent_goal.to_string()), - ); - let (templated_graph, template_diagnostics) = TemplateTransform { - context: self.context.clone(), - source_name: Some(source_name), - source_text: Some(source_text), - render_mode: self.render_mode, - } - .apply_with_diagnostics(graph) - .map_err(ImportPrepareError::Hard)?; - graph = templated_graph; - diagnostics.extend(template_diagnostics); - - if let Some(message) = Self::unresolved_imported_prompt_error(&graph) { - return Err(ImportPrepareError::Soft(message)); - } - - let nested_imports = Self::collect_import_nodes(&graph); - for (placeholder_id, import_path) in nested_imports { - let nested_diagnostics = self.expand_import( - &mut graph, - &placeholder_id, - &import_path, - parent_goal, - &import_base_dir, - import_stack, - )?; - diagnostics.extend(nested_diagnostics); - } - - let mut prepared = - Self::validate_imported_graph(graph).map_err(ImportPrepareError::Soft)?; - prepared.diagnostics = diagnostics; - Ok(prepared) - }) - } - - fn splice_import( - graph: &mut Graph, - placeholder_id: &str, - resolved_path: &Path, - placeholder: &PlaceholderOptions, - 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::>(); - let is_empty = prepared.is_empty(); - - let PreparedImport { - graph: imported_graph, - start_id, - exit_id, - entry_id, - exit_predecessor_id, - diagnostics: _, - } = prepared; - - for node_id in imported_graph.nodes.keys() { - if node_id == &start_id || node_id == &exit_id { - continue; - } - - let prefixed_id = format!("{placeholder_id}.{node_id}"); - if graph.nodes.contains_key(&prefixed_id) { - return Err(format!( - "import placeholder '{placeholder_id}' would overwrite existing node '{prefixed_id}'" - )); - } - } - - if 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); - - for (node_id, mut merged_node) in imported_graph.nodes { - if node_id == start_id || node_id == exit_id { - continue; - } - - let prefixed_id = format!("{placeholder_id}.{node_id}"); - merged_node.id.clone_from(&prefixed_id); - let imported_attrs = std::mem::take(&mut merged_node.attrs); - merged_node.attrs.clone_from(&placeholder.default_attrs); - merged_node.attrs.extend(imported_attrs); - Self::remap_retry_target(&mut merged_node.attrs, placeholder_id); - - for class_name in &placeholder.class_names { - merged_node.add_class(class_name); - } - merged_node.add_class(&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(); - - for (key, value) in &node.attrs { - if key == "import" { - continue; - } - - // The parser already split `class` into `node.classes`. - if key == "class" { - 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(PlaceholderOptions { - default_attrs, - class_names: node.classes.clone(), - normalized_class: Self::normalize_class_name(placeholder_id), - }) - } - - fn validate_imported_graph(graph: Graph) -> Result { - let has_non_sentinel_nodes = graph.nodes.iter().any(|(id, node)| { - !Self::is_start_sentinel(id, node) && !Self::is_exit_sentinel(id, node) - }); - 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(); - if has_non_sentinel_nodes && entry_id == exit_id { - return Err( - "imported start node cannot route directly to exit when non-sentinel nodes exist" - .to_string(), - ); - } - - 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(); - if has_non_sentinel_nodes && exit_predecessor_id == start_id { - return Err( - "imported exit node cannot be reached directly from start when non-sentinel nodes exist" - .to_string(), - ); - } - - Ok(PreparedImport { - graph, - start_id, - exit_id, - entry_id, - exit_predecessor_id, - diagnostics: Vec::new(), - }) - } - - 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" - | "acp.command" - | "acp.config" - | "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 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) - }) && matches!( - self.graph.edges.as_slice(), - [edge] if edge.from == self.start_id && edge.to == self.exit_id - ) - } -} - -impl Transform for ImportTransform { - fn apply(&self, graph: Graph) -> Result { - let (graph, diagnostics) = self.apply_with_diagnostics(graph)?; - if !diagnostics.is_empty() { - return Err(Error::ValidationFailed { diagnostics }); - } - Ok(graph) - } -} - -impl ImportTransform { - pub(crate) fn apply_with_diagnostics( - &self, - graph: Graph, - ) -> Result<(Graph, Vec), Error> { - let mut graph = graph; - let imports = Self::collect_import_nodes(&graph); - let mut import_stack = Vec::new(); - let mut diagnostics = Vec::new(); - let path_ctx = self.context.clone().with_goal("{{ goal }}"); - let mut ignored_goal_diagnostics = Vec::new(); - let goal_target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal") - .with_source_origin(self.source_text.as_deref(), graph.goal()) - .with_template_store(template_render_store( - &self.current_dir, - Arc::clone(&self.resolver), - self.source_name.as_deref(), - )?); - let parent_goal = render_template_for_target( - graph.goal(), - &path_ctx, - self.render_mode, - &goal_target, - &mut ignored_goal_diagnostics, - )?; - - for (placeholder_id, import_path) in imports { - if let Err(error) = validate_static_reference(&import_path, ReferenceKind::Import) { - Self::poison_placeholder(&mut graph, &placeholder_id, &error.to_string()); - continue; - } - let target = TemplateRenderTarget::node_attr( - self.source_name.clone(), - placeholder_id.clone(), - "import", - ) - .with_source_origin(self.source_text.as_deref(), &import_path); - let rendered_import_path = render_template_for_target( - &import_path, - &path_ctx, - self.render_mode, - &target, - &mut diagnostics, - )?; - let import_diagnostics = self.expand_import( - &mut graph, - &placeholder_id, - &rendered_import_path, - &parent_goal, - &self.current_dir, - &mut import_stack, - )?; - diagnostics.extend(import_diagnostics); - } - - Ok((graph, diagnostics)) - } -} - -#[cfg(test)] -#[expect(clippy::disallowed_methods, reason = "tests stage transform fixtures")] -mod tests { - use std::path::Path; - use std::sync::Arc; - - use fabro_graphviz::graph::AttrValue; - use fabro_graphviz::parser; - use fabro_util::error::collect_chain; - - use super::*; - use crate::file_resolver::FilesystemFileResolver; - - 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 graph = parse_graph(dot); - ImportTransform::new( - base_dir.to_path_buf(), - Arc::new(FilesystemFileResolver::new( - fallback_dir.map(Path::to_path_buf), - )), - TemplateContext::new(), - ) - .apply(graph) - .unwrap() - } - - #[test] - fn templated_import_path_poison_placeholder_before_rendering() { - let dir = tempfile::tempdir().unwrap(); - let graph = parse_graph( - r#"digraph Test { - start [shape=Mdiamond] - validate [import="{{ inputs.path }}"] - exit [shape=Msquare] - start -> validate -> exit - }"#, - ); - - let graph = ImportTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - TemplateContext::new(), - ) - .with_template_options( - Some("workflow.fabro".to_string()), - Some("workflow source {{ inputs.path }}".to_string()), - RenderMode::Strict, - ) - .apply(graph) - .unwrap(); - - let error = graph.nodes["validate"] - .attrs - .get("import_error") - .and_then(AttrValue::as_str) - .expect("templated import path should poison placeholder"); - assert!( - error.contains("templates are not supported in import references"), - "unexpected error: {error}" - ); - } - - #[test] - fn imported_workflow_template_error_names_imported_file() { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("child.fabro"), - r#"digraph Child { - graph [goal="{{ inputs.foo }}"] - start [shape=Mdiamond] - work [prompt="Do it"] - exit [shape=Msquare] - start -> work -> exit - }"#, - ); - let graph = parse_graph( - r#"digraph Test { - start [shape=Mdiamond] - validate [import="./child.fabro"] - exit [shape=Msquare] - start -> validate -> exit - }"#, - ); - - let err = ImportTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - TemplateContext::new(), - ) - .with_template_options( - Some("workflow.fabro".to_string()), - Some("workflow source".to_string()), - RenderMode::Strict, - ) - .apply(graph) - .unwrap_err(); - let rendered = collect_chain(&err).join(": "); - - assert!(rendered.contains("child.fabro"), "{rendered}"); - assert!(rendered.contains("inputs.foo"), "{rendered}"); - assert!(!rendered.contains(""), "{rendered}"); - } - - #[test] - fn imported_model_stylesheets_are_ignored_with_a_warning() { - for stylesheet in [ - "* { reasoning_effort: high; }", - "{% if inputs.deep %}* { reasoning_effort: high; }{% endif %}", - ] { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("child.fabro"), - &format!( - r#"digraph Child {{ - graph [model_stylesheet="{stylesheet}"] - start [shape=Mdiamond] - work [prompt="Do it"] - exit [shape=Msquare] - start -> work -> exit - }}"#, - ), - ); - let graph = parse_graph( - r#"digraph Test { - start [shape=Mdiamond] - child [import="./child.fabro"] - exit [shape=Msquare] - start -> child -> exit - }"#, - ); - - let (graph, diagnostics) = ImportTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - TemplateContext::new(), - ) - .apply_with_diagnostics(graph) - .unwrap(); - - assert_eq!( - graph.nodes["child.work"] - .attrs - .get("reasoning_effort") - .and_then(AttrValue::as_str), - None - ); - let warning = diagnostics - .iter() - .find(|diagnostic| diagnostic.rule == IMPORTED_MODEL_STYLESHEET_IGNORED_RULE) - .expect("expected ignored imported stylesheet warning"); - assert!( - warning - .source_path - .as_deref() - .is_some_and(|path| path.ends_with("child.fabro")), - "{warning:?}" - ); - assert!( - diagnostics - .iter() - .all(|diagnostic| diagnostic.rule != "detemplated_attribute"), - "{diagnostics:?}" - ); - } - } - - 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 import_parent_goal_resolves_vars_before_imported_prompt_uses_goal() { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("validate.fabro"), - r#"digraph validate { - start [shape=Mdiamond] - lint [prompt="Goal: {{ goal }}"] - exit [shape=Msquare] - start -> lint -> exit - }"#, - ); - let graph = parse_graph( - r#"digraph Deploy { - graph [goal="Ship {{ vars.SERVICE }}"] - start [shape=Mdiamond] - validate [import="./validate.fabro"] - exit [shape=Msquare] - start -> validate -> exit - }"#, - ); - - let graph = ImportTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - TemplateContext::new().with_vars(HashMap::from([( - "SERVICE".to_string(), - "billing".to_string(), - )])), - ) - .apply(graph) - .unwrap(); - - assert_eq!( - graph.nodes["validate.lint"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Goal: Ship billing") - ); - } - - #[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_eq!(graph.nodes["validate.lint"].id, "validate.lint"); - 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 edge_only_node_in_imported_fragment_stays_missing() { - 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 -> typo -> 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.typo")); - assert!(graph.nodes.contains_key("validate.lint")); - } - - #[test] - fn edge_only_body_is_not_treated_as_empty_import() { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("validate.fabro"), - r"digraph validate { - start [shape=Mdiamond] - exit [shape=Msquare] - start -> typo -> 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.typo")); - assert!( - graph - .edges - .iter() - .any(|edge| edge.from == "start" && edge.to == "validate.typo") - ); - assert!( - graph - .edges - .iter() - .any(|edge| edge.from == "validate.typo" && edge.to == "exit") - ); - } - - #[test] - fn import_reports_structural_diagnostic_for_imported_prompt_templates() { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("validate.fabro"), - r#"digraph validate { - start [shape=Mdiamond] - lint [prompt="Run {{ inputs.task }}"] - exit [shape=Msquare] - start -> lint -> exit - }"#, - ); - - let graph = parse_graph( - r#"digraph Deploy { - start [shape=Mdiamond] - validate [import="./validate.fabro"] - exit [shape=Msquare] - start -> validate -> exit - }"#, - ); - - let (graph, diagnostics) = ImportTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - TemplateContext::new(), - ) - .apply_with_diagnostics(graph) - .unwrap(); - - assert_eq!( - graph.nodes["validate.lint"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Run ") - ); - let diagnostic = diagnostics - .iter() - .find(|diagnostic| diagnostic.rule == "template_undefined_variable") - .expect("expected imported prompt template diagnostic"); - assert_eq!(diagnostic.node_id.as_deref(), Some("lint")); - assert!( - diagnostic - .source_path - .as_deref() - .is_some_and(|path| path.ends_with("validate.fabro")), - "{diagnostic:?}" - ); - assert!( - diagnostic - .message - .contains("node `lint` attribute `prompt`"), - "{diagnostic:?}" - ); - } - - #[test] - fn templated_import_path_poison_placeholder() { - let dir = tempfile::tempdir().unwrap(); - let graph = apply_import( - r#"digraph Deploy { - start [shape=Mdiamond] - validate [import="./{{ inputs.workflow }}.fabro"] - exit [shape=Msquare] - start -> validate -> exit - }"#, - dir.path(), - None, - ); - - let error = graph.nodes["validate"] - .attrs - .get("import_error") - .and_then(AttrValue::as_str) - .expect("templated import path should poison placeholder"); - assert!( - error.contains("templates are not supported in import references"), - "unexpected error: {error}" - ); - } - - #[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", backend="acp", acp.command="python fake_agent.py", 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") - ); - assert_eq!( - graph.nodes["validate.test"] - .attrs - .get("backend") - .and_then(AttrValue::as_str), - Some("acp") - ); - assert_eq!( - graph.nodes["validate.test"] - .attrs - .get("acp.command") - .and_then(AttrValue::as_str), - Some("python fake_agent.py") - ); - } - - #[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 imported_start_and_exit_sentinels_must_be_declared() { - let dir = tempfile::tempdir().unwrap(); - let host = r#"digraph Deploy { - start [shape=Mdiamond] - validate [import="./validate.fabro"] - exit [shape=Msquare] - start -> validate -> exit - }"#; - let cases = [ - ( - r#"digraph validate { - work [prompt="Run checks"] - exit [shape=Msquare] - start -> work -> exit - }"#, - "imported workflow must have exactly one start node, found 0", - ), - ( - r#"digraph validate { - start [shape=Mdiamond] - work [prompt="Run checks"] - start -> work -> exit - }"#, - "imported workflow must have exactly one exit node, found 0", - ), - ]; - - for (source, expected_error) in cases { - write_file(&dir.path().join("validate.fabro"), source); - let graph = apply_import(host, dir.path(), None); - - assert_eq!( - graph.nodes["validate"] - .attrs - .get("import_error") - .and_then(AttrValue::as_str), - Some(expected_error) - ); - } - } - - #[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=succeeded"] - 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=succeeded"] - 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=succeeded")); - - 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=succeeded"] - 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")); - } - - #[test] - fn start_to_exit_with_orphan_nodes_poison_placeholder() { - let dir = tempfile::tempdir().unwrap(); - write_file( - &dir.path().join("broken.fabro"), - r#"digraph broken { - start [shape=Mdiamond] - orphan [prompt="Never connected"] - exit [shape=Msquare] - start -> exit - }"#, - ); - - let graph = apply_import( - r#"digraph Host { - start [shape=Mdiamond] - broken [import="./broken.fabro"] - exit [shape=Msquare] - start -> broken -> exit - }"#, - dir.path(), - None, - ); - - assert_eq!( - graph.nodes["broken"] - .attrs - .get("import_error") - .and_then(AttrValue::as_str), - Some("imported start node cannot route directly to exit when non-sentinel nodes exist") - ); - assert!( - !graph - .edges - .iter() - .any(|edge| edge.to == "broken.exit" || edge.from == "broken.start") - ); - } - - #[test] - fn namespace_collision_poison_placeholder() { - let dir = tempfile::tempdir().unwrap(); - write_file(&dir.path().join("validate.fabro"), basic_import_source()); - - let mut graph = parse_graph( - r#"digraph Host { - start [shape=Mdiamond] - validate [import="./validate.fabro"] - exit [shape=Msquare] - start -> validate -> exit - }"#, - ); - let mut colliding_node = Node::new("validate.lint"); - colliding_node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Preexisting host node".to_string()), - ); - graph - .nodes - .insert("validate.lint".to_string(), colliding_node); - let graph = ImportTransform::new( - dir.path().to_path_buf(), - Arc::new(FilesystemFileResolver::new(None)), - TemplateContext::new(), - ) - .apply(graph) - .unwrap(); - - assert_eq!( - graph.nodes["validate"] - .attrs - .get("import_error") - .and_then(AttrValue::as_str), - Some("import placeholder 'validate' would overwrite existing node 'validate.lint'") - ); - assert_eq!( - graph.nodes["validate.lint"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Preexisting host node") - ); - } -} diff --git a/lib/components/fabro-workflow/src/transforms/importable_field.rs b/lib/components/fabro-workflow/src/transforms/importable_field.rs deleted file mode 100644 index cf4b5c2d1..000000000 --- a/lib/components/fabro-workflow/src/transforms/importable_field.rs +++ /dev/null @@ -1,131 +0,0 @@ -//! The `ImportableField` type: a workflow field that is either inline -//! content or an `@path` file import. -//! -//! Three field consumers share this classification: -//! - node `prompt` and the graph `goal` are *templated* importable fields — the -//! inline value (or an imported file's contents) is MiniJinja-rendered; -//! - `output_schema` is a *verbatim* importable field — inline content and -//! imported file contents are used as-is, never rendered. -//! -//! This type owns the `@`-classification and static-reference validation that -//! used to be hand-rolled at each call site. The render-vs-verbatim handling -//! and the file-store plumbing stay with each consumer in -//! [`super::file_inlining`], where the `FileResolver` and current-dir context -//! live. - -use fabro_template::validate_static_reference; -use fabro_types::graph::ReferenceKind; - -use crate::error::Error; - -/// A field value that is either inline content or an `@path` file import. -/// -/// Borrows the classified string: callers always already hold the inline value -/// (and fall back to it), so the type never needs to own a copy. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum ImportableField<'a> { - /// Inline content — the literal value or, for templated fields, the - /// already-rendered text. The caller keeps the value itself; this variant - /// carries no payload. - Inline, - /// An `@path` file import. `path` has the leading `@` stripped. - Import { path: &'a str }, -} - -impl<'a> ImportableField<'a> { - /// Classify a value: a leading `@` marks a file import, everything else is - /// inline. - /// - /// Callers of templated fields (`prompt`/`goal`) classify the - /// *already-rendered* string, because a leading `@` may be produced by - /// rendering (e.g. `{{ inputs.prompt_file }}` expanding to - /// `@prompts/work.md`). - pub(crate) fn parse(value: &'a str) -> Self { - match value.strip_prefix('@') { - Some(path) => Self::Import { path }, - None => Self::Inline, - } - } - - /// The validated import path (leading `@` stripped), or `None` for inline - /// content. Validating here means a caller cannot extract a path without it - /// being checked: an import is a static reference and must not contain - /// template syntax (e.g. `@prompts/{{ inputs.x }}.md`). - pub(crate) fn import_path(&self) -> Result, Error> { - match self { - Self::Import { path } => { - validate_static_reference(path, ReferenceKind::FileInline) - .map_err(|error| Error::Validation(error.to_string()))?; - Ok(Some(path)) - } - Self::Inline => Ok(None), - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn parse_classifies_inline_value() { - assert_eq!( - ImportableField::parse("Do the work"), - ImportableField::Inline - ); - } - - #[test] - fn parse_classifies_at_reference_as_import() { - assert_eq!( - ImportableField::parse("@prompts/work.md"), - ImportableField::Import { - path: "prompts/work.md", - } - ); - } - - #[test] - fn parse_strips_only_the_leading_at() { - // A non-leading `@` (e.g. an email address) is inline, not an import. - assert_eq!( - ImportableField::parse("ping me@example.com"), - ImportableField::Inline - ); - } - - #[test] - fn import_path_returns_validated_path_for_imports_only() { - assert_eq!( - ImportableField::parse("@goal.md").import_path().unwrap(), - Some("goal.md") - ); - assert_eq!( - ImportableField::parse("inline").import_path().unwrap(), - None - ); - } - - #[test] - fn import_path_accepts_inline_and_plain_import_paths() { - ImportableField::parse("plain inline text") - .import_path() - .unwrap(); - ImportableField::parse("@prompts/work.md") - .import_path() - .unwrap(); - } - - #[test] - fn import_path_rejects_template_syntax() { - let err = ImportableField::parse("@prompts/{{ inputs.prompt_file }}") - .import_path() - .unwrap_err(); - - assert!( - err.to_string() - .contains("templates are not supported in file inline references"), - "unexpected error: {err}" - ); - } -} diff --git a/lib/components/fabro-workflow/src/transforms/mod.rs b/lib/components/fabro-workflow/src/transforms/mod.rs deleted file mode 100644 index e3ed639c7..000000000 --- a/lib/components/fabro-workflow/src/transforms/mod.rs +++ /dev/null @@ -1,23 +0,0 @@ -use fabro_graphviz::graph::Graph; - -use crate::error::Error; - -/// A transform that modifies the pipeline graph after parsing and before -/// validation. -pub trait Transform { - fn apply(&self, graph: Graph) -> Result; -} - -mod file_inlining; -mod import; -mod importable_field; -mod model_stylesheet_template; -pub mod stylesheet; -mod stylesheet_application; -pub mod variable_expansion; - -pub use file_inlining::FileInliningTransform; -pub use import::ImportTransform; -pub(crate) use model_stylesheet_template::ModelStylesheetTemplateTransform; -pub use stylesheet_application::StylesheetApplicationTransform; -pub use variable_expansion::{RenderMode, ScriptInterpolationTransform, TemplateTransform}; diff --git a/lib/components/fabro-workflow/src/transforms/model_stylesheet_template.rs b/lib/components/fabro-workflow/src/transforms/model_stylesheet_template.rs deleted file mode 100644 index d38036364..000000000 --- a/lib/components/fabro-workflow/src/transforms/model_stylesheet_template.rs +++ /dev/null @@ -1,225 +0,0 @@ -use std::path::PathBuf; -use std::sync::Arc; - -use fabro_graphviz::graph::{AttrValue, Graph}; -use fabro_template::TemplateContext; -use fabro_types::diagnostic::Diagnostic; - -use super::file_inlining::template_render_store; -use super::variable_expansion::{ - RenderMode, TemplateRenderOutcome, TemplateRenderTarget, render_template_for_target_outcome, -}; -use crate::error::Error; -use crate::file_resolver::FileResolver; - -/// Renders the root graph's `model_stylesheet` with its restricted template -/// context after imports are expanded and before stylesheet parsing. -pub(crate) struct ModelStylesheetTemplateTransform { - pub context: TemplateContext, - pub source_name: Option, - pub source_text: Option, - pub render_mode: RenderMode, - /// Enables `{% include %}` resolution; without it the stylesheet renders - /// from its inline text alone. - pub file_resolution: Option<(PathBuf, Arc)>, -} - -impl ModelStylesheetTemplateTransform { - pub(crate) fn apply_with_diagnostics( - &self, - mut graph: Graph, - ) -> Result<(Graph, Vec), Error> { - let stylesheet = graph.model_stylesheet(); - if stylesheet.is_empty() { - return Ok((graph, Vec::new())); - } - - let mut target = - TemplateRenderTarget::graph_attr(self.source_name.clone(), "model_stylesheet") - .with_source_origin(self.source_text.as_deref(), stylesheet) - .with_restricted_namespace_fix( - "`model_stylesheet` templates expose only `inputs` and `vars`; use one of \ - those values or a MiniJinja local value", - ); - if let Some((current_dir, resolver)) = &self.file_resolution { - target = target.with_template_store(template_render_store( - current_dir, - Arc::clone(resolver), - self.source_name.as_deref(), - )?); - } - - let mut diagnostics = Vec::new(); - let outcome = render_template_for_target_outcome( - stylesheet, - &self.context.for_model_stylesheet(), - self.render_mode, - &target, - &mut diagnostics, - )?; - let rendered = match outcome { - TemplateRenderOutcome::Rendered(rendered) => rendered, - // Do not feed raw or partly rendered MiniJinja source to the - // stylesheet parser during structural validation. - TemplateRenderOutcome::Unresolved => String::new(), - }; - - graph - .attrs - .insert("model_stylesheet".to_string(), AttrValue::String(rendered)); - Ok((graph, diagnostics)) - } -} - -#[cfg(test)] -mod tests { - use std::collections::HashMap; - - use fabro_graphviz::graph::Graph; - use fabro_util::error::collect_chain; - - use super::*; - - fn graph_with_stylesheet(stylesheet: &str) -> Graph { - let mut graph = Graph::new("test"); - graph.attrs.insert( - "model_stylesheet".to_string(), - AttrValue::String(stylesheet.to_string()), - ); - graph - } - - fn transform( - context: TemplateContext, - stylesheet: &str, - render_mode: RenderMode, - ) -> Result<(Graph, Vec), Error> { - ModelStylesheetTemplateTransform { - context, - source_name: Some("workflow.fabro".to_string()), - source_text: Some(format!( - "digraph Test {{ graph [model_stylesheet=\"{stylesheet}\"] }}" - )), - render_mode, - file_resolution: None, - } - .apply_with_diagnostics(graph_with_stylesheet(stylesheet)) - } - - #[test] - fn preserves_static_stylesheet_bytes() { - let stylesheet = "\n * { reasoning_effort: low; }\n"; - - let (graph, diagnostics) = - transform(TemplateContext::new(), stylesheet, RenderMode::Strict).unwrap(); - - assert!(diagnostics.is_empty()); - assert_eq!(graph.model_stylesheet(), stylesheet); - } - - #[test] - fn renders_inputs_vars_control_flow_and_locals_once() { - let stylesheet = r"{% set prefix = '.tier-' %} -{% for effort in inputs.efforts %} -{{ prefix }}{{ loop.index }} { reasoning_effort: {{ effort }}; } -{% endfor %} -.selected { model: {{ vars.MODEL }}; } -.literal { model: {{ inputs.literal }}; }"; - let context = TemplateContext::new() - .with_goal("must stay unavailable") - .with_inputs(HashMap::from([ - ( - "efforts".to_string(), - toml::Value::Array(vec![ - toml::Value::String("low".to_string()), - toml::Value::String("high".to_string()), - ]), - ), - ( - "literal".to_string(), - toml::Value::String("{{ vars.MODEL }}".to_string()), - ), - ])) - .with_vars(HashMap::from([("MODEL".to_string(), "sonnet".to_string())])); - - let (graph, diagnostics) = transform(context, stylesheet, RenderMode::Strict).unwrap(); - - assert!(diagnostics.is_empty()); - assert!( - graph - .model_stylesheet() - .contains(".tier-1 { reasoning_effort: low; }") - ); - assert!( - graph - .model_stylesheet() - .contains(".tier-2 { reasoning_effort: high; }") - ); - assert!( - graph - .model_stylesheet() - .contains(".selected { model: sonnet; }") - ); - assert!( - graph - .model_stylesheet() - .contains(".literal { model: {{ vars.MODEL }}; }") - ); - } - - #[test] - fn structural_undefined_value_clears_stylesheet_and_reports_context() { - for (expression, expected_fix) in [ - ("inputs.effort", "[run.inputs]"), - ("vars.MODEL", "fabro variable set MODEL"), - ("goal", "expose only `inputs` and `vars`"), - ("env.MODEL", "expose only `inputs` and `vars`"), - ("secrets.MODEL", "expose only `inputs` and `vars`"), - ] { - let stylesheet = format!("* {{ model: {{{{ {expression} }}}}; }}"); - let (graph, diagnostics) = transform( - TemplateContext::new().with_goal("hidden"), - &stylesheet, - RenderMode::Structural, - ) - .unwrap(); - - assert_eq!(graph.model_stylesheet(), "", "expression: {expression}"); - assert_eq!(diagnostics.len(), 1, "expression: {expression}"); - assert_eq!(diagnostics[0].rule, "template_undefined_variable"); - assert!( - diagnostics[0] - .message - .contains("graph attribute `model_stylesheet`"), - "{:?}", - diagnostics[0] - ); - assert!( - diagnostics[0] - .fix - .as_deref() - .is_some_and(|fix| fix.contains(expected_fix)), - "{:?}", - diagnostics[0] - ); - } - } - - #[test] - fn syntax_error_preserves_owner_and_source_chain() { - let error = transform( - TemplateContext::new(), - "* { model: {% if %}; }", - RenderMode::Strict, - ) - .unwrap_err(); - let chain = collect_chain(&error).join(": "); - - assert!( - chain.contains("graph attribute `model_stylesheet`"), - "{chain}" - ); - assert!(chain.contains("template syntax error"), "{chain}"); - assert!(chain.contains("workflow.fabro"), "{chain}"); - } -} diff --git a/lib/components/fabro-workflow/src/transforms/stylesheet.rs b/lib/components/fabro-workflow/src/transforms/stylesheet.rs deleted file mode 100644 index eac959255..000000000 --- a/lib/components/fabro-workflow/src/transforms/stylesheet.rs +++ /dev/null @@ -1,258 +0,0 @@ -use fabro_graphviz::graph::{AttrValue, Graph}; -pub use fabro_graphviz::stylesheet::{Rule, Selector, Stylesheet, parse_stylesheet}; - -/// Recognized stylesheet properties. -const STYLESHEET_PROPERTIES: &[&str] = - &["model", "provider", "reasoning_effort", "speed", "backend"]; - -/// Apply a stylesheet to a graph. Rules are applied by specificity order; -/// higher specificity wins. Explicit node attributes are never overridden. -/// -/// # Panics -/// -/// Panics if the internal node map is inconsistent (should not happen). -pub fn apply_stylesheet(stylesheet: &Stylesheet, graph: &mut Graph) { - let mut sorted_rules: Vec<&Rule> = stylesheet.rules.iter().collect(); - sorted_rules.sort_by_key(|r| r.selector.specificity()); - - let node_ids: Vec = graph.nodes.keys().cloned().collect(); - - for node_id in &node_ids { - let mut applied: std::collections::HashMap = - std::collections::HashMap::new(); - - for rule in &sorted_rules { - let node = &graph.nodes[node_id.as_str()]; - let matches = match &rule.selector { - Selector::Universal => true, - Selector::Shape(shape) => node.shape() == shape, - Selector::Class(cls) => node.classes.contains(cls), - Selector::Id(id) => node_id == id, - }; - - if matches { - for decl in &rule.declarations { - if STYLESHEET_PROPERTIES.contains(&decl.property.as_str()) { - let spec = rule.selector.specificity(); - match applied.get(&decl.property) { - Some((_, existing_spec)) if spec < *existing_spec => {} - _ => { - applied.insert(decl.property.clone(), (decl.value.clone(), spec)); - } - } - } - } - } - } - - let node = graph - .nodes - .get_mut(node_id.as_str()) - .expect("node_id was collected from graph.nodes.keys() on the line above, so it must still exist"); - for (prop, (val, _)) in &applied { - if !node.attrs.contains_key(prop) { - node.attrs - .insert(prop.clone(), AttrValue::String(val.clone())); - } - } - } -} - -#[cfg(test)] -mod tests { - use fabro_graphviz::graph::Node; - - use super::*; - - #[test] - fn apply_universal_to_all_nodes() { - let ss = parse_stylesheet("* { model: sonnet; }").unwrap(); - let mut graph = Graph::new("test"); - graph.nodes.insert("a".into(), Node::new("a")); - graph.nodes.insert("b".into(), Node::new("b")); - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["a"].attrs.get("model"), - Some(&AttrValue::String("sonnet".into())) - ); - assert_eq!( - graph.nodes["b"].attrs.get("model"), - Some(&AttrValue::String("sonnet".into())) - ); - } - - #[test] - fn apply_class_overrides_universal() { - let ss = parse_stylesheet("* { model: sonnet; } .code { model: opus; }").unwrap(); - let mut graph = Graph::new("test"); - - let mut code_node = Node::new("impl"); - code_node.classes.push("code".into()); - graph.nodes.insert("impl".into(), code_node); - - let plain_node = Node::new("plan"); - graph.nodes.insert("plan".into(), plain_node); - - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["impl"].attrs.get("model"), - Some(&AttrValue::String("opus".into())) - ); - assert_eq!( - graph.nodes["plan"].attrs.get("model"), - Some(&AttrValue::String("sonnet".into())) - ); - } - - #[test] - fn apply_id_overrides_class() { - let ss = parse_stylesheet(".code { model: opus; } #special { model: gpt; }").unwrap(); - let mut graph = Graph::new("test"); - - let mut node = Node::new("special"); - node.classes.push("code".into()); - graph.nodes.insert("special".into(), node); - - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["special"].attrs.get("model"), - Some(&AttrValue::String("gpt".into())) - ); - } - - #[test] - fn explicit_attrs_not_overridden() { - let ss = parse_stylesheet("* { model: sonnet; }").unwrap(); - let mut graph = Graph::new("test"); - - let mut node = Node::new("a"); - node.attrs - .insert("model".into(), AttrValue::String("explicit".into())); - graph.nodes.insert("a".into(), node); - - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["a"].attrs.get("model"), - Some(&AttrValue::String("explicit".into())) - ); - } - - #[test] - fn spec_section_86_example() { - let input = r" - * { model: claude-sonnet-4-5; provider: anthropic; } - .code { model: claude-opus-4-6; provider: anthropic; } - #critical_review { model: gpt-5.2; provider: openai; reasoning_effort: high; } - "; - let ss = parse_stylesheet(input).unwrap(); - let mut graph = Graph::new("test"); - - let mut plan = Node::new("plan"); - plan.classes.push("planning".into()); - graph.nodes.insert("plan".into(), plan); - - let mut implement = Node::new("implement"); - implement.classes.push("code".into()); - graph.nodes.insert("implement".into(), implement); - - let mut review = Node::new("critical_review"); - review.classes.push("code".into()); - graph.nodes.insert("critical_review".into(), review); - - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["plan"].attrs.get("model"), - Some(&AttrValue::String("claude-sonnet-4-5".into())) - ); - - assert_eq!( - graph.nodes["implement"].attrs.get("model"), - Some(&AttrValue::String("claude-opus-4-6".into())) - ); - - assert_eq!( - graph.nodes["critical_review"].attrs.get("model"), - Some(&AttrValue::String("gpt-5.2".into())) - ); - assert_eq!( - graph.nodes["critical_review"].attrs.get("provider"), - Some(&AttrValue::String("openai".into())) - ); - assert_eq!( - graph.nodes["critical_review"].attrs.get("reasoning_effort"), - Some(&AttrValue::String("high".into())) - ); - } - - #[test] - fn apply_shape_selector_to_matching_nodes() { - let ss = parse_stylesheet("box { model: opus; }").unwrap(); - let mut graph = Graph::new("test"); - - // Default shape is "box" - let box_node = Node::new("a"); - graph.nodes.insert("a".into(), box_node); - - let mut diamond_node = Node::new("b"); - diamond_node - .attrs - .insert("shape".into(), AttrValue::String("Mdiamond".into())); - graph.nodes.insert("b".into(), diamond_node); - - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["a"].attrs.get("model"), - Some(&AttrValue::String("opus".into())) - ); - // Mdiamond node should NOT get the box rule - assert_eq!(graph.nodes["b"].attrs.get("model"), None); - } - - #[test] - fn apply_backend_property_via_stylesheet() { - let ss = parse_stylesheet("* { backend: acp; }").unwrap(); - let mut graph = Graph::new("test"); - graph.nodes.insert("a".into(), Node::new("a")); - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["a"].attrs.get("backend"), - Some(&AttrValue::String("acp".into())) - ); - } - - #[test] - fn backend_property_not_overridden_by_stylesheet() { - let ss = parse_stylesheet("* { backend: acp; }").unwrap(); - let mut graph = Graph::new("test"); - let mut node = Node::new("a"); - node.attrs - .insert("backend".into(), AttrValue::String("api".into())); - graph.nodes.insert("a".into(), node); - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["a"].attrs.get("backend"), - Some(&AttrValue::String("api".into())) - ); - } - - #[test] - fn apply_speed_property() { - let ss = parse_stylesheet("* { speed: fast; }").unwrap(); - let mut graph = Graph::new("test"); - graph.nodes.insert("a".into(), Node::new("a")); - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["a"].attrs.get("speed"), - Some(&AttrValue::String("fast".into())) - ); - } -} diff --git a/lib/components/fabro-workflow/src/transforms/stylesheet_application.rs b/lib/components/fabro-workflow/src/transforms/stylesheet_application.rs deleted file mode 100644 index 6e7ca2292..000000000 --- a/lib/components/fabro-workflow/src/transforms/stylesheet_application.rs +++ /dev/null @@ -1,41 +0,0 @@ -use fabro_graphviz::graph::Graph; - -use super::Transform; -use super::stylesheet::{apply_stylesheet, parse_stylesheet}; -use crate::error::Error; - -/// Applies the `model_stylesheet` graph attribute to resolve LLM properties for -/// each node. -pub struct StylesheetApplicationTransform; - -impl Transform for StylesheetApplicationTransform { - fn apply(&self, graph: Graph) -> Result { - let mut graph = graph; - let stylesheet_text = graph.model_stylesheet().to_string(); - if stylesheet_text.is_empty() { - return Ok(graph); - } - let Ok(stylesheet) = parse_stylesheet(&stylesheet_text) else { - return Ok(graph); - }; - apply_stylesheet(&stylesheet, &mut graph); - Ok(graph) - } -} - -#[cfg(test)] -mod tests { - use fabro_graphviz::graph::{Graph, Node}; - - use super::*; - - #[test] - fn stylesheet_transform_empty_stylesheet() { - let mut graph = Graph::new("test"); - graph.nodes.insert("a".to_string(), Node::new("a")); - - let transform = StylesheetApplicationTransform; - // Should not panic with empty stylesheet - let _graph = transform.apply(graph).unwrap(); - } -} diff --git a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs deleted file mode 100644 index a8ba8a368..000000000 --- a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs +++ /dev/null @@ -1,1416 +0,0 @@ -use std::borrow::Cow; -use std::collections::HashMap; -use std::fmt::Write as _; -use std::sync::Arc; - -use fabro_graphviz::graph::{AttrValue, Graph, Node}; -use fabro_template::{ - TemplateContext, TemplateError, TemplateRenderMode, TemplateSource, TemplateSourceOrigin, - TemplateStore, validate_static_reference, -}; -use fabro_types::diagnostic::{Diagnostic, Severity}; -use fabro_types::graph::{AttributeScope, ReferenceKind, reference_kind_for_attribute}; -use fabro_types::settings::interp::Namespace; -use fabro_types::settings::{InterpString, ResolveCtx, ResolveError, ResolveErrorKind}; -use fabro_util::error::collect_chain; -use fabro_util::shell; - -use super::Transform; -use crate::error::Error; -use crate::pipeline::types::{GOAL_SELF_REFERENCE_RULE, TEMPLATE_UNDEFINED_VARIABLE_RULE}; - -/// How the template-expansion pass should treat undefined input variables. -/// -/// Both validate and run-create render structurally so they can report every -/// unbound `{{ inputs.* }}` variable in one pass rather than aborting on the -/// first. Run-create then promotes the resulting warnings to errors, which -/// keeps its hard-fail behavior. -#[derive(Clone, Copy, Debug)] -pub enum RenderMode { - /// Undefined inputs abort the pass with a hard error. No production caller - /// uses this today; run-create promotes structural warnings instead. - Strict, - /// Undefined inputs render as empty and become warning diagnostics on the - /// returned `Validated`, so structural lints still run. Used by - /// `fabro validate` and by run-create. - Structural, -} - -#[derive(Clone)] -pub(crate) struct TemplateRenderTarget { - pub source_name: Option, - pub node_id: Option, - pub edge: Option<(String, String)>, - pub owner: String, - /// Fix text for undefined variables outside `inputs`/`vars`, set by - /// targets whose template context is a restricted projection. - restricted_namespace_fix: Option, - source_origin: Option, - template_store: Option, -} - -#[derive(Clone)] -pub(crate) struct TemplateRenderStore { - source: TemplateSource, - store: Arc, -} - -impl TemplateRenderStore { - #[must_use] - pub(crate) fn new(source: TemplateSource, store: Arc) -> Self { - Self { source, store } - } - - fn render( - &self, - text: &str, - ctx: &TemplateContext, - mode: TemplateRenderMode, - origin: Option<&TemplateSourceOrigin>, - ) -> Result { - let mut source = match origin { - Some(origin) => self.source.clone().with_origin(origin.clone()), - None => self.source.clone(), - }; - text.clone_into(&mut source.content); - fabro_template::render_source(&source, ctx, Arc::clone(&self.store), mode) - } -} - -impl TemplateRenderTarget { - #[must_use] - pub(crate) fn graph_attr(source_name: Option, attr_name: impl Into) -> Self { - let attr_name = attr_name.into(); - Self { - source_name, - node_id: None, - edge: None, - owner: format!("graph attribute `{attr_name}`"), - restricted_namespace_fix: None, - source_origin: None, - template_store: None, - } - } - - #[must_use] - pub(crate) fn node_attr( - source_name: Option, - node_id: impl Into, - attr_name: impl Into, - ) -> Self { - let node_id = node_id.into(); - let attr_name = attr_name.into(); - Self { - source_name, - node_id: Some(node_id.clone()), - edge: None, - owner: format!("node `{node_id}` attribute `{attr_name}`"), - restricted_namespace_fix: None, - source_origin: None, - template_store: None, - } - } - - #[must_use] - pub(crate) fn edge_attr( - source_name: Option, - from: impl Into, - to: impl Into, - attr_name: impl Into, - ) -> Self { - let from = from.into(); - let to = to.into(); - let attr_name = attr_name.into(); - Self { - source_name, - node_id: None, - edge: Some((from.clone(), to.clone())), - owner: format!("edge `{from} -> {to}` attribute `{attr_name}`"), - restricted_namespace_fix: None, - source_origin: None, - template_store: None, - } - } - - #[must_use] - pub(crate) fn with_source_name(mut self, source_name: impl Into) -> Self { - self.source_name = Some(source_name.into()); - self - } - - #[must_use] - pub(crate) fn with_source_origin(mut self, source_text: Option<&str>, value: &str) -> Self { - self.source_origin = source_text.and_then(|source_text| { - TemplateSourceOrigin::from_first_fragment_match(source_text, value) - }); - self - } - - #[must_use] - pub(crate) fn with_template_store(mut self, template_store: TemplateRenderStore) -> Self { - self.template_store = Some(template_store); - self - } - - #[must_use] - pub(crate) fn with_restricted_namespace_fix(mut self, fix: impl Into) -> Self { - self.restricted_namespace_fix = Some(fix.into()); - self - } - - #[must_use] - fn template_source_name(&self) -> String { - self.source_name - .clone() - .unwrap_or_else(|| "workflow".to_string()) - } -} - -pub(crate) enum TemplateRenderOutcome { - Rendered(String), - Unresolved, -} - -pub(crate) fn render_template_for_target( - text: &str, - ctx: &TemplateContext, - render_mode: RenderMode, - target: &TemplateRenderTarget, - diagnostics: &mut Vec, -) -> Result { - match render_template_for_target_outcome(text, ctx, render_mode, target, diagnostics)? { - TemplateRenderOutcome::Rendered(rendered) => Ok(rendered), - TemplateRenderOutcome::Unresolved => { - render_template_with_mode(text, ctx, TemplateRenderMode::Lenient, target) - .map_err(|err| template_error_for_target(target, err)) - } - } -} - -pub(crate) fn render_template_for_target_outcome( - text: &str, - ctx: &TemplateContext, - render_mode: RenderMode, - target: &TemplateRenderTarget, - diagnostics: &mut Vec, -) -> Result { - match render_mode { - RenderMode::Strict => { - render_template_with_mode(text, ctx, TemplateRenderMode::Strict, target) - .map(TemplateRenderOutcome::Rendered) - .map_err(|err| template_error_for_target(target, err)) - } - RenderMode::Structural => { - match render_template_with_mode(text, ctx, TemplateRenderMode::Strict, target) { - Ok(rendered) => Ok(TemplateRenderOutcome::Rendered(rendered)), - Err(err @ TemplateError::UndefinedVariable { .. }) => { - diagnostics.push(template_diagnostic(&err, target)); - Ok(TemplateRenderOutcome::Unresolved) - } - Err(err) => Err(template_error_for_target(target, err)), - } - } - } -} - -fn render_template_with_mode( - text: &str, - ctx: &TemplateContext, - mode: TemplateRenderMode, - target: &TemplateRenderTarget, -) -> Result { - match target.template_store.as_ref() { - Some(template_store) => { - template_store.render(text, ctx, mode, target.source_origin.as_ref()) - } - None => fabro_template::render_named_with_origin( - target.template_source_name(), - text, - ctx, - mode, - target.source_origin.as_ref(), - ), - } -} - -fn template_error_for_target(target: &TemplateRenderTarget, err: TemplateError) -> Error { - let rendered = collect_chain(&err).join(": "); - Error::template( - format!("template expansion failed in {}: {rendered}", target.owner), - err, - ) -} - -fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) -> Diagnostic { - let expression = error.expression(); - let mut message = match expression { - Some(expr) => format!("undefined template variable `{expr}`"), - None => "undefined template variable".to_string(), - }; - let _ = write!(message, " in {}", target.owner); - - let location = error.location(); - - Diagnostic { - rule: TEMPLATE_UNDEFINED_VARIABLE_RULE.to_owned(), - severity: Severity::Warning, - message, - node_id: target.node_id.clone(), - edge: target.edge.clone(), - fix: Some(template_variable_fix(expression, target)), - source_path: location.source_name.or_else(|| target.source_name.clone()), - line: location.line, - column: location.column, - span_start: location.span_start, - span_len: location.span_len, - related: Vec::new(), - } -} - -fn template_variable_fix(expression: Option<&str>, target: &TemplateRenderTarget) -> String { - let mut parts = expression.unwrap_or_default().split('.'); - let namespace = parts.next().unwrap_or_default().parse::(); - let name = parts.next().unwrap_or(""); - - match (namespace, &target.restricted_namespace_fix) { - (Ok(Namespace::Inputs), _) => input_binding_fix(name), - (Ok(Namespace::Vars), _) => variable_binding_fix(name), - (_, Some(fix)) => fix.clone(), - (Ok(Namespace::Goal), None) => GOAL_BINDING_FIX.to_string(), - (Ok(namespace), None) => format!("`{namespace}` is not available in workflow templates"), - (Err(_), None) => { - format!( - "define `{}` in the template context", - expression.unwrap_or("the value") - ) - } - } -} - -fn input_binding_fix(name: &str) -> String { - format!("bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=`") -} - -fn variable_binding_fix(name: &str) -> String { - format!("set it with `fabro variable set {name} `") -} - -const GOAL_BINDING_FIX: &str = "set a graph `goal` on the workflow"; - -/// Substitutes `{{ goal }}`, `{{ inputs.* }}`, and `{{ vars.* }}` in one -/// command node `script`. -/// -/// Scripts interpolate through [`InterpString`] tokens rather than the -/// MiniJinja pass that renders prompts. Shell source is full of brace syntax -/// that must survive untouched — `jq` filters, `awk` programs, Go templates, -/// brace expansion — and `InterpString` claims only the bare `{{ goal }}` and -/// `{{ .NAME }}`, leaving everything else literal. -/// -/// `env` and `secrets` are deliberately not wired, so a token in either -/// namespace fails as [`ResolveErrorKind::Unavailable`]. A script reads the -/// environment with `$NAME`, which needs no interpolation, and a resolved -/// secret would be baked into the `CommandStarted` event that records the -/// script verbatim. -/// -/// Shell values are quoted as one argument. Python values are quoted as string -/// literals. In both languages the token must stand where one value is valid; -/// callers must not wrap it in another string literal. -fn interpolate_script<'a>( - text: &'a str, - ctx: &TemplateContext, - language: &str, - render_mode: RenderMode, - target: &TemplateRenderTarget, - diagnostics: &mut Vec, -) -> Result, Error> { - if !text.contains("{{") { - return Ok(Cow::Borrowed(text)); - } - - let parsed = InterpString::parse(text); - if parsed.is_literal() { - return Ok(Cow::Borrowed(text)); - } - - let mut resolve_ctx = ResolveCtx::new() - .with_inputs(|name| { - ctx.input(name) - .map(|value| quote_script_value(&value, language)) - }) - .with_vars(|name| { - ctx.var(name) - .map(|value| quote_script_value(&value, language)) - }); - // The graph goal is rendered before any node attribute, so by here it is - // the final text. Substituting it does not re-interpolate: whatever the - // goal contains lands in the script as literal characters. - if let Some(goal) = ctx.goal() { - resolve_ctx = resolve_ctx.with_goal(quote_script_value(goal, language)); - } - match parsed.resolve_with(&mut resolve_ctx) { - Ok(resolved) => Ok(Cow::Owned(resolved)), - // An unbound input or variable is the same authoring gap the prompt - // pass reports, so it follows the same mode split: a hard error at - // run-create, a diagnostic during `fabro validate`. - Err(err) if err.kind == ResolveErrorKind::Missing => match render_mode { - RenderMode::Strict => Err(script_interpolation_error(target, err, language)), - RenderMode::Structural => { - diagnostics.push(script_undefined_variable_diagnostic(&err, target)); - // Leave the script in source form. Validation never executes - // it, and showing the unresolved token beats emptying it. - Ok(Cow::Borrowed(text)) - } - }, - // An unsupported namespace can never resolve here, however the inputs - // are bound, so it fails in both modes — the same treatment - // `render_attrs` gives an invalid static reference. - Err(err) => Err(script_interpolation_error(target, err, language)), - } -} - -fn quote_script_value(value: &str, language: &str) -> String { - if language == "python" { - serde_json::to_string(value).expect("serializing a string to JSON should not fail") - } else { - shell::shell_quote(value) - } -} - -fn script_interpolation_error( - target: &TemplateRenderTarget, - source: ResolveError, - language: &str, -) -> Error { - let fix = script_interpolation_fix(&source, Some(language)); - Error::ScriptInterpolation { - owner: target.owner.clone(), - fix, - source, - } -} - -fn script_undefined_variable_diagnostic( - err: &ResolveError, - target: &TemplateRenderTarget, -) -> Diagnostic { - Diagnostic { - rule: TEMPLATE_UNDEFINED_VARIABLE_RULE.to_owned(), - severity: Severity::Warning, - message: format!("{err} in {}", target.owner), - node_id: target.node_id.clone(), - edge: target.edge.clone(), - fix: Some(script_interpolation_fix(err, None)), - source_path: target.source_name.clone(), - ..Diagnostic::default() - } -} - -fn script_interpolation_fix(err: &ResolveError, language: Option<&str>) -> String { - let name = &err.name; - match err.namespace { - Namespace::Inputs => input_binding_fix(name), - Namespace::Vars => variable_binding_fix(name), - Namespace::Env if language == Some("python") => format!( - "`script` does not interpolate environment variables; read it in Python as \ - `os.environ[\"{name}\"]` instead" - ), - Namespace::Env => format!( - "`script` does not interpolate environment variables; read it in the shell as \ - `${name}` instead" - ), - Namespace::Secrets if language == Some("python") => format!( - "`script` does not interpolate secrets; expose `{name}` to the sandbox through \ - `[environments..env]` and read it in Python as `os.environ[\"{name}\"]`" - ), - Namespace::Secrets => format!( - "`script` does not interpolate secrets; expose `{name}` to the sandbox through \ - `[environments..env]` and read it in the shell as `${name}`" - ), - Namespace::Goal => GOAL_BINDING_FIX.to_string(), - } -} - -const DETEMPLATED_ATTRIBUTE_RULE: &str = "detemplated_attribute"; - -/// Warning emitted when an attribute that is no longer a template still -/// contains template syntax — the syntax is now treated as literal text. -fn detemplated_attribute_diagnostic(attr_name: &str, target: &TemplateRenderTarget) -> Diagnostic { - Diagnostic { - rule: DETEMPLATED_ATTRIBUTE_RULE.to_owned(), - severity: Severity::Warning, - message: format!( - "`{attr_name}` in {} is no longer a template; `{{{{ … }}}}` / `{{% … %}}` is treated \ - as literal text. Node `prompt`, graph `goal`, and graph `model_stylesheet` support \ - templating. Node command `script` supports `{{{{ goal }}}}`, \ - `{{{{ inputs.* }}}}`, and `{{{{ vars.* }}}}` interpolation.", - target.owner - ), - node_id: target.node_id.clone(), - edge: target.edge.clone(), - fix: Some(format!( - "remove the template syntax from `{attr_name}`, or move the dynamic value into a \ - `prompt`/`goal`/`model_stylesheet`" - )), - source_path: target.source_name.clone(), - ..Diagnostic::default() - } -} - -/// Error emitted when the graph `goal` references `{{ goal }}` — a goal cannot -/// reference itself. Prompts may reference the rendered goal; the goal renders -/// without `goal` in scope, so a self-reference is always a mistake. -fn goal_self_reference_diagnostic( - target: &TemplateRenderTarget, - error: Option<&TemplateError>, -) -> Diagnostic { - let location = error.map(TemplateError::location).unwrap_or_default(); - Diagnostic { - rule: GOAL_SELF_REFERENCE_RULE.to_owned(), - severity: Severity::Error, - message: format!( - "the graph `goal` cannot reference itself (`{{{{ goal }}}}`) in {}", - target.owner - ), - node_id: target.node_id.clone(), - edge: target.edge.clone(), - fix: Some( - "remove the `{{ goal }}` reference from the goal; a node `prompt` can reference the \ - goal instead" - .to_string(), - ), - source_path: location.source_name.or_else(|| target.source_name.clone()), - line: location.line, - column: location.column, - span_start: location.span_start, - span_len: location.span_len, - ..Diagnostic::default() - } -} - -/// Renders graph goals and node prompts, and diagnoses template syntax in -/// attributes that do not support it. -pub struct TemplateTransform { - pub context: TemplateContext, - pub source_name: Option, - pub source_text: Option, - pub render_mode: RenderMode, -} - -impl TemplateTransform { - #[must_use] - pub fn new(inputs: HashMap) -> Self { - Self { - context: TemplateContext::new().with_inputs(inputs), - source_name: None, - source_text: None, - render_mode: RenderMode::Structural, - } - } - - pub(crate) fn resolved_goal( - &self, - graph: &Graph, - diagnostics: &mut Vec, - ) -> Result { - let goal = graph.goal(); - if let Some(reference) = goal.strip_prefix('@') { - validate_static_reference(reference, ReferenceKind::GraphGoalFile) - .map_err(|error| Error::Validation(error.to_string()))?; - return Ok(goal.to_string()); - } - let target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal") - .with_source_origin(self.source_text.as_deref(), goal); - // The goal renders with no `goal` in scope, so it cannot reference - // itself. Flag the self-reference with a friendly diagnostic before the - // render would otherwise produce a generic "undefined variable `goal`". - if fabro_template::references_top_level_variable(goal, "goal") { - let location_error = self.goal_self_reference_location(goal, &target); - diagnostics.push(goal_self_reference_diagnostic( - &target, - location_error.as_ref(), - )); - return Ok(goal.to_string()); - } - let ctx = self.context.clone(); - render_template_for_target(goal, &ctx, self.render_mode, &target, diagnostics) - } - - fn goal_self_reference_location( - &self, - goal: &str, - target: &TemplateRenderTarget, - ) -> Option { - let ctx = self.context.clone(); - match render_template_with_mode(goal, &ctx, TemplateRenderMode::Strict, target) { - Err(err @ TemplateError::UndefinedVariable { .. }) - if err.expression() == Some("goal") => - { - Some(err) - } - _ => None, - } - } - - fn render_attrs( - attrs: &mut HashMap, - ctx: &TemplateContext, - source_name: Option<&String>, - source_text: Option<&str>, - render_mode: RenderMode, - scope: AttributeScope, - owner_for_attr: impl Fn(&str) -> TemplateRenderTarget, - diagnostics: &mut Vec, - ) -> Result<(), Error> { - for (attr_name, value) in attrs { - if let AttrValue::String(text) = value { - // The graph `goal` is rendered separately and must not be - // re-rendered here. - if matches!(scope, AttributeScope::Graph) && attr_name == "goal" { - continue; - } - // The root model stylesheet has its own restricted template - // pass after imports are expanded. Imported stylesheets stay - // ignored and are diagnosed by ImportTransform. - if matches!(scope, AttributeScope::Graph) && attr_name == "model_stylesheet" { - continue; - } - if attr_name == "stack.child_dot_source" { - continue; - } - // Command scripts use narrow value interpolation in a separate - // one-shot transform after imports are expanded. - if matches!(scope, AttributeScope::Node) && attr_name == "script" { - continue; - } - if let Some(kind) = reference_kind_for_attribute(scope, attr_name, text) { - validate_static_reference(text, kind.into()) - .map_err(|error| Error::Validation(error.to_string()))?; - continue; - } - let target = owner_for_attr(attr_name) - .with_source_name(source_name.cloned().unwrap_or_else(|| "workflow".into())) - .with_source_origin(source_text, text); - if matches!(scope, AttributeScope::Node) && attr_name == "prompt" { - // `prompt` is the only node attribute rendered as a full - // MiniJinja template. - *text = - render_template_for_target(text, ctx, render_mode, &target, diagnostics)?; - } else if fabro_template::contains_template_syntax(text) { - // Every other attribute is no longer a template (`label`, - // `model`, `provider`, `speed`, `condition`, edge `label`, - // …): leave it literal and warn so authors can migrate. - diagnostics.push(detemplated_attribute_diagnostic(attr_name, &target)); - } - } - } - Ok(()) - } - - pub(crate) fn apply_with_diagnostics( - &self, - graph: Graph, - ) -> Result<(Graph, Vec), Error> { - let mut diagnostics = Vec::new(); - let mut graph = graph; - let resolved_goal = self.resolved_goal(&graph, &mut diagnostics)?; - graph - .attrs - .insert("goal".to_string(), AttrValue::String(resolved_goal.clone())); - let ctx = self.context.clone().with_goal(resolved_goal); - - Self::render_attrs( - &mut graph.attrs, - &ctx, - self.source_name.as_ref(), - self.source_text.as_deref(), - self.render_mode, - AttributeScope::Graph, - |attr_name| TemplateRenderTarget::graph_attr(self.source_name.clone(), attr_name), - &mut diagnostics, - )?; - for (node_id, node) in &mut graph.nodes { - Self::render_attrs( - &mut node.attrs, - &ctx, - self.source_name.as_ref(), - self.source_text.as_deref(), - self.render_mode, - AttributeScope::Node, - |attr_name| { - TemplateRenderTarget::node_attr( - self.source_name.clone(), - node_id.clone(), - attr_name, - ) - }, - &mut diagnostics, - )?; - } - for edge in &mut graph.edges { - let from = edge.from.clone(); - let to = edge.to.clone(); - Self::render_attrs( - &mut edge.attrs, - &ctx, - self.source_name.as_ref(), - self.source_text.as_deref(), - self.render_mode, - AttributeScope::Edge, - |attr_name| { - TemplateRenderTarget::edge_attr( - self.source_name.clone(), - from.clone(), - to.clone(), - attr_name, - ) - }, - &mut diagnostics, - )?; - } - - Ok((graph, diagnostics)) - } -} - -impl Transform for TemplateTransform { - fn apply(&self, graph: Graph) -> Result { - let (graph, diagnostics) = self.apply_with_diagnostics(graph)?; - if !diagnostics.is_empty() { - return Err(Error::ValidationFailed { diagnostics }); - } - Ok(graph) - } -} - -/// Interpolates command node scripts once, after import expansion is complete. -/// -/// Keeping this pass separate from [`TemplateTransform`] prevents imported -/// scripts from being scanned once in their source graph and again after they -/// are merged into the root graph. -pub struct ScriptInterpolationTransform { - pub context: TemplateContext, - pub source_name: Option, - pub render_mode: RenderMode, -} - -impl ScriptInterpolationTransform { - fn command_script_language(node: &Node) -> Option<&'static str> { - let is_command = matches!(node.handler_type(), Some("command" | "tool")); - is_command.then(|| { - if node.attrs.get("language").and_then(AttrValue::as_str) == Some("python") { - "python" - } else { - "shell" - } - }) - } - - pub(crate) fn apply_with_diagnostics( - &self, - graph: Graph, - ) -> Result<(Graph, Vec), Error> { - let mut graph = graph; - let mut diagnostics = Vec::new(); - let ctx = self.context.clone().with_goal(graph.goal().to_string()); - - for (node_id, node) in &mut graph.nodes { - let language = Self::command_script_language(node); - let Some(AttrValue::String(text)) = node.attrs.get_mut("script") else { - continue; - }; - let target = TemplateRenderTarget::node_attr( - self.source_name.clone(), - node_id.clone(), - "script", - ) - .with_source_name( - self.source_name - .clone() - .unwrap_or_else(|| "workflow".to_string()), - ); - - if let Some(language) = language { - if let Cow::Owned(resolved) = interpolate_script( - text, - &ctx, - language, - self.render_mode, - &target, - &mut diagnostics, - )? { - *text = resolved; - } - } else if fabro_template::contains_template_syntax(text) { - diagnostics.push(detemplated_attribute_diagnostic("script", &target)); - } - } - - Ok((graph, diagnostics)) - } -} - -impl Transform for ScriptInterpolationTransform { - fn apply(&self, graph: Graph) -> Result { - let (graph, diagnostics) = self.apply_with_diagnostics(graph)?; - if !diagnostics.is_empty() { - return Err(Error::ValidationFailed { diagnostics }); - } - Ok(graph) - } -} - -#[cfg(test)] -mod tests { - use std::collections::HashMap; - - use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; - - use super::*; - - #[test] - fn template_transform_renders_prompt_and_leaves_other_attrs_literal() { - let mut graph = Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - AttrValue::String("Fix bugs".to_string()), - ); - graph.attrs.insert( - "label".to_string(), - AttrValue::String("Workflow: {{ goal }}".to_string()), - ); - - let mut node = Node::new("plan"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Achieve: {{ goal }} now".to_string()), - ); - node.attrs.insert( - "label".to_string(), - AttrValue::String("{{ inputs.name }}".to_string()), - ); - graph.nodes.insert("plan".to_string(), node); - - graph.edges.push(Edge { - from: "start".to_string(), - to: "plan".to_string(), - attrs: HashMap::from([( - "label".to_string(), - AttrValue::String("{{ inputs.greeting }}".to_string()), - )]), - }); - - let transform = TemplateTransform::new(HashMap::from([ - ( - "name".to_string(), - toml::Value::String("Planner".to_string()), - ), - ( - "greeting".to_string(), - toml::Value::String("hello".to_string()), - ), - ])); - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - // `prompt` is the only templated attribute and is still rendered. - assert_eq!( - graph.nodes["plan"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Achieve: Fix bugs now") - ); - // `label` (node, graph, edge) is no longer a template: left literal. - assert_eq!( - graph.nodes["plan"].attrs.get("label"), - Some(&AttrValue::String("{{ inputs.name }}".to_string())) - ); - assert_eq!( - graph.attrs.get("label"), - Some(&AttrValue::String("Workflow: {{ goal }}".to_string())) - ); - assert_eq!( - graph.edges[0].attrs.get("label"), - Some(&AttrValue::String("{{ inputs.greeting }}".to_string())) - ); - // Each demoted `label` still containing template syntax warns. - let detemplated = diagnostics - .iter() - .filter(|d| d.rule == DETEMPLATED_ATTRIBUTE_RULE) - .count(); - assert_eq!( - detemplated, 3, - "expected a migration warning per demoted label, got: {diagnostics:?}" - ); - } - - /// Build a one-node graph whose `test` node carries `script`. - fn script_graph(script: &str) -> Graph { - let mut graph = Graph::new("test"); - graph - .attrs - .insert("goal".to_string(), AttrValue::String("Ship it".to_string())); - let mut node = Node::new("test"); - node.attrs.insert( - "shape".to_string(), - AttrValue::String("parallelogram".to_string()), - ); - node.attrs - .insert("script".to_string(), AttrValue::String(script.to_string())); - graph.nodes.insert("test".to_string(), node); - graph - } - - fn script_transform( - inputs: &[(&str, toml::Value)], - vars: &[(&str, &str)], - render_mode: RenderMode, - ) -> ScriptInterpolationTransform { - ScriptInterpolationTransform { - context: TemplateContext::new() - .with_inputs( - inputs - .iter() - .map(|(k, v)| ((*k).to_string(), v.clone())) - .collect(), - ) - .with_vars( - vars.iter() - .map(|(k, v)| ((*k).to_string(), (*v).to_string())) - .collect(), - ), - source_name: None, - render_mode, - } - } - - fn script_of(graph: &Graph) -> &str { - graph.nodes["test"] - .attrs - .get("script") - .and_then(AttrValue::as_str) - .expect("script attribute should still be a string") - } - - #[test] - fn script_interpolates_inputs_and_vars() { - let graph = script_graph("cargo test -p {{ inputs.crate }} --profile {{ vars.PROFILE }}"); - let transform = script_transform( - &[("crate", toml::Value::String("fabro-workflow".into()))], - &[("PROFILE", "ci")], - RenderMode::Structural, - ); - - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!( - script_of(&graph), - "cargo test -p fabro-workflow --profile ci" - ); - assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}"); - } - - #[test] - fn script_substitutes_the_rendered_goal() { - let graph = script_graph("gh pr create --title {{ goal }}"); - let transform = script_transform(&[], &[], RenderMode::Structural); - - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!(script_of(&graph), "gh pr create --title 'Ship it'"); - assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}"); - } - - #[test] - fn shell_script_quotes_substituted_values_as_one_argument() { - let graph = script_graph("deploy --release {{ inputs.release }}"); - let transform = script_transform( - &[( - "release", - toml::Value::String("stable; touch /tmp/pwned".to_string()), - )], - &[], - RenderMode::Strict, - ); - - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!( - script_of(&graph), - "deploy --release 'stable; touch /tmp/pwned'" - ); - assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}"); - } - - #[test] - fn python_script_quotes_substituted_values_as_string_literals() { - let mut graph = script_graph("print({{ inputs.value }})"); - graph.nodes.get_mut("test").unwrap().attrs.insert( - "language".to_string(), - AttrValue::String("python".to_string()), - ); - let transform = script_transform( - &[( - "value", - toml::Value::String("'); __import__('os').system('id'); #".to_string()), - )], - &[], - RenderMode::Strict, - ); - - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!( - script_of(&graph), - r#"print("'); __import__('os').system('id'); #")"# - ); - assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}"); - } - - /// The reason `script` uses `InterpString` rather than MiniJinja: shell - /// source is full of brace syntax that must reach the shell untouched. - #[test] - fn script_leaves_non_token_braces_literal() { - let script = "jq '{name: .name}' f.json | awk '{print $1}'; echo {{ .Values.image }}; \ - touch {a,b}.txt; {% raw %}"; - let graph = script_graph(script); - let transform = script_transform(&[], &[], RenderMode::Structural); - - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!(script_of(&graph), script); - assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}"); - } - - #[test] - fn script_rejects_secrets_tokens_in_both_render_modes() { - for render_mode in [RenderMode::Structural, RenderMode::Strict] { - let graph = script_graph("curl -H \"Authorization: {{ secrets.API_KEY }}\" $URL"); - let transform = script_transform(&[], &[], render_mode); - - let err = transform - .apply_with_diagnostics(graph) - .expect_err("secrets must never interpolate into a script"); - - let message = err.to_string(); - assert!( - message.contains("does not interpolate secrets"), - "unexpected message: {message}" - ); - assert!( - message.contains("$API_KEY"), - "should point at the shell alternative: {message}" - ); - } - } - - #[test] - fn script_rejects_env_tokens_and_points_at_shell_expansion() { - let graph = script_graph("echo {{ env.HOME }}"); - let transform = script_transform(&[], &[], RenderMode::Structural); - - let err = transform - .apply_with_diagnostics(graph) - .expect_err("env is not interpolated in a script"); - - let message = err.to_string(); - assert!(message.contains("$HOME"), "unexpected message: {message}"); - } - - #[test] - fn script_missing_input_warns_and_preserves_source_in_structural_mode() { - let script = "cargo test -p {{ inputs.crate }}"; - let graph = script_graph(script); - let transform = script_transform(&[], &[], RenderMode::Structural); - - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!(script_of(&graph), script); - let diagnostic = diagnostics - .iter() - .find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) - .expect("an unbound input should warn"); - assert_eq!(diagnostic.severity, Severity::Warning); - assert_eq!(diagnostic.node_id.as_deref(), Some("test")); - assert!( - diagnostic - .fix - .as_deref() - .unwrap_or_default() - .contains("--input crate="), - "unexpected fix: {:?}", - diagnostic.fix - ); - } - - #[test] - fn script_missing_input_is_an_error_in_strict_mode() { - let graph = script_graph("cargo test -p {{ inputs.crate }}"); - let transform = script_transform(&[], &[], RenderMode::Strict); - - let err = transform - .apply_with_diagnostics(graph) - .expect_err("strict mode must reject an unbound input"); - - assert!( - err.to_string().contains("inputs.crate"), - "unexpected message: {err}" - ); - let source = std::error::Error::source(&err) - .expect("script interpolation errors should preserve ResolveError as their source"); - assert!( - source.to_string().contains("inputs.crate"), - "unexpected source: {source}" - ); - } - - /// A typed input must produce the same text in a script as in a prompt, so - /// authors do not have to reason about two stringification rules. - #[test] - fn script_and_prompt_stringify_typed_inputs_identically() { - let mut graph = - script_graph("retry --times {{ inputs.attempts }} --fast {{ inputs.fast }}"); - graph.nodes.get_mut("test").unwrap().attrs.insert( - "prompt".to_string(), - AttrValue::String( - "retry --times {{ inputs.attempts }} --fast {{ inputs.fast }}".to_string(), - ), - ); - let context = TemplateContext::new().with_inputs(HashMap::from([ - ("attempts".to_string(), toml::Value::Integer(3)), - ("fast".to_string(), toml::Value::Boolean(true)), - ])); - let (graph, _) = TemplateTransform { - context: context.clone(), - source_name: None, - source_text: None, - render_mode: RenderMode::Structural, - } - .apply_with_diagnostics(graph) - .unwrap(); - let (graph, _) = ScriptInterpolationTransform { - context, - source_name: None, - render_mode: RenderMode::Structural, - } - .apply_with_diagnostics(graph) - .unwrap(); - - assert_eq!(script_of(&graph), "retry --times 3 --fast true"); - assert_eq!( - graph.nodes["test"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some(script_of(&graph)), - ); - } - - /// `script` is node-scoped. A graph or edge attribute of the same name is - /// not a command node script and keeps the demoted-attribute behavior. - #[test] - fn script_interpolation_is_node_scoped() { - let mut graph = script_graph("echo ok"); - graph.attrs.insert( - "script".to_string(), - AttrValue::String("echo {{ inputs.crate }}".to_string()), - ); - let (graph, mut diagnostics) = TemplateTransform::new(HashMap::new()) - .apply_with_diagnostics(graph) - .unwrap(); - let transform = script_transform(&[], &[], RenderMode::Structural); - let (graph, script_diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - diagnostics.extend(script_diagnostics); - - assert_eq!( - graph.attrs.get("script"), - Some(&AttrValue::String("echo {{ inputs.crate }}".to_string())) - ); - assert!( - diagnostics - .iter() - .any(|d| d.rule == DETEMPLATED_ATTRIBUTE_RULE), - "graph-scope `script` should still warn: {diagnostics:?}" - ); - } - - #[test] - fn non_command_node_script_stays_literal() { - let mut graph = script_graph("echo {{ inputs.value }}"); - graph - .nodes - .get_mut("test") - .unwrap() - .attrs - .insert("shape".to_string(), AttrValue::String("box".to_string())); - let transform = script_transform( - &[("value", toml::Value::String("changed".to_string()))], - &[], - RenderMode::Structural, - ); - - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!(script_of(&graph), "echo {{ inputs.value }}"); - assert!( - diagnostics - .iter() - .any(|diagnostic| diagnostic.rule == DETEMPLATED_ATTRIBUTE_RULE), - "non-command scripts should keep the literal-template warning: {diagnostics:?}" - ); - } - - #[test] - fn template_transform_leaves_non_string_attrs_unchanged() { - let mut graph = Graph::new("test"); - let mut node = Node::new("plan"); - node.attrs - .insert("max_retries".to_string(), AttrValue::Integer(3)); - graph.nodes.insert("plan".to_string(), node); - - let transform = TemplateTransform::new(HashMap::new()); - let graph = transform.apply(graph).unwrap(); - - assert_eq!( - graph.nodes["plan"].attrs.get("max_retries"), - Some(&AttrValue::Integer(3)) - ); - } - - #[test] - fn template_transform_supports_empty_goal() { - let mut graph = Graph::new("test"); - let mut node = Node::new("plan"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Goal: {{ goal }}".to_string()), - ); - graph.nodes.insert("plan".to_string(), node); - - let transform = TemplateTransform::new(HashMap::new()); - let graph = transform.apply(graph).unwrap(); - - let prompt = graph.nodes["plan"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str) - .unwrap(); - assert_eq!(prompt, "Goal: "); - } - - #[test] - fn template_transform_rejects_goal_self_reference() { - let source = r#"digraph Test { - graph [goal="Improve on {{ goal }}"] - }"#; - let mut graph = Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - AttrValue::String("Improve on {{ goal }}".to_string()), - ); - let mut node = Node::new("plan"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Work: {{ goal }}".to_string()), - ); - graph.nodes.insert("plan".to_string(), node); - - let transform = TemplateTransform { - context: TemplateContext::new(), - source_name: Some("workflow.fabro".to_string()), - source_text: Some(source.to_string()), - render_mode: RenderMode::Structural, - }; - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - let self_ref: Vec<_> = diagnostics - .iter() - .filter(|d| d.rule == GOAL_SELF_REFERENCE_RULE) - .collect(); - assert_eq!( - self_ref.len(), - 1, - "expected one goal_self_reference diagnostic" - ); - assert_eq!(self_ref[0].severity, Severity::Error); - assert!(self_ref[0].message.contains("cannot reference itself")); - assert_eq!(self_ref[0].source_path.as_deref(), Some("workflow.fabro")); - assert_eq!(self_ref[0].line, Some(2)); - assert!(self_ref[0].span_start.is_some()); - assert_eq!( - graph.attrs.get("goal").and_then(AttrValue::as_str), - Some("Improve on {{ goal }}") - ); - } - - #[test] - fn template_transform_warns_on_undefined_variable() { - let mut graph = Graph::new("test"); - let mut node = Node::new("plan"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("{{ inputs.missing }}".to_string()), - ); - graph.nodes.insert("plan".to_string(), node); - - let transform = TemplateTransform::new(HashMap::new()); - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - let prompt = graph.nodes["plan"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str) - .unwrap(); - assert_eq!(prompt, ""); - assert_eq!(diagnostics.len(), 1); - let diag = &diagnostics[0]; - assert_eq!(diag.rule, "template_undefined_variable"); - assert!( - diag.message.contains("inputs.missing"), - "message: {}", - diag.message - ); - assert!( - diag.message.contains("in node `plan`"), - "message: {}", - diag.message - ); - assert_eq!(diag.node_id.as_deref(), Some("plan")); - } - - #[test] - fn template_transform_renders_graph_goal_once_before_other_attrs() { - let mut graph = Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - AttrValue::String("Demo {{ inputs.app_dir }}".to_string()), - ); - let mut node = Node::new("plan"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Goal: {{ goal }}".to_string()), - ); - graph.nodes.insert("plan".to_string(), node); - - let transform = TemplateTransform::new(HashMap::new()); - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!( - graph.attrs.get("goal").and_then(AttrValue::as_str), - Some("Demo ") - ); - assert_eq!( - graph.nodes["plan"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Goal: Demo ") - ); - assert_eq!(diagnostics.len(), 1); - assert_eq!(diagnostics[0].rule, "template_undefined_variable"); - assert_eq!(diagnostics[0].node_id, None); - } - - #[test] - fn template_transform_does_not_rerender_goal_output() { - let mut graph = Graph::new("test"); - graph.attrs.insert( - "goal".to_string(), - AttrValue::String("Demo {{ inputs.literal }}".to_string()), - ); - let mut node = Node::new("plan"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Goal: {{ goal }}".to_string()), - ); - graph.nodes.insert("plan".to_string(), node); - - let transform = TemplateTransform::new(HashMap::from([( - "literal".to_string(), - toml::Value::String("{{ inputs.should_not_render }}".to_string()), - )])); - let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert!(diagnostics.is_empty()); - assert_eq!( - graph.attrs.get("goal").and_then(AttrValue::as_str), - Some("Demo {{ inputs.should_not_render }}") - ); - assert_eq!( - graph.nodes["plan"] - .attrs - .get("prompt") - .and_then(AttrValue::as_str), - Some("Goal: Demo {{ inputs.should_not_render }}") - ); - } - - #[test] - fn template_transform_rejects_templated_child_workflow_path() { - let mut graph = Graph::new("test"); - let mut node = Node::new("child"); - node.attrs.insert( - "stack.child_workflow".to_string(), - AttrValue::String("../{{ inputs.child }}/workflow.fabro".to_string()), - ); - graph.nodes.insert("child".to_string(), node); - - let err = TemplateTransform::new(HashMap::new()) - .apply(graph) - .unwrap_err(); - assert!( - err.to_string() - .contains("templates are not supported in child workflow references"), - "unexpected error: {err}" - ); - } - - #[test] - fn template_transform_hard_fails_on_syntax_error() { - let mut graph = Graph::new("test"); - let mut node = Node::new("plan"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("Do {{ unterminated".to_string()), - ); - graph.nodes.insert("plan".to_string(), node); - - let err = TemplateTransform::new(HashMap::new()) - .apply(graph) - .unwrap_err(); - assert!( - err.to_string().contains("template syntax error"), - "unexpected error: {err}" - ); - } - - #[test] - fn template_transform_reports_structural_diagnostics_with_owner_context() { - let mut graph = Graph::new("test"); - let mut node = Node::new("plan"); - node.attrs.insert( - "prompt".to_string(), - AttrValue::String("{{ inputs.missing }}".to_string()), - ); - graph.nodes.insert("plan".to_string(), node); - - let transform = TemplateTransform { - context: TemplateContext::new(), - source_name: Some("workflow.fabro".to_string()), - source_text: None, - render_mode: RenderMode::Structural, - }; - let (_, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); - - assert_eq!(diagnostics.len(), 1); - assert_eq!(diagnostics[0].node_id.as_deref(), Some("plan")); - assert_eq!( - diagnostics[0].source_path.as_deref(), - Some("workflow.fabro") - ); - assert!( - diagnostics[0] - .message - .contains("node `plan` attribute `prompt`") - ); - } -}