refactor: semantic function clustering — dedup linter helpers, drop stub files, rename outliers

pkg/actionpins/actionpins.go

@@ -254,11 +254,6 @@ func ExtractVersion(uses string) string {
 	return after
 }
 
-// isValidFullSHA checks if a string is a valid 40-character hexadecimal SHA.
-func isValidFullSHA(s string) bool {
-	return gitutil.IsValidFullSHA(s)
-}
-
 // findVersionBySHA returns the version string for a given SHA in the embedded pins
 // for the specified repo. Returns "" if no matching pin is found.
 func findVersionBySHA(repo, sha string) string {
@@ -311,7 +306,7 @@ func ResolveActionPin(actionRepo, version string, ctx *PinContext) (string, erro
 	}
 	actionPinsLog.Printf("Resolving action pin: repo=%s, version=%s, strict_mode=%t", actionRepo, version, ctx.StrictMode)
 
-	isAlreadySHA := isValidFullSHA(version)
+	isAlreadySHA := gitutil.IsValidFullSHA(version)
 
 	if ctx.Resolver != nil && !isAlreadySHA {
 		actionPinsLog.Printf("Attempting dynamic resolution for %s@%s", actionRepo, version)

pkg/cli/mcp_helpers.go

@@ -39,6 +39,9 @@ func GetBinaryPath() (string, error) {
 	return resolvedPath, nil
 }
 
+// boolPtr returns a pointer to the given bool value, used for optional *bool fields.
+func boolPtr(b bool) *bool { return &b }
+
 // logAndValidateBinaryPath determines the binary path, logs it, and validates it exists.
 // Returns the detected binary path and an error if the path cannot be determined or if the file doesn't exist.
 // This is a helper used by both runMCPServer and validateMCPServerConfiguration.

pkg/cli/update_container_pins.go

@@ -114,7 +114,7 @@ func UpdateContainerPins(ctx context.Context, workflowDir string, verbose bool)
 		}
 
 		// Attempt to resolve the digest without pulling.
-		digest, resolveErr := resolveContainerDigest(ctx, image, verbose)
+		digest, resolveErr := fetchContainerDigest(ctx, image, verbose)
 		if resolveErr != nil {
 			containerPinsLog.Printf("Failed to resolve digest for %s: %v", image, resolveErr)
 			failedImages = append(failedImages, imageFailure{image: image, reason: resolveErr.Error()})
@@ -221,14 +221,14 @@ func collectImagesFromLockFiles(workflowDir string) ([]string, error) {
 // while still bounding any hung Docker daemon or slow network connections.
 const dockerCmdTimeout = 60 * time.Second
 
-// resolveContainerDigest returns the SHA-256 content digest for the given image tag.
+// fetchContainerDigest returns the SHA-256 content digest for the given image tag.
 // It tries three strategies in order:
 //  1. "docker buildx imagetools inspect" (no pull, preferred — works when docker daemon is running)
 //  2. "crane digest" (no pull, no daemon — works in CI without Docker)
 //  3. "docker pull" + "docker inspect" (fallback that pulls the full image)
 //
 // Returns an error when all three strategies fail.
-func resolveContainerDigest(ctx context.Context, image string, verbose bool) (string, error) {
+func fetchContainerDigest(ctx context.Context, image string, verbose bool) (string, error) {
 	containerPinsLog.Printf("Resolving digest for container image: %s", image)
 
 	type strategy struct {

pkg/linters/errormessage/errormessage.go

@@ -5,14 +5,15 @@ package errormessage
 import (
 	"go/ast"
 	"go/token"
-	"go/types"
 	"path/filepath"
 	"strconv"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
+
+	"github.com/github/gh-aw/pkg/linters/internal/nolint"
 )
 
 var (
@@ -41,7 +42,7 @@ func run(pass *analysis.Pass) (any, error) {
 	}
 
 	insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-	noLintLinesByFile := buildNoLintLineIndex(pass)
+	noLintLinesByFile := nolint.BuildLineIndex(pass, "errormessage")
 
 	nodeFilter := []ast.Node{(*ast.CallExpr)(nil)}
 	insp.Preorder(nodeFilter, func(n ast.Node) {
@@ -54,7 +55,7 @@ func run(pass *analysis.Pass) (any, error) {
 		if !shouldCheckFile(pos.Filename, changed) || strings.HasSuffix(pos.Filename, "_test.go") {
 			return
 		}
-		if hasNoLintDirective(pos, noLintLinesByFile) {
+		if nolint.HasDirective(pos, noLintLinesByFile) {
 			return
 		}
 
@@ -242,63 +243,5 @@ func returnsError(pass *analysis.Pass, call *ast.CallExpr) bool {
 	if t == nil {
 		return false
 	}
-	return implementsError(t)
-}
-
-func implementsError(t types.Type) bool {
-	obj := types.Universe.Lookup("error")
-	if obj == nil {
-		return false
-	}
-	errIface, ok := obj.Type().Underlying().(*types.Interface)
-	if !ok {
-		return false
-	}
-
-	if types.Implements(t, errIface) {
-		return true
-	}
-	if p, ok := t.(*types.Pointer); ok {
-		return types.Implements(p, errIface)
-	}
-	return types.Implements(types.NewPointer(t), errIface)
-}
-
-func hasNoLintDirective(position token.Position, noLintLinesByFile map[string]map[int]struct{}) bool {
-	if position.Filename == "" {
-		return false
-	}
-
-	noLintLines := noLintLinesByFile[position.Filename]
-	if noLintLines == nil {
-		return false
-	}
-
-	_, sameLine := noLintLines[position.Line]
-	_, previousLine := noLintLines[position.Line-1]
-	return sameLine || previousLine
-}
-
-func buildNoLintLineIndex(pass *analysis.Pass) map[string]map[int]struct{} {
-	noLintLinesByFile := make(map[string]map[int]struct{}, len(pass.Files))
-	for _, file := range pass.Files {
-		filename := pass.Fset.PositionFor(file.Pos(), false).Filename
-		if filename == "" {
-			continue
-		}
-		for _, group := range file.Comments {
-			for _, comment := range group.List {
-				text := strings.TrimPrefix(comment.Text, "//")
-				if !strings.HasPrefix(text, "nolint:errormessage") && !strings.HasPrefix(text, "nolint:all") {
-					continue
-				}
-				line := pass.Fset.PositionFor(comment.Slash, false).Line
-				if noLintLinesByFile[filename] == nil {
-					noLintLinesByFile[filename] = make(map[int]struct{})
-				}
-				noLintLinesByFile[filename][line] = struct{}{}
-			}
-		}
-	}
-	return noLintLinesByFile
+	return nolint.ImplementsError(t)
 }

pkg/linters/errstringmatch/errstringmatch.go

@@ -12,6 +12,8 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
+
+	"github.com/github/gh-aw/pkg/linters/internal/nolint"
 )
 
 // Analyzer is the err-string-match analysis pass.
@@ -25,7 +27,7 @@ var Analyzer = &analysis.Analyzer{
 
 func run(pass *analysis.Pass) (any, error) {
 	insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-	noLintLinesByFile := buildNoLintLineIndex(pass)
+	noLintLinesByFile := nolint.BuildLineIndex(pass, "errstringmatch")
 
 	nodeFilter := []ast.Node{
 		(*ast.CallExpr)(nil),
@@ -58,7 +60,7 @@ func run(pass *analysis.Pass) (any, error) {
 		if !isStringLiteral(pass, outer.Args[1]) {
 			return
 		}
-		if hasNoLintDirective(position, noLintLinesByFile) {
+		if nolint.HasDirective(position, noLintLinesByFile) {
 			return
 		}
 
@@ -103,29 +105,7 @@ func isErrDotError(pass *analysis.Pass, expr ast.Expr) bool {
 	if t == nil {
 		return false
 	}
-	return implementsError(pass, t)
-}
-
-// implementsError reports whether t implements the built-in error interface.
-func implementsError(pass *analysis.Pass, t types.Type) bool {
-	errIface := pass.Pkg.Scope().Lookup("error")
-	if errIface == nil {
-		// Look up the universe scope.
-		obj := types.Universe.Lookup("error")
-		if obj == nil {
-			return false
-		}
-		iface, ok := obj.Type().Underlying().(*types.Interface)
-		if !ok {
-			return false
-		}
-		return types.Implements(t, iface) || types.Implements(types.NewPointer(t), iface)
-	}
-	iface, ok := errIface.Type().Underlying().(*types.Interface)
-	if !ok {
-		return false
-	}
-	return types.Implements(t, iface) || types.Implements(types.NewPointer(t), iface)
+	return nolint.ImplementsError(t)
 }
 
 // isStringLiteral returns true when expr is a string literal or untyped string constant.
@@ -142,42 +122,3 @@ func isStringLiteral(pass *analysis.Pass, expr ast.Expr) bool {
 	basic, ok := t.Underlying().(*types.Basic)
 	return ok && basic.Kind() == types.String
 }
-
-func hasNoLintDirective(position token.Position, noLintLinesByFile map[string]map[int]struct{}) bool {
-	if position.Filename == "" {
-		return false
-	}
-
-	noLintLines := noLintLinesByFile[position.Filename]
-	if noLintLines == nil {
-		return false
-	}
-
-	_, sameLine := noLintLines[position.Line]
-	_, previousLine := noLintLines[position.Line-1]
-	return sameLine || previousLine
-}
-
-func buildNoLintLineIndex(pass *analysis.Pass) map[string]map[int]struct{} {
-	noLintLinesByFile := make(map[string]map[int]struct{}, len(pass.Files))
-	for _, file := range pass.Files {
-		filename := pass.Fset.PositionFor(file.Pos(), false).Filename
-		if filename == "" {
-			continue
-		}
-		for _, group := range file.Comments {
-			for _, comment := range group.List {
-				text := strings.TrimPrefix(comment.Text, "//")
-				if !strings.HasPrefix(text, "nolint:errstringmatch") && !strings.HasPrefix(text, "nolint:all") {
-					continue
-				}
-				line := pass.Fset.PositionFor(comment.Slash, false).Line
-				if noLintLinesByFile[filename] == nil {
-					noLintLinesByFile[filename] = make(map[int]struct{})
-				}
-				noLintLinesByFile[filename][line] = struct{}{}
-			}
-		}
-	}
-	return noLintLinesByFile
-}

pkg/linters/internal/nolint/nolint.go

@@ -0,0 +1,77 @@
+// Package nolint provides shared helpers for nolint-directive detection
+// used by linters within pkg/linters.
+package nolint
+
+import (
+	"go/token"
+	"go/types"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// HasDirective reports whether the given source position is covered by a
+// suppression directive for linterName (or "nolint:all").  Both same-line and
+// previous-line directives are recognised, matching golangci-lint behaviour.
+func HasDirective(position token.Position, idx map[string]map[int]struct{}) bool {
+	if position.Filename == "" {
+		return false
+	}
+
+	noLintLines := idx[position.Filename]
+	if noLintLines == nil {
+		return false
+	}
+
+	_, sameLine := noLintLines[position.Line]
+	_, previousLine := noLintLines[position.Line-1]
+	return sameLine || previousLine
+}
+
+// BuildLineIndex scans all comments in the analysis pass and returns a map
+// from filename → set of line numbers that carry a nolint directive for
+// linterName (e.g. "errstringmatch") or "all".
+func BuildLineIndex(pass *analysis.Pass, linterName string) map[string]map[int]struct{} {
+	prefix := "nolint:" + linterName
+	noLintLinesByFile := make(map[string]map[int]struct{}, len(pass.Files))
+	for _, file := range pass.Files {
+		filename := pass.Fset.PositionFor(file.Pos(), false).Filename
+		if filename == "" {
+			continue
+		}
+		for _, group := range file.Comments {
+			for _, comment := range group.List {
+				text := strings.TrimPrefix(comment.Text, "//")
+				if !strings.HasPrefix(text, prefix) && !strings.HasPrefix(text, "nolint:all") {
+					continue
+				}
+				line := pass.Fset.PositionFor(comment.Slash, false).Line
+				if noLintLinesByFile[filename] == nil {
+					noLintLinesByFile[filename] = make(map[int]struct{})
+				}
+				noLintLinesByFile[filename][line] = struct{}{}
+			}
+		}
+	}
+	return noLintLinesByFile
+}
+
+// ImplementsError reports whether t implements the built-in error interface.
+func ImplementsError(t types.Type) bool {
+	obj := types.Universe.Lookup("error")
+	if obj == nil {
+		return false
+	}
+	errIface, ok := obj.Type().Underlying().(*types.Interface)
+	if !ok {
+		return false
+	}
+
+	if types.Implements(t, errIface) {
+		return true
+	}
+	if p, ok := t.(*types.Pointer); ok {
+		return types.Implements(p, errIface)
+	}
+	return types.Implements(types.NewPointer(t), errIface)
+}

pkg/workflow/awf_helpers.go

@@ -563,7 +563,7 @@ func buildAWFImageTagWithDigests(imageTag string, workflowData *WorkflowData) st
 
 	parts := []string{imageTag}
 	for _, spec := range specs {
-		digest := resolveContainerDigest(spec.image, workflowData)
+		digest := lookupContainerDigest(spec.image, workflowData)
 		if digest == "" {
 			continue
 		}
@@ -576,9 +576,9 @@ func buildAWFImageTagWithDigests(imageTag string, workflowData *WorkflowData) st
 	return strings.Join(parts, ",")
 }
 
-// resolveContainerDigest resolves a container image digest from cache first, then
+// lookupContainerDigest resolves a container image digest from cache first, then
 // falls back to embedded container pins.
-func resolveContainerDigest(image string, workflowData *WorkflowData) string {
+func lookupContainerDigest(image string, workflowData *WorkflowData) string {
 	var cache *ActionCache
 	if workflowData != nil {
 		cache = workflowData.ActionCache