From bae11269ebf784f5071ca32a9f75312e81b0864b Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 26 Oct 2026 19:10:40 +0000 Subject: [PATCH] Reject unsupported options for `codex queue` (#57273) ## What changed - Validate inherited options before and after the `queue` subcommand, with explicit errors for unsupported flags and positional prompts. - Reject `--model` and `-c model=...` because queued messages use the thread's model. - Allow `--cd` for local server startup, but reject it with `--remote`. - Hide unsupported inherited options from queue help and clarify the scope of startup configuration overrides. ## Testing Add regression coverage for unsupported options in both argument scopes, model configuration overrides, remote `--cd` conflicts, and queue help output. Preserve coverage for overrides that bypass a running local daemon. GitOrigin-RevId: 703ba26f38930a01559847e66e1af3d0d338d1e9 --- codex-rs/cli/src/config_override_args.rs | 8 ++- codex-rs/cli/src/main.rs | 17 +++++ codex-rs/cli/src/queue_cmd.rs | 87 +++++++++++++++++++++++- codex-rs/cli/tests/queue.rs | 54 ++++++++++----- 4 files changed, 145 insertions(+), 21 deletions(-) diff --git a/codex-rs/cli/src/config_override_args.rs b/codex-rs/cli/src/config_override_args.rs index 931df6c8f8..3046e2da22 100644 --- a/codex-rs/cli/src/config_override_args.rs +++ b/codex-rs/cli/src/config_override_args.rs @@ -3,7 +3,7 @@ use crate::MultitoolCli; use clap::Command; use clap::CommandFactory; -use clap::Parser; +use clap::FromArgMatches; use codex_utils_cli::CliConfigOverrides; use std::ffi::OsString; @@ -11,7 +11,11 @@ pub(super) fn try_parse_from( args: impl IntoIterator>, ) -> Result { let args: Vec = args.into_iter().map(Into::into).collect(); - let mut cli = MultitoolCli::try_parse_from(&args)?; + let mut command = MultitoolCli::command(); + let mut matches = command.try_get_matches_from_mut(&args)?; + crate::queue_cmd::validate_options(&command, &matches)?; + let mut cli = MultitoolCli::from_arg_matches_mut(&mut matches) + .map_err(|error| error.format(&mut command))?; if cli.subcommand.is_none() || cli.config_overrides.raw_overrides.is_empty() { return Ok(cli); } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index c22ebc3efd..7b167742ad 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -3561,6 +3561,23 @@ enabled = true err.to_string() } + #[test] + fn queue_help_lists_supported_options() { + let help = help_from_args(&["codex", "queue", "--help"]); + let (general, local) = help + .split_once("Local server options (without --remote):") + .unwrap(); + assert!(general.contains("--message ")); + assert!(!general.contains("--model")); + assert!(!general.contains("-c model=")); + let local_flags = local + .lines() + .filter(|line| line.starts_with(" -") || line.starts_with(" --")) + .map(str::trim) + .collect::>(); + insta::assert_snapshot!(local_flags.join("\n"), @"-C, --cd "); + } + #[test] fn tcp_tunnel_is_hidden_and_accepts_its_control_input_contract() { let root_help = help_from_args(&["codex", "--help"]); diff --git a/codex-rs/cli/src/queue_cmd.rs b/codex-rs/cli/src/queue_cmd.rs index 8a1b997f39..3a859afbc0 100644 --- a/codex-rs/cli/src/queue_cmd.rs +++ b/codex-rs/cli/src/queue_cmd.rs @@ -1,5 +1,11 @@ +use clap::Arg; +use clap::ArgMatches; +use clap::Command; +use clap::CommandFactory; use clap::Parser; use clap::builder::NonEmptyStringValueParser; +use clap::error::ErrorKind; +use clap::parser::ValueSource; use codex_arg0::Arg0DispatchPaths; use codex_tui::Cli as TuiCli; use codex_utils_cli::CliConfigOverrides; @@ -10,6 +16,7 @@ use crate::finalize_session_archive_interactive; use crate::resolve_remote_endpoint; #[derive(Debug, Parser)] +#[command(defer(configure_help))] pub(crate) struct QueueCommand { /// Session UUID or exact session name. #[arg(long, value_name = "THREAD")] @@ -31,6 +38,76 @@ pub(crate) struct QueueCommand { pub(crate) config_overrides: SessionArchiveConfigOverrides, } +/// Only inherited options need restrictions; queue owns its other arguments. +fn restricted_options(command: &Command) -> impl Iterator { + command.get_arguments().filter(|arg| { + !matches!( + arg.get_long(), + Some( + "remote" + | "remote-auth-token-env" + | "config" + | "enable" + | "disable" + | "profile" + | "strict-config" + ) + ) + }) +} + +fn configure_help(command: Command) -> Command { + let interactive = TuiCli::command(); + command.mut_args(|arg| { + if arg.get_long() == Some("config") { + arg.long_help(None).help("Override a startup configuration value (key=value). Queued messages use the thread's model.") + } else if !restricted_options(&interactive).any(|shared| shared.get_id() == arg.get_id()) { + arg + } else if arg.get_long() == Some("cd") { + arg.help_heading("Local server options (without --remote)") + } else { + arg.hide(true) + } + }) +} + +/// Check both scopes before root flags such as `--approve-for-me` become config overrides. +pub(super) fn validate_options(command: &Command, root: &ArgMatches) -> Result<(), clap::Error> { + let Some(queue) = root.subcommand_matches("queue") else { + return Ok(()); + }; + let remote = root + .get_one::("remote") + .or_else(|| queue.get_one::("remote")); + let supplied = [root, queue].into_iter().flat_map(|scope| { + scope + .ids() + .filter(|id| scope.value_source(id.as_str()) == Some(ValueSource::CommandLine)) + }); + for id in supplied { + let Some(arg) = restricted_options(command).find(|arg| arg.get_id() == id) else { + continue; + }; + let message = match arg.get_long() { + Some("cd") if remote.is_none() => continue, + Some("cd") => "`codex queue` does not support `--cd` with `--remote`".to_string(), + // These flags already have dedicated diagnostics in the CLI and session startup. + Some("worktree" | "no-daemon") => continue, + Some("model") => { + "`codex queue` does not support `--model`; queued messages use the thread's model" + .to_string() + } + Some("image") => "`codex queue` does not support image attachments".to_string(), + Some(name) => format!("`codex queue` does not support `--{name}`"), + None => { + "`codex queue` does not accept a positional prompt; use `--message`".to_string() + } + }; + return Err(clap::Error::raw(ErrorKind::ArgumentConflict, message)); + } + Ok(()) +} + pub(crate) async fn run_queue_command( command: QueueCommand, interactive: TuiCli, @@ -48,8 +125,14 @@ pub(crate) async fn run_queue_command( } = command; let cli = finalize_session_archive_interactive(interactive, root_config_overrides, config_overrides); - if !cli.images.is_empty() { - anyhow::bail!("`codex queue` does not support image attachments"); + if cli.config_overrides.raw_overrides.iter().any(|entry| { + entry + .split_once('=') + .is_some_and(|(key, _)| key.trim() == "model") + }) { + anyhow::bail!( + "`codex queue` does not support a `model` config override; queued messages use the thread's model" + ); } let explicit_remote_endpoint = resolve_remote_endpoint( remote.remote.or(root_remote), diff --git a/codex-rs/cli/tests/queue.rs b/codex-rs/cli/tests/queue.rs index 7d15124cb1..eff0d90da9 100644 --- a/codex-rs/cli/tests/queue.rs +++ b/codex-rs/cli/tests/queue.rs @@ -182,20 +182,39 @@ fn queue_rejects_empty_message() -> Result<()> { } #[test] -fn queue_rejects_image_attachments() -> Result<()> { - let output = std::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?) - .args([ - "queue", - "--thread", - THREAD_ID, - "--message", - "do the thing", - "--image", - "screenshot.png", - ]) - .output()?; - assert!(!output.status.success()); - assert!(String::from_utf8(output.stderr)?.contains("does not support image attachments")); +fn queue_rejects_unsupported_options() -> Result<()> { + let codex_home = TempDir::new()?; + for (args, expected) in [ + ("queue --image screenshot.png", "image attachments"), + ("queue --model other-model", "`--model`"), + ("--oss queue", "`--oss`"), + ("queue --local-provider ollama", "`--local-provider`"), + ( + "queue --dangerously-bypass-hook-trust", + "`--dangerously-bypass-hook-trust`", + ), + ( + "-c model=other queue -c analytics.enabled=false", + "`model` config override", + ), + ("queue -s read-only", "`--sandbox`"), + ("--approve-for-me queue", "`--approve-for-me`"), + ("prompt queue", "positional prompt; use `--message`"), + ("-C . queue --remote unix://", "`--cd` with `--remote`"), + ("--remote unix:// queue -C .", "`--cd` with `--remote`"), + ] { + let output = std::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?) + .env("CODEX_HOME", codex_home.path()) + .args(args.split_whitespace()) + .args(["--thread", THREAD_ID, "--message", "do the thing"]) + .output()?; + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr)?; + assert!( + stderr.contains("`codex queue`") && stderr.contains(expected), + "{args:?}: {stderr}" + ); + } Ok(()) } @@ -631,8 +650,10 @@ async fn queue_rejects_overrides_that_bypass_local_daemon() -> Result<()> { .env("CODEX_HOME", codex_home.path()) .args([ "queue", + "-C", + ".", "-c", - "model=\"test-model\"", + "analytics.enabled=false", "--thread", THREAD_ID, "--message", @@ -640,12 +661,11 @@ async fn queue_rejects_overrides_that_bypass_local_daemon() -> Result<()> { ]) .output() .await?; - server.await??; - assert!(!output.status.success()); assert!( String::from_utf8(output.stderr)? .contains("embedded app server while a local app-server daemon is running") ); + server.await??; Ok(()) }