github/OpenHands
1
1
Fork 0
mirror of https://github.com/All-Hands-AI/OpenHands.git synced 2026-04-29 03:00:45 -04:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Pass plugin_path as a separate field instead of using get_resolved_source()

Browse Source 
- Remove get_resolved_source() method from PluginSpec model
- Update _finalize_conversation_request to pass plugin_source (without path)
  and plugin_path as separate fields to StartConversationRequest
- Remove 6 tests for get_resolved_source() method
- Update test_finalize_conversation_request_plugin_with_path to verify
  separate plugin_path field (skipped until SDK PRs #1647/#1651 are merged)

This addresses the review comment requesting plugin_path be passed separately
to avoid the SDK's parse_plugin_source() validation rejecting GitHub shorthand
with extra path segments.

Depends on: software-agent-sdk PRs #1647 and #1651

Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
openhands
2026-01-12 10:07:11 +00:00
parent be208d0b2f
commit 9a60fcfe14
3 changed files with 10 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
openhands/app_server/app_conversation/app_conversation_models.py
View File

@@ -43,20 +43,6 @@ class PluginSpec(BaseModel):
description='User-provided values for plugin input parameters',
)
def get_resolved_source(self) -> str:
"""Get the plugin source with path appended if specified.
For marketplace repositories that contain multiple plugins in subdirectories,
this returns the source with the subdirectory path appended.
"""
if self.path:
# Normalize the source (remove trailing slash if present)
base = self.source.rstrip('/')
# Normalize the path (remove leading/trailing slashes)
subpath = self.path.strip('/')
return f'{base}/{subpath}'
return self.source
class AppConversationInfo(BaseModel):
"""Conversation info which does not contain status."""

3
openhands/app_server/app_conversation/live_status_app_conversation_service.py
View File

@@ -1080,8 +1080,9 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
),
initial_message=final_initial_message,
secrets=secrets,
plugin_source=plugin.get_resolved_source() if plugin else None,
plugin_source=plugin.source if plugin else None,
plugin_ref=plugin.ref if plugin else None,
plugin_path=plugin.path if plugin else None,
)
async def _build_start_conversation_request_for_user(

74
tests/unit/app_server/test_live_status_app_conversation_service.py
View File

@@ -2087,13 +2087,16 @@ class TestPluginHandling:
assert result.initial_message is None
@pytest.mark.asyncio
@pytest.mark.skip(
reason='Requires SDK PRs #1647 and #1651 which add plugin_path to StartConversationRequest'
)
@patch(
'openhands.app_server.app_conversation.live_status_app_conversation_service.ExperimentManagerImpl'
)
async def test_finalize_conversation_request_plugin_with_path(
self, mock_experiment_manager
):
"""Test _finalize_conversation_request resolves plugin path correctly."""
"""Test _finalize_conversation_request passes plugin_path separately."""
from openhands.app_server.app_conversation.app_conversation_models import (
PluginSpec,
)
@@ -2138,12 +2141,11 @@ class TestPluginHandling:
# Assert
assert isinstance(result, StartConversationRequest)
# The plugin_source should be the resolved source with path appended
assert (
result.plugin_source
== 'github:owner/marketplace-repo/plugins/city-weather'
)
# plugin_source should be the source only (without path appended)
assert result.plugin_source == 'github:owner/marketplace-repo'
assert result.plugin_ref == 'main'
# plugin_path should be passed separately
assert result.plugin_path == 'plugins/city-weather'
@pytest.mark.asyncio
async def test_build_start_conversation_request_for_user_with_plugin(self):
@@ -2286,66 +2288,6 @@ class TestPluginSpecModel:
assert plugin.path == 'plugins/weather'
assert plugin.parameters == {'timeout': 30}
def test_get_resolved_source_without_path(self):
"""Test get_resolved_source returns source unchanged when no path is set."""
from openhands.app_server.app_conversation.app_conversation_models import (
PluginSpec,
)
plugin = PluginSpec(source='github:owner/repo')
assert plugin.get_resolved_source() == 'github:owner/repo'
def test_get_resolved_source_with_path(self):
"""Test get_resolved_source appends path to source."""
from openhands.app_server.app_conversation.app_conversation_models import (
PluginSpec,
)
plugin = PluginSpec(source='github:owner/repo', path='plugins/my-plugin')
assert plugin.get_resolved_source() == 'github:owner/repo/plugins/my-plugin'
def test_get_resolved_source_normalizes_trailing_slash_on_source(self):
"""Test get_resolved_source removes trailing slash from source."""
from openhands.app_server.app_conversation.app_conversation_models import (
PluginSpec,
)
plugin = PluginSpec(source='github:owner/repo/', path='plugins/my-plugin')
assert plugin.get_resolved_source() == 'github:owner/repo/plugins/my-plugin'
def test_get_resolved_source_normalizes_leading_slash_on_path(self):
"""Test get_resolved_source removes leading slash from path."""
from openhands.app_server.app_conversation.app_conversation_models import (
PluginSpec,
)
plugin = PluginSpec(source='github:owner/repo', path='/plugins/my-plugin')
assert plugin.get_resolved_source() == 'github:owner/repo/plugins/my-plugin'
def test_get_resolved_source_normalizes_both_slashes(self):
"""Test get_resolved_source handles both trailing and leading slashes."""
from openhands.app_server.app_conversation.app_conversation_models import (
PluginSpec,
)
plugin = PluginSpec(source='github:owner/repo/', path='/plugins/my-plugin/')
assert plugin.get_resolved_source() == 'github:owner/repo/plugins/my-plugin'
def test_get_resolved_source_with_local_path(self):
"""Test get_resolved_source works with local filesystem paths."""
from openhands.app_server.app_conversation.app_conversation_models import (
PluginSpec,
)
plugin = PluginSpec(
source='/home/user/.cache/plugins/repo-abc123',
path='plugins/city-weather',
)
assert (
plugin.get_resolved_source()
== '/home/user/.cache/plugins/repo-abc123/plugins/city-weather'
)
class TestAppConversationStartRequestWithPlugin:
"""Test cases for AppConversationStartRequest with plugin field."""