github/OpenHands
1
0
Fork 0
mirror of https://github.com/OpenHands/OpenHands.git synced 2026-03-22 05:37:20 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat: add registered_marketplaces to Settings for multiple marketplace support

Browse Source 
This PR introduces support for registering multiple marketplaces with explicit
auto-load semantics, providing an alternative to the single marketplace_path
approach in PR #13117.

## Changes

### New Models

**MarketplaceRegistration** - Registration for a plugin marketplace:
- name: Identifier for this marketplace registration
- source: Marketplace source (github:owner/repo, git URL, or local path)
- ref: Optional branch, tag, or commit
- repo_path: Subdirectory path for monorepos
- auto_load: 'all' to load plugins at conversation start, None for on-demand

### Updated Settings Model

Added registered_marketplaces field to Settings (list of MarketplaceRegistration).
This allows users to configure multiple marketplaces with different loading
behaviors.

### Updated Skill Loading

- skill_loader: Added registered_marketplaces parameter to pass marketplace
  registrations to the agent-server API
- app_conversation_service_base: Added registered_marketplaces parameter to
  load_and_merge_all_skills method

### Tests

- Added comprehensive tests for MarketplaceRegistration model
- Added tests for Settings.registered_marketplaces field
- Added tests for skill_loader marketplace handling

## Key Behaviors

- Marketplace resolution composes instance → org → user (additive)
- auto_load='all' loads all plugins at conversation start
- auto_load=None registers marketplace for on-demand resolution
- Path validation rejects absolute paths and directory traversal

## Dependencies

This PR is designed to work with SDK PR #2495 which provides:
- MarketplaceRegistry class for managing registered marketplaces
- Plugin resolution via 'plugin-name@marketplace-name' syntax
- Lazy fetching and caching of marketplace manifests

## Related

- Alternative to #13117 (marketplace_path setting)
- Leverages SDK PR OpenHands/software-agent-sdk#2495
- Enables #12916 (org-level default resources)
- Aligns with #13188 (instance-default org proposal)
- Supports #10947 (OpenHands configuration proposal)
This commit is contained in:
openhands
2026-03-19 17:47:24 +00:00
parent 38648bddb3
commit 3a234b78a4
5 changed files with 417 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
openhands/app_server/app_conversation/app_conversation_service_base.py
View File

