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

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

* cleanup migration code

* address regex inaccuracy

.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
 

apps/sim/lib/workflows/migrations/subblock-migrations.test.ts

@@ -0,0 +1,183 @@
+/**
+ * @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 input: 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 { blocks, migrated } = migrateSubblockIds(input)
+
+      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 input: 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 { blocks, migrated } = migrateSubblockIds(input)
+
+      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 input: Record<string, BlockState> = {
+        b1: makeBlock({
+          type: 'knowledge',
+          subBlocks: {
+            knowledgeBaseSelector: {
+              id: 'knowledgeBaseSelector',
+              type: 'knowledge-base-selector',
+              value: 'kb-uuid',
+            },
+          },
+        }),
+      }
+
+      const { blocks, migrated } = migrateSubblockIds(input)
+
+      expect(migrated).toBe(false)
+      expect(blocks['b1'].subBlocks['knowledgeBaseSelector'].value).toBe('kb-uuid')
+    })
+  })
+
+  it('should not mutate the input blocks', () => {
+    const input: Record<string, BlockState> = {
+      b1: makeBlock({
+        type: 'knowledge',
+        subBlocks: {
+          knowledgeBaseId: {
+            id: 'knowledgeBaseId',
+            type: 'knowledge-base-selector',
+            value: 'kb-uuid',
+          },
+        },
+      }),
+    }
+
+    const { blocks } = migrateSubblockIds(input)
+
+    expect(input['b1'].subBlocks['knowledgeBaseId']).toBeDefined()
+    expect(blocks['b1'].subBlocks['knowledgeBaseSelector']).toBeDefined()
+    expect(blocks).not.toBe(input)
+  })
+
+  it('should skip blocks with no registered migrations', () => {
+    const input: Record<string, BlockState> = {
+      b1: makeBlock({
+        type: 'function',
+        subBlocks: {
+          code: { id: 'code', type: 'code', value: 'console.log("hi")' },
+        },
+      }),
+    }
+
+    const { blocks, migrated } = migrateSubblockIds(input)
+
+    expect(migrated).toBe(false)
+    expect(blocks['b1'].subBlocks['code'].value).toBe('console.log("hi")')
+  })
+
+  it('should migrate multiple blocks in one pass', () => {
+    const input: 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 { blocks, migrated } = migrateSubblockIds(input)
+
+    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 input: Record<string, BlockState> = {
+      b1: makeBlock({ type: 'knowledge', subBlocks: {} }),
+    }
+
+    const { migrated } = migrateSubblockIds(input)
+
+    expect(migrated).toBe(false)
+  })
+})

apps/sim/lib/workflows/migrations/subblock-migrations.ts

@@ -0,0 +1,90 @@
+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.
+ * Returns a new subBlocks record if anything changed, or the original if not.
+ */
+function migrateBlockSubblockIds(
+  subBlocks: Record<string, BlockState['subBlocks'][string]>,
+  renames: Record<string, string>
+): { subBlocks: Record<string, BlockState['subBlocks'][string]>; migrated: boolean } {
+  let migrated = false
+
+  for (const oldId of Object.keys(renames)) {
+    if (oldId in subBlocks) {
+      migrated = true
+      break
+    }
+  }
+
+  if (!migrated) return { subBlocks, migrated: false }
+
+  const result = { ...subBlocks }
+
+  for (const [oldId, newId] of Object.entries(renames)) {
+    if (!(oldId in result)) continue
+
+    if (newId in result) {
+      delete result[oldId]
+      continue
+    }
+
+    const oldEntry = result[oldId]
+    result[newId] = { ...oldEntry, id: newId }
+    delete result[oldId]
+  }
+
+  return { subBlocks: result, migrated: true }
+}
+
+/**
+ * Applies subblock-ID migrations to every block in a workflow.
+ * Returns a new blocks record with migrated subBlocks where needed.
+ */
+export function migrateSubblockIds(blocks: Record<string, BlockState>): {
+  blocks: Record<string, BlockState>
+  migrated: boolean
+} {
+  let anyMigrated = false
+  const result: Record<string, BlockState> = {}
+
+  for (const [blockId, block] of Object.entries(blocks)) {
+    const renames = SUBBLOCK_ID_MIGRATIONS[block.type]
+    if (!renames || !block.subBlocks) {
+      result[blockId] = block
+      continue
+    }
+
+    const { subBlocks, migrated } = migrateBlockSubblockIds(block.subBlocks, renames)
+    if (migrated) {
+      logger.info('Migrated legacy subblock IDs', {
+        blockId: block.id,
+        blockType: block.type,
+      })
+      anyMigrated = true
+      result[blockId] = { ...block, subBlocks }
+    } else {
+      result[blockId] = block
+    }
+  }
+
+  return { blocks: result, migrated: anyMigrated }
+}

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'
@@ -113,10 +114,10 @@ export async function loadDeployedWorkflowState(
       resolvedWorkspaceId = wfRow?.workspaceId ?? undefined
     }
 
