From 473bd958264b40ab76c5963d56f22493ed3304d4 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 25 Apr 2026 19:05:18 -0400 Subject: [PATCH] Address PR #1221 review round 3 feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - InspectorView: simulate the connect flow through "connecting" with a randomized 50-500ms handshake delay and an 85% success rate. On success, measure observed latency from connect start; on failure, set a plausible error message. Connection error message flows through to the active ServerCard's connection state. Real wiring (Phase 3) replaces this with `useConnection`. - InspectorView: document that the view owns connection state (input `serversInput` connection fields are intentionally ignored), and mark `noop` callbacks as Phase-3 stubs. - ServerStatusIndicator: add `showLabel?: boolean` escape hatch so consumers in constrained containers can override the default viewport-based visibility. - ServerListScreen.stories: add `WithActiveServer` story exercising the dimming + `inert` behaviour in isolation. - ViewHeader.stories: rename "Logging" → "Logs" to match `ALL_TABS` in InspectorView. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ServerStatusIndicator.tsx | 8 +- .../groups/ViewHeader/ViewHeader.stories.tsx | 2 +- .../ServerListScreen.stories.tsx | 7 ++ .../views/InspectorView/InspectorView.tsx | 97 +++++++++++++++---- 4 files changed, 93 insertions(+), 21 deletions(-) diff --git a/clients/web/src/components/elements/ServerStatusIndicator/ServerStatusIndicator.tsx b/clients/web/src/components/elements/ServerStatusIndicator/ServerStatusIndicator.tsx index cb6afb45..fc4858e2 100644 --- a/clients/web/src/components/elements/ServerStatusIndicator/ServerStatusIndicator.tsx +++ b/clients/web/src/components/elements/ServerStatusIndicator/ServerStatusIndicator.tsx @@ -6,6 +6,10 @@ export interface ServerStatusIndicatorProps { status: ConnectionStatus; latencyMs?: number; retryCount?: number; + // Override for the default viewport-based label visibility. Useful when + // the indicator renders inside a constrained container (sidebar, card) + // where the global media query doesn't reflect available space. + showLabel?: boolean; } const statusColorVar: Record = { @@ -42,8 +46,10 @@ export function ServerStatusIndicator({ status, latencyMs, retryCount, + showLabel: showLabelProp, }: ServerStatusIndicatorProps) { - const showLabel = useMediaQuery("(min-width: 1200px)"); + const wideViewport = useMediaQuery("(min-width: 1200px)"); + const showLabel = showLabelProp ?? wideViewport; const label = getLabel(status, latencyMs, retryCount); return ( diff --git a/clients/web/src/components/groups/ViewHeader/ViewHeader.stories.tsx b/clients/web/src/components/groups/ViewHeader/ViewHeader.stories.tsx index b606faf0..e5c1c4a6 100644 --- a/clients/web/src/components/groups/ViewHeader/ViewHeader.stories.tsx +++ b/clients/web/src/components/groups/ViewHeader/ViewHeader.stories.tsx @@ -32,7 +32,7 @@ export const Connected: Story = { "Resources", "Prompts", "Tasks", - "Logging", + "Logs", "History", ], onTabChange: fn(), diff --git a/clients/web/src/components/screens/ServerListScreen/ServerListScreen.stories.tsx b/clients/web/src/components/screens/ServerListScreen/ServerListScreen.stories.tsx index 02ba67d9..a5177a14 100644 --- a/clients/web/src/components/screens/ServerListScreen/ServerListScreen.stories.tsx +++ b/clients/web/src/components/screens/ServerListScreen/ServerListScreen.stories.tsx @@ -98,3 +98,10 @@ export const MixedStates: Story = { ], }, }; + +export const WithActiveServer: Story = { + args: { + servers: [connectedStdioServer, disconnectedStdioServer, failedHttpServer], + activeServer: connectedStdioServer.id, + }, +}; diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index 6bfa3859..0371d6e9 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -40,8 +40,24 @@ const ALL_TABS: string[] = [ "History", ]; +// Demo stub: every screen-action callback below resolves to noop. Phase 3 +// wiring will replace each with its real `useManaged*` / `useConnection` +// hook call. Anything still pointing here in Phase 3 is unfinished. const noop = () => undefined; +// Demo stub: simulated handshake delays and a sample of plausible failure +// reasons. Replace with real handshake telemetry once `useConnection` is wired. +const STUB_MIN_DELAY_MS = 50; +const STUB_MAX_DELAY_MS = 500; +const STUB_SUCCESS_RATE = 0.85; +const STUB_ERROR_MESSAGES = [ + "Connection refused", + "Handshake timeout", + "Protocol version mismatch", + "Authentication required", + "Server returned invalid response", +]; + export interface InspectorViewProps { servers: ServerEntry[]; tools: Tool[]; @@ -78,24 +94,36 @@ export function InspectorView({ const [connectionStatus, setConnectionStatus] = useState("disconnected"); const [latencyMs, setLatencyMs] = useState(undefined); + const [errorMessage, setErrorMessage] = useState( + undefined, + ); const [activeTab, setActiveTab] = useState(SERVERS_TAB); const [availableTabs, setAvailableTabs] = useState([SERVERS_TAB]); const [logLevel, setLogLevel] = useState("info"); const [autoScroll, setAutoScroll] = useState(true); - // Per-card connection state derives from "is this the active server, - // and what's our current global connection status?". Cards that aren't - // the active server always render as disconnected. + // The view is the single source of truth for connection state. Any + // `connection` field on incoming `serversInput` items is intentionally + // ignored — cards mirror the global `connectionStatus` for the active + // server and render as `disconnected` otherwise. const servers = useMemo( () => - serversInput.map((s) => ({ - ...s, - connection: - s.id === activeServer - ? { status: connectionStatus } - : { status: "disconnected" }, - })), - [serversInput, activeServer, connectionStatus], + serversInput.map((s) => { + if (s.id !== activeServer) { + return { ...s, connection: { status: "disconnected" } }; + } + if (connectionStatus === "error" && errorMessage) { + return { + ...s, + connection: { + status: "error", + error: { message: errorMessage }, + }, + }; + } + return { ...s, connection: { status: connectionStatus } }; + }), + [serversInput, activeServer, connectionStatus, errorMessage], ); function disconnect() { @@ -103,10 +131,16 @@ export function InspectorView({ setConnectionStatus("disconnected"); setInitializeResult(undefined); setLatencyMs(undefined); + setErrorMessage(undefined); setAvailableTabs([SERVERS_TAB]); setActiveTab(SERVERS_TAB); } + // Demo stub: simulates the full connect → connecting → connected/error + // transition with a randomized handshake delay. Real wiring will dispatch + // `useConnection.connect(id)` and let the hook drive these state changes. + // The `protocolVersion`, `capabilities`, and `serverInfo` populated below + // are placeholders until a real `InitializeResult` arrives from the server. function handleToggleConnection(id: string) { if (id === activeServer && connectionStatus === "connected") { disconnect(); @@ -114,15 +148,40 @@ export function InspectorView({ } const target = serversInput.find((s) => s.id === id); if (!target) return; + + // Capture `start` at the "connecting" edge and compute observed + // latency at the "connected" edge. Both edges are owned by this + // handler so the timing is deterministic — no useEffect chain needed + // (which would otherwise trip `react-hooks/set-state-in-effect`). + const start = Date.now(); setActiveServer(id); - setConnectionStatus("connected"); - setInitializeResult({ - protocolVersion: "2025-06-18", - capabilities: {}, - serverInfo: target.info ?? { name: target.name, version: "0.0.0" }, - }); - setLatencyMs(42); - setAvailableTabs(ALL_TABS); + setLatencyMs(undefined); + setErrorMessage(undefined); + setConnectionStatus("connecting"); + + const delay = + STUB_MIN_DELAY_MS + + Math.floor(Math.random() * (STUB_MAX_DELAY_MS - STUB_MIN_DELAY_MS + 1)); + + window.setTimeout(() => { + if (Math.random() < STUB_SUCCESS_RATE) { + setInitializeResult({ + protocolVersion: "2025-06-18", + capabilities: {}, + serverInfo: target.info ?? { name: target.name, version: "0.0.0" }, + }); + setAvailableTabs(ALL_TABS); + setLatencyMs(Date.now() - start); + setConnectionStatus("connected"); + } else { + setErrorMessage( + STUB_ERROR_MESSAGES[ + Math.floor(Math.random() * STUB_ERROR_MESSAGES.length) + ], + ); + setConnectionStatus("error"); + } + }, delay); } return (