fix(workflows): split references without the non-overlapping token pass

Review round 3.

An oversized value fell back to plain splitting, which tore a comma-bearing
reference into fragments that were then validated as literal ids. The fallback
existed to avoid `findWorkflowReferenceTokens`, which is superlinear in candidate
count.

That pass was never needed here. It returns contractually NON-overlapping tokens,
and the O(tokens^2) overlap check is the cost of producing that partition. A
splitter only needs to know whether an index sits inside SOME reference, so
scanning environment placeholders and `<...>` candidates independently gives the
union directly - cheaper and more accurate, since nothing is suppressed.

Splitting is now linear and the size fallback is gone, so a comma-bearing
reference survives at any length:

  240KB of `<a.b>,`          682ms -> 15ms
  240KB of `<a.p({{X}},y)>,` 177ms -> 12ms
  1MB of literal ids                   1ms

The length cap now applies to a single ENTRY rather than the whole field, which
is all it was ever needed for: classifying one entry tokenizes it, and there is
no cheap way to tell a literal from a dynamic binding past that size.

Exports `ENV_REFERENCE_PATTERN` from `@sim/utils` rather than duplicating the
pattern, so the two stay in step.

Author: mzxchandra

apps/sim/lib/workflows/editing/validation.test.ts

@@ -1322,6 +1322,26 @@ describe('collectUnresolvedReferences', () => {
     expect(refs).toHaveLength(0)
   })
 
