fix(resolver): link a declared npm dependency to its tracked repository by peterkc · Pull Request #39 · peterkc/gortex

peterkc · 2026-10-22T18:18:37Z

Summary

A JS/TS import of an npm package that the importer declares in package.json stayed external:: even when the package's source repository was tracked. The declared-dependency skip ends the directory cascade, and the package entry file has no package-qualified name. The cross-repo pass now retries these imports and links them, and the calls through them, to the tracked repository.

Diagrams

Before
  consumer: import { f } from "@scope/p"; f()
    import -> external::@scope/p::f         <-- declared dep, cascade skipped
    call   -> unresolved::f

After
  cross-repo pass, before call reachability:
    "@scope/p" -> tracked repo whose package.json name is "@scope/p"
               -> entry: exports, types, main, root index
    import -> provider/src/index.ts::f      (public TS symbol, else the file)
    call   -> provider/src/index.ts::f      (CrossRepo)

Changes

Map a declared package name to a tracked repository through package.json names and npm or pnpm workspace members, then pick the entry through exports (exact, conditional, wildcard, subpaths), types, main or the root index.
Bind the imported symbol only when the TypeScript extractor marks it public; otherwise bind the file.
Leave the import 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 c821417 is unchanged.
Cost: manifests are read lazily, once per resolver, only when a declared import reaches the lookup. Rejections run on light edge projections; hydration is paged and probes are memoized per pass.
Limit: a JavaScript package binds to the file, because only the TypeScript extractor records visibility.
Limit: 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.

Testing

All tests pass (go test -race ./...)
New tests added for new functionality
Benchmarks run if performance-relevant
Two-repo fixtures fail on main (import external, call unresolved) and pass here, on the in-memory graph and SQLite.
SQLite per-repo TrackRepoCtx in both orders, then the daemon's whole-graph resolve, links; links survive close, reopen and warm reconcile.
Focused -race, go vet and golangci-lint pass on internal/resolver and internal/indexer. The full suite was not run locally.

Checklist

Code follows existing patterns in the codebase
No unnecessary abstractions added
Language extractor includes Meta["methods"] for interfaces (if applicable)
Methods have EdgeMemberOf edges to their containing type (if applicable)

coderabbitai · 2026-10-22T18:18:49Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one): 1

coderabbit

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Central YAML; Path: .coderabbit.yaml
Review profile: CHILL
Plan: Team
Run ID: b8301674-c5fe-4947-85ec-bc34d07b5ea2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

peterkc · 2026-10-22T20:27:58Z

@cubic review

cubic-dev-ai · 2026-10-22T20:28:03Z

@cubic review

@peterkc I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 7 files

_{Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic}

peterkc · 2026-10-22T21:13:40Z

@cubic-dev-ai incremental review

peterkc · 2026-10-22T21:13:47Z

Continued upstream as zzet#952.

cubic-dev-ai · 2026-10-22T21:13:49Z

@cubic-dev-ai incremental review

@peterkc I’ve started reviewing the changes since the last completed review. I’ll post the results here when it’s done.

cubic-dev-ai

All reported issues were addressed across 5 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic}

peterkc · 2026-10-22T21:33:59Z

@cubic-dev-ai incremental review

cubic-dev-ai · 2026-10-22T21:34:05Z

@cubic-dev-ai incremental review

@peterkc I’ve started reviewing the changes since the last completed review. I’ll post the results here when it’s done.

cubic-dev-ai

1 issue found across 3 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic}

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/indexer/npm_tracked_package.go">

<violation number="1" location="internal/indexer/npm_tracked_package.go:203">
P2: This treats recognized conditions with null or invalid targets as unsupported conditions, so an exports array such as `[{"import":null}]` falls back to `main` or `index`. Distinguish unknown-only condition objects from recognized conditions that have no valid target, which should not enable the legacy root fallback.</violation>
</file>

peterkc · 2026-10-22T21:44:38Z

@cubic-dev-ai incremental review

cubic-dev-ai · 2026-10-22T21:44:44Z

@cubic-dev-ai incremental review

@peterkc I’ve started reviewing the changes since the last completed review. I’ll post the results here when it’s done.

cubic-dev-ai

0 issues found across 2 files (changes from recent commits).

_{Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Turn on auto-fix | Re-trigger cubic}

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

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.

…gets 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.

…tion 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.

… 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::<spec> 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.

…track After the rebase onto zzet#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.

…rder Review of the rebase onto zzet#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.

cubic-dev-ai Bot reviewed Oct 22, 2026

View reviewed changes

Comment thread internal/indexer/workspace_resolve.go

Comment thread internal/resolver/cross_repo.go Outdated

Comment thread internal/indexer/npm_tracked_package.go Outdated

Comment thread internal/indexer/npm_tracked_package.go Outdated

peterkc force-pushed the fix/npm-dep-tracked-repo branch from a98c8cf to 799179f Compare October 22, 2026 21:09

cubic-dev-ai Bot reviewed Oct 22, 2026

View reviewed changes

Comment thread internal/indexer/npm_tracked_package.go

Comment thread internal/resolver/npm_package.go Outdated

cubic-dev-ai Bot reviewed Oct 22, 2026

View reviewed changes

Comment thread internal/indexer/npm_tracked_package.go Outdated

cubic-dev-ai Bot reviewed Oct 22, 2026

View reviewed changes

peterkc added 7 commits October 23, 2026 12:55

peterkc force-pushed the fix/npm-dep-tracked-repo branch from 780a691 to bc59652 Compare October 24, 2026 06:36

peterkc closed this Oct 26, 2026

peterkc deleted the fix/npm-dep-tracked-repo branch October 26, 2026 18:59

Conversation

peterkc commented Oct 22, 2026

Summary

Changes

Testing

Checklist

Uh oh!

coderabbitai Bot commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

peterkc commented Oct 22, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 22, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peterkc commented Oct 22, 2026

Uh oh!

peterkc commented Oct 22, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 22, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

peterkc commented Oct 22, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 22, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peterkc commented Oct 22, 2026

Uh oh!

cubic-dev-ai Bot commented Oct 22, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Oct 22, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading