From f48b777717e09eb68ef34736d328e96f7a39e9ac Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 4 May 2026 11:50:01 +0200 Subject: [PATCH] feat: support template interpolation in multi-agent usage hints (#20973) ## Why `multi_agent_v2` usage hints sometimes need to reference resolved config values such as the effective thread limit. Those values only exist after config layering, defaulting, and feature materialization, so the raw TOML alone was not enough to render them. ## What changed - allow `features.multi_agent_v2.{usage_hint_text,root_agent_usage_hint_text,subagent_usage_hint_text}` to use `{{ ... }}` placeholders backed by the materialized effective config - fail config loading with a targeted error when a referenced placeholder does not exist or does not resolve to a scalar value - move resolved-config materialization into a shared helper so config interpolation and config-lock export/replay both serialize the same resolved feature, memory, and agent settings ## Example ``` [features.multi_agent_v2] enabled = true usage_hint_text = "lorem {{ features.multi_agent_v2.max_concurrent_threads_per_session }} ipsum" ``` gets rendered as ``` "description": String("... \lorem 4 ipsum"), ``` --- codex-rs/core/src/config/config_tests.rs | 62 ++++++ codex-rs/core/src/config/mod.rs | 58 ++++++ .../core/src/config/template_interpolation.rs | 189 ++++++++++++++++++ codex-rs/core/src/session/config_lock.rs | 89 +-------- 4 files changed, 313 insertions(+), 85 deletions(-) create mode 100644 codex-rs/core/src/config/template_interpolation.rs diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 1352de991e..b48cf020f2 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -8468,6 +8468,68 @@ hide_spawn_agent_metadata = true Ok(()) } +#[tokio::test] +async fn multi_agent_v2_usage_hint_templates_use_materialized_config_values() -> std::io::Result<()> +{ + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[features.multi_agent_v2] +enabled = true +usage_hint_text = "Limit {{ features.multi_agent_v2.max_concurrent_threads_per_session }}" +root_agent_usage_hint_text = "Root {{ features.multi_agent_v2.max_concurrent_threads_per_session }}" +subagent_usage_hint_text = "Subagent {{ features.multi_agent_v2.max_concurrent_threads_per_session }}" +"#, + )?; + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + + assert_eq!( + config.multi_agent_v2.usage_hint_text.as_deref(), + Some("Limit 4") + ); + assert_eq!( + config.multi_agent_v2.root_agent_usage_hint_text.as_deref(), + Some("Root 4") + ); + assert_eq!( + config.multi_agent_v2.subagent_usage_hint_text.as_deref(), + Some("Subagent 4") + ); + + Ok(()) +} + +#[tokio::test] +async fn multi_agent_v2_usage_hint_templates_fail_when_placeholder_is_missing() { + let codex_home = TempDir::new().expect("create codex home"); + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[features.multi_agent_v2] +enabled = true +usage_hint_text = "{{ features.multi_agent_v2.does_not_exist }}" +"#, + ) + .expect("write config"); + + let err = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await + .expect_err("config load should fail"); + + assert!( + err.to_string() + .contains("features.multi_agent_v2.does_not_exist"), + "unexpected error: {err}", + ); +} + #[tokio::test] async fn profile_multi_agent_v2_config_overrides_base() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 20b2a923f8..2154bb0170 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -132,6 +132,7 @@ mod network_proxy_spec; mod permissions; #[cfg(test)] mod schema; +pub(crate) mod template_interpolation; pub use codex_config::Constrained; pub use codex_config::ConstraintError; pub use codex_config::ConstraintResult; @@ -2015,6 +2016,63 @@ impl Config { overrides: ConfigOverrides, codex_home: AbsolutePathBuf, config_layer_stack: ConfigLayerStack, + ) -> std::io::Result { + let config = Self::build_config_with_layer_stack( + fs, + cfg.clone(), + overrides.clone(), + codex_home.clone(), + config_layer_stack.clone(), + ) + .await?; + let mut interpolation_source_cfg = cfg.clone(); + template_interpolation::apply_resolved_config_fields( + &config, + &mut interpolation_source_cfg, + ) + .map_err(|err| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("failed to materialize config for interpolation: {err}"), + ) + })?; + let interpolation_source = + toml::Value::try_from(interpolation_source_cfg).map_err(|err| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("failed to serialize config for interpolation: {err}"), + ) + })?; + let mut interpolated_cfg = cfg; + let interpolated = template_interpolation::interpolate_config_string_fields( + &mut interpolated_cfg, + &interpolation_source, + ) + .map_err(|err| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("failed to interpolate config template fields: {err}"), + ) + })?; + if interpolated { + return Self::build_config_with_layer_stack( + fs, + interpolated_cfg, + overrides, + codex_home, + config_layer_stack, + ) + .await; + } + Ok(config) + } + + async fn build_config_with_layer_stack( + fs: &dyn ExecutorFileSystem, + cfg: ConfigToml, + overrides: ConfigOverrides, + codex_home: AbsolutePathBuf, + config_layer_stack: ConfigLayerStack, ) -> std::io::Result { // Keep the large config-construction future off small test thread stacks. Box::pin(async move { diff --git a/codex-rs/core/src/config/template_interpolation.rs b/codex-rs/core/src/config/template_interpolation.rs new file mode 100644 index 0000000000..7e38fed86f --- /dev/null +++ b/codex-rs/core/src/config/template_interpolation.rs @@ -0,0 +1,189 @@ +use anyhow::Context; +use anyhow::bail; +use codex_config::config_toml::ConfigToml; +use codex_config::types::MemoriesToml; +use codex_features::AppsMcpPathOverrideConfigToml; +use codex_features::Feature; +use codex_features::FeatureToml; +use codex_features::FeaturesToml; +use codex_features::MultiAgentV2ConfigToml; +use codex_utils_template::Template; +use toml::Value as TomlValue; + +use super::Config; + +const INTERPOLATED_CONFIG_STRING_FIELDS: &[&str] = &[ + "features.multi_agent_v2.usage_hint_text", + "features.multi_agent_v2.root_agent_usage_hint_text", + "features.multi_agent_v2.subagent_usage_hint_text", +]; + +pub(crate) fn materialized_config_toml(config: &Config) -> anyhow::Result { + let mut materialized: ConfigToml = config + .config_layer_stack + .effective_config() + .try_into() + .context("failed to deserialize effective config for config interpolation")?; + apply_resolved_config_fields(config, &mut materialized)?; + Ok(materialized) +} + +pub(crate) fn interpolate_config_string_fields( + config_toml: &mut ConfigToml, + interpolation_source: &TomlValue, +) -> anyhow::Result { + let mut target_value = TomlValue::try_from(config_toml.clone()) + .context("failed to serialize config for interpolation")?; + let mut changed = false; + + for field_path in INTERPOLATED_CONFIG_STRING_FIELDS { + let Some(value) = value_mut_at_path(&mut target_value, field_path) else { + continue; + }; + let Some(template_source) = value.as_str() else { + bail!("interpolated config field `{field_path}` must be a string"); + }; + let template = Template::parse(template_source) + .with_context(|| format!("failed to parse template in config field `{field_path}`"))?; + let rendered = render_template(&template, interpolation_source, field_path)?; + if rendered != template_source { + *value = TomlValue::String(rendered); + changed = true; + } + } + + if changed { + *config_toml = target_value + .try_into() + .context("failed to deserialize interpolated config")?; + } + + Ok(changed) +} + +pub(crate) fn apply_resolved_config_fields( + config: &Config, + config_toml: &mut ConfigToml, +) -> anyhow::Result<()> { + config_toml.web_search = Some(config.web_search_mode.value()); + config_toml.model_provider = Some(config.model_provider_id.clone()); + config_toml.plan_mode_reasoning_effort = config.plan_mode_reasoning_effort; + config_toml.model_verbosity = config.model_verbosity; + config_toml.include_permissions_instructions = Some(config.include_permissions_instructions); + config_toml.include_apps_instructions = Some(config.include_apps_instructions); + config_toml.include_environment_context = Some(config.include_environment_context); + config_toml.background_terminal_max_timeout = Some(config.background_terminal_max_timeout); + + // Feature aliases and feature configs need to be written in their resolved + // form; otherwise replay can drift when a legacy key maps to the same + // runtime feature. + let features = config_toml + .features + .get_or_insert_with(FeaturesToml::default); + features.materialize_resolved_enabled(config.features.get()); + let mut multi_agent_v2: MultiAgentV2ConfigToml = + resolved_config_to_toml(&config.multi_agent_v2, "features.multi_agent_v2")?; + multi_agent_v2.enabled = Some(config.features.enabled(Feature::MultiAgentV2)); + features.multi_agent_v2 = Some(FeatureToml::Config(multi_agent_v2)); + features.apps_mcp_path_override = Some(FeatureToml::Config(AppsMcpPathOverrideConfigToml { + enabled: Some(config.features.enabled(Feature::AppsMcpPathOverride)), + path: config.apps_mcp_path_override.clone(), + })); + + config_toml.memories = Some(resolved_config_to_toml::( + &config.memories, + "memories", + )?); + + let agents = config_toml.agents.get_or_insert_with(Default::default); + // Multi-agent v2 owns thread fanout through its feature config. Preserve + // the legacy agents.max_threads setting only when v2 is disabled. + agents.max_threads = if config.features.enabled(Feature::MultiAgentV2) { + None + } else { + config.agent_max_threads + }; + agents.max_depth = Some(config.agent_max_depth); + agents.job_max_runtime_seconds = config.agent_job_max_runtime_seconds; + agents.interrupt_message = Some(config.agent_interrupt_message_enabled); + + config_toml + .skills + .get_or_insert_with(Default::default) + .include_instructions = Some(config.include_skill_instructions); + + Ok(()) +} + +fn render_template( + template: &Template, + interpolation_source: &TomlValue, + field_path: &str, +) -> anyhow::Result { + let variables = + template + .placeholders() + .map(|placeholder| { + let value = lookup_scalar_path(interpolation_source, placeholder).with_context(|| { + format!("failed to render config field `{field_path}` placeholder `{placeholder}`") + })?; + Ok((placeholder.to_string(), value)) + }) + .collect::>>()?; + + template + .render( + variables + .iter() + .map(|(name, value)| (name.as_str(), value.as_str())), + ) + .with_context(|| format!("failed to render config field `{field_path}`")) +} + +fn lookup_scalar_path(value: &TomlValue, path: &str) -> anyhow::Result { + let resolved = value_at_path(value, path) + .with_context(|| format!("template placeholder `{path}` does not exist"))?; + + match resolved { + TomlValue::String(value) => Ok(value.clone()), + TomlValue::Integer(value) => Ok(value.to_string()), + TomlValue::Float(value) => Ok(value.to_string()), + TomlValue::Boolean(value) => Ok(value.to_string()), + _ => bail!( + "template placeholder `{path}` must resolve to a scalar string, integer, float, or boolean" + ), + } +} + +fn value_at_path<'a>(value: &'a TomlValue, path: &str) -> Option<&'a TomlValue> { + let mut current = value; + for segment in path.split('.') { + current = current.as_table()?.get(segment)?; + } + Some(current) +} + +fn value_mut_at_path<'a>(value: &'a mut TomlValue, path: &str) -> Option<&'a mut TomlValue> { + let mut current = value; + let mut segments = path.split('.').peekable(); + + while let Some(segment) = segments.next() { + let table = current.as_table_mut()?; + if segments.peek().is_none() { + return table.get_mut(segment); + } + current = table.get_mut(segment)?; + } + + Some(current) +} + +fn resolved_config_to_toml( + value: &impl serde::Serialize, + label: &'static str, +) -> anyhow::Result +where + Toml: serde::de::DeserializeOwned + serde::Serialize, +{ + crate::config_lock::toml_round_trip(value, label).map_err(anyhow::Error::from) +} diff --git a/codex-rs/core/src/session/config_lock.rs b/codex-rs/core/src/session/config_lock.rs index d1f190510a..f0a8d209d3 100644 --- a/codex-rs/core/src/session/config_lock.rs +++ b/codex-rs/core/src/session/config_lock.rs @@ -1,19 +1,12 @@ use anyhow::Context; use codex_config::config_toml::ConfigLockfileToml; use codex_config::config_toml::ConfigToml; -use codex_config::types::MemoriesToml; -use codex_features::AppsMcpPathOverrideConfigToml; -use codex_features::Feature; -use codex_features::FeatureToml; -use codex_features::FeaturesToml; -use codex_features::MultiAgentV2ConfigToml; use codex_protocol::ThreadId; -use crate::config::Config; +use crate::config::template_interpolation::materialized_config_toml; use crate::config_lock::ConfigLockReplayOptions; use crate::config_lock::clear_config_lock_debug_controls; use crate::config_lock::config_lockfile; -use crate::config_lock::toml_round_trip; use crate::config_lock::validate_config_lock_replay; use super::SessionConfiguration; @@ -81,20 +74,12 @@ fn session_configuration_to_lock_config_toml( sc: &SessionConfiguration, ) -> anyhow::Result { let config = sc.original_config_do_not_use.as_ref(); - // Start from the resolved layer stack, then patch in values that are only - // known after session setup. Export and replay validation both use this - // path, so every field here is part of the lockfile contract. - let mut lock_config: ConfigToml = config - .config_layer_stack - .effective_config() - .try_into() - .context("failed to deserialize effective config for config lock")?; + let mut lock_config = materialized_config_toml(config)?; if config.config_lock_save_fields_resolved_from_model_catalog { save_session_resolved_fields(sc, &mut lock_config); } - save_config_resolved_fields(config, &mut lock_config)?; drop_lockfile_inputs(&mut lock_config); Ok(lock_config) @@ -118,64 +103,6 @@ fn save_session_resolved_fields(sc: &SessionConfiguration, lock_config: &mut Con lock_config.approvals_reviewer = Some(sc.approvals_reviewer); } -/// Saves values stored on `Config` after higher-level resolution, -/// normalization, defaulting, or feature materialization. -/// -/// Persist the resolved representation so replay compares against the behavior -/// Codex actually ran with, not only the user-authored TOML inputs. -fn save_config_resolved_fields( - config: &Config, - lock_config: &mut ConfigToml, -) -> anyhow::Result<()> { - lock_config.web_search = Some(config.web_search_mode.value()); - lock_config.model_provider = Some(config.model_provider_id.clone()); - lock_config.plan_mode_reasoning_effort = config.plan_mode_reasoning_effort; - lock_config.model_verbosity = config.model_verbosity; - lock_config.include_permissions_instructions = Some(config.include_permissions_instructions); - lock_config.include_apps_instructions = Some(config.include_apps_instructions); - lock_config.include_environment_context = Some(config.include_environment_context); - lock_config.background_terminal_max_timeout = Some(config.background_terminal_max_timeout); - - // Feature aliases and feature configs need to be written in their resolved - // form; otherwise replay can drift when a legacy key maps to the same - // runtime feature. - let features = lock_config - .features - .get_or_insert_with(FeaturesToml::default); - features.materialize_resolved_enabled(config.features.get()); - let mut multi_agent_v2: MultiAgentV2ConfigToml = - resolved_config_to_toml(&config.multi_agent_v2, "features.multi_agent_v2")?; - multi_agent_v2.enabled = Some(config.features.enabled(Feature::MultiAgentV2)); - features.multi_agent_v2 = Some(FeatureToml::Config(multi_agent_v2)); - features.apps_mcp_path_override = Some(FeatureToml::Config(AppsMcpPathOverrideConfigToml { - enabled: Some(config.features.enabled(Feature::AppsMcpPathOverride)), - path: config.apps_mcp_path_override.clone(), - })); - lock_config.memories = Some(resolved_config_to_toml::( - &config.memories, - "memories", - )?); - - let agents = lock_config.agents.get_or_insert_with(Default::default); - // Multi-agent v2 owns thread fanout through its feature config. Preserve - // the legacy agents.max_threads setting only when v2 is disabled. - agents.max_threads = if config.features.enabled(Feature::MultiAgentV2) { - None - } else { - config.agent_max_threads - }; - agents.max_depth = Some(config.agent_max_depth); - agents.job_max_runtime_seconds = config.agent_job_max_runtime_seconds; - agents.interrupt_message = Some(config.agent_interrupt_message_enabled); - - lock_config - .skills - .get_or_insert_with(Default::default) - .include_instructions = Some(config.include_skill_instructions); - - Ok(()) -} - fn drop_lockfile_inputs(lock_config: &mut ConfigToml) { // The lockfile should contain replayable values, not the profile, // debug-control, file-include, and environment-specific inputs that @@ -195,19 +122,11 @@ fn drop_lockfile_inputs(lock_config: &mut ConfigToml) { lock_config.experimental_use_freeform_apply_patch = None; } -fn resolved_config_to_toml( - value: &impl serde::Serialize, - label: &'static str, -) -> anyhow::Result -where - Toml: serde::de::DeserializeOwned + serde::Serialize, -{ - toml_round_trip(value, label).map_err(anyhow::Error::from) -} - #[cfg(test)] mod tests { use super::*; + use codex_features::FeatureToml; + use codex_features::MultiAgentV2ConfigToml; use pretty_assertions::assert_eq; use std::sync::Arc;