fix(tables): mirror what a failed resume did instead of inferring it from the error

An admission refusal can also mean the execution already finished, so treating every refusal as a kept pause wrote paused over a completed cell. markResumeAttemptFailed and markResumeFailed now return what their transaction actually did (pause still resumable / execution failed), the manager records that outcome on the rethrown error, and the resume job writes paused, error, or nothing.

Author: waleedlatif1

apps/sim/background/resume-execution.ts

@@ -22,8 +22,8 @@ import { classifyWorkflowCellTerminalResult } from '@/lib/table/workflow-cell-re
 import type { CellResumeContext } from '@/lib/table/workflow-columns'
 import {
   createResumeAttemptTimeoutController,
+  getFailedResumeOutcome,
   PauseResumeManager,
-  wasPausedExecutionRetained,
 } from '@/lib/workflows/executor/human-in-the-loop-manager'
 import { RESUME_EXECUTION_CONCURRENCY_LIMIT } from '@/background/concurrency-limits'
 import { ExecutionSnapshot } from '@/executor/execution/snapshot'
@@ -364,12 +364,15 @@ 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 the execution instead: a pause the resume kept
- * resumable goes back to paused, and any other failure ended the run.
+ * partial `running` state. Mirror what the failed attempt did to the execution:
+ * a pause that stayed resumable goes back to paused, a failed execution fails
+ * the cell, and an attempt that changed nothing leaves the cell alone.
  */
 async function writeFailedResumeCellTerminal(writers: CellWriters, error: unknown): Promise<void> {
+  const outcome = getFailedResumeOutcome(error)
+  if (!outcome) return
   try {
-    if (wasPausedExecutionRetained(error)) {
+    if (outcome === 'pause_retained') {
       await writers.writeCellTerminal('paused', null)
     } else {
       await writers.writeCellTerminal('error', getErrorMessage(error, 'Resume execution failed'))

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

@@ -57,7 +57,7 @@ const mocks = {
   ...hoisted,
   getPausedExecutionById: humanInTheLoopManagerMockFns.mockGetPausedExecutionById,
   startResumeExecution: humanInTheLoopManagerMockFns.mockStartResumeExecution,
-  wasPausedExecutionRetained: humanInTheLoopManagerMockFns.mockWasPausedExecutionRetained,
+  getFailedResumeOutcome: humanInTheLoopManagerMockFns.mockGetFailedResumeOutcome,
   createResumeAttemptTimeoutController:
     humanInTheLoopManagerMockFns.mockCreateResumeAttemptTimeoutController,
   findCellContextByExecutionId: tableWorkflowColumnsMockFns.mockFindCellContextByExecutionId,
@@ -226,9 +226,10 @@ describe('resuming a paused table cell', () => {
       return payload?.executionState
     }
 
-    it('marks the cell failed when the resumed run itself failed', async () => {
+    it('marks the cell failed when the resume failed the execution', async () => {
       const runFailure = new Error('writeLedger: Unique constraint violation')
       mocks.startResumeExecution.mockRejectedValueOnce(runFailure)
+      mocks.getFailedResumeOutcome.mockReturnValueOnce('execution_failed')
 
       await expect(executeResumeJob(PAYLOAD)).rejects.toBe(runFailure)
 
@@ -242,7 +243,7 @@ describe('resuming a paused table cell', () => {
     it('puts the cell back to paused when the pause stayed resumable', async () => {
       const admissionRefusal = new Error('Execution can no longer be resumed')
       mocks.startResumeExecution.mockRejectedValueOnce(admissionRefusal)
-      mocks.wasPausedExecutionRetained.mockReturnValueOnce(true)
+      mocks.getFailedResumeOutcome.mockReturnValueOnce('pause_retained')
 
       await expect(executeResumeJob(PAYLOAD)).rejects.toBe(admissionRefusal)
 
@@ -253,9 +254,20 @@ describe('resuming a paused table cell', () => {
       })
     }, 20_000)
 
+    it('leaves the cell alone when the failed attempt changed nothing', async () => {
+      const staleRefusal = new Error('Execution can no longer be resumed')
+      mocks.startResumeExecution.mockRejectedValueOnce(staleRefusal)
+      mocks.getFailedResumeOutcome.mockReturnValueOnce(undefined)
+
+      await expect(executeResumeJob(PAYLOAD)).rejects.toBe(staleRefusal)
+
+      expect(lastCellExecutionState()).toBeUndefined()
+    }, 20_000)
+
     it('still reports the resume failure when the cell write also fails', async () => {
       const runFailure = new Error('Block failed')
       mocks.startResumeExecution.mockRejectedValueOnce(runFailure)
+      mocks.getFailedResumeOutcome.mockReturnValueOnce('execution_failed')
       mocks.writeWorkflowGroupState.mockRejectedValueOnce(new Error('Database unavailable'))
 
       await expect(executeResumeJob(PAYLOAD)).rejects.toBe(runFailure)

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

@@ -61,10 +61,10 @@ vi.mock('@/lib/execution/payloads/large-value-metadata', () => largeValueMetadat
 
 import {
   createResumeAttemptTimeoutController,
+  getFailedResumeOutcome,
   PauseResumeManager,
   requireResumeDeploymentVersion,
   updateResumeOutputInAggregationBuffers,
-  wasPausedExecutionRetained,
 } from '@/lib/workflows/executor/human-in-the-loop-manager'
 import { getAutomaticResumeWaitingMetadata } from '@/lib/workflows/executor/paused-execution-metadata'
 import { AUTOMATIC_RESUME_WAITING_REASON_MAX_LENGTH } from '@/lib/workflows/executor/resume-policy'
@@ -85,7 +85,7 @@ if (!humanInTheLoopLogger) {
 }
 
 interface PauseResumeManagerInternals {
-  markResumeFailed: (...args: unknown[]) => Promise<void>
+  markResumeFailed: (...args: unknown[]) => Promise<boolean>
   runResumeExecution: (...args: unknown[]) => Promise<unknown>
 }
 
@@ -211,7 +211,7 @@ describe('queued resume attempt deadlines', () => {
   })
 })
 
-describe('which failed resumes keep the pause resumable', () => {
+describe('what a failed resume did to its paused execution', () => {
   type StartResumeArgs = Parameters<typeof PauseResumeManager.startResumeExecution>[0]
   const pausedExecution = {
     id: 'paused-execution-1',
@@ -231,50 +231,87 @@ describe('which failed resumes keep the pause resumable', () => {
     resumeInput: { approved: true },
     userId: 'user-1',
   }
+  const attemptArgs = {
+    resumeEntryId: 'resume-entry-1',
+    pausedExecutionId: 'paused-execution-1',
+    parentExecutionId: 'parent-execution-1',
+    contextId: 'context-1',
+    failureReason: 'Execution can no longer be resumed',
+    preserveForRetry: true,
+  }
 
   beforeEach(() => {
     resetDbChainMock()
   })
 
-  it('keeps the pause resumable when the paused log can no longer be claimed', async () => {
-    const markResumeAttemptFailedSpy = vi
-      .spyOn(PauseResumeManager, 'markResumeAttemptFailed')
-      .mockResolvedValueOnce()
+  it('reports the pause still resumable when a refused attempt leaves it paused', async () => {
+    queueTableRows(workflowExecutionLogs, [{ status: 'paused' }])
+    queueTableRows(pausedExecutions, [{ automaticResumeRetryCount: 0, status: 'paused' }])
 
-    try {
-      const thrown = await PauseResumeManager.startResumeExecution(resumeArgs).catch(
-        (error: unknown) => error
-      )
+    await expect(PauseResumeManager.markResumeAttemptFailed(attemptArgs)).resolves.toBe(true)
+  })
 
-      expect(thrown).toMatchObject({ name: 'ResumeAdmissionError' })
-      expect(wasPausedExecutionRetained(thrown)).toBe(true)
-    } finally {
-      markResumeAttemptFailedSpy.mockRestore()
+  it.each(['completed', 'failed', 'cancelled'])(
+    'reports the pause not resumable when the execution is already %s',
+    async (logStatus) => {
+      queueTableRows(workflowExecutionLogs, [{ status: logStatus }])
+      queueTableRows(pausedExecutions, [{ automaticResumeRetryCount: 0, status: 'paused' }])
+
+      await expect(PauseResumeManager.markResumeAttemptFailed(attemptArgs)).resolves.toBe(false)
     }
-  })
+  )
 
-  it('does not keep the pause when the resumed run itself failed', async () => {
-    const rawError = new Error('Block failed')
-    const managerInternals = PauseResumeManager as unknown as PauseResumeManagerInternals
-    const runResumeExecutionSpy = vi
-      .spyOn(managerInternals, 'runResumeExecution')
-      .mockRejectedValueOnce(rawError)
-    const markResumeFailedSpy = vi
-      .spyOn(managerInternals, 'markResumeFailed')
-      .mockResolvedValueOnce()
-    const processQueuedResumesSpy = vi
-      .spyOn(PauseResumeManager, 'processQueuedResumes')
-      .mockResolvedValueOnce()
+  it.each([
+    { stillResumable: true, outcome: 'pause_retained' },
+    { stillResumable: false, outcome: undefined },
+  ])(
+    'records a refused attempt as $outcome when the pause is resumable: $stillResumable',
+    async ({ stillResumable, outcome }) => {
+      const markResumeAttemptFailedSpy = vi
+        .spyOn(PauseResumeManager, 'markResumeAttemptFailed')
+        .mockResolvedValueOnce(stillResumable)
+
+      try {
+        const thrown = await PauseResumeManager.startResumeExecution(resumeArgs).catch(
+          (error: unknown) => error
+        )
 
-    try {
-      await expect(PauseResumeManager.startResumeExecution(resumeArgs)).rejects.toBe(rawError)
-      expect(wasPausedExecutionRetained(rawError)).toBe(false)
-    } finally {
-      runResumeExecutionSpy.mockRestore()
-      markResumeFailedSpy.mockRestore()
-      processQueuedResumesSpy.mockRestore()
+        expect(thrown).toMatchObject({ name: 'ResumeAdmissionError' })
+        expect(getFailedResumeOutcome(thrown)).toBe(outcome)
+      } finally {
+        markResumeAttemptFailedSpy.mockRestore()
+      }
     }
-  })
+  )
+
+  it.each([
+    { executionFailed: true, outcome: 'execution_failed' },
+    { executionFailed: false, outcome: undefined },
+  ])(
+    'records a failed run as $outcome when it failed the execution: $executionFailed',
+    async ({ executionFailed, outcome }) => {
+      const rawError = new Error('Block failed')
+      const managerInternals = PauseResumeManager as unknown as PauseResumeManagerInternals
+      const runResumeExecutionSpy = vi
+        .spyOn(managerInternals, 'runResumeExecution')
+        .mockRejectedValueOnce(rawError)
+      const markResumeFailedSpy = vi
+        .spyOn(managerInternals, 'markResumeFailed')
+        .mockResolvedValueOnce(executionFailed)
+      const processQueuedResumesSpy = vi
+        .spyOn(PauseResumeManager, 'processQueuedResumes')
+        .mockResolvedValueOnce()
+
+      try {
+        await expect(PauseResumeManager.startResumeExecution(resumeArgs)).rejects.toBe(rawError)
+        expect(getFailedResumeOutcome(rawError)).toBe(outcome)
+      } finally {
+        runResumeExecutionSpy.mockRestore()
+        markResumeFailedSpy.mockRestore()
+        processQueuedResumesSpy.mockRestore()
+      }
+    }
+  )
 })
 
 describe('resume failure diagnostic projection', () => {