Backend: Model providers — provider store + nested models + named-secret key (#15492) - #4455
Draft
juanmichelini wants to merge 7 commits into
Draft
Backend: Model providers — provider store + nested models + named-secret key (#15492)#4455juanmichelini wants to merge 7 commits into
juanmichelini wants to merge 7 commits into
Conversation
Contributor
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Add a first-class Provider Connection object: connect a vendor once with one key, pick from its model catalog, and spawn LLM profiles that reference the connection's key by name (secret:<name>) instead of inline-duplicating it. Agent-server (openhands-agent-server): - persistence: ProviderConnection + PersistedConnections models, FileConnectionsStore (single JSON, file-locked, atomic write), get_connections_store/reset_stores. - llm_connections router (mounted at /api/llm/connections): create/list/get/patch/ delete + /validate. The connection's key is stored as a named secret via the existing SecretsStore; responses never echo the key (api_key_set only). validate_provider_key is module-level so tests monkeypatch it (no network). Key is per-connection so a second key for a provider is additive later. - api.create_app registers the LLM secret:<name> resolver against the SecretsStore. SDK (openhands-sdk): - LLM._get_api_key_value resolves a secret:<name> api_key via an injectable resolver (register_llm_secret_resolver); raw keys pass through unchanged, and an unset resolver degrades gracefully (None), so standalone SDK use is unaffected. This realizes secret-by-name at runtime with a minimal, testable change (rotation = one store write, every profile picks it up). Tests: 20 new tests covering CRUD, masking, validate (incl. injected validator), connection limit, persistence roundtrip/schema guard, and LLM secret-ref resolution (with/without resolver, raw-key passthrough). Existing llm/settings/ profiles router tests still pass (no regressions). Refs OpenHands/OpenHands#15492, Linear OSS-5295. Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
Coverage Report •
|
||||||||||||||||||||||||||||||||||||||||||||||||||
…connect/rotate
- validate now returns a `verified` flag and only claims a key is authenticated
when a live provider probe ran (opt-in via ?live=true / OH_CONNECTIONS_LIVE_VALIDATE);
catalog-only responses are explicitly verified=false so the UI/docs stop
overstating validation
- add POST /connections/{id}/profiles to create an LLM profile bound to a
connection's key by reference (secret:<name>), wiring the 'pick from every
model the provider offers in Agent Profile' half of #15492
- DELETE now returns the profiles that referenced the connection so clients can
warn instead of silently breaking auth
- rotate the named secret inside the connections lock, after confirming the
record still exists, to avoid orphaned rotated keys under concurrent delete
- validate persists the returned catalog onto the connection's models
- tidy: top-level imports for PersistedConnections/get_llm_profile_store,
document the process-global secret resolver coupling
Co-authored-by: openhands <openhands@all-hands.dev>
Replace the auto-catalog connection design with a provider-first model: providers hold one key (named secret) and a nested, user-managed model list. - persistence: ModelProvider/ProviderModel/PersistedProviders + FileProvidersStore - llm_providers.py: /api/llm/model-providers CRUD + nested model CRUD + optional key-probe test (never mutates the curated model list) - Remove the core-LLM global secret resolver, the backfill-on-GET that rewrote user LLM profiles, and validate() clobbering the model list - Keep named-secret key storage (secret_name + api_key_set; key never returned) Co-authored-by: openhands <openhands@all-hands.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backend: Model providers (provider-first)
Foundation for the provider-centric "connect a provider once, then manage its models under it" feature from the wireframes in OpenHands/OpenHands#15492. Blocks the other PRs.
Data model (single source of truth)
providers.jsondocument viaFileProvidersStore, reusing the existing atomic-write + file-lock primitives (same as workspaces/secrets).SecretsStore); the provider record holds onlysecret_name. Responses exposeapi_key_setand never the value or the secret name.Endpoints (mounted under
/api/llm)Create writes the secret first and rolls it back if the record write fails; delete removes the named secret; update can rotate the key in one write.
testprobes the stored key and returnssuggested_models(the provider's advertised catalog) purely as an "add model" convenience — it never overwrites the curated list.Running a provider's models
The provider store is authoritative. Composing a runnable
LLMfrom(provider fields + model + resolved key)at conversation-activation time in the agent-server is a documented follow-up; this PR is the store + API foundation.Design: simpler than the first draft
This revision deliberately drops the heavier "auto-catalog connection" path in favor of a smaller, provider-centric store. Compared to the earlier iteration, it removes:
secret:resolver injected into coreopenhands.sdk.llm.LLM(core SDK is now untouched vs.main);secret:<name>references (a read must not mutate saved profiles);validate()overwriting the curated model list with the full provider catalog.and keeps named-secret key storage (
secret_name+api_key_set; key never returned).Testing
tests/agent_server/test_llm_providers.py(11 tests): create, key-stored-as-named-secret, key never echoed, get/404, update + rotate key, update-requires-a-field, delete-removes-provider-and-secret, nested add/edit/remove model, test-probe-never-mutates-models, bad-key, and custom-endpoint-catalog-without-probe.Related PRs
This PR was created by an AI agent (OpenHands) on behalf of the user.