[WIP] Refactor large functions in pkg/workflow and pkg/cli

pkg/cli/access_log.go

@@ -86,85 +86,76 @@ func (d *DomainAnalysis) AddMetrics(other LogAnalysis) {
 // parseSquidAccessLog parses a squid access log file and extracts domain information
 func parseSquidAccessLog(logPath string, verbose bool) (*DomainAnalysis, error) {
 	accessLogLog.Printf("Parsing squid access log: %s", logPath)
-
 	file, err := os.Open(logPath)
 	if err != nil {
 		accessLogLog.Printf("Failed to open access log %s: %v", logPath, err)
 		return nil, fmt.Errorf("failed to open access log: %w", err)
 	}
 	defer file.Close()
-
 	analysis := &DomainAnalysis{}
+	allowedDomainsSet := make(map[string]struct{})
+	blockedDomainsSet := make(map[string]struct{})
+	if err := scanSquidAccessLog(file, analysis, allowedDomainsSet, blockedDomainsSet, verbose); err != nil {
+		return nil, err
+	}
+	sort.Strings(analysis.AllowedDomains)
+	sort.Strings(analysis.BlockedDomains)
+	accessLogLog.Printf("Parsed access log: total_requests=%d, allowed=%d, blocked=%d, unique_allowed_domains=%d, unique_blocked_domains=%d", analysis.TotalRequests, analysis.AllowedRequests, analysis.BlockedRequests, len(analysis.AllowedDomains), len(analysis.BlockedDomains))
+	return analysis, nil
+}
 
-	allowedDomainsSet := make(map[string]struct {
-	})
-	blockedDomainsSet := make(map[string]struct {
-	})
-
+func scanSquidAccessLog(file *os.File, analysis *DomainAnalysis, allowedDomainsSet, blockedDomainsSet map[string]struct{}, verbose bool) error {
 	scanner := bufio.NewScanner(file)
 	for scanner.Scan() {
-		line := strings.TrimSpace(scanner.Text())
-		if line == "" || strings.HasPrefix(line, "#") {
-			continue
-		}
-
-		entry, err := parseSquidLogLine(line)
-		if err != nil {
-			if verbose {
-				fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse log line: %v", err)))
-			}
-			continue
-		}
-
-		analysis.TotalRequests++
-
-		// Extract domain from URL
-		domain := stringutil.ExtractDomainFromURL(entry.URL)
-		if domain == "" {
-			continue
-		}
-
-		// Determine if request was allowed or blocked based on status code
-		// Squid typically returns:
-		// - 200, 206, 304: Allowed/successful
-		// - 403: Forbidden (blocked by ACL)
-		// - 407: Proxy authentication required
-		// - 502, 503: Connection/upstream errors
-		statusCode := entry.Status
-		isAllowed := statusCode == "TCP_HIT/200" || statusCode == "TCP_MISS/200" ||
-			statusCode == "TCP_REFRESH_MODIFIED/200" || statusCode == "TCP_IMS_HIT/304" ||
-			strings.Contains(statusCode, "/200") || strings.Contains(statusCode, "/206") ||
-			strings.Contains(statusCode, "/304")
-
-		if isAllowed {
-			analysis.AllowedRequests++
-			if !setutil.Contains(allowedDomainsSet, domain) {
-				allowedDomainsSet[domain] = struct {
-				}{}
-				analysis.AllowedDomains = append(analysis.AllowedDomains, domain)
-			}
-		} else {
-			analysis.BlockedRequests++
-			if !setutil.Contains(blockedDomainsSet, domain) {
-				blockedDomainsSet[domain] = struct {
-				}{}
-				analysis.BlockedDomains = append(analysis.BlockedDomains, domain)
-			}
+		if err := processSquidLogLine(scanner.Text(), analysis, allowedDomainsSet, blockedDomainsSet, verbose); err != nil {
+			return err
 		}
 	}
-
 	if err := scanner.Err(); err != nil {
-		return nil, fmt.Errorf("error reading access log: %w", err)
+		return fmt.Errorf("error reading access log: %w", err)
 	}
+	return nil
+}
 
-	// Sort domains for consistent output
-	sort.Strings(analysis.AllowedDomains)
-	sort.Strings(analysis.BlockedDomains)
+func processSquidLogLine(rawLine string, analysis *DomainAnalysis, allowedDomainsSet, blockedDomainsSet map[string]struct{}, verbose bool) error {
+	line := strings.TrimSpace(rawLine)
+	if line == "" || strings.HasPrefix(line, "#") {
+		return nil
+	}
+	entry, err := parseSquidLogLine(line)
+	if err != nil {
+		if verbose {
+			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse log line: %v", err)))
+		}
+		return nil
+	}
+	analysis.TotalRequests++
+	domain := stringutil.ExtractDomainFromURL(entry.URL)
+	if domain == "" {
+		return nil
+	}
+	if isAllowedSquidStatus(entry.Status) {
+		analysis.AllowedRequests++
+		appendUniqueDomain(&analysis.AllowedDomains, allowedDomainsSet, domain)
+		return nil
+	}
+	analysis.BlockedRequests++
+	appendUniqueDomain(&analysis.BlockedDomains, blockedDomainsSet, domain)
+	return nil
+}
 
-	accessLogLog.Printf("Parsed access log: total_requests=%d, allowed=%d, blocked=%d, unique_allowed_domains=%d, unique_blocked_domains=%d",
-		analysis.TotalRequests, analysis.AllowedRequests, analysis.BlockedRequests, len(analysis.AllowedDomains), len(analysis.BlockedDomains))
+func isAllowedSquidStatus(statusCode string) bool {
+	return statusCode == "TCP_HIT/200" || statusCode == "TCP_MISS/200" ||
+		statusCode == "TCP_REFRESH_MODIFIED/200" || statusCode == "TCP_IMS_HIT/304" ||
+		strings.Contains(statusCode, "/200") || strings.Contains(statusCode, "/206") || strings.Contains(statusCode, "/304")
+}
 
-	return analysis, nil
+func appendUniqueDomain(domains *[]string, seen map[string]struct{}, domain string) {
+	if setutil.Contains(seen, domain) {
+		return
+	}
+	seen[domain] = struct{}{}
+	*domains = append(*domains, domain)
 }
 
 // parseSquidLogLine parses a single squid access log line

pkg/cli/actions_build_command.go

@@ -201,55 +201,66 @@ func validateActionYml(actionPath string) error {
 // buildAction builds a single action by bundling its dependencies
 func buildAction(actionsDir, actionName string) error {
 	actionsBuildLog.Printf("Building action: %s", actionName)
-
 	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("\n📦 Building action: "+actionName))
-
 	actionPath := filepath.Join(actionsDir, actionName)
-
-	// Validate action.yml
-	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("  ✓ Validating action.yml"))
-	if err := validateActionYml(actionPath); err != nil {
+	if err := validateBuiltAction(actionPath); err != nil {
 		return err
 	}
-
-	// Special handling for setup: build shell script with embedded files
 	if actionName == "setup" {
 		return buildSetupAction(actionsDir, actionName)
 	}
-
-	// Check if this is a composite action (doesn't need JavaScript bundling)
 	isComposite, err := isCompositeAction(actionPath)
 	if err != nil {
 		return fmt.Errorf("failed to check action type: %w", err)
 	}
-
 	if isComposite {
 		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("  ✓ Composite action - no JavaScript bundling needed"))
 		return nil
 	}
+	return bundleJavaScriptAction(actionPath, actionName)
+}
+
+func validateBuiltAction(actionPath string) error {
+	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("  ✓ Validating action.yml"))
+	return validateActionYml(actionPath)
+}
+
+func bundleJavaScriptAction(actionPath, actionName string) error {
+	outputPath, sourceContent, err := readActionSourceFile(actionPath)
+	if err != nil {
+		return err
+	}
+	dependencies := getActionDependencies(actionName)
+	fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("  ✓ Found %d dependencies", len(dependencies))))
+	files := collectActionDependencyFiles(dependencies)
+	outputContent, err := buildBundledActionSource(sourceContent, files)
+	if err != nil {
+		return err
+	}
+	if err := os.WriteFile(outputPath, []byte(outputContent), constants.FilePermSensitive); err != nil {
+		return fmt.Errorf("failed to write output file: %w", err)
+	}
+	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("  ✓ Built "+outputPath))
+	fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("  ✓ Embedded %d files", len(files))))
+	return nil
+}
 
