chore: expose requestid for cross tests

[cyaiox]

Pull Request Description

What does this PR change?

Adds a small, compile-time-gated test affordance to UnityAppWebBrowser.OpenUrl(string) so cross-stack Playwright tests in explorer-automation can intercept the auth-funnel URL the client would otherwise hand off to the system browser.

Problem

Cross-stack tests for the device-pairing / auth flow need the URL the client opens in the system browser to drive the web side. Today:

• The URL is never logged, never surfaced in the Unity scene, and only readable from the system browser via osascript (CI-hostile, mac-only, fragile).
• Letting Application.OpenURL fire under test also pops a stale tab the Playwright runner ignores, confusing anyone watching the run.

Solution

Under #if ALTTESTER only:

• Write the escaped URL to Path.Combine(Application.persistentDataPath, "auth-url.txt") — Playwright polls and reads it.
• Suppress Application.OpenURL entirely.

Production builds are untouched — the entire block is stripped at compile time when ALTTESTER isn't defined.

Implementation notes

• Gated with the same #if ALTTESTER pattern used in SceneFacade.cs, AlttesterSceneReadinessProbe.cs, and MainSceneLoader.cs.
• Output path uses Application.persistentDataPath so the affordance works on macOS, Windows, and Linux without a hardcoded per-OS path. Matches the convention used by DCLPlayerPrefs, ChatHistoryStorage, and ThirdWebLoginService.
• I/O failures are reported via ReportHub.LogException(e, ReportCategory.AUTHENTICATION) so they participate in the project's diagnostics / Sentry pipeline (per CLAUDE.md — ReportHub over Debug.Log*).
• Zero new dependencies, no DI / plugin / asmdef wiring changes.

Coordination required

The Playwright consumer (explorer-automation/web/tests/auth/specs/client-to-web-handoff.spec.ts) must read from Unity's persistentDataPath, which is platform-specific:

• macOS: ~/Library/Application Support/Decentraland/Decentraland Unity Explorer/auth-url.txt
• Windows: %USERPROFILE%\AppData\LocalLow\Decentraland\Decentraland Unity Explorer\auth-url.txt
• Linux: ~/.config/unity3d/Decentraland/Decentraland Unity Explorer/auth-url.txt

Exact folder names come from companyName / productName in ProjectSettings.asset.

Files changed

• Explorer/Assets/DCL/NetworkDefinitions/Browser/UnityAppWebBrowser.cs (+21 / -1)

Test Instructions

Prerequisites

• An ALTTESTER build of Unity Explorer (Scripting Define Symbol ALTTESTER).
• The companion explorer-automation checkout updated to read auth-url.txt from Application.persistentDataPath (see Coordination required above).

Test Steps

Smoke — production build (no ALTTESTER)

1. Build the project without the ALTTESTER define.
2. Trigger an auth-funnel link (e.g. open the sign-in screen and click "Sign in").
3. Expected: the system browser opens at the auth URL exactly as before. No file is created on disk.

ALTTESTER build (cross-stack test path)

1. Build with ALTTESTER defined.
2. Trigger an auth-funnel link.
3. Expected:
   • The system browser does not open.
   • auth-url.txt is written under Application.persistentDataPath containing the escaped URL.
4. Run the Playwright suite: client-to-web-handoff.spec.ts should pick up the URL and complete the handoff.

Failure-path smoke

1. With ALTTESTER defined, point Application.persistentDataPath at a read-only location (or temporarily mark the file read-only after creation).
2. Trigger the auth flow.
3. Expected: a ReportCategory.AUTHENTICATION exception is logged via ReportHub; auth flow continues without throwing.

Additional Testing Notes

• Verify the entire #if ALTTESTER block (including using System.IO; and using DCL.Diagnostics;) is stripped from a non-ALTTESTER build — quick check via ilspy / dotPeek on the produced assembly.
• The branch name (chore/expose-requestid-for-cross-tests) is from an earlier scope; this PR does not expose a request ID. Worth renaming the PR title to match: "chore(browser): expose auth URL to disk for cross-stack tests (ALTTESTER)".

Quality Checklist

• Changes have been tested locally
• Documentation has been updated (if required) — N/A, no public API change
• Performance impact has been considered — runs once per auth flow, only in instrumented builds
• For SDK features: Test scene is included — N/A

Code Review Reference

Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does — especially useful for first-time contributors.

[github-actions]

Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below.

