fix(resolution): a binding in a module that exports nothing is not a cross-file candidate

[bompus]

Bare-name fallback can connect imports and calls to private JS/TS bindings or documentation headings. Removing an unreachable candidate before ranking can also promote a different unrelated target.

This extends the module-boundary fix for #1719:

• Reject an unreachable call target after ranking, without promoting a runner-up. Preserve import ranking among modules that can export bindings.
• Ignore ESM export examples in strings/comments when checking module visibility, while preserving actual exports, later exports, regex literals, CommonJS exports, and ambient globals.
• Reject JSON data as a call target and reject code imports into Markdown through both name matching and framework resolution.
• Keep same-file calls, real middleware imports/calls, local file dependencies, and Markdown-origin references covered by positive controls.

Markdown fixtures inject indexed heading nodes, so this update can be reviewed before #1699's extractor lands. The broader export-status redesign in #1721 remains separate.

Validation on Windows x64, Node 24.16.0:

• Full build passed.
• Four negative regression fixtures fail on the previous fix(resolution): a binding in a module that exports nothing is not a cross-file candidate #1720 head (247413c) and pass with this update.
• Focused tests: 32 passed with the native kernel required; 32 passed with WASM forced (226 tests deliberately excluded by the focus filter).
• Full suite: 4,181 passed, 24 failed, 44 skipped. All failures are temporary-directory EPERM cleanup errors. Twenty-three match the recorded upstream-main failures; the additional daemon cleanup failure passed all 10 tests on an isolated rerun. The original full-run failures remain recorded. Existing Windows cleanup work is tracked in test: close the handles and children Windows teardown trips over #1717.
• The combined patch applies cleanly to upstream main at b9ca4b7.

The update changes six code/test files and contains no fork-specific README or benchmark-data changes.

[danusha2345]

Reproduced on the same checkout (vite 8492422, base b9ca4b7, edge sets joined back to symbols): LOST 320, GAINED 18, all 18 exact-match, fuzzy 13 → 13. The 18 are the six targets you list (create-vite/src/index.ts::start ×5, parse.spec.ts::run ×4, build.spec.ts::dirname ×4, decoder.ts::version ×3, msg, content). Your test passes here too (190/190 in resolution.test.ts).

Composition with #1718. The two auto-merge — adjacent lines in matchFuzzy, no conflict — and stacked on b9ca4b7 the result is exactly additive: LOST 332, GAINED 18, fuzzy 13 → 1; against #1720 alone it is LOST 12 / GAINED 0, against #1718 alone LOST 320 / GAINED 18. Both PRs' test files pass on the merge (195/195). So there is no ordering question between them either.

One shape note, not a blocker. In matchFuzzy the sealed-module test is applied as a filter on the candidate set, which is the form #1718 moves off for reachability: thinning a crowd to one survivor lets the strategy commit to it, and that is the same mechanism as the 18 you documented on the exact-match side, just one strategy over. On vite it costs nothing — fuzzy is 13 → 13 alone and 13 → 1 stacked — so there is no instance to point at, only the predicate. If #1718 lands first, the cheap version is to put the check beside the reachability one on the survivor:

