Simplify Xcode MCP compatibility plumbing

This commit is contained in:
Eric Traut
2026-05-04 15:08:32 -07:00
parent 58fdaec898
commit efc64b0349
22 changed files with 137 additions and 328 deletions
+55 -87
View File
@@ -189,43 +189,12 @@ 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
@@ -235,11 +204,13 @@ fn client_compatibility_flags_for_app_server_client(
//
// TODO: Remove this Xcode 26.4 compatibility path once that client version
// has aged out of support.
if client_name.eq_ignore_ascii_case("xcode") && client_version.starts_with("26.4") {
flags.mcp_elicitation = McpElicitationCompatibility::CodexAppsOnly;
}
flags
let mcp_elicitation =
if client_name.eq_ignore_ascii_case("xcode") && client_version.starts_with("26.4") {
McpElicitationCompatibility::CodexAppsOnly
} else {
McpElicitationCompatibility::Default
};
ClientCompatibilityFlags { mcp_elicitation }
}
impl Default for ConnectionSessionState {
@@ -293,7 +264,12 @@ impl ConnectionSessionState {
pub(crate) fn client_compatibility_flags(&self) -> ClientCompatibilityFlags {
self.initialized
.get()
.map(|session| session.client_compatibility_flags)
.map(|session| {
client_compatibility_flags_for_app_server_client(
&session.app_server_client_name,
&session.client_version,
)
})
.unwrap_or_default()
}
@@ -433,6 +409,7 @@ impl MessageProcessor {
thread_state_manager.clone(),
);
let thread_processor = ThreadRequestProcessor::new(
auth_manager.clone(),
Arc::clone(&thread_manager),
outgoing.clone(),
analytics_events_client.clone(),
@@ -815,12 +792,10 @@ impl MessageProcessor {
);
let serialization_scope = codex_request.serialization_scope();
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 app_server_client_name = session.app_server_client_name().map(str::to_string);
let client_version = session.client_version().map(str::to_string);
let client_compatibility_flags = session.client_compatibility_flags();
let 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);
@@ -834,7 +809,10 @@ impl MessageProcessor {
connection_request_id,
codex_request,
request_context,
initialized_client_context,
app_server_client_name,
client_version,
client_compatibility_flags,
device_key_requests_allowed,
)
.await;
if let Err(error) = result {
@@ -857,12 +835,16 @@ impl MessageProcessor {
Ok(())
}
#[allow(clippy::too_many_arguments)]
async fn handle_initialized_client_request(
self: Arc<Self>,
connection_request_id: ConnectionRequestId,
codex_request: ClientRequest,
request_context: RequestContext,
initialized_client_context: InitializedClientRequestContext,
app_server_client_name: Option<String>,
client_version: Option<String>,
client_compatibility_flags: ClientCompatibilityFlags,
device_key_requests_allowed: bool,
) -> Result<(), JSONRPCErrorError> {
let connection_id = connection_request_id.connection_id;
let request_id = ConnectionRequestId {
@@ -909,7 +891,7 @@ impl MessageProcessor {
self.device_key_processor.create(
request_id.clone(),
params,
initialized_client_context.device_key_requests_allowed,
device_key_requests_allowed,
);
Ok(None)
}
@@ -917,7 +899,7 @@ impl MessageProcessor {
self.device_key_processor.public(
request_id.clone(),
params,
initialized_client_context.device_key_requests_allowed,
device_key_requests_allowed,
);
Ok(None)
}
@@ -925,7 +907,7 @@ impl MessageProcessor {
self.device_key_processor.sign(
request_id.clone(),
params,
initialized_client_context.device_key_requests_allowed,
device_key_requests_allowed,
);
Ok(None)
}
@@ -984,9 +966,9 @@ impl MessageProcessor {
.thread_start(
request_id.clone(),
params,
initialized_client_context.app_server_client_name.clone(),
initialized_client_context.client_version.clone(),
initialized_client_context.client_compatibility_flags,
app_server_client_name.clone(),
client_version.clone(),
client_compatibility_flags,
request_context,
)
.await
@@ -998,20 +980,12 @@ impl MessageProcessor {
}
ClientRequest::ThreadResume { params, .. } => {
self.thread_processor
.thread_resume(
request_id.clone(),
params,
initialized_client_context.client_compatibility_flags,
)
.thread_resume(request_id.clone(), params, client_compatibility_flags)
.await
}
ClientRequest::ThreadFork { params, .. } => {
self.thread_processor
.thread_fork(
request_id.clone(),
params,
initialized_client_context.client_compatibility_flags,
)
.thread_fork(request_id.clone(), params, client_compatibility_flags)
.await
}
ClientRequest::ThreadArchive { params, .. } => {
@@ -1165,8 +1139,8 @@ impl MessageProcessor {
.turn_start(
request_id.clone(),
params,
initialized_client_context.app_server_client_name.clone(),
initialized_client_context.client_version.clone(),
app_server_client_name.clone(),
client_version.clone(),
)
.await
}
@@ -1333,30 +1307,24 @@ mod client_compatibility_tests {
#[test]
fn xcode_26_4_uses_legacy_mcp_elicitation_compatibility() {
for (name, version) in [
("Xcode", "26.4"),
("xcode", "26.4"),
("Xcode", "26.4.1"),
("Xcode", "26.4-beta"),
] {
assert_eq!(
client_compatibility_flags_for_app_server_client(name, 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()
);
fn mcp_elicitation(name: &str, version: &str) -> McpElicitationCompatibility {
client_compatibility_flags_for_app_server_client(name, version).mcp_elicitation
}
assert_eq!(
mcp_elicitation("xcode", "26.4.1"),
McpElicitationCompatibility::CodexAppsOnly
);
assert_eq!(
mcp_elicitation("Xcode", "26.4-beta"),
McpElicitationCompatibility::CodexAppsOnly
);
assert_eq!(
mcp_elicitation("Xcode", "26.5"),
McpElicitationCompatibility::Default
);
assert_eq!(
mcp_elicitation("codex-vscode", "26.4"),
McpElicitationCompatibility::Default
);
}
}
@@ -310,7 +310,6 @@ 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::new(
.initialize(InitializedConnectionSessionState {
experimental_api_enabled,
opt_out_notification_methods.into_iter().collect(),
name.clone(),
version,
))
opted_out_notification_methods: opt_out_notification_methods.into_iter().collect(),
app_server_client_name: name.clone(),
client_version: version,
})
.is_err()
{
return Err(invalid_request("Already initialized"));
@@ -246,6 +246,7 @@ 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,
@@ -264,6 +265,7 @@ 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,
@@ -278,6 +280,7 @@ impl ThreadRequestProcessor {
thread_goal_processor: ThreadGoalRequestProcessor,
) -> Self {
Self {
auth_manager,
thread_manager,
outgoing,
analytics_events_client,
@@ -964,6 +967,7 @@ impl ThreadRequestProcessor {
.collect()
};
let core_dynamic_tool_count = core_dynamic_tools.len();
config.client_compatibility_flags = client_compatibility_flags;
let NewThread {
thread_id,
@@ -984,7 +988,6 @@ impl ThreadRequestProcessor {
dynamic_tools: core_dynamic_tools,
persist_extended_history,
metrics_service_name: service_name,
client_compatibility_flags,
parent_trace: request_trace,
environments,
})
@@ -2406,7 +2409,7 @@ impl ThreadRequestProcessor {
.await;
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
let config = match self
let mut config = match self
.config_manager
.load_for_cwd(request_overrides, typesafe_overrides, history_cwd)
.await
@@ -2418,25 +2421,20 @@ impl ThreadRequestProcessor {
return Ok(());
}
};
config.client_compatibility_flags = client_compatibility_flags;
let instruction_sources = Self::instruction_sources_from_config(&config).await;
let response_history = thread_history.clone();
match self
.thread_manager
.start_thread_with_options(StartThreadOptions {
config: config.clone(),
initial_history: thread_history,
session_source: None,
dynamic_tools: Vec::new(),
.resume_thread_with_history(
config.clone(),
thread_history,
self.auth_manager.clone(),
persist_extended_history,
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),
})
self.request_trace_context(&request_id).await,
)
.await
{
Ok(NewThread {
@@ -3022,11 +3020,12 @@ impl ThreadRequestProcessor {
);
typesafe_overrides.ephemeral = ephemeral.then_some(true);
// Derive a Config using the same logic as new conversation, honoring overrides if provided.
let config = self
let mut config = self
.config_manager
.load_for_cwd(request_overrides, typesafe_overrides, history_cwd)
.await
.map_err(|err| config_load_error(&err))?;
config.client_compatibility_flags = client_compatibility_flags;
let fallback_model_provider = config.model_provider_id.clone();
let instruction_sources = Self::instruction_sources_from_config(&config).await;
@@ -3038,7 +3037,7 @@ impl ThreadRequestProcessor {
..
} = self
.thread_manager
.fork_thread_from_history_with_client_compatibility_flags(
.fork_thread_from_history(
ForkSnapshot::Interrupted,
config,
InitialHistory::Resumed(ResumedHistory {
@@ -3048,7 +3047,6 @@ impl ThreadRequestProcessor {
}),
persist_extended_history,
self.request_trace_context(&request_id).await,
client_compatibility_flags,
)
.await
.map_err(|err| match err {
@@ -904,6 +904,7 @@ impl TurnRequestProcessor {
};
let mut config = self.config.as_ref().clone();
config.client_compatibility_flags = parent_thread.client_compatibility_flags().await;
if let Some(review_model) = &config.review_model {
config.model = Some(review_model.clone());
}
@@ -915,13 +916,12 @@ impl TurnRequestProcessor {
..
} = self
.thread_manager
.fork_thread_with_client_compatibility_flags(
.fork_thread(
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| {
@@ -202,7 +202,6 @@ 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(),
@@ -283,10 +283,6 @@ async fn codex_apps_only_custom_server_elicitation_is_declined_without_event() {
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"),
},
@@ -294,14 +290,8 @@ async fn codex_apps_only_custom_server_elicitation_is_declined_without_event() {
.await
.expect("elicitation should auto decline");
assert_eq!(
response,
ElicitationResponse {
action: ElicitationAction::Decline,
content: None,
meta: None,
}
);
assert_eq!(response.action, ElicitationAction::Decline);
assert_eq!(response.content, None);
assert!(matches!(
rx_event.try_recv(),
Err(async_channel::TryRecvError::Empty)
@@ -846,7 +836,6 @@ 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, McpElicitationCompatibility::Default);
assert_eq!(capability, Some(ElicitationCapability::default()));
assert_eq!(
serde_json::to_value(capability).expect("serialize elicitation capability"),
serde_json::json!({})
@@ -855,16 +844,14 @@ fn elicitation_capability_uses_2025_06_18_shape_for_all_servers() {
}
#[test]
fn codex_apps_only_elicitation_capability_keeps_codex_apps_enabled() {
fn codex_apps_only_elicitation_capability_suppresses_custom_servers() {
let compatibility = McpElicitationCompatibility::CodexAppsOnly;
assert_eq!(
elicitation_capability_for_server(
CODEX_APPS_MCP_SERVER_NAME,
McpElicitationCompatibility::CodexAppsOnly,
),
elicitation_capability_for_server(CODEX_APPS_MCP_SERVER_NAME, compatibility),
Some(ElicitationCapability::default())
);
assert_eq!(
elicitation_capability_for_server("custom_mcp", McpElicitationCompatibility::CodexAppsOnly,),
elicitation_capability_for_server("custom_mcp", compatibility),
None
);
}
+7 -6
View File
@@ -191,14 +191,15 @@ impl ElicitationRequestManager {
.boxed()
})
}
pub(crate) fn compatibility(&self) -> McpElicitationCompatibility {
self.compatibility
}
}
/// 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.
/// `CodexAppsOnly` preserves the pre-PR #17043 behavior for old clients by
/// keeping support for `codex_apps` while suppressing custom MCP servers.
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
pub enum McpElicitationCompatibility {
#[default]
@@ -207,7 +208,7 @@ pub enum McpElicitationCompatibility {
}
impl McpElicitationCompatibility {
pub(crate) fn allows_server_elicitation(self, server_name: &str) -> bool {
pub fn allows_server_elicitation(self, server_name: &str) -> bool {
match self {
Self::Default => true,
Self::CodexAppsOnly => server_name == CODEX_APPS_MCP_SERVER_NAME,
+2 -5
View File
@@ -139,7 +139,6 @@ 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,
@@ -182,7 +181,6 @@ impl AsyncManagedClient {
tool_filter: startup_tool_filter,
tx_event,
elicitation_requests,
elicitation_compatibility,
codex_apps_tools_cache_context,
},
)
@@ -466,10 +464,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, elicitation_compatibility);
let elicitation =
elicitation_capability_for_server(&server_name, elicitation_requests.compatibility());
let params = InitializeRequestParams {
meta: None,
capabilities: ClientCapabilities {
@@ -552,7 +550,6 @@ struct StartServerTaskParams {
tool_filter: ToolFilter,
tx_event: Sender<Event>,
elicitation_requests: ElicitationRequestManager,
elicitation_compatibility: McpElicitationCompatibility,
codex_apps_tools_cache_context: Option<CodexAppsToolsCacheContext>,
}
+2 -3
View File
@@ -62,7 +62,7 @@ use crate::session::completed_session_loop_termination;
/// The returned `ops_tx` allows the caller to submit additional `Op`s to the sub-agent.
#[allow(clippy::too_many_arguments)]
pub(crate) async fn run_codex_thread_interactive(
config: Config,
mut config: Config,
auth_manager: Arc<AuthManager>,
models_manager: SharedModelsManager,
parent_session: Arc<Session>,
@@ -73,7 +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;
config.client_compatibility_flags = parent_session.client_compatibility_flags().await;
let CodexSpawnOk { codex, .. } = Box::pin(Codex::spawn(CodexSpawnArgs {
config,
auth_manager,
@@ -89,7 +89,6 @@ 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)),
+4
View File
@@ -6447,6 +6447,7 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
check_for_update_on_startup: true,
disable_paste_burst: false,
tui_notifications: Default::default(),
client_compatibility_flags: Default::default(),
animations: true,
show_tooltips: true,
tui_vim_mode_default: false,
@@ -6649,6 +6650,7 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
check_for_update_on_startup: true,
disable_paste_burst: false,
tui_notifications: Default::default(),
client_compatibility_flags: Default::default(),
animations: true,
show_tooltips: true,
tui_vim_mode_default: false,
@@ -6805,6 +6807,7 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
check_for_update_on_startup: true,
disable_paste_burst: false,
tui_notifications: Default::default(),
client_compatibility_flags: Default::default(),
animations: true,
show_tooltips: true,
tui_vim_mode_default: false,
@@ -6946,6 +6949,7 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
check_for_update_on_startup: true,
disable_paste_burst: false,
tui_notifications: Default::default(),
client_compatibility_flags: Default::default(),
animations: true,
show_tooltips: true,
tui_vim_mode_default: false,
+6
View File
@@ -1,3 +1,4 @@
use crate::ClientCompatibilityFlags;
use crate::agents_md::AgentsMdManager;
use crate::config::edit::ConfigEdit;
use crate::config::edit::ConfigEditsBuilder;
@@ -505,6 +506,10 @@ pub struct Config {
/// TUI notification settings, including enabled events, delivery method, and focus condition.
pub tui_notifications: TuiNotificationSettings,
/// Runtime-only client compatibility flags selected by the app server
/// before creating a session. These are not loaded from config.toml.
pub client_compatibility_flags: ClientCompatibilityFlags,
/// Enable ASCII animations and shimmer effects in the TUI.
pub animations: bool,
@@ -3135,6 +3140,7 @@ impl Config {
.as_ref()
.map(|t| t.notification_settings.clone())
.unwrap_or_default(),
client_compatibility_flags: ClientCompatibilityFlags::default(),
animations: cfg.tui.as_ref().map(|t| t.animations).unwrap_or(true),
show_tooltips: cfg.tui.as_ref().map(|t| t.show_tooltips).unwrap_or(true),
model_availability_nux: cfg
+5 -1
View File
@@ -1099,7 +1099,11 @@ async fn maybe_request_mcp_tool_approval(
let tool_call_mcp_elicitation_enabled = turn_context
.config
.features
.enabled(Feature::ToolCallMcpElicitation);
.enabled(Feature::ToolCallMcpElicitation)
&& turn_context
.client_compatibility_flags
.mcp_elicitation
.allows_server_elicitation(&invocation.server);
if routes_approval_to_guardian(turn_context) {
let review_id = new_guardian_review_id();
+22 -8
View File
@@ -1,4 +1,6 @@
use super::*;
use crate::ClientCompatibilityFlags;
use crate::McpElicitationCompatibility;
use crate::config::ConfigBuilder;
use crate::session::tests::make_session_and_context;
use crate::session::tests::make_session_and_context_with_rx;
@@ -18,6 +20,7 @@ use codex_hooks::HooksConfig;
use codex_model_provider::create_model_provider;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::GranularApprovalConfig;
use core_test_support::PathExt;
use core_test_support::responses::ev_assistant_message;
@@ -2276,7 +2279,19 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
#[tokio::test]
async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval() {
let (session, turn_context, _rx_event) = make_session_and_context_with_rx().await;
let (session, mut turn_context, rx_event) = make_session_and_context_with_rx().await;
{
let turn_context = Arc::get_mut(&mut turn_context).expect("single turn context ref");
let mut config = (*turn_context.config).clone();
config
.features
.enable(Feature::ToolCallMcpElicitation)
.expect("test setup should allow enabling tool call MCP elicitation");
turn_context.config = Arc::new(config);
turn_context.client_compatibility_flags = ClientCompatibilityFlags {
mcp_elicitation: McpElicitationCompatibility::CodexAppsOnly,
};
}
{
let mut active_turn = session.active_turn.lock().await;
*active_turn = Some(ActiveTurn::default());
@@ -2302,7 +2317,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
openai_file_input_params: None,
};
let mut approval_task = {
let approval_task = {
let session = Arc::clone(&session);
let turn_context = Arc::clone(&turn_context);
tokio::spawn(async move {
@@ -2319,12 +2334,11 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
})
};
assert!(
tokio::time::timeout(std::time::Duration::from_millis(200), &mut approval_task)
.await
.is_err(),
"prompt mode should wait for approval instead of auto-allowing"
);
let event = tokio::time::timeout(std::time::Duration::from_secs(1), rx_event.recv())
.await
.expect("approval request should emit an event")
.expect("event channel should stay open");
assert!(matches!(event.msg, EventMsg::RequestUserInput(_)));
approval_task.abort();
}
+1 -3
View File
@@ -401,7 +401,6 @@ 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.
@@ -463,7 +462,6 @@ impl Codex {
dynamic_tools,
persist_extended_history,
metrics_service_name,
client_compatibility_flags,
inherited_shell_snapshot,
user_shell_override,
inherited_exec_policy,
@@ -615,7 +613,6 @@ 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,
@@ -843,6 +840,7 @@ impl Session {
.lock()
.await
.session_configuration
.original_config_do_not_use
.client_compatibility_flags
}
+1 -3
View File
@@ -1,5 +1,4 @@
use super::*;
use crate::ClientCompatibilityFlags;
use crate::goals::GoalRuntimeState;
use codex_otel::LEGACY_NOTIFY_CONFIGURED_METRIC;
use codex_protocol::permissions::FileSystemPath;
@@ -84,8 +83,6 @@ 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, ...)
@@ -1001,6 +998,7 @@ impl Session {
tool_plugin_provenance,
auth,
session_configuration
.original_config_do_not_use
.client_compatibility_flags
.mcp_elicitation,
)
-8
View File
@@ -1,6 +1,5 @@
use super::turn_context::TurnEnvironment;
use super::*;
use crate::ClientCompatibilityFlags;
use crate::config::ConfigBuilder;
use crate::config::test_config;
use crate::context::ContextualUserFragment;
@@ -2373,7 +2372,6 @@ 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,
@@ -2477,7 +2475,6 @@ 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,
@@ -2936,7 +2933,6 @@ 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,
@@ -3462,7 +3458,6 @@ 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,
@@ -3568,7 +3563,6 @@ 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,
@@ -3781,7 +3775,6 @@ 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,
@@ -5062,7 +5055,6 @@ 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,7 +750,6 @@ 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(),
+3 -1
View File
@@ -530,7 +530,9 @@ 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,
client_compatibility_flags: session_configuration
.original_config_do_not_use
.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 -106
View File
@@ -1,4 +1,3 @@
use crate::ClientCompatibilityFlags;
use crate::SkillsManager;
use crate::agent::AgentControl;
use crate::codex_thread::CodexThread;
@@ -220,7 +219,6 @@ 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>,
}
@@ -561,7 +559,6 @@ impl ThreadManager {
dynamic_tools,
persist_extended_history,
metrics_service_name: None,
client_compatibility_flags: ClientCompatibilityFlags::default(),
parent_trace: None,
environments,
}))
@@ -584,7 +581,6 @@ 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,
@@ -775,31 +771,6 @@ 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,
@@ -809,29 +780,6 @@ 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>,
{
@@ -841,7 +789,6 @@ impl ThreadManager {
history,
persist_extended_history,
parent_trace,
client_compatibility_flags,
)
.await
}
@@ -853,7 +800,6 @@ 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);
@@ -861,18 +807,14 @@ impl ThreadManager {
self.state.environment_manager.as_ref(),
&config.cwd,
);
Box::pin(self.state.spawn_thread_with_source(
Box::pin(self.state.spawn_thread(
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,
@@ -915,44 +857,6 @@ 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,
@@ -1048,7 +952,6 @@ impl ThreadManagerState {
Vec::new(),
persist_extended_history,
metrics_service_name,
ClientCompatibilityFlags::default(),
inherited_shell_snapshot,
inherited_exec_policy,
/*parent_trace*/ None,
@@ -1081,7 +984,6 @@ impl ThreadManagerState {
Vec::new(),
/*persist_extended_history*/ false,
/*metrics_service_name*/ None,
ClientCompatibilityFlags::default(),
inherited_shell_snapshot,
inherited_exec_policy,
/*parent_trace*/ None,
@@ -1115,7 +1017,6 @@ impl ThreadManagerState {
Vec::new(),
persist_extended_history,
/*metrics_service_name*/ None,
ClientCompatibilityFlags::default(),
inherited_shell_snapshot,
inherited_exec_policy,
/*parent_trace*/ None,
@@ -1149,7 +1050,6 @@ impl ThreadManagerState {
dynamic_tools,
persist_extended_history,
metrics_service_name,
ClientCompatibilityFlags::default(),
/*inherited_shell_snapshot*/ None,
/*inherited_exec_policy*/ None,
parent_trace,
@@ -1170,7 +1070,6 @@ 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>,
@@ -1217,9 +1116,6 @@ 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, ..
@@ -1238,7 +1134,6 @@ impl ThreadManagerState {
dynamic_tools,
persist_extended_history,
metrics_service_name,
client_compatibility_flags,
inherited_shell_snapshot,
inherited_exec_policy,
parent_rollout_thread_trace,
-50
View File
@@ -1,6 +1,4 @@
use super::*;
use crate::ClientCompatibilityFlags;
use crate::McpElicitationCompatibility;
use crate::config::test_config;
use crate::rollout::RolloutRecorder;
use crate::session::session::SessionSettingsUpdate;
@@ -321,7 +319,6 @@ 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(),
@@ -334,51 +331,6 @@ 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");
@@ -403,7 +355,6 @@ 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(),
})
@@ -455,7 +406,6 @@ 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(),
})
-1
View File
@@ -244,7 +244,6 @@ 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,
})