Skip to content

Commit 5bf3d40

Browse files
Copilotpelikhan
andauthored
refactor(workflow): co-locate heredoc utils and consolidate any→[]string helpers (#26428)
* Initial plan * refactor(workflow): consolidate any→[]string helpers and move normalizeHeredocDelimiters to strings.go Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3f255865-2b14-4042-95a3-b72d8d7b1fd5 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent 31ef7c6 commit 5bf3d40

5 files changed

Lines changed: 82 additions & 76 deletions

File tree

pkg/workflow/compiler.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"os"
88
"path/filepath"
9-
"regexp"
109
"slices"
1110
"strings"
1211
"time"
@@ -20,21 +19,6 @@ import (
2019

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

23-
// heredocDelimiterRE matches randomized heredoc delimiters of the form GH_AW_<NAME>_<16hexchars>_EOF.
24-
// Used to normalize delimiters when comparing compiled output to skip unnecessary writes.
25-
var heredocDelimiterRE = regexp.MustCompile(`GH_AW_([A-Z0-9_]+)_[0-9a-f]{16}_EOF`)
26-
27-
// normalizeHeredocDelimiters replaces randomized heredoc delimiter tokens with a stable
28-
// placeholder so that two compilations of the same workflow compare as equal even though
29-
// each run embeds different random tokens.
30-
func normalizeHeredocDelimiters(content string) string {
31-
// Fast path: skip regex if content contains no heredoc delimiters
32-
if !strings.Contains(content, "GH_AW_") {
33-
return content
34-
}
35-
return heredocDelimiterRE.ReplaceAllString(content, "GH_AW_${1}_NORM_EOF")
36-
}
37-
3822
const (
3923
// MaxLockFileSize is the maximum allowed size for generated lock workflow files (500KB)
4024
MaxLockFileSize = 512000 // 500KB in bytes

pkg/workflow/compiler_filters_validation.go

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
package workflow
4747

4848
import (
49-
"errors"
5049
"fmt"
5150
"strings"
5251
)
@@ -204,25 +203,3 @@ func validateGlobList(eventMap map[string]any, eventName, filterKey string, isPa
204203
}
205204
return nil
206205
}
207-
208-
// toStringSlice converts an any value to a []string, supporting []string, []any, and string.
209-
func toStringSlice(val any) ([]string, error) {
210-
switch v := val.(type) {
211-
case []string:
212-
return v, nil
213-
case []any:
214-
result := make([]string, 0, len(v))
215-
for _, item := range v {
216-
s, ok := item.(string)
217-
if !ok {
218-
return nil, errors.New("non-string item in list")
219-
}
220-
result = append(result, s)
221-
}
222-
return result, nil
223-
case string:
224-
return []string{v}, nil
225-
default:
226-
return nil, fmt.Errorf("unsupported type %T", val)
227-
}
228-
}

pkg/workflow/role_checks.go

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -504,43 +504,6 @@ func (c *Compiler) extractSkipBots(frontmatter map[string]any) []string {
504504
return extractSkipField(frontmatter, "skip-bots")
505505
}
506506

507-
// extractStringSliceField extracts a string slice from various input formats
508-
// Handles: string, []string, []any (with string elements)
509-
// Returns nil if the input is empty or invalid
510-
func extractStringSliceField(value any, fieldName string) []string {
511-
switch v := value.(type) {
512-
case string:
513-
// Single string
514-
if v == "" {
515-
return nil
516-
}
517-
roleLog.Printf("Extracted single %s: %s", fieldName, v)
518-
return []string{v}
519-
case []string:
520-
// Already a string slice
521-
if len(v) == 0 {
522-
return nil
523-
}
524-
roleLog.Printf("Extracted %d %s: %v", len(v), fieldName, v)
525-
return v
526-
case []any:
527-
// Array of any - extract strings
528-
var result []string
529-
for _, item := range v {
530-
if str, ok := item.(string); ok && str != "" {
531-
result = append(result, str)
532-
}
533-
}
534-
if len(result) == 0 {
535-
return nil
536-
}
537-
roleLog.Printf("Extracted %d %s from array: %v", len(result), fieldName, result)
538-
return result
539-
}
540-
roleLog.Printf("No valid %s found or unsupported type: %T", fieldName, value)
541-
return nil
542-
}
543-
544507
// mergeStringSlicesDedup merges two string slices with deduplication (preserving order of first occurrence).
545508
// Logs the merged result under the given logLabel when the result is non-empty.
546509
func mergeStringSlicesDedup(top, imported []string, logLabel string) []string {

pkg/workflow/strings.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,21 @@ func GenerateHeredocDelimiterFromSeed(name string, seed string) string {
297297
return "GH_AW_" + upperName + "_" + tag + "_EOF"
298298
}
299299

300+
// heredocDelimiterRE matches randomized heredoc delimiters of the form GH_AW_<NAME>_<16hexchars>_EOF.
301+
// Used to normalize delimiters when comparing compiled output to skip unnecessary writes.
302+
var heredocDelimiterRE = regexp.MustCompile(`GH_AW_([A-Z0-9_]+)_[0-9a-f]{16}_EOF`)
303+
304+
// normalizeHeredocDelimiters replaces randomized heredoc delimiter tokens with a stable
305+
// placeholder so that two compilations of the same workflow compare as equal even though
306+
// each run embeds different random tokens.
307+
func normalizeHeredocDelimiters(content string) string {
308+
// Fast path: skip regex if content contains no heredoc delimiters
309+
if !strings.Contains(content, "GH_AW_") {
310+
return content
311+
}
312+
return heredocDelimiterRE.ReplaceAllString(content, "GH_AW_${1}_NORM_EOF")
313+
}
314+
300315
// ValidateHeredocContent checks that content does not contain the heredoc delimiter
301316
// anywhere (substring match). The check is intentionally stricter than what shell
302317
// heredocs require (delimiter on its own line) — rejecting any occurrence eliminates

pkg/workflow/validation_helpers.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@
1111
// - validateMountStringFormat() - Parses and validates a "source:dest:mode" mount string
1212
// - containsTrigger() - Reports whether an 'on:' section includes a named trigger
1313
//
14+
// # Type Conversion Helpers (any → []string)
15+
//
16+
// - parseStringSliceAny() - Canonical coercion of []string/[]any to []string; skips non-strings
17+
// - toStringSlice() - Strict variant: returns error on non-string elements; also accepts bare string
18+
// - extractStringSliceField() - Accepts string/[]string/[]any; skips empty strings; wraps bare string
19+
//
1420
// # Design Rationale
1521
//
1622
// These helpers consolidate 76+ duplicate validation patterns identified in the
@@ -189,6 +195,67 @@ func parseStringSliceAny(raw any, log *logger.Logger) []string {
189195
}
190196
}
191197

198+
// toStringSlice converts an any value to a []string, supporting []string, []any, and string.
199+
// Unlike parseStringSliceAny, this function returns an error when a []any element is not a string,
200+
// and also accepts a bare string value (wrapping it in a single-element slice).
201+
func toStringSlice(val any) ([]string, error) {
202+
switch v := val.(type) {
203+
case []string:
204+
return v, nil
205+
case []any:
206+
result := make([]string, 0, len(v))
207+
for _, item := range v {
208+
s, ok := item.(string)
209+
if !ok {
210+
return nil, errors.New("non-string item in list")
211+
}
212+
result = append(result, s)
213+
}
214+
return result, nil
215+
case string:
216+
return []string{v}, nil
217+
default:
218+
return nil, fmt.Errorf("unsupported type %T", val)
219+
}
220+
}
221+
222+
// extractStringSliceField extracts a string slice from various input formats.
223+
// Handles: string, []string, []any (with string elements).
224+
// Returns nil if the input is empty or invalid.
225+
func extractStringSliceField(value any, fieldName string) []string {
226+
switch v := value.(type) {
227+
case string:
228+
// Single string
229+
if v == "" {
230+
return nil
231+
}
232+
validationHelpersLog.Printf("Extracted single %s: %s", fieldName, v)
233+
return []string{v}
234+
case []string:
235+
// Already a string slice
236+
if len(v) == 0 {
237+
return nil
238+
}
239+
validationHelpersLog.Printf("Extracted %d %s: %v", len(v), fieldName, v)
240+
return v
241+
case []any:
242+
// Array of any - extract strings
243+
var result []string
244+
for _, item := range v {
245+
if str, ok := item.(string); ok && str != "" {
246+
result = append(result, str)
247+
}
248+
}
249+
if len(result) == 0 {
250+
return nil
251+
}
252+
validationHelpersLog.Printf("Extracted %d %s from array: %v", len(result), fieldName, result)
253+
return result
254+
}
255+
validationHelpersLog.Printf("No valid %s found or unsupported type: %T", fieldName, value)
256+
return nil
257+
}
258+
192259
// validateNoDuplicateIDs checks that all items have unique IDs extracted by idFunc.
193260
// The onDuplicate callback creates the error to return when a duplicate is found.
194261
func validateNoDuplicateIDs[T any](items []T, idFunc func(T) string, onDuplicate func(string) error) error {

0 commit comments

Comments
 (0)