diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/document-tag-entry/document-tag-entry.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/document-tag-entry/document-tag-entry.tsx index a61e9e35f3a..4e51a715a66 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/document-tag-entry/document-tag-entry.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/document-tag-entry/document-tag-entry.tsx @@ -21,6 +21,7 @@ import { getActiveWorkflowSearchHighlight } from '@/app/workspace/[workspaceId]/ import { useDependsOnGate } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-depends-on-gate' import { useSubBlockInput } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-input' import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-value' +import { parseJsonArrayValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils' import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider' import { useAccessibleReferencePrefixes } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes' import type { SubBlockConfig } from '@/blocks/types' @@ -98,22 +99,14 @@ export function DocumentTagEntry({ const currentValue = isPreview ? previewValue : storeValue - const parseTags = (tagValue: string | null): DocumentTag[] => { - if (!tagValue) return [] - try { - const parsed = JSON.parse(tagValue) - if (!Array.isArray(parsed)) return [] - return parsed.map((t: DocumentTag) => ({ - ...t, - fieldType: t.fieldType || 'text', - collapsed: t.collapsed ?? false, - })) - } catch { - return [] - } - } + const parseTags = (tagValue: unknown): DocumentTag[] => + parseJsonArrayValue(tagValue).map((t) => ({ + ...t, + fieldType: t.fieldType || 'text', + collapsed: t.collapsed ?? false, + })) - const parsedTags = parseTags(currentValue || null) + const parsedTags = parseTags(currentValue) const tags: DocumentTag[] = parsedTags.length > 0 ? parsedTags : [createDefaultTag()] const isReadOnly = isPreview || disabled diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-tag-filters/knowledge-tag-filters.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-tag-filters/knowledge-tag-filters.tsx index db9ee31e574..ba68be1525f 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-tag-filters/knowledge-tag-filters.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-tag-filters/knowledge-tag-filters.tsx @@ -21,6 +21,7 @@ import { TagDropdown } from '@/app/workspace/[workspaceId]/w/[workflowId]/compon import { getActiveWorkflowSearchHighlight } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/workflow-search-highlight' import { useDependsOnGate } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-depends-on-gate' import { useSubBlockInput } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-input' +import { parseJsonArrayValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils' import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider' import { useAccessibleReferencePrefixes } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes' import type { SubBlockConfig } from '@/blocks/types' @@ -100,24 +101,16 @@ export function KnowledgeTagFilters({ disabled, }) - const parseFilters = (filterValue: string | null): TagFilter[] => { - if (!filterValue) return [] - try { - const parsed = JSON.parse(filterValue) - if (!Array.isArray(parsed)) return [] - return parsed.map((f: TagFilter) => ({ - ...f, - fieldType: f.fieldType || 'text', - operator: f.operator || 'eq', - collapsed: f.collapsed ?? false, - })) - } catch { - return [] - } - } + const parseFilters = (filterValue: unknown): TagFilter[] => + parseJsonArrayValue(filterValue).map((f) => ({ + ...f, + fieldType: f.fieldType || 'text', + operator: f.operator || 'eq', + collapsed: f.collapsed ?? false, + })) const currentValue = isPreview ? previewValue : storeValue - const parsedFilters = parseFilters(currentValue || null) + const parsedFilters = parseFilters(currentValue) const filters: TagFilter[] = parsedFilters.length > 0 ? parsedFilters : [createDefaultFilter()] const isReadOnly = isPreview || disabled diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils.test.ts new file mode 100644 index 00000000000..1c4304512e1 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils.test.ts @@ -0,0 +1,43 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { parseJsonArrayValue } from './utils' + +interface TagFilter { + id: string + tagName: string +} + +describe('parseJsonArrayValue', () => { + it('parses a JSON string array, the shape edit_workflow now persists', () => { + const filters: TagFilter[] = [{ id: 'f1', tagName: 'Department' }] + + expect(parseJsonArrayValue(JSON.stringify(filters))).toEqual(filters) + }) + + // Rows written by builds predating the edit_workflow stringify fix still hold raw arrays. + it('passes through an already-parsed array', () => { + const filters: TagFilter[] = [{ id: 'f1', tagName: 'Department' }] + + expect(parseJsonArrayValue(filters)).toEqual(filters) + }) + + it.each([ + ['null', null], + ['undefined', undefined], + ['an empty string', ''], + ])('returns an empty array for %s', (_label, value) => { + expect(parseJsonArrayValue(value)).toEqual([]) + }) + + it.each([ + ['a malformed JSON string', '{not json'], + ['a JSON string parsing to null', 'null'], + ['a JSON string parsing to an object', '{"a":1}'], + ['a JSON string parsing to a number', '5'], + ['a bare object', { a: 1 }], + ])('returns an empty array rather than throwing for %s', (_label, value) => { + expect(parseJsonArrayValue(value)).toEqual([]) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils.ts index 1812992214f..d5d513a7b3c 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/utils.ts @@ -16,3 +16,30 @@ export function resolvePreviewContextValue(raw: unknown): unknown { } return raw } + +/** + * Parses a sub-block value that may be stored as a JSON string or as an already-parsed array. + * + * @remarks + * These sub-blocks contract on a JSON string, which is what their components write. Copilot's + * `edit_workflow` persisted them as raw arrays until it was fixed to re-serialize, so rows + * written by older builds still hold an array. Both shapes are accepted on read. + * + * The element type is asserted, not validated -- callers are responsible for defaulting any + * fields a malformed entry may be missing. + * + * @param value - The stored sub-block value, of unknown shape + * @returns The parsed array, or `[]` when the value is absent, unparseable, or not an array + */ +export function parseJsonArrayValue(value: unknown): T[] { + if (!value) return [] + let parsed: unknown = value + if (typeof value === 'string') { + try { + parsed = JSON.parse(value) + } catch { + return [] + } + } + return Array.isArray(parsed) ? (parsed as T[]) : [] +} diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.test.ts index b17b7ca32b4..95ee53b039a 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.test.ts @@ -2,7 +2,7 @@ * @vitest-environment node */ import { describe, expect, it, vi } from 'vitest' -import { createBlockFromParams } from './builders' +import { createBlockFromParams, normalizeSubblockValue } from './builders' const agentBlockConfig = { type: 'agent', @@ -20,10 +20,25 @@ const conditionBlockConfig = { subBlocks: [{ id: 'conditions', type: 'condition-input' }], } +const knowledgeBlockConfig = { + type: 'knowledge', + name: 'Knowledge', + outputs: {}, + subBlocks: [ + { id: 'tagFilters', type: 'knowledge-tag-filters' }, + { id: 'documentTags', type: 'document-tag-entry' }, + ], +} + +const blocksByType: Record = { + agent: agentBlockConfig, + condition: conditionBlockConfig, + knowledge: knowledgeBlockConfig, +} + vi.mock('@/blocks/registry', () => ({ - getAllBlocks: () => [agentBlockConfig, conditionBlockConfig], - getBlock: (type: string) => - type === 'agent' ? agentBlockConfig : type === 'condition' ? conditionBlockConfig : undefined, + getAllBlocks: () => [agentBlockConfig, conditionBlockConfig, knowledgeBlockConfig], + getBlock: (type: string) => blocksByType[type], })) describe('createBlockFromParams', () => { @@ -69,4 +84,73 @@ describe('createBlockFromParams', () => { expect(parsed[0].id).toBe('condition-1-if') expect(parsed[1].id).toBe('condition-1-else') }) + + it('persists knowledge tag subblocks as JSON strings, not raw arrays', () => { + const block = createBlockFromParams('kb-1', { + type: 'knowledge', + name: 'Knowledge 1', + inputs: { + tagFilters: [{ tagName: 'Department', tagSlot: 'tag1', tagValue: 'it' }], + documentTags: [{ tagName: 'Team', tagSlot: 'tag2', value: 'infra' }], + }, + triggerMode: false, + }) + + expect(typeof block.subBlocks.tagFilters.value).toBe('string') + expect(typeof block.subBlocks.documentTags.value).toBe('string') + + const filters = JSON.parse(block.subBlocks.tagFilters.value) + expect(filters[0].tagName).toBe('Department') + expect(filters[0].id).toEqual(expect.any(String)) + }) +}) + +describe('normalizeSubblockValue', () => { + it.each(['tagFilters', 'documentTags', 'conditions', 'routes'])( + 'serializes %s to a JSON string the subblock component can parse', + (key) => { + const result = normalizeSubblockValue(key, [{ id: 'not-a-uuid', title: 'a' }]) + + expect(typeof result).toBe('string') + expect(JSON.parse(result as string)[0].title).toBe('a') + } + ) + + it('accepts a JSON string as input and still returns a string', () => { + const result = normalizeSubblockValue('tagFilters', JSON.stringify([{ tagName: 'Department' }])) + + expect(typeof result).toBe('string') + expect(JSON.parse(result as string)[0].tagName).toBe('Department') + }) + + it('leaves array-with-id subblocks that are not string-serialized as raw arrays', () => { + const result = normalizeSubblockValue('inputFormat', [{ id: 'x', name: 'field' }]) + + expect(Array.isArray(result)).toBe(true) + }) + + it('passes through subblock keys that need no normalization', () => { + expect(normalizeSubblockValue('systemPrompt', 'hello')).toBe('hello') + }) + + // Validation treats null as an explicit clear. Coercing it to "[]" would persist a value + // where the caller asked for none, so the agent reads back an empty filter rather than an + // absent one -- the same absent-vs-empty ambiguity that caused the original data loss. + it.each(['tagFilters', 'documentTags', 'conditions', 'routes'])( + 'passes a null %s through as a clear rather than serializing it to "[]"', + (key) => { + expect(normalizeSubblockValue(key, null)).toBeNull() + expect(normalizeSubblockValue(key, undefined)).toBeUndefined() + } + ) + + it('still serializes an explicitly empty array, which clears the field with a value', () => { + expect(normalizeSubblockValue('tagFilters', [])).toBe('[]') + }) + + it('replaces non-uuid ids so copilot-authored rows match UI-created ones', () => { + const result = normalizeSubblockValue('tagFilters', [{ id: 'filter-1', tagName: 'Department' }]) + + expect(JSON.parse(result as string)[0].id).not.toBe('filter-1') + }) }) diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts index f27adb77b4e..c4653c3c603 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts @@ -93,15 +93,7 @@ export function createBlockFromParams( return } - let sanitizedValue = value - - // Normalize array subblocks with id fields (inputFormat, table rows, etc.) - if (shouldNormalizeArrayIds(key)) { - sanitizedValue = normalizeArrayWithIds(value) - if (JSON_STRING_SUBBLOCK_KEYS.has(key)) { - sanitizedValue = JSON.stringify(sanitizedValue) - } - } + let sanitizedValue = normalizeSubblockValue(key, value) sanitizedValue = normalizeConditionRouterIds(blockId, key, sanitizedValue) @@ -274,29 +266,35 @@ const ARRAY_WITH_ID_SUBBLOCK_TYPES = new Set([ * Subblock keys whose UI components expect a JSON string, not a raw array. * After normalizeArrayWithIds returns an array, these must be re-stringified. */ -export const JSON_STRING_SUBBLOCK_KEYS = new Set(['conditions', 'routes']) +const JSON_STRING_SUBBLOCK_KEYS = new Set(['conditions', 'routes', 'tagFilters', 'documentTags']) + +/** + * Coerces a subblock value to an array, accepting either a raw array or the JSON string + * the string-serialized subblocks persist. + * + * @returns The array, or `null` when the value is not an array and does not parse to one. + * Callers supply their own fallback, which differs by site. + */ +function parseJsonArray(value: unknown): any[] | null { + if (Array.isArray(value)) return value + if (typeof value !== 'string') return null + + try { + const parsed = JSON.parse(value) + return Array.isArray(parsed) ? parsed : null + } catch { + return null + } +} /** * Normalizes array subblock values by ensuring each item has a valid UUID. * The LLM may generate arbitrary IDs like "input-desc-001" or "row-1" which need * to be converted to proper UUIDs for consistency with UI-created items. */ -export function normalizeArrayWithIds(value: unknown): any[] { - let arr: any[] - - if (Array.isArray(value)) { - arr = value - } else if (typeof value === 'string') { - try { - const parsed = JSON.parse(value) - if (!Array.isArray(parsed)) return [] - arr = parsed - } catch { - return [] - } - } else { - return [] - } +function normalizeArrayWithIds(value: unknown): any[] { + const arr = parseJsonArray(value) + if (!arr) return [] return arr.map((item: any) => { if (!item || typeof item !== 'object') { @@ -315,10 +313,30 @@ export function normalizeArrayWithIds(value: unknown): any[] { /** * Checks if a subblock key should have its array items normalized with UUIDs. */ -export function shouldNormalizeArrayIds(key: string): boolean { +function shouldNormalizeArrayIds(key: string): boolean { return ARRAY_WITH_ID_SUBBLOCK_TYPES.has(key) } +/** + * Normalizes an array-with-id subblock value, re-serializing it to a JSON string for the + * subblock keys whose UI components read a string rather than a raw array. + * + * Every write path that persists LLM-supplied subblock values must route through this so the + * two concerns cannot drift apart; returns non-array-with-id values untouched. + * + * @remarks + * A nullish value passes through unchanged. `validateValueForSubBlockType` treats null as an + * explicit clear, and coercing it to `"[]"` here would persist a value where the caller asked + * for none -- leaving `sanitizeForCopilot` to show the agent an empty filter rather than an + * absent one, and callers that branch on the field's presence to see it as set. + */ +export function normalizeSubblockValue(key: string, value: unknown): unknown { + if (!shouldNormalizeArrayIds(key)) return value + if (value === null || value === undefined) return value + const normalized = normalizeArrayWithIds(value) + return JSON_STRING_SUBBLOCK_KEYS.has(key) ? JSON.stringify(normalized) : normalized +} + /** * Normalizes condition/router branch IDs to use canonical block-scoped format. * The LLM provides branch structure (if/else-if/else or routes) but should not @@ -327,19 +345,8 @@ export function shouldNormalizeArrayIds(key: string): boolean { export function normalizeConditionRouterIds(blockId: string, key: string, value: unknown): unknown { if (key !== 'conditions' && key !== 'routes') return value - let parsed: any[] - if (typeof value === 'string') { - try { - parsed = JSON.parse(value) - if (!Array.isArray(parsed)) return value - } catch { - return value - } - } else if (Array.isArray(value)) { - parsed = value - } else { - return value - } + const parsed = parseJsonArray(value) + if (!parsed) return value let elseIfCounter = 0 const normalized = parsed.map((item, index) => { diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts index b2e27179d0b..7914f4eaf59 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts @@ -221,6 +221,35 @@ describe('handleEditOperation nestedNodes merge', () => { expect(state.blocks['new-agent']).toBeUndefined() }) + it('persists string-serialized subblocks as JSON strings on merged children', () => { + const workflow = makeLoopWorkflow() + + const { state } = applyOperationsToWorkflowState(workflow, [ + { + operation_type: 'edit', + block_id: 'loop-1', + params: { + nestedNodes: { + 'new-condition': { + type: 'condition', + name: 'Condition 1', + inputs: { + conditions: [ + { id: 'x', title: 'if', value: 'x > 1' }, + { id: 'y', title: 'else', value: '' }, + ], + }, + }, + }, + }, + }, + ]) + + const value = state.blocks['condition-1'].subBlocks.conditions.value + expect(typeof value).toBe('string') + expect(JSON.parse(value as string)[0].title).toBe('if') + }) + it('preserves edges for matched children when connections are not provided', () => { const workflow = makeLoopWorkflow() diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts index 8e7e33df487..015791daad1 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts @@ -8,12 +8,10 @@ import { applyTriggerConfigToBlockSubblocks, createBlockFromParams, filterDisallowedTools, - JSON_STRING_SUBBLOCK_KEYS, - normalizeArrayWithIds, normalizeConditionRouterIds, normalizeResponseFormat, + normalizeSubblockValue, normalizeTools, - shouldNormalizeArrayIds, updateCanonicalModesForInputs, } from './builders' import type { EditWorkflowOperation, OperationContext } from './types' @@ -209,10 +207,7 @@ function mergeNestedNodesForParent( Object.entries(childValidation.validInputs).forEach(([key, value]) => { if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(key)) return - let sanitizedValue = value - if (shouldNormalizeArrayIds(key)) { - sanitizedValue = normalizeArrayWithIds(value) - } + let sanitizedValue = normalizeSubblockValue(key, value) sanitizedValue = normalizeConditionRouterIds(existingId, key, sanitizedValue) if (key === 'tools' && Array.isArray(value)) { sanitizedValue = filterDisallowedTools( @@ -444,15 +439,7 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(key)) { return } - let sanitizedValue = value - - // Normalize array subblocks with id fields (inputFormat, table rows, etc.) - if (shouldNormalizeArrayIds(key)) { - sanitizedValue = normalizeArrayWithIds(value) - if (JSON_STRING_SUBBLOCK_KEYS.has(key)) { - sanitizedValue = JSON.stringify(sanitizedValue) - } - } + let sanitizedValue = normalizeSubblockValue(key, value) sanitizedValue = normalizeConditionRouterIds(block_id, key, sanitizedValue) @@ -920,15 +907,7 @@ export function handleInsertIntoSubflowOperation( return } - let sanitizedValue = value - - // Normalize array subblocks with id fields (inputFormat, table rows, etc.) - if (shouldNormalizeArrayIds(key)) { - sanitizedValue = normalizeArrayWithIds(value) - if (JSON_STRING_SUBBLOCK_KEYS.has(key)) { - sanitizedValue = JSON.stringify(sanitizedValue) - } - } + let sanitizedValue = normalizeSubblockValue(key, value) sanitizedValue = normalizeConditionRouterIds(block_id, key, sanitizedValue) diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts index 1a83b20593a..2d18a244180 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts @@ -68,7 +68,11 @@ const knowledgeBlockConfig = { type: 'knowledge', name: 'Knowledge', outputs: {}, - subBlocks: [{ id: 'knowledgeBaseId', type: 'knowledge-base-selector' }], + subBlocks: [ + { id: 'knowledgeBaseId', type: 'knowledge-base-selector' }, + { id: 'tagFilters', type: 'knowledge-tag-filters' }, + { id: 'documentTags', type: 'document-tag-entry' }, + ], } const canonicalCredBlockConfig = { @@ -260,6 +264,47 @@ describe('validateInputsForBlock', () => { expect(result.errors[0]?.error).toContain('expected a JSON array') }) + // Without this guard, normalizeArrayWithIds coerces any unparseable value to [], which the + // write path then persists as "[]" -- silently destroying a tag filter the user configured. + it.each([ + ['a double-encoded JSON string', JSON.stringify(JSON.stringify([{ tagName: 'Department' }]))], + ['an unparseable string', 'not-json'], + ['an object', { tagName: 'Department' }], + ['a number', 5], + ])('rejects knowledge-tag-filters values that are %s', (_label, value) => { + const result = validateInputsForBlock('knowledge', { tagFilters: value }, 'kb-1') + + expect(result.validInputs.tagFilters).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0]?.error).toContain('expected a JSON array') + }) + + it('rejects non-array document-tag-entry values', () => { + const result = validateInputsForBlock('knowledge', { documentTags: 'not-json' }, 'kb-1') + + expect(result.validInputs.documentTags).toBeUndefined() + expect(result.errors).toHaveLength(1) + expect(result.errors[0]?.error).toContain('expected a JSON array') + }) + + it.each([ + ['a JSON string array', JSON.stringify([{ tagName: 'Department', tagValue: 'IT' }])], + ['a raw array', [{ tagName: 'Department', tagValue: 'IT' }]], + ['an empty array, clearing the filter', []], + ])('accepts knowledge-tag-filters values that are %s', (_label, value) => { + const result = validateInputsForBlock('knowledge', { tagFilters: value }, 'kb-1') + + expect(result.errors).toHaveLength(0) + expect(result.validInputs.tagFilters).toBeDefined() + }) + + it('accepts a null knowledge-tag-filters value so the field can still be cleared', () => { + const result = validateInputsForBlock('knowledge', { tagFilters: null }, 'kb-1') + + expect(result.errors).toHaveLength(0) + expect(result.validInputs.tagFilters).toBeNull() + }) + it('accepts known agent model ids', () => { const result = validateInputsForBlock('agent', { model: 'claude-sonnet-4-6' }, 'agent-1') diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts index 1bda4606fac..c8c02734c03 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts @@ -367,7 +367,9 @@ export function validateValueForSubBlockType( } case 'condition-input': - case 'router-input': { + case 'router-input': + case 'knowledge-tag-filters': + case 'document-tag-entry': { const parsedValue = typeof value === 'string' ? (() => { diff --git a/apps/sim/lib/copilot/vfs/serializers.test.ts b/apps/sim/lib/copilot/vfs/serializers.test.ts index adddcba0c07..8b6de1abddc 100644 --- a/apps/sim/lib/copilot/vfs/serializers.test.ts +++ b/apps/sim/lib/copilot/vfs/serializers.test.ts @@ -49,3 +49,70 @@ describe('VFS metadata serializers', () => { expect(knowledgeBase.documentCount).toBe(19) }) }) + +describe('serializeKBMeta', () => { + const baseKb = { + id: 'kb-1', + name: 'Support Docs', + description: null, + embeddingModel: 'text-embedding-3-small', + embeddingDimension: 1536, + tokenCount: 42, + createdAt: new Date('2026-01-01T00:00:00.000Z'), + updatedAt: new Date('2026-01-02T00:00:00.000Z'), + documentCount: 3, + } + + it('includes tag definitions when present', () => { + const json = JSON.parse( + serializeKBMeta({ + ...baseKb, + tagDefinitions: [ + { tagName: 'Important', tagSlot: 'tag1', fieldType: 'text' }, + { tagName: 'Department', tagSlot: 'tag2', fieldType: 'text' }, + ], + }) + ) + + const textOperators = ['eq', 'neq', 'contains', 'not_contains', 'starts_with', 'ends_with'] + expect(json.tagDefinitions).toEqual([ + { tagName: 'Important', tagSlot: 'tag1', fieldType: 'text', operators: textOperators }, + { tagName: 'Department', tagSlot: 'tag2', fieldType: 'text', operators: textOperators }, + ]) + }) + + // `between` is legal for number/date but not text/boolean -- the agent cannot infer this. + it.each([ + ['number', ['eq', 'neq', 'gt', 'gte', 'lt', 'lte', 'between']], + ['date', ['eq', 'neq', 'gt', 'gte', 'lt', 'lte', 'between']], + ['boolean', ['eq', 'neq']], + ])('exposes the operators legal for a %s tag', (fieldType, expected) => { + const json = JSON.parse( + serializeKBMeta({ + ...baseKb, + tagDefinitions: [{ tagName: 'Tag', tagSlot: 'tag1', fieldType }], + }) + ) + + expect(json.tagDefinitions[0].operators).toEqual(expected) + }) + + it('emits an empty operator list for an unrecognized field type rather than throwing', () => { + const json = JSON.parse( + serializeKBMeta({ + ...baseKb, + tagDefinitions: [{ tagName: 'Tag', tagSlot: 'tag1', fieldType: 'mystery' }], + }) + ) + + expect(json.tagDefinitions[0].operators).toEqual([]) + }) + + it('omits tag definitions when empty or undefined', () => { + const empty = JSON.parse(serializeKBMeta({ ...baseKb, tagDefinitions: [] })) + const missing = JSON.parse(serializeKBMeta(baseKb)) + + expect(empty).not.toHaveProperty('tagDefinitions') + expect(missing).not.toHaveProperty('tagDefinitions') + }) +}) diff --git a/apps/sim/lib/copilot/vfs/serializers.ts b/apps/sim/lib/copilot/vfs/serializers.ts index 7bab9f202a7..ae7a971a794 100644 --- a/apps/sim/lib/copilot/vfs/serializers.ts +++ b/apps/sim/lib/copilot/vfs/serializers.ts @@ -1,5 +1,6 @@ import { getCopilotToolDescription } from '@/lib/copilot/tools/descriptions' import { isHosted } from '@/lib/core/config/env-flags' +import { type FilterFieldType, getOperatorsForFieldType } from '@/lib/knowledge/filters/types' import { isSubBlockHidden } from '@/lib/workflows/subblocks/visibility' import { isCustomBlockType } from '@/blocks/custom/build-config' import type { BlockConfig, SubBlockConfig } from '@/blocks/types' @@ -84,7 +85,26 @@ export function serializeRecentExecutions( } /** - * Serialize knowledge base metadata for VFS meta.json + * A knowledge base tag definition, reduced to the fields the agent needs to bind a tag filter. + * + * @remarks + * `tagName` is the DB's `displayName`. It is renamed at this boundary because that is the key + * a `tagFilters` entry must carry -- an entry written with `displayName` validates and persists + * but never filters anything. + */ +export interface KbTagDefinitionSummary { + tagName: string + tagSlot: string + fieldType: string +} + +/** + * Serialize knowledge base metadata for VFS meta.json. + * + * `tagDefinitions` exposes the KB's defined tags (`tagName` → `tagSlot`) plus the operators + * legal for each tag's `fieldType`, so the agent can bind a knowledge-tag filter without + * guessing a tag name it cannot otherwise see or an operator the field does not accept + * (`between` is valid for number/date but not text/boolean). */ export function serializeKBMeta(kb: { id: string @@ -97,6 +117,7 @@ export function serializeKBMeta(kb: { updatedAt: Date documentCount: number connectorTypes?: string[] + tagDefinitions?: KbTagDefinitionSummary[] }): string { return JSON.stringify( { @@ -109,6 +130,15 @@ export function serializeKBMeta(kb: { documentCount: kb.documentCount, connectorTypes: kb.connectorTypes && kb.connectorTypes.length > 0 ? kb.connectorTypes : undefined, + tagDefinitions: + kb.tagDefinitions && kb.tagDefinitions.length > 0 + ? kb.tagDefinitions.map((tag) => ({ + ...tag, + operators: getOperatorsForFieldType(tag.fieldType as FilterFieldType).map( + (op) => op.value + ), + })) + : undefined, createdAt: kb.createdAt.toISOString(), updatedAt: kb.updatedAt.toISOString(), }, diff --git a/apps/sim/lib/copilot/vfs/workspace-vfs.ts b/apps/sim/lib/copilot/vfs/workspace-vfs.ts index a76bc739144..851af57e1bf 100644 --- a/apps/sim/lib/copilot/vfs/workspace-vfs.ts +++ b/apps/sim/lib/copilot/vfs/workspace-vfs.ts @@ -6,6 +6,7 @@ import { customTools as customToolsTable, document, jobExecutionLogs, + knowledgeBaseTagDefinitions, knowledgeConnector, mcpServers as mcpServersTable, skill as skillTable, @@ -18,7 +19,7 @@ import { } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' -import { and, desc, eq, isNotNull, isNull, ne, or, sql } from 'drizzle-orm' +import { and, desc, eq, inArray, isNotNull, isNull, ne, or, sql } from 'drizzle-orm' import { listApiKeys } from '@/lib/api-key/service' import { buildWorkspaceContextMd, @@ -50,7 +51,7 @@ import { canonicalWorkspaceFilePath, encodeVfsPathSegments, } from '@/lib/copilot/vfs/path-utils' -import type { DeploymentData } from '@/lib/copilot/vfs/serializers' +import type { DeploymentData, KbTagDefinitionSummary } from '@/lib/copilot/vfs/serializers' import { serializeApiKeys, serializeBlockSchema, @@ -1455,6 +1456,8 @@ export class WorkspaceVFS { ): Promise { const kbs = await getKnowledgeBases(userId, workspaceId) + const tagDefinitionsByKb = await this.loadKbTagDefinitions(kbs.map((kb) => kb.id)) + await Promise.all( kbs.map(async (kb) => { const safeName = sanitizeName(kb.name) @@ -1473,6 +1476,7 @@ export class WorkspaceVFS { updatedAt: kb.updatedAt, documentCount: kb.docCount, connectorTypes: kb.connectorTypes, + tagDefinitions: tagDefinitionsByKb.get(kb.id), }) ) @@ -1544,6 +1548,61 @@ export class WorkspaceVFS { })) } + /** + * Load tag definitions for the given knowledge bases in a single query, grouped by + * KB id and ordered by tag slot. Surfaced inline in each KB's meta.json so the agent + * knows which tags exist (and their slot binding) when editing a knowledge-tag filter. + * + * @remarks + * Tag definitions are an optional enrichment, so a query failure degrades to a meta.json + * without them rather than rejecting. This materializer runs inside the top-level + * `Promise.all`, whose rejection would fail the entire workspace VFS build and leave the + * agent unable to read any file. + */ + private async loadKbTagDefinitions( + kbIds: string[] + ): Promise> { + const byKb = new Map() + if (kbIds.length === 0) return byKb + + let rows: Array<{ + knowledgeBaseId: string + tagSlot: string + displayName: string + fieldType: string + }> + try { + rows = await db + .select({ + knowledgeBaseId: knowledgeBaseTagDefinitions.knowledgeBaseId, + tagSlot: knowledgeBaseTagDefinitions.tagSlot, + displayName: knowledgeBaseTagDefinitions.displayName, + fieldType: knowledgeBaseTagDefinitions.fieldType, + }) + .from(knowledgeBaseTagDefinitions) + .where(inArray(knowledgeBaseTagDefinitions.knowledgeBaseId, kbIds)) + .orderBy(knowledgeBaseTagDefinitions.tagSlot) + } catch (err) { + logger.warn('Failed to load knowledge base tag definitions', { + error: toError(err).message, + }) + return byKb + } + + for (const row of rows) { + const entry = { + tagName: row.displayName, + tagSlot: row.tagSlot, + fieldType: row.fieldType, + } + const existing = byKb.get(row.knowledgeBaseId) + if (existing) existing.push(entry) + else byKb.set(row.knowledgeBaseId, [entry]) + } + + return byKb + } + /** * Materialize tables using the shared listTables function. * Returns a summary for WORKSPACE.md generation. diff --git a/apps/sim/lib/workflows/sanitization/json-sanitizer.test.ts b/apps/sim/lib/workflows/sanitization/json-sanitizer.test.ts new file mode 100644 index 00000000000..72cfcde085f --- /dev/null +++ b/apps/sim/lib/workflows/sanitization/json-sanitizer.test.ts @@ -0,0 +1,67 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer' +import type { WorkflowState } from '@/stores/workflows/workflow/types' + +/** + * Builds a minimal one-block workflow whose knowledge block carries the two + * subblock keys `edit_workflow` is allowed to write. + */ +function makeKnowledgeWorkflow(tagFiltersValue: unknown) { + return { + blocks: { + 'kb-1': { + id: 'kb-1', + type: 'knowledge', + name: 'Knowledge 1', + position: { x: 0, y: 0 }, + enabled: true, + outputs: {}, + subBlocks: { + operation: { id: 'operation', type: 'dropdown', value: 'search' }, + tagFilters: { id: 'tagFilters', type: 'knowledge-tag-filters', value: tagFiltersValue }, + documentTags: { + id: 'documentTags', + type: 'document-tag-entry', + value: JSON.stringify([{ id: 't1', tagName: 'Team' }]), + }, + }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + } as unknown as WorkflowState +} + +describe('sanitizeForCopilot knowledge tag subblocks', () => { + // Regression: these keys were stripped, which made them write-only for the agent -- + // edit_workflow could set a tag filter but the agent read back an absent field and + // cleared the user's filter on the next edit. + it('retains tagFilters so the agent can read back what edit_workflow writes', () => { + const value = JSON.stringify([ + { id: 'f1', tagName: 'Department', tagSlot: 'tag1', tagValue: 'it' }, + ]) + + const result = sanitizeForCopilot(makeKnowledgeWorkflow(value)) + const inputs = result.blocks['kb-1'].inputs + + expect(inputs?.tagFilters).toBe(value) + }) + + it('retains documentTags alongside tagFilters', () => { + const result = sanitizeForCopilot(makeKnowledgeWorkflow(JSON.stringify([]))) + const inputs = result.blocks['kb-1'].inputs + + expect(inputs?.documentTags).toBeDefined() + }) + + it('still omits the key when no filter is set, so absent means unset', () => { + const result = sanitizeForCopilot(makeKnowledgeWorkflow(null)) + const inputs = result.blocks['kb-1'].inputs + + expect(inputs).not.toHaveProperty('tagFilters') + }) +}) diff --git a/apps/sim/lib/workflows/sanitization/json-sanitizer.ts b/apps/sim/lib/workflows/sanitization/json-sanitizer.ts index 04802b76e49..1879fae0273 100644 --- a/apps/sim/lib/workflows/sanitization/json-sanitizer.ts +++ b/apps/sim/lib/workflows/sanitization/json-sanitizer.ts @@ -254,6 +254,13 @@ function isToolInput(value: unknown): value is ToolInput { /** * Sanitize subblocks by removing null values and simplifying structure * Maps each subblock key directly to its value instead of the full object + * + * @remarks + * `tagFilters` and `documentTags` are deliberately retained. This is the copilot's read + * view of workflow state, and `edit_workflow` can write both keys, so dropping them here + * makes the field write-only: the agent reads back an absent field and clears the user's + * filter on the next edit. Redaction for shared/exported workflows is a separate concern, + * already handled by `sanitizeWorkflowForSharing`. */ function sanitizeSubBlocks( subBlocks: BlockState['subBlocks'] @@ -310,11 +317,6 @@ function sanitizeSubBlocks( return } - // Skip knowledge base tag filters and document tags (workspace-specific data) - if (key === 'tagFilters' || key === 'documentTags') { - return - } - sanitized[key] = subBlock.value })