diff --git a/.golangci.yml b/.golangci.yml index 91ad1e4999f..46982ae4e57 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -15,6 +15,8 @@ linters: # godot linter disabled - too pedantic about comment punctuation - unconvert # Remove unnecessary conversions - testifylint # Enforce testify best practices + - intrange # Suggest integer range in for loops + - modernize # Modernize Go code using modern language features disable: - errcheck # Disabled due to exclude-functions not working properly in golangci-lint v2 - gocritic # Disabled due to disabled-checks not working properly in golangci-lint v2 diff --git a/pkg/cli/add_interactive_secrets.go b/pkg/cli/add_interactive_secrets.go index 26121fe3e60..e020fd5e9b1 100644 --- a/pkg/cli/add_interactive_secrets.go +++ b/pkg/cli/add_interactive_secrets.go @@ -23,8 +23,8 @@ func (c *AddInteractiveConfig) checkExistingSecrets() error { } // Parse the output - each secret name is on its own line - secretNames := strings.Split(strings.TrimSpace(string(output)), "\n") - for _, name := range secretNames { + secretNames := strings.SplitSeq(strings.TrimSpace(string(output)), "\n") + for name := range secretNames { name = strings.TrimSpace(name) if name != "" { c.existingSecrets[name] = true diff --git a/pkg/cli/add_interactive_workflow.go b/pkg/cli/add_interactive_workflow.go index dac353a7685..afc991e94bd 100644 --- a/pkg/cli/add_interactive_workflow.go +++ b/pkg/cli/add_interactive_workflow.go @@ -29,7 +29,7 @@ func (c *AddInteractiveConfig) checkStatusAndOfferRun(ctx context.Context) error // Try a few times to see the workflow in status var workflowFound bool - for i := 0; i < 5; i++ { + for i := range 5 { // Wait 2 seconds before each check (including the first) select { case <-ctx.Done(): diff --git a/pkg/cli/audit_input_size_test.go b/pkg/cli/audit_input_size_test.go index b953b396744..943d4a2acb7 100644 --- a/pkg/cli/audit_input_size_test.go +++ b/pkg/cli/audit_input_size_test.go @@ -84,7 +84,7 @@ func TestAuditReportIncludesInputSizes(t *testing.T) { // Verify the table has the correct number of columns (5: Tool, Calls, Max Input, Max Output, Max Duration) headerLine := "" - for _, line := range strings.Split(report, "\n") { + for line := range strings.SplitSeq(report, "\n") { if strings.Contains(line, "Max Input") { headerLine = line break diff --git a/pkg/cli/audit_report.go b/pkg/cli/audit_report.go index 343c3885acc..e591beb198f 100644 --- a/pkg/cli/audit_report.go +++ b/pkg/cli/audit_report.go @@ -81,8 +81,8 @@ type OverviewData struct { Status string `json:"status" console:"header:Status"` Conclusion string `json:"conclusion,omitempty" console:"header:Conclusion,omitempty"` CreatedAt time.Time `json:"created_at" console:"header:Created At"` - StartedAt time.Time `json:"started_at,omitempty" console:"header:Started At,omitempty"` - UpdatedAt time.Time `json:"updated_at,omitempty" console:"header:Updated At,omitempty"` + StartedAt time.Time `json:"started_at,omitzero" console:"header:Started At,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitzero" console:"header:Updated At,omitempty"` Duration string `json:"duration,omitempty" console:"header:Duration,omitempty"` Event string `json:"event" console:"header:Event"` Branch string `json:"branch" console:"header:Branch"` @@ -627,10 +627,7 @@ func stripGHALogTimestamps(content string) string { // in a generous window (positions 11-35) to handle any fractional seconds length. if len(line) > 19 && line[4] == '-' && line[7] == '-' && line[10] == 'T' { // Find the Z that ends the timestamp within a reasonable range - searchBound := 35 - if searchBound > len(line) { - searchBound = len(line) - } + searchBound := min(35, len(line)) if zIdx := strings.IndexByte(line[11:searchBound], 'Z'); zIdx >= 0 { zPos := 11 + zIdx if zPos+1 <= len(line) { diff --git a/pkg/cli/codemod_bots_test.go b/pkg/cli/codemod_bots_test.go index cc6a575f5a6..cfc534b1213 100644 --- a/pkg/cli/codemod_bots_test.go +++ b/pkg/cli/codemod_bots_test.go @@ -3,6 +3,7 @@ package cli import ( + "slices" "strings" "testing" @@ -124,13 +125,7 @@ engine: copilot assert.Contains(t, result, "bots: [dependabot, renovate]") // Ensure on: block is created lines := strings.Split(result, "\n") - foundOn := false - for _, line := range lines { - if line == "on:" { - foundOn = true - break - } - } + foundOn := slices.Contains(lines, "on:") assert.True(t, foundOn, "Should create new on: block") } diff --git a/pkg/cli/codemod_roles_test.go b/pkg/cli/codemod_roles_test.go index e4cef277044..0ae36b1c844 100644 --- a/pkg/cli/codemod_roles_test.go +++ b/pkg/cli/codemod_roles_test.go @@ -3,6 +3,7 @@ package cli import ( + "slices" "strings" "testing" @@ -168,13 +169,7 @@ engine: copilot assert.Contains(t, result, "roles: [admin, write]") // Ensure on: block is created lines := strings.Split(result, "\n") - foundOn := false - for _, line := range lines { - if line == "on:" { - foundOn = true - break - } - } + foundOn := slices.Contains(lines, "on:") assert.True(t, foundOn, "Should create new on: block") } diff --git a/pkg/cli/commands_utils_test.go b/pkg/cli/commands_utils_test.go index 4f13ac28691..c469c055ba9 100644 --- a/pkg/cli/commands_utils_test.go +++ b/pkg/cli/commands_utils_test.go @@ -263,8 +263,7 @@ This is a test workflow with some content.` b.Fatalf("Failed to create test file: %v", err) } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = extractWorkflowNameFromFile(filePath) } } @@ -278,8 +277,7 @@ More content here. @include another/file.md Final content.` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = findIncludesInContent(content, "", false) } } diff --git a/pkg/cli/compile_security_benchmark_test.go b/pkg/cli/compile_security_benchmark_test.go index b0c92d04e30..ec508a61659 100644 --- a/pkg/cli/compile_security_benchmark_test.go +++ b/pkg/cli/compile_security_benchmark_test.go @@ -55,9 +55,8 @@ PR Number: ${{ github.event.pull_request.number }} compiler := workflow.NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { // Compile with actionlint enabled (per-file mode for benchmarking) _ = CompileWorkflowWithValidation(compiler, testFile, false, false, false, true, false, false) } @@ -107,9 +106,8 @@ Issue: ${{ needs.activation.outputs.text }} compiler := workflow.NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { // Compile with zizmor enabled _ = CompileWorkflowWithValidation(compiler, testFile, false, true, false, false, false, false) } @@ -153,9 +151,8 @@ Repository: ${{ github.repository }} compiler := workflow.NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { // Compile with poutine enabled _ = CompileWorkflowWithValidation(compiler, testFile, false, false, true, false, false, false) } @@ -224,9 +221,8 @@ PR Details: compiler := workflow.NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { // Compile with all security tools enabled (zizmor, poutine, actionlint) _ = CompileWorkflowWithValidation(compiler, testFile, false, true, true, true, false, false) } @@ -276,9 +272,8 @@ PR Number: ${{ github.event.pull_request.number }} compiler := workflow.NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { // Compile without any security tools _ = CompileWorkflowWithValidation(compiler, testFile, false, false, false, false, false, false) } @@ -340,9 +335,8 @@ Process issue. lockFiles = append(lockFiles, lockFile) } - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { // Run batch actionlint on all lock files _ = RunActionlintOnFiles(lockFiles, false, false) } @@ -433,9 +427,8 @@ Triggered by: ${{ github.actor }} compiler := workflow.NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { // Compile with all security tools enabled _ = CompileWorkflowWithValidation(compiler, testFile, false, true, true, true, false, false) } diff --git a/pkg/cli/compile_stats_test.go b/pkg/cli/compile_stats_test.go index d7f8bc5fc14..b78e1414206 100644 --- a/pkg/cli/compile_stats_test.go +++ b/pkg/cli/compile_stats_test.go @@ -76,7 +76,7 @@ func TestDisplayStatsTable_LessThan10(t *testing.T) { func TestDisplayStatsTable_Exactly10(t *testing.T) { // Create test stats with exactly 10 workflows statsList := make([]*WorkflowStats, 10) - for i := 0; i < 10; i++ { + for i := range 10 { statsList[i] = &WorkflowStats{ Workflow: fmt.Sprintf("workflow%d.lock.yml", i+1), FileSize: int64((10 - i) * 1000), // Descending sizes @@ -121,7 +121,7 @@ func TestDisplayStatsTable_Exactly10(t *testing.T) { func TestDisplayStatsTable_MoreThan10(t *testing.T) { // Create test stats with 15 workflows statsList := make([]*WorkflowStats, 15) - for i := 0; i < 15; i++ { + for i := range 15 { statsList[i] = &WorkflowStats{ Workflow: fmt.Sprintf("workflow%d.lock.yml", i+1), FileSize: int64((15 - i) * 1000), // Descending sizes @@ -146,7 +146,7 @@ func TestDisplayStatsTable_MoreThan10(t *testing.T) { output := buf.String() // Should show top 10 workflows (largest sizes) - for i := 0; i < 10; i++ { + for i := range 10 { if !strings.Contains(output, statsList[i].Workflow) { t.Errorf("Expected top 10 workflow %s to be displayed, but it wasn't found", statsList[i].Workflow) } diff --git a/pkg/cli/completions_test.go b/pkg/cli/completions_test.go index e51c2a84073..824a71d1d79 100644 --- a/pkg/cli/completions_test.go +++ b/pkg/cli/completions_test.go @@ -5,6 +5,7 @@ package cli import ( "os" "path/filepath" + "slices" "strings" "testing" @@ -207,13 +208,7 @@ No description workflow // Verify the name part matches expected names name := parts[0] - found := false - for _, expectedName := range tt.wantNames { - if name == expectedName { - found = true - break - } - } + found := slices.Contains(tt.wantNames, name) assert.True(t, found, "Expected name %s to be in %v", name, tt.wantNames) } } diff --git a/pkg/cli/dependency_graph_test.go b/pkg/cli/dependency_graph_test.go index 548f70cf7de..45c52b7c1e7 100644 --- a/pkg/cli/dependency_graph_test.go +++ b/pkg/cli/dependency_graph_test.go @@ -325,7 +325,7 @@ description: MCP Tool // Create multiple top-level workflows that import the deep shared workflow workflows := make([]string, 3) - for i := 0; i < 3; i++ { + for i := range 3 { workflows[i] = filepath.Join(workflowsDir, fmt.Sprintf("workflow%d.md", i)) content := fmt.Sprintf(`--- description: Workflow %d diff --git a/pkg/cli/devcontainer.go b/pkg/cli/devcontainer.go index ae5dfc56e69..d9c5d6dd75f 100644 --- a/pkg/cli/devcontainer.go +++ b/pkg/cli/devcontainer.go @@ -3,6 +3,7 @@ package cli import ( "encoding/json" "fmt" + "maps" "os" "path/filepath" "strings" @@ -312,9 +313,7 @@ func mergeFeatures(existing DevcontainerFeatures, toAdd map[string]any) { } // Add new features - for key, value := range toAdd { - existing[key] = value - } + maps.Copy(existing, toAdd) } // getCurrentRepoName gets the current repository name from git remote in owner/repo format diff --git a/pkg/cli/docker_images_test.go b/pkg/cli/docker_images_test.go index 2221d298b0c..1513aa6b23a 100644 --- a/pkg/cli/docker_images_test.go +++ b/pkg/cli/docker_images_test.go @@ -284,7 +284,7 @@ func TestStartDockerImageDownload_ConcurrentCalls(t *testing.T) { doneChan := make(chan int, numGoroutines) // Launch multiple goroutines that all try to start downloading the same image - for i := 0; i < numGoroutines; i++ { + for i := range numGoroutines { go func(index int) { <-startChan // Wait for the signal to start started[index] = StartDockerImageDownload(context.Background(), testImage) @@ -296,7 +296,7 @@ func TestStartDockerImageDownload_ConcurrentCalls(t *testing.T) { close(startChan) // Wait for all goroutines to finish - for i := 0; i < numGoroutines; i++ { + for range numGoroutines { <-doneChan } @@ -340,7 +340,7 @@ func TestStartDockerImageDownload_ConcurrentCallsWithAvailableImage(t *testing.T doneChan := make(chan int, numGoroutines) // Launch multiple goroutines - for i := 0; i < numGoroutines; i++ { + for i := range numGoroutines { go func(index int) { <-startChan started[index] = StartDockerImageDownload(context.Background(), testImage) @@ -352,7 +352,7 @@ func TestStartDockerImageDownload_ConcurrentCallsWithAvailableImage(t *testing.T close(startChan) // Wait for all to finish - for i := 0; i < numGoroutines; i++ { + for range numGoroutines { <-doneChan } @@ -393,7 +393,7 @@ func TestStartDockerImageDownload_RaceWithExternalDownload(t *testing.T) { const numGoroutines = 5 results := make(chan bool, numGoroutines) - for i := 0; i < numGoroutines; i++ { + for range numGoroutines { go func() { results <- StartDockerImageDownload(context.Background(), testImage) }() @@ -401,7 +401,7 @@ func TestStartDockerImageDownload_RaceWithExternalDownload(t *testing.T) { // Collect results downloadStarts := 0 - for i := 0; i < numGoroutines; i++ { + for range numGoroutines { if <-results { downloadStarts++ } diff --git a/pkg/cli/engine_secrets.go b/pkg/cli/engine_secrets.go index 4f76f38977e..ddde7b8a32d 100644 --- a/pkg/cli/engine_secrets.go +++ b/pkg/cli/engine_secrets.go @@ -434,8 +434,8 @@ func uploadSecretToRepo(secretName, secretValue, repoSlug string, verbose bool) // stringContainsSecretName checks if the gh secret list output contains a secret name func stringContainsSecretName(output, secretName string) bool { - lines := strings.Split(output, "\n") - for _, line := range lines { + lines := strings.SplitSeq(output, "\n") + for line := range lines { if len(line) >= len(secretName) { if line[:len(secretName)] == secretName && (len(line) == len(secretName) || line[len(secretName)] == '\t' || line[len(secretName)] == ' ') { return true diff --git a/pkg/cli/error_formatting_test.go b/pkg/cli/error_formatting_test.go index 05accb0b78c..f4eee9a5b89 100644 --- a/pkg/cli/error_formatting_test.go +++ b/pkg/cli/error_formatting_test.go @@ -278,8 +278,8 @@ func TestErrorFormattingDoesNotMangle(t *testing.T) { // All essential parts of the message should be preserved // (formatting may add prefixes/styling but shouldn't lose content) - essentialParts := strings.Fields(tt.message) - for _, part := range essentialParts { + essentialParts := strings.FieldsSeq(tt.message) + for part := range essentialParts { // Skip very short parts like ":", "at", etc. if len(part) > 2 { assert.Contains(t, formatted, part, diff --git a/pkg/cli/file_tracker_test.go b/pkg/cli/file_tracker_test.go index 0509a4c6366..0eb7431802f 100644 --- a/pkg/cli/file_tracker_test.go +++ b/pkg/cli/file_tracker_test.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "testing" ) @@ -315,13 +316,7 @@ This uses reaction. // Should track the lock file lockFile := filepath.Join(tempDir, "test-workflow-with-reaction.lock.yml") - found := false - for _, file := range allFiles { - if file == lockFile { - found = true - break - } - } + found := slices.Contains(allFiles, lockFile) if !found { t.Errorf("Lock file %s should be tracked", lockFile) } diff --git a/pkg/cli/fix_command_test.go b/pkg/cli/fix_command_test.go index e8283d93647..f3ead84b7d7 100644 --- a/pkg/cli/fix_command_test.go +++ b/pkg/cli/fix_command_test.go @@ -532,8 +532,8 @@ This is a test workflow with slash command. } // Check that standalone "command" field was replaced (not part of slash_command) - lines := strings.Split(updatedStr, "\n") - for _, line := range lines { + lines := strings.SplitSeq(updatedStr, "\n") + for line := range lines { trimmed := strings.TrimSpace(line) if strings.HasPrefix(trimmed, "command:") && !strings.Contains(line, "slash_command") { t.Errorf("Found unreplaced 'command:' field: %s", line) diff --git a/pkg/cli/frontmatter_editor.go b/pkg/cli/frontmatter_editor.go index 6c56b1db09e..94bb1a12f99 100644 --- a/pkg/cli/frontmatter_editor.go +++ b/pkg/cli/frontmatter_editor.go @@ -213,7 +213,7 @@ func RemoveFieldFromOnTrigger(content, fieldName string) (string, error) { skipNextLine := false fieldIndentLevel := 0 - for i := 0; i < len(result.FrontmatterLines); i++ { + for i := range len(result.FrontmatterLines) { line := result.FrontmatterLines[i] trimmedLine := strings.TrimSpace(line) @@ -353,7 +353,7 @@ func SetFieldInOnTrigger(content, fieldName, fieldValue string) (string, error) onIndentLevel := 0 fieldUpdated := false - for i := 0; i < len(result.FrontmatterLines); i++ { + for i := range len(result.FrontmatterLines) { line := result.FrontmatterLines[i] trimmedLine := strings.TrimSpace(line) diff --git a/pkg/cli/git.go b/pkg/cli/git.go index fbce963830f..4ebca436c11 100644 --- a/pkg/cli/git.go +++ b/pkg/cli/git.go @@ -77,16 +77,16 @@ func parseGitHubRepoSlugFromURL(url string) string { githubHostWithoutScheme := strings.TrimPrefix(strings.TrimPrefix(githubHost, "https://"), "http://") // Handle HTTPS URLs: https://github.com/owner/repo or https://enterprise.github.com/owner/repo - if strings.HasPrefix(url, githubHost+"/") { - slug := strings.TrimPrefix(url, githubHost+"/") + if after, ok := strings.CutPrefix(url, githubHost+"/"); ok { + slug := after gitLog.Printf("Extracted slug from HTTPS URL: %s", slug) return slug } // Handle SSH URLs: git@github.com:owner/repo or git@enterprise.github.com:owner/repo sshPrefix := "git@" + githubHostWithoutScheme + ":" - if strings.HasPrefix(url, sshPrefix) { - slug := strings.TrimPrefix(url, sshPrefix) + if after, ok := strings.CutPrefix(url, sshPrefix); ok { + slug := after gitLog.Printf("Extracted slug from SSH URL: %s", slug) return slug } diff --git a/pkg/cli/health_command.go b/pkg/cli/health_command.go index 3d06a6a8038..2e37f06d7a6 100644 --- a/pkg/cli/health_command.go +++ b/pkg/cli/health_command.go @@ -143,7 +143,7 @@ func fetchWorkflowRuns(workflowName, startDate, repoOverride string, verbose boo allRuns := make([]WorkflowRun, 0) // Fetch runs in batches - for i := 0; i < MaxIterations; i++ { + for i := range MaxIterations { runs, totalCount, err := listWorkflowRunsWithPagination(opts) if err != nil { return nil, err diff --git a/pkg/cli/imports.go b/pkg/cli/imports.go index 9e41e16380c..171b6b05064 100644 --- a/pkg/cli/imports.go +++ b/pkg/cli/imports.go @@ -39,8 +39,8 @@ func resolveImportPath(importPath string, workflowPath string) string { } // If the import path is absolute (starts with /), use it as-is (relative to repo root) - if strings.HasPrefix(importPath, "/") { - return strings.TrimPrefix(importPath, "/") + if after, ok := strings.CutPrefix(importPath, "/"); ok { + return after } // Otherwise, resolve relative to the workflow file's directory diff --git a/pkg/cli/logs_awinfo_backward_compat_test.go b/pkg/cli/logs_awinfo_backward_compat_test.go index 56f24fe2ded..5aaaa055b95 100644 --- a/pkg/cli/logs_awinfo_backward_compat_test.go +++ b/pkg/cli/logs_awinfo_backward_compat_test.go @@ -169,7 +169,7 @@ func TestAwInfoMarshaling(t *testing.T) { if tt.shouldContainNew { // Check for awf_version in JSON - var temp map[string]interface{} + var temp map[string]any json.Unmarshal(data, &temp) if _, exists := temp["awf_version"]; !exists { t.Errorf("%s: JSON should contain awf_version field, got: %s", tt.description, jsonStr) @@ -178,7 +178,7 @@ func TestAwInfoMarshaling(t *testing.T) { if tt.shouldContainOld { // Check for firewall_version in JSON - var temp map[string]interface{} + var temp map[string]any json.Unmarshal(data, &temp) if _, exists := temp["firewall_version"]; !exists { t.Errorf("%s: JSON should contain firewall_version field, got: %s", tt.description, jsonStr) diff --git a/pkg/cli/logs_benchmark_test.go b/pkg/cli/logs_benchmark_test.go index 4be945fb15c..08700664a2b 100644 --- a/pkg/cli/logs_benchmark_test.go +++ b/pkg/cli/logs_benchmark_test.go @@ -43,8 +43,7 @@ tool result: {"id": 123, "body": "Issue content"} func BenchmarkParseClaudeLog(b *testing.B) { engine := &workflow.ClaudeEngine{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = engine.ParseLogMetrics(sampleClaudeLog, false) } } @@ -53,8 +52,7 @@ func BenchmarkParseClaudeLog(b *testing.B) { func BenchmarkParseClaudeLog_Large(b *testing.B) { engine := &workflow.ClaudeEngine{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = engine.ParseLogMetrics(largeClaudeLog, false) } } @@ -63,8 +61,7 @@ func BenchmarkParseClaudeLog_Large(b *testing.B) { func BenchmarkParseCopilotLog(b *testing.B) { engine := &workflow.CopilotEngine{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = engine.ParseLogMetrics(sampleCopilotLog, false) } } @@ -73,8 +70,7 @@ func BenchmarkParseCopilotLog(b *testing.B) { func BenchmarkParseCopilotLog_Large(b *testing.B) { engine := &workflow.CopilotEngine{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = engine.ParseLogMetrics(largeCopilotLog, false) } } @@ -83,8 +79,7 @@ func BenchmarkParseCopilotLog_Large(b *testing.B) { func BenchmarkParseCodexLog(b *testing.B) { engine := &workflow.CodexEngine{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = engine.ParseLogMetrics(sampleCodexLog, false) } } @@ -100,8 +95,7 @@ func BenchmarkParseCodexLog_WithErrors(b *testing.B) { engine := &workflow.CodexEngine{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = engine.ParseLogMetrics(logWithErrors, false) } } @@ -145,8 +139,7 @@ func BenchmarkAggregateWorkflowStats(b *testing.B) { }, } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { // Simulate aggregation logic totalTokens := 0 totalCost := 0.0 @@ -174,7 +167,7 @@ func BenchmarkAggregateWorkflowStats(b *testing.B) { func BenchmarkAggregateWorkflowStats_Large(b *testing.B) { // Create 100 sample workflow runs runs := make([]WorkflowRun, 100) - for i := 0; i < 100; i++ { + for i := range 100 { runs[i] = WorkflowRun{ DatabaseID: int64(12345 + i), WorkflowName: "test-workflow", @@ -188,8 +181,7 @@ func BenchmarkAggregateWorkflowStats_Large(b *testing.B) { } } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { totalTokens := 0 totalCost := 0.0 totalTurns := 0 @@ -216,8 +208,7 @@ func BenchmarkAggregateWorkflowStats_Large(b *testing.B) { func BenchmarkExtractJSONMetrics(b *testing.B) { jsonLine := `{"type":"usage","input_tokens":1000,"output_tokens":500,"cost":0.015}` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = workflow.ExtractJSONMetrics(jsonLine, false) } } @@ -226,8 +217,7 @@ func BenchmarkExtractJSONMetrics(b *testing.B) { func BenchmarkExtractJSONMetrics_Complex(b *testing.B) { jsonLine := `{"type":"result","total_input_tokens":5000,"total_output_tokens":2500,"cost":0.075,"metadata":{"tool_calls":["github.issue_read","github.add_comment"],"duration_ms":1500}}` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = workflow.ExtractJSONMetrics(jsonLine, false) } } diff --git a/pkg/cli/logs_command_test.go b/pkg/cli/logs_command_test.go index 434dc5eb1af..45079cd3e83 100644 --- a/pkg/cli/logs_command_test.go +++ b/pkg/cli/logs_command_test.go @@ -120,11 +120,11 @@ func TestLogsCommandBooleanFlags(t *testing.T) { func TestLogsCommandStructure(t *testing.T) { tests := []struct { name string - commandCreator func() interface{} + commandCreator func() any }{ { name: "logs command exists", - commandCreator: func() interface{} { + commandCreator: func() any { return NewLogsCommand() }, }, diff --git a/pkg/cli/logs_github_api.go b/pkg/cli/logs_github_api.go index bee50077056..28ed1d76c3c 100644 --- a/pkg/cli/logs_github_api.go +++ b/pkg/cli/logs_github_api.go @@ -44,8 +44,8 @@ func fetchJobStatuses(runID int64, verbose bool) (int, error) { // Parse each line as a separate JSON object failedJobs := 0 - lines := strings.Split(strings.TrimSpace(string(output)), "\n") - for _, line := range lines { + lines := strings.SplitSeq(strings.TrimSpace(string(output)), "\n") + for line := range lines { if strings.TrimSpace(line) == "" { continue } @@ -89,8 +89,8 @@ func fetchJobDetails(runID int64, verbose bool) ([]JobInfoWithDuration, error) { } var jobs []JobInfoWithDuration - lines := strings.Split(strings.TrimSpace(string(output)), "\n") - for _, line := range lines { + lines := strings.SplitSeq(strings.TrimSpace(string(output)), "\n") + for line := range lines { if strings.TrimSpace(line) == "" { continue } diff --git a/pkg/cli/logs_json_clean_test.go b/pkg/cli/logs_json_clean_test.go index 39ea1d366c7..05917e290ff 100644 --- a/pkg/cli/logs_json_clean_test.go +++ b/pkg/cli/logs_json_clean_test.go @@ -135,10 +135,7 @@ func TestStderrMessagesAfterJSON(t *testing.T) { // Find the first '{' and the matching '}' firstBrace := strings.Index(output, "{") if firstBrace != 0 { - maxLen := 50 - if len(output) < maxLen { - maxLen = len(output) - } + maxLen := min(len(output), 50) t.Errorf("Output doesn't start with JSON. It starts with: %s", output[:maxLen]) } diff --git a/pkg/cli/logs_metrics.go b/pkg/cli/logs_metrics.go index 76678cff158..71015309765 100644 --- a/pkg/cli/logs_metrics.go +++ b/pkg/cli/logs_metrics.go @@ -687,8 +687,8 @@ func extractMCPFailuresFromLogFile(logPath string, run WorkflowRun, verbose bool } } else { // Fallback: Try to parse as JSON lines (Claude logs are typically NDJSON format) - lines := strings.Split(logContent, "\n") - for _, line := range lines { + lines := strings.SplitSeq(logContent, "\n") + for line := range lines { line = strings.TrimSpace(line) if line == "" || !strings.HasPrefix(line, "{") { continue diff --git a/pkg/cli/logs_models.go b/pkg/cli/logs_models.go index c1dfb5b08cf..df644633222 100644 --- a/pkg/cli/logs_models.go +++ b/pkg/cli/logs_models.go @@ -214,8 +214,8 @@ type JobInfo struct { Name string `json:"name"` Status string `json:"status"` Conclusion string `json:"conclusion"` - StartedAt time.Time `json:"started_at,omitempty"` - CompletedAt time.Time `json:"completed_at,omitempty"` + StartedAt time.Time `json:"started_at,omitzero"` + CompletedAt time.Time `json:"completed_at,omitzero"` } // JobInfoWithDuration extends JobInfo with calculated duration @@ -240,7 +240,7 @@ type AwInfo struct { Staged bool `json:"staged"` AwfVersion string `json:"awf_version,omitempty"` // AWF firewall version (new name) FirewallVersion string `json:"firewall_version,omitempty"` // AWF firewall version (old name, for backward compatibility) - Steps AwInfoSteps `json:"steps,omitempty"` // Steps metadata + Steps AwInfoSteps `json:"steps,omitzero"` // Steps metadata CreatedAt string `json:"created_at"` // Additional fields that might be present RunID any `json:"run_id,omitempty"` diff --git a/pkg/cli/logs_orchestrator.go b/pkg/cli/logs_orchestrator.go index 40ee35d945d..da65e335975 100644 --- a/pkg/cli/logs_orchestrator.go +++ b/pkg/cli/logs_orchestrator.go @@ -183,13 +183,7 @@ func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, s } // Process slightly more than we need to account for skips due to filters. - chunkSize := remainingNeeded * 3 - if chunkSize < remainingNeeded { - chunkSize = remainingNeeded - } - if chunkSize > len(runsRemaining) { - chunkSize = len(runsRemaining) - } + chunkSize := min(max(remainingNeeded*3, remainingNeeded), len(runsRemaining)) chunk := runsRemaining[:chunkSize] runsRemaining = runsRemaining[chunkSize:] @@ -558,7 +552,6 @@ func downloadRunArtifactsConcurrent(ctx context.Context, runs []WorkflowRun, out // Panics are automatically recovered by the pool and re-raised with full stack traces // after all tasks complete. This ensures one failing download doesn't break others. for _, run := range actualRuns { - run := run // capture loop variable p.Go(func(ctx context.Context) (DownloadResult, error) { // Check for context cancellation before starting download select { diff --git a/pkg/cli/logs_parsing_core.go b/pkg/cli/logs_parsing_core.go index 341211b7fa8..fa67f99b701 100644 --- a/pkg/cli/logs_parsing_core.go +++ b/pkg/cli/logs_parsing_core.go @@ -168,8 +168,8 @@ func findAgentLogFile(logDir string, engine workflow.CodingAgentEngine) (string, // After flattening, it's at logDir/sandbox/agent/logs/ // Strip /tmp/gh-aw/ prefix to get the relative path const tmpGhAwPrefix = "/tmp/gh-aw/" - if strings.HasPrefix(logFileForParsing, tmpGhAwPrefix) { - relPath := strings.TrimPrefix(logFileForParsing, tmpGhAwPrefix) + if after, ok := strings.CutPrefix(logFileForParsing, tmpGhAwPrefix); ok { + relPath := after flattenedDir := filepath.Join(logDir, relPath) logsParsingCoreLog.Printf("Checking flattened location for logs: %s", flattenedDir) if fileutil.DirExists(flattenedDir) { diff --git a/pkg/cli/logs_report.go b/pkg/cli/logs_report.go index d1f1d872c41..17d93d6e723 100644 --- a/pkg/cli/logs_report.go +++ b/pkg/cli/logs_report.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "sort" "strings" "time" @@ -81,8 +82,8 @@ type RunData struct { MissingDataCount int `json:"missing_data_count" console:"header:Missing Data"` SafeItemsCount int `json:"safe_items_count,omitempty" console:"header:Safe Items,omitempty"` CreatedAt time.Time `json:"created_at" console:"header:Created"` - StartedAt time.Time `json:"started_at,omitempty" console:"-"` - UpdatedAt time.Time `json:"updated_at,omitempty" console:"-"` + StartedAt time.Time `json:"started_at,omitzero" console:"-"` + UpdatedAt time.Time `json:"updated_at,omitzero" console:"-"` URL string `json:"url" console:"-"` LogsPath string `json:"logs_path" console:"header:Logs Path"` Event string `json:"event" console:"-"` @@ -376,10 +377,8 @@ func buildToolUsageSummary(processedRuns []ProcessedRun) []ToolUsageSummary { // addUniqueWorkflow adds a workflow to the list if it's not already present func addUniqueWorkflow(workflows []string, workflow string) []string { - for _, wf := range workflows { - if wf == workflow { - return workflows - } + if slices.Contains(workflows, workflow) { + return workflows } return append(workflows, workflow) } diff --git a/pkg/cli/logs_summary_test.go b/pkg/cli/logs_summary_test.go index a11c513709a..32da1cb6f5b 100644 --- a/pkg/cli/logs_summary_test.go +++ b/pkg/cli/logs_summary_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "os" "path/filepath" + "slices" "testing" "time" @@ -206,13 +207,7 @@ func TestListArtifacts(t *testing.T) { // Verify all test files are in the list for _, expectedFile := range testFiles { - found := false - for _, artifact := range artifacts { - if artifact == expectedFile { - found = true - break - } - } + found := slices.Contains(artifacts, expectedFile) if !found { t.Errorf("Expected artifact %s not found in list: %v", expectedFile, artifacts) } diff --git a/pkg/cli/logs_utils.go b/pkg/cli/logs_utils.go index 0422d55ef02..596d685f5c6 100644 --- a/pkg/cli/logs_utils.go +++ b/pkg/cli/logs_utils.go @@ -52,8 +52,8 @@ func getAgenticWorkflowNames(verbose bool) ([]string, error) { } // Extract the workflow name using simple string parsing - lines := strings.Split(string(content), "\n") - for _, line := range lines { + lines := strings.SplitSeq(string(content), "\n") + for line := range lines { trimmed := strings.TrimSpace(line) if strings.HasPrefix(trimmed, "name:") { // Parse the name field diff --git a/pkg/cli/mcp_inspect_mcp.go b/pkg/cli/mcp_inspect_mcp.go index e8845b91a7c..bef7fb0d7c6 100644 --- a/pkg/cli/mcp_inspect_mcp.go +++ b/pkg/cli/mcp_inspect_mcp.go @@ -7,6 +7,7 @@ import ( "net/http" "os" "os/exec" + "slices" "strings" "time" @@ -450,11 +451,8 @@ func displayDetailedToolInfo(info *parser.MCPServerInfo, toolName string) { // Check if tool is allowed isAllowed := len(info.Config.Allowed) == 0 // Default to allowed if no allowlist - for _, allowed := range info.Config.Allowed { - if allowed == toolName { - isAllowed = true - break - } + if slices.Contains(info.Config.Allowed, toolName) { + isAllowed = true } fmt.Fprintf(os.Stderr, "\n%s\n", console.FormatSectionHeader(fmt.Sprintf("🛠️ Tool Details: %s", foundTool.Name))) @@ -569,12 +567,9 @@ func displayToolAllowanceHint(info *parser.MCPServerInfo) { } // Show first few blocked tools as examples (limit to 3 for readability) - exampleCount := len(blockedTools) - if exampleCount > 3 { - exampleCount = 3 - } + exampleCount := min(len(blockedTools), 3) - for i := 0; i < exampleCount; i++ { + for i := range exampleCount { fmt.Fprintf(os.Stderr, " - %s\n", blockedTools[i]) } diff --git a/pkg/cli/mcp_list.go b/pkg/cli/mcp_list.go index d58657d0496..0c57e04be91 100644 --- a/pkg/cli/mcp_list.go +++ b/pkg/cli/mcp_list.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "github.com/github/gh-aw/pkg/console" @@ -277,10 +278,8 @@ func formatToolsCount(allowed []string) string { } // Check for wildcard - for _, tool := range allowed { - if tool == "*" { - return "All tools" - } + if slices.Contains(allowed, "*") { + return "All tools" } if len(allowed) == 1 { diff --git a/pkg/cli/mcp_registry_improvements_test.go b/pkg/cli/mcp_registry_improvements_test.go index c2f450b640d..ae814d5b12e 100644 --- a/pkg/cli/mcp_registry_improvements_test.go +++ b/pkg/cli/mcp_registry_improvements_test.go @@ -107,7 +107,7 @@ func TestMCPRegistryClient_FlexibleValidation(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Generate server list based on count servers := make([]string, tc.serverCount) - for i := 0; i < tc.serverCount; i++ { + for i := range tc.serverCount { servers[i] = `{ "server": { "name": "test/server-` + string(rune('1'+i)) + `", @@ -175,9 +175,10 @@ func joinStrings(strs []string, sep string) string { if len(strs) == 0 { return "" } - result := strs[0] + var result strings.Builder + result.WriteString(strs[0]) for i := 1; i < len(strs); i++ { - result += sep + strs[i] + result.WriteString(sep + strs[i]) } - return result + return result.String() } diff --git a/pkg/cli/mcp_secrets_test.go b/pkg/cli/mcp_secrets_test.go index a39ad1235ae..648dbb9f46f 100644 --- a/pkg/cli/mcp_secrets_test.go +++ b/pkg/cli/mcp_secrets_test.go @@ -3,6 +3,7 @@ package cli import ( + "slices" "strings" "testing" ) @@ -198,13 +199,7 @@ func TestSecretExtraction(t *testing.T) { // Verify all expected names are present for _, expectedName := range tt.expectedNames { - found := false - for _, secretName := range requiredSecrets { - if secretName == expectedName { - found = true - break - } - } + found := slices.Contains(requiredSecrets, expectedName) if !found { t.Errorf("Expected secret %q not found in extracted secrets", expectedName) } diff --git a/pkg/cli/mcp_server_cache_test.go b/pkg/cli/mcp_server_cache_test.go index b944b8fb47d..601549d914f 100644 --- a/pkg/cli/mcp_server_cache_test.go +++ b/pkg/cli/mcp_server_cache_test.go @@ -14,7 +14,7 @@ func TestMCPCacheStore_ConcurrentPermissionAccess(t *testing.T) { cache.permissionTTL = 50 * time.Millisecond // Pre-populate - for i := 0; i < 5; i++ { + for i := range 5 { cache.SetPermission(fmt.Sprintf("actor%d", i), "owner/repo", "write") } @@ -23,16 +23,14 @@ func TestMCPCacheStore_ConcurrentPermissionAccess(t *testing.T) { var wg sync.WaitGroup - for g := 0; g < numGoroutines; g++ { - wg.Add(1) - go func() { - defer wg.Done() - for i := 0; i < numIterations; i++ { + for range numGoroutines { + wg.Go(func() { + for i := range numIterations { actor := fmt.Sprintf("actor%d", i%10) cache.GetPermission(actor, "owner/repo") cache.SetPermission(actor, "owner/repo", "write") } - }() + }) } wg.Wait() @@ -47,11 +45,11 @@ func TestMCPCacheStore_ConcurrentRepoAccess(t *testing.T) { var wg sync.WaitGroup - for g := 0; g < numGoroutines; g++ { + for g := range numGoroutines { wg.Add(1) go func(id int) { defer wg.Done() - for i := 0; i < numIterations; i++ { + for range numIterations { cache.GetRepo() cache.SetRepo(fmt.Sprintf("owner/repo-%d", id)) } diff --git a/pkg/cli/packages_test.go b/pkg/cli/packages_test.go index aab0743360e..b18e5ade120 100644 --- a/pkg/cli/packages_test.go +++ b/pkg/cli/packages_test.go @@ -5,6 +5,7 @@ package cli import ( "os" "path/filepath" + "slices" "strings" "testing" @@ -418,13 +419,7 @@ func TestCopyIncludeDependenciesFromPackageWithForce_FileTracker(t *testing.T) { } targetFile := filepath.Join(targetDir, "file.md") - found := false - for _, f := range tracker.CreatedFiles { - if f == targetFile { - found = true - break - } - } + found := slices.Contains(tracker.CreatedFiles, targetFile) if !found { t.Errorf("File should be tracked as created, got CreatedFiles: %v", tracker.CreatedFiles) } @@ -444,13 +439,7 @@ func TestCopyIncludeDependenciesFromPackageWithForce_FileTracker(t *testing.T) { t.Errorf("Unexpected error: %v", err) } - found = false - for _, f := range tracker2.ModifiedFiles { - if f == targetFile { - found = true - break - } - } + found = slices.Contains(tracker2.ModifiedFiles, targetFile) if !found { t.Errorf("File should be tracked as modified, got ModifiedFiles: %v", tracker2.ModifiedFiles) } diff --git a/pkg/cli/remote_workflow.go b/pkg/cli/remote_workflow.go index fbd00c94650..1d49dfcdad5 100644 --- a/pkg/cli/remote_workflow.go +++ b/pkg/cli/remote_workflow.go @@ -264,8 +264,8 @@ func fetchAndSaveRemoteIncludes(content string, spec *WorkflowSpec, targetDir st // Remove section reference for file fetching filePath := includePath - if idx := strings.Index(includePath, "#"); idx != -1 { - filePath = includePath[:idx] + if before, _, ok := strings.Cut(includePath, "#"); ok { + filePath = before } // Skip if already processed diff --git a/pkg/cli/repo.go b/pkg/cli/repo.go index 3558f8093f9..71e9779653b 100644 --- a/pkg/cli/repo.go +++ b/pkg/cli/repo.go @@ -65,8 +65,8 @@ func getCurrentRepoSlugUncached() (string, error) { var repoPath string // SSH format: git@github.com:owner/repo.git - if strings.HasPrefix(remoteURL, "git@github.com:") { - repoPath = strings.TrimPrefix(remoteURL, "git@github.com:") + if after, ok := strings.CutPrefix(remoteURL, "git@github.com:"); ok { + repoPath = after } else if strings.Contains(remoteURL, "github.com/") { // HTTPS format: https://github.com/owner/repo.git parts := strings.Split(remoteURL, "github.com/") diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index 69da218e643..9ee88e433f1 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -640,8 +640,8 @@ func checkFrontmatterHashMismatch(workflowPath, lockFilePath string) (bool, erro // extractHashFromLockFile extracts the frontmatter-hash from a lock file content func extractHashFromLockFile(content string) string { // Look for: # frontmatter-hash: - lines := strings.Split(content, "\n") - for _, line := range lines { + lines := strings.SplitSeq(content, "\n") + for line := range lines { if len(line) > 20 && line[:20] == "# frontmatter-hash: " { return strings.TrimSpace(line[20:]) } diff --git a/pkg/cli/run_workflow_tracking.go b/pkg/cli/run_workflow_tracking.go index dbb5d6f66ae..1669eed9afe 100644 --- a/pkg/cli/run_workflow_tracking.go +++ b/pkg/cli/run_workflow_tracking.go @@ -49,13 +49,10 @@ func getLatestWorkflowRunWithRetry(lockFileName string, repo string, verbose boo } var lastErr error - for attempt := 0; attempt < maxRetries; attempt++ { + for attempt := range maxRetries { if attempt > 0 { // Calculate delay with exponential backoff, capped at maxDelay - delay := time.Duration(attempt) * initialDelay - if delay > maxDelay { - delay = maxDelay - } + delay := min(time.Duration(attempt)*initialDelay, maxDelay) // Calculate elapsed time since start elapsed := time.Since(startTime).Round(time.Second) diff --git a/pkg/cli/secrets_command_test.go b/pkg/cli/secrets_command_test.go index bde0da5f21a..c26d9c94ee4 100644 --- a/pkg/cli/secrets_command_test.go +++ b/pkg/cli/secrets_command_test.go @@ -48,12 +48,12 @@ func TestSecretsCommandStructure(t *testing.T) { tests := []struct { name string expectedUse string - commandCreator func() interface{} + commandCreator func() any }{ { name: "secrets command exists", expectedUse: "secrets", - commandCreator: func() interface{} { + commandCreator: func() any { return NewSecretsCommand() }, }, diff --git a/pkg/cli/security_regression_test.go b/pkg/cli/security_regression_test.go index 0abf25cf448..5264a664cd9 100644 --- a/pkg/cli/security_regression_test.go +++ b/pkg/cli/security_regression_test.go @@ -264,7 +264,8 @@ Test content.` name: "large_markdown_content", contentFunc: func() string { // Create a large but valid markdown content - content := `--- + var content strings.Builder + content.WriteString(`--- on: push permissions: contents: read @@ -272,12 +273,12 @@ permissions: # Large Content Workflow -` +`) // Add lots of content - for i := 0; i < 1000; i++ { - content += "This is paragraph " + string(rune('0'+i%10)) + ".\n\n" + for i := range 1000 { + content.WriteString("This is paragraph " + string(rune('0'+i%10)) + ".\n\n") } - return content + return content.String() }, maxSizeMB: 1, description: "Large markdown should be handled", diff --git a/pkg/cli/tool_graph.go b/pkg/cli/tool_graph.go index 4738d3cb256..6cb3193c0f6 100644 --- a/pkg/cli/tool_graph.go +++ b/pkg/cli/tool_graph.go @@ -48,7 +48,7 @@ func (g *ToolGraph) AddSequence(tools []string) { } // Add transitions between consecutive tools - for i := 0; i < len(tools)-1; i++ { + for i := range len(tools) - 1 { from := tools[i] to := tools[i+1] key := fmt.Sprintf("%s->%s", from, to) @@ -260,7 +260,7 @@ func extractToolSequencesFromRun(run ProcessedRun, verbose bool) [][]string { var tools []string for _, toolCall := range metrics.ToolCalls { // Add each tool based on its call count to approximate sequence - for i := 0; i < toolCall.CallCount; i++ { + for range toolCall.CallCount { tools = append(tools, toolCall.Name) } } diff --git a/pkg/cli/trial_command_test.go b/pkg/cli/trial_command_test.go index 2ade6e9255e..3cefbf98f8c 100644 --- a/pkg/cli/trial_command_test.go +++ b/pkg/cli/trial_command_test.go @@ -493,12 +493,9 @@ func generateSimpleDiff(expected, actual string) string { actualLines := strings.Split(actual, "\n") var diff []string - maxLines := len(expectedLines) - if len(actualLines) > maxLines { - maxLines = len(actualLines) - } + maxLines := max(len(actualLines), len(expectedLines)) - for i := 0; i < maxLines; i++ { + for i := range maxLines { var expectedLine, actualLine string if i < len(expectedLines) { diff --git a/pkg/cli/validation_output_test.go b/pkg/cli/validation_output_test.go index ba42c564ff6..1aa8e7d61f9 100644 --- a/pkg/cli/validation_output_test.go +++ b/pkg/cli/validation_output_test.go @@ -188,8 +188,8 @@ func TestFormatValidationErrorPreservesStructure(t *testing.T) { // Verify the error message contains the original structured content originalMsg := structuredErr.Error() - lines := strings.Split(originalMsg, "\n") - for _, line := range lines { + lines := strings.SplitSeq(originalMsg, "\n") + for line := range lines { if strings.TrimSpace(line) != "" { assert.Contains(t, result, strings.TrimSpace(line), "Structured error should preserve line: %s", line) diff --git a/pkg/cli/vscode_config.go b/pkg/cli/vscode_config.go index 676292d6f2b..511223565f5 100644 --- a/pkg/cli/vscode_config.go +++ b/pkg/cli/vscode_config.go @@ -3,6 +3,7 @@ package cli import ( "encoding/json" "fmt" + "maps" "os" "path/filepath" @@ -38,9 +39,7 @@ func (s *VSCodeSettings) UnmarshalJSON(data []byte) error { if s.Other == nil { s.Other = make(map[string]any) } - for k, v := range raw { - s.Other[k] = v - } + maps.Copy(s.Other, raw) return nil } @@ -51,9 +50,7 @@ func (s VSCodeSettings) MarshalJSON() ([]byte, error) { result := make(map[string]any) // Add all other fields first - for k, v := range s.Other { - result[k] = v - } + maps.Copy(result, s.Other) // Add yaml.schemas if present if len(s.YAMLSchemas) > 0 { diff --git a/pkg/cli/workflow_suggestions_test.go b/pkg/cli/workflow_suggestions_test.go index 99cebbc88ec..053b5a857d5 100644 --- a/pkg/cli/workflow_suggestions_test.go +++ b/pkg/cli/workflow_suggestions_test.go @@ -5,6 +5,7 @@ package cli import ( "os" "path/filepath" + "slices" "testing" ) @@ -94,13 +95,7 @@ func TestSuggestWorkflowNames(t *testing.T) { // Check if expected suggestions are present for _, expected := range tt.expected { - found := false - for _, result := range results { - if result == expected { - found = true - break - } - } + found := slices.Contains(results, expected) if !found && len(tt.expected) > 0 { t.Errorf("Expected suggestion %q not found in results %v", expected, results) } diff --git a/pkg/cli/workflows.go b/pkg/cli/workflows.go index 32f3fab6b54..76e5ab4cf5e 100644 --- a/pkg/cli/workflows.go +++ b/pkg/cli/workflows.go @@ -293,8 +293,8 @@ func extractWorkflowNameFromFile(filePath string) (string, error) { } // Look for first H1 header - lines := strings.Split(result.Markdown, "\n") - for _, line := range lines { + lines := strings.SplitSeq(result.Markdown, "\n") + for line := range lines { line = strings.TrimSpace(line) if strings.HasPrefix(line, "# ") { return strings.TrimSpace(line[2:]), nil diff --git a/pkg/console/golden_test.go b/pkg/console/golden_test.go index 4fd0728ecbf..0c2acc0413d 100644 --- a/pkg/console/golden_test.go +++ b/pkg/console/golden_test.go @@ -4,6 +4,7 @@ package console import ( "os" + "strings" "testing" "github.com/charmbracelet/lipgloss" @@ -121,11 +122,11 @@ func TestGolden_BoxRendering(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Test RenderTitleBox which returns []string output := RenderTitleBox(tt.title, tt.width) - fullOutput := "" + var fullOutput strings.Builder for _, line := range output { - fullOutput += line + "\n" + fullOutput.WriteString(line + "\n") } - golden.RequireEqual(t, []byte(fullOutput)) + golden.RequireEqual(t, []byte(fullOutput.String())) }) } } @@ -578,11 +579,11 @@ func TestGolden_InfoSection(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { output := RenderInfoSection(tt.content) - fullOutput := "" + var fullOutput strings.Builder for _, line := range output { - fullOutput += line + "\n" + fullOutput.WriteString(line + "\n") } - golden.RequireEqual(t, []byte(fullOutput)) + golden.RequireEqual(t, []byte(fullOutput.String())) }) } } diff --git a/pkg/console/progress_test.go b/pkg/console/progress_test.go index 00b5854fdb6..2214bda14d9 100644 --- a/pkg/console/progress_test.go +++ b/pkg/console/progress_test.go @@ -288,7 +288,7 @@ func TestProgressBarEdgeCases(t *testing.T) { bar := NewProgressBar(1000000) // Update with very small increments - for i := int64(0); i <= 10; i++ { + for i := range int64(11) { output := bar.Update(i) assert.NotEmpty(t, output, "Should handle very small increments") } diff --git a/pkg/console/render.go b/pkg/console/render.go index 8ebc791b145..cb6e0f8e6ad 100644 --- a/pkg/console/render.go +++ b/pkg/console/render.go @@ -67,7 +67,7 @@ func renderStruct(val reflect.Value, title string, output *strings.Builder, dept // Track the longest field name for alignment maxFieldLen := 0 - for i := 0; i < val.NumField(); i++ { + for i := range val.NumField() { field := val.Field(i) fieldType := typ.Field(i) tag := parseConsoleTag(fieldType.Tag.Get("console")) @@ -87,7 +87,7 @@ func renderStruct(val reflect.Value, title string, output *strings.Builder, dept } // Iterate through struct fields - for i := 0; i < val.NumField(); i++ { + for i := range val.NumField() { field := val.Field(i) fieldType := typ.Field(i) @@ -173,7 +173,7 @@ func renderSlice(val reflect.Value, title string, output *strings.Builder, depth output.WriteString(RenderTable(config)) } else { // Render as list - for i := 0; i < val.Len(); i++ { + for i := range val.Len() { elem := val.Index(i) fmt.Fprintf(output, " • %v\n", formatFieldValue(elem)) } @@ -225,7 +225,7 @@ func buildTableConfig(val reflect.Value, title string) TableConfig { var fieldIndices []int var fieldTags []consoleTag - for i := 0; i < elemType.NumField(); i++ { + for i := range elemType.NumField() { field := elemType.Field(i) tag := parseConsoleTag(field.Tag.Get("console")) @@ -248,7 +248,7 @@ func buildTableConfig(val reflect.Value, title string) TableConfig { config.Headers = headers // Build rows - for i := 0; i < val.Len(); i++ { + for i := range val.Len() { elem := val.Index(i) // Dereference pointer if needed for elem.Kind() == reflect.Ptr { @@ -293,21 +293,21 @@ func parseConsoleTag(tag string) consoleTag { return result } - parts := strings.Split(tag, ",") - for _, part := range parts { + parts := strings.SplitSeq(tag, ",") + for part := range parts { part = strings.TrimSpace(part) if part == "omitempty" { result.omitempty = true - } else if strings.HasPrefix(part, "title:") { - result.title = strings.TrimPrefix(part, "title:") - } else if strings.HasPrefix(part, "header:") { - result.header = strings.TrimPrefix(part, "header:") - } else if strings.HasPrefix(part, "format:") { - result.format = strings.TrimPrefix(part, "format:") - } else if strings.HasPrefix(part, "default:") { - result.defaultVal = strings.TrimPrefix(part, "default:") - } else if strings.HasPrefix(part, "maxlen:") { - maxLenStr := strings.TrimPrefix(part, "maxlen:") + } else if after, ok := strings.CutPrefix(part, "title:"); ok { + result.title = after + } else if after, ok := strings.CutPrefix(part, "header:"); ok { + result.header = after + } else if after, ok := strings.CutPrefix(part, "format:"); ok { + result.format = after + } else if after, ok := strings.CutPrefix(part, "default:"); ok { + result.defaultVal = after + } else if after, ok := strings.CutPrefix(part, "maxlen:"); ok { + maxLenStr := after if len, err := strconv.Atoi(maxLenStr); err == nil { result.maxLen = len } diff --git a/pkg/console/render_slice_test.go b/pkg/console/render_slice_test.go index ec2f3149d9e..06bdcd4a6d4 100644 --- a/pkg/console/render_slice_test.go +++ b/pkg/console/render_slice_test.go @@ -204,7 +204,7 @@ func TestRenderSlice_DoublePointerToStructsAsTable(t *testing.T) { pps2 := &ps2 // Create slice with reflect - sliceType := reflect.TypeOf((**SliceTestStruct)(nil)) + sliceType := reflect.TypeFor[**SliceTestStruct]() val := reflect.MakeSlice(reflect.SliceOf(sliceType), 0, 2) val = reflect.Append(val, reflect.ValueOf(pps1)) val = reflect.Append(val, reflect.ValueOf(pps2)) diff --git a/pkg/console/spinner_test.go b/pkg/console/spinner_test.go index 1735d8bd80e..ab89311459b 100644 --- a/pkg/console/spinner_test.go +++ b/pkg/console/spinner_test.go @@ -94,7 +94,7 @@ func TestSpinnerMultipleStartStop(t *testing.T) { spinner := NewSpinner("Test message") // Test multiple start/stop cycles - for i := 0; i < 3; i++ { + for range 3 { spinner.Start() time.Sleep(10 * time.Millisecond) spinner.Stop() @@ -126,7 +126,7 @@ func TestSpinnerConcurrentAccess(t *testing.T) { }() // Wait for all goroutines - for i := 0; i < 3; i++ { + for range 3 { <-done } } @@ -194,7 +194,7 @@ func TestSpinnerRapidStartStop(t *testing.T) { spinner := NewSpinner("Test message") // Test rapid start/stop cycles - for i := 0; i < 10; i++ { + for range 10 { spinner.Start() spinner.Stop() } @@ -229,13 +229,13 @@ func TestSpinnerStopBeforeStartRaceCondition(t *testing.T) { // Even if spinner is disabled in test environment, test the logic // by verifying that multiple Start/Stop cycles don't panic - for i := 0; i < 100; i++ { + for range 100 { spinner.Start() spinner.Stop() } // Also test StopWithMessage immediately after Start - for i := 0; i < 100; i++ { + for range 100 { spinner.Start() spinner.StopWithMessage("Done") } diff --git a/pkg/envutil/envutil_test.go b/pkg/envutil/envutil_test.go index 1bd0316dbe9..bc16e727097 100644 --- a/pkg/envutil/envutil_test.go +++ b/pkg/envutil/envutil_test.go @@ -385,8 +385,8 @@ func BenchmarkGetIntFromEnv_ValidValue(b *testing.B) { defer os.Unsetenv(testEnvVar) log := logger.New("benchmark") - b.ResetTimer() - for i := 0; i < b.N; i++ { + + for b.Loop() { GetIntFromEnv(testEnvVar, 10, 1, 100, log) } } @@ -396,8 +396,8 @@ func BenchmarkGetIntFromEnv_DefaultValue(b *testing.B) { os.Unsetenv(testEnvVar) log := logger.New("benchmark") - b.ResetTimer() - for i := 0; i < b.N; i++ { + + for b.Loop() { GetIntFromEnv(testEnvVar, 10, 1, 100, log) } } @@ -408,8 +408,8 @@ func BenchmarkGetIntFromEnv_InvalidValue(b *testing.B) { defer os.Unsetenv(testEnvVar) log := logger.New("benchmark") - b.ResetTimer() - for i := 0; i < b.N; i++ { + + for b.Loop() { GetIntFromEnv(testEnvVar, 10, 1, 100, log) } } @@ -419,8 +419,7 @@ func BenchmarkGetIntFromEnv_NoLogger(b *testing.B) { os.Setenv(testEnvVar, "50") defer os.Unsetenv(testEnvVar) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { GetIntFromEnv(testEnvVar, 10, 1, 100, nil) } } diff --git a/pkg/logger/error_formatting_test.go b/pkg/logger/error_formatting_test.go index 9d764e60fc6..c07856146ae 100644 --- a/pkg/logger/error_formatting_test.go +++ b/pkg/logger/error_formatting_test.go @@ -162,16 +162,16 @@ func TestExtractErrorMessage(t *testing.T) { func BenchmarkExtractErrorMessage(b *testing.B) { testLine := "2024-01-01T12:00:00.123Z ERROR: connection failed to remote server" - b.ResetTimer() - for i := 0; i < b.N; i++ { + + for b.Loop() { ExtractErrorMessage(testLine) } } func BenchmarkExtractErrorMessageLong(b *testing.B) { testLine := "2024-01-01T12:00:00.123Z ERROR: " + strings.Repeat("very long error message ", 20) - b.ResetTimer() - for i := 0; i < b.N; i++ { + + for b.Loop() { ExtractErrorMessage(testLine) } } diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index b613523db2a..a0303644102 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -148,8 +148,8 @@ func computeEnabled(namespace string) bool { pattern = strings.TrimSpace(pattern) // Handle exclusion patterns (starting with -) - if strings.HasPrefix(pattern, "-") { - excludePattern := strings.TrimPrefix(pattern, "-") + if after, ok := strings.CutPrefix(pattern, "-"); ok { + excludePattern := after if matchPattern(namespace, excludePattern) { return false // Exclusions take precedence } @@ -177,12 +177,12 @@ func matchPattern(namespace, pattern string) bool { if strings.Contains(pattern, "*") { // Replace * with .* for regex-like matching, but keep it simple // Convert pattern to prefix/suffix matching - if strings.HasSuffix(pattern, "*") { - prefix := strings.TrimSuffix(pattern, "*") + if before, ok := strings.CutSuffix(pattern, "*"); ok { + prefix := before return strings.HasPrefix(namespace, prefix) } - if strings.HasPrefix(pattern, "*") { - suffix := strings.TrimPrefix(pattern, "*") + if after, ok := strings.CutPrefix(pattern, "*"); ok { + suffix := after return strings.HasSuffix(namespace, suffix) } // Middle wildcard: split and check both parts diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index f9635376791..ebf55df4852 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -5,6 +5,7 @@ package logger import ( "bytes" "os" + "slices" "strings" "testing" "time" @@ -289,11 +290,8 @@ func TestColorSelection(t *testing.T) { color3 := selectColor("other:namespace") // Just verify it's a valid color from palette or empty found := color3 == "" - for _, c := range colorPalette { - if color3 == c { - found = true - break - } + if slices.Contains(colorPalette, color3) { + found = true } if !found { t.Errorf("selectColor returned invalid color: %q", color3) diff --git a/pkg/logger/slog_adapter.go b/pkg/logger/slog_adapter.go index 984c6969691..dc240e2c32c 100644 --- a/pkg/logger/slog_adapter.go +++ b/pkg/logger/slog_adapter.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "log/slog" + "strings" ) // SlogHandler implements slog.Handler by wrapping a gh-aw Logger @@ -32,7 +33,8 @@ func (h *SlogHandler) Handle(_ context.Context, r slog.Record) error { } // Format the message with attributes - msg := r.Message + var msg strings.Builder + msg.WriteString(r.Message) if r.NumAttrs() > 0 { attrs := make([]any, 0, r.NumAttrs()*2) r.Attrs(func(a slog.Attr) bool { @@ -46,7 +48,7 @@ func (h *SlogHandler) Handle(_ context.Context, r slog.Record) error { if !ok { key = fmt.Sprint(attrs[i]) } - msg += " " + key + "=" + formatSlogValue(attrs[i+1]) + msg.WriteString(" " + key + "=" + formatSlogValue(attrs[i+1])) } } @@ -63,7 +65,7 @@ func (h *SlogHandler) Handle(_ context.Context, r slog.Record) error { levelPrefix = "[ERROR] " } - h.logger.Print(levelPrefix + msg) + h.logger.Print(levelPrefix + msg.String()) return nil } diff --git a/pkg/mathutil/mathutil_test.go b/pkg/mathutil/mathutil_test.go index f31f51f402a..856f1118e6a 100644 --- a/pkg/mathutil/mathutil_test.go +++ b/pkg/mathutil/mathutil_test.go @@ -115,13 +115,13 @@ func TestMax(t *testing.T) { } func BenchmarkMin(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { Min(42, 100) } } func BenchmarkMax(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { Max(42, 100) } } @@ -309,25 +309,25 @@ func TestMax_Identity(t *testing.T) { } func BenchmarkMin_Negative(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { Min(-42, -100) } } func BenchmarkMax_Negative(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { Max(-42, -100) } } func BenchmarkMin_Large(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { Min(2147483647, 2147483646) } } func BenchmarkMax_Large(b *testing.B) { - for i := 0; i < b.N; i++ { + for b.Loop() { Max(2147483647, 2147483646) } } diff --git a/pkg/parser/content_extractor.go b/pkg/parser/content_extractor.go index c99321dd1cd..a20e743ffb8 100644 --- a/pkg/parser/content_extractor.go +++ b/pkg/parser/content_extractor.go @@ -2,6 +2,7 @@ package parser import ( "encoding/json" + "maps" "strings" "github.com/github/gh-aw/pkg/logger" @@ -26,9 +27,7 @@ func extractToolsFromContent(content string) (string, error) { mergeField := func(fieldName string) { if fieldValue, exists := result.Frontmatter[fieldName]; exists { if fieldMap, ok := fieldValue.(map[string]any); ok { - for key, value := range fieldMap { - extracted[key] = value - } + maps.Copy(extracted, fieldMap) } } } diff --git a/pkg/parser/frontmatter_benchmark_test.go b/pkg/parser/frontmatter_benchmark_test.go index 52415f96a38..42bdbbb9e69 100644 --- a/pkg/parser/frontmatter_benchmark_test.go +++ b/pkg/parser/frontmatter_benchmark_test.go @@ -22,8 +22,7 @@ timeout-minutes: 10 This is a test workflow. ` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = ExtractFrontmatterFromContent(content) } } @@ -102,8 +101,7 @@ imports: This is a complex workflow with many features. ` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = ExtractFrontmatterFromContent(content) } } @@ -119,8 +117,7 @@ on: push Simple workflow with minimal configuration. ` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = ExtractFrontmatterFromContent(content) } } @@ -167,8 +164,7 @@ imports: Workflow demonstrating array handling in frontmatter. ` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = ExtractFrontmatterFromContent(content) } } @@ -192,8 +188,7 @@ func BenchmarkValidateSchema(b *testing.B) { "timeout-minutes": 10, } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = ValidateMainWorkflowFrontmatterWithSchema(frontmatter) } } @@ -261,8 +256,7 @@ func BenchmarkValidateSchema_Complex(b *testing.B) { }, } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = ValidateMainWorkflowFrontmatterWithSchema(frontmatter) } } diff --git a/pkg/parser/frontmatter_fuzz_test.go b/pkg/parser/frontmatter_fuzz_test.go index 5a9edc690e2..b8c1f89335f 100644 --- a/pkg/parser/frontmatter_fuzz_test.go +++ b/pkg/parser/frontmatter_fuzz_test.go @@ -199,12 +199,13 @@ on: // Deeply nested structure (testing reasonable nesting limits) // Note: Reduced from 100 to 20 levels to prevent YAML parser from hanging during fuzzing - deepNested := "---\nname: Test\ndata:\n" - for i := 0; i < 20; i++ { - deepNested += strings.Repeat(" ", i+1) + "level" + strconv.Itoa(i%10) + ":\n" + var deepNested strings.Builder + deepNested.WriteString("---\nname: Test\ndata:\n") + for i := range 20 { + deepNested.WriteString(strings.Repeat(" ", i+1) + "level" + strconv.Itoa(i%10) + ":\n") } - deepNested += strings.Repeat(" ", 21) + "value: deep\n---\n# Content" - f.Add(deepNested) + deepNested.WriteString(strings.Repeat(" ", 21) + "value: deep\n---\n# Content") + f.Add(deepNested.String()) // Very large array f.Add(`--- @@ -440,12 +441,13 @@ on: push # Content`) // Very long array with many items - longArray := "---\nname: Test\nitems:\n" - for i := 0; i < 1000; i++ { - longArray += " - item" + strconv.Itoa(i%10) + "\n" + var longArray strings.Builder + longArray.WriteString("---\nname: Test\nitems:\n") + for i := range 1000 { + longArray.WriteString(" - item" + strconv.Itoa(i%10) + "\n") } - longArray += "---\n# Content" - f.Add(longArray) + longArray.WriteString("---\n# Content") + f.Add(longArray.String()) // Run the fuzzer f.Fuzz(func(t *testing.T, content string) { diff --git a/pkg/parser/frontmatter_hash.go b/pkg/parser/frontmatter_hash.go index aa13113a3c0..35c12b4be4e 100644 --- a/pkg/parser/frontmatter_hash.go +++ b/pkg/parser/frontmatter_hash.go @@ -471,7 +471,7 @@ func extractImportsFromText(frontmatterText string) []string { inImports := false baseIndent := 0 - for i := 0; i < len(lines); i++ { + for i := range lines { line := lines[i] trimmed := strings.TrimSpace(line) diff --git a/pkg/parser/frontmatter_hash_consistency_test.go b/pkg/parser/frontmatter_hash_consistency_test.go index aa5911ff5a5..b4152990a6b 100644 --- a/pkg/parser/frontmatter_hash_consistency_test.go +++ b/pkg/parser/frontmatter_hash_consistency_test.go @@ -217,7 +217,7 @@ Use ${{ env.TEST_VAR }} and ${{ vars.CONFIG }} // Compute hash 10 times with Go var goHashes []string - for i := 0; i < 10; i++ { + for i := range 10 { hash, err := ComputeFrontmatterHashFromFile(workflowFile, cache) require.NoError(t, err, "Should compute hash iteration %d", i+1) goHashes = append(goHashes, hash) @@ -231,7 +231,7 @@ Use ${{ env.TEST_VAR }} and ${{ vars.CONFIG }} // Compute hash 10 times with JavaScript var jsHashes []string - for i := 0; i < 10; i++ { + for range 10 { hash, err := computeHashViaNode(workflowFile) if err != nil { t.Logf("JavaScript not available, skipping JS stability test") diff --git a/pkg/parser/frontmatter_hash_repository_test.go b/pkg/parser/frontmatter_hash_repository_test.go index 5eb6b04d38a..4a3d8df9260 100644 --- a/pkg/parser/frontmatter_hash_repository_test.go +++ b/pkg/parser/frontmatter_hash_repository_test.go @@ -153,7 +153,7 @@ func extractHashFromLockFile(content string) string { func splitLines(s string) []string { var lines []string start := 0 - for i := 0; i < len(s); i++ { + for i := range len(s) { if s[i] == '\n' { lines = append(lines, s[start:i]) start = i + 1 diff --git a/pkg/parser/frontmatter_hash_stability_test.go b/pkg/parser/frontmatter_hash_stability_test.go index 737ac99dd86..bf926768119 100644 --- a/pkg/parser/frontmatter_hash_stability_test.go +++ b/pkg/parser/frontmatter_hash_stability_test.go @@ -39,10 +39,7 @@ func TestGoJSHashStability(t *testing.T) { // Limit to a reasonable subset for testing (first 10 workflows) // Full validation can be done separately - testCount := 10 - if len(files) < testCount { - testCount = len(files) - } + testCount := min(10, len(files)) files = files[:testCount] cache := NewImportCache(repoRoot) diff --git a/pkg/parser/frontmatter_utils_test.go b/pkg/parser/frontmatter_utils_test.go index 2a18f9c1880..c0142145d93 100644 --- a/pkg/parser/frontmatter_utils_test.go +++ b/pkg/parser/frontmatter_utils_test.go @@ -593,7 +593,7 @@ func BenchmarkStripANSI(b *testing.B) { for _, tc := range testCases { b.Run(tc.name, func(b *testing.B) { - for i := 0; i < b.N; i++ { + for range b.N { StripANSI(tc.input) } }) diff --git a/pkg/parser/import_cycle_test.go b/pkg/parser/import_cycle_test.go index 2a836a4b130..6faebacda3e 100644 --- a/pkg/parser/import_cycle_test.go +++ b/pkg/parser/import_cycle_test.go @@ -175,7 +175,7 @@ imports: // Run the same import processing multiple times var chains [][]string - for i := 0; i < 5; i++ { + for i := range 5 { frontmatter := map[string]any{ "imports": []string{"a.md"}, } diff --git a/pkg/parser/import_processor.go b/pkg/parser/import_processor.go index 6605d22af16..a9d0ff95f0c 100644 --- a/pkg/parser/import_processor.go +++ b/pkg/parser/import_processor.go @@ -3,7 +3,9 @@ package parser import ( "encoding/json" "fmt" + "maps" "path" + "slices" "sort" "strings" @@ -114,8 +116,8 @@ type importQueueItem struct { func parseRemoteOrigin(spec string) *remoteImportOrigin { // Remove section reference if present cleanSpec := spec - if idx := strings.Index(spec, "#"); idx != -1 { - cleanSpec = spec[:idx] + if before, _, ok := strings.Cut(spec, "#"); ok { + cleanSpec = before } // Split on @ to get path and ref @@ -363,9 +365,7 @@ func processImportsFromFrontmatterWithManifestAndSource(frontmatter map[string]a log.Printf("Processing import from queue: %s", item.fullPath) // Merge inputs from this import into the aggregated inputs map - for k, v := range item.inputs { - importInputs[k] = v - } + maps.Copy(importInputs, item.inputs) // Add to processing order processedOrder = append(processedOrder, item.importPath) @@ -1045,13 +1045,7 @@ func topologicalSortImports(imports []string, baseDir string, cache *ImportCache // Find which imports are part of the cycle (those not in result) cycleNodes := make(map[string]bool) for _, imp := range imports { - found := false - for _, processed := range result { - if processed == imp { - found = true - break - } - } + found := slices.Contains(result, imp) if !found { cycleNodes[imp] = true } diff --git a/pkg/parser/include_expander.go b/pkg/parser/include_expander.go index 61e4b0fd7a7..4fbc4286be1 100644 --- a/pkg/parser/include_expander.go +++ b/pkg/parser/include_expander.go @@ -22,7 +22,7 @@ func ExpandIncludesWithManifest(content, baseDir string, extractTools bool) (str currentContent := content visited := make(map[string]bool) - for depth := 0; depth < maxDepth; depth++ { + for depth := range maxDepth { log.Printf("Include expansion depth: %d", depth) // Process includes in current content processedContent, err := processIncludesWithVisited(currentContent, baseDir, extractTools, visited) @@ -92,7 +92,7 @@ func expandIncludesForField(content, baseDir string, extractFunc func(string) (s var results []string currentContent := content - for depth := 0; depth < maxDepth; depth++ { + for range maxDepth { // Process includes in current content to extract the field processedResults, processedContent, err := processIncludesForField(currentContent, baseDir, extractFunc, emptyValue) if err != nil { diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go index e3960cbc1a4..ad054328246 100644 --- a/pkg/parser/remote_fetch.go +++ b/pkg/parser/remote_fetch.go @@ -60,14 +60,14 @@ func isCustomAgentFile(filePath string) bool { func isRepositoryImport(importPath string) bool { // Remove section reference if present cleanPath := importPath - if idx := strings.Index(importPath, "#"); idx != -1 { - cleanPath = importPath[:idx] + if before, _, ok := strings.Cut(importPath, "#"); ok { + cleanPath = before } // Remove ref if present to check the path structure pathWithoutRef := cleanPath - if idx := strings.Index(cleanPath, "@"); idx != -1 { - pathWithoutRef = cleanPath[:idx] + if before, _, ok := strings.Cut(cleanPath, "@"); ok { + pathWithoutRef = before } // Split by slash to count parts @@ -160,8 +160,8 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e func isWorkflowSpec(path string) bool { // Remove section reference if present cleanPath := path - if idx := strings.Index(path, "#"); idx != -1 { - cleanPath = path[:idx] + if before, _, ok := strings.Cut(path, "#"); ok { + cleanPath = before } // Remove ref if present @@ -203,8 +203,8 @@ func downloadIncludeFromWorkflowSpec(spec string, cache *ImportCache) (string, e // Remove section reference if present cleanSpec := spec - if idx := strings.Index(spec, "#"); idx != -1 { - cleanSpec = spec[:idx] + if before, _, ok := strings.Cut(spec, "#"); ok { + cleanSpec = before } // Split on @ to get path and ref diff --git a/pkg/parser/schedule_parser_test.go b/pkg/parser/schedule_parser_test.go index 6cb011f2a68..cb85d135255 100644 --- a/pkg/parser/schedule_parser_test.go +++ b/pkg/parser/schedule_parser_test.go @@ -1027,7 +1027,7 @@ func containsIgnoreCase(s, substr string) bool { func toLower(s string) string { b := make([]byte, len(s)) - for i := 0; i < len(s); i++ { + for i := range len(s) { c := s[i] if 'A' <= c && c <= 'Z' { c += 'a' - 'A' diff --git a/pkg/parser/schema_errors.go b/pkg/parser/schema_errors.go index bcf2ea75868..83b67da5836 100644 --- a/pkg/parser/schema_errors.go +++ b/pkg/parser/schema_errors.go @@ -125,8 +125,8 @@ func isTypeConflictLine(line string) bool { } // Embedded form: "- at '/path': got X, want Y" // Look for ": got " followed by ", want " later in the line - if idx := strings.Index(line, ": got "); idx >= 0 { - afterGot := line[idx+len(": got "):] + if _, after, ok := strings.Cut(line, ": got "); ok { + afterGot := after return strings.Contains(afterGot, ", want ") } return false diff --git a/pkg/parser/schema_strict_documentation_test.go b/pkg/parser/schema_strict_documentation_test.go index d1d5624894f..7ed6fd5b925 100644 --- a/pkg/parser/schema_strict_documentation_test.go +++ b/pkg/parser/schema_strict_documentation_test.go @@ -20,19 +20,19 @@ func TestStrictFieldSchemaDocumentation(t *testing.T) { } // Parse the schema - var schema map[string]interface{} + var schema map[string]any if err := json.Unmarshal(schemaContent, &schema); err != nil { t.Fatalf("Failed to parse schema JSON: %v", err) } // Get the properties section - properties, ok := schema["properties"].(map[string]interface{}) + properties, ok := schema["properties"].(map[string]any) if !ok { t.Fatal("Schema properties section not found or invalid") } // Get the strict field - strictField, ok := properties["strict"].(map[string]interface{}) + strictField, ok := properties["strict"].(map[string]any) if !ok { t.Fatal("Strict field not found in schema properties") } diff --git a/pkg/parser/schema_utilities.go b/pkg/parser/schema_utilities.go index 53f9a5c63b5..6868d511fac 100644 --- a/pkg/parser/schema_utilities.go +++ b/pkg/parser/schema_utilities.go @@ -1,6 +1,8 @@ package parser import ( + "slices" + "github.com/github/gh-aw/pkg/constants" ) @@ -22,13 +24,7 @@ func filterIgnoredFields(frontmatter map[string]any) map[string]any { filtered := make(map[string]any) for key, value := range frontmatter { // Skip ignored fields - ignored := false - for _, ignoredField := range constants.IgnoredFrontmatterFields { - if key == ignoredField { - ignored = true - break - } - } + ignored := slices.Contains(constants.IgnoredFrontmatterFields, key) if !ignored { filtered[key] = value } @@ -37,14 +33,6 @@ func filterIgnoredFields(frontmatter map[string]any) map[string]any { return filtered } -// min returns the smaller of two integers -func min(a, b int) int { - if a < b { - return a - } - return b -} - // removeDuplicates removes duplicate strings from a slice func removeDuplicates(strings []string) []string { seen := make(map[string]bool) diff --git a/pkg/parser/schema_validation.go b/pkg/parser/schema_validation.go index 7ff8c2c8436..41432f5e7a8 100644 --- a/pkg/parser/schema_validation.go +++ b/pkg/parser/schema_validation.go @@ -2,6 +2,7 @@ package parser import ( "fmt" + "maps" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" @@ -109,9 +110,7 @@ func ValidateIncludedFileFrontmatterWithSchema(frontmatter map[string]any) error // To validate shared workflows against the main schema, we temporarily add an 'on' field // This allows us to use the full schema validation while still enforcing the forbidden field check above tempFrontmatter := make(map[string]any) - for k, v := range filtered { - tempFrontmatter[k] = v - } + maps.Copy(tempFrontmatter, filtered) // Add a temporary 'on' field to satisfy the schema's required field tempFrontmatter["on"] = "push" @@ -137,9 +136,7 @@ func ValidateIncludedFileFrontmatterWithSchemaAndLocation(frontmatter map[string // To validate shared workflows against the main schema, we temporarily add an 'on' field tempFrontmatter := make(map[string]any) - for k, v := range filtered { - tempFrontmatter[k] = v - } + maps.Copy(tempFrontmatter, filtered) // Add a temporary 'on' field to satisfy the schema's required field tempFrontmatter["on"] = "push" diff --git a/pkg/parser/tools_merger.go b/pkg/parser/tools_merger.go index 8d47e34a34e..e48eaac47d5 100644 --- a/pkg/parser/tools_merger.go +++ b/pkg/parser/tools_merger.go @@ -4,6 +4,7 @@ import ( "bufio" "encoding/json" "fmt" + "maps" "strings" ) @@ -77,9 +78,7 @@ func MergeTools(base, additional map[string]any) (map[string]any, error) { result := make(map[string]any) // Copy base - for k, v := range base { - result[k] = v - } + maps.Copy(result, base) // Merge additional for key, newValue := range additional { @@ -130,12 +129,8 @@ func MergeTools(base, additional map[string]any) (map[string]any, error) { // Merge allowed arrays merged := mergeAllowedArrays(existingAllowed, newAllowed) mergedMap := make(map[string]any) - for k, v := range existingMap { - mergedMap[k] = v - } - for k, v := range newMap { - mergedMap[k] = v - } + maps.Copy(mergedMap, existingMap) + maps.Copy(mergedMap, newMap) mergedMap["allowed"] = merged result[key] = mergedMap continue @@ -198,9 +193,7 @@ func mergeMCPTools(existing, new map[string]any) (map[string]any, error) { result := make(map[string]any) // Copy existing properties - for k, v := range existing { - result[k] = v - } + maps.Copy(result, existing) // Merge new properties, checking for conflicts for key, newValue := range new { diff --git a/pkg/parser/yaml_error.go b/pkg/parser/yaml_error.go index 2c9429eafa4..c5d39819989 100644 --- a/pkg/parser/yaml_error.go +++ b/pkg/parser/yaml_error.go @@ -223,8 +223,8 @@ func extractFromStringParsing(errStr string, frontmatterLineOffset int) (line in // Parse "yaml: unmarshal errors: line X: message" format (multiline errors) if strings.Contains(errStr, "yaml: unmarshal errors:") && strings.Contains(errStr, "line ") { - lines := strings.Split(errStr, "\n") - for _, errorLine := range lines { + lines := strings.SplitSeq(errStr, "\n") + for errorLine := range lines { errorLine = strings.TrimSpace(errorLine) if strings.Contains(errorLine, "line ") && strings.Contains(errorLine, ":") { // Extract the first line number found in the error diff --git a/pkg/parser/yaml_import_e2e_test.go b/pkg/parser/yaml_import_e2e_test.go index e0574a98599..00c6b3c6531 100644 --- a/pkg/parser/yaml_import_e2e_test.go +++ b/pkg/parser/yaml_import_e2e_test.go @@ -4,6 +4,7 @@ package parser import ( "encoding/json" + "maps" "os" "path/filepath" "testing" @@ -101,9 +102,7 @@ This workflow imports a YAML workflow and adds additional jobs.` } var jobs map[string]any if err := json.Unmarshal([]byte(jsonLine), &jobs); err == nil { - for k, v := range jobs { - allJobs[k] = v - } + maps.Copy(allJobs, jobs) } } } diff --git a/pkg/repoutil/repoutil.go b/pkg/repoutil/repoutil.go index 2fd7c6bde90..8d793e9dfc9 100644 --- a/pkg/repoutil/repoutil.go +++ b/pkg/repoutil/repoutil.go @@ -30,8 +30,8 @@ func ParseGitHubURL(url string) (owner, repo string, err error) { var repoPath string // SSH format: git@github.com:owner/repo.git - if strings.HasPrefix(url, "git@github.com:") { - repoPath = strings.TrimPrefix(url, "git@github.com:") + if after, ok := strings.CutPrefix(url, "git@github.com:"); ok { + repoPath = after log.Printf("Detected SSH format, extracted path: %s", repoPath) } else if strings.Contains(url, "github.com/") { // HTTPS format: https://github.com/owner/repo.git diff --git a/pkg/repoutil/repoutil_test.go b/pkg/repoutil/repoutil_test.go index 71a13d1f65d..03619848c2e 100644 --- a/pkg/repoutil/repoutil_test.go +++ b/pkg/repoutil/repoutil_test.go @@ -185,21 +185,21 @@ func TestSanitizeForFilename(t *testing.T) { func BenchmarkSplitRepoSlug(b *testing.B) { slug := "github/gh-aw" - for i := 0; i < b.N; i++ { + for b.Loop() { _, _, _ = SplitRepoSlug(slug) } } func BenchmarkParseGitHubURL(b *testing.B) { url := "https://github.com/github/gh-aw.git" - for i := 0; i < b.N; i++ { + for b.Loop() { _, _, _ = ParseGitHubURL(url) } } func BenchmarkSanitizeForFilename(b *testing.B) { slug := "github/gh-aw" - for i := 0; i < b.N; i++ { + for b.Loop() { _ = SanitizeForFilename(slug) } } @@ -447,28 +447,28 @@ func TestSplitRepoSlug_Idempotent(t *testing.T) { func BenchmarkSplitRepoSlug_Valid(b *testing.B) { slug := "github/gh-aw" - for i := 0; i < b.N; i++ { + for b.Loop() { _, _, _ = SplitRepoSlug(slug) } } func BenchmarkSplitRepoSlug_Invalid(b *testing.B) { slug := "invalid" - for i := 0; i < b.N; i++ { + for b.Loop() { _, _, _ = SplitRepoSlug(slug) } } func BenchmarkParseGitHubURL_SSH(b *testing.B) { url := "git@github.com:github/gh-aw.git" - for i := 0; i < b.N; i++ { + for b.Loop() { _, _, _ = ParseGitHubURL(url) } } func BenchmarkParseGitHubURL_HTTPS(b *testing.B) { url := "https://github.com/github/gh-aw.git" - for i := 0; i < b.N; i++ { + for b.Loop() { _, _, _ = ParseGitHubURL(url) } } diff --git a/pkg/sliceutil/sliceutil.go b/pkg/sliceutil/sliceutil.go index 58f0b3209b2..265e265a076 100644 --- a/pkg/sliceutil/sliceutil.go +++ b/pkg/sliceutil/sliceutil.go @@ -1,16 +1,14 @@ // Package sliceutil provides utility functions for working with slices. package sliceutil -import "strings" +import ( + "slices" + "strings" +) // Contains checks if a string slice contains a specific string. func Contains(slice []string, item string) bool { - for _, s := range slice { - if s == item { - return true - } - } - return false + return slices.Contains(slice, item) } // ContainsAny checks if a string contains any of the given substrings. diff --git a/pkg/sliceutil/sliceutil_test.go b/pkg/sliceutil/sliceutil_test.go index a725235603d..ce63b5e2c75 100644 --- a/pkg/sliceutil/sliceutil_test.go +++ b/pkg/sliceutil/sliceutil_test.go @@ -196,7 +196,7 @@ func TestContainsIgnoreCase(t *testing.T) { func BenchmarkContains(b *testing.B) { slice := []string{"apple", "banana", "cherry", "date", "elderberry"} - for i := 0; i < b.N; i++ { + for b.Loop() { Contains(slice, "cherry") } } @@ -204,7 +204,7 @@ func BenchmarkContains(b *testing.B) { func BenchmarkContainsAny(b *testing.B) { s := "hello world from the testing framework" substrings := []string{"goodbye", "world", "farewell"} - for i := 0; i < b.N; i++ { + for b.Loop() { ContainsAny(s, substrings...) } } @@ -212,7 +212,7 @@ func BenchmarkContainsAny(b *testing.B) { func BenchmarkContainsIgnoreCase(b *testing.B) { s := "Hello World From The Testing Framework" substr := "world" - for i := 0; i < b.N; i++ { + for b.Loop() { ContainsIgnoreCase(s, substr) } } @@ -222,7 +222,7 @@ func BenchmarkContainsIgnoreCase(b *testing.B) { func TestContains_LargeSlice(t *testing.T) { // Test with a large slice largeSlice := make([]string, 1000) - for i := 0; i < 1000; i++ { + for i := range 1000 { largeSlice[i] = string(rune('a' + i%26)) } diff --git a/pkg/stringutil/identifiers.go b/pkg/stringutil/identifiers.go index 48390e802f2..51d730036ff 100644 --- a/pkg/stringutil/identifiers.go +++ b/pkg/stringutil/identifiers.go @@ -24,13 +24,13 @@ import ( // NormalizeWorkflowName("my.workflow.md") // returns "my.workflow" func NormalizeWorkflowName(name string) string { // Remove .lock.yml extension first (longer extension) - if strings.HasSuffix(name, ".lock.yml") { - return strings.TrimSuffix(name, ".lock.yml") + if before, ok := strings.CutSuffix(name, ".lock.yml"); ok { + return before } // Remove .md extension - if strings.HasSuffix(name, ".md") { - return strings.TrimSuffix(name, ".md") + if before, ok := strings.CutSuffix(name, ".md"); ok { + return before } return name diff --git a/pkg/stringutil/identifiers_test.go b/pkg/stringutil/identifiers_test.go index 279e1046d68..e282901e55c 100644 --- a/pkg/stringutil/identifiers_test.go +++ b/pkg/stringutil/identifiers_test.go @@ -155,14 +155,14 @@ func TestNormalizeSafeOutputIdentifier(t *testing.T) { func BenchmarkNormalizeWorkflowName(b *testing.B) { name := "weekly-research-workflow.lock.yml" - for i := 0; i < b.N; i++ { + for b.Loop() { NormalizeWorkflowName(name) } } func BenchmarkNormalizeSafeOutputIdentifier(b *testing.B) { identifier := "create-pull-request-review-comment" - for i := 0; i < b.N; i++ { + for b.Loop() { NormalizeSafeOutputIdentifier(identifier) } } diff --git a/pkg/stringutil/paths_test.go b/pkg/stringutil/paths_test.go index daae07c9703..caf718d4640 100644 --- a/pkg/stringutil/paths_test.go +++ b/pkg/stringutil/paths_test.go @@ -109,21 +109,21 @@ func TestNormalizePath(t *testing.T) { func BenchmarkNormalizePath(b *testing.B) { path := "a/b/c/./d/../e/f/../../g" - for i := 0; i < b.N; i++ { + for b.Loop() { NormalizePath(path) } } func BenchmarkNormalizePath_Simple(b *testing.B) { path := "a/b/c/d/e" - for i := 0; i < b.N; i++ { + for b.Loop() { NormalizePath(path) } } func BenchmarkNormalizePath_Complex(b *testing.B) { path := "./a/./b/../c/d/../../e/f/g/h/../../../i" - for i := 0; i < b.N; i++ { + for b.Loop() { NormalizePath(path) } } diff --git a/pkg/stringutil/sanitize_test.go b/pkg/stringutil/sanitize_test.go index af401bc5bf5..93ad1db4d89 100644 --- a/pkg/stringutil/sanitize_test.go +++ b/pkg/stringutil/sanitize_test.go @@ -72,7 +72,7 @@ func TestSanitizeErrorMessage(t *testing.T) { func BenchmarkSanitizeErrorMessage(b *testing.B) { message := "Failed to use API_TOKEN and DATABASE_PASSWORD with GitHubToken" - for i := 0; i < b.N; i++ { + for b.Loop() { SanitizeErrorMessage(message) } } @@ -292,14 +292,14 @@ func TestSanitizeErrorMessage_RealWorldExamples(t *testing.T) { func BenchmarkSanitizeErrorMessage_NoSecrets(b *testing.B) { message := "This is a regular error message with no secrets to redact" - for i := 0; i < b.N; i++ { + for b.Loop() { SanitizeErrorMessage(message) } } func BenchmarkSanitizeErrorMessage_ManySecrets(b *testing.B) { message := "Error with API_KEY, DATABASE_PASSWORD, AWS_SECRET, GitHubToken, and DeploySecret" - for i := 0; i < b.N; i++ { + for b.Loop() { SanitizeErrorMessage(message) } } @@ -530,21 +530,21 @@ func TestSanitizeToolID(t *testing.T) { func BenchmarkSanitizeParameterName(b *testing.B) { name := "my-complex-parameter.name" - for i := 0; i < b.N; i++ { + for b.Loop() { SanitizeParameterName(name) } } func BenchmarkSanitizePythonVariableName(b *testing.B) { name := "my-complex-parameter.name" - for i := 0; i < b.N; i++ { + for b.Loop() { SanitizePythonVariableName(name) } } func BenchmarkSanitizeToolID(b *testing.B) { toolID := "mcp-notion-server-mcp" - for i := 0; i < b.N; i++ { + for b.Loop() { SanitizeToolID(toolID) } } diff --git a/pkg/stringutil/stringutil_test.go b/pkg/stringutil/stringutil_test.go index f2efd9c694e..2247f3c1266 100644 --- a/pkg/stringutil/stringutil_test.go +++ b/pkg/stringutil/stringutil_test.go @@ -139,14 +139,14 @@ func TestNormalizeWhitespace(t *testing.T) { func BenchmarkTruncate(b *testing.B) { s := "this is a very long string that needs to be truncated for testing purposes" - for i := 0; i < b.N; i++ { + for b.Loop() { Truncate(s, 30) } } func BenchmarkNormalizeWhitespace(b *testing.B) { content := "line1 \nline2\t\nline3 \t\nline4\n\n" - for i := 0; i < b.N; i++ { + for b.Loop() { NormalizeWhitespace(content) } } @@ -260,27 +260,27 @@ func TestNormalizeWhitespace_OnlyWhitespace(t *testing.T) { func TestNormalizeWhitespace_ManyLines(t *testing.T) { // Test with many lines lines := make([]string, 100) - for i := 0; i < 100; i++ { + for i := range 100 { lines[i] = "line with trailing spaces " } - content := "" + var content strings.Builder for _, line := range lines { - content += line + "\n" + content.WriteString(line + "\n") } - result := NormalizeWhitespace(content) + result := NormalizeWhitespace(content.String()) // Check that all trailing spaces are removed expectedLines := make([]string, 100) - for i := 0; i < 100; i++ { + for i := range 100 { expectedLines[i] = "line with trailing spaces" } - expected := "" + var expected strings.Builder for _, line := range expectedLines { - expected += line + "\n" + expected.WriteString(line + "\n") } - if result != expected { + if result != expected.String() { t.Error("NormalizeWhitespace did not properly normalize many lines") } } @@ -301,28 +301,28 @@ func TestNormalizeWhitespace_PreservesContent(t *testing.T) { func BenchmarkTruncate_Short(b *testing.B) { s := "short" - for i := 0; i < b.N; i++ { + for b.Loop() { Truncate(s, 10) } } func BenchmarkTruncate_Long(b *testing.B) { s := "this is a very very very very very long string that definitely needs truncation" - for i := 0; i < b.N; i++ { + for b.Loop() { Truncate(s, 20) } } func BenchmarkNormalizeWhitespace_NoChange(b *testing.B) { content := "line1\nline2\nline3\n" - for i := 0; i < b.N; i++ { + for b.Loop() { NormalizeWhitespace(content) } } func BenchmarkNormalizeWhitespace_ManyChanges(b *testing.B) { content := "line1 \t \nline2 \t \nline3 \t \n\n\n" - for i := 0; i < b.N; i++ { + for b.Loop() { NormalizeWhitespace(content) } } @@ -559,14 +559,14 @@ func TestStripANSIEscapeCodes(t *testing.T) { func BenchmarkStripANSIEscapeCodes_Clean(b *testing.B) { s := "This is a clean string without any ANSI codes" - for i := 0; i < b.N; i++ { + for b.Loop() { StripANSIEscapeCodes(s) } } func BenchmarkStripANSIEscapeCodes_WithCodes(b *testing.B) { s := "This \x1b[31mhas\x1b[0m some \x1b[1mANSI\x1b[0m codes" - for i := 0; i < b.N; i++ { + for b.Loop() { StripANSIEscapeCodes(s) } } diff --git a/pkg/stringutil/urls_test.go b/pkg/stringutil/urls_test.go index 00d881ab880..17c3373e12e 100644 --- a/pkg/stringutil/urls_test.go +++ b/pkg/stringutil/urls_test.go @@ -127,7 +127,7 @@ func BenchmarkExtractDomainFromURL(b *testing.B) { for _, tc := range testCases { b.Run(tc, func(b *testing.B) { - for i := 0; i < b.N; i++ { + for range b.N { _ = ExtractDomainFromURL(tc) } }) diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index d2facbd156c..7a93a8329ca 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -356,13 +356,13 @@ func ApplyActionPinToTypedStep(step *WorkflowStep, data *WorkflowData) *Workflow // - "actions/checkout" -> "actions/checkout" func extractActionRepo(uses string) string { // Split on @ to separate repo from version/ref - idx := strings.Index(uses, "@") - if idx == -1 { + before, _, ok := strings.Cut(uses, "@") + if !ok { // No @ found, return the whole string return uses } // Return everything before the @ - return uses[:idx] + return before } // extractActionVersion extracts the version from a uses string @@ -372,13 +372,13 @@ func extractActionRepo(uses string) string { // - "actions/checkout" -> "" func extractActionVersion(uses string) string { // Split on @ to separate repo from version/ref - idx := strings.Index(uses, "@") - if idx == -1 { + _, after, ok := strings.Cut(uses, "@") + if !ok { // No @ found, no version return "" } // Return everything after the @ - return uses[idx+1:] + return after } // ApplyActionPinsToTypedSteps applies SHA pinning to a slice of typed WorkflowStep pointers diff --git a/pkg/workflow/action_pins_test.go b/pkg/workflow/action_pins_test.go index 53fda38d9b1..91ef2797065 100644 --- a/pkg/workflow/action_pins_test.go +++ b/pkg/workflow/action_pins_test.go @@ -65,13 +65,13 @@ func TestGetActionPinReturnsValidSHA(t *testing.T) { // Extract SHA (before the comment marker " # ") shaAndComment := parts[1] - commentIdx := strings.Index(shaAndComment, " # ") - if commentIdx == -1 { + before, _, ok := strings.Cut(shaAndComment, " # ") + if !ok { t.Errorf("GetActionPin(%s) = %s, expected comment with version tag", pin.Repo, result) return } - sha := shaAndComment[:commentIdx] + sha := before // All action pins should have valid SHAs if !isValidSHA(sha) { @@ -303,7 +303,7 @@ func TestGetActionPinsSorting(t *testing.T) { } // Verify they are sorted by version (descending) then by repository name (ascending) - for i := 0; i < len(pins)-1; i++ { + for i := range len(pins) - 1 { if pins[i].Version < pins[i+1].Version { t.Errorf("Pins not sorted correctly by version: %s (v%s) should come before %s (v%s)", pins[i].Repo, pins[i].Version, pins[i+1].Repo, pins[i+1].Version) @@ -923,9 +923,9 @@ func TestApplyActionPinsToTypedSteps(t *testing.T) { parts := strings.Split(got[i].Uses, "@") if len(parts) == 2 { shaAndComment := parts[1] - commentIdx := strings.Index(shaAndComment, " # ") - if commentIdx != -1 { - sha := shaAndComment[:commentIdx] + before, _, ok := strings.Cut(shaAndComment, " # ") + if ok { + sha := before if len(sha) != 40 { t.Errorf("ApplyActionPinsToTypedSteps() step %d uses SHA length = %d, want 40", i, len(sha)) @@ -1187,7 +1187,7 @@ func TestActionPinWarningDeduplication(t *testing.T) { os.Stderr = w // Call GetActionPinWithData multiple times - for i := 0; i < tt.callCount; i++ { + for range tt.callCount { _, _ = GetActionPinWithData(tt.repo, tt.version, data) } diff --git a/pkg/workflow/add_comment_dependencies_test.go b/pkg/workflow/add_comment_dependencies_test.go index aff34b80d8d..1f20690e089 100644 --- a/pkg/workflow/add_comment_dependencies_test.go +++ b/pkg/workflow/add_comment_dependencies_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "strings" "testing" ) @@ -120,13 +121,7 @@ func TestAddCommentJobDependencies(t *testing.T) { t.Errorf("Expected %d needs, got %d", len(tt.expectedNeeds), len(job.Needs)) } for _, expectedNeed := range tt.expectedNeeds { - found := false - for _, need := range job.Needs { - if need == expectedNeed { - found = true - break - } - } + found := slices.Contains(job.Needs, expectedNeed) if !found { t.Errorf("Expected need '%s' not found in job.Needs: %v", expectedNeed, job.Needs) } diff --git a/pkg/workflow/agentic_engine_test.go b/pkg/workflow/agentic_engine_test.go index b6db01e023a..e3352441f64 100644 --- a/pkg/workflow/agentic_engine_test.go +++ b/pkg/workflow/agentic_engine_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "testing" ) @@ -14,13 +15,7 @@ func TestEngineRegistry(t *testing.T) { supportedEngines := registry.GetSupportedEngines() expectedEngineIDs := []string{"claude", "codex", "copilot", "gemini"} for _, engineID := range expectedEngineIDs { - found := false - for _, id := range supportedEngines { - if id == engineID { - found = true - break - } - } + found := slices.Contains(supportedEngines, engineID) if !found { t.Errorf("Expected engine '%s' to be registered", engineID) } diff --git a/pkg/workflow/artifact_manager.go b/pkg/workflow/artifact_manager.go index d82a931477a..4e49cf14f0e 100644 --- a/pkg/workflow/artifact_manager.go +++ b/pkg/workflow/artifact_manager.go @@ -243,7 +243,7 @@ func findCommonParent(paths []string) string { // Find common prefix by comparing each component var commonParts []string - for i := 0; i < minLen-1; i++ { // minLen-1 to exclude the filename + for i := range minLen - 1 { // minLen-1 to exclude the filename part := splitPaths[0][i] allMatch := true for _, sp := range splitPaths[1:] { @@ -415,14 +415,14 @@ func matchesPattern(name, pattern string) bool { } // Handle leading wildcard: "*suffix" - if strings.HasPrefix(pattern, "*") { - suffix := strings.TrimPrefix(pattern, "*") + if after, ok := strings.CutPrefix(pattern, "*"); ok { + suffix := after return strings.HasSuffix(name, suffix) } // Handle trailing wildcard: "prefix*" - if strings.HasSuffix(pattern, "*") { - prefix := strings.TrimSuffix(pattern, "*") + if before, ok := strings.CutSuffix(pattern, "*"); ok { + prefix := before return strings.HasPrefix(name, prefix) } diff --git a/pkg/workflow/bash_defaults_consistency_test.go b/pkg/workflow/bash_defaults_consistency_test.go index fe09eaf8735..fb2e3849091 100644 --- a/pkg/workflow/bash_defaults_consistency_test.go +++ b/pkg/workflow/bash_defaults_consistency_test.go @@ -125,7 +125,7 @@ func TestBashDefaultsConsistency(t *testing.T) { copilotHasShell := false copilotHasGit := false - for i := 0; i < len(copilotResult); i++ { + for i := range copilotResult { // Check for --allow-all-tools flag if copilotResult[i] == "--allow-all-tools" { copilotHasShell = true // --allow-all-tools includes shell diff --git a/pkg/workflow/bundler.go b/pkg/workflow/bundler.go index c9c20f49ee1..8c2f9c84f3a 100644 --- a/pkg/workflow/bundler.go +++ b/pkg/workflow/bundler.go @@ -529,8 +529,8 @@ func deduplicateRequires(content string) string { var varNames []string var destructuredNames []string for _, imp := range imports { - if strings.HasPrefix(imp, "VAR:") { - varNames = append(varNames, strings.TrimPrefix(imp, "VAR:")) + if after, ok := strings.CutPrefix(imp, "VAR:"); ok { + varNames = append(varNames, after) } else { destructuredNames = append(destructuredNames, imp) } diff --git a/pkg/workflow/bundler_file_mode.go b/pkg/workflow/bundler_file_mode.go index b0ac3dad13c..ccac0ecfd0b 100644 --- a/pkg/workflow/bundler_file_mode.go +++ b/pkg/workflow/bundler_file_mode.go @@ -294,8 +294,8 @@ func GenerateWriteScriptsStep(files []ScriptFile) []string { steps = append(steps, fmt.Sprintf(" cat > %s << '%s'\n", filePath, delimiter)) // Write content line by line - lines := strings.Split(file.Content, "\n") - for _, line := range lines { + lines := strings.SplitSeq(file.Content, "\n") + for line := range lines { steps = append(steps, fmt.Sprintf(" %s\n", line)) } diff --git a/pkg/workflow/bundler_inline_test.go b/pkg/workflow/bundler_inline_test.go index 08a1393314f..fd243ba7a8c 100644 --- a/pkg/workflow/bundler_inline_test.go +++ b/pkg/workflow/bundler_inline_test.go @@ -45,12 +45,12 @@ function initLogFile(server) { // Check that they come before fs.existsSync usage fsRequireIndex := strings.Index(output, `require("fs")`) fsUsageIndex := strings.Index(output, "fs.existsSync") - pathRequireIndex := strings.Index(output, `require("path")`) + found := strings.Contains(output, `require("path")`) if fsRequireIndex == -1 { t.Error("fs require not found") } - if pathRequireIndex == -1 { + if !found { t.Error("path require not found") } if fsUsageIndex != -1 && fsRequireIndex > fsUsageIndex { diff --git a/pkg/workflow/bundler_quotes_test.go b/pkg/workflow/bundler_quotes_test.go index a502c1b5276..4ecce1e7753 100644 --- a/pkg/workflow/bundler_quotes_test.go +++ b/pkg/workflow/bundler_quotes_test.go @@ -35,7 +35,7 @@ function test() { } // Check that path is defined before its use - fsIndex := strings.Index(output, "const fs") + found := strings.Contains(output, "const fs") pathIndex := strings.Index(output, "const path") joinIndex := strings.Index(output, "path.join") @@ -48,7 +48,7 @@ function test() { if pathIndex > joinIndex { t.Errorf("path require appears after path.join usage (path at %d, join at %d)", pathIndex, joinIndex) } - if fsIndex == -1 { + if !found { t.Error("fs require is missing") } } diff --git a/pkg/workflow/checkout_runtime_order_test.go b/pkg/workflow/checkout_runtime_order_test.go index 8960f38ff82..da9111e582d 100644 --- a/pkg/workflow/checkout_runtime_order_test.go +++ b/pkg/workflow/checkout_runtime_order_test.go @@ -86,7 +86,7 @@ steps: if len(line) > 2 && line[0] == ' ' && line[1] == ' ' && line[2] != ' ' && line[2] != '\t' { // Calculate the position in the original string nextJobStart = 0 - for j := 0; j < i; j++ { + for j := range i { nextJobStart += len(lines[j]) + 1 // +1 for newline } break @@ -106,8 +106,8 @@ steps: // Find all step names in order stepNames := []string{} - stepLines := strings.Split(agentJobSection, "\n") - for _, line := range stepLines { + stepLines := strings.SplitSeq(agentJobSection, "\n") + for line := range stepLines { // Check if line contains "- name:" (with any amount of leading whitespace) if strings.Contains(line, "- name:") { // Extract the name part after "- name:" @@ -263,7 +263,7 @@ Run node --version to check the Node.js version. if len(line) > 2 && line[0] == ' ' && line[1] == ' ' && line[2] != ' ' && line[2] != '\t' { // Calculate the position in the original string nextJobStart = 0 - for j := 0; j < i; j++ { + for j := range i { nextJobStart += len(lines[j]) + 1 // +1 for newline } break @@ -279,8 +279,8 @@ Run node --version to check the Node.js version. // Find all step names in order stepNames := []string{} - stepLines := strings.Split(agentJobSection, "\n") - for _, line := range stepLines { + stepLines := strings.SplitSeq(agentJobSection, "\n") + for line := range stepLines { // Check if line contains "- name:" (with any amount of leading whitespace) if strings.Contains(line, "- name:") { // Extract the name part after "- name:" diff --git a/pkg/workflow/cjs_require_validation_test.go b/pkg/workflow/cjs_require_validation_test.go index 772fec8f240..893fd5a2595 100644 --- a/pkg/workflow/cjs_require_validation_test.go +++ b/pkg/workflow/cjs_require_validation_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "strings" "testing" ) @@ -186,10 +187,5 @@ func TestCJSFilesNoActionsRequires(t *testing.T) { // sliceContainsString checks if a string slice contains a specific string func sliceContainsString(slice []string, str string) bool { - for _, s := range slice { - if s == str { - return true - } - } - return false + return slices.Contains(slice, str) } diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index 844b85e5ea0..f5cf0499af6 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "strings" "time" @@ -385,17 +386,13 @@ func (e *ClaudeEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str // Add custom environment variables from engine config if workflowData.EngineConfig != nil && len(workflowData.EngineConfig.Env) > 0 { - for key, value := range workflowData.EngineConfig.Env { - env[key] = value - } + maps.Copy(env, workflowData.EngineConfig.Env) } // Add custom environment variables from agent config agentConfig := getAgentConfig(workflowData) if agentConfig != nil && len(agentConfig.Env) > 0 { - for key, value := range agentConfig.Env { - env[key] = value - } + maps.Copy(env, agentConfig.Env) claudeLog.Printf("Added %d custom env vars from agent config", len(agentConfig.Env)) } diff --git a/pkg/workflow/claude_engine_tools_test.go b/pkg/workflow/claude_engine_tools_test.go index fbf142b5ae2..916f890b906 100644 --- a/pkg/workflow/claude_engine_tools_test.go +++ b/pkg/workflow/claude_engine_tools_test.go @@ -4,6 +4,7 @@ package workflow import ( "fmt" + "slices" "sort" "strings" "testing" @@ -300,14 +301,14 @@ func TestClaudeEngineComputeAllowedTools(t *testing.T) { // Parse expected and actual results into sets for comparison expectedTools := make(map[string]bool) if tt.expected != "" { - for _, tool := range strings.Split(tt.expected, ",") { + for tool := range strings.SplitSeq(tt.expected, ",") { expectedTools[strings.TrimSpace(tool)] = true } } actualTools := make(map[string]bool) if result != "" { - for _, tool := range strings.Split(result, ",") { + for tool := range strings.SplitSeq(result, ",") { actualTools[strings.TrimSpace(tool)] = true } } @@ -426,13 +427,7 @@ func TestClaudeEngineComputeAllowedToolsWithSafeOutputs(t *testing.T) { if expectedTool == "" { continue // Skip empty strings } - found := false - for _, actualTool := range resultTools { - if actualTool == expectedTool { - found = true - break - } - } + found := slices.Contains(resultTools, expectedTool) if !found { t.Errorf("Expected tool '%s' not found in result '%s'", expectedTool, result) } @@ -443,13 +438,7 @@ func TestClaudeEngineComputeAllowedToolsWithSafeOutputs(t *testing.T) { if actual == "" { continue // Skip empty strings } - found := false - for _, expected := range expectedTools { - if expected == actual { - found = true - break - } - } + found := slices.Contains(expectedTools, actual) if !found { t.Errorf("Unexpected tool '%s' found in result '%s'", actual, result) } diff --git a/pkg/workflow/claude_logs.go b/pkg/workflow/claude_logs.go index 723b64aa0ec..b6f4f77e0e9 100644 --- a/pkg/workflow/claude_logs.go +++ b/pkg/workflow/claude_logs.go @@ -30,9 +30,9 @@ func (e *ClaudeEngine) ParseLogMetrics(logContent string, verbose bool) LogMetri } // Process line by line for error counting and fallback parsing - lines := strings.Split(logContent, "\n") + lines := strings.SplitSeq(logContent, "\n") - for _, line := range lines { + for line := range lines { // Skip empty lines if strings.TrimSpace(line) == "" { continue diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index 19acf8572b2..7d05bf098f1 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "regexp" "sort" "strings" @@ -282,17 +283,13 @@ mkdir -p "$CODEX_HOME/logs" // Add custom environment variables from engine config if workflowData.EngineConfig != nil && len(workflowData.EngineConfig.Env) > 0 { - for key, value := range workflowData.EngineConfig.Env { - env[key] = value - } + maps.Copy(env, workflowData.EngineConfig.Env) } // Add custom environment variables from agent config agentConfig := getAgentConfig(workflowData) if agentConfig != nil && len(agentConfig.Env) > 0 { - for key, value := range agentConfig.Env { - env[key] = value - } + maps.Copy(env, agentConfig.Env) codexEngineLog.Printf("Added %d custom env vars from agent config", len(agentConfig.Env)) } @@ -384,14 +381,10 @@ func (e *CodexEngine) expandNeutralToolsToCodexTools(toolsConfig *ToolsConfig) * } // Copy custom tools - for key, value := range toolsConfig.Custom { - result.Custom[key] = value - } + maps.Copy(result.Custom, toolsConfig.Custom) // Copy raw map - for key, value := range toolsConfig.raw { - result.raw[key] = value - } + maps.Copy(result.raw, toolsConfig.raw) // Handle playwright tool by converting it to an MCP tool configuration with copilot agent tools if toolsConfig.Playwright != nil { diff --git a/pkg/workflow/codex_logs.go b/pkg/workflow/codex_logs.go index 9219b1bbcdd..2c3af6625b0 100644 --- a/pkg/workflow/codex_logs.go +++ b/pkg/workflow/codex_logs.go @@ -26,7 +26,7 @@ func (e *CodexEngine) ParseLogMetrics(logContent string, verbose bool) LogMetric var currentSequence []string // Track tool sequence var lastToolName string // Track most recent tool for output size extraction - for i := 0; i < len(lines); i++ { + for i := range lines { line := lines[i] // Skip empty lines diff --git a/pkg/workflow/codex_mcp.go b/pkg/workflow/codex_mcp.go index 0a8f2c9efa3..0a023a116dc 100644 --- a/pkg/workflow/codex_mcp.go +++ b/pkg/workflow/codex_mcp.go @@ -83,8 +83,8 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an yaml.WriteString(" \n") yaml.WriteString(" # Custom configuration\n") // Write the custom config line by line with proper indentation - configLines := strings.Split(workflowData.EngineConfig.Config, "\n") - for _, line := range configLines { + configLines := strings.SplitSeq(workflowData.EngineConfig.Config, "\n") + for line := range configLines { if strings.TrimSpace(line) != "" { yaml.WriteString(" " + line + "\n") } else { diff --git a/pkg/workflow/compile_outputs_pr_test.go b/pkg/workflow/compile_outputs_pr_test.go index 404994e0523..0207007860c 100644 --- a/pkg/workflow/compile_outputs_pr_test.go +++ b/pkg/workflow/compile_outputs_pr_test.go @@ -756,10 +756,7 @@ This workflow tests the create-pull-request with fallback-as-issue disabled. // Find the next job after safe_outputs (to limit our search scope) // Extract a large section after safe_outputs job (next 2000 chars should include all job details) - endIdx := safeOutputsJobStart + 2000 - if endIdx > len(lockContentStr) { - endIdx = len(lockContentStr) - } + endIdx := min(safeOutputsJobStart+2000, len(lockContentStr)) safeOutputsJobSection := lockContentStr[safeOutputsJobStart:endIdx] // Verify permissions in safe_outputs job @@ -858,10 +855,7 @@ This workflow tests the create-pull-request with default fallback-as-issue behav } // Extract a large section after safe_outputs job (next 2000 chars should include all job details) - endIdx := safeOutputsJobStart + 2000 - if endIdx > len(lockContentStr) { - endIdx = len(lockContentStr) - } + endIdx := min(safeOutputsJobStart+2000, len(lockContentStr)) safeOutputsJobSection := lockContentStr[safeOutputsJobStart:endIdx] diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 0f401a69caf..b00fa91e8ca 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "time" @@ -299,12 +300,9 @@ Ensure proper audience validation and trust policies are configured.` // Print informational message if "projects" toolset is explicitly specified // (not when implied by "all", as users unlikely intend to use projects with "all") originalToolsets := workflowData.ParsedTools.GitHub.Toolset.ToStringSlice() - for _, toolset := range originalToolsets { - if toolset == "projects" { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("The 'projects' toolset requires a GitHub token with organization Projects permissions.")) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("See: https://github.github.com/gh-aw/reference/auth/#gh_aw_project_github_token-github-projects-v2")) - break - } + if slices.Contains(originalToolsets, "projects") { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("The 'projects' toolset requires a GitHub token with organization Projects permissions.")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("See: https://github.github.com/gh-aw/reference/auth/#gh_aw_project_github_token-github-projects-v2")) } } diff --git a/pkg/workflow/compiler_activation_jobs.go b/pkg/workflow/compiler_activation_jobs.go index ada6a9e3925..05d1ff40b3f 100644 --- a/pkg/workflow/compiler_activation_jobs.go +++ b/pkg/workflow/compiler_activation_jobs.go @@ -3,6 +3,8 @@ package workflow import ( "encoding/json" "fmt" + "maps" + "slices" "strings" "github.com/github/gh-aw/pkg/constants" @@ -328,9 +330,7 @@ func (c *Compiler) buildPreActivationJob(data *WorkflowData, needsPermissionChec // Merge custom outputs from jobs.pre-activation if present if len(customOutputs) > 0 { compilerActivationJobsLog.Printf("Adding %d custom outputs to pre-activation job", len(customOutputs)) - for key, value := range customOutputs { - outputs[key] = value - } + maps.Copy(outputs, customOutputs) } // Pre-activation job uses the user's original if condition (data.If) @@ -815,13 +815,7 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( } // Check if this job is already in depends - alreadyDepends := false - for _, dep := range depends { - if dep == jobName { - alreadyDepends = true - break - } - } + alreadyDepends := slices.Contains(depends, jobName) // Add it if not already present if !alreadyDepends { depends = append(depends, jobName) diff --git a/pkg/workflow/compiler_artifacts_test.go b/pkg/workflow/compiler_artifacts_test.go index b974eefd808..d0f46b2e573 100644 --- a/pkg/workflow/compiler_artifacts_test.go +++ b/pkg/workflow/compiler_artifacts_test.go @@ -3,6 +3,7 @@ package workflow import ( + "maps" "os" "path/filepath" "strings" @@ -64,9 +65,7 @@ func TestAccessLogUploadConditional(t *testing.T) { } if tt.mcpServers != nil { // Add mcp servers to tools map for the test - for name, config := range tt.mcpServers { - testTools[name] = config - } + maps.Copy(testTools, tt.mcpServers) } // Test generateExtractAccessLogs diff --git a/pkg/workflow/compiler_benchmark_test.go b/pkg/workflow/compiler_benchmark_test.go index 9ad34bbd39b..15af6a0bb3f 100644 --- a/pkg/workflow/compiler_benchmark_test.go +++ b/pkg/workflow/compiler_benchmark_test.go @@ -49,8 +49,7 @@ Issue details: ${{ steps.sanitized.outputs.text }} compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -98,8 +97,7 @@ Review the pull request changes and provide feedback. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -157,8 +155,7 @@ Research latest developments and create a summary. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -203,8 +200,7 @@ Analyze the issue with strict validation enabled. WithStrictMode(true), ) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -277,8 +273,7 @@ Repository: ${{ github.repository }} compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -319,8 +314,7 @@ Analyze repository commits. WithNoEmit(true), // Don't write files ) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -409,8 +403,7 @@ Debug mode: ${{ github.event.inputs.debug }} WithNoEmit(true), // Don't write files ) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } diff --git a/pkg/workflow/compiler_custom_actions_test.go b/pkg/workflow/compiler_custom_actions_test.go index 403ef7ad14c..33c61a5c736 100644 --- a/pkg/workflow/compiler_custom_actions_test.go +++ b/pkg/workflow/compiler_custom_actions_test.go @@ -205,8 +205,8 @@ core.info('Creating issue'); lockStr := string(lockContent) // Verify safe_outputs job exists (consolidated mode) - safeOutputsJobStart := strings.Index(lockStr, "safe_outputs:") - if safeOutputsJobStart == -1 { + found := strings.Contains(lockStr, "safe_outputs:") + if !found { t.Fatal("safe_outputs job not found in lock file") } diff --git a/pkg/workflow/compiler_jobs_test.go b/pkg/workflow/compiler_jobs_test.go index 170cf19d0e4..af339526e28 100644 --- a/pkg/workflow/compiler_jobs_test.go +++ b/pkg/workflow/compiler_jobs_test.go @@ -5,6 +5,7 @@ package workflow import ( "os" "path/filepath" + "slices" "strings" "testing" @@ -198,13 +199,7 @@ func TestGetCustomJobsDependingOnPreActivationEdgeCases(t *testing.T) { } // Check that expected job IDs are present for _, expectedID := range tt.expectedJobIDs { - found := false - for _, job := range result { - if job == expectedID { - found = true - break - } - } + found := slices.Contains(result, expectedID) if !found { t.Errorf("Expected job %q not found in result", expectedID) } @@ -290,13 +285,7 @@ func TestGetReferencedCustomJobs(t *testing.T) { } // Check that expected job IDs are present for _, expectedID := range tt.expectedJobIDs { - found := false - for _, job := range result { - if job == expectedID { - found = true - break - } - } + found := slices.Contains(result, expectedID) if !found { t.Errorf("Expected job %q not found in result", expectedID) } @@ -547,13 +536,7 @@ func TestBuildMainJobWithActivation(t *testing.T) { } // Check that it depends on activation job - found := false - for _, need := range job.Needs { - if need == string(constants.ActivationJobName) { - found = true - break - } - } + found := slices.Contains(job.Needs, string(constants.ActivationJobName)) if !found { t.Errorf("Expected job to depend on %s, got needs: %v", string(constants.ActivationJobName), job.Needs) } @@ -1157,13 +1140,7 @@ func TestJobsWithRepoMemoryDependencies(t *testing.T) { // Verify dependencies include detection when threat detection is enabled if threatDetectionEnabledForSafeJobs { - hasDetectionDep := false - for _, need := range pushRepoMemoryJob.Needs { - if need == string(constants.DetectionJobName) { - hasDetectionDep = true - break - } - } + hasDetectionDep := slices.Contains(pushRepoMemoryJob.Needs, string(constants.DetectionJobName)) if !hasDetectionDep { t.Error("Expected push_repo_memory to depend on detection job when threat detection is enabled") } @@ -1228,13 +1205,7 @@ func TestJobsWithCacheMemoryDependencies(t *testing.T) { } // Verify dependencies include detection - hasDetectionDep := false - for _, need := range updateCacheMemoryJob.Needs { - if need == string(constants.DetectionJobName) { - hasDetectionDep = true - break - } - } + hasDetectionDep := slices.Contains(updateCacheMemoryJob.Needs, string(constants.DetectionJobName)) if !hasDetectionDep { t.Error("Expected update_cache_memory to depend on detection job") } @@ -1955,13 +1926,7 @@ func TestBuildCustomJobsAutomaticActivationDependency(t *testing.T) { } // Check that activation is in the needs array - found := false - for _, need := range job.Needs { - if need == string(constants.ActivationJobName) { - found = true - break - } - } + found := slices.Contains(job.Needs, string(constants.ActivationJobName)) if !found { t.Error("Expected automatic dependency on activation job") } diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index c705bfe6a56..aa1f1720d00 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "maps" "strings" "github.com/github/gh-aw/pkg/logger" @@ -389,9 +390,7 @@ func (c *Compiler) mergeJobsFromYAMLImports(mainJobs map[string]any, mergedJobsJ // Initialize result with main jobs or create empty map result := make(map[string]any) - for k, v := range mainJobs { - result[k] = v - } + maps.Copy(result, mainJobs) // Split by newlines to handle multiple JSON objects from different imports lines := strings.Split(mergedJobsJSON, "\n") diff --git a/pkg/workflow/compiler_performance_benchmark_test.go b/pkg/workflow/compiler_performance_benchmark_test.go index 0348bf4a095..1326d25f03e 100644 --- a/pkg/workflow/compiler_performance_benchmark_test.go +++ b/pkg/workflow/compiler_performance_benchmark_test.go @@ -42,9 +42,8 @@ Analyze the issue: ${{ steps.sanitized.outputs.text }} compiler := NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -103,9 +102,8 @@ Review the pull request: ${{ github.event.pull_request.number }} compiler := NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -154,9 +152,8 @@ Review and test the pull request with multiple tools. compiler := NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -207,11 +204,10 @@ Standard workflow for memory profiling. compiler := NewCompiler() - b.ResetTimer() b.ReportAllocs() // Track memory allocations - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -248,9 +244,8 @@ Test parsing performance. compiler := NewCompiler() - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = compiler.ParseWorkflowFile(testFile) } } @@ -294,9 +289,8 @@ Test validation performance. compiler := NewCompiler() compiler.SetStrictMode(true) - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } @@ -331,9 +325,8 @@ Test YAML generation. compiler := NewCompiler() compiler.SetNoEmit(true) - b.ResetTimer() b.ReportAllocs() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = compiler.CompileWorkflow(testFile) } } diff --git a/pkg/workflow/compiler_safe_outputs.go b/pkg/workflow/compiler_safe_outputs.go index 78346d96abc..ea6b9475b28 100644 --- a/pkg/workflow/compiler_safe_outputs.go +++ b/pkg/workflow/compiler_safe_outputs.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "maps" "path/filepath" "strings" @@ -295,9 +296,7 @@ func (c *Compiler) applyDefaultTools(tools map[string]any, safeOutputs *SafeOutp if toolConfig, ok := githubTool.(map[string]any); ok { githubConfig = make(map[string]any) - for k, v := range toolConfig { - githubConfig[k] = v - } + maps.Copy(githubConfig, toolConfig) } else { githubConfig = make(map[string]any) } diff --git a/pkg/workflow/compiler_safe_outputs_pr_expires_test.go b/pkg/workflow/compiler_safe_outputs_pr_expires_test.go index 8da8a1427ea..c5444355b97 100644 --- a/pkg/workflow/compiler_safe_outputs_pr_expires_test.go +++ b/pkg/workflow/compiler_safe_outputs_pr_expires_test.go @@ -90,10 +90,10 @@ Test workflow for _, line := range lines { if strings.Contains(line, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG:") { // Extract the JSON value - it's in quotes and may have escaped characters - start := strings.Index(line, ": ") - if start >= 0 { + _, after, ok := strings.Cut(line, ": ") + if ok { // Get everything after ": " and trim quotes - jsonPart := strings.TrimSpace(line[start+2:]) + jsonPart := strings.TrimSpace(after) if strings.HasPrefix(jsonPart, `"`) && strings.HasSuffix(jsonPart, `"`) { // Remove surrounding quotes jsonPart = jsonPart[1 : len(jsonPart)-1] diff --git a/pkg/workflow/compiler_safe_outputs_specialized.go b/pkg/workflow/compiler_safe_outputs_specialized.go index ef89a523608..a69b984aff5 100644 --- a/pkg/workflow/compiler_safe_outputs_specialized.go +++ b/pkg/workflow/compiler_safe_outputs_specialized.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "strings" "github.com/github/gh-aw/pkg/logger" ) @@ -55,14 +56,14 @@ func (c *Compiler) buildAssignToAgentStepConfig(data *WorkflowData, mainJobName // Add allowed agents list environment variable (comma-separated) if len(cfg.Allowed) > 0 { - allowedStr := "" + var allowedStr strings.Builder for i, agent := range cfg.Allowed { if i > 0 { - allowedStr += "," + allowedStr.WriteString(",") } - allowedStr += agent + allowedStr.WriteString(agent) } - customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_AGENT_ALLOWED: %q\n", allowedStr)) + customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_AGENT_ALLOWED: %q\n", allowedStr.String())) } // Add ignore-if-error flag if set @@ -82,14 +83,14 @@ func (c *Compiler) buildAssignToAgentStepConfig(data *WorkflowData, mainJobName // Add allowed PR repos list environment variable (comma-separated) if len(cfg.AllowedPullRequestRepos) > 0 { - allowedPullRequestReposStr := "" + var allowedPullRequestReposStr strings.Builder for i, repo := range cfg.AllowedPullRequestRepos { if i > 0 { - allowedPullRequestReposStr += "," + allowedPullRequestReposStr.WriteString(",") } - allowedPullRequestReposStr += repo + allowedPullRequestReposStr.WriteString(repo) } - customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_AGENT_ALLOWED_PULL_REQUEST_REPOS: %q\n", allowedPullRequestReposStr)) + customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_AGENT_ALLOWED_PULL_REQUEST_REPOS: %q\n", allowedPullRequestReposStr.String())) } // Allow assign_to_agent to reference issues created earlier in the same run via temporary IDs (aw_...) diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index e4c700dfb66..d8e5e6828b5 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -781,7 +781,7 @@ This is a test workflow for concurrent compilation. // Create multiple workflow files var workflowFiles []string - for i := 0; i < numWorkers*workflowsPerWorker; i++ { + for i := range numWorkers * workflowsPerWorker { testFile := filepath.Join(tmpDir, fmt.Sprintf("workflow-%d.md", i)) require.NoError(t, os.WriteFile(testFile, []byte(testContent), 0644), "Failed to write test file %d", i) workflowFiles = append(workflowFiles, testFile) diff --git a/pkg/workflow/compiler_test_helpers.go b/pkg/workflow/compiler_test_helpers.go index 5d2f70aef26..b71707aa5f5 100644 --- a/pkg/workflow/compiler_test_helpers.go +++ b/pkg/workflow/compiler_test_helpers.go @@ -3,8 +3,8 @@ package workflow import "strings" func containsInNonCommentLines(content, search string) bool { - lines := strings.Split(content, "\n") - for _, line := range lines { + lines := strings.SplitSeq(content, "\n") + for line := range lines { trimmed := strings.TrimLeft(line, " \t") // Skip comment lines if strings.HasPrefix(trimmed, "#") { diff --git a/pkg/workflow/compiler_validation_test.go b/pkg/workflow/compiler_validation_test.go index 01d825bd769..42563234e24 100644 --- a/pkg/workflow/compiler_validation_test.go +++ b/pkg/workflow/compiler_validation_test.go @@ -272,7 +272,7 @@ This is a test workflow. } // Check that the first 4 lines are comment lines (disclaimer) - for i := 0; i < 4; i++ { + for i := range 4 { if !strings.HasPrefix(lines[i], "#") { t.Errorf("Line %d should be a comment (disclaimer), but got: %s", i+1, lines[i]) } diff --git a/pkg/workflow/compiler_yaml.go b/pkg/workflow/compiler_yaml.go index c0dcec63ea8..aff4fcf5315 100644 --- a/pkg/workflow/compiler_yaml.go +++ b/pkg/workflow/compiler_yaml.go @@ -67,8 +67,8 @@ func (c *Compiler) generateWorkflowHeader(yaml *strings.Builder, data *WorkflowD if data.Description != "" { cleanDescription := stringutil.StripANSI(data.Description) // Split description into lines and prefix each with "# " - descriptionLines := strings.Split(strings.TrimSpace(cleanDescription), "\n") - for _, line := range descriptionLines { + descriptionLines := strings.SplitSeq(strings.TrimSpace(cleanDescription), "\n") + for line := range descriptionLines { fmt.Fprintf(yaml, "# %s\n", strings.TrimSpace(line)) } } @@ -751,9 +751,9 @@ func processMarkdownBody(body string) ([]string, []*ExpressionMapping) { // so the workspace root is the directory that contains ".github/". func resolveWorkspaceRoot(markdownPath string) string { normalized := filepath.ToSlash(markdownPath) - if idx := strings.Index(normalized, "/.github/"); idx != -1 { + if before, _, ok := strings.Cut(normalized, "/.github/"); ok { // Absolute or non-root-relative path: strip everything from "/.github/" onward. - return filepath.FromSlash(normalized[:idx]) + return filepath.FromSlash(before) } if strings.HasPrefix(normalized, ".github/") { // Path already starts at the workspace root. diff --git a/pkg/workflow/compiler_yaml_main_job.go b/pkg/workflow/compiler_yaml_main_job.go index cf72ecb36c0..004d4c81582 100644 --- a/pkg/workflow/compiler_yaml_main_job.go +++ b/pkg/workflow/compiler_yaml_main_job.go @@ -598,8 +598,8 @@ func (c *Compiler) generateRepositoryImportCheckouts(yaml *strings.Builder, repo func parseRepositoryImportSpec(importSpec string) (owner, repo, ref string) { // Remove section reference if present (file.md#Section) cleanSpec := importSpec - if idx := strings.Index(importSpec, "#"); idx != -1 { - cleanSpec = importSpec[:idx] + if before, _, ok := strings.Cut(importSpec, "#"); ok { + cleanSpec = before } // Split on @ to get path and ref diff --git a/pkg/workflow/compiler_yaml_test.go b/pkg/workflow/compiler_yaml_test.go index 2b2e1af3da8..5ea7ae8d25d 100644 --- a/pkg/workflow/compiler_yaml_test.go +++ b/pkg/workflow/compiler_yaml_test.go @@ -1222,8 +1222,8 @@ This is a test workflow.` t.Errorf("%s: Expected to find '%s' in lock file", tt.description, tt.expected) // Find what runtime-import was actually generated - lines := strings.Split(lockContentStr, "\n") - for _, line := range lines { + lines := strings.SplitSeq(lockContentStr, "\n") + for line := range lines { if strings.Contains(line, "{{#runtime-import") { t.Logf("Found runtime-import: %s", strings.TrimSpace(line)) } @@ -1308,8 +1308,8 @@ Test prompt. // Extract metadata line metadataLine := "" - lines := strings.Split(lockContentStr, "\n") - for _, line := range lines { + lines := strings.SplitSeq(lockContentStr, "\n") + for line := range lines { if strings.Contains(line, "gh-aw-metadata:") { metadataLine = line break diff --git a/pkg/workflow/concurrency_validation.go b/pkg/workflow/concurrency_validation.go index 5de2944581a..2aac5524c98 100644 --- a/pkg/workflow/concurrency_validation.go +++ b/pkg/workflow/concurrency_validation.go @@ -341,8 +341,8 @@ func extractConcurrencyGroupFromYAML(concurrencyYAML string) string { if len(lines) > 0 { firstLine := strings.TrimSpace(lines[0]) // Only match if it starts with "concurrency:" - if strings.HasPrefix(firstLine, "concurrency:") { - value := strings.TrimSpace(strings.TrimPrefix(firstLine, "concurrency:")) + if after, ok := strings.CutPrefix(firstLine, "concurrency:"); ok { + value := strings.TrimSpace(after) // Remove quotes if present value = strings.Trim(value, `"'`) return value diff --git a/pkg/workflow/concurrency_validation_test.go b/pkg/workflow/concurrency_validation_test.go index 98d40c63940..2a58cba9940 100644 --- a/pkg/workflow/concurrency_validation_test.go +++ b/pkg/workflow/concurrency_validation_test.go @@ -701,7 +701,7 @@ func BenchmarkValidateConcurrencyGroupExpression(b *testing.B) { for _, tc := range testCases { b.Run(tc, func(b *testing.B) { - for i := 0; i < b.N; i++ { + for range b.N { _ = validateConcurrencyGroupExpression(tc) } }) @@ -710,8 +710,8 @@ func BenchmarkValidateConcurrencyGroupExpression(b *testing.B) { func BenchmarkValidateBalancedBraces(b *testing.B) { input := "gh-aw-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}" - b.ResetTimer() - for i := 0; i < b.N; i++ { + + for b.Loop() { _ = validateBalancedBraces(input) } } @@ -719,8 +719,8 @@ func BenchmarkValidateBalancedBraces(b *testing.B) { func BenchmarkValidateExpressionContent(b *testing.B) { expr := "(github.workflow || github.ref) && github.repository" fullGroup := "test-${{ (github.workflow || github.ref) && github.repository }}" - b.ResetTimer() - for i := 0; i < b.N; i++ { + + for b.Loop() { _ = validateExpressionContent(expr, fullGroup) } } diff --git a/pkg/workflow/config_parsing_helpers_test.go b/pkg/workflow/config_parsing_helpers_test.go index cb918aed894..c34ed7079e9 100644 --- a/pkg/workflow/config_parsing_helpers_test.go +++ b/pkg/workflow/config_parsing_helpers_test.go @@ -3,6 +3,7 @@ package workflow import ( + "maps" "testing" ) @@ -1043,9 +1044,7 @@ func TestPreprocessExpiresField(t *testing.T) { var configData map[string]any if tt.input != nil { configData = make(map[string]any) - for k, v := range tt.input { - configData[k] = v - } + maps.Copy(configData, tt.input) } disabled := preprocessExpiresField(configData, nil) diff --git a/pkg/workflow/copilot_engine.go b/pkg/workflow/copilot_engine.go index aa6d2a59225..c5972ae7655 100644 --- a/pkg/workflow/copilot_engine.go +++ b/pkg/workflow/copilot_engine.go @@ -118,7 +118,7 @@ func (e *CopilotEngine) GetDeclaredOutputFiles() []string { // extractAddDirPaths extracts all directory paths from copilot args that follow --add-dir flags func extractAddDirPaths(args []string) []string { var dirs []string - for i := 0; i < len(args)-1; i++ { + for i := range len(args) - 1 { if args[i] == "--add-dir" { dirs = append(dirs, args[i+1]) } diff --git a/pkg/workflow/copilot_engine_execution.go b/pkg/workflow/copilot_engine_execution.go index a14ea863cb9..0a5e9b2fbe0 100644 --- a/pkg/workflow/copilot_engine_execution.go +++ b/pkg/workflow/copilot_engine_execution.go @@ -23,6 +23,7 @@ package workflow import ( "fmt" + "maps" "strings" "time" @@ -297,17 +298,13 @@ COPILOT_CLI_INSTRUCTION="$(cat /tmp/gh-aw/aw-prompts/prompt.txt)" // Add custom environment variables from engine config if workflowData.EngineConfig != nil && len(workflowData.EngineConfig.Env) > 0 { - for key, value := range workflowData.EngineConfig.Env { - env[key] = value - } + maps.Copy(env, workflowData.EngineConfig.Env) } // Add custom environment variables from agent config agentConfig := getAgentConfig(workflowData) if agentConfig != nil && len(agentConfig.Env) > 0 { - for key, value := range agentConfig.Env { - env[key] = value - } + maps.Copy(env, agentConfig.Env) copilotExecLog.Printf("Added %d custom env vars from agent config", len(agentConfig.Env)) } diff --git a/pkg/workflow/copilot_logs.go b/pkg/workflow/copilot_logs.go index c635bb1df88..791de7ef2be 100644 --- a/pkg/workflow/copilot_logs.go +++ b/pkg/workflow/copilot_logs.go @@ -238,9 +238,9 @@ func (e *CopilotEngine) ParseLogMetrics(logContent string, verbose bool) LogMetr if hasTimestamp { // Strip the timestamp and [DEBUG] prefix to see what remains // Format: "YYYY-MM-DDTHH:MM:SS.sssZ [DEBUG] {json content}" - debugIndex := strings.Index(line, "[DEBUG]") - if debugIndex != -1 { - cleanLine := strings.TrimSpace(line[debugIndex+7:]) // Skip "[DEBUG]" + _, after, ok := strings.Cut(line, "[DEBUG]") + if ok { + cleanLine := strings.TrimSpace(after) // Skip "[DEBUG]" // If after stripping, the line starts with JSON characters, it's part of JSON // Otherwise, it's a new log entry and we should end the block diff --git a/pkg/workflow/copilot_participant_steps.go b/pkg/workflow/copilot_participant_steps.go index be9ea585f9a..d7fa717f81b 100644 --- a/pkg/workflow/copilot_participant_steps.go +++ b/pkg/workflow/copilot_participant_steps.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "slices" "github.com/github/gh-aw/pkg/logger" ) @@ -44,13 +45,7 @@ func buildCopilotParticipantSteps(config CopilotParticipantConfig) []string { steps = append(steps, " persist-credentials: false\n") // Check if any participant is "copilot" to determine token preference - hasCopilotParticipant := false - for _, participant := range config.Participants { - if participant == "copilot" { - hasCopilotParticipant = true - break - } - } + hasCopilotParticipant := slices.Contains(config.Participants, "copilot") // Choose the first non-empty custom token for precedence effectiveCustomToken := config.CustomToken diff --git a/pkg/workflow/create_discussion_dependencies_test.go b/pkg/workflow/create_discussion_dependencies_test.go index 5f412cc64b7..403797b037a 100644 --- a/pkg/workflow/create_discussion_dependencies_test.go +++ b/pkg/workflow/create_discussion_dependencies_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "strings" "testing" @@ -60,13 +61,7 @@ func TestCreateDiscussionJobDependencies(t *testing.T) { t.Errorf("Expected %d needs, got %d: %v", len(tt.expectedNeeds), len(job.Needs), job.Needs) } for _, expectedNeed := range tt.expectedNeeds { - found := false - for _, need := range job.Needs { - if need == expectedNeed { - found = true - break - } - } + found := slices.Contains(job.Needs, expectedNeed) if !found { t.Errorf("Expected need '%s' not found in job.Needs: %v", expectedNeed, job.Needs) } diff --git a/pkg/workflow/create_issue.go b/pkg/workflow/create_issue.go index 047322cda6d..da25618bddd 100644 --- a/pkg/workflow/create_issue.go +++ b/pkg/workflow/create_issue.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "slices" "github.com/github/gh-aw/pkg/logger" ) @@ -93,12 +94,7 @@ func (c *Compiler) parseIssuesConfig(outputMap map[string]any) *CreateIssuesConf // hasCopilotAssignee checks if "copilot" is in the assignees list func hasCopilotAssignee(assignees []string) bool { - for _, a := range assignees { - if a == "copilot" { - return true - } - } - return false + return slices.Contains(assignees, "copilot") } // filterNonCopilotAssignees returns assignees excluding "copilot" diff --git a/pkg/workflow/create_issue_handler_config_test.go b/pkg/workflow/create_issue_handler_config_test.go index 60eaceaa9e1..369717e9577 100644 --- a/pkg/workflow/create_issue_handler_config_test.go +++ b/pkg/workflow/create_issue_handler_config_test.go @@ -83,13 +83,13 @@ Create an issue with title "Test" and body "Test body". } // Parse the JSON - var config map[string]interface{} + var config map[string]any if err := json.Unmarshal([]byte(configJSON), &config); err != nil { t.Fatalf("Failed to parse handler config JSON: %v\nJSON: %s", err, configJSON) } // Verify create_issue config exists - createIssueConfig, ok := config["create_issue"].(map[string]interface{}) + createIssueConfig, ok := config["create_issue"].(map[string]any) if !ok { t.Fatal("Expected create_issue in handler config") } @@ -100,7 +100,7 @@ Create an issue with title "Test" and body "Test body". } // Verify labels are present - labels, ok := createIssueConfig["labels"].([]interface{}) + labels, ok := createIssueConfig["labels"].([]any) if !ok { t.Fatal("Expected labels array in create_issue config") } @@ -114,7 +114,7 @@ Create an issue with title "Test" and body "Test body". } // Verify assignees are present (this is the main test) - assignees, ok := createIssueConfig["assignees"].([]interface{}) + assignees, ok := createIssueConfig["assignees"].([]any) if !ok { t.Fatal("Expected assignees array in create_issue config") } @@ -185,18 +185,18 @@ Create an issue. } // Parse the JSON - var config map[string]interface{} + var config map[string]any if err := json.Unmarshal([]byte(configJSON), &config); err != nil { t.Fatalf("Failed to parse handler config JSON: %v\nJSON: %s", err, configJSON) } // Verify assignees are present as array - createIssueConfig, ok := config["create_issue"].(map[string]interface{}) + createIssueConfig, ok := config["create_issue"].(map[string]any) if !ok { t.Fatal("Expected create_issue in handler config") } - assignees, ok := createIssueConfig["assignees"].([]interface{}) + assignees, ok := createIssueConfig["assignees"].([]any) if !ok { t.Fatal("Expected assignees array in create_issue config") } @@ -267,20 +267,20 @@ Create an issue. } // Parse the JSON - var config map[string]interface{} + var config map[string]any if err := json.Unmarshal([]byte(configJSON), &config); err != nil { t.Fatalf("Failed to parse handler config JSON: %v\nJSON: %s", err, configJSON) } // Verify create_issue config exists - createIssueConfig, ok := config["create_issue"].(map[string]interface{}) + createIssueConfig, ok := config["create_issue"].(map[string]any) if !ok { t.Fatal("Expected create_issue in handler config") } // Verify assignees field is not present (or is empty) if assignees, ok := createIssueConfig["assignees"]; ok { - if arr, ok := assignees.([]interface{}); ok && len(arr) > 0 { + if arr, ok := assignees.([]any); ok && len(arr) > 0 { t.Errorf("Expected no assignees in create_issue config, got: %v", assignees) } } diff --git a/pkg/workflow/dangerous_permissions_validation_test.go b/pkg/workflow/dangerous_permissions_validation_test.go index da27d94e953..2669926387b 100644 --- a/pkg/workflow/dangerous_permissions_validation_test.go +++ b/pkg/workflow/dangerous_permissions_validation_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "strings" "testing" ) @@ -208,13 +209,7 @@ func TestFindWritePermissions(t *testing.T) { if tt.expectedScopes != nil { // Check that all expected scopes are present for _, expectedScope := range tt.expectedScopes { - found := false - for _, scope := range writePerms { - if scope == expectedScope { - found = true - break - } - } + found := slices.Contains(writePerms, expectedScope) if !found { t.Errorf("Expected to find scope %s in write permissions", expectedScope) } diff --git a/pkg/workflow/dependabot.go b/pkg/workflow/dependabot.go index d9e5545f86a..fa91d5bc965 100644 --- a/pkg/workflow/dependabot.go +++ b/pkg/workflow/dependabot.go @@ -487,8 +487,8 @@ func (c *Compiler) generateRequirementsTxt(path string, deps []PipDependency, fo } // Parse existing requirements - lines := strings.Split(string(existingData), "\n") - for _, line := range lines { + lines := strings.SplitSeq(string(existingData), "\n") + for line := range lines { line = strings.TrimSpace(line) if line == "" || strings.HasPrefix(line, "#") { continue @@ -629,9 +629,9 @@ func (c *Compiler) generateGoMod(path string, deps []GoDependency, forceOverwrit return fmt.Errorf("failed to read existing go.mod: %w", err) } - existingLines := strings.Split(string(existingData), "\n") + existingLines := strings.SplitSeq(string(existingData), "\n") // Keep module declaration and go version - for _, line := range existingLines { + for line := range existingLines { trimmed := strings.TrimSpace(line) if strings.HasPrefix(trimmed, "module ") || strings.HasPrefix(trimmed, "go ") { lines = append(lines, line) diff --git a/pkg/workflow/docker_api_proxy_test.go b/pkg/workflow/docker_api_proxy_test.go index 86761f608a6..3b3bf625f1c 100644 --- a/pkg/workflow/docker_api_proxy_test.go +++ b/pkg/workflow/docker_api_proxy_test.go @@ -1,6 +1,7 @@ package workflow import ( + "slices" "testing" "github.com/github/gh-aw/pkg/constants" @@ -46,13 +47,7 @@ func TestCollectDockerImages_APIProxyForEnginesWithLLMGateway(t *testing.T) { images := collectDockerImages(nil, workflowData, ActionModeRelease) apiProxyImage := constants.DefaultFirewallRegistry + "/api-proxy:" + awfImageTag - found := false - for _, img := range images { - if img == apiProxyImage { - found = true - break - } - } + found := slices.Contains(images, apiProxyImage) if found != tt.expectAPIProxy { if tt.expectAPIProxy { diff --git a/pkg/workflow/domains_protocol_test.go b/pkg/workflow/domains_protocol_test.go index 2697ade28e1..0d350720d87 100644 --- a/pkg/workflow/domains_protocol_test.go +++ b/pkg/workflow/domains_protocol_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "strings" "testing" ) @@ -78,13 +79,7 @@ func TestProtocolSpecificDomains(t *testing.T) { // Check that all expected domains are present for _, expected := range tt.expectedDomains { - found := false - for _, domain := range result { - if domain == expected { - found = true - break - } - } + found := slices.Contains(result, expected) if !found { t.Errorf("Expected domain %q not found in result: %v", expected, result) } diff --git a/pkg/workflow/domains_sort_test.go b/pkg/workflow/domains_sort_test.go index 07cac3fc2b1..66b366d6b09 100644 --- a/pkg/workflow/domains_sort_test.go +++ b/pkg/workflow/domains_sort_test.go @@ -16,7 +16,7 @@ func TestGetAllowedDomainsSorted(t *testing.T) { domains := GetAllowedDomains(permissions) // Verify that domains are sorted - for i := 0; i < len(domains)-1; i++ { + for i := range len(domains) - 1 { if domains[i] >= domains[i+1] { t.Errorf("Domains not sorted: %q >= %q at index %d", domains[i], domains[i+1], i) } @@ -30,7 +30,7 @@ func TestGetAllowedDomainsSorted(t *testing.T) { domains := GetAllowedDomains(permissions) // Verify that domains are sorted - for i := 0; i < len(domains)-1; i++ { + for i := range len(domains) - 1 { if domains[i] >= domains[i+1] { t.Errorf("Domains not sorted: %q >= %q at index %d", domains[i], domains[i+1], i) } @@ -44,7 +44,7 @@ func TestGetAllowedDomainsSorted(t *testing.T) { domains := GetAllowedDomains(permissions) // Verify that domains are sorted - for i := 0; i < len(domains)-1; i++ { + for i := range len(domains) - 1 { if domains[i] >= domains[i+1] { t.Errorf("Domains not sorted: %q >= %q at index %d", domains[i], domains[i+1], i) } @@ -196,7 +196,7 @@ func TestGetAllowedDomainsSortedAndUnique(t *testing.T) { domains := GetAllowedDomains(permissions) // Verify sorted - for i := 0; i < len(domains)-1; i++ { + for i := range len(domains) - 1 { if domains[i] >= domains[i+1] { t.Errorf("Domains not sorted: %q >= %q at index %d", domains[i], domains[i+1], i) } @@ -231,7 +231,7 @@ func TestGetAllowedDomainsSortedAndUnique(t *testing.T) { } // Verify sorted - for i := 0; i < len(domains)-1; i++ { + for i := range len(domains) - 1 { if domains[i] >= domains[i+1] { t.Errorf("Ecosystem domains not sorted: %q >= %q at index %d", domains[i], domains[i+1], i) } @@ -249,7 +249,7 @@ func TestGetCopilotAllowedDomainsSorted(t *testing.T) { // Split the CSV and verify sorted domains := strings.Split(domainsStr, ",") - for i := 0; i < len(domains)-1; i++ { + for i := range len(domains) - 1 { if domains[i] >= domains[i+1] { t.Errorf("Copilot domains not sorted: %q >= %q at index %d", domains[i], domains[i+1], i) } diff --git a/pkg/workflow/domains_test.go b/pkg/workflow/domains_test.go index dbfc6607798..c6d0f41a45d 100644 --- a/pkg/workflow/domains_test.go +++ b/pkg/workflow/domains_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "strings" "testing" ) @@ -129,7 +130,7 @@ func TestGetDomainEcosystem_Determinism(t *testing.T) { {"static.crates.io", "rust"}, } for _, c := range cases { - for i := 0; i < 20; i++ { + for i := range 20 { got := GetDomainEcosystem(c.domain) if got != c.expected { t.Errorf("call %d: GetDomainEcosystem(%q) = %q, want %q", i, c.domain, got, c.expected) @@ -583,13 +584,7 @@ func TestGetAllowedDomains_VariousCombinations(t *testing.T) { // Check that expected domains are present for _, expectedDomain := range tt.expectContains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' not found in result. Got: %v", expectedDomain, domains) } @@ -597,11 +592,8 @@ func TestGetAllowedDomains_VariousCombinations(t *testing.T) { // Check that unexpected domains are NOT present for _, unexpectedDomain := range tt.expectNotContains { - for _, domain := range domains { - if domain == unexpectedDomain { - t.Errorf("Domain '%s' should not be in result, but was found", unexpectedDomain) - break - } + if slices.Contains(domains, unexpectedDomain) { + t.Errorf("Domain '%s' should not be in result, but was found", unexpectedDomain) } } @@ -731,13 +723,7 @@ func TestNetworkPermissions_EdgeCases(t *testing.T) { } for _, expectedDomain := range tt.expectContains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' not found in result: %v", expectedDomain, domains) } @@ -872,13 +858,7 @@ func TestGetDomainsFromRuntimes(t *testing.T) { } for _, expected := range tt.expectContains { - found := false - for _, domain := range domains { - if domain == expected { - found = true - break - } - } + found := slices.Contains(domains, expected) if !found { t.Errorf("Expected domain '%s' not found in result: %v", expected, domains) } diff --git a/pkg/workflow/ecosystem_domains_test.go b/pkg/workflow/ecosystem_domains_test.go index 1a27faac801..f8f1577d8f0 100644 --- a/pkg/workflow/ecosystem_domains_test.go +++ b/pkg/workflow/ecosystem_domains_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "testing" ) @@ -23,13 +24,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in defaults, but it was not found", expectedDomain) } @@ -46,13 +41,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, excludedDomain := range excludedDomains { - found := false - for _, domain := range domains { - if domain == excludedDomain { - found = true - break - } - } + found := slices.Contains(domains, excludedDomain) if found { t.Errorf("Domain '%s' should NOT be included in defaults, but it was found", excludedDomain) } @@ -74,13 +63,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in containers ecosystem, but it was not found", expectedDomain) } @@ -102,13 +85,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in dotnet ecosystem, but it was not found", expectedDomain) } @@ -130,13 +107,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in python ecosystem, but it was not found", expectedDomain) } @@ -159,13 +130,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in go ecosystem, but it was not found", expectedDomain) } @@ -196,13 +161,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in java ecosystem, but it was not found", expectedDomain) } @@ -231,13 +190,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in node ecosystem, but it was not found", expectedDomain) } @@ -259,13 +212,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in python ecosystem, but it was not found", expectedDomain) } @@ -287,13 +234,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in github ecosystem, but it was not found", expectedDomain) } @@ -317,13 +258,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { allExpected = append(allExpected, expectedCustom...) for _, expectedDomain := range allExpected { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included in combined ecosystems, but it was not found", expectedDomain) } @@ -344,13 +279,7 @@ func TestEcosystemDomainExpansion(t *testing.T) { } for _, expectedDomain := range expectedDomains { - found := false - for _, domain := range domains { - if domain == expectedDomain { - found = true - break - } - } + found := slices.Contains(domains, expectedDomain) if !found { t.Errorf("Expected domain '%s' to be included as literal domain, but it was not found", expectedDomain) } diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index f7aac8a321e..e29cc4c20d1 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -218,8 +218,8 @@ func FormatStepWithCommandAndEnv(stepLines []string, command string, env map[str stepLines = append(stepLines, " run: |") // Split command into lines and indent them properly - commandLines := strings.Split(command, "\n") - for _, line := range commandLines { + commandLines := strings.SplitSeq(command, "\n") + for line := range commandLines { // Don't add indentation to empty lines if line == "" { stepLines = append(stepLines, "") diff --git a/pkg/workflow/expression_builder.go b/pkg/workflow/expression_builder.go index 1780a9791f8..b90870f3965 100644 --- a/pkg/workflow/expression_builder.go +++ b/pkg/workflow/expression_builder.go @@ -321,8 +321,8 @@ func RenderConditionAsIf(yaml *strings.Builder, condition ConditionNode, indent conditionStr := condition.Render() // Format the condition with proper indentation - lines := strings.Split(conditionStr, "\n") - for _, line := range lines { + lines := strings.SplitSeq(conditionStr, "\n") + for line := range lines { yaml.WriteString(indent + line + "\n") } } diff --git a/pkg/workflow/expression_parser_comprehensive_test.go b/pkg/workflow/expression_parser_comprehensive_test.go index 7743bba1cfc..ce817baf186 100644 --- a/pkg/workflow/expression_parser_comprehensive_test.go +++ b/pkg/workflow/expression_parser_comprehensive_test.go @@ -591,8 +591,7 @@ func BenchmarkParseExpression(b *testing.B) { "(github.event_name == 'issues' && github.event.action == 'opened') || (github.event_name == 'pull_request' && !github.event.pull_request.draft)", } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for i := 0; b.Loop(); i++ { expr := expressions[i%len(expressions)] _, err := ParseExpression(expr) if err != nil { @@ -613,8 +612,7 @@ This workflow uses several expressions: - Real-world example: ${{ github.actor && github.run_number }} ` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { err := validateExpressionSafety(content) if err != nil { b.Fatalf("Safety validation error: %v", err) diff --git a/pkg/workflow/expression_parser_fuzz_test.go b/pkg/workflow/expression_parser_fuzz_test.go index bf2873e8a0e..bab1fb62f69 100644 --- a/pkg/workflow/expression_parser_fuzz_test.go +++ b/pkg/workflow/expression_parser_fuzz_test.go @@ -83,12 +83,13 @@ func FuzzExpressionParser(f *testing.F) { // Very long expressions to test buffer handling f.Add("Very long valid: ${{ github.event.pull_request.head.repo.full_name }}") - longExpression := "Long expression: ${{ " - for i := 0; i < 100; i++ { - longExpression += "github.workflow && " + var longExpression strings.Builder + longExpression.WriteString("Long expression: ${{ ") + for range 100 { + longExpression.WriteString("github.workflow && ") } - longExpression += "github.repository }}" - f.Add(longExpression) + longExpression.WriteString("github.repository }}") + f.Add(longExpression.String()) // Expressions with excessive whitespace f.Add("Lots of spaces: ${{ github.workflow }}") diff --git a/pkg/workflow/expression_validation.go b/pkg/workflow/expression_validation.go index fe90f47f85f..9709d932458 100644 --- a/pkg/workflow/expression_validation.go +++ b/pkg/workflow/expression_validation.go @@ -48,6 +48,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "strings" "github.com/github/gh-aw/pkg/constants" @@ -257,11 +258,8 @@ func validateSingleExpression(expression string, opts ExpressionValidationOption // check if this expression matches env.* pattern allowed = true } else { - for _, allowedExpr := range constants.AllowedExpressions { - if expression == allowedExpr { - allowed = true - break - } + if slices.Contains(constants.AllowedExpressions, expression) { + allowed = true } } @@ -326,11 +324,8 @@ func validateSingleExpression(expression string, opts ExpressionValidationOption } else if opts.EnvRe.MatchString(property) { propertyAllowed = true } else { - for _, allowedExpr := range constants.AllowedExpressions { - if property == allowedExpr { - propertyAllowed = true - break - } + if slices.Contains(constants.AllowedExpressions, property) { + propertyAllowed = true } } @@ -356,12 +351,7 @@ func validateSingleExpression(expression string, opts ExpressionValidationOption // containsExpression checks if an expression is in the list func containsExpression(list *[]string, expr string) bool { - for _, item := range *list { - if item == expr { - return true - } - } - return false + return slices.Contains(*list, expr) } // ValidateExpressionSafetyPublic is a public wrapper for validateExpressionSafety diff --git a/pkg/workflow/expressions_benchmark_test.go b/pkg/workflow/expressions_benchmark_test.go index be991dcb71f..a1ec1d6582b 100644 --- a/pkg/workflow/expressions_benchmark_test.go +++ b/pkg/workflow/expressions_benchmark_test.go @@ -11,8 +11,7 @@ func BenchmarkValidateExpression(b *testing.B) { expression := "github.event.issue.number" unauthorizedExprs := []string{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = validateSingleExpression(expression, ExpressionValidationOptions{ NeedsStepsRe: needsStepsRegex, InputsRe: inputsRegex, @@ -29,8 +28,7 @@ func BenchmarkValidateExpression_Complex(b *testing.B) { expression := "github.event.pull_request.number == github.event.issue.number" unauthorizedExprs := []string{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = validateSingleExpression(expression, ExpressionValidationOptions{ NeedsStepsRe: needsStepsRegex, InputsRe: inputsRegex, @@ -47,8 +45,7 @@ func BenchmarkValidateExpression_NeedsOutputs(b *testing.B) { expression := "steps.sanitized.outputs.text" unauthorizedExprs := []string{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = validateSingleExpression(expression, ExpressionValidationOptions{ NeedsStepsRe: needsStepsRegex, InputsRe: inputsRegex, @@ -65,8 +62,7 @@ func BenchmarkValidateExpression_StepsOutputs(b *testing.B) { expression := "steps.my-step.outputs.result" unauthorizedExprs := []string{} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = validateSingleExpression(expression, ExpressionValidationOptions{ NeedsStepsRe: needsStepsRegex, InputsRe: inputsRegex, @@ -92,8 +88,7 @@ Repository: ${{ github.repository }} Run ID: ${{ github.run_id }} ` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = validateExpressionSafety(markdown) } } @@ -134,8 +129,7 @@ func BenchmarkValidateExpressionSafety_Complex(b *testing.B) { - Mode: ${{ env.DEPLOYMENT_MODE }} ` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = validateExpressionSafety(markdown) } } @@ -147,8 +141,7 @@ func BenchmarkValidateExpressionSafety_Minimal(b *testing.B) { Analyze issue #${{ github.event.issue.number }}. ` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = validateExpressionSafety(markdown) } } @@ -157,8 +150,7 @@ Analyze issue #${{ github.event.issue.number }}. func BenchmarkParseExpression_Simple(b *testing.B) { expression := "github.event.issue.number" - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = ParseExpression(expression) } } @@ -167,8 +159,7 @@ func BenchmarkParseExpression_Simple(b *testing.B) { func BenchmarkParseExpression_Comparison(b *testing.B) { expression := "github.event.issue.number == 123" - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = ParseExpression(expression) } } @@ -177,8 +168,7 @@ func BenchmarkParseExpression_Comparison(b *testing.B) { func BenchmarkParseExpression_Logical(b *testing.B) { expression := "github.event.issue.state == 'open' && github.event.issue.locked == false" - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = ParseExpression(expression) } } @@ -187,8 +177,7 @@ func BenchmarkParseExpression_Logical(b *testing.B) { func BenchmarkParseExpression_ComplexNested(b *testing.B) { expression := "(github.event.issue.state == 'open' || github.event.pull_request.state == 'open') && !cancelled()" - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = ParseExpression(expression) } } diff --git a/pkg/workflow/features.go b/pkg/workflow/features.go index 55e840cb661..d320442c24d 100644 --- a/pkg/workflow/features.go +++ b/pkg/workflow/features.go @@ -62,9 +62,9 @@ func isFeatureEnabled(flag constants.FeatureFlag, workflowData *WorkflowData) bo featuresLog.Printf("Checking GH_AW_FEATURES environment variable: %s", features) // Split by comma and check each feature - featureList := strings.Split(features, ",") + featureList := strings.SplitSeq(features, ",") - for _, feature := range featureList { + for feature := range featureList { if strings.ToLower(strings.TrimSpace(feature)) == flagLower { featuresLog.Printf("Feature found in GH_AW_FEATURES: %s=true", flagLower) return true diff --git a/pkg/workflow/fetch.go b/pkg/workflow/fetch.go index a7ed1a88962..8b82a6a55b4 100644 --- a/pkg/workflow/fetch.go +++ b/pkg/workflow/fetch.go @@ -1,6 +1,7 @@ package workflow import ( + "maps" "strings" "github.com/github/gh-aw/pkg/logger" @@ -27,9 +28,7 @@ func AddMCPFetchServerIfNeeded(tools map[string]any, engine CodingAgentEngine) ( // Create a copy of the tools map to avoid modifying the original updatedTools := make(map[string]any) - for key, value := range tools { - updatedTools[key] = value - } + maps.Copy(updatedTools, tools) // Remove the web-fetch tool since we'll replace it with an MCP server delete(updatedTools, "web-fetch") diff --git a/pkg/workflow/filters.go b/pkg/workflow/filters.go index 357fa9f05b4..2410cf29ada 100644 --- a/pkg/workflow/filters.go +++ b/pkg/workflow/filters.go @@ -1,6 +1,10 @@ package workflow -import "github.com/github/gh-aw/pkg/logger" +import ( + "slices" + + "github.com/github/gh-aw/pkg/logger" +) var filtersLog = logger.New("workflow:filters") @@ -165,11 +169,9 @@ func (c *Compiler) applyPullRequestForkFilter(data *WorkflowData, frontmatter ma } // If "*" wildcard is present, skip fork filtering (allow all forks) - for _, pattern := range allowedForks { - if pattern == "*" { - filtersLog.Print("Wildcard fork pattern detected, allowing all forks") - return // No fork filtering needed - } + if slices.Contains(allowedForks, "*") { + filtersLog.Print("Wildcard fork pattern detected, allowing all forks") + return // No fork filtering needed } // Build condition for allowed forks with glob support diff --git a/pkg/workflow/firewall.go b/pkg/workflow/firewall.go index 0420e79c5da..04ac608ff61 100644 --- a/pkg/workflow/firewall.go +++ b/pkg/workflow/firewall.go @@ -1,6 +1,7 @@ package workflow import ( + "slices" "strings" "github.com/github/gh-aw/pkg/constants" @@ -154,11 +155,9 @@ func enableFirewallByDefaultForEngine(engineID string, networkPermissions *Netwo // Check if allowed contains "*" (unrestricted network access) // If it does, do NOT enable the firewall by default - for _, domain := range networkPermissions.Allowed { - if domain == "*" { - firewallLog.Print("Wildcard '*' in allowed domains, skipping AWF auto-enablement") - return - } + if slices.Contains(networkPermissions.Allowed, "*") { + firewallLog.Print("Wildcard '*' in allowed domains, skipping AWF auto-enablement") + return } // Enable firewall by default for the engine (copilot, claude, codex) diff --git a/pkg/workflow/firewall_validation.go b/pkg/workflow/firewall_validation.go index 494fa2b1ab4..d41e78c8416 100644 --- a/pkg/workflow/firewall_validation.go +++ b/pkg/workflow/firewall_validation.go @@ -12,6 +12,7 @@ package workflow import ( "fmt" + "slices" "github.com/github/gh-aw/pkg/logger" ) @@ -51,11 +52,9 @@ func ValidateLogLevel(level string) error { } valid := []string{"debug", "info", "warn", "error"} - for _, v := range valid { - if level == v { - firewallValidationLog.Printf("Valid log-level: %s", level) - return nil - } + if slices.Contains(valid, level) { + firewallValidationLog.Printf("Valid log-level: %s", level) + return nil } firewallValidationLog.Printf("Invalid log-level: %s", level) return fmt.Errorf("invalid log-level '%s', must be one of: %v", level, valid) diff --git a/pkg/workflow/frontmatter_extraction_metadata.go b/pkg/workflow/frontmatter_extraction_metadata.go index b621c8c9d6e..ef1300b4236 100644 --- a/pkg/workflow/frontmatter_extraction_metadata.go +++ b/pkg/workflow/frontmatter_extraction_metadata.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "math" "strings" @@ -23,10 +24,8 @@ func (c *Compiler) extractFeatures(frontmatter map[string]any) map[string]any { // Features should be an object with any values (boolean or string) if featuresMap, ok := value.(map[string]any); ok { result := make(map[string]any) - for key, val := range featuresMap { - // Accept any value type (boolean, string, etc.) - result[key] = val - } + // Accept any value type (boolean, string, etc.) + maps.Copy(result, featuresMap) frontmatterMetadataLog.Printf("Extracted %d features", len(result)) return result } diff --git a/pkg/workflow/frontmatter_extraction_yaml.go b/pkg/workflow/frontmatter_extraction_yaml.go index 30a9ed69892..b66252641dc 100644 --- a/pkg/workflow/frontmatter_extraction_yaml.go +++ b/pkg/workflow/frontmatter_extraction_yaml.go @@ -47,16 +47,17 @@ func (c *Compiler) indentYAMLLines(yamlContent, indent string) string { } // First line doesn't get additional indentation - result := lines[0] + var result strings.Builder + result.WriteString(lines[0]) for i := 1; i < len(lines); i++ { if strings.TrimSpace(lines[i]) != "" { - result += "\n" + indent + lines[i] + result.WriteString("\n" + indent + lines[i]) } else { - result += "\n" + lines[i] + result.WriteString("\n" + lines[i]) } } - return result + return result.String() } // extractTopLevelYAMLSection extracts a top-level YAML section from frontmatter diff --git a/pkg/workflow/gemini_logs.go b/pkg/workflow/gemini_logs.go index 5a073fda53e..636b9e9d99b 100644 --- a/pkg/workflow/gemini_logs.go +++ b/pkg/workflow/gemini_logs.go @@ -11,8 +11,8 @@ var geminiLogsLog = logger.New("workflow:gemini_logs") // GeminiResponse represents the JSON structure returned by Gemini CLI type GeminiResponse struct { - Response string `json:"response"` - Stats map[string]interface{} `json:"stats"` + Response string `json:"response"` + Stats map[string]any `json:"stats"` } // ParseLogMetrics parses Gemini CLI log output and extracts metrics. @@ -31,8 +31,8 @@ func (e *GeminiEngine) ParseLogMetrics(logContent string, verbose bool) LogMetri toolCallCounts := make(map[string]int) // Try to parse the JSON response from Gemini - lines := strings.Split(logContent, "\n") - for _, line := range lines { + lines := strings.SplitSeq(logContent, "\n") + for line := range lines { line = strings.TrimSpace(line) if line == "" { continue @@ -51,9 +51,9 @@ func (e *GeminiEngine) ParseLogMetrics(logContent string, verbose bool) LogMetri // Extract token usage from stats if available if response.Stats != nil { - if models, ok := response.Stats["models"].(map[string]interface{}); ok { + if models, ok := response.Stats["models"].(map[string]any); ok { for _, modelStats := range models { - if stats, ok := modelStats.(map[string]interface{}); ok { + if stats, ok := modelStats.(map[string]any); ok { if inputTokens, ok := stats["input_tokens"].(float64); ok { metrics.TokenUsage += int(inputTokens) } @@ -65,7 +65,7 @@ func (e *GeminiEngine) ParseLogMetrics(logContent string, verbose bool) LogMetri } // Aggregate tool calls using a map to avoid duplicates - if tools, ok := response.Stats["tools"].(map[string]interface{}); ok { + if tools, ok := response.Stats["tools"].(map[string]any); ok { for toolName := range tools { toolCallCounts[toolName]++ } diff --git a/pkg/workflow/git_commands_test.go b/pkg/workflow/git_commands_test.go index 90838848136..c9871d9a6f5 100644 --- a/pkg/workflow/git_commands_test.go +++ b/pkg/workflow/git_commands_test.go @@ -3,6 +3,7 @@ package workflow import ( + "maps" "strings" "testing" ) @@ -83,9 +84,7 @@ func TestApplyDefaultGitCommandsForSafeOutputs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Create a copy of input tools to avoid modifying test data tools := make(map[string]any) - for k, v := range tt.tools { - tools[k] = v - } + maps.Copy(tools, tt.tools) // Apply both default tool functions in sequence tools = compiler.applyDefaultTools(tools, tt.safeOutputs, nil, nil) @@ -191,9 +190,7 @@ func TestAdditionalClaudeToolsForSafeOutputs(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Create a copy of input tools to avoid modifying test data tools := make(map[string]any) - for k, v := range tt.tools { - tools[k] = v - } + maps.Copy(tools, tt.tools) // Apply both default tool functions in sequence tools = compiler.applyDefaultTools(tools, tt.safeOutputs, nil, nil) diff --git a/pkg/workflow/git_patch_test.go b/pkg/workflow/git_patch_test.go index 7e46460f06c..c7e0cc4191f 100644 --- a/pkg/workflow/git_patch_test.go +++ b/pkg/workflow/git_patch_test.go @@ -11,20 +11,6 @@ import ( "github.com/github/gh-aw/pkg/testutil" ) -func min(a, b int) int { - if a < b { - return a - } - return b -} - -func max(a, b int) int { - if a > b { - return a - } - return b -} - func TestGitPatchGeneration(t *testing.T) { // Create a temporary directory for the test tmpDir := testutil.TempDir(t, "test-*") diff --git a/pkg/workflow/github_cli_test.go b/pkg/workflow/github_cli_test.go index 904d93152d9..5d13e632055 100644 --- a/pkg/workflow/github_cli_test.go +++ b/pkg/workflow/github_cli_test.go @@ -6,6 +6,7 @@ import ( "context" "os" "os/exec" + "slices" "strings" "testing" @@ -97,11 +98,8 @@ func TestExecGH(t *testing.T) { if tt.expectGHToken { found := false expectedEnv := "GH_TOKEN=" + tt.expectValue - for _, env := range cmd.Env { - if env == expectedEnv { - found = true - break - } + if slices.Contains(cmd.Env, expectedEnv) { + found = true } assert.True(t, found, "Expected environment to contain %s, but it wasn't found", expectedEnv) } else { @@ -141,13 +139,7 @@ func TestExecGHWithMultipleArgs(t *testing.T) { } // Verify environment contains GH_TOKEN - found := false - for _, env := range cmd.Env { - if env == "GH_TOKEN=test-token" { - found = true - break - } - } + found := slices.Contains(cmd.Env, "GH_TOKEN=test-token") assert.True(t, found, "Expected environment to contain GH_TOKEN=test-token") } @@ -229,11 +221,8 @@ func TestExecGHContext(t *testing.T) { if tt.expectGHToken { found := false expectedEnv := "GH_TOKEN=" + tt.expectValue - for _, env := range cmd.Env { - if env == expectedEnv { - found = true - break - } + if slices.Contains(cmd.Env, expectedEnv) { + found = true } assert.True(t, found, "Expected environment to contain %s, but it wasn't found", expectedEnv) } else { @@ -340,11 +329,8 @@ func TestSetupGHCommand(t *testing.T) { if tt.expectGHToken { found := false expectedEnv := "GH_TOKEN=" + tt.expectValue - for _, env := range cmd.Env { - if env == expectedEnv { - found = true - break - } + if slices.Contains(cmd.Env, expectedEnv) { + found = true } assert.True(t, found, "Expected environment to contain %s", expectedEnv) } else { diff --git a/pkg/workflow/github_folder_checkout_optimization_test.go b/pkg/workflow/github_folder_checkout_optimization_test.go index b128720ecd8..ca61146bf9d 100644 --- a/pkg/workflow/github_folder_checkout_optimization_test.go +++ b/pkg/workflow/github_folder_checkout_optimization_test.go @@ -133,7 +133,7 @@ This workflow uses runtime imports: {{runtime-import:shared/example.md}} for i, line := range lines { if len(line) > 2 && line[0] == ' ' && line[1] == ' ' && line[2] != ' ' && line[2] != '\t' { nextJobStart = 0 - for j := 0; j < i; j++ { + for j := range i { nextJobStart += len(lines[j]) + 1 // +1 for newline } break diff --git a/pkg/workflow/github_tool_to_toolset.go b/pkg/workflow/github_tool_to_toolset.go index 2fae897bf66..57c78bd8c2e 100644 --- a/pkg/workflow/github_tool_to_toolset.go +++ b/pkg/workflow/github_tool_to_toolset.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "sort" + "strings" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" @@ -93,14 +94,15 @@ func ValidateGitHubToolsAgainstToolsets(allowedTools []string, enabledToolsets [ // Report unknown tools with suggestions if any were found if len(unknownTools) > 0 { githubToolToToolsetLog.Printf("Found %d unknown tools", len(unknownTools)) - errMsg := fmt.Sprintf("Unknown GitHub tool(s): %s\n\n", formatList(unknownTools)) + var errMsg strings.Builder + errMsg.WriteString(fmt.Sprintf("Unknown GitHub tool(s): %s\n\n", formatList(unknownTools))) if len(suggestions) > 0 { - errMsg += "Did you mean:\n" + errMsg.WriteString("Did you mean:\n") for _, s := range suggestions { - errMsg += fmt.Sprintf(" %s\n", s) + errMsg.WriteString(fmt.Sprintf(" %s\n", s)) } - errMsg += "\n" + errMsg.WriteString("\n") } // Show a few examples of valid tools @@ -110,14 +112,11 @@ func ValidateGitHubToolsAgainstToolsets(allowedTools []string, enabledToolsets [ } sort.Strings(validTools) - exampleCount := 10 - if len(validTools) < exampleCount { - exampleCount = len(validTools) - } - errMsg += fmt.Sprintf("Valid GitHub tools include: %s\n\n", formatList(validTools[:exampleCount])) - errMsg += "See all tools: https://github.com/github/gh-aw/blob/main/pkg/workflow/data/github_tool_to_toolset.json" + exampleCount := min(10, len(validTools)) + errMsg.WriteString(fmt.Sprintf("Valid GitHub tools include: %s\n\n", 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 fmt.Errorf("%s", errMsg) + return fmt.Errorf("%s", errMsg.String()) } if len(missingToolsets) > 0 { diff --git a/pkg/workflow/imports.go b/pkg/workflow/imports.go index 4760219d44a..7693a2c0b01 100644 --- a/pkg/workflow/imports.go +++ b/pkg/workflow/imports.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "maps" "sort" "strings" "sync" @@ -68,9 +69,7 @@ func (c *Compiler) MergeMCPServers(topMCPServers map[string]any, importedMCPServ // Initialize result with top-level MCP servers result := make(map[string]any) - for k, v := range topMCPServers { - result[k] = v - } + maps.Copy(result, topMCPServers) // Split by newlines to handle multiple JSON objects from different imports lines := strings.Split(importedMCPServersJSON, "\n") @@ -278,9 +277,7 @@ func (c *Compiler) ValidateIncludedPermissions(topPermissionsYAML string, import // Combine all required permissions for the suggestion allRequired := make(map[PermissionScope]PermissionLevel) - for scope, level := range missingPermissions { - allRequired[scope] = level - } + maps.Copy(allRequired, missingPermissions) for scope, info := range insufficientPermissions { allRequired[scope] = info.required } @@ -713,9 +710,7 @@ func (c *Compiler) MergeFeatures(topFeatures map[string]any, importedFeatures [] // Start with top-level features or create a new map result := make(map[string]any) if topFeatures != nil { - for k, v := range topFeatures { - result[k] = v - } + maps.Copy(result, topFeatures) importsLog.Printf("Starting with %d top-level features", len(topFeatures)) } diff --git a/pkg/workflow/imports_recursive_test.go b/pkg/workflow/imports_recursive_test.go index e56a593e686..3efc5c67ab5 100644 --- a/pkg/workflow/imports_recursive_test.go +++ b/pkg/workflow/imports_recursive_test.go @@ -461,7 +461,7 @@ imports: // Extract import lines var importLines []string - for _, line := range strings.Split(manifest, "\n") { + for line := range strings.SplitSeq(manifest, "\n") { if strings.Contains(line, "# - ") { importName := strings.TrimSpace(strings.TrimPrefix(line, "# - ")) importLines = append(importLines, importName) diff --git a/pkg/workflow/jobs.go b/pkg/workflow/jobs.go index 468b5225488..6f797198c31 100644 --- a/pkg/workflow/jobs.go +++ b/pkg/workflow/jobs.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "sort" "strings" @@ -76,9 +77,7 @@ func (jm *JobManager) GetJob(name string) (*Job, bool) { func (jm *JobManager) GetAllJobs() map[string]*Job { // Return a copy to prevent external modification result := make(map[string]*Job) - for name, job := range jm.jobs { - result[name] = job - } + maps.Copy(result, jm.jobs) return result } @@ -138,16 +137,16 @@ func (jm *JobManager) ValidateDuplicateSteps() error { func extractStepName(stepYAML string) string { // Look for "name: " in the step YAML // Format is typically " - name: Step Name" with various indentation - lines := strings.Split(stepYAML, "\n") - for _, line := range lines { + lines := strings.SplitSeq(stepYAML, "\n") + for line := range lines { trimmed := strings.TrimSpace(line) // Remove leading dash if present trimmed = strings.TrimPrefix(trimmed, "-") trimmed = strings.TrimSpace(trimmed) - if strings.HasPrefix(trimmed, "name:") { + if after, ok := strings.CutPrefix(trimmed, "name:"); ok { // Extract the name value after "name:" - name := strings.TrimSpace(strings.TrimPrefix(trimmed, "name:")) + name := strings.TrimSpace(after) // Remove quotes if present name = strings.Trim(name, "\"'") return name @@ -262,8 +261,8 @@ func (jm *JobManager) renderJob(job *Job) string { if strings.Contains(job.If, "\n") { // Already has newlines, use existing logic - lines := strings.Split(job.If, "\n") - for _, line := range lines { + lines := strings.SplitSeq(job.If, "\n") + for line := range lines { if strings.TrimSpace(line) != "" { fmt.Fprintf(&yaml, " %s\n", strings.TrimSpace(line)) } diff --git a/pkg/workflow/js.go b/pkg/workflow/js.go index ce5a6dcf8e2..5d14936e914 100644 --- a/pkg/workflow/js.go +++ b/pkg/workflow/js.go @@ -320,7 +320,7 @@ func isInsideStringLiteral(text string) bool { inDoubleQuote := false inBacktick := false - for i := 0; i < len(runes); i++ { + for i := range runes { switch runes[i] { case '\'': if !inDoubleQuote && !inBacktick { @@ -367,7 +367,7 @@ func isInsideRegexLiteral(text string) bool { inBacktick := false inRegex := false - for i := 0; i < len(runes); i++ { + for i := range runes { switch runes[i] { case '\'': if !inDoubleQuote && !inBacktick && !inRegex { @@ -509,8 +509,8 @@ func FormatJavaScriptForYAML(script string) []string { // Remove JavaScript comments first cleanScript := removeJavaScriptComments(script) - scriptLines := strings.Split(cleanScript, "\n") - for _, line := range scriptLines { + scriptLines := strings.SplitSeq(cleanScript, "\n") + for line := range scriptLines { // Skip empty lines when inlining to YAML if strings.TrimSpace(line) != "" { formattedLines = append(formattedLines, fmt.Sprintf(" %s\n", line)) @@ -534,8 +534,8 @@ func WriteJavaScriptToYAML(yaml *strings.Builder, script string) { // Remove JavaScript comments first cleanScript := removeJavaScriptComments(script) - scriptLines := strings.Split(cleanScript, "\n") - for _, line := range scriptLines { + scriptLines := strings.SplitSeq(cleanScript, "\n") + for line := range scriptLines { // Skip empty lines when inlining to YAML if strings.TrimSpace(line) != "" { fmt.Fprintf(yaml, " %s\n", line) diff --git a/pkg/workflow/js_comments_test.go b/pkg/workflow/js_comments_test.go index 44888db8938..50bf5fc5f75 100644 --- a/pkg/workflow/js_comments_test.go +++ b/pkg/workflow/js_comments_test.go @@ -710,7 +710,7 @@ async function main() { for _, tc := range testCases { b.Run(tc.name, func(b *testing.B) { - for i := 0; i < b.N; i++ { + for range b.N { removeJavaScriptComments(tc.input) } }) diff --git a/pkg/workflow/js_test.go b/pkg/workflow/js_test.go index 06c59a4fe90..0ec4162c096 100644 --- a/pkg/workflow/js_test.go +++ b/pkg/workflow/js_test.go @@ -315,8 +315,7 @@ const result = await octokit.rest.pulls.create({ console.log('PR created:', result.data.html_url);` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { FormatJavaScriptForYAML(script) } } @@ -347,8 +346,7 @@ const result = await octokit.rest.pulls.create({ console.log('PR created:', result.data.html_url);` - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { var yaml strings.Builder WriteJavaScriptToYAML(&yaml, script) } diff --git a/pkg/workflow/known_needs_expressions.go b/pkg/workflow/known_needs_expressions.go index 63ce709ebd6..f3c3d92387a 100644 --- a/pkg/workflow/known_needs_expressions.go +++ b/pkg/workflow/known_needs_expressions.go @@ -2,7 +2,9 @@ package workflow import ( "fmt" + "slices" "sort" + "strings" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" @@ -100,21 +102,21 @@ func generateKnownNeedsExpressions(data *WorkflowData, preActivationJobCreated b func normalizeJobNameForEnvVar(jobName string) string { // Already in the correct format for most job names // Just uppercase and replace hyphens with underscores - result := "" + var result strings.Builder for _, char := range jobName { if char == '-' { - result += "_" + result.WriteString("_") } else if (char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') || (char >= '0' && char <= '9') || char == '_' { if char >= 'a' && char <= 'z' { - result += string(char - 32) // Convert to uppercase + result.WriteString(string(char - 32)) // Convert to uppercase } else if char >= 'A' && char <= 'Z' { - result += string(char) + result.WriteString(string(char)) } else { - result += string(char) + result.WriteString(string(char)) } } } - return result + return result.String() } // normalizeOutputNameForEnvVar converts an output name to a valid environment variable segment @@ -213,13 +215,7 @@ func getCustomJobsBeforeActivation(data *WorkflowData) []string { needsList := parseNeedsField(needsField) // Check if it depends on pre_activation - dependsOnPreActivation := false - for _, dep := range needsList { - if dep == string(constants.PreActivationJobName) { - dependsOnPreActivation = true - break - } - } + dependsOnPreActivation := slices.Contains(needsList, string(constants.PreActivationJobName)) // Only include if it depends on pre_activation (and not on activation/agent/detection) if dependsOnPreActivation { diff --git a/pkg/workflow/label_trigger_parser.go b/pkg/workflow/label_trigger_parser.go index a9dd97ccd19..fa81920051f 100644 --- a/pkg/workflow/label_trigger_parser.go +++ b/pkg/workflow/label_trigger_parser.go @@ -58,8 +58,8 @@ func parseLabelTriggerShorthand(input string) (entityType string, labelNames []s rawLabels := tokens[startIdx:] for _, token := range rawLabels { // Split on commas to handle "label1,label2,label3" format - parts := strings.Split(token, ",") - for _, part := range parts { + parts := strings.SplitSeq(token, ",") + for part := range parts { cleanLabel := strings.TrimSpace(part) if cleanLabel != "" { labelNames = append(labelNames, cleanLabel) diff --git a/pkg/workflow/lock_file_bundling_test.go b/pkg/workflow/lock_file_bundling_test.go index c656159627f..0ea1d13ecbe 100644 --- a/pkg/workflow/lock_file_bundling_test.go +++ b/pkg/workflow/lock_file_bundling_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "strings" "testing" @@ -121,13 +122,7 @@ func TestLockFilesHaveNoBundledRequires(t *testing.T) { } // Add to failed files if not already present - alreadyFailed := false - for _, f := range failedFiles { - if f == relPath { - alreadyFailed = true - break - } - } + alreadyFailed := slices.Contains(failedFiles, relPath) if !alreadyFailed { failedFiles = append(failedFiles, relPath) } diff --git a/pkg/workflow/lock_schema.go b/pkg/workflow/lock_schema.go index d21a03aeb2e..755857197ea 100644 --- a/pkg/workflow/lock_schema.go +++ b/pkg/workflow/lock_schema.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "regexp" + "slices" "strings" "github.com/github/gh-aw/pkg/logger" @@ -34,12 +35,7 @@ var SupportedSchemaVersions = []LockSchemaVersion{ // IsSchemaVersionSupported checks if a schema version is supported func IsSchemaVersionSupported(version LockSchemaVersion) bool { - for _, v := range SupportedSchemaVersions { - if v == version { - return true - } - } - return false + return slices.Contains(SupportedSchemaVersions, version) } // ExtractMetadataFromLockFile extracts structured metadata from a lock file's comment header diff --git a/pkg/workflow/mcp_benchmark_test.go b/pkg/workflow/mcp_benchmark_test.go index 29989435f6b..ad398b3451a 100644 --- a/pkg/workflow/mcp_benchmark_test.go +++ b/pkg/workflow/mcp_benchmark_test.go @@ -15,8 +15,7 @@ func BenchmarkRenderPlaywrightMCPConfig(b *testing.B) { } playwrightConfig := parsePlaywrightTool(playwrightTool) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { var yaml strings.Builder renderPlaywrightMCPConfig(&yaml, playwrightConfig, true) } @@ -29,8 +28,7 @@ func BenchmarkGeneratePlaywrightDockerArgs(b *testing.B) { } playwrightConfig := parsePlaywrightTool(playwrightTool) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = generatePlaywrightDockerArgs(playwrightConfig) } } @@ -43,8 +41,7 @@ func BenchmarkRenderPlaywrightMCPConfig_Complex(b *testing.B) { } playwrightConfig := parsePlaywrightTool(playwrightTool) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { var yaml strings.Builder renderPlaywrightMCPConfig(&yaml, playwrightConfig, true) } @@ -54,8 +51,7 @@ func BenchmarkRenderPlaywrightMCPConfig_Complex(b *testing.B) { func BenchmarkExtractExpressionsFromPlaywrightArgs(b *testing.B) { customArgs := []string{"--debug", "--timeout", "${{ github.event.inputs.timeout }}"} - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = extractExpressionsFromPlaywrightArgs(customArgs) } } diff --git a/pkg/workflow/mcp_config_custom.go b/pkg/workflow/mcp_config_custom.go index 8f17eaf2c67..cef17177522 100644 --- a/pkg/workflow/mcp_config_custom.go +++ b/pkg/workflow/mcp_config_custom.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "os" "sort" "strings" @@ -498,9 +499,7 @@ func collectHTTPMCPHeaderSecrets(tools map[string]any) map[string]string { // Extract MCP config to get headers if mcpConfig, err := getMCPConfig(toolConfig, toolName); err == nil { secrets := ExtractSecretsFromMap(mcpConfig.Headers) - for varName, expr := range secrets { - allSecrets[varName] = expr - } + maps.Copy(allSecrets, secrets) } } } diff --git a/pkg/workflow/mcp_config_serena_renderer.go b/pkg/workflow/mcp_config_serena_renderer.go index bfa8521a1d4..c0515310e44 100644 --- a/pkg/workflow/mcp_config_serena_renderer.go +++ b/pkg/workflow/mcp_config_serena_renderer.go @@ -1,6 +1,7 @@ package workflow import ( + "slices" "strings" "github.com/github/gh-aw/pkg/constants" @@ -51,13 +52,7 @@ func selectSerenaContainer(serenaTool any) string { // Check if all requested languages are supported by the default container defaultSupported := true for _, lang := range requestedLanguages { - supported := false - for _, supportedLang := range constants.SerenaLanguageSupport[constants.DefaultSerenaMCPServerContainer] { - if lang == supportedLang { - supported = true - break - } - } + supported := slices.Contains(constants.SerenaLanguageSupport[constants.DefaultSerenaMCPServerContainer], lang) if !supported { defaultSupported = false mcpSerenaLog.Printf("Language '%s' not found in default container support list", lang) @@ -72,13 +67,7 @@ func selectSerenaContainer(serenaTool any) string { // Check if Oraios container supports the languages oraiosSupported := true for _, lang := range requestedLanguages { - supported := false - for _, supportedLang := range constants.SerenaLanguageSupport[constants.OraiosSerenaContainer] { - if lang == supportedLang { - supported = true - break - } - } + supported := slices.Contains(constants.SerenaLanguageSupport[constants.OraiosSerenaContainer], lang) if !supported { oraiosSupported = false break diff --git a/pkg/workflow/mcp_config_validation.go b/pkg/workflow/mcp_config_validation.go index ebc12566a39..9d97a9021ce 100644 --- a/pkg/workflow/mcp_config_validation.go +++ b/pkg/workflow/mcp_config_validation.go @@ -164,10 +164,7 @@ func getRawMCPConfig(toolConfig map[string]any) (map[string]any, error) { validFields = append(validFields, k) } sort.Strings(validFields) - maxFields := 10 - if len(validFields) < maxFields { - maxFields = len(validFields) - } + maxFields := min(10, len(validFields)) return nil, fmt.Errorf("unknown property '%s' in tool configuration. Valid properties include: %s.\n\nExample:\ntools:\n my-tool:\n command: \"node server.js\"\n args: [\"--verbose\"]\n\nSee: %s", field, strings.Join(validFields[:maxFields], ", "), constants.DocsToolsURL) } } diff --git a/pkg/workflow/mcp_environment.go b/pkg/workflow/mcp_environment.go index 7e91c734ffa..6244e2e4203 100644 --- a/pkg/workflow/mcp_environment.go +++ b/pkg/workflow/mcp_environment.go @@ -46,6 +46,10 @@ package workflow import ( + "maps" + + "slices" + "github.com/github/gh-aw/pkg/logger" ) @@ -57,13 +61,7 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor envVars := make(map[string]string) // Check for GitHub MCP server token - hasGitHub := false - for _, toolName := range mcpTools { - if toolName == "github" { - hasGitHub = true - break - } - } + hasGitHub := slices.Contains(mcpTools, "github") if hasGitHub { githubTool := tools["github"] @@ -93,13 +91,7 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor } // Check for safe-outputs env vars - hasSafeOutputs := false - for _, toolName := range mcpTools { - if toolName == "safe-outputs" { - hasSafeOutputs = true - break - } - } + hasSafeOutputs := slices.Contains(mcpTools, "safe-outputs") if hasSafeOutputs { envVars["GH_AW_SAFE_OUTPUTS"] = "${{ env.GH_AW_SAFE_OUTPUTS }}" // Only add upload-assets env vars if upload-assets is configured @@ -120,9 +112,7 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor // Add tool-specific env vars (secrets passthrough) safeInputsSecrets := collectSafeInputsSecrets(workflowData.SafeInputs) - for envVarName, secretExpr := range safeInputsSecrets { - envVars[envVarName] = secretExpr - } + maps.Copy(envVars, safeInputsSecrets) } // Check for safe-outputs env vars @@ -145,22 +135,14 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor } // Check for Playwright domain secrets - hasPlaywright := false - for _, toolName := range mcpTools { - if toolName == "playwright" { - hasPlaywright = true - break - } - } + hasPlaywright := slices.Contains(mcpTools, "playwright") if hasPlaywright { // Extract all expressions from playwright custom args using ExpressionExtractor if playwrightTool, ok := tools["playwright"]; ok { playwrightConfig := parsePlaywrightTool(playwrightTool) customArgs := getPlaywrightCustomArgs(playwrightConfig) playwrightArgSecrets := extractExpressionsFromPlaywrightArgs(customArgs) - for envVarName, originalExpr := range playwrightArgSecrets { - envVars[envVarName] = originalExpr - } + maps.Copy(envVars, playwrightArgSecrets) } } @@ -191,18 +173,14 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor if mcpConfig.Type == "http" && len(mcpConfig.Headers) > 0 { headerSecrets := ExtractSecretsFromMap(mcpConfig.Headers) mcpEnvironmentLog.Printf("Extracted %d secrets from HTTP MCP server '%s'", len(headerSecrets), toolName) - for envVarName, secretExpr := range headerSecrets { - envVars[envVarName] = secretExpr - } + maps.Copy(envVars, headerSecrets) } // Also extract secrets from env section if present if len(mcpConfig.Env) > 0 { envSecrets := ExtractSecretsFromMap(mcpConfig.Env) mcpEnvironmentLog.Printf("Extracted %d secrets from env section of MCP server '%s'", len(envSecrets), toolName) - for envVarName, secretExpr := range envSecrets { - envVars[envVarName] = secretExpr - } + maps.Copy(envVars, envSecrets) } } } @@ -216,9 +194,7 @@ func collectMCPEnvironmentVariables(tools map[string]any, mcpTools []string, wor if mcpConfig != nil && len(mcpConfig.Env) > 0 { mcpEnvironmentLog.Printf("Adding %d environment variables from plugin '%s' MCP configuration", len(mcpConfig.Env), pluginID) // Add ALL environment variables from plugin MCP config (not just secrets) - for envVarName, envVarValue := range mcpConfig.Env { - envVars[envVarName] = envVarValue - } + maps.Copy(envVars, mcpConfig.Env) } } } diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index c01bf4e43e5..16561579d7e 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.go @@ -62,6 +62,7 @@ package workflow import ( "encoding/json" "fmt" + "slices" "sort" "strings" @@ -142,13 +143,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, } // Install gh-aw extension if agentic-workflows tool is enabled - hasAgenticWorkflows := false - for _, toolName := range mcpTools { - if toolName == "agentic-workflows" { - hasAgenticWorkflows = true - break - } - } + hasAgenticWorkflows := slices.Contains(mcpTools, "agentic-workflows") // Check if shared/mcp/gh-aw.md is imported (which already installs gh-aw) hasGhAwImport := false @@ -221,7 +216,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, toolsDelimiter := GenerateHeredocDelimiter("SAFE_OUTPUTS_TOOLS") yaml.WriteString(" cat > /opt/gh-aw/safeoutputs/tools.json << '" + toolsDelimiter + "'\n") // Write each line of the indented JSON with proper YAML indentation - for _, line := range strings.Split(filteredToolsJSON, "\n") { + for line := range strings.SplitSeq(filteredToolsJSON, "\n") { yaml.WriteString(" " + line + "\n") } yaml.WriteString(" " + toolsDelimiter + "\n") @@ -247,7 +242,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, validationDelimiter := GenerateHeredocDelimiter("SAFE_OUTPUTS_VALIDATION") yaml.WriteString(" cat > /opt/gh-aw/safeoutputs/validation.json << '" + validationDelimiter + "'\n") // Write each line of the indented JSON with proper YAML indentation - for _, line := range strings.Split(validationConfigJSON, "\n") { + for line := range strings.SplitSeq(validationConfigJSON, "\n") { yaml.WriteString(" " + line + "\n") } yaml.WriteString(" " + validationDelimiter + "\n") @@ -315,7 +310,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, toolsJSON := generateSafeInputsToolsConfig(workflowData.SafeInputs) toolsDelimiter := GenerateHeredocDelimiter("SAFE_INPUTS_TOOLS") yaml.WriteString(" cat > /opt/gh-aw/safe-inputs/tools.json << '" + toolsDelimiter + "'\n") - for _, line := range strings.Split(toolsJSON, "\n") { + for line := range strings.SplitSeq(toolsJSON, "\n") { yaml.WriteString(" " + line + "\n") } yaml.WriteString(" " + toolsDelimiter + "\n") @@ -355,7 +350,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, toolScript := generateSafeInputShellToolScript(toolConfig) shDelimiter := GenerateHeredocDelimiter(fmt.Sprintf("SAFE_INPUTS_SH_%s", strings.ToUpper(toolName))) fmt.Fprintf(yaml, " cat > /opt/gh-aw/safe-inputs/%s.sh << '%s'\n", toolName, shDelimiter) - for _, line := range strings.Split(toolScript, "\n") { + for line := range strings.SplitSeq(toolScript, "\n") { yaml.WriteString(" " + line + "\n") } fmt.Fprintf(yaml, " %s\n", shDelimiter) @@ -365,7 +360,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, toolScript := generateSafeInputPythonToolScript(toolConfig) pyDelimiter := GenerateHeredocDelimiter(fmt.Sprintf("SAFE_INPUTS_PY_%s", strings.ToUpper(toolName))) fmt.Fprintf(yaml, " cat > /opt/gh-aw/safe-inputs/%s.py << '%s'\n", toolName, pyDelimiter) - for _, line := range strings.Split(toolScript, "\n") { + for line := range strings.SplitSeq(toolScript, "\n") { yaml.WriteString(" " + line + "\n") } fmt.Fprintf(yaml, " %s\n", pyDelimiter) @@ -375,7 +370,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, toolScript := generateSafeInputGoToolScript(toolConfig) goDelimiter := GenerateHeredocDelimiter(fmt.Sprintf("SAFE_INPUTS_GO_%s", strings.ToUpper(toolName))) fmt.Fprintf(yaml, " cat > /opt/gh-aw/safe-inputs/%s.go << '%s'\n", toolName, goDelimiter) - for _, line := range strings.Split(toolScript, "\n") { + for line := range strings.SplitSeq(toolScript, "\n") { yaml.WriteString(" " + line + "\n") } fmt.Fprintf(yaml, " %s\n", goDelimiter) @@ -551,82 +546,83 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, containerImage += ":" + string(constants.DefaultMCPGatewayVersion) } - containerCmd := "docker run -i --rm --network host" - containerCmd += " -v /var/run/docker.sock:/var/run/docker.sock" // Enable docker-in-docker for MCP gateway + var containerCmd strings.Builder + containerCmd.WriteString("docker run -i --rm --network host") + containerCmd.WriteString(" -v /var/run/docker.sock:/var/run/docker.sock") // Enable docker-in-docker for MCP gateway // Pass required gateway environment variables - containerCmd += " -e MCP_GATEWAY_PORT" - containerCmd += " -e MCP_GATEWAY_DOMAIN" - containerCmd += " -e MCP_GATEWAY_API_KEY" - containerCmd += " -e MCP_GATEWAY_PAYLOAD_DIR" - containerCmd += " -e DEBUG" + containerCmd.WriteString(" -e MCP_GATEWAY_PORT") + containerCmd.WriteString(" -e MCP_GATEWAY_DOMAIN") + containerCmd.WriteString(" -e MCP_GATEWAY_API_KEY") + containerCmd.WriteString(" -e MCP_GATEWAY_PAYLOAD_DIR") + containerCmd.WriteString(" -e DEBUG") // Pass environment variables that MCP servers reference in their config // These are needed because awmg v0.0.12+ validates and resolves ${VAR} patterns at config load time // Environment variables used by MCP gateway - containerCmd += " -e MCP_GATEWAY_LOG_DIR" + containerCmd.WriteString(" -e MCP_GATEWAY_LOG_DIR") // Environment variables used by safeoutputs MCP server - containerCmd += " -e GH_AW_MCP_LOG_DIR" - containerCmd += " -e GH_AW_SAFE_OUTPUTS" - containerCmd += " -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH" - containerCmd += " -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH" - containerCmd += " -e GH_AW_ASSETS_BRANCH" - containerCmd += " -e GH_AW_ASSETS_MAX_SIZE_KB" - containerCmd += " -e GH_AW_ASSETS_ALLOWED_EXTS" - containerCmd += " -e DEFAULT_BRANCH" + containerCmd.WriteString(" -e GH_AW_MCP_LOG_DIR") + containerCmd.WriteString(" -e GH_AW_SAFE_OUTPUTS") + containerCmd.WriteString(" -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH") + containerCmd.WriteString(" -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH") + containerCmd.WriteString(" -e GH_AW_ASSETS_BRANCH") + containerCmd.WriteString(" -e GH_AW_ASSETS_MAX_SIZE_KB") + containerCmd.WriteString(" -e GH_AW_ASSETS_ALLOWED_EXTS") + containerCmd.WriteString(" -e DEFAULT_BRANCH") // Environment variables used by GitHub MCP server - containerCmd += " -e GITHUB_MCP_SERVER_TOKEN" + containerCmd.WriteString(" -e GITHUB_MCP_SERVER_TOKEN") // For Copilot engine with GitHub remote MCP, also pass GITHUB_PERSONAL_ACCESS_TOKEN // This allows the gateway to expand ${GITHUB_PERSONAL_ACCESS_TOKEN} references in headers if hasGitHub && getGitHubType(githubTool) == "remote" && engine.GetID() == "copilot" { - containerCmd += " -e GITHUB_PERSONAL_ACCESS_TOKEN" + containerCmd.WriteString(" -e GITHUB_PERSONAL_ACCESS_TOKEN") } - containerCmd += " -e GITHUB_MCP_LOCKDOWN" + containerCmd.WriteString(" -e GITHUB_MCP_LOCKDOWN") // Standard GitHub Actions environment variables (repository context) - containerCmd += " -e GITHUB_REPOSITORY" - containerCmd += " -e GITHUB_SERVER_URL" - containerCmd += " -e GITHUB_SHA" - containerCmd += " -e GITHUB_WORKSPACE" - containerCmd += " -e GITHUB_TOKEN" + containerCmd.WriteString(" -e GITHUB_REPOSITORY") + containerCmd.WriteString(" -e GITHUB_SERVER_URL") + containerCmd.WriteString(" -e GITHUB_SHA") + containerCmd.WriteString(" -e GITHUB_WORKSPACE") + containerCmd.WriteString(" -e GITHUB_TOKEN") // GitHub Actions run context - containerCmd += " -e GITHUB_RUN_ID" - containerCmd += " -e GITHUB_RUN_NUMBER" - containerCmd += " -e GITHUB_RUN_ATTEMPT" - containerCmd += " -e GITHUB_JOB" - containerCmd += " -e GITHUB_ACTION" + containerCmd.WriteString(" -e GITHUB_RUN_ID") + containerCmd.WriteString(" -e GITHUB_RUN_NUMBER") + containerCmd.WriteString(" -e GITHUB_RUN_ATTEMPT") + containerCmd.WriteString(" -e GITHUB_JOB") + containerCmd.WriteString(" -e GITHUB_ACTION") // GitHub Actions event context - containerCmd += " -e GITHUB_EVENT_NAME" - containerCmd += " -e GITHUB_EVENT_PATH" + containerCmd.WriteString(" -e GITHUB_EVENT_NAME") + containerCmd.WriteString(" -e GITHUB_EVENT_PATH") // GitHub Actions actor context - containerCmd += " -e GITHUB_ACTOR" - containerCmd += " -e GITHUB_ACTOR_ID" - containerCmd += " -e GITHUB_TRIGGERING_ACTOR" + containerCmd.WriteString(" -e GITHUB_ACTOR") + containerCmd.WriteString(" -e GITHUB_ACTOR_ID") + containerCmd.WriteString(" -e GITHUB_TRIGGERING_ACTOR") // GitHub Actions workflow context - containerCmd += " -e GITHUB_WORKFLOW" - containerCmd += " -e GITHUB_WORKFLOW_REF" - containerCmd += " -e GITHUB_WORKFLOW_SHA" + containerCmd.WriteString(" -e GITHUB_WORKFLOW") + containerCmd.WriteString(" -e GITHUB_WORKFLOW_REF") + containerCmd.WriteString(" -e GITHUB_WORKFLOW_SHA") // GitHub Actions ref context - containerCmd += " -e GITHUB_REF" - containerCmd += " -e GITHUB_REF_NAME" - containerCmd += " -e GITHUB_REF_TYPE" - containerCmd += " -e GITHUB_HEAD_REF" - containerCmd += " -e GITHUB_BASE_REF" + containerCmd.WriteString(" -e GITHUB_REF") + containerCmd.WriteString(" -e GITHUB_REF_NAME") + containerCmd.WriteString(" -e GITHUB_REF_TYPE") + containerCmd.WriteString(" -e GITHUB_HEAD_REF") + containerCmd.WriteString(" -e GITHUB_BASE_REF") // Environment variables used by safeinputs MCP server // Only add if safe-inputs is actually enabled (has tools configured) if IsSafeInputsEnabled(workflowData.SafeInputs, workflowData) { - containerCmd += " -e GH_AW_SAFE_INPUTS_PORT" - containerCmd += " -e GH_AW_SAFE_INPUTS_API_KEY" + containerCmd.WriteString(" -e GH_AW_SAFE_INPUTS_PORT") + containerCmd.WriteString(" -e GH_AW_SAFE_INPUTS_API_KEY") } // Environment variables used by safeoutputs MCP server // Only add if safe-outputs is actually enabled (has tools configured) if HasSafeOutputsEnabled(workflowData.SafeOutputs) { - containerCmd += " -e GH_AW_SAFE_OUTPUTS_PORT" - containerCmd += " -e GH_AW_SAFE_OUTPUTS_API_KEY" + containerCmd.WriteString(" -e GH_AW_SAFE_OUTPUTS_PORT") + containerCmd.WriteString(" -e GH_AW_SAFE_OUTPUTS_API_KEY") } if len(gatewayConfig.Env) > 0 { // Using functional helper to extract map keys envVarNames := sliceutil.MapToSlice(gatewayConfig.Env) sort.Strings(envVarNames) for _, envVarName := range envVarNames { - containerCmd += " -e " + envVarName + containerCmd.WriteString(" -e " + envVarName) } } @@ -684,7 +680,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, sort.Strings(envVarNames) for _, envVarName := range envVarNames { - containerCmd += " -e " + envVarName + containerCmd.WriteString(" -e " + envVarName) } if mcpSetupGeneratorLog.Enabled() && len(envVarNames) > 0 { @@ -695,38 +691,38 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, // Add volume mounts // First, add the payload directory mount (rw for both agent and gateway) if payloadDir != "" { - containerCmd += " -v " + payloadDir + ":" + payloadDir + ":rw" + containerCmd.WriteString(" -v " + payloadDir + ":" + payloadDir + ":rw") } // Then add user-configured mounts if len(gatewayConfig.Mounts) > 0 { for _, mount := range gatewayConfig.Mounts { - containerCmd += " -v " + mount + containerCmd.WriteString(" -v " + mount) } } // Add entrypoint override if specified if gatewayConfig.Entrypoint != "" { - containerCmd += " --entrypoint " + shellEscapeArg(gatewayConfig.Entrypoint) + containerCmd.WriteString(" --entrypoint " + shellEscapeArg(gatewayConfig.Entrypoint)) } - containerCmd += " " + containerImage + containerCmd.WriteString(" " + containerImage) if len(gatewayConfig.EntrypointArgs) > 0 { for _, arg := range gatewayConfig.EntrypointArgs { - containerCmd += " " + shellEscapeArg(arg) + containerCmd.WriteString(" " + shellEscapeArg(arg)) } } if len(gatewayConfig.Args) > 0 { for _, arg := range gatewayConfig.Args { - containerCmd += " " + shellEscapeArg(arg) + containerCmd.WriteString(" " + shellEscapeArg(arg)) } } // Build the export command with proper quoting that allows variable expansion // We need to break out of quotes for ${GITHUB_WORKSPACE} variables - cmdWithExpandableVars := buildDockerCommandWithExpandableVars(containerCmd) + cmdWithExpandableVars := buildDockerCommandWithExpandableVars(containerCmd.String()) yaml.WriteString(" export MCP_GATEWAY_DOCKER_COMMAND=" + cmdWithExpandableVars + "\n") yaml.WriteString(" \n") diff --git a/pkg/workflow/metrics_test.go b/pkg/workflow/metrics_test.go index 6dfced79b45..8ad1a4ad57f 100644 --- a/pkg/workflow/metrics_test.go +++ b/pkg/workflow/metrics_test.go @@ -865,7 +865,7 @@ func TestFinalizeToolMetrics(t *testing.T) { // Verify tools are sorted by name if len(metrics.ToolCalls) > 1 { - for i := 0; i < len(metrics.ToolCalls)-1; i++ { + for i := range len(metrics.ToolCalls) - 1 { if metrics.ToolCalls[i].Name > metrics.ToolCalls[i+1].Name { t.Errorf("Tool calls not sorted: %s comes before %s", metrics.ToolCalls[i].Name, metrics.ToolCalls[i+1].Name) @@ -946,7 +946,7 @@ func TestFinalizeToolCallsAndSequence(t *testing.T) { // Verify tools are sorted by name if len(metrics.ToolCalls) > 1 { - for i := 0; i < len(metrics.ToolCalls)-1; i++ { + for i := range len(metrics.ToolCalls) - 1 { if metrics.ToolCalls[i].Name > metrics.ToolCalls[i+1].Name { t.Errorf("Tool calls not sorted: %s comes before %s", metrics.ToolCalls[i].Name, metrics.ToolCalls[i+1].Name) diff --git a/pkg/workflow/neutral_tools_simple_test.go b/pkg/workflow/neutral_tools_simple_test.go index fac889292b9..70b179a301c 100644 --- a/pkg/workflow/neutral_tools_simple_test.go +++ b/pkg/workflow/neutral_tools_simple_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "testing" ) @@ -122,12 +123,7 @@ func TestNeutralToolsWithoutSafeOutputs(t *testing.T) { // Helper function to check if a tool is present in the comma-separated result func containsTool(result, tool string) bool { tools := splitTools(result) - for _, t := range tools { - if t == tool { - return true - } - } - return false + return slices.Contains(tools, tool) } func splitTools(result string) []string { diff --git a/pkg/workflow/notify_comment.go b/pkg/workflow/notify_comment.go index b183967e71a..7649ea8bb42 100644 --- a/pkg/workflow/notify_comment.go +++ b/pkg/workflow/notify_comment.go @@ -3,6 +3,8 @@ package workflow import ( "encoding/json" "fmt" + "slices" + "strings" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" @@ -336,13 +338,7 @@ func (c *Compiler) buildConclusionJob(data *WorkflowData, mainJobName string, sa ) // Check if add_comment job exists in the safe output jobs - hasAddCommentJob := false - for _, jobName := range safeOutputJobNames { - if jobName == "add_comment" { - hasAddCommentJob = true - break - } - } + hasAddCommentJob := slices.Contains(safeOutputJobNames, "add_comment") // Build the condition based on whether add_comment job exists var condition ConditionNode @@ -461,15 +457,15 @@ func buildSafeOutputJobsEnvVars(jobNames []string) (string, []string) { // toEnvVarCase converts a string to uppercase environment variable case func toEnvVarCase(s string) string { // Convert to uppercase and keep underscores - result := "" + var result strings.Builder for _, ch := range s { if ch >= 'a' && ch <= 'z' { - result += string(ch - 32) // Convert to uppercase + result.WriteString(string(ch - 32)) // Convert to uppercase } else if ch >= 'A' && ch <= 'Z' { - result += string(ch) + result.WriteString(string(ch)) } else if ch == '_' { - result += "_" + result.WriteString("_") } } - return result + return result.String() } diff --git a/pkg/workflow/notify_comment_test.go b/pkg/workflow/notify_comment_test.go index 1447e1d9ff7..ee86b9a5c77 100644 --- a/pkg/workflow/notify_comment_test.go +++ b/pkg/workflow/notify_comment_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "strings" "testing" @@ -203,13 +204,7 @@ func TestConclusionJob(t *testing.T) { t.Errorf("Expected %d needs, got %d: %v", len(tt.expectNeeds), len(job.Needs), job.Needs) } for _, expectedNeed := range tt.expectNeeds { - found := false - for _, need := range job.Needs { - if need == expectedNeed { - found = true - break - } - } + found := slices.Contains(job.Needs, expectedNeed) if !found { t.Errorf("Expected need '%s' not found in job.Needs: %v", expectedNeed, job.Needs) } @@ -316,13 +311,7 @@ func TestConclusionJobIntegration(t *testing.T) { // Verify job depends on the safe output jobs for _, expectedNeed := range safeOutputJobNames { - found := false - for _, need := range job.Needs { - if need == expectedNeed { - found = true - break - } - } + found := slices.Contains(job.Needs, expectedNeed) if !found { t.Errorf("Expected conclusion job to depend on '%s'", expectedNeed) } diff --git a/pkg/workflow/package_extraction.go b/pkg/workflow/package_extraction.go index 14a4af8a56d..ced416490ec 100644 --- a/pkg/workflow/package_extraction.go +++ b/pkg/workflow/package_extraction.go @@ -96,6 +96,7 @@ package workflow import ( + "slices" "strings" "github.com/github/gh-aw/pkg/logger" @@ -242,9 +243,9 @@ func (pe *PackageExtractor) ExtractPackages(commands string) []string { } var packages []string - lines := strings.Split(commands, "\n") + lines := strings.SplitSeq(commands, "\n") - for _, line := range lines { + for line := range lines { words := strings.Fields(line) for i, word := range words { // Check if this word matches one of our command names @@ -288,12 +289,7 @@ func (pe *PackageExtractor) getRequiredSubcommands() []string { // isCommandName checks if the given word matches any of the configured command names func (pe *PackageExtractor) isCommandName(word string) bool { - for _, cmdName := range pe.CommandNames { - if word == cmdName { - return true - } - } - return false + return slices.Contains(pe.CommandNames, word) } // extractWithSubcommands extracts a package name when any of the required subcommands must be present @@ -301,11 +297,9 @@ func (pe *PackageExtractor) isCommandName(word string) bool { func (pe *PackageExtractor) extractWithSubcommands(words []string, commandIndex int, subcommands []string) string { // Look for any of the required subcommands after the command name for j := commandIndex + 1; j < len(words); j++ { - for _, subcommand := range subcommands { - if words[j] == subcommand { - // Found a matching subcommand - now find the package name - return pe.findPackageName(words, j+1) - } + if slices.Contains(subcommands, words[j]) { + // Found a matching subcommand - now find the package name + return pe.findPackageName(words, j+1) } } return "" diff --git a/pkg/workflow/permissions_factory.go b/pkg/workflow/permissions_factory.go index de0fc1f022d..5095c625c54 100644 --- a/pkg/workflow/permissions_factory.go +++ b/pkg/workflow/permissions_factory.go @@ -1,6 +1,10 @@ package workflow -import "github.com/github/gh-aw/pkg/logger" +import ( + "maps" + + "github.com/github/gh-aw/pkg/logger" +) var permissionsFactoryLog = logger.New("workflow:permissions_factory") @@ -48,9 +52,7 @@ func NewPermissionsFromMap(perms map[PermissionScope]PermissionLevel) *Permissio permissionsFactoryLog.Printf("Creating permissions from map: scope_count=%d", len(perms)) } p := NewPermissions() - for scope, level := range perms { - p.permissions[scope] = level - } + maps.Copy(p.permissions, perms) return p } diff --git a/pkg/workflow/permissions_operations.go b/pkg/workflow/permissions_operations.go index c639283be66..e6db07205d6 100644 --- a/pkg/workflow/permissions_operations.go +++ b/pkg/workflow/permissions_operations.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "sort" "strings" @@ -233,9 +234,7 @@ func (p *Permissions) RenderToYAML() string { } // Override with explicit permissions - for scope, level := range p.permissions { - allPerms[scope] = level - } + maps.Copy(allPerms, p.permissions) if len(allPerms) == 0 { // If explicitEmpty is true, render "permissions: {}" diff --git a/pkg/workflow/permissions_validation.go b/pkg/workflow/permissions_validation.go index 2525c85dade..65ac7e18a74 100644 --- a/pkg/workflow/permissions_validation.go +++ b/pkg/workflow/permissions_validation.go @@ -4,6 +4,7 @@ import ( _ "embed" "encoding/json" "fmt" + "slices" "sort" "strings" @@ -234,19 +235,10 @@ func checkMissingPermissions(permissions *Permissions, required map[PermissionSc continue } - requiresScope := false - for _, readScope := range perms.ReadPermissions { - if readScope == scope { - requiresScope = true - break - } - } + requiresScope := slices.Contains(perms.ReadPermissions, scope) if !requiresScope { - for _, writeScope := range perms.WritePermissions { - if writeScope == scope { - requiresScope = true - break - } + if slices.Contains(perms.WritePermissions, scope) { + requiresScope = true } } @@ -292,11 +284,8 @@ func formatMissingPermissionsMessage(result *PermissionsValidationResult) string var requiredBy []string if len(result.MissingToolsetDetails) > 0 { for toolset, toolsetScopes := range result.MissingToolsetDetails { - for _, ts := range toolsetScopes { - if ts == scope { - requiredBy = append(requiredBy, toolset) - break - } + if slices.Contains(toolsetScopes, scope) { + requiredBy = append(requiredBy, toolset) } } } diff --git a/pkg/workflow/processing_benchmark_test.go b/pkg/workflow/processing_benchmark_test.go index bd446bc9a82..cff80cc0359 100644 --- a/pkg/workflow/processing_benchmark_test.go +++ b/pkg/workflow/processing_benchmark_test.go @@ -41,8 +41,7 @@ Simple tool processing test. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = compiler.ParseWorkflowFile(testFile) } } @@ -95,8 +94,7 @@ Complex tool configuration processing. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = compiler.ParseWorkflowFile(testFile) } } @@ -135,8 +133,7 @@ Simple safe outputs configuration. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = compiler.ParseWorkflowFile(testFile) } } @@ -190,8 +187,7 @@ Complex safe outputs configuration. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = compiler.ParseWorkflowFile(testFile) } } @@ -232,8 +228,7 @@ Network permissions processing. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = compiler.ParseWorkflowFile(testFile) } } @@ -273,8 +268,7 @@ Permission processing test. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = compiler.ParseWorkflowFile(testFile) } } @@ -312,8 +306,7 @@ Role processing test. compiler := NewCompiler() - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _, _ = compiler.ParseWorkflowFile(testFile) } } diff --git a/pkg/workflow/pull_request_activity_types_test.go b/pkg/workflow/pull_request_activity_types_test.go index 26fff53ee47..f2dc2c4bd35 100644 --- a/pkg/workflow/pull_request_activity_types_test.go +++ b/pkg/workflow/pull_request_activity_types_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "strings" "testing" ) @@ -58,13 +59,7 @@ func TestPullRequestActivityTypeEnumValidation(t *testing.T) { } // Check if the activity type is in the types array - foundType := false - for _, irType := range ir.Types { - if irType == activityType { - foundType = true - break - } - } + foundType := slices.Contains(ir.Types, activityType) if !foundType { t.Errorf("Activity type %q not found in parsed types: %v", activityType, ir.Types) @@ -360,13 +355,7 @@ func TestPullRequestActivityTypeInTriggerParser(t *testing.T) { t.Run("document unsupported but valid types", func(t *testing.T) { for officialType := range officialTypes { - isSupported := false - for _, supported := range currentlySupported { - if supported == officialType { - isSupported = true - break - } - } + isSupported := slices.Contains(currentlySupported, officialType) if !isSupported { t.Logf("Official GitHub Actions type %q is not in trigger_parser.go validTypes map", officialType) } diff --git a/pkg/workflow/redact_secrets.go b/pkg/workflow/redact_secrets.go index 4dd218a3d04..161b0c657be 100644 --- a/pkg/workflow/redact_secrets.go +++ b/pkg/workflow/redact_secrets.go @@ -3,6 +3,7 @@ package workflow import ( "fmt" "regexp" + "slices" "sort" "strings" @@ -152,8 +153,8 @@ func (c *Compiler) renderStepFromMap(yaml *strings.Builder, step map[string]any, // Handle multi-line strings (especially for 'run' field) if field == "run" && strings.Contains(v, "\n") { fmt.Fprintf(yaml, "%s: |\n", field) - lines := strings.Split(v, "\n") - for _, line := range lines { + lines := strings.SplitSeq(v, "\n") + for line := range lines { fmt.Fprintf(yaml, "%s %s\n", indent, line) } } else { @@ -174,13 +175,7 @@ func (c *Compiler) renderStepFromMap(yaml *strings.Builder, step map[string]any, // Add any remaining fields not in the predefined order for field, value := range step { // Skip fields we've already processed - skip := false - for _, f := range fieldOrder { - if f == field { - skip = true - break - } - } + skip := slices.Contains(fieldOrder, field) if skip { continue } @@ -195,8 +190,8 @@ func (c *Compiler) renderStepFromMap(yaml *strings.Builder, step map[string]any, // Handle multi-line strings if strings.Contains(v, "\n") { fmt.Fprintf(yaml, "%s: |\n", field) - lines := strings.Split(v, "\n") - for _, line := range lines { + lines := strings.SplitSeq(v, "\n") + for line := range lines { fmt.Fprintf(yaml, "%s %s\n", indent, line) } } else { diff --git a/pkg/workflow/regex_benchmark_test.go b/pkg/workflow/regex_benchmark_test.go index 399f68a794f..f9d6fce64fa 100644 --- a/pkg/workflow/regex_benchmark_test.go +++ b/pkg/workflow/regex_benchmark_test.go @@ -11,8 +11,7 @@ import ( func BenchmarkRegexCompileInFunction(b *testing.B) { text := "2024-01-15 12:34:56 [ERROR] Something went wrong with the process" - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { // Old approach: compile regex every time re := regexp.MustCompile(`^\d{4}-\d{2}-\d{2}\s+\d{2}:\d{2}:\d{2}(\.\d+)?\s+`) _ = re.ReplaceAllString(text, "") @@ -25,8 +24,7 @@ func BenchmarkRegexPrecompiled(b *testing.B) { // Pre-compile regex once re := regexp.MustCompile(`^\d{4}-\d{2}-\d{2}\s+\d{2}:\d{2}:\d{2}(\.\d+)?\s+`) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { // New approach: use pre-compiled regex _ = re.ReplaceAllString(text, "") } @@ -42,8 +40,7 @@ func BenchmarkLogProcessingOld(b *testing.B) { "2024-01-15 12:35:00 [ERROR] Max retries exceeded", } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { for _, line := range lines { // Old approach: compile for each line re1 := regexp.MustCompile(`^\d{4}-\d{2}-\d{2}\s+\d{2}:\d{2}:\d{2}(\.\d+)?\s+`) @@ -68,8 +65,7 @@ func BenchmarkLogProcessingNew(b *testing.B) { re1 := regexp.MustCompile(`^\d{4}-\d{2}-\d{2}\s+\d{2}:\d{2}:\d{2}(\.\d+)?\s+`) re2 := regexp.MustCompile(`(?i)^\[?(ERROR|WARNING|WARN|INFO|DEBUG)\]?\s*[:-]?\s*`) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { for _, line := range lines { // New approach: use pre-compiled regexes cleaned := re1.ReplaceAllString(line, "") @@ -88,8 +84,7 @@ func BenchmarkCodexLogParsingOld(b *testing.B) { "] success in 0.5s", } - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { for _, line := range lines { // Old approach: compile for each line regexp.MustCompile(`\] tool ([^(]+)\(`).FindStringSubmatch(line) @@ -118,8 +113,7 @@ func BenchmarkCodexLogParsingNew(b *testing.B) { re4 := regexp.MustCompile(`^exec (.+?) in`) re5 := regexp.MustCompile(`in\s+(\d+(?:\.\d+)?)\s*s`) - b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { for _, line := range lines { // New approach: use pre-compiled regexes re1.FindStringSubmatch(line) diff --git a/pkg/workflow/role_checks.go b/pkg/workflow/role_checks.go index 3f16ff103f1..7cfa2b8c969 100644 --- a/pkg/workflow/role_checks.go +++ b/pkg/workflow/role_checks.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "slices" "sort" "strings" @@ -365,13 +366,7 @@ func (c *Compiler) hasSafeEventsOnly(data *WorkflowData, frontmatter map[string] } // Check if this event is in the safe list - isSafe := false - for _, safeEvent := range constants.SafeWorkflowEvents { - if eventName == safeEvent { - isSafe = true - break - } - } + isSafe := slices.Contains(constants.SafeWorkflowEvents, eventName) if !isSafe { hasUnsafeEvents = true break @@ -405,13 +400,7 @@ func (c *Compiler) hasSafeEventsOnly(data *WorkflowData, frontmatter map[string] // so it's only considered "safe" if "write" is in the allowed roles if hasWorkflowDispatch && !hasUnsafeEvents { // Check if "write" is in the allowed roles - hasWriteRole := false - for _, role := range data.Roles { - if role == "write" { - hasWriteRole = true - break - } - } + hasWriteRole := slices.Contains(data.Roles, "write") // If write is not in the allowed roles, workflow_dispatch needs permission checks if !hasWriteRole { return false diff --git a/pkg/workflow/runtime_deduplication.go b/pkg/workflow/runtime_deduplication.go index e0d06061d16..35d74ebd1f7 100644 --- a/pkg/workflow/runtime_deduplication.go +++ b/pkg/workflow/runtime_deduplication.go @@ -25,8 +25,8 @@ func DeduplicateRuntimeSetupStepsFromCustomSteps(customSteps string, runtimeRequ // This is necessary because YAML treats "# comment" as a comment, not part of the value // Format: "uses: action@sha # v1.0.0" -> after unmarshal, only "action@sha" remains versionComments := make(map[string]string) // key: action@sha, value: # v1.0.0 - lines := strings.Split(customSteps, "\n") - for _, line := range lines { + lines := strings.SplitSeq(customSteps, "\n") + for line := range lines { trimmed := strings.TrimSpace(line) if strings.HasPrefix(trimmed, "uses:") && strings.Contains(trimmed, " # ") { // Extract the uses value and version comment diff --git a/pkg/workflow/runtime_detection.go b/pkg/workflow/runtime_detection.go index 32117aa3009..d67d94c2bc0 100644 --- a/pkg/workflow/runtime_detection.go +++ b/pkg/workflow/runtime_detection.go @@ -66,8 +66,8 @@ func DetectRuntimeRequirements(workflowData *WorkflowData) []RuntimeRequirement // detectFromCustomSteps scans custom steps YAML for runtime commands func detectFromCustomSteps(customSteps string, requirements map[string]*RuntimeRequirement) { log.Print("Scanning custom steps for runtime commands") - lines := strings.Split(customSteps, "\n") - for _, line := range lines { + lines := strings.SplitSeq(customSteps, "\n") + for line := range lines { // Look for run: commands if strings.Contains(line, "run:") { // Extract the command part diff --git a/pkg/workflow/runtime_setup_test.go b/pkg/workflow/runtime_setup_test.go index c90b68cd6d7..ea8d602152a 100644 --- a/pkg/workflow/runtime_setup_test.go +++ b/pkg/workflow/runtime_setup_test.go @@ -505,13 +505,13 @@ func getRequirementIDs(requirements map[string]*RuntimeRequirement) []string { } func stepsToString(steps []GitHubActionStep) string { - var result string + var result strings.Builder for _, step := range steps { for _, line := range step { - result += line + "\n" + result.WriteString(line + "\n") } } - return result + return result.String() } func TestRuntimeFilteringWithExistingSetupActions(t *testing.T) { diff --git a/pkg/workflow/runtime_step_generator.go b/pkg/workflow/runtime_step_generator.go index 158f2b595e5..e5deb676c1e 100644 --- a/pkg/workflow/runtime_step_generator.go +++ b/pkg/workflow/runtime_step_generator.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "sort" "github.com/github/gh-aw/pkg/logger" @@ -131,9 +132,7 @@ func generateSetupStep(req *RuntimeRequirement) GitHubActionStep { allExtraFields := make(map[string]string) // Add runtime extra fields (already formatted) - for k, v := range runtime.ExtraWithFields { - allExtraFields[k] = v - } + maps.Copy(allExtraFields, runtime.ExtraWithFields) // Add user extra fields (need formatting), these override runtime fields for k, v := range req.ExtraFields { diff --git a/pkg/workflow/safe_inputs_generator_test.go b/pkg/workflow/safe_inputs_generator_test.go index bf35504d39c..753d50b24a4 100644 --- a/pkg/workflow/safe_inputs_generator_test.go +++ b/pkg/workflow/safe_inputs_generator_test.go @@ -347,7 +347,7 @@ func TestSafeInputsStableCodeGeneration(t *testing.T) { iterations := 10 entryScripts := make([]string, iterations) - for i := 0; i < iterations; i++ { + for i := range iterations { entryScripts[i] = generateSafeInputsMCPServerScript(config) } @@ -361,7 +361,7 @@ func TestSafeInputsStableCodeGeneration(t *testing.T) { // Generate the tools config JSON multiple times and verify identical output toolsConfigs := make([]string, iterations) - for i := 0; i < iterations; i++ { + for i := range iterations { toolsConfigs[i] = generateSafeInputsToolsConfig(config) } @@ -372,17 +372,10 @@ func TestSafeInputsStableCodeGeneration(t *testing.T) { // Find first difference for debugging for j := 0; j < len(toolsConfigs[0]) && j < len(toolsConfigs[i]); j++ { if toolsConfigs[0][j] != toolsConfigs[i][j] { - start := j - 50 - if start < 0 { - start = 0 - } + start := max(j-50, 0) end := j + 50 - if end > len(toolsConfigs[0]) { - end = len(toolsConfigs[0]) - } - if end > len(toolsConfigs[i]) { - end = len(toolsConfigs[i]) - } + end = min(end, len(toolsConfigs[0])) + end = min(end, len(toolsConfigs[i])) t.Errorf("First difference at position %d:\n Expected: %q\n Got: %q", j, toolsConfigs[0][start:end], toolsConfigs[i][start:end]) break } @@ -405,7 +398,7 @@ func TestSafeInputsStableCodeGeneration(t *testing.T) { // Test JavaScript tool script stability jsScripts := make([]string, iterations) - for i := 0; i < iterations; i++ { + for i := range iterations { jsScripts[i] = generateSafeInputJavaScriptToolScript(config.Tools["alpha-tool"]) } diff --git a/pkg/workflow/safe_inputs_renderer_test.go b/pkg/workflow/safe_inputs_renderer_test.go index 2a76f1859c8..d557518a089 100644 --- a/pkg/workflow/safe_inputs_renderer_test.go +++ b/pkg/workflow/safe_inputs_renderer_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "testing" ) @@ -62,13 +63,7 @@ func TestGetSafeInputsEnvVars(t *testing.T) { } for _, expected := range tt.contains { - found := false - for _, v := range result { - if v == expected { - found = true - break - } - } + found := slices.Contains(result, expected) if !found { t.Errorf("Expected to contain %s, got %v", expected, result) } @@ -137,7 +132,7 @@ func TestCollectSafeInputsSecretsStability(t *testing.T) { // Test collectSafeInputsSecrets stability iterations := 10 secretResults := make([]map[string]string, iterations) - for i := 0; i < iterations; i++ { + for i := range iterations { secretResults[i] = collectSafeInputsSecrets(config) } @@ -176,7 +171,7 @@ func TestGetSafeInputsEnvVarsStability(t *testing.T) { // Test getSafeInputsEnvVars stability iterations := 10 envResults := make([][]string, iterations) - for i := 0; i < iterations; i++ { + for i := range iterations { envResults[i] = getSafeInputsEnvVars(config) } diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index 16d81b875bd..4a4cb3b35d1 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "strings" "github.com/github/gh-aw/pkg/constants" @@ -324,9 +325,7 @@ func mergeSafeJobs(base map[string]*SafeJobConfig, additional map[string]*SafeJo result := make(map[string]*SafeJobConfig) // Copy base safe-jobs - for name, config := range base { - result[name] = config - } + maps.Copy(result, base) // Add additional safe-jobs, checking for conflicts for name, config := range additional { diff --git a/pkg/workflow/safe_outputs_config_generation_helpers.go b/pkg/workflow/safe_outputs_config_generation_helpers.go index 673335ef051..bf2ef1ba898 100644 --- a/pkg/workflow/safe_outputs_config_generation_helpers.go +++ b/pkg/workflow/safe_outputs_config_generation_helpers.go @@ -1,6 +1,7 @@ package workflow import ( + "maps" "strings" "github.com/github/gh-aw/pkg/logger" @@ -186,9 +187,7 @@ func generateTargetConfigWithRepos(targetConfig SafeOutputTargetConfig, max *str } // Add any additional fields - for key, value := range additionalFields { - config[key] = value - } + maps.Copy(config, additionalFields) return config } diff --git a/pkg/workflow/safe_outputs_domains_validation.go b/pkg/workflow/safe_outputs_domains_validation.go index 4a63d928eb2..164b410aa65 100644 --- a/pkg/workflow/safe_outputs_domains_validation.go +++ b/pkg/workflow/safe_outputs_domains_validation.go @@ -104,10 +104,10 @@ func validateDomainPattern(domain string) error { // Strip protocol prefix if present (http:// or https://) // This allows protocol-specific domain filtering domainWithoutProtocol := domain - if strings.HasPrefix(domain, "https://") { - domainWithoutProtocol = strings.TrimPrefix(domain, "https://") - } else if strings.HasPrefix(domain, "http://") { - domainWithoutProtocol = strings.TrimPrefix(domain, "http://") + if after, ok := strings.CutPrefix(domain, "https://"); ok { + domainWithoutProtocol = after + } else if after, ok := strings.CutPrefix(domain, "http://"); ok { + domainWithoutProtocol = after } // Check for wildcard-only pattern diff --git a/pkg/workflow/safe_outputs_tools_generation.go b/pkg/workflow/safe_outputs_tools_generation.go index 6733834701d..c655909a8d4 100644 --- a/pkg/workflow/safe_outputs_tools_generation.go +++ b/pkg/workflow/safe_outputs_tools_generation.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "maps" "sort" "github.com/github/gh-aw/pkg/stringutil" @@ -275,9 +276,7 @@ func generateFilteredToolsJSON(data *WorkflowData, markdownPath string) (string, if enabledTools[toolName] { // Create a copy of the tool to avoid modifying the original enhancedTool := make(map[string]any) - for k, v := range tool { - enhancedTool[k] = v - } + maps.Copy(enhancedTool, tool) // Enhance the description with configuration details if description, ok := enhancedTool["description"].(string); ok { diff --git a/pkg/workflow/sanitize_incoming_text_fuzz_test.go b/pkg/workflow/sanitize_incoming_text_fuzz_test.go index 5b7cd622d85..133482b59ee 100644 --- a/pkg/workflow/sanitize_incoming_text_fuzz_test.go +++ b/pkg/workflow/sanitize_incoming_text_fuzz_test.go @@ -134,7 +134,7 @@ func FuzzSanitizeIncomingText(f *testing.F) { if strings.Contains(text, "@") && !strings.Contains(text, "`@") { // Check that result doesn't have bare mentions // Pattern: @ followed by alphanumeric (not preceded by backtick) - for i := 0; i < len(result.Sanitized)-1; i++ { + for i := range len(result.Sanitized) - 1 { if result.Sanitized[i] == '@' { // Check if preceded by backtick if i > 0 && result.Sanitized[i-1] == '`' { diff --git a/pkg/workflow/sanitize_label_fuzz_test.go b/pkg/workflow/sanitize_label_fuzz_test.go index 09eeaa9e78d..57ed59c39e9 100644 --- a/pkg/workflow/sanitize_label_fuzz_test.go +++ b/pkg/workflow/sanitize_label_fuzz_test.go @@ -120,7 +120,7 @@ func FuzzSanitizeLabelContent(f *testing.F) { // Verify ALL mentions are neutralized (wrapped in backticks) if strings.Contains(text, "@") && !strings.Contains(text, "`@") { // Check that result doesn't have bare mentions - for i := 0; i < len(result.Sanitized)-1; i++ { + for i := range len(result.Sanitized) - 1 { if result.Sanitized[i] == '@' { // Check if preceded by backtick if i > 0 && result.Sanitized[i-1] == '`' { diff --git a/pkg/workflow/script_registry.go b/pkg/workflow/script_registry.go index c210af4f353..602d9a813a9 100644 --- a/pkg/workflow/script_registry.go +++ b/pkg/workflow/script_registry.go @@ -385,7 +385,7 @@ func GetAllScriptFilenames() []string { sortedFilenames := make([]string, len(filenames)) copy(sortedFilenames, filenames) // Using a simple sort to avoid importing sort package issues - for i := 0; i < len(sortedFilenames); i++ { + for i := range sortedFilenames { for j := i + 1; j < len(sortedFilenames); j++ { if sortedFilenames[i] > sortedFilenames[j] { sortedFilenames[i], sortedFilenames[j] = sortedFilenames[j], sortedFilenames[i] diff --git a/pkg/workflow/script_registry_test.go b/pkg/workflow/script_registry_test.go index 362f2c8b148..3962d822b56 100644 --- a/pkg/workflow/script_registry_test.go +++ b/pkg/workflow/script_registry_test.go @@ -89,7 +89,7 @@ func TestScriptRegistry_ConcurrentAccess(t *testing.T) { var wg sync.WaitGroup results := make([]string, 10) - for i := 0; i < 10; i++ { + for i := range 10 { wg.Add(1) go func(idx int) { defer wg.Done() diff --git a/pkg/workflow/secret_extraction.go b/pkg/workflow/secret_extraction.go index 514d010d90b..606d594d353 100644 --- a/pkg/workflow/secret_extraction.go +++ b/pkg/workflow/secret_extraction.go @@ -1,6 +1,7 @@ package workflow import ( + "maps" "regexp" "strings" @@ -83,9 +84,7 @@ func ExtractSecretsFromMap(values map[string]string) map[string]string { for _, value := range values { secrets := ExtractSecretsFromValue(value) - for varName, expr := range secrets { - allSecrets[varName] = expr - } + maps.Copy(allSecrets, secrets) } secretLog.Printf("Extracted total of %d unique secrets from map", len(allSecrets)) diff --git a/pkg/workflow/security_fuzz_test.go b/pkg/workflow/security_fuzz_test.go index 08cc1e67f26..b7f235c6407 100644 --- a/pkg/workflow/security_fuzz_test.go +++ b/pkg/workflow/security_fuzz_test.go @@ -198,12 +198,13 @@ func FuzzTemplateRendering(f *testing.F) { f.Add("${{ ${{ nested }} }}") // Long expressions - longExpr := "${{ " - for i := 0; i < 50; i++ { - longExpr += "github.workflow && " + var longExpr strings.Builder + longExpr.WriteString("${{ ") + for range 50 { + longExpr.WriteString("github.workflow && ") } - longExpr += "github.repository }}" - f.Add(longExpr) + longExpr.WriteString("github.repository }}") + f.Add(longExpr.String()) // Special characters f.Add("${{ github.workflow }}™©®") diff --git a/pkg/workflow/serena_container_selection_test.go b/pkg/workflow/serena_container_selection_test.go index 78da04d2ec6..21536516bc9 100644 --- a/pkg/workflow/serena_container_selection_test.go +++ b/pkg/workflow/serena_container_selection_test.go @@ -3,6 +3,7 @@ package workflow import ( + "slices" "testing" "github.com/github/gh-aw/pkg/constants" @@ -96,13 +97,7 @@ func TestSerenaLanguageSupport(t *testing.T) { // Verify some expected languages are present in default container expectedLangs := []string{"go", "typescript", "python", "java", "rust"} for _, lang := range expectedLangs { - found := false - for _, supportedLang := range defaultLangs { - if supportedLang == lang { - found = true - break - } - } + found := slices.Contains(defaultLangs, lang) if !found { t.Errorf("Expected language '%s' not found in default container support list", lang) } diff --git a/pkg/workflow/sh.go b/pkg/workflow/sh.go index bf337b6da6a..cc656bbc47b 100644 --- a/pkg/workflow/sh.go +++ b/pkg/workflow/sh.go @@ -41,8 +41,8 @@ func WritePromptFileToYAML(yaml *strings.Builder, filename string, indent string // WriteShellScriptToYAML writes a shell script with proper indentation to a strings.Builder func WriteShellScriptToYAML(yaml *strings.Builder, script string, indent string) { - scriptLines := strings.Split(script, "\n") - for _, line := range scriptLines { + scriptLines := strings.SplitSeq(script, "\n") + for line := range scriptLines { // Skip empty lines at the beginning or end if strings.TrimSpace(line) != "" { fmt.Fprintf(yaml, "%s%s\n", indent, line) diff --git a/pkg/workflow/shell_backslash_integration_test.go b/pkg/workflow/shell_backslash_integration_test.go index 70fe6bb15ad..c5f1ac26b73 100644 --- a/pkg/workflow/shell_backslash_integration_test.go +++ b/pkg/workflow/shell_backslash_integration_test.go @@ -1,5 +1,4 @@ //go:build integration -// +build integration package workflow diff --git a/pkg/workflow/step_order_validation.go b/pkg/workflow/step_order_validation.go index ba6f6d602a5..7bd64a10f51 100644 --- a/pkg/workflow/step_order_validation.go +++ b/pkg/workflow/step_order_validation.go @@ -3,6 +3,7 @@ package workflow import ( "fmt" "path/filepath" + "slices" "strings" "github.com/github/gh-aw/pkg/logger" @@ -198,10 +199,8 @@ func isPathScannedBySecretRedaction(path string) bool { if strings.HasPrefix(path, "/tmp/") && strings.Contains(path, "*") { ext := filepath.Ext(path) safeExtensions := []string{".txt", ".json", ".log", ".jsonl"} - for _, safeExt := range safeExtensions { - if ext == safeExt { - return true - } + if slices.Contains(safeExtensions, ext) { + return true } } @@ -211,10 +210,8 @@ func isPathScannedBySecretRedaction(path string) bool { // Path must have one of the scanned extensions: .txt, .json, .log, .jsonl ext := filepath.Ext(path) scannedExtensions := []string{".txt", ".json", ".log", ".jsonl"} - for _, scannedExt := range scannedExtensions { - if ext == scannedExt { - return true - } + if slices.Contains(scannedExtensions, ext) { + return true } // If path is a directory (ends with /), we assume it contains scannable files diff --git a/pkg/workflow/step_types.go b/pkg/workflow/step_types.go index 9df75edc924..ab0211aa62d 100644 --- a/pkg/workflow/step_types.go +++ b/pkg/workflow/step_types.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "github.com/github/gh-aw/pkg/logger" "github.com/goccy/go-yaml" @@ -154,16 +155,12 @@ func (s *WorkflowStep) Clone() *WorkflowStep { if s.With != nil { clone.With = make(map[string]any, len(s.With)) - for k, v := range s.With { - clone.With[k] = v - } + maps.Copy(clone.With, s.With) } if s.Env != nil { clone.Env = make(map[string]string, len(s.Env)) - for k, v := range s.Env { - clone.Env[k] = v - } + maps.Copy(clone.Env, s.Env) } return clone diff --git a/pkg/workflow/stop_after.go b/pkg/workflow/stop_after.go index c705d1a57ea..39d971cc287 100644 --- a/pkg/workflow/stop_after.go +++ b/pkg/workflow/stop_after.go @@ -173,14 +173,14 @@ func ExtractStopTimeFromLockFile(lockFilePath string) string { // Legacy fallback: Look for GH_AW_STOP_TIME in the workflow body // Use legacy method if: no metadata, legacy format, metadata exists but stop_time is empty, or metadata was malformed if err != nil || metadata == nil || isLegacy || metadata.StopTime == "" { - lines := strings.Split(contentStr, "\n") - for _, line := range lines { + lines := strings.SplitSeq(contentStr, "\n") + for line := range lines { // Look for GH_AW_STOP_TIME: YYYY-MM-DD HH:MM:SS // This is in the env section of the stop time check job if strings.Contains(line, "GH_AW_STOP_TIME:") { prefix := "GH_AW_STOP_TIME:" - if idx := strings.Index(line, prefix); idx != -1 { - return strings.TrimSpace(line[idx+len(prefix):]) + if _, after, ok := strings.Cut(line, prefix); ok { + return strings.TrimSpace(after) } } } diff --git a/pkg/workflow/strict_mode_validation.go b/pkg/workflow/strict_mode_validation.go index 585791210e1..fd8c5a6fbbd 100644 --- a/pkg/workflow/strict_mode_validation.go +++ b/pkg/workflow/strict_mode_validation.go @@ -42,6 +42,7 @@ package workflow import ( "fmt" "os" + "slices" "strings" "github.com/github/gh-aw/pkg/console" @@ -90,19 +91,15 @@ func (c *Compiler) validateStrictNetwork(networkPermissions *NetworkPermissions) } // If allowed list contains "defaults", that's acceptable (this is the automatic default) - for _, domain := range networkPermissions.Allowed { - if domain == "defaults" { - strictModeValidationLog.Printf("Network validation passed: allowed list contains 'defaults'") - return nil - } + if slices.Contains(networkPermissions.Allowed, "defaults") { + strictModeValidationLog.Printf("Network validation passed: allowed list contains 'defaults'") + return nil } // Check for wildcard "*" in allowed domains - for _, domain := range networkPermissions.Allowed { - if domain == "*" { - strictModeValidationLog.Printf("Network validation failed: wildcard detected") - return fmt.Errorf("strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access. Specify explicit domains or use ecosystem identifiers like 'python', 'node', 'containers'. See: https://github.github.com/gh-aw/reference/network/#available-ecosystem-identifiers") - } + if slices.Contains(networkPermissions.Allowed, "*") { + strictModeValidationLog.Printf("Network validation failed: wildcard detected") + return fmt.Errorf("strict mode: wildcard '*' is not allowed in network.allowed domains to prevent unrestricted internet access. Specify explicit domains or use ecosystem identifiers like 'python', 'node', 'containers'. See: https://github.github.com/gh-aw/reference/network/#available-ecosystem-identifiers") } strictModeValidationLog.Printf("Network validation passed: allowed_count=%d", len(networkPermissions.Allowed)) @@ -551,11 +548,9 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N // Check if allowed contains "*" (unrestricted network access) // If it does, firewall is not required - for _, domain := range networkPermissions.Allowed { - if domain == "*" { - strictModeValidationLog.Printf("Wildcard '*' in allowed domains, skipping firewall validation") - return nil - } + if slices.Contains(networkPermissions.Allowed, "*") { + strictModeValidationLog.Printf("Wildcard '*' in allowed domains, skipping firewall validation") + return nil } // At this point, we have network domains (or defaults) and copilot/codex engine diff --git a/pkg/workflow/strings.go b/pkg/workflow/strings.go index d1904b17068..e62511c4c84 100644 --- a/pkg/workflow/strings.go +++ b/pkg/workflow/strings.go @@ -77,6 +77,7 @@ package workflow import ( "regexp" + "slices" "strings" "github.com/github/gh-aw/pkg/logger" @@ -154,13 +155,7 @@ func SanitizeName(name string, opts *SanitizeOptions) string { result = strings.ReplaceAll(result, " ", "-") // Check if underscores should be preserved - preserveUnderscore := false - for _, char := range opts.PreserveSpecialChars { - if char == '_' { - preserveUnderscore = true - break - } - } + preserveUnderscore := slices.Contains(opts.PreserveSpecialChars, '_') // Replace underscores with hyphens if not preserved if !preserveUnderscore { @@ -168,19 +163,20 @@ func SanitizeName(name string, opts *SanitizeOptions) string { } // Build character preservation pattern based on options - preserveChars := "a-z0-9-" // Always preserve alphanumeric and hyphens + var preserveChars strings.Builder + preserveChars.WriteString("a-z0-9-") // Always preserve alphanumeric and hyphens if len(opts.PreserveSpecialChars) > 0 { for _, char := range opts.PreserveSpecialChars { // Escape special regex characters switch char { case '.', '_': - preserveChars += string(char) + preserveChars.WriteString(string(char)) } } } // Create pattern for characters to remove/replace - pattern := regexp.MustCompile(`[^` + preserveChars + `]+`) + pattern := regexp.MustCompile(`[^` + preserveChars.String() + `]+`) // Replace unwanted characters with hyphens or empty based on context if len(opts.PreserveSpecialChars) > 0 { diff --git a/pkg/workflow/temp_dir_before_custom_steps_test.go b/pkg/workflow/temp_dir_before_custom_steps_test.go index 16afb56d4b1..07ea22e1816 100644 --- a/pkg/workflow/temp_dir_before_custom_steps_test.go +++ b/pkg/workflow/temp_dir_before_custom_steps_test.go @@ -77,7 +77,7 @@ steps: if len(line) > 2 && line[0] == ' ' && line[1] == ' ' && line[2] != ' ' && line[2] != '\t' { // Calculate the position in the original string nextJobStart = 0 - for j := 0; j < i; j++ { + for j := range i { nextJobStart += len(lines[j]) + 1 // +1 for newline } break @@ -93,8 +93,8 @@ steps: // Find all step names in order stepNames := []string{} - stepLines := strings.Split(agentJobSection, "\n") - for _, line := range stepLines { + stepLines := strings.SplitSeq(agentJobSection, "\n") + for line := range stepLines { // Check if line contains "- name:" (with any amount of leading whitespace) if strings.Contains(line, "- name:") { // Extract the name part after "- name:" diff --git a/pkg/workflow/template_fuzz_test.go b/pkg/workflow/template_fuzz_test.go index 09b2d95abb8..ff6bcf61e84 100644 --- a/pkg/workflow/template_fuzz_test.go +++ b/pkg/workflow/template_fuzz_test.go @@ -83,12 +83,13 @@ func FuzzWrapExpressionsInTemplateConditionals(f *testing.F) { f.Add("{{#if github.actor\x00}}content{{/if}}") // Very long expressions - longExpr := "{{#if " - for i := 0; i < 100; i++ { - longExpr += "github.event.pull_request.head.repo." + var longExpr strings.Builder + longExpr.WriteString("{{#if ") + for range 100 { + longExpr.WriteString("github.event.pull_request.head.repo.") } - longExpr += "name}}content{{/if}}" - f.Add(longExpr) + longExpr.WriteString("name}}content{{/if}}") + f.Add(longExpr.String()) // Complex markdown structures f.Add(`# Header diff --git a/pkg/workflow/template_injection_validation.go b/pkg/workflow/template_injection_validation.go index df600f5b54b..fd0e0aff16d 100644 --- a/pkg/workflow/template_injection_validation.go +++ b/pkg/workflow/template_injection_validation.go @@ -193,9 +193,9 @@ type TemplateInjectionViolation struct { // extractRunSnippet extracts a relevant snippet from the run block containing the expression func extractRunSnippet(runContent string, expression string) string { - lines := strings.Split(runContent, "\n") + lines := strings.SplitSeq(runContent, "\n") - for _, line := range lines { + for line := range lines { if strings.Contains(line, expression) { // Return the trimmed line containing the expression trimmed := strings.TrimSpace(line) diff --git a/pkg/workflow/template_injection_validation_fuzz_test.go b/pkg/workflow/template_injection_validation_fuzz_test.go index e34ef01b275..accfbd14f97 100644 --- a/pkg/workflow/template_injection_validation_fuzz_test.go +++ b/pkg/workflow/template_injection_validation_fuzz_test.go @@ -176,12 +176,13 @@ func FuzzValidateNoTemplateInjection(f *testing.F) { f.Add("\n\n\n") // Very long expressions - longExpression := "jobs:\n test:\n steps:\n - run: echo \"" - for i := 0; i < 50; i++ { - longExpression += "${{ github.event.issue.title }} " + var longExpression strings.Builder + longExpression.WriteString("jobs:\n test:\n steps:\n - run: echo \"") + for range 50 { + longExpression.WriteString("${{ github.event.issue.title }} ") } - longExpression += "\"" - f.Add(longExpression) + longExpression.WriteString("\"") + f.Add(longExpression.String()) // Unicode and special characters f.Add(`jobs: diff --git a/pkg/workflow/tools.go b/pkg/workflow/tools.go index a029ea03cb3..06021b1c0d9 100644 --- a/pkg/workflow/tools.go +++ b/pkg/workflow/tools.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "maps" "os" "strings" "time" @@ -62,9 +63,7 @@ func (c *Compiler) applyDefaults(data *WorkflowData, markdownPath string) error // Check if there are other events to merge if len(data.CommandOtherEvents) > 0 { // Merge other events into command events - for key, value := range data.CommandOtherEvents { - commandEventsMap[key] = value - } + maps.Copy(commandEventsMap, data.CommandOtherEvents) } // Convert merged events to YAML @@ -196,9 +195,7 @@ func (c *Compiler) mergeToolsAndMCPServers(topTools, mcpServers map[string]any, } // Add MCP servers to the tools collection - for serverName, serverConfig := range mcpServers { - result[serverName] = serverConfig - } + maps.Copy(result, mcpServers) // Merge included tools return c.MergeTools(result, includedTools) @@ -210,14 +207,12 @@ func mergeRuntimes(topRuntimes map[string]any, importedRuntimesJSON string) (map result := make(map[string]any) // Start with top-level runtimes - for id, config := range topRuntimes { - result[id] = config - } + maps.Copy(result, topRuntimes) // Merge imported runtimes (newline-separated JSON objects) if importedRuntimesJSON != "" { - lines := strings.Split(strings.TrimSpace(importedRuntimesJSON), "\n") - for _, line := range lines { + lines := strings.SplitSeq(strings.TrimSpace(importedRuntimesJSON), "\n") + for line := range lines { line = strings.TrimSpace(line) if line == "" || line == "{}" { continue @@ -229,9 +224,7 @@ func mergeRuntimes(topRuntimes map[string]any, importedRuntimesJSON string) (map } // Merge imported runtimes - later imports override earlier ones - for id, config := range importedRuntimes { - result[id] = config - } + maps.Copy(result, importedRuntimes) } } diff --git a/pkg/workflow/tools_parser.go b/pkg/workflow/tools_parser.go index 0cc33be62e7..d204e1f903e 100644 --- a/pkg/workflow/tools_parser.go +++ b/pkg/workflow/tools_parser.go @@ -52,6 +52,7 @@ package workflow import ( "fmt" + "maps" "github.com/github/gh-aw/pkg/logger" ) @@ -74,9 +75,7 @@ func NewTools(toolsMap map[string]any) *Tools { } // Copy raw map - for k, v := range toolsMap { - tools.raw[k] = v - } + maps.Copy(tools.raw, toolsMap) // Extract and parse known tools if val, exists := toolsMap["github"]; exists { diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index db86020a6b9..12c30a6bfdb 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -1,6 +1,8 @@ package workflow import ( + "maps" + "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/types" ) @@ -151,9 +153,7 @@ func mcpServerConfigToMap(config MCPServerConfig) map[string]any { } // Add custom fields (these override standard fields if there are conflicts) - for key, value := range config.CustomFields { - result[key] = value - } + maps.Copy(result, config.CustomFields) return result } diff --git a/pkg/workflow/trigger_parser.go b/pkg/workflow/trigger_parser.go index 68e773e3e67..d0abcf26403 100644 --- a/pkg/workflow/trigger_parser.go +++ b/pkg/workflow/trigger_parser.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "maps" "strings" "github.com/github/gh-aw/pkg/logger" @@ -103,9 +104,7 @@ func (ir *TriggerIR) ToYAMLMap() map[string]any { } // Add filters - for key, value := range ir.Filters { - eventConfig[key] = value - } + maps.Copy(eventConfig, ir.Filters) // If event config has content, add it; otherwise omit the event entirely for simple triggers if len(eventConfig) > 0 { @@ -118,9 +117,7 @@ func (ir *TriggerIR) ToYAMLMap() map[string]any { } // Add additional events - for event, config := range ir.AdditionalEvents { - result[event] = config - } + maps.Copy(result, ir.AdditionalEvents) return result } diff --git a/pkg/workflow/trigger_parser_fuzz_test.go b/pkg/workflow/trigger_parser_fuzz_test.go index 4084fcc76c6..cb44e6a3942 100644 --- a/pkg/workflow/trigger_parser_fuzz_test.go +++ b/pkg/workflow/trigger_parser_fuzz_test.go @@ -256,7 +256,7 @@ func FuzzTriggerIRToYAMLMap(f *testing.F) { // Parse types from comma-separated string var types []string if typesStr != "" { - for _, typ := range strings.Split(typesStr, ",") { + for typ := range strings.SplitSeq(typesStr, ",") { typ = strings.TrimSpace(typ) if typ != "" { types = append(types, typ) diff --git a/pkg/workflow/unified_prompt_creation_test.go b/pkg/workflow/unified_prompt_creation_test.go index d109551cc1c..5f3369f91a6 100644 --- a/pkg/workflow/unified_prompt_creation_test.go +++ b/pkg/workflow/unified_prompt_creation_test.go @@ -727,7 +727,7 @@ func TestGenerateUnifiedPromptCreationStep_LargeUserPromptChunking(t *testing.T) // Create many chunks to simulate large prompt userPromptChunks := make([]string, 10) - for i := 0; i < 10; i++ { + for i := range 10 { userPromptChunks[i] = fmt.Sprintf("# Section %d\n\nContent for section %d.", i+1, i+1) } @@ -737,14 +737,14 @@ func TestGenerateUnifiedPromptCreationStep_LargeUserPromptChunking(t *testing.T) output := yaml.String() // Verify all chunks are present - for i := 0; i < 10; i++ { + for i := range 10 { assert.Contains(t, output, fmt.Sprintf("# Section %d", i+1), "Should contain section %d", i+1) } // Verify chunks are in order positions := make([]int, 10) - for i := 0; i < 10; i++ { + for i := range 10 { positions[i] = strings.Index(output, fmt.Sprintf("# Section %d", i+1)) require.NotEqual(t, -1, positions[i], "Section %d should be present", i+1) } @@ -756,7 +756,7 @@ func TestGenerateUnifiedPromptCreationStep_LargeUserPromptChunking(t *testing.T) // Verify all chunks come after system tag closes systemClosePos := strings.Index(output, "") - for i := 0; i < 10; i++ { + for i := range 10 { assert.Less(t, systemClosePos, positions[i], "Section %d should come after system tag closes", i+1) } diff --git a/pkg/workflow/unified_prompt_step.go b/pkg/workflow/unified_prompt_step.go index 02a0e7816c9..a578e9b1ed3 100644 --- a/pkg/workflow/unified_prompt_step.go +++ b/pkg/workflow/unified_prompt_step.go @@ -99,8 +99,8 @@ func (c *Compiler) generateUnifiedPromptStep(yaml *strings.Builder, data *Workfl yaml.WriteString(" cat << '" + delimiter + "' >> \"$GH_AW_PROMPT\"\n") normalizedContent := normalizeLeadingWhitespace(section.Content) cleanedContent := removeConsecutiveEmptyLines(normalizedContent) - contentLines := strings.Split(cleanedContent, "\n") - for _, line := range contentLines { + contentLines := strings.SplitSeq(cleanedContent, "\n") + for line := range contentLines { yaml.WriteString(" " + line + "\n") } yaml.WriteString(" " + delimiter + "\n") @@ -127,8 +127,8 @@ func (c *Compiler) generateUnifiedPromptStep(yaml *strings.Builder, data *Workfl // Write content directly to open heredoc normalizedContent := normalizeLeadingWhitespace(section.Content) cleanedContent := removeConsecutiveEmptyLines(normalizedContent) - contentLines := strings.Split(cleanedContent, "\n") - for _, line := range contentLines { + contentLines := strings.SplitSeq(cleanedContent, "\n") + for line := range contentLines { yaml.WriteString(" " + line + "\n") } } @@ -503,8 +503,8 @@ func (c *Compiler) generateUnifiedPromptCreationStep(yaml *strings.Builder, buil yaml.WriteString(" cat << '" + delimiter + "'\n") normalizedContent := normalizeLeadingWhitespace(section.Content) cleanedContent := removeConsecutiveEmptyLines(normalizedContent) - contentLines := strings.Split(cleanedContent, "\n") - for _, line := range contentLines { + contentLines := strings.SplitSeq(cleanedContent, "\n") + for line := range contentLines { yaml.WriteString(" " + line + "\n") } yaml.WriteString(" " + delimiter + "\n") @@ -531,8 +531,8 @@ func (c *Compiler) generateUnifiedPromptCreationStep(yaml *strings.Builder, buil // Write content directly to open heredoc normalizedContent := normalizeLeadingWhitespace(section.Content) cleanedContent := removeConsecutiveEmptyLines(normalizedContent) - contentLines := strings.Split(cleanedContent, "\n") - for _, line := range contentLines { + contentLines := strings.SplitSeq(cleanedContent, "\n") + for line := range contentLines { yaml.WriteString(" " + line + "\n") } } @@ -583,8 +583,8 @@ func (c *Compiler) generateUnifiedPromptCreationStep(yaml *strings.Builder, buil // Each user prompt chunk is written as a separate heredoc yaml.WriteString(" cat << '" + delimiter + "'\n") - lines := strings.Split(chunk, "\n") - for _, line := range lines { + lines := strings.SplitSeq(chunk, "\n") + for line := range lines { yaml.WriteString(" ") yaml.WriteString(line) yaml.WriteByte('\n') diff --git a/pkg/workflow/unified_prompt_step_test.go b/pkg/workflow/unified_prompt_step_test.go index 0ca95ec5ab3..24fe02dcf91 100644 --- a/pkg/workflow/unified_prompt_step_test.go +++ b/pkg/workflow/unified_prompt_step_test.go @@ -453,7 +453,7 @@ func TestGenerateUnifiedPromptStep_EnvVarsSorted(t *testing.T) { // Skip the first entry which is GH_AW_PROMPT if len(envVarLines) > 0 { // Check that the remaining variables are in sorted order - for i := 0; i < len(envVarLines)-1; i++ { + for i := range len(envVarLines) - 1 { current := envVarLines[i] next := envVarLines[i+1] if current > next { diff --git a/pkg/workflow/validation_helpers.go b/pkg/workflow/validation_helpers.go index 9e5fd5b0cde..810cd75e958 100644 --- a/pkg/workflow/validation_helpers.go +++ b/pkg/workflow/validation_helpers.go @@ -29,6 +29,7 @@ package workflow import ( "fmt" + "slices" "strings" "github.com/github/gh-aw/pkg/logger" @@ -106,10 +107,8 @@ func ValidateMinLength(field, value string, minLength int) error { // ValidateInList validates that a value is in an allowed list func ValidateInList(field, value string, allowedValues []string) error { - for _, allowed := range allowedValues { - if value == allowed { - return nil - } + if slices.Contains(allowedValues, value) { + return nil } validationHelpersLog.Printf("List validation failed: field=%s, value=%s not in allowed list", field, value) diff --git a/pkg/workflow/wasm_golden_test.go b/pkg/workflow/wasm_golden_test.go index 0a17ae3387c..ea7b0b830cf 100644 --- a/pkg/workflow/wasm_golden_test.go +++ b/pkg/workflow/wasm_golden_test.go @@ -157,7 +157,7 @@ This workflow tests that compilation is deterministic. ` results := make([]string, 3) - for i := 0; i < 3; i++ { + for i := range 3 { compiler := NewCompiler( WithNoEmit(true), WithSkipValidation(true), diff --git a/pkg/workflow/yaml.go b/pkg/workflow/yaml.go index 3e419d96813..3db8550590a 100644 --- a/pkg/workflow/yaml.go +++ b/pkg/workflow/yaml.go @@ -85,6 +85,7 @@ package workflow import ( "regexp" + "slices" "sort" "strings" @@ -250,13 +251,7 @@ func OrderMapFields(data map[string]any, priorityFields []string) yaml.MapSlice var remainingKeys []string for key := range data { // Skip if it's already been added as a priority field - isPriority := false - for _, priorityField := range priorityFields { - if key == priorityField { - isPriority = true - break - } - } + isPriority := slices.Contains(priorityFields, key) if !isPriority { remainingKeys = append(remainingKeys, key) } diff --git a/scripts/lint_error_messages.go b/scripts/lint_error_messages.go index 0a3f6a6e996..314088930a3 100644 --- a/scripts/lint_error_messages.go +++ b/scripts/lint_error_messages.go @@ -330,10 +330,3 @@ func suggestImprovement(message string) string { return "Add 'Example:' section showing correct usage" } - -func max(a, b int) int { - if a > b { - return a - } - return b -}