-    const resolvedBlocks = state.blocks || {}
-    const { blocks: migratedBlocks } = resolvedWorkspaceId
-      ? await migrateCredentialIds(resolvedBlocks, resolvedWorkspaceId)
-      : { blocks: resolvedBlocks }
+    const { blocks: migratedBlocks } = await applyBlockMigrations(
+      state.blocks || {},
+      resolvedWorkspaceId
+    )
 
     return {
       blocks: migratedBlocks,
@@ -133,6 +134,50 @@ export async function loadDeployedWorkflowState(
   }
 }
 
+interface MigrationContext {
+  blocks: Record<string, BlockState>
+  workspaceId?: string
+  migrated: boolean
+}
+
+type BlockMigration = (ctx: MigrationContext) => MigrationContext | Promise<MigrationContext>
+
+function createMigrationPipeline(migrations: BlockMigration[]) {
+  return async (
+    blocks: Record<string, BlockState>,
+    workspaceId?: string
+  ): Promise<{ blocks: Record<string, BlockState>; migrated: boolean }> => {
+    let ctx: MigrationContext = { blocks, workspaceId, migrated: false }
+    for (const migration of migrations) {
+      ctx = await migration(ctx)
+    }
+    return { blocks: ctx.blocks, migrated: ctx.migrated }
+  }
+}
+
+const applyBlockMigrations = createMigrationPipeline([
+  (ctx) => {
+    const { blocks } = sanitizeAgentToolsInBlocks(ctx.blocks)
+    return { ...ctx, blocks }
+  },
+
+  (ctx) => ({
+    ...ctx,
+    blocks: migrateAgentBlocksToMessagesFormat(ctx.blocks),
+  }),
+
+  async (ctx) => {
+    if (!ctx.workspaceId) return ctx
+    const { blocks, migrated } = await migrateCredentialIds(ctx.blocks, ctx.workspaceId)
+    return { ...ctx, blocks, migrated: ctx.migrated || migrated }
+  },
+
+  (ctx) => {
+    const { blocks, migrated } = migrateSubblockIds(ctx.blocks)
+    return { ...ctx, blocks, migrated: ctx.migrated || migrated }
+  },
+])
+
 /**
  * Migrates agent blocks from old format (systemPrompt/userPrompt) to new format (messages array)
  * This ensures backward compatibility for workflows created before the messages-input refactor.
@@ -356,22 +401,16 @@ export async function loadWorkflowFromNormalizedTables(
       blocksMap[block.id] = assembled
     })
 
-    // Sanitize any invalid custom tools in agent blocks to prevent client crashes
-    const { blocks: sanitizedBlocks } = sanitizeAgentToolsInBlocks(blocksMap)
+    const { blocks: finalBlocks, migrated } = await applyBlockMigrations(
+      blocksMap,
+      workflowRow?.workspaceId ?? undefined
+    )
 
-    // Migrate old agent block format (systemPrompt/userPrompt) to new messages array format
-    const migratedBlocks = migrateAgentBlocksToMessagesFormat(sanitizedBlocks)
-
-    // Migrate legacy account.id → credential.id in OAuth subblocks
-    const { blocks: credMigratedBlocks, migrated: credentialsMigrated } = workflowRow?.workspaceId
-      ? await migrateCredentialIds(migratedBlocks, workflowRow.workspaceId)
-      : { blocks: migratedBlocks, migrated: false }
-
-    if (credentialsMigrated) {
+    if (migrated) {
       Promise.resolve().then(async () => {
         try {
-          for (const [blockId, block] of Object.entries(credMigratedBlocks)) {
-            if (block.subBlocks !== migratedBlocks[blockId]?.subBlocks) {
+          for (const [blockId, block] of Object.entries(finalBlocks)) {
+            if (block.subBlocks !== blocksMap[blockId]?.subBlocks) {
               await db
                 .update(workflowBlocks)
                 .set({ subBlocks: block.subBlocks, updatedAt: new Date() })
@@ -381,7 +420,7 @@ export async function loadWorkflowFromNormalizedTables(
             }
           }
         } catch (err) {
-          logger.warn('Failed to persist credential ID migration', { workflowId, error: err })
+          logger.warn('Failed to persist block migrations', { workflowId, error: err })
         }
       })
     }
@@ -422,13 +461,13 @@ export async function loadWorkflowFromNormalizedTables(
           forEachItems: (config as Loop).forEachItems ?? '',
           whileCondition: (config as Loop).whileCondition ?? '',
           doWhileCondition: (config as Loop).doWhileCondition ?? '',
-          enabled: credMigratedBlocks[subflow.id]?.enabled ?? true,
+          enabled: finalBlocks[subflow.id]?.enabled ?? true,
         }
         loops[subflow.id] = loop
 
-        if (credMigratedBlocks[subflow.id]) {
-          const block = credMigratedBlocks[subflow.id]
-          credMigratedBlocks[subflow.id] = {
+        if (finalBlocks[subflow.id]) {
+          const block = finalBlocks[subflow.id]
+          finalBlocks[subflow.id] = {
             ...block,
             data: {
               ...block.data,
@@ -449,7 +488,7 @@ export async function loadWorkflowFromNormalizedTables(
             (config as Parallel).parallelType === 'collection'
               ? (config as Parallel).parallelType
               : 'count',
-          enabled: credMigratedBlocks[subflow.id]?.enabled ?? true,
+          enabled: finalBlocks[subflow.id]?.enabled ?? true,
         }
         parallels[subflow.id] = parallel
       } else {
@@ -458,7 +497,7 @@ export async function loadWorkflowFromNormalizedTables(
     })
 
     return {
-      blocks: credMigratedBlocks,
+      blocks: finalBlocks,
       edges: edgesArray,
       loops,
       parallels,

apps/sim/scripts/check-subblock-id-stability.ts

@@ -0,0 +1,170 @@
+#!/usr/bin/env bun
+
+/**
+ * CI check: detect subblock ID renames that would break deployed workflows.
+ *
+ * Compares the current block registry against the parent commit.
+ * If any subblock ID was removed from a block, it must have a corresponding
+ * entry in SUBBLOCK_ID_MIGRATIONS — otherwise this script exits non-zero.
+ *
+ * Usage:
+ *   bun run apps/sim/scripts/check-subblock-id-stability.ts [base-ref]
+ *
+ * base-ref defaults to HEAD~1.  In a PR CI pipeline, pass the merge base:
+ *   bun run apps/sim/scripts/check-subblock-id-stability.ts origin/main
+ */
+
+import { execSync } from 'child_process'
+import { SUBBLOCK_ID_MIGRATIONS } from '@/lib/workflows/migrations/subblock-migrations'
+import { getAllBlocks } from '@/blocks/registry'
+
+const baseRef = process.argv[2] || 'HEAD~1'
+
+const gitRoot = execSync('git rev-parse --show-toplevel', { encoding: 'utf-8' }).trim()
+const gitOpts = { encoding: 'utf-8' as const, cwd: gitRoot }
+
+type IdMap = Record<string, Set<string>>
+
+/**
+ * Extracts subblock IDs from the `subBlocks: [ ... ]` section of a block
+ * definition. Only grabs the top-level `id:` of each subblock object —
+ * ignores nested IDs inside `options`, `columns`, etc.
+ */
+function extractSubBlockIds(source: string): string[] {
+  const startIdx = source.indexOf('subBlocks:')
+  if (startIdx === -1) return []
+
+  const bracketStart = source.indexOf('[', startIdx)
+  if (bracketStart === -1) return []
+
+  const ids: string[] = []
+  let braceDepth = 0
+  let bracketDepth = 0
+  let i = bracketStart + 1
+  bracketDepth = 1
+
+  while (i < source.length && bracketDepth > 0) {
+    const ch = source[i]
+
+    if (ch === '[') bracketDepth++
+    else if (ch === ']') {
+      bracketDepth--
+      if (bracketDepth === 0) break
+    } else if (ch === '{') {
+      braceDepth++
+      if (braceDepth === 1) {
+        const ahead = source.slice(i, i + 200)
+        const idMatch = ahead.match(/{\s*(?:\/\/[^\n]*\n\s*)*id:\s*['"]([^'"]+)['"]/)
+        if (idMatch) {
+          ids.push(idMatch[1])
+        }
+      }
+    } else if (ch === '}') {
+      braceDepth--
+    }
+
+    i++
+  }
+
+  return ids
+}
+
+function getCurrentIds(): IdMap {
+  const map: IdMap = {}
+  for (const block of getAllBlocks()) {
+    map[block.type] = new Set(block.subBlocks.map((sb) => sb.id))
+  }
+  return map
+}
+
+function getPreviousIds(): IdMap {
+  const registryPath = 'apps/sim/blocks/registry.ts'
+  const blocksDir = 'apps/sim/blocks/blocks'
+
+  let hasChanges = false
+  try {
+    const diff = execSync(
+      `git diff --name-only ${baseRef} HEAD -- ${registryPath} ${blocksDir}`,
+      gitOpts
+    ).trim()
+    hasChanges = diff.length > 0
+  } catch {
+    console.log('⚠ Could not diff against base ref — skipping check')
+    process.exit(0)
+  }
+
+  if (!hasChanges) {
+    console.log('✓ No block definition changes detected — nothing to check')
+    process.exit(0)
+  }
+
+  const map: IdMap = {}
+
+  try {
+    const blockFiles = execSync(`git ls-tree -r --name-only ${baseRef} -- ${blocksDir}`, gitOpts)
+      .trim()
+      .split('\n')
+      .filter((f) => f.endsWith('.ts') && !f.endsWith('.test.ts'))
+
+    for (const filePath of blockFiles) {
+      let content: string
+      try {
+        content = execSync(`git show ${baseRef}:${filePath}`, gitOpts)
+      } catch {
+        continue
+      }
+
+      const typeMatch = content.match(/BlockConfig\s*=\s*\{[\s\S]*?type:\s*['"]([^'"]+)['"]/)
+      if (!typeMatch) continue
+      const blockType = typeMatch[1]
+
+      const ids = extractSubBlockIds(content)
+      if (ids.length === 0) continue
+
+      map[blockType] = new Set(ids)
+    }
+  } catch (err) {
+    console.log(`⚠ Could not read previous block files from ${baseRef} — skipping check`, err)
+    process.exit(0)
+  }
+
+  return map
+}
+
+const previous = getPreviousIds()
+const current = getCurrentIds()
+const errors: string[] = []
+
+for (const [blockType, prevIds] of Object.entries(previous)) {
+  const currIds = current[blockType]
+  if (!currIds) continue
+
+  const migrations = SUBBLOCK_ID_MIGRATIONS[blockType] ?? {}
+
+  for (const oldId of prevIds) {
+    if (currIds.has(oldId)) continue
+
+    if (oldId in migrations) continue
+
+    errors.push(
+      `Block "${blockType}": subblock ID "${oldId}" was removed.\n` +
+        `  → Add a migration in SUBBLOCK_ID_MIGRATIONS (lib/workflows/migrations/subblock-migrations.ts)\n` +
+        `    mapping "${oldId}" to its replacement ID.`
+    )
+  }
+}
+
+if (errors.length > 0) {
+  console.error('✗ Subblock ID stability check FAILED\n')
+  console.error(
+    'Removing subblock IDs breaks deployed workflows.\n' +
+      'Either revert the rename or add a migration entry.\n'
+  )
+  for (const err of errors) {
+    console.error(`  ${err}\n`)
+  }
+  process.exit(1)
+} else {
+  console.log('✓ Subblock ID stability check passed')
+  process.exit(0)
+}