fix(golang): reduce required co-package updates to minimal necessary set (#63)

* feat(golang): reduce required co-package updates to minimal necessary set

- Filter CheckTransitiveRequirements to only flag direct project deps;
  indirect deps are resolved automatically by Go's MVS and cannot cause
  API breakage in the project's own code
- Add FindVersionGroupPackages to detect tightly-coupled module ecosystems
  (e.g. all go.opentelemetry.io/otel/* packages) using module family prefix
  heuristics, covering both co-released and version-drifted siblings
- Add second API compat pass over discovered co-updates so community
  packages that import a co-updated dep (e.g. otelgrpc importing otel)
  are surfaced as API compat alerts

* feat(golang): find minimum compatible version for API compat alerts

When CheckAPICompatibilityWithCache flags a package (e.g. go-ldap/ldap/v3)
that imports a dep being updated (e.g. go-ntlmssp), walk the package's
version history to find the lowest release whose go.mod requires the
updated dep at >= the target version.

This converts API compat alerts from no-op suggestions (pkg@current) into
actionable version recommendations (pkg@min-compatible), directly addressing
build failures where a dep's API signature changed between versions.

* feat(golang): add API surface diff to detect genuine breaking changes

Add CheckAPIBreakingChanges which compares the exported API of a package
between two versions using golang.org/x/exp/apidiff. This provides a
precise signal for whether a dep version bump introduces incompatible
changes (e.g. ProcessChallenge gaining a fourth argument in go-ntlmssp),
as opposed to compatible additions (new functions, new fields).

The function type-checks both versions using golang.org/x/tools/go/packages
in isolated temp modules, then runs apidiff.Changes to classify each
exported symbol change as compatible or incompatible.

This can be used alongside findMinCompatibleVersion and
CheckAPICompatibilityWithCache to distinguish false-positive compat alerts
from confirmed breaking changes that require downstream packages to update.

* fix(golang): skip main module and detect unavailable package versions

- resolveAndFilterPackages now skips any package whose path matches the
  main module of the go.mod being analyzed, logging a warning instead of
  letting it reach gobump where it would fail with 'bumping the main
  module is not allowed'
- checkMissingTransitiveDeps applies the same guard as a second line of
  defence for co-updates discovered through transitive checks
- CheckAPIBreakingChanges now treats an unloadable new version (removed
  package or internally broken release) as a breaking change rather than
  returning an error, giving callers a clear signal not to bump to that
  version

* chore(golang): fix golangci-lint violations

- Add sentinel errors errPackageNotFound and errNoTypeInfo to replace
  dynamic fmt.Errorf calls in loadPackageTypes (err113)
- Wrap packages.Error with %w in loadPackageTypes (errorlint)
- Wrap defer os.RemoveAll in closure discarding return value (errcheck)
- Fix stdlib import grouping for go/types and errors (gci, gofumpt)
- Extract runCoUpdateAPICompatChecks helper from checkMissingTransitiveDeps
  to reduce cyclomatic complexity below threshold (gocyclo)

* refactor(golang): extract mainModulePath helper to remove duplication

* chore: go mod tidy

* refactor(golang): consolidate co-update detection into shared detectCoUpdates

Extract the co-update detection core from checkMissingTransitiveDeps into
a new detectCoUpdates function so both the update path and analyzer path
use identical logic.

Previously checkTransitiveRequirementsForStrategy in analyzer.go had its
own diverged implementation: it used the deprecated CheckAPICompatibility,
had no version-group ecosystem detection, no findMinCompatibleVersion, no
main module skip, and included indirect project deps in version checks.

Both paths now share: FindVersionGroupPackages for ecosystem grouping,
CheckAPICompatibilityWithCache with findMinCompatibleVersion for actionable
version recommendations, main module skip, direct-only dep filtering, and
the second API compat pass for co-update deps.

* fix(golang): apply main module skip in analyze path

RecommendStrategy was adding the main module to strategy.DirectUpdates
without checking whether the package being updated is the go.mod's own
module. The update path had this guard in resolveAndFilterPackages; now
both paths produce consistent output.

* fix(golang): surface API compat co-updates in all output types

When findMinCompatibleVersion identifies a concrete minimum version for
an API compat alert (e.g. otelgrpc@v0.68.0 when bumping otel to v1.43.0),
add it to strategy.DirectUpdates so it appears in JSON, YAML, and deps
file output — not just as a human-readable warning string.

Packages already being updated are skipped to avoid duplicates.

* chore(golang): add map capacity and eliminate redundant go.mod parse

- Set initial capacity on allMissingDeps and apiCompatibilityAlerts in
  detectCoUpdates to avoid realloc when co-updates match update count
- Parse go.mod once in RecommendStrategy and pass it to
  checkTransitiveRequirementsForStrategy to avoid reading the same file
  twice; fall back to parsing internally when caller passes nil

* docs(golang): document why GONOSUMCHECK is set in loadPackageTypes

* fix(golang): skip version group packages on different major version track

Packages like go.opentelemetry.io/otel/exporters/* use a v0.x cadence
while core otel uses v1.x. FindVersionGroupPackages was including them
in the version group and recommending them at the wrong target version
(e.g. v1.43.0 instead of v0.19.0).

Skip version group candidates whose current major differs from the target
major. The second-pass API compat check handles these packages correctly
via findMinCompatibleVersion, which finds the right v0.x version.

* test(golang): regression tests for cross-major version group handling

Add TestDetectCoUpdates_CrossMajorVersionGroupSkipped to guard against
recommending the wrong version for otel exporter packages. The otel
exporter family (go.opentelemetry.io/otel/exporters/*) uses v0.x while
core otel uses v1.x — the fix must find v0.19.0 via the API compat chain
rather than v1.43.0 from the version group.

Add a FindVersionGroupPackages test case documenting that the function
itself returns cross-major family members; the filter lives in
detectCoUpdates.

* fix(golang): exclude cross-major packages from version group in FindVersionGroupPackages

Move the cross-major version guard into FindVersionGroupPackages itself so
that all callers are protected, not just detectCoUpdates. Previously,
otel/exporters/prometheus@v0.60.0 would be included in the version group
when bumping core otel to v1.43.0, and the function would return it with
a target of v1.43.0 — a version that does not exist, causing go mod tidy
to fail.

The fix compares each candidate's semver major against the source package's
current version major and excludes any mismatch. The second-pass API compat
chain (findMinCompatibleVersion) then finds the correct v0.x version instead.

Removes the now-redundant duplicate guard that was in detectCoUpdates.

* fix(golang): actively resolve correct version for cross-major family packages

When FindVersionGroupPackages returns a package on a different major
version track (e.g. otel/exporters/prometheus v0.60.0 when targeting
otel v1.43.0), detectCoUpdates now calls findMinCompatibleVersion to
find the correct v0.x version (e.g. v0.65.0) rather than either
applying the wrong v1.x target or relying on the second-pass API compat
chain to catch it.

FindVersionGroupPackages returns all family members regardless of major
version — the caller is responsible for resolving the right version.

* chore(golang): fix log message and reason string for cross-major co-updates

- Move familyRoot declaration inside the cross-major branch where it is used
- Remove duplicate familyRoot argument from log message
- Use distinct Reason string for cross-major packages so it doesn't
  incorrectly say 'both at X' when the versions differ

Author: kbsteere

go.mod

@@ -13,7 +13,9 @@ require (
 	github.com/samber/lo v1.53.0
 	github.com/spf13/cobra v1.10.2
 	github.com/stretchr/testify v1.11.1
-	golang.org/x/mod v0.34.0
+	golang.org/x/exp v0.0.0-20231006140011-7918f672742d
+	golang.org/x/mod v0.35.0
+	golang.org/x/tools v0.44.0
 	k8s.io/apimachinery v0.35.3
 	sigs.k8s.io/release-utils v0.12.4
 )
@@ -40,9 +42,10 @@ require (
 	github.com/rivo/uniseg v0.4.7 // indirect
 	github.com/spf13/pflag v1.0.9 // indirect
 	github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
-	golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect
-	golang.org/x/sys v0.38.0 // indirect
+	golang.org/x/sync v0.20.0 // indirect
+	golang.org/x/sys v0.43.0 // indirect
 	golang.org/x/text v0.31.0 // indirect
+	golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated // indirect
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

go.sum

@@ -71,13 +71,21 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJu
 go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
 golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI=
 golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo=
-golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI=
-golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY=
+golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM=
+golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU=
+golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4=
+golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc=
-golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
+golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI=
+golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
 golang.org/x/text v0.31.0 h1:aC8ghyu4JhP8VojJ2lEHBnochRno1sgL6nEi9WGFGMM=
 golang.org/x/text v0.31.0/go.mod h1:tKRAlv61yKIjGGHX/4tP1LTbc13YSec1pxVEWXzfoeM=
+golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c=
+golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI=
+golang.org/x/tools/go/expect v0.1.0-deprecated h1:jY2C5HGYR5lqex3gEniOQL0r7Dq5+VGVgY1nudX5lXY=
+golang.org/x/tools/go/expect v0.1.0-deprecated/go.mod h1:eihoPOH+FgIqa3FpoTwguz/bVUSGBlGQU67vpBeOrBY=
+golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated h1:1h2MnaIAIXISqTFKdENegdpAgUXz6NrPEsbIeWaBRvM=
+golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated/go.mod h1:RVAQXBGNv1ib0J382/DPCRS/BPnsGebyM1Gj5VSDpG8=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=

pkg/languages/golang/analyzer.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 
 	"github.com/chainguard-dev/clog"
 	"github.com/chainguard-dev/omnibump/pkg/analyzer"
@@ -377,7 +378,23 @@ func (ga *GolangAnalyzer) RecommendStrategy(ctx context.Context, analysis *analy
 		AffectedDependencies: make(map[string][]string),
 	}
 
+	// Parse go.mod once here and pass it to checkTransitiveRequirementsForStrategy
+	// to avoid a second disk read for the same file.
+	var strategyModFile *modfile.File
+	mainModule := ""
+	if rootPath, ok := analysis.Metadata["moduleRoot"].(string); ok {
+		if mf, _, err := ParseGoModfile(filepath.Join(rootPath, "go.mod")); err == nil {
+			strategyModFile = mf
+			mainModule = mainModulePath(mf)
+		}
+	}
+
 	for _, dep := range deps {
+		if dep.Name == mainModule {
+			log.Warnf("Skipping %s: it is the main module of this go.mod and cannot be bumped as a dependency", dep.Name)
+			continue
+		}
+
 		if depInfo, exists := analysis.Dependencies[dep.Name]; exists {
 			// Check if this is a replaced dependency
 			if replaced, ok := depInfo.Metadata["replaced"].(bool); ok && replaced {
@@ -399,8 +416,9 @@ func (ga *GolangAnalyzer) RecommendStrategy(ctx context.Context, analysis *analy
 		log.Debugf("Will update %s to %s", dep.Name, dep.Version)
 	}
 
-	// Check transitive requirements for all packages being updated
-	ga.checkTransitiveRequirementsForStrategy(ctx, analysis, strategy)
+	// Check transitive requirements for all packages being updated.
+	// Pass the already-parsed modFile to avoid a redundant disk read.
+	ga.checkTransitiveRequirementsForStrategy(ctx, analysis, strategy, strategyModFile)
 
 	// Deduplicate DirectUpdates — the same package can be added from multiple paths
 	// (direct update, parent bump for an indirect dep, transitive co-update).
@@ -417,86 +435,36 @@ func (ga *GolangAnalyzer) checkTransitiveRequirementsForStrategy(
 	ctx context.Context,
 	analysis *analyzer.AnalysisResult,
 	strategy *analyzer.Strategy,
+	modFile *modfile.File,
 ) {
 	log := clog.FromContext(ctx)
 
-	// Get module root from analysis
-	modRoot := "."
-	if rootPath, ok := analysis.Metadata["moduleRoot"].(string); ok {
-		modRoot = rootPath
-	}
-
-	// Parse go.mod (if moduleRoot is set and file exists)
-	modFilePath := filepath.Join(modRoot, "go.mod")
-	modFile, _, err := ParseGoModfile(modFilePath)
-	if err != nil {
-		// If we can't parse go.mod, skip transitive checking
-		// This can happen in tests or when analyzing remotely
-		log.Debugf("Could not parse go.mod for transitive checking: %v", err)
-		return
-	}
-
-	// Build map of packages being updated
-	packagesBeingUpdated := make(map[string]string)
-	for _, dep := range strategy.DirectUpdates {
-		packagesBeingUpdated[dep.Name] = dep.Version
-	}
-
-	// Check each package for missing transitive requirements
-	allMissingDeps := make(map[string]MissingDependency)
-
-	for _, dep := range strategy.DirectUpdates {
-		missingDeps, err := CheckTransitiveRequirements(ctx, dep.Name, dep.Version, modFile)
-		if err != nil {
-			log.Warnf("Could not check transitive requirements for %s@%s: %v", dep.Name, dep.Version, err)
-			continue
+	// Caller may pass nil if go.mod could not be parsed (e.g. remote analysis without checkout).
+	if modFile == nil {
+		// Fall back to parsing go.mod ourselves if caller couldn't provide it.
+		modRoot := "."
+		if rootPath, ok := analysis.Metadata["moduleRoot"].(string); ok {
+			modRoot = rootPath
 		}
-
-		// Also check for potential API/schema incompatibilities in other packages
-		apiIssues, err := CheckAPICompatibility(ctx, dep.Name, dep.Version, modFile)
+		var err error
+		modFile, _, err = ParseGoModfile(filepath.Join(modRoot, "go.mod"))
 		if err != nil {
-			log.Debugf("Could not check API compatibility for %s@%s: %v", dep.Name, dep.Version, err)
-		} else if len(apiIssues) > 0 {
-			// Collect API compatibility issues for grouped warning
-			for _, issue := range apiIssues {
-				strategy.Warnings = append(strategy.Warnings,
-					fmt.Sprintf("API Compatibility Alert - %s imports %s which is being updated to %s (may require manual version bump)",
-						issue.Package, dep.Name, dep.Version))
-				log.Infof("API compatibility alert for %s", issue.Package)
-			}
+			log.Debugf("Could not parse go.mod for transitive checking: %v", err)
+			return
 		}
+	}
 
-		// Only add missing deps that are NOT already being updated
-		for _, missing := range missingDeps {
-			// Skip if this dependency is already in the update list
-			if targetVer, beingUpdated := packagesBeingUpdated[missing.Package]; beingUpdated {
-				// Check if the version being updated is sufficient
-				if semver.IsValid(targetVer) && semver.IsValid(missing.RequiredVersion) {
-					if semver.Compare(targetVer, missing.RequiredVersion) >= 0 {
-						log.Debugf("Dependency %s requirement satisfied by update to %s", missing.Package, targetVer)
-						continue
-					}
-				}
-			}
-
-			// Add to missing deps (deduplicate, keep highest required version)
-			if existing, exists := allMissingDeps[missing.Package]; exists {
-				if semver.IsValid(missing.RequiredVersion) && semver.IsValid(existing.RequiredVersion) {
-					if semver.Compare(missing.RequiredVersion, existing.RequiredVersion) > 0 {
-						allMissingDeps[missing.Package] = missing
-					}
-				}
-			} else {
-				allMissingDeps[missing.Package] = missing
-			}
-		}
+	packagesToUpdate := make(map[string]string, len(strategy.DirectUpdates))
+	for _, dep := range strategy.DirectUpdates {
+		packagesToUpdate[dep.Name] = dep.Version
 	}
 
-	// Add missing dependencies to DirectUpdates, skipping no-ops (where version isn't changing)
+	allMissingDeps, apiCompatibilityAlerts := detectCoUpdates(ctx, packagesToUpdate, modFile)
+
+	// Add missing dependencies to DirectUpdates, skipping no-ops (where version isn't changing).
 	if len(allMissingDeps) > 0 {
 		log.Infof("Found %d additional dependencies that need co-updating", len(allMissingDeps))
 		for _, missing := range allMissingDeps {
-			// Skip no-op updates (where required version equals current version)
 			if missing.CurrentVersion == missing.RequiredVersion {
 				log.Debugf("Skipping no-op update for %s (already at %s)", missing.Package, missing.CurrentVersion)
 				continue
@@ -515,6 +483,60 @@ func (ga *GolangAnalyzer) checkTransitiveRequirementsForStrategy(
 			log.Infof("Adding co-update: %s@%s", missing.Package, missing.RequiredVersion)
 		}
 	}
+
+	// Surface API compatibility alerts. When detectCoUpdates determined a minimum
+	// compatible version, add it as a DirectUpdate so it appears in all output types
+	// (JSON, YAML, deps file). A warning is also emitted for human-readable context.
+	// When no version could be determined, emit a warning only.
+	for pkg, recommendedVer := range apiCompatibilityAlerts {
+		// Skip packages that are already being updated — they're handled by DirectUpdates.
+		if _, alreadyUpdating := packagesToUpdate[pkg]; alreadyUpdating {
+			continue
+		}
+		currentVer := getVersion(modFile, pkg)
+		importingPkg, importingVer := findImporterForAlert(pkg, packagesToUpdate, modFile)
+		if recommendedVer != "" && recommendedVer != currentVer {
+			strategy.DirectUpdates = append(strategy.DirectUpdates, analyzer.Dependency{
+				Name:    pkg,
+				Version: recommendedVer,
+				Metadata: map[string]any{
+					"required_by": "api compatibility check",
+					"reason":      fmt.Sprintf("imports %s@%s which has breaking API changes", importingPkg, importingVer),
+				},
+			})
+			log.Infof("Adding API compat co-update: %s@%s (imports %s)", pkg, recommendedVer, importingPkg)
+			strategy.Warnings = append(strategy.Warnings,
+				fmt.Sprintf("API Compatibility Alert - updating %s to %s (imports %s@%s with breaking changes)",
+					pkg, recommendedVer, importingPkg, importingVer))
+			continue
+		}
+		strategy.Warnings = append(strategy.Warnings,
+			fmt.Sprintf("API Compatibility Alert - %s imports %s which is being updated to %s (may require manual version bump)",
+				pkg, importingPkg, importingVer))
+	}
+}
+
+// findImporterForAlert returns a representative (package, version) being updated
+// that triggered the API compatibility alert for the given affected package.
+// Falls back to ("", "") when no concrete importer can be determined; in that case
+// callers should still emit the alert because the affected package is the most
+// actionable signal for the user.
+func findImporterForAlert(affectedPkg string, packagesToUpdate map[string]string, modFile *modfile.File) (string, string) {
+	// Prefer a deterministic choice: walk packagesToUpdate in sorted order.
+	names := make([]string, 0, len(packagesToUpdate))
+	for name := range packagesToUpdate {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+	for _, name := range names {
+		// affectedPkg shouldn't be the importer itself.
+		if name == affectedPkg {
+			continue
+		}
+		return name, packagesToUpdate[name]
+	}
+	// Last resort: report the affected package's current version, with empty importer.
+	return affectedPkg, getVersion(modFile, affectedPkg)
 }
 
 // handleIndirectDependency resolves an indirect dependency to parent bumps.