Name	Link
Commit	24ef121
Logs	https://github.com/decentraland/unity-explorer/actions/runs/27960419904
Download Windows	https://github.com/decentraland/unity-explorer/suites/75369265593/artifacts/7796639147
Download Windows S3	https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/chore/expose-requestid-for-cross-tests/pr-23202-24ef121/Decentraland_windows64.zip
Download Mac	https://github.com/decentraland/unity-explorer/suites/75369265593/artifacts/7796874529
Download Mac S3	https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/chore/expose-requestid-for-cross-tests/pr-23202-24ef121/Decentraland_macos.zip
Built on	2026-06-22T15:28:18Z

[cyaiox]

@claude review

[cyaiox]

@claude review

[m3taphysics]

PR #8862, run #26298583791

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

Framework 13 i7

Metric	Change	Baseline	Delta	Improvement
Samples	2701	2700
CPU average	33.3 ms	33.3 ms	-0.0 ms	0.0%	⚪
CPU 1% worst	33.5 ms	33.9 ms	-0.3 ms	1.0%	⚪
CPU 0.1% worst	35.2 ms	38.2 ms	-3.0 ms	7.9%	🟢
GPU average	7.7 ms	7.6 ms	0.1 ms	-0.9%	⚪
GPU 1% worst	19.3 ms	18.4 ms	0.8 ms	-4.4%	🔴
GPU 0.1% worst	25.0 ms	23.0 ms	2.0 ms	-8.7%	🔴

[decentraland-bot]

Review — chore: expose requestid for cross tests

Files changed: 1 (+20 -1) · CI: pending (editmode/playmode tests, macOS/Windows builds)

Clean, minimal change. The entire #if ALTTESTER block is stripped from production builds — zero risk to shipped code.

Findings

[P2 — Minor] Uri.EscapeUriString deprecation
Uri.EscapeUriString(url) is deprecated in .NET 5+ because it doesn't escape # and ? characters that may appear in the fragment/query. Unity's runtime may still accept it, but consider switching to constructing a new Uri(url) and calling .AbsoluteUri, or using a targeted escape for the components that need it. Not blocking since the previous code already used EscapeUriString — this PR doesn't introduce the pattern, it just preserves it.

