Skip to content

[MCP] Add support for streamable http servers with codex mcp add and replace bearer token handling #4904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Oct 8, 2025
Merged

[MCP] Add support for streamable http servers with codex mcp add and replace bearer token handling #4904

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
83b898c
Added config
gpeal Oct 3, 2025
15f39d7
Works
gpeal Oct 3, 2025
29bd732
Add mode
gpeal Oct 3, 2025
256021d
Added config tests
gpeal Oct 6, 2025
29292e4
Cleanup
gpeal Oct 7, 2025
d238c88
Merge remote-tracking branch 'origin/main' into gpeal/oauth-store-config
gpeal Oct 7, 2025
fdc3171
Cleanup
gpeal Oct 7, 2025
0f21d19
Rename
gpeal Oct 7, 2025
8d94b18
Use env
gpeal Oct 7, 2025
6ba1947
Cleanup
gpeal Oct 7, 2025
198bd48
Cleanup
gpeal Oct 8, 2025
8a6d501
Merge remote-tracking branch 'origin/main' into gpeal/bearer-token-op…
gpeal Oct 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions codex-rs/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

166 changes: 122 additions & 44 deletions codex-rs/cli/src/mcp_cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use anyhow::Context;
use anyhow::Result;
use anyhow::anyhow;
use anyhow::bail;
use clap::ArgGroup;
use codex_common::CliConfigOverrides;
use codex_core::config::Config;
use codex_core::config::ConfigOverrides;
Expand Down Expand Up @@ -77,13 +78,61 @@ 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")]
pub env: Vec<(String, String)>,
#[command(flatten)]
pub transport_args: AddMcpTransportArgs,
}

#[derive(Debug, clap::Args)]
#[command(
Copy link
Collaborator

@bolinfest bolinfest Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right to me. Chat suggested:

#[derive(Debug, clap::Args)]
pub struct AddMcpTransportArgs {
    #[command(flatten)]
    #[group(required = true, multiple = false, name = "transport")]
    pub stdio: Option<AddMcpStdioArgs>,

    #[command(flatten)]
    #[group(name = "transport")]
    pub streamable_http: Option<AddMcpStreamableHttpArgs>,
}

Copy link
Collaborator Author

@gpeal gpeal Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clap is a bit finicky to work with but I wrote unit tests to verify the failure and this change breaks a couple of them.

group(
ArgGroup::new("transport")
.args(["command", "url"])
.required(true)
.multiple(false)
)
)]
pub struct AddMcpTransportArgs {
#[command(flatten)]
pub stdio: Option<AddMcpStdioArgs>,

#[command(flatten)]
pub streamable_http: Option<AddMcpStreamableHttpArgs>,
}

#[derive(Debug, clap::Args)]
pub struct AddMcpStdioArgs {
/// Command to launch the MCP server.
#[arg(trailing_var_arg = true, num_args = 1..)]
/// Use --url for a streamable HTTP server.
#[arg(
trailing_var_arg = true,
num_args = 0..,
)]
pub command: Vec<String>,

/// Environment variables to set when launching the server.
/// Only valid with stdio servers.
#[arg(
long,
value_parser = parse_env_pair,
value_name = "KEY=VALUE",
)]
pub env: Vec<(String, String)>,
}

#[derive(Debug, clap::Args)]
pub struct AddMcpStreamableHttpArgs {
/// URL for a streamable HTTP MCP server.
#[arg(long)]
pub url: String,

/// Optional environment variable to read for a bearer token.
/// Only valid with streamable HTTP servers.
#[arg(
long = "bearer-token-env-var",
value_name = "ENV_VAR",
requires = "url"
)]
pub bearer_token_env_var: Option<String>,
}

#[derive(Debug, clap::Parser)]
Expand Down Expand Up @@ -140,37 +189,55 @@ 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,
transport_args,
} = 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,
let transport = match transport_args {
AddMcpTransportArgs {
stdio: Some(stdio), ..
} => {
let mut command_parts = stdio.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 stdio.env.is_empty() {
None
} else {
let mut map = HashMap::new();
for (key, value) in stdio.env {
map.insert(key, value);
}
Some(map)
};
McpServerTransportConfig::Stdio {
command: command_bin,
args: command_args,
env: env_map,
}
}
AddMcpTransportArgs {
streamable_http: Some(streamable_http),
..
} => McpServerTransportConfig::StreamableHttp {
url: streamable_http.url,
bearer_token_env_var: streamable_http.bearer_token_env_var,
},
AddMcpTransportArgs { .. } => bail!("exactly one of --command or --url must be provided"),
};

