refactor(copilot): decide the run phase where the caller lives, not in the executor

Ten review rounds chased the same question — when exactly may a side effect
have occurred — through six positions in the executor, ending with a
callback on every block of every execution in the product. Against 543k
executions a week, serving a disclosure read about fifty times a week. The
precision was never the point: `attempted` and `performed` both mean an
execution exists under this id, and the caller was already handed the id
that resolves it.

Revert all of it. The engine, the orchestrator, both context types and the
callback threading through execute-workflow and execution-core go back to
staging untouched; the executor's only remaining change is the id carrier
in utils/errors.ts.

The phase now comes from what the copilot layer already holds. Its `try`
opens on the executor call, so everything it catches is post-dispatch by
construction while authorization, admission and provenance export throw
past it having created nothing — no id means nothing exists, an id means
resolve it. A result in hand says how the run ended, which separates
cancelled and paused from completed.

The harness that motivated this is now in the diff: every outcome the run
path can produce, driven through the real handler and the real projection,
asserted on the retry decision a caller can reach and on no run content
crossing. Six mutations were used to confirm it fails for the right
reasons; one of them found the dispatch flag this refactor introduced was
already dead, and it is gone.

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

Author: icecrasher321

apps/sim/executor/execution/executor.ts

@@ -415,7 +415,6 @@ export class DAGExecutor {
 
     const context: ExecutionContext = {
       workflowId,
-      onBlocksMayRun: this.contextExtensions.onBlocksMayRun,
       workspaceId: this.contextExtensions.workspaceId,
       executionId: this.contextExtensions.executionId,
       largeValueExecutionIds: this.contextExtensions.largeValueExecutionIds,

apps/sim/executor/execution/types.ts

@@ -238,16 +238,6 @@ export interface PiiBlockOutputRedaction {
 }
 
 export interface ContextExtensions {
-  /**
-   * Fired once, immediately before the engine may run a block.
-   *
-   * Everything the executor does first — DAG construction, snapshot restoration, pipeline
-   * assembly — can reject a request having changed nothing, so a caller that needs to know
-   * whether a side effect was possible cannot infer it from having called `execute`. Only
-   * the executor knows where that line falls, so it reports it rather than being guessed at
-   * from the outside.
-   */
-  onBlocksMayRun?: () => void
   workspaceId?: string
   executionId?: string
   largeValueExecutionIds?: string[]

apps/sim/executor/orchestrators/node.ts

@@ -95,16 +95,6 @@ export class NodeExecutionOrchestrator {
       }
     }
 
-    /**
-     * The block handler, and therefore the first moment a side effect is possible. Every
-     * earlier position was a proxy with something in front of it — engine startup, the
-     * cancellation subscription, queue setup, and above this line a cache hit, loop and
-     * parallel scope initialization, and a sentinel that returns without reaching a handler.
-     * Nothing separates this call from the handler, so nothing can precede it.
-     *
-     * Fired per block rather than once; observers record a boolean, so repeats are free.
-     */
-    ctx.onBlocksMayRun?.()
     const output = await this.blockExecutor.execute(ctx, node, node.block)
     const isFinalOutput = node.outgoingEdges.size === 0
     return {

apps/sim/executor/types.ts

@@ -372,8 +372,6 @@ export interface ExecutorDelegationOrigin {
 }
 
 export interface ExecutionContext {
-  /** See {@link ContextExtensions.onBlocksMayRun}. Fired by the engine, once. */
-  onBlocksMayRun?: () => void
   workflowId: string
   workspaceId?: string
   executionId?: string

apps/sim/executor/utils/errors.ts

@@ -67,27 +67,6 @@ 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,7 +10,6 @@ const { mocks } = vi.hoisted(() => ({
     executeWorkflowUseCase: vi.fn(),
     hasExecutionResult: vi.fn(),
     readAttemptedExecutionId: vi.fn(),
-    readBlocksMayHaveRun: vi.fn(),
   },
 }))
 
@@ -31,7 +30,6 @@ 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', () => ({
@@ -62,8 +60,6 @@ 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 () => {
@@ -299,20 +295,6 @@ 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,11 +29,7 @@ import {
   setWorkflowBlockEnabled,
 } from '@/lib/workflows/application/update-workflow-content'
 import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer'
-import {
-  hasExecutionResult,
-  readAttemptedExecutionId,
-  readBlocksMayHaveRun,
-} from '@/executor/utils/errors'
+import { hasExecutionResult, readAttemptedExecutionId } from '@/executor/utils/errors'
 import type { WorkflowState } from '@/stores/workflows/workflow/types'
 
 function stripBinaryFields(value: unknown): unknown {
@@ -65,30 +61,24 @@ function runRejected(error: string): ToolCallResult {
 }
 
 /**
- * Derives the phase from two facts, neither of them guessed.
+ * The phase of a run whose result came back, from how that run ended.
+ *
+ * A result in hand means the executor reached a terminal state and recorded it, so the
+ * caller can read the whole story by id — `performed`. Cancelled and paused stopped partway
+ * and may have run every block, one, or none, which is exactly what `attempted` says.
  *
- * `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.
+ * Deliberately does not separate "ran no blocks" from "ran some". Establishing that would
+ * take a callback on every block of every execution in the product, and buys the caller
+ * nothing it cannot get by resolving the id it was already handed.
  */
-function executionPhase(
-  blocksMayHaveRun: boolean | undefined,
-  status: ExecutionResultStatus
-): ToolEffectPhase {
-  if (blocksMayHaveRun === false) return TOOL_EFFECT_PHASE.notAttempted
+function settledPhase(status: ExecutionResultStatus): ToolEffectPhase {
   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
@@ -123,12 +113,7 @@ function buildExecutionError(error: unknown): ToolCallResult {
         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)
+      settledPhase(error.executionResult.status)
     )
   }
   logger.error('Copilot workflow execution command failed', { error })
@@ -291,7 +276,7 @@ export async function executeRunWorkflow(
       lifecycle: copilotRunLifecycle(context),
     })
 
-    return buildExecutionOutput(result, runPhase(result))
+    return buildExecutionOutput(result, settledPhase(result.status))
   } catch (error) {
     return buildExecutionError(error)
   }
@@ -413,7 +398,7 @@ export async function executeRunWorkflowUntilBlock(
       lifecycle: copilotRunLifecycle(context),
     })
 
-    return buildExecutionOutput(result, runPhase(result), {
+    return buildExecutionOutput(result, settledPhase(result.status), {
       stoppedAfterBlockId: params.stopAfterBlockId,
     })
   } catch (error) {
@@ -490,7 +475,9 @@ export async function executeRunFromBlock(
       lifecycle: copilotRunLifecycle(context),
     })
 
-    return buildExecutionOutput(result, runPhase(result), { startBlockId: params.startBlockId })
+    return buildExecutionOutput(result, settledPhase(result.status), {
+      startBlockId: params.startBlockId,
+    })
   } catch (error) {
     return buildExecutionError(error)
   }
@@ -576,7 +563,7 @@ export async function executeRunBlock(
       lifecycle: copilotRunLifecycle(context),
     })
 
-    return buildExecutionOutput(result, runPhase(result), { blockId: params.blockId })
+    return buildExecutionOutput(result, settledPhase(result.status), { blockId: params.blockId })
   } catch (error) {
     return buildExecutionError(error)
   }