refactor(web): address review — scope failure UX to connect attempts, dedupe timing (#1621)

Review feedback on #1622:

- #1 (medium): `connectionStatus === "error"` also covers a mid-session
  crash of a previously-connected server, not just a failed connect
  attempt. Scope the auto-open + failure tab set to connect attempts only:
  the auto-open effect's error arm now requires the previous status to not
  be `connected` (connecting/disconnected → error), and `failed` (render)
  requires `erroredServerId`, which the parent sets only for connect-
  attempt failures. A mid-session crash now closes the column like any
  teardown instead of reopening a user-closed column / dropping the live
  Logs tab. This also aligns the column behavior with the red border.
- #3 (minor): extract the column slide duration into a shared
  `monitorColumnAnimation.ts` (MONITOR_COLUMN_ANIM_MS +
  FAILED_CARD_SCROLL_DELAY_MS) so InspectorView's Transition and
  ServerCard's scroll delay can't drift.
- #4 (nit): composite key for ConsoleScreen entry rows instead of the
  array index.

Added a mid-session-crash test (connected → error with no erroredServerId
closes the column). Verified live: a connect failure still opens the
column to Console. Full `npm run ci` passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw

clients/web/src/components/groups/ServerCard/ServerCard.tsx

@@ -9,6 +9,7 @@ import { ServerStatusIndicator } from "../../elements/ServerStatusIndicator/Serv
 import { TransportBadge } from "../../elements/TransportBadge/TransportBadge";
 import { ConnectionToggle } from "../../elements/ConnectionToggle/ConnectionToggle";
 import { ContentViewer } from "../../elements/ContentViewer/ContentViewer";
+import { FAILED_CARD_SCROLL_DELAY_MS } from "../../views/InspectorView/monitorColumnAnimation";
 
 export interface ServerCardProps extends ServerEntry {
   activeServer?: string;
@@ -101,14 +102,6 @@ const RemoveButton = Button.withProps({
   color: "red.6",
 });
 
-// Delay before scrolling a newly-failed card into view (#1621). A connection
-// failure both flags the card and opens the monitoring column, which reflows
-// the server grid (fewer, narrower columns) and can push the failed card below
-// the fold. Deferring past the column's open lets that reflow settle so
-// `scrollIntoView` targets the card's final position rather than its pre-reflow
-// one. Kept just above the column's slide duration.
-const ERROR_SCROLL_DELAY_MS = 320;
-
 function getTransport(config: MCPServerConfig): ServerType {
   return config.type ?? "stdio";
 }
@@ -165,9 +158,11 @@ export function ServerCard({
   }, [highlighted, scrollOnHighlight]);
 
   // Scroll the failed card into view on the disconnected→errored transition
-  // (#1621), deferred so the monitoring column's open + grid reflow settle
-  // first. Guarded by a ref so a re-render while still errored doesn't re-scroll
-  // and fight the user if they've scrolled away.
+  // (#1621), deferred past the monitoring column's open (`FAILED_CARD_SCROLL_
+  // DELAY_MS`, derived from the column's slide duration) so the grid reflow
+  // settles before `scrollIntoView` measures the card. Guarded by a ref so a
+  // re-render while still errored doesn't re-scroll and fight the user if they've
+  // scrolled away.
   const wasErroredRef = useRef(errored);
   useEffect(() => {
     const justErrored = errored && !wasErroredRef.current;
@@ -175,7 +170,7 @@ export function ServerCard({
     if (!justErrored) return;
     const timer = setTimeout(() => {
       rootRef.current?.scrollIntoView({ behavior: "smooth", block: "center" });
-    }, ERROR_SCROLL_DELAY_MS);
+    }, FAILED_CARD_SCROLL_DELAY_MS);
     return () => clearTimeout(timer);
   }, [errored]);
   const transport = getTransport(config);

clients/web/src/components/screens/ConsoleScreen/ConsoleScreen.tsx

@@ -183,7 +183,7 @@ export function ConsoleScreen({
           <EmbeddableScrollArea embedded={embedded} viewportRef={viewportRef}>
             <Stack gap="xs">
               {filteredEntries.map((entry, index) => (
-                <EntryRow key={index}>
+                <EntryRow key={`${entry.timestamp.getTime()}-${index}`}>
                   <TimestampText>
                     {formatTimestamp(entry.timestamp)}
                   </TimestampText>

clients/web/src/components/views/InspectorView/InspectorView.test.tsx

@@ -1383,11 +1383,14 @@ describe("InspectorView", () => {
     });
 
     // A failed connect leaves the errored server's id set but no
-    // initializeResult (capabilities were never negotiated).
+    // initializeResult (capabilities were never negotiated). `erroredServerId`
+    // is the parent's connect-attempt-failure signal (it survives the failure's
+    // `disconnect` clearing `activeServer`), which gates the failure column.
     function failedHttp(overrides: Partial<InspectorViewProps> = {}) {
       return makeProps({
         servers: [httpServer],
         activeServer: "beta",
+        erroredServerId: "beta",
         connectionStatus: "error",
         initializeResult: undefined,
         ...overrides,
@@ -1498,6 +1501,41 @@ describe("InspectorView", () => {
       ).toBeNull();
     });
 
+    it("does not treat a mid-session crash as a connect-attempt failure (#1621)", async () => {
+      // A previously-connected server that crashes settles to `error` too, but
+      // with no `erroredServerId` (the parent sets that only for connect
+      // attempts). That must NOT reorganize the column into the failure tab set
+      // — it closes like any session teardown, so a user who closed the column
+      // mid-session isn't reopened onto diagnostics.
+      monitorWide.value = true;
+      window.localStorage.setItem("inspector.monitor.pinned", "true");
+      const { rerender } = renderWithMantine(
+        <StatefulInspectorViewHost {...connectedHttp()} />,
+      );
+      // Connected + pinned: the column is open with the live monitor tabs.
+      expect(
+        await screen.findByRole("button", { name: "Close monitoring column" }),
+      ).toBeInTheDocument();
+
+      // Crash: connected → error, activeServer cleared, NO erroredServerId.
+      rerender(
+        <StatefulInspectorViewHost
+          {...makeProps({
+            servers: [httpServer],
+            activeServer: undefined,
+            connectionStatus: "error",
+          })}
+        />,
+      );
+      // The column closes (after its slide-out) rather than switching to the
+      // failure tabs.
+      await waitFor(() =>
+        expect(
+          screen.queryByRole("button", { name: "Close monitoring column" }),
+        ).toBeNull(),
+      );
+    });
+
     const stdioServer: ServerEntry = {
       id: "gamma",
       name: "Gamma",

clients/web/src/components/views/InspectorView/InspectorView.tsx

@@ -88,6 +88,7 @@ import { MonitoringScreen } from "../../groups/MonitoringScreen/MonitoringScreen
 import { ResizeHandle } from "../../elements/ResizeHandle/ResizeHandle";
 import { getServerType } from "@inspector/core/mcp/config.js";
 import { INSPECTOR_SERVERS_TAB } from "../../../utils/inspectorTabs";
+import { MONITOR_COLUMN_ANIM_MS } from "./monitorColumnAnimation";
 
 const SORT_DEFAULT: SortDirection = "newest-first";
 
@@ -283,8 +284,9 @@ const MonitoringColumn = Stack.withProps({
 // side column sliding open rather than snapping in. The primary screen keeps its
 // standard `ScreenStage` transition. Mantine plays `out → in` on enter and
 // `in → out` on exit. `AppShell.Main`'s `overflow: hidden` clips the off-screen
-// portion during the slide.
-const COLUMN_ANIM_MS = 300;
+// portion during the slide. The duration is shared (`ServerCard` waits on it
+// before scrolling a failed card into view) so the two can't drift.
+const COLUMN_ANIM_MS = MONITOR_COLUMN_ANIM_MS;
 const columnSlide: MantineTransition = {
   in: { opacity: 1, transform: "translateX(0)" },
   out: { opacity: 0, transform: "translateX(100%)" },
@@ -695,22 +697,30 @@ export function InspectorView({
   });
 
   // Open the monitoring column when a connection is established (#1616) OR when a
-  // connection attempt fails (#1621). Gated on the *transition into* the target
+  // connect *attempt* fails (#1621). Gated on the *transition into* the target
   // status (via the ref) rather than the status itself, so it fires once on an
   // actual connect/failure — not on every render, and not on a mount that starts
   // already in that status (which would fight a user who closed it). On success
-  // the column surfaces the live stream; on failure it surfaces the History /
-  // Network entries that explain what went wrong. The column still only *appears*
-  // when wide + a monitor tab is available (`effectivePinned`); this just sets the
-  // preference. Closing it stays closed until the next connect/failure, since this
-  // effect only re-runs when `connectionStatus` changes.
+  // the column surfaces the live stream; on failure it surfaces the diagnostics
+  // that explain what went wrong. The column still only *appears* when wide + a
+  // monitor tab is available (`effectivePinned`); this just sets the preference.
+  //
+  // `"error"` is also the resting status of a *mid-session crash* of a
+  // previously-connected server (per isTerminalStatus/#1490), but that is NOT a
+  // connect attempt: reopening a column the user closed mid-session (and swapping
+  // their live tab set for the failure set) would be surprising. So the error
+  // arm requires the previous status to NOT be `connected` — i.e. we went
+  // connecting/disconnected → error, never connected → error. This keeps the
+  // auto-open aligned with the red-border (`erroredServerId`), which the parent
+  // also sets only for connect-attempt failures.
   const prevStatusRef = useRef(connectionStatus);
   useEffect(() => {
     const prev = prevStatusRef.current;
     prevStatusRef.current = connectionStatus;
     const becameConnected =
       connectionStatus === "connected" && prev !== "connected";
-    const becameError = connectionStatus === "error" && prev !== "error";
+    const becameError =
+      connectionStatus === "error" && prev !== "error" && prev !== "connected";
     if (becameConnected || becameError) {
       setMonitorPinned(true);
     }
@@ -797,9 +807,12 @@ export function InspectorView({
   // `monitorPinned`, so it re-opens when the condition returns. Only the column's
   // close button writes `monitorPinned = false`.
   const connected = connectionStatus === "connected";
-  // A failed connection attempt (#1621) keeps the column available so the user
-  // can see why, even though no live session exists.
-  const failed = connectionStatus === "error";
+  // A failed connection *attempt* (#1621) keeps the column available so the user
+  // can see why, even though no live session exists. Gated on `erroredServerId`
+  // (set by the parent only for connect-attempt failures, not mid-session
+  // crashes) so a crash of a previously-connected server doesn't reorganize the
+  // column into the failure tab set — matching the auto-open effect above.
+  const failed = connectionStatus === "error" && erroredServerId !== undefined;
   const monitorAvailable = useMemo<string[]>(() => {
     if (connected) return availableTabs.filter((t) => MONITOR_TABS.includes(t));
     if (failed) {

clients/web/src/components/views/InspectorView/monitorColumnAnimation.ts

@@ -0,0 +1,15 @@
+/**
+ * Duration (ms) of the monitoring column's open/close slide (#1616). Shared so
+ * dependent timings can't silently drift from the animation:
+ *   - `InspectorView` drives the column's Mantine `Transition` with it.
+ *   - `ServerCard` waits just past it before scrolling a newly-failed card into
+ *     view (#1621), so the column's open + the resulting grid reflow settle
+ *     before `scrollIntoView` measures the card's position.
+ */
+export const MONITOR_COLUMN_ANIM_MS = 300;
+
+/**
+ * Delay (ms) before scrolling a newly-failed server card into view — the column
+ * animation plus a small buffer for the reflow to commit.
+ */
+export const FAILED_CARD_SCROLL_DELAY_MS = MONITOR_COLUMN_ANIM_MS + 20;