Python: [Breaking] Refactor Skill API to async resource and script lookup#6135
Conversation
…okup Port of .NET commit 08541ee. Replace property-based Skill.content/resources/scripts with async by-name lookup methods: - content property -> async get_content() -> str - resources property -> async get_resource(name) -> SkillResource | None - scripts property -> async get_script(name) -> SkillScript | None SkillsProvider now always includes all three tools (load_skill, read_skill_resource, run_skill_script) and both instruction blocks regardless of whether any skills have resources or scripts. ClassSkill retains resources/scripts properties as overridable hooks for subclass reflection-based discovery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the Python Skills API to use async content/resource/script lookup methods and updates provider/tool behavior to always expose the skill interaction tools.
Changes:
- Replaces
Skill.contentwith asyncget_content(), and adds asyncget_resource()/get_script()lookup methods. - Updates
SkillsProviderto call async skill lookups and always registerload_skill,read_skill_resource, andrun_skill_script. - Revises skill tests for async content loading, always-present tools/instructions, and updated internal resource/script storage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
python/packages/core/agent_framework/_skills.py |
Refactors skill content/resource/script APIs and provider tool/instruction creation. |
python/packages/core/tests/core/test_skills.py |
Updates unit tests to match async skill APIs and always-registered tool behavior. |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 85%
✓ Correctness
No actionable issues found in this dimension.
✓ Security Reliability
No actionable issues found in this dimension.
✓ Test Coverage
No actionable issues found in this dimension.
✓ Design Approach
The async lookup refactor is mostly coherent, but the new
InlineSkill/FileSkilllookup methods hard-code the private backing lists instead of going through an overridable collection hook. That makes these two skill types behave differently fromClassSkilland silently drops any subclass that customizes resource/script exposure via a property or descriptor. That change makes the tests validate private storage rather than the supported extension point, so they can miss regressions in implementations that customizescriptswhile still passing these tests.
Automated review by semenshi's agents
Motivation and Context
Port of .NET commit 08541ee (.NET: [Breaking] Refactor AgentSkill API to async resource and script lookup #6030).
Description
Refactor
SkillAPI from synchronous properties to async by-name lookup methods:contentproperty toasync get_content() -> strresourcesproperty toasync get_resource(name) -> SkillResource | Nonescriptsproperty toasync get_script(name) -> SkillScript | NoneEach skill subclass owns its lookup.
ClassSkillretainsresources/scriptsproperties as overridable hooks.SkillsProvideralways includes all 3 tools and both instruction blocks regardless of whether skills have resources or scripts.Contribution Checklist