feat: convert preview asset bundles in-process instead of via a sidecar - #1508
Open
eordano wants to merge 6 commits into
Open
feat: convert preview asset bundles in-process instead of via a sidecar#1508eordano wants to merge 6 commits into
eordano wants to merge 6 commits into
Conversation
Deploying js-sdk-toolchain with
|
| Latest commit: |
1f54f73
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d156070a.js-sdk-toolchain.pages.dev |
| Branch Preview URL: | https://feat-abgen-in-process.js-sdk-toolchain.pages.dev |
Contributor
Test this pull request
|
Rebased onto main (0f012e0, "replace legacy web explorer preview with Bevy Web" #1502). The branch's 38 commits are collapsed into one: they included an entire abgen *sidecar* implementation that later commits deleted and replaced with the in-process addon, so replaying them would have meant resolving conflicts in code that does not survive. sdk-commands start now converts through @dcl/abgen-node — a function call, not a spawned process — and serves the bundles from the preview server itself, so there is one origin and no port to coordinate. Conversion is keyed on a sha256 of the publishable files. Identical content means the same key, so a request for work already in flight joins it rather than starting a second, and a run publishes only if its key is still the one most recently asked for. Results publish by atomic rename out of a private staging directory, with an advisory lock and an age-gated sweep, so two previews of one project cannot interleave bundle files into a directory a reader is about to serve. Two conflicts with #1502, both resolved in main's favour: - wireFileWatcherToWebSockets: main now sends updateScene unconditionally, because the legacy JSON protocol is the only one Bevy and Godot understand. This branch had gated it behind a desktopClient flag, which would have regressed Bevy Web, so the flag is dropped and only the onProjectChanged invalidation is layered on. - its 'unlink' handler: main's (file) signature is correct; this branch read (_, file), which left file undefined, since chokidar emits the path as the first argument. 52/52 green across the four start-command suites.
eordano
force-pushed
the
feat/abgen-in-process
branch
from
August 3, 2026 01:58
b66d29c to
1255cff
Compare
optionalDependencies made the whole module optional, which was never the distinction that mattered: @dcl/abgen-node declares no os/cpu constraint and its five per-platform binaries are optional inside it, so the JavaScript always installs and only the native addon is gated. Marking the wrapper optional therefore bought nothing and let an install silently skip it for unrelated reasons — a preview quietly serving raw GLTFs with no cause anyone could name. Depending on it outright makes the module's presence a fact and leaves the one genuinely platform-dependent piece — the .node binary — as the only thing that can be missing. loadAbgen still catches that and degrades to raw GLTFs, and now says which platform had no prebuild rather than telling the user to install a package that is already there.
eordano
marked this pull request as ready for review
August 3, 2026 10:06
exec()'s `shell` option existed so the abgen binary could be spawned directly and killed without a wrapping shell. Nothing spawns a binary here any more, and nothing in the tree passed it — the only caller was the test exercising the option, on a path named after the binary itself. The other two changes to exec() stay, because neither is about the sidecar: draining the pipes when silent is true fixes a deadlock for any chatty child (explorer-alpha spawns two), and removing the signal listeners on close fixes a leak. The spawn-error test stays too; it only passed shell:false incidentally. Also drops three comments still describing an asset-bundle sidecar that no longer exists.
0.15.2 restores the case-insensitive collapse of oversized bundle names, which was reverted from abgen before 0.15.0 and so is broken in the version this branch pinned: a Windows bundle name long enough to collapse was written under one digest and looked up under another. Preview is exactly where that bites, since the local scene is converted and served by the same process. Also carries the refusal to write filename components that cannot fit in NAME_MAX, which now fails with the offending path instead of the OS's ENAMETOOLONG from somewhere inside a conversion.
0.15.3 made setMaxThreads reachable: it was exported from the addon but missing from the generated wrapper, so every earlier binary carried the symbol while the package did not expose it. Nothing here calls it yet, but a floor below 0.15.3 promises an API that is not there. 0.15.4 also makes the Windows library self-contained — it imports no MinGW runtime, so there is nothing to place beside the addon on that platform.
Regenerated packages/@dcl/sdk-commands/package-lock.json, which is the lock that governs this dependency - the root lock covers only root dependencies and has never carried it. Every changed line belongs to abgen: the addon and its five platform packages move 0.15.2 -> 0.15.4, with new integrity hashes and a license field that now reads Apache-2.0 rather than AGPL-3.0-or-later.
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
1 participant
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.
Preview converts asset bundles by calling
@dcl/abgen-node, not by running asidecar. No spawned process, no port to allocate, no binary to resolve or
download, no proxy hop — and
abgen-binary.ts, 160 lines ofresolve-cache-download, is deleted, because getting a binary onto the box is
what an npm dependency does.
getPort(0)ABGEN_BIN/ cached release / PATH / download/optimized-assetsThe previewed scene converts in-process and is answered from memory. Every
other entity — wearables, emotes, anything the preview server does not host —
reads through to the production ab-cdn, which already has prebuilt bundles.
That mirrors the sidecar, and for the same reason: the preview server serves
the local scene's files, so converting remote entities locally could only 404.
Reconversion is content-keyed
Editing a scene has to rebuild its bundles, and the obvious implementation is
wrong in two ways that only appear under a fast editor.
Conversion is keyed on a sha256 of the publishable files. Identical content
means the same key, so a request for work already in flight joins it instead of
starting a second run over the same bytes into the same directory. The key also
decides publication: a run stores its result only if its key is still the one
most recently asked for. Without that, a slow pre-edit conversion finishing
after a fast post-edit one would overwrite it,
stalewas already false sonothing re-ran, and preview served pre-edit bundles until the next save — with
readyresolving to the new scene whileget()returned the old one.Keyed on content rather than mtime because a touch with no edit must not
reconvert, and
readProjectFilesalready reads every byte, so the digest isfree.
Deciding whether to start a run is itself async — the key comes from reading
the files — so a gate holds read-compare-register and opens the moment a
decision is made. Held across the conversion it would serialise the work, and a
newer edit has to be free to start while a superseded run finishes.
The cache is safe across processes
A second
sdk-commands starton the same project — another terminal, a staleprocess, CI running preview twice — used to interleave bundle files into one
directory that a reader was about to serve.
Conversions now stage into a private directory and publish with a single
rename, atomic on every filesystem this runs on. A reader sees the entry wholeor not at all, and two previews racing the same content produce identical bytes,
so the loser discards its own rather than merging into a directory it does not
own. An advisory lock keeps the second preview off work already running; every
failure mode in it — stale lock, timeout, unwritable directory — degrades to
converting twice, which the atomic publish already makes safe, so correctness
never rests on the lock. A killed conversion's staging is swept on the next
start, age-gated so a live conversion's directory is never removed from under
it.
Startup and degradation
Conversion runs once at startup and persists under
.dcl-optimized-assets, sothe explorer's manifest request is always a hit and only a scene's first preview
pays the wait. A manifest request arriving mid-conversion waits on it rather
than 404ing into a permanent raw-GLTF fallback.
A missing addon, a failed conversion, and a per-asset failure all fall back to
raw GLTFs.
@dcl/abgen-nodeis a plain dependency, not an optional one: itdeclares no
os/cpuconstraint and its per-platform binaries are optionalinside it, so the JavaScript always installs and only the native addon can be
missing. Marking the wrapper optional bought nothing and let an install skip it
for unrelated reasons — a preview quietly serving raw GLTFs with no cause anyone
could name. The load is still guarded, and now names the platform that has no
prebuild instead of telling you to install a package that is already there.
Rebase note
Rebased onto
mainafter #1502. Two conflicts, both resolved in main's favour:wireFileWatcherToWebSockets— main sendsupdateSceneunconditionallybecause the legacy JSON protocol is the only one Bevy and Godot understand.
This branch had gated it behind a
desktopClientflag, which would haveregressed Bevy Web; the flag is dropped and only the
onProjectChangedinvalidation is layered on.
unlinkhandler — main's(file)signature is correct. This branch read(_, file), leavingfileundefined, since chokidar emits the path first.The 38 commits are collapsed into one because they included an entire sidecar
implementation that later commits delete; replaying them would have meant
resolving conflicts in code that does not survive.
Tests
52 passing across the four
startsuites.asset-bundles.spec.tscoversin-process conversion, on-disk cache reuse, CDN read-through, both degrade
paths, the content-keyed dedup, latest-wins, the atomic publish, lock takeover
and the staging sweep;
asset-bundles-proxy.spec.tscovers local serving,read-through header scrubbing, wait-for-conversion and malformed names.
abgen-binary.spec.tsis removed with the module.Each locking mechanism is mutation-checked — disabling the in-flight join, the
staleness check, the lock-age inspection or the sweep's age gate each fails its
own test and no other.
tsc --noEmitreports no errors in any changed file; the pre-existing@dcl/ecs/dist-cjsresolution errors are the unbuilt sibling package.