From 1ed70a90b18c17c4672f97686e8a7c5f624e4ffa Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Wed, 29 Apr 2026 23:13:09 +0200 Subject: [PATCH] fix(dm): render system messages in sidebar preview instead of raw JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DmLastMessagePreview lacked a `type` field, so the sidebar rendered `lastMessage.content` verbatim — surfacing JSON like `{"event":"space_invite",...}` for space invites and member-add events. Adds `type` to the preview payload (populated server-side from `dm_messages.type`) and routes all sidebar call sites through a single `formatDmSidebarPreview` helper that renders human-readable text for each system event and skips the group `Sender:` prefix on system rows. --- docs/systems/dm-system.md | 22 ++++ packages/server/src/routes/dm.ts | 6 +- packages/server/src/ws/handler.ts | 1 + packages/shared/src/types.ts | 6 + .../web/src/components/layout/DmListItem.tsx | 20 +-- .../src/components/layout/MobileDmsScreen.tsx | 12 +- packages/web/src/utils/dmFormatters.ts | 120 +++++++++++++++++- 7 files changed, 164 insertions(+), 23 deletions(-) diff --git a/docs/systems/dm-system.md b/docs/systems/dm-system.md index 48b04fd0..f52e1c19 100644 --- a/docs/systems/dm-system.md +++ b/docs/systems/dm-system.md @@ -530,6 +530,27 @@ JSON content shape: The `spaceInstanceOrigin` is the space's home instance, **not** the sender's. The recipient's client uses it to fetch the live preview (`getApiForOrigin(spaceInstanceOrigin).spaces.invitePreview`) and to call `joinByCode(code, spaceInstanceOrigin)` on click. +### Sidebar Preview Rendering + +System messages MUST NOT surface their raw JSON `content` in the DM sidebar preview. The sidebar uses the `type` field on the `lastMessage` payload (`'user' | 'system'`) to dispatch: + +| Event | Sidebar preview | +|-------|-----------------| +| `space_invite` | `📨 Sent invite to {snapshot.spaceName}` (or `📨 Sent a space invite` if name missing) | +| `member_added` | `{actorName} added {targetDisplayName}` | +| `member_removed` (`reason='leave'`) | `{targetDisplayName} left the group` | +| `member_removed` (kick) | `{actorName} removed {targetDisplayName}` | +| `owner_changed` | `{newOwnerDisplayName} is now the group owner` | +| Unknown / malformed JSON | `System message` | + +`actorName` is resolved from the channel `members` roster by `lastMessage.userId`, falling back to the embedded `user` object on `DmMessageWithUser` payloads (used for federation bootstrap). System messages are NEVER prefixed with `${sender}: ` in group DMs — the rendered text already incorporates the actor. + +User messages keep the existing behavior: text/attachment formatting via `formatDmPreview`, with a `${senderDisplayName}: ` prefix in group DMs when the author is not the current user. + +The single source of truth on the client is `packages/web/src/utils/dmFormatters.ts:formatDmSidebarPreview(dm, currentUser)`. All call sites (`DmListItem`, `MobileDmsScreen`) MUST use it — never read `lastMessage.content` directly. + +**Server contract:** Every code path that emits a `DmLastMessagePreview` (REST `GET/POST /api/dm`, `POST /api/dm/:id/members`, WS `ready` payload, `dm_channel_created` reopen) MUST include the `type` field copied from the `dm_messages.type` column. Without this, the client cannot distinguish system from user messages and falls back to rendering raw JSON. + ### Instance-Local Creation System messages are NOT relayed via federation. Each instance creates its own independently: @@ -725,3 +746,4 @@ For full wire formats, see `docs/systems/websocket.md`. | Cross-instance duplicate channels | Duplicate sidebar entries | `dm_channel_created` broadcast to ALL members including remote | Local-only broadcast principle | | Bootstrap vs incremental confusion | N/A (design note) | `bootstrapped` flag is function-local; batch events work correctly because bootstrap adds ALL roster members | No fix needed -- documented as correct behavior | | Duplicated membership system messages across restarts | 4× "Jannis added youruser" in group DM, channel keeps flipping to unread after each deploy | Membership event processors inserted system messages unconditionally. Each approval-flow re-peering reset peer `last_synced_at = 0`, so initial sync replayed every historical `member_add` / `member_remove` / `ownership_transfer` on next boot. Each replay's new snowflake ID exceeded the user's `read_states.last_read_message_id`, flipping unread. | Dedup by `(sourceInstance, event.messageId)` on the inserted system message. Both bootstrap and incremental paths in `processMemberAddEvent` now persist these fields so replay is a no-op. | +| Raw JSON in DM sidebar previews | DM sidebar showed `{"event":"space_invite",...}` / `{"event":"member_added",...}` as the last-message preview | `DmLastMessagePreview` shape omitted `type`, so the client could not distinguish system from user messages and rendered `lastMessage.content` verbatim. | Added `type` to `DmLastMessagePreview`, populated it from `dm_messages.type` in every server emission site, and routed the sidebar through a single `formatDmSidebarPreview` helper that renders human-readable text for each system event. | diff --git a/packages/server/src/routes/dm.ts b/packages/server/src/routes/dm.ts index 75ef571b..6661d7b0 100644 --- a/packages/server/src/routes/dm.ts +++ b/packages/server/src/routes/dm.ts @@ -439,7 +439,7 @@ export async function dmRoutes(app: FastifyInstance): Promise { .groupBy(schema.dmMessages.dmChannelId) .all(); - const lastMessageMap = new Map(); + const lastMessageMap = new Map(); if (maxTimestamps.length > 0) { // Build conditions to fetch the actual message rows matching max timestamps const conditions = maxTimestamps.map(t => @@ -455,6 +455,7 @@ export async function dmRoutes(app: FastifyInstance): Promise { lastMessageMap.set(m.dmChannelId, { id: m.id, dmChannelId: m.dmChannelId, userId: m.userId, content: m.content, createdAt: m.createdAt, + type: m.type === 'system' ? 'system' : 'user', }); } } @@ -502,6 +503,7 @@ export async function dmRoutes(app: FastifyInstance): Promise { userId: lastMsg.userId, content: lastMsg.content, createdAt: lastMsg.createdAt, + type: lastMsg.type, attachments: lastMsgAttachmentMap.get(lastMsg.id) ?? [], } : null, }); @@ -621,6 +623,7 @@ export async function dmRoutes(app: FastifyInstance): Promise { userId: lastMsg.userId, content: lastMsg.content, createdAt: lastMsg.createdAt, + type: lastMsg.type === 'system' ? 'system' : 'user', } : null, }; @@ -1145,6 +1148,7 @@ export async function dmRoutes(app: FastifyInstance): Promise { userId: lastMsg.userId, content: lastMsg.content, createdAt: lastMsg.createdAt, + type: lastMsg.type === 'system' ? 'system' : 'user', } : null, }; diff --git a/packages/server/src/ws/handler.ts b/packages/server/src/ws/handler.ts index 40ccd08e..c730af2e 100644 --- a/packages/server/src/ws/handler.ts +++ b/packages/server/src/ws/handler.ts @@ -1328,6 +1328,7 @@ function buildReadyPayload(userId: string): { userId: last.userId, content: last.content, createdAt: last.createdAt, + type: last.type === 'system' ? 'system' : 'user', attachments: dmLastMsgAttachmentMap.get(last.id) ?? [], } : null, }); diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 5bd3be90..060d7e22 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -294,6 +294,12 @@ export interface DmLastMessagePreview { userId: string; content: string | null; createdAt: number; + /** + * 'system' for membership/lifecycle JSON payloads (member_added, member_removed, + * owner_changed, space_invite). 'user' (or omitted) for normal user-authored + * messages. The sidebar renderer relies on this to avoid showing raw JSON. + */ + type?: 'user' | 'system'; attachments?: Array<{ type: string; filename: string }>; } diff --git a/packages/web/src/components/layout/DmListItem.tsx b/packages/web/src/components/layout/DmListItem.tsx index e67e3c5b..aa1d6764 100644 --- a/packages/web/src/components/layout/DmListItem.tsx +++ b/packages/web/src/components/layout/DmListItem.tsx @@ -2,7 +2,7 @@ import type { DmChannel, User } from '@backspace/shared'; import { Avatar } from '../ui/Avatar'; import { Tooltip } from '../ui/Tooltip'; import { parseFederatedUsername, isSelf } from '../../utils/identity'; -import { formatDmTimestamp, formatDmPreview } from '../../utils/dmFormatters'; +import { formatDmTimestamp, formatDmSidebarPreview } from '../../utils/dmFormatters'; import { getRejectedPeerOrigins, getAwaitingApprovalPeerOrigins } from '../../hooks/useWebSocket'; function isMemberUnreachable(homeInstance: string | null | undefined): boolean { @@ -98,19 +98,11 @@ export function DmListItem({ dm, isActive, isUnread, user, onSelect, onClose, on } text-txt-tertiary hover:text-txt-primary transition-opacity flex-shrink-0 ml-1`; // ── Preview text ────────────────────────────────────────────────────── - const preview = formatDmPreview(dm.lastMessage ?? null); - let previewText: string | null = null; - if (isGroup) { - const lastMsg = dm.lastMessage; - const senderName = (lastMsg && 'user' in lastMsg ? lastMsg.user?.displayName : undefined) - ?? dm.members.find(m => m.id === lastMsg?.userId)?.displayName - ?? 'Unknown'; - previewText = preview - ? `${senderName}: ${preview}` - : `${dm.members.length} Members`; - } else { - previewText = preview; - } + // formatDmSidebarPreview handles user/system messages and applies the + // sender prefix for group user-messages. We only need to provide the + // empty-group fallback ourselves. + const preview = formatDmSidebarPreview(dm, user); + const previewText = preview ?? (isGroup ? `${dm.members.length} Members` : null); const itemJsx = (
s.pushMobileScreen); @@ -151,11 +152,10 @@ export function MobileDmsScreen() { const readState = readStates.get(dm.id); const isUnread = lastMsgId && (!readState || readState < lastMsgId); - const preview = dm.lastMessage?.content; + // formatDmSidebarPreview returns the full preview line (system messages + // get human-readable text; group user-messages get the "Sender: " prefix). + const preview = formatDmSidebarPreview(dm, authUser ?? null); const previewTime = dm.lastMessage?.createdAt; - const previewSender = dm.lastMessage - ? dm.members.find(m => m.id === dm.lastMessage!.userId) - : null; const mainUser = otherMembers[0]; const avatarUrl = mainUser?.avatar @@ -206,9 +206,7 @@ export function MobileDmsScreen() { })()} {preview && (

- {previewSender && previewSender.id !== authUser?.id - ? `${previewSender.displayName ?? parseFederatedUsername(previewSender.username).baseName}: ${preview}` - : preview} + {preview}

)}
diff --git a/packages/web/src/utils/dmFormatters.ts b/packages/web/src/utils/dmFormatters.ts index 7fa3405f..58234a37 100644 --- a/packages/web/src/utils/dmFormatters.ts +++ b/packages/web/src/utils/dmFormatters.ts @@ -1,3 +1,6 @@ +import type { DmChannel, DmMessageWithUser, DmLastMessagePreview, User, SpaceInviteSystemPayload } from '@backspace/shared'; +import { parseFederatedUsername, isSelf } from './identity'; + // ─── DM Preview Formatting ──────────────────────────────────────────────────── /** @@ -50,7 +53,7 @@ function getUnifiedAttachmentIcon(attachments: PreviewAttachment[]): string { } /** - * Format a DM lastMessage into a sidebar preview string. + * Format a DM lastMessage's user-authored content into a sidebar preview string. * Returns null if there is nothing displayable. * * Handles: @@ -58,6 +61,10 @@ function getUnifiedAttachmentIcon(attachments: PreviewAttachment[]): string { * - Attachment only (single) → "📷 Image", "🎬 Video", "🎵 Audio", "📎 filename.ext" * - Attachment only (multiple) → "📎 N files" * - Text + attachments → "text 📷" (appends unified icon) + * + * NOTE: This helper does NOT understand system messages — for those, use + * `formatDmSidebarPreview` which inspects `type` and routes to a system-message + * renderer. Calling this directly on a system message would surface raw JSON. */ export function formatDmPreview(lastMessage: PreviewMessage | null | undefined): string | null { if (!lastMessage) return null; @@ -85,6 +92,117 @@ export function formatDmPreview(lastMessage: PreviewMessage | null | undefined): return `${content} ${icon}`; } +// ─── System Message Preview Formatting ─────────────────────────────────────── + +interface SystemEventPayload { + event?: unknown; + targetUserId?: unknown; + targetDisplayName?: unknown; + newOwnerId?: unknown; + newOwnerDisplayName?: unknown; + reason?: unknown; + // space_invite payload + snapshot?: { spaceName?: unknown }; +} + +function asString(v: unknown): string | null { + return typeof v === 'string' && v.length > 0 ? v : null; +} + +function resolveDisplayName(user: User | null | undefined): string { + if (!user) return 'Unknown'; + if (user.displayName) return user.displayName; + return parseFederatedUsername(user.username ?? '').baseName || 'Unknown'; +} + +/** + * Format a system DM message (member_added, member_removed, owner_changed, + * space_invite) into a human-readable sidebar preview. Falls back to a generic + * label for unknown event shapes so we never leak raw JSON to the sidebar. + */ +function formatSystemPreview(content: string | null, actor: User | null | undefined): string { + let data: SystemEventPayload = {}; + if (content) { + try { data = JSON.parse(content) as SystemEventPayload; } catch { /* malformed → generic fallback */ } + } + const event = asString(data.event); + const actorName = resolveDisplayName(actor); + + switch (event) { + case 'space_invite': { + const spaceName = asString((data as Partial).snapshot?.spaceName); + return spaceName + ? `📨 Sent invite to ${spaceName}` + : '📨 Sent a space invite'; + } + case 'member_added': { + const target = asString(data.targetDisplayName) ?? 'someone'; + return `${actorName} added ${target}`; + } + case 'member_removed': { + const target = asString(data.targetDisplayName) ?? 'someone'; + const reason = asString(data.reason); + if (reason === 'leave') return `${target} left the group`; + return `${actorName} removed ${target}`; + } + case 'owner_changed': { + const newOwner = asString(data.newOwnerDisplayName) ?? 'A member'; + return `${newOwner} is now the group owner`; + } + default: + return 'System message'; + } +} + +// ─── Unified Sidebar Preview ───────────────────────────────────────────────── + +type LastMessageLike = DmLastMessagePreview | DmMessageWithUser; + +function isSystemMessage(m: LastMessageLike): boolean { + return m.type === 'system'; +} + +/** + * Produce the full sidebar preview line for a DM channel. Handles: + * - User messages → text/attachment formatting (with `Sender: ` prefix in groups + * when the author is not the current user) + * - System messages → human-readable rendering with no sender prefix (the system + * text already incorporates the actor where appropriate) + * - Empty state → null (caller decides the fallback, e.g. "N Members") + */ +export function formatDmSidebarPreview( + dm: Pick, + currentUser: { id: string; username: string } | null, +): string | null { + const lastMessage = dm.lastMessage ?? null; + if (!lastMessage) return null; + + // Resolve the message author from the channel members. Falls back to the + // user object embedded in DmMessageWithUser if the member roster doesn't + // include them (e.g. a remote actor in federation bootstrap). + const actor: User | null = ( + dm.members.find(m => m.id === lastMessage.userId) + ?? ('user' in lastMessage ? lastMessage.user : null) + ?? null + ); + + if (isSystemMessage(lastMessage)) { + return formatSystemPreview(lastMessage.content ?? null, actor); + } + + const text = formatDmPreview(lastMessage); + if (!text) return null; + + const isGroup = !!dm.ownerId; + if (!isGroup) return text; + + // Group user messages: prefix with sender display name unless it's the current user. + const authoredBySelf = currentUser ? isSelf({ id: lastMessage.userId, username: actor?.username ?? '', homeInstance: actor?.homeInstance ?? null }, currentUser) : false; + if (authoredBySelf) return text; + + return `${resolveDisplayName(actor)}: ${text}`; +} + // ─── DM Timestamp Formatting ───────────────────────────────────────────────── /**