Feature/yuchen.liu/mcp skills resources provider - #84
Closed
Million-mo wants to merge 30 commits into
Closed
Conversation
Million-mo
commented
Aug 22, 2026
Million-mo
commented
Owner
OpenCode sends skill commands without the 'skill:' prefix (e.g., 'systematic_troubleshooting' instead of 'skill:systematic_troubleshooting'). Changes: - Remove 'skill:' prefix from skill command names in skill_bridge.py - Update session_routes.py to detect skill commands by looking them up in pool.skill_commands instead of checking for 'skill:' prefix - Fix TextPart parameter names (message_id/session_id vs messageID/sessionID) This aligns AgentPool with OpenCode's actual protocol behavior where skills are first-class commands without prefixes.
OpenCode requires commands to have 'source' field (command | mcp | skill) and 'template' field for skill commands (SKILL.md content). Changes: - Add source: Literal[command, mcp, skill] to Command model - Add template: str to Command model for skill content - Update skill_bridge to store SkillCommand objects for template access - Update list_commands to include source=skill and template for skills This enables OpenCode to: 1. Display skill badges in slash command picker 2. Load skill template when skill command is selected
…ilable When pool.skill_commands is None (skill command registry not initialized), but skills are available via pool.skill_provider (MCP provider), the GET /command endpoint was returning empty list. Now list_commands falls back to fetching skills directly from skill_provider when skill_bridge is not available. This ensures skills from MCP providers are exposed as slash commands in OpenCode even when the skill command registry is not initialized.
This commit implements the complete RFC-0020 specification for loading skills via skill:// URIs from MCP providers. Core Features: - skill:// URI scheme support with provider/skill/reference paths - AggregatingResourceProvider for combining local + MCP skill sources - LocalResourceProvider with filesystem caching - Extended MCPResourceProvider with skill:// resource discovery - Argument substitution ($1, $2, \, \) - Security: path traversal protection, null byte detection Bug Fixes: - Fix skill description from SKILL.md frontmatter - Fix content loading via get_skill_instructions() - Support underscore in skill names (e.g., diagnosis_planning) - Handle PurePosixPath for skill:// URIs Tests: 429 new tests (security, performance, integration, E2E) Refs: RFC-0020
When using bare skill name (not URI), load_skill only checked pool.skills (local filesystem skills), missing MCP-based skills in pool.skill_provider. Now load_skill: 1. Checks both pool.skills and pool.skill_provider 2. Loads instructions from appropriate source This allows agents to load MCP-based skills by name.
list_skills only checked pool.skills (local skills), missing MCP-based skills from pool.skill_provider. Changes: 1. list_skills now checks both pool.skills and pool.skill_provider 2. Added comprehensive tests for MCP skills integration - test_list_skills_includes_mcp_skills - test_load_skill_finds_mcp_skills - test_load_skill_returns_error_for_missing_skill - test_list_skills_shows_empty_when_no_skills - test_load_skill_with_uri Run tests: uv run pytest tests/skills/test_mcp_skills_integration.py -v
Skill names must use hyphens (-) not underscores (_). Updated test to use 'systematic-troubleshooting' format. All 5 tests now pass: - test_list_skills_includes_mcp_skills ✓ - test_load_skill_finds_mcp_skills ✓ - test_load_skill_returns_error_for_missing_skill ✓ - test_list_skills_shows_empty_when_no_skills ✓ - test_load_skill_with_uri ✓
uri_resolver.py validation only allowed hyphens, but skill.py allowed both hyphens and underscores. This caused MCP skills with underscores in their names to fail validation. Now both files consistently allow: lowercase letters, digits, hyphens, and underscores in skill names.
Added test_load_skill_finds_mcp_skills_with_underscore to verify that skills with underscores in their names can be loaded. All 6 tests pass: - test_list_skills_includes_mcp_skills ✓ - test_load_skill_finds_mcp_skills_with_hyphen ✓ - test_load_skill_finds_mcp_skills_with_underscore ✓ - test_load_skill_returns_error_for_missing_skill ✓ - test_list_skills_shows_empty_when_no_skills ✓ - test_load_skill_with_uri ✓
1. _load_reference_content now receives pool parameter for virtual paths 2. Bare skill names now use skill_resolver for proper provider search instead of manually checking pool.skills and pool.skill_provider This ensures proper skill resolution with correct priority order.
1. _sync_commands now syncs MCP skills first, then local skills (local skills override MCP skills with same name) 2. list_skills now loads MCP skills first, then local skills (local skills take priority over MCP skills) This ensures local skills always have higher priority as per RFC.
Implemented skills_changed signal emission in on_added and on_removed callbacks using asyncio.create_task to handle async signal from sync callback context. This allows UI and other components to react to dynamic filesystem changes without manual refresh.
Remove eager fetching of instructions during skill discovery. Instructions are now lazy-loaded when load_instructions() is called or when explicitly requested via get_skill_instructions(). This improves performance for servers with many skills.
For PurePosixPath skills (virtual/MCP-based), load instructions from skill_provider instead of relying on skill.load_instructions() which returns empty for virtual paths. This fixes prompt-based skills not loading their instructions.
Added optional arguments parameter to get_skill_instructions for prompt-based MCP skills. Arguments are passed to prompt rendering to support server-side argument substitution.
Updated all provider implementations to include optional arguments parameter in get_skill_instructions method: - base.py: Updated abstract method signature - aggregating.py: Passes arguments to child providers - local.py: Accepts arguments (ignored for local skills) This ensures consistency across all provider implementations.
Added mock implementations for: - skill_resolver.resolve() to return appropriate skill based on URI - skill_provider.get_skill_instructions() to return test instructions All 6 tests now pass.
- Fix potential infinite loop in _on_skills_changed by removing dead event forwarding - Raise ValueError in load_instructions for virtual skills without pre-set instructions - Document argument space handling in skill-uri-usage.md - Add missing json import in MCP skills example
Update all callers of load_instructions() to properly handle virtual/MCP-based skills by using skill_provider.get_skill_instructions() when available: - agent_routes.py: Use provider for both skill_bridge and fallback paths - session_routes.py: Use provider for skill instructions lookup - skill_bridge.py: Add skill_provider parameter to OpenCodeSkillBridge - server.py: Pass skill_provider when creating OpenCodeSkillBridge
Add <uri> element to skill metadata format so agents know the correct skill:// URI to use when referencing skills from MCP servers. This improves the hit rate for skill loading by providing the full URI instead of requiring the agent to guess the format.
1. Fix bare skill name loading in skills.py - use bare name directly with resolver 2. Fix virtual path detection - use type() instead of isinstance() since UPath is subclass of PurePosixPath 3. Fix skill provider setup - use existing resource_provider from SkillsManager
Add checks for absolute paths (starting with /) in addition to .. traversal: 1. mcp_provider.py read_reference: Reject paths starting with / 2. uri_resolver.py: Reject reference paths starting with / This prevents attackers from reading arbitrary files like /etc/passwd
1. Fix path traversal in skills.py _load_reference_content 2. Make _setup_skills_provider async in pool.py 3. Use PurePosixPath for virtual URIs in mcp_provider.py 4. Fix read_reference return type to tuple[bytes, str] 5. Add read_reference to AggregatingResourceProvider
1. Fix command_registry.py to call _sync_commands() for proper priority 2. Use actual provider name from skill metadata for skill_uri
1. local.py: Use rglob for recursive reference search 2. base.py: Add missing skill-related methods (get_skills, get_skill, get_references, read_reference)
MCP servers may use underscores in skill names while users may use hyphens. This fix treats hyphens and underscores as equivalent when matching skill names. Changes: 1. uri_resolver.py: Normalize skill names by replacing hyphens with underscores 2. mcp_provider.py: Add get_skill method with normalized name matching
Owner
Author
|
Owner
Author
|
Owner
Author
|
Owner
Author
|
Owner
Author
|
Owner
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.