diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index dfd339083f..2c41881481 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1994,6 +1994,7 @@ dependencies = [ "codex-rollout", "codex-sandboxing", "codex-shell-command", + "codex-skill-search-extension", "codex-skills-extension", "codex-state", "codex-thread-store", @@ -3291,6 +3292,7 @@ dependencies = [ "codex-login", "codex-protocol", "codex-shell-command", + "codex-skill-search-extension", "codex-utils-absolute-path", "codex-utils-cli", "codex-utils-json-to-toml", @@ -3809,6 +3811,24 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "codex-skill-search-extension" +version = "0.0.0" +dependencies = [ + "async-trait", + "bm25", + "codex-core", + "codex-extension-api", + "codex-features", + "codex-protocol", + "codex-tools", + "codex-utils-absolute-path", + "pretty_assertions", + "serde", + "serde_json", + "tokio", +] + [[package]] name = "codex-skills" version = "0.0.0" @@ -4599,6 +4619,7 @@ dependencies = [ "codex-model-provider-info", "codex-models-manager", "codex-protocol", + "codex-skill-search-extension", "codex-utils-absolute-path", "codex-utils-cargo-bin", "codex-utils-path-uri", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 3b6a95ae23..39d3ac83c7 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -51,6 +51,7 @@ members = [ "ext/image-generation", "ext/memories", "ext/mcp", + "ext/skill-search", "ext/skills", "ext/web-search", "external-agent-migration", @@ -190,6 +191,7 @@ codex-lmstudio = { path = "lmstudio" } codex-login = { path = "login" } codex-message-history = { path = "message-history" } codex-memories-extension = { path = "ext/memories" } +codex-skill-search-extension = { path = "ext/skill-search" } codex-web-search-extension = { path = "ext/web-search" } codex-memories-read = { path = "memories/read" } codex-memories-write = { path = "memories/write" } diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 3d08059a63..df5c0da3e4 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -49,6 +49,7 @@ codex-hooks = { workspace = true } codex-otel = { workspace = true } codex-plugin = { workspace = true } codex-shell-command = { workspace = true } +codex-skill-search-extension = { workspace = true } codex-skills-extension = { workspace = true } codex-utils-cli = { workspace = true } codex-utils-pty = { workspace = true } diff --git a/codex-rs/app-server/src/extensions.rs b/codex-rs/app-server/src/extensions.rs index c979baacfa..4cbcbba2dd 100644 --- a/codex-rs/app-server/src/extensions.rs +++ b/codex-rs/app-server/src/extensions.rs @@ -74,6 +74,7 @@ where codex_guardian::install(&mut builder, guardian_agent_spawner); codex_memories_extension::install(&mut builder, codex_otel::global()); codex_mcp_extension::install(&mut builder); + codex_skill_search_extension::install(&mut builder); codex_web_search_extension::install(&mut builder, auth_manager.clone()); codex_image_generation_extension::install(&mut builder, auth_manager); let mut skill_providers = codex_skills_extension::SkillProviders::new() diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 9072cff3c2..17456d92e4 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -9518,6 +9518,29 @@ include_instructions = false Ok(()) } +#[tokio::test] +async fn skill_search_tool_disables_static_skill_instructions() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +skill_search_tool = true + +[skills] +include_instructions = true +"#, + )?; + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + + assert!(!config.include_skill_instructions); + Ok(()) +} + #[tokio::test] async fn approvals_reviewer_stays_manual_only_when_guardian_feature_is_enabled() -> std::io::Result<()> { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 1c3942853e..6a4e3f2a55 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -3245,11 +3245,12 @@ impl Config { let include_apps_instructions = cfg.include_apps_instructions.unwrap_or(true); let include_collaboration_mode_instructions = cfg.include_collaboration_mode_instructions.unwrap_or(true); - let include_skill_instructions = cfg - .skills - .as_ref() - .and_then(|skills| skills.include_instructions) - .unwrap_or(true); + let include_skill_instructions = !features.enabled(Feature::SkillSearchTool) + && cfg + .skills + .as_ref() + .and_then(|skills| skills.include_instructions) + .unwrap_or(true); let include_environment_context = cfg.include_environment_context.unwrap_or(true); let guardian_policy_config = guardian_policy_config_from_requirements(config_layer_stack.requirements_toml()) diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index 9a7dfc8298..762106746f 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -105,7 +105,6 @@ pub(super) async fn spawn_review_thread( extension_data.insert(HostLoadedSkills::new( parent_turn_context.turn_skills.outcome.clone(), )); - let review_turn_context = TurnContext { sub_id: review_turn_id.clone(), trace_id: current_span_trace_id(), diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index da1d825310..3abc9e2af1 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1219,7 +1219,7 @@ pub(crate) async fn built_tools( mcp_tools, deferred_mcp_tools, discoverable_tools, - extension_tool_executors: extension_tool_executors(sess), + extension_tool_executors: extension_tool_executors(sess, turn_context), dynamic_tools: turn_context.dynamic_tools.as_slice(), }, ))) diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index a095499ea0..90bccb7511 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -225,6 +225,7 @@ impl ToolRouter { pub(crate) fn extension_tool_executors( session: &Session, + turn: &TurnContext, ) -> Vec>> { session .services @@ -235,6 +236,7 @@ pub(crate) fn extension_tool_executors( contributor.tools( &session.services.session_extension_data, &session.services.thread_extension_data, + &turn.extension_data, ) }) .collect() diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index 4024cb61f7..c0c04069b4 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -37,6 +37,7 @@ impl codex_extension_api::ToolContributor for ExtensionEchoContributor { &self, _session_store: &ExtensionData, _thread_store: &ExtensionData, + _turn_store: &ExtensionData, ) -> Vec>> { vec![Arc::new(ExtensionEchoExecutor)] } @@ -341,7 +342,7 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow deferred_mcp_tools: None, mcp_tools: None, discoverable_tools: None, - extension_tool_executors: extension_tool_executors(&session), + extension_tool_executors: extension_tool_executors(&session, &turn), dynamic_tools: turn.dynamic_tools.as_slice(), }, ); diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index 76f06df1ed..1df5e352fa 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -27,6 +27,7 @@ codex-login = { workspace = true } codex-model-provider-info = { workspace = true } codex-models-manager = { workspace = true } codex-protocol = { workspace = true } +codex-skill-search-extension = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-cargo-bin = { workspace = true } codex-utils-path-uri = { workspace = true } diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 1582b44d7d..58cb81fa72 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -25,6 +25,7 @@ use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::RemoveOptions; use codex_extension_api::ExtensionRegistry; +use codex_extension_api::ExtensionRegistryBuilder; use codex_extension_api::LoadUserInstructionsFuture; use codex_extension_api::UserInstructionsProvider; use codex_extension_api::empty_extension_registry; @@ -360,6 +361,13 @@ impl TestCodexBuilder { self } + pub fn with_skill_search_extension(mut self) -> Self { + let mut builder = ExtensionRegistryBuilder::::new(); + codex_skill_search_extension::install(&mut builder); + self.extensions = Arc::new(builder.build()); + self + } + pub fn with_windows_cmd_shell(self) -> Self { if cfg!(windows) { self.with_user_shell(get_shell_by_model_provided_path(&PathBuf::from("cmd.exe"))) diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 55c75016d9..fb3358a22a 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -4,6 +4,7 @@ use anyhow::Result; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::ExecutorFileSystem; +use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Op; @@ -12,14 +13,18 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; 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_response_created; use core_test_support::responses::mount_sse_once; +use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; +use pretty_assertions::assert_eq; +use serde_json::Value; use std::sync::Arc; async fn write_repo_skill( @@ -130,3 +135,126 @@ async fn user_turn_includes_skill_instructions() -> Result<()> { Ok(()) } + +fn top_level_tool_names(body: &Value) -> Vec { + body.get("tools") + .and_then(Value::as_array) + .map(|tools| { + tools + .iter() + .filter_map(|tool| tool.get("name").and_then(Value::as_str)) + .map(str::to_string) + .collect() + }) + .unwrap_or_default() +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn skill_search_tool_is_visible_and_returns_matching_repo_skill() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let search_call_id = "skill-search-call"; + let mut builder = test_codex() + .with_skill_search_extension() + .with_config(|config| { + config + .features + .enable(Feature::SkillSearchTool) + .expect("skill search feature should be configurable"); + }) + .with_workspace_setup(move |cwd, fs| async move { + write_repo_skill( + cwd, + fs, + "demo", + "Find demo workflows", + "Use this skill for demo workflow work.", + ) + .await + }); + let test = builder.build_with_remote_env(&server).await?; + + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + search_call_id, + "skill_search", + r#"{"query":"demo workflows"}"#, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-2", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); + test.codex + .submit(Op::UserTurn { + environments: None, + items: vec![UserInput::Text { + text: "Find the right repo skill.".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.config.cwd.to_path_buf(), + approval_policy: AskForApproval::Never, + approvals_reviewer: None, + sandbox_policy, + permission_profile, + model: session_model, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + .await?; + + core_test_support::wait_for_event(test.codex.as_ref(), |event| { + matches!(event, codex_protocol::protocol::EventMsg::TurnComplete(_)) + }) + .await; + + let requests = mock.requests(); + assert_eq!(requests.len(), 2); + + let first_request = &requests[0]; + let tool_names = top_level_tool_names(&first_request.body_json()); + assert!( + tool_names.iter().any(|name| name == "skill_search"), + "skill_search should be visible to the model: {tool_names:?}" + ); + assert!( + first_request.body_contains_text("Use `skill_search` when a task may benefit from one"), + "skill-search guidance should replace the static skills block" + ); + assert!( + !first_request.body_contains_text(""), + "static skills instructions should be disabled when skill_search is enabled" + ); + + let output = mock + .function_call_output_text(search_call_id) + .expect("skill_search output should be posted back to the model"); + assert!( + output.contains("- demo: Find demo workflows"), + "skill_search output should include the matching repo skill: {output}" + ); + assert!( + output.contains(".agents/skills/demo/SKILL.md"), + "skill_search output should include the SKILL.md path: {output}" + ); + + Ok(()) +} diff --git a/codex-rs/ext/extension-api/src/contributors.rs b/codex-rs/ext/extension-api/src/contributors.rs index efc22f5fd9..ef7b8ac7b0 100644 --- a/codex-rs/ext/extension-api/src/contributors.rs +++ b/codex-rs/ext/extension-api/src/contributors.rs @@ -208,6 +208,7 @@ pub trait ToolContributor: Send + Sync { &self, session_store: &ExtensionData, thread_store: &ExtensionData, + turn_store: &ExtensionData, ) -> Vec>>; } diff --git a/codex-rs/ext/memories/src/extension.rs b/codex-rs/ext/memories/src/extension.rs index 23c80a8779..99d20aa767 100644 --- a/codex-rs/ext/memories/src/extension.rs +++ b/codex-rs/ext/memories/src/extension.rs @@ -100,6 +100,7 @@ impl ToolContributor for MemoriesExtension { &self, _session_store: &ExtensionData, thread_store: &ExtensionData, + _turn_store: &ExtensionData, ) -> Vec>> { let Some(config) = thread_store.get::() else { return Vec::new(); diff --git a/codex-rs/ext/memories/src/tests.rs b/codex-rs/ext/memories/src/tests.rs index b78c20c9f1..a2790d2a81 100644 --- a/codex-rs/ext/memories/src/tests.rs +++ b/codex-rs/ext/memories/src/tests.rs @@ -41,7 +41,8 @@ fn tools_are_not_contributed_without_thread_config() { extension .tools( &ExtensionData::new("session"), - &ExtensionData::new("thread") + &ExtensionData::new("thread"), + &ExtensionData::new("turn") ) .is_empty() ); @@ -59,7 +60,11 @@ fn tools_are_not_contributed_when_disabled() { assert!( extension - .tools(&ExtensionData::new("session"), &thread_store) + .tools( + &ExtensionData::new("session"), + &thread_store, + &ExtensionData::new("turn"), + ) .is_empty() ); } @@ -92,7 +97,11 @@ fn tools_are_contributed_when_enabled_with_dedicated_tools() { }); let tool_names = extension - .tools(&ExtensionData::new("session"), &thread_store) + .tools( + &ExtensionData::new("session"), + &thread_store, + &ExtensionData::new("turn"), + ) .into_iter() .map(|tool| tool.tool_name()) .collect::>(); diff --git a/codex-rs/ext/skill-search/BUILD.bazel b/codex-rs/ext/skill-search/BUILD.bazel new file mode 100644 index 0000000000..c24937d1ea --- /dev/null +++ b/codex-rs/ext/skill-search/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "skill-search", + crate_name = "codex_skill_search_extension", +) diff --git a/codex-rs/ext/skill-search/Cargo.toml b/codex-rs/ext/skill-search/Cargo.toml new file mode 100644 index 0000000000..6fb5b166b4 --- /dev/null +++ b/codex-rs/ext/skill-search/Cargo.toml @@ -0,0 +1,29 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-skill-search-extension" +version.workspace = true + +[lib] +name = "codex_skill_search_extension" +path = "src/lib.rs" +doctest = false + +[lints] +workspace = true + +[dependencies] +async-trait = { workspace = true } +bm25 = { workspace = true } +codex-core = { workspace = true } +codex-extension-api = { workspace = true } +codex-features = { workspace = true } +codex-protocol = { workspace = true } +codex-tools = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } + +[dev-dependencies] +codex-utils-absolute-path = { workspace = true } +pretty_assertions = { workspace = true } +tokio = { workspace = true, features = ["macros"] } diff --git a/codex-rs/ext/skill-search/src/extension.rs b/codex-rs/ext/skill-search/src/extension.rs new file mode 100644 index 0000000000..89bcf6ffae --- /dev/null +++ b/codex-rs/ext/skill-search/src/extension.rs @@ -0,0 +1,162 @@ +use std::sync::Arc; + +use codex_core::config::Config; +use codex_core::skills::SkillLoadOutcome; +use codex_extension_api::ConfigContributor; +use codex_extension_api::ContextContributor; +use codex_extension_api::ExtensionData; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::PromptFragment; +use codex_extension_api::ThreadLifecycleContributor; +use codex_extension_api::ThreadStartInput; +use codex_extension_api::ToolCall; +use codex_extension_api::ToolContributor; +use codex_extension_api::ToolExecutor; +use codex_features::Feature; + +use crate::tool::SkillSearchTool; + +const SKILL_SEARCH_GUIDANCE: &str = "## Skills\nSkills are local instruction bundles stored in `SKILL.md` files. Use `skill_search` when a task may benefit from one, open the returned path before following it, and use only the smallest relevant set for the current turn. If no result fits, continue without a skill."; + +#[derive(Clone, Copy, Debug, Default)] +pub(crate) struct SkillSearchExtension; + +#[derive(Clone, Copy, Debug)] +pub(crate) struct SkillSearchExtensionConfig { + pub(crate) enabled: bool, +} + +impl SkillSearchExtensionConfig { + fn from_config(config: &Config) -> Self { + Self { + enabled: config.features.enabled(Feature::SkillSearchTool), + } + } +} + +impl ContextContributor for SkillSearchExtension { + fn contribute<'a>( + &'a self, + _session_store: &'a ExtensionData, + thread_store: &'a ExtensionData, + ) -> std::pin::Pin> + Send + 'a>> { + Box::pin(async move { + let Some(config) = thread_store.get::() else { + return Vec::new(); + }; + if !config.enabled { + return Vec::new(); + } + + vec![PromptFragment::developer_policy(SKILL_SEARCH_GUIDANCE)] + }) + } +} + +impl ThreadLifecycleContributor for SkillSearchExtension { + fn on_thread_start(&self, input: ThreadStartInput<'_, Config>) { + input + .thread_store + .insert(SkillSearchExtensionConfig::from_config(input.config)); + } +} + +impl ConfigContributor for SkillSearchExtension { + fn on_config_changed( + &self, + _session_store: &ExtensionData, + thread_store: &ExtensionData, + _previous_config: &Config, + new_config: &Config, + ) { + thread_store.insert(SkillSearchExtensionConfig::from_config(new_config)); + } +} + +impl ToolContributor for SkillSearchExtension { + fn tools( + &self, + _session_store: &ExtensionData, + thread_store: &ExtensionData, + turn_store: &ExtensionData, + ) -> Vec>> { + let Some(config) = thread_store.get::() else { + return Vec::new(); + }; + if !config.enabled { + return Vec::new(); + } + + let skills = turn_store + .get::() + .map_or_else(Vec::new, |outcome| { + outcome.allowed_skills_for_implicit_invocation() + }); + let tool = turn_store.get_or_init(|| SkillSearchTool::new(skills)); + vec![tool] + } +} + +/// Installs the skills context contributor and skill_search tool. +pub fn install(registry: &mut ExtensionRegistryBuilder) { + let extension = Arc::new(SkillSearchExtension); + registry.thread_lifecycle_contributor(extension.clone()); + registry.config_contributor(extension.clone()); + registry.prompt_contributor(extension.clone()); + registry.tool_contributor(extension); +} + +#[cfg(test)] +mod tests { + use codex_extension_api::PromptSlot; + use codex_extension_api::ToolContributor; + use codex_extension_api::ToolName; + use pretty_assertions::assert_eq; + + use super::*; + use crate::tool::SKILL_SEARCH_TOOL_NAME; + + #[tokio::test] + async fn prompt_contribution_is_gated_by_feature_config() { + let extension = SkillSearchExtension; + let session_store = ExtensionData::new("session"); + let thread_store = ExtensionData::new("thread"); + + assert!( + extension + .contribute(&session_store, &thread_store) + .await + .is_empty() + ); + + thread_store.insert(SkillSearchExtensionConfig { enabled: true }); + let fragments = extension.contribute(&session_store, &thread_store).await; + + assert_eq!(fragments.len(), 1); + assert_eq!(fragments[0].slot(), PromptSlot::DeveloperPolicy); + assert!(fragments[0].text().contains(SKILL_SEARCH_TOOL_NAME)); + } + + #[test] + fn tool_contribution_is_gated_by_feature_config() { + let extension = SkillSearchExtension; + let session_store = ExtensionData::new("session"); + let thread_store = ExtensionData::new("thread"); + let turn_store = ExtensionData::new("turn"); + + assert!( + extension + .tools(&session_store, &thread_store, &turn_store) + .is_empty() + ); + + thread_store.insert(SkillSearchExtensionConfig { enabled: true }); + let tool_names = extension + .tools(&session_store, &thread_store, &turn_store) + .into_iter() + .map(|tool| tool.tool_name()) + .collect::>(); + + assert_eq!(tool_names, vec![ToolName::plain(SKILL_SEARCH_TOOL_NAME)]); + } +} diff --git a/codex-rs/ext/skill-search/src/lib.rs b/codex-rs/ext/skill-search/src/lib.rs new file mode 100644 index 0000000000..285221cc69 --- /dev/null +++ b/codex-rs/ext/skill-search/src/lib.rs @@ -0,0 +1,4 @@ +mod extension; +mod tool; + +pub use extension::install; diff --git a/codex-rs/ext/skill-search/src/tool.rs b/codex-rs/ext/skill-search/src/tool.rs new file mode 100644 index 0000000000..1a0d2ab5d4 --- /dev/null +++ b/codex-rs/ext/skill-search/src/tool.rs @@ -0,0 +1,297 @@ +use std::collections::BTreeMap; + +use bm25::Document; +use bm25::Language; +use bm25::SearchEngine; +use bm25::SearchEngineBuilder; +use codex_core::skills::SkillMetadata; +use codex_extension_api::FunctionCallError; +use codex_extension_api::ResponsesApiTool; +use codex_extension_api::ToolCall; +use codex_extension_api::ToolExecutor; +use codex_extension_api::ToolName; +use codex_extension_api::ToolOutput; +use codex_extension_api::ToolSpec; +use codex_protocol::models::FunctionCallOutputBody; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::ResponseInputItem; +use codex_tools::JsonSchema; +use codex_tools::ToolPayload; +use serde::Deserialize; +use serde_json::Value; + +pub(crate) const SKILL_SEARCH_TOOL_NAME: &str = "skill_search"; +const DEFAULT_SKILL_SEARCH_LIMIT: usize = 8; + +#[derive(Clone, Debug)] +struct SkillSearchEntry { + name: String, + description: String, + path: String, + search_text: String, +} + +impl SkillSearchEntry { + fn from_metadata(skill: SkillMetadata) -> Self { + let path = skill.path_to_skills_md.to_string_lossy().replace('\\', "/"); + let search_text = format!("{}\n{}", skill.name, skill.description); + Self { + name: skill.name, + description: skill.description, + path, + search_text, + } + } + + fn render(&self) -> String { + if self.description.is_empty() { + format!("- {}: (file: {})", self.name, self.path) + } else { + format!( + "- {}: {} (file: {})", + self.name, self.description, self.path + ) + } + } +} + +pub(crate) struct SkillSearchTool { + entries: Vec, + search_engine: SearchEngine, +} + +impl SkillSearchTool { + pub(crate) fn new(skills: Vec) -> Self { + let entries = skills + .into_iter() + .map(SkillSearchEntry::from_metadata) + .collect::>(); + let documents = entries + .iter() + .map(|entry| entry.search_text.clone()) + .enumerate() + .map(|(idx, search_text)| Document::new(idx, search_text)) + .collect::>(); + let search_engine = + SearchEngineBuilder::::with_documents(Language::English, documents).build(); + + Self { + entries, + search_engine, + } + } + + fn search(&self, query: &str, limit: usize) -> String { + self.search_engine + .search(query, limit) + .into_iter() + .filter_map(|result| self.entries.get(result.document.id)) + .map(SkillSearchEntry::render) + .collect::>() + .join("\n") + } +} + +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct SkillSearchArgs { + query: String, + limit: Option, +} + +#[async_trait::async_trait] +impl ToolExecutor for SkillSearchTool { + fn tool_name(&self) -> ToolName { + ToolName::plain(SKILL_SEARCH_TOOL_NAME) + } + + fn spec(&self) -> Option { + Some(ToolSpec::Function(ResponsesApiTool { + name: SKILL_SEARCH_TOOL_NAME.to_string(), + description: "Search available Codex skills by relevance and return plain-text matches with their descriptions and SKILL.md paths.".to_string(), + strict: false, + defer_loading: None, + parameters: JsonSchema::object( + BTreeMap::from([ + ( + "limit".to_string(), + JsonSchema::number(Some(format!( + "Maximum number of skills to return (defaults to {DEFAULT_SKILL_SEARCH_LIMIT})." + ))), + ), + ( + "query".to_string(), + JsonSchema::string(Some("Search query for available skills.".to_string())), + ), + ]), + Some(vec!["query".to_string()]), + Some(false.into()), + ), + output_schema: None, + })) + } + + fn supports_parallel_tool_calls(&self) -> bool { + true + } + + async fn handle(&self, call: ToolCall) -> Result, FunctionCallError> { + let args = parse_args(&call)?; + let query = args.query.trim(); + if query.is_empty() { + return Err(FunctionCallError::RespondToModel( + "query must not be empty".to_string(), + )); + } + let limit = args.limit.unwrap_or(DEFAULT_SKILL_SEARCH_LIMIT); + if limit == 0 { + return Err(FunctionCallError::RespondToModel( + "limit must be greater than zero".to_string(), + )); + } + + Ok(Box::new(PlainTextToolOutput { + text: self.search(query, limit), + })) + } +} + +fn parse_args(call: &ToolCall) -> Result { + let arguments = call.function_arguments()?; + let value = if arguments.trim().is_empty() { + Value::Object(serde_json::Map::new()) + } else { + serde_json::from_str(arguments) + .map_err(|err| FunctionCallError::RespondToModel(err.to_string()))? + }; + serde_json::from_value(value).map_err(|err| FunctionCallError::RespondToModel(err.to_string())) +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct PlainTextToolOutput { + text: String, +} + +impl ToolOutput for PlainTextToolOutput { + fn log_preview(&self) -> String { + self.text.clone() + } + + fn success_for_logging(&self) -> bool { + true + } + + fn to_response_item(&self, call_id: &str, _payload: &ToolPayload) -> ResponseInputItem { + ResponseInputItem::FunctionCallOutput { + call_id: call_id.to_string(), + output: FunctionCallOutputPayload { + body: FunctionCallOutputBody::Text(self.text.clone()), + success: Some(true), + }, + } + } +} + +#[cfg(test)] +mod tests { + use codex_core::skills::SkillPolicy; + use codex_protocol::models::FunctionCallOutputBody; + use codex_protocol::protocol::SkillScope; + use codex_tools::ToolPayload; + use codex_utils_absolute_path::test_support::PathBufExt; + use codex_utils_absolute_path::test_support::test_path_buf; + use pretty_assertions::assert_eq; + use serde_json::json; + + use super::*; + + fn skill(name: &str, description: &str, path: &str) -> SkillMetadata { + SkillMetadata { + name: name.to_string(), + description: description.to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: Some(SkillPolicy { + allow_implicit_invocation: Some(true), + products: Vec::new(), + }), + path_to_skills_md: test_path_buf(path).abs(), + scope: SkillScope::User, + plugin_id: None, + } + } + + fn call(arguments: serde_json::Value) -> ToolCall { + ToolCall { + call_id: "call-skill-search".to_string(), + tool_name: ToolName::plain(SKILL_SEARCH_TOOL_NAME), + payload: ToolPayload::Function { + arguments: arguments.to_string(), + }, + } + } + + async fn output_text(tool: &SkillSearchTool, arguments: serde_json::Value) -> String { + let output = tool + .handle(call(arguments)) + .await + .expect("skill_search should return output"); + let response = output.to_response_item( + "call-skill-search", + &ToolPayload::Function { + arguments: "{}".to_string(), + }, + ); + let ResponseInputItem::FunctionCallOutput { output, .. } = response else { + panic!("expected function output"); + }; + let FunctionCallOutputBody::Text(text) = output.body else { + panic!("expected text output"); + }; + text + } + + #[tokio::test] + async fn search_returns_plain_text_skill_lines() { + let tool = SkillSearchTool::new(vec![ + skill("slides", "Build presentation decks", "/tmp/slides/SKILL.md"), + skill("sheets", "Analyze spreadsheet data", "/tmp/sheets/SKILL.md"), + ]); + + assert_eq!( + output_text(&tool, json!({ "query": "presentation deck" })).await, + "- slides: Build presentation decks (file: /tmp/slides/SKILL.md)" + ); + } + + #[tokio::test] + async fn search_returns_empty_text_when_no_skill_matches() { + let tool = SkillSearchTool::new(vec![skill( + "slides", + "Build presentation decks", + "/tmp/slides/SKILL.md", + )]); + + assert_eq!(output_text(&tool, json!({ "query": "quantum" })).await, ""); + } + + #[tokio::test] + async fn search_rejects_empty_query_and_zero_limit() { + let tool = SkillSearchTool::new(Vec::new()); + + let empty_query = tool + .handle(call(json!({ "query": " " }))) + .await + .err() + .expect("empty query should fail"); + let zero_limit = tool + .handle(call(json!({ "query": "docs", "limit": 0 }))) + .await + .err() + .expect("zero limit should fail"); + + assert_eq!(empty_query.to_string(), "query must not be empty"); + assert_eq!(zero_limit.to_string(), "limit must be greater than zero"); + } +} diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 2f8e91d709..842803667c 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -147,6 +147,8 @@ pub enum Feature { AppsMcpPathOverride, /// Removed compatibility flag retained as a no-op now that tool_search is always enabled. ToolSearch, + /// Enable skill discovery through a model-visible skill_search tool. + SkillSearchTool, /// Always defer MCP tools behind tool_search instead of exposing small sets directly. ToolSearchAlwaysDeferMcpTools, /// Expose MCP model-visible namespaces without the legacy `mcp__` prefix. @@ -1000,6 +1002,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Removed, default_enabled: false, }, + FeatureSpec { + id: Feature::SkillSearchTool, + key: "skill_search_tool", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::ToolSearchAlwaysDeferMcpTools, key: "tool_search_always_defer_mcp_tools", diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index 0a4f925f4f..1e89298d64 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -189,6 +189,16 @@ fn tool_search_is_removed_and_disabled_by_default() { assert_eq!(feature_for_key("tool_search"), Some(Feature::ToolSearch)); } +#[test] +fn skill_search_tool_is_under_development_and_disabled_by_default() { + assert_eq!(Feature::SkillSearchTool.stage(), Stage::UnderDevelopment); + assert_eq!(Feature::SkillSearchTool.default_enabled(), false); + assert_eq!( + feature_for_key("skill_search_tool"), + Some(Feature::SkillSearchTool) + ); +} + #[test] fn browser_controls_are_stable_and_enabled_by_default() { assert_eq!(Feature::InAppBrowser.stage(), Stage::Stable); diff --git a/codex-rs/mcp-server/Cargo.toml b/codex-rs/mcp-server/Cargo.toml index 9d568722e8..6088f784af 100644 --- a/codex-rs/mcp-server/Cargo.toml +++ b/codex-rs/mcp-server/Cargo.toml @@ -26,6 +26,7 @@ codex-exec-server = { workspace = true } codex-extension-api = { workspace = true } codex-login = { workspace = true } codex-protocol = { workspace = true } +codex-skill-search-extension = { workspace = true } codex-utils-cli = { workspace = true } codex-utils-json-to-toml = { workspace = true } rmcp = { workspace = true } diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index ce19575a9c..f731152abe 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -6,7 +6,7 @@ use codex_core::StateDbHandle; use codex_core::ThreadManager; use codex_core::config::Config; use codex_exec_server::EnvironmentManager; -use codex_extension_api::empty_extension_registry; +use codex_extension_api::ExtensionRegistryBuilder; use codex_home::CodexHomeUserInstructionsProvider; use codex_login::AuthManager; use codex_login::default_client::USER_AGENT_SUFFIX; @@ -66,12 +66,14 @@ impl MessageProcessor { let user_instructions_provider = Arc::new(CodexHomeUserInstructionsProvider::new( config.codex_home.clone(), )); + let mut extensions = ExtensionRegistryBuilder::::new(); + codex_skill_search_extension::install(&mut extensions); let thread_manager = Arc::new(ThreadManager::new( config.as_ref(), auth_manager, SessionSource::Mcp, environment_manager, - empty_extension_registry(), + Arc::new(extensions.build()), user_instructions_provider, /*analytics_events_client*/ None, codex_core::thread_store_from_config(config.as_ref(), state_db.clone()),