Improve Local Proxy setup and model guidance#652
Conversation
Deploying maple with
|
| Latest commit: |
1532145
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://25ee7c1c.maple-ca8.pages.dev |
| Branch Preview URL: | https://codex-opencode-maple-proxy-r.maple-ca8.pages.dev |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe PR adds proxy model discovery and normalization, refactors local proxy configuration and lifecycle handling, and introduces model-aware connection guides and model catalog rendering in the desktop API settings flow. ChangesLocal proxy experience
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant LocalProxySettings
participant useProxyModels
participant ProxyConfigSection
participant ProxyClientGuides
participant ProxyModelList
LocalProxySettings->>useProxyModels: load proxy chat models
useProxyModels->>ProxyConfigSection: provide models and query state
ProxyConfigSection->>ProxyClientGuides: provide base URL and selected model
ProxyConfigSection->>ProxyModelList: provide model catalog and status
ProxyClientGuides-->>LocalProxySettings: render connection examples
ProxyModelList-->>LocalProxySettings: render model metadata
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 1
🤖 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 `@frontend/src/components/settings/api/useProxyModels.ts`:
- Around line 10-18: Update the queryKey in the proxy model catalog query to
include auth.user?.user.id ?? null, ensuring React Query scopes cached catalogs
per authenticated user while preserving the existing fetchProxyChatModels
queryFn and staleTime behavior.
🪄 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: 24c734f4-b25a-4868-8dd0-eda4b1fdda06
📒 Files selected for processing (10)
frontend/src-tauri/src/proxy.rsfrontend/src/components/apikeys/ProxyClientGuides.tsxfrontend/src/components/apikeys/ProxyConfigSection.tsxfrontend/src/components/apikeys/ProxyModelList.tsxfrontend/src/components/settings/api/ApiSettingsLayout.tsxfrontend/src/components/settings/api/LocalProxySettings.tsxfrontend/src/components/settings/api/useProxyModels.tsfrontend/src/services/proxyModels.test.tsfrontend/src/services/proxyModels.tsfrontend/src/services/proxyService.ts
💤 Files with no reviewable changes (1)
- frontend/src/components/settings/api/ApiSettingsLayout.tsx
| queryKey: ["proxy-chat-model-catalog"], | ||
| queryFn: () => | ||
| fetchProxyChatModels({ | ||
| fetchModelCatalog: async () => | ||
| (await fetchModelCatalog()) as unknown as OpenSecretModelCatalog, | ||
| fetchModels: async () => (await fetchModels()) as OpenSecretModel[] | ||
| }), | ||
| enabled: !!auth.user && !auth.loading, | ||
| staleTime: 5 * 60 * 1000 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does TanStack Query v5 share cached data across different queryKey values?
💡 Result:
No, TanStack Query v5 does not share cached data across different queryKey values [1][2]. In TanStack Query, the queryKey serves as the unique identifier for a query's cached data [1][3]. The library manages caching and refetching based on these keys; because each unique queryKey is treated as a distinct entry in the cache, queries with different keys are cached and managed independently [1][2]. Key points regarding how this works: - Deterministic Hashing: Query keys are hashed deterministically [1]. If two queryKeys are functionally identical (e.g., they contain the same data in the same order), they are considered the same and will share the same cache entry [1]. However, if the keys differ—even by the order of elements or the inclusion of different variables—the library treats them as separate queries [1][2]. - Independent Caching: Adding variables to a queryKey (e.g., ['todos', todoId]) ensures that each unique variable creates a separate cache entry [1][2]. This is the intended behavior, as it allows the library to independently track, update, and invalidate data for specific entities or parameters [1][2][4]. - Partial Matching: While TanStack Query does not automatically share data between different keys, it does allow you to perform operations across multiple queries using partial matching (e.g., with query filters in methods like invalidateQueries or findAll) [5][4]. This enables you to target a group of related queries—such as all queries starting with the prefix ['todos']—but this is a mechanism for cache management rather than data sharing [4]. If you need to share data between queries, you should typically structure your keys to be identical for the data you want to share, or use cache management tools to manually synchronize data between different keys if they represent related but distinct information [1][4].
Citations:
- 1: https://tanstack.com/query/v5/docs/framework/react/guides/query-keys
- 2: https://tanstack.com/query/v5/docs/framework/react/guides/query-keys.md
- 3: https://tanstack.com/query/v5/docs/framework/react/guides/queries
- 4: https://tanstack.com/query/v5/docs/framework/react/guides/query-invalidation
- 5: https://tanstack.com/query/v5/docs/reference/QueryCache
🏁 Script executed:
set -e
git ls-files frontend/src/components/settings/api/useProxyModels.ts frontend/src/components/settings/api frontend/src/components/settings/api/useApiKeys.ts frontend/src | sed -n '1,120p'
echo '--- useProxyModels.ts ---'
cat -n frontend/src/components/settings/api/useProxyModels.ts
echo '--- useApiKeys.ts ---'
cat -n frontend/src/components/settings/api/useApiKeys.tsRepository: OpenSecretCloud/Maple
Length of output: 7318
🏁 Script executed:
set -e
python3 - <<'PY'
from pathlib import Path
for p in [
Path("frontend/src/components/settings/api/useProxyModels.ts"),
Path("frontend/src/components/settings/api/useApiKeys.ts"),
]:
print(f"\n### {p} ###")
if p.exists():
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if "useQuery" in line or "queryKey" in line or "staleTime" in line or "auth" in line or "fetchProxyChatModels" in line or "fetchModelCatalog" in line or "fetchModels" in line:
print(f"{i}: {line}")
else:
print("missing")
PYRepository: OpenSecretCloud/Maple
Length of output: 1103
Scope the cache key by user
["proxy-chat-model-catalog"] is shared across accounts, and the 5-minute staleTime can reuse a previous user's cached catalog after an account switch. Add auth.user?.user.id ?? null to the key, or drop staleTime if this should refetch on every mount.
♻️ Suggested change
- return useQuery<OpenSecretModel[]>({
- queryKey: ["proxy-chat-model-catalog"],
+ return useQuery<OpenSecretModel[]>({
+ queryKey: ["proxy-chat-model-catalog", auth.user?.user.id ?? null],📝 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.
| queryKey: ["proxy-chat-model-catalog"], | |
| queryFn: () => | |
| fetchProxyChatModels({ | |
| fetchModelCatalog: async () => | |
| (await fetchModelCatalog()) as unknown as OpenSecretModelCatalog, | |
| fetchModels: async () => (await fetchModels()) as OpenSecretModel[] | |
| }), | |
| enabled: !!auth.user && !auth.loading, | |
| staleTime: 5 * 60 * 1000 | |
| queryKey: ["proxy-chat-model-catalog", auth.user?.user.id ?? null], | |
| queryFn: () => | |
| fetchProxyChatModels({ | |
| fetchModelCatalog: async () => | |
| (await fetchModelCatalog()) as unknown as OpenSecretModelCatalog, | |
| fetchModels: async () => (await fetchModels()) as OpenSecretModel[] | |
| }), | |
| enabled: !!auth.user && !auth.loading, | |
| staleTime: 5 * 60 * 1000 |
🤖 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 `@frontend/src/components/settings/api/useProxyModels.ts` around lines 10 - 18,
Update the queryKey in the proxy model catalog query to include
auth.user?.user.id ?? null, ensuring React Query scopes cached catalogs per
authenticated user while preserving the existing fetchProxyChatModels queryFn
and staleTime behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/apikeys/ProxyModelList.tsx (1)
15-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReset the error state after failed copies. The success path already returns to
idle, but a clipboard failure leaves the button stuck on the error icon/label until another click. Consider clearing that state after a short delay or on the next interaction.🤖 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 `@frontend/src/components/apikeys/ProxyModelList.tsx` around lines 15 - 63, The ModelId handleCopy error path leaves copyState stuck at "error"; reset it to "idle" after a short delay, matching the existing success-path timeout, while preserving the current error icon and accessible label during that interval.
🧹 Nitpick comments (1)
frontend/src/components/apikeys/ProxyModelList.tsx (1)
87-111: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a retry button to the fetch error state. The app disables automatic retries/refetch-on-focus, so a transient model-catalog failure leaves users with only the restart instruction. A small
Try againbutton wired torefetch()would give them an in-place recovery path.🤖 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 `@frontend/src/components/apikeys/ProxyModelList.tsx` around lines 87 - 111, Update the isError state in ProxyModelList to include a small “Try again” button that invokes the query’s refetch() function. Keep the existing error message and ensure the button provides an in-place recovery path without changing loading or empty-state behavior.
🤖 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 `@frontend/src/components/apikeys/ProxyModelList.tsx`:
- Around line 77-85: Update the header text in ProxyModelList so the singular
case renders “1 chat model ID” and all other counts render “N chat model IDs”;
preserve the existing “Chat model IDs” label when the list is empty.
---
Outside diff comments:
In `@frontend/src/components/apikeys/ProxyModelList.tsx`:
- Around line 15-63: The ModelId handleCopy error path leaves copyState stuck at
"error"; reset it to "idle" after a short delay, matching the existing
success-path timeout, while preserving the current error icon and accessible
label during that interval.
---
Nitpick comments:
In `@frontend/src/components/apikeys/ProxyModelList.tsx`:
- Around line 87-111: Update the isError state in ProxyModelList to include a
small “Try again” button that invokes the query’s refetch() function. Keep the
existing error message and ensure the button provides an in-place recovery path
without changing loading or empty-state behavior.
🪄 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: ef9cd006-03ef-4a67-8a38-dc80c06157b0
📒 Files selected for processing (3)
frontend/src/components/apikeys/ProxyConfigSection.tsxfrontend/src/components/apikeys/ProxyModelList.tsxfrontend/src/components/settings/api/LocalProxySettings.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/apikeys/ProxyConfigSection.tsx
| <div> | ||
| <p className="text-sm font-medium"> | ||
| {models.length > 0 ? `${models.length} chat model IDs` : "Chat model IDs"} | ||
| </p> | ||
| <p className="mt-1 max-w-2xl text-xs leading-relaxed text-muted-foreground"> | ||
| Fetched from Maple's current client catalog. This confirms configured support, not | ||
| real-time provider uptime or every API-only model. | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pluralization bug in header text.
${models.length} chat model IDs renders "1 chat model IDs" when exactly one model is present (e.g. when only the fallback/catalog model is available).
✏️ Proposed fix
- {models.length > 0 ? `${models.length} chat model IDs` : "Chat model IDs"}
+ {models.length > 0
+ ? `${models.length} chat model ${models.length === 1 ? "ID" : "IDs"}`
+ : "Chat model IDs"}📝 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.
| <div> | |
| <p className="text-sm font-medium"> | |
| {models.length > 0 ? `${models.length} chat model IDs` : "Chat model IDs"} | |
| </p> | |
| <p className="mt-1 max-w-2xl text-xs leading-relaxed text-muted-foreground"> | |
| Fetched from Maple's current client catalog. This confirms configured support, not | |
| real-time provider uptime or every API-only model. | |
| </p> | |
| </div> | |
| <div> | |
| <p className="text-sm font-medium"> | |
| {models.length > 0 | |
| ? `${models.length} chat model ${models.length === 1 ? "ID" : "IDs"}` | |
| : "Chat model IDs"} | |
| </p> | |
| <p className="mt-1 max-w-2xl text-xs leading-relaxed text-muted-foreground"> | |
| Fetched from Maple's current client catalog. This confirms configured support, not | |
| real-time provider uptime or every API-only model. | |
| </p> | |
| </div> |
🤖 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 `@frontend/src/components/apikeys/ProxyModelList.tsx` around lines 77 - 85,
Update the header text in ProxyModelList so the singular case renders “1 chat
model ID” and all other counts render “N chat model IDs”; preserve the existing
“Chat model IDs” label when the list is empty.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const handleAutoStartChange = async (checked: boolean) => { | ||
| const updatedConfig = { ...config, auto_start: checked }; | ||
| setConfig(updatedConfig); | ||
| try { | ||
| await proxyService.saveManualProxySettings(updatedConfig); | ||
| setMessage({ | ||
| type: "success", | ||
| text: checked ? "Auto-start enabled" : "Auto-start disabled" | ||
| }); | ||
| } catch { | ||
| setMessage({ type: "error", text: "Failed to save the auto-start setting" }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🟡 Auto-start toggle stays flipped on screen even when the setting fails to save
The auto-start preference is applied to the on-screen toggle and in-memory settings (setConfig(updatedConfig) at frontend/src/components/apikeys/ProxyConfigSection.tsx:179) before the save is attempted, but the failure branch only shows an error and never restores the previous value, so the toggle keeps showing the new state that was not actually saved.
Impact: After a save failure the user sees auto-start as enabled/disabled when it was not persisted, and the next launch silently reverts it, causing confusion.
Optimistic update without revert in handleAutoStartChange
In frontend/src/components/apikeys/ProxyConfigSection.tsx:177-189, handleAutoStartChange optimistically calls setConfig(updatedConfig) then awaits proxyService.saveManualProxySettings(updatedConfig). On rejection the catch only sets an error message; it does not call setConfig to restore the prior auto_start value. The Switch is controlled by config.auto_start ?? false (:421), so it continues to render the un-persisted state, and the in-memory config.auto_start (later reused when building the start config in handleStartProxy) also disagrees with what is stored on disk. The PR's own release notes state "Auto-start changes now roll back when saving fails," which this handler does not implement.
| const handleAutoStartChange = async (checked: boolean) => { | |
| const updatedConfig = { ...config, auto_start: checked }; | |
| setConfig(updatedConfig); | |
| try { | |
| await proxyService.saveManualProxySettings(updatedConfig); | |
| setMessage({ | |
| type: "success", | |
| text: checked ? "Auto-start enabled" : "Auto-start disabled" | |
| }); | |
| } catch { | |
| setMessage({ type: "error", text: "Failed to save the auto-start setting" }); | |
| } | |
| }; | |
| const handleAutoStartChange = async (checked: boolean) => { | |
| const previousConfig = config; | |
| const updatedConfig = { ...config, auto_start: checked }; | |
| setConfig(updatedConfig); | |
| try { | |
| await proxyService.saveManualProxySettings(updatedConfig); | |
| setMessage({ | |
| type: "success", | |
| text: checked ? "Auto-start enabled" : "Auto-start disabled" | |
| }); | |
| } catch { | |
| setConfig(previousConfig); | |
| setMessage({ type: "error", text: "Failed to save the auto-start setting" }); | |
| } | |
| }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
3d0c18c to
1532145
Compare
| } catch (error) { | ||
| console.error(`Failed to copy ${label}:`, error); | ||
| setCopyState("error"); | ||
| } |
There was a problem hiding this comment.
🟡 Copy button stays stuck on an error icon after a failed copy
When copying fails, the copy control is switched to its error appearance (at frontend/src/components/apikeys/ProxyClientGuides.tsx:48-51) without any timer to return it to normal, unlike the success case which clears itself after two seconds, so it keeps showing a failure indicator indefinitely.
Impact: Users see a persistent "copy failed" state on the button until they happen to click and succeed again, which can look broken.
State transition mechanism
In CopyButton, the success path calls setCopyState("copied") then setTimeout(() => setCopyState("idle"), 2000) (frontend/src/components/apikeys/ProxyClientGuides.tsx:46-47), but the catch branch only calls setCopyState("error") with no reset timer. The same pattern exists in ModelId at frontend/src/components/apikeys/ProxyModelList.tsx:23-25. The state only recovers on a subsequent successful copy.
| } catch (error) { | |
| console.error(`Failed to copy ${label}:`, error); | |
| setCopyState("error"); | |
| } | |
| } catch (error) { | |
| console.error(`Failed to copy ${label}:`, error); | |
| setCopyState("error"); | |
| setTimeout(() => setCopyState("idle"), 2000); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@frontend/src/components/apikeys/ProxyConfigSection.tsx`:
- Around line 102-111: The date-based key generation flow around keyName and
existingKey must not dead-end when today’s maple-desktop key already exists and
no API key is configured. Generate a collision-free key name by appending a
unique suffix, or reuse/allow selection of the existing key through the
component’s available flow, while preserving the existing behavior for
non-colliding keys.
- Around line 100-132: Move the port parsing and range validation in the save
flow ahead of the apiKey generation block, so invalid ports return before
onRequestNewApiKey is called. Preserve the existing validation message and key
creation behavior for valid ports.
🪄 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: 7ff0bad7-5399-40ed-8926-d0a25cdf2d3b
📒 Files selected for processing (5)
frontend/src/components/apikeys/ProxyClientGuides.tsxfrontend/src/components/apikeys/ProxyConfigSection.tsxfrontend/src/components/settings/api/LocalProxySettings.tsxfrontend/src/services/proxyModels.test.tsfrontend/src/services/proxyModels.ts
💤 Files with no reviewable changes (2)
- frontend/src/services/proxyModels.test.ts
- frontend/src/services/proxyModels.ts
| let apiKey = config.api_key; | ||
| if (!apiKey) { | ||
| const date = new Date().toISOString().split("T")[0].replace(/-/g, ""); | ||
| const keyName = `maple-desktop-${date}`; | ||
|
|
||
| // Check if a key with this name already exists | ||
| const existingKey = apiKeys.find((k) => k.name === keyName); | ||
| const existingKey = apiKeys.find((key) => key.name === keyName); | ||
| if (existingKey) { | ||
| // Use the existing key (we don't have access to the actual key value) | ||
| setMessage({ | ||
| type: "error", | ||
| text: "Please select an existing API key or create a new one" | ||
| }); | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| // Request a new API key | ||
| try { | ||
| apiKey = await onRequestNewApiKey(keyName); | ||
| setConfig((prev) => ({ ...prev, api_key: apiKey })); | ||
| setConfig((previous) => ({ ...previous, api_key: apiKey })); | ||
| } catch { | ||
| setMessage({ | ||
| type: "error", | ||
| text: "Failed to create API key. Please create an API key manually first" | ||
| }); | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // Validate port range (1-65535) | ||
| const port = Number(config.port); | ||
| if (!Number.isInteger(port) || port < 1 || port > 65535) { | ||
| setMessage({ | ||
| type: "error", | ||
| text: `Invalid port: ${config.port}. Port must be between 1 and 65535.` | ||
| }); | ||
| setIsLoading(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate port before creating an API key. The key-generation block (Lines 100-123) runs before the port range check (Lines 125-132). With an out-of-range port (e.g. 99999, reachable via the number input) and no saved api_key, onRequestNewApiKey creates a real, non-reversible key on the account, then the flow aborts with an "invalid port" error — leaving an orphaned credential. Move the port validation ahead of the key creation.
🐛 Proposed reorder
try {
+ const port = Number(config.port);
+ if (!Number.isInteger(port) || port < 1 || port > 65535) {
+ setMessage({
+ type: "error",
+ text: `Invalid port: ${config.port}. Port must be between 1 and 65535.`
+ });
+ return;
+ }
+
let apiKey = config.api_key;
if (!apiKey) {
const date = new Date().toISOString().split("T")[0].replace(/-/g, "");
const keyName = `maple-desktop-${date}`;
const existingKey = apiKeys.find((key) => key.name === keyName);
if (existingKey) {
setMessage({
type: "error",
text: "Please select an existing API key or create a new one"
});
return;
}
try {
apiKey = await onRequestNewApiKey(keyName);
setConfig((previous) => ({ ...previous, api_key: apiKey }));
} catch {
setMessage({
type: "error",
text: "Failed to create API key. Please create an API key manually first"
});
return;
}
}
-
- const port = Number(config.port);
- if (!Number.isInteger(port) || port < 1 || port > 65535) {
- setMessage({
- type: "error",
- text: `Invalid port: ${config.port}. Port must be between 1 and 65535.`
- });
- return;
- }📝 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.
| let apiKey = config.api_key; | |
| if (!apiKey) { | |
| const date = new Date().toISOString().split("T")[0].replace(/-/g, ""); | |
| const keyName = `maple-desktop-${date}`; | |
| // Check if a key with this name already exists | |
| const existingKey = apiKeys.find((k) => k.name === keyName); | |
| const existingKey = apiKeys.find((key) => key.name === keyName); | |
| if (existingKey) { | |
| // Use the existing key (we don't have access to the actual key value) | |
| setMessage({ | |
| type: "error", | |
| text: "Please select an existing API key or create a new one" | |
| }); | |
| setIsLoading(false); | |
| return; | |
| } | |
| // Request a new API key | |
| try { | |
| apiKey = await onRequestNewApiKey(keyName); | |
| setConfig((prev) => ({ ...prev, api_key: apiKey })); | |
| setConfig((previous) => ({ ...previous, api_key: apiKey })); | |
| } catch { | |
| setMessage({ | |
| type: "error", | |
| text: "Failed to create API key. Please create an API key manually first" | |
| }); | |
| setIsLoading(false); | |
| return; | |
| } | |
| } | |
| // Validate port range (1-65535) | |
| const port = Number(config.port); | |
| if (!Number.isInteger(port) || port < 1 || port > 65535) { | |
| setMessage({ | |
| type: "error", | |
| text: `Invalid port: ${config.port}. Port must be between 1 and 65535.` | |
| }); | |
| setIsLoading(false); | |
| return; | |
| } | |
| try { | |
| const port = Number(config.port); | |
| if (!Number.isInteger(port) || port < 1 || port > 65535) { | |
| setMessage({ | |
| type: "error", | |
| text: `Invalid port: ${config.port}. Port must be between 1 and 65535.` | |
| }); | |
| return; | |
| } | |
| let apiKey = config.api_key; | |
| if (!apiKey) { | |
| const date = new Date().toISOString().split("T")[0].replace(/-/g, ""); | |
| const keyName = `maple-desktop-${date}`; | |
| const existingKey = apiKeys.find((key) => key.name === keyName); | |
| if (existingKey) { | |
| setMessage({ | |
| type: "error", | |
| text: "Please select an existing API key or create a new one" | |
| }); | |
| return; | |
| } | |
| try { | |
| apiKey = await onRequestNewApiKey(keyName); | |
| setConfig((previous) => ({ ...previous, api_key: apiKey })); | |
| } catch { | |
| setMessage({ | |
| type: "error", | |
| text: "Failed to create API key. Please create an API key manually first" | |
| }); | |
| return; | |
| } | |
| } |
🤖 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 `@frontend/src/components/apikeys/ProxyConfigSection.tsx` around lines 100 -
132, Move the port parsing and range validation in the save flow ahead of the
apiKey generation block, so invalid ports return before onRequestNewApiKey is
called. Preserve the existing validation message and key creation behavior for
valid ports.
| const date = new Date().toISOString().split("T")[0].replace(/-/g, ""); | ||
| const keyName = `maple-desktop-${date}`; | ||
|
|
||
| // Check if a key with this name already exists | ||
| const existingKey = apiKeys.find((k) => k.name === keyName); | ||
| const existingKey = apiKeys.find((key) => key.name === keyName); | ||
| if (existingKey) { | ||
| // Use the existing key (we don't have access to the actual key value) | ||
| setMessage({ | ||
| type: "error", | ||
| text: "Please select an existing API key or create a new one" | ||
| }); | ||
| setIsLoading(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Potential start dead-end when today's key name already exists. keyName is date-based (maple-desktop-<today>), so if a key with that name exists but config.api_key is empty, the flow errors with "select an existing API key or create a new one" — yet this component offers no key selector, and re-creating would collide on the same name for the rest of the day. Consider appending a unique suffix or letting the user pick an existing key.
🤖 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 `@frontend/src/components/apikeys/ProxyConfigSection.tsx` around lines 102 -
111, The date-based key generation flow around keyName and existingKey must not
dead-end when today’s maple-desktop key already exists and no API key is
configured. Generate a collision-free key name by appending a unique suffix, or
reuse/allow selection of the existing key through the component’s available
flow, while preserving the existing behavior for non-colliding keys.
1 participant
Summary
Validation
Separate follow-ups
Potential proxy behavior changes are deliberately excluded from this documentation and UI PR:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation