Merge branch 'main' into etraut/next-turn-state-core

This commit is contained in:
Eric Traut
2026-05-14 12:58:21 -07:00
committed by GitHub
49 changed files with 1231 additions and 161 deletions
@@ -160,6 +160,7 @@ impl McpRequestProcessor {
http_headers,
env_http_headers,
&resolved_scopes.scopes,
server.oauth_client_id(),
server.oauth_resource.as_deref(),
timeout_secs,
config.mcp_oauth_callback_port,
@@ -1346,6 +1346,7 @@ impl PluginRequestProcessor {
let notification_name = name.clone();
tokio::spawn(async move {
let oauth_client_id = server.oauth_client_id();
let first_attempt = perform_oauth_login_silent(
&name,
&oauth_config.url,
@@ -1353,6 +1354,7 @@ impl PluginRequestProcessor {
oauth_config.http_headers.clone(),
oauth_config.env_http_headers.clone(),
&resolved_scopes.scopes,
oauth_client_id,
server.oauth_resource.as_deref(),
callback_port,
callback_url.as_deref(),
@@ -1368,6 +1370,7 @@ impl PluginRequestProcessor {
oauth_config.http_headers,
oauth_config.env_http_headers,
&[],
oauth_client_id,
server.oauth_resource.as_deref(),
callback_port,
callback_url.as_deref(),
@@ -1,8 +1,10 @@
use std::time::Duration;
use anyhow::Result;
use app_test_support::ChatGptAuthFixture;
use app_test_support::McpProcess;
use app_test_support::to_response;
use app_test_support::write_chatgpt_auth;
use app_test_support::write_models_cache;
use codex_app_server_protocol::JSONRPCError;
use codex_app_server_protocol::JSONRPCResponse;
@@ -13,10 +15,16 @@ use codex_app_server_protocol::ModelServiceTier;
use codex_app_server_protocol::ModelUpgradeInfo;
use codex_app_server_protocol::ReasoningEffortOption;
use codex_app_server_protocol::RequestId;
use codex_config::types::AuthCredentialsStoreMode;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::openai_models::ModelsResponse;
use core_test_support::responses::mount_models_once;
use pretty_assertions::assert_eq;
use serde_json::json;
use tempfile::TempDir;
use tokio::time::timeout;
use wiremock::MockServer;
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
const INVALID_REQUEST_ERROR_CODE: i64 = -32600;
@@ -148,6 +156,101 @@ async fn list_models_includes_hidden_models() -> Result<()> {
Ok(())
}
#[tokio::test]
async fn list_models_uses_chatgpt_remote_catalog_as_source_of_truth() -> Result<()> {
let server = MockServer::start().await;
let remote_model: ModelInfo = serde_json::from_value(json!({
"slug": "chatgpt-remote-only",
"display_name": "ChatGPT Remote Only",
"description": "Remote-only model for app-server model/list coverage",
"default_reasoning_level": "medium",
"supported_reasoning_levels": [
{"effort": "low", "description": "low"},
{"effort": "medium", "description": "medium"}
],
"shell_type": "shell_command",
"visibility": "list",
"minimal_client_version": [0, 1, 0],
"supported_in_api": true,
"priority": 0,
"upgrade": null,
"base_instructions": "base instructions",
"supports_reasoning_summaries": false,
"support_verbosity": false,
"default_verbosity": null,
"apply_patch_tool_type": null,
"truncation_policy": {"mode": "bytes", "limit": 10_000},
"supports_parallel_tool_calls": false,
"supports_image_detail_original": false,
"context_window": 272_000,
"max_context_window": 272_000,
"experimental_supported_tools": [],
}))?;
let models_mock = mount_models_once(
&server,
ModelsResponse {
models: vec![remote_model.clone()],
},
)
.await;
let codex_home = TempDir::new()?;
let server_uri = server.uri();
std::fs::write(
codex_home.path().join("config.toml"),
format!(
r#"
model = "mock-model"
approval_policy = "never"
sandbox_mode = "read-only"
openai_base_url = "{server_uri}/v1"
"#
),
)?;
write_chatgpt_auth(
codex_home.path(),
ChatGptAuthFixture::new("chatgpt-access-token").plan_type("pro"),
AuthCredentialsStoreMode::File,
)?;
let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_list_models_request(ModelListParams {
limit: Some(100),
cursor: None,
include_hidden: None,
})
.await?;
let response: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??;
let ModelListResponse {
data: items,
next_cursor,
} = to_response::<ModelListResponse>(response)?;
let mut expected_presets: Vec<ModelPreset> = vec![remote_model.into()];
ModelPreset::mark_default_by_picker_visibility(&mut expected_presets);
let expected_items = expected_presets
.iter()
.map(model_from_preset)
.collect::<Vec<_>>();
assert_eq!(items, expected_items);
assert!(next_cursor.is_none());
assert_eq!(
models_mock.requests().len(),
1,
"expected a single /models request"
);
Ok(())
}
#[tokio::test]
async fn list_models_pagination_works() -> Result<()> {
let codex_home = TempDir::new()?;
+1 -1
View File
@@ -152,7 +152,7 @@ async fn run_list(overrides: Vec<(String, toml::Value)>) -> Result<()> {
.context("failed to load configuration")?;
let configured_marketplaces = config
.config_layer_stack
.get_user_layer()
.get_active_user_layer()
.and_then(|layer| layer.config.get("marketplaces"))
.and_then(toml::Value::as_table);
let Some(configured_marketplaces) = configured_marketplaces else {
+6
View File
@@ -199,6 +199,7 @@ async fn perform_oauth_login_retry_without_scopes(
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
resolved_scopes: &ResolvedMcpOAuthScopes,
oauth_client_id: Option<&str>,
oauth_resource: Option<&str>,
callback_port: Option<u16>,
callback_url: Option<&str>,
@@ -210,6 +211,7 @@ async fn perform_oauth_login_retry_without_scopes(
http_headers.clone(),
env_http_headers.clone(),
&resolved_scopes.scopes,
oauth_client_id,
oauth_resource,
callback_port,
callback_url,
@@ -226,6 +228,7 @@ async fn perform_oauth_login_retry_without_scopes(
http_headers,
env_http_headers,
&[],
oauth_client_id,
oauth_resource,
callback_port,
callback_url,
@@ -309,6 +312,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
};
@@ -338,6 +342,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
oauth_config.http_headers,
oauth_config.env_http_headers,
&resolved_scopes,
/*oauth_client_id*/ None,
/*oauth_resource*/ None,
config.mcp_oauth_callback_port,
config.mcp_oauth_callback_url.as_deref(),
@@ -431,6 +436,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs)
http_headers,
env_http_headers,
&resolved_scopes,
server.oauth_client_id(),
server.oauth_resource.as_deref(),
config.mcp_oauth_callback_port,
config.mcp_oauth_callback_url.as_deref(),
+1 -1
View File
@@ -349,7 +349,7 @@ fn configured_marketplace_snapshot_issues(
load_errors: &[MarketplaceListError],
marketplace_name: Option<&str>,
) -> Vec<ConfiguredMarketplaceSnapshotIssue> {
let Some(user_layer) = plugins_input.config_layer_stack.get_user_layer() else {
let Some(user_layer) = plugins_input.config_layer_stack.get_active_user_layer() else {
return Vec::new();
};
let Some(configured_marketplaces) = user_layer
@@ -893,6 +893,7 @@ fn mcp_init_error_display_prompts_for_github_pat() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
}),
@@ -945,6 +946,7 @@ fn mcp_init_error_display_reports_generic_errors() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
}),
+1
View File
@@ -446,6 +446,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig) -> McpServerConfig {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
}
+2
View File
@@ -278,6 +278,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -302,6 +303,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
+1
View File
@@ -96,6 +96,7 @@ pub use mcp_types::AppToolApproval;
pub use mcp_types::McpServerConfig;
pub use mcp_types::McpServerDisabledReason;
pub use mcp_types::McpServerEnvVar;
pub use mcp_types::McpServerOAuthConfig;
pub use mcp_types::McpServerToolConfig;
pub use mcp_types::McpServerTransportConfig;
pub use mcp_types::RawMcpServerConfig;
+9
View File
@@ -212,6 +212,15 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem {
{
entry["scopes"] = array_from_strings(scopes);
}
if let Some(oauth) = &config.oauth
&& let Some(client_id) = &oauth.client_id
&& !client_id.is_empty()
{
let mut oauth_table = TomlTable::new();
oauth_table.set_implicit(false);
oauth_table["client_id"] = value(client_id.clone());
entry["oauth"] = TomlItem::Table(oauth_table);
}
if let Some(resource) = &config.oauth_resource
&& !resource.is_empty()
{
+62
View File
@@ -1,4 +1,5 @@
use super::*;
use crate::McpServerOAuthConfig;
use crate::McpServerToolConfig;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
@@ -33,6 +34,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow:
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::from([
(
@@ -82,3 +84,63 @@ approval_mode = "approve"
Ok(())
}
#[tokio::test]
async fn replace_mcp_servers_serializes_oauth_client_id() -> anyhow::Result<()> {
let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos();
let codex_home = std::env::temp_dir().join(format!(
"codex-config-mcp-oauth-edit-test-{}-{unique_suffix}",
std::process::id()
));
let servers = BTreeMap::from([(
"maas_outlook".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url: "https://example.com/mcp".to_string(),
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
experimental_environment: None,
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: None,
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: Some(McpServerOAuthConfig {
client_id: Some("eci-prd-pub-codex-123".to_string()),
}),
oauth_resource: None,
tools: HashMap::new(),
},
)]);
ConfigEditsBuilder::new(&codex_home)
.replace_mcp_servers(&servers)
.apply()
.await?;
let config_path = codex_home.join(CONFIG_TOML_FILE);
let serialized = std::fs::read_to_string(&config_path)?;
assert_eq!(
serialized,
r#"[mcp_servers.maas_outlook]
url = "https://example.com/mcp"
[mcp_servers.maas_outlook.oauth]
client_id = "eci-prd-pub-codex-123"
"#
);
let loaded = load_global_mcp_servers(&codex_home).await?;
assert_eq!(loaded, servers);
std::fs::remove_dir_all(&codex_home)?;
Ok(())
}
+26
View File
@@ -114,6 +114,15 @@ impl AsRef<str> for McpServerEnvVar {
}
}
/// OAuth client settings used when Codex launches an MCP OAuth flow.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)]
#[schemars(deny_unknown_fields)]
pub struct McpServerOAuthConfig {
/// Explicit OAuth client identifier to present during authorization and token exchange.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub client_id: Option<String>,
}
#[derive(Serialize, Debug, Clone, PartialEq)]
pub struct McpServerConfig {
#[serde(flatten)]
@@ -167,6 +176,10 @@ pub struct McpServerConfig {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub scopes: Option<Vec<String>>,
/// Optional OAuth client settings for MCP login.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub oauth: Option<McpServerOAuthConfig>,
/// Optional OAuth resource parameter to include during MCP login (RFC 8707).
#[serde(default, skip_serializing_if = "Option::is_none")]
pub oauth_resource: Option<String>,
@@ -176,6 +189,14 @@ pub struct McpServerConfig {
pub tools: HashMap<String, McpServerToolConfig>,
}
impl McpServerConfig {
pub fn oauth_client_id(&self) -> Option<&str> {
self.oauth
.as_ref()
.and_then(|oauth| oauth.client_id.as_deref())
}
}
/// Raw MCP config shape used for deserialization and supported-field JSON
/// Schema generation.
///
@@ -233,6 +254,8 @@ pub struct RawMcpServerConfig {
#[serde(default)]
pub scopes: Option<Vec<String>>,
#[serde(default)]
pub oauth: Option<McpServerOAuthConfig>,
#[serde(default)]
pub oauth_resource: Option<String>,
/// Legacy display-name field accepted for backward compatibility.
#[serde(default, rename = "name")]
@@ -267,6 +290,7 @@ impl TryFrom<RawMcpServerConfig> for McpServerConfig {
enabled_tools,
disabled_tools,
scopes,
oauth,
oauth_resource,
_name: _,
tools,
@@ -297,6 +321,7 @@ impl TryFrom<RawMcpServerConfig> for McpServerConfig {
throw_if_set("stdio", "bearer_token", bearer_token.as_ref())?;
throw_if_set("stdio", "http_headers", http_headers.as_ref())?;
throw_if_set("stdio", "env_http_headers", env_http_headers.as_ref())?;
throw_if_set("stdio", "oauth", oauth.as_ref())?;
throw_if_set("stdio", "oauth_resource", oauth_resource.as_ref())?;
let env_vars = env_vars.unwrap_or_default();
for env_var in &env_vars {
@@ -338,6 +363,7 @@ impl TryFrom<RawMcpServerConfig> for McpServerConfig {
enabled_tools,
disabled_tools,
scopes,
oauth,
oauth_resource,
tools: tools.unwrap_or_default(),
})
+34
View File
@@ -283,6 +283,26 @@ fn deserialize_streamable_http_server_config_with_oauth_resource() {
);
}
#[test]
fn deserialize_streamable_http_server_config_with_oauth_client_id() {
let cfg: McpServerConfig = toml::from_str(
r#"
url = "https://example.com/mcp"
[oauth]
client_id = "eci-prd-pub-codex-123"
"#,
)
.expect("should deserialize http config with oauth client id");
assert_eq!(
cfg.oauth,
Some(McpServerOAuthConfig {
client_id: Some("eci-prd-pub-codex-123".to_string()),
})
);
}
#[test]
fn deserialize_server_config_with_tool_filters() {
let cfg: McpServerConfig = toml::from_str(
@@ -393,6 +413,7 @@ fn deserialize_ignores_unknown_server_fields() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
}
@@ -439,6 +460,19 @@ fn deserialize_rejects_headers_for_stdio() {
)
.expect_err("should reject env_http_headers for stdio transport");
let err = toml::from_str::<McpServerConfig>(
r#"
command = "echo"
oauth = { client_id = "eci-prd-pub-codex-123" }
"#,
)
.expect_err("should reject oauth for stdio transport");
assert!(
err.to_string().contains("oauth is not supported for stdio"),
"unexpected error: {err}"
);
let err = toml::from_str::<McpServerConfig>(
r#"
command = "echo"
+1
View File
@@ -7,6 +7,7 @@ pub use crate::mcp_types::AppToolApproval;
pub use crate::mcp_types::McpServerConfig;
pub use crate::mcp_types::McpServerDisabledReason;
pub use crate::mcp_types::McpServerEnvVar;
pub use crate::mcp_types::McpServerOAuthConfig;
pub use crate::mcp_types::McpServerToolConfig;
pub use crate::mcp_types::McpServerTransportConfig;
pub use crate::mcp_types::RawMcpServerConfig;
+15 -7
View File
@@ -1053,13 +1053,21 @@ fn normalize_plugin_mcp_server_value(
}
}
if let Some(JsonValue::Object(oauth)) = object.remove("oauth")
&& oauth.contains_key("callbackPort")
{
warn!(
plugin = %plugin_root.display(),
"plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings"
);
if let Some(JsonValue::Object(mut oauth)) = object.remove("oauth") {
if oauth.remove("callbackPort").is_some() {
warn!(
plugin = %plugin_root.display(),
"plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings"
);
}
if let Some(client_id) = oauth.remove("clientId") {
oauth.entry("client_id".to_string()).or_insert(client_id);
}
if !oauth.is_empty() {
object.insert("oauth".to_string(), JsonValue::Object(oauth));
}
}
if let Some(JsonValue::String(cwd)) = object.get("cwd")
@@ -22,6 +22,7 @@ use codex_config::ConfigLayerStack;
use codex_config::ConfigRequirements;
use codex_config::ConfigRequirementsToml;
use codex_config::McpServerConfig;
use codex_config::McpServerOAuthConfig;
use codex_config::McpServerToolConfig;
use codex_config::types::McpServerTransportConfig;
use codex_login::CodexAuth;
@@ -230,6 +231,9 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: Some(McpServerOAuthConfig {
client_id: Some("client-id".to_string()),
}),
oauth_resource: None,
tools: HashMap::new(),
},
@@ -694,6 +698,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -805,6 +810,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -966,6 +972,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
};
+19
View File
@@ -1200,6 +1200,17 @@
}
]
},
"McpServerOAuthConfig": {
"additionalProperties": false,
"description": "OAuth client settings used when Codex launches an MCP OAuth flow.",
"properties": {
"client_id": {
"description": "Explicit OAuth client identifier to present during authorization and token exchange.",
"type": "string"
}
},
"type": "object"
},
"McpServerToolConfig": {
"additionalProperties": false,
"description": "Per-tool approval settings for a single MCP server tool.",
@@ -2124,6 +2135,14 @@
"description": "Legacy display-name field accepted for backward compatibility.",
"type": "string"
},
"oauth": {
"allOf": [
{
"$ref": "#/definitions/McpServerOAuthConfig"
}
],
"default": null
},
"oauth_resource": {
"default": null,
"type": "string"
+23
View File
@@ -36,6 +36,7 @@ use codex_config::types::BundledSkillsConfig;
use codex_config::types::FeedbackConfigToml;
use codex_config::types::HistoryPersistence;
use codex_config::types::McpServerEnvVar;
use codex_config::types::McpServerOAuthConfig;
use codex_config::types::McpServerToolConfig;
use codex_config::types::McpServerTransportConfig;
use codex_config::types::MemoriesConfig;
@@ -124,6 +125,7 @@ fn stdio_mcp(command: &str) -> McpServerConfig {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
}
@@ -148,6 +150,7 @@ fn http_mcp(url: &str) -> McpServerConfig {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
}
@@ -4428,6 +4431,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -4721,6 +4725,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -4797,6 +4802,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -4858,6 +4864,7 @@ async fn replace_mcp_servers_serializes_sourced_env_vars() -> anyhow::Result<()>
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -4909,6 +4916,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -4963,6 +4971,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5033,6 +5042,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5115,6 +5125,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5150,6 +5161,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5220,6 +5232,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers()
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5245,6 +5258,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers()
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5333,6 +5347,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5383,6 +5398,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5433,6 +5449,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> {
enabled_tools: Some(vec!["allowed".to_string()]),
disabled_tools: Some(vec!["blocked".to_string()]),
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -5487,6 +5504,9 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: Some(McpServerOAuthConfig {
client_id: Some("eci-prd-pub-codex-123".to_string()),
}),
oauth_resource: Some("https://resource.example.com".to_string()),
tools: HashMap::new(),
},
@@ -5500,6 +5520,8 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh
let config_path = codex_home.path().join(CONFIG_TOML_FILE);
let serialized = std::fs::read_to_string(&config_path)?;
assert!(serialized.contains("[mcp_servers.docs.oauth]"));
assert!(serialized.contains(r#"client_id = "eci-prd-pub-codex-123""#));
assert!(serialized.contains(r#"oauth_resource = "https://resource.example.com""#));
let loaded = load_global_mcp_servers(codex_home.path()).await?;
@@ -5508,6 +5530,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh
docs.oauth_resource.as_deref(),
Some("https://resource.example.com")
);
assert_eq!(docs.oauth_client_id(), Some("eci-prd-pub-codex-123"));
Ok(())
}
+9
View File
@@ -341,6 +341,15 @@ mod document_helpers {
{
entry["scopes"] = array_from_iter(scopes.iter().cloned());
}
if let Some(oauth) = &config.oauth
&& let Some(client_id) = &oauth.client_id
&& !client_id.is_empty()
{
let mut oauth_table = TomlTable::new();
oauth_table.set_implicit(false);
oauth_table["client_id"] = value(client_id.clone());
entry["oauth"] = TomlItem::Table(oauth_table);
}
if let Some(resource) = &config.oauth_resource
&& !resource.is_empty()
{
+13
View File
@@ -1,5 +1,6 @@
use super::*;
use codex_config::types::AppToolApproval;
use codex_config::types::McpServerOAuthConfig;
use codex_config::types::McpServerToolConfig;
use codex_config::types::McpServerTransportConfig;
use codex_config::types::SessionPickerViewMode;
@@ -940,6 +941,7 @@ fn blocking_replace_mcp_servers_round_trips() {
enabled_tools: Some(vec!["one".to_string(), "two".to_string()]),
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -969,6 +971,9 @@ fn blocking_replace_mcp_servers_round_trips() {
enabled_tools: None,
disabled_tools: Some(vec!["forbidden".to_string()]),
scopes: None,
oauth: Some(McpServerOAuthConfig {
client_id: Some("eci-prd-pub-codex-123".to_string()),
}),
oauth_resource: Some("https://resource.example.com".to_string()),
tools: HashMap::new(),
},
@@ -994,6 +999,9 @@ oauth_resource = \"https://resource.example.com\"
[mcp_servers.http.http_headers]
Z-Header = \"z\"
[mcp_servers.http.oauth]
client_id = \"eci-prd-pub-codex-123\"
[mcp_servers.stdio]
command = \"cmd\"
args = [\"--flag\"]
@@ -1035,6 +1043,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::from([(
"search".to_string(),
@@ -1099,6 +1108,7 @@ foo = { command = "cmd" }
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -1153,6 +1163,7 @@ foo = { command = "cmd" } # keep me
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -1206,6 +1217,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -1260,6 +1272,7 @@ foo = { command = "cmd" }
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -151,6 +151,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
server_config.scopes.clone(),
oauth_config.discovered_scopes.clone(),
);
let oauth_client_id = server_config.oauth_client_id();
let first_attempt = perform_oauth_login(
&name,
&oauth_config.url,
@@ -158,6 +159,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
oauth_config.http_headers.clone(),
oauth_config.env_http_headers.clone(),
&resolved_scopes.scopes,
oauth_client_id,
server_config.oauth_resource.as_deref(),
config.mcp_oauth_callback_port,
config.mcp_oauth_callback_url.as_deref(),
@@ -173,6 +175,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
oauth_config.http_headers,
oauth_config.env_http_headers,
&[],
oauth_client_id,
server_config.oauth_resource.as_deref(),
config.mcp_oauth_callback_port,
config.mcp_oauth_callback_url.as_deref(),
@@ -369,6 +372,7 @@ fn mcp_dependency_to_server_config(
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
});
@@ -398,6 +402,7 @@ fn mcp_dependency_to_server_config(
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
});
+54 -14
View File
@@ -296,11 +296,14 @@ impl TestCodexBuilder {
};
let base_url = format!("{}/v1", server.uri());
let test_env = TestEnv::local().await?;
Box::pin(self.build_with_home_and_base_url(base_url, home, /*resume_from*/ None, test_env))
.await
Box::pin(self.build_with_home_and_base_url(
base_url, home, /*resume_from*/ None, test_env,
/*include_local_environment*/ false,
))
.await
}
pub async fn build_remote_aware(
pub async fn build_with_remote_env(
&mut self,
server: &wiremock::MockServer,
) -> anyhow::Result<TestCodex> {
@@ -310,8 +313,28 @@ impl TestCodexBuilder {
};
let base_url = format!("{}/v1", server.uri());
let test_env = test_env().await?;
Box::pin(self.build_with_home_and_base_url(base_url, home, /*resume_from*/ None, test_env))
.await
Box::pin(self.build_with_home_and_base_url(
base_url, home, /*resume_from*/ None, test_env,
/*include_local_environment*/ false,
))
.await
}
pub async fn build_with_remote_and_local_env(
&mut self,
server: &wiremock::MockServer,
) -> anyhow::Result<TestCodex> {
let home = match self.home.clone() {
Some(home) => home,
None => Arc::new(TempDir::new()?),
};
let base_url = format!("{}/v1", server.uri());
let test_env = test_env().await?;
Box::pin(self.build_with_home_and_base_url(
base_url, home, /*resume_from*/ None, test_env,
/*include_local_environment*/ true,
))
.await
}
pub async fn build_with_streaming_server(
@@ -329,6 +352,7 @@ impl TestCodexBuilder {
home,
/*resume_from*/ None,
test_env,
/*include_local_environment*/ false,
))
.await
}
@@ -350,8 +374,11 @@ impl TestCodexBuilder {
config.realtime.version = RealtimeWsVersion::V1;
}));
let test_env = TestEnv::local().await?;
Box::pin(self.build_with_home_and_base_url(base_url, home, /*resume_from*/ None, test_env))
.await
Box::pin(self.build_with_home_and_base_url(
base_url, home, /*resume_from*/ None, test_env,
/*include_local_environment*/ false,
))
.await
}
pub async fn resume(
@@ -362,8 +389,14 @@ impl TestCodexBuilder {
) -> anyhow::Result<TestCodex> {
let base_url = format!("{}/v1", server.uri());
let test_env = TestEnv::local().await?;
Box::pin(self.build_with_home_and_base_url(base_url, home, Some(rollout_path), test_env))
.await
Box::pin(self.build_with_home_and_base_url(
base_url,
home,
Some(rollout_path),
test_env,
/*include_local_environment*/ false,
))
.await
}
async fn build_with_home_and_base_url(
@@ -372,6 +405,7 @@ impl TestCodexBuilder {
home: Arc<TempDir>,
resume_from: Option<PathBuf>,
test_env: TestEnv,
include_local_environment: bool,
) -> anyhow::Result<TestCodex> {
let (config, fallback_cwd) = self
.prepare_config(base_url, &home, test_env.cwd().clone())
@@ -391,13 +425,19 @@ impl TestCodexBuilder {
std::env::current_exe()?,
codex_linux_sandbox_exe,
)?;
let environment_manager = Arc::new(
let environment_manager = Arc::new(if include_local_environment {
codex_exec_server::EnvironmentManager::create_for_tests_with_local(
exec_server_url,
local_runtime_paths,
)
.await
} else {
codex_exec_server::EnvironmentManager::create_for_tests(
exec_server_url,
local_runtime_paths,
)
.await,
);
.await
});
let file_system = test_env.environment().get_filesystem();
let mut workspace_setups = vec![];
swap(&mut self.workspace_setups, &mut workspace_setups);
@@ -788,9 +828,9 @@ impl TestCodexHarness {
Ok(Self { server, test })
}
pub async fn with_remote_aware_builder(mut builder: TestCodexBuilder) -> Result<Self> {
pub async fn with_remote_env_builder(mut builder: TestCodexBuilder) -> Result<Self> {
let server = start_mock_server().await;
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
Ok(Self { server, test })
}
+1 -1
View File
@@ -16,7 +16,7 @@ async fn agents_instructions(mut builder: TestCodexBuilder) -> Result<String> {
)
.await;
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
test.submit_turn("hello").await?;
let request = resp_mock.single_request();
+1 -1
View File
@@ -64,7 +64,7 @@ async fn apply_patch_harness_with(
});
// Box harness construction so apply_patch_cli tests do not inline the
// full test-thread startup path into each test future.
Box::pin(TestCodexHarness::with_remote_aware_builder(builder)).await
Box::pin(TestCodexHarness::with_remote_env_builder(builder)).await
}
async fn submit_without_wait(harness: &TestCodexHarness, prompt: &str) -> Result<()> {
+1
View File
@@ -244,6 +244,7 @@ async fn run_code_mode_turn_with_rmcp_config(
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -32,7 +32,7 @@ async fn hierarchical_agents_appends_to_project_doc_in_user_instructions() {
Ok::<(), anyhow::Error>(())
});
let test = builder
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await
.expect("build test codex");
@@ -76,7 +76,7 @@ async fn hierarchical_agents_emits_when_no_project_doc() {
.expect("test config should allow feature update");
});
let test = builder
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await
.expect("build test codex");
+1
View File
@@ -193,6 +193,7 @@ fn insert_rmcp_test_server(config: &mut Config, command: String, approval_mode:
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
+4 -4
View File
@@ -58,7 +58,7 @@ async fn unified_exec_test(server: &wiremock::MockServer) -> Result<TestCodex> {
"unified exec should enable for test: {result:?}",
);
});
builder.build_remote_aware(server).await
builder.build_with_remote_and_local_env(server).await
}
async fn submit_turn_with_approval_and_environments(
@@ -77,7 +77,7 @@ async fn submit_turn_with_approval_and_environments(
cwd: test.cwd.path().to_path_buf(),
approval_policy: AskForApproval::OnRequest,
approvals_reviewer: Some(ApprovalsReviewer::User),
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
sandbox_policy: SandboxPolicy::new_read_only_policy(),
permission_profile: None,
model: test.session_configured.model.clone(),
effort: None,
@@ -350,7 +350,7 @@ async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result<
let mut builder = test_codex().with_config(|config| {
config.include_apply_patch_tool = true;
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_and_local_env(&server).await?;
let local_cwd = TempDir::new()?;
let file_name = "apply_patch_remote_freeform.txt";
let remote_cwd = PathBuf::from(format!(
@@ -439,7 +439,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> {
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config.approvals_reviewer = ApprovalsReviewer::User;
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_and_local_env(&server).await?;
let local_cwd = TempDir::new()?;
let remote_cwd = PathBuf::from(format!(
"/tmp/codex-remote-apply-patch-approval-cwd-{}",
+15 -14
View File
@@ -332,6 +332,7 @@ fn insert_mcp_server(
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -479,7 +480,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
fixture
.codex
@@ -602,7 +603,7 @@ async fn stdio_server_uses_configured_cwd_before_runtime_fallback() -> anyhow::R
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
let expected_cwd = expected_cwd
@@ -655,7 +656,7 @@ async fn remote_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() -
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
let expected_cwd = expected_cwd
@@ -774,7 +775,7 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()>
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
wait_for_mcp_server(&fixture, server_name).await?;
@@ -872,7 +873,7 @@ async fn stdio_mcp_parallel_tool_calls_default_false_runs_serially() -> anyhow::
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
fixture
.codex
@@ -989,7 +990,7 @@ async fn stdio_mcp_parallel_tool_calls_opt_in_runs_concurrently() -> anyhow::Res
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
fixture
.codex
@@ -1070,7 +1071,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
wait_for_mcp_server(&fixture, server_name).await?;
@@ -1202,7 +1203,7 @@ async fn stdio_image_responses_preserve_original_detail_metadata() -> anyhow::Re
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
wait_for_mcp_server(&fixture, server_name).await?;
@@ -1337,7 +1338,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
fixture
@@ -1439,7 +1440,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> {
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
fixture
.codex
@@ -1557,7 +1558,7 @@ async fn stdio_server_propagates_explicit_local_env_var_source() -> anyhow::Resu
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
fixture
@@ -1649,7 +1650,7 @@ async fn remote_stdio_env_var_source_does_not_copy_local_env() -> anyhow::Result
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
fixture
@@ -1832,7 +1833,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> {
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
// Phase 4: submit the user turn that should trigger the MCP tool call.
fixture
@@ -2018,7 +2019,7 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> {
},
);
})
.build_remote_aware(&server)
.build_with_remote_env(&server)
.await?;
// Phase 5: wait for MCP startup before the turn is submitted, which keeps
// failures tied to server startup/discovery.
+3
View File
@@ -999,6 +999,7 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> {
enabled_tools: Some(vec!["echo".to_string(), "image".to_string()]),
disabled_tools: Some(vec!["image".to_string()]),
scopes: None,
oauth: None,
oauth_resource: None,
supports_parallel_tool_calls: false,
tools: HashMap::new(),
@@ -1127,6 +1128,7 @@ async fn tool_search_surfaced_mcp_tool_errors_are_returned_to_model() -> Result<
enabled_tools: Some(vec!["echo".to_string()]),
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
supports_parallel_tool_calls: false,
tools: HashMap::new(),
@@ -1272,6 +1274,7 @@ async fn tool_search_uses_non_app_mcp_server_instructions_as_namespace_descripti
enabled_tools: Some(vec!["echo".to_string()]),
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
supports_parallel_tool_calls: false,
tools: HashMap::new(),
+1 -1
View File
@@ -50,7 +50,7 @@ async fn user_turn_includes_skill_instructions() -> Result<()> {
let mut builder = test_codex().with_workspace_setup(move |cwd, fs| async move {
write_repo_skill(cwd, fs, "demo", "demo skill", skill_body).await
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let skill_path = test
.config
@@ -386,6 +386,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result<
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
+3
View File
@@ -398,6 +398,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()>
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -497,6 +498,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
@@ -779,6 +781,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> {
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
+24 -24
View File
@@ -379,7 +379,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let cwd = test.config.cwd.to_path_buf();
let call_id = "uexec-begin-event";
@@ -438,7 +438,7 @@ async fn unified_exec_resolves_relative_workdir() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let workdir_rel = std::path::PathBuf::from("uexec_relative_workdir");
let workdir = create_workspace_directory(&test, &workdir_rel).await?;
@@ -507,7 +507,7 @@ async fn unified_exec_respects_workdir_override() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let workdir = create_workspace_directory(&test, "uexec_workdir_test").await?;
@@ -572,7 +572,7 @@ async fn unified_exec_emits_exec_command_end_event() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec-end-event";
let args = json!({
@@ -645,7 +645,7 @@ async fn unified_exec_emits_output_delta_for_exec_command() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec-delta-1";
let args = json!({
@@ -703,7 +703,7 @@ async fn unified_exec_full_lifecycle_with_background_end_event() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec-full-lifecycle";
// This timing force the long-standing PTY
@@ -910,7 +910,7 @@ allow_local_binding = true
PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy_for_config),
);
});
let test = builder.build_remote_aware(server).await?;
let test = builder.build_with_remote_env(server).await?;
assert!(
test.config.permissions.network.is_some(),
"expected managed network proxy config to be present"
@@ -991,7 +991,7 @@ async fn unified_exec_emits_terminal_interaction_for_write_stdin() -> Result<()>
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let open_call_id = "uexec-open";
let open_args = json!({
@@ -1074,7 +1074,7 @@ async fn unified_exec_terminal_interaction_captures_delayed_output() -> Result<(
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let open_call_id = "uexec-delayed-open";
let open_args = json!({
@@ -1253,7 +1253,7 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let open_call_id = "uexec-open-session";
let open_args = json!({
@@ -1367,7 +1367,7 @@ async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec-metadata";
let args = serde_json::json!({
@@ -1462,7 +1462,7 @@ async fn exec_command_clamps_model_requested_max_output_tokens_to_policy() -> Re
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec-clamped-max-output";
let args = serde_json::json!({
@@ -1524,7 +1524,7 @@ async fn write_stdin_clamps_model_requested_max_output_tokens_to_policy() -> Res
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let start_call_id = "uexec-stdin-clamp-start";
let start_args = serde_json::json!({
@@ -1611,7 +1611,7 @@ async fn unified_exec_defaults_to_pipe() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec-default-pipe";
let args = serde_json::json!({
@@ -1680,7 +1680,7 @@ async fn unified_exec_can_enable_tty() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec-tty-enabled";
let args = serde_json::json!({
@@ -1746,7 +1746,7 @@ async fn unified_exec_respects_early_exit_notifications() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec-early-exit";
let args = serde_json::json!({
@@ -1829,7 +1829,7 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let start_call_id = "uexec-cat-start";
let send_call_id = "uexec-cat-send";
@@ -1983,7 +1983,7 @@ async fn unified_exec_emits_end_event_when_session_dies_via_stdin() -> Result<()
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let start_call_id = "uexec-end-on-exit-start";
let start_args = serde_json::json!({
@@ -2266,7 +2266,7 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let first_call_id = "uexec-start";
let first_args = serde_json::json!({
@@ -2365,7 +2365,7 @@ async fn unified_exec_streams_after_lagged_output() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let script = r#"python3 - <<'PY'
import sys
@@ -2485,7 +2485,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let first_call_id = "uexec-timeout";
let first_args = serde_json::json!({
@@ -2574,7 +2574,7 @@ async fn unified_exec_formats_large_output_summary() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let script = r#"python3 - <<'PY'
import sys
@@ -3002,7 +3002,7 @@ async fn unified_exec_runs_on_all_platforms() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let call_id = "uexec";
let args = serde_json::json!({
@@ -3066,7 +3066,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
.enable(Feature::UnifiedExec)
.expect("test config should allow feature update");
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
const MAX_SESSIONS_FOR_TEST: i32 = 64;
const FILLER_SESSIONS: i32 = MAX_SESSIONS_FOR_TEST - 1;
+14 -16
View File
@@ -174,7 +174,7 @@ async fn assert_user_turn_local_image_resizes_to(
let server = start_mock_server().await;
let mut builder = test_codex();
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -266,7 +266,7 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex();
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -562,7 +562,7 @@ async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<()
let server = start_mock_server().await;
let mut builder = test_codex();
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_and_local_env(&server).await?;
let local_cwd = TempDir::new()?;
fs::write(local_cwd.path().join("remote.png"), b"not a remote image")?;
let local_selection = TurnEnvironmentSelection {
@@ -582,9 +582,7 @@ async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<()
/*sandbox*/ None,
)
.await?;
let png = BASE64_STANDARD.decode(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII=",
)?;
let png = png_bytes(/*width*/ 1, /*height*/ 1, [0, 255, 0, 255])?;
test.fs()
.write_file(&image_path, png, /*sandbox*/ None)
.await?;
@@ -664,7 +662,7 @@ async fn view_image_tool_can_preserve_original_resolution_when_requested_on_gpt5
let server = start_mock_server().await;
let mut builder = test_codex().with_model("gpt-5.3-codex");
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -755,7 +753,7 @@ async fn view_image_tool_errors_clearly_for_unsupported_detail_values() -> anyho
let server = start_mock_server().await;
let mut builder = test_codex().with_model("gpt-5.3-codex");
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -833,7 +831,7 @@ async fn view_image_tool_treats_null_detail_as_omitted() -> anyhow::Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex().with_model("gpt-5.3-codex");
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -923,7 +921,7 @@ async fn view_image_tool_resizes_when_model_lacks_original_detail_support() -> a
let server = start_mock_server().await;
let mut builder = test_codex().with_model("gpt-5.2");
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -1017,7 +1015,7 @@ async fn view_image_tool_does_not_force_original_resolution_with_capability_only
let server = start_mock_server().await;
let mut builder = test_codex().with_model("gpt-5.3-codex");
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -1108,7 +1106,7 @@ async fn view_image_tool_errors_when_path_is_directory() -> anyhow::Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex();
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -1178,7 +1176,7 @@ async fn view_image_tool_errors_for_non_image_files() -> anyhow::Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex();
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
@@ -1255,7 +1253,7 @@ async fn view_image_tool_errors_when_file_missing() -> anyhow::Result<()> {
let server = start_mock_server().await;
let mut builder = test_codex();
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
config,
@@ -1385,7 +1383,7 @@ async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> an
.with_config(|config| {
config.model = Some(model_slug.to_string());
});
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex { codex, .. } = &test;
let rel_path = "assets/example.png";
@@ -1472,7 +1470,7 @@ async fn replaces_invalid_local_image_after_bad_request() -> anyhow::Result<()>
let completion_mock = responses::mount_sse_once(&server, success_response).await;
let mut builder = test_codex();
let test = builder.build_remote_aware(&server).await?;
let test = builder.build_with_remote_env(&server).await?;
let TestCodex {
codex,
session_configured,
+14
View File
@@ -135,6 +135,20 @@ impl EnvironmentManager {
}
}
/// Builds a test-only manager that keeps the provider default while also
/// allowing tests to select the local environment explicitly.
pub async fn create_for_tests_with_local(
exec_server_url: Option<String>,
local_runtime_paths: ExecServerRuntimePaths,
) -> Self {
let mut snapshot = DefaultEnvironmentProvider::new(exec_server_url).snapshot_inner();
snapshot.include_local = true;
match Self::from_provider_snapshot(snapshot, local_runtime_paths) {
Ok(manager) => manager,
Err(err) => panic!("test provider with local should create valid environments: {err}"),
}
}
/// Builds a manager from a provider-supplied startup snapshot.
pub async fn from_provider<P>(
provider: &P,
+1
View File
@@ -409,6 +409,7 @@ async fn thread_start_params_include_review_policy_when_review_policy_is_manual_
let codex_home = tempdir().expect("create temp codex home");
let cwd = tempdir().expect("create temp cwd");
let config = ConfigBuilder::default()
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
approvals_reviewer: Some(ApprovalsReviewer::User),
+19
View File
@@ -3,11 +3,13 @@ use crate::collaboration_mode_presets::builtin_collaboration_mode_presets;
use crate::config::ModelsManagerConfig;
use crate::model_info;
use async_trait::async_trait;
use codex_app_server_protocol::AuthMode;
use codex_login::AuthManager;
use codex_protocol::config_types::CollaborationModeMask;
use codex_protocol::error::Result as CoreResult;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::openai_models::ModelVisibility;
use codex_protocol::openai_models::ModelsResponse;
use std::fmt;
use std::path::PathBuf;
@@ -319,6 +321,23 @@ impl OpenAiModelsManager {
/// Replace the cached remote models and rebuild the derived presets list.
async fn apply_remote_models(&self, models: Vec<ModelInfo>) {
// Use the remote models list as the source of truth if it contains at least one
// non-hidden model and the user is using ChatGPT auth.
let should_use_remote_models_only = !models.is_empty()
&& models
.iter()
.any(|model| model.visibility == ModelVisibility::List)
&& self.auth_manager.as_ref().is_some_and(|auth_manager| {
matches!(
auth_manager.auth_mode(),
Some(AuthMode::Chatgpt | AuthMode::ChatgptAuthTokens)
)
});
if should_use_remote_models_only {
*self.remote_models.write().await = models;
return;
}
let mut existing_models = load_remote_models_from_file().unwrap_or_default();
for model in models {
if let Some(existing_index) = existing_models
@@ -366,6 +366,160 @@ async fn refresh_available_models_sorts_by_priority() {
assert_eq!(endpoint.fetch_count(), 1, "expected a single model fetch");
}
#[tokio::test]
async fn refresh_available_models_uses_remote_only_catalog_for_chatgpt_auth() {
let remote_models = vec![remote_model(
"chatgpt-visible-source-of-truth",
"ChatGPT Visible",
/*priority*/ 0,
)];
let codex_home = tempdir().expect("temp dir");
let endpoint = TestModelsEndpoint::new(vec![remote_models.clone()]);
let manager = openai_manager_for_tests(codex_home.path().to_path_buf(), endpoint.clone());
manager
.refresh_available_models(RefreshStrategy::OnlineIfUncached)
.await
.expect("refresh succeeds");
assert_eq!(manager.get_remote_models().await, remote_models);
assert_eq!(endpoint.fetch_count(), 1, "expected a single model fetch");
}
#[tokio::test]
async fn refresh_available_models_uses_cached_remote_only_catalog_for_chatgpt_auth() {
let remote_models = vec![remote_model(
"chatgpt-cached-source-of-truth",
"ChatGPT Cached",
/*priority*/ 0,
)];
let codex_home = tempdir().expect("temp dir");
let fetch_endpoint = TestModelsEndpoint::new(vec![remote_models.clone()]);
let fetch_manager =
openai_manager_for_tests(codex_home.path().to_path_buf(), fetch_endpoint.clone());
fetch_manager
.refresh_available_models(RefreshStrategy::OnlineIfUncached)
.await
.expect("initial refresh succeeds");
let cache_endpoint = TestModelsEndpoint::new(Vec::new());
let cache_manager =
openai_manager_for_tests(codex_home.path().to_path_buf(), cache_endpoint.clone());
cache_manager
.refresh_available_models(RefreshStrategy::OnlineIfUncached)
.await
.expect("cached refresh succeeds");
assert_eq!(cache_manager.get_remote_models().await, remote_models);
assert_eq!(
cache_endpoint.fetch_count(),
0,
"fresh cache should avoid a model fetch"
);
}
#[tokio::test]
async fn get_model_info_uses_fallback_for_bundled_models_when_chatgpt_remote_is_authoritative() {
let remote_models = vec![remote_model(
"chatgpt-authoritative-model-info",
"ChatGPT Model Info",
/*priority*/ 0,
)];
let codex_home = tempdir().expect("temp dir");
let endpoint = TestModelsEndpoint::new(vec![remote_models]);
let manager = openai_manager_for_tests(codex_home.path().to_path_buf(), endpoint);
let bundled_slug = load_remote_models_from_file()
.expect("bundled models should parse")
.first()
.expect("bundled models should contain at least one model")
.slug
.clone();
manager
.refresh_available_models(RefreshStrategy::OnlineIfUncached)
.await
.expect("refresh succeeds");
let model_info = manager
.get_model_info(&bundled_slug, &ModelsManagerConfig::default())
.await;
assert_eq!(model_info.slug, bundled_slug);
assert!(model_info.used_fallback_model_metadata);
}
#[tokio::test]
async fn refresh_available_models_preserves_bundled_catalog_for_empty_chatgpt_remote() {
let codex_home = tempdir().expect("temp dir");
let endpoint = TestModelsEndpoint::new(vec![Vec::new()]);
let manager = openai_manager_for_tests(codex_home.path().to_path_buf(), endpoint);
let expected = load_remote_models_from_file().expect("bundled models should parse");
manager
.refresh_available_models(RefreshStrategy::OnlineIfUncached)
.await
.expect("refresh succeeds");
assert_eq!(manager.get_remote_models().await, expected);
}
#[tokio::test]
async fn refresh_available_models_merges_hidden_only_chatgpt_remote_with_bundled_catalog() {
let hidden_remote = remote_model_with_visibility(
"chatgpt-hidden-only",
"ChatGPT Hidden",
/*priority*/ 0,
"hide",
);
let codex_home = tempdir().expect("temp dir");
let endpoint = TestModelsEndpoint::new(vec![vec![hidden_remote.clone()]]);
let manager = openai_manager_for_tests(codex_home.path().to_path_buf(), endpoint);
let mut expected = load_remote_models_from_file().expect("bundled models should parse");
expected.push(hidden_remote);
manager
.refresh_available_models(RefreshStrategy::OnlineIfUncached)
.await
.expect("refresh succeeds");
assert_eq!(manager.get_remote_models().await, expected);
}
#[tokio::test]
async fn refresh_available_models_keeps_merging_for_api_auth() {
let remote_models = vec![remote_model(
"api-auth-visible-remote",
"API Auth Visible",
/*priority*/ 0,
)];
let codex_home = tempdir().expect("temp dir");
let endpoint = Arc::new(TestModelsEndpoint {
has_command_auth: true,
uses_codex_backend: false,
responses: Mutex::new(vec![remote_models.clone()].into()),
fetch_count: AtomicUsize::new(0),
});
let manager = openai_manager_for_tests_with_auth(
codex_home.path().to_path_buf(),
endpoint.clone(),
Some(AuthManager::from_auth_for_testing(CodexAuth::from_api_key(
"test-api-key",
))),
);
let mut expected = load_remote_models_from_file().expect("bundled models should parse");
expected.extend(remote_models);
manager
.refresh_available_models(RefreshStrategy::OnlineIfUncached)
.await
.expect("refresh succeeds");
assert_eq!(manager.get_remote_models().await, expected);
assert_eq!(endpoint.fetch_count(), 1, "expected a single model fetch");
}
#[tokio::test]
async fn refresh_available_models_uses_cache_when_fresh() {
let remote_models = vec![remote_model("cached", "Cached", /*priority*/ 5)];
+125 -4
View File
@@ -11,6 +11,9 @@ use base64::Engine;
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
use reqwest::ClientBuilder;
use reqwest::Url;
use rmcp::transport::AuthorizationManager;
use rmcp::transport::AuthorizationSession;
use rmcp::transport::auth::OAuthClientConfig;
use rmcp::transport::auth::OAuthState;
use sha2::Digest;
use sha2::Sha256;
@@ -81,6 +84,7 @@ pub async fn perform_oauth_login(
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
scopes: &[String],
oauth_client_id: Option<&str>,
oauth_resource: Option<&str>,
callback_port: Option<u16>,
callback_url: Option<&str>,
@@ -92,6 +96,7 @@ pub async fn perform_oauth_login(
http_headers,
env_http_headers,
scopes,
oauth_client_id,
oauth_resource,
callback_port,
callback_url,
@@ -108,6 +113,7 @@ pub async fn perform_oauth_login_silent(
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
scopes: &[String],
oauth_client_id: Option<&str>,
oauth_resource: Option<&str>,
callback_port: Option<u16>,
callback_url: Option<&str>,
@@ -119,6 +125,7 @@ pub async fn perform_oauth_login_silent(
http_headers,
env_http_headers,
scopes,
oauth_client_id,
oauth_resource,
callback_port,
callback_url,
@@ -135,6 +142,7 @@ async fn perform_oauth_login_with_browser_output(
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
scopes: &[String],
oauth_client_id: Option<&str>,
oauth_resource: Option<&str>,
callback_port: Option<u16>,
callback_url: Option<&str>,
@@ -150,6 +158,7 @@ async fn perform_oauth_login_with_browser_output(
store_mode,
headers,
scopes,
oauth_client_id,
oauth_resource,
/*launch_browser*/ true,
callback_port,
@@ -169,6 +178,7 @@ pub async fn perform_oauth_login_return_url(
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
scopes: &[String],
oauth_client_id: Option<&str>,
oauth_resource: Option<&str>,
timeout_secs: Option<i64>,
callback_port: Option<u16>,
@@ -184,6 +194,7 @@ pub async fn perform_oauth_login_return_url(
store_mode,
headers,
scopes,
oauth_client_id,
oauth_resource,
/*launch_browser*/ false,
callback_port,
@@ -436,6 +447,7 @@ impl OauthLoginFlow {
store_mode: OAuthCredentialsStoreMode,
headers: OauthHeaders,
scopes: &[String],
oauth_client_id: Option<&str>,
oauth_resource: Option<&str>,
launch_browser: bool,
callback_port: Option<u16>,
@@ -471,11 +483,15 @@ impl OauthLoginFlow {
let default_headers = build_default_headers(http_headers, env_http_headers)?;
let http_client = apply_default_headers(ClientBuilder::new(), &default_headers).build()?;
let mut oauth_state = OAuthState::new(server_url, Some(http_client)).await?;
let scope_refs: Vec<&str> = scopes.iter().map(String::as_str).collect();
oauth_state
.start_authorization(&scope_refs, &redirect_uri, Some("Codex"))
.await?;
let oauth_state = start_authorization(
server_url,
http_client,
&scope_refs,
&redirect_uri,
oauth_client_id,
)
.await?;
let auth_url = append_query_param(
&oauth_state.get_authorization_url().await?,
"resource",
@@ -585,6 +601,41 @@ impl OauthLoginFlow {
}
}
async fn start_authorization(
server_url: &str,
http_client: reqwest::Client,
scopes: &[&str],
redirect_uri: &str,
oauth_client_id: Option<&str>,
) -> Result<OAuthState> {
let Some(oauth_client_id) = oauth_client_id.filter(|client_id| !client_id.trim().is_empty())
else {
let mut oauth_state = OAuthState::new(server_url, Some(http_client)).await?;
oauth_state
.start_authorization(scopes, redirect_uri, Some("Codex"))
.await?;
return Ok(oauth_state);
};
let mut auth_manager = AuthorizationManager::new(server_url).await?;
auth_manager.with_client(http_client)?;
let metadata = auth_manager.discover_metadata().await?;
auth_manager.set_metadata(metadata);
auth_manager.configure_client(OAuthClientConfig {
client_id: oauth_client_id.to_string(),
client_secret: None,
scopes: scopes.iter().map(|scope| (*scope).to_string()).collect(),
redirect_uri: redirect_uri.to_string(),
})?;
let auth_url = auth_manager.get_authorization_url(scopes).await?;
Ok(OAuthState::Session(AuthorizationSession {
auth_manager,
auth_url,
redirect_uri: redirect_uri.to_string(),
}))
}
fn append_query_param(url: &str, key: &str, value: Option<&str>) -> String {
let Some(value) = value else {
return url.to_string();
@@ -604,7 +655,13 @@ fn append_query_param(url: &str, key: &str, value: Option<&str>) -> String {
#[cfg(test)]
mod tests {
use axum::Json;
use axum::Router;
use axum::routing::get;
use pretty_assertions::assert_eq;
use reqwest::Url;
use serde_json::json;
use tokio::net::TcpListener;
use super::CallbackOutcome;
use super::OAuthProviderError;
@@ -613,6 +670,70 @@ mod tests {
use super::callback_id_from_server_url;
use super::callback_path_from_redirect_uri;
use super::parse_oauth_callback;
use super::start_authorization;
async fn spawn_oauth_metadata_server() -> String {
let listener = TcpListener::bind("127.0.0.1:0")
.await
.expect("bind metadata listener");
let addr = listener.local_addr().expect("read metadata listener addr");
let base_url = format!("http://{addr}");
let metadata = json!({
"authorization_endpoint": format!("{base_url}/oauth/authorize"),
"token_endpoint": format!("{base_url}/oauth/token"),
"scopes_supported": [""],
});
let path_scoped_metadata = metadata.clone();
let app = Router::new()
.route(
"/.well-known/oauth-authorization-server/mcp",
get(move || {
let metadata = path_scoped_metadata.clone();
async move { Json(metadata) }
}),
)
.route(
"/.well-known/oauth-authorization-server",
get(move || {
let metadata = metadata.clone();
async move { Json(metadata) }
}),
);
tokio::spawn(async move {
axum::serve(listener, app)
.await
.expect("serve oauth metadata");
});
base_url
}
#[tokio::test]
async fn start_authorization_uses_configured_client_id() {
let base_url = spawn_oauth_metadata_server().await;
let oauth_state = start_authorization(
&format!("{base_url}/mcp"),
reqwest::Client::new(),
&[],
"http://127.0.0.1/callback",
Some("eci-prd-pub-codex-123"),
)
.await
.expect("start oauth authorization");
let authorization_url = oauth_state
.get_authorization_url()
.await
.expect("read authorization url");
let auth_url = Url::parse(&authorization_url).expect("authorization url should parse");
let client_id = auth_url
.query_pairs()
.find(|(key, _)| key == "client_id")
.map(|(_, value)| value.into_owned());
assert_eq!(client_id.as_deref(), Some("eci-prd-pub-codex-123"));
}
#[test]
fn parse_oauth_callback_accepts_default_path() {
@@ -47,6 +47,7 @@ use codex_app_server_protocol::FileSystemSandboxEntry;
use codex_app_server_protocol::FileSystemSpecialPath;
use codex_app_server_protocol::McpServerElicitationAction;
use codex_app_server_protocol::NetworkApprovalContext;
use codex_app_server_protocol::NetworkApprovalProtocol;
use codex_app_server_protocol::NetworkPolicyRuleAction;
use codex_app_server_protocol::RequestId;
use codex_features::Features;
@@ -354,8 +355,25 @@ impl ApprovalOverlay {
return;
};
if request.thread_label().is_none() {
let subject = match request {
ApprovalRequest::Exec {
network_approval_context: Some(network_approval_context),
..
} => history_cell::ApprovalDecisionSubject::NetworkAccess {
target: network_approval_target(network_approval_context, command),
},
_ => {
if let Some(target) = network_approval_command_target(command) {
history_cell::ApprovalDecisionSubject::NetworkAccess {
target: target.to_string(),
}
} else {
history_cell::ApprovalDecisionSubject::Command(command.to_vec())
}
}
};
let cell = history_cell::new_approval_decision_cell(
command.to_vec(),
subject,
command_decision_to_review_decision(&decision),
history_cell::ApprovalDecisionActor::User,
);
@@ -623,6 +641,35 @@ fn approval_footer_hint(
Line::from(spans)
}
fn network_approval_target(
network_approval_context: &NetworkApprovalContext,
command: &[String],
) -> String {
if let Some(target) = network_approval_command_target(command) {
return target.to_string();
}
let scheme = match network_approval_context.protocol {
NetworkApprovalProtocol::Http => "http",
NetworkApprovalProtocol::Https => "https",
NetworkApprovalProtocol::Socks5Tcp => "socks5-tcp",
NetworkApprovalProtocol::Socks5Udp => "socks5-udp",
};
format!("{scheme}://{}", network_approval_context.host)
}
fn network_approval_command_target(command: &[String]) -> Option<&str> {
match command {
[program, target] if program == "network-access" && !target.is_empty() => {
Some(target.as_str())
}
[command] => command
.strip_prefix("network-access ")
.filter(|target| !target.is_empty()),
_ => None,
}
}
fn build_header(request: &ApprovalRequest) -> Box<dyn Renderable> {
match request {
ApprovalRequest::Exec {
@@ -1102,6 +1149,21 @@ mod tests {
.join("\n")
}
fn render_history_cell_lines(
cell: &dyn crate::history_cell::HistoryCell,
width: u16,
) -> Vec<String> {
cell.display_lines(width)
.iter()
.map(|line| {
line.spans
.iter()
.map(|span| span.content.as_ref())
.collect::<String>()
})
.collect()
}
fn normalize_snapshot_paths(rendered: String) -> String {
[
(absolute_path("/tmp/readme.txt"), "/tmp/readme.txt"),
@@ -2111,7 +2173,7 @@ mod tests {
"git add tui/src/render/mod.rs tui/src/render/renderable.rs".into(),
];
let cell = history_cell::new_approval_decision_cell(
command,
history_cell::ApprovalDecisionSubject::Command(command),
ReviewDecision::Approved,
history_cell::ApprovalDecisionActor::User,
);
@@ -2134,6 +2196,73 @@ mod tests {
assert_eq!(rendered, expected);
}
#[test]
fn exec_history_cell_does_not_render_blank_action_for_empty_command() {
let approved = history_cell::new_approval_decision_cell(
history_cell::ApprovalDecisionSubject::Command(Vec::new()),
ReviewDecision::Approved,
history_cell::ApprovalDecisionActor::User,
);
assert_eq!(
render_history_cell_lines(approved.as_ref(), /*width*/ 80),
vec!["✔ You approved this request this time".to_string()]
);
let approved_for_session = history_cell::new_approval_decision_cell(
history_cell::ApprovalDecisionSubject::Command(Vec::new()),
ReviewDecision::ApprovedForSession,
history_cell::ApprovalDecisionActor::User,
);
assert_eq!(
render_history_cell_lines(approved_for_session.as_ref(), /*width*/ 80),
vec!["✔ You approved this request every time this session".to_string()]
);
}
#[test]
fn network_access_command_history_uses_target_without_structured_context() {
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let mut view = make_overlay(
ApprovalRequest::Exec {
thread_id: ThreadId::new(),
thread_label: None,
id: "test".into(),
command: vec![
"network-access".to_string(),
"https://example.com:8443".to_string(),
],
reason: None,
available_decisions: vec![
CommandExecutionApprovalDecision::Accept,
CommandExecutionApprovalDecision::Cancel,
],
network_approval_context: None,
additional_permissions: None,
},
tx,
Features::with_defaults(),
);
view.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
let mut decision = None;
while let Ok(event) = rx.try_recv() {
if let AppEvent::InsertHistoryCell(cell) = event {
decision = Some(cell);
break;
}
}
let decision = decision.expect("expected decision cell in history");
assert_eq!(
render_history_cell_lines(decision.as_ref(), /*width*/ 80),
vec![
"✔ You approved codex network access to https://example.com:8443 this time"
.to_string(),
]
);
}
#[test]
fn esc_cancels_mcp_elicitation() {
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
@@ -145,6 +145,129 @@ fn app_server_exec_approval_request_preserves_permissions_context() {
);
}
#[tokio::test]
async fn network_exec_approval_history_describes_session_host_allowance() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
let request = exec_approval_request_from_params(
AppServerCommandExecutionRequestApprovalParams {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "item-1".to_string(),
started_at_ms: 0,
approval_id: Some("approval-1".to_string()),
reason: None,
network_approval_context: Some(codex_app_server_protocol::NetworkApprovalContext {
host: "example.com".to_string(),
protocol: codex_app_server_protocol::NetworkApprovalProtocol::Https,
}),
command: Some("network-access https://example.com:8443".to_string()),
cwd: None,
command_actions: None,
additional_permissions: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
available_decisions: Some(vec![
codex_app_server_protocol::CommandExecutionApprovalDecision::AcceptForSession,
codex_app_server_protocol::CommandExecutionApprovalDecision::Cancel,
]),
},
&test_path_buf("/tmp").abs(),
);
handle_exec_approval_request(&mut chat, "sub-network", request);
chat.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE));
let decision = drain_insert_history(&mut rx)
.pop()
.expect("expected decision cell in history");
assert_snapshot!(
"network_exec_approval_history_session_host_allowance",
lines_to_single_string(&decision)
);
}
#[tokio::test]
async fn network_exec_approval_history_describes_one_time_host_allowance() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
let request = exec_approval_request_from_params(
AppServerCommandExecutionRequestApprovalParams {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "item-1".to_string(),
started_at_ms: 0,
approval_id: Some("approval-1".to_string()),
reason: None,
network_approval_context: Some(codex_app_server_protocol::NetworkApprovalContext {
host: "example.com".to_string(),
protocol: codex_app_server_protocol::NetworkApprovalProtocol::Http,
}),
command: None,
cwd: None,
command_actions: None,
additional_permissions: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
available_decisions: Some(vec![
codex_app_server_protocol::CommandExecutionApprovalDecision::Accept,
codex_app_server_protocol::CommandExecutionApprovalDecision::Cancel,
]),
},
&test_path_buf("/tmp").abs(),
);
handle_exec_approval_request(&mut chat, "sub-network", request);
chat.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
let decision = drain_insert_history(&mut rx)
.pop()
.expect("expected decision cell in history");
assert_snapshot!(
"network_exec_approval_history_one_time_host_allowance",
lines_to_single_string(&decision)
);
}
#[tokio::test]
async fn network_exec_approval_history_describes_canceled_host_request() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
let request = exec_approval_request_from_params(
AppServerCommandExecutionRequestApprovalParams {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "item-1".to_string(),
started_at_ms: 0,
approval_id: Some("approval-1".to_string()),
reason: None,
network_approval_context: Some(codex_app_server_protocol::NetworkApprovalContext {
host: "example.com".to_string(),
protocol: codex_app_server_protocol::NetworkApprovalProtocol::Socks5Tcp,
}),
command: Some("network-access socks5-tcp://example.com:1080".to_string()),
cwd: None,
command_actions: None,
additional_permissions: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
available_decisions: Some(vec![
codex_app_server_protocol::CommandExecutionApprovalDecision::Accept,
codex_app_server_protocol::CommandExecutionApprovalDecision::Cancel,
]),
},
&test_path_buf("/tmp").abs(),
);
handle_exec_approval_request(&mut chat, "sub-network", request);
chat.handle_key_event(KeyEvent::new(KeyCode::Char('n'), KeyModifiers::NONE));
let decision = drain_insert_history(&mut rx)
.pop()
.expect("expected decision cell in history");
assert_snapshot!(
"network_exec_approval_history_canceled_host_request",
lines_to_single_string(&decision)
);
}
#[test]
fn app_server_request_permissions_preserves_file_system_permissions() {
let read_path = AbsolutePathBuf::try_from(PathBuf::from(test_path_display("/tmp/read-only")))
@@ -0,0 +1,6 @@
---
source: tui/src/chatwidget/tests/approval_requests.rs
expression: lines_to_single_string(&decision)
---
✗ You canceled the request for codex network access to
socks5-tcp://example.com:1080
@@ -0,0 +1,5 @@
---
source: tui/src/chatwidget/tests/approval_requests.rs
expression: lines_to_single_string(&decision)
---
✔ You approved codex network access to http://example.com this time
@@ -0,0 +1,6 @@
---
source: tui/src/chatwidget/tests/approval_requests.rs
expression: lines_to_single_string(&decision)
---
✔ You approved codex network access to https://example.com:8443 every time this
session
+3 -3
View File
@@ -149,7 +149,7 @@ impl ChatWidget {
if ev.status == GuardianAssessmentStatus::Approved {
let cell = if let Some(command) = guardian_command(&ev.action) {
history_cell::new_approval_decision_cell(
command,
history_cell::ApprovalDecisionSubject::Command(command),
crate::history_cell::ReviewDecision::Approved,
history_cell::ApprovalDecisionActor::Guardian,
)
@@ -169,7 +169,7 @@ impl ChatWidget {
if ev.status == GuardianAssessmentStatus::TimedOut {
let cell = if let Some(command) = guardian_command(&ev.action) {
history_cell::new_approval_decision_cell(
command,
history_cell::ApprovalDecisionSubject::Command(command),
crate::history_cell::ReviewDecision::TimedOut,
history_cell::ApprovalDecisionActor::Guardian,
)
@@ -213,7 +213,7 @@ impl ChatWidget {
self.review.recent_auto_review_denials.push(ev.clone());
let cell = if let Some(command) = guardian_command(&ev.action) {
history_cell::new_approval_decision_cell(
command,
history_cell::ApprovalDecisionSubject::Command(command),
crate::history_cell::ReviewDecision::Denied,
history_cell::ApprovalDecisionActor::Guardian,
)
+174 -65
View File
@@ -1089,6 +1089,11 @@ fn exec_snippet(command: &[String]) -> String {
truncate_exec_snippet(&full_cmd)
}
fn non_empty_exec_snippet(command: &[String]) -> Option<String> {
let snippet = exec_snippet(command);
(!snippet.is_empty()).then_some(snippet)
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum ReviewDecision {
Approved,
@@ -1104,8 +1109,14 @@ pub(crate) enum ReviewDecision {
Abort,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) enum ApprovalDecisionSubject {
Command(Vec<String>),
NetworkAccess { target: String },
}
pub fn new_approval_decision_cell(
command: Vec<String>,
subject: ApprovalDecisionSubject,
decision: ReviewDecision,
actor: ApprovalDecisionActor,
) -> Box<dyn HistoryCell> {
@@ -1113,19 +1124,37 @@ pub fn new_approval_decision_cell(
use codex_protocol::approvals::NetworkPolicyRuleAction;
let (symbol, summary): (Span<'static>, Vec<Span<'static>>) = match decision {
Approved => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
Approved => match subject {
ApprovalDecisionSubject::Command(command) => {
let summary = if let Some(snippet) = non_empty_exec_snippet(&command) {
vec![
actor.subject().into(),
"approved".bold(),
" codex to run ".into(),
Span::from(snippet).dim(),
" this time".bold(),
]
} else {
vec![
actor.subject().into(),
"approved".bold(),
" this request".into(),
" this time".bold(),
]
};
("✔ ".green(), summary)
}
ApprovalDecisionSubject::NetworkAccess { target } => (
"✔ ".green(),
vec![
actor.subject().into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" codex network access to ".into(),
Span::from(target).dim(),
" this time".bold(),
],
)
}
),
},
ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment,
} => {
@@ -1140,84 +1169,164 @@ pub fn new_approval_decision_cell(
],
)
}
ApprovedForSession => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
ApprovedForSession => match subject {
ApprovalDecisionSubject::Command(command) => {
let summary = if let Some(snippet) = non_empty_exec_snippet(&command) {
vec![
actor.subject().into(),
"approved".bold(),
" codex to run ".into(),
Span::from(snippet).dim(),
" every time this session".bold(),
]
} else {
vec![
actor.subject().into(),
"approved".bold(),
" this request".into(),
" every time this session".bold(),
]
};
("✔ ".green(), summary)
}
ApprovalDecisionSubject::NetworkAccess { target } => (
"✔ ".green(),
vec![
actor.subject().into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" every time this session".bold(),
],
)
}
NetworkPolicyAmendment {
network_policy_amendment,
} => match network_policy_amendment.action {
NetworkPolicyRuleAction::Allow => (
"✔ ".green(),
vec![
actor.subject().into(),
"persisted".bold(),
" Codex network access to ".into(),
Span::from(network_policy_amendment.host).dim(),
],
),
NetworkPolicyRuleAction::Deny => (
"✗ ".red(),
vec![
actor.subject().into(),
"denied".bold(),
" codex network access to ".into(),
Span::from(network_policy_amendment.host).dim(),
" and saved that rule".into(),
Span::from(target).dim(),
" every time this session".bold(),
],
),
},
Denied => {
let snippet = Span::from(exec_snippet(&command)).dim();
let summary = match actor {
ApprovalDecisionActor::User => vec![
NetworkPolicyAmendment {
network_policy_amendment,
} => {
let target = match subject {
ApprovalDecisionSubject::NetworkAccess { target } => target,
ApprovalDecisionSubject::Command(_) => network_policy_amendment.host,
};
match network_policy_amendment.action {
NetworkPolicyRuleAction::Allow => (
"✔ ".green(),
vec![
actor.subject().into(),
"persisted".bold(),
" Codex network access to ".into(),
Span::from(target).dim(),
],
),
NetworkPolicyRuleAction::Deny => (
"✗ ".red(),
vec![
actor.subject().into(),
"denied".bold(),
" codex network access to ".into(),
Span::from(target).dim(),
" and saved that rule".into(),
],
),
}
}
Denied => match subject {
ApprovalDecisionSubject::Command(command) => {
let summary = if let Some(snippet) = non_empty_exec_snippet(&command) {
let snippet = Span::from(snippet).dim();
match actor {
ApprovalDecisionActor::User => vec![
actor.subject().into(),
"did not approve".bold(),
" codex to run ".into(),
snippet,
],
ApprovalDecisionActor::Guardian => vec![
"Request ".into(),
"denied".bold(),
" for codex to run ".into(),
snippet,
],
}
} else {
match actor {
ApprovalDecisionActor::User => vec![
actor.subject().into(),
"did not approve".bold(),
" this request".into(),
],
ApprovalDecisionActor::Guardian => {
vec!["Request ".into(), "denied".bold()]
}
}
};
("✗ ".red(), summary)
}
ApprovalDecisionSubject::NetworkAccess { target } => (
"✗ ".red(),
vec![
actor.subject().into(),
"did not approve".bold(),
" codex to run ".into(),
snippet,
" codex network access to ".into(),
Span::from(target).dim(),
],
ApprovalDecisionActor::Guardian => vec![
"Request ".into(),
"denied".bold(),
" for codex to run ".into(),
snippet,
],
};
("✗ ".red(), summary)
}
TimedOut => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
),
},
TimedOut => match subject {
ApprovalDecisionSubject::Command(command) => {
let summary = if let Some(snippet) = non_empty_exec_snippet(&command) {
vec![
"Review ".into(),
"timed out".bold(),
" before codex could run ".into(),
Span::from(snippet).dim(),
]
} else {
vec![
"Review ".into(),
"timed out".bold(),
" before this request could be approved".into(),
]
};
("✗ ".red(), summary)
}
ApprovalDecisionSubject::NetworkAccess { target } => (
"✗ ".red(),
vec![
"Review ".into(),
"timed out".bold(),
" before codex could run ".into(),
snippet,
" before codex could access ".into(),
Span::from(target).dim(),
],
)
}
Abort => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
),
},
Abort => match subject {
ApprovalDecisionSubject::Command(command) => {
let summary = if let Some(snippet) = non_empty_exec_snippet(&command) {
vec![
actor.subject().into(),
"canceled".bold(),
" the request to run ".into(),
Span::from(snippet).dim(),
]
} else {
vec![
actor.subject().into(),
"canceled".bold(),
" this request".into(),
]
};
("✗ ".red(), summary)
}
ApprovalDecisionSubject::NetworkAccess { target } => (
"✗ ".red(),
vec![
actor.subject().into(),
"canceled".bold(),
" the request to run ".into(),
snippet,
" the request for codex network access to ".into(),
Span::from(target).dim(),
],
)
}
),
},
};
Box::new(PrefixedWrappedHistoryCell::new(
+1 -1
View File
@@ -1133,7 +1133,7 @@ mod tests {
cells.push(apply_begin_cell);
let apply_end_cell: Arc<dyn HistoryCell> = history_cell::new_approval_decision_cell(
vec!["ls".into()],
history_cell::ApprovalDecisionSubject::Command(vec!["ls".into()]),
ReviewDecision::Approved,
history_cell::ApprovalDecisionActor::User,
)