improvement(settings): reduce navigation and section load latency

Author: waleedlatif1

.agents/skills/react-query-best-practices/SKILL.md

@@ -35,6 +35,9 @@ Read these before analyzing:
 - Every query must have an explicit `staleTime` (default 0 is almost never correct), assigned from a named exported constant — never an inline numeric literal. A server-side prefetch hydrating the same query key must import and reuse that constant instead of restating the number
 - `keepPreviousData` / `placeholderData` only on variable-key queries (where params change), never on static keys
 - Use `enabled` to prevent queries from running without required params
+- Compose caller-controlled `enabled` options with required-param guards (`Boolean(id) && (options?.enabled ?? true)`). Never spread options after an internal guard, because `{ enabled: true }` can silently re-enable an invalid request.
+- A disabled query can still report `isPending: true`. Aggregate loading state only for queries that are applicable/enabled, or an optional query can hold the whole surface in a permanent loading state.
+- Deferred authorization or policy queries must fail closed. Do not give pending/error data the same fallback as a successfully loaded unrestricted policy; disable guarded actions until the policy query succeeds.
 
 ### Mutations
 - Use `onSettled` (not `onSuccess`) for cache reconciliation — it fires on both success and error
@@ -46,7 +49,7 @@ Read these before analyzing:
 - Never copy query data into useState. Use query data directly in components.
 - Never copy query data into Zustand stores (exception: mutation callbacks that coordinate cross-store state like temp ID replacement)
 - The query cache is not a local state manager — `setQueryData` is for optimistic updates only
-- Forms are the one deliberate exception: copy server data into local form state with `staleTime: Infinity`
+- Forms are the one deliberate exception: once query data exists, initialize a keyed form subtree from it with lazy state initializers. Do not synchronize query data into draft state with an Effect; key the form by resource identity so switching resources resets every draft/modal/upload field together. Keep independent queries in the outer wrapper so they still start in parallel.
 
 ## Steps
 

.agents/skills/you-might-not-need-an-effect/SKILL.md

@@ -16,3 +16,7 @@ Steps:
 1. Read https://react.dev/learn/you-might-not-need-an-effect to understand the guidelines
 2. Analyze the specified scope for useEffect anti-patterns
 3. If fix=true, apply the fixes. If fix=false, propose the fixes without applying.
+
+## Query-backed forms
+
+When query data supplies the initial values for an editable form, do not copy it into draft state in an Effect. Render loading chrome in an outer component, then mount a keyed form child once data exists and initialize its state lazily from props. Key by the resource identity so every related draft, dialog, and upload state resets together when the resource changes. Keep independent queries in the outer component to preserve parallel fetching.

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

