Skip to content

Fix event hooks not persisting across server syncs #799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gsxdsm merged 2 commits into from
Feb 22, 2026
Merged

Fix event hooks not persisting across server syncs #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bcf783c
Changes from fix/event-hook-persistence
gsxdsm Feb 22, 2026
a80da2c
feat: Add explicit permission escape hatch for clearing eventHooks an…
gsxdsm Feb 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions apps/server/src/services/settings-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>).__allowEmptyEventHooks === true;
// Remove the flag so it doesn't get persisted
delete (sanitizedUpdates as Record<string, unknown>).__allowEmptyEventHooks;

// Only guard eventHooks if explicit permission wasn't granted
if (!allowEmptyEventHooks) {
ignoreEmptyArrayOverwrite('eventHooks');
}

// Empty object overwrite guard
const ignoreEmptyObjectOverwrite = <K extends keyof GlobalSettings>(key: K): void => {
const nextVal = sanitizedUpdates[key] as unknown;
Expand Down
45 changes: 32 additions & 13 deletions apps/ui/src/components/views/settings-view/event-hooks/event-hooks-section.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -26,24 +30,39 @@ export function EventHooksSection() {
setDialogOpen(true);
};

const handleDeleteHook = (hookId: string) => {
setEventHooks(eventHooks.filter((h) => h.id !== hookId));
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 = (hookId: string, enabled: boolean) => {
setEventHooks(eventHooks.map((h) => (h.id === hookId ? { ...h, enabled } : h)));
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 = (hook: EventHook) => {
if (editingHook) {
// Update existing
setEventHooks(eventHooks.map((h) => (h.id === hook.id ? hook : h)));
} else {
// Add new
setEventHooks([...eventHooks, 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);
};
Comment on lines +33 to 66
Copy link

@coderabbitai coderabbitai bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find app-store.ts
fd -t f 'app-store' --extension ts

Repository: AutoMaker-Org/automaker

Length of output: 97

🏁 Script executed:

# Search for setEventHooks implementation
rg -n 'setEventHooks' --type ts -A 30 -B 2 | head -100

Repository: AutoMaker-Org/automaker

Length of output: 9928

Implement rollback in setEventHooks on server failure, or restore prior state in each handler.

The setEventHooks implementation in app-store.ts (lines 1418–1426) uses an optimistic-update pattern: it mutates the Zustand store immediately (set({ eventHooks: hooks })) before awaiting the server call (httpApi.settings.updateGlobal). If the server rejects the change, the catch block only logs the error and does not revert the local state.

This means if the network fails or the server returns an error, the UI permanently reflects the mutation (hook deleted/toggled/added) while the server retains the original state—a divergence that persists until the next sync.

Option 1: Implement rollback in the store:

setEventHooks: async (hooks) => {
  const previous = get().eventHooks;  // Save prior state
  set({ eventHooks: hooks });
  try {
    const httpApi = getHttpApiClient();
    await httpApi.settings.updateGlobal({ eventHooks: hooks });
  } catch (error) {
    logger.error('Failed to sync event hooks:', error);
    set({ eventHooks: previous });  // Revert on error
  }
},

Option 2: Add explicit rollback in each component handler:

  const handleDeleteHook = async (hookId: string) => {
+   const previousHooks = eventHooks;
    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');
+     await setEventHooks(previousHooks).catch(() => {});
    }
  };
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@apps/ui/src/components/views/settings-view/event-hooks/event-hooks-section.tsx`
around lines 33 - 66, The component handlers (handleDeleteHook,
handleToggleHook, handleSaveHook) call setEventHooks which currently performs an
optimistic update without rollback; add a proper rollback by saving the previous
eventHooks state before calling setEventHooks and restoring it if the server
call fails, or implement the rollback inside setEventHooks itself by capturing
get().eventHooks into a local variable, applying the optimistic set, calling
httpApi.settings.updateGlobal, and on catch restoring the saved previous state
and logging the error (ensure you reference setEventHooks in app-store.ts for
this change).


// Group hooks by trigger type for better organization
Expand Down
9 changes: 9 additions & 0 deletions apps/ui/src/hooks/use-settings-migration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
15 changes: 15 additions & 0 deletions apps/ui/src/routes/__root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +604 to +605
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In conjunction with my comment on settings-service.ts, this serverHooks.length > 0 check should be removed. It prevents the client from syncing an empty eventHooks array from the server (e.g., when a user deletes all hooks on another device). This can lead to data inconsistencies where this client retains stale hooks and may even re-sync them back to the server later.

Removing this check will ensure that deleting all hooks is correctly propagated across clients.

                    JSON.stringify(serverHooks) !== JSON.stringify(currentHooks)

) {
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);
}
Expand Down
10 changes: 9 additions & 1 deletion apps/ui/src/store/app-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1415,7 +1415,15 @@ export const useAppStore = create<AppState & AppActions>()((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) => {
Expand Down
2 changes: 1 addition & 1 deletion apps/ui/src/store/types/state-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ export interface AppActions {
setPromptCustomization: (customization: PromptCustomization) => Promise<void>;

// Event Hook actions
setEventHooks: (hooks: EventHook[]) => void;
setEventHooks: (hooks: EventHook[]) => Promise<void>;

// Claude-Compatible Provider actions (new system)
addClaudeCompatibleProvider: (provider: ClaudeCompatibleProvider) => Promise<void>;
Expand Down
2 changes: 2 additions & 0 deletions libs/types/src/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down