fix(workspaces): persist visit recency server-side so sidebar order is stable on refresh (#8284)

* fix(workspaces): persist visit recency server-side so sidebar order is stable on refresh

* fix(workspaces): serialize visits and stamp them with the database clock

* chore(workspaces): drop leftover churn and mark the legacy last-active column for contract

* chore(db): drop contract marker on settings.last_active_workspace_id

* chore(workspaces): write the permission-group exemption as TSDoc

Author: waleedlatif1

apps/sim/app/api/workspaces/[id]/visit/route.test.ts

@@ -0,0 +1,71 @@
+/**
+ * @vitest-environment node
+ */
+import { createMockRequest } from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const mocks = vi.hoisted(() => ({
+  getSession: vi.fn(),
+  role: vi.fn(),
+  context: vi.fn(),
+  record: vi.fn(),
+}))
+
+vi.mock('@/lib/auth', () => ({ getSession: mocks.getSession }))
+vi.mock('@sim/platform-authz/workspace', async (importOriginal) => ({
+  ...(await importOriginal<typeof import('@sim/platform-authz/workspace')>()),
+  resolveEffectiveWorkspacePermission: mocks.role,
+}))
+vi.mock('@/lib/workspaces/application/workspace-context', () => ({
+  resolveActiveWorkspaceApplicationContext: mocks.context,
+}))
+vi.mock('@/lib/workspaces/visits', () => ({ recordWorkspaceVisitRecord: mocks.record }))
+
+import { OrchestrationError } from '@/lib/core/orchestration/types'
+import { POST } from '@/app/api/workspaces/[id]/visit/route'
+
+const routeContext = { params: Promise.resolve({ id: 'ws-1' }) }
+
+describe('POST /api/workspaces/[id]/visit', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mocks.getSession.mockResolvedValue({ user: { id: 'user-1' }, session: { id: 'session-1' } })
+    mocks.context.mockImplementation(async (workspaceId: string) => ({
+      workspaceId,
+      workspaceOrganizationId: null,
+      allowPersonalApiKeys: true,
+    }))
+    mocks.role.mockResolvedValue('read')
+    mocks.record.mockResolvedValue(undefined)
+  })
+
+  it('401s without a session and records nothing', async () => {
+    mocks.getSession.mockResolvedValue(null)
+
+    const res = await POST(createMockRequest('POST'), routeContext)
+
+    expect(res.status).toBe(401)
+    expect(mocks.record).not.toHaveBeenCalled()
+  })
+
+  it('records the visit for a workspace member', async () => {
+    const res = await POST(createMockRequest('POST'), routeContext)
+
+    expect(res.status).toBe(200)
+    await expect(res.json()).resolves.toEqual({ success: true })
+    expect(mocks.record).toHaveBeenCalledWith('user-1', 'ws-1')
+  })
+
+  it('answers 404 for a workspace outside the caller reach, same as a missing one', async () => {
+    mocks.role.mockResolvedValue(null)
+    const denied = await POST(createMockRequest('POST'), routeContext)
+
+    mocks.context.mockRejectedValue(new OrchestrationError('not_found', 'Workspace not found'))
+    const missing = await POST(createMockRequest('POST'), routeContext)
+
+    expect(denied.status).toBe(404)
+    expect(missing.status).toBe(404)
+    expect(await denied.json()).toEqual(await missing.json())
+    expect(mocks.record).not.toHaveBeenCalled()
+  })
+})

apps/sim/app/api/workspaces/[id]/visit/route.ts

@@ -0,0 +1,24 @@
+import { recordWorkspaceVisitContract } from '@/lib/api/contracts/workspaces'
+import {
+  defineInternalJsonRoute,
+  internalRateLimits,
+  internalSessionAuth,
+} from '@/lib/api/server/routes'
+import { internalWorkspaceErrorPolicies } from '@/lib/workspaces/api/route-policies'
+import {
+  recordWorkspaceVisit,
+  workspaceVisitOperations,
+} from '@/lib/workspaces/application/record-workspace-visit'
+
+export const POST = defineInternalJsonRoute({
+  contract: recordWorkspaceVisitContract,
+  auth: internalSessionAuth,
+  operation: workspaceVisitOperations.record,
+  rateLimit: internalRateLimits.none({
+    reason: 'One idempotent upsert per workspace page load, replacing the settings write it made.',
+  }),
+  errorPolicy: internalWorkspaceErrorPolicies.concealWorkspaceAuthorization,
+  mapInput: ({ params }) => ({ workspaceId: params.id }),
+  useCase: recordWorkspaceVisit,
+  present: () => ({ success: true as const }),
+})

