refactor(web): centralize role-based route guards and fix anti-patterns (#32302)

web/__tests__/apps/app-list-browsing-flow.test.tsx

@@ -390,13 +390,13 @@ describe('App List Browsing Flow', () => {
     })
   })
 
-  // -- Dataset operator redirect --
-  describe('Dataset Operator Redirect', () => {
-    it('should redirect dataset operators to /datasets', () => {
+  // -- Dataset operator behavior --
+  describe('Dataset Operator Behavior', () => {
+    it('should not redirect at list component level for dataset operators', () => {
       mockIsCurrentWorkspaceDatasetOperator = true
       renderList()
 
-      expect(mockRouterReplace).toHaveBeenCalledWith('/datasets')
+      expect(mockRouterReplace).not.toHaveBeenCalled()
     })
   })
 

web/app/(commonLayout)/app/(appDetailLayout)/layout.tsx

@@ -1,10 +1,7 @@
 'use client'
 import type { FC } from 'react'
-import { useRouter } from 'next/navigation'
 import * as React from 'react'
-import { useEffect } from 'react'
 import { useTranslation } from 'react-i18next'
-import { useAppContext } from '@/context/app-context'
 import useDocumentTitle from '@/hooks/use-document-title'
 
 export type IAppDetail = {
@@ -12,16 +9,9 @@ export type IAppDetail = {
 }
 
 const AppDetail: FC<IAppDetail> = ({ children }) => {
-  const router = useRouter()
-  const { isCurrentWorkspaceDatasetOperator } = useAppContext()
   const { t } = useTranslation()
   useDocumentTitle(t('menus.appDetail', { ns: 'common' }))
 
-  useEffect(() => {
-    if (isCurrentWorkspaceDatasetOperator)
-      return router.replace('/datasets')
-  }, [isCurrentWorkspaceDatasetOperator, router])
-
   return (
     <>
       {children}

web/app/(commonLayout)/datasets/layout.spec.tsx

@@ -0,0 +1,108 @@
+import type { ReactNode } from 'react'
+import { render, screen, waitFor } from '@testing-library/react'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import DatasetsLayout from './layout'
+
+const mockReplace = vi.fn()
+const mockUseAppContext = vi.fn()
+
+vi.mock('next/navigation', () => ({
+  useRouter: () => ({
+    replace: mockReplace,
+  }),
+}))
+
+vi.mock('@/context/app-context', () => ({
+  useAppContext: () => mockUseAppContext(),
+}))
+
+vi.mock('@/context/external-api-panel-context', () => ({
+  ExternalApiPanelProvider: ({ children }: { children: ReactNode }) => <>{children}</>,
+}))
+
+vi.mock('@/context/external-knowledge-api-context', () => ({
+  ExternalKnowledgeApiProvider: ({ children }: { children: ReactNode }) => <>{children}</>,
+}))
+
+type AppContextMock = {
+  isCurrentWorkspaceEditor: boolean
+  isCurrentWorkspaceDatasetOperator: boolean
+  isLoadingCurrentWorkspace: boolean
+  currentWorkspace: {
+    id: string
+  }
+}
+
+const baseContext: AppContextMock = {
+  isCurrentWorkspaceEditor: true,
+  isCurrentWorkspaceDatasetOperator: false,
+  isLoadingCurrentWorkspace: false,
+  currentWorkspace: {
+    id: 'workspace-1',
+  },
+}
+
+const setAppContext = (overrides: Partial<AppContextMock> = {}) => {
+  mockUseAppContext.mockReturnValue({
+    ...baseContext,
+    ...overrides,
+  })
+}
+
+describe('DatasetsLayout', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    setAppContext()
+  })
+
+  it('should render loading when workspace is still loading', () => {
+    setAppContext({
+      isLoadingCurrentWorkspace: true,
+      currentWorkspace: { id: '' },
+    })
+
+    render((
+      <DatasetsLayout>
+        <div data-testid="datasets-content">datasets</div>
+      </DatasetsLayout>
+    ))
+
+    expect(screen.getByRole('status')).toBeInTheDocument()
+    expect(screen.queryByTestId('datasets-content')).not.toBeInTheDocument()
+    expect(mockReplace).not.toHaveBeenCalled()
+  })
+
+  it('should redirect non-editor and non-dataset-operator users to /apps', async () => {
+    setAppContext({
+      isCurrentWorkspaceEditor: false,
+      isCurrentWorkspaceDatasetOperator: false,
+    })
+
+    render((
+      <DatasetsLayout>
+        <div data-testid="datasets-content">datasets</div>
+      </DatasetsLayout>
+    ))
+
+    expect(screen.queryByTestId('datasets-content')).not.toBeInTheDocument()
+    await waitFor(() => {
+      expect(mockReplace).toHaveBeenCalledWith('/apps')
+    })
+  })
+
+  it('should render children for dataset operators', () => {
+    setAppContext({
+      isCurrentWorkspaceEditor: false,
+      isCurrentWorkspaceDatasetOperator: true,
+    })
+
+    render((
+      <DatasetsLayout>
+        <div data-testid="datasets-content">datasets</div>
+      </DatasetsLayout>
+    ))
+
+    expect(screen.getByTestId('datasets-content')).toBeInTheDocument()
+    expect(mockReplace).not.toHaveBeenCalled()
+  })
+})

web/app/(commonLayout)/datasets/layout.tsx

@@ -10,16 +10,22 @@ import { ExternalKnowledgeApiProvider } from '@/context/external-knowledge-api-c
 export default function DatasetsLayout({ children }: { children: React.ReactNode }) {
   const { isCurrentWorkspaceEditor, isCurrentWorkspaceDatasetOperator, currentWorkspace, isLoadingCurrentWorkspace } = useAppContext()
   const router = useRouter()
+  const shouldRedirect = !isLoadingCurrentWorkspace
+    && currentWorkspace.id
+    && !(isCurrentWorkspaceEditor || isCurrentWorkspaceDatasetOperator)
 
   useEffect(() => {
-    if (isLoadingCurrentWorkspace || !currentWorkspace.id)
-      return
-    if (!(isCurrentWorkspaceEditor || isCurrentWorkspaceDatasetOperator))
+    if (shouldRedirect)
       router.replace('/apps')
-  }, [isCurrentWorkspaceEditor, isCurrentWorkspaceDatasetOperator, isLoadingCurrentWorkspace, currentWorkspace, router])
+  }, [shouldRedirect, router])
 
-  if (isLoadingCurrentWorkspace || !(isCurrentWorkspaceEditor || isCurrentWorkspaceDatasetOperator))
+  if (isLoadingCurrentWorkspace || !currentWorkspace.id)
     return <Loading type="app" />
+
+  if (shouldRedirect) {
+    return null
+  }
+
   return (
     <ExternalKnowledgeApiProvider>
       <ExternalApiPanelProvider>

web/app/(commonLayout)/layout.tsx

@@ -14,6 +14,7 @@ import { ModalContextProvider } from '@/context/modal-context'
 import { ProviderContextProvider } from '@/context/provider-context'
 import PartnerStack from '../components/billing/partner-stack'
 import Splash from '../components/splash'
+import RoleRouteGuard from './role-route-guard'
 
 const Layout = ({ children }: { children: ReactNode }) => {
   return (
@@ -28,7 +29,9 @@ const Layout = ({ children }: { children: ReactNode }) => {
                 <HeaderWrapper>
                   <Header />
                 </HeaderWrapper>
-                {children}
+                <RoleRouteGuard>
+                  {children}
+                </RoleRouteGuard>
                 <PartnerStack />
                 <ReadmePanel />
                 <GotoAnything />

web/app/(commonLayout)/role-route-guard.spec.tsx

@@ -0,0 +1,109 @@
+import { render, screen, waitFor } from '@testing-library/react'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import RoleRouteGuard from './role-route-guard'
+
+const mockReplace = vi.fn()
+const mockUseAppContext = vi.fn()
+let mockPathname = '/apps'
+
+vi.mock('next/navigation', () => ({
+  usePathname: () => mockPathname,
+  useRouter: () => ({
+    replace: mockReplace,
+  }),
+}))
+
+vi.mock('@/context/app-context', () => ({
+  useAppContext: () => mockUseAppContext(),
+}))
+
+type AppContextMock = {
+  isCurrentWorkspaceDatasetOperator: boolean
+  isLoadingCurrentWorkspace: boolean
+}
+
+const baseContext: AppContextMock = {
+  isCurrentWorkspaceDatasetOperator: false,
+  isLoadingCurrentWorkspace: false,
+}
+
+const setAppContext = (overrides: Partial<AppContextMock> = {}) => {
+  mockUseAppContext.mockReturnValue({
+    ...baseContext,
+    ...overrides,
+  })
+}
+
+describe('RoleRouteGuard', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockPathname = '/apps'
+    setAppContext()
+  })
+
+  it('should render loading while workspace is loading', () => {
+    setAppContext({
+      isLoadingCurrentWorkspace: true,
+    })
+
+    render((
+      <RoleRouteGuard>
+        <div data-testid="guarded-content">content</div>
+      </RoleRouteGuard>
+    ))
+
+    expect(screen.getByRole('status')).toBeInTheDocument()
+    expect(screen.queryByTestId('guarded-content')).not.toBeInTheDocument()
+    expect(mockReplace).not.toHaveBeenCalled()
+  })
+
+  it('should redirect dataset operator on guarded routes', async () => {
+    setAppContext({
+      isCurrentWorkspaceDatasetOperator: true,
+    })
+
+    render((
+      <RoleRouteGuard>
+        <div data-testid="guarded-content">content</div>
+      </RoleRouteGuard>
+    ))
+
+    expect(screen.queryByTestId('guarded-content')).not.toBeInTheDocument()
+    await waitFor(() => {
+      expect(mockReplace).toHaveBeenCalledWith('/datasets')
+    })
+  })
+
+  it('should allow dataset operator on non-guarded routes', () => {
+    mockPathname = '/plugins'
+    setAppContext({
+      isCurrentWorkspaceDatasetOperator: true,
+    })
+
+    render((
+      <RoleRouteGuard>
+        <div data-testid="guarded-content">content</div>
+      </RoleRouteGuard>
+    ))
+
+    expect(screen.getByTestId('guarded-content')).toBeInTheDocument()
+    expect(mockReplace).not.toHaveBeenCalled()
+  })
+
+  it('should not block non-guarded routes while workspace is loading', () => {
+    mockPathname = '/plugins'
+    setAppContext({
+      isLoadingCurrentWorkspace: true,
+    })
+
+    render((
+      <RoleRouteGuard>
+        <div data-testid="guarded-content">content</div>
+      </RoleRouteGuard>
+    ))
+
+    expect(screen.getByTestId('guarded-content')).toBeInTheDocument()
+    expect(screen.queryByRole('status')).not.toBeInTheDocument()
+    expect(mockReplace).not.toHaveBeenCalled()
+  })
+})

web/app/(commonLayout)/role-route-guard.tsx

@@ -0,0 +1,33 @@
+'use client'
+
+import type { ReactNode } from 'react'
+import { usePathname, useRouter } from 'next/navigation'
+import { useEffect } from 'react'
+import Loading from '@/app/components/base/loading'
+import { useAppContext } from '@/context/app-context'
+
+const datasetOperatorRedirectRoutes = ['/apps', '/app', '/explore', '/tools'] as const
+
+const isPathUnderRoute = (pathname: string, route: string) => pathname === route || pathname.startsWith(`${route}/`)
+
+export default function RoleRouteGuard({ children }: { children: ReactNode }) {
+  const { isCurrentWorkspaceDatasetOperator, isLoadingCurrentWorkspace } = useAppContext()
+  const pathname = usePathname()
+  const router = useRouter()
+  const shouldGuardRoute = datasetOperatorRedirectRoutes.some(route => isPathUnderRoute(pathname, route))
+  const shouldRedirect = shouldGuardRoute && !isLoadingCurrentWorkspace && isCurrentWorkspaceDatasetOperator
+
+  useEffect(() => {
+    if (shouldRedirect)
+      router.replace('/datasets')
+  }, [shouldRedirect, router])
+
+  // Block rendering only for guarded routes to avoid permission flicker.
+  if (shouldGuardRoute && isLoadingCurrentWorkspace)
+    return <Loading type="app" />
+
+  if (shouldRedirect)
+    return null
+
+  return <>{children}</>
+}

web/app/(commonLayout)/tools/page.tsx

@@ -1,24 +1,14 @@
 'use client'
 import type { FC } from 'react'
-import { useRouter } from 'next/navigation'
 import * as React from 'react'
-import { useEffect } from 'react'
 import { useTranslation } from 'react-i18next'
 import ToolProviderList from '@/app/components/tools/provider-list'
-import { useAppContext } from '@/context/app-context'
 import useDocumentTitle from '@/hooks/use-document-title'
 
 const ToolsList: FC = () => {
-  const router = useRouter()
-  const { isCurrentWorkspaceDatasetOperator } = useAppContext()
   const { t } = useTranslation()
   useDocumentTitle(t('menus.tools', { ns: 'common' }))
 
-  useEffect(() => {
-    if (isCurrentWorkspaceDatasetOperator)
-      return router.replace('/datasets')
-  }, [isCurrentWorkspaceDatasetOperator, router])
-
   return <ToolProviderList />
 }
 export default React.memo(ToolsList)

web/app/components/apps/__tests__/list.spec.tsx

@@ -368,13 +368,13 @@ describe('List', () => {
     })
   })
 
-  describe('Dataset Operator Redirect', () => {
-    it('should redirect dataset operators to datasets page', () => {
+  describe('Dataset Operator Behavior', () => {
+    it('should not trigger redirect at component level for dataset operators', () => {
       mockIsCurrentWorkspaceDatasetOperator.mockReturnValue(true)
 
       renderList()
 
-      expect(mockReplace).toHaveBeenCalledWith('/datasets')
+      expect(mockReplace).not.toHaveBeenCalled()
     })
   })
 

web/app/components/apps/list.tsx

@@ -1,19 +1,8 @@
 'use client'
 
 import type { FC } from 'react'
-import {
-  RiApps2Line,
-  RiDragDropLine,
-  RiExchange2Line,
-  RiFile4Line,
-  RiMessage3Line,
-  RiRobot3Line,
-} from '@remixicon/react'
 import { useDebounceFn } from 'ahooks'
 import dynamic from 'next/dynamic'
-import {
-  useRouter,
-} from 'next/navigation'
 import { parseAsString, useQueryState } from 'nuqs'
 import { useCallback, useEffect, useRef, useState } from 'react'
 import { useTranslation } from 'react-i18next'
@@ -37,16 +26,6 @@ import useAppsQueryState from './hooks/use-apps-query-state'
 import { useDSLDragDrop } from './hooks/use-dsl-drag-drop'
 import NewAppCard from './new-app-card'
 
-// Define valid tabs at module scope to avoid re-creation on each render and stale closures
-const validTabs = new Set<string | AppModeEnum>([
-  'all',
-  AppModeEnum.WORKFLOW,
-  AppModeEnum.ADVANCED_CHAT,
-  AppModeEnum.CHAT,
-  AppModeEnum.AGENT_CHAT,
-  AppModeEnum.COMPLETION,
-])
-
 const TagManagementModal = dynamic(() => import('@/app/components/base/tag-management'), {
   ssr: false,
 })
@@ -62,7 +41,6 @@ const List: FC<Props> = ({
 }) => {
   const { t } = useTranslation()
   const { systemFeatures } = useGlobalPublicStore()
-  const router = useRouter()
   const { isCurrentWorkspaceEditor, isCurrentWorkspaceDatasetOperator, isLoadingCurrentWorkspace } = useAppContext()
   const showTagManagementModal = useTagStore(s => s.showTagManagementModal)
   const [activeTab, setActiveTab] = useQueryState(
@@ -125,12 +103,12 @@ const List: FC<Props> = ({
 
   const anchorRef = useRef<HTMLDivElement>(null)
   const options = [
-    { value: 'all', text: t('types.all', { ns: 'app' }), icon: <RiApps2Line className="mr-1 h-[14px] w-[14px]" /> },
-    { value: AppModeEnum.WORKFLOW, text: t('types.workflow', { ns: 'app' }), icon: <RiExchange2Line className="mr-1 h-[14px] w-[14px]" /> },
-    { value: AppModeEnum.ADVANCED_CHAT, text: t('types.advanced', { ns: 'app' }), icon: <RiMessage3Line className="mr-1 h-[14px] w-[14px]" /> },
-    { value: AppModeEnum.CHAT, text: t('types.chatbot', { ns: 'app' }), icon: <RiMessage3Line className="mr-1 h-[14px] w-[14px]" /> },
-    { value: AppModeEnum.AGENT_CHAT, text: t('types.agent', { ns: 'app' }), icon: <RiRobot3Line className="mr-1 h-[14px] w-[14px]" /> },
-    { value: AppModeEnum.COMPLETION, text: t('types.completion', { ns: 'app' }), icon: <RiFile4Line className="mr-1 h-[14px] w-[14px]" /> },
+    { value: 'all', text: t('types.all', { ns: 'app' }), icon: <span className="i-ri-apps-2-line mr-1 h-[14px] w-[14px]" /> },
+    { value: AppModeEnum.WORKFLOW, text: t('types.workflow', { ns: 'app' }), icon: <span className="i-ri-exchange-2-line mr-1 h-[14px] w-[14px]" /> },
+    { value: AppModeEnum.ADVANCED_CHAT, text: t('types.advanced', { ns: 'app' }), icon: <span className="i-ri-message-3-line mr-1 h-[14px] w-[14px]" /> },
+    { value: AppModeEnum.CHAT, text: t('types.chatbot', { ns: 'app' }), icon: <span className="i-ri-message-3-line mr-1 h-[14px] w-[14px]" /> },
+    { value: AppModeEnum.AGENT_CHAT, text: t('types.agent', { ns: 'app' }), icon: <span className="i-ri-robot-3-line mr-1 h-[14px] w-[14px]" /> },
+    { value: AppModeEnum.COMPLETION, text: t('types.completion', { ns: 'app' }), icon: <span className="i-ri-file-4-line mr-1 h-[14px] w-[14px]" /> },
   ]
 
   useEffect(() => {
@@ -140,11 +118,6 @@ const List: FC<Props> = ({
     }
   }, [refetch])
 
-  useEffect(() => {
-    if (isCurrentWorkspaceDatasetOperator)
-      return router.replace('/datasets')
-  }, [router, isCurrentWorkspaceDatasetOperator])
-
   useEffect(() => {
     if (isCurrentWorkspaceDatasetOperator)
       return
@@ -272,7 +245,7 @@ const List: FC<Props> = ({
             role="region"
             aria-label={t('newApp.dropDSLToCreateApp', { ns: 'app' })}
           >
-            <RiDragDropLine className="h-4 w-4" />
+            <span className="i-ri-drag-drop-line h-4 w-4" />
             <span className="system-xs-regular">{t('newApp.dropDSLToCreateApp', { ns: 'app' })}</span>
           </div>
         )}

web/app/components/datasets/list/__tests__/index.spec.tsx

@@ -232,7 +232,7 @@ describe('List', () => {
   })
 
   describe('Branch Coverage', () => {
-    it('should redirect normal role users to /apps', async () => {
+    it('should not redirect normal role users at component level', async () => {
       // Re-mock useAppContext with normal role
       vi.doMock('@/context/app-context', () => ({
         useAppContext: () => ({
@@ -249,7 +249,7 @@ describe('List', () => {
       render(<ListComponent />)
 
       await waitFor(() => {
-        expect(mockReplace).toHaveBeenCalledWith('/apps')
+        expect(mockReplace).not.toHaveBeenCalled()
       })
     })
 

web/app/components/datasets/list/index.tsx

@@ -1,9 +1,8 @@
 'use client'
 
 import { useBoolean, useDebounceFn } from 'ahooks'
-import { useRouter } from 'next/navigation'
 // Libraries
-import { useEffect, useState } from 'react'
+import { useState } from 'react'
 import { useTranslation } from 'react-i18next'
 
 import Button from '@/app/components/base/button'
@@ -28,8 +27,7 @@ import Datasets from './datasets'
 const List = () => {
   const { t } = useTranslation()
   const { systemFeatures } = useGlobalPublicStore()
-  const router = useRouter()
-  const { currentWorkspace, isCurrentWorkspaceOwner } = useAppContext()
+  const { isCurrentWorkspaceOwner } = useAppContext()
   const showTagManagementModal = useTagStore(s => s.showTagManagementModal)
   const { showExternalApiPanel, setShowExternalApiPanel } = useExternalApiPanel()
   const [includeAll, { toggle: toggleIncludeAll }] = useBoolean(false)
@@ -54,11 +52,6 @@ const List = () => {
     handleTagsUpdate()
   }
 
-  useEffect(() => {
-    if (currentWorkspace.role === 'normal')
-      return router.replace('/apps')
-  }, [currentWorkspace, router])
-
   const isCurrentWorkspaceManager = useAppContextSelector(state => state.isCurrentWorkspaceManager)
   const { data: apiBaseInfo } = useDatasetApiBaseUrl()
 
@@ -96,7 +89,7 @@ const List = () => {
             onClick={() => setShowExternalApiPanel(true)}
           >
             <ApiConnectionMod className="h-4 w-4 text-components-button-secondary-text" />
-            <div className="system-sm-medium flex items-center justify-center gap-1 px-0.5 text-components-button-secondary-text">{t('externalAPIPanelTitle', { ns: 'dataset' })}</div>
+            <div className="flex items-center justify-center gap-1 px-0.5 text-components-button-secondary-text system-sm-medium">{t('externalAPIPanelTitle', { ns: 'dataset' })}</div>
           </Button>
         </div>
       </div>

web/app/components/explore/__tests__/index.spec.tsx

@@ -63,7 +63,7 @@ describe('Explore', () => {
   })
 
   describe('Effects', () => {
-    it('should redirect dataset operators to /datasets', async () => {
+    it('should not redirect dataset operators at component level', async () => {
       ;(useAppContext as Mock).mockReturnValue({
         isCurrentWorkspaceDatasetOperator: true,
       })
@@ -75,7 +75,7 @@ describe('Explore', () => {
       ))
 
       await waitFor(() => {
-        expect(mockReplace).toHaveBeenCalledWith('/datasets')
+        expect(mockReplace).not.toHaveBeenCalled()
       })
     })
 

web/app/components/explore/index.tsx

@@ -1,23 +1,12 @@
 'use client'
-import { useRouter } from 'next/navigation'
 import * as React from 'react'
-import { useEffect } from 'react'
 import Sidebar from '@/app/components/explore/sidebar'
-import { useAppContext } from '@/context/app-context'
 
 const Explore = ({
   children,
 }: {
   children: React.ReactNode
 }) => {
-  const router = useRouter()
-  const { isCurrentWorkspaceDatasetOperator } = useAppContext()
-
-  useEffect(() => {
-    if (isCurrentWorkspaceDatasetOperator)
-      router.replace('/datasets')
-  }, [isCurrentWorkspaceDatasetOperator, router])
-
   return (
     <div className="flex h-full overflow-hidden border-t border-divider-regular bg-background-body">
       <Sidebar />

web/eslint-suppressions.json

@@ -1241,11 +1241,6 @@
       "count": 1
     }
   },
-  "app/components/apps/list.tsx": {
-    "unused-imports/no-unused-vars": {
-      "count": 1
-    }
-  },
   "app/components/apps/new-app-card.tsx": {
     "ts/no-explicit-any": {
       "count": 1
@@ -3873,11 +3868,6 @@
       "count": 1
     }
   },
-  "app/components/datasets/list/index.tsx": {
-    "tailwindcss/enforce-consistent-class-order": {
-      "count": 1
-    }
-  },
   "app/components/datasets/list/new-dataset-card/option.tsx": {
     "tailwindcss/enforce-consistent-class-order": {
       "count": 1