refactor: drop the --optimized-assets-url debug arg

[dalkia]

Pull Request Description

What does this PR change?

Removes the --optimized-assets-url CLI/deeplink argument. It was a debug utility for pointing the AB/LOD/registry endpoints at an arbitrary server; with the embedded abgen sidecar (base PR) seeding that override itself, the escape hatch has no remaining use.

• The flag constant, its MainSceneLoader read, its deeplink description, its never-permitted deeplink-test assertion and the allowlist doc mentions are gone — the SEC-052-class attack surface no longer exists rather than being defended.
• The plumbing the arg shared with the sidecar is renamed to what it now exclusively carries: cliOptimizedAssetsUrl → localAbBaseUrl, optimizedAssetsBaseOverride → localAbBaseOverride. ResolveOptimizedAssetsUrl's precedence (override → feature-flag variant → dedicated host) is unchanged.
• The local-ab gate in MainSceneLoader loses its IsNullOrEmpty(cliOptimizedAssetsUrl) clause — the sidecar no longer yields to anything.
• Docs (abgen-sidecar.md, how-to-connect-to-a-local-scene.md) and stale comments/tooltips updated.

Stacked on #9831 (the serial-sidecar refactor) — review only the top commit.

Test Instructions

Steps (standard run):

metaforge explorer run XXXX  # ← replace with this PR number

Expected result:
Client boots and loads scenes normally; asset bundles resolve from the production CDN (no behavior change outside LSD).

Prerequisites

• For the LSD path: a local scene running via npm run start (or Creator Hub)

Test Steps

1. In the Unity Editor, set Initial Realm to Localhost and tick "Use Local Asset Bundles" in the Main Scene Loader.
2. Enter play mode — the abgen sidecar starts and the scene loads as locally converted asset bundles (AB panel shows conversion progress).
3. Launch with --optimized-assets-url=https://example.com — the arg is ignored (unknown parameter); AB endpoints are unaffected.
4. AppArgsTests and DecentralandUrlsSourceShould pass (the sidecar-override test now uses localAbBaseUrl).

Additional Testing Notes

• The deeplink parser drops optimized-assets-url as an unknown param now that it is not a known flag; the never-permitted regression assertion was removed with the flag.

Quality Checklist

• Changes have been tested locally
• Documentation has been updated (if required)
• Performance impact has been considered
• For SDK features: Test scene is included

🤖 Generated with Claude Code

[github-actions]

🚦 CI Status

New build in progress — per-platform Unity Cloud links land here as soon as each build is created.

Lint in progress, come back later!

All Unity tests passed ✅

TESTS SUITE	Result	Passed	Failed	Skipped
EditMode	✅ Passed	25526	0	13
PlayMode	✅ Passed	248	0	37

[decentraland-bot]

Review — refactor: drop the --optimized-assets-url debug arg (top commit only)

Reviewing commit 762de69 as requested by the PR description ("review only the top commit").

---

STEP 2 — Root-cause check: ✅ PASS

The --optimized-assets-url CLI/deeplink argument was a debug escape hatch for pointing the AB/LOD/registry endpoints at an arbitrary server. With the embedded abgen sidecar now being the sole source of optimized-assets URL overrides, this external arg has no remaining consumers. The diff removes the dead code path entirely — this is the correct action, not a symptom workaround.

STEP 3 — Design & integration: ✅ PASS

No new long-lived units are introduced. This is pure removal + rename.

Gate condition simplification verified: MainSceneLoader.InitializeFlowAsync previously checked string.IsNullOrEmpty(cliOptimizedAssetsUrl) as part of the sidecar-start gate. That variable came from applicationParametersParser.TryGetValue(AppArgsFlags.OPTIMIZED_ASSETS_URL, ...), so an external CLI arg could prevent the sidecar from starting. Now localAbBaseUrl is initialized to null on line 328 and only set by abgenSidecar.BaseUrl on line 340 — the IsNullOrEmpty check is structurally unnecessary because nothing else can seed it before the gate runs. Correct removal.

Resolution precedence unchanged: ResolveOptimizedAssetsUrl still follows override → feature-flag variant → dedicated host. Only the field name feeding the override slot changed (optimizedAssetsBaseOverride → localAbBaseOverride).

Rename completeness verified: rg over the full tree confirms zero remaining references to OPTIMIZED_ASSETS_URL, optimized-assets-url, cliOptimizedAssetsUrl, or optimizedAssetsBaseOverride in any .cs or .md file. The rename is complete.

STEP 4 — Member audit: ✅ PASS

No new public properties or accessors. The parameter rename propagates consistently through:

