fix(sea): files inside symlinks are not resolved correctly (#295)

[mpotthoff]

Fixes #295

This change does require us to always walk up the full path hierarchy to detect any parent symlinks, which will worsen the performance. I also had to remove the object-has-key fast path.
To at least keep the performance the same for projects that don't use any symlinks, I added a precomputed flag that determines whether any symlink exists. If there are no symlinks, we can immediately return out of the function.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.75%. Comparing base (8d3d7af) to head (843326b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #296      +/-   ##
==========================================
- Coverage   87.19%   86.75%   -0.45%     
==========================================
  Files          23       23              
  Lines        7929     7929              
  Branches     1214     1217       +3     
==========================================
- Hits         6914     6879      -35     
- Misses       1008     1042      +34     
- Partials        7        8       +1     

see 4 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[robertsLando]

Review summary

Verdict: Ship with changes — one Major (perf) worth landing first.

The issue and the fix are both confirmed — verified end-to-end, not just read

The bug is real: lib/walker.ts:459 records exactly one manifest entry per link (this.symLinks[file] = realFile), so node_modules/@t/lib/package.json has no key, and the pre-fix _resolveSymlink was exact-key only. The non-SEA prelude already did prefix matching (prelude/bootstrap.js:239-252, vfsKey.startsWith(k + sep)) — SEA was the outlier, which is exactly why standard mode worked.

The fix is correct. Built at 843326b on node v22.20.0:

build	result
committed test, with fix	exit 0
committed test, without the sea-vfs-setup.js hunk	ENOENT ... '/test-99-#295/lib/log.js', exit 1
the npm-workspace repro from #295 (node_modules/@t/lib -> ../../packages/lib, ESM bare import), SEA with fix	exit 0
same repro, SEA without fix	ERR_MODULE_NOT_FOUND at resolveBareSpecifier → vfsResolveHook, exit 1
same repro, standard (non-SEA) mode	exit 0 — confirms the report

Replaying _resolveSymlink against synthetic manifests: the #295 shape, multi-segment remainders, and chained links all resolve correctly; cycles terminate at MAX_SYMLINK_DEPTH (i is never reset); parentIdx > 0 correctly refuses the empty root key; win32 C:/… keys walk correctly and stop at C:; and there is no substring/prefix trap (/a/bc does not match /a/bcd). Nice work — the parent walk is also strictly better than bootstrap's version (O(path depth) hash lookups instead of O(number of symlinks) scans).

Top 3 risks

1. Measured ~60× slowdown of _resolveSymlink for any project that has symlinks — i.e. every project this PR fixes. See the inline comment on line 345.
2. Two divergent VFS symlink implementations (SEA vs bootstrap.js) — the root-cause class of #295 itself. Inline on line 353.
3. A latent ELOOP throw out of existsSync / internalModuleStat, which must not throw. Inline on line 365.

Findings outside the diff

• Major · Tests — test/test.js:60-87: test-99-#295 is a host-only SEA test (runSeaHostOnly, ignores the target arg) but wasn't added to the npmTests array. It therefore falls through the **/main.js glob and builds a SEA binary in both test:22 and test:24, each matrixed over 3 OSes, while never running in test:host — the exact redundancy the comment above npmTests exists to prevent. Adding 'test-99-#295' to that list fixes it. (test-93-sea-compress has the same omission — pre-existing, but worth folding in while you're there.)
• Minor · Design — prelude/sea-vfs-setup.js:286-292: the class JSDoc still advertises internalModuleStat() O(1) manifest hash lookup (no tree walk), statSync() O(1) and existsSync() O(1). All three are now O(path depth) with a directory walk whenever the manifest has symlinks. That doc block is what contributors read first, so it should either be restated or made true again by the memo above.
• Minor · Design — prelude/sea-vfs-setup.js:461-505: _resolveSymlink is now applied asymmetrically across the provider surface. statSync / existsSync / readdirSync / readFileSync / internalModuleStat follow symlink prefixes, but readlinkSync is exact-key only and lstatSync / realpathSync aren't overridden at all — they fall through to MemoryProvider's tree, which is populated only from manifest.directories. So a path that now stats fine has no corresponding realpath. Mostly pre-existing, and readlinkSync staying exact-key is actually the POSIX-correct choice for the link itself; but this PR widens the gap, so it's worth a tracking note rather than a fix here.
• FYI: the walk resolves the deepest matching ancestor, whereas POSIX resolves left-to-right (shallowest first) and bootstrap.js takes the first insertion-order match. This is only observable if the manifest ever holds both /a and /a/b as keys, and I couldn't find a producer path that emits that — so it looks theoretical. One comment line documenting the invariant would be enough.
• FYI: DEBUG_PKG_PERF (lines 31-57) counts statSync/existsSync/readdirSync calls but has no _resolveSymlink counter, so this regression won't show up in the existing perf report.
• FYI: dir naming, main.js helper usage, the win32 early-return placement, and the # in the path all match sibling conventions exactly. core.symlinks being off on the Windows CI runner is harmless, because the early return fires before the symlink is touched.

Coverage

Specialists run: Correctness, DRY & Codebase Fit, Performance, Tests, Design/API/BackCompat, plus an empirical build-and-run verifier. Not run: Security, Operability, Readability — no files in their lane (the symlink map is build-time output from the developer's own tree, not a trust boundary; no logging/error-path changes; no file over 80 changed lines).