feat: MCP support for agentic CLI providers (#6972)
Signed-off-by: Adrian Cole <adrian@tetrate.io>
This commit is contained in:
@@ -16,7 +16,7 @@ use super::utils::{filter_extensions_from_system_prompt, RequestLog};
|
||||
use crate::config::base::{CodexCommand, CodexReasoningEffort, CodexSkipGitCheck};
|
||||
use crate::config::paths::Paths;
|
||||
use crate::config::search_path::SearchPaths;
|
||||
use crate::config::{Config, GooseMode};
|
||||
use crate::config::{Config, ExtensionConfig, GooseMode};
|
||||
use crate::conversation::message::{Message, MessageContent};
|
||||
use crate::model::ModelConfig;
|
||||
use crate::subprocess::configure_subprocess;
|
||||
@@ -50,6 +50,8 @@ pub struct CodexProvider {
|
||||
reasoning_effort: String,
|
||||
/// Whether to skip git repo check
|
||||
skip_git_check: bool,
|
||||
/// CLI config overrides for MCP servers
|
||||
mcp_config_overrides: Vec<String>,
|
||||
}
|
||||
|
||||
impl CodexProvider {
|
||||
@@ -140,6 +142,10 @@ impl CodexProvider {
|
||||
self.reasoning_effort
|
||||
));
|
||||
|
||||
for override_config in &self.mcp_config_overrides {
|
||||
cmd.arg("-c").arg(override_config);
|
||||
}
|
||||
|
||||
// JSON output format for structured parsing
|
||||
cmd.arg("--json");
|
||||
|
||||
@@ -539,7 +545,92 @@ fn prepare_input(
|
||||
Ok((prompt, temp_files))
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
fn toml_quote(s: &str) -> String {
|
||||
let mut out = String::with_capacity(s.len() + 2);
|
||||
out.push('"');
|
||||
for ch in s.chars() {
|
||||
match ch {
|
||||
'\\' => out.push_str("\\\\"),
|
||||
'"' => out.push_str("\\\""),
|
||||
'\n' => out.push_str("\\n"),
|
||||
'\r' => out.push_str("\\r"),
|
||||
'\t' => out.push_str("\\t"),
|
||||
'\u{0008}' => out.push_str("\\b"),
|
||||
'\u{000C}' => out.push_str("\\f"),
|
||||
c if c.is_control() => {
|
||||
// TOML \uXXXX for other control characters
|
||||
for unit in c.encode_utf16(&mut [0; 2]) {
|
||||
out.push_str(&format!("\\u{:04X}", unit));
|
||||
}
|
||||
}
|
||||
c => out.push(c),
|
||||
}
|
||||
}
|
||||
out.push('"');
|
||||
out
|
||||
}
|
||||
|
||||
// Codex CLI only supports inline `-c key=value` TOML overrides — no file-based
|
||||
// config merging. Resolved secrets (from env_keys/keystore) in envs/headers end
|
||||
// up in process argv, visible via `ps`. Claude Code avoids this by writing to a
|
||||
// temp file with 0o600 permissions.
|
||||
// Tracking: https://github.com/openai/codex/issues/2628
|
||||
fn codex_mcp_config_overrides(extensions: &[ExtensionConfig]) -> Vec<String> {
|
||||
let mut overrides = Vec::new();
|
||||
for extension in extensions {
|
||||
match extension {
|
||||
ExtensionConfig::StreamableHttp { uri, headers, .. } => {
|
||||
let key = extension.key();
|
||||
overrides.push(format!("mcp_servers.{}.url={}", key, toml_quote(uri)));
|
||||
if !headers.is_empty() {
|
||||
let mut hkeys: Vec<_> = headers.keys().collect();
|
||||
hkeys.sort();
|
||||
let entries: Vec<_> = hkeys
|
||||
.iter()
|
||||
.map(|k| format!("{} = {}", toml_quote(k), toml_quote(&headers[*k])))
|
||||
.collect();
|
||||
overrides.push(format!(
|
||||
"mcp_servers.{}.http_headers={{{}}}",
|
||||
key,
|
||||
entries.join(", ")
|
||||
));
|
||||
}
|
||||
}
|
||||
ExtensionConfig::Stdio {
|
||||
cmd, args, envs, ..
|
||||
} => {
|
||||
let key = extension.key();
|
||||
overrides.push(format!("mcp_servers.{}.command={}", key, toml_quote(cmd)));
|
||||
if !args.is_empty() {
|
||||
let items: Vec<_> = args.iter().map(|a| toml_quote(a)).collect();
|
||||
overrides.push(format!("mcp_servers.{}.args=[{}]", key, items.join(", ")));
|
||||
}
|
||||
let env_map = envs.get_env();
|
||||
if !env_map.is_empty() {
|
||||
let mut ekeys: Vec<_> = env_map.keys().collect();
|
||||
ekeys.sort();
|
||||
let entries: Vec<_> = ekeys
|
||||
.iter()
|
||||
.map(|k| {
|
||||
format!("{} = {}", toml_quote(k), toml_quote(&env_map[k.as_str()]))
|
||||
})
|
||||
.collect();
|
||||
overrides.push(format!(
|
||||
"mcp_servers.{}.env={{{}}}",
|
||||
key,
|
||||
entries.join(", ")
|
||||
));
|
||||
}
|
||||
}
|
||||
ExtensionConfig::Sse { name, .. } => {
|
||||
tracing::debug!(name, "skipping SSE extension, migrate to streamable_http");
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
overrides
|
||||
}
|
||||
|
||||
impl ProviderDef for CodexProvider {
|
||||
type Provider = Self;
|
||||
|
||||
@@ -560,7 +651,10 @@ impl ProviderDef for CodexProvider {
|
||||
.with_unlisted_models()
|
||||
}
|
||||
|
||||
fn from_env(model: ModelConfig) -> BoxFuture<'static, Result<Self::Provider>> {
|
||||
fn from_env(
|
||||
model: ModelConfig,
|
||||
extensions: Vec<ExtensionConfig>,
|
||||
) -> BoxFuture<'static, Result<Self::Provider>> {
|
||||
Box::pin(async move {
|
||||
let config = Config::global();
|
||||
let command: String = config.get_codex_command().unwrap_or_default().into();
|
||||
@@ -591,12 +685,18 @@ impl ProviderDef for CodexProvider {
|
||||
.map(|s| s.to_lowercase() == "true")
|
||||
.unwrap_or(false);
|
||||
|
||||
let mut resolved = Vec::with_capacity(extensions.len());
|
||||
for ext in extensions {
|
||||
resolved.push(ext.resolve(config).await?);
|
||||
}
|
||||
|
||||
Ok(Self {
|
||||
command: resolved_command,
|
||||
model,
|
||||
name: CODEX_PROVIDER_NAME.to_string(),
|
||||
reasoning_effort,
|
||||
skip_git_check,
|
||||
mcp_config_overrides: codex_mcp_config_overrides(&resolved),
|
||||
})
|
||||
})
|
||||
}
|
||||
@@ -669,7 +769,9 @@ impl Provider for CodexProvider {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::agents::extension::Envs;
|
||||
use goose_test_support::TEST_IMAGE_B64;
|
||||
use std::collections::HashMap;
|
||||
use test_case::test_case;
|
||||
|
||||
#[test]
|
||||
@@ -685,6 +787,96 @@ mod tests {
|
||||
.any(|m| m.name == CODEX_DEFAULT_MODEL));
|
||||
}
|
||||
|
||||
#[test_case(
|
||||
ExtensionConfig::Stdio {
|
||||
name: "lookup".into(),
|
||||
cmd: "node".into(),
|
||||
args: vec!["server.js".into()],
|
||||
envs: Envs::new([("API_KEY".into(), "secret".into())].into()),
|
||||
env_keys: vec![],
|
||||
description: "Lookup".into(),
|
||||
timeout: Some(30),
|
||||
bundled: None,
|
||||
available_tools: vec![],
|
||||
},
|
||||
&[
|
||||
r#"mcp_servers.lookup.command="node""#,
|
||||
r#"mcp_servers.lookup.args=["server.js"]"#,
|
||||
r#"mcp_servers.lookup.env={"API_KEY" = "secret"}"#,
|
||||
]
|
||||
; "stdio_converts_to_mcp_overrides"
|
||||
)]
|
||||
#[test_case(
|
||||
ExtensionConfig::StreamableHttp {
|
||||
name: "lookup".into(),
|
||||
description: String::new(),
|
||||
uri: "http://localhost/mcp".into(),
|
||||
envs: Envs::default(),
|
||||
env_keys: vec![],
|
||||
headers: HashMap::from([("Authorization".into(), "Bearer token".into())]),
|
||||
timeout: None,
|
||||
bundled: Some(false),
|
||||
available_tools: vec![],
|
||||
},
|
||||
&[
|
||||
r#"mcp_servers.lookup.url="http://localhost/mcp""#,
|
||||
r#"mcp_servers.lookup.http_headers={"Authorization" = "Bearer token"}"#,
|
||||
]
|
||||
; "streamable_http_converts_to_mcp_overrides"
|
||||
)]
|
||||
#[test_case(
|
||||
ExtensionConfig::StreamableHttp {
|
||||
name: "mcp_kiwi_com".into(),
|
||||
description: String::new(),
|
||||
uri: "https://mcp.kiwi.com".into(),
|
||||
envs: Envs::default(),
|
||||
env_keys: vec![],
|
||||
headers: HashMap::new(),
|
||||
timeout: None,
|
||||
bundled: None,
|
||||
available_tools: vec![],
|
||||
},
|
||||
&[
|
||||
r#"mcp_servers.mcp_kiwi_com.url="https://mcp.kiwi.com""#,
|
||||
]
|
||||
; "resolved_name_used_as_key_http"
|
||||
)]
|
||||
#[test_case(
|
||||
ExtensionConfig::Stdio {
|
||||
name: "my-server".into(),
|
||||
cmd: "/usr/bin/my-server".into(),
|
||||
args: vec![],
|
||||
envs: Envs::default(),
|
||||
env_keys: vec![],
|
||||
description: String::new(),
|
||||
timeout: None,
|
||||
bundled: None,
|
||||
available_tools: vec![],
|
||||
},
|
||||
&[
|
||||
r#"mcp_servers.my-server.command="/usr/bin/my-server""#,
|
||||
]
|
||||
; "resolved_name_used_as_key_stdio"
|
||||
)]
|
||||
fn test_codex_mcp_overrides(config: ExtensionConfig, expected: &[&str]) {
|
||||
let overrides = codex_mcp_config_overrides(&[config]);
|
||||
let expected: Vec<String> = expected.iter().map(|s| s.to_string()).collect();
|
||||
assert_eq!(overrides, expected);
|
||||
}
|
||||
|
||||
#[test_case("simple", r#""simple""# ; "no_special_chars")]
|
||||
#[test_case(r#"back\slash"#, r#""back\\slash""# ; "backslash")]
|
||||
#[test_case(r#"has"quote"#, r#""has\"quote""# ; "double_quote")]
|
||||
#[test_case("line\nbreak", r#""line\nbreak""# ; "newline")]
|
||||
#[test_case("tab\there", r#""tab\there""# ; "tab")]
|
||||
#[test_case("cr\rhere", r#""cr\rhere""# ; "carriage_return")]
|
||||
#[test_case("bell\u{0008}here", r#""bell\bhere""# ; "backspace")]
|
||||
#[test_case("ff\u{000C}here", r#""ff\fhere""# ; "form_feed")]
|
||||
#[test_case("null\u{0000}here", r#""null\u0000here""# ; "null_control_char")]
|
||||
fn test_toml_quote(input: &str, expected: &str) {
|
||||
assert_eq!(toml_quote(input), expected);
|
||||
}
|
||||
|
||||
#[test_case("image/png", ".png" ; "png image")]
|
||||
#[test_case("image/jpeg", ".jpg" ; "jpeg image")]
|
||||
fn test_prepare_input_image(mime: &str, expected_ext: &str) {
|
||||
@@ -765,6 +957,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let lines = vec!["Hello, world!".to_string()];
|
||||
@@ -784,6 +977,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
// Test with actual Codex CLI output format
|
||||
@@ -816,6 +1010,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let lines: Vec<String> = vec![];
|
||||
@@ -863,6 +1058,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let lines = vec![
|
||||
@@ -887,6 +1083,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let lines = vec![
|
||||
@@ -958,6 +1155,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let lines: Vec<String> = lines.iter().map(|s| s.to_string()).collect();
|
||||
@@ -973,6 +1171,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let lines = vec![
|
||||
@@ -999,6 +1198,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let messages = vec![Message::new(
|
||||
@@ -1030,6 +1230,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let messages: Vec<Message> = vec![];
|
||||
@@ -1072,6 +1273,7 @@ mod tests {
|
||||
name: "codex".to_string(),
|
||||
reasoning_effort: "high".to_string(),
|
||||
skip_git_check: false,
|
||||
mcp_config_overrides: Vec::new(),
|
||||
};
|
||||
|
||||
let lines = vec![
|
||||
|
||||
Reference in New Issue
Block a user