fix serde/parsing of command line configs to remove ambiguity

codex-rs/cli/src/mcp_cmd.rs

@@ -77,12 +77,20 @@ pub struct AddArgs {
     /// Name for the MCP server configuration.
     pub name: String,
 
-    /// Environment variables to set when launching the server.
-    #[arg(long, value_parser = parse_env_pair, value_name = "KEY=VALUE")]
+    /// Environment variables to set when launching a stdio server.
+    #[arg(long, value_parser = parse_env_pair, value_name = "KEY=VALUE", conflicts_with = "url")]
     pub env: Vec<(String, String)>,
 
-    /// Command to launch the MCP server.
-    #[arg(trailing_var_arg = true, num_args = 1..)]
+    /// URL for a streamable HTTP MCP server.
+    #[arg(long, value_name = "URL")]
+    pub url: Option<String>,
+
+    /// Optional bearer token for streamable HTTP servers.
+    #[arg(long, value_name = "TOKEN", requires = "url")]
+    pub bearer_token: Option<String>,
+
+    /// Command to launch a stdio MCP server.
+    #[arg(trailing_var_arg = true)]
     pub command: Vec<String>,
 }
 
@@ -140,39 +148,57 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
     // Validate any provided overrides even though they are not currently applied.
     config_overrides.parse_overrides().map_err(|e| anyhow!(e))?;
 
-    let AddArgs { name, env, command } = add_args;
+    let AddArgs {
+        name,
+        env,
+        url,
+        bearer_token,
+        command,
+    } = add_args;
 
     validate_server_name(&name)?;
 
-    let mut command_parts = command.into_iter();
-    let command_bin = command_parts
-        .next()
-        .ok_or_else(|| anyhow!("command is required"))?;
-    let command_args: Vec<String> = command_parts.collect();
-
-    let env_map = if env.is_empty() {
-        None
-    } else {
-        let mut map = HashMap::new();
-        for (key, value) in env {
-            map.insert(key, value);
-        }
-        Some(map)
-    };
-
     let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?;
     let mut servers = load_global_mcp_servers(&codex_home)
         .await
         .with_context(|| format!("failed to load MCP servers from {}", codex_home.display()))?;
 
