fix(kbs): legacy subblock id migration + CI check

icecrasher321 · icecrasher321 · commit 8e116d67eb51 · 2026-03-05T11:59:20.000-08:00
diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml
@@ -90,6 +90,16 @@ jobs:
 
           echo "✅ All feature flags are properly configured"
 
+      - name: Check subblock ID stability
+        run: |
+          if [ "${{ github.event_name }}" = "pull_request" ]; then
+            BASE_REF="origin/${{ github.base_ref }}"
+            git fetch --depth=1 origin "${{ github.base_ref }}" 2>/dev/null || true
+          else
+            BASE_REF="HEAD~1"
+          fi
+          bun run apps/sim/scripts/check-subblock-id-stability.ts "$BASE_REF"
+
       - name: Lint code
         run: bun run lint:check
 
diff --git a/apps/sim/lib/workflows/migrations/subblock-migrations.test.ts b/apps/sim/lib/workflows/migrations/subblock-migrations.test.ts
@@ -0,0 +1,162 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import type { BlockState } from '@/stores/workflows/workflow/types'
+import { migrateSubblockIds } from './subblock-migrations'
+
+function makeBlock(overrides: Partial<BlockState> & { type: string }): BlockState {
+  return {
+    id: 'block-1',
+    name: 'Test',
+    position: { x: 0, y: 0 },
+    subBlocks: {},
+    outputs: {},
+    enabled: true,
+    ...overrides,
+  } as BlockState
+}
+
+describe('migrateSubblockIds', () => {
+  describe('knowledge block', () => {
+    it('should rename knowledgeBaseId to knowledgeBaseSelector', () => {
+      const blocks: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'knowledge',
+          subBlocks: {
+            operation: { id: 'operation', type: 'dropdown', value: 'search' },
+            knowledgeBaseId: {
+              id: 'knowledgeBaseId',
+              type: 'knowledge-base-selector',
+              value: 'kb-uuid-123',
+            },
+          },
+        }),
+      }
+
+      const migrated = migrateSubblockIds(blocks)
+
+      expect(migrated).toBe(true)
+      expect(blocks['b1'].subBlocks['knowledgeBaseSelector']).toEqual({
+        id: 'knowledgeBaseSelector',
+        type: 'knowledge-base-selector',
+        value: 'kb-uuid-123',
+      })
+      expect(blocks['b1'].subBlocks['knowledgeBaseId']).toBeUndefined()
+      expect(blocks['b1'].subBlocks['operation'].value).toBe('search')
+    })
+
+    it('should prefer new key when both old and new exist', () => {
+      const blocks: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'knowledge',
+          subBlocks: {
+            knowledgeBaseId: {
+              id: 'knowledgeBaseId',
+              type: 'knowledge-base-selector',
+              value: 'stale-kb',
+            },
+            knowledgeBaseSelector: {
+              id: 'knowledgeBaseSelector',
+              type: 'knowledge-base-selector',
+              value: 'fresh-kb',
+            },
+          },
+        }),
+      }
+
+      const migrated = migrateSubblockIds(blocks)
+
+      expect(migrated).toBe(true)
+      expect(blocks['b1'].subBlocks['knowledgeBaseSelector'].value).toBe('fresh-kb')
+      expect(blocks['b1'].subBlocks['knowledgeBaseId']).toBeUndefined()
+    })
+
+    it('should not touch blocks that already use the new key', () => {
+      const blocks: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'knowledge',
+          subBlocks: {
+            knowledgeBaseSelector: {
+              id: 'knowledgeBaseSelector',
+              type: 'knowledge-base-selector',
+              value: 'kb-uuid',
+            },
+          },
+        }),
+      }
+
+      const migrated = migrateSubblockIds(blocks)
+
+      expect(migrated).toBe(false)
+      expect(blocks['b1'].subBlocks['knowledgeBaseSelector'].value).toBe('kb-uuid')
+    })
+  })
+
+  it('should skip blocks with no registered migrations', () => {
+    const blocks: Record<string, BlockState> = {
+      b1: makeBlock({
+        type: 'function',
+        subBlocks: {
+          code: { id: 'code', type: 'code', value: 'console.log("hi")' },
+        },
+      }),
+    }
+
+    const migrated = migrateSubblockIds(blocks)
+
+    expect(migrated).toBe(false)
+    expect(blocks['b1'].subBlocks['code'].value).toBe('console.log("hi")')
+  })
+
+  it('should migrate multiple blocks in one pass', () => {
+    const blocks: Record<string, BlockState> = {
+      b1: makeBlock({
+        id: 'b1',
+        type: 'knowledge',
+        subBlocks: {
+          knowledgeBaseId: {
+            id: 'knowledgeBaseId',
+            type: 'knowledge-base-selector',
+            value: 'kb-1',
+          },
+        },
+      }),
+      b2: makeBlock({
+        id: 'b2',
+        type: 'knowledge',
+        subBlocks: {
+          knowledgeBaseId: {
+            id: 'knowledgeBaseId',
+            type: 'knowledge-base-selector',
+            value: 'kb-2',
+          },
+        },
+      }),
+      b3: makeBlock({
+        id: 'b3',
+        type: 'function',
+        subBlocks: {
+          code: { id: 'code', type: 'code', value: '' },
+        },
+      }),
+    }
+
+    const migrated = migrateSubblockIds(blocks)
+
+    expect(migrated).toBe(true)
+    expect(blocks['b1'].subBlocks['knowledgeBaseSelector'].value).toBe('kb-1')
+    expect(blocks['b2'].subBlocks['knowledgeBaseSelector'].value).toBe('kb-2')
+    expect(blocks['b3'].subBlocks['code']).toBeDefined()
+  })
+
+  it('should handle blocks with empty subBlocks', () => {
+    const blocks: Record<string, BlockState> = {
+      b1: makeBlock({ type: 'knowledge', subBlocks: {} }),
+    }
+
+    const migrated = migrateSubblockIds(blocks)
+
+    expect(migrated).toBe(false)
+  })
+})
diff --git a/apps/sim/lib/workflows/migrations/subblock-migrations.ts b/apps/sim/lib/workflows/migrations/subblock-migrations.ts
@@ -0,0 +1,77 @@
+import { createLogger } from '@sim/logger'
+import type { BlockState } from '@/stores/workflows/workflow/types'
+
+const logger = createLogger('SubblockMigrations')
+
+/**
+ * Maps old subblock IDs to their current equivalents per block type.
+ *
+ * When a subblock is renamed in a block definition, old deployed/saved states
+ * still carry the value under the previous key. Without this mapping the
+ * serializer silently drops the value, breaking execution.
+ *
+ * Format: { blockType: { oldSubblockId: newSubblockId } }
+ */
+export const SUBBLOCK_ID_MIGRATIONS: Record<string, Record<string, string>> = {
+  knowledge: {
+    knowledgeBaseId: 'knowledgeBaseSelector',
+  },
+}
+
+/**
+ * Migrates legacy subblock IDs inside a single block's subBlocks map.
+ * If an old key is found and the new key does not already exist, the entry
+ * is moved to the new key. When both exist the new key wins (user already
+ * re-saved) and the old entry is removed to avoid orphans.
+ *
+ * Returns true if any migration was applied.
+ */
+function migrateBlockSubblockIds(block: BlockState, renames: Record<string, string>): boolean {
+  const subBlocks = block.subBlocks
+  if (!subBlocks) return false
+
+  let migrated = false
+
+  for (const [oldId, newId] of Object.entries(renames)) {
+    if (!(oldId in subBlocks)) continue
+
+    if (newId in subBlocks) {
+      delete subBlocks[oldId]
+      migrated = true
+      continue
+    }
+
+    const oldEntry = subBlocks[oldId]
+    subBlocks[newId] = { ...oldEntry, id: newId }
+    delete subBlocks[oldId]
+    migrated = true
+  }
+
+  return migrated
+}
+
+/**
+ * Applies subblock-ID migrations to every block in a workflow.
+ * Safe to call on any state – blocks whose type has no registered
+ * migrations are left untouched.
+ *
+ * Mutates `blocks` in place and returns whether anything changed.
+ */
+export function migrateSubblockIds(blocks: Record<string, BlockState>): boolean {
+  let anyMigrated = false
+
+  for (const block of Object.values(blocks)) {
+    const renames = SUBBLOCK_ID_MIGRATIONS[block.type]
+    if (!renames) continue
+
+    if (migrateBlockSubblockIds(block, renames)) {
+      logger.info('Migrated legacy subblock IDs', {
+        blockId: block.id,
+        blockType: block.type,
+      })
+      anyMigrated = true
+    }
+  }
+
+  return anyMigrated
+}
diff --git a/apps/sim/lib/workflows/persistence/utils.ts b/apps/sim/lib/workflows/persistence/utils.ts
@@ -14,6 +14,7 @@ import { and, desc, eq, inArray, sql } from 'drizzle-orm'
 import type { Edge } from 'reactflow'
 import { v4 as uuidv4 } from 'uuid'
 import type { DbOrTx } from '@/lib/db/types'
