diff --git a/apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts b/apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts index 7bbf9a5ea01..967e432ef39 100644 --- a/apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts +++ b/apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts @@ -41,8 +41,10 @@ import { extractWorkflowsFromZip, parseWorkflowJson, } from '@/lib/workflows/operations/import-export' +import { prepareWorkflowStateForPersistence } from '@/lib/workflows/persistence/prepare-state' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' import { deduplicateWorkflowName } from '@/lib/workflows/utils' +import { normalizeImportedVariables } from '@/lib/workflows/variables/parse' import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils' import { withAdminAuthParams } from '@/app/api/v1/admin/middleware' import { @@ -52,7 +54,6 @@ import { } from '@/app/api/v1/admin/responses' import type { ImportResult, - WorkflowVariable, WorkspaceImportRequest, WorkspaceImportResponse, } from '@/app/api/v1/admin/types' @@ -270,7 +271,22 @@ async function importSingleWorkflow( variables: {}, }) - const saveResult = await saveWorkflowToNormalizedTables(workflowId, workflowData) + /** + * Same normalization the editor, the v1 import API and the single-workflow + * admin import run, via the one shared implementation. Without it this route + * wrote raw parsed state, so a dangling edge tripped the `workflow_edges` + * foreign key and failed the restore, and blocks missing their backfilled + * columns could land unopenable. + */ + const { state: preparedState, warnings } = prepareWorkflowStateForPersistence(workflowData) + if (warnings.length > 0) { + logger.warn(`Admin API: normalized "${dedupedName}" with warnings`, { warnings }) + } + + const saveResult = await saveWorkflowToNormalizedTables(workflowId, { + ...workflowData, + ...preparedState, + }) if (!saveResult.success) { await db.delete(workflow).where(eq(workflow.id, workflowId)) @@ -282,18 +298,13 @@ async function importSingleWorkflow( } } - if (workflowData.variables && Array.isArray(workflowData.variables)) { - const variablesRecord: Record = {} - workflowData.variables.forEach((v) => { - const varId = v.id || generateId() - variablesRecord[varId] = { - id: varId, - name: v.name, - type: v.type || 'string', - value: v.value, - } - }) - + /** + * Previously guarded on `Array.isArray`, which silently dropped every + * variable in the current record form — the exact shape this workspace's + * own export emits — so an export/import round trip lost all of them. + */ + const variablesRecord = normalizeImportedVariables(workflowData.variables) + if (Object.keys(variablesRecord).length > 0) { await db .update(workflow) .set({ variables: variablesRecord, updatedAt: new Date() }) diff --git a/apps/sim/lib/workflows/operations/import-export.ts b/apps/sim/lib/workflows/operations/import-export.ts index 06967ab7969..13b658cca63 100644 --- a/apps/sim/lib/workflows/operations/import-export.ts +++ b/apps/sim/lib/workflows/operations/import-export.ts @@ -1,6 +1,5 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import { generateId } from '@sim/utils/id' import { isRecordLike } from '@sim/utils/object' import { ApiClientError } from '@/lib/api/client/errors' import { requestJson } from '@/lib/api/client/request' @@ -18,6 +17,7 @@ import { sanitizeForExport, } from '@/lib/workflows/sanitization/json-sanitizer' import { sanitizeMalformedSubBlocks } from '@/lib/workflows/sanitization/subblocks' +import { normalizeImportedVariables } from '@/lib/workflows/variables/parse' import { regenerateWorkflowIds } from '@/stores/workflows/utils' import type { Variable, WorkflowState } from '@/stores/workflows/workflow/types' @@ -724,37 +724,15 @@ export async function persistImportedWorkflow({ throw new Error(`Failed to save workflow state for ${newWorkflowId}`) } - if (workflowData.variables) { - const variablesArray = Array.isArray(workflowData.variables) - ? workflowData.variables - : Object.values(workflowData.variables) - - if (variablesArray.length > 0) { - type WorkflowVariablesBodyInput = NonNullable< - Parameters>[1]['body'] - > - const variablesRecord: WorkflowVariablesBodyInput['variables'] = {} - - for (const variable of variablesArray) { - const id = - typeof variable.id === 'string' && variable.id.trim() ? variable.id : generateId() - - variablesRecord[id] = { - id, - name: variable.name, - type: variable.type, - value: variable.value, - } - } - - try { - await requestJson(workflowVariablesContract, { - params: { id: newWorkflowId }, - body: { variables: variablesRecord }, - }) - } catch { - throw new Error(`Failed to save variables for ${newWorkflowId}`) - } + const variablesRecord = normalizeImportedVariables(workflowData.variables) + if (Object.keys(variablesRecord).length > 0) { + try { + await requestJson(workflowVariablesContract, { + params: { id: newWorkflowId }, + body: { variables: variablesRecord }, + }) + } catch { + throw new Error(`Failed to save variables for ${newWorkflowId}`) } } diff --git a/apps/sim/lib/workflows/variables/parse.test.ts b/apps/sim/lib/workflows/variables/parse.test.ts new file mode 100644 index 00000000000..1484fa8604d --- /dev/null +++ b/apps/sim/lib/workflows/variables/parse.test.ts @@ -0,0 +1,119 @@ +/** + * @vitest-environment node + * + * Pins the contract between the two halves of every workflow export/import + * round trip: `parseWorkflowVariables` produces what exports emit, and + * `normalizeImportedVariables` consumes what imports receive. A shape the first + * emits that the second drops is silent data loss on a restore, which is + * exactly the bug the workspace importer shipped with. + */ + +import { describe, expect, it } from 'vitest' +import { normalizeImportedVariables, parseWorkflowVariables } from '@/lib/workflows/variables/parse' + +const VARIABLE = { + id: 'var-1', + name: 'apiHost', + type: 'string' as const, + value: 'https://example.com', +} + +describe('parseWorkflowVariables', () => { + it('returns undefined for null so callers can omit the field', () => { + expect(parseWorkflowVariables(null)).toBeUndefined() + }) + + it('reads the current record form', () => { + expect(parseWorkflowVariables({ 'var-1': VARIABLE })).toEqual({ 'var-1': VARIABLE }) + }) + + it('re-keys the legacy array form by variable id', () => { + expect(parseWorkflowVariables([VARIABLE] as never)).toEqual({ 'var-1': VARIABLE }) + }) + + it('parses a JSON string column value', () => { + expect(parseWorkflowVariables(JSON.stringify({ 'var-1': VARIABLE }) as never)).toEqual({ + 'var-1': VARIABLE, + }) + }) + + it('skips legacy array rows without a usable id instead of emitting an "undefined" key', () => { + const result = parseWorkflowVariables([VARIABLE, { name: 'orphan' }, null] as never) + + expect(Object.keys(result ?? {})).toEqual(['var-1']) + }) +}) + +describe('normalizeImportedVariables', () => { + it('accepts the record form', () => { + expect(normalizeImportedVariables({ 'var-1': VARIABLE })).toEqual({ 'var-1': VARIABLE }) + }) + + it('accepts the legacy array form', () => { + expect(normalizeImportedVariables([VARIABLE])).toEqual({ 'var-1': VARIABLE }) + }) + + it('returns an empty record for null, undefined and non-objects', () => { + expect(normalizeImportedVariables(null)).toEqual({}) + expect(normalizeImportedVariables(undefined)).toEqual({}) + expect(normalizeImportedVariables('nope')).toEqual({}) + }) + + it('falls back to the map key when an entry carries no id', () => { + expect(normalizeImportedVariables({ fromKey: { name: 'x', value: 1 } })).toMatchObject({ + fromKey: { id: 'fromKey', name: 'x' }, + }) + }) + + it('coerces an unrecognized type to string rather than persisting it', () => { + const result = normalizeImportedVariables({ v: { ...VARIABLE, type: 'secret' } }) + + expect(result['var-1'].type).toBe('string') + }) + + it('keeps a __proto__-keyed variable as an ordinary own property', () => { + /** + * Built via `JSON.parse` rather than a literal: an object literal's + * `__proto__` sets the prototype, while `JSON.parse` creates a real own + * key — and `JSON.parse` is how an import payload actually arrives. + */ + const payload = JSON.parse('{"__proto__": {"name": "sneaky", "value": 1}}') + + const result = normalizeImportedVariables(payload) + + expect(Object.keys(result)).toContain('__proto__') + expect(Object.getPrototypeOf(result)).toBe(Object.prototype) + }) + + it('trims a padded id so the key is referenceable', () => { + expect(Object.keys(normalizeImportedVariables([{ ...VARIABLE, id: ' var-1 ' }]))).toEqual([ + 'var-1', + ]) + }) + + it('skips non-object entries instead of writing junk variables', () => { + expect(normalizeImportedVariables(['nope', null, VARIABLE])).toEqual({ 'var-1': VARIABLE }) + }) +}) + +describe('export -> import variable round trip', () => { + /** + * The regression guard. Workspace export writes whatever + * `parseWorkflowVariables` returns — the record form — and the importer used + * to guard on `Array.isArray`, so every variable was silently dropped on + * restore. + */ + it('survives the record form that exports actually emit', () => { + const exported = parseWorkflowVariables({ 'var-1': VARIABLE }) + + expect(exported).toBeDefined() + expect(Array.isArray(exported)).toBe(false) + expect(normalizeImportedVariables(exported)).toEqual({ 'var-1': VARIABLE }) + }) + + it('survives a legacy array-form export', () => { + const exported = parseWorkflowVariables([VARIABLE] as never) + + expect(normalizeImportedVariables(exported)).toEqual({ 'var-1': VARIABLE }) + }) +})