fix(mentions): address PR review feedback from copilot

- Batch Set updates to avoid repeated allocations in loop
- Reset pendingMentionTimestamp on user logout to prevent cross-user state leaks
- Precompute message timestamp Map for O(1) lookup in IntersectionObserver
- Remove unused authMutation (redundant with HTTP endpoint flow)
- Add future timestamp validation to prevent notification suppression attacks
- Include lastReadMentionTimestamp in session validation for cross-device persistence

commit: 128e01c78179176c2e15beec17fc66e849efe137
author: Kit · 9mo ago · 2026-01-24 05:25 UTC
parent: 51dcfa2
signature: SSH UnverifiedSHA256:H91DFuEG4jpk5edzLA4aUKkpv4zv9Q339ipRmkph6Tk The key is unknown

4 files · +29 -25

M src/convex/mutations.ts +0 -19


@@ -113,22 +113,3 @@ export const getOrCreateDefaultRoom = mutation({
113	113		};
114	114		}
115	115		});
116		-
117		-	// Authenticated mutation: Update the timestamp of the last read mention
118		-	export const updateLastReadMentionTimestamp = authMutation({
119		-	args: {
120		-	timestamp: v.number()
121		-	},
122		-	handler: async (ctx, args) => {
123		-	const currentTimestamp = ctx.user.lastReadMentionTimestamp ?? 0;
124		-
125		-	// Only update if the new timestamp is greater (more recent)
126		-	if (args.timestamp > currentTimestamp) {
127		-	await ctx.db.patch(ctx.user._id, {
128		-	lastReadMentionTimestamp: args.timestamp
129		-	});
130		-	}
131		-
132		-	return { success: true };
133		-	}
134		-	});


@@ -113,22 +113,3 @@ export const getOrCreateDefaultRoom = mutation({
113		};	113	};
114		}	114	}
115		});	115	});
116	-
117	-	// Authenticated mutation: Update the timestamp of the last read mention
118	-	export const updateLastReadMentionTimestamp = authMutation({
119	-	args: {
120	-	timestamp: v.number()
121	-	},
122	-	handler: async (ctx, args) => {
123	-	const currentTimestamp = ctx.user.lastReadMentionTimestamp ?? 0;
124	-
125	-	// Only update if the new timestamp is greater (more recent)
126	-	if (args.timestamp > currentTimestamp) {
127	-	await ctx.db.patch(ctx.user._id, {
128	-	lastReadMentionTimestamp: args.timestamp
129	-	});
130	-	}
131	-
132	-	return { success: true };
133	-	}
134	-	});

M src/lib/api/session.server.ts +2 -1


@@ -92,7 +92,8 @@ export async function validateSessionToken(token: string): Promise<SessionValida
92	92		nickname: result.user.nickname,
93	93		status: result.user.status as SafeUser['status'],
94	94		avatarUrl: result.user.avatarUrl,
95		-	lastSeen: result.user.lastSeen
	95	+	lastSeen: result.user.lastSeen,
	96	+	lastReadMentionTimestamp: result.user.lastReadMentionTimestamp
96	97		};
97	98
98	99		log.debug('Session validated', { tokenHash, userId: user.id });


@@ -92,7 +92,8 @@ export async function validateSessionToken(token: string): Promise<SessionValida
92		nickname: result.user.nickname,	92		nickname: result.user.nickname,
93		status: result.user.status as SafeUser['status'],	93		status: result.user.status as SafeUser['status'],
94		avatarUrl: result.user.avatarUrl,	94		avatarUrl: result.user.avatarUrl,
95	-	lastSeen: result.user.lastSeen	95	+	lastSeen: result.user.lastSeen,
			96	+	lastReadMentionTimestamp: result.user.lastReadMentionTimestamp
96		};	97		};
97			98
98		log.debug('Session validated', { tokenHash, userId: user.id });	99		log.debug('Session validated', { tokenHash, userId: user.id });

M src/lib/components/chat-room.svelte +17 -5


@@ -348,6 +348,8 @@
348	348		if (mentionUnreadCount !== 0) {
349	349		mentionUnreadCount = 0;
350	350		}
	351	+	// Reset pending timestamp when user logs out
	352	+	pendingMentionTimestamp = null;
351	353		// mention observer reset
352	354		if (mentionObserver) {
353	355		mentionObserver.disconnect();

@@ -357,16 +359,21 @@
357	359		}
358	360		const mentionIds = new SvelteSet<string>();
359	361		const lastReadTimestamp = currentUser.lastReadMentionTimestamp ?? 0;
	362	+	const mentionIdsToPreObserve: string[] = [];
360	363
361	364		for (const message of visibleMessages) {
362	365		if (shouldHighlightMention(message)) {
363	366		mentionIds.add(message.id);
364	367		// Pre-mark as observed if message is older than lastReadMentionTimestamp
365	368		if (message.timestamp <= lastReadTimestamp && !mentionObservedIds.has(message.id)) {
366		-	mentionObservedIds = new SvelteSet([...mentionObservedIds, message.id]);
	369	+	mentionIdsToPreObserve.push(message.id);
367	370		}
368	371		}
369	372		}
	373	+	// Batch update observed IDs to avoid repeated Set allocations
	374	+	if (mentionIdsToPreObserve.length > 0) {
	375	+	mentionObservedIds = new SvelteSet([...mentionObservedIds, ...mentionIdsToPreObserve]);
	376	+	}
