diff --git a/packages/ui/src/components/sections/providers/ProvidersPage.test.ts b/packages/ui/src/components/sections/providers/ProvidersPage.test.ts index 05050304..25d4dbcc 100644 --- a/packages/ui/src/components/sections/providers/ProvidersPage.test.ts +++ b/packages/ui/src/components/sections/providers/ProvidersPage.test.ts @@ -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); + }); +}); diff --git a/packages/ui/src/components/sections/providers/ProvidersPage.tsx b/packages/ui/src/components/sections/providers/ProvidersPage.tsx index 9ca24ca5..32a819ce 100644 --- a/packages/ui/src/components/sections/providers/ProvidersPage.tsx +++ b/packages/ui/src/components/sections/providers/ProvidersPage.tsx @@ -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>({}); + // 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(null); const [editingCustomProviderId, setEditingCustomProviderId] = React.useState(null); const [editingCustomFormInitial, setEditingCustomFormInitial] = React.useState(null); const [editingCustomScope, setEditingCustomScope] = React.useState(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')} diff --git a/packages/ui/src/components/sections/providers/providerAuth.ts b/packages/ui/src/components/sections/providers/providerAuth.ts index ed6561b6..61a570f4 100644 --- a/packages/ui/src/components/sections/providers/providerAuth.ts +++ b/packages/ui/src/components/sections/providers/providerAuth.ts @@ -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;