fix(tables): settle the cell as completed when a completed resume's later step throws

Leaving a completed log alone reported no outcome, so the cell stayed on its
last running state. markResumeFailed now reports what the attempt left the
execution as, and a run that completed before a later step threw settles the
cell as completed. The pause point is still marked failed as before.

Author: waleedlatif1

apps/sim/background/resume-execution.ts

@@ -364,19 +364,26 @@ async function buildResumeCellWriters(
 /**
  * A resume that throws never reaches the terminal write in
  * {@link runResumeAndCellTerminal}, which would leave the cell showing its last
- * partial `running` state. Mirror what the failed attempt did to the execution:
- * a pause that stayed resumable goes back to paused, and a failed execution
- * fails the cell.
+ * partial `running` state. Mirror what the failed attempt left the execution
+ * as: a pause that stayed resumable goes back to paused, a failed execution
+ * fails the cell, and a run that completed before a later step threw
+ * completes it.
  */
 async function writeFailedResumeCellTerminal(
   writers: CellWriters,
   outcome: FailedResumeOutcome,
   error: unknown
 ): Promise<void> {
-  if (outcome === 'pause_retained') {
-    await writers.writeCellTerminal('paused', null)
-  } else {
-    await writers.writeCellTerminal('error', getErrorMessage(error, 'Resume execution failed'))
+  switch (outcome) {
+    case 'pause_retained':
+      await writers.writeCellTerminal('paused', null)
+      return
+    case 'execution_completed':
+      await writers.writeCellTerminal('completed', null)
+      return
+    case 'execution_failed':
+      await writers.writeCellTerminal('error', getErrorMessage(error, 'Resume execution failed'))
+      return
   }
 }
 

apps/sim/background/resume-governed-subject.test.ts

@@ -253,6 +253,19 @@ describe('resuming a paused table cell', () => {
       })
     }, 20_000)
 
