From 0b00695b55eee5ff9883d6a3e2f3ad2a250d0b7f Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Thu, 22 Oct 2026 11:17:07 -0700 Subject: [PATCH 1/7] fix(resolver): link a declared npm dependency to its tracked repository A JS/TS import of a package the importer declares in package.json stayed external, even when the package's source repository was tracked: the declared-dependency skip ended the directory cascade, and the package's entry file has no package-qualified name. The cross-repo pass now retries such imports before call reachability. It maps the package name to a tracked repository through package.json names and npm or pnpm workspace members, and picks the entry through exports, types, main or the root index. A symbol binds only when the TypeScript extractor marks it public; otherwise the import binds to the file. The import stays external when the importer's own repository provides the package, when several tracked repositories in the workspace provide it, for npm: aliases, and for workspace paths that leave the repository. The same-repository directory skip from c8214172 is unchanged. Manifests are read lazily, once per resolver, only when a declared import reaches the lookup. Missing-target probes are memoized per pass. Known limits: a JavaScript package binds to the file, since only the TypeScript extractor records visibility; provider-side changes (tracking the provider later, editing its package.json or entry file) reach consumer imports only at the next whole-graph pass or a consumer edit. --- internal/indexer/npm_tracked_package.go | 95 +++++++ internal/indexer/npm_tracked_package_test.go | 264 +++++++++++++++++++ internal/indexer/workspace_resolve.go | 1 + internal/resolver/cross_repo.go | 6 +- internal/resolver/cross_repo_incremental.go | 13 +- internal/resolver/npm_package.go | 184 +++++++++++++ internal/resolver/npm_package_test.go | 251 ++++++++++++++++++ 7 files changed, 812 insertions(+), 2 deletions(-) create mode 100644 internal/indexer/npm_tracked_package.go create mode 100644 internal/indexer/npm_tracked_package_test.go create mode 100644 internal/resolver/npm_package.go create mode 100644 internal/resolver/npm_package_test.go diff --git a/internal/indexer/npm_tracked_package.go b/internal/indexer/npm_tracked_package.go new file mode 100644 index 000000000..cfa9412e4 --- /dev/null +++ b/internal/indexer/npm_tracked_package.go @@ -0,0 +1,95 @@ +package indexer + +import ( + "encoding/json" + "path" + "sync" + + "github.com/zzet/gortex/internal/modules" + "github.com/zzet/gortex/internal/resolver" +) + +// npmPackageLookup lazily snapshots manifest evidence once per resolver. A new +// resolver captures fresh roots and reads fresh manifests after edits or track/untrack. +func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { + type entry struct { + dir string + exports *packageExports + types string + main string + } + roots := map[string]string{} + for prefix, meta := range mi.AllMetadata() { + if meta != nil && meta.RootPath != "" { + roots[prefix] = meta.RootPath + } + } + if len(roots) == 0 { + return nil + } + packages := map[string][]entry{} + var once sync.Once + build := func() { + for prefix, root := range roots { + rootSrc, _ := readDiskFile(joinPath(root, "package.json")) + workspace := &modules.WorkspaceManifest{Ecosystem: modules.WorkspaceNPM, Patterns: modules.ParsePackageJSONWorkspaces(rootSrc)} + if len(workspace.Patterns) == 0 { + src, _ := readDiskFile(joinPath(root, "pnpm-workspace.yaml")) + workspace.Ecosystem = modules.WorkspacePnpm + workspace.Patterns = modules.ParsePnpmWorkspaceYAML(src) + } + dirs := []string{"."} + for _, member := range modules.ResolveWorkspaceMembers(root, workspace) { + dirs = append(dirs, member.Dir) + } + for _, dir := range dirs { + if path.IsAbs(dir) || hasDotDotSegment(dir) { + continue + } + src := rootSrc + if dir != "." { + src, _ = readDiskFile(joinPath(root, joinRel(dir, "package.json"))) + } + var manifest struct { + Name string `json:"name"` + Types string `json:"types"` + Main string `json:"main"` + } + if json.Unmarshal(src, &manifest) != nil || manifest.Name == "" { + continue + } + packages[manifest.Name] = append(packages[manifest.Name], entry{ + dir: path.Join(prefix, dir), exports: parsePackageExports(src), types: manifest.Types, main: manifest.Main, + }) + } + } + } + return func(specifier string) map[string][]string { + once.Do(build) + name, sub := splitPackageSpecifier(specifier) + if name == "" || hasDotDotSegment(sub) || len(packages[name]) == 0 { + return nil + } + targets := map[string][]string{} + for _, pkg := range packages[name] { + targets[pkg.dir] = nil + var paths []string + if pkg.exports != nil { + paths = append(paths, resolveExportsSubpath(pkg.exports.subpaths, sub)) + } + if sub == "" { + paths = append(paths, pkg.types, pkg.main, "index") + } else if pkg.exports == nil { + paths = append(paths, sub) + } + for _, target := range paths { + // Never let manifest paths bind another package/repository. + if target == "" || path.IsAbs(target) || hasDotDotSegment(target) { + continue + } + targets[pkg.dir] = append(targets[pkg.dir], path.Join(pkg.dir, target)) + } + } + return targets + } +} diff --git a/internal/indexer/npm_tracked_package_test.go b/internal/indexer/npm_tracked_package_test.go new file mode 100644 index 000000000..bf7580048 --- /dev/null +++ b/internal/indexer/npm_tracked_package_test.go @@ -0,0 +1,264 @@ +package indexer + +import ( + "context" + "path" + "path/filepath" + "strconv" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/graph/store_sqlite" + "github.com/zzet/gortex/internal/parser/languages" + "github.com/zzet/gortex/internal/search" +) + +func TestMultiIndexer_TrackedNpmPackage(t *testing.T) { + cases := []struct { + name, packageName, manifest, entry, packageDir, consumerName, providerWS, providerName, specifier, depVersion string + resolves, pnpm bool + }{ + {name: "scoped exports", packageName: "@scope/p", manifest: `"exports":"./src/index.ts","types":"./wrong.ts","main":"./wrong.ts"`, entry: "src/index.ts", resolves: true}, + {name: "unscoped subpath exports", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./utils":"./src/util.js"}`, entry: "src/util.ts", resolves: true}, + {name: "scoped subpath exports", packageName: "@scope/p", specifier: "@scope/p/utils", manifest: `"exports":{"./utils":{"import":"./src/util.ts"}}`, entry: "src/util.ts", resolves: true}, + {name: "subpath relative", packageName: "p", specifier: "p/utils", entry: "utils/index.ts", resolves: true}, + {name: "subpath wildcard", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":"./src/*.ts"}`, entry: "src/utils.ts", resolves: true}, + {name: "unmapped subpath refused", packageName: "p", specifier: "p/private", manifest: `"exports":{".":"./index.ts"}`, entry: "private.ts"}, + {name: "npm alias collision refused", packageName: "p", depVersion: "npm:other@^1", entry: "index.ts"}, + {name: "types", packageName: "p", manifest: `"types":"./src/types.ts","main":"./wrong.ts"`, entry: "src/types.ts", resolves: true}, + {name: "main", packageName: "p", manifest: `"main":"./src/main.ts"`, entry: "src/main.ts", resolves: true}, + {name: "directory index", packageName: "p", entry: "index.ts", resolves: true}, + {name: "workspace package", packageName: "@scope/p", manifest: `"exports":{".":{"import":"./src/index.ts"}}`, entry: "src/index.ts", packageDir: "packages/lib", resolves: true}, + {name: "pnpm workspace package", packageName: "p", entry: "index.ts", packageDir: "packages/lib", resolves: true, pnpm: true}, + {name: "untracked package collision", packageName: "p", providerName: "other", entry: "p/index.ts"}, + {name: "same repo package refused", packageName: "p", providerName: "other", entry: "index.ts", consumerName: "p"}, + {name: "workspace boundary refused", packageName: "p", manifest: `"main":"./src/index.ts"`, entry: "src/index.ts", providerWS: "private"}, + {name: "entry escape refused", packageName: "p", manifest: `"exports":"../src/index.ts","types":"/src/index.ts","main":"../src/index.ts"`, entry: "src/index.ts"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + provider := t.TempDir() + consumer := t.TempDir() + packageRoot := filepath.Join(provider, tc.packageDir) + providerName := tc.providerName + if providerName == "" { + providerName = tc.packageName + } + manifest := `{"name":` + strconv.Quote(providerName) + if tc.manifest != "" { + manifest += "," + tc.manifest + } + writePackageJSON(t, packageRoot, manifest+"}") + if tc.packageDir != "" { + if tc.pnpm { + writePackageJSON(t, provider, `{"name":"monorepo"}`) + writeWorkspaceFile(t, provider, "pnpm-workspace.yaml", "packages:\n - 'packages/*'\n") + } else { + writePackageJSON(t, provider, `{"name":"monorepo","workspaces":["packages/*"]}`) + } + } + writeWorkspaceFile(t, packageRoot, tc.entry, "export function f(): number { return 42; }\n") + consumerName := tc.consumerName + if consumerName == "" { + consumerName = "app" + } + specifier := tc.specifier + if specifier == "" { + specifier = tc.packageName + } + depVersion := tc.depVersion + if depVersion == "" { + depVersion = "^1.0.0" + } + writePackageJSON(t, consumer, `{"name":`+strconv.Quote(consumerName)+`,"dependencies":{`+strconv.Quote(tc.packageName)+`:`+strconv.Quote(depVersion)+`}}`) + writeWorkspaceFile(t, consumer, "src/app.ts", "import { f } from "+strconv.Quote(specifier)+";\nexport function run(): number { return f(); }\n") + if tc.consumerName == "p" { + writeWorkspaceFile(t, consumer, "index.ts", "export function unrelated(): number { return 0; }\n") + } + // A same-named entry point inside the importer is never package evidence. + writeWorkspaceFile(t, consumer, "src/p/index.ts", "export function unrelated(): number { return 0; }\n") + providerWS := tc.providerWS + if providerWS == "" { + providerWS = "shared" + } + wantFile := path.Join("provider", tc.packageDir, tc.entry) + + cfgPath := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{Repos: []config.RepoEntry{ + {Path: provider, Name: "provider", Workspace: providerWS}, + {Path: consumer, Name: "consumer", Workspace: "shared"}, + }} + gc.SetConfigPath(cfgPath) + require.NoError(t, gc.Save()) + cm, err := config.NewConfigManager(cfgPath) + require.NoError(t, err) + g := graph.New() + reg := newTestRegistry() + reg.Register(languages.NewTypeScriptExtractor()) + mi := NewMultiIndexer(g, reg, search.NewNull(), cm, zap.NewNop()) + _, err = mi.IndexAll() + require.NoError(t, err) + + require.NotNil(t, g.GetNode(wantFile+"::f")) + t.Logf("package-qualified nodes: %v", g.GetNodesByQualNames([]string{tc.packageName})[tc.packageName]) + for _, n := range g.AllNodes() { + if n.FilePath == wantFile { + t.Logf("entry node: id=%s kind=%s qualName=%s", n.ID, n.Kind, n.QualName) + } + } + var importTargets []string + var calls int + for _, e := range g.AllEdges() { + if e.FilePath != "consumer/src/app.ts" { + continue + } + switch e.Kind { + case graph.EdgeImports: + importTargets = append(importTargets, e.To) + t.Logf("import: %s -> %s crossRepo=%v", e.From, e.To, e.CrossRepo) + assert.Equal(t, tc.resolves, e.CrossRepo) + case graph.EdgeCalls: + calls++ + t.Logf("call: %s -> %s crossRepo=%v", e.From, e.To, e.CrossRepo) + if tc.resolves { + assert.Equal(t, wantFile+"::f", e.To) + assert.True(t, e.CrossRepo) + } else { + assert.Equal(t, "unresolved::f", e.To) + assert.False(t, e.CrossRepo) + } + } + } + if tc.resolves { + assert.ElementsMatch(t, []string{wantFile, wantFile + "::f"}, importTargets) + } else { + external := "external::" + specifier + module := external + if tc.depVersion != "" { + module = "external::other" + } + assert.ElementsMatch(t, []string{module, external + "::f"}, importTargets) + } + assert.Equal(t, 1, calls) + }) + } +} + +func TestTrackedNpmPackageWorkspaceEscape(t *testing.T) { + base := t.TempDir() + a, b := filepath.Join(base, "a"), filepath.Join(base, "b", "pkg") + writePackageJSON(t, a, `{"name":"a","workspaces":["../b/*"]}`) + writePackageJSON(t, b, `{"name":"p","main":"index.ts"}`) + mi := &MultiIndexer{repos: map[string]*RepoMetadata{"a": {RootPath: a}}} + lookup := mi.npmPackageLookup() + require.NotNil(t, lookup) + assert.Empty(t, lookup("p"), "an escaped workspace must not supply another repo's graph prefix") +} + +func TestMultiIndexer_TrackedNpmPackageTrackSQLite(t *testing.T) { + for _, providerFirst := range []bool{true, false} { + t.Run(strconv.FormatBool(providerFirst), func(t *testing.T) { + provider, consumer := t.TempDir(), t.TempDir() + writePackageJSON(t, provider, `{"name":"p","exports":"./src/index.ts"}`) + writeWorkspaceFile(t, provider, "src/index.ts", "export function f(): number { return 42; }\n") + writePackageJSON(t, consumer, `{"name":"app","dependencies":{"p":"^1"}}`) + writeWorkspaceFile(t, consumer, "src/app.ts", "import { f } from 'p';\nexport function run(): number { return f(); }\n") + entries := []config.RepoEntry{{Path: provider, Name: "provider", Workspace: "shared"}, {Path: consumer, Name: "consumer", Workspace: "shared"}} + if !providerFirst { + entries[0], entries[1] = entries[1], entries[0] + } + cfgPath := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{Repos: entries} + gc.SetConfigPath(cfgPath) + require.NoError(t, gc.Save()) + cm, err := config.NewConfigManager(cfgPath) + require.NoError(t, err) + db := filepath.Join(t.TempDir(), "graph.sqlite") + g, err := store_sqlite.Open(db) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, g.Close()) }) + newIndexer := func() *MultiIndexer { + reg := newTestRegistry() + reg.Register(languages.NewTypeScriptExtractor()) + return NewMultiIndexer(g, reg, search.NewNull(), cm, zap.NewNop()) + } + mi := newIndexer() + for _, entry := range entries { + _, err := mi.TrackRepoCtx(context.Background(), entry) + require.NoError(t, err) + } + // The daemon runs this whole-store resolve after cold per-repo tracking. + require.NoError(t, mi.RunPreEnrichResolve(context.Background(), nil, nil)) + check := func(stage string) { + t.Helper() + var imports, calls []string + for _, e := range g.AllEdges() { + if e.FilePath != "consumer/src/app.ts" { + continue + } + switch e.Kind { + case graph.EdgeImports: + imports = append(imports, e.To) + assert.True(t, e.CrossRepo, stage) + case graph.EdgeCalls: + calls = append(calls, e.To) + assert.True(t, e.CrossRepo, stage) + } + } + assert.ElementsMatch(t, []string{"provider/src/index.ts", "provider/src/index.ts::f"}, imports, stage) + assert.Equal(t, []string{"provider/src/index.ts::f"}, calls, stage) + } + check("cold track") + mtimes := map[string]map[string]int64{} + for _, entry := range entries { + mtimes[entry.Name] = mi.FileMtimes(entry.Name) + } + require.NoError(t, mi.Close(context.Background())) + require.NoError(t, g.Close()) + g, err = store_sqlite.Open(db) + require.NoError(t, err) + warm := newIndexer() + t.Cleanup(func() { require.NoError(t, warm.Close(context.Background())) }) + for _, entry := range entries { + _, err := warm.ReconcileRepoCtx(context.Background(), entry, mtimes[entry.Name]) + require.NoError(t, err) + } + // A clean warm restart skips the global pass and keeps persisted links. + check("warm restart") + }) + } +} + +func TestTrackedNpmPackageLazySnapshot(t *testing.T) { + root := t.TempDir() + mi := &MultiIndexer{repos: map[string]*RepoMetadata{"lib": {RootPath: root}}} + lookup := mi.npmPackageLookup() + require.NotNil(t, lookup) + // Construction must not read manifests. First use sees this newly written file. + writePackageJSON(t, root, `{"name":"p","main":"index.ts"}`) + require.Contains(t, lookup("p"), "lib") + writePackageJSON(t, root, `{"name":"q","main":"other.ts"}`) + assert.Contains(t, lookup("p"), "lib", "one pass keeps its snapshot") + fresh := mi.npmPackageLookup() + assert.Empty(t, fresh("p")) + assert.Contains(t, fresh("q"), "lib", "the next pass sees manifest edits") + mi.repos = map[string]*RepoMetadata{"new": {RootPath: root}} + assert.Contains(t, mi.npmPackageLookup()("q"), "new", "track/untrack replaces roots") + assert.Contains(t, fresh("q"), "lib", "old pass keeps captured roots") +} + +func TestTrackedNpmPackagePnpmWorkspace(t *testing.T) { + root := t.TempDir() + writePackageJSON(t, root, `{"name":"monorepo"}`) + writeWorkspaceFile(t, root, "pnpm-workspace.yaml", "packages:\n - 'packages/*'\n") + writePackageJSON(t, filepath.Join(root, "packages", "lib"), `{"name":"p","main":"index.ts"}`) + mi := &MultiIndexer{repos: map[string]*RepoMetadata{"lib": {RootPath: root}}} + lookup := mi.npmPackageLookup() + require.NotNil(t, lookup) + assert.Equal(t, map[string][]string{"lib/packages/lib": {"lib/packages/lib/index.ts", "lib/packages/lib/index"}}, lookup("p")) +} diff --git a/internal/indexer/workspace_resolve.go b/internal/indexer/workspace_resolve.go index e10fb8bfc..610c65d9d 100644 --- a/internal/indexer/workspace_resolve.go +++ b/internal/indexer/workspace_resolve.go @@ -498,6 +498,7 @@ func (mi *MultiIndexer) newCrossRepoResolver() *resolver.CrossRepoResolver { cr.SetCrossWorkspaceDepLookup(mi.crossWorkspacePassLookup()) cr.SetNpmAliasResolver(mi.npmAliasResolver()) cr.SetNpmDependencyLookup(mi.npmDependencyLookup()) + cr.SetNpmPackageLookup(mi.npmPackageLookup()) cr.SetPathAliasResolver(mi.pathAliasResolver()) cr.SetWorkspaceMembership(mi.workspaceMembershipResolver()) mi.applyRemoteStitch(cr) diff --git a/internal/resolver/cross_repo.go b/internal/resolver/cross_repo.go index 0d0ae26a2..951d8728e 100644 --- a/internal/resolver/cross_repo.go +++ b/internal/resolver/cross_repo.go @@ -4,6 +4,7 @@ import ( "context" "os" "runtime" + "slices" "sort" "strconv" "strings" @@ -140,7 +141,8 @@ type CrossRepoResolver struct { // importer as a dependency resolving outside the repository. Same // contract as the field of the same name on Resolver — see // npm_dependency.go. - npmDep NpmDependencyLookup + npmDep NpmDependencyLookup + npmPackages NpmPackageLookup // pathAlias expands a JS/TS tsconfig/jsconfig path-alias / baseUrl // import specifier to the repo-prefixed file stem it targets. Same // contract as the field of the same name on Resolver — see @@ -388,6 +390,7 @@ func (cr *CrossRepoResolver) ResolveAllContext(ctx context.Context) (*CrossRepoS // Every exit applies the moves the pass deferred (a no-op after the // normal flush below); it runs before the deferred unlock above. defer cr.placeholderSrcIdx.applyDeferred(cr.graph) + cr.resolveTrackedNpmImports(graph.EdgesLightSeq(cr.graph, graph.EdgeImports), stats) // Share the master resolver's stable high-water/keyset stream. The cold // residual can exceed 200k edges; retaining it plus the cross-repo name, // raw-name, repo and qualified-name caches was the second whole-corpus heap @@ -672,6 +675,7 @@ func (cr *CrossRepoResolver) ResolveForFile(repoPrefix, relPath string) *CrossRe // scopes. Caller holds cr.mu. func (cr *CrossRepoResolver) resolveScopedLocked(edges []*graph.Edge) *CrossRepoStats { stats := &CrossRepoStats{ByRepo: make(map[string]int)} + cr.resolveTrackedNpmImports(slices.Values(edges), stats) pending := make([]*graph.Edge, 0, len(edges)) for _, edge := range edges { if edge != nil && graph.IsUnresolvedTarget(edge.To) { diff --git a/internal/resolver/cross_repo_incremental.go b/internal/resolver/cross_repo_incremental.go index f43a5e934..1c06d5300 100644 --- a/internal/resolver/cross_repo_incremental.go +++ b/internal/resolver/cross_repo_incremental.go @@ -69,10 +69,21 @@ func (cr *CrossRepoResolver) ResolveMutationFrontiersBounded(resolutionFiles, ed } logIncomingAdmissionRefusal(cr.logger, "cross_repo_preparation", len(frontier.stubKeys), frontier.incomingAdmission, frontier.incomingRefusal) + // Per-repo resolution has already made registry imports external. + // Keep outgoing imports for the tracked-package retry as well. + if cr.npmPackages != nil { + for _, edges := range frontier.outByNode { + for _, edge := range edges { + if trackedNpmImportSpec(edge) != "" { + frontier.pending = append(frontier.pending, edge) + } + } + } + } pending := make([]*graph.Edge, 0, len(frontier.pending)) seen := make(map[graph.EdgeIdentity]struct{}, len(frontier.pending)) for _, edge := range frontier.pending { - if edge == nil || !graph.IsUnresolvedTarget(edge.To) { + if edge == nil || !graph.IsUnresolvedTarget(edge.To) && trackedNpmImportSpec(edge) == "" { continue } identity := graph.EdgeIdentityFor(edge) diff --git a/internal/resolver/npm_package.go b/internal/resolver/npm_package.go new file mode 100644 index 000000000..943fc1df6 --- /dev/null +++ b/internal/resolver/npm_package.go @@ -0,0 +1,184 @@ +package resolver + +import ( + "iter" + "strings" + + "github.com/zzet/gortex/internal/graph" +) + +// NpmPackageLookup maps a bare npm specifier to tracked package directories. +// Each directory has ordered graph-path entry candidates (exports, types, main, +// then the directory index). The indexer owns manifest reads; the resolver +// probes indexed files and applies repository/workspace policy. +type NpmPackageLookup func(specifier string) map[string][]string + +// SetNpmPackageLookup installs tracked-package evidence for the cross-repo pass. +// Call before resolution; nil keeps the external-package short-circuit unchanged. +func (cr *CrossRepoResolver) SetNpmPackageLookup(fn NpmPackageLookup) { + cr.npmPackages = fn +} + +type npmProbeKey struct { + dir, spec string +} + +type npmImportMemo struct { + probes map[npmProbeKey]*graph.Node + nodes map[string]*graph.Node +} + +func (m *npmImportMemo) getNode(cr *CrossRepoResolver, id string) *graph.Node { + if node, seen := m.nodes[id]; seen { + return node + } + node := cr.cachedGetNode(id) + m.nodes[id] = node + return node +} + +func (cr *CrossRepoResolver) trackedNpmImport(callerRepo, callerWS, importPath string, memo *npmImportMemo) *graph.Node { + spec, symbol := splitImportSpecSymbol(importPath) + packages := cr.npmPackages(spec) + // Manifest ownership, not a successful probe, blocks linking a monorepo + // import to a clone or fork of its own workspace package. + for dir := range packages { + if dir == callerRepo || strings.HasPrefix(dir, callerRepo+"/") { + return nil + } + } + var local, foreign []*graph.Node + for dir, stems := range packages { + key := npmProbeKey{dir: dir, spec: spec} + node, seen := memo.probes[key] + if !seen { + for _, stem := range stems { + if file := probeJSTSFile(cr.cachedGetNode, stem); file != "" { + node = cr.cachedGetNode(file) + break + } + } + memo.probes[key] = node // Cache misses too; reset on the next pass. + } + if node == nil { + continue + } + if candidateWorkspaceID(node) == callerWS { + local = append(local, node) + } else if cr.crossWorkspaceEligible(callerWS, candidateWorkspaceID(node), spec) { + foreign = append(foreign, node) + } + } + candidates := local + if len(candidates) == 0 { + candidates = foreign + } + // Package names do not identify a unique tracked checkout when more + // than one provider is eligible in the winning workspace tier. + if len(candidates) != 1 { + return nil + } + node := candidates[0] + if symbol != "" { + if exported := memo.getNode(cr, node.ID+"::"+symbol); exported != nil && exported.Meta["visibility"] == "public" { + node = exported + } + } + return node +} + +// The per-repo pass has already made unindexed npm imports external. Retry only +// imports with positive tracked-package evidence before building call reachability. +func (cr *CrossRepoResolver) resolveTrackedNpmImports(edges iter.Seq[*graph.Edge], stats *CrossRepoStats) { + if cr.npmPackages == nil { + return + } + // Scan metadata-free projections. Hydrate only imports whose package name + // is tracked, so ordinary node_modules dependencies stay on the cheap path. + var eligible []*graph.Edge + for e := range edges { + spec := trackedNpmImportSpec(e) + if spec == "" || !cr.eligibleTrackedNpmImport(e, spec) { + continue + } + module, _ := splitImportSpecSymbol(spec) + if len(cr.npmPackages(module)) == 0 { + continue + } + eligible = append(eligible, e) + } + if len(eligible) == 0 { + return + } + memo := &npmImportMemo{probes: map[npmProbeKey]*graph.Node{}, nodes: map[string]*graph.Node{}} + // Close the light-edge cursor before reading or writing full edges. Hydrate + // only one bounded page at a time; metadata never accumulates across pages. + for start := 0; start < len(eligible); start += resolvePendingPageRows { + page := eligible[start:min(start+resolvePendingPageRows, len(eligible))] + sites := make([]graph.EdgeSite, 0, len(page)) + for _, e := range page { + sites = append(sites, graph.EdgeSite{From: e.From, Line: e.Line, Kind: graph.EdgeImports}) + } + full := cr.graph.GetEdgeCandidates(nil, sites) + var batch []graph.EdgeReindex + for _, projected := range page { + for _, e := range full.Site(projected.From, projected.Line, graph.EdgeImports) { + if e.To != projected.To { + continue + } + oldTo := e.To + if cr.resolveTrackedNpmImport(e, trackedNpmImportSpec(e), stats, memo) { + batch = append(batch, graph.EdgeReindex{Edge: e, OldTo: oldTo}) + projected.To, projected.CrossRepo = e.To, e.CrossRepo + } + } + } + if len(batch) > 0 { + cr.graph.ReindexEdges(batch) + } + } +} + +func trackedNpmImportSpec(e *graph.Edge) string { + if e == nil || e.Kind != graph.EdgeImports { + return "" + } + if strings.HasPrefix(e.To, "external::") { + return strings.TrimPrefix(e.To, "external::") + } + if target := graph.UnresolvedName(e.To); strings.HasPrefix(target, "import::") { + return strings.TrimPrefix(target, "import::") + } + return "" +} + +func (cr *CrossRepoResolver) resolveTrackedNpmImport(e *graph.Edge, importPath string, stats *CrossRepoStats, memo *npmImportMemo) bool { + from := memo.getNode(cr, e.From) + callerRepo := "" + if from != nil { + callerRepo = from.RepoPrefix + } + callerWS := candidateWorkspaceID(from) + picked := cr.trackedNpmImport(callerRepo, callerWS, importPath, memo) + if picked == nil { + return false + } + e.To = picked.ID + e.CrossRepo = true + stats.Resolved++ + stats.CrossRepoEdges++ + stats.ByRepo[picked.RepoPrefix]++ + return true +} + +// These checks use only the light projection and must precede manifest scans +// and full-edge hydration. +func (cr *CrossRepoResolver) eligibleTrackedNpmImport(e *graph.Edge, importPath string) bool { + callerFile := jsTSImportCallerFile(e) + if !isJSTSBareSpecifier(callerFile, importPath) || !declaresExternalNpmDep(cr.npmDep, callerFile, importPath) { + return false + } + spec, _ := splitImportSpecSymbol(importPath) + _, aliased := rewriteNpmAliasImport(cr.npmAlias, callerFile, spec) + return !aliased +} diff --git a/internal/resolver/npm_package_test.go b/internal/resolver/npm_package_test.go new file mode 100644 index 000000000..6b5837a9d --- /dev/null +++ b/internal/resolver/npm_package_test.go @@ -0,0 +1,251 @@ +package resolver + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/graph/store_sqlite" +) + +func TestCrossRepoTrackedNpmPackage(t *testing.T) { + for _, backend := range []string{"memory", "sqlite"} { + for _, target := range []string{"external::p", "unresolved::import::p"} { + for _, mode := range []string{"all", "repo", "file", "mutation", "import only"} { + t.Run(backend+"/"+target+"/"+mode, func(t *testing.T) { + var g graph.Store = graph.New() + if backend == "sqlite" { + store, err := store_sqlite.Open(filepath.Join(t.TempDir(), "graph.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, store.Close()) }) + g = store + } + g.AddBatch([]*graph.Node{ + {ID: "app/src/app.ts", Kind: graph.KindFile, FilePath: "app/src/app.ts", RepoPrefix: "app", Language: "typescript"}, + {ID: "app/src/app.ts::run", Kind: graph.KindFunction, Name: "run", FilePath: "app/src/app.ts", RepoPrefix: "app", Language: "typescript"}, + {ID: "lib/src/index.ts", Kind: graph.KindFile, FilePath: "lib/src/index.ts", RepoPrefix: "lib", Language: "typescript"}, + {ID: "lib/src/index.ts::f", Kind: graph.KindFunction, Name: "f", FilePath: "lib/src/index.ts", RepoPrefix: "lib", Language: "typescript"}, + }, nil) + imp := &graph.Edge{From: "app/src/app.ts", To: target, Kind: graph.EdgeImports, FilePath: "app/src/app.ts", Line: 1, Meta: map[string]any{"test_marker": "preserved"}} + g.AddEdge(imp) + if mode != "import only" { + g.AddEdge(&graph.Edge{From: "app/src/app.ts::run", To: "unresolved::f", Kind: graph.EdgeCalls, FilePath: "app/src/app.ts", Line: 2}) + } + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, spec string) bool { return spec == "p" }) + cr.SetNpmPackageLookup(func(spec string) map[string][]string { + if spec != "p" { + return nil + } + return map[string][]string{"lib": {"lib/src/index.ts", "lib"}} + }) + var stats *CrossRepoStats + switch mode { + case "all", "import only": + stats = cr.ResolveAll() + case "repo": + stats = cr.ResolveForRepo("app") + case "file": + stats = cr.ResolveForFile("app", "src/app.ts") + case "mutation": + stats = cr.ResolveFilesAndIncoming([]string{"app/src/app.ts"}) + } + wantResolved := 2 + if mode == "import only" { + wantResolved = 1 + } + assert.Equal(t, wantResolved, stats.Resolved) + assert.Equal(t, wantResolved, stats.CrossRepoEdges) + imports := g.GetOutEdges("app/src/app.ts") + require.Len(t, imports, 1) + var found bool + for _, e := range imports { + if e.Kind != graph.EdgeImports { + continue + } + found = true + assert.Equal(t, "lib/src/index.ts", e.To) + assert.True(t, e.CrossRepo) + assert.Equal(t, "preserved", e.Meta["test_marker"]) + } + assert.True(t, found) + if mode != "import only" { + var calls []*graph.Edge + for _, e := range g.GetOutEdges("app/src/app.ts::run") { + if e.Kind == graph.EdgeCalls { + calls = append(calls, e) + } + } + require.Len(t, calls, 1) + assert.Equal(t, "lib/src/index.ts::f", calls[0].To) + assert.True(t, calls[0].CrossRepo) + } + // A second pass is idempotent and leaves no old target bucket. + assert.Zero(t, cr.ResolveAll().Resolved) + assert.Empty(t, g.GetInEdges(target)) + }) + } + } + } +} + +func TestCrossRepoTrackedNpmPackageAmbiguousOrUndeclared(t *testing.T) { + for _, declared := range []bool{false, true} { + t.Run(map[bool]string{false: "undeclared", true: "ambiguous"}[declared], func(t *testing.T) { + g := graph.New() + g.AddNode(&graph.Node{ID: "app/a.ts", Kind: graph.KindFile, FilePath: "app/a.ts", RepoPrefix: "app", Language: "typescript"}) + g.AddNode(&graph.Node{ID: "one/index.ts", Kind: graph.KindFile, FilePath: "one/index.ts", RepoPrefix: "one", Language: "typescript"}) + g.AddNode(&graph.Node{ID: "two/index.ts", Kind: graph.KindFile, FilePath: "two/index.ts", RepoPrefix: "two", Language: "typescript"}) + imp := &graph.Edge{From: "app/a.ts", To: "external::p", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1} + g.AddEdge(imp) + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return declared }) + cr.SetNpmPackageLookup(func(_ string) map[string][]string { + targets := map[string][]string{"one": {"one/index.ts"}} + if declared { + targets["two"] = []string{"two/index.ts"} + } + return targets + }) + assert.Zero(t, cr.ResolveAll().Resolved) + assert.Equal(t, "external::p", imp.To) + assert.False(t, imp.CrossRepo) + }) + } +} + +func TestCrossRepoTrackedNpmPackageBoundaries(t *testing.T) { + for _, tc := range []struct { + name, visibility string + dirs map[string][]string + want string + }{ + {"own package missing index", "private", map[string][]string{"app/packages/p": {"app/packages/p/missing"}, "one": {"one/index.ts"}}, "external::p::f"}, + {"own root package", "private", map[string][]string{"app": {"app/missing"}, "one": {"one/index.ts"}}, "external::p::f"}, + {"same workspace ambiguous", "private", map[string][]string{"one": {"one/index.ts"}, "two": {"two/index.ts"}}, "external::p::f"}, + {"private symbol", "private", map[string][]string{"one": {"one/index.ts"}}, "one/index.ts"}, + {"public symbol", "public", map[string][]string{"one": {"one/index.ts"}}, "one/index.ts::f"}, + {"unknown visibility", "", map[string][]string{"one": {"one/index.ts"}}, "one/index.ts"}, + } { + t.Run(tc.name, func(t *testing.T) { + g := graph.New() + g.AddNode(&graph.Node{ID: "app/a.ts", Kind: graph.KindFile, FilePath: "app/a.ts", RepoPrefix: "app", WorkspaceID: "shared"}) + for _, repo := range []string{"one", "two"} { + file := repo + "/index.ts" + g.AddNode(&graph.Node{ID: file, Kind: graph.KindFile, FilePath: file, RepoPrefix: repo, WorkspaceID: "shared"}) + g.AddNode(&graph.Node{ID: file + "::f", Kind: graph.KindFunction, Name: "f", FilePath: file, RepoPrefix: repo, WorkspaceID: "shared", Meta: map[string]any{"visibility": tc.visibility}}) + } + imp := &graph.Edge{From: "app/a.ts", To: "external::p::f", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1} + g.AddEdge(imp) + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return true }) + cr.SetNpmPackageLookup(func(_ string) map[string][]string { return tc.dirs }) + cr.ResolveAll() + assert.Equal(t, tc.want, imp.To) + assert.Equal(t, tc.want != "external::p::f", imp.CrossRepo) + }) + } +} + +type npmProbeCountingStore struct { + graph.Store + providerReads int +} + +func (g *npmProbeCountingStore) GetNode(id string) *graph.Node { + if len(id) >= 4 && id[:4] == "lib/" { + g.providerReads++ + } + return g.Store.GetNode(id) +} + +func TestCrossRepoTrackedNpmPackageProbeMemo(t *testing.T) { + for _, declared := range []bool{false, true} { + t.Run(map[bool]string{false: "undeclared", true: "missing target"}[declared], func(t *testing.T) { + g := &npmProbeCountingStore{Store: graph.New()} + g.AddNode(&graph.Node{ID: "app/a.ts", Kind: graph.KindFile, FilePath: "app/a.ts", RepoPrefix: "app"}) + for line := 1; line <= 100; line++ { + g.AddEdge(&graph.Edge{From: "app/a.ts", To: "external::p", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: line}) + } + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return declared }) + cr.SetNpmPackageLookup(func(_ string) map[string][]string { return map[string][]string{"lib": {"lib/missing"}} }) + for pass := 0; pass < 2; pass++ { + g.providerReads = 0 + cr.ResolveAll() + t.Logf("pass=%d declared=%v provider GetNode calls=%d", pass, declared, g.providerReads) + if declared { + assert.Equal(t, 2*len(jsTSImportExts), g.providerReads) + } else { + assert.Zero(t, g.providerReads) + } + } + }) + } +} + +type npmHydrationCountingStore struct { + graph.Store + sites, maxSites int + reads map[string]int +} + +func (g *npmHydrationCountingStore) GetEdgeCandidates(endpoints []graph.EdgeEndpoint, sites []graph.EdgeSite) graph.EdgeCandidateSet { + g.sites += len(sites) + g.maxSites = max(g.maxSites, len(sites)) + return g.Store.GetEdgeCandidates(endpoints, sites) +} + +func (g *npmHydrationCountingStore) GetNode(id string) *graph.Node { + g.reads[id]++ + return g.Store.GetNode(id) +} + +func TestCrossRepoTrackedNpmPackageHydration(t *testing.T) { + for _, backend := range []string{"memory", "sqlite"} { + for _, mode := range []string{"undeclared", "alias", "declared"} { + t.Run(backend+"/"+mode, func(t *testing.T) { + var store graph.Store = graph.New() + if backend == "sqlite" { + db, err := store_sqlite.Open(filepath.Join(t.TempDir(), "graph.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + store = db + } + g := &npmHydrationCountingStore{Store: store, reads: map[string]int{}} + g.AddNode(&graph.Node{ID: "app/a.ts", Kind: graph.KindFile, RepoPrefix: "app"}) + g.AddNode(&graph.Node{ID: "lib/index.ts", Kind: graph.KindFile, RepoPrefix: "lib"}) + g.AddNode(&graph.Node{ID: "lib/index.ts::f", Kind: graph.KindFunction, RepoPrefix: "lib", Meta: map[string]any{"visibility": "public"}}) + var edges []*graph.Edge + for line := 1; line <= resolvePendingPageRows+3; line++ { + e := &graph.Edge{From: "app/a.ts", To: "external::p::f", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: line} + edges = append(edges, e) + g.AddEdge(e) + } + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return mode != "undeclared" }) + if mode == "alias" { + cr.SetNpmAliasResolver(func(_, _ string) string { return "other" }) + } + lookups := 0 + cr.SetNpmPackageLookup(func(_ string) map[string][]string { lookups++; return map[string][]string{"lib": {"lib/index.ts"}} }) + stats := &CrossRepoStats{ByRepo: map[string]int{}} + cr.resolveTrackedNpmImports(graph.EdgesLightSeq(store, graph.EdgeImports), stats) + if mode != "declared" { + assert.Zero(t, g.sites) + assert.Zero(t, lookups, "rejected imports must not trigger manifest lookup") + assert.Zero(t, stats.Resolved) + } else { + assert.Equal(t, len(edges), g.sites) + assert.LessOrEqual(t, g.maxSites, resolvePendingPageRows) + assert.Equal(t, len(edges), stats.Resolved) + assert.Equal(t, 1, g.reads["app/a.ts"]) + assert.Equal(t, 1, g.reads["lib/index.ts::f"]) + } + }) + } + } +} From f6405cd3745acd886368a41e539411e8413c30d9 Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Thu, 22 Oct 2026 14:06:02 -0700 Subject: [PATCH 2/7] fix(resolver): resolve a tracked npm package only through its exports When package.json declares exports, Node resolves the package only through it. The tracked-package lookup still fell back to types, main, the root index or a subpath folder, so it could bind an import that Node refuses: a root the exports map omits, a subpath it does not map, or one it blocks with null. The lookup now records which entries exports declares, independently of the conditions parsePackageExports supports. A declared entry with a supported target binds that target. A declared entry with only other conditions (types, require, node) falls back to the legacy candidates, as before. An entry exports does not declare, or declares as null, stays external. A null exports field counts as absent, as in Node. ResolveAllContext now checks its context while it scans import edges for tracked packages and between hydration pages, and returns the context error. --- internal/indexer/npm_tracked_package.go | 83 ++++++++++++++++---- internal/indexer/npm_tracked_package_test.go | 18 +++++ internal/resolver/cross_repo.go | 6 +- internal/resolver/npm_package.go | 20 +++-- internal/resolver/npm_package_test.go | 58 +++++++++++++- 5 files changed, 162 insertions(+), 23 deletions(-) diff --git a/internal/indexer/npm_tracked_package.go b/internal/indexer/npm_tracked_package.go index cfa9412e4..c0751e037 100644 --- a/internal/indexer/npm_tracked_package.go +++ b/internal/indexer/npm_tracked_package.go @@ -1,6 +1,7 @@ package indexer import ( + "bytes" "encoding/json" "path" "sync" @@ -13,10 +14,12 @@ import ( // resolver captures fresh roots and reads fresh manifests after edits or track/untrack. func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { type entry struct { - dir string - exports *packageExports - types string - main string + dir string + exports *packageExports + declared map[string]string + hasExports bool + types string + main string } roots := map[string]string{} for prefix, meta := range mi.AllMetadata() { @@ -51,15 +54,27 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { src, _ = readDiskFile(joinPath(root, joinRel(dir, "package.json"))) } var manifest struct { - Name string `json:"name"` - Types string `json:"types"` - Main string `json:"main"` + Name string `json:"name"` + Exports json.RawMessage `json:"exports"` + Types string `json:"types"` + Main string `json:"main"` } if json.Unmarshal(src, &manifest) != nil || manifest.Name == "" { continue } + exports := parsePackageExports(src) + declared := declaredNpmExports(manifest.Exports) + // Keep unsupported and blocked keys in the target map so an + // exact key cannot fall through to a broader wildcard target. + if exports != nil { + for key := range declared { + if _, ok := exports.subpaths[key]; !ok { + exports.subpaths[key] = "" + } + } + } packages[manifest.Name] = append(packages[manifest.Name], entry{ - dir: path.Join(prefix, dir), exports: parsePackageExports(src), types: manifest.Types, main: manifest.Main, + dir: path.Join(prefix, dir), exports: exports, declared: declared, hasExports: hasPackageExports(manifest.Exports), types: manifest.Types, main: manifest.Main, }) } } @@ -73,14 +88,23 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { targets := map[string][]string{} for _, pkg := range packages[name] { targets[pkg.dir] = nil - var paths []string + var target string if pkg.exports != nil { - paths = append(paths, resolveExportsSubpath(pkg.exports.subpaths, sub)) + target = resolveExportsSubpath(pkg.exports.subpaths, sub) + } + declaration := resolveExportsSubpath(pkg.declared, sub) + if pkg.hasExports && declaration != "declared" { + continue } - if sub == "" { - paths = append(paths, pkg.types, pkg.main, "index") - } else if pkg.exports == nil { - paths = append(paths, sub) + // A declared entry with unsupported conditions keeps legacy + // candidates; a supported target remains authoritative. + paths := []string{target} + if !pkg.hasExports || target == "" { + if sub == "" { + paths = append(paths, pkg.types, pkg.main, "index") + } else { + paths = append(paths, sub) + } } for _, target := range paths { // Never let manifest paths bind another package/repository. @@ -93,3 +117,34 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { return targets } } + +// declaredNpmExports retains entry keys independently of supported conditions. +// Values are matcher markers, not paths, so wildcard substitution leaves them intact. +func declaredNpmExports(raw json.RawMessage) map[string]string { + var asString string + if json.Unmarshal(raw, &asString) == nil && hasPackageExports(raw) { + return map[string]string{".": "declared"} + } + var entries map[string]json.RawMessage + if json.Unmarshal(raw, &entries) != nil { + return nil + } + if !hasSubpathKeys(entries) { + return map[string]string{".": "declared"} + } + declared := make(map[string]string, len(entries)) + for key, target := range entries { + declared[key] = "declared" + if bytes.Equal(bytes.TrimSpace(target), []byte("null")) { + declared[key] = "blocked" + } + } + return declared +} + +// hasPackageExports reports whether package.json declares exports. Node treats +// a null exports field like an absent one and falls back to main. +func hasPackageExports(raw json.RawMessage) bool { + s := bytes.TrimSpace(raw) + return len(s) > 0 && !bytes.Equal(s, []byte("null")) +} diff --git a/internal/indexer/npm_tracked_package_test.go b/internal/indexer/npm_tracked_package_test.go index bf7580048..2cd070f3d 100644 --- a/internal/indexer/npm_tracked_package_test.go +++ b/internal/indexer/npm_tracked_package_test.go @@ -29,6 +29,24 @@ func TestMultiIndexer_TrackedNpmPackage(t *testing.T) { {name: "subpath relative", packageName: "p", specifier: "p/utils", entry: "utils/index.ts", resolves: true}, {name: "subpath wildcard", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":"./src/*.ts"}`, entry: "src/utils.ts", resolves: true}, {name: "unmapped subpath refused", packageName: "p", specifier: "p/private", manifest: `"exports":{".":"./index.ts"}`, entry: "private.ts"}, + {name: "exports missing target blocks types", packageName: "p", manifest: `"exports":"./missing.ts","types":"./types.ts"`, entry: "types.ts"}, + {name: "exports missing target blocks main", packageName: "p", manifest: `"exports":"./missing.ts","main":"./main.ts"`, entry: "main.ts"}, + {name: "exports missing target blocks index", packageName: "p", manifest: `"exports":"./missing.ts"`, entry: "index.ts"}, + {name: "exports subpath only blocks root", packageName: "p", manifest: `"exports":{"./utils":"./utils.ts"},"types":"./index.ts","main":"./index.ts"`, entry: "index.ts"}, + {name: "exports empty object blocks root", packageName: "p", manifest: `"exports":{}`, entry: "index.ts"}, + {name: "exports null uses legacy index", packageName: "p", manifest: `"exports":null`, entry: "index.ts", resolves: true}, + {name: "exports empty string blocks subpath", packageName: "p", specifier: "p/private", manifest: `"exports":""`, entry: "private.ts"}, + {name: "exports root-only unsupported conditions block subpath", packageName: "p", specifier: "p/private", manifest: `"exports":{"browser":"./index.ts"}`, entry: "private.ts"}, + {name: "exports blocked subpath", packageName: "p", specifier: "p/private", manifest: `"exports":{"./private":null}`, entry: "private.ts"}, + {name: "exports types require root uses legacy main", packageName: "p", manifest: `"main":"./src/index.ts","exports":{".":{"types":"./src/index.ts","require":"./dist/index.cjs"}}`, entry: "src/index.ts", resolves: true}, + {name: "exports types require root uses legacy types", packageName: "p", manifest: `"types":"./src/index.ts","exports":{".":{"types":"./dist/index.d.ts","require":"./dist/index.cjs"}}`, entry: "src/index.ts", resolves: true}, + {name: "exports require subpath uses legacy path", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./utils":{"require":"./dist/utils.cjs"}}`, entry: "utils/index.ts", resolves: true}, + {name: "exports browser root uses legacy index", packageName: "p", manifest: `"exports":{"browser":"./dist/index.js"}`, entry: "index.ts", resolves: true}, + {name: "exports require wildcard uses legacy path", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":{"require":"./dist/*.cjs"}}`, entry: "utils/index.ts", resolves: true}, + {name: "exports null overrides wildcard", packageName: "p", specifier: "p/private", manifest: `"exports":{"./*":"./src/*.ts","./private":null}`, entry: "src/private.ts"}, + {name: "exports null wildcard blocks legacy", packageName: "p", specifier: "p/private", manifest: `"exports":{"./*":null}`, entry: "private.ts"}, + {name: "exports require exact overrides wildcard", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":"./wrong/*.ts","./utils":{"require":"./dist/utils.cjs"}}`, entry: "utils/index.ts", resolves: true}, + {name: "exports conditional root", packageName: "p", manifest: `"exports":{"import":"./src/index.ts"}`, entry: "src/index.ts", resolves: true}, {name: "npm alias collision refused", packageName: "p", depVersion: "npm:other@^1", entry: "index.ts"}, {name: "types", packageName: "p", manifest: `"types":"./src/types.ts","main":"./wrong.ts"`, entry: "src/types.ts", resolves: true}, {name: "main", packageName: "p", manifest: `"main":"./src/main.ts"`, entry: "src/main.ts", resolves: true}, diff --git a/internal/resolver/cross_repo.go b/internal/resolver/cross_repo.go index 951d8728e..e761c393f 100644 --- a/internal/resolver/cross_repo.go +++ b/internal/resolver/cross_repo.go @@ -390,7 +390,9 @@ func (cr *CrossRepoResolver) ResolveAllContext(ctx context.Context) (*CrossRepoS // Every exit applies the moves the pass deferred (a no-op after the // normal flush below); it runs before the deferred unlock above. defer cr.placeholderSrcIdx.applyDeferred(cr.graph) - cr.resolveTrackedNpmImports(graph.EdgesLightSeq(cr.graph, graph.EdgeImports), stats) + if err := cr.resolveTrackedNpmImports(ctx, graph.EdgesLightSeq(cr.graph, graph.EdgeImports), stats); err != nil { + return stats, err + } // Share the master resolver's stable high-water/keyset stream. The cold // residual can exceed 200k edges; retaining it plus the cross-repo name, // raw-name, repo and qualified-name caches was the second whole-corpus heap @@ -675,7 +677,7 @@ func (cr *CrossRepoResolver) ResolveForFile(repoPrefix, relPath string) *CrossRe // scopes. Caller holds cr.mu. func (cr *CrossRepoResolver) resolveScopedLocked(edges []*graph.Edge) *CrossRepoStats { stats := &CrossRepoStats{ByRepo: make(map[string]int)} - cr.resolveTrackedNpmImports(slices.Values(edges), stats) + _ = cr.resolveTrackedNpmImports(context.Background(), slices.Values(edges), stats) pending := make([]*graph.Edge, 0, len(edges)) for _, edge := range edges { if edge != nil && graph.IsUnresolvedTarget(edge.To) { diff --git a/internal/resolver/npm_package.go b/internal/resolver/npm_package.go index 943fc1df6..3d912be04 100644 --- a/internal/resolver/npm_package.go +++ b/internal/resolver/npm_package.go @@ -1,6 +1,7 @@ package resolver import ( + "context" "iter" "strings" @@ -8,9 +9,9 @@ import ( ) // NpmPackageLookup maps a bare npm specifier to tracked package directories. -// Each directory has ordered graph-path entry candidates (exports, types, main, -// then the directory index). The indexer owns manifest reads; the resolver -// probes indexed files and applies repository/workspace policy. +// Each directory has graph-path entry candidates from exports, or legacy +// types/main/directory-index entry points only when exports is absent. The indexer +// owns manifest reads; the resolver probes indexed files and applies repository/workspace policy. type NpmPackageLookup func(specifier string) map[string][]string // SetNpmPackageLookup installs tracked-package evidence for the cross-repo pass. @@ -89,14 +90,17 @@ func (cr *CrossRepoResolver) trackedNpmImport(callerRepo, callerWS, importPath s // The per-repo pass has already made unindexed npm imports external. Retry only // imports with positive tracked-package evidence before building call reachability. -func (cr *CrossRepoResolver) resolveTrackedNpmImports(edges iter.Seq[*graph.Edge], stats *CrossRepoStats) { +func (cr *CrossRepoResolver) resolveTrackedNpmImports(ctx context.Context, edges iter.Seq[*graph.Edge], stats *CrossRepoStats) error { if cr.npmPackages == nil { - return + return ctx.Err() } // Scan metadata-free projections. Hydrate only imports whose package name // is tracked, so ordinary node_modules dependencies stay on the cheap path. var eligible []*graph.Edge for e := range edges { + if err := ctx.Err(); err != nil { + return err + } spec := trackedNpmImportSpec(e) if spec == "" || !cr.eligibleTrackedNpmImport(e, spec) { continue @@ -108,12 +112,15 @@ func (cr *CrossRepoResolver) resolveTrackedNpmImports(edges iter.Seq[*graph.Edge eligible = append(eligible, e) } if len(eligible) == 0 { - return + return ctx.Err() } memo := &npmImportMemo{probes: map[npmProbeKey]*graph.Node{}, nodes: map[string]*graph.Node{}} // Close the light-edge cursor before reading or writing full edges. Hydrate // only one bounded page at a time; metadata never accumulates across pages. for start := 0; start < len(eligible); start += resolvePendingPageRows { + if err := ctx.Err(); err != nil { + return err + } page := eligible[start:min(start+resolvePendingPageRows, len(eligible))] sites := make([]graph.EdgeSite, 0, len(page)) for _, e := range page { @@ -137,6 +144,7 @@ func (cr *CrossRepoResolver) resolveTrackedNpmImports(edges iter.Seq[*graph.Edge cr.graph.ReindexEdges(batch) } } + return ctx.Err() } func trackedNpmImportSpec(e *graph.Edge) string { diff --git a/internal/resolver/npm_package_test.go b/internal/resolver/npm_package_test.go index 6b5837a9d..98969172b 100644 --- a/internal/resolver/npm_package_test.go +++ b/internal/resolver/npm_package_test.go @@ -1,6 +1,7 @@ package resolver import ( + "context" "path/filepath" "testing" @@ -233,7 +234,7 @@ func TestCrossRepoTrackedNpmPackageHydration(t *testing.T) { lookups := 0 cr.SetNpmPackageLookup(func(_ string) map[string][]string { lookups++; return map[string][]string{"lib": {"lib/index.ts"}} }) stats := &CrossRepoStats{ByRepo: map[string]int{}} - cr.resolveTrackedNpmImports(graph.EdgesLightSeq(store, graph.EdgeImports), stats) + require.NoError(t, cr.resolveTrackedNpmImports(context.Background(), graph.EdgesLightSeq(store, graph.EdgeImports), stats)) if mode != "declared" { assert.Zero(t, g.sites) assert.Zero(t, lookups, "rejected imports must not trigger manifest lookup") @@ -249,3 +250,58 @@ func TestCrossRepoTrackedNpmPackageHydration(t *testing.T) { } } } + +// Cancellation must stop the npm prepass, not only the residual resolver. +type npmCancelHydrationStore struct { + graph.Store + cancel context.CancelFunc + pages int +} + +func (g *npmCancelHydrationStore) GetEdgeCandidates(endpoints []graph.EdgeEndpoint, sites []graph.EdgeSite) graph.EdgeCandidateSet { + g.pages++ + g.cancel() + return g.Store.GetEdgeCandidates(endpoints, sites) +} + +func TestCrossRepoTrackedNpmPackageCancellation(t *testing.T) { + for _, phase := range []string{"before", "scan", "hydrate"} { + t.Run(phase, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + g := &npmCancelHydrationStore{Store: graph.New(), cancel: cancel} + g.AddNode(&graph.Node{ID: "app/a.ts", Kind: graph.KindFile, RepoPrefix: "app"}) + g.AddNode(&graph.Node{ID: "lib/index.ts", Kind: graph.KindFile, RepoPrefix: "lib"}) + for line := 1; line <= resolvePendingPageRows+3; line++ { + g.AddEdge(&graph.Edge{From: "app/a.ts", To: "external::p", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: line}) + } + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return true }) + lookups := 0 + cr.SetNpmPackageLookup(func(_ string) map[string][]string { + lookups++ + if phase == "scan" { + cancel() + } + return map[string][]string{"lib": {"lib/index.ts"}} + }) + if phase == "before" { + cancel() + } + stats, err := cr.ResolveAllContext(ctx) + require.ErrorIs(t, err, context.Canceled) + if phase == "hydrate" { + assert.Equal(t, 1, g.pages, "must not hydrate another page after cancellation") + assert.LessOrEqual(t, stats.Resolved, resolvePendingPageRows) + } else { + assert.Zero(t, g.pages) + assert.Zero(t, stats.Resolved) + if phase == "scan" { + assert.Equal(t, 1, lookups, "must stop scanning after cancellation") + } else { + assert.Zero(t, lookups) + } + } + }) + } +} From 6a53e1e013b8f5e321cee8aaa68d8b319c6f0aad Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Thu, 22 Oct 2026 14:31:52 -0700 Subject: [PATCH 3/7] fix(resolver): read a top-level npm exports array as ordered root targets A tracked package whose exports field is an array had no declared entries, so every import of it stayed external. Node reads such an array as an ordered list of root targets and takes the first supported one before it looks at the filesystem. The lookup now declares the root for an array and picks its first item that is a "./" path or a conditional object with a condition the indexer reads, searching nested arrays in order and skipping null and invalid items. That target is authoritative, as for other exports entries. An array whose items are all null, or an empty array, blocks the root. An array with only unsupported conditions keeps the legacy candidates. Subpaths stay unexported. The NpmPackageLookup comment now states that legacy candidates also apply to a declared entry without a supported target. --- internal/indexer/npm_tracked_package.go | 60 +++++++++++++++++++- internal/indexer/npm_tracked_package_test.go | 8 +++ internal/resolver/npm_package.go | 8 ++- 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/internal/indexer/npm_tracked_package.go b/internal/indexer/npm_tracked_package.go index c0751e037..a14540e3d 100644 --- a/internal/indexer/npm_tracked_package.go +++ b/internal/indexer/npm_tracked_package.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "path" + "strings" "sync" "github.com/zzet/gortex/internal/modules" @@ -17,6 +18,7 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { dir string exports *packageExports declared map[string]string + arrayRoot npmArrayRoot hasExports bool types string main string @@ -74,7 +76,7 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { } } packages[manifest.Name] = append(packages[manifest.Name], entry{ - dir: path.Join(prefix, dir), exports: exports, declared: declared, hasExports: hasPackageExports(manifest.Exports), types: manifest.Types, main: manifest.Main, + dir: path.Join(prefix, dir), exports: exports, declared: declared, arrayRoot: npmExportsArrayRoot(manifest.Exports), hasExports: hasPackageExports(manifest.Exports), types: manifest.Types, main: manifest.Main, }) } } @@ -96,6 +98,14 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { if pkg.hasExports && declaration != "declared" { continue } + if sub == "" && pkg.arrayRoot.isArray { + // Node picks the first supported array target before it looks at + // the filesystem; an array of only null items blocks the root. + if pkg.arrayRoot.blocked { + continue + } + target = pkg.arrayRoot.target + } // A declared entry with unsupported conditions keeps legacy // candidates; a supported target remains authoritative. paths := []string{target} @@ -125,6 +135,9 @@ func declaredNpmExports(raw json.RawMessage) map[string]string { if json.Unmarshal(raw, &asString) == nil && hasPackageExports(raw) { return map[string]string{".": "declared"} } + if npmExportsArrayRoot(raw).isArray { + return map[string]string{".": "declared"} + } var entries map[string]json.RawMessage if json.Unmarshal(raw, &entries) != nil { return nil @@ -148,3 +161,48 @@ func hasPackageExports(raw json.RawMessage) bool { s := bytes.TrimSpace(raw) return len(s) > 0 && !bytes.Equal(s, []byte("null")) } + +// npmArrayRoot describes a top-level exports array, which Node reads as an +// ordered list of root targets. +type npmArrayRoot struct { + isArray bool + target string // first supported target, "" when none + blocked bool // no item names a target, not even an unsupported one +} + +// npmExportsArrayRoot picks the first supported target of a top-level exports +// array, as Node does before it checks the filesystem. Nested arrays are +// searched in order; items that are not "./" paths are skipped. When no item +// is supported but some item has only conditions the indexer does not read, +// the root is neither targeted nor blocked, so legacy candidates apply. +func npmExportsArrayRoot(raw json.RawMessage) npmArrayRoot { + var items []json.RawMessage + if json.Unmarshal(raw, &items) != nil || items == nil { + return npmArrayRoot{} + } + target, unsupported := firstNpmArrayTarget(items) + return npmArrayRoot{isArray: true, target: target, blocked: target == "" && !unsupported} +} + +func firstNpmArrayTarget(items []json.RawMessage) (target string, unsupported bool) { + for _, item := range items { + var nested []json.RawMessage + if json.Unmarshal(item, &nested) == nil && nested != nil { + t, u := firstNpmArrayTarget(nested) + if t != "" { + return t, false + } + unsupported = unsupported || u + continue + } + var conditions map[string]json.RawMessage + isObject := json.Unmarshal(item, &conditions) == nil && conditions != nil + if t := pickConditionalTarget(item); strings.HasPrefix(t, "./") { + return t, false + } + if isObject { + unsupported = true + } + } + return "", unsupported +} diff --git a/internal/indexer/npm_tracked_package_test.go b/internal/indexer/npm_tracked_package_test.go index 2cd070f3d..56ec2d442 100644 --- a/internal/indexer/npm_tracked_package_test.go +++ b/internal/indexer/npm_tracked_package_test.go @@ -47,6 +47,14 @@ func TestMultiIndexer_TrackedNpmPackage(t *testing.T) { {name: "exports null wildcard blocks legacy", packageName: "p", specifier: "p/private", manifest: `"exports":{"./*":null}`, entry: "private.ts"}, {name: "exports require exact overrides wildcard", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":"./wrong/*.ts","./utils":{"require":"./dist/utils.cjs"}}`, entry: "utils/index.ts", resolves: true}, {name: "exports conditional root", packageName: "p", manifest: `"exports":{"import":"./src/index.ts"}`, entry: "src/index.ts", resolves: true}, + {name: "exports array root", packageName: "p", manifest: `"exports":["./src/index.ts"]`, entry: "src/index.ts", resolves: true}, + {name: "exports array first target is authoritative", packageName: "p", manifest: `"exports":["./missing.ts","./src/index.ts"],"main":"./src/index.ts"`, entry: "src/index.ts"}, + {name: "exports array skips invalid and null items", packageName: "p", manifest: `"exports":[null,"src/x.ts",{"import":"./src/index.ts"}]`, entry: "src/index.ts", resolves: true}, + {name: "exports nested array", packageName: "p", manifest: `"exports":[["./src/index.ts"]]`, entry: "src/index.ts", resolves: true}, + {name: "exports empty array blocks root", packageName: "p", manifest: `"exports":[]`, entry: "index.ts"}, + {name: "exports null-only array blocks root", packageName: "p", manifest: `"exports":[null],"main":"./index.ts"`, entry: "index.ts"}, + {name: "exports array with unsupported conditions uses legacy", packageName: "p", manifest: `"exports":[{"require":"./dist/index.cjs"}],"main":"./src/main.ts"`, entry: "src/main.ts", resolves: true}, + {name: "exports array blocks subpath", packageName: "p", specifier: "p/private", manifest: `"exports":["./index.ts"]`, entry: "private.ts"}, {name: "npm alias collision refused", packageName: "p", depVersion: "npm:other@^1", entry: "index.ts"}, {name: "types", packageName: "p", manifest: `"types":"./src/types.ts","main":"./wrong.ts"`, entry: "src/types.ts", resolves: true}, {name: "main", packageName: "p", manifest: `"main":"./src/main.ts"`, entry: "src/main.ts", resolves: true}, diff --git a/internal/resolver/npm_package.go b/internal/resolver/npm_package.go index 3d912be04..c43bb17ba 100644 --- a/internal/resolver/npm_package.go +++ b/internal/resolver/npm_package.go @@ -9,9 +9,11 @@ import ( ) // NpmPackageLookup maps a bare npm specifier to tracked package directories. -// Each directory has graph-path entry candidates from exports, or legacy -// types/main/directory-index entry points only when exports is absent. The indexer -// owns manifest reads; the resolver probes indexed files and applies repository/workspace policy. +// Each directory has graph-path entry candidates from exports. Legacy +// types/main/directory-index entry points are added when exports is absent, and +// when exports declares the requested entry but gives no target the indexer +// supports (for example only `types` or `require` conditions). The indexer owns +// manifest reads; the resolver probes indexed files and applies repository/workspace policy. type NpmPackageLookup func(specifier string) map[string][]string // SetNpmPackageLookup installs tracked-package evidence for the cross-repo pass. From 9db30328210d4bf9fc9e179ef102a90871072457 Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Thu, 22 Oct 2026 14:43:06 -0700 Subject: [PATCH 4/7] fix(resolver): block the npm root for a recognized null exports condition An exports array item such as {"import": null} names a condition the indexer reads, but its target is null or invalid. Node resolves such an item to nothing, so the root stays unexported. The lookup treated it like an object with only unknown conditions and fell back to main or the root index. Only objects without a recognized condition now leave the root to the legacy candidates. --- internal/indexer/npm_tracked_package.go | 13 ++++++++++++- internal/indexer/npm_tracked_package_test.go | 2 ++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/indexer/npm_tracked_package.go b/internal/indexer/npm_tracked_package.go index a14540e3d..2c0854d25 100644 --- a/internal/indexer/npm_tracked_package.go +++ b/internal/indexer/npm_tracked_package.go @@ -200,9 +200,20 @@ func firstNpmArrayTarget(items []json.RawMessage) (target string, unsupported bo if t := pickConditionalTarget(item); strings.HasPrefix(t, "./") { return t, false } - if isObject { + // Only conditions the indexer cannot read leave the root to legacy + // candidates; a recognized condition with a null or invalid target + // resolves to nothing, as in Node. + if isObject && !hasRecognizedNpmCondition(conditions) { unsupported = true } } return "", unsupported } + +// hasRecognizedNpmCondition reports whether a conditional exports object +// names a condition pickConditionalTarget reads. +func hasRecognizedNpmCondition(conditions map[string]json.RawMessage) bool { + _, imp := conditions["import"] + _, def := conditions["default"] + return imp || def +} diff --git a/internal/indexer/npm_tracked_package_test.go b/internal/indexer/npm_tracked_package_test.go index 56ec2d442..fabc180cf 100644 --- a/internal/indexer/npm_tracked_package_test.go +++ b/internal/indexer/npm_tracked_package_test.go @@ -53,6 +53,8 @@ func TestMultiIndexer_TrackedNpmPackage(t *testing.T) { {name: "exports nested array", packageName: "p", manifest: `"exports":[["./src/index.ts"]]`, entry: "src/index.ts", resolves: true}, {name: "exports empty array blocks root", packageName: "p", manifest: `"exports":[]`, entry: "index.ts"}, {name: "exports null-only array blocks root", packageName: "p", manifest: `"exports":[null],"main":"./index.ts"`, entry: "index.ts"}, + {name: "exports array recognized null condition blocks root", packageName: "p", manifest: `"exports":[{"import":null}],"main":"./index.ts"`, entry: "index.ts"}, + {name: "exports array recognized invalid condition blocks root", packageName: "p", manifest: `"exports":[{"default":"index.ts"}],"main":"./index.ts"`, entry: "index.ts"}, {name: "exports array with unsupported conditions uses legacy", packageName: "p", manifest: `"exports":[{"require":"./dist/index.cjs"}],"main":"./src/main.ts"`, entry: "src/main.ts", resolves: true}, {name: "exports array blocks subpath", packageName: "p", specifier: "p/private", manifest: `"exports":["./index.ts"]`, entry: "private.ts"}, {name: "npm alias collision refused", packageName: "p", depVersion: "npm:other@^1", entry: "index.ts"}, From 6aa9d9a55b5c9e464d378e32f891689a5b4fa8a8 Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Fri, 23 Oct 2026 12:54:02 -0700 Subject: [PATCH 5/7] fix(resolver): bind npm calls through the file's own imports and keep links on provider changes Review follow-up. A linked import no longer makes the whole provider repo match bare calls by name: JS/TS calls resolve through the caller file's own per-binding imports (renamed imports by their local alias, recorded as import_alias in edge Meta), and npm-linked repos no longer count as reachable for plain name matching. Default-import calls stay unresolved. Ambiguity counts the manifests that claim a package in each tier before probing, so a fork cannot win when the real package's entry is not indexed. Linked imports keep their specifier; provider saves, full reindexes and untracks park them as external:: with their calls, and the whole-graph pass re-checks linked edges and reverts those the lookup no longer selects, removing stale cross_repo_calls edges. Also: the types condition is tried when the selected target is not indexed, no fallback probe for exports entries with only unsupported conditions, a test that an own-workspace provider wins, and docs. --- docs/multi-repo.md | 5 +- internal/indexer/incremental_batch.go | 1 + internal/indexer/indexer.go | 3 +- internal/indexer/npm_import_restub.go | 127 +++++++ internal/indexer/npm_lifecycle_test.go | 120 +++++++ internal/indexer/npm_tracked_package.go | 74 ++++- internal/indexer/npm_tracked_package_test.go | 247 ++++++++++---- internal/indexer/repo_eviction_scope.go | 1 + internal/indexer/repository_untrack.go | 1 + .../parser/languages/helpers_jsimports.go | 1 + .../languages/per_binding_imports_test.go | 6 + internal/resolver/cross_repo.go | 101 ++++-- internal/resolver/npm_package.go | 160 +++++++-- internal/resolver/npm_package_test.go | 311 ++++++++++++++++-- 14 files changed, 994 insertions(+), 164 deletions(-) create mode 100644 internal/indexer/npm_import_restub.go create mode 100644 internal/indexer/npm_lifecycle_test.go diff --git a/docs/multi-repo.md b/docs/multi-repo.md index a0d571155..1023c8b78 100644 --- a/docs/multi-repo.md +++ b/docs/multi-repo.md @@ -445,6 +445,9 @@ Scoped tool responses carry a `scope_applied` meta field plus a one-line widen h ## How it works - **Qualified node IDs** — IDs are always `/::` (e.g., `frontend/src/app.ts::App`), including for a workspace tracking a single repo: a lone repo is simply the first tracked repo. Tools still accept an unqualified path (`src/app.ts`) when exactly one repo is tracked, since there is then nothing to be ambiguous about. -- **Cross-repo edges** — the resolver links symbols across repo boundaries with same-repo preference. Cross-repo edges carry a `cross_repo: true` flag. +- **Cross-repo edges** — the resolver links symbols across repo boundaries with same-repo preference. Cross-repo edges carry a `cross_repo: true` flag. JS/TS imports can link a declared npm dependency to a tracked repository whose `package.json` claims that package name, including npm and pnpm workspace members. A provider in the caller's workspace wins over eligible foreign providers. Multiple manifest claims in the winning tier keep the import external, even if only one entry is indexed. + - Entry lookup respects `exports` keys, wildcard subpaths and `null` blocks. It probes the selected `import`/`default` target first, then a `types` condition when that file is not indexed. Without `exports`, it tries `types`, `main` and directory indexes. Unsupported-condition subpaths never guess a directory target. + - Bare calls use the caller file's named import binding, including its local alias, not another same-named function anywhere in the provider. A per-binding import must resolve to a symbol marked public by the extractor. Linked imports retain their specifier, survive provider eviction as external imports, and are rechecked by whole-graph resolution. + - **Limits:** Build-output-only entries, such as `main` and `types` pointing into excluded `dist/`, stay external unless an indexed target is declared. Gortex does not infer `src/` from `dist/`. The importing repository's own packages, `npm:` aliases and workspace paths that escape the repository are not linked by this pass. Sibling dependencies declared with `file:`, `link:` or `portal:` are not supported. Default-import bare calls have no exact binding evidence yet and stay unresolved. Existing linked imports from older builds need the consumer file reindexed to acquire the specifier and alias metadata. Namespace member calls, CommonJS `require` calls and package re-exports are not linked by this npm import pass. - **Impact analysis** — `explain_change_impact`, `verify_change`, and `get_test_targets` follow cross-repo edges automatically, grouping results by repository. - **Shared repos** — the same repo can appear in multiple projects with different reference tags. It's indexed once and shared across projects. diff --git a/internal/indexer/incremental_batch.go b/internal/indexer/incremental_batch.go index 63003d45c..8caac04db 100644 --- a/internal/indexer/incremental_batch.go +++ b/internal/indexer/incremental_batch.go @@ -1352,6 +1352,7 @@ func restubIncomingRefsFromView( ) []*graph.Edge { evicted := structuralPriorIDs(stages) reparsed := reparsedPaths(stages) + restubTrackedNpmImportsFromView(g, evicted, view.inByNode) var reindexes []graph.EdgeReindex var carried []*graph.Edge carriedSeen := make(map[*graph.Edge]struct{}) diff --git a/internal/indexer/indexer.go b/internal/indexer/indexer.go index 3d9ba3d18..dc00096da 100644 --- a/internal/indexer/indexer.go +++ b/internal/indexer/indexer.go @@ -5878,7 +5878,7 @@ func (idx *Indexer) restubIncomingRefs(graphPath string) { continue } evicted[n.ID] = struct{}{} - if n.Name != "" && graph.IsReferenceableSymbol(n.Kind) { + if n.Kind == graph.KindFile || (n.Name != "" && graph.IsReferenceableSymbol(n.Kind)) { refIDs = append(refIDs, n.ID) } } @@ -5889,6 +5889,7 @@ func (idx *Indexer) restubIncomingRefs(graphPath string) { // operation. On SQLite this is chunked under the parameter limit; more // importantly, it avoids one transaction/query per definition in the file. inEdges := idx.graph.GetInEdgesByNodeIDs(refIDs) + restubTrackedNpmImportsFromView(idx.graph, evicted, inEdges) var batch []graph.EdgeReindex for _, n := range nodes { if n == nil || n.Name == "" || !graph.IsReferenceableSymbol(n.Kind) { diff --git a/internal/indexer/npm_import_restub.go b/internal/indexer/npm_import_restub.go new file mode 100644 index 000000000..ac2c9e199 --- /dev/null +++ b/internal/indexer/npm_import_restub.go @@ -0,0 +1,127 @@ +package indexer + +import ( + "strings" + + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/resolver" +) + +// Park only npm-linked incoming imports and their calls before replacing or +// removing a provider. Other references keep their existing eviction rules. +func restubTrackedNpmRepoIncoming(g graph.Store, repo string) { + if repo == "" { + return + } + var ids []string + flush := func() { + identities := graph.InEdgeIdentitiesByNodeIDs(g, ids) + var sites []graph.EdgeSite + for _, edges := range identities { + for _, edge := range edges { + if edge.Kind == graph.EdgeImports && graph.RepoPrefixOfID(edge.From) != repo { + sites = append(sites, graph.EdgeSite{From: edge.From, Line: edge.Line, Kind: graph.EdgeImports}) + } + } + } + if len(sites) == 0 { + ids = ids[:0] + return + } + imports := g.GetEdgeCandidates(nil, sites) + var targets []string + for target, edges := range identities { + marked := false + for _, edge := range edges { + if edge.Kind != graph.EdgeImports || graph.RepoPrefixOfID(edge.From) == repo { + continue + } + for _, candidate := range imports.Site(edge.From, edge.Line, graph.EdgeImports) { + if candidate.To == target { + if spec, _ := candidate.Meta[resolver.NpmImportSpecifierMeta].(string); spec != "" { + marked = true + } + } + } + } + if marked { + targets = append(targets, target) + } + } + if len(targets) > 0 { + incoming := g.GetInEdgesByNodeIDs(targets) + evicted := make(map[string]struct{}) + for _, edges := range incoming { + for _, edge := range edges { + if edge != nil && graph.RepoPrefixOfID(edge.From) == repo { + evicted[edge.From] = struct{}{} + } + } + } + restubTrackedNpmImportsFromView(g, evicted, incoming) + } + ids = ids[:0] + } + for node := range graph.RepoNodeIdentitiesSeq(g, []string{repo}, + graph.KindFile, graph.KindFunction, graph.KindMethod, graph.KindType, + graph.KindInterface, graph.KindVariable, graph.KindConstant) { + ids = append(ids, node.ID) + if len(ids) == 512 { + flush() + } + } + if len(ids) > 0 { + flush() + } +} + +func restubTrackedNpmImportsFromView(g graph.Store, evicted map[string]struct{}, incoming map[string][]*graph.Edge) { + consumers := make(map[string]map[string]string) + var batch []graph.EdgeReindex + for _, edges := range incoming { + for _, edge := range edges { + if edge == nil || edge.Kind != graph.EdgeImports { + continue + } + spec, _ := edge.Meta[resolver.NpmImportSpecifierMeta].(string) + if _, sourceEvicted := evicted[edge.From]; spec == "" || sourceEvicted { + continue + } + oldTo := edge.To + if _, name, symbol := strings.Cut(oldTo, "::"); symbol { + if alias, _ := edge.Meta["import_alias"].(string); alias != "" { + name = alias + } + if consumers[oldTo] == nil { + consumers[oldTo] = make(map[string]string) + } + consumers[oldTo][edge.From] = name + } + edge.To = "external::" + spec + edge.CrossRepo = false + batch = append(batch, graph.EdgeReindex{Edge: edge, OldTo: oldTo}) + } + } + for target, files := range consumers { + for _, edge := range incoming[target] { + if edge == nil || edge.Kind != graph.EdgeCalls || !edge.CrossRepo { + continue + } + if _, sourceEvicted := evicted[edge.From]; sourceEvicted { + continue + } + name, imported := files[edge.FilePath] + if !imported { + continue + } + graph.StashRestubProvenance(edge) + edge.To = graph.UnresolvedMarker + name + edge.CrossRepo, edge.ConfidenceLabel = false, "" + batch = append(batch, graph.EdgeReindex{Edge: edge, OldTo: target}) + } + } + + if len(batch) > 0 { + g.ReindexEdges(batch) + } +} diff --git a/internal/indexer/npm_lifecycle_test.go b/internal/indexer/npm_lifecycle_test.go new file mode 100644 index 000000000..e47852b23 --- /dev/null +++ b/internal/indexer/npm_lifecycle_test.go @@ -0,0 +1,120 @@ +package indexer + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/graph/store_sqlite" + "go.uber.org/zap" +) + +func TestTrackedNpmImportsSurviveEviction(t *testing.T) { + for _, backend := range []string{"memory", "sqlite"} { + for _, mode := range []string{"file", "batch", "replace", "untrack"} { + t.Run(backend+"/"+mode, func(t *testing.T) { + var g graph.Store = graph.New() + if backend == "sqlite" { + db, err := store_sqlite.Open(filepath.Join(t.TempDir(), "graph.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + g = db + } + nodes := []*graph.Node{ + {ID: "lib/index.ts", Kind: graph.KindFile, FilePath: "lib/index.ts", RepoPrefix: "lib"}, + {ID: "lib/index.ts::f", Kind: graph.KindFunction, Name: "f", FilePath: "lib/index.ts", RepoPrefix: "lib"}, + } + g.AddBatch(nodes, []*graph.Edge{ + {From: "app/a.ts", To: "lib/index.ts", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1, CrossRepo: true, Meta: map[string]any{"npm_import": "p"}}, + {From: "app/a.ts", To: "lib/index.ts::f", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1, CrossRepo: true, Meta: map[string]any{"npm_import": "p::f"}}, + {From: "app/a.ts", To: "lib/index.ts", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 2}, + }) + switch mode { + case "file": + idx := New(g, newTestRegistry(), config.IndexConfig{}, zap.NewNop()) + idx.restubIncomingRefs("lib/index.ts") + g.EvictFile("lib/index.ts") + case "batch": + stages := []*incrementalBatchStage{{graphPath: "lib/index.ts", priorNodes: nodes}} + restubIncomingRefsFromView(g, stages, loadIncrementalPriorView(g, stages)) + g.EvictFile("lib/index.ts") + case "replace": + evictRepoCurrentGeneration(g, "lib") + case "untrack": + mi := &MultiIndexer{graph: g} + _, _, err := mi.purgeRepositoryPayload("lib", nil) + require.NoError(t, err) + } + var targets []string + for _, e := range g.GetOutEdges("app/a.ts") { + require.Equal(t, 1, e.Line, "unmarked relative import must still be removed") + targets = append(targets, e.To) + assert.False(t, e.CrossRepo) + } + assert.ElementsMatch(t, []string{"external::p", "external::p::f"}, targets) + }) + } + } +} + +func TestTrackedNpmRepoParkingIsScoped(t *testing.T) { + for _, marked := range []bool{false, true} { + t.Run(map[bool]string{false: "not npm provider", true: "npm provider"}[marked], func(t *testing.T) { + g := graph.New() + g.AddNode(&graph.Node{ID: "lib/index.ts::f", Kind: graph.KindFunction, Name: "f", RepoPrefix: "lib"}) + meta := map[string]any{} + if marked { + meta["npm_import"] = "p::f" + } + g.AddEdge(&graph.Edge{From: "app/a.ts", To: "lib/index.ts::f", Kind: graph.EdgeImports, FilePath: "app/a.ts", Meta: meta}) + g.AddEdge(&graph.Edge{From: "app/a.ts::run", To: "lib/index.ts::f", Kind: graph.EdgeCalls, FilePath: "app/a.ts", CrossRepo: true}) + g.AddEdge(&graph.Edge{From: "other/a.ts::run", To: "lib/index.ts::f", Kind: graph.EdgeCalls, FilePath: "other/a.ts", CrossRepo: true}) + g.AddEdge(&graph.Edge{From: "app/a.ts::run", To: "lib/index.ts::f", Kind: graph.EdgeReferences, FilePath: "app/a.ts", CrossRepo: true}) + evictRepoCurrentGeneration(g, "lib") + assert.Empty(t, g.GetOutEdges("other/a.ts::run")) + calls := g.GetOutEdges("app/a.ts::run") + if marked { + require.Len(t, calls, 1) + assert.Equal(t, graph.EdgeCalls, calls[0].Kind) + assert.Equal(t, "unresolved::f", calls[0].To) + assert.False(t, calls[0].CrossRepo) + } else { + assert.Empty(t, calls) + } + }) + } +} + +type npmRestubCountingStore struct { + *graph.Graph + incomingReads int +} + +func (g *npmRestubCountingStore) GetInEdgesByNodeIDs(ids []string) map[string][]*graph.Edge { + g.incomingReads++ + return g.Graph.GetInEdgesByNodeIDs(ids) +} + +func TestNonNpmEvictionDoesNotHydrateIncoming(t *testing.T) { + for _, withImport := range []bool{false, true} { + t.Run(map[bool]string{false: "calls only", true: "unmarked import"}[withImport], func(t *testing.T) { + g := &npmRestubCountingStore{Graph: graph.New()} + g.AddBatch([]*graph.Node{ + {ID: "lib/a.go", Kind: graph.KindFile, RepoPrefix: "lib"}, + {ID: "lib/a.go::f", Kind: graph.KindFunction, Name: "f", RepoPrefix: "lib"}, + }, []*graph.Edge{ + {From: "lib/a.go", To: "lib/a.go::f", Kind: graph.EdgeCalls}, + {From: "other/a.go::run", To: "lib/a.go::f", Kind: graph.EdgeCalls, CrossRepo: true}, + }) + if withImport { + g.AddEdge(&graph.Edge{From: "other/a.go", To: "lib/a.go", Kind: graph.EdgeImports, CrossRepo: true}) + } + evictRepoCurrentGeneration(g, "lib") + assert.Zero(t, g.incomingReads, "non-npm eviction must not hydrate incoming adjacency") + assert.Empty(t, g.GetOutEdges("other/a.go::run")) + }) + } +} diff --git a/internal/indexer/npm_tracked_package.go b/internal/indexer/npm_tracked_package.go index 2c0854d25..a07588b82 100644 --- a/internal/indexer/npm_tracked_package.go +++ b/internal/indexer/npm_tracked_package.go @@ -15,18 +15,25 @@ import ( // resolver captures fresh roots and reads fresh manifests after edits or track/untrack. func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { type entry struct { - dir string - exports *packageExports - declared map[string]string - arrayRoot npmArrayRoot - hasExports bool - types string - main string + dir string + workspaceID string + exports *packageExports + declared map[string]string + typesExports map[string]string + arrayRoot npmArrayRoot + hasExports bool + types string + main string } roots := map[string]string{} + workspaces := map[string]string{} for prefix, meta := range mi.AllMetadata() { if meta != nil && meta.RootPath != "" { roots[prefix] = meta.RootPath + workspaces[prefix] = prefix + if idx := mi.GetIndexer(prefix); idx != nil && idx.WorkspaceID() != "" { + workspaces[prefix] = idx.WorkspaceID() + } } } if len(roots) == 0 { @@ -76,20 +83,20 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { } } packages[manifest.Name] = append(packages[manifest.Name], entry{ - dir: path.Join(prefix, dir), exports: exports, declared: declared, arrayRoot: npmExportsArrayRoot(manifest.Exports), hasExports: hasPackageExports(manifest.Exports), types: manifest.Types, main: manifest.Main, + dir: path.Join(prefix, dir), workspaceID: workspaces[prefix], exports: exports, declared: declared, typesExports: npmTypesExportTargets(manifest.Exports), arrayRoot: npmExportsArrayRoot(manifest.Exports), hasExports: hasPackageExports(manifest.Exports), types: manifest.Types, main: manifest.Main, }) } } } - return func(specifier string) map[string][]string { + return func(specifier string) map[string]resolver.NpmPackageTarget { once.Do(build) name, sub := splitPackageSpecifier(specifier) if name == "" || hasDotDotSegment(sub) || len(packages[name]) == 0 { return nil } - targets := map[string][]string{} + targets := map[string]resolver.NpmPackageTarget{} for _, pkg := range packages[name] { - targets[pkg.dir] = nil + targets[pkg.dir] = resolver.NpmPackageTarget{WorkspaceID: pkg.workspaceID} var target string if pkg.exports != nil { target = resolveExportsSubpath(pkg.exports.subpaths, sub) @@ -106,10 +113,10 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { } target = pkg.arrayRoot.target } - // A declared entry with unsupported conditions keeps legacy - // candidates; a supported target remains authoritative. - paths := []string{target} - if !pkg.hasExports || target == "" { + // Root entries with unsupported conditions keep legacy candidates. + // A subpath must name its target; never invent a directory probe. + paths := []string{target, resolveExportsSubpath(pkg.typesExports, sub)} + if !pkg.hasExports || (target == "" && sub == "") { if sub == "" { paths = append(paths, pkg.types, pkg.main, "index") } else { @@ -121,7 +128,9 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { if target == "" || path.IsAbs(target) || hasDotDotSegment(target) { continue } - targets[pkg.dir] = append(targets[pkg.dir], path.Join(pkg.dir, target)) + candidate := targets[pkg.dir] + candidate.Entries = append(candidate.Entries, path.Join(pkg.dir, target)) + targets[pkg.dir] = candidate } } return targets @@ -217,3 +226,36 @@ func hasRecognizedNpmCondition(conditions map[string]json.RawMessage) bool { _, def := conditions["default"] return imp || def } + +// npmTypesExportTargets keeps declaration-file alternatives separate from the +// runtime target. The resolver tries them only after the runtime file misses. +func npmTypesExportTargets(raw json.RawMessage) map[string]string { + var entries map[string]json.RawMessage + if json.Unmarshal(raw, &entries) != nil || entries == nil { + return nil + } + if !hasSubpathKeys(entries) { + return map[string]string{".": npmTypesCondition(raw)} + } + targets := make(map[string]string, len(entries)) + for key, value := range entries { + targets[key] = npmTypesCondition(value) + } + return targets +} + +func npmTypesCondition(raw json.RawMessage) string { + var conditions map[string]json.RawMessage + if json.Unmarshal(raw, &conditions) != nil { + return "" + } + if target := pickConditionalTarget(conditions["types"]); target != "" { + return target + } + for _, condition := range []string{"import", "default"} { + if target := npmTypesCondition(conditions[condition]); target != "" { + return target + } + } + return "" +} diff --git a/internal/indexer/npm_tracked_package_test.go b/internal/indexer/npm_tracked_package_test.go index fabc180cf..756103d6a 100644 --- a/internal/indexer/npm_tracked_package_test.go +++ b/internal/indexer/npm_tracked_package_test.go @@ -15,6 +15,7 @@ import ( "github.com/zzet/gortex/internal/graph" "github.com/zzet/gortex/internal/graph/store_sqlite" "github.com/zzet/gortex/internal/parser/languages" + "github.com/zzet/gortex/internal/resolver" "github.com/zzet/gortex/internal/search" ) @@ -23,6 +24,9 @@ func TestMultiIndexer_TrackedNpmPackage(t *testing.T) { name, packageName, manifest, entry, packageDir, consumerName, providerWS, providerName, specifier, depVersion string resolves, pnpm bool }{ + {name: "exports missing runtime uses types condition", packageName: "p", manifest: `"exports":{"types":"./src/index.ts","import":"./dist/index.js"}`, entry: "src/index.ts", resolves: true}, + {name: "exports subpath missing runtime uses types condition", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./utils":{"types":"./src/utils.ts","import":"./dist/utils.js"}}`, entry: "src/utils.ts", resolves: true}, + {name: "unsupported subpath does not guess directory", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./utils":{"browser":"./dist/utils.js"}}`, entry: "utils/index.ts"}, {name: "scoped exports", packageName: "@scope/p", manifest: `"exports":"./src/index.ts","types":"./wrong.ts","main":"./wrong.ts"`, entry: "src/index.ts", resolves: true}, {name: "unscoped subpath exports", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./utils":"./src/util.js"}`, entry: "src/util.ts", resolves: true}, {name: "scoped subpath exports", packageName: "@scope/p", specifier: "@scope/p/utils", manifest: `"exports":{"./utils":{"import":"./src/util.ts"}}`, entry: "src/util.ts", resolves: true}, @@ -40,12 +44,12 @@ func TestMultiIndexer_TrackedNpmPackage(t *testing.T) { {name: "exports blocked subpath", packageName: "p", specifier: "p/private", manifest: `"exports":{"./private":null}`, entry: "private.ts"}, {name: "exports types require root uses legacy main", packageName: "p", manifest: `"main":"./src/index.ts","exports":{".":{"types":"./src/index.ts","require":"./dist/index.cjs"}}`, entry: "src/index.ts", resolves: true}, {name: "exports types require root uses legacy types", packageName: "p", manifest: `"types":"./src/index.ts","exports":{".":{"types":"./dist/index.d.ts","require":"./dist/index.cjs"}}`, entry: "src/index.ts", resolves: true}, - {name: "exports require subpath uses legacy path", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./utils":{"require":"./dist/utils.cjs"}}`, entry: "utils/index.ts", resolves: true}, + {name: "exports require subpath uses legacy path", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./utils":{"require":"./dist/utils.cjs"}}`, entry: "utils/index.ts"}, {name: "exports browser root uses legacy index", packageName: "p", manifest: `"exports":{"browser":"./dist/index.js"}`, entry: "index.ts", resolves: true}, - {name: "exports require wildcard uses legacy path", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":{"require":"./dist/*.cjs"}}`, entry: "utils/index.ts", resolves: true}, + {name: "exports require wildcard uses legacy path", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":{"require":"./dist/*.cjs"}}`, entry: "utils/index.ts"}, {name: "exports null overrides wildcard", packageName: "p", specifier: "p/private", manifest: `"exports":{"./*":"./src/*.ts","./private":null}`, entry: "src/private.ts"}, {name: "exports null wildcard blocks legacy", packageName: "p", specifier: "p/private", manifest: `"exports":{"./*":null}`, entry: "private.ts"}, - {name: "exports require exact overrides wildcard", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":"./wrong/*.ts","./utils":{"require":"./dist/utils.cjs"}}`, entry: "utils/index.ts", resolves: true}, + {name: "exports require exact overrides wildcard", packageName: "p", specifier: "p/utils", manifest: `"exports":{"./*":"./wrong/*.ts","./utils":{"require":"./dist/utils.cjs"}}`, entry: "utils/index.ts"}, {name: "exports conditional root", packageName: "p", manifest: `"exports":{"import":"./src/index.ts"}`, entry: "src/index.ts", resolves: true}, {name: "exports array root", packageName: "p", manifest: `"exports":["./src/index.ts"]`, entry: "src/index.ts", resolves: true}, {name: "exports array first target is authoritative", packageName: "p", manifest: `"exports":["./missing.ts","./src/index.ts"],"main":"./src/index.ts"`, entry: "src/index.ts"}, @@ -188,77 +192,124 @@ func TestTrackedNpmPackageWorkspaceEscape(t *testing.T) { assert.Empty(t, lookup("p"), "an escaped workspace must not supply another repo's graph prefix") } -func TestMultiIndexer_TrackedNpmPackageTrackSQLite(t *testing.T) { - for _, providerFirst := range []bool{true, false} { - t.Run(strconv.FormatBool(providerFirst), func(t *testing.T) { - provider, consumer := t.TempDir(), t.TempDir() - writePackageJSON(t, provider, `{"name":"p","exports":"./src/index.ts"}`) - writeWorkspaceFile(t, provider, "src/index.ts", "export function f(): number { return 42; }\n") - writePackageJSON(t, consumer, `{"name":"app","dependencies":{"p":"^1"}}`) - writeWorkspaceFile(t, consumer, "src/app.ts", "import { f } from 'p';\nexport function run(): number { return f(); }\n") - entries := []config.RepoEntry{{Path: provider, Name: "provider", Workspace: "shared"}, {Path: consumer, Name: "consumer", Workspace: "shared"}} - if !providerFirst { - entries[0], entries[1] = entries[1], entries[0] - } - cfgPath := filepath.Join(t.TempDir(), "config.yaml") - gc := &config.GlobalConfig{Repos: entries} - gc.SetConfigPath(cfgPath) - require.NoError(t, gc.Save()) - cm, err := config.NewConfigManager(cfgPath) - require.NoError(t, err) - db := filepath.Join(t.TempDir(), "graph.sqlite") - g, err := store_sqlite.Open(db) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, g.Close()) }) - newIndexer := func() *MultiIndexer { - reg := newTestRegistry() - reg.Register(languages.NewTypeScriptExtractor()) - return NewMultiIndexer(g, reg, search.NewNull(), cm, zap.NewNop()) - } - mi := newIndexer() - for _, entry := range entries { - _, err := mi.TrackRepoCtx(context.Background(), entry) - require.NoError(t, err) - } - // The daemon runs this whole-store resolve after cold per-repo tracking. - require.NoError(t, mi.RunPreEnrichResolve(context.Background(), nil, nil)) - check := func(stage string) { - t.Helper() - var imports, calls []string - for _, e := range g.AllEdges() { - if e.FilePath != "consumer/src/app.ts" { - continue +func TestMultiIndexer_TrackedNpmPackageLifecycle(t *testing.T) { + for _, binding := range []string{"f", "renamed"} { + for _, backend := range []string{"memory", "sqlite"} { + for _, providerFirst := range []bool{true, false} { + t.Run(binding+"/"+backend+"/"+strconv.FormatBool(providerFirst), func(t *testing.T) { + provider, consumer := t.TempDir(), t.TempDir() + writePackageJSON(t, provider, `{"name":"p","exports":"./src/index.ts"}`) + writeWorkspaceFile(t, provider, "src/index.ts", "export function f(): number { return 42; }\n") + writePackageJSON(t, consumer, `{"name":"app","dependencies":{"p":"^1"}}`) + importName := "f" + if binding != "f" { + importName += " as " + binding } - switch e.Kind { - case graph.EdgeImports: - imports = append(imports, e.To) - assert.True(t, e.CrossRepo, stage) - case graph.EdgeCalls: - calls = append(calls, e.To) - assert.True(t, e.CrossRepo, stage) + writeWorkspaceFile(t, consumer, "src/app.ts", "import { "+importName+" } from 'p';\nexport function run(): number { return "+binding+"(); }\n") + entries := []config.RepoEntry{{Path: provider, Name: "provider", Workspace: "shared"}, {Path: consumer, Name: "consumer", Workspace: "shared"}} + if !providerFirst { + entries[0], entries[1] = entries[1], entries[0] } - } - assert.ElementsMatch(t, []string{"provider/src/index.ts", "provider/src/index.ts::f"}, imports, stage) - assert.Equal(t, []string{"provider/src/index.ts::f"}, calls, stage) - } - check("cold track") - mtimes := map[string]map[string]int64{} - for _, entry := range entries { - mtimes[entry.Name] = mi.FileMtimes(entry.Name) - } - require.NoError(t, mi.Close(context.Background())) - require.NoError(t, g.Close()) - g, err = store_sqlite.Open(db) - require.NoError(t, err) - warm := newIndexer() - t.Cleanup(func() { require.NoError(t, warm.Close(context.Background())) }) - for _, entry := range entries { - _, err := warm.ReconcileRepoCtx(context.Background(), entry, mtimes[entry.Name]) - require.NoError(t, err) + cfgPath := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{Repos: entries} + gc.SetConfigPath(cfgPath) + require.NoError(t, gc.Save()) + cm, err := config.NewConfigManager(cfgPath) + require.NoError(t, err) + db := filepath.Join(t.TempDir(), "graph.sqlite") + var g graph.Store = graph.New() + var sqlite *store_sqlite.Store + if backend == "sqlite" { + sqlite, err = store_sqlite.Open(db) + require.NoError(t, err) + g = sqlite + t.Cleanup(func() { require.NoError(t, sqlite.Close()) }) + } + newIndexer := func() *MultiIndexer { + reg := newTestRegistry() + reg.Register(languages.NewTypeScriptExtractor()) + return NewMultiIndexer(g, reg, search.NewNull(), cm, zap.NewNop()) + } + mi := newIndexer() + for _, entry := range entries { + _, err := mi.TrackRepoCtx(context.Background(), entry) + require.NoError(t, err) + } + // The daemon runs this whole-store resolve after cold per-repo tracking. + require.NoError(t, mi.RunPreEnrichResolve(context.Background(), nil, nil)) + check := func(stage string) { + t.Helper() + var imports, calls []string + for _, e := range g.AllEdges() { + if e.FilePath != "consumer/src/app.ts" { + continue + } + switch e.Kind { + case graph.EdgeImports: + imports = append(imports, e.To) + assert.True(t, e.CrossRepo, stage) + case graph.EdgeCalls: + calls = append(calls, e.To) + assert.True(t, e.CrossRepo, stage) + } + } + assert.ElementsMatch(t, []string{"provider/src/index.ts", "provider/src/index.ts::f"}, imports, stage) + assert.Equal(t, []string{"provider/src/index.ts::f"}, calls, stage) + } + check("cold track") + warm := mi + if backend == "sqlite" { + mtimes := map[string]map[string]int64{} + for _, entry := range entries { + mtimes[entry.Name] = mi.FileMtimes(entry.Name) + } + require.NoError(t, mi.Close(context.Background())) + require.NoError(t, sqlite.Close()) + sqlite, err = store_sqlite.Open(db) + require.NoError(t, err) + g = sqlite + warm = newIndexer() + for _, entry := range entries { + _, err := warm.ReconcileRepoCtx(context.Background(), entry, mtimes[entry.Name]) + require.NoError(t, err) + } + // A clean warm restart skips the global pass and keeps persisted links. + check("warm restart") + } + t.Cleanup(func() { require.NoError(t, warm.Close(context.Background())) }) + writeWorkspaceFile(t, provider, "src/index.ts", "export function f(): number { return 43; }\n") + _, err = warm.IncrementalReindexRepo("provider", []string{"src/index.ts"}) + require.NoError(t, err) + require.NoError(t, warm.RunPreEnrichResolve(context.Background(), nil, nil)) + check("provider save") + _, err = warm.IndexRepo("provider") + require.NoError(t, err) + require.NoError(t, warm.RunPreEnrichResolve(context.Background(), nil, nil)) + check("provider reindex") + warm.UntrackRepo("provider") + var imports []string + for _, e := range g.GetOutEdges("consumer/src/app.ts") { + if e.Kind == graph.EdgeImports { + imports = append(imports, e.To) + assert.False(t, e.CrossRepo) + } + } + assert.ElementsMatch(t, []string{"external::p", "external::p::f"}, imports, "provider untrack") + var pendingCalls []string + for _, e := range g.GetOutEdges("consumer/src/app.ts::run") { + if e.Kind == graph.EdgeCalls { + pendingCalls = append(pendingCalls, e.To) + assert.False(t, e.CrossRepo) + } + } + assert.Equal(t, []string{"unresolved::" + binding}, pendingCalls, "provider untrack") + _, err = warm.TrackRepoCtx(context.Background(), config.RepoEntry{Path: provider, Name: "provider", Workspace: "shared"}) + require.NoError(t, err) + require.NoError(t, warm.RunPreEnrichResolve(context.Background(), nil, nil)) + check("provider retrack") + }) } - // A clean warm restart skips the global pass and keeps persisted links. - check("warm restart") - }) + } } } @@ -288,5 +339,57 @@ func TestTrackedNpmPackagePnpmWorkspace(t *testing.T) { mi := &MultiIndexer{repos: map[string]*RepoMetadata{"lib": {RootPath: root}}} lookup := mi.npmPackageLookup() require.NotNil(t, lookup) - assert.Equal(t, map[string][]string{"lib/packages/lib": {"lib/packages/lib/index.ts", "lib/packages/lib/index"}}, lookup("p")) + assert.Equal(t, map[string]resolver.NpmPackageTarget{"lib/packages/lib": {WorkspaceID: "lib", Entries: []string{"lib/packages/lib/index.ts", "lib/packages/lib/index"}}}, lookup("p")) +} + +func TestTrackedNpmCallShapes(t *testing.T) { + for _, tc := range []struct { + name, source string + calls, imports, reexports []string + }{ + {"namespace", "import * as m from 'p'; export function run() { return m.f(); }", []string{"external-call::stdlib::p"}, []string{"provider/index.ts"}, nil}, + {"default", "import f from 'p'; export function run() { return f(); }", []string{"unresolved::f"}, []string{"provider/index.ts"}, nil}, + {"require member", "export function run() { return require('p').f(); }", []string{"unresolved::require", "unresolved::*.f"}, nil, nil}, + {"require destructured", "const {f} = require('p'); export function run() { return f(); }", []string{"unresolved::require", "unresolved::f"}, nil, nil}, + {"reexport", "export * from 'p';", nil, nil, []string{"external::p"}}, + } { + t.Run(tc.name, func(t *testing.T) { + provider, consumer := t.TempDir(), t.TempDir() + writePackageJSON(t, provider, `{"name":"p","exports":"./index.ts"}`) + writeWorkspaceFile(t, provider, "index.ts", "export default function f() { return 42; }\n") + writePackageJSON(t, consumer, `{"name":"app","dependencies":{"p":"^1"}}`) + writeWorkspaceFile(t, consumer, "app.ts", tc.source) + cfgPath := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{Repos: []config.RepoEntry{{Path: provider, Name: "provider", Workspace: "shared"}, {Path: consumer, Name: "consumer", Workspace: "shared"}}} + gc.SetConfigPath(cfgPath) + require.NoError(t, gc.Save()) + cm, err := config.NewConfigManager(cfgPath) + require.NoError(t, err) + reg := newTestRegistry() + reg.Register(languages.NewTypeScriptExtractor()) + g := graph.New() + mi := NewMultiIndexer(g, reg, search.NewNull(), cm, zap.NewNop()) + t.Cleanup(func() { require.NoError(t, mi.Close(context.Background())) }) + _, err = mi.IndexAll() + require.NoError(t, err) + var calls, imports, reexports []string + for _, e := range g.AllEdges() { + if e.FilePath != "consumer/app.ts" { + continue + } + switch e.Kind { + case graph.EdgeCalls: + calls = append(calls, e.To) + assert.False(t, e.CrossRepo, "no module-wide call-name guesses") + case graph.EdgeImports: + imports = append(imports, e.To) + case graph.EdgeReExports: + reexports = append(reexports, e.To) + } + } + assert.ElementsMatch(t, tc.calls, calls) + assert.ElementsMatch(t, tc.imports, imports) + assert.ElementsMatch(t, tc.reexports, reexports) + }) + } } diff --git a/internal/indexer/repo_eviction_scope.go b/internal/indexer/repo_eviction_scope.go index f8cd8b761..bed6e0478 100644 --- a/internal/indexer/repo_eviction_scope.go +++ b/internal/indexer/repo_eviction_scope.go @@ -10,6 +10,7 @@ import ( // does not expose generations has only one logical generation, so its ordinary // Store eviction is the compatible fallback. func evictRepoCurrentGeneration(target graph.Store, repoPrefix string) (nodesRemoved, edgesRemoved int) { + restubTrackedNpmRepoIncoming(target, repoPrefix) if scoped, ok := target.(graph.CurrentGenerationRepoEvicter); ok { return scoped.EvictRepoCurrentGeneration(repoPrefix) } diff --git a/internal/indexer/repository_untrack.go b/internal/indexer/repository_untrack.go index 3b6a82cf4..e34ad5510 100644 --- a/internal/indexer/repository_untrack.go +++ b/internal/indexer/repository_untrack.go @@ -264,6 +264,7 @@ func (mi *MultiIndexer) finalizeCompletedRepositoryUntrackConfig( func (mi *MultiIndexer) purgeRepositoryPayload( repoPrefix string, meta *RepoMetadata, ) (nodesRemoved, edgesRemoved int, err error) { + restubTrackedNpmRepoIncoming(mi.graph, repoPrefix) if purger, ok := mi.graph.(interface{ PurgeRepo(string) error }); ok { if err := purger.PurgeRepo(repoPrefix); err != nil { return 0, 0, fmt.Errorf("purge repository payload: %w", err) diff --git a/internal/parser/languages/helpers_jsimports.go b/internal/parser/languages/helpers_jsimports.go index b86539a37..81bbb1dc8 100644 --- a/internal/parser/languages/helpers_jsimports.go +++ b/internal/parser/languages/helpers_jsimports.go @@ -86,6 +86,7 @@ func emitJSPerBindingImports(importNode *sitter.Node, importPath, fileID, filePa } if alias != "" && alias != orig { edge.Alias = alias + edge.Meta = map[string]any{"import_alias": alias} } result.Edges = append(result.Edges, edge) } diff --git a/internal/parser/languages/per_binding_imports_test.go b/internal/parser/languages/per_binding_imports_test.go index af4b0af16..f25885323 100644 --- a/internal/parser/languages/per_binding_imports_test.go +++ b/internal/parser/languages/per_binding_imports_test.go @@ -43,6 +43,12 @@ func assertPerBindingImports(t *testing.T, res *parser.ExtractionResult) { if bar.Alias != "baz" { t.Errorf("bar import alias = %q, want baz", bar.Alias) } + if bar.Meta["import_alias"] != "baz" { + t.Errorf("persisted bar import alias = %v, want baz", bar.Meta["import_alias"]) + } + if _, present := foo.Meta["import_alias"]; present { + t.Error("an unrenamed import must not carry import_alias") + } // Named re-export edges: a (no rename) and b→c (renamed exported name). a := importEdge(res, graph.EdgeReExports, "unresolved::import::up::a") diff --git a/internal/resolver/cross_repo.go b/internal/resolver/cross_repo.go index e761c393f..5ca43be07 100644 --- a/internal/resolver/cross_repo.go +++ b/internal/resolver/cross_repo.go @@ -112,6 +112,7 @@ type CrossRepoResolver struct { // rows. A Go call the per-repo package guard refused must not regain a // same-repo name-only binding just because the repository is reachable. importedDirsByFile map[string]map[string]struct{} + jsTSBindingsByFile map[string]map[string]string // depModuleIndex bridges Go imports to dep:: contract // nodes from the caller's go.mod. Same shape and rationale as // the field of the same name on Resolver — see resolver.go for @@ -801,6 +802,7 @@ func (cr *CrossRepoResolver) clearDirIndexes() { func (cr *CrossRepoResolver) buildReachableReposIndex() { idx := make(map[string]map[string]struct{}) dirs := make(map[string]map[string]struct{}) + cr.jsTSBindingsByFile = make(map[string]map[string]string) // Materialise metadata-free import projections and batch-load only target // placement fields. A per-edge point lookup here is a query round-trip per // import on a disk backend, which under the cross-repo pass's import @@ -823,29 +825,74 @@ func (cr *CrossRepoResolver) buildReachableReposIndex() { idList = append(idList, id) } placements := graph.NodePlacementsByIDs(cr.graph, idList) - for _, e := range imports { - // Only resolved imports carry evidence — an unresolved import - // target tells us nothing about which repo the caller reaches. - to, ok := placements[e.To] - if !ok { - continue - } - if to.FilePath != "" { - dir := filePathDir(to.FilePath) - if dirs[e.From] == nil { - dirs[e.From] = make(map[string]struct{}) + for start := 0; start < len(imports); start += resolvePendingPageRows { + page := imports[start:min(start+resolvePendingPageRows, len(imports))] + var sites []graph.EdgeSite + for _, e := range page { + if e.CrossRepo && isJSTSPath(e.From) { + sites = append(sites, graph.EdgeSite{From: e.From, Line: e.Line, Kind: graph.EdgeImports}) } - dirs[e.From][dir] = struct{}{} - } - if to.RepoPrefix == "" { - continue } - set := idx[e.From] - if set == nil { - set = make(map[string]struct{}) - idx[e.From] = set + // Markers belong to the persisted import, including imports outside a + // scoped resolution frontier. Hydrate only cross-repo JS/TS imports. + full := cr.graph.GetEdgeCandidates(nil, sites) + for _, e := range page { + for _, candidate := range full.Site(e.From, e.Line, graph.EdgeImports) { + if candidate.To == e.To && candidate.FilePath == e.FilePath { + e = candidate + break + } + } + // Only resolved imports carry evidence — an unresolved import + // target tells us nothing about which repo the caller reaches. + to, ok := placements[e.To] + if !ok { + continue + } + if to.FilePath != "" { + dir := filePathDir(to.FilePath) + if dirs[e.From] == nil { + dirs[e.From] = make(map[string]struct{}) + } + dirs[e.From][dir] = struct{}{} + } + if to.RepoPrefix == "" { + continue + } + marked, _ := e.Meta[NpmImportSpecifierMeta].(string) + npmLinked := marked != "" && e.CrossRepo + alias, _ := e.Meta["import_alias"].(string) + if alias == "" { + alias = e.Alias + } + if isJSTSPath(e.From) { + // JS/TS imports name bindings, not every symbol in the provider. + if _, name, ok := strings.Cut(e.To, "::"); ok { + if npmLinked && alias != "" { + name = alias + } + bindings := cr.jsTSBindingsByFile[e.From] + if bindings == nil { + bindings = make(map[string]string) + cr.jsTSBindingsByFile[e.From] = bindings + } + if prior, exists := bindings[name]; !exists || prior == e.To { + bindings[name] = e.To + } else { + bindings[name] = "" // Conflicting bindings must not pick a winner. + } + } + } + if npmLinked { + continue + } + set := idx[e.From] + if set == nil { + set = make(map[string]struct{}) + idx[e.From] = set + } + set[to.RepoPrefix] = struct{}{} } - set[to.RepoPrefix] = struct{}{} } cr.reachableReposByFile = idx cr.importedDirsByFile = dirs @@ -854,6 +901,7 @@ func (cr *CrossRepoResolver) buildReachableReposIndex() { func (cr *CrossRepoResolver) clearReachableReposIndex() { cr.reachableReposByFile = nil cr.importedDirsByFile = nil + cr.jsTSBindingsByFile = nil } // A package-guard refusal is stronger than the cross-repo pass's same-repo @@ -1321,6 +1369,19 @@ func (cr *CrossRepoResolver) csharpVerdictCaller(e *graph.Edge) bool { } func (cr *CrossRepoResolver) resolveFunctionCall(e *graph.Edge, funcName string, stats *CrossRepoStats) { + file := cr.callerFileID(e) + if target := cr.jsTSBindingsByFile[file][funcName]; target != "" { + if imported := cr.cachedGetNode(target); imported != nil && (imported.Kind == graph.KindFunction || imported.Kind == graph.KindMethod) { + e.To = imported.ID + stats.Resolved++ + if isCrossRepoHop(cr.callerRepoPrefix(e), imported.RepoPrefix) { + e.CrossRepo = true + stats.CrossRepoEdges++ + stats.ByRepo[imported.RepoPrefix]++ + } + return + } + } candidates := cr.scopedCandidates(e, funcName) if len(candidates) == 0 { stats.Unresolved++ diff --git a/internal/resolver/npm_package.go b/internal/resolver/npm_package.go index c43bb17ba..9ee1c6d17 100644 --- a/internal/resolver/npm_package.go +++ b/internal/resolver/npm_package.go @@ -8,13 +8,21 @@ import ( "github.com/zzet/gortex/internal/graph" ) +// NpmPackageTarget records a manifest claim even when Entries is empty or +// none of its graph-path candidates is indexed. WorkspaceID is independent of +// entry probing, so unindexed providers still count in their workspace tier. +type NpmPackageTarget struct { + WorkspaceID string + Entries []string +} + // NpmPackageLookup maps a bare npm specifier to tracked package directories. -// Each directory has graph-path entry candidates from exports. Legacy -// types/main/directory-index entry points are added when exports is absent, and -// when exports declares the requested entry but gives no target the indexer -// supports (for example only `types` or `require` conditions). The indexer owns -// manifest reads; the resolver probes indexed files and applies repository/workspace policy. -type NpmPackageLookup func(specifier string) map[string][]string +// The indexer owns manifests and workspace placement; the resolver probes the +// indexed entry candidates and applies repository/workspace policy. +type NpmPackageLookup func(specifier string) map[string]NpmPackageTarget + +// NpmImportSpecifierMeta preserves the original npm import across provider eviction. +const NpmImportSpecifierMeta = "npm_import" // SetNpmPackageLookup installs tracked-package evidence for the cross-repo pass. // Call before resolution; nil keeps the external-package short-circuit unchanged. @@ -50,38 +58,41 @@ func (cr *CrossRepoResolver) trackedNpmImport(callerRepo, callerWS, importPath s return nil } } - var local, foreign []*graph.Node - for dir, stems := range packages { - key := npmProbeKey{dir: dir, spec: spec} - node, seen := memo.probes[key] - if !seen { - for _, stem := range stems { - if file := probeJSTSFile(cr.cachedGetNode, stem); file != "" { - node = cr.cachedGetNode(file) - break - } - } - memo.probes[key] = node // Cache misses too; reset on the next pass. + var local, foreign []string + for dir, pkg := range packages { + ws := pkg.WorkspaceID + if ws == "" { + ws, _, _ = strings.Cut(dir, "/") } - if node == nil { - continue - } - if candidateWorkspaceID(node) == callerWS { - local = append(local, node) - } else if cr.crossWorkspaceEligible(callerWS, candidateWorkspaceID(node), spec) { - foreign = append(foreign, node) + if ws == callerWS { + local = append(local, dir) + } else if cr.crossWorkspaceEligible(callerWS, ws, spec) { + foreign = append(foreign, dir) } } candidates := local if len(candidates) == 0 { candidates = foreign } - // Package names do not identify a unique tracked checkout when more - // than one provider is eligible in the winning workspace tier. + // Manifest claims, including unindexed entries, decide ambiguity. if len(candidates) != 1 { return nil } - node := candidates[0] + dir := candidates[0] + key := npmProbeKey{dir: dir, spec: spec} + node, seen := memo.probes[key] + if !seen { + for _, stem := range packages[dir].Entries { + if file := probeJSTSFile(cr.cachedGetNode, stem); file != "" { + node = cr.cachedGetNode(file) + break + } + } + memo.probes[key] = node + } + if node == nil { + return nil + } if symbol != "" { if exported := memo.getNode(cr, node.ID+"::"+symbol); exported != nil && exported.Meta["visibility"] == "public" { node = exported @@ -104,14 +115,25 @@ func (cr *CrossRepoResolver) resolveTrackedNpmImports(ctx context.Context, edges return err } spec := trackedNpmImportSpec(e) - if spec == "" || !cr.eligibleTrackedNpmImport(e, spec) { + if marked, _ := e.Meta[NpmImportSpecifierMeta].(string); marked != "" && e.CrossRepo { + eligible = append(eligible, e) continue } - module, _ := splitImportSpecSymbol(spec) - if len(cr.npmPackages(module)) == 0 { + if spec == "" { + // Light edges omit Meta. Hydrate resolved cross-repo JS/TS + // imports so the persisted npm marker can be rechecked too. + if e.CrossRepo && isJSTSPath(jsTSImportCallerFile(e)) { + eligible = append(eligible, e) + } continue } - eligible = append(eligible, e) + if !cr.eligibleTrackedNpmImport(e, spec) { + continue + } + module, _ := splitImportSpecSymbol(spec) + if len(cr.npmPackages(module)) != 0 { + eligible = append(eligible, e) + } } if len(eligible) == 0 { return ctx.Err() @@ -135,8 +157,12 @@ func (cr *CrossRepoResolver) resolveTrackedNpmImports(ctx context.Context, edges if e.To != projected.To { continue } + spec := trackedNpmImportSpec(e) + if spec == "" { + continue // A non-npm cross-repo import keeps its semantics. + } oldTo := e.To - if cr.resolveTrackedNpmImport(e, trackedNpmImportSpec(e), stats, memo) { + if cr.resolveTrackedNpmImport(e, spec, stats, memo) { batch = append(batch, graph.EdgeReindex{Edge: e, OldTo: oldTo}) projected.To, projected.CrossRepo = e.To, e.CrossRepo } @@ -144,15 +170,66 @@ func (cr *CrossRepoResolver) resolveTrackedNpmImports(ctx context.Context, edges } if len(batch) > 0 { cr.graph.ReindexEdges(batch) + cr.restubCallsForChangedNpmImports(batch) } } return ctx.Err() } +// A call resolved through a binding cannot outlive that binding's target. +func (cr *CrossRepoResolver) restubCallsForChangedNpmImports(changes []graph.EdgeReindex) { + callers := make(map[string]map[string]string) + var ids []string + for _, change := range changes { + if !strings.Contains(change.OldTo, "::") || strings.HasPrefix(change.OldTo, "external::") || graph.IsUnresolvedTarget(change.OldTo) { + continue + } + if callers[change.OldTo] == nil { + callers[change.OldTo] = make(map[string]string) + ids = append(ids, change.OldTo) + } + _, name, _ := strings.Cut(change.OldTo, "::") + if alias, _ := change.Edge.Meta["import_alias"].(string); alias != "" { + name = alias + } + callers[change.OldTo][change.Edge.From] = name + } + if len(ids) == 0 { + return + } + var batch []graph.EdgeReindex + var obsolete []*graph.Edge + for target, edges := range cr.graph.GetInEdgesByNodeIDs(ids) { + for _, call := range edges { + if call == nil || call.Kind != graph.EdgeCalls || !call.CrossRepo { + continue + } + name, affected := callers[target][cr.callerFileID(call)] + if !affected { + continue + } + parallel := *call + parallel.Kind = graph.EdgeCrossRepoCalls + obsolete = append(obsolete, ¶llel) + graph.StashRestubProvenance(call) + call.To = graph.UnresolvedMarker + name + call.CrossRepo, call.ConfidenceLabel = false, "" + batch = append(batch, graph.EdgeReindex{Edge: call, OldTo: target}) + } + } + if len(batch) > 0 { + graph.RemoveEdgesExact(cr.graph, obsolete) + cr.graph.ReindexEdges(batch) + } +} + func trackedNpmImportSpec(e *graph.Edge) string { if e == nil || e.Kind != graph.EdgeImports { return "" } + if spec, _ := e.Meta[NpmImportSpecifierMeta].(string); spec != "" { + return spec + } if strings.HasPrefix(e.To, "external::") { return strings.TrimPrefix(e.To, "external::") } @@ -169,10 +246,25 @@ func (cr *CrossRepoResolver) resolveTrackedNpmImport(e *graph.Edge, importPath s callerRepo = from.RepoPrefix } callerWS := candidateWorkspaceID(from) - picked := cr.trackedNpmImport(callerRepo, callerWS, importPath, memo) + var picked *graph.Node + if cr.eligibleTrackedNpmImport(e, importPath) { + picked = cr.trackedNpmImport(callerRepo, callerWS, importPath, memo) + } if picked == nil { + if spec, _ := e.Meta[NpmImportSpecifierMeta].(string); spec != "" && e.To != "external::"+spec { + e.To = "external::" + spec + e.CrossRepo = false + return true + } + return false + } + if e.To == picked.ID { return false } + if e.Meta == nil { + e.Meta = make(map[string]any) + } + e.Meta[NpmImportSpecifierMeta] = importPath e.To = picked.ID e.CrossRepo = true stats.Resolved++ diff --git a/internal/resolver/npm_package_test.go b/internal/resolver/npm_package_test.go index 98969172b..68a910a40 100644 --- a/internal/resolver/npm_package_test.go +++ b/internal/resolver/npm_package_test.go @@ -14,7 +14,7 @@ import ( func TestCrossRepoTrackedNpmPackage(t *testing.T) { for _, backend := range []string{"memory", "sqlite"} { - for _, target := range []string{"external::p", "unresolved::import::p"} { + for _, target := range []string{"external::p::f", "unresolved::import::p::f"} { for _, mode := range []string{"all", "repo", "file", "mutation", "import only"} { t.Run(backend+"/"+target+"/"+mode, func(t *testing.T) { var g graph.Store = graph.New() @@ -28,7 +28,7 @@ func TestCrossRepoTrackedNpmPackage(t *testing.T) { {ID: "app/src/app.ts", Kind: graph.KindFile, FilePath: "app/src/app.ts", RepoPrefix: "app", Language: "typescript"}, {ID: "app/src/app.ts::run", Kind: graph.KindFunction, Name: "run", FilePath: "app/src/app.ts", RepoPrefix: "app", Language: "typescript"}, {ID: "lib/src/index.ts", Kind: graph.KindFile, FilePath: "lib/src/index.ts", RepoPrefix: "lib", Language: "typescript"}, - {ID: "lib/src/index.ts::f", Kind: graph.KindFunction, Name: "f", FilePath: "lib/src/index.ts", RepoPrefix: "lib", Language: "typescript"}, + {ID: "lib/src/index.ts::f", Kind: graph.KindFunction, Name: "f", FilePath: "lib/src/index.ts", RepoPrefix: "lib", Language: "typescript", Meta: map[string]any{"visibility": "public"}}, }, nil) imp := &graph.Edge{From: "app/src/app.ts", To: target, Kind: graph.EdgeImports, FilePath: "app/src/app.ts", Line: 1, Meta: map[string]any{"test_marker": "preserved"}} g.AddEdge(imp) @@ -37,11 +37,11 @@ func TestCrossRepoTrackedNpmPackage(t *testing.T) { } cr := NewCrossRepo(g) cr.SetNpmDependencyLookup(func(_, spec string) bool { return spec == "p" }) - cr.SetNpmPackageLookup(func(spec string) map[string][]string { + cr.SetNpmPackageLookup(func(spec string) map[string]NpmPackageTarget { if spec != "p" { return nil } - return map[string][]string{"lib": {"lib/src/index.ts", "lib"}} + return map[string]NpmPackageTarget{"lib": {Entries: []string{"lib/src/index.ts", "lib"}}} }) var stats *CrossRepoStats switch mode { @@ -68,7 +68,7 @@ func TestCrossRepoTrackedNpmPackage(t *testing.T) { continue } found = true - assert.Equal(t, "lib/src/index.ts", e.To) + assert.Equal(t, "lib/src/index.ts::f", e.To) assert.True(t, e.CrossRepo) assert.Equal(t, "preserved", e.Meta["test_marker"]) } @@ -104,10 +104,10 @@ func TestCrossRepoTrackedNpmPackageAmbiguousOrUndeclared(t *testing.T) { g.AddEdge(imp) cr := NewCrossRepo(g) cr.SetNpmDependencyLookup(func(_, _ string) bool { return declared }) - cr.SetNpmPackageLookup(func(_ string) map[string][]string { - targets := map[string][]string{"one": {"one/index.ts"}} + cr.SetNpmPackageLookup(func(_ string) map[string]NpmPackageTarget { + targets := map[string]NpmPackageTarget{"one": {WorkspaceID: "shared", Entries: []string{"one/index.ts"}}} if declared { - targets["two"] = []string{"two/index.ts"} + targets["two"] = NpmPackageTarget{Entries: []string{"two/index.ts"}} } return targets }) @@ -121,15 +121,15 @@ func TestCrossRepoTrackedNpmPackageAmbiguousOrUndeclared(t *testing.T) { func TestCrossRepoTrackedNpmPackageBoundaries(t *testing.T) { for _, tc := range []struct { name, visibility string - dirs map[string][]string + dirs map[string]NpmPackageTarget want string }{ - {"own package missing index", "private", map[string][]string{"app/packages/p": {"app/packages/p/missing"}, "one": {"one/index.ts"}}, "external::p::f"}, - {"own root package", "private", map[string][]string{"app": {"app/missing"}, "one": {"one/index.ts"}}, "external::p::f"}, - {"same workspace ambiguous", "private", map[string][]string{"one": {"one/index.ts"}, "two": {"two/index.ts"}}, "external::p::f"}, - {"private symbol", "private", map[string][]string{"one": {"one/index.ts"}}, "one/index.ts"}, - {"public symbol", "public", map[string][]string{"one": {"one/index.ts"}}, "one/index.ts::f"}, - {"unknown visibility", "", map[string][]string{"one": {"one/index.ts"}}, "one/index.ts"}, + {"own package missing index", "private", map[string]NpmPackageTarget{"app/packages/p": {Entries: []string{"app/packages/p/missing"}}, "one": {WorkspaceID: "shared", Entries: []string{"one/index.ts"}}}, "external::p::f"}, + {"own root package", "private", map[string]NpmPackageTarget{"app": {Entries: []string{"app/missing"}}, "one": {WorkspaceID: "shared", Entries: []string{"one/index.ts"}}}, "external::p::f"}, + {"same workspace ambiguous", "private", map[string]NpmPackageTarget{"one": {WorkspaceID: "shared", Entries: []string{"one/index.ts"}}, "two": {WorkspaceID: "shared", Entries: []string{"two/index.ts"}}}, "external::p::f"}, + {"private symbol", "private", map[string]NpmPackageTarget{"one": {WorkspaceID: "shared", Entries: []string{"one/index.ts"}}}, "one/index.ts"}, + {"public symbol", "public", map[string]NpmPackageTarget{"one": {WorkspaceID: "shared", Entries: []string{"one/index.ts"}}}, "one/index.ts::f"}, + {"unknown visibility", "", map[string]NpmPackageTarget{"one": {WorkspaceID: "shared", Entries: []string{"one/index.ts"}}}, "one/index.ts"}, } { t.Run(tc.name, func(t *testing.T) { g := graph.New() @@ -143,7 +143,7 @@ func TestCrossRepoTrackedNpmPackageBoundaries(t *testing.T) { g.AddEdge(imp) cr := NewCrossRepo(g) cr.SetNpmDependencyLookup(func(_, _ string) bool { return true }) - cr.SetNpmPackageLookup(func(_ string) map[string][]string { return tc.dirs }) + cr.SetNpmPackageLookup(func(_ string) map[string]NpmPackageTarget { return tc.dirs }) cr.ResolveAll() assert.Equal(t, tc.want, imp.To) assert.Equal(t, tc.want != "external::p::f", imp.CrossRepo) @@ -173,7 +173,9 @@ func TestCrossRepoTrackedNpmPackageProbeMemo(t *testing.T) { } cr := NewCrossRepo(g) cr.SetNpmDependencyLookup(func(_, _ string) bool { return declared }) - cr.SetNpmPackageLookup(func(_ string) map[string][]string { return map[string][]string{"lib": {"lib/missing"}} }) + cr.SetNpmPackageLookup(func(_ string) map[string]NpmPackageTarget { + return map[string]NpmPackageTarget{"lib": {Entries: []string{"lib/missing"}}} + }) for pass := 0; pass < 2; pass++ { g.providerReads = 0 cr.ResolveAll() @@ -232,7 +234,10 @@ func TestCrossRepoTrackedNpmPackageHydration(t *testing.T) { cr.SetNpmAliasResolver(func(_, _ string) string { return "other" }) } lookups := 0 - cr.SetNpmPackageLookup(func(_ string) map[string][]string { lookups++; return map[string][]string{"lib": {"lib/index.ts"}} }) + cr.SetNpmPackageLookup(func(_ string) map[string]NpmPackageTarget { + lookups++ + return map[string]NpmPackageTarget{"lib": {Entries: []string{"lib/index.ts"}}} + }) stats := &CrossRepoStats{ByRepo: map[string]int{}} require.NoError(t, cr.resolveTrackedNpmImports(context.Background(), graph.EdgesLightSeq(store, graph.EdgeImports), stats)) if mode != "declared" { @@ -278,12 +283,12 @@ func TestCrossRepoTrackedNpmPackageCancellation(t *testing.T) { cr := NewCrossRepo(g) cr.SetNpmDependencyLookup(func(_, _ string) bool { return true }) lookups := 0 - cr.SetNpmPackageLookup(func(_ string) map[string][]string { + cr.SetNpmPackageLookup(func(_ string) map[string]NpmPackageTarget { lookups++ if phase == "scan" { cancel() } - return map[string][]string{"lib": {"lib/index.ts"}} + return map[string]NpmPackageTarget{"lib": {Entries: []string{"lib/index.ts"}}} }) if phase == "before" { cancel() @@ -305,3 +310,269 @@ func TestCrossRepoTrackedNpmPackageCancellation(t *testing.T) { }) } } + +func TestTrackedNpmCallUsesImportBinding(t *testing.T) { + for _, backend := range []string{"memory", "sqlite"} { + t.Run(backend, func(t *testing.T) { + var g graph.Store = graph.New() + if backend == "sqlite" { + db, err := store_sqlite.Open(filepath.Join(t.TempDir(), "graph.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + g = db + } + g.AddBatch([]*graph.Node{ + {ID: "app/a.ts", Kind: graph.KindFile, FilePath: "app/a.ts", RepoPrefix: "app", Language: "typescript"}, + {ID: "app/a.ts::run", Kind: graph.KindFunction, FilePath: "app/a.ts", RepoPrefix: "app", Language: "typescript"}, + {ID: "lib/z.ts", Kind: graph.KindFile, FilePath: "lib/z.ts", RepoPrefix: "lib", Language: "typescript"}, + {ID: "lib/z.ts::f", Name: "f", Kind: graph.KindFunction, FilePath: "lib/z.ts", RepoPrefix: "lib", Language: "typescript", Meta: map[string]any{"visibility": "public"}}, + {ID: "lib/a/helpers.ts::f", Name: "f", Kind: graph.KindFunction, FilePath: "lib/a/helpers.ts", RepoPrefix: "lib", Language: "typescript"}, + {ID: "lib/internal.ts::debounce", Name: "debounce", Kind: graph.KindFunction, FilePath: "lib/internal.ts", RepoPrefix: "lib", Language: "typescript"}, + }, []*graph.Edge{ + {From: "app/a.ts", To: "external::p", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1}, + {From: "app/a.ts", To: "external::p::f", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1}, + {From: "app/a.ts", To: "external::lodash::debounce", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 2}, + {From: "app/a.ts::run", To: "unresolved::f", Kind: graph.EdgeCalls, FilePath: "app/a.ts", Line: 3}, + {From: "app/a.ts::run", To: "unresolved::debounce", Kind: graph.EdgeCalls, FilePath: "app/a.ts", Line: 4}, + }) + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return true }) + cr.SetNpmPackageLookup(func(spec string) map[string]NpmPackageTarget { + if spec == "p" { + return map[string]NpmPackageTarget{"lib": {Entries: []string{"lib/z.ts"}}} + } + return nil + }) + cr.ResolveAll() + var calls []*graph.Edge + for _, e := range g.GetOutEdges("app/a.ts::run") { + if e.Kind == graph.EdgeCalls { + calls = append(calls, e) + } + } + require.Len(t, calls, 2) + for _, e := range calls { + if e.Line == 3 { + assert.Equal(t, "lib/z.ts::f", e.To) + } else { + assert.Equal(t, "unresolved::debounce", e.To) + assert.False(t, e.CrossRepo) + } + } + }) + } +} + +func TestTrackedNpmAmbiguityCountsUnindexedManifests(t *testing.T) { + for _, workspace := range []string{"shared", "foreign"} { + t.Run(workspace, func(t *testing.T) { + g := graph.New() + g.AddBatch([]*graph.Node{ + {ID: "app/a.ts", Kind: graph.KindFile, RepoPrefix: "app", WorkspaceID: "shared"}, + {ID: "fork/index.ts", Kind: graph.KindFile, RepoPrefix: "fork", WorkspaceID: workspace}, + }, []*graph.Edge{{From: "app/a.ts", To: "external::p", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1}}) + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return true }) + cr.SetNpmPackageLookup(func(_ string) map[string]NpmPackageTarget { + return map[string]NpmPackageTarget{"real": {WorkspaceID: workspace, Entries: []string{"real/dist/index.js"}}, "fork": {WorkspaceID: workspace, Entries: []string{"fork/index.ts"}}} + }) + cr.ResolveAll() + imports := g.GetOutEdges("app/a.ts") + require.Len(t, imports, 1) + assert.Equal(t, "external::p", imports[0].To) + assert.False(t, imports[0].CrossRepo) + }) + } +} + +func TestTrackedNpmLinkedImportsRechecked(t *testing.T) { + for _, backend := range []string{"memory", "sqlite"} { + for _, change := range []string{"renamed", "blocked", "dependency removed", "alias", "repointed"} { + t.Run(backend+"/"+change, func(t *testing.T) { + var g graph.Store = graph.New() + if backend == "sqlite" { + db, err := store_sqlite.Open(filepath.Join(t.TempDir(), "graph.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + g = db + } + g.AddBatch([]*graph.Node{ + {ID: "app/a.ts", Kind: graph.KindFile, RepoPrefix: "app"}, + {ID: "app/a.ts::run", Kind: graph.KindFunction, FilePath: "app/a.ts", RepoPrefix: "app"}, + {ID: "lib/index.ts", Kind: graph.KindFile, RepoPrefix: "lib"}, + {ID: "lib/index.ts::f", Name: "f", Kind: graph.KindFunction, RepoPrefix: "lib", Meta: map[string]any{"visibility": "public"}}, + {ID: "next/index.ts", Kind: graph.KindFile, RepoPrefix: "next"}, + {ID: "next/index.ts::f", Name: "f", Kind: graph.KindFunction, RepoPrefix: "next", Meta: map[string]any{"visibility": "public"}}, + }, []*graph.Edge{ + {From: "app/a.ts", To: "external::p", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1}, + {From: "app/a.ts", To: "external::p::f", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1}, + {From: "app/a.ts::run", To: "unresolved::f", Kind: graph.EdgeCalls, FilePath: "app/a.ts", Line: 2}, + }) + changed := false + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return !changed || change != "dependency removed" }) + cr.SetNpmAliasResolver(func(_, _ string) string { + if changed && change == "alias" { + return "other" + } + return "" + }) + cr.SetNpmPackageLookup(func(_ string) map[string]NpmPackageTarget { + if changed && change == "repointed" { + return map[string]NpmPackageTarget{"next": {Entries: []string{"next/index.ts"}}} + } + if changed && change == "renamed" { + return nil + } + if changed && change == "blocked" { + return map[string]NpmPackageTarget{"lib": {}} + } + return map[string]NpmPackageTarget{"lib": {Entries: []string{"lib/index.ts"}}} + }) + cr.ResolveAll() + for _, e := range g.GetOutEdges("app/a.ts") { + require.True(t, e.CrossRepo) + assert.NotEmpty(t, e.Meta["npm_import"]) + } + for _, e := range g.GetOutEdges("app/a.ts::run") { + if e.Kind == graph.EdgeCalls { + require.Equal(t, "lib/index.ts::f", e.To) + } + } + changed = true + cr.ResolveAll() + if change == "repointed" { + calls := g.GetOutEdges("app/a.ts::run") + require.Len(t, calls, 2, "one base call and one parallel") + for _, e := range calls { + assert.Equal(t, "next/index.ts::f", e.To, "base and parallel must both target the new provider") + } + assert.Empty(t, g.GetInEdges("lib/index.ts::f")) + return + } + var targets []string + for _, e := range g.GetOutEdges("app/a.ts") { + targets = append(targets, e.To) + assert.False(t, e.CrossRepo) + } + assert.ElementsMatch(t, []string{"external::p", "external::p::f"}, targets) + for _, e := range g.GetOutEdges("app/a.ts::run") { + assert.NotEqual(t, graph.EdgeCrossRepoCalls, e.Kind, "restubbed call must not keep its old parallel") + if e.Kind == graph.EdgeCalls { + assert.Equal(t, "unresolved::f", e.To) + assert.False(t, e.CrossRepo) + } + } + assert.Zero(t, cr.ResolveAll().Resolved, "repeated rejection is idempotent") + }) + } + } +} + +func TestTrackedNpmOwnWorkspaceWins(t *testing.T) { + g := graph.New() + g.AddBatch([]*graph.Node{ + {ID: "app/a.ts", Kind: graph.KindFile, RepoPrefix: "app", WorkspaceID: "shared"}, + {ID: "local/index.ts", Kind: graph.KindFile, RepoPrefix: "local", WorkspaceID: "shared"}, + {ID: "foreign/index.ts", Kind: graph.KindFile, RepoPrefix: "foreign", WorkspaceID: "elsewhere"}, + }, []*graph.Edge{{From: "app/a.ts", To: "external::p", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 1}}) + cr := NewCrossRepo(g) + cr.SetCrossWorkspaceDepLookup(func(_ string) []CrossWorkspaceDepRule { + return []CrossWorkspaceDepRule{{Workspace: "elsewhere", Modules: []string{"p"}}} + }) + cr.SetNpmDependencyLookup(func(_, _ string) bool { return true }) + cr.SetNpmPackageLookup(func(_ string) map[string]NpmPackageTarget { + return map[string]NpmPackageTarget{"local": {WorkspaceID: "shared", Entries: []string{"local/index.ts"}}, "foreign": {WorkspaceID: "elsewhere", Entries: []string{"foreign/index.ts"}}} + }) + cr.ResolveAll() + imports := g.GetOutEdges("app/a.ts") + require.Len(t, imports, 1) + assert.Equal(t, "local/index.ts", imports[0].To) + assert.True(t, imports[0].CrossRepo) +} + +func TestTrackedNpmRenamedImportDoesNotExposeOriginal(t *testing.T) { + for _, backend := range []string{"memory", "sqlite"} { + t.Run(backend, func(t *testing.T) { + var g graph.Store = graph.New() + if backend == "sqlite" { + db, err := store_sqlite.Open(filepath.Join(t.TempDir(), "graph.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + g = db + } + g.AddBatch([]*graph.Node{ + {ID: "app/a.ts", Kind: graph.KindFile, RepoPrefix: "app"}, + {ID: "app/a.ts::run", Kind: graph.KindFunction, FilePath: "app/a.ts", RepoPrefix: "app"}, + {ID: "lib/index.ts", Kind: graph.KindFile, RepoPrefix: "lib"}, + {ID: "lib/index.ts::f", Kind: graph.KindFunction, Name: "f", FilePath: "lib/index.ts", RepoPrefix: "lib", Meta: map[string]any{"visibility": "public"}}, + }, []*graph.Edge{ + {From: "app/a.ts", To: "external::p::f", Kind: graph.EdgeImports, Alias: "renamed", Meta: map[string]any{"import_alias": "renamed"}, FilePath: "app/a.ts", Line: 1}, + {From: "app/a.ts", To: "external::lodash::f", Kind: graph.EdgeImports, FilePath: "app/a.ts", Line: 2}, + {From: "app/a.ts::run", To: "unresolved::f", Kind: graph.EdgeCalls, FilePath: "app/a.ts", Line: 3}, + {From: "app/a.ts::run", To: "unresolved::renamed", Kind: graph.EdgeCalls, FilePath: "app/a.ts", Line: 4}, + }) + cr := NewCrossRepo(g) + cr.SetNpmDependencyLookup(func(_, spec string) bool { return spec == "p" || spec == "lodash" }) + cr.SetNpmPackageLookup(func(spec string) map[string]NpmPackageTarget { + if spec == "p" { + return map[string]NpmPackageTarget{"lib": {Entries: []string{"lib/index.ts"}}} + } + return nil + }) + cr.ResolveAll() + // The provider changes, but the consumer's marked import is outside + // the outgoing frontier. Its incoming bare f() must stay unresolved. + cr.ResolveMutationFrontiers([]string{"lib/index.ts"}, nil, []string{"lib/index.ts"}) + var calls []*graph.Edge + for _, e := range g.GetOutEdges("app/a.ts::run") { + if e.Kind == graph.EdgeCalls { + calls = append(calls, e) + } + } + require.Len(t, calls, 2) + for _, call := range calls { + if call.Line == 3 { + assert.Equal(t, "unresolved::f", call.To) + assert.False(t, call.CrossRepo) + } else { + assert.Equal(t, "lib/index.ts::f", call.To) + assert.True(t, call.CrossRepo) + } + } + }) + } +} + +func TestJSTSCrossRepoFileImportFallback(t *testing.T) { + for _, backend := range []string{"memory", "sqlite"} { + t.Run(backend, func(t *testing.T) { + var g graph.Store = graph.New() + if backend == "sqlite" { + db, err := store_sqlite.Open(filepath.Join(t.TempDir(), "graph.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + g = db + } + g.AddBatch([]*graph.Node{ + {ID: "app/a.ts", Kind: graph.KindFile, RepoPrefix: "app"}, + {ID: "app/a.ts::run", Kind: graph.KindFunction, FilePath: "app/a.ts", RepoPrefix: "app"}, + {ID: "lib/index.ts", Kind: graph.KindFile, RepoPrefix: "lib"}, + {ID: "lib/index.ts::f", Kind: graph.KindFunction, Name: "f", FilePath: "lib/index.ts", RepoPrefix: "lib"}, + }, []*graph.Edge{ + {From: "app/a.ts", To: "lib/index.ts", Kind: graph.EdgeImports, FilePath: "app/a.ts", CrossRepo: true}, + {From: "app/a.ts::run", To: "unresolved::f", Kind: graph.EdgeCalls, FilePath: "app/a.ts"}, + }) + NewCrossRepo(g).ResolveAll() + var calls []*graph.Edge + for _, e := range g.GetOutEdges("app/a.ts::run") { + if e.Kind == graph.EdgeCalls { + calls = append(calls, e) + } + } + require.Len(t, calls, 1) + assert.Equal(t, "lib/index.ts::f", calls[0].To) + assert.True(t, calls[0].CrossRepo) + }) + } +} From 884e3c8c482715d058f54cede2e1a1834aec82ce Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Fri, 23 Oct 2026 22:40:46 -0700 Subject: [PATCH 6/7] fix(resolver): relink parked npm consumers after a full reindex or retrack After the rebase onto #806, two review findings remained: - A full provider reindex or untrack/retrack parked consumer imports to external:: and their calls to unresolved::, but IndexRepo and track had no cross-repo pass for those consumers, so the links stayed lost until a whole-graph resolve. The lifecycle test hid this by calling RunPreEnrichResolve. Parking now returns the exact consumer files; a save carries them into the batch's cross-repo frontier, and IndexRepo and track run the scoped cross-repo resolver on consumers whose package the provider now claims. The test no longer forces a whole-graph resolve. - The binding map used an import's alias only for npm imports, so a resolved non-npm `{ f as g }` import exposed `f`. The map is now built only from hydrated cross-repo JS/TS imports and always uses the local binding name. --- internal/indexer/affected_by_bound_test.go | 2 +- internal/indexer/file_meta.go | 2 + internal/indexer/incremental_batch.go | 13 +++--- .../indexer/incremental_restub_gate_test.go | 2 +- internal/indexer/incremental_watcher_batch.go | 5 ++- internal/indexer/indexer.go | 14 ++++--- internal/indexer/multi.go | 4 +- internal/indexer/npm_import_restub.go | 37 ++++++++++++++++- internal/indexer/npm_lifecycle_test.go | 5 ++- internal/indexer/npm_tracked_package_test.go | 13 +++--- internal/resolver/cross_repo.go | 4 +- internal/resolver/npm_package_test.go | 40 +++++++++++++++++++ 12 files changed, 116 insertions(+), 25 deletions(-) diff --git a/internal/indexer/affected_by_bound_test.go b/internal/indexer/affected_by_bound_test.go index 0a5853638..812e5518c 100644 --- a/internal/indexer/affected_by_bound_test.go +++ b/internal/indexer/affected_by_bound_test.go @@ -387,7 +387,7 @@ func TestDeletionShapedStagesCarryNoInEdges(t *testing.T) { stages := []*incrementalBatchStage{{graphPath: restubDefPath, priorNodes: prior}} view := loadIncrementalPriorView(st, stages) - carried := restubIncomingRefsFromView(st, stages, view) + carried, _ := restubIncomingRefsFromView(st, stages, view) require.Empty(t, carried, "a deletion stage must park every in-edge; a carried edge has no re-state path here") assert.Equal(t, 1, st.restubWrites("Foo"), diff --git a/internal/indexer/file_meta.go b/internal/indexer/file_meta.go index 4ae4fd0d7..dd646109a 100644 --- a/internal/indexer/file_meta.go +++ b/internal/indexer/file_meta.go @@ -195,6 +195,8 @@ func persistFileMetaRows(target graph.Store, repoPrefix string, rows []graph.Fil const reparsePendingEnrichmentBatchSize = 256 type reparsePendingEnrichmentBatch struct { + // npmConsumerFiles are unchanged importers parked by this mutation. + npmConsumerFiles []string byFile map[string]bool deferResolverCatchup bool deferredAffectedFiles map[string]struct{} diff --git a/internal/indexer/incremental_batch.go b/internal/indexer/incremental_batch.go index 8caac04db..dcc9082ed 100644 --- a/internal/indexer/incremental_batch.go +++ b/internal/indexer/incremental_batch.go @@ -1092,7 +1092,10 @@ func (idx *Indexer) commitStructuralIncrementalBatch( markerBatch.recordDeferredPriorBindings(stagePriorBindings(idx.graph, stages, view)) } idx.applyLap("prior_bindings") - carried := restubIncomingRefsFromView(idx.graph, stages, view) + carried, npmConsumers := restubIncomingRefsFromView(idx.graph, stages, view) + if markerBatch != nil { + markerBatch.npmConsumerFiles = appendUniqueSorted(markerBatch.npmConsumerFiles, npmConsumers...) + } idx.applyLap("restub") // The capability state of the files, read before the eviction below // deletes it; the carried set says which incoming accesses_field rows @@ -1349,10 +1352,10 @@ func restubIncomingRefsFromView( g graph.Store, stages []*incrementalBatchStage, view incrementalPriorView, -) []*graph.Edge { +) ([]*graph.Edge, []string) { evicted := structuralPriorIDs(stages) reparsed := reparsedPaths(stages) - restubTrackedNpmImportsFromView(g, evicted, view.inByNode) + npmConsumers := restubTrackedNpmImportsFromView(g, evicted, view.inByNode) var reindexes []graph.EdgeReindex var carried []*graph.Edge carriedSeen := make(map[*graph.Edge]struct{}) @@ -1506,7 +1509,7 @@ func restubIncomingRefsFromView( if len(reindexes) > 0 { g.ReindexEdges(reindexes) } - return carried + return carried, npmConsumers } // dataflowSourceSite is a reference's site: the file that records it and its @@ -2484,7 +2487,7 @@ func (idx *Indexer) evictDeletedFilesBatched(deleted []string, plan *DerivedInva // carry therefore means a deletion stage stopped being conservative, // which would destroy those edges with no compile error to show for // it — so the assumption is checked, not just documented. - if carried := restubIncomingRefsFromView(idx.graph, stages, view); len(carried) > 0 { + if carried, _ := restubIncomingRefsFromView(idx.graph, stages, view); len(carried) > 0 { idx.logger.Error("indexer: deletion restub carried in-edges with no re-state path", zap.String("repo", idx.repoPrefix), zap.Int("carried", len(carried)), diff --git a/internal/indexer/incremental_restub_gate_test.go b/internal/indexer/incremental_restub_gate_test.go index 361fc7425..7f25e99ba 100644 --- a/internal/indexer/incremental_restub_gate_test.go +++ b/internal/indexer/incremental_restub_gate_test.go @@ -118,7 +118,7 @@ func runRestubStage( stages := []*incrementalBatchStage{stage} view := loadIncrementalPriorView(st, stages) - carried := restubIncomingRefsFromView(st, stages, view) + carried, _ := restubIncomingRefsFromView(st, stages, view) evictFilesBatched(st, []string{restubDefPath}) st.AddBatch(fresh, append(append([]*graph.Edge{}, freshEdges...), carried...)) diff --git a/internal/indexer/incremental_watcher_batch.go b/internal/indexer/incremental_watcher_batch.go index a8f32b59d..86f241bbf 100644 --- a/internal/indexer/incremental_watcher_batch.go +++ b/internal/indexer/incremental_watcher_batch.go @@ -537,6 +537,9 @@ func (mi *MultiIndexer) resolveIncrementalRepoMutationMode( batch.dropDeferredPriorBindings() } crossRepoFiles := appendUniqueSorted(nil, files...) + if batch != nil { + crossRepoFiles = appendUniqueSorted(crossRepoFiles, batch.npmConsumerFiles...) + } if result != nil { crossRepoFiles = appendUniqueSorted(crossRepoFiles, result.DerivedInvalidation.Files...) } @@ -585,7 +588,7 @@ func (mi *MultiIndexer) resolveIncrementalRepoMutationMode( } crossRepoFiles = appendUniqueSorted(crossRepoFiles, semanticFiles...) } - if needed || len(semanticFiles) > 0 { + if needed || len(semanticFiles) > 0 || batch != nil && len(batch.npmConsumerFiles) > 0 { if idx != nil { idx.observeIncrementalCatchup("cross_repo", crossRepoFiles) } diff --git a/internal/indexer/indexer.go b/internal/indexer/indexer.go index dc00096da..4d7bcbe7d 100644 --- a/internal/indexer/indexer.go +++ b/internal/indexer/indexer.go @@ -5111,7 +5111,10 @@ func (idx *Indexer) indexFile( // batch replacement by deleting its prior documents explicitly; // canonical contracts follow the store's owner-retention decision. idx.deleteSymbolFTS(oldFTSNodeIDs) - idx.restubIncomingRefs(graphPath) + npmConsumers := idx.restubIncomingRefs(graphPath) + if markerBatch != nil { + markerBatch.npmConsumerFiles = appendUniqueSorted(markerBatch.npmConsumerFiles, npmConsumers...) + } idx.graph.EvictFile(graphPath) } @@ -5866,10 +5869,10 @@ func (idx *Indexer) reresolveFileScopedRaw(filePath string) error { // agnostic: GetInEdges + ReindexEdges are the same Store primitives the // resolver uses, so this behaves identically on the in-memory and disk // stores. -func (idx *Indexer) restubIncomingRefs(graphPath string) { +func (idx *Indexer) restubIncomingRefs(graphPath string) []string { nodes := idx.graph.GetFileNodes(graphPath) if len(nodes) == 0 { - return + return nil } evicted := make(map[string]struct{}, len(nodes)) refIDs := make([]string, 0, len(nodes)) @@ -5883,13 +5886,13 @@ func (idx *Indexer) restubIncomingRefs(graphPath string) { } } if len(refIDs) == 0 { - return + return nil } // Fetch the whole changed file's incoming adjacency in one bounded store // operation. On SQLite this is chunked under the parameter limit; more // importantly, it avoids one transaction/query per definition in the file. inEdges := idx.graph.GetInEdgesByNodeIDs(refIDs) - restubTrackedNpmImportsFromView(idx.graph, evicted, inEdges) + npmConsumers := restubTrackedNpmImportsFromView(idx.graph, evicted, inEdges) var batch []graph.EdgeReindex for _, n := range nodes { if n == nil || n.Name == "" || !graph.IsReferenceableSymbol(n.Kind) { @@ -5920,6 +5923,7 @@ func (idx *Indexer) restubIncomingRefs(graphPath string) { if len(batch) > 0 { idx.graph.ReindexEdges(batch) } + return npmConsumers } // embeddingDimsOrDefault returns the embedder's reported vector width, diff --git a/internal/indexer/multi.go b/internal/indexer/multi.go index 875e88b07..a82eb7465 100644 --- a/internal/indexer/multi.go +++ b/internal/indexer/multi.go @@ -2580,8 +2580,7 @@ func (mi *MultiIndexer) indexRepoRaw(repoPrefix string) (*IndexResult, error) { oldIdx.Close() } - // TODO: After re-indexing, run CrossRepoResolver.ResolveForRepo(repoPrefix) - // to update cross-repo edges. This will be implemented in Task 7.1. + mi.runCrossRepoResolveFiles(mi.npmConsumerFilesForRepo(repoPrefix)) mi.ReconcileContractEdges() @@ -3146,6 +3145,7 @@ func (mi *MultiIndexer) trackPreparedRepoSourceCtx( mi.indexers[prefix] = idx mi.mu.Unlock() installed = true + mi.runCrossRepoResolveFiles(mi.npmConsumerFilesForRepo(prefix)) // Add to global config. entry.Path = absPath diff --git a/internal/indexer/npm_import_restub.go b/internal/indexer/npm_import_restub.go index ac2c9e199..5fffbe307 100644 --- a/internal/indexer/npm_import_restub.go +++ b/internal/indexer/npm_import_restub.go @@ -75,7 +75,8 @@ func restubTrackedNpmRepoIncoming(g graph.Store, repo string) { } } -func restubTrackedNpmImportsFromView(g graph.Store, evicted map[string]struct{}, incoming map[string][]*graph.Edge) { +func restubTrackedNpmImportsFromView(g graph.Store, evicted map[string]struct{}, incoming map[string][]*graph.Edge) []string { + var consumerFiles []string consumers := make(map[string]map[string]string) var batch []graph.EdgeReindex for _, edges := range incoming { @@ -87,6 +88,7 @@ func restubTrackedNpmImportsFromView(g graph.Store, evicted map[string]struct{}, if _, sourceEvicted := evicted[edge.From]; spec == "" || sourceEvicted { continue } + consumerFiles = appendUniqueSorted(consumerFiles, edge.From) oldTo := edge.To if _, name, symbol := strings.Cut(oldTo, "::"); symbol { if alias, _ := edge.Meta["import_alias"].(string); alias != "" { @@ -124,4 +126,37 @@ func restubTrackedNpmImportsFromView(g graph.Store, evicted map[string]struct{}, if len(batch) > 0 { g.ReindexEdges(batch) } + return consumerFiles +} + +// npmConsumerFilesForRepo finds parked imports whose package is claimed by this +// provider. Used only after a full index or track; incremental saves carry their +// exact parked consumer files directly from the restub step. +func (mi *MultiIndexer) npmConsumerFilesForRepo(repo string) []string { + lookup := mi.npmPackageLookup() + if lookup == nil { + return nil + } + files := make(map[string]struct{}) + claims := make(map[string]bool) + for edge := range graph.EdgesLightSeq(mi.graph, graph.EdgeImports) { + if !strings.HasPrefix(edge.To, "external::") { + continue + } + spec, _, _ := strings.Cut(strings.TrimPrefix(edge.To, "external::"), "::") + claimed, known := claims[spec] + if !known { + for dir := range lookup(spec) { + if dir == repo || strings.HasPrefix(dir, repo+"/") { + claimed = true + break + } + } + claims[spec] = claimed + } + if claimed { + files[edge.From] = struct{}{} + } + } + return sortedStringKeys(files) } diff --git a/internal/indexer/npm_lifecycle_test.go b/internal/indexer/npm_lifecycle_test.go index e47852b23..ba98e9b04 100644 --- a/internal/indexer/npm_lifecycle_test.go +++ b/internal/indexer/npm_lifecycle_test.go @@ -35,11 +35,12 @@ func TestTrackedNpmImportsSurviveEviction(t *testing.T) { switch mode { case "file": idx := New(g, newTestRegistry(), config.IndexConfig{}, zap.NewNop()) - idx.restubIncomingRefs("lib/index.ts") + assert.Equal(t, []string{"app/a.ts"}, idx.restubIncomingRefs("lib/index.ts")) g.EvictFile("lib/index.ts") case "batch": stages := []*incrementalBatchStage{{graphPath: "lib/index.ts", priorNodes: nodes}} - restubIncomingRefsFromView(g, stages, loadIncrementalPriorView(g, stages)) + _, consumers := restubIncomingRefsFromView(g, stages, loadIncrementalPriorView(g, stages)) + assert.Equal(t, []string{"app/a.ts"}, consumers) g.EvictFile("lib/index.ts") case "replace": evictRepoCurrentGeneration(g, "lib") diff --git a/internal/indexer/npm_tracked_package_test.go b/internal/indexer/npm_tracked_package_test.go index 756103d6a..5eaea9961 100644 --- a/internal/indexer/npm_tracked_package_test.go +++ b/internal/indexer/npm_tracked_package_test.go @@ -277,14 +277,18 @@ func TestMultiIndexer_TrackedNpmPackageLifecycle(t *testing.T) { check("warm restart") } t.Cleanup(func() { require.NoError(t, warm.Close(context.Background())) }) - writeWorkspaceFile(t, provider, "src/index.ts", "export function f(): number { return 43; }\n") - _, err = warm.IncrementalReindexRepo("provider", []string{"src/index.ts"}) + writeWorkspaceFile(t, provider, "src/index.ts", "export function f(value = 0): number { return 43 + value; }\nexport function added(): number { return 1; }\n") + saved, err := warm.IncrementalReindexRepo("provider", []string{filepath.Join(provider, "src/index.ts")}) require.NoError(t, err) - require.NoError(t, warm.RunPreEnrichResolve(context.Background(), nil, nil)) + require.Equal(t, 1, saved.StaleFileCount) + require.Empty(t, saved.FailedFiles) check("provider save") + unchanged, err := warm.IncrementalReindexRepo("provider", []string{filepath.Join(provider, "src/index.ts")}) + require.NoError(t, err) + assert.Zero(t, unchanged.StaleFileCount) + check("unchanged provider") _, err = warm.IndexRepo("provider") require.NoError(t, err) - require.NoError(t, warm.RunPreEnrichResolve(context.Background(), nil, nil)) check("provider reindex") warm.UntrackRepo("provider") var imports []string @@ -305,7 +309,6 @@ func TestMultiIndexer_TrackedNpmPackageLifecycle(t *testing.T) { assert.Equal(t, []string{"unresolved::" + binding}, pendingCalls, "provider untrack") _, err = warm.TrackRepoCtx(context.Background(), config.RepoEntry{Path: provider, Name: "provider", Workspace: "shared"}) require.NoError(t, err) - require.NoError(t, warm.RunPreEnrichResolve(context.Background(), nil, nil)) check("provider retrack") }) } diff --git a/internal/resolver/cross_repo.go b/internal/resolver/cross_repo.go index 5ca43be07..ca812f386 100644 --- a/internal/resolver/cross_repo.go +++ b/internal/resolver/cross_repo.go @@ -865,10 +865,10 @@ func (cr *CrossRepoResolver) buildReachableReposIndex() { if alias == "" { alias = e.Alias } - if isJSTSPath(e.From) { + if e.CrossRepo && isJSTSPath(e.From) { // JS/TS imports name bindings, not every symbol in the provider. if _, name, ok := strings.Cut(e.To, "::"); ok { - if npmLinked && alias != "" { + if alias != "" { name = alias } bindings := cr.jsTSBindingsByFile[e.From] diff --git a/internal/resolver/npm_package_test.go b/internal/resolver/npm_package_test.go index 68a910a40..54b99c8e3 100644 --- a/internal/resolver/npm_package_test.go +++ b/internal/resolver/npm_package_test.go @@ -576,3 +576,43 @@ func TestJSTSCrossRepoFileImportFallback(t *testing.T) { }) } } + +func TestJSTSRenamedImportPreservesSameRepoCall(t *testing.T) { + for _, backend := range []string{"memory", "sqlite"} { + for _, importedRepo := range []string{"lib", "app"} { + t.Run(backend+"/"+importedRepo, func(t *testing.T) { + var g graph.Store = graph.New() + if backend == "sqlite" { + db, err := store_sqlite.Open(filepath.Join(t.TempDir(), "graph.sqlite")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + g = db + } + target := importedRepo + "/x.ts::f" + importedName := "f" + if importedRepo == "app" { + importedName = "" // Do not add a second same-repo name candidate. + } + g.AddBatch([]*graph.Node{ + {ID: "app/a.ts", Kind: graph.KindFile, RepoPrefix: "app"}, + {ID: "app/a.ts::run", Kind: graph.KindFunction, FilePath: "app/a.ts", RepoPrefix: "app"}, + {ID: "app/b.ts::f", Kind: graph.KindFunction, Name: "f", FilePath: "app/b.ts", RepoPrefix: "app"}, + {ID: target, Kind: graph.KindFunction, Name: importedName, FilePath: importedRepo + "/x.ts", RepoPrefix: importedRepo}, + }, []*graph.Edge{ + {From: "app/a.ts", To: target, Kind: graph.EdgeImports, Alias: "g", Meta: map[string]any{"import_alias": "g"}, FilePath: "app/a.ts", CrossRepo: importedRepo != "app"}, + {From: "app/a.ts::run", To: "unresolved::f", Kind: graph.EdgeCalls, FilePath: "app/a.ts"}, + }) + NewCrossRepo(g).ResolveAll() + var calls []*graph.Edge + for _, edge := range g.GetOutEdges("app/a.ts::run") { + if edge.Kind == graph.EdgeCalls { + calls = append(calls, edge) + } + } + require.Len(t, calls, 1) + assert.Equal(t, "app/b.ts::f", calls[0].To) + assert.False(t, calls[0].CrossRepo) + }) + } + } +} From bc59652407812fab0d2f12d9e88517e226adb5d9 Mon Sep 17 00:00:00 2001 From: Peter Chanthamynavong Date: Fri, 23 Oct 2026 23:23:01 -0700 Subject: [PATCH 7/7] fix(resolver): keep npm links across warm restarts and either track order Review of the rebase onto #806 found four more cases: - On SQLite a full shadow replacement of the provider (warm restart with full_retrack) went through EvictRepoForShadowReplacement, which skipped the npm parking, so the consumer's imports and calls were deleted; main keeps them as external stubs. Parking now runs before that call too, and a full retrack relinks parked consumers when resolution is not deferred. - A warm restart after an offline provider edit parked the consumer, but the exact warm resolve saw only the provider's derived files. IndexResult now carries the parked consumer files in a separate CrossRepoResolveFiles, which the daemon's warm frontier adds to the resolve set; derived plans stay provider-only. - Tracking a consumer after its provider left it unlinked until a whole-graph pass, because only imports claimed by the newly tracked repo were retried. The repo's own external imports claimed by any tracked package are now retried too. - A root exports object whose recognized condition is null (for example {"import": null}) still fell back to types, main or index. It now blocks the root, as the array form already did. The docs now say that an import of a missing or non-public symbol links only to the entry file, and list two existing limits: lenient non-"./" export targets, and file-scoped call binding. --- cmd/gortex/daemon_state.go | 22 +++- cmd/gortex/daemon_state_warm_npm_test.go | 128 +++++++++++++++++++ docs/multi-repo.md | 4 +- internal/indexer/indexer.go | 5 + internal/indexer/multi.go | 9 +- internal/indexer/npm_import_restub.go | 26 ++-- internal/indexer/npm_tracked_package.go | 13 ++ internal/indexer/npm_tracked_package_test.go | 55 ++++++++ 8 files changed, 238 insertions(+), 24 deletions(-) create mode 100644 cmd/gortex/daemon_state_warm_npm_test.go diff --git a/cmd/gortex/daemon_state.go b/cmd/gortex/daemon_state.go index 29642d583..3a479c3da 100644 --- a/cmd/gortex/daemon_state.go +++ b/cmd/gortex/daemon_state.go @@ -315,15 +315,17 @@ type warmupTimings struct { // parallel reconcile pool. Any incomplete result fails closed to the existing // repo/global pipeline; an ordinary warm delta stays file-scoped end to end. type warmupDeltaFrontier struct { - mu sync.Mutex - exact bool - plans map[string]indexer.DerivedInvalidationPlan + mu sync.Mutex + exact bool + plans map[string]indexer.DerivedInvalidationPlan + crossRepoFiles map[string]struct{} } func newWarmupDeltaFrontier() *warmupDeltaFrontier { return &warmupDeltaFrontier{ - exact: true, - plans: make(map[string]indexer.DerivedInvalidationPlan), + exact: true, + plans: make(map[string]indexer.DerivedInvalidationPlan), + crossRepoFiles: make(map[string]struct{}), } } @@ -343,13 +345,21 @@ func (frontier *warmupDeltaFrontier) record(result *indexer.IndexResult) { plan := frontier.plans[result.RepoPrefix] plan.Merge(result.DerivedInvalidation) frontier.plans[result.RepoPrefix] = plan + for _, file := range result.CrossRepoResolveFiles { + if file != "" { + frontier.crossRepoFiles[file] = struct{}{} + } + } } func (frontier *warmupDeltaFrontier) snapshot(changedRepos int, scopeUnknown bool) (map[string]indexer.DerivedInvalidationPlan, []string, bool) { frontier.mu.Lock() defer frontier.mu.Unlock() plans := make(map[string]indexer.DerivedInvalidationPlan, len(frontier.plans)) - fileSet := make(map[string]struct{}) + fileSet := make(map[string]struct{}, len(frontier.crossRepoFiles)) + for file := range frontier.crossRepoFiles { + fileSet[file] = struct{}{} + } for prefix, plan := range frontier.plans { plans[prefix] = plan for _, file := range plan.Files { diff --git a/cmd/gortex/daemon_state_warm_npm_test.go b/cmd/gortex/daemon_state_warm_npm_test.go new file mode 100644 index 000000000..2e09bcc1b --- /dev/null +++ b/cmd/gortex/daemon_state_warm_npm_test.go @@ -0,0 +1,128 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/zzet/gortex/internal/config" + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/graph/store_sqlite" + "github.com/zzet/gortex/internal/indexer" + "github.com/zzet/gortex/internal/parser" + "github.com/zzet/gortex/internal/parser/languages" + "github.com/zzet/gortex/internal/search" + "go.uber.org/zap" +) + +func TestWarmupDeltaFrontierTrackedNpmOfflineEdit(t *testing.T) { + for _, providerFirst := range []bool{true, false} { + t.Run(strconv.FormatBool(providerFirst), func(t *testing.T) { + dir := t.TempDir() + provider, consumer := filepath.Join(dir, "provider"), filepath.Join(dir, "consumer") + write := func(rel, body string) { + t.Helper() + p := filepath.Join(dir, rel) + require.NoError(t, os.MkdirAll(filepath.Dir(p), 0o755)) + require.NoError(t, os.WriteFile(p, []byte(body), 0o644)) + } + write("provider/package.json", `{"name":"p","exports":"./src/index.ts"}`) + write("provider/src/index.ts", "export function f(): number { return 42; }\n") + for _, n := range []string{"a", "b", "c", "d"} { + write("provider/src/"+n+".ts", "export function h"+n+"(): number { return 1; }\n") + } + write("consumer/package.json", `{"name":"app","dependencies":{"p":"^1"}}`) + write("consumer/src/app.ts", "import { f } from 'p';\nexport function run(): number { return f(); }\n") + entries := []config.RepoEntry{{Path: provider, Name: "provider", Workspace: "shared"}, {Path: consumer, Name: "consumer", Workspace: "shared"}} + if !providerFirst { + entries[0], entries[1] = entries[1], entries[0] + } + cfgPath := filepath.Join(t.TempDir(), "config.yaml") + gc := &config.GlobalConfig{Repos: entries} + gc.SetConfigPath(cfgPath) + require.NoError(t, gc.Save()) + cm, err := config.NewConfigManager(cfgPath) + require.NoError(t, err) + db := filepath.Join(t.TempDir(), "graph.sqlite") + sqlite, err := store_sqlite.Open(db) + require.NoError(t, err) + t.Cleanup(func() { _ = sqlite.Close() }) + var g graph.Store = sqlite + newIndexer := func() *indexer.MultiIndexer { + reg := parser.NewRegistry() + reg.Register(languages.NewTypeScriptExtractor()) + return indexer.NewMultiIndexer(g, reg, search.NewNull(), cm, zap.NewNop()) + } + mi := newIndexer() + t.Cleanup(func() { _ = mi.Close(context.Background()) }) + for _, entry := range entries { + _, err := mi.TrackRepoCtx(context.Background(), entry) + require.NoError(t, err) + } + require.NoError(t, mi.RunPreEnrichResolve(context.Background(), nil, nil)) + check := func(stage string) { + t.Helper() + var imports, calls []string + for _, e := range g.AllEdges() { + if e.FilePath != "consumer/src/app.ts" { + continue + } + switch e.Kind { + case graph.EdgeImports: + imports = append(imports, e.To) + require.True(t, e.CrossRepo, stage) + case graph.EdgeCalls: + calls = append(calls, e.To) + require.True(t, e.CrossRepo, stage) + } + } + require.ElementsMatch(t, []string{"provider/src/index.ts", "provider/src/index.ts::f"}, imports, stage) + require.Equal(t, []string{"provider/src/index.ts::f"}, calls, stage) + } + check("cold") + mtimes := map[string]map[string]int64{} + for _, entry := range entries { + mtimes[entry.Name] = mi.FileMtimes(entry.Name) + } + require.NoError(t, mi.Close(context.Background())) + require.NoError(t, sqlite.Close()) + // Offline provider edit. + p := filepath.Join(provider, "src/index.ts") + require.NoError(t, os.WriteFile(p, []byte("export function f(v = 0): number { return 43 + v; }\n"), 0o644)) + future := time.Now().Add(5 * time.Second) + require.NoError(t, os.Chtimes(p, future, future)) + sqlite, err = store_sqlite.Open(db) + require.NoError(t, err) + g = sqlite + warm := newIndexer() + mi = warm + warm.BeginParallelBatch() + frontier := newWarmupDeltaFrontier() + require.NoError(t, warm.RunRepositoryTopologyBatch(context.Background(), func(ctx context.Context) error { + for _, entry := range entries { + res, err := warm.ReconcileRepoCtx(ctx, entry, mtimes[entry.Name]) + require.NoError(t, err) + if res != nil { + require.False(t, res.FullRetrack) + if entry.Name == "provider" { + require.Equal(t, 1, res.StaleFileCount) + require.Equal(t, []string{"provider/src/index.ts"}, res.DerivedInvalidation.Files) + } + frontier.record(res) + } + } + return nil + })) + plans, files, exact := frontier.snapshot(2, false) + require.True(t, exact) + require.Equal(t, []string{"provider/src/index.ts"}, plans["provider"].Files) + require.NoError(t, warm.RunPreEnrichResolveFiles(context.Background(), files, nil)) + warm.EndBatch() + check("after exact warm resolve") + }) + } +} diff --git a/docs/multi-repo.md b/docs/multi-repo.md index 1023c8b78..41e019c65 100644 --- a/docs/multi-repo.md +++ b/docs/multi-repo.md @@ -447,7 +447,7 @@ Scoped tool responses carry a `scope_applied` meta field plus a one-line widen h - **Qualified node IDs** — IDs are always `/::` (e.g., `frontend/src/app.ts::App`), including for a workspace tracking a single repo: a lone repo is simply the first tracked repo. Tools still accept an unqualified path (`src/app.ts`) when exactly one repo is tracked, since there is then nothing to be ambiguous about. - **Cross-repo edges** — the resolver links symbols across repo boundaries with same-repo preference. Cross-repo edges carry a `cross_repo: true` flag. JS/TS imports can link a declared npm dependency to a tracked repository whose `package.json` claims that package name, including npm and pnpm workspace members. A provider in the caller's workspace wins over eligible foreign providers. Multiple manifest claims in the winning tier keep the import external, even if only one entry is indexed. - Entry lookup respects `exports` keys, wildcard subpaths and `null` blocks. It probes the selected `import`/`default` target first, then a `types` condition when that file is not indexed. Without `exports`, it tries `types`, `main` and directory indexes. Unsupported-condition subpaths never guess a directory target. - - Bare calls use the caller file's named import binding, including its local alias, not another same-named function anywhere in the provider. A per-binding import must resolve to a symbol marked public by the extractor. Linked imports retain their specifier, survive provider eviction as external imports, and are rechecked by whole-graph resolution. - - **Limits:** Build-output-only entries, such as `main` and `types` pointing into excluded `dist/`, stay external unless an indexed target is declared. Gortex does not infer `src/` from `dist/`. The importing repository's own packages, `npm:` aliases and workspace paths that escape the repository are not linked by this pass. Sibling dependencies declared with `file:`, `link:` or `portal:` are not supported. Default-import bare calls have no exact binding evidence yet and stay unresolved. Existing linked imports from older builds need the consumer file reindexed to acquire the specifier and alias metadata. Namespace member calls, CommonJS `require` calls and package re-exports are not linked by this npm import pass. + - Bare calls use the caller file's named import binding, including its local alias, not another same-named function anywhere in the provider. A per-binding import links to a symbol marked public by the extractor; otherwise it links only to the entry file and creates no call binding. Linked imports retain their specifier, survive provider eviction as external imports, and are rechecked by whole-graph resolution. + - **Limits:** Build-output-only entries, such as `main` and `types` pointing into excluded `dist/`, stay external unless an indexed target is declared. Gortex does not infer `src/` from `dist/`. The importing repository's own packages, `npm:` aliases and workspace paths that escape the repository are not linked by this pass. Sibling dependencies declared with `file:`, `link:` or `portal:` are not supported. Default-import bare calls have no exact binding evidence yet and stay unresolved. Existing linked imports from older builds need the consumer file reindexed to acquire the specifier and alias metadata. Namespace member calls, CommonJS `require` calls and package re-exports are not linked by this npm import pass. Export targets without `./` are accepted leniently, as in workspace resolution, even though Node rejects them. Call binding is file-scoped: a parameter or local callable with the imported name can bind to the import. - **Impact analysis** — `explain_change_impact`, `verify_change`, and `get_test_targets` follow cross-repo edges automatically, grouping results by repository. - **Shared repos** — the same repo can appear in multiple projects with different reference tags. It's indexed once and shared across projects. diff --git a/internal/indexer/indexer.go b/internal/indexer/indexer.go index 4d7bcbe7d..f8c46d30c 100644 --- a/internal/indexer/indexer.go +++ b/internal/indexer/indexer.go @@ -131,6 +131,9 @@ type IndexResult struct { // incremental reconcile. A zero plan proves that no graph-wide derived // pass is required; callers must not infer work merely from stale count. DerivedInvalidation DerivedInvalidationPlan `json:"derived_invalidation,omitempty"` + // CrossRepoResolveFiles contains parked consumers outside this repository's + // derived frontier. Batch callers must include them in cross-repo resolution. + CrossRepoResolveFiles []string `json:"cross_repo_resolve_files,omitempty"` // mutationErr carries a point-operation compatibility error through the // resolver/semantic/derived tail. It is intentionally unexported and never @@ -3409,6 +3412,7 @@ func (idx *Indexer) indexCtxRaw(ctx context.Context, root string) (result *Index if sqlite, ok := diskTarget.(*store_sqlite.Store); ok && sqlite.SupportsShadowReplacementRetirement() { var err error shadowReplacementStarted = true + restubTrackedNpmRepoIncoming(diskTarget, idx.RepoPrefix()) evictedNodes, evictedEdges, err = sqlite.EvictRepoForShadowReplacement(ctx, idx.RepoPrefix()) if err != nil { retErr = fmt.Errorf("indexer: retire shadow replacement rows: %w", err) @@ -7315,6 +7319,7 @@ func (idx *Indexer) incrementalReindexPathsMode( CountsCached: countsCached, capabilityPrior: idx.takeCapabilityPrior(), } + result.CrossRepoResolveFiles = appendUniqueSorted(nil, markerBatch.npmConsumerFiles...) for _, path := range failedFiles { result.Errors = append(result.Errors, IndexError{FilePath: path, Error: idx.fileIndexFailureError(path).Error()}) } diff --git a/internal/indexer/multi.go b/internal/indexer/multi.go index a82eb7465..992a13df7 100644 --- a/internal/indexer/multi.go +++ b/internal/indexer/multi.go @@ -3438,8 +3438,13 @@ func (mi *MultiIndexer) ReconcileRepoCtx(ctx context.Context, entry config.RepoE prefix: result.DerivedInvalidation, }) } - } else if result.FullRetrack && !batchMode.deferGlobalPasses { - mi.ReconcileContractEdges() + } else if result.FullRetrack { + if !batchMode.deferResolve { + mi.runCrossRepoResolveFiles(mi.npmConsumerFilesForRepo(prefix)) + } + if !batchMode.deferGlobalPasses { + mi.ReconcileContractEdges() + } } publishTiming.complete(nil) diff --git a/internal/indexer/npm_import_restub.go b/internal/indexer/npm_import_restub.go index 5fffbe307..4cf8651f7 100644 --- a/internal/indexer/npm_import_restub.go +++ b/internal/indexer/npm_import_restub.go @@ -129,33 +129,31 @@ func restubTrackedNpmImportsFromView(g graph.Store, evicted map[string]struct{}, return consumerFiles } -// npmConsumerFilesForRepo finds parked imports whose package is claimed by this -// provider. Used only after a full index or track; incremental saves carry their -// exact parked consumer files directly from the restub step. +// npmConsumerFilesForRepo finds parked imports claimed by this provider and +// this repo's imports claimed by any tracked provider. Used after a full index +// or track; incremental saves carry exact consumer files from the restub step. func (mi *MultiIndexer) npmConsumerFilesForRepo(repo string) []string { lookup := mi.npmPackageLookup() if lookup == nil { return nil } files := make(map[string]struct{}) - claims := make(map[string]bool) + claims := make(map[string]map[string]resolver.NpmPackageTarget) for edge := range graph.EdgesLightSeq(mi.graph, graph.EdgeImports) { if !strings.HasPrefix(edge.To, "external::") { continue } spec, _, _ := strings.Cut(strings.TrimPrefix(edge.To, "external::"), "::") - claimed, known := claims[spec] + providers, known := claims[spec] if !known { - for dir := range lookup(spec) { - if dir == repo || strings.HasPrefix(dir, repo+"/") { - claimed = true - break - } - } - claims[spec] = claimed + providers = lookup(spec) + claims[spec] = providers } - if claimed { - files[edge.From] = struct{}{} + for dir := range providers { + if dir == repo || strings.HasPrefix(dir, repo+"/") || graph.RepoPrefixOfID(edge.From) == repo { + files[edge.From] = struct{}{} + break + } } } return sortedStringKeys(files) diff --git a/internal/indexer/npm_tracked_package.go b/internal/indexer/npm_tracked_package.go index a07588b82..41c3386ea 100644 --- a/internal/indexer/npm_tracked_package.go +++ b/internal/indexer/npm_tracked_package.go @@ -24,6 +24,8 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { hasExports bool types string main string + + recognizedRoot bool } roots := map[string]string{} workspaces := map[string]string{} @@ -71,6 +73,12 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { if json.Unmarshal(src, &manifest) != nil || manifest.Name == "" { continue } + var conditions map[string]json.RawMessage + _ = json.Unmarshal(manifest.Exports, &conditions) + if root, ok := conditions["."]; ok { + conditions = nil + _ = json.Unmarshal(root, &conditions) + } exports := parsePackageExports(src) declared := declaredNpmExports(manifest.Exports) // Keep unsupported and blocked keys in the target map so an @@ -83,6 +91,8 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { } } packages[manifest.Name] = append(packages[manifest.Name], entry{ + recognizedRoot: hasRecognizedNpmCondition(conditions), + dir: path.Join(prefix, dir), workspaceID: workspaces[prefix], exports: exports, declared: declared, typesExports: npmTypesExportTargets(manifest.Exports), arrayRoot: npmExportsArrayRoot(manifest.Exports), hasExports: hasPackageExports(manifest.Exports), types: manifest.Types, main: manifest.Main, }) } @@ -113,6 +123,9 @@ func (mi *MultiIndexer) npmPackageLookup() resolver.NpmPackageLookup { } target = pkg.arrayRoot.target } + if sub == "" && target == "" && pkg.recognizedRoot { + continue // A recognized null/empty root blocks legacy candidates. + } // Root entries with unsupported conditions keep legacy candidates. // A subpath must name its target; never invent a directory probe. paths := []string{target, resolveExportsSubpath(pkg.typesExports, sub)} diff --git a/internal/indexer/npm_tracked_package_test.go b/internal/indexer/npm_tracked_package_test.go index 5eaea9961..c0e905e1c 100644 --- a/internal/indexer/npm_tracked_package_test.go +++ b/internal/indexer/npm_tracked_package_test.go @@ -38,6 +38,11 @@ func TestMultiIndexer_TrackedNpmPackage(t *testing.T) { {name: "exports missing target blocks index", packageName: "p", manifest: `"exports":"./missing.ts"`, entry: "index.ts"}, {name: "exports subpath only blocks root", packageName: "p", manifest: `"exports":{"./utils":"./utils.ts"},"types":"./index.ts","main":"./index.ts"`, entry: "index.ts"}, {name: "exports empty object blocks root", packageName: "p", manifest: `"exports":{}`, entry: "index.ts"}, + {name: "exports import null blocks main", packageName: "p", manifest: `"main":"index.ts","exports":{"import":null}`, entry: "index.ts"}, + {name: "exports import null blocks require", packageName: "p", manifest: `"exports":{"import":null,"require":"./cjs.js"}`, entry: "index.ts"}, + {name: "exports default null blocks index", packageName: "p", manifest: `"exports":{"default":null}`, entry: "index.ts"}, + {name: "exports dot import null blocks types", packageName: "p", manifest: `"types":"index.ts","exports":{".":{"import":null}}`, entry: "index.ts"}, + {name: "exports node condition keeps legacy index", packageName: "p", manifest: `"exports":{"node":"./n.js"}`, entry: "index.ts", resolves: true}, {name: "exports null uses legacy index", packageName: "p", manifest: `"exports":null`, entry: "index.ts", resolves: true}, {name: "exports empty string blocks subpath", packageName: "p", specifier: "p/private", manifest: `"exports":""`, entry: "private.ts"}, {name: "exports root-only unsupported conditions block subpath", packageName: "p", specifier: "p/private", manifest: `"exports":{"browser":"./index.ts"}`, entry: "private.ts"}, @@ -235,6 +240,21 @@ func TestMultiIndexer_TrackedNpmPackageLifecycle(t *testing.T) { _, err := mi.TrackRepoCtx(context.Background(), entry) require.NoError(t, err) } + // Live tracking must link in either order without a whole-store pass. + var liveImports, liveCalls []string + for _, e := range g.AllEdges() { + if e.FilePath != "consumer/src/app.ts" { + continue + } + if e.Kind == graph.EdgeImports { + liveImports = append(liveImports, e.To) + } + if e.Kind == graph.EdgeCalls { + liveCalls = append(liveCalls, e.To) + } + } + assert.ElementsMatch(t, []string{"provider/src/index.ts", "provider/src/index.ts::f"}, liveImports) + assert.Equal(t, []string{"provider/src/index.ts::f"}, liveCalls) // The daemon runs this whole-store resolve after cold per-repo tracking. require.NoError(t, mi.RunPreEnrichResolve(context.Background(), nil, nil)) check := func(stage string) { @@ -275,6 +295,41 @@ func TestMultiIndexer_TrackedNpmPackageLifecycle(t *testing.T) { } // A clean warm restart skips the global pass and keeps persisted links. check("warm restart") + require.NoError(t, warm.Close(context.Background())) + require.NoError(t, sqlite.Close()) + sqlite, err = store_sqlite.Open(db) + require.NoError(t, err) + g = sqlite + warm = newIndexer() + t.Setenv("GORTEX_WARMUP_FULL_RETRACK", "1") + for _, entry := range entries { + result, err := warm.ReconcileRepoCtx(context.Background(), entry, mtimes[entry.Name]) + require.NoError(t, err) + require.True(t, result.FullRetrack) + if entry.Name == "provider" { + if providerFirst { + // The consumer is not restored yet: retain its parked edges. + var imports, calls []string + for _, e := range g.GetOutEdges("consumer/src/app.ts") { + if e.Kind == graph.EdgeImports { + imports = append(imports, e.To) + } + } + for _, e := range g.GetOutEdges("consumer/src/app.ts::run") { + if e.Kind == graph.EdgeCalls { + calls = append(calls, e.To) + } + } + assert.ElementsMatch(t, []string{"external::p", "external::p::f"}, imports, "provider full retrack") + assert.Equal(t, []string{"unresolved::" + binding}, calls, "provider full retrack") + } else { + check("provider full retrack") + } + } + } + check("warm full retrack") + require.NoError(t, warm.RunPreEnrichResolve(context.Background(), nil, nil)) + check("full retrack global resolve") } t.Cleanup(func() { require.NoError(t, warm.Close(context.Background())) }) writeWorkspaceFile(t, provider, "src/index.ts", "export function f(value = 0): number { return 43 + value; }\nexport function added(): number { return 1; }\n")