Skip to content
Merged
255 changes: 253 additions & 2 deletions packages/cli/src/ui/hooks/useProviderUpdates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ describe('useProviderUpdates', () => {
[PROVIDER_METADATA_NS]: {} as Record<string, unknown>,
} as Record<string, unknown>,
setValue: vi.fn(),
setValues: vi.fn(),
forScope: vi.fn(() => ({ path: '/tmp/settings.json' })),
isTrusted: true,
workspace: { settings: {} },
Expand Down Expand Up @@ -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<string, unknown>)[
METADATA_KEY
] = {
Expand All @@ -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<string, unknown>)[
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<string, unknown>)[
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.
Comment thread
yiliang114 marked this conversation as resolved.
const now = Date.now();
const dateNowSpy = vi.spyOn(Date, 'now').mockReturnValue(now);
(mockSettings.merged[PROVIDER_METADATA_NS] as Record<string, unknown>)[
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();
Comment thread
yiliang114 marked this conversation as resolved.

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<string, unknown>)[
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();
Comment thread
yiliang114 marked this conversation as resolved.

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<string, unknown>)[
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<string, unknown>)[
METADATA_KEY
Expand Down
59 changes: 55 additions & 4 deletions packages/cli/src/ui/hooks/useProviderUpdates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment thread
yiliang114 marked this conversation as resolved.

function getProviderMetadata(
settings: LoadedSettings,
metadataKey: string,
Expand Down Expand Up @@ -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
Comment thread
yiliang114 marked this conversation as resolved.
) {
continue;
}
Comment thread
yiliang114 marked this conversation as resolved.

const existingModelIds = getInstalledOwnedModelIds(settings, provider);
const newModelIds = provider.models!.map((s) => s.id);
const diff = computeModelDiff(existingModelIds, newModelIds, currentModel);
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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.
Comment thread
yiliang114 marked this conversation as resolved.
// 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();
Comment thread
yiliang114 marked this conversation as resolved.
try {
settings.setValues(
pendingList.flatMap((p) => [
Comment thread
yiliang114 marked this conversation as resolved.
{
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(
Comment thread
yiliang114 marked this conversation as resolved.
{
type: 'error',
text: t('Failed to save update postponement: {{message}}', {
message: getErrorMessage(error),
}),
},
Date.now(),
);
}
}
},
});
}, [settings, config, executeUpdate]);
}, [settings, config, executeUpdate, addItem]);

useEffect(() => {
checkForUpdates();
Expand Down
Loading