@@ -97,6 +97,7 @@ class AppConversationServiceBase(AppConversationService, ABC):
selected_repository: str | None,
project_dir: str,
agent_server_url: str,
registered_marketplaces: list | None = None,
) -> list[Skill]:
"""Load skills from all sources via the agent-server.
@@ -107,12 +108,16 @@ class AppConversationServiceBase(AppConversationService, ABC):
- Organization skills (from {org}/.openhands repo)
- Project/repo skills (from repo .agents/skills/, .openhands/microagents/, and legacy .openhands/skills/)
- Sandbox skills (from exposed URLs)
- Registered marketplaces (from user settings)
Args:
sandbox: SandboxInfo containing exposed URLs and agent-server URL
selected_repository: Repository name or None
project_dir: Project root directory (resolved via get_project_dir).
agent_server_url: Agent-server URL (required)
registered_marketplaces: Optional list of MarketplaceRegistration objects
from user settings. Marketplaces with auto_load='all' will have
their plugins loaded automatically.
Returns:
List of merged Skill objects from all sources, or empty list on failure
@@ -141,6 +146,7 @@ class AppConversationServiceBase(AppConversationService, ABC):
load_user=True,
load_project=True,
load_org=True,
registered_marketplaces=registered_marketplaces,
)
_logger.info(

33
openhands/app_server/app_conversation/skill_loader.py
View File

@@ -10,7 +10,10 @@ thin proxy that:
All source-specific skill loading is handled by the agent-server.
"""
from __future__ import annotations
import logging
from typing import TYPE_CHECKING
import httpx
from pydantic import BaseModel
@@ -22,6 +25,9 @@ from openhands.integrations.service_types import AuthenticationError
from openhands.sdk.context.skills import Skill
from openhands.sdk.context.skills.trigger import KeywordTrigger, TaskTrigger
if TYPE_CHECKING:
from openhands.storage.data_models.settings import MarketplaceRegistration
_logger = logging.getLogger(__name__)
@@ -59,6 +65,16 @@ class SkillInfo(BaseModel):
is_agentskills_format: bool = False
class MarketplaceRegistrationPayload(BaseModel):
"""Marketplace registration for agent-server API request."""
name: str
source: str
ref: str | None = None
repo_path: str | None = None
auto_load: str | None = None
async def _is_gitlab_repository(repo_name: str, user_context: UserContext) -> bool:
"""Check if a repository is hosted on GitLab.
@@ -272,6 +288,7 @@ async def load_skills_from_agent_server(
load_user: bool = True,
load_project: bool = True,
load_org: bool = True,
registered_marketplaces: list[MarketplaceRegistration] | None = None,
) -> list[Skill]:
"""Load all skills from the agent-server.
@@ -288,12 +305,27 @@ async def load_skills_from_agent_server(
load_user: Whether to load user skills (default: True)
load_project: Whether to load project skills (default: True)
load_org: Whether to load organization skills (default: True)
registered_marketplaces: List of marketplace registrations (optional)
Returns:
List of Skill objects merged from all sources.
Returns empty list on error.
"""
try:
# Convert marketplace registrations to API payload format
marketplace_payloads = None
if registered_marketplaces:
marketplace_payloads = [
MarketplaceRegistrationPayload(
name=reg.name,
source=reg.source,
ref=reg.ref,
repo_path=reg.repo_path,
auto_load=reg.auto_load,
).model_dump()
for reg in registered_marketplaces
]
# Build request payload
payload = {
'load_public': load_public,
@@ -303,6 +335,7 @@ async def load_skills_from_agent_server(
'project_dir': project_dir,
'org_config': org_config.model_dump() if org_config else None,
'sandbox_config': sandbox_config.model_dump() if sandbox_config else None,
'registered_marketplaces': marketplace_payloads,
}
# Build headers

85
openhands/storage/data_models/settings.py
View File

@@ -1,7 +1,7 @@
from __future__ import annotations
from enum import Enum
from typing import Annotated
from typing import Annotated, Literal
from pydantic import (
BaseModel,
@@ -20,6 +20,78 @@ from openhands.core.config.utils import load_openhands_config
from openhands.storage.data_models.secrets import Secrets
class MarketplaceRegistration(BaseModel):
"""Registration for a plugin marketplace.
Represents a marketplace that can be registered for plugin resolution.
Marketplaces can be auto-loaded (plugins loaded at conversation start)
or registered only (available for explicit plugin references).
Examples:
>>> # Auto-load all plugins from a marketplace
>>> MarketplaceRegistration(
... name="public",
... source="github:OpenHands/skills",
... auto_load="all"
... )
>>> # Register marketplace without auto-loading
>>> MarketplaceRegistration(
... name="experimental",
... source="github:acme/experimental"
... )
>>> # Marketplace in monorepo subdirectory
>>> MarketplaceRegistration(
... name="team",
... source="github:acme/monorepo",
... repo_path="marketplaces/internal",
... auto_load="all"
... )
"""
name: str = Field(description='Identifier for this marketplace registration')
source: str = Field(
description="Marketplace source: 'github:owner/repo', git URL, or local path"
)
ref: str | None = Field(
default=None,
description='Optional branch, tag, or commit (only for git sources)',
)
repo_path: str | None = Field(
default=None,
description=(
'Subdirectory path within the git repository containing the marketplace '
"(e.g., 'marketplaces/internal' for monorepos). "
'Only relevant for git sources, not local paths.'
),
)
auto_load: Literal['all'] | None = Field(
default=None,
description=(
'Auto-load behavior for this marketplace. '
"'all' = load all plugins at conversation start. "
'None = registered for resolution but not auto-loaded.'
),
)
@field_validator('repo_path')
@classmethod
def validate_repo_path(cls, v: str | None) -> str | None:
"""Validate repo_path is a safe relative path within the repository."""
if v is None:
return v
# Must be relative (no absolute paths)
if v.startswith('/'):
raise ValueError('repo_path must be relative, not absolute')
# No parent directory traversal
if '..' in v:
raise ValueError(
"repo_path cannot contain '..' (parent directory traversal)"
)
return v
class SandboxGroupingStrategy(str, Enum):
"""Strategy for grouping conversations within sandboxes."""
@@ -72,6 +144,17 @@ class Settings(BaseModel):
sandbox_grouping_strategy: SandboxGroupingStrategy = (
SandboxGroupingStrategy.NO_GROUPING
)
# Marketplace registrations for plugin resolution
# Users can register multiple marketplaces with different auto-load behaviors
registered_marketplaces: list[MarketplaceRegistration] = Field(
default_factory=list,
description=(
'List of marketplace registrations for plugin resolution. '
"Marketplaces with auto_load='all' will have their plugins loaded "
'automatically at conversation start. '
'See MarketplaceRegistration for details.'
),
)
model_config = ConfigDict(
validate_assignment=True,

141
tests/unit/app_server/test_skill_loader.py
View File

@@ -691,3 +691,144 @@ class TestGetOrgRepositoryUrl:
# Assert
assert result is None
# ===== Tests for registered_marketplaces support =====
class TestLoadSkillsWithMarketplaces:
"""Test load_skills_from_agent_server with registered_marketplaces parameter."""
@pytest.mark.asyncio
@patch('httpx.AsyncClient')
async def test_passes_registered_marketplaces_in_payload(self, mock_client_class):
"""Test that registered_marketplaces is included in the API payload."""
from openhands.storage.data_models.settings import MarketplaceRegistration
# Arrange
mock_response = MagicMock()
mock_response.json.return_value = {
'skills': [],
'sources': {},
}
mock_response.raise_for_status = MagicMock()
mock_client = AsyncMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=None)
mock_client.post = AsyncMock(return_value=mock_response)
mock_client_class.return_value = mock_client
marketplaces = [
MarketplaceRegistration(
name='public',
source='github:OpenHands/skills',
auto_load='all',
),
MarketplaceRegistration(
name='team',
source='github:acme/plugins',
ref='v1.0.0',
repo_path='marketplaces/internal',
),
]
# Act
await load_skills_from_agent_server(
agent_server_url='http://localhost:8000',
session_api_key='test-key',
project_dir='/workspace/project',
registered_marketplaces=marketplaces,
)
# Assert
mock_client.post.assert_called_once()
call_args = mock_client.post.call_args
payload = call_args[1]['json']
assert 'registered_marketplaces' in payload
assert len(payload['registered_marketplaces']) == 2
# Verify first marketplace
mp1 = payload['registered_marketplaces'][0]
assert mp1['name'] == 'public'
assert mp1['source'] == 'github:OpenHands/skills'
assert mp1['auto_load'] == 'all'
assert mp1['ref'] is None
assert mp1['repo_path'] is None
# Verify second marketplace
mp2 = payload['registered_marketplaces'][1]
assert mp2['name'] == 'team'
assert mp2['source'] == 'github:acme/plugins'
assert mp2['ref'] == 'v1.0.0'
assert mp2['repo_path'] == 'marketplaces/internal'
assert mp2['auto_load'] is None
@pytest.mark.asyncio
@patch('httpx.AsyncClient')
async def test_handles_none_registered_marketplaces(self, mock_client_class):
"""Test that None registered_marketplaces is handled correctly."""
# Arrange
mock_response = MagicMock()
mock_response.json.return_value = {
'skills': [],
'sources': {},
}
mock_response.raise_for_status = MagicMock()
mock_client = AsyncMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=None)
mock_client.post = AsyncMock(return_value=mock_response)
mock_client_class.return_value = mock_client
# Act
await load_skills_from_agent_server(
agent_server_url='http://localhost:8000',
session_api_key='test-key',
project_dir='/workspace/project',
registered_marketplaces=None,
)
# Assert
mock_client.post.assert_called_once()
call_args = mock_client.post.call_args
payload = call_args[1]['json']
# registered_marketplaces should be None when not provided
assert payload.get('registered_marketplaces') is None
@pytest.mark.asyncio
@patch('httpx.AsyncClient')
async def test_handles_empty_registered_marketplaces(self, mock_client_class):
"""Test that empty registered_marketplaces list is handled correctly."""
# Arrange
mock_response = MagicMock()
mock_response.json.return_value = {
'skills': [],
'sources': {},
}
mock_response.raise_for_status = MagicMock()
mock_client = AsyncMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=None)
mock_client.post = AsyncMock(return_value=mock_response)
mock_client_class.return_value = mock_client
# Act
await load_skills_from_agent_server(
agent_server_url='http://localhost:8000',
session_api_key='test-key',
project_dir='/workspace/project',
registered_marketplaces=[],
)
# Assert
mock_client.post.assert_called_once()
call_args = mock_client.post.call_args
payload = call_args[1]['json']
# Empty list should be passed as None (falsy check in code)
assert payload.get('registered_marketplaces') is None

155
tests/unit/storage/data_models/test_settings.py
View File

@@ -1,14 +1,15 @@
import warnings
from unittest.mock import patch
from pydantic import SecretStr
import pytest
from pydantic import SecretStr, ValidationError
from openhands.core.config.llm_config import LLMConfig
from openhands.core.config.openhands_config import OpenHandsConfig
from openhands.core.config.sandbox_config import SandboxConfig
from openhands.core.config.security_config import SecurityConfig
from openhands.server.routes.settings import convert_to_settings
from openhands.storage.data_models.settings import Settings
from openhands.storage.data_models.settings import MarketplaceRegistration, Settings
def test_settings_from_config():
@@ -127,3 +128,153 @@ def test_settings_no_pydantic_frozen_field_warning():
assert len(frozen_warnings) == 0, (
f'Pydantic frozen field warnings found: {[str(w.message) for w in frozen_warnings]}'
)
# --- Tests for MarketplaceRegistration ---
class TestMarketplaceRegistration:
"""Tests for MarketplaceRegistration model."""
def test_basic_registration(self):
"""Test creating a basic marketplace registration."""
reg = MarketplaceRegistration(
name='test-marketplace',
source='github:owner/repo',
)
assert reg.name == 'test-marketplace'
assert reg.source == 'github:owner/repo'
assert reg.ref is None
assert reg.repo_path is None
assert reg.auto_load is None
def test_registration_with_auto_load(self):
"""Test registration with auto_load='all'."""
reg = MarketplaceRegistration(
name='public',
source='github:OpenHands/skills',
auto_load='all',
)
assert reg.auto_load == 'all'
def test_registration_with_ref(self):
"""Test registration with specific ref."""
reg = MarketplaceRegistration(
name='versioned',
source='github:owner/repo',
ref='v1.0.0',
)
assert reg.ref == 'v1.0.0'
def test_registration_with_repo_path(self):
"""Test registration with repo_path for monorepos."""
reg = MarketplaceRegistration(
name='monorepo-marketplace',
source='github:acme/monorepo',
repo_path='marketplaces/internal',
)
assert reg.repo_path == 'marketplaces/internal'
def test_repo_path_validation_rejects_absolute(self):
"""Test that absolute repo_path is rejected."""
with pytest.raises(ValidationError, match='must be relative'):
MarketplaceRegistration(
name='test',
source='github:owner/repo',
repo_path='/absolute/path',
)
def test_repo_path_validation_rejects_traversal(self):
"""Test that parent directory traversal is rejected."""
with pytest.raises(ValidationError, match="cannot contain '..'"):
MarketplaceRegistration(
name='test',
source='github:owner/repo',
repo_path='../escape/path',
)
def test_serialization(self):
"""Test that MarketplaceRegistration serializes correctly."""
reg = MarketplaceRegistration(
name='test',
source='github:owner/repo',
ref='main',
repo_path='plugins',
auto_load='all',
)
data = reg.model_dump()
assert data == {
'name': 'test',
'source': 'github:owner/repo',
'ref': 'main',
'repo_path': 'plugins',
'auto_load': 'all',
}
# --- Tests for Settings.registered_marketplaces ---
class TestSettingsRegisteredMarketplaces:
"""Tests for registered_marketplaces field in Settings."""
def test_settings_default_empty_registered_marketplaces(self):
"""Test that Settings defaults to empty registered_marketplaces."""
settings = Settings()
assert settings.registered_marketplaces == []
def test_settings_with_registered_marketplaces(self):
"""Test Settings with registered_marketplaces configured."""
marketplaces = [
MarketplaceRegistration(
name='public',
source='github:OpenHands/skills',
auto_load='all',
),
MarketplaceRegistration(
name='team',
source='github:acme/plugins',
),
]
settings = Settings(registered_marketplaces=marketplaces)
assert len(settings.registered_marketplaces) == 2
assert settings.registered_marketplaces[0].name == 'public'
assert settings.registered_marketplaces[0].auto_load == 'all'
assert settings.registered_marketplaces[1].name == 'team'
assert settings.registered_marketplaces[1].auto_load is None
def test_settings_serialization_with_registered_marketplaces(self):
"""Test Settings serialization includes registered_marketplaces."""
marketplaces = [
MarketplaceRegistration(
name='test',
source='github:owner/repo',
auto_load='all',
),
]
settings = Settings(registered_marketplaces=marketplaces)
data = settings.model_dump()
assert 'registered_marketplaces' in data
assert len(data['registered_marketplaces']) == 1
assert data['registered_marketplaces'][0]['name'] == 'test'
assert data['registered_marketplaces'][0]['auto_load'] == 'all'
def test_settings_from_dict_with_registered_marketplaces(self):
"""Test creating Settings from dict with registered_marketplaces."""
data = {
'registered_marketplaces': [
{
'name': 'custom',
'source': 'github:custom/repo',
'ref': 'v1.0.0',
'auto_load': 'all',
}
]
}
settings = Settings.model_validate(data)
assert len(settings.registered_marketplaces) == 1
assert settings.registered_marketplaces[0].name == 'custom'
assert settings.registered_marketplaces[0].ref == 'v1.0.0'