improvement(settings): shared save/discard + unsaved-changes guard

Consolidate every editable settings surface onto one stack instead of
per-page custom logic:

- SaveDiscardActions: the canonical dirty-gated Discard+Save chip pair
- useSettingsUnsavedGuard: syncs local dirty into useSettingsDirtyStore (so the
  sidebar section-switch confirm applies) + provides guardBack/UnsavedChangesModal
  for detail sub-views' back chip
- useSettingsBeforeUnload: a single beforeunload in the settings shell

Migrate whitelabeling, sso, access-control group-detail, data-retention, and
secrets-manager (drops its duplicate beforeunload). Deletes the hand-rolled
'Unsaved changes' modals; the leave-confirm standardizes on Keep editing /
Discard. Documents the pattern in the settings rule + add-settings-page skill.

Author: waleedlatif1

.claude/rules/sim-settings-pages.md

@@ -101,6 +101,46 @@ Adding a new settings page:
   Conditional items become array spreads: `...(canManage ? [{…}] : [])`. Never
   hand-roll the `<DropdownMenu>` + `<MoreHorizontal>` trigger per page.
 
+## Save / Discard + unsaved-changes guard
+
+Any settings surface with editable state uses **one** shared stack — never
+hand-roll a Save button, a Discard button, a `beforeunload`, or an "Unsaved
+changes" modal:
+
+- **`SaveDiscardActions`** (`…/components/save-discard-actions/save-discard-actions`)
+  — the canonical dirty-gated **Discard + Save** chip pair. Renders nothing when
+  `!dirty`; otherwise a fragment so it composes beside sibling chips (a detail
+  view's Delete / Remove override, a Share chip). Props: `dirty`, `saving`,
+  `onSave`, `onDiscard`, `saveDisabled?`, `saveLabel?`, `savingLabel?`. Put it in
+  the `SettingsPanel actions` slot (top-level pages) or the detail header bar.
+- **`useSettingsUnsavedGuard({ isDirty })`** (`…/settings/hooks/use-settings-unsaved-guard`)
+  — syncs the page's local `isDirty` into the shared `useSettingsDirtyStore` (so
+  the sidebar's **section-switch** confirm + the centralized `beforeunload` both
+  apply for free) and returns `{ showUnsavedModal, setShowUnsavedModal, guardBack,
+  confirmDiscard }` for a detail view's **in-view back** chip.
+  - **Top-level pages** (whitelabeling, sso): call it **unassigned** —
+    `useSettingsUnsavedGuard({ isDirty: hasChanges })` — they only need the
+    store-sync; the sidebar/`beforeunload` do the rest.
+  - **Detail sub-views** (data-retention, access-control group-detail): route the
+    back chip through `onClick={() => guard.guardBack(closeFn)}` and render the
+    shared `<UnsavedChangesModal open={guard.showUnsavedModal}
+    onOpenChange={guard.setShowUnsavedModal} onDiscard={guard.confirmDiscard} />`
+    (from `@/app/workspace/[workspaceId]/components/credential-detail`). The
+    in-view header **Discard** chip (via `SaveDiscardActions onDiscard`) is a
+    *reset to original* — distinct from the back-confirm's discard, which leaves.
+- **`useSettingsBeforeUnload`** is mounted **once** in the settings shell
+  (`settings/[section]/settings.tsx`) — never add a per-page `beforeunload`.
+- **Dirty *computation* stays local** (shapes differ: field-compare vs
+  normalize+stringify) — only how dirty is *consumed* is shared. Derive it (a
+  `const`/`useMemo`), never store it in `useState`.
+- **CRITICAL — rules of hooks:** call `useSettingsUnsavedGuard(...)`
+  **unconditionally, before every early-return gate** (entitlement / loading /
+  not-entitled `return <SettingsEmptyState>`). A hook placed after a gate is
+  skipped on gated renders and crashes.
+- The route-based credential detail keeps its own `useUnsavedChangesGuard` (it
+  guards real `router.push` navigation + browser Back via a history sentinel);
+  it already shares `UnsavedChangesModal`, so copy stays unified.
+
 ## Detail sub-views (the one exception)
 
 A drill-down view reached from a list row (selected MCP server, workflow MCP
@@ -119,5 +159,6 @@ A settings page is design-system-clean when:
 - [ ] Header chips are in `actions`; a standalone search is in the `search` prop.
 - [ ] Its `NavigationItem` has an accurate, consistent-length `description`.
 - [ ] Detail sub-views and entitlement/loading gates keep their own chrome (intentional).
+- [ ] If it has editable state: Save/Discard go through `SaveDiscardActions`, dirty is wired via `useSettingsUnsavedGuard` (called before any early-return gate), and there is **no** hand-rolled Save button / `beforeunload` / "Unsaved changes" modal.
 - [ ] No business logic, handlers, or conditional rendering changed by the migration.
 - [ ] `tsc`, `biome`, and the page's tests pass.

.claude/skills/add-settings-page/SKILL.md

@@ -29,7 +29,15 @@ Key paths:
    bar, scroll region, content column, or title block. Put header buttons in
    `actions`, a standalone search in `search={{ value, onChange, placeholder }}`,
    and the page content as `children`. Modals go beside the panel inside a `<>`.
-4. **Verify:** `cd apps/sim && bunx tsc --noEmit`; `bunx biome check --write <file>`.
+4. **If the page has editable state**, wire the shared save/discard stack — put
+   `SaveDiscardActions` (dirty-gated Discard+Save chips) in `actions`, and call
+   `useSettingsUnsavedGuard({ isDirty })` **before any early-return gate**.
+   Detail sub-views additionally route the back chip through
+   `guard.guardBack(closeFn)` and render the shared `UnsavedChangesModal`. Never
+   hand-roll a Save button, a `beforeunload`, or an "Unsaved changes" modal —
+   they're centralized. See the "Save / Discard + unsaved-changes guard" section
+   in `.claude/rules/sim-settings-pages.md`.
+5. **Verify:** `cd apps/sim && bunx tsc --noEmit`; `bunx biome check --write <file>`.
 
 ## Mode B — Audit existing settings pages
 
@@ -44,6 +52,11 @@ For each page component, confirm the checklist in `.claude/rules/sim-settings-pa
    `git grep -n "text-\[var(--text-body)\] text-lg" -- 'apps/sim/**/settings/' 'apps/sim/ee/'`
 3. Confirm each page imports `SettingsPanel` and that its `NavigationItem` has an
    accurate `description` of consistent length with its peers.
+   - Editable pages: confirm Save/Discard go through `SaveDiscardActions` and
+     dirty is wired via `useSettingsUnsavedGuard` (called before early-return
+     gates) — flag any hand-rolled Save button, `beforeunload`, or unsaved modal.
+     `git grep -n "beforeunload" -- 'apps/sim/**/settings/' 'apps/sim/ee/'`
+     should only hit the centralized `use-settings-before-unload.ts`.
 4. When migrating a page, change ONLY the structural shell→`SettingsPanel` swap:
    move header chips to `actions`, the standalone search to `search`, delete the
    `<h1>` title block, replace the three closing `</div>` (column/scroll/shell)

apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx

@@ -7,6 +7,7 @@ import { useSession } from '@/lib/auth/auth-client'
 import { captureEvent } from '@/lib/posthog/client'
 import { General } from '@/app/workspace/[workspaceId]/settings/components/general/general'
 import { SettingsSectionProvider } from '@/app/workspace/[workspaceId]/settings/components/settings-panel'
+import { useSettingsBeforeUnload } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-before-unload'
 import type { SettingsSection } from '@/app/workspace/[workspaceId]/settings/navigation'
 import {
   isBillingEnabled,
@@ -105,6 +106,8 @@ export function SettingsPage({ section }: SettingsPageProps) {
   const { data: session, isPending: sessionLoading } = useSession()
   const posthog = usePostHog()
 
+  useSettingsBeforeUnload()
+
   const isAdminRole = session?.user?.role === 'admin'
   const normalizedSection: SettingsSection =
     (section as string) === 'subscription' ? 'billing' : section

apps/sim/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions.tsx

@@ -0,0 +1,42 @@
+import { Chip } from '@/components/emcn'
+
+interface SaveDiscardActionsProps {
+  /** When false, renders nothing. */
+  dirty: boolean
+  /** A save is in flight — disables both chips and shows `savingLabel` on Save. */
+  saving: boolean
+  onSave: () => void
+  onDiscard: () => void
+  /** Disables Save independently of `saving` (e.g. validation errors, empty required field). */
+  saveDisabled?: boolean
+  savingLabel?: string
+  saveLabel?: string
+}
+
+/**
+ * The canonical dirty-gated Discard + Save chip pair for settings surfaces.
+ * Renders nothing when not `dirty`; otherwise a fragment (no wrapper) so it
+ * composes beside sibling chips in a `SettingsPanel` actions slot or a detail
+ * header bar (e.g. group-detail's Delete, data-retention's Remove override).
+ */
+export function SaveDiscardActions({
+  dirty,
+  saving,
+  onSave,
+  onDiscard,
+  saveDisabled = false,
+  savingLabel = 'Saving...',
+  saveLabel = 'Save',
+}: SaveDiscardActionsProps) {
+  if (!dirty) return null
+  return (
+    <>
+      <Chip onClick={onDiscard} disabled={saving}>
+        Discard
+      </Chip>
+      <Chip variant='primary' onClick={onSave} disabled={saving || saveDisabled}>
+        {saving ? savingLabel : saveLabel}
+      </Chip>
+    </>
+  )
+}

apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx

@@ -501,16 +501,6 @@ export function SecretsManager() {
     })
   }, [])
 
-  useEffect(() => {
-    const handler = (e: BeforeUnloadEvent) => {
-      e.preventDefault()
-    }
-    if (hasChanges) {
-      window.addEventListener('beforeunload', handler)
-    }
-    return () => window.removeEventListener('beforeunload', handler)
-  }, [hasChanges])
-
   /**
    * Navigation guard: intercept link clicks in the capture phase before
    * Next.js App Router processes them. This is needed because Next.js

apps/sim/app/workspace/[workspaceId]/settings/hooks/use-settings-before-unload.ts

@@ -0,0 +1,21 @@
+import { useEffect } from 'react'
+import { useSettingsDirtyStore } from '@/stores/settings/dirty/store'
+
+/**
+ * Registers a single `beforeunload` guard for the whole settings surface,
+ * active only while some section reports unsaved changes via
+ * `useSettingsDirtyStore`. Mounted once in the settings shell so individual
+ * pages never register their own.
+ */
+export function useSettingsBeforeUnload() {
+  const isDirty = useSettingsDirtyStore((s) => s.isDirty)
+
+  useEffect(() => {
+    if (!isDirty) return
+    const handleBeforeUnload = (event: BeforeUnloadEvent) => {
+      event.preventDefault()
+    }
+    window.addEventListener('beforeunload', handleBeforeUnload)
+    return () => window.removeEventListener('beforeunload', handleBeforeUnload)
+  }, [isDirty])
+}

apps/sim/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard.ts

@@ -0,0 +1,59 @@
+import { useCallback, useEffect, useRef, useState } from 'react'
+import { useSettingsDirtyStore } from '@/stores/settings/dirty/store'
+
+interface UseSettingsUnsavedGuardParams {
+  isDirty: boolean
+}
+
+interface SettingsUnsavedGuard {
+  showUnsavedModal: boolean
+  setShowUnsavedModal: (open: boolean) => void
+  /** Run `onLeave` immediately when clean; when dirty, open the confirm modal and defer it. */
+  guardBack: (onLeave: () => void) => void
+  /** Confirmed discard — close the modal and run the deferred leave action. */
+  confirmDiscard: () => void
+}
+
+/**
+ * Wires a settings surface's local dirty state into the shared
+ * `useSettingsDirtyStore`, so the sidebar's section-switch confirmation and the
+ * centralized `beforeunload` both apply without per-page wiring. Also provides
+ * an in-view back/close guard (`guardBack` + the shared `UnsavedChangesModal`)
+ * for detail sub-views whose "back" is an in-component state change rather than
+ * a route navigation.
+ */
+export function useSettingsUnsavedGuard({
+  isDirty,
+}: UseSettingsUnsavedGuardParams): SettingsUnsavedGuard {
+  const setDirty = useSettingsDirtyStore((s) => s.setDirty)
+  const reset = useSettingsDirtyStore((s) => s.reset)
+  const isDirtyRef = useRef(isDirty)
+  const pendingLeaveRef = useRef<(() => void) | null>(null)
+  const [showUnsavedModal, setShowUnsavedModal] = useState(false)
+
+  useEffect(() => {
+    isDirtyRef.current = isDirty
+    setDirty(isDirty)
+  }, [isDirty, setDirty])
+
+  useEffect(() => {
+    return () => reset()
+  }, [reset])
+
+  const guardBack = useCallback((onLeave: () => void) => {
+    if (isDirtyRef.current) {
+      pendingLeaveRef.current = onLeave
+      setShowUnsavedModal(true)
+    } else {
+      onLeave()
+    }
+  }, [])
+
+  const confirmDiscard = useCallback(() => {
+    setShowUnsavedModal(false)
+    pendingLeaveRef.current?.()
+    pendingLeaveRef.current = null
+  }, [])
+
+  return { showUnsavedModal, setShowUnsavedModal, guardBack, confirmDiscard }
+}

apps/sim/ee/access-control/components/group-detail.tsx

@@ -29,12 +29,15 @@ import type { ShareAuthType } from '@/lib/api/contracts/public-shares'
 import { cn } from '@/lib/core/utils/cn'
 import { isBlockTypeAccessControlExempt } from '@/lib/permission-groups/block-access'
 import type { PermissionGroupConfig } from '@/lib/permission-groups/types'
+import { UnsavedChangesModal } from '@/app/workspace/[workspaceId]/components/credential-detail'
 import {
   MemberAvatar,
   MemberRow,
 } from '@/app/workspace/[workspaceId]/settings/components/member-list'
 import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu'
+import { SaveDiscardActions } from '@/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions'
 import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section'
+import { useSettingsUnsavedGuard } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard'
 import { getAllBlocks } from '@/blocks'
 import type { BlockConfig } from '@/blocks/types'
 import { WorkspaceSelect } from '@/ee/access-control/components/workspace-select'
@@ -603,7 +606,6 @@ export function GroupDetail({
   const [addMembersError, setAddMembersError] = useState<string | null>(null)
   const [selectedMemberIds, setSelectedMemberIds] = useState<Set<string>>(() => new Set())
   const [showDeleteConfirm, setShowDeleteConfirm] = useState(false)
-  const [showUnsavedChanges, setShowUnsavedChanges] = useState(false)
 
   const { data: members = [], isPending: membersLoading } = usePermissionGroupMembers(
     organizationId,
@@ -823,6 +825,7 @@ export function GroupDetail({
   const hasConfigChanges = useMemo(() => {
     return JSON.stringify(viewingGroup.config) !== JSON.stringify(editingConfig)
   }, [viewingGroup.config, editingConfig])
+  const guard = useSettingsUnsavedGuard({ isDirty: hasConfigChanges })
 
   const filteredProviders = useMemo(() => {
     if (!providerSearchTerm.trim()) return allProviderIds
@@ -1096,22 +1099,20 @@ export function GroupDetail({
     }))
   }, [])
 
-  /** Persists the editing buffer. Returns whether the save succeeded so callers can decide whether to navigate away. */
-  const handleSaveConfig = useCallback(async (): Promise<boolean> => {
+  /** Persists the editing buffer. */
+  const handleSaveConfig = useCallback(async () => {
     try {
       await updatePermissionGroup.mutateAsync({
         id: viewingGroup.id,
         organizationId,
         config: editingConfig,
       })
       setViewingGroup((prev) => ({ ...prev, config: editingConfig }))
-      return true
     } catch (error) {
       logger.error('Failed to update config', error)
       toast.error("Couldn't save changes", {
         description: getErrorMessage(error, 'Please try again in a moment.'),
       })
-      return false
     }
   }, [viewingGroup.id, editingConfig, organizationId, updatePermissionGroup])
 
@@ -1120,12 +1121,8 @@ export function GroupDetail({
   }, [viewingGroup.config])
 
   const handleBack = useCallback(() => {
-    if (hasConfigChanges) {
-      setShowUnsavedChanges(true)
-      return
-    }
-    onBack()
-  }, [hasConfigChanges, onBack])
+    guard.guardBack(onBack)
+  }, [guard.guardBack, onBack])
 
   const handleScopeChange = useCallback(
     async (workspaceIds: string[]) => {
@@ -1270,20 +1267,12 @@ export function GroupDetail({
             Access Control
           </Chip>
           <div className='flex items-center gap-1'>
-            {hasConfigChanges && (
-              <>
-                <Chip onClick={handleDiscardConfig} disabled={updatePermissionGroup.isPending}>
-                  Discard
-                </Chip>
-                <Chip
-                  variant='primary'
-                  onClick={handleSaveConfig}
-                  disabled={updatePermissionGroup.isPending}
-                >
-                  {updatePermissionGroup.isPending ? 'Saving...' : 'Save'}
-                </Chip>
-              </>
-            )}
+            <SaveDiscardActions
+              dirty={hasConfigChanges}
+              saving={updatePermissionGroup.isPending}
+              onSave={handleSaveConfig}
+              onDiscard={handleDiscardConfig}
+            />
             <Chip
               variant='destructive'
               onClick={() => setShowDeleteConfirm(true)}
@@ -1689,45 +1678,11 @@ export function GroupDetail({
         }}
       />
 
-      <ChipModal
-        open={showUnsavedChanges}
-        onOpenChange={setShowUnsavedChanges}
-        size='sm'
-        srTitle='Unsaved Changes'
-      >
-        <ChipModalHeader onClose={() => setShowUnsavedChanges(false)}>
-          Unsaved Changes
-        </ChipModalHeader>
-        <ChipModalBody>
-          <p className='px-2 text-[var(--text-secondary)] text-sm'>
-            You have unsaved changes. Do you want to save them before leaving?
-          </p>
-        </ChipModalBody>
-        <ChipModalFooter
-          onCancel={() => setShowUnsavedChanges(false)}
-          secondaryActions={[
-            {
-              label: 'Discard Changes',
-              onClick: () => {
-                setShowUnsavedChanges(false)
-                onBack()
-              },
-              variant: 'destructive',
-            },
-          ]}
-          primaryAction={{
-            label: updatePermissionGroup.isPending ? 'Saving...' : 'Save Changes',
-            onClick: async () => {
-              const saved = await handleSaveConfig()
-              if (saved) {
-                setShowUnsavedChanges(false)
-                onBack()
-              }
-            },
-            disabled: updatePermissionGroup.isPending,
-          }}
-        />
-      </ChipModal>
+      <UnsavedChangesModal
+        open={guard.showUnsavedModal}
+        onOpenChange={guard.setShowUnsavedModal}
+        onDiscard={guard.confirmDiscard}
+      />
     </>
   )
 }