From c7c9ee0069eb2f0ebdfc57a57528f2d792aff897 Mon Sep 17 00:00:00 2001 From: Alona Date: Wed, 17 Jun 2026 22:56:08 -0500 Subject: [PATCH] feat: surface a clear error when a custom sandbox image's agent-server SDK mismatches (#14883) --- .../live_status_app_conversation_service.py | 90 +++++++++++- .../sandbox/sandbox_spec_service.py | 7 + .../test_verify_agent_server_version.py | 136 ++++++++++++++++++ 3 files changed, 231 insertions(+), 2 deletions(-) create mode 100644 tests/unit/app_server/test_verify_agent_server_version.py diff --git a/openhands/app_server/app_conversation/live_status_app_conversation_service.py b/openhands/app_server/app_conversation/live_status_app_conversation_service.py index b0b8a2138f..e3473c4769 100644 --- a/openhands/app_server/app_conversation/live_status_app_conversation_service.py +++ b/openhands/app_server/app_conversation/live_status_app_conversation_service.py @@ -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 = """ 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: diff --git a/openhands/app_server/sandbox/sandbox_spec_service.py b/openhands/app_server/sandbox/sandbox_spec_service.py index 29f7fc505f..bfb70eea55 100644 --- a/openhands/app_server/sandbox/sandbox_spec_service.py +++ b/openhands/app_server/sandbox/sandbox_spec_service.py @@ -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_') diff --git a/tests/unit/app_server/test_verify_agent_server_version.py b/tests/unit/app_server/test_verify_agent_server_version.py new file mode 100644 index 0000000000..30ceaf1219 --- /dev/null +++ b/tests/unit/app_server/test_verify_agent_server_version.py @@ -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: '; 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