diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 1352de991..b48cf020f 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 20b2a923f..2154bb017 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 000000000..7e38fed86 --- /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 d1f190510..f0a8d209d 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;