mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
Fix codex exec --fork clap panic by validating after parse (#13613)
### Motivation - A recent `--fork` addition used `conflicts_with = "command"` in the `clap` attribute which references a non-existent id and caused a runtime panic (`Argument or group 'command' ... does not exist`) during command construction, breaking CI and tooling that invokes `codex exec`. - The intent was to disallow `--fork` together with any subcommand; this must be enforced without using an invalid clap conflict target. ### Description - Removed the invalid `conflicts_with = "command"` attribute from the `--fork` flag in `codex-rs/exec/src/cli.rs` and instead added an explicit `Cli::validate()` method that returns a `clap::Error` with `ErrorKind::ArgumentConflict` when `fork_session_id` and a subcommand are both present. - Wired `Cli::validate()` into the binary entrypoint in `codex-rs/exec/src/main.rs` so the parsed CLI is validated before execution and the standard clap error handling is used (`err.exit()` on validation failure). - Updated the existing unit test to call `Cli::validate()` after parse so the test still asserts the intended conflict behavior. - Ran formatting (`just fmt`) after changes. ### Testing - Ran `just fmt` in `codex-rs` successfully. - Ran unit/integration tests for the modified crate with environment stubs: `PKG_CONFIG_PATH=/tmp/libcap-stub/lib/pkgconfig NO_PROXY=127.0.0.1,localhost no_proxy=127.0.0.1,localhost cargo test -p codex-exec` and `cargo test -p codex-exec --test all` which completed with the `codex-exec` tests passing locally. - Verified a targeted failing originator test after the change and it passed when run with the same env overrides. - Attempted to run `sdk/typescript` tests but the `pnpm`/`corepack` install failed due to network/proxy download errors (403 from proxy) when fetching `pnpm`, which is environmental and unrelated to this change. ------ [Codex Task](https://chatgpt.com/codex/tasks/task_i_69a9d1a216ec8323a648cfa001def5ae)
This commit is contained in:
@@ -15,7 +15,7 @@ pub struct Cli {
|
||||
/// Fork from an existing session id (or thread name) before sending the prompt.
|
||||
///
|
||||
/// This creates a new session with copied history, similar to `codex fork`.
|
||||
#[arg(long = "fork", value_name = "SESSION_ID", conflicts_with = "command")]
|
||||
#[arg(long = "fork", value_name = "SESSION_ID")]
|
||||
pub fork_session_id: Option<String>,
|
||||
|
||||
/// Optional image(s) to attach to the initial prompt.
|
||||
@@ -120,6 +120,19 @@ pub struct Cli {
|
||||
pub prompt: Option<String>,
|
||||
}
|
||||
|
||||
impl Cli {
|
||||
pub fn validate(self) -> Result<Self, clap::Error> {
|
||||
if self.fork_session_id.is_some() && self.command.is_some() {
|
||||
return Err(clap::Error::raw(
|
||||
clap::error::ErrorKind::ArgumentConflict,
|
||||
"--fork cannot be used with subcommands",
|
||||
));
|
||||
}
|
||||
|
||||
Ok(self)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, clap::Subcommand)]
|
||||
pub enum Command {
|
||||
/// Resume a previous session by id or pick the most recent with --last.
|
||||
@@ -335,6 +348,7 @@ mod tests {
|
||||
#[test]
|
||||
fn fork_option_conflicts_with_subcommands() {
|
||||
let err = Cli::try_parse_from(["codex-exec", "--fork", "session-123", "resume"])
|
||||
.and_then(Cli::validate)
|
||||
.expect_err("fork should conflict with subcommands");
|
||||
|
||||
assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict);
|
||||
|
||||
@@ -29,7 +29,10 @@ fn main() -> anyhow::Result<()> {
|
||||
arg0_dispatch_or_else(|arg0_paths: Arg0DispatchPaths| async move {
|
||||
let top_cli = TopCli::parse();
|
||||
// Merge root-level overrides into inner CLI struct so downstream logic remains unchanged.
|
||||
let mut inner = top_cli.inner;
|
||||
let mut inner = match top_cli.inner.validate() {
|
||||
Ok(inner) => inner,
|
||||
Err(err) => err.exit(),
|
||||
};
|
||||
inner
|
||||
.config_overrides
|
||||
.raw_overrides
|
||||
|
||||
Reference in New Issue
Block a user