370	377		const nextMentionIds = mentionIds;
371	378		if (!setsEqual(nextMentionIds, mentionMessageIds)) {
372	379		mentionMessageIds = nextMentionIds;

@@ -386,6 +393,11 @@
386	393		if (mentionObserver) {
387	394		mentionObserver.disconnect();
388	395		}
	396	+	// Precompute message timestamp map to avoid O(n) lookup in callback
	397	+	const messageTimestampMap = new Map<string, number>();
	398	+	for (const msg of visibleMessages) {
	399	+	messageTimestampMap.set(msg.id, msg.timestamp);
	400	+	}
389	401		mentionObserver = new IntersectionObserver(
390	402		(entries) => {
391	403		const updatedObserved = new SvelteSet(mentionObservedIds);

@@ -402,10 +414,10 @@
402	414		updatedObserved.add(messageId);
403	415		changed = true;
404	416
405		-	// Find the message to get its timestamp
406		-	const message = visibleMessages.find((m) => m.id === messageId);
407		-	if (message && message.timestamp > maxTimestamp) {
408		-	maxTimestamp = message.timestamp;
	417	+	// Get timestamp from precomputed map (O(1) instead of O(n))
	418	+	const timestamp = messageTimestampMap.get(messageId);
	419	+	if (timestamp !== undefined && timestamp > maxTimestamp) {
	420	+	maxTimestamp = timestamp;
409	421		}
410	422		}
411	423		}

 			if (mentionUnreadCount !== 0) {
 				mentionUnreadCount = 0;
 			}
+			// Reset pending timestamp when user logs out
+			pendingMentionTimestamp = null;
 			// mention observer reset
 			if (mentionObserver) {
 				mentionObserver.disconnect();
 		}
 		const mentionIds = new SvelteSet<string>();
 		const lastReadTimestamp = currentUser.lastReadMentionTimestamp ?? 0;
+		const mentionIdsToPreObserve: string[] = [];
 		for (const message of visibleMessages) {
 			if (shouldHighlightMention(message)) {
 				mentionIds.add(message.id);
 				// Pre-mark as observed if message is older than lastReadMentionTimestamp
 				if (message.timestamp <= lastReadTimestamp && !mentionObservedIds.has(message.id)) {
-					mentionObservedIds = new SvelteSet([...mentionObservedIds, message.id]);
+					mentionIdsToPreObserve.push(message.id);
 				}
 			}
 		}
+		// Batch update observed IDs to avoid repeated Set allocations
+		if (mentionIdsToPreObserve.length > 0) {
+			mentionObservedIds = new SvelteSet([...mentionObservedIds, ...mentionIdsToPreObserve]);
+		}
 		const nextMentionIds = mentionIds;
 		if (!setsEqual(nextMentionIds, mentionMessageIds)) {
 			mentionMessageIds = nextMentionIds;
 		if (mentionObserver) {
 			mentionObserver.disconnect();
 		}
+		// Precompute message timestamp map to avoid O(n) lookup in callback
+		const messageTimestampMap = new Map<string, number>();
+		for (const msg of visibleMessages) {
+			messageTimestampMap.set(msg.id, msg.timestamp);
+		}
 		mentionObserver = new IntersectionObserver(
 			(entries) => {
 				const updatedObserved = new SvelteSet(mentionObservedIds);
 						updatedObserved.add(messageId);
 						changed = true;
-						// Find the message to get its timestamp
+						// Get timestamp from precomputed map (O(1) instead of O(n))
-						const message = visibleMessages.find((m) => m.id === messageId);
+						const timestamp = messageTimestampMap.get(messageId);
-						if (message && message.timestamp > maxTimestamp) {
+						if (timestamp !== undefined && timestamp > maxTimestamp) {
-							maxTimestamp = message.timestamp;
+							maxTimestamp = timestamp;
 						}
 					}
 				}

M src/routes/api/chat/mention-read/+server.ts +10 -0


@@ -32,6 +32,16 @@ export const POST: RequestHandler = async ({ request, cookies }) => {
32	32		});
33	33		}
34	34
	35	+	// Prevent far-future timestamps that could suppress notifications permanently
	36	+	const now = Date.now();
	37	+	const maxSkewMs = 5 * 60 * 1000; // 5 minutes clock skew tolerance
	38	+	if (timestamp > now + maxSkewMs) {
	39	+	return new Response(JSON.stringify({ error: 'Timestamp too far in the future' }), {
	40	+	status: 400,
	41	+	headers: { 'Content-Type': 'application/json' }
	42	+	});
	43	+	}
	44	+
35	45		await users.updateLastReadMentionTimestamp(user.id, timestamp);
36	46
37	47		return new Response(JSON.stringify({ success: true }), {


@@ -32,6 +32,16 @@ export const POST: RequestHandler = async ({ request, cookies }) => {
32	});	32		});
33	}	33		}
34		34
		35	+	// Prevent far-future timestamps that could suppress notifications permanently
		36	+	const now = Date.now();
		37	+	const maxSkewMs = 5 * 60 * 1000; // 5 minutes clock skew tolerance
		38	+	if (timestamp > now + maxSkewMs) {
		39	+	return new Response(JSON.stringify({ error: 'Timestamp too far in the future' }), {
		40	+	status: 400,
		41	+	headers: { 'Content-Type': 'application/json' }
		42	+	});
		43	+	}
		44	+
35	await users.updateLastReadMentionTimestamp(user.id, timestamp);	45		await users.updateLastReadMentionTimestamp(user.id, timestamp);
36		46
37	return new Response(JSON.stringify({ success: true }), {	47		return new Response(JSON.stringify({ success: true }), {