diff --git a/openhands-agent-server/openhands/agent_server/profiles_router.py b/openhands-agent-server/openhands/agent_server/profiles_router.py index cea019baee..ddc23615c9 100644 --- a/openhands-agent-server/openhands/agent_server/profiles_router.py +++ b/openhands-agent-server/openhands/agent_server/profiles_router.py @@ -20,7 +20,13 @@ get_llm_profile_store, get_settings_store, ) -from openhands.sdk.llm import LLM +from openhands.sdk.llm import LLM, Message, TextContent +from openhands.sdk.llm.exceptions import ( + LLMError, + LLMRateLimitError, + LLMServiceUnavailableError, + LLMTimeoutError, +) from openhands.sdk.llm.llm_profile_store import ( PROFILE_NAME_PATTERN, ProfileLimitExceeded, @@ -211,6 +217,111 @@ async def save_profile( return ProfileMutationResponse(name=name, message=f"Profile '{name}' saved") +class ValidateProfileRequest(BaseModel): + """Request body for LLM pre-flight validation. + + Accepts an LLM config (same shape as ``SaveProfileRequest.llm``) so the + frontend can validate a draft *before* persisting it. + """ + + llm: LLM + + +class ValidateProfileError(BaseModel): + """Structured error returned when validation fails.""" + + type: str + message: str + + +class ValidateProfileResponse(BaseModel): + """Result of an LLM pre-flight check.""" + + valid: bool + error: ValidateProfileError | None = None + + +# Errors that should NOT block saving — they are transient and unrelated to +# the correctness of the configuration itself. +_TRANSIENT_ERROR_TYPES = (LLMRateLimitError, LLMTimeoutError) + + +@profiles_router.post( + "/{name}/validate", + response_model=ValidateProfileResponse, +) +async def validate_profile( + request: Request, + name: ProfileName, + body: ValidateProfileRequest, +) -> ValidateProfileResponse: + """Pre-flight check: fire a minimal LLM completion to catch misconfigurations. + + Instantiates the submitted LLM config and sends a 1-token completion to + surface errors like invalid model names, missing provider prefixes, bad + base URLs, and invalid API keys — *before* the profile is saved. + + Transient errors (rate limits, timeouts) are treated as non-blocking: the + response is ``valid=True`` with no error, since those don't indicate a + misconfigured profile. + """ + cipher = get_cipher(request) + llm = decrypt_incoming_llm_secrets(body.llm, cipher) if cipher else body.llm + + messages = [ + Message( + role="user", + content=[TextContent(text="ping")], + ) + ] + + try: + # Mirror the runtime dispatch (see ``amake_llm_completion``) and stay + # async so provider I/O doesn't pin the FastAPI event loop. + if llm.uses_responses_api(): + await llm.aresponses(messages=messages, max_tokens=1) + else: + await llm.acompletion(messages=messages, max_tokens=1) + except _TRANSIENT_ERROR_TYPES as exc: + # Transient — don't block the save + logger.info( + f"Profile '{name}' pre-flight hit a transient error " + f"({type(exc).__name__}); not blocking save." + ) + return ValidateProfileResponse(valid=True) + except ( + # LLMServiceUnavailableError (provider 503) is intentionally treated as + # a blocking config error: at this layer a provider outage is + # indistinguishable from a wrong base URL, so we prefer a false + # negative over silently saving a misconfigured profile. + LLMServiceUnavailableError, + LLMError, + ) as exc: + error_type = type(exc).__name__ + logger.info(f"Profile '{name}' pre-flight failed: {error_type}: {exc.message}") + return ValidateProfileResponse( + valid=False, + error=ValidateProfileError( + type=error_type, + message=exc.message, + ), + ) + except Exception as exc: + # Unknown errors — block the save and surface the raw message so the + # user can debug, but classify generically. + msg = str(exc) or type(exc).__name__ + logger.info(f"Profile '{name}' pre-flight failed (unknown): {msg}") + return ValidateProfileResponse( + valid=False, + error=ValidateProfileError( + type=type(exc).__name__, + message=msg, + ), + ) + + return ValidateProfileResponse(valid=True) + + @profiles_router.delete("/{name}", response_model=ProfileMutationResponse) async def delete_profile( request: Request, name: ProfileName diff --git a/tests/agent_server/test_profiles_router.py b/tests/agent_server/test_profiles_router.py index e7530e71a6..ea7db2532a 100644 --- a/tests/agent_server/test_profiles_router.py +++ b/tests/agent_server/test_profiles_router.py @@ -1248,3 +1248,193 @@ def test_list_profiles_no_auto_create_after_deleting_active_profile(client, stor body = response.json() assert body["profiles"] == [] assert body["active_profile"] is None + + +# ── Pre-flight validation: POST /api/profiles/{name}/validate ──────────────── + + +def test_validate_profile_success(client): + """POST /api/profiles/{name}/validate returns valid=True when the LLM works.""" + from unittest.mock import MagicMock + + with ( + patch("openhands.sdk.llm.llm.LLM.uses_responses_api", return_value=False), + patch( + "openhands.sdk.llm.llm.LLM.acompletion", + return_value=MagicMock(), + ), + ): + response = client.post( + "/api/profiles/test-profile/validate", + json={"llm": {"model": "gpt-4o", "api_key": "sk-test"}}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["valid"] is True + assert body["error"] is None + + +def test_validate_profile_responses_api(client): + """Pre-flight uses the Responses API when the profile model requires it. + + Regression: the endpoint must route through ``aresponses`` for profiles + where ``uses_responses_api()`` is true, matching the runtime dispatch used + by real conversations (`amake_llm_completion`) — otherwise preflight would + validate a different path than the server actually calls. + """ + from unittest.mock import MagicMock + + traced: dict[str, bool] = {} + + def fake_append(messages, **kwargs): + traced["called"] = True + return MagicMock() + + with ( + patch( + "openhands.sdk.llm.llm.LLM.uses_responses_api", + return_value=True, + ), + patch("openhands.sdk.llm.llm.LLM.aresponses", side_effect=fake_append), + ): + response = client.post( + "/api/profiles/responses-profile/validate", + json={ + "llm": { + "model": "gpt-4o", + "api_mode": "responses", + "api_key": "sk-test", + } + }, + ) + + assert response.status_code == 200 + body = response.json() + assert body["valid"] is True + assert body["error"] is None + assert traced["called"], "expected aresponses to be used for responses-api profiles" + + +def test_validate_profile_invalid_model_returns_error(client): + """POST /api/profiles/{name}/validate returns valid=False on bad model.""" + from openhands.sdk.llm.exceptions import LLMBadRequestError + + with ( + patch("openhands.sdk.llm.llm.LLM.uses_responses_api", return_value=False), + patch( + "openhands.sdk.llm.llm.LLM.acompletion", + side_effect=LLMBadRequestError( + "The supported API model names are deepseek-v4-pro or " + "deepseek-v4-flash, but you passed deepseek-chat" + ), + ), + ): + response = client.post( + "/api/profiles/bad-model/validate", + json={"llm": {"model": "deepseek-chat", "api_key": "sk-test"}}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["valid"] is False + assert body["error"]["type"] == "LLMBadRequestError" + assert "deepseek" in body["error"]["message"] + + +def test_validate_profile_auth_error_returns_error(client): + """POST /api/profiles/{name}/validate returns valid=False on bad API key.""" + from openhands.sdk.llm.exceptions import LLMAuthenticationError + + with ( + patch("openhands.sdk.llm.llm.LLM.uses_responses_api", return_value=False), + patch( + "openhands.sdk.llm.llm.LLM.acompletion", + side_effect=LLMAuthenticationError("Invalid or missing API credentials"), + ), + ): + response = client.post( + "/api/profiles/bad-key/validate", + json={"llm": {"model": "gpt-4o", "api_key": "sk-invalid"}}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["valid"] is False + assert body["error"]["type"] == "LLMAuthenticationError" + + +def test_validate_profile_rate_limit_does_not_block(client): + """POST /api/profiles/{name}/validate returns valid=True on rate limit.""" + from openhands.sdk.llm.exceptions import LLMRateLimitError + + with ( + patch("openhands.sdk.llm.llm.LLM.uses_responses_api", return_value=False), + patch( + "openhands.sdk.llm.llm.LLM.acompletion", + side_effect=LLMRateLimitError("Rate limit exceeded"), + ), + ): + response = client.post( + "/api/profiles/rate-limited/validate", + json={"llm": {"model": "gpt-4o", "api_key": "sk-test"}}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["valid"] is True + assert body["error"] is None + + +def test_validate_profile_service_unavailable_blocks(client): + """POST /api/profiles/{name}/validate returns valid=False on provider 503.""" + from openhands.sdk.llm.exceptions import LLMServiceUnavailableError + + with ( + patch("openhands.sdk.llm.llm.LLM.uses_responses_api", return_value=False), + patch( + "openhands.sdk.llm.llm.LLM.acompletion", + side_effect=LLMServiceUnavailableError("Service unavailable"), + ), + ): + response = client.post( + "/api/profiles/unavailable/validate", + json={"llm": {"model": "gpt-4o", "api_key": "sk-test"}}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["valid"] is False + assert body["error"]["type"] == "LLMServiceUnavailableError" + + +def test_validate_profile_unknown_error_returns_error(client): + """POST /api/profiles/{name}/validate catches unexpected exceptions.""" + + with ( + patch("openhands.sdk.llm.llm.LLM.uses_responses_api", return_value=False), + patch( + "openhands.sdk.llm.llm.LLM.acompletion", + side_effect=RuntimeError("Connection refused"), + ), + ): + response = client.post( + "/api/profiles/unknown-err/validate", + json={"llm": {"model": "gpt-4o", "api_key": "sk-test"}}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["valid"] is False + assert body["error"]["type"] == "RuntimeError" + assert "Connection refused" in body["error"]["message"] + + +def test_validate_profile_invalid_name(client): + """POST /api/profiles/{name}/validate returns 422 for invalid profile names.""" + response = client.post( + "/api/profiles/invalid name/validate", + json={"llm": {"model": "gpt-4o", "api_key": "sk-test"}}, + ) + + assert response.status_code == 422