• MainSceneLoader.InitializeFlowAsync → local variable localAbBaseUrl
• GatewayUrlsSource constructor → parameter localAbBaseUrl → forwarded to base
• DecentralandUrlsSource constructor → parameter localAbBaseUrl → stored as field localAbBaseOverride
• DecentralandUrlsSource.ResolveOptimizedAssetsUrl → reads localAbBaseOverride

All consumers consistent. No orphaned references.

STEP 5 — Line-level review: ✅ No issues

All changes are mechanical rename + removal. No bugs, no missing error handling, no resource leaks, no performance regressions, no security vulnerabilities introduced.

Security review: ✅ PASS — Security improvement

This commit is a net security improvement. The --optimized-assets-url deeplink parameter (SEC-052 class) could redirect AB/LOD/registry endpoints at attacker-controlled infrastructure for the entire session. The attack surface is now eliminated entirely rather than merely defended by the allowlist. The only remaining path to override the optimized-assets URL is:

1. The embedded abgen sidecar (internal, gated on LSD + --local-ab, sets localAbBaseUrl programmatically)
2. The optimized-assets feature flag (server-controlled, not user-injectable)

Neither is reachable via deeplink or CLI injection.

STEP 8 — Non-blocking warnings: None

Main.unity is not in the changed files.

---

REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Removes one CLI flag constant and renames the shared plumbing across URL resolution and MainSceneLoader; no logic, ECS, or async changes.
QA_REQUIRED: YES

---

Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub

[decentraland-bot]

PR #9923, run #33425569114

Overall: ✅ no significant changes

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table

