fix(cancel): report cancellation durability truthfully

Return explicit durability results for execution cancellation so success only reflects persisted cancellation state instead of best-effort Redis availability.

Author: PlaneInABottle

apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts

@@ -0,0 +1,112 @@
+/**
+ * @vitest-environment node
+ */
+
+import { NextRequest } from 'next/server'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const mockCheckHybridAuth = vi.fn()
+const mockAuthorizeWorkflowByWorkspacePermission = vi.fn()
+const mockMarkExecutionCancelled = vi.fn()
+
+vi.mock('@sim/logger', () => ({
+  createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }),
+}))
+
+vi.mock('@/lib/auth/hybrid', () => ({
+  checkHybridAuth: (...args: unknown[]) => mockCheckHybridAuth(...args),
+}))
+
+vi.mock('@/lib/execution/cancellation', () => ({
+  markExecutionCancelled: (...args: unknown[]) => mockMarkExecutionCancelled(...args),
+}))
+
+vi.mock('@/lib/workflows/utils', () => ({
+  authorizeWorkflowByWorkspacePermission: (params: unknown) =>
+    mockAuthorizeWorkflowByWorkspacePermission(params),
+}))
+
+import { POST } from './route'
+
+describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockCheckHybridAuth.mockResolvedValue({ success: true, userId: 'user-1' })
+    mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true })
+  })
+
+  it('returns success when cancellation was durably recorded', async () => {
+    mockMarkExecutionCancelled.mockResolvedValue({
+      durablyRecorded: true,
+      reason: 'recorded',
+    })
+
+    const response = await POST(
+      new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
+        method: 'POST',
+      }),
+      {
+        params: Promise.resolve({ id: 'wf-1', executionId: 'ex-1' }),
+      }
+    )
+
+    expect(response.status).toBe(200)
+    await expect(response.json()).resolves.toEqual({
+      success: true,
+      executionId: 'ex-1',
+      redisAvailable: true,
+      durablyRecorded: true,
+      reason: 'recorded',
+    })
+  })
+
+  it('returns unsuccessful response when Redis is unavailable', async () => {
+    mockMarkExecutionCancelled.mockResolvedValue({
+      durablyRecorded: false,
+      reason: 'redis_unavailable',
+    })
+
+    const response = await POST(
+      new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
+        method: 'POST',
+      }),
+      {
+        params: Promise.resolve({ id: 'wf-1', executionId: 'ex-1' }),
+      }
+    )
+
+    expect(response.status).toBe(200)
+    await expect(response.json()).resolves.toEqual({
+      success: false,
+      executionId: 'ex-1',
+      redisAvailable: false,
+      durablyRecorded: false,
+      reason: 'redis_unavailable',
+    })
+  })
+
+  it('returns unsuccessful response when Redis persistence fails', async () => {
+    mockMarkExecutionCancelled.mockResolvedValue({
+      durablyRecorded: false,
+      reason: 'redis_write_failed',
+    })
+
+    const response = await POST(
+      new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
+        method: 'POST',
+      }),
+      {
+        params: Promise.resolve({ id: 'wf-1', executionId: 'ex-1' }),
+      }
+    )
+
+    expect(response.status).toBe(200)
+    await expect(response.json()).resolves.toEqual({
+      success: false,
+      executionId: 'ex-1',
+      redisAvailable: false,
+      durablyRecorded: false,
+      reason: 'redis_write_failed',
+    })
+  })
+})

apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts

@@ -35,20 +35,23 @@ export async function POST(
 
     logger.info('Cancel execution requested', { workflowId, executionId, userId: auth.userId })
 
-    const marked = await markExecutionCancelled(executionId)
+    const cancellation = await markExecutionCancelled(executionId)
 
-    if (marked) {
+    if (cancellation.durablyRecorded) {
       logger.info('Execution marked as cancelled in Redis', { executionId })
     } else {
-      logger.info('Redis not available, cancellation will rely on connection close', {
+      logger.warn('Execution cancellation was not durably recorded', {
         executionId,
+        reason: cancellation.reason,
       })
     }
 
     return NextResponse.json({
-      success: true,
+      success: cancellation.durablyRecorded,
       executionId,
-      redisAvailable: marked,
+      redisAvailable: cancellation.durablyRecorded,
+      durablyRecorded: cancellation.durablyRecorded,
+      reason: cancellation.reason,
     })
   } catch (error: any) {
     logger.error('Failed to cancel execution', { workflowId, executionId, error: error.message })

apps/sim/lib/execution/cancellation.test.ts

@@ -0,0 +1,48 @@
+import { loggerMock } from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const mockGetRedisClient = vi.fn()
+const mockRedisSet = vi.fn()
+
+vi.mock('@sim/logger', () => loggerMock)
+
+vi.mock('@/lib/core/config/redis', () => ({
+  getRedisClient: mockGetRedisClient,
+}))
+
+import { markExecutionCancelled } from './cancellation'
+
+describe('markExecutionCancelled', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('returns redis_unavailable when no Redis client exists', async () => {
+    mockGetRedisClient.mockReturnValue(null)
+
+    await expect(markExecutionCancelled('execution-1')).resolves.toEqual({
+      durablyRecorded: false,
+      reason: 'redis_unavailable',
+    })
+  })
+
+  it('returns recorded when Redis write succeeds', async () => {
+    mockRedisSet.mockResolvedValue('OK')
+    mockGetRedisClient.mockReturnValue({ set: mockRedisSet })
+
+    await expect(markExecutionCancelled('execution-1')).resolves.toEqual({
+      durablyRecorded: true,
+      reason: 'recorded',
+    })
+  })
+
+  it('returns redis_write_failed when Redis write throws', async () => {
+    mockRedisSet.mockRejectedValue(new Error('set failed'))
+    mockGetRedisClient.mockReturnValue({ set: mockRedisSet })
+
+    await expect(markExecutionCancelled('execution-1')).resolves.toEqual({
+      durablyRecorded: false,
+      reason: 'redis_write_failed',
+    })
+  })
+})

apps/sim/lib/execution/cancellation.ts

@@ -6,27 +6,36 @@ const logger = createLogger('ExecutionCancellation')
 const EXECUTION_CANCEL_PREFIX = 'execution:cancel:'
 const EXECUTION_CANCEL_EXPIRY = 60 * 60
 
+export type ExecutionCancellationRecordResult =
+  | { durablyRecorded: true; reason: 'recorded' }
+  | {
+      durablyRecorded: false
+      reason: 'redis_unavailable' | 'redis_write_failed'
+    }
+
 export function isRedisCancellationEnabled(): boolean {
   return getRedisClient() !== null
 }
 
 /**
  * Mark an execution as cancelled in Redis.
- * Returns true if Redis is available and the flag was set, false otherwise.
+ * Returns whether the cancellation was durably recorded.
  */
-export async function markExecutionCancelled(executionId: string): Promise<boolean> {
+export async function markExecutionCancelled(
+  executionId: string
+): Promise<ExecutionCancellationRecordResult> {
   const redis = getRedisClient()
   if (!redis) {
-    return false
+    return { durablyRecorded: false, reason: 'redis_unavailable' }
   }
 
   try {
     await redis.set(`${EXECUTION_CANCEL_PREFIX}${executionId}`, '1', 'EX', EXECUTION_CANCEL_EXPIRY)
     logger.info('Marked execution as cancelled', { executionId })
-    return true
+    return { durablyRecorded: true, reason: 'recorded' }
   } catch (error) {
     logger.error('Failed to mark execution as cancelled', { executionId, error })
-    return false
+    return { durablyRecorded: false, reason: 'redis_write_failed' }
   }
 }