diff --git a/lib/crewai/src/crewai/experimental/skills/__init__.py b/lib/crewai/src/crewai/experimental/skills/__init__.py index 760813851..3974f8b9a 100644 --- a/lib/crewai/src/crewai/experimental/skills/__init__.py +++ b/lib/crewai/src/crewai/experimental/skills/__init__.py @@ -12,7 +12,6 @@ import sys from crewai.skills import cache, events, registry from crewai.skills.cache import SkillCacheManager from crewai.skills.registry import ( - SkillNotCachedError, is_registry_ref, parse_registry_ref, resolve_registry_ref, @@ -27,7 +26,6 @@ sys.modules[__name__ + ".registry"] = registry __all__ = [ "SkillCacheManager", - "SkillNotCachedError", "cache", "events", "is_registry_ref", diff --git a/lib/crewai/src/crewai/skills/__init__.py b/lib/crewai/src/crewai/skills/__init__.py index daaac82ed..8d72f8af6 100644 --- a/lib/crewai/src/crewai/skills/__init__.py +++ b/lib/crewai/src/crewai/skills/__init__.py @@ -15,7 +15,6 @@ from crewai.skills.loader import ( from crewai.skills.models import Skill, SkillFrontmatter from crewai.skills.parser import SkillParseError from crewai.skills.registry import ( - SkillNotCachedError, is_registry_ref, parse_registry_ref, resolve_registry_ref, @@ -26,7 +25,6 @@ __all__ = [ "Skill", "SkillCacheManager", "SkillFrontmatter", - "SkillNotCachedError", "SkillParseError", "activate_skill", "discover_skills", diff --git a/lib/crewai/src/crewai/skills/registry.py b/lib/crewai/src/crewai/skills/registry.py index d82613e89..d6b9fa98f 100644 --- a/lib/crewai/src/crewai/skills/registry.py +++ b/lib/crewai/src/crewai/skills/registry.py @@ -7,8 +7,8 @@ via the CrewAI+ API with a global cache at ~/.crewai/skills/. from __future__ import annotations import logging +import os from pathlib import Path -import sys from typing import Any from crewai.skills.cache import SkillCacheManager @@ -17,17 +17,6 @@ from crewai.skills.cache import SkillCacheManager _logger = logging.getLogger(__name__) -class SkillNotCachedError(Exception): - """Raised when a registry skill is not cached and the environment is non-interactive.""" - - def __init__(self, ref: str) -> None: - super().__init__( - f"Skill {ref!r} is not cached locally. " - f"Run `crewai skill install {ref}` to install it first." - ) - self.ref = ref - - def is_registry_ref(value: Any) -> bool: """Return True if *value* looks like a registry reference (@org/name).""" return isinstance(value, str) and value.startswith("@") @@ -68,17 +57,6 @@ def parse_registry_ref(ref: str) -> tuple[str, str]: return org, name -def _is_noninteractive() -> bool: - """Return True in CI or explicitly non-interactive environments.""" - import os - - return ( - os.environ.get("CI") == "1" - or os.environ.get("CREWAI_NONINTERACTIVE") == "1" - or not sys.stdin.isatty() - ) - - def resolve_registry_ref( ref: str, source: Any = None, @@ -88,7 +66,7 @@ def resolve_registry_ref( Resolution order: 1. ./skills/{name}/ in the current working directory (project-local) 2. ~/.crewai/skills/{org}/{name}/ (global cache) - 3. Download from registry (interactive only; raises SkillNotCachedError in CI) + 3. Download from registry Args: ref: A registry reference, e.g. '@acme/my-skill'. @@ -96,9 +74,6 @@ def resolve_registry_ref( Returns: A Skill loaded at INSTRUCTIONS disclosure level. - - Raises: - SkillNotCachedError: When not cached and running in non-interactive mode. """ from crewai.skills.loader import activate_skill from crewai.skills.parser import load_skill_metadata @@ -124,9 +99,6 @@ def resolve_registry_ref( "Failed to load cached skill at %s", cached_path, exc_info=True ) - if _is_noninteractive(): - raise SkillNotCachedError(ref) - return download_skill(org, name, source=source) @@ -172,7 +144,15 @@ def download_skill( try: from crewai_core.plus_api import PlusAPI - api = PlusAPI() + from crewai.auth.token import get_auth_token + from crewai.context import get_platform_integration_token + + api = PlusAPI( + api_key=os.getenv("CREWAI_USER_PAT") + or get_platform_integration_token() + or get_auth_token(), + organization_id=os.getenv("CREWAI_ORGANIZATION_UUID"), + ) response = api.get_skill(org, name) response.raise_for_status() data = response.json() diff --git a/lib/crewai/tests/skills/test_experimental_shim.py b/lib/crewai/tests/skills/test_experimental_shim.py index db92dfa2a..7ed57c4e0 100644 --- a/lib/crewai/tests/skills/test_experimental_shim.py +++ b/lib/crewai/tests/skills/test_experimental_shim.py @@ -5,14 +5,12 @@ def test_experimental_namespace_reexports_public_names(): from crewai.experimental import skills as experimental_skills from crewai.skills.cache import SkillCacheManager from crewai.skills.registry import ( - SkillNotCachedError, is_registry_ref, parse_registry_ref, resolve_registry_ref, ) assert experimental_skills.SkillCacheManager is SkillCacheManager - assert experimental_skills.SkillNotCachedError is SkillNotCachedError assert experimental_skills.is_registry_ref is is_registry_ref assert experimental_skills.parse_registry_ref is parse_registry_ref assert experimental_skills.resolve_registry_ref is resolve_registry_ref diff --git a/lib/crewai/tests/skills/test_registry.py b/lib/crewai/tests/skills/test_registry.py index 5f588ccf7..25afe50c1 100644 --- a/lib/crewai/tests/skills/test_registry.py +++ b/lib/crewai/tests/skills/test_registry.py @@ -1,18 +1,40 @@ -"""Tests for SkillRegistry.""" - from __future__ import annotations +import base64 +from io import BytesIO from pathlib import Path from unittest.mock import MagicMock, patch +from zipfile import ZipFile +from crewai.context import platform_context +from crewai.skills.cache import SkillCacheManager from crewai.skills.registry import ( - SkillNotCachedError, + download_skill, is_registry_ref, parse_registry_ref, ) import pytest +def _skill_archive(name: str) -> bytes: + archive = BytesIO() + with ZipFile(archive, "w") as zip_file: + zip_file.writestr( + "SKILL.md", + f"---\nname: {name}\ndescription: Test skill.\n---\n\nInstructions.", + ) + return archive.getvalue() + + +def _mock_skill_response(name: str) -> MagicMock: + response = MagicMock() + response.json.return_value = { + "latest_version": "1.0.0", + "file": base64.b64encode(_skill_archive(name)).decode(), + } + return response + + class TestIsRegistryRef: def test_at_prefixed(self) -> None: assert is_registry_ref("@acme/my-skill") is True @@ -54,10 +76,7 @@ class TestParseRegistryRef: class TestResolveRegistryRef: - """Test resolution order and CI mode behaviour.""" - def _make_skill_dir(self, base: Path, name: str) -> Path: - """Write a minimal SKILL.md into base/name/.""" skill_dir = base / name skill_dir.mkdir(parents=True) (skill_dir / "SKILL.md").write_text( @@ -65,42 +84,58 @@ class TestResolveRegistryRef: ) return skill_dir - def test_resolves_project_local(self, tmp_path: Path) -> None: - """Local ./skills/{name}/ takes priority over cache.""" + def test_prefers_project_local_skill_over_cached_skill( + self, tmp_path: Path + ) -> None: skills_dir = tmp_path / "skills" skills_dir.mkdir() self._make_skill_dir(skills_dir, "my-skill") + cache_dir = tmp_path / "cache" / "acme" / "my-skill" + cache_dir.mkdir(parents=True) + (cache_dir / "SKILL.md").write_text( + "---\nname: cached-skill\ndescription: Cached.\n---\n\nCached instructions." + ) mock_cache = MagicMock() - mock_cache.get_cached_path.return_value = None + mock_cache.get_cached_path.return_value = cache_dir with ( - patch("crewai.skills.registry._is_noninteractive", return_value=False), patch.object(Path, "cwd", return_value=tmp_path), patch("crewai.skills.registry.SkillCacheManager", return_value=mock_cache), ): from crewai.skills.registry import resolve_registry_ref + skill = resolve_registry_ref("@acme/my-skill") assert skill.name == "my-skill" - def test_raises_in_ci_when_not_cached(self, tmp_path: Path) -> None: - """In CI mode, raise SkillNotCachedError if no local or cached copy.""" - mock_cache = MagicMock() - mock_cache.get_cached_path.return_value = None + def test_downloads_and_caches_uncached_skill_in_noninteractive_environment( + self, tmp_path: Path + ) -> None: + cache = SkillCacheManager(cache_root=tmp_path / "cache") + api = MagicMock() + api.get_skill.return_value = _mock_skill_response("ghost-skill") with ( - patch("crewai.skills.registry._is_noninteractive", return_value=True), + patch.dict("os.environ", {"CI": "1", "CREWAI_NONINTERACTIVE": "1"}), patch.object(Path, "cwd", return_value=tmp_path), - patch("crewai.skills.registry.SkillCacheManager", return_value=mock_cache), + patch("crewai.auth.token.get_auth_token", return_value="saved-login"), + patch("crewai.skills.registry.SkillCacheManager", return_value=cache), + patch("crewai_core.plus_api.PlusAPI", return_value=api), ): from crewai.skills.registry import resolve_registry_ref - with pytest.raises(SkillNotCachedError) as exc_info: - resolve_registry_ref("@acme/ghost-skill") - assert "@acme/ghost-skill" in str(exc_info.value) - def test_resolves_from_cache(self, tmp_path: Path) -> None: - """Falls back to global cache when no project-local skill exists.""" + skill = resolve_registry_ref("@acme/ghost-skill") + + assert skill.name == "ghost-skill" + api.get_skill.assert_called_once_with("acme", "ghost-skill") + assert cache.get_cached_path("acme", "ghost-skill") == ( + tmp_path / "cache" / "acme" / "ghost-skill" + ) + + def test_resolves_cached_skill_when_project_local_skill_is_missing( + self, tmp_path: Path + ) -> None: cache_dir = tmp_path / "acme" / "cached-skill" cache_dir.mkdir(parents=True) (cache_dir / "SKILL.md").write_text( @@ -110,18 +145,89 @@ class TestResolveRegistryRef: mock_cache = MagicMock() mock_cache.get_cached_path.return_value = cache_dir - # tmp_path has no ./skills/ directory with ( - patch("crewai.skills.registry._is_noninteractive", return_value=False), patch.object(Path, "cwd", return_value=tmp_path), patch("crewai.skills.registry.SkillCacheManager", return_value=mock_cache), ): from crewai.skills.registry import resolve_registry_ref + skill = resolve_registry_ref("@acme/cached-skill") assert skill.name == "cached-skill" - def test_skill_not_cached_error_contains_ref(self) -> None: - err = SkillNotCachedError("@foo/bar") - assert "@foo/bar" in str(err) - assert err.ref == "@foo/bar" + +class TestDownloadSkillAuthentication: + def test_user_pat_takes_precedence_over_platform_token( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + cache = SkillCacheManager(cache_root=tmp_path / "cache") + api = MagicMock() + api.get_skill.return_value = _mock_skill_response("my-skill") + monkeypatch.setenv("CREWAI_USER_PAT", "user-pat") + monkeypatch.delenv("CREWAI_ORGANIZATION_UUID", raising=False) + + with ( + platform_context("platform-token"), + patch("crewai.skills.registry.SkillCacheManager", return_value=cache), + patch("crewai_core.plus_api.PlusAPI", return_value=api) as plus_api, + ): + download_skill("acme", "my-skill") + + plus_api.assert_called_once_with(api_key="user-pat", organization_id=None) + + def test_uses_platform_token_when_user_pat_is_missing( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + cache = SkillCacheManager(cache_root=tmp_path / "cache") + api = MagicMock() + api.get_skill.return_value = _mock_skill_response("my-skill") + monkeypatch.delenv("CREWAI_USER_PAT", raising=False) + monkeypatch.delenv("CREWAI_ORGANIZATION_UUID", raising=False) + + with ( + platform_context("platform-token"), + patch("crewai.skills.registry.SkillCacheManager", return_value=cache), + patch("crewai_core.plus_api.PlusAPI", return_value=api) as plus_api, + ): + download_skill("acme", "my-skill") + + plus_api.assert_called_once_with(api_key="platform-token", organization_id=None) + + def test_uses_user_pat_and_organization_from_environment( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + cache = SkillCacheManager(cache_root=tmp_path / "cache") + api = MagicMock() + api.get_skill.return_value = _mock_skill_response("my-skill") + monkeypatch.delenv("CREWAI_PLATFORM_INTEGRATION_TOKEN", raising=False) + monkeypatch.setenv("CREWAI_USER_PAT", "user-pat") + monkeypatch.setenv("CREWAI_ORGANIZATION_UUID", "organization-uuid") + + with ( + patch("crewai.skills.registry.SkillCacheManager", return_value=cache), + patch("crewai_core.plus_api.PlusAPI", return_value=api) as plus_api, + ): + download_skill("acme", "my-skill") + + plus_api.assert_called_once_with( + api_key="user-pat", organization_id="organization-uuid" + ) + + def test_uses_saved_cli_login_when_runtime_tokens_are_missing( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + cache = SkillCacheManager(cache_root=tmp_path / "cache") + api = MagicMock() + api.get_skill.return_value = _mock_skill_response("my-skill") + monkeypatch.delenv("CREWAI_USER_PAT", raising=False) + monkeypatch.delenv("CREWAI_PLATFORM_INTEGRATION_TOKEN", raising=False) + monkeypatch.delenv("CREWAI_ORGANIZATION_UUID", raising=False) + + with ( + patch("crewai.auth.token.get_auth_token", return_value="saved-login"), + patch("crewai.skills.registry.SkillCacheManager", return_value=cache), + patch("crewai_core.plus_api.PlusAPI", return_value=api) as plus_api, + ): + download_skill("acme", "my-skill") + + plus_api.assert_called_once_with(api_key="saved-login", organization_id=None)