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..1b03917a9 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,54 @@ 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. + +Values are substituted as-is, without shell quoting — the script is yours to quote. If a value can contain spaces or shell metacharacters, quote it at the use site. This matters most for `{{ goal }}`, which is free-form prose: + +```dot +build [shape=parallelogram, script="docker build -t \"{{ inputs.image }}\" ."] +``` + +Substituted text is never scanned again, so a goal or input containing `{{ ... }}` reaches the shell 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. A script runs in a shell, so read environment variables the shell way: + +```dot +deploy [shape=parallelogram, script="deploy --token $DEPLOY_TOKEN"] +``` + +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 +163,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 +173,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/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 2ec40d443..61c069ade 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -711,6 +711,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/transforms/variable_expansion.rs b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs index 66434266a..9384a9b8f 100644 --- a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs +++ b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs @@ -7,6 +7,7 @@ use fabro_template::{ TemplateContext, TemplateError, TemplateRenderMode, TemplateSource, TemplateSourceOrigin, TemplateStore, }; +use fabro_types::settings::{InterpString, Namespace, ResolveCtx, ResolveError, ResolveErrorKind}; use fabro_util::error::collect_chain; use fabro_validate::{Diagnostic, Severity}; @@ -234,6 +235,107 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) -> } } +/// Substitutes `{{ inputs.* }}` and `{{ vars.* }}` in a command node `script`. +/// +/// Substitutes `{{ goal }}`, `{{ inputs.* }}`, and `{{ vars.* }}` in a 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. +/// +/// Resolved values are substituted verbatim, not shell-quoted — matching +/// `[[run.prepare.steps]].script`, where the shell snippet is the author's to +/// quote. Callers wanting a value treated as a single argument must quote it +/// in the script. This matters most for `{{ goal }}`, which is free-form prose. +fn interpolate_script( + text: &str, + ctx: &TemplateContext, + render_mode: RenderMode, + target: &TemplateRenderTarget, + diagnostics: &mut Vec, +) -> Result { + let mut resolve_ctx = ResolveCtx::new() + .with_inputs(|name| ctx.input(name)) + .with_vars(|name| ctx.var(name)); + // 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(goal); + } + match InterpString::parse(text).resolve_with(&mut resolve_ctx) { + Ok(resolved) => Ok(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)), + 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(text.to_string()) + } + }, + // 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)), + } +} + +fn script_interpolation_error(target: &TemplateRenderTarget, err: &ResolveError) -> Error { + Error::Validation(format!( + "script interpolation failed in {}: {err} ({})", + target.owner, + script_interpolation_fix(err) + )) +} + +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)), + source_path: target.source_name.clone(), + ..Diagnostic::default() + } +} + +fn script_interpolation_fix(err: &ResolveError) -> String { + let name = &err.name; + match err.namespace { + Namespace::Inputs => format!( + "bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=`" + ), + Namespace::Vars => format!("set it with `fabro variable set {name} `"), + Namespace::Env => format!( + "`script` does not interpolate environment variables; read it in the shell as \ + `${name}` instead" + ), + 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 @@ -244,7 +346,8 @@ 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 \ + `script` supports `{{{{ inputs.* }}}}` / `{{{{ vars.* }}}}` interpolation.", target.owner ), node_id: target.node_id.clone(), @@ -382,9 +485,14 @@ 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 matches!(scope, AttributeScope::Node) && attr_name == "script" { + // `script` interpolates a narrow token set instead — see + // `interpolate_script`. + *text = interpolate_script(text, ctx, render_mode, &target, diagnostics)?; } else if fabro_template::contains_template_syntax(text) { // Every other attribute is no longer a template (`label`, // `model`, `provider`, `speed`, `condition`, edge `label`, @@ -556,6 +664,229 @@ 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("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, + ) -> TemplateTransform { + TemplateTransform { + 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, + source_text: 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:?}"); + } + + /// 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}" + ); + } + + /// 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 transform = script_transform( + &[ + ("attempts", toml::Value::Integer(3)), + ("fast", toml::Value::Boolean(true)), + ], + &[], + RenderMode::Structural, + ); + + let (graph, _) = transform.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 transform = script_transform(&[], &[], RenderMode::Structural); + + let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap(); + + 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 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..bb4950663 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,45 @@ mod tests { assert_eq!(rendered, "Goal: Fix bugs"); } + #[test] + fn input_and_var_return_none_when_unbound() { + let ctx = TemplateContext::new(); + + assert_eq!(ctx.input("missing"), None); + assert_eq!(ctx.var("MISSING"), None); + } + + /// `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..b91c158ea 100644 --- a/lib/foundation/fabro-types/src/settings/interp.rs +++ b/lib/foundation/fabro-types/src/settings/interp.rs @@ -1,15 +1,18 @@ //! 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` only. 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` @@ -43,6 +46,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 +63,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 +79,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 +126,7 @@ impl Namespace { } chars.all(|ch| ch.is_ascii_alphanumeric() || ch == '_' || ch == '-') } + Self::Goal => name == GOAL_TOKEN, } } } @@ -118,6 +143,8 @@ pub struct ResolveCtx<'a> { env: Option>, vars: Option>, secrets: Option>, + inputs: Option>, + goal: Option>, } type LookupFn<'a> = Box Option + 'a>; @@ -146,16 +173,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 +315,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(" }}"); } } @@ -463,14 +513,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 \ @@ -754,10 +809,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 +819,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 = diff --git a/lib/foundation/fabro-types/src/settings/mod.rs b/lib/foundation/fabro-types/src/settings/mod.rs index 5bf91e4d9..e7c44be84 100644 --- a/lib/foundation/fabro-types/src/settings/mod.rs +++ b/lib/foundation/fabro-types/src/settings/mod.rs @@ -25,7 +25,7 @@ pub use cli::{ CliLoggingSettings, CliNamespace, CliOutputSettings, CliTargetSettings, CliUpdatesSettings, }; pub use duration::{Duration, ParseDurationError}; -pub use interp::{InterpString, ResolveCtx, ResolveError, ResolveErrorKind}; +pub use interp::{InterpString, Namespace, ResolveCtx, ResolveError, ResolveErrorKind}; pub use model_ref::{ AmbiguousModelRef, ModelRef, ModelRegistry, ParseModelRefError, ResolvedModelRef, };