feat(skills): support MCP-served Agent Skills per the skills-over-MCP SEP

Implements the host side of the pre-submission SEP draft at
modelcontextprotocol/experimental-ext-skills#69 ("io.modelcontextprotocol/skills")
so that skills served by connected MCP servers are discovered, surfaced in
the model's context, and loaded on demand through Goose's existing
skills pipeline — identical treatment to filesystem-based skills.

Discovery
- New crates/goose/src/agents/platform_extensions/mcp_skills.rs reads each
  connected server's skill://index.json at extension-connect time (5s
  timeout, graceful on failure) and caches concrete skill entries on the
  owning Extension. Scheme-agnostic: index entries MAY use any URI scheme
  per the SEP, so the cache stores whatever the index gave us.
- Capability declaration (capabilities.extensions["io.modelcontextprotocol/skills"])
  is recognized via rmcp's typed ExtensionCapabilities but not gated on —
  direct resource/read of skill://index.json always runs.

Dynamic instructions
- New McpClientTrait::get_dynamic_instructions hook, queried per-turn from
  ExtensionManager::get_extensions_info. SkillsClient implements it to
  render an MCP-skills section alongside the existing FS list, with
  <server>__<name> disambiguation on name collisions.

Loading
- load_skill now routes to MCP skills by name (or <server>__<name>), with
  supporting-file composition against the cached base_uri. URI-shaped
  inputs are rejected with a redirect hint pointing at read_resource
  (which already exists on the extensionmanager platform extension).

Hardening
- developer/edit.rs: added reject_uri_path guard on file_read, file_write,
  and file_edit so the model can't accidentally Path()-resolve a URI
  (a documented pitfall from fast-agent's SEP host implementation).
- Sharpened extensionmanager's read_resource tool description to steer
  the model toward load_skill for named skills.

API / UI
- SlashCommand gains an optional `origin` field identifying MCP-served
  skills; /config/slash_commands accepts an optional session_id to
  include that session's MCP skills. SkillsView renders a "MCP · <server>"
  pill on MCP-origin entries.

Security (per SEP)
- MCP skill content is treated as untrusted model input. Only `name` and
  `description` are parsed from frontmatter; no hook / pre-post / shell
  fields are honored.

Tests
- 13 new unit tests: MCP index discovery (6), load_skill MCP routing (5),
  URI rejection in developer tools (2).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: olaservo

crates/goose-server/src/routes/config_management.rs

@@ -143,6 +143,10 @@ pub struct SlashCommand {
     pub command: String,
     pub help: String,
     pub command_type: CommandType,
+    /// For MCP-sourced skills, the name of the originating MCP extension
+    /// (e.g. "github"). `None` for filesystem skills and non-skill commands.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub origin: Option<String>,
 }
 #[derive(Serialize, ToSchema)]
 pub struct SlashCommandsResponse {
@@ -395,6 +399,9 @@ pub async fn get_provider_models(
 pub struct SlashCommandsQuery {
     /// Optional working directory to discover local skills from
     pub working_dir: Option<String>,
+    /// Optional session id; when provided, the endpoint also includes
+    /// MCP-served skills from the corresponding agent's connected extensions.
+    pub session_id: Option<String>,
 }
 
 #[utoipa::path(
@@ -406,6 +413,7 @@ pub struct SlashCommandsQuery {
     )
 )]
 pub async fn get_slash_commands(
+    axum::extract::State(state): axum::extract::State<Arc<AppState>>,
     axum::extract::Query(query): axum::extract::Query<SlashCommandsQuery>,
 ) -> Result<Json<SlashCommandsResponse>, ErrorResponse> {
     let mut commands: Vec<_> = slash_commands::list_commands()
@@ -414,6 +422,7 @@ pub async fn get_slash_commands(
             command: command.command.clone(),
             help: command.recipe_path.clone(),
             command_type: CommandType::Recipe,
+            origin: None,
         })
         .collect();
 
@@ -422,6 +431,7 @@ pub async fn get_slash_commands(
             command: cmd_def.name.to_string(),
             help: cmd_def.description.to_string(),
             command_type: CommandType::Builtin,
+            origin: None,
         });
     }
 
@@ -433,9 +443,27 @@ pub async fn get_slash_commands(
             command: source.name,
             help: source.description,
             command_type: CommandType::Skill,
+            origin: None,
         });
     }
 
