fix: filter phantom package paths from ResolvePackagePaths

cmd/gaze/main.go

@@ -217,7 +217,7 @@ func runAnalyze(p analyzeParams) error {
 		return fmt.Errorf("getting working directory: %w", err)
 	}
 
-	pkgPaths, err := loader.ResolvePackagePaths(p.patterns, moduleDir)
+	pkgPaths, err := loader.ResolvePackagePaths(p.patterns, moduleDir, p.stderr)
 	if err != nil {
 		return fmt.Errorf("resolving package patterns: %w", err)
 	}
@@ -1066,7 +1066,7 @@ func runQuality(p qualityParams) error {
 		return fmt.Errorf("getting working directory: %w", err)
 	}
 
-	pkgPaths, err := loader.ResolvePackagePaths(p.patterns, moduleDir)
+	pkgPaths, err := loader.ResolvePackagePaths(p.patterns, moduleDir, p.stderr)
 	if err != nil {
 		return fmt.Errorf("resolving package patterns: %w", err)
 	}

internal/aireport/runner_steps.go

@@ -51,7 +51,14 @@ func resolveQualityDeps(deps []qualityPipelineDeps) qualityPipelineDeps {
 		d = deps[0]
 	}
 	if d.resolvePackagePaths == nil {
-		d.resolvePackagePaths = loader.ResolvePackagePaths
+		// Wrap loader.ResolvePackagePaths to match the DI function
+		// type (2-param). Warnings are discarded (nil stderr) to
+		// avoid cascading the io.Writer parameter through the DI
+		// type and all test fakes. Direct callers in cmd/gaze and
+		// goprovider pass stderr for user-facing warning output.
+		d.resolvePackagePaths = func(patterns []string, moduleDir string) ([]string, error) {
+			return loader.ResolvePackagePaths(patterns, moduleDir, nil)
+		}
 	}
 	if d.loadAndAnalyze == nil {
 		d.loadAndAnalyze = analysis.LoadAndAnalyze

internal/loader/loader.go

@@ -5,6 +5,7 @@ package loader
 import (
 	"fmt"
 	"go/token"
+	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -151,7 +152,12 @@ func FindModuleRoot(startDir string) (string, error) {
 // wildcards) to individual fully-qualified package paths. It uses a
 // lightweight NeedName load mode, deduplicates results, and filters
 // out test-variant packages (those with a "_test" suffix).
-func ResolvePackagePaths(patterns []string, moduleDir string) ([]string, error) {
+//
+// Packages with load errors (e.g., nonexistent patterns that produce
+// synthetic packages from go/packages) are skipped and a warning is
+// written to stderr for each error. If stderr is nil, warnings are
+// silently discarded.
+func ResolvePackagePaths(patterns []string, moduleDir string, stderr io.Writer) ([]string, error) {
 	if len(patterns) == 0 {
 		return nil, nil
 	}
@@ -170,6 +176,17 @@ func ResolvePackagePaths(patterns []string, moduleDir string) ([]string, error)
 		if pkg.PkgPath == "" || seen[pkg.PkgPath] || strings.HasSuffix(pkg.PkgPath, "_test") {
 			continue
 		}
+		// Skip packages that failed to load (phantom paths from
+		// unresolvable patterns). go/packages returns err == nil
+		// at the call level but populates pkg.Errors.
+		if len(pkg.Errors) > 0 {
+			if stderr != nil {
+				for _, e := range pkg.Errors {
+					_, _ = fmt.Fprintf(stderr, "warning: skipping %s: %s\n", pkg.PkgPath, e)
+				}
+			}
+			continue
+		}
 		seen[pkg.PkgPath] = true
 		pkgPaths = append(pkgPaths, pkg.PkgPath)
 	}

internal/loader/loader_test.go

@@ -1,6 +1,7 @@
 package loader_test
 
 import (
+	"bytes"
 	"os"
 	"path/filepath"
 	"strings"
@@ -121,7 +122,7 @@ func TestResolvePackagePaths_Wildcard(t *testing.T) {
 	}
 
 	root := findModuleRoot(t)
-	paths, err := loader.ResolvePackagePaths([]string{"./..."}, root)
+	paths, err := loader.ResolvePackagePaths([]string{"./..."}, root, nil)
 	if err != nil {
 		t.Fatalf("ResolvePackagePaths failed: %v", err)
 	}
@@ -153,7 +154,7 @@ func TestResolvePackagePaths_SinglePackage(t *testing.T) {
 
 	root := findModuleRoot(t)
 	paths, err := loader.ResolvePackagePaths(
-		[]string{"github.com/unbound-force/gaze/internal/loader"}, root,
+		[]string{"github.com/unbound-force/gaze/internal/loader"}, root, nil,
 	)
 	if err != nil {
 		t.Fatalf("ResolvePackagePaths failed: %v", err)
@@ -168,7 +169,7 @@ func TestResolvePackagePaths_SinglePackage(t *testing.T) {
 
 func TestResolvePackagePaths_EmptyPatterns(t *testing.T) {
 	root := findModuleRoot(t)
-	paths, err := loader.ResolvePackagePaths([]string{}, root)
+	paths, err := loader.ResolvePackagePaths([]string{}, root, nil)
 	if err != nil {
 		t.Fatalf("ResolvePackagePaths failed: %v", err)
 	}
@@ -177,6 +178,71 @@ func TestResolvePackagePaths_EmptyPatterns(t *testing.T) {
 	}
 }
 
+func TestResolvePackagePaths_InvalidPattern(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping slow test: loads package via go/packages")
+	}
+
+	root := findModuleRoot(t)
+	var buf bytes.Buffer
+	paths, err := loader.ResolvePackagePaths(
+		[]string{"github.com/nonexistent/does/not/exist"}, root, &buf,
+	)
+	if err != nil {
+		t.Fatalf("ResolvePackagePaths returned unexpected error: %v", err)
+	}
+	if len(paths) != 0 {
+		t.Errorf("expected empty paths for nonexistent pattern, got %v", paths)
+	}
+
+	// Verify warning was written to stderr.
+	stderr := buf.String()
+	if !strings.Contains(stderr, "warning: skipping") {
+		t.Errorf("expected warning prefix in stderr, got %q", stderr)
+	}
+	if !strings.Contains(stderr, "github.com/nonexistent/does/not/exist") {
+		t.Errorf("expected package path in stderr warning, got %q", stderr)
+	}
+}
+
+func TestResolvePackagePaths_MixedValidAndInvalid(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping slow test: loads packages via go/packages")
+	}
+
+	root := findModuleRoot(t)
+	var buf bytes.Buffer
+	paths, err := loader.ResolvePackagePaths(
+		[]string{
+			"github.com/unbound-force/gaze/internal/loader",
+			"github.com/nonexistent/does/not/exist",
+		}, root, &buf,
+	)
+	if err != nil {
+		t.Fatalf("ResolvePackagePaths returned unexpected error: %v", err)
+	}
+	if len(paths) != 1 {
+		t.Fatalf("expected 1 valid path, got %d: %v", len(paths), paths)
+	}
+	if paths[0] != "github.com/unbound-force/gaze/internal/loader" {
+		t.Errorf("expected valid path %q, got %q",
+			"github.com/unbound-force/gaze/internal/loader", paths[0])
+	}
+
+	// Verify warning was emitted for the invalid pattern only.
+	stderr := buf.String()
+	if !strings.Contains(stderr, "warning: skipping") {
+		t.Errorf("expected warning prefix in stderr, got %q", stderr)
+	}
+	if !strings.Contains(stderr, "github.com/nonexistent/does/not/exist") {
+		t.Errorf("expected invalid package path in stderr warning, got %q", stderr)
+	}
+	// The valid package should not appear in warnings.
+	if strings.Contains(stderr, "github.com/unbound-force/gaze/internal/loader") {
+		t.Errorf("valid package should not appear in warnings, got %q", stderr)
+	}
+}
+
 func TestIsMainPkg_Library(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping slow test: loads package via go/packages")

internal/provider/goprovider/contract.go

@@ -99,7 +99,7 @@ func BuildContractCoverageFunc(
 	stderr io.Writer,
 	aiMapperFn ...quality.AIMapperFunc,
 ) (func(pkg, function string) (crap.ContractCoverageInfo, bool), []string) {
-	pkgPaths, err := loader.ResolvePackagePaths(patterns, moduleDir)
+	pkgPaths, err := loader.ResolvePackagePaths(patterns, moduleDir, stderr)
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "quality pipeline: failed to resolve packages: %v\n", err)
 		return nil, nil

openspec/changes/fix-phantom-package-paths/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: unbound-force
+created: 2026-07-07

openspec/changes/fix-phantom-package-paths/design.md

@@ -0,0 +1,65 @@
+## Context
+
+`loader.ResolvePackagePaths` (`internal/loader/loader.go:154-177`) resolves package patterns to fully-qualified paths using `go/packages.Load`. The `go/packages` API returns `err == nil` at the call level even when individual packages fail to load -- per-package errors are stored in `pkg.Errors`. The current implementation never checks `pkg.Errors`, so synthetic packages for nonexistent patterns pass through the filter and are returned as valid paths.
+
+This was identified in issue #104 with a confirmed failing test on `main`. The issue comment (#104, council analysis) provides a thorough critique of the naive fix, identifying five gaps that this design addresses.
+
+## Goals / Non-Goals
+
+### Goals
+
+- Filter out packages with load errors in `ResolvePackagePaths`, preventing phantom paths from entering downstream analysis
+- Surface warnings for skipped packages so users understand why a pattern was dropped
+- Add a test for the invalid-pattern case in `internal/loader/loader_test.go`
+- Fix the currently-failing `TestBuildContractCoverageFunc_InvalidPattern` test
+- Close #104
+
+### Non-Goals
+
+- Consolidating the duplicate `resolvePackagePaths` copies across packages (tracked in #139; both copies now delegate to `loader.ResolvePackagePaths`, so fixing the shared function fixes all callers)
+- Differentiating error kinds (`ListError` vs `TypeError` vs `ParseError`) -- the council comment suggests this but it adds complexity for marginal benefit in the `ResolvePackagePaths` context, which uses `NeedName` mode only (type errors are not produced at this load level)
+- Checking `pkg.Name == ""` as a complementary defense -- while noted in the critique, `pkg.Errors` is the canonical signal; `Name == ""` is an implementation detail of `go/packages` that could change
+
+## Decisions
+
+### D1: Add `io.Writer` parameter for warning output
+
+`ResolvePackagePaths` currently has no way to surface diagnostics. Rather than returning a `[]error` or using a global logger, adding an `io.Writer` parameter follows the project's established pattern (see `BuildContractCoverageFunc`, `analyzePackageCoverage`, `quality.Options.Stderr`). Four of five callers already have an `io.Writer` available (`BuildContractCoverageFunc`, `runQualityStep`, `runAnalyze`, `runQuality`). The fifth (`runClassifyStep`) does not; it passes `nil` per D4 to avoid cascading signature changes.
+
+**Signature change**: `ResolvePackagePaths(patterns []string, moduleDir string) ([]string, error)` becomes `ResolvePackagePaths(patterns []string, moduleDir string, stderr io.Writer) ([]string, error)`.
+
+This aligns with Constitution Principle IV (Testability) -- tests can capture warnings via `bytes.Buffer`.
+
+### D2: Skip all errored packages, warn per-error
+
+For each package with `len(pkg.Errors) > 0`, emit one warning line per error to `stderr` and skip the package. This is consistent with `LoadModule` (lines 113-120) which also silently excludes errored packages, though `LoadModule` does not log warnings.
+
+The council critique notes that silently dropping packages is "arguably worse" than the current behavior. The warning output addresses this -- the package is dropped from the result but the user is informed why.
+
+### D3: Do not return an error for partial resolution
+
+If some patterns resolve and others don't, return the valid paths with warnings for the invalid ones. Only the existing behavior of returning `(nil, nil)` for completely empty input is preserved. This matches the best-effort semantics already documented in `BuildContractCoverageFunc`: "if the quality pipeline fails for any package, those packages are silently skipped."
+
+If ALL patterns fail to resolve (non-empty input producing zero valid paths), the function returns `(nil, nil)` -- not an error. The caller (`BuildContractCoverageFunc`) already handles this case correctly by returning `nil`. Callers cannot distinguish "no input" from "all input invalid" via return values alone; the warning output to `stderr` is the only differentiator.
+
+### D4: Nil-safe stderr handling
+
+If `stderr` is nil, warnings are silently discarded. This avoids forcing callers that don't care about diagnostics to pass a writer. The function checks `stderr != nil` before writing.
+
+## Coverage Strategy
+
+Unit test in `internal/loader/loader_test.go` (`TestResolvePackagePaths_InvalidPattern`) covers the new `pkg.Errors` filtering logic directly. Existing `TestBuildContractCoverageFunc_InvalidPattern` in `internal/crap/contract_test.go` serves as integration-level regression guard through the `BuildContractCoverageFunc` -> `ResolvePackagePaths` call chain. Both guarded by `testing.Short()` since they invoke `go/packages.Load`. No e2e test needed -- the fix is internal to the loader package.
+
+## Risks / Trade-offs
+
+### R1: Signature change is breaking for callers
+
+Adding `io.Writer` changes the exported function signature. All known callers are within this module (`internal/crap/contract.go`, `internal/aireport/runner_steps.go`), so the blast radius is contained. External consumers cannot import `internal/` packages.
+
+### R2: NeedName mode limits error kinds
+
+`ResolvePackagePaths` uses `packages.NeedName` -- a lightweight mode. At this load level, errors are primarily `ListError` (package not found). The blanket `len(pkg.Errors) > 0` check is safe here because `TypeError` and `ParseError` are not produced without `NeedTypes`/`NeedSyntax`. If the load mode were ever expanded, this decision would need revisiting.
+
+### R3: No error aggregation
+
+The function does not aggregate errors into a returned `error`. This is intentional (D3) but means a caller passing `stderr=nil` would silently get fewer results than expected. Callers that need strict validation should check result count against input count.

openspec/changes/fix-phantom-package-paths/proposal.md

@@ -0,0 +1,63 @@
+## Why
+
+`loader.ResolvePackagePaths` does not check `pkg.Errors` on packages returned by `go/packages.Load`. When a pattern does not resolve to a real package, `go/packages` returns a synthetic `*packages.Package` with `PkgPath` set to the input pattern and `Errors` populated. Because the filter only rejects empty/duplicate/`_test` paths, the phantom path is returned as valid.
+
+This violates the function's contract ("resolves package patterns to individual fully-qualified package paths") and causes a failing unit test on `main`. For a test-quality tool, shipping with red tests is a credibility problem (Constitution Principle IV).
+
+Ref: https://github.com/unbound-force/gaze/issues/104
+
+## What Changes
+
+Add `pkg.Errors` checking to `loader.ResolvePackagePaths` so that packages with load errors are skipped rather than returned as valid paths. Log warnings for skipped packages. Add a test for the invalid-pattern case directly in `internal/loader/loader_test.go`.
+
+## Capabilities
+
+### New Capabilities
+
+- None
+
+### Modified Capabilities
+
+- `loader.ResolvePackagePaths`: Now filters out packages with load errors (phantom paths) and logs warnings via a new `io.Writer` parameter for the caller to observe diagnostics.
+
+### Removed Capabilities
+
+- None
+
+## Impact
+
+- **`internal/loader/loader.go`**: `ResolvePackagePaths` gains `pkg.Errors` filtering. Signature changes to accept `io.Writer` for warning output.
+- **`internal/loader/loader_test.go`**: New test for invalid/nonexistent pattern.
+- **`internal/crap/contract.go`**: Caller updated to pass `stderr` to `ResolvePackagePaths`.
+- **`internal/aireport/runner_steps.go`**: Two call sites updated. `runQualityStep` (line 90) passes its existing `stderr` parameter. `runClassifyStep` (line 192) passes `nil` since it lacks an `io.Writer` parameter -- warnings are discarded for classify-step resolution, which is acceptable per D4.
+- **`cmd/gaze/main.go`**: Two callers updated -- `runAnalyze` (line 220) and `runQuality` (line 1069), both pass their existing `p.stderr` parameter.
+- Fixes failing `TestBuildContractCoverageFunc_InvalidPattern` test on `main`.
+- Closes #104.
+
+## Constitution Alignment
+
+Assessed against the Gaze project constitution.
+
+### I. Accuracy
+
+**Assessment**: PASS
+
+This fix improves accuracy by preventing phantom package paths from entering the analysis pipeline. Currently, nonexistent patterns produce paths that silently fail downstream, dropping GazeCRAP/quality contributions without a clear error. After this fix, invalid patterns are detected early and surfaced as warnings.
+
+### II. Minimal Assumptions
+
+**Assessment**: PASS
+
+The fix uses `pkg.Errors` -- the standard `go/packages` mechanism for reporting per-package load failures. It does not introduce new assumptions about project structure or coding style. The `go/packages` API documents that `Load` returns `err == nil` even when individual packages fail; checking `pkg.Errors` is the expected usage pattern.
+
+### III. Actionable Output
+
+**Assessment**: PASS
+
+Warning messages for skipped packages give users clear, actionable feedback: the specific pattern that failed and the specific error from `go/packages`. This replaces silent failure downstream with early, explicit diagnostics.
+
+### IV. Testability
+
+**Assessment**: PASS
+
+The fix directly restores a currently-failing test. The `io.Writer` parameter enables test-time capture of warning output without global state. A new dedicated test in `loader_test.go` verifies the invalid-pattern filtering behavior in isolation.

openspec/changes/fix-phantom-package-paths/specs/resolve-package-paths.md

@@ -0,0 +1,55 @@
+## ADDED Requirements
+
+### Requirement: Phantom Package Filtering
+
+`loader.ResolvePackagePaths` MUST skip packages where `len(pkg.Errors) > 0` and MUST NOT include their `PkgPath` in the returned slice. This prevents synthetic packages returned by `go/packages.Load` for nonexistent patterns from being treated as valid package paths.
+
+#### Scenario: Nonexistent package pattern
+
+- **GIVEN** `ResolvePackagePaths` is called with pattern `"github.com/nonexistent/does/not/exist"`
+- **WHEN** `go/packages.Load` returns a synthetic package with `PkgPath` set to the pattern and `Errors` populated
+- **THEN** the returned slice MUST be empty and no error MUST be returned
+
+#### Scenario: Mix of valid and invalid patterns
+
+- **GIVEN** `ResolvePackagePaths` is called with patterns `["github.com/unbound-force/gaze/internal/loader", "github.com/nonexistent/pkg"]`
+- **WHEN** `go/packages.Load` returns one valid package and one errored package
+- **THEN** the returned slice MUST contain only the valid package path and a warning MUST be written to `stderr` for the invalid pattern
+
+### Requirement: Warning Output for Skipped Packages
+
+`loader.ResolvePackagePaths` MUST accept an `io.Writer` parameter for diagnostic output. When a package is skipped due to load errors, the function MUST write a warning line per error to the writer in the format: `warning: skipping <PkgPath>: <error message>`.
+
+#### Scenario: Warning output for invalid pattern
+
+- **GIVEN** `ResolvePackagePaths` is called with a nonexistent pattern and a non-nil `io.Writer`
+- **WHEN** the package is skipped due to load errors
+- **THEN** the writer MUST contain a warning line including the package path and error detail
+
+#### Scenario: Nil stderr writer
+
+- **GIVEN** `ResolvePackagePaths` is called with `stderr` as `nil`
+- **WHEN** a package has load errors
+- **THEN** the package MUST still be skipped and no panic MUST occur
+
+## MODIFIED Requirements
+
+### Requirement: ResolvePackagePaths Signature
+
+`loader.ResolvePackagePaths` signature changes from:
+
+```go
+func ResolvePackagePaths(patterns []string, moduleDir string) ([]string, error)
+```
+
+to:
+
+```go
+func ResolvePackagePaths(patterns []string, moduleDir string, stderr io.Writer) ([]string, error)
+```
+
+Previously: The function accepted only patterns and moduleDir. The new `io.Writer` parameter enables diagnostic output without global logger dependency.
+
+## REMOVED Requirements
+
+None.