@@ -40,6 +40,9 @@ vi.mock('@/components/settings/navigation', () => ({
   getOrganizationSettingsFeatures: vi.fn(() => ({})),
   isOrganizationSettingsSectionAvailable: mockIsOrganizationSettingsSectionAvailable,
   resolveWorkspaceNavigation: mockResolveWorkspaceNavigation,
+  workspaceSectionUsesPermissionConfig: vi.fn((section: string) =>
+    ['secrets', 'api-keys', 'inbox', 'mcp', 'custom-tools'].includes(section)
+  ),
 }))
 
 vi.mock('@/lib/auth', () => ({
@@ -84,7 +87,7 @@ const { mockGetQueryClient, mockPrefetchGeneralSettings } = vi.hoisted(() => ({
 }))
 
 const { mockSections, mockAliases } = vi.hoisted(() => ({
-  mockSections: ['general', 'billing', 'secrets', 'sessions', 'admin'],
+  mockSections: ['general', 'billing', 'secrets', 'sessions', 'admin', 'teammates'],
   /** Mirrors the real alias table so a legacy segment behaves here as it does in production. */
   mockAliases: {
     subscription: 'billing',
@@ -136,6 +139,21 @@ const PERSONAL_HOST_CONTEXT = {
   },
 }
 
+const ORGANIZATION_HOST_CONTEXT = {
+  workspace: {
+    id: 'workspace-b',
+    billedAccountUserId: 'owner-b',
+  },
+  hostOrganizationId: 'organization-b',
+  ownerBilling: {
+    isEnterprise: true,
+  },
+  viewer: {
+    permission: 'admin',
+    isHostOrganizationAdmin: true,
+  },
+}
+
 function pageProps(section: string) {
   return {
     params: Promise.resolve({ workspaceId: 'workspace-b', section }),
@@ -194,6 +212,43 @@ describe('WorkspaceSettingsSectionPage unavailable sections', () => {
     expect(mockPrefetchGeneralSettings).not.toHaveBeenCalled()
   })
 
+  it('resolves a permission group only when its config can hide the requested section', async () => {
+    mockGetWorkspaceHostContext.mockResolvedValue(ORGANIZATION_HOST_CONTEXT)
+    mockResolveWorkspaceNavigation.mockReturnValue([{ id: 'teammates' }])
+
+    await WorkspaceSettingsSectionPage(pageProps('teammates'))
+
+    expect(mockResolveWorkspaceGroup).not.toHaveBeenCalled()
+
+    mockResolveWorkspaceNavigation.mockReturnValue([{ id: 'secrets' }])
+    await WorkspaceSettingsSectionPage(pageProps('secrets'))
+
+    expect(mockResolveWorkspaceGroup).toHaveBeenCalledTimes(1)
+    expect(mockResolveWorkspaceGroup).toHaveBeenCalledWith(
+      'viewer-a',
+      'organization-b',
+      'workspace-b'
+    )
+  })
+
+  it('overlaps general-settings hydration with the organization section gate', async () => {
+    let resolveCanOpenSection: ((value: boolean) => void) | undefined
+    mockGetWorkspaceHostContext.mockResolvedValue(ORGANIZATION_HOST_CONTEXT)
+    mockCanOpenOrganizationSettingsSection.mockReturnValue(
+      new Promise<boolean>((resolve) => {
+        resolveCanOpenSection = resolve
+      })
+    )
+
+    const render = WorkspaceSettingsSectionPage(pageProps('billing'))
+    await vi.waitFor(() => expect(mockCanOpenOrganizationSettingsSection).toHaveBeenCalledTimes(1))
+
+    expect(mockPrefetchGeneralSettings).toHaveBeenCalledWith(expect.any(QueryClient), 'viewer-a')
+
+    resolveCanOpenSection?.(true)
+    await render
+  })
+
   it('gates the hydration on the resolved section, not the raw segment', async () => {
     // `/settings/subscription` is a legacy link for billing, which does read the key. Billing on
     // a personal workspace is only reachable by the billed account owner.
@@ -210,5 +265,6 @@ describe('WorkspaceSettingsSectionPage unavailable sections', () => {
     await expect(WorkspaceSettingsSectionPage(pageProps('general'))).rejects.toThrow(
       'NEXT_NOT_FOUND'
     )
+    expect(mockPrefetchGeneralSettings).not.toHaveBeenCalled()
   })
 })

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

@@ -8,6 +8,7 @@ import {
   type OrganizationSettingsSection,
   resolveWorkspaceNavigation,
   type WorkspaceSettingsSection,
+  workspaceSectionUsesPermissionConfig,
 } from '@/components/settings/navigation'
 import { getSession } from '@/lib/auth'
 import { isOrganizationOnEnterprisePlan } from '@/lib/billing'
@@ -113,6 +114,16 @@ export default async function WorkspaceSettingsSectionPage({
   if (!hostContext) notFound()
   if (requiresPlatformAdmin && !isViewerPlatformAdmin) notFound()
 
+  const queryClient = getQueryClient()
+  /**
+   * Start the viewer-scoped prefetch as soon as workspace access is established. Organization
+   * and section-entitlement gates remain authoritative, but their independent reads no longer
+   * serialize in front of this data. The promise is still awaited before dehydration below.
+   */
+  const generalSettingsPrefetch = GENERAL_SETTINGS_SECTIONS.has(parsed)
+    ? prefetchGeneralSettings(queryClient, session.user.id)
+    : Promise.resolve()
+
   const workspaceSection = WORKSPACE_SECTION_MAP[parsed]
   if (workspaceSection) {
     /**
@@ -130,12 +141,14 @@ export default async function WorkspaceSettingsSectionPage({
      * check it could not act on. Passing `false` elsewhere is safe in the one direction that
      * matters: it can only remove `forks` from a list this gate is not asking about.
      *
-     * `permissionConfig` is deliberately NOT narrowed the same way. Its keys hide sections, so
-     * skipping the lookup for a section that turns out to be config-gated would reveal it —
-     * fail-open, where the others fail closed.
+     * Permission-group config is narrowed by the same policy map that hides navigation items.
+     * Every other section is independent of that config, so resolving the viewer's group for it
+     * can never change this gate's answer.
      */
     const [permissionGroup, forksAvailable] = await Promise.all([
-      hostContext.hostOrganizationId && hostContext.ownerBilling.isEnterprise
+      hostContext.hostOrganizationId &&
+      hostContext.ownerBilling.isEnterprise &&
+      workspaceSectionUsesPermissionConfig(workspaceSection)
         ? resolveWorkspaceGroup(session.user.id, hostContext.hostOrganizationId, workspaceId)
         : null,
       workspaceSection === 'forks'
@@ -206,17 +219,14 @@ export default async function WorkspaceSettingsSectionPage({
     }
   }
 
-  const queryClient = getQueryClient()
   /**
    * Scoped to the sections that actually read the key. The prefetch has to be awaited — an
    * unsettled query is dropped from the dehydrated payload, so firing and forgetting would
    * waterfall anyway — which means running it unconditionally charged the other ~25 sections
    * a blocking round-trip for a cache entry they never touch. The viewer's profile is seeded
    * by the workspace layout under a different key and is not repeated here.
    */
-  if (GENERAL_SETTINGS_SECTIONS.has(parsed)) {
-    await prefetchGeneralSettings(queryClient)
-  }
+  await generalSettingsPrefetch
 
   return (
     <HydrationBoundary state={dehydrate(queryClient)}>

apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.test.ts

@@ -0,0 +1,44 @@
+/**
+ * @vitest-environment node
+ */
+import { QueryClient } from '@tanstack/react-query'
+import { describe, expect, it, vi } from 'vitest'
+
+const { mockGetUserSettings } = vi.hoisted(() => ({
+  mockGetUserSettings: vi.fn(),
+}))
+
+vi.mock('@/lib/users/queries', () => ({
+  getUserSettings: mockGetUserSettings,
+}))
+
+import { prefetchGeneralSettings } from '@/app/workspace/[workspaceId]/settings/[section]/prefetch'
+import { generalSettingsKeys } from '@/hooks/queries/general-settings'
+
+describe('prefetchGeneralSettings', () => {
+  it('uses the authenticated viewer id supplied by the route', async () => {
+    mockGetUserSettings.mockResolvedValue({
+      autoConnect: true,
+      superUserModeEnabled: false,
+      mothershipEnvironment: 'prod',
+      theme: 'system',
+      telemetryEnabled: true,
+      billingUsageNotificationsEnabled: true,
+      errorNotificationsEnabled: true,
+      snapToGridSize: 0,
+      showActionBar: true,
+      autoFocusOnClick: true,
+      copilotAutoAllowedTools: [],
+      timezone: null,
+    })
+    const queryClient = new QueryClient()
+
+    await prefetchGeneralSettings(queryClient, 'viewer-a')
+
+    expect(mockGetUserSettings).toHaveBeenCalledWith('viewer-a')
+    expect(queryClient.getQueryData(generalSettingsKeys.settings())).toMatchObject({
+      theme: 'system',
+      telemetryEnabled: true,
+    })
+  })
+})

apps/sim/app/workspace/[workspaceId]/settings/[section]/prefetch.ts

@@ -1,5 +1,4 @@
 import type { QueryClient } from '@tanstack/react-query'
-import { getSession } from '@/lib/auth'
 import { getUserSettings } from '@/lib/users/queries'
 import {
   GENERAL_SETTINGS_STALE_TIME,
@@ -13,16 +12,19 @@ import {
  * Uses the same query key and mapper as the client `useGeneralSettings` hook, so the
  * hydrated entry is indistinguishable from one a client fetch produced.
  *
- * Callers must `await` this. Only a settled query is dehydrated, so an unawaited prefetch
- * is dropped from the payload entirely and the panel waterfalls on every load as if it had
- * never been prefetched.
+ * The authenticated caller supplies the viewer ID it already resolved. Re-reading the session
+ * inside the query would add another dependency to a prefetch that is deliberately started as
+ * soon as workspace access succeeds.
+ *
+ * Callers must await the returned promise before dehydration. Only a settled query is included
+ * by the current dehydration policy, so dropping the promise would leave the panel to fetch on
+ * the client as if it had never been prefetched.
  */
-export function prefetchGeneralSettings(queryClient: QueryClient) {
+export function prefetchGeneralSettings(queryClient: QueryClient, userId: string) {
   return queryClient.prefetchQuery({
     queryKey: generalSettingsKeys.settings(),
     queryFn: async () => {
-      const session = await getSession()
-      const data = await getUserSettings(session?.user?.id ?? null)
+      const data = await getUserSettings(userId)
       return mapGeneralSettingsResponse(data)
     },
     staleTime: GENERAL_SETTINGS_STALE_TIME,

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

@@ -115,12 +115,10 @@ const Terminal = dynamic(() =>
     (m) => m.Terminal
   )
 )
-const WhitelabelingSettings = dynamic(
-  () =>
-    import('@/ee/whitelabeling/components/whitelabeling-settings').then(
-      (m) => m.WhitelabelingSettings
-    ),
-  { ssr: false }
+const WhitelabelingSettings = dynamic(() =>
+  import('@/ee/whitelabeling/components/whitelabeling-settings').then(
+    (m) => m.WhitelabelingSettings
+  )
 )
 
 interface SettingsPageProps {

apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.test.tsx

@@ -10,6 +10,7 @@ const {
   mockPersonalQuery,
   mockUpdateOrganizationLimit,
   mockUpdateUserLimit,
+  mockUseInvoices,
   mockUseOrganizationBilling,
   mockUseSubscriptionData,
   mockUseUsageLimitData,
@@ -18,6 +19,7 @@ const {
   mockPersonalQuery: { current: null as unknown },
   mockUpdateOrganizationLimit: vi.fn(),
   mockUpdateUserLimit: vi.fn(),
+  mockUseInvoices: vi.fn(),
   mockUseOrganizationBilling: vi.fn(),
   mockUseSubscriptionData: vi.fn(),
   mockUseUsageLimitData: vi.fn(),
@@ -106,7 +108,10 @@ vi.mock('@/hooks/queries/organization', () => ({
 }))
 
 vi.mock('@/hooks/queries/subscription', () => ({
-  useInvoices: () => ({ data: { invoices: [], hasMore: false } }),
+  useInvoices: (...args: unknown[]) => {
+    mockUseInvoices(...args)
+    return { data: { invoices: [], hasMore: false } }
+  },
   useOpenBillingPortal: () => ({ isPending: false, mutate: vi.fn() }),
   useSubscriptionData: (...args: unknown[]) => {
     mockUseSubscriptionData(...args)
@@ -276,6 +281,10 @@ describe('Billing payer scope', () => {
     expect(mockUseSubscriptionData).toHaveBeenCalledWith(
       expect.objectContaining({ enabled: false })
     )
+    expect(mockUseInvoices).toHaveBeenCalledWith({
+      context: 'organization',
+      organizationId: 'org-target',
+    })
     expect(mockUseUsageLimitData).not.toHaveBeenCalled()
     expect(
       container.querySelector('a[href="/workspace/organization-workspace/upgrade"]')?.textContent
@@ -336,6 +345,10 @@ describe('Billing payer scope', () => {
 
     expect(container.textContent).toContain('Personal Free plan')
     expect(container.querySelector('main > p')).toBeNull()
+    expect(mockUseInvoices).toHaveBeenCalledWith({
+      context: 'user',
+      organizationId: undefined,
+    })
   })
 
   it('renders an explicit free organization state without subscription controls', async () => {

apps/sim/app/workspace/[workspaceId]/settings/components/billing/billing.tsx

@@ -199,12 +199,16 @@ export function Billing({
   const isTeamAdmin = isOrgAdminRole(userRole)
   const shouldUseOrganizationBillingContext = isOrganizationScope
 
+  /**
+   * Invoice lookup is safe to start with the payer query: the endpoint returns an empty list
+   * when the payer has no Stripe customer. Waiting to derive `isFree` serialized two independent
+   * requests for every paid account and organization.
+   */
   const { data: invoicesData } = useInvoices({
     context: shouldUseOrganizationBillingContext ? 'organization' : 'user',
     organizationId: shouldUseOrganizationBillingContext
       ? (billingOrganizationId ?? undefined)
       : undefined,
-    enabled: !subscription.isFree,
   })
 
   const planIncludedAmount =

apps/sim/app/workspace/[workspaceId]/settings/components/browser/browser.test.tsx

@@ -449,6 +449,19 @@ describe('Browser settings', () => {
     ])
   })
 
+  it('reserves the real header actions while preferences are loading', async () => {
+    const bridge = createBridge()
+    bridge.settings.getPreferences = vi.fn(() => new Promise(() => {}))
+    mockBridge.current = bridge
+
+    await render()
+
+    const actions = [...container.querySelectorAll<HTMLButtonElement>('header button')]
+    expect(actions.map((button) => button.textContent)).toEqual(['Passwords', 'Clear all'])
+    expect(actions.every((button) => button.disabled)).toBe(true)
+    expect(container.querySelector('section[aria-label="General"]')).toBeNull()
+  })
+
   it('lists each data type as a standard settings row in one section', async () => {
     await render()