Normalize file paths to Unix format in compiled .lock.yml files#13853
Normalize file paths to Unix format in compiled .lock.yml files#13853
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot recompile |
|
@copilot make sure the paths passed in runtime-import is using ToSlash. |
Recompilation completed successfully. All 145 workflows compiled with no changes to existing |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added |
There was a problem hiding this comment.
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestCompilerGeneratesUnixPaths(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| markdownContent string | ||
| expectedImportPaths []string | ||
| expectedIncludePaths []string | ||
| expectedSourcePath string | ||
| }{ | ||
| { | ||
| name: "imports with forward slashes", | ||
| markdownContent: `--- | ||
| on: issues | ||
| imports: | ||
| - shared/common.md | ||
| - shared/reporting.md | ||
| source: workflows/test-workflow.md | ||
| --- | ||
|
|
||
| # Test Workflow | ||
|
|
||
| This is a test workflow with imports.`, | ||
| expectedImportPaths: []string{ | ||
| "shared/common.md", | ||
| "shared/reporting.md", | ||
| }, | ||
| expectedSourcePath: "workflows/test-workflow.md", | ||
| }, | ||
| { | ||
| name: "includes with forward slashes", | ||
| markdownContent: `--- | ||
| on: pull_request | ||
| --- | ||
|
|
||
| # Test Include Workflow | ||
|
|
||
| {{#import shared/tools.md}} | ||
|
|
||
| This workflow includes external tools.`, | ||
| expectedIncludePaths: []string{ | ||
| "shared/tools.md", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Create temporary directory for test | ||
| tmpDir := t.TempDir() | ||
|
|
||
| // Create shared directory and files for imports/includes | ||
| sharedDir := filepath.Join(tmpDir, "shared") | ||
| err := os.MkdirAll(sharedDir, 0755) | ||
| require.NoError(t, err, "Failed to create shared directory") | ||
|
|
||
| // Create shared/common.md (shared workflow - minimal valid content) | ||
| commonContent := `# Common Shared Workflow | ||
|
|
||
| This is a shared workflow.` | ||
| commonFile := filepath.Join(sharedDir, "common.md") | ||
| err = os.WriteFile(commonFile, []byte(commonContent), 0644) | ||
| require.NoError(t, err, "Failed to create common.md") | ||
|
|
||
| // Create shared/reporting.md (shared workflow - minimal valid content) | ||
| reportingContent := `# Reporting Shared Workflow | ||
|
|
||
| This is a shared workflow.` | ||
| reportingFile := filepath.Join(sharedDir, "reporting.md") | ||
| err = os.WriteFile(reportingFile, []byte(reportingContent), 0644) | ||
| require.NoError(t, err, "Failed to create reporting.md") | ||
|
|
||
| // Create shared/tools.md (shared workflow - minimal valid content) | ||
| toolsContent := `# Tools Shared Workflow | ||
|
|
||
| This is a shared workflow.` | ||
| toolsFile := filepath.Join(sharedDir, "tools.md") | ||
| err = os.WriteFile(toolsFile, []byte(toolsContent), 0644) | ||
| require.NoError(t, err, "Failed to create tools.md") | ||
|
|
||
| // Create workflows directory for source path | ||
| workflowsDir := filepath.Join(tmpDir, "workflows") | ||
| err = os.MkdirAll(workflowsDir, 0755) | ||
| require.NoError(t, err, "Failed to create workflows directory") | ||
|
|
||
| // Write markdown file | ||
| markdownPath := filepath.Join(tmpDir, "test-workflow.md") | ||
| err = os.WriteFile(markdownPath, []byte(tt.markdownContent), 0644) | ||
| require.NoError(t, err, "Failed to write markdown file") | ||
|
|
||
| // Compile the workflow | ||
| compiler := NewCompiler() | ||
| err = compiler.CompileWorkflow(markdownPath) | ||
| require.NoError(t, err, "Compilation should succeed") | ||
|
|
||
| // Read the generated .lock.yml file | ||
| lockFile := filepath.Join(tmpDir, "test-workflow.lock.yml") | ||
| lockContent, err := os.ReadFile(lockFile) | ||
| require.NoError(t, err, "Failed to read lock file") | ||
|
|
||
| lockYAML := string(lockContent) | ||
|
|
||
| // Verify that file paths in the manifest use forward slashes (Unix-compatible) | ||
| // Note: The ASCII art header contains backslashes, so we only check the manifest section | ||
| manifestStart := strings.Index(lockYAML, "# Resolved workflow manifest:") | ||
| sourceStart := strings.Index(lockYAML, "# Source:") | ||
|
|
||
| // Verify expected import paths are present with forward slashes | ||
| for _, importPath := range tt.expectedImportPaths { | ||
| expectedLine := "# - " + importPath | ||
| assert.Contains(t, lockYAML, expectedLine, "Lock file should contain import path: %s", importPath) | ||
|
|
||
| // Ensure no backslash version exists | ||
| backslashPath := strings.ReplaceAll(importPath, "/", "\\") | ||
| backslashLine := "# - " + backslashPath | ||
| assert.NotContains(t, lockYAML, backslashLine, "Lock file should not contain backslash version of: %s", importPath) | ||
| } | ||
|
|
||
| // Verify expected include paths are present with forward slashes | ||
| for _, includePath := range tt.expectedIncludePaths { | ||
| expectedLine := "# - " + includePath | ||
| assert.Contains(t, lockYAML, expectedLine, "Lock file should contain include path: %s", includePath) | ||
|
|
||
| // Ensure no backslash version exists | ||
| backslashPath := strings.ReplaceAll(includePath, "/", "\\") | ||
| backslashLine := "# - " + backslashPath | ||
| assert.NotContains(t, lockYAML, backslashLine, "Lock file should not contain backslash version of: %s", includePath) | ||
| } | ||
|
|
||
| // Verify source path uses forward slashes | ||
| if tt.expectedSourcePath != "" { | ||
| expectedLine := "# Source: " + tt.expectedSourcePath | ||
| assert.Contains(t, lockYAML, expectedLine, "Lock file should contain source path: %s", tt.expectedSourcePath) | ||
|
|
||
| // Ensure no backslash version exists | ||
| backslashPath := strings.ReplaceAll(tt.expectedSourcePath, "/", "\\") | ||
| backslashLine := "# Source: " + backslashPath | ||
| assert.NotContains(t, lockYAML, backslashLine, "Lock file should not contain backslash version of source: %s", tt.expectedSourcePath) | ||
| } | ||
|
|
||
| // Verify that manifest section does not contain backslashes in file paths | ||
| if manifestStart >= 0 { | ||
| manifestEnd := strings.Index(lockYAML[manifestStart:], "\n\n") | ||
| if manifestEnd >= 0 { | ||
| manifest := lockYAML[manifestStart : manifestStart+manifestEnd] | ||
| assert.NotContains(t, manifest, "\\", "Lock file manifest should not contain backslashes in file paths") | ||
| } | ||
| } | ||
|
|
||
| // Verify that source section does not contain backslashes in file paths | ||
| if sourceStart >= 0 && tt.expectedSourcePath != "" { | ||
| sourceEnd := strings.Index(lockYAML[sourceStart:], "\n") | ||
| if sourceEnd >= 0 { | ||
| sourceLine := lockYAML[sourceStart : sourceStart+sourceEnd] | ||
| // Check that the source line doesn't contain a Windows path | ||
| backslashPath := strings.ReplaceAll(tt.expectedSourcePath, "/", "\\") | ||
| assert.NotContains(t, sourceLine, backslashPath, "Source line should not contain Windows-style path") | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
The test suite doesn't verify the runtime-import path normalization. Consider adding a test case that checks the generated runtime-import macro path in the lock file contains forward slashes. For example, verify that a workflow file at workflows\test.md on Windows generates {{#runtime-import workflows/test.md}} instead of {{#runtime-import workflows\test.md}}.
The compiler generates platform-specific paths in
.lock.ymlfiles. On Windows, this produces backslash separators which breaks cross-platform workflows.Changes
Applied
filepath.ToSlash()normalization at all YAML output points:pkg/workflow/compiler_yaml.go: NormalizeSource,ImportedFiles,IncludedFiles, andruntime-importpaths before writing to YAML header and promptpkg/parser/include_expander.go: Normalize paths when building the included files manifestExample
Before (Windows):
After (all platforms):
Added test coverage in
compiler_path_normalization_test.gofor imports, includes, source paths, and nested directory structures.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.