Skip to content

fix(skills): resolve registry skills through the runtime's CrewAI+ client#6658

Merged
joaomdmoura merged 4 commits into
mainfrom
fix/skill-registry-deployment-auth
Jul 26, 2026
Merged

fix(skills): resolve registry skills through the runtime's CrewAI+ client#6658
joaomdmoura merged 4 commits into
mainfrom
fix/skill-registry-deployment-auth

Conversation

@joaomdmoura

@joaomdmoura joaomdmoura commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Loading an Agent Repository agent that carries skills fails with a 401 Unauthorized from the skills registry when the agent runs on a managed runtime.

load_agent_from_repository resolves its API client through a hook, so a managed runtime can install a client authenticated for the environment it runs in. download_skill didn't use that hook — it built its own PlusAPI from CREWAI_USER_PAT, the platform integration token, or the saved CLI login. Where none of those exist, the agent fetch succeeds and the skill download beside it fails.

Skills resolve their client the same way agents do

  • resolve_plus_client() in agent_utils, beside the hook it reads, is now used by both Agent Repository and Skills Repository lookups — so a new registry lookup can't reach for a credential that isn't there
  • A client that can't fetch skills falls back to environment credentials and warns, leaving older runtimes exactly as they behave today
  • resolve_plus_response() shares the sync/async bridging both lookups need, since PlusAPI is synchronous and managed clients are not

Version pinning

The same bug was hiding silent drift: Agent Repository agents record a version per skill, and that version was parsed off the response and dropped, so agents resolved whatever version happened to be newest.

  • Registry refs accept @org/name@version — and @org/name@v1.2.0, since people write it both ways — parsed by parse_skill_ref() into a SkillRef(org, name, version)
  • parse_registry_ref() keeps its (org, name) return shape and simply drops the pin, so existing callers are unaffected
  • Repository pins now travel with the refs, so publishing a new version of a skill no longer changes every agent that uses it
  • A pinned ref only accepts a project-local copy that declares that version in its metadata.version frontmatter, and the cache reports a miss when the version it recorded differs — a pin re-resolves rather than loading a different version. Unpinned refs keep hitting the cache as before
  • An unknown pin fails instead of quietly serving the newest version

Notes

  • docs/edge/en/concepts/skills.mdx documents pinning
  • 763 lines added, of which 403 are tests and 26 are docs
  • Tests: lib/crewai/tests/skills/ (135) and the agent_utils / from_repository suites — green. Remaining failures in tests/utilities/ and tests/agents/test_agent.py reproduce on a clean main (missing optional providers / API keys). ruff clean; the only mypy findings in scope are pre-existing IBM stub errors
  • Registry-side support for serving pinned versions ships separately

🤖 Generated with Claude Code


Note

Medium Risk
Changes authentication and resolution for registry skills on hosted runtimes and alters cache/local matching when pins are used; incorrect registry behavior could surface as download failures rather than silent wrong versions.

Overview
Fixes 401 skill downloads on managed runtimes by routing registry fetches through the same resolve_plus_client / resolve_plus_response path as Agent Repository loads, instead of always building a separate PlusAPI from user PAT / CLI login. Hosted clients that lack get_skill or version support fall back to env credentials with a warning.

Adds explicit version pinning for @org/skill refs (@org/name@1.2.0, optional leading v): parse_skill_ref / SkillRef, cache lookups that miss on version mismatch, local ./skills/ checks against metadata.version, and download logic that forwards the pin to the API and refuses to cache when the registry returns a different version. Agent Repository agents now merge skill_versions into skill refs so recorded pins are applied at load time.

Docs add a Pin a Version section in skills.mdx.

Reviewed by Cursor Bugbot for commit 44a830f. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI review requested due to automatic review settings July 26, 2026 11:55
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Skill references now support optional version pins across parsing, local/cache/registry resolution, downloads, and repository-agent loading. Client handling supports installed clients and asynchronous responses, with tests and documentation covering the new behavior.

Changes

Skill versioning and repository resolution