+func readActionSourceFile(actionPath string) (string, []byte, error) {
 	srcPath := filepath.Join(actionPath, "src", "index.js")
 	outputPath := filepath.Join(actionPath, "index.js")
-
-	// Check if source file exists
 	if _, err := os.Stat(srcPath); os.IsNotExist(err) {
-		return fmt.Errorf("source file not found: %s", srcPath)
+		return "", nil, fmt.Errorf("source file not found: %s", srcPath)
 	}
-
 	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("  ✓ Reading source file"))
 	sourceContent, err := os.ReadFile(srcPath)
 	if err != nil {
-		return fmt.Errorf("failed to read source file: %w", err)
+		return "", nil, fmt.Errorf("failed to read source file: %w", err)
 	}
+	return outputPath, sourceContent, nil
+}
 
-	// Get dependencies for this action
-	dependencies := getActionDependencies(actionName)
-	fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("  ✓ Found %d dependencies", len(dependencies))))
-
-	// Get all JavaScript sources
+func collectActionDependencyFiles(dependencies []string) map[string]string {
 	sources := workflow.GetJavaScriptSources()
-
-	// Read dependency files
 	files := make(map[string]string)
 	for _, dep := range dependencies {
 		if content, ok := sources[dep]; ok {
@@ -259,30 +270,17 @@ func buildAction(actionsDir, actionName string) error {
 			fmt.Fprintln(os.Stderr, console.FormatWarningMessage("    ⚠ Warning: Could not find "+dep))
 		}
 	}
+	return files
+}
 
-	// Generate FILES object with embedded content
+func buildBundledActionSource(sourceContent []byte, files map[string]string) (string, error) {
 	filesJSON, err := json.MarshalIndent(files, "", "  ")
 	if err != nil {
-		return fmt.Errorf("failed to marshal files: %w", err)
+		return "", fmt.Errorf("failed to marshal files: %w", err)
 	}
-
-	// Indent the JSON for proper embedding
 	indentedJSON := strings.ReplaceAll(string(filesJSON), "\n", "\n  ")
 	indentedJSON = "  " + strings.TrimPrefix(indentedJSON, " ")
-
-	// Replace the FILES placeholder in source
-	// Match: const FILES = { ... };
-	outputContent := filesConstPattern.ReplaceAllString(string(sourceContent), fmt.Sprintf("const FILES = %s;", strings.TrimSpace(indentedJSON)))
-
-	// Write output file with restrictive permissions (0600 for security)
-	if err := os.WriteFile(outputPath, []byte(outputContent), constants.FilePermSensitive); err != nil {
-		return fmt.Errorf("failed to write output file: %w", err)
-	}
-
-	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("  ✓ Built "+outputPath))
-	fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("  ✓ Embedded %d files", len(files))))
-
-	return nil
+	return filesConstPattern.ReplaceAllString(string(sourceContent), fmt.Sprintf("const FILES = %s;", strings.TrimSpace(indentedJSON))), nil
 }
 
 // isCompositeAction checks if an action uses the 'composite' runtime