Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions packages/cli/src/ui/hooks/useProviderUpdates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
TOKEN_PLAN_ENV_KEY,
tokenPlanProvider,
buildProviderTemplate,
computeModelListVersion,
computeProviderTemplateVersion,
PROVIDER_METADATA_NS,
} from '@qwen-code/qwen-code-core';
import { useProviderUpdates } from './useProviderUpdates.js';
Expand All @@ -35,13 +35,19 @@ const chinaTemplate = buildProviderTemplate(
codingPlanProvider,
CODING_PLAN_CHINA_BASE_URL,
);
const chinaVersion = computeModelListVersion(chinaTemplate);
const chinaVersion = computeProviderTemplateVersion(
codingPlanProvider,
CODING_PLAN_CHINA_BASE_URL,
);

const tokenTemplate = buildProviderTemplate(
tokenPlanProvider,
TOKEN_PLAN_BASE_URL,
);
const tokenVersion = computeModelListVersion(tokenTemplate);
const tokenVersion = computeProviderTemplateVersion(
tokenPlanProvider,
TOKEN_PLAN_BASE_URL,
);

const METADATA_KEY = 'coding-plan';
const TOKEN_METADATA_KEY = 'token-plan';
Expand Down Expand Up @@ -222,7 +228,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,
Expand Down Expand Up @@ -264,6 +270,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 () => {
Expand Down Expand Up @@ -303,11 +314,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`,
Expand Down
13 changes: 6 additions & 7 deletions packages/cli/src/ui/hooks/useProviderUpdates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import {
ALL_PROVIDERS,
applyProviderInstallPlan,
buildInstallPlan,
buildProviderTemplate,
computeModelListVersion,
computeProviderTemplateVersion,
getDefaultModelIds,
PROVIDER_METADATA_NS,
providerMatchesCredentials,
Expand Down Expand Up @@ -205,8 +204,7 @@ function findAllPendingUpdates(
if (!metadata.version) continue;

const baseUrl = metadata.baseUrl || resolveBaseUrl(provider);
const currentTemplate = buildProviderTemplate(provider, baseUrl);
const currentVersion = computeModelListVersion(currentTemplate);
const currentVersion = computeProviderTemplateVersion(provider, baseUrl);

if (metadata.version === currentVersion) continue;
if (metadata.ignoredVersion === currentVersion) continue;
Expand Down Expand Up @@ -255,9 +253,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.
Expand Down Expand Up @@ -386,7 +385,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);
Expand Down
107 changes: 103 additions & 4 deletions packages/core/src/providers/__tests__/provider-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,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'],
Expand All @@ -77,6 +83,12 @@ describe('buildInstallPlan', () => {
name: '[Test] unknown-model',
});
expect(models?.[1]?.generationConfig).toBeUndefined();
// The stored version must match the launch-time check, which hashes
// the built-in template only — not the final model list that includes
// custom/unknown IDs.
expect(plan.providerState?.['providerMetadata.test']?.['version']).toBe(
computeProviderTemplateVersionSrc(config, 'https://api.test.com/v1'),
);
});

it('applies advancedConfig to editable unknown model IDs only', () => {
Expand Down Expand Up @@ -670,11 +682,16 @@ describe('getAllProviderBaseUrls', () => {
// branch that hasn't been built yet. Re-import via the relative source path so
// these new edge-case tests exercise the in-tree implementation.
import {
ALL_PROVIDERS as ALL_PROVIDERS_SRC,
findProviderByCredentials as findProviderByCredentialsSrc,
getAllProviderBaseUrls as getAllProviderBaseUrlsSrc,
} from '../all-providers.js';
import {
buildInstallPlan as buildInstallPlanSrc,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] Still unresolved from the previous review: source import placed after first use

buildInstallPlanSrc is first used at line 72 but only imported here at line 686. ESM hoisting makes this legal, so tests pass, but a reader scanning the top import block cannot tell where buildInstallPlanSrc comes from, and any future reordering or a switch to a non-hoisting transform breaks it.

Please move this source-relative import block (and the ../all-providers.js block above it) to the top of the module alongside the @qwen-code/qwen-code-core imports.


This review was generated by QoderWork AI

computeProviderTemplateVersion as computeProviderTemplateVersionSrc,
PROVIDER_METADATA_NS,
resolveBaseUrl as resolveBaseUrlSrc,
resolveMetadataKey as resolveMetadataKeySrc,
providerMatchesCredentials as providerMatchesCredentialsSrc,
} from '../provider-config.js';

Expand Down Expand Up @@ -795,8 +812,6 @@ describe('providerMatchesCredentials with function envKey (custom provider)', ()
});
});

import { resolveMetadataKey as resolveMetadataKeySrc } from '../provider-config.js';

describe('customHeaders in ProviderConfig', () => {
it('merges customHeaders into generationConfig for fixed models', () => {
const config = makeConfig({
Expand Down Expand Up @@ -916,3 +931,87 @@ describe('resolveMetadataKey dotted-id guard', () => {
expect(() => resolveMetadataKeySrc(config)).toThrow(/must not contain/);
});
});

describe('stored provider version matches the launch-time check', () => {
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
// 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) =>
resolveMetadataKeySrc(provider),
);

it('covers every built-in provider that records metadata', () => {
expect(providersWithMetadata.length).toBeGreaterThan(0);
});

for (const provider of providersWithMetadata) {
const metadataKey = resolveMetadataKeySrc(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 = computeProviderTemplateVersionSrc(
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(computeProviderTemplateVersionSrc(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}.${resolveMetadataKeySrc(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 = computeProviderTemplateVersionSrc(makeConfig(), baseUrl);
const after = computeProviderTemplateVersionSrc(
makeConfig({
models: [{ id: 'model-a-renamed', contextWindowSize: 8192 }],
}),
baseUrl,
);
expect(after).not.toBe(before);
});
});
1 change: 1 addition & 0 deletions packages/core/src/providers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export {
buildInstallPlan,
buildProviderTemplate,
computeModelListVersion,
computeProviderTemplateVersion,
findExistingProviderModels,
getDefaultBaseUrlForProtocol,
getDefaultModelIds,
Expand Down
21 changes: 18 additions & 3 deletions packages/core/src/providers/provider-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,12 @@ export const PROVIDER_METADATA_NS = 'providerMetadata';
function resolveProviderState(
config: ProviderConfig,
baseUrl: string,
models: ProviderModelConfig[],
): ProviderInstallState | undefined {
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
const key = resolveMetadataKey(config);
if (key) {
return {
[`${PROVIDER_METADATA_NS}.${key}`]: {
version: computeModelListVersion(models),
version: computeProviderTemplateVersion(config, baseUrl),
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
baseUrl,
},
};
Expand Down Expand Up @@ -287,7 +286,7 @@ export function buildInstallPlan(
...(ownsModel ? { ownsModel } : {}),
},
],
providerState: resolveProviderState(config, inputs.baseUrl, models),
providerState: resolveProviderState(config, inputs.baseUrl),
};
}

Expand All @@ -299,6 +298,22 @@ 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,41 +5,59 @@
*/

import { describe, expect, it } from 'vitest';
import {
buildProviderTemplate,
codingPlanProvider,
computeProviderTemplateVersion,
getDefaultModelIds,
tokenPlanProvider,
} from '@qwen-code/qwen-code-core';

import {
CodingPlanRegion,
getSubscriptionPlanConfig,
} from './subscriptionPlanDefinitions.js';

describe('subscription plan definitions', () => {
it('keeps Token Plan on its dedicated model list', () => {
it('keeps Token Plan on its dedicated model list, sourced from the core preset', () => {
const tokenPlan = getSubscriptionPlanConfig('token');
const codingPlan = getSubscriptionPlanConfig('coding');

expect(tokenPlan.template.map((model) => model.id)).toEqual([
'qwen3.7-plus',
'qwen3.6-plus',
'qwen3.7-max',
'qwen3.8-max-preview',
'qwen3.6-flash',
'deepseek-v4-pro',
'deepseek-v4-flash',
'deepseek-v3.2',
'kimi-k2.7-code',
'kimi-k2.6',
'kimi-k2.5',
'glm-5.2',
'glm-5.1',
'glm-5',
'MiniMax-M2.5',
]);
// Pinning ids here would let this copy drift from the preset the CLI reads
// — which is how a renamed built-in stayed in the IDE's list. Assert the
// shared source instead.
expect(tokenPlan.template.map((model) => model.id)).toEqual(
getDefaultModelIds(tokenPlanProvider),
);
expect(codingPlan.template.map((model) => model.id)).toEqual(
getDefaultModelIds(codingPlanProvider),
);
expect(tokenPlan.template.map((model) => model.id)).not.toEqual(
codingPlan.template.map((model) => model.id),
);
expect(codingPlan.template.map((model) => model.id)).not.toContain(
'qwen3.7-max',
);
expect(
tokenPlan.template.find((model) => model.id === 'deepseek-v4-pro')
?.generationConfig,
).toEqual({ contextWindowSize: 1000000 });
});

it('records a version the CLI can reproduce from the same preset', () => {
// The CLI recomputes this on every launch to detect a pending provider
// update; a version it cannot reproduce shows the update prompt right
// after signing in from the IDE.
for (const [planId, provider] of [
['token', tokenPlanProvider],
['coding', codingPlanProvider],
] as const) {
for (const region of [CodingPlanRegion.CHINA, CodingPlanRegion.GLOBAL]) {
const plan = getSubscriptionPlanConfig(planId, region);
expect(plan.version).toBe(
computeProviderTemplateVersion(provider, plan.baseUrl),
);
expect(plan.template).toEqual(
buildProviderTemplate(provider, plan.baseUrl),
);
}
}
});

it('defaults Token Plan to China and supports the Singapore region', () => {
Expand Down
Loading
Loading