docs: bugsweep learnings from week 2026-W34

[decentraland-bot]

Learnings from the week 2026-W34 bugsweep — decentraland/unity-explorer#9908 (merged). Distilled from human review comments and the code-review commits humans pushed on top of the bot's fixes. Opened for review like any other PR — please correct or reject anything that doesn't hold.

Lesson 1 — Local scene development must not connect the chat (DM) room

Landed in: .claude/skills/chat-system/SKILL.md (new Rooms & local scene development section)

In local scene development the Friends subsystem is disabled, so DMs delivered over the LiveKit chat room are unanswerable. The bot's fix (#9832) connected the room and then dropped inbound DMs downstream; the reviewer asked to fix it at the source instead.

"Drop it, drop connection to either global room or the livekit chat room as a whole" — reviewer directive on #9832

The human rework reverted the bot's DM filter and passed IConnectiveRoom.Null.INSTANCE for the chat room in CommsContainer (mirroring the island-room gate), plus an early-return in PrivateConversationUserStateService (commit b0eabda0).

Lesson 2 — A transient fetch failure is not a permanent capability downgrade

Landed in: CLAUDE.md §11 (Anti-Patterns)

The bot's fix (#9839) set IsSmart = false when a smart wearable's scene.json was missing. The reviewer asked to keep the DTO-declared flag and evict instead.

"keep item.IsSmart as true but log the issue" — reviewer directive on #9839

The human rework (commit 83d28b0f) replaced item.IsSmart = false with cache.Remove(id), keeping the wearable smart, logging with the id, and evicting the poisoned entry for retry.

---

Considered and dropped

• "Move magic number -32601 to a const" (DCL.Web3.Web3Exception: The method is not allowed: wallet_switchEthereumChain #9833, applied in commit edad8bdd). A real, human-applied correction, but the bugsweep quality bar already states "named constants, never magic numbers" — re-stating it here would duplicate existing guidance; it was a one-off miss, not a gap.
• "Let's postfix it with Event" (inline comment on AnalyticsDiskFullDetected.cs). A single reviewer preference that was not applied (the file kept its name) and has no supporting convention in the EventBus folder — not a durable rule.
• Bot (Jarvis) P2 findings (nullability-contract mismatch, null-forgiving-operator comment). Bot-only, and both already cite existing CLAUDE.md rules — no new guidance.

Requested by Alejandro Jimenez via Slack.

[github-actions]

🚦 CI Status

Build skipped — no changes detected under Explorer/.

Waiting for lint to start…

Waiting for tests to start…

Bare-metal benchmarks run automatically after each successful build; results land in this section. Add the perf_test label to run the in-repo Unity performance suite instead (skips normal CI and blocks merge while set).

On demand — comment /visual-tests on this PR to run the visual regression suite against its build.

[github-actions]

Slack notification sent to #explorer-ext-contributions for external review.
To re-send, delete this comment and re-add the ext-contribution label.

[decentraland-bot]

Review — docs: bugsweep learnings from week 2026-W34

STEP 2 — Root-cause check

N/A — this is a documentation-only PR capturing distilled learnings from the 2026-W34 bugsweep (#9908). It does not fix a bug or modify runtime behavior.

STEP 3 — Design & integration

N/A — no new code units, systems, or runtime logic introduced. Both changes are additive documentation:

1. .claude/skills/chat-system/SKILL.md — New "Rooms & local scene development" section placed between "Message Bus Decorator Chain" and "Chat Command Pattern." Logical placement: the preceding section describes the LiveKit transport layer, and the new section explains when that transport must not be connected. All referenced symbols verified on dev:

   • CommsContainer.Create — exists
   • IConnectiveRoom.Null.INSTANCE — exists
   • PrivateConversationUserStateService.InitializeAsync — exists
   • FeatureId.LocalSceneDevelopment — exists

2. CLAUDE.md §11 Anti-Patterns — New bullet "Encoding a transient fetch failure as a permanent capability downgrade" appended at the end of the list. No duplication with existing bullets. SmartWearableCache reference verified on dev.

STEP 4 — Member audit

N/A — no public properties or accessors added/changed.

STEP 5 — Line-level review

No issues found. Both additions:

• Follow the established formatting conventions (bold pattern name, concrete example, prescriptive fix)
• Reference actual code entities that exist on dev
• Are non-duplicative — the PR body documents three candidate lessons that were correctly dropped to avoid restating existing guidance
• Frame principles durably ("gate the connection at the source", "evict the half-built cache entry") rather than coupling to specific implementation details

Security review

No security issues. Pure documentation changes — no secrets, credentials, executable code, or sensitive data.

STEP 6 — Complexity

SIMPLE — 2 markdown files, 7 additions, 0 deletions, documentation only.

STEP 7 — QA assessment

NO — changes are limited to .claude/skills/ and CLAUDE.md documentation. No runtime code is modified.

STEP 8 — Non-blocking warnings

None. Main.unity is not in the changeset.

STEP 9 — Verdict

REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Documentation-only changes to two markdown guidance files, no runtime code touched.
QA_REQUIRED: NO

---

Reviewed by Jarvis 🤖 · Requested via Slack (auto-triggered by decentraland-bot)