improvement(perf): restore the parallel file read, and close the gaps a diff audit surfaced

Author: waleedlatif1

apps/sim/app/workspace/[workspaceId]/lib/prefetch.test.ts

@@ -527,6 +527,8 @@ describe('workspace list prefetches', () => {
 
       expect(mockListWorkspaceFilesWithShares).toHaveBeenCalledWith(WORKSPACE_ID, 'active', {
         maxRows: WORKSPACE_FILE_SEED_MAX,
+        /** A failed read must reach the catch, not degrade to a cached empty list. */
+        throwOnError: true,
       })
       expect(client.getQueryData(workspaceFilesKeys.list(WORKSPACE_ID, 'active'))).toEqual(files)
     })

apps/sim/app/workspace/[workspaceId]/prefetch.ts

@@ -130,6 +130,12 @@ async function seedWorkspaceFiles(queryClient: QueryClient, workspaceId: string)
   try {
     const files = await listWorkspaceFilesWithShares(workspaceId, 'active', {
       maxRows: WORKSPACE_FILE_SEED_MAX,
+      /**
+       * A failed read must reach the catch below, not degrade to an empty list: seeding
+       * `[]` would cache "this workspace has no files" as authoritative for the entry's
+       * lifetime, which is worse than seeding nothing and letting the client fetch.
+       */
+      throwOnError: true,
     })
     if (!files) return
     queryClient.setQueryData(workspaceFilesKeys.list(workspaceId, 'active'), files)

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx

@@ -191,10 +191,6 @@ vi.mock('@/executor/utils/start-block', () => ({
   coerceValue: (_type: string, value: unknown) => value,
 }))
 
-vi.mock('@/hooks/queries/subscription', () => ({
-  subscriptionKeys: { users: () => ['subscription', 'users'] },
-}))
-
 vi.mock('@/hooks/queries/utils/workflow-cache', () => ({
   getWorkflows: () => [],
 }))

apps/sim/executor/handlers/evaluator/evaluator-handler.test.ts

@@ -145,6 +145,37 @@ describe('EvaluatorBlockHandler', () => {
     expect(handler.canHandle(nonEvalBlock)).toBe(false)
   })
 
+  /**
+   * The admission checks the removed `/api/providers` hop owned. Mirrors the router's
+   * coverage — both handlers reach the provider through the same shared entry point.
+   */
+  const admissionInputs = {
+    content: 'Evaluate this.',
+    metrics: [{ name: 'score1', description: 'First score', range: { min: 0, max: 10 } }],
+    model: 'gpt-4o',
+    apiKey: 'test-api-key',
+  }
+
+  it('refuses to reach the provider without an execution subject', async () => {
+    mockContext.userId = undefined
+
+    await expect(handler.execute(mockContext, mockBlock, admissionInputs)).rejects.toThrow(
+      'Unauthorized'
+    )
+    expect(mockExecuteProviderRequest).not.toHaveBeenCalled()
+  })
+
+  it('refuses to reach the provider when the subject lost workspace access', async () => {
+    mockContext.workspaceId = 'test-workspace'
+    mockCheckWorkspaceAccess.mockResolvedValue({ hasAccess: false })
+
+    await expect(handler.execute(mockContext, mockBlock, admissionInputs)).rejects.toThrow(
+      'Forbidden'
+    )
+    expect(mockCheckWorkspaceAccess).toHaveBeenCalledWith('test-workspace', 'test-user')
+    expect(mockExecuteProviderRequest).not.toHaveBeenCalled()
+  })
+
   it('should execute evaluator block correctly with basic inputs', async () => {
     const inputs = {
       content: 'This is the content to evaluate.',

apps/sim/executor/utils/provider-request.ts

@@ -60,17 +60,32 @@ export async function executeBlockProviderRequest({
    * model-emitted tool calls, and the route this replaces never carried one.
    * Router and evaluator requests declare no tools, so passing the executor's
    * context here would widen the trusted surface without changing any outcome.
+   *
+   * The whole runtime context is omitted when there is no registry, rather than
+   * passed carrying `undefined`. `executeProviderTool` reads a present context with
+   * an absent registry as "provenance was expected and is missing" and fails the
+   * call closed with no error text — unreachable while these blocks declare no
+   * tools, but a silent failure the day one does.
    */
   const response = await executeProviderRequest(
     providerId,
     { ...request, userId: ctx.userId },
-    { resolvedSecretTraceRegistry }
+    resolvedSecretTraceRegistry ? { resolvedSecretTraceRegistry } : undefined
   )
 
-  if (response instanceof ReadableStream || (response !== null && 'stream' in response)) {
+  if (
+    response instanceof ReadableStream ||
+    (typeof response === 'object' && response !== null && 'stream' in response)
+  ) {
     logger.error('Provider returned a stream for a non-streaming block request', { providerId })
     throw new Error('Provider returned a streaming response for a non-streaming request')
   }
 
+  logger.info('Provider request completed', {
+    providerId,
+    model: request.model,
+    workflowId: ctx.workflowId,
+  })
+
   return response
 }

apps/sim/hooks/queries/schedules.ts

@@ -249,6 +249,11 @@ export function useRedeployWorkflowSchedule() {
       const { workflowId, blockId } = data
       await Promise.all([
         queryClient.invalidateQueries({ queryKey: scheduleKeys.schedule(workflowId, blockId) }),
+        /**
+         * A redeploy recreates the schedule, so the id-keyed reads go stale too. They are
+         * a separate subtree from `schedule(workflowId, blockId)`, which does not cover them.
+         */
+        queryClient.invalidateQueries({ queryKey: scheduleKeys.byIds() }),
         queryClient.invalidateQueries({ queryKey: deploymentKeys.info(workflowId) }),
         queryClient.invalidateQueries({ queryKey: deploymentKeys.versions(workflowId) }),
       ])

apps/sim/hooks/queries/tables.test.ts

@@ -129,6 +129,33 @@ describe('useDeleteColumn optimistic update', () => {
     expect(ctx?.rowSnapshots?.length).toBeGreaterThan(0)
   })
 
+  /**
+   * The `find` cache hangs off the same `rowsRoot` parent as the paged rows but holds
+   * `{matches, truncated}` — no `pages`, no `rows`. A cache walk starting at the shared
+   * parent reaches it and throws inside `onMutate`, rejecting the mutation before it ever
+   * reaches the server: search a table, dismiss the search, then edit a cell.
+   */
+  it('survives a cached search result hanging off the shared rows prefix', async () => {
+    setCache(tableKeys.detail(TABLE_ID), {
+      id: TABLE_ID,
+      schema: { columns: [{ name: 'age', type: 'number' }] },
+    })
+    setCache(ROWS_KEY, {
+      rows: [{ id: 'r1', data: { age: 1 } }],
+      totalCount: 1,
+    })
+    setCache(tableKeys.find(TABLE_ID, 'q'), { matches: [{ rowId: 'r1', column: 'age' }] })
+
+    const hook = useDeleteColumn({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID })
+
+    await expect(hook.onMutate?.('age')).resolves.toBeDefined()
+
+    const rows = getCache<{ rows: Array<{ data: Record<string, unknown> }> }>(ROWS_KEY)
+    expect(rows?.rows[0]?.data).toEqual({})
+    /** The find entry is match coordinates, not row values — it must be left untouched. */
+    expect(getCache<{ matches: unknown[] }>(tableKeys.find(TABLE_ID, 'q'))?.matches).toHaveLength(1)
+  })
+
   it('rolls back schema and rows on error using snapshots', async () => {
     const originalDetail = {
       id: TABLE_ID,

apps/sim/lib/workflows/persistence/save-normalized-state.test.ts

@@ -0,0 +1,83 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import { parseWorkflowStateForPersistence } from '@/lib/workflows/persistence/save-normalized-state'
+
+/**
+ * A checkpoint blob as the revert route builds it: JSONB-derived blocks and edges, plus a
+ * real `Date` for `deployedAt`.
+ */
+function checkpointState(overrides?: Record<string, unknown>) {
+  return {
+    blocks: {
+      'block-1': {
+        id: 'block-1',
+        type: 'starter',
+        name: 'Start',
+        position: { x: 0, y: 0 },
+        subBlocks: {},
+        outputs: {},
+        enabled: true,
+      },
+    },
+    edges: [],
+    loops: {},
+    parallels: {},
+    isDeployed: false,
+    lastSaved: 1_754_000_000_000,
+    ...overrides,
+  }
+}
+
+describe('parseWorkflowStateForPersistence', () => {
+  /**
+   * The revert used to reach this schema by POSTing the blob over HTTP, so every value
+   * arrived JSON-serialized. In-process the blob keeps its runtime types. Both forms must
+   * parse identically, or a checkpoint that reverted before would start failing.
+   */
+  it('accepts a Date for deployedAt exactly as it accepted the serialized string', () => {
+    const deployedAt = new Date('2026-01-02T03:04:05.678Z')
+
+    const fromDate = parseWorkflowStateForPersistence(checkpointState({ deployedAt }))
+    const overTheWire = parseWorkflowStateForPersistence(
+      JSON.parse(JSON.stringify(checkpointState({ deployedAt })))
+    )
+
+    expect(fromDate.success).toBe(true)
+    expect(overTheWire.success).toBe(true)
+    expect(fromDate.data?.deployedAt).toEqual(deployedAt)
+    expect(overTheWire.data?.deployedAt).toEqual(fromDate.data?.deployedAt)
+  })
+
+  it('round-trips a JSONB-shaped blob without dropping blocks or edges', () => {
+    const state = checkpointState()
+
+    const parsed = parseWorkflowStateForPersistence(state)
+
+    expect(parsed.success).toBe(true)
+    expect(Object.keys(parsed.data?.blocks ?? {})).toEqual(['block-1'])
+    expect(parsed.data?.lastSaved).toBe(1_754_000_000_000)
+  })
+
+  it('accepts a null deployedAt, which the revert passes for a never-deployed checkpoint', () => {
+    const parsed = parseWorkflowStateForPersistence(checkpointState({ deployedAt: null }))
+
+    expect(parsed.success).toBe(true)
+    expect(parsed.data?.deployedAt).toBeNull()
+  })
+
+  /** The validation the removed HTTP hop used to provide: a malformed blob must not be written. */
+  it('rejects a blob whose blocks are malformed', () => {
+    const parsed = parseWorkflowStateForPersistence({
+      blocks: { 'block-1': { id: 'block-1' } },
+      edges: [],
+    })
+
+    expect(parsed.success).toBe(false)
+  })
+
+  it('rejects a blob missing blocks entirely', () => {
+    expect(parseWorkflowStateForPersistence({ edges: [] }).success).toBe(false)
+  })
+})

apps/sim/lib/workspace-files/queries.test.ts

@@ -56,15 +56,16 @@ describe('listWorkspaceFilesWithShares', () => {
   /**
    * `maxRows` exists for a caller that will only use the list if the whole workspace fits
    * its payload budget, so overflow must be reported as `null` — a prefix returned here
-   * would be presented as the workspace's complete file list.
+   * would be presented as the workspace's complete file list. The share read still runs
+   * concurrently and is discarded: the under-budget workspaces are the common case, and
+   * serializing the two reads to save this one would tax every normal request.
    */
-  it('returns null without joining shares when the workspace exceeds maxRows', async () => {
+  it('returns null when the workspace exceeds maxRows', async () => {
     mockListWorkspaceFiles.mockResolvedValue([STORED_FILE, STORED_FILE, STORED_FILE])
 
     const result = await listWorkspaceFilesWithShares('ws-1', 'active', { maxRows: 2 })
 
     expect(result).toBeNull()
-    expect(mockGetWorkspaceShares).not.toHaveBeenCalled()
     expect(mockListWorkspaceFiles).toHaveBeenCalledWith('ws-1', { scope: 'active', limit: 3 })
   })
 
@@ -77,6 +78,37 @@ describe('listWorkspaceFilesWithShares', () => {
     expect(mockGetWorkspaceShares).toHaveBeenCalledWith('file', 'ws-1')
   })
 
+  /** The boundary the `>` comparison turns on: exactly maxRows must still be the list. */
+  it('returns the list when it sits exactly on maxRows', async () => {
+    mockListWorkspaceFiles.mockResolvedValue([STORED_FILE, STORED_FILE])
+
+    const result = await listWorkspaceFilesWithShares('ws-1', 'active', { maxRows: 2 })
+
+    expect(result).toHaveLength(2)
+  })
+
+  /**
+   * The file read swallows errors and returns `[]` by default. A caller seeding a cache
+   * must not receive that: an empty list would be cached as "this workspace has no files".
+   */
+  it('propagates a failed read instead of degrading to an empty list', async () => {
+    await listWorkspaceFilesWithShares('ws-1', 'active', { throwOnError: true })
+
+    expect(mockListWorkspaceFiles).toHaveBeenCalledWith(
+      'ws-1',
+      expect.objectContaining({ throwOnError: true })
+    )
+  })
+
+  it('does not ask the file read to throw unless the caller opts in', async () => {
+    await listWorkspaceFilesWithShares('ws-1', 'active')
+
+    expect(mockListWorkspaceFiles).toHaveBeenCalledWith(
+      'ws-1',
+      expect.not.objectContaining({ throwOnError: true })
+    )
+  })
+
   it('joins each file public share onto its row', async () => {
     const share = {
       id: 'share-1',

apps/sim/lib/workspace-files/queries.ts

@@ -16,26 +16,33 @@ import {
  *
  * Callers authorize the viewer against `workspaceId` first.
  *
- * `maxRows` bounds the work for a caller that will only use the list if the whole
+ * `maxRows` bounds the result for a caller that will only use the list if the whole
  * workspace fits a payload budget: the read stops one row past the budget and returns
- * `null` on overflow, before the share join and the contract parse — so the workspaces
- * the budget exists to protect are the ones that pay least to be rejected. Returning
- * `null` rather than the prefix is what stops a caller presenting a truncated read as
- * the workspace's files.
+ * `null` on overflow rather than the prefix, which is what stops a caller presenting a
+ * truncated read as the workspace's files. The two reads still run concurrently — the
+ * workspaces under the budget are the common case, and serializing them to save a share
+ * read on the rare oversized one would tax every normal request to do it.
+ *
+ * `throwOnError` propagates a failed file read instead of letting it degrade to an empty
+ * list. A caller seeding a cache needs that distinction: an empty list would be cached
+ * as authoritative, telling the user the workspace has no files.
  */
 export async function listWorkspaceFilesWithShares(
   workspaceId: string,
   scope: WorkspaceFileScope,
-  options?: { maxRows?: number }
+  options?: { maxRows?: number; throwOnError?: boolean }
 ) {
   const maxRows = options?.maxRows
-  const files = await listWorkspaceFiles(workspaceId, {
-    scope,
-    ...(maxRows === undefined ? {} : { limit: maxRows + 1 }),
-  })
+  const [files, shares] = await Promise.all([
+    listWorkspaceFiles(workspaceId, {
+      scope,
+      ...(maxRows === undefined ? {} : { limit: maxRows + 1 }),
+      ...(options?.throwOnError ? { throwOnError: true } : {}),
+    }),
+    getWorkspaceShares('file', workspaceId),
+  ])
   if (maxRows !== undefined && files.length > maxRows) return null
 
-  const shares = await getWorkspaceShares('file', workspaceId)
   const withShares = files.map((file) => ({ ...file, share: shares.get(file.id) ?? null }))
   return listWorkspaceFilesContract.response.schema.shape.files.parse(withShares)
 }