Skip to content

Commit 74a1f06

Browse files
icecrasher321claude
andcommitted
fix(copilot): report dispatch from the block handler itself
I claimed last round that nothing could precede the signal. That was wrong: `executeNode` returns early on a cache hit, initializes loop and parallel scopes, and handles a sentinel that never reaches a handler — all after the point it fired. Both reviewers found the same thing. Move it to the line before `blockExecutor.execute`, which is the handler call. Nothing separates the two, so unlike every previous position this one cannot have something in front of it. Fired per block rather than once, since observers record a boolean and repeats cost nothing. Also accept functions as carriers of the run markers. They key a WeakMap exactly as objects do, so excluding them dropped the record for a thrown function and lost the distinction the markers exist to make. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d27d944 commit 74a1f06

3 files changed

Lines changed: 12 additions & 17 deletions

File tree

apps/sim/executor/execution/engine.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ export class ExecutionEngine {
3838
private cancellationController = new AbortController()
3939
private abortSignalListener: (() => void) | null = null
4040
private cancellationUnsubscribe: (() => void) | null = null
41-
private reportedBlocksMayRun = false
4241
private execLogger: Logger
4342

4443
constructor(
@@ -63,13 +62,6 @@ export class ExecutionEngine {
6362
this.initializeAbortHandler()
6463
}
6564

66-
/** Fires the caller's dispatch observer exactly once, however many nodes follow. */
67-
private reportBlocksMayRun(): void {
68-
if (this.reportedBlocksMayRun) return
69-
this.reportedBlocksMayRun = true
70-
this.context.onBlocksMayRun?.()
71-
}
72-
7365
private async subscribeToCancellationSignal(): Promise<void> {
7466
if (!this.context.executionId) return
7567
const executionId = this.context.executionId
@@ -429,14 +421,6 @@ export class ExecutionEngine {
429421
private async executeNodeAsync(nodeId: string): Promise<void> {
430422
try {
431423
const wasAlreadyExecuted = this.context.executedBlocks.has(nodeId)
432-
/**
433-
* The single moment a side effect becomes possible: the last statement before a block
434-
* handler runs. Every earlier candidate was a proxy that a reviewer could then find a
435-
* fallible step in front of — startup, the cancellation subscription, queue and
436-
* subflow initialization all reject having run nothing. There is nothing between here
437-
* and the handler, so there is nothing left to be in front of.
438-
*/
439-
this.reportBlocksMayRun()
440424
const result = await this.nodeOrchestrator.executeNode(this.context, nodeId)
441425

442426
if (!wasAlreadyExecuted) {

apps/sim/executor/orchestrators/node.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ export class NodeExecutionOrchestrator {
9595
}
9696
}
9797

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

apps/sim/executor/utils/errors.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ export function readAttemptedExecutionId(error: unknown): string | undefined {
102102
* costs nothing today because every throw site past the dispatch boundary raises an `Error`.
103103
*/
104104
function isRecordedThrown(value: unknown): value is object {
105-
return typeof value === 'object' && value !== null
105+
/** Functions key a WeakMap as well as objects do, so excluding them would drop the record. */
106+
return (typeof value === 'object' || typeof value === 'function') && value !== null
106107
}
107108

108109
export interface BlockExecutionErrorDetails {

0 commit comments

Comments
 (0)