Add multi-provider LLM abstraction layer (OpenRouter + OpenClaw) - #1
Open
AMPMIO wants to merge 3 commits into
Open
Add multi-provider LLM abstraction layer (OpenRouter + OpenClaw)#1AMPMIO wants to merge 3 commits into
AMPMIO wants to merge 3 commits into
Conversation
Introduces LLMProvider protocol, OpenAICompatibleProvider for OpenRouter/OpenClaw, AnthropicProvider wrapper for existing Worker flow, KeychainManager for secure API key storage, ProviderConfiguration for settings, and ProviderManager factory. Migrates CompanionManager from direct ClaudeAPI usage to the new provider system. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- SettingsView: provider picker (Worker/OpenRouter/OpenClaw), API key fields with Keychain storage, endpoint config, connection test button - CompanionPanelView: provider-aware model presets + custom model ID input field, settings gear in footer - Worker: new /chat-openrouter route proxying to OpenRouter API Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- ModelCatalogService: fetches OpenRouter model catalog, caches for 1hr, supports search filtering, vision-only filter, and provider grouping - VoiceProvider: TTSProvider and STTProvider protocol stubs for future local Whisper/Qwen3 TTS integration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a provider abstraction layer (LLMProvider protocol) supporting Anthropic Worker Proxy, OpenRouter, and OpenClaw backends, with keychain-backed credential storage, provider configuration/management, a model catalog service, updated companion panel/settings UI, voice provider stubs, and a new worker proxy route for OpenRouter chat completions. ChangesProvider Integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CompanionManager
participant ProviderManager
participant LLMProvider
participant UpstreamAPI
CompanionManager->>ProviderManager: currentProvider.chatStreaming(...)
ProviderManager->>LLMProvider: chatStreaming(images, prompt, history, model, onTextChunk)
LLMProvider->>UpstreamAPI: POST chat completions request
UpstreamAPI-->>LLMProvider: streamed delta.content chunks
LLMProvider-->>CompanionManager: onTextChunk(accumulated text)
LLMProvider-->>ProviderManager: (text, duration)
sequenceDiagram
participant User
participant SettingsView
participant ProviderManager
participant LLMProvider
User->>SettingsView: tap Test Connection
SettingsView->>ProviderManager: currentProvider.chat(ping)
ProviderManager->>LLMProvider: chat(...)
LLMProvider-->>SettingsView: (text, duration) or error
SettingsView-->>User: display Success/Error result
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
leanring-buddy/CompanionManager.swift (1)
71-80: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUse the configured Worker URL for dependent clients.
Line 73 keeps a hard-coded Worker URL while
SettingsViewwritesproviderManager.configuration.workerBaseURL; TTS still initializes from the static placeholder, so changing the Worker URL in settings won’t affect TTS/proxy calls. MakeProviderConfiguration.workerBaseURLthe single source of truth and rebuild dependent clients when it changes.🤖 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 `@leanring-buddy/CompanionManager.swift` around lines 71 - 80, The TTS client is still initialized from the hard-coded CompanionManager.workerBaseURL placeholder instead of the configured ProviderConfiguration.workerBaseURL, so settings changes do not propagate. Make ProviderManager/ProviderConfiguration the single source of truth for the Worker URL, and update CompanionManager.elevenLabsTTSClient to read from the current configuration rather than the static constant. If the Worker URL can change at runtime, rebuild or refresh dependent clients like ElevenLabsTTSClient when the configuration changes so proxy calls use the updated URL.
🤖 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 `@leanring-buddy/AnthropicProvider.swift`:
- Around line 27-45: The shared mutable claudeAPI.model assignment in
AnthropicProvider’s analyzeImageStreaming/chat flow can race across concurrent
calls and send the wrong model. Update the request path so the model is passed
per call into ClaudeAPI’s request construction, or instantiate a fresh ClaudeAPI
inside each method before calling analyzeImageStreaming/analyzeImage, instead of
mutating shared state.
In `@leanring-buddy/CompanionManager.swift`:
- Around line 75-76: `CompanionPanelView` is observing `CompanionManager`, but
updates made inside the nested `ProviderManager` are not being forwarded, so the
provider UI can stay stale. Update `CompanionManager` to forward
`ProviderManager`’s `objectWillChange` through its own publisher, wiring the
subscription in the `CompanionManager` initializer or setup so changes from
`providerManager` propagate to observers like `CompanionPanelView`.
In `@leanring-buddy/CompanionPanelView.swift`:
- Around line 624-628: The quick preset list in CompanionPanelView’s
modelPresets is using tuples, which cannot be referenced with id: \.id in
ForEach and will fail to compile. Replace the tuple-based preset collection with
a small Identifiable type, then update the ForEach in the quick presets HStack
to iterate that new type using its id and keep modelOptionButton(label:modelID:)
wired to the preset fields.
In `@leanring-buddy/KeychainManager.swift`:
- Around line 14-27: The write path in KeychainManager’s save/add logic deletes
the existing keychain item before adding a new one, which makes replacement
non-atomic. Update the existing item in place using add-or-update semantics
instead of calling SecItemDelete first, so a failed SecItemAdd/replace does not
remove the prior API key. Keep the fix localized to the method that builds the
delete/add queries in KeychainManager.
In `@leanring-buddy/OpenAICompatibleProvider.swift`:
- Around line 145-150: The request setup in OpenAICompatibleProvider should fail
fast when apiKey is blank instead of sending an empty Bearer token. Add an early
validation in the provider initialization/request path used before building the
URLRequest so that a blank apiKey throws ProviderError.missingAPIKey, and ensure
ProviderManager’s construction path relies on that same check rather than
allowing "Authorization: Bearer ". Keep the fix localized to
OpenAICompatibleProvider and the API key handling used by its request creation.
In `@leanring-buddy/ProviderConfiguration.swift`:
- Around line 39-42: The default model value is currently OpenRouter-formatted,
but `AnthropicProvider` passes it directly to `ClaudeAPI`, so provider-specific
defaults need to be split. Update `ProviderConfiguration` to use an
Anthropic-native default model ID for worker mode in `defaultModel`, while
keeping the provider-prefixed format for OpenRouter-related configuration and
any code paths that select OpenRouter models. Use the existing
`defaultWorkerBaseURL`, `defaultOpenRouterBaseURL`, and `AnthropicProvider`
references to locate the affected selection logic.
- Around line 63-67: The default provider selection in
ProviderConfiguration.init currently migrates missing activeAPIProvider values
to OpenRouter, which changes existing installs away from the previous Worker
behavior. Update the fallback so absent provider keys continue to resolve to
.workerProxy, or add an explicit one-time migration gate in
ProviderConfiguration that only switches older installs when appropriate. Keep
the change localized around activeProvider initialization and Self.providerKey
handling.
In `@leanring-buddy/ProviderManager.swift`:
- Around line 56-60: The ProviderManager/OpenAICompatibleProvider setup is
force-unwrapping a user-configured endpoint URL, which can crash when
openClawEndpoint is malformed. Update the URL construction in ProviderManager so
it validates the endpoint before creating the /api/sessions/main/messages URL,
and handle the failure by surfacing a provider configuration error instead of
using a forced unwrap. Use the existing config.openClawEndpoint and the provider
creation path around OpenAICompatibleProvider to locate and fix this flow.
In `@leanring-buddy/SettingsView.swift`:
- Around line 134-172: The OpenClaw settings flow currently accepts a bearer
token for a default plain-HTTP endpoint, which should be avoided. Update the
OpenClaw configuration UI and validation in SettingsView so the endpoint
defaults to HTTPS, and prevent non-HTTPS values from being saved in the
TextField/onChange path except for explicit loopback or local development hosts.
Keep the token handling in providerManager.configuration.openClawToken, but gate
providerManager.updateProvider() behind endpoint validation so insecure URLs are
rejected before use.
- Around line 80-84: Reset the selected model when changing providers in
SettingsView’s providerTab flow: providerManager.setActiveProvider currently
rebuilds the provider but leaves selectedModelID unchanged, so update that
selection at the same time. Use a helper like defaultModelID(for:) to derive the
appropriate model for each APIProviderType, and apply it whenever the active
provider changes so connection tests and chat always use a model valid for the
newly selected provider.
In `@worker/src/index.ts`:
- Around line 89-101: The OpenRouter proxy in handleOpenRouterChat currently
forwards any caller’s request body with the server-side API key, so add
authentication on this route and reject unauthenticated requests before the
fetch call. Also validate and constrain the chat completion payload in
handleOpenRouterChat by enforcing an allowlist of permitted models and hard caps
for token usage and other expensive parameters before forwarding to OpenRouter.
---
Outside diff comments:
In `@leanring-buddy/CompanionManager.swift`:
- Around line 71-80: The TTS client is still initialized from the hard-coded
CompanionManager.workerBaseURL placeholder instead of the configured
ProviderConfiguration.workerBaseURL, so settings changes do not propagate. Make
ProviderManager/ProviderConfiguration the single source of truth for the Worker
URL, and update CompanionManager.elevenLabsTTSClient to read from the current
configuration rather than the static constant. If the Worker URL can change at
runtime, rebuild or refresh dependent clients like ElevenLabsTTSClient when the
configuration changes so proxy calls use the updated URL.
🪄 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: 1ea70c08-0c85-4ec0-9ef9-64f64fbbc843
📒 Files selected for processing (13)
CHANGELOG.mdleanring-buddy/AnthropicProvider.swiftleanring-buddy/CompanionManager.swiftleanring-buddy/CompanionPanelView.swiftleanring-buddy/KeychainManager.swiftleanring-buddy/LLMProvider.swiftleanring-buddy/ModelCatalogService.swiftleanring-buddy/OpenAICompatibleProvider.swiftleanring-buddy/ProviderConfiguration.swiftleanring-buddy/ProviderManager.swiftleanring-buddy/SettingsView.swiftleanring-buddy/VoiceProvider.swiftworker/src/index.ts
Comment on lines
+27
to
+45
| claudeAPI.model = model | ||
| return try await claudeAPI.analyzeImageStreaming( | ||
| images: images, | ||
| systemPrompt: systemPrompt, | ||
| conversationHistory: conversationHistory, | ||
| userPrompt: userPrompt, | ||
| onTextChunk: onTextChunk | ||
| ) | ||
| } | ||
|
|
||
| func chat( | ||
| images: [(data: Data, label: String)], | ||
| systemPrompt: String, | ||
| conversationHistory: [(userPlaceholder: String, assistantResponse: String)], | ||
| userPrompt: String, | ||
| model: String | ||
| ) async throws -> (text: String, duration: TimeInterval) { | ||
| claudeAPI.model = model | ||
| return try await claudeAPI.analyzeImage( |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Avoid mutating shared model state before async requests.
claudeAPI.model = model is shared mutable state; concurrent provider calls with different models can race and send the wrong model. Prefer passing model into ClaudeAPI request construction or create a per-call ClaudeAPI instance.
🤖 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 `@leanring-buddy/AnthropicProvider.swift` around lines 27 - 45, The shared
mutable claudeAPI.model assignment in AnthropicProvider’s
analyzeImageStreaming/chat flow can race across concurrent calls and send the
wrong model. Update the request path so the model is passed per call into
ClaudeAPI’s request construction, or instantiate a fresh ClaudeAPI inside each
method before calling analyzeImageStreaming/analyzeImage, instead of mutating
shared state.
Comment on lines
+75
to
+76
| /// Multi-provider LLM manager. Handles OpenRouter, OpenClaw, and Worker Proxy modes. | ||
| let providerManager = ProviderManager() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Forward ProviderManager changes through CompanionManager.
CompanionPanelView observes CompanionManager, but provider changes happen inside the nested ProviderManager. Without forwarding objectWillChange, the provider badge/presets can stay stale after edits in the settings popover.
Proposed fix
/// Multi-provider LLM manager. Handles OpenRouter, OpenClaw, and Worker Proxy modes.
let providerManager = ProviderManager()
+ private var providerManagerCancellable: AnyCancellable?
+
+ init() {
+ providerManagerCancellable = providerManager.objectWillChange
+ .sink { [weak self] _ in
+ self?.objectWillChange.send()
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Multi-provider LLM manager. Handles OpenRouter, OpenClaw, and Worker Proxy modes. | |
| let providerManager = ProviderManager() | |
| /// Multi-provider LLM manager. Handles OpenRouter, OpenClaw, and Worker Proxy modes. | |
| let providerManager = ProviderManager() | |
| private var providerManagerCancellable: AnyCancellable? | |
| init() { | |
| providerManagerCancellable = providerManager.objectWillChange | |
| .sink { [weak self] _ in | |
| self?.objectWillChange.send() | |
| } | |
| } |
🤖 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 `@leanring-buddy/CompanionManager.swift` around lines 75 - 76,
`CompanionPanelView` is observing `CompanionManager`, but updates made inside
the nested `ProviderManager` are not being forwarded, so the provider UI can
stay stale. Update `CompanionManager` to forward `ProviderManager`’s
`objectWillChange` through its own publisher, wiring the subscription in the
`CompanionManager` initializer or setup so changes from `providerManager`
propagate to observers like `CompanionPanelView`.
Comment on lines
+624
to
+628
| // Quick presets based on provider | ||
| HStack(spacing: 0) { | ||
| modelOptionButton(label: "Sonnet", modelID: "claude-sonnet-4-6") | ||
| modelOptionButton(label: "Opus", modelID: "claude-opus-4-6") | ||
| ForEach(modelPresets, id: \.id) { preset in | ||
| modelOptionButton(label: preset.label, modelID: preset.id) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'ForEach\(modelPresets,\s*id:\s*\\\.id\)|private var modelPresets: \[\(id:' leanring-buddy/CompanionPanelView.swiftRepository: AMPMIO/open-clicky
Length of output: 286
🌐 Web query:
Swift key path tuple member labels ForEach id: \.id tuple labeled elements compile
💡 Result:
In Swift, you can use key paths to access tuple elements, but you are limited to using integer indices (e.g., .0, .1) [1]. Accessing tuple members via their labels (e.g., .name) using key paths is not supported because Swift key paths are fundamentally tied to struct and class properties, and tuple labels are not stored as property identifiers in the same way [2][3][4]. When working with ForEach in SwiftUI, if you are iterating over a collection of tuples, you must use the integer index in the key path to identify the element (e.g., id: .0.id) [5]. Because named tuple elements do not support key path member lookup by label, the compiler will fail to resolve .labelName if you attempt to use it as a key path [2][4]. To work around this limitation in SwiftUI, common patterns include: 1. Using Integer Indices: If the tuple structure is known and fixed, use the numeric index, such as .0.id [5]. 2. Creating a Wrapper Struct: Define a struct to hold your data instead of using a tuple. This allows you to use meaningful property names in your key paths (e.g., .id) [5]. 3. Computed Identifiable Property: If you must use a tuple, you can extend the collection or create a container that conforms to Identifiable, mapping the tuple elements to an identifiable property [5]. In summary, the limitation exists because the Swift type system treats tuple labels as distinct from the stored property accessors required by the KeyPath implementation [3][6].
Citations:
- 1: https://github.com/swiftlang/swift/blob/12f8d9c6c94ba944771baed4ed42bfe6717d9e74/test/stdlib/KeyPath.swift
- 2: https://forums.swift.org/t/how-to-access-tuple-elements-with-key-paths/11149
- 3: https://forums.swift.org/t/keypaths-tuples-and-swift-5/18465
- 4: https://medium.com/@swiftmoji/keying-into-tuples-swiftmoji-entry-1-e81b9a9d0bc9
- 5: https://oleb.net/2020/foreach-enumerated/
- 6: [CSDiag] Diagnose KeyPath tuple access as unsupported even when sugared swiftlang/swift#20518
🏁 Script executed:
#!/bin/bash
sed -n '620,690p' leanring-buddy/CompanionPanelView.swiftRepository: AMPMIO/open-clicky
Length of output: 2914
Replace the tuple presets with an Identifiable type. ForEach(modelPresets, id: \.id) can’t key-path into tuple labels, so this won’t compile. Use a small Identifiable struct for the preset list instead.
🤖 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 `@leanring-buddy/CompanionPanelView.swift` around lines 624 - 628, The quick
preset list in CompanionPanelView’s modelPresets is using tuples, which cannot
be referenced with id: \.id in ForEach and will fail to compile. Replace the
tuple-based preset collection with a small Identifiable type, then update the
ForEach in the quick presets HStack to iterate that new type using its id and
keep modelOptionButton(label:modelID:) wired to the preset fields.
Comment on lines
+14
to
+27
| // Delete any existing item first | ||
| let deleteQuery: [String: Any] = [ | ||
| kSecClass as String: kSecClassGenericPassword, | ||
| kSecAttrService as String: service, | ||
| ] | ||
| SecItemDelete(deleteQuery as CFDictionary) | ||
|
|
||
| let addQuery: [String: Any] = [ | ||
| kSecClass as String: kSecClassGenericPassword, | ||
| kSecAttrService as String: service, | ||
| kSecValueData as String: data, | ||
| ] | ||
| let status = SecItemAdd(addQuery as CFDictionary, nil) | ||
| return status == errSecSuccess |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files | rg '^leanring-buddy/|KeychainManager\.swift$|ProviderConfiguration|keychain' || true
echo '--- KeychainManager.swift outline ---'
ast-grep outline leanring-buddy/KeychainManager.swift --view expanded || true
echo '--- KeychainManager.swift with line numbers ---'
wc -l leanring-buddy/KeychainManager.swift
cat -n leanring-buddy/KeychainManager.swift | sed -n '1,200p'
echo '--- references to KeychainManager.save ---'
rg -n "KeychainManager\.save|save\(key:.*service:" leanring-buddy || trueRepository: AMPMIO/open-clicky
Length of output: 6875
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- ProviderConfiguration.swift outline ---'
ast-grep outline leanring-buddy/ProviderConfiguration.swift --view expanded || true
echo '--- relevant lines ---'
cat -n leanring-buddy/ProviderConfiguration.swift | sed -n '1,180p'
echo '--- keychain service constants and use sites ---'
rg -n "openRouterKeyService|openClawTokenService|KeychainManager\.(save|retrieve|delete)" leanring-buddy/ProviderConfiguration.swift leanring-buddy/*.swift || trueRepository: AMPMIO/open-clicky
Length of output: 6283
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- AGENTS.md ---'
wc -l leanring-buddy/AGENTS.md
cat -n leanring-buddy/AGENTS.md | sed -n '1,220p'Repository: AMPMIO/open-clicky
Length of output: 1951
Preserve the existing keychain item when replacing it. Deleting first makes this write non-atomic; if SecItemAdd fails, the previous API key is already gone. Use add-or-update semantics instead.
🤖 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 `@leanring-buddy/KeychainManager.swift` around lines 14 - 27, The write path in
KeychainManager’s save/add logic deletes the existing keychain item before
adding a new one, which makes replacement non-atomic. Update the existing item
in place using add-or-update semantics instead of calling SecItemDelete first,
so a failed SecItemAdd/replace does not remove the prior API key. Keep the fix
localized to the method that builds the delete/add queries in KeychainManager.
Comment on lines
+145
to
+150
| var request = URLRequest(url: baseURL) | ||
| request.httpMethod = "POST" | ||
| request.timeoutInterval = 120 | ||
| request.setValue("application/json", forHTTPHeaderField: "Content-Type") | ||
| request.setValue("Bearer \(apiKey)", forHTTPHeaderField: "Authorization") | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fail fast when the API key is blank.
ProviderManager can construct this provider with "", so this currently sends Authorization: Bearer instead of using the existing ProviderError.missingAPIKey.
Proposed fix
) throws -> URLRequest {
+ guard !apiKey.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
+ throw ProviderError.missingAPIKey(provider: displayName)
+ }
+
var request = URLRequest(url: baseURL)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var request = URLRequest(url: baseURL) | |
| request.httpMethod = "POST" | |
| request.timeoutInterval = 120 | |
| request.setValue("application/json", forHTTPHeaderField: "Content-Type") | |
| request.setValue("Bearer \(apiKey)", forHTTPHeaderField: "Authorization") | |
| guard !apiKey.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { | |
| throw ProviderError.missingAPIKey(provider: displayName) | |
| } | |
| var request = URLRequest(url: baseURL) | |
| request.httpMethod = "POST" | |
| request.timeoutInterval = 120 | |
| request.setValue("application/json", forHTTPHeaderField: "Content-Type") | |
| request.setValue("Bearer \(apiKey)", forHTTPHeaderField: "Authorization") |
🤖 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 `@leanring-buddy/OpenAICompatibleProvider.swift` around lines 145 - 150, The
request setup in OpenAICompatibleProvider should fail fast when apiKey is blank
instead of sending an empty Bearer token. Add an early validation in the
provider initialization/request path used before building the URLRequest so that
a blank apiKey throws ProviderError.missingAPIKey, and ensure ProviderManager’s
construction path relies on that same check rather than allowing "Authorization:
Bearer ". Keep the fix localized to OpenAICompatibleProvider and the API key
handling used by its request creation.
Comment on lines
+63
to
+67
| init() { | ||
| let defaults = UserDefaults.standard | ||
| let providerRaw = defaults.string(forKey: Self.providerKey) ?? APIProviderType.openRouter.rawValue | ||
| self.activeProvider = APIProviderType(rawValue: providerRaw) ?? .openRouter | ||
| self.selectedModelID = defaults.string(forKey: Self.selectedModelKey) ?? Self.defaultModel |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the migration default on the existing Worker provider.
When activeAPIProvider is absent, this now defaults existing installs to OpenRouter, which has no stored key and changes the previous Claude Worker behavior. Default to .workerProxy or add an explicit migration gate.
Proposed fix
- let providerRaw = defaults.string(forKey: Self.providerKey) ?? APIProviderType.openRouter.rawValue
+ let providerRaw = defaults.string(forKey: Self.providerKey) ?? APIProviderType.workerProxy.rawValue📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| init() { | |
| let defaults = UserDefaults.standard | |
| let providerRaw = defaults.string(forKey: Self.providerKey) ?? APIProviderType.openRouter.rawValue | |
| self.activeProvider = APIProviderType(rawValue: providerRaw) ?? .openRouter | |
| self.selectedModelID = defaults.string(forKey: Self.selectedModelKey) ?? Self.defaultModel | |
| init() { | |
| let defaults = UserDefaults.standard | |
| let providerRaw = defaults.string(forKey: Self.providerKey) ?? APIProviderType.workerProxy.rawValue | |
| self.activeProvider = APIProviderType(rawValue: providerRaw) ?? .openRouter | |
| self.selectedModelID = defaults.string(forKey: Self.selectedModelKey) ?? Self.defaultModel |
🤖 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 `@leanring-buddy/ProviderConfiguration.swift` around lines 63 - 67, The default
provider selection in ProviderConfiguration.init currently migrates missing
activeAPIProvider values to OpenRouter, which changes existing installs away
from the previous Worker behavior. Update the fallback so absent provider keys
continue to resolve to .workerProxy, or add an explicit one-time migration gate
in ProviderConfiguration that only switches older installs when appropriate.
Keep the change localized around activeProvider initialization and
Self.providerKey handling.
Comment on lines
+56
to
+60
| let endpoint = config.openClawEndpoint.isEmpty | ||
| ? "http://localhost:18789" | ||
| : config.openClawEndpoint | ||
| let url = URL(string: "\(endpoint)/api/sessions/main/messages")! | ||
| return OpenAICompatibleProvider( |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Do not force-unwrap user-configured endpoint URLs.
A malformed openClawEndpoint will crash when switching providers. Validate the URL and surface a provider configuration error instead of using !.
🤖 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 `@leanring-buddy/ProviderManager.swift` around lines 56 - 60, The
ProviderManager/OpenAICompatibleProvider setup is force-unwrapping a
user-configured endpoint URL, which can crash when openClawEndpoint is
malformed. Update the URL construction in ProviderManager so it validates the
endpoint before creating the /api/sessions/main/messages URL, and handle the
failure by surfacing a provider configuration error instead of using a forced
unwrap. Use the existing config.openClawEndpoint and the provider creation path
around OpenAICompatibleProvider to locate and fix this flow.
Comment on lines
+80
to
+84
| private func providerTab(_ provider: APIProviderType) -> some View { | ||
| let isSelected = providerManager.configuration.activeProvider == provider | ||
| return Button(action: { | ||
| providerManager.setActiveProvider(provider) | ||
| }) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reset the selected model when switching providers.
setActiveProvider rebuilds the provider but does not update selectedModelID; after switching between Worker Proxy and OpenRouter/OpenClaw, the connection test and chat can keep a model ID from the previous provider.
Proposed fix
return Button(action: {
providerManager.setActiveProvider(provider)
+ providerManager.setSelectedModel(defaultModelID(for: provider))
}) {private func defaultModelID(for provider: APIProviderType) -> String {
switch provider {
case .workerProxy:
return "claude-sonnet-4-6"
case .openRouter:
return "anthropic/claude-sonnet-4-6"
case .openClaw:
return "anthropic/claude-sonnet-4-6"
}
}🤖 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 `@leanring-buddy/SettingsView.swift` around lines 80 - 84, Reset the selected
model when changing providers in SettingsView’s providerTab flow:
providerManager.setActiveProvider currently rebuilds the provider but leaves
selectedModelID unchanged, so update that selection at the same time. Use a
helper like defaultModelID(for:) to derive the appropriate model for each
APIProviderType, and apply it whenever the active provider changes so connection
tests and chat always use a model valid for the newly selected provider.
Comment on lines
+134
to
+172
| TextField("http://your-vps:18789", text: $openClawEndpointInput) | ||
| .textFieldStyle(.plain) | ||
| .font(.system(size: 12, design: .monospaced)) | ||
| .padding(8) | ||
| .background( | ||
| RoundedRectangle(cornerRadius: 6, style: .continuous) | ||
| .fill(DS.Colors.surface2) | ||
| ) | ||
| .overlay( | ||
| RoundedRectangle(cornerRadius: 6, style: .continuous) | ||
| .stroke(DS.Colors.borderSubtle, lineWidth: 0.5) | ||
| ) | ||
| .onChange(of: openClawEndpointInput) { newValue in | ||
| providerManager.configuration.openClawEndpoint = newValue | ||
| providerManager.updateProvider() | ||
| } | ||
| } | ||
|
|
||
| VStack(alignment: .leading, spacing: 4) { | ||
| Text("Bearer Token") | ||
| .font(.system(size: 11, weight: .medium)) | ||
| .foregroundColor(DS.Colors.textSecondary) | ||
|
|
||
| SecureField("Token", text: $openClawTokenInput) | ||
| .textFieldStyle(.plain) | ||
| .font(.system(size: 12, design: .monospaced)) | ||
| .padding(8) | ||
| .background( | ||
| RoundedRectangle(cornerRadius: 6, style: .continuous) | ||
| .fill(DS.Colors.surface2) | ||
| ) | ||
| .overlay( | ||
| RoundedRectangle(cornerRadius: 6, style: .continuous) | ||
| .stroke(DS.Colors.borderSubtle, lineWidth: 0.5) | ||
| ) | ||
| .onChange(of: openClawTokenInput) { newValue in | ||
| providerManager.configuration.openClawToken = newValue | ||
| providerManager.updateProvider() | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don’t encourage bearer tokens over plain HTTP.
The OpenClaw placeholder is http://your-vps:18789 while Line 170 stores a bearer token for that endpoint. Use HTTPS by default and reject non-HTTPS URLs except explicit loopback/local development endpoints.
🤖 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 `@leanring-buddy/SettingsView.swift` around lines 134 - 172, The OpenClaw
settings flow currently accepts a bearer token for a default plain-HTTP
endpoint, which should be avoided. Update the OpenClaw configuration UI and
validation in SettingsView so the endpoint defaults to HTTPS, and prevent
non-HTTPS values from being saved in the TextField/onChange path except for
explicit loopback or local development hosts. Keep the token handling in
providerManager.configuration.openClawToken, but gate
providerManager.updateProvider() behind endpoint validation so insecure URLs are
rejected before use.
Comment on lines
+89
to
+101
| async function handleOpenRouterChat(request: Request, env: Env): Promise<Response> { | ||
| const body = await request.text(); | ||
|
|
||
| const response = await fetch("https://openrouter.ai/api/v1/chat/completions", { | ||
| method: "POST", | ||
| headers: { | ||
| "Authorization": `Bearer ${env.OPENROUTER_API_KEY}`, | ||
| "content-type": "application/json", | ||
| "HTTP-Referer": "https://github.com/AMPMIO/open-clicky", | ||
| "X-Title": "Clicky", | ||
| }, | ||
| body, | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Protect the OpenRouter proxy from arbitrary public use.
This route forwards any POST body with the server-side OpenRouter key, so anyone who discovers the Worker URL can choose expensive models or large token limits. Add client authentication and enforce an allowlist/caps before forwarding.
🤖 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 `@worker/src/index.ts` around lines 89 - 101, The OpenRouter proxy in
handleOpenRouterChat currently forwards any caller’s request body with the
server-side API key, so add authentication on this route and reject
unauthenticated requests before the fetch call. Also validate and constrain the
chat completion payload in handleOpenRouterChat by enforcing an allowlist of
permitted models and hard caps for token usage and other expensive parameters
before forwarding to OpenRouter.
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
1 participant
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.
Introduces LLMProvider protocol, OpenAICompatibleProvider for
OpenRouter/OpenClaw, AnthropicProvider wrapper for existing Worker flow,
KeychainManager for secure API key storage, ProviderConfiguration for
settings, and ProviderManager factory. Migrates CompanionManager from
direct ClaudeAPI usage to the new provider system.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes