refactor: stabilize flow delete execution

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

frontend/src/lib/components/flows/flowDeleteController.test.ts

@@ -0,0 +1,142 @@
+import { describe, expect, it, vi } from 'vitest'
+
+vi.mock('../aiProviderStorage', () => ({
+	loadStoredConfig: () => undefined
+}))
+
+vi.mock('./flowInfers', () => ({
+	AI_AGENT_SCHEMA: { properties: {} }
+}))
+
+import { GroupDisplayState } from '$lib/components/graph/groupEditor.svelte'
+import {
+	GroupedModulesProxy,
+	type ExtendedOpenFlow
+} from '$lib/components/graph/groupedModulesProxy.svelte'
+import type { FlowModule, OpenFlow } from '$lib/gen'
+import type { StateStore } from '$lib/utils'
+import { executeDeletePlan, prepareDeleteRequest } from './flowDeleteController'
+
+function makeRawModule(id: string, expr?: string): FlowModule {
+	return {
+		id,
+		summary: id,
+		value: {
+			type: 'rawscript',
+			content: '',
+			language: 'python3',
+			input_transforms: expr
+				? {
+						user_input: {
+							type: 'javascript',
+							expr
+						}
+					}
+				: {}
+		} as any
+	} as FlowModule
+}
+
+function makeAiAgent(id: string, tools: any[]): FlowModule {
+	return {
+		id,
+		summary: id,
+		value: {
+			type: 'aiagent',
+			tools,
+			input_transforms: {}
+		} as any
+	} as FlowModule
+}
+
+function makeFlowModuleTool(module: FlowModule) {
+	return {
+		id: module.id,
+		summary: module.summary,
+		value: {
+			tool_type: 'flowmodule',
+			...module.value
+		}
+	}
+}
+
+describe('flowDeleteController', () => {
+	it('prepares confirmations and executes grouped ai-agent deletes end to end', () => {
+		const agent = makeAiAgent('agent_step', [
+			makeFlowModuleTool(makeRawModule('lookup_user')),
+			makeFlowModuleTool(
+				makeAiAgent('support_agent', [makeFlowModuleTool(makeRawModule('create_ticket'))])
+			)
+		])
+		const dependent = makeRawModule('dependent_step', 'results.create_ticket?.value')
+		const flowStore = {
+			val: {
+				summary: 'Flow',
+				value: {
+					modules: [agent, dependent],
+					groups: [
+						{
+							summary: 'Agent group',
+							start_id: 'agent_step',
+							end_id: 'agent_step'
+						}
+					]
+				}
+			} as OpenFlow
+		} satisfies StateStore<OpenFlow>
+		const proxy = new GroupedModulesProxy(flowStore as unknown as StateStore<ExtendedOpenFlow>)
+		const displayState = new GroupDisplayState(() => flowStore.val.value.groups ?? [])
+
+		const request = prepareDeleteRequest({
+			ids: ['agent_step'],
+			flow: flowStore.val,
+			tree: proxy.items,
+			proxy,
+			displayState
+		})
+
+		expect(request?.needsDependencyConfirmation).toBe(true)
+		expect(request?.plan.affectedGroups).toHaveLength(1)
+		expect(request?.plan.affectedGroups[0]).toMatchObject({
+			summary: 'Agent group',
+			start_id: 'agent_step',
+			end_id: 'agent_step'
+		})
+
+		const selectionManager = {
+			clearSelection: vi.fn(),
+			selectId: vi.fn()
+		}
+		const flowStateStore = {
+			val: {
+				agent_step: {},
+				lookup_user: {},
+				support_agent: {},
+				create_ticket: {},
+				dependent_step: {}
+			}
+		} as StateStore<any>
+		const onDelete = vi.fn()
+
+		const result = executeDeletePlan(request!.plan, {
+			flowStore,
+			flowStateStore,
+			selectionManager,
+			proxy,
+			displayState,
+			onDelete
+		})
+
+		expect(result.removedStateIds).toEqual([
+			'agent_step',
+			'lookup_user',
+			'support_agent',
+			'create_ticket'
+		])
+		expect(flowStore.val.value.modules.map((module) => module.id)).toEqual(['dependent_step'])
+		expect(flowStore.val.value.groups ?? []).toEqual([])
+		expect(Object.keys(flowStateStore.val)).toEqual(['dependent_step'])
+		expect(selectionManager.selectId).toHaveBeenCalledWith('dependent_step')
+		expect(onDelete).toHaveBeenCalledWith('agent_step')
+	})
+})

