Skip to content

[MCP] Allow specifying cwd and additional env vars #5246

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
gpeal merged 6 commits into from
Oct 17, 2025
Merged

[MCP] Allow specifying cwd and additional env vars #5246

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
16c3867
Works
gpeal Oct 16, 2025
fb8c20e
Fix test
gpeal Oct 16, 2025
9ae6a49
Cleanup
gpeal Oct 16, 2025
cac65ac
Cleanup
gpeal Oct 16, 2025
dad9a47
Merge remote-tracking branch 'origin/main' into gpeal/cwd-and-env-stdio
gpeal Oct 17, 2025
284bf74
Cleanup
gpeal Oct 17, 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
1 change: 1 addition & 0 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.

102 changes: 63 additions & 39 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 @@ -6,6 +6,7 @@ use anyhow::anyhow;
use anyhow::bail;
use clap::ArgGroup;
use codex_common::CliConfigOverrides;
use codex_common::format_env_display::format_env_display;
use codex_core::config::Config;
use codex_core::config::ConfigOverrides;
use codex_core::config::find_codex_home;
Expand Down Expand Up @@ -227,6 +228,8 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
command: command_bin,
args: command_args,
env: env_map,
env_vars: Vec::new(),
cwd: None,
}
}
AddMcpTransportArgs {
Expand Down Expand Up @@ -377,11 +380,19 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
.copied()
.unwrap_or(McpAuthStatus::Unsupported);
let transport = match &cfg.transport {
McpServerTransportConfig::Stdio { command, args, env } => serde_json::json!({
McpServerTransportConfig::Stdio {
command,
args,
env,
env_vars,
cwd,
} => serde_json::json!({

@bolinfest bolinfest Oct 16, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why aren't we just using serde for this again? Because this is UI, I guess?

@gpeal gpeal Oct 16, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm just adding a couple of fields and didn't want to expand the scope of this PR to refactor all of the code around it since this PR is big enough already.

"type": "stdio",
"command": command,
"args": args,
"env": env,
"env_vars": env_vars,
"cwd": cwd,
}),
McpServerTransportConfig::StreamableHttp {
url,
Expand Down Expand Up @@ -419,30 +430,29 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
return Ok(());
}

let mut stdio_rows: Vec<[String; 6]> = Vec::new();
let mut stdio_rows: Vec<[String; 7]> = Vec::new();
let mut http_rows: Vec<[String; 5]> = Vec::new();

for (name, cfg) in entries {
match &cfg.transport {
McpServerTransportConfig::Stdio { command, args, env } => {
McpServerTransportConfig::Stdio {
command,
args,
env,
env_vars,
cwd,
} => {
let args_display = if args.is_empty() {
"-".to_string()
} else {
args.join(" ")
};
let env_display = match env.as_ref() {
None => "-".to_string(),
Some(map) if map.is_empty() => "-".to_string(),
Some(map) => {
let mut pairs: Vec<_> = map.iter().collect();
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
pairs
.into_iter()
.map(|(k, v)| format!("{k}={v}"))
.collect::<Vec<_>>()
.join(", ")
}
};
let env_display = format_env_display(env.as_ref(), env_vars);
let cwd_display = cwd
.as_ref()
.map(|path| path.display().to_string())
.filter(|value| !value.is_empty())
.unwrap_or_else(|| "-".to_string());
let status = if cfg.enabled {
"enabled".to_string()
} else {
Expand All @@ -458,6 +468,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
command.clone(),
args_display,
env_display,
cwd_display,
status,
auth_status,
]);
Expand Down Expand Up @@ -493,6 +504,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
"Command".len(),
"Args".len(),
"Env".len(),
"Cwd".len(),
"Status".len(),
"Auth".len(),
];
Expand All @@ -503,36 +515,40 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
}

println!(
"{name:<name_w$} {command:<cmd_w$} {args:<args_w$} {env:<env_w$} {status:<status_w$} {auth:<auth_w$}",
"{name:<name_w$} {command:<cmd_w$} {args:<args_w$} {env:<env_w$} {cwd:<cwd_w$} {status:<status_w$} {auth:<auth_w$}",
name = "Name",
command = "Command",
args = "Args",
env = "Env",
cwd = "Cwd",
status = "Status",
auth = "Auth",
name_w = widths[0],
cmd_w = widths[1],
args_w = widths[2],
env_w = widths[3],
status_w = widths[4],
auth_w = widths[5],
cwd_w = widths[4],
status_w = widths[5],
auth_w = widths[6],
);

for row in &stdio_rows {
println!(
"{name:<name_w$} {command:<cmd_w$} {args:<args_w$} {env:<env_w$} {status:<status_w$} {auth:<auth_w$}",
"{name:<name_w$} {command:<cmd_w$} {args:<args_w$} {env:<env_w$} {cwd:<cwd_w$} {status:<status_w$} {auth:<auth_w$}",
name = row[0].as_str(),
command = row[1].as_str(),
args = row[2].as_str(),
env = row[3].as_str(),
status = row[4].as_str(),
auth = row[5].as_str(),
cwd = row[4].as_str(),
status = row[5].as_str(),
auth = row[6].as_str(),
name_w = widths[0],
cmd_w = widths[1],
args_w = widths[2],
env_w = widths[3],
status_w = widths[4],
auth_w = widths[5],
cwd_w = widths[4],
status_w = widths[5],
auth_w = widths[6],
);
}
}
Expand Down Expand Up @@ -601,11 +617,19 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re

if get_args.json {
let transport = match &server.transport {
McpServerTransportConfig::Stdio { command, args, env } => serde_json::json!({
McpServerTransportConfig::Stdio {
command,
args,
env,
env_vars,
cwd,
} => serde_json::json!({
"type": "stdio",
"command": command,
"args": args,
"env": env,
"env_vars": env_vars,
"cwd": cwd,
}),
McpServerTransportConfig::StreamableHttp {
url,
Expand Down Expand Up @@ -634,7 +658,13 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
println!("{}", get_args.name);
println!(" enabled: {}", server.enabled);
match &server.transport {
McpServerTransportConfig::Stdio { command, args, env } => {
McpServerTransportConfig::Stdio {
command,
args,
env,
env_vars,
cwd,
} => {
println!(" transport: stdio");
println!(" command: {command}");
let args_display = if args.is_empty() {
Expand All @@ -643,19 +673,13 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
args.join(" ")
};
println!(" args: {args_display}");
let env_display = match env.as_ref() {
None => "-".to_string(),
Some(map) if map.is_empty() => "-".to_string(),
Some(map) => {
let mut pairs: Vec<_> = map.iter().collect();
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
pairs
.into_iter()
.map(|(k, v)| format!("{k}={v}"))
.collect::<Vec<_>>()
.join(", ")
}
};
let cwd_display = cwd
.as_ref()
.map(|path| path.display().to_string())
.filter(|value| !value.is_empty())
.unwrap_or_else(|| "-".to_string());
println!(" cwd: {cwd_display}");
let env_display = format_env_display(env.as_ref(), env_vars);
println!(" env: {env_display}");
}
McpServerTransportConfig::StreamableHttp {
Expand Down
10 changes: 9 additions & 1 deletion codex-rs/cli/tests/mcp_add_remove.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,18 @@ async fn add_and_remove_server_updates_global_config() -> Result<()> {
assert_eq!(servers.len(), 1);
let docs = servers.get("docs").expect("server should exist");
match &docs.transport {
McpServerTransportConfig::Stdio { command, args, env } => {
McpServerTransportConfig::Stdio {
command,
args,
env,
env_vars,
cwd,
} => {
assert_eq!(command, "echo");
assert_eq!(args, &vec!["hello".to_string()]);
assert!(env.is_none());
assert!(env_vars.is_empty());
assert!(cwd.is_none());
}
other => panic!("unexpected transport: {other:?}"),
}
Expand Down
28 changes: 27 additions & 1 deletion codex-rs/cli/tests/mcp_list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use std::path::Path;

use anyhow::Result;
use codex_core::config::load_global_mcp_servers;
use codex_core::config::write_global_mcp_servers;
use codex_core::config_types::McpServerTransportConfig;
use predicates::prelude::PredicateBooleanExt;
use predicates::str::contains;
use pretty_assertions::assert_eq;
Expand Down Expand Up @@ -46,6 +49,20 @@ fn list_and_get_render_expected_output() -> Result<()> {
.assert()
.success();

let runtime = tokio::runtime::Runtime::new()?;

@bolinfest bolinfest Oct 16, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems unusual: is there a reason why you need to introduce a Runtime instead of making this an async test? Also, I see the runtime is explicitly dropped. Can you change to async or add a comment why this is being done?

@gpeal gpeal Oct 17, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bad Codex. Thanks 🙏

let mut servers = runtime.block_on(load_global_mcp_servers(codex_home.path()))?;
drop(runtime);
let docs_entry = servers
.get_mut("docs")
.expect("docs server should exist after add");
match &mut docs_entry.transport {
McpServerTransportConfig::Stdio { env_vars, .. } => {
*env_vars = vec!["APP_TOKEN".to_string(), "WORKSPACE_ID".to_string()];
}
other => panic!("unexpected transport: {other:?}"),
}
write_global_mcp_servers(codex_home.path(), &servers)?;

let mut list_cmd = codex_command(codex_home.path())?;
let list_output = list_cmd.args(["mcp", "list"]).output()?;
assert!(list_output.status.success());
Expand All @@ -54,6 +71,8 @@ fn list_and_get_render_expected_output() -> Result<()> {
assert!(stdout.contains("docs"));
assert!(stdout.contains("docs-server"));
assert!(stdout.contains("TOKEN=secret"));
assert!(stdout.contains("APP_TOKEN=$APP_TOKEN"));
assert!(stdout.contains("WORKSPACE_ID=$WORKSPACE_ID"));
assert!(stdout.contains("Status"));
assert!(stdout.contains("Auth"));
assert!(stdout.contains("enabled"));
Expand All @@ -79,7 +98,12 @@ fn list_and_get_render_expected_output() -> Result<()> {
],
"env": {
"TOKEN": "secret"
}
},
"env_vars": [
"APP_TOKEN",
"WORKSPACE_ID"
],
"cwd": null
},
"startup_timeout_sec": null,
"tool_timeout_sec": null,
Expand All @@ -98,6 +122,8 @@ fn list_and_get_render_expected_output() -> Result<()> {
assert!(stdout.contains("command: docs-server"));
assert!(stdout.contains("args: --port 4000"));
assert!(stdout.contains("env: TOKEN=secret"));
assert!(stdout.contains("APP_TOKEN=$APP_TOKEN"));
assert!(stdout.contains("WORKSPACE_ID=$WORKSPACE_ID"));
assert!(stdout.contains("enabled: true"));
assert!(stdout.contains("remove: codex mcp remove docs"));

Expand Down
66 changes: 66 additions & 0 deletions codex-rs/common/src/format_env_display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
use std::collections::HashMap;

pub fn format_env_display(env: Option<&HashMap<String, String>>, env_vars: &[String]) -> String {
let mut parts: Vec<String> = Vec::new();

if let Some(map) = env {
let mut pairs: Vec<_> = map.iter().collect();
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
parts.extend(
pairs
.into_iter()
.map(|(key, value)| format!("{key}={value}")),
);
}

if !env_vars.is_empty() {
parts.extend(env_vars.iter().map(|var| format!("{var}=${var}")));
}

if parts.is_empty() {
"-".to_string()
} else {
parts.join(", ")
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn returns_dash_when_empty() {
assert_eq!(format_env_display(None, &[]), "-");

let empty_map = HashMap::new();
assert_eq!(format_env_display(Some(&empty_map), &[]), "-");
}

#[test]
fn formats_sorted_env_pairs() {
let mut env = HashMap::new();
env.insert("B".to_string(), "two".to_string());
env.insert("A".to_string(), "one".to_string());

assert_eq!(format_env_display(Some(&env), &[]), "A=one, B=two");
}

#[test]
fn formats_env_vars_with_dollar_prefix() {
let vars = vec!["TOKEN".to_string(), "PATH".to_string()];

assert_eq!(format_env_display(None, &vars), "TOKEN=$TOKEN, PATH=$PATH");
}

#[test]
fn combines_env_pairs_and_vars() {
let mut env = HashMap::new();
env.insert("HOME".to_string(), "/tmp".to_string());
let vars = vec!["TOKEN".to_string()];

assert_eq!(
format_env_display(Some(&env), &vars),
"HOME=/tmp, TOKEN=$TOKEN"
);
}
}
3 changes: 3 additions & 0 deletions codex-rs/common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ mod sandbox_mode_cli_arg;
#[cfg(feature = "cli")]
pub use sandbox_mode_cli_arg::SandboxModeCliArg;

#[cfg(feature = "cli")]
pub mod format_env_display;

#[cfg(any(feature = "cli", test))]
mod config_override;

Expand Down
Loading
Loading