Skip to content

Commit c1a4e30

Browse files
icecrasher321claude
andcommitted
fix(copilot): let the executor say when a block could first run
Review was right that entering `execute` is still too early: DAG construction, snapshot restoration and pipeline assembly all happen inside it and reject a malformed graph having changed nothing, so a validation failure reported a run to resolve. Only the executor knows where that line falls, so it reports it. A `onBlocksMayRun` context extension fires immediately before `engine.run` on both entry points, and execution-core records the run from there rather than guessing at it from outside. A rejected graph now correctly says nothing started. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent cb48743 commit c1a4e30

3 files changed

Lines changed: 25 additions & 8 deletions

File tree

apps/sim/executor/execution/executor.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export class DAGExecutor {
101101
this.registerRestoredClonedSubflows(context.subflowParentMap, restoredClonedSubflows)
102102

103103
const engine = this.buildExecutionPipeline(context, dag, state)
104+
this.contextExtensions.onBlocksMayRun?.()
104105
return await engine.run(triggerBlockId)
105106
}
106107

@@ -258,6 +259,7 @@ export class DAGExecutor {
258259
context.subflowParentMap = this.buildSubflowParentMap(dag)
259260

260261
const engine = this.buildExecutionPipeline(context, dag, state, filteredSnapshot)
262+
this.contextExtensions.onBlocksMayRun?.()
261263
const result = await engine.run()
262264
if (result.metadata) {
263265
result.metadata.largeValueKeys = context.largeValueKeys

apps/sim/executor/execution/types.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,16 @@ export interface PiiBlockOutputRedaction {
238238
}
239239

240240
export interface ContextExtensions {
241+
/**
242+
* Fired once, immediately before the engine may run a block.
243+
*
244+
* Everything the executor does first — DAG construction, snapshot restoration, pipeline
245+
* assembly — can reject a request having changed nothing, so a caller that needs to know
246+
* whether a side effect was possible cannot infer it from having called `execute`. Only
247+
* the executor knows where that line falls, so it reports it rather than being guessed at
248+
* from the outside.
249+
*/
250+
onBlocksMayRun?: () => void
241251
workspaceId?: string
242252
executionId?: string
243253
largeValueExecutionIds?: string[]

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,19 @@ async function executeWorkflowCoreImpl(
961961

962962
const principalSubject = resolvePrincipalSubject(metadata.principal)
963963
const contextExtensions: ContextExtensions = {
964+
/**
965+
* The only honest answer to "could a side effect have occurred", and the executor is
966+
* the only thing that knows it: everything before this — DAG construction, snapshot
967+
* restoration, pipeline assembly — can reject a request having changed nothing.
968+
*
969+
* The logging session was the wrong proxy in both directions. `safeStart`'s result is
970+
* never checked, so blocks run even when it fails, reporting that nothing started for
971+
* a run that did; and it flips before trigger resolution and serialization, reporting
972+
* a run for failures that never reached a block.
973+
*/
974+
onBlocksMayRun: () => {
975+
executorStarted = true
976+
},
964977
stream: !!onStream,
965978
selectedOutputs,
966979
executionId,
@@ -1052,14 +1065,6 @@ async function executeWorkflowCoreImpl(
10521065
contextExtensions,
10531066
})
10541067

1055-
/**
1056-
* The last statement before a block can run, and therefore the only honest answer to
1057-
* "could a side effect have occurred". The logging session is the wrong proxy in both
1058-
* directions: `safeStart`'s result is never checked, so blocks execute even when it
1059-
* fails — reporting nothing started for a run that did — and it flips before trigger
1060-
* resolution and serialization, reporting a run for failures that never reached a block.
1061-
*/
1062-
executorStarted = true
10631068
const result = runFromBlock
10641069
? ((await executorInstance.executeFromBlock(
10651070
workflowId,

0 commit comments

Comments
 (0)