diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 7f09216eab3d..9a89002e1e79 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1097,6 +1097,9 @@ pub enum SkillScope { Repo, System, Admin, + /// Served by a connected MCP server per the Skills-over-MCP extension + /// (`io.modelcontextprotocol/skills`). + Mcp, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -1110,6 +1113,12 @@ pub struct SkillMetadata { pub short_description: Option, pub path: PathBuf, pub scope: SkillScope, + #[ts(optional)] + #[serde(default, skip_serializing_if = "Option::is_none")] + pub uri: Option, + #[ts(optional)] + #[serde(default, skip_serializing_if = "Option::is_none")] + pub server_name: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -1137,6 +1146,8 @@ impl From for SkillMetadata { short_description: value.short_description, path: value.path, scope: value.scope.into(), + uri: value.uri, + server_name: value.server_name, } } } @@ -1148,6 +1159,7 @@ impl From for SkillScope { CoreSkillScope::Repo => Self::Repo, CoreSkillScope::System => Self::System, CoreSkillScope::Admin => Self::Admin, + CoreSkillScope::Mcp => Self::Mcp, } } } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index d1804801d589..0a8029d50a5d 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -3324,6 +3324,8 @@ fn skills_to_info( short_description: skill.short_description.clone(), path: skill.path.clone(), scope: skill.scope.into(), + uri: skill.uri.clone(), + server_name: skill.server_name.clone(), }) .collect() } diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index ef872e5972f7..84e9067547c3 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -245,6 +245,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }; servers.insert(name.clone(), new_entry); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 5fb86e8718eb..c403752b29f0 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -128,6 +128,7 @@ use crate::skills::SkillInjections; use crate::skills::SkillMetadata; use crate::skills::SkillsManager; use crate::skills::build_skill_injections; +use crate::skills::load_mcp_skill_manifests; use crate::state::ActiveTurn; use crate::state::SessionServices; use crate::state::SessionState; @@ -204,6 +205,49 @@ fn maybe_push_chat_wire_api_deprecation( }); } +/// Bring up the session-scoped MCP connection manager ahead of skill +/// discovery and turn-1 tool use, and return its handle, cancellation +/// token, and the event receiver. `initialize()` emits its own events, +/// but `SessionConfigured` must be the first event callers see (see +/// `conversation_manager::finalize_spawn`), so MCP events flow through a +/// dedicated channel that `Session::new` later fans into the main +/// event stream. +async fn spawn_mcp_manager( + config: &Config, +) -> ( + Arc>, + CancellationToken, + Receiver, +) { + let manager: Arc> = + Arc::new(RwLock::new(McpConnectionManager::default())); + let cancellation_token = CancellationToken::new(); + let (tx_mcp, rx_mcp) = async_channel::unbounded::(); + let auth_statuses = compute_auth_statuses( + config.mcp_servers.iter(), + config.mcp_oauth_credentials_store_mode, + ) + .await; + let sandbox_state = crate::mcp_connection_manager::SandboxState { + sandbox_policy: config.sandbox_policy.get().clone(), + codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), + sandbox_cwd: config.cwd.clone(), + }; + manager + .write() + .await + .initialize( + config.mcp_servers.clone(), + config.mcp_oauth_credentials_store_mode, + auth_statuses, + tx_mcp, + cancellation_token.clone(), + sandbox_state, + ) + .await; + (manager, cancellation_token, rx_mcp) +} + impl Codex { /// Spawn a new [`Codex`] and initialize the session. pub async fn spawn( @@ -217,20 +261,30 @@ impl Codex { let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); - let loaded_skills = config + let mut loaded_skills = config .features .enabled(Feature::Skills) .then(|| skills_manager.skills_for_cwd(&config.cwd)); - if let Some(outcome) = &loaded_skills { - for err in &outcome.errors { - error!( - "failed to load skill {}: {}", - err.path.display(), - err.message - ); - } - } + let (mcp_connection_manager, mcp_startup_cancellation_token, rx_mcp) = + spawn_mcp_manager(&config).await; + + let session_mcp_skills: Arc> = + if let Some(outcome) = loaded_skills.as_mut() { + let guard = mcp_connection_manager.read().await; + let added = merge_mcp_skills(outcome, &guard, &config).await; + drop(guard); + for err in &outcome.errors { + error!( + "failed to load skill {}: {}", + err.path.display(), + err.message + ); + } + Arc::new(added) + } else { + Arc::new(Vec::new()) + }; let user_instructions = get_user_instructions( &config, @@ -280,6 +334,10 @@ impl Codex { conversation_history, session_source_clone, skills_manager, + mcp_connection_manager, + mcp_startup_cancellation_token, + session_mcp_skills, + rx_mcp, ) .await .map_err(|e| { @@ -545,6 +603,10 @@ impl Session { initial_history: InitialHistory, session_source: SessionSource, skills_manager: Arc, + mcp_connection_manager: Arc>, + mcp_startup_cancellation_token: CancellationToken, + session_mcp_skills: Arc>, + rx_mcp: async_channel::Receiver, ) -> anyhow::Result> { debug!( "Configuring session: model={}; provider={:?}", @@ -583,14 +645,12 @@ impl Session { let rollout_fut = RolloutRecorder::new(&config, rollout_params); let history_meta_fut = crate::message_history::history_metadata(&config); - let auth_statuses_fut = compute_auth_statuses( - config.mcp_servers.iter(), - config.mcp_oauth_credentials_store_mode, - ); - // Join all independent futures. - let (rollout_recorder, (history_log_id, history_entry_count), auth_statuses) = - tokio::join!(rollout_fut, history_meta_fut, auth_statuses_fut); + // Join the independent startup futures. The MCP connection manager + // has already been initialized by `Codex::spawn` and is passed in — + // see the caller for why this is handled outside `Session::new`. + let (rollout_recorder, (history_log_id, history_entry_count)) = + tokio::join!(rollout_fut, history_meta_fut); let rollout_recorder = rollout_recorder.map_err(|e| { error!("failed to initialize rollout recorder: {e:#}"); @@ -653,8 +713,8 @@ impl Session { let state = SessionState::new(session_configuration.clone()); let services = SessionServices { - mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), - mcp_startup_cancellation_token: CancellationToken::new(), + mcp_connection_manager, + mcp_startup_cancellation_token, unified_exec_manager: UnifiedExecSessionManager::default(), notifier: UserNotifier::new(config.notify.clone()), rollout: Mutex::new(Some(rollout_recorder)), @@ -666,6 +726,7 @@ impl Session { models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), skills_manager, + session_mcp_skills, }; let sess = Arc::new(Session { @@ -702,26 +763,21 @@ impl Session { sess.send_event_raw(event).await; } - // Construct sandbox_state before initialize() so it can be sent to each - // MCP server immediately after it becomes ready (avoiding blocking). - let sandbox_state = SandboxState { - sandbox_policy: session_configuration.sandbox_policy.get().clone(), - codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), - sandbox_cwd: session_configuration.cwd.clone(), - }; - sess.services - .mcp_connection_manager - .write() - .await - .initialize( - config.mcp_servers.clone(), - config.mcp_oauth_credentials_store_mode, - auth_statuses.clone(), - tx_event.clone(), - sess.services.mcp_startup_cancellation_token.clone(), - sandbox_state, - ) - .await; + // Flush MCP startup / elicitation events that `Codex::spawn` routed + // through a dedicated channel so they couldn't arrive before + // `SessionConfigured`. The forwarder runs for the session lifetime + // because the MCP connection manager also reuses this channel for + // elicitation events produced mid-session. + { + let tx_forward = tx_event.clone(); + tokio::spawn(async move { + while let Ok(event) = rx_mcp.recv().await { + if tx_forward.send(event).await.is_err() { + break; + } + } + }); + } // record_initial_history can emit events. We record only after the SessionConfiguredEvent is emitted. sess.record_initial_history(initial_history).await; @@ -1961,9 +2017,13 @@ mod handlers { }; let skills = if sess.enabled(Feature::Skills) { let skills_manager = &sess.services.skills_manager; + let session_mcp_skills = sess.services.session_mcp_skills.as_ref(); cwds.into_iter() .map(|cwd| { - let outcome = skills_manager.skills_for_cwd_with_options(&cwd, force_reload); + let mut outcome = + skills_manager.skills_for_cwd_with_options(&cwd, force_reload); + // Filesystem wins on name collision. + outcome.add_mcp_skills(session_mcp_skills.iter().cloned()); let errors = super::errors_to_info(&outcome.errors); let skills = super::skills_to_info(&outcome.skills); SkillsListEntry { @@ -2174,6 +2234,8 @@ fn skills_to_info(skills: &[SkillMetadata]) -> Vec { short_description: skill.short_description.clone(), path: skill.path.clone(), scope: skill.scope, + uri: skill.uri.clone(), + server_name: skill.server_name.clone(), }) .collect() } @@ -2188,6 +2250,47 @@ fn errors_to_info(errors: &[SkillError]) -> Vec { .collect() } +/// Fetch Skills-over-MCP manifests from every connected MCP server whose +/// config opts in, and merge them into `outcome`. Filesystem skills win on +/// name collision (matching the precedence in `skill_roots_for_cwd`). +/// +/// The caller is expected to have already initialized `manager` against +/// `config.mcp_servers` — we reuse the session's live connections rather +/// than spawning throwaway children. +/// +/// Returns the MCP-origin skills that survived masking, so the session can +/// make them visible per-turn for `$Mention` resolution. +async fn merge_mcp_skills( + outcome: &mut crate::skills::SkillLoadOutcome, + manager: &McpConnectionManager, + config: &Config, +) -> Vec { + let any_eligible = config + .mcp_servers + .iter() + .any(|(_, server_cfg)| server_cfg.enabled && server_cfg.mcp_skills); + if !any_eligible { + return Vec::new(); + } + + // `load_mcp_skill_manifests` re-filters by `enabled && mcp_skills`, so + // we pass the raw server map and let it be the single point of truth. + let mcp_outcome = load_mcp_skill_manifests(manager, &config.mcp_servers).await; + outcome.errors.extend(mcp_outcome.errors); + + let skills = mcp_outcome.skills; + let before = outcome.skills.len(); + let masked = outcome.add_mcp_skills(skills); + if !masked.is_empty() { + warn!( + "skills: {} MCP skill(s) masked by filesystem skills with the same name: {}", + masked.len(), + masked.join(", ") + ); + } + outcome.skills[before..].to_vec() +} + /// Takes a user message as input and runs a loop where, at each turn, the model /// replies with either: /// @@ -2227,9 +2330,15 @@ pub(crate) async fn run_task( sess.send_event(&turn_context, event).await; let skills_outcome = sess.enabled(Feature::Skills).then(|| { - sess.services + let mut outcome = sess + .services .skills_manager - .skills_for_cwd(&turn_context.cwd) + .skills_for_cwd(&turn_context.cwd); + // MCP-served skills were discovered at session start and cached on + // the session; merge them in with filesystem skills winning on name + // collision so `$Mention` can resolve URI-backed skills too. + outcome.add_mcp_skills(sess.services.session_mcp_skills.iter().cloned()); + outcome }); let SkillInjections { @@ -3143,6 +3252,7 @@ mod tests { models_manager, tool_approvals: Mutex::new(ApprovalStore::default()), skills_manager, + session_mcp_skills: Arc::new(Vec::new()), }; let turn_context = Session::make_turn_context( @@ -3230,6 +3340,7 @@ mod tests { models_manager, tool_approvals: Mutex::new(ApprovalStore::default()), skills_manager, + session_mcp_skills: Arc::new(Vec::new()), }; let turn_context = Arc::new(Session::make_turn_context( diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index a24c09e36b75..efb86bda7363 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -1127,6 +1127,7 @@ gpt-5 = "gpt-5.1" tool_timeout_sec: None, enabled_tools: Some(vec!["one".to_string(), "two".to_string()]), disabled_tools: None, + mcp_skills: true, }, ); @@ -1148,6 +1149,7 @@ gpt-5 = "gpt-5.1" tool_timeout_sec: None, enabled_tools: None, disabled_tools: Some(vec!["forbidden".to_string()]), + mcp_skills: true, }, ); @@ -1212,6 +1214,7 @@ foo = { command = "cmd" } tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); @@ -1255,6 +1258,7 @@ foo = { command = "cmd" } # keep me tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); @@ -1297,6 +1301,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); @@ -1340,6 +1345,7 @@ foo = { command = "cmd" } tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 53864851ae03..ec97c916a2b9 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2111,6 +2111,7 @@ trust_level = "trusted" tool_timeout_sec: Some(Duration::from_secs(5)), enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); @@ -2263,6 +2264,7 @@ bearer_token = "secret" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, )]); @@ -2331,6 +2333,7 @@ ZIG_VAR = "3" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, )]); @@ -2379,6 +2382,7 @@ ZIG_VAR = "3" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, )]); @@ -2425,6 +2429,7 @@ ZIG_VAR = "3" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, )]); @@ -2487,6 +2492,7 @@ startup_timeout_sec = 2.0 tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, )]); apply_blocking( @@ -2561,6 +2567,7 @@ X-Auth = "DOCS_AUTH" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, )]); @@ -2588,6 +2595,7 @@ X-Auth = "DOCS_AUTH" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); apply_blocking( @@ -2653,6 +2661,7 @@ url = "https://example.com/mcp" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ), ( @@ -2670,6 +2679,7 @@ url = "https://example.com/mcp" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ), ]); @@ -2750,6 +2760,7 @@ url = "https://example.com/mcp" tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, )]); @@ -2792,6 +2803,7 @@ url = "https://example.com/mcp" tool_timeout_sec: None, enabled_tools: Some(vec!["allowed".to_string()]), disabled_tools: Some(vec!["blocked".to_string()]), + mcp_skills: true, }, )]); diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 3aa72d5ce505..8d0a6c0a33b8 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -45,6 +45,13 @@ pub struct McpServerConfig { /// Explicit deny-list of tools. These tools will be removed after applying `enabled_tools`. #[serde(default, skip_serializing_if = "Option::is_none")] pub disabled_tools: Option>, + + /// When `true`, Codex attempts to discover Agent Skills served by this + /// server via the Skills-over-MCP extension (`skill://index.json`). + /// Defaults to `true`. Set `mcp_skills = false` to suppress skill + /// discovery for a server while keeping its tools available. + #[serde(default = "default_mcp_skills")] + pub mcp_skills: bool, } impl<'de> Deserialize<'de> for McpServerConfig { @@ -86,6 +93,8 @@ impl<'de> Deserialize<'de> for McpServerConfig { enabled_tools: Option>, #[serde(default)] disabled_tools: Option>, + #[serde(default)] + mcp_skills: Option, } let mut raw = RawMcpServerConfig::deserialize(deserializer)?; @@ -102,6 +111,7 @@ impl<'de> Deserialize<'de> for McpServerConfig { let enabled = raw.enabled.unwrap_or_else(default_enabled); let enabled_tools = raw.enabled_tools.clone(); let disabled_tools = raw.disabled_tools.clone(); + let mcp_skills = raw.mcp_skills.unwrap_or_else(default_mcp_skills); fn throw_if_set(transport: &str, field: &str, value: Option<&T>) -> Result<(), E> where @@ -155,6 +165,7 @@ impl<'de> Deserialize<'de> for McpServerConfig { enabled, enabled_tools, disabled_tools, + mcp_skills, }) } } @@ -163,6 +174,10 @@ const fn default_enabled() -> bool { true } +const fn default_mcp_skills() -> bool { + true +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] #[serde(untagged, deny_unknown_fields, rename_all = "snake_case")] pub enum McpServerTransportConfig { @@ -696,6 +711,20 @@ mod tests { assert!(cfg.enabled); assert!(cfg.enabled_tools.is_none()); assert!(cfg.disabled_tools.is_none()); + // Skills-over-MCP discovery defaults on; opt-out is per-server. + assert!(cfg.mcp_skills); + } + + #[test] + fn mcp_skills_can_be_disabled_per_server() { + let cfg: McpServerConfig = toml::from_str( + r#" + command = "echo" + mcp_skills = false + "#, + ) + .expect("should deserialize command config with mcp_skills"); + assert!(!cfg.mcp_skills); } #[test] diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 6c0b48b1bd5c..d9e731bb9490 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -804,6 +804,30 @@ async fn start_server_task( .and_then(|exp| exp.get(MCP_SANDBOX_STATE_CAPABILITY)) .is_some(); + // Observability for the Skills-over-MCP extension. Discovery is not + // gated on this per the SEP; we just log so operators can confirm the + // server self-advertises. Per SEP-2133 extensions negotiation the + // declaration lives under `capabilities.extensions`; check + // `capabilities.experimental` as a fallback for older servers. + let skill_cap = initialize_result + .capabilities + .extensions + .as_ref() + .and_then(|ext| ext.get(crate::skills::SKILLS_EXTENSION_ID)) + .or_else(|| { + initialize_result + .capabilities + .experimental + .as_ref() + .and_then(|exp| exp.get(crate::skills::SKILLS_EXTENSION_ID)) + }); + if skill_cap.is_some() { + tracing::info!( + "skills: server `{server_name}` declares `{}` capability", + crate::skills::SKILLS_EXTENSION_ID + ); + } + let managed = ManagedClient { client: Arc::clone(&client), tools, @@ -1118,6 +1142,7 @@ mod tests { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, auth_status: McpAuthStatus::Unsupported, }; @@ -1162,6 +1187,7 @@ mod tests { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, auth_status: McpAuthStatus::Unsupported, }; diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core/src/skills/injection.rs index a143fce1f22e..594b936bd94f 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core/src/skills/injection.rs @@ -36,6 +36,29 @@ pub(crate) async fn build_skill_injections( }; for skill in mentioned_skills { + if let (Some(uri), Some(server)) = + (skill.uri.as_deref(), skill.server_name.as_deref()) + { + // MCP skills can't be read from disk; emit a model-facing + // instruction naming the exact `read_mcp_resource` call so + // activation is deterministic rather than heuristic. + result.warnings.push(format!( + "Skill {} is served by MCP server {server}; the model will load it via read_mcp_resource({uri}).", + skill.name + )); + let instructions = format!( + "The user explicitly invoked the `{name}` skill, which is served by MCP server `{server}`. \ + Before acting on the user's request, call read_mcp_resource(server=\"{server}\", uri=\"{uri}\") \ + to load the SKILL.md body. Use the server and URI exactly as given; do not rewrite them.", + name = skill.name, + ); + result.items.push(ResponseItem::from(SkillInstructions { + name: skill.name, + path: uri.to_string(), + contents: instructions, + })); + continue; + } match fs::read_to_string(&skill.path).await { Ok(contents) => { result.items.push(ResponseItem::from(SkillInstructions { diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index bce13fbb0571..0d215491dc82 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -234,8 +234,44 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil fn parse_skill_file(path: &Path, scope: SkillScope) -> Result { let contents = fs::read_to_string(path).map_err(SkillParseError::Read)?; + let parsed = parse_skill_frontmatter_text_inner(&contents)?; + let resolved_path = normalize_path(path).unwrap_or_else(|_| path.to_path_buf()); + + Ok(SkillMetadata { + name: parsed.name, + description: parsed.description, + short_description: parsed.short_description, + path: resolved_path, + scope, + uri: None, + server_name: None, + }) +} + +#[derive(Debug, Clone)] +pub(crate) struct ParsedFrontmatter { + pub(crate) name: String, + pub(crate) description: String, + pub(crate) short_description: Option, +} + +/// Parses the YAML frontmatter at the top of a SKILL.md document and +/// returns the validated name/description/short-description fields. +/// Shared between filesystem loading and MCP-served skill loading so both +/// paths enforce the same Agent Skills schema. Errors stringify through +/// `Display` at the boundary — callers uniformly wrap the message into a +/// `SkillError`, so leaking the enum across modules adds API surface +/// without helping any caller. +pub(crate) fn parse_skill_frontmatter_text( + contents: &str, +) -> Result { + parse_skill_frontmatter_text_inner(contents).map_err(|e| e.to_string()) +} - let frontmatter = extract_frontmatter(&contents).ok_or(SkillParseError::MissingFrontmatter)?; +fn parse_skill_frontmatter_text_inner( + contents: &str, +) -> Result { + let frontmatter = extract_frontmatter(contents).ok_or(SkillParseError::MissingFrontmatter)?; let parsed: SkillFrontmatter = serde_yaml::from_str(&frontmatter).map_err(SkillParseError::InvalidYaml)?; @@ -259,14 +295,10 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result, + pub(crate) errors: Vec, +} + +/// Minimum-viable schema for `skill://index.json`. Mirrors the Agent Skills +/// well-known URI index shape, with `url` holding a full MCP resource URI. +#[derive(Debug, Deserialize)] +struct IndexDocument { + #[serde(default)] + skills: Vec, +} + +#[derive(Debug, Deserialize)] +struct IndexEntry { + #[serde(default)] + r#type: Option, + #[serde(default)] + name: Option, + #[serde(default)] + url: Option, +} + +/// Fetch skill manifests from every server in `mcp_configs` whose +/// configuration opts into skill discovery. The returned outcome aggregates +/// successes and per-skill errors across all servers. Servers and their +/// per-skill reads run concurrently. +pub(crate) async fn load_mcp_skill_manifests( + conn: &McpConnectionManager, + mcp_configs: &HashMap, +) -> McpSkillsOutcome { + let server_futs = mcp_configs + .iter() + .filter(|(_, cfg)| cfg.enabled && cfg.mcp_skills) + .map(|(name, _)| load_from_server(conn, name.as_str())); + + let mut outcome = McpSkillsOutcome::default(); + for server_outcome in futures::future::join_all(server_futs).await { + outcome.skills.extend(server_outcome.skills); + outcome.errors.extend(server_outcome.errors); + } + // Stable ordering across sessions: HashMap iteration above is + // nondeterministic, and filesystem skills are already sorted by + // `load_skills_from_roots`. Match that behavior so `user_instructions` + // diffs cleanly across runs. + outcome.skills.sort_by(|a, b| { + a.server_name + .cmp(&b.server_name) + .then_with(|| a.name.cmp(&b.name)) + }); + outcome +} + +async fn load_from_server(conn: &McpConnectionManager, server_name: &str) -> McpSkillsOutcome { + let index_params = ReadResourceRequestParams { + uri: SKILL_INDEX_URI.to_owned(), + }; + let index_result = match timeout( + SKILL_READ_TIMEOUT, + conn.read_resource(server_name, index_params), + ) + .await + { + Ok(Ok(result)) => result, + Ok(Err(err)) => { + // Per SEP: "Hosts MUST NOT treat an absent or empty index as + // proof that a server has no skills." So this is informational. + debug!("skills: server `{server_name}` did not return {SKILL_INDEX_URI}: {err:#}"); + return McpSkillsOutcome::default(); + } + Err(_) => { + warn!( + "skills: server `{server_name}` timed out after {:?} reading {SKILL_INDEX_URI}", + SKILL_READ_TIMEOUT + ); + return McpSkillsOutcome::default(); + } + }; + + let Some(index_text) = first_text(&index_result) else { + warn!("skills: server `{server_name}` returned non-text contents for {SKILL_INDEX_URI}"); + return McpSkillsOutcome::default(); + }; + + let index: IndexDocument = match serde_json::from_str(index_text) { + Ok(doc) => doc, + Err(err) => { + warn!("skills: server `{server_name}` returned malformed {SKILL_INDEX_URI}: {err:#}"); + return McpSkillsOutcome::default(); + } + }; + + info!( + "skills: server `{server_name}` published index with {} entries", + index.skills.len() + ); + + let entry_futs = index + .skills + .into_iter() + .map(|entry| load_entry(conn, server_name, entry)); + + let mut outcome = McpSkillsOutcome::default(); + for entry_result in futures::future::join_all(entry_futs).await { + match entry_result { + EntryOutcome::Skill(skill) => outcome.skills.push(skill), + EntryOutcome::Error(err) => outcome.errors.push(err), + EntryOutcome::Skipped => {} + } + } + outcome +} + +enum EntryOutcome { + Skill(SkillMetadata), + Error(SkillError), + Skipped, +} + +async fn load_entry( + conn: &McpConnectionManager, + server_name: &str, + entry: IndexEntry, +) -> EntryOutcome { + match entry.r#type.as_deref().unwrap_or("") { + "skill-md" => {} + "mcp-resource-template" => { + debug!( + "skills: server `{server_name}` advertised resource template entry (deferred): {entry:?}" + ); + return EntryOutcome::Skipped; + } + other => { + debug!( + "skills: server `{server_name}` entry has unrecognized type `{other}`; skipping" + ); + return EntryOutcome::Skipped; + } + } + + let Some(uri) = entry.url.clone() else { + return EntryOutcome::Error(SkillError { + path: std::path::PathBuf::from(format!("mcp://{server_name}/")), + message: "skill-md index entry missing required `url` field".to_owned(), + }); + }; + + if let Err(reason) = validate_entry_uri(&uri) { + return EntryOutcome::Error(SkillError { + path: std::path::PathBuf::from(format!("mcp://{server_name}/")), + message: format!("`{server_name}` advertised invalid skill-md url: {reason}"), + }); + } + + let read_params = ReadResourceRequestParams { uri: uri.clone() }; + let read_result = match timeout( + SKILL_READ_TIMEOUT, + conn.read_resource(server_name, read_params), + ) + .await + { + Ok(Ok(result)) => result, + Ok(Err(err)) => { + return EntryOutcome::Error(SkillError { + path: std::path::PathBuf::from(&uri), + message: format!("resources/read failed for `{server_name}` ({uri}): {err:#}"), + }); + } + Err(_) => { + return EntryOutcome::Error(SkillError { + path: std::path::PathBuf::from(&uri), + message: format!( + "resources/read timed out after {SKILL_READ_TIMEOUT:?} for `{server_name}` ({uri})" + ), + }); + } + }; + + let Some(body) = first_text(&read_result) else { + return EntryOutcome::Error(SkillError { + path: std::path::PathBuf::from(&uri), + message: format!("`{server_name}` returned non-text content for `{uri}`"), + }); + }; + + match parse_skill_frontmatter_text(body) { + Ok(parsed) => { + // The SEP requires the SKILL.md path's final segment to equal the + // frontmatter `name`; the index entry `name` is advisory. We log + // mismatches so skill authors notice but still load the skill. + if let Some(entry_name) = entry.name.as_deref() + && entry_name != parsed.name + { + warn!( + "skills: `{server_name}` index entry name `{entry_name}` differs from SKILL.md frontmatter `{}`", + parsed.name + ); + } + + // The SKILL.md frontmatter `description` is the authoritative, + // length-validated value. The index entry's `description` is + // advisory only (like its `name`) and is deliberately NOT used — + // preferring it would bypass `MAX_DESCRIPTION_LEN` validation and + // admit unbounded server-controlled text into `user_instructions`. + EntryOutcome::Skill(SkillMetadata { + name: parsed.name, + description: parsed.description, + short_description: parsed.short_description, + path: std::path::PathBuf::from(&uri), + scope: SkillScope::Mcp, + uri: Some(uri), + server_name: Some(server_name.to_owned()), + }) + } + Err(message) => EntryOutcome::Error(SkillError { + path: std::path::PathBuf::from(&uri), + message, + }), + } +} + +/// Validate a server-supplied skill-md URL before we echo it into +/// `user_instructions` or hand it to `resources/read`. Keeps us defensive +/// against untrusted MCP servers without being strict about URI schemes: +/// the SEP allows domain-native schemes (e.g. `github://`), so we only +/// enforce a plausible `scheme:` prefix, bounded length, and no control +/// characters. +fn validate_entry_uri(uri: &str) -> Result<(), String> { + if uri.is_empty() { + return Err("url is empty".to_owned()); + } + if uri.len() > MAX_URI_LEN { + return Err(format!( + "url is {} bytes; limit is {MAX_URI_LEN}", + uri.len() + )); + } + if uri.chars().any(char::is_control) { + return Err("url contains control characters".to_owned()); + } + let Some(colon) = uri.find(':') else { + return Err("url is missing a scheme (expected e.g. `skill://...`)".to_owned()); + }; + let scheme = &uri[..colon]; + if scheme.is_empty() { + return Err("url scheme is empty".to_owned()); + } + let mut scheme_chars = scheme.chars(); + let first_ok = scheme_chars.next().is_some_and(|c| c.is_ascii_alphabetic()); + let rest_ok = scheme_chars + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')); + if !first_ok || !rest_ok { + return Err(format!("url scheme `{scheme}` is not a valid URI scheme")); + } + Ok(()) +} + +fn first_text(result: &ReadResourceResult) -> Option<&str> { + result.contents.iter().find_map(|content| match content { + ReadResourceResultContents::TextResourceContents(text) => Some(text.text.as_str()), + ReadResourceResultContents::BlobResourceContents(_) => None, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn index_document_parses_canonical_example() { + let src = r#"{ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "pull-requests", + "type": "skill-md", + "description": "Review GitHub PRs", + "url": "skill://pull-requests/SKILL.md" + }, + { + "type": "mcp-resource-template", + "description": "Per-product docs", + "url": "skill://docs/{product}/SKILL.md" + } + ] + }"#; + let doc: IndexDocument = serde_json::from_str(src).expect("parses"); + assert_eq!(doc.skills.len(), 2); + assert_eq!(doc.skills[0].r#type.as_deref(), Some("skill-md")); + assert_eq!(doc.skills[0].name.as_deref(), Some("pull-requests")); + assert_eq!( + doc.skills[0].url.as_deref(), + Some("skill://pull-requests/SKILL.md") + ); + assert_eq!( + doc.skills[1].r#type.as_deref(), + Some("mcp-resource-template") + ); + } + + #[test] + fn index_document_tolerates_unknown_fields() { + let src = r#"{ + "$schema": "x", + "extra": 42, + "skills": [ + { + "type": "skill-md", + "name": "pr", + "description": "d", + "url": "skill://pr/SKILL.md", + "unknown_field": true + } + ] + }"#; + let doc: IndexDocument = serde_json::from_str(src).expect("parses"); + assert_eq!(doc.skills.len(), 1); + } + + #[test] + fn index_document_tolerates_empty_skills() { + let src = r#"{ "$schema": "x", "skills": [] }"#; + let doc: IndexDocument = serde_json::from_str(src).expect("parses"); + assert!(doc.skills.is_empty()); + } + + #[test] + fn validate_entry_uri_accepts_canonical_skill_scheme() { + assert!(validate_entry_uri("skill://pull-requests/SKILL.md").is_ok()); + } + + #[test] + fn validate_entry_uri_accepts_domain_native_scheme() { + assert!(validate_entry_uri("github://skills/review/SKILL.md").is_ok()); + } + + #[test] + fn validate_entry_uri_rejects_empty() { + assert!(validate_entry_uri("").is_err()); + } + + #[test] + fn validate_entry_uri_rejects_missing_scheme() { + assert!(validate_entry_uri("pull-requests/SKILL.md").is_err()); + } + + #[test] + fn validate_entry_uri_rejects_control_chars() { + assert!(validate_entry_uri("skill://has\nnewline").is_err()); + assert!(validate_entry_uri("skill://has\ttab").is_err()); + assert!(validate_entry_uri("skill://has\0null").is_err()); + } + + #[test] + fn validate_entry_uri_rejects_invalid_scheme() { + assert!(validate_entry_uri("1bad://x").is_err()); + assert!(validate_entry_uri("://x").is_err()); + assert!(validate_entry_uri("bad scheme://x").is_err()); + } + + #[test] + fn validate_entry_uri_rejects_too_long() { + let long = format!("skill://{}", "a".repeat(MAX_URI_LEN)); + assert!(validate_entry_uri(&long).is_err()); + } + + #[test] + fn first_text_prefers_text_over_blob() { + use mcp_types::BlobResourceContents; + use mcp_types::TextResourceContents; + + let result = ReadResourceResult { + contents: vec![ + ReadResourceResultContents::BlobResourceContents(BlobResourceContents { + blob: "abc".to_owned(), + mime_type: Some("application/octet-stream".to_owned()), + uri: "x".to_owned(), + }), + ReadResourceResultContents::TextResourceContents(TextResourceContents { + mime_type: Some("text/markdown".to_owned()), + text: "---\nname: x\n---\n".to_owned(), + uri: "x".to_owned(), + }), + ], + }; + assert_eq!(first_text(&result), Some("---\nname: x\n---\n")); + } +} diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index cf7c180502b2..3d56a1ca5ca2 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -1,6 +1,7 @@ pub mod injection; pub mod loader; pub mod manager; +pub mod mcp_loader; pub mod model; pub mod render; pub mod system; @@ -9,6 +10,8 @@ pub(crate) use injection::SkillInjections; pub(crate) use injection::build_skill_injections; pub use loader::load_skills; pub use manager::SkillsManager; +pub(crate) use mcp_loader::SKILLS_EXTENSION_ID; +pub(crate) use mcp_loader::load_mcp_skill_manifests; pub use model::SkillError; pub use model::SkillLoadOutcome; pub use model::SkillMetadata; diff --git a/codex-rs/core/src/skills/model.rs b/codex-rs/core/src/skills/model.rs index 9063d7a25039..7b05c56a3b57 100644 --- a/codex-rs/core/src/skills/model.rs +++ b/codex-rs/core/src/skills/model.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::path::PathBuf; use codex_protocol::protocol::SkillScope; @@ -7,8 +8,17 @@ pub struct SkillMetadata { pub name: String, pub description: String, pub short_description: Option, + /// For filesystem skills, the absolute path to SKILL.md. For MCP-served + /// skills, the `skill://` URI of SKILL.md, stored here so existing + /// display paths continue to work. pub path: PathBuf, pub scope: SkillScope, + /// `Some` for MCP-served skills. Equal to the `skill://` URI of the + /// skill's SKILL.md resource. `None` for filesystem skills. + pub uri: Option, + /// `Some` for MCP-served skills. The name of the MCP server from which + /// this skill's index entry was fetched. `None` for filesystem skills. + pub server_name: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -22,3 +32,88 @@ pub struct SkillLoadOutcome { pub skills: Vec, pub errors: Vec, } + +impl SkillLoadOutcome { + /// Append MCP-served skills into this outcome with filesystem-wins + /// precedence on name collision. Returns the names of MCP skills that + /// were masked by an existing entry so callers can log them. + pub fn add_mcp_skills(&mut self, mcp_skills: I) -> Vec + where + I: IntoIterator, + { + let existing: HashSet = + self.skills.iter().map(|skill| skill.name.clone()).collect(); + let mut masked = Vec::new(); + for skill in mcp_skills { + if existing.contains(&skill.name) { + masked.push(skill.name.clone()); + } else { + self.skills.push(skill); + } + } + masked + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn fs_skill(name: &str) -> SkillMetadata { + SkillMetadata { + name: name.to_string(), + description: String::new(), + short_description: None, + path: PathBuf::from(format!("/skills/{name}/SKILL.md")), + scope: SkillScope::Repo, + uri: None, + server_name: None, + } + } + + fn mcp_skill(name: &str, server: &str) -> SkillMetadata { + SkillMetadata { + name: name.to_string(), + description: String::new(), + short_description: None, + path: PathBuf::from(format!("skill://{name}/SKILL.md")), + scope: SkillScope::Mcp, + uri: Some(format!("skill://{name}/SKILL.md")), + server_name: Some(server.to_string()), + } + } + + #[test] + fn add_mcp_skills_filesystem_wins_and_reports_masked() { + let mut outcome = SkillLoadOutcome { + skills: vec![fs_skill("alpha"), fs_skill("beta")], + errors: Vec::new(), + }; + + let masked = outcome.add_mcp_skills(vec![ + mcp_skill("alpha", "srv"), + mcp_skill("gamma", "srv"), + ]); + + assert_eq!(masked, vec!["alpha".to_string()]); + let names: Vec<&str> = outcome.skills.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["alpha", "beta", "gamma"]); + let alpha = outcome + .skills + .iter() + .find(|s| s.name == "alpha") + .expect("alpha present"); + assert!(alpha.uri.is_none(), "filesystem alpha must survive merge"); + } + + #[test] + fn add_mcp_skills_with_empty_input_is_noop() { + let mut outcome = SkillLoadOutcome { + skills: vec![fs_skill("alpha")], + errors: Vec::new(), + }; + let masked = outcome.add_mcp_skills(std::iter::empty()); + assert!(masked.is_empty()); + assert_eq!(outcome.skills.len(), 1); + } +} diff --git a/codex-rs/core/src/skills/render.rs b/codex-rs/core/src/skills/render.rs index f767849b2435..849c1806b3d1 100644 --- a/codex-rs/core/src/skills/render.rs +++ b/codex-rs/core/src/skills/render.rs @@ -5,15 +5,40 @@ pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { return None; } + let has_mcp_skill = skills.iter().any(|skill| skill.uri.is_some()); + let mut lines: Vec = Vec::new(); lines.push("## Skills".to_string()); lines.push("These skills are discovered at startup from multiple local sources. Each entry includes a name, description, and file path so you can open the source for full instructions.".to_string()); + if has_mcp_skill { + lines.push( + concat!( + "Some skills are served by connected MCP servers rather than the filesystem. ", + "Those entries show `(uri: ://..., server: )` instead of ", + "`(file: ...)`; the scheme is usually `skill://` but servers MAY use a ", + "domain-native scheme (for example `github://`). To read such a skill, call ", + "the `read_mcp_resource` tool with the exact `server` and `uri` from the ", + "catalog entry — do not rewrite the URI. Relative references inside a ", + "URI-backed SKILL.md (e.g. `references/GUIDE.md`) resolve against the ", + "skill's directory: strip the trailing `SKILL.md` and append the relative ", + "path.", + ) + .to_string(), + ); + } + for skill in skills { - let path_str = skill.path.to_string_lossy().replace('\\', "/"); let name = skill.name.as_str(); let description = skill.description.as_str(); - lines.push(format!("- {name}: {description} (file: {path_str})")); + if let Some((uri, server)) = skill.uri.as_deref().zip(skill.server_name.as_deref()) { + lines.push(format!( + "- {name}: {description} (uri: {uri}, server: {server})" + )); + } else { + let path_str = skill.path.to_string_lossy().replace('\\', "/"); + lines.push(format!("- {name}: {description} (file: {path_str})")); + } } lines.push( @@ -39,3 +64,81 @@ pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { Some(lines.join("\n")) } + +#[cfg(test)] +mod tests { + use super::*; + use codex_protocol::protocol::SkillScope; + use std::path::PathBuf; + + fn fs_skill(name: &str, description: &str, path: &str) -> SkillMetadata { + SkillMetadata { + name: name.to_string(), + description: description.to_string(), + short_description: None, + path: PathBuf::from(path), + scope: SkillScope::User, + uri: None, + server_name: None, + } + } + + fn mcp_skill(name: &str, description: &str, uri: &str, server: &str) -> SkillMetadata { + SkillMetadata { + name: name.to_string(), + description: description.to_string(), + short_description: None, + path: PathBuf::from(uri), + scope: SkillScope::Mcp, + uri: Some(uri.to_string()), + server_name: Some(server.to_string()), + } + } + + #[test] + fn returns_none_when_empty() { + assert!(render_skills_section(&[]).is_none()); + } + + #[test] + fn filesystem_only_does_not_emit_mcp_preamble() { + let skills = vec![fs_skill( + "git-flow", + "manage commits", + "/skills/git/SKILL.md", + )]; + let rendered = render_skills_section(&skills).expect("non-empty"); + assert!(rendered.contains("- git-flow: manage commits (file: /skills/git/SKILL.md)")); + assert!(!rendered.contains("read_mcp_resource")); + assert!(!rendered.contains("uri:")); + } + + #[test] + fn mcp_only_emits_uri_line_and_preamble() { + let skills = vec![mcp_skill( + "pull-requests", + "review PRs", + "skill://pull-requests/SKILL.md", + "github-skills", + )]; + let rendered = render_skills_section(&skills).expect("non-empty"); + assert!(rendered.contains("read_mcp_resource")); + assert!(rendered.contains( + "- pull-requests: review PRs (uri: skill://pull-requests/SKILL.md, server: github-skills)" + )); + } + + #[test] + fn mixed_sources_render_each_kind_with_matching_suffix() { + let skills = vec![ + fs_skill("local", "local skill", "/skills/local/SKILL.md"), + mcp_skill("remote", "remote skill", "skill://remote/SKILL.md", "srv"), + ]; + let rendered = render_skills_section(&skills).expect("non-empty"); + assert!(rendered.contains("- local: local skill (file: /skills/local/SKILL.md)")); + assert!( + rendered.contains("- remote: remote skill (uri: skill://remote/SKILL.md, server: srv)") + ); + assert!(rendered.contains("read_mcp_resource")); + } +} diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 63b66647e5bb..f5b5faa4bc23 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -5,6 +5,7 @@ use crate::RolloutRecorder; use crate::exec_policy::ExecPolicyManager; use crate::mcp_connection_manager::McpConnectionManager; use crate::models_manager::manager::ModelsManager; +use crate::skills::SkillMetadata; use crate::skills::SkillsManager; use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecSessionManager; @@ -28,4 +29,9 @@ pub(crate) struct SessionServices { pub(crate) otel_manager: OtelManager, pub(crate) tool_approvals: Mutex, pub(crate) skills_manager: Arc, + /// Skills discovered from connected MCP servers at session start. + /// Stored here (rather than on `SkillsManager`, which is a process-wide + /// singleton) so per-turn `$Mention` resolution can see them without + /// leaking MCP skills across sessions. + pub(crate) session_mcp_skills: Arc>, } diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 617b3b8a2126..e6c303410fd3 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -96,6 +96,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); }) @@ -233,6 +234,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); }) @@ -428,6 +430,7 @@ async fn stdio_image_completions_round_trip() -> anyhow::Result<()> { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); }) @@ -571,6 +574,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); }) @@ -719,6 +723,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); }) @@ -899,6 +904,7 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); }) diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index 4b916e51b493..a65afe6e6de9 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -435,6 +435,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); config.tool_output_token_limit = Some(500); @@ -527,6 +528,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); }); @@ -787,6 +789,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { tool_timeout_sec: None, enabled_tools: None, disabled_tools: None, + mcp_skills: true, }, ); }); diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index 81eb80764be5..3cbb0fb1a136 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -207,6 +207,7 @@ impl MessageProcessor { capabilities: mcp_types::ServerCapabilities { completions: None, experimental: None, + extensions: None, logging: None, prompts: None, resources: None, diff --git a/codex-rs/mcp-types/src/lib.rs b/codex-rs/mcp-types/src/lib.rs index 7418bea85425..a0ccb0ee3118 100644 --- a/codex-rs/mcp-types/src/lib.rs +++ b/codex-rs/mcp-types/src/lib.rs @@ -1276,6 +1276,12 @@ pub struct ServerCapabilities { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub experimental: Option, + /// Namespaced extensions declared by the server per SEP-2133 + /// (Extensions). Each key is a reverse-DNS extension identifier such as + /// `io.modelcontextprotocol/skills`; each value is extension-defined. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub extensions: Option, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub logging: Option, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index cff2a8ad98da..d3bf84264e17 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1744,6 +1744,9 @@ pub enum SkillScope { Repo, System, Admin, + /// Served by a connected MCP server per the Skills-over-MCP extension + /// (`io.modelcontextprotocol/skills`). + Mcp, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] @@ -1755,6 +1758,12 @@ pub struct SkillMetadata { pub short_description: Option, pub path: PathBuf, pub scope: SkillScope, + #[ts(optional)] + #[serde(default, skip_serializing_if = "Option::is_none")] + pub uri: Option, + #[ts(optional)] + #[serde(default, skip_serializing_if = "Option::is_none")] + pub server_name: Option, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 819ea2b95587..7b3f0aca5481 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3749,6 +3749,8 @@ fn skills_for_cwd(cwd: &Path, skills_entries: &[SkillsListEntry]) -> Vec Vec