diff --git a/packages/cli/src/ui/hooks/useProviderUpdates.test.ts b/packages/cli/src/ui/hooks/useProviderUpdates.test.ts index 61ca2dd975..9616a3ada6 100644 --- a/packages/cli/src/ui/hooks/useProviderUpdates.test.ts +++ b/packages/cli/src/ui/hooks/useProviderUpdates.test.ts @@ -222,7 +222,7 @@ describe('useProviderUpdates', () => { expect(entry?.diff.added).toContain(addedModelId); }); - it('preserves user-added custom models when executing an update', async () => { + it('persists the template version and preserves custom models', async () => { const customModel = { id: 'my-custom-model', baseUrl: CODING_PLAN_CHINA_BASE_URL, @@ -264,6 +264,11 @@ describe('useProviderUpdates', () => { expect.objectContaining({ id: 'my-custom-model' }), ]), ); + expect(mockSettings.setValue).toHaveBeenCalledWith( + expect.anything(), + `${PROVIDER_METADATA_NS}.${METADATA_KEY}.version`, + chinaVersion, + ); }); it('executes update when user confirms with "update"', async () => { @@ -303,11 +308,6 @@ describe('useProviderUpdates', () => { expect(mockSettings.setValue).toHaveBeenCalled(); }); - expect(mockSettings.setValue).toHaveBeenCalledWith( - expect.anything(), - `${PROVIDER_METADATA_NS}.${METADATA_KEY}.version`, - chinaVersion, - ); expect(mockSettings.setValue).toHaveBeenCalledWith( expect.anything(), `${PROVIDER_METADATA_NS}.${METADATA_KEY}.baseUrl`, diff --git a/packages/cli/src/ui/hooks/useProviderUpdates.ts b/packages/cli/src/ui/hooks/useProviderUpdates.ts index 1b62f7bafe..c77c52aa58 100644 --- a/packages/cli/src/ui/hooks/useProviderUpdates.ts +++ b/packages/cli/src/ui/hooks/useProviderUpdates.ts @@ -14,7 +14,8 @@ import { ALL_PROVIDERS, applyProviderInstallPlan, buildInstallPlan, - computeProviderTemplateVersion, + buildProviderTemplate, + computeModelListVersion, getDefaultModelIds, PROVIDER_METADATA_NS, providerMatchesCredentials, @@ -204,7 +205,8 @@ function findAllPendingUpdates( if (!metadata.version) continue; const baseUrl = metadata.baseUrl || resolveBaseUrl(provider); - const currentVersion = computeProviderTemplateVersion(provider, baseUrl); + const currentTemplate = buildProviderTemplate(provider, baseUrl); + const currentVersion = computeModelListVersion(currentTemplate); if (metadata.version === currentVersion) continue; if (metadata.ignoredVersion === currentVersion) continue; @@ -253,9 +255,10 @@ export function useProviderUpdates( const migrated = useRef(false); const executeUpdate = useCallback( - async (providerCfg: ProviderConfig, baseUrl?: string) => { + async (pending: PendingUpdate) => { try { - const resolved = resolveBaseUrl(providerCfg, baseUrl); + const providerCfg = pending.provider; + const resolved = resolveBaseUrl(providerCfg, pending.baseUrl); // An update only refreshes built-in models — user-added custom IDs // must be carried through so they are not deleted by the // prepend-and-remove-owned merge. @@ -268,6 +271,9 @@ export function useProviderUpdates( apiKey: '', modelIds: [...defaultIds, ...customIds], }); + installPlan.providerState![ + `${PROVIDER_METADATA_NS}.${pending.metadataKey}` + ]!['version'] = pending.currentVersion; delete installPlan.env; const previousModel = config.getModel(); const activeConfig = config.getContentGeneratorConfig(); @@ -384,7 +390,7 @@ export function useProviderUpdates( setUpdateRequest(undefined); if (choice === 'update') { for (const p of pendingList) { - await executeUpdate(p.provider, p.baseUrl); + await executeUpdate(p); } } else if (choice === 'skip') { const persistScope = getPersistScopeForModelSelection(settings); diff --git a/packages/core/src/providers/__tests__/provider-config.test.ts b/packages/core/src/providers/__tests__/provider-config.test.ts index d5dd3dd557..e068486ec7 100644 --- a/packages/core/src/providers/__tests__/provider-config.test.ts +++ b/packages/core/src/providers/__tests__/provider-config.test.ts @@ -13,20 +13,11 @@ import { findExistingProviderModels, findProviderByCredentials, getDefaultModelIds, - PROVIDER_METADATA_NS, resolveBaseUrl, shouldShowStep, providerMatchesCredentials, type ProviderConfig, } from '@qwen-code/qwen-code-core'; -// Imported from source: the invariant below must be checked against the -// current implementation, not a previously built `dist`. -import { - buildInstallPlan as buildInstallPlanSrc, - computeProviderTemplateVersion, - resolveMetadataKey, -} from '../provider-config.js'; -import { ALL_PROVIDERS as ALL_PROVIDERS_SRC } from '../all-providers.js'; import { TOKEN_PLAN_CHINA_BASE_URL, TOKEN_PLAN_ENV_KEY, @@ -71,8 +62,14 @@ describe('buildInstallPlan', () => { }); it('builds a plan with editable models and unknown IDs', () => { - const config = makeConfig({ modelsEditable: true }); - const plan = buildInstallPlan(config, { + const config = makeConfig({ + modelsEditable: true, + models: [ + { id: 'model-a', contextWindowSize: 8192, enableThinking: true }, + { id: 'model-b' }, + ], + }); + const plan = buildInstallPlanSrc(config, { baseUrl: 'https://api.test.com/v1', apiKey: 'sk-test', modelIds: ['model-a', 'unknown-model'], @@ -86,6 +83,9 @@ describe('buildInstallPlan', () => { name: '[Test] unknown-model', }); expect(models?.[1]?.generationConfig).toBeUndefined(); + expect(plan.providerState?.['providerMetadata.test']?.['version']).toBe( + computeModelListVersion(models ?? []), + ); }); it('applies advancedConfig to editable unknown model IDs only', () => { @@ -683,6 +683,7 @@ import { getAllProviderBaseUrls as getAllProviderBaseUrlsSrc, } from '../all-providers.js'; import { + buildInstallPlan as buildInstallPlanSrc, resolveBaseUrl as resolveBaseUrlSrc, providerMatchesCredentials as providerMatchesCredentialsSrc, } from '../provider-config.js'; @@ -925,84 +926,3 @@ describe('resolveMetadataKey dotted-id guard', () => { expect(() => resolveMetadataKeySrc(config)).toThrow(/must not contain/); }); }); - -describe('stored provider version matches the launch-time check', () => { - // The prompt in useProviderUpdates clears only when the version recorded by - // an install/update equals the one recomputed at launch from the built-in - // template. Assert that invariant for every built-in provider, with a custom - // model installed — the case that used to make the two diverge forever. - const providersWithMetadata = ALL_PROVIDERS_SRC.filter((provider) => - resolveMetadataKey(provider), - ); - - it('covers every built-in provider that records metadata', () => { - expect(providersWithMetadata.length).toBeGreaterThan(0); - }); - - for (const provider of providersWithMetadata) { - const metadataKey = resolveMetadataKey(provider)!; - - it(`holds for ${metadataKey} when a custom model is installed`, () => { - const baseUrl = resolveBaseUrl(provider); - const defaultIds = getDefaultModelIds(provider); - const plan = buildInstallPlanSrc(provider, { - baseUrl, - apiKey: 'sk-test', - // A user-added id plus a built-in that upstream renamed away: both - // reach the plan as "custom" ids and must not affect the version. - modelIds: [...defaultIds, 'user-added-model', 'renamed-away-builtin'], - }); - - const stored = - plan.providerState?.[`${PROVIDER_METADATA_NS}.${metadataKey}`]; - const launchVersion = computeProviderTemplateVersion(provider, baseUrl); - - expect(stored?.['version']).toBe(launchVersion); - // The plan still carries the custom ids — only the version ignores them. - expect(plan.modelProviders?.[0]?.models.map((m) => m.id)).toEqual( - expect.arrayContaining(['user-added-model', 'renamed-away-builtin']), - ); - }); - } -}); - -describe('computeProviderTemplateVersion', () => { - it('equals hashing the built-in template by hand', () => { - const config = makeConfig(); - const baseUrl = resolveBaseUrl(config); - expect(computeProviderTemplateVersion(config, baseUrl)).toBe( - computeModelListVersion(buildProviderTemplate(config, baseUrl)), - ); - }); - - it('ignores installed custom models', () => { - const config = makeConfig(); - const baseUrl = resolveBaseUrl(config); - const withCustom = buildInstallPlanSrc(config, { - baseUrl, - apiKey: 'sk-test', - modelIds: [...getDefaultModelIds(config), 'extra-model'], - }); - const withoutCustom = buildInstallPlanSrc(config, { - baseUrl, - apiKey: 'sk-test', - modelIds: getDefaultModelIds(config), - }); - const key = `${PROVIDER_METADATA_NS}.${resolveMetadataKey(config)}`; - expect(withCustom.providerState?.[key]?.['version']).toBe( - withoutCustom.providerState?.[key]?.['version'], - ); - }); - - it('changes when the built-in template changes', () => { - const baseUrl = 'https://api.test.com/v1'; - const before = computeProviderTemplateVersion(makeConfig(), baseUrl); - const after = computeProviderTemplateVersion( - makeConfig({ - models: [{ id: 'model-a-renamed', contextWindowSize: 8192 }], - }), - baseUrl, - ); - expect(after).not.toBe(before); - }); -}); diff --git a/packages/core/src/providers/index.ts b/packages/core/src/providers/index.ts index 450f5a1a30..fbe784bae5 100644 --- a/packages/core/src/providers/index.ts +++ b/packages/core/src/providers/index.ts @@ -23,7 +23,6 @@ export { buildInstallPlan, buildProviderTemplate, computeModelListVersion, - computeProviderTemplateVersion, findExistingProviderModels, getDefaultBaseUrlForProtocol, getDefaultModelIds, diff --git a/packages/core/src/providers/provider-config.ts b/packages/core/src/providers/provider-config.ts index 7a5d7a36c8..88577e24b7 100644 --- a/packages/core/src/providers/provider-config.ts +++ b/packages/core/src/providers/provider-config.ts @@ -229,12 +229,13 @@ export const PROVIDER_METADATA_NS = 'providerMetadata'; function resolveProviderState( config: ProviderConfig, baseUrl: string, + models: ProviderModelConfig[], ): ProviderInstallState | undefined { const key = resolveMetadataKey(config); if (key) { return { [`${PROVIDER_METADATA_NS}.${key}`]: { - version: computeProviderTemplateVersion(config, baseUrl), + version: computeModelListVersion(models), baseUrl, }, }; @@ -286,7 +287,7 @@ export function buildInstallPlan( ...(ownsModel ? { ownsModel } : {}), }, ], - providerState: resolveProviderState(config, inputs.baseUrl), + providerState: resolveProviderState(config, inputs.baseUrl, models), }; } @@ -298,22 +299,6 @@ export function computeModelListVersion(models: ProviderModelConfig[]): string { return createHash('sha256').update(JSON.stringify(models)).digest('hex'); } -/** - * Version of a provider's built-in template (default models only). - * - * The version recorded at install time and the one recomputed at launch to - * detect a pending update must hash the same input, or the update prompt can - * never clear. Both sides call this instead of composing - * `buildProviderTemplate` + `computeModelListVersion` by hand, so the two - * cannot drift apart again. - */ -export function computeProviderTemplateVersion( - config: ProviderConfig, - baseUrl: string, -): string { - return computeModelListVersion(buildProviderTemplate(config, baseUrl)); -} - /** * Default base URLs per protocol, used as placeholder/fallback when the user * doesn't supply one for a custom provider. Kept in core so the CLI flow