feat: add API key support for custom OpenAI-compatible providers and …#905
feat: add API key support for custom OpenAI-compatible providers and …#905unn-Known1 wants to merge 1 commit into
Conversation
…integrate into configuration workflows
📝 WalkthroughWalkthroughThis PR adds API key authentication support for custom OpenAI-compatible model backends. It introduces a new UI component for configuring base URL and API key, extends the configuration schema to persist credentials, and threads the API key parameter through provider factory functions and all construction callsites. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CustomModelSection as CustomModelSection<br/>(UI)
participant TauriCmd as Tauri Command Handler
participant ConfigOps as Config Ops
participant ProviderFactory as Provider Factory
participant LocalProvider as Custom OpenAI<br/>Provider
User->>CustomModelSection: Enter base_url & api_key
CustomModelSection->>CustomModelSection: Trim values, clear errors
CustomModelSection->>TauriCmd: openhumanUpdateModelSettings(api_url, api_key)
TauriCmd->>ConfigOps: apply_model_settings(patch)
ConfigOps->>ConfigOps: Update config.api_key<br/>(None if empty)
ConfigOps->>ConfigOps: Persist snapshot
TauriCmd->>CustomModelSection: Success response
CustomModelSection->>CustomModelSection: Show success, auto-clear after 3s
Note over User,LocalProvider: Subsequent LLM interaction
User->>LocalProvider: Request inference
LocalProvider->>ProviderFactory: create_intelligent_routing_provider<br/>(api_url, api_key, config, options)
ProviderFactory->>ProviderFactory: Select "custom_openai" backend<br/>if both url & key present
ProviderFactory->>LocalProvider: Initialize with Bearer auth
LocalProvider->>LocalProvider: Use API key for requests
LocalProvider-->>User: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
scratch.js (1)
1-3:⚠️ Potential issue | 🔴 CriticalRemove scratch file from the PR.
This appears to be a development/debug scratch file that was accidentally committed. The file has no functionality beyond an import and an exploratory comment, and it's not related to the PR's stated objective of adding API key support for OpenAI-compatible providers.
Additionally, the import path
./app/src/services/coreRpcClient.tswon't work correctly from the repository root in a JavaScript file.🗑️ Proposed fix
Delete this file entirely, or if it's needed for local development, add it to
.gitignore:+# Local development scratch files +scratch.js🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scratch.js` around lines 1 - 3, Remove the accidental development scratch file scratch.js from the PR; delete the file (or if you want to keep it for personal local experiments, move it out of the repo and add its path to .gitignore) and ensure any stray import statements like callCoreRpc from ./app/src/services/coreRpcClient.ts are not left in the tree because that relative TS import is invalid from a JS file at the repo root.src/openhuman/tools/impl/agent/delegate.rs (1)
187-198:⚠️ Potential issue | 🟠 MajorDelegate tool passes
Nonefor api_key, breaking authentication for custom OpenAI-compatible backends.The
DelegateToolstruct has noapi_keyfield and its constructors don't accept one. At line 187–189,execute()callscreate_backend_inference_provider(None, None, ...), hardcoding a null api_key. Every other call site (triage/routing, linkedin_enrichment) correctly passesconfig.api_key.as_deref(). Any user configuring a custom backend with required authentication will see delegation fail with auth errors while non-delegate inference succeeds.Add an
api_key: Option<String>field toDelegateTool, thread it throughnew_with_options()andwith_depth_and_options()constructors, and passself.api_key.as_deref()to the provider factory at line 188. Update the caller intools/ops.rsto extract and forward the api_key from available config context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/tools/impl/agent/delegate.rs` around lines 187 - 198, Add an api_key: Option<String> field to DelegateTool and thread it through its constructors (new_with_options and with_depth_and_options), ensuring callers set it from the config context in tools/ops.rs; then replace the hardcoded None in execute() when calling providers::create_backend_inference_provider with self.api_key.as_deref() so the provider factory receives the configured API key; ensure any constructor signatures and the call site in tools/ops.rs are updated to accept and forward the api_key from the available config.src/openhuman/routing/factory.rs (1)
43-74:⚠️ Potential issue | 🟡 MinorRun
cargo fmton this hunk before merge.CI is already failing
cargo fmt --all -- --checkhere on the newmatches!/if-elseformatting, so this will stay red until the file is reformatted.As per coding guidelines, "
**/*.rs: Runcargo fmtandcargo checkpre-merge for Rust files at both repo root (Cargo.toml) andapp/src-tauri/Cargo.toml" and "**/*.rs: Rust code must followcargo fmtformatting standards before merging."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/routing/factory.rs` around lines 43 - 74, Run rustfmt/cargo fmt on this hunk to fix formatting around the matches! and if/else block; specifically reformat the code that computes use_openai_compat_local and the subsequent let (provider_label, local_base, health) = ... expression (which references override_base, provider_kind, local_ai_config, ollama_base, LocalHealthChecker) so it matches cargo fmt style. Execute `cargo fmt --all` (or rustfmt on src/openhuman/routing/factory.rs) and recommit the changed file; no logical changes required, only formatting.src/openhuman/config/ops.rs (1)
228-246:⚠️ Potential issue | 🟠 MajorDo not echo the stored API key back through config snapshots.
This now persists
config.api_keyand immediately returnssnapshot_config_json(config), so bothupdate_model_settingsand the existing config snapshot flow can expose the raw credential to RPC consumers. The new UI already hydratesresult.config.api_keyback into renderer state, which turns this into a full secret round-trip. Please keep API keys write-only here and return only a redacted/configured signal; if the UI still needs “unchanged vs clear vs replace” behavior, the patch contract needs tri-state semantics instead of relying on reading the saved key back.Based on learnings, "New or changed behavior must be accompanied by matching rustdoc or code comments. Update
AGENTS.mdor architecture docs when rules or user-visible behavior change." Also, as per coding guidelines, "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/config/ops.rs` around lines 228 - 246, The current code saves the real API key into config and then calls snapshot_config_json(config), which returns the raw secret; instead, after persisting via config.save() keep the real key in storage but create a sanitized copy for snapshots: clone the config (or construct a sanitized Config), clear or replace config.api_key on that copy (e.g., set sanitized.api_key = None or Some("[REDACTED]")), and pass the sanitized copy to snapshot_config_json instead of the real config; reference update.api_key, config.api_key, config.save(), and snapshot_config_json in your changes.src/openhuman/providers/ops.rs (1)
331-343:⚠️ Potential issue | 🟠 MajorAdd a unit test for the new
api_keybranch.
factory_backendonly exercises the(None, None)path, which routes to the existingOpenHumanBackendProvider. The new OpenAI-compatible branch (bothapi_urlandapi_keyset) is entirely uncovered, so a future regression that swaps the arms or changes the provider id / auth style would not be caught. Please add at least one test that constructs the provider with both args populated and asserts success (and ideally asserts the returned provider's id / type when reachable).🧪 Proposed additional test
#[cfg(test)] mod tests { use super::*; #[test] fn factory_backend() { assert!(create_backend_inference_provider( None, None, &ProviderRuntimeOptions::default() ) .is_ok()); } + + #[test] + fn factory_backend_custom_openai_compatible() { + // Both api_url and api_key set → should take the OpenAI-compatible branch. + assert!(create_backend_inference_provider( + Some("https://example.invalid/v1"), + Some("test-key"), + &ProviderRuntimeOptions::default(), + ) + .is_ok()); + } + + #[test] + fn factory_backend_api_key_without_url_falls_back() { + // Key without URL must not panic and must return a provider (fallback). + assert!(create_backend_inference_provider( + None, + Some("test-key"), + &ProviderRuntimeOptions::default(), + ) + .is_ok()); + } }As per coding guidelines: "Ship unit tests and coverage for behavior you are adding or changing before building additional features on top".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/providers/ops.rs` around lines 331 - 343, Add a unit test that covers the new OpenAI-compatible branch by calling create_backend_inference_provider with both Some(api_url) and Some(api_key) (and a ProviderRuntimeOptions::default()) and asserting the Result is Ok; additionally, downcast or inspect the returned provider to verify its identity/type or id matches the OpenAI-compatible provider (reference create_backend_inference_provider and ProviderRuntimeOptions to locate the factory and the provider type/id used in the new branch), ensuring the test lives alongside the existing factory_backend test in the #[cfg(test)] mod.
🧹 Nitpick comments (1)
src/openhuman/providers/ops.rs (1)
156-162: Consider promoting the hardcoded"custom_openai"id to a constant.Both the provider id string and the
AuthStyle::Bearerchoice are inlined here. SinceINFERENCE_BACKEND_IDis already defined at Line 8 for the OpenHuman backend, defining a siblingpub const CUSTOM_OPENAI_BACKEND_ID: &str = "custom_openai";would keep the two ids colocated and make future references (metrics, routing tables, fallback lookups) grep-friendly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/providers/ops.rs` around lines 156 - 162, Introduce a sibling public constant for the hardcoded provider id and use it in the OpenAiCompatibleProvider instantiation: add pub const CUSTOM_OPENAI_BACKEND_ID: &str = "custom_openai"; near the existing INFERENCE_BACKEND_ID, then replace the inline "custom_openai" in the call to OpenAiCompatibleProvider::new with CUSTOM_OPENAI_BACKEND_ID; keep the AuthStyle::Bearer usage as-is but ensure the new constant is exported so other modules (metrics, routing, fallbacks) can reference it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/settings/panels/local-model/CustomModelSection.tsx`:
- Around line 7-112: Add a colocated Vitest file CustomModelSection.test.tsx
that imports CustomModelSection and tests its key behaviors: mock
openhumanGetConfig to return initial config and assert that apiUrl/apiKey are
populated on mount and loading state toggles; mock openhumanGetConfig to throw
and assert error display; test handleSave by mocking
openhumanUpdateModelSettings and asserting that blank inputs are sent as null
(normalization), that isSaving disables the button and that the transient
saveSuccess message appears then disappears (use fake timers to advance 3s);
also test save failure by having openhumanUpdateModelSettings reject and
asserting the error message is shown; use vi.mock to stub
openhumanGetConfig/openhumanUpdateModelSettings and `@testing-library/react` for
render/userEvent queries.
In `@app/src/components/settings/panels/LocalModelDebugPanel.tsx`:
- Around line 36-38: Imports from ./local-model are not alphabetized and there's
an extra blank line in the JSX which causes Prettier to fail; reorder the
imports so ModelDownloadSection, ModelStatusSection, CustomModelSection are
alphabetically sorted (place CustomModelSection in the correct spot), remove the
stray blank line before the new JSX element near the bottom of
LocalModelDebugPanel.tsx, then run pnpm prettier --write on the file (and
optionally tsc --noEmit in app/) to ensure style/type checks pass.
In `@src/openhuman/config/schema/local_ai.rs`:
- Around line 14-15: LocalAiConfig currently derives Debug and exposes the
sensitive api_key field; implement a custom Debug impl for LocalAiConfig (or
remove Debug from the derive) so that api_key is redacted (e.g., always print
api_key: "***") instead of its raw value; update the impl to reference the
struct name LocalAiConfig and the api_key field so any debug printing (for
example via tracing::debug!(?config.local_ai)) will not leak the API key.
In `@src/openhuman/providers/ops.rs`:
- Around line 153-176: Formatting in create_resilient_provider and surrounding
calls (notably the OpenAiCompatibleProvider::new call and the
create_resilient_provider_with_options invocation) is off and failing CI; run
cargo fmt across the workspace (including all changed Rust files and workspace
Cargo.toml manifests) and reformat the code so create_resilient_provider,
create_resilient_provider_with_options, OpenAiCompatibleProvider::new, and
ProviderRuntimeOptions::default usages follow rustfmt styling, then re-run the
formatter check before pushing.
- Around line 150-168: Update the create_backend_inference_provider doc comment
to reflect that providing api_key + api_url enables a Bearer-authenticated
OpenAI-compatible custom endpoint; ensure ProviderRuntimeOptions is not silently
ignored on the custom branch by either documenting that relevant fields (e.g.
reasoning_enabled, auth_profile_override) are not used or by threading required
fields into OpenAiCompatibleProvider::new; add a tracing::warn! when exactly one
of api_url or api_key is Some to avoid silent misconfiguration; add
tracing::debug! logs on each branch decision — when selecting the custom
endpoint log the URL host (never the key) and when falling back log that
OpenHuman backend is used; and add a unit test in the tests module that calls
create_backend_inference_provider(Some("https://api.example.com"),
Some("sk-test"), &ProviderRuntimeOptions::default()) and asserts the result is
Ok() to cover the custom api_key path (use OpenAiCompatibleProvider::new and
openhuman_backend::OpenHumanBackendProvider::new as reference points).
---
Outside diff comments:
In `@scratch.js`:
- Around line 1-3: Remove the accidental development scratch file scratch.js
from the PR; delete the file (or if you want to keep it for personal local
experiments, move it out of the repo and add its path to .gitignore) and ensure
any stray import statements like callCoreRpc from
./app/src/services/coreRpcClient.ts are not left in the tree because that
relative TS import is invalid from a JS file at the repo root.
In `@src/openhuman/config/ops.rs`:
- Around line 228-246: The current code saves the real API key into config and
then calls snapshot_config_json(config), which returns the raw secret; instead,
after persisting via config.save() keep the real key in storage but create a
sanitized copy for snapshots: clone the config (or construct a sanitized
Config), clear or replace config.api_key on that copy (e.g., set
sanitized.api_key = None or Some("[REDACTED]")), and pass the sanitized copy to
snapshot_config_json instead of the real config; reference update.api_key,
config.api_key, config.save(), and snapshot_config_json in your changes.
In `@src/openhuman/providers/ops.rs`:
- Around line 331-343: Add a unit test that covers the new OpenAI-compatible
branch by calling create_backend_inference_provider with both Some(api_url) and
Some(api_key) (and a ProviderRuntimeOptions::default()) and asserting the Result
is Ok; additionally, downcast or inspect the returned provider to verify its
identity/type or id matches the OpenAI-compatible provider (reference
create_backend_inference_provider and ProviderRuntimeOptions to locate the
factory and the provider type/id used in the new branch), ensuring the test
lives alongside the existing factory_backend test in the #[cfg(test)] mod.
In `@src/openhuman/routing/factory.rs`:
- Around line 43-74: Run rustfmt/cargo fmt on this hunk to fix formatting around
the matches! and if/else block; specifically reformat the code that computes
use_openai_compat_local and the subsequent let (provider_label, local_base,
health) = ... expression (which references override_base, provider_kind,
local_ai_config, ollama_base, LocalHealthChecker) so it matches cargo fmt style.
Execute `cargo fmt --all` (or rustfmt on src/openhuman/routing/factory.rs) and
recommit the changed file; no logical changes required, only formatting.
In `@src/openhuman/tools/impl/agent/delegate.rs`:
- Around line 187-198: Add an api_key: Option<String> field to DelegateTool and
thread it through its constructors (new_with_options and
with_depth_and_options), ensuring callers set it from the config context in
tools/ops.rs; then replace the hardcoded None in execute() when calling
providers::create_backend_inference_provider with self.api_key.as_deref() so the
provider factory receives the configured API key; ensure any constructor
signatures and the call site in tools/ops.rs are updated to accept and forward
the api_key from the available config.
---
Nitpick comments:
In `@src/openhuman/providers/ops.rs`:
- Around line 156-162: Introduce a sibling public constant for the hardcoded
provider id and use it in the OpenAiCompatibleProvider instantiation: add pub
const CUSTOM_OPENAI_BACKEND_ID: &str = "custom_openai"; near the existing
INFERENCE_BACKEND_ID, then replace the inline "custom_openai" in the call to
OpenAiCompatibleProvider::new with CUSTOM_OPENAI_BACKEND_ID; keep the
AuthStyle::Bearer usage as-is but ensure the new constant is exported so other
modules (metrics, routing, fallbacks) can reference it.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc94b6a9-2e47-4b51-835b-35a34af21b69
📒 Files selected for processing (18)
app/src/components/settings/panels/LocalModelDebugPanel.tsxapp/src/components/settings/panels/local-model/CustomModelSection.tsxapp/src/utils/tauriCommands/config.tsscratch.jssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/triage/routing.rssrc/openhuman/channels/routes.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/config/ops.rssrc/openhuman/config/schema/local_ai.rssrc/openhuman/config/schema/types.rssrc/openhuman/config/schemas.rssrc/openhuman/learning/linkedin_enrichment.rssrc/openhuman/local_ai/ops.rssrc/openhuman/providers/ops.rssrc/openhuman/routing/factory.rssrc/openhuman/threads/ops.rssrc/openhuman/tools/impl/agent/delegate.rs
| const CustomModelSection = () => { | ||
| const [apiUrl, setApiUrl] = useState(''); | ||
| const [apiKey, setApiKey] = useState(''); | ||
| const [isLoading, setIsLoading] = useState(true); | ||
| const [isSaving, setIsSaving] = useState(false); | ||
| const [error, setError] = useState(''); | ||
| const [saveSuccess, setSaveSuccess] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| let mounted = true; | ||
| const fetchConfig = async () => { | ||
| try { | ||
| const { result } = await openhumanGetConfig(); | ||
| if (mounted) { | ||
| setApiUrl(String(result.config.api_url || '')); | ||
| setApiKey(String(result.config.api_key || '')); | ||
| } | ||
| } catch (err) { | ||
| if (mounted) { | ||
| setError(err instanceof Error ? err.message : 'Failed to load custom backend config'); | ||
| } | ||
| } finally { | ||
| if (mounted) setIsLoading(false); | ||
| } | ||
| }; | ||
| void fetchConfig(); | ||
| return () => { | ||
| mounted = false; | ||
| }; | ||
| }, []); | ||
|
|
||
| const handleSave = async () => { | ||
| setIsSaving(true); | ||
| setError(''); | ||
| setSaveSuccess(false); | ||
| try { | ||
| await openhumanUpdateModelSettings({ | ||
| api_url: apiUrl.trim() || null, | ||
| api_key: apiKey.trim() || null, | ||
| }); | ||
| setSaveSuccess(true); | ||
| setTimeout(() => setSaveSuccess(false), 3000); | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : 'Failed to save custom backend settings'); | ||
| } finally { | ||
| setIsSaving(false); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <section className="space-y-3"> | ||
| <h3 className="text-sm font-semibold text-stone-900">Custom Provider (OpenAI Compatible)</h3> | ||
| <div className="bg-stone-50 rounded-lg border border-stone-200 p-4 space-y-4"> | ||
| <p className="text-xs text-stone-500"> | ||
| Configure a custom OpenAI-compatible backend (like vLLM or LiteLLM). If set, this will route requests to your custom endpoint instead of the standard local or built-in backends. | ||
| </p> | ||
|
|
||
| {isLoading ? ( | ||
| <div className="text-sm text-stone-500 animate-pulse">Loading settings…</div> | ||
| ) : ( | ||
| <div className="space-y-3"> | ||
| <div> | ||
| <label className="block text-xs font-medium text-stone-700 mb-1"> | ||
| Base URL | ||
| </label> | ||
| <input | ||
| type="text" | ||
| value={apiUrl} | ||
| onChange={e => setApiUrl(e.target.value)} | ||
| placeholder="http://localhost:8000/v1" | ||
| className="w-full rounded-md border border-stone-300 bg-white px-3 py-2 text-sm text-stone-900 focus:border-primary-500 focus:outline-none focus:ring-1 focus:ring-primary-500" | ||
| /> | ||
| </div> | ||
| <div> | ||
| <label className="block text-xs font-medium text-stone-700 mb-1"> | ||
| API Key | ||
| </label> | ||
| <input | ||
| type="password" | ||
| value={apiKey} | ||
| onChange={e => setApiKey(e.target.value)} | ||
| placeholder="sk-..." | ||
| className="w-full rounded-md border border-stone-300 bg-white px-3 py-2 text-sm text-stone-900 focus:border-primary-500 focus:outline-none focus:ring-1 focus:ring-primary-500" | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="flex items-center gap-3 pt-2"> | ||
| <button | ||
| type="button" | ||
| onClick={handleSave} | ||
| disabled={isSaving} | ||
| className="rounded-lg bg-primary-600 px-4 py-2 text-sm font-medium text-white hover:bg-primary-700 focus:outline-none disabled:opacity-50"> | ||
| {isSaving ? 'Saving…' : 'Save Config'} | ||
| </button> | ||
| {saveSuccess && <span className="text-xs text-green-600">Saved successfully.</span>} | ||
| </div> | ||
|
|
||
| {error && <div className="text-xs text-red-600 mt-2">{error}</div>} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </section> | ||
| ); | ||
| }; | ||
|
|
||
| export default CustomModelSection; |
There was a problem hiding this comment.
Please add a colocated Vitest for this new settings flow.
This component adds non-trivial behavior around initial load, blank-to-null normalization, save failures, and the transient success state, but there isn’t a CustomModelSection.test.tsx covering those paths in the changes here. This feature is now part of credential/config handling, so it’s worth pinning down before it ships.
As per coding guidelines, "Ship unit tests and coverage for behavior you are adding or changing before building additional features on top" and "app/src/**/*.{ts,tsx}: Co-locate unit tests as *.test.ts or *.test.tsx under app/src/**; use Vitest with app/test/vitest.config.ts."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/local-model/CustomModelSection.tsx` around
lines 7 - 112, Add a colocated Vitest file CustomModelSection.test.tsx that
imports CustomModelSection and tests its key behaviors: mock openhumanGetConfig
to return initial config and assert that apiUrl/apiKey are populated on mount
and loading state toggles; mock openhumanGetConfig to throw and assert error
display; test handleSave by mocking openhumanUpdateModelSettings and asserting
that blank inputs are sent as null (normalization), that isSaving disables the
button and that the transient saveSuccess message appears then disappears (use
fake timers to advance 3s); also test save failure by having
openhumanUpdateModelSettings reject and asserting the error message is shown;
use vi.mock to stub openhumanGetConfig/openhumanUpdateModelSettings and
`@testing-library/react` for render/userEvent queries.
| import ModelDownloadSection from './local-model/ModelDownloadSection'; | ||
| import ModelStatusSection from './local-model/ModelStatusSection'; | ||
| import CustomModelSection from './local-model/CustomModelSection'; |
There was a problem hiding this comment.
Prettier check is failing on this file.
Pipeline reports Prettier --check style issues. The new CustomModelSection import is out of alphabetical order relative to the adjacent ./local-model/* imports and there's a stray blank line before the new element at L447–448. Run pnpm prettier --write on this file before merging.
🎨 Likely fix
-import ModelDownloadSection from './local-model/ModelDownloadSection';
-import ModelStatusSection from './local-model/ModelStatusSection';
-import CustomModelSection from './local-model/CustomModelSection';
+import CustomModelSection from './local-model/CustomModelSection';
+import ModelDownloadSection from './local-model/ModelDownloadSection';
+import ModelStatusSection from './local-model/ModelStatusSection'; />
-
-
- <CustomModelSection />
+
+ <CustomModelSection />As per coding guidelines: "Run Prettier and ESLint pre-merge; use tsc --noEmit in app/ to check for type errors".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/settings/panels/LocalModelDebugPanel.tsx` around lines 36
- 38, Imports from ./local-model are not alphabetized and there's an extra blank
line in the JSX which causes Prettier to fail; reorder the imports so
ModelDownloadSection, ModelStatusSection, CustomModelSection are alphabetically
sorted (place CustomModelSection in the correct spot), remove the stray blank
line before the new JSX element near the bottom of LocalModelDebugPanel.tsx,
then run pnpm prettier --write on the file (and optionally tsc --noEmit in app/)
to ensure style/type checks pass.
| #[serde(default)] | ||
| pub api_key: Option<String>, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find any Debug/tracing prints of LocalAiConfig or config.local_ai that could exfiltrate the new api_key.
rg -nP --type=rust -C2 '\blocal_ai\b.*\{:\?\}|\?.*local_ai|tracing::\w+!\([^)]*local_ai'
rg -nP --type=rust -C2 '\{:\?\}.*LocalAiConfig|LocalAiConfig.*\{:\?\}'Repository: tinyhumansai/openhuman
Length of output: 8781
🏁 Script executed:
# Find the LocalAiConfig struct definition and its derives
rg -nA 20 'struct LocalAiConfig' src/openhuman/config/schema/local_ai.rsRepository: tinyhumansai/openhuman
Length of output: 888
🏁 Script executed:
# Check if there's a custom Debug impl for LocalAiConfig
rg -nA 10 'impl.*Debug.*LocalAiConfig|impl.*LocalAiConfig.*Debug' src/openhuman/config/schema/Repository: tinyhumansai/openhuman
Length of output: 48
🏁 Script executed:
# More targeted search for {:?} debug prints of the entire config.local_ai
rg -nP --type=rust 'tracing::\w+!\([^)]*\{:\?\}[^)]*local_ai|local_ai[^)]*\{:\?\}'Repository: tinyhumansai/openhuman
Length of output: 259
🏁 Script executed:
# Get the full struct definition including derives
rg -nB 5 'pub struct LocalAiConfig' src/openhuman/config/schema/local_ai.rs | head -20Repository: tinyhumansai/openhuman
Length of output: 230
🏁 Script executed:
# Check what service_state is at that line in ops.rs
rg -nB 10 -A 5 'service_state=\{:\?\}' src/openhuman/app_state/ops.rsRepository: tinyhumansai/openhuman
Length of output: 828
🏁 Script executed:
# Search for any debug macro invocations that might dump config.local_ai or LocalAiConfig with {:?}
rg -nP --type=rust 'debug!\([^)]*\?[^)]*local_ai|tracing::debug!\([^)]*\?[^)]*local_ai'Repository: tinyhumansai/openhuman
Length of output: 328
🏁 Script executed:
# Also check if LocalAiConfig is ever logged anywhere with the ? shorthand
rg -nP --type=rust '\?\s*config\.local_ai|\?\s*local_ai\b'Repository: tinyhumansai/openhuman
Length of output: 174
🏁 Script executed:
# Check if there are any existing redacting newtypes or Secret wrappers in the codebase
rg -nA 5 'struct Secret|newtype.*secret|RedactedString' src/Repository: tinyhumansai/openhuman
Length of output: 835
Implement a custom Debug impl or skip the Debug derive for LocalAiConfig to redact the api_key field.
LocalAiConfig derives Debug and contains pub api_key: Option<String>. While no current debug prints of the full struct exist, the derived Debug impl will expose the raw API key if ever used in tracing::debug!(?config.local_ai) or similar debug output. Per coding guidelines: "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields."
Add a custom Debug impl that prints api_key: "***", or remove Debug from the derive macro if it's not needed elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/config/schema/local_ai.rs` around lines 14 - 15, LocalAiConfig
currently derives Debug and exposes the sensitive api_key field; implement a
custom Debug impl for LocalAiConfig (or remove Debug from the derive) so that
api_key is redacted (e.g., always print api_key: "***") instead of its raw
value; update the impl to reference the struct name LocalAiConfig and the
api_key field so any debug printing (for example via
tracing::debug!(?config.local_ai)) will not leak the API key.
| /// Create the OpenHuman backend inference client (session JWT only). | ||
| pub fn create_backend_inference_provider( | ||
| api_url: Option<&str>, | ||
| api_key: Option<&str>, | ||
| options: &ProviderRuntimeOptions, | ||
| ) -> anyhow::Result<Box<dyn Provider>> { | ||
| Ok(Box::new(openhuman_backend::OpenHumanBackendProvider::new( | ||
| api_url, options, | ||
| ))) | ||
| if let (Some(url), Some(key)) = (api_url, api_key) { | ||
| Ok(Box::new(crate::openhuman::providers::compatible::OpenAiCompatibleProvider::new( | ||
| "custom_openai", | ||
| url, | ||
| Some(key), | ||
| crate::openhuman::providers::compatible::AuthStyle::Bearer, | ||
| ))) | ||
| } else { | ||
| Ok(Box::new(openhuman_backend::OpenHumanBackendProvider::new( | ||
| api_url, options, | ||
| ))) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect OpenAiCompatibleProvider::new signature to confirm whether
# ProviderRuntimeOptions fields (reasoning_enabled, auth_profile_override,
# secrets_encrypt, openhuman_dir) have a counterpart in the compatible provider.
fd -e rs compatible
ast-grep --pattern $'impl OpenAiCompatibleProvider {
$$$
pub fn new($$$) -> $_ {
$$$
}
$$$
}'
ast-grep --pattern $'pub enum AuthStyle {
$$$
}'Repository: tinyhumansai/openhuman
Length of output: 17603
🏁 Script executed:
#!/bin/bash
# Locate ops.rs and inspect lines 145-180 to verify the code structure and logging
fd -type f -name "ops.rs" | head -5Repository: tinyhumansai/openhuman
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Read ops.rs to confirm lines 150-168 match the review's snippet
if fd -type f -name "ops.rs" | grep -q "src/openhuman/providers/ops.rs"; then
echo "=== ops.rs lines 145-180 ==="
sed -n '145,180p' src/openhuman/providers/ops.rs
echo ""
echo "=== ops.rs lines 330-350 (test area) ==="
sed -n '330,350p' src/openhuman/providers/ops.rs
fiRepository: tinyhumansai/openhuman
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Check file size of ops.rs
wc -l src/openhuman/providers/ops.rsRepository: tinyhumansai/openhuman
Length of output: 101
🏁 Script executed:
#!/bin/bash
# Correct: read ops.rs lines 145-180
sed -n '145,180p' src/openhuman/providers/ops.rsRepository: tinyhumansai/openhuman
Length of output: 1439
🏁 Script executed:
#!/bin/bash
# Check test area lines 330-350
sed -n '330,350p' src/openhuman/providers/ops.rsRepository: tinyhumansai/openhuman
Length of output: 321
🏁 Script executed:
#!/bin/bash
# Check if there are any tracing/logging statements in the file
rg "tracing::" src/openhuman/providers/ops.rsRepository: tinyhumansai/openhuman
Length of output: 90
Update rustdoc, add branch-decision logging, handle unused options parameter explicitly, and provide test coverage for the custom API endpoint path.
This newly-branched factory has several gaps:
-
Stale rustdoc (line 150) — The comment still says "session JWT only", but the function can now return
OpenAiCompatibleProvider. Update to document thatapi_keyenables Bearer-authenticated custom OpenAI-compatible endpoints. -
options: &ProviderRuntimeOptionsis silently ignored on the custom path (lines 156–162). Settings likereasoning_enabledandauth_profile_overridehave no effect when a custom endpoint is configured. Either add a comment acknowledging this or thread relevant fields intoOpenAiCompatibleProvider::new()if that becomes a requirement. -
api_keyis silently discarded whenapi_urlisNone(and vice versa). A user who provides a key without the URL will get the default OpenHuman backend with no warning. Add atracing::warn!when exactly one of the pair is provided, to prevent silent misconfiguration. -
Missing branch-decision logging. Per coding guidelines, branch decisions require development-oriented logs with grep-friendly prefixes. Add
tracing::debug!when selecting the custom endpoint (recording the URL host, never the key) and when falling back to OpenHuman backend. -
No test coverage for the new custom api_key path. The only test (lines 335–343) covers
(None, None). Add a test for the(Some(url), Some(key))branch.
🧩 Example shape
-/// Create the OpenHuman backend inference client (session JWT only).
+/// Create an inference backend provider.
+///
+/// * If both `api_url` and `api_key` are provided, returns an
+/// [`OpenAiCompatibleProvider`] with Bearer authentication.
+/// * Otherwise, returns an [`OpenHumanBackendProvider`]. Note: `ProviderRuntimeOptions`
+/// only applies to the OpenHuman backend path.
pub fn create_backend_inference_provider(
api_url: Option<&str>,
api_key: Option<&str>,
options: &ProviderRuntimeOptions,
) -> anyhow::Result<Box<dyn Provider>> {
+ if api_key.is_some() != api_url.is_some() {
+ tracing::warn!(
+ target: "providers",
+ "[providers] api_key/api_url mismatch; ignoring partial config and using OpenHuman backend"
+ );
+ }
if let (Some(url), Some(key)) = (api_url, api_key) {
+ tracing::debug!(
+ target: "providers",
+ provider = "custom_openai",
+ "[providers] selecting custom OpenAI-compatible backend"
+ );
Ok(Box::new(crate::openhuman::providers::compatible::OpenAiCompatibleProvider::new(
"custom_openai",
url,
Some(key),
crate::openhuman::providers::compatible::AuthStyle::Bearer,
)))
} else {
+ tracing::debug!(
+ target: "providers",
+ "[providers] selecting OpenHuman backend"
+ );
Ok(Box::new(openhuman_backend::OpenHumanBackendProvider::new(
api_url, options,
)))
}
}For the test, add to the tests module:
#[test]
fn factory_backend_custom_api() {
let result = create_backend_inference_provider(
Some("https://api.example.com"),
Some("sk-test"),
&ProviderRuntimeOptions::default(),
);
assert!(result.is_ok());
}🧰 Tools
🪛 GitHub Actions: Type Check
[error] 154-154: cargo fmt --all -- --check failed due to formatting differences (OpenAiCompatibleProvider::new call indentation/line wrapping).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/providers/ops.rs` around lines 150 - 168, Update the
create_backend_inference_provider doc comment to reflect that providing api_key
+ api_url enables a Bearer-authenticated OpenAI-compatible custom endpoint;
ensure ProviderRuntimeOptions is not silently ignored on the custom branch by
either documenting that relevant fields (e.g. reasoning_enabled,
auth_profile_override) are not used or by threading required fields into
OpenAiCompatibleProvider::new; add a tracing::warn! when exactly one of api_url
or api_key is Some to avoid silent misconfiguration; add tracing::debug! logs on
each branch decision — when selecting the custom endpoint log the URL host
(never the key) and when falling back log that OpenHuman backend is used; and
add a unit test in the tests module that calls
create_backend_inference_provider(Some("https://api.example.com"),
Some("sk-test"), &ProviderRuntimeOptions::default()) and asserts the result is
Ok() to cover the custom api_key path (use OpenAiCompatibleProvider::new and
openhuman_backend::OpenHumanBackendProvider::new as reference points).
| api_key: Option<&str>, | ||
| options: &ProviderRuntimeOptions, | ||
| ) -> anyhow::Result<Box<dyn Provider>> { | ||
| Ok(Box::new(openhuman_backend::OpenHumanBackendProvider::new( | ||
| api_url, options, | ||
| ))) | ||
| if let (Some(url), Some(key)) = (api_url, api_key) { | ||
| Ok(Box::new(crate::openhuman::providers::compatible::OpenAiCompatibleProvider::new( | ||
| "custom_openai", | ||
| url, | ||
| Some(key), | ||
| crate::openhuman::providers::compatible::AuthStyle::Bearer, | ||
| ))) | ||
| } else { | ||
| Ok(Box::new(openhuman_backend::OpenHumanBackendProvider::new( | ||
| api_url, options, | ||
| ))) | ||
| } | ||
| } | ||
|
|
||
| /// Create provider chain with retry and fallback behavior. | ||
| pub fn create_resilient_provider( | ||
| api_url: Option<&str>, | ||
| api_key: Option<&str>, | ||
| reliability: &crate::openhuman::config::ReliabilityConfig, | ||
| ) -> anyhow::Result<Box<dyn Provider>> { | ||
| create_resilient_provider_with_options(api_url, reliability, &ProviderRuntimeOptions::default()) | ||
| create_resilient_provider_with_options(api_url, api_key, reliability, &ProviderRuntimeOptions::default()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Run cargo fmt — CI is red on this file.
The Type Check job fails on cargo fmt --all -- --check for lines 154, 173, and 334, which maps to the OpenAiCompatibleProvider::new call wrapping (Lines 157–162), the create_resilient_provider_with_options call on Line 176, and the test-assertion wrapping (Lines 337–342). Please run cargo fmt at both Cargo.toml and app/src-tauri/Cargo.toml before pushing.
As per coding guidelines: "Rust code must follow cargo fmt formatting standards before merging. Run cargo fmt on all changed Rust files."
Also applies to: 337-340
🧰 Tools
🪛 GitHub Actions: Type Check
[error] 154-154: cargo fmt --all -- --check failed due to formatting differences (OpenAiCompatibleProvider::new call indentation/line wrapping).
[error] 173-173: cargo fmt --all -- --check failed due to formatting differences (create_resilient_provider_with_options call line wrapping).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/providers/ops.rs` around lines 153 - 176, Formatting in
create_resilient_provider and surrounding calls (notably the
OpenAiCompatibleProvider::new call and the
create_resilient_provider_with_options invocation) is off and failing CI; run
cargo fmt across the workspace (including all changed Rust files and workspace
Cargo.toml manifests) and reformat the code so create_resilient_provider,
create_resilient_provider_with_options, OpenAiCompatibleProvider::new, and
ProviderRuntimeOptions::default usages follow rustfmt styling, then re-run the
formatter check before pushing.
1 participant
…integrate into configuration workflows
Summary
Problem
Solution
Submission Checklist
app/) and/orcargo test(core) for logic you add or changeapp/test/e2e, mock backend,tests/json_rpc_e2e.rsas appropriate)//////!(Rust), JSDoc or brief file/module headers (TS) on public APIs and non-obvious modules(Any feature related checklist can go in here)
Impact
Related
Summary by CodeRabbit
Release Notes