diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index 40090f9305..6ae6f632a0 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -317,6 +317,7 @@ func extractActionVersion(uses string) string { // ApplyActionPinsToSteps applies SHA pinning to a slice of step maps // Returns a new slice with pinned references +// Deprecated: Use ApplyActionPinsToWorkflowSteps for better type safety func ApplyActionPinsToSteps(steps []any, data *WorkflowData) []any { result := make([]any, len(steps)) for i, step := range steps { @@ -329,6 +330,24 @@ func ApplyActionPinsToSteps(steps []any, data *WorkflowData) []any { return result } +// ApplyActionPinsToWorkflowSteps applies SHA pinning to a slice of WorkflowStep +// Returns a new slice with pinned references +func ApplyActionPinsToWorkflowSteps(steps []WorkflowStep, data *WorkflowData) []WorkflowStep { + result := make([]WorkflowStep, len(steps)) + for i, step := range steps { + stepMap := step.ToMap() + pinnedMap := ApplyActionPinToStep(stepMap, data) + pinnedStep, err := MapToStep(pinnedMap) + if err != nil { + // If conversion fails, keep the original step + result[i] = step + } else { + result[i] = *pinnedStep + } + } + return result +} + // GetActionPinByRepo returns the ActionPin for a given repository, if it exists // When multiple versions exist for the same repo, it returns the latest version by semver func GetActionPinByRepo(repo string) (ActionPin, bool) { diff --git a/pkg/workflow/action_pins_test.go b/pkg/workflow/action_pins_test.go index 534ebe5a0b..107f98dc3c 100644 --- a/pkg/workflow/action_pins_test.go +++ b/pkg/workflow/action_pins_test.go @@ -719,3 +719,55 @@ func TestGetActionPinWithData_SemverPreference(t *testing.T) { }) } } + +// TestApplyActionPinsToWorkflowSteps tests the ApplyActionPinsToWorkflowSteps function +func TestApplyActionPinsToWorkflowSteps(t *testing.T) { + steps := []WorkflowStep{ + { + Name: "Checkout code", + Uses: "actions/checkout@v4", + }, + { + Name: "Setup Node", + Uses: "actions/setup-node@v4", + }, + { + Name: "Run script", + Run: "npm test", + }, + } + + data := &WorkflowData{} + result := ApplyActionPinsToWorkflowSteps(steps, data) + + // Verify result has same length + if len(result) != len(steps) { + t.Errorf("ApplyActionPinsToWorkflowSteps() returned %d steps, want %d", len(result), len(steps)) + } + + // Verify first step is pinned + if !strings.Contains(result[0].Uses, "@") { + t.Errorf("First step not pinned: %s", result[0].Uses) + } + if !strings.HasPrefix(result[0].Uses, "actions/checkout@") { + t.Errorf("First step action changed: %s", result[0].Uses) + } + + // Verify second step is pinned + if !strings.Contains(result[1].Uses, "@") { + t.Errorf("Second step not pinned: %s", result[1].Uses) + } + if !strings.HasPrefix(result[1].Uses, "actions/setup-node@") { + t.Errorf("Second step action changed: %s", result[1].Uses) + } + + // Verify third step (run step) is unchanged + if result[2].Run != "npm test" { + t.Errorf("Third step run changed: %s", result[2].Run) + } + + // Verify original steps are not modified + if steps[0].Uses != "actions/checkout@v4" { + t.Errorf("Original step was modified: %s", steps[0].Uses) + } +} diff --git a/pkg/workflow/runtime_setup.go b/pkg/workflow/runtime_setup.go index cceb77cc78..9e34809cfb 100644 --- a/pkg/workflow/runtime_setup.go +++ b/pkg/workflow/runtime_setup.go @@ -397,6 +397,7 @@ func detectSerenaLanguages(serenaConfig *SerenaToolConfig, requirements map[stri } // detectFromEngineSteps scans engine steps for runtime commands +// Deprecated: Use detectFromWorkflowSteps for better type safety func detectFromEngineSteps(steps []map[string]any, requirements map[string]*RuntimeRequirement) { for _, step := range steps { if run, hasRun := step["run"]; hasRun { @@ -407,6 +408,15 @@ func detectFromEngineSteps(steps []map[string]any, requirements map[string]*Runt } } +// detectFromWorkflowSteps scans WorkflowStep slice for runtime commands +func detectFromWorkflowSteps(steps []WorkflowStep, requirements map[string]*RuntimeRequirement) { + for _, step := range steps { + if step.Run != "" { + detectRuntimeFromCommand(step.Run, requirements) + } + } +} + // filterExistingSetupActions removes runtimes from requirements if they already have setup actions in the custom steps // updateRequiredRuntime updates the version requirement, choosing the highest version func updateRequiredRuntime(runtime *Runtime, newVersion string, requirements map[string]*RuntimeRequirement) { diff --git a/pkg/workflow/runtime_setup_test.go b/pkg/workflow/runtime_setup_test.go index 55f06a67f5..9624eb2d7a 100644 --- a/pkg/workflow/runtime_setup_test.go +++ b/pkg/workflow/runtime_setup_test.go @@ -709,3 +709,91 @@ func TestDeduplicateErrorMessageFormat(t *testing.T) { len(errMsg), errMsg) } } + +// TestDetectFromWorkflowSteps tests the detectFromWorkflowSteps function +func TestDetectFromWorkflowSteps(t *testing.T) { + tests := []struct { + name string + steps []WorkflowStep + expected []string // Expected runtime IDs to be detected + }{ + { + name: "detect node from npm command", + steps: []WorkflowStep{ + {Name: "Install", Run: "npm install"}, + {Name: "Test", Run: "npm test"}, + }, + expected: []string{"node"}, + }, + { + name: "detect python from pip command", + steps: []WorkflowStep{ + {Name: "Install", Run: "pip install -r requirements.txt"}, + }, + expected: []string{"python"}, + }, + { + name: "detect go from go command", + steps: []WorkflowStep{ + {Name: "Build", Run: "go build"}, + }, + expected: []string{"go"}, + }, + { + name: "detect multiple runtimes", + steps: []WorkflowStep{ + {Name: "Node", Run: "npm install"}, + {Name: "Python", Run: "python script.py"}, + }, + expected: []string{"node", "python"}, + }, + { + name: "no runtimes detected", + steps: []WorkflowStep{ + {Name: "Echo", Run: "echo hello"}, + {Name: "Checkout", Uses: "actions/checkout@v4"}, + }, + expected: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + requirements := make(map[string]*RuntimeRequirement) + detectFromWorkflowSteps(tt.steps, requirements) + + // Check that expected runtimes were detected + for _, expectedID := range tt.expected { + if _, found := requirements[expectedID]; !found { + t.Errorf("Expected runtime %s to be detected, but it wasn't", expectedID) + } + } + + // Check that no unexpected runtimes were detected + if len(tt.expected) != len(requirements) { + var detected []string + for id := range requirements { + detected = append(detected, id) + } + t.Errorf("Expected %d runtimes %v, got %d runtimes %v", + len(tt.expected), tt.expected, len(requirements), detected) + } + }) + } +} + +// TestDetectFromEngineSteps_BackwardCompatibility tests that the old function still works +func TestDetectFromEngineSteps_BackwardCompatibility(t *testing.T) { + steps := []map[string]any{ + {"name": "Install", "run": "npm install"}, + {"name": "Test", "run": "npm test"}, + } + + requirements := make(map[string]*RuntimeRequirement) + detectFromEngineSteps(steps, requirements) + + // Check that node was detected + if _, found := requirements["node"]; !found { + t.Error("Expected node runtime to be detected") + } +} diff --git a/pkg/workflow/step_types.go b/pkg/workflow/step_types.go index dcd4f2ba3c..65be71f216 100644 --- a/pkg/workflow/step_types.go +++ b/pkg/workflow/step_types.go @@ -181,3 +181,222 @@ func (s *WorkflowStep) ToYAML() (string, error) { stepTypesLog.Printf("Successfully converted step to YAML: size=%d bytes", len(yamlBytes)) return string(yamlBytes), nil } + +// WorkflowJob represents a GitHub Actions job configuration from frontmatter +// This is different from the internal Job type used by the compiler +type WorkflowJob struct { + Name string `yaml:"name,omitempty"` + RunsOn any `yaml:"runs-on,omitempty"` // Can be string or array + Needs []string `yaml:"needs,omitempty"` // Job dependencies + If string `yaml:"if,omitempty"` // Conditional expression + Steps []WorkflowStep `yaml:"steps,omitempty"` // Job steps + Permissions map[string]string `yaml:"permissions,omitempty"` // Job-level permissions + Environment any `yaml:"environment,omitempty"` // Can be string or map + Concurrency any `yaml:"concurrency,omitempty"` // Can be string or map + TimeoutMinutes int `yaml:"timeout-minutes,omitempty"` + Container any `yaml:"container,omitempty"` // Can be string or map + Services map[string]any `yaml:"services,omitempty"` // Service containers + Env map[string]string `yaml:"env,omitempty"` // Environment variables + Outputs map[string]string `yaml:"outputs,omitempty"` // Job outputs + Strategy map[string]any `yaml:"strategy,omitempty"` // Matrix strategy + ContinueOnError bool `yaml:"continue-on-error,omitempty"` + + // Reusable workflow fields + Uses string `yaml:"uses,omitempty"` // Path to reusable workflow + With map[string]any `yaml:"with,omitempty"` // Inputs for reusable workflow + Secrets any `yaml:"secrets,omitempty"` // Can be "inherit" or map[string]string +} + +// ToMap converts a WorkflowJob to a map[string]any for YAML generation +func (j *WorkflowJob) ToMap() map[string]any { + result := make(map[string]any) + + if j.Name != "" { + result["name"] = j.Name + } + if j.RunsOn != nil { + result["runs-on"] = j.RunsOn + } + if len(j.Needs) > 0 { + result["needs"] = j.Needs + } + if j.If != "" { + result["if"] = j.If + } + if len(j.Steps) > 0 { + steps := make([]map[string]any, len(j.Steps)) + for i, step := range j.Steps { + steps[i] = step.ToMap() + } + result["steps"] = steps + } + if len(j.Permissions) > 0 { + result["permissions"] = j.Permissions + } + if j.Environment != nil { + result["environment"] = j.Environment + } + if j.Concurrency != nil { + result["concurrency"] = j.Concurrency + } + if j.TimeoutMinutes > 0 { + result["timeout-minutes"] = j.TimeoutMinutes + } + if j.Container != nil { + result["container"] = j.Container + } + if len(j.Services) > 0 { + result["services"] = j.Services + } + if len(j.Env) > 0 { + result["env"] = j.Env + } + if len(j.Outputs) > 0 { + result["outputs"] = j.Outputs + } + if len(j.Strategy) > 0 { + result["strategy"] = j.Strategy + } + if j.ContinueOnError { + result["continue-on-error"] = j.ContinueOnError + } + if j.Uses != "" { + result["uses"] = j.Uses + } + if len(j.With) > 0 { + result["with"] = j.With + } + if j.Secrets != nil { + result["secrets"] = j.Secrets + } + + return result +} + +// MapToJob converts a map[string]any to a WorkflowJob +func MapToJob(jobMap map[string]any) (*WorkflowJob, error) { + stepTypesLog.Printf("Converting map to workflow job: map_keys=%d", len(jobMap)) + if jobMap == nil { + return nil, fmt.Errorf("job map is nil") + } + + job := &WorkflowJob{} + + if name, ok := jobMap["name"].(string); ok { + job.Name = name + } + if runsOn, ok := jobMap["runs-on"]; ok { + job.RunsOn = runsOn + } + if needs, ok := jobMap["needs"]; ok { + switch v := needs.(type) { + case []any: + for _, need := range v { + if needStr, ok := need.(string); ok { + job.Needs = append(job.Needs, needStr) + } + } + case []string: + job.Needs = v + case string: + job.Needs = []string{v} + } + } + if ifCond, ok := jobMap["if"].(string); ok { + job.If = ifCond + } + if steps, ok := jobMap["steps"].([]any); ok { + for _, stepAny := range steps { + if stepMap, ok := stepAny.(map[string]any); ok { + step, err := MapToStep(stepMap) + if err != nil { + return nil, fmt.Errorf("failed to convert step: %w", err) + } + job.Steps = append(job.Steps, *step) + } + } + } + if permissions, ok := jobMap["permissions"].(map[string]any); ok { + job.Permissions = make(map[string]string) + for k, v := range permissions { + if strVal, ok := v.(string); ok { + job.Permissions[k] = strVal + } + } + } + if environment, ok := jobMap["environment"]; ok { + job.Environment = environment + } + if concurrency, ok := jobMap["concurrency"]; ok { + job.Concurrency = concurrency + } + if timeoutMinutes, ok := jobMap["timeout-minutes"].(int); ok { + job.TimeoutMinutes = timeoutMinutes + } + if container, ok := jobMap["container"]; ok { + job.Container = container + } + if services, ok := jobMap["services"].(map[string]any); ok { + job.Services = services + } + if env, ok := jobMap["env"].(map[string]any); ok { + job.Env = make(map[string]string) + for k, v := range env { + if strVal, ok := v.(string); ok { + job.Env[k] = strVal + } + } + } + if outputs, ok := jobMap["outputs"].(map[string]any); ok { + job.Outputs = make(map[string]string) + for k, v := range outputs { + if strVal, ok := v.(string); ok { + job.Outputs[k] = strVal + } + } + } + if strategy, ok := jobMap["strategy"].(map[string]any); ok { + job.Strategy = strategy + } + if continueOnError, ok := jobMap["continue-on-error"].(bool); ok { + job.ContinueOnError = continueOnError + } + if uses, ok := jobMap["uses"].(string); ok { + job.Uses = uses + } + if with, ok := jobMap["with"].(map[string]any); ok { + job.With = with + } + if secrets, ok := jobMap["secrets"]; ok { + job.Secrets = secrets + } + + stepTypesLog.Printf("Successfully converted job: name=%s, steps=%d", job.Name, len(job.Steps)) + return job, nil +} + +// StepsToAny converts []WorkflowStep to []any for compatibility with existing code +func StepsToAny(steps []WorkflowStep) []any { + result := make([]any, len(steps)) + for i, step := range steps { + result[i] = step.ToMap() + } + return result +} + +// StepsFromAny converts []any to []WorkflowStep +func StepsFromAny(stepsAny []any) ([]WorkflowStep, error) { + steps := make([]WorkflowStep, 0, len(stepsAny)) + for i, stepAny := range stepsAny { + stepMap, ok := stepAny.(map[string]any) + if !ok { + return nil, fmt.Errorf("step %d is not a map[string]any", i) + } + step, err := MapToStep(stepMap) + if err != nil { + return nil, fmt.Errorf("failed to convert step %d: %w", i, err) + } + steps = append(steps, *step) + } + return steps, nil +} diff --git a/pkg/workflow/step_types_test.go b/pkg/workflow/step_types_test.go index f9e17a99f3..2de634c83f 100644 --- a/pkg/workflow/step_types_test.go +++ b/pkg/workflow/step_types_test.go @@ -454,6 +454,34 @@ func compareStepValues(a, b any) bool { } } return true + case []string: + bSlice, ok := b.([]string) + if !ok { + return false + } + if len(aVal) != len(bSlice) { + return false + } + for i := range aVal { + if aVal[i] != bSlice[i] { + return false + } + } + return true + case []map[string]any: + bSlice, ok := b.([]map[string]any) + if !ok { + return false + } + if len(aVal) != len(bSlice) { + return false + } + for i := range aVal { + if !compareStepValues(aVal[i], bSlice[i]) { + return false + } + } + return true default: return a == b } @@ -492,3 +520,283 @@ func compareSteps(a, b *WorkflowStep) bool { return true } + +// Tests for WorkflowJob type + +func TestWorkflowJob_ToMap(t *testing.T) { + tests := []struct { + name string + job *WorkflowJob + want map[string]any + }{ + { + name: "simple job with steps", + job: &WorkflowJob{ + Name: "Test Job", + RunsOn: "ubuntu-latest", + Steps: []WorkflowStep{ + {Name: "Checkout", Uses: "actions/checkout@v4"}, + {Name: "Run tests", Run: "npm test"}, + }, + }, + want: map[string]any{ + "name": "Test Job", + "runs-on": "ubuntu-latest", + "steps": []map[string]any{ + {"name": "Checkout", "uses": "actions/checkout@v4"}, + {"name": "Run tests", "run": "npm test"}, + }, + }, + }, + { + name: "job with all fields", + job: &WorkflowJob{ + Name: "Complex Job", + RunsOn: "ubuntu-latest", + Needs: []string{"build"}, + If: "success()", + TimeoutMinutes: 30, + Permissions: map[string]string{"contents": "read"}, + Env: map[string]string{"NODE_ENV": "test"}, + Steps: []WorkflowStep{ + {Name: "Test", Run: "npm test"}, + }, + }, + want: map[string]any{ + "name": "Complex Job", + "runs-on": "ubuntu-latest", + "needs": []string{"build"}, + "if": "success()", + "timeout-minutes": 30, + "permissions": map[string]string{"contents": "read"}, + "env": map[string]string{"NODE_ENV": "test"}, + "steps": []map[string]any{ + {"name": "Test", "run": "npm test"}, + }, + }, + }, + { + name: "reusable workflow job", + job: &WorkflowJob{ + Uses: "./.github/workflows/reusable.yml", + With: map[string]any{"config": "test"}, + }, + want: map[string]any{ + "uses": "./.github/workflows/reusable.yml", + "with": map[string]any{"config": "test"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.job.ToMap() + if !compareJobMaps(got, tt.want) { + t.Errorf("WorkflowJob.ToMap() = %+v, want %+v", got, tt.want) + } + }) + } +} + +func TestMapToJob(t *testing.T) { + tests := []struct { + name string + jobMap map[string]any + want *WorkflowJob + wantErr bool + }{ + { + name: "simple job with steps", + jobMap: map[string]any{ + "name": "Test Job", + "runs-on": "ubuntu-latest", + "steps": []any{ + map[string]any{"name": "Checkout", "uses": "actions/checkout@v4"}, + map[string]any{"name": "Run tests", "run": "npm test"}, + }, + }, + want: &WorkflowJob{ + Name: "Test Job", + RunsOn: "ubuntu-latest", + Steps: []WorkflowStep{ + {Name: "Checkout", Uses: "actions/checkout@v4"}, + {Name: "Run tests", Run: "npm test"}, + }, + }, + wantErr: false, + }, + { + name: "job with needs as string", + jobMap: map[string]any{ + "name": "Deploy", + "runs-on": "ubuntu-latest", + "needs": "build", + }, + want: &WorkflowJob{ + Name: "Deploy", + RunsOn: "ubuntu-latest", + Needs: []string{"build"}, + }, + wantErr: false, + }, + { + name: "job with needs as array", + jobMap: map[string]any{ + "name": "Deploy", + "runs-on": "ubuntu-latest", + "needs": []any{"build", "test"}, + }, + want: &WorkflowJob{ + Name: "Deploy", + RunsOn: "ubuntu-latest", + Needs: []string{"build", "test"}, + }, + wantErr: false, + }, + { + name: "nil job map", + jobMap: nil, + want: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := MapToJob(tt.jobMap) + if (err != nil) != tt.wantErr { + t.Errorf("MapToJob() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + return + } + if !compareJobs(got, tt.want) { + t.Errorf("MapToJob() = %+v, want %+v", got, tt.want) + } + }) + } +} + +func TestStepsToAny(t *testing.T) { + steps := []WorkflowStep{ + {Name: "Step 1", Uses: "actions/checkout@v4"}, + {Name: "Step 2", Run: "echo hello"}, + } + + result := StepsToAny(steps) + + if len(result) != 2 { + t.Errorf("StepsToAny() length = %d, want 2", len(result)) + } + + // Check first step + if stepMap, ok := result[0].(map[string]any); ok { + if stepMap["name"] != "Step 1" { + t.Errorf("StepsToAny()[0][name] = %v, want Step 1", stepMap["name"]) + } + } else { + t.Error("StepsToAny()[0] is not a map[string]any") + } +} + +func TestStepsFromAny(t *testing.T) { + stepsAny := []any{ + map[string]any{"name": "Step 1", "uses": "actions/checkout@v4"}, + map[string]any{"name": "Step 2", "run": "echo hello"}, + } + + steps, err := StepsFromAny(stepsAny) + if err != nil { + t.Fatalf("StepsFromAny() error = %v", err) + } + + if len(steps) != 2 { + t.Errorf("StepsFromAny() length = %d, want 2", len(steps)) + } + + if steps[0].Name != "Step 1" { + t.Errorf("StepsFromAny()[0].Name = %s, want Step 1", steps[0].Name) + } + if steps[1].Run != "echo hello" { + t.Errorf("StepsFromAny()[1].Run = %s, want echo hello", steps[1].Run) + } +} + +// Helper functions for comparing jobs + +func compareJobs(a, b *WorkflowJob) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + + if a.Name != b.Name || a.If != b.If || a.TimeoutMinutes != b.TimeoutMinutes { + return false + } + + // Compare RunsOn + if !compareStepValues(a.RunsOn, b.RunsOn) { + return false + } + + // Compare Needs + if len(a.Needs) != len(b.Needs) { + return false + } + for i := range a.Needs { + if a.Needs[i] != b.Needs[i] { + return false + } + } + + // Compare Steps + if len(a.Steps) != len(b.Steps) { + return false + } + for i := range a.Steps { + if !compareSteps(&a.Steps[i], &b.Steps[i]) { + return false + } + } + + return true +} + +func compareJobMaps(a, b map[string]any) bool { + if len(a) != len(b) { + return false + } + + for key, aVal := range a { + bVal, ok := b[key] + if !ok { + return false + } + + // Special handling for steps (array of maps) + if key == "steps" { + aSteps, aOK := aVal.([]map[string]any) + bSteps, bOK := bVal.([]map[string]any) + if aOK && bOK { + if len(aSteps) != len(bSteps) { + return false + } + for i := range aSteps { + if !compareStepValues(aSteps[i], bSteps[i]) { + return false + } + } + continue + } + } + + if !compareStepValues(aVal, bVal) { + return false + } + } + + return true +}