Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions pkg/workflow/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"slices"
"strings"
"time"
Expand All @@ -20,21 +19,6 @@ import (

var log = logger.New("workflow:compiler")

// heredocDelimiterRE matches randomized heredoc delimiters of the form GH_AW_<NAME>_<16hexchars>_EOF.
// Used to normalize delimiters when comparing compiled output to skip unnecessary writes.
var heredocDelimiterRE = regexp.MustCompile(`GH_AW_([A-Z0-9_]+)_[0-9a-f]{16}_EOF`)

// normalizeHeredocDelimiters replaces randomized heredoc delimiter tokens with a stable
// placeholder so that two compilations of the same workflow compare as equal even though
// each run embeds different random tokens.
func normalizeHeredocDelimiters(content string) string {
// Fast path: skip regex if content contains no heredoc delimiters
if !strings.Contains(content, "GH_AW_") {
return content
}
return heredocDelimiterRE.ReplaceAllString(content, "GH_AW_${1}_NORM_EOF")
}

const (
// MaxLockFileSize is the maximum allowed size for generated lock workflow files (500KB)
MaxLockFileSize = 512000 // 500KB in bytes
Expand Down
23 changes: 0 additions & 23 deletions pkg/workflow/compiler_filters_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
package workflow

import (
"errors"
"fmt"
"strings"
)
Expand Down Expand Up @@ -204,25 +203,3 @@ func validateGlobList(eventMap map[string]any, eventName, filterKey string, isPa
}
return nil
}

// toStringSlice converts an any value to a []string, supporting []string, []any, and string.
func toStringSlice(val any) ([]string, error) {
switch v := val.(type) {
case []string:
return v, nil
case []any:
result := make([]string, 0, len(v))
for _, item := range v {
s, ok := item.(string)
if !ok {
return nil, errors.New("non-string item in list")
}
result = append(result, s)
}
return result, nil
case string:
return []string{v}, nil
default:
return nil, fmt.Errorf("unsupported type %T", val)
}
}
37 changes: 0 additions & 37 deletions pkg/workflow/role_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,43 +504,6 @@ func (c *Compiler) extractSkipBots(frontmatter map[string]any) []string {
return extractSkipField(frontmatter, "skip-bots")
}

// extractStringSliceField extracts a string slice from various input formats
// Handles: string, []string, []any (with string elements)
// Returns nil if the input is empty or invalid
func extractStringSliceField(value any, fieldName string) []string {
switch v := value.(type) {
case string:
// Single string
if v == "" {
return nil
}
roleLog.Printf("Extracted single %s: %s", fieldName, v)
return []string{v}
case []string:
// Already a string slice
if len(v) == 0 {
return nil
}
roleLog.Printf("Extracted %d %s: %v", len(v), fieldName, v)
return v
case []any:
// Array of any - extract strings
var result []string
for _, item := range v {
if str, ok := item.(string); ok && str != "" {
result = append(result, str)
}
}
if len(result) == 0 {
return nil
}
roleLog.Printf("Extracted %d %s from array: %v", len(result), fieldName, result)
return result
}
roleLog.Printf("No valid %s found or unsupported type: %T", fieldName, value)
return nil
}

// mergeStringSlicesDedup merges two string slices with deduplication (preserving order of first occurrence).
// Logs the merged result under the given logLabel when the result is non-empty.
func mergeStringSlicesDedup(top, imported []string, logLabel string) []string {
Expand Down
15 changes: 15 additions & 0 deletions pkg/workflow/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,21 @@ func GenerateHeredocDelimiterFromSeed(name string, seed string) string {
return "GH_AW_" + upperName + "_" + tag + "_EOF"
}

// heredocDelimiterRE matches randomized heredoc delimiters of the form GH_AW_<NAME>_<16hexchars>_EOF.
// Used to normalize delimiters when comparing compiled output to skip unnecessary writes.
var heredocDelimiterRE = regexp.MustCompile(`GH_AW_([A-Z0-9_]+)_[0-9a-f]{16}_EOF`)

// normalizeHeredocDelimiters replaces randomized heredoc delimiter tokens with a stable
// placeholder so that two compilations of the same workflow compare as equal even though
// each run embeds different random tokens.
func normalizeHeredocDelimiters(content string) string {
// Fast path: skip regex if content contains no heredoc delimiters
if !strings.Contains(content, "GH_AW_") {
return content
}
return heredocDelimiterRE.ReplaceAllString(content, "GH_AW_${1}_NORM_EOF")
}

// ValidateHeredocContent checks that content does not contain the heredoc delimiter
// anywhere (substring match). The check is intentionally stricter than what shell
// heredocs require (delimiter on its own line) — rejecting any occurrence eliminates
Expand Down
67 changes: 67 additions & 0 deletions pkg/workflow/validation_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
// - validateMountStringFormat() - Parses and validates a "source:dest:mode" mount string
// - containsTrigger() - Reports whether an 'on:' section includes a named trigger
//
// # Type Conversion Helpers (any → []string)
//
// - parseStringSliceAny() - Canonical coercion of []string/[]any to []string; skips non-strings
// - toStringSlice() - Strict variant: returns error on non-string elements; also accepts bare string
// - extractStringSliceField() - Accepts string/[]string/[]any; skips empty strings; wraps bare string
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file-level doc says extractStringSliceField() “skips empty strings”, but the implementation only skips empty strings for string and []any inputs; for []string it returns the slice as-is (so empty-string elements are preserved). Either update the docs to match the actual behavior, or filter empty strings from the []string case as well so the documented behavior is correct.

Suggested change
// - extractStringSliceField() - Accepts string/[]string/[]any; skips empty strings; wraps bare string
// - extractStringSliceField() - Accepts string/[]string/[]any; wraps bare string; skips empty strings only when coercing from string or []any

Copilot uses AI. Check for mistakes.
//
// # Design Rationale
//
// These helpers consolidate 76+ duplicate validation patterns identified in the
Expand Down Expand Up @@ -189,6 +195,67 @@ func parseStringSliceAny(raw any, log *logger.Logger) []string {
}
}

// toStringSlice converts an any value to a []string, supporting []string, []any, and string.
// Unlike parseStringSliceAny, this function returns an error when a []any element is not a string,
// and also accepts a bare string value (wrapping it in a single-element slice).
func toStringSlice(val any) ([]string, error) {
switch v := val.(type) {
case []string:
return v, nil
case []any:
result := make([]string, 0, len(v))
for _, item := range v {
s, ok := item.(string)
if !ok {
return nil, errors.New("non-string item in list")
}
result = append(result, s)
}
return result, nil
case string:
return []string{v}, nil
default:
return nil, fmt.Errorf("unsupported type %T", val)
}
}

// extractStringSliceField extracts a string slice from various input formats.
// Handles: string, []string, []any (with string elements).
// Returns nil if the input is empty or invalid.
func extractStringSliceField(value any, fieldName string) []string {
switch v := value.(type) {
case string:
// Single string
if v == "" {
return nil
}
validationHelpersLog.Printf("Extracted single %s: %s", fieldName, v)
return []string{v}
Comment on lines +232 to +233
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractStringSliceField now logs via validationHelpersLog, but previously (when this helper lived in role_checks.go) it logged via roleLog. That changes log categorization/output, which is observable behavior and conflicts with the PR description’s “No behavioral changes”. Consider taking a logger as a parameter (or otherwise preserving the previous logger) if you want to keep log output stable, or update the PR description to acknowledge the logging change.

Copilot uses AI. Check for mistakes.
case []string:
// Already a string slice
if len(v) == 0 {
return nil
}
validationHelpersLog.Printf("Extracted %d %s: %v", len(v), fieldName, v)
return v
case []any:
// Array of any - extract strings
var result []string
for _, item := range v {
if str, ok := item.(string); ok && str != "" {
result = append(result, str)
}
}
if len(result) == 0 {
return nil
}
validationHelpersLog.Printf("Extracted %d %s from array: %v", len(result), fieldName, result)
return result
}
validationHelpersLog.Printf("No valid %s found or unsupported type: %T", fieldName, value)
return nil
}

// validateNoDuplicateIDs checks that all items have unique IDs extracted by idFunc.
// The onDuplicate callback creates the error to return when a duplicate is found.
func validateNoDuplicateIDs[T any](items []T, idFunc func(T) string, onDuplicate func(string) error) error {
Expand Down
Loading