Layer / File(s) Summary
Version reference and cache contracts
lib/crewai/src/crewai/skills/registry.py, lib/crewai/src/crewai/skills/validation.py, lib/crewai/src/crewai/skills/cache.py, lib/crewai/src/crewai/skills/__init__.py, lib/crewai/src/crewai/experimental/skills/__init__.py, lib/crewai/tests/skills/test_cache.py
Versioned skill references, normalized version comparison, version-aware cache lookups, and public exports are implemented and tested.
Pinned local, cached, and registry resolution
lib/crewai/src/crewai/skills/registry.py, lib/crewai/tests/skills/test_registry.py, docs/edge/en/concepts/skills.mdx
Resolution checks project-local metadata and cached metadata before downloading the requested version, records resolved versions, and documents pinning rules.
Client selection and repository-agent skill pinning
lib/crewai/src/crewai/utilities/agent_utils.py, lib/crewai/tests/agents/test_agent.py, lib/crewai/tests/utilities/test_agent_utils.py
Repository agent loading selects an installed or default client, resolves synchronous or asynchronous responses, and appends recorded skill versions to unpinned references.

Sequence Diagram(s)

sequenceDiagram
  participant AgentRepository
  participant load_agent_from_repository
  participant PlusClient
  participant SkillResolver
  AgentRepository->>load_agent_from_repository: return agent and skill_versions
  load_agent_from_repository->>PlusClient: select client and resolve response
  PlusClient-->>load_agent_from_repository: return agent payload
  load_agent_from_repository->>SkillResolver: pin skill refs to recorded versions
  SkillResolver-->>load_agent_from_repository: return pinned skill refs
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.07% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the main change: routing registry skill resolution through the runtime CrewAI+ client.
Description check ✅ Passed The description is clearly related to the changeset and matches the client resolution and version-pinning work.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/skill-registry-deployment-auth

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread lib/crewai/tests/utilities/test_plus_client_factory.py Fixed
Comment thread lib/crewai/src/crewai/skills/registry.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes CrewAI Enterprise skill downloads failing with 401s by routing registry skill fetches through the same runtime-supplied CrewAI+ client resolution path used for Agent Repository agent fetches, instead of always building a credentialed PlusAPI from user/CLI environment state. It also adds opt-in skill version pinning (@org/name@version) and makes the global cache/version resolution aware of pins so pinned agents don’t silently drift to newly published skill versions.

Changes:

  • Introduces crewai.utilities.plus_client_factory to consistently resolve the CrewAI+ client (including legacy _create_plus_client_hook) and to normalize sync/async responses via resolve_response().
  • Adds version-pinned skill refs (SkillRef, parse_skill_ref) and enforces version-aware resolution across local skills, cache, and registry downloads.
  • Extends tests and docs to cover pinning behavior, cache pin semantics, and repository-provided skill version pins.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/crewai/tests/utilities/test_plus_client_factory.py Adds unit tests for plus-client factory selection and sync/async response normalization.
lib/crewai/tests/skills/test_registry.py Adds coverage for pinned refs, version-aware cache behavior, and runtime client resolution for skill downloads.
lib/crewai/tests/skills/test_cache.py Adds tests for cache lookups that must match a requested pinned version (including v prefix equivalence).
lib/crewai/tests/agents/test_agent.py Validates Agent Repository responses pin skills to recorded skill_versions when loading agents.
lib/crewai/src/crewai/utilities/plus_client_factory.py New shared utility for resolving the runtime’s CrewAI+ client and bridging sync/async call sites.
lib/crewai/src/crewai/utilities/agent_utils.py Updates agent repository loading to use the shared plus-client factory and folds skill_versions into pinned skill refs.
lib/crewai/src/crewai/skills/validation.py Adds helpers to normalize/compare versions (supports optional leading v).
lib/crewai/src/crewai/skills/registry.py Implements pinned ref parsing and version-aware local/cache/registry resolution; routes skill downloads through resolved runtime client.
lib/crewai/src/crewai/skills/cache.py Makes cache lookup version-aware for pinned refs by validating stored metadata version.
lib/crewai/src/crewai/skills/init.py Exposes SkillRef and parse_skill_ref from the public skills module.
lib/crewai/src/crewai/experimental/skills/init.py Exposes the same new registry-ref APIs in the experimental skills namespace.
docs/edge/en/concepts/skills.mdx Documents how to pin skill versions and how Agent Repository agents are auto-pinned.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/crewai/src/crewai/skills/registry.py
Comment thread lib/crewai/src/crewai/skills/registry.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
lib/crewai/src/crewai/utilities/plus_client_factory.py (1)

86-92: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoff

resolve_response blocks the caller's event loop thread while the worker loop runs.

