fix(execution): scope the snapshot FK retry, keep a suspended identity's access read out of the run, tidy tests

Author: waleedlatif1

apps/sim/lib/core/async-jobs/backends/trigger-dev.test.ts

@@ -274,30 +274,31 @@ describe('TriggerDevJobQueue status mapping', () => {
     })
   })
 
-  it('resolves a caller-chosen job id through its tag without retrieving the id itself', async () => {
-    mockList.mockReturnValueOnce(createListPage([{ id: 'run_1', tags: ['jobId:schedule_abc'] }]))
-    mockRetrieve.mockResolvedValueOnce({
-      id: 'run_1',
-      payload: {},
-      status: 'QUEUED',
-      taskIdentifier: 'schedule-execution',
+  /** Trigger.dev can only retrieve its own run ids; anything else fails, and slowly. */
+  function retrieveOnlyRunIds() {
+    mockRetrieve.mockImplementation(async (id: string) => {
+      if (!id.startsWith('run_')) throw new Error(`Retrieved a caller-chosen job id: ${id}`)
+      return { id, payload: {}, status: 'QUEUED', taskIdentifier: 'schedule-execution' }
     })
+  }
+
+  it('resolves a caller-chosen job id through its tag', async () => {
+    retrieveOnlyRunIds()
+    mockList.mockReturnValueOnce(createListPage([{ id: 'run_1', tags: ['jobId:schedule_abc'] }]))
     const queue = new TriggerDevJobQueue()
 
     await expect(queue.getJob('schedule_abc')).resolves.toMatchObject({
       id: 'run_1',
       status: 'pending',
     })
-    expect(mockRetrieve).toHaveBeenCalledTimes(1)
-    expect(mockRetrieve).toHaveBeenCalledWith('run_1')
   })
 
   it('returns null for a caller-chosen job id with no tagged run', async () => {
+    retrieveOnlyRunIds()
     mockList.mockReturnValueOnce(createListPage([]))
     const queue = new TriggerDevJobQueue()
 
     await expect(queue.getJob('schedule_abc')).resolves.toBeNull()
-    expect(mockRetrieve).not.toHaveBeenCalled()
   })
 
   it('falls back to the tag lookup when a run id is not found', async () => {

apps/sim/lib/environment/utils.ts

@@ -538,12 +538,15 @@ export async function getExecutionEnvironment(
     return getPersonalAndWorkspaceEnv(personalUserId, workspaceId)
   }
 
-  const [suspended, actorAccess, personalAccess] = await Promise.all([
+  const personalAccessRead = checkWorkspaceAccess(workspaceId, personalUserId)
+  personalAccessRead.catch(() => {})
+  const [suspended, actorAccess] = await Promise.all([
     personalIdentitySuspended,
     checkWorkspaceAccess(workspaceId, workspaceUserId),
-    checkWorkspaceAccess(workspaceId, personalUserId),
   ])
+  // A suspended identity's access is never consulted, so its read cannot fail the run.
   if (suspended) return resolveSuspendedIdentity()
+  const personalAccess = await personalAccessRead
 
   /**
    * A workspace that no longer exists and one an identity may not read are

apps/sim/lib/logs/execution/logger.ts

@@ -8,7 +8,12 @@ import {
   workspace,
 } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
-import { describeError, getErrorMessage, getPostgresErrorCode } from '@sim/utils/errors'
+import {
+  describeError,
+  getErrorMessage,
+  getPostgresConstraintName,
+  getPostgresErrorCode,
+} from '@sim/utils/errors'
 import { generateId } from '@sim/utils/id'
 import { and, eq, inArray, sql } from 'drizzle-orm'
 import { checkUsageStatus as checkResolvedUsageStatus } from '@/lib/billing/calculations/usage-monitor'
@@ -95,6 +100,8 @@ const EXECUTION_LOG_IDLE_TIMEOUT_MS = 5_000
 // (favor waiting over dropping a charge); only trips on a pathological lock hold.
 const USAGE_RECONCILE_LOCK_TIMEOUT_MS = 10_000
 const FOREIGN_KEY_VIOLATION = '23503'
+/** The log's snapshot foreign key as Postgres names it (identifiers are cut at 63 bytes). */
+const STATE_SNAPSHOT_FOREIGN_KEY = 'workflow_execution_logs_state_snapshot_id_workflow_execution_sn'
 
 type ExecutionData = WorkflowExecutionLog['executionData']
 
@@ -667,7 +674,12 @@ export class ExecutionLogger implements IExecutionLoggerService {
     try {
       inserted = await insertRunningLog(snapshot.id)
     } catch (error) {
-      if (getPostgresErrorCode(error) !== FOREIGN_KEY_VIOLATION) throw error
+      if (
+        getPostgresErrorCode(error) !== FOREIGN_KEY_VIOLATION ||
+        getPostgresConstraintName(error) !== STATE_SNAPSHOT_FOREIGN_KEY
+      ) {
+        throw error
+      }
       /**
        * A snapshot resolved before the insert can be deleted underneath it by
        * orphan cleanup when no log references it yet. Resolve it again from the

apps/sim/lib/logs/execution/start-execution.integration.ts

@@ -102,11 +102,12 @@ beforeAll(async () => {
 })
 
 afterAll(async () => {
-  await db.delete(workspace).where(eq(workspace.id, ids.workspace))
+  // Snapshots outlive their workflow (workflow_id is set null), so remove them first.
+  await db.delete(workflowExecutionLogs).where(eq(workflowExecutionLogs.workflowId, ids.workflow))
   await db
     .delete(workflowExecutionSnapshots)
     .where(eq(workflowExecutionSnapshots.workflowId, ids.workflow))
-  await db.delete(workflow).where(eq(workflow.id, ids.workflow))
+  await db.delete(workspace).where(eq(workspace.id, ids.workspace))
   await db.delete(user).where(eq(user.id, ids.owner))
 })