Skip to content

Commit 83d361e

Browse files
committed
fix(workflows): split multi-select values without tearing references
A `<block.path>` or `{{ENV_VAR}}` token may legitimately contain a comma (`<start.pick(a,b)>`), and its fragments read as plain literals once split, so a naive `.split(',')` turns one dynamically bound value into several bogus ids. Adds `splitOutsideReferences`, which treats only commas outside every reference token as separators. Token spans are marked once into a lookup rather than rescanned per comma: a per-comma `tokens.some()` is O(commas x tokens) and took ~2.5s on a 240KB value of repeated `{{A}},`, which is reachable on the 10MB graph-write paths. Known limitation, unchanged from the `.split(',')` this replaces and covered by a characterization test: the tokenizer suppresses a workflow span that overlaps an environment token, so `<start.body.pick({{A}},b)>` still splits.
1 parent 1a7c827 commit 83d361e

2 files changed

Lines changed: 93 additions & 0 deletions

File tree

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
22
import {
33
containsReference,
44
isLikelyReferenceSegment,
5+
splitOutsideReferences,
56
splitReferenceSegment,
67
} from '@/lib/workflows/sanitization/references'
78

@@ -90,3 +91,56 @@ describe('containsReference', () => {
9091
expect(containsReference('a<b<c>d')).toBe(false)
9192
})
9293
})
94+
95+
describe('splitOutsideReferences', () => {
96+
it('splits on separator commas', () => {
97+
expect(splitOutsideReferences('kb_a,kb_b')).toEqual(['kb_a', 'kb_b'])
98+
})
99+
100+
it('trims entries and drops empties', () => {
101+
expect(splitOutsideReferences(' kb_a , , kb_b ')).toEqual(['kb_a', 'kb_b'])
102+
})
103+
104+
it('keeps a comma that sits inside a workflow reference', () => {
105+
expect(splitOutsideReferences('<start.pick(a,b)>')).toEqual(['<start.pick(a,b)>'])
106+
})
107+
108+
it('keeps a comma inside a reference while still splitting around it', () => {
109+
expect(splitOutsideReferences('kb_a,<start.pick(x,y)>,kb_b')).toEqual([
110+
'kb_a',
111+
'<start.pick(x,y)>',
112+
'kb_b',
113+
])
114+
})
115+
116+
it('keeps a comma inside an env-var placeholder', () => {
117+
expect(splitOutsideReferences('{{A,B}},kb_a')).toEqual(['{{A,B}}', 'kb_a'])
118+
})
119+
120+
it('returns a single entry when there is no separator', () => {
121+
expect(splitOutsideReferences('kb_a')).toEqual(['kb_a'])
122+
})
123+
124+
it('stays linear on a large value instead of rescanning tokens per comma', () => {
125+
// A per-comma `tokens.some()` is O(commas x tokens) and took ~2.5s on this input.
126+
const value = '{{A}},'.repeat(40000)
127+
const startedAt = performance.now()
128+
const parts = splitOutsideReferences(value)
129+
const elapsedMs = performance.now() - startedAt
130+
131+
expect(parts).toHaveLength(40000)
132+
expect(elapsedMs).toBeLessThan(1000)
133+
})
134+
135+
it('does not yet protect a comma inside a reference that nests an env-var placeholder', () => {
136+
// Known limitation, unchanged from the plain `.split(',')` this replaced: the tokenizer
137+
// reports the inner `{{A}}` and suppresses the outer `<...>` span, so the comma reads as a
138+
// separator. Characterized rather than fixed - the suppression lives in the shared
139+
// `@sim/utils/workflow-references` tokenizer.
140+
expect(splitOutsideReferences('<start.body.pick({{A}},b)>,kb_literal')).toEqual([
141+
'<start.body.pick({{A}}',
142+
'b)>',
143+
'kb_literal',
144+
])
145+
})
146+
})

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,45 @@ export function containsReference(value: unknown): boolean {
2323
return findWorkflowReferenceTokens(value).length > 0
2424
}
2525

26+
/**
27+
* Split a comma-separated multi-select value without tearing a reference apart.
28+
*
29+
* A `<block.path>` or `{{ENV_VAR}}` token may legitimately contain a comma (`<start.pick(a,b)>`),
30+
* and its fragments read as plain literals once split, so a naive `.split(',')` turns one dynamic
31+
* value into several bogus ones. Only commas outside every reference token are separators.
32+
*/
33+
export function splitOutsideReferences(value: string): string[] {
34+
const tokens = findWorkflowReferenceTokens(value)
35+
if (tokens.length === 0) {
36+
return value
37+
.split(',')
38+
.map((part) => part.trim())
39+
.filter(Boolean)
40+
}
41+
42+
// Marked once up front rather than scanned per comma: a per-comma `tokens.some()` is
43+
// O(commas x tokens), which reached ~2.5s on a 240KB value of repeated `{{A}},`.
44+
const insideReference = new Uint8Array(value.length)
45+
for (const token of tokens) {
46+
const end = Math.min(token.end, value.length)
47+
for (let index = Math.max(token.start, 0); index < end; index += 1) {
48+
insideReference[index] = 1
49+
}
50+
}
51+
52+
const parts: string[] = []
53+
let partStart = 0
54+
for (let index = 0; index < value.length; index += 1) {
55+
if (value[index] === ',' && !insideReference[index]) {
56+
parts.push(value.slice(partStart, index))
57+
partStart = index + 1
58+
}
59+
}
60+
parts.push(value.slice(partStart))
61+
62+
return parts.map((part) => part.trim()).filter(Boolean)
63+
}
64+
2665
export function extractReferencePrefixes(value: string): Array<{ raw: string; prefix: string }> {
2766
if (!value || typeof value !== 'string') {
2867
return []

0 commit comments

Comments
 (0)