diff --git a/.changeset/patch-ignore-frontmatter-fields.md b/.changeset/patch-ignore-frontmatter-fields.md new file mode 100644 index 0000000000..cac958aa9f --- /dev/null +++ b/.changeset/patch-ignore-frontmatter-fields.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Add support for silently ignoring description and applyTo fields in frontmatter diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 8b939daf95..da0cf7384f 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -323,6 +323,9 @@ var PriorityJobFields = []string{"name", "runs-on", "needs", "if", "permissions" // Fields appear in this order first, followed by remaining fields alphabetically var PriorityWorkflowFields = []string{"on", "permissions", "if", "network", "imports", "safe-outputs", "steps"} +// IgnoredFrontmatterFields are fields that should be silently ignored during frontmatter validation +var IgnoredFrontmatterFields = []string{"description", "applyTo"} + func GetWorkflowDir() string { return filepath.Join(".github", "workflows") } diff --git a/pkg/parser/schema.go b/pkg/parser/schema.go index 5b0ecdcd39..4cb04eb36e 100644 --- a/pkg/parser/schema.go +++ b/pkg/parser/schema.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/githubnext/gh-aw/pkg/console" + "github.com/githubnext/gh-aw/pkg/constants" "github.com/githubnext/gh-aw/pkg/logger" "github.com/santhosh-tekuri/jsonschema/v6" ) @@ -26,54 +27,91 @@ var includedFileSchema string //go:embed schemas/mcp_config_schema.json var mcpConfigSchema string +// filterIgnoredFields removes ignored fields from frontmatter without warnings +func filterIgnoredFields(frontmatter map[string]any) map[string]any { + if frontmatter == nil { + return nil + } + + // Create a copy of the frontmatter map without ignored fields + filtered := make(map[string]any) + for key, value := range frontmatter { + // Skip ignored fields + ignored := false + for _, ignoredField := range constants.IgnoredFrontmatterFields { + if key == ignoredField { + ignored = true + break + } + } + if !ignored { + filtered[key] = value + } + } + + return filtered +} + // ValidateMainWorkflowFrontmatterWithSchema validates main workflow frontmatter using JSON schema func ValidateMainWorkflowFrontmatterWithSchema(frontmatter map[string]any) error { schemaLog.Print("Validating main workflow frontmatter with schema") + // Filter out ignored fields before validation + filtered := filterIgnoredFields(frontmatter) + // First run the standard schema validation - if err := validateWithSchema(frontmatter, mainWorkflowSchema, "main workflow file"); err != nil { + if err := validateWithSchema(filtered, mainWorkflowSchema, "main workflow file"); err != nil { schemaLog.Printf("Schema validation failed for main workflow: %v", err) return err } // Then run custom validation for engine-specific rules - return validateEngineSpecificRules(frontmatter) + return validateEngineSpecificRules(filtered) } // ValidateMainWorkflowFrontmatterWithSchemaAndLocation validates main workflow frontmatter with file location info func ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter map[string]any, filePath string) error { + // Filter out ignored fields before validation + filtered := filterIgnoredFields(frontmatter) + // First run the standard schema validation with location - if err := validateWithSchemaAndLocation(frontmatter, mainWorkflowSchema, "main workflow file", filePath); err != nil { + if err := validateWithSchemaAndLocation(filtered, mainWorkflowSchema, "main workflow file", filePath); err != nil { return err } // Then run custom validation for engine-specific rules - return validateEngineSpecificRules(frontmatter) + return validateEngineSpecificRules(filtered) } // ValidateIncludedFileFrontmatterWithSchema validates included file frontmatter using JSON schema func ValidateIncludedFileFrontmatterWithSchema(frontmatter map[string]any) error { schemaLog.Print("Validating included file frontmatter with schema") + // Filter out ignored fields before validation + filtered := filterIgnoredFields(frontmatter) + // First run the standard schema validation - if err := validateWithSchema(frontmatter, includedFileSchema, "included file"); err != nil { + if err := validateWithSchema(filtered, includedFileSchema, "included file"); err != nil { schemaLog.Printf("Schema validation failed for included file: %v", err) return err } // Then run custom validation for engine-specific rules - return validateEngineSpecificRules(frontmatter) + return validateEngineSpecificRules(filtered) } // ValidateIncludedFileFrontmatterWithSchemaAndLocation validates included file frontmatter with file location info func ValidateIncludedFileFrontmatterWithSchemaAndLocation(frontmatter map[string]any, filePath string) error { + // Filter out ignored fields before validation + filtered := filterIgnoredFields(frontmatter) + // First run the standard schema validation with location - if err := validateWithSchemaAndLocation(frontmatter, includedFileSchema, "included file", filePath); err != nil { + if err := validateWithSchemaAndLocation(filtered, includedFileSchema, "included file", filePath); err != nil { return err } // Then run custom validation for engine-specific rules - return validateEngineSpecificRules(frontmatter) + return validateEngineSpecificRules(filtered) } // ValidateMCPConfigWithSchema validates MCP configuration using JSON schema diff --git a/pkg/parser/schema_test.go b/pkg/parser/schema_test.go index 18b58db2d2..d7ab7bbff4 100644 --- a/pkg/parser/schema_test.go +++ b/pkg/parser/schema_test.go @@ -4,6 +4,8 @@ import ( "os" "strings" "testing" + + "github.com/githubnext/gh-aw/pkg/constants" ) func TestValidateMainWorkflowFrontmatterWithSchema(t *testing.T) { @@ -1166,3 +1168,241 @@ timeout_minu tes: 10 t.Errorf("Error message should contain file path, got: %s", errorMsg) } } + +func TestFilterIgnoredFields(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + expected map[string]any + }{ + { + name: "nil frontmatter", + frontmatter: nil, + expected: nil, + }, + { + name: "empty frontmatter", + frontmatter: map[string]any{}, + expected: map[string]any{}, + }, + { + name: "frontmatter with description only", + frontmatter: map[string]any{ + "description": "This is a test workflow", + "on": "push", + }, + expected: map[string]any{ + "on": "push", + }, + }, + { + name: "frontmatter with applyTo only", + frontmatter: map[string]any{ + "applyTo": "some-value", + "on": "push", + }, + expected: map[string]any{ + "on": "push", + }, + }, + { + name: "frontmatter with both description and applyTo", + frontmatter: map[string]any{ + "description": "This is a test workflow", + "applyTo": "some-value", + "on": "push", + "engine": "claude", + }, + expected: map[string]any{ + "on": "push", + "engine": "claude", + }, + }, + { + name: "frontmatter with only valid fields", + frontmatter: map[string]any{ + "on": "push", + "engine": "claude", + }, + expected: map[string]any{ + "on": "push", + "engine": "claude", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := filterIgnoredFields(tt.frontmatter) + + if tt.expected == nil { + if result != nil { + t.Errorf("Expected nil, got %v", result) + } + return + } + + if len(result) != len(tt.expected) { + t.Errorf("Expected %d fields, got %d fields", len(tt.expected), len(result)) + } + + for key, expectedValue := range tt.expected { + if actualValue, ok := result[key]; !ok { + t.Errorf("Expected field %q not found in result", key) + } else { + // For simple types, compare directly + // For maps, we need to compare keys (simple check for this test) + switch v := expectedValue.(type) { + case map[string]any: + if actualMap, ok := actualValue.(map[string]any); !ok { + t.Errorf("Field %q: expected map, got %T", key, actualValue) + } else if len(actualMap) != len(v) { + t.Errorf("Field %q: expected map with %d keys, got %d keys", key, len(v), len(actualMap)) + } + default: + if actualValue != expectedValue { + t.Errorf("Field %q: expected %v, got %v", key, expectedValue, actualValue) + } + } + } + } + + // Check that ignored fields are not present + for _, ignoredField := range constants.IgnoredFrontmatterFields { + if _, ok := result[ignoredField]; ok { + t.Errorf("Ignored field %q should not be present in result", ignoredField) + } + } + }) + } +} + +func TestValidateMainWorkflowWithIgnoredFields(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + wantErr bool + errContains string + }{ + { + name: "valid frontmatter with description field - should be silently ignored", + frontmatter: map[string]any{ + "on": "push", + "description": "This is a test workflow description", + "engine": "claude", + }, + wantErr: false, + }, + { + name: "valid frontmatter with applyTo field - should be silently ignored", + frontmatter: map[string]any{ + "on": "push", + "applyTo": "some-target", + "engine": "claude", + }, + wantErr: false, + }, + { + name: "valid frontmatter with both description and applyTo - should be silently ignored", + frontmatter: map[string]any{ + "on": "push", + "description": "Test workflow", + "applyTo": "some-target", + "engine": "claude", + }, + wantErr: false, + }, + { + name: "invalid frontmatter with ignored fields - other validation should still work", + frontmatter: map[string]any{ + "on": "push", + "description": "Test workflow", + "applyTo": "some-target", + "invalid_field": "should-fail", + }, + wantErr: true, + errContains: "invalid_field", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateMainWorkflowFrontmatterWithSchema(tt.frontmatter) + + if (err != nil) != tt.wantErr { + t.Errorf("ValidateMainWorkflowFrontmatterWithSchema() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if err != nil && tt.errContains != "" { + if !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("Error message should contain %q, got: %v", tt.errContains, err) + } + } + }) + } +} + +func TestValidateIncludedFileWithIgnoredFields(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + wantErr bool + errContains string + }{ + { + name: "valid included file with description field - should be silently ignored", + frontmatter: map[string]any{ + "description": "This is a shared config", + "tools": map[string]any{ + "github": map[string]any{ + "allowed": []string{"get_repository"}, + }, + }, + }, + wantErr: false, + }, + { + name: "valid included file with applyTo field - should be silently ignored", + frontmatter: map[string]any{ + "applyTo": "some-target", + "tools": map[string]any{ + "github": map[string]any{ + "allowed": []string{"get_repository"}, + }, + }, + }, + wantErr: false, + }, + { + name: "valid included file with both description and applyTo - should be silently ignored", + frontmatter: map[string]any{ + "description": "Shared config", + "applyTo": "some-target", + "tools": map[string]any{ + "github": map[string]any{ + "allowed": []string{"get_repository"}, + }, + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateIncludedFileFrontmatterWithSchema(tt.frontmatter) + + if (err != nil) != tt.wantErr { + t.Errorf("ValidateIncludedFileFrontmatterWithSchema() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if err != nil && tt.errContains != "" { + if !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("Error message should contain %q, got: %v", tt.errContains, err) + } + } + }) + } +}