fix(resources): dismiss the upload overlay on a folder drop and re-check permission per item

The drag hook stops propagation on a drop it handles, so the page-level handler
that cleared the upload overlay never ran and the chrome stayed up over the
finished upload. Both consuming paths now share one dismissal.

Drop the batch permission memo: each item in a bulk move or delete commits
independently, so reusing one allow verdict let a revocation part-way through a
batch go unseen by the remaining items. The workspace context is still resolved
once per batch, which was the larger saving.

Author: waleedlatif1

apps/sim/app/workspace/[workspaceId]/files/files.tsx

@@ -323,6 +323,18 @@ export function Files() {
   const uploading = uploadProgress.total > 0
   const [isDraggingOver, setIsDraggingOver] = useState(false)
   const dragCounterRef = useRef(0)
+  /**
+   * Takes down the "Drop to upload" overlay.
+   *
+   * Every path that consumes an OS file drag has to call this, including the one that never
+   * reaches the page-level handler: a drop on a folder row is handled by the drag hook, which
+   * stops propagation, so `handleDrop` below never runs and the counter it would have zeroed
+   * keeps the overlay on screen over the finished upload.
+   */
+  const dismissUploadOverlay = useCallback(() => {
+    dragCounterRef.current = 0
+    setIsDraggingOver(false)
+  }, [])
   const [
     { search: urlSearchTerm, type: typeFilter, size: sizeFilter, uploadedBy: uploadedByFilter },
     setFileFilters,
@@ -810,6 +822,7 @@ export function Files() {
     externalDrop: {
       matches: hasExternalFiles,
       onDropIntoFolder: (dataTransfer, targetFolderId) => {
+        dismissUploadOverlay()
         const dropped = Array.from(dataTransfer.files ?? [])
         if (dropped.length > 0) void uploadFiles(dropped, targetFolderId)
       },
@@ -851,8 +864,7 @@ export function Files() {
      * began in — pulling the user out of the folder they just spring-opened to receive it.
      */
     rowDragDropConfig.externalDropHandled()
-    dragCounterRef.current = 0
-    setIsDraggingOver(false)
+    dismissUploadOverlay()
     const dropped = Array.from(e.dataTransfer.files)
     if (dropped.length > 0) await uploadFiles(dropped)
   }

apps/sim/lib/core/application/index.ts

@@ -21,11 +21,9 @@ export type {
   WorkspaceAuthorizationContext,
   WorkspaceAuthorizationOptions,
   WorkspaceDelegationPolicy,
-  WorkspacePermissionCache,
 } from '@/lib/core/application/workspace-authorization'
 export {
   authorizeWorkspaceOperation,
-  createWorkspacePermissionCache,
   DelegatedServiceAuthorizationError,
   DelegatedWorkspaceAuthorizationError,
   InsufficientWorkspacePermissionsError,

apps/sim/lib/core/application/workspace-authorization.ts

@@ -27,65 +27,6 @@ export interface WorkspaceAuthorizationOptions<C extends WorkspaceAuthorizationC
   executor?: Pick<typeof db, 'select'>
   forUpdate?: boolean
   delegation?: WorkspaceDelegationPolicy<C>
-  /**
-   * Memo for the human-permission lookup, supplied by a caller that authorizes many items in one
-   * operation. Ignored alongside `executor` or `forUpdate` — see
-   * {@link createWorkspacePermissionCache}.
-   */
-  permissionCache?: WorkspacePermissionCache
-}
-
-export interface WorkspacePermissionCache {
-  resolve(
-    userId: string,
-    workspaceId: string,
-    workspaceOrganizationId: string | null
-  ): Promise<PermissionType | null>
-}
-
-/**
- * Memoizes the effective-permission lookup across the items of one bulk operation.
- *
- * A batch authorizes every item separately — delegation scope is per-resource, so the check
- * cannot simply be hoisted out of the loop — but the human-permission half of it reads the same
- * `(user, workspace, organization)` triple every time, two queries deep. On a hundred-item
- * request that is two hundred round trips for a value that cannot change within the batch.
- *
- * Caller-owned and request-scoped on purpose: nothing here outlives the operation that created
- * it, so a permission changed between requests is always seen by the next one. Skipped entirely
- * when the caller passes its own `executor` (a transaction has its own snapshot to honour) or
- * `forUpdate` (that lookup takes a row lock, which is a side effect, not a read).
- *
- * Neither of the repo's two existing memo idioms fits. `coalesceLocally` evicts on settle, so a
- * sequential per-item loop would re-query every item. React `cache()` cannot be skipped per call
- * for the `executor`/`forUpdate` paths and has no request scope in the worker runtime. An
- * implicit process-wide memo on an authorization read is a lifetime worth refusing outright.
- */
-export function createWorkspacePermissionCache(): WorkspacePermissionCache {
-  const entries = new Map<string, Promise<PermissionType | null>>()
-  return {
-    resolve(userId, workspaceId, workspaceOrganizationId) {
-      /** Structural, so no id can run into the next and answer another workspace's question. */
-      const key = JSON.stringify([userId, workspaceId, workspaceOrganizationId])
-      const cached = entries.get(key)
-      if (cached) return cached
-      /**
-       * The in-flight promise is what gets stored, so concurrent items share one query rather
-       * than racing to start their own. Evicted if it rejects: a transient database failure must
-       * not become the permanent answer for the rest of the batch.
-       */
-      const pending = resolveEffectiveWorkspacePermission(
-        userId,
-        workspaceId,
-        workspaceOrganizationId
-      ).catch((error) => {
-        entries.delete(key)
-        throw error
-      })
-      entries.set(key, pending)
-      return pending
-    },
-  }
 }
 
 export class InsufficientWorkspacePermissionsError extends ForbiddenOperationError {
@@ -210,16 +151,13 @@ async function requireCurrentHumanPermission<C extends WorkspaceAuthorizationCon
   required: PermissionType,
   options?: WorkspaceAuthorizationOptions<C>
 ): Promise<void> {
-  const memo = options?.executor || options?.forUpdate ? undefined : options?.permissionCache
-  const permission = memo
-    ? await memo.resolve(userId, context.workspaceId, context.workspaceOrganizationId)
-    : await resolveEffectiveWorkspacePermission(
-        userId,
-        context.workspaceId,
-        context.workspaceOrganizationId,
-        options?.executor,
-        { forUpdate: options?.forUpdate }
-      )
+  const permission = await resolveEffectiveWorkspacePermission(
+    userId,
+    context.workspaceId,
+    context.workspaceOrganizationId,
+    options?.executor,
+    { forUpdate: options?.forUpdate }
+  )
   requirePermission(permission, required)
 }
 

apps/sim/lib/core/application/workspace-permission-cache.test.ts

@@ -1,6 +1,6 @@
 import { AuditAction, AuditResourceType } from '@sim/audit'
 import { createLogger } from '@sim/logger'
-import { authorizeWorkspaceOperation, createWorkspacePermissionCache } from '@/lib/core/application'
+import { authorizeWorkspaceOperation } from '@/lib/core/application'
 import { classifyBulkItemError } from '@/lib/core/application/bulk-items'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { PlatformEvents } from '@/lib/core/telemetry'
@@ -12,10 +12,7 @@ import {
   planFolderSelection,
 } from '@/lib/folders/bulk'
 import { findActiveFolder } from '@/lib/folders/queries'
-import {
-  type KnowledgeAuthorizationOptions,
-  knowledgeDelegationPolicy,
-} from '@/lib/knowledge/application/authorization'
+import { knowledgeDelegationPolicy } from '@/lib/knowledge/application/authorization'
 import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case'
 import {
   type BoundedKnowledgeSelection,
@@ -124,21 +121,11 @@ async function runKnowledgeItems(
   knowledgeBaseIds: readonly string[],
   workspace: KnowledgeWorkspaceContext,
   covered: ReadonlySet<string>,
-  authorize: (
-    canonical: ActiveKnowledgeBaseContext,
-    options: KnowledgeAuthorizationOptions
-  ) => Promise<void>,
+  authorize: (canonical: ActiveKnowledgeBaseContext) => Promise<void>,
   apply: (canonical: ActiveKnowledgeBaseContext) => Promise<string>,
   succeeded: BulkKnowledgeItem[],
   outcome: BulkKnowledgeOutcome
 ): Promise<unknown | undefined> {
-  /**
-   * Built here rather than by each caller so a bulk loop cannot forget it: every item authorizes
-   * against the same `(user, workspace, organization)` triple, and without the memo that is two
-   * identical queries per item.
-   */
-  const permissionCache = createWorkspacePermissionCache()
-
   for (const knowledgeBaseId of knowledgeBaseIds) {
     let knowledgeBaseName = knowledgeBaseId
     try {
@@ -153,7 +140,7 @@ async function runKnowledgeItems(
         })
         continue
       }
-      await authorize(canonical, { permissionCache })
+      await authorize(canonical)
       succeeded.push({
         kind: 'knowledgeBase',
         id: canonical.knowledgeBaseId,
@@ -233,9 +220,8 @@ export const bulkMoveKnowledgeItems = defineAuthorizedKnowledgeUseCase({
       context.knowledgeBaseIds,
       context,
       plan.covered,
-      (canonical, options) =>
+      (canonical) =>
         authorizeWorkspaceOperation(principal, knowledgeOperations.bulkMoveItems, canonical, {
-          ...options,
           delegation: knowledgeDelegationPolicy,
         }),
       async (canonical) =>
@@ -336,9 +322,8 @@ export const bulkDeleteKnowledgeItems = defineAuthorizedKnowledgeUseCase({
       context.knowledgeBaseIds,
       context,
       plan.covered,
-      (canonical, options) =>
+      (canonical) =>
         authorizeWorkspaceOperation(principal, knowledgeOperations.bulkDeleteItems, canonical, {
-          ...options,
           delegation: knowledgeDelegationPolicy,
         }),
       async (canonical) => {

apps/sim/lib/knowledge/application/bulk.ts

@@ -2,7 +2,6 @@ import type { Principal } from '@sim/auth/principal'
 import {
   authorizeWorkspaceOperation,
   type WorkspaceAuthorizationContext,
-  type WorkspaceAuthorizationOptions,
   type WorkspaceDelegationPolicy,
 } from '@/lib/core/application'
 import type { TableOperation } from '@/lib/table/application/operations'
@@ -31,21 +30,12 @@ export const tableDelegationPolicy: WorkspaceDelegationPolicy<TableAuthorization
   },
 }
 
-/** Everything {@link authorizeWorkspaceOperation} takes except the delegation policy, which is
- *  fixed for this domain. */
-export type TableAuthorizationOptions = Omit<
-  WorkspaceAuthorizationOptions<TableAuthorizationContext>,
-  'delegation'
->
-
 export function authorizeTableOperation(
   principal: Principal,
   operation: TableOperation,
-  context: TableAuthorizationContext,
-  options?: TableAuthorizationOptions
+  context: TableAuthorizationContext
 ) {
   return authorizeWorkspaceOperation(principal, operation, context, {
-    ...options,
     delegation: tableDelegationPolicy,
   })
 }

apps/sim/lib/table/application/authorization.ts

@@ -292,15 +292,15 @@ describe('table bulk application use cases', () => {
   })
 
   /**
-   * The batch authorizes every table separately — delegation scope is per-resource — but the
-   * human-permission half of that check reads the same row every time. Without the shared memo a
-   * hundred-table request is a hundred identical lookups, two queries deep.
+   * The canonical workspace context is what bounded and authorized the request; it cannot differ
+   * per item, so the batch resolves it once and composes each table onto it. Resolving it per
+   * item was a whole extra load each.
    *
-   * Two calls, not one: the use case authorizes the operation itself before the loop starts, and
-   * that check is outside the batch memo. What matters is that the count does not grow with the
-   * selection.
+   * Note this deliberately does NOT memoize the per-item permission check: each item commits
+   * independently, so every one of them re-reads the caller's current permission and a
+   * revocation part-way through a batch stops the rest.
    */
-  it('resolves the caller permission once for the whole batch, however many items it carries', async () => {
+  it('loads the workspace context once however many items the batch carries', async () => {
     const move = (tableIds: string[]) =>
       bulkMoveTables.execute({
         principal,
@@ -314,14 +314,33 @@ describe('table bulk application use cases', () => {
 
     const small = await move(['table-1', 'table-2', 'table-3'])
     expect(small.moved).toHaveLength(3)
-    const afterSmall = mocks.resolvePermission.mock.calls.length
+    expect(mocks.resolveWorkspaceContext).toHaveBeenCalledTimes(1)
 
-    mocks.resolvePermission.mockClear()
+    mocks.resolveWorkspaceContext.mockClear()
     const large = await move(Array.from({ length: 25 }, (_, index) => `table-${index}`))
     expect(large.moved).toHaveLength(25)
+    expect(mocks.resolveWorkspaceContext).toHaveBeenCalledTimes(1)
+  })
+
+  /** A revocation part-way through a batch must stop the items that have not run yet. */
+  it('re-checks the caller permission for every item', async () => {
+    mocks.resolvePermission.mockResolvedValueOnce('write').mockResolvedValueOnce('write')
+    mocks.resolvePermission.mockResolvedValue(null)
+
+    const result = await bulkMoveTables.execute({
+      principal,
+      input: {
+        assertedWorkspaceId: 'workspace-1',
+        tableIds: ['table-1', 'table-2', 'table-3'],
+        folderIds: [],
+        targetFolderId: 'folder-1',
+      },
+    })
 
-    expect(mocks.resolvePermission).toHaveBeenCalledTimes(afterSmall)
-    expect(afterSmall).toBe(2)
+    expect(result.moved).toHaveLength(1)
+    expect(result.failed.concat(result.notFound as never[])).toHaveLength(2)
+    /** One for the operation itself, then one per item — no memo may collapse these. */
+    expect(mocks.resolvePermission).toHaveBeenCalledTimes(4)
   })
 
   it('moves tables and folders in one operation', async () => {