Skip to content

Commit 1888e62

Browse files
committed
fix(workflows): correct reference resolution for active mode, cycles, cache, and graph size
Addresses review findings on the reference viewer: - Resolve the workflow-block child via resolveActiveCanonicalValue (the shared SOT) instead of basic-first `||`, so an advanced-mode block whose old basic workflowId value lingers resolves to the active manual value. - Keep self-references (A -> A) and render them as a cycle leaf instead of dropping the edge, matching the cycle-safe viewer's purpose. - Set the references query staleTime to 0 so reopening the always-mounted modal refetches live editor state instead of serving a stale cached graph. - Bound converging paths: a node already expanded elsewhere in the tree is emitted once more as a plain leaf (edge stays visible) rather than re-expanded, so a densely reconverging graph can't grow exponentially.
1 parent f11c3d2 commit 1888e62

3 files changed

Lines changed: 103 additions & 14 deletions

File tree

apps/sim/hooks/queries/workflow-references.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,13 @@ import {
55
type WorkflowReferencesResponse,
66
} from '@/lib/api/contracts/workflow-references'
77

8-
/** Short — the graph reflects live editor state and should stay fresh on reopen. */
9-
export const WORKFLOW_REFERENCES_STALE_TIME = 30 * 1000
8+
/**
9+
* Zero — the graph reflects live editor state (workflow blocks/names can change
10+
* between opens). The modal stays mounted with the query disabled while closed, so
11+
* a non-zero window would serve a cached graph on reopen without refetching. Zero
12+
* marks the data stale immediately, forcing a refetch each time the modal reopens.
13+
*/
14+
export const WORKFLOW_REFERENCES_STALE_TIME = 0
1015

1116
export const workflowReferenceKeys = {
1217
all: ['workflow-references'] as const,

apps/sim/lib/workflows/references/operations.test.ts

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ function workflowBlock(
2727
type,
2828
childFromSelector: mode === 'basic' ? childId : null,
2929
childFromManual: mode === 'manual' ? childId : null,
30+
canonicalModes: null,
3031
}
3132
}
3233

@@ -65,6 +66,22 @@ describe('resolveWorkflowReferences', () => {
6566
expect(callees.map((n) => n.id)).toEqual(['b'])
6667
})
6768

