From 112e0285b3b80ba8c0b05981a70635fcd50d213c Mon Sep 17 00:00:00 2001 From: waleed Date: Fri, 15 May 2026 11:44:08 -0700 Subject: [PATCH] fix(logs,workspace): prevent cancelled status overwrite on race and move impersonation banner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../{ => components}/impersonation-banner.tsx | 0 .../app/workspace/[workspaceId]/layout.tsx | 2 +- .../logs/execution/logging-session.test.ts | 69 +++++++++++++++++++ .../sim/lib/logs/execution/logging-session.ts | 17 +++++ 4 files changed, 87 insertions(+), 1 deletion(-) rename apps/sim/app/workspace/[workspaceId]/{ => components}/impersonation-banner.tsx (100%) diff --git a/apps/sim/app/workspace/[workspaceId]/impersonation-banner.tsx b/apps/sim/app/workspace/[workspaceId]/components/impersonation-banner.tsx similarity index 100% rename from apps/sim/app/workspace/[workspaceId]/impersonation-banner.tsx rename to 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 index 5d253320ce1..ff0ce22c713 100644 --- 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 index b4a42952b58..95805c74eac 100644 --- 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 index 571242afb63..573d6f26f6d 100644 --- 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()