mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Preserve config overrides across CLI subcommand boundaries (#56022)
## Why When `-c` or `--config` appears at multiple command levels, Clap retains only the deepest scope's values, dropping earlier overrides. ## What changed Collect config overrides from each command scope in order while preserving typed child arguments and their existing precedence. Arguments forwarded after `--` remain unchanged. ## Testing Add regression coverage for overrides split across nested `mcp add` commands, forwarded `-c` arguments, and validation of an invalid override before `app-server` when another override follows it. GitOrigin-RevId: 40c64eda866497e2f59491a523c6777a8bfe5d5f
This commit is contained in:
@@ -0,0 +1,49 @@
|
||||
//! Preserve config overrides across command boundaries without changing typed child arguments.
|
||||
|
||||
use crate::MultitoolCli;
|
||||
use clap::Command;
|
||||
use clap::CommandFactory;
|
||||
use clap::Parser;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
use std::ffi::OsString;
|
||||
|
||||
pub(super) fn try_parse_from(
|
||||
args: impl IntoIterator<Item = impl Into<OsString>>,
|
||||
) -> Result<MultitoolCli, clap::Error> {
|
||||
let args: Vec<OsString> = args.into_iter().map(Into::into).collect();
|
||||
let mut cli = MultitoolCli::try_parse_from(&args)?;
|
||||
if cli.subcommand.is_none() || cli.config_overrides.raw_overrides.is_empty() {
|
||||
return Ok(cli);
|
||||
}
|
||||
|
||||
// Clap propagates the deepest global argument's values instead of appending
|
||||
// values from each scope. Reparse with local config arguments to recover them.
|
||||
let matches = localize_config_arg(MultitoolCli::command()).try_get_matches_from(args)?;
|
||||
|
||||
// Keep child copies unchanged: existing dispatch merges apply them after
|
||||
// generated feature overrides, and clearing them would change precedence.
|
||||
cli.config_overrides.raw_overrides = std::iter::successors(Some(&matches), |scope| {
|
||||
scope.subcommand().map(|(_, child)| child)
|
||||
})
|
||||
.filter_map(|scope| scope.get_many::<String>("raw_overrides"))
|
||||
.flatten()
|
||||
.cloned()
|
||||
.collect();
|
||||
Ok(cli)
|
||||
}
|
||||
|
||||
fn localize_config_arg(mut command: Command) -> Command {
|
||||
if !command
|
||||
.get_arguments()
|
||||
.any(|arg| arg.get_id() == "raw_overrides")
|
||||
{
|
||||
command = command.args(CliConfigOverrides::command().get_arguments().cloned());
|
||||
}
|
||||
command
|
||||
.mut_arg("raw_overrides", |arg| arg.global(false))
|
||||
.mut_subcommands(localize_config_arg)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "config_override_args_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,37 @@
|
||||
//! Regression coverage for overrides split across CLI command boundaries.
|
||||
|
||||
use super::try_parse_from;
|
||||
use crate::Subcommand;
|
||||
use crate::mcp_cmd::McpSubcommand;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn preserves_scoped_overrides_and_forwarded_arguments() {
|
||||
let cli = try_parse_from([
|
||||
"codex",
|
||||
"--config=a=1",
|
||||
"mcp",
|
||||
"-cb=2",
|
||||
"add",
|
||||
"example",
|
||||
"-c",
|
||||
"a=3",
|
||||
"--",
|
||||
"program",
|
||||
"-c",
|
||||
"payload",
|
||||
])
|
||||
.expect("parse");
|
||||
assert_eq!(cli.config_overrides.raw_overrides, ["a=1", "b=2", "a=3"]);
|
||||
let Some(Subcommand::Mcp(mcp)) = cli.subcommand else {
|
||||
panic!("expected mcp");
|
||||
};
|
||||
assert_eq!(mcp.config_overrides.raw_overrides, ["a=3"]);
|
||||
let McpSubcommand::Add(add) = mcp.subcommand else {
|
||||
panic!("expected add");
|
||||
};
|
||||
assert_eq!(
|
||||
add.transport_args.stdio.expect("stdio transport").command,
|
||||
["program", "-c", "payload"],
|
||||
);
|
||||
}
|
||||
@@ -53,6 +53,7 @@ static ALLOCATOR: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
|
||||
#[cfg(any(target_os = "macos", target_os = "windows"))]
|
||||
mod app_cmd;
|
||||
mod cloud_config;
|
||||
mod config_override_args;
|
||||
mod daemon_install;
|
||||
mod daemon_telemetry;
|
||||
#[cfg(any(target_os = "macos", target_os = "windows"))]
|
||||
@@ -1059,7 +1060,8 @@ async fn cli_main(
|
||||
remote,
|
||||
mut interactive,
|
||||
subcommand,
|
||||
} = MultitoolCli::parse();
|
||||
} = config_override_args::try_parse_from(std::env::args_os())
|
||||
.unwrap_or_else(|error| error.exit());
|
||||
// Retain the launch target through TUI exit, even if a launcher changes selection.
|
||||
let daemon_cli_executable = arg0_paths
|
||||
.codex_self_exe
|
||||
|
||||
@@ -38,6 +38,24 @@ foo = "bar"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_server_preserves_config_overrides_across_subcommand() -> Result<()> {
|
||||
let home = TempDir::new()?;
|
||||
codex_command(home.path())?
|
||||
.args([
|
||||
"-c",
|
||||
"missing-equals",
|
||||
"app-server",
|
||||
"--print-startup-config",
|
||||
"-c",
|
||||
"model=example",
|
||||
])
|
||||
.assert()
|
||||
.failure()
|
||||
.stdout(contains("Invalid override (missing '='): missing-equals"));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agents_accept_interactive_configuration_overrides() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
Reference in New Issue
Block a user