Make declarative refs work across flows and crews#6326
Conversation
![corridor-security[bot]](https://avatars.githubusercontent.com/in/1177339?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: This PR centralizes declarative reference resolution and extends crew JSON tool loading to support module:ClassName refs with BaseTool subclass validation.
Risk: Low risk. No exploitable security vulnerabilities were identified; the added dynamic import path is tied to project configuration/runtime tool loading and includes class/type validation, while validation paths avoid importing project code.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a shared ChangesDeclarative refs and import-ref tools
Sequence Diagram(s)sequenceDiagram
participant json_loader._resolve_tools
participant json_loader._instantiate_tool_import_ref
participant resolve_class_ref
participant project_tools.LookupTool
json_loader._resolve_tools->>json_loader._instantiate_tool_import_ref: tools entry "project_tools:LookupTool"
json_loader._instantiate_tool_import_ref->>resolve_class_ref: resolve BaseTool subclass
resolve_class_ref->>project_tools.LookupTool: import module:qualname
resolve_class_ref-->>json_loader._instantiate_tool_import_ref: validated class
json_loader._instantiate_tool_import_ref-->>json_loader._resolve_tools: instantiated tool
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/src/crewai/project/json_loader.py (1)
1857-1866: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffError mapping is coupled to
resolve_ref/resolve_class_refmessage text.The branch keys off
message.startswith("unresolvable ")and the literal"expected 'module:qualname'"substring. Any future wording change indeclarative_refswill silently reclassify those errors as "expected a BaseTool class", producing misleading guidance. Consider distinguishing the cases structurally (e.g. dedicated exception subclasses or an error code onInvalidRefError) rather than string sniffing.🤖 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/project/json_loader.py` around lines 1857 - 1866, The error mapping in the InvalidRefError handler is relying on resolve_ref/resolve_class_ref message text, which is brittle and can misclassify future errors. Update the JSONProjectError translation in json_loader to distinguish unresolvable references from invalid class references using structured data such as dedicated InvalidRefError subclasses or an error code/flag on InvalidRefError, and then branch on that instead of checking string prefixes/substrings before raising the appropriate JSONProjectError message.
🤖 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/src/crewai/project/json_loader.py`:
- Around line 1857-1866: The error mapping in the InvalidRefError handler is
relying on resolve_ref/resolve_class_ref message text, which is brittle and can
misclassify future errors. Update the JSONProjectError translation in
json_loader to distinguish unresolvable references from invalid class references
using structured data such as dedicated InvalidRefError subclasses or an error
code/flag on InvalidRefError, and then branch on that instead of checking string
prefixes/substrings before raising the appropriate JSONProjectError message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c6bb7f9-816f-4a8b-9d30-940f4867e067
📒 Files selected for processing (6)
lib/crewai/src/crewai/flow/runtime/__init__.pylib/crewai/src/crewai/flow/runtime/_actions.pylib/crewai/src/crewai/flow/runtime/_refs.pylib/crewai/src/crewai/project/json_loader.pylib/crewai/src/crewai/utilities/declarative_refs.pylib/crewai/tests/project/test_json_loader.py
💤 Files with no reviewable changes (1)
- lib/crewai/src/crewai/flow/runtime/_refs.py
Declarative flows already used `module:qualname` refs for runtime objects, but crew JSON tools still had their own lookup path. That meant examples like `project_tools:LookupTool` were treated as named `crewai_tools` lookups and failed with guidance that only mentioned `SerperDevTool` or `custom:<name>`. Invalid refs such as `not_tools:NotATool` also missed the same BaseTool validation used by flow tool actions. Move ref resolution into a shared declarative helper, use it from flow tool actions and crew JSON loading, and require tool refs to resolve to `BaseTool` classes before instantiation. Validation still checks tool refs structurally, so validating a crew does not import or execute project code.
vinibrsl
force-pushed
the
single-ref-loading
branch
from
096586d to
2ae2bda
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2ae2bda. Configure here.
2 participants
Declarative flows already used
module:qualnamerefs for runtime objects, but crew JSON tools still had their own lookup path. That meant examples likeproject_tools:LookupToolwere treated as namedcrewai_toolslookups and failed with guidance that only mentionedSerperDevToolorcustom:<name>. Invalid refs such asnot_tools:NotAToolalso missed the same BaseTool validation used by flow tool actions.Move ref resolution into a shared declarative helper, use it from flow tool actions and crew JSON loading, and require tool refs to resolve to
BaseToolclasses before instantiation. Validation still checks tool refs structurally, so validating a crew does not import or execute project code.Note
Medium Risk
Shared ref helpers import user modules at load/runtime; behavior changes how crew JSON tool strings with colons are interpreted, though validation avoids executing tool import side effects.
Overview
Unifies
module:qualnamereference handling so declarative flows and JSON crew projects share one contract instead of parallel lookup logic.Ref resolution moves from
flow/runtime/_refs.pyintocrewai.utilities.declarative_refs(resolve_ref,resolve_class_ref,InvalidRefError). Flow runtime keeps auto-instantiation for providers via a local_resolve_instance_ref. Flow tool actions now useresolve_class_refwith aBaseToolsubclass check before no-arg instantiation.Crew JSON tool strings that contain
:(e.g.project_tools:LookupTool) are resolved as import refs with the sameBaseToolvalidation and instantiation errors as flow tools—not as barecrewai_toolsclass names. Unknown-tool guidance mentionsmodule:ClassNamealongsideSerperDevToolandcustom:<name>.Tests add success/failure paths for import-based tools and assert
validate_crew_projectdoes not import tool refs (structural validation only).Reviewed by Cursor Bugbot for commit 2ae2bda. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
module:ClassNameimport references in tool configuration.Bug Fixes
Tests
module:ClassNamecases.