fix: eliminate phantom notifications across the entire read-state pipeline
Root cause: own messages echoed by the server marked channels unread when the user had already navigated away. Seven related bugs compounded the problem — stale read states, missing cleanup on space/DM removal, REST broadcast ignoring VIEW_CHANNEL, and no validation on channel_ack writes. Frontend: - Skip markChannelUnread for the user's own messages (federation-aware) - Walk backward past temp_ IDs in ackChannel instead of bailing - Re-fire ack timer when temp message is replaced by server-confirmed ID - Add removeChannelStates to clean up unread/read/message caches - Clean up chatStore on removeSpace, removeDmChannel, removeInstanceSpaces Server: - Use sendToChannel instead of sendToSpace for REST message creation - Clean up read_states on space deletion, member kick/leave, and ban - Validate channel membership before accepting channel_ack writes - Clean up read_states on DM leave and DM channel deletion
This commit is contained in:
@@ -695,6 +695,12 @@ export async function dmRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
))
|
))
|
||||||
.run();
|
.run();
|
||||||
|
|
||||||
|
// Clean up read_states for the departing user
|
||||||
|
db.delete(schema.readStates).where(and(
|
||||||
|
eq(schema.readStates.userId, request.userId),
|
||||||
|
eq(schema.readStates.channelId, id),
|
||||||
|
)).run();
|
||||||
|
|
||||||
// Check remaining members
|
// Check remaining members
|
||||||
const remainingMembers = db.select()
|
const remainingMembers = db.select()
|
||||||
.from(schema.dmMembers)
|
.from(schema.dmMembers)
|
||||||
@@ -743,6 +749,9 @@ export async function dmRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Clean up all read_states for this DM channel (all members' rows)
|
||||||
|
db.delete(schema.readStates).where(eq(schema.readStates.channelId, id)).run();
|
||||||
|
|
||||||
// Delete the DM channel (cascades to dm_messages)
|
// Delete the DM channel (cascades to dm_messages)
|
||||||
db.delete(schema.dmChannels).where(eq(schema.dmChannels.id, id)).run();
|
db.delete(schema.dmChannels).where(eq(schema.dmChannels.id, id)).run();
|
||||||
|
|
||||||
|
|||||||
@@ -343,7 +343,7 @@ export async function messageRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
const messageWithUser = buildMessageWithUser(message, user, attachmentRows, [], replyTo);
|
const messageWithUser = buildMessageWithUser(message, user, attachmentRows, [], replyTo);
|
||||||
|
|
||||||
// Broadcast via WebSocket
|
// Broadcast via WebSocket
|
||||||
connectionManager.sendToSpace(spaceId, {
|
connectionManager.sendToChannel(spaceId, id, {
|
||||||
type: 'message_created',
|
type: 'message_created',
|
||||||
message: messageWithUser,
|
message: messageWithUser,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -508,8 +508,12 @@ export async function spaceRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
const spaceIcon = server.icon;
|
const spaceIcon = server.icon;
|
||||||
const spaceBanner = server.banner;
|
const spaceBanner = server.banner;
|
||||||
|
|
||||||
// Delete all channels (messages cascade), members, folder refs, then space atomically
|
// Delete all channels (messages cascade), members, folder refs, read states, then space atomically
|
||||||
db.transaction((tx) => {
|
db.transaction((tx) => {
|
||||||
|
// Clean up read_states for all channels in this space (no FK cascade — channelId is plain text)
|
||||||
|
if (channelIds.length > 0) {
|
||||||
|
tx.delete(schema.readStates).where(inArray(schema.readStates.channelId, channelIds)).run();
|
||||||
|
}
|
||||||
tx.delete(schema.channels).where(eq(schema.channels.spaceId, id)).run();
|
tx.delete(schema.channels).where(eq(schema.channels.spaceId, id)).run();
|
||||||
tx.delete(schema.spaceMembers).where(eq(schema.spaceMembers.spaceId, id)).run();
|
tx.delete(schema.spaceMembers).where(eq(schema.spaceMembers.spaceId, id)).run();
|
||||||
tx.delete(schema.spaceFolderMembers).where(eq(schema.spaceFolderMembers.spaceId, id)).run();
|
tx.delete(schema.spaceFolderMembers).where(eq(schema.spaceFolderMembers.spaceId, id)).run();
|
||||||
@@ -941,6 +945,16 @@ export async function spaceRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
)
|
)
|
||||||
).run();
|
).run();
|
||||||
|
|
||||||
|
// Clean up read_states for the departing user in this space's channels
|
||||||
|
const spaceChannelIds = db.select({ id: schema.channels.id })
|
||||||
|
.from(schema.channels).where(eq(schema.channels.spaceId, id)).all().map(c => c.id);
|
||||||
|
if (spaceChannelIds.length > 0) {
|
||||||
|
db.delete(schema.readStates).where(and(
|
||||||
|
eq(schema.readStates.userId, uid),
|
||||||
|
inArray(schema.readStates.channelId, spaceChannelIds),
|
||||||
|
)).run();
|
||||||
|
}
|
||||||
|
|
||||||
// Broadcast member_left event
|
// Broadcast member_left event
|
||||||
connectionManager.sendToSpace(id, {
|
connectionManager.sendToSpace(id, {
|
||||||
type: 'member_left',
|
type: 'member_left',
|
||||||
@@ -1269,6 +1283,10 @@ export async function spaceRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
|
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
|
|
||||||
|
// Fetch channel IDs before the transaction for read_states cleanup
|
||||||
|
const banChannelIds = db.select({ id: schema.channels.id })
|
||||||
|
.from(schema.channels).where(eq(schema.channels.spaceId, id)).all().map(c => c.id);
|
||||||
|
|
||||||
db.transaction((tx) => {
|
db.transaction((tx) => {
|
||||||
// Insert ban record
|
// Insert ban record
|
||||||
tx.insert(schema.bans).values({
|
tx.insert(schema.bans).values({
|
||||||
@@ -1291,6 +1309,14 @@ export async function spaceRoutes(app: FastifyInstance): Promise<void> {
|
|||||||
eq(schema.memberRoles.userId, targetId),
|
eq(schema.memberRoles.userId, targetId),
|
||||||
)).run();
|
)).run();
|
||||||
|
|
||||||
|
// Clean up read_states for the banned user in this space's channels
|
||||||
|
if (banChannelIds.length > 0) {
|
||||||
|
tx.delete(schema.readStates).where(and(
|
||||||
|
eq(schema.readStates.userId, targetId),
|
||||||
|
inArray(schema.readStates.channelId, banChannelIds),
|
||||||
|
)).run();
|
||||||
|
}
|
||||||
|
|
||||||
// Clean up any voice restrictions for the banned member
|
// Clean up any voice restrictions for the banned member
|
||||||
tx.delete(schema.voiceRestrictions).where(and(
|
tx.delete(schema.voiceRestrictions).where(and(
|
||||||
eq(schema.voiceRestrictions.spaceId, id),
|
eq(schema.voiceRestrictions.spaceId, id),
|
||||||
|
|||||||
@@ -1040,6 +1040,14 @@ function handleChannelAck(event: Record<string, unknown>, userId: string): void
|
|||||||
// Validate messageId is a valid snowflake (numeric string) — reject temp/garbage IDs
|
// Validate messageId is a valid snowflake (numeric string) — reject temp/garbage IDs
|
||||||
if (!/^\d+$/.test(messageId)) return;
|
if (!/^\d+$/.test(messageId)) return;
|
||||||
|
|
||||||
|
// Validate channel membership — reject acks for channels the user doesn't belong to
|
||||||
|
const spaceId = getChannelSpaceId(channelId);
|
||||||
|
if (spaceId) {
|
||||||
|
if (!isMember(spaceId, userId)) return;
|
||||||
|
} else {
|
||||||
|
if (!isDmMember(channelId, userId)) return;
|
||||||
|
}
|
||||||
|
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
|
|
||||||
const existing = db.select()
|
const existing = db.select()
|
||||||
|
|||||||
@@ -69,6 +69,9 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess
|
|||||||
}
|
}
|
||||||
}, [channelId, loadMessages, canReadHistory]);
|
}, [channelId, loadMessages, canReadHistory]);
|
||||||
|
|
||||||
|
// Track the last message ID so the ack re-fires when a temp message is replaced by its server-confirmed ID
|
||||||
|
const lastMessageId = messages.length > 0 ? messages[messages.length - 1]?.id ?? '' : '';
|
||||||
|
|
||||||
// Ack channel when messages load or when new messages arrive while near bottom
|
// Ack channel when messages load or when new messages arrive while near bottom
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (messages.length > 0 && isNearBottom) {
|
if (messages.length > 0 && isNearBottom) {
|
||||||
@@ -76,7 +79,7 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess
|
|||||||
ackTimerRef.current = setTimeout(() => ackChannel(channelId), 200);
|
ackTimerRef.current = setTimeout(() => ackChannel(channelId), 200);
|
||||||
}
|
}
|
||||||
return () => clearTimeout(ackTimerRef.current);
|
return () => clearTimeout(ackTimerRef.current);
|
||||||
}, [channelId, messages.length, isNearBottom, ackChannel]);
|
}, [channelId, messages.length, lastMessageId, isNearBottom, ackChannel]);
|
||||||
|
|
||||||
// Reset scroll tracking on channel switch so initial-load scroll fires
|
// Reset scroll tracking on channel switch so initial-load scroll fires
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
|||||||
@@ -284,7 +284,8 @@ function handleEvent(origin: string, event: ServerEvent): void {
|
|||||||
addRealtimeMessage(event.message.channelId, event.message);
|
addRealtimeMessage(event.message.channelId, event.message);
|
||||||
{
|
{
|
||||||
const { currentChannelId, markChannelUnread } = useChatStore.getState();
|
const { currentChannelId, markChannelUnread } = useChatStore.getState();
|
||||||
if (event.message.channelId !== currentChannelId) {
|
const myId = isHome ? useAuthStore.getState().user?.id : getMyUserIdForOrigin(origin);
|
||||||
|
if (event.message.channelId !== currentChannelId && event.message.userId !== myId) {
|
||||||
markChannelUnread(event.message.channelId);
|
markChannelUnread(event.message.channelId);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -440,7 +441,8 @@ function handleEvent(origin: string, event: ServerEvent): void {
|
|||||||
}
|
}
|
||||||
{
|
{
|
||||||
const { currentChannelId, markChannelUnread } = useChatStore.getState();
|
const { currentChannelId, markChannelUnread } = useChatStore.getState();
|
||||||
if (event.message.dmChannelId !== currentChannelId) {
|
const myId = isHome ? useAuthStore.getState().user?.id : getMyUserIdForOrigin(origin);
|
||||||
|
if (event.message.dmChannelId !== currentChannelId && event.message.userId !== myId) {
|
||||||
markChannelUnread(event.message.dmChannelId);
|
markChannelUnread(event.message.dmChannelId);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -57,6 +57,7 @@ interface ChatState {
|
|||||||
markChannelUnread: (channelId: string) => void;
|
markChannelUnread: (channelId: string) => void;
|
||||||
ackChannel: (channelId: string) => void;
|
ackChannel: (channelId: string) => void;
|
||||||
onChannelAck: (channelId: string, messageId: string) => void;
|
onChannelAck: (channelId: string, messageId: string) => void;
|
||||||
|
removeChannelStates: (channelIds: Set<string>) => void;
|
||||||
updateUserInMessages: (user: { id: string; [key: string]: any }) => void;
|
updateUserInMessages: (user: { id: string; [key: string]: any }) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -565,16 +566,22 @@ export const useChatStore = create<ChatState>((set, get) => ({
|
|||||||
ackChannel: (channelId: string) => {
|
ackChannel: (channelId: string) => {
|
||||||
const msgs = get().messages.get(channelId);
|
const msgs = get().messages.get(channelId);
|
||||||
if (!msgs || msgs.length === 0) return;
|
if (!msgs || msgs.length === 0) return;
|
||||||
const lastMsg = msgs[msgs.length - 1];
|
|
||||||
if (!lastMsg) return;
|
// Walk backward to find the last server-confirmed (non-temp) message
|
||||||
const messageId = lastMsg.id;
|
let messageId: string | null = null;
|
||||||
// Don't ack optimistic/temp messages — wait for the real server ID
|
for (let i = msgs.length - 1; i >= 0; i--) {
|
||||||
if (messageId.startsWith('temp_')) return;
|
const msg = msgs[i];
|
||||||
|
if (msg && !msg.id.startsWith('temp_')) {
|
||||||
|
messageId = msg.id;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (!messageId) return; // All messages are temp — nothing to ack yet
|
||||||
|
|
||||||
// Update local state immediately
|
// Update local state immediately
|
||||||
set((state) => {
|
set((state) => {
|
||||||
const newReadStates = new Map(state.readStates);
|
const newReadStates = new Map(state.readStates);
|
||||||
newReadStates.set(channelId, messageId);
|
newReadStates.set(channelId, messageId!);
|
||||||
const newUnread = new Set(state.unreadChannels);
|
const newUnread = new Set(state.unreadChannels);
|
||||||
newUnread.delete(channelId);
|
newUnread.delete(channelId);
|
||||||
return { readStates: newReadStates, unreadChannels: newUnread };
|
return { readStates: newReadStates, unreadChannels: newUnread };
|
||||||
@@ -595,6 +602,21 @@ export const useChatStore = create<ChatState>((set, get) => ({
|
|||||||
});
|
});
|
||||||
},
|
},
|
||||||
|
|
||||||
|
removeChannelStates: (channelIds: Set<string>) => {
|
||||||
|
if (channelIds.size === 0) return;
|
||||||
|
set((state) => {
|
||||||
|
const newUnread = new Set(state.unreadChannels);
|
||||||
|
const newReadStates = new Map(state.readStates);
|
||||||
|
const newMessages = new Map(state.messages);
|
||||||
|
for (const channelId of channelIds) {
|
||||||
|
newUnread.delete(channelId);
|
||||||
|
newReadStates.delete(channelId);
|
||||||
|
newMessages.delete(channelId);
|
||||||
|
}
|
||||||
|
return { unreadChannels: newUnread, readStates: newReadStates, messages: newMessages };
|
||||||
|
});
|
||||||
|
},
|
||||||
|
|
||||||
updateUserInMessages: (user: { id: string; homeUserId?: string | null; [key: string]: any }) => {
|
updateUserInMessages: (user: { id: string; homeUserId?: string | null; [key: string]: any }) => {
|
||||||
set((state) => {
|
set((state) => {
|
||||||
const newMessages = new Map(state.messages);
|
const newMessages = new Map(state.messages);
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import { api, BackspaceApiClient } from '../api/client';
|
|||||||
import { resolveAssetUrl, normalizeUserAssets } from '../utils/assetUrls';
|
import { resolveAssetUrl, normalizeUserAssets } from '../utils/assetUrls';
|
||||||
import { isSelf } from '../utils/identity';
|
import { isSelf } from '../utils/identity';
|
||||||
import { useAuthStore } from './authStore';
|
import { useAuthStore } from './authStore';
|
||||||
|
import { useChatStore } from './chatStore';
|
||||||
|
|
||||||
// ─── Instance-aware types ─────────────────────────────────────────────────────
|
// ─── Instance-aware types ─────────────────────────────────────────────────────
|
||||||
|
|
||||||
@@ -147,9 +148,13 @@ export const useSpaceStore = create<SpaceState>((set, get) => ({
|
|||||||
};
|
};
|
||||||
}),
|
}),
|
||||||
|
|
||||||
removeDmChannel: (id) => set((state) => ({
|
removeDmChannel: (id) => {
|
||||||
dmChannels: state.dmChannels.filter(c => c.id !== id)
|
set((state) => ({
|
||||||
})),
|
dmChannels: state.dmChannels.filter(c => c.id !== id)
|
||||||
|
}));
|
||||||
|
// Clean up unread/read state for the closed DM
|
||||||
|
useChatStore.getState().removeChannelStates(new Set([id]));
|
||||||
|
},
|
||||||
|
|
||||||
addDmMember: (dmChannelId, user) => set((state) => ({
|
addDmMember: (dmChannelId, user) => set((state) => ({
|
||||||
dmChannels: state.dmChannels.map(dm =>
|
dmChannels: state.dmChannels.map(dm =>
|
||||||
@@ -408,13 +413,14 @@ export const useSpaceStore = create<SpaceState>((set, get) => ({
|
|||||||
},
|
},
|
||||||
|
|
||||||
removeSpace: (spaceId: string) => {
|
removeSpace: (spaceId: string) => {
|
||||||
set((state) => {
|
// Collect channel IDs before set() so we can clean up chatStore after
|
||||||
// Collect channel IDs belonging to this space for map cleanup
|
const currentState = get();
|
||||||
const channelIdsToRemove = new Set<string>();
|
const channelIdsToRemove = new Set<string>();
|
||||||
for (const [channelId, sid] of state.channelToSpaceMap) {
|
for (const [channelId, sid] of currentState.channelToSpaceMap) {
|
||||||
if (sid === spaceId) channelIdsToRemove.add(channelId);
|
if (sid === spaceId) channelIdsToRemove.add(channelId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
set((state) => {
|
||||||
const channelToSpaceMap = new Map(state.channelToSpaceMap);
|
const channelToSpaceMap = new Map(state.channelToSpaceMap);
|
||||||
const channelPermissions = new Map(state.channelPermissions);
|
const channelPermissions = new Map(state.channelPermissions);
|
||||||
const channelOriginMap = new Map(state.channelOriginMap);
|
const channelOriginMap = new Map(state.channelOriginMap);
|
||||||
@@ -439,6 +445,11 @@ export const useSpaceStore = create<SpaceState>((set, get) => ({
|
|||||||
spacePermissions,
|
spacePermissions,
|
||||||
};
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Clean up orphaned unread/read states and cached messages in chatStore
|
||||||
|
if (channelIdsToRemove.size > 0) {
|
||||||
|
useChatStore.getState().removeChannelStates(channelIdsToRemove);
|
||||||
|
}
|
||||||
},
|
},
|
||||||
|
|
||||||
updateMemberPresence: (userId: string, status: string) => {
|
updateMemberPresence: (userId: string, status: string) => {
|
||||||
@@ -739,6 +750,13 @@ export const useSpaceStore = create<SpaceState>((set, get) => ({
|
|||||||
},
|
},
|
||||||
|
|
||||||
removeInstanceSpaces: (origin: string) => {
|
removeInstanceSpaces: (origin: string) => {
|
||||||
|
// Collect channel IDs before set() for chatStore cleanup
|
||||||
|
const currentState = get();
|
||||||
|
const channelIdsToRemove = new Set<string>();
|
||||||
|
for (const [channelId, chOrigin] of currentState.channelOriginMap) {
|
||||||
|
if (chOrigin === origin) channelIdsToRemove.add(channelId);
|
||||||
|
}
|
||||||
|
|
||||||
set((state) => {
|
set((state) => {
|
||||||
const remainingSpaces = state.spaces.filter(s => s._instanceOrigin !== origin);
|
const remainingSpaces = state.spaces.filter(s => s._instanceOrigin !== origin);
|
||||||
|
|
||||||
@@ -749,13 +767,11 @@ export const useSpaceStore = create<SpaceState>((set, get) => ({
|
|||||||
const channelOriginMap = new Map(state.channelOriginMap);
|
const channelOriginMap = new Map(state.channelOriginMap);
|
||||||
const spacePermissions = new Map(state.spacePermissions);
|
const spacePermissions = new Map(state.spacePermissions);
|
||||||
|
|
||||||
for (const [channelId, chOrigin] of state.channelOriginMap) {
|
for (const channelId of channelIdsToRemove) {
|
||||||
if (chOrigin === origin) {
|
channelToSpaceMap.delete(channelId);
|
||||||
channelToSpaceMap.delete(channelId);
|
channelLastMessageIds.delete(channelId);
|
||||||
channelLastMessageIds.delete(channelId);
|
channelPermissions.delete(channelId);
|
||||||
channelPermissions.delete(channelId);
|
channelOriginMap.delete(channelId);
|
||||||
channelOriginMap.delete(channelId);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
for (const s of state.spaces) {
|
for (const s of state.spaces) {
|
||||||
if (s._instanceOrigin === origin) {
|
if (s._instanceOrigin === origin) {
|
||||||
@@ -775,6 +791,11 @@ export const useSpaceStore = create<SpaceState>((set, get) => ({
|
|||||||
: null,
|
: null,
|
||||||
};
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Clean up orphaned unread/read states and cached messages in chatStore
|
||||||
|
if (channelIdsToRemove.size > 0) {
|
||||||
|
useChatStore.getState().removeChannelStates(channelIdsToRemove);
|
||||||
|
}
|
||||||
},
|
},
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user