🤖 fix: instant UI updates when saving provider settings

ibetitsmike · ibetitsmike · commit 8a42d154cad0 · 2025-12-03T15:33:37.000Z
Use optimistic updates for immediate feedback when saving API keys/base URLs.
Subscription still exists for external changes (CLI, other components).

Changes:
- Add useProvidersConfig hook with optimistic update support
- ProvidersSection updates UI immediately on save, persists in background
- ModelsSection uses hook for cross-component sync
- ChatInput subscribes to config changes for OpenAI key status

_Generated with `mux`_
diff --git a/src/browser/components/ChatInput/index.tsx b/src/browser/components/ChatInput/index.tsx
@@ -413,25 +413,40 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
   }, [api]);
 
   // Check if OpenAI API key is configured (for voice input)
+  // Subscribe to config changes so key status updates immediately when set in Settings
   useEffect(() => {
-    let isMounted = true;
+    if (!api) return;
+    const abortController = new AbortController();
+    const signal = abortController.signal;
 
     const checkOpenAIKey = async () => {
       try {
-        const config = await api?.providers.getConfig();
-        if (isMounted) {
+        const config = await api.providers.getConfig();
+        if (!signal.aborted) {
           setOpenAIKeySet(config?.openai?.apiKeySet ?? false);
         }
-      } catch (error) {
-        console.error("Failed to check OpenAI API key:", error);
+      } catch {
+        // Ignore errors fetching config
       }
     };
 
+    // Initial fetch
     void checkOpenAIKey();
 
-    return () => {
-      isMounted = false;
-    };
+    // Subscribe to provider config changes via oRPC
+    (async () => {
+      try {
+        const iterator = await api.providers.onConfigChanged(undefined, { signal });
+        for await (const _ of iterator) {
+          if (signal.aborted) break;
+          void checkOpenAIKey();
+        }
+      } catch {
+        // Subscription cancelled via abort signal - expected on cleanup
+      }
+    })();
+
+    return () => abortController.abort();
   }, [api]);
 
   // Allow external components (e.g., CommandPalette, Queued message edits) to insert text
diff --git a/src/browser/components/Settings/sections/ModelsSection.tsx b/src/browser/components/Settings/sections/ModelsSection.tsx
@@ -1,11 +1,11 @@
-import React, { useState, useEffect, useCallback } from "react";
+import React, { useState, useCallback } from "react";
 import { Plus, Loader2 } from "lucide-react";
-import type { ProvidersConfigMap } from "../types";
 import { SUPPORTED_PROVIDERS, PROVIDER_DISPLAY_NAMES } from "@/common/constants/providers";
 import { KNOWN_MODELS } from "@/common/constants/knownModels";
 import { useModelLRU } from "@/browser/hooks/useModelLRU";
 import { ModelRow } from "./ModelRow";
 import { useAPI } from "@/browser/contexts/API";
+import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig";
 
 interface NewModelForm {
   provider: string;
@@ -20,22 +20,13 @@ interface EditingState {
 
 export function ModelsSection() {
   const { api } = useAPI();
-  const [config, setConfig] = useState<ProvidersConfigMap | null>(null);
+  const { config, loading } = useProvidersConfig();
   const [newModel, setNewModel] = useState<NewModelForm>({ provider: "", modelId: "" });
   const [saving, setSaving] = useState(false);
   const [editing, setEditing] = useState<EditingState | null>(null);
   const [error, setError] = useState<string | null>(null);
   const { defaultModel, setDefaultModel } = useModelLRU();
 
-  // Load config on mount
-  useEffect(() => {
-    if (!api) return;
-    void (async () => {
-      const cfg = await api.providers.getConfig();
-      setConfig(cfg ?? null);
-    })();
-  }, [api]);
-
   // Check if a model already exists (for duplicate prevention)
   const modelExists = useCallback(
     (provider: string, modelId: string, excludeOriginal?: string): boolean => {
@@ -65,10 +56,7 @@ export function ModelsSection() {
       const updatedModels = [...currentModels, trimmedModelId];
 
       await api.providers.setModels({ provider: newModel.provider, models: updatedModels });
-
-      // Refresh config
-      const cfg = await api.providers.getConfig();
-      setConfig(cfg ?? null);
+      // Config refresh happens automatically via useProvidersConfig subscription
       setNewModel({ provider: "", modelId: "" });
     } finally {
       setSaving(false);
@@ -84,10 +72,7 @@ export function ModelsSection() {
         const updatedModels = currentModels.filter((m) => m !== modelId);
 
         await api.providers.setModels({ provider, models: updatedModels });
-
-        // Refresh config
-        const cfg = await api.providers.getConfig();
-        setConfig(cfg ?? null);
+        // Config refresh happens automatically via useProvidersConfig subscription
       } finally {
         setSaving(false);
       }
@@ -131,18 +116,15 @@ export function ModelsSection() {
       );
 
       await api.providers.setModels({ provider: editing.provider, models: updatedModels });
-
-      // Refresh config
-      const cfg = await api.providers.getConfig();
-      setConfig(cfg ?? null);
+      // Config refresh happens automatically via useProvidersConfig subscription
       setEditing(null);
     } finally {
       setSaving(false);
     }
   }, [api, editing, config, modelExists]);
 
   // Show loading state while config is being fetched
-  if (config === null) {
+  if (loading || !config) {
     return (
       <div className="flex items-center justify-center gap-2 py-12">
         <Loader2 className="text-muted h-5 w-5 animate-spin" />
diff --git a/src/browser/components/Settings/sections/ProvidersSection.tsx b/src/browser/components/Settings/sections/ProvidersSection.tsx
@@ -1,10 +1,10 @@
-import React, { useState, useEffect, useCallback } from "react";
+import React, { useState, useCallback } from "react";
 import { ChevronDown, ChevronRight, Check, X } from "lucide-react";
-import type { ProvidersConfigMap } from "../types";
 import { SUPPORTED_PROVIDERS } from "@/common/constants/providers";
 import type { ProviderName } from "@/common/constants/providers";
 import { ProviderWithIcon } from "@/browser/components/ProviderIcon";
 import { useAPI } from "@/browser/contexts/API";
+import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig";
 
 interface FieldConfig {
   key: string;
@@ -66,23 +66,13 @@ function getProviderFields(provider: ProviderName): FieldConfig[] {
 
 export function ProvidersSection() {
   const { api } = useAPI();
-  const [config, setConfig] = useState<ProvidersConfigMap>({});
+  const { config, updateOptimistically } = useProvidersConfig();
   const [expandedProvider, setExpandedProvider] = useState<string | null>(null);
   const [editingField, setEditingField] = useState<{
     provider: string;
     field: string;
   } | null>(null);
   const [editValue, setEditValue] = useState("");
-  const [saving, setSaving] = useState(false);
-
-  // Load config on mount
-  useEffect(() => {
-    if (!api) return;
-    void (async () => {
-      const cfg = await api.providers.getConfig();
-      setConfig(cfg);
-    })();
-  }, [api]);
 
   const handleToggleProvider = (provider: string) => {
     setExpandedProvider((prev) => (prev === provider ? null : provider));
@@ -102,41 +92,48 @@ export function ProvidersSection() {
     setEditValue("");
   };
 
-  const handleSaveEdit = useCallback(async () => {
+  const handleSaveEdit = useCallback(() => {
     if (!editingField || !api) return;
 
-    setSaving(true);
-    try {
-      const { provider, field } = editingField;
-      await api.providers.setProviderConfig({ provider, keyPath: [field], value: editValue });
+    const { provider, field } = editingField;
 
-      // Refresh config
-      const cfg = await api.providers.getConfig();
-      setConfig(cfg);
-      setEditingField(null);
-      setEditValue("");
-    } finally {
-      setSaving(false);
+    // Optimistic update for instant feedback
+    if (field === "apiKey") {
+      updateOptimistically(provider, { apiKeySet: editValue !== "" });
+    } else if (field === "baseUrl") {
+      updateOptimistically(provider, { baseUrl: editValue || undefined });
+    } else if (field === "voucher") {
+      updateOptimistically(provider, { voucherSet: editValue !== "" });
     }
-  }, [api, editingField, editValue]);
+
+    setEditingField(null);
+    setEditValue("");
+
+    // Save in background
+    void api.providers.setProviderConfig({ provider, keyPath: [field], value: editValue });
+  }, [api, editingField, editValue, updateOptimistically]);
 
   const handleClearField = useCallback(
-    async (provider: string, field: string) => {
+    (provider: string, field: string) => {
       if (!api) return;
-      setSaving(true);
-      try {
-        await api.providers.setProviderConfig({ provider, keyPath: [field], value: "" });
-        const cfg = await api.providers.getConfig();
-        setConfig(cfg);
-      } finally {
-        setSaving(false);
+
+      // Optimistic update for instant feedback
+      if (field === "apiKey") {
+        updateOptimistically(provider, { apiKeySet: false });
+      } else if (field === "baseUrl") {
+        updateOptimistically(provider, { baseUrl: undefined });
+      } else if (field === "voucher") {
+        updateOptimistically(provider, { voucherSet: false });
       }
+
+      // Save in background
+      void api.providers.setProviderConfig({ provider, keyPath: [field], value: "" });
     },
-    [api]
+    [api, updateOptimistically]
   );
 
   const isConfigured = (provider: string): boolean => {
-    const providerConfig = config[provider];
+    const providerConfig = config?.[provider];
     if (!providerConfig) return false;
 
     // For Bedrock, check if any AWS credential field is set
@@ -155,7 +152,7 @@ export function ProvidersSection() {
   };
 
   const getFieldValue = (provider: string, field: string): string | undefined => {
-    const providerConfig = config[provider];
+    const providerConfig = config?.[provider];
     if (!providerConfig) return undefined;
 
     // For bedrock, check aws nested object for region
@@ -169,7 +166,7 @@ export function ProvidersSection() {
   };
 
   const isFieldSet = (provider: string, field: string, fieldConfig: FieldConfig): boolean => {
-    const providerConfig = config[provider];
+    const providerConfig = config?.[provider];
     if (!providerConfig) return false;
 
     if (fieldConfig.type === "secret") {
@@ -261,14 +258,13 @@ export function ProvidersSection() {
                             className="bg-modal-bg border-border-medium focus:border-accent flex-1 rounded border px-2 py-1.5 font-mono text-xs focus:outline-none"
                             autoFocus
                             onKeyDown={(e) => {
-                              if (e.key === "Enter") void handleSaveEdit();
+                              if (e.key === "Enter") handleSaveEdit();
                               if (e.key === "Escape") handleCancelEdit();
                             }}
                           />
                           <button
                             type="button"
-                            onClick={() => void handleSaveEdit()}
-                            disabled={saving}
+                            onClick={handleSaveEdit}
                             className="p-1 text-green-500 hover:text-green-400"
                           >
                             <Check className="h-4 w-4" />
@@ -296,8 +292,7 @@ export function ProvidersSection() {
                               : fieldConfig.type === "secret" && fieldIsSet) && (
                               <button
                                 type="button"
-                                onClick={() => void handleClearField(provider, fieldConfig.key)}
-                                disabled={saving}
+                                onClick={() => handleClearField(provider, fieldConfig.key)}
                                 className="text-muted hover:text-error text-xs"
                               >
                                 Clear
diff --git a/src/browser/hooks/useProvidersConfig.ts b/src/browser/hooks/useProvidersConfig.ts
@@ -0,0 +1,69 @@
+import { useEffect, useState, useCallback } from "react";
+import { useAPI } from "@/browser/contexts/API";
+import type { ProvidersConfigMap, ProviderConfigInfo } from "@/common/orpc/types";
+
+/**
+ * Hook to get provider config with automatic refresh on config changes.
+ * Subscribes to the backend's onConfigChanged event for external changes.
+ * Use updateOptimistically for instant UI feedback when saving.
+ */
+export function useProvidersConfig() {
+  const { api } = useAPI();
+  const [config, setConfig] = useState<ProvidersConfigMap | null>(null);
+  const [loading, setLoading] = useState(true);
+
+  const refresh = useCallback(async () => {
+    if (!api) return;
+    try {
+      const cfg = await api.providers.getConfig();
+      setConfig(cfg);
+    } catch {
+      // Ignore errors fetching config
+    } finally {
+      setLoading(false);
+    }
+  }, [api]);
+
+  /**
+   * Optimistically update local state for instant UI feedback.
+   * Call this immediately when saving, before the API call completes.
+   */
+  const updateOptimistically = useCallback(
+    (provider: string, updates: Partial<ProviderConfigInfo>) => {
+      setConfig((prev) => {
+        if (!prev) return prev;
+        return {
+          ...prev,
+          [provider]: { ...prev[provider], ...updates },
+        };
+      });
+    },
+    []
+  );
+
+  useEffect(() => {
+    if (!api) return;
+    const abortController = new AbortController();
+    const signal = abortController.signal;
+
+    // Initial fetch
+    void refresh();
+
+    // Subscribe to provider config changes via oRPC (for external changes)
+    (async () => {
+      try {
+        const iterator = await api.providers.onConfigChanged(undefined, { signal });
+        for await (const _ of iterator) {
+          if (signal.aborted) break;
+          void refresh();
+        }
+      } catch {
+        // Subscription cancelled via abort signal - expected on cleanup
+      }
+    })();
+
+    return () => abortController.abort();
+  }, [api, refresh]);
+
+  return { config, loading, refresh, updateOptimistically };
+}