69+
it('uses the active mode, not a retained inactive value', () => {
70+
// Advanced mode active (canonicalModes override), but a stale basic value
71+
// (`b`) lingers. Must resolve to the advanced value (`c`), not the stale basic.
72+
const blocks: ReferenceBlockRow[] = [
73+
{
74+
parentId: 'a',
75+
type: 'workflow',
76+
childFromSelector: 'b',
77+
childFromManual: 'c',
78+
canonicalModes: { workflowId: 'advanced' },
79+
},
80+
]
81+
const { callees } = resolveWorkflowReferences('a', workflows, blocks, [])
82+
expect(callees.map((n) => n.id)).toEqual(['c'])
83+
})
84+
6885
it('marks cycles as leaves and stops recursing', () => {
6986
// A → B → A
7087
const blocks = [workflowBlock('a', 'b'), workflowBlock('b', 'a')]
@@ -76,11 +93,30 @@ describe('resolveWorkflowReferences', () => {
7693
expect(callees[0].children[0]).toMatchObject({ id: 'a', cycle: true, children: [] })
7794
})
7895

79-
it('drops self-references', () => {
96+
it('shows a self-reference as a cycle leaf', () => {
97+
// A → A: the reference is real and belongs in the cycle-safe viewer.
8098
const blocks = [workflowBlock('a', 'a')]
8199
const { callers, callees } = resolveWorkflowReferences('a', workflows, blocks, [])
82-
expect(callers).toEqual([])
83-
expect(callees).toEqual([])
100+
expect(callees).toEqual([{ id: 'a', name: 'A', cycle: true, children: [] }])
101+
expect(callers).toEqual([{ id: 'a', name: 'A', cycle: true, children: [] }])
102+
})
103+
104+
it('bounds converging paths (diamond) instead of re-expanding', () => {
105+
// A → B, A → C, B → D, C → D. D reconverges; it must appear under both B and C
106+
// but only expand once (here D is a leaf anyway; the guard prevents blow-up).
107+
const blocks = [
108+
workflowBlock('a', 'b'),
109+
workflowBlock('a', 'c'),
110+
workflowBlock('b', 'd'),
111+
workflowBlock('c', 'd'),
112+
]
113+
const { callees } = resolveWorkflowReferences('a', workflows, blocks, [])
114+
const b = callees.find((n) => n.id === 'b')
115+
const c = callees.find((n) => n.id === 'c')
116+
// D expands under the first-visited branch (B) and is a plain leaf under C.
117+
expect(b?.children.map((n) => n.id)).toEqual(['d'])
118+
expect(c?.children.map((n) => n.id)).toEqual(['d'])
119+
expect(c?.children[0]).toMatchObject({ id: 'd', cycle: false, children: [] })
84120
})
85121

86122
it('drops dangling / out-of-workspace child ids', () => {
@@ -92,7 +128,13 @@ describe('resolveWorkflowReferences', () => {
92128
it('resolves references made through custom blocks', () => {
93129
// D places custom_block_x, which is bound to source workflow C.
94130
const blocks: ReferenceBlockRow[] = [
95-
{ parentId: 'd', type: 'custom_block_x', childFromSelector: null, childFromManual: null },
131+
{
132+
parentId: 'd',
133+
type: 'custom_block_x',
134+
childFromSelector: null,
135+
childFromManual: null,
136+
canonicalModes: null,
137+
},
96138
]
97139
const customBlocks: CustomBlockLink[] = [{ type: 'custom_block_x', workflowId: 'c' }]
98140

@@ -110,6 +152,7 @@ describe('resolveWorkflowReferences', () => {
110152
type: 'custom_block_unknown',
111153
childFromSelector: null,
112154
childFromManual: null,
155+
canonicalModes: null,
113156
},
114157
]
115158
const { callees } = resolveWorkflowReferences('d', workflows, blocks, [])

apps/sim/lib/workflows/references/operations.ts

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ import { createLogger } from '@sim/logger'
44
import { and, eq, inArray, isNull, like, or, sql } from 'drizzle-orm'
55
import type { ReferenceNode } from '@/lib/api/contracts/workflow-references'
66
import { getCustomBlockRowsForWorkspace } from '@/lib/workflows/custom-blocks/operations'
7+
import {
8+
type CanonicalGroup,
9+
type CanonicalModeOverrides,
10+
resolveActiveCanonicalValue,
11+
} from '@/lib/workflows/subblocks/visibility'
712
import { CUSTOM_BLOCK_TYPE_PREFIX } from '@/blocks/custom/build-config'
813
import { BlockType, isWorkflowBlockType } from '@/executor/constants'
914

@@ -16,6 +21,18 @@ const logger = createLogger('WorkflowReferences')
1621
*/
1722
const MAX_REFERENCE_DEPTH = 25
1823

24+
/**
25+
* The `workflowId` canonical pair on a workflow / workflow_input block: the basic
26+
* `workflowId` selector and the advanced `manualWorkflowId` input. Used with
27+
* {@link resolveActiveCanonicalValue} so the reference resolves to the value of the
28+
* block's ACTIVE mode — never a dormant mode's retained (stale) value.
29+
*/
30+
const WORKFLOW_ID_CANONICAL_GROUP: CanonicalGroup = {
31+
canonicalId: 'workflowId',
32+
basicId: 'workflowId',
33+
advancedIds: ['manualWorkflowId'],
34+
}
35+
1936
/** A workspace-local, non-archived workflow node. */
2037
export interface WorkflowNode {
2138
id: string
@@ -30,6 +47,11 @@ export interface ReferenceBlockRow {
3047
childFromSelector: string | null
3148
/** `manualWorkflowId` sub-block value (advanced mode), if a workflow block. */
3249
childFromManual: string | null
50+
/**
51+
* The block's `data.canonicalModes` override (basic/advanced per canonical id),
52+
* used to pick the active `workflowId` value. Absent for most blocks.
53+
*/
54+
canonicalModes: CanonicalModeOverrides | null
3355
}
3456

3557
/** A custom-block type slug bound to its source workflow. */
@@ -56,8 +78,11 @@ interface ReferenceGraph {
5678
* - **custom blocks** (`custom_block_<id>`), whose type slug maps to a bound
5779
* source workflow via `customBlocks`.
5880
*
59-
* Edges are scoped to workspace-local, non-archived workflows: self-references,
60-
* empty values, and ids outside `workflows` are dropped.
81+
* The workflow-block child is the value of the block's ACTIVE mode
82+
* ({@link resolveActiveCanonicalValue}), so a dormant basic/advanced value can't
83+
* mask the live one. Edges are scoped to workspace-local, non-archived workflows;
84+
* empty values and ids outside `workflows` are dropped. A workflow that calls
85+
* itself is kept — the tree builder renders it as a `cycle` leaf.
6186
*/
6287
export function buildReferenceGraph(
6388
workflows: WorkflowNode[],
@@ -74,7 +99,7 @@ export function buildReferenceGraph(
7499
const reverse = new Map<string, Set<string>>()
75100

76101
const addEdge = (parentId: string, childId: string) => {
77-
if (!childId || childId === parentId) return
102+
if (!childId) return
78103
if (!nameById.has(parentId) || !nameById.has(childId)) return
79104
if (!forward.has(parentId)) forward.set(parentId, new Set())
80105
forward.get(parentId)?.add(childId)
@@ -84,8 +109,12 @@ export function buildReferenceGraph(
84109

85110
for (const block of blocks) {
86111
if (isWorkflowBlockType(block.type)) {
87-
const childId = block.childFromSelector || block.childFromManual
88-
if (childId) addEdge(block.parentId, childId)
112+
const active = resolveActiveCanonicalValue(
113+
WORKFLOW_ID_CANONICAL_GROUP,
114+
{ workflowId: block.childFromSelector, manualWorkflowId: block.childFromManual },
115+
block.canonicalModes ?? undefined
116+
)
117+
if (typeof active === 'string' && active) addEdge(block.parentId, active)
89118
continue
90119
}
91120
const sourceId = sourceByCustomType.get(block.type)
@@ -97,16 +126,22 @@ export function buildReferenceGraph(
97126

98127
/**
99128
* Expand a direction of the graph into a tree rooted at `rootId`. `adjacency` is
100-
* either the forward (callees) or reverse (callers) map. A node already on the
101-
* current DFS path is emitted as a `cycle: true` leaf and not re-expanded, so
102-
* `A → B → A` terminates. Children are sorted by name for stable rendering.
129+
* either the forward (callees) or reverse (callers) map.
130+
*
131+
* A node already on the current DFS path is emitted as a `cycle: true` leaf and
132+
* not re-expanded, so `A → B → A` (and a self-call `A → A`) terminates. A node
133+
* already fully expanded elsewhere in this tree (reachable via another acyclic
134+
* path — a diamond) is emitted once more as a plain leaf without re-expanding its
135+
* subtree: the edge stays visible, but a densely reconverging graph can't blow up
136+
* exponentially. Children are sorted by name for stable rendering.
103137
*/
104138
function buildTree(
105139
rootId: string,
106140
adjacency: Map<string, Set<string>>,
107141
nameById: Map<string, string>
108142
): ReferenceNode[] {
109143
const path = new Set<string>([rootId])
144+
const expanded = new Set<string>()
110145

111146
const expand = (id: string, depth: number): ReferenceNode[] => {
112147
if (depth >= MAX_REFERENCE_DEPTH) return []
@@ -126,6 +161,11 @@ function buildTree(
126161
nodes.push({ id: childId, name, cycle: true, children: [] })
127162
continue
128163
}
164+
if (expanded.has(childId)) {
165+
nodes.push({ id: childId, name, cycle: false, children: [] })
166+
continue
167+
}
168+
expanded.add(childId)
129169
path.add(childId)
130170
nodes.push({ id: childId, name, cycle: false, children: expand(childId, depth + 1) })
131171
path.delete(childId)
@@ -185,6 +225,7 @@ export async function getWorkflowReferences(
185225
childFromManual: sql<
186226
string | null
187227
>`${workflowBlocks.subBlocks} -> 'manualWorkflowId' ->> 'value'`,
228+
canonicalModes: sql<CanonicalModeOverrides | null>`${workflowBlocks.data} -> 'canonicalModes'`,
188229
})
189230
.from(workflowBlocks)
190231
.innerJoin(workflow, eq(workflow.id, workflowBlocks.workflowId))

0 commit comments

Comments
 (0)