Skip to content

Commit 9795c42

Browse files
icecrasher321icecrasher321
committed
address comments
1 parent 7175e0d commit 9795c42

7 files changed

Lines changed: 174 additions & 38 deletions

File tree

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

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ describe('migrateSubblockIds', () => {
139139
type: 'function',
140140
subBlocks: {
141141
code: { id: 'code', type: 'unknown', value: 'console.log("hi")' },
142+
language: { value: 'javascript' },
142143
undefined: { type: 'unknown', value: null },
143144
noId: { type: 'short-input', value: 'stale' },
144145
noType: { id: 'noType', value: 'stale' },
@@ -157,23 +158,41 @@ describe('migrateSubblockIds', () => {
157158
type: 'code',
158159
value: 'console.log("hi")',
159160
})
160-
expect(blocks.b1.subBlocks.undefined).toBeUndefined()
161-
expect(blocks.b1.subBlocks.noId).toEqual({ id: 'noId', type: 'short-input', value: 'stale' })
162-
expect(blocks.b1.subBlocks.noType).toEqual({
163-
id: 'noType',
164-
type: 'short-input',
165-
value: 'stale',
161+
expect(blocks.b1.subBlocks.language).toEqual({
162+
id: 'language',
163+
type: 'dropdown',
164+
value: 'javascript',
166165
})
166+
expect(blocks.b1.subBlocks.undefined).toBeUndefined()
167+
expect(blocks.b1.subBlocks.noId).toBeUndefined()
168+
expect(blocks.b1.subBlocks.noType).toBeUndefined()
167169
expect(blocks.b1.subBlocks.unknownType).toBeUndefined()
168-
expect(blocks.b1.subBlocks.notRecord).toEqual({
169-
id: 'notRecord',
170-
type: 'short-input',
171-
value: 'stale',
172-
})
173-
expect(blocks.b1.subBlocks.arrayValue).toEqual({
174-
id: 'arrayValue',
175-
type: 'short-input',
176-
value: ['a', 'b'],
170+
expect(blocks.b1.subBlocks.notRecord).toBeUndefined()
171+
expect(blocks.b1.subBlocks.arrayValue).toBeUndefined()
172+
})
173+
174+
it('should preserve malformed legacy subBlocks before renaming them', () => {
175+
const input: Record<string, BlockState> = {
176+
b1: makeBlock({
177+
type: 'knowledge',
178+
subBlocks: {
179+
knowledgeBaseId: {
180+
id: 'knowledgeBaseId',
181+
type: 'unknown',
182+
value: 'kb-uuid-123',
183+
},
184+
},
185+
}),
186+
}
187+
188+
const { blocks, migrated } = migrateSubblockIds(input)
189+
190+
expect(migrated).toBe(true)
191+
expect(blocks.b1.subBlocks.knowledgeBaseId).toBeUndefined()
192+
expect(blocks.b1.subBlocks.knowledgeBaseSelector).toEqual({
193+
id: 'knowledgeBaseSelector',
194+
type: 'knowledge-base-selector',
195+
value: 'kb-uuid-123',
177196
})
178197
})
179198

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

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { createLogger } from '@sim/logger'
2+
import { DEFAULT_SUBBLOCK_TYPE } from '@sim/workflow-persistence/subblocks'
3+
import { isPlainRecord } from '@/lib/core/utils/records'
24
import { sanitizeMalformedSubBlocks } from '@/lib/workflows/sanitization/subblocks'
35
import {
46
buildCanonicalIndex,
@@ -69,6 +71,7 @@ export const SUBBLOCK_ID_MIGRATIONS: Record<string, Record<string, string>> = {
6971
* Returns a new subBlocks record if anything changed, or the original if not.
7072
*/
7173
function migrateBlockSubblockIds(
74+
blockType: string,
7275
subBlocks: Record<string, BlockState['subBlocks'][string]>,
7376
renames: Record<string, string>
7477
): { subBlocks: Record<string, BlockState['subBlocks'][string]>; migrated: boolean } {
@@ -84,6 +87,7 @@ function migrateBlockSubblockIds(
8487
if (!migrated) return { subBlocks, migrated: false }
8588

8689
const result = { ...subBlocks }
90+
const blockConfig = getBlock(blockType)
8791

8892
for (const [oldId, newId] of Object.entries(renames)) {
8993
if (!(oldId in result)) continue
@@ -94,7 +98,24 @@ function migrateBlockSubblockIds(
9498
}
9599

96100
const oldEntry = result[oldId]
97-
result[newId] = { ...oldEntry, id: newId }
101+
const configuredType = blockConfig?.subBlocks?.find((config) => config.id === newId)?.type
102+
result[newId] = isPlainRecord(oldEntry)
103+
? {
104+
...oldEntry,
105+
id: newId,
106+
type:
107+
configuredType ||
108+
(typeof oldEntry.type === 'string' && oldEntry.type.length > 0
109+
? oldEntry.type === 'unknown'
110+
? DEFAULT_SUBBLOCK_TYPE
111+
: oldEntry.type
112+
: DEFAULT_SUBBLOCK_TYPE),
113+
}
114+
: ({
115+
id: newId,
116+
type: configuredType || DEFAULT_SUBBLOCK_TYPE,
117+
value: oldEntry,
118+
} as BlockState['subBlocks'][string])
98119
delete result[oldId]
99120
}
100121

@@ -118,25 +139,23 @@ export function migrateSubblockIds(blocks: Record<string, BlockState>): {
118139
continue
119140
}
120141

121-
const sanitized = sanitizeMalformedSubBlocks(block)
122142
const renames = SUBBLOCK_ID_MIGRATIONS[block.type]
123-
if (!renames) {
124-
result[blockId] = sanitized.changed ? { ...block, subBlocks: sanitized.subBlocks } : block
125-
anyMigrated = anyMigrated || sanitized.changed
126-
continue
127-
}
143+
const renamed = renames
144+
? migrateBlockSubblockIds(block.type, block.subBlocks, renames)
145+
: { subBlocks: block.subBlocks, migrated: false }
146+
const renamedBlock = renamed.migrated ? { ...block, subBlocks: renamed.subBlocks } : block
147+
const sanitized = sanitizeMalformedSubBlocks(renamedBlock)
148+
const blockMigrated = renamed.migrated || sanitized.changed
128149

129-
const { subBlocks, migrated } = migrateBlockSubblockIds(sanitized.subBlocks, renames)
130-
const blockMigrated = sanitized.changed || migrated
131150
if (blockMigrated) {
132-
if (migrated) {
151+
if (renamed.migrated) {
133152
logger.info('Migrated legacy subblock IDs', {
134153
blockId: block.id,
135154
blockType: block.type,
136155
})
137156
}
138157
anyMigrated = true
139-
result[blockId] = { ...block, subBlocks }
158+
result[blockId] = { ...renamedBlock, subBlocks: sanitized.subBlocks }
140159
} else {
141160
result[blockId] = block
142161
}

‎apps/sim/lib/workflows/operations/import-export.test.ts‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
import { describe, expect, it } from 'vitest'
1+
import { describe, expect, it, vi } from 'vitest'
2+
3+
vi.unmock('@/blocks/registry')
4+
25
import {
36
extractWorkflowName,
47
parseWorkflowJson,
@@ -77,6 +80,59 @@ describe('workflow import/export parsing', () => {
7780

7881
expect(extractWorkflowName(content, 'wf.json')).toBe('Wrapped Workflow')
7982
})
83+
84+
it('parses API envelopes that contain state without an export version', () => {
85+
const content = JSON.stringify({
86+
data: {
87+
workflow: {
88+
name: 'API Workflow',
89+
},
90+
state: createLegacyState(),
91+
},
92+
})
93+
94+
const result = parseWorkflowJson(content, false)
95+
96+
expect(result.errors).toEqual([])
97+
expect(result.data?.blocks['start-1']).toBeDefined()
98+
expect(result.data?.blocks['start-1'].subBlocks.undefined).toBeUndefined()
99+
})
100+
101+
it('preserves malformed legacy renamed subBlocks during import parsing', () => {
102+
const state = {
103+
...createLegacyState(),
104+
blocks: {
105+
knowledge: {
106+
id: 'knowledge',
107+
type: 'knowledge',
108+
name: 'Knowledge',
109+
position: { x: 0, y: 0 },
110+
enabled: true,
111+
subBlocks: {
112+
operation: { id: 'operation', type: 'dropdown', value: 'search' },
113+
knowledgeBaseId: {
114+
id: 'knowledgeBaseId',
115+
type: 'unknown',
116+
value: 'kb-uuid-123',
117+
},
118+
},
119+
outputs: {},
120+
data: {},
121+
},
122+
},
123+
}
124+
const content = JSON.stringify({ data: { workflow: { name: 'Knowledge Workflow' }, state } })
125+
126+
const result = parseWorkflowJson(content, false)
127+
128+
expect(result.errors).toEqual([])
129+
expect(result.data?.blocks.knowledge.subBlocks.knowledgeBaseId).toBeUndefined()
130+
expect(result.data?.blocks.knowledge.subBlocks.knowledgeBaseSelector).toEqual({
131+
id: 'knowledgeBaseSelector',
132+
type: 'knowledge-base-selector',
133+
value: 'kb-uuid-123',
134+
})
135+
})
80136
})
81137

82138
describe('sanitizePathSegment', () => {

‎apps/sim/lib/workflows/operations/import-export.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
type WorkflowStateContractInput,
1010
workflowVariablesContract,
1111
} from '@/lib/api/contracts/workflows'
12+
import { migrateSubblockIds } from '@/lib/workflows/migrations/subblock-migrations'
1213
import {
1314
type ExportWorkflowState,
1415
sanitizeForExport,
@@ -474,9 +475,10 @@ export function extractWorkflowName(content: string, filename: string): string {
474475
* with a stable block field.
475476
*/
476477
function normalizeSubblockValues(blocks: Record<string, any>): Record<string, any> {
478+
const { blocks: migratedBlocks } = migrateSubblockIds(blocks)
477479
const normalizedBlocks: Record<string, any> = {}
478480

479-
Object.entries(blocks).forEach(([blockId, block]) => {
481+
Object.entries(migratedBlocks).forEach(([blockId, block]) => {
480482
const normalizedBlock = { ...block }
481483

482484
if (block.subBlocks) {
@@ -532,8 +534,8 @@ export function parseWorkflowJson(
532534

533535
// Handle new export format (version/exportedAt/state) or old format (blocks/edges at root)
534536
let workflowData: any
535-
if (data.version && data.state) {
536-
// New format with versioning
537+
if (isRecord(data.state)) {
538+
// Export/API envelope format with workflow state nested under `state`
537539
logger.info('Parsing workflow JSON with version', {
538540
version: data.version,
539541
exportedAt: data.exportedAt,

‎apps/sim/lib/workflows/persistence/utils.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,13 @@ const applyBlockMigrations = createMigrationPipeline([
158158
blocks: migrateAgentBlocksToMessagesFormat(ctx.blocks),
159159
}),
160160

161-
async (ctx) => {
162-
const { blocks, migrated } = await migrateCredentialIds(ctx.blocks, ctx.workspaceId)
161+
(ctx) => {
162+
const { blocks, migrated } = migrateSubblockIds(ctx.blocks)
163163
return { ...ctx, blocks, migrated: ctx.migrated || migrated }
164164
},
165165

166-
(ctx) => {
167-
const { blocks, migrated } = migrateSubblockIds(ctx.blocks)
166+
async (ctx) => {
167+
const { blocks, migrated } = await migrateCredentialIds(ctx.blocks, ctx.workspaceId)
168168
return { ...ctx, blocks, migrated: ctx.migrated || migrated }
169169
},
170170

@@ -249,6 +249,7 @@ async function migrateCredentialIds(
249249

250250
for (const block of Object.values(blocks)) {
251251
for (const [subBlockId, subBlock] of Object.entries(block.subBlocks || {})) {
252+
if (!subBlock || typeof subBlock !== 'object') continue
252253
const value = (subBlock as { value?: unknown }).value
253254
if (
254255
CREDENTIAL_SUBBLOCK_IDS.has(subBlockId) &&
@@ -350,7 +351,9 @@ export async function loadWorkflowFromNormalizedTables(
350351
const { blocks: finalBlocks, migrated } = await applyBlockMigrations(raw.blocks, raw.workspaceId)
351352

352353
if (migrated) {
353-
Promise.resolve().then(() => persistMigratedBlocks(workflowId, raw.blocks, finalBlocks))
354+
Promise.resolve().then(() =>
355+
persistMigratedBlocks(workflowId, raw.blocks, finalBlocks, raw.blockUpdatedAt)
356+
)
354357
}
355358

356359
const patchedLoops: Record<string, Loop> = { ...raw.loops }

‎apps/sim/lib/workflows/sanitization/subblocks.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ export function sanitizeMalformedSubBlocks(
3838
const configuredType = blockConfig?.subBlocks?.find((config) => config.id === subBlockId)?.type
3939

4040
if (!isPlainRecord(subBlock)) {
41+
if (!configuredType) {
42+
logger.warn('Skipping malformed subBlock: unrecognized value entry', {
43+
blockId: block.id,
44+
subBlockId,
45+
})
46+
changed = true
47+
continue
48+
}
49+
4150
logger.warn('Repairing malformed subBlock value', { blockId: block.id, subBlockId })
4251
result[subBlockId] = {
4352
id: subBlockId,
@@ -60,6 +69,21 @@ export function sanitizeMalformedSubBlocks(
6069
const id = typeof subBlock.id === 'string' && subBlock.id.length > 0 ? subBlock.id : subBlockId
6170
const typeFromConfig =
6271
configuredType || blockConfig?.subBlocks?.find((config) => config.id === id)?.type
72+
const missingMetadata =
73+
typeof subBlock.id !== 'string' ||
74+
subBlock.id.length === 0 ||
75+
typeof subBlock.type !== 'string' ||
76+
subBlock.type.length === 0
77+
78+
if (missingMetadata && !typeFromConfig) {
79+
logger.warn('Skipping malformed subBlock: unrecognized metadata entry', {
80+
blockId: block.id,
81+
subBlockId,
82+
})
83+
changed = true
84+
continue
85+
}
86+
6387
const type =
6488
typeof subBlock.type === 'string' && subBlock.type.length > 0 && subBlock.type !== 'unknown'
6589
? subBlock.type

‎packages/workflow-persistence/src/load.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ import { db, workflow, workflowBlocks, workflowEdges, workflowSubflows } from '@
22
import { createLogger } from '@sim/logger'
33
import type { BlockState, Loop, Parallel } from '@sim/workflow-types/workflow'
44
import { SUBFLOW_TYPES } from '@sim/workflow-types/workflow'
5-
import { eq } from 'drizzle-orm'
5+
import { and, eq } from 'drizzle-orm'
66
import type { Edge } from 'reactflow'
77
import type { NormalizedWorkflowData } from './types'
88

99
const logger = createLogger('WorkflowPersistenceLoad')
1010

1111
export interface RawNormalizedWorkflow extends NormalizedWorkflowData {
1212
workspaceId: string
13+
blockUpdatedAt: Record<string, Date>
1314
}
1415

1516
/**
@@ -50,6 +51,7 @@ export async function loadWorkflowFromNormalizedTablesRaw(
5051
}
5152

5253
const blocksMap: Record<string, BlockState> = {}
54+
const blockUpdatedAt: Record<string, Date> = {}
5355
blocks.forEach((block) => {
5456
const blockData = (block.data ?? {}) as BlockState['data']
5557

@@ -73,6 +75,7 @@ export async function loadWorkflowFromNormalizedTablesRaw(
7375
}
7476

7577
blocksMap[block.id] = assembled
78+
blockUpdatedAt[block.id] = block.updatedAt
7679
})
7780

7881
const edgesArray: Edge[] = edges.map((edge) => ({
@@ -151,6 +154,7 @@ export async function loadWorkflowFromNormalizedTablesRaw(
151154
parallels,
152155
isFromNormalizedTables: true,
153156
workspaceId: workflowRow.workspaceId,
157+
blockUpdatedAt,
154158
}
155159
} catch (error) {
156160
logger.error(`Error loading workflow ${workflowId} from normalized tables:`, error)
@@ -161,7 +165,8 @@ export async function loadWorkflowFromNormalizedTablesRaw(
161165
export async function persistMigratedBlocks(
162166
workflowId: string,
163167
originalBlocks: Record<string, BlockState>,
164-
migratedBlocks: Record<string, BlockState>
168+
migratedBlocks: Record<string, BlockState>,
169+
originalBlockUpdatedAt: Record<string, Date> = {}
165170
): Promise<void> {
166171
try {
167172
for (const [blockId, block] of Object.entries(migratedBlocks)) {
@@ -173,7 +178,15 @@ export async function persistMigratedBlocks(
173178
data: block.data,
174179
updatedAt: new Date(),
175180
})
176-
.where(eq(workflowBlocks.id, blockId))
181+
.where(
182+
originalBlockUpdatedAt[blockId]
183+
? and(
184+
eq(workflowBlocks.id, blockId),
185+
eq(workflowBlocks.workflowId, workflowId),
186+
eq(workflowBlocks.updatedAt, originalBlockUpdatedAt[blockId])
187+
)
188+
: and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId))
189+
)
177190
}
178191
}
179192
} catch (err) {

0 commit comments

Comments
 (0)