if (finalCandidates.length === 1 && isLexicallyReachable(finalCandidates[0]!, ref, context) && isCrossFileReachable(finalCandidates[0]!, ref, context)) {

That has no more order-sensitivity than the length === 1 test already there, so it should not trip #874 the way the exact-match variant you tried did. Happy to leave it to whichever PR merges second.

Two limits of the source test, both rare, both worth a line in the doc comment rather than a change: a module with imports, no exports and a declare global { … } block is sealed although its globals are reachable from every file; and exports["x"] = … (bracket form) is not matched as a CommonJS export, so such a file would be sealed too. Neither occurs on vite.

The removals themselves I did not re-audit file by file; your 320/320 claim is about the target files' source, and the 14-file table reads as the expected shape (entry scripts and specs).

[bompus]

Thanks for reproducing it independently — and the #1718 stack result is more than I had; I had only measured against #1715.

First, a correction you should not have to discover: the #874 claim in my description was wrong, and you repeated it back to me. I withdraw it.

index-command.test.ts is flaky on my Windows host, full stop. Five runs of that file alone:

Branch	Result	Which test
this branch	1 failed, 4 clean	#874 > is idempotent
unpatched b9ca4b7	1 failed, 4 clean	#1067 > rebuilds to current disk state

A different test each time, and once with no patch at all. The variant could not have caused it in any case: #874's fixture is two files that both open with export function, so neither is ever sealed and the rule is inert on it. I should have checked that before publishing a mechanism. Description updated.

The variant is still rejected, on the real reason. It removes 57 rows this branch keeps, and 41 of them are same-file — resolve.spec.ts calling its own createTestServer, external.ts its own createIsExternal, six spec files their own getSourceMapUrl. "A sealed sibling was dropped" is a property of the whole candidate set, so a legitimate local resolution gets declined because some unrelated file elsewhere happened to be sealed. Of the 16 cross-file ones several also look right (hmr-full-bundle-mode/main.js → hmr.js::assetUrl, relative, same directory). So it buys 9 fewer manufactured edges by destroying 41 correct ones — not a trade, a bug.

Your two limits: both fixed in code rather than documented, and the second is bigger than you framed it. declare global is not only about globals being conceptually reachable — the extractor emits real nodes for ambient members:

strayVar:  variable@ambient.ts
StrayFace: interface@ambient.ts

(the bare function stray(): number signature yields none). So a sealed ambient file would hide nodes that genuinely are reachable from every file. The predicate now reads:

/^[ \t]*export[\s{*]|^[ \t]*declare\s+global\b|\bmodule\.exports\b|\bexports\s*[.[]/m

which covers the bracket form too. You were right that neither occurs on vite — I re-ran the arm rather than take it on trust, and the widened predicate gives an edge set identical to the previous revision's, LOST 0 / GAINED 0 against it, so your 320/18 still holds row for row. The test now covers both shapes, asserting through getIncomingEdges so the non-callable ambient var is asked the same question as the functions.

On the matchFuzzy shape note — agreed, and I'd rather it went in whichever of us merges second, as you suggest. Applying it as a survivor test instead of a set filter is the better form for the reason you give, and since fuzzy is 13 → 13 here there is no measurement that distinguishes them on this corpus; it is a predicate argument, and it holds.

Head is now 172d9f7. Suite unchanged at 4,172 passing / 23 failing, the same 23 that fail on unpatched b9ca4b7.

[bompus]

All three open name-matcher PRs stacked on their shared base and measured on the same corpus, plus a fixture defect in this PR that the stack exposed.

Composition

vitejs/vite @ 8492422, all arms from base b9ca4b7, kernel purged and rebuilt per arm, db copied out immediately after indexing. #1715 is d0efd27, #1718 is 4c8f165, #1720 is 172d9f7. Only CHANGELOG.md conflicted between #1715 and #1718; name-matcher.ts conflicted between the #1715 side and #1720 in two spots, both pure additions sharing a closing brace, resolved by keeping both — no logic was rewritten to make them fit.

Arm	Edges	vs base
base b9ca4b7	27,778	—
#1720 alone	27,476	320 removed / 18 added
#1715 alone	25,291	2,487 net
#1715 + #1718	25,283	2,495 net
all three	24,986	2,805 removed / 13 added

The arms interact; the deltas are not additive

On top of	#1720 contributes
base	320 removed / 18 added
#1715 + #1718	310 removed / 13 added

So #1720 overlaps the other two by only ~10 of its 320 removals and 5 of its 18 additions — they are very largely complementary. #1718 shows the same effect from the other side: 12 standalone, but 8 on top of #1715.

The stack found a defect in this PR's test, not in the others

The three-way suite ran 4,205 passing / 24 failing. Twenty-three of those fail on unpatched b9ca4b7 too. The twenty-fourth was this PR's own #1719 test, and #1715 was the cause — confirmed by building #1720 + #1715 directly rather than by elimination.

The defect was mine. The fixture bound every name through import { widget, gadget, … } from 'some-external-pkg'. A bare specifier names a package that is not in the graph, so no project node is the right target for such a reference — which is exactly what #1715 declines, correctly. Four of the five positive assertions were therefore riding on a resolution that should not happen. Rebinding them as free references reaches the same exact-match path without asserting it, and the test then passes both alone and stacked on #1715.

A fifth assertion was worse than wrong. strayVar — the declare global coverage added earlier in this thread — turns out never to have been observable: a bare identifier read (console.log(strayVar)) emits no edge at all, so that assertion only ever passed through the bare-import binding it should not have had. It is replaced by an interface inside declare global, reached through a type annotation, paired with a control file of identical shape whose interface is not in a declare global. The pair differs only by that clause, so the assertion discriminates on it (StrayFace reached via references/exact-match, HiddenFace not) instead of passing whichever way the guard goes.

Pushed as b88e24c. The guard itself is unchanged — the 320/18 measurement stands, and the independent reproduction at exactly 320/18 is unaffected.

On the matchFuzzy shape

The merge puts both patterns in one function, which makes the inconsistency raised earlier concrete: #1720 filters the candidate set before the survivor count, while #1718 tests the single survivor after it. They are not equivalent under a set-wide predicate — the earlier decline-variant of this PR removed 57 more edges than the filter form, 41 of them same-file, precisely because a set-wide property declined a legitimate local resolution when an unrelated file elsewhere was sealed. Worth settling in one place rather than per-PR.

[bompus]

Restating the suite line from the matrix above, now that the fixture is fixed. Same three-way tree, only the test file changed, so the edge counts are unaffected — 24,986, and the 310 removed / 13 added that #1720 contributes on top of #1715 + #1718 both stand.

The three-way suite is 4,205 passing / 24 failing, and the #1719 test is no longer among them. The count is unchanged but the set is not: exactly one name swapped.

gone	Bindings in a module that exports nothing (#1719) > drops them as cross-file candidates…
new	Shared MCP daemon (issue #411) > does NOT reap a live-but-quiet client on the inactivity backstop (#692)

The daemon test passes 10/10 on three consecutive isolated runs of mcp-daemon.test.ts on this same tree, so it is a flake under full-suite load rather than anything the stack does — the change touched only a resolution fixture and cannot reach it. That leaves 23 failures, which is the set that fails on unpatched b9ca4b7.

Worth flagging for anyone reproducing the matrix: an equal failure count across two arms is not the same set, and here it hid both a real fix and a new flake. The arms are also worth committing before building from them — two of mine were resolved merges left in a working tree that a later git checkout -f silently discarded, which produced arms labelled as stacked while actually holding a single PR. Neither failed loudly; the branch-deletion output is what exposed it.

[danusha2345]

Rebuilt the three-way arm here to settle the matchFuzzy shape by measurement rather than argument.

Matrix reproduces. Same merge, same two name-matcher.ts hunks kept both-sides, CHANGELOG.md by union. My base sits 9 rows above yours (wasm arm; I have not staged the kernel on this box), so the comparison is on deltas: all three on b9ca4b7 gives me 2,836 removed / 44 added against your 2,805 / 13 — and the gap is my row key, which includes resolvedBy. 29 PluginContext rows (import type { PluginContext } from 'rolldown' → pluginContainer.ts) only change resolver, from exact-match to a framework resolver at 0.8, once #1715 stops exact-match binding them; keyed on source/target/kind as you do, that is 2,807 / 15, inside the wasm-vs-kernel noise. All five test files across the three PRs pass on the merge (224).

The shape. With the three merged, I moved the sealed test off the fuzzy candidate set and onto the survivor, beside the reachability one:

const callableCandidates = applyLanguageGate(candidates.filter((n) => callableKinds.has(n.kind)), ref);
…
if (
  finalCandidates.length === 1 &&
  isLexicallyReachable(finalCandidates[0]!, ref, context) &&
  isCrossFileReachable(finalCandidates[0]!, ref, context)
) {

Re-indexed vite: row-identical to the filter form — 24,995 both ways, LOST 0 / GAINED 0 between them, fuzzy 1 in both — and the three PRs' tests still pass (206 across the three fuzzy/resolution files). So on this corpus there is no instance either way; what the survivor form buys is closing the manufacturing path for the case that has one.

On your 57 / 41 same-file number: that was the exact-match decline-variant, where "the guard created the uniqueness" is a set-wide property. The survivor form in fuzzy is not set-wide — isCrossFileReachable already exempts a same-file candidate, and a [sealed X, same-file Y] pair that fuzzy would now decline as a crowd of two is resolved by exact-match first (two candidates → findBestMatch, and the same-file proximity term picks Y), so fuzzy never sees it. Which is why I would settle it as: in matchFuzzy, every per-candidate test sits on the survivor; in matchByExactName, the sealed filter stays as this PR has it, since the decline-variant there trips #874 and the 18 it would have saved are the price of determinism.

It is a three-line change. Happy to carry it in #1718 if that merges first, or you can take it here if this one does — either way the diff is the block above.

Thanks for tracing the fixture failure to #1715 rather than to the stack; declining import { widget } from 'some-external-pkg' is exactly what it is for.

[bompus]

Taken here, as 247413c — matchFuzzy now tests the survivor and filters nothing; matchByExactName keeps the filter. Your framing is what settles it for me: fuzzy declines an ambiguous name outright, so a filter there can only ever turn a decline into a 0.5 guess, while exact-match ranks a crowd instead of declining it, which is exactly the case a filter is for. That is the split written into the isCrossFileReachable doc comment now, so the next reader does not have to re-derive why the same predicate is applied at two different points.

Thank you for reproducing it on your base rather than arguing the shape — and for isolating the 29 PluginContext rows to resolvedBy in your row key. 2,807 / 15 against my 2,805 / 13 is the wasm-vs-kernel noise I would expect; I have not staged the kernel comparison the other way, so I am taking your 24,995-both-ways row-identical result as measured rather than reproducing it: no vite checkout survives on this box, and the change can only remove edges, never add them.

One shape it declines that the filter form resolved, which your corpus has no instance of and which is a second effect, not the manufactured-uniqueness one. For a candidate set [sealed X (ts), real Y (js)] under a ts ref, the filter form drops X before the same-language split, so Y survives and resolves cross-language at 0.3; the survivor form keeps X as the same-language survivor, finds it sealed, and declines. Strictly a decline, and a 0.3 cross-language fuzzy edge onto a name whose same-language holder is sealed is not one I would defend — but it is a behavior change beyond the three lines' stated purpose, so it belongs on the record rather than in someone's later bisect. It is in the commit message too.

Local checks: tsc --noEmit clean, and resolution.test.ts passes the sealed-module cases unchanged. The four reds in that file on my Windows host are fs.rmSync EPERM in a finally on PHP/C++ temp dirs — the assertions passed, the cleanup did not — which is the same host flake as the 23 in the description.

If #1718 merges first I will rebase onto it; the survivor block then just gains your isLexicallyReachable term beside this one, which is the form you quoted.

[bompus]

@colbymchenry, could you review the updated head 8ec7374?

It rejects invalid call targets without promoting unrelated replacements, handles export examples in strings/comments, and prevents JSON data and Markdown headings from becoming inappropriate targets. Four negative fixtures reproduce on the previous head and pass with the update.

The build passes, and 32 focused tests pass with native extraction and again with WASM forced. The full Windows run is 4,181 passed / 24 cleanup failures / 44 skipped: 23 failures match the recorded upstream baseline; the additional daemon cleanup failure passes its isolated 10-test rerun. Details are in the updated PR description.

[danusha2345]

Measured 8ec7374 the same way as the earlier heads (vite 8492422, base b9ca4b7, wasm arm, rows keyed with resolvedBy).

Standalone: 27,462 edges, LOST 333 / GAINED 8 against base (was 320 / 18 at 86b40ce); resolution.test.ts, frameworks-integration.test.ts and index-command.test.ts — the #874 re-index test — all pass, 226. So the reject-after-ranking form for calls does not trip #874 here, which was the thing to check.

Stacked on #1715 + #1718 (merge: the import line and the fuzzy survivor block, resolved as you predicted — your block plus the isLexicallyReachable term): 24,977 edges, base → 2,843 removed / 33 added, of which 29 are the PluginContext resolvedBy moves; against the previous three-way it is LOST 18 / GAINED 0. All seven test files across the three PRs pass, 260 including #874.

One thing this head does that the previous one did not, and it is the imports branch. Against 86b40ce, four rows appear:

import { dirname } from 'node:path'   (pluginConverter.ts, searchRoot.ts, ssrManifestPlugin.ts, ssr/runtime/__tests__/utils.ts)
  → packages/vite/src/node/__tests__/plugins/css.spec.ts::dirname       (const dirname = import.meta.dirname, unexported)

The mechanism is the one the description names for calls: build.spec.ts is now sealed, its dirname leaves the candidate set, and for imports the set is still filtered, so css.spec.ts's dirname is the runner-up promoted to sole survivor. The reason css.spec.ts is not sealed itself is the CommonJS test running on raw source: the file's fixtures contain exports.foo = foo; inside template literals (lines 621, 638, 655, 670, 728), and HAS_CJS_EXPORT.test(source) reads that as a real export. On vite exactly two files are un-sealed only by that raw test — config.spec.ts and css.spec.ts — and both hold CommonJS fixtures in strings. In the stack #1715 absorbs all four (the binding is node:path), so the three-way gained list has no trace of them; standalone they are the same class as the 18.

Two ways to close it, either fine: run the CJS test on the masked code like the ESM one and accept that a module.exports written inside a ${…} interpolation is missed (I could not find one on vite, and a file that exports only from inside a template literal is a fixture, not a module); or keep the raw test but make imports reject-after-ranking too, the way calls now does, so a filtered-out sealed candidate can never promote a runner-up. The second is the smaller diff and is consistent with the rule the description states.

Nothing else moved between the two heads that I can see: the 27 extra removals are build.spec.ts::dirname (16), create-vite/src/index.ts::start (6), parse.spec.ts::run (4) and one dep-json-require row, all from the 18 the previous head had added.