• Each build is measured 3 times, interleaved with the other build (change, baseline, change, baseline, ...) in the same session, so both see the same world content and machine state. The values are the median, and (min–max) is the lowest and highest of those runs.
• Δ is Change minus Baseline (a negative Δ means Change is faster).
• 🟢 faster / 🔴 slower — a difference that passed every check: the runs are fully separated (every run of one build faster than every run of the other), and the median difference is at least 3% and at least 0.5 ms.
• ⚪ within noise — the builds' runs overlap, or the difference is tiny; it cannot be told apart from random variation. Treat it as no change.
• — informational — the 0.1% worst metrics average only the few worst frames of a run, so a single OS hiccup swings them by a lot; they are shown for context and never earn a verdict.
• ⚠️ no verdict — the two builds' sessions were not comparable (very different sample counts, or too few usable runs), so no conclusion is drawn from them.
• Exceptions per run — the average number of exceptions in a run's log, not counting teardown ones logged while the app quits. Flagged only on a difference of at least 2 per run and 1.5× the other build; exception kinds the baseline never threw are called out under the table. The Exception breakdown groups all of them by the explorer's report category and exception type (as totals across the runs).
• A run that logged unusually many exceptions (at least 10 and 5× the median of its build's runs — e.g. a service was down during it) is excluded from all numbers and called out under the table.
• The Overall line at the top only reacts to a metric that moved on two or more machines, or by 10% or more on one — a single modest 🟢/🔴 cell can still be a statistical fluke.

Intel Core i5

Metric	Baseline	Change	Δ	Result
Samples	2427 (×3)	2431 (×3)
CPU average	36.9 ms (36.8–40.2)	36.9 ms (35.3–38.4)	-0.0 ms	⚪ within noise
CPU 1% worst	381.6 ms (381.2–465.1)	382.5 ms (247.9–385.5)	0.8 ms	⚪ within noise
CPU 0.1% worst	447.2 ms (439.1–485.5)	448.8 ms (420.6–462.7)	1.6 ms	— informational
GPU average	23.2 ms (23.0–26.0)	22.9 ms (22.6–24.2)	-0.3 ms	⚪ within noise
GPU 1% worst	383.4 ms (362.4–467.5)	342.7 ms (247.7–390.6)	-40.6 ms	⚪ within noise
GPU 0.1% worst	453.2 ms (441.9–487.9)	455.4 ms (433.0–468.0)	2.3 ms	— informational
Exceptions per run	0	0.33	+0.33	⚪ no significant change

Exception breakdown

Exception	Baseline (3 runs)	Change (3 runs)
[SCENE_LOADING] Exception	0	1

Apple M1

Metric	Baseline	Change	Δ	Result
Samples	3381 (×3)	3400 (×3)
CPU average	26.4 ms (26.4–26.7)	26.4 ms (25.4–26.4)	-0.0 ms	⚪ within noise
CPU 1% worst	206.5 ms (184.1–226.5)	167.8 ms (164.2–216.3)	-38.7 ms	⚪ within noise
CPU 0.1% worst	245.4 ms (243.1–248.2)	242.2 ms (241.5–245.4)	-3.2 ms	— informational
GPU average	19.1 ms (18.6–21.8)	20.3 ms (19.6–20.3)	1.3 ms	⚪ within noise
GPU 1% worst	53.0 ms (52.8–53.0)	48.6 ms (47.6–55.9)	-4.3 ms	⚪ within noise
GPU 0.1% worst	59.3 ms (58.9–60.5)	58.3 ms (55.5–60.1)	-1.0 ms	— informational
Exceptions per run	0	0	0	⚪ no significant change

[decentraland-bot]

PR Review: refactor: drop the --optimized-assets-url debug arg

STEP 2 — Root-cause check: ✅ PASS

The problem is dead code: the --optimized-assets-url CLI/deeplink argument is no longer used now that the embedded abgen sidecar seeds the override itself. This PR removes the dead code at its root — the flag constant, its CLI read, its deeplink description, its test assertion, and all doc references — rather than leaving orphaned code behind.

STEP 3 — Design & integration: ✅ PASS

No new long-lived units are introduced. This is a pure removal and rename. The existing lifecycle is unchanged:

• MainSceneLoader still owns the sidecar bootstrap lifecycle (construct → StartAsync → set override URL → pass to GatewayUrlsSource).
• DecentralandUrlsSource still resolves the optimized-assets URL via the same precedence chain (override → feature-flag variant → dedicated host).
• GatewayUrlsSource still passes the parameter through to its base class.

Behavioral change analysis: Removing the string.IsNullOrEmpty(cliOptimizedAssetsUrl) guard from MainSceneLoader.cs:330 is correct. Before, the sidecar would not start if a CLI URL was provided. Now, since the CLI flag constant (OPTIMIZED_ASSETS_URL) is deleted, the parser can never produce a value for it — the guard would always pass. Removing the dead branch is the right call.

Constructor parameter chain: The rename cliOptimizedAssetsUrl → localAbBaseUrl is consistent across all three layers (MainSceneLoader → GatewayUrlsSource → DecentralandUrlsSource). The field name localAbBaseOverride follows the existing convention (gatekeeperBaseOverride).

Teardown / consumption trace: No new subscriptions, callbacks, connections, or disposables introduced.

STEP 4 — Member audit: ✅ PASS

No new public properties or accessors added. The only changes are parameter renames, which are consistent across the full call chain.

STEP 5 — Line-level review: ✅ No blocking issues

All 13 changed files reviewed. No remaining references to the old names (OPTIMIZED_ASSETS_URL, optimized-assets-url, cliOptimizedAssetsUrl, optimizedAssetsBaseOverride) found anywhere in the codebase via ripgrep.

[P2] ResolveOptimizedAssetsUrl method name (DecentralandUrlsSource.cs:212): The method name still says "OptimizedAssetsUrl", which echoes the removed CLI argument's name. However, this is the concept (the URL for optimized assets — AB CDN, LOD CDN, registry), not the CLI arg. The xmldoc (line 209) was already updated to reference the sidecar. The name remains semantically accurate. Optional follow-up rename to e.g. ResolveAssetBundleBaseUrl would improve clarity but is not required by this PR.

[P2] CacheBehaviour.FeatureFlagsDependent for local override (DecentralandUrlsSource.cs:215): When localAbBaseOverride is set, the method returns FeatureFlagsDependent, meaning the URL is re-resolved on every call until feature flags load. For a static localhost URL this is semantically wrong (it won't change when flags load), but functionally harmless — and it serves a secondary purpose: GatewayUrlsSource (line ~148) uses this cache behavior to skip gateway rewriting, which is correct for a localhost sidecar. This is a pre-existing design issue, unchanged by this PR.

Security review: ✅ No issues found

• The SEC-052-class attack surface (arbitrary infrastructure pointing via deeplink) is eliminated, not just defended.
• The DeepLinkAllowlist deny-by-default mechanism already drops any unknown parameter, so the removed test assertion for optimized-assets-url is redundant with the flag's removal — there is nothing left to test.
• The override URL (localAbBaseOverride) can now only originate from abgenSidecar.BaseUrl inside the MainSceneLoader sidecar bootstrap path, guarded by LaunchMode.LocalSceneDevelopment && useLocalAssetBundles. No CLI/deeplink injection path remains.
• The feature-flag string "optimized-assets" in FeatureFlagsStrings.cs is a server-side feature flag (controls the AB pipeline fleet-wide), not the removed CLI argument — correctly retained.

STEP 6 — Complexity: SIMPLE

Pure removal/rename across 13 files. No ECS systems, components, async patterns, or complex logic changed.

STEP 7 — QA assessment: YES

Changes runtime code (MainSceneLoader.cs, DecentralandUrlsSource.cs) that affects asset bundle URL resolution in local scene development mode.

STEP 8 — Non-blocking warnings

None. No Main scene modified.

STEP 9 — Verdict

REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Pure removal of a dead CLI argument and consistent rename of internal plumbing — no systems, components, or async flows changed.
QA_REQUIRED: YES

---

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