From a6d82b5252ec6254c2bd243a7766ccb2505eb9b8 Mon Sep 17 00:00:00 2001
From: decentraland-bot <44584806+decentraland-bot@users.noreply.github.com>
Date: Mon, 7 Sep 2026 08:56:11 +0000
Subject: [PATCH] docs: bugsweep learnings from week 2026-W34
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Learnings distilled from the review of the week 2026-W34 bugsweep PR
(decentraland/unity-explorer#9908), routed to the code-context files.

- chat-system skill: in local scene development the LiveKit chat room
  (DM transport) must not be connected — gate at the source in
  CommsContainer (IConnectiveRoom.Null.INSTANCE, mirroring the island
  room) and early-return in PrivateConversationUserStateService, not by
  filtering DMs downstream. Evidence: reviewer directive on #9832 ("drop
  connection to ... the livekit chat room as a whole") and the human
  rework that reverted the bot's DM filter (commit b0eabda0).

- CLAUDE.md §11: a transient asset/metadata fetch failure must not be
  encoded as a permanent capability downgrade — keep the DTO-declared
  flag, log, and evict the poisoned cache entry for retry. Evidence:
  reviewer directive on #9839 ("keep item.IsSmart as true but log the
  issue") and the human rework (commit 83d28b0f) that replaced
  IsSmart=false with cache.Remove(id).
---
 .claude/skills/chat-system/SKILL.md | 6 ++++++
 CLAUDE.md                           | 1 +
 2 files changed, 7 insertions(+)

diff --git a/.claude/skills/chat-system/SKILL.md b/.claude/skills/chat-system/SKILL.md
index ec3b0027db8..9444fe6ebb5 100644
--- a/.claude/skills/chat-system/SKILL.md
+++ b/.claude/skills/chat-system/SKILL.md
@@ -75,6 +75,12 @@ Each decorator wraps `origin.Send()` and forwards `origin.MessageAdded` events,
 
 ---
 
+## Rooms & local scene development
+
+Comms rooms are wired in `CommsContainer.Create`. In **local scene development** the Friends subsystem is disabled, so the LiveKit **chat room** (the transport that carries DMs) must not be connected at all: `CommsContainer` passes `IConnectiveRoom.Null.INSTANCE` for the chat room in that mode, exactly as it already does for the archipelago island room. Gate the connection **at the source** — do not connect the room and then drop inbound DMs downstream, which leaves an unanswerable conversation half-alive. `PrivateConversationUserStateService.InitializeAsync` must also early-return when `FeatureId.LocalSceneDevelopment` is enabled, because waiting on a room that never connects only burns the timeout and logs a spurious error.
+
+---
+
 ## Chat Command Pattern
 
 ### Interface
diff --git a/CLAUDE.md b/CLAUDE.md
index 860b658407e..7685a3894b8 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -144,6 +144,7 @@ Reviewers have repeatedly identified AI-generated code by these smells. Check yo
 * **Reimplementing primitives that already exist.** Before writing manual atlas UV math, check `TMP_Sprite Asset`. Before hand-batching profile lookups, check the batched `GetProfilesAsync(IReadOnlyList<string>, ct)` overload. Before adding a bespoke event pathway, check `ViewEventBus` / `ChatEvents`.
 * **Comments that narrate caller/external behavior.** A comment must state only what the annotated code itself does or guarantees ("remove the corrupt file so the next read doesn't hit it"), never what callers or upper layers will do with the result ("so callers treat it as a miss and re-download"). External behavior can change without this code changing, silently turning the comment into a lie.
 * **Suppressing `CheckNamespace` with a ReSharper comment.** Never add `// ReSharper disable once CheckNamespace` (or the file-wide variant) — fix the namespace or leave the warning visible. Rationale and full rule: [`docs/code-style-guidelines.md` § Namespaces](docs/code-style-guidelines.md#namespaces).
+* **Encoding a transient fetch failure as a permanent capability downgrade.** When an asset or metadata fetch fails, don't flip a flag that the item's authoritative DTO owns — e.g. setting `IsSmart = false` on a smart wearable whose `scene.json` was missing turns a recoverable content miss into a wrong, sticky classification. Keep the DTO-declared flag, log the failure with the item id, and **evict the half-built cache entry** (`cache.Remove(id)`) so the next request re-fetches it instead of being served the poisoned entry. (`SmartWearableCache` was corrected this way: keep `IsSmart` true, log, and evict rather than downgrade.)
 
 ### Other project-specific rules