From 41fb7e7e1fce4b20c745d74bc458563fec133f38 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Wed, 24 Jun 2026 08:43:18 -0400 Subject: [PATCH] inputs is template-only: reject {{ inputs.* }} in InterpString with a clear error (D12) (#513) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the `inputs`-template-only half of **D12**. Independent off `main` — touches only `fabro-types` interp; no overlap with #511 or #512. ## What changes for users `{{ inputs.* }}` in an `InterpString` field (command, script, header, env, URL — MCP transports, prepare steps, hooks, server settings) now fails with a **clear, actionable message**: > `{{ inputs.X }}` is only available in prompts and goals, not in command, script, header, env, or URL fields It *already* failed there (no resolve context ever provided an inputs lookup, so it errored as a generic "unavailable"); this makes the rejection explicit and points the user at where `inputs` belongs. ## How - **`ResolveCtx` drops its unused `inputs` lookup** (`with_inputs` had zero production callers). The type now structurally cannot resolve `inputs` in an `InterpString` field; `lookup_for(Inputs)` returns `None`. - The `Unavailable` error message is `inputs`-specific and points to prompts/goals. - **`substitute_with` still preserves `inputs` tokens** (unknown-namespace passthrough), so `run.goal` — an `InterpString` that feeds a template — keeps forwarding `{{ inputs.* }}` to its prompt/goal render. This is the load-bearing behavior that makes "inputs works in goals" coexist with "inputs rejected in InterpString fields", and it's covered by an existing test (`substitute_variables_preserves_late_bound_tokens`). - Module docs updated: three resolvable namespaces in `InterpString` (`env`/`vars`/`secrets`); `inputs` is template-only. ## Note on timing The rejection fires at **resolve time** (use-time / run boundary), not at `fabro validate`. That matches how the other late-bound namespaces behave and keeps this PR small; a validate-time fail-fast would need to distinguish goal (forwards inputs) from pure-`InterpString` fields and is a larger, separate change if we want it. ## Tests `resolve_with_rejects_inputs_as_template_only` (rejection + friendly message); `substitute_variables_preserves_late_bound_tokens` confirms goal forwarding is unaffected. Verified: `cargo build --workspace`, nightly `clippy --workspace --all-targets -D warnings`, `fmt`, `cargo nextest run --workspace` (**6796 passed**). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- lib/crates/fabro-types/src/settings/interp.rs | 75 +++++++++++-------- 1 file changed, 45 insertions(+), 30 deletions(-) diff --git a/lib/crates/fabro-types/src/settings/interp.rs b/lib/crates/fabro-types/src/settings/interp.rs index e5b00dc06..8395b7cc9 100644 --- a/lib/crates/fabro-types/src/settings/interp.rs +++ b/lib/crates/fabro-types/src/settings/interp.rs @@ -1,17 +1,19 @@ //! Interpolation for config strings. //! //! An [`InterpString`] field may contain narrow `{{ .NAME }}` -//! tokens — no template logic — drawn from the four [`Namespace`]s: `env`, -//! `vars`, `secrets`, and `inputs`. Which namespaces actually resolve is -//! scope-determined by the caller through [`ResolveCtx`]: server-scope -//! settings provide `env` (and eventually `secrets`), run-scope settings -//! additionally provide `vars` and `inputs`. A token whose namespace is not -//! available in the resolution context fails loudly instead of passing -//! through as literal text. +//! tokens — no template logic. Three [`Namespace`]s resolve here: `env`, +//! `vars`, and `secrets`. `inputs` is **template-only** (D12): 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. //! -//! Resolution timing is split: `vars`/`inputs` substitute early (server-side, -//! at run creation) via [`InterpString::substitute_with`], while -//! `env`/`secrets` resolve late, at consumption time in the process that owns +//! Resolution timing is split: `vars` substitutes early (server-side, at run +//! creation) via [`InterpString::substitute_with`], while `env`/`secrets` +//! resolve late, at consumption time in the process that owns //! the value, via [`InterpString::resolve_with`]. Provenance tracking lets //! outward-facing renderers redact env- and secret-sourced values uniformly. @@ -114,7 +116,6 @@ pub struct ResolveCtx<'a> { env: Option>, vars: Option>, secrets: Option>, - inputs: Option>, } type LookupFn<'a> = Box Option + 'a>; @@ -143,18 +144,16 @@ impl<'a> ResolveCtx<'a> { self } - #[must_use] - pub fn with_inputs(mut self, lookup: impl FnMut(&str) -> Option + 'a) -> Self { - self.inputs = Some(Box::new(lookup)); - 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(), - Namespace::Inputs => self.inputs.as_mut(), + // `inputs` is template-only (D12): 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, } } } @@ -506,12 +505,22 @@ impl fmt::Display for ResolveError { "{noun} {:?} referenced by {{{{ {namespace}.{} }}}} is not set", self.name, self.name ), - ResolveErrorKind::Unavailable => write!( - f, - "{noun} {:?} referenced by {{{{ {namespace}.{} }}}} is not supported in this \ - interpolation context", - self.name, self.name - ), + ResolveErrorKind::Unavailable => match namespace { + // `inputs` is template-only (D12): it never resolves in an + // `InterpString` field. Point the user at where it works. + Namespace::Inputs => write!( + f, + "{{{{ inputs.{} }}}} is only available in prompts and goals, not in other \ + config fields", + self.name + ), + _ => write!( + f, + "{noun} {:?} referenced by {{{{ {namespace}.{} }}}} is not supported in \ + this interpolation context", + self.name, self.name + ), + }, } } } @@ -806,15 +815,21 @@ mod tests { } #[test] - fn resolve_with_inputs_substitutes_without_provenance() { + fn resolve_with_rejects_inputs_as_template_only() { + // D12: `inputs` is template-only. An `{{ inputs.* }}` token never + // resolves in an `InterpString` field — it fails loudly, pointing the + // user at prompts and goals. let s = InterpString::parse("run-{{ inputs.ticket-id }}"); - let resolved = s - .resolve_with(&mut ResolveCtx::new().with_inputs(lookup_from(&[("ticket-id", "1234")]))) - .unwrap(); + let err = s.resolve_with(&mut ResolveCtx::new()).unwrap_err(); - assert_eq!(resolved.value, "run-1234"); - assert_eq!(resolved.provenance, Provenance::Literal); + assert_eq!(err.namespace, Namespace::Inputs); + assert_eq!(err.kind, ResolveErrorKind::Unavailable); + assert!( + err.to_string() + .contains("only available in prompts and goals"), + "unexpected message: {err}" + ); } #[test]