openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-17 17:08:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

Backport Agent Plugin runtime boundaries (#37027, #36967)

Browse Source 
Adapt the Agent Plugin runtime boundary enforcement to the alpha.6.4 APIs and skip symlinks while installing plugins.

(cherry picked from commit 56b82e676c)

(cherry picked from commit 720c9d68e1)
This commit is contained in:
jacobzhou-oai
2026-08-05 17:37:06 +00:00
committed by Jacob Zhou
parent 3044612e13
commit 8ac86822a1
63 changed files with 1839 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/Cargo.lock
Generated
+1
View File
@@ -2920,6 +2920,7 @@ dependencies = [
"codex-utils-absolute-path",
"codex-utils-path-uri",
"codex-utils-plugins",
"codex-utils-string",
"dirs",
"dunce",
"futures",
codex-rs/analytics/src/analytics_client_tests.rs
+1
View File
@@ -5375,6 +5375,7 @@ fn sample_plugin_metadata() -> PluginTelemetryMetadata {
capability_summary: Some(PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: None,
description: None,
has_skills: true,
mcp_server_names: vec!["mcp-1".to_string(), "mcp-2".to_string()],
codex-rs/app-server/src/request_processors.rs
+1
View File
@@ -458,6 +458,7 @@ use codex_protocol::protocol::W3cTraceContext;
use codex_protocol::protocol::strip_user_message_prefix;
use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS;
use codex_protocol::user_input::UserInput as CoreInputItem;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::perform_oauth_login_return_url;
use codex_rollout::is_persisted_rollout_item;
use codex_rollout::state_db::StateDbHandle;
codex-rs/app-server/src/request_processors/mcp_processor.rs
+7
View File
@@ -149,6 +149,11 @@ impl McpRequestProcessor {
"No MCP server named '{name}' found."
)));
};
let redirect_mode = if server.is_agent_plugin() {
StreamableHttpRedirectMode::AgentPluginV1
} else {
StreamableHttpRedirectMode::Legacy
};
let server = server.config();
let (url, http_headers, env_http_headers) = match &server.transport {
@@ -176,6 +181,7 @@ impl McpRequestProcessor {
&server.transport,
Arc::clone(&http_client),
codex_rmcp_client::OAuthDiscoveryTimeout::Requested,
redirect_mode,
)
.await
} else {
@@ -199,6 +205,7 @@ impl McpRequestProcessor {
mcp_config.mcp_oauth_callback_port,
mcp_config.mcp_oauth_callback_url.as_deref(),
http_client,
redirect_mode,
)
.await
.map_err(|err| internal_error(format!("failed to login to MCP server '{name}': {err}")))?;
codex-rs/app-server/src/request_processors/plugins.rs
+30 -4
View File
@@ -12,6 +12,7 @@ use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME;
use codex_core_plugins::PluginListBackgroundTaskOptions;
use codex_core_plugins::is_openai_curated_marketplace_name;
use codex_core_plugins::loader::load_configured_plugin_mcp_servers;
use codex_core_plugins::manifest::is_agent_plugin_manifest;
use codex_core_plugins::remote::REMOTE_CREATED_BY_ME_MARKETPLACE_NAME;
use codex_core_plugins::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
use codex_core_plugins::remote::REMOTE_WORKSPACE_MARKETPLACE_NAME;
@@ -32,10 +33,19 @@ use codex_plugin::PluginId;
use codex_plugin::PluginTelemetryMetadata;
use codex_protocol::auth::AuthMode as DomainAuthMode;
use codex_rmcp_client::OAuthDiscoveryTimeout;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::perform_oauth_login_silent;
mod search;
fn plugin_redirect_mode(plugin_root: &Path) -> StreamableHttpRedirectMode {
if is_agent_plugin_manifest(plugin_root) {
StreamableHttpRedirectMode::AgentPluginV1
} else {
StreamableHttpRedirectMode::Legacy
}
}
#[derive(Clone)]
pub(crate) struct PluginRequestProcessor {
auth_manager: Arc<AuthManager>,
@@ -1540,8 +1550,14 @@ impl PluginRequestProcessor {
)
.await;
if !plugin_mcp_servers.is_empty() {
self.start_plugin_mcp_oauth_logins(&config, &result.plugin_id, plugin_mcp_servers)
.await;
let redirect_mode = plugin_redirect_mode(result.installed_path.as_path());
self.start_plugin_mcp_oauth_logins(
&config,
&result.plugin_id,
plugin_mcp_servers,
redirect_mode,
)
.await;
}
let plugin_app_declarations = load_plugin_apps(result.installed_path.as_path()).await;
@@ -1717,8 +1733,14 @@ impl PluginRequestProcessor {
)
.await;
if !plugin_mcp_servers.is_empty() {
self.start_plugin_mcp_oauth_logins(&config, &result.plugin_id, plugin_mcp_servers)
.await;
let redirect_mode = plugin_redirect_mode(result.installed_path.as_path());
self.start_plugin_mcp_oauth_logins(
&config,
&result.plugin_id,
plugin_mcp_servers,
redirect_mode,
)
.await;
}
let is_chatgpt_auth = auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth);
@@ -1869,6 +1891,7 @@ impl PluginRequestProcessor {
config: &Config,
plugin_id: &PluginId,
mut plugin_mcp_servers: HashMap<String, McpServerConfig>,
redirect_mode: StreamableHttpRedirectMode,
) {
let plugin_id = plugin_id.as_key();
config.apply_plugin_mcp_server_requirements(&plugin_id, &mut plugin_mcp_servers);
@@ -1900,6 +1923,7 @@ impl PluginRequestProcessor {
&server.transport,
Arc::clone(&http_client),
OAuthDiscoveryTimeout::LOCAL,
redirect_mode,
)
.await;
let oauth_config = match login_support {
@@ -1944,6 +1968,7 @@ impl PluginRequestProcessor {
callback_port,
callback_url.as_deref(),
Arc::clone(&http_client),
redirect_mode,
)
.await;
@@ -1962,6 +1987,7 @@ impl PluginRequestProcessor {
callback_port,
callback_url.as_deref(),
http_client,
redirect_mode,
)
.await
}
codex-rs/app-server/tests/suite/v2/plugin_install.rs
+1 -1
View File
@@ -2927,7 +2927,7 @@ fn remote_plugin_bundle_tar_gz_bytes_with_entries(
app_manifest: Option<&str>,
mcp_config: Option<&str>,
) -> Result<Vec<u8>> {
let skill = "# Plan Work\n\nTrack work in Linear.\n";
let skill = "---\nname: plan-work\ndescription: Track work in Linear.\n---\n\n# Plan Work\n";
let encoder = GzEncoder::new(Vec::new(), Compression::default());
let mut tar = tar::Builder::new(encoder);
let mut entries = vec![
codex-rs/cli/src/mcp_cmd.rs
+3
View File
@@ -32,6 +32,7 @@ use codex_mcp::resolve_oauth_scopes;
use codex_mcp::should_retry_without_scopes;
use codex_protocol::protocol::McpAuthStatus;
use codex_rmcp_client::OAuthDiscoveryTimeout;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::delete_oauth_tokens;
use codex_rmcp_client::perform_oauth_login;
use codex_utils_cli::CliConfigOverrides;
@@ -406,6 +407,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
&transport,
Arc::clone(&http_client),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await;
match login_support {
@@ -511,6 +513,7 @@ async fn run_login(config: &Config, login_args: LoginArgs) -> Result<()> {
&server.transport,
Arc::clone(&http_client),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await
} else {
codex-rs/codex-mcp/src/catalog.rs
+23
View File
@@ -10,6 +10,7 @@ use codex_config::McpServerConfig;
pub struct McpPluginAttribution {
plugin_id: String,
display_name: String,
agent_plugin: bool,
}
impl McpPluginAttribution {
@@ -17,6 +18,15 @@ impl McpPluginAttribution {
Self {
plugin_id,
display_name,
agent_plugin: false,
}
}
pub fn agent_plugin(plugin_id: String, display_name: String) -> Self {
Self {
plugin_id,
display_name,
agent_plugin: true,
}
}
@@ -27,6 +37,10 @@ impl McpPluginAttribution {
pub fn display_name(&self) -> &str {
&self.display_name
}
pub fn is_agent_plugin(&self) -> bool {
self.agent_plugin
}
}
/// The component that declared an MCP server registration.
@@ -46,6 +60,15 @@ pub enum McpServerSource {
}
impl McpServerSource {
pub fn is_agent_plugin(&self) -> bool {
match self {
Self::Plugin(attribution) | Self::SelectedPlugin(attribution) => {
attribution.is_agent_plugin()
}
Self::Config | Self::Compatibility { .. } | Self::Extension { .. } => false,
}
}
fn disabled_registration_is_name_veto(&self) -> bool {
// A selected package's policy applies to its registration, not to a higher runtime source
// that happens to use the same logical server name.
codex-rs/codex-mcp/src/mcp/auth.rs
+13 -1
View File
@@ -12,6 +12,7 @@ use codex_login::CodexAuth;
use codex_rmcp_client::McpAuthState;
use codex_rmcp_client::OAuthDiscoveryTimeout;
use codex_rmcp_client::OAuthProviderError;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::determine_streamable_http_auth_status;
use codex_rmcp_client::discover_streamable_http_oauth;
use futures::FutureExt;
@@ -61,6 +62,7 @@ pub async fn oauth_login_support(
transport: &McpServerTransportConfig,
http_client: Arc<dyn HttpClient>,
discovery_timeout: OAuthDiscoveryTimeout,
redirect_mode: StreamableHttpRedirectMode,
) -> McpOAuthLoginSupport {
let Some(mut config) = oauth_login_candidate(transport) else {
return McpOAuthLoginSupport::Unsupported;
@@ -71,6 +73,7 @@ pub async fn oauth_login_support(
config.env_http_headers.clone(),
http_client,
discovery_timeout,
redirect_mode,
)
.await
{
@@ -108,8 +111,9 @@ pub async fn discover_supported_scopes(
transport: &McpServerTransportConfig,
http_client: Arc<dyn HttpClient>,
discovery_timeout: OAuthDiscoveryTimeout,
redirect_mode: StreamableHttpRedirectMode,
) -> Option<Vec<String>> {
match oauth_login_support(transport, http_client, discovery_timeout).await {
match oauth_login_support(transport, http_client, discovery_timeout, redirect_mode).await {
McpOAuthLoginSupport::Supported(config) => config.discovered_scopes,
McpOAuthLoginSupport::Unsupported | McpOAuthLoginSupport::Unknown(_) => None,
}
@@ -166,6 +170,11 @@ where
{
let futures = servers.into_iter().map(|(name, server)| {
let name = name.clone();
let redirect_mode = if server.is_agent_plugin() {
StreamableHttpRedirectMode::AgentPluginV1
} else {
StreamableHttpRedirectMode::Legacy
};
let config = server.config().clone();
let runtime_context = runtime_context.clone();
let has_runtime_auth = matches!(&config.auth, McpServerAuth::ChatGpt)
@@ -185,6 +194,7 @@ where
keyring_backend_kind,
has_runtime_auth,
&runtime_context,
redirect_mode,
)
.await
{
@@ -212,6 +222,7 @@ async fn compute_auth_status(
keyring_backend_kind: AuthKeyringBackendKind,
has_runtime_auth: bool,
runtime_context: &McpRuntimeContext,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<McpAuthState> {
if !config.enabled {
return Ok(McpAuthState::Unsupported);
@@ -256,6 +267,7 @@ async fn compute_auth_status(
keyring_backend_kind,
http_client,
discovery_timeout,
redirect_mode,
)
.boxed()
.await
codex-rs/codex-mcp/src/mcp/mod.rs
+8 -1
View File
@@ -300,7 +300,14 @@ pub fn effective_mcp_servers_from_configured(
}
McpServerAuth::OAuth => {}
}
(name, EffectiveMcpServer::configured(server))
let agent_plugin = config
.mcp_server_catalog
.server(&name)
.is_some_and(|server| server.source().is_agent_plugin());
(
name,
EffectiveMcpServer::configured(server).with_agent_plugin(agent_plugin),
)
})
.collect::<HashMap<_, _>>();
if !host_owned_codex_apps_enabled(config, auth) {
codex-rs/codex-mcp/src/mcp/mod_tests.rs
+3
View File
@@ -148,6 +148,7 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() {
PluginCapabilitySummary {
config_name: "alpha@test".to_string(),
display_name: "alpha-plugin".to_string(),
plugin_namespace: None,
app_connector_ids: vec![AppConnectorId("connector_example".to_string())],
mcp_server_names: vec!["alpha".to_string()],
..PluginCapabilitySummary::default()
@@ -155,6 +156,7 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() {
PluginCapabilitySummary {
config_name: "beta@test".to_string(),
display_name: "beta-plugin".to_string(),
plugin_namespace: None,
app_connector_ids: vec![
AppConnectorId("connector_example".to_string()),
AppConnectorId("connector_gmail".to_string()),
@@ -219,6 +221,7 @@ fn selected_mcp_attribution_does_not_join_an_unrelated_local_summary() {
PluginCapabilitySummary {
config_name: "shared-plugin-id".to_string(),
display_name: "Local GitHub".to_string(),
plugin_namespace: None,
mcp_server_names: vec!["github".to_string()],
..PluginCapabilitySummary::default()
},
codex-rs/codex-mcp/src/rmcp_client.rs
+8 -1
View File
@@ -58,6 +58,7 @@ use codex_rmcp_client::LocalStdioServerLauncher;
use codex_rmcp_client::McpProtocolMode;
use codex_rmcp_client::RmcpClient;
use codex_rmcp_client::StdioServerLauncher;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::ToolWithConnectorId;
use codex_rmcp_client::is_authentication_required_error;
use futures::future::BoxFuture;
@@ -1082,7 +1083,12 @@ async fn make_rmcp_client(
Ok(token) => token,
Err(error) => return Err(error.into()),
};
RmcpClient::new_streamable_http_client_with_protocol_mode(
let redirect_mode = if server.is_agent_plugin() {
StreamableHttpRedirectMode::AgentPluginV1
} else {
StreamableHttpRedirectMode::Legacy
};
RmcpClient::new_streamable_http_client_with_protocol_mode_and_redirect_mode(
oauth_credential_name.as_ref(),
&url,
resolved_bearer_token,
@@ -1093,6 +1099,7 @@ async fn make_rmcp_client(
http_client,
runtime_auth_provider,
protocol_mode,
redirect_mode,
)
.await
.map_err(StartupOutcomeError::from)
codex-rs/codex-mcp/src/server.rs
+17 -1
View File
@@ -23,11 +23,20 @@ use tracing::warn;
#[derive(Debug, Clone)]
pub struct EffectiveMcpServer {
config: McpServerConfig,
agent_plugin: bool,
}
impl EffectiveMcpServer {
pub fn configured(config: McpServerConfig) -> Self {
Self { config }
Self {
config,
agent_plugin: false,
}
}
pub fn with_agent_plugin(mut self, agent_plugin: bool) -> Self {
self.agent_plugin = agent_plugin;
self
}
pub fn config(&self) -> &McpServerConfig {
@@ -41,6 +50,10 @@ impl EffectiveMcpServer {
pub fn required(&self) -> bool {
self.config.required
}
pub fn is_agent_plugin(&self) -> bool {
self.agent_plugin
}
}
pub(crate) fn has_explicit_http_authorization(config: &McpServerConfig) -> bool {
@@ -91,6 +104,7 @@ pub(crate) struct McpServerConnectionIdentity {
codex_apps_cache_identity: Option<(PathBuf, ConnectorRuntimeContextKey)>,
client_elicitation_capability: ElicitationCapability,
supports_openai_form_elicitation: bool,
agent_plugin: bool,
}
impl McpServerConnectionIdentity {
@@ -164,6 +178,7 @@ impl McpServerConnectionIdentity {
codex_apps_cache_identity,
client_elicitation_capability,
supports_openai_form_elicitation,
agent_plugin: server.is_agent_plugin(),
}
}
@@ -192,6 +207,7 @@ impl McpServerConnectionIdentity {
&& self.codex_apps_cache_identity == other.codex_apps_cache_identity
&& self.client_elicitation_capability == other.client_elicitation_capability
&& self.supports_openai_form_elicitation == other.supports_openai_form_elicitation
&& self.agent_plugin == other.agent_plugin
}
pub(crate) fn oauth_credentials(&self) -> Result<&Option<StoredOAuthTokens>, &String> {
codex-rs/core-plugins/src/loader.rs
+190 -35
View File
@@ -3,10 +3,11 @@ use crate::app_mcp_routing::apps_route_available;
use crate::command_migration::migrated_command_skills_root;
use crate::is_openai_curated_marketplace_name;
use crate::manifest::PluginManifest;
use crate::manifest::PluginManifestFormat;
use crate::manifest::PluginManifestHooks;
use crate::manifest::PluginManifestMcpServers;
use crate::manifest::PluginManifestPaths;
use crate::manifest::load_plugin_manifest;
use crate::manifest::load_plugin_manifest_with_format;
use crate::marketplace::MarketplacePluginSource;
use crate::marketplace::find_marketplace_plugin;
use crate::marketplace::list_marketplaces_with_home;
@@ -23,6 +24,7 @@ use crate::store::plugin_version_for_source_with_fallback_manifest;
use codex_config::ConfigLayerStack;
use codex_config::HooksFile;
use codex_config::types::McpServerConfig;
use codex_config::types::McpServerTransportConfig;
use codex_config::types::PluginConfig;
use codex_config::types::PluginMcpServerConfig;
use codex_connectors::parse_plugin_app_config;
@@ -33,6 +35,7 @@ use codex_core_skills::config_rules::skill_config_rules_from_stack;
use codex_core_skills::loader::SkillRoot;
use codex_core_skills::loader::load_skills_from_roots;
use codex_exec_server::LOCAL_FS;
use codex_mcp::parse_agent_plugin_mcp_config;
use codex_mcp::parse_plugin_mcp_config;
use codex_plugin::AppDeclaration;
use codex_plugin::LoadedPlugin;
@@ -831,6 +834,7 @@ async fn load_plugin(
root,
enabled: plugin.enabled,
skill_roots: Vec::new(),
skill_discovery_mode: SkillDiscoveryMode::Recursive,
disabled_skill_paths: HashSet::new(),
has_enabled_skills: false,
mcp_servers: HashMap::new(),
@@ -873,12 +877,19 @@ async fn load_plugin(
return loaded_plugin;
}
let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else {
let Some(loaded_manifest) = load_plugin_manifest_with_format(plugin_root.as_path()) else {
loaded_plugin.error = Some("missing or invalid plugin.json".to_string());
return loaded_plugin;
};
loaded_plugin.skill_discovery_mode = match loaded_manifest.format {
PluginManifestFormat::Legacy => SkillDiscoveryMode::Recursive,
PluginManifestFormat::AgentPlugin => SkillDiscoveryMode::DirectChildren,
};
let manifest = loaded_manifest.manifest;
let manifest_paths = &manifest.paths;
let plugin_data_root = store.plugin_data_root(&loaded_plugin_id);
let mcp_plugin_data_root = store.mcp_data_root(&loaded_plugin_id, loaded_manifest.format);
loaded_plugin.plugin_namespace = Some(manifest.name.clone());
match scope {
PluginLoadScope::AllCapabilities {
@@ -890,40 +901,51 @@ async fn load_plugin(
} => {
loaded_plugin.manifest_name = Some(manifest.display_name().to_string());
loaded_plugin.manifest_description = manifest.description.clone();
loaded_plugin.skill_roots = plugin_skill_roots(&plugin_root, manifest_paths);
loaded_plugin.skill_roots =
plugin_skill_roots(&plugin_root, manifest_paths, loaded_manifest.format);
let plugin_identity = PluginIdentity {
plugin_id: loaded_plugin_id.as_key(),
remote_plugin_id: loaded_plugin.remote_plugin_id.clone(),
};
let resolved_skills = load_plugin_skills_with_identity(
let resolved_skills = load_plugin_skill_inventory(
&plugin_root,
&plugin_identity,
&manifest,
loaded_manifest.format,
*restriction_product,
skill_config_rules,
*plugin_skill_snapshots,
Arc::clone(root_scan_slots),
)
.await;
.await
.resolve(skill_config_rules);
let has_enabled_skills = resolved_skills.has_enabled_skills();
loaded_plugin.disabled_skill_paths = resolved_skills.disabled_skill_paths;
loaded_plugin.has_enabled_skills = has_enabled_skills;
loaded_plugin.mcp_servers = load_plugin_mcp_servers_from_manifest(
loaded_plugin.mcp_servers = load_plugin_mcp_servers_from_manifest_with_format(
plugin_root.as_path(),
manifest_paths,
Some(&plugin.mcp_servers),
Some(mcp_plugin_data_root.as_path()),
loaded_manifest.format,
)
.await;
loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await;
if loaded_manifest.format == PluginManifestFormat::Legacy {
loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await;
}
}
PluginLoadScope::HooksOnly => {}
}
let (hook_sources, hook_load_warnings) = load_plugin_hooks(
&plugin_root,
&loaded_plugin_id,
&store.plugin_data_root(&loaded_plugin_id),
manifest_paths,
);
let (hook_sources, hook_load_warnings) =
if loaded_manifest.format == PluginManifestFormat::AgentPlugin {
(Vec::new(), Vec::new())
} else {
load_plugin_hooks(
&plugin_root,
&loaded_plugin_id,
&plugin_data_root,
manifest_paths,
)
};
loaded_plugin.hook_sources = hook_sources;
loaded_plugin.hook_load_warnings = hook_load_warnings;
loaded_plugin
@@ -961,7 +983,7 @@ impl PluginSkillInventory {
)
}
fn resolve(self, skill_config_rules: &SkillConfigRules) -> ResolvedPluginSkills {
pub(crate) fn resolve(self, skill_config_rules: &SkillConfigRules) -> ResolvedPluginSkills {
let disabled_skill_paths = resolve_disabled_skill_paths(&self.skills, skill_config_rules);
ResolvedPluginSkills {
skills: self.skills,
@@ -1031,6 +1053,7 @@ pub(crate) async fn load_plugin_skills_with_identity(
plugin_root,
plugin_identity,
manifest,
PluginManifestFormat::Legacy,
restriction_product,
plugin_skill_snapshots,
root_scan_slots,
@@ -1043,11 +1066,16 @@ pub(crate) async fn load_plugin_skill_inventory(
plugin_root: &AbsolutePathBuf,
plugin_identity: &PluginIdentity,
manifest: &PluginManifest,
manifest_format: PluginManifestFormat,
restriction_product: Option<Product>,
plugin_skill_snapshots: Option<&PluginSkillSnapshots>,
root_scan_slots: Arc<Semaphore>,
) -> PluginSkillInventory {
let roots = plugin_skill_roots(plugin_root, &manifest.paths)
let discovery_mode = match manifest_format {
PluginManifestFormat::Legacy => SkillDiscoveryMode::Recursive,
PluginManifestFormat::AgentPlugin => SkillDiscoveryMode::DirectChildren,
};
let roots = plugin_skill_roots(plugin_root, &manifest.paths, manifest_format)
.into_iter()
.map(|path| SkillRoot {
path,
@@ -1056,7 +1084,7 @@ pub(crate) async fn load_plugin_skill_inventory(
plugin_identity: Some(plugin_identity.clone()),
plugin_namespace: Some(manifest.name.clone()),
plugin_root: Some(plugin_root.clone()),
discovery_mode: SkillDiscoveryMode::Recursive,
discovery_mode,
})
.collect::<Vec<_>>();
let outcome = load_skills_from_roots(roots, plugin_skill_snapshots, root_scan_slots).await;
@@ -1098,15 +1126,18 @@ pub(crate) async fn load_plugin_skill_inventory(
fn plugin_skill_roots(
plugin_root: &AbsolutePathBuf,
manifest_paths: &PluginManifestPaths,
manifest_format: PluginManifestFormat,
) -> Vec<AbsolutePathBuf> {
let mut paths = if manifest_paths.skills.is_empty() {
default_skill_roots(plugin_root)
} else {
manifest_paths.skills.clone()
};
let migrated_command_skills = migrated_command_skills_root(plugin_root);
if migrated_command_skills.is_dir() {
paths.push(migrated_command_skills);
if manifest_format == PluginManifestFormat::Legacy {
let migrated_command_skills = migrated_command_skills_root(plugin_root);
if migrated_command_skills.is_dir() {
paths.push(migrated_command_skills);
}
}
paths.sort_unstable();
paths.dedup();
@@ -1146,8 +1177,11 @@ fn default_mcp_config_paths(plugin_root: &Path) -> Vec<AbsolutePathBuf> {
}
pub async fn load_plugin_apps(plugin_root: &Path) -> Vec<AppDeclaration> {
if let Some(manifest) = load_plugin_manifest(plugin_root) {
return load_plugin_apps_from_manifest(plugin_root, &manifest.paths).await;
if let Some(loaded_manifest) = load_plugin_manifest_with_format(plugin_root) {
if loaded_manifest.format == PluginManifestFormat::AgentPlugin {
return Vec::new();
}
return load_plugin_apps_from_manifest(plugin_root, &loaded_manifest.manifest.paths).await;
}
load_apps_from_paths(plugin_root, default_app_config_paths(plugin_root)).await
}
@@ -1345,14 +1379,40 @@ pub async fn plugin_capability_summary_from_root(
plugin_id: &PluginId,
plugin_root: &AbsolutePathBuf,
) -> Option<PluginCapabilitySummary> {
let manifest = load_plugin_manifest(plugin_root.as_path())?;
let loaded_manifest = load_plugin_manifest_with_format(plugin_root.as_path())?;
let manifest_format = loaded_manifest.format;
let manifest = loaded_manifest.manifest;
let plugin_identity = PluginIdentity {
plugin_id: plugin_id.as_key(),
remote_plugin_id: None,
};
let manifest_paths = &manifest.paths;
let has_skills = !plugin_skill_roots(plugin_root, manifest_paths).is_empty();
let mut mcp_server_names = load_plugin_mcp_servers_from_manifest(
let has_skills = match manifest_format {
PluginManifestFormat::Legacy => {
!plugin_skill_roots(plugin_root, manifest_paths, manifest_format).is_empty()
}
PluginManifestFormat::AgentPlugin => {
!load_plugin_skill_inventory(
plugin_root,
&plugin_identity,
&manifest,
manifest_format,
/*restriction_product*/ None,
/*plugin_skill_snapshots*/ None,
Arc::new(Semaphore::new(1)),
)
.await
.skills
.is_empty()
}
};
let mut mcp_server_names = load_plugin_mcp_servers_from_manifest_with_format(
plugin_root.as_path(),
manifest_paths,
/*plugin_policy*/ None,
/*plugin_data_root*/ None,
manifest_format,
)
.await
.into_keys()
@@ -1360,16 +1420,17 @@ pub async fn plugin_capability_summary_from_root(
mcp_server_names.sort_unstable();
mcp_server_names.dedup();
let app_declarations = load_apps_from_paths(
plugin_root.as_path(),
plugin_app_config_paths(plugin_root.as_path(), manifest_paths),
)
.await;
let app_declarations = if manifest_format == PluginManifestFormat::AgentPlugin {
Vec::new()
} else {
load_plugin_apps_from_manifest(plugin_root.as_path(), manifest_paths).await
};
let app_connector_ids = app_connector_ids_from_declarations(&app_declarations);
Some(PluginCapabilitySummary {
config_name: plugin_id.as_key(),
display_name: plugin_id.plugin_name.clone(),
plugin_namespace: Some(manifest.name.clone()),
description: None,
has_skills,
mcp_server_names,
@@ -1426,17 +1487,26 @@ async fn load_declared_plugin_mcp_servers(
plugin_root: &Path,
plugin_policy: Option<&HashMap<String, PluginMcpServerConfig>>,
) -> HashMap<String, McpServerConfig> {
let Some(manifest) = load_plugin_manifest(plugin_root) else {
let Some(loaded_manifest) = load_plugin_manifest_with_format(plugin_root) else {
return HashMap::new();
};
load_plugin_mcp_servers_from_manifest(plugin_root, &manifest.paths, plugin_policy).await
load_plugin_mcp_servers_from_manifest_with_format(
plugin_root,
&loaded_manifest.manifest.paths,
plugin_policy,
/*plugin_data_root*/ None,
loaded_manifest.format,
)
.await
}
pub(crate) async fn load_plugin_mcp_servers_from_manifest(
pub(crate) async fn load_plugin_mcp_servers_from_manifest_with_format(
plugin_root: &Path,
manifest_paths: &PluginManifestPaths,
plugin_policy: Option<&HashMap<String, PluginMcpServerConfig>>,
plugin_data_root: Option<&Path>,
manifest_format: PluginManifestFormat,
) -> HashMap<String, McpServerConfig> {
let mut mcp_servers = HashMap::new();
match &manifest_paths.mcp_servers {
@@ -1457,7 +1527,13 @@ pub(crate) async fn load_plugin_mcp_servers_from_manifest(
}
Some(PluginManifestMcpServers::Path(_)) | None => {
for mcp_config_path in plugin_mcp_config_paths(plugin_root, manifest_paths) {
let plugin_mcp = load_mcp_servers_from_file(plugin_root, &mcp_config_path).await;
let plugin_mcp = load_mcp_servers_from_file(
plugin_root,
plugin_data_root,
manifest_format,
&mcp_config_path,
)
.await;
for (name, mut config) in plugin_mcp.mcp_servers {
if let Some(policy) = plugin_policy.and_then(|policy| policy.get(&name)) {
apply_plugin_mcp_server_policy(&mut config, policy);
@@ -1480,12 +1556,74 @@ pub(crate) async fn load_plugin_mcp_servers_from_manifest(
async fn load_mcp_servers_from_file(
plugin_root: &Path,
plugin_data_root: Option<&Path>,
manifest_format: PluginManifestFormat,
mcp_config_path: &AbsolutePathBuf,
) -> PluginMcpDiscovery {
let is_agent_plugin_mcp = manifest_format == PluginManifestFormat::AgentPlugin;
if is_agent_plugin_mcp {
match tokio::fs::symlink_metadata(mcp_config_path.as_path()).await {
Ok(metadata) if metadata.file_type().is_symlink() || !metadata.is_file() => {
warn!(
path = %mcp_config_path.display(),
"Agent Plugins MCP config is not a regular file; disabling MCP"
);
return PluginMcpDiscovery::default();
}
Ok(_) => {}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
return PluginMcpDiscovery::default();
}
Err(err) => {
warn!(
path = %mcp_config_path.display(),
"failed to inspect Agent Plugins MCP config; disabling MCP: {err}"
);
return PluginMcpDiscovery::default();
}
}
let resolved_root = match tokio::fs::canonicalize(plugin_root).await {
Ok(path) => path,
Err(err) => {
warn!(
plugin = %plugin_root.display(),
"failed to resolve Agent Plugins root; disabling MCP: {err}"
);
return PluginMcpDiscovery::default();
}
};
let resolved_config = match tokio::fs::canonicalize(mcp_config_path.as_path()).await {
Ok(path) => path,
Err(err) => {
warn!(
path = %mcp_config_path.display(),
"failed to resolve Agent Plugins MCP config; disabling MCP: {err}"
);
return PluginMcpDiscovery::default();
}
};
if !resolved_config.starts_with(&resolved_root) {
warn!(
plugin = %plugin_root.display(),
path = %mcp_config_path.display(),
"Agent Plugins MCP config resolves outside the plugin root; disabling MCP"
);
return PluginMcpDiscovery::default();
}
}
let Ok(contents) = tokio::fs::read_to_string(mcp_config_path.as_path()).await else {
return PluginMcpDiscovery::default();
};
let parsed = match parse_plugin_mcp_config(plugin_root, &contents) {
let fallback_data_root = plugin_root.join(".plugin-data");
let mut parsed = match if is_agent_plugin_mcp {
parse_agent_plugin_mcp_config(
plugin_root,
plugin_data_root.unwrap_or(&fallback_data_root),
&contents,
)
} else {
parse_plugin_mcp_config(plugin_root, &contents)
} {
Ok(parsed) => parsed,
Err(err) => {
warn!(
@@ -1495,6 +1633,23 @@ async fn load_mcp_servers_from_file(
return PluginMcpDiscovery::default();
}
};
if is_agent_plugin_mcp
&& let Some(plugin_data_root) = plugin_data_root
&& parsed
.servers
.values()
.any(|server| matches!(&server.transport, McpServerTransportConfig::Stdio { .. }))
&& let Err(err) = tokio::fs::create_dir_all(plugin_data_root).await
{
warn!(
plugin = %plugin_root.display(),
path = %plugin_data_root.display(),
"failed to create Agent Plugins data directory; disabling stdio MCP servers: {err}"
);
parsed.servers.retain(|_, server| {
!matches!(&server.transport, McpServerTransportConfig::Stdio { .. })
});
}
for error in parsed.errors {
warn!(
plugin = %plugin_root.display(),
codex-rs/core-plugins/src/loader_tests.rs
+176 -2
View File
@@ -1,5 +1,6 @@
use super::*;
use crate::manifest::load_plugin_manifest;
use crate::manifest::load_plugin_manifest_with_format;
use crate::test_support::write_file;
use codex_config::ConfigLayerEntry;
use codex_config::ConfigLayerSource;
@@ -7,6 +8,7 @@ use codex_config::ConfigRequirements;
use codex_config::ConfigRequirementsToml;
use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS;
use codex_plugin::PluginId;
use codex_utils_plugins::AGENT_PLUGIN_SCHEMA_URI;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
@@ -25,6 +27,177 @@ fn user_layer(path: AbsolutePathBuf, config: &str) -> ConfigLayerEntry {
)
}
#[tokio::test]
async fn agent_plugin_overlay_apps_are_not_runtime_active() {
let temp_dir = TempDir::new().expect("tempdir");
let plugin_root = temp_dir.path().join("plugin");
write_file(
&plugin_root.join("plugin.json"),
&format!(r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"plugin"}}"#),
);
write_file(
&plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"plugin","apps":"./.app.json"}"#,
);
write_file(
&plugin_root.join(".app.json"),
r#"{"apps":{"example":{"id":"connector_example"}}}"#,
);
assert!(load_plugin_apps(&plugin_root).await.is_empty());
}
#[cfg(unix)]
#[tokio::test]
async fn agent_plugin_mcp_rejects_config_symlink_outside_plugin_root() {
let temp_dir = TempDir::new().expect("tempdir");
let plugin_root = temp_dir.path().join("plugin");
let outside_config = temp_dir.path().join("outside-mcp.json");
fs::create_dir_all(&plugin_root).expect("create plugin root");
fs::write(
plugin_root.join("plugin.json"),
format!(r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"plugin"}}"#),
)
.expect("write Agent Plugins manifest");
fs::write(
&outside_config,
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/mcp.schema.json","mcpServers":{"outside":{"type":"stdio","command":"echo"}}}"#,
)
.expect("write outside MCP config");
std::os::unix::fs::symlink(&outside_config, plugin_root.join("mcp.json"))
.expect("create MCP symlink");
let config_path = AbsolutePathBuf::from_absolute_path(plugin_root.join("mcp.json"))
.expect("absolute MCP path");
let discovered = load_mcp_servers_from_file(
&plugin_root,
/*plugin_data_root*/ None,
PluginManifestFormat::AgentPlugin,
&config_path,
)
.await;
assert!(discovered.mcp_servers.is_empty());
}
#[tokio::test]
async fn agent_plugin_mcp_rejects_present_nonregular_config() {
let temp_dir = TempDir::new().expect("tempdir");
let plugin_root = temp_dir.path().join("plugin");
let config_path = plugin_root.join("mcp.json");
fs::create_dir_all(&config_path).expect("create nonregular MCP config");
let discovered = load_mcp_servers_from_file(
&plugin_root,
/*plugin_data_root*/ None,
PluginManifestFormat::AgentPlugin,
&AbsolutePathBuf::from_absolute_path(config_path).expect("absolute MCP path"),
)
.await;
assert!(discovered.mcp_servers.is_empty());
}
#[tokio::test]
async fn legacy_manifest_can_point_at_root_mcp_json() {
let temp_dir = TempDir::new().expect("tempdir");
let plugin_root = temp_dir.path().join("plugin");
fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest directory");
fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"plugin","mcpServers":"./mcp.json"}"#,
)
.expect("write legacy manifest");
fs::write(
plugin_root.join("mcp.json"),
r#"{"mcpServers":{"legacy":{"command":"echo"}}}"#,
)
.expect("write legacy MCP config");
let manifest = load_plugin_manifest(&plugin_root).expect("load legacy manifest");
let discovered = load_plugin_mcp_servers_from_manifest_with_format(
&plugin_root,
&manifest.paths,
/*plugin_policy*/ None,
/*plugin_data_root*/ None,
PluginManifestFormat::Legacy,
)
.await;
assert_eq!(
discovered.keys().collect::<Vec<_>>(),
vec![&"legacy".to_string()]
);
}
#[tokio::test]
async fn installed_agent_plugin_uses_isolated_data_root_for_stdio_mcp() {
let temp_dir = TempDir::new().expect("tempdir");
let plugin_root = temp_dir.path().join("plugins/cache/c/a-b/local");
write_file(
&plugin_root.join("plugin.json"),
&format!(r#"{{"$schema":"{AGENT_PLUGIN_SCHEMA_URI}","name":"a-b"}}"#),
);
write_file(
&plugin_root.join("mcp.json"),
r#"{
"$schema": "https://agent-plugins.org/schemas/1.0.0/mcp.schema.json",
"mcpServers": {
"example": {
"type": "stdio",
"command": "echo"
}
}
}"#,
);
let stack = ConfigLayerStack::new(
vec![user_layer(
user_config_path(&temp_dir, "config.toml"),
"[plugins.\"a-b@c\"]\nenabled = true\n",
)],
ConfigRequirements::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack");
let store = PluginStore::new(temp_dir.path().to_path_buf());
let plugins = load_plugins_from_layer_stack(
&stack,
RemoteInstalledPluginsSnapshot::default(),
&store,
/*plugin_skill_snapshots*/ None,
Some(Product::Codex),
/*remote_global_catalog_active*/ false,
Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
)
.await;
let expected_data_root = temp_dir
.path()
.join("plugins")
.join("data")
.join("agent-plugins")
.join("6920dd17774030852d11d1b94758fcaae4f894c7b2f36301ed174bc3b33e0743");
let expected_data_root = AbsolutePathBuf::from_absolute_path(expected_data_root)
.expect("absolute Agent Plugin data root")
.canonicalize()
.expect("canonical Agent Plugin data root");
let server = plugins
.first()
.and_then(|plugin| plugin.mcp_servers.get("example"))
.expect("Agent plugin stdio MCP server");
let McpServerTransportConfig::Stdio { env, .. } = &server.transport else {
panic!("expected stdio MCP server");
};
assert_eq!(
env.as_ref()
.and_then(|env| env.get("PLUGIN_DATA"))
.map(String::as_str),
expected_data_root.as_path().to_str()
);
assert!(expected_data_root.as_path().is_dir());
}
#[test]
fn configured_plugins_from_stack_merges_user_layers() {
let temp_dir = TempDir::new().expect("tempdir");
@@ -265,7 +438,8 @@ fn write_hook_file(plugin_root: &AbsolutePathBuf, relative_path: &str, event: &s
}
fn load_sources(plugin_root: &AbsolutePathBuf) -> (Vec<PluginHookSource>, Vec<String>) {
let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest");
let loaded_manifest =
load_plugin_manifest_with_format(plugin_root.as_path()).expect("manifest");
let plugin_data_root = AbsolutePathBuf::try_from(
plugin_root
.as_path()
@@ -278,7 +452,7 @@ fn load_sources(plugin_root: &AbsolutePathBuf) -> (Vec<PluginHookSource>, Vec<St
plugin_root,
&plugin_id(),
&plugin_data_root,
&manifest.paths,
&loaded_manifest.manifest.paths,
)
}
codex-rs/core-plugins/src/manager.rs
+37 -15
View File
@@ -10,8 +10,8 @@ use crate::loader::curated_plugin_cache_version;
use crate::loader::load_plugin_apps_from_manifest;
use crate::loader::load_plugin_hooks;
use crate::loader::load_plugin_hooks_from_layer_stack;
use crate::loader::load_plugin_mcp_servers_from_manifest;
use crate::loader::load_plugin_skills_with_identity;
use crate::loader::load_plugin_mcp_servers_from_manifest_with_format;
use crate::loader::load_plugin_skill_inventory;
use crate::loader::load_plugins_from_layer_stack;
use crate::loader::log_plugin_load_errors;
use crate::loader::materialize_marketplace_plugin_source;
@@ -21,8 +21,10 @@ use crate::loader::refresh_curated_plugin_cache;
use crate::loader::refresh_non_curated_plugin_cache_detailed;
use crate::loader::refresh_non_curated_plugin_cache_force_reinstall_detailed;
use crate::loader::remote_installed_plugins_to_config;
use crate::manifest::PluginManifestFormat;
use crate::manifest::PluginManifestInterface;
use crate::manifest::load_plugin_manifest;
use crate::manifest::load_plugin_manifest_with_format;
use crate::marketplace::MarketplaceError;
use crate::marketplace::MarketplaceInterface;
use crate::marketplace::MarketplaceListError;
@@ -416,6 +418,7 @@ impl From<PluginDetail> for PluginCapabilitySummary {
Self {
config_name: value.id,
display_name: value.name,
plugin_namespace: None,
description: prompt_safe_plugin_description(value.description.as_deref()),
has_skills,
mcp_server_names: value.mcp_server_names,
@@ -1997,18 +2000,24 @@ impl PluginsManager {
"path does not exist or is not a directory".to_string(),
));
}
let manifest =
let loaded_manifest =
if codex_utils_plugins::find_plugin_manifest_path(source_path.as_path()).is_some() {
load_plugin_manifest(source_path.as_path())
load_plugin_manifest_with_format(source_path.as_path())
} else {
plugin
.manifest_fallback
.as_ref()
.and_then(|fallback| fallback.parse_for_plugin_root(source_path.as_path()))
.map(|manifest| crate::manifest::LoadedPluginManifest {
manifest,
format: PluginManifestFormat::Legacy,
})
}
.ok_or_else(|| {
MarketplaceError::InvalidPlugin("missing or invalid plugin.json".to_string())
})?;
let manifest_format = loaded_manifest.format;
let manifest = loaded_manifest.manifest;
let description = manifest.description.clone();
let marketplace_category = plugin
.interface
@@ -2022,21 +2031,27 @@ impl PluginsManager {
plugin_id: plugin_id.as_key(),
remote_plugin_id: self.remote_plugin_id_for(&plugin_id),
};
let resolved_skills = load_plugin_skills_with_identity(
let skill_config_rules = codex_core_skills::config_rules::skill_config_rules_from_stack(
&config.config_layer_stack,
);
let resolved_skills = load_plugin_skill_inventory(
&source_path,
&plugin_identity,
&manifest,
manifest_format,
self.restriction_product,
&codex_core_skills::config_rules::skill_config_rules_from_stack(
&config.config_layer_stack,
),
/*plugin_skill_snapshots*/ None,
Arc::clone(&self.skill_root_scan_slots),
)
.await;
.await
.resolve(&skill_config_rules);
let plugin_data_root = self.store.plugin_data_root(&plugin_id);
let (hook_sources, _hook_load_warnings) =
load_plugin_hooks(&source_path, &plugin_id, &plugin_data_root, &manifest.paths);
let (hook_sources, _hook_load_warnings) = if manifest_format == PluginManifestFormat::Legacy
{
load_plugin_hooks(&source_path, &plugin_id, &plugin_data_root, &manifest.paths)
} else {
(Vec::new(), Vec::new())
};
let hooks = plugin_hook_declarations(&hook_sources)
.into_iter()
.map(|hook| PluginHookSummary {
@@ -2045,15 +2060,22 @@ impl PluginsManager {
})
.collect();
let auth_mode = self.auth_mode();
let mut app_declarations =
load_plugin_apps_from_manifest(source_path.as_path(), &manifest.paths).await;
let mut mcp_servers = load_plugin_mcp_servers_from_manifest(
let mut app_declarations = if manifest_format == PluginManifestFormat::Legacy {
load_plugin_apps_from_manifest(source_path.as_path(), &manifest.paths).await
} else {
Vec::new()
};
let mcp_data_root = (manifest_format == PluginManifestFormat::AgentPlugin)
.then(|| self.store.mcp_data_root(&plugin_id, manifest_format));
let mut mcp_servers = load_plugin_mcp_servers_from_manifest_with_format(
source_path.as_path(),
&manifest.paths,
/*plugin_policy*/ None,
mcp_data_root.as_deref(),
manifest_format,
)
.await;
if auth_mode.is_some() {
if manifest_format == PluginManifestFormat::Legacy && auth_mode.is_some() {
apply_app_mcp_routing_policy(
&mut app_declarations,
&mut mcp_servers,
codex-rs/core-plugins/src/manager_tests.rs
+118 -2
View File
@@ -52,6 +52,7 @@ use codex_protocol::protocol::Product;
use codex_skills::SkillConfigRules;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_plugins::SkillDiscoveryMode;
use pretty_assertions::assert_eq;
use std::fs;
use std::path::Path;
@@ -675,6 +676,7 @@ async fn plugin_auth_projection_reprojects_cached_plugins_when_auth_changes() {
PluginCapabilitySummary {
config_name: "docs@test".to_string(),
display_name: "docs".to_string(),
plugin_namespace: Some("docs".to_string()),
description: None,
has_skills: false,
mcp_server_names: vec!["docs".to_string()],
@@ -683,6 +685,7 @@ async fn plugin_auth_projection_reprojects_cached_plugins_when_auth_changes() {
PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: Some("sample".to_string()),
description: None,
has_skills: false,
mcp_server_names: Vec::new(),
@@ -705,6 +708,7 @@ async fn plugin_auth_projection_reprojects_cached_plugins_when_auth_changes() {
PluginCapabilitySummary {
config_name: "docs@test".to_string(),
display_name: "docs".to_string(),
plugin_namespace: Some("docs".to_string()),
description: None,
has_skills: false,
mcp_server_names: vec!["docs".to_string()],
@@ -713,6 +717,7 @@ async fn plugin_auth_projection_reprojects_cached_plugins_when_auth_changes() {
PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: Some("sample".to_string()),
description: None,
has_skills: false,
mcp_server_names: vec!["sample".to_string()],
@@ -739,7 +744,11 @@ fn write_plugin_with_version(
format!(r#"{{"name":"{manifest_name}"{version}}}"#),
)
.unwrap();
fs::write(plugin_root.join("skills/SKILL.md"), "skill").unwrap();
fs::write(
plugin_root.join("skills/SKILL.md"),
format!("---\nname: {manifest_name}-skill\ndescription: test skill\n---\n\n# Test skill\n"),
)
.unwrap();
fs::write(plugin_root.join(".mcp.json"), r#"{"mcpServers":{}}"#).unwrap();
}
@@ -931,6 +940,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
root: AbsolutePathBuf::try_from(plugin_root.clone()).unwrap(),
enabled: true,
skill_roots: vec![plugin_root.join("skills").abs()],
skill_discovery_mode: SkillDiscoveryMode::Recursive,
disabled_skill_paths: HashSet::new(),
has_enabled_skills: true,
mcp_servers: HashMap::from([(
@@ -973,6 +983,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
&[PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: Some("sample".to_string()),
description: Some("Plugin that includes the sample MCP server and Skills".to_string(),),
has_skills: true,
mcp_server_names: vec!["sample".to_string()],
@@ -1232,6 +1243,7 @@ async fn installed_plugin_telemetry_metadata_collects_capabilities() {
capability_summary: Some(PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: Some("sample".to_string()),
description: None,
has_skills: true,
mcp_server_names: Vec::new(),
@@ -1264,6 +1276,7 @@ async fn installed_plugin_telemetry_metadata_resolves_persisted_remote_identity(
capability_summary: Some(PluginCapabilitySummary {
config_name: "linear@openai-curated-remote".to_string(),
display_name: "linear".to_string(),
plugin_namespace: Some("linear".to_string()),
description: None,
has_skills: true,
mcp_server_names: Vec::new(),
@@ -1317,6 +1330,7 @@ async fn installed_plugin_telemetry_metadata_prefers_remote_snapshot_identity()
capability_summary: Some(PluginCapabilitySummary {
config_name: "linear@openai-curated-remote".to_string(),
display_name: "linear".to_string(),
plugin_namespace: Some("linear".to_string()),
description: None,
has_skills: true,
mcp_server_names: Vec::new(),
@@ -1354,6 +1368,7 @@ fn capability_summary_telemetry_metadata_uses_local_identity() {
let summary = PluginCapabilitySummary {
config_name: "linear@openai-curated-remote".to_string(),
display_name: "Linear".to_string(),
plugin_namespace: Some("linear".to_string()),
description: Some("Track work".to_string()),
has_skills: true,
mcp_server_names: vec!["linear".to_string()],
@@ -1387,6 +1402,7 @@ fn capability_summary_telemetry_metadata_resolves_persisted_remote_identity() {
let summary = PluginCapabilitySummary {
config_name: "linear@openai-curated-remote".to_string(),
display_name: "Linear".to_string(),
plugin_namespace: Some("linear".to_string()),
description: Some("Track work".to_string()),
has_skills: true,
mcp_server_names: vec!["linear".to_string()],
@@ -1772,6 +1788,7 @@ enabled = true
&[PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: Some("sample".to_string()),
description: None,
has_skills: true,
mcp_server_names: Vec::new(),
@@ -1817,6 +1834,7 @@ async fn plugin_telemetry_metadata_uses_default_mcp_config_path() {
Some(PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: Some("sample".to_string()),
description: None,
has_skills: false,
mcp_server_names: vec!["sample".to_string()],
@@ -1858,6 +1876,7 @@ async fn plugin_capability_summary_uses_manifest_mcp_server_objects() {
Some(PluginCapabilitySummary {
config_name: "counter-sample@test".to_string(),
display_name: "counter-sample".to_string(),
plugin_namespace: Some("counter-sample".to_string()),
description: None,
has_skills: false,
mcp_server_names: vec!["counter".to_string()],
@@ -2273,7 +2292,6 @@ async fn load_plugin_skills_dedupes_overlapping_manifest_roots() {
interface: None,
};
let plugin_id = PluginId::parse("sample@test").expect("plugin id should parse");
let resolved = load_plugin_skills(
&plugin_root,
&plugin_id,
@@ -2502,6 +2520,7 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions
root: AbsolutePathBuf::try_from(plugin_root).unwrap(),
enabled: false,
skill_roots: Vec::new(),
skill_discovery_mode: SkillDiscoveryMode::Recursive,
disabled_skill_paths: HashSet::new(),
has_enabled_skills: false,
mcp_servers: HashMap::new(),
@@ -2679,6 +2698,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() {
root: AbsolutePathBuf::try_from(codex_home.path().join(dir_name)).unwrap(),
enabled: true,
skill_roots: Vec::new(),
skill_discovery_mode: SkillDiscoveryMode::Recursive,
disabled_skill_paths: HashSet::new(),
has_enabled_skills: false,
mcp_servers: HashMap::new(),
@@ -2690,6 +2710,12 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() {
let summary = |config_name: &str, display_name: &str| PluginCapabilitySummary {
config_name: config_name.to_string(),
display_name: display_name.to_string(),
plugin_namespace: Some(
config_name
.split_once('@')
.map_or(config_name, |(name, _)| name)
.to_string(),
),
description: None,
..PluginCapabilitySummary::default()
};
@@ -3985,6 +4011,96 @@ plugins = true
);
}
#[tokio::test]
async fn agent_plugin_read_and_tool_suggestions_use_portable_capabilities_only() {
let tmp = tempfile::tempdir().unwrap();
let repo_root = tmp.path().join("repo");
let plugin_root = repo_root.join("agent-plugin");
fs::create_dir_all(repo_root.join(".git")).unwrap();
fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap();
write_file(
&repo_root.join(".agents/plugins/marketplace.json"),
r#"{"name":"debug","plugins":[{"name":"agent-plugin","source":"./agent-plugin"}]}"#,
);
write_file(
&plugin_root.join("plugin.json"),
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"agent.tools"}"#,
);
write_file(
&plugin_root.join("skills/direct/SKILL.md"),
"---\nname: direct\ndescription: Direct skill\n---\n",
);
write_file(
&plugin_root.join("skills/group/nested/SKILL.md"),
"---\nname: nested\ndescription: Nested skill\n---\n",
);
write_file(
&plugin_root.join("mcp.json"),
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/mcp.schema.json","mcpServers":{"portable":{"type":"stdio","command":"echo"}}}"#,
);
write_file(
&plugin_root.join(".codex-plugin/plugin.json"),
r#"{"apps":"./.app.json","hooks":"./hooks/hooks.json"}"#,
);
write_file(
&plugin_root.join(".app.json"),
r#"{"apps":{"legacy":{"id":"connector_legacy"}}}"#,
);
write_file(
&plugin_root.join("hooks/hooks.json"),
r#"{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"echo legacy"}]}]}}"#,
);
write_file(
&tmp.path().join(CONFIG_TOML_FILE),
"[features]\nplugins = true\n",
);
let config = load_config(tmp.path(), &repo_root).await;
let manager = PluginsManager::new(tmp.path().to_path_buf());
let plugin = manager
.list_marketplaces_for_config(
&config,
&[AbsolutePathBuf::try_from(repo_root).unwrap()],
/*include_openai_curated*/ false,
)
.unwrap()
.marketplaces
.into_iter()
.find(|marketplace| marketplace.name == "debug")
.unwrap()
.plugins
.into_iter()
.find(|plugin| plugin.name == "agent-plugin")
.unwrap();
let detail = manager
.read_plugin_detail_for_marketplace_plugin(&config, "debug", plugin.clone())
.await
.unwrap();
let suggestion = manager
.tool_suggest_metadata_for_marketplace_plugin(
"debug",
&plugin,
&SkillConfigRules::default(),
)
.await
.unwrap();
assert_eq!(
detail
.skills
.iter()
.map(|skill| skill.name.as_str())
.collect::<Vec<_>>(),
vec!["agent.tools:direct"]
);
assert_eq!(detail.mcp_server_names, vec!["portable"]);
assert!(detail.apps.is_empty());
assert!(detail.hooks.is_empty());
assert!(suggestion.has_skills);
assert_eq!(suggestion.mcp_server_names, vec!["portable"]);
assert!(suggestion.app_connector_ids.is_empty());
}
#[tokio::test]
async fn read_plugin_for_config_does_not_fallback_from_invalid_plugin_manifest() {
let tmp = tempfile::tempdir().unwrap();
codex-rs/core-plugins/src/manifest.rs
+28 -1
View File
@@ -31,6 +31,17 @@ pub type PluginManifestPaths = codex_plugin::manifest::PluginManifestPaths<Absol
pub type UriPluginManifest = codex_plugin::manifest::PluginManifest<PathUri>;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum PluginManifestFormat {
Legacy,
AgentPlugin,
}
pub(crate) struct LoadedPluginManifest {
pub manifest: PluginManifest,
pub format: PluginManifestFormat,
}
#[derive(Debug, Default, Deserialize)]
#[serde(rename_all = "camelCase")]
struct RawPluginManifest {
@@ -143,6 +154,15 @@ enum RawPluginManifestHooks {
/// Loads a plugin manifest from the local host filesystem.
pub fn load_plugin_manifest(plugin_root: &Path) -> Option<PluginManifest> {
load_plugin_manifest_with_format(plugin_root).map(|loaded| loaded.manifest)
}
pub fn is_agent_plugin_manifest(plugin_root: &Path) -> bool {
load_plugin_manifest_with_format(plugin_root)
.is_some_and(|loaded| loaded.format == PluginManifestFormat::AgentPlugin)
}
pub(crate) fn load_plugin_manifest_with_format(plugin_root: &Path) -> Option<LoadedPluginManifest> {
let manifest_path = find_plugin_manifest_path(plugin_root)?;
let contents = fs::read_to_string(&manifest_path).ok()?;
let is_agent_plugin = manifest_path == plugin_root.join(AGENT_PLUGIN_MANIFEST_RELATIVE_PATH);
@@ -162,7 +182,14 @@ pub fn load_plugin_manifest(plugin_root: &Path) -> Option<PluginManifest> {
.as_ref()
.map(|(path, contents)| (path.as_path(), contents.as_str())),
) {
Ok(manifest) => Some(manifest),
Ok(manifest) => Some(LoadedPluginManifest {
manifest,
format: if is_agent_plugin {
PluginManifestFormat::AgentPlugin
} else {
PluginManifestFormat::Legacy
},
}),
Err(err) => {
tracing::warn!(
path = %manifest_path.display(),
codex-rs/core-plugins/src/remote.rs
+1
View File
@@ -1524,6 +1524,7 @@ pub async fn resolve_remote_plugin_uninstall_target(
let fallback_capability_summary = PluginCapabilitySummary {
config_name: plugin_id.as_key(),
display_name: plugin.release.display_name,
plugin_namespace: Some(plugin_id.plugin_name.clone()),
description: prompt_safe_plugin_description(Some(&plugin.release.description)),
has_skills: !plugin.release.skills.is_empty(),
mcp_server_names,
codex-rs/core-plugins/src/script_attribution_tests.rs
+2
View File
@@ -10,6 +10,7 @@ use crate::test_support::write_curated_plugin_sha_with;
use crate::test_support::write_openai_api_curated_marketplace;
use crate::test_support::write_openai_curated_marketplace;
use codex_plugin::PluginLoadOutcome;
use codex_utils_plugins::SkillDiscoveryMode;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::collections::HashSet;
@@ -30,6 +31,7 @@ fn loaded_plugin(config_name: &str, root: &Path, enabled: bool) -> LoadedPlugin
root: path(root),
enabled,
skill_roots: Vec::new(),
skill_discovery_mode: SkillDiscoveryMode::Recursive,
disabled_skill_paths: HashSet::new(),
has_enabled_skills: false,
mcp_servers: HashMap::new(),
codex-rs/core-plugins/src/store.rs
+23 -10
View File
@@ -1,5 +1,6 @@
use crate::command_migration::migrate_plugin_commands;
use crate::manifest::PluginManifest;
use crate::manifest::PluginManifestFormat;
use crate::manifest::load_plugin_manifest;
use crate::manifest::parse_plugin_manifest;
use codex_plugin::PluginId;
@@ -24,6 +25,7 @@ use std::path::PathBuf;
pub const DEFAULT_PLUGIN_VERSION: &str = "local";
pub const PLUGINS_CACHE_DIR: &str = "plugins/cache";
pub const PLUGINS_DATA_DIR: &str = "plugins/data";
const AGENT_PLUGINS_DATA_DIR: &str = "agent-plugins";
const REMOTE_PLUGIN_INSTALL_METADATA_FILE: &str = ".codex-remote-plugin-install.json";
const REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION: u8 = 1;
const DEFAULT_AGENT_PLUGIN_VERSION: &str = "1.0.0";
@@ -143,6 +145,27 @@ impl PluginStore {
))
}
pub(crate) fn agent_plugin_data_root(&self, plugin_id: &PluginId) -> AbsolutePathBuf {
let mut digest = Sha256::new();
digest.update(plugin_id.marketplace_name.as_bytes());
digest.update([0]);
digest.update(plugin_id.plugin_name.as_bytes());
self.data_root
.join(AGENT_PLUGINS_DATA_DIR)
.join(hex_prefix(&digest.finalize(), /*count*/ 32))
}
pub(crate) fn mcp_data_root(
&self,
plugin_id: &PluginId,
manifest_format: PluginManifestFormat,
) -> AbsolutePathBuf {
match manifest_format {
PluginManifestFormat::AgentPlugin => self.agent_plugin_data_root(plugin_id),
PluginManifestFormat::Legacy => self.plugin_data_root(plugin_id),
}
}
pub fn active_plugin_version(&self, plugin_id: &PluginId) -> Option<String> {
let mut discovered_versions = fs::read_dir(self.plugin_base_root(plugin_id).as_path())
.ok()?
@@ -745,16 +768,6 @@ fn copy_dir_recursive(source: &Path, target: &Path) -> Result<(), PluginStoreErr
} else if file_type.is_file() {
fs::copy(&source_path, &target_path)
.map_err(|err| PluginStoreError::io("failed to copy plugin file", err))?;
} else if file_type.is_symlink() {
return Err(PluginStoreError::Invalid(format!(
"plugin source contains unsupported symbolic link: {}",
source_path.display()
)));
} else {
return Err(PluginStoreError::Invalid(format!(
"plugin source contains unsupported file type: {}",
source_path.display()
)));
}
}
codex-rs/core-plugins/src/store_tests.rs
+32 -14
View File
@@ -162,6 +162,28 @@ fn plugin_data_root_derives_path_from_key() {
);
}
#[test]
fn agent_plugin_data_root_is_stable_and_unambiguous() {
let tmp = tempdir().unwrap();
let store = PluginStore::new(tmp.path().to_path_buf());
let first = PluginId::new("a-b".to_string(), "c".to_string()).unwrap();
let second = PluginId::new("a".to_string(), "b-c".to_string()).unwrap();
let first_root = store.agent_plugin_data_root(&first);
let second_root = store.agent_plugin_data_root(&second);
let expected_parent = tmp.path().join("plugins/data/agent-plugins");
assert_ne!(first_root, second_root);
assert_eq!(
first_root.as_path(),
expected_parent.join("6920dd17774030852d11d1b94758fcaae4f894c7b2f36301ed174bc3b33e0743")
);
assert_eq!(
second_root.as_path(),
expected_parent.join("fa89b988ebbe54a68fdcbeb87fb913a5238d482084a3cee49a86288c2d45fa90")
);
}
#[test]
fn install_with_version_uses_requested_cache_version() {
let tmp = tempdir().unwrap();
@@ -396,7 +418,7 @@ fn agent_plugin_install_does_not_migrate_commands() {
#[cfg(unix)]
#[test]
fn agent_plugin_install_rejects_symlinked_skill_file() {
fn agent_plugin_install_skips_symlinked_skill_file() {
let tmp = tempdir().unwrap();
let plugin_root = tmp.path().join("agent-plugin");
let skill_root = plugin_root.join("skills/greet");
@@ -411,19 +433,17 @@ fn agent_plugin_install_rejects_symlinked_skill_file() {
std::os::unix::fs::symlink(&outside_skill, skill_root.join("SKILL.md")).unwrap();
let plugin_id = PluginId::new("agent-plugin".to_string(), "debug".to_string()).unwrap();
let err = PluginStore::new(tmp.path().to_path_buf())
let result = PluginStore::new(tmp.path().to_path_buf())
.install(AbsolutePathBuf::try_from(plugin_root).unwrap(), plugin_id)
.expect_err("symlinked Agent Plugin skill should be rejected");
.expect("install Agent Plugin");
assert!(
err.to_string()
.contains("plugin source contains unsupported symbolic link")
);
assert!(result.installed_path.join("plugin.json").is_file());
assert!(!result.installed_path.join("skills/greet/SKILL.md").exists());
}
#[cfg(unix)]
#[test]
fn agent_plugin_install_rejects_symlinked_executable() {
fn agent_plugin_install_skips_symlinked_executable() {
let tmp = tempdir().unwrap();
let plugin_root = tmp.path().join("agent-plugin");
let bin_root = plugin_root.join("bin");
@@ -438,14 +458,12 @@ fn agent_plugin_install_rejects_symlinked_executable() {
std::os::unix::fs::symlink(&outside_executable, bin_root.join("tool")).unwrap();
let plugin_id = PluginId::new("agent-plugin".to_string(), "debug".to_string()).unwrap();
let err = PluginStore::new(tmp.path().to_path_buf())
let result = PluginStore::new(tmp.path().to_path_buf())
.install(AbsolutePathBuf::try_from(plugin_root).unwrap(), plugin_id)
.expect_err("symlinked Agent Plugin executable should be rejected");
.expect("install Agent Plugin");
assert!(
err.to_string()
.contains("plugin source contains unsupported symbolic link")
);
assert!(result.installed_path.join("plugin.json").is_file());
assert!(!result.installed_path.join("bin/tool").exists());
}
#[test]
codex-rs/core-plugins/src/tool_suggest_metadata.rs
+14 -3
View File
@@ -21,7 +21,8 @@ use crate::loader::load_plugin_mcp_servers;
use crate::loader::load_plugin_skill_inventory;
use crate::manager::ConfiguredMarketplacePlugin;
use crate::manager::remote_plugin_install_required_description;
use crate::manifest::load_plugin_manifest;
use crate::manifest::PluginManifestFormat;
use crate::manifest::load_plugin_manifest_with_format;
use crate::marketplace::MarketplaceError;
use crate::marketplace::MarketplacePluginSource;
@@ -53,6 +54,7 @@ struct PluginArtifactIdentity {
pub(crate) struct ToolSuggestMetadataFragment {
config_name: String,
display_name: String,
plugin_namespace: Option<String>,
description: Option<String>,
mcp_server_names: Vec<String>,
app_declarations: Vec<AppDeclaration>,
@@ -86,6 +88,7 @@ impl ToolSuggestMetadataFragment {
PluginCapabilitySummary {
config_name: self.config_name.clone(),
display_name: self.display_name.clone(),
plugin_namespace: self.plugin_namespace.clone(),
description: self.description.clone(),
has_skills: self
.skill_inventory
@@ -206,6 +209,7 @@ async fn load_plugin_metadata(
return Ok(Arc::new(ToolSuggestMetadataFragment {
config_name: plugin.id.clone(),
display_name: plugin.name.clone(),
plugin_namespace: Some(plugin.name.clone()),
description: prompt_safe_plugin_description(Some(
&remote_plugin_install_required_description(&plugin.source),
)),
@@ -217,16 +221,18 @@ async fn load_plugin_metadata(
if !plugin_root.as_path().is_dir() {
return Err("path does not exist or is not a directory".to_string());
}
let manifest = load_plugin_manifest(plugin_root.as_path())
let loaded_manifest = load_plugin_manifest_with_format(plugin_root.as_path())
.ok_or_else(|| "missing or invalid plugin.json".to_string())?;
let plugin_identity = PluginIdentity {
plugin_id: plugin_id.as_key(),
remote_plugin_id: None,
};
let manifest = loaded_manifest.manifest;
let skill_inventory = load_plugin_skill_inventory(
plugin_root,
&plugin_identity,
&manifest,
loaded_manifest.format,
restriction_product,
/*plugin_skill_snapshots*/ None,
root_scan_slots,
@@ -239,11 +245,16 @@ async fn load_plugin_metadata(
.collect::<Vec<_>>();
mcp_server_names.sort_unstable();
mcp_server_names.dedup();
let app_declarations = load_plugin_apps(plugin_root.as_path()).await;
let app_declarations = if loaded_manifest.format == PluginManifestFormat::AgentPlugin {
Vec::new()
} else {
load_plugin_apps(plugin_root.as_path()).await
};
Ok(Arc::new(ToolSuggestMetadataFragment {
config_name: plugin.id.clone(),
display_name: plugin.name.clone(),
plugin_namespace: Some(manifest.name.clone()),
description: prompt_safe_plugin_description(manifest.description.as_deref()),
mcp_server_names,
app_declarations,
codex-rs/core-skills/Cargo.toml
+1
View File
@@ -27,6 +27,7 @@ codex-skills = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-path-uri = { workspace = true }
codex-utils-plugins = { workspace = true }
codex-utils-string = { workspace = true }
dirs = { workspace = true }
dunce = { workspace = true }
futures = { workspace = true }
codex-rs/core-skills/src/injection.rs
+20
View File
@@ -16,6 +16,9 @@ use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL;
use codex_utils_string::take_bytes_at_char_boundary;
use crate::MAX_SKILL_PROMPT_BYTES;
#[derive(Debug, Default)]
pub struct SkillInjections {
@@ -93,6 +96,18 @@ pub async fn build_skill_injections(
let path = PathUri::from_abs_path(&skill.path_to_skills_md);
match fs.read_file_text(&path, /*sandbox*/ None).await {
Ok(contents) => {
let (contents, truncated) =
if loaded_skills.is_some_and(|outcome| outcome.is_agent_plugin_skill(skill)) {
bounded_skill_prompt_contents(&contents)
} else {
(contents, false)
};
if truncated {
result.warnings.push(format!(
"Skill `{}` exceeded the main prompt context limit and was truncated.",
skill.name
));
}
emit_skill_injected_metric(otel, skill, "ok");
invocations.push(SkillInvocation {
skill_name: skill.name.clone(),
@@ -125,6 +140,11 @@ pub async fn build_skill_injections(
result
}
fn bounded_skill_prompt_contents(contents: &str) -> (String, bool) {
let bounded = take_bytes_at_char_boundary(contents, MAX_SKILL_PROMPT_BYTES);
(bounded.to_string(), bounded.len() < contents.len())
}
fn normalize_host_skill_path(path: &str) -> String {
normalize_skill_path(path).replace('\\', "/")
}
codex-rs/core-skills/src/injection_tests.rs
+10
View File
@@ -21,6 +21,16 @@ fn make_skill(name: &str, path: &str) -> SkillMetadata {
}
}
#[test]
fn skill_prompt_contents_are_bounded_at_utf8_boundaries() {
let contents = format!("{}é", "a".repeat(MAX_SKILL_PROMPT_BYTES - 1));
let (bounded, truncated) = bounded_skill_prompt_contents(&contents);
assert_eq!(bounded.len(), MAX_SKILL_PROMPT_BYTES - 1);
assert_eq!(truncated, true);
}
fn set<'a>(items: &'a [&'a str]) -> HashSet<&'a str> {
items.iter().copied().collect()
}
codex-rs/core-skills/src/lib.rs
+6
View File
@@ -10,6 +10,12 @@ pub mod service;
mod skill_instructions;
pub mod system;
/// Hard byte limit for one model-visible skill instruction body.
///
/// Both the legacy explicit-injection path and the skills extension use this
/// limit so a skill cannot bypass context bounds by changing how it is loaded.
pub const MAX_SKILL_PROMPT_BYTES: usize = 8_000;
pub(crate) use invocation_utils::build_implicit_skill_path_indexes;
pub use invocation_utils::detect_implicit_skill_invocation_for_command;
pub use mention_counts::build_skill_name_counts;
codex-rs/core-skills/src/loader.rs
+3 -1
View File
@@ -118,7 +118,7 @@ const SKILLS_METADATA_DIR: &str = "agents";
const SKILLS_METADATA_FILENAME: &str = "openai.yaml";
const SKILLS_DIR_NAME: &str = "skills";
const MAX_NAME_LEN: usize = 64;
const MAX_QUALIFIED_NAME_LEN: usize = 128;
const MAX_QUALIFIED_NAME_LEN: usize = MAX_NAME_LEN * 2 + 1;
const MAX_DESCRIPTION_LEN: usize = 1024;
const MAX_SHORT_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN;
const MAX_DEFAULT_PROMPT_LEN: usize = MAX_DESCRIPTION_LEN;
@@ -191,6 +191,7 @@ where
#[derive(Clone)]
pub(crate) struct SkillRootSnapshot {
pub(crate) root: AbsolutePathBuf,
pub(crate) is_agent_plugin: bool,
pub(crate) skills: Vec<SkillMetadata>,
pub(crate) errors: Vec<SkillError>,
pub(crate) file_system: Arc<dyn ExecutorFileSystem>,
@@ -203,6 +204,7 @@ pub(crate) async fn load_skill_root(root: SkillRoot) -> SkillRootSnapshot {
load_skills_under_root(&root, &canonical_root, &mut outcome).await;
SkillRootSnapshot {
root: canonical_root,
is_agent_plugin: root.discovery_mode == SkillDiscoveryMode::DirectChildren,
skills: outcome.skills,
errors: outcome.errors,
file_system: root.file_system,
codex-rs/core-skills/src/loader_tests.rs
+2 -2
View File
@@ -1800,7 +1800,7 @@ async fn keeps_inherited_namespace_when_symlink_target_is_scan_root_ancestor() {
#[tokio::test]
async fn plugin_skill_name_length_limit_allows_max_qualified_name() {
let root = tempfile::tempdir().expect("tempdir");
let plugin_name = "p".repeat(MAX_NAME_LEN - 1);
let plugin_name = "p".repeat(MAX_NAME_LEN);
let skill_name = "s".repeat(MAX_NAME_LEN);
let plugin_root = root.path().join("plugins").join(&plugin_name);
let frontmatter = format!("name: {skill_name}\ndescription: search sample data");
@@ -1855,7 +1855,7 @@ async fn plugin_skill_name_length_limit_allows_max_qualified_name() {
#[tokio::test]
async fn plugin_skill_name_length_limit_rejects_overlong_qualified_name() {
let root = tempfile::tempdir().expect("tempdir");
let plugin_name = "p".repeat(MAX_NAME_LEN);
let plugin_name = "p".repeat(MAX_NAME_LEN + 1);
let skill_name = "s".repeat(MAX_NAME_LEN);
let plugin_root = root.path().join("plugins").join(&plugin_name);
let frontmatter = format!("name: {skill_name}\ndescription: search sample data");
codex-rs/core-skills/src/model.rs
+9
View File
@@ -28,6 +28,7 @@ pub struct SkillLoadOutcome {
pub disabled_paths: HashSet<AbsolutePathBuf>,
pub(crate) skill_roots: Vec<AbsolutePathBuf>,
pub(crate) skill_root_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
pub(crate) agent_plugin_skill_paths: HashSet<AbsolutePathBuf>,
pub(crate) file_systems_by_skill_path: SkillFileSystemsByPath,
pub(crate) implicit_skills_by_scripts_dir: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
pub(crate) implicit_skills_by_doc_path: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
@@ -56,6 +57,11 @@ impl SkillLoadOutcome {
.map(|skill| (skill, self.is_skill_enabled(skill)))
}
pub fn is_agent_plugin_skill(&self, skill: &SkillMetadata) -> bool {
self.agent_plugin_skill_paths
.contains(&skill.path_to_skills_md)
}
/// Returns the discovery root that supplied a loaded skill path.
pub fn skill_root_for_path(&self, path: &AbsolutePathBuf) -> Option<&AbsolutePathBuf> {
self.skill_root_by_path.get(path)
@@ -159,6 +165,9 @@ pub fn filter_skill_load_outcome_for_product(
.map(|(path, root)| (path.clone(), root.clone()))
.collect(),
);
outcome
.agent_plugin_skill_paths
.retain(|path| retained_paths.contains(path));
let retained_roots: HashSet<AbsolutePathBuf> =
outcome.skill_root_by_path.values().cloned().collect();
outcome
codex-rs/core-skills/src/root_loader.rs
+12 -6
View File
@@ -134,6 +134,7 @@ fn merge_skill_root_snapshots(snapshots: Vec<SkillRootSnapshot>) -> SkillLoadOut
for snapshot in snapshots {
let SkillRootSnapshot {
root,
is_agent_plugin,
skills,
errors,
file_system,
@@ -142,12 +143,14 @@ fn merge_skill_root_snapshots(snapshots: Vec<SkillRootSnapshot>) -> SkillLoadOut
skill_roots.push(root.clone());
}
for skill in &skills {
skill_root_by_path
.entry(skill.path_to_skills_md.clone())
.or_insert_with(|| root.clone());
file_systems_by_skill_path
.entry(skill.path_to_skills_md.clone())
.or_insert_with(|| Arc::clone(&file_system));
let path = skill.path_to_skills_md.clone();
if !skill_root_by_path.contains_key(&path) {
skill_root_by_path.insert(path.clone(), root.clone());
file_systems_by_skill_path.insert(path.clone(), Arc::clone(&file_system));
if is_agent_plugin {
outcome.agent_plugin_skill_paths.insert(path);
}
}
}
outcome.skills.extend(skills);
outcome.errors.extend(errors);
@@ -166,6 +169,9 @@ fn merge_skill_root_snapshots(snapshots: Vec<SkillRootSnapshot>) -> SkillLoadOut
let used_roots = skill_root_by_path.values().cloned().collect::<HashSet<_>>();
skill_roots.retain(|root| used_roots.contains(root));
file_systems_by_skill_path.retain(|path, _| retained_skill_paths.contains(path));
outcome
.agent_plugin_skill_paths
.retain(|path| retained_skill_paths.contains(path));
outcome.skill_roots = skill_roots;
outcome.skill_root_by_path = Arc::new(skill_root_by_path);
outcome.file_systems_by_skill_path = SkillFileSystemsByPath::new(file_systems_by_skill_path);
codex-rs/core/src/config/mod.rs
+11 -4
View File
@@ -1679,10 +1679,17 @@ impl Config {
{
let mut plugin_mcp_servers = plugin.mcp_servers.clone();
self.apply_plugin_mcp_server_requirements(&plugin.config_name, &mut plugin_mcp_servers);
let attribution = McpPluginAttribution::new(
plugin.config_name.clone(),
plugin.display_name().to_string(),
);
let attribution = if plugin.is_agent_plugin() {
McpPluginAttribution::agent_plugin(
plugin.config_name.clone(),
plugin.display_name().to_string(),
)
} else {
McpPluginAttribution::new(
plugin.config_name.clone(),
plugin.display_name().to_string(),
)
};
for (name, plugin_server) in plugin_mcp_servers {
catalog.register(McpServerRegistration::from_plugin(
name,
codex-rs/core/src/mcp_skill_dependencies.rs
+2
View File
@@ -12,6 +12,7 @@ use codex_protocol::request_user_input::RequestUserInputQuestion;
use codex_protocol::request_user_input::RequestUserInputQuestionOption;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_rmcp_client::OAuthDiscoveryTimeout;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::perform_oauth_login;
use tokio_util::sync::CancellationToken;
use tracing::warn;
@@ -152,6 +153,7 @@ pub(crate) async fn maybe_install_mcp_dependencies(
&server_config.transport,
Arc::clone(&http_client),
discovery_timeout,
StreamableHttpRedirectMode::Legacy,
)
.await;
let oauth_config = match login_support {
codex-rs/core/src/mcp_tool_exposure.rs
+38 -2
View File
@@ -11,6 +11,9 @@ use codex_tools::ToolName;
use tracing::instrument;
use tracing::warn;
const MAX_AGENT_PLUGIN_MCP_SPEC_BYTES: usize = 8_000;
const MAX_AGENT_PLUGIN_MCP_TOTAL_BYTES: usize = 64_000;
use crate::config::Config;
use crate::connectors;
use crate::tools::handlers::McpHandler;
@@ -21,6 +24,7 @@ pub(crate) fn append_mcp_tools(
all_mcp_tools: &[McpToolInfo],
connectors: Option<&[connectors::AppInfo]>,
config: &Config,
mcp_server_catalog: &codex_mcp::ResolvedMcpCatalog,
search_tool_enabled: bool,
registry: &mut ToolRegistry,
) -> HashSet<ToolName> {
@@ -35,11 +39,43 @@ pub(crate) fn append_mcp_tools(
ToolExposure::Direct
};
let mut registered_tools = HashSet::new();
let mut agent_plugin_bytes = 0usize;
for tool in non_app_tools.chain(app_tools) {
let tool_name = tool.canonical_tool_name();
match McpHandler::new(tool.clone()) {
let agent_plugin = mcp_server_catalog
.server(&tool.server_name)
.is_some_and(|server| server.source().is_agent_plugin());
let handler = if agent_plugin {
McpHandler::new_agent_plugin(tool.clone())
} else {
McpHandler::new(tool.clone())
};
match handler {
Ok(handler) => {
if registry.register_external_with_exposure(Arc::new(handler), exposure) {
let fits_agent_budget = if agent_plugin {
handler.model_spec_bytes().is_ok_and(|bytes| {
if bytes > MAX_AGENT_PLUGIN_MCP_SPEC_BYTES {
return false;
}
let next = agent_plugin_bytes.saturating_add(bytes);
if next <= MAX_AGENT_PLUGIN_MCP_TOTAL_BYTES {
agent_plugin_bytes = next;
true
} else {
false
}
})
} else {
true
};
let tool_exposure = if fits_agent_budget {
exposure
} else {
ToolExposure::Hidden
};
if registry.register_external_with_exposure(Arc::new(handler), tool_exposure)
&& fits_agent_budget
{
registered_tools.insert(tool_name);
}
}
codex-rs/core/src/mcp_tool_exposure_test.rs
+105
View File
@@ -2,6 +2,9 @@ use std::collections::HashMap;
use std::sync::Arc;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::McpPluginAttribution;
use codex_mcp::McpServerRegistration;
use codex_mcp::ResolvedMcpCatalog;
use codex_mcp::ToolInfo;
use codex_tools::ToolExposure;
use codex_tools::ToolName;
@@ -96,12 +99,29 @@ fn runtimes_by_name(
connectors: Option<&[AppInfo]>,
config: &Config,
search_tool_enabled: bool,
) -> HashMap<ToolName, ToolExposure> {
runtimes_by_name_with_catalog(
tools,
connectors,
config,
&ResolvedMcpCatalog::default(),
search_tool_enabled,
)
}
fn runtimes_by_name_with_catalog(
tools: &[ToolInfo],
connectors: Option<&[AppInfo]>,
config: &Config,
mcp_server_catalog: &ResolvedMcpCatalog,
search_tool_enabled: bool,
) -> HashMap<ToolName, ToolExposure> {
let mut registry = ToolRegistry::default();
append_mcp_tools(
tools,
connectors,
config,
mcp_server_catalog,
search_tool_enabled,
&mut registry,
);
@@ -111,6 +131,91 @@ fn runtimes_by_name(
.collect()
}
#[tokio::test]
async fn agent_plugin_budget_hides_only_overflow_agent_tools() {
let codex_home = tempdir().expect("tempdir should succeed");
std::fs::write(
codex_home.path().join(CONFIG_TOML_FILE),
"[mcp_servers.agent]\ncommand = \"echo\"\n",
)
.expect("write config");
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.build()
.await
.expect("config should build");
let agent_config = config.mcp_servers.get()["agent"].clone();
let legacy_config = agent_config.clone();
let mut catalog = ResolvedMcpCatalog::builder();
catalog.register(McpServerRegistration::from_plugin(
"agent".to_string(),
McpPluginAttribution::agent_plugin("agent@test".to_string(), "Agent".to_string()),
/*plugin_order*/ 0,
agent_config,
));
catalog.register(McpServerRegistration::from_plugin(
"legacy".to_string(),
McpPluginAttribution::new("legacy@test".to_string(), "Legacy".to_string()),
/*plugin_order*/ 1,
legacy_config,
));
let catalog = catalog.build();
let mut tools = (0..40)
.map(|index| {
let name = format!("tool_{index}");
let mut tool = make_mcp_tool(
"agent",
&name,
"mcp__agent",
&name,
/*connector_id*/ None,
/*connector_name*/ None,
);
tool.namespace_description = Some("n".repeat(1_000));
tool.tool.description = Some("d".repeat(1_000).into());
tool
})
.collect::<Vec<_>>();
let oversized_name = "x".repeat(MAX_AGENT_PLUGIN_MCP_SPEC_BYTES);
let oversized_agent_tool = make_mcp_tool(
"agent",
"oversized_agent_tool",
"mcp__agent",
&oversized_name,
/*connector_id*/ None,
/*connector_name*/ None,
);
tools.push(oversized_agent_tool.clone());
let legacy_tool = make_mcp_tool(
"legacy",
"legacy_tool",
"mcp__legacy",
&oversized_name,
/*connector_id*/ None,
/*connector_name*/ None,
);
tools.push(legacy_tool.clone());
let runtimes = runtimes_by_name_with_catalog(
&tools, /*connectors*/ None, &config, &catalog, /*search_tool_enabled*/ false,
);
let agent_exposures = tools[..40]
.iter()
.map(|tool| runtimes[&tool.canonical_tool_name()])
.collect::<Vec<_>>();
assert!(agent_exposures.contains(&ToolExposure::Direct));
assert!(agent_exposures.contains(&ToolExposure::Hidden));
assert_eq!(
runtimes[&oversized_agent_tool.canonical_tool_name()],
ToolExposure::Hidden
);
assert_eq!(
runtimes[&legacy_tool.canonical_tool_name()],
ToolExposure::Direct
);
}
fn with_visibility(mut tool: ToolInfo, visibility: &[&str]) -> ToolInfo {
tool.tool.meta = Some(MetaObject(
serde_json::json!({ "ui": { "visibility": visibility } })
codex-rs/core/src/plugins/mentions_tests.rs
+1
View File
@@ -18,6 +18,7 @@ fn plugin(config_name: &str, display_name: &str) -> PluginCapabilitySummary {
PluginCapabilitySummary {
config_name: config_name.to_string(),
display_name: display_name.to_string(),
plugin_namespace: None,
description: None,
has_skills: true,
mcp_server_names: Vec::new(),
codex-rs/core/src/plugins/render.rs
+22 -3
View File
@@ -3,6 +3,11 @@ use crate::context::AvailablePluginsInstructions;
#[cfg(test)]
use crate::context::ContextualUserFragment;
use crate::plugins::PluginCapabilitySummary;
use codex_utils_string::take_bytes_at_char_boundary;
const MAX_EXPLICIT_PLUGIN_INSTRUCTIONS_BYTES: usize = 4 * 1024;
const TRUNCATED_PLUGIN_INSTRUCTIONS_SUFFIX: &str =
"\n- Additional plugin capabilities omitted to fit the context limit.";
#[cfg(test)]
pub(crate) fn render_plugins_section(plugins: &[PluginCapabilitySummary]) -> Option<String> {
@@ -20,9 +25,12 @@ pub(crate) fn render_explicit_plugin_instructions(
)];
if plugin.has_skills {
let skill_namespace = plugin
.plugin_namespace
.as_deref()
.unwrap_or(plugin.display_name.as_str());
lines.push(format!(
"- Skills from this plugin are prefixed with `{}:`.",
plugin.display_name
"- Skills from this plugin are prefixed with `{skill_namespace}:`."
));
}
@@ -54,7 +62,18 @@ pub(crate) fn render_explicit_plugin_instructions(
lines.push("Use these plugin-associated capabilities to help solve the task.".to_string());
Some(lines.join("\n"))
Some(bound_explicit_plugin_instructions(lines.join("\n")))
}
fn bound_explicit_plugin_instructions(rendered: String) -> String {
if rendered.len() <= MAX_EXPLICIT_PLUGIN_INSTRUCTIONS_BYTES {
return rendered;
}
let max_prefix_bytes = MAX_EXPLICIT_PLUGIN_INSTRUCTIONS_BYTES
.saturating_sub(TRUNCATED_PLUGIN_INSTRUCTIONS_SUFFIX.len());
let prefix = take_bytes_at_char_boundary(&rendered, max_prefix_bytes);
format!("{prefix}{TRUNCATED_PLUGIN_INSTRUCTIONS_SUFFIX}")
}
#[cfg(test)]
codex-rs/core/src/plugins/render_tests.rs
+41
View File
@@ -11,6 +11,7 @@ fn render_plugins_section_keeps_plugin_usage_guidance_without_listing_plugins()
let rendered = render_plugins_section(&[PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: None,
description: Some("inspect sample data".to_string()),
has_skills: true,
..PluginCapabilitySummary::default()
@@ -21,3 +22,43 @@ fn render_plugins_section_keeps_plugin_usage_guidance_without_listing_plugins()
assert_eq!(rendered, expected);
}
#[test]
fn explicit_plugin_instructions_use_manifest_namespace_for_skills() {
let rendered = render_explicit_plugin_instructions(
&PluginCapabilitySummary {
config_name: "acme.tools@test".to_string(),
display_name: "Acme Developer Tools".to_string(),
plugin_namespace: Some("acme.tools".to_string()),
has_skills: true,
..PluginCapabilitySummary::default()
},
&[],
&[],
)
.expect("skill capability should render");
assert!(rendered.contains("`acme.tools:`"));
assert!(!rendered.contains("`Acme Developer Tools:`"));
}
#[test]
fn explicit_plugin_instructions_are_bounded() {
let servers = (0..1_024)
.map(|index| format!("server-{index}"))
.collect::<Vec<_>>();
let rendered = render_explicit_plugin_instructions(
&PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
..PluginCapabilitySummary::default()
},
&servers,
&[],
)
.expect("MCP capability should render");
assert!(rendered.len() <= MAX_EXPLICIT_PLUGIN_INSTRUCTIONS_BYTES);
assert!(rendered.ends_with(TRUNCATED_PLUGIN_INSTRUCTIONS_SUFFIX));
}
codex-rs/core/src/tools/handlers/mcp.rs
+40 -3
View File
@@ -23,6 +23,7 @@ use codex_tools::ToolName;
use codex_tools::ToolSearchInfo;
use codex_tools::ToolSearchSourceInfo;
use codex_tools::ToolSpec;
use codex_tools::agent_plugin_mcp_tool_to_responses_api_tool;
use codex_tools::mcp_tool_to_responses_api_tool;
use codex_utils_string::take_bytes_at_char_boundary;
use futures::future::BoxFuture;
@@ -31,6 +32,7 @@ use serde_json::Value;
const LEGACY_MCP_TOOL_NAME_PREFIX: &str = "mcp__";
const MCP_TOOL_NAME_DELIMITER: &str = "__";
const MAX_AGENT_PLUGIN_MCP_NAMESPACE_DESCRIPTION_BYTES: usize = 1_000;
const MAX_MCP_NAMESPACE_DESCRIPTION_BYTES: usize = 512 * 1024;
pub struct McpHandler {
@@ -40,10 +42,38 @@ pub struct McpHandler {
impl McpHandler {
pub fn new(tool_info: ToolInfo) -> Result<Self, serde_json::Error> {
let spec = create_tool_spec(&tool_info)?;
Self::with_agent_plugin(tool_info, /*agent_plugin*/ false)
}
pub fn new_agent_plugin(tool_info: ToolInfo) -> Result<Self, serde_json::Error> {
Self::with_agent_plugin(tool_info, /*agent_plugin*/ true)
}
fn with_agent_plugin(
mut tool_info: ToolInfo,
agent_plugin: bool,
) -> Result<Self, serde_json::Error> {
if agent_plugin {
tool_info.namespace_description =
tool_info
.namespace_description
.as_deref()
.map(|description| {
take_bytes_at_char_boundary(
description,
MAX_AGENT_PLUGIN_MCP_NAMESPACE_DESCRIPTION_BYTES,
)
.to_string()
});
}
let spec = create_tool_spec(&tool_info, agent_plugin)?;
Ok(Self { tool_info, spec })
}
pub(crate) fn model_spec_bytes(&self) -> Result<usize, serde_json::Error> {
serde_json::to_vec(&self.spec).map(|spec| spec.len())
}
fn hook_tool_name(&self) -> HookToolName {
HookToolName::new(ensure_mcp_prefix(&join_tool_name(&self.tool_name())))
}
@@ -236,9 +266,16 @@ impl CoreToolRuntime for McpHandler {
}
}
fn create_tool_spec(tool_info: &ToolInfo) -> Result<ToolSpec, serde_json::Error> {
fn create_tool_spec(
tool_info: &ToolInfo,
agent_plugin: bool,
) -> Result<ToolSpec, serde_json::Error> {
let tool_name = tool_info.canonical_tool_name();
let tool = mcp_tool_to_responses_api_tool(&tool_name, &tool_info.tool)?;
let tool = if agent_plugin {
agent_plugin_mcp_tool_to_responses_api_tool(&tool_name, &tool_info.tool)?
} else {
mcp_tool_to_responses_api_tool(&tool_name, &tool_info.tool)?
};
let description = tool_info
.namespace_description
.as_deref()
codex-rs/core/src/tools/handlers/mcp_search_tests.rs
+21
View File
@@ -89,6 +89,27 @@ fn mcp_namespace_descriptions_are_bounded_at_512_kib() {
assert_eq!(namespace.description, expected_description);
}
#[test]
fn agent_plugin_namespace_descriptions_use_the_stricter_bound() {
let expected_description = "é".repeat(MAX_AGENT_PLUGIN_MCP_NAMESPACE_DESCRIPTION_BYTES / 2);
let mut info = tool_info();
info.namespace_description = Some(format!("{expected_description}overflow"));
let handler = McpHandler::new_agent_plugin(info).expect("MCP tool spec should build");
let search_info = handler.search_info().expect("MCP search info");
assert_eq!(
search_info.source_info,
Some(ToolSearchSourceInfo {
name: "Calendar".to_string(),
description: Some(expected_description.clone()),
})
);
let LoadableToolSpec::Namespace(namespace) = search_info.entry.output else {
panic!("expected namespace search output");
};
assert_eq!(namespace.description, expected_description);
}
fn tool_info() -> ToolInfo {
ToolInfo {
server_name: "codex-apps".to_string(),
codex-rs/core/src/tools/spec_plan.rs
+1
View File
@@ -149,6 +149,7 @@ pub(crate) fn build_tool_router(
mcp.tools(),
connectors,
&turn_context.config,
&mcp.config().mcp_server_catalog,
search_tool_enabled(turn_context),
&mut registry,
);
codex-rs/core/tests/suite/plugins.rs
+261
View File
@@ -31,6 +31,7 @@ use core_test_support::responses::ResponsesRequest;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
use core_test_support::responses::ev_function_call_with_namespace;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::ev_tool_search_call;
use core_test_support::responses::mount_sse_once;
@@ -164,6 +165,38 @@ fn write_sample_plugin_skill(plugin_root: std::path::PathBuf) -> std::path::Path
skill_dir.join("SKILL.md")
}
fn write_agent_plugin_skill_plugin(home: &TempDir) -> std::path::PathBuf {
let plugin_root = home.path().join("plugins/cache/test/acme.tools/local");
let direct_skill = plugin_root.join("skills/review");
let nested_skill = plugin_root.join("skills/group/hidden");
std::fs::create_dir_all(&direct_skill).expect("create direct skill");
std::fs::create_dir_all(&nested_skill).expect("create nested skill");
std::fs::write(
plugin_root.join("plugin.json"),
r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"acme.tools","extensions":{"com.openai":{"interface":{"displayName":"Acme Developer Tools"}}}}"#,
)
.expect("write Agent Plugin manifest");
std::fs::write(
direct_skill.join("SKILL.md"),
format!(
"---\nname: review\ndescription: Review code\n---\n\n{}\nAGENT_SKILL_TRUNCATED_TAIL\n",
"x".repeat(9_000)
),
)
.expect("write direct skill");
std::fs::write(
nested_skill.join("SKILL.md"),
"---\nname: hidden\ndescription: Hidden skill\n---\n\nHidden.\n",
)
.expect("write nested skill");
std::fs::write(
home.path().join("config.toml"),
"[features]\nplugins = true\n\n[plugins.\"acme.tools@test\"]\nenabled = true\n",
)
.expect("write Agent Plugin config");
direct_skill.join("SKILL.md")
}
fn write_plugin_mcp_plugin(home: &TempDir, command: &str) {
let plugin_root = write_sample_plugin_manifest_and_config(home);
std::fs::write(
@@ -485,6 +518,234 @@ async fn capability_sections_render_in_developer_message_in_order() -> Result<()
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn agent_plugin_skills_use_shared_catalog_and_direct_child_discovery() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let resp_mock = mount_sse_once(
&server,
sse(vec![ev_response_created("resp1"), ev_completed("resp1")]),
)
.await;
let codex_home = Arc::new(TempDir::new()?);
let skill_path = std::fs::canonicalize(write_agent_plugin_skill_plugin(codex_home.as_ref()))?;
let test_codex = test_codex()
.with_home(Arc::clone(&codex_home))
.with_extensions(skills_extensions())
.build(&server)
.await?;
test_codex
.codex
.submit(Op::UserInput {
items: vec![UserInput::Skill {
name: "acme.tools:review".into(),
path: skill_path,
}],
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: Default::default(),
})
.await?;
let warning = wait_for_event(&test_codex.codex, |ev| {
matches!(
ev,
EventMsg::Warning(warning)
if warning.message.contains("main prompt context limit")
)
})
.await;
wait_for_event(&test_codex.codex, |ev| {
matches!(ev, EventMsg::TurnComplete(_))
})
.await;
let developer_text = resp_mock
.single_request()
.message_input_texts("developer")
.join("\n");
assert!(developer_text.contains("acme.tools:review: Review code"));
assert!(!developer_text.contains("acme.tools:hidden"));
let user_text = resp_mock
.single_request()
.message_input_texts("user")
.join("\n");
assert!(user_text.contains("acme.tools:review"));
assert!(!user_text.contains("AGENT_SKILL_TRUNCATED_TAIL"));
let EventMsg::Warning(warning) = warning else {
unreachable!("wait_for_event matched an Agent skill truncation warning")
};
assert!(warning.message.contains("acme.tools:review"));
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn legacy_plugin_skill_prompt_remains_complete() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let resp_mock = mount_sse_once(
&server,
sse(vec![ev_response_created("resp1"), ev_completed("resp1")]),
)
.await;
let codex_home = Arc::new(TempDir::new()?);
let skill_path = write_plugin_skill_plugin(codex_home.as_ref());
let skill_contents = format!(
"---\nname: sample-search\ndescription: inspect sample data\n---\n\n{}\nLEGACY_SKILL_FULL_TAIL\n",
"x".repeat(9_000)
);
std::fs::write(&skill_path, &skill_contents)?;
let skill_path = std::fs::canonicalize(skill_path)?;
let test_codex = test_codex()
.with_home(codex_home)
.with_extensions(skills_extensions())
.build(&server)
.await?;
test_codex
.codex
.submit(Op::UserInput {
items: vec![UserInput::Skill {
name: "sample:sample-search".into(),
path: skill_path,
}],
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: Default::default(),
})
.await?;
wait_for_event(&test_codex.codex, |ev| {
matches!(ev, EventMsg::TurnComplete(_))
})
.await;
let user_text = resp_mock
.single_request()
.message_input_texts("user")
.join("\n");
assert!(user_text.contains(&skill_contents));
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn agent_plugin_root_mcp_stdio_tool_round_trip_expands_reserved_paths() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let search_call_id = "search-agent-echo";
let tool_call_id = "call-agent-echo";
let mock = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_tool_search_call(search_call_id, &serde_json::json!({"query": "echo"})),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_function_call_with_namespace(
tool_call_id,
"mcp__agent",
"echo",
r#"{"message":"ping"}"#,
),
ev_completed("resp-2"),
]),
sse(vec![
ev_response_created("resp-3"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-3"),
]),
],
)
.await;
let codex_home = Arc::new(TempDir::new()?);
write_agent_plugin_skill_plugin(codex_home.as_ref());
let plugin_root = codex_home
.path()
.join("plugins/cache/test/acme.tools/local");
let stdio_server = match stdio_server_bin() {
Ok(path) => path,
Err(err) => {
eprintln!("test_stdio_server binary not available, skipping test: {err}");
return Ok(());
}
};
let stdio_server_name = format!("test_stdio_server{}", std::env::consts::EXE_SUFFIX);
std::fs::copy(stdio_server, plugin_root.join(&stdio_server_name))?;
let mcp_config = serde_json::json!({
"$schema": "https://agent-plugins.org/schemas/1.0.0/mcp.schema.json",
"mcpServers": {
"agent": {
"type": "stdio",
"command": format!("./{stdio_server_name}"),
"env": {"MCP_TEST_VALUE": "${PLUGIN_ROOT}|${PLUGIN_DATA}"}
}
}
});
std::fs::write(
plugin_root.join("mcp.json"),
serde_json::to_vec_pretty(&mcp_config)?,
)?;
let test_codex = test_codex()
.with_home(Arc::clone(&codex_home))
.build(&server)
.await?;
wait_for_mcp_server(&test_codex.codex, "agent").await?;
let data_root = std::fs::read_dir(codex_home.path().join("plugins/data/agent-plugins"))?
.next()
.expect("Agent Plugin data root")?
.path()
.canonicalize()?;
let expected_env = format!(
"{}|{}",
plugin_root.canonicalize()?.display(),
data_root.display()
);
test_codex
.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "call the Agent Plugin echo tool".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: Default::default(),
})
.await?;
let end = wait_for_event(&test_codex.codex, |event| {
matches!(event, EventMsg::McpToolCallEnd(_))
})
.await;
wait_for_event(&test_codex.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let EventMsg::McpToolCallEnd(end) = end else {
unreachable!("wait_for_event matched an MCP tool end")
};
let result = end.result.as_ref().expect("Agent Plugin MCP tool result");
assert_eq!(
result
.structured_content
.as_ref()
.and_then(|content| content.get("env"))
.and_then(serde_json::Value::as_str),
Some(expected_env.as_str())
);
let requests = mock.requests();
let search_output = requests[1].tool_search_output(search_call_id);
assert!(namespace_child_tool(&search_output, "mcp__agent", "echo").is_some());
assert!(requests[2].function_call_output(tool_call_id).is_object());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn agent_turns_route_curated_plugin_skills_after_auth_switch() -> Result<()> {
const CHATGPT_CURATED_PLUGIN_SKILL: &str = "chatgpt-plugin:chatgpt-skill";
codex-rs/ext/skills/src/render.rs
+2 -2
View File
@@ -5,6 +5,7 @@ use std::path::Component;
use std::path::Path;
use std::path::PathBuf;
use codex_core_skills::MAX_SKILL_PROMPT_BYTES;
use codex_protocol::protocol::SkillScope;
use codex_utils_string::approx_token_count;
use codex_utils_string::take_bytes_at_char_boundary;
@@ -17,7 +18,6 @@ use crate::fragments::AvailableSkillsInstructions;
const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000;
const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2;
const MAX_MAIN_PROMPT_BYTES: usize = 8_000;
const MAX_CATALOG_SKILL_DESCRIPTION_CHARS: usize = 1_024;
const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "...";
const SKILL_DESCRIPTION_TRUNCATION_WARNING_THRESHOLD_CHARS: usize = 100;
@@ -1083,7 +1083,7 @@ pub(crate) fn truncate_catalog_skill_description(description: &str) -> Cow<'_, s
}
pub(crate) fn truncate_main_prompt_contents(contents: &str) -> (String, bool) {
truncate_utf8_to_bytes(contents, MAX_MAIN_PROMPT_BYTES)
truncate_utf8_to_bytes(contents, MAX_SKILL_PROMPT_BYTES)
}
pub(crate) fn truncate_utf8_to_bytes(contents: &str, max_bytes: usize) -> (String, bool) {
codex-rs/plugin/src/lib.rs
+1
View File
@@ -52,6 +52,7 @@ pub fn app_connector_ids_from_declarations<'a>(
pub struct PluginCapabilitySummary {
pub config_name: String,
pub display_name: String,
pub plugin_namespace: Option<String>,
pub description: Option<String>,
pub has_skills: bool,
pub mcp_server_names: Vec<String>,
codex-rs/plugin/src/load_outcome.rs
+8 -1
View File
@@ -25,6 +25,7 @@ pub struct LoadedPlugin<M> {
pub root: AbsolutePathBuf,
pub enabled: bool,
pub skill_roots: Vec<AbsolutePathBuf>,
pub skill_discovery_mode: SkillDiscoveryMode,
pub disabled_skill_paths: HashSet<AbsolutePathBuf>,
pub has_enabled_skills: bool,
pub mcp_servers: HashMap<String, M>,
@@ -42,6 +43,10 @@ impl<M> LoadedPlugin<M> {
pub fn display_name(&self) -> &str {
self.manifest_name.as_deref().unwrap_or(&self.config_name)
}
pub fn is_agent_plugin(&self) -> bool {
self.skill_discovery_mode == SkillDiscoveryMode::DirectChildren
}
}
fn plugin_capability_summary_from_loaded<M>(
@@ -57,6 +62,7 @@ fn plugin_capability_summary_from_loaded<M>(
let summary = PluginCapabilitySummary {
config_name: plugin.config_name.clone(),
display_name: plugin.display_name().to_string(),
plugin_namespace: plugin.plugin_namespace.clone(),
description: prompt_safe_plugin_description(plugin.manifest_description.as_deref()),
has_skills: plugin.has_enabled_skills,
mcp_server_names,
@@ -143,7 +149,7 @@ impl<M: Clone> PluginLoadOutcome<M> {
},
plugin_namespace: plugin_namespace.clone(),
plugin_root: plugin.root.clone(),
discovery_mode: SkillDiscoveryMode::Recursive,
discovery_mode: plugin.skill_discovery_mode,
});
}
}
@@ -241,6 +247,7 @@ mod tests {
root: test_path(config_name),
enabled: true,
skill_roots,
skill_discovery_mode: SkillDiscoveryMode::Recursive,
disabled_skill_paths: HashSet::new(),
has_enabled_skills: true,
mcp_servers: HashMap::new(),
codex-rs/rmcp-client/src/auth_status.rs
+63 -5
View File
@@ -12,6 +12,7 @@ use rmcp::transport::AuthorizationManager;
use rmcp::transport::auth::AuthError;
use tracing::debug;
use crate::http_client_adapter::StreamableHttpRedirectMode;
use crate::oauth::StoredOAuthTokenStatus;
use crate::oauth::oauth_token_status;
use crate::oauth_http_client::OAuthHttpClientAdapter;
@@ -85,7 +86,9 @@ pub async fn determine_streamable_http_auth_status(
keyring_backend_kind: AuthKeyringBackendKind,
http_client: Arc<dyn HttpClient>,
discovery_timeout: OAuthDiscoveryTimeout,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<McpAuthState> {
let has_configured_headers = has_configured_headers(&http_headers, &env_http_headers);
let default_headers = match auth_status_before_discovery(
server_name,
url,
@@ -106,6 +109,8 @@ pub async fn determine_streamable_http_auth_status(
default_headers,
http_client,
discovery_timeout,
has_configured_headers,
redirect_mode,
)
.await,
)
@@ -193,13 +198,17 @@ pub async fn discover_streamable_http_oauth(
env_http_headers: Option<HashMap<String, String>>,
http_client: Arc<dyn HttpClient>,
discovery_timeout: OAuthDiscoveryTimeout,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<Option<StreamableHttpOAuthDiscovery>> {
let has_configured_headers = has_configured_headers(&http_headers, &env_http_headers);
let default_headers = build_default_headers(http_headers, env_http_headers)?;
discover_streamable_http_oauth_with_headers_and_http_client(
url,
default_headers,
http_client,
discovery_timeout,
has_configured_headers,
redirect_mode,
)
.await
}
@@ -209,13 +218,24 @@ async fn discover_streamable_http_oauth_with_headers_and_http_client(
default_headers: HeaderMap,
http_client: Arc<dyn HttpClient>,
discovery_timeout: OAuthDiscoveryTimeout,
has_configured_headers: bool,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<Option<StreamableHttpOAuthDiscovery>> {
let oauth_http_client = match discovery_timeout {
OAuthDiscoveryTimeout::Requested => {
OAuthHttpClientAdapter::new(http_client, default_headers)
}
OAuthDiscoveryTimeout::Requested => OAuthHttpClientAdapter::new_with_redirect_mode(
http_client,
default_headers,
has_configured_headers,
redirect_mode,
),
OAuthDiscoveryTimeout::Capped(max_timeout) => {
OAuthHttpClientAdapter::new_with_max_timeout(http_client, default_headers, max_timeout)
OAuthHttpClientAdapter::new_with_max_timeout_and_redirect_mode(
http_client,
default_headers,
max_timeout,
has_configured_headers,
redirect_mode,
)
}
};
let mut authorization_manager =
@@ -224,6 +244,18 @@ async fn discover_streamable_http_oauth_with_headers_and_http_client(
discover_streamable_http_oauth_with_manager(&authorization_manager).await
}
fn has_configured_headers(
http_headers: &Option<HashMap<String, String>>,
env_http_headers: &Option<HashMap<String, String>>,
) -> bool {
http_headers
.as_ref()
.is_some_and(|headers| !headers.is_empty())
|| env_http_headers
.as_ref()
.is_some_and(|headers| !headers.is_empty())
}
async fn discover_streamable_http_oauth_with_manager(
authorization_manager: &AuthorizationManager,
) -> Result<Option<StreamableHttpOAuthDiscovery>> {
@@ -268,6 +300,7 @@ mod tests {
use axum::http::header::WWW_AUTHENTICATE;
use axum::routing::get;
use codex_exec_server::ExecServerError;
use codex_exec_server::HttpRedirectPolicy;
use codex_exec_server::HttpRequestParams;
use codex_exec_server::HttpRequestResponse;
use codex_exec_server::HttpResponseBodyStream;
@@ -308,6 +341,7 @@ mod tests {
#[derive(Default)]
struct RecordingHttpClient {
headers: Mutex<Option<Vec<(String, String)>>>,
redirect_policy: Mutex<Option<HttpRedirectPolicy>>,
timeout_ms: Mutex<Option<Option<u64>>>,
}
@@ -342,6 +376,11 @@ mod tests {
.timeout_ms
.lock()
.expect("timeout recorder lock should not be poisoned") = Some(params.timeout_ms);
*self
.redirect_policy
.lock()
.expect("redirect policy recorder lock should not be poisoned") =
Some(params.redirect_policy);
Box::pin(async {
Err(ExecServerError::HttpRequest(
"expected discovery request failure".to_string(),
@@ -440,6 +479,7 @@ mod tests {
AuthKeyringBackendKind::default(),
test_http_client(),
OAuthDiscoveryTimeout::Requested,
StreamableHttpRedirectMode::Legacy,
)
.await
.expect("status should compute");
@@ -464,6 +504,7 @@ mod tests {
AuthKeyringBackendKind::default(),
test_http_client(),
OAuthDiscoveryTimeout::Requested,
StreamableHttpRedirectMode::Legacy,
)
.await
.expect("status should compute");
@@ -518,6 +559,7 @@ mod tests {
/*env_http_headers*/ None,
test_http_client(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await;
assert_eq!(
@@ -560,6 +602,7 @@ mod tests {
/*env_http_headers*/ None,
test_http_client(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await
.expect_err("cross-origin OAuth discovery redirects must be rejected");
@@ -602,6 +645,7 @@ mod tests {
AuthKeyringBackendKind::default(),
test_http_client(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await
.expect_err("transient OAuth discovery failures must not become unsupported access");
@@ -632,6 +676,7 @@ mod tests {
/*env_http_headers*/ None,
test_http_client(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await
.expect("discovery should succeed")
@@ -653,6 +698,7 @@ mod tests {
/*env_http_headers*/ None,
http_client.clone(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await;
@@ -679,6 +725,7 @@ mod tests {
/*env_http_headers*/ None,
http_client.clone(),
OAuthDiscoveryTimeout::Requested,
StreamableHttpRedirectMode::Legacy,
)
.await;
@@ -693,7 +740,7 @@ mod tests {
}
#[tokio::test]
async fn routed_oauth_discovery_preserves_configured_headers() {
async fn routed_agent_plugin_oauth_discovery_stops_with_configured_headers() {
let http_client = Arc::new(RecordingHttpClient::default());
let discovery = discover_streamable_http_oauth(
@@ -705,6 +752,7 @@ mod tests {
/*env_http_headers*/ None,
http_client.clone(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::AgentPluginV1,
)
.await;
@@ -722,6 +770,13 @@ mod tests {
.map(|(_, value)| value.as_str()),
Some("configured-value")
);
assert_eq!(
*http_client
.redirect_policy
.lock()
.expect("redirect policy recorder lock should not be poisoned"),
Some(HttpRedirectPolicy::Stop)
);
}
#[tokio::test]
@@ -774,6 +829,7 @@ mod tests {
/*env_http_headers*/ None,
test_http_client(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await
.expect("discovery should succeed")
@@ -800,6 +856,7 @@ mod tests {
/*env_http_headers*/ None,
test_http_client(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await
.expect("discovery should succeed")
@@ -822,6 +879,7 @@ mod tests {
/*env_http_headers*/ None,
test_http_client(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await
.expect("support check should succeed")
codex-rs/rmcp-client/src/http_client_adapter.rs
+26 -4
View File
@@ -63,11 +63,19 @@ const HEADER_SESSION_ID: &str = "Mcp-Session-Id";
const NON_JSON_RESPONSE_BODY_PREVIEW_BYTES: usize = 8_192;
const LEGACY_HTTP_PREVALIDATION_ERROR_CODE: ErrorCode = ErrorCode(-32000);
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum StreamableHttpRedirectMode {
Legacy,
AgentPluginV1,
}
#[derive(Clone)]
pub(crate) struct StreamableHttpClientAdapter {
http_client: Arc<dyn HttpClient>,
default_headers: HeaderMap,
auth_provider: Option<SharedAuthProvider>,
has_configured_headers: bool,
redirect_mode: StreamableHttpRedirectMode,
}
#[derive(Debug, thiserror::Error)]
@@ -87,13 +95,21 @@ impl StreamableHttpClientAdapter {
http_client: Arc<dyn HttpClient>,
default_headers: HeaderMap,
auth_provider: Option<SharedAuthProvider>,
has_configured_headers: bool,
redirect_mode: StreamableHttpRedirectMode,
) -> Self {
Self {
http_client,
default_headers,
auth_provider,
has_configured_headers,
redirect_mode,
}
}
fn redirect_policy(&self, headers: &HeaderMap) -> HttpRedirectPolicy {
mcp_redirect_policy(self.redirect_mode, headers, self.has_configured_headers)
}
}
impl StreamableHttpClient for StreamableHttpClientAdapter {
@@ -150,7 +166,7 @@ impl StreamableHttpClient for StreamableHttpClientAdapter {
let redirect_policy = if mcp_method.as_deref() == Some(DiscoverRequestMethod::VALUE) {
HttpRedirectPolicy::Stop
} else {
mcp_redirect_policy(&headers)
self.redirect_policy(&headers)
};
let body = serde_json::to_vec(&message).map_err(StreamableHttpError::Deserialize)?;
@@ -348,7 +364,7 @@ impl StreamableHttpClient for StreamableHttpClientAdapter {
session.to_string(),
StreamableHttpClientAdapterError::Header,
)?;
let redirect_policy = mcp_redirect_policy(&headers);
let redirect_policy = self.redirect_policy(&headers);
let response = self
.http_client
@@ -421,7 +437,7 @@ impl StreamableHttpClient for StreamableHttpClientAdapter {
StreamableHttpClientAdapterError::Header,
)?;
}
let redirect_policy = mcp_redirect_policy(&headers);
let redirect_policy = self.redirect_policy(&headers);
let (response, body_stream) = self
.http_client
@@ -646,11 +662,17 @@ fn parse_json_rpc_error(body: &[u8]) -> Option<ServerJsonRpcMessage> {
}
}
fn mcp_redirect_policy(headers: &HeaderMap) -> HttpRedirectPolicy {
fn mcp_redirect_policy(
mode: StreamableHttpRedirectMode,
headers: &HeaderMap,
has_configured_headers: bool,
) -> HttpRedirectPolicy {
if headers
.get(HEADER_MCP_PROTOCOL_VERSION)
.and_then(|value| value.to_str().ok())
== Some(ProtocolVersion::V_2026_07_28.as_str())
|| (mode == StreamableHttpRedirectMode::AgentPluginV1
&& (has_configured_headers || headers.contains_key(AUTHORIZATION)))
{
HttpRedirectPolicy::Stop
} else {
codex-rs/rmcp-client/src/http_client_adapter_tests.rs
+62
View File
@@ -1,11 +1,15 @@
use std::io::ErrorKind;
use codex_exec_server::HttpRedirectPolicy;
use http::HeaderMap;
use http::HeaderValue;
use http::header::AUTHORIZATION;
use pretty_assertions::assert_eq;
use super::HttpHeader;
use super::SseEventSizeLimit;
use super::StreamableHttpRedirectMode;
use super::mcp_redirect_policy;
use super::protocol_headers;
#[test]
@@ -25,6 +29,64 @@ fn protocol_headers_preserve_utf8_values() {
);
}
#[test]
fn legacy_configured_headers_follow_redirects() {
assert_eq!(
mcp_redirect_policy(
StreamableHttpRedirectMode::Legacy,
&HeaderMap::new(),
/*has_configured_headers*/ true,
),
HttpRedirectPolicy::Follow
);
}
#[test]
fn agent_plugin_configured_headers_stop_redirects() {
assert_eq!(
mcp_redirect_policy(
StreamableHttpRedirectMode::AgentPluginV1,
&HeaderMap::new(),
/*has_configured_headers*/ true,
),
HttpRedirectPolicy::Stop
);
}
#[test]
fn requests_without_sensitive_headers_follow_redirects() {
assert_eq!(
mcp_redirect_policy(
StreamableHttpRedirectMode::AgentPluginV1,
&HeaderMap::new(),
/*has_configured_headers*/ false,
),
HttpRedirectPolicy::Follow
);
}
#[test]
fn authorization_redirects_depend_on_mode() {
let mut headers = HeaderMap::new();
headers.insert(AUTHORIZATION, HeaderValue::from_static("Bearer secret"));
assert_eq!(
mcp_redirect_policy(
StreamableHttpRedirectMode::Legacy,
&headers,
/*has_configured_headers*/ false,
),
HttpRedirectPolicy::Follow
);
assert_eq!(
mcp_redirect_policy(
StreamableHttpRedirectMode::AgentPluginV1,
&headers,
/*has_configured_headers*/ false,
),
HttpRedirectPolicy::Stop
);
}
#[test]
fn lf_terminators_reset_the_event_limit() {
let mut limit = SseEventSizeLimit::new(Some(8));
codex-rs/rmcp-client/src/lib.rs
+1
View File
@@ -24,6 +24,7 @@ pub use auth_status::determine_streamable_http_auth_status;
pub use auth_status::determine_streamable_http_auth_status_from_credentials;
pub use auth_status::discover_streamable_http_oauth;
pub use codex_protocol::protocol::McpAuthStatus;
pub use http_client_adapter::StreamableHttpRedirectMode;
pub use in_process_transport::InProcessTransportFactory;
pub use oauth::StoredOAuthTokens;
pub use oauth::WrappedOAuthTokenResponse;
codex-rs/rmcp-client/src/oauth_http_client.rs
+109 -1
View File
@@ -8,6 +8,7 @@ use codex_exec_server::HttpHeader;
use codex_exec_server::HttpRedirectPolicy;
use codex_exec_server::HttpRequestParams;
use http::HeaderMap;
use http::header::AUTHORIZATION;
use oauth2::HttpRequest;
use oauth2::HttpResponse;
use rmcp::transport::auth::OAuthHttpClient;
@@ -17,6 +18,7 @@ use rmcp::transport::auth::OAuthHttpRedirectPolicy;
use rmcp::transport::auth::OAuthHttpRequest;
use crate::auth_status::OAuthDiscoveryTimeout;
use crate::http_client_adapter::StreamableHttpRedirectMode;
const MAX_OAUTH_HTTP_RESPONSE_BODY_BYTES: usize = 1024 * 1024;
static NEXT_OAUTH_REQUEST_ID: AtomicU64 = AtomicU64::new(0);
@@ -40,26 +42,50 @@ pub(crate) struct OAuthHttpClientAdapter {
http_client: Arc<dyn HttpClient>,
default_headers: HeaderMap,
timeout: OAuthDiscoveryTimeout,
has_configured_headers: bool,
redirect_mode: StreamableHttpRedirectMode,
}
impl OAuthHttpClientAdapter {
#[cfg(test)]
pub(crate) fn new(http_client: Arc<dyn HttpClient>, default_headers: HeaderMap) -> Self {
Self {
http_client,
default_headers,
timeout: OAuthDiscoveryTimeout::Requested,
has_configured_headers: false,
redirect_mode: StreamableHttpRedirectMode::Legacy,
}
}
pub(crate) fn new_with_max_timeout(
pub(crate) fn new_with_redirect_mode(
http_client: Arc<dyn HttpClient>,
default_headers: HeaderMap,
has_configured_headers: bool,
redirect_mode: StreamableHttpRedirectMode,
) -> Self {
Self {
http_client,
default_headers,
timeout: OAuthDiscoveryTimeout::Requested,
has_configured_headers,
redirect_mode,
}
}
pub(crate) fn new_with_max_timeout_and_redirect_mode(
http_client: Arc<dyn HttpClient>,
default_headers: HeaderMap,
max_timeout: Duration,
has_configured_headers: bool,
redirect_mode: StreamableHttpRedirectMode,
) -> Self {
Self {
http_client,
default_headers,
timeout: OAuthDiscoveryTimeout::Capped(max_timeout),
has_configured_headers,
redirect_mode,
}
}
@@ -84,6 +110,12 @@ impl OAuthHttpClientAdapter {
headers.remove(name);
}
headers.extend(parts.headers);
let redirect_policy = oauth_redirect_policy(
self.redirect_mode,
&headers,
self.has_configured_headers,
redirect_policy,
);
let headers = headers
.iter()
@@ -139,8 +171,84 @@ impl OAuthHttpClientAdapter {
}
}
fn oauth_redirect_policy(
mode: StreamableHttpRedirectMode,
headers: &HeaderMap,
has_configured_headers: bool,
requested_policy: HttpRedirectPolicy,
) -> HttpRedirectPolicy {
if mode == StreamableHttpRedirectMode::AgentPluginV1
&& (has_configured_headers || headers.contains_key(AUTHORIZATION))
{
HttpRedirectPolicy::Stop
} else {
requested_policy
}
}
impl OAuthHttpClient for OAuthHttpClientAdapter {
fn execute(&self, request: OAuthHttpRequest) -> OAuthHttpClientFuture<'_> {
Box::pin(self.execute_request(request.request, request.redirect_policy, request.timeout))
}
}
#[cfg(test)]
mod tests {
use http::HeaderValue;
use pretty_assertions::assert_eq;
use super::*;
#[test]
fn agent_plugin_oauth_stops_only_for_sensitive_headers() {
assert_eq!(
policy(
StreamableHttpRedirectMode::AgentPluginV1,
/*has_configured_headers*/ true,
/*has_authorization*/ false,
),
HttpRedirectPolicy::Stop
);
assert_eq!(
policy(
StreamableHttpRedirectMode::AgentPluginV1,
/*has_configured_headers*/ false,
/*has_authorization*/ true,
),
HttpRedirectPolicy::Stop
);
assert_eq!(
policy(
StreamableHttpRedirectMode::AgentPluginV1,
/*has_configured_headers*/ false,
/*has_authorization*/ false,
),
HttpRedirectPolicy::Follow
);
assert_eq!(
policy(
StreamableHttpRedirectMode::Legacy,
/*has_configured_headers*/ true,
/*has_authorization*/ true,
),
HttpRedirectPolicy::Follow
);
}
fn policy(
mode: StreamableHttpRedirectMode,
has_configured_headers: bool,
has_authorization: bool,
) -> HttpRedirectPolicy {
let mut headers = HeaderMap::new();
if has_authorization {
headers.insert(AUTHORIZATION, HeaderValue::from_static("Bearer secret"));
}
oauth_redirect_policy(
mode,
&headers,
has_configured_headers,
HttpRedirectPolicy::Follow,
)
}
}
codex-rs/rmcp-client/src/perform_oauth_login.rs
+24 -1
View File
@@ -27,6 +27,7 @@ use urlencoding::decode;
use crate::StoredOAuthTokens;
use crate::WrappedOAuthTokenResponse;
use crate::http_client_adapter::StreamableHttpRedirectMode;
use crate::oauth::compute_expires_at_millis;
use crate::oauth_http_client::OAuthHttpClientAdapter;
use crate::save_oauth_tokens;
@@ -38,6 +39,7 @@ struct OAuthHttpContext {
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
http_client: Arc<dyn HttpClient>,
redirect_mode: StreamableHttpRedirectMode,
}
struct CallbackServerGuard {
@@ -109,6 +111,7 @@ pub async fn perform_oauth_login(
callback_url,
http_client,
/*emit_browser_url*/ true,
StreamableHttpRedirectMode::Legacy,
)
.await
}
@@ -127,6 +130,7 @@ pub async fn perform_oauth_login_silent(
callback_port: Option<u16>,
callback_url: Option<&str>,
http_client: Arc<dyn HttpClient>,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<()> {
perform_oauth_login_with_browser_output(
server_name,
@@ -142,6 +146,7 @@ pub async fn perform_oauth_login_silent(
callback_url,
http_client,
/*emit_browser_url*/ false,
redirect_mode,
)
.await
}
@@ -161,11 +166,13 @@ async fn perform_oauth_login_with_browser_output(
callback_url: Option<&str>,
http_client: Arc<dyn HttpClient>,
emit_browser_url: bool,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<()> {
let http_context = OAuthHttpContext {
http_headers,
env_http_headers,
http_client,
redirect_mode,
};
OauthLoginFlow::new(
server_name,
@@ -201,11 +208,13 @@ pub async fn perform_oauth_login_return_url(
callback_port: Option<u16>,
callback_url: Option<&str>,
http_client: Arc<dyn HttpClient>,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<OauthLoginHandle> {
let http_context = OAuthHttpContext {
http_headers,
env_http_headers,
http_client,
redirect_mode,
};
let flow = OauthLoginFlow::new(
server_name,
@@ -515,9 +524,21 @@ impl OauthLoginFlow {
http_headers,
env_http_headers,
http_client,
redirect_mode,
} = http_context;
let has_configured_headers = http_headers
.as_ref()
.is_some_and(|headers| !headers.is_empty())
|| env_http_headers
.as_ref()
.is_some_and(|headers| !headers.is_empty());
let default_headers = build_default_headers(http_headers, env_http_headers)?;
let oauth_http_client = Arc::new(OAuthHttpClientAdapter::new(http_client, default_headers));
let oauth_http_client = Arc::new(OAuthHttpClientAdapter::new_with_redirect_mode(
http_client,
default_headers,
has_configured_headers,
redirect_mode,
));
let scope_refs: Vec<&str> = scopes.iter().map(String::as_str).collect();
let oauth_state = start_authorization(
@@ -728,6 +749,7 @@ mod tests {
use super::CallbackOutcome;
use super::OAuthHttpClientAdapter;
use super::OAuthProviderError;
use super::StreamableHttpRedirectMode;
use super::append_callback_id_to_redirect_uri;
use super::append_query_param;
use super::callback_id_from_server_url;
@@ -975,6 +997,7 @@ mod tests {
/*callback_port*/ None,
/*callback_url*/ None,
http_client.clone(),
StreamableHttpRedirectMode::Legacy,
)
.await
.expect_err("OAuth metadata discovery should fail through the supplied client");
codex-rs/rmcp-client/src/rmcp_client.rs
+59 -2
View File
@@ -70,6 +70,7 @@ use tracing::warn;
use crate::elicitation_client_service::ElicitationClientService;
use crate::http_client_adapter::StreamableHttpClientAdapter;
use crate::http_client_adapter::StreamableHttpClientAdapterError;
use crate::http_client_adapter::StreamableHttpRedirectMode;
use crate::in_process_transport::InProcessTransportFactory;
use crate::oauth::OAuthPersistor;
use crate::oauth::ResolvedOAuthCredentialStore;
@@ -139,6 +140,7 @@ enum TransportRecipe {
pinned_credential_store: Arc<OnceLock<ResolvedOAuthCredentialStore>>,
http_client: Arc<dyn HttpClient>,
auth_provider: Option<SharedAuthProvider>,
redirect_mode: StreamableHttpRedirectMode,
},
}
@@ -480,6 +482,36 @@ impl RmcpClient {
http_client: Arc<dyn HttpClient>,
auth_provider: Option<SharedAuthProvider>,
protocol_mode: McpProtocolMode,
) -> Result<Self> {
Self::new_streamable_http_client_with_protocol_mode_and_redirect_mode(
server_name,
url,
bearer_token,
http_headers,
env_http_headers,
store_mode,
keyring_backend_kind,
http_client,
auth_provider,
protocol_mode,
StreamableHttpRedirectMode::Legacy,
)
.await
}
#[allow(clippy::too_many_arguments)]
pub async fn new_streamable_http_client_with_protocol_mode_and_redirect_mode(
server_name: &str,
url: &str,
bearer_token: Option<String>,
http_headers: Option<HashMap<String, String>>,
env_http_headers: Option<HashMap<String, String>>,
store_mode: OAuthCredentialsStoreMode,
keyring_backend_kind: AuthKeyringBackendKind,
http_client: Arc<dyn HttpClient>,
auth_provider: Option<SharedAuthProvider>,
protocol_mode: McpProtocolMode,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<Self> {
let transport_recipe = TransportRecipe::StreamableHttp {
server_name: server_name.to_string(),
@@ -492,6 +524,7 @@ impl RmcpClient {
pinned_credential_store: Arc::new(OnceLock::new()),
http_client,
auth_provider,
redirect_mode,
};
let transport = Self::create_pending_transport(&transport_recipe).await?;
Ok(Self {
@@ -914,7 +947,14 @@ impl RmcpClient {
pinned_credential_store,
http_client,
auth_provider,
redirect_mode,
} => {
let has_configured_headers = http_headers
.as_ref()
.is_some_and(|headers| !headers.is_empty())
|| env_http_headers
.as_ref()
.is_some_and(|headers| !headers.is_empty());
let default_headers =
build_default_headers(http_headers.clone(), env_http_headers.clone())?;
let auth_provider =
@@ -976,6 +1016,8 @@ impl RmcpClient {
credential_store,
default_headers.clone(),
Arc::clone(http_client),
has_configured_headers,
*redirect_mode,
)
.await
{
@@ -1007,6 +1049,8 @@ impl RmcpClient {
Arc::clone(http_client),
default_headers,
/*auth_provider*/ None,
has_configured_headers,
*redirect_mode,
),
http_config,
);
@@ -1026,6 +1070,8 @@ impl RmcpClient {
Arc::clone(http_client),
default_headers,
auth_provider,
has_configured_headers,
*redirect_mode,
),
http_config,
);
@@ -1333,6 +1379,7 @@ impl RmcpClient {
}
}
#[allow(clippy::too_many_arguments)]
async fn create_oauth_transport_and_runtime(
server_name: &str,
url: &str,
@@ -1340,13 +1387,17 @@ async fn create_oauth_transport_and_runtime(
credential_store: ResolvedOAuthCredentialStore,
default_headers: HeaderMap,
http_client: Arc<dyn HttpClient>,
has_configured_headers: bool,
redirect_mode: StreamableHttpRedirectMode,
) -> Result<(
StreamableHttpClientTransport<AuthClient<StreamableHttpClientAdapter>>,
OAuthPersistor,
)> {
let oauth_http_client = Arc::new(OAuthHttpClientAdapter::new(
let oauth_http_client = Arc::new(OAuthHttpClientAdapter::new_with_redirect_mode(
http_client.clone(),
default_headers.clone(),
has_configured_headers,
redirect_mode,
));
let mut manager =
AuthorizationManager::new_with_oauth_http_client(url.to_string(), oauth_http_client)
@@ -1370,7 +1421,13 @@ async fn create_oauth_transport_and_runtime(
};
let auth_client = AuthClient::new(
StreamableHttpClientAdapter::new(http_client, default_headers, /*auth_provider*/ None),
StreamableHttpClientAdapter::new(
http_client,
default_headers,
/*auth_provider*/ None,
has_configured_headers,
redirect_mode,
),
manager,
);
let auth_manager = auth_client.auth_manager.clone();
codex-rs/rmcp-client/tests/mcp_2026_discovery.rs
+1 -1
View File
@@ -230,7 +230,7 @@ async fn modern_discovery_does_not_follow_redirects_with_sensitive_headers() ->
}
#[tokio::test]
async fn legacy_mcp_requests_preserve_existing_redirect_behavior() -> anyhow::Result<()> {
async fn legacy_mcp_requests_follow_redirects_with_configured_headers() -> anyhow::Result<()> {
let redirect_target = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/forwarded"))
codex-rs/rmcp-client/tests/mcp_2026_oauth_discovery.rs
+5
View File
@@ -6,6 +6,7 @@ use codex_http_client::HttpClientFactory;
use codex_http_client::OutboundProxyPolicy;
use codex_rmcp_client::OAuthDiscoveryTimeout;
use codex_rmcp_client::StreamableHttpOAuthDiscovery;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::discover_streamable_http_oauth;
use pretty_assertions::assert_eq;
use rmcp::transport::auth::AuthError;
@@ -108,6 +109,7 @@ async fn discover_legacy_oauth_without_starting_an_mcp_session(
OutboundProxyPolicy::ReqwestDefault,
))),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await;
let routed_discovery = discover_streamable_http_oauth(
@@ -118,6 +120,7 @@ async fn discover_legacy_oauth_without_starting_an_mcp_session(
OutboundProxyPolicy::ReqwestDefault,
))),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await;
@@ -197,6 +200,7 @@ async fn oauth_discovery_does_not_invent_support_for_an_unauthenticated_legacy_s
OutboundProxyPolicy::ReqwestDefault,
))),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await?;
let local_discovery = discover_streamable_http_oauth(
@@ -207,6 +211,7 @@ async fn oauth_discovery_does_not_invent_support_for_an_unauthenticated_legacy_s
OutboundProxyPolicy::ReqwestDefault,
))),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await?;
codex-rs/rmcp-client/tests/streamable_http_oauth_startup.rs
+2
View File
@@ -12,6 +12,7 @@ use codex_rmcp_client::McpLoginRequirement;
use codex_rmcp_client::OAuthDiscoveryTimeout;
use codex_rmcp_client::RmcpClient;
use codex_rmcp_client::StoredOAuthTokens;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::WrappedOAuthTokenResponse;
use codex_rmcp_client::determine_streamable_http_auth_status;
use codex_rmcp_client::is_authentication_required_error;
@@ -297,6 +298,7 @@ async fn auth_status(server_url: &str) -> anyhow::Result<McpAuthState> {
AuthKeyringBackendKind::default(),
Environment::default_for_tests().get_http_client(),
OAuthDiscoveryTimeout::LOCAL,
StreamableHttpRedirectMode::Legacy,
)
.await
}
codex-rs/tools/src/lib.rs
+2
View File
@@ -38,6 +38,7 @@ pub use json_schema::JsonSchemaType;
pub use json_schema::parse_tool_input_schema;
pub use json_schema::parse_tool_input_schema_without_compaction;
pub use mcp_tool::mcp_call_tool_result_output_schema;
pub use mcp_tool::parse_agent_plugin_mcp_tool;
pub use mcp_tool::parse_mcp_tool;
pub use request_plugin_install::REQUEST_PLUGIN_INSTALL_APPROVAL_KIND_VALUE;
pub use request_plugin_install::REQUEST_PLUGIN_INSTALL_PERSIST_ALWAYS_VALUE;
@@ -56,6 +57,7 @@ pub use responses_api::LoadableToolSpec;
pub use responses_api::ResponsesApiNamespace;
pub use responses_api::ResponsesApiNamespaceTool;
pub use responses_api::ResponsesApiTool;
pub use responses_api::agent_plugin_mcp_tool_to_responses_api_tool;
pub use responses_api::coalesce_loadable_tool_specs;
pub use responses_api::default_namespace_description;
pub use responses_api::dynamic_tool_to_responses_api_tool;
codex-rs/tools/src/mcp_tool.rs
+24 -1
View File
@@ -1,9 +1,25 @@
use crate::ToolDefinition;
use crate::parse_tool_input_schema;
use codex_utils_string::take_bytes_at_char_boundary;
use serde_json::Value as JsonValue;
use serde_json::json;
const MAX_MCP_TOOL_DESCRIPTION_BYTES: usize = 1_000;
pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result<ToolDefinition, serde_json::Error> {
parse_mcp_tool_with_description_limit(tool, /*description_limit*/ None)
}
pub fn parse_agent_plugin_mcp_tool(
tool: &rmcp::model::Tool,
) -> Result<ToolDefinition, serde_json::Error> {
parse_mcp_tool_with_description_limit(tool, Some(MAX_MCP_TOOL_DESCRIPTION_BYTES))
}
fn parse_mcp_tool_with_description_limit(
tool: &rmcp::model::Tool,
description_limit: Option<usize>,
) -> Result<ToolDefinition, serde_json::Error> {
let mut serialized_input_schema = serde_json::Value::Object(tool.input_schema.as_ref().clone());
// OpenAI models mandate the "properties" field in the schema. Some MCP
@@ -27,7 +43,14 @@ pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result<ToolDefinition, serde_
Ok(ToolDefinition {
name: tool.name.to_string(),
description: tool.description.clone().map(Into::into).unwrap_or_default(),
description: tool
.description
.as_deref()
.map(|description| match description_limit {
Some(limit) => take_bytes_at_char_boundary(description, limit).to_string(),
None => description.to_string(),
})
.unwrap_or_default(),
input_schema,
output_schema: Some(mcp_call_tool_result_output_schema(
structured_content_schema,
codex-rs/tools/src/mcp_tool_tests.rs
+35
View File
@@ -1,4 +1,5 @@
use super::mcp_call_tool_result_output_schema;
use super::parse_agent_plugin_mcp_tool;
use super::parse_mcp_tool;
use crate::JsonSchema;
use crate::ToolDefinition;
@@ -39,6 +40,40 @@ fn parse_mcp_tool_inserts_empty_properties() {
);
}
#[test]
fn agent_plugin_mcp_tool_bounds_model_visible_description() {
let description = format!("{}é", "a".repeat(super::MAX_MCP_TOOL_DESCRIPTION_BYTES - 1));
let tool = mcp_tool(
"bounded_description",
&description,
serde_json::json!({"type": "object"}),
);
let parsed = parse_agent_plugin_mcp_tool(&tool).expect("parse Agent Plugin MCP tool");
assert_eq!(
parsed.description.len(),
super::MAX_MCP_TOOL_DESCRIPTION_BYTES - 1
);
}
#[test]
fn legacy_mcp_tool_preserves_long_description() {
let description = "a".repeat(super::MAX_MCP_TOOL_DESCRIPTION_BYTES + 100);
let tool = mcp_tool(
"legacy_description",
&description,
serde_json::json!({"type": "object"}),
);
assert_eq!(
parse_mcp_tool(&tool)
.expect("parse legacy MCP tool")
.description,
description
);
}
#[test]
fn parse_mcp_tool_preserves_top_level_output_schema() {
let mut tool = mcp_tool(
codex-rs/tools/src/responses_api.rs
+20
View File
@@ -1,6 +1,7 @@
use crate::JsonSchema;
use crate::ToolDefinition;
use crate::ToolName;
use crate::parse_agent_plugin_mcp_tool;
use crate::parse_dynamic_tool;
use crate::parse_mcp_tool;
use codex_protocol::dynamic_tools::DynamicToolFunctionSpec;
@@ -8,6 +9,8 @@ use serde::Deserialize;
use serde::Serialize;
use serde_json::Value;
const MAX_SERIALIZED_MCP_TOOL_BYTES: usize = 8_000;
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
pub struct FreeformTool {
pub name: String,
@@ -118,6 +121,23 @@ pub fn mcp_tool_to_responses_api_tool(
))
}
pub fn agent_plugin_mcp_tool_to_responses_api_tool(
tool_name: &ToolName,
tool: &rmcp::model::Tool,
) -> Result<ResponsesApiTool, serde_json::Error> {
let mut tool = tool_definition_to_responses_api_tool(
parse_agent_plugin_mcp_tool(tool)?.renamed(tool_name.name.clone()),
);
if serde_json::to_vec(&tool)?.len() > MAX_SERIALIZED_MCP_TOOL_BYTES {
tool.parameters = JsonSchema::object(
Default::default(),
/*required*/ None,
Some(true.into()),
);
}
Ok(tool)
}
pub fn mcp_tool_to_deferred_responses_api_tool(
tool_name: &ToolName,
tool: &rmcp::model::Tool,
codex-rs/tools/src/responses_api_tests.rs
+41
View File
@@ -3,6 +3,7 @@ use super::LoadableToolSpec;
use super::ResponsesApiNamespace;
use super::ResponsesApiNamespaceTool;
use super::ResponsesApiTool;
use super::agent_plugin_mcp_tool_to_responses_api_tool;
use super::dynamic_tool_to_responses_api_tool;
use super::mcp_tool_to_deferred_responses_api_tool;
use super::tool_definition_to_responses_api_tool;
@@ -153,6 +154,46 @@ fn mcp_tool_to_deferred_responses_api_tool_sets_defer_loading() {
);
}
#[test]
fn agent_plugin_mcp_tool_uses_fallback_for_oversized_schema() {
let properties: serde_json::Map<String, serde_json::Value> = (0..1_024)
.map(|index| (format!("property_{index}"), json!({"type": "string"})))
.collect();
let tool = rmcp::model::Tool::new(
"oversized",
"Large schema",
std::sync::Arc::new(rmcp::model::object(json!({
"type": "object",
"properties": properties,
}))),
);
assert_eq!(
agent_plugin_mcp_tool_to_responses_api_tool(&ToolName::from("oversized"), &tool)
.expect("Agent Plugin MCP tool should use a fallback schema")
.parameters,
JsonSchema::object(BTreeMap::new(), /*required*/ None, Some(true.into()))
);
}
#[test]
fn legacy_mcp_tool_accepts_oversized_schema() {
let properties: serde_json::Map<String, serde_json::Value> = (0..1_024)
.map(|index| (format!("property_{index}"), json!({"type": "string"})))
.collect();
let tool = rmcp::model::Tool::new(
"oversized",
"Large legacy schema",
std::sync::Arc::new(rmcp::model::object(json!({
"type": "object",
"properties": properties,
}))),
);
super::mcp_tool_to_responses_api_tool(&ToolName::from("oversized"), &tool)
.expect("legacy MCP conversion must preserve existing acceptance");
}
#[test]
fn loadable_tool_spec_namespace_serializes_with_deferred_child_tools() {
let namespace = LoadableToolSpec::Namespace(ResponsesApiNamespace {
codex-rs/tui/src/app/plugin_mentions.rs
+3
View File
@@ -50,6 +50,7 @@ fn plugin_mention_from_summary(
Some(PluginCapabilitySummary {
config_name: plugin.id.clone(),
display_name: plugin_mention_display_name(&plugin),
plugin_namespace: Some(plugin.name.clone()),
description: plugin_mention_description(marketplace_name, &plugin),
has_skills: false,
mcp_server_names: Vec::new(),
@@ -129,6 +130,7 @@ mod tests {
PluginCapabilitySummary {
config_name: "active@server-marketplace".to_string(),
display_name: "active".to_string(),
plugin_namespace: Some("active".to_string()),
description: Some("server-marketplace".to_string()),
has_skills: false,
mcp_server_names: Vec::new(),
@@ -137,6 +139,7 @@ mod tests {
PluginCapabilitySummary {
config_name: "active-shared@server-marketplace".to_string(),
display_name: "active-shared".to_string(),
plugin_namespace: Some("active-shared".to_string()),
description: Some("server-marketplace".to_string()),
has_skills: false,
mcp_server_names: Vec::new(),
codex-rs/tui/src/bottom_pane/chat_composer.rs
+7
View File
@@ -6712,6 +6712,7 @@ mod tests {
composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "Sample Plugin".to_string(),
plugin_namespace: None,
description: None,
has_skills: true,
mcp_server_names: vec!["sample".to_string()],
@@ -6764,6 +6765,7 @@ mod tests {
PluginCapabilitySummary {
config_name: format!("{name}@test"),
display_name: name.to_string(),
plugin_namespace: None,
description: Some(description.to_string()),
has_skills: false,
mcp_server_names: vec![name.to_string()],
@@ -7436,6 +7438,7 @@ mod tests {
composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary {
config_name: "google-calendar@debug".to_string(),
display_name: "Google Calendar".to_string(),
plugin_namespace: None,
description: Some(
"Connect Google Calendar for scheduling, availability, and event management."
.to_string(),
@@ -7488,6 +7491,7 @@ mod tests {
composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "Sample Plugin".to_string(),
plugin_namespace: None,
description: Some(
"Plugin that includes the Figma MCP server and Skills for common workflows"
.to_string(),
@@ -7513,6 +7517,7 @@ mod tests {
composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "Sample Plugin".to_string(),
plugin_namespace: None,
description: Some("Plugin with skills and an MCP server".to_string()),
has_skills: true,
mcp_server_names: vec!["sample".to_string()],
@@ -7620,6 +7625,7 @@ mod tests {
composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary {
config_name: "google-calendar@debug".to_string(),
display_name: "Google Calendar".to_string(),
plugin_namespace: None,
description: Some(
"Connect Google Calendar for scheduling, availability, and event management."
.to_string(),
@@ -8188,6 +8194,7 @@ mod tests {
composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "sample".to_string(),
plugin_namespace: None,
description: None,
has_skills: true,
mcp_server_names: vec!["sample".to_string()],
codex-rs/tui/src/chatwidget/tests/plan_mode.rs
+1
View File
@@ -1310,6 +1310,7 @@ async fn submit_user_message_emits_structured_plugin_mentions_from_bindings() {
.set_plugin_mentions(Some(vec![codex_plugin::PluginCapabilitySummary {
config_name: "sample@test".to_string(),
display_name: "Sample Plugin".to_string(),
plugin_namespace: None,
description: None,
has_skills: true,
mcp_server_names: Vec::new(),