fix(provenance): normalize a post-execution failure so it can carry the result

Round 3, cubic's finding accepted. The guard added last round required
the caught value to already be an `Error`, so a non-Error raised by
post-execution work skipped the attach and was rethrown bare — the same
hole this branch closed in the executor, left open one layer up by my
own change. A Copilot run would have reported an executed workflow as
never started and vouched for content it cannot describe.

Normalize once at the top of the catch and use that value throughout,
including the rethrow, matching what the executor does. `toError`
returns an `Error` unchanged, so a custom error class keeps its
identity and every ordinary failure is untouched — the existing
identity assertion on the rejection path still holds.

Two tests: the result reaches an ordinary post-execution failure, and a
non-Error one is normalized so it can carry the result too. The second
fails against the previous guard.

Author: icecrasher321

apps/sim/lib/workflows/executor/execute-workflow.test.ts

@@ -56,6 +56,7 @@ vi.mock('@/lib/workflows/executor/pause-persistence', () => ({
 }))
 
 import { executeWorkflow } from '@/lib/workflows/executor/execute-workflow'
+import { hasExecutionResult } from '@/executor/utils/errors'
 
 const workflowExecutionLoggerCallIndex = loggerMock.createLogger.mock.calls.findIndex(
   ([name]) => name === 'WorkflowExecution'
@@ -296,6 +297,44 @@ describe('executeWorkflow', () => {
     expect(executionSettled).toBe(true)
   })
 
+  /**
+   * Post-execution work runs after the core has produced a result and the executor never sees
+   * its failure, so this layer is the only one that can carry the result onto it. Callers read a
+   * missing result as proof that no block ran — a Copilot run would report an executed workflow
+   * as never started and vouch for content it cannot describe.
+   */
+  it('carries the execution result onto a post-execution failure', async () => {
+    const result = { success: true, output: { ran: true }, logs: [] }
+    executeWorkflowCoreMock.mockResolvedValueOnce(result)
+    handlePostExecutionPauseStateMock.mockRejectedValueOnce(new Error('pause persistence failed'))
+
+    const thrown = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
+      enabled: true,
+      principal,
+      billingAttribution,
+    }).catch((error: unknown) => error)
+
+    expect(hasExecutionResult(thrown)).toBe(true)
+    expect((thrown as { executionResult?: unknown }).executionResult).toBe(result)
+  })
+
+  /** A non-Error cannot carry the result, so it is normalized before anything reads it. */
+  it('normalizes a non-Error post-execution failure so it can carry the result', async () => {
+    const result = { success: true, output: { ran: true }, logs: [] }
+    executeWorkflowCoreMock.mockResolvedValueOnce(result)
+    handlePostExecutionPauseStateMock.mockRejectedValueOnce('pause persistence exploded')
+
+    const thrown = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
+      enabled: true,
+      principal,
+      billingAttribution,
+    }).catch((error: unknown) => error)
+
+    expect(thrown).toBeInstanceOf(Error)
+    expect(hasExecutionResult(thrown)).toBe(true)
+    expect((thrown as { executionResult?: unknown }).executionResult).toBe(result)
+  })
+
   it('transfers post-execution ownership with successful streaming metadata', async () => {
     const result = await executeWorkflow(workflow, 'request-1', undefined, 'actor-1', {
       enabled: true,

apps/sim/lib/workflows/executor/execute-workflow.ts

@@ -1,5 +1,6 @@
 import type { WorkflowExecutionPrincipal } from '@sim/auth/principal'
 import { createLogger } from '@sim/logger'
+import { toError } from '@sim/utils/errors'
 import { generateId } from '@sim/utils/id'
 import {
   assertBillingAttributionSnapshot,
@@ -247,14 +248,20 @@ export async function executeWorkflow(
     }
 
     return result
-  } catch (error: unknown) {
+  } catch (caught: unknown) {
+    /**
+     * Normalized before anything reads it, for the reason the executor normalizes its own throw:
+     * a value that cannot carry the result would otherwise reach callers bare, and they read a
+     * missing result as proof that no block ran. `toError` returns an `Error` unchanged, so a
+     * custom error class keeps its identity and every ordinary failure is untouched.
+     */
+    const error = toError(caught)
     /**
      * Carries the run's result on a failure raised after it produced one — the post-execution
-     * work below the executor call can throw, and callers read a missing result as proof that no
-     * block ran. Skipped when the executor already attached its own, which is the more specific
-     * record, and when the throw is not an object to carry it.
+     * work below the executor call can throw, and the executor never saw it. Skipped when the
+     * executor already attached its own, which is the more specific record.
      */
-    if (executionResult && error instanceof Error && !hasExecutionResult(error)) {
+    if (executionResult && !hasExecutionResult(error)) {
       attachExecutionResult(error, executionResult)
     }
     const errorDiagnostic = loggingSession.projectDiagnosticError(error)