Skip to content

Commit 1db7b9b

Browse files
CopilotCopilot
authored
fix: propagate yaml.Unmarshal errors in workflow_builder.go (5 silent sites) (#41577)
1 parent d5a2522 commit 1db7b9b

2 files changed

Lines changed: 172 additions & 117 deletions

File tree

‎pkg/workflow/compiler_orchestrator_workflow_test.go‎

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1807,9 +1807,9 @@ func TestMergeJobsFromYAMLImports_PreservesJobOrder(t *testing.T) {
18071807
assert.Contains(t, result, "job-d")
18081808
}
18091809

1810-
// TestProcessAndMergeSteps_InvalidYAML tests handling of invalid YAML in imported steps
1811-
func TestProcessAndMergeSteps_InvalidYAML(t *testing.T) {
1812-
tmpDir := testutil.TempDir(t, "invalid-steps-yaml")
1810+
// TestProcessAndMergeSteps_InvalidYAML_MergedSteps tests that malformed MergedSteps YAML returns an error
1811+
func TestProcessAndMergeSteps_InvalidYAML_MergedSteps(t *testing.T) {
1812+
tmpDir := testutil.TempDir(t, "invalid-merged-steps-yaml")
18131813
compiler := NewCompiler()
18141814
actionCache := NewActionCache(tmpDir)
18151815
actionResolver := NewActionResolver(actionCache)
@@ -1829,12 +1829,73 @@ func TestProcessAndMergeSteps_InvalidYAML(t *testing.T) {
18291829
MergedSteps: "invalid: [yaml",
18301830
}
18311831

1832-
// Should handle gracefully without panicking
1833-
require.NoError(t, compiler.processAndMergeSteps(frontmatter, workflowData, importsResult))
1832+
// Malformed MergedSteps YAML must propagate as an error
1833+
err := compiler.processAndMergeSteps(frontmatter, workflowData, importsResult)
1834+
require.Error(t, err, "malformed MergedSteps YAML should return an error")
1835+
assert.Contains(t, err.Error(), "failed to parse imported steps")
1836+
}
18341837

1835-
// Should still have main steps
1836-
assert.NotEmpty(t, workflowData.CustomSteps)
1837-
assert.Contains(t, workflowData.CustomSteps, "Main")
1838+
// TestProcessAndMergePreSteps_InvalidYAML tests that malformed imported pre-steps YAML returns an error
1839+
func TestProcessAndMergePreSteps_InvalidYAML(t *testing.T) {
1840+
tmpDir := testutil.TempDir(t, "invalid-pre-steps-yaml")
1841+
compiler := NewCompiler()
1842+
actionCache := NewActionCache(tmpDir)
1843+
actionResolver := NewActionResolver(actionCache)
1844+
workflowData := &WorkflowData{
1845+
ActionCache: actionCache,
1846+
ActionResolver: actionResolver,
1847+
}
1848+
1849+
frontmatter := map[string]any{}
1850+
importsResult := &parser.ImportsResult{
1851+
MergedPreSteps: "invalid: [yaml",
1852+
}
1853+
1854+
err := compiler.processAndMergePreSteps(frontmatter, workflowData, importsResult)
1855+
require.Error(t, err, "malformed imported pre-steps YAML should return an error")
1856+
assert.Contains(t, err.Error(), "failed to parse imported pre-steps")
1857+
}
1858+
1859+
// TestProcessAndMergePreAgentSteps_InvalidYAML tests that malformed imported pre-agent-steps YAML returns an error
1860+
func TestProcessAndMergePreAgentSteps_InvalidYAML(t *testing.T) {
1861+
tmpDir := testutil.TempDir(t, "invalid-pre-agent-steps-yaml")
1862+
compiler := NewCompiler()
1863+
actionCache := NewActionCache(tmpDir)
1864+
actionResolver := NewActionResolver(actionCache)
1865+
workflowData := &WorkflowData{
1866+
ActionCache: actionCache,
1867+
ActionResolver: actionResolver,
1868+
}
1869+
1870+
frontmatter := map[string]any{}
1871+
importsResult := &parser.ImportsResult{
1872+
MergedPreAgentSteps: "invalid: [yaml",
1873+
}
1874+
1875+
err := compiler.processAndMergePreAgentSteps(frontmatter, workflowData, importsResult)
1876+
require.Error(t, err, "malformed imported pre-agent-steps YAML should return an error")
1877+
assert.Contains(t, err.Error(), "failed to parse imported pre-agent-steps")
1878+
}
1879+
1880+
// TestProcessAndMergePostSteps_InvalidYAML tests that malformed imported post-steps YAML returns an error
1881+
func TestProcessAndMergePostSteps_InvalidYAML(t *testing.T) {
1882+
tmpDir := testutil.TempDir(t, "invalid-post-steps-yaml")
1883+
compiler := NewCompiler()
1884+
actionCache := NewActionCache(tmpDir)
1885+
actionResolver := NewActionResolver(actionCache)
1886+
workflowData := &WorkflowData{
1887+
ActionCache: actionCache,
1888+
ActionResolver: actionResolver,
1889+
}
1890+
1891+
frontmatter := map[string]any{}
1892+
importsResult := &parser.ImportsResult{
1893+
MergedPostSteps: "invalid: [yaml",
1894+
}
1895+
1896+
err := compiler.processAndMergePostSteps(frontmatter, workflowData, importsResult)
1897+
require.Error(t, err, "malformed imported post-steps YAML should return an error")
1898+
assert.Contains(t, err.Error(), "failed to parse imported post-steps")
18381899
}
18391900

18401901
// TestProcessAndMergeServices_EmptyImportedServices tests handling of empty imported services

‎pkg/workflow/workflow_builder.go‎

Lines changed: 103 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -413,45 +413,45 @@ func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData
413413
// Parse other imported steps if present (these go after copilot-setup but before main steps)
414414
var otherImportedSteps []any
415415
if importsResult.MergedSteps != "" {
416-
if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &otherImportedSteps); err == nil {
417-
// Convert to typed steps for action pinning
418-
typedOtherSteps, err := SliceToSteps(otherImportedSteps)
419-
if err != nil {
420-
workflowBuilderLog.Printf("Failed to convert other imported steps to typed steps: %v", err)
421-
} else {
422-
// Apply action pinning to other imported steps
423-
typedOtherSteps, err = applyActionPinsToTypedSteps(typedOtherSteps, workflowData)
424-
if err != nil {
425-
return fmt.Errorf("imported steps: %w", err)
426-
}
427-
// Convert back to []any for YAML marshaling
428-
otherImportedSteps = StepsToSlice(typedOtherSteps)
429-
}
416+
if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &otherImportedSteps); err != nil {
417+
return fmt.Errorf("failed to parse imported steps: %w", err)
430418
}
419+
// Convert to typed steps for action pinning
420+
typedOtherSteps, err := SliceToSteps(otherImportedSteps)
421+
if err != nil {
422+
return fmt.Errorf("failed to convert imported steps: %w", err)
423+
}
424+
// Apply action pinning to other imported steps
425+
typedOtherSteps, err = applyActionPinsToTypedSteps(typedOtherSteps, workflowData)
426+
if err != nil {
427+
return fmt.Errorf("imported steps: %w", err)
428+
}
429+
// Convert back to []any for YAML marshaling
430+
otherImportedSteps = StepsToSlice(typedOtherSteps)
431431
}
432432

433433
// If there are main workflow steps, parse them
434434
var mainSteps []any
435435
if workflowData.CustomSteps != "" {
436436
var mainStepsWrapper map[string]any
437-
if err := yaml.Unmarshal([]byte(workflowData.CustomSteps), &mainStepsWrapper); err == nil {
438-
if mainStepsVal, hasSteps := mainStepsWrapper["steps"]; hasSteps {
439-
if steps, ok := mainStepsVal.([]any); ok {
440-
mainSteps = steps
441-
// Convert to typed steps for action pinning
442-
typedMainSteps, err := SliceToSteps(mainSteps)
443-
if err != nil {
444-
workflowBuilderLog.Printf("Failed to convert main steps to typed steps: %v", err)
445-
} else {
446-
// Apply action pinning to main steps
447-
typedMainSteps, err = applyActionPinsToTypedSteps(typedMainSteps, workflowData)
448-
if err != nil {
449-
return fmt.Errorf("steps: %w", err)
450-
}
451-
// Convert back to []any for YAML marshaling
452-
mainSteps = StepsToSlice(typedMainSteps)
453-
}
437+
if err := yaml.Unmarshal([]byte(workflowData.CustomSteps), &mainStepsWrapper); err != nil {
438+
return fmt.Errorf("failed to parse custom steps: %w", err)
439+
}
440+
if mainStepsVal, hasSteps := mainStepsWrapper["steps"]; hasSteps {
441+
if steps, ok := mainStepsVal.([]any); ok {
442+
mainSteps = steps
443+
// Convert to typed steps for action pinning
444+
typedMainSteps, err := SliceToSteps(mainSteps)
445+
if err != nil {
446+
return fmt.Errorf("failed to convert main steps: %w", err)
447+
}
448+
// Apply action pinning to main steps
449+
typedMainSteps, err = applyActionPinsToTypedSteps(typedMainSteps, workflowData)
450+
if err != nil {
451+
return fmt.Errorf("steps: %w", err)
454452
}
453+
// Convert back to []any for YAML marshaling
454+
mainSteps = StepsToSlice(typedMainSteps)
455455
}
456456
}
457457
}
@@ -491,40 +491,38 @@ func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowD
491491
var importedPreSteps []any
492492
if importsResult.MergedPreSteps != "" {
493493
if err := yaml.Unmarshal([]byte(importsResult.MergedPreSteps), &importedPreSteps); err != nil {
494-
workflowBuilderLog.Printf("Failed to unmarshal imported pre-steps: %v", err)
495-
} else {
496-
typedImported, err := SliceToSteps(importedPreSteps)
497-
if err != nil {
498-
workflowBuilderLog.Printf("Failed to convert imported pre-steps to typed steps: %v", err)
499-
} else {
500-
typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData)
501-
if err != nil {
502-
return fmt.Errorf("imported pre-steps: %w", err)
503-
}
504-
importedPreSteps = StepsToSlice(typedImported)
505-
}
494+
return fmt.Errorf("failed to parse imported pre-steps: %w", err)
495+
}
496+
typedImported, err := SliceToSteps(importedPreSteps)
497+
if err != nil {
498+
return fmt.Errorf("failed to convert imported pre-steps: %w", err)
499+
}
500+
typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData)
501+
if err != nil {
502+
return fmt.Errorf("imported pre-steps: %w", err)
506503
}
504+
importedPreSteps = StepsToSlice(typedImported)
507505
}
508506

509507
// Parse main workflow pre-steps if present
510508
var mainPreSteps []any
511509
if mainPreStepsYAML != "" {
512510
var mainWrapper map[string]any
513-
if err := yaml.Unmarshal([]byte(mainPreStepsYAML), &mainWrapper); err == nil {
514-
if mainVal, ok := mainWrapper["pre-steps"]; ok {
515-
if steps, ok := mainVal.([]any); ok {
516-
mainPreSteps = steps
517-
typedMain, err := SliceToSteps(mainPreSteps)
518-
if err != nil {
519-
workflowBuilderLog.Printf("Failed to convert main pre-steps to typed steps: %v", err)
520-
} else {
521-
typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData)
522-
if err != nil {
523-
return fmt.Errorf("pre-steps: %w", err)
524-
}
525-
mainPreSteps = StepsToSlice(typedMain)
526-
}
511+
if err := yaml.Unmarshal([]byte(mainPreStepsYAML), &mainWrapper); err != nil {
512+
return fmt.Errorf("failed to parse pre-steps: %w", err)
513+
}
514+
if mainVal, ok := mainWrapper["pre-steps"]; ok {
515+
if steps, ok := mainVal.([]any); ok {
516+
mainPreSteps = steps
517+
typedMain, err := SliceToSteps(mainPreSteps)
518+
if err != nil {
519+
return fmt.Errorf("failed to convert pre-steps: %w", err)
520+
}
521+
typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData)
522+
if err != nil {
523+
return fmt.Errorf("pre-steps: %w", err)
527524
}
525+
mainPreSteps = StepsToSlice(typedMain)
528526
}
529527
}
530528
}
@@ -554,39 +552,37 @@ func (c *Compiler) processAndMergePreAgentSteps(frontmatter map[string]any, work
554552
var importedPreAgentSteps []any
555553
if importsResult.MergedPreAgentSteps != "" {
556554
if err := yaml.Unmarshal([]byte(importsResult.MergedPreAgentSteps), &importedPreAgentSteps); err != nil {
557-
workflowBuilderLog.Printf("Failed to unmarshal imported pre-agent-steps: %v", err)
558-
} else {
559-
typedImported, err := SliceToSteps(importedPreAgentSteps)
560-
if err != nil {
561-
workflowBuilderLog.Printf("Failed to convert imported pre-agent-steps to typed steps: %v", err)
562-
} else {
563-
typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData)
564-
if err != nil {
565-
return fmt.Errorf("imported pre-agent-steps: %w", err)
566-
}
567-
importedPreAgentSteps = StepsToSlice(typedImported)
568-
}
555+
return fmt.Errorf("failed to parse imported pre-agent-steps: %w", err)
569556
}
557+
typedImported, err := SliceToSteps(importedPreAgentSteps)
558+
if err != nil {
559+
return fmt.Errorf("failed to convert imported pre-agent-steps: %w", err)
560+
}
561+
typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData)
562+
if err != nil {
563+
return fmt.Errorf("imported pre-agent-steps: %w", err)
564+
}
565+
importedPreAgentSteps = StepsToSlice(typedImported)
570566
}
571567

572568
var mainPreAgentSteps []any
573569
if mainPreAgentStepsYAML != "" {
574570
var mainWrapper map[string]any
575-
if err := yaml.Unmarshal([]byte(mainPreAgentStepsYAML), &mainWrapper); err == nil {
576-
if mainVal, ok := mainWrapper["pre-agent-steps"]; ok {
577-
if steps, ok := mainVal.([]any); ok {
578-
mainPreAgentSteps = steps
579-
typedMain, err := SliceToSteps(mainPreAgentSteps)
580-
if err != nil {
581-
workflowBuilderLog.Printf("Failed to convert main pre-agent-steps to typed steps: %v", err)
582-
} else {
583-
typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData)
584-
if err != nil {
585-
return fmt.Errorf("pre-agent-steps: %w", err)
586-
}
587-
mainPreAgentSteps = StepsToSlice(typedMain)
588-
}
571+
if err := yaml.Unmarshal([]byte(mainPreAgentStepsYAML), &mainWrapper); err != nil {
572+
return fmt.Errorf("failed to parse pre-agent-steps: %w", err)
573+
}
574+
if mainVal, ok := mainWrapper["pre-agent-steps"]; ok {
575+
if steps, ok := mainVal.([]any); ok {
576+
mainPreAgentSteps = steps
577+
typedMain, err := SliceToSteps(mainPreAgentSteps)
578+
if err != nil {
579+
return fmt.Errorf("failed to convert pre-agent-steps: %w", err)
580+
}
581+
typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData)
582+
if err != nil {
583+
return fmt.Errorf("pre-agent-steps: %w", err)
589584
}
585+
mainPreAgentSteps = StepsToSlice(typedMain)
590586
}
591587
}
592588
}
@@ -616,40 +612,38 @@ func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflow
616612
var importedPostSteps []any
617613
if importsResult.MergedPostSteps != "" {
618614
if err := yaml.Unmarshal([]byte(importsResult.MergedPostSteps), &importedPostSteps); err != nil {
619-
workflowBuilderLog.Printf("Failed to unmarshal imported post-steps: %v", err)
620-
} else {
621-
typedImported, err := SliceToSteps(importedPostSteps)
622-
if err != nil {
623-
workflowBuilderLog.Printf("Failed to convert imported post-steps to typed steps: %v", err)
624-
} else {
625-
typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData)
626-
if err != nil {
627-
return fmt.Errorf("imported post-steps: %w", err)
628-
}
629-
importedPostSteps = StepsToSlice(typedImported)
630-
}
615+
return fmt.Errorf("failed to parse imported post-steps: %w", err)
631616
}
617+
typedImported, err := SliceToSteps(importedPostSteps)
618+
if err != nil {
619+
return fmt.Errorf("failed to convert imported post-steps: %w", err)
620+
}
621+
typedImported, err = applyActionPinsToTypedSteps(typedImported, workflowData)
622+
if err != nil {
623+
return fmt.Errorf("imported post-steps: %w", err)
624+
}
625+
importedPostSteps = StepsToSlice(typedImported)
632626
}
633627

634628
// Parse main workflow post-steps if present
635629
var mainPostSteps []any
636630
if mainPostStepsYAML != "" {
637631
var mainWrapper map[string]any
638-
if err := yaml.Unmarshal([]byte(mainPostStepsYAML), &mainWrapper); err == nil {
639-
if mainVal, ok := mainWrapper["post-steps"]; ok {
640-
if steps, ok := mainVal.([]any); ok {
641-
mainPostSteps = steps
642-
typedMain, err := SliceToSteps(mainPostSteps)
643-
if err != nil {
644-
workflowBuilderLog.Printf("Failed to convert main post-steps to typed steps: %v", err)
645-
} else {
646-
typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData)
647-
if err != nil {
648-
return fmt.Errorf("post-steps: %w", err)
649-
}
650-
mainPostSteps = StepsToSlice(typedMain)
651-
}
632+
if err := yaml.Unmarshal([]byte(mainPostStepsYAML), &mainWrapper); err != nil {
633+
return fmt.Errorf("failed to parse post-steps: %w", err)
634+
}
635+
if mainVal, ok := mainWrapper["post-steps"]; ok {
636+
if steps, ok := mainVal.([]any); ok {
637+
mainPostSteps = steps
638+
typedMain, err := SliceToSteps(mainPostSteps)
639+
if err != nil {
640+
return fmt.Errorf("failed to convert post-steps: %w", err)
641+
}
642+
typedMain, err = applyActionPinsToTypedSteps(typedMain, workflowData)
643+
if err != nil {
644+
return fmt.Errorf("post-steps: %w", err)
652645
}
646+
mainPostSteps = StepsToSlice(typedMain)
653647
}
654648
}
655649
}

0 commit comments

Comments
 (0)