+  it('keeps a comma-bearing reference intact even in an oversized list', async () => {
+    // Splitting scans reference regions directly and stays linear, so size does not force a
+    // fallback that would tear `<start.pick(a,b)>` into fragments validated as ids.
+    mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] })
+    const padding = Array.from({ length: 400 }, (_, index) => `kb_${'x'.repeat(30)}${index}`)
+    const value = [...padding, '<start.pick(a,b)>', 'kb_missing'].join(',')
+    expect(value.length).toBeGreaterThan(10_000)
+    const state = {
+      blocks: {
+        kb1: { type: 'knowledge', name: 'KB', subBlocks: { knowledgeBaseId: { value } } },
+      },
+    }
+    await collectUnresolvedReferences(state, CTX)
+
+    const [, ids] = mockValidateSelectorIds.mock.calls[0]
+    expect(ids).toContain('kb_missing')
+    expect(ids).not.toContain('<start.pick(a')
+    expect(ids).not.toContain('b)>')
+  })
+
   it('still validates the literal entries of an oversized list', async () => {
     // The cap gives up reference-aware splitting, not validation: the entries are still short,
     // so each is classified and the literals are still checked.

apps/sim/lib/workflows/editing/validation.ts

@@ -1083,15 +1083,15 @@ interface SelectorFieldToValidate {
 }
 
 /**
- * Longest selector string reference detection will tokenize.
+ * Longest single selector entry `containsReference` will classify.
  *
- * `findWorkflowReferenceTokens` parses the whole string and is superlinear in candidate count, so
- * an oversized value is expensive on a write path that admits megabytes. Past this a value is
- * split plainly instead: its entries are still short, so each one is classified and validated as
- * usual, and only the comma-inside-a-reference protection is given up. An individual ENTRY past
- * the cap is skipped, since there is no cheap way to tell a literal from a dynamic binding.
+ * Classifying one entry tokenizes it, and `findWorkflowReferenceTokens` is superlinear in
+ * candidate count, so an entry of unbounded length is expensive on a write path that admits
+ * megabytes. Splitting is unaffected - it scans reference regions directly and stays linear - so
+ * only an individual oversized ENTRY is skipped, where there is no cheap way to tell a literal
+ * from a dynamic binding. A long LIST of ordinary ids still splits and validates normally.
  */
-const MAX_SELECTOR_VALUE_LENGTH = 10_000
+const MAX_SELECTOR_ENTRY_LENGTH = 10_000
 
 /**
  * Walk a workflow state and collect selector/credential fields to validate.
@@ -1148,17 +1148,12 @@ function collectSelectorFields(
       if (!subBlockValue) continue
 
       const isOversized = (entry: unknown) =>
-        typeof entry === 'string' && entry.length > MAX_SELECTOR_VALUE_LENGTH
+        typeof entry === 'string' && entry.length > MAX_SELECTOR_ENTRY_LENGTH
 
       // Handle comma-separated values for multi-select
       let values: string | string[] = subBlockValue
       if (typeof subBlockValue === 'string' && subBlockValue.includes(',')) {
-        values = isOversized(subBlockValue)
-          ? subBlockValue
-              .split(',')
-              .map((entry: string) => entry.trim())
-              .filter(Boolean)
-          : splitOutsideReferences(subBlockValue)
+        values = splitOutsideReferences(subBlockValue)
       }
 
       // A dynamically bound value only acquires its id at execution time, so a static

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

@@ -1,4 +1,5 @@
 import {
+  ENV_REFERENCE_PATTERN,
   findWorkflowReferenceTokens,
   isLikelyWorkflowReferenceSegment,
   splitWorkflowReferenceSegment,
@@ -24,14 +25,24 @@ export function containsReference(value: unknown): boolean {
 }
 
 /**
- * Mark every `<...>` region that reads as a workflow reference, including one the tokenizer
- * dropped for overlapping an `{{ENV_VAR}}` token.
+ * Mark every region of `value` that belongs to a reference, as a union rather than a partition.
  *
- * `findWorkflowReferenceTokens` is contractually NON-overlapping, so for `<a.pick({{B}},c)>` it
- * reports only the inner `{{B}}` and discards the outer candidate. That is right for a tokenizer
- * and wrong for a splitter, which needs the UNION of protected regions rather than a disjoint set.
+ * Deliberately does NOT use `findWorkflowReferenceTokens`. That returns contractually
+ * non-overlapping tokens, which costs an O(tokens^2) overlap check and drops a `<...>` candidate
+ * that wraps an `{{ENV_VAR}}` - both wrong here. A splitter only needs to know whether an index is
+ * inside SOME reference, so scanning each kind independently is both cheaper and more accurate.
  */
-function markCandidateReferenceRegions(value: string, insideReference: Uint8Array): void {
+function markReferenceRegions(value: string, insideReference: Uint8Array): void {
+  const mark = (start: number, end: number) => {
+    for (let index = Math.max(start, 0); index < Math.min(end, value.length); index += 1) {
+      insideReference[index] = 1
+    }
+  }
+
+  for (const match of value.matchAll(ENV_REFERENCE_PATTERN)) {
+    mark(match.index, match.index + match[0].length)
+  }
+
   let candidateStart = -1
   for (let index = 0; index < value.length; index += 1) {
     const character = value[index]
@@ -49,10 +60,7 @@ function markCandidateReferenceRegions(value: string, insideReference: Uint8Arra
     const split = splitReferenceSegment(candidate)
     if (split && isLikelyReferenceSegment(candidate)) {
       const start = candidateStart + split.leading.length
-      const end = Math.min(start + split.reference.length, value.length)
-      for (let position = start; position < end; position += 1) {
-        insideReference[position] = 1
-      }
+      mark(start, start + split.reference.length)
     }
     candidateStart = -1
   }
@@ -66,34 +74,15 @@ function markCandidateReferenceRegions(value: string, insideReference: Uint8Arra
  * value into several bogus ones. Only commas outside every reference region are separators.
  */
 export function splitOutsideReferences(value: string): string[] {
-  const plainSplit = () =>
-    value
+  if (!value.includes(REFERENCE.START) && !value.includes(REFERENCE.ENV_VAR_START)) {
+    return value
       .split(',')
       .map((part) => part.trim())
       .filter(Boolean)
-
-  if (!value.includes(REFERENCE.START) && !value.includes(REFERENCE.ENV_VAR_START)) {
-    return plainSplit()
   }
 
-  // Marked once up front rather than scanned per comma: a per-comma `tokens.some()` is
-  // O(commas x tokens), which reached ~2.5s on a 240KB value of repeated `{{A}},`.
-  const tokens = findWorkflowReferenceTokens(value)
   const insideReference = new Uint8Array(value.length)
-  let hasEnvironmentToken = false
-  for (const token of tokens) {
-    if (token.kind === 'environment') hasEnvironmentToken = true
-    const end = Math.min(token.end, value.length)
-    for (let index = Math.max(token.start, 0); index < end; index += 1) {
-      insideReference[index] = 1
-    }
-  }
-
-  // Overlap with an environment token is the only reason the tokenizer drops a workflow
-  // candidate, so without one it already reported every region and re-scanning is duplicate work.
-  if (hasEnvironmentToken) {
-    markCandidateReferenceRegions(value, insideReference)
-  }
+  markReferenceRegions(value, insideReference)
 
   const parts: string[] = []
   let partStart = 0

packages/utils/src/workflow-references.ts

@@ -3,7 +3,11 @@ const REFERENCE_END = '>'
 const REFERENCE_PATH_DELIMITER = '.'
 const INVALID_REFERENCE_CHARS = /[+*/=<>!&|]/
 const LEADING_REFERENCE_PATTERN = /^[<>=!\s]*$/
-const ENV_REFERENCE_PATTERN = /\{\{[^{}\r\n]+\}\}/g
+/**
+ * `{{ENV_VAR}}` placeholders. Exported so a consumer that needs the UNION of reference regions
+ * can find them without going through the non-overlapping token pass.
+ */
+export const ENV_REFERENCE_PATTERN = /\{\{[^{}\r\n]+\}\}/g
 
 export type WorkflowReferenceTokenKind = 'environment' | 'workflow'