From d58f6cdb33fa1cc3d00e8059c0f0abc09856feef Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Wed, 8 Apr 2026 13:57:24 +0800 Subject: [PATCH] fix(web): replace actor_id self-event filtering with idempotent cache updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit actor_id identifies the user, not the browser tab. Filtering WS events by actor_id broke multi-tab sync — other tabs of the same user would silently miss updates. Instead, make all WS cache handlers idempotent (dedup checks on add, no-op on duplicate merge/filter) so mutations and WS events coexist safely without filtering. - WSClient: pass actor_id to event handlers for future per-handler use - use-realtime-sync: remove isSelf() gating from onAny and specific handlers - useCreateIssue: add .some() dedup guard + onSettled invalidation - use-issue-reactions: remove payload-level self-filter (dedup already present) - use-issue-timeline: remove payload-level self-filter on comment:created, reaction:added, reaction:removed (dedup already present) - Clean up useCallback deps that no longer reference userId Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/web/core/issues/mutations.ts | 8 +++++++- .../issues/hooks/use-issue-reactions.ts | 7 ++----- .../features/issues/hooks/use-issue-timeline.ts | 17 +++-------------- apps/web/features/realtime/hooks.ts | 2 +- apps/web/features/realtime/provider.tsx | 2 +- apps/web/features/realtime/use-realtime-sync.ts | 12 ++++-------- apps/web/shared/api/ws-client.ts | 4 ++-- 7 files changed, 20 insertions(+), 32 deletions(-) diff --git a/apps/web/core/issues/mutations.ts b/apps/web/core/issues/mutations.ts index ff35facc..47a2953e 100644 --- a/apps/web/core/issues/mutations.ts +++ b/apps/web/core/issues/mutations.ts @@ -21,11 +21,14 @@ export function useCreateIssue() { mutationFn: (data: CreateIssueRequest) => api.createIssue(data), onSuccess: (newIssue) => { qc.setQueryData(issueKeys.list(wsId), (old) => - old + old && !old.issues.some((i) => i.id === newIssue.id) ? { ...old, issues: [...old.issues, newIssue], total: old.total + 1 } : old, ); }, + onSettled: () => { + qc.invalidateQueries({ queryKey: issueKeys.list(wsId) }); + }, }); } @@ -204,6 +207,9 @@ export function useCreateComment(issueId: string) { }, ); }, + onSettled: () => { + qc.invalidateQueries({ queryKey: issueKeys.timeline(issueId) }); + }, }); } diff --git a/apps/web/features/issues/hooks/use-issue-reactions.ts b/apps/web/features/issues/hooks/use-issue-reactions.ts index 7e7418e7..7fec1fd8 100644 --- a/apps/web/features/issues/hooks/use-issue-reactions.ts +++ b/apps/web/features/issues/hooks/use-issue-reactions.ts @@ -34,8 +34,6 @@ export function useIssueReactions(issueId: string, userId?: string) { (payload: unknown) => { const { reaction, issue_id } = payload as IssueReactionAddedPayload; if (issue_id !== issueId) return; - if (reaction.actor_type === "member" && reaction.actor_id === userId) - return; qc.setQueryData( issueKeys.reactions(issueId), (old) => { @@ -45,7 +43,7 @@ export function useIssueReactions(issueId: string, userId?: string) { }, ); }, - [qc, issueId, userId], + [qc, issueId], ), ); @@ -55,7 +53,6 @@ export function useIssueReactions(issueId: string, userId?: string) { (payload: unknown) => { const p = payload as IssueReactionRemovedPayload; if (p.issue_id !== issueId) return; - if (p.actor_type === "member" && p.actor_id === userId) return; qc.setQueryData( issueKeys.reactions(issueId), (old) => @@ -69,7 +66,7 @@ export function useIssueReactions(issueId: string, userId?: string) { ), ); }, - [qc, issueId, userId], + [qc, issueId], ), ); diff --git a/apps/web/features/issues/hooks/use-issue-timeline.ts b/apps/web/features/issues/hooks/use-issue-timeline.ts index fd303c17..894c2880 100644 --- a/apps/web/features/issues/hooks/use-issue-timeline.ts +++ b/apps/web/features/issues/hooks/use-issue-timeline.ts @@ -63,11 +63,6 @@ export function useIssueTimeline(issueId: string, userId?: string) { (payload: unknown) => { const { comment } = payload as CommentCreatedPayload; if (comment.issue_id !== issueId) return; - if ( - comment.author_type === "member" && - comment.author_id === userId - ) - return; qc.setQueryData( issueKeys.timeline(issueId), (old) => { @@ -77,7 +72,7 @@ export function useIssueTimeline(issueId: string, userId?: string) { }, ); }, - [qc, issueId, userId], + [qc, issueId], ), ); @@ -161,11 +156,6 @@ export function useIssueTimeline(issueId: string, userId?: string) { (payload: unknown) => { const { reaction, issue_id } = payload as ReactionAddedPayload; if (issue_id !== issueId) return; - if ( - reaction.actor_type === "member" && - reaction.actor_id === userId - ) - return; qc.setQueryData( issueKeys.timeline(issueId), (old) => @@ -177,7 +167,7 @@ export function useIssueTimeline(issueId: string, userId?: string) { }), ); }, - [qc, issueId, userId], + [qc, issueId], ), ); @@ -187,7 +177,6 @@ export function useIssueTimeline(issueId: string, userId?: string) { (payload: unknown) => { const p = payload as ReactionRemovedPayload; if (p.issue_id !== issueId) return; - if (p.actor_type === "member" && p.actor_id === userId) return; qc.setQueryData( issueKeys.timeline(issueId), (old) => @@ -207,7 +196,7 @@ export function useIssueTimeline(issueId: string, userId?: string) { }), ); }, - [qc, issueId, userId], + [qc, issueId], ), ); diff --git a/apps/web/features/realtime/hooks.ts b/apps/web/features/realtime/hooks.ts index 17d660ef..562a897c 100644 --- a/apps/web/features/realtime/hooks.ts +++ b/apps/web/features/realtime/hooks.ts @@ -4,7 +4,7 @@ import { useEffect } from "react"; import type { WSEventType } from "@/shared/types"; import { useWS } from "./provider"; -type EventHandler = (payload: unknown) => void; +type EventHandler = (payload: unknown, actorId?: string) => void; /** * Hook that subscribes to a WebSocket event and calls the handler. diff --git a/apps/web/features/realtime/provider.tsx b/apps/web/features/realtime/provider.tsx index 6381ba44..c4558e92 100644 --- a/apps/web/features/realtime/provider.tsx +++ b/apps/web/features/realtime/provider.tsx @@ -22,7 +22,7 @@ const WS_URL = ? `${window.location.protocol === "https:" ? "wss:" : "ws:"}//${window.location.host}/ws` : "ws://localhost:8080/ws"); -type EventHandler = (payload: unknown) => void; +type EventHandler = (payload: unknown, actorId?: string) => void; interface WSContextValue { subscribe: (event: WSEventType, handler: EventHandler) => () => void; diff --git a/apps/web/features/realtime/use-realtime-sync.ts b/apps/web/features/realtime/use-realtime-sync.ts index 4302cc34..cf79e75f 100644 --- a/apps/web/features/realtime/use-realtime-sync.ts +++ b/apps/web/features/realtime/use-realtime-sync.ts @@ -86,20 +86,16 @@ export function useRealtimeSync(ws: WSClient | null) { ]); const unsubAny = ws.onAny((msg) => { - const myUserId = useAuthStore.getState().user?.id; - if (msg.actor_id && msg.actor_id === myUserId) { - logger.debug("skipping self-event", msg.type); - return; - } if (specificEvents.has(msg.type)) return; const prefix = msg.type.split(":")[0] ?? ""; const refresh = refreshMap[prefix]; if (refresh) debouncedRefresh(prefix, refresh); }); - // --- Specific event handlers (granular updates, no full refetch) --- - // NOTE: ws.on() passes msg.payload (no actor_id). Self-event suppression - // requires WSClient changes to expose actor_id — tracked as separate task. + // --- Specific event handlers (granular cache updates) --- + // No self-event filtering: actor_id identifies the USER, not the TAB. + // Filtering by actor_id would block other tabs of the same user. + // Instead, both mutations and WS handlers use dedup checks to be idempotent. const unsubIssueUpdated = ws.on("issue:updated", (p) => { const { issue } = p as IssueUpdatedPayload; diff --git a/apps/web/shared/api/ws-client.ts b/apps/web/shared/api/ws-client.ts index 17282a44..05f1d05c 100644 --- a/apps/web/shared/api/ws-client.ts +++ b/apps/web/shared/api/ws-client.ts @@ -1,7 +1,7 @@ import type { WSMessage, WSEventType } from "@/shared/types"; import { type Logger, noopLogger } from "@/shared/logger"; -type EventHandler = (payload: unknown) => void; +type EventHandler = (payload: unknown, actorId?: string) => void; export class WSClient { private ws: WebSocket | null = null; @@ -53,7 +53,7 @@ export class WSClient { const eventHandlers = this.handlers.get(msg.type); if (eventHandlers) { for (const handler of eventHandlers) { - handler(msg.payload); + handler(msg.payload, msg.actor_id); } } for (const handler of this.anyHandlers) {