From 226c9193d6645246034e8c198709ef59954be538 Mon Sep 17 00:00:00 2001 From: hugo Date: Sun, 1 Mar 2026 13:14:15 +0100 Subject: [PATCH] fix: isolate cleanup from key retrieval, add deleteSetting test --- src/main/ipc/chatHandlers.ts | 8 ++++++-- tests/engine/ChatEngine.test.ts | 17 +++++++++++++++++ tests/ipc/chatHandlersKeychain.test.ts | 6 +++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/main/ipc/chatHandlers.ts b/src/main/ipc/chatHandlers.ts index 46c8d13..26e9c22 100644 --- a/src/main/ipc/chatHandlers.ts +++ b/src/main/ipc/chatHandlers.ts @@ -62,11 +62,15 @@ async function getOpenCodeManager(): Promise { // Load API key from encrypted storage const keyStore = getSecureKeyStore(); openCodeManagerInitPromise = (async () => { + // Clean up old plain-text key from settings (pre-keychain storage) try { - // Clean up old plain-text key from settings (pre-keychain storage) await keyStore.cleanupPlainTextKey('opencode_api_key'); + } catch { + // Best-effort cleanup; not critical + } - // Load API key from encrypted storage + // Load API key from encrypted storage + try { const key = await keyStore.retrieve('opencode_api_key'); if (key) { openCodeManager!.setApiKey(key); diff --git a/tests/engine/ChatEngine.test.ts b/tests/engine/ChatEngine.test.ts index 2fe76a0..3e0ba73 100644 --- a/tests/engine/ChatEngine.test.ts +++ b/tests/engine/ChatEngine.test.ts @@ -776,6 +776,23 @@ describe('ChatEngine', () => { }); }); + describe('deleteSetting', () => { + it('should delete a setting by key', async () => { + let capturedPredicate: any; + vi.mocked(mockLocalDb.delete).mockImplementation(() => ({ + where: vi.fn((predicate) => { + capturedPredicate = predicate; + return Promise.resolve(); + }), + } as any)); + + await chatEngine.deleteSetting('opencode_api_key'); + + expect(mockLocalDb.delete).toHaveBeenCalled(); + expect(capturedPredicate).toBeDefined(); + }); + }); + describe('setSelectedModel', () => { it('should save selected model', async () => { let capturedValues: any; diff --git a/tests/ipc/chatHandlersKeychain.test.ts b/tests/ipc/chatHandlersKeychain.test.ts index 27a868e..1d729cf 100644 --- a/tests/ipc/chatHandlersKeychain.test.ts +++ b/tests/ipc/chatHandlersKeychain.test.ts @@ -240,7 +240,7 @@ describe('chatHandlers keychain integration', () => { expect(manager.setApiKey).not.toHaveBeenCalled(); }); - it('still initializes when cleanupPlainTextKey() throws on init', async () => { + it('still initializes and loads key when cleanupPlainTextKey() throws on init', async () => { secureKeyStoreCleanupError = new Error('delete failed'); const mod = await import('../../src/main/ipc/chatHandlers'); @@ -252,6 +252,10 @@ describe('chatHandlers keychain integration', () => { const result = await handler!(undefined); // Init should complete even if cleanup fails expect(result.ready).toBe(true); + + // The encrypted key should still be loaded despite cleanup failure + const manager = openCodeManagerInstances[0]; + expect(manager.setApiKey).toHaveBeenCalledWith('encrypted-stored-key'); }); it('returns error when store() throws on chat:setApiKey', async () => {