From 1d732916f1549e47ef4bb28b3fa9efbc5564b2cb Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 22 Feb 2026 00:36:08 -0800 Subject: [PATCH] Fix event hooks not persisting across server syncs (#799) * Changes from fix/event-hook-persistence * feat: Add explicit permission escape hatch for clearing eventHooks and improve error handling in UI --- apps/server/src/services/settings-service.ts | 11 ++++ .../event-hooks/event-hooks-section.tsx | 53 +++++++++++++------ apps/ui/src/hooks/use-settings-migration.ts | 9 ++++ apps/ui/src/routes/__root.tsx | 15 ++++++ apps/ui/src/store/app-store.ts | 10 +++- apps/ui/src/store/types/state-types.ts | 2 +- libs/types/src/settings.ts | 2 + 7 files changed, 83 insertions(+), 19 deletions(-) diff --git a/apps/server/src/services/settings-service.ts b/apps/server/src/services/settings-service.ts index 6a3d804e..ebf8556f 100644 --- a/apps/server/src/services/settings-service.ts +++ b/apps/server/src/services/settings-service.ts @@ -573,6 +573,17 @@ export class SettingsService { ignoreEmptyArrayOverwrite('claudeApiProfiles'); // Note: claudeCompatibleProviders intentionally NOT guarded - users should be able to delete all providers + // Check for explicit permission to clear eventHooks (escape hatch for intentional clearing) + const allowEmptyEventHooks = + (sanitizedUpdates as Record).__allowEmptyEventHooks === true; + // Remove the flag so it doesn't get persisted + delete (sanitizedUpdates as Record).__allowEmptyEventHooks; + + // Only guard eventHooks if explicit permission wasn't granted + if (!allowEmptyEventHooks) { + ignoreEmptyArrayOverwrite('eventHooks'); + } + // Empty object overwrite guard const ignoreEmptyObjectOverwrite = (key: K): void => { const nextVal = sanitizedUpdates[key] as unknown; diff --git a/apps/ui/src/components/views/settings-view/event-hooks/event-hooks-section.tsx b/apps/ui/src/components/views/settings-view/event-hooks/event-hooks-section.tsx index 519ca370..21c18f68 100644 --- a/apps/ui/src/components/views/settings-view/event-hooks/event-hooks-section.tsx +++ b/apps/ui/src/components/views/settings-view/event-hooks/event-hooks-section.tsx @@ -9,6 +9,10 @@ import type { EventHook, EventHookTrigger } from '@automaker/types'; import { EVENT_HOOK_TRIGGER_LABELS } from '@automaker/types'; import { EventHookDialog } from './event-hook-dialog'; import { EventHistoryView } from './event-history-view'; +import { toast } from 'sonner'; +import { createLogger } from '@automaker/utils/logger'; + +const logger = createLogger('EventHooks'); export function EventHooksSection() { const { eventHooks, setEventHooks } = useAppStore(); @@ -26,24 +30,39 @@ export function EventHooksSection() { setDialogOpen(true); }; - const handleDeleteHook = (hookId: string) => { - setEventHooks(eventHooks.filter((h) => h.id !== hookId)); - }; - - const handleToggleHook = (hookId: string, enabled: boolean) => { - setEventHooks(eventHooks.map((h) => (h.id === hookId ? { ...h, enabled } : h))); - }; - - const handleSaveHook = (hook: EventHook) => { - if (editingHook) { - // Update existing - setEventHooks(eventHooks.map((h) => (h.id === hook.id ? hook : h))); - } else { - // Add new - setEventHooks([...eventHooks, hook]); + const handleDeleteHook = async (hookId: string) => { + try { + await setEventHooks(eventHooks.filter((h) => h.id !== hookId)); + } catch (error) { + logger.error('Failed to delete event hook:', error); + toast.error('Failed to delete event hook'); + } + }; + + const handleToggleHook = async (hookId: string, enabled: boolean) => { + try { + await setEventHooks(eventHooks.map((h) => (h.id === hookId ? { ...h, enabled } : h))); + } catch (error) { + logger.error('Failed to toggle event hook:', error); + toast.error('Failed to update event hook'); + } + }; + + const handleSaveHook = async (hook: EventHook) => { + try { + if (editingHook) { + // Update existing + await setEventHooks(eventHooks.map((h) => (h.id === hook.id ? hook : h))); + } else { + // Add new + await setEventHooks([...eventHooks, hook]); + } + setDialogOpen(false); + setEditingHook(null); + } catch (error) { + logger.error('Failed to save event hook:', error); + toast.error('Failed to save event hook'); } - setDialogOpen(false); - setEditingHook(null); }; // Group hooks by trigger type for better organization diff --git a/apps/ui/src/hooks/use-settings-migration.ts b/apps/ui/src/hooks/use-settings-migration.ts index 9bed05b8..a69afbe1 100644 --- a/apps/ui/src/hooks/use-settings-migration.ts +++ b/apps/ui/src/hooks/use-settings-migration.ts @@ -363,6 +363,15 @@ export function mergeSettings( merged.claudeCompatibleProviders = localSettings.claudeCompatibleProviders; } + // Event hooks - preserve from localStorage if server is empty + if ( + (!serverSettings.eventHooks || serverSettings.eventHooks.length === 0) && + localSettings.eventHooks && + localSettings.eventHooks.length > 0 + ) { + merged.eventHooks = localSettings.eventHooks; + } + // Preserve new settings fields from localStorage if server has defaults // Use nullish coalescing to accept stored falsy values (e.g. false) if (localSettings.enableAiCommitMessages != null && merged.enableAiCommitMessages == null) { diff --git a/apps/ui/src/routes/__root.tsx b/apps/ui/src/routes/__root.tsx index ac19d2a4..d133b37f 100644 --- a/apps/ui/src/routes/__root.tsx +++ b/apps/ui/src/routes/__root.tsx @@ -594,6 +594,21 @@ function RootLayoutContent() { logger.info( '[FAST_HYDRATE] Background reconcile: cache updated (store untouched)' ); + + // Selectively reconcile event hooks from server. + // Unlike projects/theme, eventHooks aren't rendered on the main view, + // so updating them won't cause a visible re-render flash. + const serverHooks = (finalSettings as GlobalSettings).eventHooks ?? []; + const currentHooks = useAppStore.getState().eventHooks; + if ( + JSON.stringify(serverHooks) !== JSON.stringify(currentHooks) && + serverHooks.length > 0 + ) { + logger.info( + `[FAST_HYDRATE] Reconciling eventHooks from server (server=${serverHooks.length}, store=${currentHooks.length})` + ); + useAppStore.setState({ eventHooks: serverHooks }); + } } catch (e) { logger.debug('[FAST_HYDRATE] Failed to update cache:', e); } diff --git a/apps/ui/src/store/app-store.ts b/apps/ui/src/store/app-store.ts index 902ed885..96e60422 100644 --- a/apps/ui/src/store/app-store.ts +++ b/apps/ui/src/store/app-store.ts @@ -1415,7 +1415,15 @@ export const useAppStore = create()((set, get) => ({ }, // Event Hook actions - setEventHooks: (hooks) => set({ eventHooks: hooks }), + setEventHooks: async (hooks) => { + set({ eventHooks: hooks }); + try { + const httpApi = getHttpApiClient(); + await httpApi.settings.updateGlobal({ eventHooks: hooks }); + } catch (error) { + logger.error('Failed to sync event hooks:', error); + } + }, // Claude-Compatible Provider actions (new system) addClaudeCompatibleProvider: async (provider) => { diff --git a/apps/ui/src/store/types/state-types.ts b/apps/ui/src/store/types/state-types.ts index aeeaa2a8..df8d600d 100644 --- a/apps/ui/src/store/types/state-types.ts +++ b/apps/ui/src/store/types/state-types.ts @@ -634,7 +634,7 @@ export interface AppActions { setPromptCustomization: (customization: PromptCustomization) => Promise; // Event Hook actions - setEventHooks: (hooks: EventHook[]) => void; + setEventHooks: (hooks: EventHook[]) => Promise; // Claude-Compatible Provider actions (new system) addClaudeCompatibleProvider: (provider: ClaudeCompatibleProvider) => Promise; diff --git a/libs/types/src/settings.ts b/libs/types/src/settings.ts index 6af9faf5..108883d3 100644 --- a/libs/types/src/settings.ts +++ b/libs/types/src/settings.ts @@ -1672,6 +1672,8 @@ export const DEFAULT_GLOBAL_SETTINGS: GlobalSettings = { skillsSources: ['user', 'project'], enableSubagents: true, subagentsSources: ['user', 'project'], + // Event hooks + eventHooks: [], // New provider system claudeCompatibleProviders: [], // Deprecated - kept for migration