From 0c52b89e40b2f6b53f31f808ee92b15d886d163f Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Wed, 25 Mar 2026 11:37:23 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20code=20review=20=E2=80=94=20WS=20sync=20?= =?UTF-8?q?bug,=20Hub=20race=20condition,=20error=20feedback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix useRealtimeSync never receiving WSClient (useRef → useState for re-render trigger, keeping ref for lazy subscribe callback) - Fix Hub.Run() global broadcast mutating map under RLock (same two-phase collect+cleanup pattern as BroadcastToWorkspace) - Move visibleStatuses to module-level constant (prevent useCallback recreation every render) - Replace console.error with toast.error for user-facing operations in issues page and inbox page Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/web/app/(dashboard)/inbox/page.tsx | 5 +++-- apps/web/app/(dashboard)/issues/page.tsx | 23 +++++++++++++---------- apps/web/features/realtime/provider.tsx | 8 ++++++-- server/internal/realtime/hub.go | 20 ++++++++++++++++++-- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/apps/web/app/(dashboard)/inbox/page.tsx b/apps/web/app/(dashboard)/inbox/page.tsx index d519e7e1..721c7ea7 100644 --- a/apps/web/app/(dashboard)/inbox/page.tsx +++ b/apps/web/app/(dashboard)/inbox/page.tsx @@ -2,6 +2,7 @@ import { useState, useEffect, useMemo } from "react"; import { useInboxStore } from "@multica/store"; +import { toast } from "sonner"; import { AlertCircle, Bot, @@ -205,7 +206,7 @@ export default function InboxPage() { await api.markInboxRead(id); useInboxStore.getState().markRead(id); } catch (err) { - console.error("Failed to mark read:", err); + toast.error("Failed to mark as read"); } }; @@ -218,7 +219,7 @@ export default function InboxPage() { setSelectedId(""); } } catch (err) { - console.error("Failed to archive:", err); + toast.error("Failed to archive"); } }; diff --git a/apps/web/app/(dashboard)/issues/page.tsx b/apps/web/app/(dashboard)/issues/page.tsx index f3cc265a..fe300191 100644 --- a/apps/web/app/(dashboard)/issues/page.tsx +++ b/apps/web/app/(dashboard)/issues/page.tsx @@ -2,6 +2,7 @@ import { useState, useCallback, useEffect, useMemo } from "react"; import { useIssueStore } from "@multica/store"; +import { toast } from "sonner"; import Link from "next/link"; import { Columns3, @@ -55,6 +56,15 @@ function formatDate(date: string): string { }); } +const BOARD_STATUSES: IssueStatus[] = [ + "backlog", + "todo", + "in_progress", + "in_review", + "done", + "blocked", +]; + // --------------------------------------------------------------------------- // Board View — Card // --------------------------------------------------------------------------- @@ -187,14 +197,7 @@ function BoardView({ }) ); - const visibleStatuses: IssueStatus[] = [ - "backlog", - "todo", - "in_progress", - "in_review", - "done", - "blocked", - ]; + const visibleStatuses = BOARD_STATUSES; const handleDragStart = useCallback( (event: DragStartEvent) => { @@ -358,7 +361,7 @@ function CreateIssueDialog({ onCreated }: { onCreated: (issue: Issue) => void }) reset(); setOpen(false); } catch (err) { - console.error("Failed to create issue:", err); + toast.error("Failed to create issue"); } finally { setSubmitting(false); } @@ -491,7 +494,7 @@ export default function IssuesPage() { // Persist to API api.updateIssue(issueId, { status: newStatus }).catch((err) => { - console.error("Failed to update issue:", err); + toast.error("Failed to move issue"); // Revert on error by refetching api.listIssues({ limit: 200 }).then((res) => { useIssueStore.getState().setIssues(res.issues); diff --git a/apps/web/features/realtime/provider.tsx b/apps/web/features/realtime/provider.tsx index 11b5382f..83bc7c36 100644 --- a/apps/web/features/realtime/provider.tsx +++ b/apps/web/features/realtime/provider.tsx @@ -4,6 +4,7 @@ import { createContext, useContext, useEffect, + useState, useRef, useCallback, type ReactNode, @@ -27,6 +28,7 @@ const WSContext = createContext(null); export function WSProvider({ children }: { children: ReactNode }) { const user = useAuthStore((s) => s.user); const workspace = useWorkspaceStore((s) => s.workspace); + const [wsClient, setWsClient] = useState(null); const wsRef = useRef(null); useEffect(() => { @@ -38,16 +40,18 @@ export function WSProvider({ children }: { children: ReactNode }) { const ws = new WSClient(WS_URL); ws.setAuth(token, workspace.id); wsRef.current = ws; + setWsClient(ws); ws.connect(); return () => { ws.disconnect(); wsRef.current = null; + setWsClient(null); }; }, [user, workspace]); - // Centralized WS → store sync - useRealtimeSync(wsRef.current); + // Centralized WS → store sync (uses state so it re-subscribes when WS changes) + useRealtimeSync(wsClient); const subscribe = useCallback( (event: WSEventType, handler: EventHandler) => { diff --git a/server/internal/realtime/hub.go b/server/internal/realtime/hub.go index 2a38fc5a..b648c57f 100644 --- a/server/internal/realtime/hub.go +++ b/server/internal/realtime/hub.go @@ -92,17 +92,33 @@ func (h *Hub) Run() { case message := <-h.broadcast: // Global broadcast for daemon events (no workspace filtering) h.mu.RLock() + var slow []*Client for _, clients := range h.rooms { for client := range clients { select { case client.send <- message: default: - close(client.send) - delete(clients, client) + slow = append(slow, client) } } } h.mu.RUnlock() + if len(slow) > 0 { + h.mu.Lock() + for _, client := range slow { + room := client.workspaceID + if clients, ok := h.rooms[room]; ok { + if _, exists := clients[client]; exists { + delete(clients, client) + close(client.send) + if len(clients) == 0 { + delete(h.rooms, room) + } + } + } + } + h.mu.Unlock() + } } } }