Repository navigation
fix(module): preserve external URLs through package validation - #119
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
External URL resolution must not enter filesystem package-scope validation. Carry the resolver's external flag into that guard so protocol-relative Windows specifiers and non-ASCII external URLs preserve their results. Local file resolution still reports invalid package metadata, including immediately after an external resolution.
The change stays at the package-validation boundary and preserves the resolve-once loader from oven-sh#44473. The maintained regression covers
Bun.resolve(),Bun.resolveSync(),import.meta.resolveSync()and the external-then-local validation sequence. The original implementation and authorship are Peter Steinberger's.Reviewed head
e2a4b03bf1632e65e85eeb398c4da68f58f805admerges actual main8ae8f41cab23ad6a5643f9c0d61c4af50a993400. Only the additive compatibility notes and append-only changelog needed conflict resolution. The runtime delta remains three Rust lines. Main's resolver-origin behavior and resolve-once ownership are preserved.The exact macOS arm64 release binary passes all 12 Rust target checks, its 3 own rows, and all 57 surrounding module/resolver/plugin rows. The frozen broader selection completes 46/47 rows, retaining only the two networking reconnect assertions independently reproduced on the current main tree. No assertions, skips or deadlines changed.
P2 autoreview, formatting, source lints, JavaScript lint, Clippy, Miri and lol-html tests pass. The Rust workflow's explicitly advisory Mordant job reports the pre-existing
bare_bool_argsstyle finding insrc/resolver/package_json.rs, counted across three platforms. The complete resolver tree, baseline, configuration, script, workflow and toolchain blobs match main; no suppression or baseline was changed. Its retained report is Rust lints run 37391232646.Final exact-head CI qualification:
0da83c99db6bcdc2062771c35eb22b7c5d07ca79equals the reviewed head tree.1c5a823e37884e62579755c603081da6d1392e3f2f3d266ab6f7fa5b335d7240; ARM64be84e9455bcf1ae9f329be85acff30be42672e3fa9ee7faaa47754e216810205. Signing and publication are disabled.No runtime test exception or manual CI rerun is needed for this head. Historical Windows and OpenClaw consumer proof remains explicitly bound to its original sources below.
Original implementation and historical Windows/OpenClaw consumer evidence
On Windows,
Bun.resolveSync("//example/´?q", import.meta.dir)tries to read an UNC package configuration instead of returning the external specifier. The resolver correctly classifies the URL as external, but that flag was dropped before filesystem package-scope validation.Preserve the external flag through the resolver result and skip filesystem package validation only for external results. Tests cover the async and sync APIs, non-ASCII spellings, and a subsequent local resolution that must still reject malformed package metadata. This preserves Bun's existing external-URL API; Node's local package validation remains intact.
The existing native Windows regression fails on current main. Node 24 validates the external HTTPS/local-invalid-package control. P2 review is scoped-clean, all 12 Rust targets pass without skips on the integration, and scoped formatting passes. Upstream searches found no matching fix.
Native Windows Server 2022 x64 proof used matched source and verified binaries: the resolver runner improved from 37/42 test files passing on fork main
d2d2a26ef973cdd97b37953f5dba58052acad74fto 42/42 passing, with both dependency-install steps also successful. The passing executable was built from local integration03aa23b6adea8e12235339d8650cae41fa1ea80d(tree1c5084c96b361801cd9f595795774ec41e81e250), combining four independent fixes. This branch's production and test files are byte-identical in that integration. Executable SHA-256:cbd65560d2f8651dc82722d1d957803651e2803f0a1a66ae039b946d3b53b156.Exact-head fork CI passes both required lanes: Linux x64 (14/14 selected and compatibility test files) and macOS arm64 (10/10); both dependency-install steps also pass in each lane. Native Windows proof is recorded above because this PR workflow has no Windows test lane.
Final native Windows proof uses local integration
9a2e4b08f3b3f2e17e05a23c35aeade9c6da06f3, executable SHA-25619453c39dbb599eff57c1544a1c117dbc7bde7947ba2d41c9829cee9f20b10e0. The maintained module selection passes 2/2 files, the resolver selection 42/42, and broad compatibility 29/29; each run also passes both install steps. Source and executable guards pass before and after. Both Node 24 and the fork pass the complete 12-step installed OpenClaw path, including real update, foreground Gateway, plugins, a verified mock-provider turn, sustained readiness and Doctor. Foreground cleanup uses taskkill. No Windows build is published or signed.The full 186-file/32-envelope comparison on identical OpenClaw
deb44a7f20b774769104ab4262c8ada438d4fc8ereports all 3,184 assertions on each runtime: Node 2,896 pass / 13 fail / 275 skip (30 successful envelope exits), fork 2,891 pass / 16 fail / 277 skip (29 successful exits), with no missing reports, unhandled errors or suite failures. The three extra fork failures are shared Windows path limits reproduced with equal-length Node/fork/stock controls; the raw totals are preserved. All three shared timing failures pass isolated on Node and fork under unchanged deadlines (92 pass / 5 skip each). Runtime-specific conditional skips remain explicit. The original d2d2 runtime also passes the six-file/two-envelope OpenClaw consumer control (220 pass / 3 skip); these selected consumers provide non-regression evidence. The installed-update proof separately exercises the native-addon failure.The three affected OpenClaw fixture files are fully revalidated on Node 24 and the fork: frozen before
deb44a7f20b774769104ab4262c8ada438d4fc8egives 40 pass / 13 fail / 3 skip on each; the corrected private fixture commit96222ca4829457295a7fa3b00324b9d96b989291and exact public head343017e1202b579274fe87d51f6491ba8eeeeee4each give 53 pass / 3 existing skips on both runtimes. All 56 cases are accounted for, every one of the 13 before-failures now explicitly passes, and the skip set is unchanged. This is complete three-file before/after and public-head proof, not a rerun of the full 32-envelope matrices. The verified public proof archive SHA-256 is504d7ef2308d3c0a48d4b26135c233cdc720e8dc10c6e6061ddbe4d79e0c9f7a. The fixture correction openclaw/openclaw#165577 was merged from that exact tested public head as8bc338dbed0ddf5c35b2178eb624ffc88ccef7e7. Its inherited-CI exception is recorded on the OpenClaw PR; no CI rerun or tested-head change was used for this proof.