From de216897b98415900672b7a7d8fcc1b3634ae8c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 13:21:06 +0000 Subject: [PATCH 1/5] Initial plan From 8a02da290a09ff926090dc4d18d5fae46f1c776a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 13:40:34 +0000 Subject: [PATCH 2/5] Fix ApplicationConfig format: preserve structure on round-trip and show warning when sub-files can't be resolved Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-editor/sessions/cde0510e-a5f2-413a-8ccb-0d0493cb622c Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- src/components/WorkflowEditor.tsx | 8 + src/stores/workflowStore.ts | 20 +- src/types/workflow.ts | 14 + src/utils/index.ts | 1 + .../serialization-applicationconfig.test.ts | 360 ++++++++++++++++++ src/utils/serialization-multifile.test.ts | 12 +- src/utils/serialization.ts | 118 ++++++ 7 files changed, 525 insertions(+), 8 deletions(-) create mode 100644 src/utils/serialization-applicationconfig.test.ts diff --git a/src/components/WorkflowEditor.tsx b/src/components/WorkflowEditor.tsx index e3b8368..76e72fe 100644 --- a/src/components/WorkflowEditor.tsx +++ b/src/components/WorkflowEditor.tsx @@ -67,6 +67,14 @@ export function WorkflowEditor(props: WorkflowEditorProps) { 'info', ); } + // ApplicationConfig format detected but no file resolver provided — warn the user + // rather than silently converting the format. + if (config._applicationConfig && !sourceMapProp) { + addToast( + 'ApplicationConfig format detected. Configure a workspace file resolver to render the full application graph from referenced sub-files.', + 'warning', + ); + } } const mapFromProp = sourceMapProp ? new Map(Object.entries(sourceMapProp)) : undefined; importFromConfig(config, mapFromProp); diff --git a/src/stores/workflowStore.ts b/src/stores/workflowStore.ts index 2c57bd3..305dd92 100644 --- a/src/stores/workflowStore.ts +++ b/src/stores/workflowStore.ts @@ -10,7 +10,7 @@ import { applyEdgeChanges, addEdge as rfAddEdge, } from '@xyflow/react'; -import type { WorkflowConfig, WorkflowTab, CrossWorkflowLink } from '../types/workflow.ts'; +import type { WorkflowConfig, WorkflowTab, CrossWorkflowLink, ApplicationConfigMeta } from '../types/workflow.ts'; import { MODULE_TYPE_MAP } from '../types/workflow.ts'; import useModuleSchemaStore from './moduleSchemaStore.ts'; import { nodesToConfig, configToNodes, nodeComponentType, exportToFiles, exportMainFileYaml } from '../utils/serialization.ts'; @@ -105,6 +105,10 @@ interface WorkflowStore { importedTriggers: Record; importedPipelines: Record; + // ApplicationConfig format metadata — preserved so the main file can be + // round-tripped back to the original `application:` structure on export. + applicationConfig: ApplicationConfigMeta | null; + // Multi-file resolution: maps module name → source file path sourceMap: Map; setSourceMap: (sourceMap: Map) => void; @@ -219,6 +223,9 @@ const useWorkflowStore = create()( importedTriggers: {}, importedPipelines: {}, + // ApplicationConfig format metadata (null = flat WorkflowConfig format) + applicationConfig: null, + // Multi-file resolution sourceMap: new Map(), setSourceMap: (sourceMap) => set({ sourceMap }), @@ -453,7 +460,7 @@ const useWorkflowStore = create()( }, exportToConfig: () => { - const { nodes, edges, importedWorkflows, importedTriggers, importedPipelines } = get(); + const { nodes, edges, importedWorkflows, importedTriggers, importedPipelines, applicationConfig } = get(); const moduleTypeMap = useModuleSchemaStore.getState().moduleTypeMap; const config = nodesToConfig(nodes, edges, moduleTypeMap); if (Object.keys(config.workflows).length === 0 && Object.keys(importedWorkflows).length > 0) { @@ -465,6 +472,11 @@ const useWorkflowStore = create()( if (Object.keys(importedPipelines).length > 0) { config.pipelines = importedPipelines; } + // Reattach ApplicationConfig metadata so configToYaml / exportMainFileYaml + // can reconstruct the original `application:` format on export. + if (applicationConfig) { + config._applicationConfig = applicationConfig; + } return config; }, @@ -491,6 +503,8 @@ const useWorkflowStore = create()( importedWorkflows: config.workflows ?? {}, importedTriggers: config.triggers ?? {}, importedPipelines: config.pipelines ?? {}, + // Preserve ApplicationConfig metadata (null clears it for flat WorkflowConfig files) + applicationConfig: config._applicationConfig ?? null, }; if (sourceMap) { updates.sourceMap = sourceMap; @@ -501,7 +515,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: {}, applicationConfig: null }); }, exportLayout: (): LayoutData => { diff --git a/src/types/workflow.ts b/src/types/workflow.ts index 2b14df2..e76e106 100644 --- a/src/types/workflow.ts +++ b/src/types/workflow.ts @@ -10,6 +10,18 @@ export interface ModuleConfig { ui_position?: { x: number; y: number }; } +/** + * Metadata preserved when a YAML file uses the ApplicationConfig format + * (`application:` top-level key with `workflows[].file` references). + * Stored alongside the merged WorkflowConfig so the original structure can be + * reconstructed on export without converting to the flat WorkflowConfig format. + */ +export interface ApplicationConfigMeta { + name?: string; + version?: string; + workflows: Array<{ file: string }>; +} + export interface WorkflowConfig { name?: string; version?: string; @@ -23,6 +35,8 @@ export interface WorkflowConfig { infrastructure?: Record; sidecars?: unknown[]; _originalKeys?: string[]; + /** Present when the source file used the ApplicationConfig format. */ + _applicationConfig?: ApplicationConfigMeta; } // Workflow section types for edge extraction diff --git a/src/utils/index.ts b/src/utils/index.ts index b1ec8bd..4805def 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -12,6 +12,7 @@ export { exportToFiles, exportMainFileYaml, hasFileReferences, + buildApplicationConfigYaml, } from './serialization'; export { layoutNodes } from './autoLayout'; export { diff --git a/src/utils/serialization-applicationconfig.test.ts b/src/utils/serialization-applicationconfig.test.ts new file mode 100644 index 0000000..99c36c7 --- /dev/null +++ b/src/utils/serialization-applicationconfig.test.ts @@ -0,0 +1,360 @@ +/** + * Tests for ApplicationConfig format recognition and round-trip preservation. + * + * The ApplicationConfig format uses a top-level `application:` key with + * `workflows[].file` references to sub-files, e.g.: + * + * application: + * name: my-service + * workflows: + * - file: base.yaml + * - file: users.yaml + * + * These tests verify that: + * 1. parseYaml / parseYamlSafe detect and preserve the ApplicationConfig metadata + * 2. configToYaml round-trips the ApplicationConfig format unchanged + * 3. resolveImports sets _applicationConfig on the returned merged config + * 4. buildMainFileContent / exportMainFileYaml emits ApplicationConfig format + * (not flat WorkflowConfig with imports:) when the original was ApplicationConfig + * 5. exportToFiles preserves ApplicationConfig for the main file + */ + +import { describe, it, expect } from 'vitest'; +import { + parseYaml, + parseYamlSafe, + configToYaml, + resolveImports, + exportToFiles, + exportMainFileYaml, + buildApplicationConfigYaml, +} from './serialization.ts'; + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +const APPLICATION_CONFIG_YAML = `\ +application: + name: my-service + version: 1.0.0 + workflows: + - file: base.yaml + - file: users.yaml + - file: billing.yaml +`; + +const APPLICATION_CONFIG_NO_VERSION = `\ +application: + name: my-service + workflows: + - file: base.yaml +`; + +const BASE_YAML = `\ +modules: + - name: cache + type: nosql.redis + config: + host: localhost +`; + +const USERS_YAML = `\ +modules: + - name: user-db + type: database.postgres + config: + dsn: postgres://localhost/users + - name: user-handler + type: api.handler + config: {} +workflows: + http: + server: http-server + router: router + routes: + - method: GET + path: /users + handler: user-handler +`; + +const BILLING_YAML = `\ +modules: + - name: billing-db + type: database.postgres + config: + dsn: postgres://localhost/billing +`; + +// --------------------------------------------------------------------------- +// parseYaml — ApplicationConfig detection +// --------------------------------------------------------------------------- + +describe('parseYaml — ApplicationConfig format detection', () => { + it('detects application: key and sets _applicationConfig', () => { + const config = parseYaml(APPLICATION_CONFIG_YAML); + expect(config._applicationConfig).toBeDefined(); + expect(config._applicationConfig!.name).toBe('my-service'); + expect(config._applicationConfig!.version).toBe('1.0.0'); + expect(config._applicationConfig!.workflows).toEqual([ + { file: 'base.yaml' }, + { file: 'users.yaml' }, + { file: 'billing.yaml' }, + ]); + }); + + it('extracts name and version into top-level config fields', () => { + const config = parseYaml(APPLICATION_CONFIG_YAML); + expect(config.name).toBe('my-service'); + expect(config.version).toBe('1.0.0'); + }); + + it('handles ApplicationConfig without version', () => { + const config = parseYaml(APPLICATION_CONFIG_NO_VERSION); + expect(config._applicationConfig).toBeDefined(); + expect(config._applicationConfig!.name).toBe('my-service'); + expect(config._applicationConfig!.version).toBeUndefined(); + }); + + it('sets imports from file references so hasFileReferences returns true', () => { + const config = parseYaml(APPLICATION_CONFIG_YAML); + expect(config.imports).toEqual(['base.yaml', 'users.yaml', 'billing.yaml']); + }); + + it('produces empty modules/workflows/triggers — content comes from sub-files', () => { + const config = parseYaml(APPLICATION_CONFIG_YAML); + expect(config.modules).toHaveLength(0); + expect(config.workflows).toEqual({}); + expect(config.triggers).toEqual({}); + }); + + it('does NOT detect flat WorkflowConfig as ApplicationConfig', () => { + const flat = `modules:\n - name: foo\n type: http.server\nworkflows: {}\ntriggers: {}\n`; + const config = parseYaml(flat); + expect(config._applicationConfig).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// parseYamlSafe — ApplicationConfig detection +// --------------------------------------------------------------------------- + +describe('parseYamlSafe — ApplicationConfig format detection', () => { + it('detects application: key and sets _applicationConfig', () => { + const { config, error } = parseYamlSafe(APPLICATION_CONFIG_YAML); + expect(error).toBeUndefined(); + expect(config._applicationConfig).toBeDefined(); + expect(config._applicationConfig!.name).toBe('my-service'); + expect(config._applicationConfig!.workflows).toHaveLength(3); + }); + + it('extracts name/version into top-level config fields', () => { + const { config } = parseYamlSafe(APPLICATION_CONFIG_YAML); + expect(config.name).toBe('my-service'); + expect(config.version).toBe('1.0.0'); + }); + + it('sets imports from file references', () => { + const { config } = parseYamlSafe(APPLICATION_CONFIG_YAML); + expect(config.imports).toEqual(['base.yaml', 'users.yaml', 'billing.yaml']); + }); +}); + +// --------------------------------------------------------------------------- +// buildApplicationConfigYaml +// --------------------------------------------------------------------------- + +describe('buildApplicationConfigYaml', () => { + it('serialises ApplicationConfigMeta to application: format', () => { + const yaml = buildApplicationConfigYaml({ + name: 'my-service', + version: '1.0.0', + workflows: [{ file: 'base.yaml' }, { file: 'users.yaml' }], + }); + expect(yaml).toContain('application:'); + expect(yaml).toContain('name: my-service'); + expect(yaml).toContain('version: 1.0.0'); + expect(yaml).toContain('- file: base.yaml'); + expect(yaml).toContain('- file: users.yaml'); + expect(yaml).not.toContain('imports:'); + expect(yaml).not.toContain('modules:'); + }); + + it('omits version when not set', () => { + const yaml = buildApplicationConfigYaml({ + name: 'my-service', + workflows: [{ file: 'base.yaml' }], + }); + expect(yaml).toContain('name: my-service'); + expect(yaml).not.toContain('version:'); + }); +}); + +// --------------------------------------------------------------------------- +// configToYaml — ApplicationConfig round-trip +// --------------------------------------------------------------------------- + +describe('configToYaml — ApplicationConfig round-trip', () => { + it('emits application: format when _applicationConfig is set', () => { + const config = parseYaml(APPLICATION_CONFIG_YAML); + const output = configToYaml(config); + expect(output).toContain('application:'); + expect(output).toContain('name: my-service'); + expect(output).toContain('workflows:'); + expect(output).toContain('- file: base.yaml'); + expect(output).toContain('- file: users.yaml'); + expect(output).toContain('- file: billing.yaml'); + // Must NOT convert to flat format + expect(output).not.toContain('imports:'); + expect(output).not.toContain('modules:'); + }); + + it('round-trips the ApplicationConfig YAML with minimal whitespace changes', () => { + const config = parseYaml(APPLICATION_CONFIG_YAML); + const output = configToYaml(config); + // The round-tripped YAML must be parseable back to the same structure + const reparsed = parseYaml(output); + expect(reparsed._applicationConfig!.name).toBe('my-service'); + expect(reparsed._applicationConfig!.version).toBe('1.0.0'); + expect(reparsed._applicationConfig!.workflows).toHaveLength(3); + }); +}); + +// --------------------------------------------------------------------------- +// resolveImports — sets _applicationConfig on returned config +// --------------------------------------------------------------------------- + +describe('resolveImports — preserves _applicationConfig metadata', () => { + function makeResolver(files: Record) { + return async (path: string): Promise => files[path] ?? null; + } + + it('sets _applicationConfig on the merged config', async () => { + const resolver = makeResolver({ + 'base.yaml': BASE_YAML, + 'users.yaml': USERS_YAML, + 'billing.yaml': BILLING_YAML, + }); + const { config, error } = await resolveImports(APPLICATION_CONFIG_YAML, resolver); + expect(error).toBeUndefined(); + expect(config._applicationConfig).toBeDefined(); + expect(config._applicationConfig!.name).toBe('my-service'); + expect(config._applicationConfig!.version).toBe('1.0.0'); + expect(config._applicationConfig!.workflows).toEqual([ + { file: 'base.yaml' }, + { file: 'users.yaml' }, + { file: 'billing.yaml' }, + ]); + }); + + it('merged config still has all resolved modules from sub-files', async () => { + const resolver = makeResolver({ + 'base.yaml': BASE_YAML, + 'users.yaml': USERS_YAML, + 'billing.yaml': BILLING_YAML, + }); + const { config } = await resolveImports(APPLICATION_CONFIG_YAML, resolver); + const names = config.modules.map((m) => m.name); + expect(names).toContain('cache'); + expect(names).toContain('user-db'); + expect(names).toContain('user-handler'); + expect(names).toContain('billing-db'); + }); + + it('merged config has workflows from sub-files', async () => { + const resolver = makeResolver({ + 'base.yaml': BASE_YAML, + 'users.yaml': USERS_YAML, + 'billing.yaml': BILLING_YAML, + }); + const { config } = await resolveImports(APPLICATION_CONFIG_YAML, resolver); + expect(config.workflows).toHaveProperty('http'); + }); +}); + +// --------------------------------------------------------------------------- +// exportMainFileYaml — ApplicationConfig format preservation +// --------------------------------------------------------------------------- + +describe('exportMainFileYaml — preserves ApplicationConfig format', () => { + function makeResolver(files: Record) { + return async (path: string): Promise => files[path] ?? null; + } + + it('emits application: format for the main file, not flat imports:', async () => { + const resolver = makeResolver({ + 'base.yaml': BASE_YAML, + 'users.yaml': USERS_YAML, + 'billing.yaml': BILLING_YAML, + }); + const { config, sourceMap } = await resolveImports(APPLICATION_CONFIG_YAML, resolver); + const mainYaml = exportMainFileYaml(config, sourceMap); + + expect(mainYaml).toContain('application:'); + expect(mainYaml).toContain('- file: base.yaml'); + expect(mainYaml).toContain('- file: users.yaml'); + expect(mainYaml).toContain('- file: billing.yaml'); + // Must NOT produce flat format + expect(mainYaml).not.toContain('imports:'); + expect(mainYaml).not.toContain('modules:'); + }); + + it('does not inline sub-file content into the main ApplicationConfig file', async () => { + const resolver = makeResolver({ + 'base.yaml': BASE_YAML, + 'users.yaml': USERS_YAML, + 'billing.yaml': BILLING_YAML, + }); + const { config, sourceMap } = await resolveImports(APPLICATION_CONFIG_YAML, resolver); + const mainYaml = exportMainFileYaml(config, sourceMap); + + // Sub-file module names must not appear in the main ApplicationConfig + expect(mainYaml).not.toContain('cache'); + expect(mainYaml).not.toContain('user-db'); + expect(mainYaml).not.toContain('billing-db'); + }); +}); + +// --------------------------------------------------------------------------- +// exportToFiles — ApplicationConfig format preservation +// --------------------------------------------------------------------------- + +describe('exportToFiles — preserves ApplicationConfig for main file', () => { + function makeResolver(files: Record) { + return async (path: string): Promise => files[path] ?? null; + } + + it('main file is in application: format, sub-files contain their modules', async () => { + const resolver = makeResolver({ + 'base.yaml': BASE_YAML, + 'users.yaml': USERS_YAML, + 'billing.yaml': BILLING_YAML, + }); + const { config, sourceMap } = await resolveImports(APPLICATION_CONFIG_YAML, resolver); + const fileMap = exportToFiles(config, sourceMap); + + const mainYaml = fileMap.get(null)!; + expect(mainYaml).toContain('application:'); + expect(mainYaml).not.toContain('imports:'); + expect(mainYaml).not.toContain('modules:'); + + // Sub-files still contain their modules + expect(fileMap.get('base.yaml')).toContain('cache'); + expect(fileMap.get('users.yaml')).toContain('user-db'); + expect(fileMap.get('billing.yaml')).toContain('billing-db'); + }); + + it('preserves name and version in the emitted application: block', async () => { + const resolver = makeResolver({ + 'base.yaml': BASE_YAML, + 'users.yaml': USERS_YAML, + 'billing.yaml': BILLING_YAML, + }); + const { config, sourceMap } = await resolveImports(APPLICATION_CONFIG_YAML, resolver); + const fileMap = exportToFiles(config, sourceMap); + const mainYaml = fileMap.get(null)!; + + expect(mainYaml).toContain('name: my-service'); + expect(mainYaml).toContain('version: 1.0.0'); + }); +}); diff --git a/src/utils/serialization-multifile.test.ts b/src/utils/serialization-multifile.test.ts index 56e8dbe..7f9e7e2 100644 --- a/src/utils/serialization-multifile.test.ts +++ b/src/utils/serialization-multifile.test.ts @@ -351,9 +351,10 @@ describe('resolveImports — pipeline sourceMap enables correct round-trip expor const fileMap = exportToFiles(config, sourceMap); - // Main file modules list must be empty (all modules belong to imported files) + // Main file preserves the original ApplicationConfig format — it has no modules: block. const mainYaml = fileMap.get(null)!; - expect(mainYaml).toMatch(/^modules:\s*\[\]/m); + expect(mainYaml).toContain('application:'); + expect(mainYaml).not.toContain('modules:'); // Modules appear in their respective source files expect(fileMap.get('api.yaml')).toContain('http-server'); @@ -372,10 +373,11 @@ describe('resolveImports — pipeline sourceMap enables correct round-trip expor const fileMap = exportToFiles(config, sourceMap); const mainYaml = fileMap.get(null)!; - // Main file should reference imported files, not inline their content - expect(mainYaml).toContain('imports:'); + // Main file preserves ApplicationConfig format with application.workflows[].file references + expect(mainYaml).toContain('application:'); + expect(mainYaml).toContain('workflows:'); // Each imported file path must appear as a reference - const importedFiles = ['api.yaml', 'base.yaml', 'database.yaml']; + const importedFiles = ['api.yaml', 'base.yaml']; const hasAtLeastOneRef = importedFiles.some((f) => mainYaml.includes(f)); expect(hasAtLeastOneRef).toBe(true); }); diff --git a/src/utils/serialization.ts b/src/utils/serialization.ts index e0ed137..90b806e 100644 --- a/src/utils/serialization.ts +++ b/src/utils/serialization.ts @@ -805,7 +805,27 @@ export function nodeComponentType(moduleType: string): string { return 'infrastructureNode'; } +/** + * Serialise an ApplicationConfigMeta back to the original `application:` format YAML. + * Used to preserve the ApplicationConfig structure on export instead of converting + * to the flat WorkflowConfig format. + */ +export function buildApplicationConfigYaml( + appConfig: import('../types/workflow.ts').ApplicationConfigMeta, +): string { + const appBlock: Record = {}; + if (appConfig.name !== undefined) appBlock.name = appConfig.name; + if (appConfig.version !== undefined) appBlock.version = appConfig.version; + appBlock.workflows = appConfig.workflows; + return yaml.dump({ application: appBlock }, { lineWidth: -1, noRefs: true, sortKeys: false }); +} + export function configToYaml(config: WorkflowConfig): string { + // If this config was originally in ApplicationConfig format, preserve that structure. + if (config._applicationConfig) { + return buildApplicationConfigYaml(config._applicationConfig); + } + // Strip internal tracking fields and omit empty top-level arrays/objects // that were not present in the original YAML const originalKeys = config._originalKeys; @@ -841,6 +861,34 @@ export function parseYaml(text: string): WorkflowConfig { return { modules: [], workflows: {}, triggers: {}, _originalKeys: [] }; } const _originalKeys = Object.keys(parsed); + + // Detect ApplicationConfig format: top-level `application:` key with `workflows[].file` refs + const application = parsed.application as Record | undefined; + if (application && typeof application === 'object') { + const appWorkflows = application.workflows as Array> | undefined; + if (Array.isArray(appWorkflows) && appWorkflows.some((w) => typeof w.file === 'string')) { + const fileRefs = appWorkflows + .filter((w) => typeof w.file === 'string') + .map((w) => ({ file: w.file as string })); + const _applicationConfig = { + name: application.name as string | undefined, + version: application.version !== undefined ? String(application.version) : undefined, + workflows: fileRefs, + }; + const config: WorkflowConfig = { + modules: (parsed.modules ?? []) as ModuleConfig[], + workflows: (parsed.workflows ?? {}) as Record, + triggers: (parsed.triggers ?? {}) as Record, + imports: fileRefs.map((r) => r.file), + _originalKeys, + _applicationConfig, + }; + if (_applicationConfig.name) config.name = _applicationConfig.name; + if (_applicationConfig.version) config.version = _applicationConfig.version; + return config; + } + } + const config: WorkflowConfig = { modules: (parsed.modules ?? []) as ModuleConfig[], workflows: (parsed.workflows ?? {}) as Record, @@ -884,6 +932,34 @@ export function parseYamlSafe(text: string): { config: WorkflowConfig; error?: s return { config: { modules: [], workflows: {}, triggers: {}, _originalKeys: [] }, error: 'YAML parsed to non-object value' }; } const _originalKeys = Object.keys(parsed); + + // Detect ApplicationConfig format: top-level `application:` key with `workflows[].file` refs + const application = parsed.application as Record | undefined; + if (application && typeof application === 'object') { + const appWorkflows = application.workflows as Array> | undefined; + if (Array.isArray(appWorkflows) && appWorkflows.some((w) => typeof w.file === 'string')) { + const fileRefs = appWorkflows + .filter((w) => typeof w.file === 'string') + .map((w) => ({ file: w.file as string })); + const _applicationConfig = { + name: application.name as string | undefined, + version: application.version !== undefined ? String(application.version) : undefined, + workflows: fileRefs, + }; + const config: WorkflowConfig = { + modules: (parsed.modules ?? []) as ModuleConfig[], + workflows: (parsed.workflows ?? {}) as Record, + triggers: (parsed.triggers ?? {}) as Record, + imports: fileRefs.map((r) => r.file), + _originalKeys, + _applicationConfig, + }; + if (_applicationConfig.name) config.name = _applicationConfig.name; + if (_applicationConfig.version) config.version = _applicationConfig.version; + return { config }; + } + } + const config: WorkflowConfig = { modules: (parsed.modules ?? []) as ModuleConfig[], workflows: (parsed.workflows ?? {}) as Record, @@ -1162,9 +1238,20 @@ export async function resolveImports( // Handle `application.workflows[].file:` directive — conflicts are reported as errors const application = parsed.application as Record | undefined; + let applicationConfig: import('../types/workflow.ts').ApplicationConfigMeta | undefined; if (application && typeof application === 'object') { const appWorkflows = (application.workflows ?? []) as Array>; if (Array.isArray(appWorkflows)) { + const fileRefs = appWorkflows + .filter((entry) => typeof entry.file === 'string') + .map((entry) => ({ file: entry.file as string })); + if (fileRefs.length > 0) { + applicationConfig = { + name: application.name as string | undefined, + version: application.version !== undefined ? String(application.version) : undefined, + workflows: fileRefs, + }; + } for (const entry of appWorkflows) { const filePath = entry.file as string | undefined; if (!filePath) continue; @@ -1188,6 +1275,12 @@ export async function resolveImports( if (configName) config.name = configName; if (configVersion) config.version = configVersion; + // Preserve ApplicationConfig structure so round-trip export can reconstruct + // the original `application:` format instead of converting to flat imports:. + if (applicationConfig) { + config._applicationConfig = applicationConfig; + } + return { config, sourceMap, @@ -1280,7 +1373,32 @@ function buildMainFileContent( fileModules: Map, filePipelines: Map>, ): { yaml: string; importedFiles: string[] } { + // If the original config was ApplicationConfig format, preserve that structure. + // Only fall back to flat format if there are main-file modules (newly added nodes + // that don't belong to any sub-file) — in that case we can't emit pure ApplicationConfig. const mainModules = fileModules.get(null) ?? []; + if (config._applicationConfig && mainModules.length === 0) { + // Reconstruct the application: block, updating name/version from the live config + const appConfig = { + ...config._applicationConfig, + ...(config.name !== undefined ? { name: config.name } : {}), + ...(config.version !== undefined ? { version: config.version } : {}), + }; + // Collect ALL files that have content (direct + transitively imported). + // The direct application.workflows[].file references are included first so + // they are processed; any additional transitively imported files are appended. + const directFiles = new Set(config._applicationConfig.workflows.map((w) => w.file)); + const allImportedFiles = [ + ...directFiles, + ...[...fileModules.keys()].filter((k) => k !== null && !directFiles.has(k as string)), + ...[...filePipelines.keys()].filter((k) => k !== null && !directFiles.has(k as string)), + ] as string[]; + return { + yaml: buildApplicationConfigYaml(appConfig), + importedFiles: allImportedFiles, + }; + } + const mainConfig: Record = {}; if (config.name !== undefined) mainConfig.name = config.name; if (config.version !== undefined) mainConfig.version = config.version; From fe10bb1f630723c86617862ff66187eeb51e6f2d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 13:43:59 +0000 Subject: [PATCH 3/5] Address code review: use top-level ApplicationConfigMeta import, consolidate file collection logic Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-editor/sessions/cde0510e-a5f2-413a-8ccb-0d0493cb622c Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- src/utils/serialization.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/utils/serialization.ts b/src/utils/serialization.ts index 90b806e..fe743c6 100644 --- a/src/utils/serialization.ts +++ b/src/utils/serialization.ts @@ -12,6 +12,7 @@ import type { EventWorkflowConfig, WorkflowTab, ModuleTypeInfo, + ApplicationConfigMeta, } from '../types/workflow.ts'; import { MODULE_TYPE_MAP } from '../types/workflow.ts'; import { layoutNodes } from './autoLayout.ts'; @@ -810,9 +811,7 @@ export function nodeComponentType(moduleType: string): string { * Used to preserve the ApplicationConfig structure on export instead of converting * to the flat WorkflowConfig format. */ -export function buildApplicationConfigYaml( - appConfig: import('../types/workflow.ts').ApplicationConfigMeta, -): string { +export function buildApplicationConfigYaml(appConfig: ApplicationConfigMeta): string { const appBlock: Record = {}; if (appConfig.name !== undefined) appBlock.name = appConfig.name; if (appConfig.version !== undefined) appBlock.version = appConfig.version; @@ -1238,7 +1237,7 @@ export async function resolveImports( // Handle `application.workflows[].file:` directive — conflicts are reported as errors const application = parsed.application as Record | undefined; - let applicationConfig: import('../types/workflow.ts').ApplicationConfigMeta | undefined; + let applicationConfig: ApplicationConfigMeta | undefined; if (application && typeof application === 'object') { const appWorkflows = (application.workflows ?? []) as Array>; if (Array.isArray(appWorkflows)) { @@ -1385,17 +1384,18 @@ function buildMainFileContent( ...(config.version !== undefined ? { version: config.version } : {}), }; // Collect ALL files that have content (direct + transitively imported). - // The direct application.workflows[].file references are included first so - // they are processed; any additional transitively imported files are appended. - const directFiles = new Set(config._applicationConfig.workflows.map((w) => w.file)); - const allImportedFiles = [ - ...directFiles, - ...[...fileModules.keys()].filter((k) => k !== null && !directFiles.has(k as string)), - ...[...filePipelines.keys()].filter((k) => k !== null && !directFiles.has(k as string)), - ] as string[]; + // Start with direct application.workflows[].file references for ordering, + // then append any additional files from the sourceMap (transitively imported). + const allFiles = new Set(config._applicationConfig.workflows.map((w) => w.file)); + for (const k of fileModules.keys()) { + if (k !== null) allFiles.add(k); + } + for (const k of filePipelines.keys()) { + if (k !== null) allFiles.add(k); + } return { yaml: buildApplicationConfigYaml(appConfig), - importedFiles: allImportedFiles, + importedFiles: [...allFiles], }; } From 7ef4683b65911e930f02cb5098ab8bec08039a9d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 14:39:18 +0000 Subject: [PATCH 4/5] Address PR review: guard configToYaml, filter empty export files, fix warning condition, strengthen test assertion Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-editor/sessions/365aa754-1dd3-4dce-8884-3575138941a1 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- src/components/WorkflowEditor.tsx | 2 +- .../serialization-applicationconfig.test.ts | 30 +++++++++++- src/utils/serialization-multifile.test.ts | 5 +- src/utils/serialization.ts | 46 +++++++++++++++---- 4 files changed, 70 insertions(+), 13 deletions(-) diff --git a/src/components/WorkflowEditor.tsx b/src/components/WorkflowEditor.tsx index 76e72fe..d666a37 100644 --- a/src/components/WorkflowEditor.tsx +++ b/src/components/WorkflowEditor.tsx @@ -69,7 +69,7 @@ export function WorkflowEditor(props: WorkflowEditorProps) { } // ApplicationConfig format detected but no file resolver provided — warn the user // rather than silently converting the format. - if (config._applicationConfig && !sourceMapProp) { + if (config._applicationConfig && !onResolveFile) { addToast( 'ApplicationConfig format detected. Configure a workspace file resolver to render the full application graph from referenced sub-files.', 'warning', diff --git a/src/utils/serialization-applicationconfig.test.ts b/src/utils/serialization-applicationconfig.test.ts index 99c36c7..e3536cd 100644 --- a/src/utils/serialization-applicationconfig.test.ts +++ b/src/utils/serialization-applicationconfig.test.ts @@ -195,7 +195,7 @@ describe('buildApplicationConfigYaml', () => { // --------------------------------------------------------------------------- describe('configToYaml — ApplicationConfig round-trip', () => { - it('emits application: format when _applicationConfig is set', () => { + it('emits application: format when _applicationConfig is set and config is metadata-only', () => { const config = parseYaml(APPLICATION_CONFIG_YAML); const output = configToYaml(config); expect(output).toContain('application:'); @@ -209,6 +209,18 @@ describe('configToYaml — ApplicationConfig round-trip', () => { expect(output).not.toContain('modules:'); }); + it('falls back to flat WorkflowConfig when _applicationConfig is set but config has real content', () => { + // Simulate a resolved ApplicationConfig that has merged sub-file modules + const config = parseYaml(APPLICATION_CONFIG_YAML); + config.modules = [{ name: 'cache', type: 'nosql.redis' }]; + const output = configToYaml(config); + // Must NOT emit pure application: format since there is real module content + expect(output).not.toBe(buildApplicationConfigYaml(config._applicationConfig!)); + // Instead serialises the full WorkflowConfig (modules are present) + expect(output).toContain('cache'); + expect(output).toContain('modules:'); + }); + it('round-trips the ApplicationConfig YAML with minimal whitespace changes', () => { const config = parseYaml(APPLICATION_CONFIG_YAML); const output = configToYaml(config); @@ -357,4 +369,20 @@ describe('exportToFiles — preserves ApplicationConfig for main file', () => { expect(mainYaml).toContain('name: my-service'); expect(mainYaml).toContain('version: 1.0.0'); }); + + it('does not write empty YAML files for sub-files that have no exported content', async () => { + // Only base.yaml has content; users.yaml and billing.yaml are missing from the resolver + const resolver = makeResolver({ + 'base.yaml': BASE_YAML, + // users.yaml and billing.yaml are intentionally unresolvable + }); + const { config, sourceMap } = await resolveImports(APPLICATION_CONFIG_YAML, resolver); + const fileMap = exportToFiles(config, sourceMap); + + // base.yaml has content and should be included + expect(fileMap.has('base.yaml')).toBe(true); + // users.yaml and billing.yaml had no resolvable content — should NOT be in the file map + expect(fileMap.has('users.yaml')).toBe(false); + expect(fileMap.has('billing.yaml')).toBe(false); + }); }); diff --git a/src/utils/serialization-multifile.test.ts b/src/utils/serialization-multifile.test.ts index 7f9e7e2..535f16c 100644 --- a/src/utils/serialization-multifile.test.ts +++ b/src/utils/serialization-multifile.test.ts @@ -378,8 +378,9 @@ describe('resolveImports — pipeline sourceMap enables correct round-trip expor expect(mainYaml).toContain('workflows:'); // Each imported file path must appear as a reference const importedFiles = ['api.yaml', 'base.yaml']; - const hasAtLeastOneRef = importedFiles.some((f) => mainYaml.includes(f)); - expect(hasAtLeastOneRef).toBe(true); + importedFiles.forEach((file) => { + expect(mainYaml).toContain(file); + }); }); }); diff --git a/src/utils/serialization.ts b/src/utils/serialization.ts index fe743c6..1749b45 100644 --- a/src/utils/serialization.ts +++ b/src/utils/serialization.ts @@ -819,9 +819,19 @@ export function buildApplicationConfigYaml(appConfig: ApplicationConfigMeta): st return yaml.dump({ application: appBlock }, { lineWidth: -1, noRefs: true, sortKeys: false }); } +function isMetadataOnlyApplicationConfig(config: WorkflowConfig): boolean { + const hasModules = (config.modules?.length ?? 0) > 0; + const hasWorkflows = Object.keys(config.workflows ?? {}).length > 0; + const hasTriggers = Object.keys(config.triggers ?? {}).length > 0; + const hasPipelines = Object.keys(config.pipelines ?? {}).length > 0; + return !hasModules && !hasWorkflows && !hasTriggers && !hasPipelines; +} + export function configToYaml(config: WorkflowConfig): string { - // If this config was originally in ApplicationConfig format, preserve that structure. - if (config._applicationConfig) { + // Preserve `application:` output only while the config is still metadata-only. + // If real main-file content exists, serialise the full WorkflowConfig instead + // so top-level modules/workflows/triggers/pipelines are not dropped. + if (config._applicationConfig && isMetadataOnlyApplicationConfig(config)) { return buildApplicationConfigYaml(config._applicationConfig); } @@ -1383,19 +1393,37 @@ function buildMainFileContent( ...(config.name !== undefined ? { name: config.name } : {}), ...(config.version !== undefined ? { version: config.version } : {}), }; - // Collect ALL files that have content (direct + transitively imported). - // Start with direct application.workflows[].file references for ordering, - // then append any additional files from the sourceMap (transitively imported). - const allFiles = new Set(config._applicationConfig.workflows.map((w) => w.file)); + // Collect only files that actually have exported content (modules or pipelines). + // Preserve direct application.workflows[].file ordering first, then append any + // additional transitive files discovered through the source maps. + const hasExportedContent = (file: string): boolean => { + const modules = fileModules.get(file) ?? []; + const pipelines = filePipelines.get(file) ?? {}; + return modules.length > 0 || Object.keys(pipelines).length > 0; + }; + const importedFiles: string[] = []; + const seenFiles = new Set(); + for (const workflow of config._applicationConfig.workflows) { + if (hasExportedContent(workflow.file) && !seenFiles.has(workflow.file)) { + seenFiles.add(workflow.file); + importedFiles.push(workflow.file); + } + } for (const k of fileModules.keys()) { - if (k !== null) allFiles.add(k); + if (k !== null && hasExportedContent(k) && !seenFiles.has(k)) { + seenFiles.add(k); + importedFiles.push(k); + } } for (const k of filePipelines.keys()) { - if (k !== null) allFiles.add(k); + if (k !== null && hasExportedContent(k) && !seenFiles.has(k)) { + seenFiles.add(k); + importedFiles.push(k); + } } return { yaml: buildApplicationConfigYaml(appConfig), - importedFiles: [...allFiles], + importedFiles, }; } From 90f9ae39de112b3df251a6b84c8a9520c384c8e1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Apr 2026 15:34:33 +0000 Subject: [PATCH 5/5] Merge master into branch: integrate _extraTopLevelKeys, importedPassthrough, and preserve ApplicationConfig format - Keep both _applicationConfig and _extraTopLevelKeys on WorkflowConfig - Replace applicationConfig store field with importedPassthrough (master's approach) - Explicitly reattach _applicationConfig in exportToConfig from importedPassthrough - Integrate KNOWN_TOP_LEVEL_KEYS and extractExtraTopLevelKeys from master alongside buildApplicationConfigYaml and isMetadataOnlyApplicationConfig from our branch - Use master's buildMainFileContent (delegate to configToYaml); clear workflows/triggers in mainOnlyConfig for ApplicationConfig format so isMetadataOnlyApplicationConfig works correctly on the merged config - All 4055 tests pass Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-editor/sessions/a1a28ad8-429d-4928-80f4-956801c8d6b4 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- src/stores/workflowStore.ts | 39 +++-- src/types/workflow.ts | 2 + src/utils/serialization-bugs.test.ts | 160 +++++++++++++++++++- src/utils/serialization.ts | 215 +++++++++++++++++---------- 4 files changed, 324 insertions(+), 92 deletions(-) diff --git a/src/stores/workflowStore.ts b/src/stores/workflowStore.ts index 305dd92..f3674d0 100644 --- a/src/stores/workflowStore.ts +++ b/src/stores/workflowStore.ts @@ -10,7 +10,7 @@ import { applyEdgeChanges, addEdge as rfAddEdge, } from '@xyflow/react'; -import type { WorkflowConfig, WorkflowTab, CrossWorkflowLink, ApplicationConfigMeta } from '../types/workflow.ts'; +import type { WorkflowConfig, WorkflowTab, CrossWorkflowLink } from '../types/workflow.ts'; import { MODULE_TYPE_MAP } from '../types/workflow.ts'; import useModuleSchemaStore from './moduleSchemaStore.ts'; import { nodesToConfig, configToNodes, nodeComponentType, exportToFiles, exportMainFileYaml } from '../utils/serialization.ts'; @@ -105,9 +105,14 @@ interface WorkflowStore { importedTriggers: Record; importedPipelines: Record; - // ApplicationConfig format metadata — preserved so the main file can be - // round-tripped back to the original `application:` structure on export. - applicationConfig: ApplicationConfigMeta | null; + /** + * 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; @@ -222,9 +227,7 @@ const useWorkflowStore = create()( importedWorkflows: {}, importedTriggers: {}, importedPipelines: {}, - - // ApplicationConfig format metadata (null = flat WorkflowConfig format) - applicationConfig: null, + importedPassthrough: null, // Multi-file resolution sourceMap: new Map(), @@ -460,9 +463,15 @@ const useWorkflowStore = create()( }, exportToConfig: () => { - const { nodes, edges, importedWorkflows, importedTriggers, importedPipelines, applicationConfig } = 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; } @@ -474,8 +483,8 @@ const useWorkflowStore = create()( } // Reattach ApplicationConfig metadata so configToYaml / exportMainFileYaml // can reconstruct the original `application:` format on export. - if (applicationConfig) { - config._applicationConfig = applicationConfig; + if (originalConfig?._applicationConfig) { + config._applicationConfig = originalConfig._applicationConfig; } return config; }, @@ -496,6 +505,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, @@ -503,8 +515,7 @@ const useWorkflowStore = create()( importedWorkflows: config.workflows ?? {}, importedTriggers: config.triggers ?? {}, importedPipelines: config.pipelines ?? {}, - // Preserve ApplicationConfig metadata (null clears it for flat WorkflowConfig files) - applicationConfig: config._applicationConfig ?? null, + importedPassthrough, }; if (sourceMap) { updates.sourceMap = sourceMap; @@ -515,7 +526,7 @@ const useWorkflowStore = create()( clearCanvas: () => { get().pushHistory(); - set({ nodes: [], edges: [], selectedNodeId: null, selectedEdgeId: null, nodeCounter: 0, importedWorkflows: {}, importedTriggers: {}, importedPipelines: {}, applicationConfig: null }); + 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 e76e106..6db2158 100644 --- a/src/types/workflow.ts +++ b/src/types/workflow.ts @@ -37,6 +37,8 @@ export interface WorkflowConfig { _originalKeys?: string[]; /** Present when the source file used the ApplicationConfig format. */ _applicationConfig?: ApplicationConfigMeta; + /** 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 1749b45..e04f9f4 100644 --- a/src/utils/serialization.ts +++ b/src/utils/serialization.ts @@ -541,6 +541,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; } @@ -806,6 +810,25 @@ 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; +} + /** * Serialise an ApplicationConfigMeta back to the original `application:` format YAML. * Used to preserve the ApplicationConfig structure on export instead of converting @@ -838,27 +861,64 @@ 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 }); } @@ -928,6 +988,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: [] }; @@ -984,6 +1048,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 }; @@ -1283,6 +1366,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; + } // Preserve ApplicationConfig structure so round-trip export can reconstruct // the original `application:` format instead of converting to flat imports:. @@ -1376,71 +1465,16 @@ 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, fileModules: Map, filePipelines: Map>, ): { yaml: string; importedFiles: string[] } { - // If the original config was ApplicationConfig format, preserve that structure. - // Only fall back to flat format if there are main-file modules (newly added nodes - // that don't belong to any sub-file) — in that case we can't emit pure ApplicationConfig. const mainModules = fileModules.get(null) ?? []; - if (config._applicationConfig && mainModules.length === 0) { - // Reconstruct the application: block, updating name/version from the live config - const appConfig = { - ...config._applicationConfig, - ...(config.name !== undefined ? { name: config.name } : {}), - ...(config.version !== undefined ? { version: config.version } : {}), - }; - // Collect only files that actually have exported content (modules or pipelines). - // Preserve direct application.workflows[].file ordering first, then append any - // additional transitive files discovered through the source maps. - const hasExportedContent = (file: string): boolean => { - const modules = fileModules.get(file) ?? []; - const pipelines = filePipelines.get(file) ?? {}; - return modules.length > 0 || Object.keys(pipelines).length > 0; - }; - const importedFiles: string[] = []; - const seenFiles = new Set(); - for (const workflow of config._applicationConfig.workflows) { - if (hasExportedContent(workflow.file) && !seenFiles.has(workflow.file)) { - seenFiles.add(workflow.file); - importedFiles.push(workflow.file); - } - } - for (const k of fileModules.keys()) { - if (k !== null && hasExportedContent(k) && !seenFiles.has(k)) { - seenFiles.add(k); - importedFiles.push(k); - } - } - for (const k of filePipelines.keys()) { - if (k !== null && hasExportedContent(k) && !seenFiles.has(k)) { - seenFiles.add(k); - importedFiles.push(k); - } - } - return { - yaml: buildApplicationConfigYaml(appConfig), - importedFiles, - }; - } - - 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 = [ @@ -1449,11 +1483,38 @@ 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, + // For ApplicationConfig format, all workflows and triggers live exclusively in sub-files. + // Clear them from the main-file view so isMetadataOnlyApplicationConfig can correctly + // identify the main file as metadata-only and emit application: format. + ...(config._applicationConfig ? { workflows: {}, triggers: {} } : {}), + // 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 }; } /**