Skip to content

feat(web): room social layer - broadcast reactions + in-room chat#67

Merged
knzeng-e merged 3 commits into
mainfrom
feat/room-social-layer
Jul 6, 2026
Merged

feat(web): room social layer - broadcast reactions + in-room chat#67
knzeng-e merged 3 commits into
mainfrom
feat/room-social-layer

Conversation

@knzeng-e

@knzeng-e knzeng-e commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Reactions and chat now ride the signaling relay, closing the chatter gap named by ticket 15 (Option A) as ticket 20.

  • room:reaction / room:chat events: server validates (curated emoji allowlist, single-line 280-char sanitization), rate-limits per socket (fail closed, silent drops), and echoes to the whole room - the echo is the single render path
  • last-50 chat ring buffer per room, replayed to late joiners via the room:join ack; in-memory only, wiped with the room, never on /status
  • attributed reaction petals: sender initials + name-hashed hue over the cover (Constellation honesty rule); the phase C CSS had already hidden the old local-only react bar, which is removed
  • new RoomChat aside (messages, curated reaction row, input) mounted in the room-mode lower grid; session state in useSession, transport- agnostic message shapes in shared/types.ts for a future Statement Store presence layer
  • 7 new signaling integration tests; socket-events reference updated

Reactions and chat now ride the signaling relay, closing the chatter gap
named by ticket 15 (Option A) as ticket 20.

- room:reaction / room:chat events: server validates (curated emoji
  allowlist, single-line 280-char sanitization), rate-limits per socket
  (fail closed, silent drops), and echoes to the whole room - the echo is
  the single render path
- last-50 chat ring buffer per room, replayed to late joiners via the
  room:join ack; in-memory only, wiped with the room, never on /status
- attributed reaction petals: sender initials + name-hashed hue over the
  cover (Constellation honesty rule); the phase C CSS had already hidden
  the old local-only react bar, which is removed
- new RoomChat aside (messages, curated reaction row, input) mounted in
  the room-mode lower grid; session state in useSession, transport-
  agnostic message shapes in shared/types.ts for a future Statement
  Store presence layer
- 7 new signaling integration tests; socket-events reference updated

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@knzeng-e knzeng-e self-assigned this Jul 5, 2026
@netlify

netlify Bot commented Jul 5, 2026

Copy link
Copy Markdown

Deploy Preview for muzinga ready!

Name Link
🔨 Latest commit e067d40
🔍 Latest deploy log https://app.netlify.com/projects/muzinga/deploys/6a4b6728763c390008121ae4
😎 Deploy Preview https://deploy-preview-67--muzinga.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@knzeng-e knzeng-e added the enhancement New feature or request label Jul 5, 2026
@knzeng-e

knzeng-e commented Jul 5, 2026

Copy link
Copy Markdown
Owner Author

Hey @claude, review this please

@claude

claude Bot commented Jul 5, 2026

Copy link
Copy Markdown

Claude finished @knzeng-e's task in 3m 11s —— View job


Review

  • Gather context (PR diff, backlog ticket 20, CLAUDE.md)
  • Review server-side signaling changes (signaling.mjs, signaling-utils.mjs)
  • Review tests (signaling.test.mjs)
  • Review frontend changes (RoomChat.tsx, useSession.ts, shared/types.ts, shared/social.ts, PlayerView.tsx)
  • Review docs (socket-events.md, backlog ticket)
  • Post review feedback

This is a clean, well-scoped implementation of ticket 20. Server-side echo as the single render path, curated emoji allowlist, sanitize-then-cap chat, in-memory-only history that never touches /status — all matches the doctrine in docs/backlog/20-room-social-layer.md and is backed by 7 solid integration tests covering broadcast, allowlist enforcement, sanitization, history cap, rate limiting, participant verification, and /status privacy. No wallet addresses leak into social payloads, no curly quotes introduced. docs/reference/socket-events.md accurately documents the new events.

One thing worth a look, one minor nit:

1. Rate limits are trivially bypassable by reconnecting (web/server/signaling.mjs:461-462, web/server/signaling-utils.mjs:85-104)
chatLimiter/reactionLimiter are keyed by socket.id, and clearSocketRoom wipes both buckets on every disconnect/leave (web/server/signaling.mjs:458-463). A scripted client can flood past the 5-message/5s or 10-reaction/5s window simply by disconnecting and rejoining the room with a fresh socket — room:join itself isn't rate-limited, and rejoining doesn't push the listener over maxListenersPerRoom since the old slot is freed first. Given the "fail closed" framing in the ticket, it's worth deciding whether this is an accepted MVP tradeoff or whether the limiter should key on something more durable (e.g. display name + room, or a short per-IP cooldown on room:join). Not a blocker for a demo-scale room, but worth a note in the ticket's constraints if intentional.
Fix this →

2. Duplicated initials logic(reaction.senderName || '?').slice(0, 2).toUpperCase() in web/src/views/PlayerView.tsx:265 duplicates the exact same line already in web/src/components/Presence.tsx:23 (Avatar's initials). Minor; could import a shared initialsFor(name) helper instead, purely cosmetic.

Everything else checked out: sanitizeChatText/sanitizeReactionEmoji are correctly fail-closed, getParticipant correctly re-verifies role/room membership per event (no spoofing via stale roomId), the chat ring buffer math (push + shift capped at chatHistoryLimit) is correct, and the client-side useSession/RoomChat/PlayerView wiring only ever renders server-echoed state (no optimistic-UI divergence), matching the "honesty rule" called out in the ticket.

Note: I was not able to run npm run typecheck/lint/build in this sandbox (no node_modules installed and shell commands required approval I couldn't obtain in this automated context) — this was a static read-through review only. Worth confirming CI is green before merge.

@knzeng-e
knzeng-e merged commit e1e713d into main Jul 6, 2026
9 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Dotify sprints Jul 6, 2026
@knzeng-e
knzeng-e deleted the feat/room-social-layer branch July 6, 2026 08:32
knzeng-e added a commit that referenced this pull request Jul 6, 2026
Re-checked the backlog tables against issue/PR state and merged work:

- Ticket 08 (frontend refactor, #9): "In progress" -> "Delivered on main".
  Issue #9 is closed and the refactor PRs (8b-1..8b-5, PR9 shared/ tree,
  PR10-13) are merged.
- improvement-plan Now table: PR8b and PR9 "Planned" -> "Delivered on main"
  (PR10 split-mega-hooks stays Proposed; useCatalog/useSession are not split
  yet).
- Ticket 20 (social layer): "Delivered on feat/room-social-layer" ->
  "Delivered on main (PR #67)" now that #67 is merged.
- Ticket 21 (this PR): now tracked by a dedicated issue #69; row reads
  "In review (issue #69, PR #68)".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant