Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 112 additions & 1 deletion openhands-agent-server/openhands/agent_server/profiles_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Important: This does not reliably cap the preflight to one output token. select_responses_options() expects max_output_tokens, while select_chat_options() expects max_completion_tokens; passing max_tokens=1 leaves the SDK's full default budget in place (max_output_tokens / max_completion_tokens) and can also send an unsupported Responses API parameter. Use the canonical kwargs for each path (and assert them in tests) so the endpoint validates the same minimal request it documents.

else:
await llm.acompletion(messages=messages, max_tokens=1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retry latency can block the endpoint for minutes.

acompletion/aresponses are wrapped in the LLM's tenacity retry decorator with defaults: num_retries=5, retry_min_wait=8s, retry_max_wait=64s, timeout=300s. Retryable exceptions include APIConnectionError, RateLimitError, ServiceUnavailableError, LiteLLMTimeout, and InternalServerError.

A bad base_url will retry with exponential backoff before returning — ~2 min in the fast-failure case, up to ~27 min if the host accepts connections but never responds. For an interactive pre-flight check, consider a throwaway copy with reduced settings:

probe = llm.model_copy(update={"num_retries": 0, "timeout": 30})

The first-attempt error is all a validation probe needs.

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,
Comment thread
neubig marked this conversation as resolved.
LLMError,
Comment thread
neubig marked this conversation as resolved.
) 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
Expand Down
190 changes: 190 additions & 0 deletions tests/agent_server/test_profiles_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Comment thread
neubig marked this conversation as resolved.

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
Loading