Skip to content

Commit 999e6f6

Browse files
committed
improvement(executor): make run-from-block refusals actionable
1 parent 2fc9739 commit 999e6f6

5 files changed

Lines changed: 255 additions & 34 deletions

File tree

‎apps/sim/executor/execution/executor.test.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,10 @@ describe('DAGExecutor run-from-block snapshot metadata', () => {
197197
version: '1',
198198
blocks: [
199199
createBlock('start', BlockType.STARTER),
200-
createBlock('producer', BlockType.FUNCTION),
200+
{
201+
...createBlock('producer', BlockType.FUNCTION),
202+
metadata: { id: 'function', name: 'Fetch' },
203+
},
201204
createBlock('consumer', BlockType.FUNCTION),
202205
],
203206
connections: [
@@ -220,7 +223,12 @@ describe('DAGExecutor run-from-block snapshot metadata', () => {
220223
const error = await executor.executeFromBlock('wf', 'consumer', sourceSnapshot).catch((e) => e)
221224

222225
expect(error).toBeInstanceOf(RunFromBlockValidationError)
223-
expect(error.message).toBe('Upstream dependency not executed: producer')
226+
expect(error.message).toBe('Upstream dependency not executed: Fetch')
227+
expect(error.refusal).toEqual({
228+
kind: 'upstream_not_executed',
229+
blockId: 'consumer',
230+
upstreamBlockId: 'producer',
231+
})
224232
})
225233
})
226234

‎apps/sim/executor/execution/executor.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,15 @@ export class DAGExecutor {
153153
const dag = this.dagBuilder.build(this.workflow, { includeAllBlocks: true })
154154

155155
const executedBlocks = new Set(sourceSnapshot.executedBlocks)
156-
const validation = validateRunFromBlock(startBlockId, dag, executedBlocks)
156+
const blockNames = new Map<string, string>()
157+
for (const block of this.workflow.blocks) {
158+
if (block.metadata?.name) blockNames.set(block.id, block.metadata.name)
159+
}
160+
const validation = validateRunFromBlock(startBlockId, dag, executedBlocks, blockNames)
157161
if (!validation.valid) {
158162
throw new RunFromBlockValidationError(
159-
validation.error ?? `Cannot run from block: ${startBlockId}`
163+
validation.error ?? `Cannot run from block: ${startBlockId}`,
164+
validation.refusal
160165
)
161166
}
162167

‎apps/sim/executor/utils/run-from-block.ts‎

Lines changed: 49 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,37 @@ export function resolveContainerToSentinelStart(blockId: string, dag: DAG): stri
2929
export interface RunFromBlockValidation {
3030
valid: boolean
3131
error?: string
32+
/** Why the start was refused, by id, for callers that phrase their own guidance. */
33+
refusal?: RunFromBlockRefusal
3234
}
3335

36+
/**
37+
* The structured reason behind a refused run-from-block start. `blockId` is always the
38+
* requested start block.
39+
*/
40+
export type RunFromBlockRefusal =
41+
| { kind: 'block_not_found'; blockId: string }
42+
| {
43+
kind: 'inside_subflow'
44+
blockId: string
45+
subflowId: string
46+
subflowType: 'loop' | 'parallel'
47+
}
48+
| { kind: 'upstream_not_executed'; blockId: string; upstreamBlockId: string }
49+
3450
/**
3551
* A run-from-block start the executor refused before running anything: the block is
3652
* missing, sits inside a loop or parallel, or has an upstream dependency the source
3753
* snapshot never executed. Its own class so the application layer can hand the reason
3854
* to the caller as a validation failure instead of the generic system-error fallback.
3955
*/
4056
export class RunFromBlockValidationError extends Error {
41-
constructor(message: string) {
57+
readonly refusal?: RunFromBlockRefusal
58+
59+
constructor(message: string, refusal?: RunFromBlockRefusal) {
4260
super(message)
4361
this.name = 'RunFromBlockValidationError'
62+
this.refusal = refusal
4463
}
4564
}
4665

@@ -158,39 +177,50 @@ export function computeExecutionSets(dag: DAG, startBlockId: string): ExecutionS
158177
* @param blockId - The block ID to validate
159178
* @param dag - The workflow DAG
160179
* @param executedBlocks - Set of blocks that were executed in the source run
180+
* @param blockNames - Display names by block id, so the message names blocks instead of ids
161181
* @returns Validation result with error message if invalid
162182
*/
163183
export function validateRunFromBlock(
164184
blockId: string,
165185
dag: DAG,
166-
executedBlocks: Set<string>
186+
executedBlocks: Set<string>,
187+
blockNames: ReadonlyMap<string, string> = new Map()
167188
): RunFromBlockValidation {
189+
const label = (id: string) => blockNames.get(id) ?? id
190+
const insideSubflow = (
191+
subflowId: string | undefined,
192+
subflowType: 'loop' | 'parallel'
193+
): RunFromBlockValidation => {
194+
if (!subflowId) {
195+
return { valid: false, error: `Cannot run from block inside ${subflowType}: ${blockId}` }
196+
}
197+
return {
198+
valid: false,
199+
error: `Cannot run from block inside ${subflowType}: ${label(subflowId)}. Run the ${subflowType} or the whole workflow instead.`,
200+
refusal: { kind: 'inside_subflow', blockId, subflowId, subflowType },
201+
}
202+
}
203+
168204
const node = dag.nodes.get(blockId)
169205
const isLoopContainer = dag.loopConfigs.has(blockId)
170206
const isParallelContainer = dag.parallelConfigs.has(blockId)
171207
const isContainer = isLoopContainer || isParallelContainer
172208
let validationNode = node
173209

174210
if (!node && !isContainer) {
175-
return { valid: false, error: `Block not found in workflow: ${blockId}` }
211+
return {
212+
valid: false,
213+
error: `Block not found in workflow: ${blockId}`,
214+
refusal: { kind: 'block_not_found', blockId },
215+
}
176216
}
177217

178218
if (isContainer) {
179219
const parentLoopId = findParentLoop(blockId, dag)
180-
if (parentLoopId) {
181-
return {
182-
valid: false,
183-
error: `Cannot run from block inside loop: ${parentLoopId}`,
184-
}
185-
}
220+
if (parentLoopId) return insideSubflow(parentLoopId, 'loop')
186221

187222
const parentParallelId = findParentParallel(blockId, dag)
188-
if (parentParallelId) {
189-
return {
190-
valid: false,
191-
error: `Cannot run from block inside parallel: ${parentParallelId}`,
192-
}
193-
}
223+
if (parentParallelId) return insideSubflow(parentParallelId, 'parallel')
194224

195225
const sentinelStartId = resolveContainerToSentinelStart(blockId, dag)
196226
if (!sentinelStartId || !dag.nodes.has(sentinelStartId)) {
@@ -203,19 +233,8 @@ export function validateRunFromBlock(
203233
}
204234

205235
if (node) {
206-
if (node.metadata.isLoopNode) {
207-
return {
208-
valid: false,
209-
error: `Cannot run from block inside loop: ${node.metadata.subflowId}`,
210-
}
211-
}
212-
213-
if (node.metadata.isParallelBranch) {
214-
return {
215-
valid: false,
216-
error: `Cannot run from block inside parallel: ${node.metadata.subflowId}`,
217-
}
218-
}
236+
if (node.metadata.isLoopNode) return insideSubflow(node.metadata.subflowId, 'loop')
237+
if (node.metadata.isParallelBranch) return insideSubflow(node.metadata.subflowId, 'parallel')
219238

220239
if (node.metadata.isSentinel) {
221240
return { valid: false, error: 'Cannot run from sentinel node' }
@@ -236,7 +255,8 @@ export function validateRunFromBlock(
236255
if (!executedBlocks.has(sourceId)) {
237256
return {
238257
valid: false,
239-
error: `Upstream dependency not executed: ${sourceId}`,
258+
error: `Upstream dependency not executed: ${label(sourceId)}`,
259+
refusal: { kind: 'upstream_not_executed', blockId, upstreamBlockId: sourceId },
240260
}
241261
}
242262
}

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

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,131 @@ describe('Copilot workflow run application commands', () => {
306306
expect(readAttemptedExecutionId(error)).toBe('child-execution-1')
307307
})
308308

309+
describe('a refused run-from-block start tells the agent what to do next', () => {
310+
const blocks = {
311+
'start-1': { id: 'start-1', name: 'Start', enabled: true },
312+
'fetch-1': { id: 'fetch-1', name: 'Fetch', enabled: true },
313+
'loop-1': { id: 'loop-1', name: 'Each row', enabled: true },
314+
'agent-1': { id: 'agent-1', name: 'Summarize', enabled: true },
315+
'note-1': { id: 'note-1', name: 'Notes', enabled: false },
316+
}
317+
318+
async function refuse(
319+
blockId: string,
320+
error: RunFromBlockValidationError,
321+
workflowBlocks: Record<string, unknown> = blocks
322+
) {
323+
mockLoadDraft.mockResolvedValue({ blocks: workflowBlocks, edges: [] })
324+
mocks.sourceState.mockResolvedValueOnce({
325+
blockStates: {},
326+
executedBlocks: [],
327+
blockLogs: [],
328+
decisions: {},
329+
completedLoops: [],
330+
activeExecutionPath: [],
331+
})
332+
mockExecuteWorkflow.mockRejectedValueOnce(error)
333+
const thrown = await runFromBlockFromCopilot
334+
.execute({
335+
principal,
336+
input: {
337+
workflowId: 'workflow-1',
338+
useDraftState: true,
339+
lifecycle,
340+
blockId,
341+
sourceExecutionId: 'source-execution-1',
342+
},
343+
})
344+
.catch((caught) => caught)
345+
expect(thrown).toBeInstanceOf(OrchestrationError)
346+
expect(thrown.code).toBe('validation')
347+
return thrown.message as string
348+
}
349+
350+
it('names the upstream block that never ran and the calls that get past it', async () => {
351+
const message = await refuse(
352+
'agent-1',
353+
new RunFromBlockValidationError('Upstream dependency not executed: Fetch', {
354+
kind: 'upstream_not_executed',
355+
blockId: 'agent-1',
356+
upstreamBlockId: 'fetch-1',
357+
})
358+
)
359+
360+
expect(message).toContain('"Summarize" (agent-1)')
361+
expect(message).toContain('"Fetch" (fetch-1)')
362+
expect(message).toContain('execution source-execution-1')
363+
expect(message).toContain('run_workflow_until_block')
364+
expect(message).toContain('stopAfterBlockId "agent-1"')
365+
expect(message).toContain('executionId')
366+
})
367+
368+
it('names the container a nested block sits in and points at it', async () => {
369+
const message = await refuse(
370+
'agent-1',
371+
new RunFromBlockValidationError('Cannot run from block inside loop: Each row', {
372+
kind: 'inside_subflow',
373+
blockId: 'agent-1',
374+
subflowId: 'loop-1',
375+
subflowType: 'loop',
376+
})
377+
)
378+
379+
expect(message).toContain('inside loop "Each row" (loop-1)')
380+
expect(message).toContain('block id "loop-1"')
381+
expect(message).toContain('run_workflow')
382+
})
383+
384+
it('lists the blocks the workflow does have when the id is unknown', async () => {
385+
const message = await refuse(
386+
'agent-x',
387+
new RunFromBlockValidationError('Block not found in workflow: agent-x', {
388+
kind: 'block_not_found',
389+
blockId: 'agent-x',
390+
})
391+
)
392+
393+
expect(message).toContain('"agent-x" is not in the draft workflow')
394+
expect(message).toContain('"Fetch" (fetch-1)')
395+
expect(message).toContain('"Each row" (loop-1)')
396+
})
397+
398+
it('bounds the block list on a large workflow', async () => {
399+
const many = Object.fromEntries(
400+
Array.from({ length: 25 }, (_, index) => [
401+
`b-${index}`,
402+
{ id: `b-${index}`, name: `Block ${index}`, enabled: true },
403+
])
404+
)
405+
const message = await refuse(
406+
'missing',
407+
new RunFromBlockValidationError('Block not found in workflow: missing', {
408+
kind: 'block_not_found',
409+
blockId: 'missing',
410+
}),
411+
many
412+
)
413+
414+
expect(message).toContain('(b-19)')
415+
expect(message).not.toContain('(b-20)')
416+
expect(message).toContain('and 5 more')
417+
})
418+
419+
it('says a known but unrunnable block is disabled instead of missing', async () => {
420+
const message = await refuse(
421+
'note-1',
422+
new RunFromBlockValidationError('Block not found in workflow: note-1', {
423+
kind: 'block_not_found',
424+
blockId: 'note-1',
425+
})
426+
)
427+
428+
expect(message).toContain('"Notes" (note-1)')
429+
expect(message).toContain('disabled')
430+
expect(message).not.toContain('is not in the')
431+
})
432+
})
433+
309434
/**
310435
* A caller whose result was withheld decides about retry from one fact: whether a run
311436
* exists. This layer owns that answer, because it is the last place that can distinguish

0 commit comments

Comments
 (0)