frontend/src/lib/components/flows/flowDeleteController.ts

@@ -0,0 +1,135 @@
+import type { GroupDisplayState } from '$lib/components/graph/groupEditor.svelte'
+import type { GroupedModulesProxy } from '$lib/components/graph/groupedModulesProxy.svelte'
+import type { SelectionManager } from '$lib/components/graph/selectionUtils.svelte'
+import type { FlowStructureNode } from '$lib/components/graph/flowStructure'
+import type { FlowModule, OpenFlow } from '$lib/gen'
+import { push, type History } from '$lib/history.svelte'
+import { refreshStateStore } from '$lib/svelte5Utils.svelte'
+import type { StateStore } from '$lib/utils'
+import { collectFlowNodeIds } from './agentToolTree'
+import {
+	createDeletePlan,
+	removeDeletePlanTools,
+	type DeletePlan,
+	type DeleteTarget
+} from './flowDeleteUtils'
+import type { FlowState } from './flowState'
+import { deleteFlowStateById } from './flowStateUtils.svelte'
+import { dfsByModule } from './previousResults'
+
+export type PreparedDeleteRequest = {
+	plan: DeletePlan
+	needsDependencyConfirmation: boolean
+}
+
+export type DeletePlanExecutionContext = {
+	history?: History<OpenFlow>
+	flowStore: StateStore<OpenFlow>
+	flowStateStore: StateStore<FlowState>
+	selectionManager: Pick<SelectionManager, 'clearSelection' | 'selectId'>
+	proxy: GroupedModulesProxy
+	displayState: GroupDisplayState
+	onDelete?: (id: string) => void
+}
+
+export type DeletePlanExecutionResult = {
+	removedStateIds: string[]
+	removedToolStateIds: string[]
+}
+
+export function prepareDeleteRequest(args: {
+	ids: string[]
+	flow: OpenFlow
+	tree: FlowStructureNode[]
+	proxy: GroupedModulesProxy
+	displayState: GroupDisplayState
+}): PreparedDeleteRequest | undefined {
+	const plan = createDeletePlan(args)
+	if (!plan) {
+		return undefined
+	}
+
+	return {
+		plan,
+		needsDependencyConfirmation: Object.keys(plan.dependents).length > 0
+	}
+}
+
+export function executeDeletePlan(
+	plan: DeletePlan,
+	args: DeletePlanExecutionContext
+): DeletePlanExecutionResult {
+	push(args.history, args.flowStore.val)
+	const hasPreprocessor = Boolean(args.flowStore.val.value.preprocessor_module)
+	const removedStructureStateIds = resolveLiveStructureStateIds(
+		plan.targets,
+		args.flowStore.val.value.modules,
+		hasPreprocessor
+	)
+
+	if (plan.selection.kind === 'clear') {
+		args.selectionManager.clearSelection()
+	} else {
+		args.selectionManager.selectId(plan.selection.id)
+	}
+
+	if (plan.targets.some((target) => target.kind === 'preprocessor')) {
+		args.flowStore.val.value.preprocessor_module = undefined
+	}
+
+	if (plan.structureIds.length > 0) {
+		const structureDelete = args.proxy.prepareDelete(plan.structureIds, {
+			displayState: args.displayState
+		})
+		structureDelete.commit({ removeDuplicates: plan.removeDuplicates })
+	}
+
+	const removedToolStateIds = removeDeletePlanTools(plan.targets, args.flowStore.val.value.modules)
+	const removedStateIds = uniqueIds([...removedStructureStateIds, ...removedToolStateIds])
+
+	for (const id of removedStateIds) {
+		deleteFlowStateById(id, args.flowStateStore)
+	}
+
+	refreshStateStore(args.flowStore)
+
+	if (plan.inputIds.length === 1) {
+		args.onDelete?.(plan.targets[0]?.id ?? plan.inputIds[0])
+	}
+
+	return {
+		removedStateIds,
+		removedToolStateIds
+	}
+}
+
+function resolveLiveStructureStateIds(
+	targets: DeleteTarget[],
+	modules: FlowModule[],
+	hasPreprocessor: boolean
+): string[] {
+	const removedIds: string[] = []
+
+	for (const target of targets) {
+		if (target.kind === 'preprocessor') {
+			if (hasPreprocessor) {
+				removedIds.push(target.id)
+			}
+			continue
+		}
+		if (target.kind !== 'structure_node') {
+			continue
+		}
+
+		const module = dfsByModule(target.id, modules)[0]
+		if (module) {
+			removedIds.push(...collectFlowNodeIds(module))
+		}
+	}
+
+	return uniqueIds(removedIds)
+}
+
+function uniqueIds(ids: string[]): string[] {
+	return [...new Set(ids)]
+}

