fix(providers): refresh credentials after auth save (#2884)

fix(providers): refresh credentials after auth save
2026-08-29 00:51:02 +03:00
parent 8b7ebca187 c788cd2237
commit 25c2e7b3ff
3 changed files with 351 additions and 29 deletions
@@ -4,8 +4,11 @@ import {
  getOAuthAuthMethods,
  normalizeAuthType,
  parseAuthPayload,
-  requiresOpenCodeRestartAfterOAuth,
+requiresOpenCodeRestartAfterOAuth,
+  providerHasCredentials,
+  shouldAutoOpenAuthPanel,
  shouldShowApiKeyAuth,
+  shouldShowModelsSection,
 } from './providerAuth';

 describe('ProvidersPage available provider loading', () => {
@@ -72,3 +75,148 @@ describe('provider auth method helpers', () => {
    expect(requiresOpenCodeRestartAfterOAuth('github-copilot')).toBe(true);
  });
 });
+
+describe('provider credential state helpers', () => {
+  test('providerHasCredentials requires key, options.apiKey, declared env, or auth source', () => {
+    // Built-in catalog entry with no credential signal at all.
+    expect(providerHasCredentials({ key: undefined, authSourceExists: false })).toBe(false);
+    expect(providerHasCredentials({ key: '', authSourceExists: false })).toBe(false);
+    expect(providerHasCredentials({ key: '   ', authSourceExists: false })).toBe(false);
+
+    // OpenCode reports an active credential via provider.key.
+    expect(providerHasCredentials({ key: 'sk-...', authSourceExists: false })).toBe(true);
+    // Auth.json provenance alone is enough while sources are authoritative.
+    expect(providerHasCredentials({ key: undefined, authSourceExists: true })).toBe(true);
+  });
+
+  test('providerHasCredentials counts declared env vars for multi-variable providers', () => {
+    // Bedrock/Azure/Vertex resolve credentials from several env vars, so
+    // OpenCode never sets Provider.key for them; the declared env list is the
+    // only signal that the provider is configured.
+    expect(providerHasCredentials({ key: undefined, authSourceExists: false, envDeclared: true })).toBe(true);
+    expect(providerHasCredentials({ key: undefined, authSourceExists: false, envDeclared: false })).toBe(false);
+  });
+
+  test('providerHasCredentials treats options.apiKey as a usable credential', () => {
+    // Config-defined providers ship provider.options to the client but never
+    // reach Provider.key, so the only authoritative signal is options.apiKey.
+    expect(providerHasCredentials({ key: undefined, authSourceExists: false, optionsApiKey: 'sk-config' })).toBe(true);
+    expect(providerHasCredentials({ key: undefined, authSourceExists: false, optionsApiKey: '' })).toBe(false);
+    expect(providerHasCredentials({ key: undefined, authSourceExists: false, optionsApiKey: '   ' })).toBe(false);
+    expect(providerHasCredentials({ key: undefined, authSourceExists: false, optionsApiKey: null })).toBe(false);
+  });
+
+  test('env-less OAuth-only provider without credentials opens panel and hides models', () => {
+    const hasCredentials = providerHasCredentials({
+      key: undefined,
+      authSourceExists: false,
+    });
+    expect(hasCredentials).toBe(false);
+    expect(shouldAutoOpenAuthPanel({
+      sourcesLoaded: true,
+      hasCredentials,
+      userDismissed: false,
+    })).toBe(true);
+    expect(shouldShowModelsSection({
+      modelCount: 1,
+      sourcesLoaded: true,
+      hasCredentials,
+    })).toBe(false);
+  });
+
+  test('provider with stored auth or key shows Connected and models', () => {
+    const fromKey = providerHasCredentials({ key: 'sk-live', authSourceExists: false });
+    const fromAuth = providerHasCredentials({ key: undefined, authSourceExists: true });
+    expect(fromKey).toBe(true);
+    expect(fromAuth).toBe(true);
+    expect(shouldAutoOpenAuthPanel({
+      sourcesLoaded: true,
+      hasCredentials: fromKey,
+      userDismissed: false,
+    })).toBe(false);
+    expect(shouldShowModelsSection({
+      modelCount: 3,
+      sourcesLoaded: true,
+      hasCredentials: fromAuth,
+    })).toBe(true);
+  });
+
+  test('editable custom provider keeps models visible even with no credentials signal', () => {
+    // Config-defined custom providers (e.g. local LM Studio/Ollama style)
+    // are user-editable in place; a stale 'Credentials missing' must not
+    // hide their models section. Without the exemption, a keyless local
+    // custom provider regresses to 'Credentials missing' with models hidden.
+    const hasCredentials = providerHasCredentials({
+      key: undefined,
+      authSourceExists: false,
+      optionsApiKey: null,
+    });
+    expect(hasCredentials).toBe(false);
+    expect(shouldShowModelsSection({
+      modelCount: 1,
+      sourcesLoaded: true,
+      hasCredentials: false,
+      isEditableCustomProvider: true,
+    })).toBe(true);
+    expect(shouldShowModelsSection({
+      modelCount: 1,
+      sourcesLoaded: true,
+      hasCredentials: false,
+      isEditableCustomProvider: false,
+    })).toBe(false);
+  });
+
+  test('auth save followed by providers refresh recognizes credentials without stale missing state', () => {
+    // Pre-save: sources say no auth, provider has no key yet.
+    const before = providerHasCredentials({
+      key: undefined,
+      authSourceExists: false,
+    });
+    expect(before).toBe(false);
+    expect(shouldShowModelsSection({
+      modelCount: 2,
+      sourcesLoaded: true,
+      hasCredentials: before,
+    })).toBe(false);
+
+    // After reloadOpenCodeConfiguration, providers array gets a key even if the
+    // sources snapshot has not been refetched yet.
+    const afterProvidersRefresh = providerHasCredentials({
+      key: 'oauth-token-present',
+      authSourceExists: false,
+    });
+    expect(afterProvidersRefresh).toBe(true);
+    expect(shouldAutoOpenAuthPanel({
+      sourcesLoaded: true,
+      hasCredentials: afterProvidersRefresh,
+      userDismissed: false,
+    })).toBe(false);
+    expect(shouldShowModelsSection({
+      modelCount: 2,
+      sourcesLoaded: true,
+      hasCredentials: afterProvidersRefresh,
+    })).toBe(true);
+
+    // After sources refetch completes, auth.exists also becomes true.
+    expect(providerHasCredentials({
+      key: 'oauth-token-present',
+      authSourceExists: true,
+    })).toBe(true);
+  });
+
+  test('explicit hide keeps the auth panel closed while credentials are still missing', () => {
+    expect(shouldAutoOpenAuthPanel({
+      sourcesLoaded: true,
+      hasCredentials: false,
+      userDismissed: true,
+    })).toBe(false);
+  });
+
+  test('models stay visible while sources are still loading', () => {
+    expect(shouldShowModelsSection({
+      modelCount: 4,
+      sourcesLoaded: false,
+      hasCredentials: false,
+    })).toBe(true);
+  });
+});
@@ -19,7 +19,9 @@ import {
 import { toast } from '@/components/ui';
 import { Icon } from "@/components/icon/Icon";
 import type { IconName } from "@/components/icon/icons";
-import { noteDeferredRestartFromPayload, recordDeferredOpenCodeRestart } from '@/lib/opencode/deferredRestart';
+import { reloadOpenCodeConfiguration } from '@/stores/useAgentsStore';
+import type { ConfigChangeScope } from '@/lib/configSync';
+import { recordDeferredOpenCodeRestart } from '@/lib/opencode/deferredRestart';
 import { cn } from '@/lib/utils';
 import type { ModelMetadata } from '@/types';
 import { getCurrentIntlLocale, useI18n } from '@/lib/i18n';
@@ -29,8 +31,11 @@ import { requiresProviderAuth, shouldLoadAvailableProviders } from './providerAv
 import {
  getOAuthAuthMethods,
  parseAuthPayload,
+  providerHasCredentials,
  requiresOpenCodeRestartAfterOAuth,
+  shouldAutoOpenAuthPanel,
  shouldShowApiKeyAuth,
+  shouldShowModelsSection,
  type AuthMethod,
  type OAuthAuthMethodEntry,
 } from './providerAuth';
@@ -48,6 +53,14 @@ import {
  type ProviderConfigScope,
 } from './custom-provider-form';

+/**
+ * Providers whose credentials come from several env vars (Bedrock, Azure,
+ * Vertex) never get a single resolved `Provider.key` from OpenCode, so the
+ * declared env list is the only signal that they are configured at all.
+ */
+const providerDeclaresEnv = (provider: { env?: string[] } | undefined): boolean =>
+  Array.isArray(provider?.env) && provider.env.some((name) => name.trim().length > 0);
+
 const formatCompactNumber = (value: number) => new Intl.NumberFormat(getCurrentIntlLocale(), {
  notation: 'compact',
  compactDisplay: 'short',
@@ -170,7 +183,11 @@ export const ProvidersPage: React.FC = () => {
  const [providerSearchQuery, setProviderSearchQuery] = React.useState('');
  const [providerDropdownOpen, setProviderDropdownOpen] = React.useState(false);
  const [providerSources, setProviderSources] = React.useState<Record<string, ProviderSources>>({});
+  // Bumped after auth writes so the source snapshot is refetched even when the
+  // selected provider id is unchanged (OAuth/API key success path).
+  const [providerSourcesRevision, setProviderSourcesRevision] = React.useState(0);
  const [showAuthPanel, setShowAuthPanel] = React.useState(false);
+  const [authPanelDismissedForId, setAuthPanelDismissedForId] = React.useState<string | null>(null);
  const [editingCustomProviderId, setEditingCustomProviderId] = React.useState<string | null>(null);
  const [editingCustomFormInitial, setEditingCustomFormInitial] = React.useState<CustomProviderFormState | null>(null);
  const [editingCustomScope, setEditingCustomScope] = React.useState<ProviderConfigScope | null>(null);
@@ -298,6 +315,7 @@ export const ProvidersPage: React.FC = () => {
  React.useEffect(() => {
    if (selectedProviderId === ADD_PROVIDER_ID) {
      setShowAuthPanel(true);
+      setAuthPanelDismissedForId(null);
      setEditingCustomProviderId(null);
      setEditingCustomFormInitial(null);
      setEditingCustomScope(null);
@@ -306,6 +324,7 @@ export const ProvidersPage: React.FC = () => {
    }

    setShowAuthPanel(false);
+    setAuthPanelDismissedForId(null);
    if (editingCustomProviderId && editingCustomProviderId !== selectedProviderId) {
      setEditingCustomProviderId(null);
      setEditingCustomFormInitial(null);
@@ -315,7 +334,7 @@ export const ProvidersPage: React.FC = () => {
  }, [selectedProviderId, editingCustomProviderId]);

  // Unauthenticated providers (OAuth-only plugins before login) should open the
-  // auth panel instead of a false "Connected" summary.
+  // auth panel instead of a false "Connected" summary. Respect an explicit Hide.
  React.useEffect(() => {
    if (!selectedProviderId || selectedProviderId === ADD_PROVIDER_ID) {
      return;
@@ -325,15 +344,26 @@ export const ProvidersPage: React.FC = () => {
      return;
    }
    const provider = providers.find((entry) => entry.id === selectedProviderId);
-    const envEntries = Array.isArray(provider?.env)
-      ? provider.env.filter((entry): entry is string => typeof entry === 'string' && entry.trim().length > 0)
-      : [];
-    const hasCreds = Boolean(sources.auth.exists) || envEntries.length > 0;
-    const isCustomProvider = Boolean(provider && isConfigDefinedCustomProvider(provider, sources));
-    if (requiresProviderAuth(true, hasCreds, isCustomProvider)) {
+    const hasCreds = providerHasCredentials({
+      key: provider?.key,
+      authSourceExists: sources.auth.exists,
+      optionsApiKey: (provider as { options?: { apiKey?: string | null } } | undefined)?.options?.apiKey ?? null,
+      envDeclared: providerDeclaresEnv(provider),
+    });
+    const isEditableCustomProvider = Boolean(
+      provider && isConfigDefinedCustomProvider(provider, sources)
+    );
+    if (
+      shouldAutoOpenAuthPanel({
+        sourcesLoaded: true,
+        hasCredentials: hasCreds,
+        userDismissed: authPanelDismissedForId === selectedProviderId,
+        isEditableCustomProvider,
+      })
+    ) {
      setShowAuthPanel(true);
    }
-  }, [selectedProviderId, providerSources, providers]);
+  }, [selectedProviderId, providerSources, providers, authPanelDismissedForId]);

  React.useEffect(() => {
    if (!selectedProviderId || selectedProviderId === ADD_PROVIDER_ID) {
@@ -376,8 +406,60 @@ export const ProvidersPage: React.FC = () => {
    return () => {
      cancelled = true;
    };
-  }, [selectedProviderId, settingsDirectory, t]);
+  }, [selectedProviderId, providerSourcesRevision, settingsDirectory, t]);

+  const refreshProviderSources = React.useCallback(() => {
+    setProviderSourcesRevision((revision) => revision + 1);
+  }, []);
+
+  const markAuthWriteSucceeded = React.useCallback((providerId: string) => {
+    // Optimistically mark auth present so a providers refresh that has not yet
+    // stamped provider.key cannot reopen the panel / hide models with a stale
+    // "Credentials missing" summary before the source refetch lands.
+    setProviderSources((prev) => {
+      const existing = prev[providerId];
+      return {
+        ...prev,
+        [providerId]: {
+          auth: { exists: true, path: existing?.auth.path ?? null },
+          user: existing?.user ?? { exists: false, path: null },
+          project: existing?.project ?? { exists: false, path: null },
+          ...(existing?.custom ? { custom: existing.custom } : {}),
+        },
+      };
+    });
+    setAuthPanelDismissedForId(null);
+    setShowAuthPanel(false);
+    setSelectedProvider(providerId);
+    refreshProviderSources();
+  }, [refreshProviderSources, setSelectedProvider]);
+
+  // The mutation above already persisted to disk. If OpenCode is externally
+  // managed (e.g. the user is running a separate `opencode serve` they have to
+  // restart themselves), reloadOpenCodeConfiguration throws with
+  // `requiresManualRestart`. Surface the restart guidance instead of a
+  // misleading "mutation failed" toast and ensure the deferred-restart
+  // payload is recorded so the Settings page can show pending-restart
+  // guidance consistently across providers, API keys, custom providers,
+  // and disconnects.
+  const applyConfigReloadOrRecordDeferred = React.useCallback(
+    async (scope: ConfigChangeScope, idForDeferred?: string) => {
+      try {
+        await reloadOpenCodeConfiguration({ scopes: [scope], mode: 'active' });
+        return 'reloaded';
+      } catch (error) {
+        const requiresManual = (error as Error & { requiresManualRestart?: boolean })?.requiresManualRestart === true;
+        if (requiresManual) {
+          if (idForDeferred) {
+            recordDeferredOpenCodeRestart(scope, { id: idForDeferred });
+          }
+          return 'manual-restart';
+        }
+        throw error;
+      }
+    },
+    [],
+  );
  const selectedProvider = providers.find((provider) => provider.id === selectedProviderId);
  const selectedSources = selectedProviderId ? providerSources[selectedProviderId] : undefined;

@@ -402,8 +484,12 @@ export const ProvidersPage: React.FC = () => {

      toast.success(t('settings.providers.page.toast.apiKeySaved'));
      setApiKeyInputs((prev) => ({ ...prev, [providerId]: '' }));
-      recordDeferredOpenCodeRestart('providers', { id: providerId });
-      setSelectedProvider(providerId);
+      // Mutation succeeded: the auth key is on disk. The reload can fail with
+      // requiresManualRestart when OpenCode is externally managed; the helper
+      // records the deferred-restart payload instead of throwing a misleading
+      // "mutation failed" toast.
+      await applyConfigReloadOrRecordDeferred('providers', providerId);
+      markAuthWriteSucceeded(providerId);
    } catch (error) {
      console.error('Failed to save API key:', error);
      toast.error(t('settings.providers.page.toast.apiKeySaveFailed'));
@@ -460,8 +546,11 @@ export const ProvidersPage: React.FC = () => {
      setEditingCustomScope(null);
      setCustomAuthFailureHint(null);
      setLastCustomPersistId(null);
-      noteDeferredRestartFromPayload(payload, 'providers', { id: plan.providerID });
-      setSelectedProvider(plan.providerID);
+      // Mutation succeeded; route through the helper so an externally managed
+      // OpenCode does not produce a misleading "save failed" toast for a write
+      // that already persisted.
+      await applyConfigReloadOrRecordDeferred('providers', plan.providerID);
+      markAuthWriteSucceeded(plan.providerID);
    } catch (error) {
      console.error('Failed to save custom provider:', error);
      toast.error(
@@ -482,7 +571,9 @@ export const ProvidersPage: React.FC = () => {
    if (requiresOpenCodeRestartAfterOAuth(providerId)) {
      recordDeferredOpenCodeRestart('providers', { id: providerId });
    }
-    setSelectedProvider(providerId);
+    // Optimistic mark + sources refetch so the page does not stick on a stale
+    // "Credentials missing" summary while the providers refresh lands.
+    markAuthWriteSucceeded(providerId);
  };

  const handleDisconnectProvider = async (providerId: string) => {
@@ -504,9 +595,12 @@ export const ProvidersPage: React.FC = () => {
      }

      toast.success(t('settings.providers.page.toast.providerDisconnected'));
-      // Only accumulate when the server actually deferred a restart (e.g. auth removed).
-      // removed:false payloads must not create a phantom pending Apply & Restart.
-      noteDeferredRestartFromPayload(payload, 'providers', { id: providerId });
+      // Use the helper so an externally managed OpenCode that requires a manual
+      // restart records the deferred-restart guidance instead of toasting a
+      // misleading "disconnect failed" for a write that already persisted.
+      await applyConfigReloadOrRecordDeferred('providers', providerId);
+      setAuthPanelDismissedForId(null);
+      refreshProviderSources();
    } catch (error) {
      console.error('Failed to disconnect provider:', error);
      toast.error(t('settings.providers.page.toast.providerDisconnectFailed'));
@@ -771,18 +865,19 @@ export const ProvidersPage: React.FC = () => {
  const sourcesLoaded = Boolean(selectedSources);
  const isEditableCustomProvider = sourcesLoaded
    && isConfigDefinedCustomProvider(selectedProvider, selectedSources);
-  const providerEnv = Array.isArray(selectedProvider.env)
-    ? selectedProvider.env.filter((entry): entry is string => typeof entry === 'string' && entry.trim().length > 0)
-    : [];
-  const hasStoredAuth = Boolean(selectedSources?.auth.exists);
-  const hasEnvCredentials = providerEnv.length > 0;
-  const hasCredentials = hasStoredAuth || hasEnvCredentials;
-  const authStatusIncomplete = requiresProviderAuth(
+  const hasCredentials = providerHasCredentials({
+    key: selectedProvider.key,
+    authSourceExists: selectedSources?.auth.exists,
+    optionsApiKey: (selectedProvider as { options?: { apiKey?: string | null } }).options?.apiKey ?? null,
+    envDeclared: providerDeclaresEnv(selectedProvider),
+  });
+  const authStatusIncomplete = requiresProviderAuth(sourcesLoaded, hasCredentials, isEditableCustomProvider);
+  const showModelsSection = shouldShowModelsSection({
+    modelCount: providerModels.length,
    sourcesLoaded,
    hasCredentials,
    isEditableCustomProvider,
-  );
-  const showModelsSection = providerModels.length > 0 && !authStatusIncomplete;
+  });
  const incompleteAuthHint = !showApiKeyAuth && oauthAuthMethods.length > 0
    ? t('settings.providers.page.auth.useReconnectHint')
    : t('settings.providers.page.auth.incompleteHint');
@@ -852,7 +947,11 @@ export const ProvidersPage: React.FC = () => {
              variant="outline"
              size="xs"
              className="!font-normal"
-              onClick={() => setShowAuthPanel((prev) => !prev)}
+              onClick={() => {
+                const nextOpen = !showAuthPanel;
+                setShowAuthPanel(nextOpen);
+                setAuthPanelDismissedForId(nextOpen ? null : selectedProvider.id);
+              }}
            >
              {showAuthPanel ? t('settings.providers.page.actions.hide') : t('settings.providers.page.actions.reconnect')}
            </Button>
@@ -60,3 +60,78 @@ export const getOAuthAuthMethods = (methods: AuthMethod[]): OAuthAuthMethodEntry

 export const requiresOpenCodeRestartAfterOAuth = (providerId: string): boolean =>
  providerId !== 'claude-code';
+
+export interface ProviderCredentialInput {
+  /** Present when OpenCode reports an active credential (api/env/oauth). */
+  key?: string | null;
+  /** OpenChamber auth.json provenance for this provider. */
+  authSourceExists?: boolean | null;
+  /**
+   * Provider.options is shipped to the client for config-defined providers
+   * but never reaches `Provider.key` (upstream only sets `key` from a single
+   * resolved env var or an api-type auth.json entry). Treat a non-empty
+   * `options.apiKey` as a usable login, per
+   * `packages/web/server/lib/walkthrough/DOCUMENTATION.md:134`.
+   */
+  optionsApiKey?: string | null;
+  /**
+   * The provider declares environment variables it reads credentials from.
+   * Multi-variable providers (Bedrock, Azure, Vertex) never resolve a single
+   * `Provider.key` upstream, so without this signal they read as
+   * "Credentials missing" even when fully configured.
+   */
+  envDeclared?: boolean;
+}
+
+/**
+ * Prefer authoritative credential signals. Declared env vars are the weakest of
+ * them — the array holds variable *names*, not values — but for providers whose
+ * credentials span several env vars it is the only signal OpenCode exposes.
+ */
+export const providerHasCredentials = (input: ProviderCredentialInput): boolean => {
+  if (typeof input.key === 'string' && input.key.trim().length > 0) {
+    return true;
+  }
+  if (typeof input.optionsApiKey === 'string' && input.optionsApiKey.trim().length > 0) {
+    return true;
+  }
+  if (input.envDeclared === true) {
+    return true;
+  }
+  return input.authSourceExists === true;
+};
+
+export const shouldShowModelsSection = (input: {
+  modelCount: number;
+  sourcesLoaded: boolean;
+  hasCredentials: boolean;
+  /**
+   * Config-defined custom providers (providerSources.custom present and parsed
+   * via `isConfigDefinedCustomProvider`) are user-editable in place, so a
+   * stale `Credentials missing` signal must not hide their models section.
+   * Optional for back-compat; defaults to `false`, restoring the pre-rewrite
+   * exemption that `requiresProviderAuth` carried via `providerAvailability.ts`.
+   */
+  isEditableCustomProvider?: boolean;
+}): boolean =>
+  input.modelCount > 0 &&
+  (!input.sourcesLoaded || input.hasCredentials || Boolean(input.isEditableCustomProvider));
+
+export const shouldAutoOpenAuthPanel = (input: {
+  sourcesLoaded: boolean;
+  hasCredentials: boolean;
+  userDismissed: boolean;
+  /**
+   * Config-defined custom providers (providerSources.custom present and parsed
+   * via `isConfigDefinedCustomProvider`) do not auto-open the auth panel: the
+   * provider is editable directly in the form, and a stale `Credentials
+   * missing` summary would be misleading. Optional for back-compat; defaults to
+   * `false`, restoring the pre-rewrite exemption that `requiresProviderAuth`
+   * carried via `providerAvailability.ts`.
+   */
+  isEditableCustomProvider?: boolean;
+}): boolean =>
+  input.sourcesLoaded &&
+  !input.hasCredentials &&
+  !input.userDismissed &&
+  !input.isEditableCustomProvider;