fix(workspaces): serialize same-workspace pin toggles and tolerate replays

Splitting pins into rows removed the lost-update race between *different*
workspaces but not the one on a single row: pin then unpin the same workspace
and the DELETE could overtake its INSERT, delete nothing, and leave the
workspace pinned. A mutation scope serializes them; TanStack runs onMutate
before the scope gate, so the optimistic update is still immediate.

Both duplicate-click replays now resolve to their end state rather than
erroring — a repeat pin answers 409, a repeat unpin 404, and each means the
row is already how the caller wants it. Rollback undoes its own toggle instead
of restoring a snapshot, so a sibling toggle's optimistic state survives.

Also: cap the switcher to the height Radix measured, since six rows can push
the footer actions off a short viewport with nothing able to scroll to them;
drop a dead pinned-item invalidation and a redundant ref; return the pin set
from the hook to match usePinnedIds; and exclude workspace pins from the
unscoped pinned-items listing, where they would read as a resource inside
themselves.

Author: waleedlatif1

apps/sim/app/api/pinned-items/route.ts

@@ -2,7 +2,7 @@ import { db, pinnedItem } from '@sim/db'
 import { createLogger } from '@sim/logger'
 import { getPostgresErrorCode } from '@sim/utils/errors'
 import { generateId } from '@sim/utils/id'
-import { and, eq } from 'drizzle-orm'
+import { and, eq, ne } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
 import {
   createPinnedItemContract,
@@ -59,7 +59,15 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
       and(
         eq(pinnedItem.userId, session.user.id),
         eq(pinnedItem.workspaceId, workspaceId),
-        resourceType ? eq(pinnedItem.resourceType, resourceType) : undefined
+        /**
+         * A `workspace` pin stores `workspaceId === resourceId`, so it would otherwise
+         * appear in this workspace's unscoped listing as a resource *inside* itself.
+         * It is read from the workspace-list payload instead, so it is excluded here
+         * rather than left for a future unscoped caller to mistake for a real resource.
+         */
+        resourceType
+          ? eq(pinnedItem.resourceType, resourceType)
+          : ne(pinnedItem.resourceType, 'workspace')
       )
     )
 

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx

