From 859e3fd6c5a8673b394238a3bd4a674fd879adbe Mon Sep 17 00:00:00 2001 From: paisley <8197966+su8su@users.noreply.github.com> Date: Mon, 23 Mar 2026 19:11:53 +0800 Subject: [PATCH] Fix provider display (#641) --- .../services/providers/provider-service.ts | 112 +++++++++++------- electron/utils/openclaw-auth.ts | 12 ++ electron/utils/secure-storage.ts | 5 +- .../provider-service-stale-cleanup.test.ts | 91 +++++++++++--- 4 files changed, 162 insertions(+), 58 deletions(-) diff --git a/electron/services/providers/provider-service.ts b/electron/services/providers/provider-service.ts index dff5775c1..37c8eacfd 100644 --- a/electron/services/providers/provider-service.ts +++ b/electron/services/providers/provider-service.ts @@ -6,11 +6,11 @@ import type { ProviderAccount, ProviderConfig, ProviderDefinition, + ProviderType, } from '../../shared/providers/types'; import { BUILTIN_PROVIDER_TYPES } from '../../shared/providers/types'; import { ensureProviderStoreMigrated } from './provider-migration'; import { - deleteProviderAccount, getDefaultProviderAccountId, getProviderAccount, listProviderAccounts, @@ -73,30 +73,22 @@ export class ProviderService { return accounts; } - // Sync check: remove stale accounts whose provider no longer exists in - // OpenClaw JSON (e.g. user deleted openclaw.json manually). + // Sync check: hide accounts whose provider no longer exists in OpenClaw + // JSON (e.g. user deleted openclaw.json manually). We intentionally do + // NOT delete from the store — this preserves API key associations so that + // when the user restores the config, accounts reappear with keys intact. { const activeProviders = await getActiveOpenClawProviders(); + // When OpenClaw config has no providers (e.g. user deleted the file), + // treat ALL accounts as stale so ClawX stays in sync. + const configEmpty = activeProviders.size === 0; - // If the OpenClaw config is empty or unreadable, skip cleanup entirely - // to avoid accidentally wiping valid accounts during transient states - // (e.g. gateway restart, file lock, first launch before config sync). - if (activeProviders.size === 0) { - logger.warn( - '[provider-sync] OpenClaw config has no active providers — skipping stale-account cleanup to preserve existing accounts', - ); - return accounts; + if (configEmpty) { + logger.info('[provider-sync] OpenClaw config empty — hiding all provider accounts from display'); + return []; } - const staleIds: string[] = []; - - for (const account of accounts) { - const isBuiltin = (BUILTIN_PROVIDER_TYPES as readonly string[]).includes(account.vendorId); - // Builtin providers (anthropic, openai, etc.) are always retained - // because they don't require an explicit models.providers entry in - // openclaw.json — the runtime recognises them natively. - if (isBuiltin) continue; - + accounts = accounts.filter((account) => { const openClawKey = getOpenClawProviderKeyForType(account.vendorId, account.id); const isActive = activeProviders.has(account.vendorId) || @@ -104,16 +96,28 @@ export class ProviderService { activeProviders.has(openClawKey); if (!isActive) { - staleIds.push(account.id); + logger.info(`[provider-sync] Hiding stale provider account "${account.id}" (not in OpenClaw config)`); } - } + return isActive; + }); + } - if (staleIds.length > 0) { - for (const id of staleIds) { - logger.info(`[provider-sync] Removing stale provider account "${id}" (no longer in OpenClaw config)`); - await deleteProviderAccount(id); - } - return accounts.filter((a) => !staleIds.includes(a.id)); + // Import: detect providers in OpenClaw config not yet in the ClawX store. + { + const { providers: openClawProviders, defaultModel } = await getOpenClawProvidersConfig(); + const existingIds = new Set(accounts.map((a) => a.id)); + const existingVendorIds = new Set(accounts.map((a) => a.vendorId)); + const newAccounts = ProviderService.buildAccountsFromOpenClawEntries( + openClawProviders, existingIds, existingVendorIds, defaultModel, + ); + for (const account of newAccounts) { + await saveProviderAccount(account); + accounts.push(account); + } + if (newAccounts.length > 0) { + logger.info( + `[provider-sync] Imported ${newAccounts.length} new provider(s) from openclaw.json: ${newAccounts.map((a) => a.id).join(', ')}`, + ); } } @@ -127,19 +131,51 @@ export class ProviderService { private async seedAccountsFromOpenClawConfig(): Promise { const { providers, defaultModel } = await getOpenClawProvidersConfig(); - // Determine the provider prefix from the default model (e.g. "siliconflow/deepseek..." → "siliconflow") + const seeded = ProviderService.buildAccountsFromOpenClawEntries( + providers, new Set(), new Set(), defaultModel, + ); + + for (const account of seeded) { + await saveProviderAccount(account); + } + + if (seeded.length > 0) { + logger.info( + `[provider-seed] Seeded ${seeded.length} provider account(s) from openclaw.json: ${seeded.map((a) => a.id).join(', ')}`, + ); + } + + return seeded; + } + + /** + * Build ProviderAccount objects from OpenClaw config entries, skipping any + * whose id or vendorId is already represented by an existing account. + */ + static buildAccountsFromOpenClawEntries( + providers: Record>, + existingIds: Set, + existingVendorIds: Set, + defaultModel: string | undefined, + ): ProviderAccount[] { const defaultModelProvider = defaultModel?.includes('/') ? defaultModel.split('/')[0] : undefined; const now = new Date().toISOString(); - const seeded: ProviderAccount[] = []; + const built: ProviderAccount[] = []; for (const [key, entry] of Object.entries(providers)) { + if (existingIds.has(key)) continue; + const definition = getProviderDefinition(key); const isBuiltin = (BUILTIN_PROVIDER_TYPES as readonly string[]).includes(key); const vendorId = isBuiltin ? key : 'custom'; + // Skip if an account with this vendorId already exists (e.g. user already + // created "openrouter-uuid" via UI — no need to import bare "openrouter"). + if (existingVendorIds.has(vendorId)) continue; + const baseUrl = typeof entry.baseUrl === 'string' ? entry.baseUrl : definition?.providerConfig?.baseUrl; // Infer model from the default model if it belongs to this provider @@ -152,7 +188,7 @@ export class ProviderService { const account: ProviderAccount = { id: key, - vendorId: (vendorId as ProviderAccount['vendorId']), + vendorId: (vendorId as ProviderAccount['vendorId'] as ProviderType), label: definition?.name ?? key.charAt(0).toUpperCase() + key.slice(1), authMode: definition?.defaultAuthMode ?? 'api_key', baseUrl, @@ -167,17 +203,10 @@ export class ProviderService { updatedAt: now, }; - await saveProviderAccount(account); - seeded.push(account); + built.push(account); } - if (seeded.length > 0) { - logger.info( - `[provider-seed] Seeded ${seeded.length} provider account(s) from openclaw.json: ${seeded.map((a) => a.id).join(', ')}`, - ); - } - - return seeded; + return built; } async getAccount(accountId: string): Promise { @@ -242,8 +271,7 @@ export class ProviderService { */ async listLegacyProviders(): Promise { logLegacyProviderApiUsage('listLegacyProviders', 'listAccounts'); - await ensureProviderStoreMigrated(); - const accounts = await listProviderAccounts(); + const accounts = await this.listAccounts(); return accounts.map(providerAccountToConfig); } diff --git a/electron/utils/openclaw-auth.ts b/electron/utils/openclaw-auth.ts index af4b7f1db..a6b298a8d 100644 --- a/electron/utils/openclaw-auth.ts +++ b/electron/utils/openclaw-auth.ts @@ -742,6 +742,18 @@ export async function getActiveOpenClawProviders(): Promise> { } } } + + // 3. agents.defaults.model.primary — the default model reference encodes + // the provider prefix (e.g. "qwen-portal/coder-model" → "qwen-portal"). + // This covers providers that are active via OAuth or env-key but don't + // have an explicit models.providers entry. + const agents = config.agents as Record | undefined; + const defaults = agents?.defaults as Record | undefined; + const modelConfig = defaults?.model as Record | undefined; + const primaryModel = typeof modelConfig?.primary === 'string' ? modelConfig.primary : undefined; + if (primaryModel?.includes('/')) { + activeProviders.add(primaryModel.split('/')[0]); + } } catch (err) { console.warn('Failed to read openclaw.json for active providers:', err); } diff --git a/electron/utils/secure-storage.ts b/electron/utils/secure-storage.ts index d84d93df2..fc2b6fa5e 100644 --- a/electron/utils/secure-storage.ts +++ b/electron/utils/secure-storage.ts @@ -285,8 +285,9 @@ export async function getAllProvidersWithKeyInfo(): Promise< const openClawKey = getOpenClawProviderKeyForType(provider.type, provider.id); const isActive = activeOpenClawProviders.has(provider.type) || activeOpenClawProviders.has(provider.id) || activeOpenClawProviders.has(openClawKey); if (configMissing || (!isBuiltin && !isActive)) { - console.log(`[Sync] Provider ${provider.id} (${provider.type}) missing from OpenClaw, dropping from ClawX UI`); - await deleteProvider(provider.id); + console.log(`[Sync] Provider ${provider.id} (${provider.type}) missing from OpenClaw, hiding from UI`); + // Skip from display but don't delete from store — preserves API key + // associations so that restoring openclaw.json brings accounts back intact. continue; } diff --git a/tests/unit/provider-service-stale-cleanup.test.ts b/tests/unit/provider-service-stale-cleanup.test.ts index b16d6acd1..420aa342d 100644 --- a/tests/unit/provider-service-stale-cleanup.test.ts +++ b/tests/unit/provider-service-stale-cleanup.test.ts @@ -90,7 +90,7 @@ describe('ProviderService.listAccounts stale-account cleanup', () => { service = new ProviderService(); }); - it('preserves all accounts when activeProviders is empty (config missing/unreadable)', async () => { + it('hides ALL accounts when activeProviders is empty (config missing/deleted)', async () => { const accounts = [ makeAccount({ id: 'custom-1', vendorId: 'custom' as ProviderAccount['vendorId'] }), makeAccount({ id: 'moonshot-1', vendorId: 'moonshot' as ProviderAccount['vendorId'] }), @@ -101,15 +101,12 @@ describe('ProviderService.listAccounts stale-account cleanup', () => { const result = await service.listAccounts(); - // All accounts should be preserved — none deleted - expect(result).toEqual(accounts); + // All accounts hidden (not deleted) when config is empty + expect(result).toEqual([]); expect(mocks.deleteProviderAccount).not.toHaveBeenCalled(); - expect(mocks.loggerWarn).toHaveBeenCalledWith( - expect.stringContaining('skipping stale-account cleanup'), - ); }); - it('removes stale non-builtin accounts when config has active providers', async () => { + it('hides stale non-builtin accounts when config has active providers', async () => { const accounts = [ makeAccount({ id: 'moonshot-1', vendorId: 'moonshot' as ProviderAccount['vendorId'] }), makeAccount({ id: 'custom-stale', vendorId: 'custom' as ProviderAccount['vendorId'] }), @@ -120,27 +117,26 @@ describe('ProviderService.listAccounts stale-account cleanup', () => { const result = await service.listAccounts(); - // custom-stale should be deleted (non-builtin, not active) - expect(mocks.deleteProviderAccount).toHaveBeenCalledWith('custom-stale'); - expect(mocks.deleteProviderAccount).toHaveBeenCalledTimes(1); + // custom-stale hidden (not deleted) from display + expect(mocks.deleteProviderAccount).not.toHaveBeenCalled(); expect(result).toHaveLength(1); expect(result[0].id).toBe('moonshot-1'); }); - it('never removes builtin provider accounts even if not in activeProviders', async () => { + it('hides builtin provider accounts when not in activeProviders', async () => { const accounts = [ makeAccount({ id: 'anthropic-1', vendorId: 'anthropic' as ProviderAccount['vendorId'] }), makeAccount({ id: 'openai-1', vendorId: 'openai' as ProviderAccount['vendorId'] }), ]; mocks.listProviderAccounts.mockResolvedValue(accounts); - // Config has some providers, but NOT anthropic or openai explicitly + // Config has moonshot, but NOT anthropic or openai mocks.getActiveOpenClawProviders.mockResolvedValue(new Set(['moonshot'])); const result = await service.listAccounts(); - // Builtin accounts should be preserved regardless + // Builtin accounts also hidden when not in OpenClaw config expect(mocks.deleteProviderAccount).not.toHaveBeenCalled(); - expect(result).toEqual(accounts); + expect(result).toEqual([]); }); it('returns empty when no accounts and no active OpenClaw providers', async () => { @@ -168,4 +164,71 @@ describe('ProviderService.listAccounts stale-account cleanup', () => { expect(mocks.deleteProviderAccount).not.toHaveBeenCalled(); expect(result).toEqual(accounts); }); + + it('imports new providers from OpenClaw config not yet in ClawX store', async () => { + const accounts = [ + makeAccount({ id: 'moonshot', vendorId: 'moonshot' as ProviderAccount['vendorId'] }), + ]; + mocks.listProviderAccounts.mockResolvedValue(accounts); + mocks.getActiveOpenClawProviders.mockResolvedValue(new Set(['moonshot', 'siliconflow'])); + mocks.getOpenClawProvidersConfig.mockResolvedValue({ + providers: { + moonshot: { baseUrl: 'https://api.moonshot.cn/v1' }, + siliconflow: { baseUrl: 'https://api.siliconflow.cn/v1' }, + }, + defaultModel: undefined, + }); + + const result = await service.listAccounts(); + + // moonshot already exists, siliconflow should be imported + expect(mocks.saveProviderAccount).toHaveBeenCalledTimes(1); + expect(mocks.saveProviderAccount).toHaveBeenCalledWith( + expect.objectContaining({ id: 'siliconflow' }), + ); + expect(result).toHaveLength(2); + expect(result.map((a: ProviderAccount) => a.id)).toContain('siliconflow'); + }); + + it('does not import providers already in ClawX store', async () => { + const accounts = [ + makeAccount({ id: 'moonshot', vendorId: 'moonshot' as ProviderAccount['vendorId'] }), + ]; + mocks.listProviderAccounts.mockResolvedValue(accounts); + mocks.getActiveOpenClawProviders.mockResolvedValue(new Set(['moonshot'])); + mocks.getOpenClawProvidersConfig.mockResolvedValue({ + providers: { + moonshot: { baseUrl: 'https://api.moonshot.cn/v1' }, + }, + defaultModel: undefined, + }); + + const result = await service.listAccounts(); + + expect(mocks.saveProviderAccount).not.toHaveBeenCalled(); + expect(result).toHaveLength(1); + }); + + it('does not create duplicate when account id differs but vendorId matches', async () => { + // User added openrouter via UI → id is "openrouter-uuid", vendorId is "openrouter" + // openclaw.json has "openrouter" entry → should NOT import because vendorId matches + const accounts = [ + makeAccount({ id: 'openrouter-uuid-1234', vendorId: 'openrouter' as ProviderAccount['vendorId'] }), + ]; + mocks.listProviderAccounts.mockResolvedValue(accounts); + mocks.getActiveOpenClawProviders.mockResolvedValue(new Set(['openrouter'])); + mocks.getOpenClawProvidersConfig.mockResolvedValue({ + providers: { + openrouter: { baseUrl: 'https://openrouter.ai/api/v1' }, + }, + defaultModel: 'openrouter/openai/gpt-5.4', + }); + + const result = await service.listAccounts(); + + // Should NOT create a duplicate "openrouter" account + expect(mocks.saveProviderAccount).not.toHaveBeenCalled(); + expect(result).toHaveLength(1); + expect(result[0].id).toBe('openrouter-uuid-1234'); + }); });