From b4719ad070f44f0237150dc35d60639699dfe331 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 23 Apr 2026 16:03:01 -0700 Subject: [PATCH] fix(canvas): Legend avoids TemplatePalette + silence WS handshake races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Two unrelated but small UI fixes surfaced while testing the Canvas **1. Legend hidden under the open TemplatePalette.** Legend is `fixed bottom-6 left-4 z-30`. TemplatePalette's drawer (when open) is `fixed top-0 left-0 w-[280px] z-30` — same z-index, same left-edge column. The Legend overlapped the palette's bottom 180 px. Published the palette-open state to the canvas store so the Legend can shift right (to `left-[296px]` — 280 px palette + 16 px gap) while the palette is open, animated via a 200 ms `transition-[left]` to match the palette's slide. Closes cleanly back to `left-4` when the palette is dismissed. Files: - `store/canvas.ts` — added `templatePaletteOpen` + `setTemplatePaletteOpen`. - `TemplatePalette.tsx` — calls `setTemplatePaletteOpen(open)` on every open/close transition via a new useEffect. - `Legend.tsx` — reads the flag and swaps `left-4` <-> `left-[296px]`. **2. "WebSocket is closed before the connection is established" spam.** Two components (`ChatTab`, `AgentCommsPanel`) open their own short- lived WebSocket to tail the ACTIVITY_LOGGED stream. Their cleanup path called `ws.close()` unconditionally, which trips a browser console warning when React StrictMode re-runs the effect in dev and the handshake hasn't completed yet. Confirmed via DevTools console on the running canvas. Added a `closeWebSocketGracefully(ws)` helper in `lib/ws-close.ts`: - OPEN / CLOSING → close immediately (normal path). - CONNECTING → defer close to the 'open' listener so the browser sees a full handshake. Also wires an 'error' listener that cancels the queued close if the handshake fails (no double-close). - CLOSED → no-op. Both consumers now call the helper in their useEffect cleanup. Silences the warning without changing observable behaviour. ### Tests `canvas/src/lib/__tests__/ws-close.test.ts` — 5 cases with a fake WebSocket covering each readyState branch plus the error-before-open cancellation path. Full vitest suite: 927/927 pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/Legend.tsx | 8 +- canvas/src/components/TemplatePalette.tsx | 9 ++ canvas/src/components/tabs/ChatTab.tsx | 5 +- .../components/tabs/chat/AgentCommsPanel.tsx | 5 +- canvas/src/lib/__tests__/ws-close.test.ts | 85 +++++++++++++++++++ canvas/src/lib/ws-close.ts | 38 +++++++++ canvas/src/store/canvas.ts | 7 ++ 7 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 canvas/src/lib/__tests__/ws-close.test.ts create mode 100644 canvas/src/lib/ws-close.ts diff --git a/canvas/src/components/Legend.tsx b/canvas/src/components/Legend.tsx index ad7ec8fa..10964fd3 100644 --- a/canvas/src/components/Legend.tsx +++ b/canvas/src/components/Legend.tsx @@ -1,12 +1,18 @@ "use client"; import { STATUS_CONFIG } from "@/lib/design-tokens"; +import { useCanvasStore } from "@/store/canvas"; const LEGEND_STATUSES = ["online", "provisioning", "degraded", "failed", "paused", "offline"] as const; export function Legend() { + // TemplatePalette (when open) is fixed top-0 left-0 w-[280px] — the + // default bottom-6 left-4 position of this legend would sit under it. + // Shift past the 280 px palette + a 16 px gap when the palette is open. + const paletteOpen = useCanvasStore((s) => s.templatePaletteOpen); + const leftClass = paletteOpen ? "left-[296px]" : "left-4"; return ( -
+
Legend
{/* Status */} diff --git a/canvas/src/components/TemplatePalette.tsx b/canvas/src/components/TemplatePalette.tsx index 8387f538..2d2b1718 100644 --- a/canvas/src/components/TemplatePalette.tsx +++ b/canvas/src/components/TemplatePalette.tsx @@ -2,6 +2,7 @@ import { useState, useEffect, useCallback, useRef } from "react"; import { api } from "@/lib/api"; +import { useCanvasStore } from "@/store/canvas"; import { checkDeploySecrets, type PreflightResult } from "@/lib/deploy-preflight"; import { MissingKeysModal } from "./MissingKeysModal"; import { ConfirmDialog } from "./ConfirmDialog"; @@ -226,6 +227,14 @@ function ImportAgentButton({ onImported }: { onImported: () => void }) { export function TemplatePalette() { const [open, setOpen] = useState(false); + // Publish palette-open state to the canvas store so Legend (and any + // future floating left-bottom UI) can shift right to avoid being + // hidden behind the 280 px palette drawer. + const setTemplatePaletteOpen = useCanvasStore((s) => s.setTemplatePaletteOpen); + useEffect(() => { + setTemplatePaletteOpen(open); + }, [open, setTemplatePaletteOpen]); + const [templates, setTemplates] = useState([]); const [loading, setLoading] = useState(false); const [creating, setCreating] = useState(null); diff --git a/canvas/src/components/tabs/ChatTab.tsx b/canvas/src/components/tabs/ChatTab.tsx index 719393b1..daf6d48f 100644 --- a/canvas/src/components/tabs/ChatTab.tsx +++ b/canvas/src/components/tabs/ChatTab.tsx @@ -6,6 +6,7 @@ import remarkGfm from "remark-gfm"; import { api } from "@/lib/api"; import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas"; import { WS_URL } from "@/store/socket"; +import { closeWebSocketGracefully } from "@/lib/ws-close"; import { type ChatMessage, createMessage, appendMessageDeduped } from "./chat/types"; import { extractResponseText, extractRequestText } from "./chat/message-parser"; import { AgentCommsPanel } from "./chat/AgentCommsPanel"; @@ -304,7 +305,9 @@ function MyChatPanel({ workspaceId, data }: Props) { } catch { /* ignore */ } }; - return () => ws.close(); + return () => { + closeWebSocketGracefully(ws); + }; }, [sending, workspaceId, resolveWorkspaceName]); const sendMessage = async () => { diff --git a/canvas/src/components/tabs/chat/AgentCommsPanel.tsx b/canvas/src/components/tabs/chat/AgentCommsPanel.tsx index 18a36884..7315e7be 100644 --- a/canvas/src/components/tabs/chat/AgentCommsPanel.tsx +++ b/canvas/src/components/tabs/chat/AgentCommsPanel.tsx @@ -4,6 +4,7 @@ import { useState, useEffect, useRef } from "react"; import { api } from "@/lib/api"; import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas"; import { WS_URL } from "@/store/socket"; +import { closeWebSocketGracefully } from "@/lib/ws-close"; import { extractResponseText, extractRequestText } from "./message-parser"; interface ActivityEntry { @@ -122,7 +123,9 @@ export function AgentCommsPanel({ workspaceId }: { workspaceId: string }) { } } catch { /* ignore */ } }; - return () => ws.close(); + return () => { + closeWebSocketGracefully(ws); + }; }, [workspaceId]); useEffect(() => { diff --git a/canvas/src/lib/__tests__/ws-close.test.ts b/canvas/src/lib/__tests__/ws-close.test.ts new file mode 100644 index 00000000..4bb37991 --- /dev/null +++ b/canvas/src/lib/__tests__/ws-close.test.ts @@ -0,0 +1,85 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi } from "vitest"; +import { closeWebSocketGracefully } from "../ws-close"; + +// Minimal test-double for WebSocket. jsdom doesn't ship a +// spec-compliant WebSocket, so we roll our own with just the bits the +// helper touches: readyState, close(), addEventListener("open") / +// ("error"). This lets us verify the graceful-close semantics without +// a live server. +function makeFakeWS(initialState: number) { + const listeners: Record void>> = {}; + const ws = { + readyState: initialState, + close: vi.fn(), + addEventListener: vi.fn( + (type: string, handler: () => void, _opts?: { once?: boolean }) => { + (listeners[type] ??= []).push(handler); + }, + ), + removeEventListener: vi.fn( + (type: string, handler: () => void) => { + const arr = listeners[type]; + if (!arr) return; + const idx = arr.indexOf(handler); + if (idx >= 0) arr.splice(idx, 1); + }, + ), + // Helpers for tests to fire the queued listeners. + fire(type: string) { + (listeners[type] ?? []).slice().forEach((h) => h()); + }, + }; + return ws as unknown as WebSocket & { fire(type: string): void }; +} + +describe("closeWebSocketGracefully", () => { + it("calls close() immediately when the socket is OPEN", () => { + const ws = makeFakeWS(WebSocket.OPEN); + closeWebSocketGracefully(ws); + expect(ws.close).toHaveBeenCalledOnce(); + }); + + it("calls close() immediately when the socket is CLOSING", () => { + const ws = makeFakeWS(WebSocket.CLOSING); + closeWebSocketGracefully(ws); + expect(ws.close).toHaveBeenCalledOnce(); + }); + + it("is a no-op when the socket is already CLOSED", () => { + const ws = makeFakeWS(WebSocket.CLOSED); + closeWebSocketGracefully(ws); + expect(ws.close).not.toHaveBeenCalled(); + expect(ws.addEventListener).not.toHaveBeenCalled(); + }); + + it("defers close until 'open' when the socket is CONNECTING", () => { + const ws = makeFakeWS(WebSocket.CONNECTING); + closeWebSocketGracefully(ws); + + // close() NOT called yet — handshake hasn't completed. + expect(ws.close).not.toHaveBeenCalled(); + // Two listeners queued: one for 'open' (close on connect), one + // for 'error' (cancel the queued close if handshake fails). + expect(ws.addEventListener).toHaveBeenCalledWith( + "open", expect.any(Function), { once: true }, + ); + expect(ws.addEventListener).toHaveBeenCalledWith( + "error", expect.any(Function), { once: true }, + ); + + // Simulate the handshake completing — close() should fire now. + (ws as unknown as { fire: (t: string) => void }).fire("open"); + expect(ws.close).toHaveBeenCalledOnce(); + }); + + it("does NOT call close() when the CONNECTING socket errors instead of opening", () => { + const ws = makeFakeWS(WebSocket.CONNECTING); + closeWebSocketGracefully(ws); + + // Simulate handshake failure — the browser has already torn the + // socket down, no explicit close() needed. + (ws as unknown as { fire: (t: string) => void }).fire("error"); + expect(ws.close).not.toHaveBeenCalled(); + }); +}); diff --git a/canvas/src/lib/ws-close.ts b/canvas/src/lib/ws-close.ts new file mode 100644 index 00000000..7684ebac --- /dev/null +++ b/canvas/src/lib/ws-close.ts @@ -0,0 +1,38 @@ +/** + * closeWebSocketGracefully closes a WebSocket without tripping the + * browser console warning "WebSocket is closed before the connection is + * established". That warning fires when `ws.close()` runs while + * readyState is still CONNECTING (0) — most often triggered by React + * StrictMode's double-invoked useEffect in dev, or any rapid + * mount/unmount (tab switch, route change) during the WS handshake. + * + * Behaviour by state: + * - OPEN / CLOSING: close immediately (the normal path). + * - CONNECTING: defer the close until 'open' fires, so the + * browser sees a full handshake before the shutdown. + * - CLOSED: no-op. + * + * Returns the ws unchanged for chaining. + */ +export function closeWebSocketGracefully(ws: WebSocket): WebSocket { + const state = ws.readyState; + if (state === WebSocket.OPEN || state === WebSocket.CLOSING) { + ws.close(); + return ws; + } + if (state === WebSocket.CONNECTING) { + const onOpen = () => { + ws.close(); + }; + ws.addEventListener("open", onOpen, { once: true }); + // Also wire an error listener — if the handshake fails we don't + // need to close (the browser already tore it down) and we should + // clear the queued onOpen handler. + ws.addEventListener( + "error", + () => ws.removeEventListener("open", onOpen), + { once: true }, + ); + } + return ws; +} diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index e6f6f28a..8527be4d 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -58,6 +58,11 @@ interface CanvasState { // hidden behind the panel when a workspace is selected. sidePanelWidth: number; setSidePanelWidth: (w: number) => void; + // Whether the TemplatePalette left-drawer is open. Consumed by the + // Legend so it can shift right and avoid being hidden under the + // palette. Set by TemplatePalette's toggle button. + templatePaletteOpen: boolean; + setTemplatePaletteOpen: (open: boolean) => void; hydrate: (workspaces: WorkspaceData[]) => void; applyEvent: (msg: WSMessage) => void; onNodesChange: (changes: NodeChange>[]) => void; @@ -124,6 +129,8 @@ export const useCanvasStore = create((set, get) => ({ contextMenu: null, sidePanelWidth: 480, // matches SIDEPANEL_DEFAULT_WIDTH in SidePanel.tsx setSidePanelWidth: (w) => set({ sidePanelWidth: w }), + templatePaletteOpen: false, + setTemplatePaletteOpen: (open) => set({ templatePaletteOpen: open }), // Batch selection selectedNodeIds: new Set(), toggleNodeSelection: (id) => {