-    let new_entry = McpServerConfig {
-        transport: McpServerTransportConfig::Stdio {
-            command: command_bin,
-            args: command_args,
-            env: env_map,
-        },
-        startup_timeout_sec: None,
-        tool_timeout_sec: None,
+    let new_entry = if let Some(url) = url {
+        if !command.is_empty() {
+            bail!("command arguments are not supported when --url is provided");
+        }
+
+        McpServerConfig {
+            transport: McpServerTransportConfig::StreamableHttp { url, bearer_token },
+            startup_timeout_sec: None,
+            tool_timeout_sec: None,
+        }
+    } else {
+        let mut command_parts = command.into_iter();
+        let command_bin = command_parts
+            .next()
+            .ok_or_else(|| anyhow!("command is required when --url is not provided"))?;
+        let command_args: Vec<String> = command_parts.collect();
+
+        let env_map = if env.is_empty() {
+            None
+        } else {
+            let mut map = HashMap::new();
+            for (key, value) in env {
+                map.insert(key, value);
+            }
+            Some(map)
+        };
+
+        McpServerConfig {
+            transport: McpServerTransportConfig::Stdio {
+                command: command_bin,
+                args: command_args,
+                env: env_map,
+            },
+            startup_timeout_sec: None,
+            tool_timeout_sec: None,
+        }
     };
 
     servers.insert(name.clone(), new_entry);

codex-rs/cli/tests/mcp_add_remove.rs

@@ -93,3 +93,106 @@ async fn add_with_env_preserves_key_order_and_values() -> Result<()> {
 
     Ok(())
 }
+
+#[tokio::test]
+async fn add_streamable_http_server_updates_global_config() -> Result<()> {
+    let codex_home = TempDir::new()?;
+
+    let mut add_cmd = codex_command(codex_home.path())?;
+    add_cmd
+        .args([
+            "mcp",
+            "add",
+            "remote",
+            "--url",
+            "http://127.0.0.1:1234/mcp",
+            "--bearer-token",
+            "token-123",
+        ])
+        .assert()
+        .success()
+        .stdout(contains("Added global MCP server 'remote'."));
+
+    let servers = load_global_mcp_servers(codex_home.path()).await?;
+    assert_eq!(servers.len(), 1);
+    let remote = servers.get("remote").expect("server should exist");
+    match &remote.transport {
+        McpServerTransportConfig::StreamableHttp { url, bearer_token } => {
+            assert_eq!(url, "http://127.0.0.1:1234/mcp");
+            assert_eq!(bearer_token.as_deref(), Some("token-123"));
+        }
+        other => panic!("unexpected transport: {other:?}"),
+    }
+
+    Ok(())
+}
+
+#[tokio::test]
+async fn add_streamable_http_server_rejects_command_args() -> Result<()> {
+    let codex_home = TempDir::new()?;
+
+    let mut add_cmd = codex_command(codex_home.path())?;
+    add_cmd
+        .args([
+            "mcp",
+            "add",
+            "broken",
+            "--url",
+            "http://127.0.0.1:3845/mcp",
+            "--",
+            "python",
+            "server.py",
+        ])
+        .assert()
+        .failure()
+        .stderr(contains(
+            "command arguments are not supported when --url is provided",
+        ));
+
+    let servers = load_global_mcp_servers(codex_home.path()).await?;
+    assert!(servers.is_empty());
+
+    Ok(())
+}
+
+#[tokio::test]
+async fn add_streamable_http_server_rejects_env_flag() -> Result<()> {
+    let codex_home = TempDir::new()?;
+
+    let mut add_cmd = codex_command(codex_home.path())?;
+    add_cmd
+        .args([
+            "mcp",
+            "add",
+            "broken",
+            "--url",
+            "http://127.0.0.1:3945/mcp",
+            "--env",
+            "FOO=bar",
+        ])
+        .assert()
+        .failure()
+        .stderr(contains("cannot be used with '--env"));
+
+    let servers = load_global_mcp_servers(codex_home.path()).await?;
+    assert!(servers.is_empty());
+
+    Ok(())
+}
+
+#[tokio::test]
+async fn add_bearer_token_requires_url() -> Result<()> {
+    let codex_home = TempDir::new()?;
+
+    let mut add_cmd = codex_command(codex_home.path())?;
+    add_cmd
+        .args(["mcp", "add", "broken", "--bearer-token", "secret"])
+        .assert()
+        .failure()
+        .stderr(contains("required arguments were not provided"));
+
+    let servers = load_global_mcp_servers(codex_home.path()).await?;
+    assert!(servers.is_empty());
+
+    Ok(())
+}

codex-rs/core/src/config_types.rs

@@ -40,14 +40,8 @@ impl<'de> Deserialize<'de> for McpServerConfig {
     {
         #[derive(Deserialize)]
         struct RawMcpServerConfig {
-            command: Option<String>,
-            #[serde(default)]
-            args: Option<Vec<String>>,
-            #[serde(default)]
-            env: Option<HashMap<String, String>>,
-
-            url: Option<String>,
-            bearer_token: Option<String>,
+            #[serde(flatten)]
+            transport: RawMcpServerTransportConfig,
 
             #[serde(default)]
             startup_timeout_sec: Option<f64>,
@@ -57,6 +51,31 @@ impl<'de> Deserialize<'de> for McpServerConfig {
             tool_timeout_sec: Option<Duration>,
         }
 
+        #[derive(Deserialize)]
+        #[serde(untagged)]
+        enum RawMcpServerTransportConfig {
+            Stdio(RawMcpServerStdio),
+            StreamableHttp(RawMcpServerStreamableHttp),
+        }
+
+        #[derive(Deserialize)]
+        #[serde(deny_unknown_fields)]
+        struct RawMcpServerStdio {
+            command: String,
+            #[serde(default)]
+            args: Vec<String>,
+            #[serde(default)]
+            env: Option<HashMap<String, String>>,
+        }
+
+        #[derive(Deserialize)]
+        #[serde(deny_unknown_fields)]
+        struct RawMcpServerStreamableHttp {
+            url: String,
+            #[serde(default)]
+            bearer_token: Option<String>,
+        }
+
         let raw = RawMcpServerConfig::deserialize(deserializer)?;
 
         let startup_timeout_sec = match (raw.startup_timeout_sec, raw.startup_timeout_ms) {
@@ -68,49 +87,14 @@ impl<'de> Deserialize<'de> for McpServerConfig {
             (None, None) => None,
         };
 
-        fn throw_if_set<E, T>(transport: &str, field: &str, value: Option<&T>) -> Result<(), E>
-        where
-            E: SerdeError,
-        {
-            if value.is_none() {
-                return Ok(());
+        let transport = match raw.transport {
+            RawMcpServerTransportConfig::Stdio(RawMcpServerStdio { command, args, env }) => {
+                McpServerTransportConfig::Stdio { command, args, env }
             }
-            Err(E::custom(format!(
-                "{field} is not supported for {transport}",
-            )))
-        }
-
-        let transport = match raw {
-            RawMcpServerConfig {
-                command: Some(command),
-                args,
-                env,
+            RawMcpServerTransportConfig::StreamableHttp(RawMcpServerStreamableHttp {
                 url,
                 bearer_token,
-                ..
-            } => {
-                throw_if_set("stdio", "url", url.as_ref())?;
-                throw_if_set("stdio", "bearer_token", bearer_token.as_ref())?;
-                McpServerTransportConfig::Stdio {
-                    command,
-                    args: args.unwrap_or_default(),
-                    env,
-                }
-            }
-            RawMcpServerConfig {
-                url: Some(url),
-                bearer_token,
-                command,
-                args,
-                env,
-                ..
-            } => {
-                throw_if_set("streamable_http", "command", command.as_ref())?;
-                throw_if_set("streamable_http", "args", args.as_ref())?;
-                throw_if_set("streamable_http", "env", env.as_ref())?;
-                McpServerTransportConfig::StreamableHttp { url, bearer_token }
-            }
-            _ => return Err(SerdeError::custom("invalid transport")),
+            }) => McpServerTransportConfig::StreamableHttp { url, bearer_token },
         };
 
         Ok(Self {
@@ -541,6 +525,17 @@ mod tests {
         .expect_err("should reject command+url");
     }
 
+    #[test]
+    fn deserialize_rejects_args_for_http_transport() {
+        toml::from_str::<McpServerConfig>(
+            r#"
+            url = "https://example.com"
+            args = ["hello"]
+        "#,
+        )
+        .expect_err("should reject args for http transport");
+    }
+
     #[test]
     fn deserialize_rejects_env_for_http_transport() {
         toml::from_str::<McpServerConfig>(
@@ -552,6 +547,16 @@ mod tests {
         .expect_err("should reject env for http transport");
     }
 
+    #[test]
+    fn deserialize_rejects_bearer_token_without_url() {
+        toml::from_str::<McpServerConfig>(
+            r#"
+            bearer_token = "secret"
+        "#,
+        )
+        .expect_err("should reject bearer token without url");
+    }
+
     #[test]
     fn deserialize_rejects_bearer_token_for_stdio_transport() {
         toml::from_str::<McpServerConfig>(

docs/config.md

@@ -392,9 +392,12 @@ experimental_use_rmcp_client = true
 ### MCP CLI commands
 
 ```shell
-# Add a server (env can be repeated; `--` separates the launcher command)
+# Add a stdio server (env can be repeated; `--` separates the launcher command)
 codex mcp add docs -- docs-server --port 4000
 
+# Add a streamable HTTP server (requires experimental_use_rmcp_client = true)
+codex mcp add figma --url http://127.0.0.1:3845/mcp --bearer-token token
+
 # List configured servers (pretty table or JSON)
 codex mcp list
 codex mcp list --json