fix(workflows): keep validating literal entries of an oversized selector value

Review round 2.

The length cap skipped the whole field, so an oversized list of plain literal ids
lost validation it previously had. Literals never needed tokenization, so the cap
was broader than the cost it was there to bound.

It now gives up only the reference-aware split: past the cap the value is split
plainly, and its entries are classified and validated as usual, since they are
short enough that the tokenizer's per-candidate cost does not apply (1MB of
literal ids across 30000 entries measures ~9ms). Only an individual entry past
the cap is skipped, where there is no cheap way to tell a literal from a dynamic
binding.

Author: mzxchandra

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

@@ -1304,9 +1304,7 @@ describe('collectUnresolvedReferences', () => {
     expect(refs).toHaveLength(0)
   })
 
-  it('skips an oversized selector value instead of tokenizing it', async () => {
-    // Reference detection tokenizes the whole string and the tokenizer is superlinear in
-    // candidate count, so an implausible value is skipped rather than parsed.
+  it('skips a single oversized entry, which cannot be classified cheaply', async () => {
     mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['x'] })
     const state = {
       blocks: {
@@ -1324,6 +1322,28 @@ describe('collectUnresolvedReferences', () => {
     expect(refs).toHaveLength(0)
   })
 
+  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.
+    mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] })
+    const padding = Array.from({ length: 400 }, (_, index) => `kb_${'x'.repeat(30)}${index}`)
+    const value = [...padding, '<start.kbId>', 'kb_missing'].join(',')
+    expect(value.length).toBeGreaterThan(10_000)
+    const state = {
+      blocks: {
+        kb1: { type: 'knowledge', name: 'KB', subBlocks: { knowledgeBaseId: { value } } },
+      },
+    }
+    const startedAt = performance.now()
+    const refs = await collectUnresolvedReferences(state, CTX)
+
+    expect(performance.now() - startedAt).toBeLessThan(1000)
+    const [, ids] = mockValidateSelectorIds.mock.calls[0]
+    expect(ids).toContain('kb_missing')
+    expect(ids).not.toContain('<start.kbId>')
+    expect(refs).toHaveLength(1)
+  })
+
   it('still validates a plain literal id (the guard must not over-skip)', async () => {
     mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] })
     const state = {

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

@@ -1083,11 +1083,13 @@ interface SelectorFieldToValidate {
 }
 
 /**
- * A selector value is an id, or a short comma-separated list of them. Reference detection
- * tokenizes the whole string and `findWorkflowReferenceTokens` is superlinear in candidate count,
- * so an oversized value is both implausible and expensive on a write path that admits megabytes.
- * Past this the field is skipped rather than tokenized: the lint is advisory, so declining to
- * check is the safe direction.
+ * Longest selector string reference detection will tokenize.
+ *
+ * `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.
  */
 const MAX_SELECTOR_VALUE_LENGTH = 10_000
 
@@ -1144,25 +1146,32 @@ function collectSelectorFields(
 
       const subBlockValue = blockData.subBlocks?.[subBlockConfig.id]?.value
       if (!subBlockValue) continue
-      if (typeof subBlockValue === 'string' && subBlockValue.length > MAX_SELECTOR_VALUE_LENGTH) {
-        continue
-      }
+
+      const isOversized = (entry: unknown) =>
+        typeof entry === 'string' && entry.length > MAX_SELECTOR_VALUE_LENGTH
 
       // Handle comma-separated values for multi-select
       let values: string | string[] = subBlockValue
       if (typeof subBlockValue === 'string' && subBlockValue.includes(',')) {
-        values = splitOutsideReferences(subBlockValue)
+        values = isOversized(subBlockValue)
+          ? subBlockValue
+              .split(',')
+              .map((entry: string) => entry.trim())
+              .filter(Boolean)
+          : splitOutsideReferences(subBlockValue)
       }
 
       // A dynamically bound value only acquires its id at execution time, so a static
       // id-existence check cannot evaluate it. Filtered per entry rather than on the whole
       // string, because a multi-select can mix literal ids with dynamic ones: testing
       // `<a.b>,kb_real,<c.d>` as a whole would drop `kb_real` along with the references.
       if (Array.isArray(values)) {
-        const literalValues = values.filter((entry) => !containsReference(entry))
+        const literalValues = values.filter(
+          (entry) => !isOversized(entry) && !containsReference(entry)
+        )
         if (literalValues.length === 0) continue
         values = literalValues
-      } else if (containsReference(values)) {
+      } else if (isOversized(values) || containsReference(values)) {
         continue
       }