Skip to content

Commit debdb17

Browse files
authored
refactor(db): remove SQLITE_READS_* feature flags and dual-write pattern (#1793)
* refactor(feature-flags): remove SQLITE_READS_* keys and simplify getFeatureFlag to always use SQLite * refactor(settings): remove dual-write and feature-flag gating from settings modules * refactor(threads): remove dual-write and read directly from SQLite * refactor(chat-settings): remove dual-write and read directly from SQLite * refactor(shutdown): remove dual-write and read directly from SQLite * refactor(db): convert reconcileFromStore to a one-time migration with sentinel * fix(db): write migration sentinel directly to ensure transaction atomicity * refactor(db): remove silent error swallowing from all DB writers With electron-store removed as a fallback, DB write failures must propagate to callers so they can handle and log errors appropriately.
1 parent 8d70cdd commit debdb17

21 files changed

Lines changed: 546 additions & 956 deletions

main/src/auto-update.ts

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ import Store from 'electron-store'
1717
import { fetchLatestRelease } from './utils/toolhive-version'
1818
import { writeSetting } from './db/writers/settings-writer'
1919
import { readSetting } from './db/readers/settings-reader'
20-
import { getFeatureFlag } from './feature-flags/flags'
21-
import { featureFlagKeys } from '../../utils/feature-flags'
2220

2321
export type UpdateState =
2422
| 'checking'
@@ -533,7 +531,7 @@ export function initAutoUpdate({
533531
mainWindowGetter: () => BrowserWindow | null
534532
windowCreator: () => Promise<BrowserWindow>
535533
}) {
536-
const isAutoUpdateEnabled = autoUpdateStore.get('isAutoUpdateEnabled')
534+
const isAutoUpdateEnabled = getIsAutoUpdateEnabled()
537535
return Sentry.startSpanManual(
538536
{
539537
name: 'Auto-update initialization',
@@ -650,11 +648,10 @@ export function setAutoUpdateEnabled(enabled: boolean) {
650648
`[update] Auto update ${enabled ? 'enabled' : 'disabled'} dynamically`
651649
)
652650

653-
autoUpdateStore.set('isAutoUpdateEnabled', enabled)
654651
try {
655652
writeSetting('isAutoUpdateEnabled', String(enabled))
656653
} catch (err) {
657-
log.error('[DB] Failed to dual-write isAutoUpdateEnabled:', err)
654+
log.error('[DB] Failed to write isAutoUpdateEnabled:', err)
658655
}
659656

660657
if (!enabled) {
@@ -691,19 +688,17 @@ export function getUpdateState() {
691688
}
692689

693690
export function getIsAutoUpdateEnabled() {
694-
if (getFeatureFlag(featureFlagKeys.SQLITE_READS_SETTINGS)) {
695-
try {
696-
const value = readSetting('isAutoUpdateEnabled')
697-
if (value !== undefined) return value === 'true'
698-
} catch (err) {
699-
log.error('[DB] SQLite read failed, falling back to electron-store:', err)
700-
}
691+
try {
692+
const value = readSetting('isAutoUpdateEnabled')
693+
if (value !== undefined) return value === 'true'
694+
} catch (err) {
695+
log.error('[DB] SQLite read failed:', err)
701696
}
702-
return autoUpdateStore.get('isAutoUpdateEnabled')
697+
return true
703698
}
704699

705700
export async function getLatestAvailableVersion() {
706-
const isAutoUpdateEnabled = autoUpdateStore.get('isAutoUpdateEnabled')
701+
const isAutoUpdateEnabled = getIsAutoUpdateEnabled()
707702
return Sentry.startSpanManual(
708703
{
709704
name: 'Check for latest version',
@@ -748,7 +743,7 @@ export async function getLatestAvailableVersion() {
748743
}
749744

750745
export function manualUpdate() {
751-
const isAutoUpdateEnabled = autoUpdateStore.get('isAutoUpdateEnabled')
746+
const isAutoUpdateEnabled = getIsAutoUpdateEnabled()
752747
return Sentry.startSpanManual(
753748
{
754749
name: 'Manual update triggered',

main/src/chat/settings-storage.ts

Lines changed: 38 additions & 192 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ import {
2626
readSelectedModel as readSelectedModelFromDb,
2727
readEnabledMcpTools as readEnabledMcpToolsFromDb,
2828
} from '../db/readers/chat-settings-reader'
29-
import { getFeatureFlag } from '../feature-flags/flags'
30-
import { featureFlagKeys } from '../../../utils/feature-flags'
3129

3230
// Extract provider IDs from CHAT_PROVIDER_INFO
3331
type ProviderId = (typeof CHAT_PROVIDER_INFO)[number]['id']
@@ -60,45 +58,7 @@ interface ChatSettings {
6058
enabledMcpTools: Record<string, string[]> // serverName -> [toolName1, toolName2]
6159
}
6260

63-
// Type guard functions
64-
function isRecord(value: unknown): value is Record<string, unknown> {
65-
return typeof value === 'object' && value !== null && !Array.isArray(value)
66-
}
67-
68-
function isStringArray(value: unknown): value is string[] {
69-
return Array.isArray(value) && value.every((item) => typeof item === 'string')
70-
}
71-
72-
function isProvidersRecord(value: unknown): value is ChatSettings['providers'] {
73-
if (!isRecord(value)) return false
74-
return Object.entries(value).every(([providerId, item]) => {
75-
if (!isRecord(item) || !isStringArray(item.enabledTools)) return false
76-
77-
// Check if it's a local server provider (with endpointURL) or cloud provider (with apiKey)
78-
if (isLocalProvider(providerId)) {
79-
return typeof item.endpointURL === 'string'
80-
} else {
81-
return typeof item.apiKey === 'string'
82-
}
83-
})
84-
}
85-
86-
function isToolsRecord(
87-
value: unknown
88-
): value is ChatSettings['enabledMcpTools'] {
89-
if (!isRecord(value)) return false
90-
return Object.values(value).every((item) => isStringArray(item))
91-
}
92-
93-
function isSelectedModel(value: unknown): value is ChatSettingsSelectedModel {
94-
return (
95-
isRecord(value) &&
96-
typeof value.provider === 'string' &&
97-
typeof value.model === 'string'
98-
)
99-
}
100-
101-
// Create a secure store for chat settings (API keys and model selection)
61+
// Kept for one-time reconciliation migration; remove after migration grace period
10262
export const chatSettingsStore = new Store<ChatSettings>({
10363
name: 'chat-settings',
10464
encryptionKey: 'toolhive-chat-encryption-key', // Basic encryption for API keys
@@ -116,81 +76,38 @@ export const chatSettingsStore = new Store<ChatSettings>({
11676
// Get chat settings for a provider
11777
export function getChatSettings(providerId: ProviderId): ChatSettingsProvider {
11878
try {
119-
if (getFeatureFlag(featureFlagKeys.SQLITE_READS_CHAT_SETTINGS)) {
120-
try {
121-
const dbProvider = readChatProviderFromDb(providerId)
122-
if (dbProvider) {
123-
if (isLocalProvider(providerId)) {
124-
return {
125-
providerId,
126-
endpointURL: dbProvider.endpointURL ?? '',
127-
enabledTools: [],
128-
}
129-
} else {
130-
return {
131-
providerId,
132-
apiKey: dbProvider.apiKey ?? '',
133-
enabledTools: [],
134-
}
135-
}
136-
}
137-
} catch (err) {
138-
log.error(
139-
'[DB] SQLite read failed, falling back to electron-store:',
140-
err
141-
)
142-
}
143-
}
144-
const providers = chatSettingsStore.get('providers')
145-
if (isProvidersRecord(providers)) {
146-
const existing = providers[providerId]
147-
if (existing) return existing
148-
149-
// Return default based on provider type
79+
const dbProvider = readChatProviderFromDb(providerId)
80+
if (dbProvider) {
15081
if (isLocalProvider(providerId)) {
15182
return {
15283
providerId,
153-
endpointURL: '',
84+
endpointURL: dbProvider.endpointURL ?? '',
15485
enabledTools: [],
15586
}
15687
} else {
15788
return {
15889
providerId,
159-
apiKey: '',
90+
apiKey: dbProvider.apiKey ?? '',
16091
enabledTools: [],
16192
}
16293
}
16394
}
95+
} catch (err) {
96+
log.error('[DB] SQLite read failed:', err)
97+
}
16498

165-
// Fallback defaults
166-
if (isLocalProvider(providerId)) {
167-
return {
168-
providerId,
169-
endpointURL: '',
170-
enabledTools: [],
171-
}
172-
} else {
173-
return {
174-
providerId,
175-
apiKey: '',
176-
enabledTools: [],
177-
}
99+
// Fallback defaults
100+
if (isLocalProvider(providerId)) {
101+
return {
102+
providerId,
103+
endpointURL: '',
104+
enabledTools: [],
178105
}
179-
} catch (error) {
180-
log.error('Failed to get chat settings:', error)
181-
// Fallback defaults
182-
if (isLocalProvider(providerId)) {
183-
return {
184-
providerId,
185-
endpointURL: '',
186-
enabledTools: [],
187-
}
188-
} else {
189-
return {
190-
providerId,
191-
apiKey: '',
192-
enabledTools: [],
193-
}
106+
} else {
107+
return {
108+
providerId,
109+
apiKey: '',
110+
enabledTools: [],
194111
}
195112
}
196113
}
@@ -227,22 +144,14 @@ function saveChatSettings(
227144
enabledTools: settings.enabledTools,
228145
}
229146

230-
const providers = chatSettingsStore.get('providers')
231-
const typedProviders = isProvidersRecord(providers) ? providers : {}
232-
typedProviders[providerId] = settingsWithProviderId
233-
chatSettingsStore.set('providers', typedProviders)
234-
try {
235-
if (isLocalProvider(providerId)) {
236-
writeProvider(providerId, {
237-
endpointURL: extractEndpointURL(settingsWithProviderId),
238-
})
239-
} else {
240-
writeProvider(providerId, {
241-
apiKey: extractApiKey(settingsWithProviderId),
242-
})
243-
}
244-
} catch (err) {
245-
log.error('[DB] Failed to dual-write provider settings:', err)
147+
if (isLocalProvider(providerId)) {
148+
writeProvider(providerId, {
149+
endpointURL: extractEndpointURL(settingsWithProviderId),
150+
})
151+
} else {
152+
writeProvider(providerId, {
153+
apiKey: extractApiKey(settingsWithProviderId),
154+
})
246155
}
247156
return { success: true }
248157
} catch (error) {
@@ -264,23 +173,9 @@ export function clearChatSettings(providerId?: ProviderId): {
264173
} {
265174
try {
266175
if (providerId) {
267-
const providers = chatSettingsStore.get('providers')
268-
const typedProviders = isProvidersRecord(providers) ? providers : {}
269-
delete typedProviders[providerId]
270-
chatSettingsStore.set('providers', typedProviders)
271-
try {
272-
deleteProvider(providerId)
273-
} catch (err) {
274-
log.error('[DB] Failed to dual-write provider deletion:', err)
275-
}
176+
deleteProvider(providerId)
276177
} else {
277-
// Clear all providers if no specific provider is given
278-
chatSettingsStore.set('providers', {})
279-
try {
280-
clearAllProviders()
281-
} catch (err) {
282-
log.error('[DB] Failed to dual-write clear all providers:', err)
283-
}
178+
clearAllProviders()
284179
}
285180
return { success: true }
286181
} catch (error) {
@@ -294,30 +189,12 @@ export function clearChatSettings(providerId?: ProviderId): {
294189
// Get selected model
295190
export function getSelectedModel(): ChatSettingsSelectedModel {
296191
try {
297-
if (getFeatureFlag(featureFlagKeys.SQLITE_READS_CHAT_SETTINGS)) {
298-
try {
299-
const dbModel = readSelectedModelFromDb()
300-
if (dbModel.provider && dbModel.model) return dbModel
301-
} catch (err) {
302-
log.error(
303-
'[DB] SQLite read failed, falling back to electron-store:',
304-
err
305-
)
306-
}
307-
}
308-
const selectedModel = chatSettingsStore.get('selectedModel')
309-
if (
310-
isSelectedModel(selectedModel) &&
311-
selectedModel.provider &&
312-
selectedModel.model
313-
) {
314-
return selectedModel
315-
}
316-
return { provider: '', model: '' }
317-
} catch (error) {
318-
log.error('Failed to get selected model:', error)
319-
return { provider: '', model: '' }
192+
const dbModel = readSelectedModelFromDb()
193+
if (dbModel.provider && dbModel.model) return dbModel
194+
} catch (err) {
195+
log.error('[DB] SQLite read failed:', err)
320196
}
197+
return { provider: '', model: '' }
321198
}
322199

323200
// Save selected model
@@ -326,12 +203,7 @@ export function saveSelectedModel(
326203
model: string
327204
): { success: boolean; error?: string } {
328205
try {
329-
chatSettingsStore.set('selectedModel', { provider, model })
330-
try {
331-
writeSelectedModel(provider, model)
332-
} catch (err) {
333-
log.error('[DB] Failed to dual-write selected model:', err)
334-
}
206+
writeSelectedModel(provider, model)
335207
return { success: true }
336208
} catch (error) {
337209
return {
@@ -347,15 +219,7 @@ export function saveEnabledMcpTools(
347219
toolNames: string[]
348220
): { success: boolean; error?: string } {
349221
try {
350-
const enabledMcpTools = chatSettingsStore.get('enabledMcpTools')
351-
const typedTools = isToolsRecord(enabledMcpTools) ? enabledMcpTools : {}
352-
typedTools[serverName] = toolNames
353-
chatSettingsStore.set('enabledMcpTools', typedTools)
354-
try {
355-
writeEnabledMcpTools(serverName, toolNames)
356-
} catch (err) {
357-
log.error('[DB] Failed to dual-write enabled MCP tools:', err)
358-
}
222+
writeEnabledMcpTools(serverName, toolNames)
359223
return { success: true }
360224
} catch (error) {
361225
return {
@@ -370,20 +234,7 @@ export async function getEnabledMcpTools(): Promise<
370234
ChatSettings['enabledMcpTools']
371235
> {
372236
try {
373-
if (getFeatureFlag(featureFlagKeys.SQLITE_READS_CHAT_SETTINGS)) {
374-
try {
375-
return readEnabledMcpToolsFromDb()
376-
} catch (err) {
377-
log.error(
378-
'[DB] SQLite read failed, falling back to electron-store:',
379-
err
380-
)
381-
}
382-
}
383-
const enabledMcpTools = chatSettingsStore.get('enabledMcpTools')
384-
if (!isToolsRecord(enabledMcpTools)) {
385-
return {}
386-
}
237+
const enabledMcpTools = readEnabledMcpToolsFromDb()
387238

388239
// Skip validation during shutdown to prevent interrupting teardown
389240
if (getTearingDownState()) {
@@ -438,16 +289,11 @@ export async function getEnabledMcpTools(): Promise<
438289

439290
// Remove stopped servers from storage in one operation
440291
if (serversToRemove.length > 0) {
441-
const updatedTools = { ...enabledMcpTools }
442-
for (const serverName of serversToRemove) {
443-
delete updatedTools[serverName]
444-
}
445-
chatSettingsStore.set('enabledMcpTools', updatedTools)
446292
for (const serverName of serversToRemove) {
447293
try {
448294
deleteEnabledMcpTools(serverName)
449295
} catch (err) {
450-
log.error('[DB] Failed to dual-write MCP tools cleanup:', err)
296+
log.error('[DB] Failed to delete MCP tools for server:', err)
451297
}
452298
}
453299
}

0 commit comments

Comments
 (0)