Inside a running loop this stalls that loop for the whole round trip (registry download included), and any coroutine bound to the outer loop's resources (a shared httpx.AsyncClient, per-loop locks) will fail or hang when driven by a second loop. Fine as a pragmatic bridge for the sync entry points here, but consider exposing an async path (await-able resolve_response_async) for callers already in a loop, and reusing a single module-level executor instead of creating one per call.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai/src/crewai/utilities/plus_client_factory.py` around lines 86 - 92,
Update resolve_response to provide an awaitable resolve_response_async path for
callers already running in an event loop, keeping coroutine execution on the
caller’s loop so loop-bound resources remain valid. Retain the synchronous
bridge for sync entry points, but replace the per-call ThreadPoolExecutor in the
running-loop branch with a reusable module-level executor.
lib/crewai/src/crewai/utilities/agent_utils.py (1)

1135-1141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Default client credentials diverge from the skills registry's default.

lib/crewai/src/crewai/skills/registry.py (_plus_client) builds its default as PlusAPI(api_key=CREWAI_USER_PAT or platform_integration_token or get_auth_token(), organization_id=CREWAI_ORGANIZATION_UUID), while this one uses only get_auth_token() with no org id. Two lookups against the same service now authenticate differently outside a deployment. Consider hoisting a single shared default builder into plus_client_factory so behavior can't drift further.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai/src/crewai/utilities/agent_utils.py` around lines 1135 - 1141,
Update build_default_client in the default-client resolution flow to use the
same credential precedence and organization identifier as the registry’s
_plus_client: CREWAI_USER_PAT, then platform_integration_token, then
get_auth_token(), together with CREWAI_ORGANIZATION_UUID. Prefer reusing or
hoisting a shared default builder in plus_client_factory so both lookups
construct PlusAPI consistently.
lib/crewai/tests/skills/test_registry.py (1)

230-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a case where the downloaded archive declares no metadata.version.

Every pinned fixture embeds the version in SKILL.md, so the cached-hit path never exercises the mismatch between the cache metadata (registry version) and the frontmatter check in _load_matching_skill. A second resolve_registry_ref with _skill_archive(name) (no version) would surface the repeat-download behavior flagged in lib/crewai/src/crewai/skills/registry.py.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai/tests/skills/test_registry.py` around lines 230 - 248, Add a test
case alongside
test_caches_the_pinned_version_so_it_resolves_without_a_second_download that
configures the mocked registry response with an archive from
_skill_archive(name) lacking metadata.version, resolves the same pinned
reference twice, and asserts api.get_skill is called once. Ensure the fixture
still supplies the registry’s pinned version so the test exercises the cache
metadata versus _load_matching_skill frontmatter mismatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/crewai/src/crewai/skills/cache.py`:
- Around line 68-75: Update _records_version to validate that the parsed
metadata is a dictionary before calling meta.get; treat valid non-object JSON
such as lists or strings as unreadable, log it through the existing debug path,
and return False.

In `@lib/crewai/src/crewai/skills/registry.py`:
- Around line 140-147: Update the cached-entry branch in the skill resolution
flow to trust the version validated by SkillCacheManager.get_cached_path(...,
version=version). Load the cached skill without reapplying
_load_matching_skill’s SKILL.md frontmatter version check, while preserving the
existing frontmatter validation for project-local skills without cache metadata
and retaining the download fallback.

---

Nitpick comments:
In `@lib/crewai/src/crewai/utilities/agent_utils.py`:
- Around line 1135-1141: Update build_default_client in the default-client
resolution flow to use the same credential precedence and organization
identifier as the registry’s _plus_client: CREWAI_USER_PAT, then
platform_integration_token, then get_auth_token(), together with
CREWAI_ORGANIZATION_UUID. Prefer reusing or hoisting a shared default builder in
plus_client_factory so both lookups construct PlusAPI consistently.

In `@lib/crewai/src/crewai/utilities/plus_client_factory.py`:
- Around line 86-92: Update resolve_response to provide an awaitable
resolve_response_async path for callers already running in an event loop,
keeping coroutine execution on the caller’s loop so loop-bound resources remain
valid. Retain the synchronous bridge for sync entry points, but replace the
per-call ThreadPoolExecutor in the running-loop branch with a reusable
module-level executor.

