diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index e561d7480..5f22beab7 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -222,17 +222,11 @@ fn value_at_path<'a>(document: &'a toml::Value, path: &[&str]) -> Option<&'a tom /// Pull the resolved CLI target configuration out of `[cli.target]`. /// Returns either an http(s) URL or a unix socket path. -#[expect( - clippy::disallowed_methods, - reason = "known leak: cli.target.* is a url/connection field that should resolve {{ env.* }} \ - tokens but consumes them raw today; strict resolution scheduled in the \ - interpolation unification (Phase 2 keep-rows)" -)] fn cli_target_from_settings(settings: &CliNamespace) -> Option { let target = settings.target.as_ref()?; match target { - CliTargetSettings::Http { url } => Some(url.as_source()), - CliTargetSettings::Unix { path } => Some(path.as_source()), + CliTargetSettings::Http { url } => Some(url.clone()), + CliTargetSettings::Unix { path } => Some(path.clone()), } } diff --git a/lib/crates/fabro-config/src/layers/cli.rs b/lib/crates/fabro-config/src/layers/cli.rs index 472d58412..b044d2d46 100644 --- a/lib/crates/fabro-config/src/layers/cli.rs +++ b/lib/crates/fabro-config/src/layers/cli.rs @@ -1,6 +1,5 @@ //! Sparse `[cli]` settings layer definitions. -use fabro_types::settings::InterpString; use fabro_types::settings::cli::{CliAuthStrategy, OutputFormat, OutputVerbosity}; use fabro_types::settings::run::AgentPermissions; use serde::{Deserialize, Serialize}; @@ -32,11 +31,11 @@ pub struct CliLayer { pub enum CliTargetLayer { Http { #[serde(default)] - url: Option, + url: Option, }, Unix { #[serde(default)] - path: Option, + path: Option, }, } diff --git a/lib/crates/fabro-config/src/layers/run.rs b/lib/crates/fabro-config/src/layers/run.rs index 505385a3b..af35ce44b 100644 --- a/lib/crates/fabro-config/src/layers/run.rs +++ b/lib/crates/fabro-config/src/layers/run.rs @@ -19,7 +19,7 @@ pub struct RunLayer { #[serde(default, skip_serializing_if = "Option::is_none")] pub goal: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub working_dir: Option, + pub working_dir: Option, /// Flat string-to-string map. Replaces wholesale across layers. #[serde(default, skip_serializing_if = "ReplaceMap::is_empty")] pub metadata: ReplaceMap, diff --git a/lib/crates/fabro-config/src/project.rs b/lib/crates/fabro-config/src/project.rs index bf48db569..4fc58e61e 100644 --- a/lib/crates/fabro-config/src/project.rs +++ b/lib/crates/fabro-config/src/project.rs @@ -12,7 +12,7 @@ use std::fmt::Write; use std::path::{Path, PathBuf}; -use fabro_types::settings::{InterpString, RunNamespace}; +use fabro_types::settings::RunNamespace; use serde::Serialize; use crate::{Error, Result, WorkflowSettingsBuilder, run}; @@ -144,14 +144,8 @@ fn sibling_workflow_toml_for(graph: &Path) -> Option { (toml_graph == graph).then_some(candidate) } -#[expect( - clippy::disallowed_methods, - reason = "known leak: run.working_dir is a path kept as InterpString (not demoted); it should \ - resolve {{ env.* }}/{{ vars.* }} tokens but consumes them raw today; strict \ - resolution scheduled in the interpolation unification (Phase 2 keep-rows)" -)] pub fn resolve_working_directory_from_run(run: &RunNamespace, caller_cwd: &Path) -> PathBuf { - let Some(work_dir) = run.working_dir.as_ref().map(InterpString::as_source) else { + let Some(work_dir) = run.working_dir.as_deref() else { return caller_cwd.to_path_buf(); }; let path = PathBuf::from(work_dir); @@ -557,7 +551,7 @@ directory = "../custom" let cwd = Path::new("/tmp/workspace"); let resolved = resolve_working_directory_from_run( &RunNamespace { - working_dir: Some(InterpString::parse("repo")), + working_dir: Some("repo".to_string()), ..RunNamespace::default() }, cwd, diff --git a/lib/crates/fabro-config/src/resolve/cli.rs b/lib/crates/fabro-config/src/resolve/cli.rs index a9400b251..12ebebb9d 100644 --- a/lib/crates/fabro-config/src/resolve/cli.rs +++ b/lib/crates/fabro-config/src/resolve/cli.rs @@ -3,7 +3,7 @@ use fabro_types::settings::cli::{ CliLoggingSettings, CliNamespace, CliOutputSettings, CliTargetSettings, CliUpdatesSettings, }; -use super::{ResolveError, require_interp}; +use super::{ResolveError, require_string}; use crate::{CliExecLayer, CliLayer, CliTargetLayer}; pub fn resolve_cli(layer: &CliLayer, errors: &mut Vec) -> CliNamespace { @@ -46,12 +46,18 @@ fn resolve_target( errors: &mut Vec, ) -> Option { match target { - Some(CliTargetLayer::Http { url }) => Some(CliTargetSettings::Http { - url: require_interp(url.as_ref(), "cli.target.url", errors), - }), - Some(CliTargetLayer::Unix { path }) => Some(CliTargetSettings::Unix { - path: require_interp(path.as_ref(), "cli.target.path", errors), - }), + Some(CliTargetLayer::Http { url }) => { + super::warn_if_demoted_template("cli.target.http.url", url.as_deref()); + Some(CliTargetSettings::Http { + url: require_string(url.as_ref(), "cli.target.url", errors), + }) + } + Some(CliTargetLayer::Unix { path }) => { + super::warn_if_demoted_template("cli.target.unix.path", path.as_deref()); + Some(CliTargetSettings::Unix { + path: require_string(path.as_ref(), "cli.target.path", errors), + }) + } None => None, } } diff --git a/lib/crates/fabro-config/src/resolve/mod.rs b/lib/crates/fabro-config/src/resolve/mod.rs index 54796bbf5..fd7f1e01a 100644 --- a/lib/crates/fabro-config/src/resolve/mod.rs +++ b/lib/crates/fabro-config/src/resolve/mod.rs @@ -29,6 +29,19 @@ pub(crate) fn require_interp( }) } +pub(crate) fn require_string( + value: Option<&String>, + path: &str, + errors: &mut Vec, +) -> String { + value.cloned().unwrap_or_else(|| { + errors.push(ResolveError::Missing { + path: path.to_string(), + }); + String::new() + }) +} + #[expect( clippy::disallowed_methods, reason = "parsed_value special case: the TCP listen address parses the literal source \ diff --git a/lib/crates/fabro-config/src/resolve/run.rs b/lib/crates/fabro-config/src/resolve/run.rs index b554c03f9..01e4e9462 100644 --- a/lib/crates/fabro-config/src/resolve/run.rs +++ b/lib/crates/fabro-config/src/resolve/run.rs @@ -49,6 +49,8 @@ pub fn resolve_run( }); } + super::warn_if_demoted_template("run.working_dir", layer.working_dir.as_deref()); + RunNamespace { goal: resolve_goal(layer.goal.as_ref()), working_dir: layer.working_dir.clone(), diff --git a/lib/crates/fabro-config/src/tests/resolve_cli.rs b/lib/crates/fabro-config/src/tests/resolve_cli.rs index 3216f8800..550da9271 100644 --- a/lib/crates/fabro-config/src/tests/resolve_cli.rs +++ b/lib/crates/fabro-config/src/tests/resolve_cli.rs @@ -3,7 +3,6 @@ reason = "sync test fixture setup; not on a Tokio path" )] -use fabro_types::settings::InterpString; use fabro_types::settings::cli::{CliTargetSettings, OutputFormat, OutputVerbosity}; use fabro_types::settings::run::AgentPermissions; use temp_env::with_var; @@ -42,7 +41,7 @@ url = "https://config.example.com" assert_eq!( user_settings.cli.target, Some(CliTargetSettings::Http { - url: InterpString::parse("https://config.example.com"), + url: "https://config.example.com".to_string(), }) ); } @@ -80,10 +79,6 @@ fn user_settings_resolve_returns_defaults_when_default_settings_file_is_missing( }); } -#[expect( - clippy::disallowed_methods, - reason = "test asserts the raw template source" -)] #[test] fn resolves_cli_target_exec_and_output_settings() { let cli = UserSettingsBuilder::from_toml( @@ -125,7 +120,7 @@ level = "debug" let CliTargetSettings::Http { url } = cli.target.expect("target") else { panic!("expected http target"); }; - assert_eq!(url.as_source(), "https://config.example.com"); + assert_eq!(url, "https://config.example.com"); assert!(cli.exec.prevent_idle_sleep); assert_eq!(cli.exec.model.provider.as_deref(), Some("openai")); diff --git a/lib/crates/fabro-config/src/tests/resolve_run.rs b/lib/crates/fabro-config/src/tests/resolve_run.rs index 877e7ebd0..d942a428d 100644 --- a/lib/crates/fabro-config/src/tests/resolve_run.rs +++ b/lib/crates/fabro-config/src/tests/resolve_run.rs @@ -586,9 +586,10 @@ name = "sonnet" } other => panic!("expected file goal, got {other:?}"), } + // run.working_dir is demoted (D11): the env token stays literal text. assert_eq!( - settings.working_dir, - Some(InterpString::parse("{{ env.FABRO_WORKDIR }}")) + settings.working_dir.as_deref(), + Some("{{ env.FABRO_WORKDIR }}") ); assert_eq!(settings.model.provider, Some("anthropic".to_string())); assert_eq!(settings.model.name, Some("sonnet".to_string())); diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index 010d7ab6a..a900e9b54 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -1785,7 +1785,7 @@ mod runs { goal: Some(RunGoal::Inline(InterpString::parse( "Add rate limiting to auth endpoints", ))), - working_dir: Some(InterpString::parse("/workspace/api-server")), + working_dir: Some("/workspace/api-server".to_string()), model: RunModelSettings { provider: Some("anthropic".to_string()), name: Some("claude-opus-4-6".to_string()), diff --git a/lib/crates/fabro-types/src/settings/cli.rs b/lib/crates/fabro-types/src/settings/cli.rs index a429c266a..0d97d7b89 100644 --- a/lib/crates/fabro-types/src/settings/cli.rs +++ b/lib/crates/fabro-types/src/settings/cli.rs @@ -9,7 +9,6 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; -use super::interp::InterpString; use super::run::{AgentPermissions, McpServerSettings}; /// A structurally resolved `[cli]` view for consumers. @@ -26,8 +25,8 @@ pub struct CliNamespace { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "lowercase")] pub enum CliTargetSettings { - Http { url: InterpString }, - Unix { path: InterpString }, + Http { url: String }, + Unix { path: String }, } #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index 15f453ee9..cd5e30949 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -22,7 +22,7 @@ use super::size::Size; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct RunNamespace { pub goal: Option, - pub working_dir: Option, + pub working_dir: Option, pub metadata: HashMap, pub inputs: HashMap, pub model: RunModelSettings, @@ -82,10 +82,10 @@ impl RunNamespace { F: FnMut(&str) -> Option, { substitute_goal(&mut self.goal, &mut lookup)?; - substitute_option(&mut self.working_dir, &mut lookup)?; substitute_string_map(&mut self.metadata, &mut lookup)?; - // run.model.provider/name and run.git.author.* were demoted to plain - // `String` and removed from this pass (D2): values stay literal. + // run.working_dir, run.model.provider/name, and run.git.author.* were + // demoted to plain `String` and removed from this pass (D2/D11): + // values stay literal. substitute_option_string(&mut self.model.controls.reasoning_effort, &mut lookup)?; substitute_option_string(&mut self.model.controls.speed, &mut lookup)?; substitute_string_vec(&mut self.checkpoint.exclude_globs, &mut lookup)?; @@ -317,7 +317,6 @@ mod run_namespace_variable_substitution_tests { goal: Some(RunGoal::Inline(InterpString::parse( "deploy {{ vars.ENV }} in {{ env.REGION }}", ))), - working_dir: Some(InterpString::parse("/workspace/{{ vars.ENV }}")), prepare: RunPrepareSettings { commands: vec!["echo {{ vars.ENV }} {{ env.REGION }}".to_string()], timeout_ms: 1_000, @@ -376,10 +375,6 @@ mod run_namespace_variable_substitution_tests { goal_source, Some("deploy prod in {{ env.REGION }}".to_string()) ); - assert_eq!( - run.working_dir.as_ref().map(InterpString::as_source), - Some("/workspace/prod".to_string()) - ); assert_eq!(run.prepare.commands, vec![ "echo prod {{ env.REGION }}".to_string() ]); @@ -408,10 +403,12 @@ mod run_namespace_variable_substitution_tests { #[test] fn demoted_fields_do_not_interpolate() { - // Demoted fields (run.model.*, run.git.author.*, run.scm.owner/ - // repository) were removed from the vars pass (D2): `{{ vars.* }}` - // and `{{ env.* }}` stay literal even when a value is available. + // Demoted fields (run.working_dir, run.model.*, run.git.author.*, + // run.scm.owner/repository) were removed from the vars pass (D2/D11): + // `{{ vars.* }}` and `{{ env.* }}` stay literal even when a value is + // available. let mut run = RunNamespace { + working_dir: Some("/workspace/{{ vars.ENV }}".to_string()), model: super::RunModelSettings { provider: Some("{{ vars.PROVIDER }}".to_string()), name: Some("{{ vars.MODEL }}".to_string()), @@ -435,6 +432,10 @@ mod run_namespace_variable_substitution_tests { run.substitute_variables(|_| Some("SUBSTITUTED".to_string())) .unwrap(); + assert_eq!( + run.working_dir.as_deref(), + Some("/workspace/{{ vars.ENV }}") + ); assert_eq!(run.model.provider.as_deref(), Some("{{ vars.PROVIDER }}")); assert_eq!(run.model.name.as_deref(), Some("{{ vars.MODEL }}")); let author = run.git.author.as_ref().unwrap(); diff --git a/lib/crates/fabro-workflow/src/operations/create.rs b/lib/crates/fabro-workflow/src/operations/create.rs index 3c3ed5a72..9dba7fe6f 100644 --- a/lib/crates/fabro-workflow/src/operations/create.rs +++ b/lib/crates/fabro-workflow/src/operations/create.rs @@ -1276,7 +1276,7 @@ mod tests { }, settings: settings_from_run_layer({ RunLayer { - working_dir: Some(InterpString::parse("workspace")), + working_dir: Some("workspace".to_string()), execution: Some(RunExecutionLayer { mode: Some(RunMode::DryRun), ..RunExecutionLayer::default() diff --git a/lib/crates/fabro-workflow/src/operations/source.rs b/lib/crates/fabro-workflow/src/operations/source.rs index 40461766e..bb00adaca 100644 --- a/lib/crates/fabro-workflow/src/operations/source.rs +++ b/lib/crates/fabro-workflow/src/operations/source.rs @@ -126,11 +126,12 @@ fn resolve_goal_override( #[cfg(test)] mod tests { + use fabro_types::settings::InterpString; + use super::*; #[test] fn resolve_workflow_uses_explicit_cwd_for_relative_work_dir() { - use fabro_types::settings::InterpString; use fabro_types::settings::run::RunNamespace; let dir = tempfile::tempdir().unwrap(); @@ -141,7 +142,7 @@ mod tests { }, settings: WorkflowSettings { run: RunNamespace { - working_dir: Some(InterpString::parse("workspace")), + working_dir: Some("workspace".to_string()), ..RunNamespace::default() }, ..WorkflowSettings::default() @@ -155,7 +156,6 @@ mod tests { #[test] fn resolve_workflow_reads_goal_override_from_dense_run_settings() { - use fabro_types::settings::InterpString; use fabro_types::settings::run::{RunGoal, RunNamespace}; let dir = tempfile::tempdir().unwrap();