From 07c5c4cfcc990b9b19c975e448950deda2ea7f61 Mon Sep 17 00:00:00 2001 From: paisley <8197966+su8su@users.noreply.github.com> Date: Tue, 9 Jun 2026 18:05:56 +0800 Subject: [PATCH] fix: reassign default provider when deleting the current default (#1108) Co-authored-by: Cursor --- electron/services/providers-api.ts | 24 ++++ .../specs/rules/provider-default-invariant.md | 22 ++++ .../gateway-backend-communication.md | 1 + .../reassign-default-provider-after-delete.md | 67 ++++++++++ tests/e2e/provider-lifecycle.spec.ts | 46 +++++++ tests/unit/host-services.test.ts | 114 +++++++++++++++++- 6 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 harness/specs/rules/provider-default-invariant.md create mode 100644 harness/specs/tasks/reassign-default-provider-after-delete.md diff --git a/electron/services/providers-api.ts b/electron/services/providers-api.ts index a839b92a..80c38947 100644 --- a/electron/services/providers-api.ts +++ b/electron/services/providers-api.ts @@ -46,6 +46,21 @@ function hasObjectChanges>( return keys.some((key) => JSON.stringify(existing[key]) !== JSON.stringify(patch[key])); } +function selectReplacementDefaultAccount( + accounts: ProviderAccount[], + deletedAccountId: string, +): ProviderAccount | undefined { + return accounts + .filter((account) => account.id !== deletedAccountId) + .sort((left, right) => { + if (left.enabled !== right.enabled) { + return left.enabled ? -1 : 1; + } + const updatedAtOrder = right.updatedAt.localeCompare(left.updatedAt); + return updatedAtOrder !== 0 ? updatedAtOrder : left.id.localeCompare(right.id); + })[0]; +} + function payloadString(payload: unknown, key: string): string | undefined { if (typeof payload === 'string') return payload; if (!isRecord(payload)) return undefined; @@ -362,7 +377,16 @@ async function deleteAccount( ); return { success: true }; } + const currentDefaultAccountId = await providerService.getDefaultAccountId(); + const replacementDefault = currentDefaultAccountId === accountId + ? selectReplacementDefaultAccount(await providerService.listAccounts(), accountId) + : undefined; + await providerService.deleteAccount(accountId); + if (replacementDefault) { + await providerService.setDefaultAccount(replacementDefault.id); + await syncDefaultProviderToRuntime(replacementDefault.id); + } await syncDeletedProviderToRuntime( existing ? providerAccountToConfig(existing) : null, accountId, diff --git a/harness/specs/rules/provider-default-invariant.md b/harness/specs/rules/provider-default-invariant.md new file mode 100644 index 00000000..9eede86e --- /dev/null +++ b/harness/specs/rules/provider-default-invariant.md @@ -0,0 +1,22 @@ +--- +id: provider-default-invariant +title: Provider Default Invariant +type: ai-coding-rule +appliesTo: + - gateway-backend-communication +requiredProfiles: + - fast + - comms +--- + +Provider account deletion must keep the ClawX account store and OpenClaw runtime default model aligned. + +Rules: + +- when deleting the current default and at least one provider account remains, exactly one remaining account must be selected as the default +- deleting the current default account must select a remaining enabled account before any disabled account +- replacement selection must be deterministic, preferring the most recently updated account within the same enabled state +- deleting a non-default account must not change the current default +- deleting the last account may leave the default account and OpenClaw default model unset +- replacement defaults must be persisted through the provider service and synchronized to OpenClaw before the deleted provider runtime config is removed +- tests must cover deleting the default account, a non-default account, and the last account diff --git a/harness/specs/scenarios/gateway-backend-communication.md b/harness/specs/scenarios/gateway-backend-communication.md index 8064b370..6ac01de6 100644 --- a/harness/specs/scenarios/gateway-backend-communication.md +++ b/harness/specs/scenarios/gateway-backend-communication.md @@ -32,6 +32,7 @@ requiredRules: - channel-plugin-migration-guards - capability-owner-resolution - active-config-guards + - provider-default-invariant - provider-model-metadata-preservation - comms-regression - docs-sync diff --git a/harness/specs/tasks/reassign-default-provider-after-delete.md b/harness/specs/tasks/reassign-default-provider-after-delete.md new file mode 100644 index 00000000..6017d863 --- /dev/null +++ b/harness/specs/tasks/reassign-default-provider-after-delete.md @@ -0,0 +1,67 @@ +--- +id: reassign-default-provider-after-delete +title: Reassign the default provider after deleting it +scenario: gateway-backend-communication +taskType: runtime-bridge +intent: Keep ClawX provider account state and the OpenClaw default model valid when the user deletes the current default provider while other provider accounts remain. +touchedAreas: + - harness/specs/tasks/reassign-default-provider-after-delete.md + - harness/specs/rules/provider-default-invariant.md + - harness/specs/scenarios/gateway-backend-communication.md + - electron/services/providers-api.ts + - tests/unit/host-services.test.ts + - tests/e2e/provider-lifecycle.spec.ts +expectedUserBehavior: + - Deleting the current default provider automatically promotes a remaining provider to default. + - The promoted provider remains default after the app reloads its provider snapshot. + - Deleting a non-default provider leaves the current default unchanged. + - Deleting the final provider leaves no default provider. +requiredProfiles: + - fast + - comms + - e2e +requiredRules: + - active-config-guards + - backend-communication-boundary + - provider-default-invariant + - renderer-main-boundary + - comms-regression + - docs-sync +requiredTests: + - tests/unit/host-services.test.ts + - tests/e2e/provider-lifecycle.spec.ts +acceptance: + - The typed providers deleteAccount host action detects whether the deleted account is the current default. + - When accounts remain, replacement selection prefers enabled accounts and then the most recently updated account. + - The replacement default is persisted and synchronized to OpenClaw before the deleted provider runtime configuration is removed. + - Deleting a non-default account does not invoke default-provider persistence or runtime synchronization. + - Deleting the last account does not attempt to assign a replacement default. + - Renderer transport boundaries remain unchanged. + - Focused unit tests, Electron E2E coverage, harness validation, communication replay, and communication compare pass. +docs: + required: false +--- + +## Background + +Deleting a provider account currently removes `defaultProviderAccountId` and +the matching `agents.defaults.model.primary` value when that provider was the +default. The delete path does not promote another configured account, leaving +the Models page and OpenClaw runtime without a default even when usable +providers remain. + +## Scope + +- Select a deterministic replacement when the current default account is + deleted. +- Persist and synchronize that replacement through the existing Main-owned + provider APIs. +- Add unit and Electron E2E regression coverage. + +## Out Of Scope + +- Adding a replacement-provider chooser to the delete UI. +- Changing account ordering in the Models page. +- Assigning a default when no provider accounts remain. +- README updates because the documented provider management interface and + workflow do not change. diff --git a/tests/e2e/provider-lifecycle.spec.ts b/tests/e2e/provider-lifecycle.spec.ts index 81258587..7fb4274d 100644 --- a/tests/e2e/provider-lifecycle.spec.ts +++ b/tests/e2e/provider-lifecycle.spec.ts @@ -20,6 +20,52 @@ async function seedTestProvider(page: Parameters[0]): Prom } test.describe('ClawX provider lifecycle', () => { + test('promotes a remaining provider after deleting the default provider', async ({ page }) => { + await completeSetup(page); + + await page.evaluate(async () => { + const now = new Date().toISOString(); + const providers = [ + { + id: 'moonshot-default-e2e', + name: 'Moonshot Default E2E', + type: 'moonshot', + baseUrl: 'https://api.moonshot.cn/v1', + model: 'kimi-k2.6', + enabled: true, + createdAt: now, + updatedAt: now, + }, + { + id: 'deepseek-replacement-e2e', + name: 'DeepSeek Replacement E2E', + type: 'deepseek', + baseUrl: 'https://api.deepseek.com/v1', + model: 'deepseek-v4-pro', + enabled: true, + createdAt: now, + updatedAt: new Date(Date.now() + 1_000).toISOString(), + }, + ]; + + for (const provider of providers) { + await window.electron.ipcRenderer.invoke('provider:save', provider); + } + await window.electron.ipcRenderer.invoke('provider:setDefault', providers[0].id); + }); + + await page.getByTestId('sidebar-nav-models').click(); + await expect(page.getByTestId('provider-card-moonshot-default-e2e')).toContainText('Default'); + await expect(page.getByTestId('provider-card-deepseek-replacement-e2e')).toBeVisible(); + + await page.getByTestId('provider-card-moonshot-default-e2e').hover(); + await page.getByTestId('provider-delete-moonshot-default-e2e').click(); + + await expect(page.getByTestId('provider-card-moonshot-default-e2e')).toHaveCount(0); + await expect(page.getByTestId('provider-card-deepseek-replacement-e2e')).toContainText('Default'); + await expect(page.getByTestId('provider-set-default-deepseek-replacement-e2e')).toHaveCount(0); + }); + test('shows a saved provider and removes it cleanly after deletion', async ({ page }) => { await completeSetup(page); await seedTestProvider(page); diff --git a/tests/unit/host-services.test.ts b/tests/unit/host-services.test.ts index a26d1251..b7073e1b 100644 --- a/tests/unit/host-services.test.ts +++ b/tests/unit/host-services.test.ts @@ -32,6 +32,7 @@ const { saveChannelConfigMock, setSettingMock, syncDefaultProviderToRuntimeMock, + syncDeletedProviderToRuntimeMock, syncSavedProviderToRuntimeMock, syncLaunchAtStartupSettingFromStoreMock, syncProxyConfigToOpenClawMock, @@ -99,6 +100,7 @@ const { saveChannelConfigMock: vi.fn(), setSettingMock: vi.fn(), syncDefaultProviderToRuntimeMock: vi.fn(), + syncDeletedProviderToRuntimeMock: vi.fn(), syncSavedProviderToRuntimeMock: vi.fn(), syncLaunchAtStartupSettingFromStoreMock: vi.fn(), syncProxyConfigToOpenClawMock: vi.fn(), @@ -192,7 +194,7 @@ vi.mock('@electron/services/providers/provider-runtime-sync', () => ({ syncAgentModelOverrideToRuntime: vi.fn(), syncDefaultProviderToRuntime: (...args: unknown[]) => syncDefaultProviderToRuntimeMock(...args), syncDeletedProviderApiKeyToRuntime: vi.fn(), - syncDeletedProviderToRuntime: vi.fn(), + syncDeletedProviderToRuntime: (...args: unknown[]) => syncDeletedProviderToRuntimeMock(...args), syncProviderApiKeyToRuntime: vi.fn(), syncSavedProviderToRuntime: (...args: unknown[]) => syncSavedProviderToRuntimeMock(...args), syncUpdatedProviderToRuntime: vi.fn(), @@ -481,6 +483,116 @@ describe('host services', () => { expect(syncDefaultProviderToRuntimeMock).toHaveBeenCalledWith('custom-local', gatewayManager); }); + it('promotes the newest enabled account before removing the deleted default from runtime', async () => { + const deletedAccount = { + id: 'default-account', + vendorId: 'moonshot', + label: 'Default', + authMode: 'api_key', + model: 'kimi-k2.6', + enabled: true, + createdAt: '2026-05-01T00:00:00.000Z', + updatedAt: '2026-05-01T00:00:00.000Z', + }; + const newestDisabledAccount = { + ...deletedAccount, + id: 'disabled-newest', + label: 'Disabled Newest', + enabled: false, + updatedAt: '2026-06-03T00:00:00.000Z', + }; + const olderEnabledAccount = { + ...deletedAccount, + id: 'enabled-older', + label: 'Enabled Older', + updatedAt: '2026-06-01T00:00:00.000Z', + }; + const newestEnabledAccount = { + ...deletedAccount, + id: 'enabled-newest', + label: 'Enabled Newest', + updatedAt: '2026-06-02T00:00:00.000Z', + }; + providerServiceMock.getAccount.mockResolvedValue(deletedAccount); + providerServiceMock.getDefaultAccountId.mockResolvedValue(deletedAccount.id); + providerServiceMock.listAccounts.mockResolvedValue([ + deletedAccount, + newestDisabledAccount, + olderEnabledAccount, + newestEnabledAccount, + ]); + const gatewayManager = { debouncedReload: vi.fn(), debouncedRestart: vi.fn() }; + const { createProvidersApi } = await import('@electron/services/providers-api'); + + await expect(createProvidersApi({ + gatewayManager: gatewayManager as never, + mainWindow: {} as never, + }).deleteAccount({ accountId: deletedAccount.id })).resolves.toEqual({ success: true }); + + expect(providerServiceMock.setDefaultAccount).toHaveBeenCalledWith(newestEnabledAccount.id); + expect(syncDefaultProviderToRuntimeMock).toHaveBeenCalledWith(newestEnabledAccount.id); + expect(syncDeletedProviderToRuntimeMock).toHaveBeenCalledWith( + expect.objectContaining({ id: deletedAccount.id, type: deletedAccount.vendorId }), + deletedAccount.id, + gatewayManager, + undefined, + ); + expect(syncDefaultProviderToRuntimeMock.mock.invocationCallOrder[0]) + .toBeLessThan(syncDeletedProviderToRuntimeMock.mock.invocationCallOrder[0]); + }); + + it('does not change the default provider when deleting a non-default account', async () => { + const account = { + id: 'secondary-account', + vendorId: 'moonshot', + label: 'Secondary', + authMode: 'api_key', + model: 'kimi-k2.6', + enabled: true, + createdAt: '2026-05-01T00:00:00.000Z', + updatedAt: '2026-05-01T00:00:00.000Z', + }; + providerServiceMock.getAccount.mockResolvedValue(account); + providerServiceMock.getDefaultAccountId.mockResolvedValue('default-account'); + const gatewayManager = { debouncedReload: vi.fn(), debouncedRestart: vi.fn() }; + const { createProvidersApi } = await import('@electron/services/providers-api'); + + await expect(createProvidersApi({ + gatewayManager: gatewayManager as never, + mainWindow: {} as never, + }).deleteAccount({ accountId: account.id })).resolves.toEqual({ success: true }); + + expect(providerServiceMock.listAccounts).not.toHaveBeenCalled(); + expect(providerServiceMock.setDefaultAccount).not.toHaveBeenCalled(); + expect(syncDefaultProviderToRuntimeMock).not.toHaveBeenCalled(); + }); + + it('leaves the default unset when deleting the final provider account', async () => { + const account = { + id: 'only-account', + vendorId: 'moonshot', + label: 'Only Account', + authMode: 'api_key', + model: 'kimi-k2.6', + enabled: true, + createdAt: '2026-05-01T00:00:00.000Z', + updatedAt: '2026-05-01T00:00:00.000Z', + }; + providerServiceMock.getAccount.mockResolvedValue(account); + providerServiceMock.getDefaultAccountId.mockResolvedValue(account.id); + providerServiceMock.listAccounts.mockResolvedValue([account]); + const gatewayManager = { debouncedReload: vi.fn(), debouncedRestart: vi.fn() }; + const { createProvidersApi } = await import('@electron/services/providers-api'); + + await expect(createProvidersApi({ + gatewayManager: gatewayManager as never, + mainWindow: {} as never, + }).deleteAccount({ accountId: account.id })).resolves.toEqual({ success: true }); + + expect(providerServiceMock.setDefaultAccount).not.toHaveBeenCalled(); + expect(syncDefaultProviderToRuntimeMock).not.toHaveBeenCalled(); + }); + it('builds channel accounts from config without gateway rpc in config mode', async () => { const openClawConfig = { channels: {