diff --git a/src/agentpool/delegation/pool.py b/src/agentpool/delegation/pool.py index 3a3373890..759252660 100644 --- a/src/agentpool/delegation/pool.py +++ b/src/agentpool/delegation/pool.py @@ -10,7 +10,7 @@ from anyenv import ProcessManager import anyio -from upathtools import UPath +from upathtools import UPath, to_upath from agentpool.common_types import NodeName, SupportsStructuredOutput from agentpool.delegation.message_flow_tracker import MessageFlowTracker @@ -101,10 +101,13 @@ def __init__( # noqa: PLR0915 match manifest: case None: self.manifest = AgentsManifest() + self._config_file_path: UPath | None = None case str() | os.PathLike() | UPath(): + self._config_file_path = to_upath(manifest) self.manifest = AgentsManifest.from_file(manifest) case AgentsManifest(): self.manifest = manifest + self._config_file_path = None case _: raise ValueError(f"Invalid config type: {type(manifest)}") registry.configure_observability(self.manifest.observability) @@ -124,7 +127,12 @@ def __init__( # noqa: PLR0915 self.connection_registry = ConnectionRegistry() servers = self.manifest.get_mcp_servers() self.mcp = MCPManager(name="pool_mcp", servers=servers, owner="pool") - self.skills = SkillsManager(name="pool_skills", owner="pool") + self.skills = SkillsManager( + name="pool_skills", + owner="pool", + config=self.manifest.skills, + config_file_path=self._config_file_path, + ) self._tasks = TaskRegistry() self.prompt_manager = PromptManager(self.manifest.prompts) # Main agent name: explicit param > manifest.default_agent > None (will use first) diff --git a/src/agentpool/models/manifest.py b/src/agentpool/models/manifest.py index 25f6acd97..0949c9d73 100644 --- a/src/agentpool/models/manifest.py +++ b/src/agentpool/models/manifest.py @@ -25,6 +25,7 @@ from agentpool_config.observability import ObservabilityConfig from agentpool_config.output_types import StructuredResponseConfig from agentpool_config.pool_server import MCPPoolServerConfig +from agentpool_config.skills import SkillsConfig from agentpool_config.storage import StorageConfig from agentpool_config.system_prompts import PromptLibraryConfig from agentpool_config.task import Job @@ -329,6 +330,22 @@ class AgentsManifest(Schema): Docs: https://phil65.github.io/agentpool/YAML%20Configuration/prompt_configuration/ """ + skills: SkillsConfig = Field(default_factory=SkillsConfig) + """Custom skill discovery paths configuration. + + Defines where to search for custom skills. Skills are discovered from + configured directories following "first path wins" semantics. + + Example: + ```yaml + skills: + paths: + - ./my-skills + - s3://bucket/skills + include_default: true + ``` + """ + commands: dict[str, CommandConfig | str] = Field( default_factory=dict, examples=[ diff --git a/src/agentpool/skills/manager.py b/src/agentpool/skills/manager.py index fc69829ca..d84b8da0d 100644 --- a/src/agentpool/skills/manager.py +++ b/src/agentpool/skills/manager.py @@ -8,11 +8,12 @@ from agentpool.log import get_logger from agentpool.skills.registry import SkillsRegistry +from agentpool_config.skills import SkillsConfig # noqa: TC001 if TYPE_CHECKING: from fsspec import AbstractFileSystem - from upathtools import JoinablePathLike + from upathtools import JoinablePathLike, UPath from agentpool.skills.skill import Skill @@ -33,6 +34,8 @@ def __init__( name: str = "skills", owner: str | None = None, skills_dirs: list[JoinablePathLike] | None = None, + config: SkillsConfig | None = None, + config_file_path: UPath | None = None, ) -> None: """Initialize the skills manager. @@ -40,20 +43,24 @@ def __init__( name: Name for this manager owner: Owner of this manager skills_dirs: Directories to search for skills + config: Optional skills configuration from manifest + config_file_path: Optional path to configuration file for resolving relative paths """ self.name = name self.owner = owner self.registry = SkillsRegistry(skills_dirs) self._initialized = False + self._config = config + self._config_file_path = config_file_path def __repr__(self) -> str: skill_count = len(self.registry.list_items()) if self._initialized else "?" return f"SkillsManager(name={self.name!r}, skills={skill_count})" async def __aenter__(self) -> Self: - """Initialize the skills manager and discover skills.""" + """Initialize to skills manager and discover skills.""" try: - await self.registry.discover_skills() + await self.discover_skills(self._config, self._config_file_path) self._initialized = True count = len(self.registry.list_items()) logger.info("Skills manager initialized", name=self.name, skill_count=count) @@ -101,9 +108,38 @@ async def add_skills_directory( await self.registry.register_skills_from_path(upath) logger.info("Added skills directory", path=str(path)) + async def discover_skills( + self, + config: SkillsConfig | None = None, + config_file_path: UPath | None = None, + ) -> None: + """Discover skills from configured paths. + + Args: + config: Optional skills configuration. + config_file_path: Optional path to the configuration file for resolving relative paths. + """ + from agentpool_config.skills import DEFAULT_SKILLS_PATHS + + default_paths = [p.expanduser() for p in DEFAULT_SKILLS_PATHS] + if config: + paths = config.get_effective_paths(config_file_path) + else: + paths = self.registry.skills_dirs + + for path in reversed(paths): + upath = to_upath(path).expanduser() + if not upath.exists(): + if any(upath == dp for dp in default_paths): + logger.debug("Default skills directory not found", path=upath) + else: + logger.warning("Custom skills directory not found", path=upath) + continue + await self.registry.register_skills_from_path(upath, replace=True) + async def refresh(self) -> None: """Force rediscovery of all skills.""" - await self.registry.discover_skills() + await self.discover_skills() skill_count = len(self.registry.list_items()) logger.info("Skills refreshed", name=self.name, skill_count=skill_count) diff --git a/src/agentpool/skills/registry.py b/src/agentpool/skills/registry.py index 39cf2f5c3..c5a2d9ae0 100644 --- a/src/agentpool/skills/registry.py +++ b/src/agentpool/skills/registry.py @@ -55,6 +55,7 @@ async def register_skills_from_path( self, skills_dir: JoinablePathLike | AbstractFileSystem, base_path: str | None = None, + replace: bool = True, **storage_options: Any, ) -> None: """Register skills from a given path. @@ -63,6 +64,7 @@ async def register_skills_from_path( skills_dir: Path to the directory containing skills, or filesystem instance. base_path: When skills_dir is a filesystem, the path within that filesystem to look for skills. Defaults to root_marker if not specified. + replace: Whether to replace existing skills with same name. storage_options: Additional options to pass to the filesystem. """ if isinstance(skills_dir, AbstractFileSystem): @@ -80,7 +82,7 @@ async def register_skills_from_path( # List entries in skills directory entries = await fs._ls(search_path, detail=True) except FileNotFoundError: - logger.warning("Skills directory not found", path=search_path) + logger.debug("Skills directory not found", path=search_path) return # Filter for directories that might contain skills skill_dirs = [ @@ -111,7 +113,7 @@ async def register_skills_from_path( try: skill = self._parse_skill(skill_dir_path) - self.register(skill.name, skill, replace=True) + self.register(skill.name, skill, replace=replace) except Exception as e: # noqa: BLE001 # Log but don't fail discovery for one bad skill print(f"Warning: Failed to parse skill at {skill_dir_path}: {e}") diff --git a/src/agentpool_config/__init__.py b/src/agentpool_config/__init__.py index 1cd800144..db19d59e2 100644 --- a/src/agentpool_config/__init__.py +++ b/src/agentpool_config/__init__.py @@ -36,6 +36,7 @@ PromptHookConfig, ) from agentpool_config.toolsets import ToolsetConfig +from agentpool_config.skills import SkillsConfig, DEFAULT_SKILLS_PATHS from agentpool_config.resolution import ( ConfigLayer, ConfigSource, @@ -64,6 +65,7 @@ Field(discriminator="type"), ] __all__ = [ + "DEFAULT_SKILLS_PATHS", "AnyToolConfig", "BaseEventHandlerConfig", "BaseHookConfig", @@ -84,6 +86,7 @@ "ResolvedConfig", "SSEMCPServerConfig", "SessionQuery", + "SkillsConfig", "StdioMCPServerConfig", "StdoutEventHandlerConfig", "StreamableHTTPMCPServerConfig", diff --git a/src/agentpool_config/skills.py b/src/agentpool_config/skills.py index f6d2178dd..1aa0d59dc 100644 --- a/src/agentpool_config/skills.py +++ b/src/agentpool_config/skills.py @@ -1,17 +1,97 @@ """Skills configuration.""" -from dataclasses import dataclass +from __future__ import annotations +from pydantic import ConfigDict, Field +from schemez import Schema +from upathtools import UPath -@dataclass -class Skill: - """Skill configuration.""" - url: str - name: str +DEFAULT_SKILLS_PATHS = [ + UPath("~/.claude/skills/"), + UPath(".claude/skills/"), +] -dev_browser = Skill( - url="https://github.com/SawyerHood/dev-browser/tree/main/skills/dev-browser", - name="dev-browser", -) +class SkillsConfig(Schema): + """Configuration for custom skill discovery paths. + + Skills are discovered from configured directories, allowing + users to add custom skills from local paths. The discovery + follows "first path wins" semantics - earlier paths in the list + take precedence over later ones. + + Default paths (when include_default=True): + - ~/.claude/skills/ (user home directory) + - .claude/skills/ (relative to current directory) + """ + + model_config = ConfigDict( + json_schema_extra={ + "x-icon": "octicon:mortar-board-16", + "x-doc-title": "Skills Configuration", + } + ) + + paths: list[UPath] = Field( + default_factory=list, + title="Custom skill paths", + examples=[["/path/to/skills", "./my-skills", "s3://bucket/skills"]], + ) + """List of custom paths to search for skills. + + Paths can be: + - Absolute: /home/user/skills + - Relative: ./my-skills (resolved against config file location or CWD) + - Remote: s3://bucket/skills, github://org/repo/skills + + Earlier paths take precedence over later ones ("first path wins"). + """ + + include_default: bool = Field( + default=True, + title="Include default paths", + examples=[True, False], + ) + """Whether to include default skill paths in discovery. + + Default paths are appended after custom paths: + - ~/.claude/skills/ + - .claude/skills/ + + Set to False to disable default paths entirely. + """ + + def get_effective_paths(self, config_file_path: UPath | None = None) -> list[UPath]: + """Get the effective list of paths for skill discovery. + + Resolves relative paths against the config file location (if provided) + or current working directory, then appends default paths if enabled. + + Args: + config_file_path: Path to the YAML configuration file. + Relative paths in self.paths are resolved against this file's + parent directory. If None, relative paths are resolved against + the current working directory. + + Returns: + List of UPath objects for skill discovery, ordered by priority + (custom paths first, then default paths if enabled). + """ + result: list[UPath] = [] + + # Resolve custom paths + base_path = config_file_path.parent if config_file_path is not None else UPath.cwd() + + for path in self.paths: + if path.is_absolute(): + result.append(path) + else: + # Resolve relative paths against base path and normalize + result.append((base_path / path).resolve()) + + # Append default paths if enabled + if self.include_default: + result.extend(DEFAULT_SKILLS_PATHS) + + return result diff --git a/tests/test_config/__init__.py b/tests/test_config/__init__.py new file mode 100644 index 000000000..56da449c6 --- /dev/null +++ b/tests/test_config/__init__.py @@ -0,0 +1 @@ +"""Tests for configuration models.""" diff --git a/tests/test_config/test_skills_config.py b/tests/test_config/test_skills_config.py new file mode 100644 index 000000000..5300ec6b3 --- /dev/null +++ b/tests/test_config/test_skills_config.py @@ -0,0 +1,197 @@ +"""Tests for SkillsConfig model.""" + +from __future__ import annotations + +import pytest +from upathtools import UPath + +from agentpool_config.skills import DEFAULT_SKILLS_PATHS, SkillsConfig + + +def test_skills_config_default_values(): + """Test SkillsConfig with default values.""" + config = SkillsConfig() + + assert config.paths == [] + assert config.include_default is True + + +def test_skills_config_with_custom_paths(): + """Test SkillsConfig with custom paths.""" + config = SkillsConfig(paths=[UPath("./my-skills"), UPath("/absolute/path")]) + + assert len(config.paths) == 2 + assert config.paths[0] == UPath("./my-skills") + assert config.paths[1] == UPath("/absolute/path") + assert config.include_default is True + + +def test_skills_config_include_default_false(): + """Test SkillsConfig with include_default set to False.""" + config = SkillsConfig(include_default=False) + + assert config.paths == [] + assert config.include_default is False + + +def test_get_effective_paths_custom_only(): + """Test get_effective_paths with custom paths only (no defaults).""" + config = SkillsConfig( + paths=[UPath("./skills"), UPath("/absolute/skills")], + include_default=False, + ) + + result = config.get_effective_paths() + + assert len(result) == 2 + # Custom paths should be resolved to absolute + assert result[0].is_absolute() + assert str(result[0]).endswith("skills") + assert result[1] == UPath("/absolute/skills") + + +def test_get_effective_paths_with_defaults(): + """Test get_effective_paths includes default paths when enabled.""" + config = SkillsConfig( + paths=[UPath("./custom-skills")], + include_default=True, + ) + + result = config.get_effective_paths() + + assert len(result) == 3 + # First should be custom path (resolved to absolute) + assert result[0].is_absolute() + assert str(result[0]).endswith("custom-skills") + # Last two should be default paths + assert result[1] == DEFAULT_SKILLS_PATHS[0] # ~/.claude/skills/ + assert result[2] == DEFAULT_SKILLS_PATHS[1] # .claude/skills/ + + +def test_get_effective_paths_with_config_file_path(): + """Test get_effective_paths resolves relative paths against config file.""" + # Create a mock config file path + config_file = UPath("/home/user/project/config.yml") + + config = SkillsConfig( + paths=[UPath("../shared-skills"), UPath("./local-skills")], + include_default=False, + ) + + result = config.get_effective_paths(config_file_path=config_file) + + assert len(result) == 2 + # ../shared-skills from /home/user/project/config.yml -> /home/user/shared-skills + # Use endswith because resolve() may resolve to different absolute path on different OS + assert str(result[0]).endswith("/home/user/shared-skills") + # ./local-skills from /home/user/project/config.yml -> /home/user/project/local-skills + assert str(result[1]).endswith("/home/user/project/local-skills") + + +def test_get_effective_paths_absolute_paths_unaffected(): + """Test that absolute paths are not modified by config_file_path.""" + config_file = UPath("/some/other/path/config.yml") + + config = SkillsConfig( + paths=[UPath("/custom/absolute/skills")], + include_default=False, + ) + + result = config.get_effective_paths(config_file_path=config_file) + + assert len(result) == 1 + assert result[0] == UPath("/custom/absolute/skills") + + +def test_get_effective_paths_no_config_file_uses_cwd(): + """Test that relative paths resolve to CWD when no config_file_path.""" + # We can't easily test exact path without knowing test CWD, + # but we can verify the path is absolute + config = SkillsConfig( + paths=[UPath("./test-skills")], + include_default=False, + ) + + result = config.get_effective_paths(config_file_path=None) + + assert len(result) == 1 + assert result[0].is_absolute() + assert str(result[0]).endswith("test-skills") + + +def test_get_effective_paths_remote_paths(): + """Test that remote paths are preserved as-is.""" + config = SkillsConfig( + paths=[UPath("s3://bucket/skills"), UPath("github://org/repo/skills")], + include_default=False, + ) + + result = config.get_effective_paths() + + assert len(result) == 2 + assert result[0] == UPath("s3://bucket/skills") + assert result[1] == UPath("github://org/repo/skills") + + +def test_get_effective_paths_first_path_wins(): + """Test 'first path wins' priority - custom paths before defaults.""" + config = SkillsConfig( + paths=[UPath("./my-skills")], + include_default=True, + ) + + result = config.get_effective_paths() + + # Custom paths come first + assert str(result[0]).endswith("my-skills") + # Default paths come after + assert result[1] == DEFAULT_SKILLS_PATHS[0] + assert result[2] == DEFAULT_SKILLS_PATHS[1] + + +def test_pydantic_validation(): + """Test that SkillsConfig validates properly with Pydantic.""" + # Valid config + config = SkillsConfig(paths=[UPath("/path")], include_default=True) + assert config.paths == [UPath("/path")] + assert config.include_default is True + + # Invalid types should raise ValidationError + from pydantic import ValidationError + + with pytest.raises(ValidationError): + SkillsConfig(paths=["not", "a", "list"], include_default="not a bool") + + +def test_empty_config_no_defaults(): + """Test empty config with defaults disabled returns empty list.""" + config = SkillsConfig(paths=[], include_default=False) + + result = config.get_effective_paths() + + assert result == [] + + +def test_config_yaml_roundtrip(): + """Test that SkillsConfig can be serialized/deserialized.""" + config = SkillsConfig( + paths=[UPath("./skills"), UPath("/absolute/skills")], + include_default=True, + ) + + # Serialize to dict + config_dict = config.model_dump() + + # Deserialize back + config2 = SkillsConfig(**config_dict) + + assert config2.paths == config.paths + assert config2.include_default == config.include_default + + # Verify effective paths are the same + paths1 = config.get_effective_paths() + paths2 = config2.get_effective_paths() + + assert len(paths1) == len(paths2) + for p1, p2 in zip(paths1, paths2, strict=True): + assert p1 == p2 diff --git a/tests/test_skills/test_manager_config.py b/tests/test_skills/test_manager_config.py new file mode 100644 index 000000000..4937762e6 --- /dev/null +++ b/tests/test_skills/test_manager_config.py @@ -0,0 +1,122 @@ +"""Tests for SkillsManager configuration-based discovery.""" + +from __future__ import annotations + +import logging +from pathlib import Path +import tempfile +from textwrap import dedent + +import pytest +from upathtools import UPath + +from agentpool.skills.manager import SkillsManager +from agentpool_config.skills import SkillsConfig + + +@pytest.fixture +def skill_dirs(): + """Create two temporary directories with conflicting test skills.""" + with tempfile.TemporaryDirectory() as temp_dir: + base = Path(temp_dir) + dir_a = base / "dir_a" + dir_b = base / "dir_b" + dir_a.mkdir() + dir_b.mkdir() + + # Skill in dir_a + skill_a = dir_a / "my_skill" + skill_a.mkdir() + (skill_a / "SKILL.md").write_text( + dedent(""" + --- + name: my_skill + description: Description from A + --- + Instructions A + """).strip() + ) + + # Same skill name in dir_b + skill_b = dir_b / "my_skill" + skill_b.mkdir() + (skill_b / "SKILL.md").write_text( + dedent(""" + --- + name: my_skill + description: Description from B + --- + Instructions B + """).strip() + ) + + yield dir_a, dir_b + + +@pytest.mark.asyncio +async def test_discover_skills_priority(skill_dirs: tuple[Path, Path]): + """Test that the first path in the config takes precedence (first path wins).""" + dir_a, dir_b = skill_dirs + # config.paths = [dir_a, dir_b] -> A should win because it's processed LAST in reversed list + config = SkillsConfig(paths=[UPath(dir_a), UPath(dir_b)], include_default=False) + + manager = SkillsManager() + await manager.discover_skills(config=config) + + skill = manager.get_skill("my_skill") + assert skill.description == "Description from A" + + # Now swap priority: [dir_b, dir_a] -> B should win + config_swapped = SkillsConfig(paths=[UPath(dir_b), UPath(dir_a)], include_default=False) + manager_swapped = SkillsManager() + await manager_swapped.discover_skills(config=config_swapped) + + skill_swapped = manager_swapped.get_skill("my_skill") + assert skill_swapped.description == "Description from B" + + +@pytest.mark.asyncio +async def test_discover_skills_no_config(skill_dirs: tuple[Path, Path]): + """Test discovery without a config object.""" + dir_a, _ = skill_dirs + manager = SkillsManager(skills_dirs=[dir_a]) + await manager.discover_skills() + + skill = manager.get_skill("my_skill") + assert skill.description == "Description from A" + + +@pytest.mark.asyncio +async def test_discover_skills_logging( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +): + """Test that missing custom paths log WARNING and missing default paths log DEBUG.""" + from agentpool_config import skills + + mock_default = [UPath("/non/existent/default/path")] + monkeypatch.setattr(skills, "DEFAULT_SKILLS_PATHS", mock_default) + + config = SkillsConfig(paths=[UPath("/non/existent/custom/path")], include_default=True) + + manager = SkillsManager() + with caplog.at_level(logging.DEBUG): + await manager.discover_skills(config=config) + + # Check for WARNING for custom path + assert any( + "Custom skills directory not found" in record.message and record.levelno == logging.WARNING + for record in caplog.records + ) + + # Check for DEBUG for default paths + assert any( + "Default skills directory not found" in record.message and record.levelno == logging.DEBUG + for record in caplog.records + ) + + # Check for DEBUG for default paths (they likely don't exist in the test environment) + print(f"Logged messages: {[r.message for r in caplog.records]}") + assert any( + "Default skills directory not found" in record.message and record.levelno == logging.DEBUG + for record in caplog.records + ) diff --git a/tests/test_skills/test_skills_integration.py b/tests/test_skills/test_skills_integration.py new file mode 100644 index 000000000..d3679f750 --- /dev/null +++ b/tests/test_skills/test_skills_integration.py @@ -0,0 +1,222 @@ +"""Integration tests for configurable skill loading paths in AgentPool.""" + +from __future__ import annotations + +import os +from pathlib import Path +import tempfile +from textwrap import dedent +from typing import Any + +import pytest +from upathtools import UPath +import yaml + +from agentpool.delegation.pool import AgentPool + + +def create_skill(path: Path, name: str, description: str, instructions: str): + """Create a skill in the specified directory.""" + skill_dir = path / name + skill_dir.mkdir(parents=True, exist_ok=True) + (skill_dir / "SKILL.md").write_text( + dedent(f""" + --- + name: {name} + description: {description} + --- + {instructions} + """).strip() + ) + + +@pytest.fixture +def temp_skills(): + """Create temporary skill directories.""" + with tempfile.TemporaryDirectory() as temp_dir: + base = Path(temp_dir) + dir_a = base / "dir_a" + dir_b = base / "dir_b" + dir_a.mkdir() + dir_b.mkdir() + + create_skill(dir_a, "skill_a", "Description A", "Instructions A") + create_skill(dir_b, "skill_b", "Description B", "Instructions B") + create_skill(dir_b, "conflict_skill", "Conflict from B", "Instructions Conflict B") + create_skill(dir_a, "conflict_skill", "Conflict from A", "Instructions Conflict A") + + yield dir_a, dir_b + + +@pytest.mark.asyncio +async def test_skills_backward_compatibility(): + """Init pool with a manifest having NO skills section. + + Assert that default paths are searched. + """ + # Create a manifest without skills section + manifest_dict: dict[str, Any] = { + "agents": { + "test_agent": { + "type": "native", + "model": "openai:gpt-4o", + } + } + } + + with tempfile.TemporaryDirectory() as temp_dir: + config_path = Path(temp_dir) / "config.yml" + config_path.write_text(yaml.dump(manifest_dict)) + + async with AgentPool(config_path) as pool: + # We can't easily assert default paths existence since they depend on the environment, + # but we can verify that the SkillsManager is initialized with default config. + assert pool.skills._config is not None # type: ignore + assert pool.skills._config.include_default is True # type: ignore + assert pool.skills._config.paths == [] # type: ignore + + +@pytest.mark.asyncio +async def test_skills_custom_path(temp_skills: tuple[Path, Path]): + """Init pool with a manifest having a custom skills.paths. + + Assert that skills from that path are loaded. + """ + dir_a, _ = temp_skills + + manifest_dict: dict[str, Any] = { + "skills": {"paths": [str(dir_a)], "include_default": False}, + "agents": { + "test_agent": { + "type": "native", + "model": "openai:gpt-4o", + } + }, + } + + with tempfile.TemporaryDirectory() as temp_dir: + config_path = Path(temp_dir) / "config.yml" + config_path.write_text(yaml.dump(manifest_dict)) + + async with AgentPool(config_path) as pool: + skills = pool.skills.list_skills() + skill_names = [s.name for s in skills] + assert "skill_a" in skill_names + assert "skill_b" not in skill_names + + skill = pool.skills.get_skill("skill_a") + assert skill.description == "Description A" + + +@pytest.mark.asyncio +async def test_skills_disable_defaults(monkeypatch: pytest.MonkeyPatch): + """Init pool with skills.include_default: false. + + Assert that default paths are NOT searched. + """ + from agentpool_config import skills + + # Mock default paths to something we can control + with tempfile.TemporaryDirectory() as temp_dir: + default_dir = Path(temp_dir) / "default_skills" + default_dir.mkdir() + create_skill(default_dir, "default_skill", "Default", "Instructions") + + monkeypatch.setattr(skills, "DEFAULT_SKILLS_PATHS", [UPath(default_dir)]) + + manifest_dict: dict[str, Any] = { + "skills": {"include_default": False}, + "agents": { + "test_agent": { + "type": "native", + "model": "openai:gpt-4o", + } + }, + } + + with tempfile.TemporaryDirectory() as temp_dir_2: + config_path = Path(temp_dir_2) / "config.yml" + config_path.write_text(yaml.dump(manifest_dict)) + + async with AgentPool(config_path) as pool: + skills_list = pool.skills.list_skills() + skill_names = [s.name for s in skills_list] + assert "default_skill" not in skill_names + + +@pytest.mark.asyncio +async def test_skills_conflict_resolution(temp_skills: tuple[Path, Path]): + """Init pool with two paths containing the same skill name. + + Assert that the version from the EARLIER path in the list is the one loaded. + """ + dir_a, dir_b = temp_skills + + # [dir_a, dir_b] -> dir_a should win + manifest_dict: dict[str, Any] = { + "skills": {"paths": [str(dir_a), str(dir_b)], "include_default": False}, + "agents": { + "test_agent": { + "type": "native", + "model": "openai:gpt-4o", + } + }, + } + + with tempfile.TemporaryDirectory() as temp_dir: + config_path = Path(temp_dir) / "config.yml" + config_path.write_text(yaml.dump(manifest_dict)) + + async with AgentPool(config_path) as pool: + skill = pool.skills.get_skill("conflict_skill") + assert skill.description == "Conflict from A" + + # [dir_b, dir_a] -> dir_b should win + manifest_dict["skills"]["paths"] = [str(dir_b), str(dir_a)] + + with tempfile.TemporaryDirectory() as temp_dir: + config_path = Path(temp_dir) / "config.yml" + config_path.write_text(yaml.dump(manifest_dict)) + + async with AgentPool(config_path) as pool: + skill = pool.skills.get_skill("conflict_skill") + assert skill.description == "Conflict from B" + + +@pytest.mark.asyncio +async def test_skills_relative_paths(): + """Init pool from a YAML file that specifies relative skill paths. + + Assert that they are resolved correctly relative to the YAML file. + """ + with tempfile.TemporaryDirectory() as temp_dir: + config_dir = Path(temp_dir) + # Create a relative path from config_dir to dir_a + # In this test, we can just move dir_a inside config_dir/skills + skills_dir = config_dir / "my_skills" + skills_dir.mkdir() + create_skill(skills_dir, "rel_skill", "Relative Description", "Instructions") + + manifest_dict: dict[str, Any] = { + "skills": {"paths": ["./my_skills"], "include_default": False}, + "agents": { + "test_agent": { + "type": "native", + "model": "openai:gpt-4o", + } + }, + } + + config_path = config_dir / "config.yml" + config_path.write_text(yaml.dump(manifest_dict)) + + # Run from a different CWD to ensure relative path is resolved against config file + old_cwd = Path.cwd() + os.chdir(tempfile.gettempdir()) + try: + async with AgentPool(config_path) as pool: + skill = pool.skills.get_skill("rel_skill") + assert skill is not None + assert skill.description == "Relative Description" + finally: + os.chdir(old_cwd)