fix(workflow): align inferred command behavior

This commit is contained in:
Bryan Helmkamp 2026-07-29 10:54:43 -04:00
parent cb24f47b59
commit d37fc0027c
No known key found for this signature in database
11 changed files with 285 additions and 224 deletions

View file

@ -174,6 +174,36 @@ describe("parseFabro", () => {
expect(draft.nodes.find((n) => n.id === "plain")?.shape).toBe("box");
});
test("shapeless script node keeps command behavior after round trip", () => {
const draft = expectOk(
parseFabro(`digraph G {
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
build [script="cargo build"]
start -> build -> exit
}`),
);
expect(draft.nodes.find((n) => n.id === "build")?.shape).toBe(
"parallelogram",
);
const reparsed = expectOk(parseFabro(renderFabro(draft)));
expect(reparsed.nodes.find((n) => n.id === "build")?.shape).toBe(
"parallelogram",
);
});
test("explicit type disables command shape inference", () => {
const draft = expectOk(
parseFabro(`digraph G {
work [type=agent, script="cargo build"]
}`),
);
expect(draft.nodes.find((n) => n.id === "work")?.shape).toBe("box");
});
test("unknown shape falls back to box", () => {
const draft = expectOk(
parseFabro(`digraph G {

View file

@ -147,7 +147,7 @@ export function parseFabro(src: string): ParseResult {
}
function buildNode(id: string, attrs: Record<string, AttrValue>): Node {
const shape = coerceShape(attrs.shape, id);
const shape = coerceShape(attrs, id);
const label = typeof attrs.label === "string" ? attrs.label : id;
const node: Node = { id, label, shape };
if (typeof attrs.prompt === "string") node.prompt = attrs.prompt;
@ -159,10 +159,18 @@ function buildNode(id: string, attrs: Record<string, AttrValue>): Node {
return node;
}
function coerceShape(raw: AttrValue | undefined, nodeId: string): Shape {
function coerceShape(
attrs: Record<string, AttrValue>,
nodeId: string,
): Shape {
const raw = attrs.shape;
if (typeof raw !== "string") {
// Shape omitted: default to start/exit terminals if id matches,
// otherwise `box` (Fabro's agent default).
// Without an explicit shape or type, `script` selects the command
// handler. Keep the inferred shape when the draft is rendered again.
if (typeof attrs.type !== "string" && attrs.script !== undefined) {
return "parallelogram";
}
// The playground also infers terminal shapes from reserved ids.
if (nodeId === "start") return "mdiamond";
if (nodeId === "exit") return "msquare";
return "box";

View file

@ -153,7 +153,7 @@ Node and edge defaults declared inside a subgraph are scoped — they don't leak
## Node types
Each node's `shape` attribute determines its execution behavior. See [Nodes & Stages](/workflows/stages-and-nodes) for detailed documentation of each type.
Fabro uses an explicit `type` attribute to select a node's handler. Without `type`, the `shape` attribute selects the handler. See [Nodes & Stages](/workflows/stages-and-nodes) for detailed documentation of each type.
| Shape | Handler | Purpose |
|---|---|---|
@ -161,7 +161,7 @@ Each node's `shape` attribute determines its execution behavior. See [Nodes & St
| `Msquare` | exit | Workflow terminal (exactly one required) |
| `box` (default) | agent | Multi-turn LLM with tool access |
| `tab` | prompt | Single LLM call, no tools |
| `parallelogram` (inferred from `script`) | command | Execute a shell script |
| `parallelogram` (inferred from `script` when `shape` and `type` are omitted) | command | Execute a shell script |
| `hexagon` | human | Human-in-the-loop decision gate |
| `diamond` | conditional | Route based on conditions |
| `component` | parallel | Fan-out to concurrent branches |
@ -169,20 +169,20 @@ Each node's `shape` attribute determines its execution behavior. See [Nodes & St
| `insulator` | wait | Pause for a duration |
| `house` | stack.manager_loop | Sub-workflow orchestration |
The `type` attribute can also be set explicitly to override the shape-based mapping.
The `type` attribute can be set explicitly to override the shape-based mapping and attribute inference.
Start nodes can also be identified by ID (`start` or `Start`). Exit nodes can be identified by ID (`exit`, `Exit`, `end`, or `End`).
### Omitting the shape
The two most common node types don't need a `shape`. A node that sets `script` is a command node; every other shapeless node is an agent node:
The two most common node types don't need a `shape` or `type`. When both are omitted, a node that sets `script` is a command node. Every other node defaults to an agent node:
```dot
plan [prompt="Plan the work"] // agent
build [script="cargo build"] // command
```
An explicit `shape` always wins, so `box` with a `script` is still an agent node — and the `script` is then read by nothing. Setting both `script` and `prompt` on one node is an error: no handler reads both.
An explicit `shape` or `type` always wins. For example, `shape=box` with a `script` is still an agent node, and the `script` has no effect. Setting both `script` and `prompt` on one node is an error because command handlers consume `script`, while LLM handlers consume `prompt`.
Other node types still need their shape, because their attributes don't identify them uniquely. In particular, agent and prompt nodes take the same attributes, so a prompt node needs `shape=tab`.
@ -257,7 +257,7 @@ audit [
| Attribute | Type | Description |
|---|---|---|
| `script` | String | Shell command to execute (required). Its presence also makes `shape=parallelogram` optional. |
| `script` | String | Shell command to execute (required). Its presence infers a command node when `shape` and `type` are omitted. |
| `language` | String | `"shell"` (default) or `"python"` |
| `output_schema` | String | Optional structured output validation. Accepts `routing`, `@path/to/schema.json`, or an inline JSON Schema object string. See [Structured output validation](#structured-output-validation). |

View file

@ -11,7 +11,7 @@ This distinction matters for observability and debugging: the workflow graph sho
## Node types
Every node's Graphviz `shape` attribute determines its execution behavior. If no shape is specified, the node defaults to an agent.
An explicit `type` attribute selects a node's execution behavior. Otherwise, its Graphviz `shape` selects the behavior. When both are omitted, a node with `script` is a command node and every other node defaults to an agent.
### Start
@ -35,7 +35,7 @@ exit [shape=Msquare, label="Exit"]
### Agent
**Shape:** `box` (default — a node with no shape and no `script` is an agent node)
**Shape:** `box` (default when `shape`, `type`, and `script` are omitted)
Runs an LLM with access to tools — bash, file editing, sub-agents — in an agentic loop. The agent works autonomously, calling tools as needed, until it decides the task is complete.
@ -94,7 +94,7 @@ Prompt nodes accept the same attributes as agent nodes (`prompt`, `reasoning_eff
### Command
**Shape:** `parallelogram` (optional — inferred from `script`)
**Shape:** `parallelogram` (optional when `shape` and `type` are omitted — inferred from `script`)
Runs a shell script inside the configured sandbox and captures its output. The output is available to downstream nodes as context. This ensures command nodes execute in the same environment as agent nodes.
@ -102,7 +102,7 @@ Runs a shell script inside the configured sandbox and captures its output. The o
test [label="Run Tests", script="cargo test 2>&1 || true"]
```
`script` is what makes a node a command node, so the shape can be left off. Writing `shape=parallelogram` explicitly is still valid and does the same thing.
When a node has no explicit `shape` or `type`, the presence of `script` makes it a command node. Writing `shape=parallelogram` explicitly is still valid and does the same thing.
| Attribute | Description |
|---|---|

View file

@ -45,24 +45,15 @@ mod tests {
use fabro_graphviz::graph::{AttrValue, Node};
use super::Rule;
use crate::rules::test_support::minimal_graph;
use crate::rules::test_support;
use crate::{LintRule, Severity};
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 errors_on_command_shape_without_script() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
node_with("run", &[("shape", "parallelogram"), ("label", "Build")]),
test_support::node_with_attrs("run", &[("shape", "parallelogram"), ("label", "Build")]),
);
let d = Rule.apply(&g);
@ -75,9 +66,11 @@ mod tests {
#[test]
fn errors_on_explicit_command_type_without_script() {
let mut g = minimal_graph();
g.nodes
.insert("run".to_string(), node_with("run", &[("type", "command")]));
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
test_support::node_with_attrs("run", &[("type", "command")]),
);
let d = Rule.apply(&g);
@ -86,29 +79,51 @@ mod tests {
}
#[test]
fn errors_on_blank_script() {
let mut g = minimal_graph();
fn errors_on_legacy_tool_type_without_script() {
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
node_with("run", &[("shape", "parallelogram"), ("script", " ")]),
test_support::node_with_attrs("run", &[("type", "tool")]),
);
assert_eq!(Rule.apply(&g).len(), 1);
}
#[test]
fn accepts_command_node_with_script() {
let mut g = minimal_graph();
fn errors_on_blank_script() {
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
node_with("run", &[
test_support::node_with_attrs("run", &[("shape", "parallelogram"), ("script", " ")]),
);
assert_eq!(Rule.apply(&g).len(), 1);
}
#[test]
fn errors_on_non_string_script() {
let mut g = test_support::minimal_graph();
let mut node = Node::new("run");
node.attrs
.insert("script".to_string(), AttrValue::Integer(123));
g.nodes.insert("run".to_string(), node);
assert_eq!(Rule.apply(&g).len(), 1);
}
#[test]
fn accepts_command_node_with_script() {
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
test_support::node_with_attrs("run", &[
("shape", "parallelogram"),
("script", "cargo build"),
]),
);
g.nodes.insert(
"build".to_string(),
node_with("build", &[("script", "cargo build")]),
test_support::node_with_attrs("build", &[("script", "cargo build")]),
);
assert!(Rule.apply(&g).is_empty());
@ -116,14 +131,14 @@ mod tests {
#[test]
fn ignores_non_command_nodes() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"plan".to_string(),
node_with("plan", &[("prompt", "do it")]),
test_support::node_with_attrs("plan", &[("prompt", "do it")]),
);
g.nodes.insert(
"gate".to_string(),
node_with("gate", &[("shape", "hexagon")]),
test_support::node_with_attrs("gate", &[("shape", "hexagon")]),
);
assert!(Rule.apply(&g).is_empty());

View file

@ -1,4 +1,4 @@
use fabro_graphviz::graph::{self, Graph};
use fabro_graphviz::graph::{self, Graph, Node};
use crate::{Diagnostic, LintRule, Severity};
@ -25,8 +25,30 @@ const HANDLER_SPECIFIC_ATTRS: &[(&str, &[&str])] = &[
("review_target", &["human"]),
];
const SCRIPT_PROMPT_CONFLICT_RULE: &str = "script_prompt_conflict";
struct Rule;
fn script_prompt_conflict(node: &Node) -> Diagnostic {
Diagnostic {
rule: SCRIPT_PROMPT_CONFLICT_RULE.to_string(),
severity: Severity::Error,
message: format!(
"Node '{}' sets both 'script' and 'prompt'. No built-in handler reads both: command \
handlers consume 'script', while LLM handlers consume 'prompt'",
node.id
),
node_id: Some(node.id.clone()),
edge: None,
fix: Some(
"Remove whichever attribute is wrong, or split the node into a command node and an \
agent node"
.to_string(),
),
..Diagnostic::default()
}
}
impl LintRule for Rule {
fn name(&self) -> &'static str {
"inert_attribute"
@ -35,6 +57,12 @@ impl LintRule for Rule {
fn apply(&self, graph: &Graph) -> Vec<Diagnostic> {
let mut diagnostics = Vec::new();
for node in graph.nodes.values() {
let has_script_prompt_conflict =
node.attrs.contains_key("script") && node.attrs.contains_key("prompt");
if has_script_prompt_conflict {
diagnostics.push(script_prompt_conflict(node));
}
// An unknown shape or type is covered by the type_known rule; a
// node this rule cannot classify is skipped rather than guessed at.
let Some(handler) = node.handler_type() else {
@ -47,6 +75,9 @@ impl LintRule for Rule {
if !node.attrs.contains_key(*attr) {
continue;
}
if has_script_prompt_conflict && matches!(*attr, "script" | "prompt") {
continue;
}
if consumers.contains(&handler) {
continue;
}
@ -74,24 +105,19 @@ impl LintRule for Rule {
#[cfg(test)]
mod tests {
use fabro_graphviz::graph::{AttrValue, Node};
use fabro_graphviz::graph::{AttrValue, Edge, Node};
use super::Rule;
use crate::rules::test_support::minimal_graph;
use crate::rules::test_support;
use crate::{LintRule, Severity};
fn node_with_attr(id: &str, shape: &str, attr: &str, value: &str) -> Node {
let mut node = Node::new(id);
node.attrs
.insert("shape".to_string(), AttrValue::String(shape.to_string()));
node.attrs
.insert(attr.to_string(), AttrValue::String(value.to_string()));
node
test_support::node_with_attrs(id, &[("shape", shape), (attr, value)])
}
#[test]
fn warns_on_script_on_agent_node() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"work".to_string(),
node_with_attr("work", "box", "script", "echo hi"),
@ -104,9 +130,77 @@ mod tests {
assert_eq!(d[0].node_id.as_deref(), Some("work"));
}
#[test]
fn built_in_rules_report_script_prompt_conflict_once() {
let mut g = test_support::minimal_graph();
g.nodes.insert(
"work".to_string(),
test_support::node_with_attrs("work", &[
("script", "cargo build"),
("prompt", "do it"),
]),
);
g.edges = vec![Edge::new("start", "work"), Edge::new("work", "exit")];
let diagnostics = crate::validate(&g, &[]);
let work_diagnostics = diagnostics
.iter()
.filter(|diagnostic| diagnostic.node_id.as_deref() == Some("work"))
.collect::<Vec<_>>();
assert_eq!(work_diagnostics.len(), 1, "diagnostics: {diagnostics:?}");
assert_eq!(work_diagnostics[0].rule, "script_prompt_conflict");
assert_eq!(work_diagnostics[0].severity, Severity::Error);
}
#[test]
fn conflict_error_replaces_inert_warning_for_explicit_shapes() {
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
test_support::node_with_attrs("run", &[
("shape", "parallelogram"),
("script", "cargo build"),
("prompt", "do it"),
]),
);
g.nodes.insert(
"plan".to_string(),
test_support::node_with_attrs("plan", &[
("shape", "box"),
("script", "cargo build"),
("prompt", "do it"),
]),
);
let diagnostics = Rule.apply(&g);
assert_eq!(diagnostics.len(), 2);
assert!(
diagnostics
.iter()
.all(|diagnostic| diagnostic.rule == "script_prompt_conflict"
&& diagnostic.severity == Severity::Error)
);
}
#[test]
fn conflict_uses_attribute_presence() {
let mut g = test_support::minimal_graph();
let mut node = test_support::node_with_attrs("work", &[("prompt", "do it")]);
node.attrs
.insert("script".to_string(), AttrValue::Integer(123));
g.nodes.insert("work".to_string(), node);
let diagnostics = Rule.apply(&g);
assert_eq!(diagnostics.len(), 1);
assert_eq!(diagnostics[0].rule, "script_prompt_conflict");
}
#[test]
fn warns_on_prompt_on_start_and_command_nodes() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes
.get_mut("start")
.expect("minimal graph has start")
@ -126,7 +220,7 @@ mod tests {
#[test]
fn warns_on_duration_on_command_node() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
node_with_attr("run", "parallelogram", "duration", "30s"),
@ -139,7 +233,7 @@ mod tests {
#[test]
fn warns_on_parallel_attrs_on_agent_node() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
let mut node = Node::new("work");
node.attrs
.insert("max_parallel".to_string(), AttrValue::Integer(4));
@ -150,7 +244,7 @@ mod tests {
#[test]
fn warns_on_output_retries_on_command_node() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
node_with_attr("run", "parallelogram", "output_retries", "2"),
@ -165,7 +259,7 @@ mod tests {
#[test]
fn accepts_attrs_on_their_own_handler_types() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"run".to_string(),
node_with_attr("run", "parallelogram", "script", "echo hi"),
@ -202,12 +296,19 @@ mod tests {
"human".to_string(),
node_with_attr("human", "hexagon", "review_target", "true"),
);
g.nodes.insert(
"legacy_command".to_string(),
test_support::node_with_attrs("legacy_command", &[
("type", "tool"),
("script", "echo legacy"),
]),
);
assert!(Rule.apply(&g).is_empty());
}
#[test]
fn warns_on_review_target_on_non_human_node() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"work".to_string(),
node_with_attr("work", "box", "review_target", "true"),
@ -222,7 +323,7 @@ mod tests {
#[test]
fn accepts_prompt_on_shapeless_node_defaulting_to_agent() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
let mut node = Node::new("work");
node.attrs.insert(
"prompt".to_string(),
@ -234,7 +335,7 @@ mod tests {
#[test]
fn accepts_prompt_on_fan_in_judge() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"merge".to_string(),
node_with_attr("merge", "tripleoctagon", "prompt", "pick the best"),
@ -244,7 +345,7 @@ mod tests {
#[test]
fn ignores_unclassifiable_node_shapes() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
g.nodes.insert(
"odd".to_string(),
node_with_attr("odd", "doubleoctagon", "script", "echo hi"),
@ -254,7 +355,7 @@ mod tests {
#[test]
fn ignores_handler_specific_attrs_on_unrecognized_explicit_types() {
let mut g = minimal_graph();
let mut g = test_support::minimal_graph();
let mut node = Node::new("custom");
node.attrs.insert(
"type".to_string(),

View file

@ -23,7 +23,6 @@ mod reachability;
mod reserved_keyword_node_id;
mod retry_target_exists;
mod script_absolute_cd;
mod script_prompt_conflict;
mod selection_valid;
mod start_no_incoming;
mod start_node;
@ -63,7 +62,6 @@ pub fn built_in_rules() -> Vec<Box<dyn LintRule>> {
all_conditional_edges::rule(),
orphan_custom_outcome::rule(),
script_absolute_cd::rule(),
script_prompt_conflict::rule(),
command_requires_script::rule(),
import_error::rule(),
join_policy_removed::rule(),

View file

@ -1,135 +0,0 @@
use fabro_graphviz::graph::Graph;
use crate::{Diagnostic, LintRule, Severity};
pub(super) fn rule() -> Box<dyn LintRule> {
Box::new(Rule)
}
struct Rule;
impl LintRule for Rule {
fn name(&self) -> &'static str {
"script_prompt_conflict"
}
fn apply(&self, graph: &Graph) -> Vec<Diagnostic> {
let mut diagnostics = Vec::new();
for node in graph.nodes.values() {
if node.script().is_none() || node.prompt().is_none() {
continue;
}
diagnostics.push(Diagnostic {
rule: self.name().to_string(),
severity: Severity::Error,
message: format!(
"Node '{}' sets both 'script' and 'prompt'. No node type reads both: \
'script' selects the command handler and 'prompt' selects an LLM handler",
node.id
),
node_id: Some(node.id.clone()),
edge: None,
fix: Some(
"Remove whichever attribute is wrong, or split the node into a command node \
and an agent node"
.to_string(),
),
..Diagnostic::default()
});
}
diagnostics
}
}
#[cfg(test)]
mod tests {
use fabro_graphviz::graph::{AttrValue, Node};
use super::Rule;
use crate::rules::test_support::minimal_graph;
use crate::{LintRule, Severity};
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 errors_when_shapeless_node_sets_both() {
let mut g = minimal_graph();
g.nodes.insert(
"work".to_string(),
node_with("work", &[("script", "cargo build"), ("prompt", "do it")]),
);
let d = Rule.apply(&g);
assert_eq!(d.len(), 1);
assert_eq!(d[0].severity, Severity::Error);
assert!(d[0].message.contains("'script'"));
assert!(d[0].message.contains("'prompt'"));
assert_eq!(d[0].node_id.as_deref(), Some("work"));
}
#[test]
fn errors_even_when_shape_is_explicit() {
// An explicit shape resolves the handler, but the unread attribute is
// still a mistake, and reporting it the same way everywhere keeps
// adding a shape from turning an error into a warning.
let mut g = minimal_graph();
g.nodes.insert(
"run".to_string(),
node_with("run", &[
("shape", "parallelogram"),
("script", "cargo build"),
("prompt", "do it"),
]),
);
g.nodes.insert(
"plan".to_string(),
node_with("plan", &[
("shape", "box"),
("script", "cargo build"),
("prompt", "do it"),
]),
);
let d = Rule.apply(&g);
assert_eq!(d.len(), 2);
assert!(d.iter().all(|d| d.severity == Severity::Error));
}
#[test]
fn accepts_either_attribute_alone() {
let mut g = minimal_graph();
g.nodes.insert(
"build".to_string(),
node_with("build", &[("script", "cargo build")]),
);
g.nodes.insert(
"plan".to_string(),
node_with("plan", &[("prompt", "do it")]),
);
assert!(Rule.apply(&g).is_empty());
}
#[test]
fn ignores_legacy_tool_command_attribute() {
let mut g = minimal_graph();
g.nodes.insert(
"work".to_string(),
node_with("work", &[
("tool_command", "cargo build"),
("prompt", "do it"),
]),
);
assert!(Rule.apply(&g).is_empty());
}
}

View file

@ -1,5 +1,14 @@
use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node};
pub(crate) fn node_with_attrs(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
}
pub(crate) fn minimal_graph() -> Graph {
let mut g = Graph::new("test");
let mut start = Node::new("start");

View file

@ -18,6 +18,10 @@ fn timeout_ms(node: &Node) -> Option<u64> {
node.timeout().map(crate::millis_u64)
}
fn non_blank_script(node: &Node) -> Option<&str> {
node.script().filter(|script| !script.trim().is_empty())
}
/// Executes an external script configured via node attributes.
pub struct CommandHandler;
@ -31,7 +35,9 @@ impl Handler for CommandHandler {
_run_dir: &Path,
_services: &EngineServices,
) -> Result<Outcome, Error> {
let script = node.script().unwrap_or("");
let Some(script) = non_blank_script(node) else {
return Ok(Outcome::fail_classify("No script specified"));
};
let mut outcome = Outcome::simulated(&node.id);
outcome.notes = Some(format!("[Simulated] Command skipped: {script}"));
@ -49,11 +55,9 @@ impl Handler for CommandHandler {
run_dir: &Path,
services: &EngineServices,
) -> Result<Outcome, Error> {
let script = node.script().unwrap_or("");
if script.is_empty() {
let Some(script) = non_blank_script(node) else {
return Ok(Outcome::fail_classify("No script specified"));
}
};
let language = node
.attrs
@ -392,22 +396,38 @@ mod tests {
}
#[tokio::test]
async fn script_handler_no_script() {
async fn missing_and_blank_scripts_fail_execution_and_simulation() {
let handler = CommandHandler;
let node = Node::new("script_node");
let context = Context::new();
let graph = Graph::new("test");
let run_dir = tempfile::tempdir().unwrap();
let services = make_services();
let outcome = handler
.execute(&node, &context, &graph, run_dir.path(), &services)
.await
.unwrap();
assert_eq!(outcome.status, StageOutcome::Failed {
retry_requested: false,
});
assert_eq!(outcome.failure_reason(), Some("No script specified"));
for script in [None, Some(" \t\n")] {
let mut node = Node::new("script_node");
if let Some(script) = script {
node.attrs
.insert("script".to_string(), AttrValue::String(script.to_string()));
}
let outcomes = [
handler
.execute(&node, &context, &graph, run_dir.path(), &services)
.await
.unwrap(),
handler
.simulate(&node, &context, &graph, run_dir.path(), &services)
.await
.unwrap(),
];
for outcome in outcomes {
assert_eq!(outcome.status, StageOutcome::Failed {
retry_requested: false,
});
assert_eq!(outcome.failure_reason(), Some("No script specified"));
}
}
}
#[tokio::test]

View file

@ -153,18 +153,17 @@ impl Node {
self.str_attr("label").unwrap_or(&self.id)
}
/// The node's Graphviz shape, which selects its handler.
/// The node's Graphviz shape, which contributes to handler selection.
///
/// An explicit `shape` attribute always wins. Otherwise the shape is
/// inferred from the attributes the node carries: `script` is read by the
/// command handler and by nothing else, so a shapeless node that sets it
/// is a command node. Everything else falls back to `box` (agent).
/// An explicit `shape` or `type` attribute disables inference. Otherwise,
/// the presence of `script` infers `parallelogram`. Everything else falls
/// back to `box`.
#[must_use]
pub fn shape(&self) -> &str {
if let Some(shape) = self.str_attr("shape") {
return shape;
}
if self.script().is_some() {
if self.node_type().is_none() && self.attrs.contains_key("script") {
return "parallelogram";
}
"box"
@ -333,8 +332,10 @@ impl Node {
/// mapping.
#[must_use]
pub fn handler_type(&self) -> Option<&str> {
if let Some(t) = self.node_type() {
return Some(t);
match self.node_type() {
Some("tool") => return Some("command"),
Some(node_type) => return Some(node_type),
None => {}
}
shape_to_handler_type(self.shape())
}
@ -623,6 +624,7 @@ mod tests {
assert_eq!(node.shape(), "box");
assert_eq!(node.node_type(), None);
assert_eq!(node.prompt(), None);
assert_eq!(node.script(), None);
assert_eq!(node.for_each(), None);
assert_eq!(node.output_schema(), None);
assert_eq!(node.output_retries(), 2);
@ -678,17 +680,30 @@ mod tests {
#[test]
fn explicit_type_wins_over_script_inference() {
let node = node_with("odd", &[("type", "agent"), ("script", "cargo build")]);
assert_eq!(node.shape(), "parallelogram");
assert_eq!(node.shape(), "box");
assert_eq!(node.handler_type(), Some("agent"));
}
#[test]
fn empty_script_still_infers_command() {
fn any_script_attribute_value_infers_command() {
// The command-requires-script lint reports this; inference only asks
// whether the attribute is present so the diagnostic lands on a
// command node rather than a silently-agent one.
let node = node_with("build", &[("script", "")]);
assert_eq!(node.shape(), "parallelogram");
let empty = node_with("empty", &[("script", "")]);
assert_eq!(empty.shape(), "parallelogram");
assert_eq!(empty.handler_type(), Some("command"));
let mut non_string = Node::new("non_string");
non_string
.attrs
.insert("script".to_string(), AttrValue::Integer(123));
assert_eq!(non_string.shape(), "parallelogram");
assert_eq!(non_string.handler_type(), Some("command"));
}
#[test]
fn legacy_tool_type_resolves_to_command() {
let node = node_with("build", &[("type", "tool")]);
assert_eq!(node.handler_type(), Some("command"));
}