feat(dist): ship one binary per platform package, carry the verb in __NUB_ARGV0 - #665
Conversation
…_NUB_ARGV0 Every platform package shipped the SAME binary twice — bin/nub and bin/nubx, byte-identical, 45 MB each — solely so the Rust CLI could read its verb off argv[0]'s basename. That doubled the packages to 77-99 MiB and put six of eight over cnpm/npmmirror's 80 MiB unpacked sync cap, so the mirror has been stuck at 0.0.31 since 2026-06-10 and installs behind it fail with "no version of @nubjs/nub-darwin-x64 matches range 0.6.0". The verb now travels in `__NUB_ARGV0`. argv[0] stays the fallback and still carries every direct invocation: the installer's ~/.nub/bin/nubx symlink, `nub pm shim` hardlinks, nubx-dev. Why an env var rather than a link. Shipping the second name is not available: npm pack silently DROPS symlinks, and hardlinks survive packing but are mangled on install (npm and bun drop the file, pnpm writes a 0-byte one). Materializing it at runtime would work on POSIX but trades a static guarantee for a filesystem write that can fail. And sh cannot fake argv[0] portably — `exec -a` is a bash/zsh-ism that dash, i.e. /bin/sh on Debian and Ubuntu, rejects. Deleting the duplicate alone is NOT sufficient, and fails silently. Measured on a real install with bin/nubx removed and nothing else changed: call 1 is correct (the Node path falls back to bin/nub and sets argv0) and call 2 runs `nub` — exit 0, no error, wrong command. Both dispatch paths therefore set the variable: the healed sh trampoline as a prefix assignment, and the Node spawn via opts.env. The Node-path env var is also what makes Windows work. Node documents argv0 as affecting only the process TITLE on Windows, and Windows takes that path on every call because the heal is POSIX-only. capture_argv0_override runs unconditionally as the first statement of normalize_invocation_environment, before its early returns, and ERASES the variable. Erasing is load-bearing: nub spawns Node, a script under it may re-enter nub, and an inherited __NUB_ARGV0=nubx would silently put that grandchild in exec mode. The value is cached in a OnceLock because Argv0::detect runs more than once per process and the variable is gone by then. Because release.yml tars the GitHub archive from the platform package directory, the curl/Homebrew download halves too (49.0 MB -> ~24.5 MB), and install.sh's existing claim that "the release archive ships only bin/nub" becomes true. Verification: clippy --all-targets --all-features clean, fmt clean, argv0 unit tests pass, launcher matrix 25 ok/0 failed with a one-binary host package, and a new tests/verb-dispatch probe against the REAL binary is 11 ok/0 failed covering both PM shim shapes, both verbs, first AND second call, and env hygiene. A branch-scoped workflow runs that probe on real ubuntu and windows runners.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…file; align verb-dispatch action pins The Docker non-owner leg chmod'd the platform package's nubx, which no longer exists — the image build failed before the probe ran. The launcher package keeps both stubs (they are bin-field entries npm chmods on install); only the platform package lost its second file. verb-dispatch.yml used action pins whose version comments did not match, and omitted persist-credentials: false — both zizmor errors. Now copied verbatim from launcher.yml / ci.yml.
There was a problem hiding this comment.
Pull request overview
This PR updates Nub’s launcher + CLI verb-dispatch mechanism so each platform package ships only one native binary (bin/nub), avoiding duplicate nub/nubx copies that pushed several platform packages over cnpm/npmmirror’s unpacked size cap.
Changes:
- Switch verb selection for the one-binary platform package to use
__NUB_ARGV0(set by the launcher and erased at Nub startup), with argv[0] remaining as a fallback for direct invocations. - Update launcher/fixture tests to validate verb dispatch through both the Node-launcher path and the healed POSIX sh-trampoline path.
- Update release packaging scripts/workflows to ship/carry only
bin/nub(andnub.exeon Windows) in platform packages.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/verb-dispatch/probe.mjs |
Adds a real-binary probe validating nub/nubx dispatch with a one-binary platform package on Linux + Windows. |
tests/launcher/run-launcher-matrix.sh |
Extends the launcher matrix assertions to validate the healed trampoline shape and post-heal dispatch for nubx. |
tests/launcher/make-fixture.sh |
Updates the fake native fixture to prioritize __NUB_ARGV0 over argv[0] for mode selection. |
npm/nub/postinstall.js |
Adjusts installer chmod commentary/logic for the one-binary platform package layout. |
npm/nub/bin/launch.js |
Implements the one-binary verb transport (__NUB_ARGV0) across both healed and Node-spawn paths. |
npm/build-local.sh |
Updates local platform package build script to ship only bin/nub. |
crates/nub-cli/src/cli.rs |
Captures + erases __NUB_ARGV0 at startup and makes Argv0::detect() consult the override first. |
.github/workflows/verb-dispatch.yml |
Adds a branch-scoped CI probe job for Linux + Windows verb dispatch with a real build. |
.github/workflows/release.yml |
Updates release assembly to package only bin/nub (and nub.exe) per platform package and archive. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const child = execFileSync(path.join(hostPkg, "bin", `nub${exe}`), [script], { | ||
| encoding: "utf8", env: { ...process.env, __NUB_ARGV0: "nubx" }, | ||
| }).trim(); |
| // package past cnpm/npmmirror's 80 MiB sync cap. The verb now travels in `__NUB_ARGV0` | ||
| // (npm/nub/bin/launch.js sets it on both dispatch paths; Argv0::capture_argv0_override | ||
| // reads it and erases it). |
| # not two: the launcher passes the verb in `__NUB_ARGV0` rather than relying | ||
| # on a separately-named `nubx` file (npm/nub/bin/launch.js, | ||
| # Argv0::capture_argv0_override). This is what keeps every platform package | ||
| # under cnpm/npmmirror's 80 MiB sync cap, and it halves the release archive |
| pub unsafe fn capture_argv0_override() { | ||
| ARGV0_OVERRIDE.get_or_init(|| { | ||
| let verb = env::var("__NUB_ARGV0").ok().filter(|v| !v.is_empty()); | ||
| if verb.is_some() { | ||
| // SAFETY: upheld by this function's caller — no other thread exists yet. | ||
| unsafe { env::remove_var("__NUB_ARGV0") }; | ||
| } | ||
| verb | ||
| }); | ||
| } |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Two artifact consumers still expect bin/nubx inside the produced archive and were not updated: the Homebrew formula generator (brew install nub will fail on the next release, with no CI gate to catch it) and the winget portable manifest (the nubx command alias disappears from the Windows zip). A third, tests/launcher/Dockerfile.non-owner, should already be failing on this PR.
Reviewed changes — the full 9-file diff at 88ae5f5, plus the packaging consumers of the platform package and release archive, the __NUB_ARGV0 producer/consumer paths, and the dlx consent gate the new probe reaches.
- One binary per platform package —
release.ymlandnpm/build-local.shstop emitting the byte-identical second copy, halving each package under cnpm/npmmirror's 80 MiB cap. - Verb moves into
__NUB_ARGV0— set by the healed sh trampoline, the polyglot JS fallback, and the Node direct-spawn; read once into aOnceLockand erased byArgv0::capture_argv0_override, called first innormalize_invocation_environment. - argv[0] stays the fallback — still carries the installer's
~/.nub/bin/nubxsymlink,nub pm shimhardlinks, andnubx-dev. - Backward-compatible resolution —
resolveBinaryandpostinstall.jskeep probingbin/<verb>so a newer launcher still works against an older platform package. - Test coverage —
tests/launcher/now models a one-binary host package and asserts the post-heal second call; newtests/verb-dispatch/probe.mjsplus a branch-scoped Windows/Linux workflow.
🚨 The Homebrew formula still installs bin/nubx from an archive that no longer has it
.github/scripts/gen-homebrew-formula.sh:90 is bin.install "bin/nub", "bin/nubx". The tarball is now packed from a bin/ holding only nub, so Homebrew raises Errno::ENOENT and brew install nub / brew upgrade nub fails outright. The formula is regenerated and pushed to nubjs/homebrew-tap by the bump-homebrew-tap job on every real tag push, and no workflow in this repo runs brew install against it — the break first surfaces at end-user install time.
Technical details
# Homebrew formula installs a file the archive no longer contains
## Affected sites
- `.github/scripts/gen-homebrew-formula.sh:90` — `bin.install "bin/nub", "bin/nubx"`; `bin/nubx` is absent from `nub-{darwin,linux}-*.tar.gz` after this PR.
- `.github/scripts/gen-homebrew-formula.sh:84` — the surrounding comment still describes "bin/ (nub + nubx, both real copies)".
- `.github/workflows/release.yml:1550-1618` — `bump-homebrew-tap` regenerates and pushes the formula on every tag-push release with no install verification.
## Required outcome
- A Homebrew install of the next release puts both `nub` and `nubx` on PATH, from an archive that ships only `bin/nub`.
- The generator's comment matches what the archive actually contains.
## Suggested approach
- `bin.install "bin/nub"` followed by `bin.install_symlink bin/"nub" => "nubx"`, mirroring what `install.sh` and `perform_selfowned_upgrade` already do.
## Open questions for the human
- Worth adding a `brew install --formula` smoke to `bump-homebrew-tap`, or at least a grep-level check that every path the formula installs exists in the packed archive? This class of break is invisible to CI today.⚠️ The winget portable install loses the nubx alias
winget/manifests/n/Nubjs/Nub/0.2.5/Nubjs.Nub.installer.yaml:12 declares bin\nubx.exe as a NestedInstallerFiles entry with PortableCommandAlias: nubx. The win32 zip is packed from the same $PKG/bin the assemble step no longer copies nubx.exe into, so from the next release that file does not exist. Windows users installing via winget get nub but no nubx, and if winget-releaser/komac carries the previous manifest's file list forward rather than re-deriving it, winget install Nubjs.Nub fails validation instead of merely losing the alias. winget-validate.yml will not catch either case — the committed manifest pins the already-published v0.2.5 zip, which does still contain both binaries.
Technical details
# winget zip no longer contains the `nubx.exe` it declares
## Affected sites
- `winget/manifests/n/Nubjs/Nub/0.2.5/Nubjs.Nub.installer.yaml:12-13` — `RelativeFilePath: bin\nubx.exe` / `PortableCommandAlias: nubx`.
- `.github/workflows/release.yml` (assemble step, ~line 715-733) — the win32 branch no longer copies `nub.exe` to `nubx.exe`.
- `.github/workflows/release.yml:1619-1670` — `submit-winget` auto-generates the next manifest via `vedantmgoyal9/winget-releaser`.
- `.github/workflows/winget-validate.yml` — validates the committed v0.2.5 manifest against the v0.2.5 release, so it is blind to this.
## Required outcome
- Either the win32 zip keeps a `bin\nubx.exe` (a copy — the cnpm cap this PR targets applies to npm packages, not the GitHub release zip), or the `nubx` portable alias is intentionally dropped and the manifest reflects that.
- Whichever way it goes, confirm what `winget-releaser`/komac does with `NestedInstallerFiles` on an update: re-derive from the new zip, or carry the old list forward.
## Open questions for the human
- Is losing `nubx` for winget users acceptable? `install.ps1` sidesteps this by copying `nub.exe` → `nubx.exe` at install time, but a winget portable install has no such hook.ℹ️ Nitpicks
tests/launcher/README.md:34still documents the old assertion ("asserts the healednubxnamesbin/nubx"), which the diff inverted.tests/launcher/run-launcher-matrix.sh:12— the header's scenario table still readsnubx-verb nubx keeps its verb through the heal (bin/nubx, "nubx-mode").npm/nub/bin/launch.js:108-112— the staged-copy rationale claims "the healed sh trampolineexecs this path with no argv0 override", which this PR makes false. The naming constraint it justifies is now belt-and-suspenders rather than load-bearing..github/workflows/verb-dispatch.yml:71-84— the step comment says it will "assert the packagedbin/actually halved and would clear cnpm/npmmirror's 83,886,080-byte unpacked cap", but the step onlyechos the three numbers. Either compare against the cap and exit non-zero, or reword the comment to say it reports.
Claude Opus ｜ 𝕏
| if (argv0Name) { | ||
| opts.argv0 = argv0Name; | ||
| opts.env = Object.assign({}, process.env, { __NUB_ARGV0: argv0Name }); | ||
| } |
There was a problem hiding this comment.
bin/nub calls launch() with no argument, so argv0Name is undefined and this block never fires for the nub verb — spawn then inherits process.env verbatim, letting a pre-existing __NUB_ARGV0=nubx silently win. The other two producers (the sh trampoline at :181 and the polyglot fallback at :185) assert the verb unconditionally for every verb; only this path doesn't, and it's the one Windows takes on every call.
Technical details
# The `nub` verb is the only dispatch path that doesn't pin `__NUB_ARGV0`
## Affected sites
- `npm/nub/bin/launch.js:240-243` — `opts.env` set only when `argv0Name` is truthy.
- `npm/nub/bin/nub:8` — `require("./launch.js")()`, no argument, so `argv0Name` is `undefined`.
- `crates/nub-cli/src/cli.rs:616-618` — the override wins over argv[0] unconditionally, so an inherited value is decisive.
## Required outcome
- Every dispatch path asserts the verb it means, so the invariant the header comment states ("the variable survives exactly one process") actually holds on all three producers.
## Suggested approach
- Set `opts.env` from `verb` (which is already `argv0Name || "nub"`) unconditionally; keep `opts.argv0` gated so the `nub` path's argv[0] is unchanged.
- Optional complement on the Rust side: `Argv0::classify` currently accepts `node` and every PM shim name from the override, though the launcher only ever emits `nub`/`nubx`. Narrowing the override to those two values would keep the internal channel as small as its producer.| if (argv0Name) { | |
| opts.argv0 = argv0Name; | |
| opts.env = Object.assign({}, process.env, { __NUB_ARGV0: argv0Name }); | |
| } | |
| opts.env = Object.assign({}, process.env, { __NUB_ARGV0: verb }); | |
| if (argv0Name) opts.argv0 = argv0Name; |
| push: | ||
| branches: [dedup-platform-binary] |
There was a problem hiding this comment.
Scoped to dedup-platform-binary, this workflow stops firing the moment the branch merges. It is the only automated Windows coverage of the verb plumbing — launcher.yml excludes Windows for the reason this PR's own header calls "now stale" — so on merge that coverage drops to zero on the platform where every call takes the Node launcher path. Worth deciding whether this graduates to a path-filtered pull_request/push: main trigger or gets deleted as a landed ad-hoc probe.
There was a problem hiding this comment.
ℹ️ Nothing new in the follow-up commit; the items from the previous review are still open.
Reviewed changes — only e1db603, the delta since the previous pullfrog review at 88ae5f5.
Dockerfile.non-ownerchmod fixed — line 21 now modes onlynub-host/bin/nub. Lines 19-20 correctly keep@nubjs/nub/bin/nubx(the launcher package's committed stub, still shipped) and the2>/dev/null || true-guarded/opt/nub/bin/nubx. This clears thelauncher.ymlnon-ownerjob break flagged last round.verb-dispatch.ymlaction pins aligned —checkout@93cb6ef(v5.0.1) withpersist-credentials: false,setup-node@a0853c2(v5),rust-cache@e18b497. All three match the SHAs every other workflow in.github/workflows/uses.
Still open from the previous review, unchanged by this commit: the Homebrew formula's bin.install "bin/nub", "bin/nubx", the winget manifest's bin\nubx.exe, the nub-verb gap in launch.js's direct-spawn opts, the env-hygiene check in probe.mjs, and the branch-scoped trigger on verb-dispatch.yml.
Claude Opus ｜ 𝕏
…tatus execFileSync throws on a non-zero exit, and `nub <script>` in a bare temp dir with no dependencies warns and exits non-zero on Windows (macOS only warns). That failed the Windows leg even though both dispatch assertions on it passed: ok: node bin/nub -> nub mode ok: node bin/nubx -> nubx mode The assertion is what the CHILD SAW in its environment, not nub's exit status, so use spawnSync and read stdout+stderr either way. Also distinguishes the three outcomes — erased, leaked, and never-reported — so a future failure says which.
There was a problem hiding this comment.
Important
The env-hygiene check now reports instead of throwing, but it still cannot observe the child on Windows — the nubx subject never resolves there, so the call lands in the CI dlx refusal and the check reports env-hygiene child never reported. The Windows leg of verb-dispatch.yml goes red every run, and the erasure invariant stays untested on the one platform where every call carries __NUB_ARGV0.
Reviewed changes — only 2a865ff, the delta since the prior pullfrog review at e1db603.
- Env-hygiene check no longer aborts the probe —
execFileSync→spawnSyncwith stdout and stderr merged, so a non-zero exit from nub no longer throws pastRESULT:and thefs.rmSynccleanup. A miss is now a countedFAILcarrying the exit status, and a genuine leak is distinguished from a child that never reported.
One correction to the previous review, since it informed this commit: on Linux the check does reach its assertion. find_bin's unix candidate list is [""], and Path::join lets an absolute component replace the base, so the absolute script path resolves to itself and runs — verified against the published 0.6.0 binary in CI (CHILD:undefined, exit 0). Windows is where it breaks, for a different reason.
Claude Opus ｜ 𝕏
…c mode Setting __NUB_ARGV0=nubx on the parent made it dispatch as nubx, which treats the script path as a tool to fetch and refuses under CI: nubx: refusing to download ...\probe-env.js in CI Nothing to do with env hygiene. The property under test is that the variable is REMOVED at startup, which capture_argv0_override does for any non-empty value, so the value is irrelevant — use 'nub'. Passed on macOS only because CI was unset there. Documented CI=true as the local reproduction so the next probe change is checked against the runner's conditions rather than a bare shell.
There was a problem hiding this comment.
ℹ️ The Windows-leg breakage is genuinely fixed; one branch was left behind by the value swap.
Reviewed changes — only acef1cb, the delta since the prior pullfrog review at 2a865ff. It touches tests/verb-dispatch/probe.mjs and nothing else.
- Env-hygiene check no longer dispatches in exec mode —
__NUB_ARGV0for that spawn is now"nub"rather than"nubx", so the parent stays in file-run mode. I traced the chain to confirm the check can now reach its assertion on both legs:classify("nub")→Argv0::Nub→run_nub()→ the flag scan's finalrun_file_with_compatarm (cli.rs:2038) →run_file_in_dir, withnubx_consent's CI refusal out of the picture entirely. Nothing else aborts first —verify_deps::resolve_policylands on the hardcodedPolicy::Warndefault (project_config.rs:213), anddiscover_node's PATH fast path satisfies the rootengines.node >=22.15.0fromsetup-node's Node 22, so no network access is involved. - Rationale recorded at the call site and in the header — the block now explains why the value is irrelevant to the assertion, and the header documents
CI=trueas the local reproduction of the runner's conditions.
ℹ️ Nitpicks
tests/verb-dispatch/probe.mjs:153-156— this paragraph now contradicts the one directly below it. It attributes theexecFileSyncWindows failure to a dependency warning; lines 160-163 correctly identify the dlx refusal as the cause. Its "a bare temp dir has no dependencies" premise is also wrong: the spawn passes nocwd, so the child inherits the probe's cwd — the checkout root underverb-dispatch.yml, not the fixture's temp dir.
Claude Opus ｜ 𝕏
| }); | ||
| const seen = `${res.stdout ?? ""}${res.stderr ?? ""}`; | ||
| if (seen.includes("CHILD:undefined")) ok("__NUB_ARGV0 erased before any child sees it"); | ||
| else if (seen.includes("CHILD:nubx")) no("__NUB_ARGV0 LEAKED to child — erasure is not working"); |
There was a problem hiding this comment.
The override value is now nub, so a genuine leak prints CHILD:nub and this branch can never match. The leak instead falls through to branch 3 and reports env-hygiene child never reported (exit=0): CHILD:nub — still a counted FAIL, so no false pass, but the message contradicts the output it prints alongside it.
| else if (seen.includes("CHILD:nubx")) no("__NUB_ARGV0 LEAKED to child — erasure is not working"); | |
| else if (seen.includes("CHILD:nub")) no("__NUB_ARGV0 LEAKED to child — erasure is not working"); |
…VM timing run Provisioning the VS C++ workload, cmake and an -msvc rustup host on a VM costs far more than one CI dispatch, and a windows-gnu cross-compile is not the same artifact. Release rather than fast: the timing question is about the shipped binary, and fast carries debuginfo at roughly 3x the size, which moves image-load time.
There was a problem hiding this comment.
ℹ️ No critical issues in the new commit — one substantive note on the artifact job, inline.
Reviewed changes — only fb69d56, the delta since the prior pullfrog review at acef1cb. It touches .github/workflows/verb-dispatch.yml and nothing else.
- New
release-artifactjob — awindows-latestjob that runscargo build -p nub-cli --release, stagesout/platform/bin/nub.exeplus the launcher package copied fromnpm/nub/., and uploads both aswin-release-verbdispatchwith 5-day retention, to feed a manual Windows-VM timing run. - Cross-reference from the size step — the Packaged-size comment now points at that job as the source of the binary the VM run uses.
- Action pins —
checkout@93cb6ef,rust-cache@e18b497, andupload-artifact@ea165f8dmatch the SHAs the rest of.github/workflows/uses; the upload pin is identical across all seven other call sites in the repo.
ℹ️ Nitpicks
.github/workflows/verb-dispatch.yml:109-111—out/platformcarries nopackage.json, so the staged launcher inout/launchercannotrequire.resolve("@nubjs/nub-win32-x64/bin/nub.exe")against it; using the artifact means hand-assembling anode_modulestree on the VM. Writing a two-fieldpackage.jsonin the staging step would make the artifact self-contained..github/workflows/verb-dispatch.yml:94-105— the job fires on every branch push matching thepathsfilter, and[profile.release]islto = "thin"withcodegen-units = 1, so this is now the workflow's most expensive job while asserting nothing. Anif: github.event_name == 'workflow_dispatch'gate would keep a one-off artifact on demand, and folds it into the same delete-or-graduate decision as the branch-scoped trigger.
Claude Opus ｜ 𝕏
| with: | ||
| key: verb-dispatch-release-windows | ||
| - name: Build release | ||
| run: cargo build -p nub-cli --release |
There was a problem hiding this comment.
This is not the shipped binary: release.yml:664-666 builds every platform binary with --features embed-runtime, so the published nub.exe carries the zstd runtime blob (and a populated primer) that a plain --release build omits — it is materially smaller, which is exactly the quantity the job says it exists to time. On the VM it also resolves no runtime at all: the feature-off find_preload arm walks up from the exe for a runtime/ sidecar (not staged here) and then falls back to the compile-time CARGO_MANIFEST_DIR path, i.e. this runner's checkout, so nub runs un-augmented and never pays the first-run extract-and-verify the shipped binary pays.
Technical details
# The staged artifact omits `embed-runtime`, so it is not the binary being timed
## Affected sites
- `.github/workflows/verb-dispatch.yml:105` — `cargo build -p nub-cli --release`, no `--features embed-runtime`.
- `.github/workflows/release.yml:664-666` — every shipped platform binary is `cargo build --release -p nub-cli --features embed-runtime` (`cross build` for the cross targets), after staging the addon and the vendored `runtime/node_modules`.
- `crates/nub-core/src/node/spawn.rs` `find_preload` — the `#[cfg(not(feature = "embed-runtime"))]` arm walks up from the binary's dir for `runtime/preload.mjs`, then falls back to `env!("CARGO_MANIFEST_DIR")/../../runtime/preload.mjs`. Neither path exists on the VM, so augmentation is silently off.
- `crates/nub-core/src/node/runtime_cache.rs` — the first-run zstd inflate + BLAKE3 verify that the shipped binary performs is compiled only under the feature, so it is absent from this artifact.
- No committed `crates/aube-resolver/data/primer-*.rkyv.*` in the tree, so this build also embeds an empty primer where the release build embeds the downloaded blob.
## Required outcome
- The uploaded `nub.exe` matches the shipped artifact closely enough for a load-time measurement to mean something: embedded runtime blob present, and the runtime resolvable on a machine that is not the build runner.
## Suggested approach
- Mirror `ci.yml`'s `windows-embed-roundtrip` job, which already does this on `windows-latest`: build and stage the real addon (`cd crates/nub-native && cargo build --release`, then copy `target/release/nub_native.dll` to `runtime/addons/nub-native.node`), then `cargo build -p nub-cli --release --features embed-runtime`.
- If a fully faithful artifact is not worth the extra steps, say so in the job comment instead — the current comment argues the opposite, that the profile was chosen because the question is about the shipped binary.…ilds Closes the two gaps the dispatch probe left on Windows. The probe hand-assembles a node_modules tree and calls `node bin/nubx` directly, so npm's own .cmd/.ps1 shim generation and PATH resolution were never exercised — production Windows is cmd.exe -> nub.cmd -> node bin/nub -> spawn nub.exe and only the last two hops were covered. And nothing measured speed. win-e2e.mjs does a real `npm install -g`, dispatches both verbs through npm's own generated shims on PATH, then A/Bs the launcher by holding the BINARY FIXED and swapping only launch.js — two builds would confound the delta with codegen noise. Two harness faults found and fixed while bringing it up, both mine: - `npm install -g <dir>` SYMLINKS the directory, so the launcher's realpath became the source tree and resolution never saw the platform package. Pack to a tarball first, which is what a registry install does and also honours `files`. - a declared `file:` optionalDependency is not installed into a global prefix and its failed attempt shadows a manual placement. Local (macOS, release-equivalent): 5 ok / 0 failed, delta -2.3 ms. Runs on windows-latest and ubuntu-latest so the two are comparable.
There was a problem hiding this comment.
ℹ️ No critical issues in the new commit — but the new A/B harness measures less than it claims. Details inline.
Reviewed changes — only b2df038, the delta since the prior pullfrog review at fb69d56. It touches .github/workflows/verb-dispatch.yml and adds tests/verb-dispatch/win-e2e.mjs.
release-artifactbecamerelease-e2e, matrixed over two OSes —windows-latest+ubuntu-latest, withsetup-node@22,fetch-depth: 0, and a step that reads the pre-changelaunch.jsout oforigin/maininto/tmp/launch-old.jsas the A/B baseline. Artifact renamedrelease-verbdispatch-${{ matrix.os }}; the staging step branches on OS for the binary name.- New
win-e2e.mjsharness —npm packthe launcher,npm install -gthe tarball into a temp prefix, hand-place a one-binary platform package as a sibling in the globalnode_modules, then dispatch both verbs through npm's own generated shims withshell: true. This is the productioncmd.exe → nub.cmd → node bin/nub → nub.exechain thatprobe.mjscould not reach, and the comments explaining why a packed tarball is used instead ofnpm install -g <dir>(which symlinks, breaking module resolution) and whyoptionalDependenciesmust be deleted are both load-bearing and worth having recorded. - A/B timing on top of that install — 40 samples plus 5 warmups per arm per verb, median + IQR, with a
max(5 ms, 10%)budget on thenubdelta.
The --features embed-runtime note on the build step is unchanged by this commit and its thread is still open; the timing numbers now depend on it, since they are taken off that binary.
ℹ️ Nitpicks
tests/verb-dispatch/win-e2e.mjs:145-148—launchJsAltis a dead intermediate:installedLaunchis always equal to it, so three names collapse to onefs.existsSynccheck.tests/verb-dispatch/win-e2e.mjs:84-96— a failednpm packrecords aFAILand falls through, so the next step runsnpm install -g <root>(tgz ?? "") and reports a second, unrelated failure on top. An early exit after the pack failure would keep the output readable..github/workflows/verb-dispatch.yml:121-126— the baseline isorigin/main:npm/nub/bin/launch.js, a moving ref. If this workflow is ever graduated past the merge, the A/B silently becomes new-vs-new and reports a0.0 msdelta forever. Pinning to the merge-base commit keeps it honest.- The
win-prefix no longer describeswin-e2e.mjsnow that it runs on ubuntu too.
Claude Opus ｜ 𝕏
| const samples = []; | ||
| for (let i = 0; i < N + WARMUP; i++) { | ||
| const t = process.hrtime.bigint(); | ||
| spawnSync(`${verb} --version`, { shell: true, encoding: "utf8", env, cwd: root }); |
There was a problem hiding this comment.
On the ubuntu leg this times the healed sh trampoline, not launch.js, so both arms are the identical path and the delta assertion below cannot fail. The dispatch section above already made the first call for each verb, which is what heals prefix/bin/nub and prefix/bin/nubx; from then on POSIX never reads launch.js again. timeIt also ignores spawnSync's status, so an arm that fails instantly times fast and still passes.
Technical details
# The ubuntu A/B arm swaps a file that is no longer in the code path
## Affected sites
- `tests/verb-dispatch/win-e2e.mjs:125-141` — the dispatch loop calls `nub` and `nubx` once each through `prefix/bin`, which triggers `healPathEntry`.
- `npm/nub/bin/launch.js:187-204` — the heal scans `PATH` for an entry named `<verb>`; `leadsToUs` matches because `npm install -g` makes `<prefix>/bin/<verb>` a symlink whose realpath is the installed launcher. The entry is replaced with an sh trampoline that `exec`s the native directly.
- `tests/verb-dispatch/win-e2e.mjs:167-173` — `installedLaunch` is rewritten per arm, but nothing reads it after the heal.
- `tests/verb-dispatch/win-e2e.mjs:152-161` — `timeIt` discards the `spawnSync` result, so exit status is never checked.
- `tests/verb-dispatch/win-e2e.mjs:14-15` — the header states "the only variable is the launcher change", which holds on Windows (the heal is a win32 no-op) but not on ubuntu.
## Measured
On this runner, with a fake native standing in for `nub.exe`: after `npm pack` + `npm install -g --prefix`, `prefix/bin/nub -> ../lib/node_modules/@nubjs/nub/bin/nub`. One call per verb turns both entries into `#!/bin/sh` trampolines carrying `__NUB_ARGV0='<verb>'`. Overwriting the installed `launch.js` with `throw new Error(...)` afterwards still dispatches `nub` and `nubx` correctly — proof the file is out of the path.
## Required outcome
- Either the ubuntu leg's A/B compares arms that actually differ, or it does not report `ok: launcher change costs nothing measurable` on a comparison it cannot fail.
- A timing arm that failed to run is not reported as a passing measurement.
## Suggested approach
- Re-heal per arm: after writing the arm's `launch.js`, restore `prefix/bin/<verb>` to the npm symlink (or point `PATH` at a fresh per-arm bin dir) so the timed call goes through Node again.
- Or scope the timing to `isWin` and have the ubuntu leg run only the install + dispatch assertions, which are the parts that carry information there.
- Assert `status === 0` inside `timeIt` and fail the arm otherwise.| // The old launcher cannot dispatch nubx off a one-binary package, so old/nubx is a | ||
| // WRONG-ANSWER timing, not a comparable arm — nub is the honest comparison. | ||
| const dNub = results["new/nub"].med - results["old/nub"].med; | ||
| console.log(` delta (nub, new - old): ${dNub >= 0 ? "+" : ""}${dNub.toFixed(1)} ms`); | ||
| const budget = Math.max(5, results["old/nub"].med * 0.10); | ||
| Math.abs(dNub) <= budget | ||
| ? ok(`launcher change costs nothing measurable on nub (|${dNub.toFixed(1)}| <= ${budget.toFixed(1)} ms)`) | ||
| : no(`launcher change moved nub by ${dNub.toFixed(1)} ms (budget ${budget.toFixed(1)})`); |
There was a problem hiding this comment.
The header names the cost to measure as nubx spawning bin/nub.exe instead of bin/nubx.exe, but the only compared arm is nub — the verb whose path barely changed. Excluding old/nubx is right as written, yet a valid baseline is available: give the old arm a two-binary platform package (nub.exe copied to nubx.exe), which is exactly the pre-change shipping layout, and the comparison covers the verb the change actually affects.
Technical details
# The A/B compares the one verb the change does not affect
## Affected sites
- `tests/verb-dispatch/win-e2e.mjs:11-12` — states the quantity under test: `nubx` now spawns `bin/nub.exe` rather than `bin/nubx.exe`, plus an env-var assignment.
- `tests/verb-dispatch/win-e2e.mjs:46-48` — the platform package is staged with one binary for BOTH arms, which is what makes `old/nubx` a wrong-answer arm.
- `tests/verb-dispatch/win-e2e.mjs:177-184` — only `new/nub` vs `old/nub` feeds the budget assertion; `old/nubx` is timed (45 spawns) and discarded.
## Required outcome
- The reported delta covers `nubx`, or the harness states plainly that `nubx` is unmeasured rather than timing an arm it then throws away.
## Suggested approach
- Stage the old arm against a two-binary platform package: copy `nub.exe` to `nubx.exe` in the placed package before the `old` arm and remove it afterwards. That is the pre-change layout, so `old/nubx` becomes a correct-answer baseline and `new/nubx - old/nubx` is the number the header promises.
- If that is not worth the extra staging, drop the `old/nubx` timing loop and say `nubx` is covered for correctness only.Windows pays ~94 ms per call, of which ~58 ms is a Node boot. Measured baselines (run 30680335776): cmd /c rem 10.22, native no-op exe 6.11, nub.exe --version by absolute path 19.19, node -e "" 57.98. That Node hop is not inherent to Windows — it is a consequence of our bin target starting with '#!/usr/bin/env node'. npm's cmd-shim branches on exactly that (cmd-shim/lib/index.js:42) and for a SHEBANG-LESS target emits a direct invocation in ALL THREE shims (.cmd, .ps1, extensionless sh) with no node and nothing to delete. It is also why pnpm ships 'This file intentionally left blank' and bun names its bins bin/bun.exe on every platform. This probe measures the ceiling of each mechanism before any is built: A today's shape (node-shebang bin) B shebang-less bin target C B plus a real nub.exe in the bin dir (PATHEXT) D the binary by absolute path — the floor B deliberately uses the real binary as the bin target. That is NOT shippable (a cross-platform package cannot carry a 45 MB Windows exe); it measures whether the mechanism is worth building a tiny native launcher for. Local (macOS): A 77.6, B 13.9, C 12.6, D 8.5 ms — B is 5.6x. POSIX already gets this via the heal; Windows, where the heal is a no-op, is the real question.
There was a problem hiding this comment.
ℹ️ No critical issues in the new commit — but the new harness prints a speedup for a variant that never ran. Details inline.
Reviewed changes — only 0d4119f, the delta since the prior pullfrog review at b2df038. Pure additions: one workflow step and one new file; no existing line was modified.
- New
Launcher latency variantsstep inrelease-e2e— runs the new harness againsttarget/release/nub{,.exe}on both matrix legs, after thewin-e2estep and before staging the artifact. - New
tests/verb-dispatch/win-latency-variants.mjs— four variants (A today's node-shebangbintarget, B a shebang-lessbintarget, C = B plus a realnub.exein the bin dir so PATHEXT resolves.exeahead of.cmd, D the binary by absolute path as the floor), eachnpm packed andnpm install -g'd into its own prefix, then 40 timednub --versioncalls through whatever npm generated, resolved offPATH. Reports median + IQR plus a speedup-vs-A summary. - Rationale recorded in the header — the measured
~94 msWindows breakdown, thecmd-shim/lib/index.js:42shebang branch the hypothesis rests on, and an explicit note that variant B's use of the real 45 MB binary as abintarget is a ceiling measurement rather than a shippable shape. All three are worth having in the file.
This harness asserts nothing and always exits 0, which is defensible for a measurement probe — the caveat is below. It also inherits the still-open --features embed-runtime thread on the build step, since these numbers come off that same binary.
ℹ️ Nothing runs this harness once the branch merges
win-latency-variants.mjs measures a hypothesis for a future change — a shebang-less bin target fronted by a tiny native launcher — and verifies nothing about the one-binary change this PR makes. Its only caller is verb-dispatch.yml, which is scoped to push: branches: [dedup-platform-binary], so on merge the repo gains a third committed harness under tests/verb-dispatch/ that nothing invokes. Whether that is deliberate staging for the follow-up or should move out of this PR is a call only you can make.
Technical details
# Exploratory harness lands in a PR whose workflow dies on merge
## Affected sites
- `tests/verb-dispatch/win-latency-variants.mjs` — committed, 145 lines, no caller outside `verb-dispatch.yml`.
- `.github/workflows/verb-dispatch.yml:23-24` — `push: branches: [dedup-platform-binary]`, so the workflow stops firing after the merge.
- `.github/workflows/verb-dispatch.yml:139-144` — the only invocation.
## Required outcome
- Each of the three harnesses under `tests/verb-dispatch/` has a stated fate: graduated onto a trigger that survives the merge, or deleted with the branch, or explicitly kept as an on-demand `workflow_dispatch` probe.
## Open questions for the human
- Is the latency investigation a follow-up PR? If so, is there a reason to land its harness here rather than there — and would that follow-up want the harness driven from a `workflow_dispatch`-gated job rather than a branch push?
- `probe.mjs` and `win-e2e.mjs` assert real invariants about this change and would keep paying rent on a surviving trigger; `win-latency-variants.mjs` would not. Worth splitting their fates.ℹ️ Nitpicks
tests/verb-dispatch/win-latency-variants.mjs:111-119— off Windows,extraBinDirFilereturns immediately, soC-exe-on-PATHis a byte-identical rerun of B. Measured on this runner:B 2.0 ms/C 2.0 ms, and the summary still prints both as separate mechanisms with separate speedups. Skipping the row when!isWin(or labelling it= B) keeps the ubuntu output honest.tests/verb-dispatch/win-latency-variants.mjs:37-46—median,iqr, and the two-candidatenpm-cli.jsprobe are copy-pasted fromwin-e2e.mjs:69-72,150-151. Two throwaway harnesses in one directory is where a tiny shared_util.mjsstarts paying off.- The
win-prefix names a harness the workflow runs onubuntu-latesttoo — same observation aswin-e2e.mjslast round, now in a second filename.
Claude Opus ｜ 𝕏
| const a = rows.find((r) => r.label.startsWith("A"))?.med; | ||
| for (const r of rows.slice(1)) { | ||
| if (r.med && a) console.log(` ${r.label} vs today: ${(a / r.med).toFixed(1)}x faster (-${(a - r.med).toFixed(1)} ms)`); | ||
| } |
There was a problem hiding this comment.
Measured on this runner, not inferred: pointed at a binary that exits 3 on every call, the harness prints works=false on A/B/C and still emits B-shebangless-bin vs today: 14.6x faster (-26.5 ms), exit 0. works is computed and printed but never consulted here, and D-direct-exec (line 133) hardcodes works: true while discarding its own spawnSync result — it reported works=true against that same exit-3 binary. Since the header makes these numbers a go/no-go threshold for building a native launcher, a variant that fails fast is the failure mode that most closely resembles a win.
Technical details
# The speedup summary does not consult whether the variant ran
## Affected sites
- `tests/verb-dispatch/win-latency-variants.mjs:141-144` — the `vs today` loop gates only on `r.med && a`, never on `r.works`.
- `tests/verb-dispatch/win-latency-variants.mjs:133` — the `D-direct-exec` row sets `works: true` unconditionally; line 129's `spawnSync(direct, ["--version"])` return value is discarded, so nothing checks that the floor measurement executed anything.
- `tests/verb-dispatch/win-latency-variants.mjs:79-84` — `works` is derived from iteration 0 only (a warmup), which is adequate, but it is the sole signal and it is advisory.
- `tests/verb-dispatch/win-latency-variants.mjs:19-20` — the header states the decision these numbers drive: "If B lands near 29 ms the approach is worth building a tiny native launcher for; if it lands near 90 ms the idea is dead and we stop."
## Measured
`VERB_TIMING_N=3 node tests/verb-dispatch/win-latency-variants.mjs <sh script whose only statement is `exit 3`>` on ubuntu:
A-node-shebang 28.4 ms works=false
B-shebangless-bin 1.9 ms works=false
C-exe-on-PATH 2.0 ms works=false
D-direct-exec 1.5 ms works=true
B-shebangless-bin vs today: 14.6x faster (-26.5 ms)
Exit status 0.
## Required outcome
- A variant whose invocation did not succeed does not contribute a `vs today` line, or contributes one that is unmistakably marked as invalid.
- `D-direct-exec` establishes `works` from what it actually observed rather than asserting it.
## Suggested approach
- Capture `r.status` / `r.stdout` in D exactly as `variant()` does, and set `works` from the same semver test.
- Skip (or prefix with a loud marker) any row in the summary loop where `works !== true`, and consider `process.exit(1)` when the A row itself did not run, since every comparison is relative to it.
|
Shipped in v0.7.0: https://github.com/nubjs/nub/releases/tag/v0.7.0 |
Platform packages shipped
bin/nubandbin/nubxbyte-identical at 45 MB so the CLI could read its verb off argv[0]. Six of eight exceeded cnpm/npmmirror's 80 MiB cap, so that mirror is pinned at 0.0.31 and installs behind it fail. Now 90.3 → ~45 MiB; release archive 49.0 → ~24.5 MB.The second name cannot ship:
npm packdrops symlinks, hardlinks are mangled on install,exec -ais a bashism dash rejects.Deleting the duplicate alone fails silently — call 2 runs
nub, exit 0. So the trampoline and the Node spawn both set the var; the latter is what makes Windows work. argv[0] stays the fallback; the var is erased at startup.Verified on ubuntu + windows.