fix(logs,workspace): prevent cancelled status overwrite on race and move impersonation banner (#4617)

waleedlatif1 · web-flow · commit d3d8f9c8616b · 2026-05-15T12:14:37.000-07:00
- Guard completeWithError against overwriting a cancelled execution status — cancel route writes cancelled to DB optimistically, but a block error racing the 500ms Redis check could finalize with failed before the engine detects cancellation
- Add tests covering the guard: cancelled DB status skips the write, non-cancelled proceeds normally, DB failure falls through to cost-only fallback, and subsequent attempts are deduped after guard marks session complete
- Move ImpersonationBanner from workspace root into components/ folder
diff --git a/apps/sim/app/workspace/[workspaceId]/components/impersonation-banner.tsx b/apps/sim/app/workspace/[workspaceId]/components/impersonation-banner.tsx
diff --git a/apps/sim/app/workspace/[workspaceId]/layout.tsx b/apps/sim/app/workspace/[workspaceId]/layout.tsx
@@ -1,8 +1,8 @@
 import { redirect } from 'next/navigation'
 import { ToastProvider } from '@/components/emcn'
 import { getSession } from '@/lib/auth'
+import { ImpersonationBanner } from '@/app/workspace/[workspaceId]/components/impersonation-banner'
 import { NavTour } from '@/app/workspace/[workspaceId]/components/product-tour'
-import { ImpersonationBanner } from '@/app/workspace/[workspaceId]/impersonation-banner'
 import { GlobalCommandsProvider } from '@/app/workspace/[workspaceId]/providers/global-commands-provider'
 import { ProviderModelsLoader } from '@/app/workspace/[workspaceId]/providers/provider-models-loader'
 import { SettingsLoader } from '@/app/workspace/[workspaceId]/providers/settings-loader'
diff --git a/apps/sim/lib/logs/execution/logging-session.test.ts b/apps/sim/lib/logs/execution/logging-session.test.ts
@@ -453,6 +453,75 @@ describe('LoggingSession completion retries', () => {
   })
 })
 
+describe('completeWithError cancelled-status guard', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    dbMocks.updateWhere.mockResolvedValue(undefined)
+    dbMocks.execute.mockResolvedValue(undefined)
+  })
+
+  it('skips writing failed and marks session complete when DB status is already cancelled', async () => {
+    dbMocks.selectLimit.mockResolvedValue([{ status: 'cancelled' }])
+    const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
+
+    await session.safeCompleteWithError({ error: { message: 'block errored mid-cancel' } })
+
+    expect(completeWorkflowExecutionMock).not.toHaveBeenCalled()
+    expect(session.hasCompleted()).toBe(true)
+  })
+
+  it('writes failed when DB status is running (no cancel in flight)', async () => {
+    dbMocks.selectLimit.mockResolvedValue([{ status: 'running' }])
+    completeWorkflowExecutionMock.mockResolvedValue({})
+    const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
+
+    await session.safeCompleteWithError({ error: { message: 'genuine block failure' } })
+
+    expect(completeWorkflowExecutionMock).toHaveBeenCalledWith(
+      expect.objectContaining({ status: 'failed' })
+    )
+    expect(session.hasCompleted()).toBe(true)
+  })
+
+  it('writes failed when no execution log exists yet', async () => {
+    dbMocks.selectLimit.mockResolvedValue([])
+    completeWorkflowExecutionMock.mockResolvedValue({})
+    const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
+
+    await session.safeCompleteWithError({ error: { message: 'pre-log error' } })
+
+    expect(completeWorkflowExecutionMock).toHaveBeenCalledWith(
+      expect.objectContaining({ status: 'failed' })
+    )
+  })
+
+  it('deduplicates all subsequent completion attempts after guard early-return', async () => {
+    dbMocks.selectLimit.mockResolvedValue([{ status: 'cancelled' }])
+    completeWorkflowExecutionMock.mockResolvedValue({})
+    const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
+
+    await session.safeCompleteWithError({ error: { message: 'error 1' } })
+    await session.safeCompleteWithError({ error: { message: 'error 2' } })
+    await session.safeComplete({ finalOutput: { ok: true } })
+
+    expect(completeWorkflowExecutionMock).not.toHaveBeenCalled()
+    expect(session.hasCompleted()).toBe(true)
+  })
+
+  it('falls through to cost-only fallback when the DB check itself throws', async () => {
+    dbMocks.selectLimit.mockRejectedValueOnce(new Error('DB connection lost'))
+    completeWorkflowExecutionMock.mockResolvedValue({})
+    const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
+
+    await session.safeCompleteWithError({ error: { message: 'block failed' } })
+
+    expect(completeWorkflowExecutionMock).toHaveBeenCalledWith(
+      expect.objectContaining({ finalizationPath: 'force_failed' })
+    )
+    expect(session.hasCompleted()).toBe(true)
+  })
+})
+
 describe('LoggingSession.markExecutionAsFailed workflowId scoping', () => {
   beforeEach(() => {
     vi.clearAllMocks()
diff --git a/apps/sim/lib/logs/execution/logging-session.ts b/apps/sim/lib/logs/execution/logging-session.ts
@@ -544,6 +544,23 @@ export class LoggingSession {
     this.completing = true
 
     try {
+      const currentLog = await db
+        .select({ status: workflowExecutionLogs.status })
+        .from(workflowExecutionLogs)
+        .where(
+          and(
+            eq(workflowExecutionLogs.workflowId, this.workflowId),
+            eq(workflowExecutionLogs.executionId, this.executionId)
+          )
+        )
+        .limit(1)
+        .then((rows) => rows[0])
+
+      if (currentLog?.status === 'cancelled') {
+        this.completed = true
+        return
+      }
+
       const { endedAt, totalDurationMs, error, traceSpans, skipCost } = params
 
       const endTime = endedAt ? new Date(endedAt) : new Date()
