mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 13:38:55 +08:00
feat: surface a clear error when a custom sandbox image's agent-server SDK mismatches (#14883)
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import asyncio
|
||||
import importlib.metadata
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
@@ -13,6 +14,7 @@ from uuid import UUID, uuid4
|
||||
|
||||
import httpx
|
||||
from fastapi import Request
|
||||
from packaging.version import InvalidVersion, Version
|
||||
from pydantic import Field, SecretStr, TypeAdapter
|
||||
|
||||
from openhands.agent_server.models import (
|
||||
@@ -84,7 +86,11 @@ from openhands.app_server.sandbox.sandbox_models import (
|
||||
SandboxStatus,
|
||||
)
|
||||
from openhands.app_server.sandbox.sandbox_service import SandboxService
|
||||
from openhands.app_server.sandbox.sandbox_spec_service import SandboxSpecService
|
||||
from openhands.app_server.sandbox.sandbox_spec_service import (
|
||||
SandboxSpecService,
|
||||
get_agent_server_image,
|
||||
is_custom_agent_server_image,
|
||||
)
|
||||
from openhands.app_server.services.injector import InjectorState
|
||||
from openhands.app_server.services.jwt_service import JwtService
|
||||
from openhands.app_server.settings.llm_profiles import resolve_profile_llm
|
||||
@@ -128,6 +134,14 @@ _conversation_info_type_adapter = TypeAdapter(list[ConversationInfo | None])
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _expected_sdk_version() -> str | None:
|
||||
"""App's pinned openhands-sdk version, or None if its metadata is unresolvable."""
|
||||
try:
|
||||
return importlib.metadata.version('openhands-sdk')
|
||||
except importlib.metadata.PackageNotFoundError:
|
||||
return None
|
||||
|
||||
|
||||
# Planning agent instruction to prevent "Ready to proceed?" behavior
|
||||
PLANNING_AGENT_INSTRUCTION = """<IMPORTANT_PLANNING_BOUNDARIES>
|
||||
You are a Planning Agent that can ONLY create plans - you CANNOT execute code or make changes.
|
||||
@@ -292,6 +306,12 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
assert sandbox is not None
|
||||
agent_server_url = self._get_agent_server_url(sandbox)
|
||||
|
||||
# Custom sandbox images can ship an incompatible openhands-sdk; fail
|
||||
# fast with a clear error instead of an opaque 500 on create.
|
||||
await self._verify_agent_server_version(
|
||||
agent_server_url, sandbox.session_api_key
|
||||
)
|
||||
|
||||
# Mirror the user's LLM profiles into the sandbox so the agent's
|
||||
# built-in switch_llm tool can resolve them (in SaaS profiles live
|
||||
# on the app-server, not the sandbox filesystem). Before conversation
|
||||
@@ -377,7 +397,22 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
timeout=self.sandbox_startup_timeout,
|
||||
)
|
||||
|
||||
response.raise_for_status()
|
||||
try:
|
||||
response.raise_for_status()
|
||||
except httpx.HTTPStatusError as exc:
|
||||
# A custom image that 500s on create is usually an openhands-sdk
|
||||
# mismatch /server_info couldn't reveal; add an actionable hint.
|
||||
if is_custom_agent_server_image():
|
||||
expected = _expected_sdk_version()
|
||||
raise SandboxError(
|
||||
f'Conversation create failed (HTTP '
|
||||
f'{exc.response.status_code}) on custom sandbox image '
|
||||
f'{get_agent_server_image()}. Verify its openhands-sdk '
|
||||
f'matches this release'
|
||||
+ (f' ({expected})' if expected else '')
|
||||
+ '; rebuild/re-pin the image if not.'
|
||||
) from exc
|
||||
raise
|
||||
info = ConversationInfo.model_validate(response.json())
|
||||
# Determine kind / llm_model from the request we built (its
|
||||
# ``agent`` is the source of truth here): the response echoes
|
||||
@@ -793,6 +828,57 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
httpx_client=self.httpx_client,
|
||||
)
|
||||
|
||||
async def _verify_agent_server_version(
|
||||
self, agent_server_url: str, session_api_key: str | None
|
||||
) -> None:
|
||||
"""Fail fast with a clear error when an admin-pinned custom sandbox image
|
||||
runs a different openhands-sdk minor than this app, instead of the opaque
|
||||
500 the agent-server returns on create. Best-effort: only custom images are
|
||||
checked, and we fail open on anything we can't read."""
|
||||
if os.getenv('OH_SKIP_AGENT_SERVER_VERSION_CHECK', '').strip().lower() in (
|
||||
'1',
|
||||
'true',
|
||||
'yes',
|
||||
):
|
||||
return
|
||||
# Proxy-default images move with the release; only custom-pinned can drift.
|
||||
if not is_custom_agent_server_image():
|
||||
return
|
||||
expected = _expected_sdk_version()
|
||||
if not expected:
|
||||
return
|
||||
try:
|
||||
headers = {'X-Session-API-Key': session_api_key} if session_api_key else {}
|
||||
resp = await self.httpx_client.get(
|
||||
f'{agent_server_url.rstrip("/")}/server_info',
|
||||
headers=headers,
|
||||
timeout=30.0,
|
||||
)
|
||||
resp.raise_for_status()
|
||||
reported = str(resp.json().get('sdk_version', '')).strip()
|
||||
except Exception:
|
||||
# 404 (image predates /server_info) or transient errors: can't verify,
|
||||
# so don't block — the create POST still surfaces a custom-image hint.
|
||||
_logger.warning(
|
||||
'Could not read /server_info to verify agent-server SDK version',
|
||||
exc_info=True,
|
||||
)
|
||||
return
|
||||
# Endpoint present but metadata missing -> nothing to compare against.
|
||||
if reported in ('', 'unknown'):
|
||||
return
|
||||
try:
|
||||
if Version(reported).release[:2] == Version(expected).release[:2]:
|
||||
return
|
||||
except InvalidVersion:
|
||||
return
|
||||
raise SandboxError(
|
||||
f'Sandbox image {get_agent_server_image()} runs openhands-sdk '
|
||||
f'{reported}, but this release requires {expected}. Rebuild/re-pin the '
|
||||
'custom sandbox image to a matching openhands-sdk, or set '
|
||||
'OH_SKIP_AGENT_SERVER_VERSION_CHECK=1 to bypass.'
|
||||
)
|
||||
|
||||
async def _seed_sandbox_profiles(
|
||||
self, agent_server_url: str, session_api_key: str | None
|
||||
) -> None:
|
||||
|
||||
@@ -69,6 +69,13 @@ def get_agent_server_image() -> str:
|
||||
return AGENT_SERVER_IMAGE
|
||||
|
||||
|
||||
def is_custom_agent_server_image() -> bool:
|
||||
"""True only when an admin pinned a custom sandbox image (tag differs from the
|
||||
release-default tag). Default/upgrade installs keep the release tag, never gated."""
|
||||
tag = os.getenv('AGENT_SERVER_IMAGE_TAG')
|
||||
return bool(tag) and tag != AGENT_SERVER_IMAGE.rsplit(':', 1)[-1]
|
||||
|
||||
|
||||
# Prefixes for environment variables that should be auto-forwarded to agent-server
|
||||
# These are typically configuration variables that affect the agent's behavior
|
||||
AUTO_FORWARD_PREFIXES = ('LLM_', 'LMNR_')
|
||||
|
||||
@@ -0,0 +1,136 @@
|
||||
"""Tests for the agent-server SDK version check on conversation start.
|
||||
|
||||
Covers _verify_agent_server_version (only enforced for custom sandbox images,
|
||||
fail-open on anything it can't read) and the is_custom_agent_server_image gate.
|
||||
"""
|
||||
|
||||
import os
|
||||
from unittest.mock import AsyncMock, Mock, patch
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from openhands.app_server.app_conversation.live_status_app_conversation_service import (
|
||||
LiveStatusAppConversationService,
|
||||
_expected_sdk_version,
|
||||
)
|
||||
from openhands.app_server.errors import SandboxError
|
||||
from openhands.app_server.sandbox.sandbox_spec_service import (
|
||||
AGENT_SERVER_IMAGE,
|
||||
is_custom_agent_server_image,
|
||||
)
|
||||
|
||||
MODULE = 'openhands.app_server.app_conversation.live_status_app_conversation_service'
|
||||
PINNED_TAG = AGENT_SERVER_IMAGE.rsplit(':', 1)[-1]
|
||||
# A custom pin = a tag different from the release-default one.
|
||||
CUSTOM_ENV = {
|
||||
'AGENT_SERVER_IMAGE_REPOSITORY': 'harbor.example/agent-server',
|
||||
'AGENT_SERVER_IMAGE_TAG': 'custom-9',
|
||||
}
|
||||
|
||||
|
||||
def _service(client):
|
||||
# Only httpx_client is touched by the method under test.
|
||||
svc = LiveStatusAppConversationService.__new__(LiveStatusAppConversationService)
|
||||
svc.httpx_client = client
|
||||
return svc
|
||||
|
||||
|
||||
def _resp(sdk_version):
|
||||
return Mock(
|
||||
raise_for_status=Mock(),
|
||||
json=Mock(return_value={'sdk_version': sdk_version}),
|
||||
)
|
||||
|
||||
|
||||
async def _verify(client, env):
|
||||
with patch.dict(os.environ, env, clear=False):
|
||||
await _service(client)._verify_agent_server_version('http://agent.test/', 'k')
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_default_image_skips_check():
|
||||
client = Mock(get=AsyncMock())
|
||||
await _verify(client, {'AGENT_SERVER_IMAGE_TAG': PINNED_TAG})
|
||||
client.get.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize('val', ['1', 'true', 'TRUE', 'yes'])
|
||||
async def test_opt_out_skips_check(val):
|
||||
client = Mock(get=AsyncMock())
|
||||
await _verify(client, {**CUSTOM_ENV, 'OH_SKIP_AGENT_SERVER_VERSION_CHECK': val})
|
||||
client.get.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_matching_version_passes():
|
||||
expected = _expected_sdk_version()
|
||||
assert expected
|
||||
client = Mock(get=AsyncMock(return_value=_resp(expected)))
|
||||
await _verify(client, CUSTOM_ENV)
|
||||
client.get.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize('suffix', ['.99', '.0+c24'])
|
||||
async def test_patch_or_local_retag_tolerated(suffix):
|
||||
major, minor = _expected_sdk_version().split('.')[:2]
|
||||
client = Mock(get=AsyncMock(return_value=_resp(f'{major}.{minor}{suffix}')))
|
||||
await _verify(client, CUSTOM_ENV) # major.minor equal -> no raise
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_minor_mismatch_raises():
|
||||
expected = _expected_sdk_version()
|
||||
reported = f'{int(expected.split(".")[0]) + 1}.0.0'
|
||||
client = Mock(get=AsyncMock(return_value=_resp(reported)))
|
||||
with pytest.raises(SandboxError) as ei:
|
||||
await _verify(client, CUSTOM_ENV)
|
||||
# str() of a SandboxError (HTTPException) is '500: <msg>'; assert substrings.
|
||||
assert reported in str(ei.value)
|
||||
assert 'OH_SKIP_AGENT_SERVER_VERSION_CHECK' in str(ei.value)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize('reported', ['unknown', '', 'not-a-version'])
|
||||
async def test_unverifiable_versions_pass(reported):
|
||||
client = Mock(get=AsyncMock(return_value=_resp(reported)))
|
||||
await _verify(client, CUSTOM_ENV)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_200_server_info_fails_open():
|
||||
resp = Mock(
|
||||
raise_for_status=Mock(
|
||||
side_effect=httpx.HTTPStatusError(
|
||||
'x', request=Mock(), response=Mock(status_code=404)
|
||||
)
|
||||
)
|
||||
)
|
||||
client = Mock(get=AsyncMock(return_value=resp))
|
||||
await _verify(client, CUSTOM_ENV)
|
||||
client.get.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_transport_error_fails_open():
|
||||
client = Mock(get=AsyncMock(side_effect=httpx.ConnectError('down')))
|
||||
await _verify(client, CUSTOM_ENV)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_expected_version_skips_request():
|
||||
client = Mock(get=AsyncMock())
|
||||
with patch(f'{MODULE}._expected_sdk_version', return_value=None):
|
||||
await _verify(client, CUSTOM_ENV)
|
||||
client.get.assert_not_awaited()
|
||||
|
||||
|
||||
def test_is_custom_agent_server_image():
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
assert is_custom_agent_server_image() is False
|
||||
with patch.dict(os.environ, {'AGENT_SERVER_IMAGE_TAG': PINNED_TAG}, clear=False):
|
||||
assert is_custom_agent_server_image() is False
|
||||
with patch.dict(os.environ, {'AGENT_SERVER_IMAGE_TAG': 'custom-9'}, clear=False):
|
||||
assert is_custom_agent_server_image() is True
|
||||
Reference in New Issue
Block a user