mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: add layered --profile-v2 config files (#17141)
## Why `--profile-v2 <name>` gives launchers and runtime entry points a named profile config without making each profile duplicate the base user config. The base `$CODEX_HOME/config.toml` still loads first, then `$CODEX_HOME/<name>.config.toml` layers above it and becomes the active writable user config for that session. That keeps shared defaults, plugin/MCP setup, and managed/user constraints in one place while letting a named profile override only the pieces that need to differ. ## What Changed - Added the shared `--profile-v2 <name>` runtime option with validated plain names, now represented by `ProfileV2Name`. - Extended config layer state so the base user config and selected profile config are both `User` layers; APIs expose the active user layer and merged effective user config. - Threaded profile selection through runtime entry points: `codex`, `codex exec`, `codex review`, `codex resume`, `codex fork`, and `codex debug prompt-input`. - Made user-facing config writes go to the selected profile file when active, including TUI/settings persistence, app-server config writes, and MCP/app tool approval persistence. - Made plugin, marketplace, MCP, hooks, and config reload paths read from the merged user config so base and profile layers both participate. - Updated app-server config layer schemas to mark profile-backed user layers. ## Limits `--profile-v2` is still rejected for config-management subcommands such as feature, MCP, and marketplace edits. Those paths remain tied to the base `config.toml` until they have explicit profile-selection semantics. Some adjacent background writes may still update base or global state rather than the selected profile: - marketplace auto-upgrade metadata - automatic MCP dependency installs from skills - remote plugin sync or uninstall config edits - personality migration marker/default writes ## Verification Added targeted coverage for profile name validation, layer ordering/merging, selected-profile writes, app-server config writes, session hot reload, plugin config merging, hooks/config fixture updates, and MCP/app approval persistence. --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
17cd321c32
commit
deedf3b2c4
@@ -184,7 +184,7 @@ invalid = ["#,
|
||||
.await?;
|
||||
|
||||
let user_layer = layers
|
||||
.get_user_layer()
|
||||
.get_active_user_layer()
|
||||
.expect("expected a user layer even when CODEX_HOME/config.toml is ignored");
|
||||
assert_eq!(
|
||||
user_layer.config,
|
||||
@@ -329,7 +329,7 @@ command = "python3 /tmp/user-hook.py"
|
||||
|
||||
assert!(
|
||||
layers
|
||||
.get_user_layer()
|
||||
.get_active_user_layer()
|
||||
.and_then(|layer| layer.config.get("hooks"))
|
||||
.is_some(),
|
||||
"hooks should still deserialize from config.toml"
|
||||
@@ -572,11 +572,12 @@ async fn returns_empty_when_all_layers_missing() {
|
||||
.await
|
||||
.expect("load layers");
|
||||
let user_layer = layers
|
||||
.get_user_layer()
|
||||
.get_active_user_layer()
|
||||
.expect("expected a user layer even when CODEX_HOME/config.toml does not exist");
|
||||
let expected_user_layer = ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, tmp.path()),
|
||||
profile: None,
|
||||
},
|
||||
TomlValue::Table(toml::map::Map::new()),
|
||||
);
|
||||
@@ -614,6 +615,78 @@ async fn returns_empty_when_all_layers_missing() {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selected_user_config_file_layers_over_base_user_config() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let managed_path = tmp.path().join("managed_config.toml");
|
||||
let selected_config = tmp.path().join("work.config.toml");
|
||||
|
||||
std::fs::write(
|
||||
tmp.path().join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
model = "gpt-main"
|
||||
approval_policy = "on-failure"
|
||||
"#,
|
||||
)
|
||||
.expect("write default user config");
|
||||
std::fs::write(&selected_config, r#"model = "gpt-work""#).expect("write selected user config");
|
||||
|
||||
let mut overrides = LoaderOverrides::with_managed_config_path_for_tests(managed_path);
|
||||
overrides.user_config_path =
|
||||
Some(AbsolutePathBuf::from_absolute_path(&selected_config).expect("selected config path"));
|
||||
overrides.user_config_profile = Some("work".parse().expect("profile-v2 name"));
|
||||
|
||||
let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd");
|
||||
let layers = load_config_layers_state(
|
||||
LOCAL_FS.as_ref(),
|
||||
tmp.path(),
|
||||
Some(cwd),
|
||||
&[] as &[(String, TomlValue)],
|
||||
overrides,
|
||||
CloudRequirementsLoader::default(),
|
||||
&codex_config::NoopThreadConfigLoader,
|
||||
)
|
||||
.await
|
||||
.expect("load layers");
|
||||
|
||||
let user_layers = layers.get_user_layers(
|
||||
super::ConfigLayerStackOrdering::LowestPrecedenceFirst,
|
||||
/*include_disabled*/ false,
|
||||
);
|
||||
assert_eq!(user_layers.len(), 2);
|
||||
assert_eq!(
|
||||
user_layers[0].name,
|
||||
ConfigLayerSource::User {
|
||||
file: AbsolutePathBuf::from_absolute_path(tmp.path().join(CONFIG_TOML_FILE))
|
||||
.expect("base user config path"),
|
||||
profile: None,
|
||||
}
|
||||
);
|
||||
let user_layer = layers.get_active_user_layer().expect("selected user layer");
|
||||
assert_eq!(
|
||||
user_layer.name,
|
||||
ConfigLayerSource::User {
|
||||
file: AbsolutePathBuf::from_absolute_path(&selected_config)
|
||||
.expect("selected user config path"),
|
||||
profile: Some("work".to_string()),
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
layers
|
||||
.effective_config()
|
||||
.get("model")
|
||||
.and_then(TomlValue::as_str),
|
||||
Some("gpt-work")
|
||||
);
|
||||
assert_eq!(
|
||||
layers
|
||||
.effective_config()
|
||||
.get("approval_policy")
|
||||
.and_then(TomlValue::as_str),
|
||||
Some("on-failure")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn includes_thread_config_layers_in_stack() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
@@ -653,6 +726,7 @@ async fn includes_thread_config_layers_in_stack() -> anyhow::Result<()> {
|
||||
ConfigLayerSource::SessionFlags,
|
||||
ConfigLayerSource::User {
|
||||
file: AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, tmp.path()),
|
||||
profile: None,
|
||||
},
|
||||
ConfigLayerSource::System {
|
||||
file: expected_system_config,
|
||||
|
||||
@@ -7,6 +7,7 @@ use crate::config::edit::apply_blocking;
|
||||
use assert_matches::assert_matches;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerEntry;
|
||||
use codex_config::ProfileV2Name;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::config_toml::AgentRoleToml;
|
||||
use codex_config::config_toml::AgentsToml;
|
||||
@@ -3334,6 +3335,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io::
|
||||
ConfigLayerEntry::new(
|
||||
codex_app_server_protocol::ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
profile: None,
|
||||
},
|
||||
toml::toml! {
|
||||
[mcp_servers.session_overrides_user]
|
||||
@@ -3388,6 +3390,7 @@ async fn rebuild_preserving_session_layers_refreshes_requirements() -> std::io::
|
||||
ConfigLayerEntry::new(
|
||||
codex_app_server_protocol::ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
profile: None,
|
||||
},
|
||||
toml::toml! {
|
||||
[mcp_servers.session_overrides_user]
|
||||
@@ -3518,6 +3521,7 @@ async fn rebuild_preserving_session_layers_refreshes_plugin_derived_mcp_config()
|
||||
vec![ConfigLayerEntry::new(
|
||||
codex_app_server_protocol::ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
profile: None,
|
||||
},
|
||||
toml::toml! {
|
||||
[features]
|
||||
@@ -3544,7 +3548,10 @@ async fn rebuild_preserving_session_layers_refreshes_plugin_derived_mcp_config()
|
||||
.await?;
|
||||
let thread_layer_stack = ConfigLayerStack::new(
|
||||
vec![ConfigLayerEntry::new(
|
||||
codex_app_server_protocol::ConfigLayerSource::User { file: user_file },
|
||||
codex_app_server_protocol::ConfigLayerSource::User {
|
||||
file: user_file,
|
||||
profile: None,
|
||||
},
|
||||
toml::toml! {
|
||||
[features]
|
||||
plugins = false
|
||||
@@ -5489,6 +5496,52 @@ async fn set_model_updates_defaults() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn for_config_writes_selected_user_config_file() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let base_config = codex_home.path().join(CONFIG_TOML_FILE);
|
||||
let selected_config = codex_home.path().join("work.config.toml");
|
||||
tokio::fs::write(&base_config, r#"model_provider = "openai""#).await?;
|
||||
tokio::fs::write(&selected_config, r#"model = "gpt-old""#).await?;
|
||||
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.loader_overrides(LoaderOverrides {
|
||||
user_config_path: Some(selected_config.abs()),
|
||||
user_config_profile: Some("work".parse().expect("profile-v2 name")),
|
||||
..LoaderOverrides::without_managed_config_for_tests()
|
||||
})
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
ConfigEditsBuilder::for_config(&config)
|
||||
.set_model(Some("gpt-new"), Some(ReasoningEffort::High))
|
||||
.apply()
|
||||
.await?;
|
||||
|
||||
let selected_serialized = tokio::fs::read_to_string(&selected_config).await?;
|
||||
let selected: ConfigToml = toml::from_str(&selected_serialized)?;
|
||||
assert_eq!(selected.model.as_deref(), Some("gpt-new"));
|
||||
assert_eq!(selected.model_reasoning_effort, Some(ReasoningEffort::High));
|
||||
assert_eq!(
|
||||
tokio::fs::read_to_string(&base_config).await?,
|
||||
r#"model_provider = "openai""#
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn profile_v2_config_path_resolves_validated_names() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let profile_name: ProfileV2Name = "work".parse()?;
|
||||
assert_eq!(
|
||||
resolve_profile_v2_config_path(codex_home.path(), &profile_name),
|
||||
codex_home.path().join("work.config.toml").abs()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn set_model_overwrites_existing_model() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -6080,6 +6133,7 @@ config_file = "./agents/researcher.toml"
|
||||
vec![codex_config::ConfigLayerEntry::new(
|
||||
codex_app_server_protocol::ConfigLayerSource::User {
|
||||
file: codex_home.path().join(CONFIG_TOML_FILE).abs(),
|
||||
profile: None,
|
||||
},
|
||||
layer_config,
|
||||
)],
|
||||
|
||||
@@ -1037,13 +1037,21 @@ pub fn apply_blocking(
|
||||
codex_home: &Path,
|
||||
profile: Option<&str>,
|
||||
edits: &[ConfigEdit],
|
||||
) -> anyhow::Result<()> {
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
apply_blocking_to_resolved_file(&config_path, profile, edits)
|
||||
}
|
||||
|
||||
fn apply_blocking_to_resolved_file(
|
||||
resolved_config_file: &Path,
|
||||
legacy_profile: Option<&str>,
|
||||
edits: &[ConfigEdit],
|
||||
) -> anyhow::Result<()> {
|
||||
if edits.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let write_paths = resolve_symlink_write_paths(&config_path)?;
|
||||
let write_paths = resolve_symlink_write_paths(resolved_config_file)?;
|
||||
let serialized = match write_paths.read_path {
|
||||
Some(path) => match std::fs::read_to_string(&path) {
|
||||
Ok(contents) => contents,
|
||||
@@ -1059,7 +1067,7 @@ pub fn apply_blocking(
|
||||
serialized.parse::<DocumentMut>()?
|
||||
};
|
||||
|
||||
let profile = profile.map(ToOwned::to_owned).or_else(|| {
|
||||
let profile = legacy_profile.map(ToOwned::to_owned).or_else(|| {
|
||||
doc.get("profile")
|
||||
.and_then(|item| item.as_str())
|
||||
.map(ToOwned::to_owned)
|
||||
@@ -1078,7 +1086,7 @@ pub fn apply_blocking(
|
||||
|
||||
write_atomically(&write_paths.write_path, &document.doc.to_string()).with_context(|| {
|
||||
format!(
|
||||
"failed to persist config.toml at {}",
|
||||
"failed to persist config at {}",
|
||||
write_paths.write_path.display()
|
||||
)
|
||||
})?;
|
||||
@@ -1087,30 +1095,50 @@ pub fn apply_blocking(
|
||||
}
|
||||
|
||||
/// Persist edits asynchronously by offloading the blocking writer.
|
||||
///
|
||||
/// `profile` selects a legacy `[profiles.<name>]` section inside
|
||||
/// `$CODEX_HOME/config.toml`; profile-v2 callers should resolve their target
|
||||
/// file before constructing a [ConfigEditsBuilder].
|
||||
pub async fn apply(
|
||||
codex_home: &Path,
|
||||
profile: Option<&str>,
|
||||
edits: Vec<ConfigEdit>,
|
||||
) -> anyhow::Result<()> {
|
||||
let codex_home = codex_home.to_path_buf();
|
||||
let config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let profile = profile.map(ToOwned::to_owned);
|
||||
task::spawn_blocking(move || apply_blocking(&codex_home, profile.as_deref(), &edits))
|
||||
.await
|
||||
.context("config persistence task panicked")?
|
||||
task::spawn_blocking(move || {
|
||||
apply_blocking_to_resolved_file(&config_path, profile.as_deref(), &edits)
|
||||
})
|
||||
.await
|
||||
.context("config persistence task panicked")?
|
||||
}
|
||||
|
||||
/// Fluent builder to batch config edits and apply them atomically.
|
||||
#[derive(Default)]
|
||||
pub struct ConfigEditsBuilder {
|
||||
codex_home: PathBuf,
|
||||
config_path: PathBuf,
|
||||
profile: Option<String>,
|
||||
edits: Vec<ConfigEdit>,
|
||||
}
|
||||
|
||||
impl ConfigEditsBuilder {
|
||||
pub fn new(codex_home: &Path) -> Self {
|
||||
Self::for_config_path(&codex_home.join(CONFIG_TOML_FILE))
|
||||
}
|
||||
|
||||
pub fn for_config(config: &crate::config::Config) -> Self {
|
||||
let config_path = config
|
||||
.config_layer_stack
|
||||
.get_user_config_file()
|
||||
.map(codex_utils_absolute_path::AbsolutePathBuf::to_path_buf)
|
||||
.unwrap_or_else(|| config.codex_home.join(CONFIG_TOML_FILE).to_path_buf());
|
||||
Self::for_config_path(&config_path)
|
||||
}
|
||||
|
||||
pub fn for_config_path(config_path: &Path) -> Self {
|
||||
Self {
|
||||
codex_home: codex_home.to_path_buf(),
|
||||
config_path: config_path.to_path_buf(),
|
||||
profile: None,
|
||||
edits: Vec::new(),
|
||||
}
|
||||
@@ -1369,13 +1397,13 @@ impl ConfigEditsBuilder {
|
||||
|
||||
/// Apply edits on a blocking thread.
|
||||
pub fn apply_blocking(self) -> anyhow::Result<()> {
|
||||
apply_blocking(&self.codex_home, self.profile.as_deref(), &self.edits)
|
||||
apply_blocking_to_resolved_file(&self.config_path, self.profile.as_deref(), &self.edits)
|
||||
}
|
||||
|
||||
/// Apply edits asynchronously via a blocking offload.
|
||||
pub async fn apply(self) -> anyhow::Result<()> {
|
||||
task::spawn_blocking(move || {
|
||||
apply_blocking(&self.codex_home, self.profile.as_deref(), &self.edits)
|
||||
apply_blocking_to_resolved_file(&self.config_path, self.profile.as_deref(), &self.edits)
|
||||
})
|
||||
.await
|
||||
.context("config persistence task panicked")?
|
||||
|
||||
@@ -18,6 +18,7 @@ use codex_config::FeatureRequirementsToml;
|
||||
use codex_config::McpServerIdentity;
|
||||
use codex_config::McpServerRequirement;
|
||||
use codex_config::PluginRequirementsToml;
|
||||
use codex_config::ProfileV2Name;
|
||||
use codex_config::ResidencyRequirement;
|
||||
use codex_config::SandboxModeRequirement;
|
||||
use codex_config::Sourced;
|
||||
@@ -187,6 +188,7 @@ pub(crate) const DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS: Option<u64> = None;
|
||||
const LOCAL_DEV_BUILD_VERSION: &str = "0.0.0";
|
||||
|
||||
pub const CONFIG_TOML_FILE: &str = "config.toml";
|
||||
const CONFIG_PROFILE_V2_SUFFIX: &str = ".config.toml";
|
||||
|
||||
fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option<PathBuf> {
|
||||
let raw = std::env::var(codex_state::SQLITE_HOME_ENV).ok()?;
|
||||
@@ -1272,6 +1274,52 @@ impl Config {
|
||||
)
|
||||
.await
|
||||
}
|
||||
/// This is a secondary way of creating [Config], which is appropriate when
|
||||
/// the harness is meant to be used with a specific configuration that
|
||||
/// ignores user settings. For example, the `codex exec` subcommand is
|
||||
/// designed to use [AskForApproval::Never] exclusively.
|
||||
///
|
||||
/// Further, [ConfigOverrides] contains some options that are not supported
|
||||
/// in [ConfigToml], such as `cwd`, `codex_self_exe`, `codex_linux_sandbox_exe`, and
|
||||
/// `main_execve_wrapper_exe`.
|
||||
pub async fn load_with_cli_overrides_and_harness_overrides(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
harness_overrides: ConfigOverrides,
|
||||
) -> std::io::Result<Self> {
|
||||
ConfigBuilder::default()
|
||||
.cli_overrides(cli_overrides)
|
||||
.harness_overrides(harness_overrides)
|
||||
.build()
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
pub fn resolve_profile_v2_config_path(
|
||||
codex_home: &Path,
|
||||
profile_name: &ProfileV2Name,
|
||||
) -> AbsolutePathBuf {
|
||||
AbsolutePathBuf::resolve_path_against_base(
|
||||
format!("{profile_name}{CONFIG_PROFILE_V2_SUFFIX}"),
|
||||
codex_home,
|
||||
)
|
||||
}
|
||||
|
||||
/// DEPRECATED: Use [Config::load_with_cli_overrides()] instead because working
|
||||
/// with [ConfigToml] directly means that [ConfigRequirements] have not been
|
||||
/// applied yet, which risks failing to enforce required constraints.
|
||||
pub async fn load_config_as_toml_with_cli_overrides(
|
||||
codex_home: &Path,
|
||||
cwd: Option<&AbsolutePathBuf>,
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
loader_overrides: LoaderOverrides,
|
||||
) -> std::io::Result<ConfigToml> {
|
||||
load_config_as_toml_with_cli_and_loader_overrides(
|
||||
codex_home,
|
||||
cwd,
|
||||
cli_overrides,
|
||||
loader_overrides,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// DEPRECATED for most callers: prefer [Config::load_with_cli_overrides()] or
|
||||
|
||||
@@ -84,6 +84,7 @@ pub(crate) fn lock_layer_from_config(
|
||||
Ok(ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: lock_path.clone(),
|
||||
profile: None,
|
||||
},
|
||||
value,
|
||||
))
|
||||
|
||||
@@ -612,6 +612,7 @@ async fn loads_policies_from_multiple_config_layers() -> anyhow::Result<()> {
|
||||
ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: user_config_toml,
|
||||
profile: None,
|
||||
},
|
||||
TomlValue::Table(Default::default()),
|
||||
),
|
||||
|
||||
@@ -15,7 +15,6 @@ use crate::arc_monitor::monitor_action;
|
||||
use crate::config::Config;
|
||||
use crate::config::edit::ConfigEdit;
|
||||
use crate::config::edit::ConfigEditsBuilder;
|
||||
use crate::config::load_global_mcp_servers;
|
||||
use crate::connectors;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianMcpAnnotations;
|
||||
@@ -2004,8 +2003,7 @@ async fn maybe_persist_mcp_tool_approval(
|
||||
remember_mcp_tool_approval(sess, key).await;
|
||||
return;
|
||||
};
|
||||
persist_codex_app_tool_approval(&turn_context.config.codex_home, &connector_id, &tool_name)
|
||||
.await
|
||||
persist_codex_app_tool_approval(&turn_context.config, &connector_id, &tool_name).await
|
||||
} else {
|
||||
persist_non_app_mcp_tool_approval(sess, &turn_context.config, &key.server, &tool_name).await
|
||||
};
|
||||
@@ -2026,11 +2024,11 @@ async fn maybe_persist_mcp_tool_approval(
|
||||
}
|
||||
|
||||
async fn persist_codex_app_tool_approval(
|
||||
codex_home: &AbsolutePathBuf,
|
||||
config: &Config,
|
||||
connector_id: &str,
|
||||
tool_name: &str,
|
||||
) -> anyhow::Result<()> {
|
||||
ConfigEditsBuilder::new(codex_home)
|
||||
ConfigEditsBuilder::for_config(config)
|
||||
.with_edits([ConfigEdit::SetPath {
|
||||
segments: vec![
|
||||
"apps".to_string(),
|
||||
@@ -2051,11 +2049,12 @@ async fn persist_custom_mcp_tool_approval(
|
||||
server: &str,
|
||||
tool_name: &str,
|
||||
) -> anyhow::Result<()> {
|
||||
let Some(config_folder) = custom_mcp_tool_approval_config_folder(config, server).await? else {
|
||||
let Some(config_edits_builder) = custom_mcp_tool_approval_config_builder(config, server)?
|
||||
else {
|
||||
anyhow::bail!("MCP server `{server}` is not configured in config.toml");
|
||||
};
|
||||
|
||||
persist_custom_mcp_tool_approval_at(&config_folder, server, tool_name).await
|
||||
persist_custom_mcp_tool_approval_with(config_edits_builder, server, tool_name).await
|
||||
}
|
||||
|
||||
async fn persist_non_app_mcp_tool_approval(
|
||||
@@ -2064,8 +2063,9 @@ async fn persist_non_app_mcp_tool_approval(
|
||||
server: &str,
|
||||
tool_name: &str,
|
||||
) -> anyhow::Result<()> {
|
||||
if let Some(config_folder) = custom_mcp_tool_approval_config_folder(config, server).await? {
|
||||
return persist_custom_mcp_tool_approval_at(&config_folder, server, tool_name).await;
|
||||
if let Some(config_edits_builder) = custom_mcp_tool_approval_config_builder(config, server)? {
|
||||
return persist_custom_mcp_tool_approval_with(config_edits_builder, server, tool_name)
|
||||
.await;
|
||||
}
|
||||
|
||||
let plugin_config_name = sess
|
||||
@@ -2080,7 +2080,7 @@ async fn persist_non_app_mcp_tool_approval(
|
||||
.map(|plugin| plugin.config_name.clone());
|
||||
|
||||
if let Some(plugin_config_name) = plugin_config_name {
|
||||
return ConfigEditsBuilder::new(&config.codex_home)
|
||||
return ConfigEditsBuilder::for_config(config)
|
||||
.with_edits([ConfigEdit::SetPath {
|
||||
segments: vec![
|
||||
"plugins".to_string(),
|
||||
@@ -2100,26 +2100,24 @@ async fn persist_non_app_mcp_tool_approval(
|
||||
anyhow::bail!("MCP server `{server}` is not configured in config.toml or an enabled plugin")
|
||||
}
|
||||
|
||||
async fn custom_mcp_tool_approval_config_folder(
|
||||
fn custom_mcp_tool_approval_config_builder(
|
||||
config: &Config,
|
||||
server: &str,
|
||||
) -> anyhow::Result<Option<AbsolutePathBuf>> {
|
||||
) -> anyhow::Result<Option<ConfigEditsBuilder>> {
|
||||
if let Some(project_config_folder) = project_mcp_tool_approval_config_folder(config, server) {
|
||||
return Ok(Some(project_config_folder));
|
||||
return Ok(Some(ConfigEditsBuilder::new(&project_config_folder)));
|
||||
}
|
||||
|
||||
let servers = load_global_mcp_servers(&config.codex_home).await?;
|
||||
Ok(servers
|
||||
.contains_key(server)
|
||||
.then(|| config.codex_home.clone()))
|
||||
Ok(user_mcp_server_is_configured(config, server)?
|
||||
.then(|| ConfigEditsBuilder::for_config(config)))
|
||||
}
|
||||
|
||||
async fn persist_custom_mcp_tool_approval_at(
|
||||
config_folder: &AbsolutePathBuf,
|
||||
async fn persist_custom_mcp_tool_approval_with(
|
||||
config_edits_builder: ConfigEditsBuilder,
|
||||
server: &str,
|
||||
tool_name: &str,
|
||||
) -> anyhow::Result<()> {
|
||||
ConfigEditsBuilder::new(config_folder)
|
||||
config_edits_builder
|
||||
.with_edits([ConfigEdit::SetPath {
|
||||
segments: vec![
|
||||
"mcp_servers".to_string(),
|
||||
@@ -2134,6 +2132,21 @@ async fn persist_custom_mcp_tool_approval_at(
|
||||
.await
|
||||
}
|
||||
|
||||
fn user_mcp_server_is_configured(config: &Config, server: &str) -> anyhow::Result<bool> {
|
||||
let Some(mcp_servers_toml) = config
|
||||
.config_layer_stack
|
||||
.effective_user_config()
|
||||
.as_ref()
|
||||
.and_then(|user_config| user_config.get("mcp_servers"))
|
||||
.cloned()
|
||||
else {
|
||||
return Ok(false);
|
||||
};
|
||||
let servers =
|
||||
HashMap::<String, codex_config::types::McpServerConfig>::deserialize(mcp_servers_toml)?;
|
||||
Ok(servers.contains_key(server))
|
||||
}
|
||||
|
||||
fn project_mcp_tool_approval_config_folder(
|
||||
config: &Config,
|
||||
server: &str,
|
||||
|
||||
@@ -30,7 +30,6 @@ use codex_rollout_trace::ToolDispatchInvocation;
|
||||
use codex_rollout_trace::ToolDispatchPayload;
|
||||
use codex_rollout_trace::ToolDispatchRequester;
|
||||
use codex_rollout_trace::replay_bundle;
|
||||
use core_test_support::PathExt;
|
||||
use core_test_support::hooks::trusted_config_layer_stack;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
@@ -1874,8 +1873,13 @@ fn accepted_elicitation_without_content_defaults_to_accept() {
|
||||
#[tokio::test]
|
||||
async fn persist_codex_app_tool_approval_writes_tool_override() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(tmp.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("load config");
|
||||
|
||||
persist_codex_app_tool_approval(&tmp.path().abs(), "calendar", "calendar/list_events")
|
||||
persist_codex_app_tool_approval(&config, "calendar", "calendar/list_events")
|
||||
.await
|
||||
.expect("persist approval");
|
||||
|
||||
@@ -2116,7 +2120,7 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn maybe_persist_mcp_tool_approval_reloads_session_config_for_custom_server() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let (session, mut turn_context) = make_session_and_context().await;
|
||||
let codex_home = session.codex_home().await;
|
||||
std::fs::create_dir_all(&codex_home).expect("create codex home");
|
||||
std::fs::write(
|
||||
@@ -2124,6 +2128,12 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config_for_custom_serve
|
||||
"[mcp_servers.docs]\ncommand = \"docs-server\"\n",
|
||||
)
|
||||
.expect("seed config");
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.clone().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("load config");
|
||||
turn_context.config = Arc::new(config);
|
||||
let key = McpToolApprovalKey {
|
||||
server: "docs".to_string(),
|
||||
connector_id: None,
|
||||
|
||||
@@ -90,7 +90,7 @@ fn collect_layer_mtimes(stack: &ConfigLayerStack) -> Vec<LayerMtime> {
|
||||
.filter_map(|layer| {
|
||||
let path = match &layer.name {
|
||||
ConfigLayerSource::System { file } => Some(file.clone()),
|
||||
ConfigLayerSource::User { file } => Some(file.clone()),
|
||||
ConfigLayerSource::User { file, .. } => Some(file.clone()),
|
||||
ConfigLayerSource::Project { dot_codex_folder } => {
|
||||
Some(dot_codex_folder.join(CONFIG_TOML_FILE))
|
||||
}
|
||||
|
||||
@@ -179,6 +179,8 @@ use crate::context_manager::ContextManager;
|
||||
use crate::context_manager::TotalTokenUsageBreakdown;
|
||||
use crate::thread_rollout_truncation::initial_history_has_prior_user_turns;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use codex_config::ConfigLayerSource;
|
||||
use codex_config::ConfigLayerStackOrdering;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
||||
@@ -1479,37 +1481,62 @@ impl Session {
|
||||
//
|
||||
// Prefer `refresh_runtime_config()` when the host can already provide a materialized
|
||||
// config snapshot. This file-based path exists for legacy local reload flows.
|
||||
let config_toml_path = {
|
||||
let config_toml_paths = {
|
||||
let state = self.state.lock().await;
|
||||
state
|
||||
.session_configuration
|
||||
.codex_home
|
||||
.join(CONFIG_TOML_FILE)
|
||||
let config = &state.session_configuration.original_config_do_not_use;
|
||||
let user_config_paths = config
|
||||
.config_layer_stack
|
||||
.get_user_layers(
|
||||
ConfigLayerStackOrdering::LowestPrecedenceFirst,
|
||||
/*include_disabled*/ true,
|
||||
)
|
||||
.into_iter()
|
||||
.filter_map(|layer| match &layer.name {
|
||||
ConfigLayerSource::User { file, .. } => Some(file.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
if user_config_paths.is_empty() {
|
||||
vec![
|
||||
state
|
||||
.session_configuration
|
||||
.codex_home
|
||||
.join(CONFIG_TOML_FILE),
|
||||
]
|
||||
} else {
|
||||
user_config_paths
|
||||
}
|
||||
};
|
||||
|
||||
let user_config = match std::fs::read_to_string(&config_toml_path) {
|
||||
Ok(contents) => match toml::from_str::<toml::Value>(&contents) {
|
||||
Ok(config) => config,
|
||||
let mut reloaded_user_configs = Vec::with_capacity(config_toml_paths.len());
|
||||
for config_toml_path in config_toml_paths {
|
||||
let user_config = match std::fs::read_to_string(&config_toml_path) {
|
||||
Ok(contents) => match toml::from_str::<toml::Value>(&contents) {
|
||||
Ok(config) => config,
|
||||
Err(err) => {
|
||||
warn!("failed to parse user config while reloading layer: {err}");
|
||||
return;
|
||||
}
|
||||
},
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
|
||||
toml::Value::Table(Default::default())
|
||||
}
|
||||
Err(err) => {
|
||||
warn!("failed to parse user config while reloading layer: {err}");
|
||||
warn!("failed to read user config while reloading layer: {err}");
|
||||
return;
|
||||
}
|
||||
},
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
|
||||
toml::Value::Table(Default::default())
|
||||
}
|
||||
Err(err) => {
|
||||
warn!("failed to read user config while reloading layer: {err}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
};
|
||||
reloaded_user_configs.push((config_toml_path, user_config));
|
||||
}
|
||||
|
||||
let next_config = {
|
||||
let state = self.state.lock().await;
|
||||
let mut config = (*state.session_configuration.original_config_do_not_use).clone();
|
||||
config.config_layer_stack = config
|
||||
.config_layer_stack
|
||||
.with_user_config(&config_toml_path, user_config);
|
||||
for (config_toml_path, user_config) in reloaded_user_configs {
|
||||
config.config_layer_stack = config
|
||||
.config_layer_stack
|
||||
.with_user_config(&config_toml_path, user_config);
|
||||
}
|
||||
config.tool_suggest =
|
||||
resolve_tool_suggest_config_from_layer_stack(&config.config_layer_stack);
|
||||
config
|
||||
|
||||
@@ -12,6 +12,7 @@ use crate::test_support::models_manager_with_provider;
|
||||
use crate::tools::format_exec_output_str;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::ConfigLayerStackOrdering;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_config::NetworkConstraints;
|
||||
use codex_config::NetworkDomainPermissionToml;
|
||||
use codex_config::NetworkDomainPermissionsToml;
|
||||
@@ -1210,6 +1211,70 @@ async fn reload_user_config_layer_updates_effective_apps_config() {
|
||||
assert_eq!(app.destructive_enabled, Some(false));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reload_user_config_layer_updates_base_and_selected_profile_layers() {
|
||||
let (session, _turn_context) = make_session_and_context().await;
|
||||
let codex_home = session.codex_home().await;
|
||||
std::fs::create_dir_all(&codex_home).expect("create codex home");
|
||||
let base_config_path = codex_home.join(CONFIG_TOML_FILE);
|
||||
let profile_config_path = codex_home.join("work.config.toml");
|
||||
std::fs::write(
|
||||
&base_config_path,
|
||||
"model = \"base\"\napproval_policy = \"on-failure\"\n",
|
||||
)
|
||||
.expect("write base user config");
|
||||
std::fs::write(&profile_config_path, "model = \"profile-old\"\n")
|
||||
.expect("write profile user config");
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.to_path_buf())
|
||||
.loader_overrides(LoaderOverrides {
|
||||
user_config_path: Some(profile_config_path.abs()),
|
||||
user_config_profile: Some("work".parse().expect("profile-v2 name")),
|
||||
..LoaderOverrides::without_managed_config_for_tests()
|
||||
})
|
||||
.build()
|
||||
.await
|
||||
.expect("load profile config");
|
||||
{
|
||||
let mut state = session.state.lock().await;
|
||||
state.session_configuration.original_config_do_not_use = Arc::new(config);
|
||||
}
|
||||
std::fs::write(
|
||||
&base_config_path,
|
||||
"model = \"base\"\napproval_policy = \"never\"\n",
|
||||
)
|
||||
.expect("update base user config");
|
||||
std::fs::write(&profile_config_path, "model = \"profile-new\"\n")
|
||||
.expect("update profile user config");
|
||||
|
||||
session.reload_user_config_layer().await;
|
||||
|
||||
let config = session.get_config().await;
|
||||
assert_eq!(
|
||||
config
|
||||
.config_layer_stack
|
||||
.get_user_config_file()
|
||||
.map(codex_utils_absolute_path::AbsolutePathBuf::as_path),
|
||||
Some(profile_config_path.as_path())
|
||||
);
|
||||
let effective_user_config = config
|
||||
.config_layer_stack
|
||||
.effective_user_config()
|
||||
.expect("merged user config");
|
||||
assert_eq!(
|
||||
effective_user_config
|
||||
.get("model")
|
||||
.and_then(toml::Value::as_str),
|
||||
Some("profile-new")
|
||||
);
|
||||
assert_eq!(
|
||||
effective_user_config
|
||||
.get("approval_policy")
|
||||
.and_then(toml::Value::as_str),
|
||||
Some("never")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn reload_user_config_layer_refreshes_hooks() -> anyhow::Result<()> {
|
||||
let session = make_session_with_config(|config| {
|
||||
|
||||
Reference in New Issue
Block a user