From 02be9c582777b45b2982fab4f160eff8b952a34f Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Sun, 25 Jan 2026 23:36:51 -0800 Subject: [PATCH] update --- codex-rs/core/src/mcp_connection_manager.rs | 27 ++++++++++++++----- codex-rs/core/src/project_doc.rs | 4 +-- codex-rs/core/src/skills/render.rs | 1 + .../core/src/tools/handlers/mcp_install.rs | 13 ++++----- codex-rs/core/src/tools/spec.rs | 2 +- 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 0e3674863e..2b16735c67 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -157,6 +157,16 @@ pub(crate) struct ToolInfo { pub(crate) connector_name: Option, } +pub(crate) struct AddServerParams { + pub(crate) server_name: String, + pub(crate) config: McpServerConfig, + pub(crate) store_mode: OAuthCredentialsStoreMode, + pub(crate) auth_entry: Option, + pub(crate) tx_event: Sender, + pub(crate) cancel_token: CancellationToken, + pub(crate) sandbox_state: SandboxState, +} + type ResponderMap = HashMap<(String, RequestId), oneshot::Sender>; #[derive(Clone, Default)] @@ -437,14 +447,17 @@ impl McpConnectionManager { pub async fn add_server( &mut self, - server_name: String, - config: McpServerConfig, - store_mode: OAuthCredentialsStoreMode, - auth_entry: Option, - tx_event: Sender, - cancel_token: CancellationToken, - sandbox_state: SandboxState, + params: AddServerParams, ) -> Result { + let AddServerParams { + server_name, + config, + store_mode, + auth_entry, + tx_event, + cancel_token, + sandbox_state, + } = params; if cancel_token.is_cancelled() { return Err("MCP startup was cancelled".to_string()); } diff --git a/codex-rs/core/src/project_doc.rs b/codex-rs/core/src/project_doc.rs index c763755af5..591b3112b4 100644 --- a/codex-rs/core/src/project_doc.rs +++ b/codex-rs/core/src/project_doc.rs @@ -516,7 +516,7 @@ mod tests { ) .unwrap_or_else(|_| cfg.codex_home.join("skills/pdf-processing/SKILL.md")); let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); - let usage_rules = "- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.\n- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.\n- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.\n- How to use a skill (progressive disclosure):\n 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.\n 2) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.\n 3) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.\n 4) If `assets/` or templates exist, reuse them instead of recreating from scratch.\n- Coordination and sequencing:\n - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.\n - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.\n- Context hygiene:\n - Keep context small: summarize long sections instead of pasting them; only load extra files when needed.\n - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.\n - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.\n- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."; + let usage_rules = "- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.\n- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.\n- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.\n- MCP availability: If MCP tools are required in this skill but not available, use the install_mcp_tool tool to install the missing MCPs.\n- How to use a skill (progressive disclosure):\n 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.\n 2) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.\n 3) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.\n 4) If `assets/` or templates exist, reuse them instead of recreating from scratch.\n- Coordination and sequencing:\n - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.\n - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.\n- Context hygiene:\n - Keep context small: summarize long sections instead of pasting them; only load extra files when needed.\n - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.\n - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.\n- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."; let expected = format!( "base doc\n\n## Skills\nA skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.\n### Available skills\n- pdf-processing: extract from pdfs (file: {expected_path_str})\n### How to use skills\n{usage_rules}" ); @@ -540,7 +540,7 @@ mod tests { dunce::canonicalize(cfg.codex_home.join("skills/linting/SKILL.md").as_path()) .unwrap_or_else(|_| cfg.codex_home.join("skills/linting/SKILL.md")); let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); - let usage_rules = "- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.\n- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.\n- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.\n- How to use a skill (progressive disclosure):\n 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.\n 2) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.\n 3) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.\n 4) If `assets/` or templates exist, reuse them instead of recreating from scratch.\n- Coordination and sequencing:\n - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.\n - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.\n- Context hygiene:\n - Keep context small: summarize long sections instead of pasting them; only load extra files when needed.\n - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.\n - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.\n- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."; + let usage_rules = "- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.\n- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.\n- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.\n- MCP availability: If MCP tools are required in this skill but not available, use the install_mcp_tool tool to install the missing MCPs.\n- How to use a skill (progressive disclosure):\n 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.\n 2) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.\n 3) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.\n 4) If `assets/` or templates exist, reuse them instead of recreating from scratch.\n- Coordination and sequencing:\n - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.\n - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.\n- Context hygiene:\n - Keep context small: summarize long sections instead of pasting them; only load extra files when needed.\n - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.\n - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.\n- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."; let expected = format!( "## Skills\nA skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.\n### Available skills\n- linting: run clippy (file: {expected_path_str})\n### How to use skills\n{usage_rules}" ); diff --git a/codex-rs/core/src/skills/render.rs b/codex-rs/core/src/skills/render.rs index f998a51042..28613bb99d 100644 --- a/codex-rs/core/src/skills/render.rs +++ b/codex-rs/core/src/skills/render.rs @@ -22,6 +22,7 @@ pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { r###"- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths. - Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned. - Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback. +- MCP availability: If MCP tools are required in this skill but not available, use the install_mcp_tool tool to install the missing MCPs. - How to use a skill (progressive disclosure): 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow. 2) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything. diff --git a/codex-rs/core/src/tools/handlers/mcp_install.rs b/codex-rs/core/src/tools/handlers/mcp_install.rs index 73e1ea4a93..3f5594ac57 100644 --- a/codex-rs/core/src/tools/handlers/mcp_install.rs +++ b/codex-rs/core/src/tools/handlers/mcp_install.rs @@ -18,6 +18,7 @@ use crate::function_tool::FunctionCallError; use crate::mcp::McpServerAuthFlow; use crate::mcp::auth::compute_auth_statuses; use crate::mcp::install_mcp_server; +use crate::mcp_connection_manager::AddServerParams; use crate::mcp_connection_manager::DEFAULT_STARTUP_TIMEOUT; use crate::mcp_connection_manager::SandboxState; use crate::protocol::SandboxPolicy; @@ -286,15 +287,15 @@ impl ToolHandler for McpInstallHandler { let startup_handle = { let mut manager = session.services.mcp_connection_manager.write().await; manager - .add_server( - name.clone(), - server_config, - config.mcp_oauth_credentials_store_mode, + .add_server(AddServerParams { + server_name: name.clone(), + config: server_config, + store_mode: config.mcp_oauth_credentials_store_mode, auth_entry, - session.get_tx_event(), + tx_event: session.get_tx_event(), cancel_token, sandbox_state, - ) + }) .await .map_err(FunctionCallError::RespondToModel)? }; diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 06f7dc01bd..c18f39c987 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1059,7 +1059,7 @@ fn create_install_mcp_tool() -> ToolSpec { ToolSpec::Function(ResponsesApiTool { name: "install_mcp_tool".to_string(), - description: "Use when the model needs to use an mcp tool but it's not already available. Also prefer to use this tool instead of calling `codex mcp add`. Before calling this tool, model must search online (if allowed) for the exact mcp config params required to install this mcp, until it's confident it has the correct configuration or it cannot figure it out.".to_string(), + description: "Use when the model needs to use an mcp tool but it's not available. You must call this tool instead whenever you want to run the `codex mcp add` command. Before calling this tool, model must search online (if allowed) for the exact mcp config params required to install this mcp, until you are confident it has the correct configuration or it cannot figure it out.".to_string(), strict: false, parameters: JsonSchema::Object { properties,