+    // Session-scoped MCP skills. When a session id is supplied, surface
+    // any skills that session's agent has discovered from connected MCP
+    // extensions. Same CommandType::Skill — the UI distinguishes via
+    // the `origin` badge.
+    if let Some(session_id) = query.session_id.as_deref() {
+        if let Ok(agent) = state.get_agent(session_id.to_string()).await {
+            for entry in agent.extension_manager.aggregated_mcp_skills().await {
+                commands.push(SlashCommand {
+                    command: entry.name,
+                    help: entry.description,
+                    command_type: CommandType::Skill,
+                    origin: Some(entry.server),
+                });
+            }
+        }
+    }
+
     Ok(Json(SlashCommandsResponse { commands }))
 }
 

crates/goose/src/agents/agent.rs

@@ -2051,7 +2051,11 @@ impl Agent {
             .await?;
         let extensions_info = self
             .extension_manager
-            .get_extensions_info(&session.working_dir)
+            .get_extensions_info(
+                session_id,
+                &session.working_dir,
+                tokio_util::sync::CancellationToken::new(),
+            )
             .await;
         tracing::debug!("Retrieved {} extensions info", extensions_info.len());
         let (extension_count, tool_count) = self

crates/goose/src/agents/extension_manager.rs

@@ -77,6 +77,12 @@ struct Extension {
     client: McpClientBox,
     server_info: Option<ServerInfo>,
     _temp_dir: Option<tempfile::TempDir>,
+    /// Cache of MCP-served skills discovered from this extension's
+    /// `skill://index.json` at connect time. Populated synchronously
+    /// during extension registration (see `populate_mcp_skills_cache`).
+    /// Empty for extensions that don't serve a parseable index.
+    /// TODO: invalidate on `notifications/resources/list_changed`.
+    mcp_skills: Vec<crate::agents::platform_extensions::mcp_skills::McpSkillEntry>,
 }
 
 impl Extension {
@@ -86,13 +92,15 @@ impl Extension {
         client: McpClientBox,
         server_info: Option<ServerInfo>,
         temp_dir: Option<tempfile::TempDir>,
+        mcp_skills: Vec<crate::agents::platform_extensions::mcp_skills::McpSkillEntry>,
     ) -> Self {
         Self {
             client,
             config,
             resolved_config,
             server_info,
             _temp_dir: temp_dir,
+            mcp_skills,
         }
     }
 
@@ -900,16 +908,30 @@ impl ExtensionManager {
         };
 
         let server_info = client.get_info().cloned();
+        let client_arc: McpClientBox = Arc::from(client);
+
+        // Populate MCP skills cache before inserting so the first turn's
+        // system prompt sees any skills this server publishes. The fetch
+        // is bounded by `INDEX_FETCH_TIMEOUT`; on timeout/error the cache
+        // is empty and extension registration still succeeds.
+        let mcp_skills = crate::agents::platform_extensions::mcp_skills::fetch_server_skills(
+            &sanitized_name,
+            client_arc.as_ref(),
+            "",
+            CancellationToken::new(),
+        )
+        .await;
 
         let mut extensions = self.extensions.lock().await;
         extensions.insert(
             sanitized_name,
             Extension::new(
                 config,
                 resolved_config,
-                Arc::from(client),
+                client_arc,
                 server_info,
                 temp_dir,
+                mcp_skills,
             ),
         );
         drop(extensions);
@@ -927,26 +949,76 @@ impl ExtensionManager {
         temp_dir: Option<TempDir>,
     ) {
         let normalized = name_to_key(&name);
+
+        // Populate MCP skills cache at connect time. See the doc on
+        // `mcp_skills` above for why this is synchronous.
+        let mcp_skills = crate::agents::platform_extensions::mcp_skills::fetch_server_skills(
+            &normalized,
+            client.as_ref(),
+            "",
+            CancellationToken::new(),
+        )
+        .await;
+
         self.extensions.lock().await.insert(
             normalized,
-            Extension::new(config.clone(), config.clone(), client, info, temp_dir),
+            Extension::new(
+                config.clone(),
+                config.clone(),
+                client,
+                info,
+                temp_dir,
+                mcp_skills,
+            ),
         );
         self.invalidate_tools_cache_and_bump_version().await;
     }
 
-    /// Get extensions info for building the system prompt
-    pub async fn get_extensions_info(&self, working_dir: &std::path::Path) -> Vec<ExtensionInfo> {
+    /// Get extensions info for building the system prompt. Combines each
+    /// extension's static `InitializeResult.instructions` with any per-turn
+    /// dynamic contribution via `McpClientTrait::get_dynamic_instructions`.
+    pub async fn get_extensions_info(
+        &self,
+        session_id: &str,
+        working_dir: &std::path::Path,
+        cancel_token: CancellationToken,
+    ) -> Vec<ExtensionInfo> {
         let working_dir_str = working_dir.to_string_lossy();
-        self.extensions
-            .lock()
-            .await
-            .iter()
-            .map(|(name, ext)| {
-                let instructions = ext.get_instructions().unwrap_or_default();
-                let instructions = instructions.replace("{{WORKING_DIR}}", &working_dir_str);
-                ExtensionInfo::new(name, &instructions, ext.supports_resources())
-            })
-            .collect()
+
+        let snapshots: Vec<(String, String, McpClientBox, bool)> = {
+            let extensions = self.extensions.lock().await;
+            extensions
+                .iter()
+                .map(|(name, ext)| {
+                    (
+                        name.clone(),
+                        ext.get_instructions().unwrap_or_default(),
+                        ext.client.clone(),
+                        ext.supports_resources(),
+                    )
+                })
+                .collect()
+        };
+
+        let mut infos = Vec::with_capacity(snapshots.len());
+        for (name, static_instructions, client, supports_resources) in snapshots {
+            let dynamic = client
+                .get_dynamic_instructions(session_id, cancel_token.clone())
+                .await
+                .unwrap_or_default();
+
+            let combined = if dynamic.is_empty() {
+                static_instructions
+            } else if static_instructions.is_empty() {
+                dynamic
+            } else {
+                format!("{}\n{}", static_instructions, dynamic)
+            };
+
+            let combined = combined.replace("{{WORKING_DIR}}", &working_dir_str);
+            infos.push(ExtensionInfo::new(&name, &combined, supports_resources));
+        }
+        infos
     }
 
     /// Get aggregated usage statistics
@@ -982,6 +1054,19 @@ impl ExtensionManager {
         Ok(self.extensions.lock().await.keys().cloned().collect())
     }
 
+    /// Snapshot every connected extension's cached MCP skill entries. Read
+    /// path for the skills platform extension's per-turn system-prompt
+    /// assembly — no network I/O.
+    pub async fn aggregated_mcp_skills(
+        &self,
+    ) -> Vec<crate::agents::platform_extensions::mcp_skills::McpSkillEntry> {
+        let mut out = Vec::new();
+        for ext in self.extensions.lock().await.values() {
+            out.extend(ext.mcp_skills.iter().cloned());
+        }
+        out
+    }
+
     pub async fn is_extension_enabled(&self, name: &str) -> bool {
         let normalized = name_to_key(name);
         self.extensions.lock().await.contains_key(&normalized)
@@ -1842,7 +1927,8 @@ mod tests {
                 bundled: None,
                 available_tools,
             };
-            let extension = Extension::new(config.clone(), config.clone(), client, None, None);
+            let extension =
+                Extension::new(config.clone(), config.clone(), client, None, None, Vec::new());
             self.extensions
                 .lock()
                 .await

crates/goose/src/agents/mcp_client.rs

@@ -101,6 +101,19 @@ pub trait McpClientTrait: Send + Sync {
         None
     }
 
+    /// Optional per-turn dynamic addition to the extension's instructions.
+    /// Returned text is appended to the static `InitializeResult.instructions`
+    /// when `ExtensionManager::get_extensions_info` assembles the system
+    /// prompt. Called on every reply, so implementations MUST NOT do network
+    /// I/O inline — read from caches instead. Default: no dynamic contribution.
+    async fn get_dynamic_instructions(
+        &self,
+        _session_id: &str,
+        _cancel_token: CancellationToken,
+    ) -> Option<String> {
+        None
+    }
+
     async fn update_working_dir(&self, _new_dir: PathBuf) -> Result<(), Error> {
         Ok(())
     }

crates/goose/src/agents/platform_extensions/developer/edit.rs

@@ -43,6 +43,9 @@ impl EditTools {
         params: FileReadParams,
         working_dir: Option<&Path>,
     ) -> CallToolResult {
+        if let Some(err) = reject_uri_path(&params.path) {
+            return err;
+        }
         let path = resolve_path(&params.path, working_dir);
 
         match fs::read_to_string(&path) {
@@ -67,6 +70,9 @@ impl EditTools {
         params: FileWriteParams,
         working_dir: Option<&Path>,
     ) -> CallToolResult {
+        if let Some(err) = reject_uri_path(&params.path) {
+            return err;
+        }
         let path = resolve_path(&params.path, working_dir);
 
         if let Some(parent) = path.parent() {
@@ -111,6 +117,9 @@ impl EditTools {
         params: FileEditParams,
         working_dir: Option<&Path>,
     ) -> CallToolResult {
+        if let Some(err) = reject_uri_path(&params.path) {
+            return err;
+        }
         let path = resolve_path(&params.path, working_dir);
 
         let content = match fs::read_to_string(&path) {
@@ -225,6 +234,43 @@ pub fn resolve_path(path: &str, working_dir: Option<&Path>) -> PathBuf {
     }
 }
 
+/// Guards file-reading / editing paths against URIs being passed in.
+///
+/// Hosts that expose both a filesystem reader and an MCP resource reader
+/// can trip over `skill://…`, `github://…`, and other scheme-prefixed
+/// strings: the model sometimes tries the filesystem reader on them, and
+/// `PathBuf::from` silently produces a bogus relative path under cwd.
+/// See `Skills SEP host implementation guidelines.md` for the recorded
+/// pitfall. Detect the `<scheme>://` shape and redirect the model toward
+/// `read_resource` / `load_skill` with a clear error.
+pub fn reject_uri_path(path: &str) -> Option<CallToolResult> {
+    // A very narrow predicate: we only want to match `<scheme>://...`,
+    // not paths that happen to contain `://` (rare on POSIX, impossible
+    // on well-formed Windows absolute paths). RFC 3986 schemes start
+    // with ASCII alpha and may contain alphanumerics, `+`, `-`, `.`.
+    let Some(colon_idx) = path.find("://") else {
+        return None;
+    };
+    let scheme = &path[..colon_idx];
+    if scheme.is_empty() {
+        return None;
+    }
+    let mut chars = scheme.chars();
+    let first = chars.next().unwrap();
+    if !first.is_ascii_alphabetic() {
+        return None;
+    }
+    if !chars.all(|c| c.is_ascii_alphanumeric() || c == '+' || c == '-' || c == '.') {
+        return None;
+    }
+
+    Some(CallToolResult::error(vec![Content::text(format!(
+        "'{}' is an MCP resource URI, not a filesystem path. Use the read_resource tool (it takes server + uri) for raw URIs, or load_skill for named skills.",
+        path
+    ))
+    .with_priority(0.0)]))
+}
+
 fn count_lines_before(content: &str, byte_pos: usize) -> usize {
     content
         .char_indices()
@@ -297,6 +343,30 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_reject_uri_path_rejects_schemed_inputs() {
+        // Recognizes the canonical `skill://` scheme as well as domain-
+        // native schemes that the SEP explicitly permits.
+        assert!(reject_uri_path("skill://pull-requests/SKILL.md").is_some());
+        assert!(reject_uri_path("github://owner/repo/skills/x/SKILL.md").is_some());
+        assert!(reject_uri_path("repo://foo").is_some());
+        assert!(reject_uri_path("https://example.com/a").is_some());
+        // RFC 3986 allows +, -, . in scheme; all should be recognized.
+        assert!(reject_uri_path("svn+ssh://host/path").is_some());
+    }
+
+    #[test]
+    fn test_reject_uri_path_passes_through_filesystem_paths() {
+        assert!(reject_uri_path("/absolute/path").is_none());
+        assert!(reject_uri_path("C:\\Users\\me\\file.txt").is_none());
+        assert!(reject_uri_path("relative/path.md").is_none());
+        // A literal colon in the name is not enough — we require `://`.
+        assert!(reject_uri_path("my:file").is_none());
+        // `://` without a valid scheme prefix is passed through.
+        assert!(reject_uri_path("://foo").is_none());
+        assert!(reject_uri_path("1bad://foo").is_none());
+    }
+
     #[test_case(None, None, "line1\nline2\nline3" ; "full content")]
     #[test_case(Some(2), None, "line2\nline3" ; "from line 2")]
     #[test_case(None, Some(2), "line1\nline2\n" ; "limit 2")]