diff --git a/electron/services/providers/provider-runtime-sync.ts b/electron/services/providers/provider-runtime-sync.ts index 383e2d1a..d042dc6b 100644 --- a/electron/services/providers/provider-runtime-sync.ts +++ b/electron/services/providers/provider-runtime-sync.ts @@ -317,7 +317,8 @@ async function syncRuntimeProviderConfig( config: ProviderConfig, context: RuntimeProviderSyncContext, ): Promise { - await syncProviderConfigToOpenClaw(context.runtimeProviderKey, config.model, { + const modelId = normalizeRuntimeModelId(context.runtimeProviderKey, config.model); + await syncProviderConfigToOpenClaw(context.runtimeProviderKey, modelId, { baseUrl: normalizeProviderBaseUrl(config, config.baseUrl || context.meta?.baseUrl, context.api), api: context.api, apiKeyEnv: context.meta?.apiKeyEnv, @@ -339,7 +340,7 @@ async function syncCustomProviderAgentModel( return; } - const modelId = config.model; + const modelId = normalizeRuntimeModelId(runtimeProviderKey, config.model); await updateAgentModelProvider(runtimeProviderKey, { baseUrl: normalizeProviderBaseUrl(config, config.baseUrl, config.apiProtocol || 'openai-completions'), api: config.apiProtocol || 'openai-completions', @@ -414,6 +415,16 @@ function parseModelRef(modelRef: string): { providerKey: string; modelId: string }; } +function normalizeRuntimeModelId( + runtimeProviderKey: string, + modelId: string | undefined, +): string | undefined { + const value = modelId?.trim(); + if (!value) return undefined; + const prefix = `${runtimeProviderKey}/`; + return value.startsWith(prefix) ? value.slice(prefix.length) : value; +} + async function buildRuntimeProviderConfigMap(): Promise> { const configs = await getAllProviders(); const runtimeMap = new Map(); @@ -542,7 +553,8 @@ export async function syncUpdatedProviderToRuntime( const defaultProviderId = await getDefaultProvider(); const isDefaultProvider = defaultProviderId === config.id; if (isDefaultProvider) { - const modelOverride = config.model ? `${ock}/${config.model}` : undefined; + const selectedModelId = normalizeRuntimeModelId(ock, config.model); + const modelOverride = selectedModelId ? `${ock}/${selectedModelId}` : undefined; if (!isUnregisteredProviderType(config.type)) { if (shouldUseExplicitDefaultOverride(config, ock)) { await setOpenClawDefaultModelWithOverride(ock, modelOverride, { diff --git a/harness/specs/rules/provider-model-selection-authority.md b/harness/specs/rules/provider-model-selection-authority.md new file mode 100644 index 00000000..d9349d9e --- /dev/null +++ b/harness/specs/rules/provider-model-selection-authority.md @@ -0,0 +1,19 @@ +--- +id: provider-model-selection-authority +title: Provider Model Selection Authority +type: ai-coding-rule +appliesTo: + - gateway-backend-communication +--- + +An OAuth provider account's explicit `model` is the authoritative model exposed +for that account in interactive model selectors. Historical runtime model rows +may remain available for capability preservation, but must not reappear as +alternate OAuth selections through synchronized `metadata.customModels`. + +Before writing a selected model ID to OpenClaw, strip one leading provider +prefix when it exactly matches the resolved runtime provider key. Preserve all +other slashes because they may be part of a valid model ID. + +Custom and local multi-model accounts may continue to project all configured +`metadata.customModels`. Do not collapse their lists to the selected model. diff --git a/harness/specs/scenarios/gateway-backend-communication.md b/harness/specs/scenarios/gateway-backend-communication.md index 6ac01de6..c2e58798 100644 --- a/harness/specs/scenarios/gateway-backend-communication.md +++ b/harness/specs/scenarios/gateway-backend-communication.md @@ -34,6 +34,7 @@ requiredRules: - active-config-guards - provider-default-invariant - provider-model-metadata-preservation + - provider-model-selection-authority - comms-regression - docs-sync forbiddenPatterns: diff --git a/harness/specs/tasks/fix-oauth-model-picker-stale-id.md b/harness/specs/tasks/fix-oauth-model-picker-stale-id.md new file mode 100644 index 00000000..df395882 --- /dev/null +++ b/harness/specs/tasks/fix-oauth-model-picker-stale-id.md @@ -0,0 +1,62 @@ +--- +id: fix-oauth-model-picker-stale-id +title: Hide stale OAuth model IDs after provider edits +scenario: gateway-backend-communication +taskType: runtime-bridge +intent: Keep the chat model picker aligned with the currently selected OAuth model while preserving historical OpenClaw model metadata and normalizing provider-prefixed model input before runtime sync. +touchedAreas: + - harness/specs/tasks/fix-oauth-model-picker-stale-id.md + - harness/specs/rules/provider-model-selection-authority.md + - harness/specs/scenarios/gateway-backend-communication.md + - src/lib/model-options.ts + - electron/services/providers/provider-runtime-sync.ts + - tests/unit/model-options.test.ts + - tests/unit/provider-runtime-sync.test.ts + - tests/e2e/chat-model-picker.spec.ts +expectedUserBehavior: + - Editing an OpenAI OAuth account from gpt-5.5 to gpt-5.6 removes gpt-5.5 for that account from the chat model picker. + - Entering openai/gpt-5.6 is normalized to the runtime model ID gpt-5.6 instead of producing openai/openai/gpt-5.6. + - Custom multi-model providers continue to expose every configured custom model. +requiredProfiles: + - fast + - comms + - e2e +requiredRules: + - active-config-guards + - backend-communication-boundary + - provider-model-metadata-preservation + - provider-model-selection-authority + - renderer-main-boundary +requiredTests: + - tests/unit/model-options.test.ts + - tests/unit/provider-runtime-sync.test.ts + - tests/e2e/chat-model-picker.spec.ts +acceptance: + - OAuth browser accounts with an explicit account.model contribute only that normalized model to the chat picker. + - Provider-prefixed selected model IDs are stripped exactly once before OpenClaw provider and default-model synchronization. + - Existing models.providers rows remain merged by exact ID so model capability metadata is not deleted. + - Custom provider multi-model picker behavior remains unchanged. + - Focused tests, harness validation, communication replay, and communication compare pass. +docs: + required: false +--- + +## Background + +OpenClaw provider synchronization intentionally retains existing model rows to +preserve capability metadata. Provider account snapshots copy those rows into +`metadata.customModels`, but the chat picker previously treated that historical +list as authoritative even after an OAuth account's selected model changed. + +## Scope + +- Make the explicit OAuth account model authoritative for chat picker options. +- Normalize a matching runtime-provider prefix before runtime configuration writes. +- Preserve custom-provider multi-model options and OpenClaw model-row metadata. +- Cover the visible picker behavior with Electron E2E. + +## Out Of Scope + +- Deleting historical model capability rows from `openclaw.json`. +- Removing independently configured custom provider accounts. +- Changing OAuth login defaults. diff --git a/src/lib/model-options.ts b/src/lib/model-options.ts index 46603310..ebe558fb 100644 --- a/src/lib/model-options.ts +++ b/src/lib/model-options.ts @@ -51,6 +51,15 @@ export function splitModelRef(modelRef: string | null | undefined): { providerKe }; } +export function normalizeModelIdForRuntimeProvider( + modelId: string | null | undefined, + runtimeProviderKey: string, +): string { + const value = (modelId || '').trim(); + const prefix = `${runtimeProviderKey}/`; + return value.startsWith(prefix) ? value.slice(prefix.length) : value; +} + export function formatModelRefLabel(modelRef: string | null | undefined): string { const parsed = splitModelRef(modelRef); return parsed?.modelId || (modelRef || '').trim() || 'Model'; @@ -161,16 +170,15 @@ export function buildConfiguredModelOptions( for (const account of entries) { const runtimeProviderKey = resolveRuntimeProviderKey(account); const modelIds = (() => { + const selectedModelId = normalizeModelIdForRuntimeProvider(account.model, runtimeProviderKey); + if (account.authMode === 'oauth_browser' && selectedModelId) { + return [selectedModelId]; + } const configured = (account.metadata?.customModels ?? []) - .map((modelId) => modelId.trim()) + .map((modelId) => normalizeModelIdForRuntimeProvider(modelId, runtimeProviderKey)) .filter(Boolean); if (configured.length > 0) return configured; - if (!account.model?.trim()) return []; - return [ - account.model.startsWith(`${runtimeProviderKey}/`) - ? account.model.slice(runtimeProviderKey.length + 1) - : account.model.trim(), - ].filter(Boolean); + return selectedModelId ? [selectedModelId] : []; })(); for (const modelId of modelIds) { const modelRef = `${runtimeProviderKey}/${modelId}`; diff --git a/tests/e2e/chat-model-picker.spec.ts b/tests/e2e/chat-model-picker.spec.ts index 8eb20c72..d3702879 100644 --- a/tests/e2e/chat-model-picker.spec.ts +++ b/tests/e2e/chat-model-picker.spec.ts @@ -130,6 +130,18 @@ test.describe('ClawX chat model picker', () => { createdAt: now, updatedAt: now, }, + { + id: 'openai-oauth', + vendorId: 'openai', + label: 'OpenAI', + authMode: 'oauth_browser', + model: 'openai/gpt-5.6', + metadata: { customModels: ['gpt-5.5', 'openai/gpt-5.6'] }, + enabled: true, + isDefault: false, + createdAt: now, + updatedAt: now, + }, ]); } if (request?.module === 'providers' && request.action === 'list') { @@ -145,7 +157,9 @@ test.describe('ClawX chat model picker', () => { ]); } if (request?.module === 'providers' && request.action === 'vendors') { - return makeResponse(request.id, []); + return makeResponse(request.id, [ + { id: 'openai', name: 'OpenAI', supportedAuthModes: ['api_key', 'oauth_browser'] }, + ]); } if (request?.module === 'providers' && request.action === 'getDefaultAccount') { return makeResponse(request.id, { accountId: 'alpha1234' }); @@ -169,6 +183,9 @@ test.describe('ClawX chat model picker', () => { await page.getByTestId('chat-model-picker-button').click(); await expect(page.getByTestId('chat-model-picker-menu')).toBeVisible(); await expect(page.getByTestId('chat-model-picker-menu')).toContainText('provider/model-beta (Beta)'); + await expect(page.getByTestId('chat-model-picker-menu')).toContainText('gpt-5.6 (OpenAI)'); + await expect(page.getByTestId('chat-model-picker-menu')).not.toContainText('gpt-5.5 (OpenAI)'); + await expect(page.getByTestId('chat-model-picker-menu')).not.toContainText('openai/gpt-5.6 (OpenAI)'); await page.getByTestId('chat-model-picker-menu').getByRole('button', { name: 'provider/model-beta (Beta)' }).click(); await expect(page.getByTestId('chat-model-picker-button')).toContainText('provider/model-beta (Beta)'); diff --git a/tests/unit/model-options.test.ts b/tests/unit/model-options.test.ts index da87e85f..4e179776 100644 --- a/tests/unit/model-options.test.ts +++ b/tests/unit/model-options.test.ts @@ -5,6 +5,7 @@ import { formatModelRefLabel, formatProviderDisplayName, isConfiguredModelRefAvailable, + normalizeModelIdForRuntimeProvider, resolveConfiguredModelRef, resolveRuntimeProviderKey, } from '../../src/lib/model-options'; @@ -59,6 +60,9 @@ describe('model option helpers', () => { it('formats model refs using only the text after the provider prefix', () => { expect(formatModelRefLabel('openrouter/openai/gpt-5.5')).toBe('openai/gpt-5.5'); expect(formatModelRefLabel('custom-alpha1234/model-alpha')).toBe('model-alpha'); + expect(normalizeModelIdForRuntimeProvider('openai/gpt-5.6', 'openai')).toBe('gpt-5.6'); + expect(normalizeModelIdForRuntimeProvider('openrouter/openai/gpt-5.6', 'openrouter')) + .toBe('openai/gpt-5.6'); }); it('formats provider display names using custom labels or vendor names', () => { @@ -145,6 +149,33 @@ describe('model option helpers', () => { ]); }); + it('uses only the selected OAuth model when synced metadata contains stale model IDs', () => { + const openAiAccount = account({ + id: 'openai-oauth', + vendorId: 'openai', + label: 'OpenAI', + authMode: 'oauth_browser', + model: 'openai/gpt-5.6', + metadata: { customModels: ['gpt-5.5', 'openai/gpt-5.6'] }, + }); + + const options = buildConfiguredModelOptions( + [openAiAccount], + [], + vendors, + openAiAccount.id, + ); + + expect(options).toEqual([ + { + modelRef: 'openai/gpt-5.6', + label: 'gpt-5.6 (OpenAI)', + runtimeProviderKey: 'openai', + accountId: 'openai-oauth', + }, + ]); + }); + it('preserves custom runtime keys that are already normalized', () => { const runtimeKey = resolveRuntimeProviderKey(account({ id: 'custom-enterpri', diff --git a/tests/unit/provider-runtime-sync.test.ts b/tests/unit/provider-runtime-sync.test.ts index e198a450..93158cb4 100644 --- a/tests/unit/provider-runtime-sync.test.ts +++ b/tests/unit/provider-runtime-sync.test.ts @@ -266,6 +266,37 @@ describe('provider-runtime-sync refresh strategy', () => { ); }); + it('normalizes a provider-prefixed model before updating OpenAI runtime config', async () => { + const openaiProvider = createProvider({ + id: 'openai-personal', + type: 'openai', + model: 'openai/gpt-5.6', + }); + mocks.getProviderAccount.mockResolvedValue({ authMode: 'oauth_browser' }); + mocks.getDefaultProvider.mockResolvedValue(openaiProvider.id); + mocks.getProviderConfig.mockReturnValue({ + api: 'openai-responses', + baseUrl: 'https://api.openai.com/v1', + apiKeyEnv: 'OPENAI_API_KEY', + }); + + await syncUpdatedProviderToRuntime(openaiProvider, undefined); + + expect(mocks.syncProviderConfigToOpenClaw).toHaveBeenCalledWith( + 'openai', + 'gpt-5.6', + expect.objectContaining({ + api: 'openai-responses', + baseUrl: 'https://api.openai.com/v1', + }), + ); + expect(mocks.setOpenClawDefaultModel).toHaveBeenCalledWith( + 'openai', + 'openai/gpt-5.6', + [], + ); + }); + it('syncs a targeted agent model override to runtime provider registry', async () => { mocks.getAllProviders.mockResolvedValue([ createProvider({