docs(designs): Add skills sdk design#528
docs(designs): Add skills sdk design#528mkmeral wants to merge 9 commits intostrands-agents:mainfrom
Conversation
This design proposes adding a skills parameter to the Agent class for loading and managing AgentSkills.io compatible skills. Key features: - Simple API: Agent(skills='./skills') - Dynamic skill management between invocations - Active skill tracking - allowed_tools enforcement via hooks - Session manager integration Addresses: strands-agents/sdk-python#1181
- Follow documentation style guide (collaborative 'we', active voice) - Teach the concept before diving into API details - Show the problem first, then the solution - Use concrete examples throughout - Remove unnecessary complexity - Clearer structure following the design template
Key changes to the Skills SDK design: - Remove BeforeToolCallEvent hook approach for tool restrictions - Add pre-filtering mechanism: filter tools BEFORE sending to model - Model only sees tools it's allowed to use (cleaner, no wasted tokens) - Add detailed implementation section showing the filtering flow - Add 'Hook-Based Tool Enforcement' to Alternatives Considered section explaining why pre-filtering is the better approach The insight: why show the model tools it can't use? Pre-filtering is more efficient and requires no error recovery logic.
Less is more - the original doc is concise and matter-of-fact. Removed verbose explanations, ASCII diagrams, and repetition.
Documentation Deployment FailedThe documentation deployment encountered an error. Please check the deployment logs for more details. |
- Add docstrings to API signatures (properties, class methods) - Document error handling: SkillLoadError, when it's raised - Clarify active_skill detection mechanism - Add comment on allowed_tools=None meaning - Show proper import patterns in module exports - Reference DECISIONS.md re: why Skill doesn't extend HookProvider - Document behavior for non-existent tools in allowed_tools (warn, don't fail)
Documentation Deployment FailedThe documentation deployment encountered an error. Please check the deployment logs for more details. |
Documentation Deployment CompleteYour documentation preview has been successfully deployed! Preview URL: https://d3ehv1nix5p99z.cloudfront.net/pr-528/ |
labeveryday
left a comment
labeveryday
left a comment
There was a problem hiding this comment.
This will be a nice addition to Strands. Looking forward to the release.
| 1. Loads skill metadata (name, description, location) from each `SKILL.md` | ||
| 2. Registers a `skills` tool on the agent with `activate` and `deactivate` actions | ||
| 3. Registers a `BeforeInvocationEvent` hook that appends skill metadata to the system prompt | ||
| 4. When a skill with `allowed_tools` is activated, optionally removes non-allowed tools from the agent (keeping them in memory for restoration on deactivate) |
There was a problem hiding this comment.
What happens when a skill specifies allowed_tools that don't match any registered tools? How do you handle this?
There was a problem hiding this comment.
The tool filtering is opt-in. Unless skills are designed for the same ecosystem as the application, tool names will not match. By default, we won't filter, but devs can enable it
| ... | ||
| ``` | ||
|
|
||
| The `@tool` and `@hook` decorators inside the plugin class auto-register with the agent when the plugin is passed to the Agent constructor. This is the DX we want: declare what you need, the plugin protocol handles the wiring. |
There was a problem hiding this comment.
The plugin class does not show how the Plugin protocol discovers and registers. Can you clarify the DX claim?
There was a problem hiding this comment.
We have defined plugins as an abstraction so far, I'm also using skills as a playground to discover how it should be implemented. I'm working on a proposed implementation based on my learnings during this design. I'll share a draft PR soon
|
|
||
| ### Multiple active skills | ||
|
|
||
| Only one skill can be active at a time. Activating a new skill while one is already active implicitly deactivates the previous one (restoring any filtered tools before applying the new skill's `allowed_tools`). This keeps the mental model simple and avoids ambiguity around conflicting `allowed_tools` sets. |
There was a problem hiding this comment.
This feels like a gap. Real workflows often combine or daisy chain skills to review code and write docs, analyze security and check performance. The single-skill constraint forces sequential activation, which means:
- The agent loses the previous skill's instructions when switching
- You can't blend behaviors from multiple skills in one response (Unless it stays in context)
- Tool filtering becomes all-or-nothing per skill
There was a problem hiding this comment.
so technically you don't loose the instruction, because we return it as tool result, so it's still in messages. It's essentially only impacts tools.
That said, we can support multiple active skills by using and/or for allowed tools.
There was a problem hiding this comment.
I think specifying this flow would be enough.
|
|
||
| def __init__(self, skills: list[str | Path | Skill]): |
There was a problem hiding this comment.
Why is this _loaded_skills and not just _skills? I get whats happening but it would make it clear this is just the private internal storage for skills.
3 participants
WIP