Repository navigation
Saved AI providers (desktop + mobile) + shared model picker - #95
Conversation
Desktop: - Save named OpenAI-compatible providers (name/baseUrl/apiKey/model) and switch the active one; shared with the Coding Agent (one list, per-config active selection). Persisted to localStorage + Electron cache. - Extract the chat model dropdown into a shared ModelPicker reused in AI settings, Agent settings, and the provider form. - Drop hardcoded fallback model lists; cache fetched models per endpoint and auto-refresh when uncached. - Allow 127.0.0.1 loopback endpoints in the CSP (not just localhost). Mobile: - Saved OpenAI providers (meta DB list + per-provider keychain keys) with a one-time migration of the existing flat config; chip switcher in settings.
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis change adds saved OpenAI-compatible providers with separate AI and Coding Agent selections, migrates mobile configuration, introduces shared provider and model-picker controls, caches endpoint model lists, removes preset pills, and allows Electron loopback access through both ChangesProvider and model settings
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProviderManager
participant UISlice
participant AISettings
participant ModelPicker
User->>ProviderManager: Select saved provider
ProviderManager->>UISlice: Update active provider
UISlice-->>AISettings: Mirror base URL, key, and model
AISettings->>ModelPicker: Ensure endpoint models
ModelPicker-->>User: Display fetched or cached models
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 9
🧹 Nitpick comments (2)
mobile/src/chat/ai-config.ts (1)
186-190: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnguarded keychain writes reject into UI handlers.
Lines 188 and 216 call
SecureStore.setItemAsyncbare, while every delete path uses.catch(() => {}). Callers (addProvider,saveinmobile/src/components/AiSettingsForm.tsx) don't wrap these, so a keychain failure becomes an unhandled rejection with no user feedback. Either let the error surface deliberately (documented, and handled in the form) or keep it best-effort and return a success flag so the UI can report "key not saved".🤖 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 `@mobile/src/chat/ai-config.ts` around lines 186 - 190, Update the provider persistence flow around the SecureStore.setItemAsync calls in addProvider/save to handle keychain write failures explicitly: either propagate a documented error and handle it in AiSettingsForm, or make writes best-effort while returning a success indicator the UI can use to report that the key was not saved. Apply the same behavior to both write paths, including the path near line 216, while preserving existing delete handling.src/store/slices/ui.ts (1)
72-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate
setAIConfig/setAgentConfigontopersistAi/persistAgent.
persistAi/persistAgentduplicate logic already inlined insetAIConfig(Lines 350-353) andsetAgentConfig(Lines 433-436, unchanged). Beyond duplication, the two paths diverge:persistAgentcasts toRecord<string, unknown>before callingsaveAgentSettings(Line 84), butsetAgentConfig's inline call does not (Line 435). Worth confirming vianpm run type-check:allwhether one of these is actually wrong, then routing both setters through the shared helpers to remove the duplication and the discrepancy.♻️ Proposed refactor
setAIConfig(patch) { set((s) => { const next = { ...s.aiConfig, ...patch }; - storage.set(AI_CONFIG_KEY, next); - if (typeof window !== "undefined" && window.electron && window.electron.saveAiSettings) { - window.electron.saveAiSettings(next as unknown as Record<string, unknown>).catch(() => {}); - } + persistAi(next); return { aiConfig: next }; }); },setAgentConfig(patch) { set((s) => { const next = { ...s.agentConfig, ...patch }; - storage.set(AGENT_CONFIG_KEY, next); - if (typeof window !== "undefined" && window.electron && window.electron.saveAgentSettings) { - window.electron.saveAgentSettings(next).catch(() => {}); - } + persistAgent(next); return { agentConfig: next }; }); },As per coding guidelines,
**/*.ts: "Runnpm run type-check:allafter changes" — please verify the cast discrepancy noted above with that command.🤖 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/store/slices/ui.ts` around lines 72 - 86, Update setAIConfig and setAgentConfig to delegate persistence entirely to persistAi and persistAgent, removing their duplicated storage and Electron save logic while preserving existing state updates. Run npm run type-check:all to verify the saveAgentSettings cast discrepancy and retain the type-correct helper invocation.Source: Coding guidelines
🤖 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 `@electron/lib/config-cache.ts`:
- Around line 106-111: Update saved-provider persistence across
electron/lib/config-cache.ts lines 106-111 and src/store/slices/ui.ts lines
72-86 so provider apiKey values never enter localStorage or the plain JSON
ai-settings-cache.json. Reuse the existing OS-keychain-backed secret store used
for MCP/service secrets, storing only provider references in the renderer/cache
and resolving raw keys on the Electron side while preserving savedProviders and
activeProviderId behavior.
In `@electron/lib/protocol.ts`:
- Around line 130-131: Update the Content Security Policy directives in the
protocol configuration so loopback expansion remains limited to connect-src:
remove localhost/127.0.0.1 HTTP origins from default-src and script-src, while
preserving any existing entries required for actual renderer script sources.
In `@mobile/src/chat/ai-config.ts`:
- Around line 93-96: Update the SavedProvider type guard in the parsed provider
filter to require that p.model is a string, alongside id, name, and baseUrl,
before accepting the stored row. Preserve the existing filtering behavior for
all other fields.
- Around line 361-370: Update getOpenAIApiKey so getLegacyApiKey is used only
when no providers exist, preserving provider-specific lookup otherwise; do not
fall back to the legacy key when an active provider has no key, preventing
unrelated providers and cleared keys from inheriting it. Also remove
SECURE_KEY_APIKEY after successful migration so the legacy copy does not
persist.
- Around line 116-136: Update migrateToProvidersOnce so the
PROVIDERS_MIGRATION_FLAG read is inside its try/catch, allowing getSetting or
migration-source failures to be swallowed and retried later. Add a module-level
in-flight Promise guard so concurrent callers share one migration execution;
clear the guard when that execution settles, while preserving the existing
readProviders check, provider creation, legacy-key storage, and flag-setting
behavior.
In `@mobile/src/components/ai-settings/ProviderList.tsx`:
- Around line 49-53: Update the delete Pressable in ProviderList to expose an
accessibilityRole and descriptive accessibilityLabel, and require an Alert.alert
confirmation before invoking onDelete(p.id). Preserve the existing
selected-and-multiple-providers visibility condition and only delete after the
user confirms.
In `@mobile/src/components/AiSettingsForm.tsx`:
- Around line 165-188: Update switchProvider and addProvider to prevent
concurrent invocations with a shared in-flight guard or equivalent disabled
state, ensuring rapid interactions cannot persist edits to the wrong provider.
In switchProvider, after persisting the outgoing provider via setOpenAIEndpoint
and setOpenAIApiKey, refresh providers with listSavedProviders before selecting
or loading another provider so subsequent switches use current values.
- Around line 137-161: Update the loading effect in AiSettingsForm so failures
from listSavedProviders or subsequent provider/key reads are caught and always
call setLoading(false), while preserving the existing successful-load state
updates and cancellation guard. Ensure rejected loads cannot leave the form
stuck on the ActivityIndicator.
In `@src/store/slices/ui.ts`:
- Around line 60-70: The reconcileConfig function should return configs with an
empty activeProviderId unchanged, preventing unselected surfaces from inheriting
list[0] or another surface’s provider. Retain reconciliation for configs with a
selected provider, including fallback when that previously selected provider was
deleted, and apply the equivalent guard in mobile/src/chat/ai-config.ts if its
implementation matches this behavior.
---
Nitpick comments:
In `@mobile/src/chat/ai-config.ts`:
- Around line 186-190: Update the provider persistence flow around the
SecureStore.setItemAsync calls in addProvider/save to handle keychain write
failures explicitly: either propagate a documented error and handle it in
AiSettingsForm, or make writes best-effort while returning a success indicator
the UI can use to report that the key was not saved. Apply the same behavior to
both write paths, including the path near line 216, while preserving existing
delete handling.
In `@src/store/slices/ui.ts`:
- Around line 72-86: Update setAIConfig and setAgentConfig to delegate
persistence entirely to persistAi and persistAgent, removing their duplicated
storage and Electron save logic while preserving existing state updates. Run npm
run type-check:all to verify the saveAgentSettings cast discrepancy and retain
the type-correct helper invocation.
🪄 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 Plus
Run ID: a9b415fb-70e6-42c5-bdbd-368f046b7d8e
📒 Files selected for processing (15)
changelogs/v2.5.9.mdelectron/lib/config-cache.tselectron/lib/protocol.tsmobile/changelogs/v0.1.3.mdmobile/src/chat/ai-config.tsmobile/src/components/AiSettingsForm.tsxmobile/src/components/ai-settings/ProviderList.tsxmobile/src/components/ai-settings/styles.tssrc/components/chat/chat-panel/ChatQuickSettings.tsxsrc/components/settings/AISettings.tsxsrc/components/settings/AgentSettings.tsxsrc/components/settings/ProviderManager.tsxsrc/components/settings/endpoint-components.tsxsrc/components/ui/model-picker.tsxsrc/store/slices/ui.ts
| // Saved-provider switcher state (array + active id). `configRecord` is | ||
| // typed for scalars, so read the raw config object for these. | ||
| savedProviders: Array.isArray((config as { savedProviders?: unknown }).savedProviders) | ||
| ? (config as { savedProviders: NonNullable<CachedConfig["aiConfig"]>["savedProviders"] }).savedProviders | ||
| : current.aiConfig?.savedProviders, | ||
| activeProviderId: typeof configRecord.activeProviderId === "string" ? configRecord.activeProviderId : current.aiConfig?.activeProviderId, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo root files relevant =="
git ls-files | rg '(^|/)(config-cache\.ts|ui\.ts)$|(^|/)package\.json$|node_modules/next/dist/docs/' || true
echo
echo "== electron/lib/config-cache.ts outline =="
ast-grep outline electron/lib/config-cache.ts || true
echo
echo "== electron/lib/config-cache.ts sections =="
wc -l electron/lib/config-cache.ts
sed -n '1,180p' electron/lib/config-cache.ts
echo
echo "== src/store/slices/ui.ts sections =="
wc -l src/store/slices/ui.ts
sed -n '1,130p' src/store/slices/ui.ts
echo
echo "== search savedProviders definitions/usages =="
rg -n "savedProviders|activeProviderId|AI_CONFIG_KEY|saveAiSettings|safeStorage|encryptString|decryptString" -S .Repository: ddutchie/cairn
Length of output: 23053
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== storage and encryption implementation candidates =="
fd -a 'storage\.(ts|tsx|js)$|secure-store\.ts$|settings-handlers\.ts$' . | sed 's#^\./##'
echo
echo "== src/lib/storage.ts =="
wc -l src/lib/storage.ts
sed -n '1,220p' src/lib/storage.ts
echo
echo "== electron/lib/secure-store.ts relevant =="
wc -l electron/lib/secure-store.ts
sed -n '1,260p' electron/lib/secure-store.ts
echo
echo "== electron/ipc/settings-handlers.ts =="
wc -l electron/ipc/settings-handlers.ts
sed -n '1,120p' electron/ipc/settings-handlers.ts
echo
echo "== package metadata next/electron versions =="
node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('package.json','utf8'));
console.log({dependencies:p.dependencies?.next, devDependencies: p.devDependencies?.electron, engines:p.engines});
JSRepository: ddutchie/cairn
Length of output: 14080
Store saved-provider secrets through the encrypted store. saavedProviders carries apiKey, and persistAi() writes the full object to localStorage plus calls saveAiSettings(), which persists it to ai-settings-cache.json via plain JSON. Use the same OS-keychain-backed secret store used for MCP/service secrets, or store only provider references in localStorage/cache and keep the raw keys on the Electron side encrypted.
📍 Affects 2 files
electron/lib/config-cache.ts#L106-L111(this comment)src/store/slices/ui.ts#L72-L86
🤖 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 `@electron/lib/config-cache.ts` around lines 106 - 111, Update saved-provider
persistence across electron/lib/config-cache.ts lines 106-111 and
src/store/slices/ui.ts lines 72-86 so provider apiKey values never enter
localStorage or the plain JSON ai-settings-cache.json. Reuse the existing
OS-keychain-backed secret store used for MCP/service secrets, storing only
provider references in the renderer/cache and resolving raw keys on the Electron
side while preserving savedProviders and activeProviderId behavior.
There was a problem hiding this comment.
Will follow up with quick PR
There was a problem hiding this comment.
@ddutchie, sounds good. The finding will remain open until the follow-up PR moves saved-provider API keys out of renderer/plain-JSON persistence and into the encrypted secret store.
Would you like me to open a GitHub issue to track it?
- reconcileConfig: don't auto-select list[0] for an unselected surface - setAIConfig/setAgentConfig delegate to persistAi/persistAgent (dedupe) - CSP: scope 127.0.0.1 loopback to connect-src only (drop from default/script-src) - mobile: require model string in provider type guard - mobile: getOpenAIApiKey no longer inherits legacy key when a provider is active; migration deletes the legacy keychain key - mobile: migrateToProvidersOnce reads flag inside try + shared in-flight guard - mobile: ProviderList delete confirmation + a11y labels - mobile: switchProvider/addProvider concurrency guard; refresh list before switch - mobile: loading effect always clears the spinner on failure
1 participant
What does this PR do?
Adds saveable, switchable AI providers on desktop and mobile: users can store several named OpenAI-compatible connections (name, base URL, API key, model) and switch the active one instead of retyping their endpoint. On desktop the saved-provider list is shared between AI Chat and the Coding Agent (each keeps its own active selection), the chat model dropdown is extracted into a shared
ModelPickerreused across the settings pages, and model lists are fetched/cached per endpoint (no more hardcoded fallback model names). Also fixes local endpoints failing on127.0.0.1and renames the provider toggles to OpenAI / On-Device.Type of change
Screenshots / recording
Checklist
npm run type-check:allpassesnpm run lintpassesnpm testpasses (runsnpm run compilefirst soelectron/bundle-guard.test.tsactually executes —npm run test:bundleto run just that)npm run test:e2epasses (run before merging UI changes or cutting a release)var(--accent),var(--text-primary), etc.)text-[Npx]pixel font classes — rem equivalents only (text-[0.714rem],text-xs, etc.)handle()and returnIpcResult<T>— n/a (no new IPC handlers)schema.ts— n/a (config persists to localStorage + the Electron config cache, not SQLite)electron/db/queries.ts— n/a (no new SQL)dependenciesordevDependencies— n/a (none added)--external:<pkg>flag changes — n/a (none)Notes for reviewer
Design decisions (confirmed with product along the way):
aiConfig.savedProviders; the Coding Agent reads the same list but keeps its ownactiveProviderIdonagentConfig, so it can run on a different model than chat.add/update/deletereconcile both configs; the Electron config cache persists the list underaiConfigand only the active id underagentConfig.LOCAL_FALLBACK_MODELS/CLOUD_FALLBACK_MODELS.useEndpointConfignow caches fetched models per endpoint (base URL + whether a key is present) in localStorage with a 7-day TTL, andensureModels()hydrates from cache or fetches once. Auto-fetch effects are keyed on provider / active-provider id (not the URL/key text) to avoid fetching on every keystroke — Refresh re-queries after edits.127.0.0.1CSP fix. The renderer's CSP only allowedhttp://localhost:*; since CSP matches origins literally,http://127.0.0.1:*was blocked (model fetch/test failed while chat, which routes over IPC, worked — hence the confusing behaviour). Both loopback spellings are now whitelisted.getOpenAI*/setOpenAI*callers keep working via the active provider.ModelPicker+TruncatedModelwere extracted fromChatQuickSettingsintosrc/components/ui/model-picker.tsxand reused in AI settings, Coding Agent settings, and the provider form (with asizevariant for the settings-row density).Summary by CodeRabbit
New Features
127.0.0.1local endpoints in addition tolocalhost.Bug Fixes