diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index cb17e02f6..4ce881603 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -184,10 +184,19 @@ pub(crate) fn prepare_manifest_with_environment_defaults( pub(crate) fn validate_prepared_manifest( prepared: &PreparedManifest, catalog: Arc, +) -> Result { + validate_prepared_manifest_with_vars(prepared, catalog, HashMap::new()) +} + +pub(crate) fn validate_prepared_manifest_with_vars( + prepared: &PreparedManifest, + catalog: Arc, + vars: HashMap, ) -> Result { validate(ValidateInput { workflow: WorkflowInput::Bundled(prepared.workflow_input.clone()), settings: prepared.settings.clone(), + vars, cwd: prepared.cwd.clone(), custom_transforms: Vec::new(), catalog, @@ -199,10 +208,12 @@ pub(crate) fn create_run_input( configured_providers: Vec, provenance: RunProvenance, web_url: Option, + vars: HashMap, ) -> CreateRunInput { CreateRunInput { workflow: WorkflowInput::Bundled(prepared.workflow_input), settings: prepared.settings, + vars, cwd: prepared.cwd, workflow_slug: None, workflow_path: Some(prepared.target_path), diff --git a/lib/crates/fabro-server/src/server/handler/runs.rs b/lib/crates/fabro-server/src/server/handler/runs.rs index 621b0bf5c..311c76263 100644 --- a/lib/crates/fabro-server/src/server/handler/runs.rs +++ b/lib/crates/fabro-server/src/server/handler/runs.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::io::ErrorKind; use std::sync::Arc; @@ -645,7 +645,8 @@ pub(crate) async fn create_run_from_manifest( Ok(prepared) => prepared, Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; - if let Err(err) = substitute_run_variables(&state, &mut prepared.settings).await { + let vars = snapshot_run_variables(&state).await; + if let Err(err) = substitute_run_variables(&vars, &mut prepared.settings) { return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } @@ -691,6 +692,7 @@ pub(crate) async fn create_run_from_manifest( ready_provider_ids.clone(), provenance, web_url.clone(), + vars, ); create_input.run_id = Some(run_id); create_input.submitted_manifest_bytes = Some(submitted_manifest_bytes); @@ -901,11 +903,16 @@ async fn run_preflight( Ok(prepared) => prepared, Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; - if let Err(err) = substitute_run_variables(&state, &mut prepared.settings).await { + let vars = snapshot_run_variables(&state).await; + if let Err(err) = substitute_run_variables(&vars, &mut prepared.settings) { return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } - let mut validated = match run_manifest::validate_prepared_manifest(&prepared, state.catalog()) { + let mut validated = match run_manifest::validate_prepared_manifest_with_vars( + &prepared, + state.catalog(), + vars, + ) { Ok(validated) => validated, Err(WorkflowError::Parse(_)) => { return ApiError::bad_request("Validation failed").into_response(); @@ -938,11 +945,16 @@ async fn validate_run_manifest( Ok(prepared) => prepared, Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; - if let Err(err) = substitute_run_variables(&state, &mut prepared.settings).await { + let vars = snapshot_run_variables(&state).await; + if let Err(err) = substitute_run_variables(&vars, &mut prepared.settings) { return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } - let validated = match run_manifest::validate_prepared_manifest(&prepared, state.catalog()) { + let validated = match run_manifest::validate_prepared_manifest_with_vars( + &prepared, + state.catalog(), + vars, + ) { Ok(validated) => validated, Err(WorkflowError::Parse(_)) => { return ApiError::bad_request("Validation failed").into_response(); @@ -956,14 +968,17 @@ async fn validate_run_manifest( .into_response() } -async fn substitute_run_variables( - state: &AppState, +async fn snapshot_run_variables(state: &AppState) -> HashMap { + state.variables.read().await.value_map() +} + +fn substitute_run_variables( + variables: &HashMap, settings: &mut WorkflowSettings, ) -> Result<(), ResolveError> { - let variables = state.variables.read().await; settings .run - .substitute_variables(|name| variables.get_value(name).map(str::to_string)) + .substitute_variables(|name| variables.get(name).cloned()) } async fn get_run_status( diff --git a/lib/crates/fabro-server/tests/it/api/variables.rs b/lib/crates/fabro-server/tests/it/api/variables.rs index cc5b390c6..d2fcfb0fc 100644 --- a/lib/crates/fabro-server/tests/it/api/variables.rs +++ b/lib/crates/fabro-server/tests/it/api/variables.rs @@ -4,7 +4,7 @@ use tower::ServiceExt; use crate::helpers::{ MINIMAL_DOT, api, body_json, minimal_manifest_json, response_json, response_status, - test_app_state, + test_app_state, test_app_state_with_options, test_settings, }; fn json_request(method: Method, path: &str, body: &serde_json::Value) -> Request { @@ -246,3 +246,123 @@ id = "local" assert_eq!(body["run"]["goal"]["value"], "secret: token-from-variable"); } + +#[tokio::test] +async fn run_create_interpolates_variables_into_node_prompts() { + // End-to-end through the real run-create path: a server variable resolves + // inside a node `prompt` (a DOT graph attribute the settings substitution + // pass never touches), proving the variable store is snapshotted into the + // template render context at create time. + let app = fabro_server::test_support::build_test_router(test_app_state_with_options( + test_settings(), + 5, + )); + + let create_variable = app + .clone() + .oneshot(json_request( + Method::POST, + "/variables", + &serde_json::json!({ "name": "SERVICE", "value": "billing" }), + )) + .await + .expect("POST /variables should route"); + response_status(create_variable, StatusCode::OK, "POST /api/v1/variables").await; + + let dot = r#"digraph Test { + graph [goal="Ship it"] + start [shape=Mdiamond] + work [shape=box, prompt="Service: {{ vars.SERVICE }}"] + exit [shape=Msquare] + start -> work -> exit + }"#; + + let create_run = app + .clone() + .oneshot(json_request( + Method::POST, + "/runs", + &minimal_manifest_json(dot), + )) + .await + .expect("POST /runs should route"); + let create_status = create_run.status(); + let create_body = body_json(create_run.into_body()).await; + assert_eq!(create_status, StatusCode::CREATED, "{create_body}"); + let run_id = create_body["id"] + .as_str() + .expect("create run response should include id"); + + // The persisted `run.created` event carries the fully-rendered graph. + let events = app + .oneshot(empty_request( + Method::GET, + &format!("/runs/{run_id}/events"), + )) + .await + .expect("GET run events should route"); + let body = response_json( + events, + StatusCode::OK, + format!("GET /api/v1/runs/{run_id}/events"), + ) + .await; + let created = body["data"] + .as_array() + .expect("events response should include data") + .iter() + .find(|event| event["event"] == "run.created") + .expect("expected a run.created event"); + assert_eq!( + created["properties"]["graph"]["nodes"]["work"]["attrs"]["prompt"]["String"], + "Service: billing", + "node prompt should interpolate the run variable; event: {created}" + ); +} + +#[tokio::test] +async fn run_validate_resolves_variables_in_node_prompts() { + let app = fabro_server::test_support::build_test_router(test_app_state_with_options( + test_settings(), + 5, + )); + + let create_variable = app + .clone() + .oneshot(json_request( + Method::POST, + "/variables", + &serde_json::json!({ "name": "SERVICE", "value": "billing" }), + )) + .await + .expect("POST /variables should route"); + response_status(create_variable, StatusCode::OK, "POST /api/v1/variables").await; + + let dot = r#"digraph Test { + graph [goal="Ship it"] + start [shape=Mdiamond] + work [shape=box, prompt="Service: {{ vars.SERVICE }}"] + exit [shape=Msquare] + start -> work -> exit + }"#; + + let validate = app + .oneshot(json_request( + Method::POST, + "/validate", + &minimal_manifest_json(dot), + )) + .await + .expect("POST /validate should route"); + let body = response_json(validate, StatusCode::OK, "POST /api/v1/validate").await; + assert_eq!(body["ok"], true, "{body}"); + let diagnostics = body["workflow"]["diagnostics"] + .as_array() + .expect("validate response should include diagnostics"); + assert!( + !diagnostics + .iter() + .any(|diagnostic| diagnostic["rule"] == "template_undefined_variable"), + "vars.SERVICE should resolve during validation; diagnostics: {diagnostics:?}" + ); +} diff --git a/lib/crates/fabro-template/src/lib.rs b/lib/crates/fabro-template/src/lib.rs index 16e5a53c2..a61cd8134 100644 --- a/lib/crates/fabro-template/src/lib.rs +++ b/lib/crates/fabro-template/src/lib.rs @@ -48,13 +48,25 @@ pub struct TemplateErrorLocation { pub span_len: Option, } -#[derive(Debug, Default, Clone)] +#[derive(Debug, Clone)] pub struct TemplateContext { goal: Option, - inputs: HashMap, + inputs: Value, + vars: Value, env: Option, } +impl Default for TemplateContext { + fn default() -> Self { + Self { + goal: None, + inputs: Value::from_serialize(HashMap::::new()), + vars: Value::from_serialize(HashMap::::new()), + env: None, + } + } +} + impl TemplateContext { #[must_use] pub fn new() -> Self { @@ -69,7 +81,14 @@ impl TemplateContext { #[must_use] pub fn with_inputs(mut self, inputs: HashMap) -> Self { - self.inputs = inputs; + self.inputs = Value::from_serialize(inputs); + self + } + + /// Run-scoped `{{ vars.* }}` available to prompts and goals. + #[must_use] + pub fn with_vars(mut self, vars: HashMap) -> Self { + self.vars = Value::from_serialize(vars); self } @@ -107,9 +126,15 @@ impl TemplateContext { fn into_value(self) -> Value { let goal = self.goal.map(Value::from); - let inputs = Value::from_serialize(self.inputs); + let inputs = self.inputs; + let vars = self.vars; let env = self.env; - Value::from_object(RenderContext { goal, inputs, env }) + Value::from_object(RenderContext { + goal, + inputs, + vars, + env, + }) } } @@ -117,6 +142,7 @@ impl TemplateContext { struct RenderContext { goal: Option, inputs: Value, + vars: Value, env: Option, } @@ -125,6 +151,7 @@ impl Object for RenderContext { match key { "goal" => self.goal.clone(), "inputs" => Some(self.inputs.clone()), + "vars" => Some(self.vars.clone()), "env" => self.env.clone(), _ => None, } @@ -834,6 +861,30 @@ mod tests { )); } + #[test] + fn renders_vars_variable() { + let ctx = TemplateContext::new().with_vars(HashMap::from([( + "SERVICE".to_string(), + "billing".to_string(), + )])); + + let rendered = render("Service: {{ vars.SERVICE }}", &ctx).unwrap(); + + assert_eq!(rendered, "Service: billing"); + } + + #[test] + fn unknown_vars_member_is_strict_error() { + let ctx = TemplateContext::new().with_vars(HashMap::from([( + "SERVICE".to_string(), + "billing".to_string(), + )])); + + let err = render("{{ vars.MISSING }}", &ctx).unwrap_err(); + + assert!(err.to_string().to_lowercase().contains("undefined")); + } + #[test] fn renders_typed_input_values() { let ctx = TemplateContext::new().with_inputs(HashMap::from([ diff --git a/lib/crates/fabro-variable/src/lib.rs b/lib/crates/fabro-variable/src/lib.rs index 1b15f3249..86d0994da 100644 --- a/lib/crates/fabro-variable/src/lib.rs +++ b/lib/crates/fabro-variable/src/lib.rs @@ -158,6 +158,17 @@ impl VariableStore { data } + /// Snapshot every variable as a `name -> value` map, dropping descriptions + /// and timestamps. Used to seed the template render context (`{{ vars.* + /// }}`) at run creation. + #[must_use] + pub fn value_map(&self) -> HashMap { + self.entries + .iter() + .map(|(name, entry)| (name.clone(), entry.value.clone())) + .collect() + } + pub fn remove(&mut self, name: &str) -> Result<(), Error> { Self::validate_name(name)?; if self.entries.remove(name).is_none() { diff --git a/lib/crates/fabro-workflow/src/handler/manager_loop.rs b/lib/crates/fabro-workflow/src/handler/manager_loop.rs index d7e1dff7b..d16291b68 100644 --- a/lib/crates/fabro-workflow/src/handler/manager_loop.rs +++ b/lib/crates/fabro-workflow/src/handler/manager_loop.rs @@ -72,6 +72,7 @@ fn parse_child_graph(node: &Node, services: &EngineServices) -> Result Result, pub cwd: PathBuf, pub workflow_slug: Option, pub workflow_path: Option, @@ -63,6 +68,7 @@ pub struct CreatedRun { struct PersistCreateOptions { settings: WorkflowSettings, + vars: HashMap, run_id: Option, run_dir: Option, workflow_slug: Option, @@ -97,6 +103,7 @@ pub async fn create( let CreateRunInput { workflow: _, settings: _, + vars, cwd: _, workflow_slug, workflow_path, @@ -141,6 +148,7 @@ pub async fn create( &raw_source, PersistCreateOptions { settings, + vars, run_id: Some(run_id), run_dir: Some(persisted_run_dir), workflow_slug: workflow_slug.or(resolved_workflow_slug), @@ -292,6 +300,7 @@ fn create_from_source( file_resolver, Vec::new(), Some(&options.settings), + options.vars.clone(), goal_override, RenderMode::Structural, &options.catalog, @@ -314,6 +323,7 @@ pub(super) fn preprocess_and_validate( file_resolver: Option>, custom_transforms: Vec>, settings: Option<&WorkflowSettings>, + vars: HashMap, goal_override: Option<&str>, render_mode: RenderMode, catalog: &Arc, @@ -321,11 +331,12 @@ pub(super) fn preprocess_and_validate( let inputs = run_inputs(settings); let mut parsed = pipeline::parse(dot_source)?; apply_goal_override(&mut parsed.graph, goal_override); + let template_context = TemplateContext::new().with_inputs(inputs).with_vars(vars); let transformed = pipeline::transform(parsed, &TransformOptions { current_dir, file_resolver, - inputs, + template_context, source_name, render_mode, custom_transforms, @@ -355,6 +366,7 @@ fn persist_validated( ) -> Result { let PersistCreateOptions { settings, + vars: _, run_id, run_dir, workflow_slug, @@ -473,6 +485,7 @@ mod tests { base_dir: None, }, settings, + vars: HashMap::new(), cwd: PathBuf::from("."), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -480,6 +493,25 @@ mod tests { .unwrap() } + /// Drive the create-time pipeline with an explicit variable snapshot, the + /// way the server does (`Structural` render mode, undefined vars promoted + /// to errors at run-create). + fn validate_dot_with_vars(dot_source: &str, vars: HashMap) -> Validated { + preprocess_and_validate( + dot_source, + Some("workflow.fabro".to_string()), + Some(PathBuf::from(".")), + None, + Vec::new(), + Some(&WorkflowSettings::default()), + vars, + None, + RenderMode::Structural, + &test_catalog(), + ) + .unwrap() + } + const MINIMAL_DOT: &str = r#"digraph Test { graph [goal="Build feature"] start [shape=Mdiamond] @@ -552,6 +584,57 @@ mod tests { ); } + #[test] + fn vars_resolve_in_node_prompt_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", prompt="Service: {{ vars.SERVICE }}"] + start -> work -> exit + }"#; + let vars = HashMap::from([("SERVICE".to_string(), "billing".to_string())]); + let validated = validate_dot_with_vars(dot, vars); + validated.raise_on_errors().unwrap(); + assert!( + !validated + .diagnostics() + .iter() + .any(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE), + "vars.SERVICE should resolve through the create pipeline; got: {:?}", + validated.diagnostics() + ); + } + + #[test] + fn unknown_var_in_prompt_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", prompt="Service: {{ vars.MISSING }}"] + start -> work -> exit + }"#; + let mut validated = validate_dot_with_vars(dot, HashMap::new()); + + // `fabro validate` surfaces a warning, not a hard failure. + 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("vars.MISSING"), + "message: {}", + diagnostic.message + ); + + // Run-create promotes the same diagnostic to a hard error. + 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 { @@ -590,6 +673,7 @@ mod tests { None, Vec::new(), Some(&WorkflowSettings::default()), + HashMap::new(), None, RenderMode::Strict, &test_catalog(), @@ -626,6 +710,7 @@ mod tests { ))), Vec::new(), Some(&WorkflowSettings::default()), + HashMap::new(), None, RenderMode::Strict, &test_catalog(), @@ -684,6 +769,7 @@ mod tests { ..RunLayer::default() } }), + vars: HashMap::new(), cwd: PathBuf::from("."), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -732,6 +818,7 @@ mod tests { base_dir: Some(dir.path().to_path_buf()), }, settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -750,6 +837,7 @@ mod tests { base_dir: Some(dir.path().to_path_buf()), }, settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -773,6 +861,7 @@ mod tests { base_dir: Some(dir.path().to_path_buf()), }, settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -791,6 +880,7 @@ mod tests { base_dir: Some(dir.path().to_path_buf()), }, settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -882,6 +972,7 @@ mod tests { base_dir: None, }, settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: PathBuf::from("."), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -926,6 +1017,7 @@ mod tests { base_dir: None, }, settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: PathBuf::from("."), custom_transforms: vec![Box::new(TagTransform)], catalog: test_catalog(), @@ -960,6 +1052,7 @@ mod tests { let validated = validate(ValidateInput { workflow: WorkflowInput::Path(dot_path), settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -1001,6 +1094,7 @@ mod tests { let validated = validate(ValidateInput { workflow: WorkflowInput::Path(dot_path), settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -1050,6 +1144,7 @@ mod tests { ]), }), settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: PathBuf::from("."), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -1100,6 +1195,7 @@ mod tests { ]), }), settings: WorkflowSettings::default(), + vars: HashMap::new(), cwd: PathBuf::from("."), custom_transforms: Vec::new(), catalog: test_catalog(), @@ -1134,6 +1230,7 @@ mod tests { base_dir: None, }, settings: test_default_settings(), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), workflow_slug: None, workflow_path: None, @@ -1200,6 +1297,7 @@ mod tests { ..RunLayer::default() } }), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), workflow_slug: Some("slug".to_string()), workflow_path: None, @@ -1314,6 +1412,7 @@ mod tests { ..RunLayer::default() } }), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), workflow_slug: None, workflow_path: None, @@ -1354,6 +1453,7 @@ mod tests { base_dir: None, }, settings: dry_run_only_settings(), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), workflow_slug: None, workflow_path: None, @@ -1433,6 +1533,7 @@ mod tests { base_dir: None, }, settings: dry_run_with_storage(&storage_dir), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), workflow_slug: Some("slug".to_string()), workflow_path: None, @@ -1486,6 +1587,7 @@ mod tests { base_dir: None, }, settings: dry_run_with_storage(&storage_dir), + vars: HashMap::new(), cwd: dir.path().to_path_buf(), workflow_slug: Some("slug".to_string()), workflow_path: None, diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index 404e843ea..a006e3ec2 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -1418,6 +1418,7 @@ reasoning = false }), ..RunLayer::default() }), + vars: std::collections::HashMap::new(), cwd: storage_root .parent() .unwrap_or_else(|| Path::new(".")) @@ -1843,6 +1844,7 @@ reasoning = false }), ..RunLayer::default() }), + vars: std::collections::HashMap::new(), cwd: temp.path().to_path_buf(), workflow_slug: Some("bundle-child".to_string()), workflow_path: Some(ManifestPath::from_wire("workflow.fabro").unwrap()), diff --git a/lib/crates/fabro-workflow/src/operations/validate.rs b/lib/crates/fabro-workflow/src/operations/validate.rs index 1a4769fc9..83375f761 100644 --- a/lib/crates/fabro-workflow/src/operations/validate.rs +++ b/lib/crates/fabro-workflow/src/operations/validate.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; @@ -14,6 +15,9 @@ use crate::transforms::Transform; pub struct ValidateInput { pub workflow: WorkflowInput, pub settings: WorkflowSettings, + /// Run-scoped variables (`{{ vars.* }}`) available to prompts and goals. + /// Empty for offline/CLI validation. + pub vars: HashMap, pub cwd: PathBuf, pub custom_transforms: Vec>, pub catalog: Arc, @@ -41,6 +45,7 @@ pub fn validate(input: ValidateInput) -> Result { resolved.file_resolver, input.custom_transforms, Some(&resolved.settings), + input.vars, resolved.goal_override.as_deref(), RenderMode::Structural, &input.catalog, diff --git a/lib/crates/fabro-workflow/src/pipeline/transform.rs b/lib/crates/fabro-workflow/src/pipeline/transform.rs index 85a90e91a..4f14ceeb7 100644 --- a/lib/crates/fabro-workflow/src/pipeline/transform.rs +++ b/lib/crates/fabro-workflow/src/pipeline/transform.rs @@ -22,7 +22,7 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result Result Result work -> exit + }"#; + let parsed = parse(dot).unwrap(); + let transformed = transform(parsed, &TransformOptions { + template_context: fabro_template::TemplateContext::new().with_vars(HashMap::from([( + "SERVICE".to_string(), + "billing".to_string(), + )])), + ..transform_options() + }) + .unwrap(); + let prompt = transformed.graph.nodes["work"] + .attrs + .get("prompt") + .and_then(AttrValue::as_str) + .unwrap(); + assert_eq!(prompt, "Service: billing"); + } + + #[test] + fn transform_interpolates_vars_in_graph_goal_and_through_prompt() { + // The goal interpolates `{{ vars.* }}`, and a prompt that embeds the + // goal sees the vars-resolved text. + let dot = r#"digraph Test { + graph [goal="Ship {{ vars.SERVICE }}"] + start [shape=Mdiamond] + work [prompt="Goal: {{ goal }}"] + exit [shape=Msquare] + start -> work -> exit + }"#; + let parsed = parse(dot).unwrap(); + let transformed = transform(parsed, &TransformOptions { + template_context: fabro_template::TemplateContext::new().with_vars(HashMap::from([( + "SERVICE".to_string(), + "billing".to_string(), + )])), + ..transform_options() + }) + .unwrap(); + assert_eq!( + transformed + .graph + .attrs + .get("goal") + .and_then(AttrValue::as_str), + Some("Ship billing") + ); + assert_eq!( + transformed.graph.nodes["work"] + .attrs + .get("prompt") + .and_then(AttrValue::as_str), + Some("Goal: Ship billing") + ); + } + + #[test] + fn transform_with_empty_vars_warns_on_unknown_var() { + // Offline / no variable store: `{{ vars.* }}` is undefined, surfacing a + // structural-mode warning (promoted to a hard error at run-create). + let dot = r#"digraph Test { + graph [goal="Fix bugs"] + start [shape=Mdiamond] + work [prompt="Service: {{ vars.MISSING }}"] + exit [shape=Msquare] + start -> work -> exit + }"#; + let parsed = parse(dot).unwrap(); + let transformed = transform(parsed, &TransformOptions { + template_context: fabro_template::TemplateContext::new(), + render_mode: crate::operations::RenderMode::Structural, + ..transform_options() + }) + .unwrap(); + let diag = transformed + .diagnostics + .iter() + .find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) + .expect("expected a template_undefined_variable diagnostic for vars.MISSING"); + assert!( + diag.message.contains("vars.MISSING"), + "message: {}", + diag.message + ); + } + #[test] fn transform_reports_goal_self_reference_once_across_passes() { // FileInlining renders the goal for prompt context, but TemplateTransform @@ -257,7 +352,7 @@ mod tests { let transformed = transform(parsed, &TransformOptions { current_dir: Some(dir.path().to_path_buf()), file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))), - inputs: HashMap::new(), + template_context: fabro_template::TemplateContext::new(), source_name: None, render_mode: crate::operations::RenderMode::Structural, custom_transforms: vec![], diff --git a/lib/crates/fabro-workflow/src/pipeline/types.rs b/lib/crates/fabro-workflow/src/pipeline/types.rs index 8e7a25cac..8449453a8 100644 --- a/lib/crates/fabro-workflow/src/pipeline/types.rs +++ b/lib/crates/fabro-workflow/src/pipeline/types.rs @@ -7,6 +7,7 @@ use fabro_interview::Interviewer; use fabro_mcp::config::McpServerSettings; use fabro_model::{Catalog, FallbackTarget, ProviderId}; use fabro_sandbox::SandboxSpec; +use fabro_template::TemplateContext; use fabro_types::settings::run::{PullRequestSettings, RunModelControls}; use fabro_types::{ManifestPath, RunId}; use fabro_validate::{Diagnostic, Severity}; @@ -325,7 +326,7 @@ pub struct Finalized { pub struct TransformOptions { pub current_dir: Option, pub file_resolver: Option>, - pub inputs: HashMap, + pub template_context: TemplateContext, pub source_name: Option, pub render_mode: RenderMode, pub custom_transforms: Vec>, diff --git a/lib/crates/fabro-workflow/src/pipeline/validate.rs b/lib/crates/fabro-workflow/src/pipeline/validate.rs index 166a48078..14999afae 100644 --- a/lib/crates/fabro-workflow/src/pipeline/validate.rs +++ b/lib/crates/fabro-workflow/src/pipeline/validate.rs @@ -44,7 +44,7 @@ mod tests { let transformed = transform::transform(parsed, &TransformOptions { current_dir: None, file_resolver: None, - inputs: std::collections::HashMap::new(), + template_context: fabro_template::TemplateContext::new(), source_name: None, render_mode: crate::operations::RenderMode::Strict, custom_transforms: vec![], diff --git a/lib/crates/fabro-workflow/src/transforms/file_inlining.rs b/lib/crates/fabro-workflow/src/transforms/file_inlining.rs index cc45fc77a..1fa18d75a 100644 --- a/lib/crates/fabro-workflow/src/transforms/file_inlining.rs +++ b/lib/crates/fabro-workflow/src/transforms/file_inlining.rs @@ -126,7 +126,7 @@ fn manifest_path_is_within_root(path: &ManifestPath, root: &ManifestPath) -> boo pub struct FileInliningTransform { current_dir: PathBuf, resolver: Arc, - inputs: HashMap, + context: TemplateContext, source_name: Option, source_text: Option, goal_override: Option, @@ -139,7 +139,7 @@ impl FileInliningTransform { Self { current_dir, resolver, - inputs: HashMap::new(), + context: TemplateContext::new(), source_name: None, source_text: None, goal_override: None, @@ -150,12 +150,12 @@ impl FileInliningTransform { #[must_use] pub fn with_template_options( mut self, - inputs: HashMap, + context: TemplateContext, source_name: Option, source_text: Option, render_mode: RenderMode, ) -> Self { - self.inputs = inputs; + self.context = context; self.source_name = source_name; self.source_text = source_text; self.render_mode = render_mode; @@ -168,6 +168,13 @@ impl FileInliningTransform { self } + /// Run-scoped `{{ vars.* }}` available to prompts and the goal. + #[must_use] + pub fn with_vars(mut self, vars: HashMap) -> Self { + self.context = self.context.with_vars(vars); + self + } + pub(crate) fn apply_with_diagnostics( &self, graph: Graph, @@ -183,9 +190,7 @@ impl FileInliningTransform { // pass owns canonical goal validation diagnostics. None => graph.goal().to_string(), }; - let ctx = TemplateContext::new() - .with_goal(resolved_goal) - .with_inputs(self.inputs.clone()); + let ctx = self.context.clone().with_goal(resolved_goal); for (node_id, node) in &mut graph.nodes { // `prompt` is an importable template: MiniJinja-render the value, @@ -248,7 +253,7 @@ impl FileInliningTransform { let Some(AttrValue::String(goal)) = graph.attrs.get("goal") else { return Ok(()); }; - let ctx = TemplateContext::for_input_scan(self.inputs.clone()); + let ctx = self.context.clone().with_goal("{{ goal }}"); let target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal") .with_source_origin(self.source_text.as_deref(), goal) .with_template_store(template_render_store( diff --git a/lib/crates/fabro-workflow/src/transforms/import.rs b/lib/crates/fabro-workflow/src/transforms/import.rs index 5127bedf8..546468cf4 100644 --- a/lib/crates/fabro-workflow/src/transforms/import.rs +++ b/lib/crates/fabro-workflow/src/transforms/import.rs @@ -19,7 +19,7 @@ use crate::transforms::variable_expansion::{ pub struct ImportTransform { current_dir: PathBuf, resolver: Arc, - inputs: HashMap, + context: TemplateContext, source_name: Option, source_text: Option, render_mode: RenderMode, @@ -56,18 +56,26 @@ impl ImportTransform { pub fn new( current_dir: PathBuf, resolver: Arc, - inputs: HashMap, + context: TemplateContext, ) -> Self { Self { current_dir, resolver, - inputs, + context, source_name: None, source_text: None, render_mode: RenderMode::Structural, } } + /// Run-scoped `{{ vars.* }}`, propagated into imported subgraphs so their + /// prompts and goals interpolate variables too. + #[must_use] + pub fn with_vars(mut self, vars: HashMap) -> Self { + self.context = self.context.with_vars(vars); + self + } + #[must_use] pub fn with_template_options( mut self, @@ -205,7 +213,7 @@ impl ImportTransform { let (inlined_graph, file_diagnostics) = FileInliningTransform::new(import_base_dir.clone(), Arc::clone(&self.resolver)) .with_template_options( - self.inputs.clone(), + self.context.clone(), Some(source_name.clone()), Some(source_text.clone()), self.render_mode, @@ -221,7 +229,7 @@ impl ImportTransform { AttrValue::String(parent_goal.to_string()), ); let (templated_graph, template_diagnostics) = TemplateTransform { - inputs: self.inputs.clone(), + context: self.context.clone(), source_name: Some(source_name), source_text: Some(source_text), render_mode: self.render_mode, @@ -672,7 +680,7 @@ impl ImportTransform { let imports = Self::collect_import_nodes(&graph); let mut import_stack = Vec::new(); let mut diagnostics = Vec::new(); - let path_ctx = TemplateContext::for_input_scan(self.inputs.clone()); + let path_ctx = self.context.clone().with_goal("{{ goal }}"); let mut ignored_goal_diagnostics = Vec::new(); let goal_target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal") .with_source_origin(self.source_text.as_deref(), graph.goal()) @@ -754,7 +762,7 @@ mod tests { Arc::new(FilesystemFileResolver::new( fallback_dir.map(Path::to_path_buf), )), - HashMap::new(), + TemplateContext::new(), ) .apply(graph) .unwrap() @@ -775,7 +783,7 @@ mod tests { let graph = ImportTransform::new( dir.path().to_path_buf(), Arc::new(FilesystemFileResolver::new(None)), - HashMap::new(), + TemplateContext::new(), ) .with_template_options( Some("workflow.fabro".to_string()), @@ -821,7 +829,7 @@ mod tests { let err = ImportTransform::new( dir.path().to_path_buf(), Arc::new(FilesystemFileResolver::new(None)), - HashMap::new(), + TemplateContext::new(), ) .with_template_options( Some("workflow.fabro".to_string()), @@ -847,6 +855,48 @@ mod tests { }"# } + #[test] + fn import_parent_goal_resolves_vars_before_imported_prompt_uses_goal() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("validate.fabro"), + r#"digraph validate { + start [shape=Mdiamond] + lint [prompt="Goal: {{ goal }}"] + exit [shape=Msquare] + start -> lint -> exit + }"#, + ); + let graph = parse_graph( + r#"digraph Deploy { + graph [goal="Ship {{ vars.SERVICE }}"] + start [shape=Mdiamond] + validate [import="./validate.fabro"] + exit [shape=Msquare] + start -> validate -> exit + }"#, + ); + + let graph = ImportTransform::new( + dir.path().to_path_buf(), + Arc::new(FilesystemFileResolver::new(None)), + TemplateContext::new().with_vars(HashMap::from([( + "SERVICE".to_string(), + "billing".to_string(), + )])), + ) + .apply(graph) + .unwrap(); + + assert_eq!( + graph.nodes["validate.lint"] + .attrs + .get("prompt") + .and_then(AttrValue::as_str), + Some("Goal: Ship billing") + ); + } + #[test] fn basic_import_replaces_placeholder_and_rewires_edges() { let dir = tempfile::tempdir().unwrap(); @@ -934,7 +984,7 @@ mod tests { let (graph, diagnostics) = ImportTransform::new( dir.path().to_path_buf(), Arc::new(FilesystemFileResolver::new(None)), - HashMap::new(), + TemplateContext::new(), ) .apply_with_diagnostics(graph) .unwrap(); @@ -1635,7 +1685,7 @@ mod tests { let graph = ImportTransform::new( dir.path().to_path_buf(), Arc::new(FilesystemFileResolver::new(None)), - HashMap::new(), + TemplateContext::new(), ) .apply(graph) .unwrap(); diff --git a/lib/crates/fabro-workflow/src/transforms/variable_expansion.rs b/lib/crates/fabro-workflow/src/transforms/variable_expansion.rs index 9a6fda263..b822a930b 100644 --- a/lib/crates/fabro-workflow/src/transforms/variable_expansion.rs +++ b/lib/crates/fabro-workflow/src/transforms/variable_expansion.rs @@ -289,9 +289,10 @@ fn goal_self_reference_diagnostic( } } -/// Expands `{{ goal }}` / `{{ inputs.* }}` across all string attributes. +/// Expands `{{ goal }}` / `{{ inputs.* }}` / `{{ vars.* }}` across all string +/// attributes. pub struct TemplateTransform { - pub inputs: HashMap, + pub context: TemplateContext, pub source_name: Option, pub source_text: Option, pub render_mode: RenderMode, @@ -301,13 +302,19 @@ impl TemplateTransform { #[must_use] pub fn new(inputs: HashMap) -> Self { Self { - inputs, + context: TemplateContext::new().with_inputs(inputs), source_name: None, source_text: None, render_mode: RenderMode::Structural, } } + #[must_use] + pub fn with_vars(mut self, vars: HashMap) -> Self { + self.context = self.context.with_vars(vars); + self + } + pub(crate) fn resolved_goal( &self, graph: &Graph, @@ -332,7 +339,7 @@ impl TemplateTransform { )); return Ok(goal.to_string()); } - let ctx = TemplateContext::new().with_inputs(self.inputs.clone()); + let ctx = self.context.clone(); render_template_for_target(goal, &ctx, self.render_mode, &target, diagnostics) } @@ -341,7 +348,7 @@ impl TemplateTransform { goal: &str, target: &TemplateRenderTarget, ) -> Option { - let ctx = TemplateContext::new().with_inputs(self.inputs.clone()); + let ctx = self.context.clone(); match render_template_with_mode(goal, &ctx, TemplateRenderMode::Strict, target) { Err(err @ TemplateError::UndefinedVariable { .. }) if err.expression() == Some("goal") => @@ -405,9 +412,7 @@ impl TemplateTransform { graph .attrs .insert("goal".to_string(), AttrValue::String(resolved_goal.clone())); - let ctx = TemplateContext::new() - .with_goal(resolved_goal) - .with_inputs(self.inputs.clone()); + let ctx = self.context.clone().with_goal(resolved_goal); Self::render_attrs( &mut graph.attrs, @@ -613,7 +618,7 @@ mod tests { graph.nodes.insert("plan".to_string(), node); let transform = TemplateTransform { - inputs: HashMap::new(), + context: TemplateContext::new(), source_name: Some("workflow.fabro".to_string()), source_text: Some(source.to_string()), render_mode: RenderMode::Structural, @@ -792,7 +797,7 @@ mod tests { graph.nodes.insert("plan".to_string(), node); let transform = TemplateTransform { - inputs: HashMap::new(), + context: TemplateContext::new(), source_name: Some("workflow.fabro".to_string()), source_text: None, render_mode: RenderMode::Structural, diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index 4ddf6b75c..9524a1761 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -4684,7 +4684,7 @@ async fn import_e2e_through_engine() { file_resolver: Some(std::sync::Arc::new( fabro_workflow::file_resolver::FilesystemFileResolver::new(None), )), - inputs: std::collections::HashMap::new(), + template_context: fabro_template::TemplateContext::new(), source_name: None, render_mode: fabro_workflow::operations::RenderMode::Strict, custom_transforms: vec![],