apps/sim/app/o/[organizationId]/components/organization-sidebar/hooks/use-organization-workspaces.test.tsx

@@ -2,23 +2,23 @@
  * @vitest-environment jsdom
  */
 import { act } from 'react'
-import { createRoot, hydrateRoot, type Root } from 'react-dom/client'
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
+import { hydrateRoot, type Root } from 'react-dom/client'
 import { renderToString } from 'react-dom/server'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
-import { STORAGE_KEYS, WorkspaceRecencyStorage } from '@/lib/core/utils/browser-storage'
 
-const { mockUseWorkspacesQuery, pins } = vi.hoisted(() => ({
-  pins: { current: new Set<string>() },
-  mockUseWorkspacesQuery: vi.fn(),
-}))
-
-vi.mock('@/hooks/queries/workspace', () => ({
-  useWorkspacesQuery: mockUseWorkspacesQuery,
-  EMPTY_PINNED_WORKSPACE_IDS: new Set<string>(),
-  usePinnedWorkspaceIds: () => ({ data: pins.current }),
-}))
+vi.mock('@/lib/api/client/request', () => ({ requestJson: vi.fn() }))
 
 import { useOrganizationWorkspaces } from '@/app/o/[organizationId]/components/organization-sidebar/hooks/use-organization-workspaces'
+import { workspaceKeys } from '@/hooks/queries/workspace'
+
+/** In the order the server returns them: most recently visited first. */
+const workspaces = [
+  { id: 'recent', organizationId: 'org-1' },
+  { id: 'other-org', organizationId: 'org-2' },
+  { id: 'earlier', organizationId: 'org-1' },
+  { id: 'unvisited', organizationId: 'org-1' },
+]
 
 function Harness() {
   const { workspaces } = useOrganizationWorkspaces('org-1')
@@ -31,22 +31,23 @@ function Harness() {
   )
 }
 
+/** A client holding the list exactly as the layout prefetch hydrates it. */
+function seededClient(pinnedWorkspaceIds: string[]) {
+  const queryClient = new QueryClient()
+  queryClient.setQueryData(workspaceKeys.list('active'), {
+    workspaces,
+    lastActiveWorkspaceId: null,
+    pinnedWorkspaceIds,
+    creationPolicy: null,
+  })
+  return queryClient
+}
+
 let container: HTMLDivElement
 let root: Root | undefined
 
 beforeEach(() => {
   vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true)
-  localStorage.clear()
-  pins.current = new Set()
-  mockUseWorkspacesQuery.mockReturnValue({
-    data: [
-      { id: 'newest', organizationId: 'org-1' },
-      { id: 'other-org', organizationId: 'org-2' },
-      { id: 'older', organizationId: 'org-1' },
-      { id: 'oldest', organizationId: 'org-1' },
-    ],
-    isLoading: false,
-  })
   container = document.createElement('div')
   document.body.appendChild(container)
 })
@@ -55,69 +56,51 @@ afterEach(async () => {
   if (root) await act(async () => root?.unmount())
   root = undefined
   container.remove()
-  localStorage.clear()
   vi.unstubAllGlobals()
 })
 
 function workspaceIds() {
   return Array.from(container.querySelectorAll('li'), (item) => item.textContent)
 }
 
