scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-14 23:22:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Merge pull request #654 from fabro-sh/fix/reject-edge-only-nodes

Browse source 
Reject nodes referenced by an edge but never declared
This commit is contained in:
Bryan Helmkamp 2026-07-28 14:28:11 -04:00 • committed by GitHub
commit a10ffb02b5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 451 additions and 128 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
docs/public/reference/dot-language.mdx
View file

@ -113,7 +113,7 @@ plan [label="Plan", prompt="Create an implementation plan."]
**Node identifiers** must start with a letter or underscore, followed by letters, digits, or underscores (e.g. `run_tests`, `gate_1`, `_private`).
Nodes referenced in edges are auto-created if not explicitly declared.
Every node used by an edge needs its own declaration. Validation fails when an edge names a node the workflow never declares, because that is nearly always a typo or a rename that missed an edge. The declaration can come before or after the edges that use it, and it can live in a subgraph.
### Edge declarations

83
lib/apps/fabro-cli/src/shared/utilities.rs
View file

@ -49,54 +49,49 @@ where
pub(crate) fn print_diagnostics(diagnostics: &[Diagnostic], styles: &Styles, printer: Printer) {
for d in diagnostics {
let location = match (&d.node_id, &d.edge) {
(Some(node), _) => format!(" [node: {node}]"),
(_, Some((from, to))) => format!(" [edge: {from} -> {to}]"),
_ => String::new(),
};
let source_prefix = source_prefix(d);
match d.severity {
Severity::Error if source_prefix.is_empty() => fabro_util::printerr!(
printer,
"{}{location}: {} ({})",
styles.red.apply_to("error"),
d.message,
styles.dim.apply_to(&d.rule),
),
Severity::Error => fabro_util::printerr!(
printer,
"{}: {source_prefix}{}{location} ({})",
styles.red.apply_to("error"),
d.message,
styles.dim.apply_to(&d.rule),
),
Severity::Warning if source_prefix.is_empty() => fabro_util::printerr!(
printer,
"{}{location}: {} ({})",
styles.yellow.apply_to("warning"),
d.message,
styles.dim.apply_to(&d.rule),
),
Severity::Warning => fabro_util::printerr!(
printer,
"{}: {source_prefix}{}{location} ({})",
styles.yellow.apply_to("warning"),
d.message,
styles.dim.apply_to(&d.rule),
),
Severity::Info => fabro_util::printerr!(
printer,
"{}",
styles.dim.apply_to(if source_prefix.is_empty() {
format!("info{location}: {} ({})", d.message, d.rule)
} else {
format!("info: {source_prefix}{}{location} ({})", d.message, d.rule)
}),
),
print_diagnostic(d, styles, printer);
// The fix is the actionable half of a diagnostic, so it follows every
// severity rather than hiding behind --verbose. Rules that have nothing
// useful to suggest leave it unset.
if let Some(fix) = &d.fix {
fabro_util::printerr!(printer, " {} {fix}", styles.dim.apply_to("fix:"));
}
}
}
fn print_diagnostic(d: &Diagnostic, styles: &Styles, printer: Printer) {
let location = match (&d.node_id, &d.edge) {
(Some(node), _) => format!(" [node: {node}]"),
(_, Some((from, to))) => format!(" [edge: {from} -> {to}]"),
_ => String::new(),
};
let source_prefix = source_prefix(d);
let body = if source_prefix.is_empty() {
format!("{location}: {}", d.message)
} else {
format!(": {source_prefix}{}{location}", d.message)
};
match d.severity {
Severity::Error => fabro_util::printerr!(
printer,
"{}{body} ({})",
styles.red.apply_to("error"),
styles.dim.apply_to(&d.rule),
),
Severity::Warning => fabro_util::printerr!(
printer,
"{}{body} ({})",
styles.yellow.apply_to("warning"),
styles.dim.apply_to(&d.rule),
),
Severity::Info => fabro_util::printerr!(
printer,
"{}",
styles.dim.apply_to(format!("info{body} ({})", d.rule)),
),
}
}
fn source_prefix(diagnostic: &Diagnostic) -> String {
match (
diagnostic.source_path.as_deref(),

4
lib/apps/fabro-cli/tests/it/cmd/graph.rs
View file

@ -95,7 +95,9 @@ fn graph_allow_invalid_renders_after_diagnostics() {
----- stdout -----
----- stderr -----
error: Pipeline must have exactly one start node (shape=Mdiamond or id start/Start) (start_node)
fix: Add a node with shape=Mdiamond or id 'start'
error [node: exit]: Exit node 'exit' has 1 outgoing edge(s) but must have none (exit_no_outgoing)
fix: Remove outgoing edges from the exit node
");
let svg = read_text(&output_path);
@ -119,7 +121,9 @@ fn graph_invalid_workflow_fails_after_diagnostics() {
----- stdout -----
----- stderr -----
error: Pipeline must have exactly one start node (shape=Mdiamond or id start/Start) (start_node)
fix: Add a node with shape=Mdiamond or id 'start'
error [node: exit]: Exit node 'exit' has 1 outgoing edge(s) but must have none (exit_no_outgoing)
fix: Remove outgoing edges from the exit node
× Validation failed
");
}

4
lib/apps/fabro-cli/tests/it/cmd/preflight.rs
View file

@ -52,7 +52,9 @@ fn preflight_invalid_workflow_fails_with_validation_output() {
Workflow: Invalid (2 nodes, 1 edges)
Graph: [FIXTURES]/invalid.fabro
error: Pipeline must have exactly one start node (shape=Mdiamond or id start/Start) (start_node)
fix: Add a node with shape=Mdiamond or id 'start'
error [node: exit]: Exit node 'exit' has 1 outgoing edge(s) but must have none (exit_no_outgoing)
fix: Remove outgoing edges from the exit node
× Validation failed
");
}
@ -74,7 +76,9 @@ fn preflight_rejects_unbound_template_inputs() {
Goal: Demo
error: [FIXTURES]/templated_unbound.fabro:2:26: undefined template variable `inputs.app_dir` in graph attribute `goal` (template_undefined_variable)
fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=<value>`
error: [FIXTURES]/templated_unbound.fabro:7:44: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable)
fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=<value>`
× Validation failed
");
}

27
lib/apps/fabro-cli/tests/it/cmd/validate.rs
View file

@ -82,6 +82,7 @@ fn branching() {
Workflow: Branch (6 nodes, 6 edges)
Graph: [FIXTURES]/branching.fabro
warning [node: implement]: Node 'implement' has goal_gate=true but no retry_target or fallback_retry_target (goal_gate_has_retry)
fix: Add retry_target or fallback_retry_target attribute
Validation: OK
");
}
@ -163,7 +164,9 @@ fn bare_fabro_with_unbound_inputs_validates_structurally_with_warning() {
Workflow: TemplatedUnbound (3 nodes, 2 edges)
Graph: [FIXTURES]/templated_unbound.fabro
warning: [FIXTURES]/templated_unbound.fabro:2:26: undefined template variable `inputs.app_dir` in graph attribute `goal` (template_undefined_variable)
fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=<value>`
warning: [FIXTURES]/templated_unbound.fabro:7:44: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable)
fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=<value>`
Validation: OK
");
}
@ -186,6 +189,7 @@ fn bare_fabro_with_unbound_inputs_in_imported_prompt_validates_structurally_with
Workflow: TemplatedUnboundImported (3 nodes, 2 edges)
Graph: [FIXTURES]/templated_unbound_imported/workflow.fabro
warning: [FIXTURES]/templated_unbound_imported/work.md:1:12: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable)
fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=<value>`
Validation: OK
");
}
@ -207,6 +211,7 @@ fn bare_fabro_with_unbound_inputs_in_template_partial_validates_structurally_wit
Workflow: TemplatedUnboundPartial (3 nodes, 2 edges)
Graph: [FIXTURES]/templated_unbound_partial/workflow.fabro
warning: [FIXTURES]/templated_unbound_partial/test-include.partial.md:1:4: undefined template variable `inputs.hello` in node `test_imported_include` attribute `prompt` [node: test_imported_include] (template_undefined_variable)
fix: bind `inputs.hello` via `[run.inputs]` in workflow.toml, or pass `--input inputs.hello=<value>`
Validation: OK
");
}
@ -278,6 +283,26 @@ fn validate_reports_missing_template_dependency() {
");
}
/// A node named only by an edge is almost always a typo, so validation must
/// fail instead of quietly running it as a default agent stage.
#[test]
fn edge_only_node() {
let context = test_context!();
let mut cmd = context.validate();
cmd.arg(fixture("edge_only_node.fabro"));
fabro_snapshot!(context.filters(), cmd, @"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
Workflow: EdgeOnlyNode (2 nodes, 2 edges)
Graph: [FIXTURES]/edge_only_node.fabro
error [node: misspelled_node]: Node 'misspelled_node' is referenced by edge 'start -> misspelled_node' but has no node declaration (edge_target_exists)
fix: Declare node 'misspelled_node' or correct the edge endpoint
× Validation failed
");
}
#[test]
fn invalid() {
let context = test_context!();
@ -291,7 +316,9 @@ fn invalid() {
Workflow: Invalid (2 nodes, 1 edges)
Graph: [FIXTURES]/invalid.fabro
error: Pipeline must have exactly one start node (shape=Mdiamond or id start/Start) (start_node)
fix: Add a node with shape=Mdiamond or id 'start'
error [node: exit]: Exit node 'exit' has 1 outgoing edge(s) but must have none (exit_no_outgoing)
fix: Remove outgoing edges from the exit node
× Validation failed
");
}

1
lib/apps/fabro-cli/tests/it/workflow/dry_run_examples.rs
View file

@ -19,6 +19,7 @@ fn dry_run_branching() {
Goal: Implement and validate a feature
warning [node: implement]: Node 'implement' has goal_gate=true but no retry_target or fallback_retry_target (goal_gate_has_retry)
fix: Add retry_target or fallback_retry_target attribute
Run: [ULID]
Web UI: http://localhost:3000/runs/[ULID]
Sandbox: local (ready in [TIME])

143
lib/components/fabro-graphviz/src/parser/semantic.rs
View file

@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::time::Duration;
use crate::error::Error;
@ -57,29 +57,44 @@ fn derive_class_from_label(label: &str) -> String {
.collect()
}
fn collect_declared_node_ids(statements: &[Statement], node_ids: &mut HashSet<String>) {
for statement in statements {
match statement {
Statement::Node(node) => {
node_ids.insert(node.id.clone());
}
Statement::Subgraph(subgraph) => {
collect_declared_node_ids(&subgraph.statements, node_ids);
}
_ => {}
}
}
}
struct SemanticState {
graph: Graph,
node_defaults: HashMap<String, AttrValue>,
edge_defaults: HashMap<String, AttrValue>,
graph: Graph,
declared_node_ids: HashSet<String>,
node_defaults: HashMap<String, AttrValue>,
edge_defaults: HashMap<String, AttrValue>,
}
impl SemanticState {
fn new(name: String) -> Self {
fn new(name: String, declared_node_ids: HashSet<String>) -> Self {
Self {
graph: Graph::new(name),
graph: Graph::new(name),
declared_node_ids,
node_defaults: HashMap::new(),
edge_defaults: HashMap::new(),
}
}
fn ensure_node(&mut self, id: &str) {
if !self.graph.nodes.contains_key(id) {
fn ensure_node(&mut self, id: &str) -> &mut Node {
let node_defaults = &self.node_defaults;
self.graph.nodes.entry(id.to_string()).or_insert_with(|| {
let mut node = Node::new(id);
for (k, v) in &self.node_defaults {
node.attrs.insert(k.clone(), v.clone());
}
self.graph.nodes.insert(id.to_string(), node);
}
node.attrs.clone_from(node_defaults);
node
})
}
fn add_class_to_node(node: &mut Node, cls: &str) {
@ -90,12 +105,7 @@ impl SemanticState {
}
fn process_node(&mut self, node_stmt: &NodeStmt, subgraph_class: Option<&str>) {
self.ensure_node(&node_stmt.id);
let node = self
.graph
.nodes
.get_mut(&node_stmt.id)
.expect("node was just inserted by ensure_node, so get_mut cannot return None");
let node = self.ensure_node(&node_stmt.id);
if let Some(attrs) = &node_stmt.attrs {
for (k, v) in attrs {
node.attrs.insert(k.clone(), convert_value(v));
@ -111,11 +121,6 @@ impl SemanticState {
.and_then(AttrValue::as_str)
.map(String::from);
if let Some(class_str) = class_str {
let node = self
.graph
.nodes
.get_mut(&node_stmt.id)
.expect("node was just inserted by ensure_node, so get_mut cannot return None");
for cls in class_str.split(',') {
let cls = cls.trim().to_string();
if !cls.is_empty() && !node.classes.contains(&cls) {
@ -127,12 +132,11 @@ impl SemanticState {
fn process_edge(&mut self, edge_stmt: &EdgeStmt, subgraph_class: Option<&str>) {
for id in &edge_stmt.nodes {
self.ensure_node(id);
if !self.declared_node_ids.contains(id) {
continue;
}
let node = self.ensure_node(id);
if let Some(cls) = subgraph_class {
let node =
self.graph.nodes.get_mut(id).expect(
"node was just inserted by ensure_node, so get_mut cannot return None",
);
Self::add_class_to_node(node, cls);
}
}
@ -251,7 +255,9 @@ impl SemanticState {
///
/// Returns an error if the AST cannot be converted to a valid graph.
pub fn ast_to_graph(dot: &DotGraph) -> Result<Graph, Error> {
let mut state = SemanticState::new(dot.name.clone());
let mut declared_node_ids = HashSet::new();
collect_declared_node_ids(&dot.statements, &mut declared_node_ids);
let mut state = SemanticState::new(dot.name.clone(), declared_node_ids);
let empty = HashMap::new();
state.process_statements(&dot.statements, None, &empty, &empty);
Ok(state.graph)
@ -515,7 +521,7 @@ mod tests {
}
#[test]
fn ast_to_graph_implicit_nodes_from_edges() {
fn ast_to_graph_keeps_undeclared_edge_endpoints_out_of_nodes() {
let dot = DotGraph {
name: "Implicit".into(),
statements: vec![Statement::Edge(EdgeStmt {
@ -524,8 +530,83 @@ mod tests {
})],
};
let graph = ast_to_graph(&dot).unwrap();
assert!(graph.nodes.is_empty());
assert_eq!(graph.edges, vec![Edge::new("a", "b")]);
}
#[test]
fn ast_to_graph_includes_only_declared_edge_endpoints() {
let dot = DotGraph {
name: "Declared".into(),
statements: vec![
Statement::Node(NodeStmt {
id: "a".into(),
attrs: None,
}),
Statement::Edge(EdgeStmt {
nodes: vec!["a".into(), "b".into()],
attrs: None,
}),
],
};
let graph = ast_to_graph(&dot).unwrap();
assert!(graph.nodes.contains_key("a"));
assert!(!graph.nodes.contains_key("b"));
}
#[test]
fn ast_to_graph_declaration_after_edge_still_counts() {
let dot = DotGraph {
name: "DeclaredLater".into(),
statements: vec![
Statement::NodeDefaults(vec![("model".into(), AstValue::Str("first".into()))]),
Statement::Edge(EdgeStmt {
nodes: vec!["a".into(), "b".into()],
attrs: None,
}),
Statement::NodeDefaults(vec![("model".into(), AstValue::Str("second".into()))]),
Statement::Node(NodeStmt {
id: "b".into(),
attrs: Some(vec![("prompt".into(), AstValue::Str("Do it".into()))]),
}),
],
};
let graph = ast_to_graph(&dot).unwrap();
assert!(graph.nodes.contains_key("b"));
assert!(!graph.nodes.contains_key("a"));
assert_eq!(
graph.nodes["b"]
.attrs
.get("model")
.and_then(AttrValue::as_str),
Some("first")
);
}
#[test]
fn ast_to_graph_subgraph_declaration_counts() {
let dot = DotGraph {
name: "SubgraphDeclared".into(),
statements: vec![
Statement::Edge(EdgeStmt {
nodes: vec!["start".into(), "plan".into()],
attrs: None,
}),
Statement::Subgraph(SubgraphStmt {
name: Some("cluster_loop".into()),
statements: vec![Statement::Node(NodeStmt {
id: "plan".into(),
attrs: None,
})],
}),
],
};
let graph = ast_to_graph(&dot).unwrap();
assert!(graph.nodes.contains_key("plan"));
assert!(!graph.nodes.contains_key("start"));
}
}

161
lib/components/fabro-validate/src/rules/edge_target_exists.rs
View file

@ -1,3 +1,5 @@
use std::collections::HashSet;
use fabro_graphviz::graph::Graph;
use crate::{Diagnostic, LintRule, Severity};
@ -8,6 +10,26 @@ pub(super) fn rule() -> Box<dyn LintRule> {
struct Rule;
impl Rule {
fn diagnostic(&self, node_id: &str, from: &str, to: &str) -> Diagnostic {
Diagnostic {
rule: self.name().to_string(),
severity: Severity::Error,
message: format!(
"Node '{node_id}' is referenced by edge '{from} -> {to}' but has no node \
declaration"
),
node_id: Some(node_id.to_string()),
edge: Some((from.to_string(), to.to_string())),
fix: Some(format!(
"Declare node '{node_id}' or correct the edge endpoint"
)),
..Diagnostic::default()
}
}
}
impl LintRule for Rule {
fn name(&self) -> &'static str {
"edge_target_exists"
@ -15,36 +37,12 @@ impl LintRule for Rule {
fn apply(&self, graph: &Graph) -> Vec<Diagnostic> {
let mut diagnostics = Vec::new();
let mut reported = HashSet::new();
for edge in &graph.edges {
if !graph.nodes.contains_key(&edge.to) {
diagnostics.push(Diagnostic {
rule: self.name().to_string(),
severity: Severity::Error,
message: format!(
"Edge from '{}' targets non-existent node '{}'",
edge.from, edge.to
),
node_id: None,
edge: Some((edge.from.clone(), edge.to.clone())),
fix: Some(format!("Define node '{}' or fix the edge target", edge.to)),
..Diagnostic::default()
});
}
if !graph.nodes.contains_key(&edge.from) {
diagnostics.push(Diagnostic {
rule: self.name().to_string(),
severity: Severity::Error,
message: format!("Edge source '{}' references non-existent node", edge.from),
node_id: None,
edge: Some((edge.from.clone(), edge.to.clone())),
fix: Some(format!(
"Define node '{}' or fix the edge source",
edge.from
)),
..Diagnostic::default()
});
for endpoint in [&edge.to, &edge.from] {
if !graph.nodes.contains_key(endpoint) && reported.insert(endpoint) {
diagnostics.push(self.diagnostic(endpoint, &edge.from, &edge.to));
}
}
}
diagnostics
@ -53,11 +51,112 @@ impl LintRule for Rule {
#[cfg(test)]
mod tests {
use fabro_graphviz::graph::Edge;
use fabro_graphviz::graph::{Edge, Graph};
use fabro_graphviz::parser;
use super::Rule;
use crate::rules::test_support::minimal_graph;
use crate::{LintRule, Severity};
use crate::{Diagnostic, LintRule, Severity};
fn parse(dot: &str) -> Graph {
parser::parse(dot).expect("fixture should parse")
}
fn undeclared_nodes(graph: &Graph) -> Vec<String> {
Rule.apply(graph)
.into_iter()
.map(|d| d.node_id.expect("diagnostic should name a node"))
.collect()
}
#[test]
fn edge_only_node_is_rejected() {
let graph = parse(
r"digraph EdgeOnly {
start [shape=Mdiamond]
exit [shape=Msquare]
start -> misspelled_node
misspelled_node -> exit
}",
);
let diagnostics = Rule.apply(&graph);
assert_eq!(diagnostics.len(), 1, "diagnostics: {diagnostics:?}");
let Diagnostic {
severity,
node_id,
edge,
..
} = &diagnostics[0];
assert_eq!(*severity, Severity::Error);
assert_eq!(node_id.as_deref(), Some("misspelled_node"));
assert_eq!(
edge.clone(),
Some(("start".to_string(), "misspelled_node".to_string()))
);
}
#[test]
fn declaration_after_the_edge_is_accepted() {
let graph = parse(
r#"digraph DeclaredLater {
start -> work
work [prompt="Do the work"]
work -> exit
start [shape=Mdiamond]
exit [shape=Msquare]
}"#,
);
assert!(Rule.apply(&graph).is_empty());
}
#[test]
fn chained_edges_report_every_undeclared_endpoint() {
let graph = parse(
r"digraph Chained {
start [shape=Mdiamond]
exit [shape=Msquare]
start -> first -> second -> exit
}",
);
assert_eq!(undeclared_nodes(&graph), vec!["first", "second"]);
}
#[test]
fn a_node_is_reported_once_no_matter_how_many_edges_use_it() {
let graph = parse(
r"digraph Repeated {
start [shape=Mdiamond]
exit [shape=Msquare]
start -> typo
typo -> exit
typo -> start
}",
);
assert_eq!(undeclared_nodes(&graph), vec!["typo"]);
}
#[test]
fn subgraph_declaration_is_accepted() {
let graph = parse(
r#"digraph Subgraphed {
start [shape=Mdiamond]
exit [shape=Msquare]
subgraph cluster_loop {
label = "Loop A"
plan [prompt="Plan the work"]
}
start -> plan -> exit
}"#,
);
assert!(Rule.apply(&graph).is_empty());
}
#[test]
fn edge_target_exists_rule_missing_target() {

122
lib/components/fabro-workflow/src/transforms/import.rs
View file

@ -339,18 +339,18 @@ impl ImportTransform {
.edges
.retain(|edge| edge.from != placeholder_id && edge.to != placeholder_id);
for (node_id, node) in imported_graph.nodes {
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}");
let mut merged_node = Node::new(&prefixed_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(node.attrs);
merged_node.attrs.extend(imported_attrs);
Self::remap_retry_target(&mut merged_node.attrs, placeholder_id);
merged_node.classes = node.classes;
for class_name in &placeholder.class_names {
Self::push_class(&mut merged_node.classes, class_name);
}
@ -649,7 +649,10 @@ impl PreparedImport {
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
)
}
}
@ -909,6 +912,7 @@ mod tests {
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"));
@ -951,6 +955,72 @@ mod tests {
);
}
#[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();
@ -1130,6 +1200,48 @@ mod tests {
);
}
#[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();

3
lib/components/fabro-workflow/tests/it/integration.rs
View file

@ -388,6 +388,9 @@ fn parse_and_validate_human_gate() {
type="human"
]
ship_it [prompt="Ship the change"]
fixes [prompt="Apply the requested fixes"]
start -> review_gate
review_gate -> ship_it [label="[A] Approve"]
review_gate -> fixes [label="[F] Fix"]

19
lib/foundation/fabro-types/src/run_event/mod.rs
View file

@ -926,8 +926,6 @@ impl<'de> Deserialize<'de> for RunEvent {
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use serde_json::json;
use super::*;
@ -992,20 +990,9 @@ mod tests {
#[test]
fn run_event_deserializes_adjacent_layout() {
let settings = WorkflowSettings::default();
let graph = Graph {
name: "test".to_string(),
nodes: HashMap::from([("start".to_string(), Node {
id: "start".to_string(),
attrs: HashMap::new(),
classes: Vec::new(),
})]),
edges: vec![Edge {
from: "start".to_string(),
to: "done".to_string(),
attrs: HashMap::new(),
}],
attrs: HashMap::new(),
};
let mut graph = Graph::new("test");
graph.nodes.insert("start".to_string(), Node::new("start"));
graph.edges.push(Edge::new("start", "done"));
let line = json!({
"id": "evt_2",

10
test/edge_only_node.fabro Normal file
View file

@ -0,0 +1,10 @@
digraph EdgeOnlyNode {
graph [goal="Reference a node that was never declared"]
/* `misspelled_node` is only ever named by an edge, never declared. */
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
start -> misspelled_node
misspelled_node -> exit
}