Enable v2 linters: modernize, gocritic, gosec, unconvert (#7447)

Author: Copilot

.golangci.yml

@@ -5,6 +5,11 @@ linters:
   enable:
     - misspell
     - revive
+    - modernize      # Modern Go patterns (NEW in v2.6.0)
+    - gocritic       # Comprehensive opinionated linter
+    - gosec          # Security-focused
+    # godot linter disabled - too pedantic about comment punctuation
+    - unconvert      # Remove unnecessary conversions
   settings:
     errcheck:
       exclude-functions:
@@ -17,6 +22,24 @@ linters:
         - os.Remove
         - os.RemoveAll
         - os.WriteFile
+    gocritic:
+      disabled-checks:
+        - ifElseChain      # else-if chains are often clearer than switches
+        - singleCaseSwitch # Single case switches can be intentional for consistency
+        - appendAssign     # Appending to different variable is often intentional
+        - unlambda         # Explicit lambdas can be clearer than direct function refs
+        - elseif           # else-if pattern is acceptable
+        - assignOp         # Long form assignment can be clearer
+        - argOrder         # False positives on string contains
+        - dupBranchBody    # Duplicate branches can be intentional for clarity
+    gosec:
+      excludes:
+        - G101 # Ignore "Potential hardcoded credentials" - often false positives
+        - G602 # Ignore "slice bounds check" - handled by runtime
+        - G115 # Ignore "integer overflow conversion" - acceptable in most cases
+      config:
+        G204: "0644" # Allow common file permissions in tests
+        G306: "0644" # Allow common file permissions
   exclusions:
     generated: lax
     presets:
@@ -46,6 +69,130 @@ linters:
       - linters:
           - revive
         text: exported
+      - linters:
+          - gosec
+        text: "G204" # Allow exec.Command in controlled contexts
+        path: pkg/awmg/gateway\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow docker commands in actionlint
+        path: pkg/cli/actionlint\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow git commands in remote_fetch
+        path: pkg/parser/remote_fetch\.go
+      - linters:
+          - gosec
+        text: "G404" # Allow math/rand for non-crypto purposes
+        path: pkg/cli/(add_command|update_git)\.go
+      - linters:
+          - gosec
+        text: "G306" # Allow 0644 permissions in test files
+        path: _test\.go
+      - linters:
+          - gosec
+        text: "G305" # Allow file path operations in logs_download
+        path: pkg/cli/logs_download\.go
+      - linters:
+          - gosec
+        text: "G110" # Allow decompression in logs_download
+        path: pkg/cli/logs_download\.go
+      - linters:
+          - gocritic
+        text: "deprecatedComment" # Allow existing deprecated comment format
+      - linters:
+          - gocritic
+        text: "commentFormatting" # Allow commented out code
+      - linters:
+          - gocritic
+        text: "badCall" # filepath.Join with 1 arg is acceptable
+      - linters:
+          - godot
+        path: _test\.go # Don't require periods in test comments
+      - linters:
+          - modernize
+        text: "omitzero" # omitzero is acceptable for struct tags
+      - linters:
+          - modernize
+        text: "mapsloop" # maps.Copy requires Go 1.21+, keep compatible
+      - linters:
+          - modernize
+        text: "bloop" # b.Loop() is new, keep compatible with older Go
+      - linters:
+          - modernize
+        text: "minmax" # min/max builtins are Go 1.21+, keep compatible
+      - linters:
+          - modernize
+        text: "forvar" # Copying loop variable is sometimes clearer
+      - linters:
+          - modernize
+        text: "plusbuild" # Keep build constraint for compatibility
+        path: shell_backslash_integration_test\.go
+      - linters:
+          - modernize
+        text: "any" # Keep interface{} for clarity in schema tests
+        path: schema_strict_documentation_test\.go
+      - linters:
+          - modernize
+        text: "any" # Keep interface{} for clarity in tests
+        path: logs_awinfo_backward_compat_test\.go
+      - linters:
+          - modernize
+        text: "rangeint" # Keep traditional loops for compatibility
+      - linters:
+          - modernize
+        text: "stringsseq" # SplitSeq requires Go 1.23+, keep compatible
+      - linters:
+          - modernize
+        text: "slicescontains" # slices.Contains requires Go 1.21+
+      - linters:
+          - modernize
+        text: "stringscutprefix" # strings.Cut* requires Go 1.20+
+      - linters:
+          - modernize
+        text: "stringsbuilder" # Minor optimization, acceptable pattern
+      - linters:
+          - modernize
+        text: "reflecttypefor" # TypeFor requires Go 1.22+
+      - linters:
+          - unconvert
+        path: _test\.go # Allow explicit conversions in tests for clarity
+      - linters:
+          - gosec
+        text: "G204" # Allow git commands in download_workflow
+        path: pkg/cli/download_workflow\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow exec.Command with config in mcp_inspect
+        path: pkg/cli/mcp_inspect\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow exec.Command with config in mcp_inspect_mcp
+        path: pkg/cli/mcp_inspect_mcp\.go
+      - linters:
+          - gosec
+        text: "G306" # 0755 is correct permission for executable script
+        path: pkg/cli/mcp_inspect\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow docker commands in poutine
+        path: pkg/cli/poutine\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow node command in tests
+        path: pkg/workflow/js_comments_test\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow npx command in integration tests
+        path: pkg/workflow/playwright_mcp_integration_test\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow exec of binary in status tests
+        path: pkg/cli/status_command_test\.go
+      - linters:
+          - gosec
+        text: "G204" # Allow docker commands in zizmor
+        path: pkg/cli/zizmor\.go
     paths:
       - third_party$
       - builtin$

cmd/awmg/main.go

@@ -8,7 +8,7 @@ import (
 	"github.com/githubnext/gh-aw/pkg/console"
 )
 
-// Build-time variables
+// Build-time variables.
 var (
 	version = "dev"
 )

pkg/awmg/gateway.go

@@ -20,26 +20,26 @@ import (
 
 var gatewayLog = logger.New("awmg:gateway")
 
-// version is set by the main package
+// version is set by the main package.
 var version = "dev"
 
-// SetVersionInfo sets the version information for the awmg package
+// SetVersionInfo sets the version information for the awmg package.
 func SetVersionInfo(v string) {
 	version = v
 }
 
-// GetVersion returns the current version
+// GetVersion returns the current version.
 func GetVersion() string {
 	return version
 }
 
-// MCPGatewayConfig represents the configuration for the MCP gateway
+// MCPGatewayConfig represents the configuration for the MCP gateway.
 type MCPGatewayConfig struct {
 	MCPServers map[string]MCPServerConfig `json:"mcpServers"`
 	Gateway    GatewaySettings            `json:"gateway,omitempty"`
 }
 
-// MCPServerConfig represents configuration for a single MCP server
+// MCPServerConfig represents configuration for a single MCP server.
 type MCPServerConfig struct {
 	Command   string            `json:"command,omitempty"`
 	Args      []string          `json:"args,omitempty"`
@@ -48,7 +48,7 @@ type MCPServerConfig struct {
 	Container string            `json:"container,omitempty"`
 }
 
-// GatewaySettings represents gateway-specific settings
+// GatewaySettings represents gateway-specific settings.
 type GatewaySettings struct {
 	Port   int    `json:"port,omitempty"`
 	APIKey string `json:"apiKey,omitempty"`

pkg/campaign/template.go

@@ -20,7 +20,7 @@ var projectUpdateInstructionsTemplate string
 //go:embed prompts/closing_instructions.md
 var closingInstructionsTemplate string
 
-// CampaignPromptData holds data for rendering campaign orchestrator prompts
+// CampaignPromptData holds data for rendering campaign orchestrator prompts.
 type CampaignPromptData struct {
 	// ProjectURL is the GitHub Project URL
 	ProjectURL string
@@ -38,7 +38,7 @@ type CampaignPromptData struct {
 	MaxDiscoveryPagesPerRun int
 }
 
-// renderTemplate renders a template string with the given data
+// renderTemplate renders a template string with the given data.
 func renderTemplate(tmplStr string, data CampaignPromptData) (string, error) {
 	// Create custom template functions for Handlebars-style conditionals
 	funcMap := template.FuncMap{
@@ -66,7 +66,7 @@ func renderTemplate(tmplStr string, data CampaignPromptData) (string, error) {
 	return buf.String(), nil
 }
 
-// RenderOrchestratorInstructions renders the orchestrator instructions with the given data
+// RenderOrchestratorInstructions renders the orchestrator instructions with the given data.
 func RenderOrchestratorInstructions(data CampaignPromptData) string {
 	result, err := renderTemplate(orchestratorInstructionsTemplate, data)
 	if err != nil {

pkg/cli/fileutil/fileutil.go

@@ -6,7 +6,7 @@ import (
 	"path/filepath"
 )
 
-// FileExists checks if a file exists and is not a directory
+// FileExists checks if a file exists and is not a directory.
 func FileExists(path string) bool {
 	info, err := os.Stat(path)
 	if err != nil {
@@ -15,7 +15,7 @@ func FileExists(path string) bool {
 	return !info.IsDir()
 }
 
-// DirExists checks if a directory exists
+// DirExists checks if a directory exists.
 func DirExists(path string) bool {
 	info, err := os.Stat(path)
 	if os.IsNotExist(err) {
@@ -24,7 +24,7 @@ func DirExists(path string) bool {
 	return info.IsDir()
 }
 
-// IsDirEmpty checks if a directory is empty
+// IsDirEmpty checks if a directory is empty.
 func IsDirEmpty(path string) bool {
 	files, err := os.ReadDir(path)
 	if err != nil {
@@ -33,7 +33,7 @@ func IsDirEmpty(path string) bool {
 	return len(files) == 0
 }
 
-// CopyFile copies a file from src to dst using buffered IO
+// CopyFile copies a file from src to dst using buffered IO.
 func CopyFile(src, dst string) error {
 	in, err := os.Open(src)
 	if err != nil {
@@ -53,7 +53,7 @@ func CopyFile(src, dst string) error {
 	return out.Sync()
 }
 
-// CalculateDirectorySize recursively calculates the total size of files in a directory
+// CalculateDirectorySize recursively calculates the total size of files in a directory.
 func CalculateDirectorySize(dirPath string) int64 {
 	var totalSize int64
 

pkg/constants/constants.go

@@ -5,7 +5,7 @@ import (
 	"time"
 )
 
-// CLIExtensionPrefix is the prefix used in user-facing output to refer to the CLI extension
+// CLIExtensionPrefix is the prefix used in user-facing output to refer to the CLI extension.
 const CLIExtensionPrefix = "gh aw"
 
 // Semantic types for measurements and identifiers
@@ -44,23 +44,24 @@ type LineLength int
 //	func InstallTool(name string, version Version) error { ... }
 type Version string
 
-// FeatureFlag represents a feature flag identifier
+// FeatureFlag represents a feature flag identifier.
 type FeatureFlag string
 
-// MaxExpressionLineLength is the maximum length for a single line expression before breaking into multiline
+// MaxExpressionLineLength is the maximum length for a single line expression before breaking into multiline.
 const MaxExpressionLineLength LineLength = 120
 
-// ExpressionBreakThreshold is the threshold for breaking long lines at logical points
+// ExpressionBreakThreshold is the threshold for breaking long lines at logical points.
 const ExpressionBreakThreshold LineLength = 100
 
-// DefaultMCPRegistryURL is the default MCP registry URL
+// DefaultMCPRegistryURL is the default MCP registry URL.
 const DefaultMCPRegistryURL = "https://api.mcp.github.com/v0"
 
-// DefaultClaudeCodeVersion is the default version of the Claude Code CLI
+// DefaultClaudeCodeVersion is the default version of the Claude Code CLI.
 const DefaultClaudeCodeVersion Version = "2.0.75"
 
-// DefaultCopilotVersion is the default version of the GitHub Copilot CLI
-// WARNING: UPGRADING COPILOT CLI REQUIRES A FULL INTEGRATION TEST RUN TO ENSURE COMPATIBILITY
+// DefaultCopilotVersion is the default version of the GitHub Copilot CLI.
+//
+// WARNING: UPGRADING COPILOT CLI REQUIRES A FULL INTEGRATION TEST RUN TO ENSURE COMPATIBILITY.
 const DefaultCopilotVersion Version = "0.0.372"
 
 // DefaultCopilotDetectionModel is the default model for the Copilot engine when used in the detection job

pkg/gitutil/gitutil.go

@@ -2,8 +2,8 @@ package gitutil
 
 import "strings"
 
-// IsAuthError checks if an error message indicates an authentication issue
-// This is used to detect when GitHub API calls fail due to missing or invalid credentials
+// IsAuthError checks if an error message indicates an authentication issue.
+// This is used to detect when GitHub API calls fail due to missing or invalid credentials.
 func IsAuthError(errMsg string) bool {
 	lowerMsg := strings.ToLower(errMsg)
 	return strings.Contains(lowerMsg, "gh_token") ||
@@ -15,8 +15,8 @@ func IsAuthError(errMsg string) bool {
 		strings.Contains(lowerMsg, "permission denied")
 }
 
-// IsHexString checks if a string contains only hexadecimal characters
-// This is used to validate Git commit SHAs and other hexadecimal identifiers
+// IsHexString checks if a string contains only hexadecimal characters.
+// This is used to validate Git commit SHAs and other hexadecimal identifiers.
 func IsHexString(s string) bool {
 	if len(s) == 0 {
 		return false

pkg/logger/error_formatting.go

@@ -5,19 +5,19 @@ import (
 	"strings"
 )
 
-// Pre-compiled regexes for performance (avoid recompiling in hot paths)
+// Pre-compiled regexes for performance (avoid recompiling in hot paths).
 var (
 	// Timestamp patterns for log cleanup
-	// Pattern 1: ISO 8601 with T or space separator (e.g., "2024-01-01T12:00:00.123Z " or "2024-01-01 12:00:00 ")
+	// Pattern 1: ISO 8601 with T or space separator (e.g., "2024-01-01T12:00:00.123Z " or "2024-01-01 12:00:00 ").
 	timestampPattern1 = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}[T\s]\d{2}:\d{2}:\d{2}(\.\d+)?([+-]\d{2}:\d{2}|Z)?\s*`)
-	// Pattern 2: Bracketed date-time (e.g., "[2024-01-01 12:00:00] ")
+	// Pattern 2: Bracketed date-time (e.g., "[2024-01-01 12:00:00] ").
 	timestampPattern2 = regexp.MustCompile(`^\[\d{4}-\d{2}-\d{2}\s+\d{2}:\d{2}:\d{2}\]\s*`)
-	// Pattern 3: Bracketed time only (e.g., "[12:00:00] ")
+	// Pattern 3: Bracketed time only (e.g., "[12:00:00] ").
 	timestampPattern3 = regexp.MustCompile(`^\[\d{2}:\d{2}:\d{2}\]\s+`)
-	// Pattern 4: Time only with optional milliseconds (e.g., "12:00:00.123 ")
+	// Pattern 4: Time only with optional milliseconds (e.g., "12:00:00.123 ").
 	timestampPattern4 = regexp.MustCompile(`^\d{2}:\d{2}:\d{2}(\.\d+)?\s+`)
 
-	// Log level pattern for message cleanup (case-insensitive)
+	// Log level pattern for message cleanup (case-insensitive).
 	logLevelPattern = regexp.MustCompile(`(?i)^\[?(ERROR|WARNING|WARN|INFO|DEBUG)\]?\s*[:-]?\s*`)
 )