mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-14 04:12:08 +00:00
A node with no `shape` defaulted to `box`, which resolves to the agent handler. That made a shapeless `script` node run as an LLM call prompted with its own label, while the `script` was reported as inert — wrong behavior behind a warning. `script` is read by the command handler and by nothing else, so a shapeless node that sets it is unambiguously a command node. `shape()` now infers `parallelogram` in that case. An explicit `shape` still wins. Two rules keep the inference honest: - `script_prompt_conflict` — setting both `script` and `prompt` is an error. No handler reads both. It fires regardless of shape so that adding one cannot downgrade the error to a warning. - `command_requires_script` — a command node without a script is an error. Without this the original trap just moves: a node meant as a command that omits its script silently becomes an agent again. Also drops the `tool_command` alias in favor of `script` alone, routing the six read sites through a new `Node::script()` accessor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
158 lines
4.8 KiB
Rust
158 lines
4.8 KiB
Rust
use fabro_graphviz::graph::Graph;
|
|
|
|
use crate::{Diagnostic, LintRule, Severity};
|
|
|
|
pub(super) fn rule() -> Box<dyn LintRule> {
|
|
Box::new(Rule)
|
|
}
|
|
|
|
struct Rule;
|
|
|
|
/// Returns true if `text` contains `cd` followed by whitespace and then `/`.
|
|
fn contains_cd_absolute(text: &str) -> bool {
|
|
let bytes = text.as_bytes();
|
|
let len = bytes.len();
|
|
let mut i = 0;
|
|
while i + 3 < len {
|
|
if bytes[i] == b'c' && bytes[i + 1] == b'd' && bytes[i + 2].is_ascii_whitespace() {
|
|
// found "cd<ws>", scan past remaining whitespace to check for '/'
|
|
let mut j = i + 2;
|
|
while j < len && bytes[j].is_ascii_whitespace() {
|
|
j += 1;
|
|
}
|
|
if j < len && bytes[j] == b'/' {
|
|
return true;
|
|
}
|
|
}
|
|
i += 1;
|
|
}
|
|
false
|
|
}
|
|
|
|
impl LintRule for Rule {
|
|
fn name(&self) -> &'static str {
|
|
"script_absolute_cd"
|
|
}
|
|
|
|
fn apply(&self, graph: &Graph) -> Vec<Diagnostic> {
|
|
let mut diagnostics = Vec::new();
|
|
for node in graph.nodes.values() {
|
|
if node.handler_type() != Some("command") {
|
|
continue;
|
|
}
|
|
let script = node.script().unwrap_or("");
|
|
if contains_cd_absolute(script) {
|
|
diagnostics.push(Diagnostic {
|
|
rule: self.name().to_string(),
|
|
severity: Severity::Warning,
|
|
message: format!(
|
|
"Script node '{}' contains `cd /…` with an absolute path",
|
|
node.id
|
|
),
|
|
node_id: Some(node.id.clone()),
|
|
edge: None,
|
|
fix: Some(
|
|
"Use a relative path; the engine sets the working directory to the worktree automatically"
|
|
.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};
|
|
|
|
#[test]
|
|
fn script_absolute_cd_warns_on_cd_abs_path() {
|
|
let mut g = minimal_graph();
|
|
let mut node = Node::new("run");
|
|
node.attrs.insert(
|
|
"shape".to_string(),
|
|
AttrValue::String("parallelogram".to_string()),
|
|
);
|
|
node.attrs.insert(
|
|
"script".to_string(),
|
|
AttrValue::String("cd /tmp && ls".to_string()),
|
|
);
|
|
g.nodes.insert("run".to_string(), node);
|
|
let rule = Rule;
|
|
let d = rule.apply(&g);
|
|
assert_eq!(d.len(), 1);
|
|
assert_eq!(d[0].severity, Severity::Warning);
|
|
}
|
|
|
|
#[test]
|
|
fn script_absolute_cd_no_warning_on_relative() {
|
|
let mut g = minimal_graph();
|
|
let mut node = Node::new("run");
|
|
node.attrs.insert(
|
|
"shape".to_string(),
|
|
AttrValue::String("parallelogram".to_string()),
|
|
);
|
|
node.attrs.insert(
|
|
"script".to_string(),
|
|
AttrValue::String("cd src && ls".to_string()),
|
|
);
|
|
g.nodes.insert("run".to_string(), node);
|
|
let rule = Rule;
|
|
let d = rule.apply(&g);
|
|
assert!(d.is_empty());
|
|
}
|
|
|
|
#[test]
|
|
fn script_absolute_cd_warns_on_shapeless_script_node() {
|
|
let mut g = minimal_graph();
|
|
let mut node = Node::new("run");
|
|
node.attrs.insert(
|
|
"script".to_string(),
|
|
AttrValue::String("cd /home/user && make".to_string()),
|
|
);
|
|
g.nodes.insert("run".to_string(), node);
|
|
let rule = Rule;
|
|
let d = rule.apply(&g);
|
|
assert_eq!(d.len(), 1);
|
|
assert_eq!(d[0].severity, Severity::Warning);
|
|
}
|
|
|
|
#[test]
|
|
fn script_absolute_cd_ignores_legacy_tool_command() {
|
|
let mut g = minimal_graph();
|
|
let mut node = Node::new("run");
|
|
node.attrs.insert(
|
|
"shape".to_string(),
|
|
AttrValue::String("parallelogram".to_string()),
|
|
);
|
|
node.attrs.insert(
|
|
"tool_command".to_string(),
|
|
AttrValue::String("cd /home/user && make".to_string()),
|
|
);
|
|
g.nodes.insert("run".to_string(), node);
|
|
let rule = Rule;
|
|
assert!(rule.apply(&g).is_empty());
|
|
}
|
|
|
|
#[test]
|
|
fn script_absolute_cd_skips_non_script_nodes() {
|
|
let mut g = minimal_graph();
|
|
let mut node = Node::new("gen");
|
|
node.attrs
|
|
.insert("shape".to_string(), AttrValue::String("box".to_string()));
|
|
node.attrs.insert(
|
|
"prompt".to_string(),
|
|
AttrValue::String("cd /tmp and do stuff".to_string()),
|
|
);
|
|
g.nodes.insert("gen".to_string(), node);
|
|
let rule = Rule;
|
|
let d = rule.apply(&g);
|
|
assert!(d.is_empty());
|
|
}
|
|
}
|