From 6cedc9c1822822d875ebcf6bbd5e16deae630d0a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 4 Nov 2026 01:33:11 -0800 Subject: [PATCH] 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. --- apps/sim/lib/workflows/editing/engine.ts | 24 ++++++++++-- .../lib/workflows/editing/operations.test.ts | 38 +++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/workflows/editing/engine.ts b/apps/sim/lib/workflows/editing/engine.ts index 25a9495aee8..f20daab4f5e 100644 --- a/apps/sim/lib/workflows/editing/engine.ts +++ b/apps/sim/lib/workflows/editing/engine.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import { toArray } from '@sim/utils/object' import { generateLoopBlocks, generateParallelBlocks, @@ -202,6 +203,9 @@ export function applyOperationsToWorkflowState( const declinedOperations = new Set() /** The operation that queued each entry of `ctx.deferredConnections`, index for index. */ const deferredConnectionOrigins: number[] = [] + /** The operation whose deferred connection created each edge this batch added, by edge id. */ + const edgeOrigins = new Map() + const currentEdges = () => toArray<{ id: string }>(modifiedState.edges) const recordedRefusalSince = (skipsBefore: number) => skippedItems.slice(skipsBefore).some((item) => !isDeferredSkippedItem(item)) @@ -261,6 +265,7 @@ export function applyOperationsToWorkflowState( } const skipsBefore = skippedItems.length + const edgesBefore = currentEdges().length addConnectionsAsEdges( modifiedState, blockId, @@ -270,7 +275,9 @@ export function applyOperationsToWorkflowState( validationErrors ) const origin = deferredConnectionOrigins[connectionIndex] - if (origin !== undefined && recordedRefusalSince(skipsBefore)) declinedOperations.add(origin) + if (origin === undefined) continue + if (recordedRefusalSince(skipsBefore)) declinedOperations.add(origin) + for (const edge of currentEdges().slice(edgesBefore)) edgeOrigins.set(edge.id, origin) } logger.info('Finished processing deferred connections', { @@ -287,8 +294,13 @@ export function applyOperationsToWorkflowState( // Remove edges that cross scope boundaries. This runs after all operations // and deferred connections are applied so that every block has its final // parentId. Running it per-operation would incorrectly drop edges between - // blocks that are both being moved into the same subflow in one batch. - removeInvalidScopeEdges(modifiedState, skippedItems) + // blocks that are both being moved into the same subflow in one batch. A dropped + // edge this batch created declines the operation that asked for it; an existing + // edge a move left crossing scopes is the move's documented effect, not a refusal. + for (const edgeId of removeInvalidScopeEdges(modifiedState, skippedItems)) { + const origin = edgeOrigins.get(edgeId) + if (origin !== undefined) declinedOperations.add(origin) + } reindexToolCanonicalModesAfterEdits( workflowState.blocks as Record | undefined, @@ -454,8 +466,10 @@ function resolvePendingConnections(modifiedState: any, skippedItems: SkippedItem * - Same scope: both blocks share the same parentId * - Container→child: source is the parent container of the target (start handles) * - Child→container: target is the parent container of the source (end handles) + * + * Returns the ids of the dropped edges. */ -function removeInvalidScopeEdges(modifiedState: any, skippedItems: SkippedItem[]): void { +function removeInvalidScopeEdges(modifiedState: any, skippedItems: SkippedItem[]): string[] { const { valid, dropped } = validateEdges(modifiedState.edges || [], modifiedState.blocks || {}) modifiedState.edges = valid @@ -480,4 +494,6 @@ function removeInvalidScopeEdges(modifiedState: any, skippedItems: SkippedItem[] reasons: dropped.map(({ reason }) => reason), }) } + + return dropped.map(({ edge }) => edge.id) } diff --git a/apps/sim/lib/workflows/editing/operations.test.ts b/apps/sim/lib/workflows/editing/operations.test.ts index 249df5c9f8f..e292bb3a67c 100644 --- a/apps/sim/lib/workflows/editing/operations.test.ts +++ b/apps/sim/lib/workflows/editing/operations.test.ts @@ -1487,6 +1487,44 @@ describe('connection shape validation', () => { expect(skippedItems.filter((item) => item.type === 'invalid_source_handle')).toHaveLength(2) expect(declinedOperationCount).toBe(1) }) + + it('charges an edge dropped by the final scope check to the operation that created it', () => { + const { state, skippedItems, declinedOperationCount } = applyOperationsToWorkflowState( + makeLoopWorkflow(), + [ + { + operation_type: 'add', + block_id: BLOCK_A, + params: { + type: 'function', + name: 'Block A', + inputs: { code: 'return 1' }, + connections: { source: 'agent-1' }, + }, + }, + ] + ) + + expect(state.edges.some((edge: { source: string }) => edge.source === BLOCK_A)).toBe(false) + expect(skippedItems.filter((item) => item.type === 'invalid_edge_scope')).toHaveLength(1) + expect(declinedOperationCount).toBe(1) + }) + + it('does not charge a move for the existing edges it leaves crossing scopes', () => { + const { skippedItems, declinedOperationCount } = applyOperationsToWorkflowState( + makeLoopWorkflow(), + [ + { + operation_type: 'extract_from_subflow', + block_id: 'agent-1', + params: { subflowId: 'loop-1' }, + }, + ] + ) + + expect(skippedItems.filter((item) => item.type === 'invalid_edge_scope')).toHaveLength(1) + expect(declinedOperationCount).toBe(0) + }) }) describe('conditional input validation through workflow operations', () => {