Skip to content

Commit 268d564

Browse files
olaservoclaude
andcommitted
Add Skills-over-MCP SEP support to host
Implements the io.modelcontextprotocol/skills extension (pre-submission SEP at modelcontextprotocol/experimental-ext-skills#69): skills served by connected MCP servers under the skill:// URI scheme are discovered via the well-known skill://index.json resource and surfaced through the existing skills pipeline so they behave identically to filesystem skills. - SkillManifest gains optional uri/server_name; path becomes optional - format_skills_for_prompt renders skill:// URIs as <location>/<directory> for MCP-backed skills and adds an MCP-specific preamble note - New mcp_skills_loader module: fetches skill://index.json, parses concrete skill-md entries, logs SEP capability declaration when servers advertise it (template entries deferred) - SkillReader accepts skill:// URIs, dispatches via aggregator.get_resource, enforces a URI-root trust boundary mirroring the filesystem allowed-dirs - McpAgent.initialize fetches MCP skills post-connect and merges with filesystem manifests; filesystem wins on name collision - MCPServerSettings gains mcp_skills bool for per-server opt-out 16 new unit tests; 80/80 skills suite passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b5c344f commit 268d564

8 files changed

Lines changed: 896 additions & 18 deletions

File tree

src/fast_agent/agents/mcp_agent.py

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,11 @@ async def initialize(self) -> None:
316316
"""
317317
await self.__aenter__()
318318

319+
# Discover Skills-over-MCP skills from connected servers and merge
320+
# them with any filesystem manifests before the instruction template
321+
# is built (so the frontmatter lands in {{agentSkills}}).
322+
await self._load_mcp_skill_manifests()
323+
319324
# Apply template substitution to the instruction with server instructions
320325
await self._apply_instruction_templates()
321326

@@ -583,11 +588,76 @@ def _warn_if_invalid_shell_working_directory(self, working_directory: Path | Non
583588
surface="startup_once",
584589
)
585590

591+
async def _load_mcp_skill_manifests(self) -> None:
592+
"""Fetch skills served by connected MCP servers per the Skills-over-MCP SEP.
593+
594+
Merges discovered MCP manifests with any pre-existing filesystem
595+
manifests and updates the skill reader. On name collision, the
596+
filesystem manifest wins (consistent with SkillRegistry dedup).
597+
Disabled per-server via MCPServerSettings.mcp_skills.
598+
"""
599+
try:
600+
from fast_agent.mcp.mcp_skills_loader import load_mcp_skill_manifests
601+
except Exception: # pragma: no cover - import guard
602+
return
603+
604+
server_names = tuple(self._aggregator.server_names or ())
605+
if not server_names:
606+
return
607+
608+
# Collect per-server opt-out from config (default: enabled).
609+
enabled_servers: set[str] | None = None
610+
if self._context and self._context.config and self._context.config.mcp:
611+
server_settings = self._context.config.mcp.servers or {}
612+
enabled_servers = {
613+
name
614+
for name in server_names
615+
if getattr(server_settings.get(name), "mcp_skills", True)
616+
}
617+
if not enabled_servers:
618+
return
619+
620+
try:
621+
mcp_manifests = await load_mcp_skill_manifests(
622+
self._aggregator,
623+
server_names,
624+
enabled_servers=enabled_servers,
625+
)
626+
except Exception as exc:
627+
self.logger.warning(f"Failed to load MCP skills: {exc}")
628+
return
629+
630+
if not mcp_manifests:
631+
return
632+
633+
filesystem_names = {m.name.lower() for m in self._skill_manifests}
634+
merged = list(self._skill_manifests)
635+
for mcp_manifest in mcp_manifests:
636+
key = mcp_manifest.name.lower()
637+
if key in filesystem_names:
638+
self._record_warning(
639+
f"[dim]MCP-served skill '{mcp_manifest.name}' from server "
640+
f"'{mcp_manifest.server_name}' hidden by local filesystem skill.[/dim]",
641+
surface="startup_once",
642+
)
643+
continue
644+
merged.append(mcp_manifest)
645+
filesystem_names.add(key)
646+
647+
self.set_skill_manifests(merged)
648+
586649
def set_skill_manifests(self, manifests: Sequence[SkillManifest]) -> None:
587650
self._skill_manifests = list(manifests)
588651
self._skill_map = {manifest.name: manifest for manifest in self._skill_manifests}
589652
if self._skill_manifests:
590-
self._skill_reader = SkillReader(self._skill_manifests, self.logger)
653+
# The aggregator is only needed when any manifest is URI-backed
654+
# (Skills-over-MCP), but passing it unconditionally is cheap and
655+
# keeps the reader uniform.
656+
self._skill_reader = SkillReader(
657+
self._skill_manifests,
658+
self.logger,
659+
aggregator=self._aggregator,
660+
)
591661
self._ensure_shell_runtime_for_skills()
592662
else:
593663
self._skill_reader = None

src/fast_agent/config.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,11 @@ class MCPServerSettings(BaseModel):
380380
include_instructions: bool = True
381381
"""Whether to include this server's instructions in the system prompt (default: True)."""
382382

383+
mcp_skills: bool = True
384+
"""Whether to discover and load Skills-over-MCP (io.modelcontextprotocol/skills)
385+
skills from this server (default: True). Set False to suppress reading
386+
`skill://index.json` and its entries from this server."""
387+
383388
reconnect_on_disconnect: bool = True
384389
"""Whether to automatically reconnect when the server session is terminated (e.g., 404).
385390
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
"""Load skills served by connected MCP servers per the Skills-over-MCP SEP.
2+
3+
Implements the `io.modelcontextprotocol/skills` extension: fetches the
4+
well-known `skill://index.json` resource from each connected server, parses
5+
its entries, and builds `SkillManifest` objects backed by `skill://` URIs.
6+
7+
SEP: https://github.com/modelcontextprotocol/experimental-ext-skills/pull/69
8+
"""
9+
10+
from __future__ import annotations
11+
12+
import json
13+
from typing import TYPE_CHECKING, Iterable, Sequence
14+
15+
from mcp.types import TextResourceContents
16+
17+
from fast_agent.core.logging.logger import get_logger
18+
from fast_agent.skills.registry import SkillManifest, SkillRegistry
19+
20+
if TYPE_CHECKING:
21+
from fast_agent.mcp.mcp_aggregator import MCPAggregator
22+
23+
logger = get_logger(__name__)
24+
25+
INDEX_URI = "skill://index.json"
26+
SKILLS_EXTENSION_ID = "io.modelcontextprotocol/skills"
27+
28+
29+
async def load_mcp_skill_manifests(
30+
aggregator: "MCPAggregator",
31+
server_names: Sequence[str],
32+
*,
33+
enabled_servers: set[str] | None = None,
34+
) -> list[SkillManifest]:
35+
"""Fetch and parse skill manifests from connected MCP servers.
36+
37+
For each server, reads `skill://index.json` (optional; missing index is
38+
silently skipped), then fetches each listed `SKILL.md` resource and parses
39+
its frontmatter. Returns one `SkillManifest` per concrete `skill-md` index
40+
entry. `mcp-resource-template` entries are logged and skipped (future work).
41+
42+
Errors from a single server or entry are logged as warnings; a failure
43+
never aborts the whole batch.
44+
"""
45+
46+
manifests: list[SkillManifest] = []
47+
for server_name in server_names:
48+
if enabled_servers is not None and server_name not in enabled_servers:
49+
logger.debug(
50+
"MCP skill discovery disabled for server",
51+
data={"server": server_name},
52+
)
53+
continue
54+
55+
await _log_capability_declaration(aggregator, server_name)
56+
57+
entries = await _read_index(aggregator, server_name)
58+
if not entries:
59+
continue
60+
61+
for entry in entries:
62+
entry_type = entry.get("type")
63+
if entry_type == "mcp-resource-template":
64+
logger.debug(
65+
"Skipping MCP skill template entry (not yet supported)",
66+
data={
67+
"server": server_name,
68+
"url": entry.get("url"),
69+
},
70+
)
71+
continue
72+
if entry_type != "skill-md":
73+
logger.debug(
74+
"Skipping MCP skill entry with unrecognized type",
75+
data={"server": server_name, "type": entry_type},
76+
)
77+
continue
78+
manifest = await _load_concrete_entry(aggregator, server_name, entry)
79+
if manifest is not None:
80+
manifests.append(manifest)
81+
82+
return manifests
83+
84+
85+
async def _log_capability_declaration(
86+
aggregator: "MCPAggregator", server_name: str
87+
) -> None:
88+
"""Observability: note when a server declares the skills extension capability.
89+
90+
Per SEP, servers declare `capabilities.extensions["io.modelcontextprotocol/skills"]`
91+
in their `initialize` response. Hosts don't need the declaration to load
92+
skills (direct resource read works either way), but logging when present
93+
helps diagnose discovery issues.
94+
"""
95+
try:
96+
caps = await aggregator.get_capabilities(server_name)
97+
except Exception as exc: # pragma: no cover - observability only
98+
logger.debug(
99+
"Could not read server capabilities for skills extension check",
100+
data={"server": server_name, "error": str(exc)},
101+
)
102+
return
103+
104+
if caps is None:
105+
return
106+
107+
# ServerCapabilities is a pydantic model; `extensions` may live there or
108+
# in model_extra depending on the MCP SDK version.
109+
extensions = getattr(caps, "extensions", None)
110+
if extensions is None:
111+
model_extra = getattr(caps, "model_extra", None)
112+
if isinstance(model_extra, dict):
113+
extensions = model_extra.get("extensions")
114+
if not extensions:
115+
return
116+
117+
declared = (
118+
extensions.get(SKILLS_EXTENSION_ID)
119+
if isinstance(extensions, dict)
120+
else getattr(extensions, SKILLS_EXTENSION_ID, None)
121+
)
122+
if declared is not None:
123+
logger.info(
124+
"Server declares Skills-over-MCP extension",
125+
data={"server": server_name, "extension": SKILLS_EXTENSION_ID},
126+
)
127+
128+
129+
async def _read_index(
130+
aggregator: "MCPAggregator", server_name: str
131+
) -> list[dict] | None:
132+
"""Read and parse `skill://index.json` from a server; returns None if absent."""
133+
try:
134+
result = await aggregator.get_resource(INDEX_URI, server_name=server_name)
135+
except Exception as exc:
136+
# The SEP treats the index as optional. Absence / lack of resources
137+
# support / network error all fall through to "no indexed skills".
138+
logger.debug(
139+
"No skill index from server",
140+
data={"server": server_name, "error": str(exc)},
141+
)
142+
return None
143+
144+
text = _first_text(result.contents)
145+
if not text:
146+
logger.warning(
147+
"Skill index has no text content",
148+
data={"server": server_name},
149+
)
150+
return None
151+
152+
try:
153+
parsed = json.loads(text)
154+
except Exception as exc:
155+
logger.warning(
156+
"Failed to parse skill index JSON",
157+
data={"server": server_name, "error": str(exc)},
158+
)
159+
return None
160+
161+
skills = parsed.get("skills") if isinstance(parsed, dict) else None
162+
if not isinstance(skills, list):
163+
logger.warning(
164+
"Skill index missing `skills` array",
165+
data={"server": server_name},
166+
)
167+
return None
168+
return [entry for entry in skills if isinstance(entry, dict)]
169+
170+
171+
async def _load_concrete_entry(
172+
aggregator: "MCPAggregator",
173+
server_name: str,
174+
entry: dict,
175+
) -> SkillManifest | None:
176+
url = entry.get("url")
177+
if not isinstance(url, str) or not url:
178+
logger.warning(
179+
"Skill entry missing `url`",
180+
data={"server": server_name, "entry": entry},
181+
)
182+
return None
183+
184+
try:
185+
result = await aggregator.get_resource(url, server_name=server_name)
186+
except Exception as exc:
187+
logger.warning(
188+
"Failed to read MCP skill SKILL.md",
189+
data={"server": server_name, "url": url, "error": str(exc)},
190+
)
191+
return None
192+
193+
text = _first_text(result.contents)
194+
if not text:
195+
logger.warning(
196+
"MCP skill SKILL.md has no text content",
197+
data={"server": server_name, "url": url},
198+
)
199+
return None
200+
201+
manifest, parse_error = SkillRegistry.parse_manifest_text(text)
202+
if manifest is None:
203+
logger.warning(
204+
"Failed to parse MCP skill frontmatter",
205+
data={"server": server_name, "url": url, "error": parse_error},
206+
)
207+
return None
208+
209+
# SEP: the final segment of <skill-path> MUST equal the frontmatter name.
210+
# If index or URI disagree with the frontmatter, frontmatter wins — the
211+
# spec says the skill's identity is its `name` field — but log it.
212+
url_name = _skill_name_from_uri(url)
213+
if url_name and url_name != manifest.name:
214+
logger.warning(
215+
"MCP skill URI final segment differs from frontmatter name",
216+
data={
217+
"server": server_name,
218+
"url": url,
219+
"url_name": url_name,
220+
"frontmatter_name": manifest.name,
221+
},
222+
)
223+
224+
return SkillManifest(
225+
name=manifest.name,
226+
description=manifest.description,
227+
body=manifest.body,
228+
path=None,
229+
license=manifest.license,
230+
compatibility=manifest.compatibility,
231+
metadata=manifest.metadata,
232+
allowed_tools=manifest.allowed_tools,
233+
uri=url,
234+
server_name=server_name,
235+
)
236+
237+
238+
def _first_text(contents: Iterable) -> str | None:
239+
for item in contents:
240+
if isinstance(item, TextResourceContents):
241+
return item.text
242+
return None
243+
244+
245+
def _skill_name_from_uri(url: str) -> str | None:
246+
"""Return the final `<skill-path>` segment from a `skill://.../SKILL.md` URI."""
247+
suffix = "/SKILL.md"
248+
if not url.endswith(suffix):
249+
return None
250+
stem = url[: -len(suffix)]
251+
# Strip scheme: skill://<path...>
252+
scheme_sep = stem.find("://")
253+
if scheme_sep != -1:
254+
stem = stem[scheme_sep + 3 :]
255+
# Last path segment
256+
if "/" in stem:
257+
return stem.rsplit("/", 1)[-1]
258+
return stem or None

0 commit comments

Comments
 (0)