diff --git a/packages/cli/src/ui/hooks/useProviderUpdates.test.ts b/packages/cli/src/ui/hooks/useProviderUpdates.test.ts index 92cb22a7f4d..61ca2dd975a 100644 --- a/packages/cli/src/ui/hooks/useProviderUpdates.test.ts +++ b/packages/cli/src/ui/hooks/useProviderUpdates.test.ts @@ -53,6 +53,7 @@ describe('useProviderUpdates', () => { [PROVIDER_METADATA_NS]: {} as Record, } as Record, setValue: vi.fn(), + setValues: vi.fn(), forScope: vi.fn(() => ({ path: '/tmp/settings.json' })), isTrusted: true, workspace: { settings: {} }, @@ -560,7 +561,7 @@ describe('useProviderUpdates', () => { ); }); - it('dismisses without persisting when user chooses "later"', async () => { + it('persists a cooldown (not a full update) when user chooses "later"', async () => { (mockSettings.merged[PROVIDER_METADATA_NS] as Record)[ METADATA_KEY ] = { @@ -583,15 +584,265 @@ describe('useProviderUpdates', () => { expect(result.current.providerUpdateRequest).toBeDefined(); }); - await result.current.providerUpdateRequest!.onConfirm('later'); + // Pin Date.now so the persisted timestamp can be asserted exactly. + const postponedAt = Date.now(); + const dateNowSpy = vi.spyOn(Date, 'now').mockReturnValue(postponedAt); + try { + await result.current.providerUpdateRequest!.onConfirm('later'); + } finally { + dateNowSpy.mockRestore(); + } await waitFor(() => { expect(result.current.providerUpdateRequest).toBeUndefined(); }); + // "later" persists a postponement cooldown so the prompt does not reappear + // on every launch, but it must not apply the update. The single batched + // write must contain exactly these two keys — pinning the values the + // read-side guard compares against and bounding all persisted writes. + expect(mockSettings.setValues).toHaveBeenCalledTimes(1); + expect(mockSettings.setValues).toHaveBeenCalledWith([ + { + scope: 'User', + key: `${PROVIDER_METADATA_NS}.${METADATA_KEY}.postponedVersion`, + value: chinaVersion, + }, + { + scope: 'User', + key: `${PROVIDER_METADATA_NS}.${METADATA_KEY}.postponedAt`, + value: postponedAt, + }, + ]); expect(mockSettings.setValue).not.toHaveBeenCalled(); expect(mockConfig.reloadModelProvidersConfig).not.toHaveBeenCalled(); }); + it('later persists the cooldown for all providers in one batched write', async () => { + const metadataNs = mockSettings.merged[PROVIDER_METADATA_NS] as Record< + string, + unknown + >; + metadataNs[METADATA_KEY] = { + baseUrl: CODING_PLAN_CHINA_BASE_URL, + version: 'old-version-hash', + }; + metadataNs[TOKEN_METADATA_KEY] = { + baseUrl: TOKEN_PLAN_BASE_URL, + version: 'old-version-hash', + }; + mockSettings.merged['modelProviders'] = { + [AuthType.USE_OPENAI]: [...chinaTemplate, ...tokenTemplate], + }; + + const { result } = renderHook(() => + useProviderUpdates( + mockSettings as never, + mockConfig as never, + mockAddItem, + ), + ); + + await waitFor(() => { + expect(result.current.providerUpdateRequest).toBeDefined(); + }); + + const postponedAt = Date.now(); + const dateNowSpy = vi.spyOn(Date, 'now').mockReturnValue(postponedAt); + try { + await result.current.providerUpdateRequest!.onConfirm('later'); + } finally { + dateNowSpy.mockRestore(); + } + + await waitFor(() => { + expect(result.current.providerUpdateRequest).toBeUndefined(); + }); + expect(mockSettings.setValues).toHaveBeenCalledTimes(1); + expect(mockSettings.setValues).toHaveBeenCalledWith([ + { + scope: 'User', + key: `${PROVIDER_METADATA_NS}.${METADATA_KEY}.postponedVersion`, + value: chinaVersion, + }, + { + scope: 'User', + key: `${PROVIDER_METADATA_NS}.${METADATA_KEY}.postponedAt`, + value: postponedAt, + }, + { + scope: 'User', + key: `${PROVIDER_METADATA_NS}.${TOKEN_METADATA_KEY}.postponedVersion`, + value: tokenVersion, + }, + { + scope: 'User', + key: `${PROVIDER_METADATA_NS}.${TOKEN_METADATA_KEY}.postponedAt`, + value: postponedAt, + }, + ]); + }); + + it('surfaces an error but still dismisses when persisting the cooldown fails', async () => { + (mockSettings.merged[PROVIDER_METADATA_NS] as Record)[ + METADATA_KEY + ] = { + baseUrl: CODING_PLAN_CHINA_BASE_URL, + version: 'old-version-hash', + }; + mockSettings.merged['modelProviders'] = { + [AuthType.USE_OPENAI]: chinaTemplate, + }; + + const { result } = renderHook(() => + useProviderUpdates( + mockSettings as never, + mockConfig as never, + mockAddItem, + ), + ); + + await waitFor(() => { + expect(result.current.providerUpdateRequest).toBeDefined(); + }); + + mockSettings.setValues.mockImplementationOnce(() => { + throw new Error('settings file is read-only'); + }); + + await result.current.providerUpdateRequest!.onConfirm('later'); + + await waitFor(() => { + expect(result.current.providerUpdateRequest).toBeUndefined(); + }); + expect(mockAddItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'error', + text: expect.stringContaining('settings file is read-only'), + }), + expect.any(Number), + ); + }); + + it('does not show prompt while the "later" cooldown is active', () => { + // Pin Date.now on the read side: 23h elapsed is still inside the 24h + // cooldown. Together with the 25h expiry test this pins the duration. + const now = Date.now(); + const dateNowSpy = vi.spyOn(Date, 'now').mockReturnValue(now); + try { + (mockSettings.merged[PROVIDER_METADATA_NS] as Record)[ + METADATA_KEY + ] = { + baseUrl: CODING_PLAN_CHINA_BASE_URL, + version: 'old-version-hash', + postponedVersion: chinaVersion, + postponedAt: now - 23 * 60 * 60 * 1000, + }; + mockSettings.merged['modelProviders'] = { + [AuthType.USE_OPENAI]: chinaTemplate, + }; + + const { result } = renderHook(() => + useProviderUpdates( + mockSettings as never, + mockConfig as never, + mockAddItem, + ), + ); + + expect(result.current.providerUpdateRequest).toBeUndefined(); + } finally { + dateNowSpy.mockRestore(); + } + }); + + it('shows prompt again after the "later" cooldown expires', async () => { + // Pin Date.now on the read side: 25h elapsed is past the 24h cooldown. + const now = Date.now(); + const dateNowSpy = vi.spyOn(Date, 'now').mockReturnValue(now); + (mockSettings.merged[PROVIDER_METADATA_NS] as Record)[ + METADATA_KEY + ] = { + baseUrl: CODING_PLAN_CHINA_BASE_URL, + version: 'old-version-hash', + postponedVersion: chinaVersion, + postponedAt: now - 25 * 60 * 60 * 1000, + }; + mockSettings.merged['modelProviders'] = { + [AuthType.USE_OPENAI]: chinaTemplate, + }; + + const { result } = renderHook(() => + useProviderUpdates( + mockSettings as never, + mockConfig as never, + mockAddItem, + ), + ); + dateNowSpy.mockRestore(); + + await waitFor(() => { + expect(result.current.providerUpdateRequest).toBeDefined(); + }); + }); + + it('shows prompt when the clock stepped backward after postponement', async () => { + // A backward clock jump makes the elapsed time negative; the cooldown must + // be treated as expired rather than suppressing the prompt until the wall + // clock catches up with postponedAt. + const now = Date.now(); + const dateNowSpy = vi.spyOn(Date, 'now').mockReturnValue(now); + (mockSettings.merged[PROVIDER_METADATA_NS] as Record)[ + METADATA_KEY + ] = { + baseUrl: CODING_PLAN_CHINA_BASE_URL, + version: 'old-version-hash', + postponedVersion: chinaVersion, + postponedAt: now + 60 * 60 * 1000, + }; + mockSettings.merged['modelProviders'] = { + [AuthType.USE_OPENAI]: chinaTemplate, + }; + + const { result } = renderHook(() => + useProviderUpdates( + mockSettings as never, + mockConfig as never, + mockAddItem, + ), + ); + dateNowSpy.mockRestore(); + + await waitFor(() => { + expect(result.current.providerUpdateRequest).toBeDefined(); + }); + }); + + it('shows prompt for a newer version despite an active "later" cooldown', async () => { + (mockSettings.merged[PROVIDER_METADATA_NS] as Record)[ + METADATA_KEY + ] = { + baseUrl: CODING_PLAN_CHINA_BASE_URL, + version: 'old-version-hash', + postponedVersion: 'stale-postponed-hash', + postponedAt: Date.now(), + }; + mockSettings.merged['modelProviders'] = { + [AuthType.USE_OPENAI]: chinaTemplate, + }; + + const { result } = renderHook(() => + useProviderUpdates( + mockSettings as never, + mockConfig as never, + mockAddItem, + ), + ); + + await waitFor(() => { + expect(result.current.providerUpdateRequest).toBeDefined(); + }); + }); + it('persists ignoredVersion when user chooses "skip"', async () => { (mockSettings.merged[PROVIDER_METADATA_NS] as Record)[ METADATA_KEY diff --git a/packages/cli/src/ui/hooks/useProviderUpdates.ts b/packages/cli/src/ui/hooks/useProviderUpdates.ts index 9ed6bb16a0b..3b06a2b281f 100644 --- a/packages/cli/src/ui/hooks/useProviderUpdates.ts +++ b/packages/cli/src/ui/hooks/useProviderUpdates.ts @@ -27,6 +27,7 @@ import type { LoadedSettings } from '../../config/settings.js'; import { t } from '../../i18n/index.js'; import { createLoadedSettingsAdapter } from '../../config/loadedSettingsAdapter.js'; import { getPersistScopeForModelSelection } from '../../config/modelProvidersScope.js'; +import { getErrorMessage } from '../../utils/errors.js'; // --------------------------------------------------------------------------- // Public types @@ -59,8 +60,15 @@ interface ProviderMetadata { version?: string; baseUrl?: string; ignoredVersion?: string; + postponedVersion?: string; + postponedAt?: number; } +// "Later" suppresses re-prompting for the same version for this long, so a +// user who defers is not nagged on every launch. A new model-list version +// still re-prompts immediately (postponedVersion no longer matches). +const LATER_COOLDOWN_MS = 24 * 60 * 60 * 1000; // 24h + function getProviderMetadata( settings: LoadedSettings, metadataKey: string, @@ -203,6 +211,19 @@ function findAllPendingUpdates( if (metadata.version === currentVersion) continue; if (metadata.ignoredVersion === currentVersion) continue; + // A "later" choice suppresses re-prompting for the same version while the + // cooldown is active. A new version (postponedVersion mismatch) re-prompts. + // Negative elapsed time (a backward clock jump) is treated as expired so + // the prompt is not suppressed until the wall clock catches up. + if ( + metadata.postponedVersion === currentVersion && + typeof metadata.postponedAt === 'number' && + Date.now() - metadata.postponedAt >= 0 && + Date.now() - metadata.postponedAt < LATER_COOLDOWN_MS + ) { + continue; + } + const existingModelIds = getInstalledOwnedModelIds(settings, provider); const newModelIds = provider.models!.map((s) => s.id); const diff = computeModelDiff(existingModelIds, newModelIds, currentModel); @@ -328,13 +349,11 @@ export function useProviderUpdates( return true; } catch (error) { - const errorMessage = - error instanceof Error ? error.message : String(error); addItem( { type: 'error', text: t('Failed to update provider configuration: {{message}}', { - message: errorMessage, + message: getErrorMessage(error), }), }, Date.now(), @@ -378,10 +397,42 @@ export function useProviderUpdates( p.currentVersion, ); } + } else if (choice === 'later') { + // Persist a cooldown so "later" does not re-prompt on every launch. + // One batched write keeps the version/timestamp pair atomic, so a + // partial persist cannot invalidate the cooldown guard on next launch. + const persistScope = getPersistScopeForModelSelection(settings); + const postponedAt = Date.now(); + try { + settings.setValues( + pendingList.flatMap((p) => [ + { + scope: persistScope, + key: `${PROVIDER_METADATA_NS}.${p.metadataKey}.postponedVersion`, + value: p.currentVersion, + }, + { + scope: persistScope, + key: `${PROVIDER_METADATA_NS}.${p.metadataKey}.postponedAt`, + value: postponedAt, + }, + ]), + ); + } catch (error) { + addItem( + { + type: 'error', + text: t('Failed to save update postponement: {{message}}', { + message: getErrorMessage(error), + }), + }, + Date.now(), + ); + } } }, }); - }, [settings, config, executeUpdate]); + }, [settings, config, executeUpdate, addItem]); useEffect(() => { checkForUpdates();