lint: fix 8 targeted golint categories from lint-monster scan

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.*

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)))

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:]
 					}
 				}

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
 						}

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
 						}

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
 			}

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 {

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 + "  "

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
 						}

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

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

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) {

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
 			}

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
 			}

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

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
 		}

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
 						}

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
 			}

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

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
 		}
 	}

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
 			}

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
 						}

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
 						}

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 {

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:]
 		}
 	}