Repository navigation
feat: add pre-flight LLM validation endpoint (POST /api/profiles/{name}/validate) - #4422
Conversation
Add a validation endpoint that fires a minimal 1-token LLM completion
to catch misconfigurations (invalid model names, missing provider
prefixes, bad base URLs, invalid API keys) before a profile is saved.
Transient errors (rate limits, timeouts) are non-blocking. The endpoint
returns a structured {valid, error?: {type, message}} response using
the SDK's typed exception hierarchy.
Resolves OpenHands/OpenHands#16416
Co-authored-by: openhands <openhands@all-hands.dev>
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
|
🚦 CI is currently failing on this PR's latest commit. Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request This is an automated check - no AI was used to generate this comment. |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable — the endpoint shape is straightforward, but the validation call currently uses the wrong execution path for the server/runtime.
[CRITICAL ISSUES]
- [openhands-agent-server/openhands/agent_server/profiles_router.py, Line 281] Correctness / async safety: The endpoint calls synchronous
LLM.completion()directly inside an async FastAPI route. That can block the server event loop during provider I/O/retry waits, and it validates the chat-completions path even for profiles that runtime conversations will send through the Responses API.
[TESTING GAPS]
- [tests/agent_server/test_profiles_router.py, Line 1261] Add coverage for the runtime API path selection, especially
api_mode="responses"/uses_responses_api(), so this endpoint fails if it stops validating the same path conversations actually use.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
This is an additive REST endpoint, but it sits on the profile save path and performs live LLM provider I/O from the agent-server. The main risk is operational/correctness: a validation request can pin the event loop and can report a profile as valid/invalid based on a different LLM API path than normal conversations use.
VERDICT:
❌ Needs rework: Use the async runtime-equivalent LLM path (aresponses vs acompletion, or the shared agent helper) and add a test that covers Responses API profiles before a human maintainer relies on this endpoint.
KEY INSIGHT:
Preflight validation only buys safety if it exercises the same non-blocking provider path the agent-server will use in real conversations.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.- Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
- When your PR is merged, the guideline file goes through normal code review by repository maintainers.
Resolve with AI? Install the iterate skill in your agent and run
/iterateto automatically drive this PR through CI, review, and QA until it is merge-ready.Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
…mpletion) Co-authored-by: openhands <openhands@all-hands.dev>
|
@all-hands-bot I've pushed
Both inline review threads are resolved. Full agent-server profiles test file passes (85 tests). Requesting re-review. |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable — Clean, minimal solution that solves a documented real problem (PostHog showing thousands of LLM misconfiguration exceptions). The exception hierarchy is used correctly, the async dispatch mirrors the runtime path, and the 7 tests cover the important cases. Two minor improvement opportunities below.
[IMPROVEMENT OPPORTUNITIES]
-
[
profiles_router.py, Line 287] Observability: Transient error is caught without binding (except _TRANSIENT_ERROR_TYPES:), so the log message can't include which error type was swallowed. In a rate-limit scenario vs. a timeout, these are meaningfully different signals. Bind it so the log is actionable. -
[
profiles_router.py, Line 296] Transient mis-classification:LLMServiceUnavailableErroris treated as a blocking config error, but a 503 from the provider is indistinguishable from a provider outage vs. a wrong base URL. During a provider outage, a user with a perfectly valid config will seevalid=falseand be confused. Consider moving it to_TRANSIENT_ERROR_TYPESor documenting the deliberate trade-off in a comment so the next reader understands the choice.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
New endpoint opens a real LLM I/O path from an HTTP handler: any caller authenticated to the agent server can fire arbitrary 1-token completions against any provider + API key they supply. This is consistent with whatsave_profilealready allows and presumably sits behind the same auth middleware, but it's worth noting the endpoint can be used as a low-friction key oracle (confirm validity of any key in one call). TheProfileNamepath-parameter validation (422 on invalid names) is a nice defensive touch.
VERDICT:
✅ Worth merging — Functionally correct, solves a documented pain point, 7 focused tests. The LLMServiceUnavailableError classification is the one call worth revisiting before the frontend wires this into the save flow.
KEY INSIGHT:
The dispatch correctly mirrors amake_llm_completion's uses_responses_api() branching — good future-proofing — but LLMServiceUnavailableError in the blocking handler will produce false negatives during provider outages.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.- Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
- When your PR is merged, the guideline file goes through normal code review by repository maintainers.
Resolve with AI? Install the iterate skill in your agent and run
/iterateto automatically drive this PR through CI, review, and QA until it's merge-ready.Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
HUMAN:
Validated the endpoint behavior with the listed focused pytest cases.
Summary
Adds a POST /api/profiles/{name}/validate endpoint to the agent-server that fires a minimal 1-token LLM completion call to catch misconfigurations before a profile is saved.
This resolves the backend half of OpenHands/OpenHands#16416.
AGENT:
Why
PostHog error tracking shows thousands of recurring exceptions from LLM misconfigurations — invalid model names, missing provider prefixes, bad base URLs, and invalid API keys — that only surface when a conversation starts and the first LLM call fails. All of these could be caught at save time.
Summary
REST API contract changes
Compared with base OpenAPI
c7e270aae43afor public/api/**paths.How to Test
This PR was created by an AI agent (OpenHands) on behalf of the user.
🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:03dab92-pythonRun
All tags pushed for this build
About Multi-Architecture Support
03dab92-python) is a multi-arch manifest supporting both amd64 and arm6403dab92-python-amd64) are also available if neededCloses #4429