Remove timeout_minutes from schema and add labels validation

pkg/parser/schema_deprecated_test.go

@@ -12,26 +12,16 @@ func TestGetMainWorkflowDeprecatedFields(t *testing.T) {
 		t.Fatalf("GetMainWorkflowDeprecatedFields() error = %v", err)
 	}
 
-	// Check that timeout_minutes IS in the list as a deprecated field
-	// This allows strict mode to properly detect and reject it
-	found := false
-	var timeoutMinutesField *DeprecatedField
+	// Check that timeout_minutes is NOT in the list (it has been removed from the schema)
+	// This field was fully removed per https://github.com/github/gh-aw/issues/14736
 	for _, field := range deprecatedFields {
 		if field.Name == "timeout_minutes" {
-			found = true
-			timeoutMinutesField = &field
-			break
+			t.Errorf("timeout_minutes should not be in the deprecated fields list - it has been completely removed from the schema")
 		}
 	}
 
-	if !found {
-		t.Error("timeout_minutes should be in the deprecated fields list to support strict mode validation")
-	} else {
-		// Verify it has the correct replacement
-		if timeoutMinutesField.Replacement != "timeout-minutes" {
-			t.Errorf("timeout_minutes replacement = %v, want 'timeout-minutes'", timeoutMinutesField.Replacement)
-		}
-	}
+	// The list can be empty or contain other deprecated fields, but timeout_minutes should not be present
+	t.Logf("Found %d deprecated fields in schema (timeout_minutes correctly removed)", len(deprecatedFields))
 }
 
 func TestFindDeprecatedFieldsInFrontmatter(t *testing.T) {

pkg/parser/schemas/main_workflow_schema.json

@@ -1791,12 +1791,6 @@
       "description": "Workflow timeout in minutes (GitHub Actions standard field). Defaults to 20 minutes for agentic workflows. Has sensible defaults and can typically be omitted.",
       "examples": [5, 10, 30]
     },
