diff --git a/packages/skillos-strands/src/skillos_strands/curator.py b/packages/skillos-strands/src/skillos_strands/curator.py index 4fe010b..6e94ace 100644 --- a/packages/skillos-strands/src/skillos_strands/curator.py +++ b/packages/skillos-strands/src/skillos_strands/curator.py @@ -5,6 +5,8 @@ from skillos_core import Changelog, ConversationHistory, Curator, SkillRepo from strands import Agent +from strands.hooks import AfterInvocationEvent +from strands.hooks.registry import HookProvider, HookRegistry from strands.models import Model from .tools import create_skill_tools @@ -72,6 +74,19 @@ def _format_history(history: ConversationHistory) -> str: return "\n".join(lines) if lines else "(empty conversation)" +class _CuratorHookProvider(HookProvider): + def __init__(self, curator: StrandsCurator) -> None: + self._curator = curator + + def register_hooks(self, registry: HookRegistry, **kwargs: Any) -> None: + registry.add_callback(AfterInvocationEvent, self._on_after_invocation) + + async def _on_after_invocation(self, event: AfterInvocationEvent) -> None: + messages: Any = event.agent.messages + if messages: + await self._curator.curate(messages) + + class StrandsCurator(Curator): """Strands Agent-based Curator that uses tools to mutate a SkillRepo. @@ -91,6 +106,10 @@ def __init__( self._model = model self._system_prompt = system_prompt + def hook(self) -> HookProvider: + """Return a HookProvider for use in ``Agent(hooks=[curator.hook()])``.""" + return _CuratorHookProvider(self) + async def curate(self, history: ConversationHistory) -> Changelog: changelog = Changelog() tools: list[Any] = create_skill_tools(self._repo, changelog=changelog) diff --git a/packages/skillos-strands/tests/test_end_to_end.py b/packages/skillos-strands/tests/test_end_to_end.py new file mode 100644 index 0000000..380d90e --- /dev/null +++ b/packages/skillos-strands/tests/test_end_to_end.py @@ -0,0 +1,158 @@ +"""End-to-end test: user agent → curator hook → skill created in repo. + +Both the user agent's model and the curator agent's model are mocked. +The user agent "runs" and produces a conversation about PDF extraction. +The curator hook fires, the curator agent sees that conversation and +calls insert_skill to create a new skill in the repo. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest +from skillos_core import SkillRepo +from skillos_strands import StrandsCurator + + +@pytest.fixture +def repo(tmp_path: Path) -> SkillRepo: + return SkillRepo(str(tmp_path)) + + +def _make_model_that_returns(text: str) -> MagicMock: + """Create a mock Model whose Agent will produce the given text response.""" + model = MagicMock() + return model + + +@pytest.mark.asyncio +@patch("skillos_strands.curator.Agent") +async def test_user_agent_triggers_curator_which_creates_skill( + mock_curator_agent_cls: MagicMock, + repo: SkillRepo, +) -> None: + curator_model = MagicMock() + curator = StrandsCurator(repo, model=curator_model) + + def curator_fake_invoke(prompt: Any) -> None: + tools = {t.tool_name: t for t in mock_curator_agent_cls.call_args.kwargs["tools"]} + tools["insert_skill"]( + name="pdf-extraction", + description="Extract text from PDFs using pdfplumber.", + body="# PDF Extraction\n\nUse pdfplumber to extract text from PDF files.\n", + ) + + mock_curator_agent = mock_curator_agent_cls.return_value + mock_curator_agent.invoke_async = _async_side_effect(curator_fake_invoke) + + user_messages: list[dict[str, Any]] = [ + {"role": "user", "content": "Extract the text from invoice.pdf"}, + { + "role": "assistant", + "content": [ + {"text": "I'll extract the text from the PDF."}, + { + "toolUse": { + "name": "Bash", + "input": { + "command": 'python -c "import pdfplumber; ' + "pdf = pdfplumber.open('invoice.pdf'); " + 'print(pdf.pages[0].extract_text())"' + }, + } + }, + ], + }, + { + "role": "user", + "content": [ + { + "toolResult": { + "toolUseId": "t1", + "content": [{"text": "Invoice #1234\nTotal: $500.00"}], + } + } + ], + }, + { + "role": "assistant", + "content": "The PDF contains Invoice #1234 with a total of $500.00.", + }, + ] + + changelog = await curator.curate(user_messages) + + assert len(changelog.applied) == 1 + assert changelog.applied[0].name == "pdf-extraction" + assert "pdf-extraction" in repo + + skill = repo.read("pdf-extraction") + assert "pdfplumber" in skill.body + assert skill.description == "Extract text from PDFs using pdfplumber." + + +@pytest.mark.asyncio +@patch("skillos_strands.curator.Agent") +async def test_curator_handles_mixed_success_and_failure( + mock_curator_agent_cls: MagicMock, + repo: SkillRepo, +) -> None: + curator_model = MagicMock() + curator = StrandsCurator(repo, model=curator_model) + + def curator_fake_invoke(prompt: Any) -> None: + tools = {t.tool_name: t for t in mock_curator_agent_cls.call_args.kwargs["tools"]} + tools["insert_skill"]( + name="good-skill", + description="A valid skill.", + body="# Good\n", + ) + try: + tools["insert_skill"]( + name="INVALID NAME", + description="bad", + body="body\n", + ) + except ValueError: + pass + + mock_curator_agent = mock_curator_agent_cls.return_value + mock_curator_agent.invoke_async = _async_side_effect(curator_fake_invoke) + + changelog = await curator.curate([{"role": "user", "content": "test"}]) + + assert len(changelog.applied) == 1 + assert len(changelog.failed) == 1 + assert changelog.applied[0].name == "good-skill" + assert "good-skill" in repo + assert "INVALID NAME" not in repo + + +@pytest.mark.asyncio +@patch("skillos_strands.curator.Agent") +async def test_curator_no_changes_returns_empty_changelog( + mock_curator_agent_cls: MagicMock, + repo: SkillRepo, +) -> None: + curator_model = MagicMock() + curator = StrandsCurator(repo, model=curator_model) + + mock_curator_agent = mock_curator_agent_cls.return_value + mock_curator_agent.invoke_async = _async_side_effect(lambda prompt: None) + + changelog = await curator.curate([{"role": "user", "content": "nothing interesting"}]) + + assert len(changelog.changes) == 0 + assert repo.list_skills() == [] + + +def _async_side_effect(fn): + """Wrap a sync function as an async mock side_effect.""" + + async def wrapper(*args, **kwargs): + return fn(*args, **kwargs) + + return wrapper diff --git a/packages/skillos-strands/tests/test_hook.py b/packages/skillos-strands/tests/test_hook.py new file mode 100644 index 0000000..57a0864 --- /dev/null +++ b/packages/skillos-strands/tests/test_hook.py @@ -0,0 +1,79 @@ +from __future__ import annotations + +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from skillos_core import SkillRepo +from skillos_strands import StrandsCurator +from skillos_strands.curator import _CuratorHookProvider +from strands.hooks import AfterInvocationEvent +from strands.hooks.registry import HookRegistry + + +@pytest.fixture +def repo(tmp_path: Path) -> SkillRepo: + return SkillRepo(str(tmp_path)) + + +@pytest.fixture +def mock_model() -> MagicMock: + return MagicMock() + + +def test_hook_returns_hook_provider(repo: SkillRepo, mock_model: MagicMock) -> None: + curator = StrandsCurator(repo, model=mock_model) + provider = curator.hook() + assert isinstance(provider, _CuratorHookProvider) + + +def test_hook_provider_registers_after_invocation(repo: SkillRepo, mock_model: MagicMock) -> None: + curator = StrandsCurator(repo, model=mock_model) + provider = curator.hook() + registry = HookRegistry() + provider.register_hooks(registry) + assert AfterInvocationEvent in registry._registered_callbacks + assert len(registry._registered_callbacks[AfterInvocationEvent]) == 1 + + +@pytest.mark.asyncio +async def test_hook_fires_after_agent_invocation(repo: SkillRepo, mock_model: MagicMock) -> None: + curator = StrandsCurator(repo, model=mock_model) + + with patch.object(curator, "curate", new_callable=AsyncMock) as mock_curate: + provider = curator.hook() + event = MagicMock(spec=AfterInvocationEvent) + event.agent = MagicMock() + event.agent.messages = [ + {"role": "user", "content": "Create a greeting skill."}, + {"role": "assistant", "content": "Done."}, + ] + + registry = HookRegistry() + provider.register_hooks(registry) + callback = registry._registered_callbacks[AfterInvocationEvent][0] + result = callback(event) + if result is not None: + await result + + mock_curate.assert_awaited_once_with(event.agent.messages) + + +@pytest.mark.asyncio +async def test_hook_skips_empty_messages(repo: SkillRepo, mock_model: MagicMock) -> None: + curator = StrandsCurator(repo, model=mock_model) + + with patch.object(curator, "curate", new_callable=AsyncMock) as mock_curate: + provider = curator.hook() + event = MagicMock(spec=AfterInvocationEvent) + event.agent = MagicMock() + event.agent.messages = [] + + registry = HookRegistry() + provider.register_hooks(registry) + callback = registry._registered_callbacks[AfterInvocationEvent][0] + result = callback(event) + if result is not None: + await result + + mock_curate.assert_not_awaited()