@@ -170,6 +170,11 @@ function WorkspaceHeaderImpl({
   const [isContextMenuOpen, setIsContextMenuOpen] = useState(false)
   const [menuOpenWorkspaceId, setMenuOpenWorkspaceId] = useState<string | null>(null)
   const contextMenuRef = useRef<HTMLDivElement | null>(null)
+  /**
+   * The row a context-menu action targets. Set alongside `menuOpenWorkspaceId` in
+   * {@link openContextMenuAt}, but only that state re-renders — so anything the menu
+   * *renders* must read the state, and only handlers may read this ref.
+   */
   const capturedWorkspaceRef = useRef<Workspace | null>(null)
   /**
    * Set by context-menu actions whose result is only visible in the still-open
@@ -389,10 +394,6 @@ function WorkspaceHeaderImpl({
     }
   }
 
-  /**
-   * Pinning leaves the switcher open: the row jumps to the pinned group, and
-   * closing the menu would hide the only feedback that the action landed.
-   */
   const handleTogglePinAction = () => {
     const target = capturedWorkspaceRef.current
     if (!target) return
@@ -540,7 +541,11 @@ function WorkspaceHeaderImpl({
             align='start'
             side={isCollapsed ? 'right' : 'bottom'}
             sideOffset={isCollapsed ? 16 : 8}
-            className='flex max-h-none flex-col overflow-hidden'
+            /* Overrides the 240px default cap so the six-row list is not clipped, but
+               still bounded by the space Radix measured — at six rows the menu is tall
+               enough that a short viewport would otherwise push the footer actions off
+               screen with nothing able to scroll to them. */
+            className='flex max-h-[var(--radix-dropdown-menu-content-available-height,400px)] flex-col overflow-y-auto'
             style={{
               width: `${SIDEBAR_WIDTH.DEFAULT}px`,
               maxWidth: 'calc(100vw - 24px)',
@@ -743,8 +748,6 @@ function WorkspaceHeaderImpl({
                               {workspace.name}
                             </span>
                             {pinnedWorkspaceIds.has(workspace.id) && (
-                              /* `Pin` hardcodes `aria-hidden` ahead of its prop spread,
-                                 so un-hiding it is what makes the label announce. */
                               <Pin
                                 aria-hidden={false}
                                 role='img'
@@ -887,11 +890,6 @@ function WorkspaceHeaderImpl({
             onTogglePin={handleTogglePinAction}
             onUploadLogo={handleUploadLogoAction}
             showPin={true}
-            /**
-             * Read from state, not `capturedWorkspaceRef` — both are set together in
-             * `openContextMenuAt`, but only the state re-renders, so the ref would
-             * leave the Pin/Unpin label showing the previous row's value.
-             */
             isPinned={Boolean(menuOpenWorkspaceId && pinnedWorkspaceIds.has(menuOpenWorkspaceId))}
             showRename={true}
             showUploadLogo={!!onUploadLogo}

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

@@ -38,7 +38,8 @@ vi.mock('@/hooks/queries/workspace', () => ({
   useWorkspaceCreationPolicy: mockUseWorkspaceCreationPolicy,
   useWorkspacesQuery: mockUseWorkspacesQuery,
   /** No pins: this suite is about the deep-link guard, not switcher ordering. */
-  usePinnedWorkspaceIds: () => ({ data: [] }),
+  EMPTY_PINNED_WORKSPACE_IDS: new Set<string>(),
+  usePinnedWorkspaceIds: () => ({ data: new Set<string>() }),
   useToggleWorkspacePin: () => ({ mutate: vi.fn() }),
 }))
 

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

@@ -6,6 +6,7 @@ import { updateUserSettingsContract } from '@/lib/api/contracts'
 import { WorkspaceRecencyStorage } from '@/lib/core/utils/browser-storage'
 import { useLeaveWorkspace } from '@/hooks/queries/invitations'
 import {
+  EMPTY_PINNED_WORKSPACE_IDS,
   useCreateWorkspace,
   useDeleteWorkspace,
   usePinnedWorkspaceIds,
@@ -47,7 +48,9 @@ export function useWorkspaceManagement({
   const { data: workspaceCreationPolicy = null } = useWorkspaceCreationPolicy(
     Boolean(sessionUserId)
   )
-  const { data: pinnedWorkspaceIdList } = usePinnedWorkspaceIds(Boolean(sessionUserId))
+  const { data: pinnedWorkspaceIds = EMPTY_PINNED_WORKSPACE_IDS } = usePinnedWorkspaceIds(
+    Boolean(sessionUserId)
+  )
   const { mutate: toggleWorkspacePinMutate } = useToggleWorkspacePin()
 
   const leaveWorkspaceMutation = useLeaveWorkspace()
@@ -91,11 +94,6 @@ export function useWorkspaceManagement({
     }, 1000)
   }, [])
 
-  const pinnedWorkspaceIds = useMemo(
-    () => new Set(pinnedWorkspaceIdList ?? []),
-    [pinnedWorkspaceIdList]
-  )
-
   /**
    * Pinned workspaces float to the top, recency ordering them within each group.
    * Matches `resource-sort.ts`: pinning is a user-declared priority layered over
@@ -114,18 +112,11 @@ export function useWorkspaceManagement({
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [workspaces, recencySortKey, pinnedWorkspaceIds])
 
-  const pinnedWorkspaceIdsRef = useRef<Set<string>>(pinnedWorkspaceIds)
-  pinnedWorkspaceIdsRef.current = pinnedWorkspaceIds
-
-  /** Reads the current pins through a ref so the callback identity stays stable for the memoized switcher. */
   const toggleWorkspacePin = useCallback(
     (workspaceId: string) => {
-      toggleWorkspacePinMutate({
-        workspaceId,
-        pinned: !pinnedWorkspaceIdsRef.current.has(workspaceId),
-      })
+      toggleWorkspacePinMutate({ workspaceId, pinned: !pinnedWorkspaceIds.has(workspaceId) })
     },
-    [toggleWorkspacePinMutate]
+    [pinnedWorkspaceIds, toggleWorkspacePinMutate]
   )
 
   const activeWorkspace = useMemo(() => {

apps/sim/hooks/queries/workspace.test.tsx

@@ -78,6 +78,10 @@ function readPins(queryClient: QueryClient): string[] | undefined {
     ?.pinnedWorkspaceIds
 }
 
+function apiError(status: number) {
+  return new ApiClientError({ status, message: `status ${status}`, body: {} })
+}
+
 afterEach(() => {
   act(() => {
     for (const root of mountedRoots.splice(0)) root.unmount()
@@ -121,10 +125,11 @@ describe('useToggleWorkspacePin', () => {
   })
 
   /**
-   * Two rapid toggles write different rows, so neither can overwrite the other and
-   * both survive regardless of the order the requests land in.
+   * Pin then unpin the same workspace race on the same row: an unpin that overtook
+   * its pin would delete nothing and leave the workspace pinned. The mutation scope
+   * must hold the second request until the first resolves.
    */
-  it('keeps both pins when two toggles overlap', async () => {
+  it('serializes toggles of the same workspace so the last one wins', async () => {
     const resolvers: Array<() => void> = []
     mockRequestJson.mockImplementation(
       () => new Promise((resolve) => resolvers.push(() => resolve({ pinnedItem: {} })))
@@ -134,29 +139,44 @@ describe('useToggleWorkspacePin', () => {
 
     act(() => {
       getResult().mutate({ workspaceId: 'ws-a', pinned: true })
-      getResult().mutate({ workspaceId: 'ws-b', pinned: true })
+      getResult().mutate({ workspaceId: 'ws-a', pinned: false })
     })
     await flush()
 
-    expect(readPins(queryClient)).toEqual(['ws-a', 'ws-b'])
+    // The unpin must not be on the wire while the pin is still outstanding.
+    expect(mockRequestJson).toHaveBeenCalledOnce()
+    expect(mockRequestJson.mock.calls[0][0]).toBe(createPinnedItemContract)
+    // Optimistic state already reflects the user's last click.
+    expect(readPins(queryClient)).toEqual([])
+
+    act(() => resolvers[0]())
+    await flush()
+
+    expect(mockRequestJson).toHaveBeenCalledTimes(2)
+    expect(mockRequestJson.mock.calls[1][0]).toBe(deletePinnedItemContract)
+    expect(readPins(queryClient)).toEqual([])
+  })
+
+  /** Both duplicate-click outcomes mean the row is already in the requested end state. */
+  it.each([
+    ['pin', true, 409],
+    ['unpin', false, 404],
+  ])('treats a duplicate %s (%s) as success', async (_label, pinned, status) => {
+    mockRequestJson.mockRejectedValue(apiError(status as number))
+    const { getResult, queryClient } = renderHookWithClient(() => useToggleWorkspacePin())
+    seedList(queryClient, pinned ? [] : ['ws-a'])
 
-    // Resolve out of order — the older request landing last must not undo the newer.
     act(() => {
-      resolvers[1]()
-      resolvers[0]()
+      getResult().mutate({ workspaceId: 'ws-a', pinned: pinned as boolean })
     })
     await flush()
 
-    const bodies = mockRequestJson.mock.calls.map((call) => call[1].body.resourceId)
-    expect(bodies).toEqual(['ws-a', 'ws-b'])
-    expect(readPins(queryClient)).toEqual(['ws-a', 'ws-b'])
+    expect(getResult().isError).toBe(false)
+    expect(readPins(queryClient)).toEqual(pinned ? ['ws-a'] : [])
   })
 
-  /** Re-pinning an already-pinned workspace is the desired end state, not a failure. */
-  it('treats a 409 from a duplicate pin as success', async () => {
-    mockRequestJson.mockRejectedValue(
-      new ApiClientError({ status: 409, message: 'This item is already pinned', body: {} })
-    )
+  it('rolls the optimistic pin back when the write fails', async () => {
+    mockRequestJson.mockRejectedValue(apiError(500))
     const { getResult, queryClient } = renderHookWithClient(() => useToggleWorkspacePin())
     seedList(queryClient, [])
 
@@ -165,22 +185,28 @@ describe('useToggleWorkspacePin', () => {
     })
     await flush()
 
-    expect(getResult().isError).toBe(false)
-    expect(readPins(queryClient)).toEqual(['ws-a'])
+    expect(readPins(queryClient)).toEqual([])
   })
 
-  it('rolls the optimistic pin back when the write fails', async () => {
-    mockRequestJson.mockRejectedValue(
-      new ApiClientError({ status: 500, message: 'boom', body: {} })
+  /**
+   * The rollback undoes its own toggle rather than restoring a snapshot, so a
+   * concurrent toggle's optimistic state survives a sibling's failure.
+   */
+  it('does not drop a concurrent toggle when one fails', async () => {
+    mockRequestJson.mockImplementation((contract: unknown) =>
+      contract === createPinnedItemContract && mockRequestJson.mock.calls.length === 1
+        ? Promise.reject(apiError(500))
+        : Promise.resolve({ pinnedItem: {} })
     )
     const { getResult, queryClient } = renderHookWithClient(() => useToggleWorkspacePin())
     seedList(queryClient, [])
 
     act(() => {
       getResult().mutate({ workspaceId: 'ws-a', pinned: true })
+      getResult().mutate({ workspaceId: 'ws-b', pinned: true })
     })
     await flush()
 
-    expect(readPins(queryClient)).toEqual([])
+    expect(readPins(queryClient)).toEqual(['ws-b'])
   })
 })

apps/sim/hooks/queries/workspace.ts

@@ -20,7 +20,6 @@ import {
   type WorkspaceQueryScope,
   type WorkspacesResponse,
 } from '@/lib/api/contracts'
-import { pinnedItemKeys } from '@/hooks/queries/utils/pinned-item-keys'
 import {
   normalizeWorkspace,
   normalizeWorkspacesResponse,
@@ -101,13 +100,16 @@ export function useWorkspaceCreationPolicy(enabled = true) {
   })
 }
 
-const selectPinnedWorkspaceIds = (data: WorkspacesResponse): string[] => data.pinnedWorkspaceIds
+export const EMPTY_PINNED_WORKSPACE_IDS: ReadonlySet<string> = new Set()
+
+const selectPinnedWorkspaceIds = (data: WorkspacesResponse): ReadonlySet<string> =>
+  data.pinnedWorkspaceIds.length ? new Set(data.pinnedWorkspaceIds) : EMPTY_PINNED_WORKSPACE_IDS
 
 /**
- * The viewer's pinned workspace ids, read off the workspace list the switcher
- * already loads — pins ride along on that payload rather than costing a second
- * request, which is also what lets the server prefetch hydrate them in the same
- * pass and keeps pinned-first ordering from re-sorting after hydration.
+ * The viewer's pinned workspace ids, as a `Set` so a row resolves its pin state in
+ * O(1) — mirroring {@link usePinnedIds} for the workspace-scoped kinds. Sourced
+ * from the workspace list rather than `/api/pinned-items`; see
+ * `pinnedResourceTypeSchema` for why that is the one kind read this way.
  */
 export function usePinnedWorkspaceIds(enabled = true) {
   return useQuery({
@@ -119,58 +121,75 @@ export function usePinnedWorkspaceIds(enabled = true) {
   })
 }
 
+/** Applies one toggle to a pin list. Idempotent, so replaying it cannot double-apply. */
+function applyPinToggle(pinnedWorkspaceIds: string[], workspaceId: string, pinned: boolean) {
+  const without = pinnedWorkspaceIds.filter((id) => id !== workspaceId)
+  return pinned ? [...without, workspaceId] : without
+}
+
 /**
  * Pins or unpins a workspace in the switcher.
  *
- * Writes one `pinned_item` row per pin, so a pin is an insert and an unpin is a
- * delete. Two rapid toggles touch different rows and cannot overwrite each other,
- * which is why nothing here serializes or debounces. Re-pinning an already-pinned
- * workspace answers 409 and unpinning an absent pin is a no-op, so a duplicate
- * click is idempotent rather than an error.
+ * A pin is one `pinned_item` row, so pinning inserts and unpinning deletes. Both
+ * are idempotent against their own end state — a duplicate pin answers 409 and a
+ * duplicate unpin answers 404, and each means the row is already how the caller
+ * wants it, so neither is a failure to roll back.
  *
- * Optimistic against the workspace list, because that is where the pins are read
- * and the toggle re-sorts the row under the user's cursor.
+ * Ordering still matters, because pinning and unpinning the *same* workspace race
+ * on the *same* row: an unpin that overtook its pin would delete nothing and leave
+ * the workspace pinned. `scope` serializes them, and because TanStack runs
+ * `onMutate` before the scope gate, the optimistic update is still immediate.
  */
 export function useToggleWorkspacePin() {
   const queryClient = useQueryClient()
   const queryKey = workspaceKeys.list('active')
 
   return useMutation({
+    scope: { id: 'workspace-pin' },
     mutationFn: async ({ workspaceId, pinned }: { workspaceId: string; pinned: boolean }) => {
-      if (!pinned) {
-        await requestJson(deletePinnedItemContract, {
-          params: { resourceType: 'workspace', resourceId: workspaceId },
-        })
-        return
-      }
       try {
-        await requestJson(createPinnedItemContract, {
-          body: { workspaceId, resourceType: 'workspace', resourceId: workspaceId },
-        })
+        if (pinned) {
+          await requestJson(createPinnedItemContract, {
+            body: { workspaceId, resourceType: 'workspace', resourceId: workspaceId },
+          })
+        } else {
+          await requestJson(deletePinnedItemContract, {
+            params: { resourceType: 'workspace', resourceId: workspaceId },
+          })
+        }
       } catch (error) {
-        /** Already pinned — the desired state, so not a failure to roll back. */
-        if (error instanceof ApiClientError && error.status === 409) return
+        const alreadyInEndState = pinned ? 409 : 404
+        if (error instanceof ApiClientError && error.status === alreadyInEndState) return
         throw error
       }
     },
     onMutate: async ({ workspaceId, pinned }) => {
       await queryClient.cancelQueries({ queryKey })
-      const previous = queryClient.getQueryData<WorkspacesResponse>(queryKey)
-      queryClient.setQueryData<WorkspacesResponse>(queryKey, (old) => {
-        if (!old) return old
-        const next = pinned
-          ? [...old.pinnedWorkspaceIds.filter((id) => id !== workspaceId), workspaceId]
-          : old.pinnedWorkspaceIds.filter((id) => id !== workspaceId)
-        return { ...old, pinnedWorkspaceIds: next }
-      })
-      return { previous }
+      queryClient.setQueryData<WorkspacesResponse>(queryKey, (old) =>
+        old
+          ? {
+              ...old,
+              pinnedWorkspaceIds: applyPinToggle(old.pinnedWorkspaceIds, workspaceId, pinned),
+            }
+          : old
+      )
     },
-    onError: (_error, _variables, context) => {
-      if (context?.previous) queryClient.setQueryData(queryKey, context.previous)
+    /**
+     * Undoes this toggle rather than restoring a snapshot: a snapshot taken before
+     * a concurrent toggle would silently drop that one's optimistic state too.
+     */
+    onError: (_error, { workspaceId, pinned }) => {
+      queryClient.setQueryData<WorkspacesResponse>(queryKey, (old) =>
+        old
+          ? {
+              ...old,
+              pinnedWorkspaceIds: applyPinToggle(old.pinnedWorkspaceIds, workspaceId, !pinned),
+            }
+          : old
+      )
     },
     onSettled: () => {
       queryClient.invalidateQueries({ queryKey: workspaceKeys.lists() })
-      queryClient.invalidateQueries({ queryKey: pinnedItemKeys.all })
     },
   })
 }