diff --git a/pkg/workflow/schema_validation.go b/pkg/workflow/schema_validation.go index 9ad01bd501..6649630fcc 100644 --- a/pkg/workflow/schema_validation.go +++ b/pkg/workflow/schema_validation.go @@ -40,6 +40,7 @@ package workflow import ( "encoding/json" "fmt" + "strings" "sync" "github.com/goccy/go-yaml" @@ -111,8 +112,97 @@ func (c *Compiler) validateGitHubActionsSchema(yamlContent string) error { } if err := schema.Validate(jsonObj); err != nil { - return fmt.Errorf("GitHub Actions schema validation failed: %w", err) + // Enhance error message with field-specific examples + enhancedErr := enhanceSchemaValidationError(err) + return fmt.Errorf("GitHub Actions schema validation failed: %w", enhancedErr) } return nil } + +// enhanceSchemaValidationError adds inline examples to schema validation errors +func enhanceSchemaValidationError(err error) error { + ve, ok := err.(*jsonschema.ValidationError) + if !ok { + return err + } + + // Extract field path from InstanceLocation + fieldPath := extractFieldPath(ve.InstanceLocation) + if fieldPath == "" { + return err // Cannot enhance, return original error + } + + // Get field-specific example + example := getFieldExample(fieldPath, ve) + if example == "" { + return err // No example available, return original error + } + + // Return enhanced error with example + return fmt.Errorf("%v. %s", err, example) +} + +// extractFieldPath converts InstanceLocation to a readable field path +func extractFieldPath(location []string) string { + if len(location) == 0 { + return "" + } + + // Join the location parts to form a path like "timeout-minutes" or "jobs/build/runs-on" + return location[len(location)-1] // Return the last element as the field name +} + +// getFieldExample returns an example for the given field based on the validation error +func getFieldExample(fieldPath string, err error) string { + // Map of common fields to their examples + fieldExamples := map[string]string{ + "timeout-minutes": "Example: timeout-minutes: 10", + "engine": "Valid engines are: copilot, claude, codex, custom. Example: engine: copilot", + "permissions": "Example: permissions:\\n contents: read\\n issues: write", + "on": "Example: on: push or on:\\n issues:\\n types: [opened]", + "runs-on": "Example: runs-on: ubuntu-latest", + "concurrency": "Example: concurrency: production or concurrency:\\n group: ${{ github.workflow }}\\n cancel-in-progress: true", + "env": "Example: env:\\n NODE_ENV: production", + "tools": "Example: tools:\\n github:\\n allowed: [list_issues]", + "steps": "Example: steps:\\n - name: Checkout\\n uses: actions/checkout@v4", + "jobs": "Example: jobs:\\n build:\\n runs-on: ubuntu-latest\\n steps:\\n - run: echo 'hello'", + "strategy": "Example: strategy:\\n matrix:\\n os: [ubuntu-latest, windows-latest]", + "container": "Example: container: node:20 or container:\\n image: node:20\\n options: --user root", + "services": "Example: services:\\n postgres:\\n image: postgres:15\\n env:\\n POSTGRES_PASSWORD: postgres", + "defaults": "Example: defaults:\\n run:\\n shell: bash", + "name": "Example: name: \"Build and Test\"", + "if": "Example: if: github.event_name == 'push'", + "environment": "Example: environment: production or environment:\\n name: production\\n url: https://example.com", + "outputs": "Example: outputs:\\n build-id: ${{ steps.build.outputs.id }}", + "needs": "Example: needs: build or needs: [build, test]", + "uses": "Example: uses: ./.github/workflows/reusable.yml", + "with": "Example: with:\\n node-version: '20'", + "secrets": "Example: secrets:\\n token: ${{ secrets.GITHUB_TOKEN }}", + } + + // Check if we have a specific example for this field + if example, ok := fieldExamples[fieldPath]; ok { + return example + } + + // Generic examples based on error type + errorMsg := err.Error() + if strings.Contains(errorMsg, "string") { + return fmt.Sprintf("Example: %s: \"value\"", fieldPath) + } + if strings.Contains(errorMsg, "boolean") { + return fmt.Sprintf("Example: %s: true", fieldPath) + } + if strings.Contains(errorMsg, "object") { + return fmt.Sprintf("Example: %s:\\n key: value", fieldPath) + } + if strings.Contains(errorMsg, "array") { + return fmt.Sprintf("Example: %s: [item1, item2]", fieldPath) + } + if strings.Contains(errorMsg, "integer") || strings.Contains(errorMsg, "type") { + return fmt.Sprintf("Example: %s: 10", fieldPath) + } + + return "" // No example available +} diff --git a/pkg/workflow/schema_validation_integration_test.go b/pkg/workflow/schema_validation_integration_test.go new file mode 100644 index 0000000000..838f51e346 --- /dev/null +++ b/pkg/workflow/schema_validation_integration_test.go @@ -0,0 +1,129 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSchemaValidationWithExamplesIntegration(t *testing.T) { + // Create temporary directory for test files + tmpDir, err := os.MkdirTemp("", "schema-validation-test") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + tests := []struct { + name string + workflowYAML string + expectedParts []string // Parts we expect in the error message + shouldPass bool + }{ + { + name: "valid workflow should pass", + workflowYAML: `--- +on: push +permissions: + contents: read +--- + +# Valid Workflow +This is a valid workflow. +`, + shouldPass: true, + }, + // Note: Most schema violations would be caught during frontmatter parsing + // before reaching the GitHub Actions schema validation. + // The schema validation primarily catches issues in the generated YAML + // that are structurally valid but violate GitHub Actions schema constraints. + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create test workflow file + testFile := filepath.Join(tmpDir, "test-workflow.md") + if err := os.WriteFile(testFile, []byte(tt.workflowYAML), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler(false, "", "test") + compiler.SetSkipValidation(false) // Enable schema validation + + err := compiler.CompileWorkflow(testFile) + + if tt.shouldPass && err != nil { + t.Errorf("Expected workflow to pass validation, but got error: %v", err) + } + + if !tt.shouldPass && err == nil { + t.Error("Expected workflow to fail validation, but it passed") + } + + if err != nil && len(tt.expectedParts) > 0 { + errStr := err.Error() + for _, part := range tt.expectedParts { + if !strings.Contains(errStr, part) { + t.Errorf("Error message missing expected part %q. Full error: %v", part, err) + } + } + } + }) + } +} + +func TestSchemaValidationExampleQuality(t *testing.T) { + // Test that our example map has good quality examples + tests := []struct { + field string + example string + required []string // Parts that should be in the example + }{ + { + field: "timeout-minutes", + example: getFieldExample("timeout-minutes", &mockValidationError{msg: "test"}), + required: []string{"Example:", "timeout-minutes:", "10"}, + }, + { + field: "engine", + example: getFieldExample("engine", &mockValidationError{msg: "test"}), + required: []string{"Valid engines are:", "copilot", "claude", "codex", "custom"}, + }, + { + field: "permissions", + example: getFieldExample("permissions", &mockValidationError{msg: "test"}), + required: []string{"Example:", "permissions:", "contents:", "read", "issues:", "write"}, + }, + { + field: "runs-on", + example: getFieldExample("runs-on", &mockValidationError{msg: "test"}), + required: []string{"Example:", "runs-on:", "ubuntu-latest"}, + }, + { + field: "concurrency", + example: getFieldExample("concurrency", &mockValidationError{msg: "test"}), + required: []string{"Example:", "concurrency:"}, + }, + { + field: "jobs", + example: getFieldExample("jobs", &mockValidationError{msg: "test"}), + required: []string{"Example:", "jobs:", "runs-on:", "steps:"}, + }, + } + + for _, tt := range tests { + t.Run(tt.field, func(t *testing.T) { + if tt.example == "" { + t.Errorf("No example provided for field %q", tt.field) + return + } + + for _, required := range tt.required { + if !strings.Contains(tt.example, required) { + t.Errorf("Example for %q missing required part %q. Example: %s", tt.field, required, tt.example) + } + } + }) + } +} diff --git a/pkg/workflow/schema_validation_test.go b/pkg/workflow/schema_validation_test.go new file mode 100644 index 0000000000..fecb86e568 --- /dev/null +++ b/pkg/workflow/schema_validation_test.go @@ -0,0 +1,245 @@ +package workflow + +import ( + "strings" + "testing" +) + +func TestExtractFieldPath(t *testing.T) { + tests := []struct { + name string + location []string + expected string + }{ + { + name: "single field", + location: []string{"timeout-minutes"}, + expected: "timeout-minutes", + }, + { + name: "nested field", + location: []string{"jobs", "build", "runs-on"}, + expected: "runs-on", + }, + { + name: "empty location", + location: []string{}, + expected: "", + }, + { + name: "permissions field", + location: []string{"permissions"}, + expected: "permissions", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractFieldPath(tt.location) + if result != tt.expected { + t.Errorf("extractFieldPath(%v) = %q, want %q", tt.location, result, tt.expected) + } + }) + } +} + +func TestGetFieldExample(t *testing.T) { + tests := []struct { + name string + fieldPath string + errorMsg string + expectedParts []string // Parts that should be in the example + }{ + { + name: "timeout-minutes", + fieldPath: "timeout-minutes", + errorMsg: "type error", + expectedParts: []string{"Example:", "timeout-minutes:", "10"}, + }, + { + name: "engine", + fieldPath: "engine", + errorMsg: "invalid value", + expectedParts: []string{"Valid engines are:", "copilot", "claude", "codex", "custom", "Example:"}, + }, + { + name: "permissions", + fieldPath: "permissions", + errorMsg: "type error", + expectedParts: []string{"Example:", "permissions:", "contents:", "read"}, + }, + { + name: "runs-on", + fieldPath: "runs-on", + errorMsg: "type error", + expectedParts: []string{"Example:", "runs-on:", "ubuntu-latest"}, + }, + { + name: "on trigger", + fieldPath: "on", + errorMsg: "type error", + expectedParts: []string{"Example:", "on:"}, + }, + { + name: "generic integer field", + fieldPath: "unknown-int-field", + errorMsg: "expected type integer", + expectedParts: []string{"Example:", "unknown-int-field:", "10"}, + }, + { + name: "generic string field", + fieldPath: "unknown-string-field", + errorMsg: "expected type string", + expectedParts: []string{"Example:", "unknown-string-field:", "\"value\""}, + }, + { + name: "generic boolean field", + fieldPath: "unknown-bool-field", + errorMsg: "expected type boolean", + expectedParts: []string{"Example:", "unknown-bool-field:", "true"}, + }, + { + name: "generic object field", + fieldPath: "unknown-object-field", + errorMsg: "expected type object", + expectedParts: []string{"Example:", "unknown-object-field:", "key:", "value"}, + }, + { + name: "generic array field", + fieldPath: "unknown-array-field", + errorMsg: "expected type array", + expectedParts: []string{"Example:", "unknown-array-field:", "["}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock validation error with a mock error struct + ve := &mockValidationError{msg: tt.errorMsg} + + result := getFieldExample(tt.fieldPath, ve) + + if result == "" && len(tt.expectedParts) > 0 { + t.Errorf("getFieldExample(%q, %q) returned empty string, want non-empty", tt.fieldPath, tt.errorMsg) + return + } + + for _, part := range tt.expectedParts { + if !strings.Contains(result, part) { + t.Errorf("getFieldExample(%q, %q) = %q, missing expected part %q", + tt.fieldPath, tt.errorMsg, result, part) + } + } + }) + } +} + +func TestEnhanceSchemaValidationError(t *testing.T) { + tests := []struct { + name string + location []string + errorMsg string + expectExample bool + }{ + { + name: "timeout-minutes error gets example", + location: []string{"timeout-minutes"}, + errorMsg: "type mismatch", + expectExample: true, + }, + { + name: "engine error gets example", + location: []string{"engine"}, + errorMsg: "invalid enum value", + expectExample: true, + }, + { + name: "permissions error gets example", + location: []string{"permissions"}, + errorMsg: "type error", + expectExample: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock validation error without using jsonschema.ValidationError + // since it can't be properly instantiated in tests + mockErr := &mockValidationError{msg: tt.errorMsg} + + // Instead of using enhanceSchemaValidationError (which requires a real ValidationError), + // we test the helper functions directly + fieldPath := extractFieldPath(tt.location) + example := getFieldExample(fieldPath, mockErr) + + if tt.expectExample { + if !strings.Contains(example, "Example:") { + t.Errorf("getFieldExample() = %q, want to contain 'Example:'", example) + } + } + }) + } +} + +func TestValidateGitHubActionsSchemaWithExamples(t *testing.T) { + // Note: This test requires the schema to be loaded, which happens in validateGitHubActionsSchema + // We'll test the integration by using the actual compiler method + + compiler := NewCompiler(false, "", "test") + compiler.SetSkipValidation(false) // Enable validation for this test + + tests := []struct { + name string + yamlContent string + expectError bool + expectedParts []string + }{ + { + name: "valid workflow passes", + yamlContent: ` +name: Test +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo hello +`, + expectError: false, + }, + // Note: We can't easily test invalid YAML here because the schema validation + // happens after YAML parsing, and most errors would be caught by the parser first. + // The enhancement is verified through the unit tests above. + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := compiler.validateGitHubActionsSchema(tt.yamlContent) + + if tt.expectError && err == nil { + t.Error("validateGitHubActionsSchema() expected error, got nil") + } + if !tt.expectError && err != nil { + t.Errorf("validateGitHubActionsSchema() unexpected error: %v", err) + } + + if err != nil { + errStr := err.Error() + for _, part := range tt.expectedParts { + if !strings.Contains(errStr, part) { + t.Errorf("error %q missing expected part %q", errStr, part) + } + } + } + }) + } +} + +// mockValidationError helps create validation errors for testing +type mockValidationError struct { + msg string +} + +func (m *mockValidationError) Error() string { + return m.msg +}