mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Add MCP-based skills discovery (McpSkill, McpSkillsSource, McpSkillResource)
Implement Agent Skills discovery over MCP following the SEP-2640 convention: - McpSkillsSource: reads skill://index.json to discover skills served by an MCP server - McpSkill: lazily fetches SKILL.md content via resources/read on demand - McpSkillResource: wraps MCP resource results (text and binary) - Path traversal protection in get_resource for defense in depth - Samples for Foundry Toolbox and standalone MCP skills server - Comprehensive unit tests (514 lines) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
SergeyMenshykh
@@ -157,6 +157,9 @@ from ._skills import (
|
||||
InlineSkillResource,
|
||||
InlineSkillScript,
|
||||
InMemorySkillsSource,
|
||||
McpSkill,
|
||||
McpSkillResource,
|
||||
McpSkillsSource,
|
||||
Skill,
|
||||
SkillFrontmatter,
|
||||
SkillResource,
|
||||
@@ -425,6 +428,9 @@ __all__ = [
|
||||
"MCPStdioTool",
|
||||
"MCPStreamableHTTPTool",
|
||||
"MCPWebsocketTool",
|
||||
"McpSkill",
|
||||
"McpSkillResource",
|
||||
"McpSkillsSource",
|
||||
"MemoryContextProvider",
|
||||
"MemoryFileStore",
|
||||
"MemoryIndexEntry",
|
||||
|
||||
@@ -44,6 +44,7 @@ Only use skills from trusted sources.
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import base64
|
||||
import inspect
|
||||
import json
|
||||
import logging
|
||||
@@ -60,6 +61,10 @@ from ._sessions import ContextProvider
|
||||
from ._tools import FunctionTool
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from mcp.client.session import ClientSession
|
||||
from mcp.types import ReadResourceResult
|
||||
from pydantic import AnyUrl
|
||||
|
||||
from ._agents import SupportsAgentRun
|
||||
from ._sessions import AgentSession, SessionContext
|
||||
|
||||
@@ -3288,4 +3293,415 @@ class AggregatingSkillsSource(SkillsSource):
|
||||
return result
|
||||
|
||||
|
||||
# region MCP Skills
|
||||
|
||||
|
||||
def _mcp_any_url(uri: str) -> AnyUrl:
|
||||
"""Convert a string URI to a :class:`pydantic.AnyUrl` for MCP client calls."""
|
||||
from pydantic import AnyUrl as _AnyUrl
|
||||
|
||||
return _AnyUrl(uri)
|
||||
|
||||
|
||||
def _mcp_join_text(result: ReadResourceResult) -> str:
|
||||
"""Join all :class:`TextResourceContents` items in a result into a single string."""
|
||||
from mcp.types import TextResourceContents as _TextResourceContents
|
||||
|
||||
return "\n".join(c.text for c in result.contents if isinstance(c, _TextResourceContents))
|
||||
|
||||
|
||||
class _McpSkillIndexEntry: # noqa: B903
|
||||
"""A single entry in the ``skill://index.json`` discovery document.
|
||||
|
||||
All fields are optional to support lenient deserialization; callers
|
||||
validate required fields before use.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
name: str | None = None,
|
||||
type: str | None = None,
|
||||
description: str | None = None,
|
||||
url: str | None = None,
|
||||
digest: str | None = None,
|
||||
) -> None:
|
||||
self.name = name
|
||||
self.type = type
|
||||
self.description = description
|
||||
self.url = url
|
||||
self.digest = digest
|
||||
|
||||
|
||||
class _McpSkillIndex:
|
||||
"""DTO for the ``skill://index.json`` discovery document.
|
||||
|
||||
Represents the Agent Skills Discovery v0.2.0 schema as bound to MCP
|
||||
by SEP-2640.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
schema: str | None = None,
|
||||
skills: list[_McpSkillIndexEntry] | None = None,
|
||||
) -> None:
|
||||
self.schema = schema
|
||||
self.skills: list[_McpSkillIndexEntry] = skills if skills is not None else []
|
||||
|
||||
|
||||
def _parse_mcp_skill_index(text: str) -> _McpSkillIndex:
|
||||
"""Parse a JSON string into a :class:`_McpSkillIndex`.
|
||||
|
||||
Args:
|
||||
text: Raw JSON text from ``skill://index.json``.
|
||||
|
||||
Returns:
|
||||
A populated :class:`_McpSkillIndex` instance.
|
||||
|
||||
Raises:
|
||||
json.JSONDecodeError: If the text is not valid JSON.
|
||||
ValueError: If the top-level value is not a JSON object.
|
||||
"""
|
||||
raw: dict[str, Any] = json.loads(text)
|
||||
|
||||
if not isinstance(raw, dict):
|
||||
raise ValueError("skill://index.json must be a JSON object")
|
||||
|
||||
entries: list[_McpSkillIndexEntry] = []
|
||||
|
||||
raw_skills: list[Any] = raw.get("skills") or []
|
||||
|
||||
for item in raw_skills:
|
||||
if isinstance(item, dict):
|
||||
d = cast(dict[str, Any], item)
|
||||
|
||||
entries.append(
|
||||
_McpSkillIndexEntry(
|
||||
name=d.get("name"),
|
||||
type=d.get("type"),
|
||||
description=d.get("description"),
|
||||
url=d.get("url"),
|
||||
digest=d.get("digest"),
|
||||
)
|
||||
)
|
||||
|
||||
return _McpSkillIndex(schema=raw.get("$schema"), skills=entries)
|
||||
|
||||
|
||||
@experimental(feature_id=ExperimentalFeature.SKILLS)
|
||||
class McpSkillResource(SkillResource):
|
||||
"""A :class:`SkillResource` backed by content fetched from an MCP server.
|
||||
|
||||
The :class:`~mcp.types.ReadResourceResult` is fetched eagerly by
|
||||
:meth:`McpSkill.get_resource` at construction time; :meth:`read`
|
||||
extracts text or binary content from the result.
|
||||
"""
|
||||
|
||||
def __init__(self, *, name: str, result: ReadResourceResult) -> None:
|
||||
"""Initialize an McpSkillResource.
|
||||
|
||||
Args:
|
||||
name: The resource name (e.g. a relative path or identifier).
|
||||
result: The result returned by the MCP server's ``resources/read`` request.
|
||||
"""
|
||||
super().__init__(name=name)
|
||||
self._result = result
|
||||
|
||||
async def read(self, **kwargs: Any) -> Any:
|
||||
"""Read the resource content.
|
||||
|
||||
Returns:
|
||||
A ``bytes`` object when the resource contains binary content,
|
||||
a ``str`` when it contains text, or ``None`` when the server
|
||||
returned no content blocks.
|
||||
"""
|
||||
from mcp.types import BlobResourceContents, TextResourceContents
|
||||
|
||||
for content in self._result.contents:
|
||||
if isinstance(content, BlobResourceContents):
|
||||
return base64.b64decode(content.blob)
|
||||
|
||||
text = "\n".join(c.text for c in self._result.contents if isinstance(c, TextResourceContents))
|
||||
return text if text else None
|
||||
|
||||
|
||||
@experimental(feature_id=ExperimentalFeature.SKILLS)
|
||||
class McpSkill(Skill):
|
||||
"""A :class:`Skill` discovered from an MCP server exposing the Agent Skills convention.
|
||||
|
||||
The skill is constructed from ``skill://index.json`` discovery metadata;
|
||||
:meth:`get_content` fetches the full ``SKILL.md`` content from the MCP
|
||||
server on demand via ``resources/read``.
|
||||
|
||||
Per SEP-2640, resources referenced inside SKILL.md are fetched on demand
|
||||
via the originating MCP server: :meth:`get_resource` resolves a relative
|
||||
resource name against the skill's root URI, issues a ``resources/read``
|
||||
request, and returns an :class:`McpSkillResource` with pre-fetched content.
|
||||
"""
|
||||
|
||||
_SKILL_MD_SUFFIX: Final[str] = "SKILL.md"
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
frontmatter: SkillFrontmatter,
|
||||
skill_md_uri: str,
|
||||
client: ClientSession,
|
||||
) -> None:
|
||||
"""Initialize an McpSkill.
|
||||
|
||||
Args:
|
||||
frontmatter: The parsed frontmatter metadata for this skill.
|
||||
skill_md_uri: The full MCP resource URI of the ``SKILL.md`` resource
|
||||
(e.g. ``skill://unit-converter/SKILL.md``). The skill's root URI
|
||||
is derived by stripping the trailing ``SKILL.md`` segment.
|
||||
client: The MCP client session used to fetch resources on demand.
|
||||
"""
|
||||
self._frontmatter = frontmatter
|
||||
self._skill_md_uri = skill_md_uri
|
||||
self._skill_root_uri = self._compute_skill_root_uri(skill_md_uri)
|
||||
self._client = client
|
||||
self._content: str | None = None
|
||||
|
||||
@property
|
||||
def frontmatter(self) -> SkillFrontmatter:
|
||||
"""The L1 discovery metadata for this skill."""
|
||||
return self._frontmatter
|
||||
|
||||
async def get_content(self) -> str:
|
||||
"""Get the full SKILL.md content from the MCP server.
|
||||
|
||||
Fetches the content via ``resources/read`` on the first call and
|
||||
caches the result for subsequent calls.
|
||||
|
||||
Returns:
|
||||
The SKILL.md content string.
|
||||
|
||||
Raises:
|
||||
ValueError: If the MCP server returned no text content for the
|
||||
SKILL.md resource.
|
||||
"""
|
||||
if self._content is not None:
|
||||
return self._content
|
||||
|
||||
result = await self._client.read_resource(_mcp_any_url(self._skill_md_uri))
|
||||
text = _mcp_join_text(result)
|
||||
if not text:
|
||||
raise ValueError(
|
||||
f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'."
|
||||
)
|
||||
self._content = text
|
||||
return text
|
||||
|
||||
async def get_resource(self, name: str) -> SkillResource | None:
|
||||
"""Get a sibling resource by name from the MCP server.
|
||||
|
||||
Resolves *name* as a relative path against the skill's root URI,
|
||||
issues a ``resources/read`` request to the MCP server, and returns
|
||||
an :class:`McpSkillResource` with the pre-fetched content.
|
||||
|
||||
Args:
|
||||
name: The resource name (e.g. ``references/checklist.md``).
|
||||
|
||||
Returns:
|
||||
An :class:`McpSkillResource`, or ``None`` when the name is empty
|
||||
or the resource does not exist on the server.
|
||||
"""
|
||||
if not name or not name.strip():
|
||||
return None
|
||||
|
||||
normalized = self._validate_resource_name(name)
|
||||
if normalized is None:
|
||||
return None
|
||||
|
||||
uri = self._skill_root_uri + normalized
|
||||
try:
|
||||
result = await self._client.read_resource(_mcp_any_url(uri))
|
||||
except Exception as ex:
|
||||
from mcp.shared.exceptions import McpError
|
||||
|
||||
if isinstance(ex, McpError):
|
||||
return None
|
||||
logger.debug("Unexpected error reading MCP resource '%s'", uri, exc_info=True)
|
||||
return None
|
||||
|
||||
return McpSkillResource(name=name, result=result)
|
||||
|
||||
@staticmethod
|
||||
def _validate_resource_name(name: str) -> str | None:
|
||||
"""Validate a resource name and return the normalized form.
|
||||
|
||||
Defense in depth: refuses names that could escape the skill root
|
||||
(absolute paths, embedded URI schemes, parent-traversal segments).
|
||||
The MCP server is the authority on URI resolution, but rejecting
|
||||
obviously unsafe shapes client-side avoids leaking escape attempts
|
||||
upstream.
|
||||
|
||||
Args:
|
||||
name: The raw resource name to validate.
|
||||
|
||||
Returns:
|
||||
The normalized name with backslashes replaced by forward slashes,
|
||||
or ``None`` if the name is unsafe.
|
||||
"""
|
||||
normalized = name.replace("\\", "/")
|
||||
if (
|
||||
normalized.startswith("/")
|
||||
or "://" in normalized
|
||||
or any(seg == ".." for seg in normalized.split("/"))
|
||||
):
|
||||
logger.debug("Rejecting resource name with unsafe path components: %r", name)
|
||||
return None
|
||||
return normalized
|
||||
|
||||
@staticmethod
|
||||
def _compute_skill_root_uri(skill_md_uri: str) -> str:
|
||||
"""Strip the trailing ``SKILL.md`` from the URI to produce the skill root.
|
||||
|
||||
If the URI doesn't end with ``SKILL.md``, ensures it ends with a
|
||||
trailing slash.
|
||||
"""
|
||||
if skill_md_uri.endswith(McpSkill._SKILL_MD_SUFFIX):
|
||||
return skill_md_uri[: -len(McpSkill._SKILL_MD_SUFFIX)]
|
||||
if skill_md_uri.endswith("/"):
|
||||
return skill_md_uri
|
||||
return skill_md_uri + "/"
|
||||
|
||||
|
||||
@experimental(feature_id=ExperimentalFeature.SKILLS)
|
||||
class McpSkillsSource(SkillsSource):
|
||||
"""A :class:`SkillsSource` that discovers Agent Skills served over MCP.
|
||||
|
||||
Discovery follows the SEP-2640 recommended approach: the source reads
|
||||
the well-known ``skill://index.json`` resource and constructs one
|
||||
:class:`McpSkill` per ``skill-md`` entry directly from the entry's
|
||||
``name``, ``description``, and ``url`` fields.
|
||||
|
||||
The referenced ``SKILL.md`` resource is **not** read during discovery;
|
||||
the host fetches its body on demand via ``resources/read`` when the
|
||||
skill content is needed.
|
||||
|
||||
Only index entries of type ``skill-md`` are supported; entries of any
|
||||
other type are silently skipped.
|
||||
|
||||
If ``skill://index.json`` is absent, unreadable, empty, or fails to
|
||||
parse, this source returns an empty list.
|
||||
|
||||
Examples:
|
||||
.. code-block:: python
|
||||
|
||||
from mcp.client.session import ClientSession
|
||||
|
||||
source = McpSkillsSource(client=session)
|
||||
skills = await source.get_skills()
|
||||
"""
|
||||
|
||||
_INDEX_URI: Final[str] = "skill://index.json"
|
||||
_SKILL_MD_TYPE: Final[str] = "skill-md"
|
||||
|
||||
def __init__(self, client: ClientSession) -> None:
|
||||
"""Initialize an McpSkillsSource.
|
||||
|
||||
Args:
|
||||
client: An MCP client session connected to a server that
|
||||
exposes Agent Skills resources.
|
||||
"""
|
||||
self._client = client
|
||||
|
||||
async def get_skills(self) -> list[Skill]:
|
||||
"""Discover and return skills from the MCP server.
|
||||
|
||||
Reads ``skill://index.json``, parses it, and creates an
|
||||
:class:`McpSkill` for each valid ``skill-md`` entry.
|
||||
|
||||
Returns:
|
||||
A list of discovered :class:`McpSkill` instances.
|
||||
"""
|
||||
index = await self._try_read_index()
|
||||
if index is None:
|
||||
return []
|
||||
|
||||
skills: list[Skill] = []
|
||||
for entry in index.skills:
|
||||
result = self._try_create_skill(entry)
|
||||
if result is not None:
|
||||
skills.append(result)
|
||||
logger.info("Loaded MCP skill: %s", result.frontmatter.name)
|
||||
else:
|
||||
logger.debug(
|
||||
"Skipping skill index entry '%s'",
|
||||
entry.name or "(unnamed)",
|
||||
)
|
||||
|
||||
logger.info("Successfully loaded %d skills from MCP server", len(skills))
|
||||
return skills
|
||||
|
||||
async def _try_read_index(self) -> _McpSkillIndex | None:
|
||||
"""Attempt to read and parse ``skill://index.json`` from the MCP server.
|
||||
|
||||
Returns:
|
||||
A parsed :class:`_McpSkillIndex`, or ``None`` if the index is
|
||||
absent, empty, or malformed.
|
||||
"""
|
||||
try:
|
||||
result = await self._client.read_resource(_mcp_any_url(self._INDEX_URI))
|
||||
except Exception as ex:
|
||||
from mcp.shared.exceptions import McpError
|
||||
|
||||
if isinstance(ex, McpError):
|
||||
logger.debug("No skill://index.json resource available on MCP server: %s", ex)
|
||||
else:
|
||||
logger.warning("Failed to read skill://index.json from MCP server.", exc_info=True)
|
||||
return None
|
||||
|
||||
index_text = _mcp_join_text(result)
|
||||
if not index_text:
|
||||
logger.debug("skill://index.json on MCP server returned empty/non-text contents")
|
||||
return None
|
||||
|
||||
try:
|
||||
return _parse_mcp_skill_index(index_text)
|
||||
except (json.JSONDecodeError, ValueError):
|
||||
logger.warning("Failed to parse skill://index.json JSON document.", exc_info=True)
|
||||
return None
|
||||
|
||||
def _try_create_skill(self, entry: _McpSkillIndexEntry) -> McpSkill | None:
|
||||
"""Attempt to create an :class:`McpSkill` from an index entry.
|
||||
|
||||
Args:
|
||||
entry: A single entry from the skill index.
|
||||
|
||||
Returns:
|
||||
An :class:`McpSkill` if the entry is valid, or ``None`` if the
|
||||
entry should be skipped.
|
||||
"""
|
||||
if entry.type != self._SKILL_MD_TYPE:
|
||||
logger.debug(
|
||||
"Skipping entry '%s': unsupported type '%s'",
|
||||
entry.name or "(unnamed)",
|
||||
entry.type or "(none)",
|
||||
)
|
||||
return None
|
||||
|
||||
if not entry.name or not entry.name.strip():
|
||||
logger.debug("Skipping entry: missing required 'name' field")
|
||||
return None
|
||||
|
||||
if not entry.description or not entry.description.strip():
|
||||
logger.debug("Skipping entry '%s': missing required 'description' field", entry.name)
|
||||
return None
|
||||
|
||||
if not entry.url or not entry.url.strip():
|
||||
logger.debug("Skipping entry '%s': missing required 'url' field", entry.name)
|
||||
return None
|
||||
|
||||
try:
|
||||
fm = SkillFrontmatter(name=entry.name, description=entry.description)
|
||||
except ValueError as ex:
|
||||
logger.debug("Skipping entry '%s': invalid metadata: %s", entry.name, ex)
|
||||
return None
|
||||
|
||||
return McpSkill(frontmatter=fm, skill_md_uri=entry.url, client=self._client)
|
||||
|
||||
|
||||
# endregion
|
||||
|
||||
@@ -0,0 +1,514 @@
|
||||
# Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
"""Tests for MCP-based skills (McpSkillsSource, McpSkill, McpSkillResource)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import base64
|
||||
import json
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
from mcp.shared.exceptions import McpError
|
||||
from mcp.types import (
|
||||
BlobResourceContents,
|
||||
ErrorData,
|
||||
ReadResourceResult,
|
||||
TextResourceContents,
|
||||
)
|
||||
from pydantic import AnyUrl
|
||||
|
||||
from agent_framework import McpSkill, McpSkillResource, McpSkillsSource
|
||||
from agent_framework._skills import _parse_mcp_skill_index
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures & helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
SAMPLE_SKILL_MD = """\
|
||||
---
|
||||
name: unit-converter
|
||||
description: Convert between common units.
|
||||
---
|
||||
# Unit Converter
|
||||
|
||||
Body content here.
|
||||
"""
|
||||
|
||||
SAMPLE_SKILL_INDEX = json.dumps(
|
||||
{
|
||||
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
|
||||
"skills": [
|
||||
{
|
||||
"name": "unit-converter",
|
||||
"type": "skill-md",
|
||||
"description": "Convert between common units.",
|
||||
"url": "skill://unit-converter/SKILL.md",
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _make_text_result(text: str, uri: str = "skill://test") -> ReadResourceResult:
|
||||
"""Create a ReadResourceResult with a single TextResourceContents."""
|
||||
return ReadResourceResult(
|
||||
contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")]
|
||||
)
|
||||
|
||||
|
||||
def _make_blob_result(
|
||||
data: bytes,
|
||||
uri: str = "skill://test",
|
||||
mime_type: str = "application/octet-stream",
|
||||
) -> ReadResourceResult:
|
||||
"""Create a ReadResourceResult with a single BlobResourceContents."""
|
||||
return ReadResourceResult(
|
||||
contents=[BlobResourceContents(uri=AnyUrl(uri), blob=base64.b64encode(data).decode(), mimeType=mime_type)]
|
||||
)
|
||||
|
||||
|
||||
def _make_empty_result() -> ReadResourceResult:
|
||||
"""Create a ReadResourceResult with no contents."""
|
||||
return ReadResourceResult(contents=[])
|
||||
|
||||
|
||||
def _make_client(**read_resource_responses: ReadResourceResult) -> AsyncMock:
|
||||
"""Create a mock ClientSession whose read_resource returns different results per URI.
|
||||
|
||||
Args:
|
||||
**read_resource_responses: Mapping of URI string to ReadResourceResult.
|
||||
Any URI not in this mapping raises McpError (resource not found).
|
||||
"""
|
||||
client = AsyncMock()
|
||||
|
||||
async def _read_resource(uri: AnyUrl) -> ReadResourceResult:
|
||||
uri_str = str(uri)
|
||||
if uri_str in read_resource_responses:
|
||||
return read_resource_responses[uri_str]
|
||||
raise McpError(error=ErrorData(code=-32002, message=f"Resource not found: {uri_str}"))
|
||||
|
||||
client.read_resource = AsyncMock(side_effect=_read_resource)
|
||||
return client
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _parse_mcp_skill_index tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestParseMcpSkillIndex:
|
||||
"""Tests for the _parse_mcp_skill_index helper."""
|
||||
|
||||
def test_parses_valid_index(self) -> None:
|
||||
index = _parse_mcp_skill_index(SAMPLE_SKILL_INDEX)
|
||||
assert index.schema == "https://schemas.agentskills.io/discovery/0.2.0/schema.json"
|
||||
assert len(index.skills) == 1
|
||||
assert index.skills[0].name == "unit-converter"
|
||||
assert index.skills[0].type == "skill-md"
|
||||
assert index.skills[0].url == "skill://unit-converter/SKILL.md"
|
||||
|
||||
def test_parses_empty_skills_array(self) -> None:
|
||||
index = _parse_mcp_skill_index('{"$schema": "test", "skills": []}')
|
||||
assert index.skills == []
|
||||
|
||||
def test_parses_missing_skills_key(self) -> None:
|
||||
index = _parse_mcp_skill_index('{"$schema": "test"}')
|
||||
assert index.skills == []
|
||||
|
||||
def test_raises_on_non_object(self) -> None:
|
||||
with pytest.raises(ValueError, match="must be a JSON object"):
|
||||
_parse_mcp_skill_index("[]")
|
||||
|
||||
def test_raises_on_invalid_json(self) -> None:
|
||||
with pytest.raises(json.JSONDecodeError):
|
||||
_parse_mcp_skill_index("not json")
|
||||
|
||||
def test_skips_non_dict_entries(self) -> None:
|
||||
index = _parse_mcp_skill_index('{"skills": ["not-a-dict", {"name": "ok", "type": "skill-md"}]}')
|
||||
assert len(index.skills) == 1
|
||||
assert index.skills[0].name == "ok"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# McpSkillResource tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestMcpSkillResource:
|
||||
"""Tests for McpSkillResource."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_text_content(self) -> None:
|
||||
result = _make_text_result("hello world")
|
||||
resource = McpSkillResource(name="test.md", result=result)
|
||||
content = await resource.read()
|
||||
assert content == "hello world"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_binary_content(self) -> None:
|
||||
data = bytes([0x01, 0x02, 0x03, 0x04])
|
||||
result = _make_blob_result(data)
|
||||
resource = McpSkillResource(name="icon.bin", result=result)
|
||||
content = await resource.read()
|
||||
assert content == data
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_empty_returns_none(self) -> None:
|
||||
result = _make_empty_result()
|
||||
resource = McpSkillResource(name="empty", result=result)
|
||||
content = await resource.read()
|
||||
assert content is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_multiple_text_contents_joined(self) -> None:
|
||||
result = ReadResourceResult(
|
||||
contents=[
|
||||
TextResourceContents(uri=AnyUrl("skill://a"), text="line1", mimeType="text/plain"),
|
||||
TextResourceContents(uri=AnyUrl("skill://b"), text="line2", mimeType="text/plain"),
|
||||
]
|
||||
)
|
||||
resource = McpSkillResource(name="multi", result=result)
|
||||
content = await resource.read()
|
||||
assert content == "line1\nline2"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_binary_takes_precedence_over_text(self) -> None:
|
||||
data = b"\xff\xfe"
|
||||
result = ReadResourceResult(
|
||||
contents=[
|
||||
TextResourceContents(uri=AnyUrl("skill://a"), text="text", mimeType="text/plain"),
|
||||
BlobResourceContents(
|
||||
uri=AnyUrl("skill://b"),
|
||||
blob=base64.b64encode(data).decode(),
|
||||
mimeType="application/octet-stream",
|
||||
),
|
||||
]
|
||||
)
|
||||
resource = McpSkillResource(name="mixed", result=result)
|
||||
content = await resource.read()
|
||||
# Binary content from BlobResourceContents (second item) should NOT take precedence
|
||||
# since the first item is text. The loop finds text first, but the implementation
|
||||
# checks BlobResourceContents first in the iteration order.
|
||||
# Actually: the implementation iterates all contents looking for BlobResourceContents first.
|
||||
# So the text item (first) is not a blob, the blob item (second) IS a blob -> returns bytes.
|
||||
assert content == data
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# McpSkill tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestMcpSkill:
|
||||
"""Tests for McpSkill."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_content_fetches_and_caches(self) -> None:
|
||||
client = _make_client(**{"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD)})
|
||||
from agent_framework import SkillFrontmatter
|
||||
|
||||
fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.")
|
||||
skill = McpSkill(frontmatter=fm, skill_md_uri="skill://unit-converter/SKILL.md", client=client)
|
||||
|
||||
content1 = await skill.get_content()
|
||||
content2 = await skill.get_content()
|
||||
|
||||
assert "Body content here." in content1
|
||||
assert content1 == content2
|
||||
# Only one MCP call should be made (cached)
|
||||
assert client.read_resource.call_count == 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_content_raises_on_empty(self) -> None:
|
||||
client = _make_client(**{"skill://empty/SKILL.md": _make_empty_result()})
|
||||
from agent_framework import SkillFrontmatter
|
||||
|
||||
fm = SkillFrontmatter(name="empty-skill", description="Empty skill.")
|
||||
skill = McpSkill(frontmatter=fm, skill_md_uri="skill://empty/SKILL.md", client=client)
|
||||
|
||||
with pytest.raises(ValueError, match="no text content"):
|
||||
await skill.get_content()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_resource_text(self) -> None:
|
||||
client = _make_client(
|
||||
**{
|
||||
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
|
||||
"skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"),
|
||||
}
|
||||
)
|
||||
from agent_framework import SkillFrontmatter
|
||||
|
||||
fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.")
|
||||
skill = McpSkill(frontmatter=fm, skill_md_uri="skill://unit-converter/SKILL.md", client=client)
|
||||
|
||||
resource = await skill.get_resource("references/checklist.md")
|
||||
assert resource is not None
|
||||
content = await resource.read()
|
||||
assert content == "- check thing 1\n- check thing 2"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_resource_binary(self) -> None:
|
||||
data = bytes([0x01, 0x02, 0x03, 0x04])
|
||||
client = _make_client(
|
||||
**{
|
||||
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
|
||||
"skill://unit-converter/assets/icon.bin": _make_blob_result(data),
|
||||
}
|
||||
)
|
||||
from agent_framework import SkillFrontmatter
|
||||
|
||||
fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.")
|
||||
skill = McpSkill(frontmatter=fm, skill_md_uri="skill://unit-converter/SKILL.md", client=client)
|
||||
|
||||
resource = await skill.get_resource("assets/icon.bin")
|
||||
assert resource is not None
|
||||
content = await resource.read()
|
||||
assert content == data
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_resource_unknown_returns_none(self) -> None:
|
||||
client = _make_client(**{"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD)})
|
||||
from agent_framework import SkillFrontmatter
|
||||
|
||||
fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.")
|
||||
skill = McpSkill(frontmatter=fm, skill_md_uri="skill://unit-converter/SKILL.md", client=client)
|
||||
|
||||
resource = await skill.get_resource("references/does-not-exist.md")
|
||||
assert resource is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"name",
|
||||
[
|
||||
"../escape.md",
|
||||
"references/../../escape.md",
|
||||
"..",
|
||||
"..\\escape.md",
|
||||
"/etc/passwd",
|
||||
"http://attacker.example.com/payload",
|
||||
],
|
||||
)
|
||||
async def test_get_resource_path_traversal_returns_none(self, name: str) -> None:
|
||||
# Register a permissive mock that would happily return content for any URI,
|
||||
# so the test fails unless the client-side validation rejects the name
|
||||
# before issuing the read.
|
||||
client = AsyncMock()
|
||||
client.read_resource = AsyncMock(return_value=_make_text_result("should never be returned"))
|
||||
|
||||
from agent_framework import SkillFrontmatter
|
||||
|
||||
fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.")
|
||||
skill = McpSkill(frontmatter=fm, skill_md_uri="skill://unit-converter/SKILL.md", client=client)
|
||||
|
||||
resource = await skill.get_resource(name)
|
||||
assert resource is None
|
||||
client.read_resource.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_resource_empty_name_returns_none(self) -> None:
|
||||
client = _make_client()
|
||||
from agent_framework import SkillFrontmatter
|
||||
|
||||
fm = SkillFrontmatter(name="test-skill", description="Test.")
|
||||
skill = McpSkill(frontmatter=fm, skill_md_uri="skill://test/SKILL.md", client=client)
|
||||
|
||||
assert await skill.get_resource("") is None
|
||||
assert await skill.get_resource(" ") is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_script_returns_none(self) -> None:
|
||||
client = _make_client()
|
||||
from agent_framework import SkillFrontmatter
|
||||
|
||||
fm = SkillFrontmatter(name="test-skill", description="Test.")
|
||||
skill = McpSkill(frontmatter=fm, skill_md_uri="skill://test/SKILL.md", client=client)
|
||||
|
||||
assert await skill.get_script("anything") is None
|
||||
|
||||
def test_compute_skill_root_uri_strips_suffix(self) -> None:
|
||||
assert McpSkill._compute_skill_root_uri("skill://unit-converter/SKILL.md") == "skill://unit-converter/"
|
||||
|
||||
def test_compute_skill_root_uri_trailing_slash(self) -> None:
|
||||
assert McpSkill._compute_skill_root_uri("skill://unit-converter/") == "skill://unit-converter/"
|
||||
|
||||
def test_compute_skill_root_uri_no_suffix_adds_slash(self) -> None:
|
||||
assert McpSkill._compute_skill_root_uri("skill://unit-converter") == "skill://unit-converter/"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# McpSkillsSource tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestMcpSkillsSource:
|
||||
"""Tests for McpSkillsSource."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_index_based_discovery_returns_skill(self) -> None:
|
||||
client = _make_client(
|
||||
**{
|
||||
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
|
||||
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
|
||||
}
|
||||
)
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
|
||||
assert len(skills) == 1
|
||||
assert skills[0].frontmatter.name == "unit-converter"
|
||||
assert skills[0].frontmatter.description == "Convert between common units."
|
||||
|
||||
# Content is fetched on demand, not during discovery
|
||||
content = await skills[0].get_content()
|
||||
assert "Body content here." in content
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_index_returns_empty(self) -> None:
|
||||
client = _make_client() # No resources at all
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
assert skills == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_does_not_read_skill_md_during_discovery(self) -> None:
|
||||
# Index points to a skill, but SKILL.md is not registered on the server.
|
||||
# Discovery should succeed because it only reads the index.
|
||||
client = _make_client(
|
||||
**{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")}
|
||||
)
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
|
||||
assert len(skills) == 1
|
||||
assert skills[0].frontmatter.name == "unit-converter"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_invalid_name_is_skipped(self) -> None:
|
||||
index_json = json.dumps(
|
||||
{
|
||||
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
|
||||
"skills": [
|
||||
{
|
||||
"name": "UnitConverter", # Invalid: uppercase
|
||||
"type": "skill-md",
|
||||
"description": "Convert between common units.",
|
||||
"url": "skill://UnitConverter/SKILL.md",
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")})
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
assert skills == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_required_fields_is_skipped(self) -> None:
|
||||
index_json = json.dumps(
|
||||
{
|
||||
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
|
||||
"skills": [
|
||||
{
|
||||
"name": "unit-converter",
|
||||
"type": "skill-md",
|
||||
# Missing description and url
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")})
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
assert skills == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unsupported_type_is_skipped(self) -> None:
|
||||
index_json = json.dumps(
|
||||
{
|
||||
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
|
||||
"skills": [
|
||||
{
|
||||
"name": "some-skill",
|
||||
"type": "archive",
|
||||
"description": "Packaged skill.",
|
||||
"url": "skill://some-skill.tar.gz",
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")})
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
assert skills == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_template_type_is_skipped(self) -> None:
|
||||
index_json = json.dumps(
|
||||
{
|
||||
"$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json",
|
||||
"skills": [
|
||||
{
|
||||
"type": "mcp-resource-template",
|
||||
"description": "Per-product documentation skill",
|
||||
"url": "skill://docs/{product}/SKILL.md",
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")})
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
assert skills == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_empty_index_returns_empty(self) -> None:
|
||||
client = _make_client(
|
||||
**{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")}
|
||||
)
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
assert skills == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_malformed_index_json_returns_empty(self) -> None:
|
||||
client = _make_client(
|
||||
**{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")}
|
||||
)
|
||||
source = McpSkillsSource(client=client)
|
||||
skills = await source.get_skills()
|
||||
assert skills == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sibling_text_resource(self) -> None:
|
||||
client = _make_client(
|
||||
**{
|
||||
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
|
||||
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
|
||||
"skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"),
|
||||
}
|
||||
)
|
||||
source = McpSkillsSource(client=client)
|
||||
skill = (await source.get_skills())[0]
|
||||
resource = await skill.get_resource("references/checklist.md")
|
||||
assert resource is not None
|
||||
content = await resource.read()
|
||||
assert content == "- check thing 1\n- check thing 2"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sibling_binary_resource(self) -> None:
|
||||
data = bytes([0x01, 0x02, 0x03, 0x04])
|
||||
client = _make_client(
|
||||
**{
|
||||
"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"),
|
||||
"skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD),
|
||||
"skill://unit-converter/assets/icon.bin": _make_blob_result(data),
|
||||
}
|
||||
)
|
||||
source = McpSkillsSource(client=client)
|
||||
skill = (await source.get_skills())[0]
|
||||
resource = await skill.get_resource("assets/icon.bin")
|
||||
assert resource is not None
|
||||
content = await resource.read()
|
||||
assert content == data
|
||||
Reference in New Issue
Block a user