mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
config: add strict config parsing (#20559)
## Why Codex intentionally ignores unknown `config.toml` fields by default so older and newer config files keep working across versions. That leniency also makes typo detection hard because misspelled or misplaced keys disappear silently. This change adds an opt-in strict config mode so users and tooling can fail fast on unrecognized config fields without changing the default permissive behavior. This feature is possible because `serde_ignored` exposes the exact signal Codex needs: it lets Codex run ordinary Serde deserialization while recording fields Serde would otherwise ignore. That avoids requiring `#[serde(deny_unknown_fields)]` across every config type and keeps strict validation opt-in around the existing config model. ## What Changed ### Added strict config validation - Added `serde_ignored`-based validation for `ConfigToml` in `codex-rs/config/src/strict_config.rs`. - Combined `serde_ignored` with `serde_path_to_error` so strict mode preserves typed config error paths while also collecting fields Serde would otherwise ignore. - Added strict-mode validation for unknown `[features]` keys, including keys that would otherwise be accepted by `FeaturesToml`'s flattened boolean map. - Kept typed config errors ahead of ignored-field reporting, so malformed known fields are reported before unknown-field diagnostics. - Added source-range diagnostics for top-level and nested unknown config fields, including non-file managed preference source names. ### Kept parsing single-pass per source - Reworked file and managed-config loading so strict validation reuses the already parsed `TomlValue` for that source. - For actual config files and managed config strings, the loader now reads once, parses once, and validates that same parsed value instead of deserializing multiple times. - Validated `-c` / `--config` override layers with the same base-directory context used for normal relative-path resolution, so unknown override keys are still reported when another override contains a relative path. ### Scoped `--strict-config` to config-heavy entry points - Added support for `--strict-config` on the main config-loading entry points where it is most useful: - `codex` - `codex resume` - `codex fork` - `codex exec` - `codex review` - `codex mcp-server` - `codex app-server` when running the server itself - the standalone `codex-app-server` binary - the standalone `codex-exec` binary - Commands outside that set now reject `--strict-config` early with targeted errors instead of accepting it everywhere through shared CLI plumbing. - `codex app-server` subcommands such as `proxy`, `daemon`, and `generate-*` are intentionally excluded from the first rollout. - When app-server strict mode sees invalid config, app-server exits with the config error instead of logging a warning and continuing with defaults. - Introduced a dedicated `ReviewCommand` wrapper in `codex-rs/cli` instead of extending shared `ReviewArgs`, so `--strict-config` stays on the outer config-loading command surface and does not become part of the reusable review payload used by `codex exec review`. ### Coverage - Added tests for top-level and nested unknown config fields, unknown `[features]` keys, typed-error precedence, source-location reporting, and non-file managed preference source names. - Added CLI coverage showing invalid `--enable`, invalid `--disable`, and unknown `-c` overrides still error when `--strict-config` is present, including compound-looking feature names such as `multi_agent_v2.subagent_usage_hint_text`. - Added integration coverage showing both `codex app-server --strict-config` and standalone `codex-app-server --strict-config` exit with an error for unknown config fields instead of starting with fallback defaults. - Added coverage showing unsupported command surfaces reject `--strict-config` with explicit errors. ## Example Usage Run Codex with strict config validation enabled: ```shell codex --strict-config ``` Strict config mode is also available on the supported config-heavy subcommands: ```shell codex --strict-config exec "explain this repository" codex review --strict-config --uncommitted codex mcp-server --strict-config codex app-server --strict-config --listen off codex-app-server --strict-config --listen off ``` For example, if `~/.codex/config.toml` contains a typo in a key name: ```toml model = "gpt-5" approval_polic = "on-request" ``` then `codex --strict-config` reports the misspelled key instead of silently ignoring it. The path is shortened to `~` here for readability: ```text $ codex --strict-config Error loading config.toml: ~/.codex/config.toml:2:1: unknown configuration field `approval_polic` | 2 | approval_polic = "on-request" | ^^^^^^^^^^^^^^ ``` Without `--strict-config`, Codex keeps the existing permissive behavior and ignores the unknown key. Strict config mode also validates ad-hoc `-c` / `--config` overrides: ```text $ codex --strict-config -c foo=bar Error: unknown configuration field `foo` in -c/--config override $ codex --strict-config -c features.foo=true Error: unknown configuration field `features.foo` in -c/--config override ``` Invalid feature toggles are rejected too, including values that look like nested config paths: ```text $ codex --strict-config --enable does_not_exist Error: Unknown feature flag: does_not_exist $ codex --strict-config --disable does_not_exist Error: Unknown feature flag: does_not_exist $ codex --strict-config --enable multi_agent_v2.subagent_usage_hint_text Error: Unknown feature flag: multi_agent_v2.subagent_usage_hint_text ``` Unsupported commands reject the flag explicitly: ```text $ codex --strict-config cloud list Error: `--strict-config` is not supported for `codex cloud` ``` ## Verification The `codex-cli` `strict_config` tests cover invalid `--enable`, invalid `--disable`, the compound `multi_agent_v2.subagent_usage_hint_text` case, unknown `-c` overrides, app-server strict startup failure through `codex app-server`, and rejection for unsupported commands such as `codex cloud`, `codex mcp`, `codex remote-control`, and `codex app-server proxy`. The config and config-loader tests cover unknown top-level fields, unknown nested fields, unknown `[features]` keys, source-location reporting, non-file managed config sources, and `-c` validation for keys such as `features.foo`. The app-server test suite covers standalone `codex-app-server --strict-config` startup failure for an unknown config field. ## Documentation The Codex CLI docs on developers.openai.com/codex should mention `--strict-config` as an opt-in validation mode for supported config-heavy entry points once this ships.
This commit is contained in:
committed by
GitHub
Unverified
parent
702e6a3c64
commit
889ee018e7
@@ -190,6 +190,7 @@ async fn run_command_under_sandbox(
|
||||
.map_err(anyhow::Error::msg)?,
|
||||
codex_linux_sandbox_exe,
|
||||
config_options,
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -636,12 +637,14 @@ async fn load_debug_sandbox_config(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
options: DebugSandboxConfigOptions,
|
||||
strict_config: bool,
|
||||
) -> anyhow::Result<Config> {
|
||||
load_debug_sandbox_config_with_codex_home(
|
||||
cli_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
options,
|
||||
/*codex_home*/ None,
|
||||
strict_config,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -651,6 +654,7 @@ async fn load_debug_sandbox_config_with_codex_home(
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
options: DebugSandboxConfigOptions,
|
||||
codex_home: Option<PathBuf>,
|
||||
strict_config: bool,
|
||||
) -> anyhow::Result<Config> {
|
||||
let DebugSandboxConfigOptions {
|
||||
permissions_profile,
|
||||
@@ -680,6 +684,7 @@ async fn load_debug_sandbox_config_with_codex_home(
|
||||
},
|
||||
codex_home.clone(),
|
||||
managed_requirements_mode,
|
||||
strict_config,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -697,6 +702,7 @@ async fn load_debug_sandbox_config_with_codex_home(
|
||||
},
|
||||
codex_home,
|
||||
managed_requirements_mode,
|
||||
strict_config,
|
||||
)
|
||||
.await
|
||||
.map_err(Into::into)
|
||||
@@ -707,14 +713,16 @@ async fn build_debug_sandbox_config(
|
||||
harness_overrides: ConfigOverrides,
|
||||
codex_home: Option<PathBuf>,
|
||||
managed_requirements_mode: ManagedRequirementsMode,
|
||||
strict_config: bool,
|
||||
) -> std::io::Result<Config> {
|
||||
let mut builder = ConfigBuilder::default()
|
||||
.cli_overrides(cli_overrides)
|
||||
.harness_overrides(harness_overrides);
|
||||
if let ManagedRequirementsMode::Ignore = managed_requirements_mode {
|
||||
.harness_overrides(harness_overrides)
|
||||
.strict_config(strict_config);
|
||||
if matches!(managed_requirements_mode, ManagedRequirementsMode::Ignore) {
|
||||
builder = builder.loader_overrides(LoaderOverrides {
|
||||
ignore_managed_requirements: true,
|
||||
..Default::default()
|
||||
..LoaderOverrides::default()
|
||||
});
|
||||
}
|
||||
if let Some(codex_home) = codex_home {
|
||||
@@ -783,6 +791,7 @@ mod tests {
|
||||
ConfigOverrides::default(),
|
||||
Some(codex_home_path.clone()),
|
||||
ManagedRequirementsMode::Include,
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
let legacy_config = build_debug_sandbox_config(
|
||||
@@ -793,6 +802,7 @@ mod tests {
|
||||
},
|
||||
Some(codex_home_path.clone()),
|
||||
ManagedRequirementsMode::Include,
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -805,6 +815,7 @@ mod tests {
|
||||
managed_requirements_mode: ManagedRequirementsMode::Include,
|
||||
},
|
||||
Some(codex_home_path),
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -840,6 +851,7 @@ mod tests {
|
||||
ConfigOverrides::default(),
|
||||
Some(codex_home_path.clone()),
|
||||
ManagedRequirementsMode::Include,
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
let read_only_config = build_debug_sandbox_config(
|
||||
@@ -850,6 +862,7 @@ mod tests {
|
||||
},
|
||||
Some(codex_home_path.clone()),
|
||||
ManagedRequirementsMode::Include,
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -862,6 +875,7 @@ mod tests {
|
||||
managed_requirements_mode: ManagedRequirementsMode::Include,
|
||||
},
|
||||
Some(codex_home_path),
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -905,6 +919,7 @@ mod tests {
|
||||
},
|
||||
Some(codex_home_path.clone()),
|
||||
ManagedRequirementsMode::Include,
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -917,6 +932,7 @@ mod tests {
|
||||
managed_requirements_mode: ManagedRequirementsMode::Include,
|
||||
},
|
||||
Some(codex_home_path),
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -942,6 +958,7 @@ mod tests {
|
||||
managed_requirements_mode: ManagedRequirementsMode::Ignore,
|
||||
},
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -975,6 +992,7 @@ mod tests {
|
||||
managed_requirements_mode: ManagedRequirementsMode::Ignore,
|
||||
},
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1004,6 +1022,7 @@ mod tests {
|
||||
managed_requirements_mode: ManagedRequirementsMode::Ignore,
|
||||
},
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1015,6 +1034,7 @@ mod tests {
|
||||
ConfigOverrides::default(),
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
ManagedRequirementsMode::Include,
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -1040,6 +1060,7 @@ mod tests {
|
||||
managed_requirements_mode: ManagedRequirementsMode::Ignore,
|
||||
},
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
/*strict_config*/ false,
|
||||
)
|
||||
.await?;
|
||||
|
||||
|
||||
+267
-22
@@ -55,7 +55,7 @@ use crate::marketplace_cmd::MarketplaceCli;
|
||||
use crate::mcp_cmd::McpCli;
|
||||
|
||||
use codex_core::build_models_manager;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_core::config::ConfigOverrides;
|
||||
use codex_core::config::edit::ConfigEditsBuilder;
|
||||
use codex_core::config::find_codex_home;
|
||||
@@ -109,7 +109,7 @@ enum Subcommand {
|
||||
Exec(ExecCli),
|
||||
|
||||
/// Run a code review non-interactively.
|
||||
Review(ReviewArgs),
|
||||
Review(ReviewCommand),
|
||||
|
||||
/// Manage login.
|
||||
Login(LoginCommand),
|
||||
@@ -124,7 +124,7 @@ enum Subcommand {
|
||||
Plugin(PluginCli),
|
||||
|
||||
/// Start Codex as an MCP server (stdio).
|
||||
McpServer,
|
||||
McpServer(McpServerCommand),
|
||||
|
||||
/// [experimental] Run the app server or related tooling.
|
||||
AppServer(AppServerCommand),
|
||||
@@ -266,6 +266,23 @@ struct DebugModelsCommand {
|
||||
bundled: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
struct ReviewCommand {
|
||||
/// Error out when config.toml contains fields that are not recognized by this version of Codex.
|
||||
#[arg(long = "strict-config", default_value_t = false)]
|
||||
strict_config: bool,
|
||||
|
||||
#[clap(flatten)]
|
||||
args: ReviewArgs,
|
||||
}
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
struct McpServerCommand {
|
||||
/// Error out when config.toml contains fields that are not recognized by this version of Codex.
|
||||
#[arg(long = "strict-config", default_value_t = false)]
|
||||
strict_config: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
struct DebugTraceReduceCommand {
|
||||
/// Trace bundle directory containing manifest.json and trace.jsonl.
|
||||
@@ -419,6 +436,10 @@ struct AppServerCommand {
|
||||
#[command(subcommand)]
|
||||
subcommand: Option<AppServerSubcommand>,
|
||||
|
||||
/// Error out when config.toml contains fields that are not recognized by this version of Codex.
|
||||
#[arg(long = "strict-config", default_value_t = false)]
|
||||
strict_config: bool,
|
||||
|
||||
/// Transport endpoint URL. Supported values: `stdio://` (default),
|
||||
/// `unix://`, `unix://PATH`, `ws://IP:PORT`, `off`.
|
||||
#[arg(
|
||||
@@ -810,6 +831,8 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
root_config_overrides.raw_overrides.extend(toggle_overrides);
|
||||
let root_remote = remote.remote;
|
||||
let root_remote_auth_token_env = remote.remote_auth_token_env;
|
||||
let root_strict_config = interactive.strict_config;
|
||||
reject_root_strict_config_for_subcommand(root_strict_config, &subcommand)?;
|
||||
|
||||
match subcommand {
|
||||
None => {
|
||||
@@ -835,13 +858,17 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
exec_cli
|
||||
.shared
|
||||
.inherit_exec_root_options(&interactive.shared);
|
||||
exec_cli.strict_config |= root_strict_config;
|
||||
prepend_config_flags(
|
||||
&mut exec_cli.config_overrides,
|
||||
root_config_overrides.clone(),
|
||||
);
|
||||
codex_exec::run_main(exec_cli, arg0_paths.clone()).await?;
|
||||
}
|
||||
Some(Subcommand::Review(review_args)) => {
|
||||
Some(Subcommand::Review(ReviewCommand {
|
||||
strict_config,
|
||||
args: review_args,
|
||||
})) => {
|
||||
reject_remote_mode_for_subcommand(
|
||||
root_remote.as_deref(),
|
||||
root_remote_auth_token_env.as_deref(),
|
||||
@@ -849,19 +876,25 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
)?;
|
||||
let mut exec_cli = ExecCli::try_parse_from(["codex", "exec"])?;
|
||||
exec_cli.command = Some(ExecCommand::Review(review_args));
|
||||
exec_cli.strict_config = strict_config || root_strict_config;
|
||||
prepend_config_flags(
|
||||
&mut exec_cli.config_overrides,
|
||||
root_config_overrides.clone(),
|
||||
);
|
||||
codex_exec::run_main(exec_cli, arg0_paths.clone()).await?;
|
||||
}
|
||||
Some(Subcommand::McpServer) => {
|
||||
Some(Subcommand::McpServer(McpServerCommand { strict_config })) => {
|
||||
reject_remote_mode_for_subcommand(
|
||||
root_remote.as_deref(),
|
||||
root_remote_auth_token_env.as_deref(),
|
||||
"mcp-server",
|
||||
)?;
|
||||
codex_mcp_server::run_main(arg0_paths.clone(), root_config_overrides).await?;
|
||||
codex_mcp_server::run_main(
|
||||
arg0_paths.clone(),
|
||||
root_config_overrides,
|
||||
strict_config || root_strict_config,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
Some(Subcommand::Mcp(mut mcp_cli)) => {
|
||||
reject_remote_mode_for_subcommand(
|
||||
@@ -894,11 +927,14 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
Some(Subcommand::AppServer(app_server_cli)) => {
|
||||
let AppServerCommand {
|
||||
subcommand,
|
||||
strict_config: app_server_strict_config,
|
||||
listen,
|
||||
remote_control,
|
||||
analytics_default_enabled,
|
||||
auth,
|
||||
} = app_server_cli;
|
||||
let strict_config = app_server_strict_config || root_strict_config;
|
||||
reject_strict_config_for_app_server_subcommand(strict_config, subcommand.as_ref())?;
|
||||
reject_remote_mode_for_app_server_subcommand(
|
||||
root_remote.as_deref(),
|
||||
root_remote_auth_token_env.as_deref(),
|
||||
@@ -916,6 +952,7 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
arg0_paths.clone(),
|
||||
root_config_overrides,
|
||||
codex_config::LoaderOverrides::default(),
|
||||
strict_config,
|
||||
analytics_default_enabled,
|
||||
transport,
|
||||
codex_protocol::protocol::SessionSource::VSCode,
|
||||
@@ -1320,11 +1357,11 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let config = Config::load_with_cli_overrides_and_harness_overrides(
|
||||
cli_kv_overrides,
|
||||
overrides,
|
||||
)
|
||||
.await?;
|
||||
let config = ConfigBuilder::default()
|
||||
.cli_overrides(cli_kv_overrides)
|
||||
.harness_overrides(overrides)
|
||||
.build()
|
||||
.await?;
|
||||
let mut rows = Vec::with_capacity(FEATURES.len());
|
||||
let mut name_width = 0;
|
||||
let mut stage_width = 0;
|
||||
@@ -1500,8 +1537,11 @@ async fn run_debug_prompt_input_command(
|
||||
additional_writable_roots: shared.add_dir,
|
||||
..Default::default()
|
||||
};
|
||||
let config =
|
||||
Config::load_with_cli_overrides_and_harness_overrides(cli_kv_overrides, overrides).await?;
|
||||
let config = ConfigBuilder::default()
|
||||
.cli_overrides(cli_kv_overrides)
|
||||
.harness_overrides(overrides)
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
let mut input = shared
|
||||
.images
|
||||
@@ -1532,7 +1572,10 @@ async fn run_debug_models_command(
|
||||
let cli_overrides = root_config_overrides
|
||||
.parse_overrides()
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
let config = Config::load_with_cli_overrides(cli_overrides).await?;
|
||||
let config = ConfigBuilder::default()
|
||||
.cli_overrides(cli_overrides)
|
||||
.build()
|
||||
.await?;
|
||||
let auth_manager =
|
||||
AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ true).await;
|
||||
let models_manager = build_models_manager(&config, auth_manager);
|
||||
@@ -1557,8 +1600,11 @@ async fn run_debug_clear_memories_command(
|
||||
config_profile: interactive.config_profile.clone(),
|
||||
..Default::default()
|
||||
};
|
||||
let config =
|
||||
Config::load_with_cli_overrides_and_harness_overrides(cli_kv_overrides, overrides).await?;
|
||||
let config = ConfigBuilder::default()
|
||||
.cli_overrides(cli_kv_overrides)
|
||||
.harness_overrides(overrides)
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
let state_path = state_db_path(config.sqlite_home.as_path());
|
||||
let mut cleared_state_db = false;
|
||||
@@ -1593,9 +1639,7 @@ fn prepend_config_flags(
|
||||
subcommand_config_overrides: &mut CliConfigOverrides,
|
||||
cli_config_overrides: CliConfigOverrides,
|
||||
) {
|
||||
subcommand_config_overrides
|
||||
.raw_overrides
|
||||
.splice(0..0, cli_config_overrides.raw_overrides);
|
||||
subcommand_config_overrides.prepend_root_overrides(cli_config_overrides);
|
||||
}
|
||||
|
||||
fn reject_remote_mode_for_subcommand(
|
||||
@@ -1616,12 +1660,103 @@ fn reject_remote_mode_for_subcommand(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn reject_root_strict_config_for_subcommand(
|
||||
strict_config: bool,
|
||||
subcommand: &Option<Subcommand>,
|
||||
) -> anyhow::Result<()> {
|
||||
if !strict_config {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
match unsupported_subcommand_name_for_strict_config(subcommand) {
|
||||
Some(subcommand_name) => {
|
||||
reject_strict_config_for_unsupported_subcommand(strict_config, subcommand_name)
|
||||
}
|
||||
None => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Return the selected subcommand name when a root-level `--strict-config`
|
||||
/// flag should be rejected after parsing.
|
||||
///
|
||||
/// `--strict-config` is parsed on the root interactive CLI so commands like
|
||||
/// `codex --strict-config` continue to work for the TUI and for wrappers that
|
||||
/// forward root options into another command shape. Clap will still accept that
|
||||
/// root flag before the dispatcher knows which subcommand the user selected, so
|
||||
/// unsupported subcommands need an explicit post-parse reject path.
|
||||
///
|
||||
/// `Some(...)` returns the user-facing command name fragment to embed in the
|
||||
/// rejection error, such as `cloud` or `app-server proxy`. `None` means the
|
||||
/// selected command is allowed to inherit root `--strict-config`.
|
||||
fn unsupported_subcommand_name_for_strict_config(
|
||||
subcommand: &Option<Subcommand>,
|
||||
) -> Option<&'static str> {
|
||||
match subcommand {
|
||||
None
|
||||
| Some(Subcommand::Exec(_))
|
||||
| Some(Subcommand::Review(_))
|
||||
| Some(Subcommand::McpServer(_))
|
||||
| Some(Subcommand::Resume(_))
|
||||
| Some(Subcommand::Fork(_)) => None,
|
||||
Some(Subcommand::AppServer(app_server)) if app_server.subcommand.is_none() => None,
|
||||
Some(Subcommand::AppServer(app_server)) => {
|
||||
Some(app_server_subcommand_name(app_server.subcommand.as_ref()))
|
||||
}
|
||||
Some(Subcommand::RemoteControl) => Some("remote-control"),
|
||||
Some(Subcommand::Mcp(_)) => Some("mcp"),
|
||||
Some(Subcommand::Plugin(_)) => Some("plugin"),
|
||||
#[cfg(any(target_os = "macos", target_os = "windows"))]
|
||||
Some(Subcommand::App(_)) => Some("app"),
|
||||
Some(Subcommand::Login(_)) => Some("login"),
|
||||
Some(Subcommand::Logout(_)) => Some("logout"),
|
||||
Some(Subcommand::Completion(_)) => Some("completion"),
|
||||
Some(Subcommand::Update) => Some("update"),
|
||||
Some(Subcommand::Cloud(_)) => Some("cloud"),
|
||||
Some(Subcommand::Sandbox(_)) => Some("sandbox"),
|
||||
Some(Subcommand::Debug(_)) => Some("debug"),
|
||||
Some(Subcommand::Execpolicy(_)) => Some("execpolicy"),
|
||||
Some(Subcommand::Apply(_)) => Some("apply"),
|
||||
Some(Subcommand::ResponsesApiProxy(_)) => Some("responses-api-proxy"),
|
||||
Some(Subcommand::StdioToUds(_)) => Some("stdio-to-uds"),
|
||||
Some(Subcommand::ExecServer(_)) => Some("exec-server"),
|
||||
Some(Subcommand::Features(_)) => Some("features"),
|
||||
}
|
||||
}
|
||||
|
||||
fn reject_strict_config_for_app_server_subcommand(
|
||||
strict_config: bool,
|
||||
subcommand: Option<&AppServerSubcommand>,
|
||||
) -> anyhow::Result<()> {
|
||||
if subcommand.is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
reject_strict_config_for_unsupported_subcommand(
|
||||
strict_config,
|
||||
app_server_subcommand_name(subcommand),
|
||||
)
|
||||
}
|
||||
|
||||
fn reject_strict_config_for_unsupported_subcommand(
|
||||
strict_config: bool,
|
||||
subcommand: &str,
|
||||
) -> anyhow::Result<()> {
|
||||
if strict_config {
|
||||
anyhow::bail!("`--strict-config` is not supported for `codex {subcommand}`");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn reject_remote_mode_for_app_server_subcommand(
|
||||
remote: Option<&str>,
|
||||
remote_auth_token_env: Option<&str>,
|
||||
subcommand: Option<&AppServerSubcommand>,
|
||||
) -> anyhow::Result<()> {
|
||||
let subcommand_name = match subcommand {
|
||||
let subcommand_name = app_server_subcommand_name(subcommand);
|
||||
reject_remote_mode_for_subcommand(remote, remote_auth_token_env, subcommand_name)
|
||||
}
|
||||
|
||||
fn app_server_subcommand_name(subcommand: Option<&AppServerSubcommand>) -> &'static str {
|
||||
match subcommand {
|
||||
None => "app-server",
|
||||
Some(AppServerSubcommand::Daemon(daemon)) => match daemon.subcommand {
|
||||
AppServerDaemonSubcommand::Bootstrap(_) => "app-server daemon bootstrap",
|
||||
@@ -1643,8 +1778,7 @@ fn reject_remote_mode_for_app_server_subcommand(
|
||||
Some(AppServerSubcommand::GenerateInternalJsonSchema(_)) => {
|
||||
"app-server generate-internal-json-schema"
|
||||
}
|
||||
};
|
||||
reject_remote_mode_for_subcommand(remote, remote_auth_token_env, subcommand_name)
|
||||
}
|
||||
}
|
||||
|
||||
async fn print_app_server_daemon_output(command: AppServerLifecycleCommand) -> anyhow::Result<()> {
|
||||
@@ -1816,6 +1950,7 @@ fn finalize_fork_interactive(
|
||||
fn merge_interactive_cli_flags(interactive: &mut TuiCli, subcommand_cli: TuiCli) {
|
||||
let TuiCli {
|
||||
shared,
|
||||
strict_config,
|
||||
approval_policy,
|
||||
web_search,
|
||||
prompt,
|
||||
@@ -1831,6 +1966,9 @@ fn merge_interactive_cli_flags(interactive: &mut TuiCli, subcommand_cli: TuiCli)
|
||||
if web_search {
|
||||
interactive.web_search = true;
|
||||
}
|
||||
if strict_config {
|
||||
interactive.strict_config = true;
|
||||
}
|
||||
if let Some(prompt) = prompt {
|
||||
// Normalize CRLF/CR to LF so CLI-provided text can't leak `\r` into TUI state.
|
||||
interactive.prompt = Some(prompt.replace("\r\n", "\n").replace('\r', "\n"));
|
||||
@@ -2318,6 +2456,7 @@ mod tests {
|
||||
"my-profile",
|
||||
"-C",
|
||||
"/tmp",
|
||||
"--strict-config",
|
||||
"-i",
|
||||
"/tmp/a.png,/tmp/b.png",
|
||||
]
|
||||
@@ -2340,6 +2479,7 @@ mod tests {
|
||||
Some(std::path::Path::new("/tmp"))
|
||||
);
|
||||
assert!(interactive.web_search);
|
||||
assert!(interactive.strict_config);
|
||||
let has_a = interactive
|
||||
.images
|
||||
.iter()
|
||||
@@ -2434,6 +2574,77 @@ mod tests {
|
||||
assert!(app_server.analytics_default_enabled);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_config_parses_for_supported_commands() {
|
||||
let cli = MultitoolCli::try_parse_from(["codex", "--strict-config"]).expect("parse");
|
||||
assert!(cli.interactive.strict_config);
|
||||
|
||||
let cli = MultitoolCli::try_parse_from(["codex", "mcp-server", "--strict-config"])
|
||||
.expect("parse");
|
||||
assert_matches!(
|
||||
cli.subcommand,
|
||||
Some(Subcommand::McpServer(McpServerCommand {
|
||||
strict_config: true,
|
||||
}))
|
||||
);
|
||||
|
||||
let cli =
|
||||
MultitoolCli::try_parse_from(["codex", "review", "--strict-config", "--uncommitted"])
|
||||
.expect("parse");
|
||||
assert_matches!(
|
||||
cli.subcommand,
|
||||
Some(Subcommand::Review(ReviewCommand {
|
||||
strict_config: true,
|
||||
..
|
||||
}))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn root_strict_config_is_rejected_for_unsupported_subcommands() {
|
||||
let cli = MultitoolCli::try_parse_from(["codex", "--strict-config", "mcp", "list"])
|
||||
.expect("parse");
|
||||
let err = reject_root_strict_config_for_subcommand(
|
||||
cli.interactive.strict_config,
|
||||
&cli.subcommand,
|
||||
)
|
||||
.expect_err("mcp should not support root --strict-config");
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"`--strict-config` is not supported for `codex mcp`"
|
||||
);
|
||||
|
||||
let cli = MultitoolCli::try_parse_from(["codex", "--strict-config", "remote-control"])
|
||||
.expect("parse");
|
||||
let err = reject_root_strict_config_for_subcommand(
|
||||
cli.interactive.strict_config,
|
||||
&cli.subcommand,
|
||||
)
|
||||
.expect_err("remote-control should not support root --strict-config");
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"`--strict-config` is not supported for `codex remote-control`"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_server_subcommands_reject_strict_config() {
|
||||
let app_server =
|
||||
app_server_from_args(["codex", "app-server", "--strict-config", "proxy"].as_ref());
|
||||
let err = reject_strict_config_for_app_server_subcommand(
|
||||
app_server.strict_config,
|
||||
app_server.subcommand.as_ref(),
|
||||
)
|
||||
.expect_err("app-server proxy should not support --strict-config");
|
||||
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"`--strict-config` is not supported for `codex app-server proxy`"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reject_remote_flag_for_remote_control() {
|
||||
let cli = MultitoolCli::try_parse_from(["codex", "--remote", "unix://", "remote-control"])
|
||||
@@ -2880,4 +3091,38 @@ mod tests {
|
||||
.expect_err("feature should be rejected");
|
||||
assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_config_with_unknown_enable_errors() {
|
||||
let err = strict_config_feature_toggle_error(["--enable", "does_not_exist"].as_ref());
|
||||
assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_config_with_unknown_disable_errors() {
|
||||
let err = strict_config_feature_toggle_error(["--disable", "does_not_exist"].as_ref());
|
||||
assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_config_with_compound_enable_errors() {
|
||||
let err = strict_config_feature_toggle_error(
|
||||
["--enable", "multi_agent_v2.subagent_usage_hint_text"].as_ref(),
|
||||
);
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"Unknown feature flag: multi_agent_v2.subagent_usage_hint_text"
|
||||
);
|
||||
}
|
||||
|
||||
fn strict_config_feature_toggle_error(args: &[&str]) -> anyhow::Error {
|
||||
let cli_args = std::iter::once("codex")
|
||||
.chain(std::iter::once("--strict-config"))
|
||||
.chain(args.iter().copied());
|
||||
let cli = MultitoolCli::try_parse_from(cli_args).expect("parse should succeed");
|
||||
assert!(cli.interactive.strict_config);
|
||||
cli.feature_toggles
|
||||
.to_overrides()
|
||||
.expect_err("feature should be rejected")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user