frontend/src/lib/components/flows/flowDeleteUtils.test.ts

@@ -103,7 +103,6 @@ describe('createDeletePlan', () => {
 		]
 		const commit = vi.fn()
 		const prepareDelete = vi.fn(() => ({
-			removedIds: ['agent_step'],
 			affectedGroups: [],
 			duplicateGroups: [],
 			commit
@@ -121,7 +120,9 @@ describe('createDeletePlan', () => {
 		})
 
 		expect(prepareDelete).toHaveBeenCalledWith(['agent_step'], expect.any(Object))
-		expect(plan?.stateIds).toEqual(['agent_step', 'lookup_user'])
+		expect(plan?.plannedStateIds).toEqual(['agent_step', 'lookup_user'])
+		expect(plan?.structureIds).toEqual(['agent_step'])
+		expect(plan?.affectedGroups).toEqual([])
 		expect(plan?.dependents).toEqual({
 			dependent_step: ['results.lookup_user?.value']
 		})
@@ -142,7 +143,24 @@ describe('removeDeletePlanTools', () => {
 			false
 		)
 
-		expect(removeDeletePlanTools(targets)).toEqual(['support_agent', 'create_ticket'])
+		expect(removeDeletePlanTools(targets, [rootAgent])).toEqual(['support_agent', 'create_ticket'])
 		expect((rootAgent.value as any).tools).toEqual([])
 	})
+
+	it('re-resolves live owners before removing planned tool targets', () => {
+		const rootAgent = makeAiAgent('root_agent', [
+			makeFlowModuleTool(makeRawModule('lookup_user')),
+			makeFlowModuleTool(makeAiAgent('support_agent', [makeFlowModuleTool(makeRawModule('create_ticket'))]))
+		])
+		const tree: FlowStructureNode[] = []
+		const { targets } = resolveDeleteTargets(tree, [rootAgent], ['support_agent'], false)
+
+		;(rootAgent.value as any).tools.unshift(makeFlowModuleTool(makeRawModule('new_lookup')))
+
+		expect(removeDeletePlanTools(targets, [rootAgent])).toEqual(['support_agent', 'create_ticket'])
+		expect(((rootAgent.value as any).tools as any[]).map((tool) => tool.id)).toEqual([
+			'new_lookup',
+			'lookup_user'
+		])
+	})
 })

frontend/src/lib/components/flows/flowDeleteUtils.ts

@@ -1,8 +1,5 @@
-import type { GroupDisplayState } from '$lib/components/graph/groupEditor.svelte'
-import type {
-	GroupedModulesProxy,
-	PreparedStructureDelete
-} from '$lib/components/graph/groupedModulesProxy.svelte'
+import type { FlowGroup, GroupDisplayState } from '$lib/components/graph/groupEditor.svelte'
+import type { GroupedModulesProxy } from '$lib/components/graph/groupedModulesProxy.svelte'
 import { findInStructure, type FlowStructureNode } from '$lib/components/graph/flowStructure'
 import type { FlowModule, OpenFlow } from '$lib/gen'
 import {
@@ -39,7 +36,7 @@ export type StructureDeleteTarget = DeleteTargetBase & {
 
 export type AgentToolDeleteTarget = DeleteTargetBase & {
 	kind: 'agent_tool'
-	owner: AgentToolOwner
+	expectedOwnerAgentId: string
 }
 
 export type DeleteTarget =
@@ -50,10 +47,11 @@ export type DeleteTarget =
 export type DeletePlan = {
 	inputIds: string[]
 	targets: DeleteTarget[]
-	stateIds: string[]
+	plannedStateIds: string[]
 	dependents: Record<string, string[]>
 	selection: DeleteSelection
-	structureDelete?: PreparedStructureDelete
+	structureIds: string[]
+	affectedGroups: FlowGroup[]
 	removeDuplicates: boolean
 }
 
@@ -104,7 +102,7 @@ export function resolveDeleteTargets(
 			targets.push({
 				kind: 'agent_tool',
 				id,
-				owner,
+				expectedOwnerAgentId: owner.agentId,
 				stateIds: collectAgentToolIds(owner.tool)
 			})
 			continue
@@ -146,26 +144,35 @@ export function createDeletePlan(args: {
 			? args.proxy.prepareDelete(structureIds, { displayState: args.displayState })
 			: undefined
 
-	const stateIds = uniqueIds(targets.flatMap((target) => target.stateIds))
+	const plannedStateIds = uniqueIds(targets.flatMap((target) => target.stateIds))
 
 	return {
 		inputIds: args.ids,
 		targets,
-		stateIds,
-		dependents: collectDeleteDependents(stateIds, args.flow),
-		selection: getDeleteSelection(args.ids, stateIds, args.flow.value.modules),
-		structureDelete,
-		removeDuplicates: Boolean(structureDelete && structureDelete.duplicateGroups.length > 0)
+		plannedStateIds,
+		dependents: collectDeleteDependents(plannedStateIds, args.flow),
+		selection: getDeleteSelection(args.ids, plannedStateIds, args.flow.value.modules),
+		structureIds,
+		affectedGroups: structureDelete?.affectedGroups ?? [],
+		removeDuplicates: Boolean(structureDelete?.duplicateGroups.length)
 	}
 }
 
 export function removeDeletePlanTools(
 	targets: DeleteTarget[],
+	modules: FlowModule[],
 	onRemove?: (tool: AgentTool) => void
 ): string[] {
 	const removedIds = new Set<string>()
 	const toolTargets = targets
 		.filter((target): target is AgentToolDeleteTarget => target.kind === 'agent_tool')
+		.map((target) => {
+			const owner = findAgentToolOwner(modules, target.id)
+			return owner ? { owner, target } : undefined
+		})
+		.filter((entry): entry is { owner: AgentToolOwner; target: AgentToolDeleteTarget } =>
+			Boolean(entry)
+		)
 		.sort((left, right) => {
 			if (left.owner.depth !== right.owner.depth) {
 				return right.owner.depth - left.owner.depth
@@ -176,8 +183,8 @@ export function removeDeletePlanTools(
 			return 0
 		})
 
-	for (const target of toolTargets) {
-		const removed = removeAgentToolOwner(target.owner)
+	for (const { owner } of toolTargets) {
+		const removed = removeAgentToolOwner(owner)
 		if (!removed) {
 			continue
 		}

frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte

@@ -53,11 +53,8 @@
 		agentToolToFlowModule
 	} from '../agentToolUtils'
 	import { loadFlowModuleState } from '../flowStateUtils.svelte'
-	import {
-		createDeletePlan,
-		removeDeletePlanTools,
-		type DeletePlan
-	} from '../flowDeleteUtils'
+	import type { DeletePlan } from '../flowDeleteUtils'
+	import { executeDeletePlan, prepareDeleteRequest } from '../flowDeleteController'
 	import { getNoteEditorContext } from '$lib/components/graph/noteEditor.svelte'
 	import {
 		GroupedModulesProxy,
@@ -340,55 +337,40 @@
 	}
 
 	function applyDeletePlan(plan: DeletePlan) {
-		push(history, flowStore.val)
-
-		if (plan.selection.kind === 'clear') {
-			selectionManager.clearSelection()
-		} else {
-			selectionManager.selectId(plan.selection.id)
-		}
-
-		if (plan.targets.some((target) => target.kind === 'preprocessor')) {
-			flowStore.val.value.preprocessor_module = undefined
-		}
-
-		plan.structureDelete?.commit({ removeDuplicates: plan.removeDuplicates })
-		removeDeletePlanTools(plan.targets)
-
-		for (const id of plan.stateIds) {
-			deleteFlowStateById(id, flowStateStore)
-		}
-
-		refreshStateStore(flowStore)
-
-		if (plan.inputIds.length === 1) {
-			onDelete?.(plan.targets[0]?.id ?? plan.inputIds[0])
-		}
+		executeDeletePlan(plan, {
+			history,
+			flowStore,
+			flowStateStore,
+			selectionManager,
+			proxy,
+			displayState: groupDisplayState,
+			onDelete
+		})
 	}
 
 	function requestDelete(ids: string[]) {
-		const plan = createDeletePlan({
+		const request = prepareDeleteRequest({
 			ids,
 			flow: flowStore.val,
 			tree: proxy.items,
 			proxy,
 			displayState: groupDisplayState
 		})
-		if (!plan) {
+		if (!request) {
 			return
 		}
 
 		const proceed = () => {
-			if (Object.keys(plan.dependents).length > 0) {
-				pendingDeleteConfirmation = { plan }
+			if (request.needsDependencyConfirmation) {
+				pendingDeleteConfirmation = { plan: request.plan }
 			} else {
-				applyDeletePlan(plan)
+				applyDeletePlan(request.plan)
 			}
 		}
 
-		if ((plan.structureDelete?.affectedGroups.length ?? 0) > 0) {
+		if (request.plan.affectedGroups.length > 0) {
 			pendingGroupAction = {
-				groups: plan.structureDelete!.affectedGroups,
+				groups: request.plan.affectedGroups,
 				label: 'delete',
 				confirm: proceed
 			}

frontend/src/lib/components/graph/groupedModulesProxy.svelte.ts

@@ -27,7 +27,6 @@ export type ExtendedOpenFlow = {
 }
 
 export type PreparedStructureDelete = {
-	removedIds: string[]
 	affectedGroups: FlowGroup[]
 	duplicateGroups: FlowGroup[]
 	commit: (commitOpts?: { removeDuplicates?: boolean }) => void
@@ -126,7 +125,6 @@ export class GroupedModulesProxy {
 			displayState?: import('./groupEditor.svelte').GroupDisplayState
 		}
 	): PreparedStructureDelete {
-		const removedIds: string[] = []
 		const { emptiedGroups, duplicateGroups, commit } = this.prepareMutation((tree) => {
 			for (const id of ids) {
 				const found = findInStructure(tree, id)
@@ -135,12 +133,10 @@ export class GroupedModulesProxy {
 				}
 
 				found.parentChildren.splice(found.index, 1)
-				removedIds.push(id)
 			}
 		}, opts)
 
 		return {
-			removedIds,
 			affectedGroups: [...emptiedGroups, ...duplicateGroups],
 			duplicateGroups,
 			commit