diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/layout.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/layout.tsx index 6ab029f7f5b..86e4ff29e5a 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/layout.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/layout.tsx @@ -1,18 +1,58 @@ +import { notFound, redirect } from 'next/navigation' import { SettingsHeaderProvider, SettingsHeaderShell, } from '@/app/workspace/[workspaceId]/settings/components/settings-header/settings-header' +import { resolveSettingsSection } from '@/app/workspace/[workspaceId]/settings/navigation' /** - * Persistent chrome for the settings panel pages. The header bar, title, - * description, scroll region, and centered column live in the shell and stay - * mounted across section navigation — only the body swaps. Scoped to `[section]` - * so detail routes (e.g. `secrets/[credentialId]`) keep their own chrome. + * Sections that were promoted out of settings into their own workspace routes. Kept as + * segment-level rewrites so old links and bookmarks still land somewhere sensible. */ -export default function SettingsSectionLayout({ children }: { children: React.ReactNode }) { +const TOP_LEVEL_REDIRECTS: Readonly string>> = { + integrations: (workspaceId) => `/workspace/${workspaceId}/integrations`, + skills: (workspaceId) => `/workspace/${workspaceId}/skills`, + /** Cookie preferences moved into General. */ + privacy: (workspaceId) => `/workspace/${workspaceId}/settings/general?view=privacy`, +} + +/** + * Persistent chrome for the settings panel pages: the header bar, title, description, scroll + * region and centered column. Scoped to `[section]` so detail routes (e.g. + * `secrets/[credentialId]`) keep their own chrome. + * + * The heading is resolved here rather than pushed up from the section body, so it renders with + * the shell instead of waiting on the body's lazily-loaded chunk. + * + * Whether a segment names a section at all is decided here too, above the sibling + * `loading.tsx`. Inside that Suspense boundary a `notFound()` or `redirect()` can no longer set + * the response status — React replays the boundary on the client and the shell still flushes + * 200 — so a bad or legacy URL loaded directly would answer 200 and redirect in a second round + * trip. Deciding it above the boundary keeps the 404 and the 307. Whether the *viewer* may open + * a section is a different question and stays in the page, where it belongs; those checks need + * the database and are reached almost entirely by client navigation. + * + * Authentication is already enforced by the ancestor workspace layout, so this runs only for a + * signed-in viewer. + */ +export default async function SettingsSectionLayout({ + children, + params, +}: { + children: React.ReactNode + params: Promise<{ workspaceId: string; section: string }> +}) { + const { workspaceId, section } = await params + + const topLevelHref = TOP_LEVEL_REDIRECTS[section]?.(workspaceId) + if (topLevelHref) redirect(topLevelHref) + + const resolved = resolveSettingsSection(section) + if (!resolved) notFound() + return ( - {children} + {children} ) } diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/loading.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/loading.tsx new file mode 100644 index 00000000000..925929408ab --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/loading.tsx @@ -0,0 +1,19 @@ +/** + * Route-transition fallback for the workspace settings sections. + * + * Its job is to exist. Without a loading boundary the App Router holds the outgoing section on + * screen until the incoming page's access gate resolves, so a click reads as a dead click; with + * one, the navigation commits immediately and the heading changes with it. It is also what + * makes the sidebar's `router.prefetch` worth anything — with no loading boundary in the + * subtree the scheduler skips the segment request entirely, and an `AUTO` prefetch caches the + * shell only as far as the nearest boundary. + * + * It renders no body of its own because the shell that owns the header, heading and scroll + * region renders above it and is already resolved by this point. That lands in the same place + * as the two neighbouring settings fallbacks — credit-usage renders its title and description + * over an empty body, `ResourceChromeFallback` renders a real header and column headers over + * zero rows — without restating chrome this route already has. + */ +export default function WorkspaceSettingsSectionLoading() { + return null +} diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx index 9b54c76899d..3b780559b9a 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx @@ -7,8 +7,6 @@ const { mockCanOpenOrganizationSettingsSection, mockGetSession, mockGetWorkspaceHostContext, - mockHasWorkspaceInboxAccess, - mockHasWorkspaceSandboxAccess, mockIsForkingAvailable, mockIsOrganizationOnEnterprisePlan, mockIsOrganizationSettingsSectionAvailable, @@ -20,8 +18,6 @@ const { mockCanOpenOrganizationSettingsSection: vi.fn(), mockGetSession: vi.fn(), mockGetWorkspaceHostContext: vi.fn(), - mockHasWorkspaceInboxAccess: vi.fn(), - mockHasWorkspaceSandboxAccess: vi.fn(), mockIsForkingAvailable: vi.fn(), mockIsOrganizationOnEnterprisePlan: vi.fn(), mockIsOrganizationSettingsSectionAvailable: vi.fn(), @@ -54,11 +50,6 @@ vi.mock('@/lib/billing', () => ({ isOrganizationOnEnterprisePlan: mockIsOrganizationOnEnterprisePlan, })) -vi.mock('@/lib/billing/core/subscription', () => ({ - hasWorkspaceInboxAccess: mockHasWorkspaceInboxAccess, - hasWorkspaceSandboxAccess: mockHasWorkspaceSandboxAccess, -})) - vi.mock('@/lib/core/config/env', () => ({ env: {}, getEnv: vi.fn(), @@ -84,11 +75,30 @@ vi.mock('@/lib/workspaces/host-context', () => ({ })) vi.mock('@/app/_shell/providers/get-query-client', () => ({ - getQueryClient: vi.fn(), + getQueryClient: mockGetQueryClient, +})) + +const { mockGetQueryClient, mockPrefetchGeneralSettings } = vi.hoisted(() => ({ + mockGetQueryClient: vi.fn(), + mockPrefetchGeneralSettings: vi.fn(), +})) + +const { mockSections, mockAliases } = vi.hoisted(() => ({ + mockSections: ['general', 'billing', 'secrets', 'sessions', 'admin'], + /** Mirrors the real alias table so a legacy segment behaves here as it does in production. */ + mockAliases: { + subscription: 'billing', + team: 'organization', + 'api-keys': 'apikeys', + domains: 'sso', + } as Record, })) vi.mock('@/app/workspace/[workspaceId]/settings/navigation', () => ({ - allNavigationItems: [{ id: 'general' }, { id: 'billing' }, { id: 'secrets' }, { id: 'sessions' }], + resolveSettingsSection: vi.fn((section: string) => { + const id = mockAliases[section] ?? section + return mockSections.includes(id) ? { id, meta: { title: id } } : null + }), getSettingsSectionMeta: vi.fn(() => null), })) @@ -101,13 +111,14 @@ vi.mock('@/ee/workspace-forking/lib/lineage/authz', () => ({ })) vi.mock('@/app/workspace/[workspaceId]/settings/[section]/prefetch', () => ({ - prefetchGeneralSettings: vi.fn(), + prefetchGeneralSettings: mockPrefetchGeneralSettings, })) vi.mock('@/app/workspace/[workspaceId]/settings/[section]/settings', () => ({ SettingsPage: vi.fn(() => null), })) +import { QueryClient } from '@tanstack/react-query' import WorkspaceSettingsSectionPage from '@/app/workspace/[workspaceId]/settings/[section]/page' const PERSONAL_HOST_CONTEXT = { @@ -139,11 +150,10 @@ describe('WorkspaceSettingsSectionPage unavailable sections', () => { mockResolveWorkspaceNavigation.mockReturnValue([]) mockResolveWorkspaceGroup.mockResolvedValue(null) mockIsForkingAvailable.mockResolvedValue(false) - mockHasWorkspaceInboxAccess.mockResolvedValue(false) - mockHasWorkspaceSandboxAccess.mockResolvedValue(false) mockCanOpenOrganizationSettingsSection.mockResolvedValue(false) mockIsOrganizationOnEnterprisePlan.mockResolvedValue(false) mockIsOrganizationSettingsSectionAvailable.mockReturnValue(true) + mockGetQueryClient.mockReturnValue(new QueryClient()) }) it('redirects an unavailable subscription section to General', async () => { @@ -171,6 +181,29 @@ describe('WorkspaceSettingsSectionPage unavailable sections', () => { expect(mockGetWorkspaceHostContext).not.toHaveBeenCalled() }) + it('hydrates general settings only for the sections whose body reads them', async () => { + // The saving this gate exists for: the other ~25 sections no longer block on a query they + // never touch. `general` still does, and so does an alias that resolves onto the set. + mockResolveWorkspaceNavigation.mockReturnValue([{ id: 'secrets' }]) + + await WorkspaceSettingsSectionPage(pageProps('general')) + expect(mockPrefetchGeneralSettings).toHaveBeenCalledTimes(1) + + mockPrefetchGeneralSettings.mockClear() + await WorkspaceSettingsSectionPage(pageProps('secrets')) + expect(mockPrefetchGeneralSettings).not.toHaveBeenCalled() + }) + + 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. + mockGetSession.mockResolvedValue({ user: { id: 'owner-b' } }) + + await WorkspaceSettingsSectionPage(pageProps('subscription')) + + expect(mockPrefetchGeneralSettings).toHaveBeenCalledTimes(1) + }) + it('keeps inaccessible workspaces fail-fast', async () => { mockGetWorkspaceHostContext.mockResolvedValue(null) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx index 2d98d12c5d4..3ef8452ee21 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/page.tsx @@ -11,17 +11,14 @@ import { } from '@/components/settings/navigation' import { getSession } from '@/lib/auth' import { isOrganizationOnEnterprisePlan } from '@/lib/billing' -import { hasWorkspaceInboxAccess, hasWorkspaceSandboxAccess } from '@/lib/billing/core/subscription' import { getEnv, isTruthy } from '@/lib/core/config/env' import { isBillingEnabled, isHosted } from '@/lib/core/config/env-flags' -import { isCredentialGroupsAvailable } from '@/lib/credential-groups/availability' import { canOpenOrganizationSettingsSection } from '@/lib/organizations/settings-access' import { isPlatformAdmin } from '@/lib/permissions/super-user' import { getWorkspaceHostContextForViewer } from '@/lib/workspaces/host-context' import { getQueryClient } from '@/app/_shell/providers/get-query-client' import { - allNavigationItems, - getSettingsSectionMeta, + resolveSettingsSection, type SettingsSection, } from '@/app/workspace/[workspaceId]/settings/navigation' import { resolveWorkspaceGroup } from '@/ee/access-control/utils/permission-check' @@ -33,21 +30,6 @@ interface WorkspaceSettingsSectionPageProps { params: Promise<{ workspaceId: string; section: string }> } -const SECTION_ALIASES: Readonly> = { - subscription: 'billing', - team: 'organization', - 'api-keys': 'apikeys', - // Verified domains moved into the SSO page; keep old links working. - domains: 'sso', -} - -const TOP_LEVEL_REDIRECTS: Readonly string>> = { - integrations: (workspaceId) => `/workspace/${workspaceId}/integrations`, - skills: (workspaceId) => `/workspace/${workspaceId}/skills`, - // Cookie preferences moved into General; keep old links working. - privacy: (workspaceId) => `/workspace/${workspaceId}/settings/general?view=privacy`, -} - const WORKSPACE_SECTION_MAP: Partial> = { teammates: 'teammates', secrets: 'secrets', @@ -77,12 +59,19 @@ const ORGANIZATION_SECTION_MAP: Partial item.id === normalized) - ? (normalized as SettingsSection) - : null -} +/** + * Sections whose first paint reads the general-settings query. + * + * Their bodies default a missing value (`?? true` / `?? false`) and drive a switch off it, so + * without a hydrated entry they paint the fallback and visibly flip when the client fetch + * lands. The workspace layout's `SettingsLoader` warms this key only after hydration, which + * covers client navigation but not a direct load of one of these sections. + */ +const GENERAL_SETTINGS_SECTIONS: ReadonlySet = new Set([ + 'general', + 'billing', + 'admin', +]) /** * Settings availability varies across workspaces, so a preserved section may @@ -96,9 +85,7 @@ export async function generateMetadata({ params, }: WorkspaceSettingsSectionPageProps): Promise { const { section } = await params - const parsed = parseSection(section) - const meta = parsed ? getSettingsSectionMeta(parsed) : null - return { title: meta?.label ?? 'Settings' } + return { title: resolveSettingsSection(section)?.meta.title ?? 'Settings' } } export default async function WorkspaceSettingsSectionPage({ @@ -108,30 +95,53 @@ export default async function WorkspaceSettingsSectionPage({ if (!session?.user) redirect('/login') const { workspaceId, section } = await params - const topLevelHref = TOP_LEVEL_REDIRECTS[section]?.(workspaceId) - if (topLevelHref) redirect(topLevelHref) - const parsed = parseSection(section) - if (!parsed) notFound() + /** The layout already rejected an unknown segment; this narrows the type and fails safe. */ + const resolved = resolveSettingsSection(section) + if (!resolved) notFound() + const parsed = resolved.id - const hostContext = await getWorkspaceHostContextForViewer(workspaceId, session.user.id) + /** + * Independent given the session, and both gate the same render, so they overlap rather than + * queue. Every await here sits in front of the section's body, so it is the length of this + * chain that the user waits out. + */ + const requiresPlatformAdmin = parsed === 'admin' || parsed === 'mothership' + const [hostContext, isViewerPlatformAdmin] = await Promise.all([ + getWorkspaceHostContextForViewer(workspaceId, session.user.id), + requiresPlatformAdmin ? isPlatformAdmin(session.user.id) : Promise.resolve(false), + ]) if (!hostContext) notFound() - - if (parsed === 'admin' || parsed === 'mothership') { - if (!(await isPlatformAdmin(session.user.id))) notFound() - } + if (requiresPlatformAdmin && !isViewerPlatformAdmin) notFound() const workspaceSection = WORKSPACE_SECTION_MAP[parsed] if (workspaceSection) { - const [permissionGroup, forksAvailable, inboxAvailable, sandboxes, credentialGroupsAvailable] = - await Promise.all([ - hostContext.hostOrganizationId && hostContext.ownerBilling.isEnterprise - ? resolveWorkspaceGroup(session.user.id, hostContext.hostOrganizationId, workspaceId) - : null, - isForkingAvailableForWorkspace(hostContext.hostOrganizationId, session.user.id), - hasWorkspaceInboxAccess(workspaceId), - hasWorkspaceSandboxAccess(workspaceId), - isCredentialGroupsAvailable(hostContext.ownerBilling), - ]) + /** + * The gate asks one question — is this section in the viewer's navigation — so it resolves + * only the entitlements that can answer it, and only for the section being opened. + * + * `credentialGroups` is already on the host context, derived from the same owner billing one + * await earlier, so asking again is a second feature-flag lookup for an answer in hand. + * + * `inbox` and `sandboxes` feed only `locked`, which marks a section as needing an upgrade + * rather than hiding it. This gate reads membership alone, so their two billing round-trips + * could not change the outcome for any section. + * + * `forks` is read only by the `forks` entry, so every other section resolved a lineage + * 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. + */ + const [permissionGroup, forksAvailable] = await Promise.all([ + hostContext.hostOrganizationId && hostContext.ownerBilling.isEnterprise + ? resolveWorkspaceGroup(session.user.id, hostContext.hostOrganizationId, workspaceId) + : null, + workspaceSection === 'forks' + ? isForkingAvailableForWorkspace(hostContext.hostOrganizationId, session.user.id) + : Promise.resolve(false), + ]) const customBlocksAvailable = isHosted ? hostContext.ownerBilling.isEnterprise : isTruthy(getEnv('NEXT_PUBLIC_CUSTOM_BLOCKS_ENABLED')) @@ -140,11 +150,11 @@ export default async function WorkspaceSettingsSectionPage({ permissionConfig: permissionGroup?.config ?? {}, entitlements: { byok: isHosted, - credentialGroups: credentialGroupsAvailable, - inbox: inboxAvailable, + credentialGroups: hostContext.features?.credentialGroups ?? false, + inbox: true, customBlocks: customBlocksAvailable, forks: forksAvailable, - sandboxes, + sandboxes: true, }, }) if (!navigation.some((item) => item.id === workspaceSection)) { @@ -165,23 +175,30 @@ export default async function WorkspaceSettingsSectionPage({ if (!hostContext.viewer.isHostOrganizationAdmin) { redirectToGeneralSettings(workspaceId) } - if ( - !(await canOpenOrganizationSettingsSection( + /** + * Overlapped for the same reason: neither reads the other's result. The plan lookup is + * skipped for the two sections that do not gate on it, so the only case that pays for a + * lookup it does not use is one that was about to redirect anyway. + */ + const needsEnterprisePlan = + organizationSection !== 'members' && organizationSection !== 'billing' + const [canOpenSection, isEnterpriseOrganization] = await Promise.all([ + canOpenOrganizationSettingsSection( hostContext.hostOrganizationId, session.user.id, organizationSection - )) - ) { + ), + needsEnterprisePlan + ? isOrganizationOnEnterprisePlan(hostContext.hostOrganizationId) + : Promise.resolve(false), + ]) + if (!canOpenSection) { redirectToGeneralSettings(workspaceId) } - const hasEnterprisePlan = - organizationSection !== 'members' && - organizationSection !== 'billing' && - (await isOrganizationOnEnterprisePlan(hostContext.hostOrganizationId)) if ( !isOrganizationSettingsSectionAvailable( organizationSection, - getOrganizationSettingsFeatures(hasEnterprisePlan) + getOrganizationSettingsFeatures(needsEnterprisePlan && isEnterpriseOrganization) ) ) { redirectToGeneralSettings(workspaceId) @@ -191,12 +208,15 @@ export default async function WorkspaceSettingsSectionPage({ const queryClient = getQueryClient() /** - * Awaited, not fired and forgotten: only a settled query is dehydrated, so an unawaited - * prefetch is dropped from the payload and the panel waterfalls anyway. The viewer's - * profile is already seeded by the workspace layout under the same key, so it is not - * repeated here. + * 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. */ - await prefetchGeneralSettings(queryClient) + if (GENERAL_SETTINGS_SECTIONS.has(parsed)) { + await prefetchGeneralSettings(queryClient) + } return ( diff --git a/apps/sim/app/workspace/[workspaceId]/settings/navigation.test.ts b/apps/sim/app/workspace/[workspaceId]/settings/navigation.test.ts index 9cdae3dcb08..b3c64e62c0d 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/navigation.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/settings/navigation.test.ts @@ -5,6 +5,7 @@ import { } from '@/components/settings/navigation' import { allNavigationItems, + resolveSettingsSection, sectionConfig, } from '@/app/workspace/[workspaceId]/settings/navigation' @@ -114,3 +115,40 @@ describe('unified settings navigation', () => { expect(workspaceForks?.docsLink).toBe(unifiedForks?.docsLink) }) }) + +describe('resolveSettingsSection', () => { + const LEGACY_SEGMENTS = { + subscription: 'billing', + team: 'organization', + 'api-keys': 'apikeys', + domains: 'sso', + } as const + + it('keeps legacy section links working', () => { + for (const [segment, id] of Object.entries(LEGACY_SEGMENTS)) { + expect(resolveSettingsSection(segment)?.id).toBe(id) + } + }) + + it('never shadows a real section with an alias', () => { + // The day someone adds a section whose id collides with an alias key, that section becomes + // unreachable — the alias would rewrite the segment before the catalog is consulted. + for (const segment of Object.keys(LEGACY_SEGMENTS)) { + expect(allNavigationItems.some((item) => item.id === segment)).toBe(false) + } + }) + + it('resolves a canonical segment to itself and an unknown one to null', () => { + expect(resolveSettingsSection('secrets')?.id).toBe('secrets') + expect(resolveSettingsSection('unknown')).toBeNull() + expect(resolveSettingsSection('')).toBeNull() + }) + + it('carries the catalog label through as the header title', () => { + // `billing` is the case where id and label visibly differ, and the title feeds both the + // shell heading and the document title via generateMetadata. + const billing = allNavigationItems.find((item) => item.id === 'billing') + expect(resolveSettingsSection('subscription')?.meta.title).toBe(billing?.label) + expect(billing?.label).not.toBe('billing') + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/navigation.ts b/apps/sim/app/workspace/[workspaceId]/settings/navigation.ts index d659a983c0b..57de4bd4a04 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/navigation.ts +++ b/apps/sim/app/workspace/[workspaceId]/settings/navigation.ts @@ -1,10 +1,12 @@ import { buildUnifiedSettingsNavigation, SETTINGS_NAVIGATION_BILLING_ENABLED, + toSettingsHeaderMeta, type UnifiedNavigationSection, type UnifiedSettingsNavigationItem, type UnifiedSettingsSection, } from '@/components/settings/navigation' +import type { SettingsHeaderMeta } from '@/components/settings/settings-header' export type SettingsSection = UnifiedSettingsSection @@ -23,6 +25,44 @@ export const sectionConfig: { key: NavigationSection; title: string }[] = [ export const allNavigationItems: NavigationItem[] = buildUnifiedSettingsNavigation() +/** + * Catalog entries indexed by id. Every routed navigation resolves a section, so the + * lookup runs on each settings request rather than only on the ones that render a list. + * Built first-wins to match the `find` it replaces. + */ +const navigationItemsById = new Map() +for (const item of allNavigationItems) { + if (!navigationItemsById.has(item.id)) navigationItemsById.set(item.id, item) +} + +/** + * Section segments that are no longer canonical but must keep resolving, so old links + * and bookmarks survive. Kept beside the catalog because the route layout, the page's + * access gate, and `generateMetadata` all have to normalize a segment identically. + */ +const SECTION_ALIASES: Readonly> = { + subscription: 'billing', + team: 'organization', + 'api-keys': 'apikeys', + /** Verified domains moved into the SSO page. */ + domains: 'sso', +} + +export interface ResolvedSettingsSection { + id: SettingsSection + meta: SettingsHeaderMeta +} + +/** + * Normalizes a routed `[section]` segment to its catalog entry, or `null` when the + * segment names no known section. Availability is not considered here — whether the + * viewer may open a section is the page gate's decision, not the route's. + */ +export function resolveSettingsSection(section: string): ResolvedSettingsSection | null { + const item = navigationItemsById.get((SECTION_ALIASES[section] ?? section) as SettingsSection) + return item ? { id: item.id, meta: toSettingsHeaderMeta(item) } : null +} + /** * Title + description for a settings section, the single source of truth used by * `SettingsPanel` to render the page header. Falls back to `null` for sections @@ -31,6 +71,6 @@ export const allNavigationItems: NavigationItem[] = buildUnifiedSettingsNavigati export function getSettingsSectionMeta( section: SettingsSection ): { label: string; description: string; docsLink?: string } | null { - const item = allNavigationItems.find((navItem) => navItem.id === section) + const item = navigationItemsById.get(section) return item ? { label: item.label, description: item.description, docsLink: item.docsLink } : null } diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-sidebar/settings-sidebar.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-sidebar/settings-sidebar.tsx index 3119c40fe2a..66c334073a0 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-sidebar/settings-sidebar.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-sidebar/settings-sidebar.tsx @@ -9,7 +9,7 @@ import { cn, } from '@sim/emcn' import { ChevronLeft } from '@sim/emcn/icons' -import { useQueryClient } from '@tanstack/react-query' +import { type QueryClient, useQueryClient } from '@tanstack/react-query' import { useParams, usePathname, useRouter } from 'next/navigation' import type { DesktopSettingsSurface } from '@/components/settings/navigation' import { ORGANIZATION_PLANE_UNIFIED_SECTIONS } from '@/components/settings/navigation' @@ -38,12 +38,55 @@ import { SidebarTooltip } from '@/app/workspace/[workspaceId]/w/components/sideb import { useSSOProviders } from '@/ee/sso/hooks/sso' import { useForkingAvailable } from '@/ee/workspace-forking/hooks/use-forking-available' import { prefetchWorkspaceCredentials } from '@/hooks/queries/credentials' -import { prefetchGeneralSettings, useGeneralSettings } from '@/hooks/queries/general-settings' +import { useGeneralSettings } from '@/hooks/queries/general-settings' import { useInboxConfig } from '@/hooks/queries/inbox' import { usePermissionConfig } from '@/hooks/use-permission-config' import { useSettingsNavigation } from '@/hooks/use-settings-navigation' import { useSettingsDirtyStore } from '@/stores/settings/dirty/store' +/** + * Sections whose JS chunk is warmed on hover. + * + * Deliberately not all of them, and the reason is the boundary audit rather than bundle weight. + * Each section is already `dynamic()`-imported by the settings panel, so naming it here adds an + * async-chunk reference, not parsed JS — but `check-tool-registry-boundary` counts `import()` + * as a graph edge on purpose, and listing all of them measured +126..+172 modules against six of + * the app's hottest route baselines. Code-splitting this sidebar does not help: measured, it + * moves exactly one module, because the audit follows the dynamic edge either way. + * + * These six predate this map and are already inside those baselines, so warming them is free. + * Widening it means either raising the ratchet on the routes it exists to protect, or teaching + * the audit to track async reach separately from initial-chunk weight. + * + * Every section still gets its route payload warmed — see `handlePrefetch`. + */ +const SECTION_CHUNK_WARMERS: Partial Promise>> = { + general: () => import('@/app/workspace/[workspaceId]/settings/components/general/general'), + secrets: () => import('@/app/workspace/[workspaceId]/settings/components/secrets/secrets'), + billing: () => import('@/app/workspace/[workspaceId]/settings/components/billing/billing'), + desktop: () => import('@/app/workspace/[workspaceId]/settings/components/desktop/desktop'), + browser: () => import('@/app/workspace/[workspaceId]/settings/components/browser/browser'), + terminal: () => import('@/app/workspace/[workspaceId]/settings/components/terminal/terminal'), +} + +/** + * Sections whose first paint waits on a query the sidebar is able to start early. + * + * `general` is absent because a warm cannot help here: this sidebar only renders inside the + * workspace layout, whose `SettingsLoader` holds a live observer on that key with an hour-long + * stale time, so `prefetchQuery` short-circuits on every hover. + * + * The type argument is load-bearing: workspace credentials are cached per type and the secrets + * panel subscribes to `env_workspace`, so warming the unfiltered list writes a different cache + * entry and leaves the panel to fetch cold anyway. + */ +const SECTION_QUERY_WARMERS: Partial< + Record void> +> = { + secrets: (queryClient, workspaceId) => + prefetchWorkspaceCredentials(queryClient, workspaceId, 'env_workspace'), +} + interface SettingsSidebarProps { isCollapsed?: boolean showCollapsedTooltips?: boolean @@ -226,36 +269,19 @@ export function SettingsSidebar({ return 'general' }, [pathname]) - const handlePrefetch = useCallback( - (itemId: string) => { - switch (itemId) { - case 'general': - prefetchGeneralSettings(queryClient) - void import('@/app/workspace/[workspaceId]/settings/components/general/general') - break - case 'secrets': - prefetchWorkspaceCredentials(queryClient, workspaceId) - void import('@/app/workspace/[workspaceId]/settings/components/secrets/secrets') - break - case 'billing': - void import('@/app/workspace/[workspaceId]/settings/components/billing/billing') - break - case 'desktop': - void import('@/app/workspace/[workspaceId]/settings/components/desktop/desktop') - break - case 'browser': - void import('@/app/workspace/[workspaceId]/settings/components/browser/browser') - break - case 'terminal': - void import('@/app/workspace/[workspaceId]/settings/components/terminal/terminal') - break - } - }, - [queryClient, workspaceId] - ) - const { popSettingsReturnUrl, getSettingsHref } = useSettingsNavigation() + const handlePrefetch = (section: SettingsSection) => { + /** + * The route payload first, for every section. It is the slowest hop — the access gate runs + * on the server — and the one a row can never get for free, because Next only auto-prefetches + * `` and these rows are buttons that must run the unsaved-changes guard first. + */ + router.prefetch(getSettingsHref({ section })) + void SECTION_CHUNK_WARMERS[section]?.() + SECTION_QUERY_WARMERS[section]?.(queryClient, workspaceId) + } + const handleBack = useCallback(() => { requestLeave(() => { router.push(popSettingsReturnUrl(`/workspace/${workspaceId}`)) diff --git a/apps/sim/components/settings/navigation.ts b/apps/sim/components/settings/navigation.ts index f70cc4193d1..3cf7ce68b1d 100644 --- a/apps/sim/components/settings/navigation.ts +++ b/apps/sim/components/settings/navigation.ts @@ -27,6 +27,7 @@ import { } from '@sim/emcn/icons' import { type PermissionType, permissionSatisfies } from '@sim/platform-authz/workspace' import { CodeIcon, McpIcon } from '@/components/icons' +import type { SettingsHeaderMeta } from '@/components/settings/settings-header' import { getEnv, isTruthy } from '@/lib/core/config/env' import { isAccessControlEnabled, @@ -1066,6 +1067,19 @@ export function resolveWorkspaceNavigation({ }) } +/** + * Adapts a navigation entry to the header shell's static identity. + * + * The catalog calls it `label` because it names a sidebar row; the shell calls it `title` + * because it renders a heading. One adapter keeps every plane's shell fed from the catalog + * instead of each one restating the mapping. + */ +export function toSettingsHeaderMeta( + item: Pick +): SettingsHeaderMeta { + return { title: item.label, description: item.description, docsLink: item.docsLink } +} + export function getSettingsSectionMeta( plane: SettingsPlane, section: string diff --git a/apps/sim/components/settings/settings-header-shell.test.tsx b/apps/sim/components/settings/settings-header-shell.test.tsx index bca2da37694..40cd2a33bde 100644 --- a/apps/sim/components/settings/settings-header-shell.test.tsx +++ b/apps/sim/components/settings/settings-header-shell.test.tsx @@ -7,12 +7,16 @@ * clicking Delete would invoke Save. These tests pin the pairing at the render * level, which the pure-function tests cannot reach. */ -import { act } from 'react' +import { act, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { saveDiscardActions } from '@/components/settings/save-discard-actions' import type { SettingsAction } from '@/components/settings/settings-header' -import { SettingsHeaderProvider, SettingsHeaderShell } from '@/components/settings/settings-header' +import { + SettingsHeaderProvider, + SettingsHeaderShell, + useSettingsHeader, +} from '@/components/settings/settings-header' import { SettingsPanel } from '@/components/settings/settings-panel' ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true @@ -114,3 +118,88 @@ describe('SettingsHeaderShell action routing', () => { expect(onSave).not.toHaveBeenCalled() }) }) + +const EMPTY_HEADER = {} + +describe('SettingsHeaderShell static meta', () => { + const META = { title: 'Secrets', description: 'Workspace credentials.' } + + function heading(): string | null { + return container.querySelector('h1')?.textContent?.trim() ?? null + } + + /** The header's own description, not any paragraph a body happens to render. */ + function paragraph(): string | null { + return container.querySelector('h1 + p')?.textContent?.trim() ?? null + } + + function renderWithMeta(body: ReactNode) { + act(() => { + root.render( + + {body} + + ) + }) + } + + it('renders the routed section title before any body has registered one', () => { + renderWithMeta(
) + + expect(heading()).toBe('Secrets') + expect(paragraph()).toBe('Workspace credentials.') + }) + + it('yields to a body that registers its own header', () => { + renderWithMeta( + +
+ + ) + + expect(heading()).toBe('Add secret') + expect(paragraph()).toBe('One value.') + }) + + it('keeps the meta description out of a body that registered a title without one', () => { + // Registers through the raw hook, not SettingsPanel: SettingsPanel always emits a + // `description` key (explicitly undefined), which would let a field-merge implementation + // pass this test by overwriting the meta description with undefined. + function TitleOnlyBody() { + useSettingsHeader({ title: 'Add secret' }) + return
+ } + + renderWithMeta() + + expect(heading()).toBe('Add secret') + expect(paragraph()).toBeNull() + }) + + it('lets a body claim the header with nothing in it, suppressing the meta entirely', () => { + // How SettingsUnavailable opts out: it renders its own centred heading, so the routed + // section's catalog title must not caption it. + function OwnHeadingBody() { + useSettingsHeader(EMPTY_HEADER) + return
+ } + + renderWithMeta() + + expect(heading()).toBeNull() + expect(paragraph()).toBeNull() + }) + + it('falls back to the meta title when the body unmounts mid-navigation', () => { + renderWithMeta( + +
+ + ) + expect(heading()).toBe('Add secret') + + renderWithMeta(
) + + expect(heading()).toBe('Secrets') + }) +}) diff --git a/apps/sim/components/settings/settings-header.tsx b/apps/sim/components/settings/settings-header.tsx index 1761f574679..7c1b3ae4c69 100644 --- a/apps/sim/components/settings/settings-header.tsx +++ b/apps/sim/components/settings/settings-header.tsx @@ -66,17 +66,31 @@ export interface SettingsHeaderConfig { scrollContainerRef?: Ref } +/** + * A section's static header identity, derivable from its navigation entry alone. + * + * The shell renders this until the section body mounts and registers a live config, so the + * heading paints in the route's first frame instead of arriving a chunk fetch later. + */ +export interface SettingsHeaderMeta { + title: string + description?: string + docsLink?: string +} + const EMPTY_CONFIG: SettingsHeaderConfig = {} -const RegisterContext = createContext<((config: SettingsHeaderConfig) => void) | null>(null) +const RegisterContext = createContext<((config: SettingsHeaderConfig | null) => void) | null>(null) interface ReadContextValue { - configRef: { current: SettingsHeaderConfig } + /** `null` means no body currently owns the header. */ + configRef: { current: SettingsHeaderConfig | null } signature: string } const ReadContext = createContext(null) -function computeSignature(config: SettingsHeaderConfig): string { +function computeSignature(config: SettingsHeaderConfig | null): string { + if (config === null) return 'released' return JSON.stringify({ title: config.title ?? '', description: config.description ?? '', @@ -102,10 +116,10 @@ function computeSignature(config: SettingsHeaderConfig): string { } export function SettingsHeaderProvider({ children }: { children: ReactNode }) { - const configRef = useRef(EMPTY_CONFIG) + const configRef = useRef(null) const [signature, setSignature] = useState('') - const register = useCallback((config: SettingsHeaderConfig) => { + const register = useCallback((config: SettingsHeaderConfig | null) => { configRef.current = config const next = computeSignature(config) setSignature((previous) => (previous === next ? previous : next)) @@ -123,12 +137,19 @@ export function SettingsHeaderProvider({ children }: { children: ReactNode }) { export function useSettingsHeader(config: SettingsHeaderConfig) { const register = useContext(RegisterContext) + /** + * A layout effect, not a passive one, and load-bearing for that reason: on a section swap + * the shell renders before the outgoing body's cleanup runs, so `configRef` still holds the + * previous section's config during that render. Flushing the release synchronously before + * paint is what stops the previous section's title showing for a frame. Downgrading this to + * `useEffect` reintroduces that flicker. + */ useIsomorphicLayoutEffect(() => { register?.(config) }) useIsomorphicLayoutEffect(() => { - return () => register?.(EMPTY_CONFIG) + return () => register?.(null) }, [register]) } @@ -233,10 +254,29 @@ export function orderHeaderActions( .sort((a, b) => rank(a.action) - rank(b.action)) } -export function SettingsHeaderShell({ children }: { children: ReactNode }) { +interface SettingsHeaderShellProps { + /** + * The routed section's static header identity. Owns the heading whenever no section body + * has registered one — on the route's first frame, while a lazily-loaded section chunk is + * still in flight, and across the gap where an outgoing body has already reset the config. + * Without it the heading blanks and re-fills on every section switch. + */ + meta?: SettingsHeaderMeta | null + children: ReactNode +} + +export function SettingsHeaderShell({ meta, children }: SettingsHeaderShellProps) { const read = useContext(ReadContext) const configRef = read?.configRef - const config = configRef?.current ?? EMPTY_CONFIG + const registered = configRef?.current ?? null + /** + * Meta fills in only while no body owns the header. Substituted wholesale rather than + * field-by-field: a body that registers an explicit `title` and no `description` is + * deliberately suppressing the meta description, so the two must never be merged — and a + * body that registers an empty config is deliberately asking for no heading at all, which + * is how a surface that renders its own (`SettingsUnavailable`) opts out. + */ + const config: SettingsHeaderConfig = registered ?? meta ?? EMPTY_CONFIG const { title, description, docsLink, back, actions, search, scrollContainerRef } = config return ( @@ -249,7 +289,7 @@ export function SettingsHeaderShell({ children }: { children: ReactNode }) { )} > {back ? ( - configRef?.current.back?.onSelect()}> + configRef?.current?.back?.onSelect()}> {back.text} ) : ( @@ -265,10 +305,10 @@ export function SettingsHeaderShell({ children }: { children: ReactNode }) { configRef?.current.actions?.[index]?.onSelect()} + onSelect={() => configRef?.current?.actions?.[index]?.onSelect()} onPrefetch={ action.onPrefetch - ? () => configRef?.current.actions?.[index]?.onPrefetch?.() + ? () => configRef?.current?.actions?.[index]?.onPrefetch?.() : undefined } /> @@ -291,7 +331,7 @@ export function SettingsHeaderShell({ children }: { children: ReactNode }) { icon={Search} placeholder={search.placeholder ?? 'Search...'} value={search.value} - onChange={(event) => configRef?.current.search?.onChange(event.target.value)} + onChange={(event) => configRef?.current?.search?.onChange(event.target.value)} disabled={search.disabled} autoComplete='off' className='w-full' diff --git a/apps/sim/hooks/queries/credentials.ts b/apps/sim/hooks/queries/credentials.ts index 6f64d56213f..717977baff9 100644 --- a/apps/sim/hooks/queries/credentials.ts +++ b/apps/sim/hooks/queries/credentials.ts @@ -51,10 +51,14 @@ export type { * Prefetch workspace credentials into a QueryClient cache. * Use on hover to warm data before navigation. */ -export function prefetchWorkspaceCredentials(queryClient: QueryClient, workspaceId: string) { +export function prefetchWorkspaceCredentials( + queryClient: QueryClient, + workspaceId: string, + type?: WorkspaceCredentialType +) { queryClient.prefetchQuery({ - queryKey: workspaceCredentialKeys.list(workspaceId), - queryFn: ({ signal }) => fetchWorkspaceCredentialList(workspaceId, signal), + queryKey: workspaceCredentialKeys.list(workspaceId, type), + queryFn: ({ signal }) => fetchWorkspaceCredentialList(workspaceId, signal, type), staleTime: WORKSPACE_CREDENTIAL_LIST_STALE_TIME, }) } diff --git a/apps/sim/hooks/queries/utils/fetch-workspace-credentials.ts b/apps/sim/hooks/queries/utils/fetch-workspace-credentials.ts index aae9ce81307..2480796dadf 100644 --- a/apps/sim/hooks/queries/utils/fetch-workspace-credentials.ts +++ b/apps/sim/hooks/queries/utils/fetch-workspace-credentials.ts @@ -3,6 +3,7 @@ import { type ContractJsonResponse, listWorkspaceCredentialsContract, type WorkspaceCredential, + type WorkspaceCredentialType, } from '@/lib/api/contracts' export const WORKSPACE_CREDENTIAL_LIST_STALE_TIME = 60 * 1000 @@ -25,10 +26,11 @@ export function requireWorkspaceCredentialListResponse( */ export async function fetchWorkspaceCredentialList( workspaceId: string, - signal?: AbortSignal + signal?: AbortSignal, + type?: WorkspaceCredentialType ): Promise { const data = await requestJson(listWorkspaceCredentialsContract, { - query: { workspaceId }, + query: { workspaceId, type }, signal, }) return requireWorkspaceCredentialListResponse(data)