diff --git a/src/anthropic/lib/tools/_skills.py b/src/anthropic/lib/tools/_skills.py index 0a1e9e537..db4112249 100644 --- a/src/anthropic/lib/tools/_skills.py +++ b/src/anthropic/lib/tools/_skills.py @@ -17,14 +17,14 @@ from typing import TYPE_CHECKING from pathlib import Path, PurePosixPath from functools import partial -from collections.abc import Iterable +from collections.abc import Iterable, Iterator import anyio from anyio.to_thread import run_sync if TYPE_CHECKING: from ..._client import AsyncAnthropic - from ...types.beta import BetaManagedAgentsSession + from ...types.beta import Skill, BetaManagedAgentsSession __all__ = ["download_session_skills"] @@ -217,6 +217,33 @@ async def _resolve_skill_version(client: AsyncAnthropic, skill_id: str, version: return newest +def _session_skills(session: BetaManagedAgentsSession) -> Iterator[Skill]: + """Every skill the session needs on disk: the agent's, plus each roster member's. + + Subagent threads share the coordinator's container filesystem, so a roster + member's skills must already be extracted before its thread runs. Only the + coordinator's own skills were collected before, which left subagents running + without theirs and produced no error — just degraded output. + + Roster entries are a union: thread agents carry skills, advisors do not, so + the discriminator is checked rather than the attribute. Skills shared by + several members are yielded once, because each download rmtree's its + destination and a repeat would redo work already done. + """ + seen: set[tuple[str, str | None]] = set() + coordinator = session.agent.multiagent + roster = coordinator.agents if coordinator is not None else [] + for skill in ( + *session.agent.skills, + *(skill for agent in roster if agent.type == "agent" for skill in agent.skills), + ): + key = (skill.skill_id, skill.version) + if key in seen: + continue + seen.add(key) + yield skill + + async def download_session_skills( client: AsyncAnthropic, *, @@ -224,7 +251,11 @@ async def download_session_skills( session: BetaManagedAgentsSession | None = None, session_id: str | None = None, ) -> list[Path]: - """Download the session agent's skills into ``{workdir}/skills//``. + """Download the session's skills into ``{workdir}/skills//``. + + Covers the session agent's own skills and, for a multiagent session, those + of each roster member: subagent threads share this filesystem, so their + skills must be here before they run. Reads the resolved agent off ``session``, and for each skill fetches its files via ``client.beta.skills.versions.download`` and extracts the archive @@ -258,7 +289,7 @@ async def download_session_skills( # ``skills_root`` is created lazily by the extraction below — don't create it # up front so an agent with no skills leaves no stray directory behind. downloaded: list[Path] = [] - for skill in session.agent.skills: + for skill in _session_skills(session): try: version_id = await _resolve_skill_version(client, skill.skill_id, skill.version) version = await client.beta.skills.versions.retrieve(version_id, skill_id=skill.skill_id) diff --git a/tests/lib/tools/test_skills.py b/tests/lib/tools/test_skills.py index bcbc20080..f4c678c04 100644 --- a/tests/lib/tools/test_skills.py +++ b/tests/lib/tools/test_skills.py @@ -23,7 +23,8 @@ import pytest -from anthropic.lib.tools._skills import _strip_top, _archive_top_dir, _extract_skill_archive +from anthropic.types.beta import BetaManagedAgentsSession +from anthropic.lib.tools._skills import _strip_top, _session_skills, _archive_top_dir, _extract_skill_archive def _make_zip(path: Path, entries: dict[str, bytes]) -> None: @@ -370,3 +371,88 @@ def test_tar_path_screen_applies_to_skipped_members(tmp_path: Path) -> None: assert not os.path.lexists(dest.parent / "x") assert not os.path.lexists(tmp_path / "sub" / "x") assert not os.path.lexists(tmp_path / "x") + + +# --------------------------------------------------------------------------- +# Which skills a session needs on disk (#1870). +# +# Subagent threads share the coordinator's container filesystem, so a roster +# member's skills have to be extracted before its thread runs. Collecting only +# ``session.agent.skills`` left them missing and raised nothing. +# --------------------------------------------------------------------------- + + +def _skill(skill_id: str, version: str | None = None) -> dict[str, object]: + return {"skill_id": skill_id, "version": version, "type": "skill"} + + +def _thread_agent(*skill_ids: str) -> dict[str, object]: + return { + "id": "agt_worker", + "name": "worker", + "type": "agent", + "version": 1, + "mcp_servers": [], + "model": {"model": "claude-sonnet-4-5"}, + "tools": [], + "skills": [_skill(sid) for sid in skill_ids], + } + + +_ADVISOR: dict[str, object] = {"type": "advisor", "model": "claude-haiku-4-5"} + + +def _session( + coordinator_skills: list[dict[str, object]], + roster: list[dict[str, object]] | None, +) -> BetaManagedAgentsSession: + return BetaManagedAgentsSession.construct( + id="ses_1", + agent={ + "id": "agt_coordinator", + "name": "coordinator", + "type": "agent", + "version": 1, + "mcp_servers": [], + "model": {"model": "claude-sonnet-4-5"}, + "tools": [], + "skills": coordinator_skills, + "multiagent": None if roster is None else {"type": "coordinator", "agents": roster}, + }, + ) + + +def test_session_skills_without_a_roster() -> None: + """A single-agent session is unchanged: just the agent's own skills.""" + session = _session([_skill("sk_a")], None) + assert [s.skill_id for s in _session_skills(session)] == ["sk_a"] + + +def test_session_skills_includes_roster_members() -> None: + """Roster members' skills are downloaded too, or their threads run without them.""" + session = _session([_skill("sk_a")], [_thread_agent("sk_b"), _thread_agent("sk_c")]) + assert [s.skill_id for s in _session_skills(session)] == ["sk_a", "sk_b", "sk_c"] + + +def test_session_skills_skips_advisors() -> None: + """Roster entries are a union and advisors carry no ``skills`` attribute.""" + session = _session([_skill("sk_a")], [_ADVISOR, _thread_agent("sk_b")]) + assert [s.skill_id for s in _session_skills(session)] == ["sk_a", "sk_b"] + + +def test_session_skills_deduplicates_shared_skills() -> None: + """Each download rmtree's its destination, so a shared skill is yielded once.""" + session = _session([_skill("sk_a")], [_thread_agent("sk_a"), _thread_agent("sk_b")]) + assert [s.skill_id for s in _session_skills(session)] == ["sk_a", "sk_b"] + + +def test_session_skills_treats_versions_as_distinct() -> None: + """Same skill pinned to different versions is not collapsed by the dedupe.""" + session = _session([_skill("sk_a", "1")], [_thread_agent("sk_a")]) + assert [(s.skill_id, s.version) for s in _session_skills(session)] == [("sk_a", "1"), ("sk_a", None)] + + +def test_session_skills_with_an_empty_roster() -> None: + """A coordinator with no roster members behaves like a single-agent session.""" + session = _session([_skill("sk_a")], []) + assert [s.skill_id for s in _session_skills(session)] == ["sk_a"]