-describe('useOrganizationWorkspaces', () => {
-  it('hydrates the prefetched order before applying visit history without changing the query cache', async () => {
-    localStorage.setItem(
-      STORAGE_KEYS.WORKSPACE_RECENCY,
-      JSON.stringify({ oldest: 100, older: 200, 'other-org': 300 })
+/** Server-renders, hydrates, and fails if hydration changed a single DOM node. */
+async function renderAndHydrate(pinnedWorkspaceIds: string[] = []) {
+  container.innerHTML = renderToString(
+    <QueryClientProvider client={seededClient(pinnedWorkspaceIds)}>
+      <Harness />
+    </QueryClientProvider>
+  )
+  const serverOrder = workspaceIds()
+  const onRecoverableError = vi.fn()
+  const mutations: MutationRecord[] = []
+  const observer = new MutationObserver((records) => mutations.push(...records))
+  observer.observe(container, { childList: true, subtree: true, characterData: true })
+  await act(async () => {
+    root = hydrateRoot(
+      container,
+      <QueryClientProvider client={seededClient(pinnedWorkspaceIds)}>
+        <Harness />
+      </QueryClientProvider>,
+      { onRecoverableError }
     )
-    container.innerHTML = renderToString(<Harness />)
-    expect(workspaceIds()).toEqual(['newest', 'older', 'oldest'])
-
-    const onRecoverableError = vi.fn()
-    await act(async () => {
-      root = hydrateRoot(container, <Harness />, { onRecoverableError })
-    })
-
-    expect(onRecoverableError).not.toHaveBeenCalled()
-    expect(workspaceIds()).toEqual(['older', 'oldest', 'newest'])
-    expect(mockUseWorkspacesQuery().data.map(({ id }: { id: string }) => id)).toEqual([
-      'newest',
-      'other-org',
-      'older',
-      'oldest',
-    ])
   })
+  observer.disconnect()
+  expect(onRecoverableError).not.toHaveBeenCalled()
+  expect(mutations).toEqual([])
+  return serverOrder
+}
 
-  it('preserves creation-date order when the browser has no visit history', async () => {
-    await act(async () => {
-      root = createRoot(container)
-      root.render(<Harness />)
-    })
-
-    expect(workspaceIds()).toEqual(['newest', 'older', 'oldest'])
+describe('useOrganizationWorkspaces', () => {
+  it('renders the server visit order so hydration never reshuffles rows', async () => {
+    expect(await renderAndHydrate()).toEqual(['recent', 'earlier', 'unvisited'])
   })
-  it('keeps pins first and reacts to visits without mutating the query cache', async () => {
-    pins.current = new Set(['oldest'])
-    await act(async () => {
-      root = createRoot(container)
-      root.render(<Harness />)
-    })
-    expect(workspaceIds()).toEqual(['oldest', 'newest', 'older'])
-    await act(async () => WorkspaceRecencyStorage.touch('older'))
-    expect(workspaceIds()).toEqual(['oldest', 'older', 'newest'])
-    pins.current = new Set()
-    await act(async () => root?.render(<Harness />))
-    expect(workspaceIds()).toEqual(['older', 'newest', 'oldest'])
+
+  it('lifts pins above the visit order', async () => {
+    expect(await renderAndHydrate(['unvisited'])).toEqual(['unvisited', 'recent', 'earlier'])
   })
 
-  it('follows visit history changed by another tab', async () => {
-    await act(async () => {
-      root = createRoot(container)
-      root.render(<Harness />)
-    })
-    await act(async () => {
-      localStorage.setItem(STORAGE_KEYS.WORKSPACE_RECENCY, JSON.stringify({ oldest: 300 }))
-      window.dispatchEvent(new StorageEvent('storage', { key: STORAGE_KEYS.WORKSPACE_RECENCY }))
-    })
-    expect(workspaceIds()).toEqual(['oldest', 'newest', 'older'])
+  it('does not reorder the cached list', async () => {
+    await renderAndHydrate(['unvisited'])
+    expect(workspaces.map(({ id }) => id)).toEqual(['recent', 'other-org', 'earlier', 'unvisited'])
   })
 })

apps/sim/app/o/[organizationId]/components/organization-sidebar/hooks/use-organization-workspaces.ts

@@ -1,22 +1,18 @@
 import {
   EMPTY_PINNED_WORKSPACE_IDS,
+  useOrderedWorkspacesQuery,
   usePinnedWorkspaceIds,
-  useWorkspacesQuery,
 } from '@/hooks/queries/workspace'
-import { useWorkspaceOrder } from '@/hooks/use-workspace-order'
 
 /**
  * The organization's workspaces the viewer belongs to, for the sidebar's
- * Workspaces section. Read from the viewer's workspace list — the same query the
- * workspace switcher uses — narrowed to those the organization owns.
+ * Workspaces section. Read from the viewer's workspace list — the same query and
+ * order the workspace switcher uses — narrowed to those the organization owns.
  */
 export function useOrganizationWorkspaces(organizationId: string) {
-  const { data = [], isLoading } = useWorkspacesQuery()
+  const { data = [], isLoading } = useOrderedWorkspacesQuery()
   const { data: pinnedWorkspaceIds = EMPTY_PINNED_WORKSPACE_IDS } = usePinnedWorkspaceIds()
-  const orderedWorkspaces = useWorkspaceOrder(data, pinnedWorkspaceIds)
-  const workspaces = orderedWorkspaces.filter(
-    (workspace) => workspace.organizationId === organizationId
-  )
+  const workspaces = data.filter((workspace) => workspace.organizationId === organizationId)
 
   return {
     workspaces,

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-workspace-management.test.tsx

@@ -7,15 +7,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
 const {
   mockPush,
-  mockRequestJson,
+  mockRecordWorkspaceVisit,
   mockSwitchToWorkspace,
-  mockUseWorkspacesQuery,
+  mockUseOrderedWorkspacesQuery,
   mockUseWorkspaceCreationPolicy,
 } = vi.hoisted(() => ({
   mockPush: vi.fn(),
-  mockRequestJson: vi.fn(),
+  mockRecordWorkspaceVisit: vi.fn(),
   mockSwitchToWorkspace: vi.fn(),
-  mockUseWorkspacesQuery: vi.fn(),
+  mockUseOrderedWorkspacesQuery: vi.fn(),
   mockUseWorkspaceCreationPolicy: vi.fn(),
 }))
 
@@ -24,10 +24,6 @@ vi.mock('next/navigation', () => ({
   useRouter: () => ({ push: mockPush }),
 }))
 
-vi.mock('@/lib/api/client/request', () => ({
-  requestJson: mockRequestJson,
-}))
-
 vi.mock('@/hooks/queries/invitations', () => ({
   useLeaveWorkspace: () => ({ isPending: false, mutateAsync: vi.fn() }),
 }))
@@ -37,11 +33,12 @@ vi.mock('@/hooks/queries/workspace', () => ({
   useDeleteWorkspace: () => ({ isPending: false, mutateAsync: vi.fn() }),
   useUpdateWorkspace: () => ({ mutateAsync: vi.fn() }),
   useWorkspaceCreationPolicy: mockUseWorkspaceCreationPolicy,
-  useWorkspacesQuery: mockUseWorkspacesQuery,
+  useOrderedWorkspacesQuery: mockUseOrderedWorkspacesQuery,
   /** No pins: this suite is about the deep-link guard, not switcher ordering. */
   EMPTY_PINNED_WORKSPACE_IDS: new Set<string>(),
   usePinnedWorkspaceIds: () => ({ data: new Set<string>() }),
   useToggleWorkspacePin: () => ({ mutate: vi.fn() }),
+  useRecordWorkspaceVisit: () => ({ mutate: mockRecordWorkspaceVisit }),
 }))
 
 vi.mock('@/stores/workflows/registry/store', () => ({
@@ -97,8 +94,8 @@ describe('resolveWorkspaceSwitchHref', () => {
   })
 })
 
-function Harness() {
-  useWorkspaceManagement({ workspaceId: 'workspace-denied', sessionUserId: 'user-1' })
+function Harness({ sessionUserId = 'user-1' }: { sessionUserId?: string }) {
+  useWorkspaceManagement({ workspaceId: 'workspace-denied', sessionUserId })
   return null
 }
 
@@ -110,8 +107,7 @@ describe('useWorkspaceManagement direct access guard', () => {
     container = document.createElement('div')
     document.body.appendChild(container)
     root = createRoot(container)
-    localStorage.clear()
-    mockUseWorkspacesQuery.mockReturnValue({
+    mockUseOrderedWorkspacesQuery.mockReturnValue({
       data: [
         {
           id: 'workspace-accessible',
@@ -141,4 +137,20 @@ describe('useWorkspaceManagement direct access guard', () => {
 
     expect(mockPush).not.toHaveBeenCalled()
   })
+
+  it('records the visit once for the workspace in the URL', async () => {
+    await act(async () => root.render(<Harness />))
+    await act(async () => root.render(<Harness />))
+
+    expect(mockRecordWorkspaceVisit).toHaveBeenCalledTimes(1)
+    expect(mockRecordWorkspaceVisit).toHaveBeenCalledWith('workspace-denied')
+  })
+
+  it('waits for the session before recording the visit', async () => {
+    await act(async () => root.render(<Harness sessionUserId='' />))
+    expect(mockRecordWorkspaceVisit).not.toHaveBeenCalled()
+
+    await act(async () => root.render(<Harness />))
+    expect(mockRecordWorkspaceVisit).toHaveBeenCalledTimes(1)
+  })
 })