This commit is contained in:
Matthew Zeng
2026-01-25 23:36:51 -08:00
parent f9d902e957
commit 02be9c5827
5 changed files with 31 additions and 16 deletions

View File

@@ -157,6 +157,16 @@ pub(crate) struct ToolInfo {
pub(crate) connector_name: Option<String>,
}
pub(crate) struct AddServerParams {
pub(crate) server_name: String,
pub(crate) config: McpServerConfig,
pub(crate) store_mode: OAuthCredentialsStoreMode,
pub(crate) auth_entry: Option<McpAuthStatusEntry>,
pub(crate) tx_event: Sender<Event>,
pub(crate) cancel_token: CancellationToken,
pub(crate) sandbox_state: SandboxState,
}
type ResponderMap = HashMap<(String, RequestId), oneshot::Sender<ElicitationResponse>>;
#[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<McpAuthStatusEntry>,
tx_event: Sender<Event>,
cancel_token: CancellationToken,
sandbox_state: SandboxState,
params: AddServerParams,
) -> Result<McpServerStartupHandle, String> {
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());
}

View File

@@ -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}"
);

View File

@@ -22,6 +22,7 @@ pub fn render_skills_section(skills: &[SkillMetadata]) -> Option<String> {
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.

View File

@@ -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)?
};

View File

@@ -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,