mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
Handle Xcode 26.4 MCP elicitation compatibility
This commit is contained in:
@@ -59,6 +59,8 @@ use codex_app_server_protocol::ServerRequestPayload;
|
||||
use codex_app_server_protocol::experimental_required_message;
|
||||
use codex_arg0::Arg0DispatchPaths;
|
||||
use codex_chatgpt::workspace_settings;
|
||||
use codex_core::ClientCompatibilityFlags;
|
||||
use codex_core::McpElicitationCompatibility;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::thread_store_from_config;
|
||||
@@ -187,6 +189,57 @@ pub(crate) struct InitializedConnectionSessionState {
|
||||
pub(crate) opted_out_notification_methods: HashSet<String>,
|
||||
pub(crate) app_server_client_name: String,
|
||||
pub(crate) client_version: String,
|
||||
pub(crate) client_compatibility_flags: ClientCompatibilityFlags,
|
||||
}
|
||||
|
||||
struct InitializedClientRequestContext {
|
||||
app_server_client_name: Option<String>,
|
||||
client_version: Option<String>,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
device_key_requests_allowed: bool,
|
||||
}
|
||||
|
||||
impl InitializedConnectionSessionState {
|
||||
pub(crate) fn new(
|
||||
experimental_api_enabled: bool,
|
||||
opted_out_notification_methods: HashSet<String>,
|
||||
app_server_client_name: String,
|
||||
client_version: String,
|
||||
) -> Self {
|
||||
let client_compatibility_flags = client_compatibility_flags_for_app_server_client(
|
||||
app_server_client_name.as_str(),
|
||||
client_version.as_str(),
|
||||
);
|
||||
Self {
|
||||
experimental_api_enabled,
|
||||
opted_out_notification_methods,
|
||||
app_server_client_name,
|
||||
client_version,
|
||||
client_compatibility_flags,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn client_compatibility_flags_for_app_server_client(
|
||||
client_name: &str,
|
||||
client_version: &str,
|
||||
) -> ClientCompatibilityFlags {
|
||||
let mut flags = ClientCompatibilityFlags::default();
|
||||
|
||||
// Xcode 26.4 shipped against CLI behavior from before PR #17043, when
|
||||
// Codex only advertised MCP elicitation support to the built-in
|
||||
// `codex_apps` server. If Codex advertises custom-server elicitation
|
||||
// support to that client family, those requests become new app-server
|
||||
// events/messages that the client cannot handle. Preserve the old
|
||||
// capability surface for that client only.
|
||||
//
|
||||
// TODO: Remove this Xcode 26.4 compatibility path once that client version
|
||||
// has aged out of support.
|
||||
if client_name == "Xcode" && client_version.starts_with("26.4") {
|
||||
flags.mcp_elicitation = McpElicitationCompatibility::CodexAppsOnly;
|
||||
}
|
||||
|
||||
flags
|
||||
}
|
||||
|
||||
impl Default for ConnectionSessionState {
|
||||
@@ -237,6 +290,13 @@ impl ConnectionSessionState {
|
||||
.map(|session| session.client_version.as_str())
|
||||
}
|
||||
|
||||
pub(crate) fn client_compatibility_flags(&self) -> ClientCompatibilityFlags {
|
||||
self.initialized
|
||||
.get()
|
||||
.map(|session| session.client_compatibility_flags)
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
pub(crate) fn initialize(&self, session: InitializedConnectionSessionState) -> Result<(), ()> {
|
||||
self.initialized.set(session).map_err(|_| ())
|
||||
}
|
||||
@@ -373,7 +433,6 @@ impl MessageProcessor {
|
||||
thread_state_manager.clone(),
|
||||
);
|
||||
let thread_processor = ThreadRequestProcessor::new(
|
||||
auth_manager.clone(),
|
||||
Arc::clone(&thread_manager),
|
||||
outgoing.clone(),
|
||||
analytics_events_client.clone(),
|
||||
@@ -756,9 +815,12 @@ impl MessageProcessor {
|
||||
);
|
||||
|
||||
let serialization_scope = codex_request.serialization_scope();
|
||||
let app_server_client_name = session.app_server_client_name().map(str::to_string);
|
||||
let client_version = session.client_version().map(str::to_string);
|
||||
let device_key_requests_allowed = session.allows_device_key_requests();
|
||||
let initialized_client_context = InitializedClientRequestContext {
|
||||
app_server_client_name: session.app_server_client_name().map(str::to_string),
|
||||
client_version: session.client_version().map(str::to_string),
|
||||
client_compatibility_flags: session.client_compatibility_flags(),
|
||||
device_key_requests_allowed: session.allows_device_key_requests(),
|
||||
};
|
||||
let error_request_id = connection_request_id.clone();
|
||||
let rpc_gate = Arc::clone(&session.rpc_gate);
|
||||
let processor = Arc::clone(self);
|
||||
@@ -772,9 +834,7 @@ impl MessageProcessor {
|
||||
connection_request_id,
|
||||
codex_request,
|
||||
request_context,
|
||||
app_server_client_name,
|
||||
client_version,
|
||||
device_key_requests_allowed,
|
||||
initialized_client_context,
|
||||
)
|
||||
.await;
|
||||
if let Err(error) = result {
|
||||
@@ -802,9 +862,7 @@ impl MessageProcessor {
|
||||
connection_request_id: ConnectionRequestId,
|
||||
codex_request: ClientRequest,
|
||||
request_context: RequestContext,
|
||||
app_server_client_name: Option<String>,
|
||||
client_version: Option<String>,
|
||||
device_key_requests_allowed: bool,
|
||||
initialized_client_context: InitializedClientRequestContext,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
let connection_id = connection_request_id.connection_id;
|
||||
let request_id = ConnectionRequestId {
|
||||
@@ -851,7 +909,7 @@ impl MessageProcessor {
|
||||
self.device_key_processor.create(
|
||||
request_id.clone(),
|
||||
params,
|
||||
device_key_requests_allowed,
|
||||
initialized_client_context.device_key_requests_allowed,
|
||||
);
|
||||
Ok(None)
|
||||
}
|
||||
@@ -859,7 +917,7 @@ impl MessageProcessor {
|
||||
self.device_key_processor.public(
|
||||
request_id.clone(),
|
||||
params,
|
||||
device_key_requests_allowed,
|
||||
initialized_client_context.device_key_requests_allowed,
|
||||
);
|
||||
Ok(None)
|
||||
}
|
||||
@@ -867,7 +925,7 @@ impl MessageProcessor {
|
||||
self.device_key_processor.sign(
|
||||
request_id.clone(),
|
||||
params,
|
||||
device_key_requests_allowed,
|
||||
initialized_client_context.device_key_requests_allowed,
|
||||
);
|
||||
Ok(None)
|
||||
}
|
||||
@@ -926,8 +984,9 @@ impl MessageProcessor {
|
||||
.thread_start(
|
||||
request_id.clone(),
|
||||
params,
|
||||
app_server_client_name.clone(),
|
||||
client_version.clone(),
|
||||
initialized_client_context.app_server_client_name.clone(),
|
||||
initialized_client_context.client_version.clone(),
|
||||
initialized_client_context.client_compatibility_flags,
|
||||
request_context,
|
||||
)
|
||||
.await
|
||||
@@ -939,12 +998,20 @@ impl MessageProcessor {
|
||||
}
|
||||
ClientRequest::ThreadResume { params, .. } => {
|
||||
self.thread_processor
|
||||
.thread_resume(request_id.clone(), params)
|
||||
.thread_resume(
|
||||
request_id.clone(),
|
||||
params,
|
||||
initialized_client_context.client_compatibility_flags,
|
||||
)
|
||||
.await
|
||||
}
|
||||
ClientRequest::ThreadFork { params, .. } => {
|
||||
self.thread_processor
|
||||
.thread_fork(request_id.clone(), params)
|
||||
.thread_fork(
|
||||
request_id.clone(),
|
||||
params,
|
||||
initialized_client_context.client_compatibility_flags,
|
||||
)
|
||||
.await
|
||||
}
|
||||
ClientRequest::ThreadArchive { params, .. } => {
|
||||
@@ -1098,8 +1165,8 @@ impl MessageProcessor {
|
||||
.turn_start(
|
||||
request_id.clone(),
|
||||
params,
|
||||
app_server_client_name.clone(),
|
||||
client_version.clone(),
|
||||
initialized_client_context.app_server_client_name.clone(),
|
||||
initialized_client_context.client_version.clone(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -1259,3 +1326,32 @@ impl MessageProcessor {
|
||||
#[cfg(test)]
|
||||
#[path = "message_processor_tracing_tests.rs"]
|
||||
mod message_processor_tracing_tests;
|
||||
|
||||
#[cfg(test)]
|
||||
mod client_compatibility_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn xcode_26_4_uses_legacy_mcp_elicitation_compatibility() {
|
||||
for version in ["26.4", "26.4.1", "26.4-beta"] {
|
||||
assert_eq!(
|
||||
client_compatibility_flags_for_app_server_client("Xcode", version),
|
||||
ClientCompatibilityFlags {
|
||||
mcp_elicitation: McpElicitationCompatibility::CodexAppsOnly,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
for (name, version) in [
|
||||
("Xcode", "26.3"),
|
||||
("Xcode", "26.5"),
|
||||
("Xcode", "27.0"),
|
||||
("codex-vscode", "26.4"),
|
||||
] {
|
||||
assert_eq!(
|
||||
client_compatibility_flags_for_app_server_client(name, version),
|
||||
ClientCompatibilityFlags::default()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -244,6 +244,7 @@ use codex_config::CloudRequirementsLoadErrorCode;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::loader::project_trust_key;
|
||||
use codex_config::types::McpServerTransportConfig;
|
||||
use codex_core::ClientCompatibilityFlags;
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::CodexThreadTurnContextOverrides;
|
||||
use codex_core::ForkSnapshot;
|
||||
|
||||
@@ -310,6 +310,7 @@ impl ExternalAgentConfigRequestProcessor {
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags: codex_core::ClientCompatibilityFlags::default(),
|
||||
parent_trace: None,
|
||||
environments,
|
||||
})
|
||||
|
||||
@@ -90,12 +90,12 @@ impl InitializeRequestProcessor {
|
||||
let user_agent_suffix = format!("{name}; {version}");
|
||||
let codex_home = self.config.codex_home.clone();
|
||||
if session
|
||||
.initialize(InitializedConnectionSessionState {
|
||||
.initialize(InitializedConnectionSessionState::new(
|
||||
experimental_api_enabled,
|
||||
opted_out_notification_methods: opt_out_notification_methods.into_iter().collect(),
|
||||
app_server_client_name: name.clone(),
|
||||
client_version: version,
|
||||
})
|
||||
opt_out_notification_methods.into_iter().collect(),
|
||||
name.clone(),
|
||||
version,
|
||||
))
|
||||
.is_err()
|
||||
{
|
||||
return Err(invalid_request("Already initialized"));
|
||||
|
||||
@@ -246,7 +246,6 @@ fn validate_dynamic_tools(tools: &[ApiDynamicToolSpec]) -> Result<(), String> {
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct ThreadRequestProcessor {
|
||||
pub(super) auth_manager: Arc<AuthManager>,
|
||||
pub(super) thread_manager: Arc<ThreadManager>,
|
||||
pub(super) outgoing: Arc<OutgoingMessageSender>,
|
||||
pub(super) analytics_events_client: AnalyticsEventsClient,
|
||||
@@ -265,7 +264,6 @@ pub(crate) struct ThreadRequestProcessor {
|
||||
impl ThreadRequestProcessor {
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn new(
|
||||
auth_manager: Arc<AuthManager>,
|
||||
thread_manager: Arc<ThreadManager>,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
analytics_events_client: AnalyticsEventsClient,
|
||||
@@ -280,7 +278,6 @@ impl ThreadRequestProcessor {
|
||||
thread_goal_processor: ThreadGoalRequestProcessor,
|
||||
) -> Self {
|
||||
Self {
|
||||
auth_manager,
|
||||
thread_manager,
|
||||
outgoing,
|
||||
analytics_events_client,
|
||||
@@ -303,6 +300,7 @@ impl ThreadRequestProcessor {
|
||||
params: ThreadStartParams,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
request_context: RequestContext,
|
||||
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
|
||||
self.thread_start_inner(
|
||||
@@ -310,6 +308,7 @@ impl ThreadRequestProcessor {
|
||||
params,
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
client_compatibility_flags,
|
||||
request_context,
|
||||
)
|
||||
.await
|
||||
@@ -330,8 +329,9 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadResumeParams,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
|
||||
self.thread_resume_inner(request_id, params)
|
||||
self.thread_resume_inner(request_id, params, client_compatibility_flags)
|
||||
.await
|
||||
.map(|()| None)
|
||||
}
|
||||
@@ -340,8 +340,9 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadForkParams,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
|
||||
self.thread_fork_inner(request_id, params)
|
||||
self.thread_fork_inner(request_id, params, client_compatibility_flags)
|
||||
.await
|
||||
.map(|()| None)
|
||||
}
|
||||
@@ -718,6 +719,7 @@ impl ThreadRequestProcessor {
|
||||
params: ThreadStartParams,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
request_context: RequestContext,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
let ThreadStartParams {
|
||||
@@ -784,6 +786,7 @@ impl ThreadRequestProcessor {
|
||||
request_id,
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
client_compatibility_flags,
|
||||
config,
|
||||
typesafe_overrides,
|
||||
dynamic_tools,
|
||||
@@ -856,6 +859,7 @@ impl ThreadRequestProcessor {
|
||||
request_id: ConnectionRequestId,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
config_overrides: Option<HashMap<String, serde_json::Value>>,
|
||||
typesafe_overrides: ConfigOverrides,
|
||||
dynamic_tools: Option<Vec<ApiDynamicToolSpec>>,
|
||||
@@ -980,6 +984,7 @@ impl ThreadRequestProcessor {
|
||||
dynamic_tools: core_dynamic_tools,
|
||||
persist_extended_history,
|
||||
metrics_service_name: service_name,
|
||||
client_compatibility_flags,
|
||||
parent_trace: request_trace,
|
||||
environments,
|
||||
})
|
||||
@@ -2296,6 +2301,7 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadResumeParams,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
if let Ok(thread_id) = ThreadId::from_string(¶ms.thread_id)
|
||||
&& self
|
||||
@@ -2418,13 +2424,19 @@ impl ThreadRequestProcessor {
|
||||
|
||||
match self
|
||||
.thread_manager
|
||||
.resume_thread_with_history(
|
||||
config.clone(),
|
||||
thread_history,
|
||||
self.auth_manager.clone(),
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config: config.clone(),
|
||||
initial_history: thread_history,
|
||||
session_source: None,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history,
|
||||
self.request_trace_context(&request_id).await,
|
||||
)
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags,
|
||||
parent_trace: self.request_trace_context(&request_id).await,
|
||||
environments: self
|
||||
.thread_manager
|
||||
.default_environment_selections(&config.cwd),
|
||||
})
|
||||
.await
|
||||
{
|
||||
Ok(NewThread {
|
||||
@@ -2931,6 +2943,7 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadForkParams,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
let ThreadForkParams {
|
||||
thread_id,
|
||||
@@ -3025,7 +3038,7 @@ impl ThreadRequestProcessor {
|
||||
..
|
||||
} = self
|
||||
.thread_manager
|
||||
.fork_thread_from_history(
|
||||
.fork_thread_from_history_with_client_compatibility_flags(
|
||||
ForkSnapshot::Interrupted,
|
||||
config,
|
||||
InitialHistory::Resumed(ResumedHistory {
|
||||
@@ -3035,6 +3048,7 @@ impl ThreadRequestProcessor {
|
||||
}),
|
||||
persist_extended_history,
|
||||
self.request_trace_context(&request_id).await,
|
||||
client_compatibility_flags,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| match err {
|
||||
|
||||
@@ -915,12 +915,13 @@ impl TurnRequestProcessor {
|
||||
..
|
||||
} = self
|
||||
.thread_manager
|
||||
.fork_thread(
|
||||
.fork_thread_with_client_compatibility_flags(
|
||||
ForkSnapshot::Interrupted,
|
||||
config.clone(),
|
||||
rollout_path,
|
||||
/*persist_extended_history*/ false,
|
||||
self.request_trace_context(request_id).await,
|
||||
parent_thread.client_compatibility_flags().await,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
|
||||
@@ -17,6 +17,7 @@ use crate::codex_apps::CodexAppsToolsCacheContext;
|
||||
use crate::codex_apps::CodexAppsToolsCacheKey;
|
||||
use crate::codex_apps::write_cached_codex_apps_tools_if_needed;
|
||||
use crate::elicitation::ElicitationRequestManager;
|
||||
use crate::elicitation::McpElicitationCompatibility;
|
||||
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use crate::mcp::ToolPluginProvenance;
|
||||
use crate::rmcp_client::AsyncManagedClient;
|
||||
@@ -142,13 +143,17 @@ impl McpConnectionManager {
|
||||
codex_apps_tools_cache_key: CodexAppsToolsCacheKey,
|
||||
tool_plugin_provenance: ToolPluginProvenance,
|
||||
auth: Option<&CodexAuth>,
|
||||
elicitation_compatibility: McpElicitationCompatibility,
|
||||
) -> (Self, CancellationToken) {
|
||||
let cancel_token = CancellationToken::new();
|
||||
let mut clients = HashMap::new();
|
||||
let mut server_origins = HashMap::new();
|
||||
let mut join_set = JoinSet::new();
|
||||
let elicitation_requests =
|
||||
ElicitationRequestManager::new(approval_policy.value(), initial_permission_profile);
|
||||
let elicitation_requests = ElicitationRequestManager::new_with_compatibility(
|
||||
approval_policy.value(),
|
||||
initial_permission_profile,
|
||||
elicitation_compatibility,
|
||||
);
|
||||
let tool_plugin_provenance = Arc::new(tool_plugin_provenance);
|
||||
let startup_submit_id = submit_id.clone();
|
||||
let codex_apps_auth_provider = auth
|
||||
@@ -197,6 +202,7 @@ impl McpConnectionManager {
|
||||
cancel_token.clone(),
|
||||
tx_event.clone(),
|
||||
elicitation_requests.clone(),
|
||||
elicitation_compatibility,
|
||||
codex_apps_tools_cache_context,
|
||||
Arc::clone(&tool_plugin_provenance),
|
||||
runtime_environment.clone(),
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::codex_apps::read_cached_codex_apps_tools;
|
||||
use crate::codex_apps::write_cached_codex_apps_tools;
|
||||
use crate::declared_openai_file_input_param_names;
|
||||
use crate::elicitation::ElicitationRequestManager;
|
||||
use crate::elicitation::McpElicitationCompatibility;
|
||||
use crate::elicitation::elicitation_is_rejected_by_policy;
|
||||
use crate::rmcp_client::AsyncManagedClient;
|
||||
use crate::rmcp_client::ManagedClient;
|
||||
@@ -266,6 +267,47 @@ async fn disabled_permissions_do_not_auto_accept_elicitation_with_requested_fiel
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_apps_only_custom_server_elicitation_is_declined_without_event() {
|
||||
let manager = ElicitationRequestManager::new_with_compatibility(
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::default(),
|
||||
McpElicitationCompatibility::CodexAppsOnly,
|
||||
);
|
||||
let (tx_event, rx_event) = async_channel::bounded(1);
|
||||
let sender = manager.make_sender("custom_mcp".to_string(), tx_event);
|
||||
|
||||
let response = sender(
|
||||
NumberOrString::Number(1),
|
||||
CreateElicitationRequestParams::FormElicitationParams {
|
||||
meta: None,
|
||||
message: "What should I say?".to_string(),
|
||||
requested_schema: rmcp::model::ElicitationSchema::builder()
|
||||
.required_property(
|
||||
"message",
|
||||
rmcp::model::PrimitiveSchema::String(rmcp::model::StringSchema::new()),
|
||||
)
|
||||
.build()
|
||||
.expect("schema should build"),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.expect("elicitation should auto decline");
|
||||
|
||||
assert_eq!(
|
||||
response,
|
||||
ElicitationResponse {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
meta: None,
|
||||
}
|
||||
);
|
||||
assert!(matches!(
|
||||
rx_event.try_recv(),
|
||||
Err(async_channel::TryRecvError::Empty)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_qualify_tools_short_non_duplicated_names() {
|
||||
let tools = vec![
|
||||
@@ -802,7 +844,8 @@ async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() {
|
||||
#[test]
|
||||
fn elicitation_capability_uses_2025_06_18_shape_for_all_servers() {
|
||||
for server_name in [CODEX_APPS_MCP_SERVER_NAME, "custom_mcp"] {
|
||||
let capability = elicitation_capability_for_server(server_name);
|
||||
let capability =
|
||||
elicitation_capability_for_server(server_name, McpElicitationCompatibility::Default);
|
||||
assert_eq!(capability, Some(ElicitationCapability::default()));
|
||||
assert_eq!(
|
||||
serde_json::to_value(capability).expect("serialize elicitation capability"),
|
||||
@@ -811,6 +854,21 @@ fn elicitation_capability_uses_2025_06_18_shape_for_all_servers() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_apps_only_elicitation_capability_keeps_codex_apps_enabled() {
|
||||
assert_eq!(
|
||||
elicitation_capability_for_server(
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
McpElicitationCompatibility::CodexAppsOnly,
|
||||
),
|
||||
Some(ElicitationCapability::default())
|
||||
);
|
||||
assert_eq!(
|
||||
elicitation_capability_for_server("custom_mcp", McpElicitationCompatibility::CodexAppsOnly,),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_init_error_display_prompts_for_github_pat() {
|
||||
let server_name = "github";
|
||||
|
||||
@@ -9,6 +9,7 @@ use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex as StdMutex;
|
||||
|
||||
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use crate::mcp::McpPermissionPromptAutoApproveContext;
|
||||
use crate::mcp::mcp_permission_prompt_is_auto_approved;
|
||||
use anyhow::Context;
|
||||
@@ -36,17 +37,31 @@ pub(crate) struct ElicitationRequestManager {
|
||||
requests: Arc<Mutex<ResponderMap>>,
|
||||
pub(crate) approval_policy: Arc<StdMutex<AskForApproval>>,
|
||||
pub(crate) permission_profile: Arc<StdMutex<PermissionProfile>>,
|
||||
compatibility: McpElicitationCompatibility,
|
||||
}
|
||||
|
||||
impl ElicitationRequestManager {
|
||||
pub(crate) fn new(
|
||||
approval_policy: AskForApproval,
|
||||
permission_profile: PermissionProfile,
|
||||
) -> Self {
|
||||
Self::new_with_compatibility(
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
McpElicitationCompatibility::Default,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn new_with_compatibility(
|
||||
approval_policy: AskForApproval,
|
||||
permission_profile: PermissionProfile,
|
||||
compatibility: McpElicitationCompatibility,
|
||||
) -> Self {
|
||||
Self {
|
||||
requests: Arc::new(Mutex::new(HashMap::new())),
|
||||
approval_policy: Arc::new(StdMutex::new(approval_policy)),
|
||||
permission_profile: Arc::new(StdMutex::new(permission_profile)),
|
||||
compatibility,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -73,6 +88,7 @@ impl ElicitationRequestManager {
|
||||
let elicitation_requests = self.requests.clone();
|
||||
let approval_policy = self.approval_policy.clone();
|
||||
let permission_profile = self.permission_profile.clone();
|
||||
let compatibility = self.compatibility;
|
||||
Box::new(move |id, elicitation| {
|
||||
let elicitation_requests = elicitation_requests.clone();
|
||||
let tx_event = tx_event.clone();
|
||||
@@ -80,6 +96,14 @@ impl ElicitationRequestManager {
|
||||
let approval_policy = approval_policy.clone();
|
||||
let permission_profile = permission_profile.clone();
|
||||
async move {
|
||||
if !compatibility.allows_server_elicitation(&server_name) {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
meta: None,
|
||||
});
|
||||
}
|
||||
|
||||
let approval_policy = approval_policy
|
||||
.lock()
|
||||
.map(|policy| *policy)
|
||||
@@ -169,6 +193,28 @@ impl ElicitationRequestManager {
|
||||
}
|
||||
}
|
||||
|
||||
/// Compatibility policy for the MCP elicitation capability surface.
|
||||
///
|
||||
/// The default behavior advertises MCP elicitation support to every server.
|
||||
/// `CodexAppsOnly` preserves the pre-PR #17043 capability surface by keeping
|
||||
/// support for the built-in `codex_apps` server while suppressing support for
|
||||
/// custom MCP servers.
|
||||
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
|
||||
pub enum McpElicitationCompatibility {
|
||||
#[default]
|
||||
Default,
|
||||
CodexAppsOnly,
|
||||
}
|
||||
|
||||
impl McpElicitationCompatibility {
|
||||
pub(crate) fn allows_server_elicitation(self, server_name: &str) -> bool {
|
||||
match self {
|
||||
Self::Default => true,
|
||||
Self::CodexAppsOnly => server_name == CODEX_APPS_MCP_SERVER_NAME,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn elicitation_is_rejected_by_policy(approval_policy: AskForApproval) -> bool {
|
||||
match approval_policy {
|
||||
AskForApproval::Never => true,
|
||||
|
||||
@@ -34,6 +34,7 @@ pub use mcp::resolve_oauth_scopes;
|
||||
pub use mcp::should_retry_without_scopes;
|
||||
|
||||
pub use codex_apps::filter_non_codex_apps_mcp_tools_only;
|
||||
pub use elicitation::McpElicitationCompatibility;
|
||||
pub use mcp::McpPermissionPromptAutoApproveContext;
|
||||
pub use mcp::mcp_permission_prompt_is_auto_approved;
|
||||
pub use mcp::qualified_mcp_tool_name_prefix;
|
||||
|
||||
@@ -255,6 +255,7 @@ pub async fn read_mcp_resource(
|
||||
codex_apps_tools_cache_key(auth),
|
||||
tool_plugin_provenance(config),
|
||||
auth,
|
||||
crate::McpElicitationCompatibility::default(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -320,6 +321,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail(
|
||||
codex_apps_tools_cache_key(auth),
|
||||
tool_plugin_provenance,
|
||||
auth,
|
||||
crate::McpElicitationCompatibility::default(),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -26,6 +26,7 @@ use crate::codex_apps::normalize_codex_apps_callable_namespace;
|
||||
use crate::codex_apps::normalize_codex_apps_tool_title;
|
||||
use crate::codex_apps::write_cached_codex_apps_tools_if_needed;
|
||||
use crate::elicitation::ElicitationRequestManager;
|
||||
use crate::elicitation::McpElicitationCompatibility;
|
||||
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use crate::mcp::ToolPluginProvenance;
|
||||
use crate::runtime::McpRuntimeEnvironment;
|
||||
@@ -138,6 +139,7 @@ impl AsyncManagedClient {
|
||||
cancel_token: CancellationToken,
|
||||
tx_event: Sender<Event>,
|
||||
elicitation_requests: ElicitationRequestManager,
|
||||
elicitation_compatibility: McpElicitationCompatibility,
|
||||
codex_apps_tools_cache_context: Option<CodexAppsToolsCacheContext>,
|
||||
tool_plugin_provenance: Arc<ToolPluginProvenance>,
|
||||
runtime_environment: McpRuntimeEnvironment,
|
||||
@@ -180,6 +182,7 @@ impl AsyncManagedClient {
|
||||
tool_filter: startup_tool_filter,
|
||||
tx_event,
|
||||
elicitation_requests,
|
||||
elicitation_compatibility,
|
||||
codex_apps_tools_cache_context,
|
||||
},
|
||||
)
|
||||
@@ -318,8 +321,16 @@ impl From<anyhow::Error> for StartupOutcomeError {
|
||||
}
|
||||
|
||||
pub(crate) fn elicitation_capability_for_server(
|
||||
_server_name: &str,
|
||||
server_name: &str,
|
||||
compatibility: McpElicitationCompatibility,
|
||||
) -> Option<ElicitationCapability> {
|
||||
// Keep capability advertising aligned with the client compatibility flags.
|
||||
// The sender has a matching defensive decline path for servers that send
|
||||
// elicitations despite this advertised capability.
|
||||
if !compatibility.allows_server_elicitation(server_name) {
|
||||
return None;
|
||||
}
|
||||
|
||||
// https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#capabilities
|
||||
// indicates this should be an empty object.
|
||||
Some(ElicitationCapability::default())
|
||||
@@ -455,9 +466,10 @@ async fn start_server_task(
|
||||
tool_filter,
|
||||
tx_event,
|
||||
elicitation_requests,
|
||||
elicitation_compatibility,
|
||||
codex_apps_tools_cache_context,
|
||||
} = params;
|
||||
let elicitation = elicitation_capability_for_server(&server_name);
|
||||
let elicitation = elicitation_capability_for_server(&server_name, elicitation_compatibility);
|
||||
let params = InitializeRequestParams {
|
||||
meta: None,
|
||||
capabilities: ClientCapabilities {
|
||||
@@ -540,6 +552,7 @@ struct StartServerTaskParams {
|
||||
tool_filter: ToolFilter,
|
||||
tx_event: Sender<Event>,
|
||||
elicitation_requests: ElicitationRequestManager,
|
||||
elicitation_compatibility: McpElicitationCompatibility,
|
||||
codex_apps_tools_cache_context: Option<CodexAppsToolsCacheContext>,
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
//! Internal compatibility flags selected by app-server clients.
|
||||
//!
|
||||
//! These flags keep app-server client identity at the app-server boundary. Core
|
||||
//! should receive the behavior switches it needs, not the raw client name and
|
||||
//! version that led to those switches.
|
||||
|
||||
pub use codex_mcp::McpElicitationCompatibility;
|
||||
|
||||
/// Behavior switches selected by the app-server layer for known client quirks.
|
||||
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
|
||||
pub struct ClientCompatibilityFlags {
|
||||
/// MCP elicitation capability policy to apply when starting or refreshing MCP servers.
|
||||
pub mcp_elicitation: McpElicitationCompatibility,
|
||||
}
|
||||
@@ -73,6 +73,7 @@ pub(crate) async fn run_codex_thread_interactive(
|
||||
) -> Result<Codex, CodexErr> {
|
||||
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
|
||||
let (tx_ops, rx_ops) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
|
||||
let client_compatibility_flags = parent_session.client_compatibility_flags().await;
|
||||
let CodexSpawnOk { codex, .. } = Box::pin(Codex::spawn(CodexSpawnArgs {
|
||||
config,
|
||||
auth_manager,
|
||||
@@ -88,6 +89,7 @@ pub(crate) async fn run_codex_thread_interactive(
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags,
|
||||
inherited_shell_snapshot: None,
|
||||
user_shell_override: None,
|
||||
inherited_exec_policy: Some(Arc::clone(&parent_session.services.exec_policy)),
|
||||
@@ -100,10 +102,10 @@ pub(crate) async fn run_codex_thread_interactive(
|
||||
.or_cancel(&cancel_token)
|
||||
.await??;
|
||||
let thread_config = codex.thread_config_snapshot().await;
|
||||
let client_metadata = parent_session.app_server_client_metadata().await;
|
||||
let app_server_client_metadata = parent_session.app_server_client_metadata().await;
|
||||
emit_subagent_session_started(
|
||||
&parent_session.services.analytics_events_client,
|
||||
client_metadata,
|
||||
app_server_client_metadata,
|
||||
codex.session.conversation_id,
|
||||
Some(parent_session.conversation_id),
|
||||
thread_config,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::ClientCompatibilityFlags;
|
||||
use crate::agent::AgentStatus;
|
||||
use crate::config::ConstraintResult;
|
||||
use crate::file_watcher::WatchRegistration;
|
||||
@@ -224,6 +225,10 @@ impl CodexThread {
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn client_compatibility_flags(&self) -> ClientCompatibilityFlags {
|
||||
self.codex.session.client_compatibility_flags().await
|
||||
}
|
||||
|
||||
/// Validate persistent turn context overrides without committing them.
|
||||
pub async fn validate_turn_context_overrides(
|
||||
&self,
|
||||
|
||||
@@ -280,6 +280,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager(
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
ToolPluginProvenance::default(),
|
||||
auth.as_ref(),
|
||||
codex_mcp::McpElicitationCompatibility::default(),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ mod apps;
|
||||
mod arc_monitor;
|
||||
mod client;
|
||||
mod client_common;
|
||||
mod client_compatibility;
|
||||
mod realtime_context;
|
||||
mod realtime_conversation;
|
||||
mod realtime_prompt;
|
||||
@@ -110,6 +111,8 @@ pub mod test_support;
|
||||
mod unified_exec;
|
||||
pub mod windows_sandbox;
|
||||
pub use client::X_RESPONSESAPI_INCLUDE_TIMING_METRICS_HEADER;
|
||||
pub use client_compatibility::ClientCompatibilityFlags;
|
||||
pub use client_compatibility::McpElicitationCompatibility;
|
||||
pub use codex_protocol::config_types::ModelProviderAuthInfo;
|
||||
mod event_mapping;
|
||||
pub mod review_format;
|
||||
|
||||
@@ -252,6 +252,7 @@ impl Session {
|
||||
codex_apps_tools_cache_key(auth.as_ref()),
|
||||
tool_plugin_provenance,
|
||||
auth.as_ref(),
|
||||
turn_context.client_compatibility_flags.mcp_elicitation,
|
||||
)
|
||||
.await;
|
||||
{
|
||||
|
||||
@@ -8,6 +8,7 @@ use std::sync::atomic::AtomicU64;
|
||||
use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
|
||||
use crate::ClientCompatibilityFlags;
|
||||
use crate::agent::AgentControl;
|
||||
use crate::agent::AgentStatus;
|
||||
use crate::agent::Mailbox;
|
||||
@@ -400,6 +401,7 @@ pub(crate) struct CodexSpawnArgs {
|
||||
pub(crate) dynamic_tools: Vec<DynamicToolSpec>,
|
||||
pub(crate) persist_extended_history: bool,
|
||||
pub(crate) metrics_service_name: Option<String>,
|
||||
pub(crate) client_compatibility_flags: ClientCompatibilityFlags,
|
||||
pub(crate) inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
|
||||
pub(crate) inherited_exec_policy: Option<Arc<ExecPolicyManager>>,
|
||||
/// Parent rollout trace used only to derive fresh spawned child traces.
|
||||
@@ -461,6 +463,7 @@ impl Codex {
|
||||
dynamic_tools,
|
||||
persist_extended_history,
|
||||
metrics_service_name,
|
||||
client_compatibility_flags,
|
||||
inherited_shell_snapshot,
|
||||
user_shell_override,
|
||||
inherited_exec_policy,
|
||||
@@ -612,6 +615,7 @@ impl Codex {
|
||||
environments: environment_selections.to_selections(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name,
|
||||
client_compatibility_flags,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
session_source,
|
||||
@@ -834,6 +838,14 @@ async fn thread_title_from_state_db(
|
||||
}
|
||||
|
||||
impl Session {
|
||||
pub(crate) async fn client_compatibility_flags(&self) -> ClientCompatibilityFlags {
|
||||
self.state
|
||||
.lock()
|
||||
.await
|
||||
.session_configuration
|
||||
.client_compatibility_flags
|
||||
}
|
||||
|
||||
pub(crate) async fn app_server_client_metadata(&self) -> AppServerClientMetadata {
|
||||
let state = self.state.lock().await;
|
||||
AppServerClientMetadata {
|
||||
|
||||
@@ -130,6 +130,7 @@ pub(super) async fn spawn_review_thread(
|
||||
current_date: parent_turn_context.current_date.clone(),
|
||||
timezone: parent_turn_context.timezone.clone(),
|
||||
app_server_client_name: parent_turn_context.app_server_client_name.clone(),
|
||||
client_compatibility_flags: parent_turn_context.client_compatibility_flags,
|
||||
developer_instructions: None,
|
||||
user_instructions: None,
|
||||
compact_prompt: parent_turn_context.compact_prompt.clone(),
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::ClientCompatibilityFlags;
|
||||
use crate::goals::GoalRuntimeState;
|
||||
use codex_otel::LEGACY_NOTIFY_CONFIGURED_METRIC;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
@@ -83,6 +84,8 @@ pub(crate) struct SessionConfiguration {
|
||||
pub(super) original_config_do_not_use: Arc<Config>,
|
||||
/// Optional service name tag for session metrics.
|
||||
pub(super) metrics_service_name: Option<String>,
|
||||
/// App-server client compatibility behavior selected at session creation.
|
||||
pub(super) client_compatibility_flags: ClientCompatibilityFlags,
|
||||
pub(super) app_server_client_name: Option<String>,
|
||||
pub(super) app_server_client_version: Option<String>,
|
||||
/// Source of the session (cli, vscode, exec, mcp, ...)
|
||||
@@ -315,6 +318,7 @@ pub(crate) struct SessionSettingsUpdate {
|
||||
pub(crate) app_server_client_version: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub(crate) struct AppServerClientMetadata {
|
||||
pub(crate) client_name: Option<String>,
|
||||
pub(crate) client_version: Option<String>,
|
||||
@@ -996,6 +1000,9 @@ impl Session {
|
||||
codex_apps_tools_cache_key(auth),
|
||||
tool_plugin_provenance,
|
||||
auth,
|
||||
session_configuration
|
||||
.client_compatibility_flags
|
||||
.mcp_elicitation,
|
||||
)
|
||||
.instrument(info_span!(
|
||||
"session_init.mcp_manager_init",
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::turn_context::TurnEnvironment;
|
||||
use super::*;
|
||||
use crate::ClientCompatibilityFlags;
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::config::test_config;
|
||||
use crate::context::ContextualUserFragment;
|
||||
@@ -2372,6 +2373,7 @@ async fn set_rate_limits_retains_previous_credits() {
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
session_source: SessionSource::Exec,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
@@ -2475,6 +2477,7 @@ async fn set_rate_limits_updates_plan_type_when_present() {
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
session_source: SessionSource::Exec,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
@@ -2933,6 +2936,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
session_source: SessionSource::Exec,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
@@ -3458,6 +3462,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
session_source: SessionSource::Exec,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
@@ -3563,6 +3568,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
session_source: SessionSource::Exec,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
@@ -3775,6 +3781,7 @@ async fn make_session_with_config_and_rx(
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
session_source: SessionSource::Exec,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
@@ -5055,6 +5062,7 @@ where
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
app_server_client_version: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
session_source: SessionSource::Exec,
|
||||
dynamic_tools,
|
||||
persist_extended_history: false,
|
||||
|
||||
@@ -750,6 +750,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags: crate::ClientCompatibilityFlags::default(),
|
||||
inherited_shell_snapshot: None,
|
||||
inherited_exec_policy: Some(Arc::new(parent_exec_policy)),
|
||||
parent_rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(),
|
||||
|
||||
@@ -69,6 +69,7 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) current_date: Option<String>,
|
||||
pub(crate) timezone: Option<String>,
|
||||
pub(crate) app_server_client_name: Option<String>,
|
||||
pub(crate) client_compatibility_flags: ClientCompatibilityFlags,
|
||||
pub(crate) developer_instructions: Option<String>,
|
||||
pub(crate) compact_prompt: Option<String>,
|
||||
pub(crate) user_instructions: Option<String>,
|
||||
@@ -236,6 +237,7 @@ impl TurnContext {
|
||||
current_date: self.current_date.clone(),
|
||||
timezone: self.timezone.clone(),
|
||||
app_server_client_name: self.app_server_client_name.clone(),
|
||||
client_compatibility_flags: self.client_compatibility_flags,
|
||||
developer_instructions: self.developer_instructions.clone(),
|
||||
compact_prompt: self.compact_prompt.clone(),
|
||||
user_instructions: self.user_instructions.clone(),
|
||||
@@ -528,6 +530,7 @@ impl Session {
|
||||
current_date: Some(current_date),
|
||||
timezone: Some(timezone),
|
||||
app_server_client_name: session_configuration.app_server_client_name.clone(),
|
||||
client_compatibility_flags: session_configuration.client_compatibility_flags,
|
||||
developer_instructions: session_configuration.developer_instructions.clone(),
|
||||
compact_prompt: session_configuration.compact_prompt.clone(),
|
||||
user_instructions: session_configuration.user_instructions.clone(),
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::ClientCompatibilityFlags;
|
||||
use crate::SkillsManager;
|
||||
use crate::agent::AgentControl;
|
||||
use crate::codex_thread::CodexThread;
|
||||
@@ -219,6 +220,7 @@ pub struct StartThreadOptions {
|
||||
pub dynamic_tools: Vec<codex_protocol::dynamic_tools::DynamicToolSpec>,
|
||||
pub persist_extended_history: bool,
|
||||
pub metrics_service_name: Option<String>,
|
||||
pub client_compatibility_flags: ClientCompatibilityFlags,
|
||||
pub parent_trace: Option<W3cTraceContext>,
|
||||
pub environments: Vec<TurnEnvironmentSelection>,
|
||||
}
|
||||
@@ -559,6 +561,7 @@ impl ThreadManager {
|
||||
dynamic_tools,
|
||||
persist_extended_history,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
parent_trace: None,
|
||||
environments,
|
||||
}))
|
||||
@@ -581,6 +584,7 @@ impl ThreadManager {
|
||||
options.dynamic_tools,
|
||||
options.persist_extended_history,
|
||||
options.metrics_service_name,
|
||||
options.client_compatibility_flags,
|
||||
/*inherited_shell_snapshot*/ None,
|
||||
/*inherited_exec_policy*/ None,
|
||||
options.parent_trace,
|
||||
@@ -771,6 +775,31 @@ impl ThreadManager {
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn fork_thread_with_client_compatibility_flags<S>(
|
||||
&self,
|
||||
snapshot: S,
|
||||
config: Config,
|
||||
path: PathBuf,
|
||||
persist_extended_history: bool,
|
||||
parent_trace: Option<W3cTraceContext>,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
) -> CodexResult<NewThread>
|
||||
where
|
||||
S: Into<ForkSnapshot>,
|
||||
{
|
||||
let snapshot = snapshot.into();
|
||||
let history = RolloutRecorder::get_rollout_history(&path).await?;
|
||||
self.fork_thread_from_history_with_client_compatibility_flags(
|
||||
snapshot,
|
||||
config,
|
||||
history,
|
||||
persist_extended_history,
|
||||
parent_trace,
|
||||
client_compatibility_flags,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Fork an existing thread from already-loaded store history.
|
||||
pub async fn fork_thread_from_history<S>(
|
||||
&self,
|
||||
@@ -780,6 +809,29 @@ impl ThreadManager {
|
||||
persist_extended_history: bool,
|
||||
parent_trace: Option<W3cTraceContext>,
|
||||
) -> CodexResult<NewThread>
|
||||
where
|
||||
S: Into<ForkSnapshot>,
|
||||
{
|
||||
self.fork_thread_from_history_with_client_compatibility_flags(
|
||||
snapshot,
|
||||
config,
|
||||
history,
|
||||
persist_extended_history,
|
||||
parent_trace,
|
||||
ClientCompatibilityFlags::default(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn fork_thread_from_history_with_client_compatibility_flags<S>(
|
||||
&self,
|
||||
snapshot: S,
|
||||
config: Config,
|
||||
history: InitialHistory,
|
||||
persist_extended_history: bool,
|
||||
parent_trace: Option<W3cTraceContext>,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
) -> CodexResult<NewThread>
|
||||
where
|
||||
S: Into<ForkSnapshot>,
|
||||
{
|
||||
@@ -789,6 +841,7 @@ impl ThreadManager {
|
||||
history,
|
||||
persist_extended_history,
|
||||
parent_trace,
|
||||
client_compatibility_flags,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -800,6 +853,7 @@ impl ThreadManager {
|
||||
history: InitialHistory,
|
||||
persist_extended_history: bool,
|
||||
parent_trace: Option<W3cTraceContext>,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
) -> CodexResult<NewThread> {
|
||||
let interrupted_marker = InterruptedTurnHistoryMarker::from_config(&config);
|
||||
let history = fork_history_from_snapshot(snapshot, history, interrupted_marker);
|
||||
@@ -807,14 +861,18 @@ impl ThreadManager {
|
||||
self.state.environment_manager.as_ref(),
|
||||
&config.cwd,
|
||||
);
|
||||
Box::pin(self.state.spawn_thread(
|
||||
Box::pin(self.state.spawn_thread_with_source(
|
||||
config,
|
||||
history,
|
||||
Arc::clone(&self.state.auth_manager),
|
||||
self.agent_control(),
|
||||
self.state.session_source.clone(),
|
||||
Vec::new(),
|
||||
persist_extended_history,
|
||||
/*metrics_service_name*/ None,
|
||||
client_compatibility_flags,
|
||||
/*inherited_shell_snapshot*/ None,
|
||||
/*inherited_exec_policy*/ None,
|
||||
parent_trace,
|
||||
environments,
|
||||
/*user_shell_override*/ None,
|
||||
@@ -857,6 +915,44 @@ impl ThreadManagerState {
|
||||
}
|
||||
}
|
||||
|
||||
async fn client_compatibility_flags_for_source(
|
||||
&self,
|
||||
session_source: &SessionSource,
|
||||
explicit_flags: ClientCompatibilityFlags,
|
||||
) -> ClientCompatibilityFlags {
|
||||
// Root app-server threads receive explicit flags at startup. Thread-spawn
|
||||
// subagents inherit those flags from their parent so the same client
|
||||
// compatibility behavior applies throughout the visible thread tree.
|
||||
if explicit_flags != ClientCompatibilityFlags::default() {
|
||||
return explicit_flags;
|
||||
}
|
||||
|
||||
let SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id, ..
|
||||
}) = session_source
|
||||
else {
|
||||
return explicit_flags;
|
||||
};
|
||||
|
||||
match self.get_thread(*parent_thread_id).await {
|
||||
Ok(parent_thread) => {
|
||||
parent_thread
|
||||
.codex
|
||||
.session
|
||||
.client_compatibility_flags()
|
||||
.await
|
||||
}
|
||||
Err(error) => {
|
||||
warn!(
|
||||
error = %error,
|
||||
parent_thread_id = %parent_thread_id,
|
||||
"using default client compatibility flags: failed to load parent thread"
|
||||
);
|
||||
explicit_flags
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn read_stored_thread(
|
||||
&self,
|
||||
params: ReadThreadParams,
|
||||
@@ -952,6 +1048,7 @@ impl ThreadManagerState {
|
||||
Vec::new(),
|
||||
persist_extended_history,
|
||||
metrics_service_name,
|
||||
ClientCompatibilityFlags::default(),
|
||||
inherited_shell_snapshot,
|
||||
inherited_exec_policy,
|
||||
/*parent_trace*/ None,
|
||||
@@ -984,6 +1081,7 @@ impl ThreadManagerState {
|
||||
Vec::new(),
|
||||
/*persist_extended_history*/ false,
|
||||
/*metrics_service_name*/ None,
|
||||
ClientCompatibilityFlags::default(),
|
||||
inherited_shell_snapshot,
|
||||
inherited_exec_policy,
|
||||
/*parent_trace*/ None,
|
||||
@@ -1017,6 +1115,7 @@ impl ThreadManagerState {
|
||||
Vec::new(),
|
||||
persist_extended_history,
|
||||
/*metrics_service_name*/ None,
|
||||
ClientCompatibilityFlags::default(),
|
||||
inherited_shell_snapshot,
|
||||
inherited_exec_policy,
|
||||
/*parent_trace*/ None,
|
||||
@@ -1050,6 +1149,7 @@ impl ThreadManagerState {
|
||||
dynamic_tools,
|
||||
persist_extended_history,
|
||||
metrics_service_name,
|
||||
ClientCompatibilityFlags::default(),
|
||||
/*inherited_shell_snapshot*/ None,
|
||||
/*inherited_exec_policy*/ None,
|
||||
parent_trace,
|
||||
@@ -1070,6 +1170,7 @@ impl ThreadManagerState {
|
||||
dynamic_tools: Vec<codex_protocol::dynamic_tools::DynamicToolSpec>,
|
||||
persist_extended_history: bool,
|
||||
metrics_service_name: Option<String>,
|
||||
client_compatibility_flags: ClientCompatibilityFlags,
|
||||
inherited_shell_snapshot: Option<Arc<ShellSnapshot>>,
|
||||
inherited_exec_policy: Option<Arc<crate::exec_policy::ExecPolicyManager>>,
|
||||
parent_trace: Option<W3cTraceContext>,
|
||||
@@ -1116,6 +1217,9 @@ impl ThreadManagerState {
|
||||
let parent_rollout_thread_trace = self
|
||||
.parent_rollout_thread_trace_for_source(&session_source, &initial_history)
|
||||
.await;
|
||||
let client_compatibility_flags = self
|
||||
.client_compatibility_flags_for_source(&session_source, client_compatibility_flags)
|
||||
.await;
|
||||
let tracked_session_source = session_source.clone();
|
||||
let CodexSpawnOk {
|
||||
codex, thread_id, ..
|
||||
@@ -1134,6 +1238,7 @@ impl ThreadManagerState {
|
||||
dynamic_tools,
|
||||
persist_extended_history,
|
||||
metrics_service_name,
|
||||
client_compatibility_flags,
|
||||
inherited_shell_snapshot,
|
||||
inherited_exec_policy,
|
||||
parent_rollout_thread_trace,
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use super::*;
|
||||
use crate::ClientCompatibilityFlags;
|
||||
use crate::McpElicitationCompatibility;
|
||||
use crate::config::test_config;
|
||||
use crate::rollout::RolloutRecorder;
|
||||
use crate::session::session::SessionSettingsUpdate;
|
||||
@@ -319,6 +321,7 @@ async fn start_thread_accepts_explicit_environment_when_default_environment_is_d
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
parent_trace: None,
|
||||
environments: vec![TurnEnvironmentSelection {
|
||||
environment_id: "local".to_string(),
|
||||
@@ -331,6 +334,51 @@ async fn start_thread_accepts_explicit_environment_when_default_environment_is_d
|
||||
assert_eq!(manager.list_thread_ids().await, vec![thread.thread_id]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn start_thread_options_set_client_compatibility_flags_before_initial_session() {
|
||||
let temp_dir = tempdir().expect("tempdir");
|
||||
let mut config = test_config().await;
|
||||
config.codex_home = temp_dir.path().join("codex-home").abs();
|
||||
config.cwd = config.codex_home.abs();
|
||||
std::fs::create_dir_all(&config.codex_home).expect("create codex home");
|
||||
|
||||
let manager = ThreadManager::with_models_provider_and_home_for_tests(
|
||||
CodexAuth::from_api_key("dummy"),
|
||||
config.model_provider.clone(),
|
||||
config.codex_home.to_path_buf(),
|
||||
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
|
||||
);
|
||||
|
||||
let thread = manager
|
||||
.start_thread_with_options(StartThreadOptions {
|
||||
config,
|
||||
initial_history: InitialHistory::New,
|
||||
session_source: None,
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags {
|
||||
mcp_elicitation: McpElicitationCompatibility::CodexAppsOnly,
|
||||
},
|
||||
parent_trace: None,
|
||||
environments: Vec::new(),
|
||||
})
|
||||
.await
|
||||
.expect("thread should start with client compatibility flags");
|
||||
|
||||
assert_eq!(
|
||||
thread
|
||||
.thread
|
||||
.codex
|
||||
.session
|
||||
.client_compatibility_flags()
|
||||
.await,
|
||||
ClientCompatibilityFlags {
|
||||
mcp_elicitation: McpElicitationCompatibility::CodexAppsOnly,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn start_thread_keeps_internal_threads_hidden_from_normal_lookups() {
|
||||
let temp_dir = tempdir().expect("tempdir");
|
||||
@@ -355,6 +403,7 @@ async fn start_thread_keeps_internal_threads_hidden_from_normal_lookups() {
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
parent_trace: None,
|
||||
environments: Vec::new(),
|
||||
})
|
||||
@@ -406,6 +455,7 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() {
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags: ClientCompatibilityFlags::default(),
|
||||
parent_trace: None,
|
||||
environments: environments.clone(),
|
||||
})
|
||||
|
||||
@@ -244,6 +244,7 @@ impl MemoryStartupContext {
|
||||
dynamic_tools: Vec::new(),
|
||||
persist_extended_history: false,
|
||||
metrics_service_name: None,
|
||||
client_compatibility_flags: codex_core::ClientCompatibilityFlags::default(),
|
||||
parent_trace: None,
|
||||
environments,
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user