feat(providers): slug-keyed cloud providers + per-workload model routing#1888
Conversation
Replace CloudProviderType enum with AuthStyle (bearer/anthropic/openhuman_jwt/none) on CloudProviderCreds. Add slug + label fields as the primary routing key. Legacy type/default_model fields are tolerated on read via migrate_legacy_fields() called in load.rs. Reserved slugs (cloud/openhuman/ollama/pid) are enforced. Factory rewired to slug-keyed grammar: "<slug>:<model>"; old type-prefix branches (openai:, anthropic:, etc.) work as slug lookups if the user has an entry with that slug. Auth lookup tries provider:<slug> then falls back to bare <slug> for legacy auth-profile keys. Add Anthropic AuthStyle to compatible.rs (x-api-key + anthropic-version header).
Add openhuman.providers_list_models RPC method that queries a configured cloud provider's /models endpoint and returns the model list. Handles Bearer/Anthropic auth styles from the provider entry. Registered via the standard controller schema pattern in providers/schemas.rs.
Completes the slug-based provider refactor on the frontend:
- AIPanel.tsx: drop CloudProviderType/primaryCloudId, use slug+label from
CloudProviderCreds; ProviderRef now { kind:'openhuman'|'cloud'|'local' }
with providerSlug instead of providerId/providerType
- aiSettingsApi.ts: parseProviderString/serializeProviderRef on new grammar,
setCloudProviderKey/clearCloudProviderKey keyed by slug, listProviderModels
RPC facade, AISettings drops primaryCloudId
- config.ts: CloudProviderCreds uses slug+label+auth_style; CloudProviderType
deprecated; ModelSettingsUpdate updated; AuthStyle type added
- rpcMethods.ts: providersListModels added
- AIPanel.test.tsx: fixture updated to new shape (slug/label/auth_style,
openhuman routing kind)
📝 WalkthroughWalkthroughThis PR refactors the AI provider settings system from type/primary-cloud semantics to a slug-keyed model. Cloud providers are now identified by slug, routing uses ":" grammar, and an AuthStyle enum centralizes auth behavior. Changes touch Rust config schema/migration, provider factory, new providers RPC, aiSettings API, AIPanel UI, and tests. ChangesSlug-keyed provider system
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/components/settings/panels/AIPanel.tsx (1)
694-715:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftHook the cloud-model field up to
providers_list_models.This is now only a raw text box, so the panel no longer provides the
/models-backed picker and soft validation described for this PR. Users can't browse provider-advertised models or get the intended unknown-model warning before saving.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/settings/panels/AIPanel.tsx` around lines 694 - 715, When source?.kind !== 'local' replace the plain text input with the providers-backed picker: call providers_list_models for the currently selectedCloud (e.g. in an effect when selectedCloud changes), populate a select with the returned model ids, and keep using the existing model state and setModel handler; if providers_list_models is loading/fails show a fallback empty option and keep the text input as a fallback. Also implement the soft unknown-model validation by checking whether the current model exists in the fetched providers_list_models result (use the same effect or a derived boolean) and expose a warning UI before saving if it’s not found.
🧹 Nitpick comments (1)
src/openhuman/config/schema/cloud_providers.rs (1)
100-104: 💤 Low valueConsider documenting why "pid" is reserved.
The reserved slugs include
"pid"which isn't obviously a sentinel in the routing grammar likecloud,openhuman, orollama. If this is intentional (e.g., to prevent collision with provider-id lookups), a brief comment would help future maintainers understand the rationale.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/config/schema/cloud_providers.rs` around lines 100 - 104, The is_slug_reserved function currently reserves "pid" without explanation; add a short inline comment above or beside is_slug_reserved explaining why "pid" is a sentinel (for example: reserved to avoid collision with provider-id or provider lookup routes), and if relevant note any behavior (e.g., used in provider-id lookups or routing grammar) so future maintainers understand the rationale; reference the is_slug_reserved function and the reserved set: "" | "cloud" | "openhuman" | "ollama" | "pid".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 32-38: The panel currently drops or overwrites provider.auth_style
(CloudProvider) causing auth modes to be lost; update the CloudProvider type to
include auth_style and modify toPanelProvider and toApiSettings to preserve and
round-trip provider.auth_style instead of defaulting everything to 'bearer'—when
converting API -> panel in toPanelProvider copy the incoming provider.auth_style
into the panel object, and when converting back in toApiSettings write back the
original auth_style for each provider (fall back to existing default only if
auth_style is undefined).
- Around line 875-909: The custom-provider toggle currently creates a provider
with the literal slug 'custom' and no endpoint; fix the toggle flow so when slug
=== 'custom' you open a dialog that collects the user-chosen provider slug and
endpoint (instead of calling setKeyDialogFor('custom')), validate those inputs,
then on save create a cloudProvider object using the user-provided slug and
endpoint (or defaultEndpointFor(userSlug) if appropriate) and add it to
draft.cloudProviders; also ensure any routing scrubbing or toggling logic (the
existing removal code that checks ref.providerSlug === slug) uses the actual
chosen slug when removing/creating providers. Apply the same change to the other
toggle sites referenced (the other occurrences around lines ~1125-1130 and
~1298-1310) so custom providers are created with real slugs/endpoints rather
than the literal 'custom'.
- Around line 1114-1135: The onSubmit handler currently updates the draft and
writes credentials (setCloudProviderKey) in the wrong order; change it so you do
not persist secrets or mutate draft until the credential write succeeds:
construct the upserted CloudProvider object locally (use variables slug,
pendingLocalLabel, isLocalRuntime, upserted) but do not call setDraft or
setBusyAction for the final state yet; if !isLocalRuntime and slug !==
'openhuman' await setCloudProviderKey(slug, apiKey) first and only after that
call setDraft({ ...draft, cloudProviders: [...draft.cloudProviders, upserted]
}); for local runtimes (isLocalRuntime) you may immediately setDraft since no
secret is stored, and ensure errors from setCloudProviderKey are caught and
handled (clear busy state and do not mutate draft on failure). Also keep
maskedKey generation (maskKeyLabel(true)) as part of upserted but only add
upserted to draft after successful credential persistence.
In `@src/openhuman/providers/schemas.rs`:
- Around line 117-144: Replace the call to build_runtime_proxy_client with
build_runtime_proxy_client_with_timeouts when creating the HTTP client
(currently assigned to client) so the request uses explicit timeouts (30s
request, 10s connect) like other providers; update the instantiation where
client =
crate::openhuman::config::build_runtime_proxy_client("providers.list_models") to
call build_runtime_proxy_client_with_timeouts with the same identifier string
and appropriate timeout args, leaving the subsequent request/header logic
(request, AuthStyle handling, and request.send().await) unchanged.
---
Outside diff comments:
In `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 694-715: When source?.kind !== 'local' replace the plain text
input with the providers-backed picker: call providers_list_models for the
currently selectedCloud (e.g. in an effect when selectedCloud changes), populate
a select with the returned model ids, and keep using the existing model state
and setModel handler; if providers_list_models is loading/fails show a fallback
empty option and keep the text input as a fallback. Also implement the soft
unknown-model validation by checking whether the current model exists in the
fetched providers_list_models result (use the same effect or a derived boolean)
and expose a warning UI before saving if it’s not found.
---
Nitpick comments:
In `@src/openhuman/config/schema/cloud_providers.rs`:
- Around line 100-104: The is_slug_reserved function currently reserves "pid"
without explanation; add a short inline comment above or beside is_slug_reserved
explaining why "pid" is a sentinel (for example: reserved to avoid collision
with provider-id or provider lookup routes), and if relevant note any behavior
(e.g., used in provider-id lookups or routing grammar) so future maintainers
understand the rationale; reference the is_slug_reserved function and the
reserved set: "" | "cloud" | "openhuman" | "ollama" | "pid".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99d693c2-cb58-4fee-85dc-6290cad937a6
📒 Files selected for processing (16)
app/src/components/settings/panels/AIPanel.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsxapp/src/services/api/aiSettingsApi.tsapp/src/services/rpcMethods.tsapp/src/utils/tauriCommands/config.tssrc/core/all.rssrc/openhuman/config/schema/cloud_providers.rssrc/openhuman/config/schema/load.rssrc/openhuman/config/schema/mod.rssrc/openhuman/config/schemas.rssrc/openhuman/migrations/unify_ai_provider_settings.rssrc/openhuman/migrations/unify_ai_provider_settings_tests.rssrc/openhuman/providers/compatible.rssrc/openhuman/providers/factory.rssrc/openhuman/providers/mod.rssrc/openhuman/providers/schemas.rs
The drift guard hardcoded namespace detection to config|screen_intelligence, so the new `openhuman.providers_list_models` method sliced incorrectly (namespace fell back to 'config', fnName = '_list_models') and failed the schema-source lookup. Include providers/schemas.rs in the scanned sources and branch the namespace switch on the providers_ prefix.
…fore draft CodeRabbit findings on tinyhumansai#1888: 1. auth_style was dropped in toPanelProvider and rewritten to 'bearer' in toApiSettings, silently breaking Anthropic / openhuman_jwt providers on reload-save. Preserve it through the round-trip with a new authStyle field on the panel-side CloudProvider type, and a small authStyleForSlug helper for derivation when adding. 2. The 'custom' chip toggled into ProviderKeyDialog, which only collects an API key — slug stayed literal 'custom' and endpoint was empty, so only one unusable custom provider could be created. Route custom through the existing CloudProviderEditor (full slug + label + endpoint form) instead. 3. Both onSubmit paths called setDraft BEFORE the credential write. A failed setCloudProviderKey left config referencing a provider with no stored key. Persist the key first; only mutate draft on success. 4. providers_list_models built the HTTP client without explicit timeouts, risking indefinite hangs. Use build_runtime_proxy_client_with_timeouts (30s/10s) matching the composio + multimodal patterns.
Add unit tests for aiSettingsApi.ts (parseProviderString, serializeProviderRef, loadAISettings, saveAISettings, setCloudProviderKey, clearCloudProviderKey, listProviderModels) and extend AIPanel.test.tsx with focused panel tests covering auth_style preservation, chip-toggle ON opens correct dialog, chip-toggle OFF scrubs routing entries, and failed setCloudProviderKey does not mutate draft.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/components/settings/panels/__tests__/AIPanel.test.tsx (1)
154-158: ⚡ Quick winAvoid index-based “Default” button selection in this interaction test.
getAllByText('Default')[0]is order-coupled and can become flaky if row order changes. Scope the click to the Reasoning row/container and then query its Default button.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/settings/panels/__tests__/AIPanel.test.tsx` around lines 154 - 158, The test currently picks the first "Default" button via getAllByText('Default')[0], which is order-dependent and flaky; instead locate the Reasoning row container (e.g., find the element with text 'Reasoning' or a labeled container for that row), use testing-library's within(ReasoningRow) to scope the query, then query that scoped container for its "Default" button and call fireEvent.click on that element (replace the defaultButtons usage with a scoped within lookup so only the Reasoning row's Default button is clicked).app/src/services/__tests__/rpcMethods.test.ts (1)
56-84: ⚡ Quick winStrengthen the drift guard to verify namespace/function as a pair.
Right now the test can pass if
namespace: "providers"exists anywhere andfunction: "<name>"exists in a different namespace block. Since sources are concatenated, this can hide real drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/services/__tests__/rpcMethods.test.ts` around lines 56 - 84, The test currently checks namespace and function existence separately which can yield false positives when they appear in different schema blocks; update the assertions in the 'catalog canonical methods exist in core schema registry (drift guard)' test to verify the namespace/function pair exists together by searching for the function inside the same namespace block (e.g., replace the two expect(schemaSources).toContain checks with a single check that matches "namespace: \"<namespace>\"" followed later (but before the next namespace) by "function: \"<fnName>\"" — implement this using a regex like new RegExp(`namespace:\\s*"${namespace}"[\\s\\S]*?function:\\s*"${fnName}"`) or by locating the namespace index in schemaSources and asserting the function appears before the next namespace; keep references to CORE_RPC_METHODS, methodRoot, namespace, and fnName when modifying the loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/src/components/settings/panels/__tests__/AIPanel.test.tsx`:
- Around line 154-158: The test currently picks the first "Default" button via
getAllByText('Default')[0], which is order-dependent and flaky; instead locate
the Reasoning row container (e.g., find the element with text 'Reasoning' or a
labeled container for that row), use testing-library's within(ReasoningRow) to
scope the query, then query that scoped container for its "Default" button and
call fireEvent.click on that element (replace the defaultButtons usage with a
scoped within lookup so only the Reasoning row's Default button is clicked).
In `@app/src/services/__tests__/rpcMethods.test.ts`:
- Around line 56-84: The test currently checks namespace and function existence
separately which can yield false positives when they appear in different schema
blocks; update the assertions in the 'catalog canonical methods exist in core
schema registry (drift guard)' test to verify the namespace/function pair exists
together by searching for the function inside the same namespace block (e.g.,
replace the two expect(schemaSources).toContain checks with a single check that
matches "namespace: \"<namespace>\"" followed later (but before the next
namespace) by "function: \"<fnName>\"" — implement this using a regex like new
RegExp(`namespace:\\s*"${namespace}"[\\s\\S]*?function:\\s*"${fnName}"`) or by
locating the namespace index in schemaSources and asserting the function appears
before the next namespace; keep references to CORE_RPC_METHODS, methodRoot,
namespace, and fnName when modifying the loop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e03d3f4-0fb2-4264-bf32-4e34cd25a14e
📒 Files selected for processing (5)
app/src/components/settings/panels/AIPanel.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsxapp/src/services/__tests__/rpcMethods.test.tsapp/src/services/api/__tests__/aiSettingsApi.test.tssrc/openhuman/providers/schemas.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/openhuman/providers/schemas.rs
- app/src/components/settings/panels/AIPanel.tsx
Summary
CloudProviderTypeenum — third parties likedeepseek,groq,mistral, etc. are first-class without core changes.<slug>:<model>(e.g.openai:gpt-4.1,deepseek:v3); the legacycloudsentinel and bare-prefix grammar are migrated on load.openhuman.providers_list_models { provider_id }calls each provider's/modelsendpoint so the AI Settings panel can populate a model picker and soft-validate typed model ids.PROVIDER_CATALOG(labels, endpoints, suggested auth style). Adding a new known provider is now a TypeScript-only change.default_modelis dropped fromCloudProviderCreds(every workload row names its own model).primaryCloudIdis removed — there is no inheritance to resolve.Problem
The previous design had two closed-world bottlenecks:
CloudProviderTypeenum (openhuman | openai | anthropic | openrouter | custom) — anything that wasn't openai/anthropic/openrouter had to share the singlecustom:slot, so a user couldn't have both DeepSeek and Groq configured.default_modelper provider — routing inherited a single model per provider, so per-workload variation ("reasoning uses gpt-4.1, coding uses gpt-4o-mini") wasn't expressible without per-workload override strings that the panel didn't surface well.Both made it impossible to express the user-facing scenario: "reasoning = openai:gpt-4.1, coding = openai:gpt-4o-mini, summarization = deepseek:v3".
Solution
Schema (
src/openhuman/config/schema/cloud_providers.rs):CloudProviderTypereplaced with a smallAuthStyleenum (bearer | anthropic | openhuman-jwt | none) — the core only needs to know how to attach the credential.CloudProviderCredsis now{ id, slug, label, endpoint, auth_style }.default_modelremoved. Legacytype+default_modelfields are tolerated on read (skip_serializing) so old configs deserialize cleanly.migrate_legacy_fields()derives slug/label/endpoint/auth_style from any legacytypevalue at load time. Reserved slugs (cloud,openhuman,ollama,pid) are rejected.Factory (
src/openhuman/providers/factory.rs):openhuman|ollama:<model>|<slug>:<model>. Thecloudsentinel and per-type prefix branches are gone.lookup_key_for_slug(slug)which triesprovider:<slug>first, then falls back to bare<slug>(so existingauth-profiles.jsonentries keep working). New writes always useprovider:<slug>.RPC (
src/openhuman/providers/schemas.rs):openhuman.providers_list_models { provider_id }→{ models: [{ id, owned_by?, context_window? }] }. Calls<endpoint>/modelswith the stored credential. OpenAI-compatible and Anthropic response shapes both supported.Frontend (
app/src/components/settings/panels/AIPanel.tsx):PROVIDER_CATALOG(openhuman, openai, anthropic, openrouter, deepseek, groq, mistral, together, fireworks). "Add provider" is catalog-pick OR custom (fill in slug/label/endpoint/auth_style by hand). Slug validation rejects duplicates and reserved values.providers_list_models. Unknown typed models get an amber hint, but save isn't blocked.primaryCloudIdremoved end-to-end — every workload row names its own slug + model.Lazy migration runs on
Config::load: legacy bare-prefix routes (\"openai\",\"cloud\", etc.) rewrite themselves to\"<slug>:<old default_model>\"so existing users see no behavior change on first launch.Submission Checklist
Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
Validation Blocked
Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Changes
Tests