refactor: eliminate duplicate utilities and trivial alias functions

pkg/cli/compile_external_tools.go

@@ -64,32 +64,28 @@ func runBatchLockFileTool(toolName string, lockFiles []string, verbose bool, str
 
 	compileExternalToolsLog.Printf("Running batch %s on %d lock files", toolName, len(lockFiles))
 
-	if err := runner(lockFiles, verbose, strict); err != nil {
-		if strict {
-			return fmt.Errorf("%s failed: %w", toolName, err)
-		}
-		// In non-strict mode, errors are warnings
-		if verbose {
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("%s warnings: %v", toolName, err)))
-		}
-	}
-
-	return nil
+	return handleBatchToolError(toolName, runner(lockFiles, verbose, strict), strict, verbose)
 }
 
 // runBatchDirectoryTool runs a directory-based batch tool with uniform error handling
 func runBatchDirectoryTool(toolName string, workflowDir string, verbose bool, strict bool, runner func(string, bool, bool) error) error {
 	compileExternalToolsLog.Printf("Running batch %s on directory: %s", toolName, workflowDir)
 
-	if err := runner(workflowDir, verbose, strict); err != nil {
-		if strict {
-			return fmt.Errorf("%s failed: %w", toolName, err)
-		}
-		// In non-strict mode, errors are warnings
-		if verbose {
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("%s warnings: %v", toolName, err)))
-		}
-	}
+	return handleBatchToolError(toolName, runner(workflowDir, verbose, strict), strict, verbose)
+}
 
+// handleBatchToolError applies uniform strict/non-strict error handling for batch tool results.
+// In strict mode, errors are returned wrapped. In non-strict mode, errors are logged as warnings.
+func handleBatchToolError(toolName string, err error, strict, verbose bool) error {
+	if err == nil {
+		return nil
+	}
+	if strict {
+		return fmt.Errorf("%s failed: %w", toolName, err)
+	}
+	// In non-strict mode, errors are warnings
+	if verbose {
+		fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("%s warnings: %v", toolName, err)))
+	}
 	return nil
 }

pkg/workflow/compiler_pre_activation_job.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/sliceutil"
 	"github.com/github/gh-aw/pkg/stringutil"
 )
 
@@ -476,7 +477,7 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec
 		Permissions: permissions,
 		Steps:       steps,
 		Outputs:     outputs,
-		Needs:       dedupeStringSlice(data.OnNeeds),
+		Needs:       sliceutil.Deduplicate(data.OnNeeds),
 	}
 
 	return job, nil
@@ -836,24 +837,7 @@ func parseOnNeedsValues(onMap map[string]any) ([]string, error) {
 		result = append(result, needStr)
 	}
 
-	return dedupeStringSlice(result), nil
-}
-
-func dedupeStringSlice(values []string) []string {
-	if len(values) == 0 {
-		return nil
-	}
-
-	seen := make(map[string]bool, len(values))
-	result := make([]string, 0, len(values))
-	for _, v := range values {
-		if seen[v] {
-			continue
-		}
-		seen[v] = true
-		result = append(result, v)
-	}
-	return result
+	return sliceutil.Deduplicate(result), nil
 }
 
 // referencesPreActivationOutputs returns true if the condition references the pre_activation job's

pkg/workflow/known_needs_expressions.go

@@ -44,7 +44,7 @@ func generateKnownNeedsExpressions(data *WorkflowData, preActivationJobCreated b
 		activatedExpr := fmt.Sprintf("needs.%s.outputs.%s", constants.PreActivationJobName, constants.ActivatedOutput)
 		activatedEnvVar := fmt.Sprintf("GH_AW_NEEDS_%s_OUTPUTS_%s",
 			normalizeJobNameForEnvVar(string(constants.PreActivationJobName)),
-			normalizeOutputNameForEnvVar(constants.ActivatedOutput))
+			normalizeJobNameForEnvVar(constants.ActivatedOutput))
 		mappings = append(mappings, &ExpressionMapping{
 			Original: fmt.Sprintf("${{ %s }}", activatedExpr),
 			EnvVar:   activatedEnvVar,
@@ -57,7 +57,7 @@ func generateKnownNeedsExpressions(data *WorkflowData, preActivationJobCreated b
 			matchedCmdExpr := fmt.Sprintf("needs.%s.outputs.%s", constants.PreActivationJobName, constants.MatchedCommandOutput)
 			matchedCmdEnvVar := fmt.Sprintf("GH_AW_NEEDS_%s_OUTPUTS_%s",
 				normalizeJobNameForEnvVar(string(constants.PreActivationJobName)),
-				normalizeOutputNameForEnvVar(constants.MatchedCommandOutput))
+				normalizeJobNameForEnvVar(constants.MatchedCommandOutput))
 			mappings = append(mappings, &ExpressionMapping{
 				Original: fmt.Sprintf("${{ %s }}", matchedCmdExpr),
 				EnvVar:   matchedCmdEnvVar,
@@ -96,7 +96,7 @@ func generateKnownNeedsExpressions(data *WorkflowData, preActivationJobCreated b
 				expr := fmt.Sprintf("needs.%s.outputs.%s", jobName, output)
 				envVar := fmt.Sprintf("GH_AW_NEEDS_%s_OUTPUTS_%s",
 					normalizeJobNameForEnvVar(jobName),
-					normalizeOutputNameForEnvVar(output))
+					normalizeJobNameForEnvVar(output))
 				mappings = append(mappings, &ExpressionMapping{
 					Original: fmt.Sprintf("${{ %s }}", expr),
 					EnvVar:   envVar,
@@ -178,12 +178,6 @@ func normalizeJobNameForEnvVar(jobName string) string {
 	return result.String()
 }
 
-// normalizeOutputNameForEnvVar converts an output name to a valid environment variable segment
-// Examples: "text" -> "TEXT", "comment_id" -> "COMMENT_ID"
-func normalizeOutputNameForEnvVar(outputName string) string {
-	return normalizeJobNameForEnvVar(outputName)
-}
-
 // getCustomJobsBeforeActivation returns a list of custom job names that run before the activation job
 // A custom job runs before activation ONLY if it explicitly depends on pre_activation
 // Note: Jobs without explicit 'needs' will automatically get 'needs: activation' added by the compiler,

pkg/workflow/known_needs_expressions_test.go

@@ -185,25 +185,6 @@ func TestNormalizeJobNameForEnvVar(t *testing.T) {
 	}
 }
 
-func TestNormalizeOutputNameForEnvVar(t *testing.T) {
-	tests := []struct {
-		input    string
-		expected string
-	}{
-		{"text", "TEXT"},
-		{"comment_id", "COMMENT_ID"},
-		{"issue_url", "ISSUE_URL"},
-		{"output_types", "OUTPUT_TYPES"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.input, func(t *testing.T) {
-			result := normalizeOutputNameForEnvVar(tt.input)
-			assert.Equal(t, tt.expected, result, "Output name normalization failed")
-		})
-	}
-}
-
 func TestGetCustomJobsBeforeActivation(t *testing.T) {
 	tests := []struct {
 		name         string