fix(providers): refresh credentials after auth save
Stop treating Provider.env length as credentials, refetch provider sources after OAuth/API key writes, and respect an explicit auth-panel Hide so OAuth-only providers do not stick on a stale "Credentials missing" / empty-models state after a successful login. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com>
This commit is contained in:
committed by
herjarsa
co-authored by
Serhii Dziupin
parent
6950e113f4
commit
82bb814af7
@@ -4,8 +4,11 @@ import {
|
||||
getOAuthAuthMethods,
|
||||
normalizeAuthType,
|
||||
parseAuthPayload,
|
||||
requiresOpenCodeRestartAfterOAuth,
|
||||
requiresOpenCodeRestartAfterOAuth,
|
||||
providerHasCredentials,
|
||||
shouldAutoOpenAuthPanel,
|
||||
shouldShowApiKeyAuth,
|
||||
shouldShowModelsSection,
|
||||
} from './providerAuth';
|
||||
|
||||
describe('ProvidersPage available provider loading', () => {
|
||||
@@ -72,3 +75,106 @@ describe('provider auth method helpers', () => {
|
||||
expect(requiresOpenCodeRestartAfterOAuth('github-copilot')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('provider credential state helpers', () => {
|
||||
test('providerHasCredentials ignores declared env names and requires key or auth source', () => {
|
||||
// Built-in catalog entry with env var names but no actual credential.
|
||||
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('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('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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -29,8 +29,11 @@ import { requiresProviderAuth, shouldLoadAvailableProviders } from './providerAv
|
||||
import {
|
||||
getOAuthAuthMethods,
|
||||
parseAuthPayload,
|
||||
requiresOpenCodeRestartAfterOAuth,
|
||||
requiresOpenCodeRestartAfterOAuth,
|
||||
providerHasCredentials,
|
||||
shouldAutoOpenAuthPanel,
|
||||
shouldShowApiKeyAuth,
|
||||
shouldShowModelsSection,
|
||||
type AuthMethod,
|
||||
type OAuthAuthMethodEntry,
|
||||
} from './providerAuth';
|
||||
@@ -170,7 +173,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 +305,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 +314,7 @@ export const ProvidersPage: React.FC = () => {
|
||||
}
|
||||
|
||||
setShowAuthPanel(false);
|
||||
setAuthPanelDismissedForId(null);
|
||||
if (editingCustomProviderId && editingCustomProviderId !== selectedProviderId) {
|
||||
setEditingCustomProviderId(null);
|
||||
setEditingCustomFormInitial(null);
|
||||
@@ -315,7 +324,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 +334,20 @@ 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,
|
||||
});
|
||||
if (
|
||||
shouldAutoOpenAuthPanel({
|
||||
sourcesLoaded: true,
|
||||
hasCredentials: hasCreds,
|
||||
userDismissed: authPanelDismissedForId === selectedProviderId,
|
||||
})
|
||||
) {
|
||||
setShowAuthPanel(true);
|
||||
}
|
||||
}, [selectedProviderId, providerSources, providers]);
|
||||
}, [selectedProviderId, providerSources, providers, authPanelDismissedForId]);
|
||||
|
||||
React.useEffect(() => {
|
||||
if (!selectedProviderId || selectedProviderId === ADD_PROVIDER_ID) {
|
||||
@@ -376,7 +390,33 @@ export const ProvidersPage: React.FC = () => {
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [selectedProviderId, settingsDirectory, t]);
|
||||
}, [selectedProviderId, providerSourcesRevision, 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]);
|
||||
|
||||
const selectedProvider = providers.find((provider) => provider.id === selectedProviderId);
|
||||
const selectedSources = selectedProviderId ? providerSources[selectedProviderId] : undefined;
|
||||
@@ -402,8 +442,8 @@ export const ProvidersPage: React.FC = () => {
|
||||
|
||||
toast.success(t('settings.providers.page.toast.apiKeySaved'));
|
||||
setApiKeyInputs((prev) => ({ ...prev, [providerId]: '' }));
|
||||
recordDeferredOpenCodeRestart('providers', { id: providerId });
|
||||
setSelectedProvider(providerId);
|
||||
await reloadOpenCodeConfiguration({ scopes: ["providers"], mode: "active" });
|
||||
markAuthWriteSucceeded(providerId);
|
||||
} catch (error) {
|
||||
console.error('Failed to save API key:', error);
|
||||
toast.error(t('settings.providers.page.toast.apiKeySaveFailed'));
|
||||
@@ -460,8 +500,8 @@ export const ProvidersPage: React.FC = () => {
|
||||
setEditingCustomScope(null);
|
||||
setCustomAuthFailureHint(null);
|
||||
setLastCustomPersistId(null);
|
||||
noteDeferredRestartFromPayload(payload, 'providers', { id: plan.providerID });
|
||||
setSelectedProvider(plan.providerID);
|
||||
await reloadOpenCodeConfiguration({ scopes: ['providers'], mode: 'active' });
|
||||
markAuthWriteSucceeded(plan.providerID);
|
||||
} catch (error) {
|
||||
console.error('Failed to save custom provider:', error);
|
||||
toast.error(
|
||||
@@ -482,7 +522,62 @@ export const ProvidersPage: React.FC = () => {
|
||||
if (requiresOpenCodeRestartAfterOAuth(providerId)) {
|
||||
recordDeferredOpenCodeRestart('providers', { id: providerId });
|
||||
}
|
||||
setSelectedProvider(providerId);
|
||||
};
|
||||
|
||||
const handleOAuthComplete = async (providerId: string, methodIndex: number) => {
|
||||
const codeKey = `${providerId}:${methodIndex}`;
|
||||
const code = oauthCodes[codeKey]?.trim();
|
||||
|
||||
const busyKey = `oauth-complete:${providerId}:${methodIndex}`;
|
||||
setAuthBusyKey(busyKey);
|
||||
|
||||
try {
|
||||
const requestBody: { method: number; code?: string } = { method: methodIndex };
|
||||
if (code) {
|
||||
requestBody.code = code;
|
||||
}
|
||||
|
||||
const result = await opencodeClient.getSdkClient().provider.oauth.callback({
|
||||
providerID: providerId,
|
||||
method: requestBody.method,
|
||||
code: requestBody.code,
|
||||
});
|
||||
if (result.error) {
|
||||
throw new Error(t('settings.providers.page.toast.oauthCompleteFailed'));
|
||||
}
|
||||
|
||||
toast.success(t('settings.providers.page.toast.oauthCompleted'));
|
||||
setOauthCodes((prev) => ({ ...prev, [codeKey]: '' }));
|
||||
setPendingOAuth(null);
|
||||
await reloadOpenCodeConfiguration({ scopes: ["providers"], mode: "active" });
|
||||
markAuthWriteSucceeded(providerId);
|
||||
} catch (error) {
|
||||
console.error('Failed to complete OAuth flow:', error);
|
||||
toast.error(t('settings.providers.page.toast.oauthCompleteFailed'));
|
||||
} finally {
|
||||
setAuthBusyKey(null);
|
||||
}
|
||||
};
|
||||
|
||||
const handleCopyOAuthLink = async (url: string) => {
|
||||
const result = await copyTextToClipboard(url);
|
||||
if (result.ok) {
|
||||
toast.success(t('settings.providers.page.toast.oauthLinkCopied'));
|
||||
return;
|
||||
}
|
||||
console.error('Failed to copy OAuth link:', result.error);
|
||||
toast.error(t('settings.providers.page.toast.oauthLinkCopyFailed'));
|
||||
};
|
||||
|
||||
const handleCopyOAuthCode = async (code: string) => {
|
||||
const result = await copyTextToClipboard(code);
|
||||
if (result.ok) {
|
||||
toast.success(t('settings.providers.page.toast.deviceCodeCopied'));
|
||||
return;
|
||||
}
|
||||
console.error('Failed to copy device code:', result.error);
|
||||
toast.error(t('settings.providers.page.toast.deviceCodeCopyFailed'));
|
||||
>>>>>>> e7ca14b2b (fix(providers): refresh credentials after auth save)
|
||||
};
|
||||
|
||||
const handleDisconnectProvider = async (providerId: string) => {
|
||||
@@ -506,7 +601,9 @@ 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 });
|
||||
await reloadOpenCodeConfiguration({ scopes: ["providers"], mode: "active" });
|
||||
setAuthPanelDismissedForId(null);
|
||||
refreshProviderSources();
|
||||
} catch (error) {
|
||||
console.error('Failed to disconnect provider:', error);
|
||||
toast.error(t('settings.providers.page.toast.providerDisconnectFailed'));
|
||||
@@ -771,18 +868,16 @@ 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,
|
||||
});
|
||||
const authStatusIncomplete = sourcesLoaded && !hasCredentials;
|
||||
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,17 @@ export const ProvidersPage: React.FC = () => {
|
||||
variant="outline"
|
||||
size="xs"
|
||||
className="!font-normal"
|
||||
onClick={() => setShowAuthPanel((prev) => !prev)}
|
||||
onClick={() => {
|
||||
setShowAuthPanel((prev) => {
|
||||
const next = !prev;
|
||||
if (!next) {
|
||||
setAuthPanelDismissedForId(selectedProvider.id);
|
||||
} else {
|
||||
setAuthPanelDismissedForId(null);
|
||||
}
|
||||
return next;
|
||||
});
|
||||
}}
|
||||
>
|
||||
{showAuthPanel ? t('settings.providers.page.actions.hide') : t('settings.providers.page.actions.reconnect')}
|
||||
</Button>
|
||||
|
||||
@@ -60,3 +60,33 @@ 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;
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefer authoritative credential signals. Do not treat Provider.env length as
|
||||
* proof of credentials — that array is declared env var *names*, not values.
|
||||
*/
|
||||
export const providerHasCredentials = (input: ProviderCredentialInput): boolean => {
|
||||
if (typeof input.key === 'string' && input.key.trim().length > 0) {
|
||||
return true;
|
||||
}
|
||||
return input.authSourceExists === true;
|
||||
};
|
||||
|
||||
export const shouldShowModelsSection = (input: {
|
||||
modelCount: number;
|
||||
sourcesLoaded: boolean;
|
||||
hasCredentials: boolean;
|
||||
}): boolean => input.modelCount > 0 && (!input.sourcesLoaded || input.hasCredentials);
|
||||
|
||||
export const shouldAutoOpenAuthPanel = (input: {
|
||||
sourcesLoaded: boolean;
|
||||
hasCredentials: boolean;
|
||||
userDismissed: boolean;
|
||||
}): boolean => input.sourcesLoaded && !input.hasCredentials && !input.userDismissed;
|
||||
|
||||
Reference in New Issue
Block a user