fix(tables): settle a resume whose pause cannot be saved as failed, and never fail a completed run

A resumed run that paused but whose pause state could not be persisted failed
its log yet returned a paused result, so the cell showed paused and the resume
entry was marked completed. It now throws after failing the log, so the attempt
settles as failed and reports execution_failed.

markResumeFailed also rewrote a completed log as failed when a step after a
completed run threw. A completed run's outcome now stands, and an already
failed log keeps its original end time.

Author: waleedlatif1

apps/sim/lib/workflows/executor/human-in-the-loop-manager.test.ts

@@ -22,6 +22,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { createTimeoutAbortController, getExecutionDeadlineAt } from '@/lib/core/execution-limits'
 import { abortManualExecution } from '@/lib/execution/manual-cancellation'
 import { terminalExecutionLogFields } from '@/lib/logs/execution/cancellation'
+import { LoggingSession } from '@/lib/logs/execution/logging-session'
 
 const {
   mockExecuteWorkflowCore,
@@ -265,6 +266,8 @@ describe('what a failed resume did to its paused execution', () => {
     { logStatus: 'running', pauseStatus: 'paused', executionFailed: true },
     { logStatus: 'cancelled', pauseStatus: 'paused', executionFailed: false },
     { logStatus: 'running', pauseStatus: 'cancelling', executionFailed: false },
+    { logStatus: 'failed', pauseStatus: 'paused', executionFailed: true },
+    { logStatus: 'completed', pauseStatus: 'paused', executionFailed: false },
   ])(
     'reports the execution failed: $executionFailed for a $logStatus log and $pauseStatus pause',
     async ({ logStatus, pauseStatus, executionFailed }) => {
@@ -276,6 +279,23 @@ describe('what a failed resume did to its paused execution', () => {
     }
   )
 
+  it.each([
+    { logStatus: 'completed', updated: [resumeQueue] },
+    { logStatus: 'failed', updated: [resumeQueue, pausedExecutions] },
+    { logStatus: 'running', updated: [resumeQueue, pausedExecutions, workflowExecutionLogs] },
+  ])(
+    'leaves a $logStatus execution log as it is when a resume fails late',
+    async ({ logStatus, updated }) => {
+      queueTableRows(workflowExecutionLogs, [{ status: logStatus }])
+      queueTableRows(pausedExecutions, [{ status: 'paused' }])
+      const managerInternals = PauseResumeManager as unknown as PauseResumeManagerInternals
+
+      await managerInternals.markResumeFailed(attemptArgs)
+
+      expect(dbChainMockFns.update.mock.calls.map(([table]) => table)).toEqual(updated)
+    }
+  )
+
   /** Resume args that collect every outcome the manager reports. */
   function argsReportingOutcomes(onAttemptFailed?: () => Promise<void>) {
     const outcomes: FailedResumeOutcome[] = []
@@ -313,6 +333,59 @@ describe('what a failed resume did to its paused execution', () => {
     }
   )
 
+  describe('when the resumed run pauses but its pause cannot be saved', () => {
+    const spies: { mockRestore: () => void }[] = []
+
+    function pauseRun(options: { snapshotSeed?: unknown; persistError?: Error }) {
+      const managerInternals = PauseResumeManager as unknown as PauseResumeManagerInternals
+      spies.push(
+        vi.spyOn(managerInternals, 'runResumeExecution').mockResolvedValueOnce({
+          success: true,
+          status: 'paused',
+          output: {},
+          logs: [],
+          pausePoints: [],
+          snapshotSeed: options.snapshotSeed,
+          metadata: { executionId: 'parent-execution-1', duration: 1, startTime: 'start' },
+        }),
+        vi.spyOn(managerInternals, 'markResumeFailed').mockResolvedValueOnce(true),
+        vi.spyOn(LoggingSession, 'markExecutionAsFailed').mockResolvedValueOnce(),
+        vi.spyOn(PauseResumeManager, 'processQueuedResumes').mockResolvedValueOnce()
+      )
+      if (options.persistError) {
+        spies.push(
+          vi
+            .spyOn(PauseResumeManager, 'persistPauseResult')
+            .mockRejectedValueOnce(options.persistError)
+        )
+      }
+    }
+
+    afterEach(() => {
+      for (const spy of spies.splice(0)) spy.mockRestore()
+    })
+
+    it('fails the attempt when the pause state cannot be persisted', async () => {
+      pauseRun({ snapshotSeed: createSnapshotSeed(), persistError: new Error('lock timeout') })
+      const { outcomes, args } = argsReportingOutcomes()
+
+      await expect(PauseResumeManager.startResumeExecution(args)).rejects.toThrow(
+        'Failed to persist pause state: lock timeout'
+      )
+      expect(outcomes).toEqual(['execution_failed'])
+    })
+
+    it('fails the attempt when the paused run has no snapshot seed', async () => {
+      pauseRun({})
+      const { outcomes, args } = argsReportingOutcomes()
+
+      await expect(PauseResumeManager.startResumeExecution(args)).rejects.toThrow(
+        'Missing snapshot seed for paused execution'
+      )
+      expect(outcomes).toEqual(['execution_failed'])
+    })
+  })
+
   describe('when the resumed run fails', () => {
     const rawError = new Error('Block failed')
     const spies: { mockRestore: () => void }[] = []

apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts

@@ -924,18 +924,22 @@ export class PauseResumeManager {
       })
 
       if (result.status === 'paused') {
+        /**
+         * A pause that cannot be saved fails the execution. Fail the log with the
+         * reason, then throw so the attempt settles as failed below.
+         */
         const effectiveExecutionId = result.metadata?.executionId ?? resumeExecutionId
-        if (!result.snapshotSeed) {
-          logger.error('Missing snapshot seed for paused resume execution', {
-            resumeExecutionId,
-          })
+        const failPause = async (message: string, cause?: unknown): Promise<never> => {
           await LoggingSession.markExecutionAsFailed(
             effectiveExecutionId,
-            'Missing snapshot seed for paused execution',
+            message,
             undefined,
             pausedExecution.workflowId
           )
-          await releaseExecutionSlot(resumeEntryId)
+          throw new Error(message, { cause })
+        }
+        if (!result.snapshotSeed) {
+          await failPause('Missing snapshot seed for paused execution')
         } else {
           try {
             await PauseResumeManager.persistPauseResult({
@@ -947,19 +951,10 @@ export class PauseResumeManager {
               executorUserId: result.metadata?.userId,
             })
           } catch (pauseError) {
-            logger.error(
-              'Failed to persist pause result for resumed execution',
-              projectResolvedSecretDiagnosticError(pauseError, undefined, {
-                resumeExecutionId,
-              })
-            )
-            await LoggingSession.markExecutionAsFailed(
-              effectiveExecutionId,
+            await failPause(
               `Failed to persist pause state: ${toError(pauseError).message}`,
-              undefined,
-              pausedExecution.workflowId
+              pauseError
             )
-            await releaseExecutionSlot(resumeEntryId)
           }
         }
       } else {
@@ -2268,6 +2263,9 @@ export class PauseResumeManager {
         return false
       }
 
+      /** The run completed before a later step threw; its outcome stands. */
+      if (executionLog?.status === 'completed') return false
+
       await tx
         .update(pausedExecutions)
         .set({
@@ -2278,15 +2276,17 @@ export class PauseResumeManager {
 
       if (pausedExecution?.status === 'cancelling') return false
 
-      await tx
-        .update(workflowExecutionLogs)
-        .set(terminalExecutionLogFields('failed', now))
-        .where(
-          and(
-            eq(workflowExecutionLogs.executionId, args.parentExecutionId),
-            sql`${workflowExecutionLogs.status} != 'cancelled'`
+      if (executionLog?.status !== 'failed') {
+        await tx
+          .update(workflowExecutionLogs)
+          .set(terminalExecutionLogFields('failed', now))
+          .where(
+            and(
+              eq(workflowExecutionLogs.executionId, args.parentExecutionId),
+              sql`${workflowExecutionLogs.status} != 'cancelled'`
+            )
           )
-        )
+      }
 
       return true
     })