diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index 3a3e89890..35891f419 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -19,13 +19,14 @@ use fabro_config::{ }; use fabro_graphviz::graph::AttrValue; use fabro_graphviz::parser; +use fabro_template::validate_static_reference; +use fabro_types::graph::ReferenceKind; use fabro_types::settings::interp::InterpString; use fabro_types::settings::run::{ApprovalMode, ResolvedGoalSource, ResolvedRunGoal, RunMode}; use fabro_types::{DirtyStatus, GitContext, ManifestPath, WorkflowSettings}; use fabro_workflow::git::{ GitSyncStatus, branch_needs_push, head_sha, push_branch_noninteractive, sync_status, }; -use fabro_workflow::static_reference::ReferenceKind; use crate::workflow_bundler::WorkflowBundler; @@ -265,8 +266,7 @@ fn resolve_manifest_goal( return Ok(None); }; if let Some(reference) = goal.strip_prefix('@') { - ReferenceKind::GraphGoalFile - .validate(reference) + validate_static_reference(reference, ReferenceKind::GraphGoalFile) .map_err(anyhow::Error::new)?; let goal_path = normalize_absolute_path( root_dot_path.parent().unwrap_or_else(|| Path::new(".")), diff --git a/lib/components/fabro-manifest/src/workflow_bundler.rs b/lib/components/fabro-manifest/src/workflow_bundler.rs index 61633c229..6f6b8dd9e 100644 --- a/lib/components/fabro-manifest/src/workflow_bundler.rs +++ b/lib/components/fabro-manifest/src/workflow_bundler.rs @@ -6,14 +6,14 @@ use anyhow::{Context as _, Result, anyhow}; use fabro_api::types; use fabro_config::project::WorkflowLocation; use fabro_config::{EnvironmentDockerfileLayer, EnvironmentImageLayer, SettingsLayer}; -use fabro_graphviz::graph::AttrValue; use fabro_graphviz::parser; use fabro_template::{ - BundleTemplateStore, FilesystemTemplateStore, RecordingTemplateStore, TemplateContext, - TemplateDependencyClosure, TemplateRenderMode, TemplateSource, + BundleTemplateStore, FilesystemTemplateStore, GraphReference, GraphReferenceError, + RecordingTemplateStore, TemplateContext, TemplateDependencyClosure, TemplateRenderMode, + TemplateSource, validate_static_reference, visit_graph_references, }; use fabro_types::ManifestPath; -use fabro_workflow::static_reference::{self, AttributeScope, ReferenceKind}; +use fabro_types::graph::ReferenceKind; use crate::{manifest_path_from_absolute, normalize_absolute_path}; @@ -132,117 +132,91 @@ impl<'a> WorkflowBundler<'a> { .unwrap_or_else(|| Path::new(".")); let workflow_template_root = manifest_parent_or_dot(&workflow.dot_path)?; - if let Some(goal_ref) = graph.attrs.get("goal").and_then(AttrValue::as_str) { - if goal_ref.starts_with('@') { - let bundled = self.collect_bundled_file( - files, - workflow_base_dir, - goal_ref.trim_start_matches('@'), - types::ManifestFileRefType::FileInline, - manifest_attr_reference_kind(AttributeScope::Graph, "goal", goal_ref)?, - Some(workflow.dot_path.clone()), - )?; - self.collect_bundled_template_includes(files, &bundled, &workflow_template_root)?; - } else { - self.collect_template_include_files( + // Imports and child workflows require a mutable borrow of self, so + // collect them during the walk and recurse after the visitor returns. + let mut imports = Vec::new(); + let mut children = Vec::new(); + + visit_graph_references(&graph, |reference| -> Result<()> { + match reference { + GraphReference::GoalFile { reference } => { + let bundled = self.collect_bundled_file( + files, + workflow_base_dir, + reference, + types::ManifestFileRefType::FileInline, + ReferenceKind::GraphGoalFile, + Some(workflow.dot_path.clone()), + )?; + self.collect_bundled_template_includes(files, &bundled, &workflow_template_root) + } + GraphReference::GoalInline { content } + | GraphReference::InlinePrompt { content } => self.collect_template_include_files( files, TemplateSource::new( workflow.dot_path.clone(), workflow_template_root.clone(), - goal_ref.to_owned(), + content.to_owned(), ), Some(&workflow.dot_path), - )?; + ), + GraphReference::FileInline { key, reference } => { + let bundled = self.collect_bundled_file( + files, + workflow_base_dir, + reference, + types::ManifestFileRefType::FileInline, + ReferenceKind::FileInline, + Some(workflow.dot_path.clone()), + )?; + if key == "prompt" { + self.collect_bundled_template_includes( + files, + &bundled, + &workflow_template_root, + )?; + } + Ok(()) + } + GraphReference::Import { reference } => { + let imported = self.collect_bundled_file( + files, + workflow_base_dir, + reference, + types::ManifestFileRefType::Import, + ReferenceKind::Import, + Some(workflow.dot_path.clone()), + )?; + imports.push(imported); + Ok(()) + } + GraphReference::ChildWorkflow { reference } => { + children.push(reference); + Ok(()) + } + } + }) + .map_err(|error| match error { + GraphReferenceError::StaticReference(source) => anyhow::Error::new(source), + GraphReferenceError::Visit(error) => error, + })?; + + for imported in imports { + if visited_imports.insert(imported.path.to_string()) { + let imported_source = std::fs::read_to_string(&imported.absolute_path) + .with_context(|| { + format!("Failed to read {}", imported.absolute_path.display()) + })?; + let imported_scan = WorkflowScanInput { + absolute_dot_path: imported.absolute_path, + dot_path: imported.path, + source: imported_source, + }; + self.collect_workflow_files(&imported_scan, files, visited_imports)?; } } - - for node in graph.nodes.values() { - if let Some(prompt_ref) = node.attrs.get("prompt").and_then(AttrValue::as_str) { - if !prompt_ref.starts_with('@') { - self.collect_template_include_files( - files, - TemplateSource::new( - workflow.dot_path.clone(), - workflow_template_root.clone(), - prompt_ref.to_owned(), - ), - Some(&workflow.dot_path), - )?; - } - } - - for (name, value) in &node.attrs { - let Some(value) = value.as_str() else { - continue; - }; - let Some(ReferenceKind::FileInline) = - static_reference::reference_kind_for_attribute( - AttributeScope::Node, - name, - value, - ) - else { - continue; - }; - let reference = value.strip_prefix('@').ok_or_else(|| { - anyhow!("file inline reference must start with '@': {name}={value}") - })?; - let bundled = self.collect_bundled_file( - files, - workflow_base_dir, - reference, - types::ManifestFileRefType::FileInline, - ReferenceKind::FileInline, - Some(workflow.dot_path.clone()), - )?; - - if name == "prompt" { - self.collect_bundled_template_includes( - files, - &bundled, - &workflow_template_root, - )?; - } - } - - if let Some(import_ref) = node.attrs.get("import").and_then(AttrValue::as_str) { - let imported = self.collect_bundled_file( - files, - workflow_base_dir, - import_ref, - types::ManifestFileRefType::Import, - manifest_attr_reference_kind(AttributeScope::Node, "import", import_ref)?, - Some(workflow.dot_path.clone()), - )?; - let import_key = imported.path.to_string(); - if visited_imports.insert(import_key) { - let imported_source = std::fs::read_to_string(&imported.absolute_path) - .with_context(|| { - format!("Failed to read {}", imported.absolute_path.display()) - })?; - let imported_scan = WorkflowScanInput { - absolute_dot_path: imported.absolute_path, - dot_path: imported.path, - source: imported_source, - }; - self.collect_workflow_files(&imported_scan, files, visited_imports)?; - } - } - - if let Some(child_ref) = node - .attrs - .get("stack.child_workflow") - .and_then(AttrValue::as_str) - { - manifest_attr_reference_kind( - AttributeScope::Node, - "stack.child_workflow", - child_ref, - )? - .validate(child_ref) - .map_err(anyhow::Error::new)?; - self.collect_workflow_entry(Path::new(child_ref), workflow_base_dir)?; - } + for child in children { + self.collect_workflow_entry(Path::new(child), workflow_base_dir)?; } Ok(()) @@ -347,21 +321,8 @@ impl<'a> WorkflowBundler<'a> { .parent() .unwrap_or_else(|| Path::new(".")); - for environment in layer.environments.values() { - self.collect_environment_dockerfile( - files, - base_dir, - config_path, - environment.image.as_ref(), - )?; - } - if let Some(run_environment) = layer.run.as_ref().and_then(|run| run.environment.as_ref()) { - self.collect_environment_dockerfile( - files, - base_dir, - config_path, - run_environment.image.as_ref(), - )?; + for image in layer.environment_images() { + self.collect_environment_dockerfile(files, base_dir, config_path, image)?; } Ok(()) } @@ -371,10 +332,9 @@ impl<'a> WorkflowBundler<'a> { files: &mut HashMap, base_dir: &Path, config_path: &ManifestPath, - image: Option<&EnvironmentImageLayer>, + image: &EnvironmentImageLayer, ) -> Result<()> { - let dockerfile = image.and_then(|image| image.dockerfile.as_ref()); - let Some(EnvironmentDockerfileLayer::Path { path }) = dockerfile else { + let Some(EnvironmentDockerfileLayer::Path { path }) = image.dockerfile.as_ref() else { return Ok(()); }; self.collect_bundled_file( @@ -397,9 +357,7 @@ impl<'a> WorkflowBundler<'a> { reference_kind: ReferenceKind, from: Option, ) -> Result { - reference_kind - .validate(reference) - .map_err(anyhow::Error::new)?; + validate_static_reference(reference, reference_kind).map_err(anyhow::Error::new)?; let absolute_path = normalize_absolute_path(base_dir, reference) .ok_or_else(|| anyhow!("unsupported manifest reference: {reference}"))?; @@ -463,15 +421,6 @@ fn manifest_path_is_within_root(path: &ManifestPath, root: &ManifestPath) -> boo path.starts_with(root) } -fn manifest_attr_reference_kind( - scope: AttributeScope, - key: &str, - value: &str, -) -> Result { - static_reference::reference_kind_for_attribute(scope, key, value) - .ok_or_else(|| anyhow!("unsupported manifest reference attribute: {key}={value}")) -} - #[cfg(test)] mod tests { use super::*; diff --git a/lib/components/fabro-workflow/src/handler/manager_loop.rs b/lib/components/fabro-workflow/src/handler/manager_loop.rs index 5668954c6..8cc82f634 100644 --- a/lib/components/fabro-workflow/src/handler/manager_loop.rs +++ b/lib/components/fabro-workflow/src/handler/manager_loop.rs @@ -6,7 +6,9 @@ use std::time::Duration; use async_trait::async_trait; use fabro_graphviz::graph::{AttrValue, Graph, Node}; use fabro_store::{ArtifactStore, Database}; +use fabro_template::validate_static_reference; use fabro_types::WorkflowSettings; +use fabro_types::graph::ReferenceKind; use object_store::memory::InMemory; use tokio::fs; use tokio::time::{sleep, timeout}; @@ -20,7 +22,6 @@ use crate::operations::{ValidateInput, WorkflowInput, validate_with_catalog}; use crate::outcome::{Outcome, OutcomeExt, StageOutcome}; use crate::pipeline::types::Initialized; use crate::run_options::RunOptions; -use crate::static_reference::{ReferenceKind, validate_static_reference}; use crate::{ManifestPath, pipeline, stage_scope}; /// Orchestrates a child workflow engine, polling for completion or stop diff --git a/lib/components/fabro-workflow/src/lib.rs b/lib/components/fabro-workflow/src/lib.rs index bd7f3e665..c34bec62c 100644 --- a/lib/components/fabro-workflow/src/lib.rs +++ b/lib/components/fabro-workflow/src/lib.rs @@ -332,7 +332,6 @@ pub(crate) mod sandbox_git_runtime; pub mod services; pub(crate) mod stage_execution; mod stage_scope; -pub mod static_reference; pub mod steering_hub; #[cfg(any(test, feature = "test-support"))] pub mod test_support; diff --git a/lib/components/fabro-workflow/src/static_reference.rs b/lib/components/fabro-workflow/src/static_reference.rs deleted file mode 100644 index af83539eb..000000000 --- a/lib/components/fabro-workflow/src/static_reference.rs +++ /dev/null @@ -1,143 +0,0 @@ -use std::fmt; - -use fabro_template::contains_template_syntax; -use thiserror::Error; - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum ReferenceKind { - FileInline, - Import, - ChildWorkflow, - Dockerfile, - GraphGoalFile, -} - -impl fmt::Display for ReferenceKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let label = match self { - Self::FileInline => "file inline reference", - Self::Import => "import reference", - Self::ChildWorkflow => "child workflow reference", - Self::Dockerfile => "Dockerfile reference", - Self::GraphGoalFile => "graph goal file reference", - }; - f.write_str(label) - } -} - -impl ReferenceKind { - pub fn validate(self, value: &str) -> Result<(), StaticReferenceError> { - validate_static_reference(value, self) - } -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum AttributeScope { - Graph, - Node, - Edge, -} - -#[derive(Debug, Error)] -#[error("templates are not supported in {kind}s: {value}")] -pub struct StaticReferenceError { - kind: ReferenceKind, - value: String, -} - -impl StaticReferenceError { - #[must_use] - pub fn new(kind: ReferenceKind, value: impl Into) -> Self { - Self { - kind, - value: value.into(), - } - } - - #[must_use] - pub fn kind(&self) -> ReferenceKind { - self.kind - } - - #[must_use] - pub fn value(&self) -> &str { - &self.value - } -} - -pub fn validate_static_reference( - value: &str, - kind: ReferenceKind, -) -> Result<(), StaticReferenceError> { - if contains_template_syntax(value) { - return Err(StaticReferenceError::new(kind, value)); - } - Ok(()) -} - -#[must_use] -pub fn reference_kind_for_attribute( - scope: AttributeScope, - key: &str, - value: &str, -) -> Option { - match key { - "import" => Some(ReferenceKind::Import), - "stack.child_workflow" | "stack.child_dotfile" => Some(ReferenceKind::ChildWorkflow), - "goal" if matches!(scope, AttributeScope::Graph) && value.starts_with('@') => { - Some(ReferenceKind::GraphGoalFile) - } - "prompt" | "output_schema" - if matches!(scope, AttributeScope::Node) && value.starts_with('@') => - { - Some(ReferenceKind::FileInline) - } - _ => None, - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn output_schema_at_value_is_file_inline_reference() { - assert_eq!( - reference_kind_for_attribute( - AttributeScope::Node, - "output_schema", - "@schemas/result.schema.json", - ), - Some(ReferenceKind::FileInline), - ); - } - - #[test] - fn output_schema_builtin_keyword_is_not_file_inline_reference() { - assert_eq!( - reference_kind_for_attribute(AttributeScope::Node, "output_schema", "routing"), - None, - ); - } - - #[test] - fn output_schema_reference_rejects_template_syntax() { - let error = reference_kind_for_attribute( - AttributeScope::Node, - "output_schema", - "@schemas/{{ inputs.schema }}.json", - ) - .expect("output_schema @ references should be static references") - .validate("@schemas/{{ inputs.schema }}.json") - .unwrap_err(); - - assert_eq!(error.kind(), ReferenceKind::FileInline); - assert_eq!(error.value(), "@schemas/{{ inputs.schema }}.json"); - assert!( - error - .to_string() - .contains("templates are not supported in file inline references"), - "unexpected error: {error}", - ); - } -} diff --git a/lib/components/fabro-workflow/src/transforms/import.rs b/lib/components/fabro-workflow/src/transforms/import.rs index e0082ba9d..bab7ee1b1 100644 --- a/lib/components/fabro-workflow/src/transforms/import.rs +++ b/lib/components/fabro-workflow/src/transforms/import.rs @@ -4,14 +4,14 @@ use std::sync::Arc; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_graphviz::parser; -use fabro_template::TemplateContext; +use fabro_template::{TemplateContext, validate_static_reference}; +use fabro_types::graph::ReferenceKind; use fabro_validate::Diagnostic; use super::file_inlining::template_render_store; use super::{FileInliningTransform, Transform}; use crate::error::Error; use crate::file_resolver::{FileResolver, ResolvedFile}; -use crate::static_reference::{ReferenceKind, validate_static_reference}; use crate::transforms::variable_expansion::{ RenderMode, TemplateRenderTarget, TemplateTransform, render_template_for_target, }; diff --git a/lib/components/fabro-workflow/src/transforms/importable_field.rs b/lib/components/fabro-workflow/src/transforms/importable_field.rs index acfcc52e8..cf4b5c2d1 100644 --- a/lib/components/fabro-workflow/src/transforms/importable_field.rs +++ b/lib/components/fabro-workflow/src/transforms/importable_field.rs @@ -13,8 +13,10 @@ //! [`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; -use crate::static_reference::{ReferenceKind, validate_static_reference}; /// A field value that is either inline content or an `@path` file import. /// diff --git a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs index e2c11d169..da7aba76d 100644 --- a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs +++ b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs @@ -6,8 +6,9 @@ use std::sync::Arc; use fabro_graphviz::graph::{AttrValue, Graph, Node}; use fabro_template::{ TemplateContext, TemplateError, TemplateRenderMode, TemplateSource, TemplateSourceOrigin, - TemplateStore, + TemplateStore, validate_static_reference, }; +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; @@ -17,9 +18,6 @@ use fabro_validate::{Diagnostic, Severity}; use super::Transform; use crate::error::Error; use crate::pipeline::types::{GOAL_SELF_REFERENCE_RULE, TEMPLATE_UNDEFINED_VARIABLE_RULE}; -use crate::static_reference::{ - AttributeScope, ReferenceKind, reference_kind_for_attribute, validate_static_reference, -}; /// How the template-expansion pass should treat undefined input variables. /// diff --git a/lib/foundation/fabro-template/src/lib.rs b/lib/foundation/fabro-template/src/lib.rs index f312e9416..8381f5fec 100644 --- a/lib/foundation/fabro-template/src/lib.rs +++ b/lib/foundation/fabro-template/src/lib.rs @@ -8,6 +8,7 @@ use minijinja::value::{Object, Value}; use minijinja::{AutoEscape, Environment, ErrorKind, UndefinedBehavior}; mod dependency; +mod static_reference; mod store; pub use dependency::{ @@ -15,6 +16,10 @@ pub use dependency::{ TemplateDependencyKind, TemplateDiscoveryError, discover_static_dependency_closure, extract_template_dependencies, }; +pub use static_reference::{ + GraphReference, GraphReferenceError, StaticReferenceError, validate_static_reference, + visit_graph_references, +}; pub use store::{ BundleTemplateStore, CachedTemplateStore, FilesystemTemplateStore, RecordingTemplateStore, TemplateIncludeResolver, TemplateLoadError, TemplateSource, TemplateSourceOrigin, diff --git a/lib/foundation/fabro-template/src/static_reference.rs b/lib/foundation/fabro-template/src/static_reference.rs new file mode 100644 index 000000000..b832f2956 --- /dev/null +++ b/lib/foundation/fabro-template/src/static_reference.rs @@ -0,0 +1,248 @@ +//! Static file references in workflow graphs. +//! +//! Workflow graphs name other files through a fixed attribute vocabulary +//! (`import`, `stack.child_workflow`, `@`-prefixed `prompt`/`output_schema` +//! values, and the graph `goal`). These references are *static*: they may not +//! contain template syntax, because they are resolved before any template +//! rendering happens. +//! +//! [`visit_graph_references`] is the one walker over that vocabulary. The +//! manifest bundler and workflow-version validation both consume it, so a new +//! reference-bearing attribute is added here once instead of drifting between +//! per-crate walkers. + +use fabro_types::graph::{AttributeScope, Graph, ReferenceKind, reference_kind_for_attribute}; + +use crate::contains_template_syntax; + +/// A static file reference that unexpectedly contains template syntax. +#[derive(Debug, thiserror::Error)] +#[error("templates are not supported in {kind}s: {value}")] +pub struct StaticReferenceError { + kind: ReferenceKind, + value: String, +} + +impl StaticReferenceError { + #[must_use] + pub fn new(kind: ReferenceKind, value: impl Into) -> Self { + Self { + kind, + value: value.into(), + } + } + + #[must_use] + pub fn kind(&self) -> ReferenceKind { + self.kind + } + + #[must_use] + pub fn value(&self) -> &str { + &self.value + } +} + +/// Reject static file references (imports, child workflows, `@` file values) +/// that contain template syntax. +pub fn validate_static_reference( + value: &str, + kind: ReferenceKind, +) -> Result<(), StaticReferenceError> { + if contains_template_syntax(value) { + return Err(StaticReferenceError::new(kind, value)); + } + Ok(()) +} + +/// One file reference or inline template discovered in a workflow graph. +/// +/// `@` prefixes are already stripped from file references; inline variants +/// carry template content that the consumer should feed to template-dependency +/// discovery. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum GraphReference<'graph> { + /// `graph [goal="@"]`. + GoalFile { reference: &'graph str }, + /// A non-`@` graph `goal`: inline template content. + GoalInline { content: &'graph str }, + /// `node [import=""]` — another graph file to walk. + Import { reference: &'graph str }, + /// `node [stack.child_workflow=""]`. + ChildWorkflow { reference: &'graph str }, + /// `node [="@"]` for file-inlined attributes + /// (`prompt`, `output_schema`). + FileInline { + key: &'graph str, + reference: &'graph str, + }, + /// A non-`@` node prompt: inline template content. + InlinePrompt { content: &'graph str }, +} + +/// Error from [`visit_graph_references`]. +#[derive(Debug, thiserror::Error)] +pub enum GraphReferenceError { + #[error(transparent)] + StaticReference(StaticReferenceError), + #[error(transparent)] + Visit(E), +} + +/// Walk every static file reference and inline template in one parsed graph, +/// validating that file references are template-free before emitting them. +/// +/// The walker covers a single graph; recursion into `Import` targets and +/// resolution of references against a file source are the consumer's job. +pub fn visit_graph_references<'graph, E>( + graph: &'graph Graph, + mut visit: impl FnMut(GraphReference<'graph>) -> Result<(), E>, +) -> Result<(), GraphReferenceError> { + let goal = graph.goal(); + if !goal.is_empty() { + if let Some(reference) = goal.strip_prefix('@') { + validate_static_reference(reference, ReferenceKind::GraphGoalFile) + .map_err(GraphReferenceError::StaticReference)?; + visit(GraphReference::GoalFile { reference }).map_err(GraphReferenceError::Visit)?; + } else { + visit(GraphReference::GoalInline { content: goal }) + .map_err(GraphReferenceError::Visit)?; + } + } + + for node in graph.nodes.values() { + for (key, value) in &node.attrs { + let Some(value) = value.as_str() else { + continue; + }; + let Some(kind) = reference_kind_for_attribute(AttributeScope::Node, key, value) else { + continue; + }; + let reference = match kind { + ReferenceKind::Import | ReferenceKind::ChildWorkflow => value, + // Classification only yields FileInline for `@` values. + ReferenceKind::FileInline => value + .strip_prefix('@') + .expect("file inline classification requires a leading '@'"), + ReferenceKind::Dockerfile | ReferenceKind::GraphGoalFile => continue, + }; + validate_static_reference(reference, kind) + .map_err(GraphReferenceError::StaticReference)?; + let event = match kind { + ReferenceKind::Import => GraphReference::Import { reference }, + ReferenceKind::ChildWorkflow => GraphReference::ChildWorkflow { reference }, + ReferenceKind::FileInline => GraphReference::FileInline { key, reference }, + ReferenceKind::Dockerfile | ReferenceKind::GraphGoalFile => unreachable!(), + }; + visit(event).map_err(GraphReferenceError::Visit)?; + } + + if let Some(prompt) = node.prompt().filter(|prompt| !prompt.starts_with('@')) { + visit(GraphReference::InlinePrompt { content: prompt }) + .map_err(GraphReferenceError::Visit)?; + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeSet; + + use fabro_types::graph::{AttrValue, Graph, Node, ReferenceKind}; + + use super::{GraphReference, GraphReferenceError, validate_static_reference}; + + #[test] + fn static_reference_rejects_template_syntax() { + let error = validate_static_reference( + "@schemas/{{ inputs.schema }}.json", + ReferenceKind::FileInline, + ) + .unwrap_err(); + + assert_eq!(error.kind(), ReferenceKind::FileInline); + assert_eq!(error.value(), "@schemas/{{ inputs.schema }}.json"); + assert!( + error + .to_string() + .contains("templates are not supported in file inline references"), + "unexpected error: {error}", + ); + assert!( + validate_static_reference("@schemas/result.json", ReferenceKind::FileInline).is_ok() + ); + } + + fn node_with(id: &str, attrs: &[(&str, &str)]) -> Node { + let mut node = Node::new(id); + for (key, value) in attrs { + node.attrs + .insert((*key).to_string(), AttrValue::String((*value).to_string())); + } + node + } + + #[test] + fn visits_every_reference_kind_once() { + let mut graph = Graph::new("test"); + graph.attrs.insert( + "goal".to_string(), + AttrValue::String("@goal.md".to_string()), + ); + for node in [ + node_with("imported", &[("import", "graphs/child.fabro")]), + node_with("child", &[("stack.child_workflow", "children/check.fabro")]), + node_with("file_prompt", &[("prompt", "@prompts/task.md")]), + node_with("inline", &[("prompt", "Do the {{ thing }}")]), + ] { + graph.nodes.insert(node.id.clone(), node); + } + + let mut seen = BTreeSet::new(); + super::visit_graph_references( + &graph, + |reference| -> Result<(), std::convert::Infallible> { + seen.insert(match reference { + GraphReference::GoalFile { reference } => format!("goal-file:{reference}"), + GraphReference::GoalInline { content } => format!("goal-inline:{content}"), + GraphReference::Import { reference } => format!("import:{reference}"), + GraphReference::ChildWorkflow { reference } => format!("child:{reference}"), + GraphReference::FileInline { key, reference } => { + format!("file:{key}:{reference}") + } + GraphReference::InlinePrompt { content } => format!("inline:{content}"), + }); + Ok(()) + }, + ) + .unwrap(); + + assert_eq!( + seen, + BTreeSet::from([ + "goal-file:goal.md".to_string(), + "import:graphs/child.fabro".to_string(), + "child:children/check.fabro".to_string(), + "file:prompt:prompts/task.md".to_string(), + "inline:Do the {{ thing }}".to_string(), + ]) + ); + } + + #[test] + fn rejects_template_syntax_in_references_before_visiting() { + let mut graph = Graph::new("test"); + graph.nodes.insert( + "imported".to_string(), + node_with("imported", &[("import", "graphs/{{ name }}.fabro")]), + ); + + let error = + super::visit_graph_references(&graph, |_| -> Result<(), std::convert::Infallible> { + panic!("references with template syntax must not be visited") + }) + .unwrap_err(); + assert!(matches!(error, GraphReferenceError::StaticReference(_))); + } +} diff --git a/lib/foundation/fabro-types/src/graph.rs b/lib/foundation/fabro-types/src/graph.rs index e2ed3fab4..b9f74b79f 100644 --- a/lib/foundation/fabro-types/src/graph.rs +++ b/lib/foundation/fabro-types/src/graph.rs @@ -590,6 +590,54 @@ impl Graph { } } +/// Where an attribute appears in a workflow graph. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum AttributeScope { + Graph, + Node, + Edge, +} + +/// Kinds of static (non-templated) file references a graph attribute can +/// carry. +#[derive(Clone, Copy, Debug, Eq, PartialEq, strum::Display)] +pub enum ReferenceKind { + #[strum(to_string = "file inline reference")] + FileInline, + #[strum(to_string = "import reference")] + Import, + #[strum(to_string = "child workflow reference")] + ChildWorkflow, + #[strum(to_string = "Dockerfile reference")] + Dockerfile, + #[strum(to_string = "graph goal file reference")] + GraphGoalFile, +} + +/// Classify a graph attribute as a static file reference, if it is one. +#[must_use] +pub fn reference_kind_for_attribute( + scope: AttributeScope, + key: &str, + value: &str, +) -> Option { + match key { + "import" if matches!(scope, AttributeScope::Node) => Some(ReferenceKind::Import), + "stack.child_workflow" if matches!(scope, AttributeScope::Node) => { + Some(ReferenceKind::ChildWorkflow) + } + "goal" if matches!(scope, AttributeScope::Graph) && value.starts_with('@') => { + Some(ReferenceKind::GraphGoalFile) + } + "prompt" | "output_schema" + if matches!(scope, AttributeScope::Node) && value.starts_with('@') => + { + Some(ReferenceKind::FileInline) + } + _ => None, + } +} + #[cfg(test)] mod tests { use super::*; @@ -1111,4 +1159,24 @@ mod tests { ); assert_eq!(g.loop_restart_signature_limit(), 3); } + + #[test] + fn output_schema_at_value_is_file_inline_reference() { + assert_eq!( + reference_kind_for_attribute( + AttributeScope::Node, + "output_schema", + "@schemas/result.schema.json", + ), + Some(ReferenceKind::FileInline), + ); + } + + #[test] + fn output_schema_builtin_keyword_is_not_file_inline_reference() { + assert_eq!( + reference_kind_for_attribute(AttributeScope::Node, "output_schema", "routing"), + None, + ); + } }