From 618d7c0baa3d270b4d0544cef36645ddcf3de06e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Oct 2025 16:44:49 +0000 Subject: [PATCH 1/3] Initial plan From 1c6ef774a9f9a51720a46c9908b8788da61263e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Oct 2025 16:53:16 +0000 Subject: [PATCH 2/3] Normalize include order in workflow compilation - Add alphabetical sorting of included files in compiler - Add test case to verify alphabetical ordering - Recompile workflows to apply fix to dev.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/dev.lock.yml | 2 +- pkg/workflow/compiler.go | 3 + pkg/workflow/manifest_test.go | 116 +++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dev.lock.yml b/.github/workflows/dev.lock.yml index 3cee6754470..f5ac8ed4e8f 100644 --- a/.github/workflows/dev.lock.yml +++ b/.github/workflows/dev.lock.yml @@ -5,8 +5,8 @@ # # Resolved workflow manifest: # Includes: -# - shared/use-emojis.md # - shared/keep-it-short.md +# - shared/use-emojis.md name: "Dev" on: diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 952b86afee0..5c1d40af708 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strings" "github.com/githubnext/gh-aw/pkg/console" @@ -626,6 +627,8 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) for file := range allIncludedFilesMap { allIncludedFiles = append(allIncludedFiles, file) } + // Sort files alphabetically to ensure consistent ordering in lock files + sort.Strings(allIncludedFiles) // Extract workflow name workflowName, err := parser.ExtractWorkflowNameFromMarkdown(markdownPath) diff --git a/pkg/workflow/manifest_test.go b/pkg/workflow/manifest_test.go index 3f2850fecd5..150caf6bcd9 100644 --- a/pkg/workflow/manifest_test.go +++ b/pkg/workflow/manifest_test.go @@ -188,3 +188,119 @@ Handle the issue.`, }) } } + +// TestManifestIncludeOrdering tests that included files are rendered in alphabetical order +func TestManifestIncludeOrdering(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "manifest-order-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + // Create shared directory + sharedDir := filepath.Join(tmpDir, "shared") + if err := os.Mkdir(sharedDir, 0755); err != nil { + t.Fatal(err) + } + + // Create multiple include files with names that would be out of order if not sorted + includeFiles := []string{ + "zebra.md", + "apple.md", + "middle.md", + "banana.md", + } + + for _, filename := range includeFiles { + content := "# " + filename + "\n\nSome content." + filePath := filepath.Join(sharedDir, filename) + if err := os.WriteFile(filePath, []byte(content), 0644); err != nil { + t.Fatal(err) + } + } + + // Create workflow that includes all files in non-alphabetical order + workflowContent := `--- +on: issues +engine: claude +--- + +# Test Workflow + +@include shared/zebra.md +@include shared/apple.md +@include shared/middle.md +@include shared/banana.md + +Handle the issue.` + + compiler := NewCompiler(false, "", "test") + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(workflowContent), 0644); err != nil { + t.Fatal(err) + } + + // Compile the workflow + err = compiler.CompileWorkflow(testFile) + if err != nil { + t.Fatalf("Unexpected error compiling workflow: %v", err) + } + + // Read the generated lock file + lockFile := strings.TrimSuffix(testFile, ".md") + ".lock.yml" + content, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read generated lock file: %v", err) + } + + lockContent := string(content) + + // Verify manifest section exists + if !strings.Contains(lockContent, "# Resolved workflow manifest:") { + t.Fatal("Expected manifest section but none found") + } + + // Verify includes section exists + if !strings.Contains(lockContent, "# Includes:") { + t.Fatal("Expected Includes section but none found") + } + + // Extract the includes section and verify alphabetical order + lines := strings.Split(lockContent, "\n") + var includeLines []string + inIncludesSection := false + + for _, line := range lines { + if strings.Contains(line, "# Includes:") { + inIncludesSection = true + continue + } + if inIncludesSection { + if strings.HasPrefix(line, "# - ") { + includeLines = append(includeLines, line) + } else if !strings.HasPrefix(line, "#") { + // End of includes section + break + } + } + } + + // Verify we found all includes + if len(includeLines) != 4 { + t.Fatalf("Expected 4 include lines, found %d", len(includeLines)) + } + + // Expected order is alphabetical + expectedOrder := []string{ + "# - shared/apple.md", + "# - shared/banana.md", + "# - shared/middle.md", + "# - shared/zebra.md", + } + + for i, expected := range expectedOrder { + if includeLines[i] != expected { + t.Errorf("Include line %d: expected %q, got %q", i, expected, includeLines[i]) + } + } +} From f3ecf50e4b0284036d6372bda114dc3bf6b38316 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Oct 2025 17:00:14 +0000 Subject: [PATCH 3/3] Improve test maintainability by using variable instead of magic number Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/manifest_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/manifest_test.go b/pkg/workflow/manifest_test.go index 150caf6bcd9..8a1032a7254 100644 --- a/pkg/workflow/manifest_test.go +++ b/pkg/workflow/manifest_test.go @@ -286,8 +286,9 @@ Handle the issue.` } // Verify we found all includes - if len(includeLines) != 4 { - t.Fatalf("Expected 4 include lines, found %d", len(includeLines)) + expectedCount := len(includeFiles) + if len(includeLines) != expectedCount { + t.Fatalf("Expected %d include lines, found %d", expectedCount, len(includeLines)) } // Expected order is alphabetical