In `@lib/crewai/tests/skills/test_registry.py`:
- Around line 230-248: Add a test case alongside
test_caches_the_pinned_version_so_it_resolves_without_a_second_download that
configures the mocked registry response with an archive from
_skill_archive(name) lacking metadata.version, resolves the same pinned
reference twice, and asserts api.get_skill is called once. Ensure the fixture
still supplies the registry’s pinned version so the test exercises the cache
metadata versus _load_matching_skill frontmatter mismatch.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b6825f7f-4e9f-44d2-87b8-5acadf078db7

📥 Commits

Reviewing files that changed from the base of the PR and between c52d0d9 and 4fc114a.

📒 Files selected for processing (12)
  • docs/edge/en/concepts/skills.mdx
  • lib/crewai/src/crewai/experimental/skills/__init__.py
  • lib/crewai/src/crewai/skills/__init__.py
  • lib/crewai/src/crewai/skills/cache.py
  • lib/crewai/src/crewai/skills/registry.py
  • lib/crewai/src/crewai/skills/validation.py
  • lib/crewai/src/crewai/utilities/agent_utils.py
  • lib/crewai/src/crewai/utilities/plus_client_factory.py
  • lib/crewai/tests/agents/test_agent.py
  • lib/crewai/tests/skills/test_cache.py
  • lib/crewai/tests/skills/test_registry.py
  • lib/crewai/tests/utilities/test_plus_client_factory.py

Comment thread lib/crewai/src/crewai/skills/cache.py
Comment thread lib/crewai/src/crewai/skills/registry.py
@mintlify

mintlify Bot commented Jul 26, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
crewai 🟢 Ready View Preview Jul 26, 2026, 12:10 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment thread lib/crewai/src/crewai/utilities/plus_client_factory.py Outdated
Comment thread lib/crewai/src/crewai/skills/registry.py Outdated
Copilot AI review requested due to automatic review settings July 26, 2026 12:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment thread lib/crewai/src/crewai/utilities/plus_client_factory.py Outdated
Comment thread lib/crewai/src/crewai/skills/cache.py
@mintlify

mintlify Bot commented Jul 26, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
crewai 🟡 Building Jul 26, 2026, 11:55 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

Copilot AI review requested due to automatic review settings July 26, 2026 14:57
@joaomdmoura
joaomdmoura force-pushed the fix/skill-registry-deployment-auth branch from 6cf6672 to f5fbaa1 Compare July 26, 2026 14:57
Comment thread lib/crewai/src/crewai/skills/registry.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comment thread lib/crewai/tests/utilities/test_agent_utils.py Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
lib/crewai/tests/utilities/test_agent_utils.py (1)

1292-1308: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover custom awaitables explicitly.

This only exercises native coroutines; add a small __await__ object assertion to protect the wrapper path that supports non-coroutine awaitables.

Proposed test
+    def test_awaits_a_custom_awaitable(self) -> None:
+        from crewai.utilities.agent_utils import resolve_plus_response
+
+        response = MagicMock()
+
+        class CustomAwaitable:
+            def __await__(self):
+                async def resolve() -> Any:
+                    return response
+
+                return resolve().__await__()
+
+        assert resolve_plus_response(CustomAwaitable()) is response

As per coding guidelines, “Write unit tests for new functionality that focus on behavior rather than implementation details.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai/tests/utilities/test_agent_utils.py` around lines 1292 - 1308, Add
an explicit custom awaitable case to test_awaits_an_async_response, using an
object that implements __await__ and returns the expected response. Exercise it
through resolve_plus_response, alongside the existing native coroutine coverage,
and assert the resolved object is response.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@lib/crewai/tests/utilities/test_agent_utils.py`:
- Around line 1292-1308: Add an explicit custom awaitable case to
test_awaits_an_async_response, using an object that implements __await__ and
returns the expected response. Exercise it through resolve_plus_response,
alongside the existing native coroutine coverage, and assert the resolved object
is response.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 361c086b-c094-4b38-a16d-4c3808a479f1

📥 Commits

Reviewing files that changed from the base of the PR and between 6cf6672 and f5fbaa1.

📒 Files selected for processing (11)
  • docs/edge/en/concepts/skills.mdx
  • lib/crewai/src/crewai/experimental/skills/__init__.py
  • lib/crewai/src/crewai/skills/__init__.py
  • lib/crewai/src/crewai/skills/cache.py
  • lib/crewai/src/crewai/skills/registry.py
  • lib/crewai/src/crewai/skills/validation.py
  • lib/crewai/src/crewai/utilities/agent_utils.py
  • lib/crewai/tests/agents/test_agent.py
  • lib/crewai/tests/skills/test_cache.py
  • lib/crewai/tests/skills/test_registry.py
  • lib/crewai/tests/utilities/test_agent_utils.py
