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

[olaservo]

Draft — Not for merge into this fork's main. Exists as a reviewable diff for the Skills-over-MCP SEP WG.

Host-side implementation of the pre-submission Skills-over-MCP SEP (io.modelcontextprotocol/skills). Skills served by connected MCP servers under skill:// — or any URI scheme the server chooses, per the SEP's SHOULD-not-MUST stance — are discovered at extension-connect time, surfaced into the per-turn system prompt alongside filesystem skills, and loaded on demand through Goose's existing load_skill tool.

Reference server: olaservo/github-mcp-server#add-agent-skills.

What the model sees

Goose already emits a "You have these skills..." bullet list from SkillsClient's static InitializeResult.instructions. This PR appends a second, parallel section per-turn via a new McpClientTrait::get_dynamic_instructions hook:

You also have these skills from connected MCP servers. Load them via load_skill by name; if a collision is shown in <server>__<name> form, use that exact form:
• pull-requests (github) - Submit a multi-comment GitHub pull request review using the pending-review workflow.

On FS-vs-MCP collision, FS wins and the MCP entry re-renders as <server>__<name>. MCP-vs-MCP collisions also render prefixed.

No new model-facing tool. The existing load_skill(name) handler handles both FS and MCP skills:

• load_skill("pull-requests") — cache lookup, dispatches to ExtensionManager::read_resource.
• load_skill("github__pull-requests") — disambiguation on collision; the model never supplies a server name.
• load_skill("<skill>/references/GUIDE.md") — supporting-file load, scheme-agnostic composition against the cached base_uri.
• On SKILL.md load (FS or MCP), the response appends a "Supporting Files" block listing load_skill(name: "<skill>/<rel>") pointers — FS walks the skill directory; MCP filters the owning server's resources/list response to URIs under base_uri. Only pointers are surfaced; supporting-resource bodies are never auto-loaded alongside SKILL.md.
• URI input (e.g. skill://foo/SKILL.md) is rejected with a redirect to the pre-existing read_resource tool.

Discovery is not gated on the server advertising the capability — resources/read skill://index.json is attempted unconditionally; a not-found / transport error yields an empty cache entry and a debug log. The client also advertises io.modelcontextprotocol/skills: {} in its own ClientCapabilities.extensions.

The developer extension's file tools now reject <scheme>:// input with a redirect to read_resource — prevents a generic file reader from Path()-resolving skill://.../SKILL.md into a bogus filesystem path. The guard is scheme-agnostic and shares the RFC-3986 detector with load_skill's URI rejection.

Skill content from MCP servers is treated as untrusted model input per SEP §Security. Frontmatter parsing extracts only name and description — no hook / pre-post / shell-execution fields are honored.

Code tour

The discovery module is crates/goose/src/agents/platform_extensions/mcp_skills.rs — parses each server's skill://index.json into cached McpSkillEntry { server, name, description, base_uri, uri }, 5s fetch timeout, never propagates errors. The cache lives on Extension.mcp_skills in extension_manager.rs so it's scoped to the owning server's lifecycle; aggregated_mcp_skills() is the per-turn accessor. SkillsClient grows a Weak<ExtensionManager> and a new McpClientTrait::get_dynamic_instructions(session_id) hook (default-None) that renders the MCP section per reply with a 10s-TTL FS-names cache for collision detection. Supporting-file enumeration is lazy: discovery reads only name + description per SKILL.md (FS frontmatter, or the skill://index.json entry for MCP), and the directory walk (FS) / resources/list filter (MCP) only runs when load_skill is invoked — the "only names + descriptions reach the system prompt" contract is visible in the code path, not just the output. ExtensionManager grows one new public method, list_resources_for_server, to back that lazy MCP enumeration. developer's file tools and load_skill's URI-rejection share an RFC-3986 detector in platform_extensions::mod.rs so the two guards can't drift apart. The client advertises io.modelcontextprotocol/skills: {} in ClientCapabilities.extensions via GooseClient::get_info. Server-side: SlashCommand.origin + SlashCommandsQuery.session_id on /config/slash_commands surface MCP skills to the desktop; SkillsView.tsx adds the MCP · <server> pill. The alternative ui/goose2/ surface is not updated.

The extensionmanager::read_resource model-facing tool is deliberately unchanged — the SEP's illustrative read_resource(server, uri) signature already matched.

Bugs surfaced by e2e validation

Empty-session-id permanent client lock. The first draft of ExtensionManager::add_extension / add_client called fetch_server_skills(..., session_id: "", ...) unconditionally. McpClient asserts a single session id across its lifetime — so the empty-string "first session" permanently locked the client, and every subsequent real tool call panicked. Fix: thread the existing session_id: Option<&str> into the fetch call; platform extensions that don't hit a real transport pass None (empty cache, repopulated on the next real-session call).

Platform extensions aren't auto-enabled in ad-hoc Agent builds. The desktop wires up the skills platform extension via config migrations. Ad-hoc Agent::with_config(...) builds — including e2e / scenario runners — do not auto-enable it. Without an explicit add_extension(ExtensionConfig::Platform { name: "skills", ... }), the cache populates but load_skill is never exposed; activation silently fails with the model calling whatever tool is available instead. Fix (in the harness, not production): add the platform extension explicitly after agent construction.

Running it locally

GITHUB_TOKEN=$(gh auth token) \
  cargo test -p goose --test mcp_skills_e2e \
    --no-default-features \
    --features "code-mode,aws-providers,telemetry,otel,rustls-tls" \
    mcp_skills_discovery -- --ignored --nocapture

LLM-in-the-loop e2e lives out-of-tree at skills-over-mcp-ig/clients/goose-harness/ — a sibling cargo crate that path-deps ../goose/crates/goose. Keeps this PR's diff narrow. Harness stands up a real Agent with an Anthropic provider and drives pr-review-input-validation.yaml.

Conformance vs SEP host requirements

Requirement	Where
Read InitializeResult.capabilities.extensions and recognize io.modelcontextprotocol/skills	server_declares_skills_capability in mcp_skills.rs
Enumerate skill:// resources from skill://index.json	fetch_server_skills
Accept non-skill:// URI schemes on indexed entries	base_uri derived by stripping /SKILL.md; scheme never inspected
Tolerate index-absent / malformed-index servers	errors swallowed with debug!/warn!; registration still succeeds
resources/read both index and SKILL.md	yes
Surface skill name + description in model context	SkillsClient::get_dynamic_instructions
Provide a host tool to load skill content	existing load_skill, unified FS + MCP
Resolve relative refs against the skill root	load_skill("<skill>/<rel>") composes against base_uri; MCP hint block enumerated via resources/list filtered to base_uri
MUST NOT honor in-skill local-execution mechanisms without explicit opt-in	frontmatter parser extracts only name + description
Indicate originating server when presenting a skill	server name in prompt catalog; MCP · <server> pill in SkillsView
Guard generic file-reading tools against MCP URIs	developer file tools reject <scheme>://
Client advertises skills-extension capability (optional)	GooseClient::get_info inserts io.modelcontextprotocol/skills: {}

Out of scope (follow-ups)

• type: "mcp-resource-template" entries + MCP completion-API integration for parameterized skills.
• Per-server enable_skills toggle on ExtensionConfig::StreamableHttp / Stdio.
• notifications/resources/list_changed → cache invalidation.
• Mirroring the MCP-origin badge into ui/goose2/.

🤖 Generated with Claude Code