diff --git a/.changeset/remove-nodeid-from-handles.md b/.changeset/remove-nodeid-from-handles.md index 0b0d5b639..b6f30a262 100644 --- a/.changeset/remove-nodeid-from-handles.md +++ b/.changeset/remove-nodeid-from-handles.md @@ -1,22 +1,5 @@ --- -'@workflowbuilder/sdk': major +'@workflowbuilder/sdk': patch --- -refactor!: drop `nodeId` from handle IDs. xyflow scopes handle IDs by their -owning node, so embedding the node id in the string was redundant. - -Breaking changes: - -- `getHandleId({ nodeId, handleType, innerId? })` is now `getHandleId({ handleType, innerId? })`. - The returned ID is `` for outer handles and `:inner:` for - inner handles. Update every call site to drop the `nodeId` argument. -- The `HandleId` type narrowed accordingly: `OuterHandleId = 'source' | 'target'`, - `InnerHandleId = 'source:inner:${string}' | 'target:inner:${string}'`. -- `ConnectableItem` no longer accepts `{ nodeId, innerId, handleType }`. Pass the - pre-built `handleId` directly (use `getHandleId` to construct it). - -Persisted diagrams: edges saved with the previous format -(`:[:inner:]`) will no longer resolve their -endpoints after upgrading. No automatic migration is provided. Re-save affected -diagrams in a build of the previous SDK, transform them externally, or rebuild -them in the new format before upgrading. +fix: drop `nodeId` from handle IDs. Compound nodes (decision, AI agent, conditional) can now declare default ports statically (e.g. in JSON-defined `defaultProperties`) and copy/paste no longer requires custom handle rewriting after a node ID change. `getHandleId({ nodeId })` still compiles — the argument is optional, marked `@deprecated`, and ignored at runtime. Diagrams saved with the 2.0.0 `:[:inner:]` format are auto-migrated to the new `[:inner:]` form on `setDiagramModel` and `setStoreDataFromIntegration`. diff --git a/packages/sdk/src/features/diagram/handles/get-handle-id.spec.ts b/packages/sdk/src/features/diagram/handles/get-handle-id.spec.ts index a4c8d5211..6f22e1afd 100644 --- a/packages/sdk/src/features/diagram/handles/get-handle-id.spec.ts +++ b/packages/sdk/src/features/diagram/handles/get-handle-id.spec.ts @@ -22,4 +22,9 @@ describe('getHandleId', () => { expect(id.startsWith('source')).toBe(true); expect(id).not.toContain('node-'); }); + + it('accepts the deprecated 2.0.0 `nodeId` arg and ignores it at runtime', () => { + expect(getHandleId({ nodeId: 'node-1', handleType: 'source' })).toBe('source'); + expect(getHandleId({ nodeId: 'node-1', handleType: 'target', innerId: 'tool-7' })).toBe('target:inner:tool-7'); + }); }); diff --git a/packages/sdk/src/features/diagram/handles/get-handle-id.ts b/packages/sdk/src/features/diagram/handles/get-handle-id.ts index 87a010284..d3c393dac 100644 --- a/packages/sdk/src/features/diagram/handles/get-handle-id.ts +++ b/packages/sdk/src/features/diagram/handles/get-handle-id.ts @@ -26,4 +26,11 @@ export function getHandleId({ handleType, innerId }: GetHandleIdOptions): Handle type GetHandleIdOptions = { handleType: HandleType; innerId?: string; + /** + * @deprecated Handle IDs are scoped to the owning node by xyflow, so + * `nodeId` is no longer part of the returned string. Accepted to keep + * 2.0.0 call sites compiling and ignored at runtime. Will be removed in + * the next major (3.0). + */ + nodeId?: string; }; diff --git a/packages/sdk/src/features/diagram/handles/migrate-legacy-handle-id.spec.ts b/packages/sdk/src/features/diagram/handles/migrate-legacy-handle-id.spec.ts new file mode 100644 index 000000000..70a101029 --- /dev/null +++ b/packages/sdk/src/features/diagram/handles/migrate-legacy-handle-id.spec.ts @@ -0,0 +1,141 @@ +import { + migrateLegacyHandleId, + migrateLegacyHandleIdsOnEdges, + migrateLegacyHandleIdsOnNodes, +} from './migrate-legacy-handle-id'; + +describe('migrateLegacyHandleId', () => { + it('passes through new-format outer handle ids unchanged', () => { + expect(migrateLegacyHandleId('source')).toBe('source'); + expect(migrateLegacyHandleId('target')).toBe('target'); + }); + + it('passes through new-format inner handle ids unchanged', () => { + expect(migrateLegacyHandleId('source:inner:branch-1')).toBe('source:inner:branch-1'); + expect(migrateLegacyHandleId('target:inner:tool-42')).toBe('target:inner:tool-42'); + }); + + it('strips uuid prefix from legacy outer handle ids', () => { + expect(migrateLegacyHandleId('aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:source')).toBe('source'); + expect(migrateLegacyHandleId('bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb:target')).toBe('target'); + }); + + it('strips uuid prefix from legacy inner handle ids', () => { + expect(migrateLegacyHandleId('aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:source:inner:branch-1')).toBe( + 'source:inner:branch-1', + ); + expect(migrateLegacyHandleId('bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb:target:inner:tool-42')).toBe( + 'target:inner:tool-42', + ); + }); + + it('preserves null and undefined', () => { + expect(migrateLegacyHandleId(null)).toBeNull(); + let value: string | undefined; + expect(migrateLegacyHandleId(value)).toBeUndefined(); + }); + + it('leaves unknown shapes untouched', () => { + expect(migrateLegacyHandleId('something-weird')).toBe('something-weird'); + }); + + it('does not migrate strings ending in :source/:target without a uuid prefix (no false positive)', () => { + expect(migrateLegacyHandleId('node-1:source')).toBe('node-1:source'); + expect(migrateLegacyHandleId('myCustom:target')).toBe('myCustom:target'); + expect(migrateLegacyHandleId('tenant:org:source')).toBe('tenant:org:source'); + }); +}); + +describe('migrateLegacyHandleIdsOnEdges', () => { + it('rewrites legacy handle ids on a mixed batch of edges', () => { + const uuidA = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'; + const uuidB = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb'; + const uuidC = 'cccccccc-cccc-cccc-cccc-cccccccccccc'; + + const edges = [ + { id: 'e1', source: uuidA, target: uuidB, sourceHandle: `${uuidA}:source`, targetHandle: `${uuidB}:target` }, + { + id: 'e2', + source: uuidC, + target: uuidB, + sourceHandle: `${uuidC}:source:inner:branch-1`, + targetHandle: 'target', + }, + { id: 'e3', source: uuidA, target: uuidB, sourceHandle: 'source', targetHandle: 'target' }, + ]; + + const result = migrateLegacyHandleIdsOnEdges(edges); + + expect(result[0].sourceHandle).toBe('source'); + expect(result[0].targetHandle).toBe('target'); + expect(result[1].sourceHandle).toBe('source:inner:branch-1'); + expect(result[1].targetHandle).toBe('target'); + expect(result[2]).toBe(edges[2]); + }); +}); + +describe('migrateLegacyHandleIdsOnNodes', () => { + it('rewrites legacy sourceHandle inside nested property arrays (ai-agent tools)', () => { + const agentUuid = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'; + const nodes = [ + { + id: agentUuid, + data: { + properties: { + tools: [ + { id: 'tool-1', sourceHandle: `${agentUuid}:source:inner:tool-1` }, + { id: 'tool-2', sourceHandle: `${agentUuid}:source:inner:tool-2` }, + ], + }, + }, + }, + ]; + + const [migrated] = migrateLegacyHandleIdsOnNodes(nodes); + const properties = migrated.data.properties as { tools: { sourceHandle: string }[] }; + + expect(properties.tools[0].sourceHandle).toBe('source:inner:tool-1'); + expect(properties.tools[1].sourceHandle).toBe('source:inner:tool-2'); + }); + + it('rewrites legacy sourceHandle inside decisionBranches', () => { + const decisionUuid = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb'; + const nodes = [ + { + id: decisionUuid, + data: { + properties: { + decisionBranches: [{ id: 'b-1', sourceHandle: `${decisionUuid}:source:inner:b-1` }], + }, + }, + }, + ]; + + const [migrated] = migrateLegacyHandleIdsOnNodes(nodes); + const properties = migrated.data.properties as { decisionBranches: { sourceHandle: string }[] }; + + expect(properties.decisionBranches[0].sourceHandle).toBe('source:inner:b-1'); + }); + + it('returns the same node reference when no handle ids change', () => { + const node = { + id: 'agent-1', + data: { properties: { tools: [{ id: 'tool-1', sourceHandle: 'source:inner:tool-1' }] } }, + }; + + const [result] = migrateLegacyHandleIdsOnNodes([node]); + + expect(result).toBe(node); + }); + + it('handles nodes without properties gracefully', () => { + const node = { + id: 'plain', + data: { type: 'foo', icon: 'bar' } as Record, + }; + + const [result] = migrateLegacyHandleIdsOnNodes([node]); + + expect(result).toBe(node); + }); +}); diff --git a/packages/sdk/src/features/diagram/handles/migrate-legacy-handle-id.ts b/packages/sdk/src/features/diagram/handles/migrate-legacy-handle-id.ts new file mode 100644 index 000000000..2c1cda6e6 --- /dev/null +++ b/packages/sdk/src/features/diagram/handles/migrate-legacy-handle-id.ts @@ -0,0 +1,94 @@ +import type { Edge, Node } from '@xyflow/react'; + +import { INNER_HANDLE_MARKER } from './types'; + +const UUID_PATTERN = '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}'; +const NEW_FORMAT_INNER_PREFIX = new RegExp(`^(source|target):${INNER_HANDLE_MARKER}:`); +const LEGACY_HANDLE_PATTERN = new RegExp(`^${UUID_PATTERN}:(source|target)(:${INNER_HANDLE_MARKER}:.+)?$`); + +/** + * SDK 2.0.0 persisted handle IDs as `:[:inner:]`. + * The patch dropped the `:` prefix because xyflow already scopes handle + * IDs by their owning node. Strip the prefix on load so edges saved by 2.0.0 + * resolve their endpoints after upgrade. New-format IDs pass through unchanged. + * Anything not matching the SDK-produced legacy shape (UUID prefix) is left + * alone, so user-supplied custom handle IDs are never mis-migrated. + */ +export function migrateLegacyHandleId(handleId: T): T { + if (!handleId) return handleId; + + if (handleId === 'source' || handleId === 'target') return handleId; + if (NEW_FORMAT_INNER_PREFIX.test(handleId)) return handleId; + + const match = handleId.match(LEGACY_HANDLE_PATTERN); + if (match) { + return `${match[1]}${match[2] ?? ''}` as T; + } + + return handleId; +} + +export function migrateLegacyHandleIdsOnEdges>(edges: T[]): T[] { + return edges.map((edge) => { + const sourceHandle = migrateLegacyHandleId(edge.sourceHandle); + const targetHandle = migrateLegacyHandleId(edge.targetHandle); + + if (sourceHandle === edge.sourceHandle && targetHandle === edge.targetHandle) { + return edge; + } + + return { ...edge, sourceHandle, targetHandle }; + }); +} + +/** + * Compound nodes (AI agent tools, decision branches) persist handle IDs + * inside `node.data.properties` (e.g. `tools[].sourceHandle`). Those + * strings are passed straight to `` at render time, so + * if they keep the legacy `:` prefix while edges get migrated, + * xyflow can't match edges to handles. Walk the properties tree and + * rewrite any `sourceHandle` / `targetHandle` string the same way edges + * are rewritten. + */ +export function migrateLegacyHandleIdsOnNodes>(nodes: T[]): T[] { + return nodes.map((node) => { + const properties = (node.data as { properties?: unknown } | undefined)?.properties; + const migrated = migrateHandleIdsInTree(properties); + if (migrated === properties) return node; + + return { + ...node, + data: { ...(node.data as object), properties: migrated }, + } as T; + }); +} + +function migrateHandleIdsInTree(value: unknown): unknown { + if (Array.isArray(value)) { + let changed = false; + const next = value.map((item) => { + const migrated = migrateHandleIdsInTree(item); + if (migrated !== item) changed = true; + return migrated; + }); + return changed ? next : value; + } + + if (value !== null && typeof value === 'object') { + const record = value as Record; + let changed = false; + const next: Record = {}; + for (const key in record) { + const original = record[key]; + const migrated = + (key === 'sourceHandle' || key === 'targetHandle') && typeof original === 'string' + ? migrateLegacyHandleId(original) + : migrateHandleIdsInTree(original); + if (migrated !== original) changed = true; + next[key] = migrated; + } + return changed ? next : value; + } + + return value; +} diff --git a/packages/sdk/src/store/slices/diagram-slice.ts b/packages/sdk/src/store/slices/diagram-slice.ts index 1a252b75b..aa53eb34c 100644 --- a/packages/sdk/src/store/slices/diagram-slice.ts +++ b/packages/sdk/src/store/slices/diagram-slice.ts @@ -2,6 +2,10 @@ import { type Connection, type Node, type OnConnect, addEdge } from '@xyflow/rea import { trackFutureChange } from '../../features/changes-tracker/stores/use-changes-tracker-store'; import { getEdgeZIndex } from '../../features/diagram/edges/get-edge-z-index'; +import { + migrateLegacyHandleIdsOnEdges, + migrateLegacyHandleIdsOnNodes, +} from '../../features/diagram/handles/migrate-legacy-handle-id'; import type { VariablesIndex } from '../../features/variables/types'; import { type ConnectionBeingDragged, @@ -72,8 +76,8 @@ export function useDiagramSlice(set: SetDiagramState, get: GetDiagramState) { } } - const nodes = model?.diagram.nodes.map(getNodeWithErrors) || []; - const edges = model?.diagram.edges || []; + const nodes = migrateLegacyHandleIdsOnNodes(model?.diagram.nodes.map(getNodeWithErrors) || []); + const edges = migrateLegacyHandleIdsOnEdges(model?.diagram.edges || []); const documentName = model?.name || 'Untitled'; const layoutDirection = model?.layoutDirection || 'RIGHT'; diff --git a/packages/sdk/src/store/slices/diagram-slice/actions.ts b/packages/sdk/src/store/slices/diagram-slice/actions.ts index 870368dad..b70ba0d4c 100644 --- a/packages/sdk/src/store/slices/diagram-slice/actions.ts +++ b/packages/sdk/src/store/slices/diagram-slice/actions.ts @@ -1,4 +1,8 @@ // About actions: apps/demo/src/app/store/README.md +import { + migrateLegacyHandleIdsOnEdges, + migrateLegacyHandleIdsOnNodes, +} from '../../../features/diagram/handles/migrate-legacy-handle-id'; import { selectSingleSelectedElement } from '../../../features/properties-bar/use-single-selected-element'; import type { VariableDefinition } from '../../../features/variables/types'; import type { LayoutDirection } from '../../../node/common'; @@ -106,8 +110,8 @@ export function setStoreDataFromIntegration(loadData: Partial ({ documentName: loadData.name ?? state.documentName, globalVariables: loadData.globalVariables || state.globalVariables, - nodes: (loadData.nodes ?? state.nodes).map(getNodeWithErrors), - edges: loadData.edges ?? state.edges, + nodes: (loadData.nodes ? migrateLegacyHandleIdsOnNodes(loadData.nodes) : state.nodes).map(getNodeWithErrors), + edges: loadData.edges ? migrateLegacyHandleIdsOnEdges(loadData.edges) : state.edges, layoutDirection: loadData.layoutDirection ?? state.layoutDirection, })); }