From 41658ec2effd109210b24f09e12c6ec4e48c57a2 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Fri, 27 Mar 2026 17:38:09 +0100 Subject: [PATCH] fix: repair corrupted group DMs, prevent ownerId nulling, show empty groups Three fixes for group DM data integrity and display: 1. processOwnershipTransferEvent: use resolveOrCreateReplicatedUser instead of resolveLocalUser to guarantee a valid ownerId. The previous ?? null fallback converted group DMs into 1-on-1s when resolution failed. 2. Self-healing migration: detect group DMs with UUID-format federated_id but NULL owner_id (corrupted by the old fallback) and restore owner from the first remaining member. Found and repaired 7 across both instances. 3. Sidebar: group DMs with 0 other members (last person standing) now show as "Empty Group" instead of being hidden. 1-on-1 DMs with 0 others are still correctly filtered out. --- packages/server/src/db/migrate.ts | 31 +++++++++++++++++++ packages/server/src/routes/federation.ts | 18 +++++++---- .../src/components/layout/ChannelSidebar.tsx | 6 ++-- .../web/src/components/layout/DmSearchBar.tsx | 6 ++-- 4 files changed, 51 insertions(+), 10 deletions(-) diff --git a/packages/server/src/db/migrate.ts b/packages/server/src/db/migrate.ts index edf1bfa4..44027e1f 100644 --- a/packages/server/src/db/migrate.ts +++ b/packages/server/src/db/migrate.ts @@ -635,6 +635,37 @@ export function runMigrations(db: Database.Database): void { db.exec(`ALTER TABLE federation_peers ADD COLUMN remote_max_upload_size INTEGER`); } catch { /* column already exists */ } + // ─── Data integrity: repair group DMs with nulled-out owner_id ─────────── + // A bug in processOwnershipTransferEvent (fixed in cd7aff0) could set + // owner_id to NULL when resolveLocalUser failed, converting a group DM into + // a 1-on-1-looking channel. Detect these by finding dm_channels with a + // UUID-format federated_id (group DMs) but NULL owner_id, and restore the + // owner from the first remaining member. + const corruptedGroups = db.prepare(` + SELECT c.id, c.federated_id + FROM dm_channels c + WHERE c.owner_id IS NULL + AND c.federated_id IS NOT NULL + AND c.deleted_at IS NULL + AND length(c.federated_id) = 36 + AND c.federated_id LIKE '________-____-____-____-____________' + `).all() as Array<{ id: string; federated_id: string }>; + + for (const ch of corruptedGroups) { + const firstMember = db.prepare( + `SELECT user_id FROM dm_members WHERE dm_channel_id = ? LIMIT 1` + ).get(ch.id) as { user_id: string } | undefined; + + if (firstMember) { + db.prepare(`UPDATE dm_channels SET owner_id = ? WHERE id = ?`).run(firstMember.user_id, ch.id); + console.log(`[migration] Repaired group DM ${ch.id}: restored owner_id to ${firstMember.user_id}`); + } + } + + if (corruptedGroups.length > 0) { + console.log(`[migration] Repaired ${corruptedGroups.length} corrupted group DM(s).`); + } + console.log('Migrations complete.'); } diff --git a/packages/server/src/routes/federation.ts b/packages/server/src/routes/federation.ts index 32117cd9..25f00364 100644 --- a/packages/server/src/routes/federation.ts +++ b/packages/server/src/routes/federation.ts @@ -1967,12 +1967,18 @@ function processOwnershipTransferEvent( return; } - // Resolve new owner to local user (for local ownerId) - const newOwnerLocal = resolveLocalUser(event.ownership.newOwner.homeUserId, db); + // Resolve new owner to local user — use resolveOrCreateReplicatedUser to + // guarantee we always get a valid user ID. Never fall back to null, as that + // would convert the group DM into a 1-on-1 and destroy its type identity. + const newOwnerLocal = resolveOrCreateReplicatedUser( + event.ownership.newOwner.homeUserId, + event.ownership.newOwner.homeInstance, + db, + ); db.update(schema.dmChannels) .set({ - ownerId: newOwnerLocal?.id ?? null, + ownerId: newOwnerLocal.id, ownerHomeUserId: event.ownership.newOwner.homeUserId, ownerHomeInstance: event.ownership.newOwner.homeInstance, }) @@ -1982,7 +1988,7 @@ function processOwnershipTransferEvent( connectionManager.sendToDmMembers(channel.id, { type: 'dm_owner_updated', dmChannelId: channel.id, - newOwnerId: newOwnerLocal?.id ?? event.ownership.newOwner.homeUserId, + newOwnerId: newOwnerLocal.id, }); const prevOwnerLocal = event.ownership.previousOwner @@ -1998,8 +2004,8 @@ function processOwnershipTransferEvent( userId: prevOwnerId, content: JSON.stringify({ event: 'owner_changed', - newOwnerId: newOwnerLocal?.id ?? event.ownership.newOwner.homeUserId, - newOwnerDisplayName: newOwnerLocal?.displayName ?? newOwnerBaseName, + newOwnerId: newOwnerLocal.id, + newOwnerDisplayName: newOwnerLocal.displayName ?? newOwnerBaseName, }), type: 'system', createdAt: Date.now(), diff --git a/packages/web/src/components/layout/ChannelSidebar.tsx b/packages/web/src/components/layout/ChannelSidebar.tsx index afb74de5..9f0b9cfb 100644 --- a/packages/web/src/components/layout/ChannelSidebar.tsx +++ b/packages/web/src/components/layout/ChannelSidebar.tsx @@ -489,14 +489,16 @@ export function ChannelSidebar() {
{dmChannels.map((dm) => { const otherMembers = dm.members.filter(m => !isSelf(m, user)); - if (otherMembers.length === 0) return null; const isGroup = !!dm.ownerId; + if (otherMembers.length === 0 && !isGroup) return null; const isDmUnread = unreadChannels.has(dm.id) && currentChannelId !== dm.id; const firstOtherDm = isGroup ? null : otherMembers[0]; const { baseName: dmBaseName, domain: dmDomain } = parseFederatedUsername(firstOtherDm?.username ?? ''); const dmDisplayName = isGroup - ? otherMembers.map(m => m.displayName ?? parseFederatedUsername(m.username).baseName).join(', ') + ? (otherMembers.length > 0 + ? otherMembers.map(m => m.displayName ?? parseFederatedUsername(m.username).baseName).join(', ') + : 'Empty Group') : firstOtherDm?.displayName ?? dmBaseName; const dmItem = ( diff --git a/packages/web/src/components/layout/DmSearchBar.tsx b/packages/web/src/components/layout/DmSearchBar.tsx index 145e501b..56b6c4f0 100644 --- a/packages/web/src/components/layout/DmSearchBar.tsx +++ b/packages/web/src/components/layout/DmSearchBar.tsx @@ -59,10 +59,12 @@ export function DmSearchBar() { return dmChannels .map((dm): DmItem | null => { const otherMembers = dm.members.filter(m => !isSelf(m, user)); - if (otherMembers.length === 0) return null; const isGroup = !!dm.ownerId; + if (otherMembers.length === 0 && !isGroup) return null; const displayName = isGroup - ? otherMembers.map(m => m.displayName ?? parseFederatedUsername(m.username).baseName).join(', ') + ? (otherMembers.length > 0 + ? otherMembers.map(m => m.displayName ?? parseFederatedUsername(m.username).baseName).join(', ') + : 'Empty Group') : otherMembers[0]?.displayName ?? otherMembers[0]?.username ?? ''; return { type: 'dm', dm, displayName, otherMembers, isGroup }; })