Design doc and test validation for multi-file workflow config permutations#5
Conversation
… and serialization tests - Design doc covering multi-file split strategies, YAML side-pane, visual file boundaries, node-to-file navigation, and IDE integration hooks - Implementation plan with 12 phased tasks covering fixtures, tests, components, and IDE bridge updates - Three new fixture sets: domain-split (5 files), layer-split (5 files), nested-directory (8 files, 3+ levels deep) - Three new test files with 36 test cases covering resolveImports, sourceMap, exportToFiles round-trip, configToNodes, cross-file isolation, error handling Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-editor/sessions/21f17a33-a494-45ec-acbc-a587750bdb2a Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-editor/sessions/21f17a33-a494-45ec-acbc-a587750bdb2a Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds comprehensive documentation and test/fixture coverage for multi-file workflow config permutations (domain-split, layer-split, nested directories) to harden resolveImports → sourceMap → exportToFiles → configToNodes behaviors and guide future editor features (YAML side-pane, file boundaries, IDE navigation hooks).
Changes:
- Added 3 new multi-file fixture sets (
multifile-domain,multifile-layers,multifile-nested) covering real-world split patterns. - Added 3 new Vitest suites validating import resolution, source mapping, export routing, and node creation for each split strategy.
- Added a design doc + implementation plan describing future UI/navigation work and a test matrix.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/plans/2026-03-26-multifile-config-validation-design.md | Design doc for multi-file validation + YAML side-pane + file boundaries + navigation hooks. |
| docs/plans/2026-03-26-multifile-config-validation-implementation.md | Implementation plan detailing fixtures/tests and future editor/IDE work. |
| src/utils/serialization-multifile-domain.test.ts | Domain-split serialization tests (resolveImports/sourceMap/exportToFiles/configToNodes). |
| src/utils/serialization-multifile-layers.test.ts | Layer-split serialization tests (resolveImports/sourceMap/exportToFiles/configToNodes). |
| src/utils/serialization-multifile-nested.test.ts | Nested-directory serialization tests including relative import resolution and missing-file behavior. |
| test-fixtures/multifile-domain/app.yaml | Root domain-split fixture importing domain/shared files and defining workflows. |
| test-fixtures/multifile-domain/domains/auth.yaml | Domain fixture: auth modules + pipelines. |
| test-fixtures/multifile-domain/domains/billing.yaml | Domain fixture: billing modules + pipelines. |
| test-fixtures/multifile-domain/domains/notifications.yaml | Domain fixture: notifications modules + pipelines. |
| test-fixtures/multifile-domain/shared/infra.yaml | Shared infra fixture: server/router/logger modules. |
| test-fixtures/multifile-layers/app.yaml | Root layer-split fixture importing infrastructure/middleware/services/api layers. |
| test-fixtures/multifile-layers/layers/infrastructure.yaml | Infrastructure layer fixture: shared modules. |
| test-fixtures/multifile-layers/layers/middleware.yaml | Middleware layer fixture: middleware pipelines. |
| test-fixtures/multifile-layers/layers/services.yaml | Services layer fixture: service pipelines. |
| test-fixtures/multifile-layers/layers/api.yaml | API layer fixture: server/router modules + HTTP workflows. |
| test-fixtures/multifile-nested/app.yaml | Root nested fixture importing a platform aggregator. |
| test-fixtures/multifile-nested/platform/platform.yaml | Platform-level aggregator fixture importing core + features aggregators. |
| test-fixtures/multifile-nested/platform/core/core.yaml | Core aggregator fixture importing database + cache leaf files. |
| test-fixtures/multifile-nested/platform/core/database.yaml | Core leaf fixture: database modules. |
| test-fixtures/multifile-nested/platform/core/cache.yaml | Core leaf fixture: cache module. |
| test-fixtures/multifile-nested/platform/features/features.yaml | Features aggregator fixture importing auth + payments leaf files. |
| test-fixtures/multifile-nested/platform/features/auth.yaml | Features leaf fixture: auth module + pipelines. |
| test-fixtures/multifile-nested/platform/features/payments.yaml | Features leaf fixture: payments module + pipelines. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| | Task | Type | Files | | ||
| |------|------|-------| | ||
| | 1-3 | Test fixtures | 17 new YAML files across 3 fixture sets | |
There was a problem hiding this comment.
The implementation plan summary claims there are "17 new YAML files", but this PR adds 3 fixture sets totaling 18 YAML files (domain=5, layers=5, nested=8). Please correct the count to avoid confusion when tracking deliverables.
| | 1-3 | Test fixtures | 17 new YAML files across 3 fixture sets | | |
| | 1-3 | Test fixtures | 18 new YAML files across 3 fixture sets (domain=5, layers=5, nested=8) | |
| workflows: | ||
| http: | ||
| server: http-server | ||
| router: router | ||
| routes: | ||
| - method: POST | ||
| path: /api/auth/login | ||
| handler: login | ||
| - method: POST | ||
| path: /api/auth/register | ||
| handler: register | ||
| - method: POST | ||
| path: /api/billing/charge | ||
| handler: charge | ||
| - method: POST | ||
| path: /api/billing/refund | ||
| handler: refund | ||
| - method: POST | ||
| path: /api/notify/email | ||
| handler: send-email |
There was a problem hiding this comment.
In this fixture, the HTTP workflow route handler values (e.g. login, charge) refer to pipeline names, but extractWorkflowEdges() / configToNodes() only create HTTP route edges when handler matches a module name in the merged modules: list. As written, this means you won't get route→handler edges (only the server→router "http" edge). Consider adding explicit handler modules (e.g. http.handler/api.handler) and pointing routes at those module names, or changing the fixture to match the editor's supported routing model so the cross-file edge tests are meaningful.
| workflows: | ||
| http: | ||
| server: http-server | ||
| router: router | ||
| routes: | ||
| - method: GET | ||
| path: /api/users | ||
| handler: user-service | ||
| - method: POST | ||
| path: /api/orders | ||
| handler: order-service | ||
| - method: GET | ||
| path: /api/products | ||
| handler: product-service |
There was a problem hiding this comment.
This workflows.http.routes[].handler points to service pipeline names (e.g. user-service), but the editor’s route edge generation expects handlers to be module names (so it can map them via nameToId). With this fixture, route→handler edges won’t be created. Either add handler modules and reference those here, or adjust the fixture to the supported routing model so any future edge/navigation tests reflect real behavior.
| // Should have http-route edges connecting routes to pipeline handlers | ||
| const routeEdges = edges.filter((e) => { | ||
| const data = e.data as Record<string, unknown> | undefined; | ||
| return data?.edgeType === 'http-route'; | ||
| }); | ||
| expect(routeEdges.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
This assertion only checks that at least one edge exists with edgeType === 'http-route'. That can pass with just the server→router "http" edge, even if there are no route→handler edges for the actual routes. To really validate cross-file route wiring, assert that at least one http-route edge has a label like "METHOD /path" (or otherwise specifically targets a handler node), so the test fails if only the server→router edge is present.
| // Should have http-route edges connecting routes to pipeline handlers | |
| const routeEdges = edges.filter((e) => { | |
| const data = e.data as Record<string, unknown> | undefined; | |
| return data?.edgeType === 'http-route'; | |
| }); | |
| expect(routeEdges.length).toBeGreaterThan(0); | |
| // Should have http-route edges connecting concrete HTTP routes (METHOD /path) to pipeline handlers | |
| const routeEdges = edges.filter((e) => { | |
| const data = e.data as Record<string, unknown> | undefined; | |
| return data?.edgeType === 'http-route'; | |
| }); | |
| const handlerRouteEdges = routeEdges.filter((e) => { | |
| const data = e.data as Record<string, unknown> | undefined; | |
| const label = typeof data?.label === 'string' ? data.label : ''; | |
| // Expect labels like "GET /auth/login" or "POST /foo" | |
| return /\b[A-Z]+\s+\/\S/.test(label); | |
| }); | |
| expect(handlerRouteEdges.length).toBeGreaterThan(0); |
| it('main file only has application metadata and imports', async () => { | ||
| const { config, sourceMap } = await resolveImports(FIXTURE_APP, resolver); | ||
| const fileMap = exportToFiles(config, sourceMap); | ||
| const mainYaml = fileMap.get(null)!; | ||
|
|
||
| expect(mainYaml).toContain('imports:'); | ||
| expect(mainYaml).toMatch(/^modules:\s*\[\]/m); | ||
| expect(mainYaml).not.toMatch(/^pipelines:/m); |
There was a problem hiding this comment.
The test name says the main file "only has application metadata and imports", but exportToFiles() always emits config.workflows into the main (null) file output (see buildMainFileContent()), including workflows merged from layers/api.yaml. Either update the test name/expectations to reflect that workflows are expected in the main file, or add explicit assertions (and possibly new behavior) if workflows are intended to round-trip back into layers/api.yaml.
| it('main file only has application metadata and imports', async () => { | |
| const { config, sourceMap } = await resolveImports(FIXTURE_APP, resolver); | |
| const fileMap = exportToFiles(config, sourceMap); | |
| const mainYaml = fileMap.get(null)!; | |
| expect(mainYaml).toContain('imports:'); | |
| expect(mainYaml).toMatch(/^modules:\s*\[\]/m); | |
| expect(mainYaml).not.toMatch(/^pipelines:/m); | |
| it('main file has application metadata, imports, and workflows', async () => { | |
| const { config, sourceMap } = await resolveImports(FIXTURE_APP, resolver); | |
| const fileMap = exportToFiles(config, sourceMap); | |
| const mainYaml = fileMap.get(null)!; | |
| expect(mainYaml).toContain('imports:'); | |
| expect(mainYaml).toMatch(/^modules:\s*\[\]/m); | |
| expect(mainYaml).toMatch(/^workflows:/m); |
| it('main file references only top-level import', async () => { | ||
| const { config, sourceMap } = await resolveImports(FIXTURE_APP, resolver); | ||
| const fileMap = exportToFiles(config, sourceMap); | ||
| const mainYaml = fileMap.get(null)!; | ||
|
|
||
| expect(mainYaml).toContain('imports:'); | ||
| expect(mainYaml).toMatch(/^modules:\s*\[\]/m); | ||
| // All modules are in imported files |
There was a problem hiding this comment.
This test name claims the main file references only the top-level import, but exportToFiles() currently reconstructs imports: from files that contain modules/pipelines (via sourceMap) and has no way to preserve intermediate "aggregator" files like platform/platform.yaml that contain only imports:. As written, the assertions don’t actually verify the import list. Please either (a) rename the test to reflect the current (flattened) export behavior and assert the expected leaf imports, or (b) extend the export logic/tests to preserve the original import graph if that’s the intended round-trip.
| it('main file references only top-level import', async () => { | |
| const { config, sourceMap } = await resolveImports(FIXTURE_APP, resolver); | |
| const fileMap = exportToFiles(config, sourceMap); | |
| const mainYaml = fileMap.get(null)!; | |
| expect(mainYaml).toContain('imports:'); | |
| expect(mainYaml).toMatch(/^modules:\s*\[\]/m); | |
| // All modules are in imported files | |
| it('main file imports only leaf module and pipeline files', async () => { | |
| const { config, sourceMap } = await resolveImports(FIXTURE_APP, resolver); | |
| const fileMap = exportToFiles(config, sourceMap); | |
| const mainYaml = fileMap.get(null)!; | |
| expect(mainYaml).toContain('imports:'); | |
| expect(mainYaml).toMatch(/^modules:\s*\[\]/m); | |
| // Main file should import the leaf files that actually contain modules/pipelines | |
| expect(mainYaml).toContain('platform/core/database.yaml'); | |
| expect(mainYaml).toContain('platform/core/cache.yaml'); | |
| expect(mainYaml).toContain('platform/features/auth.yaml'); | |
| expect(mainYaml).toContain('platform/features/payments.yaml'); | |
| // All modules are in imported files, not in the main file |
…s, IDE bridge compat - Add HTTP handler modules (api.query/api.command) to domain and layers fixtures so route→handler edges are created by extractWorkflowEdges() - Strengthen edge assertion to verify METHOD /path labels, not just edge type existence - Fix misleading test names: layers export includes workflows in main file; nested export flattens import graph to leaf files - Correct YAML file count in implementation plan (17→18) - Make onNavigateToSource backward-compatible via overloaded signature instead of breaking (filePath, line, col) change - Make NavigateToSourceMessage.filePath optional for IDE bridge compat Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review Comments AddressedAll 6 review comments have been addressed in commit 18ae37b: Fixtures (comments 2-3)Route Domain fixture: 14 modules (was 9) — added 5 handler modules across auth/billing/notifications Test assertions (comment 4)Domain edge test now verifies:
Test names (comments 5-6)
File count (comment 1)Implementation plan summary corrected: IDE bridge backward compatibility (not in comments, but important)Design doc updated:
All 649 tests pass. |
- Add FileTabBar: tab bar with one tab per file, active tab highlighted - Add YamlLineRenderer: YAML with line numbers, syntax tokens, clickable lines - Add YamlSidePane: combines tab bar + renderer, scroll-to-highlighted-range - Add showYamlPane prop to WorkflowEditorProps - Add yamlPaneVisible/yamlPaneWidth to uiLayoutStore - Wire into WorkflowEditor: node selection → active file + highlight range, YAML line click → select corresponding canvas node - 9 tests covering all specified behaviors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…d multi-file support - buildYamlLineMap now maps pipelines (pipelineName keys), pipeline steps (pipelineName:stepName keys), workflows, and triggers in addition to modules - Fix module inline-name detection ( - name: foo pattern) - Add buildMultiFileLineMap for per-file maps across a Map<path, content> - Add lookupNodeInLineMap for cross-file node lookup by name and optional file - Add yamlLineMap.test.ts with 19 tests covering all sections and edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add computeFileGroups() utility: groups nodes by sourceFile, computes bounding boxes with 40px padding, assigns cycling colors from 8-color palette, falls back to sourceMap when node.data.sourceFile is absent, skips groups when <2 distinct source files - Add FileGroupNode component: dashed border, subtle background tint, filename label in top-left, pointer-events none so it doesn't interfere with canvas - Register 'fileGroup' type in src/components/nodes/index.ts - Wire into WorkflowCanvas: prepend file group overlays to displayNodes when sourceMap has 2+ distinct files; groups appear behind canvas nodes (zIndex -1) - 7 tests covering all specified behaviors (684 total pass) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… overload - editor.ts: onNavigateToSource now accepts (line, col) | (filePath, line, col) overload for backward-compatible multi-file navigation; add onNodeFocusRequest hook - navigation.ts: resolveNodeSourceLocation (node → filePath+line) and resolveLineToNode (filePath+line → node) using MultiFileYamlLineMap - WorkflowCanvas: add onNodeClick handler that resolves source location and calls onNavigateToSource(filePath, line, col); accept lineMap + sourceMap props - WorkflowEditor: compute multiFileLineMap from exportToFileMap() and pass to canvas - navigation.test.ts: 12 unit tests covering all navigation helper edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add playwright.config.ts: chromium-only, starts test app on port 5174 - Add e2e/test-app/: minimal Vite+React harness serving WorkflowEditor with multi-file config scenarios (?scenario=multifile-groups|yaml-pane) - Add e2e/multifile.spec.ts: 11 Playwright tests covering: - File group overlay nodes rendered for each source file (2 groups) - File group labels show correct filename - File group overlays don't block canvas interaction - YAML side-pane renders tabs for each file (main/auth.yaml/billing.yaml) - Tab switching works, active tab marked correctly - Node click switches to correct file tab + highlights YAML lines - YAML line click activates node selection feedback - Fix group node width/height: set both node.width/height and style.width/height so React Flow correctly renders measured dimensions; move zIndex out of style to top-level node property All 11 E2E tests pass, 696 unit tests unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- YamlSidePane.test.tsx: add 'scrolls to highlighted lines' test — mocks HTMLElement.prototype.scrollIntoView and verifies it is called when highlightRange is provided (10 tests total) - FileGroupNode.test.tsx: new file with 4 render tests: - renders label text correctly - applies border color from color prop (dashed + color value) - applies background color from color prop - uses dashed border style Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Merge all multi-file describe blocks into e2e/editor.spec.ts - Remove e2e/multifile.spec.ts (now redundant) - Update legacy placeholder tests to use relative URL (test harness on :5174) - All 15 E2E tests pass Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…iewer Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ters Eliminates all sleep-based waits in editor.spec.ts to prevent CI flakiness: - beforeEach blocks: waitForTimeout(500) → expect first node toBeVisible - Post-click waits: waitForTimeout(200/300) → wait for specific DOM state changes (tab active class, highlighted lines visible) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…erence, navigation Five workstreams: schema-driven test matrix, typed schema generation from Go structs, 3-layer DSL documentation system, breadcrumb + interactive file group navigation, and property panel completeness testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8 tasks: schema-driven test matrix (279 types), property panel schema completeness, rendering tests, partial config tests, JSON field audit, breadcrumb navigation, interactive file groups, E2E navigation tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…click Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s-only, edge types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ped schema migration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… category Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ype rendering Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Task 3: add 5 missing types to renderTestTypes (cache.modular, conditional.switch, conditional.expression, http.router, api.query); fix inheritance test to use api.query (has inheritFrom delegate field) and assert 'inherited from <name>' indicator; add delete button assertion in zero-configField test. Task 5: add tracking assertion (missingDefaults ≤ 58) to json fields defaultValue test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…conditional types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wire onNavigateToSource in multifile-groups scenario to record navigation target in data-last-navigation attribute, then assert it after clicking the root breadcrumb segment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…IDE integration 11 tasks across 4 repos: canonical DSL reference, wfctl dsl-reference command, step schema export, reference pane, json field audit, CI contract, schema sync, LSP hover/completion, VS Code + JetBrains reference commands. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Regenerated from wfctl editor-schemas: now includes 182 step schemas. Added getEngineStepTypes() export to load-schemas.ts. JSON field count reduced from 73 to 51 in the underlying schema. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…gineStepTypes tests
…s source MODULE_TYPE_MAP and MODULE_TYPES are now derived from getEngineModuleTypes() (engine-schemas.json) instead of a hand-maintained static array. The STATIC_MODULE_TYPE_MAP alias and the redundant static fallback merge in moduleSchemaStore are removed. Editor-only types (conditional.switch, conditional.expression) that have no engine counterpart are preserved as explicit entries so addNode() and serialization lookups continue to work. All 4026 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… in audit test Regenerated engine-schemas.json from the workflow engine after all 51 json config fields were converted to typed schemas. JSON field count is now 0. Audit test updated from a ratchet (≤60) to a hard zero-tolerance assertion. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ay nodes Cast onNodesChange to OnNodesChange<RFNode> since displayNodes array includes both WorkflowNode and plain Node (file group overlays). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The editor's multi-file config support only had one fixture set and no coverage for real-world split patterns (by domain, by layer, deeply nested). We also lacked a design for the YAML side-pane, visual file boundaries, and node-to-file navigation hooks needed for IDE plugin integration.
Design doc (
docs/plans/2026-03-26-multifile-config-validation-design.md)onNavigateToSource(filePath, line, col)signature andnavigateToNodereverse navigation for IDE bridgeyamlLineMapcovering pipelines, steps, workflows, triggers (not just modules)Implementation plan (
docs/plans/2026-03-26-multifile-config-validation-implementation.md)12 tasks across 6 phases — from fixtures through IDE bridge protocol updates.
Test fixtures (18 YAML files)
multifile-domain/multifile-layers/multifile-nested/Serialization tests (36 cases)
Validates
resolveImports→sourceMap→exportToFiles→configToNodesround-trip for all three split strategies. Covers cross-file isolation, nested path resolution, error recovery on missing files, and no-duplication invariants.All 649 tests pass (613 existing + 36 new).
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.