feat: optimized assets preview option (local asset bundles via abgen)

[dalkia]

Context and Problem Statement

Scene preview always loads raw GLTFs, so creators never see their scene the way it renders in production — GPU-compressed textures, Unity-native meshes, converted materials. Conversion problems (assets that look fine raw but break after asset-bundle conversion) only surface after publishing, and preview performance doesn't reflect what players get.

Solution

A new Optimize Assets checkbox in the Preview options dropdown. Selecting it is inert — it's a per-project preference (persisted in settings.optimizedAssetsByPath, restored when the scene is reopened, never carried across projects). The conversion runs when Preview is pressed: the hub spawns sdk-commands start with the opt-in --asset-bundles flag, and that's the whole integration — sdk-commands owns the abgen sidecar (decentraland/js-sdk-toolchain#1498). It resolves/downloads the pinned binary, boots it against its own preview server, injects local-ab=true into the deeplink, and opens the client when the scene is ready. The explorer derives the optimized-assets base from the realm; the explorer side that consumes the param is decentraland/unity-explorer#9459.

All feedback lives on the Preview button: while converting it greys out and shows Optimizing N% (parsed from the sidecar's [n/total] converting <file> lines; older sidecars degrade to an elapsed-seconds label) with an inline ✕ to cancel. The dropdown shows a plain checkbox — no spinner, progress or ready tick next to it.

Key changes:

• Convert on Preview press, not on toggle select. Deliberate product call: a first optimized preview of a big scene blocks on conversion (cached after the first run), in exchange for deleting the background-warmup machinery — held deeplinks, detach/warmup states, and the launch races they created.
• sdk-commands owns the client launch on every version. The hub never fires the deeplink after a fresh spawn — it only captures it for re-focus (second Preview press), option flips on a running preview, and the mobile-QR endpoint. This removes both version-skew double-open scenarios flagged in review (old hub + new sdk, new hub + old sdk); the sdk's launch path is identical to what's released today.
• ✕ cancels the conversion outright: the spawn is killed and the pending Preview press settles quietly — no client opens, no reinstall-retry, the button unblocks. Keyboard-accessible (IconButton with aria-label).
• Restart on toggle mismatch: --asset-bundles is a spawn-time flag, so when a running preview disagrees with the toggle, Preview restarts it instead of reusing it. Sidecar detection keys on the presence of local-ab in the captured deeplink — backwards compatible with older sdk-commands builds, which emitted local-ab alongside a now-dropped optimized-assets-url (the hub still strips that legacy param when the toggle goes off).
• supportsAssetBundles feature-detects the flag in the scene's installed sdk-commands (mtime-cached — the multi-MB dist file is only re-read after a reinstall/bump).
• Progress channel (preview.progress) and its payload type moved to /shared/types/ipc — single home for main, preload and renderer.
• Fail-open end to end: unsupported sdk-commands, missing binary, or a sidecar that never becomes ready all degrade to today's raw-GLTF preview inside sdk-commands itself.

Earlier revisions of this PR converted in the background on toggle select, holding the deeplink until Preview (which required sdk-commands not to self-open for --hub sessions). That model and its sdk-side launch changes are fully reverted — the sdk PR's remaining delta is just the sidecar.

How to test

Requires client build 0.161.0 to be out — the explorer side that consumes local-ab (unity-explorer#9459) ships in it; earlier clients will open the preview but load raw GLTFs.

Setup — you want two scenes: one on the branch sdk-commands and one on a released version. Easiest is to check out Genesis Plaza twice; in one copy install the branch build:

npm install "https://sdk-team-cdn.decentraland.org/@dcl/js-sdk-toolchain/branch/feat/abgen-preview/dcl-sdk-commands-7.25.1-30386413173.commit-c9645cd.tgz"

Leave the other copy untouched (released @dcl/sdk-commands).

Checkbox visibility — the Optimize Assets checkbox appears in the Preview options dropdown only for the scene with the branch sdk installed (feature-detected from the installed @dcl/sdk-commands). The untouched copy must not show it. (It's also always hidden on Linux — no Unity client there; ignore that platform for testing.)

Full flow on the branch-sdk scene — validate all three paths:

1. Preview waiting for optimization: toggle on → press Preview → the button greys out showing Optimizing N%; when conversion finishes the client opens once. Re-running is fast (conversion cache).
2. Cancelling the optimization: press Preview with the toggle on, then press ✕ mid-conversion → the conversion stops, no client opens, and the button unblocks immediately. Pressing Preview again restarts (resuming from the per-asset cache).
3. Preview without optimization: toggle off → press Preview → normal raw-GLTF preview, identical to today.

Also: the toggle preference should come back when you close and reopen the scene, and should not leak into other projects.

Scenes without the branch sdk — everything works exactly as today: no checkbox, Preview opens a single client, no behavior change of any kind. Since this PR touches the shared preview launch path, running this check on a couple more scenes beyond the untouched Genesis Plaza copy (different sdk versions, big and small scenes) would be beneficial as regression coverage.

Testing

• Unit: 41 main-process tests (launch contract: fresh spawns never fire the deeplink, re-focus does, cancel settles quietly with no reinstall-retry, sidecar-mismatch restart via local-ab, legacy-deeplink compatibility, in-flight ride-along, mtime cache) + 5 renderer slice tests (cancel state machine); typecheck + lint clean
• Toggle off → behavior identical to today (no flag passed, no extra deeplink params)
• Toggle on with an older sdk-commands → warning log, preview runs raw
• Toggle on with a --asset-bundles-capable sdk-commands → sidecar boots, deeplink carries local-ab=true, scene converts (verified against a real scene + Unity Editor client with #9459)

Impact

Opt-in only; default off. The launch path is identical to released behavior for every scene — sdk-commands opens the client itself on all versions, so there is no cross-version skew in either direction. Requires a scene whose @dcl/sdk-commands includes js-sdk-toolchain#1498 for the toggle to appear (older scenes are untouched). First optimized preview of a scene blocks on conversion; wearables/emotes stream prebuilt from the production CDN and are never converted locally.

🤖 Generated with Claude Code

[github-actions]

Test this pull request on windows-latest

Download the correct version for your architecture:

win-x64

[github-actions]

Test this pull request on macos-latest

Download the correct version for your architecture:

mac-x64
mac-arm64

Click here if you don't know which version to download

For running this unsigned version of the app, you will need to run the xattr command on it:

1. Extract the app from the downloaded .dmg file (double-click it)
2. Place the extracted app anywhere you like in your file system
3. Open a terminal on the directory where the app is
4. Run xattr -c app-name, replacing "app-name" for the actual name of the app
5. Double-click the app ✅

[decentraland-bot]

Review: feat: optimized assets preview option (local asset bundles via abgen)

16 files changed (+585 −48) · CI: lint ✅ typecheck ✅ unit ✅ · E2E ❌ (macOS install step — environment issue, unrelated to this PR)

Summary

Well-structured feature that adds an opt-in "Optimize Assets" toggle to preview options, enabling local asset-bundle conversion via the sdk-commands sidecar. The warmup/cancel/detach lifecycle is complex but justified — each piece maps to a real user interaction (convert-on-toggle, cancel-on-untoggle, detach-on-✕). The inflightStarts serialization correctly prevents concurrent spawns for the same path, and the fail-open design degrades gracefully to raw GLTFs at every failure point.

Key design strengths:

• Session-only option — forced off at config load, so a multi-minute conversion is never a startup surprise
• Serialization via inflightStarts — pressing Preview mid-conversion awaits the in-flight spawn instead of racing a second one
• warmup flag semantics — detach sets it, explicit Preview clears it, launchOrKeepWarm checks it — the ordering is safe in single-threaded JS and well-commented

Findings (all P2 — none blocking)

[P2] supportsAssetBundles() reads the same file from disk 3–4 times per preview launch
cli.ts:306 — Called from warmupOptimizedAssets (line 334), the sidecar-disagree check in start (line 421), the extra-args build (line 441), and the renderer via IPC (PreviewOptions.tsx:46). The file lives in node_modules and won't change during a session. Could be cached per path and invalidated on install(). Not a correctness issue — just redundant I/O.

[P2] CSS pseudo-disable pattern skips aria-disabled
styles.css:89, component.tsx:388–394 — The button is visually disabled via pointer-events: none + opacity: 0.5 and functionally inert (onClick={undefined}), but it isn't flagged as aria-disabled="true" for assistive technology. Consider adding aria-disabled to the ButtonGroup when isOptimizing is true.

[P2] previewDetached state is reducer-internal only
slice.ts:107,129 — previewDetached is set/read exclusively inside the reducer (to gate isPreviewRunning on runScene.fulfilled). It could be a local variable in the reducer logic rather than part of the serializable state shape, making it clearer it's not intended for external consumption.

[P2] No new tests for the lifecycle functions
cli.ts — warmupOptimizedAssets, cancelOptimizedAssetsWarmup, detachPreview, supportsAssetBundles have no unit tests. Consistent with the existing codebase (no cli.ts tests exist), but the lifecycle complexity would benefit from coverage — particularly the inflightStarts serialization and warmup-then-preview handoff.

Security review

No security issues found. The new code follows established codebase patterns:

• Process args are array-based (no shell injection)
• URL params use URLSearchParams (proper encoding)
• Progress regexes are all linear (no ReDoS)
• IPC event channel carries trusted data from the main process
• supportsAssetBundles returns only a boolean (minimal information exposure)

Consumer impact

The PreviewOptions type change (optimizedAssets: boolean) is internal to the creator-hub monorepo — it is not exported as an npm package. The PreviewOptions types found in @dcl/schemas, marketplace, and ui are unrelated wearable/emote preview types. No downstream breakage.

Git conventions (ADR-6) ✅

• Branch: feat/optimized-assets-preview — matches <type>/<summary>
• Title: feat: optimized assets preview option (local asset bundles via abgen) — matches <type>: <summary>

---

Reviewed by Jarvis 🤖 · Requested by Juan Ignacio Molteni [Dalkia] (<@U03JSUQ5Z7U>) via Slack

[decentraland-bot]

Updated review — P0 found after deeper concurrency analysis

My earlier approval missed a critical concurrency issue in the cancel flow. Updating to request changes.

P0 — waitFor promise never settles on process kill → cancel permanently breaks Preview

cli.ts:491-506 — When cancelOptimizedAssetsWarmup kills a warmup process, the process.waitFor() promise inside the spawned IIFE (line 492) never settles. Here's why:

1. bin.ts:cleanup() (line 115-125) disables all matchers and clears the array when the process exits
2. waitFor (line 227-233) registers its resolve/reject callbacks as once matchers
3. When the process is killed, the exit handler (line 152-178) resolves the main promise (returned by .wait()) but then calls cleanup(), which destroys the waitFor matchers before they can fire
4. Result: the waitFor promise hangs forever

Impact chain:

• start() hangs at await spawned (line 506) → finally block never runs → inflightStarts.delete(path) never executes
• Any subsequent start() call finds the stale promise via inflightStarts.get(path) (line 406) → await pending hangs forever
• Preview button is permanently broken for this project until app restart

Reproduction: Toggle Optimize Assets ON, wait for conversion to start, then toggle OFF. Now press Preview — it hangs forever.

Suggested fix: Race waitFor against process.wait() to detect process death:

const resultLogs = await Promise.race([
  process.waitFor(dclLauncherURL, /CliError|error:/i),
  process.wait().then(() => {
    throw new Error('Process exited without producing a deeplink');
  }),
]);

process.wait() settles via the exit handler's promise.resolve() (bin.ts:175), so the race will detect process death and reject spawned, allowing the catch/finally blocks to clean up inflightStarts.

---

The P2 findings from my initial review still stand (redundant supportsAssetBundles reads, missing aria-disabled, reducer-internal state, no new tests). Everything else about the architecture, security, and consumer impact is solid.

---

Reviewed by Jarvis 🤖 · Requested by Juan Ignacio Molteni [Dalkia] (<@U03JSUQ5Z7U>) via Slack

[decentraland-bot]

Re-review after fix commit 7fbd924c

The P0 blocker (waitFor promise never settling on process kill) is fixed correctly:

const resultLogs = await Promise.race([
  process.waitFor(dclLauncherURL, /CliError|error:/i),
  process.wait().then(() => {
    throw new Error('Preview process exited before producing a deeplink');
  }),
]);

Why this works:

• Normal flow: waitFor resolves first (deeplink produced) → process.wait() stays pending while the server runs. Promise.race picks the deeplink. ✓
• Cancel flow: process killed → process.wait() settles via the exit handler (promise.resolve() at bin.ts:175) → .then() throws → spawned rejects → catch/finally blocks fire → inflightStarts cleaned up. ✓
• No unhandled rejection: Promise.race internally calls .then(resolve, reject) on all branches, so the losing branch's eventual rejection is handled. ✓

The aria-disabled P2 fix is also included in this commit. ✓

Remaining P2 findings (unchanged, not blocking)

• supportsAssetBundles() redundant disk reads (could be cached per path)
• previewDetached state is reducer-internal only (could be local to reducer logic)
• No new tests for lifecycle functions (consistent with codebase but the complexity warrants it)

CI

Lint ✅ · typecheck ✅ · unit ✅ · E2E pending (macOS install step — pre-existing infra issue)

---

Reviewed by Jarvis 🤖 · Requested by Juan Ignacio Molteni [Dalkia] (<@U03JSUQ5Z7U>) via Slack

[nicoecheza]

Review

Reviewed together with decentraland/js-sdk-toolchain#1498. The division of labor is clean (sdk owns the sidecar, hub owns when the held deeplink fires), fail-open is consistent at every layer, and the comments explaining the why are genuinely good. The Promise.race([waitFor, wait()]) fix is correct and necessary — I verified bin.ts disables all matchers on process exit, so a bare waitFor would hang forever after a cancelled warmup and wedge inflightStarts.

P1 — version-skew launch behavior (cross-PR)

launchOrKeepWarm now fires dclDeepLink after every fresh spawn, but sdk-commands is a per-scene dependency:

• Old sdk in the scene: old sdks still self-open the client from runApp. Result: two deeplink activations per first preview (one raw, one adjusted) — and with openNewInstance the adjusted one carries open-deeplink-in-new-instance=true, potentially opening two client instances. This also means "zero behavioral change with the toggle off" doesn't hold: the launch path changed for all previews.
• Suggested fix (one-liner): gate the fresh-spawn deeplink fire on supportsAssetBundles(path) — it's a reliable proxy for "this sdk no longer self-opens for hub sessions", since both changes ship in the same sdk release. Old-sdk scenes then keep today's exact behavior.

(The mirror case — old hub + new sdk — is flagged on the sdk PR.)

P2 — no tests

This PR adds the most intricate concurrency state machine in the main process (inflightStarts serialization, warmup/detach/cancel lifecycle, the exit race, progress parsing, deeplink munging, five new reducers) with zero unit tests, while the companion sdk PR is thoroughly tested. supportsAssetBundles, updateDeepLinkWithOpts, the progress regexes, and the slice reducers are all cheaply testable with the existing vitest setup — and the two races below are exactly what tests would catch.

P2 — warmup/press double-spawn race

warmupOptimizedAssets checks inflightStarts.has(path), but start() only registers the inflight promise after several awaits (sceneHasLandscapeTerrain, supportsAssetBundles, getEnv, run). Ticking the toggle then pressing Preview inside that window double-spawns; the losing process is overwritten in previewCache and leaks (alive, unreachable by killAllPreviews) until app exit. Fix: register the inflight entry synchronously at start() entry.

P3 — double deeplink on detach + re-press

Press Preview → ✕ → press Preview again mid-conversion: both start() calls (the original and the one awaiting the inflight) run launchOrKeepWarm when the spawn resolves, and the re-press cleared the warmup flag — so the deeplink fires twice. Benign for same-instance focus; two instances with openNewInstance. A per-path launch token (only the latest press fires) would fix it.

Minor

• optimizedAssetsReady is unreliable for direct (non-warmup) optimized runs: the tick only flips on a progress line with done === total, which the 2.5s poll can miss. Consider setting it in runScene.fulfilled when the launched preview carries the sidecar.
• { seconds, done?, total? } is hand-copied in four places, and preload hardcodes 'preview.progress' while PREVIEW_PROGRESS_EVENT sits unused in cli.ts. Both belong in /shared/types.
• navigator.platform is deprecated, and the "abgen ships windows/mac asset bundles only" comment disagrees with the sdk PR pinning four Linux/ARM archives and mapping linux as a build platform — one of the two stories is wrong.
• supportsAssetBundles re-reads the multi-MB sdk dist file on menu render + warmup + every start; cache by mtime.
• The ✕ CloseIcon is click-only (no keyboard access/role) inside an aria-disabled group.

Verdict

Design and UX reasoning are strong. I'd hold merge for: (a) the old-sdk double-launch gate, (b) a first tranche of unit tests for cli.ts + the slice, (c) the synchronous inflightStarts registration. Everything else can be follow-up.

[decentraland-bot]

Re-review after commits 2031a8e..341b124 (4 commits since last approval)

17 files changed (+585 −48) · CI: lint ✅ typecheck ✅ unit ✅ · E2E pending (macOS install step — pre-existing infra issue)

What changed since last review

Four commits address every P1/P2 raised by the prior review rounds:

Commit	Addresses
2031a8e persist optimize-assets toggle per project path	New feature — per-project preference in optimizedAssetsByPath, hydrated on project open
101ac1a convert on Preview press only — drop the warmup machinery	P1 version-skew double-launch, P2 warmup/press double-spawn race, P3 double deeplink on detach+re-press, Minor optimizedAssetsReady reliability
d529015 test: cover the press-to-convert launch contract and cancel flow	P2 no tests — 38 main-process tests (fresh spawn, re-focus, sidecar mismatch restart, inflight ride-along, cancel settle, crash-reinstall-retry, mtime cache) + 5 slice tests (progress, cancel state machine)
341b124 address review nits on the preview progress plumbing	Minor progress type hand-copying — PREVIEW_PROGRESS_EVENT constant and PreviewProgress type now in shared ipc.ts; navigator.platform replaced with navigator.userAgent

Prior findings — status

Finding	Status
P0 waitFor hangs on process kill (fixed in 7fbd924)	✅ Still correct — Promise.race([waitFor, wait()]) handles cancel and crash
P1 Version-skew double-launch (nicoecheza)	✅ Resolved — hub no longer fires dclDeepLink after a fresh spawn; sdk-commands self-opens the client on all versions. The deeplink is only fired for re-focus of an already-open preview
P2 Warmup/press double-spawn race (nicoecheza)	✅ Resolved — warmup machinery (warmupOptimizedAssets, launchOrKeepWarm, detachPreview, warmup flag) entirely removed
P2 No tests	✅ Resolved — cli.test.ts (390 lines) + slice.spec.ts (122 lines)
P2 supportsAssetBundles redundant reads	✅ Resolved — mtime-based cache in assetBundlesSupportCache Map
P2 Missing aria-disabled	✅ Resolved — aria-disabled={isOptimizing || undefined} on ButtonGroup
P2 previewDetached reducer-internal state	✅ Resolved — removed with warmup machinery; replaced by simpler previewCancelled
Minor navigator.platform deprecated	✅ Uses navigator.userAgent.includes('Linux')
Minor Progress type hand-copied	✅ PREVIEW_PROGRESS_EVENT constant + PreviewProgress type in /shared/types/ipc
Minor CloseIcon keyboard access	✅ Uses IconButton with aria-label

New findings (all P2 — none blocking)

[P2] inflightStarts race window is narrower but still present
cli.ts:475 — inflightStarts.set(path, spawned) is registered after several await points (sceneHasLandscapeTerrain, supportsAssetBundles, getEnv, and the synchronous run() call). A concurrent start() entering during that window would not see the inflight entry and would double-spawn, leaking the first process. In practice, the Redux loadingPreview flag disables the Preview button synchronously on dispatch, so the UI prevents the double-click. The race is only reachable through direct IPC calls (e.g. a mobile-QR start racing a Preview press). Risk is low — noting for completeness.

[P2] optimizedAssetsByPath grows unbounded
settings.ts:24 — The per-project preference map is keyed by filesystem path and entries are never cleaned up. Each entry is tiny (string → boolean), so this won't be a practical problem for a long time, but there's no pruning (e.g. dropping entries for projects that no longer exist). Fine as follow-up work.

Security review

No security issues found. Same assessment as the prior review, plus:

• New IPC handlers (cli.cancelPreview, cli.supportsAssetBundles) follow the existing handle/invoke pattern with no new attack surface
• supportsAssetBundles reads from the project's own node_modules — no path traversal risk (the path originates from the trusted project config)
• sendPreviewProgress sends structured data ({path, progress}) over IPC to the renderer — no injection vector
• optimizedAssetsByPath is keyed by the project path from config, not user-supplied input

Consumer impact

No public API surface changed. All modified types (PreviewOptions, AppSettings, PreviewProgress, Ipc) are internal to the creator-hub monorepo. No downstream breakage.

Git conventions (ADR-6) ✅

• Branch: feat/optimized-assets-preview — matches <type>/<summary>
• Title: feat: optimized assets preview option (local asset bundles via abgen) — matches <type>: <summary>

---

Reviewed by Jarvis 🤖 · Requested by Juan Ignacio Molteni [Dalkia] (<@U03JSUQ5Z7U>) via Slack

[github-actions]

Test @dcl/inspector package

• Preview: link
• Install via NPM:

  npm install "https://sdk-team-cdn.decentraland.org/creator-hub/branch/feat/optimized-assets-preview/@dcl/inspector/dcl-inspector-7.36.4-commit-658f28dcc4c96f0f20e63109a789321c67c8d4dd.tgz"

[decentraland-bot]

Re-review after commit 31a15e8

+174 lines across 4 files · CI: lint ✅ typecheck ✅ unit ✅ · E2E pending (pre-existing macOS infra issue)

What changed

Direct fix for the [P2] unbounded optimizedAssetsByPath finding from the prior review. Two complementary cleanup paths:

1. Startup pruning (config.ts) — on config load, prune entries whose project path no longer exists on disk (existsSync). Sync I/O is fine here — runs once at startup, not in a hot path. The pruned config is persisted via the existing dirty-check (JSON.stringify comparison).

2. Unlist cleanup (workspace.ts) — when a project is removed from the workspace (unlistProjects), its optimizedAssetsByPath entry is deleted in the same config mutation. delete settings.optimizedAssetsByPath?.[_path] safely no-ops when the map is undefined.

Both paths are tested:

• config.test.ts (108 lines) — prune gone entries, keep alive ones, persist pruned config
• workspace.spec.ts (+46 lines) — drop preference on unlist, handle missing map gracefully

Prior P2 findings — status

Finding	Status
inflightStarts race window	Unchanged — low risk, mitigated by UI-level button disabling
optimizedAssetsByPath unbounded growth	✅ Resolved — pruned at startup + on project unlist

Security

existsSync on config-sourced paths — no new attack surface. The paths originate from the trusted project config, not user input.

---

Reviewed by Jarvis 🤖 · Requested by Juan Ignacio Molteni [Dalkia] (<@U03JSUQ5Z7U>) via Slack

[nicoecheza]

Re-reviewed at 31a15e85. The convert-on-press refactor is a big improvement — dropping the warmup/detach machinery removed the two launch races I flagged along with the state they needed. Confirmed fixed:

• Launch contract restored. The fresh spawn no longer fires dclDeepLink ("the sdk already opened the client…"), and js-sdk-toolchain#1498 reverted its no-self-open change. Fresh spawn = sdk self-opens, re-press = hub fires the adjusted deeplink — exactly the pre-PR behavior, so neither direction of version skew double-launches or opens nothing. This was the blocker; it's gone.
• Tests. cli.test.ts is genuinely good — the fake Child lets the tests drive the deeplink print, process death and cancel independently, and the cases assert the real contract (no self-fire, cache-for-refocus, --asset-bundles only when supported, restart on toggle disagreement, reinstall-retry once, reject when the retry dies). config.test.ts, the unlistProjects specs and the slice specs cover the new persistence and cancel state. Conventions followed throughout.
• Nits addressed: PREVIEW_PROGRESS_EVENT/PreviewProgress now live in /shared/types/ipc, supportsAssetBundles is mtime-cached, the ✕ is a real IconButton with an aria-label, and the Linux gate's comment now explains the actual reason (no Unity desktop client) instead of contradicting the sdk's Linux archives.

One new issue, worth fixing before merge — cancel-then-retry silently does nothing.

cancelPreview.pending clears loadingPreview immediately, so the Preview button is live again while main is still inside await killPreview(path) — which resolves only once the process is confirmed dead: ≥100ms of poll interval, and up to the 5s force-kill timeout in bin.ts. A Preview press landing in that window hits

const pending = inflightStarts.get(path);
if (pending) {
  await pending.catch(() => {});   // swallows the rejection
  return path;                     // …and reports success
}

The in-flight promise is the spawn being killed, so it rejects, the rider swallows it and returns success without spawning anything. runScene.fulfilled then sets isPreviewRunning = true (this run's pending already cleared previewCancelled), so no client opens, nothing is running, and the UI says otherwise. Recovery is another Preview press. "Cancel, then change my mind" is a natural sequence, and the failure is silent.

The narrow fix is to keep the button blocked until the cancel actually completes — clear loadingPreview on cancelPreview.fulfilled (or add a cancelling flag) rather than on pending. Alternatively have the rider distinguish outcomes (await pending.then(() => true, () => false)) and only report success when the spawn it rode actually produced a deeplink — though a fall-through respawn then needs care, since the original's finally will delete whatever inflightStarts entry it finds. The generic case is the same shape: any rider of a spawn that later crashes currently gets a false success.

Related and worth closing at the same time: inflightStarts.set still happens several awaits after the pending check, so two start() calls in the same tick both spawn and the loser leaks (alive, evicted from previewCache, unreachable by killAllPreviews). The UI gating makes this hard to reach now, but the code comment claims the map serializes "a second Preview press (or a mobile-QR start)" and it doesn't quite — and the new test's await flush() between the two starts sits exactly on that boundary. Registering the promise synchronously at start() entry covers this and the above together.

Two small things, non-blocking:

• navigator.userAgent.includes('Linux') is safer than the old navigator.platform but still UA sniffing in the renderer; process.platform through preload would be exact.
• The existsSync prune in getConfigStorage silently drops the saved preference for any project on a temporarily unmounted volume (external drive, network share). Harmless but slightly surprising.

[decentraland-bot]

Re-review after commit fb335ad

Verdict: Approved — both remaining P2 findings from the previous review are now fixed. No new issues.

New commit analysis

fb335ad — fix: block Preview until cancel completes and register inflight starts synchronously

This commit addresses the last two actionable findings:

1. inflightStarts race (previously P2) — ✅ Fixed

Problem: inflightStarts.set(path, spawned) happened deep inside start() after several await calls. Two same-tick calls (e.g. Preview press racing a mobile-QR start) could both pass the inflightStarts.get(path) check before either registered, double-spawning a process previewCache would orphan.

Fix: Split start() into a thin synchronous wrapper + doStart() inner function. The wrapper registers the inflight entry synchronously (no await gap between check and set), and the finally guard uses identity comparison (=== entry) so a stale settle never clobbers a later start's entry. The retry path correctly recurses into doStart() instead of start(), avoiding infinite self-ride.

Verified correct:

• The synchronous doStart() call returns a Promise that gets registered before any microtask yields
• The finally cleanup is identity-guarded: a later start() that registered a new entry won't have its entry deleted by the earlier one settling
• Retry recursion into doStart() bypasses the wrapper, so the outer try/finally spans the full operation including the reinstall-retry — no leak

2. Cancel-then-retry race (previously P2) — ✅ Fixed

Problem: cancelPreview.pending immediately cleared loadingPreview, unblocking the Preview button while main was still killing the spawn. A re-press would ride the dying spawn via inflightStarts, get its rejection swallowed, and report a "running" preview with nothing actually running.

Fix: cancelPreview.pending now only sets previewCancelled = true (keeps loadingPreview = true). cancelPreview.fulfilled/.rejected clear loadingPreview only if previewCancelled is still true — this guard protects a new runScene dispatched mid-cancel (whose .pending clears previewCancelled) from having its loading state stolen. The ✕ button is disabled={previewCancelled} so double-cancel is prevented in the UI too.

State machine verified complete:

• runScene.pending → loadingPreview=true, previewCancelled=false ✓
• runScene.fulfilled → loadingPreview=false, isPreviewRunning=true (guarded by !previewCancelled) ✓
• runScene.rejected → loadingPreview=false, previewCancelled=false ✓
• cancelPreview.pending → previewCancelled=true (button stays blocked) ✓
• cancelPreview.fulfilled/rejected → if (previewCancelled) loadingPreview=false ✓

Test coverage

• +48 lines in cli.test.ts: same-tick serialization test (no flush() between starts) and ride-during-reinstall test (second start lands while install() is pending)
• +51 lines in slice.spec.ts: cancel now asserts button stays blocked until fulfilled; new test verifies a runScene dispatched mid-cancel keeps its own loadingPreview

Both race conditions are exercised with controlled async resolution (deferred promises), making the tests deterministic.

Security

No new IPC surfaces, no new user input handling, no secrets exposure. Process management (kill/spawn) is substantively unchanged — only the registration timing is reordered.

CI

• lint: ✅ pass
• unit tests: ✅ pass
• typecheck: ⏳ pending
• E2E: ⏳ pending (pre-existing macOS infra issue)

---

Reviewed by Jarvis 🤖 · Requested by Juan Ignacio Molteni (<@U03JSUQ5Z7U>) via Slack