Merge pull request #9441 from decentraland/release/2026-07-20

release: 2026-07-20

Author: anicalbano

.claude/skills/code-standards/SKILL.md

@@ -27,6 +27,10 @@ user-invocable: false
 | Events | Past tense, no `On` prefix | `ViewShowingComplete` |
 | Unused parameters | `_`, `__`, `___` | `Update(float _)` |
 
+**Exception:** serialized JSON DTO fields keep their wire-format casing — see *Serialized JSON DTOs* below.
+
+**Namespaces are domain names, not folder paths** — never suppress a folder-namespace mismatch with `// ReSharper disable once CheckNamespace`; fix the namespace or leave the warning visible. Full rule: `docs/code-style-guidelines.md` § Namespaces.
+
 ## Member Ordering
 
 Within a class, group members in this order:
@@ -131,7 +135,17 @@ The project is migrating to nullable reference types. ~80 of ~153 assemblies alr
 - **Parameters that legitimately accept null** must be typed `T?` (e.g., `string? name`).
 - **Return types that may return null** must be typed `T?`.
 - **Fields and properties that can be null** must be typed `T?`.
-- **Never use the null-forgiving operator `!`** to silence warnings — fix the root cause instead. The only acceptable use is in test code where NSubstitute returns null proxies.
+- **Never use the null-forgiving operator `!`** to silence warnings — fix the root cause instead. The only acceptable uses are test code where NSubstitute returns null proxies, and schema-required serialized DTO fields (see below).
+
+## Serialized JSON DTOs (wire-format exceptions)
+
+Deserialized DTO fields follow the wire format, not local conventions:
+
+- **Naming:** field names keep the JSON key casing — suppress per file with `// ReSharper disable InconsistentNaming` above the namespace declaration.
+- **Nullability (CS8618):** schema-*required* fields are initialized `= null!` (`= default!` when the field's type is a generic parameter); schema-*optional* fields are declared `T?`. The DTO class must carry a schema-link comment (e.g. `// Server schema: <repo>/<file>#/SchemaName`).
+- **Guards follow the declaration:** never null-guard a `null!` field (see anti-pattern 4); an optional `T?` field keeps its guards.
+
+These exceptions apply **only** to deserialized DTO fields — never to locals, return types, or non-DTO code. Authoritative rules, edge cases (strengthened contracts, fields absent from the schema), and the reference example: [`docs/code-style-guidelines.md` § Serialized JSON DTOs](../../../docs/code-style-guidelines.md#serialized-json-dtos-wire-format-exceptions).
 
 ## Comments
 

.github/prompts/review-instructions.md

@@ -1,13 +1,13 @@
 # PR REVIEW PROTOCOL
 
-You are the single automated reviewer for this PR. Execute STEP 1 → STEP 9 below **in order, as one pass**, and record each step's required output in your summary comment before moving on. The ordering is the method, not a suggestion — a line is not worth fixing if Steps 2–3 show the change is built in the wrong place. Steps differ in the context they need: Step 3 reads the surrounding files from Step 1; Steps 4–5 work only from the diff. The four lines in STEP 9 are the single verdict downstream automation parses — emit them exactly.
+You are the single automated reviewer for this PR. Execute STEP 1 → STEP 9 below **in order, as one pass**, and record each step's required output in your review body before moving on. The ordering is the method, not a suggestion — a line is not worth fixing if Steps 2–3 show the change is built in the wrong place. Steps differ in the context they need: Step 3 reads the surrounding files from Step 1; Steps 4–5 work only from the diff. The four lines in STEP 9 are the single verdict downstream automation parses — emit them exactly.
 
 Cite the specific rule or doc section (CLAUDE.md, the code-standards skill, `docs/code-style-guidelines.md`, or the relevant subsystem doc) whenever you flag a violation.
 
 --- STEP 1 — Load context & set scope ---
 - Read `CLAUDE.md` and `docs/README.md`; load the relevant subsystem doc(s) for the diff.
 - Get the diff (`gh pr diff`) and the changed-file list.
-- The PR head is checked out in the working tree and you have `Read`, `Glob`, and `Grep` (ripgrep) over the whole repo — use them. `Grep` is how you run the repo searches later steps require: lifecycle owners in Step 3, leak-opener mirrors in Step 5. Raw shell is limited to the listed `gh` commands, so search with the `Grep` tool, not `Bash(rg)`/`Bash(grep)`.
+- Clone the repo and check out the PR head into a temporary directory (`gh repo clone {owner}/{repo} <tmp-dir> -- --depth 1`, then `gh pr checkout {number}` inside it) so you can read any file in full context. Use ripgrep (`rg`) over that tree for the repo searches later steps require: lifecycle owners in Step 3, leak-opener mirrors in Step 5.
 - Don't review the diff in isolation: open the systems, facades, caches, and lifecycle owners it touches, plus the neighbouring files in the same folder. Steps 2–3 judge whether the change is built in the *right place*, which the diff alone can't show.
 
 --- STEP 2 — Root-cause check ---
@@ -56,7 +56,7 @@ Report ONLY issues that require fixes. Make two passes over the changed lines.
 4. Performance issues
 5. Missing error handling
 6. Unclear or problematic logic
-7. Resource / subscription leaks — an acquisition without a matching teardown at the corresponding disposal point. Scan for the concrete openers and confirm each has its mirror: `Subscribe(`→`Unsubscribe`, `AddListener(`→`RemoveListener`, `+=` on an event/`Action`→`-=`, a pool `Get(`/`Rent(`→`Release`/`Return`, `new CancellationTokenSource(`→`Cancel`+`Dispose`, a `Connect`/room/handle/`IDisposable` open→`Dispose`. Do this with `Grep`: search each changed `*.cs` file for the openers, then grep the same file/type for the matching mirror, and flag any opener whose mirror is missing.
+7. Resource / subscription leaks — an acquisition without a matching teardown at the corresponding disposal point. Scan for the concrete openers and confirm each has its mirror: `Subscribe(`→`Unsubscribe`, `AddListener(`→`RemoveListener`, `+=` on an event/`Action`→`-=`, a pool `Get(`/`Rent(`→`Release`/`Return`, `new CancellationTokenSource(`→`Cancel`+`Dispose`, a `Connect`/room/handle/`IDisposable` open→`Dispose`. Do this with `rg`: search each changed `*.cs` file for the openers, then grep the same file/type for the matching mirror, and flag any opener whose mirror is missing.
 8. Allocated-but-unconsumed infrastructure — buffers, measurements, events, or caches that are populated/written but never read anywhere in the diff or the codebase.
 9. Detached async for essential work — `.Forget()` or fire-and-forget `UniTaskVoid` that performs setup the feature depends on. Essential async must be awaited inside the relevant lifecycle, not left detached (CLAUDE.md §9).
 10. Nullability-contract violations — assigning `null` or a maybe-null value to a non-nullable declaration, or a defensive null-check against a non-nullable declaration. Both lie about what can be null (CLAUDE.md anti-patterns).
@@ -93,7 +93,7 @@ Report ONLY issues that require fixes. Make two passes over the changed lines.
 - Per-frame expense: a GPU/CPU-heavy op (`Blit`, `Render`, allocation) run every frame for a result that changes only on an event — move it to the event.
 - RAII for native resources: a native handle (`RenderTexture`, `NativeArray`, `ComputeBuffer`, room/connection, `IDisposable`) whose release is a manual `Release()`/`Dispose()` reachable only on the happy path — an early return or exception leaks it. Prefer `using`, or ownership by a type whose own `Dispose()` frees it, so the release cannot be skipped.
 
-**Reporting (Steps 3–5).** For each issue: Location (file and line), Problem (be specific), Fix (exact change needed), Why (brief impact). Use `mcp__github_inline_comment__create_inline_comment` for specific line issues and `Bash(gh pr comment)` for the top-level summary. Do NOT include praise or subjective style opinions. But a violation of a project standard is NOT a "nice-to-have": naming, magic numbers, member ordering, encapsulation, and resource ownership are required by CLAUDE.md / the code-standards skill / `docs/code-style-guidelines.md`. Report them as blocking and cite the rule — do not soften them into optional suggestions or skip them as nitpicks.
+**Reporting (Steps 3–5).** For each issue: Location (file and line), Problem (be specific), Fix (exact change needed), Why (brief impact). Post ONE review via `gh api repos/{owner}/{repo}/pulls/{number}/reviews --method POST` with `"event": "COMMENT"`: the top-level summary goes in the review `body` and each line-level finding goes in the `comments[]` array (with a ```suggestion block per finding). Do not post the summary as a separate issue comment. Do NOT include praise or subjective style opinions. But a violation of a project standard is NOT a "nice-to-have": naming, magic numbers, member ordering, encapsulation, and resource ownership are required by CLAUDE.md / the code-standards skill / `docs/code-style-guidelines.md`. Report them as blocking and cite the rule — do not soften them into optional suggestions or skip them as nitpicks.
 
 --- STEP 6 — Complexity assessment ---
 Classify the PR complexity as SIMPLE or COMPLEX.
@@ -144,13 +144,13 @@ QA_REQUIRED: YES when ANY of the following are true:
 - Changes asset loading, authentication, or navigation flows
 
 --- STEP 8 — Non-blocking warnings ---
-Emit these as warnings in the summary comment. They do NOT cause a FAIL on their own.
+Emit these as warnings in the review body. They do NOT cause a FAIL on their own.
 
 - **Main Scene Modified** — If `Explorer/Assets/Scenes/Main.unity` or its `.meta` appears in the changed files:
   > ⚠️ **Main scene modified** (`Explorer/Assets/Scenes/Main.unity`). This file is rarely changed intentionally — verify this wasn't pushed by mistake.
 
 --- STEP 9 — Verdict ---
-At the very end of your output, emit exactly these four lines (order matters — downstream automation parses them):
+Emit exactly these four lines at the end of the review body you post to GitHub, immediately before the attribution line (order matters — downstream automation parses them):
 REVIEW_RESULT: PASS ✅  (or FAIL ❌)
 COMPLEXITY: SIMPLE  (or COMPLEX)
 COMPLEXITY_REASON: <one sentence citing which subsystem(s) the diff touches>

.github/scripts/report-review-cost.js

@@ -1,4 +1,4 @@
-name: Require Claude Review
+name: Require Jarvis Review
 on:
   workflow_dispatch:
   pull_request:
@@ -30,5 +30,5 @@ jobs:
               sha: context.payload.pull_request.head.sha,
               state: 'pending',
               context: 'Claude Review',
-              description: 'Awaiting Claude review — comment @claude review to trigger'
+              description: 'Awaiting Jarvis review — auto-requested for team PRs, or request decentraland-bot'
             });