[P2 — Minor] Silent failure path under ALTTESTER
If File.WriteAllText throws, the exception is logged via ReportHub but Application.OpenURL is never called (the #else branch is compile-time excluded). In test builds this is the correct behavior (the test will time out with a clear error from the C# polling assertion), but worth a one-line comment noting the intentional suppression so future readers don't assume it's a bug.

[P2 — Minor] PR title vs. actual change
The PR description itself notes the branch name (chore/expose-requestid-for-cross-tests) is from an earlier scope. The PR exposes the auth URL to disk, not a request ID. Consider updating the title to match the body's suggestion: chore(browser): expose auth URL to disk for cross-stack tests (ALTTESTER).

Security

No security issues found. The file write is local-only, compile-time gated, and uses Application.persistentDataPath (no user-controlled path components). The caught exception is logged through the standard diagnostics pipeline — no sensitive data leakage.

Verdict

APPROVE — No P0 or P1 issues. The change is well-scoped, correctly gated, and follows existing project patterns (ReportHub logging, #if ALTTESTER convention).

---

Reviewed by Jarvis 🤖 · Requested by Gabriel Díaz via Slack

[m3taphysics]

PR #8862, run #26892116315

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

How to read this table

• Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
• Δ is Change minus Baseline (a negative Δ means Change is faster).
• 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
• ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
• Exceptions logged — exceptions found in the run logs; more than the baseline is flagged 🔴 even when frame times look fine.

Framework 13 i7

Metric	Baseline	Change	Δ	Result
Samples	2701 (×3)	2701 (×3)
CPU average	33.3 ms	33.3 ms	-0.0 ms	⚪ within noise
CPU 1% worst	33.9 ms (33.6–34.2)	33.7 ms (33.6–33.8)	-0.2 ms	⚪ within noise
CPU 0.1% worst	36.7 ms (34.4–38.2)	36.5 ms (35.4–38.7)	-0.2 ms	⚪ within noise
GPU average	7.4 ms (7.2–8.3)	8.4 ms (8.2–8.5)	1.1 ms	⚪ within noise
GPU 1% worst	19.9 ms (18.8–20.4)	17.9 ms (17.7–18.3)	-2.0 ms	🟢 10% faster
GPU 0.1% worst	25.4 ms (22.9–29.4)	23.4 ms (22.7–26.0)	-1.9 ms	⚪ within noise
Exceptions logged	48	44	-4	🟢 fewer errors

[github-actions]

Auto-approved by Claude — simple fix/chore with no blocking issues. QA approval is still required.

[m3taphysics]

PR #8862, run #27245602788

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

How to read this table

• Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
• Δ is Change minus Baseline (a negative Δ means Change is faster).
• 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
• ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
• Exceptions logged — exceptions found in the run logs; more than the baseline is flagged 🔴 even when frame times look fine.

Framework 13 i7

Metric	Baseline	Change	Δ	Result
Samples	2701 (×3)	2701 (×3)
CPU average	33.3 ms	33.3 ms	-0.0 ms	⚪ within noise
CPU 1% worst	33.9 ms (33.6–34.3)	33.7 ms (33.5–33.7)	-0.3 ms	⚪ within noise
CPU 0.1% worst	38.5 ms (33.9–40.8)	37.3 ms (35.1–37.4)	-1.2 ms	⚪ within noise
GPU average	7.5 ms (7.2–8.0)	8.0 ms (8.0–8.4)	0.5 ms	⚪ within noise
GPU 1% worst	18.7 ms (18.4–20.5)	18.0 ms (17.3–18.1)	-0.7 ms	⚪ within noise
GPU 0.1% worst	23.3 ms (22.4–24.9)	23.9 ms (23.7–26.6)	0.6 ms	⚪ within noise
Exceptions logged	46	12	-34	🟢 fewer errors

[m3taphysics]

⚠️ Performance test run cancelled: Framework 13 i7 / Test still had not started after 60 minutes — the runner is offline or stuck. Re-run when the machine is back, or remove it from the ENABLED_RUNNERS variable.

[m3taphysics]

⚠️ Performance test run cancelled: Framework 13 i7 / Test still had not started after 60 minutes — the runner is offline or stuck. Re-run when the machine is back, or remove it from the ENABLED_RUNNERS variable.

Dismissed: latest Claude review no longer auto-approves this PR.

[Ludmilafantaniella]

Tested on Windows and Mac. ✅

This change is compile-time gated under #if ALTTESTER so production behavior is untouched. Performed a manual QA regression to confirm no regressions were introduced:

• Log In / Log Out ✅
• Backpack and wearables in world ✅
• Emotes in world and in backpack ✅
• Teleport via map, coordinates, and Jump In ✅
• Chat and multiplayer ✅
• Profile card ✅
• Camera ✅
• Skybox ✅

[claude]

Claude finished @cyaiox's task in 1m 2s —— View job

---

Claude finished reviewing ——

---

• Gather context
• Read changed files and diff
• Analyze for issues
• Post review findings

---

Root-Cause Assessment

Problem: Cross-stack Playwright tests needed the auth URL that Unity hands off to the system browser. The system browser was inaccessible in CI, and Application.OpenURL firing in test runs caused a stale tab that confused test runners.

Fix: The diff correctly addresses the cause by: (1) adding a compile-time gate (#if ALTTESTER) that strips the entire block from production builds, and (2) adding a runtime check (appArgs?.HasFlag(AppArgsFlags.ALTTESTER) == true) so browser suppression only fires when explicitly launched with --alttester — not in Editor or QA standalones. ✅

---

Previous Blocking Issue — Resolved

The P1 flag from the last review cycle (5-line comment block violating CLAUDE.md "Never write multi-line comment blocks — one short line max") has been addressed in the latest commit (d05b5038). The comment is now a single line at UnityAppWebBrowser.cs:28. ✅

---

Remaining Issues

None. No new blocking issues are introduced.

Implementation correctness:

• appArgs?.HasFlag(AppArgsFlags.ALTTESTER) == true correctly returns false when appArgs is null — no null-reference risk. ✅
• Application.OpenURL(escaped) is unconditionally reachable in non-ALTTESTER builds (compile-time) and in ALTTESTER builds without the runtime flag (falls through). Logic is correct. ✅
• IAppArgs? appArgs = null default keeps the constructor backward-compatible with existing test callsites. ✅
• BootstrapContainer.cs wiring passes applicationParametersParser which is already present in the method signature — minimal and correct. ✅
• Exception from File.WriteAllText is caught, logged via ReportHub.LogException(e, ReportCategory.AUTHENTICATION), then return suppresses OpenURL — intentional and correct for test builds. ✅

---

REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Touches a single utility class (UnityAppWebBrowser) and its one-line wiring in BootstrapContainer; no ECS, async, plugin, or rendering systems involved.
QA_REQUIRED: YES

[github-actions]

Auto-approved by Claude — simple fix/chore with no blocking issues. QA approval is still required.