mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Make plugin guidance react to environment readiness (#30223)
## Why Generic plugin guidance is currently emitted only with initial context from host plugin state. An executor-selected plugin can become available later in the same turn, making its skills and tools usable without ever telling the model how plugin capabilities should be used. ## What - project every ready selected plugin package, including skill-only plugins - carry plugin availability with the exact MCP runtime projection while preserving MCP manager reuse when servers and connectors are unchanged - move generic plugin guidance from the static initial-context path into persisted World State - recognize legacy and retained plugin fragments so resume and compaction do not duplicate guidance ## Testing - `just test -p codex-mcp-extension` - `just test -p codex-core plugins_instructions` - `just test -p codex-core plugin_availability_change_reuses_the_mcp_manager` - `just test -p codex-app-server --test all selected_capabilit`
This commit is contained in:
@@ -28,6 +28,7 @@ use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
use codex_protocol::config_types::CollaborationMode;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::process::wait_for_pid_file;
|
||||
@@ -235,7 +236,9 @@ async fn selected_capability_stack_tracks_environment_availability_and_resume()
|
||||
for request in &requests[1..4] {
|
||||
assert_selected_skill_is_injected(request, /*expected_count*/ 1);
|
||||
assert_selected_plugin_tools(request);
|
||||
assert_plugin_guidance_count(request, /*expected_count*/ 1);
|
||||
}
|
||||
assert_plugin_guidance_count(&requests[4], /*expected_count*/ 1);
|
||||
assert_selected_skill_is_injected(&requests[5], /*expected_count*/ 2);
|
||||
assert_selected_plugin_tools(&requests[5]);
|
||||
let output = requests[2].function_call_output(MCP_CALL_ID);
|
||||
@@ -398,7 +401,9 @@ async fn selected_capabilities_become_available_between_samples_in_one_turn() ->
|
||||
assert_eq!(3, requests.len());
|
||||
assert_selected_skill_catalog_available(&requests[1]);
|
||||
assert_selected_plugin_tools(&requests[1]);
|
||||
assert_plugin_guidance_count(&requests[1], /*expected_count*/ 1);
|
||||
assert_selected_plugin_tools(&requests[2]);
|
||||
assert_plugin_guidance_count(&requests[2], /*expected_count*/ 1);
|
||||
let output = requests[2].function_call_output(MCP_CALL_ID);
|
||||
let output = output["output"]
|
||||
.as_str()
|
||||
@@ -537,6 +542,7 @@ fn assert_selected_capabilities_absent(request: &ResponsesRequest) {
|
||||
.all(|text| !text.contains(SKILL_DESCRIPTION))
|
||||
);
|
||||
assert_selected_plugin_tools_absent(request);
|
||||
assert_plugin_guidance_count(request, /*expected_count*/ 0);
|
||||
}
|
||||
|
||||
fn assert_selected_plugin_tools_absent(request: &ResponsesRequest) {
|
||||
@@ -555,6 +561,17 @@ fn assert_selected_plugin_tools_absent(request: &ResponsesRequest) {
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_plugin_guidance_count(request: &ResponsesRequest, expected_count: usize) {
|
||||
assert_eq!(
|
||||
expected_count,
|
||||
request
|
||||
.message_input_texts("developer")
|
||||
.into_iter()
|
||||
.filter(|text| text.starts_with(PLUGINS_INSTRUCTIONS_OPEN_TAG))
|
||||
.count()
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_selected_skill_is_injected(request: &ResponsesRequest, expected_count: usize) {
|
||||
assert_selected_skill_catalog_available(request);
|
||||
|
||||
|
||||
@@ -52,6 +52,7 @@ use codex_config::types::TuiNotificationSettings;
|
||||
use codex_config::types::TuiPetAnchor;
|
||||
use codex_config::types::UriBasedFileOpener;
|
||||
use codex_config::types::WindowsSandboxModeToml;
|
||||
use codex_core_plugins::PluginLoadOutcome;
|
||||
use codex_core_plugins::PluginsConfigInput;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
@@ -1528,6 +1529,14 @@ impl Config {
|
||||
) -> McpConfig {
|
||||
let plugins_input = self.plugins_config_input();
|
||||
let loaded_plugins = plugins_manager.plugins_for_config(&plugins_input).await;
|
||||
self.to_mcp_config_with_loaded_plugins(&loaded_plugins, additional_plugin_registrations)
|
||||
}
|
||||
|
||||
pub(crate) fn to_mcp_config_with_loaded_plugins(
|
||||
&self,
|
||||
loaded_plugins: &PluginLoadOutcome,
|
||||
additional_plugin_registrations: impl IntoIterator<Item = McpServerRegistration>,
|
||||
) -> McpConfig {
|
||||
let mut catalog = ResolvedMcpCatalog::builder();
|
||||
for (plugin_order, plugin) in loaded_plugins
|
||||
.plugins()
|
||||
|
||||
@@ -1,25 +1,10 @@
|
||||
use codex_plugin::PluginCapabilitySummary;
|
||||
use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_CLOSE_TAG;
|
||||
use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG;
|
||||
|
||||
use super::ContextualUserFragment;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub(crate) struct AvailablePluginsInstructions {
|
||||
plugins: Vec<PluginCapabilitySummary>,
|
||||
}
|
||||
|
||||
impl AvailablePluginsInstructions {
|
||||
pub(crate) fn from_plugins(plugins: &[PluginCapabilitySummary]) -> Option<Self> {
|
||||
if plugins.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(Self {
|
||||
plugins: plugins.to_vec(),
|
||||
})
|
||||
}
|
||||
}
|
||||
pub(crate) struct AvailablePluginsInstructions;
|
||||
|
||||
impl ContextualUserFragment for AvailablePluginsInstructions {
|
||||
fn role(&self) -> &'static str {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
mod agents_md;
|
||||
mod environment;
|
||||
mod plugins_instructions;
|
||||
|
||||
use crate::context::ContextualUserFragment;
|
||||
use codex_extension_api::PreviousWorldStateSection;
|
||||
@@ -17,6 +18,7 @@ use std::fmt;
|
||||
|
||||
pub(crate) use agents_md::AgentsMdState;
|
||||
pub(crate) use environment::EnvironmentsState;
|
||||
pub(crate) use plugins_instructions::PluginsInstructionsState;
|
||||
|
||||
trait ErasedWorldStateSection: Send + Sync {
|
||||
fn snapshot(&self) -> Option<Value>;
|
||||
@@ -62,11 +64,11 @@ impl<S: WorldStateSection> ErasedWorldStateSection for S {
|
||||
}
|
||||
|
||||
fn has_retained_fragment_matcher(&self) -> bool {
|
||||
false
|
||||
S::has_retained_fragment_matcher()
|
||||
}
|
||||
|
||||
fn matches_retained_fragment(&self, _role: &str, _text: &str) -> bool {
|
||||
false
|
||||
fn matches_retained_fragment(&self, role: &str, text: &str) -> bool {
|
||||
S::matches_retained_fragment(role, text)
|
||||
}
|
||||
|
||||
fn render_diff(
|
||||
@@ -183,6 +185,16 @@ pub(crate) trait WorldStateSection: Send + Sync + 'static {
|
||||
false
|
||||
}
|
||||
|
||||
/// Whether retained history must still contain this section's rendered fragment.
|
||||
fn has_retained_fragment_matcher() -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
/// Recognizes this section's rendered fragment in retained model history.
|
||||
fn matches_retained_fragment(_role: &str, _text: &str) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
fn render_diff(
|
||||
&self,
|
||||
previous: PreviousSectionState<'_, Self::Snapshot>,
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
use super::PreviousSectionState;
|
||||
use super::WorldStateSection;
|
||||
use crate::context::AvailablePluginsInstructions;
|
||||
use crate::context::ContextualUserFragment;
|
||||
|
||||
/// Whether generic plugin usage guidance should be visible to the model.
|
||||
#[derive(Clone, Copy, Debug, Default)]
|
||||
pub(crate) struct PluginsInstructionsState {
|
||||
available: bool,
|
||||
}
|
||||
|
||||
impl PluginsInstructionsState {
|
||||
pub(crate) fn new(available: bool) -> Self {
|
||||
Self { available }
|
||||
}
|
||||
}
|
||||
|
||||
impl WorldStateSection for PluginsInstructionsState {
|
||||
const ID: &'static str = "plugins_instructions";
|
||||
type Snapshot = bool;
|
||||
|
||||
fn snapshot(&self) -> Self::Snapshot {
|
||||
self.available
|
||||
}
|
||||
|
||||
fn matches_legacy_fragment(role: &str, text: &str) -> bool {
|
||||
role == "developer" && AvailablePluginsInstructions::matches_text(text)
|
||||
}
|
||||
|
||||
fn has_retained_fragment_matcher() -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
fn matches_retained_fragment(role: &str, text: &str) -> bool {
|
||||
Self::matches_legacy_fragment(role, text)
|
||||
}
|
||||
|
||||
fn render_diff(
|
||||
&self,
|
||||
previous: PreviousSectionState<'_, Self::Snapshot>,
|
||||
) -> Option<Box<dyn ContextualUserFragment>> {
|
||||
if !self.available
|
||||
|| matches!(previous, PreviousSectionState::Known(previous) if *previous)
|
||||
|| matches!(previous, PreviousSectionState::Unknown)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(Box::new(AvailablePluginsInstructions))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "plugins_instructions_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,72 @@
|
||||
use super::*;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn render(
|
||||
state: PluginsInstructionsState,
|
||||
previous: PreviousSectionState<'_, bool>,
|
||||
) -> Vec<String> {
|
||||
state
|
||||
.render_diff(previous)
|
||||
.into_iter()
|
||||
.map(|fragment| fragment.render())
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_only_when_plugins_become_available() {
|
||||
let unavailable = PluginsInstructionsState::new(/*available*/ false);
|
||||
let available = PluginsInstructionsState::new(/*available*/ true);
|
||||
let false_snapshot = false;
|
||||
let true_snapshot = true;
|
||||
|
||||
assert_eq!(
|
||||
render(unavailable, PreviousSectionState::Absent),
|
||||
Vec::<String>::new()
|
||||
);
|
||||
assert_eq!(render(available, PreviousSectionState::Absent).len(), 1);
|
||||
assert_eq!(
|
||||
render(available, PreviousSectionState::Known(&false_snapshot)).len(),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
render(available, PreviousSectionState::Known(&true_snapshot)),
|
||||
Vec::<String>::new()
|
||||
);
|
||||
assert_eq!(
|
||||
render(unavailable, PreviousSectionState::Known(&true_snapshot)),
|
||||
Vec::<String>::new()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_guidance_is_not_injected_again() {
|
||||
let mut world_state = super::super::WorldState::default();
|
||||
world_state.add_section(PluginsInstructionsState::new(/*available*/ true));
|
||||
let legacy: ResponseItem = ContextualUserFragment::into(AvailablePluginsInstructions);
|
||||
|
||||
assert!(
|
||||
world_state
|
||||
.render_history_diff(/*previous*/ None, &[legacy])
|
||||
.is_empty()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn persisted_guidance_is_restored_only_when_missing_from_history() {
|
||||
let mut world_state = super::super::WorldState::default();
|
||||
world_state.add_section(PluginsInstructionsState::new(/*available*/ true));
|
||||
let snapshot = world_state.snapshot();
|
||||
let retained: ResponseItem = ContextualUserFragment::into(AvailablePluginsInstructions);
|
||||
|
||||
assert_eq!(
|
||||
world_state.render_history_diff(Some(&snapshot), &[]).len(),
|
||||
1
|
||||
);
|
||||
assert!(
|
||||
world_state
|
||||
.render_history_diff(Some(&snapshot), &[retained])
|
||||
.is_empty()
|
||||
);
|
||||
}
|
||||
@@ -22,6 +22,7 @@ use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -1021,6 +1022,9 @@ fn drop_last_n_user_turns_trims_context_updates_above_rolled_back_turn() {
|
||||
user_input_text_msg("turn 1 user"),
|
||||
assistant_msg("turn 1 assistant"),
|
||||
developer_msg("Generated images are saved to /tmp as /tmp/image-1.png by default."),
|
||||
developer_msg(&format!(
|
||||
"{PLUGINS_INSTRUCTIONS_OPEN_TAG}\nROLLED_BACK_PLUGIN_INSTRUCTIONS"
|
||||
)),
|
||||
developer_msg("<collaboration_mode>ROLLED_BACK_DEV_INSTRUCTIONS</collaboration_mode>"),
|
||||
developer_msg("<multi_agent_mode>ROLLED_BACK_MULTI_AGENT_MODE</multi_agent_mode>"),
|
||||
user_input_text_msg(
|
||||
|
||||
@@ -18,6 +18,7 @@ use codex_protocol::protocol::COLLABORATION_MODE_OPEN_TAG;
|
||||
use codex_protocol::protocol::CONTEXT_WINDOW_GUIDANCE_OPEN_TAG;
|
||||
use codex_protocol::protocol::CONTEXT_WINDOW_OPEN_TAG;
|
||||
use codex_protocol::protocol::MULTI_AGENT_MODE_OPEN_TAG;
|
||||
use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG;
|
||||
use codex_protocol::protocol::REALTIME_CONVERSATION_OPEN_TAG;
|
||||
use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
@@ -33,6 +34,7 @@ const CONTEXTUAL_DEVELOPER_PREFIXES: &[&str] = &[
|
||||
"<model_switch>",
|
||||
COLLABORATION_MODE_OPEN_TAG,
|
||||
MULTI_AGENT_MODE_OPEN_TAG,
|
||||
PLUGINS_INSTRUCTIONS_OPEN_TAG,
|
||||
REALTIME_CONVERSATION_OPEN_TAG,
|
||||
SKILLS_INSTRUCTIONS_OPEN_TAG,
|
||||
"<personality_spec>",
|
||||
|
||||
+34
-16
@@ -25,6 +25,12 @@ use codex_plugin::AppConnectorId;
|
||||
|
||||
const LEGACY_CODEX_APPS_REGISTRATION_ID: &str = "legacy_codex_apps";
|
||||
|
||||
/// MCP configuration and capability availability derived from the same inputs.
|
||||
pub(crate) struct McpRuntimeProjection {
|
||||
pub(crate) config: McpConfig,
|
||||
pub(crate) plugins_available: bool,
|
||||
}
|
||||
|
||||
enum OrderedMcpOverlay {
|
||||
Set {
|
||||
contributor_id: &'static str,
|
||||
@@ -75,6 +81,7 @@ impl McpManager {
|
||||
pub async fn runtime_config(&self, config: &Config) -> McpConfig {
|
||||
self.runtime_config_with_context(McpServerContributionContext::global(config))
|
||||
.await
|
||||
.config
|
||||
}
|
||||
|
||||
pub(crate) async fn runtime_config_for_step(
|
||||
@@ -83,7 +90,7 @@ impl McpManager {
|
||||
thread_init: &ExtensionDataInit,
|
||||
thread_store: &ExtensionData,
|
||||
available_environment_ids: &[String],
|
||||
) -> McpConfig {
|
||||
) -> McpRuntimeProjection {
|
||||
self.runtime_config_with_context(McpServerContributionContext::for_step(
|
||||
config,
|
||||
thread_init,
|
||||
@@ -96,8 +103,9 @@ impl McpManager {
|
||||
async fn runtime_config_with_context(
|
||||
&self,
|
||||
context: McpServerContributionContext<'_, Config>,
|
||||
) -> McpConfig {
|
||||
) -> McpRuntimeProjection {
|
||||
let config = context.config();
|
||||
let mut selected_plugin_available = false;
|
||||
let mut selected_plugin_connector_sources = Vec::new();
|
||||
let mut selected_plugin_registrations = Vec::new();
|
||||
let mut overlays = Vec::new();
|
||||
@@ -129,17 +137,22 @@ impl McpManager {
|
||||
*config,
|
||||
),
|
||||
),
|
||||
McpServerContribution::SelectedPluginConnectors {
|
||||
McpServerContribution::SelectedPluginPackage {
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
connector_ids,
|
||||
} => selected_plugin_connector_sources.push(
|
||||
PluginConnectorSource::from_connector_ids(
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
connector_ids.into_iter().map(AppConnectorId),
|
||||
),
|
||||
),
|
||||
} => {
|
||||
selected_plugin_available = true;
|
||||
if !connector_ids.is_empty() {
|
||||
selected_plugin_connector_sources.push(
|
||||
PluginConnectorSource::from_connector_ids(
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
connector_ids.into_iter().map(AppConnectorId),
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
McpServerContribution::Remove { name } => {
|
||||
overlays.push(OrderedMcpOverlay::Remove {
|
||||
contributor_id: contributor.id(),
|
||||
@@ -152,12 +165,14 @@ impl McpManager {
|
||||
}
|
||||
}
|
||||
|
||||
let mut mcp_config = config
|
||||
.to_mcp_config_with_plugin_registrations(
|
||||
self.plugins_manager.as_ref(),
|
||||
selected_plugin_registrations,
|
||||
)
|
||||
let loaded_plugins = self
|
||||
.plugins_manager
|
||||
.plugins_for_config(&config.plugins_config_input())
|
||||
.await;
|
||||
let plugins_available =
|
||||
selected_plugin_available || !loaded_plugins.capability_summaries().is_empty();
|
||||
let mut mcp_config = config
|
||||
.to_mcp_config_with_loaded_plugins(&loaded_plugins, selected_plugin_registrations);
|
||||
let mut catalog = mcp_config.mcp_server_catalog.to_builder();
|
||||
if mcp_config.apps_enabled {
|
||||
catalog.register(McpServerRegistration::from_compatibility(
|
||||
@@ -211,7 +226,10 @@ impl McpManager {
|
||||
.merged_with(&ConnectorSnapshot::from_plugin_sources(
|
||||
selected_plugin_connector_sources,
|
||||
));
|
||||
mcp_config
|
||||
McpRuntimeProjection {
|
||||
config: mcp_config,
|
||||
plugins_available,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns config- and plugin-backed servers without runtime contributions.
|
||||
|
||||
@@ -6,7 +6,7 @@ use crate::plugins::PluginCapabilitySummary;
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn render_plugins_section(plugins: &[PluginCapabilitySummary]) -> Option<String> {
|
||||
AvailablePluginsInstructions::from_plugins(plugins).map(|instructions| instructions.render())
|
||||
(!plugins.is_empty()).then(|| AvailablePluginsInstructions.render())
|
||||
}
|
||||
|
||||
pub(crate) fn render_explicit_plugin_instructions(
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::mcp::McpRuntimeProjection;
|
||||
use codex_exec_server::ResolvedSelectedCapabilityRoot;
|
||||
use codex_mcp::ElicitationReviewRequest;
|
||||
use codex_mcp::ElicitationReviewer;
|
||||
@@ -92,6 +93,7 @@ impl Session {
|
||||
&available_environment_ids,
|
||||
)
|
||||
.await
|
||||
.config
|
||||
}
|
||||
|
||||
pub(crate) async fn runtime_mcp_servers(
|
||||
@@ -123,7 +125,7 @@ impl Session {
|
||||
if current.available_environment_ids() == available_environment_ids {
|
||||
return current;
|
||||
}
|
||||
let mcp_config = self
|
||||
let mcp_projection = self
|
||||
.services
|
||||
.mcp_manager
|
||||
.runtime_config_for_step(
|
||||
@@ -133,6 +135,7 @@ impl Session {
|
||||
&available_environment_ids,
|
||||
)
|
||||
.await;
|
||||
let mcp_config = &mcp_projection.config;
|
||||
let changed_environment_is_used_by_mcp = mcp_config
|
||||
.mcp_server_catalog
|
||||
.configured_servers()
|
||||
@@ -156,6 +159,7 @@ impl Session {
|
||||
// replacing the live manager and restarting its processes.
|
||||
let runtime = Arc::new(McpRuntimeSnapshot::new(
|
||||
Arc::new(current.config().clone()),
|
||||
mcp_projection.plugins_available,
|
||||
current.manager_arc(),
|
||||
current.runtime_context().clone(),
|
||||
available_environment_ids,
|
||||
@@ -165,7 +169,7 @@ impl Session {
|
||||
}
|
||||
self.refresh_mcp_servers_inner(
|
||||
turn_context,
|
||||
mcp_config,
|
||||
mcp_projection,
|
||||
environments,
|
||||
&available_environment_ids,
|
||||
Some(self.mcp_elicitation_reviewer()),
|
||||
@@ -309,12 +313,16 @@ impl Session {
|
||||
async fn refresh_mcp_servers_inner(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
mcp_config: McpConfig,
|
||||
mcp_projection: McpRuntimeProjection,
|
||||
environments: &TurnEnvironmentSnapshot,
|
||||
available_environment_ids: &[String],
|
||||
elicitation_reviewer: Option<ElicitationReviewerHandle>,
|
||||
) -> Arc<McpRuntimeSnapshot> {
|
||||
let auth = self.services.auth_manager.auth().await;
|
||||
let McpRuntimeProjection {
|
||||
config: mcp_config,
|
||||
plugins_available,
|
||||
} = mcp_projection;
|
||||
let mcp_config = Arc::new(mcp_config);
|
||||
let tool_plugin_provenance = codex_mcp::tool_plugin_provenance(&mcp_config);
|
||||
let mcp_servers = effective_mcp_servers(&mcp_config, auth.as_ref());
|
||||
@@ -376,6 +384,7 @@ impl Session {
|
||||
.set_elicitations_auto_deny(current_runtime.manager().elicitations_auto_deny());
|
||||
self.services.publish_mcp_runtime(
|
||||
mcp_config,
|
||||
plugins_available,
|
||||
mcp_runtime_context,
|
||||
available_environment_ids.to_vec(),
|
||||
refreshed_manager,
|
||||
@@ -448,7 +457,7 @@ impl Session {
|
||||
.latest_mcp_runtime()
|
||||
.available_environment_ids()
|
||||
.to_vec();
|
||||
let mut mcp_config = self
|
||||
let mut mcp_projection = self
|
||||
.services
|
||||
.mcp_manager
|
||||
.runtime_config_for_step(
|
||||
@@ -458,12 +467,13 @@ impl Session {
|
||||
&available_environment_ids,
|
||||
)
|
||||
.await;
|
||||
mcp_config.mcp_server_catalog = mcp_config
|
||||
mcp_projection.config.mcp_server_catalog = mcp_projection
|
||||
.config
|
||||
.mcp_server_catalog
|
||||
.with_materialized_servers(mcp_servers);
|
||||
self.refresh_mcp_servers_inner(
|
||||
turn_context,
|
||||
mcp_config,
|
||||
mcp_projection,
|
||||
&turn_context.environments,
|
||||
&available_environment_ids,
|
||||
elicitation_reviewer,
|
||||
@@ -515,7 +525,7 @@ impl Session {
|
||||
.latest_mcp_runtime()
|
||||
.available_environment_ids()
|
||||
.to_vec();
|
||||
let mcp_config = self
|
||||
let mcp_projection = self
|
||||
.services
|
||||
.mcp_manager
|
||||
.runtime_config_for_step(
|
||||
@@ -527,7 +537,7 @@ impl Session {
|
||||
.await;
|
||||
self.refresh_mcp_servers_inner(
|
||||
turn_context,
|
||||
mcp_config,
|
||||
mcp_projection,
|
||||
&turn_context.environments,
|
||||
&available_environment_ids,
|
||||
elicitation_reviewer,
|
||||
|
||||
@@ -5,9 +5,10 @@ use codex_mcp::McpConfig;
|
||||
use codex_mcp::McpConnectionManager;
|
||||
use codex_mcp::McpRuntimeContext;
|
||||
|
||||
/// MCP config, exact environment bindings, and manager used by one model request.
|
||||
/// MCP config, plugin availability, exact environment bindings, and manager for one request.
|
||||
pub struct McpRuntimeSnapshot {
|
||||
config: Arc<McpConfig>,
|
||||
plugins_available: bool,
|
||||
manager: Arc<McpConnectionManager>,
|
||||
runtime_context: McpRuntimeContext,
|
||||
available_environment_ids: Vec<String>,
|
||||
@@ -16,12 +17,14 @@ pub struct McpRuntimeSnapshot {
|
||||
impl McpRuntimeSnapshot {
|
||||
pub(crate) fn new(
|
||||
config: Arc<McpConfig>,
|
||||
plugins_available: bool,
|
||||
manager: Arc<McpConnectionManager>,
|
||||
runtime_context: McpRuntimeContext,
|
||||
available_environment_ids: Vec<String>,
|
||||
) -> Self {
|
||||
Self {
|
||||
config,
|
||||
plugins_available,
|
||||
manager,
|
||||
runtime_context,
|
||||
available_environment_ids,
|
||||
@@ -32,6 +35,10 @@ impl McpRuntimeSnapshot {
|
||||
self.config.as_ref()
|
||||
}
|
||||
|
||||
pub(crate) fn plugins_available(&self) -> bool {
|
||||
self.plugins_available
|
||||
}
|
||||
|
||||
pub fn manager(&self) -> &McpConnectionManager {
|
||||
self.manager.as_ref()
|
||||
}
|
||||
@@ -86,6 +93,7 @@ impl McpRuntimeSnapshot {
|
||||
);
|
||||
Arc::new(Self::new(
|
||||
Arc::new(mcp_config),
|
||||
/*plugins_available*/ false,
|
||||
Arc::new(manager),
|
||||
runtime_context,
|
||||
Vec::new(),
|
||||
|
||||
@@ -24,7 +24,6 @@ use crate::config::resolve_tool_suggest_config_from_layer_stack;
|
||||
use crate::connectors;
|
||||
use crate::context::ApprovedCommandPrefixSaved;
|
||||
use crate::context::AppsInstructions;
|
||||
use crate::context::AvailablePluginsInstructions;
|
||||
use crate::context::AvailableSkillsInstructions;
|
||||
use crate::context::CollaborationModeInstructions;
|
||||
use crate::context::ContextualUserFragment;
|
||||
@@ -3306,11 +3305,6 @@ impl Session {
|
||||
{
|
||||
contextual_user_sections.push(recommended_plugins.render());
|
||||
}
|
||||
if let Some(plugin_instructions) =
|
||||
AvailablePluginsInstructions::from_plugins(loaded_plugins.capability_summaries())
|
||||
{
|
||||
developer_sections.push(plugin_instructions.render());
|
||||
}
|
||||
let context_contributors = self.services.extensions.context_contributors().to_vec();
|
||||
for contributor in &context_contributors {
|
||||
for fragment in contributor
|
||||
|
||||
@@ -679,7 +679,7 @@ impl Session {
|
||||
let mcp_runtime_context_for_auth = mcp_runtime_context.clone();
|
||||
let auth_and_mcp_fut = async move {
|
||||
let auth = auth_manager_clone.auth().await;
|
||||
let mcp_config = mcp_manager_for_mcp
|
||||
let mcp_projection = mcp_manager_for_mcp
|
||||
.runtime_config_for_step(
|
||||
&config_for_mcp,
|
||||
mcp_thread_init_for_startup,
|
||||
@@ -687,8 +687,9 @@ impl Session {
|
||||
/*available_environment_ids*/ &[],
|
||||
)
|
||||
.await;
|
||||
let mcp_servers = codex_mcp::effective_mcp_servers(&mcp_config, auth.as_ref());
|
||||
let tool_plugin_provenance = codex_mcp::tool_plugin_provenance(&mcp_config);
|
||||
let mcp_config = &mcp_projection.config;
|
||||
let mcp_servers = codex_mcp::effective_mcp_servers(mcp_config, auth.as_ref());
|
||||
let tool_plugin_provenance = codex_mcp::tool_plugin_provenance(mcp_config);
|
||||
let auth_statuses = compute_auth_statuses(
|
||||
mcp_servers.iter(),
|
||||
config_for_mcp.mcp_oauth_credentials_store_mode,
|
||||
@@ -699,7 +700,7 @@ impl Session {
|
||||
.await;
|
||||
(
|
||||
auth,
|
||||
mcp_config,
|
||||
mcp_projection,
|
||||
mcp_servers,
|
||||
auth_statuses,
|
||||
tool_plugin_provenance,
|
||||
@@ -714,7 +715,7 @@ impl Session {
|
||||
let (
|
||||
thread_persistence_result,
|
||||
state_db_ctx,
|
||||
(auth, mcp_config, mcp_servers, auth_statuses, tool_plugin_provenance),
|
||||
(auth, mcp_projection, mcp_servers, auth_statuses, tool_plugin_provenance),
|
||||
) = tokio::join!(thread_persistence_fut, state_db_fut, auth_and_mcp_fut);
|
||||
|
||||
let mut live_thread_init =
|
||||
@@ -1208,7 +1209,10 @@ impl Session {
|
||||
sess.services.mcp_manager.codex_apps_tools_cache(),
|
||||
codex_apps_tools_cache_key(auth),
|
||||
config.prefix_mcp_tool_names(),
|
||||
mcp_config.client_elicitation_capability.clone(),
|
||||
mcp_projection
|
||||
.config
|
||||
.client_elicitation_capability
|
||||
.clone(),
|
||||
sess.services
|
||||
.supports_openai_form_elicitation
|
||||
.load(std::sync::atomic::Ordering::Relaxed),
|
||||
@@ -1224,7 +1228,8 @@ impl Session {
|
||||
.await;
|
||||
sess.services
|
||||
.install_mcp_connection_manager(
|
||||
Arc::new(mcp_config),
|
||||
Arc::new(mcp_projection.config),
|
||||
mcp_projection.plugins_available,
|
||||
mcp_runtime_context,
|
||||
/*available_environment_ids*/ Vec::new(),
|
||||
mcp_connection_manager,
|
||||
|
||||
@@ -7729,6 +7729,91 @@ async fn refresh_mcp_servers_keeps_the_previous_runtime_alive() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_availability_change_reuses_the_mcp_manager() {
|
||||
struct ReadyPluginContributor;
|
||||
|
||||
impl codex_extension_api::McpServerContributor<Config> for ReadyPluginContributor {
|
||||
fn id(&self) -> &'static str {
|
||||
"ready_plugin_test"
|
||||
}
|
||||
|
||||
fn contribute<'a>(
|
||||
&'a self,
|
||||
context: codex_extension_api::McpServerContributionContext<'a, Config>,
|
||||
) -> codex_extension_api::ExtensionFuture<'a, Vec<codex_extension_api::McpServerContribution>>
|
||||
{
|
||||
Box::pin(async move {
|
||||
let available = context
|
||||
.available_environment_ids()
|
||||
.is_some_and(|ids| ids.iter().any(|id| id == "executor"));
|
||||
available
|
||||
.then(
|
||||
|| codex_extension_api::McpServerContribution::SelectedPluginPackage {
|
||||
plugin_id: "skill-only".to_string(),
|
||||
plugin_display_name: "Skill Only".to_string(),
|
||||
connector_ids: Vec::new(),
|
||||
},
|
||||
)
|
||||
.into_iter()
|
||||
.collect()
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
let (mut session, turn_context) = make_session_and_context().await;
|
||||
let mut registry = codex_extension_api::ExtensionRegistryBuilder::new();
|
||||
registry.mcp_server_contributor(Arc::new(ReadyPluginContributor));
|
||||
let registry = Arc::new(registry.build());
|
||||
session.services.extensions = Arc::clone(®istry);
|
||||
session.services.mcp_manager = Arc::new(McpManager::new_with_extensions(
|
||||
Arc::clone(&session.services.plugins_manager),
|
||||
registry,
|
||||
));
|
||||
let session = Arc::new(session);
|
||||
session
|
||||
.refresh_mcp_servers_now(
|
||||
&turn_context,
|
||||
&turn_context.config,
|
||||
/*elicitation_reviewer*/ None,
|
||||
)
|
||||
.await;
|
||||
let old_runtime = session.services.latest_mcp_runtime();
|
||||
let old_manager = old_runtime.manager_arc();
|
||||
|
||||
let selected_root = codex_protocol::capabilities::SelectedCapabilityRoot {
|
||||
id: "skill-only".to_string(),
|
||||
location: codex_protocol::capabilities::CapabilityRootLocation::Environment {
|
||||
environment_id: "executor".to_string(),
|
||||
path: PathUri::from_host_native_path(turn_context.config.cwd.as_path())
|
||||
.expect("selected capability root URI"),
|
||||
},
|
||||
};
|
||||
let environment = Arc::clone(
|
||||
&turn_context
|
||||
.environments
|
||||
.primary()
|
||||
.expect("ready test environment")
|
||||
.environment,
|
||||
);
|
||||
let captured_environments = HashMap::from([("executor".to_string(), Some(environment))]);
|
||||
let resolved_roots = session
|
||||
.services
|
||||
.turn_environments
|
||||
.environment_manager()
|
||||
.resolve_selected_capability_roots(&[selected_root], &captured_environments)
|
||||
.await;
|
||||
|
||||
let new_runtime = session
|
||||
.mcp_runtime_for_step(&turn_context, &turn_context.environments, &resolved_roots)
|
||||
.await;
|
||||
|
||||
assert!(!old_runtime.plugins_available());
|
||||
assert!(new_runtime.plugins_available());
|
||||
assert!(!Arc::ptr_eq(&old_runtime, &new_runtime));
|
||||
assert!(Arc::ptr_eq(&old_manager, &new_runtime.manager_arc()));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn built_tools_uses_the_step_mcp_runtime() -> anyhow::Result<()> {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
|
||||
@@ -2,6 +2,7 @@ use super::session::Session;
|
||||
use super::step_context::StepContext;
|
||||
use crate::context::world_state::AgentsMdState;
|
||||
use crate::context::world_state::EnvironmentsState;
|
||||
use crate::context::world_state::PluginsInstructionsState;
|
||||
use crate::context::world_state::WorldState;
|
||||
use codex_extension_api::WorldStateContributionInput;
|
||||
|
||||
@@ -35,6 +36,9 @@ impl Session {
|
||||
.with_subagents(environment_subagents),
|
||||
);
|
||||
}
|
||||
world_state.add_section(PluginsInstructionsState::new(
|
||||
step_context.mcp.plugins_available(),
|
||||
));
|
||||
let environments = step_context.environments.to_selections();
|
||||
let ready_selected_capability_roots = step_context
|
||||
.selected_capability_roots
|
||||
|
||||
@@ -107,18 +107,25 @@ impl SessionServices {
|
||||
pub(crate) async fn install_mcp_connection_manager(
|
||||
&self,
|
||||
config: Arc<McpConfig>,
|
||||
plugins_available: bool,
|
||||
runtime_context: McpRuntimeContext,
|
||||
available_environment_ids: Vec<String>,
|
||||
manager: McpConnectionManager,
|
||||
) -> Result<()> {
|
||||
let runtime =
|
||||
self.publish_mcp_runtime(config, runtime_context, available_environment_ids, manager);
|
||||
let runtime = self.publish_mcp_runtime(
|
||||
config,
|
||||
plugins_available,
|
||||
runtime_context,
|
||||
available_environment_ids,
|
||||
manager,
|
||||
);
|
||||
runtime.manager().validate_required_servers().await
|
||||
}
|
||||
|
||||
pub(crate) fn publish_mcp_runtime(
|
||||
&self,
|
||||
config: Arc<McpConfig>,
|
||||
plugins_available: bool,
|
||||
runtime_context: McpRuntimeContext,
|
||||
available_environment_ids: Vec<String>,
|
||||
manager: McpConnectionManager,
|
||||
@@ -129,6 +136,7 @@ impl SessionServices {
|
||||
self.mcp_connection_manager.store(Arc::clone(&manager));
|
||||
let runtime = Arc::new(McpRuntimeSnapshot::new(
|
||||
config,
|
||||
plugins_available,
|
||||
manager,
|
||||
runtime_context,
|
||||
available_environment_ids,
|
||||
|
||||
@@ -699,11 +699,11 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
.collect::<std::collections::BTreeMap<_, _>>()
|
||||
};
|
||||
assert_eq!(
|
||||
selected_servers(&first_resolved),
|
||||
selected_servers(&first_resolved.config),
|
||||
std::collections::BTreeMap::from([("selected-a".to_string(), "env-a".to_string())])
|
||||
);
|
||||
assert_eq!(
|
||||
selected_servers(&second_resolved),
|
||||
selected_servers(&second_resolved.config),
|
||||
std::collections::BTreeMap::from([("selected-b".to_string(), "env-b".to_string())])
|
||||
);
|
||||
}
|
||||
|
||||
@@ -93,8 +93,8 @@ pub enum McpServerContribution {
|
||||
selection_order: usize,
|
||||
config: Box<McpServerConfig>,
|
||||
},
|
||||
/// Adds connector IDs declared by a plugin selected for this thread.
|
||||
SelectedPluginConnectors {
|
||||
/// Records a plugin selected for this thread and any connector IDs it declares.
|
||||
SelectedPluginPackage {
|
||||
plugin_id: String,
|
||||
plugin_display_name: String,
|
||||
connector_ids: Vec<String>,
|
||||
|
||||
@@ -186,13 +186,12 @@ impl McpServerContributor<Config> for SelectedExecutorPluginMcpContributor {
|
||||
config: Box::new(config),
|
||||
}
|
||||
}));
|
||||
if !plugin.connector_ids.is_empty() {
|
||||
contributions.push(McpServerContribution::SelectedPluginConnectors {
|
||||
plugin_id: plugin.plugin_id,
|
||||
plugin_display_name: plugin.plugin_display_name,
|
||||
connector_ids: plugin.connector_ids,
|
||||
});
|
||||
}
|
||||
// Keep the package visible even when it contributes only skills.
|
||||
contributions.push(McpServerContribution::SelectedPluginPackage {
|
||||
plugin_id: plugin.plugin_id,
|
||||
plugin_display_name: plugin.plugin_display_name,
|
||||
connector_ids: plugin.connector_ids,
|
||||
});
|
||||
}
|
||||
|
||||
contributions
|
||||
|
||||
@@ -25,6 +25,13 @@ struct ContributionSummary {
|
||||
enabled: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
struct PackageSummary {
|
||||
plugin_id: String,
|
||||
plugin_display_name: String,
|
||||
connector_ids: Vec<String>,
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selected_plugin_servers_use_managed_requirements_for_the_selected_root_id() -> TestResult {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
@@ -92,10 +99,87 @@ command = "expected-command"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selected_plugin_package_is_contributed_without_servers_or_connectors() -> TestResult {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
let plugin_root = tempfile::tempdir()?;
|
||||
std::fs::create_dir_all(plugin_root.path().join(".codex-plugin"))?;
|
||||
std::fs::create_dir_all(plugin_root.path().join("skills/deploy"))?;
|
||||
std::fs::write(
|
||||
plugin_root.path().join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"skill-only","interface":{"displayName":"Skill Only"}}"#,
|
||||
)?;
|
||||
std::fs::write(
|
||||
plugin_root.path().join("skills/deploy/SKILL.md"),
|
||||
"---\nname: deploy\ndescription: Deploy the project.\n---\n",
|
||||
)?;
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
let contributions = raw_selected_plugin_contributions(&config, plugin_root.path()).await?;
|
||||
let package = contributions.into_iter().find_map(|contribution| {
|
||||
let McpServerContribution::SelectedPluginPackage {
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
connector_ids,
|
||||
} = contribution
|
||||
else {
|
||||
return None;
|
||||
};
|
||||
Some(PackageSummary {
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
connector_ids,
|
||||
})
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
package,
|
||||
Some(PackageSummary {
|
||||
plugin_id: "selected-root".to_string(),
|
||||
plugin_display_name: "Skill Only".to_string(),
|
||||
connector_ids: Vec::new(),
|
||||
})
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn selected_plugin_contributions(
|
||||
config: &Config,
|
||||
plugin_root: &std::path::Path,
|
||||
) -> Result<Vec<ContributionSummary>, Box<dyn std::error::Error>> {
|
||||
Ok(raw_selected_plugin_contributions(config, plugin_root)
|
||||
.await?
|
||||
.into_iter()
|
||||
.filter_map(|contribution| match contribution {
|
||||
McpServerContribution::SelectedPlugin {
|
||||
name,
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
selection_order,
|
||||
config,
|
||||
} => Some(ContributionSummary {
|
||||
name,
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
selection_order,
|
||||
enabled: config.enabled,
|
||||
}),
|
||||
McpServerContribution::SelectedPluginPackage { .. } => None,
|
||||
McpServerContribution::Set { .. } | McpServerContribution::Remove { .. } => {
|
||||
panic!("expected selected plugin contribution")
|
||||
}
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
|
||||
async fn raw_selected_plugin_contributions(
|
||||
config: &Config,
|
||||
plugin_root: &std::path::Path,
|
||||
) -> Result<Vec<McpServerContribution>, Box<dyn std::error::Error>> {
|
||||
let mut builder = ExtensionRegistryBuilder::new();
|
||||
codex_mcp_extension::install_executor_plugins(
|
||||
&mut builder,
|
||||
@@ -120,27 +204,5 @@ async fn selected_plugin_contributions(
|
||||
&thread_store,
|
||||
&available_environment_ids,
|
||||
))
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|contribution| match contribution {
|
||||
McpServerContribution::SelectedPlugin {
|
||||
name,
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
selection_order,
|
||||
config,
|
||||
} => ContributionSummary {
|
||||
name,
|
||||
plugin_id,
|
||||
plugin_display_name,
|
||||
selection_order,
|
||||
enabled: config.enabled,
|
||||
},
|
||||
McpServerContribution::Set { .. }
|
||||
| McpServerContribution::SelectedPluginConnectors { .. }
|
||||
| McpServerContribution::Remove { .. } => {
|
||||
panic!("expected selected plugin contribution")
|
||||
}
|
||||
})
|
||||
.collect())
|
||||
.await)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user