fix(extensions): preserve api key writes on refresh failures
This commit is contained in:
parent
8075ed10e7
commit
1b1c27e9c6
2 changed files with 126 additions and 5 deletions
|
|
@ -336,6 +336,11 @@ function getSkillsCatalogKey(projectPath?: string): string {
|
||||||
return projectPath ?? USER_SKILLS_CATALOG_KEY;
|
return projectPath ?? USER_SKILLS_CATALOG_KEY;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function upsertApiKeyEntry(entries: ApiKeyEntry[], entry: ApiKeyEntry): ApiKeyEntry[] {
|
||||||
|
const nextEntries = entries.filter((candidate) => candidate.id !== entry.id);
|
||||||
|
return [entry, ...nextEntries];
|
||||||
|
}
|
||||||
|
|
||||||
/** Duration to show "success" state before returning to idle */
|
/** Duration to show "success" state before returning to idle */
|
||||||
const SUCCESS_DISPLAY_MS = 2_000;
|
const SUCCESS_DISPLAY_MS = 2_000;
|
||||||
const PROJECT_SCOPE_REQUIRED_MESSAGE =
|
const PROJECT_SCOPE_REQUIRED_MESSAGE =
|
||||||
|
|
@ -1286,10 +1291,29 @@ export const createExtensionsSlice: StateCreator<AppState, [], [], ExtensionsSli
|
||||||
|
|
||||||
set({ apiKeySaving: true, apiKeysError: null });
|
set({ apiKeySaving: true, apiKeysError: null });
|
||||||
try {
|
try {
|
||||||
await api.apiKeys.save(request);
|
const savedKey = await api.apiKeys.save(request);
|
||||||
// Refresh the list to get updated masked values
|
const warnings: string[] = [];
|
||||||
const [keys] = await Promise.all([api.apiKeys.list(), get().fetchCliStatus()]);
|
|
||||||
set({ apiKeys: keys, apiKeySaving: false });
|
try {
|
||||||
|
const keys = await api.apiKeys.list();
|
||||||
|
set({ apiKeys: keys });
|
||||||
|
} catch (listError) {
|
||||||
|
warnings.push(
|
||||||
|
listError instanceof Error
|
||||||
|
? `API key saved, but failed to refresh key list. ${listError.message}`
|
||||||
|
: 'API key saved, but failed to refresh key list.'
|
||||||
|
);
|
||||||
|
set((prev) => ({
|
||||||
|
apiKeys: upsertApiKeyEntry(prev.apiKeys, savedKey),
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
await get().fetchCliStatus();
|
||||||
|
const refreshError = get().cliStatusError;
|
||||||
|
if (refreshError) {
|
||||||
|
warnings.push(`API key saved, but failed to refresh provider status. ${refreshError}`);
|
||||||
|
}
|
||||||
|
set({ apiKeySaving: false, apiKeysError: warnings.length > 0 ? warnings.join(' ') : null });
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
set({
|
set({
|
||||||
apiKeySaving: false,
|
apiKeySaving: false,
|
||||||
|
|
@ -1305,10 +1329,16 @@ export const createExtensionsSlice: StateCreator<AppState, [], [], ExtensionsSli
|
||||||
|
|
||||||
try {
|
try {
|
||||||
await api.apiKeys.delete(id);
|
await api.apiKeys.delete(id);
|
||||||
await get().fetchCliStatus();
|
|
||||||
set((prev) => ({
|
set((prev) => ({
|
||||||
apiKeys: prev.apiKeys.filter((k) => k.id !== id),
|
apiKeys: prev.apiKeys.filter((k) => k.id !== id),
|
||||||
}));
|
}));
|
||||||
|
await get().fetchCliStatus();
|
||||||
|
const refreshError = get().cliStatusError;
|
||||||
|
set({
|
||||||
|
apiKeysError: refreshError
|
||||||
|
? `API key deleted, but failed to refresh provider status. ${refreshError}`
|
||||||
|
: null,
|
||||||
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
set({
|
set({
|
||||||
apiKeysError: err instanceof Error ? err.message : 'Failed to delete API key',
|
apiKeysError: err instanceof Error ? err.message : 'Failed to delete API key',
|
||||||
|
|
|
||||||
|
|
@ -1025,6 +1025,71 @@ describe('extensionsSlice', () => {
|
||||||
expect(store.getState().apiKeys).toHaveLength(1);
|
expect(store.getState().apiKeys).toHaveLength(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('keeps saved API keys updated when provider status refresh fails', async () => {
|
||||||
|
(api.apiKeys!.save as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||||
|
id: 'k1',
|
||||||
|
name: 'Codex key',
|
||||||
|
envVarName: 'OPENAI_API_KEY',
|
||||||
|
maskedValue: '***',
|
||||||
|
scope: 'user',
|
||||||
|
createdAt: '2026-04-17T10:00:00.000Z',
|
||||||
|
});
|
||||||
|
(api.apiKeys!.list as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||||
|
{
|
||||||
|
id: 'k1',
|
||||||
|
name: 'Codex key',
|
||||||
|
envVarName: 'OPENAI_API_KEY',
|
||||||
|
maskedValue: '***',
|
||||||
|
scope: 'user',
|
||||||
|
createdAt: '2026-04-17T10:00:00.000Z',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
(api.cliInstaller!.getStatus as ReturnType<typeof vi.fn>).mockRejectedValue(
|
||||||
|
new Error('refresh boom')
|
||||||
|
);
|
||||||
|
|
||||||
|
await store.getState().saveApiKey({
|
||||||
|
name: 'Codex key',
|
||||||
|
envVarName: 'OPENAI_API_KEY',
|
||||||
|
value: 'secret',
|
||||||
|
scope: 'user',
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(store.getState().apiKeys).toHaveLength(1);
|
||||||
|
expect(store.getState().apiKeysError).toContain('API key saved, but failed to refresh provider status.');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps local API key state updated when key list refresh fails after save', async () => {
|
||||||
|
(api.apiKeys!.save as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||||
|
id: 'k1',
|
||||||
|
name: 'Codex key',
|
||||||
|
envVarName: 'OPENAI_API_KEY',
|
||||||
|
maskedValue: '***',
|
||||||
|
scope: 'user',
|
||||||
|
createdAt: '2026-04-17T10:00:00.000Z',
|
||||||
|
});
|
||||||
|
(api.apiKeys!.list as ReturnType<typeof vi.fn>).mockRejectedValue(new Error('list boom'));
|
||||||
|
|
||||||
|
await store.getState().saveApiKey({
|
||||||
|
name: 'Codex key',
|
||||||
|
envVarName: 'OPENAI_API_KEY',
|
||||||
|
value: 'secret',
|
||||||
|
scope: 'user',
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(store.getState().apiKeys).toEqual([
|
||||||
|
{
|
||||||
|
id: 'k1',
|
||||||
|
name: 'Codex key',
|
||||||
|
envVarName: 'OPENAI_API_KEY',
|
||||||
|
maskedValue: '***',
|
||||||
|
scope: 'user',
|
||||||
|
createdAt: '2026-04-17T10:00:00.000Z',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
expect(store.getState().apiKeysError).toContain('API key saved, but failed to refresh key list.');
|
||||||
|
});
|
||||||
|
|
||||||
it('refreshes CLI status after deleting an API key', async () => {
|
it('refreshes CLI status after deleting an API key', async () => {
|
||||||
store.setState({
|
store.setState({
|
||||||
apiKeys: [
|
apiKeys: [
|
||||||
|
|
@ -1046,6 +1111,32 @@ describe('extensionsSlice', () => {
|
||||||
expect(store.getState().apiKeys).toEqual([]);
|
expect(store.getState().apiKeys).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('keeps local API key state updated when provider status refresh fails after delete', async () => {
|
||||||
|
store.setState({
|
||||||
|
apiKeys: [
|
||||||
|
{
|
||||||
|
id: 'k1',
|
||||||
|
name: 'Codex key',
|
||||||
|
envVarName: 'OPENAI_API_KEY',
|
||||||
|
maskedValue: '***',
|
||||||
|
scope: 'user',
|
||||||
|
createdAt: '2026-04-17T10:00:00.000Z',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
(api.apiKeys!.delete as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
|
||||||
|
(api.cliInstaller!.getStatus as ReturnType<typeof vi.fn>).mockRejectedValue(
|
||||||
|
new Error('refresh boom')
|
||||||
|
);
|
||||||
|
|
||||||
|
await store.getState().deleteApiKey('k1');
|
||||||
|
|
||||||
|
expect(store.getState().apiKeys).toEqual([]);
|
||||||
|
expect(store.getState().apiKeysError).toContain(
|
||||||
|
'API key deleted, but failed to refresh provider status.'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it('keys MCP diagnostics by scope when the same server exists in multiple scopes', async () => {
|
it('keys MCP diagnostics by scope when the same server exists in multiple scopes', async () => {
|
||||||
(api.mcpRegistry!.diagnose as ReturnType<typeof vi.fn>).mockResolvedValue([
|
(api.mcpRegistry!.diagnose as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue