forked from molecule-ai/molecule-core
fix(canvas): Legend avoids TemplatePalette + silence WS handshake races
### 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) <noreply@anthropic.com>
This commit is contained in:
parent
5eb5e38c59
commit
b4719ad070
@ -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 (
|
||||
<div className="fixed bottom-6 left-4 z-30 bg-zinc-900/95 border border-zinc-700/50 rounded-xl px-4 py-3 shadow-xl shadow-black/30 backdrop-blur-sm max-w-[280px]">
|
||||
<div className={`fixed bottom-6 ${leftClass} z-30 bg-zinc-900/95 border border-zinc-700/50 rounded-xl px-4 py-3 shadow-xl shadow-black/30 backdrop-blur-sm max-w-[280px] transition-[left] duration-200`}>
|
||||
<div className="text-[11px] font-semibold text-zinc-400 uppercase tracking-wider mb-2">Legend</div>
|
||||
|
||||
{/* Status */}
|
||||
|
||||
@ -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<Template[]>([]);
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [creating, setCreating] = useState<string | null>(null);
|
||||
|
||||
@ -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 () => {
|
||||
|
||||
@ -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(() => {
|
||||
|
||||
85
canvas/src/lib/__tests__/ws-close.test.ts
Normal file
85
canvas/src/lib/__tests__/ws-close.test.ts
Normal file
@ -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<string, Array<() => 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();
|
||||
});
|
||||
});
|
||||
38
canvas/src/lib/ws-close.ts
Normal file
38
canvas/src/lib/ws-close.ts
Normal file
@ -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;
|
||||
}
|
||||
@ -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<Node<WorkspaceNodeData>>[]) => void;
|
||||
@ -124,6 +129,8 @@ export const useCanvasStore = create<CanvasState>((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<string>(),
|
||||
toggleNodeSelection: (id) => {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user