mirror of
https://github.com/openai/codex.git
synced 2026-11-15 06:51:36 +00:00
Honor explicit plugin mentions during MCP startup (#42406)
## Why Selected plugin MCP servers may be skipped after the optional startup grace period. When a user explicitly mentions a plugin or MCP server, the turn should continue waiting so the requested tools are available to the model. ## What changed - Track explicitly mentioned plugin IDs and MCP servers across later samples, steered input, and same-turn restarts. - Require matching selected-plugin MCP servers to finish startup before capturing the tool catalog, while leaving unmentioned servers optional. - Match `plugin://` identities independently of display names and ignore targeting query parameters such as `app` and `browserFamily`. ## Testing Added coverage for linked and structured plugin mentions, direct and batched capability discovery, delayed environment attachment, steering, and same-turn restarts. GitOrigin-RevId: c509ef8df45df1a7edb0da6afdb2682dc53a4de8
This commit is contained in:
1 parent
b7f710273e
commit
460b63e5f4
14 files changed
+617
-106
No files matched your search
@@ -329,6 +329,7 @@ impl Session {
|
||||
turn_context: &TurnContext,
|
||||
selected_capability_roots: &[ResolvedSelectedCapabilityRoot],
|
||||
required_servers: &[String],
|
||||
required_plugins: &HashSet<String>,
|
||||
) -> Arc<codex_mcp::McpBinding> {
|
||||
let ready_selected_capability_roots =
|
||||
Self::ready_selected_capability_roots(selected_capability_roots);
|
||||
@@ -365,7 +366,7 @@ impl Session {
|
||||
if let Some(binding) = self
|
||||
.services
|
||||
.mcp_runtime
|
||||
.current_binding_with_required_servers(&required_servers)
|
||||
.current_binding_with_requirements(&required_servers, required_plugins)
|
||||
.await
|
||||
{
|
||||
return binding;
|
||||
|
||||
@@ -3459,6 +3459,7 @@ impl Session {
|
||||
turn_context,
|
||||
cancellation_token,
|
||||
/*required_servers*/ &[],
|
||||
/*required_plugins*/ &HashSet::new(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -3468,9 +3469,15 @@ impl Session {
|
||||
turn_context: Arc<TurnContext>,
|
||||
cancellation_token: &CancellationToken,
|
||||
required_servers: &[String],
|
||||
required_plugins: &HashSet<String>,
|
||||
) -> CodexResult<Arc<StepContext>> {
|
||||
let step_context = self
|
||||
.capture_step_context_inner(turn_context, cancellation_token, required_servers)
|
||||
.capture_step_context_inner(
|
||||
turn_context,
|
||||
cancellation_token,
|
||||
required_servers,
|
||||
required_plugins,
|
||||
)
|
||||
.await?;
|
||||
self.set_last_known_step_context(&step_context).await;
|
||||
Ok(step_context)
|
||||
@@ -3487,6 +3494,7 @@ impl Session {
|
||||
turn_context,
|
||||
cancellation_token,
|
||||
/*required_servers*/ &[],
|
||||
/*required_plugins*/ &HashSet::new(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -3497,6 +3505,7 @@ impl Session {
|
||||
turn_context: Arc<TurnContext>,
|
||||
cancellation_token: &CancellationToken,
|
||||
required_servers: &[String],
|
||||
required_plugins: &HashSet<String>,
|
||||
) -> CodexResult<Arc<StepContext>> {
|
||||
// Capture once before asynchronous planning; all request consumers
|
||||
// retain this immutable settings version even if the turn is updated.
|
||||
@@ -3574,6 +3583,7 @@ impl Session {
|
||||
turn_context.as_ref(),
|
||||
&selected_capability_roots,
|
||||
required_servers,
|
||||
required_plugins,
|
||||
),
|
||||
turn::prepare_tool_recommendations(self.as_ref(), turn_context.as_ref()),
|
||||
)
|
||||
|
||||
@@ -9072,6 +9072,7 @@ async fn refresh_mcp_servers_uses_latest_state_for_existing_turns() {
|
||||
&turn_context,
|
||||
/*selected_capability_roots*/ &[],
|
||||
/*required_servers*/ &[],
|
||||
/*required_plugins*/ &HashSet::new(),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -138,6 +138,13 @@ use tracing::warn;
|
||||
|
||||
const POST_SAMPLING_TOKEN_ESTIMATE_TARGET: &str = "codex_core::post_sampling_token_estimate";
|
||||
|
||||
/// Explicit MCP startup requirements retained across restarts within one user turn.
|
||||
#[derive(Default)]
|
||||
pub(crate) struct McpStartupRequirements {
|
||||
required_servers: Vec<String>,
|
||||
required_plugins: HashSet<String>,
|
||||
}
|
||||
|
||||
/// Takes initial turn input and runs a loop where, at each sampling request,
|
||||
/// the model replies with either:
|
||||
///
|
||||
@@ -156,6 +163,7 @@ pub(crate) async fn run_turn(
|
||||
sess: Arc<Session>,
|
||||
turn_context: Arc<TurnContext>,
|
||||
input: Vec<TurnInput>,
|
||||
mcp_startup_requirements: &mut McpStartupRequirements,
|
||||
prewarmed_client_session: Option<ModelClientSession>,
|
||||
cancellation_token: CancellationToken,
|
||||
) -> CodexResult<Option<String>> {
|
||||
@@ -192,7 +200,16 @@ pub(crate) async fn run_turn(
|
||||
}
|
||||
|
||||
let user_input = turn_user_input(&input);
|
||||
let (required_servers, mentioned_plugins) =
|
||||
let allow_plugin_mentions =
|
||||
!crate::guardian::is_basic_session_source(&turn_context.session_source);
|
||||
let McpStartupRequirements {
|
||||
required_servers,
|
||||
required_plugins,
|
||||
} = mcp_startup_requirements;
|
||||
if allow_plugin_mentions {
|
||||
required_plugins.extend(crate::plugins::collect_explicit_plugin_ids(&user_input));
|
||||
}
|
||||
let (input_required_servers, mentioned_plugins) =
|
||||
match required_mcp_servers_for_input(&sess, turn_context.as_ref(), &user_input)
|
||||
.or_cancel(&cancellation_token)
|
||||
.await
|
||||
@@ -205,12 +222,17 @@ pub(crate) async fn run_turn(
|
||||
}
|
||||
};
|
||||
|
||||
required_servers.extend(input_required_servers);
|
||||
required_servers.sort_unstable();
|
||||
required_servers.dedup();
|
||||
|
||||
// run_turn owns the step used to seed context and make the first sampling request.
|
||||
let first_step_context = match sess
|
||||
.capture_step_context_with_required_mcp_servers(
|
||||
Arc::clone(&turn_context),
|
||||
&cancellation_token,
|
||||
&required_servers,
|
||||
required_servers,
|
||||
required_plugins,
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -343,24 +365,38 @@ pub(crate) async fn run_turn(
|
||||
|
||||
// Capture once so context, advertised tools, and tool calls share one request view.
|
||||
let step_context = match next_step_context.take() {
|
||||
Some(step_context) => step_context,
|
||||
Some(step_context) if pending_input.is_empty() => step_context,
|
||||
None if pending_input.is_empty() => {
|
||||
sess.capture_step_context(Arc::clone(&turn_context), &cancellation_token)
|
||||
.await?
|
||||
sess.capture_step_context_with_required_mcp_servers(
|
||||
Arc::clone(&turn_context),
|
||||
&cancellation_token,
|
||||
required_servers,
|
||||
required_plugins,
|
||||
)
|
||||
.await?
|
||||
}
|
||||
None => {
|
||||
Some(_) | None => {
|
||||
let pending_user_input = turn_user_input(&pending_input);
|
||||
let (required_servers, _) = required_mcp_servers_for_input(
|
||||
if allow_plugin_mentions {
|
||||
required_plugins.extend(crate::plugins::collect_explicit_plugin_ids(
|
||||
&pending_user_input,
|
||||
));
|
||||
}
|
||||
let (pending_required_servers, _) = required_mcp_servers_for_input(
|
||||
&sess,
|
||||
turn_context.as_ref(),
|
||||
&pending_user_input,
|
||||
)
|
||||
.or_cancel(&cancellation_token)
|
||||
.await?;
|
||||
required_servers.extend(pending_required_servers);
|
||||
required_servers.sort_unstable();
|
||||
required_servers.dedup();
|
||||
sess.capture_step_context_with_required_mcp_servers(
|
||||
Arc::clone(&turn_context),
|
||||
&cancellation_token,
|
||||
&required_servers,
|
||||
required_servers,
|
||||
required_plugins,
|
||||
)
|
||||
.await?
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user