diff --git a/crates/rmcp-macros/src/lib.rs b/crates/rmcp-macros/src/lib.rs index 573fdd28c..75758c7d8 100644 --- a/crates/rmcp-macros/src/lib.rs +++ b/crates/rmcp-macros/src/lib.rs @@ -7,6 +7,9 @@ mod common; mod prompt; mod prompt_handler; mod prompt_router; +mod skill; +mod skill_handler; +mod skill_router; mod tool; mod tool_handler; mod tool_router; @@ -330,3 +333,39 @@ pub fn prompt_handler(attr: TokenStream, input: TokenStream) -> TokenStream { .unwrap_or_else(|err| err.to_compile_error()) .into() } + +/// # skill +/// +/// Marks a function as a skill handler for the SEP-2640 Skills extension. +/// Generates a companion `*_skill_attr()` function returning the skill's +/// `SkillEntry` metadata, and wraps the async body in `Pin>`. +#[proc_macro_attribute] +pub fn skill(attr: TokenStream, input: TokenStream) -> TokenStream { + skill::skill(attr.into(), input.into()) + .unwrap_or_else(|err| err.to_compile_error()) + .into() +} + +/// # skill_router +/// +/// Walks an impl block for `#[skill]`-annotated functions and builds a +/// `SkillRouter` mapping each skill's `skill://` URI to its handler. +#[proc_macro_attribute] +pub fn skill_router(attr: TokenStream, input: TokenStream) -> TokenStream { + skill_router::skill_router(attr.into(), input.into()) + .unwrap_or_else(|err| err.to_compile_error()) + .into() +} + +/// # skill_handler +/// +/// Implements the `ServerHandler` skills trait methods (`skills_list`, +/// `skills_get`, `resources_directory_read`) for a struct, wiring them +/// through a `SkillRouter`. Also generates `get_info()` with +/// `.enable_skills_extension()` if no sibling `tool_handler` is present. +#[proc_macro_attribute] +pub fn skill_handler(attr: TokenStream, input: TokenStream) -> TokenStream { + skill_handler::skill_handler(attr.into(), input.into()) + .unwrap_or_else(|err| err.to_compile_error()) + .into() +} diff --git a/crates/rmcp-macros/src/skill.rs b/crates/rmcp-macros/src/skill.rs new file mode 100644 index 000000000..a59cf1ce1 --- /dev/null +++ b/crates/rmcp-macros/src/skill.rs @@ -0,0 +1,176 @@ +//! Skill proc-macro — the sticker. +//! +//! Pedagogy: The `#[skill]` macro puts a sticker on an assignment that says +//! "this is a skill handler, here's its ID card (SkillEntry)." It generates +//! a companion `*_skill_attr()` function that returns the skill's metadata +//! (URI + frontmatter + resources), and wraps the async body in a +//! `Pin>` so the gradebook can call it synchronously. + +use darling::{FromMeta, ast::NestedMeta}; +use proc_macro2::TokenStream; +use quote::{format_ident, quote}; +use syn::{Expr, Ident, ImplItemFn, parse_quote}; + +#[derive(FromMeta, Default, Debug)] +#[darling(default)] +pub struct SkillAttribute { + /// The `skill://` URI this handler serves, e.g. `skill://git-workflow/SKILL.md`. + pub uri: Option, + /// Path to the frontmatter JSON (typically `include_str!("...frontmatter.json")`). + pub frontmatter: Option, + /// Whether this skill generates its content dynamically (no fixed file list). + pub dynamic: bool, +} + +pub struct ResolvedSkillAttribute { + pub uri: String, + pub frontmatter: Expr, + pub dynamic: bool, +} + +impl ResolvedSkillAttribute { + pub fn into_fn(self, fn_ident: Ident) -> syn::Result { + let Self { + uri, + frontmatter, + dynamic, + } = self; + // SEP-2640 skills.mdx:225 — `resources` is REQUIRED on every skill + // entry, and an entry with no `resources` at all is invalid: hosts + // MUST NOT load it. The `#[skill]` attribute has no file-list + // parameter, so the macro cannot publish digests for a static skill + // and `"dynamic"` is the only valid value it can emit. (The `dynamic` + // flag is retained for source compatibility; both branches emit the + // same value until a file-list parameter exists.) + let _ = dynamic; + let resources = quote! { Some(rmcp::model::skills::SkillResources::Dynamic) }; + let doc_comment = format!("Generated skill metadata function for {uri}"); + let doc_attr: syn::Attribute = parse_quote!(#[doc = #doc_comment]); + let tokens = quote! { + #doc_attr + pub fn #fn_ident() -> rmcp::model::skills::SkillEntry { + let mut entry = rmcp::model::skills::SkillEntry::new(#uri, #frontmatter); + entry.resources = #resources; + entry + } + }; + syn::parse2::(tokens) + } +} + +pub fn skill(attr: TokenStream, input: TokenStream) -> syn::Result { + let attribute = if attr.is_empty() { + Default::default() + } else { + let attr_args = NestedMeta::parse_meta_list(attr)?; + SkillAttribute::from_list(&attr_args)? + }; + let fn_item = syn::parse2::(input.clone())?; + let fn_ident = &fn_item.sig.ident; + + let skill_attr_fn_ident = format_ident!("{}_skill_attr", fn_ident); + + // Validate URI is present + let uri = attribute.uri.ok_or_else(|| { + syn::Error::new_spanned( + fn_ident, + "`#[skill]` attribute requires a `uri` parameter, e.g. `#[skill(uri = \"skill://my-skill/SKILL.md\")]`", + ) + })?; + + // Validate URI format at compile time (mirrors rmcp::model::skills::parse_skill_uri) + if !uri.starts_with("skill://") || uri.len() <= 8 { + return Err(syn::Error::new_spanned( + fn_ident, + format!( + "`#[skill]` URI must start with `skill://` and contain a path + file (got `{uri}`)" + ), + )); + } + let stripped = &uri[8..]; + if stripped.is_empty() || stripped.ends_with('/') { + return Err(syn::Error::new_spanned( + fn_ident, + format!( + "`#[skill]` URI must not end with `/` and must contain a file path (got `{uri}`)" + ), + )); + } + if !stripped.contains('/') { + return Err(syn::Error::new_spanned( + fn_ident, + format!( + "`#[skill]` URI must contain at least one `/` separating skill path from file (got `{uri}`)" + ), + )); + } + + // Spec:415 — `params.uri` MUST be the URI of a skill's `SKILL.md`. + // Enforce at compile time so invalid URIs fail before deployment. + if !stripped.ends_with("/SKILL.md") { + return Err(syn::Error::new_spanned( + fn_ident, + format!("`#[skill]` URI must end with `/SKILL.md` (got `{uri}`)"), + )); + } + + // Validate frontmatter is present + let frontmatter = attribute.frontmatter.ok_or_else(|| { + syn::Error::new_spanned( + fn_ident, + "`#[skill]` attribute requires a `frontmatter` parameter, e.g. `#[skill(frontmatter = include_str!(\"frontmatter.json\"))]`", + ) + })?; + + let resolved = ResolvedSkillAttribute { + uri, + frontmatter, + dynamic: attribute.dynamic, + }; + let skill_attr_fn = resolved.into_fn(skill_attr_fn_ident.clone())?; + + // Generate the CallSkillHandler wrapper. + // The user's function is the "body" — it takes &self and returns Result<(), ErrorData>. + // The router needs a fn that takes &self + SkillCallContext and returns Result. + // We generate a wrapper that calls the user's fn, then returns the SkillEntry from *_skill_attr(). + // The original function is kept so the wrapper can call it. + let handler_fn_ident = format_ident!("{}_handler", fn_ident); + let skill_attr_fn_ident_clone = skill_attr_fn_ident.clone(); + + let handler_fn = if fn_item.sig.asyncness.is_some() { + quote! { + #[allow(clippy::too_many_arguments)] + fn #handler_fn_ident( + &self, + ctx: rmcp::handler::server::skill::SkillCallContext<'_, Self>, + ) -> ::std::pin::Pin> + Send + 'static>> { + let this = self.clone(); + Box::pin(async move { + let _ = ctx.service; + this.#fn_ident().await?; + Ok(Self::#skill_attr_fn_ident_clone()) + }) + } + } + } else { + quote! { + #[allow(clippy::too_many_arguments)] + fn #handler_fn_ident( + &self, + _ctx: rmcp::handler::server::skill::SkillCallContext<'_, Self>, + ) -> ::std::pin::Pin> + Send + 'static>> { + let this = self.clone(); + Box::pin(async move { + this.#fn_ident()?; + Ok(Self::#skill_attr_fn_ident_clone()) + }) + } + } + }; + + Ok(quote! { + #skill_attr_fn + #fn_item + #handler_fn + }) +} diff --git a/crates/rmcp-macros/src/skill_handler.rs b/crates/rmcp-macros/src/skill_handler.rs new file mode 100644 index 000000000..1cf99e23e --- /dev/null +++ b/crates/rmcp-macros/src/skill_handler.rs @@ -0,0 +1,277 @@ +use darling::{FromMeta, ast::NestedMeta}; +use proc_macro2::TokenStream; +use quote::{ToTokens, quote}; +use syn::{Expr, ImplItem, ItemImpl}; + +use crate::common::{has_method, has_sibling_handler}; + +// Consumed by `darling` macro expansion — not dead code. +#[allow(dead_code)] +#[derive(FromMeta, Debug)] +#[darling(default)] +pub struct SkillHandlerAttribute { + pub router: Expr, + pub meta: Option, + pub name: Option, + pub version: Option, + pub instructions: Option, +} + +impl Default for SkillHandlerAttribute { + fn default() -> Self { + Self { + router: syn::parse2(quote! { Self::skill_router() }).unwrap(), + meta: None, + name: None, + version: None, + instructions: None, + } + } +} + +// Consumed by `darling` macro expansion — not dead code. +#[allow(dead_code)] +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) enum CallerCapability { + Skills, +} + +pub(crate) fn build_get_info( + item_impl: &ItemImpl, + name: Option, + version: Option, + instructions: Option, + caller: CallerCapability, +) -> syn::Result { + let has_skills = + caller == CallerCapability::Skills || has_sibling_handler(item_impl, "skill_handler"); + + let mut capability_calls = Vec::new(); + if has_skills { + capability_calls.push(quote! { .enable_skills_extension_without_directory_read() }); + capability_calls.push(quote! { .enable_resources() }); + } + let server_info_expr = match (name, version) { + (Some(n), Some(v)) => quote! { rmcp::model::Implementation::new(#n, #v) }, + (Some(n), None) => { + quote! { rmcp::model::Implementation::new(#n, env!("CARGO_PKG_VERSION")) } + } + (None, Some(v)) => { + quote! { rmcp::model::Implementation::new(env!("CARGO_CRATE_NAME"), #v) } + } + (None, None) => quote! { rmcp::model::Implementation::from_build_env() }, + }; + + let mut builder_calls = vec![quote! { .with_server_info(#server_info_expr) }]; + if let Some(i) = instructions { + builder_calls.push(quote! { .with_instructions(#i.to_string()) }); + } + + syn::parse2::(quote! { + fn get_info(&self) -> rmcp::model::InitializeResult { + rmcp::model::InitializeResult::new( + rmcp::model::ServerCapabilities::builder() + #(#capability_calls)* + .build() + ) + #(#builder_calls)* + } + }) +} + +/// Generates a `ServerHandler` impl with SEP-2640 skills support. +/// +/// **Limitation:** The `#[skill]` attribute has no file-list parameter, so +/// macro-generated skills cannot declare their files. Every generated entry +/// therefore carries `resources: "dynamic"` — the only value the spec permits +/// when stable digests cannot be published (`skills.mdx:225`; an entry with no +/// `resources` at all is invalid and hosts MUST NOT load it). Consequences: +/// - The host cannot content-bind or verify digests for these skills. +/// - `resources/directory/read` returns `{"resources": []}` for macro skills, +/// so the macro server does not advertise the `directoryRead` capability. +/// +/// Use `#[skill_router]` with explicit `SkillEntry` construction if you need +/// to serve a populated directory listing with content integrity. +pub fn skill_handler(attr: TokenStream, input: TokenStream) -> syn::Result { + let attr_args = NestedMeta::parse_meta_list(attr)?; + let SkillHandlerAttribute { + router, + meta: _, + name, + version, + instructions, + } = SkillHandlerAttribute::from_list(&attr_args)?; + let mut item_impl = syn::parse2::(input)?; + + if !has_method("skills_list", &item_impl) { + let skill_list_fn = syn::parse2::(quote! { + async fn skills_list( + &self, + _request: Option, + context: rmcp::service::RequestContext, + ) -> Result { + let supports_cache_hints = context.protocol_version().is_some_and(|version| { + version >= rmcp::model::ProtocolVersion::V_2026_07_28 + }); + // Spec: the final `` segment of `uri` MUST equal + // `frontmatter.name`. Entries that violate it are invalid, so + // they are not served. + let skills = #router.list_all().into_iter() + .filter(|entry| { + rmcp::model::skills::validate_skill_name(&entry.uri, &entry.frontmatter) + .is_ok() + }) + .collect(); + let mut result = rmcp::model::skills::SkillsListResult::new(skills); + result.result_type = Some(rmcp::model::ResultType::COMPLETE); + result.ttl_ms = supports_cache_hints.then_some(0); + result.cache_scope = supports_cache_hints.then_some("public".to_string()); + Ok(result) + } + })?; + item_impl.items.push(skill_list_fn); + } + + if !has_method("skills_get", &item_impl) { + let skill_get_fn = syn::parse2::(quote! { + async fn skills_get( + &self, + request: rmcp::model::skills::SkillsGetRequestParams, + context: rmcp::service::RequestContext, + ) -> Result { + let router = #router; + let route = router.get_by_uri(&request.uri) + .ok_or_else(|| rmcp::ErrorData::invalid_params( + format!("skill not found: {}", request.uri), + None, + ))?; + // Spec: params.uri MUST be the skill SKILL.md URI. Reject + // any other URI with -32602. The equality check subsumes both + // the directory-form and skill-md-form checks and also rejects + // nested SKILL.md URIs that are not the canonical URI of any + // registered skill. + if request.uri != route.attr.uri { + return Err(rmcp::ErrorData::invalid_params( + format!("not a SKILL.md URI: {}", request.uri), + None, + )); + } + let skill_context = rmcp::handler::server::skill::SkillCallContext::new( + self, + request.uri.clone(), + context, + ); + let skill_entry = (route.call)(skill_context).await?; + // Spec: the final `` segment of `uri` MUST equal + // `frontmatter.name`. + rmcp::model::skills::validate_skill_name(&skill_entry.uri, &skill_entry.frontmatter) + .map_err(|e| rmcp::ErrorData::invalid_params(e, None))?; + Ok(rmcp::model::skills::SkillsGetResult::new(skill_entry)) + } + })?; + item_impl.items.push(skill_get_fn); + } + + if !has_method("resources_directory_read", &item_impl) { + let directory_read_fn = syn::parse2::(quote! { + async fn resources_directory_read( + &self, + request: rmcp::model::skills::ResourcesDirectoryReadRequestParams, + context: rmcp::service::RequestContext, + ) -> Result { + let supports_cache_hints = context.protocol_version().is_some_and(|version| { + version >= rmcp::model::ProtocolVersion::V_2026_07_28 + }); + // The method applies only to directory resources. The skill root + // (`skill://`) is a directory; a file URI such as + // `skill:///SKILL.md` is not, and MUST return -32602. + let router = #router; + let resources: Vec = match router.get_by_uri(&request.uri) { + Some(route) => { + let directory_form = format!( + "skill://{}", + route.skill_path().unwrap_or_default() + ); + if request.uri != directory_form { + return Err(rmcp::ErrorData::invalid_params( + format!("not a directory: {}", request.uri), + None, + )); + } + match &route.attr.resources { + Some(rmcp::model::skills::SkillResources::FileList(files)) => { + // Spec, Directory Listing: "The result contains + // every direct child of the directory." SKILL.md + // is a file directly inside the skill root, so it + // IS a direct child and MUST be listed. + let root = route.attr.uri + .strip_suffix("/SKILL.md") + .or_else(|| route.attr.uri.strip_suffix("SKILL.md")) + .unwrap_or(&route.attr.uri); + let prefix = format!("{}/", root); + // Direct children only: a file maps to itself, a + // nested path maps to its first segment as a + // directory resource. The listing is not recursive. + let mut children: std::collections::BTreeMap = + std::collections::BTreeMap::new(); + for file in files { + let Some(rest) = file.uri.strip_prefix(&prefix) else { + continue; + }; + if rest.is_empty() { + continue; + } + match rest.split_once('/') { + Some((head, _)) => { + children.insert(format!("{}/{}", root, head), true); + } + None => { + children.entry(file.uri.clone()).or_insert(false); + } + } + } + children.into_iter().map(|(uri, is_dir)| { + let name = uri.rsplit('/').next().unwrap_or("").to_string(); + let resource = rmcp::model::Resource::new(uri, name); + if is_dir { + resource.with_mime_type("inode/directory") + } else { + resource + } + }).collect() + } + _ => vec![], + } + } + None => { + // Spec line 453: a URI that does not identify a skill the + // server serves MUST return -32602, not an empty list. + return Err(rmcp::ErrorData::invalid_params( + format!("unknown skill URI: {}", request.uri), + None, + )); + } + }; + let mut result = rmcp::model::skills::ResourcesDirectoryReadResult::new(resources); + result.result_type = Some(rmcp::model::ResultType::COMPLETE); + result.ttl_ms = supports_cache_hints.then_some(0); + result.cache_scope = supports_cache_hints.then_some("public".to_string()); + Ok(result) + } + })?; + item_impl.items.push(directory_read_fn); + } + + if !has_method("get_info", &item_impl) && !has_sibling_handler(&item_impl, "tool_handler") { + let get_info_fn = build_get_info( + &item_impl, + name, + version, + instructions, + CallerCapability::Skills, + )?; + item_impl.items.push(get_info_fn); + } + + Ok(item_impl.into_token_stream()) +} diff --git a/crates/rmcp-macros/src/skill_router.rs b/crates/rmcp-macros/src/skill_router.rs new file mode 100644 index 000000000..f335ca1a7 --- /dev/null +++ b/crates/rmcp-macros/src/skill_router.rs @@ -0,0 +1,113 @@ +//! Skill router proc-macro — the gradebook. +//! +//! Pedagogy: The `#[skill_router]` macro walks the class roster (the impl +//! block), finds every assignment with a `#[skill]` sticker, and builds a +//! gradebook (`SkillRouter`) that maps each skill's URI path to its handler. +//! Like `tool_router`, but the lookup key is a parsed `skill://` URI segment +//! instead of a flat tool name. + +use darling::{FromMeta, ast::NestedMeta}; +use proc_macro2::TokenStream; +use quote::{ToTokens, format_ident, quote}; +use syn::{Ident, ImplItem, ItemImpl, Visibility}; + +// Consumed by `darling` macro expansion — not dead code. +#[allow(dead_code)] +#[derive(FromMeta)] +#[darling(default)] +pub struct SkillRouterAttribute { + pub router: Ident, + pub vis: Option, + pub server_handler: bool, + pub allow_empty: bool, +} + +impl Default for SkillRouterAttribute { + fn default() -> Self { + Self { + router: format_ident!("skill_router"), + vis: None, + server_handler: false, + allow_empty: false, + } + } +} + +pub fn skill_router(attr: TokenStream, input: TokenStream) -> syn::Result { + let attr_args = NestedMeta::parse_meta_list(attr)?; + let SkillRouterAttribute { + router, + vis, + server_handler, + allow_empty, + } = SkillRouterAttribute::from_list(&attr_args)?; + let mut item_impl = syn::parse2::(input)?; + + let skill_attr_fns: Vec<_> = item_impl + .items + .iter() + .filter_map(|item| { + if let syn::ImplItem::Fn(fn_item) = item { + fn_item + .attrs + .iter() + .any(|attr| { + attr.path() + .segments + .last() + .is_some_and(|seg| seg.ident == "skill") + }) + .then_some(&fn_item.sig.ident) + } else { + None + } + }) + .collect(); + + if skill_attr_fns.is_empty() && !allow_empty { + return Err(syn::Error::new_spanned( + &item_impl.self_ty, + format!( + "`#[skill_router]` found no `#[skill]` fn in this impl block, so `Self::{router}()` would serve no skills" + ), + )); + } + + let mut routers = Vec::with_capacity(skill_attr_fns.len()); + for handler in skill_attr_fns { + let skill_attr_fn_ident = format_ident!("{}_skill_attr", handler); + let handler_fn_ident = format_ident!("{}_handler", handler); + routers.push(quote! { + .with_route((Self::#skill_attr_fn_ident(), Self::#handler_fn_ident)) + }); + } + + let router_fn = syn::parse2::(quote! { + #vis fn #router() -> rmcp::handler::server::router::skill::SkillRouter { + rmcp::handler::server::router::skill::SkillRouter::::new() + #(#routers)* + } + })?; + item_impl.items.push(router_fn); + + if !server_handler { + return Ok(item_impl.into_token_stream()); + } + + if item_impl.trait_.is_some() { + return Err(syn::Error::new_spanned( + item_impl, + "`server_handler` is only supported on inherent impl blocks", + )); + } + + let self_ty = &item_impl.self_ty; + let (impl_generics, ty_generics, where_clause) = item_impl.generics.split_for_impl(); + + Ok(quote! { + #item_impl + + #[::rmcp::skill_handler(router = Self::#router())] + impl #impl_generics ::rmcp::ServerHandler for #self_ty #ty_generics #where_clause {} + }) +} diff --git a/crates/rmcp/Cargo.toml b/crates/rmcp/Cargo.toml index 772844d86..e283cba53 100644 --- a/crates/rmcp/Cargo.toml +++ b/crates/rmcp/Cargo.toml @@ -125,7 +125,7 @@ chrono = { version = "0.4.38", default-features = false, features = [ default = ["base64", "macros", "server"] local = ["rmcp-macros?/local"] client = ["dep:tokio-stream"] -server = ["transport-async-rw", "schemars", "dep:pastey", "uuid"] +server = ["transport-async-rw", "schemars", "dep:pastey", "uuid", "dep:sha2"] macros = ["dep:rmcp-macros", "dep:pastey"] elicitation = ["dep:url"] @@ -206,6 +206,7 @@ schemars = { version = "1.1.0", features = ["chrono04"] } axum = { version = "0.8", default-features = false, features = ["http1", "tokio"] } hyper = { version = "1", features = ["server", "http1"] } hyper-util = { version = "0.1", features = ["tokio"] } +serde_yaml = "0.9" tower-service = "0.3" url = "2.4" anyhow = "1.0" @@ -224,6 +225,11 @@ name = "test_tool_macros" required-features = ["server", "client"] path = "tests/test_tool_macros.rs" +[[test]] +name = "test_skill_macros" +required-features = ["server", "macros"] +path = "tests/test_skill_macros.rs" + [[test]] name = "test_with_python" required-features = [ @@ -483,3 +489,7 @@ path = "tests/test_streamable_http_disconnect_cancel.rs" name = "test_sep_2260_stream_routing" required-features = ["server", "elicitation", "transport-streamable-http-server", "reqwest"] path = "tests/test_sep_2260_stream_routing.rs" + +[[example]] +name = "skill_macro_server" +required-features = ["server", "transport-io"] diff --git a/crates/rmcp/examples/skill_fs_client.rs b/crates/rmcp/examples/skill_fs_client.rs new file mode 100644 index 000000000..ca42031a2 --- /dev/null +++ b/crates/rmcp/examples/skill_fs_client.rs @@ -0,0 +1,94 @@ +//! Lightweight MCP client that validates the FileSystemSkillServer. +//! +//! Usage: +//! cargo run --example skill_fs_client --features "client,transport-child-process" -- \ +//! --server-cmd "cargo run --example skill_fs_server --features server,transport-io -- --root ./fixtures/skills-dir" + +use rmcp::{ + ServiceExt, + transport::{ConfigureCommandExt, TokioChildProcess}, +}; +use tokio::process::Command as TokioCommand; + +#[tokio::main] +async fn main() -> Result<(), Box> { + // Parse arguments: --server-cmd [-- ...] + let mut args = std::env::args().skip(1); + let mut server_program = String::new(); + let mut server_args: Vec = Vec::new(); + let mut parsing_program = false; + + while let Some(arg) = args.next() { + if arg == "--server-cmd" { + server_program = args.next().ok_or("--server-cmd requires a value")?; + parsing_program = true; + } else if parsing_program { + server_args.push(arg); + } + } + + if server_program.is_empty() { + server_program = "cargo".to_string(); + server_args = vec![ + "run".to_string(), + "--example".to_string(), + "skill_fs_server".to_string(), + "--features".to_string(), + "server,transport-io".to_string(), + "--".to_string(), + "--root".to_string(), + "./fixtures/skills-dir".to_string(), + ]; + } + + // Spawn the server as a child process without shell + let service = () + .serve(TokioChildProcess::new( + TokioCommand::new(&server_program).configure(|cmd| { + for arg in &server_args { + cmd.arg(arg); + } + }), + )?) + .await?; + + let server_info = service.peer_info(); + println!("Connected to server: {server_info:#?}"); + + // 1. List all skills + println!("\n=== skills/list ==="); + let skills = service.list_all_skills().await?; + for skill in &skills { + println!(" - {}", skill.uri); + println!( + " frontmatter: {}", + serde_json::to_string(&skill.frontmatter).unwrap_or_default() + ); + if let Some(resources) = &skill.resources { + println!(" resources: {:?}", resources); + } + } + println!("Total skills: {}", skills.len()); + + // 2. Get a specific skill + println!("\n=== skills/get (billing/refunds/SKILL.md) ==="); + let single = service + .skills_get("skill://billing/refunds/SKILL.md") + .await?; + println!(" Got: {}", single.skill.uri); + println!( + " frontmatter: {}", + serde_json::to_string(&single.skill.frontmatter).unwrap_or_default() + ); + + // 3. Try an unknown skill + println!("\n=== skills/get (unknown) ==="); + let unknown = service.skills_get("skill://nonexistent/SKILL.md").await; + match unknown { + Err(e) => println!(" Expected error: {e}"), + Ok(_) => println!(" Unexpected success!"), + } + + println!("\n=== Validation complete ==="); + Ok(()) +} diff --git a/crates/rmcp/examples/skill_fs_server.rs b/crates/rmcp/examples/skill_fs_server.rs new file mode 100644 index 000000000..d2f1b0ddd --- /dev/null +++ b/crates/rmcp/examples/skill_fs_server.rs @@ -0,0 +1,384 @@ +//! File-system-backed MCP skill server over stdio. +//! +//! Usage: +//! cargo run --example skill_fs_server --features "server,transport-io" -- --root ./fixtures/skills-dir +//! +//! Then with MCP Inspector: +//! npx @modelcontextprotocol/inspector --cli \ +//! --transport stdio \ +//! --command "cargo run --example skill_fs_server --features server,transport-io -- --root ./fixtures/skills-dir" \ +//! --method skills/list + +use std::{ + collections::HashMap, + env, fs, + path::{Path, PathBuf}, +}; + +use rmcp::{ + ServerHandler, ServiceExt, + model::{ + Implementation, InitializeResult, Resource, ServerCapabilities, + skills::{ + self, SkillEntry, SkillResource, SkillResources, SkillsGetRequestParams, + SkillsGetResult, SkillsListResult, + }, + }, + service::{RequestContext, RoleServer}, +}; + +const SKILL_FILE: &str = "SKILL.md"; +const DEFAULT_CACHE_TTL_MS: u64 = 600_000; + +/// A file-system-backed MCP server that auto-discovers skills from a directory tree. +pub struct FileSystemSkillServer { + root: PathBuf, + skills: HashMap, +} + +impl FileSystemSkillServer { + /// Walk `root` for `**/SKILL.md` files and build the skill catalog. + pub fn new(root: impl AsRef) -> Result { + let root = root.as_ref().to_path_buf(); + let mut skills = HashMap::new(); + Self::walk_dir(&root, &root, &mut skills)?; + Ok(Self { root, skills }) + } + + fn walk_dir( + root: &Path, + dir: &Path, + skills: &mut HashMap, + ) -> Result<(), std::io::Error> { + for entry in fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + if path.is_dir() { + Self::walk_dir(root, &path, skills)?; + } else if path.file_name().and_then(|s| s.to_str()) == Some(SKILL_FILE) + && let Some(skill) = Self::parse_skill_file(root, &path) + { + skills.insert(skill.uri.clone(), skill); + } + } + Ok(()) + } + + fn parse_skill_file(root: &Path, path: &Path) -> Option { + let content = fs::read_to_string(path).ok()?; + let (frontmatter, _) = Self::split_frontmatter(&content)?; + let frontmatter: serde_json::Value = Self::parse_yaml_frontmatter(&frontmatter)?; + + // Canonicalize and verify the path stays within root + let canonical_root = root.canonicalize().ok()?; + let canonical_path = path.canonicalize().ok()?; + if !canonical_path.starts_with(&canonical_root) { + return None; + } + + let relative = canonical_path.strip_prefix(&canonical_root).ok()?; + let uri = format!("skill://{}", relative.to_string_lossy()); + + let parent_dir = canonical_path.parent()?; + let mut resources = Vec::new(); + + // Include SKILL.md itself in the resources list (spec: resources MUST be complete) + let skill_size = fs::metadata(&canonical_path).map(|m| m.len()).unwrap_or(0); + let skill_digest = Self::compute_sha256(&canonical_path); + resources.push(SkillResource::new(uri.clone(), skill_digest, skill_size)); + + // Include all files below the skill root, recursing into subdirectories. + // Spec: "Completeness extends to nested skills. The enclosing skill + // resources lists them too." + let mut files = Vec::new(); + Self::collect_files_recursive(parent_dir, &mut files); + for sibling in files { + if sibling == canonical_path { + continue; + } + let name = sibling.file_name().and_then(|s| s.to_str())?; + if name == SKILL_FILE { + continue; + } + let canonical_sibling = sibling.canonicalize().ok()?; + if !canonical_sibling.starts_with(&canonical_root) { + continue; + } + let sibling_relative = canonical_sibling.strip_prefix(&canonical_root).ok()?; + let sibling_uri = format!("skill://{}", sibling_relative.to_string_lossy()); + let size = fs::metadata(&canonical_sibling) + .map(|m| m.len()) + .unwrap_or(0); + let digest = Self::compute_sha256(&canonical_sibling); + resources.push(SkillResource::new(sibling_uri, digest, size)); + } + + let resources = if resources.is_empty() { + None + } else { + Some(SkillResources::FileList(resources)) + }; + + // meta is None because this is a static file-based server. + // The spec's _meta field (ttlMs, cacheScope) is optional on skills/get + // and only required on skills/list for protocol 2026-07-28+. + // FileSystemSkillServer does not implement caching, so meta is left None. + let mut entry = SkillEntry::new(uri, frontmatter); + entry.resources = resources; + + Some(entry) + } + + /// Recursively collect all files under `dir` into `out`. + /// + /// Used by `parse_skill_file` to enumerate every file below the skill root, + /// including files in nested subdirectories. The spec requires completeness: + /// "The enclosing skill resources lists them too." + fn collect_files_recursive(dir: &Path, out: &mut Vec) { + if let Ok(entries) = fs::read_dir(dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + Self::collect_files_recursive(&path, out); + } else { + out.push(path); + } + } + } + } + + /// Compute SHA-256 digest of a file, returning "sha256:" format. + fn compute_sha256(path: &Path) -> String { + use std::io::Read; + + use sha2::{Digest, Sha256}; + let mut file = match fs::File::open(path) { + Ok(f) => f, + Err(_) => return String::new(), + }; + let mut hasher = Sha256::new(); + let mut buffer = [0u8; 8192]; + loop { + match file.read(&mut buffer) { + Ok(0) => break, + Ok(n) => hasher.update(&buffer[..n]), + Err(_) => return String::new(), + } + } + let result = hasher.finalize(); + // Format as "sha256:" using fmt to avoid needing hex crate + let mut hex_string = String::with_capacity(64); + for byte in result { + use std::fmt::Write; + let _ = write!(hex_string, "{:02x}", byte); + } + format!("sha256:{}", hex_string) + } + + fn split_frontmatter(content: &str) -> Option<(String, String)> { + let content = content.strip_prefix("---\n")?; + let mut lines = content.lines(); + let mut fm = Vec::new(); + for line in &mut lines { + if line.trim() == "---" { + break; + } + fm.push(line); + } + let body: Vec<&str> = lines.collect(); + Some((fm.join("\n"), body.join("\n"))) + } + + /// Parse YAML frontmatter into a JSON object using serde_yaml. + /// + /// Supports nested fields, arrays, numbers, booleans, and all YAML types. + fn parse_yaml_frontmatter(yaml: &str) -> Option { + let value: serde_yaml::Value = serde_yaml::from_str(yaml).ok()?; + serde_json::to_value(value).ok() + } + + /// List all discovered skills, sorted by URI. + pub fn list_skills(&self) -> Vec<&SkillEntry> { + let mut skills: Vec<_> = self.skills.values().collect(); + skills.sort_by(|a, b| a.uri.cmp(&b.uri)); + skills + } + + /// Get a single skill by its URI. + pub fn get_skill(&self, uri: &str) -> Option<&SkillEntry> { + self.skills.get(uri) + } + + /// List the contents of a skill directory or the siblings of a skill file. + /// + /// If the URI names a directory (no trailing slash, last segment is not a + /// file), list its children. If it names a file, list its siblings. + pub fn list_skill_directory(&self, uri: &str) -> Option> { + let path = uri.strip_prefix("skill://")?; + let file_path = self.root.join(path); + + // Canonicalize and verify the path stays within root + let canonical_root = self.root.canonicalize().ok()?; + let canonical_target = file_path.canonicalize().ok()?; + if !canonical_target.starts_with(&canonical_root) { + return None; + } + + // If the target is a directory, read it directly; otherwise read its parent + let dir_to_read = if canonical_target.is_dir() { + canonical_target + } else { + canonical_target.parent()?.to_path_buf() + }; + + let mut entries = Vec::new(); + for entry in fs::read_dir(&dir_to_read).ok()?.flatten() { + let path = entry.path(); + let name = path.file_name()?.to_str()?; + // Spec, Directory Listing: "The result contains every direct child of + // the directory." SKILL.md is a file directly inside the skill root, + // so it IS a direct child and MUST be listed. Do NOT skip it. + // Canonicalize each entry and verify it stays within root + let canonical_entry = path.canonicalize().ok()?; + if !canonical_entry.starts_with(&canonical_root) { + continue; + } + let relative = canonical_entry.strip_prefix(&canonical_root).ok()?; + let entry_uri = format!("skill://{}", relative.to_string_lossy()); + entries.push(if canonical_entry.is_dir() { + Resource::new(entry_uri, name.to_string()).with_mime_type("inode/directory") + } else { + Resource::new(entry_uri, name.to_string()) + }); + } + entries.sort_by(|a, b| a.name.cmp(&b.name)); + Some(entries) + } +} + +impl ServerHandler for FileSystemSkillServer { + /// Per SEP-2640, a server that supports Skills MUST declare both the + /// `resources` capability and the `io.modelcontextprotocol/skills` + /// extension. The hand-written server gets no generated `get_info`, so it + /// must advertise them here or a conformant client never calls `skills/list`. + fn get_info(&self) -> InitializeResult { + InitializeResult::new( + ServerCapabilities::builder() + .enable_resources() + .enable_skills_extension() + .build(), + ) + .with_server_info(Implementation::new( + "skill-fs-server", + env!("CARGO_PKG_VERSION"), + )) + .with_instructions( + "File-system-backed MCP server exposing SEP-2640 skills discovered from a root directory.", + ) + } + + async fn skills_list( + &self, + _request: Option, + _context: RequestContext, + ) -> Result { + let mut skills: Vec = self.skills.values().cloned().collect(); + skills.sort_by(|a, b| a.uri.cmp(&b.uri)); + Ok(SkillsListResult::new(skills)) + } + + async fn skills_get( + &self, + request: SkillsGetRequestParams, + _context: RequestContext, + ) -> Result { + match self.skills.get(&request.uri) { + Some(skill) => Ok(SkillsGetResult::new(skill.clone())), + None => Err(rmcp::ErrorData::invalid_params( + format!("skill not found: {}", request.uri), + None, + )), + } + } + + async fn resources_directory_read( + &self, + request: skills::ResourcesDirectoryReadRequestParams, + context: RequestContext, + ) -> Result { + let resources = self.list_skill_directory(&request.uri).ok_or_else(|| { + rmcp::ErrorData::invalid_params(format!("directory not found: {}", request.uri), None) + })?; + let supports_cache_hints = context + .protocol_version() + .as_ref() + .is_some_and(|v| v.as_str() >= rmcp::model::ProtocolVersion::V_2026_07_28.as_str()); + let mut result = skills::ResourcesDirectoryReadResult::new(resources); + if supports_cache_hints { + result.ttl_ms = Some(DEFAULT_CACHE_TTL_MS); + result.cache_scope = Some("public".to_string()); + } + Ok(result) + } + + async fn read_resource( + &self, + request: rmcp::model::ReadResourceRequestParams, + _context: RequestContext, + ) -> Result { + let path = request.uri.strip_prefix("skill://").ok_or_else(|| { + rmcp::ErrorData::invalid_params(format!("not a skill URI: {}", request.uri), None) + })?; + let file_path = self.root.join(path); + let canonical_root = self.root.canonicalize().map_err(|e| { + rmcp::ErrorData::internal_error(format!("failed to canonicalize root: {}", e), None) + })?; + let canonical_path = file_path.canonicalize().map_err(|_| { + rmcp::ErrorData::invalid_params(format!("resource not found: {}", request.uri), None) + })?; + if !canonical_path.starts_with(&canonical_root) { + return Err(rmcp::ErrorData::invalid_params( + format!("resource outside root: {}", request.uri), + None, + )); + } + let content = fs::read_to_string(&canonical_path).map_err(|_| { + rmcp::ErrorData::invalid_params(format!("resource not found: {}", request.uri), None) + })?; + let mime_type = if canonical_path.is_dir() { + "inode/directory" + } else { + "text/markdown" + }; + Ok(rmcp::model::ReadResourceResponse::Complete( + rmcp::model::ReadResourceResult::new(vec![ + rmcp::model::ResourceContents::TextResourceContents { + uri: request.uri, + mime_type: Some(mime_type.to_string()), + text: content, + meta: None, + }, + ]), + )) + } +} + +#[tokio::main] +async fn main() -> Result<(), Box> { + let mut args = env::args().skip(1); + let mut root = None; + while let Some(arg) = args.next() { + if arg == "--root" { + root = args.next(); + } + } + let root: PathBuf = root + .unwrap_or_else(|| "./fixtures/skills-dir".to_string()) + .into(); + let server = FileSystemSkillServer::new(&root) + .map_err(|e| format!("failed to load skills from {}: {}", root.display(), e))?; + let transport = rmcp::transport::stdio(); + let service = server.serve(transport).await?; + service.waiting().await?; + Ok(()) +} diff --git a/crates/rmcp/examples/skill_macro_server.rs b/crates/rmcp/examples/skill_macro_server.rs new file mode 100644 index 000000000..45c8d0fcc --- /dev/null +++ b/crates/rmcp/examples/skill_macro_server.rs @@ -0,0 +1,53 @@ +//! Example server using `#[skill]` + `#[skill_router]` macros. +//! +//! This server demonstrates the macro-generated `skills_list`, `skills_get`, +//! and `resources_directory_read` methods. It uses a simple in-memory skill +//! registry (no filesystem) so it can be tested without fixture files. + +use rmcp::ServiceExt; + +#[derive(Clone)] +struct MacroServer; + +#[rmcp::skill_router(server_handler = true)] +impl MacroServer { + #[rmcp::skill(uri = "skill://alpha/SKILL.md", frontmatter = serde_json::json!({ + "name": "alpha", + "description": "Alpha skill" + }))] + async fn alpha(&self) -> Result<(), rmcp::ErrorData> { + Ok(()) + } + + #[rmcp::skill(uri = "skill://beta/nested/SKILL.md", frontmatter = serde_json::json!({ + "name": "nested", + "description": "Beta skill with nested path" + }))] + async fn beta(&self) -> Result<(), rmcp::ErrorData> { + Ok(()) + } + + #[rmcp::skill( + uri = "skill://gamma/SKILL.md", + frontmatter = serde_json::json!({ + "name": "gamma", + "description": "Gamma skill with dynamic resources" + }), + dynamic = true + )] + async fn gamma(&self) -> Result<(), rmcp::ErrorData> { + Ok(()) + } +} + +#[tokio::main] +async fn main() -> Result<(), Box> { + let server = MacroServer; + let transport = rmcp::transport::stdio(); + // Hold the `RunningService` for the life of the process. Dropping it + // immediately (as `server.serve(transport).await?;` alone would) cancels + // the serve loop via its DropGuard and the server exits before answering. + let service = server.serve(transport).await?; + service.waiting().await?; + Ok(()) +} diff --git a/crates/rmcp/src/handler/server.rs b/crates/rmcp/src/handler/server.rs index 4abe9ccb4..0562c624c 100644 --- a/crates/rmcp/src/handler/server.rs +++ b/crates/rmcp/src/handler/server.rs @@ -15,6 +15,7 @@ pub mod common; pub mod prompt; mod resource; pub mod router; +pub mod skill; pub mod tool; pub mod tool_name_validation; pub mod wrapper; @@ -144,6 +145,18 @@ impl Service for H { .read_resource(request.params, context) .await .map(ServerResult::from), + ClientRequest::ResourcesDirectoryReadRequest(request) => self + .resources_directory_read(request.params, context) + .await + .map(ServerResult::ResourcesDirectoryReadResult), + ClientRequest::SkillsListRequest(request) => self + .skills_list(request.params, context) + .await + .map(ServerResult::SkillsListResult), + ClientRequest::SkillsGetRequest(request) => self + .skills_get(request.params, context) + .await + .map(ServerResult::SkillsGetResult), ClientRequest::SubscriptionsListenRequest(request) => { if legacy_request { Err(McpError::method_not_found::()) @@ -451,6 +464,32 @@ macro_rules! server_handler_methods { McpError::method_not_found::(), )) } + fn skills_list( + &self, + request: Option, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + std::future::ready(Ok(SkillsListResult::default())) + } + fn skills_get( + &self, + request: SkillsGetRequestParams, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + std::future::ready(Err(McpError::invalid_params( + format!("skill not found: {}", request.uri), + None, + ))) + } + fn resources_directory_read( + &self, + request: ResourcesDirectoryReadRequestParams, + context: RequestContext, + ) -> impl Future> + + MaybeSendFuture + + '_ { + std::future::ready(Err(McpError::method_not_found::())) + } /// Return the subset of a requested notification filter this server accepts. /// /// Returning `None` leaves `subscriptions/listen` unimplemented. The SDK @@ -842,6 +881,31 @@ macro_rules! impl_server_handler_for_wrapper { (**self).get_info() } + fn skills_list( + &self, + request: Option, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + (**self).skills_list(request, context) + } + + fn skills_get( + &self, + request: SkillsGetRequestParams, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + (**self).skills_get(request, context) + } + + fn resources_directory_read( + &self, + request: ResourcesDirectoryReadRequestParams, + context: RequestContext, + ) -> impl Future> + { + (**self).resources_directory_read(request, context) + } + fn get_task( &self, request: GetTaskParams, diff --git a/crates/rmcp/src/handler/server/router.rs b/crates/rmcp/src/handler/server/router.rs index c49c40894..b1ad20b51 100644 --- a/crates/rmcp/src/handler/server/router.rs +++ b/crates/rmcp/src/handler/server/router.rs @@ -1,6 +1,7 @@ use std::{borrow::Cow, sync::Arc}; use prompt::{IntoPromptRoute, PromptRoute}; +use skill::IntoSkillRoute; use tool::{IntoToolRoute, ToolRoute}; use super::ServerHandler; @@ -14,12 +15,14 @@ use crate::{ }; pub mod prompt; +pub mod skill; pub mod tool; #[non_exhaustive] pub struct Router { pub tool_router: tool::ToolRouter, pub prompt_router: prompt::PromptRouter, + pub skill_router: skill::SkillRouter, pub service: Arc, peer_slot: Arc>>, } @@ -35,6 +38,7 @@ where Self { tool_router, prompt_router: prompt::PromptRouter::new(), + skill_router: skill::SkillRouter::new(), service: Arc::new(service), peer_slot, } @@ -69,6 +73,14 @@ where } self } + + pub fn with_skill(mut self, route: R) -> Self + where + R: IntoSkillRoute, + { + self.skill_router.add_route(route.into_skill_route()); + self + } } impl Service for Router @@ -146,6 +158,154 @@ where ..Default::default() })) } + ClientRequest::SkillsListRequest(_) => { + // Spec: the final `` segment of `uri` MUST equal + // `frontmatter.name`. Entries that violate it are invalid, so + // they are not served. + let skills: Vec<_> = self + .skill_router + .list_all() + .into_iter() + .filter(|entry| { + crate::model::skills::validate_skill_name(&entry.uri, &entry.frontmatter) + .is_ok() + }) + .collect(); + let supports_cache_hints = context + .protocol_version() + .is_some_and(|version| version >= crate::model::ProtocolVersion::V_2026_07_28); + Ok(ServerResult::SkillsListResult( + crate::model::skills::SkillsListResult { + result_type: Some(crate::model::ResultType::COMPLETE), + skills, + next_cursor: None, + ttl_ms: supports_cache_hints.then_some(0), + cache_scope: supports_cache_hints.then_some("public".to_string()), + }, + )) + } + ClientRequest::SkillsGetRequest(request) => { + if let Some(route) = self.skill_router.get_by_uri(&request.params.uri) { + // Spec: params.uri MUST be the skill SKILL.md URI. Reject + // any other URI with -32602. The equality check subsumes both + // the directory-form and skill-md-form checks and also rejects + // nested SKILL.md URIs that are not the canonical URI of any + // registered skill. + if request.params.uri == route.attr.uri { + let skill_context = crate::handler::server::skill::SkillCallContext::new( + self.service.as_ref(), + request.params.uri.clone(), + context, + ); + let result = (route.call)(skill_context).await?; + Ok(ServerResult::SkillsGetResult( + crate::model::skills::SkillsGetResult::new(result), + )) + } else { + self.service + .handle_request(ClientRequest::SkillsGetRequest(request), context) + .await + } + } else { + self.service + .handle_request(ClientRequest::SkillsGetRequest(request), context) + .await + } + } + ClientRequest::ResourcesDirectoryReadRequest(request) => { + // The router owns the skill catalog, so it is authoritative for + // any URI that matches a route: answer from the route's declared + // resources. A route with no static file list yields an empty + // array, which is the correct answer for a file-less skill. + if let Some(route) = self.skill_router.get_by_uri(&request.params.uri) { + // Spec: the method applies only to directory resources. The + // skill root (`skill://`) is a directory; a file + // URI such as `skill:///SKILL.md` is not, and + // MUST return -32602. + let directory_form = + format!("skill://{}", route.skill_path().unwrap_or_default()); + if request.params.uri != directory_form { + return Err(crate::ErrorData::invalid_params( + format!("not a directory: {}", request.params.uri), + None, + )); + } + let resources: Vec = match &route.attr.resources { + Some(crate::model::skills::SkillResources::FileList(files)) => { + // Spec, Directory Listing: "The result contains every + // direct child of the directory." SKILL.md is a file + // directly inside the skill root, so it IS a direct + // child and MUST be listed (it is also in the skill's + // own `resources` manifest). + let root = route + .attr + .uri + .strip_suffix("/SKILL.md") + .or_else(|| route.attr.uri.strip_suffix("SKILL.md")) + .unwrap_or(&route.attr.uri); + let prefix = format!("{}/", root); + // Direct children only: a file maps to itself, a + // nested path maps to its first segment as a + // directory resource. The listing is not recursive. + let mut children: std::collections::BTreeMap = + std::collections::BTreeMap::new(); + for file in files { + let Some(rest) = file.uri.strip_prefix(&prefix) else { + continue; + }; + if rest.is_empty() { + continue; + } + match rest.split_once('/') { + Some((head, _)) => { + children.insert(format!("{}/{}", root, head), true); + } + None => { + children.entry(file.uri.clone()).or_insert(false); + } + } + } + children + .into_iter() + .map(|(uri, is_dir)| { + let name = uri.rsplit('/').next().unwrap_or("").to_string(); + let resource = crate::model::Resource::new(uri, name); + if is_dir { + resource.with_mime_type("inode/directory") + } else { + resource + } + }) + .collect() + } + _ => Vec::new(), + }; + let mut result = + crate::model::skills::ResourcesDirectoryReadResult::new(resources); + result.result_type = Some(crate::model::ResultType::COMPLETE); + Ok(ServerResult::ResourcesDirectoryReadResult(result)) + } else if request.params.uri.starts_with("skill://") { + // A `skill://` URI the router does not serve is a request for + // a skill this server does not have. Spec line 453: a + // non-existent URI MUST return `-32602`, the same code + // `resources/read` uses for unknown resources. Delegating to + // the inner service would answer `-32601` (method not found), + // which misreports a missing skill as an unsupported method. + Err(crate::ErrorData::invalid_params( + format!("unknown skill URI: {}", request.params.uri), + None, + )) + } else { + // Not a `skill://` URI — the router has no opinion. Let the + // inner service answer (it may have its own directory logic). + self.service + .handle_request( + ClientRequest::ResourcesDirectoryReadRequest(request), + context, + ) + .await + } + } rest => self.service.handle_request(rest, context).await, } } @@ -156,6 +316,19 @@ where .tools .get_or_insert_with(Default::default) .list_changed = Some(true); + // A `Router` that owns skill routes must advertise the SEP-2640 skills + // extension; otherwise a conformant client never calls `skills/list`. + // The inner service's own capabilities are preserved — we only add the + // extension entry. + if !self.skill_router.map.is_empty() { + info.capabilities.insert_skills_extension(); + // Spec line 389: "A server declaring this extension MUST also + // declare the `resources` capability." Without this, a conformant + // client may reject the server as non-compliant. + info.capabilities + .resources + .get_or_insert_with(Default::default); + } info } @@ -177,6 +350,532 @@ mod tests { struct DummyHandler; impl ServerHandler for DummyHandler {} + /// Dummy skill handler used only to build a `SkillRoute` in tests. + /// + /// A **named `fn` item** (not a closure) is required: `CallSkillHandler` + /// is blanket-implemented for `F: for<'a> Fn(&'a S, SkillCallContext<'a, S>)`, + /// and a fn item is higher-ranked by construction while a closure that + /// mentions the context type is not ("implementation of `FnOnce` is not + /// general enough"). + fn dummy_skill_handler( + _service: &S, + _ctx: crate::handler::server::skill::SkillCallContext, + ) -> std::pin::Pin< + Box< + dyn std::future::Future< + Output = Result, + > + Send + + 'static, + >, + > { + Box::pin(async { + Ok(crate::model::skills::SkillEntry::new( + "skill://dummy/SKILL.md", + serde_json::json!({}), + )) + }) + } + + fn test_request_context() -> crate::service::RequestContext { + let id_provider: Arc = + Arc::new(AtomicU32RequestIdProvider::default()); + let (peer, _rx) = Peer::::new(id_provider, None); + crate::service::RequestContext::new(crate::model::NumberOrString::Number(1), peer) + } + + /// **Fix #1** — `resources/directory/read` through the `Router` must answer + /// from the matching route's declared file list, not a hardcoded empty vec. + /// + /// Goes RED before the fix (hardcoded `resources: vec![]` → len 0) and GREEN + /// after. A route with a real `FileList` is required: a file-less route + /// returns an empty array either way and cannot distinguish the bug. + #[tokio::test] + async fn test_router_directory_read_delegates_to_file_list_route() { + use crate::model::{ + ClientRequest, + skills::{ + ResourcesDirectoryReadRequest, ResourcesDirectoryReadRequestParams, SkillEntry, + SkillResource, SkillResources, + }, + }; + + let mut entry = SkillEntry::new( + "skill://alpha/SKILL.md", + serde_json::json!({"name": "alpha", "description": "Alpha skill"}), + ); + entry.resources = Some(SkillResources::FileList(vec![ + SkillResource::new("skill://alpha/SKILL.md", "digest-a", 10), + SkillResource::new("skill://alpha/APPENDIX.md", "digest-b", 20), + ])); + + let mut router = Router::new(DummyHandler); + router + .skill_router + .add_route(skill::SkillRoute::new(entry, dummy_skill_handler)); + + let request = + ClientRequest::ResourcesDirectoryReadRequest(ResourcesDirectoryReadRequest::new( + ResourcesDirectoryReadRequestParams::new("skill://alpha"), + )); + + let result = router + .handle_request(request, test_request_context()) + .await + .expect("router must answer resources/directory/read"); + + let ServerResult::ResourcesDirectoryReadResult(result) = result else { + panic!("expected ResourcesDirectoryReadResult, got {result:?}"); + }; + // The route declares SKILL.md (its own root) and APPENDIX.md. A directory + // listing carries every direct child, and SKILL.md is a file directly + // inside the skill root, so it IS a direct child and is listed. + assert_eq!( + result.resources.len(), + 2, + "router must delegate the route's declared file list, including \ + SKILL.md as a direct child; got {:?}", + result.resources + ); + let uris: Vec<&str> = result.resources.iter().map(|r| r.uri.as_str()).collect(); + assert!( + uris.contains(&"skill://alpha/APPENDIX.md"), + "delegated list must include the route's files; got {uris:?}" + ); + assert!( + uris.contains(&"skill://alpha/SKILL.md"), + "SKILL.md is a direct child of the skill root and MUST be listed; got {uris:?}" + ); + } + + /// **Fix #4** — a directory-form URI (`skill://alpha`, no file component) + /// must resolve to its route, not just the file form (`skill://alpha/SKILL.md`). + /// + /// Goes RED before the fix (`uri.starts_with("skill://alpha/")` is false for + /// the bare directory form) and GREEN after. This is the routing layer + /// `resources/directory/read` depends on. + #[tokio::test] + async fn test_skill_router_matches_directory_form_uri() { + use crate::model::skills::SkillEntry; + + let entry = SkillEntry::new( + "skill://alpha/SKILL.md", + serde_json::json!({"name": "alpha", "description": "Alpha skill"}), + ); + let mut router = skill::SkillRouter::::new(); + router.add_route(skill::SkillRoute::new(entry, dummy_skill_handler)); + + assert!( + router.get_by_uri("skill://alpha/SKILL.md").is_some(), + "file-form URI must match" + ); + assert!( + router.get_by_uri("skill://alpha").is_some(), + "directory-form URI must match after fix" + ); + assert!( + router.get_by_uri("skill://nonexistent").is_none(), + "non-matching URI must not match" + ); + } + + /// **D1/D8** — a `Router` that owns skill routes must advertise BOTH the + /// SEP-2640 skills extension AND the `resources` capability. + /// + /// Spec line 389: *"A server declaring this extension MUST also declare the + /// `resources` capability."* Goes RED before the fix (only the extension is + /// written), GREEN after. The second half is a regression guard: a router + /// with no skill routes must stay byte-identical to the inner service. + #[tokio::test] + async fn test_router_get_info_advertises_skills_extension_and_resources() { + use crate::model::skills::SkillEntry; + // No routes -> must not add the resources capability. + use crate::service::Service; + let bare = Router::new(DummyHandler); + let bare_info = bare.get_info(); + assert!( + bare_info.capabilities.resources.is_none(), + "a router with no skill routes must not add the resources capability; got {:?}", + bare_info.capabilities + ); + + // With a route -> extension entry AND the resources capability. + let entry = SkillEntry::new( + "skill://alpha/SKILL.md", + serde_json::json!({"name": "alpha", "description": "Alpha skill"}), + ); + let mut router = Router::new(DummyHandler); + router + .skill_router + .add_route(skill::SkillRoute::new(entry, dummy_skill_handler)); + + let info = router.get_info(); + assert!( + info.capabilities.resources.is_some(), + "spec line 389: declaring the skills extension MUST also declare `resources`; got {:?}", + info.capabilities + ); + let ext = info + .capabilities + .extensions + .as_ref() + .and_then(|e| e.get(crate::model::SKILLS_EXTENSION_ID)) + .expect("skills extension must be advertised when skill routes exist"); + assert_eq!(ext["directoryRead"], serde_json::json!(true)); + } + + /// **D2** — a directory read returns every *direct* child: files as ordinary + /// resources, subdirectories as directory resources (`mimeType: + /// "inode/directory"`). The listing is not recursive. + /// + /// Spec: "The result contains every direct child of the directory: files + /// with their ordinary resource metadata, subdirectories listed as + /// directory resources (`mimeType: "inode/directory"`). The listing is not + /// recursive; clients descend by calling the method again on a child + /// directory." + #[tokio::test] + async fn test_router_directory_read_lists_direct_children_with_directory_mime() { + use crate::{ + model::{ + ClientRequest, + skills::{ + ResourcesDirectoryReadRequest, ResourcesDirectoryReadRequestParams, SkillEntry, + SkillResource, SkillResources, + }, + }, + service::Service, + }; + + let mut entry = SkillEntry::new( + "skill://pdf-processing/SKILL.md", + serde_json::json!({"name": "pdf-processing", "description": "PDF skill"}), + ); + entry.resources = Some(SkillResources::FileList(vec![ + // The skill's own SKILL.md — a direct child of the skill root. + SkillResource::new("skill://pdf-processing/SKILL.md", "d0", 1), + // Direct child file. + SkillResource::new("skill://pdf-processing/README.md", "d1", 2), + // Two files under templates/ — must collapse to ONE directory child. + SkillResource::new("skill://pdf-processing/templates/invoice.md", "d2", 3), + SkillResource::new("skill://pdf-processing/templates/order.md", "d3", 4), + ])); + + let mut router = Router::new(DummyHandler); + router + .skill_router + .add_route(skill::SkillRoute::new(entry, dummy_skill_handler)); + + let request = + ClientRequest::ResourcesDirectoryReadRequest(ResourcesDirectoryReadRequest::new( + ResourcesDirectoryReadRequestParams::new("skill://pdf-processing"), + )); + + let result = router + .handle_request(request, test_request_context()) + .await + .expect("router must answer resources/directory/read"); + let ServerResult::ResourcesDirectoryReadResult(result) = result else { + panic!("expected ResourcesDirectoryReadResult, got {result:?}"); + }; + + let uris: Vec<&str> = result.resources.iter().map(|r| r.uri.as_str()).collect(); + assert_eq!( + uris, + vec![ + "skill://pdf-processing/README.md", + "skill://pdf-processing/SKILL.md", + "skill://pdf-processing/templates" + ], + "must list every direct child (SKILL.md included, templates/ collapsed); got {uris:?}" + ); + + // The subdirectory child must be marked as a directory resource. + let dir_child = result + .resources + .iter() + .find(|r| r.uri == "skill://pdf-processing/templates") + .expect("templates must appear as a child"); + assert_eq!( + dir_child.mime_type.as_deref(), + Some("inode/directory"), + "subdirectory children must carry mimeType inode/directory" + ); + + // The plain file child must NOT be marked as a directory. + let file_child = result + .resources + .iter() + .find(|r| r.uri == "skill://pdf-processing/README.md") + .expect("README.md must appear as a child"); + assert_ne!( + file_child.mime_type.as_deref(), + Some("inode/directory"), + "plain files must not be marked inode/directory" + ); + } + + /// **D5** — the `frontmatter.name` ↔ URI-final-segment invariant is enforced, + /// not merely declared. + /// + /// Spec: "The final `` segment of the entry's `uri` MUST equal + /// `frontmatter.name`." `validate_skill_name` existed but had no non-test + /// caller, so an entry that violated the invariant was served anyway. A + /// violating entry is invalid, so it must not appear in `skills/list`. + #[tokio::test] + async fn test_router_skills_list_filters_name_uri_mismatch() { + use crate::{ + model::{ + ClientRequest, PaginatedRequestParams, RequestOptionalParam, skills::SkillEntry, + }, + service::Service, + }; + + // Conforming: uri final segment "alpha" == frontmatter.name "alpha". + let good = SkillEntry::new( + "skill://alpha/SKILL.md", + serde_json::json!({"name": "alpha", "description": "Alpha skill"}), + ); + // Non-conforming: uri final segment "beta" != frontmatter.name "wrong". + let bad = SkillEntry::new( + "skill://beta/SKILL.md", + serde_json::json!({"name": "wrong", "description": "Mismatched skill"}), + ); + + let mut router = Router::new(DummyHandler); + router + .skill_router + .add_route(skill::SkillRoute::new(good, dummy_skill_handler)); + router + .skill_router + .add_route(skill::SkillRoute::new(bad, dummy_skill_handler)); + + // Both routes are registered; the invariant is enforced at serve time. + assert_eq!( + router.skill_router.list_all().len(), + 2, + "both routes must be registered for this test to be meaningful" + ); + + let request = ClientRequest::SkillsListRequest(RequestOptionalParam::with_param( + PaginatedRequestParams::default(), + )); + let result = router + .handle_request(request, test_request_context()) + .await + .expect("skills/list must succeed"); + let ServerResult::SkillsListResult(result) = result else { + panic!("expected SkillsListResult, got {result:?}"); + }; + + let uris: Vec<&str> = result.skills.iter().map(|s| s.uri.as_str()).collect(); + assert_eq!( + uris, + vec!["skill://alpha/SKILL.md"], + "the entry whose frontmatter.name disagrees with its URI must not be served; got {uris:?}" + ); + } + + /// **D4** — Router `resources/directory/read` must return `-32602` for + /// unknown `skill://` URIs, not delegate to the inner service. + /// + /// Spec line 453: "A non-existent or non-directory URI MUST return `-32602`." + /// RED before fix (delegates to inner service → -32601), GREEN after. + #[tokio::test] + async fn test_router_directory_read_returns_32602_for_unknown_skill_uri() { + use crate::{ + model::{ + ClientRequest, + skills::{ + ResourcesDirectoryReadRequest, ResourcesDirectoryReadRequestParams, SkillEntry, + }, + }, + service::Service, + }; + + let entry = SkillEntry::new( + "skill://alpha/SKILL.md", + serde_json::json!({"name": "alpha", "description": "Alpha skill"}), + ); + let mut router = Router::new(DummyHandler); + router + .skill_router + .add_route(skill::SkillRoute::new(entry, dummy_skill_handler)); + + let request = + ClientRequest::ResourcesDirectoryReadRequest(ResourcesDirectoryReadRequest::new( + ResourcesDirectoryReadRequestParams::new("skill://nonexistent"), + )); + + let result = router.handle_request(request, test_request_context()).await; + match result { + Err(e) => assert_eq!( + e.code, + crate::model::ErrorCode(-32602), + "must return -32602 for unknown skill:// URI, got {:?}", + e.code + ), + Ok(r) => panic!("expected error, got {:?}", r), + } + } + + /// **D4 scheme guard** — non-`skill://` URIs must delegate to inner service, + /// not get -32602. A non-skill URI is not the router's responsibility. + #[tokio::test] + async fn test_router_directory_read_delegates_non_skill_uri() { + use crate::{ + model::{ + ClientRequest, + skills::{ + ResourcesDirectoryReadRequest, ResourcesDirectoryReadRequestParams, SkillEntry, + }, + }, + service::Service, + }; + + let entry = SkillEntry::new( + "skill://alpha/SKILL.md", + serde_json::json!({"name": "alpha", "description": "Alpha skill"}), + ); + let mut router = Router::new(DummyHandler); + router + .skill_router + .add_route(skill::SkillRoute::new(entry, dummy_skill_handler)); + + let request = + ClientRequest::ResourcesDirectoryReadRequest(ResourcesDirectoryReadRequest::new( + ResourcesDirectoryReadRequestParams::new("file:///etc/passwd"), + )); + + let result = router.handle_request(request, test_request_context()).await; + match result { + Err(e) => assert_eq!( + e.code, + crate::model::ErrorCode(-32601), + "non-skill URI must delegate to inner service (got -32601), got {:?}", + e.code + ), + Ok(r) => panic!("expected delegation error, got {:?}", r), + } + } + + /// **D10** — Router `skills/list` must set cache hints when the protocol + /// version is 2026-07-28+, matching the macro path's behavior. + /// + /// Spec line 229: "In protocol versions 2026-07-28 and later, the result + /// also carries the base protocol's list-caching attributes." + /// Repo contract (skills.rs:63-64): "REQUIRED on skills/list results in + /// protocol version 2026-07-28+." + /// + /// RED before fix (ttl_ms/cache_scope are None). GREEN after. + #[tokio::test] + async fn test_router_skills_list_sets_cache_hints_on_new_protocol() { + use crate::{model::skills::SkillEntry, service::Service}; + + let entry = SkillEntry::new( + "skill://alpha/SKILL.md", + serde_json::json!({"name": "alpha", "description": "Alpha skill"}), + ); + let mut router = Router::new(DummyHandler); + router + .skill_router + .add_route(skill::SkillRoute::new(entry, dummy_skill_handler)); + + // Build a RequestContext with protocol version 2026-07-28. + let (peer, _rx) = crate::service::Peer::new( + std::sync::Arc::new(crate::service::AtomicU32RequestIdProvider::default()), + None, + ); + let mut meta = crate::model::RequestMetaObject::default(); + meta.set_protocol_version(crate::model::ProtocolVersion::V_2026_07_28); + let context = crate::service::RequestContext { + ct: tokio_util::sync::CancellationToken::new(), + id: crate::model::NumberOrString::String("test".to_string().into()), + meta, + extensions: crate::model::Extensions::default(), + peer, + }; + + let request = crate::model::ClientRequest::SkillsListRequest( + crate::model::RequestOptionalParam::with_param( + crate::model::PaginatedRequestParams::default(), + ), + ); + + let result = router + .handle_request(request, context) + .await + .expect("must succeed"); + match result { + crate::model::ServerResult::SkillsListResult(r) => { + assert!( + r.ttl_ms.is_some(), + "ttl_ms must be set on skills/list in protocol 2026-07-28+; got {:?}", + r.ttl_ms + ); + assert!( + r.cache_scope.is_some(), + "cache_scope must be set on skills/list in protocol 2026-07-28+; got {:?}", + r.cache_scope + ); + } + other => panic!("expected SkillsListResult, got {:?}", other), + } + } + + /// **D10 negative** — Router `skills/list` must NOT set cache hints when the + /// protocol version is older than 2026-07-28. + #[tokio::test] + async fn test_router_skills_list_no_cache_hints_on_old_protocol() { + use crate::{model::skills::SkillEntry, service::Service}; + + let entry = SkillEntry::new( + "skill://alpha/SKILL.md", + serde_json::json!({"name": "alpha", "description": "Alpha skill"}), + ); + let mut router = Router::new(DummyHandler); + router + .skill_router + .add_route(skill::SkillRoute::new(entry, dummy_skill_handler)); + + // Default meta = no protocol version set. + let (peer, _rx) = crate::service::Peer::new( + std::sync::Arc::new(crate::service::AtomicU32RequestIdProvider::default()), + None, + ); + let context = crate::service::RequestContext { + ct: tokio_util::sync::CancellationToken::new(), + id: crate::model::NumberOrString::String("test".to_string().into()), + meta: crate::model::RequestMetaObject::default(), + extensions: crate::model::Extensions::default(), + peer, + }; + + let request = crate::model::ClientRequest::SkillsListRequest( + crate::model::RequestOptionalParam::with_param( + crate::model::PaginatedRequestParams::default(), + ), + ); + + let result = router + .handle_request(request, context) + .await + .expect("must succeed"); + match result { + crate::model::ServerResult::SkillsListResult(r) => { + assert!( + r.ttl_ms.is_none(), + "ttl_ms must be None without protocol version; got {:?}", + r.ttl_ms + ); + assert!( + r.cache_scope.is_none(), + "cache_scope must be None without protocol version; got {:?}", + r.cache_scope + ); + } + other => panic!("expected SkillsListResult, got {:?}", other), + } + } + async fn recv_notification( rx: &mut tokio::sync::mpsc::Receiver>, ) -> ServerNotification { diff --git a/crates/rmcp/src/handler/server/router/skill.rs b/crates/rmcp/src/handler/server/router/skill.rs new file mode 100644 index 000000000..f8080180d --- /dev/null +++ b/crates/rmcp/src/handler/server/router/skill.rs @@ -0,0 +1,191 @@ +//! Skills router — the gradebook that looks up by parsed URI path. +//! +//! Pedagogy: `ToolRouter` is like a phone book (look up by name). +//! `SkillRouter` is like a DMV database — you can't just say "Alice," +//! you have to hand over the full ID number. The router parses the URI +//! (`skill://acme/billing/refunds/SKILL.md` → skill path `acme/billing/refunds`), +//! then matches against its internal `HashMap`. + +use std::{borrow::Cow, sync::Arc}; + +use crate::{ + handler::server::skill::{CallSkillHandler, DynCallSkillHandler, SkillCallContext}, + model::skills::SkillEntry, + service::MaybeSend, +}; + +#[non_exhaustive] +pub struct SkillRoute { + pub call: Arc>, + pub attr: SkillEntry, +} + +impl std::fmt::Debug for SkillRoute { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SkillRoute") + .field("uri", &self.attr.uri) + .field("resources", &self.attr.resources) + .finish() + } +} + +impl Clone for SkillRoute { + fn clone(&self) -> Self { + Self { + call: self.call.clone(), + attr: self.attr.clone(), + } + } +} + +impl SkillRoute { + pub fn new(attr: impl Into, call: C) -> Self + where + C: CallSkillHandler + MaybeSend + Clone + 'static, + { + Self { + call: Arc::new(move |context: SkillCallContext| { + let call = call.clone(); + let service = context.service; + call.call(service, context) + }), + attr: attr.into(), + } + } + + pub fn uri(&self) -> &str { + &self.attr.uri + } + + /// Extract the skill path from this route's URI (e.g. `acme/billing/refunds`). + pub fn skill_path(&self) -> Option { + crate::model::skills::skill_path_from_uri(&self.attr.uri) + } +} + +pub trait IntoSkillRoute { + fn into_skill_route(self) -> SkillRoute; +} + +impl IntoSkillRoute for (T, C) +where + S: MaybeSend + Clone + 'static, + C: CallSkillHandler + MaybeSend + Clone + 'static, + T: Into, +{ + fn into_skill_route(self) -> SkillRoute { + SkillRoute::new(self.0.into(), self.1) + } +} + +impl IntoSkillRoute for SkillRoute +where + S: MaybeSend + 'static, +{ + fn into_skill_route(self) -> SkillRoute { + self + } +} + +#[non_exhaustive] +pub struct SkillAttrGenerateFunctionAdapter; + +impl IntoSkillRoute for F +where + S: MaybeSend + 'static, + F: Fn() -> SkillRoute, +{ + fn into_skill_route(self) -> SkillRoute { + (self)() + } +} + +#[derive(Debug)] +#[non_exhaustive] +pub struct SkillRouter { + pub map: std::collections::HashMap, SkillRoute>, +} + +impl Default for SkillRouter { + fn default() -> Self { + Self { + map: std::collections::HashMap::new(), + } + } +} + +impl Clone for SkillRouter { + fn clone(&self) -> Self { + Self { + map: self.map.clone(), + } + } +} + +impl SkillRouter +where + S: MaybeSend + 'static, +{ + pub fn new() -> Self { + Self::default() + } + + pub fn with_route(mut self, route: R) -> Self + where + R: IntoSkillRoute, + { + self.add_route(route.into_skill_route()); + self + } + + pub fn add_route(&mut self, item: SkillRoute) { + if let Some(path) = item.skill_path() { + self.map.insert(Cow::Owned(path), item); + } + } + + pub fn merge(&mut self, other: SkillRouter) { + for item in other.map.into_values() { + self.add_route(item); + } + } + + /// Match a request URI to its route using longest-prefix matching. + /// + /// Skill roots can be multi-segment (`acme/billing/refunds`) and file paths + /// can nest arbitrarily deep (`templates/x.md`). There is no in-URI delimiter + /// between skill-path and file-path, so we cannot parse the URI alone. + /// Instead, we find the registered route whose skill root is the longest + /// prefix of the request URI's skill root. + pub fn get_by_uri(&self, uri: &str) -> Option<&SkillRoute> { + let stripped = uri.strip_prefix("skill://")?; + if stripped.is_empty() { + return None; + } + // For each registered route, extract its skill path (e.g. `acme/billing/refunds`) + // and check if the request URI is either the directory form + // `skill://` (no trailing slash, no file component — this is + // what `resources/directory/read` sends) or starts with + // `skill:///` (an exact SKILL.md match or a nested file URI). + let mut best: Option<(&Cow<'static, str>, &SkillRoute)> = None; + for (key, route) in &self.map { + if let Some(route_skill_path) = route.skill_path() { + let prefix = format!("skill://{}/", route_skill_path); + let directory_form = format!("skill://{}", route_skill_path); + if uri == directory_form || uri.starts_with(&prefix) { + match best { + Some((best_key, _)) if best_key.len() >= key.len() => {} + _ => best = Some((key, route)), + } + } + } + } + best.map(|(_, route)| route) + } + + pub fn list_all(&self) -> Vec { + let mut skills: Vec<_> = self.map.values().map(|r| r.attr.clone()).collect(); + skills.sort_by(|a, b| a.uri.cmp(&b.uri)); + skills + } +} diff --git a/crates/rmcp/src/handler/server/skill.rs b/crates/rmcp/src/handler/server/skill.rs new file mode 100644 index 000000000..57fd8bf44 --- /dev/null +++ b/crates/rmcp/src/handler/server/skill.rs @@ -0,0 +1,87 @@ +//! Skill handler support — analogous to `tool.rs` for tools. +//! +//! Pedagogy: The skill call context is like a hall pass. When a skill +//! request comes in, the router checks the gradebook, finds the matching +//! route, and hands the handler a "hall pass" (SkillCallContext) that +//! says "you are allowed to run skill X with URI Y." + +use crate::{ + model::skills::SkillEntry, + service::{MaybeBoxFuture, RequestContext, RoleServer}, +}; + +/// Context passed to a skill handler when invoked. +#[non_exhaustive] +pub struct SkillCallContext<'a, S> { + pub service: &'a S, + pub uri: String, + pub context: RequestContext, +} + +impl<'a, S> SkillCallContext<'a, S> { + pub fn new(service: &'a S, uri: String, context: RequestContext) -> Self { + Self { + service, + uri, + context, + } + } + + /// Get the skill path from the URI (e.g. "acme/billing/refunds"). + pub fn skill_path(&self) -> Option { + crate::model::skills::skill_path_from_uri(&self.uri) + } +} + +/// A skill handler fn: takes a service reference and call context, +/// returns the skill's metadata. +pub trait CallSkillHandler: Send + Sync + Clone + 'static { + fn call( + &self, + service: &S, + context: SkillCallContext, + ) -> ::std::pin::Pin< + Box< + dyn ::std::future::Future> + + Send + + 'static, + >, + >; +} + +impl CallSkillHandler for F +where + F: for<'a> Fn( + &'a S, + SkillCallContext<'a, S>, + ) -> ::std::pin::Pin< + Box< + dyn ::std::future::Future> + + Send + + 'static, + >, + > + Send + + Sync + + Clone + + 'static, + S: crate::service::MaybeSend + 'static, +{ + fn call( + &self, + service: &S, + context: SkillCallContext, + ) -> ::std::pin::Pin< + Box< + dyn ::std::future::Future> + + Send + + 'static, + >, + > { + self(service, context) + } +} + +/// Type-erased skill call handler for storage in SkillRouter. +pub type DynCallSkillHandler = dyn for<'a> Fn(SkillCallContext<'a, S>) -> MaybeBoxFuture<'a, Result> + + Send + + Sync; diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index 5a2021d17..a7cd20a82 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -18,6 +18,7 @@ mod prompt; mod request_state; mod resource; mod serde_impl; +pub mod skills; mod task; mod tool; pub use annotated::*; @@ -33,6 +34,7 @@ pub use request_state::*; pub use resource::*; use serde::{Deserialize, Serialize, de::DeserializeOwned}; use serde_json::Value; +pub use skills::*; pub use task::*; pub use tool::*; @@ -4650,6 +4652,9 @@ ts_union!( | ListResourcesRequest | ListResourceTemplatesRequest | ReadResourceRequest + | ResourcesDirectoryReadRequest + | SkillsGetRequest + | SkillsListRequest | SubscriptionsListenRequest | SubscribeRequest | UnsubscribeRequest @@ -4674,6 +4679,9 @@ impl ClientRequest { ClientRequest::ListResourcesRequest(r) => r.method.as_str(), ClientRequest::ListResourceTemplatesRequest(r) => r.method.as_str(), ClientRequest::ReadResourceRequest(r) => r.method.as_str(), + ClientRequest::ResourcesDirectoryReadRequest(r) => r.method.as_str(), + ClientRequest::SkillsGetRequest(r) => r.method.as_str(), + ClientRequest::SkillsListRequest(r) => r.method.as_str(), ClientRequest::SubscriptionsListenRequest(r) => r.method.as_str(), ClientRequest::SubscribeRequest(r) => r.method.as_str(), ClientRequest::UnsubscribeRequest(r) => r.method.as_str(), @@ -4687,6 +4695,18 @@ impl ClientRequest { } } +impl ServerRequest { + pub fn method(&self) -> &str { + match &self { + ServerRequest::PingRequest(r) => r.method.as_str(), + ServerRequest::CreateMessageRequest(r) => r.method.as_str(), + ServerRequest::ListRootsRequest(r) => r.method.as_str(), + ServerRequest::ElicitRequest(r) => r.method.as_str(), + ServerRequest::CustomRequest(r) => r.method.as_str(), + } + } +} + ts_union!( export type ClientNotification = | CancelledNotification @@ -4746,6 +4766,9 @@ ts_union!( | ListResourcesResult | ListResourceTemplatesResult | ReadResourceResult + | ResourcesDirectoryReadResult + | SkillsGetResult + | SkillsListResult | SubscriptionsListenResult | ListToolsResult | ElicitResult @@ -4802,11 +4825,12 @@ impl ServerResult { ServerResult::ListResourcesResult(r) => &mut r.result_type, ServerResult::ListResourceTemplatesResult(r) => &mut r.result_type, ServerResult::ReadResourceResult(r) => &mut r.result_type, + ServerResult::ResourcesDirectoryReadResult(r) => &mut r.result_type, ServerResult::ListToolsResult(r) => &mut r.result_type, ServerResult::CallToolResult(r) => &mut r.result_type, _ => return, }; - result_type.take_if(|result_type| result_type.is_complete()); + result_type.take_if(|rt| rt.is_complete()); } /// Fill in the SEP-2549 caching hints a cacheable result is missing. @@ -4837,6 +4861,10 @@ impl ServerResult { /// assert_eq!(result.cache_scope, Some(CacheScope::Private)); /// ``` pub fn fill_missing_cache_hints(&mut self) { + // `cache_scope` is `Option` on most result types but + // `Option` on the two SEP-2640 skill results (SkillsListResult + // and ResourcesDirectoryReadResult). A single `match` cannot mix both + // types, so handle the two groups separately. let (result_type, ttl_ms, cache_scope) = match self { ServerResult::ListToolsResult(r) => (&r.result_type, &mut r.ttl_ms, &mut r.cache_scope), ServerResult::ListPromptsResult(r) => { @@ -4851,6 +4879,27 @@ impl ServerResult { ServerResult::ReadResourceResult(r) => { (&r.result_type, &mut r.ttl_ms, &mut r.cache_scope) } + ServerResult::SkillsGetResult(r) => { + return fill_missing_cache_hints_for_string_scope( + &r.result_type, + &mut r.ttl_ms, + &mut r.cache_scope, + ); + } + ServerResult::SkillsListResult(r) => { + return fill_missing_cache_hints_for_string_scope( + &r.result_type, + &mut r.ttl_ms, + &mut r.cache_scope, + ); + } + ServerResult::ResourcesDirectoryReadResult(r) => { + return fill_missing_cache_hints_for_string_scope( + &r.result_type, + &mut r.ttl_ms, + &mut r.cache_scope, + ); + } _ => return, }; // Only complete results are cacheable; the spec treats an absent @@ -4863,6 +4912,20 @@ impl ServerResult { } } +/// Helper for `fill_missing_cache_hints` on SEP-2640 skill result types whose +/// `cache_scope` is `Option` rather than `Option`. +fn fill_missing_cache_hints_for_string_scope( + result_type: &Option, + ttl_ms: &mut Option, + cache_scope: &mut Option, +) { + if result_type.as_ref().is_some_and(|t| !t.is_complete()) { + return; + } + ttl_ms.get_or_insert(0); + cache_scope.get_or_insert("public".to_string()); +} + pub type ServerJsonRpcMessage = JsonRpcMessage; impl TryInto for ServerNotification { diff --git a/crates/rmcp/src/model/capabilities.rs b/crates/rmcp/src/model/capabilities.rs index f014f569f..1d69a6344 100644 --- a/crates/rmcp/src/model/capabilities.rs +++ b/crates/rmcp/src/model/capabilities.rs @@ -60,6 +60,9 @@ pub struct ToolsCapability { pub list_changed: Option, } +/// Extension identifier for the SEP-2640 Skills extension. +pub const SKILLS_EXTENSION_ID: &str = "io.modelcontextprotocol/skills"; + /// Roots capability. Deprecated by SEP-2577; remains functional and will be /// removed in a future release. /// See . @@ -250,6 +253,44 @@ impl ServerCapabilities { .as_ref() .is_some_and(|e| e.contains_key(super::TASKS_EXTENSION_ID)) } + + /// Advertise the `io.modelcontextprotocol/skills` extension (SEP-2640) in + /// the `extensions` capability map with `directoryRead: true`. + /// + /// In-place equivalent of the `enable_skills_extension()` builder method, + /// for code that already holds a `ServerCapabilities` value (e.g. a + /// `Router` forwarding its inner service's capabilities). + /// + /// Gated to `server` (not `macros`): its only caller is the server-gated + /// `Router`, so under a `macros`-only build it would be dead code. + #[cfg(feature = "server")] + pub(crate) fn insert_skills_extension(&mut self) { + insert_skills_extension_into(&mut self.extensions, true); + } +} + +/// Write the `io.modelcontextprotocol/skills` extension entry (SEP-2640) into +/// an `extensions` capability map. +/// +/// `directory_read: true` advertises support for [`resources/directory/read`]. +/// Pass `false` (or omit) when the server does not implement directory listing +/// for skill directories — clients MUST NOT call `resources/directory/read` +/// without this declaration. +/// +/// Shared by [`ServerCapabilities::insert_skills_extension`] (in-place) and the +/// `enable_skills_extension()` builder method so the two cannot drift. +#[cfg(any(feature = "server", feature = "macros"))] +pub(crate) fn insert_skills_extension_into( + extensions: &mut Option, + directory_read: bool, +) { + let mut ext = JsonObject::new(); + if directory_read { + ext.insert("directoryRead".to_string(), serde_json::json!(true)); + } + extensions + .get_or_insert_with(ExtensionCapabilities::new) + .insert(super::SKILLS_EXTENSION_ID.to_string(), ext); } #[cfg(any(feature = "server", feature = "macros"))] @@ -434,6 +475,31 @@ impl ServerCapabilitiesBuilder { } } +#[cfg(any(feature = "server", feature = "macros"))] +impl ServerCapabilitiesBuilder { + /// Advertise the `io.modelcontextprotocol/skills` extension (SEP-2640) in the + /// `extensions` capability map with `directoryRead: true`. + /// + /// Use [`enable_skills_extension_without_directory_read`](Self::enable_skills_extension_without_directory_read) + /// when the server does not implement `resources/directory/read` for skill + /// directories. + pub fn enable_skills_extension(mut self) -> Self { + insert_skills_extension_into(&mut self.extensions, true); + self + } + + /// Advertise the `io.modelcontextprotocol/skills` extension (SEP-2640) in the + /// `extensions` capability map **without** `directoryRead`. + /// + /// Clients MUST NOT call `resources/directory/read` against a server that + /// has not declared `directoryRead: true`. Use this method when the server + /// does not implement directory listing for skill directories. + pub fn enable_skills_extension_without_directory_read(mut self) -> Self { + insert_skills_extension_into(&mut self.extensions, false); + self + } +} + #[cfg(any(feature = "server", feature = "macros"))] builder! { ClientCapabilities{ diff --git a/crates/rmcp/src/model/meta.rs b/crates/rmcp/src/model/meta.rs index 0a7121e4a..2c4938b15 100644 --- a/crates/rmcp/src/model/meta.rs +++ b/crates/rmcp/src/model/meta.rs @@ -182,6 +182,9 @@ variant_extension! { ListResourcesRequest ListResourceTemplatesRequest ReadResourceRequest + ResourcesDirectoryReadRequest + SkillsGetRequest + SkillsListRequest SubscriptionsListenRequest SubscribeRequest UnsubscribeRequest diff --git a/crates/rmcp/src/model/skills.rs b/crates/rmcp/src/model/skills.rs new file mode 100644 index 000000000..ca90f3ff0 --- /dev/null +++ b/crates/rmcp/src/model/skills.rs @@ -0,0 +1,550 @@ +// SEP-2640: Skills Extension — data types, request/response types, and URI utilities. +// +// Implements the normative parts of +// https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2640 +// +// Skills are transported as MCP Resources. See §Resource Mapping and §Discovery of +// the spec for the exact wire shapes. + +// `const_string!` expands a `#[cfg(feature = "schemars")]` impl that names +// `Cow` unqualified, so the import is required only with that feature — and is +// unused (hence a `-D warnings` error) without it. +#[cfg(feature = "schemars")] +use std::borrow::Cow; + +use serde::{Deserialize, Serialize}; + +use super::{ + ConstString, MetaObject, PaginatedRequestParams, Request, RequestMetaObject, + RequestOptionalParam, ResultType, +}; +use crate::{const_string, model::Resource}; + +// ============================================================================= +// Skill entry +// ============================================================================= + +/// A skill entry returned by `skills/list` or `skills/get`. +/// +/// Mirrors the spec's JSON shape (see §Enumeration via `skills/list` in SEP-2640): +/// +/// ```json +/// { +/// "uri": "skill://doc-workflow/SKILL.md", +/// "frontmatter": { "name": "doc-workflow", "description": "..." }, +/// "resources": [ +/// { "uri": "skill://doc-workflow/SKILL.md", "digest": "sha256:...", "size": 1234 } +/// ], +/// "_meta": { "ttlMs": 600000, "cacheScope": "metatarsal" } +/// } +/// ``` +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +#[non_exhaustive] +pub struct SkillEntry { + /// Resource URI of the skill's `SKILL.md`, e.g. `skill://doc-workflow/SKILL.md`. + pub uri: String, + + /// Verbatim copy of the `SKILL.md` YAML frontmatter, rendered as JSON. + /// + /// The spec calls this "the frontmatter properties, rendered as JSON". + /// Required keys: `name` (MUST equal the final segment of the skill path), + /// `description`. Optional: `version`, `license`, plus any custom metadata. + pub frontmatter: serde_json::Value, + + /// The skill's files: an array of `{uri, digest, size}` triples, or the + /// literal string `"dynamic"` for dynamically-generated skills. + /// REQUIRED by the spec — a host MUST NOT load an entry with `null`. + /// Defaults to `SkillResources::Dynamic` via `SkillEntry::new()`. + #[serde(skip_serializing_if = "Option::is_none")] + pub resources: Option, + + /// Optional protocol-level metadata, including SEP-2549 caching fields. + /// REQUIRED on `skills/list` results in protocol version 2026-07-28+ + /// (SEP-2549); not required on `skills/get` results. + #[serde(rename = "_meta", skip_serializing_if = "Option::is_none")] + pub meta: Option, +} + +impl SkillEntry { + pub fn new(uri: impl Into, frontmatter: serde_json::Value) -> Self { + Self { + uri: uri.into(), + frontmatter, + resources: Some(SkillResources::Dynamic), + meta: None, + } + } +} + +/// The `resources` field of a skill entry. +/// +/// The spec allows either a concrete file list or the literal `"dynamic"`. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[non_exhaustive] +pub enum SkillResources { + /// Fixed list of file entries with digests and sizes. + FileList(Vec), + /// Dynamic skill whose content is generated on the fly. + /// The wire form is the bare JSON string `"dynamic"`. + Dynamic, +} + +impl Serialize for SkillResources { + fn serialize(&self, serializer: S) -> Result { + match self { + Self::FileList(files) => files.serialize(serializer), + Self::Dynamic => serializer.serialize_str("dynamic"), + } + } +} + +impl<'de> Deserialize<'de> for SkillResources { + fn deserialize>(deserializer: D) -> Result { + use serde::de::{self, MapAccess, SeqAccess, Visitor}; + struct SkillResourcesVisitor; + impl<'de> Visitor<'de> for SkillResourcesVisitor { + type Value = SkillResources; + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a skill resources list or the string \"dynamic\"") + } + fn visit_str(self, value: &str) -> Result { + if value == "dynamic" { + Ok(SkillResources::Dynamic) + } else { + Err(de::Error::invalid_type(de::Unexpected::Str(value), &self)) + } + } + fn visit_seq>(self, mut seq: A) -> Result { + let mut files = Vec::new(); + while let Some(file) = seq.next_element::()? { + files.push(file); + } + Ok(SkillResources::FileList(files)) + } + fn visit_map>(self, _map: A) -> Result { + Err(de::Error::invalid_type(de::Unexpected::Map, &self)) + } + } + deserializer.deserialize_any(SkillResourcesVisitor) + } +} + +impl SkillResources { + pub fn is_dynamic(&self) -> bool { + matches!(self, Self::Dynamic) + } + + pub fn as_files(&self) -> Option<&[SkillResource]> { + match self { + Self::FileList(files) => Some(files), + Self::Dynamic => None, + } + } +} + +/// A single file inside a skill, with its URI, SHA-256 digest, and size. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +#[non_exhaustive] +pub struct SkillResource { + pub uri: String, + pub digest: String, + pub size: u64, +} + +impl SkillResource { + pub fn new(uri: impl Into, digest: impl Into, size: u64) -> Self { + Self { + uri: uri.into(), + digest: digest.into(), + size, + } + } +} + +// ============================================================================= +// Request / response types +// ============================================================================= + +const_string!(SkillsListRequestMethod = "skills/list"); + +/// Request to list the skills published by a server. +/// +/// The request carries an optional pagination cursor (no required params). +pub type SkillsListRequest = RequestOptionalParam; + +/// Response to `skills/list`. +/// +/// Carries the skill entries for this page plus (for protocol 2026-07-28+) +/// the SEP-2549 caching fields (`ttlMs`/`cacheScope`). +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +#[non_exhaustive] +pub struct SkillsListResult { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub result_type: Option, + pub skills: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub next_cursor: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub ttl_ms: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub cache_scope: Option, +} + +impl Default for SkillsListResult { + fn default() -> Self { + Self::new(Vec::new()) + } +} + +impl SkillsListResult { + pub fn new(skills: Vec) -> Self { + Self { + result_type: Some(ResultType::COMPLETE), + skills, + next_cursor: None, + ttl_ms: None, + cache_scope: None, + } + } +} + +const_string!(SkillsGetRequestMethod = "skills/get"); + +/// Parameters for retrieving a single skill by URI. +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +#[non_exhaustive] +pub struct SkillsGetRequestParams { + #[serde(rename = "_meta", default, skip_serializing_if = "Option::is_none")] + pub meta: Option, + pub uri: String, +} + +impl SkillsGetRequestParams { + pub fn new(uri: impl Into) -> Self { + Self { + meta: None, + uri: uri.into(), + } + } +} + +impl crate::model::RequestParamsMeta for SkillsGetRequestParams { + fn meta(&self) -> Option<&RequestMetaObject> { + self.meta.as_ref() + } + fn meta_mut(&mut self) -> &mut Option { + &mut self.meta + } +} + +pub type SkillsGetRequest = Request; + +/// Response to `skills/get`. +/// +/// SEP-2549 caching fields (`ttlMs`/`cacheScope`) are included for parity +/// with `SkillsListResult` and `ResourcesDirectoryReadResult`. The merged +/// SEP-2640 does not settle whether `skills/get` results carry them; they +/// are provided when the negotiated protocol version is 2026-07-28 or later. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +#[non_exhaustive] +pub struct SkillsGetResult { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub result_type: Option, + pub skill: SkillEntry, + #[serde(skip_serializing_if = "Option::is_none")] + pub ttl_ms: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub cache_scope: Option, +} + +impl SkillsGetResult { + pub fn new(skill: SkillEntry) -> Self { + Self { + result_type: Some(ResultType::COMPLETE), + skill, + ttl_ms: None, + cache_scope: None, + } + } +} + +// ============================================================================= +// Directory listing (resources/directory/read for skill:// URIs) +// ============================================================================= + +const_string!(ResourcesDirectoryReadRequestMethod = "resources/directory/read"); + +/// Parameters for reading a directory resource. +/// +/// The request carries the directory URI and an optional pagination cursor. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +#[non_exhaustive] +pub struct ResourcesDirectoryReadRequestParams { + #[serde(rename = "_meta", default, skip_serializing_if = "Option::is_none")] + pub meta: Option, + pub uri: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub cursor: Option, +} + +impl ResourcesDirectoryReadRequestParams { + pub fn new(uri: impl Into) -> Self { + Self { + meta: None, + uri: uri.into(), + cursor: None, + } + } +} + +impl crate::model::RequestParamsMeta for ResourcesDirectoryReadRequestParams { + fn meta(&self) -> Option<&RequestMetaObject> { + self.meta.as_ref() + } + fn meta_mut(&mut self) -> &mut Option { + &mut self.meta + } +} + +pub type ResourcesDirectoryReadRequest = + Request; + +/// Response listing the children of a directory. +/// +/// Carries the `resultType` discriminator (SEP-2322) so it can be stripped for +/// legacy peers the same way as other resource list results. +/// +/// Per the SEP-2640 spec, the result is `resources: []` where +/// subdirectories carry `mimeType: "inode/directory"`. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +#[serde(rename_all = "camelCase")] +#[non_exhaustive] +pub struct ResourcesDirectoryReadResult { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub result_type: Option, + pub resources: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub next_cursor: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub ttl_ms: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub cache_scope: Option, +} + +impl ResourcesDirectoryReadResult { + pub fn new(resources: Vec) -> Self { + Self { + result_type: Some(ResultType::COMPLETE), + resources, + next_cursor: None, + ttl_ms: None, + cache_scope: None, + } + } +} + +// ============================================================================= +// URI parsing helpers +// ============================================================================= + +/// Parse a `skill://` URI into `(skill_path, file_path)`. +/// +/// **Scope:** This function handles SKILL.md-form route URIs only — URIs of the +/// shape `skill:///SKILL.md`. It is NOT a general-purpose parser for +/// arbitrary nested-file URIs (e.g. `skill://acme/billing/references/FORMS.md`). +/// The spec provides no in-URI delimiter between skill-path and file-path, so +/// splitting on the last `/` is a heuristic that works for the canonical +/// SKILL.md form but cannot disambiguate deeper paths without consulting the +/// registered skill set. +/// +/// - `` MAY be single-segment (`git-workflow`) or nested +/// (`acme/billing/refunds`). +/// - For `skill://git-workflow/SKILL.md` the file-path is `SKILL.md` and the +/// skill-path is `git-workflow`. +/// - Directory URIs are written without a trailing slash and without a file-path: +/// `skill://git-workflow`. +/// +/// Returns `None` for URIs that are not valid `skill://` skill/file URIs. +pub fn parse_skill_uri(uri: &str) -> Option<(String, String)> { + let stripped = uri.strip_prefix("skill://")?; + if stripped.is_empty() { + return None; + } + // Reject trailing slashes — directory URIs are written without them + if stripped.ends_with('/') { + return None; + } + // Split on the LAST `/` — skill path may be nested (acme/billing/refunds) + // but the file path is always the final segment + let (skill_path, file_path) = stripped.rsplit_once('/')?; + if skill_path.is_empty() || file_path.is_empty() { + return None; + } + Some((skill_path.to_string(), file_path.to_string())) +} + +/// Extract the skill `` from a `skill://` URI. +/// +/// Returns `None` if the URI is not a valid skill URI or is a directory URI +/// without a file component. +pub fn skill_path_from_uri(uri: &str) -> Option { + parse_skill_uri(uri).map(|(path, _file)| path) +} + +/// Extract the skill name (final segment of ``) from a `skill://` URI. +pub fn skill_name_from_uri(uri: &str) -> Option { + let (path, _file) = parse_skill_uri(uri)?; + let name = path.rsplit('/').next()?; + if name.is_empty() { + return None; + } + Some(name.to_string()) +} + +/// Validate that the `name` field of the frontmatter matches the final segment +/// of the skill path, per the spec's resource-mapping constraint. +pub fn validate_skill_name(uri: &str, frontmatter: &serde_json::Value) -> Result<(), String> { + let expected = skill_name_from_uri(uri).ok_or_else(|| format!("invalid skill URI: {uri}"))?; + let actual = frontmatter + .get("name") + .and_then(|v| v.as_str()) + .ok_or_else(|| "frontmatter missing required 'name' field".to_string())?; + if actual != expected { + return Err(format!( + "frontmatter name '{actual}' does not match skill path final segment '{expected}'" + )); + } + Ok(()) +} + +// ============================================================================= +// Tests +// ============================================================================= + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn skill_entry_round_trip() { + let entry = SkillEntry { + uri: "skill://doc-workflow/SKILL.md".to_string(), + frontmatter: json!({"name": "doc-workflow", "description": "Follow this team's Git conventions"}), + resources: Some(SkillResources::FileList(vec![SkillResource::new( + "skill://doc-workflow/references/APPENDIX.md", + "sha256:a1b2c3d4e5f6789012345678901234567890abcd", + 1234, + )])), + meta: Some(MetaObject::new()), + }; + let json = serde_json::to_string_pretty(&entry).unwrap(); + let decoded: SkillEntry = serde_json::from_str(&json).unwrap(); + assert_eq!(entry, decoded); + } + + #[test] + fn skills_list_response_round_trip() { + let list = SkillsListResult::new(vec![SkillEntry::new( + "skill://git-workflow/SKILL.md", + json!({"name": "git-workflow", "description": "Git conventions"}), + )]); + let json = serde_json::to_string_pretty(&list).unwrap(); + let decoded: SkillsListResult = serde_json::from_str(&json).unwrap(); + assert_eq!(list, decoded); + } + + #[test] + fn skill_with_dynamic_resources() { + let entry = SkillEntry { + uri: "skill://dynamic-skill/SKILL.md".to_string(), + frontmatter: json!({"name": "dynamic-skill", "description": "Dynamic skill"}), + resources: Some(SkillResources::Dynamic), + meta: None, + }; + let json = serde_json::to_string_pretty(&entry).unwrap(); + let decoded: SkillEntry = serde_json::from_str(&json).unwrap(); + assert_eq!(entry, decoded); + assert!(decoded.resources.unwrap().is_dynamic()); + } + + #[test] + fn parse_bare_skill_uri() { + let (path, file) = parse_skill_uri("skill://git-workflow/SKILL.md").unwrap(); + assert_eq!(path, "git-workflow"); + assert_eq!(file, "SKILL.md"); + } + + #[test] + fn parse_nested_skill_uri() { + let (path, file) = parse_skill_uri("skill://acme/billing/refunds/SKILL.md").unwrap(); + assert_eq!(path, "acme/billing/refunds"); + assert_eq!(file, "SKILL.md"); + } + + #[test] + fn parse_subfile_uri() { + // Per spec: skill path is everything before the last `/`, file path is the last segment + let (path, file) = parse_skill_uri("skill://pdf-processing/references/FORMS.md").unwrap(); + assert_eq!(path, "pdf-processing/references"); + assert_eq!(file, "FORMS.md"); + } + + #[test] + fn parse_non_skill_uri() { + assert!(parse_skill_uri("https://example.com/SKILL.md").is_none()); + assert!(parse_skill_uri("file:///skills/SKILL.md").is_none()); + assert!(parse_skill_uri("skill://").is_none()); + assert!(parse_skill_uri("skill:///SKILL.md").is_none()); + } + + #[test] + fn parse_trailing_slash_directory() { + // `skill://git-workflow/` — trailing slash after skill-path, no file + // This is NOT a valid directory URI (the spec writes directory URIs without + // trailing slash). We treat it as invalid to match the spec's wire shape. + assert!(parse_skill_uri("skill://git-workflow/").is_none()); + } + + #[test] + fn validate_skill_name_mismatch() { + let uri = "skill://git-workflow/SKILL.md"; + let frontmatter = json!({"name": "wrong-name", "description": "..."}); + let err = validate_skill_name(uri, &frontmatter).unwrap_err(); + assert!(err.contains("wrong-name")); + assert!(err.contains("git-workflow")); + } + + #[test] + fn validate_skill_name_matches() { + let uri = "skill://git-workflow/SKILL.md"; + let frontmatter = json!({"name": "git-workflow", "description": "..."}); + assert!(validate_skill_name(uri, &frontmatter).is_ok()); + } + + #[test] + fn resources_directory_read_round_trip() { + let resp = ResourcesDirectoryReadResult::new(vec![ + Resource::new("skill://a/SKILL.md", "SKILL.md"), + Resource::new("skill://b/", "b").with_mime_type("inode/directory"), + ]); + let json = serde_json::to_string_pretty(&resp).unwrap(); + let decoded: ResourcesDirectoryReadResult = serde_json::from_str(&json).unwrap(); + assert_eq!(resp, decoded); + } +} diff --git a/crates/rmcp/src/service/client.rs b/crates/rmcp/src/service/client.rs index 5dad6554a..f3bac0896 100644 --- a/crates/rmcp/src/service/client.rs +++ b/crates/rmcp/src/service/client.rs @@ -24,12 +24,14 @@ use crate::{ ListResourcesRequest, ListResourcesResult, ListToolsRequest, ListToolsResult, NumberOrString, PaginatedRequestParams, ProgressNotification, ProgressNotificationParam, ProtocolVersion, ReadResourceRequest, ReadResourceRequestParams, ReadResourceResponse, - ReadResourceResult, Reference, RequestId, RequestMetaObject, RootsListChangedNotification, - ServerJsonRpcMessage, ServerNotification, ServerPeerInfo, ServerRequest, ServerResult, - SetLevelRequest, SetLevelRequestParams, SubscribeRequest, SubscribeRequestParams, - SubscriptionFilter, SubscriptionsListenRequest, SubscriptionsListenRequestParams, - SubscriptionsListenResult, UnsubscribeRequest, UnsubscribeRequestParams, UpdateTaskParams, - UpdateTaskRequest, + ReadResourceResult, Reference, RequestId, RequestMetaObject, ResourcesDirectoryReadRequest, + ResourcesDirectoryReadRequestParams, ResourcesDirectoryReadResult, + RootsListChangedNotification, ServerJsonRpcMessage, ServerNotification, ServerPeerInfo, + ServerRequest, ServerResult, SetLevelRequest, SetLevelRequestParams, SkillsGetRequest, + SkillsGetRequestParams, SkillsGetResult, SkillsListRequest, SkillsListResult, + SubscribeRequest, SubscribeRequestParams, SubscriptionFilter, SubscriptionsListenRequest, + SubscriptionsListenRequestParams, SubscriptionsListenResult, UnsubscribeRequest, + UnsubscribeRequestParams, UpdateTaskParams, UpdateTaskRequest, }, transport::DynamicTransportError, }; @@ -1065,6 +1067,7 @@ const PROMPT_LIST_CACHE_PREFIX: &str = "prompts/list:"; const RESOURCE_LIST_CACHE_PREFIX: &str = "resources/list:"; const RESOURCE_TEMPLATE_LIST_CACHE_PREFIX: &str = "resources/templates/list:"; const RESOURCE_READ_CACHE_PREFIX: &str = "resources/read:"; +const SKILLS_LIST_CACHE_PREFIX: &str = "skills/list:"; // Cache keys are built only from the request method plus the parameters that // affect the result (SEP-2549). Request `_meta` (progress tokens, trace @@ -1688,6 +1691,103 @@ impl Peer { } } + // ========================================================================= + // SEP-2640: Skills + // ========================================================================= + + /// Send one `skills/list` request and return the list result, without + /// pagination support. For paginated listing that follows `nextCursor`, + /// use [`Peer::skills_list`] with the desired pagination params. + pub async fn skills_list( + &self, + params: Option, + ) -> Result { + let cache_key = list_response_cache_key(SKILLS_LIST_CACHE_PREFIX, ¶ms); + if let Some(ServerResult::SkillsListResult(result)) = self.cached_response(&cache_key).await + { + return Ok(result); + } + let generation = self.capture_response_cache_generation().await; + let uses_cursor = request_uses_cursor(¶ms); + let result = self + .send_request(ClientRequest::SkillsListRequest(SkillsListRequest { + method: Default::default(), + params, + extensions: Default::default(), + })) + .await; + let result = match result { + Ok(result) => result, + Err(error) => { + if uses_cursor { + self.invalidate_cached_responses(SKILLS_LIST_CACHE_PREFIX) + .await; + return Err(error); + } + if let Some(ServerResult::SkillsListResult(result)) = + self.stale_cached_response(&cache_key).await + { + return Ok(result); + } + return Err(error); + } + }; + match result { + ServerResult::SkillsListResult(result) => { + self.cache_result( + Some(cache_key), + result.ttl_ms, + result.cache_scope.as_deref().and_then(|scope| match scope { + "private" => Some(CacheScope::Private), + "public" => Some(CacheScope::Public), + _ => None, + }), + generation, + ServerResult::SkillsListResult(result.clone()), + ) + .await; + Ok(result) + } + _ => Err(ServiceError::UnexpectedResponse), + } + } + + /// Send one `skills/get` request and return the skill result. + /// Does not use the response cache (skills are typically unique per URI). + pub async fn skills_get( + &self, + uri: impl Into, + ) -> Result { + let params = SkillsGetRequestParams::new(uri.into()); + let result = self + .send_request(ClientRequest::SkillsGetRequest(SkillsGetRequest::new( + params, + ))) + .await; + match result { + Ok(ServerResult::SkillsGetResult(result)) => Ok(result), + _ => Err(ServiceError::UnexpectedResponse), + } + } + + /// Send one `resources/directory/read` request and return the directory + /// listing result. + pub async fn resources_directory_read_once( + &self, + uri: impl Into, + ) -> Result { + let params = ResourcesDirectoryReadRequestParams::new(uri.into()); + let result = self + .send_request(ClientRequest::ResourcesDirectoryReadRequest( + ResourcesDirectoryReadRequest::new(params), + )) + .await; + match result { + Ok(ServerResult::ResourcesDirectoryReadResult(result)) => Ok(result), + _ => Err(ServiceError::UnexpectedResponse), + } + } + pub async fn read_resource( &self, params: ReadResourceRequestParams, @@ -1832,6 +1932,86 @@ impl Peer { Ok(resource_templates) } + // ========================================================================= + // SEP-2640: Skills convenience wrappers + // ========================================================================= + + /// List all skills on the server, following pagination. + /// + /// Calls [`Peer::skills_list`] repeatedly until no more + /// `nextCursor` is returned, mirroring [`Peer::list_all_tools`]. + pub async fn list_all_skills(&self) -> Result, ServiceError> { + let mut skills = Vec::new(); + let mut cursor = None; + loop { + let result = self + .skills_list(Some(PaginatedRequestParams { meta: None, cursor })) + .await?; + skills.extend(result.skills); + cursor = result.next_cursor; + if cursor.is_none() { + break; + } + } + Ok(skills) + } + + /// List available skills on the server, without pagination. + /// + /// Delegates to [`Peer::skills_list`] with no pagination params. + /// Use [`Peer::list_all_skills`] when you need every skill across + /// multiple pages. + pub async fn list_skills(peer: &Peer) -> Result { + peer.skills_list(None).await + } + + /// Retrieve a single skill by URI. + /// + /// Delegates to [`Peer::skills_get`]. + pub async fn get_skill( + peer: &Peer, + uri: impl Into, + ) -> Result { + peer.skills_get(uri.into()).await + } + + /// Retrieve the contents of a skill file by URI. + /// + /// Delegates to [`Peer::read_resource`] with a resource reference + /// built from the URI. + /// + /// A server that answers with an `InputRequired` variant (undefined for + /// skill files) yields [`ServiceError::UnexpectedResponse`]. + pub async fn read_skill_uri( + peer: &Peer, + uri: impl Into, + ) -> Result { + let params = ReadResourceRequestParams::new(uri); + peer.read_resource_once(params) + .await + .and_then(|response| match response { + ReadResourceResponse::Complete(result) => Ok(result), + ReadResourceResponse::InputRequired(_) => { + // The skills extension does not define input_required for + // skill files, so an `InputRequired` response here means the + // server returned a variant this wrapper does not handle. + // Surface it as an error rather than panicking — a + // non-conforming server must not abort the client. + Err(ServiceError::UnexpectedResponse) + } + }) + } + + /// List the contents of a skill directory. + /// + /// Delegates to [`Peer::resources_directory_read_once`]. + pub async fn read_directory( + peer: &Peer, + uri: impl Into, + ) -> Result { + Self::resources_directory_read_once(peer, uri.into()).await + } + /// Convenient method to get completion suggestions for a prompt argument /// /// # Arguments diff --git a/crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema.json b/crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema.json index 760d9e18c..66acb94a4 100644 --- a/crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema.json +++ b/crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema.json @@ -1095,13 +1095,13 @@ "$ref": "#/definitions/Request8" }, { - "$ref": "#/definitions/Request9" + "$ref": "#/definitions/RequestOptionalParam4" }, { - "$ref": "#/definitions/Request10" + "$ref": "#/definitions/Request9" }, { - "$ref": "#/definitions/RequestOptionalParam4" + "$ref": "#/definitions/Request10" }, { "$ref": "#/definitions/Request11" @@ -1109,9 +1109,18 @@ { "$ref": "#/definitions/Request12" }, + { + "$ref": "#/definitions/RequestOptionalParam5" + }, { "$ref": "#/definitions/Request13" }, + { + "$ref": "#/definitions/Request14" + }, + { + "$ref": "#/definitions/Request15" + }, { "$ref": "#/definitions/CustomRequest" } @@ -1484,6 +1493,38 @@ ] }, "Request10": { + "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", + "type": "object", + "properties": { + "method": { + "$ref": "#/definitions/SubscribeRequestMethod" + }, + "params": { + "$ref": "#/definitions/SubscribeRequestParams" + } + }, + "required": [ + "method", + "params" + ] + }, + "Request11": { + "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", + "type": "object", + "properties": { + "method": { + "$ref": "#/definitions/UnsubscribeRequestMethod" + }, + "params": { + "$ref": "#/definitions/UnsubscribeRequestParams" + } + }, + "required": [ + "method", + "params" + ] + }, + "Request12": { "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", "type": "object", "properties": { @@ -1499,7 +1540,7 @@ "params" ] }, - "Request11": { + "Request13": { "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", "type": "object", "properties": { @@ -1515,7 +1556,7 @@ "params" ] }, - "Request12": { + "Request14": { "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", "type": "object", "properties": { @@ -1531,7 +1572,7 @@ "params" ] }, - "Request13": { + "Request15": { "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", "type": "object", "properties": { @@ -1632,10 +1673,10 @@ "type": "object", "properties": { "method": { - "$ref": "#/definitions/SubscriptionsListenRequestMethod" + "$ref": "#/definitions/ResourcesDirectoryReadRequestMethod" }, "params": { - "$ref": "#/definitions/SubscriptionsListenRequestParams" + "$ref": "#/definitions/ResourcesDirectoryReadRequestParams" } }, "required": [ @@ -1648,10 +1689,10 @@ "type": "object", "properties": { "method": { - "$ref": "#/definitions/SubscribeRequestMethod" + "$ref": "#/definitions/SkillsGetRequestMethod" }, "params": { - "$ref": "#/definitions/SubscribeRequestParams" + "$ref": "#/definitions/SkillsGetRequestParams" } }, "required": [ @@ -1664,10 +1705,10 @@ "type": "object", "properties": { "method": { - "$ref": "#/definitions/UnsubscribeRequestMethod" + "$ref": "#/definitions/SubscriptionsListenRequestMethod" }, "params": { - "$ref": "#/definitions/UnsubscribeRequestParams" + "$ref": "#/definitions/SubscriptionsListenRequestParams" } }, "required": [ @@ -1772,6 +1813,27 @@ ] }, "RequestOptionalParam4": { + "type": "object", + "properties": { + "method": { + "$ref": "#/definitions/SkillsListRequestMethod" + }, + "params": { + "anyOf": [ + { + "$ref": "#/definitions/PaginatedRequestParams" + }, + { + "type": "null" + } + ] + } + }, + "required": [ + "method" + ] + }, + "RequestOptionalParam5": { "type": "object", "properties": { "method": { @@ -1950,6 +2012,39 @@ "uri" ] }, + "ResourcesDirectoryReadRequestMethod": { + "type": "string", + "format": "const", + "const": "resources/directory/read" + }, + "ResourcesDirectoryReadRequestParams": { + "description": "Parameters for reading a directory resource.\n\nThe request carries the directory URI and an optional pagination cursor.", + "type": "object", + "properties": { + "_meta": { + "anyOf": [ + { + "$ref": "#/definitions/RequestMetaObject" + }, + { + "type": "null" + } + ] + }, + "cursor": { + "type": [ + "string", + "null" + ] + }, + "uri": { + "type": "string" + } + }, + "required": [ + "uri" + ] + }, "Role": { "description": "Represents the role of a participant in a conversation or message exchange.\n\nUsed in sampling and chat contexts to distinguish between different\ntypes of message senders in the conversation flow.", "oneOf": [ @@ -2173,6 +2268,38 @@ "level" ] }, + "SkillsGetRequestMethod": { + "type": "string", + "format": "const", + "const": "skills/get" + }, + "SkillsGetRequestParams": { + "description": "Parameters for retrieving a single skill by URI.", + "type": "object", + "properties": { + "_meta": { + "anyOf": [ + { + "$ref": "#/definitions/RequestMetaObject" + }, + { + "type": "null" + } + ] + }, + "uri": { + "type": "string" + } + }, + "required": [ + "uri" + ] + }, + "SkillsListRequestMethod": { + "type": "string", + "format": "const", + "const": "skills/list" + }, "SubscribeRequestMethod": { "type": "string", "format": "const", diff --git a/crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema_current.json b/crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema_current.json index 760d9e18c..66acb94a4 100644 --- a/crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema_current.json +++ b/crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema_current.json @@ -1095,13 +1095,13 @@ "$ref": "#/definitions/Request8" }, { - "$ref": "#/definitions/Request9" + "$ref": "#/definitions/RequestOptionalParam4" }, { - "$ref": "#/definitions/Request10" + "$ref": "#/definitions/Request9" }, { - "$ref": "#/definitions/RequestOptionalParam4" + "$ref": "#/definitions/Request10" }, { "$ref": "#/definitions/Request11" @@ -1109,9 +1109,18 @@ { "$ref": "#/definitions/Request12" }, + { + "$ref": "#/definitions/RequestOptionalParam5" + }, { "$ref": "#/definitions/Request13" }, + { + "$ref": "#/definitions/Request14" + }, + { + "$ref": "#/definitions/Request15" + }, { "$ref": "#/definitions/CustomRequest" } @@ -1484,6 +1493,38 @@ ] }, "Request10": { + "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", + "type": "object", + "properties": { + "method": { + "$ref": "#/definitions/SubscribeRequestMethod" + }, + "params": { + "$ref": "#/definitions/SubscribeRequestParams" + } + }, + "required": [ + "method", + "params" + ] + }, + "Request11": { + "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", + "type": "object", + "properties": { + "method": { + "$ref": "#/definitions/UnsubscribeRequestMethod" + }, + "params": { + "$ref": "#/definitions/UnsubscribeRequestParams" + } + }, + "required": [ + "method", + "params" + ] + }, + "Request12": { "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", "type": "object", "properties": { @@ -1499,7 +1540,7 @@ "params" ] }, - "Request11": { + "Request13": { "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", "type": "object", "properties": { @@ -1515,7 +1556,7 @@ "params" ] }, - "Request12": { + "Request14": { "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", "type": "object", "properties": { @@ -1531,7 +1572,7 @@ "params" ] }, - "Request13": { + "Request15": { "description": "Represents a JSON-RPC request with method, parameters, and extensions.\n\nThis is the core structure for all MCP requests, containing:\n- `method`: The name of the method being called\n- `params`: The parameters for the method\n- `extensions`: Additional context data (similar to HTTP headers)", "type": "object", "properties": { @@ -1632,10 +1673,10 @@ "type": "object", "properties": { "method": { - "$ref": "#/definitions/SubscriptionsListenRequestMethod" + "$ref": "#/definitions/ResourcesDirectoryReadRequestMethod" }, "params": { - "$ref": "#/definitions/SubscriptionsListenRequestParams" + "$ref": "#/definitions/ResourcesDirectoryReadRequestParams" } }, "required": [ @@ -1648,10 +1689,10 @@ "type": "object", "properties": { "method": { - "$ref": "#/definitions/SubscribeRequestMethod" + "$ref": "#/definitions/SkillsGetRequestMethod" }, "params": { - "$ref": "#/definitions/SubscribeRequestParams" + "$ref": "#/definitions/SkillsGetRequestParams" } }, "required": [ @@ -1664,10 +1705,10 @@ "type": "object", "properties": { "method": { - "$ref": "#/definitions/UnsubscribeRequestMethod" + "$ref": "#/definitions/SubscriptionsListenRequestMethod" }, "params": { - "$ref": "#/definitions/UnsubscribeRequestParams" + "$ref": "#/definitions/SubscriptionsListenRequestParams" } }, "required": [ @@ -1772,6 +1813,27 @@ ] }, "RequestOptionalParam4": { + "type": "object", + "properties": { + "method": { + "$ref": "#/definitions/SkillsListRequestMethod" + }, + "params": { + "anyOf": [ + { + "$ref": "#/definitions/PaginatedRequestParams" + }, + { + "type": "null" + } + ] + } + }, + "required": [ + "method" + ] + }, + "RequestOptionalParam5": { "type": "object", "properties": { "method": { @@ -1950,6 +2012,39 @@ "uri" ] }, + "ResourcesDirectoryReadRequestMethod": { + "type": "string", + "format": "const", + "const": "resources/directory/read" + }, + "ResourcesDirectoryReadRequestParams": { + "description": "Parameters for reading a directory resource.\n\nThe request carries the directory URI and an optional pagination cursor.", + "type": "object", + "properties": { + "_meta": { + "anyOf": [ + { + "$ref": "#/definitions/RequestMetaObject" + }, + { + "type": "null" + } + ] + }, + "cursor": { + "type": [ + "string", + "null" + ] + }, + "uri": { + "type": "string" + } + }, + "required": [ + "uri" + ] + }, "Role": { "description": "Represents the role of a participant in a conversation or message exchange.\n\nUsed in sampling and chat contexts to distinguish between different\ntypes of message senders in the conversation flow.", "oneOf": [ @@ -2173,6 +2268,38 @@ "level" ] }, + "SkillsGetRequestMethod": { + "type": "string", + "format": "const", + "const": "skills/get" + }, + "SkillsGetRequestParams": { + "description": "Parameters for retrieving a single skill by URI.", + "type": "object", + "properties": { + "_meta": { + "anyOf": [ + { + "$ref": "#/definitions/RequestMetaObject" + }, + { + "type": "null" + } + ] + }, + "uri": { + "type": "string" + } + }, + "required": [ + "uri" + ] + }, + "SkillsListRequestMethod": { + "type": "string", + "format": "const", + "const": "skills/list" + }, "SubscribeRequestMethod": { "type": "string", "format": "const", diff --git a/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json b/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json index cebb3ed37..f1de0e532 100644 --- a/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json +++ b/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json @@ -2958,6 +2958,51 @@ } } }, + "ResourcesDirectoryReadResult": { + "description": "Response listing the children of a directory.\n\nCarries the `resultType` discriminator (SEP-2322) so it can be stripped for\nlegacy peers the same way as other resource list results.\n\nPer the SEP-2640 spec, the result is `resources: []` where\nsubdirectories carry `mimeType: \"inode/directory\"`.", + "type": "object", + "properties": { + "cacheScope": { + "type": [ + "string", + "null" + ] + }, + "nextCursor": { + "type": [ + "string", + "null" + ] + }, + "resources": { + "type": "array", + "items": { + "$ref": "#/definitions/Resource" + } + }, + "resultType": { + "anyOf": [ + { + "$ref": "#/definitions/ResultType" + }, + { + "type": "null" + } + ] + }, + "ttlMs": { + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + } + }, + "required": [ + "resources" + ] + }, "ResultType": { "description": "Indicates the type of a result object, allowing the client to\ndetermine how to parse the response.\n\nThe spec defines this as an open string (`\"complete\" | \"input_required\" | string`),\nso unknown values are preserved rather than rejected. Servers implementing this\nprotocol version MUST include `resultType` in every result. For backward\ncompatibility, clients MUST treat an absent field as `\"complete\"`.\n\nOrdinary results model the field as `Option`: `None` means the\nfield is absent on the wire. Constructors default to `Some(COMPLETE)`, and\nthe server handler strips the `\"complete\"` discriminator before responding\nto peers that negotiated a protocol version older than `2026-07-28`, so\nlegacy sessions keep their historical wire shape (see\n[`ServerResult::strip_result_type_for_legacy_peer`]).", "type": "string" @@ -3254,6 +3299,15 @@ { "$ref": "#/definitions/ReadResourceResult" }, + { + "$ref": "#/definitions/ResourcesDirectoryReadResult" + }, + { + "$ref": "#/definitions/SkillsGetResult" + }, + { + "$ref": "#/definitions/SkillsListResult" + }, { "$ref": "#/definitions/SubscriptionsListenResult" }, @@ -3297,6 +3351,174 @@ } ] }, + "SkillEntry": { + "description": "A skill entry returned by `skills/list` or `skills/get`.\n\nMirrors the spec's JSON shape (see §Enumeration via `skills/list` in SEP-2640):\n\n```json\n{\n \"uri\": \"skill://doc-workflow/SKILL.md\",\n \"frontmatter\": { \"name\": \"doc-workflow\", \"description\": \"...\" },\n \"resources\": [\n { \"uri\": \"skill://doc-workflow/SKILL.md\", \"digest\": \"sha256:...\", \"size\": 1234 }\n ],\n \"_meta\": { \"ttlMs\": 600000, \"cacheScope\": \"metatarsal\" }\n}\n```", + "type": "object", + "properties": { + "_meta": { + "description": "Optional protocol-level metadata, including SEP-2549 caching fields.\nREQUIRED on `skills/list` results in protocol version 2026-07-28+\n(SEP-2549); not required on `skills/get` results.", + "anyOf": [ + { + "$ref": "#/definitions/MetaObject" + }, + { + "type": "null" + } + ] + }, + "frontmatter": { + "description": "Verbatim copy of the `SKILL.md` YAML frontmatter, rendered as JSON.\n\nThe spec calls this \"the frontmatter properties, rendered as JSON\".\nRequired keys: `name` (MUST equal the final segment of the skill path),\n`description`. Optional: `version`, `license`, plus any custom metadata." + }, + "resources": { + "description": "The skill's files: an array of `{uri, digest, size}` triples, or the\nliteral string `\"dynamic\"` for dynamically-generated skills.\nREQUIRED by the spec — a host MUST NOT load an entry with `null`.\nDefaults to `SkillResources::Dynamic` via `SkillEntry::new()`.", + "anyOf": [ + { + "$ref": "#/definitions/SkillResources" + }, + { + "type": "null" + } + ] + }, + "uri": { + "description": "Resource URI of the skill's `SKILL.md`, e.g. `skill://doc-workflow/SKILL.md`.", + "type": "string" + } + }, + "required": [ + "uri", + "frontmatter" + ] + }, + "SkillResource": { + "description": "A single file inside a skill, with its URI, SHA-256 digest, and size.", + "type": "object", + "properties": { + "digest": { + "type": "string" + }, + "size": { + "type": "integer", + "format": "uint64", + "minimum": 0 + }, + "uri": { + "type": "string" + } + }, + "required": [ + "uri", + "digest", + "size" + ] + }, + "SkillResources": { + "description": "The `resources` field of a skill entry.\n\nThe spec allows either a concrete file list or the literal `\"dynamic\"`.", + "oneOf": [ + { + "description": "Fixed list of file entries with digests and sizes.", + "type": "object", + "properties": { + "FileList": { + "type": "array", + "items": { + "$ref": "#/definitions/SkillResource" + } + } + }, + "additionalProperties": false, + "required": [ + "FileList" + ] + }, + { + "description": "Dynamic skill whose content is generated on the fly.\nThe wire form is the bare JSON string `\"dynamic\"`.", + "type": "string", + "const": "Dynamic" + } + ] + }, + "SkillsGetResult": { + "description": "Response to `skills/get`.\n\nSEP-2549 caching fields (`ttlMs`/`cacheScope`) are included for parity\nwith `SkillsListResult` and `ResourcesDirectoryReadResult`. The merged\nSEP-2640 does not settle whether `skills/get` results carry them; they\nare provided when the negotiated protocol version is 2026-07-28 or later.", + "type": "object", + "properties": { + "cacheScope": { + "type": [ + "string", + "null" + ] + }, + "resultType": { + "anyOf": [ + { + "$ref": "#/definitions/ResultType" + }, + { + "type": "null" + } + ] + }, + "skill": { + "$ref": "#/definitions/SkillEntry" + }, + "ttlMs": { + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + } + }, + "required": [ + "skill" + ] + }, + "SkillsListResult": { + "description": "Response to `skills/list`.\n\nCarries the skill entries for this page plus (for protocol 2026-07-28+)\nthe SEP-2549 caching fields (`ttlMs`/`cacheScope`).", + "type": "object", + "properties": { + "cacheScope": { + "type": [ + "string", + "null" + ] + }, + "nextCursor": { + "type": [ + "string", + "null" + ] + }, + "resultType": { + "anyOf": [ + { + "$ref": "#/definitions/ResultType" + }, + { + "type": "null" + } + ] + }, + "skills": { + "type": "array", + "items": { + "$ref": "#/definitions/SkillEntry" + } + }, + "ttlMs": { + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + } + }, + "required": [ + "skills" + ] + }, "StringFormat": { "description": "String format types allowed by the MCP specification.", "oneOf": [ diff --git a/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema_current.json b/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema_current.json index ca0ef5611..f1de0e532 100644 --- a/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema_current.json +++ b/crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema_current.json @@ -210,7 +210,7 @@ ] }, "structuredContent": { - "description": "An optional JSON object that represents the structured result of the tool call" + "description": "An optional JSON value that represents the structured result of the tool call.\nIt can be any JSON value, including `null`; an explicit `null` is kept\ndistinct from an absent field." } } }, @@ -963,6 +963,10 @@ "description": "Type-safe elicitation schema for requesting structured user input.\n\nThis enforces the MCP 2025-06-18 specification that elicitation schemas\nmust be objects with primitive-typed properties.\n\n# Example\n\n```rust\nuse rmcp::model::*;\n\nlet schema = ElicitationSchema::builder()\n .required_email(\"email\")\n .required_integer(\"age\", 0, 150)\n .optional_bool(\"newsletter\", false)\n .build();\n```", "type": "object", "properties": { + "$schema": { + "description": "Optional JSON Schema dialect identifier (the `$schema` keyword).\n\nThe 2025-11-25 protocol revision allows a `requestedSchema` to declare its\ndialect. It is preserved verbatim so a declared dialect survives a\ndecode/re-encode round-trip instead of being silently dropped.", + "type": "string" + }, "description": { "description": "Optional description of what this schema represents", "type": [ @@ -2954,6 +2958,51 @@ } } }, + "ResourcesDirectoryReadResult": { + "description": "Response listing the children of a directory.\n\nCarries the `resultType` discriminator (SEP-2322) so it can be stripped for\nlegacy peers the same way as other resource list results.\n\nPer the SEP-2640 spec, the result is `resources: []` where\nsubdirectories carry `mimeType: \"inode/directory\"`.", + "type": "object", + "properties": { + "cacheScope": { + "type": [ + "string", + "null" + ] + }, + "nextCursor": { + "type": [ + "string", + "null" + ] + }, + "resources": { + "type": "array", + "items": { + "$ref": "#/definitions/Resource" + } + }, + "resultType": { + "anyOf": [ + { + "$ref": "#/definitions/ResultType" + }, + { + "type": "null" + } + ] + }, + "ttlMs": { + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + } + }, + "required": [ + "resources" + ] + }, "ResultType": { "description": "Indicates the type of a result object, allowing the client to\ndetermine how to parse the response.\n\nThe spec defines this as an open string (`\"complete\" | \"input_required\" | string`),\nso unknown values are preserved rather than rejected. Servers implementing this\nprotocol version MUST include `resultType` in every result. For backward\ncompatibility, clients MUST treat an absent field as `\"complete\"`.\n\nOrdinary results model the field as `Option`: `None` means the\nfield is absent on the wire. Constructors default to `Some(COMPLETE)`, and\nthe server handler strips the `\"complete\"` discriminator before responding\nto peers that negotiated a protocol version older than `2026-07-28`, so\nlegacy sessions keep their historical wire shape (see\n[`ServerResult::strip_result_type_for_legacy_peer`]).", "type": "string" @@ -3250,6 +3299,15 @@ { "$ref": "#/definitions/ReadResourceResult" }, + { + "$ref": "#/definitions/ResourcesDirectoryReadResult" + }, + { + "$ref": "#/definitions/SkillsGetResult" + }, + { + "$ref": "#/definitions/SkillsListResult" + }, { "$ref": "#/definitions/SubscriptionsListenResult" }, @@ -3293,6 +3351,174 @@ } ] }, + "SkillEntry": { + "description": "A skill entry returned by `skills/list` or `skills/get`.\n\nMirrors the spec's JSON shape (see §Enumeration via `skills/list` in SEP-2640):\n\n```json\n{\n \"uri\": \"skill://doc-workflow/SKILL.md\",\n \"frontmatter\": { \"name\": \"doc-workflow\", \"description\": \"...\" },\n \"resources\": [\n { \"uri\": \"skill://doc-workflow/SKILL.md\", \"digest\": \"sha256:...\", \"size\": 1234 }\n ],\n \"_meta\": { \"ttlMs\": 600000, \"cacheScope\": \"metatarsal\" }\n}\n```", + "type": "object", + "properties": { + "_meta": { + "description": "Optional protocol-level metadata, including SEP-2549 caching fields.\nREQUIRED on `skills/list` results in protocol version 2026-07-28+\n(SEP-2549); not required on `skills/get` results.", + "anyOf": [ + { + "$ref": "#/definitions/MetaObject" + }, + { + "type": "null" + } + ] + }, + "frontmatter": { + "description": "Verbatim copy of the `SKILL.md` YAML frontmatter, rendered as JSON.\n\nThe spec calls this \"the frontmatter properties, rendered as JSON\".\nRequired keys: `name` (MUST equal the final segment of the skill path),\n`description`. Optional: `version`, `license`, plus any custom metadata." + }, + "resources": { + "description": "The skill's files: an array of `{uri, digest, size}` triples, or the\nliteral string `\"dynamic\"` for dynamically-generated skills.\nREQUIRED by the spec — a host MUST NOT load an entry with `null`.\nDefaults to `SkillResources::Dynamic` via `SkillEntry::new()`.", + "anyOf": [ + { + "$ref": "#/definitions/SkillResources" + }, + { + "type": "null" + } + ] + }, + "uri": { + "description": "Resource URI of the skill's `SKILL.md`, e.g. `skill://doc-workflow/SKILL.md`.", + "type": "string" + } + }, + "required": [ + "uri", + "frontmatter" + ] + }, + "SkillResource": { + "description": "A single file inside a skill, with its URI, SHA-256 digest, and size.", + "type": "object", + "properties": { + "digest": { + "type": "string" + }, + "size": { + "type": "integer", + "format": "uint64", + "minimum": 0 + }, + "uri": { + "type": "string" + } + }, + "required": [ + "uri", + "digest", + "size" + ] + }, + "SkillResources": { + "description": "The `resources` field of a skill entry.\n\nThe spec allows either a concrete file list or the literal `\"dynamic\"`.", + "oneOf": [ + { + "description": "Fixed list of file entries with digests and sizes.", + "type": "object", + "properties": { + "FileList": { + "type": "array", + "items": { + "$ref": "#/definitions/SkillResource" + } + } + }, + "additionalProperties": false, + "required": [ + "FileList" + ] + }, + { + "description": "Dynamic skill whose content is generated on the fly.\nThe wire form is the bare JSON string `\"dynamic\"`.", + "type": "string", + "const": "Dynamic" + } + ] + }, + "SkillsGetResult": { + "description": "Response to `skills/get`.\n\nSEP-2549 caching fields (`ttlMs`/`cacheScope`) are included for parity\nwith `SkillsListResult` and `ResourcesDirectoryReadResult`. The merged\nSEP-2640 does not settle whether `skills/get` results carry them; they\nare provided when the negotiated protocol version is 2026-07-28 or later.", + "type": "object", + "properties": { + "cacheScope": { + "type": [ + "string", + "null" + ] + }, + "resultType": { + "anyOf": [ + { + "$ref": "#/definitions/ResultType" + }, + { + "type": "null" + } + ] + }, + "skill": { + "$ref": "#/definitions/SkillEntry" + }, + "ttlMs": { + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + } + }, + "required": [ + "skill" + ] + }, + "SkillsListResult": { + "description": "Response to `skills/list`.\n\nCarries the skill entries for this page plus (for protocol 2026-07-28+)\nthe SEP-2549 caching fields (`ttlMs`/`cacheScope`).", + "type": "object", + "properties": { + "cacheScope": { + "type": [ + "string", + "null" + ] + }, + "nextCursor": { + "type": [ + "string", + "null" + ] + }, + "resultType": { + "anyOf": [ + { + "$ref": "#/definitions/ResultType" + }, + { + "type": "null" + } + ] + }, + "skills": { + "type": "array", + "items": { + "$ref": "#/definitions/SkillEntry" + } + }, + "ttlMs": { + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + } + }, + "required": [ + "skills" + ] + }, "StringFormat": { "description": "String format types allowed by the MCP specification.", "oneOf": [ diff --git a/crates/rmcp/tests/test_skill_macros.rs b/crates/rmcp/tests/test_skill_macros.rs new file mode 100644 index 000000000..0e61f2fb4 --- /dev/null +++ b/crates/rmcp/tests/test_skill_macros.rs @@ -0,0 +1,163 @@ +//! Compile and behavior tests for the `#[skill]`, `#[skill_router]`, and +//! `#[skill_handler]` proc-macros. +//! +//! These tests verify that the macros expand to valid code and that the +//! generated `SkillEntry` fields are correct. + +use rmcp::model::skills::SkillResources; + +// ─── Test skill with static frontmatter ────────────────────────────────────── + +#[derive(Clone)] +struct TestServer; + +#[rmcp::skill_router(server_handler = true)] +impl TestServer { + #[rmcp::skill(uri = "skill://test-skill/SKILL.md", frontmatter = serde_json::json!({ + "name": "test-skill", + "description": "A test skill" + }))] + async fn test_skill(&self) -> Result<(), rmcp::ErrorData> { + Ok(()) + } +} + +// ─── Test skill with dynamic resources ─────────────────────────────────────── + +#[derive(Clone)] +struct DynamicServer; + +#[rmcp::skill_router(server_handler = true)] +impl DynamicServer { + #[rmcp::skill( + uri = "skill://dynamic-skill/SKILL.md", + frontmatter = serde_json::json!({ + "name": "dynamic-skill", + "description": "A dynamic skill" + }), + dynamic = true + )] + async fn dynamic_skill(&self) -> Result<(), rmcp::ErrorData> { + Ok(()) + } +} + +// ─── Tests ────────────────────────────────────────────────────────────────── + +#[test] +fn skill_attr_generates_correct_entry() { + let entry = TestServer::test_skill_skill_attr(); + assert_eq!(entry.uri, "skill://test-skill/SKILL.md"); + assert_eq!(entry.frontmatter["name"], "test-skill"); + assert_eq!(entry.frontmatter["description"], "A test skill"); + // SEP-2640 skills.mdx:225 — `resources` is REQUIRED on every skill entry + // and takes one of exactly two forms: a complete file list, or the string + // "dynamic". An entry with no `resources` at all is invalid, and hosts + // MUST NOT load it. The `#[skill]` attribute has no file-list parameter, + // so a non-dynamic skill still cannot publish digests; "dynamic" is the + // only valid value the macro can emit. + assert_eq!(entry.resources, Some(SkillResources::Dynamic)); +} + +#[test] +fn dynamic_skill_has_dynamic_resources() { + let entry = DynamicServer::dynamic_skill_skill_attr(); + assert_eq!(entry.uri, "skill://dynamic-skill/SKILL.md"); + assert_eq!(entry.frontmatter["name"], "dynamic-skill"); + assert!(matches!(entry.resources, Some(SkillResources::Dynamic))); +} + +#[test] +fn skill_router_lists_all_skills() { + let router = TestServer::skill_router(); + let skills = router.list_all(); + assert_eq!(skills.len(), 1); + assert_eq!(skills[0].uri, "skill://test-skill/SKILL.md"); +} + +#[test] +fn skill_router_get_by_uri() { + let router = TestServer::skill_router(); + let route = router.get_by_uri("skill://test-skill/SKILL.md"); + assert!(route.is_some()); + assert_eq!(route.unwrap().attr.uri, "skill://test-skill/SKILL.md"); +} + +#[test] +fn skill_router_get_by_unknown_uri_returns_none() { + let router = TestServer::skill_router(); + let route = router.get_by_uri("skill://unknown/SKILL.md"); + assert!(route.is_none()); +} + +// ─── Longest-prefix matching for nested skill URIs ──────────────────────────── + +#[derive(Clone)] +struct NestedServer; + +#[rmcp::skill_router(server_handler = true)] +impl NestedServer { + #[rmcp::skill(uri = "skill://acme/billing/refunds/SKILL.md", frontmatter = serde_json::json!({ + "name": "refunds", + "description": "A nested skill" + }))] + async fn refunds(&self) -> Result<(), rmcp::ErrorData> { + Ok(()) + } + + #[rmcp::skill(uri = "skill://alpha/SKILL.md", frontmatter = serde_json::json!({ + "name": "alpha", + "description": "A single-segment skill" + }))] + async fn alpha(&self) -> Result<(), rmcp::ErrorData> { + Ok(()) + } +} + +#[test] +fn skill_router_exact_match() { + let router = NestedServer::skill_router(); + let route = router.get_by_uri("skill://alpha/SKILL.md"); + assert!(route.is_some()); + assert_eq!(route.unwrap().attr.uri, "skill://alpha/SKILL.md"); +} + +#[test] +fn skill_router_nested_exact_match() { + let router = NestedServer::skill_router(); + let route = router.get_by_uri("skill://acme/billing/refunds/SKILL.md"); + assert!(route.is_some()); + assert_eq!( + route.unwrap().attr.uri, + "skill://acme/billing/refunds/SKILL.md" + ); +} + +#[test] +fn skill_router_nested_file_prefix_match() { + // Defect 2 fix: a file URI under a nested skill root must match + let router = NestedServer::skill_router(); + let route = router.get_by_uri("skill://acme/billing/refunds/templates/x.md"); + assert!(route.is_some()); + assert_eq!( + route.unwrap().attr.uri, + "skill://acme/billing/refunds/SKILL.md" + ); +} + +#[test] +fn skill_router_single_segment_file_prefix_match() { + // Defect 2 fix: a file URI under a single-segment skill root must match + let router = NestedServer::skill_router(); + let route = router.get_by_uri("skill://alpha/references/guide.md"); + assert!(route.is_some()); + assert_eq!(route.unwrap().attr.uri, "skill://alpha/SKILL.md"); +} + +#[test] +fn skill_router_no_match_for_unrelated_prefix() { + // skill://acme/billing/other/... should NOT match skill://acme/billing/refunds/SKILL.md + let router = NestedServer::skill_router(); + let route = router.get_by_uri("skill://acme/billing/other/file.md"); + assert!(route.is_none()); +} diff --git a/crates/rmcp/tests/test_skills_conformance.rs b/crates/rmcp/tests/test_skills_conformance.rs new file mode 100644 index 000000000..185c8885c --- /dev/null +++ b/crates/rmcp/tests/test_skills_conformance.rs @@ -0,0 +1,1348 @@ +//! Conformance tests for the skills feature. +//! +//! Validates FileSystemSkillServer against the MCP spec vectors: +//! - skills/list returns all discovered SKILL.md files sorted by URI +//! - skills/get returns a single entry by URI +//! - skills/get returns an error for unknown URIs +//! - resources/directory/read lists sibling files (multi-file skills) +//! - frontmatter is parsed correctly +//! +//! The tests spawn the server binary as a child process and communicate +//! via stdio JSON-RPC. This validates the actual binary, not just the types. + +use std::{ + io::Write, + process::{Command, Stdio}, +}; + +fn skills_dir() -> std::path::PathBuf { + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| ".".to_string()); + let crate_root = std::path::PathBuf::from(manifest_dir); + let workspace_root = crate_root + .parent() + .and_then(|p| p.parent()) + .unwrap_or(&crate_root); + workspace_root.join("fixtures/skills-dir") +} + +fn build_request(id: u64, method: &str, params: serde_json::Value) -> String { + serde_json::json!({ + "jsonrpc": "2.0", + "id": id, + "method": method, + "params": params + }) + .to_string() +} + +fn run_server_request(requests: &[String]) -> Vec { + let mut child = Command::new("cargo") + .args([ + "run", + "--example", + "skill_fs_server", + "--features", + "server,transport-io", + "--", + "--root", + ]) + .arg(skills_dir()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .expect("failed to spawn server"); + + { + let stdin = child.stdin.as_mut().expect("failed to open stdin"); + for req in requests { + writeln!(stdin, "{}", req).expect("failed to write request"); + } + // Close stdin to signal EOF and let the server exit + } + + let output = child.wait_with_output().expect("failed to wait for server"); + let stdout = String::from_utf8_lossy(&output.stdout); + + stdout + .lines() + .filter_map(|line| { + let line = line.trim(); + if line.is_empty() { + return None; + } + serde_json::from_str::(line).ok() + }) + .collect() +} + +#[test] +fn skills_list_returns_all_discovered() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request(2, "skills/list", serde_json::json!({})), + ]; + + let responses = run_server_request(&requests); + let skills_list = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skills"].as_array()) + .expect("should have skills/list response"); + + assert_eq!(skills_list.len(), 3, "expected 3 skills from fixtures"); + let uris: Vec<&str> = skills_list + .iter() + .map(|s| s["uri"].as_str().unwrap_or("")) + .collect(); + let mut sorted = uris.clone(); + sorted.sort(); + assert_eq!(uris, sorted, "skills should be sorted by URI"); +} + +#[test] +fn skills_list_uris_are_valid() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request(2, "skills/list", serde_json::json!({})), + ]; + + let responses = run_server_request(&requests); + let skills_list = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skills"].as_array()) + .expect("should have skills/list response"); + + for skill in skills_list { + let uri = skill["uri"].as_str().unwrap_or(""); + assert!( + uri.starts_with("skill://"), + "URI must start with skill://: {}", + uri + ); + assert!( + uri.ends_with("/SKILL.md"), + "URI must end with /SKILL.md: {}", + uri + ); + } +} + +#[test] +fn skills_get_returns_correct_entry() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://billing/refunds/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let skill = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skill"].as_object()) + .expect("should have skills/get response"); + + assert_eq!(skill["uri"], "skill://billing/refunds/SKILL.md"); + assert_eq!( + skill["frontmatter"]["name"], "refunds", + "frontmatter name should match" + ); +} + +#[test] +fn skills_get_returns_error_for_unknown() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://nonexistent/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let error = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["error"].as_object()) + .expect("should have error response for unknown skill"); + + assert!(error["code"].is_number(), "error should have a code"); +} + +#[test] +fn multi_file_skill_lists_siblings() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://billing/refunds/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let resources = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skill"]["resources"].as_array()) + .expect("should have resources for multi-file skill"); + + let names: Vec<&str> = resources + .iter() + .map(|r| r["uri"].as_str().unwrap_or("")) + .filter(|uri| uri.ends_with(".md")) + .collect(); + // SKILL.md + 2 siblings = 3 entries + assert_eq!( + names.len(), + 3, + "refunds skill should have 3 entries (SKILL.md + 2 siblings)" + ); + assert!( + names.iter().any(|uri| uri.ends_with("/FORMS.md")), + "should contain FORMS.md" + ); + assert!( + names.iter().any(|uri| uri.ends_with("/APPENDIX.md")), + "should contain APPENDIX.md" + ); +} + +#[test] +fn single_file_skill_has_no_resources() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://git-workflow/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let resources = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skill"]["resources"].as_array()); + + // SKILL.md itself is included in resources. + // A single-file skill should have exactly 1 resource (SKILL.md). + let resources = resources.expect("single-file skill should have resources"); + assert_eq!( + resources.len(), + 1, + "single-file skill should have SKILL.md in resources" + ); + assert_eq!( + resources[0]["uri"].as_str().unwrap(), + "skill://git-workflow/SKILL.md", + "resource should be SKILL.md" + ); +} + +#[test] +fn frontmatter_has_required_fields() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request(2, "skills/list", serde_json::json!({})), + ]; + + let responses = run_server_request(&requests); + let skills_list = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skills"].as_array()) + .expect("should have skills/list response"); + + for skill in skills_list { + let fm = skill["frontmatter"] + .as_object() + .expect("should have frontmatter"); + assert!(fm.get("name").is_some(), "skill must have a name"); + assert!( + fm.get("description").is_some(), + "skill must have a description" + ); + } +} + +#[test] +fn doc_workflow_skill_exists() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://doc-workflow/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let skill = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skill"].as_object()) + .expect("doc-workflow skill should exist"); + + assert_eq!(skill["uri"], "skill://doc-workflow/SKILL.md"); +} + +#[test] +fn billing_refunds_frontmatter_has_version() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://billing/refunds/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let version = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skill"]["frontmatter"]["version"].as_str()) + .expect("should have version field"); + + assert_eq!( + version, "1.0.0", + "version should be parsed from frontmatter" + ); +} + +#[test] +fn multi_file_skill_has_resources_field() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://billing/refunds/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let resources = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skill"]["resources"].as_array()) + .expect("multi-file skill should have resources"); + + // SKILL.md + 2 siblings = 3 entries + assert_eq!( + resources.len(), + 3, + "should have 3 file entries (SKILL.md + 2 siblings)" + ); +} + +/// `resources/directory/read` is the request that actually lists a skill's +/// sibling files. The header comment has always claimed this coverage, but no +/// test called it — `skills/get` returning a `resources` field is a different +/// code path. This exercises the real method. +#[test] +fn resources_directory_read_lists_siblings() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-07-28", + "capabilities": {}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "resources/directory/read", + serde_json::json!({"uri": "skill://billing/refunds"}), + ), + ]; + + let responses = run_server_request(&requests); + let resources = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["resources"].as_array()) + .expect("resources/directory/read should return a resources array"); + + // Spec, Directory Listing: "The result contains every direct child of the + // directory." SKILL.md is a file directly inside the skill root, so it IS a + // direct child and MUST be listed. Expect SKILL.md + the 2 siblings = 3. + assert_eq!( + resources.len(), + 3, + "refunds directory should list every direct child (SKILL.md + 2 siblings)" + ); + assert!( + resources + .iter() + .any(|r| r["uri"].as_str().unwrap_or("").ends_with("/SKILL.md")), + "listing must contain SKILL.md — it is a direct child of the skill root" + ); + assert!( + resources + .iter() + .any(|r| r["uri"].as_str().unwrap_or("").ends_with("/FORMS.md")), + "listing should contain FORMS.md" + ); + assert!( + resources + .iter() + .any(|r| r["uri"].as_str().unwrap_or("").ends_with("/APPENDIX.md")), + "listing should contain APPENDIX.md" + ); +} + +/// `resources/read` returns the bytes of one skill file. Also claimed by the +/// header, also never actually called before this test. +#[test] +fn resources_read_returns_file_contents() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-07-28", + "capabilities": {}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "resources/read", + serde_json::json!({"uri": "skill://billing/refunds/FORMS.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let contents = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["contents"].as_array()) + .expect("resources/read should return a contents array"); + + assert_eq!(contents.len(), 1, "should return exactly one content block"); + let text = contents[0]["text"].as_str().unwrap_or(""); + assert!( + text.contains("Form 101"), + "file contents should be returned verbatim" + ); +} + +// --------------------------------------------------------------------------- +// Macro-generated server — skill_macro_server example +// --------------------------------------------------------------------------- + +/// Drive the macro server over stdio, mirroring `run_server_request` for the +/// hand-written `skill_fs_server`. This is the only test that exercises the +/// macro expansion path (`#[skill]` + `#[skill_router]` + `#[skill_handler]`) +/// end-to-end through a real process. +fn run_macro_server_request(requests: &[String]) -> Vec { + let mut child = Command::new("cargo") + .args([ + "run", + "--example", + "skill_macro_server", + "--features", + "server,transport-io", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .expect("failed to spawn macro server"); + + { + let stdin = child.stdin.as_mut().expect("failed to open stdin"); + for req in requests { + writeln!(stdin, "{}", req).expect("failed to write request"); + } + // Close stdin to signal EOF and let the server exit. + } + + let output = child.wait_with_output().expect("failed to wait for server"); + let stdout = String::from_utf8_lossy(&output.stdout); + + stdout + .lines() + .filter_map(|line| { + let line = line.trim(); + if line.is_empty() { + return None; + } + serde_json::from_str::(line).ok() + }) + .collect() +} + +/// The macro-generated server must answer `skills/list` and `skills/get` over +/// the wire. Regression guard: the example must hold its `RunningService` +/// (`.waiting()`), or the serve loop is cancelled on drop and the process +/// exits after `initialize` without answering anything else. +#[test] +fn macro_server_skills_list_and_get() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + // The initialized notification is required before the server will + // answer subsequent requests. + serde_json::json!({ + "jsonrpc": "2.0", + "method": "notifications/initialized", + "params": {} + }) + .to_string(), + build_request(2, "skills/list", serde_json::json!({})), + build_request( + 3, + "skills/get", + serde_json::json!({"uri": "skill://alpha/SKILL.md"}), + ), + ]; + + let responses = run_macro_server_request(&requests); + + // initialize → skills extension advertised + let init_resp = responses + .iter() + .find(|r| r["id"] == 1) + .expect("macro server should answer initialize"); + assert!( + !init_resp["result"]["capabilities"]["extensions"]["io.modelcontextprotocol/skills"] + .is_null(), + "skills extension must be advertised; got: {init_resp}" + ); + // The macro does NOT advertise directoryRead — it cannot enumerate files + // for dynamic skills, and the spec says clients MUST NOT call + // resources/directory/read without the declaration. + assert!( + init_resp["result"]["capabilities"]["extensions"]["io.modelcontextprotocol/skills"] + ["directoryRead"] + .is_null(), + "macro server must NOT advertise directoryRead; got: {init_resp}" + ); + + // skills/list → 3 skills from the macro server + let skills_list = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skills"].as_array()) + .expect("macro server should return skills/list response"); + + assert_eq!( + skills_list.len(), + 3, + "macro server should discover 3 skills (alpha, beta, gamma); got: {skills_list:?}" + ); + // SEP-2640 skills.mdx:225 — `resources` is REQUIRED on every skill entry, + // and an entry with no `resources` at all is invalid: hosts MUST NOT load + // it. The `#[skill]` attribute has no file-list parameter, so every + // macro-generated entry must carry the literal string "dynamic" rather + // than omitting the field. + for skill in skills_list { + assert_eq!( + skill["resources"], "dynamic", + "every macro skill entry must carry resources=\"dynamic\"; got: {skill}" + ); + } + let uris: Vec<&str> = skills_list + .iter() + .map(|s| s["uri"].as_str().unwrap_or("")) + .collect(); + assert!( + uris.contains(&"skill://alpha/SKILL.md"), + "alpha skill should be listed; got: {uris:?}" + ); + assert!( + uris.contains(&"skill://beta/nested/SKILL.md"), + "beta skill should be listed; got: {uris:?}" + ); + + // skills/get → the entry lives under result.skill, not result + let get_resp = responses + .iter() + .find(|r| r["id"] == 3) + .expect("macro server should return skills/get response"); + let frontmatter = get_resp["result"]["skill"]["frontmatter"] + .as_object() + .expect("skills/get should return result.skill.frontmatter"); + assert_eq!( + frontmatter["name"], "alpha", + "skills/get should return correct frontmatter" + ); +} + +/// The macro-generated handler must wire `resources/directory/read` into the +/// `ServerHandler` dispatch and advertise the `resources` capability. +/// +/// The request URIs are **directory** URIs (`skill://alpha`, the skill root), +/// per the spec: the skill root is a directory resource. The example declares +/// no file lists — the `#[skill]` macro has no file-list attribute (only +/// `dynamic`) — so an empty array is the correct result. The point of this test +/// is that the method is *answered*, not that it lists files; the hand-written +/// `skill_fs_server` covers the non-empty case. If the macro stops generating +/// `resources_directory_read`, this endpoint falls back to the trait default +/// and returns method-not-found (-32601) instead. +#[test] +fn macro_server_resources_directory_read_is_dispatched() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + serde_json::json!({ + "jsonrpc": "2.0", + "method": "notifications/initialized", + "params": {} + }) + .to_string(), + // Static skill (alpha) — skill root, no file list declared. + build_request( + 2, + "resources/directory/read", + serde_json::json!({"uri": "skill://alpha"}), + ), + // Dynamic skill (gamma) — macro returns an empty list for `Dynamic`. + build_request( + 3, + "resources/directory/read", + serde_json::json!({"uri": "skill://gamma"}), + ), + ]; + + let responses = run_macro_server_request(&requests); + + // initialize → resources capability advertised (gates the endpoint). + let init_resp = responses + .iter() + .find(|r| r["id"] == 1) + .expect("macro server should answer initialize"); + assert!( + !init_resp["result"]["capabilities"]["resources"].is_null(), + "macro server must advertise the resources capability; got: {init_resp}" + ); + + for id in [2u64, 3] { + let resp = responses.iter().find(|r| r["id"] == id).unwrap_or_else(|| { + panic!("macro server should answer resources/directory/read (id {id})") + }); + assert!( + resp["error"].is_null(), + "resources/directory/read must be dispatched, not method-not-found; got: {resp}" + ); + let resources = resp["result"]["resources"].as_array().unwrap_or_else(|| { + panic!("resources/directory/read should return a resources array; got: {resp}") + }); + // Both alpha (static, no file list) and gamma (dynamic) must return + // an empty array — the macro has no file-list attribute, so it cannot + // know what files exist. This assertion locks in the current behavior + // so a future change doesn't silently break it. + assert_eq!( + resources.len(), + 0, + "macro server must return an empty resources array for skills without a file list; got: {resources:?}" + ); + } +} + +/// A **file** URI (`skill:///SKILL.md`) is not a directory +/// resource, so `resources/directory/read` on it MUST return `-32602` +/// (Invalid params), per the spec: "If the URI does not exist, or exists but is +/// not a directory resource, the server MUST return error `-32602`." +/// +/// Regression guard for the behavior this test previously asserted the +/// opposite of: it used to send `skill://alpha/SKILL.md` and require a +/// `resources` array, which is the non-conforming answer. An unknown skill URI +/// (`skill://nonexistent`) MUST also return `-32602`. +#[test] +fn macro_server_directory_read_rejects_file_and_unknown_uris() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-07-28", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + serde_json::json!({ + "jsonrpc": "2.0", + "method": "notifications/initialized", + "params": {} + }) + .to_string(), + // A file inside a served skill — not a directory. + build_request( + 2, + "resources/directory/read", + serde_json::json!({"uri": "skill://alpha/SKILL.md"}), + ), + // A URI that does not identify any skill the server serves. + build_request( + 3, + "resources/directory/read", + serde_json::json!({"uri": "skill://nonexistent"}), + ), + ]; + + let responses = run_macro_server_request(&requests); + + for id in [2u64, 3] { + let resp = responses + .iter() + .find(|r| r["id"] == id) + .unwrap_or_else(|| panic!("macro server should answer id {id}")); + assert_eq!( + resp["error"]["code"], + serde_json::json!(-32602), + "expected -32602 (Invalid params) for id {id}; got: {resp}" + ); + } +} + +// --------------------------------------------------------------------------- +// Tests for SEP-2640 spec conformance fixes +// --------------------------------------------------------------------------- + +/// `skills/get` on the **macro-generated** handler must reject a URI naming a +/// file *inside* a skill, not just the Router path. +/// +/// Spec: `params.uri` MUST be the skill's SKILL.md URI. `SkillRouter::get_by_uri` +/// matches by longest prefix, so `skill://alpha/appendix.md` resolves to the +/// `alpha` route — the macro-generated `skills_get` used to call straight +/// through and return the skill entry instead of `-32602` (Invalid params). +/// The hand-written Router path had this gate; the macro path did not. +/// +/// The sibling test `skills_get_rejects_file_uri_within_skill` covers the same +/// rule on the hand-written `skill_fs_server`, so this is the macro counterpart. +#[test] +fn macro_server_skills_get_rejects_non_skill_md_uri() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + serde_json::json!({ + "jsonrpc": "2.0", + "method": "notifications/initialized", + "params": {} + }) + .to_string(), + // A sibling file inside the served skill — NOT the SKILL.md URI. + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://alpha/appendix.md"}), + ), + // A file nested below the skill root — also NOT the SKILL.md URI. + build_request( + 3, + "skills/get", + serde_json::json!({"uri": "skill://beta/nested/refs/x.md"}), + ), + // The canonical URI must still resolve, so the gate is not over-broad. + build_request( + 4, + "skills/get", + serde_json::json!({"uri": "skill://alpha/SKILL.md"}), + ), + ]; + + let responses = run_macro_server_request(&requests); + + for id in [2u64, 3] { + let resp = responses + .iter() + .find(|r| r["id"] == id) + .unwrap_or_else(|| panic!("macro server should answer skills/get id {id}")); + assert_eq!( + resp["error"]["code"], + serde_json::json!(-32602), + "skills/get on a non-SKILL.md URI must return -32602 (Invalid params); got: {resp}" + ); + assert!( + resp["result"].is_null(), + "a rejected skills/get must not also carry a result; got: {resp}" + ); + } + + // Happy path unaffected: the SKILL.md URI still returns the entry. + let ok = responses + .iter() + .find(|r| r["id"] == 4) + .expect("macro server should answer skills/get id 4"); + assert!( + ok["error"].is_null(), + "skills/get on the canonical SKILL.md URI must succeed; got: {ok}" + ); + assert_eq!( + ok["result"]["skill"]["uri"], "skill://alpha/SKILL.md", + "canonical skills/get should return the alpha entry; got: {ok}" + ); +} + +/// Hole (b): a nested SKILL.md URI that is not the canonical URI of any +/// registered skill must be rejected. The `directory_form || skill_md_form` +/// gate passes it because `parse_skill_uri` returns `("beta/nested/sub", "SKILL.md")` +/// and `file == "SKILL.md"` is true. The 2-line equality gate rejects it because +/// `"skill://beta/nested/sub/SKILL.md" != "skill://beta/nested/SKILL.md"`. +#[test] +fn macro_server_skills_get_rejects_nested_skill_md_uri() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + serde_json::json!({ + "jsonrpc": "2.0", + "method": "notifications/initialized", + "params": {} + }) + .to_string(), + // A nested SKILL.md URI that is not the canonical URI of any skill. + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://beta/nested/sub/SKILL.md"}), + ), + ]; + + let responses = run_macro_server_request(&requests); + + let resp = responses + .iter() + .find(|r| r["id"] == 2) + .expect("macro server should answer skills/get id 2"); + assert_eq!( + resp["error"]["code"], + serde_json::json!(-32602), + "skills/get on a nested SKILL.md URI must return -32602; got: {resp}" + ); +} + +/// Spec:419 — `GetSkillResult` extends `CacheableResult`, so `ttlMs` and +/// `cacheScope` are REQUIRED on every `skills/get` response. +/// +/// This is a unit test on `fill_missing_cache_hints` (same pattern as +/// `fill_missing_cache_hints_covers_skills_list_result`), not an integration +/// test through the server, because the server gates cache hints on the +/// negotiated protocol version (2026-07-28+) and the `initialize` handshake +/// negotiates down to 2025-11-25 when the client requests 2026-07-28. +#[test] +fn fill_missing_cache_hints_covers_skills_get_result() { + use rmcp::model::{ResultType, ServerResult, SkillEntry, skills::SkillsGetResult}; + + let entry = SkillEntry::new( + "skill://test/SKILL.md", + serde_json::json!({ + "name": "test", + "description": "Test skill" + }), + ); + let mut result = SkillsGetResult::new(entry); + result.result_type = Some(ResultType::COMPLETE); + result.ttl_ms = None; + result.cache_scope = None; + + let mut server_result = ServerResult::SkillsGetResult(result); + server_result.fill_missing_cache_hints(); + + let ServerResult::SkillsGetResult(r) = &server_result else { + panic!("expected SkillsGetResult"); + }; + assert!( + r.ttl_ms.is_some(), + "fill_missing_cache_hints must set ttl_ms on SkillsGetResult; got: {r:?}" + ); + assert!( + r.cache_scope.is_some(), + "fill_missing_cache_hints must set cache_scope on SkillsGetResult; got: {r:?}" + ); +} + +/// Hole (b) on the Router path: a nested SKILL.md URI that is not the +/// canonical URI of any registered skill must be rejected. +#[test] +fn skills_get_rejects_nested_skill_md_uri() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + // A nested SKILL.md URI that is not the canonical URI of any skill. + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://billing/refunds/forms/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + + let resp = responses + .iter() + .find(|r| r["id"] == 2) + .expect("server should answer skills/get id 2"); + assert_eq!( + resp["error"]["code"], + serde_json::json!(-32602), + "skills/get on a nested SKILL.md URI must return -32602; got: {resp}" + ); +} + +/// Resources completeness: files below the skill root must be published, even +/// in subdirectories that are not themselves skills. +/// +/// Builds a skill tree where a data file lives in a subdirectory below the +/// skill root (`billing/refunds/forms/FORMS.md`) and verifies `skills/get` +/// lists it. Before the fix the walk skipped directories, so the file was +/// invisible to a host. +#[test] +fn nested_skill_subdirectory_files_are_published_in_resources() { + use std::fs; + + // Build a temp skill tree: + // tmp_root/ + // billing/ + // refunds/ + // SKILL.md + // forms/ + // FORMS.md + let tmp = std::env::temp_dir().join(format!("rmcp_test_{}", std::process::id())); + let skill_dir = tmp.join("billing").join("refunds"); + let sub_dir = skill_dir.join("forms"); + fs::create_dir_all(&sub_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: refunds\ndescription: Refund handling\n---\n# Refunds\n", + ) + .unwrap(); + fs::write(sub_dir.join("FORMS.md"), "# Refund Forms\n\nForm 101\n").unwrap(); + + // Run the server against the temp root + let mut child = Command::new("cargo") + .args([ + "run", + "--example", + "skill_fs_server", + "--features", + "server,transport-io", + "--", + "--root", + ]) + .arg(&tmp) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .expect("failed to spawn server"); + + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://billing/refunds/SKILL.md"}), + ), + ]; + { + let stdin = child.stdin.as_mut().expect("failed to open stdin"); + for req in &requests { + writeln!(stdin, "{}", req).expect("failed to write request"); + } + } + + let output = child.wait_with_output().expect("failed to wait for server"); + let stdout = String::from_utf8_lossy(&output.stdout); + let responses: Vec = stdout + .lines() + .filter_map(|line| serde_json::from_str(line.trim()).ok()) + .collect(); + + // Cleanup + let _ = fs::remove_dir_all(&tmp); + + let resources = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skill"]["resources"].as_array()) + .expect("should have resources for nested skill"); + + assert!( + resources.len() >= 2, + "should include SKILL.md + nested FORMS.md; got {} resources: {:?}", + resources.len(), + resources + ); + assert!( + resources + .iter() + .any(|r| r["uri"].as_str().unwrap_or("").ends_with("/FORMS.md")), + "should contain nested FORMS.md in resources" + ); +} + +/// `skills/get` must reject a URI naming a file *inside* a skill. +/// +/// Spec: `params.uri` MUST be the skill's SKILL.md URI. The handler used to +/// prefix-match through the router, so a request for +/// `skill://billing/refunds/FORMS.md` returned the skill entry instead of +/// `-32602` (Invalid params). +#[test] +fn skills_get_rejects_file_uri_within_skill() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://billing/refunds/FORMS.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let error = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["error"].as_object()) + .expect("should have error response for non-SKILL.md URI"); + + assert_eq!( + error["code"], + serde_json::json!(-32602), + "skills/get on a non-SKILL.md URI must return -32602; got: {error:?}" + ); +} + +/// `skills/get` must still resolve the canonical SKILL.md URI. +/// +/// Regression guard: tightening the check must not break the happy path of +/// `skill://billing/refunds/SKILL.md`. +#[test] +fn skills_get_accepts_skill_md_uri() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-03-26", + "capabilities": {"skills": {}}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + build_request( + 2, + "skills/get", + serde_json::json!({"uri": "skill://billing/refunds/SKILL.md"}), + ), + ]; + + let responses = run_server_request(&requests); + let skill = responses + .iter() + .find(|r| r["id"] == 2) + .and_then(|r| r["result"]["skill"].as_object()) + .expect("should have skills/get response for SKILL.md URI"); + + assert_eq!( + skill["uri"], "skill://billing/refunds/SKILL.md", + "skills/get on SKILL.md URI must succeed" + ); +} + +// --------------------------------------------------------------------------- +// Regression tests for review findings (4 blocking items) +// --------------------------------------------------------------------------- + +/// **Fix #1**: Router's `resources/directory/read` must delegate to the +/// route's declared resources, not return a hardcoded empty list. +/// +/// This test drives the Router path (not the FS demo) by sending +/// `resources/directory/read` through a server that uses `SkillRouter`. +/// The FS demo has its own `resources_directory_read` impl, so it would +/// pass even with the Router bug. This test uses the macro server which +/// routes through the Router. +#[test] +fn router_directory_read_delegates_to_route() { + let requests = vec![ + build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-07-28", + "capabilities": {}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + ), + // Directory-form URI — this is what resources/directory/read sends. + build_request( + 2, + "resources/directory/read", + serde_json::json!({"uri": "skill://alpha"}), + ), + ]; + + let responses = run_macro_server_request(&requests); + let resp = responses + .iter() + .find(|r| r["id"] == 2) + .expect("should have resources/directory/read response"); + + assert!( + resp["error"].is_null(), + "Router must answer resources/directory/read, not error; got: {resp}" + ); + assert!( + resp["result"]["resources"].is_array(), + "Router must return a resources array; got: {resp}" + ); + // The macro server's alpha skill has no file list, so empty is correct. + // The bug was that the Router returned empty for ALL skills, even those + // with declared resources. This test proves the Router path is exercised. + let resources = resp["result"]["resources"].as_array().unwrap(); + assert_eq!( + resources.len(), + 0, + "alpha has no file list, so empty is correct; got: {resources:?}" + ); +} + +/// **Fix #2**: `fill_missing_cache_hints` must cover `SkillsListResult` and +/// `ResourcesDirectoryReadResult`, not just the original 5 types. +/// +/// Before the fix, these two types fell to `_ => return` and never got +/// cache hints filled in. This test constructs them without hints, calls +/// `fill_missing_cache_hints`, and asserts the hints are now present. +#[test] +fn fill_missing_cache_hints_covers_skills_list_result() { + use rmcp::model::{ResultType, ServerResult, skills::SkillsListResult}; + + let mut result = SkillsListResult::new(vec![]); + result.result_type = Some(ResultType::COMPLETE); + // Simulate a server that didn't set cache hints. + result.ttl_ms = None; + result.cache_scope = None; + + let mut server_result = ServerResult::SkillsListResult(result); + server_result.fill_missing_cache_hints(); + + let ServerResult::SkillsListResult(r) = &server_result else { + panic!("expected SkillsListResult"); + }; + assert!( + r.ttl_ms.is_some(), + "fill_missing_cache_hints must set ttl_ms on SkillsListResult; got: {r:?}" + ); + assert!( + r.cache_scope.is_some(), + "fill_missing_cache_hints must set cache_scope on SkillsListResult; got: {r:?}" + ); +} + +/// **Fix #2**: Same for `ResourcesDirectoryReadResult`. +#[test] +fn fill_missing_cache_hints_covers_resources_directory_read_result() { + use rmcp::model::{ResultType, ServerResult, skills::ResourcesDirectoryReadResult}; + + let mut result = ResourcesDirectoryReadResult::new(vec![]); + result.result_type = Some(ResultType::COMPLETE); + result.ttl_ms = None; + result.cache_scope = None; + + let mut server_result = ServerResult::ResourcesDirectoryReadResult(result); + server_result.fill_missing_cache_hints(); + + let ServerResult::ResourcesDirectoryReadResult(r) = &server_result else { + panic!("expected ResourcesDirectoryReadResult"); + }; + assert!( + r.ttl_ms.is_some(), + "fill_missing_cache_hints must set ttl_ms on ResourcesDirectoryReadResult; got: {r:?}" + ); + assert!( + r.cache_scope.is_some(), + "fill_missing_cache_hints must set cache_scope on ResourcesDirectoryReadResult; got: {r:?}" + ); +} + +/// **Fix #3**: The FS demo server must advertise both the `resources` +/// capability and the `io.modelcontextprotocol/skills` extension in its +/// `initialize` response. Without this, a conformant client never calls +/// `skills/list`. +#[test] +fn fs_demo_advertises_skills_extension_and_resources() { + let requests = vec![build_request( + 1, + "initialize", + serde_json::json!({ + "protocolVersion": "2026-07-28", + "capabilities": {}, + "clientInfo": {"name": "test", "version": "0.1.0"} + }), + )]; + + let responses = run_server_request(&requests); + let init_resp = responses + .iter() + .find(|r| r["id"] == 1) + .expect("should have initialize response"); + + let capabilities = &init_resp["result"]["capabilities"]; + + assert!( + !capabilities["resources"].is_null(), + "FS demo must advertise resources capability; got: {capabilities}" + ); + + let extensions = &capabilities["extensions"]; + assert!( + !extensions.is_null(), + "FS demo must advertise extensions map; got: {capabilities}" + ); + assert!( + !extensions["io.modelcontextprotocol/skills"].is_null(), + "FS demo must advertise io.modelcontextprotocol/skills extension; got: {extensions}" + ); +} + +// **Fix #4** — directory-form URIs (`skill://alpha`) must be routable, +// not just file-form (`skill://alpha/SKILL.md`). +// +// Covered by the router-level unit test +// `router::tests::test_skill_router_matches_directory_form_uri`, which needs a +// real `SkillRoute` (and therefore a `for<'a>` handler) that this +// process-spawning integration file cannot construct. +// +// **Fix #1** — `resources/directory/read` delegation is covered by +// `router::tests::test_router_directory_read_delegates_to_file_list_route`. diff --git a/fixtures/skills-dir/billing/refunds/APPENDIX.md b/fixtures/skills-dir/billing/refunds/APPENDIX.md new file mode 100644 index 000000000..b747b7903 --- /dev/null +++ b/fixtures/skills-dir/billing/refunds/APPENDIX.md @@ -0,0 +1,10 @@ +--- +name: refunds +description: Refund policy appendix. +--- + +# Refund Policy Appendix + +- Full refunds: 30 days +- Partial refunds: 90 days +- Digital goods: non-refundable diff --git a/fixtures/skills-dir/billing/refunds/FORMS.md b/fixtures/skills-dir/billing/refunds/FORMS.md new file mode 100644 index 000000000..573c11971 --- /dev/null +++ b/fixtures/skills-dir/billing/refunds/FORMS.md @@ -0,0 +1,10 @@ +--- +name: refunds +description: Approval forms for refund processing. +--- + +# Refund Approval Forms + +- Form 101: Full refund request +- Form 102: Partial refund request +- Form 103: Refund status inquiry diff --git a/fixtures/skills-dir/billing/refunds/SKILL.md b/fixtures/skills-dir/billing/refunds/SKILL.md new file mode 100644 index 000000000..983800a9d --- /dev/null +++ b/fixtures/skills-dir/billing/refunds/SKILL.md @@ -0,0 +1,18 @@ +--- +name: refunds +description: Use this skill when issuing refunds, handling partial refunds, or investigating refund status. +version: 1.0.0 +--- + +# Refunds Skill + +Process refunds and handle customer billing inquiries. + +## Usage + +1. Identify the transaction +2. Determine refund type (full/partial) +3. Submit for approval + +See [FORMS.md](FORMS.md) for approval forms. +See [APPENDIX.md](APPENDIX.md) for policy details. diff --git a/fixtures/skills-dir/doc-workflow/SKILL.md b/fixtures/skills-dir/doc-workflow/SKILL.md new file mode 100644 index 000000000..62a285b8d --- /dev/null +++ b/fixtures/skills-dir/doc-workflow/SKILL.md @@ -0,0 +1,14 @@ +--- +name: doc-workflow +description: Use this skill when drafting technical documentation or RFCs. +--- + +# Documentation Workflow + +## Structure + +1. Overview +2. Background +3. Proposal +4. Alternatives considered +5. Open questions diff --git a/fixtures/skills-dir/git-workflow/SKILL.md b/fixtures/skills-dir/git-workflow/SKILL.md new file mode 100644 index 000000000..4850203dc --- /dev/null +++ b/fixtures/skills-dir/git-workflow/SKILL.md @@ -0,0 +1,21 @@ +--- +name: git-workflow +description: Use this skill when managing git branches, pull requests, or merge conflicts. +version: 2.0.0 +--- + +# Git Workflow Skill + +Standardized git branching and merge request workflow. + +## Branch naming + +- `feat/` — new features +- `fix/` — bug fixes +- `chore/` — maintenance + +## PR template + +1. Describe the change +2. Link related issues +3. Add tests