mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
fix(apps): persist codex mcp affinity cookies
This commit is contained in:
@@ -74,7 +74,7 @@ once_cell = { workspace = true }
|
||||
os_info = { workspace = true }
|
||||
rand = { workspace = true }
|
||||
regex-lite = { workspace = true }
|
||||
reqwest = { workspace = true, features = ["json", "stream"] }
|
||||
reqwest = { workspace = true, features = ["cookies", "json", "stream"] }
|
||||
rmcp = { workspace = true, default-features = false, features = [
|
||||
"base64",
|
||||
"macros",
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex as StdMutex;
|
||||
|
||||
use reqwest::cookie::Jar;
|
||||
|
||||
use crate::CodexAuth;
|
||||
use crate::config::Config;
|
||||
use crate::mcp::codex_apps_mcp_url;
|
||||
|
||||
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
|
||||
struct AppsMcpCookieStoreKey {
|
||||
url: String,
|
||||
account_id: Option<String>,
|
||||
chatgpt_user_id: Option<String>,
|
||||
is_workspace_account: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone, Default)]
|
||||
pub struct AppsMcpCookieStore {
|
||||
jars: Arc<StdMutex<HashMap<AppsMcpCookieStoreKey, Arc<Jar>>>>,
|
||||
}
|
||||
|
||||
impl AppsMcpCookieStore {
|
||||
pub fn jar_for(&self, config: &Config, auth: Option<&CodexAuth>) -> Arc<Jar> {
|
||||
let token_data = auth.and_then(|auth| auth.get_token_data().ok());
|
||||
self.jar_for_identity(
|
||||
codex_apps_mcp_url(config),
|
||||
token_data
|
||||
.as_ref()
|
||||
.and_then(|token_data| token_data.account_id.clone()),
|
||||
token_data
|
||||
.as_ref()
|
||||
.and_then(|token_data| token_data.id_token.chatgpt_user_id.clone()),
|
||||
token_data
|
||||
.as_ref()
|
||||
.is_some_and(|token_data| token_data.id_token.is_workspace_account()),
|
||||
)
|
||||
}
|
||||
|
||||
fn jar_for_identity(
|
||||
&self,
|
||||
url: String,
|
||||
account_id: Option<String>,
|
||||
chatgpt_user_id: Option<String>,
|
||||
is_workspace_account: bool,
|
||||
) -> Arc<Jar> {
|
||||
let key = AppsMcpCookieStoreKey {
|
||||
url,
|
||||
account_id,
|
||||
chatgpt_user_id,
|
||||
is_workspace_account,
|
||||
};
|
||||
let mut jars = self
|
||||
.jars
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
Arc::clone(jars.entry(key).or_insert_with(|| Arc::new(Jar::default())))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn jar_for_identity_reuses_matching_key() {
|
||||
let store = AppsMcpCookieStore::default();
|
||||
let left = store.jar_for_identity(
|
||||
"https://example.test/apps".to_string(),
|
||||
Some("account-1".to_string()),
|
||||
Some("user-1".to_string()),
|
||||
false,
|
||||
);
|
||||
let right = store.jar_for_identity(
|
||||
"https://example.test/apps".to_string(),
|
||||
Some("account-1".to_string()),
|
||||
Some("user-1".to_string()),
|
||||
false,
|
||||
);
|
||||
|
||||
assert!(Arc::ptr_eq(&left, &right));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn jar_for_identity_separates_account_boundaries() {
|
||||
let store = AppsMcpCookieStore::default();
|
||||
let left = store.jar_for_identity(
|
||||
"https://example.test/apps".to_string(),
|
||||
Some("account-1".to_string()),
|
||||
Some("user-1".to_string()),
|
||||
false,
|
||||
);
|
||||
let right = store.jar_for_identity(
|
||||
"https://example.test/apps".to_string(),
|
||||
Some("account-2".to_string()),
|
||||
Some("user-1".to_string()),
|
||||
false,
|
||||
);
|
||||
|
||||
assert!(!Arc::ptr_eq(&left, &right));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn jar_for_identity_separates_workspace_boundaries() {
|
||||
let store = AppsMcpCookieStore::default();
|
||||
let left = store.jar_for_identity(
|
||||
"https://example.test/apps".to_string(),
|
||||
Some("account-1".to_string()),
|
||||
Some("user-1".to_string()),
|
||||
false,
|
||||
);
|
||||
let right = store.jar_for_identity(
|
||||
"https://example.test/apps".to_string(),
|
||||
Some("account-1".to_string()),
|
||||
Some("user-1".to_string()),
|
||||
true,
|
||||
);
|
||||
|
||||
assert!(!Arc::ptr_eq(&left, &right));
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@ use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicU64;
|
||||
|
||||
use crate::AppsMcpCookieStore;
|
||||
use crate::AuthManager;
|
||||
use crate::CodexAuth;
|
||||
use crate::SandboxState;
|
||||
@@ -356,6 +357,7 @@ impl Codex {
|
||||
pub(crate) async fn spawn(
|
||||
mut config: Config,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
apps_mcp_cookie_store: Arc<AppsMcpCookieStore>,
|
||||
models_manager: Arc<ModelsManager>,
|
||||
skills_manager: Arc<SkillsManager>,
|
||||
plugins_manager: Arc<PluginsManager>,
|
||||
@@ -539,6 +541,7 @@ impl Codex {
|
||||
session_configuration,
|
||||
config.clone(),
|
||||
auth_manager.clone(),
|
||||
apps_mcp_cookie_store,
|
||||
models_manager.clone(),
|
||||
exec_policy,
|
||||
tx_event.clone(),
|
||||
@@ -1257,6 +1260,7 @@ impl Session {
|
||||
mut session_configuration: SessionConfiguration,
|
||||
config: Arc<Config>,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
apps_mcp_cookie_store: Arc<AppsMcpCookieStore>,
|
||||
models_manager: Arc<ModelsManager>,
|
||||
exec_policy: ExecPolicyManager,
|
||||
tx_event: Sender<Event>,
|
||||
@@ -1427,6 +1431,9 @@ impl Session {
|
||||
}
|
||||
|
||||
let auth = auth.as_ref();
|
||||
let apps_mcp_cookie_jar = mcp_servers
|
||||
.contains_key(CODEX_APPS_MCP_SERVER_NAME)
|
||||
.then(|| apps_mcp_cookie_store.jar_for(config.as_ref(), auth));
|
||||
let auth_mode = auth.map(CodexAuth::auth_mode).map(TelemetryAuthMode::from);
|
||||
let account_id = auth.and_then(CodexAuth::get_account_id);
|
||||
let account_email = auth.and_then(CodexAuth::get_account_email);
|
||||
@@ -1612,6 +1619,7 @@ impl Session {
|
||||
&config.permissions.approval_policy,
|
||||
))),
|
||||
mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()),
|
||||
apps_mcp_cookie_store,
|
||||
unified_exec_manager: UnifiedExecProcessManager::new(
|
||||
config.background_terminal_max_timeout,
|
||||
),
|
||||
@@ -1738,6 +1746,7 @@ impl Session {
|
||||
config.codex_home.clone(),
|
||||
codex_apps_tools_cache_key(auth),
|
||||
tool_plugin_provenance,
|
||||
apps_mcp_cookie_jar,
|
||||
)
|
||||
.await;
|
||||
{
|
||||
@@ -3912,6 +3921,13 @@ impl Session {
|
||||
auth.as_ref(),
|
||||
config.as_ref(),
|
||||
);
|
||||
let apps_mcp_cookie_jar = mcp_servers
|
||||
.contains_key(CODEX_APPS_MCP_SERVER_NAME)
|
||||
.then(|| {
|
||||
self.services
|
||||
.apps_mcp_cookie_store
|
||||
.jar_for(config.as_ref(), auth.as_ref())
|
||||
});
|
||||
let auth_statuses = compute_auth_statuses(mcp_servers.iter(), store_mode).await;
|
||||
let sandbox_state = SandboxState {
|
||||
sandbox_policy: turn_context.sandbox_policy.get().clone(),
|
||||
@@ -3934,6 +3950,7 @@ impl Session {
|
||||
config.codex_home.clone(),
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
tool_plugin_provenance,
|
||||
apps_mcp_cookie_jar,
|
||||
)
|
||||
.await;
|
||||
{
|
||||
|
||||
@@ -58,6 +58,7 @@ pub(crate) async fn run_codex_thread_interactive(
|
||||
let CodexSpawnOk { codex, .. } = Codex::spawn(
|
||||
config,
|
||||
auth_manager,
|
||||
Arc::clone(&parent_session.services.apps_mcp_cookie_store),
|
||||
models_manager,
|
||||
Arc::clone(&parent_session.services.skills_manager),
|
||||
Arc::clone(&parent_session.services.plugins_manager),
|
||||
|
||||
@@ -18,6 +18,7 @@ use rmcp::model::ToolAnnotations;
|
||||
use serde::Deserialize;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::AppsMcpCookieStore;
|
||||
use crate::AuthManager;
|
||||
use crate::CodexAuth;
|
||||
use crate::SandboxState;
|
||||
@@ -107,15 +108,48 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options(
|
||||
force_refetch: bool,
|
||||
) -> anyhow::Result<Vec<AppInfo>> {
|
||||
Ok(
|
||||
list_accessible_connectors_from_mcp_tools_with_options_and_status(config, force_refetch)
|
||||
.await?
|
||||
.connectors,
|
||||
list_accessible_connectors_from_mcp_tools_with_options_and_status_and_cookie_store(
|
||||
config,
|
||||
force_refetch,
|
||||
None,
|
||||
)
|
||||
.await?
|
||||
.connectors,
|
||||
)
|
||||
}
|
||||
|
||||
pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
config: &Config,
|
||||
force_refetch: bool,
|
||||
) -> anyhow::Result<AccessibleConnectorsStatus> {
|
||||
list_accessible_connectors_from_mcp_tools_with_options_and_status_and_cookie_store(
|
||||
config,
|
||||
force_refetch,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_cookie_store(
|
||||
config: &Config,
|
||||
force_refetch: bool,
|
||||
apps_mcp_cookie_store: Option<Arc<AppsMcpCookieStore>>,
|
||||
) -> anyhow::Result<Vec<AppInfo>> {
|
||||
Ok(
|
||||
list_accessible_connectors_from_mcp_tools_with_options_and_status_and_cookie_store(
|
||||
config,
|
||||
force_refetch,
|
||||
apps_mcp_cookie_store,
|
||||
)
|
||||
.await?
|
||||
.connectors,
|
||||
)
|
||||
}
|
||||
|
||||
async fn list_accessible_connectors_from_mcp_tools_with_options_and_status_and_cookie_store(
|
||||
config: &Config,
|
||||
force_refetch: bool,
|
||||
apps_mcp_cookie_store: Option<Arc<AppsMcpCookieStore>>,
|
||||
) -> anyhow::Result<AccessibleConnectorsStatus> {
|
||||
let auth_manager = auth_manager_from_config(config);
|
||||
let auth = auth_manager.auth().await;
|
||||
@@ -145,6 +179,11 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
codex_apps_ready: true,
|
||||
});
|
||||
}
|
||||
let apps_mcp_cookie_store =
|
||||
apps_mcp_cookie_store.unwrap_or_else(|| Arc::new(AppsMcpCookieStore::default()));
|
||||
let apps_mcp_cookie_jar = mcp_servers
|
||||
.contains_key(CODEX_APPS_MCP_SERVER_NAME)
|
||||
.then(|| apps_mcp_cookie_store.jar_for(config, auth.as_ref()));
|
||||
|
||||
let auth_status_entries =
|
||||
compute_auth_statuses(mcp_servers.iter(), config.mcp_oauth_credentials_store_mode).await;
|
||||
@@ -169,6 +208,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
|
||||
config.codex_home.clone(),
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
ToolPluginProvenance::default(),
|
||||
apps_mcp_cookie_jar,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ pub mod api_bridge;
|
||||
mod apply_patch;
|
||||
mod apps;
|
||||
mod arc_monitor;
|
||||
mod apps_mcp_cookie_store;
|
||||
pub mod auth;
|
||||
mod client;
|
||||
mod client_common;
|
||||
@@ -90,6 +91,8 @@ mod seatbelt_permissions;
|
||||
mod thread_manager;
|
||||
pub mod web_search;
|
||||
pub mod windows_sandbox_read_grants;
|
||||
pub use apps_mcp_cookie_store::AppsMcpCookieStore;
|
||||
pub use codex_protocol::protocol::InitialHistory;
|
||||
pub use thread_manager::NewThread;
|
||||
pub use thread_manager::ThreadManager;
|
||||
#[deprecated(note = "use ThreadManager")]
|
||||
|
||||
@@ -317,6 +317,7 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent
|
||||
config.codex_home.clone(),
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
tool_plugin_provenance,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -48,6 +48,7 @@ use codex_rmcp_client::SendElicitation;
|
||||
use futures::future::BoxFuture;
|
||||
use futures::future::FutureExt;
|
||||
use futures::future::Shared;
|
||||
use reqwest::cookie::Jar;
|
||||
use rmcp::model::ClientCapabilities;
|
||||
use rmcp::model::CreateElicitationRequestParams;
|
||||
use rmcp::model::ElicitationAction;
|
||||
@@ -428,6 +429,7 @@ impl AsyncManagedClient {
|
||||
server_name: String,
|
||||
config: McpServerConfig,
|
||||
store_mode: OAuthCredentialsStoreMode,
|
||||
codex_apps_cookie_jar: Option<Arc<Jar>>,
|
||||
cancel_token: CancellationToken,
|
||||
tx_event: Sender<Event>,
|
||||
elicitation_requests: ElicitationRequestManager,
|
||||
@@ -449,8 +451,15 @@ impl AsyncManagedClient {
|
||||
return Err(error.into());
|
||||
}
|
||||
|
||||
let client =
|
||||
Arc::new(make_rmcp_client(&server_name, config.transport, store_mode).await?);
|
||||
let client = Arc::new(
|
||||
make_rmcp_client(
|
||||
&server_name,
|
||||
config.transport,
|
||||
store_mode,
|
||||
codex_apps_cookie_jar,
|
||||
)
|
||||
.await?,
|
||||
);
|
||||
match start_server_task(
|
||||
server_name,
|
||||
client,
|
||||
@@ -634,6 +643,7 @@ impl McpConnectionManager {
|
||||
codex_home: PathBuf,
|
||||
codex_apps_tools_cache_key: CodexAppsToolsCacheKey,
|
||||
tool_plugin_provenance: ToolPluginProvenance,
|
||||
codex_apps_cookie_jar: Option<Arc<Jar>>,
|
||||
) -> (Self, CancellationToken) {
|
||||
let cancel_token = CancellationToken::new();
|
||||
let mut clients = HashMap::new();
|
||||
@@ -647,6 +657,11 @@ impl McpConnectionManager {
|
||||
server_origins.insert(server_name.clone(), origin);
|
||||
}
|
||||
let cancel_token = cancel_token.child_token();
|
||||
let server_cookie_jar = if server_name == CODEX_APPS_MCP_SERVER_NAME {
|
||||
codex_apps_cookie_jar.clone()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let _ = emit_update(
|
||||
&tx_event,
|
||||
McpStartupUpdateEvent {
|
||||
@@ -667,6 +682,7 @@ impl McpConnectionManager {
|
||||
server_name.clone(),
|
||||
cfg,
|
||||
store_mode,
|
||||
server_cookie_jar,
|
||||
cancel_token.clone(),
|
||||
tx_event.clone(),
|
||||
elicitation_requests.clone(),
|
||||
@@ -1402,6 +1418,7 @@ async fn make_rmcp_client(
|
||||
server_name: &str,
|
||||
transport: McpServerTransportConfig,
|
||||
store_mode: OAuthCredentialsStoreMode,
|
||||
cookie_jar: Option<Arc<Jar>>,
|
||||
) -> Result<RmcpClient, StartupOutcomeError> {
|
||||
match transport {
|
||||
McpServerTransportConfig::Stdio {
|
||||
@@ -1435,6 +1452,7 @@ async fn make_rmcp_client(
|
||||
http_headers,
|
||||
env_http_headers,
|
||||
store_mode,
|
||||
cookie_jar,
|
||||
)
|
||||
.await
|
||||
.map_err(StartupOutcomeError::from)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::AppsMcpCookieStore;
|
||||
use crate::AuthManager;
|
||||
use crate::RolloutRecorder;
|
||||
use crate::agent::AgentControl;
|
||||
@@ -54,6 +55,7 @@ impl CodeModeStoreService {
|
||||
pub(crate) struct SessionServices {
|
||||
pub(crate) mcp_connection_manager: Arc<RwLock<McpConnectionManager>>,
|
||||
pub(crate) mcp_startup_cancellation_token: Mutex<CancellationToken>,
|
||||
pub(crate) apps_mcp_cookie_store: Arc<AppsMcpCookieStore>,
|
||||
pub(crate) unified_exec_manager: UnifiedExecProcessManager,
|
||||
#[cfg_attr(not(unix), allow(dead_code))]
|
||||
pub(crate) shell_zsh_path: Option<PathBuf>,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::AppsMcpCookieStore;
|
||||
use crate::AuthManager;
|
||||
use crate::CodexAuth;
|
||||
use crate::ModelProviderInfo;
|
||||
@@ -131,6 +132,7 @@ pub struct ThreadManager {
|
||||
pub(crate) struct ThreadManagerState {
|
||||
threads: Arc<RwLock<HashMap<ThreadId, Arc<CodexThread>>>>,
|
||||
thread_created_tx: broadcast::Sender<ThreadId>,
|
||||
apps_mcp_cookie_store: Arc<AppsMcpCookieStore>,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
models_manager: Arc<ModelsManager>,
|
||||
skills_manager: Arc<SkillsManager>,
|
||||
@@ -163,6 +165,7 @@ impl ThreadManager {
|
||||
state: Arc::new(ThreadManagerState {
|
||||
threads: Arc::new(RwLock::new(HashMap::new())),
|
||||
thread_created_tx,
|
||||
apps_mcp_cookie_store: Arc::new(AppsMcpCookieStore::default()),
|
||||
models_manager: Arc::new(ModelsManager::new(
|
||||
codex_home,
|
||||
auth_manager.clone(),
|
||||
@@ -223,6 +226,7 @@ impl ThreadManager {
|
||||
state: Arc::new(ThreadManagerState {
|
||||
threads: Arc::new(RwLock::new(HashMap::new())),
|
||||
thread_created_tx,
|
||||
apps_mcp_cookie_store: Arc::new(AppsMcpCookieStore::default()),
|
||||
models_manager: Arc::new(ModelsManager::with_provider_for_tests(
|
||||
codex_home,
|
||||
auth_manager.clone(),
|
||||
@@ -265,6 +269,10 @@ impl ThreadManager {
|
||||
self.state.models_manager.clone()
|
||||
}
|
||||
|
||||
pub fn apps_mcp_cookie_store(&self) -> Arc<AppsMcpCookieStore> {
|
||||
Arc::clone(&self.state.apps_mcp_cookie_store)
|
||||
}
|
||||
|
||||
pub async fn list_models(
|
||||
&self,
|
||||
refresh_strategy: crate::models_manager::manager::RefreshStrategy,
|
||||
@@ -600,6 +608,7 @@ impl ThreadManagerState {
|
||||
} = Codex::spawn(
|
||||
config,
|
||||
auth_manager,
|
||||
Arc::clone(&self.apps_mcp_cookie_store),
|
||||
Arc::clone(&self.models_manager),
|
||||
Arc::clone(&self.skills_manager),
|
||||
Arc::clone(&self.plugins_manager),
|
||||
|
||||
Reference in New Issue
Block a user