fix(copilot): derive the run phase from what the executor saw, not the result's shape

Nine review rounds found the same class of defect, which makes it a design
problem rather than nine bugs. "Did a side effect occur" had two sources
that disagreed: a precise marker on the thrown path, and on the returned
path an inference from whatever the outcome happened to look like. Every
property used for that inference is a proxy that breaks on the paths that
matter — an engine failing before its first block still carries an
ExecutionResult, and a run that ends without one still ran every block it
had — so each round found another path where the proxy lied.

There is now one source. The engine reports the moment a block handler is
first about to run, which is terminal: no fallible step remains between it
and the handler, so there is nothing left for a later reviewer to find in
front of it. The signal is threaded to the caller and recorded against the
outcome, and the copilot adapter reads it on every exit path instead of
inspecting status or the presence of an attached result.

The phase then follows from two stated facts rather than a guess: nothing
dispatched is not_attempted whatever the result looks like, a run that
stopped partway is attempted, and one that reached the end is performed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: icecrasher321

apps/sim/executor/execution/engine.ts

@@ -38,6 +38,7 @@ export class ExecutionEngine {
   private cancellationController = new AbortController()
   private abortSignalListener: (() => void) | null = null
   private cancellationUnsubscribe: (() => void) | null = null
+  private reportedBlocksMayRun = false
   private execLogger: Logger
 
   constructor(
@@ -62,6 +63,13 @@ export class ExecutionEngine {
     this.initializeAbortHandler()
   }
 
+  /** Fires the caller's dispatch observer exactly once, however many nodes follow. */
+  private reportBlocksMayRun(): void {
+    if (this.reportedBlocksMayRun) return
+    this.reportedBlocksMayRun = true
+    this.context.onBlocksMayRun?.()
+  }
+
   private async subscribeToCancellationSignal(): Promise<void> {
     if (!this.context.executionId) return
     const executionId = this.context.executionId
@@ -107,15 +115,6 @@ export class ExecutionEngine {
       this.initializeQueue(triggerBlockId)
       await this.subscribeToCancellationSignal()
 
-      /**
-       * Past every fallible startup step — DAG construction, pipeline assembly and the
-       * cancellation subscription above all reject having run nothing — and immediately
-       * before the loop that processes blocks. This is the line a caller means by "could a
-       * side effect have occurred"; anything earlier reports a run for a request that was
-       * merely refused.
-       */
-      this.context.onBlocksMayRun?.()
-
       while (this.hasWork()) {
         if (this.checkCancellation() || this.errorFlag || this.stoppedEarlyFlag) {
           break
@@ -430,6 +429,14 @@ export class ExecutionEngine {
   private async executeNodeAsync(nodeId: string): Promise<void> {
     try {
       const wasAlreadyExecuted = this.context.executedBlocks.has(nodeId)
+      /**
+       * The single moment a side effect becomes possible: the last statement before a block
+       * handler runs. Every earlier candidate was a proxy that a reviewer could then find a
+       * fallible step in front of — startup, the cancellation subscription, queue and
+       * subflow initialization all reject having run nothing. There is nothing between here
+       * and the handler, so there is nothing left to be in front of.
+       */
+      this.reportBlocksMayRun()
       const result = await this.nodeOrchestrator.executeNode(this.context, nodeId)
 
       if (!wasAlreadyExecuted) {

apps/sim/executor/utils/errors.ts

@@ -67,6 +67,27 @@ export function attachAttemptedExecutionId(error: unknown, executionId: string):
   attemptedExecutionIds.set(error, executionId)
 }
 
+/**
+ * Whether a block handler ran, keyed by the outcome that carries it — a result or a throw.
+ *
+ * Recorded rather than inferred. Every property of an outcome that looks like it answers
+ * this is a proxy that disagrees on the paths that matter: an engine failing before its
+ * first block still carries an `ExecutionResult`, and a run that ends without one still ran
+ * every block it had. Only the executor knows, so only the executor says.
+ */
+const observedBlockDispatch = new WeakMap<object, boolean>()
+
+/** Records the executor's answer against the outcome a caller will read it from. */
+export function recordBlocksMayHaveRun(outcome: unknown, blocksMayHaveRun: boolean): void {
+  if (!isRecordedThrown(outcome)) return
+  observedBlockDispatch.set(outcome, blocksMayHaveRun)
+}
+
+/** Undefined when nothing observed this run, which callers must treat conservatively. */
+export function readBlocksMayHaveRun(outcome: unknown): boolean | undefined {
+  return isRecordedThrown(outcome) ? observedBlockDispatch.get(outcome) : undefined
+}
+
 /** Reads the dispatched-run id a thrown value carries, if dispatch was reached at all. */
 export function readAttemptedExecutionId(error: unknown): string | undefined {
   return isRecordedThrown(error) ? attemptedExecutionIds.get(error) : undefined

apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts

@@ -10,6 +10,7 @@ const { mocks } = vi.hoisted(() => ({
     executeWorkflowUseCase: vi.fn(),
     hasExecutionResult: vi.fn(),
     readAttemptedExecutionId: vi.fn(),
+    readBlocksMayHaveRun: vi.fn(),
   },
 }))
 
@@ -30,6 +31,7 @@ vi.mock('@/lib/workflows/sanitization/json-sanitizer', () => ({
 vi.mock('@/executor/utils/errors', () => ({
   hasExecutionResult: mocks.hasExecutionResult,
   readAttemptedExecutionId: mocks.readAttemptedExecutionId,
+  readBlocksMayHaveRun: mocks.readBlocksMayHaveRun,
 }))
 
 vi.mock('@/lib/core/telemetry', () => ({
@@ -60,6 +62,8 @@ describe('workflow mutation Copilot adapters', () => {
     vi.clearAllMocks()
     mocks.hasExecutionResult.mockReturnValue(false)
     mocks.readAttemptedExecutionId.mockReturnValue(undefined)
+    // Default: the executor saw a block run, which is the ordinary case.
+    mocks.readBlocksMayHaveRun.mockReturnValue(true)
   })
 
   it('maps encoded folder aliases into one create application command', async () => {
@@ -295,6 +299,20 @@ describe('workflow mutation Copilot adapters', () => {
       run: () => executeRunWorkflow({ workflowId: 'workflow-1' }, context),
       effect: { phase: 'attempted', ids: { executionId: 'execution-1' } },
     },
+    {
+      label: 'refused by the engine before any block ran',
+      arrange: () => {
+        mocks.readBlocksMayHaveRun.mockReturnValue(false)
+        mocks.executeWorkflowUseCase.mockResolvedValueOnce({
+          success: false,
+          output: {},
+          logs: [],
+          metadata: { executionId: 'execution-1' },
+        })
+      },
+      run: () => executeRunWorkflow({ workflowId: 'workflow-1' }, context),
+      effect: { phase: 'not_attempted', ids: { executionId: 'execution-1' } },
+    },
     {
       label: 'cancelled before it could finish',
       arrange: () =>

apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts

@@ -29,7 +29,11 @@ import {
   setWorkflowBlockEnabled,
 } from '@/lib/workflows/application/update-workflow-content'
 import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer'
-import { hasExecutionResult, readAttemptedExecutionId } from '@/executor/utils/errors'
+import {
+  hasExecutionResult,
+  readAttemptedExecutionId,
+  readBlocksMayHaveRun,
+} from '@/executor/utils/errors'
 import type { WorkflowState } from '@/stores/workflows/workflow/types'
 
 function stripBinaryFields(value: unknown): unknown {
@@ -61,19 +65,30 @@ function runRejected(error: string): ToolCallResult {
 }
 
 /**
- * `performed` claims the run reached the end of its work, so only a run that terminated on
- * its own may carry it. A cancelled or paused one stopped partway — it may have run every
- * block, one, or none — and `attempted` is the phase that says exactly that: an execution
- * exists under this id, resolve it before deciding anything.
+ * Derives the phase from two facts, neither of them guessed.
+ *
+ * `blocksMayHaveRun` comes from the executor and is the only thing that separates a run
+ * with side effects from a request that was refused; the status says whether that run
+ * reached the end of its work. Undefined means nothing observed the run, which is treated
+ * as "it may have" — over-reporting costs a lookup, under-reporting duplicates work.
  */
-function executionPhase(status: ExecutionResultStatus): ToolEffectPhase {
+function executionPhase(
+  blocksMayHaveRun: boolean | undefined,
+  status: ExecutionResultStatus
+): ToolEffectPhase {
+  if (blocksMayHaveRun === false) return TOOL_EFFECT_PHASE.notAttempted
   return status === 'cancelled' || status === 'paused'
     ? TOOL_EFFECT_PHASE.attempted
     : TOOL_EFFECT_PHASE.performed
 }
 
 type ExecutionResultStatus = 'completed' | 'paused' | 'cancelled' | undefined
 
+/** The phase of a run that returned, from what the executor observed about it. */
+function runPhase(result: { status?: ExecutionResultStatus }): ToolEffectPhase {
+  return executionPhase(readBlocksMayHaveRun(result), result.status)
+}
+
 function buildExecutionOutput(
   result: {
     success: boolean
@@ -83,6 +98,7 @@ function buildExecutionOutput(
     error?: string
     status?: ExecutionResultStatus
   },
+  phase: ToolEffectPhase,
   extra?: Record<string, unknown>
 ): ToolCallResult {
   return {
@@ -95,17 +111,25 @@ function buildExecutionOutput(
       logs: stripBinaryFields(result.logs),
     },
     error: result.success ? undefined : result.error || 'Workflow execution failed',
-    effect: executionEffect(executionPhase(result.status), result.metadata?.executionId),
+    effect: executionEffect(phase, result.metadata?.executionId),
   }
 }
 
 function buildExecutionError(error: unknown): ToolCallResult {
   if (hasExecutionResult(error)) {
-    return buildExecutionOutput({
-      ...error.executionResult,
-      success: false,
-      error: error.executionResult.error || 'Workflow execution failed',
-    })
+    return buildExecutionOutput(
+      {
+        ...error.executionResult,
+        success: false,
+        error: error.executionResult.error || 'Workflow execution failed',
+      },
+      /**
+       * Read off the error, not the spread copy above: the executor recorded its answer
+       * against the value it threw, and spreading makes a new object the record cannot
+       * follow.
+       */
+      executionPhase(readBlocksMayHaveRun(error), error.executionResult.status)
+    )
   }
   logger.error('Copilot workflow execution command failed', { error })
   /**
@@ -267,7 +291,7 @@ export async function executeRunWorkflow(
       lifecycle: copilotRunLifecycle(context),
     })
 
-    return buildExecutionOutput(result)
+    return buildExecutionOutput(result, runPhase(result))
   } catch (error) {
     return buildExecutionError(error)
   }
@@ -389,7 +413,9 @@ export async function executeRunWorkflowUntilBlock(
       lifecycle: copilotRunLifecycle(context),
     })
 
-    return buildExecutionOutput(result, { stoppedAfterBlockId: params.stopAfterBlockId })
+    return buildExecutionOutput(result, runPhase(result), {
+      stoppedAfterBlockId: params.stopAfterBlockId,
+    })
   } catch (error) {
     return buildExecutionError(error)
   }
@@ -464,7 +490,7 @@ export async function executeRunFromBlock(
       lifecycle: copilotRunLifecycle(context),
     })
 
-    return buildExecutionOutput(result, { startBlockId: params.startBlockId })
+    return buildExecutionOutput(result, runPhase(result), { startBlockId: params.startBlockId })
   } catch (error) {
     return buildExecutionError(error)
   }
@@ -550,7 +576,7 @@ export async function executeRunBlock(
       lifecycle: copilotRunLifecycle(context),
     })
 
-    return buildExecutionOutput(result, { blockId: params.blockId })
+    return buildExecutionOutput(result, runPhase(result), { blockId: params.blockId })
   } catch (error) {
     return buildExecutionError(error)
   }

apps/sim/lib/workflows/application/run-workflow-from-copilot.ts

@@ -29,7 +29,7 @@ import {
 } from '@/lib/workflows/triggers/run-options'
 import type { SerializableExecutionState } from '@/executor/execution/types'
 import type { ExecutionResult } from '@/executor/types'
-import { attachAttemptedExecutionId } from '@/executor/utils/errors'
+import { attachAttemptedExecutionId, recordBlocksMayHaveRun } from '@/executor/utils/errors'
 
 const logger = createLogger('CopilotWorkflowRun')
 
@@ -251,6 +251,7 @@ async function executeCopilotRun(params: {
   )
   const completePendingActivation = registry?.beginPendingActivation()
   let runReturned = false
+  let blocksMayHaveRun = false
   try {
     const result = await executeWorkflow(
       {
@@ -273,6 +274,17 @@ async function executeCopilotRun(params: {
         stopAfterBlockId: params.stopAfterBlockId,
         runFromBlock: params.runFromBlock,
         abortSignal: params.input.lifecycle.abortSignal,
+        /**
+         * Whether a block could have run, stated by the executor rather than inferred here.
+         * Every earlier attempt read it off the shape of the outcome — a status field, or
+         * whether an ExecutionResult rode along on the error — and those are proxies that
+         * disagree with reality on exactly the paths that matter: an engine that fails
+         * before its first block still carries a result, and a run that ends without one
+         * still ran every block it had.
+         */
+        onBlocksMayRun: () => {
+          blocksMayHaveRun = true
+        },
         billingAttribution: admission.billingAttribution,
         ...(trustedInitialResolvedSecretTraceProvenance
           ? { trustedInitialResolvedSecretTraceProvenance }
@@ -293,6 +305,7 @@ async function executeCopilotRun(params: {
       childExecutionId
     )
     runReturned = true
+    recordBlocksMayHaveRun(result, blocksMayHaveRun)
     if (registry) {
       await registry.importCrossingProvenance(
         result.executionState?.resolvedSecretTraceProvenance,
@@ -309,6 +322,7 @@ async function executeCopilotRun(params: {
      * what threw and an execution certainly exists.
      */
     if (runReturned) attachAttemptedExecutionId(error, childExecutionId)
+    recordBlocksMayHaveRun(error, blocksMayHaveRun)
     /**
      * Recovery must never replace the failure it is describing. Both steps below run only to
      * record and release, and either throwing would propagate a different error — one the

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

@@ -52,6 +52,12 @@ export interface ExecuteWorkflowOptions {
   useDraftState?: boolean
   /** Stop execution after this block completes. Used for "run until block" feature. */
   stopAfterBlockId?: string
+  /**
+   * Fired once, when a block handler is first about to run. The only fact that answers
+   * "could a side effect have occurred", and the only one the executor can state rather
+   * than have inferred from the shape of a result.
+   */
+  onBlocksMayRun?: () => void
   /** Run-from-block configuration using a prior execution snapshot. */
   runFromBlock?: {
     startBlockId: string
@@ -203,6 +209,7 @@ export async function executeWorkflow(
       base64MaxBytes: streamConfig?.base64MaxBytes,
       abortSignal: streamConfig?.abortSignal,
       stopAfterBlockId: streamConfig?.stopAfterBlockId,
+      onBlocksMayRun: streamConfig?.onBlocksMayRun,
       trustedInitialResolvedSecretTraceProvenance:
         streamConfig?.trustedInitialResolvedSecretTraceProvenance,
       runFromBlock: streamConfig?.runFromBlock,

apps/sim/lib/workflows/executor/execution-core.ts

@@ -117,6 +117,7 @@ export interface ExecuteWorkflowCoreOptions {
   includeFileBase64?: boolean
   base64MaxBytes?: number
   stopAfterBlockId?: string
+  onBlocksMayRun?: () => void
   /** Trusted encrypted provenance captured by a server-only pre-execution boundary. */
   trustedInitialResolvedSecretTraceProvenance?: ResolvedSecretTraceProvenanceV1
   /** Immutable deployment admitted by the durable parent log for a resumed execution. */
@@ -973,6 +974,7 @@ async function executeWorkflowCoreImpl(
        */
       onBlocksMayRun: () => {
         executorStarted = true
+        options.onBlocksMayRun?.()
       },
       stream: !!onStream,
       selectedOutputs,