+    it('marks the cell completed when the run completed before a later step failed', async () => {
+      const bookkeepingFailure = new Error('Database unavailable')
+      failResume('execution_completed', bookkeepingFailure)
+
+      await expect(executeResumeJob(PAYLOAD)).rejects.toBe(bookkeepingFailure)
+
+      expect(lastCellExecutionState()).toMatchObject({
+        status: 'completed',
+        executionId: 'parent-execution-1',
+        error: null,
+      })
+    }, 20_000)
+
     it('puts the cell back to paused when the pause stayed resumable', async () => {
       const admissionRefusal = new Error('Execution can no longer be resumed')
       failResume('pause_retained', admissionRefusal)

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

@@ -86,7 +86,7 @@ if (!humanInTheLoopLogger) {
 }
 
 interface PauseResumeManagerInternals {
-  markResumeFailed: (...args: unknown[]) => Promise<boolean>
+  markResumeFailed: (...args: unknown[]) => Promise<FailedResumeOutcome | undefined>
   runResumeExecution: (...args: unknown[]) => Promise<unknown>
 }
 
@@ -263,24 +263,24 @@ describe('what a failed resume did to its paused execution', () => {
   )
 
   it.each([
-    { 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 },
+    { logStatus: 'running', pauseStatus: 'paused', outcome: 'execution_failed' },
+    { logStatus: 'cancelled', pauseStatus: 'paused', outcome: undefined },
+    { logStatus: 'running', pauseStatus: 'cancelling', outcome: undefined },
+    { logStatus: 'failed', pauseStatus: 'paused', outcome: 'execution_failed' },
+    { logStatus: 'completed', pauseStatus: 'paused', outcome: 'execution_completed' },
   ])(
-    'reports the execution failed: $executionFailed for a $logStatus log and $pauseStatus pause',
-    async ({ logStatus, pauseStatus, executionFailed }) => {
+    'reports $outcome for a $logStatus log and $pauseStatus pause',
+    async ({ logStatus, pauseStatus, outcome }) => {
       queueTableRows(workflowExecutionLogs, [{ status: logStatus }])
       queueTableRows(pausedExecutions, [{ status: pauseStatus }])
       const managerInternals = PauseResumeManager as unknown as PauseResumeManagerInternals
 
-      await expect(managerInternals.markResumeFailed(attemptArgs)).resolves.toBe(executionFailed)
+      await expect(managerInternals.markResumeFailed(attemptArgs)).resolves.toBe(outcome)
     }
   )
 
   it.each([
-    { logStatus: 'completed', updated: [resumeQueue] },
+    { logStatus: 'completed', updated: [resumeQueue, pausedExecutions] },
     { logStatus: 'failed', updated: [resumeQueue, pausedExecutions] },
     { logStatus: 'running', updated: [resumeQueue, pausedExecutions, workflowExecutionLogs] },
   ])(
@@ -348,7 +348,7 @@ describe('what a failed resume did to its paused execution', () => {
           snapshotSeed: options.snapshotSeed,
           metadata: { executionId: 'parent-execution-1', duration: 1, startTime: 'start' },
         }),
-        vi.spyOn(managerInternals, 'markResumeFailed').mockResolvedValueOnce(true),
+        vi.spyOn(managerInternals, 'markResumeFailed').mockResolvedValueOnce('execution_failed'),
         vi.spyOn(LoggingSession, 'markExecutionAsFailed').mockResolvedValueOnce(),
         vi.spyOn(PauseResumeManager, 'processQueuedResumes').mockResolvedValueOnce()
       )
@@ -390,13 +390,14 @@ describe('what a failed resume did to its paused execution', () => {
     const rawError = new Error('Block failed')
     const spies: { mockRestore: () => void }[] = []
 
-    function failRun(options: { executionFailed: boolean; queuedResumesError?: Error }) {
+    function failRun(options: {
+      outcome: FailedResumeOutcome | undefined
+      queuedResumesError?: Error
+    }) {
       const managerInternals = PauseResumeManager as unknown as PauseResumeManagerInternals
       spies.push(
         vi.spyOn(managerInternals, 'runResumeExecution').mockRejectedValueOnce(rawError),
-        vi
-          .spyOn(managerInternals, 'markResumeFailed')
-          .mockResolvedValueOnce(options.executionFailed),
+        vi.spyOn(managerInternals, 'markResumeFailed').mockResolvedValueOnce(options.outcome),
         options.queuedResumesError
           ? vi
               .spyOn(PauseResumeManager, 'processQueuedResumes')
@@ -410,12 +411,13 @@ describe('what a failed resume did to its paused execution', () => {
     })
 
     it.each([
-      { executionFailed: true, reported: ['execution_failed'] },
-      { executionFailed: false, reported: [] },
+      { outcome: 'execution_failed' as const, reported: ['execution_failed'] },
+      { outcome: 'execution_completed' as const, reported: ['execution_completed'] },
+      { outcome: undefined, reported: [] },
     ])(
-      'reports $reported when it failed the execution: $executionFailed',
-      async ({ executionFailed, reported }) => {
-        failRun({ executionFailed })
+      'reports $reported when the attempt left the execution $outcome',
+      async ({ outcome, reported }) => {
+        failRun({ outcome })
         const { outcomes, args } = argsReportingOutcomes()
 
         await expect(PauseResumeManager.startResumeExecution(args)).rejects.toBe(rawError)
@@ -424,7 +426,7 @@ describe('what a failed resume did to its paused execution', () => {
     )
 
     it('reports the outcome before draining queued resumes, which may throw', async () => {
-      failRun({ executionFailed: true, queuedResumesError: new Error('queue drain failed') })
+      failRun({ outcome: 'execution_failed', queuedResumesError: new Error('queue drain failed') })
       const { outcomes, args } = argsReportingOutcomes()
 
       await expect(PauseResumeManager.startResumeExecution(args)).rejects.toThrow(
@@ -434,7 +436,7 @@ describe('what a failed resume did to its paused execution', () => {
     })
 
     it('rethrows the run failure when the outcome handler fails', async () => {
-      failRun({ executionFailed: true })
+      failRun({ outcome: 'execution_failed' })
       const { args } = argsReportingOutcomes(async () => {
         throw new Error('Database unavailable')
       })

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

@@ -149,11 +149,12 @@ class ResumeAdmissionError extends Error {
 }
 
 /**
- * What a failed resume attempt did to its paused execution, as reported by the
- * transaction that settled the attempt: the pause stayed resumable, or the
- * resumed run failed the execution.
+ * What a failed resume attempt left its paused execution as, read from the
+ * transaction that settled the attempt: the pause stayed resumable, the
+ * resumed run failed the execution, or the run completed before a later step
+ * of the attempt threw.
  */
-export type FailedResumeOutcome = 'pause_retained' | 'execution_failed'
+export type FailedResumeOutcome = 'pause_retained' | 'execution_failed' | 'execution_completed'
 
 /** Matches the paused execution mode to the deployment recorded on its durable root log. */
 export function requireResumeDeploymentVersion(
@@ -1032,14 +1033,13 @@ export class PauseResumeManager {
         })
         if (pauseResumable) outcome = 'pause_retained'
       } else {
-        const executionFailed = await PauseResumeManager.markResumeFailed({
+        outcome = await PauseResumeManager.markResumeFailed({
           resumeEntryId,
           pausedExecutionId: pausedExecution.id,
           parentExecutionId: pausedExecution.executionId,
           contextId,
           failureReason: message,
         })
-        if (executionFailed) outcome = 'execution_failed'
       }
       if (outcome && onAttemptFailed) {
         await onAttemptFailed(outcome, error).catch((hookError: unknown) => {
@@ -2228,7 +2228,7 @@ export class PauseResumeManager {
     parentExecutionId: string
     contextId: string
     failureReason: string
-  }): Promise<boolean> {
+  }): Promise<'execution_failed' | 'execution_completed' | undefined> {
     const now = new Date()
 
     return execDb.transaction(async (tx) => {
@@ -2260,12 +2260,9 @@ export class PauseResumeManager {
             .set({ status: 'cancelled', updatedAt: now, nextResumeAt: null })
             .where(eq(pausedExecutions.id, args.pausedExecutionId))
         }
-        return false
+        return undefined
       }
 
-      /** The run completed before a later step threw; its outcome stands. */
-      if (executionLog?.status === 'completed') return false
-
       await tx
         .update(pausedExecutions)
         .set({
@@ -2274,7 +2271,10 @@ export class PauseResumeManager {
         })
         .where(eq(pausedExecutions.id, args.pausedExecutionId))
 
-      if (pausedExecution?.status === 'cancelling') return false
+      if (pausedExecution?.status === 'cancelling') return undefined
+
+      /** The run completed before a later step of the attempt threw; its outcome stands. */
+      if (executionLog?.status === 'completed') return 'execution_completed'
 
       if (executionLog?.status !== 'failed') {
         await tx
@@ -2288,7 +2288,7 @@ export class PauseResumeManager {
           )
       }
 
-      return true
+      return 'execution_failed'
     })
   }