refactor: eliminate near-duplicate string utilities, promote general-purpose helpers to pkg/stringutil

pkg/stringutil/stringutil.go

@@ -58,6 +58,78 @@ func ParseVersionValue(version any) string {
 	}
 }
 
+// FormatList formats a slice of strings as a natural-language comma-separated list
+// with an Oxford comma and "and" before the final item.
+//
+// Examples:
+//
+//	FormatList([]string{})              // returns ""
+//	FormatList([]string{"a"})           // returns "a"
+//	FormatList([]string{"a", "b"})      // returns "a and b"
+//	FormatList([]string{"a", "b", "c"}) // returns "a, b, and c"
+func FormatList(items []string) string {
+	switch len(items) {
+	case 0:
+		return ""
+	case 1:
+		return items[0]
+	case 2:
+		return items[0] + " and " + items[1]
+	default:
+		return strings.Join(items[:len(items)-1], ", ") + ", and " + items[len(items)-1]
+	}
+}
+
+// NormalizeLeadingWhitespace removes consistent leading whitespace from all lines
+// of a multi-line string. It finds the minimum indentation across all non-empty
+// lines and strips that many leading whitespace characters (spaces or tabs) from
+// every line.
+//
+// This is useful for cleaning up content generated with extra indentation,
+// such as heredoc bodies.
+func NormalizeLeadingWhitespace(content string) string {
+	lines := strings.Split(content, "\n")
+	if len(lines) == 0 {
+		return content
+	}
+
+	// Find minimum leading whitespace (excluding empty lines)
+	minLeading := -1
+	for _, line := range lines {
+		if strings.TrimSpace(line) == "" {
+			continue // Skip empty lines
+		}
+		leading := len(line) - len(strings.TrimLeft(line, " \t"))
+		if minLeading == -1 || leading < minLeading {
+			minLeading = leading
+		}
+	}
+
+	// If no content or no leading whitespace, return as-is
+	if minLeading <= 0 {
+		return content
+	}
+
+	// Remove the minimum leading whitespace from all lines
+	var result strings.Builder
+	for i, line := range lines {
+		if i > 0 {
+			result.WriteString("\n")
+		}
+		if strings.TrimSpace(line) == "" {
+			// Keep empty lines as empty
+			result.WriteString("")
+		} else if len(line) >= minLeading {
+			// Remove leading whitespace
+			result.WriteString(line[minLeading:])
+		} else {
+			result.WriteString(line)
+		}
+	}
+
+	return result.String()
+}
+
 // IsPositiveInteger checks if a string is a positive integer.
 // Returns true for strings like "1", "123", "999" but false for:
 //   - Zero ("0")

pkg/stringutil/stringutil_test.go

@@ -414,6 +414,102 @@ func TestParseVersionValue(t *testing.T) {
 	}
 }
 
