From 6f7660b7480ce92ee94453c210e4b3206e07ec7a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 06:23:44 +0000 Subject: [PATCH 1/7] Initial plan From a8eaaf748dfe272b1c5ea3b7f39fa7306e08a3f5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 06:52:07 +0000 Subject: [PATCH 2/7] lint: fix targeted golint findings (defer-in-loop, json.Unmarshal, map[string]bool, redundant .Error(), len(s)>0, strings.Split, sort.Slice, time.Sleep) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/actions_build_command.go | 2 +- pkg/cli/audit_report.go | 2 +- pkg/cli/codemod_agent_session.go | 2 +- pkg/cli/codemod_assign_to_agent.go | 4 +-- pkg/cli/codemod_bash_anonymous.go | 2 +- pkg/cli/codemod_cli_proxy_mode.go | 4 +-- pkg/cli/codemod_difc_proxy.go | 4 +-- pkg/cli/codemod_discussion_flag.go | 4 +-- pkg/cli/codemod_engine_env_secrets.go | 6 ++-- pkg/cli/codemod_engine_steps.go | 2 +- .../codemod_engine_to_top_level_helpers.go | 2 +- pkg/cli/codemod_expires_integer.go | 2 +- pkg/cli/codemod_mcp_mode_to_type.go | 2 +- pkg/cli/codemod_mcp_network.go | 8 ++--- pkg/cli/codemod_network_firewall.go | 2 +- pkg/cli/codemod_permissions_write.go | 2 +- pkg/cli/codemod_playwright_domains.go | 4 +-- ...emod_pull_request_target_checkout_false.go | 2 +- pkg/cli/codemod_run_install_scripts.go | 2 +- pkg/cli/codemod_serena_import.go | 2 +- pkg/cli/codemod_slash_command.go | 2 +- pkg/cli/codemod_upload_assets.go | 2 +- pkg/cli/codemod_user_rate_limit.go | 2 +- pkg/cli/generate_action_metadata_command.go | 6 ++-- pkg/cli/git.go | 8 ++--- pkg/cli/logs_awinfo_backward_compat_test.go | 8 +++-- pkg/cli/mcp_inspect.go | 5 ++- pkg/cli/mcp_inspect_mcp_scripts_server.go | 31 ++++++++++++++----- pkg/cli/poutine.go | 4 +-- pkg/cli/remove_command.go | 6 ++-- pkg/cli/runner_guard.go | 2 +- pkg/cli/workflows.go | 2 +- pkg/cli/yaml_frontmatter_utils.go | 2 +- pkg/workflow/action_cache.go | 4 +-- pkg/workflow/action_cache_test.go | 22 ++++++------- pkg/workflow/action_resolver.go | 24 +++++++------- pkg/workflow/action_resolver_test.go | 12 +++---- pkg/workflow/claude_tools.go | 2 +- pkg/workflow/codex_logs.go | 2 +- pkg/workflow/compiler_safe_outputs_job.go | 2 +- pkg/workflow/dependabot_test.go | 4 ++- pkg/workflow/frontmatter_error.go | 2 +- pkg/workflow/js.go | 2 +- pkg/workflow/lsp_manager.go | 6 ++-- pkg/workflow/model_alias_validation.go | 4 +-- pkg/workflow/pip.go | 2 +- pkg/workflow/runner_topology_validation.go | 10 +++--- pkg/workflow/safe_jobs.go | 4 +-- pkg/workflow/safe_outputs_messages_config.go | 4 +-- pkg/workflow/shell.go | 2 +- pkg/workflow/xml_comments.go | 2 +- 51 files changed, 137 insertions(+), 113 deletions(-) diff --git a/pkg/cli/actions_build_command.go b/pkg/cli/actions_build_command.go index ae7bea2bae6..4c8f193eec7 100644 --- a/pkg/cli/actions_build_command.go +++ b/pkg/cli/actions_build_command.go @@ -76,7 +76,7 @@ func ActionsValidateCommand() error { for _, actionName := range actionDirs { actionPath := filepath.Join(actionsDir, actionName) if err := validateActionYml(actionPath); err != nil { - fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("✗ %s/action.yml: %s", actionName, err.Error()))) + fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("✗ %s/action.yml: %s", actionName, err))) allValid = false } else { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" ✓ %s/action.yml is valid", actionName))) diff --git a/pkg/cli/audit_report.go b/pkg/cli/audit_report.go index ed14d79e7e5..1d198a2120b 100644 --- a/pkg/cli/audit_report.go +++ b/pkg/cli/audit_report.go @@ -838,7 +838,7 @@ func stripGHALogTimestamps(content string) string { if zPos+1 <= len(line) { line = line[zPos+1:] // Skip leading space after the timestamp - if len(line) > 0 && line[0] == ' ' { + if line != "" && line[0] == ' ' { line = line[1:] } } diff --git a/pkg/cli/codemod_agent_session.go b/pkg/cli/codemod_agent_session.go index e451a753afb..23ffddfe553 100644 --- a/pkg/cli/codemod_agent_session.go +++ b/pkg/cli/codemod_agent_session.go @@ -57,7 +57,7 @@ func getAgentTaskToAgentSessionCodemod() Codemod { } // Check if we've left the safe-outputs block - if inSafeOutputsBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inSafeOutputsBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, safeOutputsIndent) { inSafeOutputsBlock = false } diff --git a/pkg/cli/codemod_assign_to_agent.go b/pkg/cli/codemod_assign_to_agent.go index 7633b891aec..69427760c0a 100644 --- a/pkg/cli/codemod_assign_to_agent.go +++ b/pkg/cli/codemod_assign_to_agent.go @@ -70,7 +70,7 @@ func getAssignToAgentDefaultAgentCodemod() Codemod { } // Check if we've left the safe-outputs block - if inSafeOutputsBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inSafeOutputsBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, safeOutputsIndent) { inSafeOutputsBlock = false inAssignToAgentBlock = false @@ -86,7 +86,7 @@ func getAssignToAgentDefaultAgentCodemod() Codemod { } // Check if we've left the assign-to-agent block - if inAssignToAgentBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inAssignToAgentBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, assignToAgentIndent) { inAssignToAgentBlock = false } diff --git a/pkg/cli/codemod_bash_anonymous.go b/pkg/cli/codemod_bash_anonymous.go index 03ceab47104..1dce1cf502e 100644 --- a/pkg/cli/codemod_bash_anonymous.go +++ b/pkg/cli/codemod_bash_anonymous.go @@ -66,7 +66,7 @@ func replaceBashAnonymousWithTrue(lines []string) ([]string, bool) { } // Check if we've left the tools block - if inToolsBlock && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if inToolsBlock && trimmed != "" && !strings.HasPrefix(trimmed, "#") { if hasExitedBlock(line, toolsIndent) { inToolsBlock = false } diff --git a/pkg/cli/codemod_cli_proxy_mode.go b/pkg/cli/codemod_cli_proxy_mode.go index 937ff95315c..bc4edf2dd43 100644 --- a/pkg/cli/codemod_cli_proxy_mode.go +++ b/pkg/cli/codemod_cli_proxy_mode.go @@ -99,7 +99,7 @@ func addGitHubModeGhProxyToTools(lines []string) []string { toolsEnd := len(lines) for i := toolsLine + 1; i < len(lines); i++ { trimmed := strings.TrimSpace(lines[i]) - if len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") && hasExitedBlock(lines[i], toolsIndent) { + if trimmed != "" && !strings.HasPrefix(trimmed, "#") && hasExitedBlock(lines[i], toolsIndent) { toolsEnd = i break } @@ -128,7 +128,7 @@ func addGitHubModeGhProxyToTools(lines []string) []string { insertAt := githubLine + 1 for i := githubLine + 1; i < len(lines); i++ { trimmed := strings.TrimSpace(lines[i]) - if len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if trimmed != "" && !strings.HasPrefix(trimmed, "#") { if hasExitedBlock(lines[i], githubIndent) { insertAt = i } else { diff --git a/pkg/cli/codemod_difc_proxy.go b/pkg/cli/codemod_difc_proxy.go index 4a5d3f5a627..64c6e815266 100644 --- a/pkg/cli/codemod_difc_proxy.go +++ b/pkg/cli/codemod_difc_proxy.go @@ -128,7 +128,7 @@ func addIntegrityProxyFalseToToolsGitHub(lines []string) []string { } // Check if we've left the tools block - if inTools && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if inTools && trimmed != "" && !strings.HasPrefix(trimmed, "#") { if hasExitedBlock(line, toolsIndent) { inTools = false inGitHub = false @@ -144,7 +144,7 @@ func addIntegrityProxyFalseToToolsGitHub(lines []string) []string { } // Inside github block: inject integrity-proxy: false before the first sub-field - if inGitHub && !fieldInserted && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if inGitHub && !fieldInserted && trimmed != "" && !strings.HasPrefix(trimmed, "#") { if hasExitedBlock(line, githubIndent) { // Exited github block without seeing any sub-fields; use default indentation fieldIndent := githubIndent + " " diff --git a/pkg/cli/codemod_discussion_flag.go b/pkg/cli/codemod_discussion_flag.go index fa31b1b49d0..b62afc57faf 100644 --- a/pkg/cli/codemod_discussion_flag.go +++ b/pkg/cli/codemod_discussion_flag.go @@ -65,7 +65,7 @@ func getDiscussionFlagRemovalCodemod() Codemod { } // Check if we've left the safe-outputs block - if inSafeOutputsBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inSafeOutputsBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, safeOutputsIndent) { inSafeOutputsBlock = false inAddCommentBlock = false @@ -81,7 +81,7 @@ func getDiscussionFlagRemovalCodemod() Codemod { } // Check if we've left the add-comment block - if inAddCommentBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inAddCommentBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, addCommentIndent) { inAddCommentBlock = false } diff --git a/pkg/cli/codemod_engine_env_secrets.go b/pkg/cli/codemod_engine_env_secrets.go index be164f683e7..bc1bb00b4c8 100644 --- a/pkg/cli/codemod_engine_env_secrets.go +++ b/pkg/cli/codemod_engine_env_secrets.go @@ -175,7 +175,7 @@ func removeUnsafeEngineEnvKeys(lines []string, unsafeKeys map[string]struct { continue } - if inEngine && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") && len(indent) <= len(engineIndent) { + if inEngine && trimmed != "" && !strings.HasPrefix(trimmed, "#") && len(indent) <= len(engineIndent) { inEngine = false inEnv = false removingKey = false @@ -189,7 +189,7 @@ func removeUnsafeEngineEnvKeys(lines []string, unsafeKeys map[string]struct { continue } - if inEnv && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") && len(indent) <= len(envIndent) { + if inEnv && trimmed != "" && !strings.HasPrefix(trimmed, "#") && len(indent) <= len(envIndent) { inEnv = false removingKey = false } @@ -207,7 +207,7 @@ func removeUnsafeEngineEnvKeys(lines []string, unsafeKeys map[string]struct { removingKey = false } - if inEnv && !removingKey && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") && len(indent) > len(envIndent) { + if inEnv && !removingKey && trimmed != "" && !strings.HasPrefix(trimmed, "#") && len(indent) > len(envIndent) { key := parseYAMLMapKey(trimmed) if key != "" && setutil.Contains(unsafeKeys, key) { modified = true diff --git a/pkg/cli/codemod_engine_steps.go b/pkg/cli/codemod_engine_steps.go index 72b145c815a..bc5ac73c9b0 100644 --- a/pkg/cli/codemod_engine_steps.go +++ b/pkg/cli/codemod_engine_steps.go @@ -60,7 +60,7 @@ func getEngineStepsToTopLevelCodemod() Codemod { } // Check if we've exited the engine block - if inEngineBlock && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if inEngineBlock && trimmed != "" && !strings.HasPrefix(trimmed, "#") { lineIndent := getIndentation(line) if len(lineIndent) <= len(engineIndent) { inEngineBlock = false diff --git a/pkg/cli/codemod_engine_to_top_level_helpers.go b/pkg/cli/codemod_engine_to_top_level_helpers.go index 602d45ace2f..2ba6c2d7472 100644 --- a/pkg/cli/codemod_engine_to_top_level_helpers.go +++ b/pkg/cli/codemod_engine_to_top_level_helpers.go @@ -67,7 +67,7 @@ func migrateEngineFieldToTopLevel( engineIndent = getIndentation(line) continue } - if inEngineBlock && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") && len(getIndentation(line)) <= len(engineIndent) { + if inEngineBlock && trimmed != "" && !strings.HasPrefix(trimmed, "#") && len(getIndentation(line)) <= len(engineIndent) { inEngineBlock = false } if inEngineBlock && strings.HasPrefix(trimmed, engineFieldPrefix) { diff --git a/pkg/cli/codemod_expires_integer.go b/pkg/cli/codemod_expires_integer.go index bc0a0ff3693..fc73616f3e8 100644 --- a/pkg/cli/codemod_expires_integer.go +++ b/pkg/cli/codemod_expires_integer.go @@ -81,7 +81,7 @@ func convertExpiresIntegersToDayStrings(lines []string) ([]string, bool) { } // Check if we've left the safe-outputs block - if inSafeOutputsBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inSafeOutputsBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, safeOutputsIndent) { inSafeOutputsBlock = false } diff --git a/pkg/cli/codemod_mcp_mode_to_type.go b/pkg/cli/codemod_mcp_mode_to_type.go index c23a17164e9..673f111f143 100644 --- a/pkg/cli/codemod_mcp_mode_to_type.go +++ b/pkg/cli/codemod_mcp_mode_to_type.go @@ -72,7 +72,7 @@ func renameModeToTypeInMCPServers(lines []string) ([]string, bool) { } // Check if we've left mcp-servers block - if inMCPServers && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inMCPServers && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, mcpServersIndent) { inMCPServers = false } diff --git a/pkg/cli/codemod_mcp_network.go b/pkg/cli/codemod_mcp_network.go index a487eef6185..ba1423f7a83 100644 --- a/pkg/cli/codemod_mcp_network.go +++ b/pkg/cli/codemod_mcp_network.go @@ -171,7 +171,7 @@ func removeFieldFromMCPServer(lines []string, serverName string, fieldName strin } // Check if we've left mcp-servers block - if inMCPServers && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inMCPServers && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, mcpServersIndent) { inMCPServers = false inServerBlock = false @@ -187,7 +187,7 @@ func removeFieldFromMCPServer(lines []string, serverName string, fieldName strin } // Check if we've left the server block - if inServerBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inServerBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { currentIndent := getIndentation(line) // Exit if we're back at mcp-servers level or less if len(currentIndent) <= len(serverIndent) && strings.Contains(line, ":") { @@ -306,7 +306,7 @@ func updateNetworkAllowed(lines []string, domains []string) []string { } // Check if we've left network block - if inNetworkBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inNetworkBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, networkIndent) { inNetworkBlock = false inAllowedBlock = false @@ -376,7 +376,7 @@ func addAllowedToNetwork(lines []string, domains []string) []string { networkIndent = getIndentation(line) } - if inNetworkBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inNetworkBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, networkIndent) { // Found the end of network block insertIndex = i diff --git a/pkg/cli/codemod_network_firewall.go b/pkg/cli/codemod_network_firewall.go index 395c109d0c5..d587f03d87a 100644 --- a/pkg/cli/codemod_network_firewall.go +++ b/pkg/cli/codemod_network_firewall.go @@ -135,7 +135,7 @@ func insertSandboxAfterNetworkBlock(lines []string, sandboxLines []string) []str inNetworkBlock = true continue } - if inNetworkBlock && len(trimmed) > 0 && isTopLevelKey(line) { + if inNetworkBlock && trimmed != "" && isTopLevelKey(line) { insertIndex = i break } diff --git a/pkg/cli/codemod_permissions_write.go b/pkg/cli/codemod_permissions_write.go index 3cd4fe419b3..3ebd0161ed3 100644 --- a/pkg/cli/codemod_permissions_write.go +++ b/pkg/cli/codemod_permissions_write.go @@ -90,7 +90,7 @@ func getMigrateWritePermissionsToReadCodemod() Codemod { } // Check if we've left the permissions block - if inPermissionsBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inPermissionsBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, permissionsIndent) { inPermissionsBlock = false } diff --git a/pkg/cli/codemod_playwright_domains.go b/pkg/cli/codemod_playwright_domains.go index aeca0e99218..7ab2b62b56a 100644 --- a/pkg/cli/codemod_playwright_domains.go +++ b/pkg/cli/codemod_playwright_domains.go @@ -127,7 +127,7 @@ func removeFieldFromPlaywright(lines []string, fieldName string) ([]string, bool } // Check if we've left the tools block - if inTools && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if inTools && trimmed != "" && !strings.HasPrefix(trimmed, "#") { if hasExitedBlock(line, toolsIndent) { inTools = false inPlaywright = false @@ -143,7 +143,7 @@ func removeFieldFromPlaywright(lines []string, fieldName string) ([]string, bool } // Check if we've left the playwright block - if inPlaywright && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if inPlaywright && trimmed != "" && !strings.HasPrefix(trimmed, "#") { if hasExitedBlock(line, playwrightIndent) { inPlaywright = false } diff --git a/pkg/cli/codemod_pull_request_target_checkout_false.go b/pkg/cli/codemod_pull_request_target_checkout_false.go index 1cbe167964a..93a532fa273 100644 --- a/pkg/cli/codemod_pull_request_target_checkout_false.go +++ b/pkg/cli/codemod_pull_request_target_checkout_false.go @@ -141,7 +141,7 @@ func ensureCheckoutFalseForPullRequestTarget(lines []string) ([]string, bool) { if strings.TrimSpace(next) == "" { continue } - if len(next) > 0 && (next[0] == ' ' || next[0] == '\t') { + if next != "" && (next[0] == ' ' || next[0] == '\t') { return lines, false } break diff --git a/pkg/cli/codemod_run_install_scripts.go b/pkg/cli/codemod_run_install_scripts.go index 7d26765e0aa..2b57c47088e 100644 --- a/pkg/cli/codemod_run_install_scripts.go +++ b/pkg/cli/codemod_run_install_scripts.go @@ -171,7 +171,7 @@ func detectFrontmatterIndent(lines []string) string { continue } ind := getIndentation(line) - if len(ind) > 0 { + if ind != "" { return ind } } diff --git a/pkg/cli/codemod_serena_import.go b/pkg/cli/codemod_serena_import.go index 7fda9010933..d2dc10d0c85 100644 --- a/pkg/cli/codemod_serena_import.go +++ b/pkg/cli/codemod_serena_import.go @@ -190,7 +190,7 @@ func removeSerenaFromToolsList(lines []string) ([]string, bool) { } // Track block exit. - if inToolsBlock && len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if inToolsBlock && trimmed != "" && !strings.HasPrefix(trimmed, "#") { if hasExitedBlock(line, toolsIndent) { inToolsBlock = false } diff --git a/pkg/cli/codemod_slash_command.go b/pkg/cli/codemod_slash_command.go index 99db468e3f5..0e3581237e6 100644 --- a/pkg/cli/codemod_slash_command.go +++ b/pkg/cli/codemod_slash_command.go @@ -50,7 +50,7 @@ func getCommandToSlashCommandCodemod() Codemod { } // Check if we've left the on block - if inOnBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inOnBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, onIndent) { inOnBlock = false } diff --git a/pkg/cli/codemod_upload_assets.go b/pkg/cli/codemod_upload_assets.go index 0909bb3d940..1d8922de291 100644 --- a/pkg/cli/codemod_upload_assets.go +++ b/pkg/cli/codemod_upload_assets.go @@ -50,7 +50,7 @@ func getUploadAssetsCodemod() Codemod { } // Check if we've left the safe-outputs block - if inSafeOutputsBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inSafeOutputsBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, safeOutputsIndent) { inSafeOutputsBlock = false } diff --git a/pkg/cli/codemod_user_rate_limit.go b/pkg/cli/codemod_user_rate_limit.go index 59321254de5..2afbcc9f93f 100644 --- a/pkg/cli/codemod_user_rate_limit.go +++ b/pkg/cli/codemod_user_rate_limit.go @@ -81,7 +81,7 @@ func renameRateLimitToUserRateLimit(lines []string) ([]string, bool) { if inUserRateLimit { lineIndent := getIndentation(line) if isDescendant(lineIndent, userRateLimitIndent) { - if len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") && userRateLimitChildIndent == "" { + if trimmed != "" && !strings.HasPrefix(trimmed, "#") && userRateLimitChildIndent == "" { userRateLimitChildIndent = lineIndent } if userRateLimitChildIndent != "" && lineIndent != userRateLimitChildIndent { diff --git a/pkg/cli/generate_action_metadata_command.go b/pkg/cli/generate_action_metadata_command.go index 776992072c1..c60b1e26c2f 100644 --- a/pkg/cli/generate_action_metadata_command.go +++ b/pkg/cli/generate_action_metadata_command.go @@ -84,7 +84,7 @@ func GenerateActionMetadataCommand() error { contentBytes, err := os.ReadFile(jsPath) if err != nil { generateActionMetadataLog.Printf("Skipping %s: failed to read file: %v", filename, err) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("⚠ Skipping %s: %s", filename, err.Error()))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("⚠ Skipping %s: %s", filename, err))) continue } content := string(contentBytes) @@ -94,7 +94,7 @@ func GenerateActionMetadataCommand() error { // Extract metadata metadata, err := extractActionMetadata(filename, content) if err != nil { - fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("✗ Failed to extract metadata from %s: %s", filename, err.Error()))) + fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("✗ Failed to extract metadata from %s: %s", filename, err))) continue } @@ -205,7 +205,7 @@ func generateHumanReadableName(actionName string) string { // Replace underscores with spaces and capitalize words words := strings.Split(actionName, "_") for i, word := range words { - if len(word) > 0 { + if word != "" { words[i] = strings.ToUpper(word[:1]) + word[1:] } } diff --git a/pkg/cli/git.go b/pkg/cli/git.go index d0b860c9ad6..f2282f51e41 100644 --- a/pkg/cli/git.go +++ b/pkg/cli/git.go @@ -519,7 +519,7 @@ func hasPendingChanges() (bool, error) { if err != nil { return false, fmt.Errorf("failed to check git status: %w", err) } - return len(strings.TrimSpace(string(output))) > 0, nil + return strings.TrimSpace(string(output)) != "", nil } // checkCleanWorkingDirectory checks if there are uncommitted changes @@ -532,7 +532,7 @@ func checkCleanWorkingDirectory(verbose bool) error { return fmt.Errorf("failed to check git status: %w", err) } - if len(strings.TrimSpace(string(output))) > 0 { + if strings.TrimSpace(string(output)) != "" { return errors.New("working directory has uncommitted changes, please commit or stash them first") } @@ -590,7 +590,7 @@ func checkWorkflowFileStatus(workflowPath string) (*WorkflowFileStatus, error) { } statusOutput := string(output) // Don't trim - the leading space is significant! - if len(statusOutput) > 0 { + if statusOutput != "" { gitLog.Printf("Git status output: %q", statusOutput) // Parse the status line (format: XY filename) // X = index (staged) status, Y = working tree (unstaged) status @@ -634,7 +634,7 @@ func checkWorkflowFileStatus(workflowPath string) (*WorkflowFileStatus, error) { return status, nil // Ignore error, return current status } - if len(strings.TrimSpace(string(output))) > 0 { + if strings.TrimSpace(string(output)) != "" { status.HasUnpushedCommits = true gitLog.Print("File has unpushed commits") } diff --git a/pkg/cli/logs_awinfo_backward_compat_test.go b/pkg/cli/logs_awinfo_backward_compat_test.go index 5aaaa055b95..29647387743 100644 --- a/pkg/cli/logs_awinfo_backward_compat_test.go +++ b/pkg/cli/logs_awinfo_backward_compat_test.go @@ -170,7 +170,9 @@ func TestAwInfoMarshaling(t *testing.T) { if tt.shouldContainNew { // Check for awf_version in JSON var temp map[string]any - json.Unmarshal(data, &temp) + if err := json.Unmarshal(data, &temp); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } if _, exists := temp["awf_version"]; !exists { t.Errorf("%s: JSON should contain awf_version field, got: %s", tt.description, jsonStr) } @@ -179,7 +181,9 @@ func TestAwInfoMarshaling(t *testing.T) { if tt.shouldContainOld { // Check for firewall_version in JSON var temp map[string]any - json.Unmarshal(data, &temp) + if err := json.Unmarshal(data, &temp); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } 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/mcp_inspect.go b/pkg/cli/mcp_inspect.go index 51de62ad520..915f84967bb 100644 --- a/pkg/cli/mcp_inspect.go +++ b/pkg/cli/mcp_inspect.go @@ -135,7 +135,10 @@ func InspectWorkflowMCP(ctx context.Context, workflowFile string, serverFilter s fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to send interrupt signal: %v", err))) } // Wait a moment for graceful shutdown - time.Sleep(mcpScriptsServerShutdownDelay) + select { + case <-time.After(mcpScriptsServerShutdownDelay): + case <-ctx.Done(): + } // Attempt force kill (may fail if process already exited gracefully, which is fine) _ = mcpScriptsServerCmd.Process.Kill() } diff --git a/pkg/cli/mcp_inspect_mcp_scripts_server.go b/pkg/cli/mcp_inspect_mcp_scripts_server.go index c4ff4d55f71..599b7c257bf 100644 --- a/pkg/cli/mcp_inspect_mcp_scripts_server.go +++ b/pkg/cli/mcp_inspect_mcp_scripts_server.go @@ -45,6 +45,27 @@ func findAvailablePort(startPort int, verbose bool) int { var errMCPScriptsServerStartupTimeout = errors.New("mcp-scripts HTTP server failed to start within timeout") +// probeServer performs a single HTTP readiness probe against the given URL. +// It returns true if the server responded successfully. The response body is +// deferred-closed inside this helper so that neither the defer-in-loop nor the +// missing-body-close linters fire on the call site. +func probeServer(ctx context.Context, client *http.Client, url string) (bool, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return false, err + } + resp, err := client.Do(req) + if err != nil { + return false, nil //nolint:nilerr // probe failure is expected during startup + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + mcpInspectLog.Printf("Warning: failed to close response body: %v", closeErr) + } + }() + return true, nil +} + // waitForServerReady waits for the HTTP server to be ready by polling the endpoint. func waitForServerReady(ctx context.Context, port int, timeout time.Duration, verbose bool) error { deadline := time.Now().Add(timeout) @@ -59,18 +80,12 @@ func waitForServerReady(ctx context.Context, port int, timeout time.Duration, ve return ctx.Err() default: } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + ready, err := probeServer(ctx, client, url) if err != nil { mcpInspectLog.Printf("Failed to create request: %v", err) return err } - resp, err := client.Do(req) - if err == nil { - defer func() { - if closeErr := resp.Body.Close(); closeErr != nil { - mcpInspectLog.Printf("Warning: failed to close response body: %v", closeErr) - } - }() + if ready { if verbose { mcpInspectLog.Printf("Server is ready on port %d", port) } diff --git a/pkg/cli/poutine.go b/pkg/cli/poutine.go index 2fab3879650..5226b52c80a 100644 --- a/pkg/cli/poutine.go +++ b/pkg/cli/poutine.go @@ -282,7 +282,7 @@ func parseAndDisplayPoutineOutput(stdout, targetFile string, verbose bool) (int, trimmed := strings.TrimSpace(stdout) if !strings.HasPrefix(trimmed, "{") { // Non-JSON output, likely an error - if len(trimmed) > 0 { + if trimmed != "" { return 0, fmt.Errorf("unexpected poutine output format: %s", trimmed) } return 0, nil @@ -392,7 +392,7 @@ func parseAndDisplayPoutineOutputForDirectory(stdout string, verbose bool, gitRo trimmed := strings.TrimSpace(stdout) if !strings.HasPrefix(trimmed, "{") { // Non-JSON output, likely an error - if len(trimmed) > 0 { + if trimmed != "" { return 0, fmt.Errorf("unexpected poutine output format: %s", trimmed) } return 0, nil diff --git a/pkg/cli/remove_command.go b/pkg/cli/remove_command.go index 61bea65c8e6..634e3a98082 100644 --- a/pkg/cli/remove_command.go +++ b/pkg/cli/remove_command.go @@ -447,13 +447,13 @@ func parseIncludePath(line string) string { case strings.HasPrefix(trimmed, "{{#import"): rest = trimmed[len("{{#import"):] // Skip optional marker '?' - if len(rest) > 0 && rest[0] == '?' { + if rest != "" && rest[0] == '?' { rest = rest[1:] } // Skip optional whitespace, then an optional single colon, then optional whitespace // (mirrors the regex \s*:?\s* in IncludeDirectivePattern) rest = strings.TrimSpace(rest) - if len(rest) > 0 && rest[0] == ':' { + if rest != "" && rest[0] == ':' { rest = strings.TrimSpace(rest[1:]) } // Extract path up to closing "}}" and require only whitespace after it. @@ -475,7 +475,7 @@ func parseIncludePath(line string) string { } // Handle @include and @import: skip optional marker '?' - if len(rest) > 0 && rest[0] == '?' { + if rest != "" && rest[0] == '?' { rest = rest[1:] } // Require at least one whitespace character after the directive keyword diff --git a/pkg/cli/runner_guard.go b/pkg/cli/runner_guard.go index c8f3895ec92..879e16a40c3 100644 --- a/pkg/cli/runner_guard.go +++ b/pkg/cli/runner_guard.go @@ -149,7 +149,7 @@ func parseAndDisplayRunnerGuardOutput(stdout string, verbose bool, gitRoot strin trimmed := strings.TrimSpace(stdout) if !strings.HasPrefix(trimmed, "{") && !strings.HasPrefix(trimmed, "[") { - if len(trimmed) > 0 { + if trimmed != "" { return 0, fmt.Errorf("unexpected runner-guard output format: %s", trimmed) } return 0, nil diff --git a/pkg/cli/workflows.go b/pkg/cli/workflows.go index c3b0288a3a0..ed9de7da3c5 100644 --- a/pkg/cli/workflows.go +++ b/pkg/cli/workflows.go @@ -431,7 +431,7 @@ func extractWorkflowNameFromFile(filePath string) (title string, err error) { // Capitalize first letter of each word words := strings.Fields(baseName) for i, word := range words { - if len(word) > 0 { + if word != "" { words[i] = strings.ToUpper(word[:1]) + word[1:] } } diff --git a/pkg/cli/yaml_frontmatter_utils.go b/pkg/cli/yaml_frontmatter_utils.go index 8673ba0b9f0..c1ffb3bf70c 100644 --- a/pkg/cli/yaml_frontmatter_utils.go +++ b/pkg/cli/yaml_frontmatter_utils.go @@ -188,7 +188,7 @@ func removeFieldFromBlock(lines []string, fieldName string, parentBlock string) } // Check if we've left the parent block - if inParentBlock && len(trimmedLine) > 0 && !strings.HasPrefix(trimmedLine, "#") { + if inParentBlock && trimmedLine != "" && !strings.HasPrefix(trimmedLine, "#") { if hasExitedBlock(line, parentIndent) { inParentBlock = false } diff --git a/pkg/workflow/action_cache.go b/pkg/workflow/action_cache.go index a77311ec3ee..076e1dc0eea 100644 --- a/pkg/workflow/action_cache.go +++ b/pkg/workflow/action_cache.go @@ -104,7 +104,7 @@ func (c *ActionCache) DeleteContainerPin(image string) { // This is used to keep actions-lock.json a faithful reflection of what the // compiled workflows actually reference — entries for old action versions that // are no longer used by any workflow are removed. -func (c *ActionCache) PruneOrphanedEntries(referencedKeys map[string]bool) int { +func (c *ActionCache) PruneOrphanedEntries(referencedKeys map[string]struct{}) int { if len(referencedKeys) == 0 { return 0 } @@ -139,7 +139,7 @@ func (c *ActionCache) PruneOrphanedEntries(referencedKeys map[string]bool) int { pruned := 0 for key := range c.Entries { - if !referencedKeys[key] && !isCompilerGenerated(key) { + if _, ok := referencedKeys[key]; !ok && !isCompilerGenerated(key) { delete(c.Entries, key) c.dirty = true pruned++ diff --git a/pkg/workflow/action_cache_test.go b/pkg/workflow/action_cache_test.go index f0a4d8ecec0..87e7863a369 100644 --- a/pkg/workflow/action_cache_test.go +++ b/pkg/workflow/action_cache_test.go @@ -899,9 +899,9 @@ func TestPruneOrphanedEntries(t *testing.T) { } // Only v1.9.1 and checkout are referenced by compiled workflows. - referenced := map[string]bool{ - "microsoft/apm-action@v1.9.1": true, - "actions/checkout@v4": true, + referenced := map[string]struct{}{ + "microsoft/apm-action@v1.9.1": {}, + "actions/checkout@v4": {}, } pruned := cache.PruneOrphanedEntries(referenced) @@ -932,7 +932,7 @@ func TestPruneOrphanedEntries_EmptyReferenced(t *testing.T) { cache.Set("actions/checkout", "v4", "sha1") cache.Set("actions/setup-node", "v4", "sha2") - pruned := cache.PruneOrphanedEntries(map[string]bool{}) + pruned := cache.PruneOrphanedEntries(map[string]struct{}{}) if pruned != 0 { t.Errorf("Expected 0 pruned entries (empty referenced set is a no-op), got %d", pruned) @@ -951,9 +951,9 @@ func TestPruneOrphanedEntries_NoneOrphaned(t *testing.T) { cache.Set("actions/checkout", "v4", "sha1") cache.Set("actions/setup-node", "v4", "sha2") - referenced := map[string]bool{ - "actions/checkout@v4": true, - "actions/setup-node@v4": true, + referenced := map[string]struct{}{ + "actions/checkout@v4": {}, + "actions/setup-node@v4": {}, } pruned := cache.PruneOrphanedEntries(referenced) @@ -976,8 +976,8 @@ func TestPruneOrphanedEntries_AllOrphaned(t *testing.T) { cache.Set("cli/gh-extension-precompile", "v2.1.0", "sha2") // Referenced set is non-empty but contains neither of the cached entries. - referenced := map[string]bool{ - "some/other-action@v1": true, + referenced := map[string]struct{}{ + "some/other-action@v1": {}, } pruned := cache.PruneOrphanedEntries(referenced) @@ -1010,8 +1010,8 @@ func TestPruneOrphanedEntries_PreservesCompilerGenerated(t *testing.T) { } // Only reference microsoft/apm-action, but not the compiler-generated or runtime-managed ones - referenced := map[string]bool{ - "microsoft/apm-action@v1.7.2": true, + referenced := map[string]struct{}{ + "microsoft/apm-action@v1.7.2": {}, } pruned := cache.PruneOrphanedEntries(referenced) diff --git a/pkg/workflow/action_resolver.go b/pkg/workflow/action_resolver.go index 424b072eebc..634a77cea59 100644 --- a/pkg/workflow/action_resolver.go +++ b/pkg/workflow/action_resolver.go @@ -18,24 +18,24 @@ var resolverLog = logger.New("workflow:action_resolver") // ActionResolver handles resolving action SHAs using GitHub CLI type ActionResolver struct { cache *ActionCache - failedResolutions map[string]bool // tracks failed resolution attempts in current run (key: "repo@version") - usedCacheKeys map[string]bool // tracks cache keys that were hit or newly set during this run + failedResolutions map[string]struct{} // tracks failed resolution attempts in current run (key: "repo@version") + usedCacheKeys map[string]struct{} // tracks cache keys that were hit or newly set during this run } // NewActionResolver creates a new action resolver func NewActionResolver(cache *ActionCache) *ActionResolver { return &ActionResolver{ cache: cache, - failedResolutions: make(map[string]bool), - usedCacheKeys: make(map[string]bool), + failedResolutions: make(map[string]struct{}), + usedCacheKeys: make(map[string]struct{}), } } // GetUsedCacheKeys returns the set of cache keys (in "repo@version" format) that // were successfully resolved from the cache or written to the cache during this run. // These represent the action pins actually referenced by the compiled workflows. -func (r *ActionResolver) GetUsedCacheKeys() map[string]bool { - keys := make(map[string]bool, len(r.usedCacheKeys)) +func (r *ActionResolver) GetUsedCacheKeys() map[string]struct{} { + keys := make(map[string]struct{}, len(r.usedCacheKeys)) maps.Copy(keys, r.usedCacheKeys) return keys } @@ -43,7 +43,7 @@ func (r *ActionResolver) GetUsedCacheKeys() map[string]bool { // MarkCacheKeyAsUsed explicitly marks a cache key as used during this compilation run. // This is useful for compiler-generated actions that aren't resolved through ResolveSHA. func (r *ActionResolver) MarkCacheKeyAsUsed(cacheKey string) { - r.usedCacheKeys[cacheKey] = true + r.usedCacheKeys[cacheKey] = struct{}{} resolverLog.Printf("Marked cache key as used: %s", cacheKey) } @@ -76,8 +76,8 @@ func (r *ActionResolver) MarkCompilerGeneratedActionsAsUsed() { marked := 0 for _, repo := range compilerGeneratedRepos { if cacheKey, _, found := r.cache.FindAnyEntryForRepo(repo); found { - if !r.usedCacheKeys[cacheKey] { - r.usedCacheKeys[cacheKey] = true + if _, ok := r.usedCacheKeys[cacheKey]; !ok { + r.usedCacheKeys[cacheKey] = struct{}{} marked++ resolverLog.Printf("Marked compiler-generated action as used: %s", cacheKey) } @@ -96,10 +96,10 @@ func (r *ActionResolver) ResolveSHA(ctx context.Context, repo, version string) ( // Create a cache key for tracking failed resolutions and cache lookups. // Computed once here and reused below to avoid duplicate allocation. cacheKey := formatActionCacheKey(repo, version) - r.usedCacheKeys[cacheKey] = true + r.usedCacheKeys[cacheKey] = struct{}{} // Check if we've already failed to resolve this action in this run - if r.failedResolutions[cacheKey] { + if _, ok := r.failedResolutions[cacheKey]; ok { resolverLog.Printf("Skipping resolution for %s@%s: already failed in this run", repo, version) return "", fmt.Errorf("previously failed to resolve %s@%s in this compilation run", repo, version) } @@ -146,7 +146,7 @@ func (r *ActionResolver) ResolveSHA(ctx context.Context, repo, version string) ( if err != nil { resolverLog.Printf("Failed to resolve %s@%s: %v", repo, version, err) // Mark this resolution as failed for this compilation run - r.failedResolutions[cacheKey] = true + r.failedResolutions[cacheKey] = struct{}{} resolverLog.Printf("Marked %s as failed, will not retry in this run", cacheKey) return "", err } diff --git a/pkg/workflow/action_resolver_test.go b/pkg/workflow/action_resolver_test.go index bd1ef33a2d6..d3b6986a966 100644 --- a/pkg/workflow/action_resolver_test.go +++ b/pkg/workflow/action_resolver_test.go @@ -89,10 +89,10 @@ func TestActionResolverFailedResolutionCache(t *testing.T) { // Verify the failed resolution was tracked cacheKey := formatActionCacheKey(repo, version) - if !resolver.failedResolutions[cacheKey] { + if _, ok := resolver.failedResolutions[cacheKey]; !ok { t.Errorf("Expected failed resolution to be tracked for %s", cacheKey) } - if !resolver.GetUsedCacheKeys()[cacheKey] { + if _, ok := resolver.GetUsedCacheKeys()[cacheKey]; !ok { t.Errorf("Expected used cache keys to track attempted resolution for %s", cacheKey) } @@ -107,7 +107,7 @@ func TestActionResolverFailedResolutionCache(t *testing.T) { if !strings.Contains(err2.Error(), expectedErrMsg) { t.Errorf("Expected error message to contain %q, got: %v", expectedErrMsg, err2) } - if !resolver.GetUsedCacheKeys()[cacheKey] { + if _, ok := resolver.GetUsedCacheKeys()[cacheKey]; !ok { t.Errorf("Expected used cache keys to retain attempted resolution key %s", cacheKey) } } @@ -233,10 +233,10 @@ func TestActionResolverUsedCacheKeysOnCacheHit(t *testing.T) { } usedKeys := resolver.GetUsedCacheKeys() - if !usedKeys["owner/action-a@v1"] { + if _, ok := usedKeys["owner/action-a@v1"]; !ok { t.Error("Expected owner/action-a@v1 to be in used cache keys after a cache hit") } - if usedKeys["owner/action-b@v2"] { + if _, ok := usedKeys["owner/action-b@v2"]; ok { t.Error("Expected owner/action-b@v2 to be absent from used cache keys (never resolved)") } } @@ -254,7 +254,7 @@ func TestActionResolverGetUsedCacheKeysReturnsCopy(t *testing.T) { usedKeys := resolver.GetUsedCacheKeys() delete(usedKeys, "owner/action-a@v1") - if !resolver.GetUsedCacheKeys()["owner/action-a@v1"] { + if _, ok := resolver.GetUsedCacheKeys()["owner/action-a@v1"]; !ok { t.Error("Expected resolver used cache keys to be immutable via returned map") } } diff --git a/pkg/workflow/claude_tools.go b/pkg/workflow/claude_tools.go index 7f00c5620a8..bdfcc56742c 100644 --- a/pkg/workflow/claude_tools.go +++ b/pkg/workflow/claude_tools.go @@ -234,7 +234,7 @@ func hasBashWildcard(commands []any) bool { // isClaudeToolName uses the existing Claude naming convention heuristic: // valid Claude tool keys are expected to start with an uppercase ASCII letter. func isClaudeToolName(toolName string) bool { - return len(toolName) > 0 && toolName[0] >= 'A' && toolName[0] <= 'Z' + return toolName != "" && toolName[0] >= 'A' && toolName[0] <= 'Z' } func appendTopLevelClaudeTools(allowedTools []string, tools map[string]any, cacheMemoryConfig *CacheMemoryConfig) []string { diff --git a/pkg/workflow/codex_logs.go b/pkg/workflow/codex_logs.go index 8798bd9b243..b130bc9fc3e 100644 --- a/pkg/workflow/codex_logs.go +++ b/pkg/workflow/codex_logs.go @@ -14,7 +14,7 @@ var codexLogsLog = logger.New("workflow:codex_logs") // ParseLogMetrics implements engine-specific log parsing for Codex func (e *CodexEngine) ParseLogMetrics(logContent string, verbose bool) LogMetrics { - codexLogsLog.Printf("Parsing Codex log metrics: log_size=%d bytes, lines=%d", len(logContent), len(strings.Split(logContent, "\n"))) + codexLogsLog.Printf("Parsing Codex log metrics: log_size=%d bytes, lines=%d", len(logContent), strings.Count(logContent, "\n")+1) var metrics LogMetrics var totalTokenUsage int diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 317d9ea26e9..ca71801f7b9 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -848,7 +848,7 @@ func scriptNameToHandlerName(scriptName string) string { var sb strings.Builder sb.WriteString("handle") for _, part := range parts { - if len(part) > 0 { + if part != "" { sb.WriteString(strings.ToUpper(part[:1]) + part[1:]) } } diff --git a/pkg/workflow/dependabot_test.go b/pkg/workflow/dependabot_test.go index 0c5af851a9a..c2827ab7b7e 100644 --- a/pkg/workflow/dependabot_test.go +++ b/pkg/workflow/dependabot_test.go @@ -605,7 +605,9 @@ func TestGenerateDependabotManifests_WithDependencies(t *testing.T) { // Verify package.json content data, _ := os.ReadFile(packageJSONPath) var pkgJSON PackageJSON - json.Unmarshal(data, &pkgJSON) + if err := json.Unmarshal(data, &pkgJSON); err != nil { + t.Fatalf("Failed to unmarshal package.json: %v", err) + } if len(pkgJSON.Dependencies) != 1 { t.Errorf("expected 1 dependency, got %d", len(pkgJSON.Dependencies)) diff --git a/pkg/workflow/frontmatter_error.go b/pkg/workflow/frontmatter_error.go index ffa12bb0f60..f0ae5140f42 100644 --- a/pkg/workflow/frontmatter_error.go +++ b/pkg/workflow/frontmatter_error.go @@ -133,6 +133,6 @@ func (c *Compiler) createFrontmatterError(filePath, content string, err error, f // frontmatter start so the IDE navigates to the right file and section rather than // defaulting to line 1, col 1. frontmatterErrorLog.Printf("Using fallback error message: %v", err) - fallbackFmt := fmt.Sprintf("%s:%d:1: error: %s", filePath, frontmatterLineOffset, err.Error()) + fallbackFmt := fmt.Sprintf("%s:%d:1: error: %s", filePath, frontmatterLineOffset, err) return parser.NewFormattedParserError(fallbackFmt) } diff --git a/pkg/workflow/js.go b/pkg/workflow/js.go index dcd9a4e5792..8314d6dc86c 100644 --- a/pkg/workflow/js.go +++ b/pkg/workflow/js.go @@ -85,7 +85,7 @@ func removeJavaScriptComments(code string) string { // Remove the trailing newline we added resultStr := result.String() - if len(resultStr) > 0 && resultStr[len(resultStr)-1] == '\n' { + if resultStr != "" && resultStr[len(resultStr)-1] == '\n' { resultStr = resultStr[:len(resultStr)-1] } diff --git a/pkg/workflow/lsp_manager.go b/pkg/workflow/lsp_manager.go index 944fa08e7b5..f22e21653e4 100644 --- a/pkg/workflow/lsp_manager.go +++ b/pkg/workflow/lsp_manager.go @@ -223,7 +223,7 @@ func (m *LSPManager) RuntimeRequirements() []RuntimeRequirement { return nil } - seen := make(map[string]bool) + seen := make(map[string]struct{}) var result []RuntimeRequirement langs := make([]string, 0, len(m.servers)) @@ -237,10 +237,10 @@ func (m *LSPManager) RuntimeRequirements() []RuntimeRequirement { if !ok || spec.RuntimeID == "" { continue } - if seen[spec.RuntimeID] { + if _, ok := seen[spec.RuntimeID]; ok { continue } - seen[spec.RuntimeID] = true + seen[spec.RuntimeID] = struct{}{} runtime := findRuntimeByID(spec.RuntimeID) if runtime == nil { lspManagerLog.Printf("LSP language %q specifies unknown runtime ID %q; skipping runtime requirement", language, spec.RuntimeID) diff --git a/pkg/workflow/model_alias_validation.go b/pkg/workflow/model_alias_validation.go index c24649a0a67..1cab53f4993 100644 --- a/pkg/workflow/model_alias_validation.go +++ b/pkg/workflow/model_alias_validation.go @@ -193,12 +193,12 @@ func validateModelIdentifierStrings(identifiers []string, context string) []stri p, err := ParseModelIdentifier(id) if err != nil { // V-MAF-001 / V-MAF-006 - errs = append(errs, fmt.Sprintf("%s: %s", context, err.Error())) + errs = append(errs, fmt.Sprintf("%s: %s", context, err)) continue } // V-MAF-002 and V-MAF-003: validate known parameter values. if err := ValidateKnownParams(p.Params); err != nil { - errs = append(errs, fmt.Sprintf("%s: %s", context, err.Error())) + errs = append(errs, fmt.Sprintf("%s: %s", context, err)) } } return errs diff --git a/pkg/workflow/pip.go b/pkg/workflow/pip.go index 76ef0996ba3..b8384794885 100644 --- a/pkg/workflow/pip.go +++ b/pkg/workflow/pip.go @@ -63,7 +63,7 @@ func extractUvPackages(workflowData *WorkflowData) []string { // extractUvFromCommands extracts uv package names from command strings func extractUvFromCommands(commands string) []string { - pipLog.Printf("Extracting uv packages from commands: line_count=%d", len(strings.Split(commands, "\n"))) + pipLog.Printf("Extracting uv packages from commands: line_count=%d", strings.Count(commands, "\n")+1) var packages []string lines := strings.Split(commands, "\n") diff --git a/pkg/workflow/runner_topology_validation.go b/pkg/workflow/runner_topology_validation.go index 079778ff281..7f237d64b93 100644 --- a/pkg/workflow/runner_topology_validation.go +++ b/pkg/workflow/runner_topology_validation.go @@ -73,7 +73,7 @@ func validateArcDindRootless(workflowData *WorkflowData) error { // Returns a deduplicated list of violation descriptions found. func findRootRequiringPatterns(content string) []string { var violations []string - seen := map[string]bool{} + seen := map[string]struct{}{} for line := range strings.SplitSeq(content, "\n") { trimmed := strings.TrimSpace(line) @@ -86,12 +86,12 @@ func findRootRequiringPatterns(content string) []string { continue } - if containsSudoCommand(trimmed) && !seen["sudo"] { - seen["sudo"] = true + if _, ok := seen["sudo"]; containsSudoCommand(trimmed) && !ok { + seen["sudo"] = struct{}{} violations = append(violations, "sudo") } - if containsAptGetInstall(trimmed) && !seen["apt-get install"] { - seen["apt-get install"] = true + if _, ok := seen["apt-get install"]; containsAptGetInstall(trimmed) && !ok { + seen["apt-get install"] = struct{}{} violations = append(violations, "apt-get install") } } diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index 4b626aeecb8..b93b818babf 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -3,7 +3,7 @@ package workflow import ( "fmt" "maps" - "sort" + "slices" "strings" "github.com/github/gh-aw/pkg/constants" @@ -201,7 +201,7 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool for rawName, cfg := range data.SafeOutputs.Jobs { entries = append(entries, safeJobEntry{stringutil.NormalizeSafeOutputIdentifier(rawName), cfg}) } - sort.Slice(entries, func(i, j int) bool { return entries[i].normalizedName < entries[j].normalizedName }) + slices.SortFunc(entries, func(a, b safeJobEntry) int { return strings.Compare(a.normalizedName, b.normalizedName) }) for _, entry := range entries { jobConfig := entry.config diff --git a/pkg/workflow/safe_outputs_messages_config.go b/pkg/workflow/safe_outputs_messages_config.go index bf6c97fa853..0f52135fe14 100644 --- a/pkg/workflow/safe_outputs_messages_config.go +++ b/pkg/workflow/safe_outputs_messages_config.go @@ -89,7 +89,7 @@ func parseMentionsConfig(mentions any) *MentionsConfig { if str, ok := item.(string); ok { // Normalize username by removing '@' prefix if present normalized := str - if len(str) > 0 && str[0] == '@' { + if str != "" && str[0] == '@' { normalized = str[1:] safeOutputMessagesLog.Printf("Normalized mention '%s' to '%s'", str, normalized) } @@ -108,7 +108,7 @@ func parseMentionsConfig(mentions any) *MentionsConfig { if str, ok := item.(string); ok { // Normalize team slug by removing '@' prefix if present normalized := str - if len(str) > 0 && str[0] == '@' { + if str != "" && str[0] == '@' { normalized = str[1:] safeOutputMessagesLog.Printf("Normalized team mention '%s' to '%s'", str, normalized) } diff --git a/pkg/workflow/shell.go b/pkg/workflow/shell.go index 8abd15fc349..d3bd3e6f3c8 100644 --- a/pkg/workflow/shell.go +++ b/pkg/workflow/shell.go @@ -158,7 +158,7 @@ func buildDockerCommandWithExpandableVars(cmd string) string { var result strings.Builder result.WriteString("'") remaining := cmd - for len(remaining) > 0 { + for remaining != "" { // Find the next variable reference nextIdx := -1 nextVar := "" diff --git a/pkg/workflow/xml_comments.go b/pkg/workflow/xml_comments.go index 120be8bed78..71809307d84 100644 --- a/pkg/workflow/xml_comments.go +++ b/pkg/workflow/xml_comments.go @@ -11,7 +11,7 @@ var xmlCommentsLog = logger.New("workflow:xml_comments") // removeXMLComments removes XML comments () from markdown content // while preserving comments that appear within code blocks func removeXMLComments(content string) string { - xmlCommentsLog.Printf("Removing XML comments from content: %d lines", len(strings.Split(content, "\n"))) + xmlCommentsLog.Printf("Removing XML comments from content: %d lines", strings.Count(content, "\n")+1) // Track if we're inside a code block to avoid removing comments in code lines := strings.Split(content, "\n") From 384dbf7207267c37e414c6114944441b242007a1 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 3 Jul 2026 07:06:32 +0000 Subject: [PATCH 3/7] docs: add draft ADR-43126 for targeted golint enforcement --- ...3126-enforce-targeted-golint-categories.md | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 docs/adr/43126-enforce-targeted-golint-categories.md diff --git a/docs/adr/43126-enforce-targeted-golint-categories.md b/docs/adr/43126-enforce-targeted-golint-categories.md new file mode 100644 index 00000000000..dc3afb6e66d --- /dev/null +++ b/docs/adr/43126-enforce-targeted-golint-categories.md @@ -0,0 +1,50 @@ +# ADR-43126: Enforce Targeted Golint Categories from Lint-Monster Scan + +**Date**: 2026-07-03 +**Status**: Draft +**Deciders**: Unknown (automated lint enforcement via Copilot SWE agent) + +--- + +### Context + +The codebase accumulated 77 custom linter findings from a lint-monster scan across 8 narrow categories. Several categories flag correctness risks: deferred response body closes inside loops cause resource leaks on every iteration; discarded `json.Unmarshal` errors silently hide deserialization failures in tests; blocking `time.Sleep` in the MCP shutdown path ignores context cancellation and can stall graceful teardown. The remaining categories cover idiomatic style (`len(s) > 0` vs `s != ""`, redundant `.Error()` in format verbs, `sort.Slice` vs `slices.SortFunc`, `strings.Split` count anti-pattern). A separate PR tracks the larger `largefunc` backlog. The most consequential change is converting `map[string]bool` set patterns to `map[string]struct{}` in `action_resolver.go` and `action_cache.go`, which propagates through the public API (`GetUsedCacheKeys()`, `PruneOrphanedEntries()`). + +### Decision + +We will fix all 77 targeted lint findings across 8 categories in a single batch PR rather than deferring to individual feature PRs. We accept the breaking change to `map[string]struct{}` as the canonical set type for cache key tracking. The blocking `time.Sleep` in the MCP shutdown path is replaced with a `select` statement that respects context cancellation. A `probeServer()` helper is extracted from `waitForServerReady` to satisfy both the `deferinloop` and `httprespbodyclose` lint rules simultaneously. + +### Alternatives Considered + +#### Alternative 1: Fix Findings Incrementally in Feature PRs + +Address each lint category only when touching the related file for another purpose, rather than proactively in a dedicated cleanup PR. + +Why not chosen: Findings accumulate indefinitely when deferred. The lint-monster scan surfaced this technical debt specifically to be addressed. Deferring creates noise in future feature PR diffs and risks the backlog growing faster than it is resolved. + +#### Alternative 2: Suppress Lint Rules with `//nolint:` Directives + +Add per-site `//nolint:` comments to silence the 77 findings without changing code semantics. + +Why not chosen: Suppression defeats the purpose of the lint rules that flag real correctness issues (discarded errors, deferred closes inside loops, blocking sleeps). Suppressions would mask future regressions of the same class and require ongoing maintenance of the directive list. + +### Consequences + +#### Positive +- Eliminates 77 known lint findings across 8 categories, reducing CI noise and technical debt. +- `map[string]struct{}` reduces memory overhead for set operations compared to `map[string]bool` (no wasted bool storage per entry). +- Context-aware `select` in the MCP shutdown path allows clean cancellation, preventing the goroutine from blocking for the full shutdown delay when the context is already cancelled. +- Discarded `json.Unmarshal` errors in tests now cause explicit `t.Fatalf` failures rather than silent bad state continuing the test. + +#### Negative +- `GetUsedCacheKeys()` and `PruneOrphanedEntries()` have a breaking API change within the package boundary; all call sites must be updated to `map[string]struct{}`. +- The extracted `probeServer()` helper adds a new function to the file surface of `mcp_inspect_mcp_scripts_server.go`, slightly increasing the file's API footprint. + +#### Neutral +- 50+ occurrences of `len(s) > 0` replaced with `s != ""` — functionally equivalent, no behavioral change. +- `sort.Slice` replaced with `slices.SortFunc` using `strings.Compare` — semantically equivalent; avoids a name collision with the `cmp` variable present in the same package's test files. +- `strings.Split(...) count` anti-pattern replaced with `strings.Count(...)+1` — algorithmically equivalent, avoids an unnecessary slice allocation. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 67eed1b2cc6feb45d6db7735f2515c127270179b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 09:07:28 +0000 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20os.ReadFile=20error=20checks=20and=20time.NewTimer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/mcp_inspect.go | 4 +++- pkg/workflow/dependabot_test.go | 14 +++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pkg/cli/mcp_inspect.go b/pkg/cli/mcp_inspect.go index 915f84967bb..0d7e59ce3f5 100644 --- a/pkg/cli/mcp_inspect.go +++ b/pkg/cli/mcp_inspect.go @@ -135,9 +135,11 @@ func InspectWorkflowMCP(ctx context.Context, workflowFile string, serverFilter s fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to send interrupt signal: %v", err))) } // Wait a moment for graceful shutdown + shutdownTimer := time.NewTimer(mcpScriptsServerShutdownDelay) select { - case <-time.After(mcpScriptsServerShutdownDelay): + case <-shutdownTimer.C: case <-ctx.Done(): + shutdownTimer.Stop() } // Attempt force kill (may fail if process already exited gracefully, which is fine) _ = mcpScriptsServerCmd.Process.Kill() diff --git a/pkg/workflow/dependabot_test.go b/pkg/workflow/dependabot_test.go index c2827ab7b7e..a421491213e 100644 --- a/pkg/workflow/dependabot_test.go +++ b/pkg/workflow/dependabot_test.go @@ -337,9 +337,14 @@ func TestGenerateDependabotConfig_PreserveExisting(t *testing.T) { } // Verify file was preserved (no error means it was skipped) - data, _ := os.ReadFile(dependabotPath) + data, err := os.ReadFile(dependabotPath) + if err != nil { + t.Fatalf("Failed to read dependabot.yml: %v", err) + } var config DependabotConfig - yaml.Unmarshal(data, &config) + if err := yaml.Unmarshal(data, &config); err != nil { + t.Fatalf("Failed to unmarshal dependabot.yml: %v", err) + } if len(config.Updates) != 1 { t.Error("existing config should be preserved without force flag") } @@ -603,7 +608,10 @@ func TestGenerateDependabotManifests_WithDependencies(t *testing.T) { } // Verify package.json content - data, _ := os.ReadFile(packageJSONPath) + data, err := os.ReadFile(packageJSONPath) + if err != nil { + t.Fatalf("Failed to read package.json: %v", err) + } var pkgJSON PackageJSON if err := json.Unmarshal(data, &pkgJSON); err != nil { t.Fatalf("Failed to unmarshal package.json: %v", err) From 1bcb40b8ede1713311116833c62caadd118414a3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 09:11:59 +0000 Subject: [PATCH 5/7] fix: drain HTTP response body in probeServer; refactor runner_topology if-init pattern Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/mcp_inspect_mcp_scripts_server.go | 2 ++ pkg/workflow/runner_topology_validation.go | 16 ++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/cli/mcp_inspect_mcp_scripts_server.go b/pkg/cli/mcp_inspect_mcp_scripts_server.go index 599b7c257bf..29a96e554ac 100644 --- a/pkg/cli/mcp_inspect_mcp_scripts_server.go +++ b/pkg/cli/mcp_inspect_mcp_scripts_server.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "net" "net/http" "os" @@ -59,6 +60,7 @@ func probeServer(ctx context.Context, client *http.Client, url string) (bool, er return false, nil //nolint:nilerr // probe failure is expected during startup } defer func() { + _, _ = io.Copy(io.Discard, resp.Body) if closeErr := resp.Body.Close(); closeErr != nil { mcpInspectLog.Printf("Warning: failed to close response body: %v", closeErr) } diff --git a/pkg/workflow/runner_topology_validation.go b/pkg/workflow/runner_topology_validation.go index 7f237d64b93..f26e755d7ec 100644 --- a/pkg/workflow/runner_topology_validation.go +++ b/pkg/workflow/runner_topology_validation.go @@ -86,13 +86,17 @@ func findRootRequiringPatterns(content string) []string { continue } - if _, ok := seen["sudo"]; containsSudoCommand(trimmed) && !ok { - seen["sudo"] = struct{}{} - violations = append(violations, "sudo") + if containsSudoCommand(trimmed) { + if _, ok := seen["sudo"]; !ok { + seen["sudo"] = struct{}{} + violations = append(violations, "sudo") + } } - if _, ok := seen["apt-get install"]; containsAptGetInstall(trimmed) && !ok { - seen["apt-get install"] = struct{}{} - violations = append(violations, "apt-get install") + if containsAptGetInstall(trimmed) { + if _, ok := seen["apt-get install"]; !ok { + seen["apt-get install"] = struct{}{} + violations = append(violations, "apt-get install") + } } } From 09d4e4449b15e5fb3f8f8d7383e9c185e09c911d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Jul 2026 03:14:33 +0000 Subject: [PATCH 6/7] fix: merge origin/main, recompile workflows, fix stringscutprefix lint Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/arc_dind_artifacts.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/arc_dind_artifacts.go b/pkg/workflow/arc_dind_artifacts.go index b679240dbc1..ee566fc208e 100644 --- a/pkg/workflow/arc_dind_artifacts.go +++ b/pkg/workflow/arc_dind_artifacts.go @@ -15,9 +15,9 @@ import ( func rewriteTmpGhAwPathsForArcDind(paths []string) []string { result := make([]string, len(paths)) for i, p := range paths { - if strings.HasPrefix(p, constants.TmpGhAwDirSlash) { + if suffix, ok := strings.CutPrefix(p, constants.TmpGhAwDirSlash); ok { // /tmp/gh-aw/foo → ${{ runner.temp }}/gh-aw/foo - result[i] = constants.GhAwRootDir + "/" + strings.TrimPrefix(p, constants.TmpGhAwDirSlash) + result[i] = constants.GhAwRootDir + "/" + suffix } else if p == constants.TmpGhAwDir { result[i] = constants.GhAwRootDir } else { From f69cbfc6588b3920086c1accf8b65d8c7fe74906 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 4 Jul 2026 04:57:04 +0000 Subject: [PATCH 7/7] test: fix TestExecGHUsesConfiguredProcessEnvLookup flake in CI Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/github_cli_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/workflow/github_cli_test.go b/pkg/workflow/github_cli_test.go index ab113d41c57..264393d14be 100644 --- a/pkg/workflow/github_cli_test.go +++ b/pkg/workflow/github_cli_test.go @@ -149,6 +149,15 @@ func TestExecGHUsesConfiguredProcessEnvLookup(t *testing.T) { SetProcessEnvLookup(nil) }) + // Unset real token env vars so os.Environ() (used by SetGHHostEnv to seed + // cmd.Env) does not carry a token from the CI runner into the assertion. + for _, key := range []string{"GH_TOKEN", "GITHUB_TOKEN"} { + if prev, ok := os.LookupEnv(key); ok { + os.Unsetenv(key) + t.Cleanup(func() { os.Setenv(key, prev) }) + } + } + cmd := ExecGH("auth", "status") require.NotNil(t, cmd.Env) assert.False(t, slices.ContainsFunc(cmd.Env, func(e string) bool {