From fa31dff0614ec6f9667cb1a9e14b41609173f5a2 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 24 Jul 2026 01:35:25 -0700 Subject: [PATCH 1/2] fix(reactions): recompute `mine` per-client on socket updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit emitReactionChange computes `mine` for the user who triggered the add/remove and broadcasts that ONE view to the whole pod room. Every other client then adopts a wrong `mine` — which flips the add/remove toggle (clicking your own reaction POSTs/re-adds instead of DELETE/removing it: "un-react didn't revert") and mis-highlights chips. 2026-07-24: reported after a second account reacted. Fix: the socket handler recomputes `mine` for THIS client from each reaction's `users` list (already on the wire), falling back to the broadcast value only when `users` is absent (older server / Mongo fallback). Extracted as the pure `recomputeReactionMine` so it's unit-tested. Backend un-react already works (DELETE removes correctly); this was purely the client trusting a broadcast `mine` that wasn't its own. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01DMeWzgFxsfBcjoLVLewEES --- .../__tests__/recomputeReactionMine.test.ts | 37 +++++++++++++++++++ frontend/src/v2/hooks/useV2PodDetail.ts | 32 ++++++++++++++-- 2 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 frontend/src/v2/__tests__/recomputeReactionMine.test.ts diff --git a/frontend/src/v2/__tests__/recomputeReactionMine.test.ts b/frontend/src/v2/__tests__/recomputeReactionMine.test.ts new file mode 100644 index 00000000..aaf002cf --- /dev/null +++ b/frontend/src/v2/__tests__/recomputeReactionMine.test.ts @@ -0,0 +1,37 @@ +// Guards the socket `mine`-broadcast fix (2026-07-24). emitReactionChange sends +// one user's `mine` view to the whole room, so other clients must recompute it +// from the reaction's user list — otherwise clicking your own reaction re-adds +// instead of removing (the "un-react didn't revert" report) and chips +// mis-highlight. +import { recomputeReactionMine } from '../hooks/useV2PodDetail'; + +describe('recomputeReactionMine', () => { + test('mine=true when the current user is in the reaction user list', () => { + const out = recomputeReactionMine( + [{ emoji: '👍', count: 2, mine: false, users: [{ id: 'a' }, { id: 'me' }] }], + 'me', + ); + expect(out[0].mine).toBe(true); + }); + + test('mine=false when the user is NOT in the list — even if the wire said true (the bug)', () => { + const out = recomputeReactionMine( + [{ emoji: '👍', count: 1, mine: true, users: [{ id: 'other' }] }], + 'me', + ); + expect(out[0].mine).toBe(false); + }); + + test('falls back to the wire mine when users is absent (older server / Mongo fallback)', () => { + const out = recomputeReactionMine([{ emoji: '👍', count: 1, mine: true }], 'me'); + expect(out[0].mine).toBe(true); + }); + + test('falls back to the wire mine when userId is missing', () => { + const out = recomputeReactionMine( + [{ emoji: '👍', count: 1, mine: true, users: [{ id: 'other' }] }], + undefined, + ); + expect(out[0].mine).toBe(true); + }); +}); diff --git a/frontend/src/v2/hooks/useV2PodDetail.ts b/frontend/src/v2/hooks/useV2PodDetail.ts index c41b0d15..3ad31c18 100644 --- a/frontend/src/v2/hooks/useV2PodDetail.ts +++ b/frontend/src/v2/hooks/useV2PodDetail.ts @@ -2,6 +2,7 @@ import { useCallback, useEffect, useState } from 'react'; import { useV2Api } from './useV2Api'; import { V2Pod, V2PodMember } from './useV2Pods'; import { useSocket } from '../../context/SocketContext'; +import { useAuth } from '../../context/AuthContext'; export interface V2Message { id: string; @@ -136,9 +137,26 @@ const chronologicalMessages = (messages: V2Message[]): V2Message[] => ( )) ); +// `emitReactionChange` computes `mine` for the user who triggered the change and +// broadcasts that single view to the whole room — so every OTHER client gets a +// wrong `mine`, which flips the add/remove toggle (clicking your own reaction +// re-adds instead of removing) and mis-highlights chips (2026-07-24). Recompute +// `mine` for THIS client from each reaction's user list; fall back to the wire +// value only when `users` is absent (older server / Mongo fallback path). +export const recomputeReactionMine = }>( + reactions: T[], + userId: string | undefined | null, +): T[] => reactions.map((r) => ({ + ...r, + mine: Array.isArray(r.users) && userId + ? r.users.some((u) => String(u.id) === String(userId)) + : r.mine, +})); + export const useV2PodDetail = (podId: string | null): UseV2PodDetailResult => { const api = useV2Api(); const { socket, connected, joinPod, leavePod } = useSocket(); + const { currentUser } = useAuth(); const [pod, setPod] = useState(null); const [messages, setMessages] = useState([]); const [agents, setAgents] = useState([]); @@ -241,12 +259,20 @@ export const useV2PodDetail = (podId: string | null): UseV2PodDetailResult => { // `messageReaction` with the new aggregated list after every add/remove // by any user. We patch only the matching message's `reactions` array // — no other state churn. - const handleReactionChange = (payload: { messageId: string; podId?: string; reactions: Array<{ emoji: string; count: number; mine: boolean }> }) => { + const handleReactionChange = (payload: { messageId: string; podId?: string; reactions: Array<{ emoji: string; count: number; mine: boolean; users?: Array<{ id: string }> }> }) => { if (!payload || !payload.messageId) return; if (payload.podId && payload.podId !== podId) return; + // `emitReactionChange` computes `mine` for the user who triggered the + // change and broadcasts that single view to the whole room. Trusting it + // gives every OTHER client a wrong `mine` — which flips the add/remove + // toggle (clicking your own reaction re-adds instead of removing) and + // mis-highlights chips. Recompute `mine` for THIS client from the + // reaction's user list (2026-07-24). Fall back to the wire value only + // when `users` is absent (older server / Mongo fallback). + const reactions = recomputeReactionMine(payload.reactions, currentUser?._id); setMessages((prev) => prev.map((m) => ( String(m.id) === String(payload.messageId) - ? { ...m, reactions: payload.reactions } + ? { ...m, reactions } : m ))); }; @@ -257,7 +283,7 @@ export const useV2PodDetail = (podId: string | null): UseV2PodDetailResult => { socket.off('messageReaction', handleReactionChange); leavePod(podId); }; - }, [podId, socket, connected, joinPod, leavePod]); + }, [podId, socket, connected, joinPod, leavePod, currentUser?._id]); const sendMessage = useCallback(async (content: string, messageType = 'text', replyToMessageId?: string): Promise => { if (!podId || !content.trim()) return null; From a52b465c0f786c0c3ba5c7484a0e8e707897cbc3 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 24 Jul 2026 01:43:49 -0700 Subject: [PATCH 2/2] test: mock useAuth in useV2PodDetailReply (hook now reads currentUser) useV2PodDetail reads currentUser to recompute reaction `mine` per-client; useAuth throws without an AuthProvider, so this renderHook-based suite needs the stub. Only affected suite; other hook consumers already provide/mocked it. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01DMeWzgFxsfBcjoLVLewEES --- frontend/src/v2/__tests__/useV2PodDetailReply.test.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontend/src/v2/__tests__/useV2PodDetailReply.test.tsx b/frontend/src/v2/__tests__/useV2PodDetailReply.test.tsx index 71de7a88..2cb95277 100644 --- a/frontend/src/v2/__tests__/useV2PodDetailReply.test.tsx +++ b/frontend/src/v2/__tests__/useV2PodDetailReply.test.tsx @@ -28,6 +28,11 @@ jest.mock('../../context/SocketContext', () => ({ leavePod: jest.fn(), }), })); +// useV2PodDetail now reads currentUser (to recompute reaction `mine` per-client); +// useAuth throws without an AuthProvider, so stub it here. +jest.mock('../../context/AuthContext', () => ({ + useAuth: () => ({ currentUser: { _id: 'me' } }), +})); describe('useV2PodDetail reply threading (#646)', () => { beforeEach(() => {