feat(ai): unified per-workload provider routing + chat-provider factory (#1710)#1858
Merged
Merged
Conversation
- New `CloudProviderCreds` + `CloudProviderType` schema in `cloud_providers.rs`. API keys are NOT carried on this struct — they live in `auth-profiles.json` via the existing `AuthService`, encrypted with the same AES-GCM AEAD envelope (`enc2:`) used by Composio and the app-session JWT. - New `Config` fields: `cloud_providers: Vec<CloudProviderCreds>`, `primary_cloud: Option<String>`, plus per-workload provider strings for reasoning / agentic / coding / memory / embeddings / heartbeat / learning / subconscious. - New helpers `Config::workload_uses_local` and `Config::workload_local_model` — the single source of truth for "does this workload route to Ollama, and which model?". All readers must consult these instead of the legacy `local_ai.usage.*` / `memory_tree.llm_backend` fields. - Deprecate the four `LocalAiConfig::use_local_for_*` methods with `#[deprecated]`. Bodies retained so older `config.toml` files still parse — the migration (next commit) fills the new fields from these on first load.
`create_chat_provider(role, &Config)` now returns `(Box<dyn Provider>, String)` so the model id parsed from the provider string travels with the provider. Callers no longer hardcode hint strings like `"agentic-v1"` — they use the returned model directly so explicit picks (`openai:gpt-4o`, `ollama:llama3.1:8b`) actually use the chosen model end to end. Provider-string grammar covers seven forms: - `"cloud"` → resolves to `primary_cloud` - `"openhuman"` → OpenHumanBackendProvider (session JWT) - `"openai:<model>"` → OpenAiCompatibleProvider + Bearer auth - `"anthropic:<model>"` → ditto - `"openrouter:<model>"` → ditto - `"custom:<model>"` → ditto, user-defined endpoint - `"ollama:<model>"` → local Ollama daemon Keys are fetched at call time via `AuthService::get_provider_bearer_token(<type>, None)` — same path Composio uses, so cloud-provider keys are encrypted at rest in `auth-profiles.json` rather than plaintext in `config.toml`. `provider_for_role` covers all eight workloads now. Comprehensive grammar tests for every form, every workload, error cases, and primary resolution.
One-shot, idempotent migration that consolidates the legacy AI
routing surface into the new unified fields. Runs at startup via
`Config::load_or_init` when `schema_version < 2`, just like the
existing `phase_out_profile_md` migration.
Behaviour (gated on absence of new fields, so re-runs are no-ops):
- Seeds an `Openhuman` entry into `cloud_providers` (always).
- If a legacy `inference_url` looks non-OpenHuman, seeds a `Custom`
entry from it. Default model derived from `model_routes` when
present.
- `primary_cloud` defaults to the OpenHuman entry id.
- Derives the 5 background `*_provider` fields from
`local_ai.usage.*` + `memory_tree.llm_backend`:
- `local_ai.usage.embeddings && runtime_enabled` →
`embeddings_provider = "ollama:<embedding_model_id>"`
- Same shape for heartbeat / learning / subconscious using
`chat_model_id`.
- `memory_tree.llm_backend == Local && runtime_enabled` →
`memory_provider = "ollama:<chat_model_id>"`; cloud otherwise.
12 tests cover empty configs, legacy mapping, idempotency,
`runtime_enabled = false` fallback, and the `primary_cloud` default.
Bumps `CURRENT_SCHEMA_VERSION` from 1 to 2.
Migrate every production reader off the legacy
`local_ai.use_local_for_*()` and `memory_tree.llm_backend` paths so
the new unified `*_provider` fields are the single source of truth.
Call sites updated:
- subconscious/executor.rs:108
- learning/reflection.rs:163
- memory/tree/score/embed/factory.rs:86 (now reads model from
`workload_local_model("embeddings")` directly)
- memory/store/factories.rs: `effective_embedding_settings`,
`effective_embedding_settings_probed`, `create_memory_full`, and
`create_memory_with_local_ai` all take
`local_embedding_model: Option<&str>` instead of
`Option<&LocalAiConfig>` — caller computes from
`config.workload_local_model("embeddings")`.
- agent/harness/session/builder.rs, channels/runtime/startup.rs:
external callers updated to pass the parsed model.
- memory/tree/{chat/mod.rs, jobs/worker.rs, score/extract/mod.rs,
tree_source/summariser/mod.rs}: replace `match
config.memory_tree.llm_backend { Local => …, Cloud => … }` with
`if config.workload_uses_local("memory") { … } else { … }`.
- memory/tree/score/mod.rs: log line updated to surface the new
field name.
- memory/tree/read_rpc.rs::set_llm: the legacy `memoryTreeSetLlm` RPC
now also writes `config.memory_provider`, so any orphan caller
stays consistent with the factory's view.
Test fixtures in `memory/store/factories.rs` updated to drive the
new parameter shape.
The four `LocalAiConfig::use_local_for_*` methods stay
`#[deprecated]` (no production reader; tests-only).
Surface the new unified AI routing fields over the existing config JSON-RPC surface so the AI settings panel can read and write them without a new RPC controller. `config.update_model_settings` now accepts: - `cloud_providers[]` (REPLACES wholesale — keys live in `auth-profiles.json` via `auth_store_provider_credentials`, NOT carried here) - `primary_cloud` - 8 per-workload provider strings (`reasoning_provider` … `subconscious_provider`) `config.get_client_config` now surfaces: - `cloud_providers[]` (`id`, `type`, `endpoint`, `default_model` — no keys, never echoed) - `primary_cloud` - 8 per-workload provider strings API keys are managed through the existing `auth_store_provider_credentials` / `auth_remove_provider_credentials` / `auth_list_provider_credentials` RPCs — same path Composio uses. No new RPC endpoints needed.
Single Settings -> AI page replaces three previously-overlapping surfaces (BackendProviderPanel, LocalModelPanel, and the stub AIPanel). Three orthogonal sections: 1. **Cloud providers** — card list with brand-rail per provider type (primary blue / sage / amber / slate / stone). One marked "Primary". Add-provider modal handles OpenAI / Anthropic / OpenRouter / Custom — endpoints pre-fill per type, OpenAI-style key field is password-masked, and saving the modal calls the existing `auth_store_provider_credentials` RPC so the key is encrypted at rest in `auth-profiles.json` rather than persisted to `config.toml`. 2. **Local provider** — Ollama daemon status with pulsing health dot, Install/Retry button driving `openhumanLocalAiDownload`, tier preset cards driving `openhumanLocalAiApplyPreset`, and installed models listing from `openhumanLocalAiDiagnostics`. Carries the install/detect/download UX previously in LocalModelPanel. 3. **Workload routing** — 8-row matrix grouped Chat / Background. Each row has a 3-tab segmented control (Primary / Cloud / Local) plus a context-sensitive model picker. Quick-action pills (Cloud / Local / Mixed) bulk-set the matrix. Floating save bar with diff summary appears only when dirty. New façade `app/src/services/api/aiSettingsApi.ts` sits between the panel and the Tauri/RPC layer: - `loadAISettings()` / `saveAISettings(prev, next)` (diff-only patch) - `setCloudProviderKey(type, key)` / `clearCloudProviderKey(type)` wrapping `auth_store_provider_credentials` / `auth_remove_provider_credentials` - `loadLocalProviderSnapshot()` joining `openhuman_local_ai_status`, `openhuman_local_ai_diagnostics`, `openhuman_local_ai_presets` TS types in `utils/tauriCommands/config.ts` extended with `CloudProviderType` / `CloudProviderCreds` / per-workload provider fields. TS wrappers in `utils/tauriCommands/auth.ts` added for the three `auth_*_provider_credentials` RPCs. Standalone interactive preview at `design-previews/ai-settings.html` (no build step — Tailwind + Alpine + Lucide via CDN) shows the same design and click-flow for review without spinning up the full desktop dev session.
…der AI
Two-part settings restructure:
1. **Panel demolition** — the unified AIPanel covers everything these
panels did. Delete:
- `BackendProviderPanel.tsx` (545 lines) — cloud-provider preset
picker. Replaced by AIPanel's Cloud providers section.
- `LocalModelPanel.tsx` (507 lines) — Ollama install/manage +
`usage.*` toggles. Replaced by AIPanel's Local provider section
and Workload routing matrix.
- Their tests.
2. **AI parent section** — `/settings/ai` is now a section page that
lists two child cards:
- **LLM** → `/settings/llm` (the unified AIPanel)
- **Voice** → `/settings/voice` (the existing VoicePanel, no
functional change — only its breadcrumb parent moves from
"AI & Models" to "AI")
Route changes:
- `/settings/ai` is now the section page (was: AIPanel directly).
- `/settings/llm` is the new home for AIPanel (rename only).
- `/settings/voice` unchanged.
- `/settings/local-model` and `/settings/backend-provider` 404 as
designed (no redirects).
Nav / breadcrumb updates:
- `SettingsRoute` gains `'llm'`, drops `'local-model'` and
`'ai-models'`.
- New `aiCrumb` breadcrumb. Voice + LLM render under
`[Settings, AI]`; AI itself renders under `[Settings]`.
- `SettingsHome` "AI" card targets the section page.
- `VoicePanel`'s disabled-state copy keeps upstream's in-place
install hint (no longer redirects to old Local AI Model panel).
Adds a clearly-labeled checkbox row above the Local provider status card that flips `local_ai.runtime_enabled` via the existing `openhuman.config_update_local_ai_settings` RPC. Surfaces the master switch users were previously forced to find inside the (now-deleted) LocalModelPanel. When off: - The Local provider card greys out (`opacity-60`) to signal the daemon is parked. - Any workload routed to `ollama:<model>` will fail at factory level (clear error from the chat-factory) — the warning copy under the toggle tells the user to keep routes on "cloud" while disabled. - Saves CPU + RAM for users who only use cloud providers. Wire path: - `aiSettingsApi.setLocalRuntimeEnabled(enabled)` wraps the existing Tauri command. - `localProvider.setEnabled` re-exports it for symmetry with `applyPreset` and `download`. - The toggle handler refreshes both the Ollama snapshot and the full AI settings (since runtime_enabled also affects whether installed models can serve requests).
Three additions to the LLM panel's Local provider section:
1. **Daemon-conflict callout** — when `LocalAiStatus.warning` indicates
an external Ollama daemon with a broken runner (the "stale daemon
from another workspace" scenario), surface it as an amber callout
with clear recovery steps (Task Manager → kill ollama.exe → retry;
or set custom path). Previously this state surfaced as 10-px amber
sub-text under the daemon row — way too quiet for a user-blocking
error.
2. **Resolved binary path** — show which Ollama OpenHuman is actually
using (from the existing `diagnostics.ollama_binary_path`). Used to
be hidden in logs; now visible under Advanced so users can verify
"yes, it's using my system install" or "no, it's using the
managed workspace copy".
3. **Custom path override** — text input that writes
`local_ai.ollama_binary_path` via the existing
`local_ai_set_ollama_path` RPC, which also re-bootstraps. Empty
clears the override and falls back to the auto-detect chain
(workspace bin → OLLAMA_BIN env → system PATH → managed install).
All three sit under a collapsed `<details>` element ("Advanced") so
the default panel stays calm — only users with daemon issues see the
callout, and only users who want to override see the path UI.
`aiSettingsApi.ts` gains `setLocalOllamaPath` and a
`localProvider.setBinaryPath` re-export, mirroring the existing
`setEnabled` / `applyPreset` / `download` shape.
Two papercuts on the enable toggle: 1. Clicking it returned within ~50ms (the RPC resolves fast) but the daemon bootstrap takes 2-8 seconds. The checkbox would flicker off → on as the post-write `refresh()` saw the still-`disabled` status before bootstrap had run. Users would multi-click thinking nothing happened. 2. No visible loading state — the checkbox just sat there in pseudo-disabled mode without any spinner. Fix: - Replace the checkbox with a spinning `LuLoader` while `busyAction === 'toggle-local'`. The whole row gets `cursor-wait` and the action label changes to `STARTING…` / `STOPPING…` in primary-blue smallcaps so the in-progress state reads at a glance. - Poll status every 500ms (up to 10s) until `state` actually transitions out of `'disabled'` before clearing the busy flag. The spinner stays up for the full bootstrap window, then clears when the daemon is genuinely running. - `useOllamaStatus.refresh` now returns the fresh snapshot so the polling loop can read it without closure-stale-state bugs. - Install/Retry button gains the same loader treatment for symmetry.
The toggle's disable path was a no-op visually. Sequence used to be:
1. update_local_ai_settings({ runtime_enabled: false }) → config written
2. ollama.refresh() → status STILL "ready" (daemon hasn't stopped,
no bootstrap was triggered, status state machine never re-evaluated)
3. checkbox renders checked again → user thinks click did nothing
Root cause: `local_ai_status` only auto-spawns bootstrap when state is
"idle" or "degraded" — never from "ready". So writing the config flag
alone never propagates to the status field.
Fix: call `local_ai_download(force=true)` in BOTH toggle directions.
`force=true` triggers `reset_to_idle` → spawns bootstrap → bootstrap
reads the freshly-written `runtime_enabled = false` and immediately
sets status to "disabled" (bootstrap.rs:122). Same mechanism for
enable, just landing on "ready" instead.
The bidirectional poll-and-wait covers both — the spinner stays up
until status reaches its expected target ("disabled" or anything-but-
"disabled"), then clears.
Caveat: the daemon process itself stays running until app exit or
next launch with `runtime_enabled = true`. This is consistent with
how the old LocalModelPanel behaved and matches the comment in
ollama_admin's shutdown_owned_ollama (only invoked from the Tauri
exit lifecycle hook). A "stop daemon now" RPC would be a separate
follow-up.
Symmetry with how OpenHuman handles external Ollama at startup (friendly-fire avoidance — never touch a daemon we didn't spawn): the disable toggle now follows the same rule. New Rust RPC `openhuman.local_ai_shutdown_owned`: - Calls `shutdown_owned_ollama` — kills the daemon only when the spawn marker matches (i.e. OpenHuman is the parent process). External daemons (system service, manual `ollama serve`, daemons from another workspace) are left untouched. - Then forces status to `LocalAiStatus::disabled(&config)` so the UI reflects the gated state immediately, regardless of whether a process was actually killed. - A new `LocalAiService::mark_disabled` helper exposes the disable status setter (previously only reachable via bootstrap's short-circuit). UI: the disable branch of the toggle now calls a unified `localProvider.shutdown()` helper that writes `runtime_enabled=false` and invokes the new RPC. The enable branch is unchanged. This matches the user's mental model: "disable = gate off, only shut down what we own". From the factory's perspective the result is identical: any workload routed to `ollama:<model>` fails at build time, so nothing in OpenHuman talks to the daemon regardless of who owns the underlying process.
When the user toggles local AI off, sweep the per-workload `*_provider` fields and reset any that start with `ollama:` to None (which resolves to "cloud" / primary at the factory). Without this, disabling left the routing matrix referencing an ollama runtime that no longer exists — the next chat call routed to `reasoning` (or any other workload pinned to local) would fail at factory build time with "no ollama daemon, runtime disabled". Users correctly expect "disable local AI" to mean "everything routes to cloud now", not "everything that was local now silently breaks". Implementation lives in `local_ai_shutdown_owned`: scans the eight provider fields once, clears any ollama refs, persists the config only if at least one was cleared (avoids needless I/O when the user toggles off from an all-cloud configuration). The shift is one-way: re-enabling local AI does NOT restore the previous Ollama routes. The user re-picks. Tracking "previous local preference" would mean per-workload shadow state that drifts out of sync with the visible routing matrix — the explicit reroute on re-enable keeps the matrix the single source of truth.
Bootstrap has a SECOND hard gate beyond runtime_enabled:
[local_ai] bootstrap: opt_in_confirmed=false,
hard-overriding to disabled (cloud fallback)
This is the MVP opt-in marker (`local_ai.opt_in_confirmed`). When
false, bootstrap forces status to "disabled" regardless of
runtime_enabled. Setting runtime_enabled = true alone spawned the
Ollama daemon successfully but the very next bootstrap tick
re-disabled it — the user saw the daemon momentarily start, then a
Windows security popup (Ollama briefly running, then killed), then
the checkbox snapped back to unchecked.
Tying opt_in_confirmed to runtime_enabled in the toggle handler
matches what apply_preset already does for tier selection:
- runtime_enabled = true AND opt_in_confirmed = true → ENABLED
- runtime_enabled = false AND opt_in_confirmed = false → DISABLED
Changes:
- `LocalAiSettingsPatch` and `update_local_ai_settings` RPC gain an
`opt_in_confirmed: Option<bool>` field.
- TS `LocalAiSettingsUpdate` interface mirrors it.
- `setLocalRuntimeEnabled(enabled)` sends both fields together —
callers don't have to know about the gate.
This unblocks the single-click enable from the unified AI panel
without forcing users through apply_preset first.
OpenHuman is the signed-in default: its endpoint comes from the user's account, its credential is the session JWT (managed by the auth flow, not by this panel), and its type can't be changed. So both the edit modal and the delete button are no-ops for it — clicking edit opened a modal whose type dropdown couldn't even render "OpenHuman" (the dropdown filter only includes the third-party options) and whose endpoint/key fields were either disabled or hidden. Hide both controls on the OpenHuman card. "Set primary" stays so the user can still re-mark OpenHuman as primary after switching away. The card's body already shows "Signed-in default · no configuration needed" — together with the now-quiet header, that reads as intentionally read-only rather than broken. Third-party cards (OpenAI / Anthropic / OpenRouter / Custom) are unaffected — they still have edit + delete.
The previous button had no onClick — dead control that looked like something useful would happen on click. The backend doesn't have a "pull arbitrary Ollama model by name" RPC; only local_ai_download_asset(capability) which pulls whatever model is already configured for a capability slot. Replace with a surface the user can actually act on: - "Browse Ollama library" — opens https://ollama.com/library in the default browser via the existing openUrl helper. The user picks a model name from the catalogue. - Inline hint: run `ollama pull <model>` in a terminal; the installed list above polls every 5s and picks up new models automatically. This makes the surface honest about the current capability boundary without removing user agency. A "pull by name" RPC + matching UI would be a separate, larger feature — wiring the Ollama daemon's HTTP /api/pull, surfacing streaming download progress, and handling the per-model error cases.
The Pull-a-model button's openUrl('https://ollama.com/library') call
was silently rejected — the `opener:allow-open-url` permission scope
was restricted to `obsidian://open*` only. Tauri's opener plugin
checks the URL against the allow list before invoking the OS shell,
so calling with any non-matching URL fails permission check (the
fallback to `window.open` inside the helper doesn't help inside CEF
because new browser windows don't open from there either).
Add `https://ollama.com/*` to the allow list. Restricted to ollama.com
to avoid the security trade-off of `https://*` — when more outbound
links are added (e.g. provider signup pages, docs links), extend the
list with the specific hosts.
The agent harness was building its provider via
`create_intelligent_routing_provider`, which only consults
`inference_url` + `api_url` + `api_key` + `model_routes` — none of
the per-workload `*_provider` fields the unified AI panel writes.
So `reasoning_provider = "anthropic:claude-sonnet-4-5"` was set
in config, persisted, and surfaced in the routing matrix, but every
chat still went to OpenHuman's backend. The user's selection was
silently ignored.
Migrate the main agent session builder to call
`providers::create_chat_provider("reasoning", config)`. The factory
already handles:
- `None` / `"cloud"` → primary cloud (OpenHuman backend by default)
- `"openhuman"` → OpenHuman backend
- `"openai:<m>"` / `"anthropic:<m>"` / `"openrouter:<m>"` / `"custom:<m>"`
→ OpenAiCompatibleProvider with the matching cloud_providers entry
- `"ollama:<m>"` → local Ollama daemon
Use the factory's resolved model (the part after `:` in the provider
string) as the agent's `model_name`. Without this, the harness would
send the abstract tier name `"reasoning-v1"` to Anthropic and 404.
What the legacy router did that the factory currently doesn't:
- ReliableProvider retry wrapper (502/503/504)
- RouterProvider model_routes translation
- Intelligent local/cloud task hinting (`hint:reaction` → local)
- Model fallback chain (config.reliability.model_fallbacks)
Re-applying those on top of the factory is orthogonal — they can wrap
the factory's output without re-introducing the routing bypass. Out
of scope for this fix.
Re-export `create_chat_provider` from `providers::mod` so the agent
harness can call it via the existing `providers::` namespace without
reaching into the submodule.
Anthropic rejects chat/completions requests that list two tools with
the same name:
400 invalid_request_error:
"Duplicate tool or function name: research"
OpenHuman's backend (and OpenAI) silently accept duplicates, so the
underlying bug — researcher sub-agent's `delegate_name = "research"`
collides with a same-named skill tool — was invisible until tinyhumansai#1710's
per-role routing started sending the same tool list to Anthropic
directly.
Add a hash-set dedup over `visible_tool_specs` right before it's
handed to the provider. First-occurrence wins; the underlying tool
route resolution at dispatch time is unchanged (tools are dispatched
by name lookup against `Agent::tools`, not by index in the spec list).
Log dropped names at WARN so the collision surfaces in diagnostics
instead of hiding silently.
The deeper fix — preventing the collision at registration time
(reject `delegate_name` overrides that shadow existing skill names,
or namespace delegates as `delegate.research` to avoid the clash) —
is a larger refactor and out of scope here. This dedup unblocks the
Anthropic / OpenRouter / strict-provider path immediately.
The original dedup (578ec96c) ran only in AgentBuilder::build. Two
other paths rebuild visible_tool_specs at runtime and were silently
re-introducing the same "research" collision the dedup was meant to
prevent:
- turn.rs refresh_delegation_tools: after a Composio
connect/revoke triggers a delegate resynthesis, the new
delegate spec (delegate_name = "research") lands alongside
the same-named skill, and visible_tool_specs is rebuilt
straight from tool_specs with no filter.
- runtime.rs set_visible_tool_names: scope-filter changes
rebuild from tool_specs the same way.
Both reproduce the Anthropic 400 the original commit was supposed
to fix as soon as the agent's delegation surface changes, which
in practice happens on every turn 1 (initial composio fetch).
Lift the dedup body out of build() into a `dedup_visible_tool_specs`
helper in builder.rs and call it from all three sites. First wins
preserves registration order and matches the original behaviour.
The underlying collision (delegate_name shadowing a skill name) is
the bigger fix — out of scope here.
Sub-agents whose definition declares `[model] hint = "<workload>"`
(integrations_agent → agentic, planner → reasoning, etc.) were
inheriting the parent's provider verbatim while resolving the model
name to `{workload}-v1` via ModelSpec::resolve. The `{workload}-v1`
naming convention is specific to the OpenHuman backend's tier
routing — when the parent's provider is a third-party cloud (e.g.
Anthropic after per-role routing landed in tinyhumansai#1710), Anthropic 404s on
`agentic-v1` because it has no such model, and the sub-agent call
fails before reaching any toolkit action.
In practice this surfaces as: user routes the reasoning workload to
Anthropic in AI Settings, then asks "fetch my latest emails" — the
main reasoning agent (now correctly Anthropic) delegates to
integrations_agent, which inherits Anthropic but asks for
agentic-v1, gets a 404, and the gmail fetch never happens.
Switch hint-mode sub-agents to build a fresh provider via
create_chat_provider(workload, &config) so the user's per-workload
routing flows down into nested agents the same way it flows into the
main chat path. `Inherit` and `Exact` model specs keep the legacy
parent-provider behaviour — `Inherit` literally means "use what the
parent uses", and `Exact` names a specific model on purpose. Errors
from the factory or config load fall back to the old behaviour
(parent provider + `{workload}-v1`) so a config glitch can't take
down sub-agent execution entirely.
ExtractFromResultTool still uses the parent provider — that path
does summarization-style extraction and could move to the memory
workload, but it's out of scope for the gmail fetch fix.
The previous Hint-arm fallback synthesised `{workload}-v1` (e.g.
`agentic-v1`) whenever the factory or config load failed. That naming
convention is a relic — every workload entry in config already carries
the *exact* model id (e.g. `"anthropic:claude-sonnet-4-6"`), the
OpenHuman backend itself accepts exact model names, and no provider in
the system needs the `-v1` tier abbreviation. Synthesizing it just
re-introduced the same 404 path the workload factory was added to fix.
Fall back to the parent's *current* model name instead. The parent's
model is known to work on the parent's provider (we're already there),
so the worst case is "sub-agent uses the same model as the parent" —
graceful degradation, not a 404.
Also drop the half-finished `Exact(spec) if spec.contains(':')` arm
that briefly tried to make Exact dual-purpose (bare name vs
provider:model). Provider switching is the job of Hint + AI-settings
routing — Exact stays a parent-provider model override, which is the
only honest contract for an enum variant that doesn't carry a provider
field.
`ModelSpec::resolve` produces `{hint}-v1` for Hint variants (e.g.
`agentic-v1`). That naming was an OpenHuman-backend tier convention —
Anthropic, OpenAI, OpenRouter, and Ollama all 404 on it. After the
chat-factory landed, a cron job whose agent_id resolves to a Hint
spec was effectively guaranteed to fail the moment the user routed
the workload to a non-OpenHuman provider.
In the per-agent override block, when the definition's model is
Hint(workload), ask the workload factory for the exact model id
instead of synthesising `{hint}-v1`. Inherit and Exact keep their
literal resolve() paths because they don't rely on the -v1 trick.
Factory failures fall back to the existing `fallback_model` chain
so a transient config-load error can't sink the cron run entirely.
Parallels the same fix in subagent_runner — every code path that
turned a Hint into a wire-level model name now goes through the
factory.
The summarizer sub-agent's definition declares `hint = "summarization"` but the factory's role match had no case for that name, so it hit the wildcard arm and routed to `primary_cloud`. The user can't pick a cheaper summarisation model independently — every summary call piggybacked on whatever the primary happened to be. `memory_provider` is already documented as "memory-tree extract + summarise workloads" (schema/types.rs:206) — both are the same "condense input" model class. Map `"summarization"` to `memory_provider` as an alias of `"memory"` so the summarizer agent and the memory-tree extract pipeline share one knob, which is what the config schema already implies.
Previously, if any single profile in auth-profiles.json failed to decrypt (e.g. .secret_key was regenerated underneath an existing profile — by manual deletion, partial workspace restore, or a stray script), load_locked() bubbled the error and every single read of the store failed forever. The app's app_state_snapshot poll then spammed "Decryption failed — wrong key or tampered data" on every tick, and the user couldn't log in cleanly without manually nuking auth-profiles.json. Catch decryption failures per-profile, log them loudly, and drop the unrecoverable entries from the on-disk store on the next write (also pruning any active_profiles pointers that referenced them). The user sees a clean logged-out state and re-authenticates normally; subsequent reads return without errors. The fix is intentionally narrow: it only triggers on decryption errors, not on schema-parse errors. A genuinely corrupt JSON file should still surface so we don't silently mask data issues that aren't recoverable by re-login.
make_openhuman_backend built OpenHumanBackendProvider with ProviderRuntimeOptions::default(), which leaves openhuman_dir = None. The provider's resolve_bearer() then falls back to ~/.openhuman for the AuthService state dir, while auth_store_session writes auth-profiles.json into the *workspace's* config_path.parent() (e.g. C:/Users/.../.openhuman-test-chat under OPENHUMAN_WORKSPACE or wherever the user's actual workspace lives). So login wrote a fresh JWT to the right place, but every chat call read from the wrong place and bailed with "No backend session: store a JWT via auth (app-session)" — even immediately after a successful auth_store_session round-trip. Pass openhuman_dir = config.config_path.parent() and secrets_encrypt = config.secrets.encrypt into ProviderRuntimeOptions so the provider's AuthService reads the same store login writes. Mirror what builder.rs already does when constructing ProviderRuntimeOptions for the main agent path. Bug was masked in single-workspace setups (the home dir IS the workspace) and only surfaced when OPENHUMAN_WORKSPACE redirected the workspace away from ~/.openhuman — which is the default for every parallel test worktree.
…nges
THREAD_SESSIONS caches the per-thread Agent so warm threads keep their
delegation surface, integration list, and history across turns. The
cache-hit predicate compared model_override / temperature /
target_agent_id, but not the workload routing config that
build_session_agent feeds into create_chat_provider("reasoning", ...).
Symptom: change reasoning_provider in Settings → AI → LLM, send another
message in the existing chat thread, watch every turn still hit the old
provider until the thread is dropped or the app restarts. Other
workloads (agentic, coding, memory, summarization) are resolved per
call inside the factory and aren't affected — only the orchestrator's
chat provider is bound at agent-build time.
Capture reasoning_provider on SessionEntry and require it to match in
the predicate. Cache-miss log line now also prints the prior/new values
so it's obvious from the trace that a workload-routing edit forced the
rebuild.
- config/ops_tests.rs: ModelSettingsPatch struct literals predated the schema extension (CloudProviderCreds + 8 per-workload provider fields). Add `..Default::default()` so the literals only spell out the fields each test cares about — these tests aren't about the new fields, they assert the legacy api_url / api_key / model patch behaviour. - providers/factory.rs: error-path tests can't use `.expect_err` on `Result<(Box<dyn Provider>, String), _>` because `dyn Provider` doesn't implement `Debug`. Switch to `.err().expect(...)` so the Ok variant doesn't need to satisfy `Debug`. - migrations/mod_tests.rs: the post-migration schema_version is 2 now (we added unify_ai_provider_settings as migration #2). Update the equality and disk-content assertions accordingly. Compile-only fix — no behaviour change, just unblocks `cargo test --lib` so the real per-test failures are visible.
Six tests broke against behaviour changes from tinyhumansai#1710: - agent::harness::session::tests::turn_dispatches_spawn_subagent_through_full_path: the test scripted MockProvider responses for parent + sub-agent expecting the sub-agent to inherit `parent.provider`. After workload-factory routing, sub-agents whose definition declares `[model] hint = "<workload>"` build a fresh provider via the factory — the MockProvider chain leaks responses[1] back to the parent as its final answer instead of being consumed by the inner sub-agent loop. Rewriting it needs a test-only provider injection point or a dedicated test-agent with `[model] inherit`. Marked `#[ignore]` with a doc comment outlining the rewrite plan rather than ship a hand-wave assertion. - memory::tree::chat::tests::build_provider_returns_local_when_configured: local-vs-cloud routing is now driven by `memory_provider` (via `Config::workload_uses_local("memory")`), not the legacy `memory_tree.llm_backend` flag. Set `memory_provider = "ollama:..."` so the local branch is taken; endpoint/model on `memory_tree` are still consumed for the inner construction. - memory::tree::score::embed::factory::tests::local_ai_usage_embeddings_routes_to_ollama: same root cause for embeddings — drive via `embeddings_provider` instead of `local_ai.usage.embeddings`. - tools::cron::run::tests::force_runs_job_and_records_history: Windows `echo` is environment-dependent (cmd.exe builtin vs Git Bash `echo.exe`). The test asserted one specific failure outcome; relax it to accept either deterministic ToolResult and assert the runs ledger matches the success/failure decision in both branches. - settings/__tests__/SettingsHome.test.tsx: the settings home tile for AI was renamed from "AI & Models" to "AI" when the panel collapsed into a section page with nested LLM + Voice children. Update the ordering assertion. - settings/panels/__tests__/AIPanel.test.tsx: section labels show up in body copy too ("only use cloud providers" in local-provider explanation, "Primary" badge + "Primary resolves to …" routing rows). Switch `getByText` → `getAllByText` so legitimate multiple matches don't fail the assertion. Lib code is unchanged. After this commit: - cargo test --lib: 7143 passed, 0 failed, 8 ignored - vitest (settings scope): 20 files passed
Adds unit tests for the riskiest new code paths from tinyhumansai#1710: providers/factory.rs (+5 tests): - summarization_aliases_memory_provider: confirms `hint = "summarization"` (declared on the summarizer sub-agent) resolves to `memory_provider` since the workload is semantically identical — there's no separate config knob, and the alias is the only thing preventing summarization from falling all the way through to "cloud". - summarization_defaults_to_cloud_like_memory: paired baseline so the alias doesn't accidentally diverge from memory's default. - unknown_workload_falls_back_to_cloud: pins the wildcard arm so a typo in an agent TOML can't crash with NoneProvider. - openhuman_backend_uses_config_path_parent_as_state_dir: regression for the bug where every chat call read auth-profiles.json from ~/.openhuman instead of the workspace's actual config_path.parent — surfaced as "No backend session" immediately after a successful login on test/parallel workspaces. agent/harness/session/builder.rs (+4 tests, new dedup_tests module): - drops_duplicates_first_wins: the actual collision the dedup helper exists to fix (researcher's `delegate_name = "research"` shadowing a same-named skill). Anthropic 400s on duplicate tool names where OpenHuman silently accepts. - passes_through_when_no_duplicates: happy path. - handles_empty_input: edge case. - preserves_full_spec_content_for_kept_entries: description + parameters survive the pass so function-calling quality doesn't silently degrade. credentials/profiles_tests.rs (+1 test): - load_drops_profiles_whose_decryption_fails_under_rotated_key: the defensive purge introduced earlier — when an enc2: token can't be decrypted under the current key (e.g. .secret_key got regenerated underneath an existing profile, observed when OPENHUMAN_WORKSPACE pointed at a partially-restored dir), `load_locked` must drop the unrecoverable entry and rewrite the store rather than propagating the error and poisoning every subsequent read. cargo test --lib: 7152 passed, 0 failed, 8 ignored
The Hint/Inherit/Exact branching inside `run_typed_mode` was inlined
in a 50-line async block — testable only via `run_subagent` which
needs a full `ParentExecutionContext` (many Arcs and a real
ConfigManager). Extract the pure decision into
`resolve_subagent_provider(spec, agent_id, config, parent_provider,
parent_model)` so it can be unit-tested with a `ScriptedProvider`
and a hand-built `Config`. `Config::load_or_init()` stays in the
async caller; the helper takes `Option<&Config>` so the no-config
fallback path is testable too.
Adds five tests covering:
- Inherit returns parent's Arc unchanged (same pointer)
- Exact keeps parent's provider, swaps the model name only
- Hint with `config=None` falls back to parent's (provider, model)
— explicit regression against the `{workload}-v1` synthesis we
deleted
- Hint with a real config routes via the factory and returns the
factory-resolved model id, not the legacy tier-name
- Hint with an invalid provider string (factory Err) falls back to
parent rather than propagating
Behaviour is unchanged — same match arms, same fallback policy, same
logs. The only call-site rewrite is in `run_typed_mode` which now
hoists `Config::load_or_init().await` and delegates to the helper.
cargo test resolve_subagent_provider: 5 passed
The thread-session cache-hit predicate was four inlined `&&`
comparisons against SessionEntry fields. Pull the cache-relevant
inputs into a `SessionCacheFingerprint` struct so:
- the predicate is a single `entry.fingerprint == current_fp`
- "what invalidates the warm-thread cache?" is answerable in one
place (add a field here = add a rebuild dimension)
- the equality contract is unit-testable without constructing a
real `Agent` (the old SessionEntry held an `Agent`, so the
predicate could only be exercised via a full `run_chat_task`
integration path)
SessionEntry now holds `{ agent, fingerprint }`. Insert site and the
cache-miss log line updated to read through `fingerprint`. Behaviour
identical — same four dimensions (model_override, temperature,
target_agent_id, reasoning_provider), same equality semantics.
Five tests:
- identical inputs → cache hit
- reasoning_provider change → rebuild (the tinyhumansai#1710 fix this guards)
- reasoning_provider None vs Some → rebuild (None routes via
factory to 'cloud', explicit value does not — distinct)
- target_agent_id flip → rebuild (welcome→orchestrator regression
guard the struct also protects)
- model_override / temperature participate
cargo test fingerprint: 5 passed
Replaces the `#[ignore]` from the earlier batch with a real fix. The full-path test (Agent::turn → SpawnSubagentTool → registry lookup → run_subagent → inner loop → result threading) needs the sub-agent to use the test's scripted MockProvider. Every shipped builtin declares `[model] hint = "<workload>"`, and after tinyhumansai#1710 a Hint sub-agent builds a fresh factory provider — so it can't share the parent's MockProvider and the scripted response chain leaks. Add `__test_inherit_echo`, a `#[cfg(test)]`-only builtin with `ModelSpec::Inherit` (Inherit keeps `parent.provider`). It is appended in `builtin_definitions::all()` only in test builds and is never compiled into release. Point the spawn test at it instead of `researcher`. This is the right split: the dispatch/threading plumbing is tested here with a deterministic provider; Hint→factory provider routing is tested independently by `subagent_runner::ops::tests::resolve_subagent_provider_*` (added in the previous commit). Neither test conflates the two concerns tinyhumansai#1710 deliberately separated. Adjusted `all_definitions_present` (BUILTINS.len() + 1 in test builds) and added `test_inherit_echo_is_present_and_inherits`. cargo test: spawn-subagent full-path + 4 builtin_definitions tests all pass; no ignored tests remain from this PR.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConsolidates AI provider config into a cloud_providers registry with per-workload routing; replaces BackendProvider/LocalModel panels with a unified AIPanel and aiSettingsApi; updates Rust schemas, adds a 1→2 migration, provider factory, runtime wiring, local runtime controls, and tests. ChangesUnified AI Provider Configuration System
Sequence DiagramsequenceDiagram
participant User
participant AIPanel
participant aiSettingsApi
participant CoreConfig
participant ProviderFactory
participant Memory
User->>AIPanel: edit settings (enable local / set memory_provider=ollama:xyz)
AIPanel->>aiSettingsApi: saveAISettings(prev,next)
aiSettingsApi->>CoreConfig: openhumanUpdateModelSettings(patch)
CoreConfig->>CoreConfig: apply patch, persist
ProviderFactory->>CoreConfig: workload_local_model("memory") / workload_uses_local("memory")
ProviderFactory->>Memory: return OllamaChatProvider(endpoint, "xyz")
Estimated code review effort 🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
|
Post-rebase / post-test-tweak formatting only — no behaviour change. cargo fmt rewrapped new code in builder.rs / factory.rs / subagent_runner/ops.rs / cron/scheduler.rs; prettier normalised the five settings/AI TS files edited after the last format:check pass (AIPanel + its test, useSettingsNavigation, aiSettingsApi, auth tauriCommands). Fixes the Rust Quality (fmt) and Prettier CI gates.
Round 2 — the first style commit only staged 9 files; cargo fmt --all also reformats the conflict-resolved config schema files (ops.rs, schemas.rs, cloud_providers.rs, types.rs), the migration, cron/run.rs, and web_tests.rs. Stage all real fmt diffs this time. No behaviour change.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/openhuman/config/schemas.rs (2)
337-379:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
get_client_config's schema in sync with its new response body.
handle_get_client_confignow returnscloud_providers,primary_cloud, and the per-workload*_providerfields, but the controller schema still advertises only the legacy payload. Anything consumingall_controller_schemas()as the RPC contract will see an incomplete response shape.Also applies to: 893-909
🤖 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/schemas.rs` around lines 337 - 379, The ControllerSchema for "get_client_config" is out of date: handle_get_client_config now returns extra keys (cloud_providers, primary_cloud, and the per-workload provider fields e.g. <workload>_provider) but the schema's outputs list lacks them; update the "get_client_config" ControllerSchema outputs to include FieldSchema entries for cloud_providers (TypeSchema::Json or appropriate map type), primary_cloud (TypeSchema::Option(Box::new(TypeSchema::String)) if optional), and each per-workload provider field name and type that handle_get_client_config emits (match the exact names used in handle_get_client_config), so all consumers of all_controller_schemas() see the complete response shape.
916-973: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd
[config][rpc]diagnostics aroundupdate_model_settings.This is now a central config-mutation path, but unlike the adjacent handlers it has no entry/validation/failure logging. A small debug/warn envelope here would make provider-routing regressions much easier to trace.
As per coding guidelines: "In Rust, use
log/tracingatdebugortracelevel for development-oriented diagnostics on new/changed flows..."🤖 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/schemas.rs` around lines 916 - 973, Add tracing diagnostics around the handle_update_model_settings flow: at the start of the async block in handle_update_model_settings emit a tracing::debug/trace message (e.g., tracing::debug!(?params, "update_model_settings received") or a sanitized subset) before deserialize_params, log validation failures (map deserialize_params errors to tracing::warn/error) and also trace the constructed config_rpc::ModelSettingsPatch (or key fields like primary_cloud/reasoning_provider) before calling config_rpc::load_and_apply_model_settings(patch).await, and on Err from that call emit tracing::error!(error = %err, "failed to apply model settings") to mirror adjacent handlers' entry/validation/failure logging.app/src/pages/Settings.tsx (1)
263-305:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the removed AI URLs redirecting to the new panel.
This drops the old pages entirely, so
/settings/ai-models,/settings/local-model, and/settings/backend-providernow fall through to the global/settingsfallback. Deep links/bookmarks will break instead of landing on the new AI surface.↪️ Suggested redirect aliases
+ <Route path="ai-models" element={<Navigate to="/settings/ai" replace />} /> + <Route path="local-model" element={<Navigate to="/settings/llm" replace />} /> + <Route path="backend-provider" element={<Navigate to="/settings/llm" replace />} />🤖 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/pages/Settings.tsx` around lines 263 - 305, The removed AI pages left deep-links like /settings/ai-models, /settings/local-model, and /settings/backend-provider falling through; add Route redirects that map those legacy paths to the new "ai" panel (use React Router's Navigate to redirect to "ai" with replace) and place them alongside the other <Route> entries (e.g. add Route path="ai-models" element={<Navigate to="ai" replace />} etc.), ensuring you use the same wrapSettingsPage wrapper where appropriate so legacy links land on the new SettingsSectionPage/AIPanel UI.
🟡 Minor comments (9)
src/openhuman/tools/impl/cron/run.rs-160-165 (1)
160-165:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRustfmt check is failing in this assertion block.
Please run
cargo fmt --alland commit the formatted output so Rust Quality passes.As per coding guidelines
Run cargo fmt and cargo check on changed Rust files before merging to the main branch.🤖 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/tools/impl/cron/run.rs` around lines 160 - 165, The assertion block in src/openhuman/tools/impl/cron/run.rs (the code using cron::list_runs and assert_eq! for spawn vs successful run) is failing rustfmt; run `cargo fmt --all` to format the changed Rust files (or run your editor's Rustfmt) and commit the resulting changes so the assertions and surrounding code conform to rustfmt style and the Rust Quality check passes.src/openhuman/config/schema/types.rs-167-225 (1)
167-225:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRustfmt is blocking this provider-routing section in CI.
Please run
cargo fmt --alland commit the formatting changes in this block before merge.As per coding guidelines
Run cargo fmt and cargo check on changed Rust files before merging to the main branch.🤖 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/types.rs` around lines 167 - 225, Rust formatting is failing for the provider-routing section; run cargo fmt --all to format src/openhuman/config/schema/types.rs (affecting the struct fields cloud_providers, primary_cloud, reasoning_provider, agentic_provider, coding_provider, memory_provider, embeddings_provider, heartbeat_provider, learning_provider, subconscious_provider), add the resulting changes to your branch, and commit them; optionally run cargo check to ensure no further issues before pushing.src/openhuman/migrations/unify_ai_provider_settings.rs-176-208 (1)
176-208:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRustfmt is failing on this migration derivation block.
Please run
cargo fmt --alland commit the wrapped formatting changes here.As per coding guidelines
Run cargo fmt and cargo check on changed Rust files before merging to the main branch.🤖 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/migrations/unify_ai_provider_settings.rs` around lines 176 - 208, Run rustfmt and commit the formatting changes: apply `cargo fmt --all` to reformat the migration code in unify_ai_provider_settings.rs (the block that computes memory_value, embeddings_value, heartbeat_value, and learning_value and calls set_field on config.memory_provider, config.embeddings_provider, config.heartbeat_provider, etc.), then add and commit the resulting whitespace/line-wrapping changes so the file passes rustfmt (and then run cargo check to validate).src/openhuman/cron/scheduler.rs-253-281 (1)
253-281:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRun rustfmt on the new model-resolution block.
cargo fmt --checkis already failing here, so this won’t merge cleanly until the block is reformatted.As per coding guidelines
Run cargo fmt and cargo check on changed Rust files before merging to the main branch.🤖 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/cron/scheduler.rs` around lines 253 - 281, The new model-resolution match block around resolved_model (matching ModelSpec::Hint/Inherit/Exact) is not formatted to project rustfmt rules; run cargo fmt and reformat the block (or run rustfmt on src/openhuman/cron/scheduler.rs) so the match arms, nested match on crate::openhuman::providers::create_chat_provider, and tracing! macro calls (including the Err arm that logs error and fallback_model) follow the project's formatting conventions; ensure spacing, line breaks, and indentation for the ModelSpec::Hint branch, create_chat_provider call, and tracing! invocations are fixed so cargo fmt --check passes.app/src/utils/tauriCommands/auth.ts-117-160 (1)
117-160:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRun Prettier on the new auth wrappers.
CI is already failing this block on formatting, so please reformat it before merge.
As per coding guidelines
Format TypeScript/JavaScript code using Prettier and lint using ESLint before merging.🤖 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/utils/tauriCommands/auth.ts` around lines 117 - 160, The new auth wrapper functions (authStoreProviderCredentials, authRemoveProviderCredentials, authListProviderCredentials) are failing CI due to formatting; run Prettier on this file (and any changed TS files) to fix spacing/line breaks and ensure ESLint autofix where applicable, then re-stage the formatted changes before committing so the callCoreRpc calls, JSDoc comments, and async function signatures conform to the project's Prettier/ESLint rules.app/src/components/settings/panels/__tests__/AIPanel.test.tsx-1-1 (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRun Prettier to fix formatting.
The pipeline is warning that Prettier formatting check failed. Run
prettier --writeon this file to fix.🤖 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` at line 1, The file AIPanel.test.tsx has Prettier formatting issues; run the formatter (e.g., prettier --write) on that file or your repository to fix spacing/linebreaks (the import line referencing screen and waitFor from '@testing-library/react' is flagged), then stage the updated AIPanel.test.tsx so the Prettier check passes in CI.src/openhuman/config/ops.rs-184-190 (1)
184-190:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix rustfmt formatting to pass CI.
The pipeline is failing because
cargo fmtrequires reformatting of theOption<Vec<...>>type declaration. Runcargo fmtto fix.🤖 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/ops.rs` around lines 184 - 190, The type declaration for the public field `cloud_providers` (currently `pub cloud_providers: Option<Vec<crate::openhuman::config::schema::cloud_providers::CloudProviderCreds>,>,`) is misformatted and failing `cargo fmt`; run `cargo fmt` or reformat that declaration to match rustfmt style (e.g., wrap generics/line breaks consistently) so the `cloud_providers: Option<Vec<...CloudProviderCreds>>` declaration is properly formatted and CI will pass.src/openhuman/agent/harness/subagent_runner/ops.rs-120-127 (1)
120-127:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix rustfmt formatting to pass CI.
The pipeline is failing because
cargo fmtrequires reformatting of thislog::warn!macro call. Runcargo fmtto fix the line wrapping.🤖 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/agent/harness/subagent_runner/ops.rs` around lines 120 - 127, The log::warn! macro call inside the Err(e) match arm in ops.rs (the branch that logs "[subagent_runner] workload '{}' provider build failed..." using variables workload, e, agent_id, parent_model) is misformatted for rustfmt; run cargo fmt to reflow/wrap the macro invocation so it matches rustfmt rules (or manually adjust the macro call's line breaks/indentation to conform) so CI passes. Ensure the macro arguments remain the same and only whitespace/line-wrapping is changed.app/src/components/settings/hooks/useSettingsNavigation.ts-20-20 (1)
20-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRun Prettier on this route update before merge.
CI is already failing
prettier --checkfor this file, so please reformat these edited blocks before landing.Also applies to: 92-92, 159-199
🤖 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/hooks/useSettingsNavigation.ts` at line 20, Run Prettier to reformat the edited hook implementation and fix CI failures: run your project's Prettier command (e.g., prettier --write) against the useSettingsNavigation hook file and reformat the updated route union (the block that now includes 'llm') as well as the other edited blocks in that file so the file adheres to the project's Prettier rules before merging.
🤖 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 721-733: The mixed preset writes local routes with model:
installed[0]?.id ?? '' which produces an empty model id when none are installed;
update applyPreset (used with WORKLOADS and draft.routing / RoutingMap) so that
in the 'mixed' branch you check installed[0]?.id and if it's falsy set
next[w.id] = { kind: 'primary' } (same fallback as the 'local' preset) instead
of { kind: 'local', model: '' } to avoid creating invalid ollama: routes.
- Around line 790-803: When removing a cloud provider in the onRemove handler
you currently only filter draft.cloudProviders; also ensure you clear or
reassign draft.primaryCloudId if it equals the removed p.id and scrub or
reassign any pinned routes/workloads that reference that provider (e.g. update
draft.workloads or draft.pinnedRoutes to remove providerId === p.id or set to a
valid provider), then call setDraft with the updated cloudProviders,
primaryCloudId and workloads/pinnedRoutes; update the onRemove in the
CloudProviderCard loop to perform these three coordinated updates so no stale
primaryCloudId or orphaned providerId references remain.
In `@app/src/services/api/aiSettingsApi.ts`:
- Around line 105-135: parseProviderString currently maps the explicit
"openhuman" token to { kind: 'primary' } and serializeProviderRef may emit an
"openhuman:" prefix that the Rust factory can't parse, causing round-trip
loss/corruption; update parseProviderString so that an explicit "openhuman" (and
the equivalent explicit "openhuman:MODEL") is parsed as { kind: 'cloud',
providerType: PROVIDER_PREFIXES['openhuman'], model: ... } instead of 'primary',
and ensure serializeProviderRef continues to emit the correct "openhuman:MODEL"
form for cloud refs; adjust the same logic locations referenced around
parseProviderString and the analogous block at the other occurrence (the 235-244
area) so explicit OpenHuman selections are preserved consistently both ways.
In `@src/openhuman/agent/harness/session/builder.rs`:
- Around line 721-729: The builder currently ignores caller-specific model
overrides by always calling create_chat_provider("reasoning", config) and taking
its returned (provider, model_name); change this so an explicit override from
the caller (the mutated Config.default_model or a model_override path) wins:
either pass the caller's override into create_chat_provider (so its signature
uses ProviderRuntimeOptions/default_model) or, if create_chat_provider must
remain unchanged, call it to get (provider, resolved_model) and then replace
resolved_model with the caller-supplied default_model/model_override before
using provider/model_name; update references to ProviderRuntimeOptions,
create_chat_provider, provider, model_name, Config.default_model, and any
model_override handling to ensure the caller override takes precedence.
In `@src/openhuman/channels/providers/web_tests.rs`:
- Around line 323-342: The new tests fingerprint_identical_inputs_are_cache_hit
and fingerprint_reasoning_provider_change_forces_rebuild are failing rustfmt;
run cargo fmt --all (or format the file) to fix spacing/line breaks around the
fp(...) calls and the assert_ne! invocation so the code matches rustfmt style,
or adjust the expressions to use rustfmt-friendly formatting (e.g., put
arguments/continuation lines on their own lines) for the fp and assert_ne!
usages.
In `@src/openhuman/channels/providers/web.rs`:
- Around line 61-71: The current reasoning_provider field only stores the
selector string and therefore doesn't change when the resolved backend/model
(e.g., primary cloud provider or model name) changes; update the cache
fingerprint to include the resolved backend and model tuple instead of (or in
addition to) the selector. Concretely, change the stored field used for cache
invalidation (currently reasoning_provider: Option<String>) to hold the resolved
pair (e.g., Option<(String /*provider_key*/, String /*model*/)> or a single
composite fingerprint string), populate that resolved (provider, model) when you
construct the cached Agent/entry (use the same resolution logic that
create_chat_provider or the primary-cloud resolution uses), and ensure the cache
lookup/eviction logic and any equality/hash used for the cache key compare this
resolved tuple so switching primary cloud or model forces a cache miss.
In `@src/openhuman/config/schema/cloud_providers.rs`:
- Around line 112-116: The ID-generation loop's long chained numeric expression
causes rustfmt failure; refactor the assignment to make it formatter-friendly by
introducing intermediate constants or variables for the two 64-bit constants and
then use them in two shorter operations (e.g., let multiplier =
6364136223846793005u64; let increment = 1442695040888963407u64; seed =
seed.wrapping_mul(multiplier).wrapping_add(increment); seed = (seed >> 33) ^
seed;), ensuring types match nanos/seed and that you still push chars[seed %
chars.len()] into suffix; this keeps the logic intact while satisfying rustfmt
for the loop that references seed, nanos, chars, and suffix.
In `@src/openhuman/config/schemas.rs`:
- Around line 941-947: The match over e.r#type currently maps unknown strings to
CloudProviderType::Custom which hides typos; instead, change the match on
e.r#type.to_ascii_lowercase().as_str() (the r#type binding) to only accept the
explicit arms ("openhuman","openai","anthropic","openrouter") and have the _ arm
return a validation error (or propagate Err) so invalid provider types fail
parsing/validation rather than being coerced to CloudProviderType::Custom;
update the surrounding function to return a Result/validation error where needed
so callers receive the failure.
In `@src/openhuman/credentials/profiles.rs`:
- Around line 279-286: The warning logs leak PII by printing raw profile
identifiers (id and dropped_ids) from profile_id() — update the log calls in the
unrecoverable-profile path (the log::warn! invocations around variables id,
p.provider and dropped_ids) to redact or omit identifiers: either log only
p.provider and counts, or replace each id/dropped_id with a deterministic short
hash or truncated-safe representation (e.g. first 8 hex chars) before
interpolation; ensure the same redaction helper is reused for both the log at
the shown log::warn! and the similar one near lines 356–361 (use a helper like
redact_profile_id(name: &str) to transform ids consistently).
In `@src/openhuman/local_ai/service/bootstrap.rs`:
- Around line 125-128: mark_disabled currently sets *self.status.lock() =
LocalAiStatus::disabled(config) without coordinating with bootstrap(), allowing
an in-flight bootstrap to overwrite the disabled state; fix by serializing with
the bootstrap lifecycle: either make mark_disabled async and acquire the same
bootstrap_lock used in bootstrap() before mutating status (e.g., call
self.bootstrap_lock.write().await or equivalent), or add a generation/epoch
field (e.g., a u64 epoch on the struct) that mark_disabled increments and
bootstrap reads/validates before writing terminal states; ensure
LocalAiStatus::disabled(config), status.lock(), and bootstrap()/bootstrap_lock
are the symbols adjusted so bootstrap only writes if the epoch matches or after
acquiring the same lock.
In `@src/openhuman/memory/tree/chat/mod.rs`:
- Around line 120-151: The Ollama branch currently checks
workload_uses_local("memory") and picks models from
memory_tree.llm_extractor_model / llm_summariser_model, which ignores the
unified local model setting; update the branch so it calls
workload_local_model("memory") to determine the authoritative local model (and
fall back to the per-consumer legacy fields only if workload_local_model returns
None), and pass that resolved model into local::OllamaChatProvider::new along
with endpoint and timeout_ms; adjust the model resolution logic around the
ChatConsumer match (and variable named model) so the unified setting overrides
legacy llm_extractor_model / llm_summariser_model values.
In `@src/openhuman/memory/tree/jobs/worker.rs`:
- Around line 152-157: Add a lightweight trace log at the branch that decides
local-vs-cloud gate routing so each job records which path was taken: inside the
if using config.workload_uses_local("memory") and the else where you
drop(gate_permit) returning None, emit a tracing::trace! (or log::trace!)
message that includes identifying context (e.g., job id or whatever
request/context variable is available nearby) and the chosen route ("local" vs
"cloud"/"global-permit"); keep the messages short and non-blocking so they only
provide diagnostic info for contention/routing.
In `@src/openhuman/memory/tree/read_rpc.rs`:
- Around line 1699-1714: staged.memory_provider is being computed from parsed
and staged.memory_tree (llm_summariser_model / llm_extractor_model) before any
request-level model overrides are applied, causing stale routing when backend ==
Local; update the code so staged.memory_provider is set only after the
request-level overrides are written (or recompute it right after
apply_overrides), i.e., move or duplicate the logic that builds the provider
string (the match on parsed with the selection of
staged.memory_tree.llm_summariser_model.or_else(||
staged.memory_tree.llm_extractor_model).unwrap_or_else(||
staged.local_ai.chat_model_id) and the format!("ollama:{m}") branch) to run
after overrides are applied so it picks up the overridden model ids instead of
the pre-override values.
In `@src/openhuman/memory/tree/tree_source/summariser/mod.rs`:
- Around line 91-107: The local-branch model detection only reads legacy
memory_tree.llm_summariser_endpoint/_model and thus drops through to
InertSummariser when a unified memory_provider like "ollama:qwen3" is used;
change the logic that sets the local `model: Option<String>` to also consult
`config.memory_provider` (e.g., split on ':' and take the right-hand token, trim
and accept non-empty) when `llm_summariser_model` is absent, and likewise derive
an endpoint if the provider format supplies one; apply the same fix to the
equivalent logic at the other location mentioned (the block around lines
118-124) so unified routing yields the expected LLM summariser instead of
InertSummariser.
In `@src/openhuman/migrations/mod.rs`:
- Around line 111-140: The current migration guard uses if config.schema_version
< 2 which allows running the 1→2 migration even when schema_version is 0 (i.e.,
0→1 hasn't completed); change the guard so the 1→2 migration only runs when
config.schema_version == 1 (or otherwise explicitly verify the 0→1 step
succeeded) before invoking unify_ai_provider_settings::run and config.save();
keep the existing rollback-on-save-failure behavior that resets
config.schema_version to previous_version and logs a warning.
In `@src/openhuman/migrations/unify_ai_provider_settings.rs`:
- Around line 132-136: The migration currently logs the raw inference_url via
log::info using the variable trimmed which can leak credentials; change this to
log a redacted version by removing or masking authentication info and sensitive
query params before logging (e.g., implement a small helper like redact_url or
redact_inference_url that parses trimmed, strips username/password and sensitive
query keys or replaces them with "<redacted>"), then pass the redacted value
into the log::info call instead of trimmed.
- Around line 230-233: The helper looks_like_openhuman is too permissive because
it substring-matches the entire URL; change it to parse the URL (using
url::Url::parse) and inspect only the host (and optionally the port) instead of
the full string so custom endpoints containing "openhuman" in paths or query
params aren't misclassified; update looks_like_openhuman to return false on
parse errors and match the host against a safe set/rules (e.g., host ==
"openhuman.ai" or host.ends_with(".openhuman.ai") or host == "openhuman" for
local names) rather than url.to_ascii_lowercase().contains("openhuman").
In `@src/openhuman/providers/factory.rs`:
- Around line 214-220: The info! calls log full provider endpoints (e.endpoint)
and may leak secrets; update the logging in the provider resolution code (the
log::info! calls in src/openhuman/providers/factory.rs around the chat-factory
blocks) to redact endpoints by parsing the URL and logging only the scheme and
host (or a redacted host) and/or provider type (e.r#type.label()) instead of the
full e.endpoint; apply the same change to the other occurrences you noted (the
similar log::info! sites around the blocks at the locations you referenced) so
no raw URL, credentials, or query strings are emitted to logs.
- Around line 144-157: The Anthropic branch (CloudProviderType::Anthropic) must
validate that the configured endpoint is an OpenAI-compatible endpoint and avoid
logging raw endpoints with embedded credentials: add a validation in the flow
that calls make_openai_compatible_provider() (e.g., in
make_cloud_provider_by_type or immediately before calling
make_openai_compatible_provider()) that checks the configured endpoint string
contains a known OpenAI-compatible host pattern (for example
"api.anthropic.com/v1" or a configurable allowlist) and return an error
(anyhow::bail!) with a clear message if it looks like a native Anthropic
endpoint; additionally, replace any info-level logs that currently print the
full endpoint URL in make_openai_compatible_provider() (and other places that
log endpoints) with a redacted form (strip userinfo/credentials or log only the
host/path) or a placeholder like "<redacted-endpoint>" to avoid leaking secrets
while preserving useful context.
---
Outside diff comments:
In `@app/src/pages/Settings.tsx`:
- Around line 263-305: The removed AI pages left deep-links like
/settings/ai-models, /settings/local-model, and /settings/backend-provider
falling through; add Route redirects that map those legacy paths to the new "ai"
panel (use React Router's Navigate to redirect to "ai" with replace) and place
them alongside the other <Route> entries (e.g. add Route path="ai-models"
element={<Navigate to="ai" replace />} etc.), ensuring you use the same
wrapSettingsPage wrapper where appropriate so legacy links land on the new
SettingsSectionPage/AIPanel UI.
In `@src/openhuman/config/schemas.rs`:
- Around line 337-379: The ControllerSchema for "get_client_config" is out of
date: handle_get_client_config now returns extra keys (cloud_providers,
primary_cloud, and the per-workload provider fields e.g. <workload>_provider)
but the schema's outputs list lacks them; update the "get_client_config"
ControllerSchema outputs to include FieldSchema entries for cloud_providers
(TypeSchema::Json or appropriate map type), primary_cloud
(TypeSchema::Option(Box::new(TypeSchema::String)) if optional), and each
per-workload provider field name and type that handle_get_client_config emits
(match the exact names used in handle_get_client_config), so all consumers of
all_controller_schemas() see the complete response shape.
- Around line 916-973: Add tracing diagnostics around the
handle_update_model_settings flow: at the start of the async block in
handle_update_model_settings emit a tracing::debug/trace message (e.g.,
tracing::debug!(?params, "update_model_settings received") or a sanitized
subset) before deserialize_params, log validation failures (map
deserialize_params errors to tracing::warn/error) and also trace the constructed
config_rpc::ModelSettingsPatch (or key fields like
primary_cloud/reasoning_provider) before calling
config_rpc::load_and_apply_model_settings(patch).await, and on Err from that
call emit tracing::error!(error = %err, "failed to apply model settings") to
mirror adjacent handlers' entry/validation/failure logging.
---
Minor comments:
In `@app/src/components/settings/hooks/useSettingsNavigation.ts`:
- Line 20: Run Prettier to reformat the edited hook implementation and fix CI
failures: run your project's Prettier command (e.g., prettier --write) against
the useSettingsNavigation hook file and reformat the updated route union (the
block that now includes 'llm') as well as the other edited blocks in that file
so the file adheres to the project's Prettier rules before merging.
In `@app/src/components/settings/panels/__tests__/AIPanel.test.tsx`:
- Line 1: The file AIPanel.test.tsx has Prettier formatting issues; run the
formatter (e.g., prettier --write) on that file or your repository to fix
spacing/linebreaks (the import line referencing screen and waitFor from
'@testing-library/react' is flagged), then stage the updated AIPanel.test.tsx so
the Prettier check passes in CI.
In `@app/src/utils/tauriCommands/auth.ts`:
- Around line 117-160: The new auth wrapper functions
(authStoreProviderCredentials, authRemoveProviderCredentials,
authListProviderCredentials) are failing CI due to formatting; run Prettier on
this file (and any changed TS files) to fix spacing/line breaks and ensure
ESLint autofix where applicable, then re-stage the formatted changes before
committing so the callCoreRpc calls, JSDoc comments, and async function
signatures conform to the project's Prettier/ESLint rules.
In `@src/openhuman/agent/harness/subagent_runner/ops.rs`:
- Around line 120-127: The log::warn! macro call inside the Err(e) match arm in
ops.rs (the branch that logs "[subagent_runner] workload '{}' provider build
failed..." using variables workload, e, agent_id, parent_model) is misformatted
for rustfmt; run cargo fmt to reflow/wrap the macro invocation so it matches
rustfmt rules (or manually adjust the macro call's line breaks/indentation to
conform) so CI passes. Ensure the macro arguments remain the same and only
whitespace/line-wrapping is changed.
In `@src/openhuman/config/ops.rs`:
- Around line 184-190: The type declaration for the public field
`cloud_providers` (currently `pub cloud_providers:
Option<Vec<crate::openhuman::config::schema::cloud_providers::CloudProviderCreds>,>,`)
is misformatted and failing `cargo fmt`; run `cargo fmt` or reformat that
declaration to match rustfmt style (e.g., wrap generics/line breaks
consistently) so the `cloud_providers: Option<Vec<...CloudProviderCreds>>`
declaration is properly formatted and CI will pass.
In `@src/openhuman/config/schema/types.rs`:
- Around line 167-225: Rust formatting is failing for the provider-routing
section; run cargo fmt --all to format src/openhuman/config/schema/types.rs
(affecting the struct fields cloud_providers, primary_cloud, reasoning_provider,
agentic_provider, coding_provider, memory_provider, embeddings_provider,
heartbeat_provider, learning_provider, subconscious_provider), add the resulting
changes to your branch, and commit them; optionally run cargo check to ensure no
further issues before pushing.
In `@src/openhuman/cron/scheduler.rs`:
- Around line 253-281: The new model-resolution match block around
resolved_model (matching ModelSpec::Hint/Inherit/Exact) is not formatted to
project rustfmt rules; run cargo fmt and reformat the block (or run rustfmt on
src/openhuman/cron/scheduler.rs) so the match arms, nested match on
crate::openhuman::providers::create_chat_provider, and tracing! macro calls
(including the Err arm that logs error and fallback_model) follow the project's
formatting conventions; ensure spacing, line breaks, and indentation for the
ModelSpec::Hint branch, create_chat_provider call, and tracing! invocations are
fixed so cargo fmt --check passes.
In `@src/openhuman/migrations/unify_ai_provider_settings.rs`:
- Around line 176-208: Run rustfmt and commit the formatting changes: apply
`cargo fmt --all` to reformat the migration code in
unify_ai_provider_settings.rs (the block that computes memory_value,
embeddings_value, heartbeat_value, and learning_value and calls set_field on
config.memory_provider, config.embeddings_provider, config.heartbeat_provider,
etc.), then add and commit the resulting whitespace/line-wrapping changes so the
file passes rustfmt (and then run cargo check to validate).
In `@src/openhuman/tools/impl/cron/run.rs`:
- Around line 160-165: The assertion block in
src/openhuman/tools/impl/cron/run.rs (the code using cron::list_runs and
assert_eq! for spawn vs successful run) is failing rustfmt; run `cargo fmt
--all` to format the changed Rust files (or run your editor's Rustfmt) and
commit the resulting changes so the assertions and surrounding code conform to
rustfmt style and the Rust Quality check passes.
🪄 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: e2246388-7dee-4a3d-9617-37c9b306f7c0
📒 Files selected for processing (57)
app/src-tauri/capabilities/default.jsonapp/src/components/settings/SettingsHome.tsxapp/src/components/settings/__tests__/SettingsHome.test.tsxapp/src/components/settings/hooks/useSettingsNavigation.tsapp/src/components/settings/panels/AIPanel.tsxapp/src/components/settings/panels/BackendProviderPanel.tsxapp/src/components/settings/panels/LocalModelPanel.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsxapp/src/components/settings/panels/__tests__/BackendProviderPanel.test.tsxapp/src/components/settings/panels/__tests__/LocalModelPanel.test.tsxapp/src/pages/Settings.tsxapp/src/services/api/aiSettingsApi.tsapp/src/utils/localAiHelpers.tsapp/src/utils/tauriCommands/auth.tsapp/src/utils/tauriCommands/config.tsapp/src/utils/tauriCommands/localAi.tsdesign-previews/ai-settings.htmlsrc/openhuman/agent/harness/builtin_definitions.rssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/session/runtime.rssrc/openhuman/agent/harness/session/tests.rssrc/openhuman/agent/harness/session/turn.rssrc/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/agent/harness/subagent_runner/ops_tests.rssrc/openhuman/channels/providers/web.rssrc/openhuman/channels/providers/web_tests.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/config/ops.rssrc/openhuman/config/ops_tests.rssrc/openhuman/config/schema/cloud_providers.rssrc/openhuman/config/schema/local_ai.rssrc/openhuman/config/schema/mod.rssrc/openhuman/config/schema/types.rssrc/openhuman/config/schemas.rssrc/openhuman/credentials/profiles.rssrc/openhuman/credentials/profiles_tests.rssrc/openhuman/cron/scheduler.rssrc/openhuman/learning/reflection.rssrc/openhuman/local_ai/ops.rssrc/openhuman/local_ai/schemas.rssrc/openhuman/local_ai/service/bootstrap.rssrc/openhuman/memory/store/factories.rssrc/openhuman/memory/tree/chat/mod.rssrc/openhuman/memory/tree/jobs/worker.rssrc/openhuman/memory/tree/read_rpc.rssrc/openhuman/memory/tree/score/embed/factory.rssrc/openhuman/memory/tree/score/extract/mod.rssrc/openhuman/memory/tree/score/mod.rssrc/openhuman/memory/tree/tree_source/summariser/mod.rssrc/openhuman/migrations/mod.rssrc/openhuman/migrations/mod_tests.rssrc/openhuman/migrations/unify_ai_provider_settings.rssrc/openhuman/migrations/unify_ai_provider_settings_tests.rssrc/openhuman/providers/factory.rssrc/openhuman/providers/mod.rssrc/openhuman/subconscious/executor.rssrc/openhuman/tools/impl/cron/run.rs
💤 Files with no reviewable changes (4)
- app/src/components/settings/panels/tests/LocalModelPanel.test.tsx
- app/src/components/settings/panels/LocalModelPanel.tsx
- app/src/components/settings/panels/tests/BackendProviderPanel.test.tsx
- app/src/components/settings/panels/BackendProviderPanel.tsx
Comment on lines
+144
to
+157
| let provider_type = match type_str { | ||
| "openai" => CloudProviderType::Openai, | ||
| "anthropic" => CloudProviderType::Anthropic, | ||
| "openrouter" => CloudProviderType::Openrouter, | ||
| "custom" => CloudProviderType::Custom, | ||
| other => anyhow::bail!( | ||
| "[chat-factory] unknown provider type '{}' in provider string '{}' for role '{}'", | ||
| other, | ||
| p, | ||
| role | ||
| ), | ||
| }; | ||
|
|
||
| make_cloud_provider_by_type(role, &provider_type, model.trim(), config) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Anthropic's direct Messages API support OpenAI-compatible request bodies and Bearer Authorization headers, or does it require Anthropic-specific headers like x-api-key / anthropic-version and its own payload format?
💡 Result:
Anthropic's direct Messages API requires specific headers and its own payload format for native requests, but Anthropic also provides an OpenAI-compatible endpoint that supports OpenAI-style request bodies and authentication [1][2][3]. Native Anthropic Messages API: - Authentication: Requires the x-api-key header (or an Authorization: Bearer header for specific OAuth tokens) and the mandatory anthropic-version header (e.g., 2023-06-01) [1]. - Payload Format: Uses a specific JSON structure, including a messages array with role/content objects and a system field for system prompts [4][5][6]. OpenAI-Compatible Endpoint: - Anthropic provides an OpenAI-compatible API surface that allows developers to use OpenAI-style request bodies and headers [2][3]. - This compatibility layer enables the use of OpenAI SDKs and tools by pointing them to the appropriate Anthropic endpoint, supporting standard OpenAI request fields like model, messages, max_tokens, and stream [2][3]. - While this endpoint facilitates compatibility, some specific features or fields may be ignored or require the native API for full functionality (e.g., certain structured output configurations) [2][3].
Citations:
- 1: https://platform.claude.com/docs/en/api/overview
- 2: https://console.anthropic.com/docs/en/api/openai-sdk
- 3: https://platform.claude.com/docs/en/api/openai-sdk
- 4: https://docs.anthropic.com/claude/reference/complete
- 5: https://docs.anthropic.com/claude/reference/messages_post
- 6: https://docs.aws.amazon.com/bedrock/latest/userguide/model-parameters-anthropic-claude-messages.html
🏁 Script executed:
cat -n src/openhuman/providers/factory.rs | head -n 160Repository: tinyhumansai/openhuman
Length of output: 7374
🏁 Script executed:
cat -n src/openhuman/providers/factory.rs | sed -n '155,320p'Repository: tinyhumansai/openhuman
Length of output: 7003
🏁 Script executed:
cat -n src/openhuman/providers/factory.rs | tail -n 100Repository: tinyhumansai/openhuman
Length of output: 5112
Anthropic provider needs validation of endpoint type and redacted logging.
CloudProviderType::Anthropic flows through make_openai_compatible_provider() for all provider types without validating the actual endpoint. This is problematic in two ways:
-
Endpoint type mismatch: Anthropic's OpenAI-compatible endpoint (
api.anthropic.com/v1) supports OpenAI-style requests and Bearer auth, so the code will work with that endpoint. However, if a user configures Anthropic's native endpoint instead, the request will fail due to missinganthropic-versionheader. The code provides no validation or helpful error to catch this misconfiguration. -
Sensitive logging: Lines 216-220 and 305-310 log raw endpoint URLs at info level, which can leak embedded credentials if users configure endpoints with API keys/tokens in the URL. Per logging guidelines, never log secrets or full PII.
Add validation to ensure Anthropic entries use the OpenAI-compatible endpoint (or document the requirement clearly), and redact the endpoint from logs.
Also applies to: 283-313, 353-368
🤖 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/providers/factory.rs` around lines 144 - 157, The Anthropic
branch (CloudProviderType::Anthropic) must validate that the configured endpoint
is an OpenAI-compatible endpoint and avoid logging raw endpoints with embedded
credentials: add a validation in the flow that calls
make_openai_compatible_provider() (e.g., in make_cloud_provider_by_type or
immediately before calling make_openai_compatible_provider()) that checks the
configured endpoint string contains a known OpenAI-compatible host pattern (for
example "api.anthropic.com/v1" or a configurable allowlist) and return an error
(anyhow::bail!) with a clear message if it looks like a native Anthropic
endpoint; additionally, replace any info-level logs that currently print the
full endpoint URL in make_openai_compatible_provider() (and other places that
log endpoints) with a redacted form (strip userinfo/credentials or log only the
host/path) or a placeholder like "<redacted-endpoint>" to avoid leaking secrets
while preserving useful context.
…(addresses CodeRabbit review)
Logging hardening (never-log-secrets rule):
- providers/factory.rs: replace raw endpoint URLs in info! with scheme+host only
via new redact_endpoint() helper; all three log sites patched.
- migrations/unify_ai_provider_settings.rs: drop inference_url value from log,
emit inference_url_present=true instead.
- credentials/profiles.rs: drop raw profile id (provider:email) from unrecoverable-
profile warning; log provider type only.
Migration guard:
- migrations/mod.rs: change guard from schema_version < 2 to == 1 so a failed
0→1 step does not get silently skipped by the 1→2 migration.
Provider-type validation:
- config/schemas.rs: reject unknown cloud provider types with a clear error
instead of silently coercing to Custom. Uses collect::<Result<Vec<_>, String>>
+ transpose()? to propagate through ControllerFuture.
looks_like_openhuman fix:
- migrations/unify_ai_provider_settings.rs: rewrite to parse host component only
so custom endpoints containing "openhuman" in a path/query don't get misclassified.
Memory routing completeness:
- read_rpc.rs: move memory_provider derivation after model overrides are applied
so local routing reflects the final persisted values.
- memory/tree/chat/mod.rs: use workload_local_model() as fallback model when
legacy llm_*_model fields are unset; switch guard from workload_uses_local
to if-let Some(routed_model).
- tree_source/summariser/mod.rs: fall back to workload_local_model("memory")
when legacy endpoint+model pair is absent, preventing InertSummariser fallback
for unified ollama:<m> routing.
- jobs/worker.rs: add trace! log for local-vs-cloud gate routing decision.
Frontend fixes:
- AIPanel.tsx: mixed preset now falls back to { kind: 'primary' } when no model
is installed (prevents empty ollama: provider strings).
- AIPanel.tsx: onRemove also clears stale primaryCloudId and scrubs pinned
workload routes that reference the deleted provider.
- aiSettingsApi.ts: add 'openhuman' to PROVIDER_PREFIXES so openhuman:<model>
round-trips as a cloud ref; remove bare 'openhuman' from primary-sentinel guard
so explicit openhuman selections are preserved.
…ext test CI's 'Rust Core Tests + Quality' runs integration tests too (not just --lib). test_integrations_agent_has_current_date_context spawns the real integrations_agent (which ships [model] hint = "agentic"). After tinyhumansai#1710 a Hint sub-agent builds a fresh provider via the workload factory instead of inheriting parent.provider — here that resolves to the OpenHuman backend and fails 'No backend session' before the MockCalendarProvider captures anything. The test only asserts prompt construction (Current Date & Time context), not provider routing, so override the cloned def's model to Inherit: it still exercises the real integrations_agent prompt/tools while routing through the captured mock. Hint→factory routing stays covered by resolve_subagent_provider_* unit tests. Only calendar_grounding_e2e.rs uses run_subagent in tests/; no other integration test is affected. Local: both tests in the file pass.
# Conflicts: # src/openhuman/agent/harness/subagent_runner/ops_tests.rs
… redact profile IDs from purge log
- parseProviderString: bare "openhuman" now maps to { kind:'cloud', providerType:'openhuman', model:'' }
instead of collapsing to primary, preventing silent rewrite on save
- serializeProviderRef: "openhuman" + empty model emits "openhuman" sentinel (not "openhuman:" with
trailing colon that the Rust factory rejects)
- profiles.rs: remove profile IDs from the purge warning log to avoid leaking PII/provider names
(addresses @coderabbitai on credentials/profiles.rs:285)
CI 'Rust Core Coverage' failed: json_rpc_e2e json_rpc_web_chat_routing_cases_use_expected_backend_models asserted web-chat model_override 'hint:reasoning' reaches the backend as 'reasoning-v1', but tinyhumansai#1710's factory bypassed the legacy IntelligentRoutingProvider wrapper (create_intelligent_routing_provider -> routing::new_provider) whose resolve_remote_model did that mapping, so 'hint:reasoning' was passed through verbatim. Apply ONLY the model-name mapping in make_openhuman_backend (the OH-backend branch — third-party cloud providers take exact ids and never see hint: strings). Mirrors resolve_remote_model's heavy-tier arm exactly: hint:reasoning->reasoning-v1, hint:chat->reasoning-quick-v1, hint:agentic->agentic-v1, hint:coding->coding-v1; lightweight hints (hint:reaction) and already-exact tier names pass through. Deliberately NOT re-wrapping in the full IntelligentRoutingProvider: it also injects local-AI health probing + a streaming shim that the web-chat SSE path doesn't tolerate (hangs chat_done). The name mapping is the only piece the backend contract needs. This also genuinely resolves CodeRabbit's deferred builder.rs:730 model_override-bypass comment (it was the same root cause). Note: local mock harness times out this test at 12s on Windows (chat_done SSE timing) on BOTH this fix and the pre-fix baseline — a pre-existing local-env artifact; CI reaches the real assertion. Factory unit tests 19/0, no regression.
sanil-23
added a commit
to sanil-23/openhuman
that referenced
this pull request
May 15, 2026
…gs rewrite to clear diff-cover gate Covers the three uncovered frontend modules from PR tinyhumansai#1858 diff-cover report: - app/src/services/api/__tests__/aiSettingsApi.test.ts (NEW, 49 tests): Full coverage of parseProviderString, serializeProviderRef, loadAISettings (happy + auth-failure degradation), saveAISettings (no-op, cloud_providers patch, primary_cloud patch, routing patch), setCloudProviderKey (openhuman guard + store path), clearCloudProviderKey, setLocalRuntimeEnabled (dual-flag contract), shutdownLocalProvider, loadLocalProviderSnapshot (all-succeed + per-call failure degradation), localProvider namespace. - app/src/utils/tauriCommands/__tests__/auth.test.ts (NEW, 13 tests): authStoreProviderCredentials, authRemoveProviderCredentials, authListProviderCredentials — happy path, non-Tauri guard, optional args, error propagation. - app/src/components/settings/panels/__tests__/AIPanel.test.tsx (EXPANDED, 36 tests): adds interaction tests for cloud provider add/edit/remove, make-primary, save/discard bar, workload routing tabs, local provider toggle/Retry/Install/preset, daemon-conflict callout, advanced Ollama path input, on top of the existing 3 render tests.
This was referenced May 16, 2026
senamakel
added a commit
to Zavianx/openhuman
that referenced
this pull request
May 16, 2026
…vider routing Upstream PR tinyhumansai#1858 (unified per-workload provider routing) made the subagent runner resolve a real Provider via the loaded Config's workload routing for ModelSpec::Hint agents, bypassing this test's mock ConcurrentProvider. The right fix is a workspace-isolated fixture that forces the workload factory to fall back to the parent provider — tracked as a follow-up.
This was referenced May 16, 2026
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
2 participants
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.
Summary
inference_url, per-role*_provider,local_ai.usage.*,memory_tree.llm_backend, two separate settings panels) into one per-workload provider factory + a single unified AI settings panel.cloud_providers(OpenHuman + OpenAI + Anthropic + OpenRouter + Custom) with a primary pointer; API keys stored encrypted viaAuthService(enc2:envelope), never inconfig.toml.reasoning,agentic,coding,memory,embeddings,heartbeat,learning,subconscious) — plus thesummarizationalias ofmemory— throughcreate_chat_provider(workload, &config), returning an exact(provider, model)so no{workload}-v1tier name ever reaches a strict provider.schema_version = 2(unify_ai_provider_settings), idempotent, seeds the OpenHuman entry and back-fills legacyusage.*/llm_backendinto the new fields.BackendProviderPanel+LocalModelPanelremoved.Problem
LLM provider configuration was scattered and contradictory: the reasoning chat path used a legacy intelligent-routing provider that ignored the user's AI-settings routing entirely; sub-agents synthesised OpenHuman-only tier names (
agentic-v1) and 404'd against Anthropic/OpenAI; tool specs with duplicate names (researcher'sdelegate_name="research"shadowing a skill) 400'd on strict providers; the OpenHuman backend read credentials from~/.openhumaninstead of the active workspace; a warm chat thread cached its provider and never picked up a routing change; and an unrecoverable encrypted profile poisoned everyapp_state_snapshotpoll forever.Solution
providers/factory.rs): one grammar (cloud | openhuman | openai:<m> | anthropic:<m> | openrouter:<m> | custom:<m> | ollama:<m>),provider_for_rolehonoring the 8 workload fields withsummarization→memory_provideraliasing and a"cloud"wildcard fallback; backend ctor now receivesconfig.config_path.parent()so auth resolves from the real workspace.create_chat_provider("reasoning", …); tool specs deduped (first-wins) on every visible-spec rebuild path (builder, delegation refresh, scope change).resolve_subagent_provider—Hint(workload)→factory,Inherit/Exact→parent, graceful fallback on factory/config failure (never the legacy tier-name).Hintspecs resolved through the factory instead of{hint}-v1.SessionCacheFingerprintaddsreasoning_providerso a Settings → AI routing change invalidates the warm-thread agent.enc2:pattern; Tauri opener allow-list extended forollama.com.Submission Checklist
resolve_subagent_provider,SessionCacheFingerprint,dedup_visible_tool_specs, profile-drop). Finaldiff-coverto be confirmed by CI (the merged Vitest+cargo-llvm-cov gate runs there; the local full Vitest run is blocked by a pre-existing infra issue — see Impact).Closes #1710.Impact
schema_version=2on first load; idempotent and reversible-safe (rolls back in-memory on save failure). Legacy fields back-filled, not dropped.cargo test --lib= 7162 passed / 0 failed / 8 ignored (0 from this PR — the previously-ignored spawn-subagent test was fixed via a#[cfg(test)]Inherit agent).cargo check --lib,pnpm typecheck,pnpm lint,pnpm format:checkall clean. Vitest settings (20 files), services (30), pages (21/158) scopes — all green.navigationloop in some test outside the settings/services/pages scopes — reproduces identically on a clean checkout before any of these commits, under both parallel and serial execution. Every scope containing this PR's changed files passes. Worth a separate infra issue.cargo checkon the Tauri shell crate fails to build vendored native deps (whisper-rs-sys,cef-dll-sys) in a bare environment — the supportedpnpm tauripath (vendored CEF-aware CLI) builds and ran the app cleanly throughout development. CI has the native toolchain.--no-verify(committed blobs verified LF; commit-to-commit diff is the focused 57-file set, not the working-tree churn).Related
Closes #1710
Summary by CodeRabbit
New Features
Bug Fixes