🚧 Files skipped from review as they are similar to previous changes (6)
  • lib/crewai/src/crewai/experimental/skills/init.py
  • lib/crewai/src/crewai/skills/init.py
  • docs/edge/en/concepts/skills.mdx
  • lib/crewai/src/crewai/skills/cache.py
  • lib/crewai/src/crewai/skills/registry.py
  • lib/crewai/tests/skills/test_registry.py

@joaomdmoura
joaomdmoura force-pushed the fix/skill-registry-deployment-auth branch from f5fbaa1 to dd15978 Compare July 26, 2026 15:16
Copilot AI review requested due to automatic review settings July 26, 2026 15:16
Comment thread lib/crewai/tests/utilities/test_agent_utils.py Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/edge/en/concepts/skills.mdx`:
- Around line 239-240: Update the pinned-reference documentation around the
cached or project-local copy behavior to distinguish validation sources:
project-local copies must match the version in SKILL.md metadata.version
frontmatter, while cached copies must match the version recorded in
.crewai_meta.json and do not require metadata.version frontmatter. Split the
existing sentence and retain the re-download behavior for mismatched versions.

In `@lib/crewai/src/crewai/skills/cache.py`:
- Around line 70-74: Update the exception tuple in the metadata-loading try
block used by get_cached_path() to also catch UnicodeDecodeError, alongside
OSError and json.JSONDecodeError. Preserve the existing debug logging and False
return so invalid UTF-8 metadata is treated as an unreadable cache entry.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b8058a34-b3c2-421b-875b-99734c917976

📥 Commits

Reviewing files that changed from the base of the PR and between f5fbaa1 and dd15978.

📒 Files selected for processing (11)
  • docs/edge/en/concepts/skills.mdx
  • lib/crewai/src/crewai/experimental/skills/__init__.py
  • lib/crewai/src/crewai/skills/__init__.py
  • lib/crewai/src/crewai/skills/cache.py
  • lib/crewai/src/crewai/skills/registry.py
  • lib/crewai/src/crewai/skills/validation.py
  • lib/crewai/src/crewai/utilities/agent_utils.py
  • lib/crewai/tests/agents/test_agent.py
  • lib/crewai/tests/skills/test_cache.py
  • lib/crewai/tests/skills/test_registry.py
  • lib/crewai/tests/utilities/test_agent_utils.py
🚧 Files skipped from review as they are similar to previous changes (9)
  • lib/crewai/src/crewai/skills/init.py
  • lib/crewai/src/crewai/experimental/skills/init.py
  • lib/crewai/tests/agents/test_agent.py
  • lib/crewai/src/crewai/skills/validation.py
  • lib/crewai/tests/skills/test_cache.py
  • lib/crewai/tests/utilities/test_agent_utils.py
  • lib/crewai/tests/skills/test_registry.py
  • lib/crewai/src/crewai/utilities/agent_utils.py
  • lib/crewai/src/crewai/skills/registry.py

Comment thread docs/edge/en/concepts/skills.mdx Outdated
Comment thread lib/crewai/src/crewai/skills/cache.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread lib/crewai/src/crewai/skills/registry.py
@joaomdmoura
joaomdmoura force-pushed the fix/skill-registry-deployment-auth branch from 15cd902 to ebc4c3d Compare July 26, 2026 15:40
Copilot AI review requested due to automatic review settings July 26, 2026 15:40

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ebc4c3d. Configure here.

Comment thread lib/crewai/src/crewai/skills/registry.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

lib/crewai/src/crewai/skills/registry.py:284

  • When resolving a pinned skill version, an older installed AMP client may implement get_skill but not accept the version= keyword. Today that turns into a generic RuntimeError("Failed to download skill ..."), which makes the root cause hard to diagnose. Consider catching TypeError from the get_skill(..., version=...) call and raising a clearer error indicating the configured client/runtime needs to be upgraded to support pinned versions.
    try:
        api = _plus_client(ref)
        # Only pass the version when pinned, so clients predating version
        # support keep working for unpinned refs. The pin goes over verbatim and
        # the registry decides how to match it, including a leading "v".
        pin = {"version": version} if version else {}
        response = resolve_plus_response(api.get_skill(org, name, **pin))
        response.raise_for_status()

Skill downloads built their own `PlusAPI` and authenticated it from
`CREWAI_USER_PAT`, the platform integration token, or the saved CLI login.
Managed runtimes have no user credential to offer: they install a client of
their own, which `load_agent_from_repository` already resolves through, so
Agent Repository lookups worked while the skill downloads beside them failed
with 401.

Skills now resolve their client the same way, via `resolve_plus_client()` next
to the hook it reads. A client that can't fetch skills falls back to
environment credentials and warns, so older runtimes behave as they do today.
`resolve_plus_response()` shares the sync/async bridging both lookups need,
since `PlusAPI` is synchronous while managed clients are not.

Version pinning, which the same bug was hiding:

- Registry refs accept `@org/name@version`, and `@org/name@v1.2.0` since people
  write it both ways. `parse_skill_ref()` returns a `SkillRef(org, name,
  version)`; `parse_registry_ref()` keeps its `(org, name)` shape and drops the
  pin, so existing callers are unaffected
- Agent Repository agents record a version per skill, which was parsed off the
  response and dropped. Those pins now travel with the refs, so publishing a
  new version of a skill no longer changes every agent that uses it
- A pinned ref only accepts a project-local copy declaring that version in its
  `metadata.version` frontmatter, and the cache reports a miss when the version
  it recorded differs — so a pin re-resolves rather than loading another
  version. Unpinned refs keep hitting the cache as before
- An unknown pin fails instead of quietly falling back to the newest version

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 26, 2026 15:58
@joaomdmoura
joaomdmoura force-pushed the fix/skill-registry-deployment-auth branch from ebc4c3d to 5439601 Compare July 26, 2026 15:58
@joaomdmoura

Copy link
Copy Markdown
Collaborator Author

Closing out the review nitpicks that live in review bodies rather than threads, so they aren't lost:

Addressed

  • Add a case where the downloaded archive declares no metadata.version — covered by test_resolves_a_pinned_skill_from_the_cache, which serves an archive with no declared version, resolves the same pin twice and asserts a single download. This was the regression behind the double-validation fix.
  • Guard non-dict / corrupted cache metadata — the lookup now treats non-object JSON, non-UTF-8 files and non-string versions as a cache miss instead of raising. Three tests in test_cache.py.

Deliberately not changed

  • Default client credentials diverge between the Agent Repository and the skills registry — real, and left as-is on purpose. The Agent Repository default reads the CLI login only; skills kept their existing precedence (CREWAI_USER_PAT → platform token → CLI login, plus the org id). Unifying them would change which credential Agent(from_repository=...) picks for existing users, which is a behaviour change beyond this fix. In practice the difference is unreachable: managed runtimes install their own client and never build a default. There's now a comment at the call site recording that the difference is intentional so it doesn't get "fixed" into a behaviour change later. Happy to do it in a follow-up if you'd rather they match.
  • Expose an async resolve_response variant and reuse a module-level executor — the blocking behaviour is inherent to a sync bridge, and this code was moved rather than written here (it previously sat inline in load_agent_from_repository). A per-call ThreadPoolExecutor(max_workers=1) matches the existing pattern elsewhere in agent_utils; a module-level executor would keep a thread alive for the process lifetime. An await-able variant would be a new public API for callers that don't exist yet.

One earlier Bugbot report — pinned download trusts mismatched versions — was fixed rather than dismissed: download_skill now compares the version the registry served against the pin and refuses before caching, since caching the newest archive under a pinned label would poison every later lookup for that pin. See test_refuses_a_version_the_registry_served_instead_of_the_pin, which also asserts nothing is cached on refusal. A later re-report of the same finding pointed at a line range that already contained the guard.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread lib/crewai/src/crewai/skills/registry.py

@alex-clawd alex-clawd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

The piece I thought would be the hard part is handled well. OSS can't import crewai-enterprise, so the runtime's authenticated client has to arrive some other way — and rather than inventing a skills-specific hook, this routes through the existing _create_plus_client_hook via a shared resolve_plus_client. Same seam the Agent Repository already uses, so hosted runtimes get one client for both instead of two mechanisms drifting apart.

Two details worth calling out:

default is a callable, not a value. So the CREWAI_USER_PAT / integration-token lookups only evaluate when no hook is installed. In a deployment the fallback never runs at all, which is the point — a hosted runtime shouldn't be reaching for user credentials even to discard them.

Falling back when the installed client predates get_skill. A runtime pinned to an older enterprise package installs a client without the method; degrading to a client that can serve the request beats raising on an attribute check.

23 pass / 4 pending, no failures.

One note, not blocking: resolve_plus_response raising TypeError for an awaitable already bound to a loop is the right guard, but it'll surface as a load-time error rather than something obviously about skills. Worth a look if anyone reports a confusing failure from an async context.

joaomdmoura and others added 2 commits July 26, 2026 13:10
A blank `version` passed to `download_skill` read as "unpinned" and quietly
resolved the latest version, which is not what a caller supplying one asked
for — and it disagreed with `parse_skill_ref`, which already rejects empty
pins. Not reachable through `resolve_registry_ref` or the Agent Repository
auto-pinning, both of which only ever pass a non-empty version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 26, 2026 16:13

@alex-clawd alex-clawd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving after the merge from main (6d12fff).

Re-checked rather than rubber-stamping: same 11 files, same +890/-81, and the substance I reviewed is unchanged —

  • resolve_plus_client still routes through the existing _create_plus_client_hook, so skills and the Agent Repository share one runtime-installed client instead of two mechanisms drifting apart
  • default is still a callable, so the CREWAI_USER_PAT / integration-token lookups never evaluate in a hosted runtime
  • the fallback for a client predating get_skill is still there

17 pass / 7 pending, no failures.

Standing note from the first pass, still not blocking: resolve_plus_response raising TypeError for a loop-bound awaitable is the correct guard, but it surfaces as a generic load-time error rather than something identifiably about skills. Worth remembering if a confusing async failure gets reported.

@alex-clawd alex-clawd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving after 828259f.

This one's a real change rather than a merge, so I read it: a blank version pin now raises instead of being treated as unpinned.

Good catch, and the right direction. "" or " " reaching download_skill previously read as "no pin" and floated to latest — silently giving a caller who did pass a version something other than what they asked for. Failing loudly is correct; it's the same principle as the 404-on-unhonourable-pin in #3851 rather than falling back.

installed.get_skill.assert_not_called() in the test is the part I'd have wanted: it proves the guard fires before the request goes out, not that the request merely returned something reasonable.

Everything from the previous passes is unchanged — resolve_plus_client through the existing _create_plus_client_hook, default still a callable so credential lookups don't evaluate in a hosted runtime, and the fallback for clients predating get_skill.

11 pass / 12 pending / 2 skipping, no failures.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread lib/crewai/src/crewai/utilities/agent_utils.py
When resolve_plus_response bridges an async client from inside a running loop
it runs the coroutine on a worker thread, which starts with empty ContextVars.
A client reading runtime state there — the platform integration token, flow
context — would see defaults rather than the caller's values, which is hard to
diagnose from the resulting auth or routing failure.

Copy the context across, matching how the parallel-summarization bridge in this
module already does it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 26, 2026 16:21

@alex-clawd alex-clawd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — 44a830f is a real bug fix, and a subtle one.

resolve_plus_response hands the coroutine to a worker thread when it's already inside a running loop, and a fresh thread starts with empty ContextVars. So a client reading runtime state would have seen defaults rather than the caller's values. Confirmed the mechanism rather than taking it on faith:

without copy_context: DEFAULT
with    copy_context: caller-value

That matters directly for this PR's purpose. get_platform_integration_token() is a ContextVar (context.py:25), so in a deployment resolving a skill from inside an async flow, the installed client could have lost the very credential this change exists to use — and it would have failed as a 401 that looks exactly like the bug being fixed, only intermittently and only from async callers. Nasty one to debug later.

ctx.run(asyncio.run, ...) is the right shape: copy at submit time on the caller's thread, so the worker sees a snapshot rather than racing.

The test asserts through the real platform_context manager rather than a stub ContextVar, so it would catch a regression in the actual credential path, not just in copy_context plumbing.

tests/utilities/test_agent_utils.py → 66 passed locally. CI 13 pass / 11 pending, no failures.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

@joaomdmoura
joaomdmoura merged commit cc07598 into main Jul 26, 2026
56 checks passed
@joaomdmoura
joaomdmoura deleted the fix/skill-registry-deployment-auth branch July 26, 2026 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants