mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Auto-deny MCP elicitations for Xcode 26.4 clients (#21113)
## Summary Xcode 26.4 was built against app-server behavior from before MCP elicitation requests became client-visible in CLI 0.120.0 via #17043. That client line does not expect the new events/messages, so this PR restores the old behavior for exactly that client/version combination. The compatibility handling stays in the app-server layer: when the initialized client is `Xcode` and its version starts with `26.4`, the app server marks the live Codex thread so MCP elicitations are auto-denied. The flag is applied on thread start/resume/fork/turn attachment, carried through `Codex`/`CodexThread`, and stored on `McpConnectionManager` so refreshed MCP managers preserve the behavior. ## Notes This is intentionally narrow and includes a TODO to remove the compatibility path once Xcode 26.4 ages out.
This commit is contained in:
committed by
GitHub
Unverified
parent
f593323ef1
commit
8c88f9a304
@@ -954,12 +954,22 @@ impl MessageProcessor {
|
||||
}
|
||||
ClientRequest::ThreadResume { params, .. } => {
|
||||
self.thread_processor
|
||||
.thread_resume(request_id.clone(), params)
|
||||
.thread_resume(
|
||||
request_id.clone(),
|
||||
params,
|
||||
app_server_client_name.clone(),
|
||||
client_version.clone(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
ClientRequest::ThreadFork { params, .. } => {
|
||||
self.thread_processor
|
||||
.thread_fork(request_id.clone(), params)
|
||||
.thread_fork(
|
||||
request_id.clone(),
|
||||
params,
|
||||
app_server_client_name.clone(),
|
||||
client_version.clone(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
ClientRequest::ThreadArchive { params, .. } => {
|
||||
|
||||
@@ -329,20 +329,34 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadResumeParams,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
|
||||
self.thread_resume_inner(request_id, params)
|
||||
.await
|
||||
.map(|()| None)
|
||||
self.thread_resume_inner(
|
||||
request_id,
|
||||
params,
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
)
|
||||
.await
|
||||
.map(|()| None)
|
||||
}
|
||||
|
||||
pub(crate) async fn thread_fork(
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadForkParams,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
|
||||
self.thread_fork_inner(request_id, params)
|
||||
.await
|
||||
.map(|()| None)
|
||||
self.thread_fork_inner(
|
||||
request_id,
|
||||
params,
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
)
|
||||
.await
|
||||
.map(|()| None)
|
||||
}
|
||||
|
||||
pub(crate) async fn thread_archive(
|
||||
@@ -590,8 +604,16 @@ impl ThreadRequestProcessor {
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
let mcp_elicitations_auto_deny = xcode_26_4_mcp_elicitations_auto_deny(
|
||||
app_server_client_name.as_deref(),
|
||||
app_server_client_version.as_deref(),
|
||||
);
|
||||
thread
|
||||
.set_app_server_client_info(app_server_client_name, app_server_client_version)
|
||||
.set_app_server_client_info(
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
mcp_elicitations_auto_deny,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| internal_error(format!("failed to set app server client info: {err}")))
|
||||
}
|
||||
@@ -2171,6 +2193,8 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadResumeParams,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
if let Ok(thread_id) = ThreadId::from_string(¶ms.thread_id)
|
||||
&& self
|
||||
@@ -2211,7 +2235,15 @@ impl ThreadRequestProcessor {
|
||||
return Ok(());
|
||||
}
|
||||
};
|
||||
match self.resume_running_thread(&request_id, ¶ms).await {
|
||||
match self
|
||||
.resume_running_thread(
|
||||
&request_id,
|
||||
¶ms,
|
||||
app_server_client_name.clone(),
|
||||
app_server_client_version.clone(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(true) => return Ok(()),
|
||||
Ok(false) => {}
|
||||
Err(error) => {
|
||||
@@ -2312,6 +2344,16 @@ impl ThreadRequestProcessor {
|
||||
session_configured,
|
||||
..
|
||||
}) => {
|
||||
if let Err(err) = Self::set_app_server_client_info(
|
||||
codex_thread.as_ref(),
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
)
|
||||
.await
|
||||
{
|
||||
self.outgoing.send_error(request_id, err).await;
|
||||
return Ok(());
|
||||
}
|
||||
let SessionConfiguredEvent { rollout_path, .. } = session_configured;
|
||||
let Some(rollout_path) = rollout_path else {
|
||||
let error =
|
||||
@@ -2448,6 +2490,8 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: &ConnectionRequestId,
|
||||
params: &ThreadResumeParams,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
) -> Result<bool, JSONRPCErrorError> {
|
||||
let running_thread = if params.history.is_some() {
|
||||
if let Ok(existing_thread_id) = ThreadId::from_string(¶ms.thread_id)
|
||||
@@ -2529,6 +2573,12 @@ impl ThreadRequestProcessor {
|
||||
thread_state.clone(),
|
||||
)
|
||||
.await?;
|
||||
Self::set_app_server_client_info(
|
||||
existing_thread.as_ref(),
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let config_snapshot = existing_thread.config_snapshot().await;
|
||||
let mismatch_details = collect_resume_override_mismatches(params, &config_snapshot);
|
||||
@@ -2812,6 +2862,8 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
params: ThreadForkParams,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
let ThreadForkParams {
|
||||
thread_id,
|
||||
@@ -2930,6 +2982,13 @@ impl ThreadRequestProcessor {
|
||||
err => internal_error(format!("error forking thread: {err}")),
|
||||
})?;
|
||||
|
||||
Self::set_app_server_client_info(
|
||||
forked_thread.as_ref(),
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
)
|
||||
.await?;
|
||||
|
||||
// Auto-attach a conversation listener when forking a thread.
|
||||
log_listener_attach_result(
|
||||
self.ensure_conversation_listener(
|
||||
@@ -3186,6 +3245,17 @@ impl ThreadRequestProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
fn xcode_26_4_mcp_elicitations_auto_deny(
|
||||
client_name: Option<&str>,
|
||||
client_version: Option<&str>,
|
||||
) -> bool {
|
||||
// Xcode 26.4 shipped before app-server MCP elicitation requests were
|
||||
// client-visible. Keep elicitations auto-denied for that client line.
|
||||
// TODO: Remove this compatibility hack once Xcode 26.4 ages out.
|
||||
client_name == Some("Xcode")
|
||||
&& client_version.is_some_and(|version| version.starts_with("26.4"))
|
||||
}
|
||||
|
||||
const THREAD_TURNS_DEFAULT_LIMIT: usize = 25;
|
||||
const THREAD_TURNS_MAX_LIMIT: usize = 100;
|
||||
|
||||
|
||||
@@ -545,8 +545,16 @@ impl TurnRequestProcessor {
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
) -> Result<(), JSONRPCErrorError> {
|
||||
let mcp_elicitations_auto_deny = xcode_26_4_mcp_elicitations_auto_deny(
|
||||
app_server_client_name.as_deref(),
|
||||
app_server_client_version.as_deref(),
|
||||
);
|
||||
thread
|
||||
.set_app_server_client_info(app_server_client_name, app_server_client_version)
|
||||
.set_app_server_client_info(
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
mcp_elicitations_auto_deny,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| internal_error(format!("failed to set app server client info: {err}")))
|
||||
}
|
||||
@@ -1096,3 +1104,14 @@ impl TurnRequestProcessor {
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
fn xcode_26_4_mcp_elicitations_auto_deny(
|
||||
client_name: Option<&str>,
|
||||
client_version: Option<&str>,
|
||||
) -> bool {
|
||||
// Xcode 26.4 shipped before app-server MCP elicitation requests were
|
||||
// client-visible. Keep elicitations auto-denied for that client line.
|
||||
// TODO: Remove this compatibility hack once Xcode 26.4 ages out.
|
||||
client_name == Some("Xcode")
|
||||
&& client_version.is_some_and(|version| version.starts_with("26.4"))
|
||||
}
|
||||
|
||||
@@ -128,6 +128,14 @@ impl McpConnectionManager {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn elicitations_auto_deny(&self) -> bool {
|
||||
self.elicitation_requests.auto_deny()
|
||||
}
|
||||
|
||||
pub fn set_elicitations_auto_deny(&self, auto_deny: bool) {
|
||||
self.elicitation_requests.set_auto_deny(auto_deny);
|
||||
}
|
||||
|
||||
#[allow(clippy::new_ret_no_self, clippy::too_many_arguments)]
|
||||
pub async fn new(
|
||||
mcp_servers: &HashMap<String, McpServerConfig>,
|
||||
|
||||
@@ -36,6 +36,7 @@ pub(crate) struct ElicitationRequestManager {
|
||||
requests: Arc<Mutex<ResponderMap>>,
|
||||
pub(crate) approval_policy: Arc<StdMutex<AskForApproval>>,
|
||||
pub(crate) permission_profile: Arc<StdMutex<PermissionProfile>>,
|
||||
auto_deny: Arc<StdMutex<bool>>,
|
||||
}
|
||||
|
||||
impl ElicitationRequestManager {
|
||||
@@ -47,6 +48,20 @@ impl ElicitationRequestManager {
|
||||
requests: Arc::new(Mutex::new(HashMap::new())),
|
||||
approval_policy: Arc::new(StdMutex::new(approval_policy)),
|
||||
permission_profile: Arc::new(StdMutex::new(permission_profile)),
|
||||
auto_deny: Arc::new(StdMutex::new(false)),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn auto_deny(&self) -> bool {
|
||||
self.auto_deny
|
||||
.lock()
|
||||
.map(|auto_deny| *auto_deny)
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
pub(crate) fn set_auto_deny(&self, auto_deny: bool) {
|
||||
if let Ok(mut current) = self.auto_deny.lock() {
|
||||
*current = auto_deny;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -73,13 +88,27 @@ impl ElicitationRequestManager {
|
||||
let elicitation_requests = self.requests.clone();
|
||||
let approval_policy = self.approval_policy.clone();
|
||||
let permission_profile = self.permission_profile.clone();
|
||||
let auto_deny = self.auto_deny.clone();
|
||||
Box::new(move |id, elicitation| {
|
||||
let elicitation_requests = elicitation_requests.clone();
|
||||
let tx_event = tx_event.clone();
|
||||
let server_name = server_name.clone();
|
||||
let approval_policy = approval_policy.clone();
|
||||
let permission_profile = permission_profile.clone();
|
||||
let auto_deny = auto_deny.clone();
|
||||
async move {
|
||||
let auto_deny = auto_deny
|
||||
.lock()
|
||||
.map(|auto_deny| *auto_deny)
|
||||
.unwrap_or(false);
|
||||
if auto_deny {
|
||||
return Ok(ElicitationResponse {
|
||||
action: ElicitationAction::Decline,
|
||||
content: None,
|
||||
meta: None,
|
||||
});
|
||||
}
|
||||
|
||||
let approval_policy = approval_policy
|
||||
.lock()
|
||||
.map(|policy| *policy)
|
||||
|
||||
@@ -221,9 +221,14 @@ impl CodexThread {
|
||||
&self,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
mcp_elicitations_auto_deny: bool,
|
||||
) -> ConstraintResult<()> {
|
||||
self.codex
|
||||
.set_app_server_client_info(app_server_client_name, app_server_client_version)
|
||||
.set_app_server_client_info(
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
mcp_elicitations_auto_deny,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
|
||||
@@ -254,6 +254,10 @@ impl Session {
|
||||
auth.as_ref(),
|
||||
)
|
||||
.await;
|
||||
{
|
||||
let current_manager = self.services.mcp_connection_manager.read().await;
|
||||
refreshed_manager.set_elicitations_auto_deny(current_manager.elicitations_auto_deny());
|
||||
}
|
||||
{
|
||||
let mut guard = self.services.mcp_startup_cancellation_token.lock().await;
|
||||
if guard.is_cancelled() {
|
||||
|
||||
@@ -758,6 +758,7 @@ impl Codex {
|
||||
&self,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
mcp_elicitations_auto_deny: bool,
|
||||
) -> ConstraintResult<()> {
|
||||
self.session
|
||||
.update_settings(SessionSettingsUpdate {
|
||||
@@ -765,7 +766,10 @@ impl Codex {
|
||||
app_server_client_version,
|
||||
..Default::default()
|
||||
})
|
||||
.await
|
||||
.await?;
|
||||
let mcp_connection_manager = self.session.services.mcp_connection_manager.read().await;
|
||||
mcp_connection_manager.set_elicitations_auto_deny(mcp_elicitations_auto_deny);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) async fn agent_status(&self) -> AgentStatus {
|
||||
|
||||
Reference in New Issue
Block a user