-    "timeout_minutes": {
-      "type": "integer",
-      "deprecated": true,
-      "description": "DEPRECATED: Use 'timeout-minutes' instead. Workflow timeout in minutes.",
-      "x-deprecation-message": "Use 'timeout-minutes' (with hyphen) instead of 'timeout_minutes' (with underscore) to follow GitHub Actions naming conventions."
-    },
     "concurrency": {
       "description": "Concurrency control to limit concurrent workflow runs (GitHub Actions standard field). Supports two forms: simple string for basic group isolation, or object with cancel-in-progress option for advanced control. Agentic workflows enhance this with automatic per-engine concurrency policies (defaults to single job per engine across all workflows) and token-based rate limiting. Default behavior: workflows in the same group queue sequentially unless cancel-in-progress is true. See https://docs.github.com/en/actions/using-jobs/using-concurrency",
       "oneOf": [

pkg/workflow/compiler.go

@@ -192,6 +192,12 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath
 		return formatCompilerError(markdownPath, "error", err.Error(), err)
 	}
 
+	// Validate labels configuration
+	log.Printf("Validating labels")
+	if err := validateLabels(workflowData); err != nil {
+		return formatCompilerError(markdownPath, "error", err.Error(), err)
+	}
+
 	// Emit experimental warning for sandbox-runtime feature
 	if isSRTEnabled(workflowData) {
 		fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Using experimental feature: sandbox-runtime firewall"))

pkg/workflow/labels_validation.go

@@ -0,0 +1,48 @@
+package workflow
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
+)
+
+var labelsValidationLog = logger.New("workflow:labels_validation")
+
+// validateLabels validates the labels field in the workflow frontmatter.
+// It checks that:
+// 1. Labels is an array (if present)
+// 2. Each label is a non-empty string
+// 3. Labels don't have leading or trailing whitespace
+func validateLabels(workflowData *WorkflowData) error {
+	// If ParsedFrontmatter is nil or Labels is empty, nothing to validate
+	if workflowData == nil || workflowData.ParsedFrontmatter == nil {
+		return nil
+	}
+
+	labels := workflowData.ParsedFrontmatter.Labels
+	if len(labels) == 0 {
+		return nil
+	}
+
+	labelsValidationLog.Printf("Validating %d labels", len(labels))
+
+	// Validate each label
+	for i, label := range labels {
+		// Check for empty labels (should be caught by schema minLength: 1, but double-check)
+		if label == "" {
+			return fmt.Errorf("labels[%d] is empty. Each label must be a non-empty string", i)
+		}
+
+		// Check for excessive whitespace
+		trimmed := strings.TrimSpace(label)
+		if trimmed != label {
+			return fmt.Errorf("labels[%d] has leading or trailing whitespace: %q. Labels should be trimmed", i, label)
+		}
+
+		labelsValidationLog.Printf("Label %d validated: %q", i, label)
+	}
+
+	labelsValidationLog.Printf("All %d labels validated successfully", len(labels))
+	return nil
+}

pkg/workflow/labels_validation_test.go

@@ -0,0 +1,102 @@
+//go:build !integration
+
+package workflow
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateLabels(t *testing.T) {
+	tests := []struct {
+		name      string
+		labels    []string
+		shouldErr bool
+		errorMsg  string
+	}{
+		{
+			name:      "valid labels",
+			labels:    []string{"automation", "security", "ci"},
+			shouldErr: false,
+		},
+		{
+			name:      "no labels",
+			labels:    []string{},
+			shouldErr: false,
+		},
+		{
+			name:      "single valid label",
+			labels:    []string{"docs"},
+			shouldErr: false,
+		},
+		{
+			name:      "empty label",
+			labels:    []string{"automation", "", "security"},
+			shouldErr: true,
+			errorMsg:  "labels[1] is empty",
+		},
+		{
+			name:      "label with leading whitespace",
+			labels:    []string{"automation", " security", "ci"},
+			shouldErr: true,
+			errorMsg:  "labels[1] has leading or trailing whitespace",
+		},
+		{
+			name:      "label with trailing whitespace",
+			labels:    []string{"automation", "security ", "ci"},
+			shouldErr: true,
+			errorMsg:  "labels[1] has leading or trailing whitespace",
+		},
+		{
+			name:      "label with both leading and trailing whitespace",
+			labels:    []string{" automation "},
+			shouldErr: true,
+			errorMsg:  "labels[0] has leading or trailing whitespace",
+		},
+		{
+			name:      "whitespace-only label",
+			labels:    []string{"automation", "   ", "security"},
+			shouldErr: true,
+			errorMsg:  "labels[1] has leading or trailing whitespace",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			workflowData := &WorkflowData{
+				ParsedFrontmatter: &FrontmatterConfig{
+					Labels: tt.labels,
+				},
+			}
+
+			err := validateLabels(workflowData)
+
+			if tt.shouldErr {
+				require.Error(t, err, "Expected validation to fail")
+				if tt.errorMsg != "" {
+					assert.Contains(t, err.Error(), tt.errorMsg, "Error message should contain expected text")
+				}
+			} else {
+				assert.NoError(t, err, "Expected validation to pass")
+			}
+		})
+	}
+}
+
+func TestValidateLabels_NilFrontmatter(t *testing.T) {
+	// Test with nil ParsedFrontmatter
+	workflowData := &WorkflowData{
+		ParsedFrontmatter: nil,
+	}
+
+	err := validateLabels(workflowData)
+	assert.NoError(t, err, "Should handle nil frontmatter gracefully")
+}
+
+func TestValidateLabels_NilWorkflowData(t *testing.T) {
+	// Test with nil workflowData
+	err := validateLabels(nil)
+	assert.NoError(t, err, "Should handle nil workflowData gracefully")
+}

pkg/workflow/strict_mode_deprecated_test.go

@@ -17,7 +17,7 @@ func TestStrictModeDeprecatedFields(t *testing.T) {
 		errorMsg    string
 	}{
 		{
-			name: "deprecated timeout_minutes field refused in strict mode",
+			name: "removed timeout_minutes field rejected (unknown property)",
 			content: `---
 on: push
 permissions:
@@ -33,7 +33,7 @@ network:
 
 # Test Workflow`,
 			expectError: true,
-			errorMsg:    "deprecated fields are not allowed",
+			errorMsg:    "Unknown property: timeout_minutes",
 		},
 		{
 			name: "non-deprecated timeout-minutes field allowed in strict mode",
@@ -54,7 +54,7 @@ network:
 			expectError: false,
 		},
 		{
-			name: "deprecated field allowed in non-strict mode",
+			name: "removed field rejected even in non-strict mode",
 			content: `---
 on: push
 permissions:
@@ -65,7 +65,8 @@ strict: false
 ---
 
 # Test Workflow`,
-			expectError: false,
+			expectError: true,
+			errorMsg:    "Unknown property: timeout_minutes",
 		},
 	}
 
@@ -142,9 +143,9 @@ network:
 	errorMsg := err.Error()
 
 	// Check that error message includes:
-	// 1. Mentions deprecated fields
-	if !strings.Contains(errorMsg, "deprecated") {
-		t.Errorf("Error message should mention 'deprecated': %s", errorMsg)
+	// 1. Mentions unknown property (timeout_minutes has been removed from schema)
+	if !strings.Contains(errorMsg, "Unknown property") {
+		t.Errorf("Error message should mention 'Unknown property': %s", errorMsg)
 	}
 
 	// 2. Mentions the specific field