let new_entry = McpServerConfig {
transport,
startup_timeout_sec: None,
tool_timeout_sec: None,
};
Expand Down Expand Up @@ -236,7 +303,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs)
_ => bail!("OAuth login is only supported for streamable HTTP servers."),
};

perform_oauth_login(&name, &url).await?;
perform_oauth_login(&name, &url, config.mcp_oauth_credentials_store_mode).await?;
println!("Successfully logged in to MCP server '{name}'.");
Ok(())
}
Expand All @@ -259,7 +326,7 @@ async fn run_logout(config_overrides: &CliConfigOverrides, logout_args: LogoutAr
_ => bail!("OAuth logout is only supported for streamable_http transports."),
};

match delete_oauth_tokens(&name, &url) {
match delete_oauth_tokens(&name, &url, config.mcp_oauth_credentials_store_mode) {
Ok(true) => println!("Removed OAuth credentials for '{name}'."),
Ok(false) => println!("No OAuth credentials stored for '{name}'."),
Err(err) => return Err(anyhow!("failed to delete OAuth credentials: {err}")),
Expand Down Expand Up @@ -288,11 +355,14 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
"args": args,
"env": env,
}),
McpServerTransportConfig::StreamableHttp { url, bearer_token } => {
McpServerTransportConfig::StreamableHttp {
url,
bearer_token_env_var,
} => {
serde_json::json!({
"type": "streamable_http",
"url": url,
"bearer_token": bearer_token,
"bearer_token_env_var": bearer_token_env_var,
})
}
};
Expand Down Expand Up @@ -345,13 +415,15 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
};
stdio_rows.push([name.clone(), command.clone(), args_display, env_display]);
}
McpServerTransportConfig::StreamableHttp { url, bearer_token } => {
let has_bearer = if bearer_token.is_some() {
"True"
} else {
"False"
};
http_rows.push([name.clone(), url.clone(), has_bearer.into()]);
McpServerTransportConfig::StreamableHttp {
url,
bearer_token_env_var,
} => {
http_rows.push([
name.clone(),
url.clone(),
bearer_token_env_var.clone().unwrap_or("-".to_string()),
]);
}
}
}
Expand Down Expand Up @@ -396,7 +468,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
}

if !http_rows.is_empty() {
let mut widths = ["Name".len(), "Url".len(), "Has Bearer Token".len()];
let mut widths = ["Name".len(), "Url".len(), "Bearer Token Env Var".len()];
for row in &http_rows {
for (i, cell) in row.iter().enumerate() {
widths[i] = widths[i].max(cell.len());
Expand All @@ -407,7 +479,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
"{:<name_w$} {:<url_w$} {:<token_w$}",
"Name",
"Url",
"Has Bearer Token",
"Bearer Token Env Var",
name_w = widths[0],
url_w = widths[1],
token_w = widths[2],
Expand Down Expand Up @@ -447,10 +519,13 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
"args": args,
"env": env,
}),
McpServerTransportConfig::StreamableHttp { url, bearer_token } => serde_json::json!({
McpServerTransportConfig::StreamableHttp {
url,
bearer_token_env_var,
} => serde_json::json!({
"type": "streamable_http",
"url": url,
"bearer_token": bearer_token,
"bearer_token_env_var": bearer_token_env_var,
}),
};
let output = serde_json::to_string_pretty(&serde_json::json!({
Expand Down Expand Up @@ -493,11 +568,14 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
};
println!(" env: {env_display}");
}
McpServerTransportConfig::StreamableHttp { url, bearer_token } => {
McpServerTransportConfig::StreamableHttp {
url,
bearer_token_env_var,
} => {
println!(" transport: streamable_http");
println!(" url: {url}");
let bearer = bearer_token.as_deref().unwrap_or("-");
println!(" bearer_token: {bearer}");
let env_var = bearer_token_env_var.as_deref().unwrap_or("-");
println!(" bearer_token_env_var: {env_var}");
}
}
if let Some(timeout) = server.startup_timeout_sec {
Expand Down
Loading
Loading