Merge branch 'main' into copilot/create-test-for-handoff-issue

This commit is contained in:
Jacob Alber
2026-05-13 16:52:06 -04:00
committed by GitHub
Unverified
15 changed files with 1005 additions and 365 deletions
+3 -3
View File
@@ -1,14 +1,14 @@
<Project>
<PropertyGroup>
<!-- Central version prefix - applies to all nuget packages. -->
<VersionPrefix>1.5.0</VersionPrefix>
<VersionPrefix>1.6.0</VersionPrefix>
<RCNumber>1</RCNumber>
<DateSuffix>260507</DateSuffix>
<DateSuffix>260512</DateSuffix>
<PackageVersion Condition="'$(IsReleaseCandidate)' == 'true'">$(VersionPrefix)-rc$(RCNumber)</PackageVersion>
<PackageVersion Condition="'$(IsReleaseCandidate)' != 'true' AND '$(VersionSuffix)' != ''">$(VersionPrefix)-$(VersionSuffix).$(DateSuffix).1</PackageVersion>
<PackageVersion Condition="'$(IsReleaseCandidate)' != 'true' AND '$(VersionSuffix)' == ''">$(VersionPrefix)-preview.$(DateSuffix).1</PackageVersion>
<PackageVersion Condition="'$(IsReleased)' == 'true'">$(VersionPrefix)</PackageVersion>
<GitTag>1.5.0</GitTag>
<GitTag>1.6.0</GitTag>
<Configurations>Debug;Release;Publish</Configurations>
<IsPackable>true</IsPackable>
+2 -1
View File
@@ -69,7 +69,8 @@ agent_framework/
### Skills (`_skills.py`)
- **`Skill`** - A skill definition bundling instructions (`content`) with metadata, resources, and scripts. Supports `@skill.resource` and `@skill.script` decorators for adding components.
- **`Skill`** - Abstract base for a skill definition bundling instructions (`content`) with frontmatter metadata, resources, and scripts. Concrete subclasses (`InlineSkill`, `FileSkill`, `ClassSkill`) accept a `frontmatter=SkillFrontmatter(...)` argument carrying the spec fields. Adding new spec fields is done in one place — on `SkillFrontmatter` — keeping the subclass constructors stable.
- **`SkillFrontmatter`** - L1 discovery metadata for a skill (`name`, `description`, `license`, `compatibility`, `allowed_tools`, `metadata`). All fields are mutable plain attributes; the constructor validates `name`, `description`, and `compatibility` against the spec but post-construction assignments are not re-validated. Spec fields are reachable on every skill via `skill.frontmatter`.
- **`SkillResource`** - Named supplementary content attached to a skill; holds either static `content` or a dynamic `function` (sync or async). Exactly one must be provided.
- **`SkillScript`** - An executable script attached to a skill; holds either an inline `function` (code-defined, runs in-process) or a `path` to a file on disk (file-based, delegated to a runner). Exactly one must be provided.
- **`SkillScriptRunner`** - Protocol for file-based script execution. Any callable matching `(skill, script, args) -> Any` satisfies it. Code-defined scripts do not use a runner.
@@ -147,6 +147,7 @@ from ._skills import (
InlineSkillScript,
InMemorySkillsSource,
Skill,
SkillFrontmatter,
SkillResource,
SkillScript,
SkillScriptRunner,
@@ -432,6 +433,7 @@ __all__ = [
"SessionContext",
"SingleEdgeGroup",
"Skill",
"SkillFrontmatter",
"SkillResource",
"SkillScript",
"SkillScriptRunner",
+47 -3
View File
@@ -10,7 +10,7 @@ import logging
import re
import sys
from abc import abstractmethod
from collections.abc import Callable, Collection, Sequence
from collections.abc import Callable, Collection, Coroutine, Sequence
from contextlib import AsyncExitStack, _AsyncGeneratorContextManager # type: ignore
from datetime import timedelta
from functools import partial
@@ -264,6 +264,7 @@ class MCPTool:
self.is_connected: bool = False
self._tools_loaded: bool = False
self._prompts_loaded: bool = False
self._pending_reload_tasks: set[asyncio.Task[None]] = set()
def __str__(self) -> str:
return f"MCPTool(name={self.name}, description={self.description})"
@@ -905,12 +906,47 @@ class MCPTool:
if isinstance(message, types.ServerNotification):
match message.root.method:
case "notifications/tools/list_changed":
await self.load_tools()
self._schedule_reload(self.load_tools())
case "notifications/prompts/list_changed":
await self.load_prompts()
self._schedule_reload(self.load_prompts())
case _:
logger.debug("Unhandled notification: %s", message.root.method)
def _schedule_reload(self, coro: Coroutine[Any, Any, None]) -> None:
"""Schedule a reload coroutine as a background task.
Reloads (load_tools / load_prompts) triggered by MCP server
notifications must NOT be awaited inside the message handler because
the handler runs on the MCP SDK's single-threaded receive loop.
Awaiting a session request (e.g. ``list_tools``) from within that loop
deadlocks: the receive loop cannot read the response while it is
blocked waiting for the handler to return.
Instead we fire the reload as an independent ``asyncio.Task`` and keep
a strong reference in ``_pending_reload_tasks`` so it is not garbage-
collected before completion. Only one reload per kind (tools / prompts)
is kept in flight; a new notification cancels the previous pending task
for the same coroutine name to avoid unbounded growth.
"""
# Cancel-and-replace: only one reload per kind should be in flight.
reload_name = f"mcp-reload:{self.name}:{coro.__qualname__}"
for existing in list(self._pending_reload_tasks):
if existing.get_name() == reload_name and not existing.done():
logger.debug("Cancelling in-flight reload %s; superseded by new notification", reload_name)
existing.cancel()
async def _safe_reload() -> None:
try:
await coro
except asyncio.CancelledError:
raise
except Exception:
logger.warning("Background MCP reload failed", exc_info=True)
task = asyncio.create_task(_safe_reload(), name=reload_name)
self._pending_reload_tasks.add(task)
task.add_done_callback(self._pending_reload_tasks.discard)
def _determine_approval_mode(
self,
*candidate_names: str,
@@ -1047,6 +1083,14 @@ class MCPTool:
params = types.PaginatedRequestParams(cursor=tool_list.nextCursor)
async def _close_on_owner(self) -> None:
# Cancel any pending reload tasks before tearing down the session.
tasks = list(self._pending_reload_tasks)
for task in tasks:
task.cancel()
self._pending_reload_tasks.clear()
if tasks:
await asyncio.gather(*tasks, return_exceptions=True)
await self._safe_close_exit_stack()
self._exit_stack = AsyncExitStack()
self.session = None
+232 -109
View File
@@ -465,41 +465,23 @@ class Skill(ABC):
A skill represents a domain-specific capability with instructions,
resources, and scripts. Concrete implementations include
:class:`FileSkill` (filesystem-backed) and :class:`InlineSkill`
(code-defined).
:class:`FileSkill` (filesystem-backed), :class:`InlineSkill`
(code-defined), and :class:`ClassSkill` (class-based).
Skill metadata follows the
`Agent Skills specification <https://agentskills.io/>`_.
Attributes:
name: Skill name (lowercase letters, numbers, hyphens only).
description: Human-readable description of the skill.
Skill spec metadata (name, description, license, compatibility,
allowed_tools, metadata) is exposed via the :attr:`frontmatter`
property, which returns a :class:`SkillFrontmatter` instance.
"""
def __init__(
self,
*,
name: str,
description: str,
) -> None:
"""Initialize a Skill.
@property
@abstractmethod
def frontmatter(self) -> SkillFrontmatter:
"""The L1 discovery metadata for this skill.
Validates the skill name and description against specification rules.
Args:
name: Skill name (lowercase letters, numbers, hyphens only;
max 64 characters; no leading/trailing/consecutive hyphens).
description: Human-readable description of the skill
(≤1024 characters).
Raises:
ValueError: If the name or description is invalid.
Contains the name, description, and other spec fields as defined by
the `Agent Skills specification <https://agentskills.io/specification>`_.
"""
_validate_skill_name(name)
_validate_skill_description(name, description)
self.name = name
self.description = description
...
@property
@abstractmethod
@@ -535,6 +517,68 @@ class Skill(ABC):
return []
@experimental(feature_id=ExperimentalFeature.SKILLS)
class SkillFrontmatter:
"""L1 discovery metadata for a :class:`Skill`.
Encapsulates all `Agent Skills specification <https://agentskills.io/specification>`_
frontmatter fields in a single object. All fields are mutable plain
attributes; callers may freely reassign them after construction.
The constructor validates ``name``, ``description``, and ``compatibility``
against specification rules and raises :class:`ValueError` on invalid
input. Assignments made after construction are **not** re-validated;
callers are expected to honor the spec.
Attributes:
name: Skill name (lowercase letters, numbers, hyphens only).
description: Human-readable description of the skill.
license: Optional license name or reference.
compatibility: Optional compatibility information (≤500 characters).
allowed_tools: Optional space-delimited pre-approved tool names.
metadata: Optional arbitrary key-value pairs (shallow-copied on
construction to avoid caller-owned dict aliasing).
"""
def __init__(
self,
*,
name: str,
description: str,
license: str | None = None,
compatibility: str | None = None,
allowed_tools: str | None = None,
metadata: dict[str, str] | None = None,
) -> None:
"""Initialize a SkillFrontmatter.
Args:
name: Skill name (lowercase letters, numbers, hyphens only;
max 64 characters; no leading/trailing/consecutive hyphens).
description: Human-readable description of the skill
(≤1024 characters).
license: Optional license name or reference.
compatibility: Optional compatibility information
(≤500 characters).
allowed_tools: Optional space-delimited pre-approved tool names.
metadata: Optional arbitrary key-value pairs.
Raises:
ValueError: If the name, description, or compatibility is invalid.
"""
_validate_skill_name(name)
_validate_skill_description(name, description)
_validate_compatibility(compatibility)
self.name = name
self.description = description
self.compatibility = compatibility
self.license = license
self.allowed_tools = allowed_tools
# Shallow-copy to avoid aliasing with caller-owned dict.
self.metadata: dict[str, str] | None = dict(metadata) if metadata is not None else None
def _validate_skill_name(name: str) -> None:
"""Validate a skill name against specification rules.
@@ -573,6 +617,21 @@ def _validate_skill_description(name: str, description: str) -> None:
)
def _validate_compatibility(compatibility: str | None) -> None:
"""Validate an optional compatibility value against specification rules.
Args:
compatibility: The optional compatibility value to validate.
Raises:
ValueError: If the value exceeds the maximum allowed length.
"""
if compatibility is not None and len(compatibility) > MAX_COMPATIBILITY_LENGTH:
raise ValueError(
f"Skill compatibility must be {MAX_COMPATIBILITY_LENGTH} characters or fewer."
)
def _build_skill_content(
name: str,
description: str,
@@ -639,23 +698,17 @@ class InlineSkill(Skill):
All resources and scripts should be configured before the skill is
registered with a :class:`SkillsProvider`.
Attributes:
name: Skill name (lowercase letters, numbers, hyphens only).
description: Human-readable description of the skill.
instructions: The skill instructions text.
Examples:
With the decorator:
.. code-block:: python
skill = InlineSkill(
name="db-skill",
description="Database operations",
frontmatter=SkillFrontmatter(
name="db-skill",
description="Database operations",
),
instructions="Use this skill for DB tasks.",
)
@skill.resource
def get_schema() -> str:
return "CREATE TABLE ..."
@@ -664,8 +717,7 @@ class InlineSkill(Skill):
def __init__(
self,
*,
name: str,
description: str,
frontmatter: SkillFrontmatter,
instructions: str,
resources: Sequence[SkillResource] | None = None,
scripts: Sequence[SkillScript] | None = None,
@@ -673,19 +725,25 @@ class InlineSkill(Skill):
"""Initialize an InlineSkill.
Args:
name: Skill name (lowercase letters, numbers, hyphens only).
description: Human-readable description of the skill (≤1024 chars).
frontmatter: Skill specification metadata (name, description,
and optional spec fields). Construct a :class:`SkillFrontmatter`
with the desired fields.
instructions: The skill instructions text.
resources: Pre-built resources to attach to this skill.
scripts: Pre-built scripts to attach to this skill.
"""
super().__init__(name=name, description=description)
self._frontmatter = frontmatter
self.instructions = instructions
self._resources: list[SkillResource] = list(resources) if resources is not None else []
self._scripts: list[SkillScript] = list(scripts) if scripts is not None else []
self._cached_content: str | None = None
@property
def frontmatter(self) -> SkillFrontmatter:
"""The L1 discovery metadata for this skill."""
return self._frontmatter
@property
def content(self) -> str:
"""Synthesized XML content with name, description, instructions, resources, and scripts.
@@ -697,7 +755,11 @@ class InlineSkill(Skill):
return self._cached_content
self._cached_content = _build_skill_content(
self.name, self.description, self.instructions, self._resources, self._scripts
self._frontmatter.name,
self._frontmatter.description,
self.instructions,
self._resources,
self._scripts,
)
return self._cached_content
@@ -932,10 +994,6 @@ class ClassSkill(Skill, ABC):
Class-based skills can be distributed via shared libraries or PyPI
packages, making them easy to reuse across projects.
Attributes:
name: Skill name (lowercase letters, numbers, hyphens only).
description: Human-readable description of the skill.
Examples:
Decorator-based (recommended):
@@ -944,8 +1002,10 @@ class ClassSkill(Skill, ABC):
class UnitConverterSkill(ClassSkill):
def __init__(self) -> None:
super().__init__(
name="unit-converter",
description="Convert between common units.",
frontmatter=SkillFrontmatter(
name="unit-converter",
description="Convert between common units.",
),
)
@property
@@ -967,8 +1027,10 @@ class ClassSkill(Skill, ABC):
class UnitConverterSkill(ClassSkill):
def __init__(self) -> None:
super().__init__(
name="unit-converter",
description="Convert between common units.",
frontmatter=SkillFrontmatter(
name="unit-converter",
description="Convert between common units.",
),
)
@property
@@ -989,22 +1051,25 @@ class ClassSkill(Skill, ABC):
def __init__(
self,
*,
name: str,
description: str,
frontmatter: SkillFrontmatter,
) -> None:
"""Initialize a ClassSkill.
Args:
name: Skill name (lowercase letters, numbers, hyphens only;
max 64 characters).
description: Human-readable description of the skill
(≤1024 characters).
frontmatter: Skill specification metadata (name, description,
and optional spec fields). Construct a :class:`SkillFrontmatter`
with the desired fields.
"""
super().__init__(name=name, description=description)
self._frontmatter = frontmatter
self._cached_content: str | None = None
self._cached_resources: list[SkillResource] | None = None
self._cached_scripts: list[SkillScript] | None = None
@property
def frontmatter(self) -> SkillFrontmatter:
"""The L1 discovery metadata for this skill."""
return self._frontmatter
@staticmethod
def resource(
func: Callable[..., Any] | None = None,
@@ -1152,7 +1217,7 @@ class ClassSkill(Skill, ABC):
resource_name = marker.get("name") or _make_method_name(attr_name)
if resource_name in seen_names:
raise ValueError(
f"Skill '{self.name}' already has a resource named '{resource_name}'. "
f"Skill '{self._frontmatter.name}' already has a resource named '{resource_name}'. "
"Ensure each @ClassSkill.resource has a unique name."
)
seen_names.add(resource_name)
@@ -1212,7 +1277,7 @@ class ClassSkill(Skill, ABC):
script_name = marker.get("name") or _make_method_name(attr_name)
if script_name in seen_names:
raise ValueError(
f"Skill '{self.name}' already has a script named '{script_name}'. "
f"Skill '{self._frontmatter.name}' already has a script named '{script_name}'. "
"Ensure each @ClassSkill.script has a unique name."
)
seen_names.add(script_name)
@@ -1240,7 +1305,11 @@ class ClassSkill(Skill, ABC):
return self._cached_content
self._cached_content = _build_skill_content(
self.name, self.description, self.instructions, self.resources, self.scripts
self._frontmatter.name,
self._frontmatter.description,
self.instructions,
self.resources,
self.scripts,
)
return self._cached_content
@@ -1250,16 +1319,13 @@ class FileSkill(Skill):
"""A :class:`Skill` discovered from a filesystem directory backed by a SKILL.md file.
Attributes:
name: Skill name (lowercase letters, numbers, hyphens only).
description: Human-readable description of the skill.
path: Absolute path to the directory containing this skill.
"""
def __init__(
self,
*,
name: str,
description: str,
frontmatter: SkillFrontmatter,
content: str,
path: str,
resources: Sequence[SkillResource] | None = None,
@@ -1268,20 +1334,26 @@ class FileSkill(Skill):
"""Initialize a FileSkill.
Args:
name: Skill name (lowercase letters, numbers, hyphens only).
description: Human-readable description of the skill (≤1024 chars).
frontmatter: Skill specification metadata parsed from the
SKILL.md file's YAML frontmatter (name, description,
and optional spec fields).
content: The full raw SKILL.md file content including YAML frontmatter.
path: Absolute path to the skill directory on disk.
resources: Resources discovered for this skill.
scripts: Scripts discovered for this skill.
"""
super().__init__(name=name, description=description)
self._frontmatter = frontmatter
self._content = content
self.path = path
self._resources: list[SkillResource] = list(resources) if resources is not None else []
self._scripts: list[SkillScript] = list(scripts) if scripts is not None else []
@property
def frontmatter(self) -> SkillFrontmatter:
"""The L1 discovery metadata for this skill."""
return self._frontmatter
@property
def content(self) -> str:
"""The skill content provided at construction time."""
@@ -1346,6 +1418,7 @@ SKILL_FILE_NAME: Final[str] = "SKILL.md"
MAX_SEARCH_DEPTH: Final[int] = 2
MAX_NAME_LENGTH: Final[int] = 64
MAX_DESCRIPTION_LENGTH: Final[int] = 1024
MAX_COMPATIBILITY_LENGTH: Final[int] = 500
DEFAULT_RESOURCE_EXTENSIONS: Final[tuple[str, ...]] = (
".md",
".json",
@@ -1366,10 +1439,24 @@ FRONTMATTER_RE = re.compile(
re.MULTILINE | re.DOTALL,
)
# Matches YAML "key: value" lines. Group 1 = key, Group 2 = quoted value,
# Group 3 = unquoted value.
# Matches top-level YAML "key: value" lines (unindented). Group 1 = key,
# Group 2 = quoted value, Group 3 = unquoted value. Only matches keys at
# column 0 so that indented children (e.g. under "metadata:") are not
# mistakenly captured as top-level fields.
YAML_KV_RE = re.compile(
r"^\s*(\w+)\s*:\s*(?:[\"'](.+?)[\"']|(.+?))\s*$",
r"^([\w-]+)\s*:\s*(?:[\"'](.+?)[\"']|(.+?))\s*$",
re.MULTILINE,
)
# Matches a YAML "metadata:" block followed by indented key-value pairs.
YAML_METADATA_BLOCK_RE = re.compile(
r"^metadata\s*:\s*$\n((?:[ \t]+\S.*\n?)+)",
re.MULTILINE,
)
# Matches indented "key: value" lines within a metadata block.
YAML_INDENTED_KV_RE = re.compile(
r"^\s+([\w-]+)\s*:\s*(?:[\"'](.+?)[\"']|(.+?))\s*$",
re.MULTILINE,
)
@@ -1377,6 +1464,7 @@ YAML_KV_RE = re.compile(
# must not start or end with a hyphen, and must not contain consecutive hyphens.
VALID_NAME_RE = re.compile(r"^[a-z0-9]([a-z0-9]*-[a-z0-9])*[a-z0-9]*$")
# Default system prompt template for advertising available skills to the model.
# Use {skills} as the placeholder for the generated skills XML list.
DEFAULT_SKILLS_INSTRUCTION_PROMPT = """\
@@ -1463,7 +1551,7 @@ class SkillsProvider(ContextProvider):
FileSkillsSource("./skills", script_runner=my_runner),
InMemorySkillsSource([my_code_skill]),
]),
predicate=lambda s: s.name != "internal",
predicate=lambda s: s.frontmatter.name != "internal",
)
)
provider = SkillsProvider(source)
@@ -1698,10 +1786,10 @@ class SkillsProvider(ContextProvider):
lines: list[str] = []
# Sort by name for deterministic output
for skill in sorted(skills, key=lambda s: s.name):
for skill in sorted(skills, key=lambda s: s.frontmatter.name):
lines.append(" <skill>")
lines.append(f" <name>{xml_escape(skill.name)}</name>")
lines.append(f" <description>{xml_escape(skill.description)}</description>")
lines.append(f" <name>{xml_escape(skill.frontmatter.name)}</name>")
lines.append(f" <description>{xml_escape(skill.frontmatter.description)}</description>")
lines.append(" </skill>")
return template.format(
@@ -1920,7 +2008,7 @@ class SkillsProvider(ContextProvider):
def _find_skill(skills: Sequence[Skill], name: str) -> Skill | None:
"""Find a skill by name (case-insensitive linear scan)."""
name_lower = name.lower()
return next((s for s in skills if s.name.lower() == name_lower), None)
return next((s for s in skills if s.frontmatter.name.lower() == name_lower), None)
def _load_skill(self, skills: Sequence[Skill], skill_name: str) -> str:
"""Return the full content for the named skill.
@@ -2179,19 +2267,18 @@ class FileSkillsSource(SkillsSource):
if parsed is None:
continue
name, description, content = parsed
frontmatter, content = parsed
if name in skills:
if frontmatter.name in skills:
logger.warning(
"Duplicate skill name '%s': skill from '%s' skipped in favor of existing skill",
name,
frontmatter.name,
skill_path,
)
continue
file_skill = FileSkill(
name=name,
description=description,
frontmatter=frontmatter,
content=content,
path=skill_path,
)
@@ -2208,8 +2295,8 @@ class FileSkillsSource(SkillsSource):
FileSkillScript(name=sn, full_path=script_full_path, runner=self._script_runner)
)
skills[file_skill.name] = file_skill
logger.info("Loaded skill: %s", file_skill.name)
skills[file_skill.frontmatter.name] = file_skill
logger.info("Loaded skill: %s", file_skill.frontmatter.name)
logger.info("Successfully loaded %d skills", len(skills))
return list(skills.values())
@@ -2438,8 +2525,9 @@ class FileSkillsSource(SkillsSource):
name: str | None,
description: str | None,
source: str,
compatibility: str | None = None,
) -> str | None:
"""Validate a skill's name and description against naming rules.
"""Validate a skill's name, description, and compatibility against naming rules.
Enforces length limits, character-set restrictions, and non-emptiness
for both file-based and code-defined skills.
@@ -2449,6 +2537,7 @@ class FileSkillsSource(SkillsSource):
description: Skill description to validate.
source: Human-readable label for diagnostics (e.g. a file path
or ``"code skill"``).
compatibility: Optional compatibility value to validate.
Returns:
A diagnostic error string if validation fails, or ``None`` if valid.
@@ -2472,24 +2561,32 @@ class FileSkillsSource(SkillsSource):
f"Must be {MAX_DESCRIPTION_LENGTH} characters or fewer."
)
if compatibility is not None and len(compatibility) > MAX_COMPATIBILITY_LENGTH:
return (
f"Skill '{name}' from '{source}' has an invalid compatibility: "
f"Must be {MAX_COMPATIBILITY_LENGTH} characters or fewer."
)
return None
@staticmethod
def _extract_frontmatter(
content: str,
skill_file_path: str,
) -> tuple[str, str] | None:
) -> SkillFrontmatter | None:
"""Extract and validate YAML frontmatter from a SKILL.md file.
Parses the ``---``-delimited frontmatter block for ``name`` and
``description`` fields.
Parses the ``---``-delimited frontmatter block for all
`agentskills.io specification <https://agentskills.io/specification>`_
fields: ``name``, ``description``, ``license``, ``compatibility``,
``allowed-tools``, and ``metadata``.
Args:
content: Raw text content of the SKILL.md file.
skill_file_path: Path to the file (used in diagnostic messages only).
Returns:
A ``(name, description)`` tuple on success, or ``None`` if the
A :class:`SkillFrontmatter` on success, or ``None`` if the
frontmatter is missing, malformed, or fails validation.
"""
match = FRONTMATTER_RE.search(content)
@@ -2500,35 +2597,63 @@ class FileSkillsSource(SkillsSource):
yaml_content = match.group(1).strip()
name: str | None = None
description: str | None = None
license_value: str | None = None
compatibility: str | None = None
allowed_tools: str | None = None
for kv_match in YAML_KV_RE.finditer(yaml_content):
key = kv_match.group(1)
value = kv_match.group(2) if kv_match.group(2) is not None else kv_match.group(3)
if key.lower() == "name":
key_lower = key.lower()
if key_lower == "name":
name = value
elif key.lower() == "description":
elif key_lower == "description":
description = value
elif key_lower == "license":
license_value = value
elif key_lower == "compatibility":
compatibility = value
elif key_lower == "allowed-tools":
allowed_tools = value
error = FileSkillsSource._validate_skill_metadata(name, description, skill_file_path)
# Parse metadata block (indented key-value pairs under "metadata:").
metadata: dict[str, str] | None = None
metadata_match = YAML_METADATA_BLOCK_RE.search(yaml_content)
if metadata_match:
metadata = {}
for kv_match in YAML_INDENTED_KV_RE.finditer(metadata_match.group(1)):
mk = kv_match.group(1)
mv = kv_match.group(2) if kv_match.group(2) is not None else kv_match.group(3)
metadata[mk] = mv
error = FileSkillsSource._validate_skill_metadata(name, description, skill_file_path, compatibility)
if error:
logger.error(error)
return None
# name and description are guaranteed non-None after validation
return name, description # type: ignore[return-value]
# name and description are guaranteed non-None after validation;
# SkillFrontmatter re-validates as a defense-in-depth invariant.
return SkillFrontmatter(
name=cast(str, name),
description=cast(str, description),
license=license_value,
compatibility=compatibility,
allowed_tools=allowed_tools,
metadata=metadata,
)
@staticmethod
def _read_and_parse_skill_file(
skill_dir_path: str,
) -> tuple[str, str, str] | None:
) -> tuple[SkillFrontmatter, str] | None:
"""Read and parse the SKILL.md file in *skill_dir_path*.
Args:
skill_dir_path: Absolute path to the directory containing ``SKILL.md``.
Returns:
A ``(name, description, content)`` tuple where *content* is the
A ``(frontmatter, content)`` tuple where *content* is the
full raw file text, or ``None`` if the file cannot be read or
its frontmatter is invalid.
"""
@@ -2540,23 +2665,21 @@ class FileSkillsSource(SkillsSource):
logger.error("Failed to read SKILL.md at '%s'", skill_file)
return None
result = FileSkillsSource._extract_frontmatter(content, str(skill_file))
if result is None:
frontmatter = FileSkillsSource._extract_frontmatter(content, str(skill_file))
if frontmatter is None:
return None
name, description = result
dir_name = Path(skill_dir_path).name
if name != dir_name:
if frontmatter.name != dir_name:
logger.error(
"SKILL.md at '%s' has frontmatter name '%s' that does not match the directory name '%s'; skipping.",
skill_file,
name,
frontmatter.name,
dir_name,
)
return None
return name, description, content
return frontmatter, content
@staticmethod
def _discover_skill_directories(skill_paths: Sequence[str]) -> list[str]:
@@ -2704,12 +2827,12 @@ class DeduplicatingSkillsSource(DelegatingSkillsSource):
result: list[Skill] = []
for skill in skills:
key = skill.name.lower()
key = skill.frontmatter.name.lower()
if key in seen:
logger.warning(
"Duplicate skill name '%s': skill skipped in favor of existing skill '%s'",
skill.name,
seen[key].name,
skill.frontmatter.name,
seen[key].frontmatter.name,
)
continue
seen[key] = skill
@@ -2730,7 +2853,7 @@ class FilteringSkillsSource(DelegatingSkillsSource):
filtered = FilteringSkillsSource(
inner_source=my_source,
predicate=lambda s: s.name != "internal",
predicate=lambda s: s.frontmatter.name != "internal",
)
skills = await filtered.get_skills()
"""
+111 -1
View File
@@ -1,6 +1,7 @@
# Copyright (c) Microsoft. All rights reserved.
# type: ignore[reportPrivateUsage]
import asyncio
import contextlib
import json
import logging
import os
@@ -1615,7 +1616,7 @@ async def test_mcp_connection_reset_integration():
async def test_mcp_tool_message_handler_notification():
"""Test that message_handler correctly processes tools/list_changed and prompts/list_changed
notifications."""
notifications by scheduling reloads as background tasks."""
tool = MCPStdioTool(name="test_tool", command="python")
# Mock the load_tools and load_prompts methods
@@ -1629,6 +1630,8 @@ async def test_mcp_tool_message_handler_notification():
result = await tool.message_handler(tools_notification)
assert result is None
# The reload is scheduled as a background task; let it run.
await asyncio.sleep(0)
tool.load_tools.assert_called_once()
# Reset mock
@@ -1641,6 +1644,7 @@ async def test_mcp_tool_message_handler_notification():
result = await tool.message_handler(prompts_notification)
assert result is None
await asyncio.sleep(0)
tool.load_prompts.assert_called_once()
# Test unhandled notification
@@ -1664,6 +1668,112 @@ async def test_mcp_tool_message_handler_error():
assert result is None
async def test_mcp_tool_message_handler_does_not_block_receive_loop():
"""Test that message_handler does not deadlock the MCP receive loop.
Regression test for https://github.com/microsoft/agent-framework/issues/4828.
When the MCP server sends a ``notifications/tools/list_changed``
notification, the handler must NOT await ``load_tools()`` synchronously
because that would block the single-threaded MCP receive loop, preventing
it from delivering the ``list_tools`` response — a classic deadlock.
"""
tool = MCPStdioTool(name="test_tool", command="python")
# Use an event to make load_tools block until we release it.
# This simulates load_tools waiting for a session response that the
# receive loop would need to deliver.
release = asyncio.Event()
async def slow_load_tools():
await release.wait()
tool.load_tools = slow_load_tools # type: ignore[assignment]
tools_notification = Mock(spec=types.ServerNotification)
tools_notification.root = Mock()
tools_notification.root.method = "notifications/tools/list_changed"
# message_handler must return immediately even though load_tools blocks.
await tool.message_handler(tools_notification)
# If the handler had awaited load_tools synchronously, we would never
# reach this line (deadlock). Verify the reload task is pending.
assert len(tool._pending_reload_tasks) == 1
# Unblock the reload so the background task finishes cleanly.
release.set()
# Wait for the pending reload task(s) to complete so their done-callbacks
# have a chance to remove them from _pending_reload_tasks.
await asyncio.wait_for(asyncio.gather(*tool._pending_reload_tasks), timeout=1)
assert len(tool._pending_reload_tasks) == 0
async def test_mcp_tool_message_handler_reload_failure_is_logged(caplog: pytest.LogCaptureFixture):
"""Background reload errors are logged, not raised into the receive loop."""
tool = MCPStdioTool(name="test_tool", command="python")
tool.load_tools = AsyncMock(side_effect=RuntimeError("connection lost"))
tools_notification = Mock(spec=types.ServerNotification)
tools_notification.root = Mock()
tools_notification.root.method = "notifications/tools/list_changed"
await tool.message_handler(tools_notification)
# Let the background task run — it should not propagate the exception.
# Snapshot tasks and await them to ensure done-callbacks fire.
pending = list(tool._pending_reload_tasks)
if pending:
await asyncio.wait_for(asyncio.gather(*pending, return_exceptions=True), timeout=1)
tool.load_tools.assert_called_once()
assert len(tool._pending_reload_tasks) == 0
# Verify the warning was actually logged with exception info.
reload_warnings = [r for r in caplog.records if "Background MCP reload failed" in r.message]
assert len(reload_warnings) == 1
assert reload_warnings[0].levelname == "WARNING"
assert reload_warnings[0].exc_info is not None
async def test_mcp_tool_message_handler_cancel_and_replace():
"""Sending two notifications in quick succession cancels the first reload task."""
tool = MCPStdioTool(name="test_tool", command="python")
release = asyncio.Event()
call_count = 0
async def blocking_load_tools():
nonlocal call_count
call_count += 1
await release.wait()
tool.load_tools = blocking_load_tools # type: ignore[assignment]
notification = Mock(spec=types.ServerNotification)
notification.root = Mock()
notification.root.method = "notifications/tools/list_changed"
# First notification — starts a blocking reload task.
await tool.message_handler(notification)
assert len(tool._pending_reload_tasks) == 1
first_task = next(iter(tool._pending_reload_tasks))
# Second notification — should cancel the first and replace it.
await tool.message_handler(notification)
# Yield to the event loop so the cancellation is processed.
with contextlib.suppress(asyncio.CancelledError):
await first_task
assert first_task.cancelled()
assert len(tool._pending_reload_tasks) == 1
second_task = next(iter(tool._pending_reload_tasks))
assert second_task is not first_task
# Unblock and let the second task finish.
release.set()
await asyncio.wait_for(asyncio.gather(*tool._pending_reload_tasks), timeout=1)
assert len(tool._pending_reload_tasks) == 0
async def test_mcp_tool_sampling_callback_no_client():
"""Test sampling callback error path when no chat client is available."""
tool = MCPStdioTool(name="test_tool", command="python")
File diff suppressed because it is too large Load Diff
@@ -11,7 +11,7 @@ import os
from textwrap import dedent
from typing import Any
from agent_framework import Agent, InlineSkill, InlineSkillResource, SkillsProvider
from agent_framework import Agent, InlineSkill, InlineSkillResource, SkillFrontmatter, SkillsProvider
from agent_framework.foundry import FoundryChatClient
from azure.identity import AzureCliCredential
from dotenv import load_dotenv
@@ -47,8 +47,9 @@ load_dotenv()
# 1. Static Resources — inline content passed at construction time
# ---------------------------------------------------------------------------
unit_converter_skill = InlineSkill(
name="unit-converter",
description="Convert between common units using a conversion factor",
frontmatter=SkillFrontmatter(
name="unit-converter", description="Convert between common units using a conversion factor"
),
instructions=dedent("""\
Use this skill when the user asks to convert between units.
@@ -1,6 +1,12 @@
---
name: unit-converter
description: Convert between common units using a multiplication factor. Use when asked to convert miles, kilometers, pounds, or kilograms.
license: MIT
compatibility: Works with any model that supports tool use.
allowed-tools: convert
metadata:
author: agent-framework-samples
version: "1.0"
---
## Usage
@@ -21,6 +21,7 @@ from agent_framework import (
FileSkillsSource,
InlineSkill,
InMemorySkillsSource,
SkillFrontmatter,
SkillsProvider,
)
from agent_framework.foundry import FoundryChatClient
@@ -73,8 +74,9 @@ load_dotenv()
# ---------------------------------------------------------------------------
volume_converter_skill = InlineSkill(
name="volume-converter",
description="Convert between gallons and liters using a conversion factor",
frontmatter=SkillFrontmatter(
name="volume-converter", description="Convert between gallons and liters using a conversion factor"
),
instructions=dedent("""\
Use this skill when the user asks to convert between gallons and liters.
@@ -118,6 +120,7 @@ def convert_volume(value: float, factor: float) -> str:
# 2. Define a class-based skill for temperature conversion
# ---------------------------------------------------------------------------
class TemperatureConverterSkill(ClassSkill):
"""A temperature-converter skill defined as a Python class.
@@ -127,8 +130,10 @@ class TemperatureConverterSkill(ClassSkill):
def __init__(self) -> None:
super().__init__(
name="temperature-converter",
description="Convert between temperature scales (Fahrenheit, Celsius, Kelvin).",
frontmatter=SkillFrontmatter(
name="temperature-converter",
description="Convert between temperature scales (Fahrenheit, Celsius, Kelvin).",
)
)
@property
@@ -178,6 +183,7 @@ class TemperatureConverterSkill(ClassSkill):
# 3. Wire everything together and run the agent
# ---------------------------------------------------------------------------
async def main() -> None:
"""Run the combined skills demo."""
endpoint = os.environ["FOUNDRY_PROJECT_ENDPOINT"]
@@ -1,6 +1,12 @@
---
name: unit-converter
description: Convert between common units using a multiplication factor. Use when asked to convert miles, kilometers, pounds, or kilograms.
license: MIT
compatibility: Works with any model that supports tool use.
allowed-tools: convert
metadata:
author: agent-framework-samples
version: "1.0"
---
## Usage
@@ -9,7 +9,7 @@ import os
# warnings.filterwarnings("ignore", message=r"\[SKILLS\].*", category=FutureWarning)
from textwrap import dedent
from agent_framework import Agent, InlineSkill, SkillsProvider
from agent_framework import Agent, InlineSkill, SkillFrontmatter, SkillsProvider
from agent_framework.foundry import FoundryChatClient
from azure.identity import AzureCliCredential
from dotenv import load_dotenv
@@ -43,8 +43,9 @@ load_dotenv()
# Define a code skill with a script that performs a sensitive operation
deployment_skill = InlineSkill(
name="deployment",
description="Tools for deploying application versions to production",
frontmatter=SkillFrontmatter(
name="deployment", description="Tools for deploying application versions to production"
),
instructions=dedent("""\
Use this skill when the user asks to deploy an application.
@@ -75,7 +75,7 @@ async def main() -> None:
FilteringSkillsSource(
FileSkillsSource(str(skills_dir), script_runner=subprocess_script_runner),
# Only keep the volume-converter skill
predicate=lambda s: s.name != "length-converter",
predicate=lambda s: s.frontmatter.name != "length-converter",
)
)
@@ -1,6 +1,12 @@
---
name: length-converter
description: Convert between common length units (miles, km, feet, meters) using a multiplication factor.
license: MIT
compatibility: Works with any model that supports tool use.
allowed-tools: convert
metadata:
author: agent-framework-samples
version: "1.0"
---
## Usage
@@ -1,6 +1,12 @@
---
name: volume-converter
description: Convert between gallons and liters using a conversion factor.
license: MIT
compatibility: Works with any model that supports tool use.
allowed-tools: convert
metadata:
author: agent-framework-samples
version: "1.0"
---
## Usage