-
Notifications
You must be signed in to change notification settings - Fork 107
feat(agent-server): Support plugin loading when starting conversations #1651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implements #1645 - adds the ability to fetch plugins from remote sources (GitHub repositories, git URLs) and cache them locally. Changes: - Add Plugin.fetch() classmethod to fetch from remote sources - Add parse_plugin_source() to parse various source formats: - GitHub shorthand: 'github:owner/repo' - Git URLs: HTTPS, SSH, git:// protocol - Local paths (returned as-is) - Add PluginFetchError exception for fetch failures - Implement caching at ~/.openhands/cache/plugins/ - Support shallow clones for efficiency - Support specific ref (branch/tag/commit) checkout - Add comprehensive unit tests (34 new tests) Co-authored-by: openhands <[email protected]>
- Add blank line after imports in fetch.py - Remove unused import PluginFetchError in plugin.py - Reformat long lines in test file Co-authored-by: openhands <[email protected]>
- Create GitHelper class to encapsulate all git operations (clone, fetch, checkout, etc.) - Refactor fetch.py to use GitHelper via dependency injection - Update unit tests to mock GitHelper instead of subprocess.run - Add integration tests for real git operations - Add support for file:// URLs for local testing This improves test coverage by: 1. Allowing unit tests to execute actual fetch.py logic while mocking git operations 2. Providing separate integration tests that use real git operations Co-authored-by: openhands <[email protected]>
Coverage Report •
|
|||||||||||||||||||||||||||||||||||
Add comprehensive tests to improve coverage for the plugin module: - fetch.py: Add tests for set_git_helper(), relative path parsing, default cache_dir fallback, and PluginFetchError re-raise - git_helper.py: Add unit tests for error handling paths including CalledProcessError and TimeoutExpired for all git operations - plugin.py: Add tests for plugin loading edge cases including manifest parsing, skills/agents/commands loading, and error handling - types.py: Add tests for AgentDefinition and CommandDefinition frontmatter parsing including complex field handling Simplify integration test to avoid skill loading complexity. Co-authored-by: openhands <[email protected]>
Co-authored-by: openhands <[email protected]>
The --forked flag was preventing coverage from being collected properly because pytest-forked uses os.fork() which doesn't combine coverage data. Changes: - Split SDK test run: forked tests run separately, then non-forked tests with --cov-append to combine coverage data - Add 'forked' pytest marker to identify tests needing process isolation - Add coverage configuration with parallel mode and branch coverage - Add coverage report exclusions for common non-coverable patterns This allows most tests to run without forking for accurate coverage, while still supporting tests that need process isolation. Co-authored-by: openhands <[email protected]>
The change to run tests without --forked exposed pre-existing test pollution issues in test_state_serialization.py where tests share state incorrectly. Reverting to the original --forked approach until those tests are fixed. Coverage collection will remain affected by --forked, but tests will pass. Co-authored-by: openhands <[email protected]>
Requested Changes: Add
|
Add optional subpath parameter to both fetch_plugin() and Plugin.fetch() methods. This allows fetching plugins from subdirectories within a repository, which is needed when plugin paths are specified separately rather than embedded in the source string. Changes: - Add subpath parameter to fetch_plugin() in fetch.py - Add subpath parameter to Plugin.fetch() in plugin.py - Apply subpath to both local paths and remote repositories - Strip leading/trailing slashes from subpath - Raise PluginFetchError if subpath doesn't exist - Add 8 new unit tests for subpath functionality Addresses review comment on PR #1647. Co-authored-by: openhands <[email protected]>
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
Co-authored-by: openhands <[email protected]>
- Add plugin_source and plugin_ref fields to StartConversationRequest - Add _load_and_merge_plugin() method to fetch and load plugins - Add _merge_plugin_into_request() method to merge plugin skills and MCP config - Plugin skills override existing skills with the same name - Plugin MCP config is merged with existing config - Add comprehensive tests for plugin loading scenarios Closes #1650
Add plugin_path field to StartConversationRequest and pass it as subpath parameter to Plugin.fetch(). This enables fetching plugins from subdirectories within repositories (e.g., monorepos with multiple plugins). Changes: - models.py: Add plugin_path optional field to StartConversationRequest - conversation_service.py: Pass plugin_path as subpath to Plugin.fetch() - test_conversation_service.py: Update tests to verify subpath handling Note: This depends on PR #1647 adding the subpath parameter to Plugin.fetch() Co-authored-by: openhands <[email protected]>
4e78b1b to
96b6102
Compare
Summary
Extends the agent server to support loading plugins when starting conversations. When a
plugin_sourceis provided in theStartConversationRequest, the agent server will fetch the plugin (usingPlugin.fetch()from #1647) and load its skills, hooks, and MCP configuration into the conversation context.Implements: #1650
Dependency: This PR is based on
feat/plugin-fetchbranch (PR #1647) and requires it to be merged first.Context
This is part of the Plugin Directory feature (OpenHands/OpenHands#12088).
The key architectural insight is that plugin fetching must happen inside the sandbox/runtime (where the agent server runs), not on the app server. This is because:
See OpenHands/OpenHands#12085 comment for how this fits with other components.
Changes
1. Extended
StartConversationRequestmodelAdded two new optional fields:
plugin_source: Plugin source to fetch (e.g.,github:owner/repo, git URL, or local path)plugin_ref: Optional branch, tag, or commit for the plugin2. Added plugin loading logic to
ConversationService_load_and_merge_plugin(): Fetches and loads a plugin whenplugin_sourceis provided_merge_plugin_into_request(): Merges plugin skills and MCP config into the agent3. Plugin content merging
Testing
Added comprehensive tests for:
Files Changed
openhands-agent-server/openhands/agent_server/models.py- Addedplugin_sourceandplugin_reffieldsopenhands-agent-server/openhands/agent_server/conversation_service.py- Added plugin loading logictests/agent_server/test_conversation_service.py- Added tests for plugin loadingRelated Issues
@jpshackelford can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:79e66ae-pythonRun
All tags pushed for this build
About Multi-Architecture Support
79e66ae-python) is a multi-arch manifest supporting both amd64 and arm6479e66ae-python-amd64) are also available if needed