diff --git a/docs/public/workflows/stages-and-nodes.mdx b/docs/public/workflows/stages-and-nodes.mdx index 819befe7e..5ab21481a 100644 --- a/docs/public/workflows/stages-and-nodes.mdx +++ b/docs/public/workflows/stages-and-nodes.mdx @@ -104,7 +104,7 @@ test [label="Run Tests", shape=parallelogram, script="cargo test 2>&1 || true"] | Attribute | Description | |---|---| -| `script` | The shell command to execute | +| `script` | The shell command to execute. Substitutes `{{ goal }}`, `{{ inputs.NAME }}`, and `{{ vars.NAME }}` — see [command node scripts](/workflows/variables#command-node-scripts) | | `language` | `"shell"` (default) or `"python"` | ### Human diff --git a/docs/public/workflows/variables.mdx b/docs/public/workflows/variables.mdx index fb42d126b..9e9857571 100644 --- a/docs/public/workflows/variables.mdx +++ b/docs/public/workflows/variables.mdx @@ -3,7 +3,7 @@ title: "Variables" description: "Using templates in workflows" --- -Fabro renders `{{ ... }}` templates in exactly two workflow attributes: the graph `goal` and node `prompt`s. Every other attribute is literal text. +Fabro renders `{{ ... }}` templates in exactly two workflow attributes: the graph `goal` and node `prompt`s. A command node's `script` gets narrower treatment — [simple value substitution](#command-node-scripts), not templating. Every other attribute is literal text. ## Template context @@ -51,7 +51,7 @@ digraph Check { } ``` -Other attributes — `script`, `label`, `model`, `provider`, `condition`, and all edge attributes — do not render templates. If one of them contains `{{ … }}` or `{% … %}`, the syntax is treated as literal text and Fabro records a `detemplated_attribute` warning suggesting you move the dynamic value into a `prompt` or `goal`. +Other attributes — `label`, `model`, `provider`, `condition`, and all edge attributes — do not render templates. If one of them contains `{{ … }}` or `{% … %}`, the syntax is treated as literal text and Fabro records a `detemplated_attribute` warning suggesting you move the dynamic value into a `prompt` or `goal`. Override individual inputs at run time with repeatable `-I` / `--input` flags: @@ -61,6 +61,62 @@ fabro run .fabro/workflows/check/workflow.toml -I repo_name=fabro-2 --input lang CLI input values use TOML scalar parsing when possible. Quoted strings, booleans, integers, and floats keep their typed values; unquoted bare text falls back to a string. Empty values such as `foo=` are accepted as empty strings. Arrays, inline tables, and datetimes are rejected. +## Command node scripts + +A [command node](/workflows/stages-and-nodes#command) `script` substitutes `{{ goal }}`, `{{ inputs.NAME }}`, and `{{ vars.NAME }}`: + +```dot title="check.fabro" +digraph Check { + test [shape=parallelogram, script="cargo test -p {{ inputs.crate }} --profile {{ vars.PROFILE }}"] + pr [shape=parallelogram, script="gh pr create --title {{ goal }}"] +} +``` + +This is value substitution, not templating. Only those three forms are recognized; every other brace sequence reaches the shell untouched, so `jq` filters, `awk` programs, Go templates, and brace expansion all keep working: + +```dot +report [shape=parallelogram, script="kubectl get pod -o go-template='{{ .status.phase }}' | jq '{phase: .}'"] +``` + +There is no `{% if %}`, no filters, and no loops, and `{{ goal }}` has no dotted form — `{{ goal.title }}` stays literal. Put branching in a [conditional node](/workflows/stages-and-nodes#conditional) or in the shell itself. + +In a shell script, Fabro quotes each substituted value as one shell argument. Put the token where one shell word is valid. Do not add quotes around the token, and do not use a token to inject multiple flags or shell syntax: + +```dot +build [shape=parallelogram, script="docker build -t {{ inputs.image }} ."] +``` + +For `language="python"`, Fabro inserts each value as a quoted string literal. Put the token where a Python expression is valid: + +```dot +report [shape=parallelogram, language="python", script="print({{ goal }})"] +``` + +Substituted text is never scanned again, so a goal or input containing `{{ ... }}` reaches the command as literal characters rather than being interpolated a second time. + +### `env` and `secrets` are not substituted + +`{{ env.NAME }}` and `{{ secrets.NAME }}` are rejected in a `script` with a validation error. Read environment variables with `$NAME` in a shell script: + +```dot +deploy [shape=parallelogram, script="deploy --token $DEPLOY_TOKEN"] +``` + +In a Python script, read them with `os.environ["NAME"]`. + +To make a secret available that way, put it in the [environment's env map](/execution/run-configuration#run-environment-and-environments-slug), where `{{ secrets.* }}` does resolve — at run start, into the sandbox environment rather than into the script text: + +```toml title="run.toml" +[environments.ci.env] +DEPLOY_TOKEN = "{{ secrets.DEPLOY_TOKEN }}" +``` + +This keeps secret values out of the `command.started` event, which records the script verbatim. + +### When values resolve + +Inputs and variables are substituted when the run is created, at the same time as goals and prompts — the persisted workflow already contains the final script. An unbound input or variable is a warning from `fabro validate` and an error at run creation, so a run never executes a partially substituted command. + ## Server-managed run config variables Use server-managed variables for non-sensitive values that should be shared across runs, such as deployment environments, default branches, regions, or image tags: @@ -115,8 +171,9 @@ Fabro keeps workflow structure static and renders workflow templates once: 2. Literal `import`, `@file`, graph-goal file, and child-workflow references are resolved. 3. The graph `goal` is rendered with the `{ inputs, vars }` context. 4. Node `prompt` attributes are rendered with the `{ goal, inputs, vars }` context. +5. Node `script` attributes have their `{{ goal }}`, `{{ inputs.* }}`, and `{{ vars.* }}` values substituted. -Templates are not supported in graph syntax, node IDs, edge structure, `import` paths, `@file` paths, child workflow paths, other file references, or any attribute besides `prompt` and `goal`. +Templates are not supported in graph syntax, node IDs, edge structure, `import` paths, `@file` paths, child workflow paths, other file references, or any attribute besides `prompt` and `goal` — and `script`, which takes value substitution rather than templates. Fabro renders the graph `goal` first and stores the rendered value back onto the graph. Prompts that use `{{ goal }}` receive that rendered value. @@ -124,6 +181,8 @@ Fabro renders the graph `goal` first and stores the rendered value back onto the Fabro renders undefined workflow variables as empty text and records a `template_undefined_variable` diagnostic. `fabro validate` reports that diagnostic as a warning so you can validate workflow structure before all inputs are known. Offline validation does not read a server's variable store, so `{{ vars.* }}` references also warn there. Run-style commands such as `fabro run`, `fabro create`, and preflight use the server snapshot and promote any still-undefined reference to an error before proceeding. +In a `script`, an undefined value records the same diagnostic but leaves the token in place rather than emptying it, so validation output shows what is unbound. + ## Template includes Prompt and goal templates support static MiniJinja loader dependencies such as `{% include "partial.md" %}`. Includes are resolved relative to the template file being rendered and can be nested. diff --git a/lib/components/fabro-workflow/src/error.rs b/lib/components/fabro-workflow/src/error.rs index 1525c570c..f8c08ca15 100644 --- a/lib/components/fabro-workflow/src/error.rs +++ b/lib/components/fabro-workflow/src/error.rs @@ -7,7 +7,7 @@ use fabro_model::ModelSelectionError; use fabro_template::TemplateError; pub use fabro_types::failure_signature::FailureSignature; pub use fabro_types::outcome::FailureCategory; -use fabro_types::settings::AmbiguousModelRef; +use fabro_types::settings::{AmbiguousModelRef, ResolveError}; use fabro_types::{ExecOutputTail, FailureReason, RunFailure}; use fabro_util::error::{SharedError, collect_causes, collect_chain, render_with_causes}; use fabro_validate::Diagnostic; @@ -280,6 +280,14 @@ pub enum Error { #[error("Validation failed")] ValidationFailed { diagnostics: Vec }, + #[error("Validation error: script interpolation failed in {owner}: {source} ({fix})")] + ScriptInterpolation { + owner: String, + fix: String, + #[source] + source: ResolveError, + }, + #[error("Model selection failed: {0}")] ModelSelection(#[from] ModelSelectionError), @@ -451,6 +459,7 @@ impl Error { .as_ref() .map_or_else(Vec::new, |source| collect_chain(source)), Self::Template { source, .. } => collect_chain(source), + Self::ScriptInterpolation { source, .. } => collect_chain(source), Self::Llm(err) => collect_causes(err), _ => Vec::new(), } @@ -478,6 +487,7 @@ impl Error { Self::Parse(_) | Self::Validation(_) | Self::ValidationFailed { .. } + | Self::ScriptInterpolation { .. } | Self::ModelSelection(_) | Self::ModelReference(_) | Self::Template { .. } @@ -501,6 +511,7 @@ impl Error { Self::Parse(_) | Self::Validation(_) | Self::ValidationFailed { .. } + | Self::ScriptInterpolation { .. } | Self::ModelSelection(_) | Self::ModelReference(_) | Self::Template { .. } diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 4f810cb54..c5d08cb7e 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -722,6 +722,58 @@ reasoning = false assert!(validated.has_errors()); } + #[test] + fn vars_resolve_in_command_script_through_create_pipeline() { + let dot = r#"digraph Test { + graph [goal="Ship it"] + start [shape=Mdiamond, label="Start"] + exit [shape=Msquare, label="Exit"] + work [label="Work", shape=parallelogram, script="deploy --stage {{ vars.STAGE }}"] + start -> work -> exit + }"#; + let vars = HashMap::from([("STAGE".to_string(), "staging".to_string())]); + let validated = validate_dot_with_vars(dot, vars); + validated.raise_on_errors().unwrap(); + + assert_eq!( + validated.graph().nodes["work"] + .attrs + .get("script") + .and_then(fabro_graphviz::graph::AttrValue::as_str), + Some("deploy --stage staging"), + ); + } + + /// The script diagnostic must carry the same rule as the prompt one so the + /// existing run-create promotion catches an unbound value before a run + /// executes a half-interpolated command. + #[test] + fn unknown_input_in_script_warns_at_validate_then_errors_at_run_create() { + let dot = r#"digraph Test { + graph [goal="Ship it"] + start [shape=Mdiamond, label="Start"] + exit [shape=Msquare, label="Exit"] + work [label="Work", shape=parallelogram, script="deploy --stage {{ inputs.stage }}"] + start -> work -> exit + }"#; + let mut validated = validate_dot_with_vars(dot, HashMap::new()); + + let diagnostic = validated + .diagnostics() + .iter() + .find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) + .expect("expected a template_undefined_variable diagnostic"); + assert_eq!(diagnostic.severity, Severity::Warning); + assert!( + diagnostic.message.contains("inputs.stage"), + "message: {}", + diagnostic.message + ); + + validated.promote_template_undefined_variables_to_errors(); + assert!(validated.has_errors()); + } + #[test] fn promote_template_undefined_rule_turns_warning_into_error() { let dot = r#"digraph Test { diff --git a/lib/components/fabro-workflow/src/pipeline/transform.rs b/lib/components/fabro-workflow/src/pipeline/transform.rs index bf3b82471..10172bf8f 100644 --- a/lib/components/fabro-workflow/src/pipeline/transform.rs +++ b/lib/components/fabro-workflow/src/pipeline/transform.rs @@ -3,8 +3,8 @@ use std::sync::Arc; use super::types::{Parsed, TransformOptions, Transformed}; use crate::error::Error; use crate::transforms::{ - FileInliningTransform, ImportTransform, StylesheetApplicationTransform, TemplateTransform, - Transform, + FileInliningTransform, ImportTransform, ScriptInterpolationTransform, + StylesheetApplicationTransform, TemplateTransform, Transform, }; /// TRANSFORM phase: apply built-in and custom transforms to a parsed graph. @@ -62,6 +62,13 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result model_resolution.apply(graph)?, @@ -238,6 +245,92 @@ mod tests { ); } + #[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 { diff --git a/lib/components/fabro-workflow/src/transforms/mod.rs b/lib/components/fabro-workflow/src/transforms/mod.rs index 1f6879cda..8246ee750 100644 --- a/lib/components/fabro-workflow/src/transforms/mod.rs +++ b/lib/components/fabro-workflow/src/transforms/mod.rs @@ -22,4 +22,4 @@ pub use import::ImportTransform; pub use model_resolution::ModelResolutionTransform; pub use preamble::PreambleTransform; pub use stylesheet_application::StylesheetApplicationTransform; -pub use variable_expansion::{RenderMode, TemplateTransform}; +pub use variable_expansion::{RenderMode, ScriptInterpolationTransform, TemplateTransform}; diff --git a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs index e739c21a4..e2c11d169 100644 --- a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs +++ b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs @@ -1,13 +1,17 @@ +use std::borrow::Cow; use std::collections::HashMap; use std::fmt::Write as _; use std::sync::Arc; -use fabro_graphviz::graph::{AttrValue, Graph}; +use fabro_graphviz::graph::{AttrValue, Graph, Node}; use fabro_template::{ TemplateContext, TemplateError, TemplateRenderMode, TemplateSource, TemplateSourceOrigin, TemplateStore, }; +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 fabro_validate::{Diagnostic, Severity}; use super::Transform; @@ -223,9 +227,7 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) -> message, node_id: target.node_id.clone(), edge: target.edge.clone(), - fix: Some(format!( - "bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=`" - )), + fix: Some(input_binding_fix(name)), source_path: location.source_name.or_else(|| target.source_name.clone()), line: location.line, column: location.column, @@ -235,6 +237,143 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) -> } } +fn input_binding_fix(name: &str) -> String { + format!("bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=`") +} + +/// 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 => format!("set it with `fabro variable set {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 => "set a graph `goal` on the workflow".to_string(), + } +} + const DETEMPLATED_ATTRIBUTE_RULE: &str = "detemplated_attribute"; /// Warning emitted when an attribute that is no longer a template still @@ -245,7 +384,9 @@ fn detemplated_attribute_diagnostic(attr_name: &str, target: &TemplateRenderTarg severity: Severity::Warning, message: format!( "`{attr_name}` in {} is no longer a template; `{{{{ … }}}}` / `{{% … %}}` is treated \ - as literal text. Only node `prompt` and graph `goal` support templating.", + as literal text. Only node `prompt` and graph `goal` support templating, and node \ + command `script` supports `{{{{ goal }}}}`, `{{{{ inputs.* }}}}`, and \ + `{{{{ vars.* }}}}` interpolation.", target.owner ), node_id: target.node_id.clone(), @@ -290,8 +431,8 @@ fn goal_self_reference_diagnostic( } } -/// Expands `{{ goal }}` / `{{ inputs.* }}` / `{{ vars.* }}` across all string -/// attributes. +/// 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, @@ -374,6 +515,11 @@ impl TemplateTransform { 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) .map_err(|error| Error::Validation(error.to_string()))?; @@ -383,7 +529,8 @@ impl TemplateTransform { .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 templated node attribute. + // `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) { @@ -473,6 +620,83 @@ impl Transform for TemplateTransform { } } +/// 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; @@ -557,6 +781,322 @@ mod tests { ); } + /// 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"); diff --git a/lib/foundation/fabro-template/src/lib.rs b/lib/foundation/fabro-template/src/lib.rs index a32a13dd7..f312e9416 100644 --- a/lib/foundation/fabro-template/src/lib.rs +++ b/lib/foundation/fabro-template/src/lib.rs @@ -97,6 +97,35 @@ impl TemplateContext { Self::new().with_goal("{{ goal }}").with_inputs(inputs) } + /// The rendered goal `{{ goal }}` resolves to, or `None` before the goal + /// is known. + #[must_use] + pub fn goal(&self) -> Option<&str> { + self.goal.as_deref() + } + + /// The text `{{ inputs.NAME }}` renders to, or `None` when unbound. + /// + /// Non-template consumers — currently command node `script` interpolation, + /// which uses `InterpString` tokens rather than MiniJinja — read values + /// through here so an input produces the same text in a script as it does + /// in a prompt. + #[must_use] + pub fn input(&self, name: &str) -> Option { + Self::rendered_member(&self.inputs, name) + } + + /// The text `{{ vars.NAME }}` renders to, or `None` when unset. + #[must_use] + pub fn var(&self, name: &str) -> Option { + Self::rendered_member(&self.vars, name) + } + + fn rendered_member(container: &Value, name: &str) -> Option { + let member = container.get_attr(name).ok()?; + (!member.is_undefined()).then(|| member.to_string()) + } + fn into_value(self) -> Value { let goal = self.goal.map(Value::from); let inputs = self.inputs; @@ -786,6 +815,53 @@ mod tests { assert_eq!(rendered, "Goal: Fix bugs"); } + #[test] + fn workflow_values_return_none_when_unbound() { + let ctx = TemplateContext::new(); + + assert_eq!(ctx.goal(), None); + assert_eq!(ctx.input("missing"), None); + assert_eq!(ctx.var("MISSING"), None); + } + + #[test] + fn goal_returns_the_rendered_value() { + let ctx = TemplateContext::new().with_goal("Ship it"); + + assert_eq!(ctx.goal(), Some("Ship it")); + } + + /// `input`/`var` exist so non-template consumers render a value the same + /// way a prompt does. Pin that equivalence across the scalar TOML types. + #[test] + fn input_matches_what_a_template_renders() { + let ctx = TemplateContext::new().with_inputs(HashMap::from([ + ("name".to_string(), toml::Value::String("fabro".into())), + ("attempts".to_string(), toml::Value::Integer(3)), + ("ratio".to_string(), toml::Value::Float(1.5)), + ("fast".to_string(), toml::Value::Boolean(true)), + ])); + + for name in ["name", "attempts", "ratio", "fast"] { + let rendered = render(&format!("{{{{ inputs.{name} }}}}"), &ctx).unwrap(); + assert_eq!(ctx.input(name), Some(rendered), "mismatch for `{name}`"); + } + } + + #[test] + fn var_matches_what_a_template_renders() { + let ctx = TemplateContext::new().with_vars(HashMap::from([( + "STAGE".to_string(), + "staging".to_string(), + )])); + + assert_eq!(ctx.var("STAGE").as_deref(), Some("staging")); + assert_eq!( + ctx.var("STAGE"), + Some(render("{{ vars.STAGE }}", &ctx).unwrap()) + ); + } + #[test] fn references_top_level_variable_detects_goal_self_reference() { assert!(references_top_level_variable("Do {{ goal }} now", "goal")); diff --git a/lib/foundation/fabro-types/src/settings/interp.rs b/lib/foundation/fabro-types/src/settings/interp.rs index e31c3e8db..1152bd7fd 100644 --- a/lib/foundation/fabro-types/src/settings/interp.rs +++ b/lib/foundation/fabro-types/src/settings/interp.rs @@ -1,15 +1,19 @@ //! Interpolation for config strings. //! //! An [`InterpString`] field may contain narrow `{{ .NAME }}` -//! tokens — no template logic. Three [`Namespace`]s resolve here: `env`, -//! `vars`, and `secrets`. `inputs` is **template-only**: it is a -//! recognized namespace so an `{{ inputs.* }}` token fails loudly with a clear -//! message instead of passing through as literal text, but it never resolves -//! in an `InterpString` field — it belongs in prompts and goals. Which of the -//! resolvable namespaces actually apply is scope-determined by the caller -//! through [`ResolveCtx`]: server-scope settings provide `env` (and eventually -//! `secrets`), run-scope settings additionally provide `vars`. A token whose -//! namespace is not available in the resolution context fails loudly. +//! tokens — no template logic. Which [`Namespace`]s resolve is +//! scope-determined by the caller through [`ResolveCtx`]: server-scope +//! settings provide `env` (and eventually `secrets`), run-scope settings +//! additionally provide `vars`, and a command node `script` provides `inputs` +//! and `vars` plus the bare `goal` value. A token whose namespace has no lookup +//! in the resolution context fails loudly rather than passing through as +//! literal text. +//! +//! Most call sites do not wire `inputs`: it is bound where a run's typed +//! `[run.inputs]` values are in scope, which is the workflow graph, not +//! general config. Everywhere else an `{{ inputs.* }}` token still parses, so +//! it fails with a clear message instead of reaching a consumer as literal +//! text. //! //! Resolution timing is split: `vars` substitutes early (server-side, at run //! creation) via [`InterpString::substitute_with`], while `env`/`secrets` @@ -28,7 +32,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use crate::variable::is_env_style_name; /// A config string that may contain `{{ env.NAME }}`, `{{ vars.NAME }}`, -/// `{{ secrets.NAME }}`, or `{{ inputs.NAME }}` tokens. +/// `{{ secrets.NAME }}`, `{{ inputs.NAME }}`, or `{{ goal }}` tokens. #[derive(Debug, Clone, PartialEq, Eq)] pub struct InterpString { segments: Vec, @@ -43,6 +47,9 @@ enum Segment { }, } +/// The sole token body with no `namespace.name` shape. +const GOAL_TOKEN: &str = "goal"; + /// The interpolation namespaces recognized inside `{{ ... }}` tokens. #[derive( Debug, Clone, Copy, PartialEq, Eq, Hash, strum::Display, strum::EnumString, strum::IntoStaticStr, @@ -57,6 +64,12 @@ pub enum Namespace { Secrets, /// `{{ inputs.NAME }}` — workflow run inputs, substituted early. Inputs, + /// The bare `{{ goal }}` token — the run goal, substituted early. + /// + /// Unlike the others this names a single value rather than a namespace of + /// them, so it has no dotted form: only the exact body `goal` produces it, + /// and `{{ goal.anything }}` stays literal text. + Goal, } impl Namespace { @@ -67,15 +80,27 @@ impl Namespace { Self::Vars => "variable", Self::Secrets => "secret", Self::Inputs => "input", + Self::Goal => "run goal", } } + /// Whether this namespace is written as a bare token rather than + /// `namespace.name`. + fn is_bare(self) -> bool { + matches!(self, Self::Goal) + } + /// Parse a trimmed `{{ ... }}` token body into a namespace + name, or /// `None` when the body is not a recognized token (it then stays literal). fn parse_token(token: &str) -> Option<(Self, String)> { let trimmed = token.trim(); + if trimmed == GOAL_TOKEN { + return Some((Self::Goal, GOAL_TOKEN.to_owned())); + } let (prefix, name) = trimmed.split_once('.')?; - let namespace = prefix.parse::().ok()?; + // A bare namespace has no dotted spelling, so `{{ goal.title }}` is not + // a token and reaches the consumer as literal text. + let namespace = prefix.parse::().ok().filter(|ns| !ns.is_bare())?; namespace .is_valid_name(name) .then(|| (namespace, name.to_owned())) @@ -102,6 +127,7 @@ impl Namespace { } chars.all(|ch| ch.is_ascii_alphanumeric() || ch == '_' || ch == '-') } + Self::Goal => name == GOAL_TOKEN, } } } @@ -118,6 +144,8 @@ pub struct ResolveCtx<'a> { env: Option>, vars: Option>, secrets: Option>, + inputs: Option>, + goal: Option>, } type LookupFn<'a> = Box Option + 'a>; @@ -146,16 +174,37 @@ impl<'a> ResolveCtx<'a> { self } + /// Typed `[run.inputs]` values, available only where a run's inputs are in + /// scope. Leaving this unwired — which every config-layer call site does — + /// keeps `{{ inputs.* }}` unavailable, so `resolve_with` fails loudly and + /// `substitute_with` preserves the token for a goal (an `InterpString` + /// that feeds a template) to forward to the template layer. + #[must_use] + pub fn with_inputs(mut self, lookup: impl FnMut(&str) -> Option + 'a) -> Self { + self.inputs = Some(Box::new(lookup)); + self + } + + /// The rendered run goal, available only where a run's goal is in scope. + /// Like [`ResolveCtx::with_inputs`], leaving it unwired keeps + /// `{{ goal }}` unavailable for that call site. + /// + /// Takes the value rather than a lookup: unlike a namespace there is + /// nothing to look up by name, so a wired goal can never be `Missing`. + #[must_use] + pub fn with_goal(mut self, goal: impl Into) -> Self { + let goal = goal.into(); + self.goal = Some(Box::new(move |_| Some(goal.clone()))); + self + } + fn lookup_for(&mut self, namespace: Namespace) -> Option<&mut LookupFn<'a>> { match namespace { Namespace::Env => self.env.as_mut(), Namespace::Vars => self.vars.as_mut(), Namespace::Secrets => self.secrets.as_mut(), - // `inputs` is template-only: an `InterpString` resolve context - // never provides it, so an `{{ inputs.* }}` token is always - // unavailable here. `substitute_with` still preserves the token so a - // goal (an `InterpString` that feeds a template) can forward it. - Namespace::Inputs => None, + Namespace::Inputs => self.inputs.as_mut(), + Namespace::Goal => self.goal.as_mut(), } } } @@ -267,9 +316,11 @@ impl InterpString { Segment::Literal(text) => out.push_str(text), Segment::Token { namespace, name } => { out.push_str("{{ "); - out.push_str(namespace.into()); - out.push('.'); - out.push_str(name); + out.push_str((*namespace).into()); + if !namespace.is_bare() { + out.push('.'); + out.push_str(name); + } out.push_str(" }}"); } } @@ -309,7 +360,7 @@ impl InterpString { /// for the namespaces it does not — their resolution happens later, /// possibly in a different process. /// - /// This is the early, server-side pass (`vars`/`inputs`); late-bound + /// This is the early, server-side pass (`vars`/`inputs`/`goal`); late-bound /// namespaces (`env`/`secrets`) survive in token form for their /// consumption-time [`InterpString::resolve_with`]. pub fn substitute_with(&self, ctx: &mut ResolveCtx<'_>) -> Result { @@ -463,14 +514,19 @@ impl fmt::Display for ResolveError { self.name, self.name ), ResolveErrorKind::Unavailable => match namespace { - // `inputs` is template-only: it never resolves in an - // `InterpString` field. Point the user at where it works. + // `inputs` and `goal` resolve only where a run's values are in + // scope. Point the user at where that is. Namespace::Inputs => write!( f, - "{{{{ inputs.{} }}}} is only available in prompts and goals, not in other \ - config fields", + "{{{{ inputs.{} }}}} is only available in prompts, goals, and command node \ + `script` attributes, not in other config fields", self.name ), + Namespace::Goal => write!( + f, + "{{{{ goal }}}} is only available in prompts and command node `script` \ + attributes, not in other config fields" + ), _ => write!( f, "{noun} {:?} referenced by {{{{ {namespace}.{} }}}} is not supported in \ @@ -504,7 +560,7 @@ impl<'de> Deserialize<'de> for InterpString { fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str( "a string, optionally containing {{ env.NAME }}, {{ vars.NAME }}, \ - {{ secrets.NAME }}, or {{ inputs.NAME }} interpolation tokens", + {{ secrets.NAME }}, {{ inputs.NAME }}, or {{ goal }} interpolation tokens", ) } @@ -657,7 +713,8 @@ mod tests { s: InterpString, } - let input = r#"{"s":"{{ env.A }}/{{ vars.B }}/{{ secrets.C }}/{{ inputs.d-key }}"}"#; + let input = + r#"{"s":"{{ env.A }}/{{ vars.B }}/{{ secrets.C }}/{{ inputs.d-key }}/{{ goal }}"}"#; let parsed: Wrap = serde_json::from_str(input).unwrap(); let rendered = serde_json::to_string(&parsed).unwrap(); assert_eq!(rendered, input); @@ -754,10 +811,9 @@ mod tests { } #[test] - fn resolve_with_rejects_inputs_as_template_only() { - // `inputs` is template-only. An `{{ inputs.* }}` token never resolves - // in an `InterpString` field — it fails loudly, pointing the - // user at prompts and goals. + fn resolve_with_rejects_inputs_when_not_wired() { + // A context that does not opt into `inputs` fails loudly, pointing the + // user at the scopes where inputs do resolve. let s = InterpString::parse("run-{{ inputs.ticket-id }}"); let err = s.resolve_with(&mut ResolveCtx::new()).unwrap_err(); @@ -765,12 +821,115 @@ mod tests { assert_eq!(err.namespace, Namespace::Inputs); assert_eq!(err.kind, ResolveErrorKind::Unavailable); assert!( - err.to_string() - .contains("only available in prompts and goals"), + err.to_string().contains("only available in prompts, goals"), "unexpected message: {err}" ); } + #[test] + fn resolve_with_resolves_inputs_when_wired() { + let s = InterpString::parse("run-{{ inputs.ticket-id }}-{{ vars.STAGE }}"); + + let resolved = s + .resolve_with( + &mut ResolveCtx::new() + .with_inputs(lookup_from(&[("ticket-id", "4821")])) + .with_vars(lookup_from(&[("STAGE", "staging")])), + ) + .unwrap(); + + assert_eq!(resolved, "run-4821-staging"); + } + + #[test] + fn resolve_with_resolves_the_bare_goal_token_when_wired() { + let s = InterpString::parse("gh pr create --title \"{{ goal }}\""); + + let resolved = s + .resolve_with(&mut ResolveCtx::new().with_goal("Fix the login bug")) + .unwrap(); + + assert_eq!(resolved, "gh pr create --title \"Fix the login bug\""); + } + + #[test] + fn resolve_with_rejects_the_goal_token_when_not_wired() { + let s = InterpString::parse("echo {{ goal }}"); + + let err = s.resolve_with(&mut ResolveCtx::new()).unwrap_err(); + + assert_eq!(err.namespace, Namespace::Goal); + assert_eq!(err.kind, ResolveErrorKind::Unavailable); + assert!( + err.to_string().contains("{{ goal }} is only available"), + "unexpected message: {err}" + ); + } + + /// `goal` names a single value, so it has no dotted spelling. Anything of + /// the form `{{ goal.x }}` must stay literal rather than becoming a token + /// that could never resolve. + #[test] + fn goal_has_no_dotted_form() { + for source in ["{{ goal.title }}", "{{ goal. }}", "{{ goals }}"] { + let s = InterpString::parse(source); + let resolved = s + .resolve_with(&mut ResolveCtx::new().with_goal("Ship it")) + .unwrap_or_else(|err| panic!("`{source}` should stay literal, got: {err}")); + assert_eq!(resolved, source); + } + } + + /// Substituted text is output, not more input. A resolved value that + /// happens to contain token syntax must land verbatim rather than being + /// scanned again — otherwise a goal or input could smuggle in a token the + /// call site never wired. + #[test] + fn resolve_with_does_not_rescan_substituted_values() { + let s = InterpString::parse("{{ goal }} | {{ vars.PAYLOAD }}"); + + let resolved = s + .resolve_with( + &mut ResolveCtx::new() + .with_goal("{{ secrets.API_KEY }}") + .with_vars(lookup_from(&[("PAYLOAD", "{{ env.HOME }}")])), + ) + .unwrap(); + + assert_eq!(resolved, "{{ secrets.API_KEY }} | {{ env.HOME }}"); + } + + #[test] + fn goal_token_round_trips_through_source_form() { + // Unwired, `substitute_with` preserves the token; `as_source` must + // reproduce the bare spelling rather than `{{ goal.goal }}`. + let s = InterpString::parse("deploy # {{ goal }}"); + + let preserved = s.substitute_with(&mut ResolveCtx::new()).unwrap(); + + #[expect(clippy::disallowed_methods, reason = "asserting the source round-trip")] + let source = preserved.as_source(); + assert_eq!(source, "deploy # {{ goal }}"); + } + + /// Wiring `inputs` at one call site must not make it resolvable anywhere + /// else. Every config-layer context leaves it unwired, and this pins that + /// the availability stays per-context rather than global. + #[test] + fn wiring_inputs_does_not_leak_into_other_contexts() { + let s = InterpString::parse("{{ inputs.id }}"); + + let wired = s + .resolve_with(&mut ResolveCtx::new().with_inputs(lookup_from(&[("id", "7")]))) + .unwrap(); + assert_eq!(wired, "7"); + + let err = s + .resolve_with(&mut ResolveCtx::new().with_env(lookup_from(&[("id", "7")]))) + .unwrap_err(); + assert_eq!(err.kind, ResolveErrorKind::Unavailable); + } + #[test] fn substitute_variables_preserves_late_bound_tokens() { let s = @@ -825,5 +984,6 @@ mod tests { assert_eq!(Namespace::Vars.to_string(), "vars"); assert_eq!(Namespace::Secrets.to_string(), "secrets"); assert_eq!(Namespace::Inputs.to_string(), "inputs"); + assert_eq!(Namespace::Goal.to_string(), "goal"); } }