Skip to content

Commit 6cedc9c

Browse files
committed
fix(workflows): charge an operation whose new edge the final scope check drops
The v2 apply-operations count treated an operation as applied when its connection created an edge that the end-of-batch scope check then removed (cross-subflow, trigger target, or note endpoint). The engine now records which operation's deferred connection created each edge and charges a dropped edge to that operation. Existing edges a move leaves crossing scopes stay uncharged: losing them is the move's effect, not a refusal.
1 parent d887612 commit 6cedc9c

2 files changed

Lines changed: 58 additions & 4 deletions

File tree

‎apps/sim/lib/workflows/editing/engine.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createLogger } from '@sim/logger'
2+
import { toArray } from '@sim/utils/object'
23
import {
34
generateLoopBlocks,
45
generateParallelBlocks,
@@ -202,6 +203,9 @@ export function applyOperationsToWorkflowState(
202203
const declinedOperations = new Set<number>()
203204
/** The operation that queued each entry of `ctx.deferredConnections`, index for index. */
204205
const deferredConnectionOrigins: number[] = []
206+
/** The operation whose deferred connection created each edge this batch added, by edge id. */
207+
const edgeOrigins = new Map<string, number>()
208+
const currentEdges = () => toArray<{ id: string }>(modifiedState.edges)
205209
const recordedRefusalSince = (skipsBefore: number) =>
206210
skippedItems.slice(skipsBefore).some((item) => !isDeferredSkippedItem(item))
207211

@@ -261,6 +265,7 @@ export function applyOperationsToWorkflowState(
261265
}
262266

263267
const skipsBefore = skippedItems.length
268+
const edgesBefore = currentEdges().length
264269
addConnectionsAsEdges(
265270
modifiedState,
266271
blockId,
@@ -270,7 +275,9 @@ export function applyOperationsToWorkflowState(
270275
validationErrors
271276
)
272277
const origin = deferredConnectionOrigins[connectionIndex]
273-
if (origin !== undefined && recordedRefusalSince(skipsBefore)) declinedOperations.add(origin)
278+
if (origin === undefined) continue
279+
if (recordedRefusalSince(skipsBefore)) declinedOperations.add(origin)
280+
for (const edge of currentEdges().slice(edgesBefore)) edgeOrigins.set(edge.id, origin)
274281
}
275282

276283
logger.info('Finished processing deferred connections', {
@@ -287,8 +294,13 @@ export function applyOperationsToWorkflowState(
287294
// Remove edges that cross scope boundaries. This runs after all operations
288295
// and deferred connections are applied so that every block has its final
289296
// parentId. Running it per-operation would incorrectly drop edges between
290-
// blocks that are both being moved into the same subflow in one batch.
291-
removeInvalidScopeEdges(modifiedState, skippedItems)
297+
// blocks that are both being moved into the same subflow in one batch. A dropped
298+
// edge this batch created declines the operation that asked for it; an existing
299+
// edge a move left crossing scopes is the move's documented effect, not a refusal.
300+
for (const edgeId of removeInvalidScopeEdges(modifiedState, skippedItems)) {
301+
const origin = edgeOrigins.get(edgeId)
302+
if (origin !== undefined) declinedOperations.add(origin)
303+
}
292304

293305
reindexToolCanonicalModesAfterEdits(
294306
workflowState.blocks as Record<string, BlockState> | undefined,
@@ -454,8 +466,10 @@ function resolvePendingConnections(modifiedState: any, skippedItems: SkippedItem
454466
* - Same scope: both blocks share the same parentId
455467
* - Container→child: source is the parent container of the target (start handles)
456468
* - Child→container: target is the parent container of the source (end handles)
469+
*
470+
* Returns the ids of the dropped edges.
457471
*/
458-
function removeInvalidScopeEdges(modifiedState: any, skippedItems: SkippedItem[]): void {
472+
function removeInvalidScopeEdges(modifiedState: any, skippedItems: SkippedItem[]): string[] {
459473
const { valid, dropped } = validateEdges(modifiedState.edges || [], modifiedState.blocks || {})
460474
modifiedState.edges = valid
461475

@@ -480,4 +494,6 @@ function removeInvalidScopeEdges(modifiedState: any, skippedItems: SkippedItem[]
480494
reasons: dropped.map(({ reason }) => reason),
481495
})
482496
}
497+
498+
return dropped.map(({ edge }) => edge.id)
483499
}

‎apps/sim/lib/workflows/editing/operations.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1487,6 +1487,44 @@ describe('connection shape validation', () => {
14871487
expect(skippedItems.filter((item) => item.type === 'invalid_source_handle')).toHaveLength(2)
14881488
expect(declinedOperationCount).toBe(1)
14891489
})
1490+
1491+
it('charges an edge dropped by the final scope check to the operation that created it', () => {
1492+
const { state, skippedItems, declinedOperationCount } = applyOperationsToWorkflowState(
1493+
makeLoopWorkflow(),
1494+
[
1495+
{
1496+
operation_type: 'add',
1497+
block_id: BLOCK_A,
1498+
params: {
1499+
type: 'function',
1500+
name: 'Block A',
1501+
inputs: { code: 'return 1' },
1502+
connections: { source: 'agent-1' },
1503+
},
1504+
},
1505+
]
1506+
)
1507+
1508+
expect(state.edges.some((edge: { source: string }) => edge.source === BLOCK_A)).toBe(false)
1509+
expect(skippedItems.filter((item) => item.type === 'invalid_edge_scope')).toHaveLength(1)
1510+
expect(declinedOperationCount).toBe(1)
1511+
})
1512+
1513+
it('does not charge a move for the existing edges it leaves crossing scopes', () => {
1514+
const { skippedItems, declinedOperationCount } = applyOperationsToWorkflowState(
1515+
makeLoopWorkflow(),
1516+
[
1517+
{
1518+
operation_type: 'extract_from_subflow',
1519+
block_id: 'agent-1',
1520+
params: { subflowId: 'loop-1' },
1521+
},
1522+
]
1523+
)
1524+
1525+
expect(skippedItems.filter((item) => item.type === 'invalid_edge_scope')).toHaveLength(1)
1526+
expect(declinedOperationCount).toBe(0)
1527+
})
14901528
})
14911529

14921530
describe('conditional input validation through workflow operations', () => {

0 commit comments

Comments
 (0)