diff --git a/src/stores/workflowStore.ts b/src/stores/workflowStore.ts index 2c57bd3..0bdaef9 100644 --- a/src/stores/workflowStore.ts +++ b/src/stores/workflowStore.ts @@ -105,6 +105,15 @@ interface WorkflowStore { importedTriggers: Record; importedPipelines: Record; + /** + * Passthrough metadata from the most recently imported WorkflowConfig. + * Stores non-visual top-level fields (name, version, _originalKeys, _extraTopLevelKeys, imports, + * requires, platform, infrastructure, sidecars) so that exportToConfig() can reconstruct + * a complete WorkflowConfig without losing unknown keys or reformatting structure. + * Pipelines are excluded here because they are tracked separately via importedPipelines. + */ + importedPassthrough: Omit | null; + // Multi-file resolution: maps module name → source file path sourceMap: Map; setSourceMap: (sourceMap: Map) => void; @@ -218,6 +227,7 @@ const useWorkflowStore = create()( importedWorkflows: {}, importedTriggers: {}, importedPipelines: {}, + importedPassthrough: null, // Multi-file resolution sourceMap: new Map(), @@ -453,9 +463,15 @@ const useWorkflowStore = create()( }, exportToConfig: () => { - const { nodes, edges, importedWorkflows, importedTriggers, importedPipelines } = get(); + const { nodes, edges, importedWorkflows, importedTriggers, importedPipelines, importedPassthrough } = get(); const moduleTypeMap = useModuleSchemaStore.getState().moduleTypeMap; - const config = nodesToConfig(nodes, edges, moduleTypeMap); + // Pass importedPassthrough as originalConfig so that nodesToConfig can restore + // non-visual fields: name, version, _originalKeys, _extraTopLevelKeys, imports, + // requires, platform, infrastructure, sidecars. + const originalConfig: import('../types/workflow.ts').WorkflowConfig | undefined = importedPassthrough + ? { modules: [], workflows: {}, triggers: {}, ...importedPassthrough } + : undefined; + const config = nodesToConfig(nodes, edges, moduleTypeMap, originalConfig); if (Object.keys(config.workflows).length === 0 && Object.keys(importedWorkflows).length > 0) { config.workflows = importedWorkflows; } @@ -484,6 +500,9 @@ const useWorkflowStore = create()( get().pushHistory(); const moduleTypeMap = useModuleSchemaStore.getState().moduleTypeMap; const { nodes, edges } = configToNodes(config, moduleTypeMap, sourceMap); + // Extract passthrough metadata (everything except the visual module/workflow/trigger data) + const { modules: _modules, workflows: _workflows, triggers: _triggers, pipelines: _pipelines, ...passthroughFields } = config; + const importedPassthrough: WorkflowStore['importedPassthrough'] = passthroughFields; const updates: Partial = { nodes, edges, @@ -491,6 +510,7 @@ const useWorkflowStore = create()( importedWorkflows: config.workflows ?? {}, importedTriggers: config.triggers ?? {}, importedPipelines: config.pipelines ?? {}, + importedPassthrough, }; if (sourceMap) { updates.sourceMap = sourceMap; @@ -501,7 +521,7 @@ const useWorkflowStore = create()( clearCanvas: () => { get().pushHistory(); - set({ nodes: [], edges: [], selectedNodeId: null, selectedEdgeId: null, nodeCounter: 0, importedWorkflows: {}, importedTriggers: {}, importedPipelines: {} }); + set({ nodes: [], edges: [], selectedNodeId: null, selectedEdgeId: null, nodeCounter: 0, importedWorkflows: {}, importedTriggers: {}, importedPipelines: {}, importedPassthrough: null }); }, exportLayout: (): LayoutData => { diff --git a/src/types/workflow.ts b/src/types/workflow.ts index 2b14df2..8fe68ea 100644 --- a/src/types/workflow.ts +++ b/src/types/workflow.ts @@ -23,6 +23,8 @@ export interface WorkflowConfig { infrastructure?: Record; sidecars?: unknown[]; _originalKeys?: string[]; + /** Preserves unknown top-level keys that are not part of the known schema (e.g. engine:, custom config blocks). */ + _extraTopLevelKeys?: Record; } // Workflow section types for edge extraction diff --git a/src/utils/serialization-bugs.test.ts b/src/utils/serialization-bugs.test.ts index 92bd0fe..5644e48 100644 --- a/src/utils/serialization-bugs.test.ts +++ b/src/utils/serialization-bugs.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { configToNodes, nodesToConfig, configToYaml, parseYamlSafe } from './serialization.ts'; +import { configToNodes, nodesToConfig, configToYaml, parseYamlSafe, parseYaml } from './serialization.ts'; import { MODULE_TYPE_MAP } from '../types/workflow.ts'; describe('Bug 1: name and version round-trip', () => { @@ -136,3 +136,161 @@ triggers: {} expect(isPartial).toBe(false); }); }); + +describe('Bug 4: unknown top-level keys (e.g. engine:) must not be dropped', () => { + it('parseYaml captures unknown top-level keys in _extraTopLevelKeys', () => { + const yamlText = ` +name: my-service +engine: + validation: + templateRefs: warn +modules: [] +workflows: {} +`; + const config = parseYaml(yamlText); + expect(config._extraTopLevelKeys).toBeDefined(); + expect(config._extraTopLevelKeys!['engine']).toEqual({ validation: { templateRefs: 'warn' } }); + }); + + it('parseYamlSafe captures unknown top-level keys in _extraTopLevelKeys', () => { + const yamlText = ` +name: my-service +engine: + validation: + templateRefs: warn +modules: [] +`; + const { config } = parseYamlSafe(yamlText); + expect(config._extraTopLevelKeys).toBeDefined(); + expect(config._extraTopLevelKeys!['engine']).toEqual({ validation: { templateRefs: 'warn' } }); + }); + + it('configToYaml emits unknown top-level keys', () => { + const yamlText = ` +name: my-service +engine: + validation: + templateRefs: warn +modules: [] +workflows: {} +`; + const config = parseYaml(yamlText); + const out = configToYaml(config); + expect(out).toContain('engine:'); + expect(out).toContain('templateRefs: warn'); + }); + + it('nodesToConfig passes through _extraTopLevelKeys from originalConfig', () => { + const yamlText = ` +name: my-service +engine: + validation: + templateRefs: warn +modules: + - name: server + type: http.server + config: + address: ":8080" +workflows: {} +`; + const config = parseYaml(yamlText); + const { nodes, edges } = configToNodes(config, MODULE_TYPE_MAP); + const exported = nodesToConfig(nodes, edges, MODULE_TYPE_MAP, config); + expect(exported._extraTopLevelKeys).toBeDefined(); + expect(exported._extraTopLevelKeys!['engine']).toEqual({ validation: { templateRefs: 'warn' } }); + const out = configToYaml(exported); + expect(out).toContain('engine:'); + }); + + it('full round-trip preserves engine block and original key ordering', () => { + const yamlText = `name: my-service +version: "1.0" +engine: + validation: + templateRefs: warn +modules: + - name: server + type: http.server + config: + address: ':8080' +pipelines: + health: + trigger: + type: http + method: GET + path: /healthz +`; + const config = parseYaml(yamlText); + const { nodes, edges } = configToNodes(config, MODULE_TYPE_MAP); + const exported = nodesToConfig(nodes, edges, MODULE_TYPE_MAP, config); + const out = configToYaml(exported); + + // engine block must be present + expect(out).toContain('engine:'); + expect(out).toContain('templateRefs: warn'); + + // Key ordering: name comes before engine comes before modules + const nameIdx = out.indexOf('name:'); + const engineIdx = out.indexOf('engine:'); + const modulesIdx = out.indexOf('modules:'); + expect(nameIdx).toBeGreaterThanOrEqual(0); + expect(engineIdx).toBeGreaterThan(nameIdx); + expect(modulesIdx).toBeGreaterThan(engineIdx); + }); +}); + +describe('Bug 5: triggers: {} must not be injected when not in original', () => { + it('does not add triggers: {} when original YAML had no triggers key', () => { + const yamlText = ` +name: my-service +modules: + - name: server + type: http.server + config: + address: ":8080" +pipelines: + health: + trigger: + type: http + method: GET + path: /healthz +`; + const config = parseYaml(yamlText); + // triggers not in original keys + expect(config._originalKeys).not.toContain('triggers'); + + const { nodes, edges } = configToNodes(config, MODULE_TYPE_MAP); + const exported = nodesToConfig(nodes, edges, MODULE_TYPE_MAP, config); + const out = configToYaml(exported); + + // triggers: {} must NOT appear in output + expect(out).not.toMatch(/^triggers:/m); + }); +}); + +describe('Bug 6: parseYamlSafe is consistent with parseYaml for all fields', () => { + it('parseYamlSafe preserves imports, requires, platform, infrastructure, sidecars', () => { + const yamlText = ` +name: my-service +imports: + - other.yaml +requires: + some-service: ">=1.0" +platform: + target: kubernetes +infrastructure: + database: + type: postgres +sidecars: + - name: proxy + image: envoy:latest +modules: [] +`; + const { config } = parseYamlSafe(yamlText); + expect(config.imports).toEqual(['other.yaml']); + expect(config.requires).toEqual({ 'some-service': '>=1.0' }); + expect(config.platform).toEqual({ target: 'kubernetes' }); + expect(config.infrastructure).toEqual({ database: { type: 'postgres' } }); + expect(config.sidecars).toHaveLength(1); + }); +}); diff --git a/src/utils/serialization.ts b/src/utils/serialization.ts index e0ed137..f209c4d 100644 --- a/src/utils/serialization.ts +++ b/src/utils/serialization.ts @@ -540,6 +540,10 @@ export function nodesToConfig( if (originalKeys) { result._originalKeys = originalKeys; } + const extraTopLevelKeys = originalConfig?._extraTopLevelKeys; + if (extraTopLevelKeys && Object.keys(extraTopLevelKeys).length > 0) { + result._extraTopLevelKeys = extraTopLevelKeys; + } return result; } @@ -805,31 +809,87 @@ export function nodeComponentType(moduleType: string): string { return 'infrastructureNode'; } +/** Known top-level keys in WorkflowConfig — anything else is preserved as an extra key. */ +const KNOWN_TOP_LEVEL_KEYS = new Set([ + 'name', 'version', 'modules', 'workflows', 'triggers', + 'pipelines', 'imports', 'requires', 'platform', 'infrastructure', 'sidecars', + // application: is the legacy multi-file format key — handled by resolveImports, not a passthrough extra + 'application', +]); + +/** Extract unknown top-level keys from a parsed YAML object into _extraTopLevelKeys. */ +function extractExtraTopLevelKeys(parsed: Record): Record | undefined { + const extra: Record = {}; + for (const key of Object.keys(parsed)) { + if (!KNOWN_TOP_LEVEL_KEYS.has(key)) { + extra[key] = parsed[key]; + } + } + return Object.keys(extra).length > 0 ? extra : undefined; +} + export function configToYaml(config: WorkflowConfig): string { // Strip internal tracking fields and omit empty top-level arrays/objects // that were not present in the original YAML const originalKeys = config._originalKeys; - const out: Record = {}; - - // Field order: name, version, imports, requires, modules, workflows, triggers, pipelines, platform, infrastructure, sidecars - if (config.name !== undefined) out.name = config.name; - if (config.version !== undefined) out.version = config.version; - if (config.imports && config.imports.length > 0) out.imports = config.imports; - if (config.requires && Object.keys(config.requires).length > 0) out.requires = config.requires; + // Determine which known fields to include const includeModules = !originalKeys || originalKeys.includes('modules') || (config.modules?.length ?? 0) > 0; - if (includeModules && config.modules !== undefined) out.modules = config.modules; - const includeWorkflows = !originalKeys || originalKeys.includes('workflows') || Object.keys(config.workflows ?? {}).length > 0; - if (includeWorkflows && config.workflows !== undefined) out.workflows = config.workflows; - const includeTriggers = !originalKeys || originalKeys.includes('triggers') || Object.keys(config.triggers ?? {}).length > 0; - if (includeTriggers && config.triggers !== undefined) out.triggers = config.triggers; - if (config.pipelines && Object.keys(config.pipelines).length > 0) out.pipelines = config.pipelines; - if (config.platform && Object.keys(config.platform).length > 0) out.platform = config.platform; - if (config.infrastructure && Object.keys(config.infrastructure).length > 0) out.infrastructure = config.infrastructure; - if (config.sidecars && config.sidecars.length > 0) out.sidecars = config.sidecars; + // Build a map of all key→value pairs that should be emitted + const valueMap: Record = {}; + if (config.name !== undefined) valueMap.name = config.name; + if (config.version !== undefined) valueMap.version = config.version; + if (config.imports && config.imports.length > 0) valueMap.imports = config.imports; + if (config.requires && Object.keys(config.requires).length > 0) valueMap.requires = config.requires; + if (includeModules && config.modules !== undefined) valueMap.modules = config.modules; + if (includeWorkflows && config.workflows !== undefined) valueMap.workflows = config.workflows; + if (includeTriggers && config.triggers !== undefined) valueMap.triggers = config.triggers; + if (config.pipelines && Object.keys(config.pipelines).length > 0) valueMap.pipelines = config.pipelines; + if (config.platform && Object.keys(config.platform).length > 0) valueMap.platform = config.platform; + if (config.infrastructure && Object.keys(config.infrastructure).length > 0) valueMap.infrastructure = config.infrastructure; + if (config.sidecars && config.sidecars.length > 0) valueMap.sidecars = config.sidecars; + // Include any extra/unknown top-level keys (e.g. engine:, custom config blocks) + if (config._extraTopLevelKeys) { + for (const [k, v] of Object.entries(config._extraTopLevelKeys)) { + valueMap[k] = v; + } + } + + // Build the output object — if we have the original key order, honour it so that + // the serialised YAML preserves the author's top-level key ordering. + const out: Record = {}; + if (originalKeys && originalKeys.length > 0) { + // First: emit keys in their original order + for (const key of originalKeys) { + if (key in valueMap) { + out[key] = valueMap[key]; + } + } + // Then: emit any new keys that weren't in the original (e.g. added by the editor) + for (const [key, value] of Object.entries(valueMap)) { + if (!originalKeys.includes(key)) { + out[key] = value; + } + } + } else { + // No original key order — use the fixed default order: + // name, version, imports, requires, modules, workflows, triggers, pipelines, platform, infrastructure, sidecars, [extra] + const DEFAULT_ORDER = ['name', 'version', 'imports', 'requires', 'modules', 'workflows', 'triggers', 'pipelines', 'platform', 'infrastructure', 'sidecars']; + for (const key of DEFAULT_ORDER) { + if (key in valueMap) { + out[key] = valueMap[key]; + } + } + // Append any extra top-level keys after the known fields + for (const [key, value] of Object.entries(valueMap)) { + if (!DEFAULT_ORDER.includes(key)) { + out[key] = value; + } + } + } return yaml.dump(out, { lineWidth: -1, noRefs: true, sortKeys: false }); } @@ -871,6 +931,10 @@ export function parseYaml(text: string): WorkflowConfig { if (parsed.sidecars) { config.sidecars = parsed.sidecars as unknown[]; } + const extraTopLevelKeys = extractExtraTopLevelKeys(parsed); + if (extraTopLevelKeys) { + config._extraTopLevelKeys = extraTopLevelKeys; + } return config; } catch { return { modules: [], workflows: {}, triggers: {}, _originalKeys: [] }; @@ -899,6 +963,25 @@ export function parseYamlSafe(text: string): { config: WorkflowConfig; error?: s if (parsed.pipelines) { config.pipelines = parsed.pipelines as Record; } + if (parsed.imports) { + config.imports = parsed.imports as string[]; + } + if (parsed.requires) { + config.requires = parsed.requires as Record; + } + if (parsed.platform) { + config.platform = parsed.platform as Record; + } + if (parsed.infrastructure) { + config.infrastructure = parsed.infrastructure as Record; + } + if (parsed.sidecars) { + config.sidecars = parsed.sidecars as unknown[]; + } + const extraTopLevelKeys = extractExtraTopLevelKeys(parsed); + if (extraTopLevelKeys) { + config._extraTopLevelKeys = extraTopLevelKeys; + } return { config }; } catch (e) { return { config: { modules: [], workflows: {}, triggers: {}, _originalKeys: [] }, error: (e as Error).message }; @@ -1187,6 +1270,12 @@ export async function resolveImports( const configVersion = (parsed.version ?? application?.version) as string | undefined; if (configName) config.name = configName; if (configVersion) config.version = configVersion; + // Preserve top-level key ordering and unknown keys from the main file + config._originalKeys = Object.keys(parsed); + const extraTopLevelKeys = extractExtraTopLevelKeys(parsed); + if (extraTopLevelKeys) { + config._extraTopLevelKeys = extraTopLevelKeys; + } return { config, @@ -1274,6 +1363,8 @@ export function exportMainFileYaml( /** * Internal helper: build the main-file YAML string and collect imported file paths. + * Delegates to configToYaml so that _extraTopLevelKeys and _originalKeys ordering + * are honoured for the main-file content (same as single-file round-trips). */ function buildMainFileContent( config: WorkflowConfig, @@ -1281,20 +1372,7 @@ function buildMainFileContent( filePipelines: Map>, ): { yaml: string; importedFiles: string[] } { const mainModules = fileModules.get(null) ?? []; - const mainConfig: Record = {}; - if (config.name !== undefined) mainConfig.name = config.name; - if (config.version !== undefined) mainConfig.version = config.version; - mainConfig.modules = mainModules; - if (Object.keys(config.workflows).length > 0) { - mainConfig.workflows = config.workflows; - } - if (Object.keys(config.triggers).length > 0) { - mainConfig.triggers = config.triggers; - } const mainPipelines = filePipelines.get(null); - if (mainPipelines && Object.keys(mainPipelines).length > 0) { - mainConfig.pipelines = mainPipelines; - } // Collect all non-null file paths across modules and pipelines const importedFiles = [ @@ -1303,11 +1381,34 @@ function buildMainFileContent( ...[...filePipelines.keys()].filter((k) => k !== null), ]), ] as string[]; - if (importedFiles.length > 0) { - mainConfig.imports = importedFiles; - } - return { yaml: yaml.dump(mainConfig, { lineWidth: -1, noRefs: true, sortKeys: false }), importedFiles }; + // For multi-file main files, ensure 'modules' is in _originalKeys so configToYaml + // always includes the modules section (even if empty, since sub-files provide modules + // via imports:). Without this, configs whose original main file had no top-level + // 'modules:' key (e.g. application: format) would omit modules from the export. + const sourceKeys = config._originalKeys; + const adjustedOriginalKeys = + sourceKeys && !sourceKeys.includes('modules') + ? [...sourceKeys, 'modules'] + : sourceKeys; + + // Build a temporary config that represents only the main-file content. + // By reusing the full config (with its _originalKeys, _extraTopLevelKeys, workflows, + // triggers, name, version, etc.) but substituting the main-file-only modules, pipelines, + // and the dynamically computed imports list, we can delegate to configToYaml and get + // correct key ordering and unknown-key preservation for free. + const mainOnlyConfig: WorkflowConfig = { + ...config, + modules: mainModules, + // Override imports with the computed list of imported file paths (omit the property + // entirely when there are no sub-files so configToYaml does not emit `imports: []`) + ...(importedFiles.length > 0 ? { imports: importedFiles } : { imports: undefined }), + // Only include main-file pipelines + pipelines: mainPipelines && Object.keys(mainPipelines).length > 0 ? mainPipelines : undefined, + _originalKeys: adjustedOriginalKeys, + }; + + return { yaml: configToYaml(mainOnlyConfig), importedFiles }; } /**