+func TestFormatList(t *testing.T) {
+	tests := []struct {
+		name     string
+		items    []string
+		expected string
+	}{
+		{
+			name:     "empty slice",
+			items:    []string{},
+			expected: "",
+		},
+		{
+			name:     "single item",
+			items:    []string{"a"},
+			expected: "a",
+		},
+		{
+			name:     "two items",
+			items:    []string{"a", "b"},
+			expected: "a and b",
+		},
+		{
+			name:     "three items",
+			items:    []string{"a", "b", "c"},
+			expected: "a, b, and c",
+		},
+		{
+			name:     "four items",
+			items:    []string{"a", "b", "c", "d"},
+			expected: "a, b, c, and d",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := FormatList(tt.items)
+			if result != tt.expected {
+				t.Errorf("FormatList(%v) = %q; want %q", tt.items, result, tt.expected)
+			}
+		})
+	}
+}
+
+func TestNormalizeLeadingWhitespace(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "removes consistent leading spaces",
+			input:    "          Line 1\n          Line 2\n          Line 3",
+			expected: "Line 1\nLine 2\nLine 3",
+		},
+		{
+			name:     "handles no leading spaces",
+			input:    "Line 1\nLine 2",
+			expected: "Line 1\nLine 2",
+		},
+		{
+			name:     "preserves relative indentation",
+			input:    "          Line 1\n            Indented Line 2\n          Line 3",
+			expected: "Line 1\n  Indented Line 2\nLine 3",
+		},
+		{
+			name:     "handles empty lines",
+			input:    "          Line 1\n\n          Line 3",
+			expected: "Line 1\n\nLine 3",
+		},
+		{
+			name:     "empty string",
+			input:    "",
+			expected: "",
+		},
+		{
+			name:     "removes consistent leading tabs",
+			input:    "\t\tLine 1\n\t\tLine 2\n\t\tLine 3",
+			expected: "Line 1\nLine 2\nLine 3",
+		},
+		{
+			name:     "removes consistent mixed tab and space indentation",
+			input:    "\t  Line 1\n\t  Line 2\n\t  Line 3",
+			expected: "Line 1\nLine 2\nLine 3",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := NormalizeLeadingWhitespace(tt.input)
+			if result != tt.expected {
+				t.Errorf("NormalizeLeadingWhitespace(%q) = %q; want %q", tt.input, result, tt.expected)
+			}
+		})
+	}
+}
+
 func TestIsPositiveInteger(t *testing.T) {
 	tests := []struct {
 		name string

pkg/workflow/compiler_experiments.go

@@ -183,7 +183,7 @@ func extractOneExperimentConfig(name string, val any) *ExperimentConfig {
 			cfg.Hypothesis = h
 		}
 		if smRaw, ok := v["secondary_metrics"]; ok {
-			cfg.SecondaryMetrics = extractStringSlice(smRaw)
+			cfg.SecondaryMetrics = parseStringSliceAny(smRaw, nil)
 		}
 		if gmRaw, ok := v["guardrail_metrics"]; ok {
 			cfg.GuardrailMetrics = extractGuardrailMetrics(gmRaw)
@@ -195,7 +195,7 @@ func extractOneExperimentConfig(name string, val any) *ExperimentConfig {
 			cfg.AnalysisType = at
 		}
 		if tagsRaw, ok := v["tags"]; ok {
-			cfg.Tags = extractStringSlice(tagsRaw)
+			cfg.Tags = parseStringSliceAny(tagsRaw, nil)
 		}
 		if notifyRaw, ok := v["notify"]; ok {
 			if notifyMap, ok := notifyRaw.(map[string]any); ok {
@@ -250,23 +250,6 @@ func extractIntField(val any) (int, bool) {
 	return 0, false
 }
 
-// extractStringSlice converts a raw value to a []string, accepting []any of string values.
-func extractStringSlice(raw any) []string {
-	switch v := raw.(type) {
-	case []string:
-		return v
-	case []any:
-		var result []string
-		for _, item := range v {
-			if s, ok := item.(string); ok {
-				result = append(result, s)
-			}
-		}
-		return result
-	}
-	return nil
-}
-
 // extractGuardrailMetrics converts a raw guardrail_metrics value into a []GuardrailMetric.
 // Each entry must be a map with "name" and "threshold" string fields.
 func extractGuardrailMetrics(raw any) []GuardrailMetric {

pkg/workflow/strings.go

@@ -413,17 +413,3 @@ func SanitizeArtifactIdentifier(name string) string {
 	}
 	return result
 }
-
-// formatList formats a list of strings as a comma-separated list with natural language conjunction
-func formatList(items []string) string {
-	if len(items) == 0 {
-		return ""
-	}
-	if len(items) == 1 {
-		return items[0]
-	}
-	if len(items) == 2 {
-		return items[0] + " and " + items[1]
-	}
-	return fmt.Sprintf("%s, and %s", formatList(items[:len(items)-1]), items[len(items)-1])
-}

pkg/workflow/tools_validation_github_toolsets.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/parser"
+	"github.com/github/gh-aw/pkg/stringutil"
 )
 
 func validateGitHubToolsAgainstToolsetsImpl(allowedTools []string, enabledToolsets []string) error {
@@ -74,7 +75,7 @@ func validateGitHubToolsAgainstToolsetsImpl(allowedTools []string, enabledToolse
 	if len(unknownTools) > 0 {
 		githubToolToToolsetLog.Printf("Found %d unknown tools", len(unknownTools))
 		var errMsg strings.Builder
-		errMsg.WriteString(fmt.Sprintf("Unknown GitHub tool(s): %s\n\n", formatList(unknownTools)))
+		errMsg.WriteString(fmt.Sprintf("Unknown GitHub tool(s): %s\n\n", stringutil.FormatList(unknownTools)))
 
 		if len(suggestions) > 0 {
 			errMsg.WriteString("Did you mean:\n")
@@ -92,7 +93,7 @@ func validateGitHubToolsAgainstToolsetsImpl(allowedTools []string, enabledToolse
 		sort.Strings(validTools)
 
 		exampleCount := min(10, len(validTools))
-		errMsg.WriteString(fmt.Sprintf("Valid GitHub tools include: %s\n\n", formatList(validTools[:exampleCount])))
+		errMsg.WriteString(fmt.Sprintf("Valid GitHub tools include: %s\n\n", stringutil.FormatList(validTools[:exampleCount])))
 		errMsg.WriteString("See all tools: https://github.com/github/gh-aw/blob/main/pkg/workflow/data/github_tool_to_toolset.json")
 
 		return errors.New(errMsg.String())

pkg/workflow/unified_prompt_step.go

@@ -25,51 +25,6 @@ type PromptSection struct {
 	EnvVars map[string]string
 }
 
-// normalizeLeadingWhitespace removes consistent leading whitespace from all lines
-// This handles content that was generated with indentation for heredocs
-func normalizeLeadingWhitespace(content string) string {
-	lines := strings.Split(content, "\n")
-	if len(lines) == 0 {
-		return content
-	}
-
-	// Find minimum leading whitespace (excluding empty lines)
-	minLeadingSpaces := -1
-	for _, line := range lines {
-		if strings.TrimSpace(line) == "" {
-			continue // Skip empty lines
-		}
-		leadingSpaces := len(line) - len(strings.TrimLeft(line, " "))
-		if minLeadingSpaces == -1 || leadingSpaces < minLeadingSpaces {
-			minLeadingSpaces = leadingSpaces
-		}
-	}
-
-	// If no content or no leading spaces, return as-is
-	if minLeadingSpaces <= 0 {
-		return content
-	}
-
-	// Remove the minimum leading whitespace from all lines
-	var result strings.Builder
-	for i, line := range lines {
-		if i > 0 {
-			result.WriteString("\n")
-		}
-		if strings.TrimSpace(line) == "" {
-			// Keep empty lines as empty
-			result.WriteString("")
-		} else if len(line) >= minLeadingSpaces {
-			// Remove leading whitespace
-			result.WriteString(line[minLeadingSpaces:])
-		} else {
-			result.WriteString(line)
-		}
-	}
-
-	return result.String()
-}
-
 // removeConsecutiveEmptyLines removes consecutive empty lines, keeping only one
 func removeConsecutiveEmptyLines(content string) string {
 	lines := strings.Split(content, "\n")
@@ -440,7 +395,7 @@ func (c *Compiler) generateUnifiedPromptCreationStep(yaml *strings.Builder, buil
 			} else {
 				// Inline content inside conditional - open heredoc, write content, close
 				yaml.WriteString("            cat << '" + delimiter + "'\n")
-				normalizedContent := normalizeLeadingWhitespace(section.Content)
+				normalizedContent := stringutil.NormalizeLeadingWhitespace(section.Content)
 				cleanedContent := removeConsecutiveEmptyLines(normalizedContent)
 				contentLines := strings.SplitSeq(cleanedContent, "\n")
 				for line := range contentLines {
@@ -480,7 +435,7 @@ func (c *Compiler) generateUnifiedPromptCreationStep(yaml *strings.Builder, buil
 					}
 				}
 				// Write content directly to open heredoc
-				normalizedContent := normalizeLeadingWhitespace(section.Content)
+				normalizedContent := stringutil.NormalizeLeadingWhitespace(section.Content)
 				cleanedContent := removeConsecutiveEmptyLines(normalizedContent)
 				contentLines := strings.SplitSeq(cleanedContent, "\n")
 				for line := range contentLines {

pkg/workflow/unified_prompt_step_test.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/github/gh-aw/pkg/stringutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -135,7 +136,7 @@ Line 3`,
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := normalizeLeadingWhitespace(tt.input)
+			result := stringutil.NormalizeLeadingWhitespace(tt.input)
 			assert.Equal(t, tt.expected, result)
 		})
 	}