+import { migrateSubblockIds } from '@/lib/workflows/migrations/subblock-migrations'
 import { sanitizeAgentToolsInBlocks } from '@/lib/workflows/sanitization/validation'
 import type { BlockState, Loop, Parallel, WorkflowState } from '@/stores/workflows/workflow/types'
 import { SUBFLOW_TYPES } from '@/stores/workflows/workflow/types'
@@ -114,12 +115,14 @@ export async function loadDeployedWorkflowState(
     }
 
     const resolvedBlocks = state.blocks || {}
-    const { blocks: migratedBlocks } = resolvedWorkspaceId
+    const { blocks: credMigratedBlocks } = resolvedWorkspaceId
       ? await migrateCredentialIds(resolvedBlocks, resolvedWorkspaceId)
       : { blocks: resolvedBlocks }
 
+    migrateSubblockIds(credMigratedBlocks)
+
     return {
-      blocks: migratedBlocks,
+      blocks: credMigratedBlocks,
       edges: state.edges || [],
       loops: state.loops || {},
       parallels: state.parallels || {},
@@ -386,6 +389,22 @@ export async function loadWorkflowFromNormalizedTables(
       })
     }
 
+    const subblockMigrated = migrateSubblockIds(credMigratedBlocks)
+    if (subblockMigrated) {
+      Promise.resolve().then(async () => {
+        try {
+          for (const [blockId, block] of Object.entries(credMigratedBlocks)) {
+            await db
+              .update(workflowBlocks)
+              .set({ subBlocks: block.subBlocks, updatedAt: new Date() })
+              .where(and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId)))
+          }
+        } catch (err) {
+          logger.warn('Failed to persist subblock ID migration', { workflowId, error: err })
+        }
+      })
+    }
+
     // Convert edges to the expected format
     const edgesArray: Edge[] = edges.map((edge) => ({
       id: edge.id,
diff --git a/apps/sim/scripts/check-subblock-id-stability.ts b/apps/sim/scripts/check-subblock-id-stability.ts
