refactor: consolidate duplicate helpers across pkg/workflow, pkg/cli, pkg/parser

pkg/cli/codemod_effective_tokens_to_ai_credits.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/stringutil"
 	"github.com/github/gh-aw/pkg/typeutil"
 )
 
@@ -112,7 +113,7 @@ func normalizeLegacyBudgetValue(raw any, allowNegativeOne bool) (string, bool) {
 	}
 
 	trimmed := strings.TrimSpace(rawStr)
-	if trimmed == "" || isExpressionValue(trimmed) {
+	if trimmed == "" || stringutil.ContainsExpression(trimmed) {
 		return "", false
 	}
 	if allowNegativeOne && trimmed == "-1" {
@@ -137,10 +138,6 @@ func convertEffectiveTokensToAICredits(effectiveTokens int) (string, bool) {
 	return strconv.Itoa(aiCredits), true
 }
 
-func isExpressionValue(value string) bool {
-	return strings.Contains(value, "${{") && strings.Contains(value, "}}")
-}
-
 func rewriteTopLevelScalarLine(line, newKey, normalizedValue string) string {
 	indent := getIndentation(line)
 	parts := strings.SplitN(line, ":", 2)

pkg/cli/codemod_steps_run_secrets_env.go

@@ -152,15 +152,15 @@ func transformStepsWithinSection(sectionLines []string, sectionIndent string) ([
 
 func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string, bool) {
 	modified := false
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 	orderedBindings := make([]string, 0)
 	bindingExprs := make(map[string]string)
 	firstRunLine := -1
 	envStart := -1
 	envEnd := -1
 	envIndent := ""
 	var envKeyIndentLen int
-	existingEnvKeys := make(map[string]bool)
+	existingEnvKeys := make(map[string]struct{})
 
 	// First pass: detect shell type so PowerShell steps get $env:VARNAME syntax.
 	// Restrict the scan to lines at the direct step-key indentation level so
@@ -209,7 +209,7 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string
 				envEnd = j
 				key := parseYAMLMapKey(t)
 				if key != "" {
-					existingEnvKeys[key] = true
+					existingEnvKeys[key] = struct{}{}
 				}
 			}
 		}
@@ -244,8 +244,8 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string
 					modified = true
 				}
 				for _, binding := range bindings {
-					if !seen[binding.Name] {
-						seen[binding.Name] = true
+					if _, ok := seen[binding.Name]; !ok {
+						seen[binding.Name] = struct{}{}
 						orderedBindings = append(orderedBindings, binding.Name)
 						bindingExprs[binding.Name] = binding.Expression
 					}
@@ -260,8 +260,8 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string
 			modified = true
 		}
 		for _, binding := range bindings {
-			if !seen[binding.Name] {
-				seen[binding.Name] = true
+			if _, ok := seen[binding.Name]; !ok {
+				seen[binding.Name] = struct{}{}
 				orderedBindings = append(orderedBindings, binding.Name)
 				bindingExprs[binding.Name] = binding.Expression
 			}
@@ -276,7 +276,7 @@ func rewriteStepRunSecretsToEnv(stepLines []string, stepIndent string) ([]string
 
 	missingBindings := make([]string, 0, len(orderedBindings))
 	for _, name := range orderedBindings {
-		if !existingEnvKeys[name] {
+		if _, ok := existingEnvKeys[name]; !ok {
 			missingBindings = append(missingBindings, name)
 		}
 	}

pkg/cli/compile_workflow_processor.go

@@ -197,12 +197,15 @@ func extractSafeOutputLabels(data *workflow.WorkflowData) []string {
 		return nil
 	}
 
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 	var labels []string
 
 	addLabel := func(label string) {
-		if label != "" && !seen[label] {
-			seen[label] = true
+		if label == "" {
+			return
+		}
+		if _, ok := seen[label]; !ok {
+			seen[label] = struct{}{}
 			labels = append(labels, label)
 		}
 	}

pkg/cli/drain3_integration.go

@@ -355,12 +355,12 @@ func buildAnomalyReasons(anomalies []struct {
 	report *agentdrain.AnomalyReport
 }) string {
 	reasons := make([]string, 0, len(anomalies))
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 	for _, a := range anomalies {
 		r := fmt.Sprintf("stage=%s score=%.2f: %s", a.evt.Stage, a.report.AnomalyScore, a.report.Reason)
-		if !seen[r] {
+		if _, ok := seen[r]; !ok {
 			reasons = append(reasons, r)
-			seen[r] = true
+			seen[r] = struct{}{}
 		}
 		if len(reasons) >= 5 {
 			break

pkg/cli/experiments_command.go

@@ -494,18 +494,21 @@ func fetchLocalExperiments() ([]ExperimentInfo, error) {
 		return nil, fmt.Errorf("failed to list experiment branches: %w", err)
 	}
 
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 	var experiments []ExperimentInfo
 
 	for line := range strings.SplitSeq(strings.TrimSpace(string(output)), "\n") {
 		if line == "" {
 			continue
 		}
 		workflowID := extractExperimentName(line)
-		if workflowID == "" || seen[workflowID] {
+		if workflowID == "" {
 			continue
 		}
-		seen[workflowID] = true
+		if _, ok := seen[workflowID]; ok {
+			continue
+		}
+		seen[workflowID] = struct{}{}
 
 		branchName := experimentsBranchPrefix + workflowID
 		// Prefer remote ref; fall back to local.

pkg/cli/generate_action_metadata_command.go

@@ -214,22 +214,22 @@ func generateHumanReadableName(actionName string) string {
 // extractInputs extracts input parameters from core.getInput() calls
 func extractInputs(content string) []ActionInput {
 	var inputs []ActionInput
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 
 	// Match core.getInput('name') or core.getInput("name")
 	matches := coreGetInputPattern.FindAllStringSubmatch(content, -1)
 
 	for _, match := range matches {
 		if len(match) > 1 {
 			inputName := match[1]
-			if !seen[inputName] {
+			if _, ok := seen[inputName]; !ok {
 				inputs = append(inputs, ActionInput{
 					Name:        inputName,
 					Description: "Input parameter: " + inputName,
 					Required:    false,
 					Default:     "",
 				})
-				seen[inputName] = true
+				seen[inputName] = struct{}{}
 			}
 		}
 	}
@@ -252,20 +252,20 @@ func extractInputs(content string) []ActionInput {
 // extractOutputs extracts output parameters from core.setOutput() calls
 func extractOutputs(content string) []ActionOutput {
 	var outputs []ActionOutput
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 
 	// Match core.setOutput('name', ...) or core.setOutput("name", ...)
 	matches := coreSetOutputPattern.FindAllStringSubmatch(content, -1)
 
 	for _, match := range matches {
 		if len(match) > 1 {
 			outputName := match[1]
-			if !seen[outputName] {
+			if _, ok := seen[outputName]; !ok {
 				outputs = append(outputs, ActionOutput{
 					Name:        outputName,
 					Description: "Output parameter: " + outputName,
 				})
-				seen[outputName] = true
+				seen[outputName] = struct{}{}
 			}
 		}
 	}
@@ -288,17 +288,17 @@ func extractOutputs(content string) []ActionOutput {
 // extractDependencies extracts require() dependencies
 func extractDependencies(content string) []string {
 	var deps []string
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 
 	// Match require('./filename.cjs') or require("./filename.cjs")
 	matches := requireCJSPattern.FindAllStringSubmatch(content, -1)
 
 	for _, match := range matches {
 		if len(match) > 1 {
 			dep := match[1]
-			if !seen[dep] {
+			if _, ok := seen[dep]; !ok {
 				deps = append(deps, dep)
-				seen[dep] = true
+				seen[dep] = struct{}{}
 			}
 		}
 	}

pkg/cli/includes.go

@@ -154,7 +154,7 @@ func fetchAndSaveRemoteFrontmatterImports(content string, spec *WorkflowSpec, ta
 	// seen is keyed by fully-resolved remote file path. It is shared across all recursion
 	// levels so that every import (at any depth) is downloaded at most once and import
 	// cycles (A imports B, B imports A) are broken without infinite recursion.
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 	fetchFrontmatterImportsRecursive(content, workflowBaseDir, frontmatterImportsOpts{
 		owner:           owner,
 		repo:            repo,
@@ -180,7 +180,7 @@ type frontmatterImportsOpts struct {
 	verbose         bool
 	force           bool
 	tracker         *FileTracker
-	seen            map[string]bool
+	seen            map[string]struct{}
 }
 
 // fetchFrontmatterImportsRecursive is the internal worker for fetchAndSaveRemoteFrontmatterImports.
@@ -301,11 +301,11 @@ func fetchFrontmatterImportsRecursive(content, currentBaseDir string, opts front
 		}
 
 		// Cycle/duplicate prevention: use the fully-resolved remote path as the key.
-		if opts.seen[remoteFilePath] {
+		if _, ok := opts.seen[remoteFilePath]; ok {
 			remoteWorkflowLog.Printf("Skipping already-seen import: %s", remoteFilePath)
 			continue
 		}
-		opts.seen[remoteFilePath] = true
+		opts.seen[remoteFilePath] = struct{}{}
 
 		// Derive the local path relative to targetDir by stripping the original base-dir
 		// prefix from the remote path. This ensures that imports in nested files resolve
@@ -410,7 +410,7 @@ func fetchAndSaveRemoteIncludes(content string, spec *WorkflowSpec, targetDir st
 
 	// Parse the workflow content to find @include directives
 	scanner := bufio.NewScanner(strings.NewReader(content))
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 
 	for scanner.Scan() {
 		line := scanner.Text()
@@ -429,10 +429,10 @@ func fetchAndSaveRemoteIncludes(content string, spec *WorkflowSpec, targetDir st
 		}
 
 		// Skip if already processed
-		if seen[filePath] {
+		if _, ok := seen[filePath]; ok {
 			continue
 		}
-		seen[filePath] = true
+		seen[filePath] = struct{}{}
 
 		// Fetch the include file
 		includeContent, _, err := FetchIncludeFromSource(includePath, spec, verbose)

pkg/cli/logs_artifact_set.go

@@ -135,12 +135,12 @@ func ResolveArtifactFilter(sets []string) []string {
 		}
 	}
 
-	seen := make(map[string]bool)
+	seen := make(map[string]struct{})
 	var names []string
 	for _, s := range sets {
 		for _, name := range artifactSetArtifacts[ArtifactSet(s)] {
-			if !seen[name] {
-				seen[name] = true
+			if _, ok := seen[name]; !ok {
+				seen[name] = struct{}{}
 				names = append(names, name)
 			}
 		}

pkg/cli/logs_parsing_firewall.go

@@ -12,11 +12,7 @@ package cli
 import (
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
-	"strings"
-
-	"github.com/github/gh-aw/pkg/constants"
 
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/fileutil"
@@ -67,40 +63,15 @@ func parseFirewallLogs(runDir string, verbose bool) error {
 		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Found firewall logs in "+logsDir))
 	}
 
-	// Create a temporary directory for running the parser
-	tempDir, err := os.MkdirTemp("", "firewall_log_parser")
-	if err != nil {
-		return fmt.Errorf("failed to create temp dir: %w", err)
-	}
-	defer os.RemoveAll(tempDir)
-
-	// Create a Node.js script that mimics the GitHub Actions environment
-	// The firewall parser expects logs in /tmp/gh-aw/squid-logs-{workflow}/
-	// We'll set GITHUB_WORKFLOW to a value that makes the parser look in our temp directory
+	// Build a Node.js script that mimics the GitHub Actions environment.
+	// The firewall parser expects logs in /tmp/gh-aw/squid-logs-{workflow}/;
+	// we inject our own directory via a custom main wrapper.
 	nodeScript := fmt.Sprintf(`
 const fs = require('fs');
 const path = require('path');
 
 // Mock @actions/core for the parser
-const core = {
-	summary: {
-		addRaw: function(content) {
-			this._content = content;
-			return this;
-		},
-		write: function() {
-			console.log(this._content);
-		},
-		_content: ''
-	},
-	setFailed: function(message) {
-		console.error('FAILED:', message);
-		process.exit(1);
-	},
-	info: function(message) {
-		// Silent in CLI mode
-	}
-};
+%s
 
 // Set up environment
 // We'll use a custom workflow name that points to our temp directory
@@ -213,29 +184,8 @@ const originalMain = function() {
 
 // Replace main() call with our custom version
 originalMain();
-`, logsDir, jsScript)
+`, jsCoreMock, logsDir, jsScript)
 
-	// Write the Node.js script
-	nodeFile := filepath.Join(tempDir, "parser.js")
-	if err := os.WriteFile(nodeFile, []byte(nodeScript), constants.FilePermPublic); err != nil {
-		return fmt.Errorf("failed to write node script: %w", err)
-	}
-
-	// Execute the Node.js script using the absolute path for cross-platform compatibility
-	// #nosec G204 -- nodeFile is an absolute path to a script written by this process to tempDir;
-	// exec.Command with separate args (not shell execution) prevents shell injection.
-	cmd := exec.Command("node", nodeFile)
-	cmd.Dir = tempDir
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("failed to execute firewall parser script: %w\nOutput: %s", err, string(output))
-	}
-
-	// Write the output to firewall.md in the run directory
 	firewallMdPath := filepath.Join(runDir, "firewall.md")
-	if err := os.WriteFile(firewallMdPath, []byte(strings.TrimSpace(string(output))), constants.FilePermPublic); err != nil {
-		return fmt.Errorf("failed to write firewall.md: %w", err)
-	}
-
-	return nil
+	return runNodeScript(nodeScript, firewallMdPath)
 }